Thermal Monitor Unit v2 is introduced on new Layscape SoC.
Compared to v1, TMUv2 has a little different register layout
and digital output is fairly linear.
Signed-off-by: Yuantian Tang <[email protected]>
---
drivers/thermal/qoriq_thermal.c | 122 +++++++++++++++++++++++++-------
1 file changed, 98 insertions(+), 24 deletions(-)
diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 3b5f5b3fb1bc..0df6dfddf804 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -13,6 +13,15 @@
#include "thermal_core.h"
#define SITES_MAX 16
+#define TMR_DISABLE 0x0
+#define TMR_ME 0x80000000
+#define TMR_ALPF 0x0c000000
+#define TMR_ALPF_V2 0x03000000
+#define TMTMIR_DEFAULT 0x0000000f
+#define TIER_DISABLE 0x0
+#define TEUMR0_V2 0x51009C00
+#define TMU_VER1 0x1
+#define TMU_VER2 0x2
/*
* QorIQ TMU Registers
@@ -23,17 +32,55 @@ struct qoriq_tmu_site_regs {
u8 res0[0x8];
};
-struct qoriq_tmu_regs {
+struct qoriq_tmu_regs_v2 {
+ u32 tmr; /* Mode Register */
+ u32 tsr; /* Status Register */
+ u32 tmsr; /* monitor site register */
+ u32 tmtmir; /* Temperature measurement interval Register */
+ u8 res0[0x10];
+ u32 tier; /* Interrupt Enable Register */
+ u32 tidr; /* Interrupt Detect Register */
+ u8 res1[0x8];
+ u32 tiiscr; /* interrupt immediate site capture register */
+ u32 tiascr; /* interrupt average site capture register */
+ u32 ticscr; /* Interrupt Critical Site Capture Register */
+ u32 res2;
+ u32 tmhtcr; /* monitor high temperature capture register */
+ u32 tmltcr; /* monitor low temperature capture register */
+ u32 tmrtrcr; /* monitor rising temperature rate capture register */
+ u32 tmftrcr; /* monitor falling temperature rate capture register */
+ u32 tmhtitr; /* High Temperature Immediate Threshold */
+ u32 tmhtatr; /* High Temperature Average Threshold */
+ u32 tmhtactr; /* High Temperature Average Crit Threshold */
+ u32 res3;
+ u32 tmltitr; /* monitor low temperature immediate threshold */
+ u32 tmltatr; /* monitor low temperature average threshold register */
+ u32 tmltactr; /* monitor low temperature average critical threshold */
+ u32 res4;
+ u32 tmrtrctr; /* monitor rising temperature rate critical threshold */
+ u32 tmftrctr; /* monitor falling temperature rate critical threshold*/
+ u8 res5[0x8];
+ u32 ttcfgr; /* Temperature Configuration Register */
+ u32 tscfgr; /* Sensor Configuration Register */
+ u8 res6[0x78];
+ struct qoriq_tmu_site_regs site[SITES_MAX];
+ u8 res7[0x9f8];
+ u32 ipbrr0; /* IP Block Revision Register 0 */
+ u32 ipbrr1; /* IP Block Revision Register 1 */
+ u8 res8[0x300];
+ u32 teumr0;
+ u32 teumr1;
+ u32 teumr2;
+ u32 res9;
+ u32 ttrcr[4]; /* Temperature Range Control Register */
+};
+
+struct qoriq_tmu_regs_v1 {
u32 tmr; /* Mode Register */
-#define TMR_DISABLE 0x0
-#define TMR_ME 0x80000000
-#define TMR_ALPF 0x0c000000
u32 tsr; /* Status Register */
u32 tmtmir; /* Temperature measurement interval Register */
-#define TMTMIR_DEFAULT 0x0000000f
u8 res0[0x14];
u32 tier; /* Interrupt Enable Register */
-#define TIER_DISABLE 0x0
u32 tidr; /* Interrupt Detect Register */
u32 tiscr; /* Interrupt Site Capture Register */
u32 ticscr; /* Interrupt Critical Site Capture Register */
@@ -53,10 +100,7 @@ struct qoriq_tmu_regs {
u32 ipbrr0; /* IP Block Revision Register 0 */
u32 ipbrr1; /* IP Block Revision Register 1 */
u8 res6[0x310];
- u32 ttr0cr; /* Temperature Range 0 Control Register */
- u32 ttr1cr; /* Temperature Range 1 Control Register */
- u32 ttr2cr; /* Temperature Range 2 Control Register */
- u32 ttr3cr; /* Temperature Range 3 Control Register */
+ u32 ttrcr[4]; /* Temperature Range Control Register */
};
struct qoriq_tmu_data;
@@ -71,7 +115,9 @@ struct qoriq_sensor {
};
struct qoriq_tmu_data {
- struct qoriq_tmu_regs __iomem *regs;
+ int ver;
+ struct qoriq_tmu_regs_v1 __iomem *regs;
+ struct qoriq_tmu_regs_v2 __iomem *regv2;
bool little_endian;
struct qoriq_sensor *sensor[SITES_MAX];
};
@@ -111,7 +157,7 @@ static const struct thermal_zone_of_device_ops tmu_tz_ops = {
static int qoriq_tmu_register_tmu_zone(struct platform_device *pdev)
{
struct qoriq_tmu_data *qdata = platform_get_drvdata(pdev);
- int id, sites = 0;
+ int id, sites = 0, sv2 = 0;
for (id = 0; id < SITES_MAX; id++) {
qdata->sensor[id] = devm_kzalloc(&pdev->dev,
@@ -130,12 +176,24 @@ static int qoriq_tmu_register_tmu_zone(struct platform_device *pdev)
return PTR_ERR(qdata->sensor[id]->tzd);
}
- sites |= 0x1 << (15 - id);
+ if (qdata->ver == TMU_VER1)
+ sites |= 0x1 << (15 - id);
+ else
+ sv2 |= 0x1 << id;
}
/* Enable monitoring */
- if (sites != 0)
- tmu_write(qdata, sites | TMR_ME | TMR_ALPF, &qdata->regs->tmr);
+ if (qdata->ver == TMU_VER1) {
+ if (sites != 0)
+ tmu_write(qdata, sites | TMR_ME | TMR_ALPF,
+ &qdata->regs->tmr);
+ } else {
+ if (sv2 != 0) {
+ tmu_write(qdata, sv2, &qdata->regv2->tmsr);
+ tmu_write(qdata, TMR_ME | TMR_ALPF_V2,
+ &qdata->regv2->tmr);
+ }
+ }
return 0;
}
@@ -148,16 +206,20 @@ static int qoriq_tmu_calibration(struct platform_device *pdev)
struct device_node *np = pdev->dev.of_node;
struct qoriq_tmu_data *data = platform_get_drvdata(pdev);
- if (of_property_read_u32_array(np, "fsl,tmu-range", range, 4)) {
- dev_err(&pdev->dev, "missing calibration range.\n");
- return -ENODEV;
+ len = of_property_count_u32_elems(np, "fsl,tmu-range");
+ if (len == -ENODATA || len == -EINVAL || len > 4) {
+ dev_err(&pdev->dev, "invalid range data.\n");
+ return len;
}
- /* Init temperature range registers */
- tmu_write(data, range[0], &data->regs->ttr0cr);
- tmu_write(data, range[1], &data->regs->ttr1cr);
- tmu_write(data, range[2], &data->regs->ttr2cr);
- tmu_write(data, range[3], &data->regs->ttr3cr);
+ val = of_property_read_u32_array(np, "fsl,tmu-range", range, len);
+ if (val != 0) {
+ dev_err(&pdev->dev, "invalid range data.\n");
+ return val;
+ }
+
+ for (i = 0; i < len; i++)
+ tmu_write(data, range[i], &data->regs->ttrcr[i]);
calibration = of_get_property(np, "fsl,tmu-calibration", &len);
if (calibration == NULL || len % 8) {
@@ -181,7 +243,12 @@ static void qoriq_tmu_init_device(struct qoriq_tmu_data *data)
tmu_write(data, TIER_DISABLE, &data->regs->tier);
/* Set update_interval */
- tmu_write(data, TMTMIR_DEFAULT, &data->regs->tmtmir);
+ if (data->ver == TMU_VER1) {
+ tmu_write(data, TMTMIR_DEFAULT, &data->regs->tmtmir);
+ } else {
+ tmu_write(data, TMTMIR_DEFAULT, &data->regv2->tmtmir);
+ tmu_write(data, TEUMR0_V2, &data->regv2->teumr0);
+ }
/* Disable monitoring */
tmu_write(data, TMR_DISABLE, &data->regs->tmr);
@@ -190,6 +257,7 @@ static void qoriq_tmu_init_device(struct qoriq_tmu_data *data)
static int qoriq_tmu_probe(struct platform_device *pdev)
{
int ret;
+ u32 ver;
struct qoriq_tmu_data *data;
struct device_node *np = pdev->dev.of_node;
@@ -214,6 +282,12 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
goto err_iomap;
}
+ /* version register offset at: 0xbf8 on both v1 and v2 */
+ ver = tmu_read(data, &data->regs->ipbrr0);
+ data->ver = (ver >> 8) & 0xff;
+ if (data->ver == TMU_VER2)
+ data->regv2 = (void __iomem *)data->regs;
+
qoriq_tmu_init_device(data); /* TMU initialization */
ret = qoriq_tmu_calibration(pdev); /* TMU calibration */
--
2.17.1
Any comments?
BR,
Andy
> -----Original Message-----
> From: Yuantian Tang <[email protected]>
> Sent: 2019??6??4?? 10:51
> To: [email protected]; [email protected]
> Cc: [email protected]; Leo Li <[email protected]>;
> [email protected]; [email protected]; Andy Tang
> <[email protected]>
> Subject: [PATCH] thermal: qoriq: add thermal monitor unit version 2 support
>
> Thermal Monitor Unit v2 is introduced on new Layscape SoC.
> Compared to v1, TMUv2 has a little different register layout and digital
> output is fairly linear.
>
> Signed-off-by: Yuantian Tang <[email protected]>
> ---
> drivers/thermal/qoriq_thermal.c | 122 +++++++++++++++++++++++++-------
> 1 file changed, 98 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/thermal/qoriq_thermal.c
> b/drivers/thermal/qoriq_thermal.c index 3b5f5b3fb1bc..0df6dfddf804 100644
> --- a/drivers/thermal/qoriq_thermal.c
> +++ b/drivers/thermal/qoriq_thermal.c
> @@ -13,6 +13,15 @@
> #include "thermal_core.h"
>
> #define SITES_MAX 16
> +#define TMR_DISABLE 0x0
> +#define TMR_ME 0x80000000
> +#define TMR_ALPF 0x0c000000
> +#define TMR_ALPF_V2 0x03000000
> +#define TMTMIR_DEFAULT 0x0000000f
> +#define TIER_DISABLE 0x0
> +#define TEUMR0_V2 0x51009C00
> +#define TMU_VER1 0x1
> +#define TMU_VER2 0x2
>
> /*
> * QorIQ TMU Registers
> @@ -23,17 +32,55 @@ struct qoriq_tmu_site_regs {
> u8 res0[0x8];
> };
>
> -struct qoriq_tmu_regs {
> +struct qoriq_tmu_regs_v2 {
> + u32 tmr; /* Mode Register */
> + u32 tsr; /* Status Register */
> + u32 tmsr; /* monitor site register */
> + u32 tmtmir; /* Temperature measurement interval Register */
> + u8 res0[0x10];
> + u32 tier; /* Interrupt Enable Register */
> + u32 tidr; /* Interrupt Detect Register */
> + u8 res1[0x8];
> + u32 tiiscr; /* interrupt immediate site capture register */
> + u32 tiascr; /* interrupt average site capture register */
> + u32 ticscr; /* Interrupt Critical Site Capture Register */
> + u32 res2;
> + u32 tmhtcr; /* monitor high temperature capture register */
> + u32 tmltcr; /* monitor low temperature capture register */
> + u32 tmrtrcr; /* monitor rising temperature rate capture register */
> + u32 tmftrcr; /* monitor falling temperature rate capture register */
> + u32 tmhtitr; /* High Temperature Immediate Threshold */
> + u32 tmhtatr; /* High Temperature Average Threshold */
> + u32 tmhtactr; /* High Temperature Average Crit Threshold */
> + u32 res3;
> + u32 tmltitr; /* monitor low temperature immediate threshold */
> + u32 tmltatr; /* monitor low temperature average threshold register */
> + u32 tmltactr; /* monitor low temperature average critical threshold */
> + u32 res4;
> + u32 tmrtrctr; /* monitor rising temperature rate critical threshold */
> + u32 tmftrctr; /* monitor falling temperature rate critical threshold*/
> + u8 res5[0x8];
> + u32 ttcfgr; /* Temperature Configuration Register */
> + u32 tscfgr; /* Sensor Configuration Register */
> + u8 res6[0x78];
> + struct qoriq_tmu_site_regs site[SITES_MAX];
> + u8 res7[0x9f8];
> + u32 ipbrr0; /* IP Block Revision Register 0 */
> + u32 ipbrr1; /* IP Block Revision Register 1 */
> + u8 res8[0x300];
> + u32 teumr0;
> + u32 teumr1;
> + u32 teumr2;
> + u32 res9;
> + u32 ttrcr[4]; /* Temperature Range Control Register */
> +};
> +
> +struct qoriq_tmu_regs_v1 {
> u32 tmr; /* Mode Register */
> -#define TMR_DISABLE 0x0
> -#define TMR_ME 0x80000000
> -#define TMR_ALPF 0x0c000000
> u32 tsr; /* Status Register */
> u32 tmtmir; /* Temperature measurement interval Register */
> -#define TMTMIR_DEFAULT 0x0000000f
> u8 res0[0x14];
> u32 tier; /* Interrupt Enable Register */
> -#define TIER_DISABLE 0x0
> u32 tidr; /* Interrupt Detect Register */
> u32 tiscr; /* Interrupt Site Capture Register */
> u32 ticscr; /* Interrupt Critical Site Capture Register */
> @@ -53,10 +100,7 @@ struct qoriq_tmu_regs {
> u32 ipbrr0; /* IP Block Revision Register 0 */
> u32 ipbrr1; /* IP Block Revision Register 1 */
> u8 res6[0x310];
> - u32 ttr0cr; /* Temperature Range 0 Control Register */
> - u32 ttr1cr; /* Temperature Range 1 Control Register */
> - u32 ttr2cr; /* Temperature Range 2 Control Register */
> - u32 ttr3cr; /* Temperature Range 3 Control Register */
> + u32 ttrcr[4]; /* Temperature Range Control Register */
> };
>
> struct qoriq_tmu_data;
> @@ -71,7 +115,9 @@ struct qoriq_sensor { };
>
> struct qoriq_tmu_data {
> - struct qoriq_tmu_regs __iomem *regs;
> + int ver;
> + struct qoriq_tmu_regs_v1 __iomem *regs;
> + struct qoriq_tmu_regs_v2 __iomem *regv2;
> bool little_endian;
> struct qoriq_sensor *sensor[SITES_MAX];
> };
> @@ -111,7 +157,7 @@ static const struct thermal_zone_of_device_ops
> tmu_tz_ops = { static int qoriq_tmu_register_tmu_zone(struct
> platform_device *pdev) {
> struct qoriq_tmu_data *qdata = platform_get_drvdata(pdev);
> - int id, sites = 0;
> + int id, sites = 0, sv2 = 0;
>
> for (id = 0; id < SITES_MAX; id++) {
> qdata->sensor[id] = devm_kzalloc(&pdev->dev, @@ -130,12
> +176,24 @@ static int qoriq_tmu_register_tmu_zone(struct platform_device
> *pdev)
> return PTR_ERR(qdata->sensor[id]->tzd);
> }
>
> - sites |= 0x1 << (15 - id);
> + if (qdata->ver == TMU_VER1)
> + sites |= 0x1 << (15 - id);
> + else
> + sv2 |= 0x1 << id;
> }
>
> /* Enable monitoring */
> - if (sites != 0)
> - tmu_write(qdata, sites | TMR_ME | TMR_ALPF,
> &qdata->regs->tmr);
> + if (qdata->ver == TMU_VER1) {
> + if (sites != 0)
> + tmu_write(qdata, sites | TMR_ME | TMR_ALPF,
> + &qdata->regs->tmr);
> + } else {
> + if (sv2 != 0) {
> + tmu_write(qdata, sv2, &qdata->regv2->tmsr);
> + tmu_write(qdata, TMR_ME | TMR_ALPF_V2,
> + &qdata->regv2->tmr);
> + }
> + }
>
> return 0;
> }
> @@ -148,16 +206,20 @@ static int qoriq_tmu_calibration(struct
> platform_device *pdev)
> struct device_node *np = pdev->dev.of_node;
> struct qoriq_tmu_data *data = platform_get_drvdata(pdev);
>
> - if (of_property_read_u32_array(np, "fsl,tmu-range", range, 4)) {
> - dev_err(&pdev->dev, "missing calibration range.\n");
> - return -ENODEV;
> + len = of_property_count_u32_elems(np, "fsl,tmu-range");
> + if (len == -ENODATA || len == -EINVAL || len > 4) {
> + dev_err(&pdev->dev, "invalid range data.\n");
> + return len;
> }
>
> - /* Init temperature range registers */
> - tmu_write(data, range[0], &data->regs->ttr0cr);
> - tmu_write(data, range[1], &data->regs->ttr1cr);
> - tmu_write(data, range[2], &data->regs->ttr2cr);
> - tmu_write(data, range[3], &data->regs->ttr3cr);
> + val = of_property_read_u32_array(np, "fsl,tmu-range", range, len);
> + if (val != 0) {
> + dev_err(&pdev->dev, "invalid range data.\n");
> + return val;
> + }
> +
> + for (i = 0; i < len; i++)
> + tmu_write(data, range[i], &data->regs->ttrcr[i]);
>
> calibration = of_get_property(np, "fsl,tmu-calibration", &len);
> if (calibration == NULL || len % 8) {
> @@ -181,7 +243,12 @@ static void qoriq_tmu_init_device(struct
> qoriq_tmu_data *data)
> tmu_write(data, TIER_DISABLE, &data->regs->tier);
>
> /* Set update_interval */
> - tmu_write(data, TMTMIR_DEFAULT, &data->regs->tmtmir);
> + if (data->ver == TMU_VER1) {
> + tmu_write(data, TMTMIR_DEFAULT, &data->regs->tmtmir);
> + } else {
> + tmu_write(data, TMTMIR_DEFAULT, &data->regv2->tmtmir);
> + tmu_write(data, TEUMR0_V2, &data->regv2->teumr0);
> + }
>
> /* Disable monitoring */
> tmu_write(data, TMR_DISABLE, &data->regs->tmr); @@ -190,6 +257,7
> @@ static void qoriq_tmu_init_device(struct qoriq_tmu_data *data) static
> int qoriq_tmu_probe(struct platform_device *pdev) {
> int ret;
> + u32 ver;
> struct qoriq_tmu_data *data;
> struct device_node *np = pdev->dev.of_node;
>
> @@ -214,6 +282,12 @@ static int qoriq_tmu_probe(struct platform_device
> *pdev)
> goto err_iomap;
> }
>
> + /* version register offset at: 0xbf8 on both v1 and v2 */
> + ver = tmu_read(data, &data->regs->ipbrr0);
> + data->ver = (ver >> 8) & 0xff;
> + if (data->ver == TMU_VER2)
> + data->regv2 = (void __iomem *)data->regs;
> +
> qoriq_tmu_init_device(data); /* TMU initialization */
>
> ret = qoriq_tmu_calibration(pdev); /* TMU calibration */
> --
> 2.17.1
Hi Rui, Edubezval,
Almost three monthes passed, I have not got your comments from you.
Could you please take a look at this patch?
BR,
Andy
> -----Original Message-----
> From: Andy Tang
> Sent: 2019??8??6?? 10:57
> To: [email protected]; [email protected]
> Cc: [email protected]; Leo Li <[email protected]>;
> [email protected]; [email protected]
> Subject: RE: [PATCH] thermal: qoriq: add thermal monitor unit version 2
> support
>
> Any comments?
>
> BR,
> Andy
>
> > -----Original Message-----
> > From: Yuantian Tang <[email protected]>
> > Sent: 2019??6??4?? 10:51
> > To: [email protected]; [email protected]
> > Cc: [email protected]; Leo Li <[email protected]>;
> > [email protected]; [email protected]; Andy Tang
> > <[email protected]>
> > Subject: [PATCH] thermal: qoriq: add thermal monitor unit version 2
> > support
> >
> > Thermal Monitor Unit v2 is introduced on new Layscape SoC.
> > Compared to v1, TMUv2 has a little different register layout and
> > digital output is fairly linear.
> >
> > Signed-off-by: Yuantian Tang <[email protected]>
> > ---
> > drivers/thermal/qoriq_thermal.c | 122
> > +++++++++++++++++++++++++-------
> > 1 file changed, 98 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/thermal/qoriq_thermal.c
> > b/drivers/thermal/qoriq_thermal.c index 3b5f5b3fb1bc..0df6dfddf804
> > 100644
> > --- a/drivers/thermal/qoriq_thermal.c
> > +++ b/drivers/thermal/qoriq_thermal.c
> > @@ -13,6 +13,15 @@
> > #include "thermal_core.h"
> >
> > #define SITES_MAX 16
> > +#define TMR_DISABLE 0x0
> > +#define TMR_ME 0x80000000
> > +#define TMR_ALPF 0x0c000000
> > +#define TMR_ALPF_V2 0x03000000
> > +#define TMTMIR_DEFAULT 0x0000000f
> > +#define TIER_DISABLE 0x0
> > +#define TEUMR0_V2 0x51009C00
> > +#define TMU_VER1 0x1
> > +#define TMU_VER2 0x2
> >
> > /*
> > * QorIQ TMU Registers
> > @@ -23,17 +32,55 @@ struct qoriq_tmu_site_regs {
> > u8 res0[0x8];
> > };
> >
> > -struct qoriq_tmu_regs {
> > +struct qoriq_tmu_regs_v2 {
> > + u32 tmr; /* Mode Register */
> > + u32 tsr; /* Status Register */
> > + u32 tmsr; /* monitor site register */
> > + u32 tmtmir; /* Temperature measurement interval Register */
> > + u8 res0[0x10];
> > + u32 tier; /* Interrupt Enable Register */
> > + u32 tidr; /* Interrupt Detect Register */
> > + u8 res1[0x8];
> > + u32 tiiscr; /* interrupt immediate site capture register */
> > + u32 tiascr; /* interrupt average site capture register */
> > + u32 ticscr; /* Interrupt Critical Site Capture Register */
> > + u32 res2;
> > + u32 tmhtcr; /* monitor high temperature capture register */
> > + u32 tmltcr; /* monitor low temperature capture register */
> > + u32 tmrtrcr; /* monitor rising temperature rate capture register */
> > + u32 tmftrcr; /* monitor falling temperature rate capture register */
> > + u32 tmhtitr; /* High Temperature Immediate Threshold */
> > + u32 tmhtatr; /* High Temperature Average Threshold */
> > + u32 tmhtactr; /* High Temperature Average Crit Threshold */
> > + u32 res3;
> > + u32 tmltitr; /* monitor low temperature immediate threshold */
> > + u32 tmltatr; /* monitor low temperature average threshold register */
> > + u32 tmltactr; /* monitor low temperature average critical threshold */
> > + u32 res4;
> > + u32 tmrtrctr; /* monitor rising temperature rate critical threshold */
> > + u32 tmftrctr; /* monitor falling temperature rate critical threshold*/
> > + u8 res5[0x8];
> > + u32 ttcfgr; /* Temperature Configuration Register */
> > + u32 tscfgr; /* Sensor Configuration Register */
> > + u8 res6[0x78];
> > + struct qoriq_tmu_site_regs site[SITES_MAX];
> > + u8 res7[0x9f8];
> > + u32 ipbrr0; /* IP Block Revision Register 0 */
> > + u32 ipbrr1; /* IP Block Revision Register 1 */
> > + u8 res8[0x300];
> > + u32 teumr0;
> > + u32 teumr1;
> > + u32 teumr2;
> > + u32 res9;
> > + u32 ttrcr[4]; /* Temperature Range Control Register */
> > +};
> > +
> > +struct qoriq_tmu_regs_v1 {
> > u32 tmr; /* Mode Register */
> > -#define TMR_DISABLE 0x0
> > -#define TMR_ME 0x80000000
> > -#define TMR_ALPF 0x0c000000
> > u32 tsr; /* Status Register */
> > u32 tmtmir; /* Temperature measurement interval Register */
> > -#define TMTMIR_DEFAULT 0x0000000f
> > u8 res0[0x14];
> > u32 tier; /* Interrupt Enable Register */
> > -#define TIER_DISABLE 0x0
> > u32 tidr; /* Interrupt Detect Register */
> > u32 tiscr; /* Interrupt Site Capture Register */
> > u32 ticscr; /* Interrupt Critical Site Capture Register */
> > @@ -53,10 +100,7 @@ struct qoriq_tmu_regs {
> > u32 ipbrr0; /* IP Block Revision Register 0 */
> > u32 ipbrr1; /* IP Block Revision Register 1 */
> > u8 res6[0x310];
> > - u32 ttr0cr; /* Temperature Range 0 Control Register */
> > - u32 ttr1cr; /* Temperature Range 1 Control Register */
> > - u32 ttr2cr; /* Temperature Range 2 Control Register */
> > - u32 ttr3cr; /* Temperature Range 3 Control Register */
> > + u32 ttrcr[4]; /* Temperature Range Control Register */
> > };
> >
> > struct qoriq_tmu_data;
> > @@ -71,7 +115,9 @@ struct qoriq_sensor { };
> >
> > struct qoriq_tmu_data {
> > - struct qoriq_tmu_regs __iomem *regs;
> > + int ver;
> > + struct qoriq_tmu_regs_v1 __iomem *regs;
> > + struct qoriq_tmu_regs_v2 __iomem *regv2;
> > bool little_endian;
> > struct qoriq_sensor *sensor[SITES_MAX];
> > };
> > @@ -111,7 +157,7 @@ static const struct thermal_zone_of_device_ops
> > tmu_tz_ops = { static int qoriq_tmu_register_tmu_zone(struct
> > platform_device *pdev) {
> > struct qoriq_tmu_data *qdata = platform_get_drvdata(pdev);
> > - int id, sites = 0;
> > + int id, sites = 0, sv2 = 0;
> >
> > for (id = 0; id < SITES_MAX; id++) {
> > qdata->sensor[id] = devm_kzalloc(&pdev->dev, @@ -130,12
> > +176,24 @@ static int qoriq_tmu_register_tmu_zone(struct
> > +platform_device
> > *pdev)
> > return PTR_ERR(qdata->sensor[id]->tzd);
> > }
> >
> > - sites |= 0x1 << (15 - id);
> > + if (qdata->ver == TMU_VER1)
> > + sites |= 0x1 << (15 - id);
> > + else
> > + sv2 |= 0x1 << id;
> > }
> >
> > /* Enable monitoring */
> > - if (sites != 0)
> > - tmu_write(qdata, sites | TMR_ME | TMR_ALPF,
> > &qdata->regs->tmr);
> > + if (qdata->ver == TMU_VER1) {
> > + if (sites != 0)
> > + tmu_write(qdata, sites | TMR_ME | TMR_ALPF,
> > + &qdata->regs->tmr);
> > + } else {
> > + if (sv2 != 0) {
> > + tmu_write(qdata, sv2, &qdata->regv2->tmsr);
> > + tmu_write(qdata, TMR_ME | TMR_ALPF_V2,
> > + &qdata->regv2->tmr);
> > + }
> > + }
> >
> > return 0;
> > }
> > @@ -148,16 +206,20 @@ static int qoriq_tmu_calibration(struct
> > platform_device *pdev)
> > struct device_node *np = pdev->dev.of_node;
> > struct qoriq_tmu_data *data = platform_get_drvdata(pdev);
> >
> > - if (of_property_read_u32_array(np, "fsl,tmu-range", range, 4)) {
> > - dev_err(&pdev->dev, "missing calibration range.\n");
> > - return -ENODEV;
> > + len = of_property_count_u32_elems(np, "fsl,tmu-range");
> > + if (len == -ENODATA || len == -EINVAL || len > 4) {
> > + dev_err(&pdev->dev, "invalid range data.\n");
> > + return len;
> > }
> >
> > - /* Init temperature range registers */
> > - tmu_write(data, range[0], &data->regs->ttr0cr);
> > - tmu_write(data, range[1], &data->regs->ttr1cr);
> > - tmu_write(data, range[2], &data->regs->ttr2cr);
> > - tmu_write(data, range[3], &data->regs->ttr3cr);
> > + val = of_property_read_u32_array(np, "fsl,tmu-range", range, len);
> > + if (val != 0) {
> > + dev_err(&pdev->dev, "invalid range data.\n");
> > + return val;
> > + }
> > +
> > + for (i = 0; i < len; i++)
> > + tmu_write(data, range[i], &data->regs->ttrcr[i]);
> >
> > calibration = of_get_property(np, "fsl,tmu-calibration", &len);
> > if (calibration == NULL || len % 8) { @@ -181,7 +243,12 @@ static
> > void qoriq_tmu_init_device(struct qoriq_tmu_data *data)
> > tmu_write(data, TIER_DISABLE, &data->regs->tier);
> >
> > /* Set update_interval */
> > - tmu_write(data, TMTMIR_DEFAULT, &data->regs->tmtmir);
> > + if (data->ver == TMU_VER1) {
> > + tmu_write(data, TMTMIR_DEFAULT, &data->regs->tmtmir);
> > + } else {
> > + tmu_write(data, TMTMIR_DEFAULT, &data->regv2->tmtmir);
> > + tmu_write(data, TEUMR0_V2, &data->regv2->teumr0);
> > + }
> >
> > /* Disable monitoring */
> > tmu_write(data, TMR_DISABLE, &data->regs->tmr); @@ -190,6 +257,7
> @@
> > static void qoriq_tmu_init_device(struct qoriq_tmu_data *data) static
> > int qoriq_tmu_probe(struct platform_device *pdev) {
> > int ret;
> > + u32 ver;
> > struct qoriq_tmu_data *data;
> > struct device_node *np = pdev->dev.of_node;
> >
> > @@ -214,6 +282,12 @@ static int qoriq_tmu_probe(struct
> platform_device
> > *pdev)
> > goto err_iomap;
> > }
> >
> > + /* version register offset at: 0xbf8 on both v1 and v2 */
> > + ver = tmu_read(data, &data->regs->ipbrr0);
> > + data->ver = (ver >> 8) & 0xff;
> > + if (data->ver == TMU_VER2)
> > + data->regv2 = (void __iomem *)data->regs;
> > +
> > qoriq_tmu_init_device(data); /* TMU initialization */
> >
> > ret = qoriq_tmu_calibration(pdev); /* TMU calibration */
> > --
> > 2.17.1
Hi Rui, Edubezval,
Would you please review this patch?
BR,
Andy
> -----Original Message-----
> From: Andy Tang
> Sent: 2019??8??29?? 16:38
> To: '[email protected]' <[email protected]>; '[email protected]'
> <[email protected]>
> Cc: '[email protected]' <[email protected]>; Leo Li
> <[email protected]>; '[email protected]'
> <[email protected]>; '[email protected]'
> <[email protected]>
> Subject: RE: [PATCH] thermal: qoriq: add thermal monitor unit version 2
> support
>
> Hi Rui, Edubezval,
>
> Almost three monthes passed, I have not got your comments from you.
> Could you please take a look at this patch?
>
> BR,
> Andy
>
> > -----Original Message-----
> > From: Andy Tang
> > Sent: 2019??8??6?? 10:57
> > To: [email protected]; [email protected]
> > Cc: [email protected]; Leo Li <[email protected]>;
> > [email protected]; [email protected]
> > Subject: RE: [PATCH] thermal: qoriq: add thermal monitor unit version
> > 2 support
> >
> > Any comments?
> >
> > BR,
> > Andy
> >
> > > -----Original Message-----
> > > From: Yuantian Tang <[email protected]>
> > > Sent: 2019??6??4?? 10:51
> > > To: [email protected]; [email protected]
> > > Cc: [email protected]; Leo Li <[email protected]>;
> > > [email protected]; [email protected]; Andy Tang
> > > <[email protected]>
> > > Subject: [PATCH] thermal: qoriq: add thermal monitor unit version 2
> > > support
> > >
> > > Thermal Monitor Unit v2 is introduced on new Layscape SoC.
> > > Compared to v1, TMUv2 has a little different register layout and
> > > digital output is fairly linear.
> > >
> > > Signed-off-by: Yuantian Tang <[email protected]>
> > > ---
> > > drivers/thermal/qoriq_thermal.c | 122
> > > +++++++++++++++++++++++++-------
> > > 1 file changed, 98 insertions(+), 24 deletions(-)
> > >
> > > diff --git a/drivers/thermal/qoriq_thermal.c
> > > b/drivers/thermal/qoriq_thermal.c index 3b5f5b3fb1bc..0df6dfddf804
> > > 100644
> > > --- a/drivers/thermal/qoriq_thermal.c
> > > +++ b/drivers/thermal/qoriq_thermal.c
> > > @@ -13,6 +13,15 @@
> > > #include "thermal_core.h"
> > >
> > > #define SITES_MAX 16
> > > +#define TMR_DISABLE 0x0
> > > +#define TMR_ME 0x80000000
> > > +#define TMR_ALPF 0x0c000000
> > > +#define TMR_ALPF_V2 0x03000000
> > > +#define TMTMIR_DEFAULT 0x0000000f
> > > +#define TIER_DISABLE 0x0
> > > +#define TEUMR0_V2 0x51009C00
> > > +#define TMU_VER1 0x1
> > > +#define TMU_VER2 0x2
> > >
> > > /*
> > > * QorIQ TMU Registers
> > > @@ -23,17 +32,55 @@ struct qoriq_tmu_site_regs {
> > > u8 res0[0x8];
> > > };
> > >
> > > -struct qoriq_tmu_regs {
> > > +struct qoriq_tmu_regs_v2 {
> > > + u32 tmr; /* Mode Register */
> > > + u32 tsr; /* Status Register */
> > > + u32 tmsr; /* monitor site register */
> > > + u32 tmtmir; /* Temperature measurement interval Register
> */
> > > + u8 res0[0x10];
> > > + u32 tier; /* Interrupt Enable Register */
> > > + u32 tidr; /* Interrupt Detect Register */
> > > + u8 res1[0x8];
> > > + u32 tiiscr; /* interrupt immediate site capture register */
> > > + u32 tiascr; /* interrupt average site capture register */
> > > + u32 ticscr; /* Interrupt Critical Site Capture Register */
> > > + u32 res2;
> > > + u32 tmhtcr; /* monitor high temperature capture register */
> > > + u32 tmltcr; /* monitor low temperature capture register */
> > > + u32 tmrtrcr; /* monitor rising temperature rate capture register
> */
> > > + u32 tmftrcr; /* monitor falling temperature rate capture register
> */
> > > + u32 tmhtitr; /* High Temperature Immediate Threshold */
> > > + u32 tmhtatr; /* High Temperature Average Threshold */
> > > + u32 tmhtactr; /* High Temperature Average Crit Threshold */
> > > + u32 res3;
> > > + u32 tmltitr; /* monitor low temperature immediate threshold */
> > > + u32 tmltatr; /* monitor low temperature average threshold
> register */
> > > + u32 tmltactr; /* monitor low temperature average critical
> threshold */
> > > + u32 res4;
> > > + u32 tmrtrctr; /* monitor rising temperature rate critical threshold
> */
> > > + u32 tmftrctr; /* monitor falling temperature rate critical
> threshold*/
> > > + u8 res5[0x8];
> > > + u32 ttcfgr; /* Temperature Configuration Register */
> > > + u32 tscfgr; /* Sensor Configuration Register */
> > > + u8 res6[0x78];
> > > + struct qoriq_tmu_site_regs site[SITES_MAX];
> > > + u8 res7[0x9f8];
> > > + u32 ipbrr0; /* IP Block Revision Register 0 */
> > > + u32 ipbrr1; /* IP Block Revision Register 1 */
> > > + u8 res8[0x300];
> > > + u32 teumr0;
> > > + u32 teumr1;
> > > + u32 teumr2;
> > > + u32 res9;
> > > + u32 ttrcr[4]; /* Temperature Range Control Register */
> > > +};
> > > +
> > > +struct qoriq_tmu_regs_v1 {
> > > u32 tmr; /* Mode Register */
> > > -#define TMR_DISABLE 0x0
> > > -#define TMR_ME 0x80000000
> > > -#define TMR_ALPF 0x0c000000
> > > u32 tsr; /* Status Register */
> > > u32 tmtmir; /* Temperature measurement interval Register
> */
> > > -#define TMTMIR_DEFAULT 0x0000000f
> > > u8 res0[0x14];
> > > u32 tier; /* Interrupt Enable Register */
> > > -#define TIER_DISABLE 0x0
> > > u32 tidr; /* Interrupt Detect Register */
> > > u32 tiscr; /* Interrupt Site Capture Register */
> > > u32 ticscr; /* Interrupt Critical Site Capture Register */
> > > @@ -53,10 +100,7 @@ struct qoriq_tmu_regs {
> > > u32 ipbrr0; /* IP Block Revision Register 0 */
> > > u32 ipbrr1; /* IP Block Revision Register 1 */
> > > u8 res6[0x310];
> > > - u32 ttr0cr; /* Temperature Range 0 Control Register */
> > > - u32 ttr1cr; /* Temperature Range 1 Control Register */
> > > - u32 ttr2cr; /* Temperature Range 2 Control Register */
> > > - u32 ttr3cr; /* Temperature Range 3 Control Register */
> > > + u32 ttrcr[4]; /* Temperature Range Control Register */
> > > };
> > >
> > > struct qoriq_tmu_data;
> > > @@ -71,7 +115,9 @@ struct qoriq_sensor { };
> > >
> > > struct qoriq_tmu_data {
> > > - struct qoriq_tmu_regs __iomem *regs;
> > > + int ver;
> > > + struct qoriq_tmu_regs_v1 __iomem *regs;
> > > + struct qoriq_tmu_regs_v2 __iomem *regv2;
> > > bool little_endian;
> > > struct qoriq_sensor *sensor[SITES_MAX];
> > > };
> > > @@ -111,7 +157,7 @@ static const struct thermal_zone_of_device_ops
> > > tmu_tz_ops = { static int qoriq_tmu_register_tmu_zone(struct
> > > platform_device *pdev) {
> > > struct qoriq_tmu_data *qdata = platform_get_drvdata(pdev);
> > > - int id, sites = 0;
> > > + int id, sites = 0, sv2 = 0;
> > >
> > > for (id = 0; id < SITES_MAX; id++) {
> > > qdata->sensor[id] = devm_kzalloc(&pdev->dev, @@ -130,12
> > > +176,24 @@ static int qoriq_tmu_register_tmu_zone(struct
> > > +platform_device
> > > *pdev)
> > > return PTR_ERR(qdata->sensor[id]->tzd);
> > > }
> > >
> > > - sites |= 0x1 << (15 - id);
> > > + if (qdata->ver == TMU_VER1)
> > > + sites |= 0x1 << (15 - id);
> > > + else
> > > + sv2 |= 0x1 << id;
> > > }
> > >
> > > /* Enable monitoring */
> > > - if (sites != 0)
> > > - tmu_write(qdata, sites | TMR_ME | TMR_ALPF,
> > > &qdata->regs->tmr);
> > > + if (qdata->ver == TMU_VER1) {
> > > + if (sites != 0)
> > > + tmu_write(qdata, sites | TMR_ME | TMR_ALPF,
> > > + &qdata->regs->tmr);
> > > + } else {
> > > + if (sv2 != 0) {
> > > + tmu_write(qdata, sv2, &qdata->regv2->tmsr);
> > > + tmu_write(qdata, TMR_ME | TMR_ALPF_V2,
> > > + &qdata->regv2->tmr);
> > > + }
> > > + }
> > >
> > > return 0;
> > > }
> > > @@ -148,16 +206,20 @@ static int qoriq_tmu_calibration(struct
> > > platform_device *pdev)
> > > struct device_node *np = pdev->dev.of_node;
> > > struct qoriq_tmu_data *data = platform_get_drvdata(pdev);
> > >
> > > - if (of_property_read_u32_array(np, "fsl,tmu-range", range, 4)) {
> > > - dev_err(&pdev->dev, "missing calibration range.\n");
> > > - return -ENODEV;
> > > + len = of_property_count_u32_elems(np, "fsl,tmu-range");
> > > + if (len == -ENODATA || len == -EINVAL || len > 4) {
> > > + dev_err(&pdev->dev, "invalid range data.\n");
> > > + return len;
> > > }
> > >
> > > - /* Init temperature range registers */
> > > - tmu_write(data, range[0], &data->regs->ttr0cr);
> > > - tmu_write(data, range[1], &data->regs->ttr1cr);
> > > - tmu_write(data, range[2], &data->regs->ttr2cr);
> > > - tmu_write(data, range[3], &data->regs->ttr3cr);
> > > + val = of_property_read_u32_array(np, "fsl,tmu-range", range, len);
> > > + if (val != 0) {
> > > + dev_err(&pdev->dev, "invalid range data.\n");
> > > + return val;
> > > + }
> > > +
> > > + for (i = 0; i < len; i++)
> > > + tmu_write(data, range[i], &data->regs->ttrcr[i]);
> > >
> > > calibration = of_get_property(np, "fsl,tmu-calibration", &len);
> > > if (calibration == NULL || len % 8) { @@ -181,7 +243,12 @@ static
> > > void qoriq_tmu_init_device(struct qoriq_tmu_data *data)
> > > tmu_write(data, TIER_DISABLE, &data->regs->tier);
> > >
> > > /* Set update_interval */
> > > - tmu_write(data, TMTMIR_DEFAULT, &data->regs->tmtmir);
> > > + if (data->ver == TMU_VER1) {
> > > + tmu_write(data, TMTMIR_DEFAULT, &data->regs->tmtmir);
> > > + } else {
> > > + tmu_write(data, TMTMIR_DEFAULT, &data->regv2->tmtmir);
> > > + tmu_write(data, TEUMR0_V2, &data->regv2->teumr0);
> > > + }
> > >
> > > /* Disable monitoring */
> > > tmu_write(data, TMR_DISABLE, &data->regs->tmr); @@ -190,6
> +257,7
> > @@
> > > static void qoriq_tmu_init_device(struct qoriq_tmu_data *data)
> > > static int qoriq_tmu_probe(struct platform_device *pdev) {
> > > int ret;
> > > + u32 ver;
> > > struct qoriq_tmu_data *data;
> > > struct device_node *np = pdev->dev.of_node;
> > >
> > > @@ -214,6 +282,12 @@ static int qoriq_tmu_probe(struct
> > platform_device
> > > *pdev)
> > > goto err_iomap;
> > > }
> > >
> > > + /* version register offset at: 0xbf8 on both v1 and v2 */
> > > + ver = tmu_read(data, &data->regs->ipbrr0);
> > > + data->ver = (ver >> 8) & 0xff;
> > > + if (data->ver == TMU_VER2)
> > > + data->regv2 = (void __iomem *)data->regs;
> > > +
> > > qoriq_tmu_init_device(data); /* TMU initialization */
> > >
> > > ret = qoriq_tmu_calibration(pdev); /* TMU calibration */
> > > --
> > > 2.17.1
On 23/09/2019 11:24, Andy Tang wrote:
> Hi Rui, Edubezval,
>
> Would you please review this patch?
Eduardo,
can you give an update about the thermal maintenance discussion we had
at LPC2019?
Thanks
-- Daniel
>> -----Original Message-----
>> From: Andy Tang
>> Sent: 2019??8??29?? 16:38
>> To: '[email protected]' <[email protected]>; '[email protected]'
>> <[email protected]>
>> Cc: '[email protected]' <[email protected]>; Leo Li
>> <[email protected]>; '[email protected]'
>> <[email protected]>; '[email protected]'
>> <[email protected]>
>> Subject: RE: [PATCH] thermal: qoriq: add thermal monitor unit version 2
>> support
>>
>> Hi Rui, Edubezval,
>>
>> Almost three monthes passed, I have not got your comments from you.
>> Could you please take a look at this patch?
>>
>> BR,
>> Andy
>>
>>> -----Original Message-----
>>> From: Andy Tang
>>> Sent: 2019??8??6?? 10:57
>>> To: [email protected]; [email protected]
>>> Cc: [email protected]; Leo Li <[email protected]>;
>>> [email protected]; [email protected]
>>> Subject: RE: [PATCH] thermal: qoriq: add thermal monitor unit version
>>> 2 support
>>>
>>> Any comments?
>>>
>>> BR,
>>> Andy
>>>
>>>> -----Original Message-----
>>>> From: Yuantian Tang <[email protected]>
>>>> Sent: 2019??6??4?? 10:51
>>>> To: [email protected]; [email protected]
>>>> Cc: [email protected]; Leo Li <[email protected]>;
>>>> [email protected]; [email protected]; Andy Tang
>>>> <[email protected]>
>>>> Subject: [PATCH] thermal: qoriq: add thermal monitor unit version 2
>>>> support
>>>>
>>>> Thermal Monitor Unit v2 is introduced on new Layscape SoC.
>>>> Compared to v1, TMUv2 has a little different register layout and
>>>> digital output is fairly linear.
>>>>
>>>> Signed-off-by: Yuantian Tang <[email protected]>
>>>> ---
>>>> drivers/thermal/qoriq_thermal.c | 122
>>>> +++++++++++++++++++++++++-------
>>>> 1 file changed, 98 insertions(+), 24 deletions(-)
>>>>
>>>> diff --git a/drivers/thermal/qoriq_thermal.c
>>>> b/drivers/thermal/qoriq_thermal.c index 3b5f5b3fb1bc..0df6dfddf804
>>>> 100644
>>>> --- a/drivers/thermal/qoriq_thermal.c
>>>> +++ b/drivers/thermal/qoriq_thermal.c
>>>> @@ -13,6 +13,15 @@
>>>> #include "thermal_core.h"
>>>>
>>>> #define SITES_MAX 16
>>>> +#define TMR_DISABLE 0x0
>>>> +#define TMR_ME 0x80000000
>>>> +#define TMR_ALPF 0x0c000000
>>>> +#define TMR_ALPF_V2 0x03000000
>>>> +#define TMTMIR_DEFAULT 0x0000000f
>>>> +#define TIER_DISABLE 0x0
>>>> +#define TEUMR0_V2 0x51009C00
>>>> +#define TMU_VER1 0x1
>>>> +#define TMU_VER2 0x2
>>>>
>>>> /*
>>>> * QorIQ TMU Registers
>>>> @@ -23,17 +32,55 @@ struct qoriq_tmu_site_regs {
>>>> u8 res0[0x8];
>>>> };
>>>>
>>>> -struct qoriq_tmu_regs {
>>>> +struct qoriq_tmu_regs_v2 {
>>>> + u32 tmr; /* Mode Register */
>>>> + u32 tsr; /* Status Register */
>>>> + u32 tmsr; /* monitor site register */
>>>> + u32 tmtmir; /* Temperature measurement interval Register
>> */
>>>> + u8 res0[0x10];
>>>> + u32 tier; /* Interrupt Enable Register */
>>>> + u32 tidr; /* Interrupt Detect Register */
>>>> + u8 res1[0x8];
>>>> + u32 tiiscr; /* interrupt immediate site capture register */
>>>> + u32 tiascr; /* interrupt average site capture register */
>>>> + u32 ticscr; /* Interrupt Critical Site Capture Register */
>>>> + u32 res2;
>>>> + u32 tmhtcr; /* monitor high temperature capture register */
>>>> + u32 tmltcr; /* monitor low temperature capture register */
>>>> + u32 tmrtrcr; /* monitor rising temperature rate capture register
>> */
>>>> + u32 tmftrcr; /* monitor falling temperature rate capture register
>> */
>>>> + u32 tmhtitr; /* High Temperature Immediate Threshold */
>>>> + u32 tmhtatr; /* High Temperature Average Threshold */
>>>> + u32 tmhtactr; /* High Temperature Average Crit Threshold */
>>>> + u32 res3;
>>>> + u32 tmltitr; /* monitor low temperature immediate threshold */
>>>> + u32 tmltatr; /* monitor low temperature average threshold
>> register */
>>>> + u32 tmltactr; /* monitor low temperature average critical
>> threshold */
>>>> + u32 res4;
>>>> + u32 tmrtrctr; /* monitor rising temperature rate critical threshold
>> */
>>>> + u32 tmftrctr; /* monitor falling temperature rate critical
>> threshold*/
>>>> + u8 res5[0x8];
>>>> + u32 ttcfgr; /* Temperature Configuration Register */
>>>> + u32 tscfgr; /* Sensor Configuration Register */
>>>> + u8 res6[0x78];
>>>> + struct qoriq_tmu_site_regs site[SITES_MAX];
>>>> + u8 res7[0x9f8];
>>>> + u32 ipbrr0; /* IP Block Revision Register 0 */
>>>> + u32 ipbrr1; /* IP Block Revision Register 1 */
>>>> + u8 res8[0x300];
>>>> + u32 teumr0;
>>>> + u32 teumr1;
>>>> + u32 teumr2;
>>>> + u32 res9;
>>>> + u32 ttrcr[4]; /* Temperature Range Control Register */
>>>> +};
>>>> +
>>>> +struct qoriq_tmu_regs_v1 {
>>>> u32 tmr; /* Mode Register */
>>>> -#define TMR_DISABLE 0x0
>>>> -#define TMR_ME 0x80000000
>>>> -#define TMR_ALPF 0x0c000000
>>>> u32 tsr; /* Status Register */
>>>> u32 tmtmir; /* Temperature measurement interval Register
>> */
>>>> -#define TMTMIR_DEFAULT 0x0000000f
>>>> u8 res0[0x14];
>>>> u32 tier; /* Interrupt Enable Register */
>>>> -#define TIER_DISABLE 0x0
>>>> u32 tidr; /* Interrupt Detect Register */
>>>> u32 tiscr; /* Interrupt Site Capture Register */
>>>> u32 ticscr; /* Interrupt Critical Site Capture Register */
>>>> @@ -53,10 +100,7 @@ struct qoriq_tmu_regs {
>>>> u32 ipbrr0; /* IP Block Revision Register 0 */
>>>> u32 ipbrr1; /* IP Block Revision Register 1 */
>>>> u8 res6[0x310];
>>>> - u32 ttr0cr; /* Temperature Range 0 Control Register */
>>>> - u32 ttr1cr; /* Temperature Range 1 Control Register */
>>>> - u32 ttr2cr; /* Temperature Range 2 Control Register */
>>>> - u32 ttr3cr; /* Temperature Range 3 Control Register */
>>>> + u32 ttrcr[4]; /* Temperature Range Control Register */
>>>> };
>>>>
>>>> struct qoriq_tmu_data;
>>>> @@ -71,7 +115,9 @@ struct qoriq_sensor { };
>>>>
>>>> struct qoriq_tmu_data {
>>>> - struct qoriq_tmu_regs __iomem *regs;
>>>> + int ver;
>>>> + struct qoriq_tmu_regs_v1 __iomem *regs;
>>>> + struct qoriq_tmu_regs_v2 __iomem *regv2;
>>>> bool little_endian;
>>>> struct qoriq_sensor *sensor[SITES_MAX];
>>>> };
>>>> @@ -111,7 +157,7 @@ static const struct thermal_zone_of_device_ops
>>>> tmu_tz_ops = { static int qoriq_tmu_register_tmu_zone(struct
>>>> platform_device *pdev) {
>>>> struct qoriq_tmu_data *qdata = platform_get_drvdata(pdev);
>>>> - int id, sites = 0;
>>>> + int id, sites = 0, sv2 = 0;
>>>>
>>>> for (id = 0; id < SITES_MAX; id++) {
>>>> qdata->sensor[id] = devm_kzalloc(&pdev->dev, @@ -130,12
>>>> +176,24 @@ static int qoriq_tmu_register_tmu_zone(struct
>>>> +platform_device
>>>> *pdev)
>>>> return PTR_ERR(qdata->sensor[id]->tzd);
>>>> }
>>>>
>>>> - sites |= 0x1 << (15 - id);
>>>> + if (qdata->ver == TMU_VER1)
>>>> + sites |= 0x1 << (15 - id);
>>>> + else
>>>> + sv2 |= 0x1 << id;
>>>> }
>>>>
>>>> /* Enable monitoring */
>>>> - if (sites != 0)
>>>> - tmu_write(qdata, sites | TMR_ME | TMR_ALPF,
>>>> &qdata->regs->tmr);
>>>> + if (qdata->ver == TMU_VER1) {
>>>> + if (sites != 0)
>>>> + tmu_write(qdata, sites | TMR_ME | TMR_ALPF,
>>>> + &qdata->regs->tmr);
>>>> + } else {
>>>> + if (sv2 != 0) {
>>>> + tmu_write(qdata, sv2, &qdata->regv2->tmsr);
>>>> + tmu_write(qdata, TMR_ME | TMR_ALPF_V2,
>>>> + &qdata->regv2->tmr);
>>>> + }
>>>> + }
>>>>
>>>> return 0;
>>>> }
>>>> @@ -148,16 +206,20 @@ static int qoriq_tmu_calibration(struct
>>>> platform_device *pdev)
>>>> struct device_node *np = pdev->dev.of_node;
>>>> struct qoriq_tmu_data *data = platform_get_drvdata(pdev);
>>>>
>>>> - if (of_property_read_u32_array(np, "fsl,tmu-range", range, 4)) {
>>>> - dev_err(&pdev->dev, "missing calibration range.\n");
>>>> - return -ENODEV;
>>>> + len = of_property_count_u32_elems(np, "fsl,tmu-range");
>>>> + if (len == -ENODATA || len == -EINVAL || len > 4) {
>>>> + dev_err(&pdev->dev, "invalid range data.\n");
>>>> + return len;
>>>> }
>>>>
>>>> - /* Init temperature range registers */
>>>> - tmu_write(data, range[0], &data->regs->ttr0cr);
>>>> - tmu_write(data, range[1], &data->regs->ttr1cr);
>>>> - tmu_write(data, range[2], &data->regs->ttr2cr);
>>>> - tmu_write(data, range[3], &data->regs->ttr3cr);
>>>> + val = of_property_read_u32_array(np, "fsl,tmu-range", range, len);
>>>> + if (val != 0) {
>>>> + dev_err(&pdev->dev, "invalid range data.\n");
>>>> + return val;
>>>> + }
>>>> +
>>>> + for (i = 0; i < len; i++)
>>>> + tmu_write(data, range[i], &data->regs->ttrcr[i]);
>>>>
>>>> calibration = of_get_property(np, "fsl,tmu-calibration", &len);
>>>> if (calibration == NULL || len % 8) { @@ -181,7 +243,12 @@ static
>>>> void qoriq_tmu_init_device(struct qoriq_tmu_data *data)
>>>> tmu_write(data, TIER_DISABLE, &data->regs->tier);
>>>>
>>>> /* Set update_interval */
>>>> - tmu_write(data, TMTMIR_DEFAULT, &data->regs->tmtmir);
>>>> + if (data->ver == TMU_VER1) {
>>>> + tmu_write(data, TMTMIR_DEFAULT, &data->regs->tmtmir);
>>>> + } else {
>>>> + tmu_write(data, TMTMIR_DEFAULT, &data->regv2->tmtmir);
>>>> + tmu_write(data, TEUMR0_V2, &data->regv2->teumr0);
>>>> + }
>>>>
>>>> /* Disable monitoring */
>>>> tmu_write(data, TMR_DISABLE, &data->regs->tmr); @@ -190,6
>> +257,7
>>> @@
>>>> static void qoriq_tmu_init_device(struct qoriq_tmu_data *data)
>>>> static int qoriq_tmu_probe(struct platform_device *pdev) {
>>>> int ret;
>>>> + u32 ver;
>>>> struct qoriq_tmu_data *data;
>>>> struct device_node *np = pdev->dev.of_node;
>>>>
>>>> @@ -214,6 +282,12 @@ static int qoriq_tmu_probe(struct
>>> platform_device
>>>> *pdev)
>>>> goto err_iomap;
>>>> }
>>>>
>>>> + /* version register offset at: 0xbf8 on both v1 and v2 */
>>>> + ver = tmu_read(data, &data->regs->ipbrr0);
>>>> + data->ver = (ver >> 8) & 0xff;
>>>> + if (data->ver == TMU_VER2)
>>>> + data->regv2 = (void __iomem *)data->regs;
>>>> +
>>>> qoriq_tmu_init_device(data); /* TMU initialization */
>>>>
>>>> ret = qoriq_tmu_calibration(pdev); /* TMU calibration */
>>>> --
>>>> 2.17.1
>
--
<http://www.linaro.org/> Linaro.org ?? Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
On Mon, 2019-09-23 at 09:24 +0000, Andy Tang wrote:
> Hi Rui, Edubezval,
>
> Would you please review this patch?
>
CC Anson Huang.
I'd prefer all the qoriq thermal patches go through his review first.
thanks,
rui
> BR,
> Andy
>
> > -----Original Message-----
> > From: Andy Tang
> > Sent: 2019年8月29日 16:38
> > To: '[email protected]' <[email protected]>;
> > '[email protected]'
> > <[email protected]>
> > Cc: '[email protected]' <[email protected]>; Leo Li
> > <[email protected]>; '[email protected]'
> > <[email protected]>; '[email protected]'
> > <[email protected]>
> > Subject: RE: [PATCH] thermal: qoriq: add thermal monitor unit
> > version 2
> > support
> >
> > Hi Rui, Edubezval,
> >
> > Almost three monthes passed, I have not got your comments from you.
> > Could you please take a look at this patch?
> >
> > BR,
> > Andy
> >
> > > -----Original Message-----
> > > From: Andy Tang
> > > Sent: 2019年8月6日 10:57
> > > To: [email protected]; [email protected]
> > > Cc: [email protected]; Leo Li <[email protected]>;
> > > [email protected]; [email protected]
> > > Subject: RE: [PATCH] thermal: qoriq: add thermal monitor unit
> > > version
> > > 2 support
> > >
> > > Any comments?
> > >
> > > BR,
> > > Andy
> > >
> > > > -----Original Message-----
> > > > From: Yuantian Tang <[email protected]>
> > > > Sent: 2019年6月4日 10:51
> > > > To: [email protected]; [email protected]
> > > > Cc: [email protected]; Leo Li <[email protected]>;
> > > > [email protected]; [email protected]; Andy
> > > > Tang
> > > > <[email protected]>
> > > > Subject: [PATCH] thermal: qoriq: add thermal monitor unit
> > > > version 2
> > > > support
> > > >
> > > > Thermal Monitor Unit v2 is introduced on new Layscape SoC.
> > > > Compared to v1, TMUv2 has a little different register layout
> > > > and
> > > > digital output is fairly linear.
> > > >
> > > > Signed-off-by: Yuantian Tang <[email protected]>
> > > > ---
> > > > drivers/thermal/qoriq_thermal.c | 122
> > > > +++++++++++++++++++++++++-------
> > > > 1 file changed, 98 insertions(+), 24 deletions(-)
> > > >
> > > > diff --git a/drivers/thermal/qoriq_thermal.c
> > > > b/drivers/thermal/qoriq_thermal.c index
> > > > 3b5f5b3fb1bc..0df6dfddf804
> > > > 100644
> > > > --- a/drivers/thermal/qoriq_thermal.c
> > > > +++ b/drivers/thermal/qoriq_thermal.c
> > > > @@ -13,6 +13,15 @@
> > > > #include "thermal_core.h"
> > > >
> > > > #define SITES_MAX 16
> > > > +#define TMR_DISABLE 0x0
> > > > +#define TMR_ME 0x80000000
> > > > +#define TMR_ALPF 0x0c000000
> > > > +#define TMR_ALPF_V2 0x03000000
> > > > +#define TMTMIR_DEFAULT 0x0000000f
> > > > +#define TIER_DISABLE 0x0
> > > > +#define TEUMR0_V2 0x51009C00
> > > > +#define TMU_VER1 0x1
> > > > +#define TMU_VER2 0x2
> > > >
> > > > /*
> > > > * QorIQ TMU Registers
> > > > @@ -23,17 +32,55 @@ struct qoriq_tmu_site_regs {
> > > > u8 res0[0x8];
> > > > };
> > > >
> > > > -struct qoriq_tmu_regs {
> > > > +struct qoriq_tmu_regs_v2 {
> > > > + u32 tmr; /* Mode Register */
> > > > + u32 tsr; /* Status Register */
> > > > + u32 tmsr; /* monitor site register */
> > > > + u32 tmtmir; /* Temperature measurement
> > > > interval Register
> >
> > */
> > > > + u8 res0[0x10];
> > > > + u32 tier; /* Interrupt Enable Register */
> > > > + u32 tidr; /* Interrupt Detect Register */
> > > > + u8 res1[0x8];
> > > > + u32 tiiscr; /* interrupt immediate site
> > > > capture register */
> > > > + u32 tiascr; /* interrupt average site
> > > > capture register */
> > > > + u32 ticscr; /* Interrupt Critical Site
> > > > Capture Register */
> > > > + u32 res2;
> > > > + u32 tmhtcr; /* monitor high temperature
> > > > capture register */
> > > > + u32 tmltcr; /* monitor low temperature
> > > > capture register */
> > > > + u32 tmrtrcr; /* monitor rising temperature rate
> > > > capture register
> >
> > */
> > > > + u32 tmftrcr; /* monitor falling temperature rate
> > > > capture register
> >
> > */
> > > > + u32 tmhtitr; /* High Temperature Immediate Threshold
> > > > */
> > > > + u32 tmhtatr; /* High Temperature Average Threshold
> > > > */
> > > > + u32 tmhtactr; /* High Temperature Average Crit
> > > > Threshold */
> > > > + u32 res3;
> > > > + u32 tmltitr; /* monitor low temperature immediate
> > > > threshold */
> > > > + u32 tmltatr; /* monitor low temperature average
> > > > threshold
> >
> > register */
> > > > + u32 tmltactr; /* monitor low temperature average
> > > > critical
> >
> > threshold */
> > > > + u32 res4;
> > > > + u32 tmrtrctr; /* monitor rising temperature rate
> > > > critical threshold
> >
> > */
> > > > + u32 tmftrctr; /* monitor falling temperature rate
> > > > critical
> >
> > threshold*/
> > > > + u8 res5[0x8];
> > > > + u32 ttcfgr; /* Temperature Configuration Register
> > > > */
> > > > + u32 tscfgr; /* Sensor Configuration Register */
> > > > + u8 res6[0x78];
> > > > + struct qoriq_tmu_site_regs site[SITES_MAX];
> > > > + u8 res7[0x9f8];
> > > > + u32 ipbrr0; /* IP Block Revision Register 0
> > > > */
> > > > + u32 ipbrr1; /* IP Block Revision Register 1
> > > > */
> > > > + u8 res8[0x300];
> > > > + u32 teumr0;
> > > > + u32 teumr1;
> > > > + u32 teumr2;
> > > > + u32 res9;
> > > > + u32 ttrcr[4]; /* Temperature Range Control Register
> > > > */
> > > > +};
> > > > +
> > > > +struct qoriq_tmu_regs_v1 {
> > > > u32 tmr; /* Mode Register */
> > > > -#define TMR_DISABLE 0x0
> > > > -#define TMR_ME 0x80000000
> > > > -#define TMR_ALPF 0x0c000000
> > > > u32 tsr; /* Status Register */
> > > > u32 tmtmir; /* Temperature measurement
> > > > interval Register
> >
> > */
> > > > -#define TMTMIR_DEFAULT 0x0000000f
> > > > u8 res0[0x14];
> > > > u32 tier; /* Interrupt Enable Register */
> > > > -#define TIER_DISABLE 0x0
> > > > u32 tidr; /* Interrupt Detect Register */
> > > > u32 tiscr; /* Interrupt Site Capture
> > > > Register */
> > > > u32 ticscr; /* Interrupt Critical Site
> > > > Capture Register */
> > > > @@ -53,10 +100,7 @@ struct qoriq_tmu_regs {
> > > > u32 ipbrr0; /* IP Block Revision Register 0
> > > > */
> > > > u32 ipbrr1; /* IP Block Revision Register 1
> > > > */
> > > > u8 res6[0x310];
> > > > - u32 ttr0cr; /* Temperature Range 0 Control
> > > > Register */
> > > > - u32 ttr1cr; /* Temperature Range 1 Control
> > > > Register */
> > > > - u32 ttr2cr; /* Temperature Range 2 Control
> > > > Register */
> > > > - u32 ttr3cr; /* Temperature Range 3 Control
> > > > Register */
> > > > + u32 ttrcr[4]; /* Temperature Range Control
> > > > Register */
> > > > };
> > > >
> > > > struct qoriq_tmu_data;
> > > > @@ -71,7 +115,9 @@ struct qoriq_sensor { };
> > > >
> > > > struct qoriq_tmu_data {
> > > > - struct qoriq_tmu_regs __iomem *regs;
> > > > + int ver;
> > > > + struct qoriq_tmu_regs_v1 __iomem *regs;
> > > > + struct qoriq_tmu_regs_v2 __iomem *regv2;
> > > > bool little_endian;
> > > > struct qoriq_sensor *sensor[SITES_MAX];
> > > > };
> > > > @@ -111,7 +157,7 @@ static const struct
> > > > thermal_zone_of_device_ops
> > > > tmu_tz_ops = { static int qoriq_tmu_register_tmu_zone(struct
> > > > platform_device *pdev) {
> > > > struct qoriq_tmu_data *qdata =
> > > > platform_get_drvdata(pdev);
> > > > - int id, sites = 0;
> > > > + int id, sites = 0, sv2 = 0;
> > > >
> > > > for (id = 0; id < SITES_MAX; id++) {
> > > > qdata->sensor[id] = devm_kzalloc(&pdev->dev, @@
> > > > -130,12
> > > > +176,24 @@ static int qoriq_tmu_register_tmu_zone(struct
> > > > +platform_device
> > > > *pdev)
> > > > return PTR_ERR(qdata-
> > > > >sensor[id]->tzd);
> > > > }
> > > >
> > > > - sites |= 0x1 << (15 - id);
> > > > + if (qdata->ver == TMU_VER1)
> > > > + sites |= 0x1 << (15 - id);
> > > > + else
> > > > + sv2 |= 0x1 << id;
> > > > }
> > > >
> > > > /* Enable monitoring */
> > > > - if (sites != 0)
> > > > - tmu_write(qdata, sites | TMR_ME | TMR_ALPF,
> > > > &qdata->regs->tmr);
> > > > + if (qdata->ver == TMU_VER1) {
> > > > + if (sites != 0)
> > > > + tmu_write(qdata, sites | TMR_ME |
> > > > TMR_ALPF,
> > > > + &qdata->regs->tmr);
> > > > + } else {
> > > > + if (sv2 != 0) {
> > > > + tmu_write(qdata, sv2, &qdata->regv2-
> > > > >tmsr);
> > > > + tmu_write(qdata, TMR_ME | TMR_ALPF_V2,
> > > > + &qdata->regv2->tmr);
> > > > + }
> > > > + }
> > > >
> > > > return 0;
> > > > }
> > > > @@ -148,16 +206,20 @@ static int qoriq_tmu_calibration(struct
> > > > platform_device *pdev)
> > > > struct device_node *np = pdev->dev.of_node;
> > > > struct qoriq_tmu_data *data =
> > > > platform_get_drvdata(pdev);
> > > >
> > > > - if (of_property_read_u32_array(np, "fsl,tmu-range",
> > > > range, 4)) {
> > > > - dev_err(&pdev->dev, "missing calibration
> > > > range.\n");
> > > > - return -ENODEV;
> > > > + len = of_property_count_u32_elems(np, "fsl,tmu-range");
> > > > + if (len == -ENODATA || len == -EINVAL || len > 4) {
> > > > + dev_err(&pdev->dev, "invalid range data.\n");
> > > > + return len;
> > > > }
> > > >
> > > > - /* Init temperature range registers */
> > > > - tmu_write(data, range[0], &data->regs->ttr0cr);
> > > > - tmu_write(data, range[1], &data->regs->ttr1cr);
> > > > - tmu_write(data, range[2], &data->regs->ttr2cr);
> > > > - tmu_write(data, range[3], &data->regs->ttr3cr);
> > > > + val = of_property_read_u32_array(np, "fsl,tmu-range",
> > > > range, len);
> > > > + if (val != 0) {
> > > > + dev_err(&pdev->dev, "invalid range data.\n");
> > > > + return val;
> > > > + }
> > > > +
> > > > + for (i = 0; i < len; i++)
> > > > + tmu_write(data, range[i], &data->regs-
> > > > >ttrcr[i]);
> > > >
> > > > calibration = of_get_property(np, "fsl,tmu-
> > > > calibration", &len);
> > > > if (calibration == NULL || len % 8) { @@ -181,7 +243,12
> > > > @@ static
> > > > void qoriq_tmu_init_device(struct qoriq_tmu_data *data)
> > > > tmu_write(data, TIER_DISABLE, &data->regs->tier);
> > > >
> > > > /* Set update_interval */
> > > > - tmu_write(data, TMTMIR_DEFAULT, &data->regs->tmtmir);
> > > > + if (data->ver == TMU_VER1) {
> > > > + tmu_write(data, TMTMIR_DEFAULT, &data->regs-
> > > > >tmtmir);
> > > > + } else {
> > > > + tmu_write(data, TMTMIR_DEFAULT, &data->regv2-
> > > > >tmtmir);
> > > > + tmu_write(data, TEUMR0_V2, &data->regv2-
> > > > >teumr0);
> > > > + }
> > > >
> > > > /* Disable monitoring */
> > > > tmu_write(data, TMR_DISABLE, &data->regs->tmr); @@
> > > > -190,6
> >
> > +257,7
> > > @@
> > > > static void qoriq_tmu_init_device(struct qoriq_tmu_data *data)
> > > > static int qoriq_tmu_probe(struct platform_device *pdev) {
> > > > int ret;
> > > > + u32 ver;
> > > > struct qoriq_tmu_data *data;
> > > > struct device_node *np = pdev->dev.of_node;
> > > >
> > > > @@ -214,6 +282,12 @@ static int qoriq_tmu_probe(struct
> > >
> > > platform_device
> > > > *pdev)
> > > > goto err_iomap;
> > > > }
> > > >
> > > > + /* version register offset at: 0xbf8 on both v1 and v2
> > > > */
> > > > + ver = tmu_read(data, &data->regs->ipbrr0);
> > > > + data->ver = (ver >> 8) & 0xff;
> > > > + if (data->ver == TMU_VER2)
> > > > + data->regv2 = (void __iomem *)data->regs;
> > > > +
> > > > qoriq_tmu_init_device(data); /* TMU initialization
> > > > */
> > > >
> > > > ret = qoriq_tmu_calibration(pdev); /* TMU
> > > > calibration */
> > > > --
> > > > 2.17.1
>
>
Hi Anson,
Thanks for your review. Please see my reply inline.
> -----Original Message-----
> From: Anson Huang
> Sent: 2019年9月24日 9:17
> To: Zhang Rui <[email protected]>; Andy Tang <[email protected]>;
> [email protected]
> Cc: [email protected]; Leo Li <[email protected]>;
> [email protected]; [email protected]
> Subject: RE: [PATCH] thermal: qoriq: add thermal monitor unit version 2
> support
>
> Hi, Andy
>
>
> > On Mon, 2019-09-23 at 09:24 +0000, Andy Tang wrote:
> > > Hi Rui, Edubezval,
> > >
> > > Would you please review this patch?
> > >
> > CC Anson Huang.
> > I'd prefer all the qoriq thermal patches go through his review first.
> >
> > thanks,
> > rui
> >
> > > BR,
> > > Andy
> > >
> > > > -----Original Message-----
> > > > From: Andy Tang
> > > > Sent: 2019年8月29日 16:38
> > > > To: '[email protected]' <[email protected]>;
> > > > '[email protected]'
> > > > <[email protected]>
> > > > Cc: '[email protected]' <[email protected]>; Leo
> > > > Li <[email protected]>; '[email protected]'
> > > > <[email protected]>; '[email protected]'
> > > > <[email protected]>
> > > > Subject: RE: [PATCH] thermal: qoriq: add thermal monitor unit
> > > > version 2 support
> > > >
> > > > Hi Rui, Edubezval,
> > > >
> > > > Almost three monthes passed, I have not got your comments from you.
> > > > Could you please take a look at this patch?
> > > >
> > > > BR,
> > > > Andy
> > > >
> > > > > -----Original Message-----
> > > > > From: Andy Tang
> > > > > Sent: 2019年8月6日 10:57
> > > > > To: [email protected]; [email protected]
> > > > > Cc: [email protected]; Leo Li <[email protected]>;
> > > > > [email protected]; [email protected]
> > > > > Subject: RE: [PATCH] thermal: qoriq: add thermal monitor unit
> > > > > version
> > > > > 2 support
> > > > >
> > > > > Any comments?
> > > > >
> > > > > BR,
> > > > > Andy
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Yuantian Tang <[email protected]>
> > > > > > Sent: 2019年6月4日 10:51
> > > > > > To: [email protected]; [email protected]
> > > > > > Cc: [email protected]; Leo Li <[email protected]>;
> > > > > > [email protected]; [email protected]; Andy
> > > > > > Tang <[email protected]>
> > > > > > Subject: [PATCH] thermal: qoriq: add thermal monitor unit
> > > > > > version 2 support
> > > > > >
> > > > > > Thermal Monitor Unit v2 is introduced on new Layscape SoC.
> > > > > > Compared to v1, TMUv2 has a little different register layout
> > > > > > and digital output is fairly linear.
> > > > > >
> > > > > > Signed-off-by: Yuantian Tang <[email protected]>
> > > > > > ---
> > > > > > drivers/thermal/qoriq_thermal.c | 122
> > > > > > +++++++++++++++++++++++++-------
> > > > > > 1 file changed, 98 insertions(+), 24 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/thermal/qoriq_thermal.c
> > > > > > b/drivers/thermal/qoriq_thermal.c index
> > > > > > 3b5f5b3fb1bc..0df6dfddf804
> > > > > > 100644
> > > > > > --- a/drivers/thermal/qoriq_thermal.c
> > > > > > +++ b/drivers/thermal/qoriq_thermal.c
> > > > > > @@ -13,6 +13,15 @@
> > > > > > #include "thermal_core.h"
> > > > > >
> > > > > > #define SITES_MAX 16
> > > > > > +#define TMR_DISABLE 0x0
> > > > > > +#define TMR_ME 0x80000000
> > > > > > +#define TMR_ALPF 0x0c000000
> > > > > > +#define TMR_ALPF_V2 0x03000000
> > > > > > +#define TMTMIR_DEFAULT 0x0000000f
> > > > > > +#define TIER_DISABLE 0x0
> > > > > > +#define TEUMR0_V2 0x51009C00
>
> Better to use either lower case or capital letter for all macro definitions,
> some are lower case and some are capital letter look like NOT aligned.
I always use capital letter to define a macro.
Did I use lower letter somewhere?
>
> > > > > > +#define TMU_VER1 0x1
> > > > > > +#define TMU_VER2 0x2
> > > > > >
> > > > > > /*
> > > > > > * QorIQ TMU Registers
> > > > > > @@ -23,17 +32,55 @@ struct qoriq_tmu_site_regs {
> > > > > > u8 res0[0x8];
> > > > > > };
> > > > > >
> > > > > > -struct qoriq_tmu_regs {
> > > > > > +struct qoriq_tmu_regs_v2 {
> > > > > > + u32 tmr; /* Mode Register */
> > > > > > + u32 tsr; /* Status Register */
> > > > > > + u32 tmsr; /* monitor site register */
> > > > > > + u32 tmtmir; /* Temperature measurement
> > > > > > interval Register
> > > >
> > > > */
> > > > > > + u8 res0[0x10];
> > > > > > + u32 tier; /* Interrupt Enable Register */
> > > > > > + u32 tidr; /* Interrupt Detect Register */
> > > > > > + u8 res1[0x8];
> > > > > > + u32 tiiscr; /* interrupt immediate site
> > > > > > capture register */
> > > > > > + u32 tiascr; /* interrupt average site
> > > > > > capture register */
> > > > > > + u32 ticscr; /* Interrupt Critical Site
> > > > > > Capture Register */
> > > > > > + u32 res2;
> > > > > > + u32 tmhtcr; /* monitor high temperature
> > > > > > capture register */
> > > > > > + u32 tmltcr; /* monitor low temperature
> > > > > > capture register */
> > > > > > + u32 tmrtrcr; /* monitor rising temperature rate
> > > > > > capture register
> > > >
> > > > */
> > > > > > + u32 tmftrcr; /* monitor falling temperature rate
> > > > > > capture register
> > > >
> > > > */
> > > > > > + u32 tmhtitr; /* High Temperature Immediate Threshold
> > > > > > */
> > > > > > + u32 tmhtatr; /* High Temperature Average Threshold
> > > > > > */
> > > > > > + u32 tmhtactr; /* High Temperature Average Crit
> > > > > > Threshold */
> > > > > > + u32 res3;
> > > > > > + u32 tmltitr; /* monitor low temperature immediate
> > > > > > threshold */
> > > > > > + u32 tmltatr; /* monitor low temperature average
> > > > > > threshold
> > > >
> > > > register */
> > > > > > + u32 tmltactr; /* monitor low temperature average
> > > > > > critical
> > > >
> > > > threshold */
> > > > > > + u32 res4;
> > > > > > + u32 tmrtrctr; /* monitor rising temperature rate
> > > > > > critical threshold
> > > >
> > > > */
> > > > > > + u32 tmftrctr; /* monitor falling temperature rate
> > > > > > critical
> > > >
> > > > threshold*/
> > > > > > + u8 res5[0x8];
> > > > > > + u32 ttcfgr; /* Temperature Configuration Register
> > > > > > */
> > > > > > + u32 tscfgr; /* Sensor Configuration Register */
> > > > > > + u8 res6[0x78];
> > > > > > + struct qoriq_tmu_site_regs site[SITES_MAX];
> > > > > > + u8 res7[0x9f8];
> > > > > > + u32 ipbrr0; /* IP Block Revision Register 0
> > > > > > */
> > > > > > + u32 ipbrr1; /* IP Block Revision Register 1
> > > > > > */
> > > > > > + u8 res8[0x300];
> > > > > > + u32 teumr0;
> > > > > > + u32 teumr1;
> > > > > > + u32 teumr2;
> > > > > > + u32 res9;
> > > > > > + u32 ttrcr[4]; /* Temperature Range Control Register
> > > > > > */
> > > > > > +};
> > > > > > +
> > > > > > +struct qoriq_tmu_regs_v1 {
> > > > > > u32 tmr; /* Mode Register */
> > > > > > -#define TMR_DISABLE 0x0
> > > > > > -#define TMR_ME 0x80000000
> > > > > > -#define TMR_ALPF 0x0c000000
> > > > > > u32 tsr; /* Status Register */
> > > > > > u32 tmtmir; /* Temperature measurement
> > > > > > interval Register
> > > >
> > > > */
> > > > > > -#define TMTMIR_DEFAULT 0x0000000f
> > > > > > u8 res0[0x14];
> > > > > > u32 tier; /* Interrupt Enable Register */
> > > > > > -#define TIER_DISABLE 0x0
> > > > > > u32 tidr; /* Interrupt Detect Register */
> > > > > > u32 tiscr; /* Interrupt Site Capture
> > > > > > Register */
> > > > > > u32 ticscr; /* Interrupt Critical Site
> > > > > > Capture Register */
> > > > > > @@ -53,10 +100,7 @@ struct qoriq_tmu_regs {
> > > > > > u32 ipbrr0; /* IP Block Revision Register 0
> > > > > > */
> > > > > > u32 ipbrr1; /* IP Block Revision Register 1
> > > > > > */
> > > > > > u8 res6[0x310];
> > > > > > - u32 ttr0cr; /* Temperature Range 0 Control
> > > > > > Register */
> > > > > > - u32 ttr1cr; /* Temperature Range 1 Control
> > > > > > Register */
> > > > > > - u32 ttr2cr; /* Temperature Range 2 Control
> > > > > > Register */
> > > > > > - u32 ttr3cr; /* Temperature Range 3 Control
> > > > > > Register */
> > > > > > + u32 ttrcr[4]; /* Temperature Range Control
> > > > > > Register */
> > > > > > };
> > > > > >
> > > > > > struct qoriq_tmu_data;
> > > > > > @@ -71,7 +115,9 @@ struct qoriq_sensor { };
> > > > > >
> > > > > > struct qoriq_tmu_data {
> > > > > > - struct qoriq_tmu_regs __iomem *regs;
> > > > > > + int ver;
> > > > > > + struct qoriq_tmu_regs_v1 __iomem *regs;
> > > > > > + struct qoriq_tmu_regs_v2 __iomem *regv2;
>
> regv2 -> regs_v2 ?
OK
>
> > > > > > bool little_endian;
> > > > > > struct qoriq_sensor *sensor[SITES_MAX];
> > > > > > };
> > > > > > @@ -111,7 +157,7 @@ static const struct
> > > > > > thermal_zone_of_device_ops tmu_tz_ops = { static int
> > > > > > qoriq_tmu_register_tmu_zone(struct
> > > > > > platform_device *pdev) {
> > > > > > struct qoriq_tmu_data *qdata = platform_get_drvdata(pdev);
> > > > > > - int id, sites = 0;
> > > > > > + int id, sites = 0, sv2 = 0;
>
> I did NOT see 'sites' and 'sv2' used together in below code, why NOT just use
> 'sites' for both V1 and V2?
Yeah, using one variable is better.
>
> > > > > >
> > > > > > for (id = 0; id < SITES_MAX; id++) {
> > > > > > qdata->sensor[id] = devm_kzalloc(&pdev->dev, @@
> > > > > > -130,12
> > > > > > +176,24 @@ static int qoriq_tmu_register_tmu_zone(struct
> > > > > > +platform_device
> > > > > > *pdev)
> > > > > > return PTR_ERR(qdata-
> > > > > > >sensor[id]->tzd);
> > > > > > }
> > > > > >
> > > > > > - sites |= 0x1 << (15 - id);
> > > > > > + if (qdata->ver == TMU_VER1)
> > > > > > + sites |= 0x1 << (15 - id);
> > > > > > + else
> > > > > > + sv2 |= 0x1 << id;
> > > > > > }
> > > > > >
> > > > > > /* Enable monitoring */
> > > > > > - if (sites != 0)
> > > > > > - tmu_write(qdata, sites | TMR_ME | TMR_ALPF,
> > > > > > &qdata->regs->tmr);
> > > > > > + if (qdata->ver == TMU_VER1) {
> > > > > > + if (sites != 0)
> > > > > > + tmu_write(qdata, sites | TMR_ME |
> > > > > > TMR_ALPF,
> > > > > > + &qdata->regs->tmr);
> > > > > > + } else {
> > > > > > + if (sv2 != 0) {
> > > > > > + tmu_write(qdata, sv2, &qdata->regv2-
> > > > > > >tmsr);
> > > > > > + tmu_write(qdata, TMR_ME | TMR_ALPF_V2,
> > > > > > + &qdata->regv2->tmr);
> > > > > > + }
> > > > > > + }
> > > > > >
> > > > > > return 0;
> > > > > > }
> > > > > > @@ -148,16 +206,20 @@ static int qoriq_tmu_calibration(struct
> > > > > > platform_device *pdev)
> > > > > > struct device_node *np = pdev->dev.of_node;
> > > > > > struct qoriq_tmu_data *data = platform_get_drvdata(pdev);
> > > > > >
> > > > > > - if (of_property_read_u32_array(np, "fsl,tmu-range",
> > > > > > range, 4)) {
> > > > > > - dev_err(&pdev->dev, "missing calibration
> > > > > > range.\n");
> > > > > > - return -ENODEV;
> > > > > > + len = of_property_count_u32_elems(np, "fsl,tmu-range");
> > > > > > + if (len == -ENODATA || len == -EINVAL || len > 4) {
>
> Can we just check the "len < 0 || len > 4" ?
OK.
>
> > > > > > + dev_err(&pdev->dev, "invalid range data.\n");
> > > > > > + return len;
> > > > > > }
> > > > > >
> > > > > > - /* Init temperature range registers */
> > > > > > - tmu_write(data, range[0], &data->regs->ttr0cr);
> > > > > > - tmu_write(data, range[1], &data->regs->ttr1cr);
> > > > > > - tmu_write(data, range[2], &data->regs->ttr2cr);
> > > > > > - tmu_write(data, range[3], &data->regs->ttr3cr);
> > > > > > + val = of_property_read_u32_array(np, "fsl,tmu-range",
> > > > > > range, len);
> > > > > > + if (val != 0) {
> > > > > > + dev_err(&pdev->dev, "invalid range data.\n");
>
> The error log is same as failure of getting range data count, so would it be
> better to change the log to "failed to read range data" here?
Yeah, better.
>
> > > > > > + return val;
> > > > > > + }
> > > > > > +
> > > > > > + for (i = 0; i < len; i++)
> > > > > > + tmu_write(data, range[i], &data->regs-
> > > > > > >ttrcr[i]);
> > > > > >
> > > > > > calibration = of_get_property(np, "fsl,tmu- calibration",
> > > > > > &len);
> > > > > > if (calibration == NULL || len % 8) { @@ -181,7 +243,12 @@
> > > > > > static void qoriq_tmu_init_device(struct qoriq_tmu_data *data)
> > > > > > tmu_write(data, TIER_DISABLE, &data->regs->tier);
> > > > > >
> > > > > > /* Set update_interval */
> > > > > > - tmu_write(data, TMTMIR_DEFAULT, &data->regs->tmtmir);
> > > > > > + if (data->ver == TMU_VER1) {
> > > > > > + tmu_write(data, TMTMIR_DEFAULT, &data->regs-
> > > > > > >tmtmir);
> > > > > > + } else {
> > > > > > + tmu_write(data, TMTMIR_DEFAULT, &data->regv2-
> > > > > > >tmtmir);
> > > > > > + tmu_write(data, TEUMR0_V2, &data->regv2-
> > > > > > >teumr0);
> > > > > > + }
> > > > > >
> > > > > > /* Disable monitoring */
> > > > > > tmu_write(data, TMR_DISABLE, &data->regs->tmr); @@
> > > > > > -190,6
> > > >
> > > > +257,7
> > > > > @@
> > > > > > static void qoriq_tmu_init_device(struct qoriq_tmu_data *data)
> > > > > > static int qoriq_tmu_probe(struct platform_device *pdev) {
> > > > > > int ret;
> > > > > > + u32 ver;
> > > > > > struct qoriq_tmu_data *data;
> > > > > > struct device_node *np = pdev->dev.of_node;
> > > > > >
> > > > > > @@ -214,6 +282,12 @@ static int qoriq_tmu_probe(struct
> > > > >
> > > > > platform_device
> > > > > > *pdev)
> > > > > > goto err_iomap;
> > > > > > }
> > > > > >
> > > > > > + /* version register offset at: 0xbf8 on both v1 and v2
> > > > > > */
> > > > > > + ver = tmu_read(data, &data->regs->ipbrr0);
> > > > > > + data->ver = (ver >> 8) & 0xff;
> > > > > > + if (data->ver == TMU_VER2)
> > > > > > + data->regv2 = (void __iomem *)data->regs;
>
> So the V2 has same register base address as V1? If yes, then I think no need
> to check the version here, just assign V2's reg to equal to V1's should be OK?
For any one specific soc, TMU must be v1 or v2. So basically only using regv1 or regv2 is enough.
Here both regv1 and regv2 are used and are assigned to the same base address.
I use regv1 to access the common registers for both regv1 and regv2.
I use regv2 to access the registers that are specific to regv2.
In this way, I can reuse the current register read/write code and only update the code for regv2 access.
BR,
Andy
>
> Anson
>
> > > > > > +
> > > > > > qoriq_tmu_init_device(data); /* TMU initialization
> > > > > > */
> > > > > >
> > > > > > ret = qoriq_tmu_calibration(pdev); /* TMU
> > > > > > calibration */
> > > > > > --
> > > > > > 2.17.1
> > >
> > >
Hi, Andy
> Subject: RE: [PATCH] thermal: qoriq: add thermal monitor unit version 2
> support
>
> Hi Anson,
>
> Thanks for your review. Please see my reply inline.
>
> > -----Original Message-----
> > From: Anson Huang
> > Sent: 2019年9月24日 9:17
> > To: Zhang Rui <[email protected]>; Andy Tang <[email protected]>;
> > [email protected]
> > Cc: [email protected]; Leo Li <[email protected]>;
> > [email protected]; [email protected]
> > Subject: RE: [PATCH] thermal: qoriq: add thermal monitor unit version
> > 2 support
> >
> > Hi, Andy
> >
> >
> > > On Mon, 2019-09-23 at 09:24 +0000, Andy Tang wrote:
> > > > Hi Rui, Edubezval,
> > > >
> > > > Would you please review this patch?
> > > >
> > > CC Anson Huang.
> > > I'd prefer all the qoriq thermal patches go through his review first.
> > >
> > > thanks,
> > > rui
> > >
> > > > BR,
> > > > Andy
> > > >
> > > > > -----Original Message-----
> > > > > From: Andy Tang
> > > > > Sent: 2019年8月29日 16:38
> > > > > To: '[email protected]' <[email protected]>;
> > > > > '[email protected]'
> > > > > <[email protected]>
> > > > > Cc: '[email protected]' <[email protected]>; Leo
> > > > > Li <[email protected]>; '[email protected]'
> > > > > <[email protected]>; '[email protected]'
> > > > > <[email protected]>
> > > > > Subject: RE: [PATCH] thermal: qoriq: add thermal monitor unit
> > > > > version 2 support
> > > > >
> > > > > Hi Rui, Edubezval,
> > > > >
> > > > > Almost three monthes passed, I have not got your comments from
> you.
> > > > > Could you please take a look at this patch?
> > > > >
> > > > > BR,
> > > > > Andy
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Andy Tang
> > > > > > Sent: 2019年8月6日 10:57
> > > > > > To: [email protected]; [email protected]
> > > > > > Cc: [email protected]; Leo Li <[email protected]>;
> > > > > > [email protected]; [email protected]
> > > > > > Subject: RE: [PATCH] thermal: qoriq: add thermal monitor unit
> > > > > > version
> > > > > > 2 support
> > > > > >
> > > > > > Any comments?
> > > > > >
> > > > > > BR,
> > > > > > Andy
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Yuantian Tang <[email protected]>
> > > > > > > Sent: 2019年6月4日 10:51
> > > > > > > To: [email protected]; [email protected]
> > > > > > > Cc: [email protected]; Leo Li <[email protected]>;
> > > > > > > [email protected]; [email protected]; Andy
> > > > > > > Tang <[email protected]>
> > > > > > > Subject: [PATCH] thermal: qoriq: add thermal monitor unit
> > > > > > > version 2 support
> > > > > > >
> > > > > > > Thermal Monitor Unit v2 is introduced on new Layscape SoC.
> > > > > > > Compared to v1, TMUv2 has a little different register layout
> > > > > > > and digital output is fairly linear.
> > > > > > >
> > > > > > > Signed-off-by: Yuantian Tang <[email protected]>
> > > > > > > ---
> > > > > > > drivers/thermal/qoriq_thermal.c | 122
> > > > > > > +++++++++++++++++++++++++-------
> > > > > > > 1 file changed, 98 insertions(+), 24 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/thermal/qoriq_thermal.c
> > > > > > > b/drivers/thermal/qoriq_thermal.c index
> > > > > > > 3b5f5b3fb1bc..0df6dfddf804
> > > > > > > 100644
> > > > > > > --- a/drivers/thermal/qoriq_thermal.c
> > > > > > > +++ b/drivers/thermal/qoriq_thermal.c
> > > > > > > @@ -13,6 +13,15 @@
> > > > > > > #include "thermal_core.h"
> > > > > > >
> > > > > > > #define SITES_MAX 16
> > > > > > > +#define TMR_DISABLE 0x0
> > > > > > > +#define TMR_ME 0x80000000
> > > > > > > +#define TMR_ALPF 0x0c000000
> > > > > > > +#define TMR_ALPF_V2 0x03000000
> > > > > > > +#define TMTMIR_DEFAULT 0x0000000f
> > > > > > > +#define TIER_DISABLE 0x0
> > > > > > > +#define TEUMR0_V2 0x51009C00
> >
> > Better to use either lower case or capital letter for all macro
> > definitions, some are lower case and some are capital letter look like NOT
> aligned.
> I always use capital letter to define a macro.
> Did I use lower letter somewhere?
I meant below:
+#define TMTMIR_DEFAULT 0x0000000f
+#define TEUMR0_V2 0x51009C00
'f' is lower case while 'C' is capital letter.
>
> >
> > > > > > > +#define TMU_VER1 0x1
> > > > > > > +#define TMU_VER2 0x2
> > > > > > >
> > > > > > > /*
> > > > > > > * QorIQ TMU Registers
> > > > > > > @@ -23,17 +32,55 @@ struct qoriq_tmu_site_regs {
> > > > > > > u8 res0[0x8];
> > > > > > > };
> > > > > > >
> > > > > > > -struct qoriq_tmu_regs {
> > > > > > > +struct qoriq_tmu_regs_v2 {
> > > > > > > + u32 tmr; /* Mode Register */
> > > > > > > + u32 tsr; /* Status Register */
> > > > > > > + u32 tmsr; /* monitor site register */
> > > > > > > + u32 tmtmir; /* Temperature measurement
> > > > > > > interval Register
> > > > >
> > > > > */
> > > > > > > + u8 res0[0x10];
> > > > > > > + u32 tier; /* Interrupt Enable Register */
> > > > > > > + u32 tidr; /* Interrupt Detect Register */
> > > > > > > + u8 res1[0x8];
> > > > > > > + u32 tiiscr; /* interrupt immediate site
> > > > > > > capture register */
> > > > > > > + u32 tiascr; /* interrupt average site
> > > > > > > capture register */
> > > > > > > + u32 ticscr; /* Interrupt Critical Site
> > > > > > > Capture Register */
> > > > > > > + u32 res2;
> > > > > > > + u32 tmhtcr; /* monitor high temperature
> > > > > > > capture register */
> > > > > > > + u32 tmltcr; /* monitor low temperature
> > > > > > > capture register */
> > > > > > > + u32 tmrtrcr; /* monitor rising temperature rate
> > > > > > > capture register
> > > > >
> > > > > */
> > > > > > > + u32 tmftrcr; /* monitor falling temperature rate
> > > > > > > capture register
> > > > >
> > > > > */
> > > > > > > + u32 tmhtitr; /* High Temperature Immediate Threshold
> > > > > > > */
> > > > > > > + u32 tmhtatr; /* High Temperature Average Threshold
> > > > > > > */
> > > > > > > + u32 tmhtactr; /* High Temperature Average Crit
> > > > > > > Threshold */
> > > > > > > + u32 res3;
> > > > > > > + u32 tmltitr; /* monitor low temperature immediate
> > > > > > > threshold */
> > > > > > > + u32 tmltatr; /* monitor low temperature average
> > > > > > > threshold
> > > > >
> > > > > register */
> > > > > > > + u32 tmltactr; /* monitor low temperature average
> > > > > > > critical
> > > > >
> > > > > threshold */
> > > > > > > + u32 res4;
> > > > > > > + u32 tmrtrctr; /* monitor rising temperature rate
> > > > > > > critical threshold
> > > > >
> > > > > */
> > > > > > > + u32 tmftrctr; /* monitor falling temperature rate
> > > > > > > critical
> > > > >
> > > > > threshold*/
> > > > > > > + u8 res5[0x8];
> > > > > > > + u32 ttcfgr; /* Temperature Configuration Register
> > > > > > > */
> > > > > > > + u32 tscfgr; /* Sensor Configuration Register */
> > > > > > > + u8 res6[0x78];
> > > > > > > + struct qoriq_tmu_site_regs site[SITES_MAX];
> > > > > > > + u8 res7[0x9f8];
> > > > > > > + u32 ipbrr0; /* IP Block Revision Register 0
> > > > > > > */
> > > > > > > + u32 ipbrr1; /* IP Block Revision Register 1
> > > > > > > */
> > > > > > > + u8 res8[0x300];
> > > > > > > + u32 teumr0;
> > > > > > > + u32 teumr1;
> > > > > > > + u32 teumr2;
> > > > > > > + u32 res9;
> > > > > > > + u32 ttrcr[4]; /* Temperature Range Control Register
> > > > > > > */
> > > > > > > +};
> > > > > > > +
> > > > > > > +struct qoriq_tmu_regs_v1 {
> > > > > > > u32 tmr; /* Mode Register */
> > > > > > > -#define TMR_DISABLE 0x0
> > > > > > > -#define TMR_ME 0x80000000
> > > > > > > -#define TMR_ALPF 0x0c000000
> > > > > > > u32 tsr; /* Status Register */
> > > > > > > u32 tmtmir; /* Temperature measurement
> > > > > > > interval Register
> > > > >
> > > > > */
> > > > > > > -#define TMTMIR_DEFAULT 0x0000000f
> > > > > > > u8 res0[0x14];
> > > > > > > u32 tier; /* Interrupt Enable Register */
> > > > > > > -#define TIER_DISABLE 0x0
> > > > > > > u32 tidr; /* Interrupt Detect Register */
> > > > > > > u32 tiscr; /* Interrupt Site Capture
> > > > > > > Register */
> > > > > > > u32 ticscr; /* Interrupt Critical Site
> > > > > > > Capture Register */
> > > > > > > @@ -53,10 +100,7 @@ struct qoriq_tmu_regs {
> > > > > > > u32 ipbrr0; /* IP Block Revision Register 0
> > > > > > > */
> > > > > > > u32 ipbrr1; /* IP Block Revision Register 1
> > > > > > > */
> > > > > > > u8 res6[0x310];
> > > > > > > - u32 ttr0cr; /* Temperature Range 0 Control
> > > > > > > Register */
> > > > > > > - u32 ttr1cr; /* Temperature Range 1 Control
> > > > > > > Register */
> > > > > > > - u32 ttr2cr; /* Temperature Range 2 Control
> > > > > > > Register */
> > > > > > > - u32 ttr3cr; /* Temperature Range 3 Control
> > > > > > > Register */
> > > > > > > + u32 ttrcr[4]; /* Temperature Range Control
> > > > > > > Register */
> > > > > > > };
> > > > > > >
> > > > > > > struct qoriq_tmu_data;
> > > > > > > @@ -71,7 +115,9 @@ struct qoriq_sensor { };
> > > > > > >
> > > > > > > struct qoriq_tmu_data {
> > > > > > > - struct qoriq_tmu_regs __iomem *regs;
> > > > > > > + int ver;
> > > > > > > + struct qoriq_tmu_regs_v1 __iomem *regs;
> > > > > > > + struct qoriq_tmu_regs_v2 __iomem *regv2;
> >
> > regv2 -> regs_v2 ?
> OK
>
> >
> > > > > > > bool little_endian;
> > > > > > > struct qoriq_sensor *sensor[SITES_MAX];
> > > > > > > };
> > > > > > > @@ -111,7 +157,7 @@ static const struct
> > > > > > > thermal_zone_of_device_ops tmu_tz_ops = { static int
> > > > > > > qoriq_tmu_register_tmu_zone(struct
> > > > > > > platform_device *pdev) {
> > > > > > > struct qoriq_tmu_data *qdata = platform_get_drvdata(pdev);
> > > > > > > - int id, sites = 0;
> > > > > > > + int id, sites = 0, sv2 = 0;
> >
> > I did NOT see 'sites' and 'sv2' used together in below code, why NOT
> > just use 'sites' for both V1 and V2?
> Yeah, using one variable is better.
>
> >
> > > > > > >
> > > > > > > for (id = 0; id < SITES_MAX; id++) {
> > > > > > > qdata->sensor[id] = devm_kzalloc(&pdev->dev, @@
> > > > > > > -130,12
> > > > > > > +176,24 @@ static int qoriq_tmu_register_tmu_zone(struct
> > > > > > > +platform_device
> > > > > > > *pdev)
> > > > > > > return PTR_ERR(qdata-
> > > > > > > >sensor[id]->tzd);
> > > > > > > }
> > > > > > >
> > > > > > > - sites |= 0x1 << (15 - id);
> > > > > > > + if (qdata->ver == TMU_VER1)
> > > > > > > + sites |= 0x1 << (15 - id);
> > > > > > > + else
> > > > > > > + sv2 |= 0x1 << id;
> > > > > > > }
> > > > > > >
> > > > > > > /* Enable monitoring */
> > > > > > > - if (sites != 0)
> > > > > > > - tmu_write(qdata, sites | TMR_ME | TMR_ALPF,
> > > > > > > &qdata->regs->tmr);
> > > > > > > + if (qdata->ver == TMU_VER1) {
> > > > > > > + if (sites != 0)
> > > > > > > + tmu_write(qdata, sites | TMR_ME |
> > > > > > > TMR_ALPF,
> > > > > > > + &qdata->regs->tmr);
> > > > > > > + } else {
> > > > > > > + if (sv2 != 0) {
> > > > > > > + tmu_write(qdata, sv2, &qdata->regv2-
> > > > > > > >tmsr);
> > > > > > > + tmu_write(qdata, TMR_ME | TMR_ALPF_V2,
> > > > > > > + &qdata->regv2->tmr);
> > > > > > > + }
> > > > > > > + }
> > > > > > >
> > > > > > > return 0;
> > > > > > > }
> > > > > > > @@ -148,16 +206,20 @@ static int
> > > > > > > qoriq_tmu_calibration(struct platform_device *pdev)
> > > > > > > struct device_node *np = pdev->dev.of_node;
> > > > > > > struct qoriq_tmu_data *data = platform_get_drvdata(pdev);
> > > > > > >
> > > > > > > - if (of_property_read_u32_array(np, "fsl,tmu-range",
> > > > > > > range, 4)) {
> > > > > > > - dev_err(&pdev->dev, "missing calibration
> > > > > > > range.\n");
> > > > > > > - return -ENODEV;
> > > > > > > + len = of_property_count_u32_elems(np, "fsl,tmu-range");
> > > > > > > + if (len == -ENODATA || len == -EINVAL || len > 4) {
> >
> > Can we just check the "len < 0 || len > 4" ?
> OK.
>
> >
> > > > > > > + dev_err(&pdev->dev, "invalid range data.\n");
> > > > > > > + return len;
> > > > > > > }
> > > > > > >
> > > > > > > - /* Init temperature range registers */
> > > > > > > - tmu_write(data, range[0], &data->regs->ttr0cr);
> > > > > > > - tmu_write(data, range[1], &data->regs->ttr1cr);
> > > > > > > - tmu_write(data, range[2], &data->regs->ttr2cr);
> > > > > > > - tmu_write(data, range[3], &data->regs->ttr3cr);
> > > > > > > + val = of_property_read_u32_array(np, "fsl,tmu-range",
> > > > > > > range, len);
> > > > > > > + if (val != 0) {
> > > > > > > + dev_err(&pdev->dev, "invalid range data.\n");
> >
> > The error log is same as failure of getting range data count, so would
> > it be better to change the log to "failed to read range data" here?
> Yeah, better.
>
> >
> > > > > > > + return val;
> > > > > > > + }
> > > > > > > +
> > > > > > > + for (i = 0; i < len; i++)
> > > > > > > + tmu_write(data, range[i], &data->regs-
> > > > > > > >ttrcr[i]);
> > > > > > >
> > > > > > > calibration = of_get_property(np, "fsl,tmu- calibration",
> > > > > > > &len);
> > > > > > > if (calibration == NULL || len % 8) { @@ -181,7 +243,12 @@
> > > > > > > static void qoriq_tmu_init_device(struct qoriq_tmu_data *data)
> > > > > > > tmu_write(data, TIER_DISABLE, &data->regs->tier);
> > > > > > >
> > > > > > > /* Set update_interval */
> > > > > > > - tmu_write(data, TMTMIR_DEFAULT, &data->regs->tmtmir);
> > > > > > > + if (data->ver == TMU_VER1) {
> > > > > > > + tmu_write(data, TMTMIR_DEFAULT, &data->regs-
> > > > > > > >tmtmir);
> > > > > > > + } else {
> > > > > > > + tmu_write(data, TMTMIR_DEFAULT, &data->regv2-
> > > > > > > >tmtmir);
> > > > > > > + tmu_write(data, TEUMR0_V2, &data->regv2-
> > > > > > > >teumr0);
> > > > > > > + }
> > > > > > >
> > > > > > > /* Disable monitoring */
> > > > > > > tmu_write(data, TMR_DISABLE, &data->regs->tmr); @@
> > > > > > > -190,6
> > > > >
> > > > > +257,7
> > > > > > @@
> > > > > > > static void qoriq_tmu_init_device(struct qoriq_tmu_data
> > > > > > > *data) static int qoriq_tmu_probe(struct platform_device *pdev) {
> > > > > > > int ret;
> > > > > > > + u32 ver;
> > > > > > > struct qoriq_tmu_data *data;
> > > > > > > struct device_node *np = pdev->dev.of_node;
> > > > > > >
> > > > > > > @@ -214,6 +282,12 @@ static int qoriq_tmu_probe(struct
> > > > > >
> > > > > > platform_device
> > > > > > > *pdev)
> > > > > > > goto err_iomap;
> > > > > > > }
> > > > > > >
> > > > > > > + /* version register offset at: 0xbf8 on both v1 and v2
> > > > > > > */
> > > > > > > + ver = tmu_read(data, &data->regs->ipbrr0);
> > > > > > > + data->ver = (ver >> 8) & 0xff;
> > > > > > > + if (data->ver == TMU_VER2)
> > > > > > > + data->regv2 = (void __iomem *)data->regs;
> >
> > So the V2 has same register base address as V1? If yes, then I think
> > no need to check the version here, just assign V2's reg to equal to V1's
> should be OK?
> For any one specific soc, TMU must be v1 or v2. So basically only using regv1
> or regv2 is enough.
> Here both regv1 and regv2 are used and are assigned to the same base
> address.
>
> I use regv1 to access the common registers for both regv1 and regv2.
> I use regv2 to access the registers that are specific to regv2.
> In this way, I can reuse the current register read/write code and only update
> the code for regv2 access.
OK, make sense.
Anson.
Hi Anson,
Points are taken. Will send out the next version.
Thanks,
Andy
> -----Original Message-----
> From: Anson Huang
> Sent: 2019年9月24日 10:11
> To: Andy Tang <[email protected]>; Zhang Rui <[email protected]>;
> [email protected]
> Cc: [email protected]; Leo Li <[email protected]>;
> [email protected]; [email protected]
> Subject: RE: [PATCH] thermal: qoriq: add thermal monitor unit version 2
> support
>
> Hi, Andy
>
> > Subject: RE: [PATCH] thermal: qoriq: add thermal monitor unit version
> > 2 support
> >
> > Hi Anson,
> >
> > Thanks for your review. Please see my reply inline.
> >
> > > -----Original Message-----
> > > From: Anson Huang
> > > Sent: 2019年9月24日 9:17
> > > To: Zhang Rui <[email protected]>; Andy Tang <[email protected]>;
> > > [email protected]
> > > Cc: [email protected]; Leo Li <[email protected]>;
> > > [email protected]; [email protected]
> > > Subject: RE: [PATCH] thermal: qoriq: add thermal monitor unit
> > > version
> > > 2 support
> > >
> > > Hi, Andy
> > >
> > >
> > > > On Mon, 2019-09-23 at 09:24 +0000, Andy Tang wrote:
> > > > > Hi Rui, Edubezval,
> > > > >
> > > > > Would you please review this patch?
> > > > >
> > > > CC Anson Huang.
> > > > I'd prefer all the qoriq thermal patches go through his review first.
> > > >
> > > > thanks,
> > > > rui
> > > >
> > > > > BR,
> > > > > Andy
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Andy Tang
> > > > > > Sent: 2019年8月29日 16:38
> > > > > > To: '[email protected]' <[email protected]>;
> > > > > > '[email protected]'
> > > > > > <[email protected]>
> > > > > > Cc: '[email protected]' <[email protected]>;
> > > > > > Leo Li <[email protected]>; '[email protected]'
> > > > > > <[email protected]>; '[email protected]'
> > > > > > <[email protected]>
> > > > > > Subject: RE: [PATCH] thermal: qoriq: add thermal monitor unit
> > > > > > version 2 support
> > > > > >
> > > > > > Hi Rui, Edubezval,
> > > > > >
> > > > > > Almost three monthes passed, I have not got your comments from
> > you.
> > > > > > Could you please take a look at this patch?
> > > > > >
> > > > > > BR,
> > > > > > Andy
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Andy Tang
> > > > > > > Sent: 2019年8月6日 10:57
> > > > > > > To: [email protected]; [email protected]
> > > > > > > Cc: [email protected]; Leo Li <[email protected]>;
> > > > > > > [email protected]; [email protected]
> > > > > > > Subject: RE: [PATCH] thermal: qoriq: add thermal monitor
> > > > > > > unit version
> > > > > > > 2 support
> > > > > > >
> > > > > > > Any comments?
> > > > > > >
> > > > > > > BR,
> > > > > > > Andy
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: Yuantian Tang <[email protected]>
> > > > > > > > Sent: 2019年6月4日 10:51
> > > > > > > > To: [email protected]; [email protected]
> > > > > > > > Cc: [email protected]; Leo Li
> > > > > > > > <[email protected]>; [email protected];
> > > > > > > > [email protected]; Andy Tang
> > > > > > > > <[email protected]>
> > > > > > > > Subject: [PATCH] thermal: qoriq: add thermal monitor unit
> > > > > > > > version 2 support
> > > > > > > >
> > > > > > > > Thermal Monitor Unit v2 is introduced on new Layscape SoC.
> > > > > > > > Compared to v1, TMUv2 has a little different register
> > > > > > > > layout and digital output is fairly linear.
> > > > > > > >
> > > > > > > > Signed-off-by: Yuantian Tang <[email protected]>
> > > > > > > > ---
> > > > > > > > drivers/thermal/qoriq_thermal.c | 122
> > > > > > > > +++++++++++++++++++++++++-------
> > > > > > > > 1 file changed, 98 insertions(+), 24 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/drivers/thermal/qoriq_thermal.c
> > > > > > > > b/drivers/thermal/qoriq_thermal.c index
> > > > > > > > 3b5f5b3fb1bc..0df6dfddf804
> > > > > > > > 100644
> > > > > > > > --- a/drivers/thermal/qoriq_thermal.c
> > > > > > > > +++ b/drivers/thermal/qoriq_thermal.c
> > > > > > > > @@ -13,6 +13,15 @@
> > > > > > > > #include "thermal_core.h"
> > > > > > > >
> > > > > > > > #define SITES_MAX 16
> > > > > > > > +#define TMR_DISABLE 0x0
> > > > > > > > +#define TMR_ME 0x80000000
> > > > > > > > +#define TMR_ALPF 0x0c000000
> > > > > > > > +#define TMR_ALPF_V2 0x03000000
> > > > > > > > +#define TMTMIR_DEFAULT 0x0000000f
> > > > > > > > +#define TIER_DISABLE 0x0
> > > > > > > > +#define TEUMR0_V2 0x51009C00
> > >
> > > Better to use either lower case or capital letter for all macro
> > > definitions, some are lower case and some are capital letter look
> > > like NOT
> > aligned.
> > I always use capital letter to define a macro.
> > Did I use lower letter somewhere?
>
> I meant below:
>
> +#define TMTMIR_DEFAULT 0x0000000f
> +#define TEUMR0_V2 0x51009C00
>
> 'f' is lower case while 'C' is capital letter.
>
> >
> > >
> > > > > > > > +#define TMU_VER1 0x1
> > > > > > > > +#define TMU_VER2 0x2
> > > > > > > >
> > > > > > > > /*
> > > > > > > > * QorIQ TMU Registers
> > > > > > > > @@ -23,17 +32,55 @@ struct qoriq_tmu_site_regs {
> > > > > > > > u8 res0[0x8];
> > > > > > > > };
> > > > > > > >
> > > > > > > > -struct qoriq_tmu_regs {
> > > > > > > > +struct qoriq_tmu_regs_v2 {
> > > > > > > > + u32 tmr; /* Mode Register */
> > > > > > > > + u32 tsr; /* Status Register */
> > > > > > > > + u32 tmsr; /* monitor site register */
> > > > > > > > + u32 tmtmir; /* Temperature measurement
> > > > > > > > interval Register
> > > > > >
> > > > > > */
> > > > > > > > + u8 res0[0x10];
> > > > > > > > + u32 tier; /* Interrupt Enable Register */
> > > > > > > > + u32 tidr; /* Interrupt Detect Register */
> > > > > > > > + u8 res1[0x8];
> > > > > > > > + u32 tiiscr; /* interrupt immediate site
> > > > > > > > capture register */
> > > > > > > > + u32 tiascr; /* interrupt average site
> > > > > > > > capture register */
> > > > > > > > + u32 ticscr; /* Interrupt Critical Site
> > > > > > > > Capture Register */
> > > > > > > > + u32 res2;
> > > > > > > > + u32 tmhtcr; /* monitor high temperature
> > > > > > > > capture register */
> > > > > > > > + u32 tmltcr; /* monitor low temperature
> > > > > > > > capture register */
> > > > > > > > + u32 tmrtrcr; /* monitor rising temperature rate
> > > > > > > > capture register
> > > > > >
> > > > > > */
> > > > > > > > + u32 tmftrcr; /* monitor falling temperature rate
> > > > > > > > capture register
> > > > > >
> > > > > > */
> > > > > > > > + u32 tmhtitr; /* High Temperature Immediate Threshold
> > > > > > > > */
> > > > > > > > + u32 tmhtatr; /* High Temperature Average Threshold
> > > > > > > > */
> > > > > > > > + u32 tmhtactr; /* High Temperature Average Crit
> > > > > > > > Threshold */
> > > > > > > > + u32 res3;
> > > > > > > > + u32 tmltitr; /* monitor low temperature immediate
> > > > > > > > threshold */
> > > > > > > > + u32 tmltatr; /* monitor low temperature average
> > > > > > > > threshold
> > > > > >
> > > > > > register */
> > > > > > > > + u32 tmltactr; /* monitor low temperature average
> > > > > > > > critical
> > > > > >
> > > > > > threshold */
> > > > > > > > + u32 res4;
> > > > > > > > + u32 tmrtrctr; /* monitor rising temperature rate
> > > > > > > > critical threshold
> > > > > >
> > > > > > */
> > > > > > > > + u32 tmftrctr; /* monitor falling temperature rate
> > > > > > > > critical
> > > > > >
> > > > > > threshold*/
> > > > > > > > + u8 res5[0x8];
> > > > > > > > + u32 ttcfgr; /* Temperature Configuration Register
> > > > > > > > */
> > > > > > > > + u32 tscfgr; /* Sensor Configuration Register */
> > > > > > > > + u8 res6[0x78];
> > > > > > > > + struct qoriq_tmu_site_regs site[SITES_MAX];
> > > > > > > > + u8 res7[0x9f8];
> > > > > > > > + u32 ipbrr0; /* IP Block Revision Register 0
> > > > > > > > */
> > > > > > > > + u32 ipbrr1; /* IP Block Revision Register 1
> > > > > > > > */
> > > > > > > > + u8 res8[0x300];
> > > > > > > > + u32 teumr0;
> > > > > > > > + u32 teumr1;
> > > > > > > > + u32 teumr2;
> > > > > > > > + u32 res9;
> > > > > > > > + u32 ttrcr[4]; /* Temperature Range Control Register
> > > > > > > > */
> > > > > > > > +};
> > > > > > > > +
> > > > > > > > +struct qoriq_tmu_regs_v1 {
> > > > > > > > u32 tmr; /* Mode Register */
> > > > > > > > -#define TMR_DISABLE 0x0
> > > > > > > > -#define TMR_ME 0x80000000
> > > > > > > > -#define TMR_ALPF 0x0c000000
> > > > > > > > u32 tsr; /* Status Register */
> > > > > > > > u32 tmtmir; /* Temperature measurement
> > > > > > > > interval Register
> > > > > >
> > > > > > */
> > > > > > > > -#define TMTMIR_DEFAULT 0x0000000f
> > > > > > > > u8 res0[0x14];
> > > > > > > > u32 tier; /* Interrupt Enable Register */
> > > > > > > > -#define TIER_DISABLE 0x0
> > > > > > > > u32 tidr; /* Interrupt Detect Register */
> > > > > > > > u32 tiscr; /* Interrupt Site Capture
> > > > > > > > Register */
> > > > > > > > u32 ticscr; /* Interrupt Critical Site
> > > > > > > > Capture Register */
> > > > > > > > @@ -53,10 +100,7 @@ struct qoriq_tmu_regs {
> > > > > > > > u32 ipbrr0; /* IP Block Revision Register 0
> > > > > > > > */
> > > > > > > > u32 ipbrr1; /* IP Block Revision Register 1
> > > > > > > > */
> > > > > > > > u8 res6[0x310];
> > > > > > > > - u32 ttr0cr; /* Temperature Range 0 Control
> > > > > > > > Register */
> > > > > > > > - u32 ttr1cr; /* Temperature Range 1 Control
> > > > > > > > Register */
> > > > > > > > - u32 ttr2cr; /* Temperature Range 2 Control
> > > > > > > > Register */
> > > > > > > > - u32 ttr3cr; /* Temperature Range 3 Control
> > > > > > > > Register */
> > > > > > > > + u32 ttrcr[4]; /* Temperature Range Control
> > > > > > > > Register */
> > > > > > > > };
> > > > > > > >
> > > > > > > > struct qoriq_tmu_data;
> > > > > > > > @@ -71,7 +115,9 @@ struct qoriq_sensor { };
> > > > > > > >
> > > > > > > > struct qoriq_tmu_data {
> > > > > > > > - struct qoriq_tmu_regs __iomem *regs;
> > > > > > > > + int ver;
> > > > > > > > + struct qoriq_tmu_regs_v1 __iomem *regs;
> > > > > > > > + struct qoriq_tmu_regs_v2 __iomem *regv2;
> > >
> > > regv2 -> regs_v2 ?
> > OK
> >
> > >
> > > > > > > > bool little_endian;
> > > > > > > > struct qoriq_sensor *sensor[SITES_MAX];
> > > > > > > > };
> > > > > > > > @@ -111,7 +157,7 @@ static const struct
> > > > > > > > thermal_zone_of_device_ops tmu_tz_ops = { static int
> > > > > > > > qoriq_tmu_register_tmu_zone(struct
> > > > > > > > platform_device *pdev) {
> > > > > > > > struct qoriq_tmu_data *qdata =
> platform_get_drvdata(pdev);
> > > > > > > > - int id, sites = 0;
> > > > > > > > + int id, sites = 0, sv2 = 0;
> > >
> > > I did NOT see 'sites' and 'sv2' used together in below code, why NOT
> > > just use 'sites' for both V1 and V2?
> > Yeah, using one variable is better.
> >
> > >
> > > > > > > >
> > > > > > > > for (id = 0; id < SITES_MAX; id++) {
> > > > > > > > qdata->sensor[id] = devm_kzalloc(&pdev->dev, @@
> > > > > > > > -130,12
> > > > > > > > +176,24 @@ static int qoriq_tmu_register_tmu_zone(struct
> > > > > > > > +platform_device
> > > > > > > > *pdev)
> > > > > > > > return PTR_ERR(qdata-
> > > > > > > > >sensor[id]->tzd);
> > > > > > > > }
> > > > > > > >
> > > > > > > > - sites |= 0x1 << (15 - id);
> > > > > > > > + if (qdata->ver == TMU_VER1)
> > > > > > > > + sites |= 0x1 << (15 - id);
> > > > > > > > + else
> > > > > > > > + sv2 |= 0x1 << id;
> > > > > > > > }
> > > > > > > >
> > > > > > > > /* Enable monitoring */
> > > > > > > > - if (sites != 0)
> > > > > > > > - tmu_write(qdata, sites | TMR_ME | TMR_ALPF,
> > > > > > > > &qdata->regs->tmr);
> > > > > > > > + if (qdata->ver == TMU_VER1) {
> > > > > > > > + if (sites != 0)
> > > > > > > > + tmu_write(qdata, sites | TMR_ME |
> > > > > > > > TMR_ALPF,
> > > > > > > > + &qdata->regs->tmr);
> > > > > > > > + } else {
> > > > > > > > + if (sv2 != 0) {
> > > > > > > > + tmu_write(qdata, sv2, &qdata->regv2-
> > > > > > > > >tmsr);
> > > > > > > > + tmu_write(qdata, TMR_ME | TMR_ALPF_V2,
> > > > > > > > + &qdata->regv2->tmr);
> > > > > > > > + }
> > > > > > > > + }
> > > > > > > >
> > > > > > > > return 0;
> > > > > > > > }
> > > > > > > > @@ -148,16 +206,20 @@ static int
> > > > > > > > qoriq_tmu_calibration(struct platform_device *pdev)
> > > > > > > > struct device_node *np = pdev->dev.of_node;
> > > > > > > > struct qoriq_tmu_data *data =
> > > > > > > > platform_get_drvdata(pdev);
> > > > > > > >
> > > > > > > > - if (of_property_read_u32_array(np, "fsl,tmu-range",
> > > > > > > > range, 4)) {
> > > > > > > > - dev_err(&pdev->dev, "missing calibration
> > > > > > > > range.\n");
> > > > > > > > - return -ENODEV;
> > > > > > > > + len = of_property_count_u32_elems(np, "fsl,tmu-range");
> > > > > > > > + if (len == -ENODATA || len == -EINVAL || len > 4) {
> > >
> > > Can we just check the "len < 0 || len > 4" ?
> > OK.
> >
> > >
> > > > > > > > + dev_err(&pdev->dev, "invalid range data.\n");
> > > > > > > > + return len;
> > > > > > > > }
> > > > > > > >
> > > > > > > > - /* Init temperature range registers */
> > > > > > > > - tmu_write(data, range[0], &data->regs->ttr0cr);
> > > > > > > > - tmu_write(data, range[1], &data->regs->ttr1cr);
> > > > > > > > - tmu_write(data, range[2], &data->regs->ttr2cr);
> > > > > > > > - tmu_write(data, range[3], &data->regs->ttr3cr);
> > > > > > > > + val = of_property_read_u32_array(np, "fsl,tmu-range",
> > > > > > > > range, len);
> > > > > > > > + if (val != 0) {
> > > > > > > > + dev_err(&pdev->dev, "invalid range data.\n");
> > >
> > > The error log is same as failure of getting range data count, so
> > > would it be better to change the log to "failed to read range data" here?
> > Yeah, better.
> >
> > >
> > > > > > > > + return val;
> > > > > > > > + }
> > > > > > > > +
> > > > > > > > + for (i = 0; i < len; i++)
> > > > > > > > + tmu_write(data, range[i], &data->regs-
> > > > > > > > >ttrcr[i]);
> > > > > > > >
> > > > > > > > calibration = of_get_property(np, "fsl,tmu-
> > > > > > > > calibration", &len);
> > > > > > > > if (calibration == NULL || len % 8) { @@ -181,7 +243,12
> > > > > > > > @@ static void qoriq_tmu_init_device(struct qoriq_tmu_data
> *data)
> > > > > > > > tmu_write(data, TIER_DISABLE, &data->regs->tier);
> > > > > > > >
> > > > > > > > /* Set update_interval */
> > > > > > > > - tmu_write(data, TMTMIR_DEFAULT, &data->regs->tmtmir);
> > > > > > > > + if (data->ver == TMU_VER1) {
> > > > > > > > + tmu_write(data, TMTMIR_DEFAULT, &data->regs-
> > > > > > > > >tmtmir);
> > > > > > > > + } else {
> > > > > > > > + tmu_write(data, TMTMIR_DEFAULT, &data->regv2-
> > > > > > > > >tmtmir);
> > > > > > > > + tmu_write(data, TEUMR0_V2, &data->regv2-
> > > > > > > > >teumr0);
> > > > > > > > + }
> > > > > > > >
> > > > > > > > /* Disable monitoring */
> > > > > > > > tmu_write(data, TMR_DISABLE, &data->regs->tmr); @@
> > > > > > > > -190,6
> > > > > >
> > > > > > +257,7
> > > > > > > @@
> > > > > > > > static void qoriq_tmu_init_device(struct qoriq_tmu_data
> > > > > > > > *data) static int qoriq_tmu_probe(struct platform_device *pdev)
> {
> > > > > > > > int ret;
> > > > > > > > + u32 ver;
> > > > > > > > struct qoriq_tmu_data *data;
> > > > > > > > struct device_node *np = pdev->dev.of_node;
> > > > > > > >
> > > > > > > > @@ -214,6 +282,12 @@ static int qoriq_tmu_probe(struct
> > > > > > >
> > > > > > > platform_device
> > > > > > > > *pdev)
> > > > > > > > goto err_iomap;
> > > > > > > > }
> > > > > > > >
> > > > > > > > + /* version register offset at: 0xbf8 on both v1 and v2
> > > > > > > > */
> > > > > > > > + ver = tmu_read(data, &data->regs->ipbrr0);
> > > > > > > > + data->ver = (ver >> 8) & 0xff;
> > > > > > > > + if (data->ver == TMU_VER2)
> > > > > > > > + data->regv2 = (void __iomem *)data->regs;
> > >
> > > So the V2 has same register base address as V1? If yes, then I think
> > > no need to check the version here, just assign V2's reg to equal to
> > > V1's
> > should be OK?
> > For any one specific soc, TMU must be v1 or v2. So basically only
> > using regv1 or regv2 is enough.
> > Here both regv1 and regv2 are used and are assigned to the same base
> > address.
> >
> > I use regv1 to access the common registers for both regv1 and regv2.
> > I use regv2 to access the registers that are specific to regv2.
> > In this way, I can reuse the current register read/write code and only
> > update the code for regv2 access.
>
> OK, make sense.
>
> Anson.
Hi, Andy
> On Mon, 2019-09-23 at 09:24 +0000, Andy Tang wrote:
> > Hi Rui, Edubezval,
> >
> > Would you please review this patch?
> >
> CC Anson Huang.
> I'd prefer all the qoriq thermal patches go through his review first.
>
> thanks,
> rui
>
> > BR,
> > Andy
> >
> > > -----Original Message-----
> > > From: Andy Tang
> > > Sent: 2019年8月29日 16:38
> > > To: '[email protected]' <[email protected]>;
> > > '[email protected]'
> > > <[email protected]>
> > > Cc: '[email protected]' <[email protected]>; Leo Li
> > > <[email protected]>; '[email protected]'
> > > <[email protected]>; '[email protected]'
> > > <[email protected]>
> > > Subject: RE: [PATCH] thermal: qoriq: add thermal monitor unit
> > > version 2 support
> > >
> > > Hi Rui, Edubezval,
> > >
> > > Almost three monthes passed, I have not got your comments from you.
> > > Could you please take a look at this patch?
> > >
> > > BR,
> > > Andy
> > >
> > > > -----Original Message-----
> > > > From: Andy Tang
> > > > Sent: 2019年8月6日 10:57
> > > > To: [email protected]; [email protected]
> > > > Cc: [email protected]; Leo Li <[email protected]>;
> > > > [email protected]; [email protected]
> > > > Subject: RE: [PATCH] thermal: qoriq: add thermal monitor unit
> > > > version
> > > > 2 support
> > > >
> > > > Any comments?
> > > >
> > > > BR,
> > > > Andy
> > > >
> > > > > -----Original Message-----
> > > > > From: Yuantian Tang <[email protected]>
> > > > > Sent: 2019年6月4日 10:51
> > > > > To: [email protected]; [email protected]
> > > > > Cc: [email protected]; Leo Li <[email protected]>;
> > > > > [email protected]; [email protected]; Andy
> > > > > Tang <[email protected]>
> > > > > Subject: [PATCH] thermal: qoriq: add thermal monitor unit
> > > > > version 2 support
> > > > >
> > > > > Thermal Monitor Unit v2 is introduced on new Layscape SoC.
> > > > > Compared to v1, TMUv2 has a little different register layout and
> > > > > digital output is fairly linear.
> > > > >
> > > > > Signed-off-by: Yuantian Tang <[email protected]>
> > > > > ---
> > > > > drivers/thermal/qoriq_thermal.c | 122
> > > > > +++++++++++++++++++++++++-------
> > > > > 1 file changed, 98 insertions(+), 24 deletions(-)
> > > > >
> > > > > diff --git a/drivers/thermal/qoriq_thermal.c
> > > > > b/drivers/thermal/qoriq_thermal.c index
> > > > > 3b5f5b3fb1bc..0df6dfddf804
> > > > > 100644
> > > > > --- a/drivers/thermal/qoriq_thermal.c
> > > > > +++ b/drivers/thermal/qoriq_thermal.c
> > > > > @@ -13,6 +13,15 @@
> > > > > #include "thermal_core.h"
> > > > >
> > > > > #define SITES_MAX 16
> > > > > +#define TMR_DISABLE 0x0
> > > > > +#define TMR_ME 0x80000000
> > > > > +#define TMR_ALPF 0x0c000000
> > > > > +#define TMR_ALPF_V2 0x03000000
> > > > > +#define TMTMIR_DEFAULT 0x0000000f
> > > > > +#define TIER_DISABLE 0x0
> > > > > +#define TEUMR0_V2 0x51009C00
Better to use either lower case or capital letter for all macro definitions,
some are lower case and some are capital letter look like NOT aligned.
> > > > > +#define TMU_VER1 0x1
> > > > > +#define TMU_VER2 0x2
> > > > >
> > > > > /*
> > > > > * QorIQ TMU Registers
> > > > > @@ -23,17 +32,55 @@ struct qoriq_tmu_site_regs {
> > > > > u8 res0[0x8];
> > > > > };
> > > > >
> > > > > -struct qoriq_tmu_regs {
> > > > > +struct qoriq_tmu_regs_v2 {
> > > > > + u32 tmr; /* Mode Register */
> > > > > + u32 tsr; /* Status Register */
> > > > > + u32 tmsr; /* monitor site register */
> > > > > + u32 tmtmir; /* Temperature measurement
> > > > > interval Register
> > >
> > > */
> > > > > + u8 res0[0x10];
> > > > > + u32 tier; /* Interrupt Enable Register */
> > > > > + u32 tidr; /* Interrupt Detect Register */
> > > > > + u8 res1[0x8];
> > > > > + u32 tiiscr; /* interrupt immediate site
> > > > > capture register */
> > > > > + u32 tiascr; /* interrupt average site
> > > > > capture register */
> > > > > + u32 ticscr; /* Interrupt Critical Site
> > > > > Capture Register */
> > > > > + u32 res2;
> > > > > + u32 tmhtcr; /* monitor high temperature
> > > > > capture register */
> > > > > + u32 tmltcr; /* monitor low temperature
> > > > > capture register */
> > > > > + u32 tmrtrcr; /* monitor rising temperature rate
> > > > > capture register
> > >
> > > */
> > > > > + u32 tmftrcr; /* monitor falling temperature rate
> > > > > capture register
> > >
> > > */
> > > > > + u32 tmhtitr; /* High Temperature Immediate Threshold
> > > > > */
> > > > > + u32 tmhtatr; /* High Temperature Average Threshold
> > > > > */
> > > > > + u32 tmhtactr; /* High Temperature Average Crit
> > > > > Threshold */
> > > > > + u32 res3;
> > > > > + u32 tmltitr; /* monitor low temperature immediate
> > > > > threshold */
> > > > > + u32 tmltatr; /* monitor low temperature average
> > > > > threshold
> > >
> > > register */
> > > > > + u32 tmltactr; /* monitor low temperature average
> > > > > critical
> > >
> > > threshold */
> > > > > + u32 res4;
> > > > > + u32 tmrtrctr; /* monitor rising temperature rate
> > > > > critical threshold
> > >
> > > */
> > > > > + u32 tmftrctr; /* monitor falling temperature rate
> > > > > critical
> > >
> > > threshold*/
> > > > > + u8 res5[0x8];
> > > > > + u32 ttcfgr; /* Temperature Configuration Register
> > > > > */
> > > > > + u32 tscfgr; /* Sensor Configuration Register */
> > > > > + u8 res6[0x78];
> > > > > + struct qoriq_tmu_site_regs site[SITES_MAX];
> > > > > + u8 res7[0x9f8];
> > > > > + u32 ipbrr0; /* IP Block Revision Register 0
> > > > > */
> > > > > + u32 ipbrr1; /* IP Block Revision Register 1
> > > > > */
> > > > > + u8 res8[0x300];
> > > > > + u32 teumr0;
> > > > > + u32 teumr1;
> > > > > + u32 teumr2;
> > > > > + u32 res9;
> > > > > + u32 ttrcr[4]; /* Temperature Range Control Register
> > > > > */
> > > > > +};
> > > > > +
> > > > > +struct qoriq_tmu_regs_v1 {
> > > > > u32 tmr; /* Mode Register */
> > > > > -#define TMR_DISABLE 0x0
> > > > > -#define TMR_ME 0x80000000
> > > > > -#define TMR_ALPF 0x0c000000
> > > > > u32 tsr; /* Status Register */
> > > > > u32 tmtmir; /* Temperature measurement
> > > > > interval Register
> > >
> > > */
> > > > > -#define TMTMIR_DEFAULT 0x0000000f
> > > > > u8 res0[0x14];
> > > > > u32 tier; /* Interrupt Enable Register */
> > > > > -#define TIER_DISABLE 0x0
> > > > > u32 tidr; /* Interrupt Detect Register */
> > > > > u32 tiscr; /* Interrupt Site Capture
> > > > > Register */
> > > > > u32 ticscr; /* Interrupt Critical Site
> > > > > Capture Register */
> > > > > @@ -53,10 +100,7 @@ struct qoriq_tmu_regs {
> > > > > u32 ipbrr0; /* IP Block Revision Register 0
> > > > > */
> > > > > u32 ipbrr1; /* IP Block Revision Register 1
> > > > > */
> > > > > u8 res6[0x310];
> > > > > - u32 ttr0cr; /* Temperature Range 0 Control
> > > > > Register */
> > > > > - u32 ttr1cr; /* Temperature Range 1 Control
> > > > > Register */
> > > > > - u32 ttr2cr; /* Temperature Range 2 Control
> > > > > Register */
> > > > > - u32 ttr3cr; /* Temperature Range 3 Control
> > > > > Register */
> > > > > + u32 ttrcr[4]; /* Temperature Range Control
> > > > > Register */
> > > > > };
> > > > >
> > > > > struct qoriq_tmu_data;
> > > > > @@ -71,7 +115,9 @@ struct qoriq_sensor { };
> > > > >
> > > > > struct qoriq_tmu_data {
> > > > > - struct qoriq_tmu_regs __iomem *regs;
> > > > > + int ver;
> > > > > + struct qoriq_tmu_regs_v1 __iomem *regs;
> > > > > + struct qoriq_tmu_regs_v2 __iomem *regv2;
regv2 -> regs_v2 ?
> > > > > bool little_endian;
> > > > > struct qoriq_sensor *sensor[SITES_MAX];
> > > > > };
> > > > > @@ -111,7 +157,7 @@ static const struct
> > > > > thermal_zone_of_device_ops tmu_tz_ops = { static int
> > > > > qoriq_tmu_register_tmu_zone(struct
> > > > > platform_device *pdev) {
> > > > > struct qoriq_tmu_data *qdata = platform_get_drvdata(pdev);
> > > > > - int id, sites = 0;
> > > > > + int id, sites = 0, sv2 = 0;
I did NOT see 'sites' and 'sv2' used together in below code, why NOT just use 'sites' for both V1 and V2?
> > > > >
> > > > > for (id = 0; id < SITES_MAX; id++) {
> > > > > qdata->sensor[id] = devm_kzalloc(&pdev->dev, @@
> > > > > -130,12
> > > > > +176,24 @@ static int qoriq_tmu_register_tmu_zone(struct
> > > > > +platform_device
> > > > > *pdev)
> > > > > return PTR_ERR(qdata-
> > > > > >sensor[id]->tzd);
> > > > > }
> > > > >
> > > > > - sites |= 0x1 << (15 - id);
> > > > > + if (qdata->ver == TMU_VER1)
> > > > > + sites |= 0x1 << (15 - id);
> > > > > + else
> > > > > + sv2 |= 0x1 << id;
> > > > > }
> > > > >
> > > > > /* Enable monitoring */
> > > > > - if (sites != 0)
> > > > > - tmu_write(qdata, sites | TMR_ME | TMR_ALPF,
> > > > > &qdata->regs->tmr);
> > > > > + if (qdata->ver == TMU_VER1) {
> > > > > + if (sites != 0)
> > > > > + tmu_write(qdata, sites | TMR_ME |
> > > > > TMR_ALPF,
> > > > > + &qdata->regs->tmr);
> > > > > + } else {
> > > > > + if (sv2 != 0) {
> > > > > + tmu_write(qdata, sv2, &qdata->regv2-
> > > > > >tmsr);
> > > > > + tmu_write(qdata, TMR_ME | TMR_ALPF_V2,
> > > > > + &qdata->regv2->tmr);
> > > > > + }
> > > > > + }
> > > > >
> > > > > return 0;
> > > > > }
> > > > > @@ -148,16 +206,20 @@ static int qoriq_tmu_calibration(struct
> > > > > platform_device *pdev)
> > > > > struct device_node *np = pdev->dev.of_node;
> > > > > struct qoriq_tmu_data *data =
> > > > > platform_get_drvdata(pdev);
> > > > >
> > > > > - if (of_property_read_u32_array(np, "fsl,tmu-range",
> > > > > range, 4)) {
> > > > > - dev_err(&pdev->dev, "missing calibration
> > > > > range.\n");
> > > > > - return -ENODEV;
> > > > > + len = of_property_count_u32_elems(np, "fsl,tmu-range");
> > > > > + if (len == -ENODATA || len == -EINVAL || len > 4) {
Can we just check the "len < 0 || len > 4" ?
> > > > > + dev_err(&pdev->dev, "invalid range data.\n");
> > > > > + return len;
> > > > > }
> > > > >
> > > > > - /* Init temperature range registers */
> > > > > - tmu_write(data, range[0], &data->regs->ttr0cr);
> > > > > - tmu_write(data, range[1], &data->regs->ttr1cr);
> > > > > - tmu_write(data, range[2], &data->regs->ttr2cr);
> > > > > - tmu_write(data, range[3], &data->regs->ttr3cr);
> > > > > + val = of_property_read_u32_array(np, "fsl,tmu-range",
> > > > > range, len);
> > > > > + if (val != 0) {
> > > > > + dev_err(&pdev->dev, "invalid range data.\n");
The error log is same as failure of getting range data count, so would it be better
to change the log to "failed to read range data" here?
> > > > > + return val;
> > > > > + }
> > > > > +
> > > > > + for (i = 0; i < len; i++)
> > > > > + tmu_write(data, range[i], &data->regs-
> > > > > >ttrcr[i]);
> > > > >
> > > > > calibration = of_get_property(np, "fsl,tmu- calibration",
> > > > > &len);
> > > > > if (calibration == NULL || len % 8) { @@ -181,7 +243,12 @@
> > > > > static void qoriq_tmu_init_device(struct qoriq_tmu_data *data)
> > > > > tmu_write(data, TIER_DISABLE, &data->regs->tier);
> > > > >
> > > > > /* Set update_interval */
> > > > > - tmu_write(data, TMTMIR_DEFAULT, &data->regs->tmtmir);
> > > > > + if (data->ver == TMU_VER1) {
> > > > > + tmu_write(data, TMTMIR_DEFAULT, &data->regs-
> > > > > >tmtmir);
> > > > > + } else {
> > > > > + tmu_write(data, TMTMIR_DEFAULT, &data->regv2-
> > > > > >tmtmir);
> > > > > + tmu_write(data, TEUMR0_V2, &data->regv2-
> > > > > >teumr0);
> > > > > + }
> > > > >
> > > > > /* Disable monitoring */
> > > > > tmu_write(data, TMR_DISABLE, &data->regs->tmr); @@
> > > > > -190,6
> > >
> > > +257,7
> > > > @@
> > > > > static void qoriq_tmu_init_device(struct qoriq_tmu_data *data)
> > > > > static int qoriq_tmu_probe(struct platform_device *pdev) {
> > > > > int ret;
> > > > > + u32 ver;
> > > > > struct qoriq_tmu_data *data;
> > > > > struct device_node *np = pdev->dev.of_node;
> > > > >
> > > > > @@ -214,6 +282,12 @@ static int qoriq_tmu_probe(struct
> > > >
> > > > platform_device
> > > > > *pdev)
> > > > > goto err_iomap;
> > > > > }
> > > > >
> > > > > + /* version register offset at: 0xbf8 on both v1 and v2
> > > > > */
> > > > > + ver = tmu_read(data, &data->regs->ipbrr0);
> > > > > + data->ver = (ver >> 8) & 0xff;
> > > > > + if (data->ver == TMU_VER2)
> > > > > + data->regv2 = (void __iomem *)data->regs;
So the V2 has same register base address as V1? If yes, then I think no
need to check the version here, just assign V2's reg to equal to V1's should be OK?
Anson
> > > > > +
> > > > > qoriq_tmu_init_device(data); /* TMU initialization
> > > > > */
> > > > >
> > > > > ret = qoriq_tmu_calibration(pdev); /* TMU
> > > > > calibration */
> > > > > --
> > > > > 2.17.1
> >
> >