From: Samuel Holland <samuel@sholland.org>
To: Chen-Yu Tsai <wens@csie.org>
Cc: Sebastian Reichel <sre@kernel.org>,
Lee Jones <lee.jones@linaro.org>,
Hans de Goede <hdegoede@redhat.com>,
Oskari Lemmela <oskari@lemmela.net>,
"open list:THERMAL" <linux-pm@vger.kernel.org>,
linux-kernel <linux-kernel@vger.kernel.org>,
linux-sunxi <linux-sunxi@googlegroups.com>
Subject: Re: [linux-sunxi] [PATCH v2 6/9] power: supply: axp20x_usb_power: Use a match structure
Date: Sun, 5 Jan 2020 11:58:57 -0600 [thread overview]
Message-ID: <f40d7523-9eac-619c-1621-42b35c6cefbc@sholland.org> (raw)
In-Reply-To: <CAGb2v67rudw=3x9QBxeSqmMN-Qrt4+=3VD0u6H7JeS4KT8caqg@mail.gmail.com>
On 1/5/20 4:34 AM, Chen-Yu Tsai wrote:
> 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.
Will do for v3.
>> 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.
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 <wens@csie.org>
>
next prev parent reply other threads:[~2020-01-05 17:59 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-05 1:24 [PATCH v2 0/9] X-Powers Power Supply Improvements Samuel Holland
2020-01-05 1:24 ` [PATCH v2 1/9] mfd: axp20x: Mark AXP20X_VBUS_IPSOUT_MGMT as volatile Samuel Holland
2020-01-05 10:07 ` Chen-Yu Tsai
2020-01-06 8:36 ` Lee Jones
2020-01-05 1:24 ` [PATCH v2 2/9] power: supply: axp20x_ac_power: Fix reporting online status Samuel Holland
2020-01-05 10:09 ` [linux-sunxi] " Chen-Yu Tsai
2020-01-05 13:00 ` Julian Calaby
2020-01-05 15:27 ` Samuel Holland
2020-01-05 1:24 ` [PATCH v2 3/9] power: supply: axp20x_ac_power: Allow offlining Samuel Holland
2020-01-05 10:11 ` [linux-sunxi] " Chen-Yu Tsai
2020-01-05 1:24 ` [PATCH v2 4/9] power: supply: axp20x_ac_power: Add wakeup control Samuel Holland
2020-01-05 10:24 ` [linux-sunxi] " Chen-Yu Tsai
2020-01-05 10:44 ` Chen-Yu Tsai
2020-01-05 1:24 ` [PATCH v2 5/9] power: supply: axp20x_usb_power: Remove unused device_node Samuel Holland
2020-01-05 10:25 ` Chen-Yu Tsai
2020-01-05 1:24 ` [PATCH v2 6/9] power: supply: axp20x_usb_power: Use a match structure Samuel Holland
2020-01-05 10:34 ` [linux-sunxi] " Chen-Yu Tsai
2020-01-05 17:58 ` Samuel Holland [this message]
2020-01-06 2:24 ` Chen-Yu Tsai
2020-01-05 1:24 ` [PATCH v2 7/9] power: supply: axp20x_usb_power: Allow offlining Samuel Holland
2020-01-05 10:40 ` [linux-sunxi] " Chen-Yu Tsai
2020-01-05 17:47 ` Samuel Holland
2020-01-06 2:22 ` Chen-Yu Tsai
2020-01-05 1:24 ` [PATCH v2 8/9] power: supply: axp20x_usb_power: Add wakeup control Samuel Holland
2020-01-05 10:47 ` [linux-sunxi] " Chen-Yu Tsai
2020-01-05 1:24 ` [PATCH v2 9/9] power: supply: axp20x_usb_power: Only poll while offline Samuel Holland
2020-01-06 4:27 ` [linux-sunxi] " Chen-Yu Tsai
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f40d7523-9eac-619c-1621-42b35c6cefbc@sholland.org \
--to=samuel@sholland.org \
--cc=hdegoede@redhat.com \
--cc=lee.jones@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=linux-sunxi@googlegroups.com \
--cc=oskari@lemmela.net \
--cc=sre@kernel.org \
--cc=wens@csie.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).