Messages in this thread |  | | From | Chen-Yu Tsai <> | Date | Sun, 5 Jan 2020 18:34:46 +0800 | Subject | Re: [linux-sunxi] [PATCH v2 6/9] power: supply: axp20x_usb_power: Use a match structure |
| |
On Sun, Jan 5, 2020 at 9:24 AM Samuel Holland <samuel@sholland.org> 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.
> Signed-off-by: Samuel Holland <samuel@sholland.org> > --- > 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.
> 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 <wens@csie.org>
|  |