On 3/31/21 11:45 PM, Tom Rini wrote:
On Wed, Mar 31, 2021 at 10:05:28PM +0200, Marek Vasut wrote:
On 3/31/21 9:40 PM, Tom Rini wrote:
On Wed, Mar 31, 2021 at 03:26:27PM -0400, Tom Rini wrote:
On Wed, Mar 31, 2021 at 09:18:28PM +0200, Marek Vasut wrote:
On 3/29/21 4:40 AM, Marek Vasut wrote:
On 3/29/21 4:09 AM, Marek Vasut wrote:
On 3/29/21 3:59 AM, Tom Rini wrote:
On Mon, Mar 29, 2021 at 03:32:51AM +0200, Marek Vasut wrote:
On 3/28/21 11:30 PM, Tom Rini wrote:
On Fri, Feb 26, 2021 at 03:21:24PM +0100, Marek Vasut wrote:

The spi_get_bus_and_cs() may be called on the same bus and chipselect
with different frequency or mode. This is valid usecase, but the code
fails to notify the controller of such a configuration change. Call
spi_set_speed_mode() in case bus frequency or bus mode changed to let
the controller update the configuration.

The problem can easily be triggered using the sspi command:
=> sspi 0:0@1000
=> sspi 0:0@2000
Without this patch, both transfers happen at 1000
Hz. With this patch,
the later transfer happens correctly at 2000 Hz.

Signed-off-by: Marek Vasut <ma...@denx.de>
Cc: Jagan Teki <ja...@amarulasolutions.com>
Cc: Patrick Delaunay <patrick.delau...@st.com>

So, very reliably I can make:
https://source.denx.de/u-boot/u-boot/-/jobs/245517
happen locally as well building with clang.  It's not
obvious to me why
the test now fails however.

Can you please be more specific / clear ? I have no idea what those 300
lines of cryptic output mean, nor what are you trying to say
by the above,
sorry.

If you build with clang, for sandbox, and run the tests, U-Boot crashes
in the unit tests that you start with "ut dm".

And that is related to this patch somehow ? How ?

... after further discussion off-list to get a better test case
description ...

It seems the problem is in sound_beep() and it is unrelated to this
patch, as the same problem happens with / without this patch being
applied, on:

a7d3ac61482 ("Merge branch '2021-03-28-assorted-bugfixes'")

$ clang --version
Debian clang version 11.0.1-2
...
$ make CC=clang HOSTCC=clang sandbox_defconfig
$ make CC=clang HOSTCC=clang
$ gdb --args ./u-boot -d arch/sandbox/dts/test.dtb
=> ut dm
...
Program received signal SIGSEGV, Segmentation fault.
dlfree (mem=<optimized out>, mem@entry=0x15c19c50) at
common/dlmalloc.c:1623
1623      if (!(inuse_bit_at_offset(next, nextsz)))   /* consolidate
forward */
(gdb) bt
#0  dlfree (mem=<optimized out>, mem@entry=0x15c19c50) at
common/dlmalloc.c:1623
#1  0x00000000004759e3 in sound_beep (dev=0x15c05580, msecs=<optimized
out>, frequency_hz=100)
      at drivers/sound/sound-uclass.c:118

Tom, any further comments ? It seems your claim that this bugfix is causing
problems with clang is not correct, so I think it should still be applied to
v2021.04 , unless you can provide more details about the clang problem ?

There's at least a few funny things going on.  Yes, apparently outside
of CI building sandbox with clang and running the tests manually crashes
in sound, before it gets to the SPI tests.  Except when it crashes on
the SPI tests instead, which I did get to happen once.  So there's some
sort of use-after-free or similar bug around here somewhere, similar to
how we can't use the -fstack-protector patch as it exposes other real
problems that need to be fixed first.

So, I'll see if with a new tag and a few other changes having been made
we once again get to the point where your patch doesn't trigger this
issue.  But since it's also not a regression fix, if no one can figure
out what to do about fixing what clang shows us, then it can wait for
v2021.07.

Nope, still gets things to blow up:
https://source.denx.de/u-boot/u-boot/-/jobs/246848

and yes, it would be nice if when the reason a pytest fails is that
U-Boot crashed, it would say something more clear than "OSError: [Errno
5] Input/output error" and you have to know that means U-Boot died.

Can you please provide a detailed test case how to reproduce this ?
So far I don't have one, the only test description I got is "install random
version of clang, run ut dm and it fails", but that kind of imprecise test
description is really not useful. Please provide more specific instructions.

Yes, follow what CI does.  It's done in a container, so you can have the
exact same runtime and the tests are in .azure-pipelines.yml and
.gitlab-ci.yml or if you look at
https://source.denx.de/u-boot/u-boot/-/jobs/246848/raw you can see the
commands in there.

Can you please provide me the exact commands to run to reproduce this problem ? I just spent a lot of time trying to find this one single command in a wall of text, "ut dm spi_find", which I think is the one failing for you ? It seems to work for me with and without the patch:

$ ./u-boot -d arch/sandbox/dts/test.dtb
sandbox: starting...


U-Boot 2021.04-rc5-00005-g583b6858733 (Apr 01 2021 - 00:34:09 +0200)

Model: sandbox
DRAM:  128 MiB
WDT:   Started with servicing (60s timeout)
MMC:   mmc2: 2 (SD), mmc1: 1 (SD), mmc0: 0 (SD)
In:    serial
Out:   vidconsole
Err:   vidconsole
Model: sandbox
SCSI:
Net: eth0: eth@10002000, eth5: eth@10003000, eth3: sbe5, eth6: eth@10004000
Hit any key to stop autoboot:  0

Without patch
=> ut dm spi_find
Test: dm_test_spi_find: spi.c
test/dm/spi.c:71, dm_test_spi_find(): 0 == spi_get_bus_and_cs(busnum, cs, speed, mode, "jedec_spi_nor", "name", &bus, &slave): Expected 0x0 (0), got 0xfffffffb (-5)
Test: dm_test_spi_find: spi.c (flat tree)
test/dm/spi.c:71, dm_test_spi_find(): 0 == spi_get_bus_and_cs(busnum, cs, speed, mode, "jedec_spi_nor", "name", &bus, &slave): Expected 0x0 (0), got 0xfffffffb (-5)
Failures: 2

With patch
=> ut dm spi_find
Test: dm_test_spi_find: spi.c
test/dm/spi.c:71, dm_test_spi_find(): 0 == spi_get_bus_and_cs(busnum, cs, speed, mode, "jedec_spi_nor", "name", &bus, &slave): Expected 0x0 (0), got 0xfffffffb (-5)
Test: dm_test_spi_find: spi.c (flat tree)
test/dm/spi.c:71, dm_test_spi_find(): 0 == spi_get_bus_and_cs(busnum, cs, speed, mode, "jedec_spi_nor", "name", &bus, &slave): Expected 0x0 (0), got 0xfffffffb (-5)
Failures: 2

Reply via email to