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.
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.
And I think it would be nice to Cc Linus Walleij.
Change looks good though, so please update the commit message and add
my:
Reviewed-by: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>
Regards,
Bjorn
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
> ---
> 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
>
