[U-Boot] [PATCH v2] dm: spi: prevent setting a speed of 0 Hz

[U-Boot] [PATCH v2] dm: spi: prevent setting a speed of 0 Hz

[Simon Goldschmidt]

When the device tree is missing a correct spi slave description below
the bus (compatible "spi-flash" or spi-max-frequency are missing),
the 'set_speed' callback can be called with 'speed' == 0 Hz.
At least with cadence qspi, this leads to a division by zero.

Prevent this by initializing speed to 100 kHz in this case (same
fallback value as is done in 'dm_spi_claim_bus') and issue a warning
to console.

Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
---

Changes in v2:
- create a define for the 100 kHz constant (used 3 times)
- assign default value when binding new device and speed is 0
- assign default value when spi_slave_ofdata_to_platdata finds
  that spi-max-frequency property is missing

 drivers/spi/spi-uclass.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c
index b84255bd27..2bc289a74c 100644
--- a/drivers/spi/spi-uclass.c
+++ b/drivers/spi/spi-uclass.c
@@ -15,6 +15,8 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+#define SPI_DEFAULT_SPEED_HZ 100000
+
 static int spi_set_speed_mode(struct udevice *bus, int speed, int mode)
 {
 	struct dm_spi_ops *ops;
@@ -58,7 +60,7 @@ int dm_spi_claim_bus(struct udevice *dev)
 			speed = spi->max_hz;
 	}
 	if (!speed)
-		speed = 100000;
+		speed = SPI_DEFAULT_SPEED_HZ;
 	if (speed != slave->speed) {
 		int ret = spi_set_speed_mode(bus, speed, slave->mode);
 
@@ -300,7 +302,13 @@ int spi_get_bus_and_cs(int busnum, int cs, int speed, int mode,
 		}
 		plat = dev_get_parent_platdata(dev);
 		plat->cs = cs;
-		plat->max_hz = speed;
+		if (speed) {
+			plat->max_hz = speed;
+		} else {
+			printf("Warning: SPI speed fallback to %u kHz\n",
+			       SPI_DEFAULT_SPEED_HZ / 1000);
+			plat->max_hz = SPI_DEFAULT_SPEED_HZ;
+		}
 		plat->mode = mode;
 		created = true;
 	} else if (ret) {
@@ -374,7 +382,8 @@ int spi_slave_ofdata_to_platdata(struct udevice *dev,
 	int value;
 
 	plat->cs = dev_read_u32_default(dev, "reg", -1);
-	plat->max_hz = dev_read_u32_default(dev, "spi-max-frequency", 0);
+	plat->max_hz = dev_read_u32_default(dev, "spi-max-frequency",
+					    SPI_DEFAULT_SPEED_HZ);
 	if (dev_read_bool(dev, "spi-cpol"))
 		mode |= SPI_CPOL;
 	if (dev_read_bool(dev, "spi-cpha"))
-- 
2.17.1

[U-Boot] [PATCH v2] dm: spi: prevent setting a speed of 0 Hz

[Jagan Teki]

On Wed, Oct 31, 2018 at 1:40 AM Simon Goldschmidt
<simon.k.r.goldschmidt@gmail.com> wrote:
>
> When the device tree is missing a correct spi slave description below
> the bus (compatible "spi-flash" or spi-max-frequency are missing),
> the 'set_speed' callback can be called with 'speed' == 0 Hz.
> At least with cadence qspi, this leads to a division by zero.
>
> Prevent this by initializing speed to 100 kHz in this case (same
> fallback value as is done in 'dm_spi_claim_bus') and issue a warning
> to console.

Why can't driver plat->frequency in cadence driver initialize 100KH?
plat->frequency = fdtdec_get_int(blob, node, "spi-max-frequency", 100000)

[U-Boot] [PATCH v2] dm: spi: prevent setting a speed of 0 Hz

[Simon Goldschmidt]

On Wed, Oct 31, 2018 at 7:22 AM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> On Wed, Oct 31, 2018 at 1:40 AM Simon Goldschmidt
> <simon.k.r.goldschmidt@gmail.com> wrote:
> >
> > When the device tree is missing a correct spi slave description below
> > the bus (compatible "spi-flash" or spi-max-frequency are missing),
> > the 'set_speed' callback can be called with 'speed' == 0 Hz.
> > At least with cadence qspi, this leads to a division by zero.
> >
> > Prevent this by initializing speed to 100 kHz in this case (same
> > fallback value as is done in 'dm_spi_claim_bus') and issue a warning
> > to console.
>
> Why can't driver plat->frequency in cadence driver initialize 100KH?
> plat->frequency = fdtdec_get_int(blob, node, "spi-max-frequency", 100000)

I'm not sure I understand.
The cadence driver initializes its 'plat->max_hz' from
"spi-max-frequency" property and defaults to 500 kHz. No problem
there.

However, the problem I want to solve is if someone puts a flash chip
below there which has compatible != "spi-flash", they will get a hard
fault which is hard to debug. This is because the node is not parsed
because of the wrong compatible string (even if there is an
"spi-max-frequency" property) and thus, "sf probe" tries to continue
with 0 Hz.

And this can happen easily when porting device trees from Linux as
there, the boards have compatible "n25q00" etc. instead of
"spi-flash", which is U-Boot specific (sadly).

This patch is not required to make valid U-Boot devicetrees work, it
is meant to get better error handling for devicetrees ported from
Linux.

An even better fix would be for U-Boot not to require the compatible =
"spi-flash" string but just work correctly with Linux device trees,
but that's not within my possibilities right now :-(

Simon

[U-Boot] [PATCH v2] dm: spi: prevent setting a speed of 0 Hz

[Simon Goldschmidt]

On 31.10.2018 07:42, Simon Goldschmidt wrote:
> On Wed, Oct 31, 2018 at 7:22 AM Jagan Teki <jagan@amarulasolutions.com> wrote:
>> On Wed, Oct 31, 2018 at 1:40 AM Simon Goldschmidt
>> <simon.k.r.goldschmidt@gmail.com> wrote:
>>> When the device tree is missing a correct spi slave description below
>>> the bus (compatible "spi-flash" or spi-max-frequency are missing),
>>> the 'set_speed' callback can be called with 'speed' == 0 Hz.
>>> At least with cadence qspi, this leads to a division by zero.
>>>
>>> Prevent this by initializing speed to 100 kHz in this case (same
>>> fallback value as is done in 'dm_spi_claim_bus') and issue a warning
>>> to console.
>> Why can't driver plat->frequency in cadence driver initialize 100KH?
>> plat->frequency = fdtdec_get_int(blob, node, "spi-max-frequency", 100000)
> I'm not sure I understand.
> The cadence driver initializes its 'plat->max_hz' from
> "spi-max-frequency" property and defaults to 500 kHz. No problem
> there.
>
> However, the problem I want to solve is if someone puts a flash chip
> below there which has compatible != "spi-flash", they will get a hard
> fault which is hard to debug. This is because the node is not parsed
> because of the wrong compatible string (even if there is an
> "spi-max-frequency" property) and thus, "sf probe" tries to continue
> with 0 Hz.
>
> And this can happen easily when porting device trees from Linux as
> there, the boards have compatible "n25q00" etc. instead of
> "spi-flash", which is U-Boot specific (sadly).
>
> This patch is not required to make valid U-Boot devicetrees work, it
> is meant to get better error handling for devicetrees ported from
> Linux.
>
> An even better fix would be for U-Boot not to require the compatible =
> "spi-flash" string but just work correctly with Linux device trees,
> but that's not within my possibilities right now :-(

Ping? This is a bug that should hit nearly everyone porting a board dts 
working in Linux to U-Boot, can we please have some kind of fix for this?
Currently, using a dts with an spi flash chip that works in Linux may 
abort in U-Boot...

Simon

[U-Boot] [PATCH v2] dm: spi: prevent setting a speed of 0 Hz

[Simon Glass]

On Fri, 16 Nov 2018 at 12:40, Simon Goldschmidt
<simon.k.r.goldschmidt@gmail.com> wrote:
>
> On 31.10.2018 07:42, Simon Goldschmidt wrote:
> > On Wed, Oct 31, 2018 at 7:22 AM Jagan Teki <jagan@amarulasolutions.com> wrote:
> >> On Wed, Oct 31, 2018 at 1:40 AM Simon Goldschmidt
> >> <simon.k.r.goldschmidt@gmail.com> wrote:
> >>> When the device tree is missing a correct spi slave description below
> >>> the bus (compatible "spi-flash" or spi-max-frequency are missing),
> >>> the 'set_speed' callback can be called with 'speed' == 0 Hz.
> >>> At least with cadence qspi, this leads to a division by zero.
> >>>
> >>> Prevent this by initializing speed to 100 kHz in this case (same
> >>> fallback value as is done in 'dm_spi_claim_bus') and issue a warning
> >>> to console.
> >> Why can't driver plat->frequency in cadence driver initialize 100KH?
> >> plat->frequency = fdtdec_get_int(blob, node, "spi-max-frequency", 100000)
> > I'm not sure I understand.
> > The cadence driver initializes its 'plat->max_hz' from
> > "spi-max-frequency" property and defaults to 500 kHz. No problem
> > there.
> >
> > However, the problem I want to solve is if someone puts a flash chip
> > below there which has compatible != "spi-flash", they will get a hard
> > fault which is hard to debug. This is because the node is not parsed
> > because of the wrong compatible string (even if there is an
> > "spi-max-frequency" property) and thus, "sf probe" tries to continue
> > with 0 Hz.
> >
> > And this can happen easily when porting device trees from Linux as
> > there, the boards have compatible "n25q00" etc. instead of
> > "spi-flash", which is U-Boot specific (sadly).
> >
> > This patch is not required to make valid U-Boot devicetrees work, it
> > is meant to get better error handling for devicetrees ported from
> > Linux.
> >
> > An even better fix would be for U-Boot not to require the compatible =
> > "spi-flash" string but just work correctly with Linux device trees,
> > but that's not within my possibilities right now :-(
>
> Ping? This is a bug that should hit nearly everyone porting a board dts
> working in Linux to U-Boot, can we please have some kind of fix for this?
> Currently, using a dts with an spi flash chip that works in Linux may
> abort in U-Boot...

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

[U-Boot] [PATCH v2] dm: spi: prevent setting a speed of 0 Hz

[sjg at google.com]

On Fri, 16 Nov 2018 at 12:40, Simon Goldschmidt
<simon.k.r.goldschmidt@gmail.com> wrote:
>
> On 31.10.2018 07:42, Simon Goldschmidt wrote:
> > On Wed, Oct 31, 2018 at 7:22 AM Jagan Teki <jagan@amarulasolutions.com> wrote:
> >> On Wed, Oct 31, 2018 at 1:40 AM Simon Goldschmidt
> >> <simon.k.r.goldschmidt@gmail.com> wrote:
> >>> When the device tree is missing a correct spi slave description below
> >>> the bus (compatible "spi-flash" or spi-max-frequency are missing),
> >>> the 'set_speed' callback can be called with 'speed' == 0 Hz.
> >>> At least with cadence qspi, this leads to a division by zero.
> >>>
> >>> Prevent this by initializing speed to 100 kHz in this case (same
> >>> fallback value as is done in 'dm_spi_claim_bus') and issue a warning
> >>> to console.
> >> Why can't driver plat->frequency in cadence driver initialize 100KH?
> >> plat->frequency = fdtdec_get_int(blob, node, "spi-max-frequency", 100000)
> > I'm not sure I understand.
> > The cadence driver initializes its 'plat->max_hz' from
> > "spi-max-frequency" property and defaults to 500 kHz. No problem
> > there.
> >
> > However, the problem I want to solve is if someone puts a flash chip
> > below there which has compatible != "spi-flash", they will get a hard
> > fault which is hard to debug. This is because the node is not parsed
> > because of the wrong compatible string (even if there is an
> > "spi-max-frequency" property) and thus, "sf probe" tries to continue
> > with 0 Hz.
> >
> > And this can happen easily when porting device trees from Linux as
> > there, the boards have compatible "n25q00" etc. instead of
> > "spi-flash", which is U-Boot specific (sadly).
> >
> > This patch is not required to make valid U-Boot devicetrees work, it
> > is meant to get better error handling for devicetrees ported from
> > Linux.
> >
> > An even better fix would be for U-Boot not to require the compatible =
> > "spi-flash" string but just work correctly with Linux device trees,
> > but that's not within my possibilities right now :-(
>
> Ping? This is a bug that should hit nearly everyone porting a board dts
> working in Linux to U-Boot, can we please have some kind of fix for this?
> Currently, using a dts with an spi flash chip that works in Linux may
> abort in U-Boot...

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

Applied to u-boot-dm/master, thanks!

[U-Boot] [PATCH v2] dm: spi: prevent setting a speed of 0 Hz

[Jagan Teki]

On Fri, Nov 23, 2018 at 1:51 AM <sjg@google.com> wrote:
>
> On Fri, 16 Nov 2018 at 12:40, Simon Goldschmidt
> <simon.k.r.goldschmidt@gmail.com> wrote:
> >
> > On 31.10.2018 07:42, Simon Goldschmidt wrote:
> > > On Wed, Oct 31, 2018 at 7:22 AM Jagan Teki <jagan@amarulasolutions.com> wrote:
> > >> On Wed, Oct 31, 2018 at 1:40 AM Simon Goldschmidt
> > >> <simon.k.r.goldschmidt@gmail.com> wrote:
> > >>> When the device tree is missing a correct spi slave description below
> > >>> the bus (compatible "spi-flash" or spi-max-frequency are missing),
> > >>> the 'set_speed' callback can be called with 'speed' == 0 Hz.
> > >>> At least with cadence qspi, this leads to a division by zero.
> > >>>
> > >>> Prevent this by initializing speed to 100 kHz in this case (same
> > >>> fallback value as is done in 'dm_spi_claim_bus') and issue a warning
> > >>> to console.
> > >> Why can't driver plat->frequency in cadence driver initialize 100KH?
> > >> plat->frequency = fdtdec_get_int(blob, node, "spi-max-frequency", 100000)
> > > I'm not sure I understand.
> > > The cadence driver initializes its 'plat->max_hz' from
> > > "spi-max-frequency" property and defaults to 500 kHz. No problem
> > > there.
> > >
> > > However, the problem I want to solve is if someone puts a flash chip
> > > below there which has compatible != "spi-flash", they will get a hard
> > > fault which is hard to debug. This is because the node is not parsed
> > > because of the wrong compatible string (even if there is an
> > > "spi-max-frequency" property) and thus, "sf probe" tries to continue
> > > with 0 Hz.
> > >
> > > And this can happen easily when porting device trees from Linux as
> > > there, the boards have compatible "n25q00" etc. instead of
> > > "spi-flash", which is U-Boot specific (sadly).
> > >
> > > This patch is not required to make valid U-Boot devicetrees work, it
> > > is meant to get better error handling for devicetrees ported from
> > > Linux.
> > >
> > > An even better fix would be for U-Boot not to require the compatible =
> > > "spi-flash" string but just work correctly with Linux device trees,
> > > but that's not within my possibilities right now :-(
> >
> > Ping? This is a bug that should hit nearly everyone porting a board dts
> > working in Linux to U-Boot, can we please have some kind of fix for this?
> > Currently, using a dts with an spi flash chip that works in Linux may
> > abort in U-Boot...
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> Applied to u-boot-dm/master, thanks!

I hold this because, there is a discussion going on with me and
Simon.k about Linux compatible "jedec,spi-nor".  adding support for
this binding would eventually not reproduce this issue.

[U-Boot] [PATCH v2] dm: spi: prevent setting a speed of 0 Hz

[Simon Goldschmidt]

[Responding from my work address due to problems with gmail]

On 23.11.2018 13:27, Jagan Teki wrote:
> On Fri, Nov 23, 2018 at 1:51 AM <sjg@google.com> wrote:
>>
>> On Fri, 16 Nov 2018 at 12:40, Simon Goldschmidt
>> <simon.k.r.goldschmidt@gmail.com> wrote:
>>>
>>> On 31.10.2018 07:42, Simon Goldschmidt wrote:
>>>> On Wed, Oct 31, 2018 at 7:22 AM Jagan Teki <jagan@amarulasolutions.com> wrote:
>>>>> On Wed, Oct 31, 2018 at 1:40 AM Simon Goldschmidt
>>>>> <simon.k.r.goldschmidt@gmail.com> wrote:
>>>>>> When the device tree is missing a correct spi slave description below
>>>>>> the bus (compatible "spi-flash" or spi-max-frequency are missing),
>>>>>> the 'set_speed' callback can be called with 'speed' == 0 Hz.
>>>>>> At least with cadence qspi, this leads to a division by zero.
>>>>>>
>>>>>> Prevent this by initializing speed to 100 kHz in this case (same
>>>>>> fallback value as is done in 'dm_spi_claim_bus') and issue a warning
>>>>>> to console.
>>>>> Why can't driver plat->frequency in cadence driver initialize 100KH?
>>>>> plat->frequency = fdtdec_get_int(blob, node, "spi-max-frequency", 100000)
>>>> I'm not sure I understand.
>>>> The cadence driver initializes its 'plat->max_hz' from
>>>> "spi-max-frequency" property and defaults to 500 kHz. No problem
>>>> there.
>>>>
>>>> However, the problem I want to solve is if someone puts a flash chip
>>>> below there which has compatible != "spi-flash", they will get a hard
>>>> fault which is hard to debug. This is because the node is not parsed
>>>> because of the wrong compatible string (even if there is an
>>>> "spi-max-frequency" property) and thus, "sf probe" tries to continue
>>>> with 0 Hz.
>>>>
>>>> And this can happen easily when porting device trees from Linux as
>>>> there, the boards have compatible "n25q00" etc. instead of
>>>> "spi-flash", which is U-Boot specific (sadly).
>>>>
>>>> This patch is not required to make valid U-Boot devicetrees work, it
>>>> is meant to get better error handling for devicetrees ported from
>>>> Linux.
>>>>
>>>> An even better fix would be for U-Boot not to require the compatible =
>>>> "spi-flash" string but just work correctly with Linux device trees,
>>>> but that's not within my possibilities right now :-(
>>>
>>> Ping? This is a bug that should hit nearly everyone porting a board dts
>>> working in Linux to U-Boot, can we please have some kind of fix for this?
>>> Currently, using a dts with an spi flash chip that works in Linux may
>>> abort in U-Boot...
>>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>
>> Applied to u-boot-dm/master, thanks!
> 
> I hold this because, there is a discussion going on with me and
> Simon.k about Linux compatible "jedec,spi-nor".  adding support for
> this binding would eventually not reproduce this issue.

That's not completely true. Adding "jedec,spi-nor" to U-Boot would
definitively make it less likely to run into this issue.

However, Linux works correctly without "jedec,spi-nor", so people can
still run into problems when porting a dts from Linux. This is why I
still would like to get this in.


Simon

[U-Boot] [PATCH v2] dm: spi: prevent setting a speed of 0 Hz

[Jagan Teki]

On Fri, Nov 23, 2018 at 6:24 PM Simon Goldschmidt
<sgoldschmidt@de.pepperl-fuchs.com> wrote:
>
> [Responding from my work address due to problems with gmail]
>
> On 23.11.2018 13:27, Jagan Teki wrote:
> > On Fri, Nov 23, 2018 at 1:51 AM <sjg@google.com> wrote:
> >>
> >> On Fri, 16 Nov 2018 at 12:40, Simon Goldschmidt
> >> <simon.k.r.goldschmidt@gmail.com> wrote:
> >>>
> >>> On 31.10.2018 07:42, Simon Goldschmidt wrote:
> >>>> On Wed, Oct 31, 2018 at 7:22 AM Jagan Teki <jagan@amarulasolutions.com> wrote:
> >>>>> On Wed, Oct 31, 2018 at 1:40 AM Simon Goldschmidt
> >>>>> <simon.k.r.goldschmidt@gmail.com> wrote:
> >>>>>> When the device tree is missing a correct spi slave description below
> >>>>>> the bus (compatible "spi-flash" or spi-max-frequency are missing),
> >>>>>> the 'set_speed' callback can be called with 'speed' == 0 Hz.
> >>>>>> At least with cadence qspi, this leads to a division by zero.
> >>>>>>
> >>>>>> Prevent this by initializing speed to 100 kHz in this case (same
> >>>>>> fallback value as is done in 'dm_spi_claim_bus') and issue a warning
> >>>>>> to console.
> >>>>> Why can't driver plat->frequency in cadence driver initialize 100KH?
> >>>>> plat->frequency = fdtdec_get_int(blob, node, "spi-max-frequency", 100000)
> >>>> I'm not sure I understand.
> >>>> The cadence driver initializes its 'plat->max_hz' from
> >>>> "spi-max-frequency" property and defaults to 500 kHz. No problem
> >>>> there.
> >>>>
> >>>> However, the problem I want to solve is if someone puts a flash chip
> >>>> below there which has compatible != "spi-flash", they will get a hard
> >>>> fault which is hard to debug. This is because the node is not parsed
> >>>> because of the wrong compatible string (even if there is an
> >>>> "spi-max-frequency" property) and thus, "sf probe" tries to continue
> >>>> with 0 Hz.
> >>>>
> >>>> And this can happen easily when porting device trees from Linux as
> >>>> there, the boards have compatible "n25q00" etc. instead of
> >>>> "spi-flash", which is U-Boot specific (sadly).
> >>>>
> >>>> This patch is not required to make valid U-Boot devicetrees work, it
> >>>> is meant to get better error handling for devicetrees ported from
> >>>> Linux.
> >>>>
> >>>> An even better fix would be for U-Boot not to require the compatible =
> >>>> "spi-flash" string but just work correctly with Linux device trees,
> >>>> but that's not within my possibilities right now :-(
> >>>
> >>> Ping? This is a bug that should hit nearly everyone porting a board dts
> >>> working in Linux to U-Boot, can we please have some kind of fix for this?
> >>> Currently, using a dts with an spi flash chip that works in Linux may
> >>> abort in U-Boot...
> >>
> >> Reviewed-by: Simon Glass <sjg@chromium.org>
> >>
> >> Applied to u-boot-dm/master, thanks!
> >
> > I hold this because, there is a discussion going on with me and
> > Simon.k about Linux compatible "jedec,spi-nor".  adding support for
> > this binding would eventually not reproduce this issue.
>
> That's not completely true. Adding "jedec,spi-nor" to U-Boot would
> definitively make it less likely to run into this issue.

I don't this is what we discussed above, adding the Linux compatible
with relevant spi-frequnecy make the issue non-reproducible this is
what exactly we ended-up.


>
> However, Linux works correctly without "jedec,spi-nor", so people can
> still run into problems when porting a dts from Linux. This is why I
> still would like to get this in.

Why? do they specify property?

[U-Boot] [PATCH v2] dm: spi: prevent setting a speed of 0 Hz

[Simon Goldschmidt]

On 23.11.2018 14:12, Jagan Teki wrote:
> On Fri, Nov 23, 2018 at 6:24 PM Simon Goldschmidt
> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>
>> [Responding from my work address due to problems with gmail]
>>
>> On 23.11.2018 13:27, Jagan Teki wrote:
>>> On Fri, Nov 23, 2018 at 1:51 AM <sjg@google.com> wrote:
>>>>
>>>> On Fri, 16 Nov 2018 at 12:40, Simon Goldschmidt
>>>> <simon.k.r.goldschmidt@gmail.com> wrote:
>>>>>
>>>>> On 31.10.2018 07:42, Simon Goldschmidt wrote:
>>>>>> On Wed, Oct 31, 2018 at 7:22 AM Jagan Teki <jagan@amarulasolutions.com> wrote:
>>>>>>> On Wed, Oct 31, 2018 at 1:40 AM Simon Goldschmidt
>>>>>>> <simon.k.r.goldschmidt@gmail.com> wrote:
>>>>>>>> When the device tree is missing a correct spi slave description below
>>>>>>>> the bus (compatible "spi-flash" or spi-max-frequency are missing),
>>>>>>>> the 'set_speed' callback can be called with 'speed' == 0 Hz.
>>>>>>>> At least with cadence qspi, this leads to a division by zero.
>>>>>>>>
>>>>>>>> Prevent this by initializing speed to 100 kHz in this case (same
>>>>>>>> fallback value as is done in 'dm_spi_claim_bus') and issue a warning
>>>>>>>> to console.
>>>>>>> Why can't driver plat->frequency in cadence driver initialize 100KH?
>>>>>>> plat->frequency = fdtdec_get_int(blob, node, "spi-max-frequency", 100000)
>>>>>> I'm not sure I understand.
>>>>>> The cadence driver initializes its 'plat->max_hz' from
>>>>>> "spi-max-frequency" property and defaults to 500 kHz. No problem
>>>>>> there.
>>>>>>
>>>>>> However, the problem I want to solve is if someone puts a flash chip
>>>>>> below there which has compatible != "spi-flash", they will get a hard
>>>>>> fault which is hard to debug. This is because the node is not parsed
>>>>>> because of the wrong compatible string (even if there is an
>>>>>> "spi-max-frequency" property) and thus, "sf probe" tries to continue
>>>>>> with 0 Hz.
>>>>>>
>>>>>> And this can happen easily when porting device trees from Linux as
>>>>>> there, the boards have compatible "n25q00" etc. instead of
>>>>>> "spi-flash", which is U-Boot specific (sadly).
>>>>>>
>>>>>> This patch is not required to make valid U-Boot devicetrees work, it
>>>>>> is meant to get better error handling for devicetrees ported from
>>>>>> Linux.
>>>>>>
>>>>>> An even better fix would be for U-Boot not to require the compatible =
>>>>>> "spi-flash" string but just work correctly with Linux device trees,
>>>>>> but that's not within my possibilities right now :-(
>>>>>
>>>>> Ping? This is a bug that should hit nearly everyone porting a board dts
>>>>> working in Linux to U-Boot, can we please have some kind of fix for this?
>>>>> Currently, using a dts with an spi flash chip that works in Linux may
>>>>> abort in U-Boot...
>>>>
>>>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>>>
>>>> Applied to u-boot-dm/master, thanks!
>>>
>>> I hold this because, there is a discussion going on with me and
>>> Simon.k about Linux compatible "jedec,spi-nor".  adding support for
>>> this binding would eventually not reproduce this issue.
>>
>> That's not completely true. Adding "jedec,spi-nor" to U-Boot would
>> definitively make it less likely to run into this issue.
> 
> I don't this is what we discussed above, adding the Linux compatible
> with relevant spi-frequnecy make the issue non-reproducible this is
> what exactly we ended-up.
> 
> 
>>
>> However, Linux works correctly without "jedec,spi-nor", so people can
>> still run into problems when porting a dts from Linux. This is why I
>> still would like to get this in.
> 
> Why? do they specify property?

There are dts files with compatible = "n25q256a" and others. I don't 
find the thread right now, but I said I would try to upstream changes to 
Linux so that the socfgpa_cyclone5 dts files would have an additional 
"jedec,spi-nor" compatible strings.

However, I cannot change all Linux devicetress and having 
"jedec-spi-nor" does *not* seem mandatory for Linux devicetrees.

This patch ensures a devicetree that works with Linux does not lead to a 
"divide-by-zero" abort with U-Boot. I do think that's an improvement for 
U-Boot!

Regards,
Simon