lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 23 Feb 2023 11:03:04 +0800
From:   Hal Feng <hal.feng@...rfivetech.com>
To:     Conor Dooley <conor@...nel.org>
CC:     Stephen Boyd <sboyd@...nel.org>, <linux-riscv@...ts.infradead.org>,
        <devicetree@...r.kernel.org>, <linux-clk@...r.kernel.org>,
        Palmer Dabbelt <palmer@...belt.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Michael Turquette <mturquette@...libre.com>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        "Emil Renner Berthing" <emil.renner.berthing@...onical.com>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 07/11] dt-bindings: clock: Add StarFive JH7110 system
 clock and reset generator

On Wed, 22 Feb 2023 16:26:46 +0000, Conor Dooley wrote:
> On Wed, Feb 22, 2023 at 09:27:37PM +0800, Hal Feng wrote:
>> On Tue, 21 Feb 2023 23:39:32 +0000, Conor Dooley wrote:
>> > On Tue, Feb 21, 2023 at 02:17:17PM -0800, Stephen Boyd wrote:
>> >> Quoting Conor Dooley (2023-02-16 10:20:34)
>> >> > On Thu, Feb 16, 2023 at 10:42:20PM +0800, Hal Feng wrote:
>> >> > > On Tue, 27 Dec 2022 20:15:20 +0000, Conor Dooley wrote:
>> >> > > > On Mon, Dec 26, 2022 at 12:26:32AM +0800, Hal Feng wrote:
>> >> > > Please see the picture of these external clocks in clock tree.
>> >> > > 
>> >> > > # mount -t debugfs none /mnt
>> >> > > # cat /mnt/clk/clk_summary
>> >> > >                                  enable  prepare  protect                                duty  hardware
>> >> > >    clock                          count    count    count        rate   accuracy phase  cycle    enable
>> >> > > -------------------------------------------------------------------------------------------------------
>> >> > >  *mclk_ext*                             0        0        0    12288000          0     0  50000         Y
>> >> > >  *tdm_ext*                              0        0        0    49152000          0     0  50000         Y
>> >> > >  *i2srx_lrck_ext*                       0        0        0      192000          0     0  50000         Y
>> >> > >  *i2srx_bclk_ext*                       0        0        0    12288000          0     0  50000         Y
>> >> > >  *i2stx_lrck_ext*                       0        0        0      192000          0     0  50000         Y
>> >> > >  *i2stx_bclk_ext*                       0        0        0    12288000          0     0  50000         Y
>> >> > >  *gmac1_rgmii_rxin*                     0        0        0   125000000          0     0  50000         Y
>> >> > >     gmac1_rx                          0        0        0   125000000          0     0  50000         Y
>> >> > >        gmac1_rx_inv                   0        0        0   125000000          0   180  50000         Y
>> >> > >  *gmac1_rmii_refin*                     0        0        0    50000000          0     0  50000         Y
>> >> > >     gmac1_rmii_rtx                    0        0        0    50000000          0     0  50000         Y
>> >> > >        gmac1_tx                       0        0        0    50000000          0     0  50000         N
>> >> > >           gmac1_tx_inv                0        0        0    50000000          0   180  50000         Y
>> >> > >  *osc*                                  4        4        0    24000000          0     0  50000         Y
>> >> > >     apb_func                          0        0        0    24000000          0     0  50000         Y
>> >> > >  ...
>> >> > > 
>> >> > > The clock "gmac1_rgmii_rxin" and the clock "gmac1_rmii_refin" are
>> >> > > actually used as the parent of other clocks.
>> >> > 
>> >> > > The "dummy" clocks
>> >> > > you said are all internal clocks.
>> >> > 
>> >> > No, what I meant by "dummy" clocks is that if you make clocks "required"
>> >> > in the binding that are not needed by the hardware for operation a
>> >> > customer of yours might have to add "dummy" clocks to their devicetree
>> >> > to pass dtbs_check.
>> >> 
>> >> They can set the phandle specifier to '<0>' to fill in the required
>> >> property when there isn't anything there. If this is inside an SoC, it
>> >> is always connected because silicon can't change after it is made
>> >> (unless this is an FPGA). Therefore, any and all input clocks should be
>> >> listed as required.
>> > 
>> >> If the clk controller has inputs that are
>> >> pads/balls/pins on the SoC then they can be optional if a valid design
>> >> can leave those pins not connected.
>> > 
>> > From the discussion on the dts patches, where the clocks have been put
>> > (intentionally) into board.dts, I've been under the impression that we
>> > are in this situation.
>> 
>> For the system (sys) clock controller, we are in this situation.
>> For the always-on (aon) clock controller, we are not, because some input
>> clocks are inside the SoC.
>> 
>> > Up to Hal to tell us if the hardware is capable of having those inputs
>> > left unfilled!
>> 
>> The situation is different for v1.2A and v1.3B boards.
>> 
>> For the v1.2A board,
>> gmac1 only requires "gmac1_rmii_refin", which support 100MHz
>> gmac0 only requires "gmac0_rgmii_rxin", which support 1000MHz
>> 
>> For the v1.3B board,
>> gmac1 only requires "gmac1_rgmii_rxin", which support 1000MHz
>> gmac0 only requires "gmac0_rgmii_rxin", which support 1000MHz
>> 
>> So we should set the "required" property depending on different
>> boards.
> 
> These were Krzk's suggestions:
> oneOf:
>  - clock-names:
>      minItems: 3
>      items:
>        - a
>        - b
>        - c
>        - d
>  - clock-names:
>      items:
>        - a
>        - b
>        - d
> 
> or maybe:
>  - clock-names:
>      minItems: 3
>      items:
>        - a
>        - b
>        - enum: [c, d]
>        - d
> 
> Might be making a mess here, but I think that becomes:
>   clock-names:
>     oneOf:
>       - items:
>           - const: osc
>           - enum:
>               - gmac1_rmii_refin
>               - gmac1_rgmii_rxin
>           - const: i2stx_bclk_ext
>           - const: i2stx_lrck_ext
>           - const: i2srx_bclk_ext
>           - const: i2srx_lrck_ext
>           - const: tdm_ext
>           - const: mclk_ext
> 
>       - items:
>           - const: osc
>           - const: gmac1_rmii_refin
>           - const: gmac1_rgmii_rxin
>           - const: i2stx_bclk_ext
>           - const: i2stx_lrck_ext
>           - const: i2srx_bclk_ext
>           - const: i2srx_lrck_ext
>           - const: tdm_ext
>           - const: mclk_ext

Will modify it and improve the description of clock items for
pointing out which clock is required on different boards.
Thank you all for your helpful suggestions.

Best regards,
Hal

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ