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

[Bin Meng]

Hi Jagan,

On Tue, Oct 29, 2019 at 6:15 PM Bin Meng <bmeng.cn at gmail.com> wrote:
>
> Hi Jagan,
>
> On Wed, Oct 16, 2019 at 11:21 PM Jagan Teki <jagan at amarulasolutions.com> wrote:
> >
> > Hi Bin,
> >
> > On Mon, Sep 9, 2019 at 6:30 PM Bin Meng <bmeng.cn at gmail.com> wrote:
> > >
> > > Add chip select number check in spi_find_chip_select().
> > >
> > > Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
> > >
> > > ---
> > >
> > > Changes in v2:
> > > - move the chip select number check to spi_find_chip_select()
> > >
> > >  drivers/spi/spi-uclass.c | 45 ++++++++++++++++++++++++++-------------------
> > >  include/spi.h            |  3 ++-
> > >  2 files changed, 28 insertions(+), 20 deletions(-)
> > >
> > > diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c
> > > index 24de0b5..cdeceb5 100644
> > > --- a/drivers/spi/spi-uclass.c
> > > +++ b/drivers/spi/spi-uclass.c
> > > @@ -179,7 +179,32 @@ int spi_chip_select(struct udevice *dev)
> > >
> > >  int spi_find_chip_select(struct udevice *bus, int cs, struct udevice **devp)
> > >  {
> > > +       struct dm_spi_ops *ops;
> > > +       struct spi_cs_info info;
> > >         struct udevice *dev;
> > > +       int ret;
> > > +
> > > +       /*
> > > +        * Ask the driver. For the moment we don't have CS info.
> > > +        * When we do we could provide the driver with a helper function
> > > +        * to figure out what chip selects are valid, or just handle the
> > > +        * request.
> > > +        */
> > > +       ops = spi_get_ops(bus);
> > > +       if (ops->cs_info) {
> > > +               ret = ops->cs_info(bus, cs, &info);
> > > +       } else {
> > > +               /*
> > > +                * We could assume there is at least one valid chip select.
> > > +                * The driver didn't care enough to tell us.
> > > +                */
> > > +               ret = 0;
> > > +       }
> > > +
> > > +       if (ret) {
> > > +               printf("Invalid cs %d (err=%d)\n", cs, ret);
> > > +               return ret;
> > > +       }
> > >
> >
> > This is breaking 'sf probe' with associated bus and cs on SiFive.
>
> No this does not break anything on SiFive. It enforces the 'sf probe'
> to use the correct CS number on SiFive. And something is wrong when CS
> is 0 on SiFive.

As discussed in another thread, this patch does not break SiFive.
Could you please merge the remaining 2 patches in this series? thanks!

Regards,
Bin