On Mon, Jun 29, 2020 at 10:43 PM Johan Hovold [off-list ref] wrote:

On Mon, Jun 29, 2020 at 10:35:06PM +0800, Baolin Wang wrote:

quoted

On Mon, Jun 29, 2020 at 10:01 PM Lee Jones [off-list ref] wrote:

quoted

On Mon, 29 Jun 2020, Johan Hovold wrote:

quoted

On Mon, Jun 29, 2020 at 01:32:14PM +0100, Lee Jones wrote:

quoted

Since ddata->irqs[] is already zeroed when allocated by devm_kcalloc() and
dividing 0 by anything is still 0, there is no need to re-assign
ddata->irqs[i].* values.  Instead, it should be safe to begin at 1.

This fixes the following W=1 warning:

 drivers/mfd/sprd-sc27xx-spi.c:255 sprd_pmic_probe() debug: sval_binop_unsigned: divide by zero

Cc: Orson Zhai <orsonzhai@gmail.com>
Cc: Baolin Wang <baolin.wang7@gmail.com>
Cc: Chunyan Zhang <zhang.lyra@gmail.com>
Signed-off-by: Lee Jones <redacted>
---
 drivers/mfd/sprd-sc27xx-spi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/sprd-sc27xx-spi.c b/drivers/mfd/sprd-sc27xx-spi.c
index c305e941e435c..694a7d429ccff 100644
--- a/drivers/mfd/sprd-sc27xx-spi.c
+++ b/drivers/mfd/sprd-sc27xx-spi.c

@@ -251,7 +251,7 @@ static int sprd_pmic_probe(struct spi_device *spi)
            return -ENOMEM;

    ddata->irq_chip.irqs = ddata->irqs;
-   for (i = 0; i < pdata->num_irqs; i++) {
+   for (i = 1; i < pdata->num_irqs; i++) {
            ddata->irqs[i].reg_offset = i / pdata->num_irqs;
            ddata->irqs[i].mask = BIT(i % pdata->num_irqs);
    }

This doesn't look right either.

First, the loop is never executed if num_irqs is zero.

The point of the patch is that 0 entries are never processed.

So what's the problem? There's no division by zero here.

And what compiler are you using, Lee? Seems broken.

quoted

quoted

quoted

Second, the current code looks bogus too as reg_offset is always set to
zero and mask to BIT(i)...

Now the result is correct, since all PMIC irq mask bits are in one
register now, which means the reg_offset is always 0 can work well.
But I think the logics still can be improved if our PMIC irq numbers
are larger than 32 in future.

The code is still bogus as pointed out above. Why do you bother to
divide by num_irqs at all?

Right, no need to divide by num_irqs, can be simplified as below. Lee,
care to resend your patch with simplifying the code? Or you want me to
send a patch?

diff --git a/drivers/mfd/sprd-sc27xx-spi.c b/drivers/mfd/sprd-sc27xx-spi.c
index 33336cde4724..2ed5f3a4e79c 100644
--- a/drivers/mfd/sprd-sc27xx-spi.c
+++ b/drivers/mfd/sprd-sc27xx-spi.c

@@ -250,10 +250,8 @@ static int sprd_pmic_probe(struct spi_device *spi)
                return -ENOMEM;

        ddata->irq_chip.irqs = ddata->irqs;
-       for (i = 0; i < pdata->num_irqs; i++) {
-               ddata->irqs[i].reg_offset = i / pdata->num_irqs;
-               ddata->irqs[i].mask = BIT(i % pdata->num_irqs);
-       }
+       for (i = 0; i < pdata->num_irqs; i++)
+               ddata->irqs[i].mask = BIT(i);

-- 
Baolin Wang

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel