From: Aleksey Makarov <aleksey.makarov@linaro.org> To: Peter Hurley <peter@hurleysoftware.com>, Andy Shevchenko <andy.shevchenko@gmail.com> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>, "Rafael J . Wysocki" <rjw@rjwysocki.net>, Len Brown <lenb@kernel.org>, "linux-serial@vger.kernel.org" <linux-serial@vger.kernel.org>, "linux-acpi@vger.kernel.org" <linux-acpi@vger.kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, linux-arm Mailing List <linux-arm-kernel@lists.infradead.org>, Russell King <linux@arm.linux.org.uk>, Leif Lindholm <leif.lindholm@linaro.org>, Graeme Gregory <graeme.gregory@linaro.org>, Al Stone <ahs3@redhat.com>, Christopher Covington <cov@codeaurora.org>, Yury Norov <ynorov@caviumnetworks.com>, "Zheng, Lv" <lv.zheng@intel.com>, Jiri Slaby <jslaby@suse.com> Subject: Re: [PATCH v5 3/6] ACPI: parse SPCR and enable matching console Date: Tue, 22 Mar 2016 20:04:11 +0300 [thread overview] Message-ID: <56F17B0B.306@linaro.org> (raw) In-Reply-To: <56F16DC2.7030901@hurleysoftware.com> 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@linaro.org> 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
WARNING: multiple messages have this Message-ID (diff)
From: aleksey.makarov@linaro.org (Aleksey Makarov) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v5 3/6] ACPI: parse SPCR and enable matching console Date: Tue, 22 Mar 2016 20:04:11 +0300 [thread overview] Message-ID: <56F17B0B.306@linaro.org> (raw) In-Reply-To: <56F16DC2.7030901@hurleysoftware.com> 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@linaro.org> 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
next prev parent reply other threads:[~2016-03-22 17:06 UTC|newest] Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-03-22 10:46 [PATCH v5 0/6] ACPI: parse the SPCR table Aleksey Makarov 2016-03-22 10:46 ` Aleksey Makarov [not found] ` <1458643595-14719-1-git-send-email-aleksey.makarov-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> 2016-03-22 10:46 ` [PATCH v5 1/6] of/serial: move earlycon early_param handling to serial Aleksey Makarov 2016-03-22 10:46 ` Aleksey Makarov 2016-03-22 10:46 ` Aleksey Makarov 2016-03-22 11:15 ` Rob Herring 2016-03-22 11:15 ` Rob Herring 2016-03-22 11:15 ` Rob Herring 2016-03-22 16:55 ` Aleksey Makarov 2016-03-22 16:55 ` Aleksey Makarov 2016-03-22 16:55 ` Aleksey Makarov [not found] ` <1458643595-14719-2-git-send-email-aleksey.makarov-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> 2016-03-22 12:28 ` kbuild test robot 2016-03-22 12:28 ` kbuild test robot 2016-03-22 12:28 ` kbuild test robot 2016-03-22 10:46 ` [PATCH v5 2/6] ACPI: add definitions of DBG2 subtypes Aleksey Makarov 2016-03-22 10:46 ` Aleksey Makarov 2016-03-22 10:46 ` Aleksey Makarov 2016-03-22 10:46 ` [PATCH v5 3/6] ACPI: parse SPCR and enable matching console Aleksey Makarov 2016-03-22 10:46 ` Aleksey Makarov 2016-03-22 11:09 ` Andy Shevchenko 2016-03-22 11:09 ` Andy Shevchenko 2016-03-22 16:07 ` Peter Hurley 2016-03-22 16:07 ` Peter Hurley 2016-03-22 17:04 ` Aleksey Makarov [this message] 2016-03-22 17:04 ` Aleksey Makarov 2016-03-22 17:21 ` Peter Hurley 2016-03-22 17:21 ` Peter Hurley 2016-03-22 12:26 ` Yury Norov 2016-03-22 12:26 ` Yury Norov 2016-03-22 12:26 ` Yury Norov 2016-03-22 14:57 ` Peter Hurley 2016-03-22 14:57 ` Peter Hurley 2016-03-22 16:51 ` Yury Norov 2016-03-22 16:51 ` Yury Norov 2016-03-22 16:51 ` Yury Norov 2016-03-22 17:08 ` Aleksey Makarov 2016-03-22 17:08 ` Aleksey Makarov 2016-03-22 17:32 ` Peter Hurley 2016-03-22 17:32 ` Peter Hurley 2016-03-22 10:46 ` [PATCH v5 4/6] ACPI: enable ACPI_SPCR_TABLE on ARM64 Aleksey Makarov 2016-03-22 10:46 ` Aleksey Makarov 2016-03-22 10:46 ` [PATCH v5 5/6] serial: pl011: add console matching function Aleksey Makarov 2016-03-22 10:46 ` Aleksey Makarov 2016-03-22 10:46 ` [PATCH v5 6/6] serial: pl011: add EARLYCON_DECLARE Aleksey Makarov 2016-03-22 10:46 ` Aleksey Makarov 2016-03-22 16:15 ` Peter Hurley 2016-03-22 16:15 ` Peter Hurley 2016-03-22 17:09 ` Aleksey Makarov 2016-03-22 17:09 ` Aleksey Makarov 2016-03-22 17:41 ` Peter Hurley 2016-03-22 17:41 ` Peter Hurley
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=56F17B0B.306@linaro.org \ --to=aleksey.makarov@linaro.org \ --cc=ahs3@redhat.com \ --cc=andy.shevchenko@gmail.com \ --cc=cov@codeaurora.org \ --cc=graeme.gregory@linaro.org \ --cc=gregkh@linuxfoundation.org \ --cc=jslaby@suse.com \ --cc=leif.lindholm@linaro.org \ --cc=lenb@kernel.org \ --cc=linux-acpi@vger.kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-serial@vger.kernel.org \ --cc=linux@arm.linux.org.uk \ --cc=lv.zheng@intel.com \ --cc=peter@hurleysoftware.com \ --cc=rjw@rjwysocki.net \ --cc=ynorov@caviumnetworks.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.