[U-Boot] [PATCH v2 10/22] spi: Add error checking for invalid bus widths

[Jagan Teki]

On Sun, Nov 20, 2016 at 2:19 AM, Fabio Estevam <festevam at gmail.com> wrote:
> On Sat, Nov 19, 2016 at 6:04 PM, Simon Glass <sjg at chromium.org> wrote:
>
>>> EPROTONOSUPPORT means: /* Protocol not supported */, which does not
>>> seem to be very appropriate here.
>>
>> This is a protocol as far as I can see - you can either use one pin or
>> four pins.
>
> Actually they are SPI modes: one, two or four pins.
>
>>> Why not return -EINVAL instead?
>>
>> The value is valid but is not supported. If we just return -EINVAL for
>> anything we don't like, it makes it harder to root-cause the error. In
>> particular we use -EINVAL when decoding the device tree. But
>> EPROTONOSUPPORT is not widely used.
>
> I think the current behaviour of not returning an error code on an
> invalid mode is correct and it matches what the kernel does in
> drivers/spi/spi.c.
>
> If an invalid mode is passed we just ignore it and operate in single
> mode instead.
>
> Maybe we can make this clearer by printing a message like this:
>
> --- a/drivers/spi/spi-uclass.c
> +++ b/drivers/spi/spi-uclass.c
> @@ -408,7 +408,7 @@ int spi_slave_ofdata_to_platdata(const void *blob, int node,
>                 mode |= SPI_TX_QUAD;
>                 break;
>         default:
> -               error("spi-tx-bus-width %d not supported\n", value);
> +               printf("spi-tx-bus-width %d not supported, operating
> in single mode\n", value);
>                 break;
>         }
>
> @@ -423,7 +423,7 @@ int spi_slave_ofdata_to_platdata(const void *blob, int node,
>                 mode |= SPI_RX_QUAD;
>                 break;
>         default:
> -               error("spi-rx-bus-width %d not supported\n", value);
> +               printf("spi-rx-bus-width %d not supported, operating
> in single mode\n", value);
>                 break;

Yes, this is what I am commenting about.

-EINVAL not needed, we can print "%d is not supporting and operating
in normal/single mode and move on", So-that the dts will fix if
something went wrong.

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.