From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
To: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Andy Gross <agross@kernel.org>, Lee Jones <lee.jones@linaro.org>,
David Heidelberg <david@ixit.cz>,
linux-arm-msm@vger.kernel.org,
Linus Walleij <linus.walleij@linaro.org>
Subject: Re: [PATCH] mfd: qcom-pm8xxx: switch away from using chained IRQ handlers
Date: Tue, 21 Sep 2021 23:47:53 +0300 [thread overview]
Message-ID: <c1988694-9499-a7c4-1156-99699e20eba0@linaro.org> (raw)
In-Reply-To: <YUo3UGfbdbuXrOyy@builder.lan>
On 21/09/2021 22:49, Bjorn Andersson wrote:
> On Tue 21 Sep 11:24 CDT 2021, Dmitry Baryshkov wrote:
>
>> PM8xxx PMIC family uses GPIO as parent IRQ. Using it together with the
>> irq_set_chained_handler_and_data() results in warnings from the GPIOLIB
>> as in this path the IRQ resources are not allocated (and thus the
>> corresponding GPIO is not marked as used for the IRQ. Use request_irq so
>> that the IRQ resources are proprely setup.
>>
>
> It's been a while since I booted a platform with ssbi PMIC (i.e. 8064
> and older), but I don't remember ever seeing a warning.
I've got Nexus 7 as an example of MDP4 hardware. The warning is more or
less recent: from 2019 if I remember correctly.
>
> Can you please include in the commit message what warning this is and
> perhaps a reference to what in GPIOLIB changed to introduce this - in
> particular so that this fix can be found when people are searching the
> archive looking for similar issues.
Ack.
> And I think it would be nice to Cc Linus Walleij.
Ack.
>
>
> Change looks good though, so please update the commit message and add
> my:
>
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
>
> Regards,
> Bjorn
>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>> drivers/mfd/qcom-pm8xxx.c | 39 ++++++++++++++++-----------------------
>> 1 file changed, 16 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/mfd/qcom-pm8xxx.c b/drivers/mfd/qcom-pm8xxx.c
>> index ec18a04de355..2f2734ba5273 100644
>> --- a/drivers/mfd/qcom-pm8xxx.c
>> +++ b/drivers/mfd/qcom-pm8xxx.c
>> @@ -65,7 +65,7 @@
>> struct pm_irq_data {
>> int num_irqs;
>> struct irq_chip *irq_chip;
>> - void (*irq_handler)(struct irq_desc *desc);
>> + irq_handler_t irq_handler;
>> };
>>
>> struct pm_irq_chip {
>> @@ -169,19 +169,16 @@ static int pm8xxx_irq_master_handler(struct pm_irq_chip *chip, int master)
>> return ret;
>> }
>>
>> -static void pm8xxx_irq_handler(struct irq_desc *desc)
>> +static irqreturn_t pm8xxx_irq_handler(int irq, void *data)
>> {
>> - struct pm_irq_chip *chip = irq_desc_get_handler_data(desc);
>> - struct irq_chip *irq_chip = irq_desc_get_chip(desc);
>> + struct pm_irq_chip *chip = data;
>> unsigned int root;
>> int i, ret, masters = 0;
>>
>> - chained_irq_enter(irq_chip, desc);
>> -
>> ret = regmap_read(chip->regmap, SSBI_REG_ADDR_IRQ_ROOT, &root);
>> if (ret) {
>> pr_err("Can't read root status ret=%d\n", ret);
>> - return;
>> + return IRQ_NONE;
>> }
>>
>> /* on pm8xxx series masters start from bit 1 of the root */
>> @@ -192,7 +189,7 @@ static void pm8xxx_irq_handler(struct irq_desc *desc)
>> if (masters & (1 << i))
>> pm8xxx_irq_master_handler(chip, i);
>>
>> - chained_irq_exit(irq_chip, desc);
>> + return IRQ_HANDLED;
>> }
>>
>> static void pm8821_irq_block_handler(struct pm_irq_chip *chip,
>> @@ -230,19 +227,17 @@ static inline void pm8821_irq_master_handler(struct pm_irq_chip *chip,
>> pm8821_irq_block_handler(chip, master, block);
>> }
>>
>> -static void pm8821_irq_handler(struct irq_desc *desc)
>> +static irqreturn_t pm8821_irq_handler(int irq, void *data)
>> {
>> - struct pm_irq_chip *chip = irq_desc_get_handler_data(desc);
>> - struct irq_chip *irq_chip = irq_desc_get_chip(desc);
>> + struct pm_irq_chip *chip = data;
>> unsigned int master;
>> int ret;
>>
>> - chained_irq_enter(irq_chip, desc);
>> ret = regmap_read(chip->regmap,
>> PM8821_SSBI_REG_ADDR_IRQ_MASTER0, &master);
>> if (ret) {
>> pr_err("Failed to read master 0 ret=%d\n", ret);
>> - goto done;
>> + return IRQ_NONE;
>> }
>>
>> /* bits 1 through 7 marks the first 7 blocks in master 0 */
>> @@ -251,19 +246,18 @@ static void pm8821_irq_handler(struct irq_desc *desc)
>>
>> /* bit 0 marks if master 1 contains any bits */
>> if (!(master & BIT(0)))
>> - goto done;
>> + return IRQ_NONE;
>>
>> ret = regmap_read(chip->regmap,
>> PM8821_SSBI_REG_ADDR_IRQ_MASTER1, &master);
>> if (ret) {
>> pr_err("Failed to read master 1 ret=%d\n", ret);
>> - goto done;
>> + return IRQ_NONE;
>> }
>>
>> pm8821_irq_master_handler(chip, 1, master);
>>
>> -done:
>> - chained_irq_exit(irq_chip, desc);
>> + return IRQ_HANDLED;
>> }
>>
>> static void pm8xxx_irq_mask_ack(struct irq_data *d)
>> @@ -574,14 +568,15 @@ static int pm8xxx_probe(struct platform_device *pdev)
>> if (!chip->irqdomain)
>> return -ENODEV;
>>
>> - irq_set_chained_handler_and_data(irq, data->irq_handler, chip);
>> + rc = devm_request_irq(&pdev->dev, irq, data->irq_handler, 0, dev_name(&pdev->dev), chip);
>> + if (rc)
>> + return rc;
>> +
>> irq_set_irq_wake(irq, 1);
>>
>> rc = of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
>> - if (rc) {
>> - irq_set_chained_handler_and_data(irq, NULL, NULL);
>> + if (rc)
>> irq_domain_remove(chip->irqdomain);
>> - }
>>
>> return rc;
>> }
>> @@ -594,11 +589,9 @@ static int pm8xxx_remove_child(struct device *dev, void *unused)
>>
>> static int pm8xxx_remove(struct platform_device *pdev)
>> {
>> - int irq = platform_get_irq(pdev, 0);
>> struct pm_irq_chip *chip = platform_get_drvdata(pdev);
>>
>> device_for_each_child(&pdev->dev, NULL, pm8xxx_remove_child);
>> - irq_set_chained_handler_and_data(irq, NULL, NULL);
>> irq_domain_remove(chip->irqdomain);
>>
>> return 0;
>> --
>> 2.30.2
>>
--
With best wishes
Dmitry
next prev parent reply other threads:[~2021-09-21 20:47 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-21 16:24 [PATCH] mfd: qcom-pm8xxx: switch away from using chained IRQ handlers Dmitry Baryshkov
2021-09-21 19:49 ` Bjorn Andersson
2021-09-21 20:47 ` Dmitry Baryshkov [this message]
2021-09-23 0:04 ` Linus Walleij
2021-09-23 5:19 ` David Heidelberg
2021-09-23 21:15 ` Linus Walleij
2021-10-06 8:03 ` Lee Jones
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=c1988694-9499-a7c4-1156-99699e20eba0@linaro.org \
--to=dmitry.baryshkov@linaro.org \
--cc=agross@kernel.org \
--cc=bjorn.andersson@linaro.org \
--cc=david@ixit.cz \
--cc=lee.jones@linaro.org \
--cc=linus.walleij@linaro.org \
--cc=linux-arm-msm@vger.kernel.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).