* [PATCH] spi: Fix probing SPI drivers without a cs-gpios node
@ 2023-01-22 15:04 John Watts
2023-01-22 17:06 ` Alexander Shiyan
2023-01-24 20:05 ` [PATCH v2] spi: Fix probing SPI drivers with no cs-gpios John Watts
0 siblings, 2 replies; 9+ messages in thread
From: John Watts @ 2023-01-22 15:04 UTC (permalink / raw)
To: barebox; +Cc: 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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] spi: Fix probing SPI drivers without a cs-gpios node
2023-01-22 15:04 [PATCH] spi: Fix probing SPI drivers without a cs-gpios node John Watts
@ 2023-01-22 17:06 ` Alexander Shiyan
2023-01-22 17:13 ` John Watts
2023-01-24 20:05 ` [PATCH v2] spi: Fix probing SPI drivers with no cs-gpios John Watts
1 sibling, 1 reply; 9+ messages in thread
From: Alexander Shiyan @ 2023-01-22 17:06 UTC (permalink / raw)
To: John Watts; +Cc: barebox
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
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] spi: Fix probing SPI drivers without a cs-gpios node
2023-01-22 17:06 ` Alexander Shiyan
@ 2023-01-22 17:13 ` John Watts
2023-01-22 18:09 ` Alexander Shiyan
0 siblings, 1 reply; 9+ messages in thread
From: John Watts @ 2023-01-22 17:13 UTC (permalink / raw)
To: Alexander Shiyan; +Cc: barebox
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
> >
> >
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] spi: Fix probing SPI drivers without a cs-gpios node
2023-01-22 17:13 ` John Watts
@ 2023-01-22 18:09 ` Alexander Shiyan
2023-01-22 18:19 ` John Watts
2023-01-23 8:43 ` Sascha Hauer
0 siblings, 2 replies; 9+ messages in thread
From: Alexander Shiyan @ 2023-01-22 18:09 UTC (permalink / raw)
To: John Watts; +Cc: barebox
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
> > >
> > >
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] spi: Fix probing SPI drivers without a cs-gpios node
2023-01-22 18:09 ` Alexander Shiyan
@ 2023-01-22 18:19 ` John Watts
2023-01-23 8:43 ` Sascha Hauer
1 sibling, 0 replies; 9+ messages in thread
From: John Watts @ 2023-01-22 18:19 UTC (permalink / raw)
To: Alexander Shiyan; +Cc: barebox
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
> > > >
> > > >
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] spi: Fix probing SPI drivers without a cs-gpios node
2023-01-22 18:09 ` Alexander Shiyan
2023-01-22 18:19 ` John Watts
@ 2023-01-23 8:43 ` Sascha Hauer
2023-01-23 9:47 ` John Watts
1 sibling, 1 reply; 9+ messages in thread
From: Sascha Hauer @ 2023-01-23 8:43 UTC (permalink / raw)
To: Alexander Shiyan; +Cc: John Watts, barebox
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 |
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] spi: Fix probing SPI drivers without a cs-gpios node
2023-01-23 8:43 ` Sascha Hauer
@ 2023-01-23 9:47 ` John Watts
0 siblings, 0 replies; 9+ messages in thread
From: John Watts @ 2023-01-23 9:47 UTC (permalink / raw)
To: Sascha Hauer; +Cc: Alexander Shiyan, barebox
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 |
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2] spi: Fix probing SPI drivers with no cs-gpios
2023-01-22 15:04 [PATCH] spi: Fix probing SPI drivers without a cs-gpios node John Watts
2023-01-22 17:06 ` Alexander Shiyan
@ 2023-01-24 20:05 ` John Watts
2023-01-26 8:14 ` Sascha Hauer
1 sibling, 1 reply; 9+ messages in thread
From: John Watts @ 2023-01-24 20:05 UTC (permalink / raw)
To: barebox; +Cc: 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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2] spi: Fix probing SPI drivers with no cs-gpios
2023-01-24 20:05 ` [PATCH v2] spi: Fix probing SPI drivers with no cs-gpios John Watts
@ 2023-01-26 8:14 ` Sascha Hauer
0 siblings, 0 replies; 9+ messages in thread
From: Sascha Hauer @ 2023-01-26 8:14 UTC (permalink / raw)
To: John Watts; +Cc: barebox
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 |
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-01-26 8:15 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-22 15:04 [PATCH] spi: Fix probing SPI drivers without a cs-gpios node John Watts
2023-01-22 17:06 ` Alexander Shiyan
2023-01-22 17:13 ` John Watts
2023-01-22 18:09 ` Alexander Shiyan
2023-01-22 18:19 ` John Watts
2023-01-23 8:43 ` Sascha Hauer
2023-01-23 9:47 ` John Watts
2023-01-24 20:05 ` [PATCH v2] spi: Fix probing SPI drivers with no cs-gpios John Watts
2023-01-26 8:14 ` Sascha Hauer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox