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

[Bin Meng]

Hi Jagan,

On Mon, Nov 18, 2019 at 3:15 PM Bin Meng <bmeng.cn at gmail.com> wrote:
>
> 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!

Ping?

Regards,
Bin