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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxx>
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@xxxxxxxxxxxx>

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




[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux