| Message ID | 1568034003-14675-3-git-send-email-bmeng.cn@gmail.com |
|---|---|
| State | Accepted |
| Commit | 7bacce524d48594dae399f9ee9280ab105f6c8cf |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
| Series | [U-Boot,v2,1/4] dm: spi: Return 0 if driver does not implement ops->cs_info | expand |
On Mon, Sep 9, 2019 at 6:30 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > Add chip select number check in spi_find_chip_select(). Since we discussed the cs check to common call in spi_find_chip_select. Would you please add some more commit description. > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> > > --- Tested-by: Jagan Teki <jagan@amarulasolutions.com> # SoPine
Hi Bin, On Mon, Sep 9, 2019 at 6:30 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > Add chip select number check in spi_find_chip_select(). > > Signed-off-by: Bin Meng <bmeng.cn@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.
Hi Jagan, On Wed, Oct 16, 2019 at 11:21 PM Jagan Teki <jagan@amarulasolutions.com> wrote: > > Hi Bin, > > On Mon, Sep 9, 2019 at 6:30 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > Add chip select number check in spi_find_chip_select(). > > > > Signed-off-by: Bin Meng <bmeng.cn@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. Regards, Bin
Hi Jagan, On Tue, Oct 29, 2019 at 6:15 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi Jagan, > > On Wed, Oct 16, 2019 at 11:21 PM Jagan Teki <jagan@amarulasolutions.com> wrote: > > > > Hi Bin, > > > > On Mon, Sep 9, 2019 at 6:30 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > Add chip select number check in spi_find_chip_select(). > > > > > > Signed-off-by: Bin Meng <bmeng.cn@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
Hi Jagan, On Mon, Nov 18, 2019 at 3:15 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi Jagan, > > On Tue, Oct 29, 2019 at 6:15 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > Hi Jagan, > > > > On Wed, Oct 16, 2019 at 11:21 PM Jagan Teki <jagan@amarulasolutions.com> wrote: > > > > > > Hi Bin, > > > > > > On Mon, Sep 9, 2019 at 6:30 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > Add chip select number check in spi_find_chip_select(). > > > > > > > > Signed-off-by: Bin Meng <bmeng.cn@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
On Thu, Jan 9, 2020 at 7:17 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi Jagan, > > On Mon, Nov 18, 2019 at 3:15 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > Hi Jagan, > > > > On Tue, Oct 29, 2019 at 6:15 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > Hi Jagan, > > > > > > On Wed, Oct 16, 2019 at 11:21 PM Jagan Teki <jagan@amarulasolutions.com> wrote: > > > > > > > > Hi Bin, > > > > > > > > On Mon, Sep 9, 2019 at 6:30 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > > > Add chip select number check in spi_find_chip_select(). > > > > > > > > > > Signed-off-by: Bin Meng <bmeng.cn@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? I took it already will send it PR soon. Jagan.
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; + } for (device_find_first_child(bus, &dev); dev; device_find_next_child(&dev)) { @@ -214,7 +239,6 @@ int spi_cs_is_valid(unsigned int busnum, unsigned int cs) int spi_cs_info(struct udevice *bus, uint cs, struct spi_cs_info *info) { struct spi_cs_info local_info; - struct dm_spi_ops *ops; int ret; if (!info) @@ -223,24 +247,7 @@ int spi_cs_info(struct udevice *bus, uint cs, struct spi_cs_info *info) /* If there is a device attached, return it */ info->dev = NULL; ret = spi_find_chip_select(bus, cs, &info->dev); - if (!ret) - return 0; - - /* - * Otherwise 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) - return ops->cs_info(bus, cs, info); - - /* - * We could assume there is at least one valid chip select. - * The driver didn't care enough to tell us. - */ - return 0; + return ret == -ENODEV ? 0 : ret; } int spi_find_bus_and_cs(int busnum, int cs, struct udevice **busp, diff --git a/include/spi.h b/include/spi.h index cc344de..6b144bc 100644 --- a/include/spi.h +++ b/include/spi.h @@ -528,7 +528,8 @@ int spi_chip_select(struct udevice *slave); * @bus: SPI bus to search * @cs: Chip select to look for * @devp: Returns the slave device if found - * @return 0 if found, -ENODEV on error + * @return 0 if found, -EINVAL if cs is invalid, -ENODEV if no device attached, + * other -ve value on error */ int spi_find_chip_select(struct udevice *bus, int cs, struct udevice **devp);
Add chip select number check in spi_find_chip_select(). Signed-off-by: Bin Meng <bmeng.cn@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(-)