Re: [PATCH 05/13] tty: serial: samsung: add gs101 earlycon support

[Krzysztof Kozlowski]

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.

Best regards,
Krzysztof