Possible misleading information in rockchip,rk3588-cru.yaml

Possible misleading information in rockchip,rk3588-cru.yaml

[Yao Zi]

Hi,

rockchip,rk3588-cru.yaml, dt-binding for RK3588 clock and reset module,
contains description of customized property "rockchip,grf",

  rockchip,grf:
    $ref: /schemas/types.yaml#/definitions/phandle
    description: >
      phandle to the syscon managing the "general register files". It is
      used for GRF muxes, if missing any muxes present in the GRF will
      not be available.

But after doing some searching, I found that clk-rk3588.c actually
defines no clock hardware with MUXGRF type. This is also true in in the
vendor code[1], it seems there is actually no GRF mux on RK3588
platform.

Best regards,
Yao Zi

[1]: https://github.com/rockchip-linux/kernel/blob/604cec4004abe5a96c734f2fab7b74809d2d742f/drivers/clk/rockchip/clk-rk3588.c

Re: Possible misleading information in rockchip,rk3588-cru.yaml

[Conor Dooley]

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

On Wed, Sep 11, 2024 at 09:20:02PM +0000, Yao Zi wrote:
> Hi,
> 
> rockchip,rk3588-cru.yaml, dt-binding for RK3588 clock and reset module,
> contains description of customized property "rockchip,grf",
> 
>   rockchip,grf:
>     $ref: /schemas/types.yaml#/definitions/phandle
>     description: >
>       phandle to the syscon managing the "general register files". It is
>       used for GRF muxes, if missing any muxes present in the GRF will
>       not be available.
> 
> But after doing some searching, I found that clk-rk3588.c actually
> defines no clock hardware with MUXGRF type. This is also true in in the
> vendor code[1], it seems there is actually no GRF mux on RK3588
> platform.

Have you been able to check the datasheet/register map for this piece of
hardware? Does it have a grf register region?
Wouldn't be surprised if it didn't, and the cause of it being in the
binding was nothing more than copy-paste.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: Possible misleading information in rockchip,rk3588-cru.yaml

[Yao Zi]

On Mon, Sep 16, 2024 at 05:33:49PM +0100, Conor Dooley wrote:
> On Wed, Sep 11, 2024 at 09:20:02PM +0000, Yao Zi wrote:
> > Hi,
> > 
> > rockchip,rk3588-cru.yaml, dt-binding for RK3588 clock and reset module,
> > contains description of customized property "rockchip,grf",
> > 
> >   rockchip,grf:
> >     $ref: /schemas/types.yaml#/definitions/phandle
> >     description: >
> >       phandle to the syscon managing the "general register files". It is
> >       used for GRF muxes, if missing any muxes present in the GRF will
> >       not be available.
> > 
> > But after doing some searching, I found that clk-rk3588.c actually
> > defines no clock hardware with MUXGRF type. This is also true in in the
> > vendor code[1], it seems there is actually no GRF mux on RK3588
> > platform.
> 
> Have you been able to check the datasheet/register map for this piece of
> hardware? Does it have a grf register region?
> Wouldn't be surprised if it didn't, and the cause of it being in the
> binding was nothing more than copy-paste.

Have checked a public datasheet[1], RK3588 does have corresponding grf
region and there are only clock related bits in PHP_GRF_CLK_CON1[2].

But these gmac clocks bits are used in dwmac-rk GMAC driver[3]
internally, out of the common clock driver, rk3588-cru. So I don't think
the CRU needs access to the grf by design.

Best regards,
Yao Zi

[1]: https://github.com/FanX-Tek/rk3588-TRM-and-Datasheet/blob/master/Rockchip%20RK3588%20TRM%20V1.0-Part1-20220309.pdf
[2]: Page 836 in [1]
[3]: net/ethernet/stmicro/stmmac/dwmac-rk.c:1132

Re: Possible misleading information in rockchip,rk3588-cru.yaml

[Conor Dooley]

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

On Mon, Sep 16, 2024 at 05:38:56PM +0000, Yao Zi wrote:
> On Mon, Sep 16, 2024 at 05:33:49PM +0100, Conor Dooley wrote:
> > On Wed, Sep 11, 2024 at 09:20:02PM +0000, Yao Zi wrote:
> > > Hi,
> > > 
> > > rockchip,rk3588-cru.yaml, dt-binding for RK3588 clock and reset module,
> > > contains description of customized property "rockchip,grf",
> > > 
> > >   rockchip,grf:
> > >     $ref: /schemas/types.yaml#/definitions/phandle
> > >     description: >
> > >       phandle to the syscon managing the "general register files". It is
> > >       used for GRF muxes, if missing any muxes present in the GRF will
> > >       not be available.
> > > 
> > > But after doing some searching, I found that clk-rk3588.c actually
> > > defines no clock hardware with MUXGRF type. This is also true in in the
> > > vendor code[1], it seems there is actually no GRF mux on RK3588
> > > platform.
> > 
> > Have you been able to check the datasheet/register map for this piece of
> > hardware? Does it have a grf register region?
> > Wouldn't be surprised if it didn't, and the cause of it being in the
> > binding was nothing more than copy-paste.
> 
> Have checked a public datasheet[1], RK3588 does have corresponding grf
> region and there are only clock related bits in PHP_GRF_CLK_CON1[2].
> 
> But these gmac clocks bits are used in dwmac-rk GMAC driver[3]
> internally, out of the common clock driver, rk3588-cru. So I don't think
> the CRU needs access to the grf by design.

That sounds like a reasonable justification for deleting it - but please
check U-Boot etc to make sure that other projects are not using this in
a different manner to the kernel. If they are not using it, then please
send a patch :)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]