diff mbox

[U-Boot,v3] spi: Add error checking for invalid bus widths

Message ID 1480474813-17451-1-git-send-email-sjg@chromium.org
State Accepted
Commit 1b7c28f5147144d7902d048ca90be58987899c25
Delegated to: Jagannadha Sutradharudu Teki
Headers show

Commit Message

Simon Glass Nov. 30, 2016, 3 a.m. UTC
At present an invalid bus width prints a message but does not return an
error. This is the opposite of the correct behaviour. Adjust it to avoid
code bloat in the common case, and avoid hard-to-debug failure in the
uncommon case.

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

Changes in v3:
- Display an error in U-Boot proper, but then continue
- Drop patches previously applied

 drivers/spi/spi-uclass.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Jagan Teki Dec. 1, 2016, 12:45 p.m. UTC | #1
On Wed, Nov 30, 2016 at 8:30 AM, Simon Glass <sjg@chromium.org> wrote:
> At present an invalid bus width prints a message but does not return an
> error. This is the opposite of the correct behaviour. Adjust it to avoid
> code bloat in the common case, and avoid hard-to-debug failure in the
> uncommon case.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> Changes in v3:
> - Display an error in U-Boot proper, but then continue
> - Drop patches previously applied
>
>  drivers/spi/spi-uclass.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c
> index f59a701..1ab5b75 100644
> --- a/drivers/spi/spi-uclass.c
> +++ b/drivers/spi/spi-uclass.c
> @@ -418,7 +418,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);
> +               warn_non_spl("spi-tx-bus-width %d not supported\n", value);

I thought in SPL it will reverse like debug and return by error, make sense?

thanks!
Simon Glass Dec. 5, 2016, 6:24 a.m. UTC | #2
Hi Jagan,

On 1 December 2016 at 05:45, Jagan Teki <jagan@openedev.com> wrote:
> On Wed, Nov 30, 2016 at 8:30 AM, Simon Glass <sjg@chromium.org> wrote:
>> At present an invalid bus width prints a message but does not return an
>> error. This is the opposite of the correct behaviour. Adjust it to avoid
>> code bloat in the common case, and avoid hard-to-debug failure in the
>> uncommon case.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>> Changes in v3:
>> - Display an error in U-Boot proper, but then continue
>> - Drop patches previously applied
>>
>>  drivers/spi/spi-uclass.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c
>> index f59a701..1ab5b75 100644
>> --- a/drivers/spi/spi-uclass.c
>> +++ b/drivers/spi/spi-uclass.c
>> @@ -418,7 +418,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);
>> +               warn_non_spl("spi-tx-bus-width %d not supported\n", value);
>
> I thought in SPL it will reverse like debug and return by error, make sense?

Sorry I don't understand this comment. The intent here is to:

- do nothing in SPL
- warn in U-Boot proper with printf()

Are you saying that it should use debug() in SPL?

Regards,
Simon
Jagan Teki Dec. 6, 2016, 6:51 p.m. UTC | #3
On Mon, Dec 5, 2016 at 7:24 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi Jagan,
>
> On 1 December 2016 at 05:45, Jagan Teki <jagan@openedev.com> wrote:
>> On Wed, Nov 30, 2016 at 8:30 AM, Simon Glass <sjg@chromium.org> wrote:
>>> At present an invalid bus width prints a message but does not return an
>>> error. This is the opposite of the correct behaviour. Adjust it to avoid
>>> code bloat in the common case, and avoid hard-to-debug failure in the
>>> uncommon case.
>>>
>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>> ---
>>>
>>> Changes in v3:
>>> - Display an error in U-Boot proper, but then continue
>>> - Drop patches previously applied
>>>
>>>  drivers/spi/spi-uclass.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c
>>> index f59a701..1ab5b75 100644
>>> --- a/drivers/spi/spi-uclass.c
>>> +++ b/drivers/spi/spi-uclass.c
>>> @@ -418,7 +418,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);
>>> +               warn_non_spl("spi-tx-bus-width %d not supported\n", value);
>>
>> I thought in SPL it will reverse like debug and return by error, make sense?
>
> Sorry I don't understand this comment. The intent here is to:
>
> - do nothing in SPL
> - warn in U-Boot proper with printf()
>
> Are you saying that it should use debug() in SPL?

I though SPL should return error, unlike U-Boot. So-that SPL should
use proper bus-width that will eventually fix the dt.

thanks!
diff mbox

Patch

diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c
index f59a701..1ab5b75 100644
--- a/drivers/spi/spi-uclass.c
+++ b/drivers/spi/spi-uclass.c
@@ -418,7 +418,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);
+		warn_non_spl("spi-tx-bus-width %d not supported\n", value);
 		break;
 	}
 
@@ -433,7 +433,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);
+		warn_non_spl("spi-rx-bus-width %d not supported\n", value);
 		break;
 	}