[U-Boot] [PATCH] spi: ignore set speed and mode if not available

[U-Boot] [PATCH] spi: ignore set speed and mode if not available

[Thomas Chou]

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(-)

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;
-- 
2.1.4

[U-Boot] [PATCH] spi: ignore set speed and mode if not available

[Marek Vasut]

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

[U-Boot] [PATCH] spi: ignore set speed and mode if not available

[Jagan Teki]

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.

[U-Boot] [PATCH] spi: ignore set speed and mode if not available

[Simon Glass]

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

[U-Boot] [PATCH] spi: ignore set speed and mode if not available

[Thomas Chou]

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

[U-Boot] [PATCH v2] spi: ignore set speed and mode if not available

[Thomas Chou]

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
-- 
2.1.4

[U-Boot] [PATCH v2] spi: ignore set speed and mode if not available

[Simon Glass]

On 4 October 2015 at 13:23, 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>
> 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(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>