[PATCH V2] spi: Update speed/mode on change

[Tom Rini]

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 at 1000
> > > > > > > => sspi 0:0 at 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 <marex at denx.de>
> > > > > > > Cc: Jagan Teki <jagan at amarulasolutions.com>
> > > > > > > Cc: Patrick Delaunay <patrick.delaunay at 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 at 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 at 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.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210331/8ed34325/attachment.sig>