[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2f33ff1e-ec4e-e060-a84d-ee38ed17c9f7@vivo.com>
Date: Tue, 27 Jun 2023 10:58:16 +0800
From: Yangtao Li <frank.li@...o.com>
To: Niklas Söderlund <niklas.soderlund@...natech.se>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>,
Daniel Lezcano <daniel.lezcano@...aro.org>,
Amit Kucheria <amitk@...nel.org>,
Zhang Rui <rui.zhang@...el.com>,
linux-renesas-soc@...r.kernel.org, linux-pm@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/6] drivers/thermal/rcar_gen3_thermal: Convert to
devm_platform_ioremap_resource()
Hi Niklas,
On 2023/6/27 0:28, Niklas Söderlund wrote:
> Hi Yangtao,
>
> Thanks for your work.
>
> On 2023-06-26 20:43:31 +0800, Yangtao Li wrote:
>> Use devm_platform_ioremap_resource() to simplify code.
>>
>> Signed-off-by: Yangtao Li <frank.li@...o.com>
> This do indeed simplify the code, but it also breaks the driver :-)
How about the patch below? Can the following rcar driver also take a
similar approach?
diff --git a/drivers/thermal/rcar_gen3_thermal.c
b/drivers/thermal/rcar_gen3_thermal.c
index 9029d01e029b..0cd9a030eb9e 100644
--- a/drivers/thermal/rcar_gen3_thermal.c
+++ b/drivers/thermal/rcar_gen3_thermal.c
@@ -481,7 +481,6 @@ static int rcar_gen3_thermal_probe(struct
platform_device *pdev)
{
struct rcar_gen3_thermal_priv *priv;
struct device *dev = &pdev->dev;
- struct resource *res;
struct thermal_zone_device *zone;
unsigned int i;
int ret;
@@ -503,22 +502,23 @@ static int rcar_gen3_thermal_probe(struct
platform_device *pdev)
for (i = 0; i < TSC_MAX_NUM; i++) {
struct rcar_gen3_thermal_tsc *tsc;
+ void __iomem *base;
- res = platform_get_resource(pdev, IORESOURCE_MEM, i);
- if (!res)
- break;
+ base = devm_platform_ioremap_resource(pdev, i);
+ if (IS_ERR(base)) {
+ if (PTR_ERR(base) == -EINVAL)
+ break;
- tsc = devm_kzalloc(dev, sizeof(*tsc), GFP_KERNEL);
- if (!tsc) {
- ret = -ENOMEM;
+ ret = PTR_ERR(base);
goto error_unregister;
}
- tsc->base = devm_ioremap_resource(dev, res);
- if (IS_ERR(tsc->base)) {
- ret = PTR_ERR(tsc->base);
+ tsc = devm_kzalloc(dev, sizeof(*tsc), GFP_KERNEL);
+ if (!tsc) {
+ ret = -ENOMEM;
goto error_unregister;
}
+ tsc->base = base;
priv->tscs[i] = tsc;
}
> Before the change, failing to find a resource at position "i", breaks
> the probe loop, and probing continues and the number of resource
> described are the number of TSC find are used.
>
> After the change failing to find all possible TCS will fail the whole
> probe process, even if some TCS where described. And not describing max
> number of TCS on each system is perfectly fine.
>
> Nacked-by: Niklas Söderlund <niklas.soderlund@...natech.se>
>
>> ---
>> drivers/thermal/rcar_gen3_thermal.c | 7 +------
>> 1 file changed, 1 insertion(+), 6 deletions(-)
>>
>> diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
>> index 9029d01e029b..5c623f13d9ec 100644
>> --- a/drivers/thermal/rcar_gen3_thermal.c
>> +++ b/drivers/thermal/rcar_gen3_thermal.c
>> @@ -481,7 +481,6 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
>> {
>> struct rcar_gen3_thermal_priv *priv;
>> struct device *dev = &pdev->dev;
>> - struct resource *res;
>> struct thermal_zone_device *zone;
>> unsigned int i;
>> int ret;
>> @@ -504,17 +503,13 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
>> for (i = 0; i < TSC_MAX_NUM; i++) {
>> struct rcar_gen3_thermal_tsc *tsc;
>>
>> - res = platform_get_resource(pdev, IORESOURCE_MEM, i);
>> - if (!res)
>> - break;
>> -
>> tsc = devm_kzalloc(dev, sizeof(*tsc), GFP_KERNEL);
>> if (!tsc) {
>> ret = -ENOMEM;
>> goto error_unregister;
>> }
>>
>> - tsc->base = devm_ioremap_resource(dev, res);
>> + tsc->base = devm_platform_ioremap_resource(pdev, i);
>> if (IS_ERR(tsc->base)) {
>> ret = PTR_ERR(tsc->base);
>> goto error_unregister;
>> --
>> 2.39.0
>>
Powered by blists - more mailing lists