diff mbox series

[v2] Revert "tty: serial: meson: Add a earlycon for the T7 SoC"

Message ID 20230827082944.5100-1-tanure@linux.com (mailing list archive)
State New, archived
Headers show
Series [v2] Revert "tty: serial: meson: Add a earlycon for the T7 SoC" | expand

Commit Message

Lucas Tanure Aug. 27, 2023, 8:29 a.m. UTC
This reverts commit 6a4197f9763325043abf7690a21124a9facbf52e.
New SoC will use ttyS0 instead of ttyAML, so T7 SoC doesn't need a OF_EARLYCON_DECLARE.

Signed-off-by: Lucas Tanure <tanure@linux.com>
---
Since V1:
- add Signed-off-by:

 drivers/tty/serial/meson_uart.c | 2 --
 1 file changed, 2 deletions(-)

Comments

Neil Armstrong Aug. 27, 2023, 9:48 a.m. UTC | #1
Hi Lucas,

Le 27/08/2023 à 10:29, Lucas Tanure a écrit :
> This reverts commit 6a4197f9763325043abf7690a21124a9facbf52e.
> New SoC will use ttyS0 instead of ttyAML, so T7 SoC doesn't need a OF_EARLYCON_DECLARE.
> 
> Signed-off-by: Lucas Tanure <tanure@linux.com>
> ---
> Since V1:
> - add Signed-off-by:
> 
>   drivers/tty/serial/meson_uart.c | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
> index c4f61d82fb727..790d910dafa5d 100644
> --- a/drivers/tty/serial/meson_uart.c
> +++ b/drivers/tty/serial/meson_uart.c
> @@ -648,8 +648,6 @@ meson_serial_early_console_setup(struct earlycon_device *device, const char *opt
>   
>   OF_EARLYCON_DECLARE(meson, "amlogic,meson-ao-uart",
>   		    meson_serial_early_console_setup);
> -OF_EARLYCON_DECLARE(meson, "amlogic,t7-uart",
> -		    meson_serial_early_console_setup);
>   
>   #define MESON_SERIAL_CONSOLE_PTR(_devname) (&meson_serial_console_##_devname)
>   #else

How do you use earlycon with this removed ?

Neil
Lucas Tanure Aug. 27, 2023, 10:04 a.m. UTC | #2
On 27-08-2023 10:48, Neil Armstrong wrote:
> Hi Lucas,
> 
> Le 27/08/2023 à 10:29, Lucas Tanure a écrit :
>> This reverts commit 6a4197f9763325043abf7690a21124a9facbf52e.
>> New SoC will use ttyS0 instead of ttyAML, so T7 SoC doesn't need a 
>> OF_EARLYCON_DECLARE.
>>
>> Signed-off-by: Lucas Tanure <tanure@linux.com>
>> ---
>> Since V1:
>> - add Signed-off-by:
>>
>>   drivers/tty/serial/meson_uart.c | 2 --
>>   1 file changed, 2 deletions(-)
>>
>> diff --git a/drivers/tty/serial/meson_uart.c 
>> b/drivers/tty/serial/meson_uart.c
>> index c4f61d82fb727..790d910dafa5d 100644
>> --- a/drivers/tty/serial/meson_uart.c
>> +++ b/drivers/tty/serial/meson_uart.c
>> @@ -648,8 +648,6 @@ meson_serial_early_console_setup(struct 
>> earlycon_device *device, const char *opt
>>   OF_EARLYCON_DECLARE(meson, "amlogic,meson-ao-uart",
>>               meson_serial_early_console_setup);
>> -OF_EARLYCON_DECLARE(meson, "amlogic,t7-uart",
>> -            meson_serial_early_console_setup);
>>   #define MESON_SERIAL_CONSOLE_PTR(_devname) 
>> (&meson_serial_console_##_devname)
>>   #else
> 
> How do you use earlycon with this removed ?
> 
> Neil
This is my kernel cmdline:
console=ttyS0,921600 no_console_suspend earlycon=ttyS0,0xfe078000

And I can see my log:
boot 64bit kernel
[    0.000000] Booting Linux on physical CPU 0x0000000000 [0x410fd092]
[    0.000000] Linux version 6.5.0-rc7-next-20230825+ (tanureal@ryzen) 
(aarch64-none-linux-gnu-gcc (GNU Toolchain for the A-profile 
Architecture 10.3-2021.07 (arm-10.29)) 10.3.1 20210621, GNU ld (GNU 
Toolchain fo3
[    0.000000] KASLR disabled due to lack of seed
[    0.000000] Machine model: Khadas vim4
[    0.000000] OF: reserved mem: 0x0000000005000000..0x00000000052fffff 
(3072 KiB) nomap non-reusable secmon@5000000
[    0.000000] OF: reserved mem: 0x0000000005300000..0x00000000072fffff 
(32768 KiB) nomap non-reusable secmon@5300000
...
[    0.079368] io scheduler mq-deadline registered
[    0.079374] io scheduler kyber registered
[    0.079549] io scheduler bfq registered
[    0.083373] fe078000.serial: ttyS0 at MMIO 0xfe078000 (irq = 14, 
base_baud = 1500000) is a meson_uart
[    0.083403] printk: console [ttyS0] enabled

As the log is OK, and T7 for now is binding against S4 code we should 
drop this patch and add a S4 one if needed.

But just having this log is not enough for testing earlycon?
I am assuming that by just having the log since 0.0000 is a good sign 
that earlycon is working.
Could you give further guidance?

Lucas Tanure
Neil Armstrong Aug. 27, 2023, 10:23 a.m. UTC | #3
Hi Lucas,

Le 27/08/2023 à 12:04, Lucas Tanure a écrit :
> On 27-08-2023 10:48, Neil Armstrong wrote:
>> Hi Lucas,
>>
>> Le 27/08/2023 à 10:29, Lucas Tanure a écrit :
>>> This reverts commit 6a4197f9763325043abf7690a21124a9facbf52e.
>>> New SoC will use ttyS0 instead of ttyAML, so T7 SoC doesn't need a OF_EARLYCON_DECLARE.
>>>
>>> Signed-off-by: Lucas Tanure <tanure@linux.com>
>>> ---
>>> Since V1:
>>> - add Signed-off-by:
>>>
>>>   drivers/tty/serial/meson_uart.c | 2 --
>>>   1 file changed, 2 deletions(-)
>>>
>>> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
>>> index c4f61d82fb727..790d910dafa5d 100644
>>> --- a/drivers/tty/serial/meson_uart.c
>>> +++ b/drivers/tty/serial/meson_uart.c
>>> @@ -648,8 +648,6 @@ meson_serial_early_console_setup(struct earlycon_device *device, const char *opt
>>>   OF_EARLYCON_DECLARE(meson, "amlogic,meson-ao-uart",
>>>               meson_serial_early_console_setup);
>>> -OF_EARLYCON_DECLARE(meson, "amlogic,t7-uart",
>>> -            meson_serial_early_console_setup);
>>>   #define MESON_SERIAL_CONSOLE_PTR(_devname) (&meson_serial_console_##_devname)
>>>   #else
>>
>> How do you use earlycon with this removed ?
>>
>> Neil
> This is my kernel cmdline:
> console=ttyS0,921600 no_console_suspend earlycon=ttyS0,0xfe078000
> 
> And I can see my log:
> boot 64bit kernel
> [    0.000000] Booting Linux on physical CPU 0x0000000000 [0x410fd092]
> [    0.000000] Linux version 6.5.0-rc7-next-20230825+ (tanureal@ryzen) (aarch64-none-linux-gnu-gcc (GNU Toolchain for the A-profile Architecture 10.3-2021.07 (arm-10.29)) 10.3.1 20210621, GNU ld (GNU Toolchain fo3
> [    0.000000] KASLR disabled due to lack of seed
> [    0.000000] Machine model: Khadas vim4
> [    0.000000] OF: reserved mem: 0x0000000005000000..0x00000000052fffff (3072 KiB) nomap non-reusable secmon@5000000
> [    0.000000] OF: reserved mem: 0x0000000005300000..0x00000000072fffff (32768 KiB) nomap non-reusable secmon@5300000
> ...
> [    0.079368] io scheduler mq-deadline registered
> [    0.079374] io scheduler kyber registered
> [    0.079549] io scheduler bfq registered
> [    0.083373] fe078000.serial: ttyS0 at MMIO 0xfe078000 (irq = 14, base_baud = 1500000) is a meson_uart
> [    0.083403] printk: console [ttyS0] enabled
> 
> As the log is OK, and T7 for now is binding against S4 code we should drop this patch and add a S4 one if needed.
> 
> But just having this log is not enough for testing earlycon?
> I am assuming that by just having the log since 0.0000 is a good sign that earlycon is working.
> Could you give further guidance?

First the kernel argument is simply "earlycon" with no other options,
the earlycon code will fetch the first uart using the DT /chosen/stdout-path property.

Then you should see something like right after "Machine model":
[    0.000000] earlycon: meson0 at MMIO 0x00000000ff803000 (options '115200n8')

The boot log you share doesn't use earlycon, it waits until all depedency of the UART
has been probed before probing the UART driver and prints the whole boot log.

Earlycon is a mechanism to use the UART HW as configured by the bootloader in
a minimal way very early in the boot in order to detect very early lockups.

Keeping OF_EARLYCON_DECLARE for t7 would enable this.

Neil

> 
> Lucas Tanure
>
Lucas Tanure Aug. 27, 2023, 10:31 a.m. UTC | #4
On 27-08-2023 11:23, Neil Armstrong wrote:
> Hi Lucas,
> 
> Le 27/08/2023 à 12:04, Lucas Tanure a écrit :
>> On 27-08-2023 10:48, Neil Armstrong wrote:
>>> Hi Lucas,
>>>
>>> Le 27/08/2023 à 10:29, Lucas Tanure a écrit :
>>>> This reverts commit 6a4197f9763325043abf7690a21124a9facbf52e.
>>>> New SoC will use ttyS0 instead of ttyAML, so T7 SoC doesn't need a 
>>>> OF_EARLYCON_DECLARE.
>>>>
>>>> Signed-off-by: Lucas Tanure <tanure@linux.com>
>>>> ---
>>>> Since V1:
>>>> - add Signed-off-by:
>>>>
>>>>   drivers/tty/serial/meson_uart.c | 2 --
>>>>   1 file changed, 2 deletions(-)
>>>>
>>>> diff --git a/drivers/tty/serial/meson_uart.c 
>>>> b/drivers/tty/serial/meson_uart.c
>>>> index c4f61d82fb727..790d910dafa5d 100644
>>>> --- a/drivers/tty/serial/meson_uart.c
>>>> +++ b/drivers/tty/serial/meson_uart.c
>>>> @@ -648,8 +648,6 @@ meson_serial_early_console_setup(struct 
>>>> earlycon_device *device, const char *opt
>>>>   OF_EARLYCON_DECLARE(meson, "amlogic,meson-ao-uart",
>>>>               meson_serial_early_console_setup);
>>>> -OF_EARLYCON_DECLARE(meson, "amlogic,t7-uart",
>>>> -            meson_serial_early_console_setup);
>>>>   #define MESON_SERIAL_CONSOLE_PTR(_devname) 
>>>> (&meson_serial_console_##_devname)
>>>>   #else
>>>
>>> How do you use earlycon with this removed ?
>>>
>>> Neil
>> This is my kernel cmdline:
>> console=ttyS0,921600 no_console_suspend earlycon=ttyS0,0xfe078000
>>
>> And I can see my log:
>> boot 64bit kernel
>> [    0.000000] Booting Linux on physical CPU 0x0000000000 [0x410fd092]
>> [    0.000000] Linux version 6.5.0-rc7-next-20230825+ (tanureal@ryzen) 
>> (aarch64-none-linux-gnu-gcc (GNU Toolchain for the A-profile 
>> Architecture 10.3-2021.07 (arm-10.29)) 10.3.1 20210621, GNU ld (GNU 
>> Toolchain fo3
>> [    0.000000] KASLR disabled due to lack of seed
>> [    0.000000] Machine model: Khadas vim4
>> [    0.000000] OF: reserved mem: 
>> 0x0000000005000000..0x00000000052fffff (3072 KiB) nomap non-reusable 
>> secmon@5000000
>> [    0.000000] OF: reserved mem: 
>> 0x0000000005300000..0x00000000072fffff (32768 KiB) nomap non-reusable 
>> secmon@5300000
>> ...
>> [    0.079368] io scheduler mq-deadline registered
>> [    0.079374] io scheduler kyber registered
>> [    0.079549] io scheduler bfq registered
>> [    0.083373] fe078000.serial: ttyS0 at MMIO 0xfe078000 (irq = 14, 
>> base_baud = 1500000) is a meson_uart
>> [    0.083403] printk: console [ttyS0] enabled
>>
>> As the log is OK, and T7 for now is binding against S4 code we should 
>> drop this patch and add a S4 one if needed.
>>
>> But just having this log is not enough for testing earlycon?
>> I am assuming that by just having the log since 0.0000 is a good sign 
>> that earlycon is working.
>> Could you give further guidance?
> 
> First the kernel argument is simply "earlycon" with no other options,
> the earlycon code will fetch the first uart using the DT 
> /chosen/stdout-path property.
> 
> Then you should see something like right after "Machine model":
> [    0.000000] earlycon: meson0 at MMIO 0x00000000ff803000 (options 
> '115200n8')
> 
> The boot log you share doesn't use earlycon, it waits until all 
> depedency of the UART
> has been probed before probing the UART driver and prints the whole boot 
> log.
> 
> Earlycon is a mechanism to use the UART HW as configured by the 
> bootloader in
> a minimal way very early in the boot in order to detect very early lockups.
> 
> Keeping OF_EARLYCON_DECLARE for t7 would enable this.
> 
> Neil
> 
>>
>> Lucas Tanure
>>
> 
But then we would need to add Earlycon for S4 too.
How can we add one Earlycon for all future SoCs, like S4 and T7?
Neil Armstrong Aug. 28, 2023, 7:58 a.m. UTC | #5
On 27/08/2023 12:31, Lucas Tanure wrote:
> On 27-08-2023 11:23, Neil Armstrong wrote:
>> Hi Lucas,
>>
>> Le 27/08/2023 à 12:04, Lucas Tanure a écrit :
>>> On 27-08-2023 10:48, Neil Armstrong wrote:
>>>> Hi Lucas,
>>>>
>>>> Le 27/08/2023 à 10:29, Lucas Tanure a écrit :
>>>>> This reverts commit 6a4197f9763325043abf7690a21124a9facbf52e.
>>>>> New SoC will use ttyS0 instead of ttyAML, so T7 SoC doesn't need a OF_EARLYCON_DECLARE.
>>>>>
>>>>> Signed-off-by: Lucas Tanure <tanure@linux.com>
>>>>> ---
>>>>> Since V1:
>>>>> - add Signed-off-by:
>>>>>
>>>>>   drivers/tty/serial/meson_uart.c | 2 --
>>>>>   1 file changed, 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
>>>>> index c4f61d82fb727..790d910dafa5d 100644
>>>>> --- a/drivers/tty/serial/meson_uart.c
>>>>> +++ b/drivers/tty/serial/meson_uart.c
>>>>> @@ -648,8 +648,6 @@ meson_serial_early_console_setup(struct earlycon_device *device, const char *opt
>>>>>   OF_EARLYCON_DECLARE(meson, "amlogic,meson-ao-uart",
>>>>>               meson_serial_early_console_setup);
>>>>> -OF_EARLYCON_DECLARE(meson, "amlogic,t7-uart",
>>>>> -            meson_serial_early_console_setup);
>>>>>   #define MESON_SERIAL_CONSOLE_PTR(_devname) (&meson_serial_console_##_devname)
>>>>>   #else
>>>>
>>>> How do you use earlycon with this removed ?
>>>>
>>>> Neil
>>> This is my kernel cmdline:
>>> console=ttyS0,921600 no_console_suspend earlycon=ttyS0,0xfe078000
>>>
>>> And I can see my log:
>>> boot 64bit kernel
>>> [    0.000000] Booting Linux on physical CPU 0x0000000000 [0x410fd092]
>>> [    0.000000] Linux version 6.5.0-rc7-next-20230825+ (tanureal@ryzen) (aarch64-none-linux-gnu-gcc (GNU Toolchain for the A-profile Architecture 10.3-2021.07 (arm-10.29)) 10.3.1 20210621, GNU ld (GNU Toolchain fo3
>>> [    0.000000] KASLR disabled due to lack of seed
>>> [    0.000000] Machine model: Khadas vim4
>>> [    0.000000] OF: reserved mem: 0x0000000005000000..0x00000000052fffff (3072 KiB) nomap non-reusable secmon@5000000
>>> [    0.000000] OF: reserved mem: 0x0000000005300000..0x00000000072fffff (32768 KiB) nomap non-reusable secmon@5300000
>>> ...
>>> [    0.079368] io scheduler mq-deadline registered
>>> [    0.079374] io scheduler kyber registered
>>> [    0.079549] io scheduler bfq registered
>>> [    0.083373] fe078000.serial: ttyS0 at MMIO 0xfe078000 (irq = 14, base_baud = 1500000) is a meson_uart
>>> [    0.083403] printk: console [ttyS0] enabled
>>>
>>> As the log is OK, and T7 for now is binding against S4 code we should drop this patch and add a S4 one if needed.
>>>
>>> But just having this log is not enough for testing earlycon?
>>> I am assuming that by just having the log since 0.0000 is a good sign that earlycon is working.
>>> Could you give further guidance?
>>
>> First the kernel argument is simply "earlycon" with no other options,
>> the earlycon code will fetch the first uart using the DT /chosen/stdout-path property.
>>
>> Then you should see something like right after "Machine model":
>> [    0.000000] earlycon: meson0 at MMIO 0x00000000ff803000 (options '115200n8')
>>
>> The boot log you share doesn't use earlycon, it waits until all depedency of the UART
>> has been probed before probing the UART driver and prints the whole boot log.
>>
>> Earlycon is a mechanism to use the UART HW as configured by the bootloader in
>> a minimal way very early in the boot in order to detect very early lockups.
>>
>> Keeping OF_EARLYCON_DECLARE for t7 would enable this.
>>
>> Neil
>>
>>>
>>> Lucas Tanure
>>>
>>
> But then we would need to add Earlycon for S4 too.
> How can we add one Earlycon for all future SoCs, like S4 and T7?
> 

As of today, S4 has an UART with amlogic,meson-ao-uart as fallback compatibl,
but if there's no AO UART on S4, then this is wrong and yes a new OF_EARLYCON_DECLARE
with amlogic,meson-s4-uart should be added.

Neil
Dmitry Rokosov Aug. 28, 2023, 8:20 a.m. UTC | #6
On Sun, Aug 27, 2023 at 11:31:35AM +0100, Lucas Tanure wrote:
> On 27-08-2023 11:23, Neil Armstrong wrote:
> > Hi Lucas,
> > 
> > Le 27/08/2023 à 12:04, Lucas Tanure a écrit :
> > > On 27-08-2023 10:48, Neil Armstrong wrote:
> > > > Hi Lucas,
> > > > 
> > > > Le 27/08/2023 à 10:29, Lucas Tanure a écrit :
> > > > > This reverts commit 6a4197f9763325043abf7690a21124a9facbf52e.
> > > > > New SoC will use ttyS0 instead of ttyAML, so T7 SoC doesn't
> > > > > need a OF_EARLYCON_DECLARE.
> > > > > 
> > > > > Signed-off-by: Lucas Tanure <tanure@linux.com>
> > > > > ---
> > > > > Since V1:
> > > > > - add Signed-off-by:
> > > > > 
> > > > >   drivers/tty/serial/meson_uart.c | 2 --
> > > > >   1 file changed, 2 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/tty/serial/meson_uart.c
> > > > > b/drivers/tty/serial/meson_uart.c
> > > > > index c4f61d82fb727..790d910dafa5d 100644
> > > > > --- a/drivers/tty/serial/meson_uart.c
> > > > > +++ b/drivers/tty/serial/meson_uart.c
> > > > > @@ -648,8 +648,6 @@ meson_serial_early_console_setup(struct
> > > > > earlycon_device *device, const char *opt
> > > > >   OF_EARLYCON_DECLARE(meson, "amlogic,meson-ao-uart",
> > > > >               meson_serial_early_console_setup);
> > > > > -OF_EARLYCON_DECLARE(meson, "amlogic,t7-uart",
> > > > > -            meson_serial_early_console_setup);
> > > > >   #define MESON_SERIAL_CONSOLE_PTR(_devname)
> > > > > (&meson_serial_console_##_devname)
> > > > >   #else
> > > > 
> > > > How do you use earlycon with this removed ?
> > > > 
> > > > Neil
> > > This is my kernel cmdline:
> > > console=ttyS0,921600 no_console_suspend earlycon=ttyS0,0xfe078000
> > > 
> > > And I can see my log:
> > > boot 64bit kernel
> > > [    0.000000] Booting Linux on physical CPU 0x0000000000 [0x410fd092]
> > > [    0.000000] Linux version 6.5.0-rc7-next-20230825+
> > > (tanureal@ryzen) (aarch64-none-linux-gnu-gcc (GNU Toolchain for the
> > > A-profile Architecture 10.3-2021.07 (arm-10.29)) 10.3.1 20210621,
> > > GNU ld (GNU Toolchain fo3
> > > [    0.000000] KASLR disabled due to lack of seed
> > > [    0.000000] Machine model: Khadas vim4
> > > [    0.000000] OF: reserved mem:
> > > 0x0000000005000000..0x00000000052fffff (3072 KiB) nomap non-reusable
> > > secmon@5000000
> > > [    0.000000] OF: reserved mem:
> > > 0x0000000005300000..0x00000000072fffff (32768 KiB) nomap
> > > non-reusable secmon@5300000
> > > ...
> > > [    0.079368] io scheduler mq-deadline registered
> > > [    0.079374] io scheduler kyber registered
> > > [    0.079549] io scheduler bfq registered
> > > [    0.083373] fe078000.serial: ttyS0 at MMIO 0xfe078000 (irq = 14,
> > > base_baud = 1500000) is a meson_uart
> > > [    0.083403] printk: console [ttyS0] enabled
> > > 
> > > As the log is OK, and T7 for now is binding against S4 code we
> > > should drop this patch and add a S4 one if needed.
> > > 
> > > But just having this log is not enough for testing earlycon?
> > > I am assuming that by just having the log since 0.0000 is a good
> > > sign that earlycon is working.
> > > Could you give further guidance?
> > 
> > First the kernel argument is simply "earlycon" with no other options,
> > the earlycon code will fetch the first uart using the DT
> > /chosen/stdout-path property.
> > 
> > Then you should see something like right after "Machine model":
> > [    0.000000] earlycon: meson0 at MMIO 0x00000000ff803000 (options
> > '115200n8')
> > 
> > The boot log you share doesn't use earlycon, it waits until all
> > depedency of the UART
> > has been probed before probing the UART driver and prints the whole boot
> > log.
> > 
> > Earlycon is a mechanism to use the UART HW as configured by the
> > bootloader in
> > a minimal way very early in the boot in order to detect very early lockups.
> > 
> > Keeping OF_EARLYCON_DECLARE for t7 would enable this.
> > 
> > Neil
> > 
> > > 
> > > Lucas Tanure
> > > 
> > 
> But then we would need to add Earlycon for S4 too.
> How can we add one Earlycon for all future SoCs, like S4 and T7?
> 

Per my understanding, you just need to declare UART dts node with the
following compatible string: "amlogic,meson-ao-uart". It will enabled
earlycon. There is no neccessary to declare separate
OF_EARLYCON_DECLARE(). It's working okay for A1 dts:

meson-a1.dtsi uart declaration:
	uart_AO_B: serial@2000 {
		compatible = "amlogic,meson-a1-uart",
			     "amlogic,meson-ao-uart";
		reg = <0x0 0x2000 0x0 0x18>;
		interrupts = <GIC_SPI 26 IRQ_TYPE_EDGE_RISING>;
		clocks = <&xtal>, <&xtal>, <&xtal>;
		clock-names = "xtal", "pclk", "baud";
		status = "disabled";
	};

earlycon declaration from dts:
	aliases {
		serial0 = &uart_AO_B;
	};

	chosen {
		stdout-path = "serial0:115200n8";
	};

> 
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic
Neil Armstrong Aug. 28, 2023, 8:23 a.m. UTC | #7
Hi,

On 28/08/2023 10:20, Dmitry Rokosov wrote:
> On Sun, Aug 27, 2023 at 11:31:35AM +0100, Lucas Tanure wrote:
>> On 27-08-2023 11:23, Neil Armstrong wrote:
>>> Hi Lucas,
>>>
>>> Le 27/08/2023 à 12:04, Lucas Tanure a écrit :
>>>> On 27-08-2023 10:48, Neil Armstrong wrote:
>>>>> Hi Lucas,
>>>>>
>>>>> Le 27/08/2023 à 10:29, Lucas Tanure a écrit :
>>>>>> This reverts commit 6a4197f9763325043abf7690a21124a9facbf52e.
>>>>>> New SoC will use ttyS0 instead of ttyAML, so T7 SoC doesn't
>>>>>> need a OF_EARLYCON_DECLARE.
>>>>>>
>>>>>> Signed-off-by: Lucas Tanure <tanure@linux.com>
>>>>>> ---
>>>>>> Since V1:
>>>>>> - add Signed-off-by:
>>>>>>
>>>>>>    drivers/tty/serial/meson_uart.c | 2 --
>>>>>>    1 file changed, 2 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/tty/serial/meson_uart.c
>>>>>> b/drivers/tty/serial/meson_uart.c
>>>>>> index c4f61d82fb727..790d910dafa5d 100644
>>>>>> --- a/drivers/tty/serial/meson_uart.c
>>>>>> +++ b/drivers/tty/serial/meson_uart.c
>>>>>> @@ -648,8 +648,6 @@ meson_serial_early_console_setup(struct
>>>>>> earlycon_device *device, const char *opt
>>>>>>    OF_EARLYCON_DECLARE(meson, "amlogic,meson-ao-uart",
>>>>>>                meson_serial_early_console_setup);
>>>>>> -OF_EARLYCON_DECLARE(meson, "amlogic,t7-uart",
>>>>>> -            meson_serial_early_console_setup);
>>>>>>    #define MESON_SERIAL_CONSOLE_PTR(_devname)
>>>>>> (&meson_serial_console_##_devname)
>>>>>>    #else
>>>>>
>>>>> How do you use earlycon with this removed ?
>>>>>
>>>>> Neil
>>>> This is my kernel cmdline:
>>>> console=ttyS0,921600 no_console_suspend earlycon=ttyS0,0xfe078000
>>>>
>>>> And I can see my log:
>>>> boot 64bit kernel
>>>> [    0.000000] Booting Linux on physical CPU 0x0000000000 [0x410fd092]
>>>> [    0.000000] Linux version 6.5.0-rc7-next-20230825+
>>>> (tanureal@ryzen) (aarch64-none-linux-gnu-gcc (GNU Toolchain for the
>>>> A-profile Architecture 10.3-2021.07 (arm-10.29)) 10.3.1 20210621,
>>>> GNU ld (GNU Toolchain fo3
>>>> [    0.000000] KASLR disabled due to lack of seed
>>>> [    0.000000] Machine model: Khadas vim4
>>>> [    0.000000] OF: reserved mem:
>>>> 0x0000000005000000..0x00000000052fffff (3072 KiB) nomap non-reusable
>>>> secmon@5000000
>>>> [    0.000000] OF: reserved mem:
>>>> 0x0000000005300000..0x00000000072fffff (32768 KiB) nomap
>>>> non-reusable secmon@5300000
>>>> ...
>>>> [    0.079368] io scheduler mq-deadline registered
>>>> [    0.079374] io scheduler kyber registered
>>>> [    0.079549] io scheduler bfq registered
>>>> [    0.083373] fe078000.serial: ttyS0 at MMIO 0xfe078000 (irq = 14,
>>>> base_baud = 1500000) is a meson_uart
>>>> [    0.083403] printk: console [ttyS0] enabled
>>>>
>>>> As the log is OK, and T7 for now is binding against S4 code we
>>>> should drop this patch and add a S4 one if needed.
>>>>
>>>> But just having this log is not enough for testing earlycon?
>>>> I am assuming that by just having the log since 0.0000 is a good
>>>> sign that earlycon is working.
>>>> Could you give further guidance?
>>>
>>> First the kernel argument is simply "earlycon" with no other options,
>>> the earlycon code will fetch the first uart using the DT
>>> /chosen/stdout-path property.
>>>
>>> Then you should see something like right after "Machine model":
>>> [    0.000000] earlycon: meson0 at MMIO 0x00000000ff803000 (options
>>> '115200n8')
>>>
>>> The boot log you share doesn't use earlycon, it waits until all
>>> depedency of the UART
>>> has been probed before probing the UART driver and prints the whole boot
>>> log.
>>>
>>> Earlycon is a mechanism to use the UART HW as configured by the
>>> bootloader in
>>> a minimal way very early in the boot in order to detect very early lockups.
>>>
>>> Keeping OF_EARLYCON_DECLARE for t7 would enable this.
>>>
>>> Neil
>>>
>>>>
>>>> Lucas Tanure
>>>>
>>>
>> But then we would need to add Earlycon for S4 too.
>> How can we add one Earlycon for all future SoCs, like S4 and T7?
>>
> 
> Per my understanding, you just need to declare UART dts node with the
> following compatible string: "amlogic,meson-ao-uart". It will enabled
> earlycon. There is no neccessary to declare separate
> OF_EARLYCON_DECLARE(). It's working okay for A1 dts:

This works on A1 because uart_AO_B is an Always-On UART, and using amlogic,meson-ao-uart
is corrent.
But for S4 & T7, Amlogic dropped the Always-On power domain, so there's no more AO uarts,
so using amlogic,meson-ao-uart on those is abusing the DT to enable a driver functionality.

So the solution is to add those OF_EARLYCON_DECLARE and only use the correct
/chosen/stdout-path property.

Neil


> 
> meson-a1.dtsi uart declaration:
> 	uart_AO_B: serial@2000 {
> 		compatible = "amlogic,meson-a1-uart",
> 			     "amlogic,meson-ao-uart";
> 		reg = <0x0 0x2000 0x0 0x18>;
> 		interrupts = <GIC_SPI 26 IRQ_TYPE_EDGE_RISING>;
> 		clocks = <&xtal>, <&xtal>, <&xtal>;
> 		clock-names = "xtal", "pclk", "baud";
> 		status = "disabled";
> 	};
> 
> earlycon declaration from dts:
> 	aliases {
> 		serial0 = &uart_AO_B;
> 	};
> 
> 	chosen {
> 		stdout-path = "serial0:115200n8";
> 	};
> 
>>
>> _______________________________________________
>> linux-amlogic mailing list
>> linux-amlogic@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-amlogic
>
Dmitry Rokosov Aug. 28, 2023, 8:46 a.m. UTC | #8
On Mon, Aug 28, 2023 at 10:23:45AM +0200, Neil Armstrong wrote:
> Hi,
> 
> On 28/08/2023 10:20, Dmitry Rokosov wrote:
> > On Sun, Aug 27, 2023 at 11:31:35AM +0100, Lucas Tanure wrote:
> > > On 27-08-2023 11:23, Neil Armstrong wrote:
> > > > Hi Lucas,
> > > > 
> > > > Le 27/08/2023 à 12:04, Lucas Tanure a écrit :
> > > > > On 27-08-2023 10:48, Neil Armstrong wrote:
> > > > > > Hi Lucas,
> > > > > > 
> > > > > > Le 27/08/2023 à 10:29, Lucas Tanure a écrit :
> > > > > > > This reverts commit 6a4197f9763325043abf7690a21124a9facbf52e.
> > > > > > > New SoC will use ttyS0 instead of ttyAML, so T7 SoC doesn't
> > > > > > > need a OF_EARLYCON_DECLARE.
> > > > > > > 
> > > > > > > Signed-off-by: Lucas Tanure <tanure@linux.com>
> > > > > > > ---
> > > > > > > Since V1:
> > > > > > > - add Signed-off-by:
> > > > > > > 
> > > > > > >    drivers/tty/serial/meson_uart.c | 2 --
> > > > > > >    1 file changed, 2 deletions(-)
> > > > > > > 
> > > > > > > diff --git a/drivers/tty/serial/meson_uart.c
> > > > > > > b/drivers/tty/serial/meson_uart.c
> > > > > > > index c4f61d82fb727..790d910dafa5d 100644
> > > > > > > --- a/drivers/tty/serial/meson_uart.c
> > > > > > > +++ b/drivers/tty/serial/meson_uart.c
> > > > > > > @@ -648,8 +648,6 @@ meson_serial_early_console_setup(struct
> > > > > > > earlycon_device *device, const char *opt
> > > > > > >    OF_EARLYCON_DECLARE(meson, "amlogic,meson-ao-uart",
> > > > > > >                meson_serial_early_console_setup);
> > > > > > > -OF_EARLYCON_DECLARE(meson, "amlogic,t7-uart",
> > > > > > > -            meson_serial_early_console_setup);
> > > > > > >    #define MESON_SERIAL_CONSOLE_PTR(_devname)
> > > > > > > (&meson_serial_console_##_devname)
> > > > > > >    #else
> > > > > > 
> > > > > > How do you use earlycon with this removed ?
> > > > > > 
> > > > > > Neil
> > > > > This is my kernel cmdline:
> > > > > console=ttyS0,921600 no_console_suspend earlycon=ttyS0,0xfe078000
> > > > > 
> > > > > And I can see my log:
> > > > > boot 64bit kernel
> > > > > [    0.000000] Booting Linux on physical CPU 0x0000000000 [0x410fd092]
> > > > > [    0.000000] Linux version 6.5.0-rc7-next-20230825+
> > > > > (tanureal@ryzen) (aarch64-none-linux-gnu-gcc (GNU Toolchain for the
> > > > > A-profile Architecture 10.3-2021.07 (arm-10.29)) 10.3.1 20210621,
> > > > > GNU ld (GNU Toolchain fo3
> > > > > [    0.000000] KASLR disabled due to lack of seed
> > > > > [    0.000000] Machine model: Khadas vim4
> > > > > [    0.000000] OF: reserved mem:
> > > > > 0x0000000005000000..0x00000000052fffff (3072 KiB) nomap non-reusable
> > > > > secmon@5000000
> > > > > [    0.000000] OF: reserved mem:
> > > > > 0x0000000005300000..0x00000000072fffff (32768 KiB) nomap
> > > > > non-reusable secmon@5300000
> > > > > ...
> > > > > [    0.079368] io scheduler mq-deadline registered
> > > > > [    0.079374] io scheduler kyber registered
> > > > > [    0.079549] io scheduler bfq registered
> > > > > [    0.083373] fe078000.serial: ttyS0 at MMIO 0xfe078000 (irq = 14,
> > > > > base_baud = 1500000) is a meson_uart
> > > > > [    0.083403] printk: console [ttyS0] enabled
> > > > > 
> > > > > As the log is OK, and T7 for now is binding against S4 code we
> > > > > should drop this patch and add a S4 one if needed.
> > > > > 
> > > > > But just having this log is not enough for testing earlycon?
> > > > > I am assuming that by just having the log since 0.0000 is a good
> > > > > sign that earlycon is working.
> > > > > Could you give further guidance?
> > > > 
> > > > First the kernel argument is simply "earlycon" with no other options,
> > > > the earlycon code will fetch the first uart using the DT
> > > > /chosen/stdout-path property.
> > > > 
> > > > Then you should see something like right after "Machine model":
> > > > [    0.000000] earlycon: meson0 at MMIO 0x00000000ff803000 (options
> > > > '115200n8')
> > > > 
> > > > The boot log you share doesn't use earlycon, it waits until all
> > > > depedency of the UART
> > > > has been probed before probing the UART driver and prints the whole boot
> > > > log.
> > > > 
> > > > Earlycon is a mechanism to use the UART HW as configured by the
> > > > bootloader in
> > > > a minimal way very early in the boot in order to detect very early lockups.
> > > > 
> > > > Keeping OF_EARLYCON_DECLARE for t7 would enable this.
> > > > 
> > > > Neil
> > > > 
> > > > > 
> > > > > Lucas Tanure
> > > > > 
> > > > 
> > > But then we would need to add Earlycon for S4 too.
> > > How can we add one Earlycon for all future SoCs, like S4 and T7?
> > > 
> > 
> > Per my understanding, you just need to declare UART dts node with the
> > following compatible string: "amlogic,meson-ao-uart". It will enabled
> > earlycon. There is no neccessary to declare separate
> > OF_EARLYCON_DECLARE(). It's working okay for A1 dts:
> 
> This works on A1 because uart_AO_B is an Always-On UART, and using amlogic,meson-ao-uart
> is corrent.
> But for S4 & T7, Amlogic dropped the Always-On power domain, so there's no more AO uarts,
> so using amlogic,meson-ao-uart on those is abusing the DT to enable a driver functionality.
> 
> So the solution is to add those OF_EARLYCON_DECLARE and only use the correct
> /chosen/stdout-path property.

Oh, I didn't know about missing Always-On UART AO power domain. In this
situation I'm fully agree with separate OF_EARLYCON_DECLARE(). Thank you
for explanation!

> > 
> > meson-a1.dtsi uart declaration:
> > 	uart_AO_B: serial@2000 {
> > 		compatible = "amlogic,meson-a1-uart",
> > 			     "amlogic,meson-ao-uart";
> > 		reg = <0x0 0x2000 0x0 0x18>;
> > 		interrupts = <GIC_SPI 26 IRQ_TYPE_EDGE_RISING>;
> > 		clocks = <&xtal>, <&xtal>, <&xtal>;
> > 		clock-names = "xtal", "pclk", "baud";
> > 		status = "disabled";
> > 	};
> > 
> > earlycon declaration from dts:
> > 	aliases {
> > 		serial0 = &uart_AO_B;
> > 	};
> > 
> > 	chosen {
> > 		stdout-path = "serial0:115200n8";
> > 	};
> > 
> > > 
> > > _______________________________________________
> > > linux-amlogic mailing list
> > > linux-amlogic@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-amlogic
> > 
>
Lucas Tanure Aug. 28, 2023, 8:47 a.m. UTC | #9
On Mon, Aug 28, 2023 at 9:46 AM Dmitry Rokosov <ddrokosov@sberdevices.ru> wrote:
>
> On Mon, Aug 28, 2023 at 10:23:45AM +0200, Neil Armstrong wrote:
> > Hi,
> >
> > On 28/08/2023 10:20, Dmitry Rokosov wrote:
> > > On Sun, Aug 27, 2023 at 11:31:35AM +0100, Lucas Tanure wrote:
> > > > On 27-08-2023 11:23, Neil Armstrong wrote:
> > > > > Hi Lucas,
> > > > >
> > > > > Le 27/08/2023 à 12:04, Lucas Tanure a écrit :
> > > > > > On 27-08-2023 10:48, Neil Armstrong wrote:
> > > > > > > Hi Lucas,
> > > > > > >
> > > > > > > Le 27/08/2023 à 10:29, Lucas Tanure a écrit :
> > > > > > > > This reverts commit 6a4197f9763325043abf7690a21124a9facbf52e.
> > > > > > > > New SoC will use ttyS0 instead of ttyAML, so T7 SoC doesn't
> > > > > > > > need a OF_EARLYCON_DECLARE.
> > > > > > > >
> > > > > > > > Signed-off-by: Lucas Tanure <tanure@linux.com>
> > > > > > > > ---
> > > > > > > > Since V1:
> > > > > > > > - add Signed-off-by:
> > > > > > > >
> > > > > > > >    drivers/tty/serial/meson_uart.c | 2 --
> > > > > > > >    1 file changed, 2 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/drivers/tty/serial/meson_uart.c
> > > > > > > > b/drivers/tty/serial/meson_uart.c
> > > > > > > > index c4f61d82fb727..790d910dafa5d 100644
> > > > > > > > --- a/drivers/tty/serial/meson_uart.c
> > > > > > > > +++ b/drivers/tty/serial/meson_uart.c
> > > > > > > > @@ -648,8 +648,6 @@ meson_serial_early_console_setup(struct
> > > > > > > > earlycon_device *device, const char *opt
> > > > > > > >    OF_EARLYCON_DECLARE(meson, "amlogic,meson-ao-uart",
> > > > > > > >                meson_serial_early_console_setup);
> > > > > > > > -OF_EARLYCON_DECLARE(meson, "amlogic,t7-uart",
> > > > > > > > -            meson_serial_early_console_setup);
> > > > > > > >    #define MESON_SERIAL_CONSOLE_PTR(_devname)
> > > > > > > > (&meson_serial_console_##_devname)
> > > > > > > >    #else
> > > > > > >
> > > > > > > How do you use earlycon with this removed ?
> > > > > > >
> > > > > > > Neil
> > > > > > This is my kernel cmdline:
> > > > > > console=ttyS0,921600 no_console_suspend earlycon=ttyS0,0xfe078000
> > > > > >
> > > > > > And I can see my log:
> > > > > > boot 64bit kernel
> > > > > > [    0.000000] Booting Linux on physical CPU 0x0000000000 [0x410fd092]
> > > > > > [    0.000000] Linux version 6.5.0-rc7-next-20230825+
> > > > > > (tanureal@ryzen) (aarch64-none-linux-gnu-gcc (GNU Toolchain for the
> > > > > > A-profile Architecture 10.3-2021.07 (arm-10.29)) 10.3.1 20210621,
> > > > > > GNU ld (GNU Toolchain fo3
> > > > > > [    0.000000] KASLR disabled due to lack of seed
> > > > > > [    0.000000] Machine model: Khadas vim4
> > > > > > [    0.000000] OF: reserved mem:
> > > > > > 0x0000000005000000..0x00000000052fffff (3072 KiB) nomap non-reusable
> > > > > > secmon@5000000
> > > > > > [    0.000000] OF: reserved mem:
> > > > > > 0x0000000005300000..0x00000000072fffff (32768 KiB) nomap
> > > > > > non-reusable secmon@5300000
> > > > > > ...
> > > > > > [    0.079368] io scheduler mq-deadline registered
> > > > > > [    0.079374] io scheduler kyber registered
> > > > > > [    0.079549] io scheduler bfq registered
> > > > > > [    0.083373] fe078000.serial: ttyS0 at MMIO 0xfe078000 (irq = 14,
> > > > > > base_baud = 1500000) is a meson_uart
> > > > > > [    0.083403] printk: console [ttyS0] enabled
> > > > > >
> > > > > > As the log is OK, and T7 for now is binding against S4 code we
> > > > > > should drop this patch and add a S4 one if needed.
> > > > > >
> > > > > > But just having this log is not enough for testing earlycon?
> > > > > > I am assuming that by just having the log since 0.0000 is a good
> > > > > > sign that earlycon is working.
> > > > > > Could you give further guidance?
> > > > >
> > > > > First the kernel argument is simply "earlycon" with no other options,
> > > > > the earlycon code will fetch the first uart using the DT
> > > > > /chosen/stdout-path property.
> > > > >
> > > > > Then you should see something like right after "Machine model":
> > > > > [    0.000000] earlycon: meson0 at MMIO 0x00000000ff803000 (options
> > > > > '115200n8')
> > > > >
> > > > > The boot log you share doesn't use earlycon, it waits until all
> > > > > depedency of the UART
> > > > > has been probed before probing the UART driver and prints the whole boot
> > > > > log.
> > > > >
> > > > > Earlycon is a mechanism to use the UART HW as configured by the
> > > > > bootloader in
> > > > > a minimal way very early in the boot in order to detect very early lockups.
> > > > >
> > > > > Keeping OF_EARLYCON_DECLARE for t7 would enable this.
> > > > >
> > > > > Neil
> > > > >
> > > > > >
> > > > > > Lucas Tanure
> > > > > >
> > > > >
> > > > But then we would need to add Earlycon for S4 too.
> > > > How can we add one Earlycon for all future SoCs, like S4 and T7?
> > > >
> > >
> > > Per my understanding, you just need to declare UART dts node with the
> > > following compatible string: "amlogic,meson-ao-uart". It will enabled
> > > earlycon. There is no neccessary to declare separate
> > > OF_EARLYCON_DECLARE(). It's working okay for A1 dts:
> >
> > This works on A1 because uart_AO_B is an Always-On UART, and using amlogic,meson-ao-uart
> > is corrent.
> > But for S4 & T7, Amlogic dropped the Always-On power domain, so there's no more AO uarts,
> > so using amlogic,meson-ao-uart on those is abusing the DT to enable a driver functionality.
> >
> > So the solution is to add those OF_EARLYCON_DECLARE and only use the correct
> > /chosen/stdout-path property.
>
> Oh, I didn't know about missing Always-On UART AO power domain. In this
> situation I'm fully agree with separate OF_EARLYCON_DECLARE(). Thank you
> for explanation!
>
> > >
> > > meson-a1.dtsi uart declaration:
> > >     uart_AO_B: serial@2000 {
> > >             compatible = "amlogic,meson-a1-uart",
> > >                          "amlogic,meson-ao-uart";
> > >             reg = <0x0 0x2000 0x0 0x18>;
> > >             interrupts = <GIC_SPI 26 IRQ_TYPE_EDGE_RISING>;
> > >             clocks = <&xtal>, <&xtal>, <&xtal>;
> > >             clock-names = "xtal", "pclk", "baud";
> > >             status = "disabled";
> > >     };
> > >
> > > earlycon declaration from dts:
> > >     aliases {
> > >             serial0 = &uart_AO_B;
> > >     };
> > >
> > >     chosen {
> > >             stdout-path = "serial0:115200n8";
> > >     };
> > >
> > > >
> > > > _______________________________________________
> > > > linux-amlogic mailing list
> > > > linux-amlogic@lists.infradead.org
> > > > http://lists.infradead.org/mailman/listinfo/linux-amlogic
> > >
> >
>
> --
> Thank you,
> Dmitry
So let's add:

OF_EARLYCON_DECLARE(meson, "amlogic,meson-s4-uart",
meson_serial_early_console_setup);

ok?
Neil Armstrong Aug. 28, 2023, 8:50 a.m. UTC | #10
On 28/08/2023 10:47, Lucas Tanure wrote:
> On Mon, Aug 28, 2023 at 9:46 AM Dmitry Rokosov <ddrokosov@sberdevices.ru> wrote:
>>
>> On Mon, Aug 28, 2023 at 10:23:45AM +0200, Neil Armstrong wrote:
>>> Hi,
>>>
>>> On 28/08/2023 10:20, Dmitry Rokosov wrote:
>>>> On Sun, Aug 27, 2023 at 11:31:35AM +0100, Lucas Tanure wrote:
>>>>> On 27-08-2023 11:23, Neil Armstrong wrote:
>>>>>> Hi Lucas,
>>>>>>
>>>>>> Le 27/08/2023 à 12:04, Lucas Tanure a écrit :
>>>>>>> On 27-08-2023 10:48, Neil Armstrong wrote:
>>>>>>>> Hi Lucas,
>>>>>>>>
>>>>>>>> Le 27/08/2023 à 10:29, Lucas Tanure a écrit :
>>>>>>>>> This reverts commit 6a4197f9763325043abf7690a21124a9facbf52e.
>>>>>>>>> New SoC will use ttyS0 instead of ttyAML, so T7 SoC doesn't
>>>>>>>>> need a OF_EARLYCON_DECLARE.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Lucas Tanure <tanure@linux.com>
>>>>>>>>> ---
>>>>>>>>> Since V1:
>>>>>>>>> - add Signed-off-by:
>>>>>>>>>
>>>>>>>>>     drivers/tty/serial/meson_uart.c | 2 --
>>>>>>>>>     1 file changed, 2 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/tty/serial/meson_uart.c
>>>>>>>>> b/drivers/tty/serial/meson_uart.c
>>>>>>>>> index c4f61d82fb727..790d910dafa5d 100644
>>>>>>>>> --- a/drivers/tty/serial/meson_uart.c
>>>>>>>>> +++ b/drivers/tty/serial/meson_uart.c
>>>>>>>>> @@ -648,8 +648,6 @@ meson_serial_early_console_setup(struct
>>>>>>>>> earlycon_device *device, const char *opt
>>>>>>>>>     OF_EARLYCON_DECLARE(meson, "amlogic,meson-ao-uart",
>>>>>>>>>                 meson_serial_early_console_setup);
>>>>>>>>> -OF_EARLYCON_DECLARE(meson, "amlogic,t7-uart",
>>>>>>>>> -            meson_serial_early_console_setup);
>>>>>>>>>     #define MESON_SERIAL_CONSOLE_PTR(_devname)
>>>>>>>>> (&meson_serial_console_##_devname)
>>>>>>>>>     #else
>>>>>>>>
>>>>>>>> How do you use earlycon with this removed ?
>>>>>>>>
>>>>>>>> Neil
>>>>>>> This is my kernel cmdline:
>>>>>>> console=ttyS0,921600 no_console_suspend earlycon=ttyS0,0xfe078000
>>>>>>>
>>>>>>> And I can see my log:
>>>>>>> boot 64bit kernel
>>>>>>> [    0.000000] Booting Linux on physical CPU 0x0000000000 [0x410fd092]
>>>>>>> [    0.000000] Linux version 6.5.0-rc7-next-20230825+
>>>>>>> (tanureal@ryzen) (aarch64-none-linux-gnu-gcc (GNU Toolchain for the
>>>>>>> A-profile Architecture 10.3-2021.07 (arm-10.29)) 10.3.1 20210621,
>>>>>>> GNU ld (GNU Toolchain fo3
>>>>>>> [    0.000000] KASLR disabled due to lack of seed
>>>>>>> [    0.000000] Machine model: Khadas vim4
>>>>>>> [    0.000000] OF: reserved mem:
>>>>>>> 0x0000000005000000..0x00000000052fffff (3072 KiB) nomap non-reusable
>>>>>>> secmon@5000000
>>>>>>> [    0.000000] OF: reserved mem:
>>>>>>> 0x0000000005300000..0x00000000072fffff (32768 KiB) nomap
>>>>>>> non-reusable secmon@5300000
>>>>>>> ...
>>>>>>> [    0.079368] io scheduler mq-deadline registered
>>>>>>> [    0.079374] io scheduler kyber registered
>>>>>>> [    0.079549] io scheduler bfq registered
>>>>>>> [    0.083373] fe078000.serial: ttyS0 at MMIO 0xfe078000 (irq = 14,
>>>>>>> base_baud = 1500000) is a meson_uart
>>>>>>> [    0.083403] printk: console [ttyS0] enabled
>>>>>>>
>>>>>>> As the log is OK, and T7 for now is binding against S4 code we
>>>>>>> should drop this patch and add a S4 one if needed.
>>>>>>>
>>>>>>> But just having this log is not enough for testing earlycon?
>>>>>>> I am assuming that by just having the log since 0.0000 is a good
>>>>>>> sign that earlycon is working.
>>>>>>> Could you give further guidance?
>>>>>>
>>>>>> First the kernel argument is simply "earlycon" with no other options,
>>>>>> the earlycon code will fetch the first uart using the DT
>>>>>> /chosen/stdout-path property.
>>>>>>
>>>>>> Then you should see something like right after "Machine model":
>>>>>> [    0.000000] earlycon: meson0 at MMIO 0x00000000ff803000 (options
>>>>>> '115200n8')
>>>>>>
>>>>>> The boot log you share doesn't use earlycon, it waits until all
>>>>>> depedency of the UART
>>>>>> has been probed before probing the UART driver and prints the whole boot
>>>>>> log.
>>>>>>
>>>>>> Earlycon is a mechanism to use the UART HW as configured by the
>>>>>> bootloader in
>>>>>> a minimal way very early in the boot in order to detect very early lockups.
>>>>>>
>>>>>> Keeping OF_EARLYCON_DECLARE for t7 would enable this.
>>>>>>
>>>>>> Neil
>>>>>>
>>>>>>>
>>>>>>> Lucas Tanure
>>>>>>>
>>>>>>
>>>>> But then we would need to add Earlycon for S4 too.
>>>>> How can we add one Earlycon for all future SoCs, like S4 and T7?
>>>>>
>>>>
>>>> Per my understanding, you just need to declare UART dts node with the
>>>> following compatible string: "amlogic,meson-ao-uart". It will enabled
>>>> earlycon. There is no neccessary to declare separate
>>>> OF_EARLYCON_DECLARE(). It's working okay for A1 dts:
>>>
>>> This works on A1 because uart_AO_B is an Always-On UART, and using amlogic,meson-ao-uart
>>> is corrent.
>>> But for S4 & T7, Amlogic dropped the Always-On power domain, so there's no more AO uarts,
>>> so using amlogic,meson-ao-uart on those is abusing the DT to enable a driver functionality.
>>>
>>> So the solution is to add those OF_EARLYCON_DECLARE and only use the correct
>>> /chosen/stdout-path property.
>>
>> Oh, I didn't know about missing Always-On UART AO power domain. In this
>> situation I'm fully agree with separate OF_EARLYCON_DECLARE(). Thank you
>> for explanation!
>>
>>>>
>>>> meson-a1.dtsi uart declaration:
>>>>      uart_AO_B: serial@2000 {
>>>>              compatible = "amlogic,meson-a1-uart",
>>>>                           "amlogic,meson-ao-uart";
>>>>              reg = <0x0 0x2000 0x0 0x18>;
>>>>              interrupts = <GIC_SPI 26 IRQ_TYPE_EDGE_RISING>;
>>>>              clocks = <&xtal>, <&xtal>, <&xtal>;
>>>>              clock-names = "xtal", "pclk", "baud";
>>>>              status = "disabled";
>>>>      };
>>>>
>>>> earlycon declaration from dts:
>>>>      aliases {
>>>>              serial0 = &uart_AO_B;
>>>>      };
>>>>
>>>>      chosen {
>>>>              stdout-path = "serial0:115200n8";
>>>>      };
>>>>
>>>>>
>>>>> _______________________________________________
>>>>> linux-amlogic mailing list
>>>>> linux-amlogic@lists.infradead.org
>>>>> http://lists.infradead.org/mailman/listinfo/linux-amlogic
>>>>
>>>
>>
>> --
>> Thank you,
>> Dmitry
> So let's add:
> 
> OF_EARLYCON_DECLARE(meson, "amlogic,meson-s4-uart",
> meson_serial_early_console_setup);
> 
> ok?


Yes

Neil
Neil Armstrong Aug. 28, 2023, 12:31 p.m. UTC | #11
Hi Greg,

On 27/08/2023 10:29, Lucas Tanure wrote:
> This reverts commit 6a4197f9763325043abf7690a21124a9facbf52e.
> New SoC will use ttyS0 instead of ttyAML, so T7 SoC doesn't need a OF_EARLYCON_DECLARE.
> 
> Signed-off-by: Lucas Tanure <tanure@linux.com>
> ---
> Since V1:
> - add Signed-off-by:
> 
>   drivers/tty/serial/meson_uart.c | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
> index c4f61d82fb727..790d910dafa5d 100644
> --- a/drivers/tty/serial/meson_uart.c
> +++ b/drivers/tty/serial/meson_uart.c
> @@ -648,8 +648,6 @@ meson_serial_early_console_setup(struct earlycon_device *device, const char *opt
>   
>   OF_EARLYCON_DECLARE(meson, "amlogic,meson-ao-uart",
>   		    meson_serial_early_console_setup);
> -OF_EARLYCON_DECLARE(meson, "amlogic,t7-uart",
> -		    meson_serial_early_console_setup);
>   
>   #define MESON_SERIAL_CONSOLE_PTR(_devname) (&meson_serial_console_##_devname)
>   #else

After some clarifications, it's ok to merge the revert:

Acked-by: Neil Armstrong <neil.armstrong@linaro.org>
Greg Kroah-Hartman Aug. 28, 2023, 2:47 p.m. UTC | #12
On Mon, Aug 28, 2023 at 02:31:06PM +0200, Neil Armstrong wrote:
> Hi Greg,
> 
> On 27/08/2023 10:29, Lucas Tanure wrote:
> > This reverts commit 6a4197f9763325043abf7690a21124a9facbf52e.
> > New SoC will use ttyS0 instead of ttyAML, so T7 SoC doesn't need a OF_EARLYCON_DECLARE.
> > 
> > Signed-off-by: Lucas Tanure <tanure@linux.com>
> > ---
> > Since V1:
> > - add Signed-off-by:
> > 
> >   drivers/tty/serial/meson_uart.c | 2 --
> >   1 file changed, 2 deletions(-)
> > 
> > diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
> > index c4f61d82fb727..790d910dafa5d 100644
> > --- a/drivers/tty/serial/meson_uart.c
> > +++ b/drivers/tty/serial/meson_uart.c
> > @@ -648,8 +648,6 @@ meson_serial_early_console_setup(struct earlycon_device *device, const char *opt
> >   OF_EARLYCON_DECLARE(meson, "amlogic,meson-ao-uart",
> >   		    meson_serial_early_console_setup);
> > -OF_EARLYCON_DECLARE(meson, "amlogic,t7-uart",
> > -		    meson_serial_early_console_setup);
> >   #define MESON_SERIAL_CONSOLE_PTR(_devname) (&meson_serial_console_##_devname)
> >   #else
> 
> After some clarifications, it's ok to merge the revert:
> 
> Acked-by: Neil Armstrong <neil.armstrong@linaro.org>

Already merged :)

thanks,

greg k-h
diff mbox series

Patch

diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index c4f61d82fb727..790d910dafa5d 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -648,8 +648,6 @@  meson_serial_early_console_setup(struct earlycon_device *device, const char *opt
 
 OF_EARLYCON_DECLARE(meson, "amlogic,meson-ao-uart",
 		    meson_serial_early_console_setup);
-OF_EARLYCON_DECLARE(meson, "amlogic,t7-uart",
-		    meson_serial_early_console_setup);
 
 #define MESON_SERIAL_CONSOLE_PTR(_devname) (&meson_serial_console_##_devname)
 #else