* [PATCH v3 01/13] thermal: qoriq: Remove unnecessary DT node is NULL check
2019-04-01 4:14 [PATCH v3 00/13] QorIQ TMU multi-sensor and HWMON support Andrey Smirnov
@ 2019-04-01 4:14 ` Andrey Smirnov
2019-04-04 3:21 ` Daniel Lezcano
2019-04-01 4:14 ` [PATCH v3 02/13] thermal: qoriq: Add local struct device pointer Andrey Smirnov
` (11 subsequent siblings)
12 siblings, 1 reply; 42+ messages in thread
From: Andrey Smirnov @ 2019-04-01 4:14 UTC (permalink / raw)
To: linux-pm
Cc: Andrey Smirnov, Chris Healy, Lucas Stach, Zhang Rui,
Eduardo Valentin, Daniel Lezcano, Angus Ainslie, linux-imx,
linux-kernel
This driver is meant to be used with Device Tree and there's no
use-case where device's DT node is going to be NULL. Remove code
protecting against that.
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Angus Ainslie (Purism) <angus@akkea.ca>
Cc: linux-imx@nxp.com
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
drivers/thermal/qoriq_thermal.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 3b5f5b3fb1bc..7b364933bfb1 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -193,11 +193,6 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
struct qoriq_tmu_data *data;
struct device_node *np = pdev->dev.of_node;
- if (!np) {
- dev_err(&pdev->dev, "Device OF-Node is NULL");
- return -ENODEV;
- }
-
data = devm_kzalloc(&pdev->dev, sizeof(struct qoriq_tmu_data),
GFP_KERNEL);
if (!data)
--
2.20.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v3 01/13] thermal: qoriq: Remove unnecessary DT node is NULL check
2019-04-01 4:14 ` [PATCH v3 01/13] thermal: qoriq: Remove unnecessary DT node is NULL check Andrey Smirnov
@ 2019-04-04 3:21 ` Daniel Lezcano
2019-04-05 17:51 ` Andrey Smirnov
0 siblings, 1 reply; 42+ messages in thread
From: Daniel Lezcano @ 2019-04-04 3:21 UTC (permalink / raw)
To: Andrey Smirnov, linux-pm
Cc: Chris Healy, Lucas Stach, Zhang Rui, Eduardo Valentin,
Angus Ainslie, linux-imx, linux-kernel
On 01/04/2019 06:14, Andrey Smirnov wrote:
> This driver is meant to be used with Device Tree and there's no
> use-case where device's DT node is going to be NULL. Remove code
> protecting against that.
May be elaborate why is never going to be NULL?
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> Cc: Chris Healy <cphealy@gmail.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Zhang Rui <rui.zhang@intel.com>
> Cc: Eduardo Valentin <edubezval@gmail.com>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Angus Ainslie (Purism) <angus@akkea.ca>
> Cc: linux-imx@nxp.com
> Cc: linux-pm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
> drivers/thermal/qoriq_thermal.c | 5 -----
> 1 file changed, 5 deletions(-)
>
> diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
> index 3b5f5b3fb1bc..7b364933bfb1 100644
> --- a/drivers/thermal/qoriq_thermal.c
> +++ b/drivers/thermal/qoriq_thermal.c
> @@ -193,11 +193,6 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
> struct qoriq_tmu_data *data;
> struct device_node *np = pdev->dev.of_node;
>
> - if (!np) {
> - dev_err(&pdev->dev, "Device OF-Node is NULL");
> - return -ENODEV;
> - }
> -
> data = devm_kzalloc(&pdev->dev, sizeof(struct qoriq_tmu_data),
> GFP_KERNEL);
> if (!data)
>
--
<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
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH v3 01/13] thermal: qoriq: Remove unnecessary DT node is NULL check
2019-04-04 3:21 ` Daniel Lezcano
@ 2019-04-05 17:51 ` Andrey Smirnov
2019-04-11 16:52 ` Daniel Lezcano
0 siblings, 1 reply; 42+ messages in thread
From: Andrey Smirnov @ 2019-04-05 17:51 UTC (permalink / raw)
To: Daniel Lezcano
Cc: linux-pm, Chris Healy, Lucas Stach, Zhang Rui, Eduardo Valentin,
Angus Ainslie, dl-linux-imx, linux-kernel
On Wed, Apr 3, 2019 at 8:21 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>
> On 01/04/2019 06:14, Andrey Smirnov wrote:
> > This driver is meant to be used with Device Tree and there's no
> > use-case where device's DT node is going to be NULL. Remove code
> > protecting against that.
>
> May be elaborate why is never going to be NULL?
>
Hmm, I am not sure what can be elaborated further than what's already
there. The driver is written to be instantiated via DT and there's no
code that tries to do that via board code or anything like that. I am
guessing you maybe read the description differently. Can you help me
by giving an example of what you think needs clarifying?
Thanks,
Andrey Smirnov
> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> > Cc: Chris Healy <cphealy@gmail.com>
> > Cc: Lucas Stach <l.stach@pengutronix.de>
> > Cc: Zhang Rui <rui.zhang@intel.com>
> > Cc: Eduardo Valentin <edubezval@gmail.com>
> > Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> > Cc: Angus Ainslie (Purism) <angus@akkea.ca>
> > Cc: linux-imx@nxp.com
> > Cc: linux-pm@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
>
> Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>
> > ---
> > drivers/thermal/qoriq_thermal.c | 5 -----
> > 1 file changed, 5 deletions(-)
> >
> > diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
> > index 3b5f5b3fb1bc..7b364933bfb1 100644
> > --- a/drivers/thermal/qoriq_thermal.c
> > +++ b/drivers/thermal/qoriq_thermal.c
> > @@ -193,11 +193,6 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
> > struct qoriq_tmu_data *data;
> > struct device_node *np = pdev->dev.of_node;
> >
> > - if (!np) {
> > - dev_err(&pdev->dev, "Device OF-Node is NULL");
> > - return -ENODEV;
> > - }
> > -
> > data = devm_kzalloc(&pdev->dev, sizeof(struct qoriq_tmu_data),
> > GFP_KERNEL);
> > if (!data)
> >
>
>
> --
> <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
>
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH v3 01/13] thermal: qoriq: Remove unnecessary DT node is NULL check
2019-04-05 17:51 ` Andrey Smirnov
@ 2019-04-11 16:52 ` Daniel Lezcano
0 siblings, 0 replies; 42+ messages in thread
From: Daniel Lezcano @ 2019-04-11 16:52 UTC (permalink / raw)
To: Andrey Smirnov
Cc: linux-pm, Chris Healy, Lucas Stach, Zhang Rui, Eduardo Valentin,
Angus Ainslie, dl-linux-imx, linux-kernel
On 05/04/2019 19:51, Andrey Smirnov wrote:
> On Wed, Apr 3, 2019 at 8:21 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>>
>> On 01/04/2019 06:14, Andrey Smirnov wrote:
>>> This driver is meant to be used with Device Tree and there's no
>>> use-case where device's DT node is going to be NULL. Remove code
>>> protecting against that.
>>
>> May be elaborate why is never going to be NULL?
>>
>
> Hmm, I am not sure what can be elaborated further than what's already
> there. The driver is written to be instantiated via DT and there's no
> code that tries to do that via board code or anything like that. I am
> guessing you maybe read the description differently. Can you help me
> by giving an example of what you think needs clarifying?
May be just say if the probe function is called, the dev.of_node is
guarantee to be filled by the underlying framework because the
compatible string was found so the check is pointless.
Anyway, it is a detail.
>>> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
>>> Cc: Chris Healy <cphealy@gmail.com>
>>> Cc: Lucas Stach <l.stach@pengutronix.de>
>>> Cc: Zhang Rui <rui.zhang@intel.com>
>>> Cc: Eduardo Valentin <edubezval@gmail.com>
>>> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
>>> Cc: Angus Ainslie (Purism) <angus@akkea.ca>
>>> Cc: linux-imx@nxp.com
>>> Cc: linux-pm@vger.kernel.org
>>> Cc: linux-kernel@vger.kernel.org
>>
>> Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>>
>>> ---
>>> drivers/thermal/qoriq_thermal.c | 5 -----
>>> 1 file changed, 5 deletions(-)
>>>
>>> diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
>>> index 3b5f5b3fb1bc..7b364933bfb1 100644
>>> --- a/drivers/thermal/qoriq_thermal.c
>>> +++ b/drivers/thermal/qoriq_thermal.c
>>> @@ -193,11 +193,6 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
>>> struct qoriq_tmu_data *data;
>>> struct device_node *np = pdev->dev.of_node;
>>>
>>> - if (!np) {
>>> - dev_err(&pdev->dev, "Device OF-Node is NULL");
>>> - return -ENODEV;
>>> - }
>>> -
>>> data = devm_kzalloc(&pdev->dev, sizeof(struct qoriq_tmu_data),
>>> GFP_KERNEL);
>>> if (!data)
>>>
>>
>>
>> --
>> <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
>>
--
<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
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v3 02/13] thermal: qoriq: Add local struct device pointer
2019-04-01 4:14 [PATCH v3 00/13] QorIQ TMU multi-sensor and HWMON support Andrey Smirnov
2019-04-01 4:14 ` [PATCH v3 01/13] thermal: qoriq: Remove unnecessary DT node is NULL check Andrey Smirnov
@ 2019-04-01 4:14 ` Andrey Smirnov
2019-04-04 3:11 ` Daniel Lezcano
2019-04-01 4:14 ` [PATCH v3 03/13] thermal: qoriq: Don't store struct thermal_zone_device reference Andrey Smirnov
` (10 subsequent siblings)
12 siblings, 1 reply; 42+ messages in thread
From: Andrey Smirnov @ 2019-04-01 4:14 UTC (permalink / raw)
To: linux-pm
Cc: Andrey Smirnov, Chris Healy, Lucas Stach, Zhang Rui,
Eduardo Valentin, Daniel Lezcano, Angus Ainslie, linux-imx,
linux-kernel
Use a local "struct device *dev" for brevity. No functional change
intended.
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Angus Ainslie (Purism) <angus@akkea.ca>
Cc: linux-imx@nxp.com
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
drivers/thermal/qoriq_thermal.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 7b364933bfb1..91f9f49d2776 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -192,8 +192,9 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
int ret;
struct qoriq_tmu_data *data;
struct device_node *np = pdev->dev.of_node;
+ struct device *dev = &pdev->dev;
- data = devm_kzalloc(&pdev->dev, sizeof(struct qoriq_tmu_data),
+ data = devm_kzalloc(dev, sizeof(struct qoriq_tmu_data),
GFP_KERNEL);
if (!data)
return -ENOMEM;
@@ -204,7 +205,7 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
data->regs = of_iomap(np, 0);
if (!data->regs) {
- dev_err(&pdev->dev, "Failed to get memory region\n");
+ dev_err(dev, "Failed to get memory region\n");
ret = -ENODEV;
goto err_iomap;
}
@@ -217,7 +218,7 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
ret = qoriq_tmu_register_tmu_zone(pdev);
if (ret < 0) {
- dev_err(&pdev->dev, "Failed to register sensors\n");
+ dev_err(dev, "Failed to register sensors\n");
ret = -ENODEV;
goto err_iomap;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v3 02/13] thermal: qoriq: Add local struct device pointer
2019-04-01 4:14 ` [PATCH v3 02/13] thermal: qoriq: Add local struct device pointer Andrey Smirnov
@ 2019-04-04 3:11 ` Daniel Lezcano
0 siblings, 0 replies; 42+ messages in thread
From: Daniel Lezcano @ 2019-04-04 3:11 UTC (permalink / raw)
To: Andrey Smirnov, linux-pm
Cc: Chris Healy, Lucas Stach, Zhang Rui, Eduardo Valentin,
Angus Ainslie, linux-imx, linux-kernel
On 01/04/2019 06:14, Andrey Smirnov wrote:
> Use a local "struct device *dev" for brevity. No functional change
> intended.
>
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> Cc: Chris Healy <cphealy@gmail.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Zhang Rui <rui.zhang@intel.com>
> Cc: Eduardo Valentin <edubezval@gmail.com>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Angus Ainslie (Purism) <angus@akkea.ca>
> Cc: linux-imx@nxp.com
> Cc: linux-pm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> drivers/thermal/qoriq_thermal.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
> index 7b364933bfb1..91f9f49d2776 100644
> --- a/drivers/thermal/qoriq_thermal.c
> +++ b/drivers/thermal/qoriq_thermal.c
> @@ -192,8 +192,9 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
> int ret;
> struct qoriq_tmu_data *data;
> struct device_node *np = pdev->dev.of_node;
> + struct device *dev = &pdev->dev;
>
> - data = devm_kzalloc(&pdev->dev, sizeof(struct qoriq_tmu_data),
> + data = devm_kzalloc(dev, sizeof(struct qoriq_tmu_data),
> GFP_KERNEL);
> if (!data)
> return -ENOMEM;
> @@ -204,7 +205,7 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
>
> data->regs = of_iomap(np, 0);
> if (!data->regs) {
> - dev_err(&pdev->dev, "Failed to get memory region\n");
> + dev_err(dev, "Failed to get memory region\n");
> ret = -ENODEV;
> goto err_iomap;
> }
> @@ -217,7 +218,7 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
>
> ret = qoriq_tmu_register_tmu_zone(pdev);
> if (ret < 0) {
> - dev_err(&pdev->dev, "Failed to register sensors\n");
> + dev_err(dev, "Failed to register sensors\n");
> ret = -ENODEV;
> goto err_iomap;
> }
>
--
<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
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v3 03/13] thermal: qoriq: Don't store struct thermal_zone_device reference
2019-04-01 4:14 [PATCH v3 00/13] QorIQ TMU multi-sensor and HWMON support Andrey Smirnov
2019-04-01 4:14 ` [PATCH v3 01/13] thermal: qoriq: Remove unnecessary DT node is NULL check Andrey Smirnov
2019-04-01 4:14 ` [PATCH v3 02/13] thermal: qoriq: Add local struct device pointer Andrey Smirnov
@ 2019-04-01 4:14 ` Andrey Smirnov
2019-04-04 3:23 ` Daniel Lezcano
2019-04-01 4:14 ` [PATCH v3 04/13] thermal: qoriq: Add local struct qoriq_sensor pointer Andrey Smirnov
` (9 subsequent siblings)
12 siblings, 1 reply; 42+ messages in thread
From: Andrey Smirnov @ 2019-04-01 4:14 UTC (permalink / raw)
To: linux-pm
Cc: Andrey Smirnov, Chris Healy, Lucas Stach, Zhang Rui,
Eduardo Valentin, Daniel Lezcano, Angus Ainslie, linux-imx,
linux-kernel
Struct thermal_zone_device reference stored as sensor's private data
isn't really used anywhere in the code. Drop it.
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Angus Ainslie (Purism) <angus@akkea.ca>
Cc: linux-imx@nxp.com
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
drivers/thermal/qoriq_thermal.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 91f9f49d2776..6d40b9788266 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -65,7 +65,6 @@ struct qoriq_tmu_data;
* Thermal zone data
*/
struct qoriq_sensor {
- struct thermal_zone_device *tzd;
struct qoriq_tmu_data *qdata;
int id;
};
@@ -114,6 +113,8 @@ static int qoriq_tmu_register_tmu_zone(struct platform_device *pdev)
int id, sites = 0;
for (id = 0; id < SITES_MAX; id++) {
+ struct thermal_zone_device *tzd;
+
qdata->sensor[id] = devm_kzalloc(&pdev->dev,
sizeof(struct qoriq_sensor), GFP_KERNEL);
if (!qdata->sensor[id])
@@ -121,13 +122,15 @@ static int qoriq_tmu_register_tmu_zone(struct platform_device *pdev)
qdata->sensor[id]->id = id;
qdata->sensor[id]->qdata = qdata;
- qdata->sensor[id]->tzd = devm_thermal_zone_of_sensor_register(
- &pdev->dev, id, qdata->sensor[id], &tmu_tz_ops);
- if (IS_ERR(qdata->sensor[id]->tzd)) {
- if (PTR_ERR(qdata->sensor[id]->tzd) == -ENODEV)
+
+ tzd = devm_thermal_zone_of_sensor_register(&pdev->dev, id,
+ qdata->sensor[id],
+ &tmu_tz_ops);
+ if (IS_ERR(tzd)) {
+ if (PTR_ERR(tzd) == -ENODEV)
continue;
else
- return PTR_ERR(qdata->sensor[id]->tzd);
+ return PTR_ERR(tzd);
}
sites |= 0x1 << (15 - id);
--
2.20.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v3 03/13] thermal: qoriq: Don't store struct thermal_zone_device reference
2019-04-01 4:14 ` [PATCH v3 03/13] thermal: qoriq: Don't store struct thermal_zone_device reference Andrey Smirnov
@ 2019-04-04 3:23 ` Daniel Lezcano
0 siblings, 0 replies; 42+ messages in thread
From: Daniel Lezcano @ 2019-04-04 3:23 UTC (permalink / raw)
To: Andrey Smirnov, linux-pm
Cc: Chris Healy, Lucas Stach, Zhang Rui, Eduardo Valentin,
Angus Ainslie, linux-imx, linux-kernel
On 01/04/2019 06:14, Andrey Smirnov wrote:
> Struct thermal_zone_device reference stored as sensor's private data
> isn't really used anywhere in the code. Drop it.
>
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> Cc: Chris Healy <cphealy@gmail.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Zhang Rui <rui.zhang@intel.com>
> Cc: Eduardo Valentin <edubezval@gmail.com>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Angus Ainslie (Purism) <angus@akkea.ca>
> Cc: linux-imx@nxp.com
> Cc: linux-pm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
> drivers/thermal/qoriq_thermal.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
> index 91f9f49d2776..6d40b9788266 100644
> --- a/drivers/thermal/qoriq_thermal.c
> +++ b/drivers/thermal/qoriq_thermal.c
> @@ -65,7 +65,6 @@ struct qoriq_tmu_data;
> * Thermal zone data
> */
> struct qoriq_sensor {
> - struct thermal_zone_device *tzd;
> struct qoriq_tmu_data *qdata;
> int id;
> };
> @@ -114,6 +113,8 @@ static int qoriq_tmu_register_tmu_zone(struct platform_device *pdev)
> int id, sites = 0;
>
> for (id = 0; id < SITES_MAX; id++) {
> + struct thermal_zone_device *tzd;
> +
> qdata->sensor[id] = devm_kzalloc(&pdev->dev,
> sizeof(struct qoriq_sensor), GFP_KERNEL);
> if (!qdata->sensor[id])
> @@ -121,13 +122,15 @@ static int qoriq_tmu_register_tmu_zone(struct platform_device *pdev)
>
> qdata->sensor[id]->id = id;
> qdata->sensor[id]->qdata = qdata;
> - qdata->sensor[id]->tzd = devm_thermal_zone_of_sensor_register(
> - &pdev->dev, id, qdata->sensor[id], &tmu_tz_ops);
> - if (IS_ERR(qdata->sensor[id]->tzd)) {
> - if (PTR_ERR(qdata->sensor[id]->tzd) == -ENODEV)
> +
> + tzd = devm_thermal_zone_of_sensor_register(&pdev->dev, id,
> + qdata->sensor[id],
> + &tmu_tz_ops);
> + if (IS_ERR(tzd)) {
> + if (PTR_ERR(tzd) == -ENODEV)
> continue;
> else
> - return PTR_ERR(qdata->sensor[id]->tzd);
> + return PTR_ERR(tzd);
> }
>
> sites |= 0x1 << (15 - id);
>
--
<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
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v3 04/13] thermal: qoriq: Add local struct qoriq_sensor pointer
2019-04-01 4:14 [PATCH v3 00/13] QorIQ TMU multi-sensor and HWMON support Andrey Smirnov
` (2 preceding siblings ...)
2019-04-01 4:14 ` [PATCH v3 03/13] thermal: qoriq: Don't store struct thermal_zone_device reference Andrey Smirnov
@ 2019-04-01 4:14 ` Andrey Smirnov
2019-04-04 3:28 ` Daniel Lezcano
2019-04-01 4:14 ` [PATCH v3 05/13] thermal: qoriq: Embed per-sensor data into struct qoriq_tmu_data Andrey Smirnov
` (8 subsequent siblings)
12 siblings, 1 reply; 42+ messages in thread
From: Andrey Smirnov @ 2019-04-01 4:14 UTC (permalink / raw)
To: linux-pm
Cc: Andrey Smirnov, Chris Healy, Lucas Stach, Zhang Rui,
Eduardo Valentin, Daniel Lezcano, Angus Ainslie, linux-imx,
linux-kernel
Add local struct qoriq_sensor pointer in qoriq_tmu_register_tmu_zone()
for brevity.
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Angus Ainslie (Purism) <angus@akkea.ca>
Cc: linux-imx@nxp.com
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
drivers/thermal/qoriq_thermal.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 6d40b9788266..e281bdcfa11f 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -114,18 +114,18 @@ static int qoriq_tmu_register_tmu_zone(struct platform_device *pdev)
for (id = 0; id < SITES_MAX; id++) {
struct thermal_zone_device *tzd;
+ struct qoriq_sensor *s;
- qdata->sensor[id] = devm_kzalloc(&pdev->dev,
+ s = qdata->sensor[id] = devm_kzalloc(&pdev->dev,
sizeof(struct qoriq_sensor), GFP_KERNEL);
if (!qdata->sensor[id])
return -ENOMEM;
- qdata->sensor[id]->id = id;
- qdata->sensor[id]->qdata = qdata;
+ s->id = id;
+ s->qdata = qdata;
tzd = devm_thermal_zone_of_sensor_register(&pdev->dev, id,
- qdata->sensor[id],
- &tmu_tz_ops);
+ s, &tmu_tz_ops);
if (IS_ERR(tzd)) {
if (PTR_ERR(tzd) == -ENODEV)
continue;
--
2.20.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v3 04/13] thermal: qoriq: Add local struct qoriq_sensor pointer
2019-04-01 4:14 ` [PATCH v3 04/13] thermal: qoriq: Add local struct qoriq_sensor pointer Andrey Smirnov
@ 2019-04-04 3:28 ` Daniel Lezcano
2019-04-05 17:57 ` Andrey Smirnov
0 siblings, 1 reply; 42+ messages in thread
From: Daniel Lezcano @ 2019-04-04 3:28 UTC (permalink / raw)
To: Andrey Smirnov, linux-pm
Cc: Chris Healy, Lucas Stach, Zhang Rui, Eduardo Valentin,
Angus Ainslie, linux-imx, linux-kernel
On 01/04/2019 06:14, Andrey Smirnov wrote:
> Add local struct qoriq_sensor pointer in qoriq_tmu_register_tmu_zone()
> for brevity.
>
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> Cc: Chris Healy <cphealy@gmail.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Zhang Rui <rui.zhang@intel.com>
> Cc: Eduardo Valentin <edubezval@gmail.com>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Angus Ainslie (Purism) <angus@akkea.ca>
> Cc: linux-imx@nxp.com
> Cc: linux-pm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
> drivers/thermal/qoriq_thermal.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
> index 6d40b9788266..e281bdcfa11f 100644
> --- a/drivers/thermal/qoriq_thermal.c
> +++ b/drivers/thermal/qoriq_thermal.c
> @@ -114,18 +114,18 @@ static int qoriq_tmu_register_tmu_zone(struct platform_device *pdev)
>
> for (id = 0; id < SITES_MAX; id++) {
> struct thermal_zone_device *tzd;
> + struct qoriq_sensor *s;
>
> - qdata->sensor[id] = devm_kzalloc(&pdev->dev,
> + s = qdata->sensor[id] = devm_kzalloc(&pdev->dev,
> sizeof(struct qoriq_sensor), GFP_KERNEL);
I would not recommend this, especially if you use a variable helper for
clarity. Keep using the 's' variable and then assign qdata->sensor[id] =
s at the end when everything is ok. May be rename it 'sensor'?
> if (!qdata->sensor[id])
> return -ENOMEM;
>
> - qdata->sensor[id]->id = id;
> - qdata->sensor[id]->qdata = qdata;
> + s->id = id;
> + s->qdata = qdata;
>
> tzd = devm_thermal_zone_of_sensor_register(&pdev->dev, id,
> - qdata->sensor[id],
> - &tmu_tz_ops);
> + s, &tmu_tz_ops);
> if (IS_ERR(tzd)) {
> if (PTR_ERR(tzd) == -ENODEV)
> continue;
>
--
<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
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH v3 04/13] thermal: qoriq: Add local struct qoriq_sensor pointer
2019-04-04 3:28 ` Daniel Lezcano
@ 2019-04-05 17:57 ` Andrey Smirnov
0 siblings, 0 replies; 42+ messages in thread
From: Andrey Smirnov @ 2019-04-05 17:57 UTC (permalink / raw)
To: Daniel Lezcano
Cc: linux-pm, Chris Healy, Lucas Stach, Zhang Rui, Eduardo Valentin,
Angus Ainslie, dl-linux-imx, linux-kernel
On Wed, Apr 3, 2019 at 8:28 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>
> On 01/04/2019 06:14, Andrey Smirnov wrote:
> > Add local struct qoriq_sensor pointer in qoriq_tmu_register_tmu_zone()
> > for brevity.
> >
> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> > Cc: Chris Healy <cphealy@gmail.com>
> > Cc: Lucas Stach <l.stach@pengutronix.de>
> > Cc: Zhang Rui <rui.zhang@intel.com>
> > Cc: Eduardo Valentin <edubezval@gmail.com>
> > Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> > Cc: Angus Ainslie (Purism) <angus@akkea.ca>
> > Cc: linux-imx@nxp.com
> > Cc: linux-pm@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > ---
> > drivers/thermal/qoriq_thermal.c | 10 +++++-----
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
> > index 6d40b9788266..e281bdcfa11f 100644
> > --- a/drivers/thermal/qoriq_thermal.c
> > +++ b/drivers/thermal/qoriq_thermal.c
> > @@ -114,18 +114,18 @@ static int qoriq_tmu_register_tmu_zone(struct platform_device *pdev)
> >
> > for (id = 0; id < SITES_MAX; id++) {
> > struct thermal_zone_device *tzd;
> > + struct qoriq_sensor *s;
> >
> > - qdata->sensor[id] = devm_kzalloc(&pdev->dev,
> > + s = qdata->sensor[id] = devm_kzalloc(&pdev->dev,
> > sizeof(struct qoriq_sensor), GFP_KERNEL);
>
> I would not recommend this, especially if you use a variable helper for
> clarity. Keep using the 's' variable and then assign qdata->sensor[id] =
> s at the end when everything is ok. May be rename it 'sensor'?
>
Sure, I'll move the assignment in v4.
Thanks,
Andrey Smirnov
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v3 05/13] thermal: qoriq: Embed per-sensor data into struct qoriq_tmu_data
2019-04-01 4:14 [PATCH v3 00/13] QorIQ TMU multi-sensor and HWMON support Andrey Smirnov
` (3 preceding siblings ...)
2019-04-01 4:14 ` [PATCH v3 04/13] thermal: qoriq: Add local struct qoriq_sensor pointer Andrey Smirnov
@ 2019-04-01 4:14 ` Andrey Smirnov
2019-04-04 7:57 ` Daniel Lezcano
2019-04-01 4:14 ` [PATCH v3 06/13] thermal: qoriq: Pass data to qoriq_tmu_register_tmu_zone() directly Andrey Smirnov
` (7 subsequent siblings)
12 siblings, 1 reply; 42+ messages in thread
From: Andrey Smirnov @ 2019-04-01 4:14 UTC (permalink / raw)
To: linux-pm
Cc: Andrey Smirnov, Chris Healy, Lucas Stach, Zhang Rui,
Eduardo Valentin, Daniel Lezcano, Angus Ainslie, linux-imx,
linux-kernel
Embed per-sensor data into struct qoriq_tmu_data so we can drop the
code allocating it. This also allows us to get rid of per-sensor back
reference to struct qoriq_tmu_data since now its address can be
caluclated using container_of().
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Angus Ainslie (Purism) <angus@akkea.ca>
Cc: linux-imx@nxp.com
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
drivers/thermal/qoriq_thermal.c | 20 ++++++++------------
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index e281bdcfa11f..deb5cb6a0baf 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -59,22 +59,24 @@ struct qoriq_tmu_regs {
u32 ttr3cr; /* Temperature Range 3 Control Register */
};
-struct qoriq_tmu_data;
-
/*
* Thermal zone data
*/
struct qoriq_sensor {
- struct qoriq_tmu_data *qdata;
int id;
};
struct qoriq_tmu_data {
struct qoriq_tmu_regs __iomem *regs;
bool little_endian;
- struct qoriq_sensor *sensor[SITES_MAX];
+ struct qoriq_sensor sensor[SITES_MAX];
};
+static struct qoriq_tmu_data *qoriq_sensor_to_data(struct qoriq_sensor *s)
+{
+ return container_of(s, struct qoriq_tmu_data, sensor[s->id]);
+}
+
static void tmu_write(struct qoriq_tmu_data *p, u32 val, void __iomem *addr)
{
if (p->little_endian)
@@ -94,7 +96,7 @@ static u32 tmu_read(struct qoriq_tmu_data *p, void __iomem *addr)
static int tmu_get_temp(void *p, int *temp)
{
struct qoriq_sensor *qsensor = p;
- struct qoriq_tmu_data *qdata = qsensor->qdata;
+ struct qoriq_tmu_data *qdata = qoriq_sensor_to_data(qsensor);
u32 val;
val = tmu_read(qdata, &qdata->regs->site[qsensor->id].tritsr);
@@ -114,15 +116,9 @@ static int qoriq_tmu_register_tmu_zone(struct platform_device *pdev)
for (id = 0; id < SITES_MAX; id++) {
struct thermal_zone_device *tzd;
- struct qoriq_sensor *s;
-
- s = qdata->sensor[id] = devm_kzalloc(&pdev->dev,
- sizeof(struct qoriq_sensor), GFP_KERNEL);
- if (!qdata->sensor[id])
- return -ENOMEM;
+ struct qoriq_sensor *s = &qdata->sensor[id];
s->id = id;
- s->qdata = qdata;
tzd = devm_thermal_zone_of_sensor_register(&pdev->dev, id,
s, &tmu_tz_ops);
--
2.20.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v3 05/13] thermal: qoriq: Embed per-sensor data into struct qoriq_tmu_data
2019-04-01 4:14 ` [PATCH v3 05/13] thermal: qoriq: Embed per-sensor data into struct qoriq_tmu_data Andrey Smirnov
@ 2019-04-04 7:57 ` Daniel Lezcano
2019-04-04 8:06 ` [PATCH RFC 1/2] thermal/drivers/of: Add a get_temp_id callback function Daniel Lezcano
2019-04-05 18:00 ` [PATCH v3 05/13] thermal: qoriq: Embed per-sensor data into struct qoriq_tmu_data Andrey Smirnov
0 siblings, 2 replies; 42+ messages in thread
From: Daniel Lezcano @ 2019-04-04 7:57 UTC (permalink / raw)
To: Andrey Smirnov, linux-pm
Cc: Chris Healy, Lucas Stach, Zhang Rui, Eduardo Valentin,
Angus Ainslie, linux-imx, linux-kernel
On 01/04/2019 06:14, Andrey Smirnov wrote:
> Embed per-sensor data into struct qoriq_tmu_data so we can drop the
> code allocating it. This also allows us to get rid of per-sensor back
> reference to struct qoriq_tmu_data since now its address can be
> caluclated using container_of().
This seems to be a repeating pattern, drivers are forced to put a back
pointer in the thermal sensor structure to regain access to the
container structure in the get_temp callback.
It would make sense to pass the sensor id to the get_temp callback as we
register with the sensor id.
One comment below.
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> Cc: Chris Healy <cphealy@gmail.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Zhang Rui <rui.zhang@intel.com>
> Cc: Eduardo Valentin <edubezval@gmail.com>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Angus Ainslie (Purism) <angus@akkea.ca>
> Cc: linux-imx@nxp.com
> Cc: linux-pm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
> drivers/thermal/qoriq_thermal.c | 20 ++++++++------------
> 1 file changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
> index e281bdcfa11f..deb5cb6a0baf 100644
> --- a/drivers/thermal/qoriq_thermal.c
> +++ b/drivers/thermal/qoriq_thermal.c
> @@ -59,22 +59,24 @@ struct qoriq_tmu_regs {
> u32 ttr3cr; /* Temperature Range 3 Control Register */
> };
>
> -struct qoriq_tmu_data;
> -
> /*
> * Thermal zone data
> */
> struct qoriq_sensor {
> - struct qoriq_tmu_data *qdata;
> int id;
> };>
> struct qoriq_tmu_data {
> struct qoriq_tmu_regs __iomem *regs;
> bool little_endian;
Why not replace the little_endian boolean by a couple of callback
read/write and assign them to ioread32|ioread32be at init time.
That will kill the tmu_read and tmu_write functions and from there you
can figure out how to remove the qdata backpointer. In addition, it will
save a few instructions to test the boolean.
> - struct qoriq_sensor *sensor[SITES_MAX];
> + struct qoriq_sensor sensor[SITES_MAX];
>
> };
>
> +static struct qoriq_tmu_data *qoriq_sensor_to_data(struct qoriq_sensor *s)
> +{
> + return container_of(s, struct qoriq_tmu_data, sensor[s->id]);
> +}
> +
> static void tmu_write(struct qoriq_tmu_data *p, u32 val, void __iomem *addr)
> {
> if (p->little_endian)
> @@ -94,7 +96,7 @@ static u32 tmu_read(struct qoriq_tmu_data *p, void __iomem *addr)
> static int tmu_get_temp(void *p, int *temp)
> {
> struct qoriq_sensor *qsensor = p;
> - struct qoriq_tmu_data *qdata = qsensor->qdata;
> + struct qoriq_tmu_data *qdata = qoriq_sensor_to_data(qsensor);
> u32 val;
>
> val = tmu_read(qdata, &qdata->regs->site[qsensor->id].tritsr);
> @@ -114,15 +116,9 @@ static int qoriq_tmu_register_tmu_zone(struct platform_device *pdev)
>
> for (id = 0; id < SITES_MAX; id++) {
> struct thermal_zone_device *tzd;
> - struct qoriq_sensor *s;
> -
> - s = qdata->sensor[id] = devm_kzalloc(&pdev->dev,
> - sizeof(struct qoriq_sensor), GFP_KERNEL);
> - if (!qdata->sensor[id])
> - return -ENOMEM;
> + struct qoriq_sensor *s = &qdata->sensor[id];
>
> s->id = id;
> - s->qdata = qdata;
>
> tzd = devm_thermal_zone_of_sensor_register(&pdev->dev, id,
> s, &tmu_tz_ops);
>
--
<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
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH RFC 1/2] thermal/drivers/of: Add a get_temp_id callback function
2019-04-04 7:57 ` Daniel Lezcano
@ 2019-04-04 8:06 ` Daniel Lezcano
2019-04-04 8:06 ` [PATCH RFC 2/2] thermal/drivers/qoriq: Use the get_temp_id() Daniel Lezcano
2019-04-13 8:18 ` [PATCH RFC 1/2] thermal/drivers/of: Add a get_temp_id callback function Andrey Smirnov
2019-04-05 18:00 ` [PATCH v3 05/13] thermal: qoriq: Embed per-sensor data into struct qoriq_tmu_data Andrey Smirnov
1 sibling, 2 replies; 42+ messages in thread
From: Daniel Lezcano @ 2019-04-04 8:06 UTC (permalink / raw)
To: andrew.smirnov; +Cc: linux-pm, Zhang Rui, Eduardo Valentin, open list
Currently when we register a sensor, we specify the sensor id and a data
pointer to be passed when the get_temp function is called. However the
sensor_id is not passed to the get_temp callback forcing the driver to
do extra allocation and adding back pointer to find out from the sensor
information the driver data and then back to the sensor id.
Add a new callback get_temp_id() which will be called if set. It will
call the get_temp_id() with the sensor id.
That will be more consistent with the registering function.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
drivers/thermal/of-thermal.c | 19 +++++++++++++------
include/linux/thermal.h | 1 +
2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
index 2df059cc07e2..787d1cbe13f3 100644
--- a/drivers/thermal/of-thermal.c
+++ b/drivers/thermal/of-thermal.c
@@ -78,6 +78,8 @@ struct __thermal_zone {
/* sensor interface */
void *sensor_data;
+ int sensor_id;
+
const struct thermal_zone_of_device_ops *ops;
};
@@ -88,10 +90,14 @@ static int of_thermal_get_temp(struct thermal_zone_device *tz,
{
struct __thermal_zone *data = tz->devdata;
- if (!data->ops->get_temp)
- return -EINVAL;
+ if (data->ops->get_temp)
+ return data->ops->get_temp(data->sensor_data, temp);
- return data->ops->get_temp(data->sensor_data, temp);
+ if (data->ops->get_temp_id)
+ return data->ops->get_temp_id(data->sensor_id,
+ data->sensor_data, temp);
+
+ return -EINVAL;
}
static int of_thermal_set_trips(struct thermal_zone_device *tz,
@@ -407,8 +413,8 @@ static struct thermal_zone_device_ops of_thermal_ops = {
/*** sensor API ***/
static struct thermal_zone_device *
-thermal_zone_of_add_sensor(struct device_node *zone,
- struct device_node *sensor, void *data,
+thermal_zone_of_add_sensor(struct device_node *zone, struct device_node *sensor,
+ int sensor_id, void *data,
const struct thermal_zone_of_device_ops *ops)
{
struct thermal_zone_device *tzd;
@@ -426,6 +432,7 @@ thermal_zone_of_add_sensor(struct device_node *zone,
mutex_lock(&tzd->lock);
tz->ops = ops;
tz->sensor_data = data;
+ tz->sensor_id = sensor_id;
tzd->ops->get_temp = of_thermal_get_temp;
tzd->ops->get_trend = of_thermal_get_trend;
@@ -516,7 +523,7 @@ thermal_zone_of_sensor_register(struct device *dev, int sensor_id, void *data,
}
if (sensor_specs.np == sensor_np && id == sensor_id) {
- tzd = thermal_zone_of_add_sensor(child, sensor_np,
+ tzd = thermal_zone_of_add_sensor(child, sensor_np, sensor_id,
data, ops);
if (!IS_ERR(tzd))
tzd->ops->set_mode(tzd, THERMAL_DEVICE_ENABLED);
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 5f4705f46c2f..2762d7e6dd86 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -351,6 +351,7 @@ struct thermal_genl_event {
* hardware.
*/
struct thermal_zone_of_device_ops {
+ int (*get_temp_id)(int, void *, int *);
int (*get_temp)(void *, int *);
int (*get_trend)(void *, int, enum thermal_trend *);
int (*set_trips)(void *, int, int);
--
2.17.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH RFC 2/2] thermal/drivers/qoriq: Use the get_temp_id()
2019-04-04 8:06 ` [PATCH RFC 1/2] thermal/drivers/of: Add a get_temp_id callback function Daniel Lezcano
@ 2019-04-04 8:06 ` Daniel Lezcano
2019-04-13 8:18 ` [PATCH RFC 1/2] thermal/drivers/of: Add a get_temp_id callback function Andrey Smirnov
1 sibling, 0 replies; 42+ messages in thread
From: Daniel Lezcano @ 2019-04-04 8:06 UTC (permalink / raw)
To: andrew.smirnov; +Cc: linux-pm, Zhang Rui, Eduardo Valentin, open list
Currently the code is adding extra back pointer to find out the general
structure for the driver from the sensor data.
Use the get_temp_id() which pass the sensor id when registering the callback.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
drivers/thermal/qoriq_thermal.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 3b5f5b3fb1bc..405d1fbdd0cb 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -92,20 +92,20 @@ static u32 tmu_read(struct qoriq_tmu_data *p, void __iomem *addr)
return ioread32be(addr);
}
-static int tmu_get_temp(void *p, int *temp)
+static int tmu_get_temp(int id, void *p, int *temp)
{
struct qoriq_sensor *qsensor = p;
struct qoriq_tmu_data *qdata = qsensor->qdata;
u32 val;
- val = tmu_read(qdata, &qdata->regs->site[qsensor->id].tritsr);
+ val = tmu_read(qdata, &qdata->regs->site[id].tritsr);
*temp = (val & 0xff) * 1000;
return 0;
}
static const struct thermal_zone_of_device_ops tmu_tz_ops = {
- .get_temp = tmu_get_temp,
+ .get_temp_id = tmu_get_temp,
};
static int qoriq_tmu_register_tmu_zone(struct platform_device *pdev)
--
2.17.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH RFC 1/2] thermal/drivers/of: Add a get_temp_id callback function
2019-04-04 8:06 ` [PATCH RFC 1/2] thermal/drivers/of: Add a get_temp_id callback function Daniel Lezcano
2019-04-04 8:06 ` [PATCH RFC 2/2] thermal/drivers/qoriq: Use the get_temp_id() Daniel Lezcano
@ 2019-04-13 8:18 ` Andrey Smirnov
2019-04-13 8:28 ` Daniel Lezcano
1 sibling, 1 reply; 42+ messages in thread
From: Andrey Smirnov @ 2019-04-13 8:18 UTC (permalink / raw)
To: Daniel Lezcano; +Cc: linux-pm, Zhang Rui, Eduardo Valentin, open list
On Thu, Apr 4, 2019 at 1:07 AM Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>
> Currently when we register a sensor, we specify the sensor id and a data
> pointer to be passed when the get_temp function is called. However the
> sensor_id is not passed to the get_temp callback forcing the driver to
> do extra allocation and adding back pointer to find out from the sensor
> information the driver data and then back to the sensor id.
>
> Add a new callback get_temp_id() which will be called if set. It will
> call the get_temp_id() with the sensor id.
>
> That will be more consistent with the registering function.
>
Tested both patches on i.MX8MQ, seem to work as expected.
Tested-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Thanks,
Andrey Smirnov
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH RFC 1/2] thermal/drivers/of: Add a get_temp_id callback function
2019-04-13 8:18 ` [PATCH RFC 1/2] thermal/drivers/of: Add a get_temp_id callback function Andrey Smirnov
@ 2019-04-13 8:28 ` Daniel Lezcano
0 siblings, 0 replies; 42+ messages in thread
From: Daniel Lezcano @ 2019-04-13 8:28 UTC (permalink / raw)
To: Andrey Smirnov; +Cc: linux-pm, Zhang Rui, Eduardo Valentin, open list
On 13/04/2019 10:18, Andrey Smirnov wrote:
> On Thu, Apr 4, 2019 at 1:07 AM Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>>
>> Currently when we register a sensor, we specify the sensor id and a data
>> pointer to be passed when the get_temp function is called. However the
>> sensor_id is not passed to the get_temp callback forcing the driver to
>> do extra allocation and adding back pointer to find out from the sensor
>> information the driver data and then back to the sensor id.
>>
>> Add a new callback get_temp_id() which will be called if set. It will
>> call the get_temp_id() with the sensor id.
>>
>> That will be more consistent with the registering function.
>>
>
> Tested both patches on i.MX8MQ, seem to work as expected.
>
> Tested-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Great, thank you very much for testing!
--
<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
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 05/13] thermal: qoriq: Embed per-sensor data into struct qoriq_tmu_data
2019-04-04 7:57 ` Daniel Lezcano
2019-04-04 8:06 ` [PATCH RFC 1/2] thermal/drivers/of: Add a get_temp_id callback function Daniel Lezcano
@ 2019-04-05 18:00 ` Andrey Smirnov
2019-04-11 16:54 ` Daniel Lezcano
1 sibling, 1 reply; 42+ messages in thread
From: Andrey Smirnov @ 2019-04-05 18:00 UTC (permalink / raw)
To: Daniel Lezcano
Cc: linux-pm, Chris Healy, Lucas Stach, Zhang Rui, Eduardo Valentin,
Angus Ainslie, dl-linux-imx, linux-kernel
On Thu, Apr 4, 2019 at 12:57 AM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>
> On 01/04/2019 06:14, Andrey Smirnov wrote:
> > Embed per-sensor data into struct qoriq_tmu_data so we can drop the
> > code allocating it. This also allows us to get rid of per-sensor back
> > reference to struct qoriq_tmu_data since now its address can be
> > caluclated using container_of().
>
> This seems to be a repeating pattern, drivers are forced to put a back
> pointer in the thermal sensor structure to regain access to the
> container structure in the get_temp callback.
>
> It would make sense to pass the sensor id to the get_temp callback as we
> register with the sensor id.
>
I'll rebase this series on the two patches you submitted.
> One comment below.
>
> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> > Cc: Chris Healy <cphealy@gmail.com>
> > Cc: Lucas Stach <l.stach@pengutronix.de>
> > Cc: Zhang Rui <rui.zhang@intel.com>
> > Cc: Eduardo Valentin <edubezval@gmail.com>
> > Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> > Cc: Angus Ainslie (Purism) <angus@akkea.ca>
> > Cc: linux-imx@nxp.com
> > Cc: linux-pm@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > ---
> > drivers/thermal/qoriq_thermal.c | 20 ++++++++------------
> > 1 file changed, 8 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
> > index e281bdcfa11f..deb5cb6a0baf 100644
> > --- a/drivers/thermal/qoriq_thermal.c
> > +++ b/drivers/thermal/qoriq_thermal.c
> > @@ -59,22 +59,24 @@ struct qoriq_tmu_regs {
> > u32 ttr3cr; /* Temperature Range 3 Control Register */
> > };
> >
> > -struct qoriq_tmu_data;
> > -
> > /*
> > * Thermal zone data
> > */
> > struct qoriq_sensor {
> > - struct qoriq_tmu_data *qdata;
> > int id;
> > };>
> > struct qoriq_tmu_data {
> > struct qoriq_tmu_regs __iomem *regs;
> > bool little_endian;
>
> Why not replace the little_endian boolean by a couple of callback
> read/write and assign them to ioread32|ioread32be at init time.
>
> That will kill the tmu_read and tmu_write functions and from there you
> can figure out how to remove the qdata backpointer. In addition, it will
> save a few instructions to test the boolean.
>
I am sure you've seen it by now, but little_endian is going away in
this series in regmap conversion patch.
Thanks
Andrey Smirnov
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH v3 05/13] thermal: qoriq: Embed per-sensor data into struct qoriq_tmu_data
2019-04-05 18:00 ` [PATCH v3 05/13] thermal: qoriq: Embed per-sensor data into struct qoriq_tmu_data Andrey Smirnov
@ 2019-04-11 16:54 ` Daniel Lezcano
0 siblings, 0 replies; 42+ messages in thread
From: Daniel Lezcano @ 2019-04-11 16:54 UTC (permalink / raw)
To: Andrey Smirnov
Cc: linux-pm, Chris Healy, Lucas Stach, Zhang Rui, Eduardo Valentin,
Angus Ainslie, dl-linux-imx, linux-kernel
On 05/04/2019 20:00, Andrey Smirnov wrote:
> On Thu, Apr 4, 2019 at 12:57 AM Daniel Lezcano
> <daniel.lezcano@linaro.org> wrote:
>>
>> On 01/04/2019 06:14, Andrey Smirnov wrote:
>>> Embed per-sensor data into struct qoriq_tmu_data so we can drop the
>>> code allocating it. This also allows us to get rid of per-sensor back
>>> reference to struct qoriq_tmu_data since now its address can be
>>> caluclated using container_of().
>>
>> This seems to be a repeating pattern, drivers are forced to put a back
>> pointer in the thermal sensor structure to regain access to the
>> container structure in the get_temp callback.
>>
>> It would make sense to pass the sensor id to the get_temp callback as we
>> register with the sensor id.
>>
>
> I'll rebase this series on the two patches you submitted.
Well I should have added the RFC tag because I was not able to test with
a platform.
[ ... ]
>> Why not replace the little_endian boolean by a couple of callback
>> read/write and assign them to ioread32|ioread32be at init time.
>>
>> That will kill the tmu_read and tmu_write functions and from there you
>> can figure out how to remove the qdata backpointer. In addition, it will
>> save a few instructions to test the boolean.
>>
>
> I am sure you've seen it by now, but little_endian is going away in
> this series in regmap conversion patch.
Right :)
--
<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
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v3 06/13] thermal: qoriq: Pass data to qoriq_tmu_register_tmu_zone() directly
2019-04-01 4:14 [PATCH v3 00/13] QorIQ TMU multi-sensor and HWMON support Andrey Smirnov
` (4 preceding siblings ...)
2019-04-01 4:14 ` [PATCH v3 05/13] thermal: qoriq: Embed per-sensor data into struct qoriq_tmu_data Andrey Smirnov
@ 2019-04-01 4:14 ` Andrey Smirnov
2019-04-04 8:09 ` Daniel Lezcano
2019-04-01 4:14 ` [PATCH v3 07/13] thermal: qoriq: Pass data to qoriq_tmu_calibration() directly Andrey Smirnov
` (6 subsequent siblings)
12 siblings, 1 reply; 42+ messages in thread
From: Andrey Smirnov @ 2019-04-01 4:14 UTC (permalink / raw)
To: linux-pm
Cc: Andrey Smirnov, Chris Healy, Lucas Stach, Zhang Rui,
Eduardo Valentin, Daniel Lezcano, Angus Ainslie, linux-imx,
linux-kernel
Pass all necessary data to qoriq_tmu_register_tmu_zone() directly
instead of passing a paltform device and then deriving it. This is
done as a first step to simplify resource deallocation code.
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Angus Ainslie (Purism) <angus@akkea.ca>
Cc: linux-imx@nxp.com
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
drivers/thermal/qoriq_thermal.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index deb5cb6a0baf..24a2a57f61c9 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -109,9 +109,9 @@ static const struct thermal_zone_of_device_ops tmu_tz_ops = {
.get_temp = tmu_get_temp,
};
-static int qoriq_tmu_register_tmu_zone(struct platform_device *pdev)
+static int qoriq_tmu_register_tmu_zone(struct device *dev,
+ struct qoriq_tmu_data *qdata)
{
- struct qoriq_tmu_data *qdata = platform_get_drvdata(pdev);
int id, sites = 0;
for (id = 0; id < SITES_MAX; id++) {
@@ -120,7 +120,7 @@ static int qoriq_tmu_register_tmu_zone(struct platform_device *pdev)
s->id = id;
- tzd = devm_thermal_zone_of_sensor_register(&pdev->dev, id,
+ tzd = devm_thermal_zone_of_sensor_register(dev, id,
s, &tmu_tz_ops);
if (IS_ERR(tzd)) {
if (PTR_ERR(tzd) == -ENODEV)
@@ -215,7 +215,7 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
if (ret < 0)
goto err_tmu;
- ret = qoriq_tmu_register_tmu_zone(pdev);
+ ret = qoriq_tmu_register_tmu_zone(dev, data);
if (ret < 0) {
dev_err(dev, "Failed to register sensors\n");
ret = -ENODEV;
--
2.20.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v3 06/13] thermal: qoriq: Pass data to qoriq_tmu_register_tmu_zone() directly
2019-04-01 4:14 ` [PATCH v3 06/13] thermal: qoriq: Pass data to qoriq_tmu_register_tmu_zone() directly Andrey Smirnov
@ 2019-04-04 8:09 ` Daniel Lezcano
0 siblings, 0 replies; 42+ messages in thread
From: Daniel Lezcano @ 2019-04-04 8:09 UTC (permalink / raw)
To: Andrey Smirnov, linux-pm
Cc: Chris Healy, Lucas Stach, Zhang Rui, Eduardo Valentin,
Angus Ainslie, linux-imx, linux-kernel
On 01/04/2019 06:14, Andrey Smirnov wrote:
> Pass all necessary data to qoriq_tmu_register_tmu_zone() directly
> instead of passing a paltform device and then deriving it. This is
> done as a first step to simplify resource deallocation code.
>
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> Cc: Chris Healy <cphealy@gmail.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Zhang Rui <rui.zhang@intel.com>
> Cc: Eduardo Valentin <edubezval@gmail.com>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Angus Ainslie (Purism) <angus@akkea.ca>
> Cc: linux-imx@nxp.com
> Cc: linux-pm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
> drivers/thermal/qoriq_thermal.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
> index deb5cb6a0baf..24a2a57f61c9 100644
> --- a/drivers/thermal/qoriq_thermal.c
> +++ b/drivers/thermal/qoriq_thermal.c
> @@ -109,9 +109,9 @@ static const struct thermal_zone_of_device_ops tmu_tz_ops = {
> .get_temp = tmu_get_temp,
> };
>
> -static int qoriq_tmu_register_tmu_zone(struct platform_device *pdev)
> +static int qoriq_tmu_register_tmu_zone(struct device *dev,
> + struct qoriq_tmu_data *qdata)
> {
> - struct qoriq_tmu_data *qdata = platform_get_drvdata(pdev);
> int id, sites = 0;
>
> for (id = 0; id < SITES_MAX; id++) {
> @@ -120,7 +120,7 @@ static int qoriq_tmu_register_tmu_zone(struct platform_device *pdev)
>
> s->id = id;
>
> - tzd = devm_thermal_zone_of_sensor_register(&pdev->dev, id,
> + tzd = devm_thermal_zone_of_sensor_register(dev, id,
> s, &tmu_tz_ops);
> if (IS_ERR(tzd)) {
> if (PTR_ERR(tzd) == -ENODEV)
> @@ -215,7 +215,7 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
> if (ret < 0)
> goto err_tmu;
>
> - ret = qoriq_tmu_register_tmu_zone(pdev);
> + ret = qoriq_tmu_register_tmu_zone(dev, data);
> if (ret < 0) {
> dev_err(dev, "Failed to register sensors\n");
> ret = -ENODEV;
>
--
<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
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v3 07/13] thermal: qoriq: Pass data to qoriq_tmu_calibration() directly
2019-04-01 4:14 [PATCH v3 00/13] QorIQ TMU multi-sensor and HWMON support Andrey Smirnov
` (5 preceding siblings ...)
2019-04-01 4:14 ` [PATCH v3 06/13] thermal: qoriq: Pass data to qoriq_tmu_register_tmu_zone() directly Andrey Smirnov
@ 2019-04-01 4:14 ` Andrey Smirnov
2019-04-04 8:11 ` Daniel Lezcano
2019-04-01 4:14 ` [PATCH v3 08/13] thermal: qoriq: Convert driver to use devm_ioremap() Andrey Smirnov
` (5 subsequent siblings)
12 siblings, 1 reply; 42+ messages in thread
From: Andrey Smirnov @ 2019-04-01 4:14 UTC (permalink / raw)
To: linux-pm
Cc: Andrey Smirnov, Chris Healy, Lucas Stach, Zhang Rui,
Eduardo Valentin, Daniel Lezcano, Angus Ainslie, linux-imx,
linux-kernel
We can simplify error cleanup code if instead of passing a "struct
platform_device *" to qoriq_tmu_calibration() and deriving a bunch of
pointers from it, we pass those pointers directly. This way we won't
be force to call platform_set_drvdata() as early in qoriq_tmu_probe()
and consequently would be able to drop the "err_iomap" error path.
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Angus Ainslie (Purism) <angus@akkea.ca>
Cc: linux-imx@nxp.com
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
drivers/thermal/qoriq_thermal.c | 22 +++++++++-------------
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 24a2a57f61c9..a3ddb55740e4 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -139,16 +139,16 @@ static int qoriq_tmu_register_tmu_zone(struct device *dev,
return 0;
}
-static int qoriq_tmu_calibration(struct platform_device *pdev)
+static int qoriq_tmu_calibration(struct device *dev,
+ struct qoriq_tmu_data *data)
{
int i, val, len;
u32 range[4];
const u32 *calibration;
- struct device_node *np = pdev->dev.of_node;
- struct qoriq_tmu_data *data = platform_get_drvdata(pdev);
+ struct device_node *np = dev->of_node;
if (of_property_read_u32_array(np, "fsl,tmu-range", range, 4)) {
- dev_err(&pdev->dev, "missing calibration range.\n");
+ dev_err(dev, "missing calibration range.\n");
return -ENODEV;
}
@@ -160,7 +160,7 @@ static int qoriq_tmu_calibration(struct platform_device *pdev)
calibration = of_get_property(np, "fsl,tmu-calibration", &len);
if (calibration == NULL || len % 8) {
- dev_err(&pdev->dev, "invalid calibration data.\n");
+ dev_err(dev, "invalid calibration data.\n");
return -ENODEV;
}
@@ -198,20 +198,17 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
if (!data)
return -ENOMEM;
- platform_set_drvdata(pdev, data);
-
data->little_endian = of_property_read_bool(np, "little-endian");
data->regs = of_iomap(np, 0);
if (!data->regs) {
dev_err(dev, "Failed to get memory region\n");
- ret = -ENODEV;
- goto err_iomap;
+ return -ENODEV;
}
qoriq_tmu_init_device(data); /* TMU initialization */
- ret = qoriq_tmu_calibration(pdev); /* TMU calibration */
+ ret = qoriq_tmu_calibration(dev, data); /* TMU calibration */
if (ret < 0)
goto err_tmu;
@@ -222,14 +219,13 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
goto err_iomap;
}
+ platform_set_drvdata(pdev, data);
+
return 0;
err_tmu:
iounmap(data->regs);
-err_iomap:
- platform_set_drvdata(pdev, NULL);
-
return ret;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v3 07/13] thermal: qoriq: Pass data to qoriq_tmu_calibration() directly
2019-04-01 4:14 ` [PATCH v3 07/13] thermal: qoriq: Pass data to qoriq_tmu_calibration() directly Andrey Smirnov
@ 2019-04-04 8:11 ` Daniel Lezcano
2019-04-05 18:04 ` Andrey Smirnov
0 siblings, 1 reply; 42+ messages in thread
From: Daniel Lezcano @ 2019-04-04 8:11 UTC (permalink / raw)
To: Andrey Smirnov, linux-pm
Cc: Chris Healy, Lucas Stach, Zhang Rui, Eduardo Valentin,
Angus Ainslie, linux-imx, linux-kernel
On 01/04/2019 06:14, Andrey Smirnov wrote:
> We can simplify error cleanup code if instead of passing a "struct
> platform_device *" to qoriq_tmu_calibration() and deriving a bunch of
> pointers from it, we pass those pointers directly. This way we won't
> be force to call platform_set_drvdata() as early in qoriq_tmu_probe()
> and consequently would be able to drop the "err_iomap" error path.
>
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> Cc: Chris Healy <cphealy@gmail.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Zhang Rui <rui.zhang@intel.com>
> Cc: Eduardo Valentin <edubezval@gmail.com>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Angus Ainslie (Purism) <angus@akkea.ca>
> Cc: linux-imx@nxp.com
> Cc: linux-pm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
> drivers/thermal/qoriq_thermal.c | 22 +++++++++-------------
> 1 file changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
> index 24a2a57f61c9..a3ddb55740e4 100644
> --- a/drivers/thermal/qoriq_thermal.c
> +++ b/drivers/thermal/qoriq_thermal.c
> @@ -139,16 +139,16 @@ static int qoriq_tmu_register_tmu_zone(struct device *dev,
> return 0;
> }
>
> -static int qoriq_tmu_calibration(struct platform_device *pdev)
> +static int qoriq_tmu_calibration(struct device *dev,
> + struct qoriq_tmu_data *data)
> {
> int i, val, len;
> u32 range[4];
> const u32 *calibration;
> - struct device_node *np = pdev->dev.of_node;
> - struct qoriq_tmu_data *data = platform_get_drvdata(pdev);
> + struct device_node *np = dev->of_node;
>
> if (of_property_read_u32_array(np, "fsl,tmu-range", range, 4)) {
> - dev_err(&pdev->dev, "missing calibration range.\n");
> + dev_err(dev, "missing calibration range.\n");
> return -ENODEV;
> }
>
> @@ -160,7 +160,7 @@ static int qoriq_tmu_calibration(struct platform_device *pdev)
>
> calibration = of_get_property(np, "fsl,tmu-calibration", &len);
> if (calibration == NULL || len % 8) {
> - dev_err(&pdev->dev, "invalid calibration data.\n");
> + dev_err(dev, "invalid calibration data.\n");
> return -ENODEV;
> }
>
> @@ -198,20 +198,17 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
> if (!data)
> return -ENOMEM;
>
> - platform_set_drvdata(pdev, data);
> -
> data->little_endian = of_property_read_bool(np, "little-endian");
>
> data->regs = of_iomap(np, 0);
> if (!data->regs) {
> dev_err(dev, "Failed to get memory region\n");
> - ret = -ENODEV;
> - goto err_iomap;
> + return -ENODEV;
> }
>
> qoriq_tmu_init_device(data); /* TMU initialization */
>
> - ret = qoriq_tmu_calibration(pdev); /* TMU calibration */
> + ret = qoriq_tmu_calibration(dev, data); /* TMU calibration */
> if (ret < 0)
> goto err_tmu;
>
> @@ -222,14 +219,13 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
> goto err_iomap;
s/goto err_iomap/goto err_tmu/ ?
> }
>
> + platform_set_drvdata(pdev, data);
> +
> return 0;
>
> err_tmu:
> iounmap(data->regs);
>
> -err_iomap:
> - platform_set_drvdata(pdev, NULL);
> -
> return ret;
> }
>
>
--
<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
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH v3 07/13] thermal: qoriq: Pass data to qoriq_tmu_calibration() directly
2019-04-04 8:11 ` Daniel Lezcano
@ 2019-04-05 18:04 ` Andrey Smirnov
0 siblings, 0 replies; 42+ messages in thread
From: Andrey Smirnov @ 2019-04-05 18:04 UTC (permalink / raw)
To: Daniel Lezcano
Cc: linux-pm, Chris Healy, Lucas Stach, Zhang Rui, Eduardo Valentin,
Angus Ainslie, dl-linux-imx, linux-kernel
On Thu, Apr 4, 2019 at 1:12 AM Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>
> On 01/04/2019 06:14, Andrey Smirnov wrote:
> > We can simplify error cleanup code if instead of passing a "struct
> > platform_device *" to qoriq_tmu_calibration() and deriving a bunch of
> > pointers from it, we pass those pointers directly. This way we won't
> > be force to call platform_set_drvdata() as early in qoriq_tmu_probe()
> > and consequently would be able to drop the "err_iomap" error path.
> >
> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> > Cc: Chris Healy <cphealy@gmail.com>
> > Cc: Lucas Stach <l.stach@pengutronix.de>
> > Cc: Zhang Rui <rui.zhang@intel.com>
> > Cc: Eduardo Valentin <edubezval@gmail.com>
> > Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> > Cc: Angus Ainslie (Purism) <angus@akkea.ca>
> > Cc: linux-imx@nxp.com
> > Cc: linux-pm@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > ---
> > drivers/thermal/qoriq_thermal.c | 22 +++++++++-------------
> > 1 file changed, 9 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
> > index 24a2a57f61c9..a3ddb55740e4 100644
> > --- a/drivers/thermal/qoriq_thermal.c
> > +++ b/drivers/thermal/qoriq_thermal.c
> > @@ -139,16 +139,16 @@ static int qoriq_tmu_register_tmu_zone(struct device *dev,
> > return 0;
> > }
> >
> > -static int qoriq_tmu_calibration(struct platform_device *pdev)
> > +static int qoriq_tmu_calibration(struct device *dev,
> > + struct qoriq_tmu_data *data)
> > {
> > int i, val, len;
> > u32 range[4];
> > const u32 *calibration;
> > - struct device_node *np = pdev->dev.of_node;
> > - struct qoriq_tmu_data *data = platform_get_drvdata(pdev);
> > + struct device_node *np = dev->of_node;
> >
> > if (of_property_read_u32_array(np, "fsl,tmu-range", range, 4)) {
> > - dev_err(&pdev->dev, "missing calibration range.\n");
> > + dev_err(dev, "missing calibration range.\n");
> > return -ENODEV;
> > }
> >
> > @@ -160,7 +160,7 @@ static int qoriq_tmu_calibration(struct platform_device *pdev)
> >
> > calibration = of_get_property(np, "fsl,tmu-calibration", &len);
> > if (calibration == NULL || len % 8) {
> > - dev_err(&pdev->dev, "invalid calibration data.\n");
> > + dev_err(dev, "invalid calibration data.\n");
> > return -ENODEV;
> > }
> >
> > @@ -198,20 +198,17 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
> > if (!data)
> > return -ENOMEM;
> >
> > - platform_set_drvdata(pdev, data);
> > -
> > data->little_endian = of_property_read_bool(np, "little-endian");
> >
> > data->regs = of_iomap(np, 0);
> > if (!data->regs) {
> > dev_err(dev, "Failed to get memory region\n");
> > - ret = -ENODEV;
> > - goto err_iomap;
> > + return -ENODEV;
> > }
> >
> > qoriq_tmu_init_device(data); /* TMU initialization */
> >
> > - ret = qoriq_tmu_calibration(pdev); /* TMU calibration */
> > + ret = qoriq_tmu_calibration(dev, data); /* TMU calibration */
> > if (ret < 0)
> > goto err_tmu;
> >
> > @@ -222,14 +219,13 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
> > goto err_iomap;
>
> s/goto err_iomap/goto err_tmu/ ?
>
Ugh, not sure how I missed this. Will fix. Thanks for catching this!
Thanks,
Andrey Smirnov
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v3 08/13] thermal: qoriq: Convert driver to use devm_ioremap()
2019-04-01 4:14 [PATCH v3 00/13] QorIQ TMU multi-sensor and HWMON support Andrey Smirnov
` (6 preceding siblings ...)
2019-04-01 4:14 ` [PATCH v3 07/13] thermal: qoriq: Pass data to qoriq_tmu_calibration() directly Andrey Smirnov
@ 2019-04-01 4:14 ` Andrey Smirnov
2019-04-04 8:23 ` Daniel Lezcano
2019-04-01 4:14 ` [PATCH v3 09/13] thermal: qoriq: Convert driver to use regmap API Andrey Smirnov
` (4 subsequent siblings)
12 siblings, 1 reply; 42+ messages in thread
From: Andrey Smirnov @ 2019-04-01 4:14 UTC (permalink / raw)
To: linux-pm
Cc: Andrey Smirnov, Chris Healy, Lucas Stach, Zhang Rui,
Eduardo Valentin, Daniel Lezcano, Angus Ainslie, linux-imx,
linux-kernel
Convert driver to use devm_ioremap() to simplify memory deallocation
and error handling code. No functional change intended.
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Angus Ainslie (Purism) <angus@akkea.ca>
Cc: linux-imx@nxp.com
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
drivers/thermal/qoriq_thermal.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index a3ddb55740e4..4f9a2543f9c3 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -192,6 +192,7 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
struct qoriq_tmu_data *data;
struct device_node *np = pdev->dev.of_node;
struct device *dev = &pdev->dev;
+ struct resource *io;
data = devm_kzalloc(dev, sizeof(struct qoriq_tmu_data),
GFP_KERNEL);
@@ -200,7 +201,13 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
data->little_endian = of_property_read_bool(np, "little-endian");
- data->regs = of_iomap(np, 0);
+ io = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!io) {
+ dev_err(dev, "Failed to get memory region\n");
+ return -ENODEV;
+ }
+
+ data->regs = devm_ioremap(dev, io->start, resource_size(io));
if (!data->regs) {
dev_err(dev, "Failed to get memory region\n");
return -ENODEV;
@@ -210,23 +217,17 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
ret = qoriq_tmu_calibration(dev, data); /* TMU calibration */
if (ret < 0)
- goto err_tmu;
+ return ret;
ret = qoriq_tmu_register_tmu_zone(dev, data);
if (ret < 0) {
dev_err(dev, "Failed to register sensors\n");
- ret = -ENODEV;
- goto err_iomap;
+ return -ENODEV;
}
platform_set_drvdata(pdev, data);
return 0;
-
-err_tmu:
- iounmap(data->regs);
-
- return ret;
}
static int qoriq_tmu_remove(struct platform_device *pdev)
@@ -236,7 +237,6 @@ static int qoriq_tmu_remove(struct platform_device *pdev)
/* Disable monitoring */
tmu_write(data, TMR_DISABLE, &data->regs->tmr);
- iounmap(data->regs);
platform_set_drvdata(pdev, NULL);
return 0;
--
2.20.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v3 08/13] thermal: qoriq: Convert driver to use devm_ioremap()
2019-04-01 4:14 ` [PATCH v3 08/13] thermal: qoriq: Convert driver to use devm_ioremap() Andrey Smirnov
@ 2019-04-04 8:23 ` Daniel Lezcano
2019-04-05 18:14 ` Andrey Smirnov
0 siblings, 1 reply; 42+ messages in thread
From: Daniel Lezcano @ 2019-04-04 8:23 UTC (permalink / raw)
To: Andrey Smirnov, linux-pm
Cc: Chris Healy, Lucas Stach, Zhang Rui, Eduardo Valentin,
Angus Ainslie, linux-imx, linux-kernel
On 01/04/2019 06:14, Andrey Smirnov wrote:
> Convert driver to use devm_ioremap() to simplify memory deallocation
> and error handling code. No functional change intended.
>
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> Cc: Chris Healy <cphealy@gmail.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Zhang Rui <rui.zhang@intel.com>
> Cc: Eduardo Valentin <edubezval@gmail.com>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Angus Ainslie (Purism) <angus@akkea.ca>
> Cc: linux-imx@nxp.com
> Cc: linux-pm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
> drivers/thermal/qoriq_thermal.c | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
> index a3ddb55740e4..4f9a2543f9c3 100644
> --- a/drivers/thermal/qoriq_thermal.c
> +++ b/drivers/thermal/qoriq_thermal.c
> @@ -192,6 +192,7 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
> struct qoriq_tmu_data *data;
> struct device_node *np = pdev->dev.of_node;
> struct device *dev = &pdev->dev;
> + struct resource *io;
>
> data = devm_kzalloc(dev, sizeof(struct qoriq_tmu_data),
> GFP_KERNEL);
> @@ -200,7 +201,13 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
>
> data->little_endian = of_property_read_bool(np, "little-endian");
>
> - data->regs = of_iomap(np, 0);
> + io = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!io) {
> + dev_err(dev, "Failed to get memory region\n");
> + return -ENODEV;
> + }
> +
> + data->regs = devm_ioremap(dev, io->start, resource_size(io));
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
base = devm_ioremap_resource(&pdev->dev, res);
?
> if (!data->regs) {
> dev_err(dev, "Failed to get memory region\n");
> return -ENODEV;
> @@ -210,23 +217,17 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
>
> ret = qoriq_tmu_calibration(dev, data); /* TMU calibration */
> if (ret < 0)
> - goto err_tmu;
> + return ret;
>
> ret = qoriq_tmu_register_tmu_zone(dev, data);
> if (ret < 0) {
> dev_err(dev, "Failed to register sensors\n");
> - ret = -ENODEV;
> - goto err_iomap;
> + return -ENODEV;
> }
>
> platform_set_drvdata(pdev, data);
>
> return 0;
> -
> -err_tmu:
> - iounmap(data->regs);
> -
> - return ret;
> }
>
> static int qoriq_tmu_remove(struct platform_device *pdev)
> @@ -236,7 +237,6 @@ static int qoriq_tmu_remove(struct platform_device *pdev)
> /* Disable monitoring */
> tmu_write(data, TMR_DISABLE, &data->regs->tmr);
>
> - iounmap(data->regs);
> platform_set_drvdata(pdev, NULL);
>
> return 0;
>
--
<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
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH v3 08/13] thermal: qoriq: Convert driver to use devm_ioremap()
2019-04-04 8:23 ` Daniel Lezcano
@ 2019-04-05 18:14 ` Andrey Smirnov
0 siblings, 0 replies; 42+ messages in thread
From: Andrey Smirnov @ 2019-04-05 18:14 UTC (permalink / raw)
To: Daniel Lezcano
Cc: linux-pm, Chris Healy, Lucas Stach, Zhang Rui, Eduardo Valentin,
Angus Ainslie, dl-linux-imx, linux-kernel
On Thu, Apr 4, 2019 at 1:23 AM Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>
> On 01/04/2019 06:14, Andrey Smirnov wrote:
> > Convert driver to use devm_ioremap() to simplify memory deallocation
> > and error handling code. No functional change intended.
> >
> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> > Cc: Chris Healy <cphealy@gmail.com>
> > Cc: Lucas Stach <l.stach@pengutronix.de>
> > Cc: Zhang Rui <rui.zhang@intel.com>
> > Cc: Eduardo Valentin <edubezval@gmail.com>
> > Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> > Cc: Angus Ainslie (Purism) <angus@akkea.ca>
> > Cc: linux-imx@nxp.com
> > Cc: linux-pm@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > ---
> > drivers/thermal/qoriq_thermal.c | 20 ++++++++++----------
> > 1 file changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
> > index a3ddb55740e4..4f9a2543f9c3 100644
> > --- a/drivers/thermal/qoriq_thermal.c
> > +++ b/drivers/thermal/qoriq_thermal.c
> > @@ -192,6 +192,7 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
> > struct qoriq_tmu_data *data;
> > struct device_node *np = pdev->dev.of_node;
> > struct device *dev = &pdev->dev;
> > + struct resource *io;
> >
> > data = devm_kzalloc(dev, sizeof(struct qoriq_tmu_data),
> > GFP_KERNEL);
> > @@ -200,7 +201,13 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
> >
> > data->little_endian = of_property_read_bool(np, "little-endian");
> >
> > - data->regs = of_iomap(np, 0);
> > + io = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + if (!io) {
> > + dev_err(dev, "Failed to get memory region\n");
> > + return -ENODEV;
> > + }
> > +
> > + data->regs = devm_ioremap(dev, io->start, resource_size(io));
>
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> base = devm_ioremap_resource(&pdev->dev, res);
> ?
>
devm_ioremap_resource() will call devm_request_mem_region() which
would change original behavior. I don't know if original code was
purposefully written to not request underlying memory region or not,
but I didn't want to change the behavior. I am more than happy to
change the code to use devm_ioremap_resource() (or probably even
devm_platform_ioremap_resource()) if we agree that such a change is
acceptable.
Thanks,
Andrey Smirnov
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v3 09/13] thermal: qoriq: Convert driver to use regmap API
2019-04-01 4:14 [PATCH v3 00/13] QorIQ TMU multi-sensor and HWMON support Andrey Smirnov
` (7 preceding siblings ...)
2019-04-01 4:14 ` [PATCH v3 08/13] thermal: qoriq: Convert driver to use devm_ioremap() Andrey Smirnov
@ 2019-04-01 4:14 ` Andrey Smirnov
2019-04-04 8:47 ` Daniel Lezcano
2019-04-01 4:14 ` [PATCH v3 10/13] thermal: qoriq: Enable all sensors before registering them Andrey Smirnov
` (3 subsequent siblings)
12 siblings, 1 reply; 42+ messages in thread
From: Andrey Smirnov @ 2019-04-01 4:14 UTC (permalink / raw)
To: linux-pm
Cc: Andrey Smirnov, Chris Healy, Lucas Stach, Zhang Rui,
Eduardo Valentin, Daniel Lezcano, Angus Ainslie, linux-imx,
linux-kernel
Convert driver to use regmap API, drop custom LE/BE IO helpers and
simplify bit manipulation using regmap_update_bits(). This also allows
us to convert some register initialization to use loops and adds
convenient debug access to TMU registers via debugfs.
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Angus Ainslie (Purism) <angus@akkea.ca>
Cc: linux-imx@nxp.com
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
drivers/thermal/qoriq_thermal.c | 159 +++++++++++++++-----------------
1 file changed, 74 insertions(+), 85 deletions(-)
diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 4f9a2543f9c3..a909acee4354 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -8,6 +8,7 @@
#include <linux/io.h>
#include <linux/of.h>
#include <linux/of_address.h>
+#include <linux/regmap.h>
#include <linux/thermal.h>
#include "thermal_core.h"
@@ -17,48 +18,27 @@
/*
* QorIQ TMU Registers
*/
-struct qoriq_tmu_site_regs {
- u32 tritsr; /* Immediate Temperature Site Register */
- u32 tratsr; /* Average Temperature Site Register */
- u8 res0[0x8];
-};
-struct qoriq_tmu_regs {
- u32 tmr; /* Mode Register */
+#define REGS_TMR 0x000 /* 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 REGS_TMTMIR 0x008 /* Temperature measurement interval Register */
#define TMTMIR_DEFAULT 0x0000000f
- u8 res0[0x14];
- u32 tier; /* Interrupt Enable Register */
+
+#define REGS_TIER 0x020 /* 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 */
- u8 res1[0x10];
- u32 tmhtcrh; /* High Temperature Capture Register */
- u32 tmhtcrl; /* Low Temperature Capture Register */
- u8 res2[0x8];
- u32 tmhtitr; /* High Temperature Immediate Threshold */
- u32 tmhtatr; /* High Temperature Average Threshold */
- u32 tmhtactr; /* High Temperature Average Crit Threshold */
- u8 res3[0x24];
- u32 ttcfgr; /* Temperature Configuration Register */
- u32 tscfgr; /* Sensor Configuration Register */
- u8 res4[0x78];
- struct qoriq_tmu_site_regs site[SITES_MAX];
- u8 res5[0x9f8];
- 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 */
-};
+#define REGS_TTCFGR 0x080 /* Temperature Configuration Register */
+#define REGS_TSCFGR 0x084 /* Sensor Configuration Register */
+
+#define REGS_TRITSR(n) (0x100 + 16 * (n)) /* Immediate Temperature
+ * Site Register
+ */
+#define REGS_TTRnCR(n) (0xf10 + 4 * (n)) /* Temperature Range n
+ * Control Register
+ */
/*
* Thermal zone data
*/
@@ -67,8 +47,7 @@ struct qoriq_sensor {
};
struct qoriq_tmu_data {
- struct qoriq_tmu_regs __iomem *regs;
- bool little_endian;
+ struct regmap *regmap;
struct qoriq_sensor sensor[SITES_MAX];
};
@@ -77,29 +56,13 @@ static struct qoriq_tmu_data *qoriq_sensor_to_data(struct qoriq_sensor *s)
return container_of(s, struct qoriq_tmu_data, sensor[s->id]);
}
-static void tmu_write(struct qoriq_tmu_data *p, u32 val, void __iomem *addr)
-{
- if (p->little_endian)
- iowrite32(val, addr);
- else
- iowrite32be(val, addr);
-}
-
-static u32 tmu_read(struct qoriq_tmu_data *p, void __iomem *addr)
-{
- if (p->little_endian)
- return ioread32(addr);
- else
- return ioread32be(addr);
-}
-
static int tmu_get_temp(void *p, int *temp)
{
struct qoriq_sensor *qsensor = p;
struct qoriq_tmu_data *qdata = qoriq_sensor_to_data(qsensor);
u32 val;
- val = tmu_read(qdata, &qdata->regs->site[qsensor->id].tritsr);
+ regmap_read(qdata->regmap, REGS_TRITSR(qsensor->id), &val);
*temp = (val & 0xff) * 1000;
return 0;
@@ -134,7 +97,8 @@ static int qoriq_tmu_register_tmu_zone(struct device *dev,
/* Enable monitoring */
if (sites != 0)
- tmu_write(qdata, sites | TMR_ME | TMR_ALPF, &qdata->regs->tmr);
+ regmap_write(qdata->regmap, REGS_TMR,
+ sites | TMR_ME | TMR_ALPF);
return 0;
}
@@ -153,10 +117,8 @@ static int qoriq_tmu_calibration(struct device *dev,
}
/* 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);
+ for (i = 0; i < ARRAY_SIZE(range); i++)
+ regmap_write(data->regmap, REGS_TTRnCR(i), range[i]);
calibration = of_get_property(np, "fsl,tmu-calibration", &len);
if (calibration == NULL || len % 8) {
@@ -166,9 +128,9 @@ static int qoriq_tmu_calibration(struct device *dev,
for (i = 0; i < len; i += 8, calibration += 2) {
val = of_read_number(calibration, 1);
- tmu_write(data, val, &data->regs->ttcfgr);
+ regmap_write(data->regmap, REGS_TTCFGR, val);
val = of_read_number(calibration + 1, 1);
- tmu_write(data, val, &data->regs->tscfgr);
+ regmap_write(data->regmap, REGS_TSCFGR, val);
}
return 0;
@@ -177,15 +139,32 @@ static int qoriq_tmu_calibration(struct device *dev,
static void qoriq_tmu_init_device(struct qoriq_tmu_data *data)
{
/* Disable interrupt, using polling instead */
- tmu_write(data, TIER_DISABLE, &data->regs->tier);
+ regmap_write(data->regmap, REGS_TIER, TIER_DISABLE);
/* Set update_interval */
- tmu_write(data, TMTMIR_DEFAULT, &data->regs->tmtmir);
+ regmap_write(data->regmap, REGS_TMTMIR, TMTMIR_DEFAULT);
/* Disable monitoring */
- tmu_write(data, TMR_DISABLE, &data->regs->tmr);
+ regmap_write(data->regmap, REGS_TMR, TMR_DISABLE);
}
+static const struct regmap_range qiriq_yes_ranges[] = {
+ regmap_reg_range(REGS_TMR, REGS_TSCFGR),
+ regmap_reg_range(REGS_TTRnCR(0), REGS_TTRnCR(3)),
+ /* Read only registers below */
+ regmap_reg_range(REGS_TRITSR(0), REGS_TRITSR(15)),
+};
+
+static const struct regmap_access_table qiriq_wr_table = {
+ .yes_ranges = qiriq_yes_ranges,
+ .n_yes_ranges = ARRAY_SIZE(qiriq_yes_ranges) - 1,
+};
+
+static const struct regmap_access_table qiriq_rd_table = {
+ .yes_ranges = qiriq_yes_ranges,
+ .n_yes_ranges = ARRAY_SIZE(qiriq_yes_ranges),
+};
+
static int qoriq_tmu_probe(struct platform_device *pdev)
{
int ret;
@@ -193,26 +172,44 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
struct device_node *np = pdev->dev.of_node;
struct device *dev = &pdev->dev;
struct resource *io;
+ const bool little_endian = of_property_read_bool(np, "little-endian");
+ const enum regmap_endian format_endian =
+ little_endian ? REGMAP_ENDIAN_LITTLE : REGMAP_ENDIAN_BIG;
+ const struct regmap_config regmap_config = {
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = 4,
+ .rd_table = &qiriq_rd_table,
+ .wr_table = &qiriq_wr_table,
+ .val_format_endian = format_endian,
+ .max_register = SZ_4K,
+ };
+ void __iomem *base;
data = devm_kzalloc(dev, sizeof(struct qoriq_tmu_data),
GFP_KERNEL);
if (!data)
return -ENOMEM;
- data->little_endian = of_property_read_bool(np, "little-endian");
-
io = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!io) {
dev_err(dev, "Failed to get memory region\n");
return -ENODEV;
}
- data->regs = devm_ioremap(dev, io->start, resource_size(io));
- if (!data->regs) {
+ base = devm_ioremap(dev, io->start, resource_size(io));
+ if (!base) {
dev_err(dev, "Failed to get memory region\n");
return -ENODEV;
}
+ data->regmap = devm_regmap_init_mmio(dev, base, ®map_config);
+ if (IS_ERR(data->regmap)) {
+ ret = PTR_ERR(data->regmap);
+ dev_err(dev, "Failed to init regmap (%d)\n", ret);
+ return ret;
+ }
+
qoriq_tmu_init_device(data); /* TMU initialization */
ret = qoriq_tmu_calibration(dev, data); /* TMU calibration */
@@ -235,7 +232,7 @@ static int qoriq_tmu_remove(struct platform_device *pdev)
struct qoriq_tmu_data *data = platform_get_drvdata(pdev);
/* Disable monitoring */
- tmu_write(data, TMR_DISABLE, &data->regs->tmr);
+ regmap_write(data->regmap, REGS_TMR, TMR_DISABLE);
platform_set_drvdata(pdev, NULL);
@@ -243,30 +240,22 @@ static int qoriq_tmu_remove(struct platform_device *pdev)
}
#ifdef CONFIG_PM_SLEEP
-static int qoriq_tmu_suspend(struct device *dev)
+
+static int qoriq_tmu_suspend_resume(struct device *dev, unsigned int val)
{
- u32 tmr;
struct qoriq_tmu_data *data = dev_get_drvdata(dev);
- /* Disable monitoring */
- tmr = tmu_read(data, &data->regs->tmr);
- tmr &= ~TMR_ME;
- tmu_write(data, tmr, &data->regs->tmr);
+ return regmap_update_bits(data->regmap, REGS_TMR, TMR_ME, val);
+}
- return 0;
+static int qoriq_tmu_suspend(struct device *dev)
+{
+ return qoriq_tmu_suspend_resume(dev, 0);
}
static int qoriq_tmu_resume(struct device *dev)
{
- u32 tmr;
- struct qoriq_tmu_data *data = dev_get_drvdata(dev);
-
- /* Enable monitoring */
- tmr = tmu_read(data, &data->regs->tmr);
- tmr |= TMR_ME;
- tmu_write(data, tmr, &data->regs->tmr);
-
- return 0;
+ return qoriq_tmu_suspend_resume(dev, TMR_ME);
}
#endif
--
2.20.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v3 09/13] thermal: qoriq: Convert driver to use regmap API
2019-04-01 4:14 ` [PATCH v3 09/13] thermal: qoriq: Convert driver to use regmap API Andrey Smirnov
@ 2019-04-04 8:47 ` Daniel Lezcano
2019-04-05 18:24 ` Andrey Smirnov
0 siblings, 1 reply; 42+ messages in thread
From: Daniel Lezcano @ 2019-04-04 8:47 UTC (permalink / raw)
To: Andrey Smirnov, linux-pm
Cc: Chris Healy, Lucas Stach, Zhang Rui, Eduardo Valentin,
Angus Ainslie, linux-imx, linux-kernel
On 01/04/2019 06:14, Andrey Smirnov wrote:
> Convert driver to use regmap API, drop custom LE/BE IO helpers and
> simplify bit manipulation using regmap_update_bits(). This also allows
> us to convert some register initialization to use loops and adds
> convenient debug access to TMU registers via debugfs.
>
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> Cc: Chris Healy <cphealy@gmail.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Zhang Rui <rui.zhang@intel.com>
> Cc: Eduardo Valentin <edubezval@gmail.com>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Angus Ainslie (Purism) <angus@akkea.ca>
> Cc: linux-imx@nxp.com
> Cc: linux-pm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
> drivers/thermal/qoriq_thermal.c | 159 +++++++++++++++-----------------
> 1 file changed, 74 insertions(+), 85 deletions(-)
>
> diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
> index 4f9a2543f9c3..a909acee4354 100644
> --- a/drivers/thermal/qoriq_thermal.c
> +++ b/drivers/thermal/qoriq_thermal.c
> @@ -8,6 +8,7 @@
> #include <linux/io.h>
> #include <linux/of.h>
> #include <linux/of_address.h>
> +#include <linux/regmap.h>
> #include <linux/thermal.h>
>
> #include "thermal_core.h"
> @@ -17,48 +18,27 @@
> /*
> * QorIQ TMU Registers
> */
> -struct qoriq_tmu_site_regs {
> - u32 tritsr; /* Immediate Temperature Site Register */
> - u32 tratsr; /* Average Temperature Site Register */
> - u8 res0[0x8];
> -};
>
> -struct qoriq_tmu_regs {
> - u32 tmr; /* Mode Register */
> +#define REGS_TMR 0x000 /* 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 REGS_TMTMIR 0x008 /* Temperature measurement interval Register */
> #define TMTMIR_DEFAULT 0x0000000f
> - u8 res0[0x14];
> - u32 tier; /* Interrupt Enable Register */
> +
> +#define REGS_TIER 0x020 /* 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 */
> - u8 res1[0x10];
> - u32 tmhtcrh; /* High Temperature Capture Register */
> - u32 tmhtcrl; /* Low Temperature Capture Register */
> - u8 res2[0x8];
> - u32 tmhtitr; /* High Temperature Immediate Threshold */
> - u32 tmhtatr; /* High Temperature Average Threshold */
> - u32 tmhtactr; /* High Temperature Average Crit Threshold */
> - u8 res3[0x24];
> - u32 ttcfgr; /* Temperature Configuration Register */
> - u32 tscfgr; /* Sensor Configuration Register */
> - u8 res4[0x78];
> - struct qoriq_tmu_site_regs site[SITES_MAX];
> - u8 res5[0x9f8];
> - 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 */
> -};
>
> +#define REGS_TTCFGR 0x080 /* Temperature Configuration Register */
> +#define REGS_TSCFGR 0x084 /* Sensor Configuration Register */
> +
> +#define REGS_TRITSR(n) (0x100 + 16 * (n)) /* Immediate Temperature
> + * Site Register
> + */
> +#define REGS_TTRnCR(n) (0xf10 + 4 * (n)) /* Temperature Range n
> + * Control Register
> + */
> /*
> * Thermal zone data
> */
> @@ -67,8 +47,7 @@ struct qoriq_sensor {
> };
>
> struct qoriq_tmu_data {
> - struct qoriq_tmu_regs __iomem *regs;
> - bool little_endian;
> + struct regmap *regmap;
> struct qoriq_sensor sensor[SITES_MAX];
> };
>
> @@ -77,29 +56,13 @@ static struct qoriq_tmu_data *qoriq_sensor_to_data(struct qoriq_sensor *s)
> return container_of(s, struct qoriq_tmu_data, sensor[s->id]);
> }
>
> -static void tmu_write(struct qoriq_tmu_data *p, u32 val, void __iomem *addr)
> -{
> - if (p->little_endian)
> - iowrite32(val, addr);
> - else
> - iowrite32be(val, addr);
> -}
> -
> -static u32 tmu_read(struct qoriq_tmu_data *p, void __iomem *addr)
> -{
> - if (p->little_endian)
> - return ioread32(addr);
> - else
> - return ioread32be(addr);
> -}
> -
> static int tmu_get_temp(void *p, int *temp)
> {
> struct qoriq_sensor *qsensor = p;
> struct qoriq_tmu_data *qdata = qoriq_sensor_to_data(qsensor);
> u32 val;
>
> - val = tmu_read(qdata, &qdata->regs->site[qsensor->id].tritsr);
> + regmap_read(qdata->regmap, REGS_TRITSR(qsensor->id), &val);
> *temp = (val & 0xff) * 1000;
>
> return 0;
> @@ -134,7 +97,8 @@ static int qoriq_tmu_register_tmu_zone(struct device *dev,
>
> /* Enable monitoring */
> if (sites != 0)
> - tmu_write(qdata, sites | TMR_ME | TMR_ALPF, &qdata->regs->tmr);
> + regmap_write(qdata->regmap, REGS_TMR,
> + sites | TMR_ME | TMR_ALPF);
>
> return 0;
> }
> @@ -153,10 +117,8 @@ static int qoriq_tmu_calibration(struct device *dev,
> }
>
> /* 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);
> + for (i = 0; i < ARRAY_SIZE(range); i++)
> + regmap_write(data->regmap, REGS_TTRnCR(i), range[i]);
>
> calibration = of_get_property(np, "fsl,tmu-calibration", &len);
> if (calibration == NULL || len % 8) {
> @@ -166,9 +128,9 @@ static int qoriq_tmu_calibration(struct device *dev,
>
> for (i = 0; i < len; i += 8, calibration += 2) {
> val = of_read_number(calibration, 1);
> - tmu_write(data, val, &data->regs->ttcfgr);
> + regmap_write(data->regmap, REGS_TTCFGR, val);
> val = of_read_number(calibration + 1, 1);
> - tmu_write(data, val, &data->regs->tscfgr);
> + regmap_write(data->regmap, REGS_TSCFGR, val);
> }
>
> return 0;
> @@ -177,15 +139,32 @@ static int qoriq_tmu_calibration(struct device *dev,
> static void qoriq_tmu_init_device(struct qoriq_tmu_data *data)
> {
> /* Disable interrupt, using polling instead */
> - tmu_write(data, TIER_DISABLE, &data->regs->tier);
> + regmap_write(data->regmap, REGS_TIER, TIER_DISABLE);
>
> /* Set update_interval */
> - tmu_write(data, TMTMIR_DEFAULT, &data->regs->tmtmir);
> + regmap_write(data->regmap, REGS_TMTMIR, TMTMIR_DEFAULT);
>
> /* Disable monitoring */
> - tmu_write(data, TMR_DISABLE, &data->regs->tmr);
> + regmap_write(data->regmap, REGS_TMR, TMR_DISABLE);
> }
>
> +static const struct regmap_range qiriq_yes_ranges[] = {
> + regmap_reg_range(REGS_TMR, REGS_TSCFGR),
> + regmap_reg_range(REGS_TTRnCR(0), REGS_TTRnCR(3)),
> + /* Read only registers below */
> + regmap_reg_range(REGS_TRITSR(0), REGS_TRITSR(15)),
> +};
> +
> +static const struct regmap_access_table qiriq_wr_table = {
> + .yes_ranges = qiriq_yes_ranges,
> + .n_yes_ranges = ARRAY_SIZE(qiriq_yes_ranges) - 1,
> +};
> +
> +static const struct regmap_access_table qiriq_rd_table = {
> + .yes_ranges = qiriq_yes_ranges,
> + .n_yes_ranges = ARRAY_SIZE(qiriq_yes_ranges),
> +};
As the table are the same, it would make sense to fold both structure to
a single one (and s/qiriq/qoriq/ ?)
> static int qoriq_tmu_probe(struct platform_device *pdev)
> {
> int ret;
> @@ -193,26 +172,44 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
> struct device_node *np = pdev->dev.of_node;
> struct device *dev = &pdev->dev;
> struct resource *io;
> + const bool little_endian = of_property_read_bool(np, "little-endian");
> + const enum regmap_endian format_endian =
> + little_endian ? REGMAP_ENDIAN_LITTLE : REGMAP_ENDIAN_BIG;
> + const struct regmap_config regmap_config = {
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = 4,
> + .rd_table = &qiriq_rd_table,
> + .wr_table = &qiriq_wr_table,
> + .val_format_endian = format_endian,
> + .max_register = SZ_4K,
> + };
> + void __iomem *base;
>
> data = devm_kzalloc(dev, sizeof(struct qoriq_tmu_data),
> GFP_KERNEL);
> if (!data)
> return -ENOMEM;
>
> - data->little_endian = of_property_read_bool(np, "little-endian");
> -
> io = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> if (!io) {
> dev_err(dev, "Failed to get memory region\n");
> return -ENODEV;
> }
>
> - data->regs = devm_ioremap(dev, io->start, resource_size(io));
> - if (!data->regs) {
> + base = devm_ioremap(dev, io->start, resource_size(io));
> + if (!base) {
> dev_err(dev, "Failed to get memory region\n");
> return -ENODEV;
> }
>
> + data->regmap = devm_regmap_init_mmio(dev, base, ®map_config);
> + if (IS_ERR(data->regmap)) {
> + ret = PTR_ERR(data->regmap);
> + dev_err(dev, "Failed to init regmap (%d)\n", ret);
> + return ret;
> + }
> +
> qoriq_tmu_init_device(data); /* TMU initialization */
>
> ret = qoriq_tmu_calibration(dev, data); /* TMU calibration */
> @@ -235,7 +232,7 @@ static int qoriq_tmu_remove(struct platform_device *pdev)
> struct qoriq_tmu_data *data = platform_get_drvdata(pdev);
>
> /* Disable monitoring */
> - tmu_write(data, TMR_DISABLE, &data->regs->tmr);
> + regmap_write(data->regmap, REGS_TMR, TMR_DISABLE);
>
> platform_set_drvdata(pdev, NULL);
>
> @@ -243,30 +240,22 @@ static int qoriq_tmu_remove(struct platform_device *pdev)
> }
>
> #ifdef CONFIG_PM_SLEEP
> -static int qoriq_tmu_suspend(struct device *dev)
> +
> +static int qoriq_tmu_suspend_resume(struct device *dev, unsigned int val)
> {
> - u32 tmr;
> struct qoriq_tmu_data *data = dev_get_drvdata(dev);
>
> - /* Disable monitoring */
> - tmr = tmu_read(data, &data->regs->tmr);
> - tmr &= ~TMR_ME;
> - tmu_write(data, tmr, &data->regs->tmr);
> + return regmap_update_bits(data->regmap, REGS_TMR, TMR_ME, val);
> +}
>
> - return 0;
> +static int qoriq_tmu_suspend(struct device *dev)
> +{
> + return qoriq_tmu_suspend_resume(dev, 0);
> }
>
> static int qoriq_tmu_resume(struct device *dev)
> {
> - u32 tmr;
> - struct qoriq_tmu_data *data = dev_get_drvdata(dev);
> -
> - /* Enable monitoring */
> - tmr = tmu_read(data, &data->regs->tmr);
> - tmr |= TMR_ME;
> - tmu_write(data, tmr, &data->regs->tmr);
> -
> - return 0;
> + return qoriq_tmu_suspend_resume(dev, TMR_ME);
> }
> #endif
>
>
--
<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
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH v3 09/13] thermal: qoriq: Convert driver to use regmap API
2019-04-04 8:47 ` Daniel Lezcano
@ 2019-04-05 18:24 ` Andrey Smirnov
2019-04-11 13:00 ` Daniel Lezcano
0 siblings, 1 reply; 42+ messages in thread
From: Andrey Smirnov @ 2019-04-05 18:24 UTC (permalink / raw)
To: Daniel Lezcano
Cc: linux-pm, Chris Healy, Lucas Stach, Zhang Rui, Eduardo Valentin,
Angus Ainslie, dl-linux-imx, linux-kernel
On Thu, Apr 4, 2019 at 1:47 AM Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>
> On 01/04/2019 06:14, Andrey Smirnov wrote:
> > Convert driver to use regmap API, drop custom LE/BE IO helpers and
> > simplify bit manipulation using regmap_update_bits(). This also allows
> > us to convert some register initialization to use loops and adds
> > convenient debug access to TMU registers via debugfs.
> >
> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> > Cc: Chris Healy <cphealy@gmail.com>
> > Cc: Lucas Stach <l.stach@pengutronix.de>
> > Cc: Zhang Rui <rui.zhang@intel.com>
> > Cc: Eduardo Valentin <edubezval@gmail.com>
> > Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> > Cc: Angus Ainslie (Purism) <angus@akkea.ca>
> > Cc: linux-imx@nxp.com
> > Cc: linux-pm@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > ---
> > drivers/thermal/qoriq_thermal.c | 159 +++++++++++++++-----------------
> > 1 file changed, 74 insertions(+), 85 deletions(-)
> >
> > diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
> > index 4f9a2543f9c3..a909acee4354 100644
> > --- a/drivers/thermal/qoriq_thermal.c
> > +++ b/drivers/thermal/qoriq_thermal.c
> > @@ -8,6 +8,7 @@
> > #include <linux/io.h>
> > #include <linux/of.h>
> > #include <linux/of_address.h>
> > +#include <linux/regmap.h>
> > #include <linux/thermal.h>
> >
> > #include "thermal_core.h"
> > @@ -17,48 +18,27 @@
> > /*
> > * QorIQ TMU Registers
> > */
> > -struct qoriq_tmu_site_regs {
> > - u32 tritsr; /* Immediate Temperature Site Register */
> > - u32 tratsr; /* Average Temperature Site Register */
> > - u8 res0[0x8];
> > -};
> >
> > -struct qoriq_tmu_regs {
> > - u32 tmr; /* Mode Register */
> > +#define REGS_TMR 0x000 /* 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 REGS_TMTMIR 0x008 /* Temperature measurement interval Register */
> > #define TMTMIR_DEFAULT 0x0000000f
> > - u8 res0[0x14];
> > - u32 tier; /* Interrupt Enable Register */
> > +
> > +#define REGS_TIER 0x020 /* 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 */
> > - u8 res1[0x10];
> > - u32 tmhtcrh; /* High Temperature Capture Register */
> > - u32 tmhtcrl; /* Low Temperature Capture Register */
> > - u8 res2[0x8];
> > - u32 tmhtitr; /* High Temperature Immediate Threshold */
> > - u32 tmhtatr; /* High Temperature Average Threshold */
> > - u32 tmhtactr; /* High Temperature Average Crit Threshold */
> > - u8 res3[0x24];
> > - u32 ttcfgr; /* Temperature Configuration Register */
> > - u32 tscfgr; /* Sensor Configuration Register */
> > - u8 res4[0x78];
> > - struct qoriq_tmu_site_regs site[SITES_MAX];
> > - u8 res5[0x9f8];
> > - 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 */
> > -};
> >
> > +#define REGS_TTCFGR 0x080 /* Temperature Configuration Register */
> > +#define REGS_TSCFGR 0x084 /* Sensor Configuration Register */
> > +
> > +#define REGS_TRITSR(n) (0x100 + 16 * (n)) /* Immediate Temperature
> > + * Site Register
> > + */
> > +#define REGS_TTRnCR(n) (0xf10 + 4 * (n)) /* Temperature Range n
> > + * Control Register
> > + */
> > /*
> > * Thermal zone data
> > */
> > @@ -67,8 +47,7 @@ struct qoriq_sensor {
> > };
> >
> > struct qoriq_tmu_data {
> > - struct qoriq_tmu_regs __iomem *regs;
> > - bool little_endian;
> > + struct regmap *regmap;
> > struct qoriq_sensor sensor[SITES_MAX];
> > };
> >
> > @@ -77,29 +56,13 @@ static struct qoriq_tmu_data *qoriq_sensor_to_data(struct qoriq_sensor *s)
> > return container_of(s, struct qoriq_tmu_data, sensor[s->id]);
> > }
> >
> > -static void tmu_write(struct qoriq_tmu_data *p, u32 val, void __iomem *addr)
> > -{
> > - if (p->little_endian)
> > - iowrite32(val, addr);
> > - else
> > - iowrite32be(val, addr);
> > -}
> > -
> > -static u32 tmu_read(struct qoriq_tmu_data *p, void __iomem *addr)
> > -{
> > - if (p->little_endian)
> > - return ioread32(addr);
> > - else
> > - return ioread32be(addr);
> > -}
> > -
> > static int tmu_get_temp(void *p, int *temp)
> > {
> > struct qoriq_sensor *qsensor = p;
> > struct qoriq_tmu_data *qdata = qoriq_sensor_to_data(qsensor);
> > u32 val;
> >
> > - val = tmu_read(qdata, &qdata->regs->site[qsensor->id].tritsr);
> > + regmap_read(qdata->regmap, REGS_TRITSR(qsensor->id), &val);
> > *temp = (val & 0xff) * 1000;
> >
> > return 0;
> > @@ -134,7 +97,8 @@ static int qoriq_tmu_register_tmu_zone(struct device *dev,
> >
> > /* Enable monitoring */
> > if (sites != 0)
> > - tmu_write(qdata, sites | TMR_ME | TMR_ALPF, &qdata->regs->tmr);
> > + regmap_write(qdata->regmap, REGS_TMR,
> > + sites | TMR_ME | TMR_ALPF);
> >
> > return 0;
> > }
> > @@ -153,10 +117,8 @@ static int qoriq_tmu_calibration(struct device *dev,
> > }
> >
> > /* 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);
> > + for (i = 0; i < ARRAY_SIZE(range); i++)
> > + regmap_write(data->regmap, REGS_TTRnCR(i), range[i]);
> >
> > calibration = of_get_property(np, "fsl,tmu-calibration", &len);
> > if (calibration == NULL || len % 8) {
> > @@ -166,9 +128,9 @@ static int qoriq_tmu_calibration(struct device *dev,
> >
> > for (i = 0; i < len; i += 8, calibration += 2) {
> > val = of_read_number(calibration, 1);
> > - tmu_write(data, val, &data->regs->ttcfgr);
> > + regmap_write(data->regmap, REGS_TTCFGR, val);
> > val = of_read_number(calibration + 1, 1);
> > - tmu_write(data, val, &data->regs->tscfgr);
> > + regmap_write(data->regmap, REGS_TSCFGR, val);
> > }
> >
> > return 0;
> > @@ -177,15 +139,32 @@ static int qoriq_tmu_calibration(struct device *dev,
> > static void qoriq_tmu_init_device(struct qoriq_tmu_data *data)
> > {
> > /* Disable interrupt, using polling instead */
> > - tmu_write(data, TIER_DISABLE, &data->regs->tier);
> > + regmap_write(data->regmap, REGS_TIER, TIER_DISABLE);
> >
> > /* Set update_interval */
> > - tmu_write(data, TMTMIR_DEFAULT, &data->regs->tmtmir);
> > + regmap_write(data->regmap, REGS_TMTMIR, TMTMIR_DEFAULT);
> >
> > /* Disable monitoring */
> > - tmu_write(data, TMR_DISABLE, &data->regs->tmr);
> > + regmap_write(data->regmap, REGS_TMR, TMR_DISABLE);
> > }
> >
> > +static const struct regmap_range qiriq_yes_ranges[] = {
> > + regmap_reg_range(REGS_TMR, REGS_TSCFGR),
> > + regmap_reg_range(REGS_TTRnCR(0), REGS_TTRnCR(3)),
> > + /* Read only registers below */
> > + regmap_reg_range(REGS_TRITSR(0), REGS_TRITSR(15)),
> > +};
> > +
> > +static const struct regmap_access_table qiriq_wr_table = {
> > + .yes_ranges = qiriq_yes_ranges,
> > + .n_yes_ranges = ARRAY_SIZE(qiriq_yes_ranges) - 1,
> > +};
> > +
> > +static const struct regmap_access_table qiriq_rd_table = {
> > + .yes_ranges = qiriq_yes_ranges,
> > + .n_yes_ranges = ARRAY_SIZE(qiriq_yes_ranges),
> > +};
>
> As the table are the same, it would make sense to fold both structure to
> a single one (and s/qiriq/qoriq/ ?)
>
Writable registers are a subset of readable registers and n_yes_ranges
is different between the two. Not sure how I could fold the two into a
single struct. Will definitely fix the typo.
Thanks,
Andrey Smirnov
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH v3 09/13] thermal: qoriq: Convert driver to use regmap API
2019-04-05 18:24 ` Andrey Smirnov
@ 2019-04-11 13:00 ` Daniel Lezcano
0 siblings, 0 replies; 42+ messages in thread
From: Daniel Lezcano @ 2019-04-11 13:00 UTC (permalink / raw)
To: Andrey Smirnov
Cc: linux-pm, Chris Healy, Lucas Stach, Zhang Rui, Eduardo Valentin,
Angus Ainslie, dl-linux-imx, linux-kernel
On 05/04/2019 20:24, Andrey Smirnov wrote:
[ ... ]
>>> +static const struct regmap_range qiriq_yes_ranges[] = {
>>> + regmap_reg_range(REGS_TMR, REGS_TSCFGR),
>>> + regmap_reg_range(REGS_TTRnCR(0), REGS_TTRnCR(3)),
>>> + /* Read only registers below */
>>> + regmap_reg_range(REGS_TRITSR(0), REGS_TRITSR(15)),
>>> +};
>>> +
>>> +static const struct regmap_access_table qiriq_wr_table = {
>>> + .yes_ranges = qiriq_yes_ranges,
>>> + .n_yes_ranges = ARRAY_SIZE(qiriq_yes_ranges) - 1,
>>> +};
>>> +
>>> +static const struct regmap_access_table qiriq_rd_table = {
>>> + .yes_ranges = qiriq_yes_ranges,
>>> + .n_yes_ranges = ARRAY_SIZE(qiriq_yes_ranges),
>>> +};
>>
>> As the table are the same, it would make sense to fold both structure to
>> a single one (and s/qiriq/qoriq/ ?)
>>
>
> Writable registers are a subset of readable registers and n_yes_ranges
> is different between the two. Not sure how I could fold the two into a
> single struct.
Right, never mind my comment.
--
<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
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v3 10/13] thermal: qoriq: Enable all sensors before registering them
2019-04-01 4:14 [PATCH v3 00/13] QorIQ TMU multi-sensor and HWMON support Andrey Smirnov
` (8 preceding siblings ...)
2019-04-01 4:14 ` [PATCH v3 09/13] thermal: qoriq: Convert driver to use regmap API Andrey Smirnov
@ 2019-04-01 4:14 ` Andrey Smirnov
2019-04-04 9:08 ` Daniel Lezcano
2019-04-01 4:14 ` [PATCH v3 11/13] thermal: qoriq: Do not report invalid temperature reading Andrey Smirnov
` (2 subsequent siblings)
12 siblings, 1 reply; 42+ messages in thread
From: Andrey Smirnov @ 2019-04-01 4:14 UTC (permalink / raw)
To: linux-pm
Cc: Andrey Smirnov, Chris Healy, Lucas Stach, Zhang Rui,
Eduardo Valentin, Daniel Lezcano, Angus Ainslie, linux-imx,
linux-kernel
Tmu_get_temp will get called as a part of sensor registration via
devm_thermal_zone_of_sensor_register(). To prevent it from retruning
bogus data we need to enable sensor monitoring before that. Looking at
the datasheet (i.MX8MQ RM) there doesn't seem to be any harm in
enabling them all, so, for the sake of simplicity, change the code to
do just that.
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Angus Ainslie (Purism) <angus@akkea.ca>
Cc: linux-imx@nxp.com
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
drivers/thermal/qoriq_thermal.c | 27 ++++++++++++++-------------
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index a909acee4354..7ff93dfcd68b 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -23,6 +23,7 @@
#define TMR_DISABLE 0x0
#define TMR_ME 0x80000000
#define TMR_ALPF 0x0c000000
+#define TMR_MSITE_ALL GENMASK(15, 0)
#define REGS_TMTMIR 0x008 /* Temperature measurement interval Register */
#define TMTMIR_DEFAULT 0x0000000f
@@ -75,7 +76,10 @@ static const struct thermal_zone_of_device_ops tmu_tz_ops = {
static int qoriq_tmu_register_tmu_zone(struct device *dev,
struct qoriq_tmu_data *qdata)
{
- int id, sites = 0;
+ int id, ret;
+
+ regmap_write(qdata->regmap, REGS_TMR,
+ TMR_MSITE_ALL | TMR_ME | TMR_ALPF);
for (id = 0; id < SITES_MAX; id++) {
struct thermal_zone_device *tzd;
@@ -85,21 +89,18 @@ static int qoriq_tmu_register_tmu_zone(struct device *dev,
tzd = devm_thermal_zone_of_sensor_register(dev, id,
s, &tmu_tz_ops);
- if (IS_ERR(tzd)) {
- if (PTR_ERR(tzd) == -ENODEV)
- continue;
- else
- return PTR_ERR(tzd);
+ ret = PTR_ERR_OR_ZERO(tzd);
+ switch (ret) {
+ case -ENODEV:
+ continue;
+ case 0:
+ break;
+ default:
+ regmap_write(qdata->regmap, REGS_TMR, TMR_DISABLE);
+ return ret;
}
-
- sites |= 0x1 << (15 - id);
}
- /* Enable monitoring */
- if (sites != 0)
- regmap_write(qdata->regmap, REGS_TMR,
- sites | TMR_ME | TMR_ALPF);
-
return 0;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v3 10/13] thermal: qoriq: Enable all sensors before registering them
2019-04-01 4:14 ` [PATCH v3 10/13] thermal: qoriq: Enable all sensors before registering them Andrey Smirnov
@ 2019-04-04 9:08 ` Daniel Lezcano
0 siblings, 0 replies; 42+ messages in thread
From: Daniel Lezcano @ 2019-04-04 9:08 UTC (permalink / raw)
To: Andrey Smirnov, linux-pm
Cc: Chris Healy, Lucas Stach, Zhang Rui, Eduardo Valentin,
Angus Ainslie, linux-imx, linux-kernel
On 01/04/2019 06:14, Andrey Smirnov wrote:
> Tmu_get_temp will get called as a part of sensor registration via
> devm_thermal_zone_of_sensor_register(). To prevent it from retruning
> bogus data we need to enable sensor monitoring before that. Looking at
> the datasheet (i.MX8MQ RM) there doesn't seem to be any harm in
> enabling them all, so, for the sake of simplicity, change the code to
> do just that.
>
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> Cc: Chris Healy <cphealy@gmail.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Zhang Rui <rui.zhang@intel.com>
> Cc: Eduardo Valentin <edubezval@gmail.com>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Angus Ainslie (Purism) <angus@akkea.ca>
> Cc: linux-imx@nxp.com
> Cc: linux-pm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> drivers/thermal/qoriq_thermal.c | 27 ++++++++++++++-------------
> 1 file changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
> index a909acee4354..7ff93dfcd68b 100644
> --- a/drivers/thermal/qoriq_thermal.c
> +++ b/drivers/thermal/qoriq_thermal.c
> @@ -23,6 +23,7 @@
> #define TMR_DISABLE 0x0
> #define TMR_ME 0x80000000
> #define TMR_ALPF 0x0c000000
> +#define TMR_MSITE_ALL GENMASK(15, 0)
>
> #define REGS_TMTMIR 0x008 /* Temperature measurement interval Register */
> #define TMTMIR_DEFAULT 0x0000000f
> @@ -75,7 +76,10 @@ static const struct thermal_zone_of_device_ops tmu_tz_ops = {
> static int qoriq_tmu_register_tmu_zone(struct device *dev,
> struct qoriq_tmu_data *qdata)
> {
> - int id, sites = 0;
> + int id, ret;
> +
> + regmap_write(qdata->regmap, REGS_TMR,
> + TMR_MSITE_ALL | TMR_ME | TMR_ALPF);
>
> for (id = 0; id < SITES_MAX; id++) {
> struct thermal_zone_device *tzd;
> @@ -85,21 +89,18 @@ static int qoriq_tmu_register_tmu_zone(struct device *dev,
>
> tzd = devm_thermal_zone_of_sensor_register(dev, id,
> s, &tmu_tz_ops);
> - if (IS_ERR(tzd)) {
> - if (PTR_ERR(tzd) == -ENODEV)
> - continue;
> - else
> - return PTR_ERR(tzd);
> + ret = PTR_ERR_OR_ZERO(tzd);
> + switch (ret) {
> + case -ENODEV:
> + continue;
> + case 0:
> + break;
> + default:
> + regmap_write(qdata->regmap, REGS_TMR, TMR_DISABLE);
> + return ret;
> }
> -
> - sites |= 0x1 << (15 - id);
> }
>
> - /* Enable monitoring */
> - if (sites != 0)
> - regmap_write(qdata->regmap, REGS_TMR,
> - sites | TMR_ME | TMR_ALPF);
> -
> return 0;
> }
>
>
--
<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
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v3 11/13] thermal: qoriq: Do not report invalid temperature reading
2019-04-01 4:14 [PATCH v3 00/13] QorIQ TMU multi-sensor and HWMON support Andrey Smirnov
` (9 preceding siblings ...)
2019-04-01 4:14 ` [PATCH v3 10/13] thermal: qoriq: Enable all sensors before registering them Andrey Smirnov
@ 2019-04-01 4:14 ` Andrey Smirnov
2019-04-04 9:05 ` Daniel Lezcano
2019-04-01 4:14 ` [PATCH v3 12/13] thermal_hwmon: Add devres wrapper for thermal_add_hwmon_sysfs() Andrey Smirnov
2019-04-01 4:14 ` [PATCH v3 13/13] thermal: qoriq: Add hwmon support Andrey Smirnov
12 siblings, 1 reply; 42+ messages in thread
From: Andrey Smirnov @ 2019-04-01 4:14 UTC (permalink / raw)
To: linux-pm
Cc: Andrey Smirnov, Chris Healy, Lucas Stach, Zhang Rui,
Eduardo Valentin, Daniel Lezcano, Angus Ainslie, linux-imx,
linux-kernel
Before returning measured temperature data to upper layer we need to
make sure that the reading was marked as "valid" to avoid reporting
bogus data.
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Angus Ainslie (Purism) <angus@akkea.ca>
Cc: linux-imx@nxp.com
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
drivers/thermal/qoriq_thermal.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 7ff93dfcd68b..9d227654f879 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -37,6 +37,7 @@
#define REGS_TRITSR(n) (0x100 + 16 * (n)) /* Immediate Temperature
* Site Register
*/
+#define TRITSR_V BIT(31)
#define REGS_TTRnCR(n) (0xf10 + 4 * (n)) /* Temperature Range n
* Control Register
*/
@@ -62,10 +63,18 @@ static int tmu_get_temp(void *p, int *temp)
struct qoriq_sensor *qsensor = p;
struct qoriq_tmu_data *qdata = qoriq_sensor_to_data(qsensor);
u32 val;
+ int ret;
- regmap_read(qdata->regmap, REGS_TRITSR(qsensor->id), &val);
- *temp = (val & 0xff) * 1000;
+ ret = regmap_read_poll_timeout(qdata->regmap,
+ REGS_TRITSR(qsensor->id),
+ val,
+ val & TRITSR_V,
+ USEC_PER_MSEC,
+ 10 * USEC_PER_MSEC);
+ if (ret)
+ return ret;
+ *temp = (val & 0xff) * 1000;
return 0;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v3 11/13] thermal: qoriq: Do not report invalid temperature reading
2019-04-01 4:14 ` [PATCH v3 11/13] thermal: qoriq: Do not report invalid temperature reading Andrey Smirnov
@ 2019-04-04 9:05 ` Daniel Lezcano
2019-04-05 18:30 ` Andrey Smirnov
0 siblings, 1 reply; 42+ messages in thread
From: Daniel Lezcano @ 2019-04-04 9:05 UTC (permalink / raw)
To: Andrey Smirnov, linux-pm
Cc: Chris Healy, Lucas Stach, Zhang Rui, Eduardo Valentin,
Angus Ainslie, linux-imx, linux-kernel
On 01/04/2019 06:14, Andrey Smirnov wrote:
> Before returning measured temperature data to upper layer we need to
> make sure that the reading was marked as "valid" to avoid reporting
> bogus data.
Is it possible to add a comment above the read_poll to describe the
layout of the register ? That will help to understand the valid bit.
Is the valid bit reset after reading the value?
Other than that, Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> Cc: Chris Healy <cphealy@gmail.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Zhang Rui <rui.zhang@intel.com>
> Cc: Eduardo Valentin <edubezval@gmail.com>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Angus Ainslie (Purism) <angus@akkea.ca>
> Cc: linux-imx@nxp.com
> Cc: linux-pm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
> drivers/thermal/qoriq_thermal.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
> index 7ff93dfcd68b..9d227654f879 100644
> --- a/drivers/thermal/qoriq_thermal.c
> +++ b/drivers/thermal/qoriq_thermal.c
> @@ -37,6 +37,7 @@
> #define REGS_TRITSR(n) (0x100 + 16 * (n)) /* Immediate Temperature
> * Site Register
> */
> +#define TRITSR_V BIT(31)
> #define REGS_TTRnCR(n) (0xf10 + 4 * (n)) /* Temperature Range n
> * Control Register
> */
> @@ -62,10 +63,18 @@ static int tmu_get_temp(void *p, int *temp)
> struct qoriq_sensor *qsensor = p;
> struct qoriq_tmu_data *qdata = qoriq_sensor_to_data(qsensor);
> u32 val;
> + int ret;
>
> - regmap_read(qdata->regmap, REGS_TRITSR(qsensor->id), &val);
> - *temp = (val & 0xff) * 1000;
> + ret = regmap_read_poll_timeout(qdata->regmap,
> + REGS_TRITSR(qsensor->id),
> + val,
> + val & TRITSR_V,
> + USEC_PER_MSEC,
> + 10 * USEC_PER_MSEC);
> + if (ret)
> + return ret;
>
> + *temp = (val & 0xff) * 1000;
> return 0;
> }
>
>
--
<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
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 11/13] thermal: qoriq: Do not report invalid temperature reading
2019-04-04 9:05 ` Daniel Lezcano
@ 2019-04-05 18:30 ` Andrey Smirnov
0 siblings, 0 replies; 42+ messages in thread
From: Andrey Smirnov @ 2019-04-05 18:30 UTC (permalink / raw)
To: Daniel Lezcano
Cc: linux-pm, Chris Healy, Lucas Stach, Zhang Rui, Eduardo Valentin,
Angus Ainslie, dl-linux-imx, linux-kernel
On Thu, Apr 4, 2019 at 2:05 AM Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>
> On 01/04/2019 06:14, Andrey Smirnov wrote:
> > Before returning measured temperature data to upper layer we need to
> > make sure that the reading was marked as "valid" to avoid reporting
> > bogus data.
>
> Is it possible to add a comment above the read_poll to describe the
> layout of the register ? That will help to understand the valid bit.
>
Sure, will do.
> Is the valid bit reset after reading the value?
>
It works a bit differently. Here's the description form the datasheet:
Valid measured temperature.
0 Not valid. Temperature out of sensor range or first
measurement still pending.
1 Valid.
Thanks,
Andrey Smirnov
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v3 12/13] thermal_hwmon: Add devres wrapper for thermal_add_hwmon_sysfs()
2019-04-01 4:14 [PATCH v3 00/13] QorIQ TMU multi-sensor and HWMON support Andrey Smirnov
` (10 preceding siblings ...)
2019-04-01 4:14 ` [PATCH v3 11/13] thermal: qoriq: Do not report invalid temperature reading Andrey Smirnov
@ 2019-04-01 4:14 ` Andrey Smirnov
2019-04-04 9:10 ` Daniel Lezcano
2019-04-01 4:14 ` [PATCH v3 13/13] thermal: qoriq: Add hwmon support Andrey Smirnov
12 siblings, 1 reply; 42+ messages in thread
From: Andrey Smirnov @ 2019-04-01 4:14 UTC (permalink / raw)
To: linux-pm
Cc: Andrey Smirnov, Chris Healy, Lucas Stach, Zhang Rui,
Eduardo Valentin, Daniel Lezcano, Angus Ainslie, linux-imx,
linux-kernel
Add devres wrapper for thermal_add_hwmon_sysfs() to simplify driver
code.
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Angus Ainslie (Purism) <angus@akkea.ca>
Cc: linux-imx@nxp.com
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
drivers/thermal/thermal_hwmon.c | 28 ++++++++++++++++++++++++++++
drivers/thermal/thermal_hwmon.h | 7 +++++++
2 files changed, 35 insertions(+)
diff --git a/drivers/thermal/thermal_hwmon.c b/drivers/thermal/thermal_hwmon.c
index 40c69a533b24..4e79524182e1 100644
--- a/drivers/thermal/thermal_hwmon.c
+++ b/drivers/thermal/thermal_hwmon.c
@@ -244,3 +244,31 @@ void thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz)
kfree(hwmon);
}
EXPORT_SYMBOL_GPL(thermal_remove_hwmon_sysfs);
+
+static void devm_thermal_hwmon_release(struct device *dev, void *res)
+{
+ thermal_remove_hwmon_sysfs(*(struct thermal_zone_device **)res);
+}
+
+int devm_thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
+{
+ struct thermal_zone_device **ptr;
+ int ret;
+
+ ptr = devres_alloc(devm_thermal_hwmon_release, sizeof(*ptr),
+ GFP_KERNEL);
+ if (!ptr)
+ return -ENOMEM;
+
+ ret = thermal_add_hwmon_sysfs(tz);
+ if (ret) {
+ devres_free(ptr);
+ return ret;
+ }
+
+ *ptr = tz;
+ devres_add(&tz->device, ptr);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(devm_thermal_add_hwmon_sysfs);
diff --git a/drivers/thermal/thermal_hwmon.h b/drivers/thermal/thermal_hwmon.h
index a160b9d62dd0..1a9d65f6a6a8 100644
--- a/drivers/thermal/thermal_hwmon.h
+++ b/drivers/thermal/thermal_hwmon.h
@@ -17,6 +17,7 @@
#ifdef CONFIG_THERMAL_HWMON
int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz);
+int devm_thermal_add_hwmon_sysfs(struct thermal_zone_device *tz);
void thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz);
#else
static inline int
@@ -25,6 +26,12 @@ thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
return 0;
}
+static inline int
+devm_thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
+{
+ return 0;
+}
+
static inline void
thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz)
{
--
2.20.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v3 12/13] thermal_hwmon: Add devres wrapper for thermal_add_hwmon_sysfs()
2019-04-01 4:14 ` [PATCH v3 12/13] thermal_hwmon: Add devres wrapper for thermal_add_hwmon_sysfs() Andrey Smirnov
@ 2019-04-04 9:10 ` Daniel Lezcano
0 siblings, 0 replies; 42+ messages in thread
From: Daniel Lezcano @ 2019-04-04 9:10 UTC (permalink / raw)
To: Andrey Smirnov, linux-pm
Cc: Chris Healy, Lucas Stach, Zhang Rui, Eduardo Valentin,
Angus Ainslie, linux-imx, linux-kernel
On 01/04/2019 06:14, Andrey Smirnov wrote:
> Add devres wrapper for thermal_add_hwmon_sysfs() to simplify driver
> code.
>
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> Cc: Chris Healy <cphealy@gmail.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Zhang Rui <rui.zhang@intel.com>
> Cc: Eduardo Valentin <edubezval@gmail.com>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Angus Ainslie (Purism) <angus@akkea.ca>
> Cc: linux-imx@nxp.com
> Cc: linux-pm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
> drivers/thermal/thermal_hwmon.c | 28 ++++++++++++++++++++++++++++
> drivers/thermal/thermal_hwmon.h | 7 +++++++
> 2 files changed, 35 insertions(+)
>
> diff --git a/drivers/thermal/thermal_hwmon.c b/drivers/thermal/thermal_hwmon.c
> index 40c69a533b24..4e79524182e1 100644
> --- a/drivers/thermal/thermal_hwmon.c
> +++ b/drivers/thermal/thermal_hwmon.c
> @@ -244,3 +244,31 @@ void thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz)
> kfree(hwmon);
> }
> EXPORT_SYMBOL_GPL(thermal_remove_hwmon_sysfs);
> +
> +static void devm_thermal_hwmon_release(struct device *dev, void *res)
> +{
> + thermal_remove_hwmon_sysfs(*(struct thermal_zone_device **)res);
> +}
> +
> +int devm_thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
> +{
> + struct thermal_zone_device **ptr;
> + int ret;
> +
> + ptr = devres_alloc(devm_thermal_hwmon_release, sizeof(*ptr),
> + GFP_KERNEL);
> + if (!ptr)
> + return -ENOMEM;
> +
> + ret = thermal_add_hwmon_sysfs(tz);
> + if (ret) {
> + devres_free(ptr);
> + return ret;
> + }
> +
> + *ptr = tz;
> + devres_add(&tz->device, ptr);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(devm_thermal_add_hwmon_sysfs);
> diff --git a/drivers/thermal/thermal_hwmon.h b/drivers/thermal/thermal_hwmon.h
> index a160b9d62dd0..1a9d65f6a6a8 100644
> --- a/drivers/thermal/thermal_hwmon.h
> +++ b/drivers/thermal/thermal_hwmon.h
> @@ -17,6 +17,7 @@
>
> #ifdef CONFIG_THERMAL_HWMON
> int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz);
> +int devm_thermal_add_hwmon_sysfs(struct thermal_zone_device *tz);
> void thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz);
> #else
> static inline int
> @@ -25,6 +26,12 @@ thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
> return 0;
> }
>
> +static inline int
> +devm_thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
> +{
> + return 0;
> +}
> +
> static inline void
> thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz)
> {
>
--
<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
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v3 13/13] thermal: qoriq: Add hwmon support
2019-04-01 4:14 [PATCH v3 00/13] QorIQ TMU multi-sensor and HWMON support Andrey Smirnov
` (11 preceding siblings ...)
2019-04-01 4:14 ` [PATCH v3 12/13] thermal_hwmon: Add devres wrapper for thermal_add_hwmon_sysfs() Andrey Smirnov
@ 2019-04-01 4:14 ` Andrey Smirnov
2019-04-04 9:11 ` Daniel Lezcano
12 siblings, 1 reply; 42+ messages in thread
From: Andrey Smirnov @ 2019-04-01 4:14 UTC (permalink / raw)
To: linux-pm
Cc: Andrey Smirnov, Chris Healy, Lucas Stach, Zhang Rui,
Eduardo Valentin, Daniel Lezcano, Angus Ainslie, linux-imx,
linux-kernel
Expose thermal readings as a HWMON device, so that it could be
accessed using lm-sensors.
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Angus Ainslie (Purism) <angus@akkea.ca>
Cc: linux-imx@nxp.com
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
drivers/thermal/qoriq_thermal.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 9d227654f879..7fb9321a0d8c 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -12,6 +12,7 @@
#include <linux/thermal.h>
#include "thermal_core.h"
+#include "thermal_hwmon.h"
#define SITES_MAX 16
@@ -103,7 +104,10 @@ static int qoriq_tmu_register_tmu_zone(struct device *dev,
case -ENODEV:
continue;
case 0:
- break;
+ ret = devm_thermal_add_hwmon_sysfs(tzd);
+ if (!ret)
+ break;
+ /* fallthrough */
default:
regmap_write(qdata->regmap, REGS_TMR, TMR_DISABLE);
return ret;
--
2.20.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v3 13/13] thermal: qoriq: Add hwmon support
2019-04-01 4:14 ` [PATCH v3 13/13] thermal: qoriq: Add hwmon support Andrey Smirnov
@ 2019-04-04 9:11 ` Daniel Lezcano
2019-04-05 18:38 ` Andrey Smirnov
0 siblings, 1 reply; 42+ messages in thread
From: Daniel Lezcano @ 2019-04-04 9:11 UTC (permalink / raw)
To: Andrey Smirnov, linux-pm
Cc: Chris Healy, Lucas Stach, Zhang Rui, Eduardo Valentin,
Angus Ainslie, linux-imx, linux-kernel
On 01/04/2019 06:14, Andrey Smirnov wrote:
> Expose thermal readings as a HWMON device, so that it could be
> accessed using lm-sensors.
>
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> Cc: Chris Healy <cphealy@gmail.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Zhang Rui <rui.zhang@intel.com>
> Cc: Eduardo Valentin <edubezval@gmail.com>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Angus Ainslie (Purism) <angus@akkea.ca>
> Cc: linux-imx@nxp.com
> Cc: linux-pm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
> drivers/thermal/qoriq_thermal.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
> index 9d227654f879..7fb9321a0d8c 100644
> --- a/drivers/thermal/qoriq_thermal.c
> +++ b/drivers/thermal/qoriq_thermal.c
> @@ -12,6 +12,7 @@
> #include <linux/thermal.h>
>
> #include "thermal_core.h"
> +#include "thermal_hwmon.h"
>
> #define SITES_MAX 16
>
> @@ -103,7 +104,10 @@ static int qoriq_tmu_register_tmu_zone(struct device *dev,
> case -ENODEV:
> continue;
> case 0:
> - break;
> + ret = devm_thermal_add_hwmon_sysfs(tzd);
> + if (!ret)
> + break;
> + /* fallthrough */
Do we really want to disable the thermal zone if the hwmon fails to
register ?
> default:
> regmap_write(qdata->regmap, REGS_TMR, TMR_DISABLE);
> return ret;
>
--
<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
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 13/13] thermal: qoriq: Add hwmon support
2019-04-04 9:11 ` Daniel Lezcano
@ 2019-04-05 18:38 ` Andrey Smirnov
0 siblings, 0 replies; 42+ messages in thread
From: Andrey Smirnov @ 2019-04-05 18:38 UTC (permalink / raw)
To: Daniel Lezcano
Cc: linux-pm, Chris Healy, Lucas Stach, Eduardo Valentin,
Angus Ainslie, dl-linux-imx, linux-kernel
On Thu, Apr 4, 2019 at 2:12 AM Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>
> On 01/04/2019 06:14, Andrey Smirnov wrote:
> > Expose thermal readings as a HWMON device, so that it could be
> > accessed using lm-sensors.
> >
> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> > Cc: Chris Healy <cphealy@gmail.com>
> > Cc: Lucas Stach <l.stach@pengutronix.de>
> > Cc: Zhang Rui <rui.zhang@intel.com>
> > Cc: Eduardo Valentin <edubezval@gmail.com>
> > Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> > Cc: Angus Ainslie (Purism) <angus@akkea.ca>
> > Cc: linux-imx@nxp.com
> > Cc: linux-pm@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > ---
> > drivers/thermal/qoriq_thermal.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
> > index 9d227654f879..7fb9321a0d8c 100644
> > --- a/drivers/thermal/qoriq_thermal.c
> > +++ b/drivers/thermal/qoriq_thermal.c
> > @@ -12,6 +12,7 @@
> > #include <linux/thermal.h>
> >
> > #include "thermal_core.h"
> > +#include "thermal_hwmon.h"
> >
> > #define SITES_MAX 16
> >
> > @@ -103,7 +104,10 @@ static int qoriq_tmu_register_tmu_zone(struct device *dev,
> > case -ENODEV:
> > continue;
> > case 0:
> > - break;
> > + ret = devm_thermal_add_hwmon_sysfs(tzd);
> > + if (!ret)
> > + break;
> > + /* fallthrough */
>
> Do we really want to disable the thermal zone if the hwmon fails to
> register ?
Original code already disables all thermal zones if any single one
fails to register, so I though we may as well be as strict for hwmon
registration as well. Doesn't have to be though. Will change in v4.
Thanks,
Andrey Smirnov
^ permalink raw reply [flat|nested] 42+ messages in thread