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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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.
> 

Well, I made this patch because I used a wrong bootargs earlycon
configuration and I ended up crashing the kernel. I couldn't see what
happens as kgdb is not available at that stage. Figuring out what was
going on made me spend some time. I hoped I'll be helpful and spare
others of the same mistakes and wasted time.

I'm ok to drop the patch as well, no pushing here. Please ignore.
Thanks for the review!

Cheers,
ta




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux