| Message ID | 874ng0qb9f.wl%kuninori.morimoto.gx@renesas.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Zhang Rui |
| Headers | show |
On Sun, 2013-03-24 at 22:49 -0700, Kuninori Morimoto wrote: > Current rcar_thermal driver didn't care about rcar_theraml_irq_disable() > when registration failure case on _probe(), and _remove(). > And, it returns without unregistering thermal zone when > registration failure case on _probe(). > This patch fixes these issue. > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > --- > drivers/thermal/rcar_thermal.c | 20 ++++++++++++++------ > 1 file changed, 14 insertions(+), 6 deletions(-) > > diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c > index 28f0919..359e943 100644 > --- a/drivers/thermal/rcar_thermal.c > +++ b/drivers/thermal/rcar_thermal.c > @@ -419,13 +419,15 @@ static int rcar_thermal_probe(struct platform_device *pdev) > priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > if (!priv) { > dev_err(dev, "Could not allocate priv\n"); > - return -ENOMEM; > + ret = -ENOMEM; > + goto error_unregister; > } > > priv->base = devm_request_and_ioremap(dev, res); > if (!priv->base) { > dev_err(dev, "Unable to ioremap priv register\n"); > - return -ENOMEM; > + ret = -ENOMEM; > + goto error_unregister; > } > > priv->common = common; > @@ -444,10 +446,10 @@ static int rcar_thermal_probe(struct platform_device *pdev) > goto error_unregister; > } > > - list_move_tail(&priv->list, &common->head); > - > if (rcar_has_irq_support(priv)) > rcar_thermal_irq_enable(priv); > + > + list_move_tail(&priv->list, &common->head); > } > why do you need this change for the rcar_thermal_irq_disable issue? thanks, rui > platform_set_drvdata(pdev, common); > @@ -457,8 +459,11 @@ static int rcar_thermal_probe(struct platform_device *pdev) > return 0; > > error_unregister: > - rcar_thermal_for_each_priv(priv, common) > + rcar_thermal_for_each_priv(priv, common) { > thermal_zone_device_unregister(priv->zone); > + if (rcar_has_irq_support(priv)) > + rcar_thermal_irq_disable(priv); > + } > > return -ENODEV; > } > @@ -468,8 +473,11 @@ static int rcar_thermal_remove(struct platform_device *pdev) > struct rcar_thermal_common *common = platform_get_drvdata(pdev); > struct rcar_thermal_priv *priv; > > - rcar_thermal_for_each_priv(priv, common) > + rcar_thermal_for_each_priv(priv, common) { > thermal_zone_device_unregister(priv->zone); > + if (rcar_has_irq_support(priv)) > + rcar_thermal_irq_disable(priv); > + } > > platform_set_drvdata(pdev, NULL); > -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Zhang Thank you for checking patch > > @@ -444,10 +446,10 @@ static int rcar_thermal_probe(struct platform_device *pdev) > > goto error_unregister; > > } > > > > - list_move_tail(&priv->list, &common->head); > > - > > if (rcar_has_irq_support(priv)) > > rcar_thermal_irq_enable(priv); > > + > > + list_move_tail(&priv->list, &common->head); > > } > > > why do you need this change for the rcar_thermal_irq_disable issue? I thought it is readable (or easy to understand) if it was added to the list after all the processings finish. listed priv's irq was enabled -> listed priv's irq should be disabled Best regards --- Kuninori Morimoto -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Zhang, again > Thank you for checking patch > > > > @@ -444,10 +446,10 @@ static int rcar_thermal_probe(struct platform_device *pdev) > > > goto error_unregister; > > > } > > > > > > - list_move_tail(&priv->list, &common->head); > > > - > > > if (rcar_has_irq_support(priv)) > > > rcar_thermal_irq_enable(priv); > > > + > > > + list_move_tail(&priv->list, &common->head); > > > } > > > > > why do you need this change for the rcar_thermal_irq_disable issue? > > I thought it is readable (or easy to understand) > if it was added to the list after all the processings finish. > > listed priv's irq was enabled -> listed priv's irq should be disabled I noticed that this patch set had 2 bugs Please give me 2nd chance Best regards --- Kuninori Morimoto -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Zhang
These are v2 patch set of rcar_thermal driver fixup
I noticed that v1 patch had bug.
These patch set are for linus/master branch,
and, these requires zhang/next's
f0e68fc3caf677e834f7bd0f601800e686b56c98
(thermal: rcar: fix missing unlock on error in rcar_thermal_update_temp())
fb84d9907f0ff0e3f7d70d55039ddf0f78d2a472
(thermal: rcar_thermal: propagate return value of thermal_zone_device_register)
current linus/master branch needs above 2 patches,
and these patch set are based on these.
I Cc:ed SH-ARM ML
Kuninori Morimoto (2):
thermal: rcar: tidyup registration failure case
thermal: rcar: add pm_runtime_xxx() support
drivers/thermal/rcar_thermal.c | 31 +++++++++++++++++++++++++------
1 file changed, 25 insertions(+), 6 deletions(-)
Best regards
---
Kuninori Morimoto
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2013-03-25 at 23:07 -0700, Kuninori Morimoto wrote: > Hi Zhang > > These are v2 patch set of rcar_thermal driver fixup > I noticed that v1 patch had bug. > it seems that you did not run any test for the previous patch set, not even build test. have you tried this patch set before sending out? thanks, rui > These patch set are for linus/master branch, > and, these requires zhang/next's > > f0e68fc3caf677e834f7bd0f601800e686b56c98 > (thermal: rcar: fix missing unlock on error in rcar_thermal_update_temp()) > > fb84d9907f0ff0e3f7d70d55039ddf0f78d2a472 > (thermal: rcar_thermal: propagate return value of thermal_zone_device_register) > > current linus/master branch needs above 2 patches, > and these patch set are based on these. > > I Cc:ed SH-ARM ML > > Kuninori Morimoto (2): > thermal: rcar: tidyup registration failure case > thermal: rcar: add pm_runtime_xxx() support > > drivers/thermal/rcar_thermal.c | 31 +++++++++++++++++++++++++------ > 1 file changed, 25 insertions(+), 6 deletions(-) > > Best regards > --- > Kuninori Morimoto > -- > To unsubscribe from this list: send the line "unsubscribe linux-pm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Zhang > On Mon, 2013-03-25 at 23:07 -0700, Kuninori Morimoto wrote: > > Hi Zhang > > > > These are v2 patch set of rcar_thermal driver fixup > > I noticed that v1 patch had bug. > > > it seems that you did not run any test for the previous patch set, not > even build test. Sorry about my v1 patch set. My v1 (v2 too) based on not only your branch (= your branch + SH-ML patch set) Of course I tested v1 patch set, But I missed when I git-format-patch. I'm sorry about it again > have you tried this patch set before sending out? Yes, these are tesed on r8a73a4 APE6 board Best regards --- Kuninori Morimoto -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c index 28f0919..359e943 100644 --- a/drivers/thermal/rcar_thermal.c +++ b/drivers/thermal/rcar_thermal.c @@ -419,13 +419,15 @@ static int rcar_thermal_probe(struct platform_device *pdev) priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); if (!priv) { dev_err(dev, "Could not allocate priv\n"); - return -ENOMEM; + ret = -ENOMEM; + goto error_unregister; } priv->base = devm_request_and_ioremap(dev, res); if (!priv->base) { dev_err(dev, "Unable to ioremap priv register\n"); - return -ENOMEM; + ret = -ENOMEM; + goto error_unregister; } priv->common = common; @@ -444,10 +446,10 @@ static int rcar_thermal_probe(struct platform_device *pdev) goto error_unregister; } - list_move_tail(&priv->list, &common->head); - if (rcar_has_irq_support(priv)) rcar_thermal_irq_enable(priv); + + list_move_tail(&priv->list, &common->head); } platform_set_drvdata(pdev, common); @@ -457,8 +459,11 @@ static int rcar_thermal_probe(struct platform_device *pdev) return 0; error_unregister: - rcar_thermal_for_each_priv(priv, common) + rcar_thermal_for_each_priv(priv, common) { thermal_zone_device_unregister(priv->zone); + if (rcar_has_irq_support(priv)) + rcar_thermal_irq_disable(priv); + } return -ENODEV; } @@ -468,8 +473,11 @@ static int rcar_thermal_remove(struct platform_device *pdev) struct rcar_thermal_common *common = platform_get_drvdata(pdev); struct rcar_thermal_priv *priv; - rcar_thermal_for_each_priv(priv, common) + rcar_thermal_for_each_priv(priv, common) { thermal_zone_device_unregister(priv->zone); + if (rcar_has_irq_support(priv)) + rcar_thermal_irq_disable(priv); + } platform_set_drvdata(pdev, NULL);
Current rcar_thermal driver didn't care about rcar_theraml_irq_disable() when registration failure case on _probe(), and _remove(). And, it returns without unregistering thermal zone when registration failure case on _probe(). This patch fixes these issue. Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> --- drivers/thermal/rcar_thermal.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)