mbox series

[V6,0/5] Use CCF to describe the UART baud rate clock

Message ID 20220118030911.12815-1-yu.tu@amlogic.com (mailing list archive)
Headers show
Series Use CCF to describe the UART baud rate clock | expand

Message

Yu Tu Jan. 18, 2022, 3:09 a.m. UTC
Using the common Clock code to describe the UART baud rate
clock makes it easier for the UART driver to be compatible
with the baud rate requirements of the UART IP on different
meson chips. Add Meson S4 SoC compatible.

Yu Tu (5):
  tty: serial: meson: Move request the register region to probe
  tty: serial: meson: Use devm_ioremap_resource to get register mapped
    memory
  tty: serial: meson: Describes the calculation of the UART baud rate
    clock using a clock frame
  tty: serial: meson: Make some bit of the REG5 register writable
  tty: serial: meson: Added S4 SOC compatibility

V5 -> V6: Change error format as discussed in the email.
V4 -> V5: Change error format.
V3 -> V4: Change CCF to describe the UART baud rate clock as discussed
in the email.
V2 -> V3: add compatible = "amlogic,meson-gx-uart". Because it must change
the DTS before it can be deleted
V1 -> V2: Use CCF to describe the UART baud rate clock.Make some changes as
discussed in the email

Link:https://lore.kernel.org/linux-amlogic/20220110104214.25321-4-yu.tu@amlogic.com/

 drivers/tty/serial/meson_uart.c | 217 ++++++++++++++++++++++----------
 1 file changed, 149 insertions(+), 68 deletions(-)


base-commit: 4d66020dcef83314092f2c8c89152a8d122627e2

Comments

Kevin Hilman Jan. 19, 2022, 10:37 p.m. UTC | #1
Hello,

Yu Tu <yu.tu@amlogic.com> writes:

> Using the common Clock code to describe the UART baud rate
> clock makes it easier for the UART driver to be compatible
> with the baud rate requirements of the UART IP on different
> meson chips. Add Meson S4 SoC compatible.

Could you describe how this was tested and on which SoCs?  There seem to
be some changes in this series that might affect previous SoCs.

Thanks,

Kevin
Yu Tu Jan. 20, 2022, 8:43 a.m. UTC | #2
Hi Kevin,
	Thank you very much for your reply.

On 2022/1/20 6:37, Kevin Hilman wrote:
> [ EXTERNAL EMAIL ]
> 
> Hello,
> 
> Yu Tu <yu.tu@amlogic.com> writes:
> 
>> Using the common Clock code to describe the UART baud rate
>> clock makes it easier for the UART driver to be compatible
>> with the baud rate requirements of the UART IP on different
>> meson chips. Add Meson S4 SoC compatible.
> 
> Could you describe how this was tested and on which SoCs?  There seem to
> be some changes in this series that might affect previous SoCs.
> 
For me, the board starts normally and prints. My intention was to add 
the S4 SOC UART compatible, but for the S4 our baud rate clock is 
calculated at 12MHz by default.So a series of changes were made at your 
suggestion.

Since most SoCs are too old, I was able to find all the platforms myself 
such as Meson6, Meson8, Meson8b, GXL and so on. I only tested it with 
G12A and S4.But when I talked to Martin earlier he tried meson8b's log.
The test patch is in the attachment.

I have found that on some boards with this change, the initcall_debug 
Uart driver takes longer to initialize. Running the stty command to 
change the baud rate at the same time may cause a jam.

I'd love to know what else you suggest.

> Thanks,
> 
> Kevin
> 
>
# cat /sys/kernel/debug/clk//clk_summary | head
                                 enable  prepare  protect                                duty  hardware
   clock                          count    count    count        rate   accuracy phase  cycle    enable
-------------------------------------------------------------------------------------------------------
 xtal                                 6        6        0    24000000          0     0  50000         Y
    ff803000.serial#xtal_div          1        1        0    12000000          0     0  50000         Y
       ff803000.serial#use_xtal       1        1        0    12000000          0     0  50000         Y
          ff803000.serial#baud_div       1        1        0      115385          0     0  50000         Y
    cts_oscin                         0        0        0    24000000          0     0  50000         Y
       g12a_ao_cec_pre                0        0        0    24000000          0     0  50000         N
          g12a_ao_cec_div             0        0        0       32742          0     0  50000         Y
# cat /sys/kernel/debug/clk/clk_summary 
                                 enable  prepare  protect                                duty  hardware
   clock                          count    count    count        rate   accuracy phase  cycle    enable
-------------------------------------------------------------------------------------------------------
[...]
 xtal                                 6        6        2    24000000          0     0  50000         Y
[...]
    c81004c0.serial#xtal_div3         0        0        0     8000000          0     0  50000         Y
[...]
    fixed_pll_dco                     1        1        0  2550000000          0     0  50000         Y
       fixed_pll                      1        1        0  2550000000          0     0  50000         Y
[...]
          fclk_div3_div               1        1        0   850000000          0     0  50000         Y
             fclk_div3                2        2        0   850000000          0     0  50000         Y
[...]
                mpeg_clk_sel          1        1        0   850000000          0     0  50000         Y
                   mpeg_clk_div       1        1        0   141666667          0     0  50000         Y
                      clk81          17       20        0   141666667          0     0  50000         Y
[...]
                         c81004c0.serial#clk81_div4       1        1        0    35416666          0     0  50000         Y
                            c81004c0.serial#use_xtal       1        1        0    35416666          0     0  50000         Y
                               c81004c0.serial#baud_div       1        1        0      115364          0     0  50000         Y
# cat /sys/kernel/debug/clk/clk_summary | head
                                 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_div          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
    hdcp22_skpclk_mux                 0        0        0    24000000          0     0  50000         Y
       hdcp22_skpclk_div              0        0        0    24000000          0     0  50000         Y
          hdcp22_skpclk_gate          0        0        0    24000000          0     0  50000         N
Kevin Hilman Jan. 24, 2022, 7:58 p.m. UTC | #3
Yu Tu <yu.tu@amlogic.com> writes:

> Hi Kevin,
> 	Thank you very much for your reply.
>
> On 2022/1/20 6:37, Kevin Hilman wrote:
>> [ EXTERNAL EMAIL ]
>> 
>> Hello,
>> 
>> Yu Tu <yu.tu@amlogic.com> writes:
>> 
>>> Using the common Clock code to describe the UART baud rate
>>> clock makes it easier for the UART driver to be compatible
>>> with the baud rate requirements of the UART IP on different
>>> meson chips. Add Meson S4 SoC compatible.
>> 
>> Could you describe how this was tested and on which SoCs?  There seem to
>> be some changes in this series that might affect previous SoCs.
>> 
> For me, the board starts normally and prints. My intention was to add 
> the S4 SOC UART compatible, but for the S4 our baud rate clock is 
> calculated at 12MHz by default.So a series of changes were made at your 
> suggestion.
>
> Since most SoCs are too old, I was able to find all the platforms myself 
> such as Meson6, Meson8, Meson8b, GXL and so on. I only tested it with 
> G12A and S4.But when I talked to Martin earlier he tried meson8b's log.
> The test patch is in the attachment.
>
> I have found that on some boards with this change, the initcall_debug 
> Uart driver takes longer to initialize. Running the stty command to 
> change the baud rate at the same time may cause a jam.

This kind of detail is important to document in the cover letter,
including a bit more detail on how to reproduce so that other can help
test or may have ideas for how to solve.

> I'd love to know what else you suggest.

I don't expect you to be able to test on all SoCs, but just to list what
SoCs and which boards you tested on.  This way, those who have other
boards can help test and we can have a better idea of how this was
tested before merging.

Thanks,

Kevin
Yu Tu Feb. 7, 2022, 2:19 p.m. UTC | #4
Hi Kevin,
	First of all,thank you very much for your reply.Due to the Chinese 
Spring Festival holiday recently, so i just reply to you now。

On 2022/1/25 3:58, Kevin Hilman wrote:
> [ EXTERNAL EMAIL ]
> 
> Yu Tu <yu.tu@amlogic.com> writes:
> 
>> Hi Kevin,
>> 	Thank you very much for your reply.
>>
>> On 2022/1/20 6:37, Kevin Hilman wrote:
>>> [ EXTERNAL EMAIL ]
>>>
>>> Hello,
>>>
>>> Yu Tu <yu.tu@amlogic.com> writes:
>>>
>>>> Using the common Clock code to describe the UART baud rate
>>>> clock makes it easier for the UART driver to be compatible
>>>> with the baud rate requirements of the UART IP on different
>>>> meson chips. Add Meson S4 SoC compatible.
>>>
>>> Could you describe how this was tested and on which SoCs?  There seem to
>>> be some changes in this series that might affect previous SoCs.
>>>
>> For me, the board starts normally and prints. My intention was to add
>> the S4 SOC UART compatible, but for the S4 our baud rate clock is
>> calculated at 12MHz by default.So a series of changes were made at your
>> suggestion.
>>
>> Since most SoCs are too old, I was able to find all the platforms myself
>> such as Meson6, Meson8, Meson8b, GXL and so on. I only tested it with
>> G12A and S4.But when I talked to Martin earlier he tried meson8b's log.
>> The test patch is in the attachment.
>>
>> I have found that on some boards with this change, the initcall_debug
>> Uart driver takes longer to initialize. Running the stty command to
>> change the baud rate at the same time may cause a jam.
> 
> This kind of detail is important to document in the cover letter,
> including a bit more detail on how to reproduce so that other can help
> test or may have ideas for how to solve.
> 
The problem recurrence method is described below:
First, add the patch I changed. It then launches normally to the console 
command line.  Finally, run the stty command to change buad rate. The 
detailed commands are as follows:
stty -F /dev/ttyAML0 115200 and stty -F /dev/ttyAML0 921600 .Alternate 
execution can reproduce.

>> I'd love to know what else you suggest.
> 
> I don't expect you to be able to test on all SoCs, but just to list what
> SoCs and which boards you tested on.  This way, those who have other
> boards can help test and we can have a better idea of how this was
> tested before merging.
> 
I only have S4 and G12A so far, so I've only tested on those two platforms.

> Thanks,
> 
> Kevin
>