Date: Thu, 21 Dec 2023 07:41:21 +0000
From: Tudor Ambarus <tudor.ambarus@...aro.org>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
 Arnd Bergmann <arnd@...db.de>, Peter Griffin <peter.griffin@...aro.org>,
 Rob Herring <robh+dt@...nel.org>, krzysztof.kozlowski+dt@...aro.org,
 Michael Turquette <mturquette@...libre.com>, Stephen Boyd
 <sboyd@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
 andi.shyti@...nel.org, Alim Akhtar <alim.akhtar@...sung.com>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Jiri Slaby <jirislaby@...nel.org>, Catalin Marinas
 <catalin.marinas@....com>, Will Deacon <will@...nel.org>,
 Sylwester Nawrocki <s.nawrocki@...sung.com>,
 Tomasz Figa <tomasz.figa@...il.com>, Chanwoo Choi <cw00.choi@...sung.com>,
 Sam Protsenko <semen.protsenko@...aro.org>
Cc: André Draszik <andre.draszik@...aro.org>,
 saravanak@...gle.com, William McVicker <willmcvicker@...gle.com>,
 linux-arm-kernel@...ts.infradead.org, linux-samsung-soc@...r.kernel.org,
 linux-clk@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-i2c@...r.kernel.org,
 linux-serial@...r.kernel.org
Subject: Re: [PATCH 05/13] tty: serial: samsung: add gs101 earlycon support



On 12/15/23 08:01, Krzysztof Kozlowski wrote:
> On 14/12/2023 15:31, Tudor Ambarus wrote:
>>
>>
>> On 12/14/23 14:19, Arnd Bergmann wrote:
>>> On Thu, Dec 14, 2023, at 13:52, Tudor Ambarus wrote:
>>>> On 12/14/23 12:01, Arnd Bergmann wrote:
>>>>> On Thu, Dec 14, 2023, at 11:52, Tudor Ambarus wrote:
>>>>>> +static int __init gs101_early_console_setup(struct earlycon_device *device,
>>>>>
>>>>
>>>> It works if in device tree one specifies the reg-io-width property and
>>>> sets it to 4. If the reg-io-width is not specified, the iotype defaults
>>>> to UPIO_MEM causing the SError interrupt on gs101 which makes the system
>>>> unusable.
>>>
>>> In the case of incorrect DT data like a missing reg-io-width property,
>>> I would expect it to still fail once the regular console or tty takes
>>> over from earlycon.
>>>
>>>> Also, if the earlycon comes specified from the kernel params, the
>>>> of_setup_earlycon() is no longer called and the earlycon will be set
>>>> solely based on the kernel params buffer, thus allowing users to crash
>>>> the kernel on wrong earlycon definitions.
>>>
>>> But that in turn is the same as specifying any other incorrect earlycon.
>>
>> I don't think you can crash the kernel if you use other earlycon as you
>> don't make accesses on the 32bit restricted bus. But I agree that if
>> using the correct earlycon name, and mmio instead mmio32, is equivalent
>> to not specifying reg-io-width in dt.
>>
>>>
>>>> If you think the change is fine, I can amend the commit message with the
>>>> description from above.
>>>
>>> I'm still not convinced we need a special case here when everything else
>>> just requires passing the correct data.
> 
> We shouldn't need any data from DT for this case, because this property
> apparently can be inferred from the compatible. IOW, GS101 SoC requires
> reg-io-width=4, everywhere, for each node, thus there is no need to
> specify this property. It should be deduced from the compatible.
> 

The entire peric0/1 block requires 32 bit data widths indeed. PERIC is
used by the Universal Serial Interface (USI) and I3C. I've checked few
other hardware blocks and all require 32 bit data widths (G3D, TPU, TNR,
PERIC, PDP, MFC, MCSC, IPP, HSI, GSA and I stopped here).

If the reg-io-width shall be inferred from the compatible in the gs101
case, then this patch stands. I'll update the serial driver as well.

Thanks,
ta

Powered by blists - more mailing lists