Re: [Linux-stm32] [PATCH 00/16] STM32 configure UART nodes for DMA

[Valentin CARON]

Hi Uwe,

We found the issue, thank you to have reported it.

stm32-usart driver was not tolerant to a probe defer from DMA when the 
earlycon is active.

You can find the patch here:
https://lore.kernel.org/lkml/20221118170602.1057863-1-valentin.caron@xxxxxxxxxxx/

Valentin

On 11/9/22 14:48, Amelie Delaunay wrote:
> On 11/8/22 16:28, Marek Vasut wrote:
> > On 11/8/22 12:59, Uwe Kleine-König wrote:
> > > On Fri, Feb 04, 2022 at 04:41:55PM +0100, Erwan LE RAY wrote:
> > > > On 2/4/22 2:22 PM, Alexandre TORGUE wrote:
> > > > > Hi Ahmad
> > > > >
> > > > > On 2/3/22 18:25, Ahmad Fatoum wrote:
> > > > > > Hello Erwan,
> > > > > >
> > > > > > On 03.02.22 18:10, Erwan Le Ray wrote:
> > > > > > > Add DMA configuration to UART nodes in stm32mp15x (SOC level) and
> > > > > > > remove it at board level to keep current PIO behavior when needed.
> > > > > > > For stm32-ed1 and stm32-dkx boards, UART4 (console) and UART7
> > > > > > > (no HW flow control pin available) are kept in PIO mode, while 
> > > > > > > USART3
> > > > > > > is now configured in DMA mode.
> > > > > > > UART4 (console UART) has to be kept in irq mode, as DMA support for
> > > > > > > console has been removed from the driver by commit e359b4411c28
> > > > > > > ("serial: stm32: fix threaded interrupt handling").
> > > > > >
> > > > > > Do I understand correctly that your first patch breaks consoles of
> > > > > > most/all boards, because they will briefly use DMA, which is refused
> > > > > > by the stm32-usart driver and then you add a patch for each board
> > > > > > to fix that breakage?
> > > > >
> > > > > We have two solutions and both have pro/drawbacks. The first one 
> > > > > (Erwan
> > > > > ones, can break the boot if the patch is taken "alone". Your 
> > > > > proposition
> > > > > avoids this breakage but deletes a non define property (which is a 
> > > > > bit
> > > > > weird). However I prefer to keep a functional behavior, and keep 
> > > > > Ahmad
> > > > > proposition. Ahmad, just one question, dt-bindings check doesn't
> > > > > complain about it ?
> > > > >
> > > > > Cheers
> > > > > Alex
> > > > >
> > > > > > Such intermittent breakage makes bisection a hassle. 
> > > > > > /delete-property/
> > > > > > is a no-op when the property doesn't exist, so you could move the 
> > > > > > first
> > > > > > patch to the very end to avoid intermittent breakage.
> > > > > >
> > > > > > I also think that the driver's behavior is a bit harsh. I think 
> > > > > > it would
> > > > > > be better for the UART driver to print a warning and fall back to
> > > > > > PIO for console instead of outright refusing and rendering the 
> > > > > > system
> > > > > > silent. That's not mutually exclusive with your patch series here,
> > > > > > of course.
> > > > > >
> > > > > > Cheers,
> > > > > > Ahmad
> > > >
> > > > The driver implementation will consider the request to probe the UART
> > > > console in DMA mode as an error (-ENODEV), and will fallback this 
> > > > UART probe
> > > > in irq mode.
> > >
> > > > Whatever the patch ordering, the boot will never be broken. The 
> > > > board dt
> > > > patches aim to get a "proper" implementation, but from functional
> > > > perspective the driver will manage a request to probe an UART 
> > > > console in DMA
> > > > mode as an error and fall it back in irq mode.
> > >
> > > I didn't debug this further yet, but my machine (with an out-of-tree
> > > dts) fails to boot 6.1-rc4 without removing the dma properties from the
> > > console UART. This is a bug isn't it? The same dts created a working
> > > setup with stm32mp157.dtsi from 5.15 + kernel 5.15.
>
> Hi Uwe,
>
> Could you confirm earlycon is enabled on your setup?
>
> Without earlycon, boot is ok, even with dma properties, at least on 
> stm32mp157c-dk2.
>
> > > I can debug this further, but maybe you know off-hand what the problem
> > > is?
> >
> > +CC Amelie, as this might be related to the DMA series that landed 
> > recently:
> >
> > $ git log --oneline v5.18..v6.0 -- drivers/dma/stm32*
>
> Hi Marek,
>
> We haven't yet investigated the issue, and if latest DMA updates could 
> explain why earlycon breaks the boot.
>
>
> +TO Valentin, as he's now in charge of UART driver.
> Valentin and I will investigate this issue.
>
> Regards,
> Amelie