RE: [PATCH 1/3] opp: of: Support multiple suspend OPPs defined in DT

2019-07-09 Thread Anson Huang
Hi, Viresh

> On 09-07-19, 15:10, anson.hu...@nxp.com wrote:
> > From: Anson Huang 
> >
> > With property "opp-supported-hw" introduced, the OPP table in DT could
> > be a large OPP table and ONLY a subset of OPPs are available, based on
> > the version of the hardware running on. That introduces restriction of
> > using "opp-suspend"
> > property to define the suspend OPP, as we are NOT sure if the OPP
> > containing "opp-suspend" property is available for the hardware
> > running on, and the of opp core does NOT allow multiple suspend OPPs
> > defined in DT OPP table.
> >
> > To eliminate this restrition, make of opp core allow multiple suspend
> > OPPs defined in DT, and pick the OPP with highest rate and with
> > "opp-suspend" property present to be suspend OPP, it can speed up the
> > suspend/resume process.
> >
> > Signed-off-by: Anson Huang 
> > ---
> >  drivers/opp/of.c | 9 ++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> Please update the DT bindings first.

OK, will send a V2 containing DT binding changes.

> 
> FWIW, all three patches look fine otherwise.

Thank you!

Anson.


RE: [PATCH] cpufreq: imx-cpufreq-dt: Assign max supported frequency as suspend frequency

2019-07-09 Thread Anson Huang

> > On 7/8/2019 10:55 AM, anson.hu...@nxp.com wrote:
> > > To reduce the suspend/resume latency, CPU's max supported frequency
> > > should be used during low level suspend/resume phase, "opp-suspend"
> > > property is NOT feasible since OPP defined in DT could be NOT
> > > supported according to speed garding and market segment fuse settings.
> > > So we can assign the cpufreq policy's suspend_freq with max
> > > available frequency provided by cpufreq driver.
> > >
> > > Signed-off-by: Anson Huang 
> >
> > > diff --git a/drivers/cpufreq/imx-cpufreq-dt.c
> > > b/drivers/cpufreq/imx-cpufreq-dt.c
> >
> > > +static int __init imx_cpufreq_dt_setup_suspend_opp(void)
> > > +{
> > > + struct cpufreq_policy *policy = cpufreq_cpu_get(0);
> > > +
> > > + policy->suspend_freq = cpufreq_quick_get_max(0);
> > > +
> > > + return 0;
> > > +}
> > > +late_initcall(imx_cpufreq_dt_setup_suspend_opp);
> >
> > The imx-cpufreq-dt driver is built as a module by default and this
> > patch produces an error:
> >
> > In file included from ../drivers/cpufreq/imx-cpufreq-dt.c:11:
> > ../include/linux/module.h:131:42: error: redefinition of ‘__inittest’
> >static inline initcall_t __maybe_unused __inittest(void)  \
> >^~
> > ../include/linux/device.h:1656:1: note: in expansion of macro ‘module_init’
> >   module_init(__driver##_init); \
> >   ^~~
> >
> > As far as I can tell late_initcall is not supported for modules.
> 
> Ah, yes, I have NOT test the module build, I ONLY tested the built-in case,
> thanks for reminder.
> 
> >
> > Viresh: "max freq as suspend freq" is something that could be useful
> > for other SOC families. The hardware can suspend at any freq; it's
> > just that the highest one makes sense because it makes suspend/resume
> slightly faster.
> 
> Agree.
> 
> >
> > Could this behavior be pushed to cpufreq-dt as a bool flag inside
> > struct cpufreq_dt_platform_data?
> >
> > Only a few other platforms use this, most others pass NULL like imx.
> > But passing custom SOC-specific flags to cpufreq-dt makes a lot of
> > sense
> 
> Although we have other methods to make it work for i.MX platforms, like
> setting the suspend freq as this patch did but ONLY be called before suspend,
> but if common cpufreq-dt can handle this case, that would benefit us and
> other platforms a lot, waiting for your opinion.

Please ignore this patch, based on the discussion, I have sent out a new patch
series to support this feature. Please review them:
https://patchwork.kernel.org/patch/11036505/

Thanks,
Anson 



[PATCH 1/3] opp: of: Support multiple suspend OPPs defined in DT

2019-07-09 Thread Anson . Huang
From: Anson Huang 

With property "opp-supported-hw" introduced, the OPP table
in DT could be a large OPP table and ONLY a subset of OPPs
are available, based on the version of the hardware running
on. That introduces restriction of using "opp-suspend"
property to define the suspend OPP, as we are NOT sure if the
OPP containing "opp-suspend" property is available for the
hardware running on, and the of opp core does NOT allow multiple
suspend OPPs defined in DT OPP table.

To eliminate this restrition, make of opp core allow multiple
suspend OPPs defined in DT, and pick the OPP with highest rate
and with "opp-suspend" property present to be suspend OPP, it
can speed up the suspend/resume process.

Signed-off-by: Anson Huang 
---
 drivers/opp/of.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/opp/of.c b/drivers/opp/of.c
index b313aca..7e8ec6c 100644
--- a/drivers/opp/of.c
+++ b/drivers/opp/of.c
@@ -617,9 +617,12 @@ static struct dev_pm_opp *_opp_add_static_v2(struct 
opp_table *opp_table,
/* OPP to select on device suspend */
if (of_property_read_bool(np, "opp-suspend")) {
if (opp_table->suspend_opp) {
-   dev_warn(dev, "%s: Multiple suspend OPPs found (%lu 
%lu)\n",
-__func__, opp_table->suspend_opp->rate,
-new_opp->rate);
+   /* Pick the OPP with higher rate as suspend OPP */
+   if (new_opp->rate > opp_table->suspend_opp->rate) {
+   opp_table->suspend_opp->suspend = false;
+   new_opp->suspend = true;
+   opp_table->suspend_opp = new_opp;
+   }
} else {
new_opp->suspend = true;
opp_table->suspend_opp = new_opp;
-- 
2.7.4



[PATCH 3/3] arm64: dts: imx8mm: Add opp-suspend property to OPP table

2019-07-09 Thread Anson . Huang
From: Anson Huang 

Add opp-suspend property to each OPP, the of opp core will
select the OPP HW supported and with highest rate to be
suspend opp, it will speed up the suspend/resume process.

Signed-off-by: Anson Huang 
---
 arch/arm64/boot/dts/freescale/imx8mm.dtsi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi 
b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index 398318b..973f457 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -108,6 +108,7 @@
opp-microvolt = <85>;
opp-supported-hw = <0xe>, <0x7>;
clock-latency-ns = <15>;
+   opp-suspend;
};
 
opp-16 {
@@ -115,6 +116,7 @@
opp-microvolt = <90>;
opp-supported-hw = <0xc>, <0x7>;
clock-latency-ns = <15>;
+   opp-suspend;
};
 
opp-18 {
@@ -122,6 +124,7 @@
opp-microvolt = <100>;
opp-supported-hw = <0x8>, <0x3>;
clock-latency-ns = <15>;
+   opp-suspend;
};
};
 
-- 
2.7.4



[PATCH 2/3] arm64: dts: imx8mq: Add opp-suspend property to OPP table

2019-07-09 Thread Anson . Huang
From: Anson Huang 

Add opp-suspend property to each OPP, the of opp core will
select the OPP HW supported and with highest rate to be
suspend opp, it will speed up the suspend/resume process.

Signed-off-by: Anson Huang 
---
 arch/arm64/boot/dts/freescale/imx8mq.dtsi | 4 
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi 
b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index 58f66cb..4ba6a25f 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -156,6 +156,7 @@
/* Industrial only */
opp-supported-hw = <0xf>, <0x4>;
clock-latency-ns = <15>;
+   opp-suspend;
};
 
opp-10 {
@@ -164,6 +165,7 @@
/* Consumer only */
opp-supported-hw = <0xe>, <0x3>;
clock-latency-ns = <15>;
+   opp-suspend;
};
 
opp-13 {
@@ -171,6 +173,7 @@
opp-microvolt = <100>;
opp-supported-hw = <0xc>, <0x4>;
clock-latency-ns = <15>;
+   opp-suspend;
};
 
opp-15 {
@@ -178,6 +181,7 @@
opp-microvolt = <100>;
opp-supported-hw = <0x8>, <0x3>;
clock-latency-ns = <15>;
+   opp-suspend;
};
};
 
-- 
2.7.4



RE: [PATCH 2/2] arm64: dts: imx8mm: Assign highest opp as suspend opp

2019-07-08 Thread Anson Huang
Hi, Viresh

> On 08-07-19, 08:54, Anson Huang wrote:
> > Each OPP has "opp-supported-hw" property as below, the first value
> > needs to be checked with speed grading fuse, and the second one needs
> > to be checked with market segment fuse, ONLY both of them passed, then
> > this OPP is supported. It calls dev_pm_opp_set_supported_hw() to tell
> > OPP framework to parse the OPP table, this is my understanding.
> >
> > opp-supported-hw = <0x8>, <0x3>;
> 
> Right, so that's what I was expecting.
> 
> One thing we can do is change the binding of OPP core a bit to allow multiple
> OPP nodes to contain the "opp-suspend" property and select the one finally
> with the highest frequency. That would be a better as a generic solution IMO.
> 
> And then a small OPP core patch will fix it.

Looks good, I will try to generate a patch for of OPP core.

Thanks,
Anson


RE: [PATCH] cpufreq: imx-cpufreq-dt: Assign max supported frequency as suspend frequency

2019-07-08 Thread Anson Huang
Hi, Leonard

> On 7/8/2019 10:55 AM, anson.hu...@nxp.com wrote:
> > To reduce the suspend/resume latency, CPU's max supported frequency
> > should be used during low level suspend/resume phase, "opp-suspend"
> > property is NOT feasible since OPP defined in DT could be NOT
> > supported according to speed garding and market segment fuse settings.
> > So we can assign the cpufreq policy's suspend_freq with max available
> > frequency provided by cpufreq driver.
> >
> > Signed-off-by: Anson Huang 
> 
> > diff --git a/drivers/cpufreq/imx-cpufreq-dt.c
> > b/drivers/cpufreq/imx-cpufreq-dt.c
> 
> > +static int __init imx_cpufreq_dt_setup_suspend_opp(void)
> > +{
> > +   struct cpufreq_policy *policy = cpufreq_cpu_get(0);
> > +
> > +   policy->suspend_freq = cpufreq_quick_get_max(0);
> > +
> > +   return 0;
> > +}
> > +late_initcall(imx_cpufreq_dt_setup_suspend_opp);
> 
> The imx-cpufreq-dt driver is built as a module by default and this patch
> produces an error:
> 
> In file included from ../drivers/cpufreq/imx-cpufreq-dt.c:11:
> ../include/linux/module.h:131:42: error: redefinition of ‘__inittest’
>static inline initcall_t __maybe_unused __inittest(void)  \
>^~
> ../include/linux/device.h:1656:1: note: in expansion of macro ‘module_init’
>   module_init(__driver##_init); \
>   ^~~
> 
> As far as I can tell late_initcall is not supported for modules.

Ah, yes, I have NOT test the module build, I ONLY tested the built-in case, 
thanks for
reminder.

> 
> Viresh: "max freq as suspend freq" is something that could be useful for
> other SOC families. The hardware can suspend at any freq; it's just that the
> highest one makes sense because it makes suspend/resume slightly faster.

Agree.

> 
> Could this behavior be pushed to cpufreq-dt as a bool flag inside struct
> cpufreq_dt_platform_data?
> 
> Only a few other platforms use this, most others pass NULL like imx. But
> passing custom SOC-specific flags to cpufreq-dt makes a lot of sense

Although we have other methods to make it work for i.MX platforms, like setting
the suspend freq as this patch did but ONLY be called before suspend, but if 
common
cpufreq-dt can handle this case, that would benefit us and other platforms a 
lot, waiting
for your opinion.

Thanks,
Anson



RE: [PATCH 2/2] arm64: dts: imx8mm: Assign highest opp as suspend opp

2019-07-08 Thread Anson Huang
Hi, Viresh

> On 08-07-19, 08:43, Anson Huang wrote:
> > Hi, Viresh
> >
> > > On 04-07-19, 07:49, Leonard Crestez wrote:
> > > > On 7/4/2019 9:23 AM, anson.hu...@nxp.com wrote:
> > > > > From: Anson Huang 
> > > > >
> > > > > Assign highest OPP as suspend OPP to reduce suspend/resume
> > > > > latency on i.MX8MM.
> > > > >
> > > > > Signed-off-by: Anson Huang 
> > > > > ---
> > > > >   arch/arm64/boot/dts/freescale/imx8mm.dtsi | 1 +
> > > > >   1 file changed, 1 insertion(+)
> > > > >
> > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > > > > b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > > > > index b11fc5e..3a62407 100644
> > > > > --- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > > > > +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > > > > @@ -136,6 +136,7 @@
> > > > >   opp-microvolt = <100>;
> > > > >   opp-supported-hw = <0x8>, <0x3>;
> > > > >   clock-latency-ns = <15>;
> > > > > + opp-suspend;
> > > > >   };
> > > > >   };
> > > >
> > > > What if the highest OPP is unavailable due to speed grading?
> > >
> > > What does this exactly mean ? How is the OPP made unavailable in
> > > your case ?
> >
> > That is because in i.MX8M series SoCs, the speed grading and market
> > segment fuses settings could affect the OPP defined in DT, in a word,
> > all possible OPPs are defined in DT, but each parts could ONLY select
> > some of them to be working OPPs, so if the "opp-suspend" is added for
> > 1 OPP in DT, if the part's speed grading or market segment fuse settings
> make that OPP as unavailable,  then that "opp-suspend"
> > is NOT working at all.
> 
> How is this selection done ? You using some OPP helper or something else ?

Each OPP has "opp-supported-hw" property as below, the first value needs to be
checked with speed grading fuse, and the second one needs to be checked with
market segment fuse, ONLY both of them passed, then this OPP is supported. It
calls dev_pm_opp_set_supported_hw() to tell OPP framework to parse the OPP
table, this is my understanding.

opp-supported-hw = <0x8>, <0x3>;

thanks,
Anson



RE: [PATCH 2/2] arm64: dts: imx8mm: Assign highest opp as suspend opp

2019-07-08 Thread Anson Huang
Hi, Viresh

> On 04-07-19, 07:49, Leonard Crestez wrote:
> > On 7/4/2019 9:23 AM, anson.hu...@nxp.com wrote:
> > > From: Anson Huang 
> > >
> > > Assign highest OPP as suspend OPP to reduce suspend/resume latency
> > > on i.MX8MM.
> > >
> > > Signed-off-by: Anson Huang 
> > > ---
> > >   arch/arm64/boot/dts/freescale/imx8mm.dtsi | 1 +
> > >   1 file changed, 1 insertion(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > > b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > > index b11fc5e..3a62407 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > > @@ -136,6 +136,7 @@
> > >   opp-microvolt = <100>;
> > >   opp-supported-hw = <0x8>, <0x3>;
> > >   clock-latency-ns = <15>;
> > > + opp-suspend;
> > >   };
> > >   };
> >
> > What if the highest OPP is unavailable due to speed grading?
> 
> What does this exactly mean ? How is the OPP made unavailable in your
> case ?

That is because in i.MX8M series SoCs, the speed grading and market segment
fuses settings could affect the OPP defined in DT, in a word, all possible OPPs
are defined in DT, but each parts could ONLY select some of them to be working
OPPs, so if the "opp-suspend" is added for 1 OPP in DT, if the part's speed 
grading or
market segment fuse settings make that OPP as unavailable,  then that 
"opp-suspend"
is NOT working at all.

> 
> What will dev_pm_opp_get_suspend_opp_freq() return in this case ?

If the OPP contains "opp-suspend" property is NOT supported by the HW, then 
there will
be no suspend OPP defined, so it will return 0. The _opp_is_supported() parses 
the opp-supported-hw
before opp-suspend.

> 
> > Ideally we
> > should find a way to suspend at the highest *supported* OPP.
> >
> > Maybe the opp-suspend marking could be assigned from imx-cpufreq-dt
> > driver code?

I ever tried that, go through the OPP table and check the fuse settings, then 
runtime add "opp-suspend"
to the opp table, but unfortunately, the " struct opp_table " is NOT opened to 
be used, it is a private
structure?

> 
> Sorry for jumping in late, the latest patch from Anson drew my attention to
> this topic :)

That is OK

Thanks,
Anson.



[PATCH] cpufreq: imx-cpufreq-dt: Assign max supported frequency as suspend frequency

2019-07-08 Thread Anson . Huang
From: Anson Huang 

To reduce the suspend/resume latency, CPU's max supported frequency
should be used during low level suspend/resume phase, "opp-suspend"
property is NOT feasible since OPP defined in DT could be NOT supported
according to speed garding and market segment fuse settings. So we
can assign the cpufreq policy's suspend_freq with max available
frequency provided by cpufreq driver.

Signed-off-by: Anson Huang 
---
 drivers/cpufreq/imx-cpufreq-dt.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/drivers/cpufreq/imx-cpufreq-dt.c b/drivers/cpufreq/imx-cpufreq-dt.c
index 4f85f31..b6607e8 100644
--- a/drivers/cpufreq/imx-cpufreq-dt.c
+++ b/drivers/cpufreq/imx-cpufreq-dt.c
@@ -4,6 +4,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -84,6 +85,16 @@ static int imx_cpufreq_dt_remove(struct platform_device 
*pdev)
return 0;
 }
 
+static int __init imx_cpufreq_dt_setup_suspend_opp(void)
+{
+   struct cpufreq_policy *policy = cpufreq_cpu_get(0);
+
+   policy->suspend_freq = cpufreq_quick_get_max(0);
+
+   return 0;
+}
+late_initcall(imx_cpufreq_dt_setup_suspend_opp);
+
 static struct platform_driver imx_cpufreq_dt_driver = {
.probe = imx_cpufreq_dt_probe,
.remove = imx_cpufreq_dt_remove,
-- 
2.7.4



RE: [PATCH 2/2] arm64: dts: imx8mm: Assign highest opp as suspend opp

2019-07-08 Thread Anson Huang
Hi, Leonard

> > On 7/4/2019 9:23 AM, anson.hu...@nxp.com wrote:
> > > From: Anson Huang 
> > >
> > > Assign highest OPP as suspend OPP to reduce suspend/resume latency
> > > on i.MX8MM.
> > >
> > > Signed-off-by: Anson Huang 
> > > ---
> > >   arch/arm64/boot/dts/freescale/imx8mm.dtsi | 1 +
> > >   1 file changed, 1 insertion(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > > b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > > index b11fc5e..3a62407 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > > @@ -136,6 +136,7 @@
> > >   opp-microvolt = <100>;
> > >   opp-supported-hw = <0x8>, <0x3>;
> > >   clock-latency-ns = <15>;
> > > + opp-suspend;
> > >   };
> > >   };
> >
> > What if the highest OPP is unavailable due to speed grading? Ideally
> > we should find a way to suspend at the highest *supported* OPP.
> >
> > Maybe the opp-suspend marking could be assigned from imx-cpufreq-dt
> > driver code?
> 
> Yes, this is also my concern, the current OPP driver does NOT handle it well,
> and I was thinking to assigne it from imx-cpufreq-dt driver, 1 option is to
> runtime add "suspend-opp" property into DT OPP node after parsing the
> speed grading fuse and OPP table, but I do NOT like that very much, as we
> need to manually create a property, the other option is to change cpu freq
> policy inside imx-cpufreq-dt driver in suspend/resume callback? Which one
> do you prefer?

After looking through the cpufreq driver, I think we can use below late init 
function
to assign the suspend_freq, then no need to add "opp-suspend " in DT, and the 
freq
get from cpufreq_quick_get_max() is already the max supported freq considering 
the
speed grading and market segment fuse settings, please ignore these 2 patches, 
I will
send out a imx-cpufreq-dt.c patch with below change to support all SoCs with 
imx-cpufreq-dt
driver.

+static int __init imx_cpufreq_dt_setup_suspend_opp(void)
+{
+   struct cpufreq_policy *policy = cpufreq_cpu_get(0);
+
+   policy->suspend_freq = cpufreq_quick_get_max(0);
+
+   return 0;
+}
+late_initcall(imx_cpufreq_dt_setup_suspend_opp);

Anson.



[PATCH] cpufreq: imx-cpufreq-dt: Add i.MX8MN support

2019-07-07 Thread Anson . Huang
From: Anson Huang 

i.MX8MN is a new SoC of i.MX8M series, it also uses speed
grading and market segment fuses for OPP definitions, add
support for this SoC.

Signed-off-by: Anson Huang 
---
 drivers/cpufreq/imx-cpufreq-dt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/imx-cpufreq-dt.c b/drivers/cpufreq/imx-cpufreq-dt.c
index b54fd26..4f85f31 100644
--- a/drivers/cpufreq/imx-cpufreq-dt.c
+++ b/drivers/cpufreq/imx-cpufreq-dt.c
@@ -44,10 +44,11 @@ static int imx_cpufreq_dt_probe(struct platform_device 
*pdev)
 * According to datasheet minimum speed grading is not supported for
 * consumer parts so clamp to 1 to avoid warning for "no OPPs"
 *
-* Applies to 8mq and 8mm.
+* Applies to i.MX8M series SoCs.
 */
if (mkt_segment == 0 && speed_grade == 0 && (
of_machine_is_compatible("fsl,imx8mm") ||
+   of_machine_is_compatible("fsl,imx8mn") ||
of_machine_is_compatible("fsl,imx8mq")))
speed_grade = 1;
 
-- 
2.7.4



[PATCH V4 2/2] arm64: dts: imx8mm: Add "fsl,imx8mq-src" as src's fallback compatible

2019-07-05 Thread Anson . Huang
From: Anson Huang 

i.MX8MM can reuse i.MX8MQ's src driver, add "fsl,imx8mq-src" as
src's fallback compatible to enable it.

Signed-off-by: Anson Huang 
Reviewed-by: Philipp Zabel 
---
No changes.
---
 arch/arm64/boot/dts/freescale/imx8mm.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi 
b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index 9db8efd..da5ee11 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -502,7 +502,7 @@
};
 
src: reset-controller@3039 {
-   compatible = "fsl,imx8mm-src", "syscon";
+   compatible = "fsl,imx8mm-src", 
"fsl,imx8mq-src", "syscon";
reg = <0x3039 0x1>;
interrupts = ;
#reset-cells = <1>;
-- 
2.7.4



[PATCH V4 1/2] dt-bindings: reset: imx7: Add support for i.MX8MM

2019-07-05 Thread Anson . Huang
From: Anson Huang 

i.MX8MM can reuse i.MX8MQ's reset driver, update the compatible
property and related info to support i.MX8MM.

Signed-off-by: Anson Huang 
---
Changes since V3:
- Add comments to those reset indices to indicate which are NOT 
supported on i.MX8MM.
---
 .../devicetree/bindings/reset/fsl,imx7-src.txt |  6 +++--
 include/dt-bindings/reset/imx8mq-reset.h   | 28 +++---
 2 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/Documentation/devicetree/bindings/reset/fsl,imx7-src.txt 
b/Documentation/devicetree/bindings/reset/fsl,imx7-src.txt
index 13e0951..c2489e4 100644
--- a/Documentation/devicetree/bindings/reset/fsl,imx7-src.txt
+++ b/Documentation/devicetree/bindings/reset/fsl,imx7-src.txt
@@ -8,6 +8,7 @@ Required properties:
 - compatible:
- For i.MX7 SoCs should be "fsl,imx7d-src", "syscon"
- For i.MX8MQ SoCs should be "fsl,imx8mq-src", "syscon"
+   - For i.MX8MM SoCs should be "fsl,imx8mm-src", "fsl,imx8mq-src", 
"syscon"
 - reg: should be register base and length as documented in the
   datasheet
 - interrupts: Should contain SRC interrupt
@@ -46,5 +47,6 @@ Example:
 
 
 For list of all valid reset indices see
- for i.MX7 and
- for i.MX8MQ
+ for i.MX7,
+ for i.MX8MQ and
+ for i.MX8MM
diff --git a/include/dt-bindings/reset/imx8mq-reset.h 
b/include/dt-bindings/reset/imx8mq-reset.h
index 57c5924..f17ef2a 100644
--- a/include/dt-bindings/reset/imx8mq-reset.h
+++ b/include/dt-bindings/reset/imx8mq-reset.h
@@ -38,26 +38,26 @@
 #define IMX8MQ_RESET_PCIEPHY_PERST 27
 #define IMX8MQ_RESET_PCIE_CTRL_APPS_EN 28
 #define IMX8MQ_RESET_PCIE_CTRL_APPS_TURNOFF29
-#define IMX8MQ_RESET_HDMI_PHY_APB_RESET30
+#define IMX8MQ_RESET_HDMI_PHY_APB_RESET30  /* i.MX8MM does 
NOT support */
 #define IMX8MQ_RESET_DISP_RESET31
 #define IMX8MQ_RESET_GPU_RESET 32
 #define IMX8MQ_RESET_VPU_RESET 33
-#define IMX8MQ_RESET_PCIEPHY2  34
-#define IMX8MQ_RESET_PCIEPHY2_PERST35
-#define IMX8MQ_RESET_PCIE2_CTRL_APPS_EN36
-#define IMX8MQ_RESET_PCIE2_CTRL_APPS_TURNOFF   37
-#define IMX8MQ_RESET_MIPI_CSI1_CORE_RESET  38
-#define IMX8MQ_RESET_MIPI_CSI1_PHY_REF_RESET   39
-#define IMX8MQ_RESET_MIPI_CSI1_ESC_RESET   40
-#define IMX8MQ_RESET_MIPI_CSI2_CORE_RESET  41
-#define IMX8MQ_RESET_MIPI_CSI2_PHY_REF_RESET   42
-#define IMX8MQ_RESET_MIPI_CSI2_ESC_RESET   43
+#define IMX8MQ_RESET_PCIEPHY2  34  /* i.MX8MM does NOT 
support */
+#define IMX8MQ_RESET_PCIEPHY2_PERST35  /* i.MX8MM does NOT 
support */
+#define IMX8MQ_RESET_PCIE2_CTRL_APPS_EN36  /* i.MX8MM does 
NOT support */
+#define IMX8MQ_RESET_PCIE2_CTRL_APPS_TURNOFF   37  /* i.MX8MM does NOT 
support */
+#define IMX8MQ_RESET_MIPI_CSI1_CORE_RESET  38  /* i.MX8MM does NOT 
support */
+#define IMX8MQ_RESET_MIPI_CSI1_PHY_REF_RESET   39  /* i.MX8MM does NOT 
support */
+#define IMX8MQ_RESET_MIPI_CSI1_ESC_RESET   40  /* i.MX8MM does NOT 
support */
+#define IMX8MQ_RESET_MIPI_CSI2_CORE_RESET  41  /* i.MX8MM does NOT 
support */
+#define IMX8MQ_RESET_MIPI_CSI2_PHY_REF_RESET   42  /* i.MX8MM does NOT 
support */
+#define IMX8MQ_RESET_MIPI_CSI2_ESC_RESET   43  /* i.MX8MM does NOT 
support */
 #define IMX8MQ_RESET_DDRC1_PRST44
 #define IMX8MQ_RESET_DDRC1_CORE_RESET  45
 #define IMX8MQ_RESET_DDRC1_PHY_RESET   46
-#define IMX8MQ_RESET_DDRC2_PRST47
-#define IMX8MQ_RESET_DDRC2_CORE_RESET  48
-#define IMX8MQ_RESET_DDRC2_PHY_RESET   49
+#define IMX8MQ_RESET_DDRC2_PRST47  /* i.MX8MM does 
NOT support */
+#define IMX8MQ_RESET_DDRC2_CORE_RESET  48  /* i.MX8MM does NOT 
support */
+#define IMX8MQ_RESET_DDRC2_PHY_RESET   49  /* i.MX8MM does NOT 
support */
 
 #define IMX8MQ_RESET_NUM   50
 
-- 
2.7.4



RE: [PATCH V3 1/2] dt-bindings: reset: imx7: Add support for i.MX8MM

2019-07-05 Thread Anson Huang
Hi, Philipp

> On Fri, 2019-07-05 at 00:26 +0000, Anson Huang wrote:
> > Hi, Philipp
> >
> > > On Thu, 2019-07-04 at 17:44 +0800, anson.hu...@nxp.com wrote:
> > > > From: Anson Huang 
> > > >
> > > > i.MX8MM can reuse i.MX8MQ's reset driver, update the compatible
> > > > property and related info to support i.MX8MM.
> > > >
> > > > Signed-off-by: Anson Huang 
> > > > ---
> > > > Changes since V2:
> > > > - Add separate line for i.MX8MM in case anything different later
> > > > for
> > >
> > > i.MX8MM.
> > > > ---
> > > >  Documentation/devicetree/bindings/reset/fsl,imx7-src.txt | 6
> > > > --
> > > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/reset/fsl,imx7-src.txt
> > > > b/Documentation/devicetree/bindings/reset/fsl,imx7-src.txt
> > > > index 13e0951..c2489e4 100644
> > > > --- a/Documentation/devicetree/bindings/reset/fsl,imx7-src.txt
> > > > +++ b/Documentation/devicetree/bindings/reset/fsl,imx7-src.txt
> > > > @@ -8,6 +8,7 @@ Required properties:
> > > >  - compatible:
> > > > - For i.MX7 SoCs should be "fsl,imx7d-src", "syscon"
> > > > - For i.MX8MQ SoCs should be "fsl,imx8mq-src", "syscon"
> > > > +   - For i.MX8MM SoCs should be "fsl,imx8mm-src", "fsl,imx8mq-src",
> > >
> > > "syscon"
> > > >  - reg: should be register base and length as documented in the
> > > >datasheet
> > > >  - interrupts: Should contain SRC interrupt @@ -46,5 +47,6 @@ Example:
> > > >
> > > >
> > > >  For list of all valid reset indices see
> > > > - for i.MX7 and
> > > > - for i.MX8MQ
> > > > + for i.MX7,
> > > > + for i.MX8MQ and
> > > > + for i.MX8MM
> > >
> > > The last line is misleading, as that file contains reset indices
> > > that are invalid for i.MX8MM.
> >
> > What is your suggestion about this line?
> 
> I would prefer to add an imx8mm-reset.h with only the existing reset bits,
> using the IMX8MM_RESET_ prefix. That would make it easy to spot errors in
> the dtsi (anything starting with IMX8MQ_ is potentially wrong).
> 
> > Just NOT change it?
> 
> The change is good in principle. It just should point to an imx8mm- reset.h 
> with
> only the existing resets on i.MX8MM, or imx8mq-reset.h should be modified to
> actually make clear which resets are valid on i.MX8MM.
> 
> > Or adding a new file imx8mm-reset.h but still use the IMX8MQ_RESET_ as
> > prefix ?
> 
> I don't think you should redefine the same macros in imx8mm-reset.h. In this
> case using IMX8MM_RESET_ would be better.
> 
> > Or keep what I changed, but adding some comments in those macros that
> > i.MX8MM does NOT support?
> 
> That would be acceptable as well.

I will go this way, thanks for suggestion.

Anson.

> 
> regards
> Philipp


[PATCH 1/6] thermal: qoriq: Use devm_platform_ioremap_resource() instead of of_iomap()

2019-07-04 Thread Anson . Huang
From: Anson Huang 

Use devm_platform_ioremap_resource() instead of of_iomap() to
save the iounmap() call in error handle path;

Signed-off-by: Anson Huang 
---
 drivers/thermal/qoriq_thermal.c | 18 ++
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 7b36493..c7c7de2 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -202,32 +202,27 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
 
data->little_endian = of_property_read_bool(np, "little-endian");
 
-   data->regs = of_iomap(np, 0);
-   if (!data->regs) {
+   data->regs = devm_platform_ioremap_resource(pdev, 0);
+   if (IS_ERR(data->regs)) {
dev_err(>dev, "Failed to get memory region\n");
-   ret = -ENODEV;
-   goto err_iomap;
+   return PTR_ERR(data->regs);
}
 
qoriq_tmu_init_device(data);/* TMU initialization */
 
ret = qoriq_tmu_calibration(pdev);  /* TMU calibration */
if (ret < 0)
-   goto err_tmu;
+   goto err;
 
ret = qoriq_tmu_register_tmu_zone(pdev);
if (ret < 0) {
dev_err(>dev, "Failed to register sensors\n");
-   ret = -ENODEV;
-   goto err_iomap;
+   goto err;
}
 
return 0;
 
-err_tmu:
-   iounmap(data->regs);
-
-err_iomap:
+err:
platform_set_drvdata(pdev, NULL);
 
return ret;
@@ -240,7 +235,6 @@ static int qoriq_tmu_remove(struct platform_device *pdev)
/* Disable monitoring */
tmu_write(data, TMR_DISABLE, >regs->tmr);
 
-   iounmap(data->regs);
platform_set_drvdata(pdev, NULL);
 
return 0;
-- 
2.7.4



[PATCH 2/6] thermal: qoriq: Use __maybe_unused instead of #if CONFIG_PM_SLEEP

2019-07-04 Thread Anson . Huang
From: Anson Huang 

Use __maybe_unused for power management related functions
instead of #if CONFIG_PM_SLEEP to simply the code.

Signed-off-by: Anson Huang 
---
 drivers/thermal/qoriq_thermal.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index c7c7de2..2b2f79b 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -240,8 +240,7 @@ static int qoriq_tmu_remove(struct platform_device *pdev)
return 0;
 }
 
-#ifdef CONFIG_PM_SLEEP
-static int qoriq_tmu_suspend(struct device *dev)
+static int __maybe_unused qoriq_tmu_suspend(struct device *dev)
 {
u32 tmr;
struct qoriq_tmu_data *data = dev_get_drvdata(dev);
@@ -254,7 +253,7 @@ static int qoriq_tmu_suspend(struct device *dev)
return 0;
 }
 
-static int qoriq_tmu_resume(struct device *dev)
+static int __maybe_unused qoriq_tmu_resume(struct device *dev)
 {
u32 tmr;
struct qoriq_tmu_data *data = dev_get_drvdata(dev);
@@ -266,7 +265,6 @@ static int qoriq_tmu_resume(struct device *dev)
 
return 0;
 }
-#endif
 
 static SIMPLE_DEV_PM_OPS(qoriq_tmu_pm_ops,
 qoriq_tmu_suspend, qoriq_tmu_resume);
-- 
2.7.4



[PATCH 6/6] arm64: dts: imx8mq: Add clock for TMU node

2019-07-04 Thread Anson . Huang
From: Anson Huang 

i.MX8MQ has clock gate for TMU module, add clock info to TMU
node for clock management.

Signed-off-by: Anson Huang 
---
 arch/arm64/boot/dts/freescale/imx8mq.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi 
b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index c61e968..edfc1aa 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -348,6 +348,7 @@
compatible = "fsl,imx8mq-tmu";
reg = <0x3026 0x1>;
interrupt = ;
+   clocks = < IMX8MQ_CLK_TMU_ROOT>;
little-endian;
fsl,tmu-range = <0xb 0xa0026 0x80048 
0x70061>;
fsl,tmu-calibration = <0x 0x0023
-- 
2.7.4



[PATCH 5/6] clk: imx8mq: Remove CLK_IS_CRITICAL flag for IMX8MQ_CLK_TMU_ROOT

2019-07-04 Thread Anson . Huang
From: Anson Huang 

IMX8MQ_CLK_TMU_ROOT is ONLY used for thermal module, the driver
should manage this clock, so no need to have CLK_IS_CRITICAL flag
set.

Signed-off-by: Anson Huang 
---
 drivers/clk/imx/clk-imx8mq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/imx/clk-imx8mq.c b/drivers/clk/imx/clk-imx8mq.c
index d407a07..91de69a 100644
--- a/drivers/clk/imx/clk-imx8mq.c
+++ b/drivers/clk/imx/clk-imx8mq.c
@@ -539,7 +539,7 @@ static int imx8mq_clocks_probe(struct platform_device *pdev)
clks[IMX8MQ_CLK_DISP_AXI_ROOT]  = 
imx_clk_gate2_shared2("disp_axi_root_clk", "disp_axi", base + 0x45d0, 0, 
_count_dcss);
clks[IMX8MQ_CLK_DISP_APB_ROOT]  = 
imx_clk_gate2_shared2("disp_apb_root_clk", "disp_apb", base + 0x45d0, 0, 
_count_dcss);
clks[IMX8MQ_CLK_DISP_RTRM_ROOT] = 
imx_clk_gate2_shared2("disp_rtrm_root_clk", "disp_rtrm", base + 0x45d0, 0, 
_count_dcss);
-   clks[IMX8MQ_CLK_TMU_ROOT] = imx_clk_gate4_flags("tmu_root_clk", 
"ipg_root", base + 0x4620, 0, CLK_IS_CRITICAL);
+   clks[IMX8MQ_CLK_TMU_ROOT] = imx_clk_gate4("tmu_root_clk", "ipg_root", 
base + 0x4620, 0);
clks[IMX8MQ_CLK_VPU_DEC_ROOT] = imx_clk_gate2_flags("vpu_dec_root_clk", 
"vpu_bus", base + 0x4630, 0, CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE);
clks[IMX8MQ_CLK_CSI1_ROOT] = imx_clk_gate4("csi1_root_clk", 
"csi1_core", base + 0x4650, 0);
clks[IMX8MQ_CLK_CSI2_ROOT] = imx_clk_gate4("csi2_root_clk", 
"csi2_core", base + 0x4660, 0);
-- 
2.7.4



[PATCH 4/6] thermal: qoriq: Add clock operations

2019-07-04 Thread Anson . Huang
From: Anson Huang 

Some platforms like i.MX8MQ has clock control for this module,
need to add clock operations to make sure the driver is working
properly.

Signed-off-by: Anson Huang 
---
 drivers/thermal/qoriq_thermal.c | 24 
 1 file changed, 24 insertions(+)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 2b2f79b..0813c1b 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -2,6 +2,7 @@
 //
 // Copyright 2016 Freescale Semiconductor, Inc.
 
+#include 
 #include 
 #include 
 #include 
@@ -72,6 +73,7 @@ struct qoriq_sensor {
 
 struct qoriq_tmu_data {
struct qoriq_tmu_regs __iomem *regs;
+   struct clk *clk;
bool little_endian;
struct qoriq_sensor *sensor[SITES_MAX];
 };
@@ -208,6 +210,19 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
return PTR_ERR(data->regs);
}
 
+   data->clk = devm_clk_get(>dev, NULL);
+   if (IS_ERR(data->clk)) {
+   if (PTR_ERR(data->clk) == -EPROBE_DEFER)
+   return -EPROBE_DEFER;
+   data->clk = NULL;
+   }
+
+   ret = clk_prepare_enable(data->clk);
+   if (ret) {
+   dev_err(>dev, "Failed to enable clock\n");
+   return ret;
+   }
+
qoriq_tmu_init_device(data);/* TMU initialization */
 
ret = qoriq_tmu_calibration(pdev);  /* TMU calibration */
@@ -235,6 +250,8 @@ static int qoriq_tmu_remove(struct platform_device *pdev)
/* Disable monitoring */
tmu_write(data, TMR_DISABLE, >regs->tmr);
 
+   clk_disable_unprepare(data->clk);
+
platform_set_drvdata(pdev, NULL);
 
return 0;
@@ -250,14 +267,21 @@ static int __maybe_unused qoriq_tmu_suspend(struct device 
*dev)
tmr &= ~TMR_ME;
tmu_write(data, tmr, >regs->tmr);
 
+   clk_disable_unprepare(data->clk);
+
return 0;
 }
 
 static int __maybe_unused qoriq_tmu_resume(struct device *dev)
 {
u32 tmr;
+   int ret;
struct qoriq_tmu_data *data = dev_get_drvdata(dev);
 
+   ret = clk_prepare_enable(data->clk);
+   if (ret)
+   return ret;
+
/* Enable monitoring */
tmr = tmu_read(data, >regs->tmr);
tmr |= TMR_ME;
-- 
2.7.4



[PATCH 3/6] dt-bindings: thermal: qoriq: Add optional clocks property

2019-07-04 Thread Anson . Huang
From: Anson Huang 

Some platforms have clock control for TMU, add optional
clocks property to the binding doc.

Signed-off-by: Anson Huang 
---
 Documentation/devicetree/bindings/thermal/qoriq-thermal.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/thermal/qoriq-thermal.txt 
b/Documentation/devicetree/bindings/thermal/qoriq-thermal.txt
index 04cbb90..28f2cba 100644
--- a/Documentation/devicetree/bindings/thermal/qoriq-thermal.txt
+++ b/Documentation/devicetree/bindings/thermal/qoriq-thermal.txt
@@ -23,6 +23,7 @@ Required properties:
 Optional property:
 - little-endian : If present, the TMU registers are little endian. If absent,
the default is big endian.
+- clocks : the clock for clocking the TMU silicon.
 
 Example:
 
-- 
2.7.4



RE: [PATCH V3 1/2] dt-bindings: reset: imx7: Add support for i.MX8MM

2019-07-04 Thread Anson Huang
Hi, Philipp

> On Thu, 2019-07-04 at 17:44 +0800, anson.hu...@nxp.com wrote:
> > From: Anson Huang 
> >
> > i.MX8MM can reuse i.MX8MQ's reset driver, update the compatible
> > property and related info to support i.MX8MM.
> >
> > Signed-off-by: Anson Huang 
> > ---
> > Changes since V2:
> > - Add separate line for i.MX8MM in case anything different later for
> i.MX8MM.
> > ---
> >  Documentation/devicetree/bindings/reset/fsl,imx7-src.txt | 6 --
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/reset/fsl,imx7-src.txt
> > b/Documentation/devicetree/bindings/reset/fsl,imx7-src.txt
> > index 13e0951..c2489e4 100644
> > --- a/Documentation/devicetree/bindings/reset/fsl,imx7-src.txt
> > +++ b/Documentation/devicetree/bindings/reset/fsl,imx7-src.txt
> > @@ -8,6 +8,7 @@ Required properties:
> >  - compatible:
> > - For i.MX7 SoCs should be "fsl,imx7d-src", "syscon"
> > - For i.MX8MQ SoCs should be "fsl,imx8mq-src", "syscon"
> > +   - For i.MX8MM SoCs should be "fsl,imx8mm-src", "fsl,imx8mq-src",
> "syscon"
> >  - reg: should be register base and length as documented in the
> >datasheet
> >  - interrupts: Should contain SRC interrupt @@ -46,5 +47,6 @@ Example:
> >
> >
> >  For list of all valid reset indices see
> > - for i.MX7 and
> > - for i.MX8MQ
> > + for i.MX7,
> > + for i.MX8MQ and
> > + for i.MX8MM
> 
> The last line is misleading, as that file contains reset indices that are 
> invalid
> for i.MX8MM.

What is your suggestion about this line? Just NOT change it? Or adding a new 
file
imx8mm-reset.h but still use the IMX8MQ_RESET_ as prefix ? Or keep what I 
changed, but
adding some comments in those macros that i.MX8MM does NOT support?

Thanks,
Anson.

> 
> regards
> Philipp


[PATCH V3 2/2] arm64: dts: imx8mm: Add "fsl,imx8mq-src" as src's fallback compatible

2019-07-04 Thread Anson . Huang
From: Anson Huang 

i.MX8MM can reuse i.MX8MQ's src driver, add "fsl,imx8mq-src" as
src's fallback compatible to enable it.

Signed-off-by: Anson Huang 
Reviewed-by: Philipp Zabel 
---
No changes.
---
 arch/arm64/boot/dts/freescale/imx8mm.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi 
b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index 172bb6f..1870c89 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -520,7 +520,7 @@
};
 
src: reset-controller@3039 {
-   compatible = "fsl,imx8mm-src", "syscon";
+   compatible = "fsl,imx8mm-src", 
"fsl,imx8mq-src", "syscon";
reg = <0x3039 0x1>;
interrupts = ;
#reset-cells = <1>;
-- 
2.7.4



[PATCH V3 1/2] dt-bindings: reset: imx7: Add support for i.MX8MM

2019-07-04 Thread Anson . Huang
From: Anson Huang 

i.MX8MM can reuse i.MX8MQ's reset driver, update the compatible
property and related info to support i.MX8MM.

Signed-off-by: Anson Huang 
---
Changes since V2:
- Add separate line for i.MX8MM in case anything different later for 
i.MX8MM.
---
 Documentation/devicetree/bindings/reset/fsl,imx7-src.txt | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/reset/fsl,imx7-src.txt 
b/Documentation/devicetree/bindings/reset/fsl,imx7-src.txt
index 13e0951..c2489e4 100644
--- a/Documentation/devicetree/bindings/reset/fsl,imx7-src.txt
+++ b/Documentation/devicetree/bindings/reset/fsl,imx7-src.txt
@@ -8,6 +8,7 @@ Required properties:
 - compatible:
- For i.MX7 SoCs should be "fsl,imx7d-src", "syscon"
- For i.MX8MQ SoCs should be "fsl,imx8mq-src", "syscon"
+   - For i.MX8MM SoCs should be "fsl,imx8mm-src", "fsl,imx8mq-src", 
"syscon"
 - reg: should be register base and length as documented in the
   datasheet
 - interrupts: Should contain SRC interrupt
@@ -46,5 +47,6 @@ Example:
 
 
 For list of all valid reset indices see
- for i.MX7 and
- for i.MX8MQ
+ for i.MX7,
+ for i.MX8MQ and
+ for i.MX8MM
-- 
2.7.4



RE: [PATCH V2 1/2] dt-bindings: reset: imx7: Add support for i.MX8MM

2019-07-04 Thread Anson Huang
Hi, Philipp

> On Thu, 2019-07-04 at 17:25 +0800, anson.hu...@nxp.com wrote:
> > From: Anson Huang 
> >
> > i.MX8MM can reuse i.MX8MQ's reset driver, update the compatible
> > property and related info to support i.MX8MM.
> >
> > Signed-off-by: Anson Huang 
> > ---
> > New patch.
> > ---
> >  Documentation/devicetree/bindings/reset/fsl,imx7-src.txt | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/reset/fsl,imx7-src.txt
> > b/Documentation/devicetree/bindings/reset/fsl,imx7-src.txt
> > index 13e0951..bc24c45 100644
> > --- a/Documentation/devicetree/bindings/reset/fsl,imx7-src.txt
> > +++ b/Documentation/devicetree/bindings/reset/fsl,imx7-src.txt
> > @@ -7,7 +7,7 @@ controller binding usage.
> >  Required properties:
> >  - compatible:
> > - For i.MX7 SoCs should be "fsl,imx7d-src", "syscon"
> > -   - For i.MX8MQ SoCs should be "fsl,imx8mq-src", "syscon"
> > +   - For i.MX8MQ/i.MX8MM SoCs should be "fsl,imx8mq-src", "syscon"
> 
> Please still add the "fsl,imx8mm-src" for i.MX8MM, just in case a significant
> difference is discovered later.

OK, then I will add a new line as below:

For i.MX8MM SoCs should be "fsl,imx8mm-src", "fsl,imx8mq-src", "syscon"

Anson.

> 
> regards
> Philipp


[PATCH V2 1/2] dt-bindings: reset: imx7: Add support for i.MX8MM

2019-07-04 Thread Anson . Huang
From: Anson Huang 

i.MX8MM can reuse i.MX8MQ's reset driver, update the compatible
property and related info to support i.MX8MM.

Signed-off-by: Anson Huang 
---
New patch.
---
 Documentation/devicetree/bindings/reset/fsl,imx7-src.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/reset/fsl,imx7-src.txt 
b/Documentation/devicetree/bindings/reset/fsl,imx7-src.txt
index 13e0951..bc24c45 100644
--- a/Documentation/devicetree/bindings/reset/fsl,imx7-src.txt
+++ b/Documentation/devicetree/bindings/reset/fsl,imx7-src.txt
@@ -7,7 +7,7 @@ controller binding usage.
 Required properties:
 - compatible:
- For i.MX7 SoCs should be "fsl,imx7d-src", "syscon"
-   - For i.MX8MQ SoCs should be "fsl,imx8mq-src", "syscon"
+   - For i.MX8MQ/i.MX8MM SoCs should be "fsl,imx8mq-src", "syscon"
 - reg: should be register base and length as documented in the
   datasheet
 - interrupts: Should contain SRC interrupt
@@ -47,4 +47,4 @@ Example:
 
 For list of all valid reset indices see
  for i.MX7 and
- for i.MX8MQ
+ for i.MX8MQ/i.MX8MM
-- 
2.7.4



[PATCH V2 2/2] arm64: dts: imx8mm: Add "fsl,imx8mq-src" as src's fallback compatible

2019-07-04 Thread Anson . Huang
From: Anson Huang 

i.MX8MM can reuse i.MX8MQ's src driver, add "fsl,imx8mq-src" as
src's fallback compatible to enable it.

Signed-off-by: Anson Huang 
Reviewed-by: Philipp Zabel 
---
No changes.
---
 arch/arm64/boot/dts/freescale/imx8mm.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi 
b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index 172bb6f..1870c89 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -520,7 +520,7 @@
};
 
src: reset-controller@3039 {
-   compatible = "fsl,imx8mm-src", "syscon";
+   compatible = "fsl,imx8mm-src", 
"fsl,imx8mq-src", "syscon";
reg = <0x3039 0x1>;
interrupts = ;
#reset-cells = <1>;
-- 
2.7.4



[PATCH 3/4] arm64: defconfig: Enable CONFIG_IMX8MM_THERMAL as module

2019-07-04 Thread Anson . Huang
From: Anson Huang 

Enable CONFIG_IMX8MM_THERMAL as module to support i.MX8MM
thermal driver.

Signed-off-by: Anson Huang 
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 126665f..eeedd3f 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -434,6 +434,7 @@ CONFIG_CPU_THERMAL=y
 CONFIG_THERMAL_EMULATION=y
 CONFIG_QORIQ_THERMAL=m
 CONFIG_IMX_SC_THERMAL=m
+CONFIG_IMX8MM_THERMAL=m
 CONFIG_ROCKCHIP_THERMAL=m
 CONFIG_RCAR_THERMAL=y
 CONFIG_RCAR_GEN3_THERMAL=y
-- 
2.7.4



[PATCH 2/4] thermal: imx8mm: Add support for i.MX8MM thermal monitoring unit

2019-07-04 Thread Anson . Huang
From: Anson Huang 

i.MX8MM has a thermal monitoring unit(TMU) inside, it ONLY has one
sensor for CPU, add support for reading immediate temperature of
this sensor.

Signed-off-by: Anson Huang 
---
 drivers/thermal/Kconfig  |  10 +++
 drivers/thermal/Makefile |   1 +
 drivers/thermal/imx8mm_thermal.c | 134 +++
 3 files changed, 145 insertions(+)
 create mode 100644 drivers/thermal/imx8mm_thermal.c

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 454cbe5..d1663cd 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -244,6 +244,16 @@ config IMX_SC_THERMAL
  sensor. It supports one critical trip point and one
  passive trip point for each thermal sensor.
 
+config IMX8MM_THERMAL
+   tristate "Temperature sensor driver for Freescale i.MX8MM SoC"
+   depends on ARCH_MXC
+   depends on OF
+   help
+ Support for Thermal Monitoring Unit (TMU) found on Freescale i.MX8MM 
SoC.
+ It supports one critical trip point and one passive trip point. The
+ cpufreq is used as the cooling device to throttle CPUs when the 
passive
+ trip is crossed.
+
 config MAX77620_THERMAL
tristate "Temperature sensor driver for Maxim MAX77620 PMIC"
depends on MFD_MAX77620
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 717a1ba..a397d4d 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -42,6 +42,7 @@ obj-$(CONFIG_ARMADA_THERMAL)  += armada_thermal.o
 obj-$(CONFIG_TANGO_THERMAL)+= tango_thermal.o
 obj-$(CONFIG_IMX_THERMAL)  += imx_thermal.o
 obj-$(CONFIG_IMX_SC_THERMAL)   += imx_sc_thermal.o
+obj-$(CONFIG_IMX8MM_THERMAL)   += imx8mm_thermal.o
 obj-$(CONFIG_MAX77620_THERMAL) += max77620_thermal.o
 obj-$(CONFIG_QORIQ_THERMAL)+= qoriq_thermal.o
 obj-$(CONFIG_DA9062_THERMAL)   += da9062-thermal.o
diff --git a/drivers/thermal/imx8mm_thermal.c b/drivers/thermal/imx8mm_thermal.c
new file mode 100644
index 000..04f8a8f
--- /dev/null
+++ b/drivers/thermal/imx8mm_thermal.c
@@ -0,0 +1,134 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2019 NXP.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "thermal_core.h"
+
+#define TER0x0 /* TMU enable */
+#define TRITSR 0x20/* TMU immediate temp */
+
+#define TER_EN BIT(31)
+#define TRITSR_VAL_MASK0xff
+
+#define TEMP_LOW_LIMIT 10
+
+struct imx8mm_tmu {
+   struct thermal_zone_device *tzd;
+   void __iomem *base;
+   struct clk *clk;
+};
+
+static int tmu_get_temp(void *data, int *temp)
+{
+   struct imx8mm_tmu *tmu = data;
+   u32 val;
+
+   /* the temp sensor need about 1ms to finish the measurement */
+   usleep_range(1000, 2000);
+
+   val = readl_relaxed(tmu->base + TRITSR) & TRITSR_VAL_MASK;
+   if (val < TEMP_LOW_LIMIT)
+   return -EAGAIN;
+
+   *temp = val * 1000;
+
+   return 0;
+}
+
+static struct thermal_zone_of_device_ops tmu_tz_ops = {
+   .get_temp = tmu_get_temp,
+};
+
+static int imx8mm_tmu_probe(struct platform_device *pdev)
+{
+   struct imx8mm_tmu *tmu;
+   u32 val;
+   int ret;
+
+   tmu = devm_kzalloc(>dev, sizeof(struct imx8mm_tmu), GFP_KERNEL);
+   if (!tmu)
+   return -ENOMEM;
+
+   tmu->base = devm_platform_ioremap_resource(pdev, 0);
+   if (IS_ERR(tmu->base))
+   return PTR_ERR(tmu->base);
+
+   tmu->clk = devm_clk_get(>dev, NULL);
+   if (IS_ERR(tmu->clk)) {
+   ret = PTR_ERR(tmu->clk);
+   if (ret != -EPROBE_DEFER)
+   dev_err(>dev,
+   "failed to get tmu clock: %d\n", ret);
+   return ret;
+   }
+
+   ret = clk_prepare_enable(tmu->clk);
+   if (ret) {
+   dev_err(>dev, "failed to enable tmu clock: %d\n", ret);
+   return ret;
+   }
+
+   tmu->tzd = devm_thermal_zone_of_sensor_register(>dev, 0,
+   tmu, _tz_ops);
+   if (IS_ERR(tmu->tzd)) {
+   dev_err(>dev,
+   "failed to register thermal zone sensor: %d\n", ret);
+   return PTR_ERR(tmu->tzd);
+   }
+
+   platform_set_drvdata(pdev, tmu);
+
+   /* enable the monitor */
+   val = readl_relaxed(tmu->base + TER);
+   val |= TER_EN;
+   writel_relaxed(val, tmu->base + TER);
+
+   return 0;
+}
+
+static int imx8mm_tmu_remove(struct platform_device *pdev)
+{
+   struct imx8mm_tmu *tmu = platform_get_drvdata(pdev);
+   u32 val;
+
+   /* disable TMU */
+   val = readl_relaxed(tmu->base + TER);
+   val &= ~TER_EN;
+   writel_rel

[PATCH 4/4] arm64: dts: imx8mm: Add thermal zone support

2019-07-04 Thread Anson . Huang
From: Anson Huang 

Add thermal zone and tmu node to support i.MX8MM thermal
driver, ONLY cpu thermal zone is supported, and cpu cooling
is also added.

Signed-off-by: Anson Huang 
---
 arch/arm64/boot/dts/freescale/imx8mm.dtsi | 43 +++
 1 file changed, 43 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi 
b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index 3a62407..1870c89 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -70,6 +70,7 @@
nvmem-cells = <_speed_grade>;
nvmem-cell-names = "speed_grade";
cpu-idle-states = <_sleep_wait>;
+   #cooling-cells = <2>;
};
 
A53_1: cpu@1 {
@@ -82,6 +83,7 @@
next-level-cache = <_L2>;
operating-points-v2 = <_opp_table>;
cpu-idle-states = <_sleep_wait>;
+   #cooling-cells = <2>;
};
 
A53_2: cpu@2 {
@@ -94,6 +96,7 @@
next-level-cache = <_L2>;
operating-points-v2 = <_opp_table>;
cpu-idle-states = <_sleep_wait>;
+   #cooling-cells = <2>;
};
 
A53_3: cpu@3 {
@@ -106,6 +109,7 @@
next-level-cache = <_L2>;
operating-points-v2 = <_opp_table>;
cpu-idle-states = <_sleep_wait>;
+   #cooling-cells = <2>;
};
 
A53_L2: l2-cache0 {
@@ -209,6 +213,38 @@
arm,no-tick-in-suspend;
};
 
+   thermal-zones {
+   cpu-thermal {
+   polling-delay-passive = <250>;
+   polling-delay = <2000>;
+   thermal-sensors = <>;
+   trips {
+   cpu_alert0: trip0 {
+   temperature = <85000>;
+   hysteresis = <2000>;
+   type = "passive";
+   };
+
+   cpu_crit0: trip1 {
+   temperature = <95000>;
+   hysteresis = <2000>;
+   type = "critical";
+   };
+   };
+
+   cooling-maps {
+   map0 {
+   trip = <_alert0>;
+   cooling-device =
+   <_0 THERMAL_NO_LIMIT 
THERMAL_NO_LIMIT>,
+   <_1 THERMAL_NO_LIMIT 
THERMAL_NO_LIMIT>,
+   <_2 THERMAL_NO_LIMIT 
THERMAL_NO_LIMIT>,
+   <_3 THERMAL_NO_LIMIT 
THERMAL_NO_LIMIT>;
+   };
+   };
+   };
+   };
+
usbphynop1: usbphynop1 {
compatible = "usb-nop-xceiv";
clocks = < IMX8MM_CLK_USB_PHY_REF>;
@@ -368,6 +404,13 @@
gpio-ranges = < 0 119 30>;
};
 
+   tmu: tmu@3026 {
+   compatible = "fsl,imx8mm-tmu";
+   reg = <0x3026 0x1>;
+   clocks = < IMX8MM_CLK_TMU_ROOT>;
+   #thermal-sensor-cells = <0>;
+   };
+
wdog1: watchdog@3028 {
compatible = "fsl,imx8mm-wdt", "fsl,imx21-wdt";
reg = <0x3028 0x1>;
-- 
2.7.4



[PATCH 1/4] dt-bindings: thermal: imx8mm-thermal: Add binding doc for i.MX8MM

2019-07-04 Thread Anson . Huang
From: Anson Huang 

Add thermal binding doc for Freescale's i.MX8MM Thermal Monitoring Unit.

Signed-off-by: Anson Huang 
---
 .../devicetree/bindings/thermal/imx8mm-thermal.txt| 15 +++
 1 file changed, 15 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/imx8mm-thermal.txt

diff --git a/Documentation/devicetree/bindings/thermal/imx8mm-thermal.txt 
b/Documentation/devicetree/bindings/thermal/imx8mm-thermal.txt
new file mode 100644
index 000..d09ae82
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/imx8mm-thermal.txt
@@ -0,0 +1,15 @@
+* Thermal Monitoring Unit (TMU) on Freescale i.MX8MM SoC
+
+Required properties:
+- compatible : Must be "fsl,imx8mm-tmu".
+- reg : Address range of TMU registers.
+- clocks : TMU's clock source.
+- #thermal-sensor-cells : Should be 0. See ./thermal.txt for a description.
+
+Example:
+tmu: tmu@3026 {
+   compatible = "fsl,imx8mm-tmu";
+   reg = <0x3026 0x1>;
+   clocks = < IMX8MM_CLK_TMU_ROOT>;
+   #thermal-sensor-cells = <0>;
+};
-- 
2.7.4



[PATCH 0/4] Add support for i.MX8MM thermal sensor driver

2019-07-04 Thread Anson . Huang
From: Anson Huang 

i.MX8MM has a thermal monitor unit (TMU) inside, it ONLY has one sensor
for CPU, add support for temperature reading for CPU, cpu cooling is
also added.

This patch series is based on below i.MX SCU thermal patch series:
https://patchwork.kernel.org/patch/11000821/

Anson Huang (4):
  dt-bindings: thermal: imx8mm-thermal: Add binding doc for i.MX8MM
  thermal: imx8mm: Add support for i.MX8MM thermal monitoring unit
  arm64: defconfig: Enable CONFIG_IMX8MM_THERMAL as module
  arm64: dts: imx8mm: Add thermal zone support

 .../devicetree/bindings/thermal/imx8mm-thermal.txt |  15 +++
 arch/arm64/boot/dts/freescale/imx8mm.dtsi  |  43 +++
 arch/arm64/configs/defconfig   |   1 +
 drivers/thermal/Kconfig|  10 ++
 drivers/thermal/Makefile   |   1 +
 drivers/thermal/imx8mm_thermal.c   | 134 +
 6 files changed, 204 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/imx8mm-thermal.txt
 create mode 100644 drivers/thermal/imx8mm_thermal.c

-- 
2.7.4



RE: [PATCH 1/2] reset: imx7: Add support for i.MX8MM SoC

2019-07-04 Thread Anson Huang
Hi, Philipp

> On Mon, 2019-07-01 at 17:39 +0800, anson.hu...@nxp.com wrote:
> > From: Anson Huang 
> >
> > i.MX8MM SoC has a subset of i.MX8MQ IP block variant, it can reuse the
> > i.MX8MQ reset controller driver and just skip those non-existing IP
> > blocks, add support for i.MX8MM SoC reset control.
> >
> > Signed-off-by: Anson Huang 
> > ---
> >  drivers/reset/reset-imx7.c | 20 
> >  1 file changed, 20 insertions(+)
> >
> > diff --git a/drivers/reset/reset-imx7.c b/drivers/reset/reset-imx7.c
> > index 3ecd770..941131f 100644
> > --- a/drivers/reset/reset-imx7.c
> > +++ b/drivers/reset/reset-imx7.c
> > @@ -207,6 +207,26 @@ static int imx8mq_reset_set(struct
> reset_controller_dev *rcdev,
> > const unsigned int bit = imx7src->signals[id].bit;
> > unsigned int value = assert ? bit : 0;
> >
> > +   if (of_machine_is_compatible("fsl,imx8mm")) {
> 
> This should be checked once during probe, not in every reset_set, if this
> check has to be made at all. On i.MX8MM these unused reset controls are
> not going to be hooked up via phandle, so this function should never be
> called with the values that are filtered out here anyway.
> And in case somebody makes an error in the device tree, does writing 1 to
> the reserved bits on i.MX8MM have any negative side effects at all?
> Or are these bits just not hooked up? If this is no problem, I assume this
> patch is not needed at all.

I just tried it on i.MX8MM, read/write to the reserved registers looks like OK,
so this patch can be ignored, although accessing reserved registers is NOT good,
but the user should have correct settings in DT, there will be no access for 
these
reserved registers.

So, let's just ignore this patch, adding the fallback compatible should be good 
for
i.MX8MM to reuse this driver.

Thanks,
Anson 

> 
> The correct way to protect against DT writers hooking up the non- existing
> reset lines would be to replace rcdev.of_xlate with a version that returns -
> EINVAL for them on i.MX8MM. Also in that case I'd check the reset-controller
> compatible, not the machine compatible.
> 
> > +   switch (id) {
> > +   case IMX8MQ_RESET_HDMI_PHY_APB_RESET:
> > +   case IMX8MQ_RESET_PCIEPHY2: /* fallthrough */
> > +   case IMX8MQ_RESET_PCIEPHY2_PERST: /* fallthrough */
> > +   case IMX8MQ_RESET_PCIE2_CTRL_APPS_EN: /* fallthrough
> */
> > +   case IMX8MQ_RESET_PCIE2_CTRL_APPS_TURNOFF: /*
> fallthrough */
> > +   case IMX8MQ_RESET_MIPI_CSI1_CORE_RESET: /* fallthrough
> */
> > +   case IMX8MQ_RESET_MIPI_CSI1_PHY_REF_RESET: /*
> fallthrough */
> > +   case IMX8MQ_RESET_MIPI_CSI1_ESC_RESET: /* fallthrough
> */
> > +   case IMX8MQ_RESET_MIPI_CSI2_CORE_RESET: /* fallthrough
> */
> > +   case IMX8MQ_RESET_MIPI_CSI2_PHY_REF_RESET: /*
> fallthrough */
> > +   case IMX8MQ_RESET_MIPI_CSI2_ESC_RESET: /* fallthrough
> */
> > +   case IMX8MQ_RESET_DDRC2_PHY_RESET: /* fallthrough */
> > +   case IMX8MQ_RESET_DDRC2_CORE_RESET: /* fallthrough */
> > +   case IMX8MQ_RESET_DDRC2_PRST: /* fallthrough */
> 
> I think it would make sense to add IMX8MM_RESET_* defines for all but
> these, to avoid confusion when reading the imx8mm.dtsi
> 
> regards
> Philipp


RE: [PATCH 2/2] arm64: dts: imx8mm: Assign highest opp as suspend opp

2019-07-04 Thread Anson Huang
Hi, Leonard

> On 7/4/2019 9:23 AM, anson.hu...@nxp.com wrote:
> > From: Anson Huang 
> >
> > Assign highest OPP as suspend OPP to reduce suspend/resume latency on
> > i.MX8MM.
> >
> > Signed-off-by: Anson Huang 
> > ---
> >   arch/arm64/boot/dts/freescale/imx8mm.dtsi | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > index b11fc5e..3a62407 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > @@ -136,6 +136,7 @@
> > opp-microvolt = <100>;
> > opp-supported-hw = <0x8>, <0x3>;
> > clock-latency-ns = <15>;
> > +   opp-suspend;
> > };
> > };
> 
> What if the highest OPP is unavailable due to speed grading? Ideally we
> should find a way to suspend at the highest *supported* OPP.
> 
> Maybe the opp-suspend marking could be assigned from imx-cpufreq-dt
> driver code?

Yes, this is also my concern, the current OPP driver does NOT handle it well, 
and
I was thinking to assigne it from imx-cpufreq-dt driver, 1 option is to runtime 
add
"suspend-opp" property into DT OPP node after parsing the speed grading fuse and
OPP table, but I do NOT like that very much, as we need to manually create a 
property,
the other option is to change cpu freq policy inside imx-cpufreq-dt driver in 
suspend/resume
callback? Which one do you prefer?

Thanks,
Anson

> 
> --
> Regards,
> Leonard



[PATCH 2/2] arm64: dts: imx8mm: Assign highest opp as suspend opp

2019-07-04 Thread Anson . Huang
From: Anson Huang 

Assign highest OPP as suspend OPP to reduce suspend/resume
latency on i.MX8MM.

Signed-off-by: Anson Huang 
---
This patch is based on https://patchwork.kernel.org/patch/11023813/
---
 arch/arm64/boot/dts/freescale/imx8mm.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi 
b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index b11fc5e..3a62407 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -136,6 +136,7 @@
opp-microvolt = <100>;
opp-supported-hw = <0x8>, <0x3>;
clock-latency-ns = <15>;
+   opp-suspend;
};
};
 
-- 
2.7.4



[PATCH 1/2] arm64: dts: imx8mq: Assign highest opp as suspend opp

2019-07-04 Thread Anson . Huang
From: Anson Huang 

Assign highest OPP as suspend OPP to reduce suspend/resume
latency on i.MX8MQ.

Signed-off-by: Anson Huang 
---
This patch is based on https://patchwork.kernel.org/patch/11023815/
---
 arch/arm64/boot/dts/freescale/imx8mq.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi 
b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index 3187428..c61e968 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -171,6 +171,7 @@
opp-microvolt = <100>;
opp-supported-hw = <0xc>, <0x4>;
clock-latency-ns = <15>;
+   opp-suspend;
};
 
opp-15 {
@@ -178,6 +179,7 @@
opp-microvolt = <100>;
opp-supported-hw = <0x8>, <0x3>;
clock-latency-ns = <15>;
+   opp-suspend;
};
};
 
-- 
2.7.4



RE: [PATCH RESEND V4 1/5] clocksource: timer-of: Support getting clock frequency from DT

2019-07-02 Thread Anson Huang
Hi, Daniel

> On 02/07/2019 11:03, Anson Huang wrote:
> > Hi, Daniel
> >
> >> Hi Anson,
> >>
> >> why did you resend the series?
> >
> > Previous patch series has build warning and I did NOT notice, sorry
> > for that,
> >
> > drivers/clocksource/timer-of.c: In function ‘timer_of_init’:
> > drivers/clocksource/timer-of.c:185:30: warning: suggest parentheses
> around comparison in operand of ‘&’ [-Wparentheses]
> >   if (to->flags & clock_flags == clock_flags)
> >   ^
> >
> > so I resend the patch series with below, sorry for missing mention of the
> changes in resent patch series.
> >
> >  +  if ((to->flags & clock_flags) == clock_flags)
> >
> > Sorry for mail storm...
> 
> No problem at all, I prefer this caught and fixed early :)
> 
> Next time just send a V5 because 'resend' means there is no change but
> there was a problem with the email (could be also interpreted as a gentle
> ping).

OK, will keep it in mind, thanks for sharing useful experience!

Thanks,
Anson



RE: [PATCH RESEND V4 1/5] clocksource: timer-of: Support getting clock frequency from DT

2019-07-02 Thread Anson Huang
Hi, Daniel

> Hi Anson,
> 
> why did you resend the series?

Previous patch series has build warning and I did NOT notice, sorry for that,

drivers/clocksource/timer-of.c: In function ‘timer_of_init’:
drivers/clocksource/timer-of.c:185:30: warning: suggest parentheses around 
comparison in operand of ‘&’ [-Wparentheses]
  if (to->flags & clock_flags == clock_flags)
  ^

so I resend the patch series with below, sorry for missing mention of the 
changes in resent patch series.

 +  if ((to->flags & clock_flags) == clock_flags)

Sorry for mail storm...

Thanks,
Anson

> 
> 
> On 02/07/2019 09:55, anson.hu...@nxp.com wrote:
> > From: Anson Huang 
> >
> > More and more platforms use platform driver model for clock driver, so
> > the clock driver is NOT ready during timer initialization phase, it
> > will cause timer initialization failed.
> >
> > To support those platforms with upper scenario, introducing a new flag
> > TIMER_OF_CLOCK_FREQUENCY which is mutually exclusive with
> > TIMER_OF_CLOCK flag to support getting timer clock frequency from DT's
> > timer node, the property name should be "clock-frequency", then of_clk
> > operations can be skipped.
> >
> > User needs to select either TIMER_OF_CLOCK_FREQUENCY or
> TIMER_OF_CLOCK
> > flag if want to use timer-of driver to initialize the clock rate.
> >
> > Signed-off-by: Anson Huang 
> > ---
> > Changes since V3:
> > - use hardcoded "clock-frequency" instead of adding new variable
> prop_name;
> > - add pre-condition check for TIMER_OF_CLOCK and
> TIMER_OF_CLOCK_FREQUENCY, they MUST be exclusive.
> > ---
> >  drivers/clocksource/timer-of.c | 29 +
> > drivers/clocksource/timer-of.h |  7 ---
> >  2 files changed, 33 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/clocksource/timer-of.c
> > b/drivers/clocksource/timer-of.c index 8054228..858f684 100644
> > --- a/drivers/clocksource/timer-of.c
> > +++ b/drivers/clocksource/timer-of.c
> > @@ -161,11 +161,30 @@ static __init int timer_of_base_init(struct
> device_node *np,
> > return 0;
> >  }
> >
> > +static __init int timer_of_clk_frequency_init(struct device_node *np,
> > + struct of_timer_clk *of_clk) {
> > +   int ret;
> > +   u32 rate;
> > +
> > +   ret = of_property_read_u32(np, "clock-frequency", );
> > +   if (!ret) {
> > +   of_clk->rate = rate;
> > +   of_clk->period = DIV_ROUND_UP(rate, HZ);
> > +   }
> > +
> > +   return ret;
> > +}
> > +
> >  int __init timer_of_init(struct device_node *np, struct timer_of *to)
> > {
> > +   unsigned long clock_flags = TIMER_OF_CLOCK |
> > +TIMER_OF_CLOCK_FREQUENCY;
> > int ret = -EINVAL;
> > int flags = 0;
> >
> > +   if ((to->flags & clock_flags) == clock_flags)
> > +   return ret;
> > +
> > if (to->flags & TIMER_OF_BASE) {
> > ret = timer_of_base_init(np, >of_base);
> > if (ret)
> > @@ -180,6 +199,13 @@ int __init timer_of_init(struct device_node *np,
> struct timer_of *to)
> > flags |= TIMER_OF_CLOCK;
> > }
> >
> > +   if (to->flags & TIMER_OF_CLOCK_FREQUENCY) {
> > +   ret = timer_of_clk_frequency_init(np, >of_clk);
> > +   if (ret)
> > +   goto out_fail;
> > +   flags |= TIMER_OF_CLOCK_FREQUENCY;
> > +   }
> > +
> > if (to->flags & TIMER_OF_IRQ) {
> > ret = timer_of_irq_init(np, >of_irq);
> > if (ret)
> > @@ -201,6 +227,9 @@ int __init timer_of_init(struct device_node *np,
> struct timer_of *to)
> > if (flags & TIMER_OF_CLOCK)
> > timer_of_clk_exit(>of_clk);
> >
> > +   if (flags & TIMER_OF_CLOCK_FREQUENCY)
> > +   to->of_clk.rate = 0;
> > +
> > if (flags & TIMER_OF_BASE)
> > timer_of_base_exit(>of_base);
> > return ret;
> > diff --git a/drivers/clocksource/timer-of.h
> > b/drivers/clocksource/timer-of.h index a5478f3..a08e108 100644
> > --- a/drivers/clocksource/timer-of.h
> > +++ b/drivers/clocksource/timer-of.h
> > @@ -4,9 +4,10 @@
> >
> >  #include 
> >
> > -#define TIMER_OF_BASE  0x1
> > -#define TIMER_OF_CLOCK 0x2
> > -#define TIMER_OF_IRQ   0x4
> > +#define TIMER_OF_BASE  0x1
> > +#define TIMER_OF_CLOCK 0x2
> > +#define TIMER_OF_IRQ   0x4
> > +#define TIMER_OF_CLOCK_FREQUENCY   0x8
> >
> >  struct of_timer_irq {
> > int irq;


[PATCH RESEND V4 3/5] clocksource: imx-sysctr: Make timer work with clock driver using platform driver model

2019-07-02 Thread Anson . Huang
From: Anson Huang 

On some i.MX8M platforms, clock driver uses platform driver
model and it is NOT ready during timer initialization phase,
the clock operations will fail and system counter driver will
fail too. As all the i.MX8M platforms' system counter clock
are from OSC which is always enabled, so it is no need to enable
clock for system counter driver, the ONLY thing is to pass
clock frequence to driver.

To make system counter driver work for upper scenario, if DT's
system counter node has property "clock-frequency" present,
setting TIMER_OF_CLOCK_FREQUENCY flag to indicate timer-of driver
to get clock frequency from DT directly instead of of_clk operation
via clk APIs.

Signed-off-by: Anson Huang 
---
Changes since V3:
- remove the .prop_name initialization acording to timer-of changes in 
V4.
---
 drivers/clocksource/timer-imx-sysctr.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/clocksource/timer-imx-sysctr.c 
b/drivers/clocksource/timer-imx-sysctr.c
index fd7d680..b82a549 100644
--- a/drivers/clocksource/timer-imx-sysctr.c
+++ b/drivers/clocksource/timer-imx-sysctr.c
@@ -98,7 +98,7 @@ static irqreturn_t sysctr_timer_interrupt(int irq, void 
*dev_id)
 }
 
 static struct timer_of to_sysctr = {
-   .flags = TIMER_OF_IRQ | TIMER_OF_CLOCK | TIMER_OF_BASE,
+   .flags = TIMER_OF_IRQ | TIMER_OF_BASE,
.clkevt = {
.name   = "i.MX system counter timer",
.features   = CLOCK_EVT_FEAT_ONESHOT |
@@ -130,6 +130,9 @@ static int __init sysctr_timer_init(struct device_node *np)
 {
int ret = 0;
 
+   to_sysctr.flags |= of_find_property(np, "clock-frequency", NULL) ?
+  TIMER_OF_CLOCK_FREQUENCY : TIMER_OF_CLOCK;
+
ret = timer_of_init(np, _sysctr);
if (ret)
return ret;
-- 
2.7.4



[PATCH RESEND V4 1/5] clocksource: timer-of: Support getting clock frequency from DT

2019-07-02 Thread Anson . Huang
From: Anson Huang 

More and more platforms use platform driver model for clock driver,
so the clock driver is NOT ready during timer initialization phase,
it will cause timer initialization failed.

To support those platforms with upper scenario, introducing a new
flag TIMER_OF_CLOCK_FREQUENCY which is mutually exclusive with
TIMER_OF_CLOCK flag to support getting timer clock frequency from
DT's timer node, the property name should be "clock-frequency",
then of_clk operations can be skipped.

User needs to select either TIMER_OF_CLOCK_FREQUENCY or TIMER_OF_CLOCK
flag if want to use timer-of driver to initialize the clock rate.

Signed-off-by: Anson Huang 
---
Changes since V3:
- use hardcoded "clock-frequency" instead of adding new variable 
prop_name;
- add pre-condition check for TIMER_OF_CLOCK and 
TIMER_OF_CLOCK_FREQUENCY, they MUST be exclusive.
---
 drivers/clocksource/timer-of.c | 29 +
 drivers/clocksource/timer-of.h |  7 ---
 2 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/drivers/clocksource/timer-of.c b/drivers/clocksource/timer-of.c
index 8054228..858f684 100644
--- a/drivers/clocksource/timer-of.c
+++ b/drivers/clocksource/timer-of.c
@@ -161,11 +161,30 @@ static __init int timer_of_base_init(struct device_node 
*np,
return 0;
 }
 
+static __init int timer_of_clk_frequency_init(struct device_node *np,
+ struct of_timer_clk *of_clk)
+{
+   int ret;
+   u32 rate;
+
+   ret = of_property_read_u32(np, "clock-frequency", );
+   if (!ret) {
+   of_clk->rate = rate;
+   of_clk->period = DIV_ROUND_UP(rate, HZ);
+   }
+
+   return ret;
+}
+
 int __init timer_of_init(struct device_node *np, struct timer_of *to)
 {
+   unsigned long clock_flags = TIMER_OF_CLOCK | TIMER_OF_CLOCK_FREQUENCY;
int ret = -EINVAL;
int flags = 0;
 
+   if ((to->flags & clock_flags) == clock_flags)
+   return ret;
+
if (to->flags & TIMER_OF_BASE) {
ret = timer_of_base_init(np, >of_base);
if (ret)
@@ -180,6 +199,13 @@ int __init timer_of_init(struct device_node *np, struct 
timer_of *to)
flags |= TIMER_OF_CLOCK;
}
 
+   if (to->flags & TIMER_OF_CLOCK_FREQUENCY) {
+   ret = timer_of_clk_frequency_init(np, >of_clk);
+   if (ret)
+   goto out_fail;
+   flags |= TIMER_OF_CLOCK_FREQUENCY;
+   }
+
if (to->flags & TIMER_OF_IRQ) {
ret = timer_of_irq_init(np, >of_irq);
if (ret)
@@ -201,6 +227,9 @@ int __init timer_of_init(struct device_node *np, struct 
timer_of *to)
if (flags & TIMER_OF_CLOCK)
timer_of_clk_exit(>of_clk);
 
+   if (flags & TIMER_OF_CLOCK_FREQUENCY)
+   to->of_clk.rate = 0;
+
if (flags & TIMER_OF_BASE)
timer_of_base_exit(>of_base);
return ret;
diff --git a/drivers/clocksource/timer-of.h b/drivers/clocksource/timer-of.h
index a5478f3..a08e108 100644
--- a/drivers/clocksource/timer-of.h
+++ b/drivers/clocksource/timer-of.h
@@ -4,9 +4,10 @@
 
 #include 
 
-#define TIMER_OF_BASE  0x1
-#define TIMER_OF_CLOCK 0x2
-#define TIMER_OF_IRQ   0x4
+#define TIMER_OF_BASE  0x1
+#define TIMER_OF_CLOCK 0x2
+#define TIMER_OF_IRQ   0x4
+#define TIMER_OF_CLOCK_FREQUENCY   0x8
 
 struct of_timer_irq {
int irq;
-- 
2.7.4



[PATCH RESEND V4 5/5] arm64: dts: imx8mm: Add system counter node

2019-07-02 Thread Anson . Huang
From: Anson Huang 

Add i.MX8MM system counter node to enable timer-imx-sysctr
broadcast timer driver.

Signed-off-by: Anson Huang 
---
No changes.
---
 arch/arm64/boot/dts/freescale/imx8mm.dtsi | 8 
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi 
b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index b726712..b11fc5e 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -532,6 +532,14 @@
#pwm-cells = <2>;
status = "disabled";
};
+
+   system_counter: timer@306a {
+   compatible = "nxp,sysctr-timer";
+   reg = <0x306a 0x3>;
+   interrupts = ,
+;
+   clock-frequency = <800>;
+   };
};
 
aips3: bus@3080 {
-- 
2.7.4



[PATCH RESEND V4 4/5] arm64: dts: imx8mq: Add system counter node

2019-07-02 Thread Anson . Huang
From: Anson Huang 

Add i.MX8MQ system counter node to enable timer-imx-sysctr
broadcast timer driver.

Signed-off-by: Anson Huang 
---
No changes.
---
 arch/arm64/boot/dts/freescale/imx8mq.dtsi | 8 
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi 
b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index 50931da..3187428 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -639,6 +639,14 @@
#pwm-cells = <2>;
status = "disabled";
};
+
+   system_counter: timer@306a {
+   compatible = "nxp,sysctr-timer";
+   reg = <0x306a 0x3>;
+   interrupts = ,
+;
+   clock-frequency = <833>;
+   };
};
 
bus@3080 { /* AIPS3 */
-- 
2.7.4



[PATCH RESEND V4 2/5] clocksource/drivers/sysctr: Add clock-frequency property

2019-07-02 Thread Anson . Huang
From: Anson Huang 

Systems which use platform driver model for clock driver require the
clock frequency to be supplied via device tree when system counter
driver is enabled.

This is necessary as in the platform driver model the of_clk operations
do not work correctly because system counter driver is initialized in
early phase of system boot up, and clock driver using platform driver
model is NOT ready at that time, it will cause system counter driver
initialization failed.

Add clock-frequency property to the device tree bindings of the NXP
system counter, so the driver can tell timer-of driver to get clock
frequency from DT directly instead of doing of_clk operations via
clk APIs.

Signed-off-by: Anson Huang 
---
No changes.
---
 .../devicetree/bindings/timer/nxp,sysctr-timer.txt| 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt 
b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
index d576599..7088a0e 100644
--- a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
+++ b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
@@ -11,15 +11,18 @@ Required properties:
 - reg : Specifies the base physical address and size of the comapre
 frame and the counter control, read & compare.
 - interrupts :  should be the first compare frames' interrupt
-- clocks : Specifies the counter clock.
-- clock-names: Specifies the clock's name of this module
+- clocks :  Specifies the counter clock, mutually exclusive with 
clock-frequency.
+- clock-names : Specifies the clock's name of this module, mutually 
exclusive with
+   clock-frequency.
+- clock-frequency : Specifies system counter clock frequency, mutually 
exclusive with
+   clocks/clock-names.
 
 Example:
 
system_counter: timer@306a {
compatible = "nxp,sysctr-timer";
-   reg = <0x306a 0x2>;/* system-counter-rd & compare */
-   clocks = <_8m>;
-   clock-names = "per";
-   interrupts = ;
+   reg = <0x306a 0x3>;
+   interrupts = ,
+;
+   clock-frequency = <833>;
};
-- 
2.7.4



[PATCH V3] soc: imx-scu: Add SoC UID(unique identifier) support

2019-07-02 Thread Anson . Huang
From: Anson Huang 

Add i.MX SCU SoC's UID(unique identifier) support, user
can read it from sysfs:

root@imx8qxpmek:~# cat /sys/devices/soc0/soc_uid
7B64280B57AC1898

Signed-off-by: Anson Huang 
Reviewed-by: Daniel Baluta 
---
Change since V2:
- The SCU FW API for getting UID does NOT have response, so we should 
set
  imx_scu_call_rpc()'s 3rd parameter as false and still can check the 
returned
  value, and comment is no needed any more.
---
 drivers/soc/imx/soc-imx-scu.c | 39 +++
 1 file changed, 39 insertions(+)

diff --git a/drivers/soc/imx/soc-imx-scu.c b/drivers/soc/imx/soc-imx-scu.c
index 676f612..50831eb 100644
--- a/drivers/soc/imx/soc-imx-scu.c
+++ b/drivers/soc/imx/soc-imx-scu.c
@@ -27,6 +27,40 @@ struct imx_sc_msg_misc_get_soc_id {
} data;
 } __packed;
 
+struct imx_sc_msg_misc_get_soc_uid {
+   struct imx_sc_rpc_msg hdr;
+   u32 uid_low;
+   u32 uid_high;
+} __packed;
+
+static ssize_t soc_uid_show(struct device *dev,
+   struct device_attribute *attr, char *buf)
+{
+   struct imx_sc_msg_misc_get_soc_uid msg;
+   struct imx_sc_rpc_msg *hdr = 
+   u64 soc_uid;
+   int ret;
+
+   hdr->ver = IMX_SC_RPC_VERSION;
+   hdr->svc = IMX_SC_RPC_SVC_MISC;
+   hdr->func = IMX_SC_MISC_FUNC_UNIQUE_ID;
+   hdr->size = 1;
+
+   ret = imx_scu_call_rpc(soc_ipc_handle, , false);
+   if (ret) {
+   pr_err("%s: get soc uid failed, ret %d\n", __func__, ret);
+   return ret;
+   }
+
+   soc_uid = msg.uid_high;
+   soc_uid <<= 32;
+   soc_uid |= msg.uid_low;
+
+   return sprintf(buf, "%016llX\n", soc_uid);
+}
+
+static DEVICE_ATTR_RO(soc_uid);
+
 static int imx_scu_soc_id(void)
 {
struct imx_sc_msg_misc_get_soc_id msg;
@@ -102,6 +136,11 @@ static int imx_scu_soc_probe(struct platform_device *pdev)
goto free_revision;
}
 
+   ret = device_create_file(soc_device_to_device(soc_dev),
+_attr_soc_uid);
+   if (ret)
+   goto free_revision;
+
return 0;
 
 free_revision:
-- 
2.7.4



RE: [PATCH V2] soc: imx-scu: Add SoC UID(unique identifier) support

2019-07-02 Thread Anson Huang
Hi, Marco

> > > + hdr->ver = IMX_SC_RPC_VERSION;
> > > + hdr->svc = IMX_SC_RPC_SVC_MISC;
> > > + hdr->func = IMX_SC_MISC_FUNC_UNIQUE_ID;
> > > + hdr->size = 1;
> > > +
> > > + /*
> > > +  * SCU FW API always returns an error even the
> > > +  * function is successfully executed, so skip
> > > +  * returned value check.
> > > +  */
> > > + imx_scu_call_rpc(soc_ipc_handle, , true);
> >
> > Please can you add a TODO: or FIXME: tag and also provide the firmware
> > version containing the bug? I know that developers are very busy and
> > follow- up fixes never reach mainline ;)
> 
> As I replied in previous mail, I will send out a V3 with below comment:
> 
> +   /*
> +* SCU FW API does NOT have returned value for
> +* this function, so skip returned value check.
> +*/
> +   imx_scu_call_rpc(soc_ipc_handle, , true);
> 
> Thanks,
> Anson.

Sorry, after further thought, regarding for SCU API without response, we should
pass the "false" as imx_scu_call_rpc()'s 3rd parameter, so I will remove the 
comment
and use below in V3:

+   ret = imx_scu_call_rpc(soc_ipc_handle, , false);
+   if (ret) {
+   pr_err("%s: get soc uid failed, ret %d\n", __func__, ret);
+   return ret;
+   }

Thanks,
Anson



RE: [PATCH V2] soc: imx-scu: Add SoC UID(unique identifier) support

2019-07-02 Thread Anson Huang
Hi, Marco

> > +   hdr->ver = IMX_SC_RPC_VERSION;
> > +   hdr->svc = IMX_SC_RPC_SVC_MISC;
> > +   hdr->func = IMX_SC_MISC_FUNC_UNIQUE_ID;
> > +   hdr->size = 1;
> > +
> > +   /*
> > +* SCU FW API always returns an error even the
> > +* function is successfully executed, so skip
> > +* returned value check.
> > +*/
> > +   imx_scu_call_rpc(soc_ipc_handle, , true);
> 
> Please can you add a TODO: or FIXME: tag and also provide the firmware
> version containing the bug? I know that developers are very busy and follow-
> up fixes never reach mainline ;)

As I replied in previous mail, I will send out a V3 with below comment:

+   /*
+* SCU FW API does NOT have returned value for
+* this function, so skip returned value check.
+*/
+   imx_scu_call_rpc(soc_ipc_handle, , true);

Thanks,
Anson.

> 
> Regards,
>   Marco



RE: [PATCH] soc: imx-scu: Add SoC UID(unique identifier) support

2019-07-02 Thread Anson Huang
Hi, Marco

> Hi Anson,
> 
> On 19-06-27 07:01, Anson Huang wrote:
> > Hi, Daniel
> >
> > > On Thu, Jun 27, 2019 at 3:48 AM Anson Huang 
> > > wrote:
> > > >
> > > > Hi, Daniel
> > > >
> > > > > On Wed, Jun 26, 2019 at 10:06 AM  wrote:
> > > > > >
> > > > > > From: Anson Huang 
> > > > > >
> > > > > > Add i.MX SCU SoC's UID(unique identifier) support, user can
> > > > > > read it from sysfs:
> > > > > >
> > > > > > root@imx8qxpmek:~# cat /sys/devices/soc0/soc_uid
> > > > > > 7B64280B57AC1898
> > > > > >
> > > > > > Signed-off-by: Anson Huang 
> > > > > > ---
> > > > > >  drivers/soc/imx/soc-imx-scu.c | 35
> > > > > > +++
> > > > > >  1 file changed, 35 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/soc/imx/soc-imx-scu.c
> > > > > > b/drivers/soc/imx/soc-imx-scu.c index 676f612..8d322a1 100644
> > > > > > --- a/drivers/soc/imx/soc-imx-scu.c
> > > > > > +++ b/drivers/soc/imx/soc-imx-scu.c
> > > > > > @@ -27,6 +27,36 @@ struct imx_sc_msg_misc_get_soc_id {
> > > > > > } data;
> > > > > >  } __packed;
> > > > > >
> > > > > > +struct imx_sc_msg_misc_get_soc_uid {
> > > > > > +   struct imx_sc_rpc_msg hdr;
> > > > > > +   u32 uid_low;
> > > > > > +   u32 uid_high;
> > > > > > +} __packed;
> > > > > > +
> > > > > > +static ssize_t soc_uid_show(struct device *dev,
> > > > > > +   struct device_attribute *attr,
> > > > > > +char
> > > > > > +*buf) {
> > > > > > +   struct imx_sc_msg_misc_get_soc_uid msg;
> > > > > > +   struct imx_sc_rpc_msg *hdr = 
> > > > > > +   u64 soc_uid;
> > > > > > +
> > > > > > +   hdr->ver = IMX_SC_RPC_VERSION;
> > > > > > +   hdr->svc = IMX_SC_RPC_SVC_MISC;
> > > > > > +   hdr->func = IMX_SC_MISC_FUNC_UNIQUE_ID;
> > > > > > +   hdr->size = 1;
> > > > > > +
> > > > > > +   /* the return value of SCU FW is in correct, skip
> > > > > > + return value check */
> > > > >
> > > > > Why do you mean by "in correct"?
> > > >
> > > > I made a mistake, it should be "incorrect", the existing SCFW of
> > > > this API returns an error value even this API is successfully
> > > > called, to make it work with current SCFW, I have to skip the
> > > > return value check for this API for now. Will send V2 patch to fix this
> typo.
> > >
> > > Thanks Anson! It makes sense now. It is a little bit sad though
> > > because we won't know when there is a "real" error :).
> > >
> > > Lets update the comment to be more specific:
> > >
> > > /* SCFW FW API always returns an error even the function is
> > > successfully executed, so skip returned value */
> >
> > OK, as for external users, the SCFW formally released has this issue,
> > so for now I have to skip the return value check for this API, once
> > next SCFW release has this issue fixed, I will add a patch to check the 
> > return
> value.
> 
> Do you really keep track of that? Please can you add a FIXME: or TODO:
> tag and add the firmware version containing that bug?

Thanks for reminder, I just double checked the SCU FW code, it is actually a 
mistake, the SCU FW
API of sc_misc_unique_id() is actually a void function, which means it does NOT 
return anything.
While in our internal kernel tree, we make SCU IPC call to sc_misc_unique_id() 
with return value
check, and the return value is failure (-5) always. When I clean up the code 
for upstream, I did NOT notice it.
So the correct comment should be, this API does NOT return anything, no need to 
check the returned value.
I will fix the comment in next version.

void sc_misc_unique_id(sc_ipc_t ipc, uint32_t *id_l, uint32_t *id_h)

Thanks,
Anson

> 
> Regards,
>   Marco
> 
> > Thanks,
> > Anson.
> > >
> > >
> > > > > > +   imx_scu_call_rpc(soc_ipc_handle, , true);
> > > > > > +
> > > > > > +   soc_uid = msg.uid_high;
> > > > > > +   soc_uid <<= 32;
> > > > > > +   soc_uid |= msg.uid_low;
> > > > > > +
> > > > > > +   return sprintf(buf, "%016llX\n", soc_uid);
> > > > >
> > > > > snprintf?
> > > >
> > > > The snprintf is to avoid buffer overflow, which in this case, I
> > > > don't know the size of "buf", and the value(u64) to be printed is
> > > > with fixed length of 64, so I think sprint is just OK.
> > >
> > > Ok.
> 
> --



[PATCH 1/2] arm64: dts: imx8mq: Add gpio-ranges property

2019-07-01 Thread Anson . Huang
From: Anson Huang 

Add "gpio-ranges" property to establish connections between GPIOs
and PINs on i.MX8MQ pinctrl driver.

Signed-off-by: Anson Huang 
---
 arch/arm64/boot/dts/freescale/imx8mq.dtsi | 5 +
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi 
b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index 477c523..3187428 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -287,6 +287,7 @@
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
+   gpio-ranges = < 0 10 30>;
};
 
gpio2: gpio@3021 {
@@ -299,6 +300,7 @@
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
+   gpio-ranges = < 0 40 21>;
};
 
gpio3: gpio@3022 {
@@ -311,6 +313,7 @@
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
+   gpio-ranges = < 0 61 26>;
};
 
gpio4: gpio@3023 {
@@ -323,6 +326,7 @@
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
+   gpio-ranges = < 0 87 32>;
};
 
gpio5: gpio@3024 {
@@ -335,6 +339,7 @@
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
+   gpio-ranges = < 0 119 30>;
};
 
tmu: tmu@3026 {
-- 
2.7.4



[PATCH 2/2] arm64: dts: imx8mm: Add gpio-ranges property

2019-07-01 Thread Anson . Huang
From: Anson Huang 

Add "gpio-ranges" property to establish connections between GPIOs
and PINs on i.MX8MM pinctrl driver.

Signed-off-by: Anson Huang 
---
 arch/arm64/boot/dts/freescale/imx8mm.dtsi | 5 +
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi 
b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index ea15457..b11fc5e 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -312,6 +312,7 @@
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
+   gpio-ranges = < 0 10 30>;
};
 
gpio2: gpio@3021 {
@@ -324,6 +325,7 @@
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
+   gpio-ranges = < 0 40 21>;
};
 
gpio3: gpio@3022 {
@@ -336,6 +338,7 @@
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
+   gpio-ranges = < 0 61 26>;
};
 
gpio4: gpio@3023 {
@@ -348,6 +351,7 @@
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
+   gpio-ranges = < 0 87 32>;
};
 
gpio5: gpio@3024 {
@@ -360,6 +364,7 @@
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
+   gpio-ranges = < 0 119 30>;
};
 
wdog1: watchdog@3028 {
-- 
2.7.4



[PATCH 2/2] arm64: dts: imx8mm: Add "fsl,imx8mq-src" as src's fallback compatible

2019-07-01 Thread Anson . Huang
From: Anson Huang 

i.MX8MM can reuse i.MX8MQ's src driver, add "fsl,imx8mq-src" as
src's fallback compatible to enable it.

Signed-off-by: Anson Huang 
---
 arch/arm64/boot/dts/freescale/imx8mm.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi 
b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index f0ac027..ea15457 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -471,7 +471,7 @@
};
 
src: reset-controller@3039 {
-   compatible = "fsl,imx8mm-src", "syscon";
+   compatible = "fsl,imx8mm-src", 
"fsl,imx8mq-src", "syscon";
reg = <0x3039 0x1>;
interrupts = ;
#reset-cells = <1>;
-- 
2.7.4



[PATCH 1/2] reset: imx7: Add support for i.MX8MM SoC

2019-07-01 Thread Anson . Huang
From: Anson Huang 

i.MX8MM SoC has a subset of i.MX8MQ IP block variant, it can reuse
the i.MX8MQ reset controller driver and just skip those non-existing
IP blocks, add support for i.MX8MM SoC reset control.

Signed-off-by: Anson Huang 
---
 drivers/reset/reset-imx7.c | 20 
 1 file changed, 20 insertions(+)

diff --git a/drivers/reset/reset-imx7.c b/drivers/reset/reset-imx7.c
index 3ecd770..941131f 100644
--- a/drivers/reset/reset-imx7.c
+++ b/drivers/reset/reset-imx7.c
@@ -207,6 +207,26 @@ static int imx8mq_reset_set(struct reset_controller_dev 
*rcdev,
const unsigned int bit = imx7src->signals[id].bit;
unsigned int value = assert ? bit : 0;
 
+   if (of_machine_is_compatible("fsl,imx8mm")) {
+   switch (id) {
+   case IMX8MQ_RESET_HDMI_PHY_APB_RESET:
+   case IMX8MQ_RESET_PCIEPHY2: /* fallthrough */
+   case IMX8MQ_RESET_PCIEPHY2_PERST: /* fallthrough */
+   case IMX8MQ_RESET_PCIE2_CTRL_APPS_EN: /* fallthrough */
+   case IMX8MQ_RESET_PCIE2_CTRL_APPS_TURNOFF: /* fallthrough */
+   case IMX8MQ_RESET_MIPI_CSI1_CORE_RESET: /* fallthrough */
+   case IMX8MQ_RESET_MIPI_CSI1_PHY_REF_RESET: /* fallthrough */
+   case IMX8MQ_RESET_MIPI_CSI1_ESC_RESET: /* fallthrough */
+   case IMX8MQ_RESET_MIPI_CSI2_CORE_RESET: /* fallthrough */
+   case IMX8MQ_RESET_MIPI_CSI2_PHY_REF_RESET: /* fallthrough */
+   case IMX8MQ_RESET_MIPI_CSI2_ESC_RESET: /* fallthrough */
+   case IMX8MQ_RESET_DDRC2_PHY_RESET: /* fallthrough */
+   case IMX8MQ_RESET_DDRC2_CORE_RESET: /* fallthrough */
+   case IMX8MQ_RESET_DDRC2_PRST: /* fallthrough */
+   return 0;
+   }
+   }
+
switch (id) {
case IMX8MQ_RESET_PCIEPHY:
case IMX8MQ_RESET_PCIEPHY2: /* fallthrough */
-- 
2.7.4



[PATCH V4 4/5] arm64: dts: imx8mq: Add system counter node

2019-07-01 Thread Anson . Huang
From: Anson Huang 

Add i.MX8MQ system counter node to enable timer-imx-sysctr
broadcast timer driver.

Signed-off-by: Anson Huang 
---
No changes.
---
 arch/arm64/boot/dts/freescale/imx8mq.dtsi | 8 
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi 
b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index 2ea600c..477c523 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -634,6 +634,14 @@
#pwm-cells = <2>;
status = "disabled";
};
+
+   system_counter: timer@306a {
+   compatible = "nxp,sysctr-timer";
+   reg = <0x306a 0x3>;
+   interrupts = ,
+;
+   clock-frequency = <833>;
+   };
};
 
bus@3080 { /* AIPS3 */
-- 
2.7.4



[PATCH V4 3/5] clocksource: imx-sysctr: Make timer work with clock driver using platform driver model

2019-07-01 Thread Anson . Huang
From: Anson Huang 

On some i.MX8M platforms, clock driver uses platform driver
model and it is NOT ready during timer initialization phase,
the clock operations will fail and system counter driver will
fail too. As all the i.MX8M platforms' system counter clock
are from OSC which is always enabled, so it is no need to enable
clock for system counter driver, the ONLY thing is to pass
clock frequence to driver.

To make system counter driver work for upper scenario, if DT's
system counter node has property "clock-frequency" present,
setting TIMER_OF_CLOCK_FREQUENCY flag to indicate timer-of driver
to get clock frequency from DT directly instead of of_clk operation
via clk APIs.

Signed-off-by: Anson Huang 
---
Changes since V3:
- remove the .prop_name initialization acording to timer-of changes in 
V4.
---
 drivers/clocksource/timer-imx-sysctr.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/clocksource/timer-imx-sysctr.c 
b/drivers/clocksource/timer-imx-sysctr.c
index fd7d680..b82a549 100644
--- a/drivers/clocksource/timer-imx-sysctr.c
+++ b/drivers/clocksource/timer-imx-sysctr.c
@@ -98,7 +98,7 @@ static irqreturn_t sysctr_timer_interrupt(int irq, void 
*dev_id)
 }
 
 static struct timer_of to_sysctr = {
-   .flags = TIMER_OF_IRQ | TIMER_OF_CLOCK | TIMER_OF_BASE,
+   .flags = TIMER_OF_IRQ | TIMER_OF_BASE,
.clkevt = {
.name   = "i.MX system counter timer",
.features   = CLOCK_EVT_FEAT_ONESHOT |
@@ -130,6 +130,9 @@ static int __init sysctr_timer_init(struct device_node *np)
 {
int ret = 0;
 
+   to_sysctr.flags |= of_find_property(np, "clock-frequency", NULL) ?
+  TIMER_OF_CLOCK_FREQUENCY : TIMER_OF_CLOCK;
+
ret = timer_of_init(np, _sysctr);
if (ret)
return ret;
-- 
2.7.4



[PATCH V4 5/5] arm64: dts: imx8mm: Add system counter node

2019-07-01 Thread Anson . Huang
From: Anson Huang 

Add i.MX8MM system counter node to enable timer-imx-sysctr
broadcast timer driver.

Signed-off-by: Anson Huang 
---
No changes.
---
 arch/arm64/boot/dts/freescale/imx8mm.dtsi | 8 
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi 
b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index 9d3bb35..ea15457 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -527,6 +527,14 @@
#pwm-cells = <2>;
status = "disabled";
};
+
+   system_counter: timer@306a {
+   compatible = "nxp,sysctr-timer";
+   reg = <0x306a 0x3>;
+   interrupts = ,
+;
+   clock-frequency = <800>;
+   };
};
 
aips3: bus@3080 {
-- 
2.7.4



[PATCH V4 2/5] clocksource/drivers/sysctr: Add clock-frequency property

2019-07-01 Thread Anson . Huang
From: Anson Huang 

Systems which use platform driver model for clock driver require the
clock frequency to be supplied via device tree when system counter
driver is enabled.

This is necessary as in the platform driver model the of_clk operations
do not work correctly because system counter driver is initialized in
early phase of system boot up, and clock driver using platform driver
model is NOT ready at that time, it will cause system counter driver
initialization failed.

Add clock-frequency property to the device tree bindings of the NXP
system counter, so the driver can tell timer-of driver to get clock
frequency from DT directly instead of doing of_clk operations via
clk APIs.

Signed-off-by: Anson Huang 
---
No change.
---
 .../devicetree/bindings/timer/nxp,sysctr-timer.txt| 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt 
b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
index d576599..7088a0e 100644
--- a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
+++ b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
@@ -11,15 +11,18 @@ Required properties:
 - reg : Specifies the base physical address and size of the comapre
 frame and the counter control, read & compare.
 - interrupts :  should be the first compare frames' interrupt
-- clocks : Specifies the counter clock.
-- clock-names: Specifies the clock's name of this module
+- clocks :  Specifies the counter clock, mutually exclusive with 
clock-frequency.
+- clock-names : Specifies the clock's name of this module, mutually 
exclusive with
+   clock-frequency.
+- clock-frequency : Specifies system counter clock frequency, mutually 
exclusive with
+   clocks/clock-names.
 
 Example:
 
system_counter: timer@306a {
compatible = "nxp,sysctr-timer";
-   reg = <0x306a 0x2>;/* system-counter-rd & compare */
-   clocks = <_8m>;
-   clock-names = "per";
-   interrupts = ;
+   reg = <0x306a 0x3>;
+   interrupts = ,
+;
+   clock-frequency = <833>;
};
-- 
2.7.4



[PATCH V4 1/5] clocksource: timer-of: Support getting clock frequency from DT

2019-07-01 Thread Anson . Huang
From: Anson Huang 

More and more platforms use platform driver model for clock driver,
so the clock driver is NOT ready during timer initialization phase,
it will cause timer initialization failed.

To support those platforms with upper scenario, introducing a new
flag TIMER_OF_CLOCK_FREQUENCY which is mutually exclusive with
TIMER_OF_CLOCK flag to support getting timer clock frequency from
DT's timer node, the property name should be "clock-frequency",
then of_clk operations can be skipped.

User needs to select either TIMER_OF_CLOCK_FREQUENCY or TIMER_OF_CLOCK
flag if want to use timer-of driver to initialize the clock rate.

Signed-off-by: Anson Huang 
---
Changes since V3:
- use hardcoded "clock-frequency" instead of adding new variable 
prop_name;
- add pre-condition check for TIMER_OF_CLOCK and 
TIMER_OF_CLOCK_FREQUENCY, they MUST be exclusive.
---
 drivers/clocksource/timer-of.c | 29 +
 drivers/clocksource/timer-of.h |  7 ---
 2 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/drivers/clocksource/timer-of.c b/drivers/clocksource/timer-of.c
index 8054228..ab155cc 100644
--- a/drivers/clocksource/timer-of.c
+++ b/drivers/clocksource/timer-of.c
@@ -161,11 +161,30 @@ static __init int timer_of_base_init(struct device_node 
*np,
return 0;
 }
 
+static __init int timer_of_clk_frequency_init(struct device_node *np,
+ struct of_timer_clk *of_clk)
+{
+   int ret;
+   u32 rate;
+
+   ret = of_property_read_u32(np, "clock-frequency", );
+   if (!ret) {
+   of_clk->rate = rate;
+   of_clk->period = DIV_ROUND_UP(rate, HZ);
+   }
+
+   return ret;
+}
+
 int __init timer_of_init(struct device_node *np, struct timer_of *to)
 {
+   unsigned long clock_flags = TIMER_OF_CLOCK | TIMER_OF_CLOCK_FREQUENCY;
int ret = -EINVAL;
int flags = 0;
 
+   if (to->flags & clock_flags == clock_flags)
+   return ret;
+
if (to->flags & TIMER_OF_BASE) {
ret = timer_of_base_init(np, >of_base);
if (ret)
@@ -180,6 +199,13 @@ int __init timer_of_init(struct device_node *np, struct 
timer_of *to)
flags |= TIMER_OF_CLOCK;
}
 
+   if (to->flags & TIMER_OF_CLOCK_FREQUENCY) {
+   ret = timer_of_clk_frequency_init(np, >of_clk);
+   if (ret)
+   goto out_fail;
+   flags |= TIMER_OF_CLOCK_FREQUENCY;
+   }
+
if (to->flags & TIMER_OF_IRQ) {
ret = timer_of_irq_init(np, >of_irq);
if (ret)
@@ -201,6 +227,9 @@ int __init timer_of_init(struct device_node *np, struct 
timer_of *to)
if (flags & TIMER_OF_CLOCK)
timer_of_clk_exit(>of_clk);
 
+   if (flags & TIMER_OF_CLOCK_FREQUENCY)
+   to->of_clk.rate = 0;
+
if (flags & TIMER_OF_BASE)
timer_of_base_exit(>of_base);
return ret;
diff --git a/drivers/clocksource/timer-of.h b/drivers/clocksource/timer-of.h
index a5478f3..a08e108 100644
--- a/drivers/clocksource/timer-of.h
+++ b/drivers/clocksource/timer-of.h
@@ -4,9 +4,10 @@
 
 #include 
 
-#define TIMER_OF_BASE  0x1
-#define TIMER_OF_CLOCK 0x2
-#define TIMER_OF_IRQ   0x4
+#define TIMER_OF_BASE  0x1
+#define TIMER_OF_CLOCK 0x2
+#define TIMER_OF_IRQ   0x4
+#define TIMER_OF_CLOCK_FREQUENCY   0x8
 
 struct of_timer_irq {
int irq;
-- 
2.7.4



RE: [PATCH V3 1/5] clocksource: timer-of: Support getting clock frequency from DT

2019-07-01 Thread Anson Huang
Hi, Daniel

> Subject: Re: [PATCH V3 1/5] clocksource: timer-of: Support getting clock
> frequency from DT
> 
> 
> Hi Anson,
> 
> thanks for taking care of adding the clock-frequency handling in the timer-of.

Sure.

> 
> On 28/06/2019 05:30, anson.hu...@nxp.com wrote:
> > From: Anson Huang 
> >
> > More and more platforms use platform driver model for clock driver, so
> > the clock driver is NOT ready during timer initialization phase, it
> > will cause timer initialization failed.
> >
> > To support those platforms with upper scenario, introducing a new flag
> > TIMER_OF_CLOCK_FREQUENCY which is mutually exclusive with
> > TIMER_OF_CLOCK flag to support getting timer clock frequency from DT,
> > then of_clk operations can be skipped.
> >
> > User needs to select either TIMER_OF_CLOCK_FREQUENCY or
> TIMER_OF_CLOCK
> > flag if want to use timer-of driver to initialize the clock rate, and
> > the corresponding clock name or property name needs to be specified.
> >
> > Signed-off-by: Anson Huang 
> > ---
> > New patch:
> > - Add new flag of TIMER_OF_CLOCK_FREQUENCY, mutually exclusive
> with TIMER_OF_CLOCK, to support
> >   getting clock frequency from DT directly;
> > - Add prop_name to of_timer_clk structure, if using
> TIMER_OF_CLOCK_FREQUENCY flag, user needs
> >   to pass the property name for timer-of driver to get clock frequency
> from DT, this is to avoid
> >   the couple of timer-of driver and DT, so timer-of driver does NOT
> use a fixed property name.
> > ---
> >  drivers/clocksource/timer-of.c | 23 +++
> > drivers/clocksource/timer-of.h |  8 +---
> >  2 files changed, 28 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/clocksource/timer-of.c
> > b/drivers/clocksource/timer-of.c index 8054228..c91a8b6 100644
> > --- a/drivers/clocksource/timer-of.c
> > +++ b/drivers/clocksource/timer-of.c
> > @@ -161,6 +161,21 @@ static __init int timer_of_base_init(struct
> device_node *np,
> > return 0;
> >  }
> >
> > +static __init int timer_of_clk_frequency_init(struct device_node *np,
> > + struct of_timer_clk *of_clk) {
> > +   int ret;
> > +   u32 rate;
> > +
> > +   ret = of_property_read_u32(np, of_clk->prop_name, );
> > +   if (!ret) {
> > +   of_clk->rate = rate;
> > +   of_clk->period = DIV_ROUND_UP(rate, HZ);
> > +   }
> > +
> > +   return ret;
> > +}
> > +
> >  int __init timer_of_init(struct device_node *np, struct timer_of *to)
> > {
> > int ret = -EINVAL;
> > @@ -178,6 +193,11 @@ int __init timer_of_init(struct device_node *np,
> struct timer_of *to)
> > if (ret)
> > goto out_fail;
> > flags |= TIMER_OF_CLOCK;
> > +   } else if (to->flags & TIMER_OF_CLOCK_FREQUENCY) {
> > +   ret = timer_of_clk_frequency_init(np, >of_clk);
> > +   if (ret)
> > +   goto out_fail;
> > +   flags |= TIMER_OF_CLOCK_FREQUENCY;
> > }
> 
> /* Pre-condition */
> 
> if (to->flags & (TIMER_OF_CLOCK | TIMER_OF_CLOCK_FREQUENCY))
>   return -EINVAL;
> 
> [ ... ]
> 
> if (to->flags & TIMER_OF_CLOCK) {
> }
> 
> if (to->flags & TIMER_OF_CLOCK_FREQUENCY) { }
> 

Ah, make sense, they are exclusive, will add it in next version.

> > if (to->flags & TIMER_OF_IRQ) {
> > @@ -201,6 +221,9 @@ int __init timer_of_init(struct device_node *np,
> struct timer_of *to)
> > if (flags & TIMER_OF_CLOCK)
> > timer_of_clk_exit(>of_clk);
> >
> > +   if (flags & TIMER_OF_CLOCK_FREQUENCY)
> > +   to->of_clk.rate = 0;
> > +
> > if (flags & TIMER_OF_BASE)
> > timer_of_base_exit(>of_base);
> > return ret;
> > diff --git a/drivers/clocksource/timer-of.h
> > b/drivers/clocksource/timer-of.h index a5478f3..f1a083e 100644
> > --- a/drivers/clocksource/timer-of.h
> > +++ b/drivers/clocksource/timer-of.h
> > @@ -4,9 +4,10 @@
> >
> >  #include 
> >
> > -#define TIMER_OF_BASE  0x1
> > -#define TIMER_OF_CLOCK 0x2
> > -#define TIMER_OF_IRQ   0x4
> > +#define TIMER_OF_BASE  0x1
> > +#define TIMER_OF_CLOCK 0x2
> > +#define TIMER_OF_IRQ   0x4
> > +#define TIMER_OF_CLOCK_FREQUENCY   0x8
> >
> >  struct of_timer_irq {
> > int irq;
> > @@ -26,6 +27,7 @@ struct of_timer_base {  struct of_timer_clk {
> > struct clk *clk;
> > const char *name;
> > +   const char *prop_name;
> 
> For the moment, keep it hardcoded with "clock-frequency" directly in the
> function.

OK, then I will NOT add any dt-binding for this property. The reason to use 
prop_name
instead of hardcode is I don't want to create a binding doc just for this 
property.

Thanks,
Anson. 



[PATCH V2 1/2] arm64: dts: imx8mm: Correct OPP table according to latest datasheet

2019-06-29 Thread Anson . Huang
From: Anson Huang 

According to latest datasheet (Rev.0.2, 04/2019) from below links,
1.8GHz is ONLY available for consumer part, so the market segment
bits for 1.8GHz opp should ONLY available for consumer part accordingly.

https://www.nxp.com/docs/en/data-sheet/IMX8MMIEC.pdf
https://www.nxp.com/docs/en/data-sheet/IMX8MMCEC.pdf

Fixes: f403a26c865b (arm64: dts: imx8mm: Add cpu speed grading and all OPPs)
Signed-off-by: Anson Huang 
---
Changes since V1:
- remove the comment to avoid any confusion.
---
 arch/arm64/boot/dts/freescale/imx8mm.dtsi | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi 
b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index 70de15c..f0ac027 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -134,8 +134,7 @@
opp-18 {
opp-hz = /bits/ 64 <18>;
opp-microvolt = <100>;
-   /* Consumer only but rely on speed grading */
-   opp-supported-hw = <0x8>, <0x7>;
+   opp-supported-hw = <0x8>, <0x3>;
clock-latency-ns = <15>;
};
};
-- 
2.7.4



[PATCH V2 2/2] arm64: dts: imx8mq: Correct OPP table according to latest datasheet

2019-06-29 Thread Anson . Huang
From: Anson Huang 

According to latest datasheet (Rev.1, 10/2018) from below links,
in the consumer datasheet, 1.5GHz is mentioned as highest opp but
depends on speed grading fuse, and in the industrial datasheet,
1.3GHz is mentioned as highest opp but depends on speed grading
fuse. 1.5GHz and 1.3GHz opp use same voltage, so no need for
consumer part to support 1.3GHz opp, with same voltage, CPU should
run at highest frequency in order to go into idle as quick as
possible, this can save power.

That means for consumer part, 1GHz/1.5GHz are supported, for
industrial part, 800MHz/1.3GHz are supported, and then check the
speed grading fuse to limit the highest CPU frequency further.
Correct the market segment bits in opp table to make them work
according to datasheets.

https://www.nxp.com/docs/en/data-sheet/IMX8MDQLQIEC.pdf
https://www.nxp.com/docs/en/data-sheet/IMX8MDQLQCEC.pdf

Fixes: 12629c5c3749 ("arm64: dts: imx8mq: Add cpu speed grading and all OPPs")
Signed-off-by: Anson Huang 
---
Changes since V1:
- remove the comment to avoid any confusion.
---
 arch/arm64/boot/dts/freescale/imx8mq.dtsi | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi 
b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index 9d99191..477c523 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -169,15 +169,14 @@
opp-13 {
opp-hz = /bits/ 64 <13>;
opp-microvolt = <100>;
-   opp-supported-hw = <0xc>, <0x7>;
+   opp-supported-hw = <0xc>, <0x4>;
clock-latency-ns = <15>;
};
 
opp-15 {
opp-hz = /bits/ 64 <15>;
opp-microvolt = <100>;
-   /* Consumer only but rely on speed grading */
-   opp-supported-hw = <0x8>, <0x7>;
+   opp-supported-hw = <0x8>, <0x3>;
clock-latency-ns = <15>;
};
};
-- 
2.7.4



RE: [PATCH 1/2] arm64: dts: imx8mq: Correct OPP table according to latest datasheet

2019-06-28 Thread Anson Huang


> -Original Message-
> From: Leonard Crestez
> Sent: Friday, June 28, 2019 2:45 PM
> To: Anson Huang ; Jacky Bai ;
> l.st...@pengutronix.de
> Cc: shawn...@kernel.org; robh...@kernel.org; mark.rutl...@arm.com;
> s.ha...@pengutronix.de; ker...@pengutronix.de; feste...@gmail.com;
> viresh.ku...@linaro.org; Daniel Baluta ; Abel
> Vesa ; andrew.smir...@gmail.com;
> ccai...@baylibre.com; an...@akkea.ca; a...@sigxcpu.org;
> devicet...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux-
> ker...@vger.kernel.org; dl-linux-imx 
> Subject: Re: [PATCH 1/2] arm64: dts: imx8mq: Correct OPP table according to
> latest datasheet
> 
> On 6/28/2019 9:16 AM, Anson Huang wrote:
> >> From: Leonard Crestez
> >>> From: Anson Huang 
> >>>
> >>> According to latest datasheet (Rev.1, 10/2018) from below links, in
> >>> the consumer datasheet, 1.5GHz is mentioned as highest opp but
> >>> depends on speed grading fuse, and in the industrial datasheet,
> >>> 1.3GHz is mentioned as highest opp but depends on speed grading
> >>> fuse. 1.5GHz and 1.3GHz opp use same voltage, so no need for
> >>> consumer part to support 1.3GHz opp, with same voltage, CPU should
> >>> run at highest frequency in order to go into idle as quick as possible, 
> >>> this
> can save power.
> >>
> >> I looked at the same datasheets and it's not clear to me that 1.3 Ghz
> >> should be disallowed for consumer parts. Power consumption increases
> >> with both voltage and frequency so having two OPPs with same voltage
> >> does make sense.
> >
> > The consumer part datasheet does NOT mention 1.3GHz at all, so
> > consumer part ONLY support 1GHz/1.5GHz, and industrial part ONLY
> > support 800MHz/1.3GHz, this is what we did in our internal tree and
> > NPI release, so better to make them aligned, otherwise, we have to change
> it when kernel upgrade.
> 
> Datasheet seems ambiguous: it mentions "max freq for volt" so my
> understanding is that any lower freqs should also work at that voltage.
> 
> This also seems to be the understanding behind commit 8cfd813c7307
> ("arm64: dts: imx8mq: fix higher CPU operating point") by Lucas. 
> 
> On datasheet page 7 it mentions that product code can have "JZ" or "HZ"
> for 1.3Ghz or 1.5Ghz. Are you saying that only industrial parts can be "JZ"?

If take a look at page 6 table2, industrial datasheet ONLY has "HZ", and 
consumer
Datasheet ONLY has "JZ". And yes, that is my understanding.

And considering our rule, I don't think is benefit for consumer part to run 
1.3GHz.

> 
> >>>   opp-hz = /bits/ 64 <15>;
> >>>   opp-microvolt = <100>;
> >>>   /* Consumer only but rely on speed grading */
> >>> - opp-supported-hw = <0x8>, <0x7>;
> >>> + opp-supported-hw = <0x8>, <0x3>;
> >>
> >> If you don't want to rely on the fact that only consumer parts should
> >> be fused for 1.5 Ghz then please delete the comment.
> >
> > Don't quite understand, 1.5GHz is indeed consumer ONLY, but if the
> > consumer part is fused to 1GHz, then 1.5GHz is also NOT available, so
> > it also rely on speed grading. So keep the comment there is OK?
> 
> What I meant with that comment is that 1.5Ghz is only mentioned for
> consumer parts but instead of explicitly banning it on industrial parts we 
> rely
> on MFG never fusing industrial parts for 1.5Ghz.
> 
> Now you're explicitly banning it on industrial parts.

Yes, industrial parts never support up to 1.5GHz, so explicitly banning it is 
just OK.
The speed grading fuse and market segment fuse are actually has some overlap,
we better to implement both of them.

> 
> This comment is indeed confusing so better to just remove all instances.

I agree to remove those comments, no need to let it introduce any confusion.

Anson.


RE: [PATCH 2/2] arm64: dts: imx8mm: Correct OPP table according to latest datasheet

2019-06-28 Thread Anson Huang
Hi, Leonard

> -Original Message-
> From: Leonard Crestez
> Sent: Friday, June 28, 2019 2:01 PM
> To: Anson Huang ; Jacky Bai ;
> l.st...@pengutronix.de
> Cc: robh...@kernel.org; mark.rutl...@arm.com; shawn...@kernel.org;
> s.ha...@pengutronix.de; ker...@pengutronix.de; feste...@gmail.com;
> viresh.ku...@linaro.org; Daniel Baluta ; Abel
> Vesa ; andrew.smir...@gmail.com;
> ccai...@baylibre.com; an...@akkea.ca; a...@sigxcpu.org;
> devicet...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux-
> ker...@vger.kernel.org; dl-linux-imx 
> Subject: Re: [PATCH 2/2] arm64: dts: imx8mm: Correct OPP table according to
> latest datasheet
> 
> On 28.06.2019 06:37, anson.hu...@nxp.com wrote:
> 
> > According to latest datasheet (Rev.0.2, 04/2019) from below links,
> > 1.8GHz is ONLY available for consumer part, so the market segment bits
> > for 1.8GHz opp should ONLY available for consumer part accordingly.
> >
> > opp-hz = /bits/ 64 <18>;
> > opp-microvolt = <100>;
> > /* Consumer only but rely on speed grading */
> > -   opp-supported-hw = <0x8>, <0x7>;
> > +   opp-supported-hw = <0x8>, <0x3>;
> 
> Only consumer parts should be fused for this highest OPP. If you don't want
> to rely on this then maybe also delete the comment above?

As I replied in previous i.MX8MQ patch, if the comments make reader confused,
should we just remove all those comments?

Thanks,
Anson.

> 
> --
> Regards,
> leonard


RE: [PATCH 1/2] arm64: dts: imx8mq: Correct OPP table according to latest datasheet

2019-06-28 Thread Anson Huang
Hi, Leonard

> -Original Message-
> From: Leonard Crestez
> Sent: Friday, June 28, 2019 1:59 PM
> To: Anson Huang ; shawn...@kernel.org; Jacky
> Bai ; l.st...@pengutronix.de
> Cc: robh...@kernel.org; mark.rutl...@arm.com; s.ha...@pengutronix.de;
> ker...@pengutronix.de; feste...@gmail.com; viresh.ku...@linaro.org;
> Daniel Baluta ; Abel Vesa ;
> andrew.smir...@gmail.com; ccai...@baylibre.com; an...@akkea.ca;
> a...@sigxcpu.org; devicet...@vger.kernel.org; linux-arm-
> ker...@lists.infradead.org; linux-kernel@vger.kernel.org; dl-linux-imx
> 
> Subject: Re: [PATCH 1/2] arm64: dts: imx8mq: Correct OPP table according to
> latest datasheet
> 
> On 28.06.2019 06:37, anson.hu...@nxp.com wrote:
> > From: Anson Huang 
> >
> > According to latest datasheet (Rev.1, 10/2018) from below links, in
> > the consumer datasheet, 1.5GHz is mentioned as highest opp but depends
> > on speed grading fuse, and in the industrial datasheet, 1.3GHz is
> > mentioned as highest opp but depends on speed grading fuse. 1.5GHz and
> > 1.3GHz opp use same voltage, so no need for consumer part to support
> > 1.3GHz opp, with same voltage, CPU should run at highest frequency in
> > order to go into idle as quick as possible, this can save power.
> 
> I looked at the same datasheets and it's not clear to me that 1.3 Ghz should
> be disallowed for consumer parts. Power consumption increases with both
> voltage and frequency so having two OPPs with same voltage does make
> sense.

The consumer part datasheet does NOT mention 1.3GHz at all, so consumer part 
ONLY
support 1GHz/1.5GHz, and industrial part ONLY support 800MHz/1.3GHz, this is 
what
we did in our internal tree and NPI release, so better to make them aligned, 
otherwise,
we have to change it when kernel upgrade.

And normally, with same voltage, i.MX SoCs always run at highest frequency, so 
it is better
to keep the rule, otherwise customer may ask, how about using same voltage to 
run at 1.2GHz or
1.1GHz?

> 
> > opp-hz = /bits/ 64 <13>;
> > opp-microvolt = <100>;
> > -   opp-supported-hw = <0xc>, <0x7>;
> > +   /* Industrial only but rely on speed grading */
> > +   opp-supported-hw = <0xc>, <0x4>;
> 
> Comment is false, you're explicitly excluding consumer parts via the second
> element.

Yes, that is what I meant to do, as we no need to support 1.3GHz for consumer
part, with 1.0V, consumer part can run up to 1.5GHz.

> 
> > opp-hz = /bits/ 64 <15>;
> > opp-microvolt = <100>;
> > /* Consumer only but rely on speed grading */
> > -   opp-supported-hw = <0x8>, <0x7>;
> > +   opp-supported-hw = <0x8>, <0x3>;
> 
> If you don't want to rely on the fact that only consumer parts should be
> fused for 1.5 Ghz then please delete the comment.

Don't quite understand, 1.5GHz is indeed consumer ONLY, but if the consumer
part is fused to 1GHz, then 1.5GHz is also NOT available, so it also rely on 
speed
grading. So keep the comment there is OK?

Thanks,
Anson.



[PATCH V3 4/5] arm64: dts: imx8mq: Add system counter node

2019-06-27 Thread Anson . Huang
From: Anson Huang 

Add i.MX8MQ system counter node to enable timer-imx-sysctr
broadcast timer driver.

Signed-off-by: Anson Huang 
---
No changes.
---
 arch/arm64/boot/dts/freescale/imx8mq.dtsi | 8 
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi 
b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index b1114a6..bea53bc 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -636,6 +636,14 @@
#pwm-cells = <2>;
status = "disabled";
};
+
+   system_counter: timer@306a {
+   compatible = "nxp,sysctr-timer";
+   reg = <0x306a 0x3>;
+   interrupts = ,
+;
+   clock-frequency = <833>;
+   };
};
 
bus@3080 { /* AIPS3 */
-- 
2.7.4



[PATCH V3 3/5] clocksource: imx-sysctr: Make timer work with clock driver using platform driver model

2019-06-27 Thread Anson . Huang
From: Anson Huang 

On some i.MX8M platforms, clock driver uses platform driver
model and it is NOT ready during timer initialization phase,
the clock operations will fail and system counter driver will
fail too. As all the i.MX8M platforms' system counter clock
are from OSC which is always enabled, so it is no need to enable
clock for system counter driver, the ONLY thing is to pass
clock frequence to driver.

To make system counter driver work for upper scenario, if DT's
system counter node has property "clock-frequency" present,
setting TIMER_OF_CLOCK_FREQUENCY flag to indicate timer-of driver
to get clock frequency from DT directly instead of of_clk operation
via clk APIs.

Signed-off-by: Anson Huang 
---
Changes since V2:
- do runtime check to decide whether using TIMER_OF_CLOCK_FREQUENCY or 
TIMER_OF_CLOCK
  according to DT node settings.
---
 drivers/clocksource/timer-imx-sysctr.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/clocksource/timer-imx-sysctr.c 
b/drivers/clocksource/timer-imx-sysctr.c
index fd7d680..73e3193 100644
--- a/drivers/clocksource/timer-imx-sysctr.c
+++ b/drivers/clocksource/timer-imx-sysctr.c
@@ -98,7 +98,7 @@ static irqreturn_t sysctr_timer_interrupt(int irq, void 
*dev_id)
 }
 
 static struct timer_of to_sysctr = {
-   .flags = TIMER_OF_IRQ | TIMER_OF_CLOCK | TIMER_OF_BASE,
+   .flags = TIMER_OF_IRQ | TIMER_OF_BASE,
.clkevt = {
.name   = "i.MX system counter timer",
.features   = CLOCK_EVT_FEAT_ONESHOT |
@@ -114,6 +114,7 @@ static struct timer_of to_sysctr = {
},
.of_clk = {
.name = "per",
+   .prop_name = "clock-frequency",
},
 };
 
@@ -130,6 +131,9 @@ static int __init sysctr_timer_init(struct device_node *np)
 {
int ret = 0;
 
+   to_sysctr.flags |= of_find_property(np, "clock-frequency", NULL) ?
+  TIMER_OF_CLOCK_FREQUENCY : TIMER_OF_CLOCK;
+
ret = timer_of_init(np, _sysctr);
if (ret)
return ret;
-- 
2.7.4



[PATCH V3 1/5] clocksource: timer-of: Support getting clock frequency from DT

2019-06-27 Thread Anson . Huang
From: Anson Huang 

More and more platforms use platform driver model for clock driver,
so the clock driver is NOT ready during timer initialization phase,
it will cause timer initialization failed.

To support those platforms with upper scenario, introducing a new
flag TIMER_OF_CLOCK_FREQUENCY which is mutually exclusive with
TIMER_OF_CLOCK flag to support getting timer clock frequency from
DT, then of_clk operations can be skipped.

User needs to select either TIMER_OF_CLOCK_FREQUENCY or TIMER_OF_CLOCK
flag if want to use timer-of driver to initialize the clock rate,
and the corresponding clock name or property name needs to be specified.

Signed-off-by: Anson Huang 
---
New patch:
- Add new flag of TIMER_OF_CLOCK_FREQUENCY, mutually exclusive with 
TIMER_OF_CLOCK, to support
  getting clock frequency from DT directly;
- Add prop_name to of_timer_clk structure, if using 
TIMER_OF_CLOCK_FREQUENCY flag, user needs
  to pass the property name for timer-of driver to get clock frequency 
from DT, this is to avoid
  the couple of timer-of driver and DT, so timer-of driver does NOT use 
a fixed property name.
---
 drivers/clocksource/timer-of.c | 23 +++
 drivers/clocksource/timer-of.h |  8 +---
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/drivers/clocksource/timer-of.c b/drivers/clocksource/timer-of.c
index 8054228..c91a8b6 100644
--- a/drivers/clocksource/timer-of.c
+++ b/drivers/clocksource/timer-of.c
@@ -161,6 +161,21 @@ static __init int timer_of_base_init(struct device_node 
*np,
return 0;
 }
 
+static __init int timer_of_clk_frequency_init(struct device_node *np,
+ struct of_timer_clk *of_clk)
+{
+   int ret;
+   u32 rate;
+
+   ret = of_property_read_u32(np, of_clk->prop_name, );
+   if (!ret) {
+   of_clk->rate = rate;
+   of_clk->period = DIV_ROUND_UP(rate, HZ);
+   }
+
+   return ret;
+}
+
 int __init timer_of_init(struct device_node *np, struct timer_of *to)
 {
int ret = -EINVAL;
@@ -178,6 +193,11 @@ int __init timer_of_init(struct device_node *np, struct 
timer_of *to)
if (ret)
goto out_fail;
flags |= TIMER_OF_CLOCK;
+   } else if (to->flags & TIMER_OF_CLOCK_FREQUENCY) {
+   ret = timer_of_clk_frequency_init(np, >of_clk);
+   if (ret)
+   goto out_fail;
+   flags |= TIMER_OF_CLOCK_FREQUENCY;
}
 
if (to->flags & TIMER_OF_IRQ) {
@@ -201,6 +221,9 @@ int __init timer_of_init(struct device_node *np, struct 
timer_of *to)
if (flags & TIMER_OF_CLOCK)
timer_of_clk_exit(>of_clk);
 
+   if (flags & TIMER_OF_CLOCK_FREQUENCY)
+   to->of_clk.rate = 0;
+
if (flags & TIMER_OF_BASE)
timer_of_base_exit(>of_base);
return ret;
diff --git a/drivers/clocksource/timer-of.h b/drivers/clocksource/timer-of.h
index a5478f3..f1a083e 100644
--- a/drivers/clocksource/timer-of.h
+++ b/drivers/clocksource/timer-of.h
@@ -4,9 +4,10 @@
 
 #include 
 
-#define TIMER_OF_BASE  0x1
-#define TIMER_OF_CLOCK 0x2
-#define TIMER_OF_IRQ   0x4
+#define TIMER_OF_BASE  0x1
+#define TIMER_OF_CLOCK 0x2
+#define TIMER_OF_IRQ   0x4
+#define TIMER_OF_CLOCK_FREQUENCY   0x8
 
 struct of_timer_irq {
int irq;
@@ -26,6 +27,7 @@ struct of_timer_base {
 struct of_timer_clk {
struct clk *clk;
const char *name;
+   const char *prop_name;
int index;
unsigned long rate;
unsigned long period;
-- 
2.7.4



[PATCH V3 5/5] arm64: dts: imx8mm: Add system counter node

2019-06-27 Thread Anson . Huang
From: Anson Huang 

Add i.MX8MM system counter node to enable timer-imx-sysctr
broadcast timer driver.

Signed-off-by: Anson Huang 
---
New patch:
- As i.MX8MM clock driver will be soon moved to using platform driver 
model, so the patch
  series I sent out for i.MX8MM system counter driver support will need 
rework accordingly,
  so I add the i.MX8MM DT support in this patch series, it uses same 
method as i.MX8MQ's
  system counter driver.
---
 arch/arm64/boot/dts/freescale/imx8mm.dtsi | 8 
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi 
b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index 63f4731..aa985a0 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -528,6 +528,14 @@
#pwm-cells = <2>;
status = "disabled";
};
+
+   system_counter: timer@306a {
+   compatible = "nxp,sysctr-timer";
+   reg = <0x306a 0x3>;
+   interrupts = ,
+;
+   clock-frequency = <800>;
+   };
};
 
aips3: bus@3080 {
-- 
2.7.4



[PATCH V3 2/5] clocksource/drivers/sysctr: Add clock-frequency property

2019-06-27 Thread Anson . Huang
From: Anson Huang 

Systems which use platform driver model for clock driver require the
clock frequency to be supplied via device tree when system counter
driver is enabled.

This is necessary as in the platform driver model the of_clk operations
do not work correctly because system counter driver is initialized in
early phase of system boot up, and clock driver using platform driver
model is NOT ready at that time, it will cause system counter driver
initialization failed.

Add clock-frequency property to the device tree bindings of the NXP
system counter, so the driver can tell timer-of driver to get clock
frequency from DT directly instead of doing of_clk operations via
clk APIs.

Signed-off-by: Anson Huang 
---
Changes since V2:
- make clock-frequency property as required property, mutually 
exclusive with clocks/clock-names.
- update the example using the DT node added in this patch series.
---
 .../devicetree/bindings/timer/nxp,sysctr-timer.txt| 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt 
b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
index d576599..7088a0e 100644
--- a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
+++ b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
@@ -11,15 +11,18 @@ Required properties:
 - reg : Specifies the base physical address and size of the comapre
 frame and the counter control, read & compare.
 - interrupts :  should be the first compare frames' interrupt
-- clocks : Specifies the counter clock.
-- clock-names: Specifies the clock's name of this module
+- clocks :  Specifies the counter clock, mutually exclusive with 
clock-frequency.
+- clock-names : Specifies the clock's name of this module, mutually 
exclusive with
+   clock-frequency.
+- clock-frequency : Specifies system counter clock frequency, mutually 
exclusive with
+   clocks/clock-names.
 
 Example:
 
system_counter: timer@306a {
compatible = "nxp,sysctr-timer";
-   reg = <0x306a 0x2>;/* system-counter-rd & compare */
-   clocks = <_8m>;
-   clock-names = "per";
-   interrupts = ;
+   reg = <0x306a 0x3>;
+   interrupts = ,
+;
+   clock-frequency = <833>;
};
-- 
2.7.4



[PATCH 1/2] arm64: dts: imx8mq: Correct OPP table according to latest datasheet

2019-06-27 Thread Anson . Huang
From: Anson Huang 

According to latest datasheet (Rev.1, 10/2018) from below links,
in the consumer datasheet, 1.5GHz is mentioned as highest opp but
depends on speed grading fuse, and in the industrial datasheet,
1.3GHz is mentioned as highest opp but depends on speed grading
fuse. 1.5GHz and 1.3GHz opp use same voltage, so no need for
consumer part to support 1.3GHz opp, with same voltage, CPU should
run at highest frequency in order to go into idle as quick as
possible, this can save power.

That means for consumer part, 1GHz/1.5GHz are supported, for
industrial part, 800MHz/1.3GHz are supported, and then check the
speed grading fuse to limit the highest CPU frequency further.
Correct the market segment bits in opp table to make them work
according to datasheets.

https://www.nxp.com/docs/en/data-sheet/IMX8MDQLQIEC.pdf
https://www.nxp.com/docs/en/data-sheet/IMX8MDQLQCEC.pdf

Fixes: 12629c5c3749 ("arm64: dts: imx8mq: Add cpu speed grading and all OPPs")
Signed-off-by: Anson Huang 
---
 arch/arm64/boot/dts/freescale/imx8mq.dtsi | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi 
b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index 9d99191..bea53bc 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -169,7 +169,8 @@
opp-13 {
opp-hz = /bits/ 64 <13>;
opp-microvolt = <100>;
-   opp-supported-hw = <0xc>, <0x7>;
+   /* Industrial only but rely on speed grading */
+   opp-supported-hw = <0xc>, <0x4>;
clock-latency-ns = <15>;
};
 
@@ -177,7 +178,7 @@
opp-hz = /bits/ 64 <15>;
opp-microvolt = <100>;
/* Consumer only but rely on speed grading */
-   opp-supported-hw = <0x8>, <0x7>;
+   opp-supported-hw = <0x8>, <0x3>;
clock-latency-ns = <15>;
};
};
-- 
2.7.4



[PATCH 2/2] arm64: dts: imx8mm: Correct OPP table according to latest datasheet

2019-06-27 Thread Anson . Huang
From: Anson Huang 

According to latest datasheet (Rev.0.2, 04/2019) from below links,
1.8GHz is ONLY available for consumer part, so the market segment
bits for 1.8GHz opp should ONLY available for consumer part accordingly.

https://www.nxp.com/docs/en/data-sheet/IMX8MMIEC.pdf
https://www.nxp.com/docs/en/data-sheet/IMX8MMCEC.pdf

Fixes: f403a26c865b (arm64: dts: imx8mm: Add cpu speed grading and all OPPs)
Signed-off-by: Anson Huang 
---
 arch/arm64/boot/dts/freescale/imx8mm.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi 
b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index c38813d..ab0d135 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -135,7 +135,7 @@
opp-hz = /bits/ 64 <18>;
opp-microvolt = <100>;
/* Consumer only but rely on speed grading */
-   opp-supported-hw = <0x8>, <0x7>;
+   opp-supported-hw = <0x8>, <0x3>;
clock-latency-ns = <15>;
};
};
-- 
2.7.4



[PATCH V2] soc: imx-scu: Add SoC UID(unique identifier) support

2019-06-27 Thread Anson . Huang
From: Anson Huang 

Add i.MX SCU SoC's UID(unique identifier) support, user
can read it from sysfs:

root@imx8qxpmek:~# cat /sys/devices/soc0/soc_uid
7B64280B57AC1898

Signed-off-by: Anson Huang 
---
Changes since V1:
- Improve the comment of skipping SCFW API return value check for 
getting UID.
---
 drivers/soc/imx/soc-imx-scu.c | 39 +++
 1 file changed, 39 insertions(+)

diff --git a/drivers/soc/imx/soc-imx-scu.c b/drivers/soc/imx/soc-imx-scu.c
index 676f612..3eacb54 100644
--- a/drivers/soc/imx/soc-imx-scu.c
+++ b/drivers/soc/imx/soc-imx-scu.c
@@ -27,6 +27,40 @@ struct imx_sc_msg_misc_get_soc_id {
} data;
 } __packed;
 
+struct imx_sc_msg_misc_get_soc_uid {
+   struct imx_sc_rpc_msg hdr;
+   u32 uid_low;
+   u32 uid_high;
+} __packed;
+
+static ssize_t soc_uid_show(struct device *dev,
+   struct device_attribute *attr, char *buf)
+{
+   struct imx_sc_msg_misc_get_soc_uid msg;
+   struct imx_sc_rpc_msg *hdr = 
+   u64 soc_uid;
+
+   hdr->ver = IMX_SC_RPC_VERSION;
+   hdr->svc = IMX_SC_RPC_SVC_MISC;
+   hdr->func = IMX_SC_MISC_FUNC_UNIQUE_ID;
+   hdr->size = 1;
+
+   /*
+* SCU FW API always returns an error even the
+* function is successfully executed, so skip
+* returned value check.
+*/
+   imx_scu_call_rpc(soc_ipc_handle, , true);
+
+   soc_uid = msg.uid_high;
+   soc_uid <<= 32;
+   soc_uid |= msg.uid_low;
+
+   return sprintf(buf, "%016llX\n", soc_uid);
+}
+
+static DEVICE_ATTR_RO(soc_uid);
+
 static int imx_scu_soc_id(void)
 {
struct imx_sc_msg_misc_get_soc_id msg;
@@ -102,6 +136,11 @@ static int imx_scu_soc_probe(struct platform_device *pdev)
goto free_revision;
}
 
+   ret = device_create_file(soc_device_to_device(soc_dev),
+_attr_soc_uid);
+   if (ret)
+   goto free_revision;
+
return 0;
 
 free_revision:
-- 
2.7.4



RE: [PATCH RESEND V2 1/3] clocksource/drivers/sysctr: Add optional clock-frequency property

2019-06-27 Thread Anson Huang
Hi, Daniel

> On 23/06/2019 14:38, anson.hu...@nxp.com wrote:
> > From: Anson Huang 
> >
> > Systems which use platform driver model for clock driver require the
> > clock frequency to be supplied via device tree when system counter
> > driver is enabled.
> >
> > This is necessary as in the platform driver model the of_clk
> > operations do not work correctly because system counter driver is
> > initialized in early phase of system boot up, and clock driver using
> > platform driver model is NOT ready at that time, it will cause system
> > counter driver initialization failed.
> >
> > Add the optinal clock-frequency to the device tree bindings of the NXP
> > system counter, so the frequency can be handed in and the of_clk
> > operations can be skipped.
> >
> > Signed-off-by: Anson Huang 
> > ---
> > Changes since V1:
> > - improve commit log, no content change.
> > ---
> >  Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt | 6
> > ++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
> > b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
> > index d576599..c9907a0 100644
> > --- a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
> > +++ b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
> > @@ -14,6 +14,11 @@ Required properties:
> >  - clocks : Specifies the counter clock.
> >  - clock-names: Specifies the clock's name of this module
> >
> > +Optional properties:
> > +
> > +- clock-frequency : Specifies system counter clock frequency and indicates
> system
> > +   counter driver to skip clock operations.
> > +
> 
> Shouldn't it be required and mutually exclusive with clocks/clock-names?
>
Yes, make sense, I ever thought about it when doing this patch, but eventually 
I picked
the optional...will fix it in next version.

Thanks,
Anson

> >  Example:
> >
> > system_counter: timer@306a {
> > @@ -22,4 +27,5 @@ Example:
> > clocks = <_8m>;
> > clock-names = "per";
> > interrupts = ;
> > +   clock-frequency = <833>;
> > };
> >
> 
> 



RE: [PATCH RESEND V2 2/3] clocksource: imx-sysctr: Make timer work with clock driver using platform driver model

2019-06-27 Thread Anson Huang
Hi, Daniel

> On 23/06/2019 14:38, anson.hu...@nxp.com wrote:
> > From: Anson Huang 
> >
> > On some i.MX8M platforms, clock driver uses platform driver model and
> > it is NOT ready during timer initialization phase, the clock
> > operations will fail and system counter driver will fail too. As all
> > the i.MX8M platforms' system counter clock are from OSC which is
> > always enabled, so it is no need to enable clock for system counter
> > driver, the ONLY thing is to pass clock frequence to driver.
> >
> > To make system counter driver work for upper scenario, add an option
> > of skipping of_clk operation for system counter driver, an optional
> > property "clock-frequency" is introduced to pass the frequency value
> > to system counter driver and indicate driver to skip of_clk
> > operations.
> >
> > Signed-off-by: Anson Huang 
> > ---
> > Changes since V1:
> > - improve commit log, no content change.
> > ---
> >  drivers/clocksource/timer-imx-sysctr.c | 8 
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/clocksource/timer-imx-sysctr.c
> > b/drivers/clocksource/timer-imx-sysctr.c
> > index fd7d680..8ff3d7e 100644
> > --- a/drivers/clocksource/timer-imx-sysctr.c
> > +++ b/drivers/clocksource/timer-imx-sysctr.c
> > @@ -129,6 +129,14 @@ static void __init sysctr_clockevent_init(void)
> > static int __init sysctr_timer_init(struct device_node *np)  {
> > int ret = 0;
> > +   u32 rate;
> > +
> > +   if (!of_property_read_u32(np, "clock-frequency",
> > + )) {
> > +   to_sysctr.of_clk.rate = rate;
> > +   to_sysctr.of_clk.period = DIV_ROUND_UP(rate, HZ);
> > +   to_sysctr.flags &= ~TIMER_OF_CLOCK;
> > +   }
> 
> Please take the opportunity to add the TIMER_OF_CLOCK_FREQUENCY flag in
> timer-of and the corresponding code above.

OK, so another patch for timer-of is necessary, if TIMER_OF_CLOCK_FREQUENCY flag
is present, just read the "clock-frequency" from DT instead of getting clock 
rate from
clock tree, right? I think this becomes a common case for timer driver, as more 
and more
platforms will use platform driver model for clock driver, it would be good if 
timer-of can
provide solution.

> 
> Then check the clock-frequency presence and add TIMER_OF_CLOCK or
> TIMER_OF_CLOCK_FREQUENCY flag to the timer-of structure.
> 
> eg:
> 
> to_sysctr.flags |= of_find_property(np, "clock-frequency", NULL) ?
>   TIMER_OF_CLOCK_FREQUENCY : TIMER_OF_CLOCK;
> 

OK.

Thanks,
Anson.



RE: [PATCH RESEND V2 1/3] clocksource/drivers/sysctr: Add optional clock-frequency property

2019-06-27 Thread Anson Huang
Hi, Daniel

> On 27/06/2019 02:43, Anson Huang wrote:
> > Hi, Daniel
> >
> >> On 26/06/2019 03:42, Anson Huang wrote:
> >>> Hi, Daniel
> >>>
> >>>> On 23/06/2019 14:38, anson.hu...@nxp.com wrote:
> >>>>> From: Anson Huang 
> >>>>>
> >>>>> Systems which use platform driver model for clock driver require
> >>>>> the clock frequency to be supplied via device tree when system
> >>>>> counter driver is enabled.
> >>>>>
> >>>>> This is necessary as in the platform driver model the of_clk
> >>>>> operations do not work correctly because system counter driver is
> >>>>> initialized in early phase of system boot up, and clock driver
> >>>>> using platform driver model is NOT ready at that time, it will
> >>>>> cause system counter driver initialization failed.
> >>>>>
> >>>>> Add the optinal clock-frequency to the device tree bindings of the
> >>>>> NXP system counter, so the frequency can be handed in and the
> >>>>> of_clk operations can be skipped.
> >>>>
> >>>> Isn't it possible to create a fixed-clock and refer to it? So no
> >>>> need to create a specific action before calling timer_of_init() ?
> >>>>
> >>>
> >>> As the clock must be ready before the TIMER_OF_DECLARE, so adding a
> >>> CLK_OF_DECLARE_DRIVER in clock driver to ONLY register a fixed-clock?
> >>> The system counter's frequency are different on different platforms,
> >>> so adding fixed clock in system counter driver is NOT a good idea,
> >>> ONLY the DT node or the clock driver can create this fixed clock
> >>> according to
> >> platforms, can you advise where to create this fixed clock is better?
> >>
> >> Can you point me to a DT with the "nxp,sysctr-timer" ?
> >
> > The DT node of system counter is new added in 3/3 of this patch
> > series, also can be found from below link:
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
> >
> hwork.kernel.org%2Fpatch%2F11011703%2Fdata=02%7C01%7Canson.
> huang%
> >
> 40nxp.com%7C8b9519ecceb346712be808d6fad675e4%7C686ea1d3bc2b4c6f
> a92cd99
> >
> c5c301635%7C0%7C0%7C636972196338405582sdata=sOQQzDFxoCqe
> VuHFuYPHh
> > F8Bdj2Zu9WS7Go%2FV9lrWa8%3Dreserved=0
> 
> Sorry, I was unclear. I meant a patch with the timer defined using a clock as
> defined currently in the binding (no clock-frequency).

OK, for i.MX8MM, we use clocks, check below patch series:

https://patchwork.kernel.org/patch/11008519/

code piece as below:

+   system_counter: timer@306a {
+   compatible = "nxp,sysctr-timer";
+   reg = <0x306a 0x3>;
+   interrupts = ,
+;
+   clocks = < IMX8MM_CLK_SYS_CTR>;
+   clock-names = "per";
+   };

Thanks,
Anson.



RE: [PATCH] soc: imx-scu: Add SoC UID(unique identifier) support

2019-06-27 Thread Anson Huang
Hi, Daniel

> -Original Message-
> From: Daniel Baluta 
> Sent: Thursday, June 27, 2019 2:44 PM
> To: Anson Huang 
> Cc: Shawn Guo ; Sascha Hauer
> ; Pengutronix Kernel Team
> ; Fabio Estevam ; Aisheng
> Dong ; Abel Vesa ; linux-
> arm-kernel ; Linux Kernel Mailing List
> ; dl-linux-imx ; Daniel
> Baluta 
> Subject: Re: [PATCH] soc: imx-scu: Add SoC UID(unique identifier) support
> 
> On Thu, Jun 27, 2019 at 3:48 AM Anson Huang 
> wrote:
> >
> > Hi, Daniel
> >
> > > -Original Message-
> > > From: Daniel Baluta 
> > > Sent: Wednesday, June 26, 2019 8:42 PM
> > > To: Anson Huang 
> > > Cc: Shawn Guo ; Sascha Hauer
> > > ; Pengutronix Kernel Team
> > > ; Fabio Estevam ;
> Aisheng
> > > Dong ; Abel Vesa ; linux-
> > > arm-kernel ; Linux Kernel
> > > Mailing List ; dl-linux-imx
> > > ; Daniel Baluta 
> > > Subject: Re: [PATCH] soc: imx-scu: Add SoC UID(unique identifier)
> > > support
> > >
> > > On Wed, Jun 26, 2019 at 10:06 AM  wrote:
> > > >
> > > > From: Anson Huang 
> > > >
> > > > Add i.MX SCU SoC's UID(unique identifier) support, user can read
> > > > it from sysfs:
> > > >
> > > > root@imx8qxpmek:~# cat /sys/devices/soc0/soc_uid
> > > > 7B64280B57AC1898
> > > >
> > > > Signed-off-by: Anson Huang 
> > > > ---
> > > >  drivers/soc/imx/soc-imx-scu.c | 35
> > > > +++
> > > >  1 file changed, 35 insertions(+)
> > > >
> > > > diff --git a/drivers/soc/imx/soc-imx-scu.c
> > > > b/drivers/soc/imx/soc-imx-scu.c index 676f612..8d322a1 100644
> > > > --- a/drivers/soc/imx/soc-imx-scu.c
> > > > +++ b/drivers/soc/imx/soc-imx-scu.c
> > > > @@ -27,6 +27,36 @@ struct imx_sc_msg_misc_get_soc_id {
> > > > } data;
> > > >  } __packed;
> > > >
> > > > +struct imx_sc_msg_misc_get_soc_uid {
> > > > +   struct imx_sc_rpc_msg hdr;
> > > > +   u32 uid_low;
> > > > +   u32 uid_high;
> > > > +} __packed;
> > > > +
> > > > +static ssize_t soc_uid_show(struct device *dev,
> > > > +   struct device_attribute *attr, char
> > > > +*buf) {
> > > > +   struct imx_sc_msg_misc_get_soc_uid msg;
> > > > +   struct imx_sc_rpc_msg *hdr = 
> > > > +   u64 soc_uid;
> > > > +
> > > > +   hdr->ver = IMX_SC_RPC_VERSION;
> > > > +   hdr->svc = IMX_SC_RPC_SVC_MISC;
> > > > +   hdr->func = IMX_SC_MISC_FUNC_UNIQUE_ID;
> > > > +   hdr->size = 1;
> > > > +
> > > > +   /* the return value of SCU FW is in correct, skip return
> > > > + value check */
> > >
> > > Why do you mean by "in correct"?
> >
> > I made a mistake, it should be "incorrect", the existing SCFW of this
> > API returns an error value even this API is successfully called, to
> > make it work with current SCFW, I have to skip the return value check
> > for this API for now. Will send V2 patch to fix this typo.
> 
> Thanks Anson! It makes sense now. It is a little bit sad though because we
> won't know when there is a "real" error :).
> 
> Lets update the comment to be more specific:
> 
> /* SCFW FW API always returns an error even the function is successfully
> executed, so skip returned value */

OK, as for external users, the SCFW formally released has this issue, so for now
I have to skip the return value check for this API, once next SCFW release has 
this issue
fixed, I will add a patch to check the return value.

Thanks,
Anson.
> 
> 
> > > > +   imx_scu_call_rpc(soc_ipc_handle, , true);
> > > > +
> > > > +   soc_uid = msg.uid_high;
> > > > +   soc_uid <<= 32;
> > > > +   soc_uid |= msg.uid_low;
> > > > +
> > > > +   return sprintf(buf, "%016llX\n", soc_uid);
> > >
> > > snprintf?
> >
> > The snprintf is to avoid buffer overflow, which in this case, I don't
> > know the size of "buf", and the value(u64) to be printed is with fixed
> > length of 64, so I think sprint is just OK.
> 
> Ok.


RE: [PATCH] soc: imx-scu: Add SoC UID(unique identifier) support

2019-06-26 Thread Anson Huang
Hi, Daniel

> -Original Message-
> From: Daniel Baluta 
> Sent: Wednesday, June 26, 2019 8:42 PM
> To: Anson Huang 
> Cc: Shawn Guo ; Sascha Hauer
> ; Pengutronix Kernel Team
> ; Fabio Estevam ; Aisheng
> Dong ; Abel Vesa ; linux-
> arm-kernel ; Linux Kernel Mailing List
> ; dl-linux-imx ; Daniel
> Baluta 
> Subject: Re: [PATCH] soc: imx-scu: Add SoC UID(unique identifier) support
> 
> On Wed, Jun 26, 2019 at 10:06 AM  wrote:
> >
> > From: Anson Huang 
> >
> > Add i.MX SCU SoC's UID(unique identifier) support, user can read it
> > from sysfs:
> >
> > root@imx8qxpmek:~# cat /sys/devices/soc0/soc_uid
> > 7B64280B57AC1898
> >
> > Signed-off-by: Anson Huang 
> > ---
> >  drivers/soc/imx/soc-imx-scu.c | 35
> > +++
> >  1 file changed, 35 insertions(+)
> >
> > diff --git a/drivers/soc/imx/soc-imx-scu.c
> > b/drivers/soc/imx/soc-imx-scu.c index 676f612..8d322a1 100644
> > --- a/drivers/soc/imx/soc-imx-scu.c
> > +++ b/drivers/soc/imx/soc-imx-scu.c
> > @@ -27,6 +27,36 @@ struct imx_sc_msg_misc_get_soc_id {
> > } data;
> >  } __packed;
> >
> > +struct imx_sc_msg_misc_get_soc_uid {
> > +   struct imx_sc_rpc_msg hdr;
> > +   u32 uid_low;
> > +   u32 uid_high;
> > +} __packed;
> > +
> > +static ssize_t soc_uid_show(struct device *dev,
> > +   struct device_attribute *attr, char *buf)
> > +{
> > +   struct imx_sc_msg_misc_get_soc_uid msg;
> > +   struct imx_sc_rpc_msg *hdr = 
> > +   u64 soc_uid;
> > +
> > +   hdr->ver = IMX_SC_RPC_VERSION;
> > +   hdr->svc = IMX_SC_RPC_SVC_MISC;
> > +   hdr->func = IMX_SC_MISC_FUNC_UNIQUE_ID;
> > +   hdr->size = 1;
> > +
> > +   /* the return value of SCU FW is in correct, skip return value
> > + check */
> 
> Why do you mean by "in correct"?

I made a mistake, it should be "incorrect", the existing SCFW of this API 
returns
an error value even this API is successfully called, to make it work with 
current
SCFW, I have to skip the return value check for this API for now. Will send V2 
patch
to fix this typo.

> > +   imx_scu_call_rpc(soc_ipc_handle, , true);
> > +
> > +   soc_uid = msg.uid_high;
> > +   soc_uid <<= 32;
> > +   soc_uid |= msg.uid_low;
> > +
> > +   return sprintf(buf, "%016llX\n", soc_uid);
> 
> snprintf?

The snprintf is to avoid buffer overflow, which in this case, I don't know the 
size
of "buf", and the value(u64) to be printed is with fixed length of 64, so I 
think
sprint is just OK.

Anson.
> 
> > +}
> > +
> > +static DEVICE_ATTR_RO(soc_uid);
> > +
> >  static int imx_scu_soc_id(void)
> >  {
> > struct imx_sc_msg_misc_get_soc_id msg; @@ -102,6 +132,11 @@
> > static int imx_scu_soc_probe(struct platform_device *pdev)
> > goto free_revision;
> > }
> >
> > +   ret = device_create_file(soc_device_to_device(soc_dev),
> > +_attr_soc_uid);
> > +   if (ret)
> > +   goto free_revision;
> > +
> > return 0;
> >
> >  free_revision:
> > --
> > 2.7.4
> >


RE: [PATCH RESEND V2 1/3] clocksource/drivers/sysctr: Add optional clock-frequency property

2019-06-26 Thread Anson Huang
Hi, Daniel

> On 26/06/2019 03:42, Anson Huang wrote:
> > Hi, Daniel
> >
> >> On 23/06/2019 14:38, anson.hu...@nxp.com wrote:
> >>> From: Anson Huang 
> >>>
> >>> Systems which use platform driver model for clock driver require the
> >>> clock frequency to be supplied via device tree when system counter
> >>> driver is enabled.
> >>>
> >>> This is necessary as in the platform driver model the of_clk
> >>> operations do not work correctly because system counter driver is
> >>> initialized in early phase of system boot up, and clock driver using
> >>> platform driver model is NOT ready at that time, it will cause
> >>> system counter driver initialization failed.
> >>>
> >>> Add the optinal clock-frequency to the device tree bindings of the
> >>> NXP system counter, so the frequency can be handed in and the of_clk
> >>> operations can be skipped.
> >>
> >> Isn't it possible to create a fixed-clock and refer to it? So no need
> >> to create a specific action before calling timer_of_init() ?
> >>
> >
> > As the clock must be ready before the TIMER_OF_DECLARE, so adding a
> > CLK_OF_DECLARE_DRIVER in clock driver to ONLY register a fixed-clock?
> > The system counter's frequency are different on different platforms,
> > so adding fixed clock in system counter driver is NOT a good idea,
> > ONLY the DT node or the clock driver can create this fixed clock according 
> > to
> platforms, can you advise where to create this fixed clock is better?
> 
> Can you point me to a DT with the "nxp,sysctr-timer" ?

The DT node of system counter is new added in 3/3 of this patch series, also 
can be found
from below link:
https://patchwork.kernel.org/patch/11011703/

thanks,
Anson



[PATCH 2/2] soc: imx8: Add i.MX8MM UID(unique identifier) support

2019-06-26 Thread Anson . Huang
From: Anson Huang 

Add i.MX8MM SoC UID(unique identifier) support, user
can read it from sysfs:

root@imx8mmevk:~# cat /sys/devices/soc0/soc_uid
B365FA0A5C85D6EE

Signed-off-by: Anson Huang 
---
 drivers/soc/imx/soc-imx8.c | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/drivers/soc/imx/soc-imx8.c b/drivers/soc/imx/soc-imx8.c
index c19ef4b..b9831576 100644
--- a/drivers/soc/imx/soc-imx8.c
+++ b/drivers/soc/imx/soc-imx8.c
@@ -66,6 +66,26 @@ static u32 __init imx8mq_soc_revision(void)
return rev;
 }
 
+static void __init imx8mm_soc_uid(void)
+{
+   void __iomem *ocotp_base;
+   struct device_node *np;
+
+   np = of_find_compatible_node(NULL, NULL, "fsl,imx8mm-ocotp");
+   if (!np)
+   return;
+
+   ocotp_base = of_iomap(np, 0);
+   WARN_ON(!ocotp_base);
+
+   soc_uid = readl_relaxed(ocotp_base + OCOTP_UID_HIGH);
+   soc_uid <<= 32;
+   soc_uid |= readl_relaxed(ocotp_base + OCOTP_UID_LOW);
+
+   iounmap(ocotp_base);
+   of_node_put(np);
+}
+
 static u32 __init imx8mm_soc_revision(void)
 {
struct device_node *np;
@@ -83,6 +103,9 @@ static u32 __init imx8mm_soc_revision(void)
 
iounmap(anatop_base);
of_node_put(np);
+
+   imx8mm_soc_uid();
+
return rev;
 }
 
-- 
2.7.4



[PATCH 1/2] soc: imx8: Add i.MX8MQ UID(unique identifier) support

2019-06-26 Thread Anson . Huang
From: Anson Huang 

Add i.MX8MQ SoC UID(unique identifier) support, user
can read it from sysfs:

root@imx8mqevk:~# cat /sys/devices/soc0/soc_uid
D56911D6F060954B

Signed-off-by: Anson Huang 
---
 drivers/soc/imx/soc-imx8.c | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/drivers/soc/imx/soc-imx8.c b/drivers/soc/imx/soc-imx8.c
index f924ae8..c19ef4b 100644
--- a/drivers/soc/imx/soc-imx8.c
+++ b/drivers/soc/imx/soc-imx8.c
@@ -16,6 +16,9 @@
 #define IMX8MQ_SW_INFO_B1  0x40
 #define IMX8MQ_SW_MAGIC_B1 0xff0055aa
 
+#define OCOTP_UID_LOW  0x410
+#define OCOTP_UID_HIGH 0x420
+
 /* Same as ANADIG_DIGPROG_IMX7D */
 #define ANADIG_DIGPROG_IMX8MM  0x800
 
@@ -24,6 +27,16 @@ struct imx8_soc_data {
u32 (*soc_revision)(void);
 };
 
+static u64 soc_uid;
+
+static ssize_t soc_uid_show(struct device *dev,
+   struct device_attribute *attr, char *buf)
+{
+   return sprintf(buf, "%016llX\n", soc_uid);
+}
+
+static DEVICE_ATTR_RO(soc_uid);
+
 static u32 __init imx8mq_soc_revision(void)
 {
struct device_node *np;
@@ -42,6 +55,10 @@ static u32 __init imx8mq_soc_revision(void)
if (magic == IMX8MQ_SW_MAGIC_B1)
rev = REV_B1;
 
+   soc_uid = readl_relaxed(ocotp_base + OCOTP_UID_HIGH);
+   soc_uid <<= 32;
+   soc_uid |= readl_relaxed(ocotp_base + OCOTP_UID_LOW);
+
iounmap(ocotp_base);
 
 out:
@@ -140,6 +157,11 @@ static int __init imx8_soc_init(void)
goto free_rev;
}
 
+   ret = device_create_file(soc_device_to_device(soc_dev),
+_attr_soc_uid);
+   if (ret)
+   goto free_rev;
+
if (IS_ENABLED(CONFIG_ARM_IMX_CPUFREQ_DT))
platform_device_register_simple("imx-cpufreq-dt", -1, NULL, 0);
 
-- 
2.7.4



[PATCH] soc: imx-scu: Add SoC UID(unique identifier) support

2019-06-26 Thread Anson . Huang
From: Anson Huang 

Add i.MX SCU SoC's UID(unique identifier) support, user
can read it from sysfs:

root@imx8qxpmek:~# cat /sys/devices/soc0/soc_uid
7B64280B57AC1898

Signed-off-by: Anson Huang 
---
 drivers/soc/imx/soc-imx-scu.c | 35 +++
 1 file changed, 35 insertions(+)

diff --git a/drivers/soc/imx/soc-imx-scu.c b/drivers/soc/imx/soc-imx-scu.c
index 676f612..8d322a1 100644
--- a/drivers/soc/imx/soc-imx-scu.c
+++ b/drivers/soc/imx/soc-imx-scu.c
@@ -27,6 +27,36 @@ struct imx_sc_msg_misc_get_soc_id {
} data;
 } __packed;
 
+struct imx_sc_msg_misc_get_soc_uid {
+   struct imx_sc_rpc_msg hdr;
+   u32 uid_low;
+   u32 uid_high;
+} __packed;
+
+static ssize_t soc_uid_show(struct device *dev,
+   struct device_attribute *attr, char *buf)
+{
+   struct imx_sc_msg_misc_get_soc_uid msg;
+   struct imx_sc_rpc_msg *hdr = 
+   u64 soc_uid;
+
+   hdr->ver = IMX_SC_RPC_VERSION;
+   hdr->svc = IMX_SC_RPC_SVC_MISC;
+   hdr->func = IMX_SC_MISC_FUNC_UNIQUE_ID;
+   hdr->size = 1;
+
+   /* the return value of SCU FW is in correct, skip return value check */
+   imx_scu_call_rpc(soc_ipc_handle, , true);
+
+   soc_uid = msg.uid_high;
+   soc_uid <<= 32;
+   soc_uid |= msg.uid_low;
+
+   return sprintf(buf, "%016llX\n", soc_uid);
+}
+
+static DEVICE_ATTR_RO(soc_uid);
+
 static int imx_scu_soc_id(void)
 {
struct imx_sc_msg_misc_get_soc_id msg;
@@ -102,6 +132,11 @@ static int imx_scu_soc_probe(struct platform_device *pdev)
goto free_revision;
}
 
+   ret = device_create_file(soc_device_to_device(soc_dev),
+_attr_soc_uid);
+   if (ret)
+   goto free_revision;
+
return 0;
 
 free_revision:
-- 
2.7.4



RE: [PATCH RESEND V2 1/3] clocksource/drivers/sysctr: Add optional clock-frequency property

2019-06-25 Thread Anson Huang
Hi, Daniel

> On 23/06/2019 14:38, anson.hu...@nxp.com wrote:
> > From: Anson Huang 
> >
> > Systems which use platform driver model for clock driver require the
> > clock frequency to be supplied via device tree when system counter
> > driver is enabled.
> >
> > This is necessary as in the platform driver model the of_clk
> > operations do not work correctly because system counter driver is
> > initialized in early phase of system boot up, and clock driver using
> > platform driver model is NOT ready at that time, it will cause system
> > counter driver initialization failed.
> >
> > Add the optinal clock-frequency to the device tree bindings of the NXP
> > system counter, so the frequency can be handed in and the of_clk
> > operations can be skipped.
> 
> Isn't it possible to create a fixed-clock and refer to it? So no need to 
> create a
> specific action before calling timer_of_init() ?
> 

As the clock must be ready before the TIMER_OF_DECLARE, so adding a 
CLK_OF_DECLARE_DRIVER in
clock driver to ONLY register a fixed-clock? The system counter's frequency are 
different on different
platforms, so adding fixed clock in system counter driver is NOT a good idea, 
ONLY the DT node or the
clock driver can create this fixed clock according to platforms, can you advise 
where to create this fixed
clock is better?

Thanks,
Anson 



RE: [PATCH 2/2] clk: imx8mm: GPT1 clock mux option #5 should be sys_pll1_80m

2019-06-25 Thread Anson Huang
Hi, Stephen

> Quoting anson.hu...@nxp.com (2019-06-25 00:06:02)
> > From: Anson Huang 
> >
> > i.MX8MM's GPT1 clock mux option #5 should be sys_pll1_80m, NOT
> > sys_pll1_800m, correct it.
> >
> > Signed-off-by: Anson Huang 
> 
> Any Fixes tags?

Oops, I forgot to add fixed tags, just resent the patch set, sorry for that.

Thanks,
Anson




[PATCH RESEND 1/2] clk: imx8mm: Fix typo of pwm3 clock's mux option #4

2019-06-25 Thread Anson . Huang
From: Anson Huang 

i.MX8MM has no sys3_pll2_out clock, PWM3 clock's mux option #4
should be sys_pll3_out, sys3_pll2_out is a typo, fix it.

Fixes: ba5625c3e272 ("clk: imx: Add clock driver support for imx8mm")
Signed-off-by: Anson Huang 
---
 drivers/clk/imx/clk-imx8mm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/imx/clk-imx8mm.c b/drivers/clk/imx/clk-imx8mm.c
index 56d53dd..516e68d 100644
--- a/drivers/clk/imx/clk-imx8mm.c
+++ b/drivers/clk/imx/clk-imx8mm.c
@@ -287,7 +287,7 @@ static const char *imx8mm_pwm2_sels[] = {"osc_24m", 
"sys_pll2_100m", "sys_pll1_1
 "sys_pll3_out", "clk_ext1", 
"sys_pll1_80m", "video_pll1_out", };
 
 static const char *imx8mm_pwm3_sels[] = {"osc_24m", "sys_pll2_100m", 
"sys_pll1_160m", "sys_pll1_40m",
-"sys3_pll2_out", "clk_ext2", 
"sys_pll1_80m", "video_pll1_out", };
+"sys_pll3_out", "clk_ext2", 
"sys_pll1_80m", "video_pll1_out", };
 
 static const char *imx8mm_pwm4_sels[] = {"osc_24m", "sys_pll2_100m", 
"sys_pll1_160m", "sys_pll1_40m",
 "sys_pll3_out", "clk_ext2", 
"sys_pll1_80m", "video_pll1_out", };
-- 
2.7.4



[PATCH RESEND 2/2] clk: imx8mm: GPT1 clock mux option #5 should be sys_pll1_80m

2019-06-25 Thread Anson . Huang
From: Anson Huang 

i.MX8MM's GPT1 clock mux option #5 should be sys_pll1_80m,
NOT sys_pll1_800m, correct it.

Fixes: ba5625c3e272 ("clk: imx: Add clock driver support for imx8mm")
Signed-off-by: Anson Huang 
---
 drivers/clk/imx/clk-imx8mm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/imx/clk-imx8mm.c b/drivers/clk/imx/clk-imx8mm.c
index 516e68d..d1a84f7 100644
--- a/drivers/clk/imx/clk-imx8mm.c
+++ b/drivers/clk/imx/clk-imx8mm.c
@@ -293,7 +293,7 @@ static const char *imx8mm_pwm4_sels[] = {"osc_24m", 
"sys_pll2_100m", "sys_pll1_1
 "sys_pll3_out", "clk_ext2", 
"sys_pll1_80m", "video_pll1_out", };
 
 static const char *imx8mm_gpt1_sels[] = {"osc_24m", "sys_pll2_100m", 
"sys_pll1_400m", "sys_pll1_40m",
-"video_pll1_out", "sys_pll1_800m", 
"audio_pll1_out", "clk_ext1" };
+"video_pll1_out", "sys_pll1_80m", 
"audio_pll1_out", "clk_ext1" };
 
 static const char *imx8mm_wdog_sels[] = {"osc_24m", "sys_pll1_133m", 
"sys_pll1_160m", "vpu_pll_out",
 "sys_pll2_125m", "sys_pll3_out", 
"sys_pll1_80m", "sys_pll2_166m", };
-- 
2.7.4



[PATCH 2/2] clk: imx8mm: GPT1 clock mux option #5 should be sys_pll1_80m

2019-06-25 Thread Anson . Huang
From: Anson Huang 

i.MX8MM's GPT1 clock mux option #5 should be sys_pll1_80m,
NOT sys_pll1_800m, correct it.

Signed-off-by: Anson Huang 
---
 drivers/clk/imx/clk-imx8mm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/imx/clk-imx8mm.c b/drivers/clk/imx/clk-imx8mm.c
index 516e68d..d1a84f7 100644
--- a/drivers/clk/imx/clk-imx8mm.c
+++ b/drivers/clk/imx/clk-imx8mm.c
@@ -293,7 +293,7 @@ static const char *imx8mm_pwm4_sels[] = {"osc_24m", 
"sys_pll2_100m", "sys_pll1_1
 "sys_pll3_out", "clk_ext2", 
"sys_pll1_80m", "video_pll1_out", };
 
 static const char *imx8mm_gpt1_sels[] = {"osc_24m", "sys_pll2_100m", 
"sys_pll1_400m", "sys_pll1_40m",
-"video_pll1_out", "sys_pll1_800m", 
"audio_pll1_out", "clk_ext1" };
+"video_pll1_out", "sys_pll1_80m", 
"audio_pll1_out", "clk_ext1" };
 
 static const char *imx8mm_wdog_sels[] = {"osc_24m", "sys_pll1_133m", 
"sys_pll1_160m", "vpu_pll_out",
 "sys_pll2_125m", "sys_pll3_out", 
"sys_pll1_80m", "sys_pll2_166m", };
-- 
2.7.4



[PATCH 1/2] clk: imx8mm: Fix typo of pwm3 clock's mux option #4

2019-06-25 Thread Anson . Huang
From: Anson Huang 

i.MX8MM has no sys3_pll2_out clock, PWM3 clock's mux option #4
should be sys_pll3_out, sys3_pll2_out is a typo, fix it.

Signed-off-by: Anson Huang 
---
 drivers/clk/imx/clk-imx8mm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/imx/clk-imx8mm.c b/drivers/clk/imx/clk-imx8mm.c
index 56d53dd..516e68d 100644
--- a/drivers/clk/imx/clk-imx8mm.c
+++ b/drivers/clk/imx/clk-imx8mm.c
@@ -287,7 +287,7 @@ static const char *imx8mm_pwm2_sels[] = {"osc_24m", 
"sys_pll2_100m", "sys_pll1_1
 "sys_pll3_out", "clk_ext1", 
"sys_pll1_80m", "video_pll1_out", };
 
 static const char *imx8mm_pwm3_sels[] = {"osc_24m", "sys_pll2_100m", 
"sys_pll1_160m", "sys_pll1_40m",
-"sys3_pll2_out", "clk_ext2", 
"sys_pll1_80m", "video_pll1_out", };
+"sys_pll3_out", "clk_ext2", 
"sys_pll1_80m", "video_pll1_out", };
 
 static const char *imx8mm_pwm4_sels[] = {"osc_24m", "sys_pll2_100m", 
"sys_pll1_160m", "sys_pll1_40m",
 "sys_pll3_out", "clk_ext2", 
"sys_pll1_80m", "video_pll1_out", };
-- 
2.7.4



RE: [PATCH 1/4] arm64: Enable TIMER_IMX_SYS_CTR for ARCH_MXC platforms

2019-06-23 Thread Anson Huang
Hi, Shawn

> -Original Message-
> From: Shawn Guo 
> Sent: Monday, June 24, 2019 10:27 AM
> To: Anson Huang 
> Cc: mark.rutl...@arm.com; Aisheng Dong ; Peng
> Fan ; feste...@gmail.com; Jacky Bai
> ; devicet...@vger.kernel.org; sb...@kernel.org;
> catalin.mari...@arm.com; s.ha...@pengutronix.de; linux-
> ker...@vger.kernel.org; Daniel Baluta ; linux-
> c...@vger.kernel.org; robh...@kernel.org; dl-linux-imx  i...@nxp.com>; ker...@pengutronix.de; Leonard Crestez
> ; w...@kernel.org; mturque...@baylibre.com;
> linux-arm-ker...@lists.infradead.org; Abel Vesa 
> Subject: Re: [PATCH 1/4] arm64: Enable TIMER_IMX_SYS_CTR for ARCH_MXC
> platforms
> 
> On Mon, Jun 24, 2019 at 10:22:01AM +0800, Shawn Guo wrote:
> > On Fri, Jun 21, 2019 at 03:07:17PM +0800, anson.hu...@nxp.com wrote:
> > > From: Anson Huang 
> > >
> > > ARCH_MXC platforms needs system counter as broadcast timer to
> > > support cpuidle, enable it by default.
> > >
> > > Signed-off-by: Anson Huang 
> > > ---
> > >  arch/arm64/Kconfig.platforms | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/arch/arm64/Kconfig.platforms
> > > b/arch/arm64/Kconfig.platforms index 4778c77..f5e623f 100644
> > > --- a/arch/arm64/Kconfig.platforms
> > > +++ b/arch/arm64/Kconfig.platforms
> > > @@ -173,6 +173,7 @@ config ARCH_MXC
> > >   select PM
> > >   select PM_GENERIC_DOMAINS
> > >   select SOC_BUS
> > > + select TIMER_IMX_SYS_CTR
> >
> > Where is that driver?
> 
> Okay, just found it in the mailbox.  They seem to be sent in the wrong order.
> Really, you should send this series only after the driver lands on mainline.

OK, just noticed that mainline does NOT have it, since I did it based on next 
tree.
I will resend the patch series after the system counter driver landing on 
mainline.

Thanks,
Anson.

> 
> Shawn


RE: [PATCH 1/4] arm64: Enable TIMER_IMX_SYS_CTR for ARCH_MXC platforms

2019-06-23 Thread Anson Huang
Hi, Shawn

> -Original Message-
> From: Shawn Guo 
> Sent: Monday, June 24, 2019 10:22 AM
> To: Anson Huang 
> Cc: catalin.mari...@arm.com; w...@kernel.org; robh...@kernel.org;
> mark.rutl...@arm.com; s.ha...@pengutronix.de; ker...@pengutronix.de;
> feste...@gmail.com; mturque...@baylibre.com; sb...@kernel.org;
> Leonard Crestez ; Aisheng Dong
> ; Jacky Bai ; Daniel Baluta
> ; Peng Fan ; Abel Vesa
> ; linux-arm-ker...@lists.infradead.org; linux-
> ker...@vger.kernel.org; devicet...@vger.kernel.org; linux-
> c...@vger.kernel.org; dl-linux-imx 
> Subject: Re: [PATCH 1/4] arm64: Enable TIMER_IMX_SYS_CTR for ARCH_MXC
> platforms
> 
> On Fri, Jun 21, 2019 at 03:07:17PM +0800, anson.hu...@nxp.com wrote:
> > From: Anson Huang 
> >
> > ARCH_MXC platforms needs system counter as broadcast timer to support
> > cpuidle, enable it by default.
> >
> > Signed-off-by: Anson Huang 
> > ---
> >  arch/arm64/Kconfig.platforms | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/arm64/Kconfig.platforms
> > b/arch/arm64/Kconfig.platforms index 4778c77..f5e623f 100644
> > --- a/arch/arm64/Kconfig.platforms
> > +++ b/arch/arm64/Kconfig.platforms
> > @@ -173,6 +173,7 @@ config ARCH_MXC
> > select PM
> > select PM_GENERIC_DOMAINS
> > select SOC_BUS
> > +   select TIMER_IMX_SYS_CTR
> 
> Where is that driver?

The driver is drivers/clocksource/timer-imx-sysctr.c, similar function as GPT.

Thanks,
Anson

> 
> Shawn
> 
> > help
> >   This enables support for the ARMv8 based SoCs in the
> >   NXP i.MX family.
> > --
> > 2.7.4
> >


[PATCH RESEND V2 3/3] arm64: dts: imx8mq: Add system counter node

2019-06-23 Thread Anson . Huang
From: Anson Huang 

Add i.MX8MQ system counter node to enable timer-imx-sysctr
broadcast timer driver.

Signed-off-by: Anson Huang 
---
No change.
---
 arch/arm64/boot/dts/freescale/imx8mq.dtsi | 8 
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi 
b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index d09b808..9d99191 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -635,6 +635,14 @@
#pwm-cells = <2>;
status = "disabled";
};
+
+   system_counter: timer@306a {
+   compatible = "nxp,sysctr-timer";
+   reg = <0x306a 0x3>;
+   interrupts = ,
+;
+   clock-frequency = <833>;
+   };
};
 
bus@3080 { /* AIPS3 */
-- 
2.7.4



[PATCH RESEND V2 1/3] clocksource/drivers/sysctr: Add optional clock-frequency property

2019-06-23 Thread Anson . Huang
From: Anson Huang 

Systems which use platform driver model for clock driver require the
clock frequency to be supplied via device tree when system counter
driver is enabled.

This is necessary as in the platform driver model the of_clk operations
do not work correctly because system counter driver is initialized in
early phase of system boot up, and clock driver using platform driver
model is NOT ready at that time, it will cause system counter driver
initialization failed.

Add the optinal clock-frequency to the device tree bindings of the NXP
system counter, so the frequency can be handed in and the of_clk
operations can be skipped.

Signed-off-by: Anson Huang 
---
Changes since V1:
- improve commit log, no content change.
---
 Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt 
b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
index d576599..c9907a0 100644
--- a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
+++ b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
@@ -14,6 +14,11 @@ Required properties:
 - clocks : Specifies the counter clock.
 - clock-names: Specifies the clock's name of this module
 
+Optional properties:
+
+- clock-frequency : Specifies system counter clock frequency and indicates 
system
+   counter driver to skip clock operations.
+
 Example:
 
system_counter: timer@306a {
@@ -22,4 +27,5 @@ Example:
clocks = <_8m>;
clock-names = "per";
interrupts = ;
+   clock-frequency = <833>;
};
-- 
2.7.4



[PATCH RESEND V2 2/3] clocksource: imx-sysctr: Make timer work with clock driver using platform driver model

2019-06-23 Thread Anson . Huang
From: Anson Huang 

On some i.MX8M platforms, clock driver uses platform driver
model and it is NOT ready during timer initialization phase,
the clock operations will fail and system counter driver will
fail too. As all the i.MX8M platforms' system counter clock
are from OSC which is always enabled, so it is no need to enable
clock for system counter driver, the ONLY thing is to pass
clock frequence to driver.

To make system counter driver work for upper scenario, add an
option of skipping of_clk operation for system counter driver,
an optional property "clock-frequency" is introduced to pass
the frequency value to system counter driver and indicate driver
to skip of_clk operations.

Signed-off-by: Anson Huang 
---
Changes since V1:
- improve commit log, no content change.
---
 drivers/clocksource/timer-imx-sysctr.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/clocksource/timer-imx-sysctr.c 
b/drivers/clocksource/timer-imx-sysctr.c
index fd7d680..8ff3d7e 100644
--- a/drivers/clocksource/timer-imx-sysctr.c
+++ b/drivers/clocksource/timer-imx-sysctr.c
@@ -129,6 +129,14 @@ static void __init sysctr_clockevent_init(void)
 static int __init sysctr_timer_init(struct device_node *np)
 {
int ret = 0;
+   u32 rate;
+
+   if (!of_property_read_u32(np, "clock-frequency",
+ )) {
+   to_sysctr.of_clk.rate = rate;
+   to_sysctr.of_clk.period = DIV_ROUND_UP(rate, HZ);
+   to_sysctr.flags &= ~TIMER_OF_CLOCK;
+   }
 
ret = timer_of_init(np, _sysctr);
if (ret)
-- 
2.7.4



RE: [PATCH V2 2/3] clocksource: imx-sysctr: Add of_clk skip option

2019-06-23 Thread Anson Huang
Hi, Thomas

> On Sun, 23 Jun 2019, anson.hu...@nxp.com wrote:
> 
> Again the short summary could be more informative. Instead of 'Add foo'
> you could say:
> 
> .: Make timer work with platform driver model
> 
> That sums up the real meat of the patch. 'Add some option' is pretty
> uninformative.

Makes sense.

> 
> > On some i.MX8M platforms, clock driver uses platform driver model and
> > it is NOT ready during timer initialization phase, the clock
> > operations will fail and system counter driver will fail too. As all
> > the i.MX8M platforms' system counter clock are from OSC which is
> > always enabled, so it is no need to enable clock for system counter
> > driver, the ONLY thing is to pass clock frequence to driver.
> >
> > This patch adds an option of skipping of_clk operation for system
> > counter driver, an optional property "clock-frequency"
> > is introduced to pass the frequency value to system counter driver and
> > indicate driver to skip of_clk operations.
> 
> The comments to the changelog of patch 1 apply here as well :)
> 
> Hint: 'This patch'

Oops...did NOT notice that, I will resend the V2 patch series as they are 
actually
similar issues.

Thanks,
Anson

> 
> Thanks,
> 
>   tglx
> 



RE: [PATCH 1/3] clocksource/drivers/sysctr: Add an optional property

2019-06-23 Thread Anson Huang
Hi, Thomas

> Anson,
> 
> A: Because it messes up the order in which people normally read text.
> Q: Why is top-posting such a bad thing?
> A: Top-posting.
> Q: What is the most annoying thing in e-mail?
> 
> A: No.
> Q: Should I include quotations after my reply?
> 
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdaringf
> ireball.net%2F2007%2F07%2Fon_topdata=02%7C01%7Canson.huang
> %40nxp.com%7C658d12e0d65a401034d208d6f7cecc86%7C686ea1d3bc2b4c6
> fa92cd99c5c301635%7C0%7C1%7C636968864908338236sdata=%2F0%
> 2B9DIT2HVuVFgLvhW7QFFNAXrRqbcTi9%2BJcasgOv08%3Dreserved=0
> 

Thanks for these info.

> On Sun, 23 Jun 2019, Anson Huang wrote:
> 
> > Hi, Thomas
> > Thanks for the useful comment, I will resend the patch with
> improvement.
> >
> > Anson.
> > 
> Also please fix your mailer to NOT copy the full mail header into the reply.
> That's absolutely useless.

Sure, thanks for reminder.

> 
> > >
> > > Anson,
> > >
> > > On Fri, 21 Jun 2019, anson.hu...@nxp.com wrote:
> > >
> > > > Subject : [PATCH 1/3] clocksource/drivers/sysctr: Add an optional
> > > > property
> 
> And finally please trim your replies, so the uninteresting parts are gone.
>

OK.

Thanks,
Anson.
 
> Thanks,
> 
>   tglx


[PATCH V2 2/3] clocksource: imx-sysctr: Add of_clk skip option

2019-06-23 Thread Anson . Huang
From: Anson Huang 

On some i.MX8M platforms, clock driver uses platform driver
model and it is NOT ready during timer initialization phase,
the clock operations will fail and system counter driver will
fail too. As all the i.MX8M platforms' system counter clock
are from OSC which is always enabled, so it is no need to enable
clock for system counter driver, the ONLY thing is to pass
clock frequence to driver.

This patch adds an option of skipping of_clk operation for
system counter driver, an optional property "clock-frequency"
is introduced to pass the frequency value to system counter
driver and indicate driver to skip of_clk operations.

Signed-off-by: Anson Huang 
---
No change.
---
 drivers/clocksource/timer-imx-sysctr.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/clocksource/timer-imx-sysctr.c 
b/drivers/clocksource/timer-imx-sysctr.c
index fd7d680..8ff3d7e 100644
--- a/drivers/clocksource/timer-imx-sysctr.c
+++ b/drivers/clocksource/timer-imx-sysctr.c
@@ -129,6 +129,14 @@ static void __init sysctr_clockevent_init(void)
 static int __init sysctr_timer_init(struct device_node *np)
 {
int ret = 0;
+   u32 rate;
+
+   if (!of_property_read_u32(np, "clock-frequency",
+ )) {
+   to_sysctr.of_clk.rate = rate;
+   to_sysctr.of_clk.period = DIV_ROUND_UP(rate, HZ);
+   to_sysctr.flags &= ~TIMER_OF_CLOCK;
+   }
 
ret = timer_of_init(np, _sysctr);
if (ret)
-- 
2.7.4



[PATCH V2 1/3] clocksource/drivers/sysctr: Add optional clock-frequency property

2019-06-23 Thread Anson . Huang
From: Anson Huang 

Systems which use platform driver model for clock driver require the
clock frequency to be supplied via device tree when system counter
driver is enabled.

This is necessary as in the platform driver model the of_clk operations
do not work correctly because system counter driver is initialized in
early phase of system boot up, and clock driver using platform driver
model is NOT ready at that time, it will cause system counter driver
initialization failed.

Add the optinal clock-frequency to the device tree bindings of the NXP
system counter, so the frequency can be handed in and the of_clk
operations can be skipped.

Signed-off-by: Anson Huang 
---
Changes since V1:
- improve commit log, no content change.
---
 Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt 
b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
index d576599..c9907a0 100644
--- a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
+++ b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
@@ -14,6 +14,11 @@ Required properties:
 - clocks : Specifies the counter clock.
 - clock-names: Specifies the clock's name of this module
 
+Optional properties:
+
+- clock-frequency : Specifies system counter clock frequency and indicates 
system
+   counter driver to skip clock operations.
+
 Example:
 
system_counter: timer@306a {
@@ -22,4 +27,5 @@ Example:
clocks = <_8m>;
clock-names = "per";
interrupts = ;
+   clock-frequency = <833>;
};
-- 
2.7.4



[PATCH V2 3/3] arm64: dts: imx8mq: Add system counter node

2019-06-23 Thread Anson . Huang
From: Anson Huang 

Add i.MX8MQ system counter node to enable timer-imx-sysctr
broadcast timer driver.

Signed-off-by: Anson Huang 
---
No change.
---
 arch/arm64/boot/dts/freescale/imx8mq.dtsi | 8 
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi 
b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index d09b808..9d99191 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -635,6 +635,14 @@
#pwm-cells = <2>;
status = "disabled";
};
+
+   system_counter: timer@306a {
+   compatible = "nxp,sysctr-timer";
+   reg = <0x306a 0x3>;
+   interrupts = ,
+;
+   clock-frequency = <833>;
+   };
};
 
bus@3080 { /* AIPS3 */
-- 
2.7.4



RE: [PATCH 1/3] clocksource/drivers/sysctr: Add an optional property

2019-06-23 Thread Anson Huang
Hi, Thomas
Thanks for the useful comment, I will resend the patch with improvement.

Anson.

> -Original Message-
> From: Thomas Gleixner 
> Sent: Sunday, June 23, 2019 6:47 PM
> To: Anson Huang 
> Cc: daniel.lezc...@linaro.org; robh...@kernel.org; mark.rutl...@arm.com;
> shawn...@kernel.org; s.ha...@pengutronix.de; ker...@pengutronix.de;
> feste...@gmail.com; l.st...@pengutronix.de; Abel Vesa
> ; ccai...@baylibre.com; an...@akkea.ca;
> andrew.smir...@gmail.com; a...@sigxcpu.org; linux-
> ker...@vger.kernel.org; devicet...@vger.kernel.org; linux-arm-
> ker...@lists.infradead.org; dl-linux-imx 
> Subject: Re: [PATCH 1/3] clocksource/drivers/sysctr: Add an optional
> property
> 
> Anson,
> 
> On Fri, 21 Jun 2019, anson.hu...@nxp.com wrote:
> 
> > Subject : [PATCH 1/3] clocksource/drivers/sysctr: Add an optional
> > property
> 
> That subject line is not really informative. From Documentation:
> 
>  The ``summary phrase`` in the email's Subject should concisely
>  describe the patch which that email contains.
> 
> That means that it should tell which property it adds so it's immediately 
> clear
> what this is about. Something like:
> 
>  Subject: clocksource/drivers/sysctr: Add optional clock-frequency property
> 
> Hmm?
> 
> > From: Anson Huang 
> >
> > This patch adds an optional property "clock-frequency" to pass
> 
> Please read Documentation/process/submitting-patches.rst and search for
> 'This patch'
> 
> > the system counter frequency value to kernel system counter driver and
> > indicate the driver to skip of_clk operations, this is to support
> > those platforms using platform driver model for clock driver.
> 
> That sentence does not parse. Please structure your changelog in the
> following order:
> 
>1) Context or problem
> 
>2) Detailed analysis (if applicable and necessary)
> 
>3) Short description of the solution (the rest is obvious from the patch
>   itself).
> 
> So something like this (assumed I decoded the above correctly):
> 
>Systems which use the system counter with the platform driver model
>require the clock frequency to be supplied via device tree.
> 
>This is necessary as in the platform driver model the of_clk operations
>do not work correctly because LENGHTY EXPLANATION WHY ...
> 
>Add the optinal clock-frequency to the device tree bindings of the NXP
>system counter so the frequency can be handed in and the of_clk
>operations can be skipped.
> 
> The important part is the missing LENGTHY EXPLANATION WHY. I can't fill
> that in because you did not provide that information.
> 
> Thanks,
> 
>   tglx


RE: [PATCH 3/3] arm64: dts: imx8mq: Add system counter node

2019-06-23 Thread Anson Huang


> -Original Message-
> From: Martin Kepplinger 
> Sent: Sunday, June 23, 2019 7:21 PM
> To: Anson Huang ; daniel.lezc...@linaro.org;
> t...@linutronix.de; robh...@kernel.org; mark.rutl...@arm.com;
> shawn...@kernel.org; s.ha...@pengutronix.de; ker...@pengutronix.de;
> feste...@gmail.com; l.st...@pengutronix.de; Abel Vesa
> ; ccai...@baylibre.com; an...@akkea.ca;
> andrew.smir...@gmail.com; a...@sigxcpu.org; linux-
> ker...@vger.kernel.org; devicet...@vger.kernel.org; linux-arm-
> ker...@lists.infradead.org
> Cc: dl-linux-imx 
> Subject: Re: [PATCH 3/3] arm64: dts: imx8mq: Add system counter node
> 
> On 22.06.19 16:16, Martin Kepplinger wrote:
> > On 21.06.19 10:28, anson.hu...@nxp.com wrote:
> >> From: Anson Huang 
> >>
> >> Add i.MX8MQ system counter node to enable timer-imx-sysctr broadcast
> >> timer driver.
> >
> > with these changes and TIMER_IMX_SYS_CTR selected, I don't see cpuidle
> > working yet (which is what I want to achieve on imx8mq). Might there
> > be a system counter clock definition or anything else missing?
> >
> 
> To be clear about what I tried running: Abel's wakeup-workaround (with the
> corresponding ATF changes):
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.o
> rg%2Flkml%2F2019%2F6%2F10%2F350data=02%7C01%7CAnson.Huan
> g%40nxp.com%7C445690743187422a2d2b08d6f7ccd665%7C686ea1d3bc2b4
> c6fa92cd99c5c301635%7C0%7C0%7C636968856477185012sdata=k1Aj
> CJ5SGUYQE7VnzciihRTKgf1yi4bDaBqCCv9DzpU%3Dreserved=0 plus
> your patches here (although you don't define a system counter clock, like you
> do in your imx8mm patches).
> 
> In any case, I might try to enable cpuidle totally wrong still :) and I'd be 
> happy
> for hints and test your changes (no matter how fit they are to be merged
> right now).

Could be something else than this patch series, i.MX8MQ uses platform driver 
model
for clock driver, to enable system counter driver, something needs to be 
changed in
system counter driver, and no need to have system counter clock definition in 
i.MX8MQ
clock tree, while i.MX8MM uses OF_CLK as clock driver initialization, so we 
need system
counter clock definition.

My understanding is, even without this patch series, cpu-idle should be able to 
work, but
The last CPU could be always powered ON to ack as broadcast timer, if with 
system counter
enabled, all CPUs can be powered OFF when entering state #1 idle independently, 
as system
counter can be broadcast timer. Correct me if anything wrong, maybe latest 
upstream kernel
has something different about this part. 

Thanks,
Anson.

> 
> thanks,
>  martin



RE: [PATCH 3/3] arm64: dts: imx8mq: Add system counter node

2019-06-23 Thread Anson Huang
Hi, Martin

> -Original Message-
> From: Martin Kepplinger 
> Sent: Saturday, June 22, 2019 10:16 PM
> To: Anson Huang ; daniel.lezc...@linaro.org;
> t...@linutronix.de; robh...@kernel.org; mark.rutl...@arm.com;
> shawn...@kernel.org; s.ha...@pengutronix.de; ker...@pengutronix.de;
> feste...@gmail.com; l.st...@pengutronix.de; Abel Vesa
> ; ccai...@baylibre.com; an...@akkea.ca;
> andrew.smir...@gmail.com; a...@sigxcpu.org; linux-
> ker...@vger.kernel.org; devicet...@vger.kernel.org; linux-arm-
> ker...@lists.infradead.org
> Cc: dl-linux-imx 
> Subject: Re: [PATCH 3/3] arm64: dts: imx8mq: Add system counter node
> 
> On 21.06.19 10:28, anson.hu...@nxp.com wrote:
> > From: Anson Huang 
> >
> > Add i.MX8MQ system counter node to enable timer-imx-sysctr broadcast
> > timer driver.
> 
> with these changes and TIMER_IMX_SYS_CTR selected, I don't see cpuidle
> working yet (which is what I want to achieve on imx8mq). Might there be a
> system counter clock definition or anything else missing?

i.MX8MQ is different about system counter enablement, with this patch series,
no need to have system counter clock definition, this patch is just to enable 
the
system counter as broadcast timer, it is necessary for further support of 
cpu-idle,
to enable cpu-idle, another DT patch is needed to add idle node, but as far as  
I
know, Abel is working on the workaround of the i.MX8MQ cpu-idle, I don't know
if it is picked up or NOT, so I believe the cpu-idle will be enabled later for 
i.MX8MQ
by Abel.

Thanks,
Anson.

> 
> thanks,
>  martin
> 



RE: [PATCH 4/4] arm64: dts: imx8mm: Add system counter node

2019-06-23 Thread Anson Huang
Hi, Martin

> -Original Message-
> From: Martin Kepplinger 
> Sent: Saturday, June 22, 2019 8:10 PM
> To: Anson Huang ; catalin.mari...@arm.com;
> w...@kernel.org; robh...@kernel.org; mark.rutl...@arm.com;
> shawn...@kernel.org; s.ha...@pengutronix.de; ker...@pengutronix.de;
> feste...@gmail.com; mturque...@baylibre.com; sb...@kernel.org;
> Leonard Crestez ; Aisheng Dong
> ; Jacky Bai ; Daniel Baluta
> ; Peng Fan ; Abel Vesa
> ; linux-arm-ker...@lists.infradead.org; linux-
> ker...@vger.kernel.org; devicet...@vger.kernel.org; linux-
> c...@vger.kernel.org
> Cc: dl-linux-imx 
> Subject: Re: [PATCH 4/4] arm64: dts: imx8mm: Add system counter node
> 
> On 21.06.19 09:07, anson.hu...@nxp.com wrote:
> > From: Anson Huang 
> >
> > Add i.MX8MM system counter node to enable timer-imx-sysctr broadcast
> > timer driver.
> >
> 
> 
> do we need similar additions to imx8mq? If so, I think these would fit in here
> too.

i.MX8MQ has something different about system counter driver enablement, I did
it in another patch series.

Anson.

> 
> thanks,
> martin



[PATCH 3/3] arm64: dts: imx8mq: Add system counter node

2019-06-21 Thread Anson . Huang
From: Anson Huang 

Add i.MX8MQ system counter node to enable timer-imx-sysctr
broadcast timer driver.

Signed-off-by: Anson Huang 
---
 arch/arm64/boot/dts/freescale/imx8mq.dtsi | 8 
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi 
b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index d09b808..9d99191 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -635,6 +635,14 @@
#pwm-cells = <2>;
status = "disabled";
};
+
+   system_counter: timer@306a {
+   compatible = "nxp,sysctr-timer";
+   reg = <0x306a 0x3>;
+   interrupts = ,
+;
+   clock-frequency = <833>;
+   };
};
 
bus@3080 { /* AIPS3 */
-- 
2.7.4



<    4   5   6   7   8   9   10   11   12   13   >