On 1/5/20 4:34 AM, Chen-Yu Tsai wrote: > On Sun, Jan 5, 2020 at 9:24 AM Samuel Holland <[email protected]> wrote: >> >> Instead of ad-hoc variant ID checks throughout the code, let's start >> moving the variant-specific details to a match structure. This allows >> for future flexibility, and it better matches the other axp20x power >> supply drivers. > > You should probably mention that there are still parts of the code > where ID matching is done.
Will do for v3. >> Signed-off-by: Samuel Holland <[email protected]> >> --- >> drivers/power/supply/axp20x_usb_power.c | 91 ++++++++++++++++--------- >> 1 file changed, 60 insertions(+), 31 deletions(-) >> >> diff --git a/drivers/power/supply/axp20x_usb_power.c >> b/drivers/power/supply/axp20x_usb_power.c >> index dd3f3f12e41d..2d7272e19a87 100644 >> --- a/drivers/power/supply/axp20x_usb_power.c >> +++ b/drivers/power/supply/axp20x_usb_power.c >> @@ -405,6 +405,50 @@ static const struct power_supply_desc >> axp22x_usb_power_desc = { >> .set_property = axp20x_usb_power_set_property, >> }; >> >> +static const char * const axp20x_irq_names[] = { >> + "VBUS_PLUGIN", >> + "VBUS_REMOVAL", >> + "VBUS_VALID", >> + "VBUS_NOT_VALID", >> + NULL >> +}; >> + >> +static const char * const axp22x_irq_names[] = { >> + "VBUS_PLUGIN", >> + "VBUS_REMOVAL", >> + NULL >> +}; >> + >> +struct axp_data { >> + const struct power_supply_desc *power_desc; >> + const char * const *irq_names; >> + enum axp20x_variants axp20x_id; >> +}; >> + >> +static const struct axp_data axp202_data = { >> + .power_desc = &axp20x_usb_power_desc, >> + .irq_names = axp20x_irq_names, >> + .axp20x_id = AXP202_ID, >> +}; >> + >> +static const struct axp_data axp221_data = { >> + .power_desc = &axp22x_usb_power_desc, >> + .irq_names = axp22x_irq_names, >> + .axp20x_id = AXP221_ID, >> +}; >> + >> +static const struct axp_data axp223_data = { >> + .power_desc = &axp22x_usb_power_desc, >> + .irq_names = axp22x_irq_names, >> + .axp20x_id = AXP223_ID, >> +}; >> + >> +static const struct axp_data axp813_data = { >> + .power_desc = &axp22x_usb_power_desc, >> + .irq_names = axp22x_irq_names, >> + .axp20x_id = AXP813_ID, >> +}; >> + >> static int configure_iio_channels(struct platform_device *pdev, >> struct axp20x_usb_power *power) >> { >> @@ -440,12 +484,7 @@ static int axp20x_usb_power_probe(struct >> platform_device *pdev) >> struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent); >> struct power_supply_config psy_cfg = {}; >> struct axp20x_usb_power *power; >> - static const char * const axp20x_irq_names[] = { "VBUS_PLUGIN", >> - "VBUS_REMOVAL", "VBUS_VALID", "VBUS_NOT_VALID", NULL }; >> - static const char * const axp22x_irq_names[] = { >> - "VBUS_PLUGIN", "VBUS_REMOVAL", NULL }; >> - const char * const *irq_names; >> - const struct power_supply_desc *usb_power_desc; >> + const struct axp_data *axp_data; >> int i, irq, ret; >> >> if (!of_device_is_available(pdev->dev.of_node)) >> @@ -456,15 +495,16 @@ static int axp20x_usb_power_probe(struct >> platform_device *pdev) >> return -EINVAL; >> } >> >> + axp_data = of_device_get_match_data(&pdev->dev); >> + >> power = devm_kzalloc(&pdev->dev, sizeof(*power), GFP_KERNEL); >> if (!power) >> return -ENOMEM; >> >> - platform_set_drvdata(pdev, power); >> - power->axp20x_id = (enum axp20x_variants)of_device_get_match_data( >> - &pdev->dev); >> - >> power->regmap = axp20x->regmap; >> + power->axp20x_id = axp_data->axp20x_id; >> + >> + platform_set_drvdata(pdev, power); > > Not sure why this needs to be reordered. It doesn't necessarily; moving the call to platform_set_drvdata() matches the order in axp20x_ac_power, which makes it easier to compare the two probe functions. I can drop it for v3 if you prefer. >> if (power->axp20x_id == AXP202_ID) { >> /* Enable vbus valid checking */ >> @@ -481,18 +521,6 @@ static int axp20x_usb_power_probe(struct >> platform_device *pdev) >> >> if (ret) >> return ret; >> - >> - usb_power_desc = &axp20x_usb_power_desc; >> - irq_names = axp20x_irq_names; >> - } else if (power->axp20x_id == AXP221_ID || >> - power->axp20x_id == AXP223_ID || >> - power->axp20x_id == AXP813_ID) { >> - usb_power_desc = &axp22x_usb_power_desc; >> - irq_names = axp22x_irq_names; >> - } else { >> - dev_err(&pdev->dev, "Unsupported AXP variant: %ld\n", >> - axp20x->variant); >> - return -EINVAL; >> } >> >> if (power->axp20x_id == AXP813_ID) { >> @@ -504,17 +532,18 @@ static int axp20x_usb_power_probe(struct >> platform_device *pdev) >> psy_cfg.of_node = pdev->dev.of_node; >> psy_cfg.drv_data = power; >> >> - power->supply = devm_power_supply_register(&pdev->dev, >> usb_power_desc, >> + power->supply = devm_power_supply_register(&pdev->dev, >> + axp_data->power_desc, >> &psy_cfg); >> if (IS_ERR(power->supply)) >> return PTR_ERR(power->supply); >> >> /* Request irqs after registering, as irqs may trigger immediately */ >> - for (i = 0; irq_names[i]; i++) { >> - irq = platform_get_irq_byname(pdev, irq_names[i]); >> + for (i = 0; axp_data->irq_names[i]; i++) { >> + irq = platform_get_irq_byname(pdev, axp_data->irq_names[i]); >> if (irq < 0) { >> dev_warn(&pdev->dev, "No IRQ for %s: %d\n", >> - irq_names[i], irq); >> + axp_data->irq_names[i], irq); >> continue; >> } >> irq = regmap_irq_get_virq(axp20x->regmap_irqc, irq); >> @@ -522,7 +551,7 @@ static int axp20x_usb_power_probe(struct platform_device >> *pdev) >> axp20x_usb_power_irq, 0, DRVNAME, power); >> if (ret < 0) >> dev_warn(&pdev->dev, "Error requesting %s IRQ: %d\n", >> - irq_names[i], ret); >> + axp_data->irq_names[i], ret); >> } >> >> INIT_DELAYED_WORK(&power->vbus_detect, axp20x_usb_power_poll_vbus); >> @@ -544,16 +573,16 @@ static int axp20x_usb_power_remove(struct >> platform_device *pdev) >> static const struct of_device_id axp20x_usb_power_match[] = { >> { >> .compatible = "x-powers,axp202-usb-power-supply", >> - .data = (void *)AXP202_ID, >> + .data = &axp202_data, >> }, { >> .compatible = "x-powers,axp221-usb-power-supply", >> - .data = (void *)AXP221_ID, >> + .data = &axp221_data, >> }, { >> .compatible = "x-powers,axp223-usb-power-supply", >> - .data = (void *)AXP223_ID, >> + .data = &axp223_data, >> }, { >> .compatible = "x-powers,axp813-usb-power-supply", >> - .data = (void *)AXP813_ID, >> + .data = &axp813_data, >> }, { /* sentinel */ } >> }; >> MODULE_DEVICE_TABLE(of, axp20x_usb_power_match); >> -- >> 2.23.0 > > Otherwise, > > Reviewed-by: Chen-Yu Tsai <[email protected]> > -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/f40d7523-9eac-619c-1621-42b35c6cefbc%40sholland.org.
