On Sun, Nov 20, 2016 at 2:19 AM, Fabio Estevam <feste...@gmail.com> wrote:
> On Sat, Nov 19, 2016 at 6:04 PM, Simon Glass <s...@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.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
