| Message ID | 20240903-correct_error_codes_sifive_plic-v1-1-d929b79663a2@rivosinc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | irqchip/sifive-plic: Fix error codes | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/vmtest-fixes-PR | fail | merge-conflict |
Hi Charlie, On 04/09/2024 01:36, Charlie Jenkins wrote: > Set error to -ENOMEM if kcalloc() fails or if irq_domain_add_linear() > fails inside of plic_probe(). > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > Reported-by: kernel test robot <lkp@intel.com> > Reported-by: Dan Carpenter <dan.carpenter@linaro.org> > Closes: https://lore.kernel.org/r/202409031122.yBh8HrxA-lkp@intel.com/ I found the following Fixes tag: Fixes: 4d936f10ff80 ("irqchip/sifive-plic: Probe plic driver early for Allwinner D1 platform") > --- > drivers/irqchip/irq-sifive-plic.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c > index 2f6ef5c495bd..0b730e305748 100644 > --- a/drivers/irqchip/irq-sifive-plic.c > +++ b/drivers/irqchip/irq-sifive-plic.c > @@ -626,8 +626,10 @@ static int plic_probe(struct fwnode_handle *fwnode) > > handler->enable_save = kcalloc(DIV_ROUND_UP(nr_irqs, 32), > sizeof(*handler->enable_save), GFP_KERNEL); > - if (!handler->enable_save) > + if (!handler->enable_save) { > + error = -ENOMEM; This is correct. > goto fail_cleanup_contexts; > + } > done: > for (hwirq = 1; hwirq <= nr_irqs; hwirq++) { > plic_toggle(handler, hwirq, 0); > @@ -639,8 +641,10 @@ static int plic_probe(struct fwnode_handle *fwnode) > > priv->irqdomain = irq_domain_create_linear(fwnode, nr_irqs + 1, > &plic_irqdomain_ops, priv); > - if (WARN_ON(!priv->irqdomain)) > + if (WARN_ON(!priv->irqdomain)) { > + error = -ENOMEM; Here though I found drivers that return either ENODEV (https://elixir.bootlin.com/linux/v6.11/source/drivers/irqchip/irq-gic.c#L1210) or ENOMEM (https://elixir.bootlin.com/linux/v6.11/source/drivers/irqchip/irq-imx-mu-msi.c#L229). Before the commit I mentioned above, we used to return ENOMEM. IMHO it's not a big deal as long as we return something different from 0 in that case, I just mention it if someone thinks differently. You can add: Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com> And I would also add: Cc: stable@vger.kernel.org Thanks, Alex > goto fail_cleanup_contexts; > + } > > /* > * We can have multiple PLIC instances so setup global state > > --- > base-commit: 6804f0edbe7747774e6ae60f20cec4ee3ad7c187 > change-id: 20240903-correct_error_codes_sifive_plic-4611f59291df
On Wed, Sep 4, 2024 at 5:11 AM Charlie Jenkins <charlie@rivosinc.com> wrote: > > Set error to -ENOMEM if kcalloc() fails or if irq_domain_add_linear() > fails inside of plic_probe(). > Like Alex mentioned, please include a Fixes tag. > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > Reported-by: kernel test robot <lkp@intel.com> > Reported-by: Dan Carpenter <dan.carpenter@linaro.org> > Closes: https://lore.kernel.org/r/202409031122.yBh8HrxA-lkp@intel.com/ LGTM. Reviewed-by: Anup Patel <anup@brainfault.org> Regards, Anup > --- > drivers/irqchip/irq-sifive-plic.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c > index 2f6ef5c495bd..0b730e305748 100644 > --- a/drivers/irqchip/irq-sifive-plic.c > +++ b/drivers/irqchip/irq-sifive-plic.c > @@ -626,8 +626,10 @@ static int plic_probe(struct fwnode_handle *fwnode) > > handler->enable_save = kcalloc(DIV_ROUND_UP(nr_irqs, 32), > sizeof(*handler->enable_save), GFP_KERNEL); > - if (!handler->enable_save) > + if (!handler->enable_save) { > + error = -ENOMEM; > goto fail_cleanup_contexts; > + } > done: > for (hwirq = 1; hwirq <= nr_irqs; hwirq++) { > plic_toggle(handler, hwirq, 0); > @@ -639,8 +641,10 @@ static int plic_probe(struct fwnode_handle *fwnode) > > priv->irqdomain = irq_domain_create_linear(fwnode, nr_irqs + 1, > &plic_irqdomain_ops, priv); > - if (WARN_ON(!priv->irqdomain)) > + if (WARN_ON(!priv->irqdomain)) { > + error = -ENOMEM; > goto fail_cleanup_contexts; > + } > > /* > * We can have multiple PLIC instances so setup global state > > --- > base-commit: 6804f0edbe7747774e6ae60f20cec4ee3ad7c187 > change-id: 20240903-correct_error_codes_sifive_plic-4611f59291df > -- > - Charlie > >
diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c index 2f6ef5c495bd..0b730e305748 100644 --- a/drivers/irqchip/irq-sifive-plic.c +++ b/drivers/irqchip/irq-sifive-plic.c @@ -626,8 +626,10 @@ static int plic_probe(struct fwnode_handle *fwnode) handler->enable_save = kcalloc(DIV_ROUND_UP(nr_irqs, 32), sizeof(*handler->enable_save), GFP_KERNEL); - if (!handler->enable_save) + if (!handler->enable_save) { + error = -ENOMEM; goto fail_cleanup_contexts; + } done: for (hwirq = 1; hwirq <= nr_irqs; hwirq++) { plic_toggle(handler, hwirq, 0); @@ -639,8 +641,10 @@ static int plic_probe(struct fwnode_handle *fwnode) priv->irqdomain = irq_domain_create_linear(fwnode, nr_irqs + 1, &plic_irqdomain_ops, priv); - if (WARN_ON(!priv->irqdomain)) + if (WARN_ON(!priv->irqdomain)) { + error = -ENOMEM; goto fail_cleanup_contexts; + } /* * We can have multiple PLIC instances so setup global state
Set error to -ENOMEM if kcalloc() fails or if irq_domain_add_linear() fails inside of plic_probe(). Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> Reported-by: kernel test robot <lkp@intel.com> Reported-by: Dan Carpenter <dan.carpenter@linaro.org> Closes: https://lore.kernel.org/r/202409031122.yBh8HrxA-lkp@intel.com/ --- drivers/irqchip/irq-sifive-plic.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) --- base-commit: 6804f0edbe7747774e6ae60f20cec4ee3ad7c187 change-id: 20240903-correct_error_codes_sifive_plic-4611f59291df