mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: John Watts <contact@jookia.org>
To: Alexander Shiyan <eagle.alexander923@gmail.com>
Cc: barebox@lists.infradead.org
Subject: Re: [PATCH] spi: Fix probing SPI drivers without a cs-gpios node
Date: Mon, 23 Jan 2023 05:19:47 +1100	[thread overview]
Message-ID: <Y81+Q8lGucHKQEup@novena-choice-citizen> (raw)
In-Reply-To: <CAP1tNvSgOGC3FgGpS8bJjf_WAMZd7RejsO_JCcGX4t8gYdrTzw@mail.gmail.com>

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
> > > >
> > > >



  reply	other threads:[~2023-01-22 18:21 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-22 15:04 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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y81+Q8lGucHKQEup@novena-choice-citizen \
    --to=contact@jookia.org \
    --cc=barebox@lists.infradead.org \
    --cc=eagle.alexander923@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox