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

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 --- v2 add comments as Simon sugggested.

drivers/spi/spi-uclass.c | 7 ++----- include/spi.h | 5 +++++ 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c index d666272..50e3553 100644 --- a/drivers/spi/spi-uclass.c +++ b/drivers/spi/spi-uclass.c @@ -21,13 +21,12 @@ 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); + /* missing get_speed() and set_mode() methods are OK */ 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 +34,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; diff --git a/include/spi.h b/include/spi.h index 51fdfd6..a418509 100644 --- a/include/spi.h +++ b/include/spi.h @@ -451,6 +451,9 @@ struct dm_spi_ops { /** * Set transfer speed. * This sets a new speed to be applied for next spi_xfer(). + * + * It is optional in the case where the hardware does not support it. + * * @bus: The SPI bus * @hz: The transfer speed * @return 0 if OK, -ve on error @@ -463,6 +466,8 @@ struct dm_spi_ops { * It is unclear if we want to set speed and mode together instead * of separately. * + * It is optional in the case where the hardware does not support it. + * * @bus: The SPI bus * @mode: Requested SPI mode (SPI_... flags) * @return 0 if OK, -ve on error