1. Patchew
  2. linux
  3. [PATCH 0/2] clk: imx: introduce fsl,protected-clocks
  4. View patch

[PATCH 1/2] dt-bindings: clock: imx8m: introduce fsl,protected-clocks property

Peng Fan (OSS) posted 2 patches 3 years, 7 months ago
[PATCH 1/2] dt-bindings: clock: imx8m: introduce fsl,protected-clocks property
Posted by Peng Fan (OSS) 3 years, 7 months ago 
From: Peng Fan <peng.fan@nxp.com>

i.MX8M Linux run on top of Jailhouse hypervisor, the root cell Linux
should not disable clocks used by inmate. This would also benifit
AMP to avoid Linux disable clocks used by Cortex-M4/M7.

So introduce fsl,protected-clocks for above case.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 Documentation/devicetree/bindings/clock/imx8m-clock.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/imx8m-clock.yaml b/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
index 458c7645ee68..0ec490ff9a09 100644
--- a/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
+++ b/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
@@ -39,6 +39,10 @@ properties:
       ID in its "clocks" phandle cell. See include/dt-bindings/clock/imx8m-clock.h
       for the full list of i.MX8M clock IDs.
 
+  fsl,protected-clocks:
+    description: List of the Protected clock.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+
 required:
   - compatible
   - reg
-- 
2.37.1
Re: [PATCH 1/2] dt-bindings: clock: imx8m: introduce fsl,protected-clocks property
Posted by Sascha Hauer 3 years, 7 months ago 
Hi Peng,

On Mon, Aug 15, 2022 at 11:36:31AM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> i.MX8M Linux run on top of Jailhouse hypervisor, the root cell Linux
> should not disable clocks used by inmate. This would also benifit
> AMP to avoid Linux disable clocks used by Cortex-M4/M7.
> 
> So introduce fsl,protected-clocks for above case.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  Documentation/devicetree/bindings/clock/imx8m-clock.yaml | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/clock/imx8m-clock.yaml b/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
> index 458c7645ee68..0ec490ff9a09 100644
> --- a/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
> +++ b/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
> @@ -39,6 +39,10 @@ properties:
>        ID in its "clocks" phandle cell. See include/dt-bindings/clock/imx8m-clock.h
>        for the full list of i.MX8M clock IDs.
>  
> +  fsl,protected-clocks:
> +    description: List of the Protected clock.
> +    $ref: /schemas/types.yaml#/definitions/uint32-array

There already is a generic protected-clocks property described in
https://github.com/devicetree-org/dt-schema/blob/0d1b78cd0c3d9a3d523ced17d7da64b03f6c18ea/dtschema/schemas/clock/clock.yaml#L131
We probably shouldn't add a property with the same name but different
meaning.

I am not sure if we want to go the route of a fsl specific property, it
looks like other SoCs could have similar problems and it might be worth
solving this problem with a broader view.

Anyway, please add a description to the binding what this property
actually does.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
RE: [PATCH 1/2] dt-bindings: clock: imx8m: introduce fsl,protected-clocks property
Posted by Peng Fan 3 years, 7 months ago 
Hi Sascha,

> Subject: Re: [PATCH 1/2] dt-bindings: clock: imx8m: introduce fsl,protected-
> clocks property
> 
> Hi Peng,
> 
> On Mon, Aug 15, 2022 at 11:36:31AM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > i.MX8M Linux run on top of Jailhouse hypervisor, the root cell Linux
> > should not disable clocks used by inmate. This would also benifit AMP
> > to avoid Linux disable clocks used by Cortex-M4/M7.
> >
> > So introduce fsl,protected-clocks for above case.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >  Documentation/devicetree/bindings/clock/imx8m-clock.yaml | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
> > b/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
> > index 458c7645ee68..0ec490ff9a09 100644
> > --- a/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
> > +++ b/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
> > @@ -39,6 +39,10 @@ properties:
> >        ID in its "clocks" phandle cell. See include/dt-bindings/clock/imx8m-
> clock.h
> >        for the full list of i.MX8M clock IDs.
> >
> > +  fsl,protected-clocks:
> > +    description: List of the Protected clock.
> > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> 
> There already is a generic protected-clocks property described in
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithu
> b.com%2Fdevicetree-org%2Fdt-
> schema%2Fblob%2F0d1b78cd0c3d9a3d523ced17d7da64b03f6c18ea%2Fdtsc
> hema%2Fschemas%2Fclock%2Fclock.yaml%23L131&amp;data=05%7C01%7
> Cpeng.fan%40nxp.com%7C5dbc72639c9147765af208da7ec63315%7C686ea
> 1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637961686968811809%7CUn
> known%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI
> 6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=D%2BfJA5h
> wblaX8VH%2BdQoN0pEFmCipfZHHf0ZVo07B4kg%3D&amp;reserved=0
> We probably shouldn't add a property with the same name but different
> meaning.
> 

Thanks for sharing the info. I should check the common bindings before
cook this patchset.

> I am not sure if we want to go the route of a fsl specific property, it looks
> like other SoCs could have similar problems and it might be worth solving
> this problem with a broader view.
> 

I see qcom just drop the clock entries before registering the clocks. But to
i.MX8M, it is not feasible to drop those clocks, unless check the
protected-clocks property before registering every clock. This is odd.

So here I just wanna let i.MX8M clk driver prepare enable the clocks listed
in protected-clocks property to avoid linux disable those clocks.
> Anyway, please add a description to the binding what this property actually
> does.
I will switch to use the common bindings.

Thanks,
Peng.
> 
> Sascha
> 
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       |
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.
> pengutronix.de%2F&amp;data=05%7C01%7Cpeng.fan%40nxp.com%7C5dbc
> 72639c9147765af208da7ec63315%7C686ea1d3bc2b4c6fa92cd99c5c301635
> %7C0%7C0%7C637961686968811809%7CUnknown%7CTWFpbGZsb3d8eyJW
> IjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7
> C3000%7C%7C%7C&amp;sdata=7UqyWwDDhaze2qZJ%2FknZ6Am7Y%2Bs%
> 2Fbpyv9SITJHgsZPw%3D&amp;reserved=0  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
Re: [PATCH 1/2] dt-bindings: clock: imx8m: introduce fsl,protected-clocks property
Posted by Sascha Hauer 3 years, 7 months ago 
On Tue, Aug 16, 2022 at 07:13:27AM +0000, Peng Fan wrote:
> Hi Sascha,
> 
> > Subject: Re: [PATCH 1/2] dt-bindings: clock: imx8m: introduce fsl,protected-
> > clocks property
> > 
> > Hi Peng,
> > 
> > On Mon, Aug 15, 2022 at 11:36:31AM +0800, Peng Fan (OSS) wrote:
> > > From: Peng Fan <peng.fan@nxp.com>
> > >
> > > i.MX8M Linux run on top of Jailhouse hypervisor, the root cell Linux
> > > should not disable clocks used by inmate. This would also benifit AMP
> > > to avoid Linux disable clocks used by Cortex-M4/M7.
> > >
> > > So introduce fsl,protected-clocks for above case.
> > >
> > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > ---
> > >  Documentation/devicetree/bindings/clock/imx8m-clock.yaml | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
> > > b/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
> > > index 458c7645ee68..0ec490ff9a09 100644
> > > --- a/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
> > > +++ b/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
> > > @@ -39,6 +39,10 @@ properties:
> > >        ID in its "clocks" phandle cell. See include/dt-bindings/clock/imx8m-
> > clock.h
> > >        for the full list of i.MX8M clock IDs.
> > >
> > > +  fsl,protected-clocks:
> > > +    description: List of the Protected clock.
> > > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > 
> > There already is a generic protected-clocks property described in
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithu
> > b.com%2Fdevicetree-org%2Fdt-
> > schema%2Fblob%2F0d1b78cd0c3d9a3d523ced17d7da64b03f6c18ea%2Fdtsc
> > hema%2Fschemas%2Fclock%2Fclock.yaml%23L131&amp;data=05%7C01%7
> > Cpeng.fan%40nxp.com%7C5dbc72639c9147765af208da7ec63315%7C686ea
> > 1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637961686968811809%7CUn
> > known%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI
> > 6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=D%2BfJA5h
> > wblaX8VH%2BdQoN0pEFmCipfZHHf0ZVo07B4kg%3D&amp;reserved=0
> > We probably shouldn't add a property with the same name but different
> > meaning.
> > 
> 
> Thanks for sharing the info. I should check the common bindings before
> cook this patchset.
> 
> > I am not sure if we want to go the route of a fsl specific property, it looks
> > like other SoCs could have similar problems and it might be worth solving
> > this problem with a broader view.
> > 
> 
> I see qcom just drop the clock entries before registering the clocks. But to
> i.MX8M, it is not feasible to drop those clocks, unless check the
> protected-clocks property before registering every clock. This is odd.
> 
> So here I just wanna let i.MX8M clk driver prepare enable the clocks listed
> in protected-clocks property to avoid linux disable those clocks.
> > Anyway, please add a description to the binding what this property actually
> > does.
> I will switch to use the common bindings.

There were cases when a property first started with a "soc," prefix and
later when people realized that it could be useful for other
drivers/SoCs as well, the prefix was removed. With that in mind I
would expect that a "fsl,protected-clocks" property behaves the same
as a "protected-clocks" property without the prefix.

If it doesn't please pick a different name. I didn't want to suggest
to just drop the "fsl," prefix and to use the generic property name
when the properties have a different meaning.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.