mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 1/6] drivers/thermal/k3: Convert to devm_platform_ioremap_resource()
@ 2023-06-26 12:43 Yangtao Li
  2023-06-26 12:43 ` [PATCH 2/6] thermal/drivers/k3_j72xx_bandgap: " Yangtao Li
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Yangtao Li @ 2023-06-26 12:43 UTC (permalink / raw)
  To: Rafael J. Wysocki, Daniel Lezcano, Amit Kucheria, Zhang Rui
  Cc: Yangtao Li, linux-pm, linux-kernel

Use devm_platform_ioremap_resource() to simplify code.

Signed-off-by: Yangtao Li <frank.li@vivo.com>
---
 drivers/thermal/k3_bandgap.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/thermal/k3_bandgap.c b/drivers/thermal/k3_bandgap.c
index 1c3e590157ec..dadb5d070455 100644
--- a/drivers/thermal/k3_bandgap.c
+++ b/drivers/thermal/k3_bandgap.c
@@ -160,7 +160,6 @@ static const struct of_device_id of_k3_bandgap_match[];
 static int k3_bandgap_probe(struct platform_device *pdev)
 {
 	int ret = 0, cnt, val, id;
-	struct resource *res;
 	struct device *dev = &pdev->dev;
 	struct k3_bandgap *bgp;
 	struct k3_thermal_data *data;
@@ -173,8 +172,7 @@ static int k3_bandgap_probe(struct platform_device *pdev)
 	if (!bgp)
 		return -ENOMEM;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	bgp->base = devm_ioremap_resource(dev, res);
+	bgp->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(bgp->base))
 		return PTR_ERR(bgp->base);
 
-- 
2.39.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 2/6] thermal/drivers/k3_j72xx_bandgap: Convert to devm_platform_ioremap_resource()
  2023-06-26 12:43 [PATCH 1/6] drivers/thermal/k3: Convert to devm_platform_ioremap_resource() Yangtao Li
@ 2023-06-26 12:43 ` Yangtao Li
  2023-06-26 12:43 ` [PATCH 3/6] thermal/drivers/qcom: " Yangtao Li
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Yangtao Li @ 2023-06-26 12:43 UTC (permalink / raw)
  To: Rafael J. Wysocki, Daniel Lezcano, Amit Kucheria, Zhang Rui
  Cc: Yangtao Li, linux-pm, linux-kernel

Use devm_platform_ioremap_resource() to simplify code.

Signed-off-by: Yangtao Li <frank.li@vivo.com>
---
 drivers/thermal/k3_j72xx_bandgap.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/thermal/k3_j72xx_bandgap.c b/drivers/thermal/k3_j72xx_bandgap.c
index 5be1f09eeb2c..6c82b92d86fd 100644
--- a/drivers/thermal/k3_j72xx_bandgap.c
+++ b/drivers/thermal/k3_j72xx_bandgap.c
@@ -346,7 +346,6 @@ static int k3_j72xx_bandgap_probe(struct platform_device *pdev)
 {
 	int ret = 0, cnt, val, id;
 	int high_max, low_temp;
-	struct resource *res;
 	struct device *dev = &pdev->dev;
 	struct k3_j72xx_bandgap *bgp;
 	struct k3_thermal_data *data;
@@ -376,13 +375,11 @@ static int k3_j72xx_bandgap_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	bgp->dev = dev;
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	bgp->base = devm_ioremap_resource(dev, res);
+	bgp->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(bgp->base))
 		return PTR_ERR(bgp->base);
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-	bgp->cfg2_base = devm_ioremap_resource(dev, res);
+	bgp->cfg2_base = devm_platform_ioremap_resource(pdev, 1);
 	if (IS_ERR(bgp->cfg2_base))
 		return PTR_ERR(bgp->cfg2_base);
 
@@ -400,8 +397,7 @@ static int k3_j72xx_bandgap_probe(struct platform_device *pdev)
 	 * https://www.ti.com/lit/er/sprz455c/sprz455c.pdf
 	 */
 	if (workaround_needed) {
-		res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
-		fuse_base = devm_ioremap_resource(dev, res);
+		fuse_base = devm_platform_ioremap_resource(pdev, 2);
 		if (IS_ERR(fuse_base))
 			return PTR_ERR(fuse_base);
 
-- 
2.39.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 3/6] thermal/drivers/qcom: Convert to devm_platform_ioremap_resource()
  2023-06-26 12:43 [PATCH 1/6] drivers/thermal/k3: Convert to devm_platform_ioremap_resource() Yangtao Li
  2023-06-26 12:43 ` [PATCH 2/6] thermal/drivers/k3_j72xx_bandgap: " Yangtao Li
@ 2023-06-26 12:43 ` Yangtao Li
  2023-06-26 12:43 ` [PATCH 4/6] drivers/thermal/rcar_gen3_thermal: " Yangtao Li
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Yangtao Li @ 2023-06-26 12:43 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Amit Kucheria,
	Thara Gopinath, Rafael J. Wysocki, Daniel Lezcano, Zhang Rui
  Cc: Yangtao Li, linux-pm, linux-arm-msm, linux-kernel

Use devm_platform_ioremap_resource() to simplify code.

Signed-off-by: Yangtao Li <frank.li@vivo.com>
---
 drivers/thermal/qcom/tsens.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
index 98c356acfe98..f2e67adcf4ee 100644
--- a/drivers/thermal/qcom/tsens.c
+++ b/drivers/thermal/qcom/tsens.c
@@ -890,7 +890,6 @@ int __init init_common(struct tsens_priv *priv)
 	void __iomem *tm_base, *srot_base;
 	struct device *dev = priv->dev;
 	u32 ver_minor;
-	struct resource *res;
 	u32 enabled;
 	int ret, i, j;
 	struct platform_device *op = of_find_device_by_node(priv->dev->of_node);
@@ -901,8 +900,7 @@ int __init init_common(struct tsens_priv *priv)
 	if (op->num_resources > 1) {
 		/* DT with separate SROT and TM address space */
 		priv->tm_offset = 0;
-		res = platform_get_resource(op, IORESOURCE_MEM, 1);
-		srot_base = devm_ioremap_resource(dev, res);
+		srot_base = devm_platform_ioremap_resource(op, 1);
 		if (IS_ERR(srot_base)) {
 			ret = PTR_ERR(srot_base);
 			goto err_put_device;
@@ -920,8 +918,7 @@ int __init init_common(struct tsens_priv *priv)
 	}
 
 	if (tsens_version(priv) >= VER_0_1) {
-		res = platform_get_resource(op, IORESOURCE_MEM, 0);
-		tm_base = devm_ioremap_resource(dev, res);
+		tm_base = devm_platform_ioremap_resource(op, 0);
 		if (IS_ERR(tm_base)) {
 			ret = PTR_ERR(tm_base);
 			goto err_put_device;
-- 
2.39.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 4/6] drivers/thermal/rcar_gen3_thermal: Convert to devm_platform_ioremap_resource()
  2023-06-26 12:43 [PATCH 1/6] drivers/thermal/k3: Convert to devm_platform_ioremap_resource() Yangtao Li
  2023-06-26 12:43 ` [PATCH 2/6] thermal/drivers/k3_j72xx_bandgap: " Yangtao Li
  2023-06-26 12:43 ` [PATCH 3/6] thermal/drivers/qcom: " Yangtao Li
@ 2023-06-26 12:43 ` Yangtao Li
  2023-06-26 16:28   ` Niklas Söderlund
  2023-06-26 12:43 ` [PATCH 5/6] thermal/drivers/rcar: " Yangtao Li
  2023-06-26 12:43 ` [PATCH 6/6] drivers/thermal/ti-soc-thermal: Use devm_platform_get_and_ioremap_resource() Yangtao Li
  4 siblings, 1 reply; 10+ messages in thread
From: Yangtao Li @ 2023-06-26 12:43 UTC (permalink / raw)
  To: Niklas Söderlund, Rafael J. Wysocki, Daniel Lezcano,
	Amit Kucheria, Zhang Rui
  Cc: Yangtao Li, linux-renesas-soc, linux-pm, linux-kernel

Use devm_platform_ioremap_resource() to simplify code.

Signed-off-by: Yangtao Li <frank.li@vivo.com>
---
 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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 5/6] thermal/drivers/rcar: Convert to devm_platform_ioremap_resource()
  2023-06-26 12:43 [PATCH 1/6] drivers/thermal/k3: Convert to devm_platform_ioremap_resource() Yangtao Li
                   ` (2 preceding siblings ...)
  2023-06-26 12:43 ` [PATCH 4/6] drivers/thermal/rcar_gen3_thermal: " Yangtao Li
@ 2023-06-26 12:43 ` Yangtao Li
  2023-06-26 16:32   ` Niklas Söderlund
  2023-06-26 12:43 ` [PATCH 6/6] drivers/thermal/ti-soc-thermal: Use devm_platform_get_and_ioremap_resource() Yangtao Li
  4 siblings, 1 reply; 10+ messages in thread
From: Yangtao Li @ 2023-06-26 12:43 UTC (permalink / raw)
  To: Niklas Söderlund, Rafael J. Wysocki, Daniel Lezcano,
	Amit Kucheria, Zhang Rui
  Cc: Yangtao Li, linux-renesas-soc, linux-pm, linux-kernel

Use devm_platform_ioremap_resource() to simplify code.

Signed-off-by: Yangtao Li <frank.li@vivo.com>
---
 drivers/thermal/rcar_thermal.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c
index b8571f7090aa..04a52d82022c 100644
--- a/drivers/thermal/rcar_thermal.c
+++ b/drivers/thermal/rcar_thermal.c
@@ -397,7 +397,6 @@ static int rcar_thermal_probe(struct platform_device *pdev)
 	struct rcar_thermal_common *common;
 	struct rcar_thermal_priv *priv;
 	struct device *dev = &pdev->dev;
-	struct resource *res;
 	const struct rcar_thermal_chip *chip = of_device_get_match_data(dev);
 	int mres = 0;
 	int i;
@@ -435,9 +434,7 @@ static int rcar_thermal_probe(struct platform_device *pdev)
 			 * Then, driver uses common registers
 			 * rcar_has_irq_support() will be enabled
 			 */
-			res = platform_get_resource(pdev, IORESOURCE_MEM,
-						    mres++);
-			common->base = devm_ioremap_resource(dev, res);
+			common->base = devm_platform_ioremap_resource(pdev, mres++);
 			if (IS_ERR(common->base)) {
 				ret = PTR_ERR(common->base);
 				goto error_unregister;
@@ -459,17 +456,13 @@ static int rcar_thermal_probe(struct platform_device *pdev)
 	}
 
 	for (i = 0;; i++) {
-		res = platform_get_resource(pdev, IORESOURCE_MEM, mres++);
-		if (!res)
-			break;
-
 		priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 		if (!priv) {
 			ret = -ENOMEM;
 			goto error_unregister;
 		}
 
-		priv->base = devm_ioremap_resource(dev, res);
+		priv->base = devm_platform_ioremap_resource(pdev, mres++);
 		if (IS_ERR(priv->base)) {
 			ret = PTR_ERR(priv->base);
 			goto error_unregister;
-- 
2.39.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 6/6] drivers/thermal/ti-soc-thermal: Use devm_platform_get_and_ioremap_resource()
  2023-06-26 12:43 [PATCH 1/6] drivers/thermal/k3: Convert to devm_platform_ioremap_resource() Yangtao Li
                   ` (3 preceding siblings ...)
  2023-06-26 12:43 ` [PATCH 5/6] thermal/drivers/rcar: " Yangtao Li
@ 2023-06-26 12:43 ` Yangtao Li
  4 siblings, 0 replies; 10+ messages in thread
From: Yangtao Li @ 2023-06-26 12:43 UTC (permalink / raw)
  To: Eduardo Valentin, Keerthy, Rafael J. Wysocki, Daniel Lezcano,
	Amit Kucheria, Zhang Rui
  Cc: Yangtao Li, linux-pm, linux-omap, linux-kernel

Convert platform_get_resource(), devm_ioremap_resource() to a single
call to devm_platform_get_and_ioremap_resource(), as this is exactly
what this function does.

Signed-off-by: Yangtao Li <frank.li@vivo.com>
---
 drivers/thermal/ti-soc-thermal/ti-bandgap.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.c b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
index a1c9a1530183..0066b9150920 100644
--- a/drivers/thermal/ti-soc-thermal/ti-bandgap.c
+++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
@@ -847,10 +847,9 @@ static struct ti_bandgap *ti_bandgap_build(struct platform_device *pdev)
 	do {
 		void __iomem *chunk;
 
-		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
+		chunk = devm_platform_get_and_ioremap_resource(pdev, i, &res);
 		if (!res)
 			break;
-		chunk = devm_ioremap_resource(&pdev->dev, res);
 		if (i == 0)
 			bgp->base = chunk;
 		if (IS_ERR(chunk))
-- 
2.39.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/6] drivers/thermal/rcar_gen3_thermal: Convert to devm_platform_ioremap_resource()
  2023-06-26 12:43 ` [PATCH 4/6] drivers/thermal/rcar_gen3_thermal: " Yangtao Li
@ 2023-06-26 16:28   ` Niklas Söderlund
  2023-06-27  2:58     ` Yangtao Li
  0 siblings, 1 reply; 10+ messages in thread
From: Niklas Söderlund @ 2023-06-26 16:28 UTC (permalink / raw)
  To: Yangtao Li
  Cc: Rafael J. Wysocki, Daniel Lezcano, Amit Kucheria, Zhang Rui,
	linux-renesas-soc, linux-pm, linux-kernel

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 :-)

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
> 

-- 
Kind Regards,
Niklas Söderlund

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 5/6] thermal/drivers/rcar: Convert to devm_platform_ioremap_resource()
  2023-06-26 12:43 ` [PATCH 5/6] thermal/drivers/rcar: " Yangtao Li
@ 2023-06-26 16:32   ` Niklas Söderlund
  0 siblings, 0 replies; 10+ messages in thread
From: Niklas Söderlund @ 2023-06-26 16:32 UTC (permalink / raw)
  To: Yangtao Li
  Cc: Rafael J. Wysocki, Daniel Lezcano, Amit Kucheria, Zhang Rui,
	linux-renesas-soc, linux-pm, linux-kernel

Hi Yangtao,

Thanks for your work.

On 2023-06-26 20:43:32 +0800, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
> 
> Signed-off-by: Yangtao Li <frank.li@vivo.com>
> ---
>  drivers/thermal/rcar_thermal.c | 11 ++---------
>  1 file changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c
> index b8571f7090aa..04a52d82022c 100644
> --- a/drivers/thermal/rcar_thermal.c
> +++ b/drivers/thermal/rcar_thermal.c
> @@ -397,7 +397,6 @@ static int rcar_thermal_probe(struct platform_device *pdev)
>  	struct rcar_thermal_common *common;
>  	struct rcar_thermal_priv *priv;
>  	struct device *dev = &pdev->dev;
> -	struct resource *res;
>  	const struct rcar_thermal_chip *chip = of_device_get_match_data(dev);
>  	int mres = 0;
>  	int i;
> @@ -435,9 +434,7 @@ static int rcar_thermal_probe(struct platform_device *pdev)
>  			 * Then, driver uses common registers
>  			 * rcar_has_irq_support() will be enabled
>  			 */
> -			res = platform_get_resource(pdev, IORESOURCE_MEM,
> -						    mres++);
> -			common->base = devm_ioremap_resource(dev, res);
> +			common->base = devm_platform_ioremap_resource(pdev, mres++);

This change is fine, please post a separate patch for it.

>  			if (IS_ERR(common->base)) {
>  				ret = PTR_ERR(common->base);
>  				goto error_unregister;
> @@ -459,17 +456,13 @@ static int rcar_thermal_probe(struct platform_device *pdev)
>  	}
>  
>  	for (i = 0;; i++) {
> -		res = platform_get_resource(pdev, IORESOURCE_MEM, mres++);
> -		if (!res)
> -			break;

This change is not OK as it changes the behavior of the driver same as 
in patch 5/6.

Nacked-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> -
>  		priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>  		if (!priv) {
>  			ret = -ENOMEM;
>  			goto error_unregister;
>  		}
>  
> -		priv->base = devm_ioremap_resource(dev, res);
> +		priv->base = devm_platform_ioremap_resource(pdev, mres++);
>  		if (IS_ERR(priv->base)) {
>  			ret = PTR_ERR(priv->base);
>  			goto error_unregister;
> -- 
> 2.39.0
> 

-- 
Kind Regards,
Niklas Söderlund

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/6] drivers/thermal/rcar_gen3_thermal: Convert to devm_platform_ioremap_resource()
  2023-06-26 16:28   ` Niklas Söderlund
@ 2023-06-27  2:58     ` Yangtao Li
  2023-06-27  7:47       ` Niklas Söderlund
  0 siblings, 1 reply; 10+ messages in thread
From: Yangtao Li @ 2023-06-27  2:58 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: Rafael J. Wysocki, Daniel Lezcano, Amit Kucheria, Zhang Rui,
	linux-renesas-soc, linux-pm, linux-kernel

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
>>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/6] drivers/thermal/rcar_gen3_thermal: Convert to devm_platform_ioremap_resource()
  2023-06-27  2:58     ` Yangtao Li
@ 2023-06-27  7:47       ` Niklas Söderlund
  0 siblings, 0 replies; 10+ messages in thread
From: Niklas Söderlund @ 2023-06-27  7:47 UTC (permalink / raw)
  To: Yangtao Li
  Cc: Rafael J. Wysocki, Daniel Lezcano, Amit Kucheria, Zhang Rui,
	linux-renesas-soc, linux-pm, linux-kernel

Hi Yangtao,

On 2023-06-27 10:58:16 +0800, Yangtao Li wrote:
> 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?

Maybe it could, I would need to test it and I'm traveling this week with 
no access to hardware. But I don't like the change you propose, as it 
makes the code more complex without any other goal then to 
s/platform_get_resource/devm_platform_ioremap_resource/.

If you have a reason, like trying to remove platform_get_resource() from 
the kernel or such I will consider the change. But if you only want to 
change things *because* I think the current code do the right thing in a 
clear way, look for resource, if found map it else use what resources 
have been found already. Adding special case based on return code is 
IMHO more complex, again if you need to go that route please add a 
comment describing the special case.

> 
> 
> 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
> > > 

-- 
Kind Regards,
Niklas Söderlund

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-06-27  7:47 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-26 12:43 [PATCH 1/6] drivers/thermal/k3: Convert to devm_platform_ioremap_resource() Yangtao Li
2023-06-26 12:43 ` [PATCH 2/6] thermal/drivers/k3_j72xx_bandgap: " Yangtao Li
2023-06-26 12:43 ` [PATCH 3/6] thermal/drivers/qcom: " Yangtao Li
2023-06-26 12:43 ` [PATCH 4/6] drivers/thermal/rcar_gen3_thermal: " Yangtao Li
2023-06-26 16:28   ` Niklas Söderlund
2023-06-27  2:58     ` Yangtao Li
2023-06-27  7:47       ` Niklas Söderlund
2023-06-26 12:43 ` [PATCH 5/6] thermal/drivers/rcar: " Yangtao Li
2023-06-26 16:32   ` Niklas Söderlund
2023-06-26 12:43 ` [PATCH 6/6] drivers/thermal/ti-soc-thermal: Use devm_platform_get_and_ioremap_resource() Yangtao Li

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®