On 03/22/2016 07:07 PM, Peter Hurley wrote:
> On 03/22/2016 04:09 AM, Andy Shevchenko wrote:
>> On Tue, Mar 22, 2016 at 12:46 PM, Aleksey Makarov
>> <aleksey.makarov@xxxxxxxxxx> wrote:
>>> + sprintf(opts, "%s,%s,0x%llx,%d", uart, iotype,
>>> + table->serial_port.address, baud_rate);
>>
>> You may use snprintf(), though my question here what would happen on
>> 32-bit kernel when you supply 64-bit address as an option?
>
> Yeah this should probably use %pa for the printf specifier.
>
> But note this exposes underlying bug in the earlycon support, because
> that was originally written without 32/64-mixed bitness in mind; ie.,
> the address is parsed and handled as unsigned long in most places.
I don't quite follow this. table->serial_port.address is explicitly u64,
not pointer, so, according to printk-formats.txt %llx is ok here, %pa is wrong.
Am I missing something?
>>> /*
>>> - * Just 'earlycon' is a valid param for devicetree earlycons;
>>> - * don't generate a warning from parse_early_params() in that case
>>> + * Just 'earlycon' is a valid param for devicetree and ACPI SPCR
>>> + * earlycons; don't generate a warning from parse_early_params()
>>> + * in that case
>>> */
>>> - if (!buf || !buf[0])
>>> - return early_init_dt_scan_chosen_serial();
>>> + if (!buf || !buf[0]) {
>>> + init_spcr_earlycon();
>>
>>> + early_init_dt_scan_chosen_serial();
>>> + return 0;
>>
>> And you hide an error?
>
> Well, this is a little bit tricky because "earlycon" early parameter with
> missing /chosen/stdout-path node is no longer an error, since ACPI may be
> specifying the earlycon instead.
Agree, but note the email by Rob Herring. The code should be like this:
if (!buf || !buf[0]) {
if (acpi_disabled) {
return early_init_dt_scan_chosen_serial();
} else {
init_spcr_earlycon();
return 0;
}
}
But that requires to have made ACPI/DT decision at this point.
Thank you
Aleksey
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
