2022-09-13 09:33:23

by Peng Fan (OSS)

[permalink] [raw]
Subject: [PATCH V3 0/2] clk: imx: introduce fsl,always-on-clocks

From: Peng Fan <[email protected]>

V3:
Rename to fsl,always-on-clocks

V2:
Use protected-clocks
https://lore.kernel.org/all/[email protected]/

V1:
Use fsl,protected-clocks
https://lore.kernel.org/all/[email protected]/

There are two cases that I wanna this property could serve:
Virtualization: root cell linux run in parallel with inmate cell
AMP: M7/4 runs in parallel with A53

The major case is:
Jailhouse hypervisor only support partition, so devices are partitioned.
But there is only CCM module that provides clock, the CCM is handled by
root cell linux, need make sure the root cell linux not shutdown the
clocks using by inmate cell.

I was thinking whether need to provide a rate entry to ask root cell
configure the clk rate for inmate cell. But NXP downstream not have it,
see https://source.codeaurora.org/external/imx/linux-imx/tree/drivers/clk/imx/clk-imx8mp.c?h=lf-5.15.y#n690
So just leave the property as uint32-array.

This patchset could also benifit AMP case, check the two commits
commit 19565ea12d61 ("clk: imx: add mcore_booted module paratemter")
commit d097cc045b64 ("clk: imx8mp: remove SYS PLL 1/2 clock gates")
Although I not plan to drop the upper two patches, this patchset
exposes opportunity for better power consumption.

Peng Fan (2):
dt-bindings: clock: imx8m/imx93: introduce fsl,always-on-clocks
property
clk: imx: support fsl,always-on-clocks

.../bindings/clock/imx8m-clock.yaml | 4 ++++
.../bindings/clock/imx93-clock.yaml | 4 ++++
drivers/clk/imx/clk-imx8mm.c | 2 ++
drivers/clk/imx/clk-imx8mn.c | 2 ++
drivers/clk/imx/clk-imx8mp.c | 2 ++
drivers/clk/imx/clk-imx8mq.c | 2 ++
drivers/clk/imx/clk.c | 21 +++++++++++++++++++
drivers/clk/imx/clk.h | 2 ++
8 files changed, 39 insertions(+)

--
2.37.1


2022-09-13 09:33:48

by Peng Fan (OSS)

[permalink] [raw]
Subject: [PATCH V3 1/2] dt-bindings: clock: imx8m/imx93: introduce fsl,always-on-clocks property

From: Peng Fan <[email protected]>

i.MX8M Linux run on top of Jailhouse hypervisor, the root cell Linux
should not disable clocks used by inmate.

So introduce fsl,always-on-clocks for above case.

Signed-off-by: Peng Fan <[email protected]>
---
Documentation/devicetree/bindings/clock/imx8m-clock.yaml | 4 ++++
Documentation/devicetree/bindings/clock/imx93-clock.yaml | 4 ++++
2 files changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/imx8m-clock.yaml b/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
index 458c7645ee68..4b481737895d 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,always-on-clocks:
+ description: List of the always-on-clocks.
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+
required:
- compatible
- reg
diff --git a/Documentation/devicetree/bindings/clock/imx93-clock.yaml b/Documentation/devicetree/bindings/clock/imx93-clock.yaml
index 21a06194e4a3..2b35e8af1438 100644
--- a/Documentation/devicetree/bindings/clock/imx93-clock.yaml
+++ b/Documentation/devicetree/bindings/clock/imx93-clock.yaml
@@ -43,6 +43,10 @@ properties:
See include/dt-bindings/clock/imx93-clock.h for the full list of
i.MX93 clock IDs.

+ fsl,always-on-clocks:
+ description: List of the always-on-clocks.
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+
required:
- compatible
- reg
--
2.37.1

2022-09-13 09:34:01

by Peng Fan (OSS)

[permalink] [raw]
Subject: [PATCH V3 2/2] clk: imx: support fsl,always-on-clocks

From: Peng Fan <[email protected]>

For the clocks listed in fsl,always-on-clocks, enable them to avoid
Linux disable them. This will benifit root Linux and inmate cell run
on top of Jailhouse hypervisor.

Signed-off-by: Peng Fan <[email protected]>
---
drivers/clk/imx/clk-imx8mm.c | 2 ++
drivers/clk/imx/clk-imx8mn.c | 2 ++
drivers/clk/imx/clk-imx8mp.c | 2 ++
drivers/clk/imx/clk-imx8mq.c | 2 ++
drivers/clk/imx/clk.c | 21 +++++++++++++++++++++
drivers/clk/imx/clk.h | 2 ++
6 files changed, 31 insertions(+)

diff --git a/drivers/clk/imx/clk-imx8mm.c b/drivers/clk/imx/clk-imx8mm.c
index aa58b2995610..d1e4d0d3c41b 100644
--- a/drivers/clk/imx/clk-imx8mm.c
+++ b/drivers/clk/imx/clk-imx8mm.c
@@ -611,6 +611,8 @@ static int imx8mm_clocks_probe(struct platform_device *pdev)

imx_register_uart_clocks(4);

+ imx_clk_always_on(dev, hws);
+
return 0;

unregister_hws:
diff --git a/drivers/clk/imx/clk-imx8mn.c b/drivers/clk/imx/clk-imx8mn.c
index 79fabc0606b0..422b535013de 100644
--- a/drivers/clk/imx/clk-imx8mn.c
+++ b/drivers/clk/imx/clk-imx8mn.c
@@ -605,6 +605,8 @@ static int imx8mn_clocks_probe(struct platform_device *pdev)

imx_register_uart_clocks(4);

+ imx_clk_always_on(dev, hws);
+
return 0;

unregister_hws:
diff --git a/drivers/clk/imx/clk-imx8mp.c b/drivers/clk/imx/clk-imx8mp.c
index 4297af85ab09..ff043fa0299d 100644
--- a/drivers/clk/imx/clk-imx8mp.c
+++ b/drivers/clk/imx/clk-imx8mp.c
@@ -714,6 +714,8 @@ static int imx8mp_clocks_probe(struct platform_device *pdev)

imx_register_uart_clocks(4);

+ imx_clk_always_on(dev, hws);
+
return 0;
}

diff --git a/drivers/clk/imx/clk-imx8mq.c b/drivers/clk/imx/clk-imx8mq.c
index 882dcad4817d..6eec3bbcde6f 100644
--- a/drivers/clk/imx/clk-imx8mq.c
+++ b/drivers/clk/imx/clk-imx8mq.c
@@ -603,6 +603,8 @@ static int imx8mq_clocks_probe(struct platform_device *pdev)

imx_register_uart_clocks(4);

+ imx_clk_always_on(dev, hws);
+
return 0;

unregister_hws:
diff --git a/drivers/clk/imx/clk.c b/drivers/clk/imx/clk.c
index 5582f18dd632..75876c81882c 100644
--- a/drivers/clk/imx/clk.c
+++ b/drivers/clk/imx/clk.c
@@ -2,6 +2,7 @@
#include <linux/bits.h>
#include <linux/clk.h>
#include <linux/clk-provider.h>
+#include <linux/device.h>
#include <linux/err.h>
#include <linux/io.h>
#include <linux/module.h>
@@ -214,4 +215,24 @@ static int __init imx_clk_disable_uart(void)
late_initcall_sync(imx_clk_disable_uart);
#endif

+int imx_clk_always_on(struct device *dev, struct clk_hw * const clks[])
+{
+ struct device_node *np = dev->of_node;
+ struct property *prop;
+ const __be32 *p;
+ u32 i;
+ int ret;
+
+ of_property_for_each_u32(np, "fsl,always-on-clocks", prop, p, i) {
+ ret = clk_prepare_enable(clks[i]->clk);
+ if (ret) {
+ dev_err(dev, "failed to enable %s\n", clk_hw_get_name(clks[i]));
+ return ret;
+ }
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(imx_clk_always_on);
+
MODULE_LICENSE("GPL v2");
diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
index dd49f90110e8..591d72ea4d31 100644
--- a/drivers/clk/imx/clk.h
+++ b/drivers/clk/imx/clk.h
@@ -22,6 +22,8 @@ void imx_mmdc_mask_handshake(void __iomem *ccm_base, unsigned int chn);
void imx_unregister_clocks(struct clk *clks[], unsigned int count);
void imx_unregister_hw_clocks(struct clk_hw *hws[], unsigned int count);

+int imx_clk_always_on(struct device *dev, struct clk_hw * const clks[]);
+
extern void imx_cscmr1_fixup(u32 *val);

enum imx_pllv1_type {
--
2.37.1

2022-09-13 09:49:14

by Peng Fan

[permalink] [raw]
Subject: RE: [PATCH V3 1/2] dt-bindings: clock: imx8m/imx93: introduce fsl,always-on-clocks property

Missed to cc: [email protected]

> Subject: [PATCH V3 1/2] dt-bindings: clock: imx8m/imx93: introduce
> fsl,always-on-clocks property
>
> From: Peng Fan <[email protected]>
>
> i.MX8M Linux run on top of Jailhouse hypervisor, the root cell Linux should
> not disable clocks used by inmate.
>
> So introduce fsl,always-on-clocks for above case.
>
> Signed-off-by: Peng Fan <[email protected]>
> ---
> Documentation/devicetree/bindings/clock/imx8m-clock.yaml | 4 ++++
> Documentation/devicetree/bindings/clock/imx93-clock.yaml | 4 ++++
> 2 files changed, 8 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
> b/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
> index 458c7645ee68..4b481737895d 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,always-on-clocks:
> + description: List of the always-on-clocks.
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> +
> required:
> - compatible
> - reg
> diff --git a/Documentation/devicetree/bindings/clock/imx93-clock.yaml
> b/Documentation/devicetree/bindings/clock/imx93-clock.yaml
> index 21a06194e4a3..2b35e8af1438 100644
> --- a/Documentation/devicetree/bindings/clock/imx93-clock.yaml
> +++ b/Documentation/devicetree/bindings/clock/imx93-clock.yaml
> @@ -43,6 +43,10 @@ properties:
> See include/dt-bindings/clock/imx93-clock.h for the full list of
> i.MX93 clock IDs.
>
> + fsl,always-on-clocks:
> + description: List of the always-on-clocks.
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> +
> required:
> - compatible
> - reg
> --
> 2.37.1

2022-09-13 10:22:20

by Alexander Stein

[permalink] [raw]
Subject: Re: [PATCH V3 0/2] clk: imx: introduce fsl,always-on-clocks

Hello,

Am Dienstag, 13. September 2022, 11:21:34 CEST schrieb Peng Fan (OSS):
> From: Peng Fan <[email protected]>
>
> V3:
> Rename to fsl,always-on-clocks
>
> V2:
> Use protected-clocks
> https://lore.kernel.org/all/[email protected]/
>
> V1:
> Use fsl,protected-clocks
> https://lore.kernel.org/all/[email protected]/
>
> There are two cases that I wanna this property could serve:
> Virtualization: root cell linux run in parallel with inmate cell
> AMP: M7/4 runs in parallel with A53
>
> The major case is:
> Jailhouse hypervisor only support partition, so devices are partitioned.
> But there is only CCM module that provides clock, the CCM is handled by
> root cell linux, need make sure the root cell linux not shutdown the
> clocks using by inmate cell.
>
> I was thinking whether need to provide a rate entry to ask root cell
> configure the clk rate for inmate cell. But NXP downstream not have it,
> see
> https://source.codeaurora.org/external/imx/linux-imx/tree/drivers/clk/imx/c
> lk-imx8mp.c?h=lf-5.15.y#n690 So just leave the property as uint32-array.
>
> This patchset could also benifit AMP case, check the two commits
> commit 19565ea12d61 ("clk: imx: add mcore_booted module paratemter")
> commit d097cc045b64 ("clk: imx8mp: remove SYS PLL 1/2 clock gates")
> Although I not plan to drop the upper two patches, this patchset
> exposes opportunity for better power consumption.

Has this the same intention/effect as 'critical-clocks' from [1]? I guess in
both cases you want to keep clocks enabled, even though there is no (local)
user.

Best regards,
Alexander

[1] https://lore.kernel.org/linux-clk/[email protected]/

> Peng Fan (2):
> dt-bindings: clock: imx8m/imx93: introduce fsl,always-on-clocks
> property
> clk: imx: support fsl,always-on-clocks
>
> .../bindings/clock/imx8m-clock.yaml | 4 ++++
> .../bindings/clock/imx93-clock.yaml | 4 ++++
> drivers/clk/imx/clk-imx8mm.c | 2 ++
> drivers/clk/imx/clk-imx8mn.c | 2 ++
> drivers/clk/imx/clk-imx8mp.c | 2 ++
> drivers/clk/imx/clk-imx8mq.c | 2 ++
> drivers/clk/imx/clk.c | 21 +++++++++++++++++++
> drivers/clk/imx/clk.h | 2 ++
> 8 files changed, 39 insertions(+)




2022-09-13 11:05:06

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH V3 0/2] clk: imx: introduce fsl,always-on-clocks

On 13/09/2022 12:29, Marco Felsch wrote:
> Hi Peng,
>
> On 22-09-13, Peng Fan (OSS) wrote:
>> From: Peng Fan <[email protected]>
>>
>> V3:
>> Rename to fsl,always-on-clocks
>>
>> V2:
>> Use protected-clocks
>> https://lore.kernel.org/all/[email protected]/
>>
>> V1:
>> Use fsl,protected-clocks
>> https://lore.kernel.org/all/[email protected]/
>>
>> There are two cases that I wanna this property could serve:
>> Virtualization: root cell linux run in parallel with inmate cell
>> AMP: M7/4 runs in parallel with A53
>>
>> The major case is:
>> Jailhouse hypervisor only support partition, so devices are partitioned.
>> But there is only CCM module that provides clock, the CCM is handled by
>> root cell linux, need make sure the root cell linux not shutdown the
>> clocks using by inmate cell.
>>
>> I was thinking whether need to provide a rate entry to ask root cell
>> configure the clk rate for inmate cell. But NXP downstream not have it,
>> see https://source.codeaurora.org/external/imx/linux-imx/tree/drivers/clk/imx/clk-imx8mp.c?h=lf-5.15.y#n690
>> So just leave the property as uint32-array.
>
> Can you please check my recent proposal? I recently stumbled over such
> an issue on the mx8mm-evk as well but with the 32k clock provided by the
> pmic. Unfortunately I forgot to add you to the to list, I will change
> that. If that proposal will fix your problem, I would like to go the
> generic way.
>
> https://patchwork.kernel.org/project/linux-clk/list/?series=676522

Your proposal does not change bindings. You cannot introduce new
properties without documenting them in the bindings.

Unless you documented them somewhere, but this is no reflected in
Patchwork at all.


Best regards,
Krzysztof

2022-09-13 11:05:41

by Marco Felsch

[permalink] [raw]
Subject: Re: [PATCH V3 0/2] clk: imx: introduce fsl,always-on-clocks

Hi Peng,

On 22-09-13, Peng Fan (OSS) wrote:
> From: Peng Fan <[email protected]>
>
> V3:
> Rename to fsl,always-on-clocks
>
> V2:
> Use protected-clocks
> https://lore.kernel.org/all/[email protected]/
>
> V1:
> Use fsl,protected-clocks
> https://lore.kernel.org/all/[email protected]/
>
> There are two cases that I wanna this property could serve:
> Virtualization: root cell linux run in parallel with inmate cell
> AMP: M7/4 runs in parallel with A53
>
> The major case is:
> Jailhouse hypervisor only support partition, so devices are partitioned.
> But there is only CCM module that provides clock, the CCM is handled by
> root cell linux, need make sure the root cell linux not shutdown the
> clocks using by inmate cell.
>
> I was thinking whether need to provide a rate entry to ask root cell
> configure the clk rate for inmate cell. But NXP downstream not have it,
> see https://source.codeaurora.org/external/imx/linux-imx/tree/drivers/clk/imx/clk-imx8mp.c?h=lf-5.15.y#n690
> So just leave the property as uint32-array.

Can you please check my recent proposal? I recently stumbled over such
an issue on the mx8mm-evk as well but with the 32k clock provided by the
pmic. Unfortunately I forgot to add you to the to list, I will change
that. If that proposal will fix your problem, I would like to go the
generic way.

https://patchwork.kernel.org/project/linux-clk/list/?series=676522

Regards,
Marco


>
> This patchset could also benifit AMP case, check the two commits
> commit 19565ea12d61 ("clk: imx: add mcore_booted module paratemter")
> commit d097cc045b64 ("clk: imx8mp: remove SYS PLL 1/2 clock gates")
> Although I not plan to drop the upper two patches, this patchset
> exposes opportunity for better power consumption.
>
> Peng Fan (2):
> dt-bindings: clock: imx8m/imx93: introduce fsl,always-on-clocks
> property
> clk: imx: support fsl,always-on-clocks
>
> .../bindings/clock/imx8m-clock.yaml | 4 ++++
> .../bindings/clock/imx93-clock.yaml | 4 ++++
> drivers/clk/imx/clk-imx8mm.c | 2 ++
> drivers/clk/imx/clk-imx8mn.c | 2 ++
> drivers/clk/imx/clk-imx8mp.c | 2 ++
> drivers/clk/imx/clk-imx8mq.c | 2 ++
> drivers/clk/imx/clk.c | 21 +++++++++++++++++++
> drivers/clk/imx/clk.h | 2 ++
> 8 files changed, 39 insertions(+)
>
> --
> 2.37.1
>
>
>

2022-09-13 11:10:43

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH V3 1/2] dt-bindings: clock: imx8m/imx93: introduce fsl,always-on-clocks property

On 13/09/2022 11:29, Peng Fan wrote:
> Missed to cc: [email protected]

This does not work.

Please resend.

Best regards,
Krzysztof

2022-09-13 12:16:02

by Marco Felsch

[permalink] [raw]
Subject: Re: [PATCH V3 0/2] clk: imx: introduce fsl,always-on-clocks

On 22-09-13, Krzysztof Kozlowski wrote:
> On 13/09/2022 12:29, Marco Felsch wrote:
> > Hi Peng,
> >
> > On 22-09-13, Peng Fan (OSS) wrote:
> >> From: Peng Fan <[email protected]>
> >>
> >> V3:
> >> Rename to fsl,always-on-clocks
> >>
> >> V2:
> >> Use protected-clocks
> >> https://lore.kernel.org/all/[email protected]/
> >>
> >> V1:
> >> Use fsl,protected-clocks
> >> https://lore.kernel.org/all/[email protected]/
> >>
> >> There are two cases that I wanna this property could serve:
> >> Virtualization: root cell linux run in parallel with inmate cell
> >> AMP: M7/4 runs in parallel with A53
> >>
> >> The major case is:
> >> Jailhouse hypervisor only support partition, so devices are partitioned.
> >> But there is only CCM module that provides clock, the CCM is handled by
> >> root cell linux, need make sure the root cell linux not shutdown the
> >> clocks using by inmate cell.
> >>
> >> I was thinking whether need to provide a rate entry to ask root cell
> >> configure the clk rate for inmate cell. But NXP downstream not have it,
> >> see https://source.codeaurora.org/external/imx/linux-imx/tree/drivers/clk/imx/clk-imx8mp.c?h=lf-5.15.y#n690
> >> So just leave the property as uint32-array.
> >
> > Can you please check my recent proposal? I recently stumbled over such
> > an issue on the mx8mm-evk as well but with the 32k clock provided by the
> > pmic. Unfortunately I forgot to add you to the to list, I will change
> > that. If that proposal will fix your problem, I would like to go the
> > generic way.
> >
> > https://patchwork.kernel.org/project/linux-clk/list/?series=676522
>
> Your proposal does not change bindings. You cannot introduce new
> properties without documenting them in the bindings.

As said, it is a proposal. Bindings will be added if it would be
accepted.

Regards,
Marco

2022-09-13 12:48:29

by Peng Fan (OSS)

[permalink] [raw]
Subject: Re: [PATCH V3 0/2] clk: imx: introduce fsl,always-on-clocks



On 9/13/2022 6:29 PM, Marco Felsch wrote:
> Hi Peng,
>
> On 22-09-13, Peng Fan (OSS) wrote:
>> From: Peng Fan <[email protected]>
>>
>> V3:
>> Rename to fsl,always-on-clocks
>>
>> V2:
>> Use protected-clocks
>> https://lore.kernel.org/all/[email protected]/
>>
>> V1:
>> Use fsl,protected-clocks
>> https://lore.kernel.org/all/[email protected]/
>>
>> There are two cases that I wanna this property could serve:
>> Virtualization: root cell linux run in parallel with inmate cell
>> AMP: M7/4 runs in parallel with A53
>>
>> The major case is:
>> Jailhouse hypervisor only support partition, so devices are partitioned.
>> But there is only CCM module that provides clock, the CCM is handled by
>> root cell linux, need make sure the root cell linux not shutdown the
>> clocks using by inmate cell.
>>
>> I was thinking whether need to provide a rate entry to ask root cell
>> configure the clk rate for inmate cell. But NXP downstream not have it,
>> see https://source.codeaurora.org/external/imx/linux-imx/tree/drivers/clk/imx/clk-imx8mp.c?h=lf-5.15.y#n690
>> So just leave the property as uint32-array.
>
> Can you please check my recent proposal? I recently stumbled over such
> an issue on the mx8mm-evk as well but with the 32k clock provided by the
> pmic. Unfortunately I forgot to add you to the to list, I will change
> that. If that proposal will fix your problem, I would like to go the
> generic way.

I vote for a generic solution.

Thanks,
Peng.

>
> https://patchwork.kernel.org/project/linux-clk/list/?series=676522
>
> Regards,
> Marco
>
>
>>
>> This patchset could also benifit AMP case, check the two commits
>> commit 19565ea12d61 ("clk: imx: add mcore_booted module paratemter")
>> commit d097cc045b64 ("clk: imx8mp: remove SYS PLL 1/2 clock gates")
>> Although I not plan to drop the upper two patches, this patchset
>> exposes opportunity for better power consumption.
>>
>> Peng Fan (2):
>> dt-bindings: clock: imx8m/imx93: introduce fsl,always-on-clocks
>> property
>> clk: imx: support fsl,always-on-clocks
>>
>> .../bindings/clock/imx8m-clock.yaml | 4 ++++
>> .../bindings/clock/imx93-clock.yaml | 4 ++++
>> drivers/clk/imx/clk-imx8mm.c | 2 ++
>> drivers/clk/imx/clk-imx8mn.c | 2 ++
>> drivers/clk/imx/clk-imx8mp.c | 2 ++
>> drivers/clk/imx/clk-imx8mq.c | 2 ++
>> drivers/clk/imx/clk.c | 21 +++++++++++++++++++
>> drivers/clk/imx/clk.h | 2 ++
>> 8 files changed, 39 insertions(+)
>>
>> --
>> 2.37.1
>>
>>
>>