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?