Re: [PATCH 4/6] drivers/thermal/rcar_gen3_thermal: Convert to devm_platform_ioremap_resource()

[Yangtao Li <frank.li@vivo.com>]

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@vivo.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@ragnatech.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
>>