Hi Jagan,
On 21 November 2016 at 10:57, Jagan Teki <ja...@openedev.com> wrote:
> 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.
Well if you add printf() values you will bloat the code for little
benefit. If the device tree is invalid it really should be fixed.
Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
