[U-Boot] [PATCH 1/2] dm: spi: Return 0 if driver does not implement ops->cs_info
If an SPI controller driver does not implement ops->cs_info, that probably means any chip select number could be valid, hence let's return 0 for spi_cs_info().
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
drivers/spi/spi-uclass.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c index 88cb2a1..24de0b5 100644 --- a/drivers/spi/spi-uclass.c +++ b/drivers/spi/spi-uclass.c @@ -237,11 +237,10 @@ int spi_cs_info(struct udevice *bus, uint cs, struct spi_cs_info *info) return ops->cs_info(bus, cs, info);
/* - * We could assume there is at least one valid chip select, but best - * to be sure and return an error in this case. The driver didn't - * care enough to tell us. + * We could assume there is at least one valid chip select. + * The driver didn't care enough to tell us. */ - return -ENODEV; + return 0; }
int spi_find_bus_and_cs(int busnum, int cs, struct udevice **busp,
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@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);
/*
On Thu, Aug 29, 2019 at 10:10 PM Bin Meng bmeng.cn@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@gmail.com
drivers/spi/spi-uclass.c | 6 ++++++ 1 file changed, 6 insertions(+)
Ping?
On Thu, Aug 29, 2019 at 7:40 PM Bin Meng bmeng.cn@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@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.
Hi Jagan,
On Sat, Sep 7, 2019 at 11:45 AM Jagan Teki jagan@amarulasolutions.com wrote:
On Thu, Aug 29, 2019 at 7:40 PM Bin Meng bmeng.cn@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@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().
Regards, Bin
On Sat, Sep 7, 2019 at 9:19 AM Bin Meng bmeng.cn@gmail.com wrote:
Hi Jagan,
On Sat, Sep 7, 2019 at 11:45 AM Jagan Teki jagan@amarulasolutions.com wrote:
On Thu, Aug 29, 2019 at 7:40 PM Bin Meng bmeng.cn@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@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?
Hi Jagan,
On Sat, Sep 7, 2019 at 11:58 AM Jagan Teki jagan@amarulasolutions.com wrote:
On Sat, Sep 7, 2019 at 9:19 AM Bin Meng bmeng.cn@gmail.com wrote:
Hi Jagan,
On Sat, Sep 7, 2019 at 11:45 AM Jagan Teki jagan@amarulasolutions.com wrote:
On Thu, Aug 29, 2019 at 7:40 PM Bin Meng bmeng.cn@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@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?
Ok, makes sense. I will do it in v2.
Regards, Bin
On Thu, Aug 29, 2019 at 10:10 PM Bin Meng bmeng.cn@gmail.com wrote:
If an SPI controller driver does not implement ops->cs_info, that probably means any chip select number could be valid, hence let's return 0 for spi_cs_info().
Signed-off-by: Bin Meng bmeng.cn@gmail.com
drivers/spi/spi-uclass.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
Ping?
On Thu, Aug 29, 2019 at 7:40 PM Bin Meng bmeng.cn@gmail.com wrote:
If an SPI controller driver does not implement ops->cs_info, that probably means any chip select number could be valid, hence let's return 0 for spi_cs_info().
Signed-off-by: Bin Meng bmeng.cn@gmail.com
Reviewed-by: Jagan Teki jagan@amarulasolutions.com
participants (2)
-
Bin Meng -
Jagan Teki