[U-Boot] [PATCH 2/2] dm: spi: Check cs number before accessing slaves

[Jagan Teki]

On Sat, Sep 7, 2019 at 9:19 AM Bin Meng <bmeng.cn at gmail.com> wrote:
>
> Hi Jagan,
>
> On Sat, Sep 7, 2019 at 11:45 AM Jagan Teki <jagan at amarulasolutions.com> wrote:
> >
> > On Thu, Aug 29, 2019 at 7:40 PM Bin Meng <bmeng.cn at gmail.com> wrote:
> > >
> > > In spi_get_bus_and_cs() only bus number is checked before accessing
> > > slaves. We should check cs number as well.
> > >
> > > Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
> > > ---
> > >
> > >  drivers/spi/spi-uclass.c | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > >
> > > diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c
> > > index 24de0b5..f633eb5 100644
> > > --- a/drivers/spi/spi-uclass.c
> > > +++ b/drivers/spi/spi-uclass.c
> > > @@ -271,6 +271,7 @@ int spi_get_bus_and_cs(int busnum, int cs, int speed, int mode,
> > >  {
> > >         struct udevice *bus, *dev;
> > >         struct dm_spi_slave_platdata *plat;
> > > +       struct spi_cs_info info;
> > >         bool created = false;
> > >         int ret;
> > >
> > > @@ -283,6 +284,11 @@ int spi_get_bus_and_cs(int busnum, int cs, int speed, int mode,
> > >                 printf("Invalid bus %d (err=%d)\n", busnum, ret);
> > >                 return ret;
> > >         }
> > > +       ret = spi_cs_info(bus, cs, &info);
> > > +       if (ret) {
> > > +               printf("Invalid cs %d (err=%d)\n", cs, ret);
> > > +               return ret;
> > > +       }
> > >         ret = spi_find_chip_select(bus, cs, &dev);
> >
> > I think it would rather check the cs_info in spi_find_chip_select
> > function itself, make more sense.
>
> This routine spi_get_bus_and_cs() has both busnum and cs as
> parameters, but only busnum is checked so far. It looks to me more
> appropriate to check the cs in spi_get_bus_and_cs() directly, instead
> of going into spi_find_chip_select().

True, but the spi_find_chip_select is also used in other calls (like
spi_find_bus_and_cs). Checking cs_info on the core function might help
to calling it on multiple places. what do you think?