All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yu Tu <yu.tu@amlogic.com>
To: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Cc: <linux-serial@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-amlogic@lists.infradead.org>,
	<linux-kernel@vger.kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jiri Slaby <jirislaby@kernel.org>,
	Neil Armstrong <narmstrong@baylibre.com>,
	Kevin Hilman <khilman@baylibre.com>,
	Jerome Brunet <jbrunet@baylibre.com>
Subject: Re: [PATCH 3/3] tty: serial: meson: add UART driver compatible with S4 SoC on-chip
Date: Tue, 28 Dec 2021 19:24:11 +0800	[thread overview]
Message-ID: <36d63dde-6bfc-925b-51ae-ee801dcfa681@amlogic.com> (raw)
In-Reply-To: <CAFBinCAr=U0BM6O329td6=Nw4oknyKQUgTGtwhq4RdCLdR_H+A@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1930 bytes --]

Hi Martin and Jerome,
	Thank you very much for your reply. I have learned a lot from your 
communication.

On 2021/12/28 4:04, Martin Blumenstingl wrote:
> [ EXTERNAL EMAIL ]
> 
> Hello,
> 
> On Mon, Dec 27, 2021 at 7:56 AM Yu Tu <yu.tu@amlogic.com> wrote:
> [...]
>>> Does the new S4 SoC use an external 12MHz XTAL or does it use a 24MHz XTAL?
>>> If there's still a 24MHz XTAL then I think this description is not
>>> correct - at least based on how I understand the UART controller.
>>>
>> The S4 SoC uses 12MHz(UART_EE_A_REG5[27]=0x1,the bit is set in romcode).
>> This register description is the same as the G12A and G12B you know.
> Thank you for this explanation!
> So the problem is that we're not touching bit 26 and bit 27 - and with
> the updated romcode you would not get any serial output since the
> divider is calculated off the wrong clock.
> 
> I agree with Jerome that we shouldn't put a flag in device-tree.
> 
> Also I did some experimenting with Jerome's idea to implement the
> clocks using CCF (common clock framework), see the attached patches.
> It was a bit tricky because some initial clean-ups were needed in the
> serial driver.
> Note: I have only briefly tested this on a 32-bit Meson8m2 SoC, see my
> attached patches and the clk_summary debugfs output.
> In fact, I expect that there are some issues with at least one of the
> patches as the whole bit 26 and bit 27 code is untested.
> 
> Do you see any problems with this patch?
> Could you try to implement CCF support with the idea from the attached
> patches (you don't need to re-use them, I just wrote them to make it
> clearer in our discussion what we're talking about).
> 
I couldn't agree with you more. I have verified it on a 64-bit S4 
platform. Please refer to the attachment for verification output 
information.
I will prepare the second version of patch according to the example 
ideas you provided.
> 
> Best regards,
> Martin

[-- Attachment #2: s4-clk-debug-output.txt --]
[-- Type: text/plain, Size: 1440 bytes --]

# cat /sys/kernel/debug/clk/clk_summary 
                                 enable  prepare  protect                                duty  hardware
   clock                          count    count    count        rate   accuracy phase  cycle    enable
-------------------------------------------------------------------------------------------------------
 xtal                                 7        7        0    24000000          0     0  50000         Y
    fe07a000.serial#xtal_div2         1        1        0    12000000          0     0  50000         Y
       fe07a000.serial#xtal2_clk_sel       1        1        0    12000000          0     0  50000         Y
          fe07a000.serial#use_xtal       1        1        0    12000000          0     0  50000         Y
             fe07a000.serial#baud_div       1        1        0      923077          0     0  50000         Y
    fe07a000.serial#xtal_div3         0        0        0     8000000          0     0  50000         Y
       fe07a000.serial#xtal_clk_sel       0        0        0     8000000          0     0  50000         Y

# 
# stty -F /dev/ttyAML0  115200

# stty -F /dev/ttyAML0 
speed 115200 baud; line = 0;
intr = ^C; quit = ^\; erase = ^?; kill = ^U; eof = ^D; eol = <undef>;
eol2 = <undef>; swtch = <undef>; start = ^Q; stop = ^S; susp = ^Z; rprnt = ^R;
werase = ^W; lnext = ^V; flush = ^O; min = 1; time = 0;
-brkint ixoff -imaxbel

WARNING: multiple messages have this Message-ID (diff)
From: Yu Tu <yu.tu@amlogic.com>
To: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Cc: <linux-serial@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-amlogic@lists.infradead.org>,
	<linux-kernel@vger.kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jiri Slaby <jirislaby@kernel.org>,
	Neil Armstrong <narmstrong@baylibre.com>,
	Kevin Hilman <khilman@baylibre.com>,
	Jerome Brunet <jbrunet@baylibre.com>
Subject: Re: [PATCH 3/3] tty: serial: meson: add UART driver compatible with S4 SoC on-chip
Date: Tue, 28 Dec 2021 19:24:11 +0800	[thread overview]
Message-ID: <36d63dde-6bfc-925b-51ae-ee801dcfa681@amlogic.com> (raw)
In-Reply-To: <CAFBinCAr=U0BM6O329td6=Nw4oknyKQUgTGtwhq4RdCLdR_H+A@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1930 bytes --]

Hi Martin and Jerome,
	Thank you very much for your reply. I have learned a lot from your 
communication.

On 2021/12/28 4:04, Martin Blumenstingl wrote:
> [ EXTERNAL EMAIL ]
> 
> Hello,
> 
> On Mon, Dec 27, 2021 at 7:56 AM Yu Tu <yu.tu@amlogic.com> wrote:
> [...]
>>> Does the new S4 SoC use an external 12MHz XTAL or does it use a 24MHz XTAL?
>>> If there's still a 24MHz XTAL then I think this description is not
>>> correct - at least based on how I understand the UART controller.
>>>
>> The S4 SoC uses 12MHz(UART_EE_A_REG5[27]=0x1,the bit is set in romcode).
>> This register description is the same as the G12A and G12B you know.
> Thank you for this explanation!
> So the problem is that we're not touching bit 26 and bit 27 - and with
> the updated romcode you would not get any serial output since the
> divider is calculated off the wrong clock.
> 
> I agree with Jerome that we shouldn't put a flag in device-tree.
> 
> Also I did some experimenting with Jerome's idea to implement the
> clocks using CCF (common clock framework), see the attached patches.
> It was a bit tricky because some initial clean-ups were needed in the
> serial driver.
> Note: I have only briefly tested this on a 32-bit Meson8m2 SoC, see my
> attached patches and the clk_summary debugfs output.
> In fact, I expect that there are some issues with at least one of the
> patches as the whole bit 26 and bit 27 code is untested.
> 
> Do you see any problems with this patch?
> Could you try to implement CCF support with the idea from the attached
> patches (you don't need to re-use them, I just wrote them to make it
> clearer in our discussion what we're talking about).
> 
I couldn't agree with you more. I have verified it on a 64-bit S4 
platform. Please refer to the attachment for verification output 
information.
I will prepare the second version of patch according to the example 
ideas you provided.
> 
> Best regards,
> Martin

[-- Attachment #2: s4-clk-debug-output.txt --]
[-- Type: text/plain, Size: 1440 bytes --]

# cat /sys/kernel/debug/clk/clk_summary 
                                 enable  prepare  protect                                duty  hardware
   clock                          count    count    count        rate   accuracy phase  cycle    enable
-------------------------------------------------------------------------------------------------------
 xtal                                 7        7        0    24000000          0     0  50000         Y
    fe07a000.serial#xtal_div2         1        1        0    12000000          0     0  50000         Y
       fe07a000.serial#xtal2_clk_sel       1        1        0    12000000          0     0  50000         Y
          fe07a000.serial#use_xtal       1        1        0    12000000          0     0  50000         Y
             fe07a000.serial#baud_div       1        1        0      923077          0     0  50000         Y
    fe07a000.serial#xtal_div3         0        0        0     8000000          0     0  50000         Y
       fe07a000.serial#xtal_clk_sel       0        0        0     8000000          0     0  50000         Y

# 
# stty -F /dev/ttyAML0  115200

# stty -F /dev/ttyAML0 
speed 115200 baud; line = 0;
intr = ^C; quit = ^\; erase = ^?; kill = ^U; eof = ^D; eol = <undef>;
eol2 = <undef>; swtch = <undef>; start = ^Q; stop = ^S; susp = ^Z; rprnt = ^R;
werase = ^W; lnext = ^V; flush = ^O; min = 1; time = 0;
-brkint ixoff -imaxbel

[-- Attachment #3: Type: text/plain, Size: 167 bytes --]

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

WARNING: multiple messages have this Message-ID (diff)
From: Yu Tu <yu.tu@amlogic.com>
To: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Cc: <linux-serial@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-amlogic@lists.infradead.org>,
	<linux-kernel@vger.kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jiri Slaby <jirislaby@kernel.org>,
	Neil Armstrong <narmstrong@baylibre.com>,
	Kevin Hilman <khilman@baylibre.com>,
	Jerome Brunet <jbrunet@baylibre.com>
Subject: Re: [PATCH 3/3] tty: serial: meson: add UART driver compatible with S4 SoC on-chip
Date: Tue, 28 Dec 2021 19:24:11 +0800	[thread overview]
Message-ID: <36d63dde-6bfc-925b-51ae-ee801dcfa681@amlogic.com> (raw)
In-Reply-To: <CAFBinCAr=U0BM6O329td6=Nw4oknyKQUgTGtwhq4RdCLdR_H+A@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1930 bytes --]

Hi Martin and Jerome,
	Thank you very much for your reply. I have learned a lot from your 
communication.

On 2021/12/28 4:04, Martin Blumenstingl wrote:
> [ EXTERNAL EMAIL ]
> 
> Hello,
> 
> On Mon, Dec 27, 2021 at 7:56 AM Yu Tu <yu.tu@amlogic.com> wrote:
> [...]
>>> Does the new S4 SoC use an external 12MHz XTAL or does it use a 24MHz XTAL?
>>> If there's still a 24MHz XTAL then I think this description is not
>>> correct - at least based on how I understand the UART controller.
>>>
>> The S4 SoC uses 12MHz(UART_EE_A_REG5[27]=0x1,the bit is set in romcode).
>> This register description is the same as the G12A and G12B you know.
> Thank you for this explanation!
> So the problem is that we're not touching bit 26 and bit 27 - and with
> the updated romcode you would not get any serial output since the
> divider is calculated off the wrong clock.
> 
> I agree with Jerome that we shouldn't put a flag in device-tree.
> 
> Also I did some experimenting with Jerome's idea to implement the
> clocks using CCF (common clock framework), see the attached patches.
> It was a bit tricky because some initial clean-ups were needed in the
> serial driver.
> Note: I have only briefly tested this on a 32-bit Meson8m2 SoC, see my
> attached patches and the clk_summary debugfs output.
> In fact, I expect that there are some issues with at least one of the
> patches as the whole bit 26 and bit 27 code is untested.
> 
> Do you see any problems with this patch?
> Could you try to implement CCF support with the idea from the attached
> patches (you don't need to re-use them, I just wrote them to make it
> clearer in our discussion what we're talking about).
> 
I couldn't agree with you more. I have verified it on a 64-bit S4 
platform. Please refer to the attachment for verification output 
information.
I will prepare the second version of patch according to the example 
ideas you provided.
> 
> Best regards,
> Martin

[-- Attachment #2: s4-clk-debug-output.txt --]
[-- Type: text/plain, Size: 1440 bytes --]

# cat /sys/kernel/debug/clk/clk_summary 
                                 enable  prepare  protect                                duty  hardware
   clock                          count    count    count        rate   accuracy phase  cycle    enable
-------------------------------------------------------------------------------------------------------
 xtal                                 7        7        0    24000000          0     0  50000         Y
    fe07a000.serial#xtal_div2         1        1        0    12000000          0     0  50000         Y
       fe07a000.serial#xtal2_clk_sel       1        1        0    12000000          0     0  50000         Y
          fe07a000.serial#use_xtal       1        1        0    12000000          0     0  50000         Y
             fe07a000.serial#baud_div       1        1        0      923077          0     0  50000         Y
    fe07a000.serial#xtal_div3         0        0        0     8000000          0     0  50000         Y
       fe07a000.serial#xtal_clk_sel       0        0        0     8000000          0     0  50000         Y

# 
# stty -F /dev/ttyAML0  115200

# stty -F /dev/ttyAML0 
speed 115200 baud; line = 0;
intr = ^C; quit = ^\; erase = ^?; kill = ^U; eof = ^D; eol = <undef>;
eol2 = <undef>; swtch = <undef>; start = ^Q; stop = ^S; susp = ^Z; rprnt = ^R;
werase = ^W; lnext = ^V; flush = ^O; min = 1; time = 0;
-brkint ixoff -imaxbel

[-- Attachment #3: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2021-12-28 11:24 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-21  7:16 [PATCH 0/3] the UART driver compatible with the Amlogic Meson S4 SoC Yu Tu
2021-12-21  7:16 ` Yu Tu
2021-12-21  7:16 ` Yu Tu
2021-12-21  7:16 ` [PATCH 1/3] tty: serial: meson: modify request_irq and free_irq Yu Tu
2021-12-21  7:16   ` Yu Tu
2021-12-21  7:16   ` Yu Tu
2021-12-21  7:30   ` Greg Kroah-Hartman
2021-12-21  7:30     ` Greg Kroah-Hartman
2021-12-21  7:30     ` Greg Kroah-Hartman
2021-12-22  8:44     ` Yu Tu
2021-12-22  8:44       ` Yu Tu
2021-12-22  8:44       ` Yu Tu
2021-12-21  7:16 ` [PATCH 2/3] tty: serial: meson: meson_uart_shutdown omit clear AML_UART_TX_EN bit Yu Tu
2021-12-21  7:16   ` Yu Tu
2021-12-21  7:16   ` Yu Tu
2021-12-21  7:32   ` Greg Kroah-Hartman
2021-12-21  7:32     ` Greg Kroah-Hartman
2021-12-21  7:32     ` Greg Kroah-Hartman
2021-12-22  9:01     ` Yu Tu
2021-12-22  9:01       ` Yu Tu
2021-12-22  9:01       ` Yu Tu
2021-12-21  7:16 ` [PATCH 3/3] tty: serial: meson: add UART driver compatible with S4 SoC on-chip Yu Tu
2021-12-21  7:16   ` Yu Tu
2021-12-21  7:16   ` Yu Tu
2021-12-21  7:34   ` Greg Kroah-Hartman
2021-12-21  7:34     ` Greg Kroah-Hartman
2021-12-21  7:34     ` Greg Kroah-Hartman
2021-12-22  9:28     ` Yu Tu
2021-12-22  9:28       ` Yu Tu
2021-12-22  9:28       ` Yu Tu
2021-12-24 17:25   ` Martin Blumenstingl
2021-12-24 17:25     ` Martin Blumenstingl
2021-12-24 17:25     ` Martin Blumenstingl
2021-12-27  6:56     ` Yu Tu
2021-12-27  6:56       ` Yu Tu
2021-12-27  6:56       ` Yu Tu
2021-12-27 11:58       ` Jerome Brunet
2021-12-27 11:58         ` Jerome Brunet
2021-12-27 11:58         ` Jerome Brunet
2021-12-27 20:04       ` Martin Blumenstingl
2021-12-27 20:04         ` Martin Blumenstingl
2021-12-27 20:04         ` Martin Blumenstingl
2021-12-28 11:24         ` Yu Tu [this message]
2021-12-28 11:24           ` Yu Tu
2021-12-28 11:24           ` Yu Tu
2021-12-21  7:30 ` [PATCH 0/3] the UART driver compatible with the Amlogic Meson S4 SoC Greg Kroah-Hartman
2021-12-21  7:30   ` Greg Kroah-Hartman
2021-12-21  7:30   ` Greg Kroah-Hartman
2021-12-22  8:19   ` Yu Tu
2021-12-22  8:19     ` Yu Tu
2021-12-22  8:19     ` Yu Tu

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=36d63dde-6bfc-925b-51ae-ee801dcfa681@amlogic.com \
    --to=yu.tu@amlogic.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jbrunet@baylibre.com \
    --cc=jirislaby@kernel.org \
    --cc=khilman@baylibre.com \
    --cc=linux-amlogic@lists.infradead.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=martin.blumenstingl@googlemail.com \
    --cc=narmstrong@baylibre.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: link
Be 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.