[PATCH] spi: Fix probing SPI drivers without a cs-gpios node

[PATCH] spi: Fix probing SPI drivers without a cs-gpios node

[John Watts]

of_gpio_named_count returns a negative value on error but this
is discarded and cast to a u16, making error handling impossible.

With debug logging enabled this effectively halts booting so the board can
print an error over serial 65534 times.

Check for this error and proceed as if there are no gpios specified.

Signed-off-by: John Watts <contact@jookia.org>
---
 drivers/spi/atmel_spi.c | 8 +++++++-
 drivers/spi/imx_spi.c   | 8 +++++++-
 drivers/spi/stm32_spi.c | 8 +++++++-
 3 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/atmel_spi.c b/drivers/spi/atmel_spi.c
index ec90330e53..a2314be8ee 100644
--- a/drivers/spi/atmel_spi.c
+++ b/drivers/spi/atmel_spi.c
@@ -388,6 +388,7 @@ static int atmel_spi_probe(struct device_d *dev)
 	struct resource *iores;
 	int ret = 0;
 	int i;
+	int num_gpios;
 	struct spi_master *master;
 	struct device_node *node = dev->device_node;
 	struct atmel_spi *as;
@@ -408,7 +409,12 @@ static int atmel_spi_probe(struct device_d *dev)
 		master->num_chipselect = pdata->num_chipselect;
 		as->cs_pins = pdata->chipselect;
 	} else {
-		master->num_chipselect = of_gpio_named_count(node, "cs-gpios");
+		num_gpios = of_gpio_named_count(node, "cs-gpios");
+		if (num_gpios < 0) {
+			num_gpios = 0;
+		}
+
+		master->num_chipselect = num_gpios;
 		as->cs_pins = xzalloc(sizeof(u32) * master->num_chipselect);
 
 		for (i = 0; i < master->num_chipselect; i++) {
diff --git a/drivers/spi/imx_spi.c b/drivers/spi/imx_spi.c
index 3e0ad2db00..b10076639a 100644
--- a/drivers/spi/imx_spi.c
+++ b/drivers/spi/imx_spi.c
@@ -564,11 +564,17 @@ static int imx_spi_dt_probe(struct imx_spi *imx)
 {
 	struct device_node *node = imx->master.dev->device_node;
 	int i;
+	int num_gpios;
 
 	if (!node)
 		return -ENODEV;
 
-	imx->master.num_chipselect = of_gpio_named_count(node, "cs-gpios");
+	num_gpios = of_gpio_named_count(node, "cs-gpios");
+	if (num_gpios < 0) {
+		num_gpios = 0;
+	}
+
+	imx->master.num_chipselect = num_gpios;
 	imx->cs_array = xzalloc(sizeof(u32) * imx->master.num_chipselect);
 
 	for (i = 0; i < imx->master.num_chipselect; i++)
diff --git a/drivers/spi/stm32_spi.c b/drivers/spi/stm32_spi.c
index 639c4f1740..a34e0b143d 100644
--- a/drivers/spi/stm32_spi.c
+++ b/drivers/spi/stm32_spi.c
@@ -513,8 +513,14 @@ static void stm32_spi_dt_probe(struct stm32_spi_priv *priv)
 {
 	struct device_node *node = priv->master.dev->device_node;
 	int i;
+	int num_gpios;
 
-	priv->master.num_chipselect = of_gpio_named_count(node, "cs-gpios");
+	num_gpios = of_gpio_named_count(node, "cs-gpios");
+	if (num_gpios < 0) {
+		num_gpios = 0;
+	}
+
+	priv->master.num_chipselect = num_gpios;
 	priv->cs_gpios = xzalloc(sizeof(u32) * priv->master.num_chipselect);
 
 	for (i = 0; i < priv->master.num_chipselect; i++)
-- 
2.39.0

Re: [PATCH] spi: Fix probing SPI drivers without a cs-gpios node

[Alexander Shiyan]

Hello.

These lines can be defined as a regular function like
get_cs_gpio_cound(node), isn't it?

вс, 22 янв. 2023 г. в 18:06, John Watts <contact@jookia.org>:
>
> of_gpio_named_count returns a negative value on error but this
> is discarded and cast to a u16, making error handling impossible.
>
> With debug logging enabled this effectively halts booting so the board can
> print an error over serial 65534 times.
>
> Check for this error and proceed as if there are no gpios specified.
>
> Signed-off-by: John Watts <contact@jookia.org>
> ---
>  drivers/spi/atmel_spi.c | 8 +++++++-
>  drivers/spi/imx_spi.c   | 8 +++++++-
>  drivers/spi/stm32_spi.c | 8 +++++++-
>  3 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/spi/atmel_spi.c b/drivers/spi/atmel_spi.c
> index ec90330e53..a2314be8ee 100644
> --- a/drivers/spi/atmel_spi.c
> +++ b/drivers/spi/atmel_spi.c
> @@ -388,6 +388,7 @@ static int atmel_spi_probe(struct device_d *dev)
>         struct resource *iores;
>         int ret = 0;
>         int i;
> +       int num_gpios;
>         struct spi_master *master;
>         struct device_node *node = dev->device_node;
>         struct atmel_spi *as;
> @@ -408,7 +409,12 @@ static int atmel_spi_probe(struct device_d *dev)
>                 master->num_chipselect = pdata->num_chipselect;
>                 as->cs_pins = pdata->chipselect;
>         } else {
> -               master->num_chipselect = of_gpio_named_count(node, "cs-gpios");
> +               num_gpios = of_gpio_named_count(node, "cs-gpios");
> +               if (num_gpios < 0) {
> +                       num_gpios = 0;
> +               }
> +
> +               master->num_chipselect = num_gpios;
>                 as->cs_pins = xzalloc(sizeof(u32) * master->num_chipselect);
>
>                 for (i = 0; i < master->num_chipselect; i++) {
> diff --git a/drivers/spi/imx_spi.c b/drivers/spi/imx_spi.c
> index 3e0ad2db00..b10076639a 100644
> --- a/drivers/spi/imx_spi.c
> +++ b/drivers/spi/imx_spi.c
> @@ -564,11 +564,17 @@ static int imx_spi_dt_probe(struct imx_spi *imx)
>  {
>         struct device_node *node = imx->master.dev->device_node;
>         int i;
> +       int num_gpios;
>
>         if (!node)
>                 return -ENODEV;
>
> -       imx->master.num_chipselect = of_gpio_named_count(node, "cs-gpios");
> +       num_gpios = of_gpio_named_count(node, "cs-gpios");
> +       if (num_gpios < 0) {
> +               num_gpios = 0;
> +       }
> +
> +       imx->master.num_chipselect = num_gpios;
>         imx->cs_array = xzalloc(sizeof(u32) * imx->master.num_chipselect);
>
>         for (i = 0; i < imx->master.num_chipselect; i++)
> diff --git a/drivers/spi/stm32_spi.c b/drivers/spi/stm32_spi.c
> index 639c4f1740..a34e0b143d 100644
> --- a/drivers/spi/stm32_spi.c
> +++ b/drivers/spi/stm32_spi.c
> @@ -513,8 +513,14 @@ static void stm32_spi_dt_probe(struct stm32_spi_priv *priv)
>  {
>         struct device_node *node = priv->master.dev->device_node;
>         int i;
> +       int num_gpios;
>
> -       priv->master.num_chipselect = of_gpio_named_count(node, "cs-gpios");
> +       num_gpios = of_gpio_named_count(node, "cs-gpios");
> +       if (num_gpios < 0) {
> +               num_gpios = 0;
> +       }
> +
> +       priv->master.num_chipselect = num_gpios;
>         priv->cs_gpios = xzalloc(sizeof(u32) * priv->master.num_chipselect);
>
>         for (i = 0; i < priv->master.num_chipselect; i++)
> --
> 2.39.0
>
>

Re: [PATCH] spi: Fix probing SPI drivers without a cs-gpios node

[John Watts]

Hi there,

Yes it would be possible to refactor it to do that.
Would it be worth it for these few instances?

John

On Sun, Jan 22, 2023 at 08:06:04PM +0300, Alexander Shiyan wrote:
> Hello.
> 
> These lines can be defined as a regular function like
> get_cs_gpio_cound(node), isn't it?
> 
> вс, 22 янв. 2023 г. в 18:06, John Watts <contact@jookia.org>:
> >
> > of_gpio_named_count returns a negative value on error but this
> > is discarded and cast to a u16, making error handling impossible.
> >
> > With debug logging enabled this effectively halts booting so the board can
> > print an error over serial 65534 times.
> >
> > Check for this error and proceed as if there are no gpios specified.
> >
> > Signed-off-by: John Watts <contact@jookia.org>
> > ---
> >  drivers/spi/atmel_spi.c | 8 +++++++-
> >  drivers/spi/imx_spi.c   | 8 +++++++-
> >  drivers/spi/stm32_spi.c | 8 +++++++-
> >  3 files changed, 21 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/spi/atmel_spi.c b/drivers/spi/atmel_spi.c
> > index ec90330e53..a2314be8ee 100644
> > --- a/drivers/spi/atmel_spi.c
> > +++ b/drivers/spi/atmel_spi.c
> > @@ -388,6 +388,7 @@ static int atmel_spi_probe(struct device_d *dev)
> >         struct resource *iores;
> >         int ret = 0;
> >         int i;
> > +       int num_gpios;
> >         struct spi_master *master;
> >         struct device_node *node = dev->device_node;
> >         struct atmel_spi *as;
> > @@ -408,7 +409,12 @@ static int atmel_spi_probe(struct device_d *dev)
> >                 master->num_chipselect = pdata->num_chipselect;
> >                 as->cs_pins = pdata->chipselect;
> >         } else {
> > -               master->num_chipselect = of_gpio_named_count(node, "cs-gpios");
> > +               num_gpios = of_gpio_named_count(node, "cs-gpios");
> > +               if (num_gpios < 0) {
> > +                       num_gpios = 0;
> > +               }
> > +
> > +               master->num_chipselect = num_gpios;
> >                 as->cs_pins = xzalloc(sizeof(u32) * master->num_chipselect);
> >
> >                 for (i = 0; i < master->num_chipselect; i++) {
> > diff --git a/drivers/spi/imx_spi.c b/drivers/spi/imx_spi.c
> > index 3e0ad2db00..b10076639a 100644
> > --- a/drivers/spi/imx_spi.c
> > +++ b/drivers/spi/imx_spi.c
> > @@ -564,11 +564,17 @@ static int imx_spi_dt_probe(struct imx_spi *imx)
> >  {
> >         struct device_node *node = imx->master.dev->device_node;
> >         int i;
> > +       int num_gpios;
> >
> >         if (!node)
> >                 return -ENODEV;
> >
> > -       imx->master.num_chipselect = of_gpio_named_count(node, "cs-gpios");
> > +       num_gpios = of_gpio_named_count(node, "cs-gpios");
> > +       if (num_gpios < 0) {
> > +               num_gpios = 0;
> > +       }
> > +
> > +       imx->master.num_chipselect = num_gpios;
> >         imx->cs_array = xzalloc(sizeof(u32) * imx->master.num_chipselect);
> >
> >         for (i = 0; i < imx->master.num_chipselect; i++)
> > diff --git a/drivers/spi/stm32_spi.c b/drivers/spi/stm32_spi.c
> > index 639c4f1740..a34e0b143d 100644
> > --- a/drivers/spi/stm32_spi.c
> > +++ b/drivers/spi/stm32_spi.c
> > @@ -513,8 +513,14 @@ static void stm32_spi_dt_probe(struct stm32_spi_priv *priv)
> >  {
> >         struct device_node *node = priv->master.dev->device_node;
> >         int i;
> > +       int num_gpios;
> >
> > -       priv->master.num_chipselect = of_gpio_named_count(node, "cs-gpios");
> > +       num_gpios = of_gpio_named_count(node, "cs-gpios");
> > +       if (num_gpios < 0) {
> > +               num_gpios = 0;
> > +       }
> > +
> > +       priv->master.num_chipselect = num_gpios;
> >         priv->cs_gpios = xzalloc(sizeof(u32) * priv->master.num_chipselect);
> >
> >         for (i = 0; i < priv->master.num_chipselect; i++)
> > --
> > 2.39.0
> >
> >

Re: [PATCH] spi: Fix probing SPI drivers without a cs-gpios node

[Alexander Shiyan]

I dont know. I said how would I do it if it was my patch. I hope
Sasсha will decide how it will be better.

вс, 22 янв. 2023 г. в 20:13, John Watts <contact@jookia.org>:
>
> Hi there,
>
> Yes it would be possible to refactor it to do that.
> Would it be worth it for these few instances?
>
> John
>
> On Sun, Jan 22, 2023 at 08:06:04PM +0300, Alexander Shiyan wrote:
> > Hello.
> >
> > These lines can be defined as a regular function like
> > get_cs_gpio_cound(node), isn't it?
> >
> > вс, 22 янв. 2023 г. в 18:06, John Watts <contact@jookia.org>:
> > >
> > > of_gpio_named_count returns a negative value on error but this
> > > is discarded and cast to a u16, making error handling impossible.
> > >
> > > With debug logging enabled this effectively halts booting so the board can
> > > print an error over serial 65534 times.
> > >
> > > Check for this error and proceed as if there are no gpios specified.
> > >
> > > Signed-off-by: John Watts <contact@jookia.org>
> > > ---
> > >  drivers/spi/atmel_spi.c | 8 +++++++-
> > >  drivers/spi/imx_spi.c   | 8 +++++++-
> > >  drivers/spi/stm32_spi.c | 8 +++++++-
> > >  3 files changed, 21 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/spi/atmel_spi.c b/drivers/spi/atmel_spi.c
> > > index ec90330e53..a2314be8ee 100644
> > > --- a/drivers/spi/atmel_spi.c
> > > +++ b/drivers/spi/atmel_spi.c
> > > @@ -388,6 +388,7 @@ static int atmel_spi_probe(struct device_d *dev)
> > >         struct resource *iores;
> > >         int ret = 0;
> > >         int i;
> > > +       int num_gpios;
> > >         struct spi_master *master;
> > >         struct device_node *node = dev->device_node;
> > >         struct atmel_spi *as;
> > > @@ -408,7 +409,12 @@ static int atmel_spi_probe(struct device_d *dev)
> > >                 master->num_chipselect = pdata->num_chipselect;
> > >                 as->cs_pins = pdata->chipselect;
> > >         } else {
> > > -               master->num_chipselect = of_gpio_named_count(node, "cs-gpios");
> > > +               num_gpios = of_gpio_named_count(node, "cs-gpios");
> > > +               if (num_gpios < 0) {
> > > +                       num_gpios = 0;
> > > +               }
> > > +
> > > +               master->num_chipselect = num_gpios;
> > >                 as->cs_pins = xzalloc(sizeof(u32) * master->num_chipselect);
> > >
> > >                 for (i = 0; i < master->num_chipselect; i++) {
> > > diff --git a/drivers/spi/imx_spi.c b/drivers/spi/imx_spi.c
> > > index 3e0ad2db00..b10076639a 100644
> > > --- a/drivers/spi/imx_spi.c
> > > +++ b/drivers/spi/imx_spi.c
> > > @@ -564,11 +564,17 @@ static int imx_spi_dt_probe(struct imx_spi *imx)
> > >  {
> > >         struct device_node *node = imx->master.dev->device_node;
> > >         int i;
> > > +       int num_gpios;
> > >
> > >         if (!node)
> > >                 return -ENODEV;
> > >
> > > -       imx->master.num_chipselect = of_gpio_named_count(node, "cs-gpios");
> > > +       num_gpios = of_gpio_named_count(node, "cs-gpios");
> > > +       if (num_gpios < 0) {
> > > +               num_gpios = 0;
> > > +       }
> > > +
> > > +       imx->master.num_chipselect = num_gpios;
> > >         imx->cs_array = xzalloc(sizeof(u32) * imx->master.num_chipselect);
> > >
> > >         for (i = 0; i < imx->master.num_chipselect; i++)
> > > diff --git a/drivers/spi/stm32_spi.c b/drivers/spi/stm32_spi.c
> > > index 639c4f1740..a34e0b143d 100644
> > > --- a/drivers/spi/stm32_spi.c
> > > +++ b/drivers/spi/stm32_spi.c
> > > @@ -513,8 +513,14 @@ static void stm32_spi_dt_probe(struct stm32_spi_priv *priv)
> > >  {
> > >         struct device_node *node = priv->master.dev->device_node;
> > >         int i;
> > > +       int num_gpios;
> > >
> > > -       priv->master.num_chipselect = of_gpio_named_count(node, "cs-gpios");
> > > +       num_gpios = of_gpio_named_count(node, "cs-gpios");
> > > +       if (num_gpios < 0) {
> > > +               num_gpios = 0;
> > > +       }
> > > +
> > > +       priv->master.num_chipselect = num_gpios;
> > >         priv->cs_gpios = xzalloc(sizeof(u32) * priv->master.num_chipselect);
> > >
> > >         for (i = 0; i < priv->master.num_chipselect; i++)
> > > --
> > > 2.39.0
> > >
> > >

Re: [PATCH] spi: Fix probing SPI drivers without a cs-gpios node

[John Watts]

I certainly appreciate the feedback.

One thing to note is that an existing driver has a similar check, so that would
be 4 places with code like this.

On Sun, Jan 22, 2023 at 09:09:38PM +0300, Alexander Shiyan wrote:
> I dont know. I said how would I do it if it was my patch. I hope
> Sasсha will decide how it will be better.
> 
> вс, 22 янв. 2023 г. в 20:13, John Watts <contact@jookia.org>:
> >
> > Hi there,
> >
> > Yes it would be possible to refactor it to do that.
> > Would it be worth it for these few instances?
> >
> > John
> >
> > On Sun, Jan 22, 2023 at 08:06:04PM +0300, Alexander Shiyan wrote:
> > > Hello.
> > >
> > > These lines can be defined as a regular function like
> > > get_cs_gpio_cound(node), isn't it?
> > >
> > > вс, 22 янв. 2023 г. в 18:06, John Watts <contact@jookia.org>:
> > > >
> > > > of_gpio_named_count returns a negative value on error but this
> > > > is discarded and cast to a u16, making error handling impossible.
> > > >
> > > > With debug logging enabled this effectively halts booting so the board can
> > > > print an error over serial 65534 times.
> > > >
> > > > Check for this error and proceed as if there are no gpios specified.
> > > >
> > > > Signed-off-by: John Watts <contact@jookia.org>
> > > > ---
> > > >  drivers/spi/atmel_spi.c | 8 +++++++-
> > > >  drivers/spi/imx_spi.c   | 8 +++++++-
> > > >  drivers/spi/stm32_spi.c | 8 +++++++-
> > > >  3 files changed, 21 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/spi/atmel_spi.c b/drivers/spi/atmel_spi.c
> > > > index ec90330e53..a2314be8ee 100644
> > > > --- a/drivers/spi/atmel_spi.c
> > > > +++ b/drivers/spi/atmel_spi.c
> > > > @@ -388,6 +388,7 @@ static int atmel_spi_probe(struct device_d *dev)
> > > >         struct resource *iores;
> > > >         int ret = 0;
> > > >         int i;
> > > > +       int num_gpios;
> > > >         struct spi_master *master;
> > > >         struct device_node *node = dev->device_node;
> > > >         struct atmel_spi *as;
> > > > @@ -408,7 +409,12 @@ static int atmel_spi_probe(struct device_d *dev)
> > > >                 master->num_chipselect = pdata->num_chipselect;
> > > >                 as->cs_pins = pdata->chipselect;
> > > >         } else {
> > > > -               master->num_chipselect = of_gpio_named_count(node, "cs-gpios");
> > > > +               num_gpios = of_gpio_named_count(node, "cs-gpios");
> > > > +               if (num_gpios < 0) {
> > > > +                       num_gpios = 0;
> > > > +               }
> > > > +
> > > > +               master->num_chipselect = num_gpios;
> > > >                 as->cs_pins = xzalloc(sizeof(u32) * master->num_chipselect);
> > > >
> > > >                 for (i = 0; i < master->num_chipselect; i++) {
> > > > diff --git a/drivers/spi/imx_spi.c b/drivers/spi/imx_spi.c
> > > > index 3e0ad2db00..b10076639a 100644
> > > > --- a/drivers/spi/imx_spi.c
> > > > +++ b/drivers/spi/imx_spi.c
> > > > @@ -564,11 +564,17 @@ static int imx_spi_dt_probe(struct imx_spi *imx)
> > > >  {
> > > >         struct device_node *node = imx->master.dev->device_node;
> > > >         int i;
> > > > +       int num_gpios;
> > > >
> > > >         if (!node)
> > > >                 return -ENODEV;
> > > >
> > > > -       imx->master.num_chipselect = of_gpio_named_count(node, "cs-gpios");
> > > > +       num_gpios = of_gpio_named_count(node, "cs-gpios");
> > > > +       if (num_gpios < 0) {
> > > > +               num_gpios = 0;
> > > > +       }
> > > > +
> > > > +       imx->master.num_chipselect = num_gpios;
> > > >         imx->cs_array = xzalloc(sizeof(u32) * imx->master.num_chipselect);
> > > >
> > > >         for (i = 0; i < imx->master.num_chipselect; i++)
> > > > diff --git a/drivers/spi/stm32_spi.c b/drivers/spi/stm32_spi.c
> > > > index 639c4f1740..a34e0b143d 100644
> > > > --- a/drivers/spi/stm32_spi.c
> > > > +++ b/drivers/spi/stm32_spi.c
> > > > @@ -513,8 +513,14 @@ static void stm32_spi_dt_probe(struct stm32_spi_priv *priv)
> > > >  {
> > > >         struct device_node *node = priv->master.dev->device_node;
> > > >         int i;
> > > > +       int num_gpios;
> > > >
> > > > -       priv->master.num_chipselect = of_gpio_named_count(node, "cs-gpios");
> > > > +       num_gpios = of_gpio_named_count(node, "cs-gpios");
> > > > +       if (num_gpios < 0) {
> > > > +               num_gpios = 0;
> > > > +       }
> > > > +
> > > > +       priv->master.num_chipselect = num_gpios;
> > > >         priv->cs_gpios = xzalloc(sizeof(u32) * priv->master.num_chipselect);
> > > >
> > > >         for (i = 0; i < priv->master.num_chipselect; i++)
> > > > --
> > > > 2.39.0
> > > >
> > > >

Re: [PATCH] spi: Fix probing SPI drivers without a cs-gpios node

[Sascha Hauer]

On Sun, Jan 22, 2023 at 09:09:38PM +0300, Alexander Shiyan wrote:
> I dont know. I said how would I do it if it was my patch. I hope
> Sasсha will decide how it will be better.

I would also prefer a dedicated function that just returns 0 if there
are no CS GPIOs found.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH] spi: Fix probing SPI drivers without a cs-gpios node

[John Watts]

Alright, shall do in V2.

On Mon, Jan 23, 2023 at 09:43:53AM +0100, Sascha Hauer wrote:
> On Sun, Jan 22, 2023 at 09:09:38PM +0300, Alexander Shiyan wrote:
> > I dont know. I said how would I do it if it was my patch. I hope
> > Sasсha will decide how it will be better.
> 
> I would also prefer a dedicated function that just returns 0 if there
> are no CS GPIOs found.
> 
> Sascha
> 
> -- 
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH v2] spi: Fix probing SPI drivers with no cs-gpios

[John Watts]

of_gpio_named_count returns a negative value on error but this
is discarded and cast to a u16, making error handling impossible.

With debug logging enabled this effectively halts booting so the board can
print an error over serial 65534 times.

Introduce of_gpio_count_csgpios which returns 0 in the case of an error
rather than a negative value.

Signed-off-by: John Watts <contact@jookia.org>
---
Changes v1 -> v2:
- Refactored logic in to separate of_gpio_count_csgpios function
---
 drivers/spi/atmel_spi.c |  2 +-
 drivers/spi/imx_spi.c   |  2 +-
 drivers/spi/stm32_spi.c |  2 +-
 include/of_gpio.h       | 17 +++++++++++++++++
 4 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/atmel_spi.c b/drivers/spi/atmel_spi.c
index 399c47c81d..9bf85874c5 100644
--- a/drivers/spi/atmel_spi.c
+++ b/drivers/spi/atmel_spi.c
@@ -408,7 +408,7 @@ static int atmel_spi_probe(struct device *dev)
 		master->num_chipselect = pdata->num_chipselect;
 		as->cs_pins = pdata->chipselect;
 	} else {
-		master->num_chipselect = of_gpio_named_count(node, "cs-gpios");
+		master->num_chipselect = of_gpio_count_csgpios(node);
 		as->cs_pins = xzalloc(sizeof(u32) * master->num_chipselect);
 
 		for (i = 0; i < master->num_chipselect; i++) {
diff --git a/drivers/spi/imx_spi.c b/drivers/spi/imx_spi.c
index eb30d757d5..f81d9e851f 100644
--- a/drivers/spi/imx_spi.c
+++ b/drivers/spi/imx_spi.c
@@ -568,7 +568,7 @@ static int imx_spi_dt_probe(struct imx_spi *imx)
 	if (!node)
 		return -ENODEV;
 
-	imx->master.num_chipselect = of_gpio_named_count(node, "cs-gpios");
+	imx->master.num_chipselect = of_gpio_count_csgpios(node);
 	imx->cs_array = xzalloc(sizeof(u32) * imx->master.num_chipselect);
 
 	for (i = 0; i < imx->master.num_chipselect; i++)
diff --git a/drivers/spi/stm32_spi.c b/drivers/spi/stm32_spi.c
index 821a95980f..0d7407c279 100644
--- a/drivers/spi/stm32_spi.c
+++ b/drivers/spi/stm32_spi.c
@@ -514,7 +514,7 @@ static void stm32_spi_dt_probe(struct stm32_spi_priv *priv)
 	struct device_node *node = priv->master.dev->of_node;
 	int i;
 
-	priv->master.num_chipselect = of_gpio_named_count(node, "cs-gpios");
+	priv->master.num_chipselect = of_gpio_count_csgpios(node);
 	priv->cs_gpios = xzalloc(sizeof(u32) * priv->master.num_chipselect);
 
 	for (i = 0; i < priv->master.num_chipselect; i++)
diff --git a/include/of_gpio.h b/include/of_gpio.h
index 30ff204baf..794a9926cd 100644
--- a/include/of_gpio.h
+++ b/include/of_gpio.h
@@ -69,6 +69,23 @@ static inline int of_gpio_count(struct device_node *np)
 	return of_gpio_named_count(np, "gpios");
 }
 
+/**
+ * of_gpio_count() - Count cs-gpios for a device
+ * @np:		device node to count cs-gpios for
+ *
+ * Same as of_gpio_named_count, but hard coded to use the 'cs-gpios' property
+ * Returns 0 on error
+ */
+static inline int of_gpio_count_csgpios(struct device_node *np)
+{
+	int count = of_gpio_named_count(np, "cs-gpios");
+
+	if (count > 0)
+		return count;
+	else
+		return 0;
+}
+
 static inline int of_get_gpio_flags(struct device_node *np, int index,
 		      enum of_gpio_flags *flags)
 {
-- 
2.39.0

Re: [PATCH v2] spi: Fix probing SPI drivers with no cs-gpios

[Sascha Hauer]

On Wed, Jan 25, 2023 at 07:05:55AM +1100, John Watts wrote:
> of_gpio_named_count returns a negative value on error but this
> is discarded and cast to a u16, making error handling impossible.
> 
> With debug logging enabled this effectively halts booting so the board can
> print an error over serial 65534 times.
> 
> Introduce of_gpio_count_csgpios which returns 0 in the case of an error
> rather than a negative value.
> 
> Signed-off-by: John Watts <contact@jookia.org>
> ---
> Changes v1 -> v2:
> - Refactored logic in to separate of_gpio_count_csgpios function
> ---

Applied, thanks

Sascha

>  drivers/spi/atmel_spi.c |  2 +-
>  drivers/spi/imx_spi.c   |  2 +-
>  drivers/spi/stm32_spi.c |  2 +-
>  include/of_gpio.h       | 17 +++++++++++++++++
>  4 files changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/spi/atmel_spi.c b/drivers/spi/atmel_spi.c
> index 399c47c81d..9bf85874c5 100644
> --- a/drivers/spi/atmel_spi.c
> +++ b/drivers/spi/atmel_spi.c
> @@ -408,7 +408,7 @@ static int atmel_spi_probe(struct device *dev)
>  		master->num_chipselect = pdata->num_chipselect;
>  		as->cs_pins = pdata->chipselect;
>  	} else {
> -		master->num_chipselect = of_gpio_named_count(node, "cs-gpios");
> +		master->num_chipselect = of_gpio_count_csgpios(node);
>  		as->cs_pins = xzalloc(sizeof(u32) * master->num_chipselect);
>  
>  		for (i = 0; i < master->num_chipselect; i++) {
> diff --git a/drivers/spi/imx_spi.c b/drivers/spi/imx_spi.c
> index eb30d757d5..f81d9e851f 100644
> --- a/drivers/spi/imx_spi.c
> +++ b/drivers/spi/imx_spi.c
> @@ -568,7 +568,7 @@ static int imx_spi_dt_probe(struct imx_spi *imx)
>  	if (!node)
>  		return -ENODEV;
>  
> -	imx->master.num_chipselect = of_gpio_named_count(node, "cs-gpios");
> +	imx->master.num_chipselect = of_gpio_count_csgpios(node);
>  	imx->cs_array = xzalloc(sizeof(u32) * imx->master.num_chipselect);
>  
>  	for (i = 0; i < imx->master.num_chipselect; i++)
> diff --git a/drivers/spi/stm32_spi.c b/drivers/spi/stm32_spi.c
> index 821a95980f..0d7407c279 100644
> --- a/drivers/spi/stm32_spi.c
> +++ b/drivers/spi/stm32_spi.c
> @@ -514,7 +514,7 @@ static void stm32_spi_dt_probe(struct stm32_spi_priv *priv)
>  	struct device_node *node = priv->master.dev->of_node;
>  	int i;
>  
> -	priv->master.num_chipselect = of_gpio_named_count(node, "cs-gpios");
> +	priv->master.num_chipselect = of_gpio_count_csgpios(node);
>  	priv->cs_gpios = xzalloc(sizeof(u32) * priv->master.num_chipselect);
>  
>  	for (i = 0; i < priv->master.num_chipselect; i++)
> diff --git a/include/of_gpio.h b/include/of_gpio.h
> index 30ff204baf..794a9926cd 100644
> --- a/include/of_gpio.h
> +++ b/include/of_gpio.h
> @@ -69,6 +69,23 @@ static inline int of_gpio_count(struct device_node *np)
>  	return of_gpio_named_count(np, "gpios");
>  }
>  
> +/**
> + * of_gpio_count() - Count cs-gpios for a device
> + * @np:		device node to count cs-gpios for
> + *
> + * Same as of_gpio_named_count, but hard coded to use the 'cs-gpios' property
> + * Returns 0 on error
> + */
> +static inline int of_gpio_count_csgpios(struct device_node *np)
> +{
> +	int count = of_gpio_named_count(np, "cs-gpios");
> +
> +	if (count > 0)
> +		return count;
> +	else
> +		return 0;
> +}
> +
>  static inline int of_get_gpio_flags(struct device_node *np, int index,
>  		      enum of_gpio_flags *flags)
>  {
> -- 
> 2.39.0
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |