| Message ID | 1442907681-14489-1-git-send-email-thomas@wytron.com.tw |
|---|---|
| State | Accepted, archived |
| Delegated to: | Thomas Chou |
| Headers | show |
On Tuesday, September 22, 2015 at 09:41:21 AM, Thomas Chou wrote: > Some cores, such as Altera SPI and QuadSPI, can not change > speed and mode at runtime. Ignore the operation which is > not available. > > Signed-off-by: Thomas Chou <thomas@wytron.com.tw> Acked-by: Marek Vasut <marex@denx.de> Thanks! Best regards, Marek Vasut
On 22 September 2015 at 13:40, Marek Vasut <marex@denx.de> wrote: > On Tuesday, September 22, 2015 at 09:41:21 AM, Thomas Chou wrote: >> Some cores, such as Altera SPI and QuadSPI, can not change >> speed and mode at runtime. Ignore the operation which is >> not available. >> >> Signed-off-by: Thomas Chou <thomas@wytron.com.tw> > > Acked-by: Marek Vasut <marex@denx.de> Reviewed-by: Jagan Teki <jteki@openedev.com> Pick this one as well. -- Jagan.
Hi Thomas, On 22 September 2015 at 08:41, Thomas Chou <thomas@wytron.com.tw> wrote: > Some cores, such as Altera SPI and QuadSPI, can not change > speed and mode at runtime. Ignore the operation which is > not available. > > Signed-off-by: Thomas Chou <thomas@wytron.com.tw> > --- > drivers/spi/spi-uclass.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) This looks OK, but can you please update the method documentation for set_speed() and set_mode() to indicate that they are optional in the case where the hardware does not support it. > > diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c > index d666272..5298073 100644 > --- a/drivers/spi/spi-uclass.c > +++ b/drivers/spi/spi-uclass.c > @@ -21,13 +21,11 @@ DECLARE_GLOBAL_DATA_PTR; > static int spi_set_speed_mode(struct udevice *bus, int speed, int mode) Please add a comment to this function indicated that missing set_speed() and set_mode() methods are OK. > { > struct dm_spi_ops *ops; > - int ret; > + int ret = 0; > > ops = spi_get_ops(bus); > if (ops->set_speed) > ret = ops->set_speed(bus, speed); > - else > - ret = -EINVAL; > if (ret) { > printf("Cannot set speed (err=%d)\n", ret); > return ret; > @@ -35,8 +33,6 @@ static int spi_set_speed_mode(struct udevice *bus, int speed, int mode) > > if (ops->set_mode) > ret = ops->set_mode(bus, mode); > - else > - ret = -EINVAL; > if (ret) { > printf("Cannot set mode (err=%d)\n", ret); > return ret; > -- > 2.1.4 > Regards, Simon
Hi Simon, On 10/03/2015 10:28 PM, Simon Glass wrote: > Hi Thomas, > > On 22 September 2015 at 08:41, Thomas Chou <thomas@wytron.com.tw> wrote: >> Some cores, such as Altera SPI and QuadSPI, can not change >> speed and mode at runtime. Ignore the operation which is >> not available. >> >> Signed-off-by: Thomas Chou <thomas@wytron.com.tw> >> --- >> drivers/spi/spi-uclass.c | 6 +----- >> 1 file changed, 1 insertion(+), 5 deletions(-) > > This looks OK, but can you please update the method documentation for > set_speed() and set_mode() to indicate that they are optional in the > case where the hardware does not support it. > >> >> diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c >> index d666272..5298073 100644 >> --- a/drivers/spi/spi-uclass.c >> +++ b/drivers/spi/spi-uclass.c >> @@ -21,13 +21,11 @@ DECLARE_GLOBAL_DATA_PTR; >> static int spi_set_speed_mode(struct udevice *bus, int speed, int mode) > > Please add a comment to this function indicated that missing > set_speed() and set_mode() methods are OK. Thanks a lot for your review. The comments will be added. Best regards, Thomas Chou
diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c index d666272..5298073 100644 --- a/drivers/spi/spi-uclass.c +++ b/drivers/spi/spi-uclass.c @@ -21,13 +21,11 @@ DECLARE_GLOBAL_DATA_PTR; static int spi_set_speed_mode(struct udevice *bus, int speed, int mode) { struct dm_spi_ops *ops; - int ret; + int ret = 0; ops = spi_get_ops(bus); if (ops->set_speed) ret = ops->set_speed(bus, speed); - else - ret = -EINVAL; if (ret) { printf("Cannot set speed (err=%d)\n", ret); return ret; @@ -35,8 +33,6 @@ static int spi_set_speed_mode(struct udevice *bus, int speed, int mode) if (ops->set_mode) ret = ops->set_mode(bus, mode); - else - ret = -EINVAL; if (ret) { printf("Cannot set mode (err=%d)\n", ret); return ret;
Some cores, such as Altera SPI and QuadSPI, can not change speed and mode at runtime. Ignore the operation which is not available. Signed-off-by: Thomas Chou <thomas@wytron.com.tw> --- drivers/spi/spi-uclass.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)