2020-06-29 18:56:45

by Lee Jones

Subject: [PATCH 4/5] mfd: sprd-sc27xx-spi: Fix divide by zero when allocating register offset/mask

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 <[email protected]>
Cc: Baolin Wang <[email protected]>
Cc: Chunyan Zhang <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
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);
}
--
2.25.1

2020-06-29 21:19:34

by Johan Hovold

Subject: Re: [PATCH 4/5] mfd: sprd-sc27xx-spi: Fix divide by zero when allocating register offset/mask

On Mon, Jun 29, 2020 at 01:32:14PM +0100, Lee Jones wrote:
> 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 <[email protected]>
> Cc: Baolin Wang <[email protected]>
> Cc: Chunyan Zhang <[email protected]>
> Signed-off-by: Lee Jones <[email protected]>
> ---
> 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.

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

Johan

2020-07-01 09:15:56

by Lee Jones

Subject: [PATCH] mfd: sprd-sc27xx-spi: Fix-up bogus IRQ register offset and mask setting

'i / pdata->num_irqs' always equates to 0 and 'BIT(i % pdata->num_irqs)'
always ends up being BIT(i) here, so make that clearer in the code. If
the code base needs to support more than 32 IRQs in the future, this will
have to be reworked, but lets just keep it simple for as long as we can.

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 <[email protected]>
Cc: Chunyan Zhang <[email protected]>
Cc: Johan Hovold <[email protected]>
Suggested-by: Baolin Wang <[email protected]>
Signed-off-by: Lee Jones <[email protected]>

diff --git a/drivers/mfd/sprd-sc27xx-spi.c b/drivers/mfd/sprd-sc27xx-spi.c
index c305e941e435c..4a1a61e1a86ea 100644
--- a/drivers/mfd/sprd-sc27xx-spi.c
+++ b/drivers/mfd/sprd-sc27xx-spi.c
@@ -251,10 +251,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);

ret = devm_regmap_add_irq_chip(&spi->dev, ddata->regmap, ddata->irq,
IRQF_ONESHOT | IRQF_NO_SUSPEND, 0,

2020-07-01 14:11:30

by Baolin Wang

Subject: Re: [PATCH] mfd: sprd-sc27xx-spi: Fix-up bogus IRQ register offset and mask setting

On Wed, Jul 1, 2020 at 5:15 PM Lee Jones <[email protected]> wrote:
>
> 'i / pdata->num_irqs' always equates to 0 and 'BIT(i % pdata->num_irqs)'
> always ends up being BIT(i) here, so make that clearer in the code. If
> the code base needs to support more than 32 IRQs in the future, this will
> have to be reworked, but lets just keep it simple for as long as we can.
>
> 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 <[email protected]>
> Cc: Chunyan Zhang <[email protected]>
> Cc: Johan Hovold <[email protected]>
> Suggested-by: Baolin Wang <[email protected]>
> Signed-off-by: Lee Jones <[email protected]>

Thanks.
Reviewed-by: Baolin Wang <[email protected]>

>
> diff --git a/drivers/mfd/sprd-sc27xx-spi.c b/drivers/mfd/sprd-sc27xx-spi.c
> index c305e941e435c..4a1a61e1a86ea 100644
> --- a/drivers/mfd/sprd-sc27xx-spi.c
> +++ b/drivers/mfd/sprd-sc27xx-spi.c
> @@ -251,10 +251,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);
>
> ret = devm_regmap_add_irq_chip(&spi->dev, ddata->regmap, ddata->irq,
> IRQF_ONESHOT | IRQF_NO_SUSPEND, 0,



--
Baolin Wang