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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 9 Nov 2022 14:48:18 +0100
From:   Amelie Delaunay <amelie.delaunay@...s.st.com>
To:     Marek Vasut <marex@...x.de>,
        Uwe Kleine-König <u.kleine-koenig@...gutronix.de>,
        Erwan LE RAY <erwan.leray@...s.st.com>,
        Valentin Caron <valentin.caron@...s.st.com>
CC:     <devicetree@...r.kernel.org>,
        Amelie DELAUNAY <amelie.delaunay@...com>,
        Maxime Coquelin <mcoquelin.stm32@...il.com>,
        <linux-kernel@...r.kernel.org>,
        Marcin Sloniewski <marcin.sloniewski@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Jagan Teki <jagan@...rulasolutions.com>,
        "Pengutronix Kernel Team" <kernel@...gutronix.de>,
        Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
        <linux-stm32@...md-mailman.stormreply.com>,
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [Linux-stm32] [PATCH 00/16] STM32 configure UART nodes for DMA

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