| Message ID | 1522379583-4503-2-git-send-email-ykaneko0929@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Eduardo Valentin |
| Headers | show |
Hi Kaneko-san, On Fri, Mar 30, 2018 at 5:13 AM, Yoshihiro Kaneko <ykaneko0929@gmail.com> wrote: > Add support for R-Car D3 (r8a77995) thermal sensor. > > Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com> Thanks for your patch! > --- a/drivers/thermal/rcar_thermal.c > +++ b/drivers/thermal/rcar_thermal.c > @@ -58,10 +58,35 @@ struct rcar_thermal_common { > spinlock_t lock; > }; > > +enum rcar_thermal_type { > + RCAR_THERMAL, > + RCAR_GEN2_THERMAL, > + RCAR_GEN3_THERMAL, > +}; > + > +struct rcar_thermal_chip { > + int use_of_thermal; This can be a single bit: unsigned int use_of_thermal : 1; > + enum rcar_thermal_type type; If you would add feature bits, you can get rid of rcar_thermal_type: unsigned int has_filonoff : 1; unsigned int has_enr : 1; unsigned int needs_suspend_resume : 1; The number of interrupts can be stored here, too. > +}; > + > +static const struct rcar_thermal_chip rcar_thermal = { > + .use_of_thermal = 0, > + .type = RCAR_THERMAL, .has_filonoff = 1, .has_enr = 0, ... .nirqs = 1, > @@ -190,7 +222,8 @@ static int rcar_thermal_update_temp(struct rcar_thermal_priv *priv) > * enable IRQ > */ > if (rcar_has_irq_support(priv)) { > - rcar_thermal_write(priv, FILONOFF, 0); > + if (priv->chip->type != RCAR_GEN3_THERMAL) if (priv->chip->has_filonoff) > @@ -438,6 +471,9 @@ static int rcar_thermal_probe(struct platform_device *pdev) > struct rcar_thermal_priv *priv; > struct device *dev = &pdev->dev; > struct resource *res, *irq; > + struct rcar_thermal_chip *chip = ((struct rcar_thermal_chip *) I don't think the cast is needed. > @@ -457,19 +493,35 @@ static int rcar_thermal_probe(struct platform_device *pdev) > pm_runtime_enable(dev); > pm_runtime_get_sync(dev); > > - irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0); > - if (irq) { > - /* > - * platform has IRQ support. > - * 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); > - if (IS_ERR(common->base)) > - return PTR_ERR(common->base); > + for (i = 0; i < nirq; i++) { for (i = 0; i < priv->nirqs; i++) { Gr{oetje,eeting}s, Geert
Hi Geert-san, 2018-03-30 18:25 GMT+09:00 Geert Uytterhoeven <geert@linux-m68k.org>: > Hi Kaneko-san, > > On Fri, Mar 30, 2018 at 5:13 AM, Yoshihiro Kaneko <ykaneko0929@gmail.com> wrote: >> Add support for R-Car D3 (r8a77995) thermal sensor. >> >> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com> > > Thanks for your patch! > >> --- a/drivers/thermal/rcar_thermal.c >> +++ b/drivers/thermal/rcar_thermal.c >> @@ -58,10 +58,35 @@ struct rcar_thermal_common { >> spinlock_t lock; >> }; >> >> +enum rcar_thermal_type { >> + RCAR_THERMAL, >> + RCAR_GEN2_THERMAL, >> + RCAR_GEN3_THERMAL, >> +}; >> + >> +struct rcar_thermal_chip { >> + int use_of_thermal; > > This can be a single bit: > > unsigned int use_of_thermal : 1; > >> + enum rcar_thermal_type type; > > If you would add feature bits, you can get rid of rcar_thermal_type: > > unsigned int has_filonoff : 1; > unsigned int has_enr : 1; > unsigned int needs_suspend_resume : 1; > > The number of interrupts can be stored here, too. It's nice! > >> +}; >> + >> +static const struct rcar_thermal_chip rcar_thermal = { >> + .use_of_thermal = 0, >> + .type = RCAR_THERMAL, > > .has_filonoff = 1, > .has_enr = 0, > ... > .nirqs = 1, > >> @@ -190,7 +222,8 @@ static int rcar_thermal_update_temp(struct rcar_thermal_priv *priv) >> * enable IRQ >> */ >> if (rcar_has_irq_support(priv)) { >> - rcar_thermal_write(priv, FILONOFF, 0); >> + if (priv->chip->type != RCAR_GEN3_THERMAL) > > if (priv->chip->has_filonoff) > >> @@ -438,6 +471,9 @@ static int rcar_thermal_probe(struct platform_device *pdev) >> struct rcar_thermal_priv *priv; >> struct device *dev = &pdev->dev; >> struct resource *res, *irq; >> + struct rcar_thermal_chip *chip = ((struct rcar_thermal_chip *) > > I don't think the cast is needed. I will make 'chip' a const variable. > >> @@ -457,19 +493,35 @@ static int rcar_thermal_probe(struct platform_device *pdev) >> pm_runtime_enable(dev); >> pm_runtime_get_sync(dev); >> >> - irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0); >> - if (irq) { >> - /* >> - * platform has IRQ support. >> - * 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); >> - if (IS_ERR(common->base)) >> - return PTR_ERR(common->base); >> + for (i = 0; i < nirq; i++) { > > for (i = 0; i < priv->nirqs; i++) { Best regards, Kaneko > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds
Hi Kaneko-san, could you re-spin this series with Geerts concerns (below) addressed. When you repost I think you can add the tested tags and drop the RFT from the prefix. I think its likely it can then be merged. On Tue, Apr 03, 2018 at 08:17:01PM +0900, Yoshihiro Kaneko wrote: > Hi Geert-san, > > 2018-03-30 18:25 GMT+09:00 Geert Uytterhoeven <geert@linux-m68k.org>: > > Hi Kaneko-san, > > > > On Fri, Mar 30, 2018 at 5:13 AM, Yoshihiro Kaneko <ykaneko0929@gmail.com> wrote: > >> Add support for R-Car D3 (r8a77995) thermal sensor. > >> > >> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com> > > > > Thanks for your patch! > > > >> --- a/drivers/thermal/rcar_thermal.c > >> +++ b/drivers/thermal/rcar_thermal.c > >> @@ -58,10 +58,35 @@ struct rcar_thermal_common { > >> spinlock_t lock; > >> }; > >> > >> +enum rcar_thermal_type { > >> + RCAR_THERMAL, > >> + RCAR_GEN2_THERMAL, > >> + RCAR_GEN3_THERMAL, > >> +}; > >> + > >> +struct rcar_thermal_chip { > >> + int use_of_thermal; > > > > This can be a single bit: > > > > unsigned int use_of_thermal : 1; > > > >> + enum rcar_thermal_type type; > > > > If you would add feature bits, you can get rid of rcar_thermal_type: > > > > unsigned int has_filonoff : 1; > > unsigned int has_enr : 1; > > unsigned int needs_suspend_resume : 1; > > > > The number of interrupts can be stored here, too. > > It's nice! > > > > >> +}; > >> + > >> +static const struct rcar_thermal_chip rcar_thermal = { > >> + .use_of_thermal = 0, > >> + .type = RCAR_THERMAL, > > > > .has_filonoff = 1, > > .has_enr = 0, > > ... > > .nirqs = 1, > > > >> @@ -190,7 +222,8 @@ static int rcar_thermal_update_temp(struct rcar_thermal_priv *priv) > >> * enable IRQ > >> */ > >> if (rcar_has_irq_support(priv)) { > >> - rcar_thermal_write(priv, FILONOFF, 0); > >> + if (priv->chip->type != RCAR_GEN3_THERMAL) > > > > if (priv->chip->has_filonoff) > > > >> @@ -438,6 +471,9 @@ static int rcar_thermal_probe(struct platform_device *pdev) > >> struct rcar_thermal_priv *priv; > >> struct device *dev = &pdev->dev; > >> struct resource *res, *irq; > >> + struct rcar_thermal_chip *chip = ((struct rcar_thermal_chip *) > > > > I don't think the cast is needed. > > I will make 'chip' a const variable. > > > > >> @@ -457,19 +493,35 @@ static int rcar_thermal_probe(struct platform_device *pdev) > >> pm_runtime_enable(dev); > >> pm_runtime_get_sync(dev); > >> > >> - irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0); > >> - if (irq) { > >> - /* > >> - * platform has IRQ support. > >> - * 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); > >> - if (IS_ERR(common->base)) > >> - return PTR_ERR(common->base); > >> + for (i = 0; i < nirq; i++) { > > > > for (i = 0; i < priv->nirqs; i++) { > > > Best regards, > Kaneko > > > > > Gr{oetje,eeting}s, > > > > Geert > > > > -- > > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > > > In personal conversations with technical people, I call myself a hacker. But > > when I'm talking to journalists I just say "programmer" or something like that. > > -- Linus Torvalds >
Hi Simon-san, 2018-05-07 21:43 GMT+09:00 Simon Horman <horms@verge.net.au>: > Hi Kaneko-san, > > could you re-spin this series with Geerts concerns (below) addressed. > > When you repost I think you can add the tested tags and drop the RFT from > the prefix. I think its likely it can then be merged. I had posted V3 that was updated with Geert-san's suggestions. Should I repost V4 with the tested tags and without the RFT prefix? > > On Tue, Apr 03, 2018 at 08:17:01PM +0900, Yoshihiro Kaneko wrote: >> Hi Geert-san, >> >> 2018-03-30 18:25 GMT+09:00 Geert Uytterhoeven <geert@linux-m68k.org>: >> > Hi Kaneko-san, >> > >> > On Fri, Mar 30, 2018 at 5:13 AM, Yoshihiro Kaneko <ykaneko0929@gmail.com> wrote: >> >> Add support for R-Car D3 (r8a77995) thermal sensor. >> >> >> >> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com> >> > >> > Thanks for your patch! >> > >> >> --- a/drivers/thermal/rcar_thermal.c >> >> +++ b/drivers/thermal/rcar_thermal.c >> >> @@ -58,10 +58,35 @@ struct rcar_thermal_common { >> >> spinlock_t lock; >> >> }; >> >> >> >> +enum rcar_thermal_type { >> >> + RCAR_THERMAL, >> >> + RCAR_GEN2_THERMAL, >> >> + RCAR_GEN3_THERMAL, >> >> +}; >> >> + >> >> +struct rcar_thermal_chip { >> >> + int use_of_thermal; >> > >> > This can be a single bit: >> > >> > unsigned int use_of_thermal : 1; >> > >> >> + enum rcar_thermal_type type; >> > >> > If you would add feature bits, you can get rid of rcar_thermal_type: >> > >> > unsigned int has_filonoff : 1; >> > unsigned int has_enr : 1; >> > unsigned int needs_suspend_resume : 1; >> > >> > The number of interrupts can be stored here, too. >> >> It's nice! >> >> > >> >> +}; >> >> + >> >> +static const struct rcar_thermal_chip rcar_thermal = { >> >> + .use_of_thermal = 0, >> >> + .type = RCAR_THERMAL, >> > >> > .has_filonoff = 1, >> > .has_enr = 0, >> > ... >> > .nirqs = 1, >> > >> >> @@ -190,7 +222,8 @@ static int rcar_thermal_update_temp(struct rcar_thermal_priv *priv) >> >> * enable IRQ >> >> */ >> >> if (rcar_has_irq_support(priv)) { >> >> - rcar_thermal_write(priv, FILONOFF, 0); >> >> + if (priv->chip->type != RCAR_GEN3_THERMAL) >> > >> > if (priv->chip->has_filonoff) >> > >> >> @@ -438,6 +471,9 @@ static int rcar_thermal_probe(struct platform_device *pdev) >> >> struct rcar_thermal_priv *priv; >> >> struct device *dev = &pdev->dev; >> >> struct resource *res, *irq; >> >> + struct rcar_thermal_chip *chip = ((struct rcar_thermal_chip *) >> > >> > I don't think the cast is needed. >> >> I will make 'chip' a const variable. >> >> > >> >> @@ -457,19 +493,35 @@ static int rcar_thermal_probe(struct platform_device *pdev) >> >> pm_runtime_enable(dev); >> >> pm_runtime_get_sync(dev); >> >> >> >> - irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0); >> >> - if (irq) { >> >> - /* >> >> - * platform has IRQ support. >> >> - * 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); >> >> - if (IS_ERR(common->base)) >> >> - return PTR_ERR(common->base); >> >> + for (i = 0; i < nirq; i++) { >> > >> > for (i = 0; i < priv->nirqs; i++) { >> >> >> Best regards, >> Kaneko >> >> > >> > Gr{oetje,eeting}s, >> > >> > Geert >> > >> > -- >> > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org >> > >> > In personal conversations with technical people, I call myself a hacker. But >> > when I'm talking to journalists I just say "programmer" or something like that. >> > -- Linus Torvalds >>
On Wed, May 09, 2018 at 10:35:16PM +0900, Yoshihiro Kaneko wrote: > Hi Simon-san, > > 2018-05-07 21:43 GMT+09:00 Simon Horman <horms@verge.net.au>: > > Hi Kaneko-san, > > > > could you re-spin this series with Geerts concerns (below) addressed. > > > > When you repost I think you can add the tested tags and drop the RFT from > > the prefix. I think its likely it can then be merged. > > I had posted V3 that was updated with Geert-san's suggestions. > Should I repost V4 with the tested tags and without the RFT prefix? Sorry, I missed that when I wrote the above. I will respond to v3.
diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c index 73e5fee..a631bff 100644 --- a/drivers/thermal/rcar_thermal.c +++ b/drivers/thermal/rcar_thermal.c @@ -58,10 +58,35 @@ struct rcar_thermal_common { spinlock_t lock; }; +enum rcar_thermal_type { + RCAR_THERMAL, + RCAR_GEN2_THERMAL, + RCAR_GEN3_THERMAL, +}; + +struct rcar_thermal_chip { + int use_of_thermal; + enum rcar_thermal_type type; +}; + +static const struct rcar_thermal_chip rcar_thermal = { + .use_of_thermal = 0, + .type = RCAR_THERMAL, +}; +static const struct rcar_thermal_chip rcar_gen2_thermal = { + .use_of_thermal = 1, + .type = RCAR_GEN2_THERMAL, +}; +static const struct rcar_thermal_chip rcar_gen3_thermal = { + .use_of_thermal = 1, + .type = RCAR_GEN3_THERMAL, +}; + struct rcar_thermal_priv { void __iomem *base; struct rcar_thermal_common *common; struct thermal_zone_device *zone; + struct rcar_thermal_chip *chip; struct delayed_work work; struct mutex lock; struct list_head list; @@ -77,13 +102,20 @@ struct rcar_thermal_priv { #define rcar_priv_to_dev(priv) ((priv)->common->dev) #define rcar_has_irq_support(priv) ((priv)->common->base) #define rcar_id_to_shift(priv) ((priv)->id * 8) -#define rcar_of_data(dev) ((unsigned long)of_device_get_match_data(dev)) -#define rcar_use_of_thermal(dev) (rcar_of_data(dev) == USE_OF_THERMAL) -#define USE_OF_THERMAL 1 static const struct of_device_id rcar_thermal_dt_ids[] = { - { .compatible = "renesas,rcar-thermal", }, - { .compatible = "renesas,rcar-gen2-thermal", .data = (void *)USE_OF_THERMAL }, + { + .compatible = "renesas,rcar-thermal", + .data = &rcar_thermal, + }, + { + .compatible = "renesas,rcar-gen2-thermal", + .data = &rcar_gen2_thermal, + }, + { + .compatible = "renesas,thermal-r8a77995", + .data = &rcar_gen3_thermal, + }, {}, }; MODULE_DEVICE_TABLE(of, rcar_thermal_dt_ids); @@ -190,7 +222,8 @@ static int rcar_thermal_update_temp(struct rcar_thermal_priv *priv) * enable IRQ */ if (rcar_has_irq_support(priv)) { - rcar_thermal_write(priv, FILONOFF, 0); + if (priv->chip->type != RCAR_GEN3_THERMAL) + rcar_thermal_write(priv, FILONOFF, 0); /* enable Rising/Falling edge interrupt */ rcar_thermal_write(priv, POSNEG, 0x1); @@ -420,7 +453,7 @@ static int rcar_thermal_remove(struct platform_device *pdev) rcar_thermal_for_each_priv(priv, common) { rcar_thermal_irq_disable(priv); - if (rcar_use_of_thermal(dev)) + if (priv->chip->use_of_thermal) thermal_remove_hwmon_sysfs(priv->zone); else thermal_zone_device_unregister(priv->zone); @@ -438,6 +471,9 @@ static int rcar_thermal_probe(struct platform_device *pdev) struct rcar_thermal_priv *priv; struct device *dev = &pdev->dev; struct resource *res, *irq; + struct rcar_thermal_chip *chip = ((struct rcar_thermal_chip *) + of_device_get_match_data(dev)); + int nirq = chip->type == RCAR_GEN3_THERMAL ? 2 : 1; int mres = 0; int i; int ret = -ENODEV; @@ -457,19 +493,35 @@ static int rcar_thermal_probe(struct platform_device *pdev) pm_runtime_enable(dev); pm_runtime_get_sync(dev); - irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0); - if (irq) { - /* - * platform has IRQ support. - * 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); - if (IS_ERR(common->base)) - return PTR_ERR(common->base); + for (i = 0; i < nirq; i++) { + irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0); + if (!irq) + continue; + if (!common->base) { + /* + * platform has IRQ support. + * 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); + if (IS_ERR(common->base)) + return PTR_ERR(common->base); + + idle = 0; /* polling delay is not needed */ + } + + ret = devm_request_irq(dev, irq->start, rcar_thermal_irq, + IRQF_SHARED, dev_name(dev), common); + if (ret) { + dev_err(dev, "irq request failed\n "); + goto error_unregister; + } - idle = 0; /* polling delay is not needed */ + /* update ENR bits */ + if (chip->type == RCAR_GEN3_THERMAL) + enr_bits |= 1 << i; } for (i = 0;; i++) { @@ -491,6 +543,7 @@ static int rcar_thermal_probe(struct platform_device *pdev) priv->common = common; priv->id = i; + priv->chip = chip; mutex_init(&priv->lock); INIT_LIST_HEAD(&priv->list); INIT_DELAYED_WORK(&priv->work, rcar_thermal_work); @@ -498,7 +551,7 @@ static int rcar_thermal_probe(struct platform_device *pdev) if (ret < 0) goto error_unregister; - if (rcar_use_of_thermal(dev)) + if (chip->use_of_thermal) priv->zone = devm_thermal_zone_of_sensor_register( dev, i, priv, &rcar_thermal_zone_of_ops); @@ -515,7 +568,7 @@ static int rcar_thermal_probe(struct platform_device *pdev) goto error_unregister; } - if (rcar_use_of_thermal(dev)) { + if (chip->use_of_thermal) { /* * thermal_zone doesn't enable hwmon as default, * but, enable it here to keep compatible @@ -531,20 +584,12 @@ static int rcar_thermal_probe(struct platform_device *pdev) list_move_tail(&priv->list, &common->head); /* update ENR bits */ - enr_bits |= 3 << (i * 8); + if (chip->type != RCAR_GEN3_THERMAL) + enr_bits |= 3 << (i * 8); } - /* enable temperature comparation */ - if (irq) { - ret = devm_request_irq(dev, irq->start, rcar_thermal_irq, 0, - dev_name(dev), common); - if (ret) { - dev_err(dev, "irq request failed\n "); - goto error_unregister; - } - + if (enr_bits) rcar_thermal_common_write(common, ENR, enr_bits); - } dev_info(dev, "%d sensor probed\n", i); @@ -556,9 +601,48 @@ static int rcar_thermal_probe(struct platform_device *pdev) return ret; } +#ifdef CONFIG_PM_SLEEP +static int rcar_thermal_suspend(struct device *dev) +{ + struct rcar_thermal_common *common = dev_get_drvdata(dev); + struct rcar_thermal_priv *priv = list_first_entry(&common->head, + typeof(*priv), list); + + if (priv->chip->type == RCAR_GEN3_THERMAL) { + rcar_thermal_common_write(common, ENR, 0); + rcar_thermal_irq_disable(priv); + rcar_thermal_bset(priv, THSCR, CPCTL, 0); + } + + return 0; +} + +static int rcar_thermal_resume(struct device *dev) +{ + struct rcar_thermal_common *common = dev_get_drvdata(dev); + struct rcar_thermal_priv *priv = list_first_entry(&common->head, + typeof(*priv), list); + int ret; + + if (priv->chip->type == RCAR_GEN3_THERMAL) { + ret = rcar_thermal_update_temp(priv); + if (ret < 0) + return ret; + rcar_thermal_irq_enable(priv); + rcar_thermal_common_write(common, ENR, 0x03); + } + + return 0; +} +#endif + +static SIMPLE_DEV_PM_OPS(rcar_thermal_pm_ops, rcar_thermal_suspend, + rcar_thermal_resume); + static struct platform_driver rcar_thermal_driver = { .driver = { .name = "rcar_thermal", + .pm = &rcar_thermal_pm_ops, .of_match_table = rcar_thermal_dt_ids, }, .probe = rcar_thermal_probe,
Add support for R-Car D3 (r8a77995) thermal sensor. Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com> --- drivers/thermal/rcar_thermal.c | 148 ++++++++++++++++++++++++++++++++--------- 1 file changed, 116 insertions(+), 32 deletions(-)