[1/2] thermal: Add support for thermal sensor for Orion SoC

Commit Message

Andrew Lunn Dec. 14, 2012, 11:03 a.m. UTC

From: Nobuhiro Iwamatsu <[e-mail]>

Some Orion SoC has thermal sensor.
This patch adds support for 88F6282 and 88F6283.

Signed-off-by: Nobuhiro Iwamatsu <[e-mail]>
Signed-off-by: Andrew Lunn <[e-mail]>
---
 .../devicetree/bindings/thermal/orion-thermal.txt  |   16 +++
 drivers/thermal/Kconfig                            |    7 ++
 drivers/thermal/Makefile                           |    1 +
 drivers/thermal/orion_thermal.c                    |  133 ++++++++++++++++++++
 4 files changed, 157 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/orion-thermal.txt
 create mode 100644 drivers/thermal/orion_thermal.c

Comments

[e-mail] Dec. 14, 2012, 11:30 a.m. UTC | #1

> -----Original Message-----
> From: [e-mail] [mailto:linux-pm-
> [e-mail]] On Behalf Of Andrew Lunn
> Sent: Friday, December 14, 2012 4:33 PM
> To: linux ARM; [e-mail]; [e-mail]
> Cc: Jason Cooper; Sebastian Hesselbarth; Thomas Petazzoni;
> [e-mail]; Andrew Lunn
> Subject: [PATCH 1/2] thermal: Add support for thermal sensor for Orion SoC
> 
> From: Nobuhiro Iwamatsu <[e-mail]>
> 
> Some Orion SoC has thermal sensor.
> This patch adds support for 88F6282 and 88F6283.
> 
> Signed-off-by: Nobuhiro Iwamatsu <[e-mail]>
> Signed-off-by: Andrew Lunn <[e-mail]>
> ---
>  .../devicetree/bindings/thermal/orion-thermal.txt  |   16 +++
>  drivers/thermal/Kconfig                            |    7 ++
>  drivers/thermal/Makefile                           |    1 +
>  drivers/thermal/orion_thermal.c                    |  133 ++++++++++++++++++++
>  4 files changed, 157 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/thermal/orion-
> thermal.txt
>  create mode 100644 drivers/thermal/orion_thermal.c
> 
> diff --git a/Documentation/devicetree/bindings/thermal/orion-thermal.txt
> b/Documentation/devicetree/bindings/thermal/orion-thermal.txt
> new file mode 100644
> index 0000000..5ce925d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/orion-thermal.txt
> @@ -0,0 +1,16 @@
> +* Orion Thermal
> +
> +This initial version is for Kirkwood 88F8262 & 88F6283 SoCs, however
> +it is expected the driver will sometime in the future be expanded to
> +also support Dove, using a different compatibility string.
> +
> +Required properties:
> +- compatible : "marvell,kirkwood-thermal"
> +- reg : Address range of the thermal registers
> +
> +Example:
> +
> +	thermal@10078 {
> +		compatible = "marvell,kirkwood";
> +		reg = <0x10078 0x4>;
> +	};
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index e1cb6bd..3bba13f 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -55,3 +55,10 @@ config EXYNOS_THERMAL
>  	help
>  	  If you say yes here you get support for TMU (Thermal Managment
>  	  Unit) on SAMSUNG EXYNOS series of SoC.
> +
> +config ORION_THERMAL
> +	tristate "Temperature sensor on Marvel Orion SoCs"
> +	depends on PLAT_ORION && THERMAL

On what tree is this patch set based on ?
Rui's -next tree ? 
If not, it would better if you could rebase it on top of that.
If it is, then you don't need '&& THERMAL' in Kconfig.

> +	help
> +	  Support for the Orion thermal sensor driver into the Linux thermal
> +	  framework. This currently supports only 88F6282 and 88F6283.
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index 885550d..2fc64aa 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -6,4 +6,5 @@ obj-$(CONFIG_THERMAL)		+= thermal_sys.o
>  obj-$(CONFIG_CPU_THERMAL)		+= cpu_cooling.o
>  obj-$(CONFIG_SPEAR_THERMAL)		+= spear_thermal.o
>  obj-$(CONFIG_RCAR_THERMAL)	+= rcar_thermal.o
> +obj-$(CONFIG_ORION_THERMAL)         	+= orion_thermal.o
>  obj-$(CONFIG_EXYNOS_THERMAL)		+= exynos_thermal.o
> diff --git a/drivers/thermal/orion_thermal.c
> b/drivers/thermal/orion_thermal.c
> new file mode 100644
> index 0000000..e8a2a68
> --- /dev/null
> +++ b/drivers/thermal/orion_thermal.c
> @@ -0,0 +1,133 @@
> +/*
> + * Orion thermal sensor driver
> + *
> + * Copyright (C) 2012 Nobuhiro Iwamatsu <[e-mail]>
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/of.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/thermal.h>
> +
> +#define THERMAL_VALID_OFFSET	9
> +#define THERMAL_VALID_MASK	0x1
> +#define THERMAL_TEMP_OFFSET	10
> +#define THERMAL_TEMP_MASK	0x1FF
> +
> +/* Orion Thermal Sensor Dev Structure */
> +struct orion_thermal_dev {
> +	void __iomem *base_addr;
> +};
> +
> +static int orion_get_temp(struct thermal_zone_device *thermal,
> +			  unsigned long *temp)
> +{
> +	unsigned long reg;
> +	struct orion_thermal_dev *thermal_dev = thermal->devdata;
> +
> +	reg = readl_relaxed(thermal_dev->base_addr);
> +
> +	/* Valid check */
> +	if (!(reg >> THERMAL_VALID_OFFSET) & THERMAL_VALID_MASK) {
> +		dev_info(&thermal->device,
> +			 "Temperature sensor reading not valid\n");
> +		return -EIO;
> +	}
> +
> +	reg = (reg >> THERMAL_TEMP_OFFSET) & THERMAL_TEMP_MASK;
> +	/* Calculate temperature. See Table 814 in 8262 hardware manual. */

Thanks for the comment.
Can we move this comment one line above ? like below:

/* comment */
reg = 
*temp =

increases readability.
BTW, is this datasheet public ?
If so, could you please add the link to the datasheet at the top ?

> +	*temp = ((322UL - reg) * 10000UL * 1000UL) / 13625UL;
> +
> +	return 0;
> +}
> +
> +static struct thermal_zone_device_ops ops = {
> +	.get_temp = orion_get_temp,
> +};
> +
> +static int orion_thermal_probe(struct platform_device *pdev)
> +{
> +	struct thermal_zone_device *thermal = NULL;
> +	struct orion_thermal_dev *thermal_dev;
> +	struct resource *res;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res) {
> +		dev_err(&pdev->dev, "Failed to get platform resource\n");
> +		return -ENODEV;
> +	}
> +
> +	thermal_dev = devm_kzalloc(&pdev->dev, sizeof(*thermal_dev),
> +				   GFP_KERNEL);
> +	if (!thermal_dev) {
> +		dev_err(&pdev->dev, "kzalloc fail\n");
> +		return -ENOMEM;
> +	}
> +
> +	thermal_dev->base_addr = devm_ioremap(&pdev->dev, res->start,
> +					      resource_size(res));
> +	if (!thermal_dev->base_addr) {
> +		dev_err(&pdev->dev, "Failed to ioremap memory\n");
> +		return -ENOMEM;
> +	}
> +
> +	thermal = thermal_zone_device_register("orion_thermal", 0, 0,
> +				   thermal_dev, &ops, 0, 0);
> +	if (IS_ERR(thermal)) {
> +		dev_err(&pdev->dev,
> +			"Failed to register thermal zone device\n");

Don't we have to free ioremap resource ? Or devm_ takes care of that ?
please educate me :-)

> +		return  PTR_ERR(thermal);
> +	}
> +
> +	platform_set_drvdata(pdev, thermal);
> +
> +	dev_info(&thermal->device,
> +		 KBUILD_MODNAME ": Thermal sensor registered\n");
> +
> +	return 0;
> +}
> +
> +static int orion_thermal_exit(struct platform_device *pdev)
> +{
> +	struct thermal_zone_device *orion_thermal =
> platform_get_drvdata(pdev);
> +
> +	thermal_zone_device_unregister(orion_thermal);
> +	platform_set_drvdata(pdev, NULL);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id orion_thermal_id_table[] = {
> +	{ .compatible = "marvell,kirkwood-thermal" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, orion_thermal_id_table);
> +
> +static struct platform_driver orion_thermal_driver = {
> +	.probe = orion_thermal_probe,
> +	.remove = orion_thermal_exit,
> +	.driver = {
> +		.name = "orion_thermal",
> +		.owner = THIS_MODULE,
> +		.of_match_table = of_match_ptr(orion_thermal_id_table),
> +	},
> +};
> +
> +module_platform_driver(orion_thermal_driver);
> +
> +MODULE_AUTHOR("Nobuhiro Iwamatsu <[e-mail]>");
> +MODULE_DESCRIPTION("orion thermal driver");
> +MODULE_LICENSE("GPL");
> --
> 1.7.10.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to [e-mail]
> 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 [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Andrew Lunn Dec. 14, 2012, 11:50 a.m. UTC | #2

On Fri, Dec 14, 2012 at 11:30:41AM +0000, R, Durgadoss wrote:
> > -----Original Message-----
> > From: [e-mail] [mailto:linux-pm-
> > [e-mail]] On Behalf Of Andrew Lunn
> > Sent: Friday, December 14, 2012 4:33 PM
> > To: linux ARM; [e-mail]; [e-mail]
> > Cc: Jason Cooper; Sebastian Hesselbarth; Thomas Petazzoni;
> > [e-mail]; Andrew Lunn
> > Subject: [PATCH 1/2] thermal: Add support for thermal sensor for Orion SoC
> > 
> > From: Nobuhiro Iwamatsu <[e-mail]>
> > 
> > Some Orion SoC has thermal sensor.
> > This patch adds support for 88F6282 and 88F6283.
> > 
> > Signed-off-by: Nobuhiro Iwamatsu <[e-mail]>
> > Signed-off-by: Andrew Lunn <[e-mail]>
> > ---
> >  .../devicetree/bindings/thermal/orion-thermal.txt  |   16 +++
> >  drivers/thermal/Kconfig                            |    7 ++
> >  drivers/thermal/Makefile                           |    1 +
> >  drivers/thermal/orion_thermal.c                    |  133 ++++++++++++++++++++
> >  4 files changed, 157 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/thermal/orion-
> > thermal.txt
> >  create mode 100644 drivers/thermal/orion_thermal.c
> > 
> > diff --git a/Documentation/devicetree/bindings/thermal/orion-thermal.txt
> > b/Documentation/devicetree/bindings/thermal/orion-thermal.txt
> > new file mode 100644
> > index 0000000..5ce925d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/thermal/orion-thermal.txt
> > @@ -0,0 +1,16 @@
> > +* Orion Thermal
> > +
> > +This initial version is for Kirkwood 88F8262 & 88F6283 SoCs, however
> > +it is expected the driver will sometime in the future be expanded to
> > +also support Dove, using a different compatibility string.
> > +
> > +Required properties:
> > +- compatible : "marvell,kirkwood-thermal"
> > +- reg : Address range of the thermal registers
> > +
> > +Example:
> > +
> > +	thermal@10078 {
> > +		compatible = "marvell,kirkwood";
> > +		reg = <0x10078 0x4>;
> > +	};
> > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> > index e1cb6bd..3bba13f 100644
> > --- a/drivers/thermal/Kconfig
> > +++ b/drivers/thermal/Kconfig
> > @@ -55,3 +55,10 @@ config EXYNOS_THERMAL
> >  	help
> >  	  If you say yes here you get support for TMU (Thermal Managment
> >  	  Unit) on SAMSUNG EXYNOS series of SoC.
> > +
> > +config ORION_THERMAL
> > +	tristate "Temperature sensor on Marvel Orion SoCs"
> > +	depends on PLAT_ORION && THERMAL
> 
> On what tree is this patch set based on ?

Not a good one. arm-soc/for-next

I posted these patches to let other know i'm working in the driver and
to get further feedback. I've also started on adding support for Dove,
which is similar, but different.

It will get rebased onto -rc1 in a couple of weeks time and by then i
hope i will have Dove support.

> If it is, then you don't need '&& THERMAL' in Kconfig.
> > +	reg = (reg >> THERMAL_TEMP_OFFSET) & THERMAL_TEMP_MASK;
> > +	/* Calculate temperature. See Table 814 in 8262 hardware manual. */
> 
> Thanks for the comment.
> Can we move this comment one line above ? like below:
> 
> /* comment */
> reg = 
> *temp =

Sure.

> BTW, is this datasheet public ?

Humm, think so. Take a look at Documentation/arm/Marvell/REAMDE. I
will check and add a reference to this README file and the specific
datasheet.

> If so, could you please add the link to the datasheet at the top ?
> 
> > +	*temp = ((322UL - reg) * 10000UL * 1000UL) / 13625UL;
> > +
> > +	return 0;
> > +}
> > +
> > +static struct thermal_zone_device_ops ops = {
> > +	.get_temp = orion_get_temp,
> > +};
> > +
> > +static int orion_thermal_probe(struct platform_device *pdev)
> > +{
> > +	struct thermal_zone_device *thermal = NULL;
> > +	struct orion_thermal_dev *thermal_dev;
> > +	struct resource *res;
> > +
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +	if (!res) {
> > +		dev_err(&pdev->dev, "Failed to get platform resource\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	thermal_dev = devm_kzalloc(&pdev->dev, sizeof(*thermal_dev),
> > +				   GFP_KERNEL);
> > +	if (!thermal_dev) {
> > +		dev_err(&pdev->dev, "kzalloc fail\n");
> > +		return -ENOMEM;
> > +	}
> > +
> > +	thermal_dev->base_addr = devm_ioremap(&pdev->dev, res->start,
> > +					      resource_size(res));
> > +	if (!thermal_dev->base_addr) {
> > +		dev_err(&pdev->dev, "Failed to ioremap memory\n");
> > +		return -ENOMEM;
> > +	}
> > +
> > +	thermal = thermal_zone_device_register("orion_thermal", 0, 0,
> > +				   thermal_dev, &ops, 0, 0);
> > +	if (IS_ERR(thermal)) {
> > +		dev_err(&pdev->dev,
> > +			"Failed to register thermal zone device\n");
> 
> Don't we have to free ioremap resource ? Or devm_ takes care of that ?
> please educate me :-)

devm_ should take care of that. If the probe function returns an
error, or the driver is unloaded, all resources allocated with devm_
get automagically freed.

    Andrew
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Jason Gunthorpe Dec. 14, 2012, 6:13 p.m. UTC | #3

On Fri, Dec 14, 2012 at 12:03:05PM +0100, Andrew Lunn wrote:
> From: Nobuhiro Iwamatsu <[e-mail]>
> 
> Some Orion SoC has thermal sensor.
> This patch adds support for 88F6282 and 88F6283.
> 
> Signed-off-by: Nobuhiro Iwamatsu <[e-mail]>
> Signed-off-by: Andrew Lunn <[e-mail]>

Tested-by: Jason Gunthorpe <[e-mail]>
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Nobuhiro Iwamatsu Dec. 14, 2012, 9:54 p.m. UTC | #4

Hi,

Thanks you for your work.
Sorry, I dont hava a time at this week about this.

On Fri, Dec 14, 2012 at 8:03 PM, Andrew Lunn <[e-mail]> wrote:
> From: Nobuhiro Iwamatsu <[e-mail]>
>
> Some Orion SoC has thermal sensor.
> This patch adds support for 88F6282 and 88F6283.
>
> Signed-off-by: Nobuhiro Iwamatsu <[e-mail]>
> Signed-off-by: Andrew Lunn <[e-mail]>
> ---
>  .../devicetree/bindings/thermal/orion-thermal.txt  |   16 +++
>  drivers/thermal/Kconfig                            |    7 ++
>  drivers/thermal/Makefile                           |    1 +
>  drivers/thermal/orion_thermal.c                    |  133 ++++++++++++++++++++
>  4 files changed, 157 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/thermal/orion-thermal.txt
>  create mode 100644 drivers/thermal/orion_thermal.c
>
> diff --git a/Documentation/devicetree/bindings/thermal/orion-thermal.txt b/Documentation/devicetree/bindings/thermal/orion-thermal.txt
> new file mode 100644
> index 0000000..5ce925d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/orion-thermal.txt
> @@ -0,0 +1,16 @@
> +* Orion Thermal
> +
> +This initial version is for Kirkwood 88F8262 & 88F6283 SoCs, however
> +it is expected the driver will sometime in the future be expanded to
> +also support Dove, using a different compatibility string.
> +
> +Required properties:
> +- compatible : "marvell,kirkwood-thermal"
> +- reg : Address range of the thermal registers
> +
> +Example:
> +
> +       thermal@10078 {
> +               compatible = "marvell,kirkwood";

compatible = "marvell,kirkwood-thermal"; ?

> +               reg = <0x10078 0x4>;
> +       };
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index e1cb6bd..3bba13f 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -55,3 +55,10 @@ config EXYNOS_THERMAL
>         help
>           If you say yes here you get support for TMU (Thermal Managment
>           Unit) on SAMSUNG EXYNOS series of SoC.
> +
> +config ORION_THERMAL
> +       tristate "Temperature sensor on Marvel Orion SoCs"

Marvel -> Marvell

> +       depends on PLAT_ORION && THERMAL
> +       help
> +         Support for the Orion thermal sensor driver into the Linux thermal
> +         framework. This currently supports only 88F6282 and 88F6283.
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index 885550d..2fc64aa 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -6,4 +6,5 @@ obj-$(CONFIG_THERMAL)           += thermal_sys.o
>  obj-$(CONFIG_CPU_THERMAL)              += cpu_cooling.o
>  obj-$(CONFIG_SPEAR_THERMAL)            += spear_thermal.o
>  obj-$(CONFIG_RCAR_THERMAL)     += rcar_thermal.o
> +obj-$(CONFIG_ORION_THERMAL)            += orion_thermal.o
>  obj-$(CONFIG_EXYNOS_THERMAL)           += exynos_thermal.o
> diff --git a/drivers/thermal/orion_thermal.c b/drivers/thermal/orion_thermal.c
> new file mode 100644
> index 0000000..e8a2a68
> --- /dev/null
> +++ b/drivers/thermal/orion_thermal.c
> @@ -0,0 +1,133 @@
> +/*
> + * Orion thermal sensor driver
> + *
> + * Copyright (C) 2012 Nobuhiro Iwamatsu <[e-mail]>
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/of.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/thermal.h>
> +
> +#define THERMAL_VALID_OFFSET   9
> +#define THERMAL_VALID_MASK     0x1
> +#define THERMAL_TEMP_OFFSET    10
> +#define THERMAL_TEMP_MASK      0x1FF
> +
> +/* Orion Thermal Sensor Dev Structure */
> +struct orion_thermal_dev {
> +       void __iomem *base_addr;
> +};
> +
> +static int orion_get_temp(struct thermal_zone_device *thermal,
> +                         unsigned long *temp)
> +{
> +       unsigned long reg;
> +       struct orion_thermal_dev *thermal_dev = thermal->devdata;
> +
> +       reg = readl_relaxed(thermal_dev->base_addr);
> +
> +       /* Valid check */
> +       if (!(reg >> THERMAL_VALID_OFFSET) & THERMAL_VALID_MASK) {
> +               dev_info(&thermal->device,
> +                        "Temperature sensor reading not valid\n");
> +               return -EIO;
> +       }
> +
> +       reg = (reg >> THERMAL_TEMP_OFFSET) & THERMAL_TEMP_MASK;
> +       /* Calculate temperature. See Table 814 in 8262 hardware manual. */
> +       *temp = ((322UL - reg) * 10000UL * 1000UL) / 13625UL;
> +
> +       return 0;
> +}
> +
> +static struct thermal_zone_device_ops ops = {
> +       .get_temp = orion_get_temp,
> +};
> +
> +static int orion_thermal_probe(struct platform_device *pdev)
> +{
> +       struct thermal_zone_device *thermal = NULL;
> +       struct orion_thermal_dev *thermal_dev;
> +       struct resource *res;
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       if (!res) {
> +               dev_err(&pdev->dev, "Failed to get platform resource\n");
> +               return -ENODEV;
> +       }
> +
> +       thermal_dev = devm_kzalloc(&pdev->dev, sizeof(*thermal_dev),
> +                                  GFP_KERNEL);
> +       if (!thermal_dev) {
> +               dev_err(&pdev->dev, "kzalloc fail\n");
> +               return -ENOMEM;
> +       }
> +
> +       thermal_dev->base_addr = devm_ioremap(&pdev->dev, res->start,
> +                                             resource_size(res));
> +       if (!thermal_dev->base_addr) {
> +               dev_err(&pdev->dev, "Failed to ioremap memory\n");
> +               return -ENOMEM;
> +       }
> +
> +       thermal = thermal_zone_device_register("orion_thermal", 0, 0,
> +                                  thermal_dev, &ops, 0, 0);
> +       if (IS_ERR(thermal)) {
> +               dev_err(&pdev->dev,
> +                       "Failed to register thermal zone device\n");
> +               return  PTR_ERR(thermal);
> +       }
> +
> +       platform_set_drvdata(pdev, thermal);
> +
> +       dev_info(&thermal->device,
> +                KBUILD_MODNAME ": Thermal sensor registered\n");
> +
> +       return 0;
> +}
> +
> +static int orion_thermal_exit(struct platform_device *pdev)
> +{
> +       struct thermal_zone_device *orion_thermal = platform_get_drvdata(pdev);
> +
> +       thermal_zone_device_unregister(orion_thermal);
> +       platform_set_drvdata(pdev, NULL);
> +
> +       return 0;
> +}
> +
> +static const struct of_device_id orion_thermal_id_table[] = {
> +       { .compatible = "marvell,kirkwood-thermal" },
> +       {}
> +};
> +MODULE_DEVICE_TABLE(of, orion_thermal_id_table);
> +
> +static struct platform_driver orion_thermal_driver = {
> +       .probe = orion_thermal_probe,
> +       .remove = orion_thermal_exit,
> +       .driver = {
> +               .name = "orion_thermal",
> +               .owner = THIS_MODULE,
> +               .of_match_table = of_match_ptr(orion_thermal_id_table),
> +       },
> +};
> +
> +module_platform_driver(orion_thermal_driver);
> +
> +MODULE_AUTHOR("Nobuhiro Iwamatsu <[e-mail]>");
> +MODULE_DESCRIPTION("orion thermal driver");
> +MODULE_LICENSE("GPL");
> --
> 1.7.10.4
>

Tested-by: Nobuhiro Iwamatsu <[e-mail]>

Best regard,
  Nobuhiro

Nobuhiro Iwamatsu Dec. 14, 2012, 10:03 p.m. UTC | #5

Hi,

On Fri, Dec 14, 2012 at 8:50 PM, Andrew Lunn <[e-mail]> wrote:
> On Fri, Dec 14, 2012 at 11:30:41AM +0000, R, Durgadoss wrote:
>> > -----Original Message-----
>> > From: [e-mail] [mailto:linux-pm-
>> > [e-mail]] On Behalf Of Andrew Lunn
>> > Sent: Friday, December 14, 2012 4:33 PM
>> > To: linux ARM; [e-mail]; [e-mail]
>> > Cc: Jason Cooper; Sebastian Hesselbarth; Thomas Petazzoni;
>> > [e-mail]; Andrew Lunn
>> > Subject: [PATCH 1/2] thermal: Add support for thermal sensor for Orion SoC
>> >
>> > From: Nobuhiro Iwamatsu <[e-mail]>
>> >
>> > Some Orion SoC has thermal sensor.
>> > This patch adds support for 88F6282 and 88F6283.
>> >
>> > Signed-off-by: Nobuhiro Iwamatsu <[e-mail]>
>> > Signed-off-by: Andrew Lunn <[e-mail]>
>> > ---
>> >  .../devicetree/bindings/thermal/orion-thermal.txt  |   16 +++
>> >  drivers/thermal/Kconfig                            |    7 ++
>> >  drivers/thermal/Makefile                           |    1 +
>> >  drivers/thermal/orion_thermal.c                    |  133 ++++++++++++++++++++
>> >  4 files changed, 157 insertions(+)
>> >  create mode 100644 Documentation/devicetree/bindings/thermal/orion-
>> > thermal.txt
>> >  create mode 100644 drivers/thermal/orion_thermal.c
>> >
>> > diff --git a/Documentation/devicetree/bindings/thermal/orion-thermal.txt
>> > b/Documentation/devicetree/bindings/thermal/orion-thermal.txt
>> > new file mode 100644
>> > index 0000000..5ce925d
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/thermal/orion-thermal.txt
>> > @@ -0,0 +1,16 @@
>> > +* Orion Thermal
>> > +
>> > +This initial version is for Kirkwood 88F8262 & 88F6283 SoCs, however
>> > +it is expected the driver will sometime in the future be expanded to
>> > +also support Dove, using a different compatibility string.
>> > +
>> > +Required properties:
>> > +- compatible : "marvell,kirkwood-thermal"
>> > +- reg : Address range of the thermal registers
>> > +
>> > +Example:
>> > +
>> > +   thermal@10078 {
>> > +           compatible = "marvell,kirkwood";
>> > +           reg = <0x10078 0x4>;
>> > +   };
>> > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
>> > index e1cb6bd..3bba13f 100644
>> > --- a/drivers/thermal/Kconfig
>> > +++ b/drivers/thermal/Kconfig
>> > @@ -55,3 +55,10 @@ config EXYNOS_THERMAL
>> >     help
>> >       If you say yes here you get support for TMU (Thermal Managment
>> >       Unit) on SAMSUNG EXYNOS series of SoC.
>> > +
>> > +config ORION_THERMAL
>> > +   tristate "Temperature sensor on Marvel Orion SoCs"
>> > +   depends on PLAT_ORION && THERMAL
>>
>> On what tree is this patch set based on ?
>
> Not a good one. arm-soc/for-next
>
> I posted these patches to let other know i'm working in the driver and
> to get further feedback. I've also started on adding support for Dove,
> which is similar, but different.
>
> It will get rebased onto -rc1 in a couple of weeks time and by then i
> hope i will have Dove support.
>
>> If it is, then you don't need '&& THERMAL' in Kconfig.
>> > +   reg = (reg >> THERMAL_TEMP_OFFSET) & THERMAL_TEMP_MASK;
>> > +   /* Calculate temperature. See Table 814 in 8262 hardware manual. */
>>
>> Thanks for the comment.
>> Can we move this comment one line above ? like below:
>>
>> /* comment */
>> reg =
>> *temp =
>
> Sure.
>
>> BTW, is this datasheet public ?
>
> Humm, think so. Take a look at Documentation/arm/Marvell/REAMDE. I
> will check and add a reference to this README file and the specific
> datasheet.

Datasheet of 88F6282 and 88F6283 is not public.
# I dont know about other SoC (about Dove).

Best regards,
  Nobuhiro

Andrew Lunn Dec. 14, 2012, 10:11 p.m. UTC | #6

On Sat, Dec 15, 2012 at 06:54:17AM +0900, Nobuhiro Iwamatsu wrote:
> Hi,
> 
> Thanks you for your work.
> Sorry, I dont hava a time at this week about this.

Its not a problem. We have plenty of time before the next merge
window. I was just interested in seeing it work on my QNAP device, so
did some of the cleanup work.

> On Fri, Dec 14, 2012 at 8:03 PM, Andrew Lunn <[e-mail]> wrote:
> > From: Nobuhiro Iwamatsu <[e-mail]>
> >
> > Some Orion SoC has thermal sensor.
> > This patch adds support for 88F6282 and 88F6283.
> >
> > Signed-off-by: Nobuhiro Iwamatsu <[e-mail]>
> > Signed-off-by: Andrew Lunn <[e-mail]>
> > ---
> >  .../devicetree/bindings/thermal/orion-thermal.txt  |   16 +++
> >  drivers/thermal/Kconfig                            |    7 ++
> >  drivers/thermal/Makefile                           |    1 +
> >  drivers/thermal/orion_thermal.c                    |  133 ++++++++++++++++++++
> >  4 files changed, 157 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/thermal/orion-thermal.txt
> >  create mode 100644 drivers/thermal/orion_thermal.c
> >
> > diff --git a/Documentation/devicetree/bindings/thermal/orion-thermal.txt b/Documentation/devicetree/bindings/thermal/orion-thermal.txt
> > new file mode 100644
> > index 0000000..5ce925d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/thermal/orion-thermal.txt
> > @@ -0,0 +1,16 @@
> > +* Orion Thermal
> > +
> > +This initial version is for Kirkwood 88F8262 & 88F6283 SoCs, however
> > +it is expected the driver will sometime in the future be expanded to
> > +also support Dove, using a different compatibility string.
> > +
> > +Required properties:
> > +- compatible : "marvell,kirkwood-thermal"
> > +- reg : Address range of the thermal registers
> > +
> > +Example:
> > +
> > +       thermal@10078 {
> > +               compatible = "marvell,kirkwood";
> 
> compatible = "marvell,kirkwood-thermal"; ?

Yep, my error.

> > +               reg = <0x10078 0x4>;
> > +       };
> > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> > index e1cb6bd..3bba13f 100644
> > --- a/drivers/thermal/Kconfig
> > +++ b/drivers/thermal/Kconfig
> > @@ -55,3 +55,10 @@ config EXYNOS_THERMAL
> >         help
> >           If you say yes here you get support for TMU (Thermal Managment
> >           Unit) on SAMSUNG EXYNOS series of SoC.
> > +
> > +config ORION_THERMAL
> > +       tristate "Temperature sensor on Marvel Orion SoCs"
> 
> Marvel -> Marvell

Missed that one, thanks.

Thanks for the Tested-by. I will add it to the next version.  I
started work on Dove support, so i will probably repost when i have
that ready for testing.

     Andrew
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Zhang Rui Jan. 4, 2013, 7:33 a.m. UTC | #7

On Fri, 2012-12-14 at 23:11 +0100, Andrew Lunn wrote:
> On Sat, Dec 15, 2012 at 06:54:17AM +0900, Nobuhiro Iwamatsu wrote:
> > Hi,
> > 
> > Thanks you for your work.
> > Sorry, I dont hava a time at this week about this.
> 
> Its not a problem. We have plenty of time before the next merge
> window. I was just interested in seeing it work on my QNAP device, so
> did some of the cleanup work.
> 
> > On Fri, Dec 14, 2012 at 8:03 PM, Andrew Lunn <[e-mail]> wrote:
> > > From: Nobuhiro Iwamatsu <[e-mail]>
> > >
> > > Some Orion SoC has thermal sensor.
> > > This patch adds support for 88F6282 and 88F6283.
> > >
> > > Signed-off-by: Nobuhiro Iwamatsu <[e-mail]>
> > > Signed-off-by: Andrew Lunn <[e-mail]>
> > > ---
> > >  .../devicetree/bindings/thermal/orion-thermal.txt  |   16 +++
> > >  drivers/thermal/Kconfig                            |    7 ++
> > >  drivers/thermal/Makefile                           |    1 +
> > >  drivers/thermal/orion_thermal.c                    |  133 ++++++++++++++++++++
> > >  4 files changed, 157 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/thermal/orion-thermal.txt
> > >  create mode 100644 drivers/thermal/orion_thermal.c
> > >
> > > diff --git a/Documentation/devicetree/bindings/thermal/orion-thermal.txt b/Documentation/devicetree/bindings/thermal/orion-thermal.txt
> > > new file mode 100644
> > > index 0000000..5ce925d
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/thermal/orion-thermal.txt
> > > @@ -0,0 +1,16 @@
> > > +* Orion Thermal
> > > +
> > > +This initial version is for Kirkwood 88F8262 & 88F6283 SoCs, however
> > > +it is expected the driver will sometime in the future be expanded to
> > > +also support Dove, using a different compatibility string.
> > > +
> > > +Required properties:
> > > +- compatible : "marvell,kirkwood-thermal"
> > > +- reg : Address range of the thermal registers
> > > +
> > > +Example:
> > > +
> > > +       thermal@10078 {
> > > +               compatible = "marvell,kirkwood";
> > 
> > compatible = "marvell,kirkwood-thermal"; ?
> 
> Yep, my error.
> 
> > > +               reg = <0x10078 0x4>;
> > > +       };
> > > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> > > index e1cb6bd..3bba13f 100644
> > > --- a/drivers/thermal/Kconfig
> > > +++ b/drivers/thermal/Kconfig
> > > @@ -55,3 +55,10 @@ config EXYNOS_THERMAL
> > >         help
> > >           If you say yes here you get support for TMU (Thermal Managment
> > >           Unit) on SAMSUNG EXYNOS series of SoC.
> > > +
> > > +config ORION_THERMAL
> > > +       tristate "Temperature sensor on Marvel Orion SoCs"
> > 
> > Marvel -> Marvell
> 
> Missed that one, thanks.
> 
> Thanks for the Tested-by. I will add it to the next version.  I
> started work on Dove support, so i will probably repost when i have
> that ready for testing.

sorry for the late response, can you resend the refreshed version on top
of the thermal next tree?

thanks,
rui

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Andrew Lunn Jan. 4, 2013, 8:15 a.m. UTC | #8

On 04/01/13 08:33, Zhang Rui wrote:
> On Fri, 2012-12-14 at 23:11 +0100, Andrew Lunn wrote:
>> On Sat, Dec 15, 2012 at 06:54:17AM +0900, Nobuhiro Iwamatsu wrote:
>>> Hi,
>>>
>>> Thanks you for your work.
>>> Sorry, I dont hava a time at this week about this.
>>
>> Its not a problem. We have plenty of time before the next merge
>> window. I was just interested in seeing it work on my QNAP device, so
>> did some of the cleanup work.
>>
>>> On Fri, Dec 14, 2012 at 8:03 PM, Andrew Lunn<[e-mail]>  wrote:
>>>> From: Nobuhiro Iwamatsu<[e-mail]>
>>>>
>>>> Some Orion SoC has thermal sensor.
>>>> This patch adds support for 88F6282 and 88F6283.
>>>>
>>>> Signed-off-by: Nobuhiro Iwamatsu<[e-mail]>
>>>> Signed-off-by: Andrew Lunn<[e-mail]>
>>>> ---
>>>>   .../devicetree/bindings/thermal/orion-thermal.txt  |   16 +++
>>>>   drivers/thermal/Kconfig                            |    7 ++
>>>>   drivers/thermal/Makefile                           |    1 +
>>>>   drivers/thermal/orion_thermal.c                    |  133 ++++++++++++++++++++
>>>>   4 files changed, 157 insertions(+)
>>>>   create mode 100644 Documentation/devicetree/bindings/thermal/orion-thermal.txt
>>>>   create mode 100644 drivers/thermal/orion_thermal.c
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/thermal/orion-thermal.txt b/Documentation/devicetree/bindings/thermal/orion-thermal.txt
>>>> new file mode 100644
>>>> index 0000000..5ce925d
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/thermal/orion-thermal.txt
>>>> @@ -0,0 +1,16 @@
>>>> +* Orion Thermal
>>>> +
>>>> +This initial version is for Kirkwood 88F8262&  88F6283 SoCs, however
>>>> +it is expected the driver will sometime in the future be expanded to
>>>> +also support Dove, using a different compatibility string.
>>>> +
>>>> +Required properties:
>>>> +- compatible : "marvell,kirkwood-thermal"
>>>> +- reg : Address range of the thermal registers
>>>> +
>>>> +Example:
>>>> +
>>>> +       thermal@10078 {
>>>> +               compatible = "marvell,kirkwood";
>>>
>>> compatible = "marvell,kirkwood-thermal"; ?
>>
>> Yep, my error.
>>
>>>> +               reg =<0x10078 0x4>;
>>>> +       };
>>>> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
>>>> index e1cb6bd..3bba13f 100644
>>>> --- a/drivers/thermal/Kconfig
>>>> +++ b/drivers/thermal/Kconfig
>>>> @@ -55,3 +55,10 @@ config EXYNOS_THERMAL
>>>>          help
>>>>            If you say yes here you get support for TMU (Thermal Managment
>>>>            Unit) on SAMSUNG EXYNOS series of SoC.
>>>> +
>>>> +config ORION_THERMAL
>>>> +       tristate "Temperature sensor on Marvel Orion SoCs"
>>>
>>> Marvel ->  Marvell
>>
>> Missed that one, thanks.
>>
>> Thanks for the Tested-by. I will add it to the next version.  I
>> started work on Dove support, so i will probably repost when i have
>> that ready for testing.
>
> sorry for the late response, can you resend the refreshed version on top
> of the thermal next tree?

Hi Rui

Im in the process of generalizing the driver so that it works for 
Kirkwood, Dove, Armada 370 and Armada XP. The combined driver is not 
finished yet. Once i have something ready i will repost.

	Andrew

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Eduardo Valentin Jan. 4, 2013, 9:40 a.m. UTC | #9

Hey Andrew,

On 14-12-2012 13:03, Andrew Lunn wrote:
> From: Nobuhiro Iwamatsu <[e-mail]>
>
> Some Orion SoC has thermal sensor.
> This patch adds support for 88F6282 and 88F6283.
>
> Signed-off-by: Nobuhiro Iwamatsu <[e-mail]>
> Signed-off-by: Andrew Lunn <[e-mail]>
> ---
>   .../devicetree/bindings/thermal/orion-thermal.txt  |   16 +++
>   drivers/thermal/Kconfig                            |    7 ++
>   drivers/thermal/Makefile                           |    1 +
>   drivers/thermal/orion_thermal.c                    |  133 ++++++++++++++++++++
>   4 files changed, 157 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/thermal/orion-thermal.txt
>   create mode 100644 drivers/thermal/orion_thermal.c
>
> diff --git a/Documentation/devicetree/bindings/thermal/orion-thermal.txt b/Documentation/devicetree/bindings/thermal/orion-thermal.txt
> new file mode 100644
> index 0000000..5ce925d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/orion-thermal.txt
> @@ -0,0 +1,16 @@
> +* Orion Thermal
> +
> +This initial version is for Kirkwood 88F8262 & 88F6283 SoCs, however
> +it is expected the driver will sometime in the future be expanded to
> +also support Dove, using a different compatibility string.
> +
> +Required properties:
> +- compatible : "marvell,kirkwood-thermal"
> +- reg : Address range of the thermal registers
> +
> +Example:
> +
> +	thermal@10078 {
> +		compatible = "marvell,kirkwood";
> +		reg = <0x10078 0x4>;
> +	};

How do you differentiate if the SoC has the temperature sensor? On your 
patch description you are very clear saying that this supports only 
88F8262 & 88F6283 SoCs.

> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index e1cb6bd..3bba13f 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -55,3 +55,10 @@ config EXYNOS_THERMAL
>   	help
>   	  If you say yes here you get support for TMU (Thermal Managment
>   	  Unit) on SAMSUNG EXYNOS series of SoC.
> +
> +config ORION_THERMAL
> +	tristate "Temperature sensor on Marvel Orion SoCs"
> +	depends on PLAT_ORION && THERMAL
> +	help
> +	  Support for the Orion thermal sensor driver into the Linux thermal
> +	  framework. This currently supports only 88F6282 and 88F6283.
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index 885550d..2fc64aa 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -6,4 +6,5 @@ obj-$(CONFIG_THERMAL)		+= thermal_sys.o
>   obj-$(CONFIG_CPU_THERMAL)		+= cpu_cooling.o
>   obj-$(CONFIG_SPEAR_THERMAL)		+= spear_thermal.o
>   obj-$(CONFIG_RCAR_THERMAL)	+= rcar_thermal.o
> +obj-$(CONFIG_ORION_THERMAL)         	+= orion_thermal.o
>   obj-$(CONFIG_EXYNOS_THERMAL)		+= exynos_thermal.o
> diff --git a/drivers/thermal/orion_thermal.c b/drivers/thermal/orion_thermal.c
> new file mode 100644
> index 0000000..e8a2a68
> --- /dev/null
> +++ b/drivers/thermal/orion_thermal.c
> @@ -0,0 +1,133 @@
> +/*
> + * Orion thermal sensor driver
> + *
> + * Copyright (C) 2012 Nobuhiro Iwamatsu <[e-mail]>
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/of.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/thermal.h>
> +
> +#define THERMAL_VALID_OFFSET	9
> +#define THERMAL_VALID_MASK	0x1
> +#define THERMAL_TEMP_OFFSET	10
> +#define THERMAL_TEMP_MASK	0x1FF
> +
> +/* Orion Thermal Sensor Dev Structure */
> +struct orion_thermal_dev {
> +	void __iomem *base_addr;
> +};
> +
> +static int orion_get_temp(struct thermal_zone_device *thermal,
> +			  unsigned long *temp)
> +{
> +	unsigned long reg;
> +	struct orion_thermal_dev *thermal_dev = thermal->devdata;
> +
> +	reg = readl_relaxed(thermal_dev->base_addr);
> +
> +	/* Valid check */
> +	if (!(reg >> THERMAL_VALID_OFFSET) & THERMAL_VALID_MASK) {
> +		dev_info(&thermal->device,

This state seams to be severe enough to get a dev_err level message.

> +			 "Temperature sensor reading not valid\n");
> +		return -EIO;
> +	}
> +
> +	reg = (reg >> THERMAL_TEMP_OFFSET) & THERMAL_TEMP_MASK;
> +	/* Calculate temperature. See Table 814 in 8262 hardware manual. */
> +	*temp = ((322UL - reg) * 10000UL * 1000UL) / 13625UL;
> +
> +	return 0;
> +}
> +
> +static struct thermal_zone_device_ops ops = {
> +	.get_temp = orion_get_temp,
> +};
> +
> +static int orion_thermal_probe(struct platform_device *pdev)
> +{
> +	struct thermal_zone_device *thermal = NULL;
> +	struct orion_thermal_dev *thermal_dev;
> +	struct resource *res;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res) {
> +		dev_err(&pdev->dev, "Failed to get platform resource\n");
> +		return -ENODEV;
> +	}
> +
> +	thermal_dev = devm_kzalloc(&pdev->dev, sizeof(*thermal_dev),
> +				   GFP_KERNEL);
> +	if (!thermal_dev) {
> +		dev_err(&pdev->dev, "kzalloc fail\n");
> +		return -ENOMEM;
> +	}
> +
> +	thermal_dev->base_addr = devm_ioremap(&pdev->dev, res->start,
> +					      resource_size(res));


I believe you forgot to request this memory. I suggest you do:
+	thermal_dev->base_addr = devm_request_and_ioremap(&pdev->dev, res->start,
+					      res);


> +	if (!thermal_dev->base_addr) {
> +		dev_err(&pdev->dev, "Failed to ioremap memory\n");
> +		return -ENOMEM;
> +	}
> +
> +	thermal = thermal_zone_device_register("orion_thermal", 0, 0,
> +				   thermal_dev, &ops, 0, 0);
> +	if (IS_ERR(thermal)) {
> +		dev_err(&pdev->dev,
> +			"Failed to register thermal zone device\n");
> +		return  PTR_ERR(thermal);
> +	}
> +
> +	platform_set_drvdata(pdev, thermal);
> +
> +	dev_info(&thermal->device,
> +		 KBUILD_MODNAME ": Thermal sensor registered\n");

Do you really need to be verbose? I suppose one can always check sysfs 
entries to see if there is a successful driver & device binding...

> +
> +	return 0;
> +}
> +
> +static int orion_thermal_exit(struct platform_device *pdev)
> +{
> +	struct thermal_zone_device *orion_thermal = platform_get_drvdata(pdev);
> +
> +	thermal_zone_device_unregister(orion_thermal);
> +	platform_set_drvdata(pdev, NULL);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id orion_thermal_id_table[] = {
> +	{ .compatible = "marvell,kirkwood-thermal" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, orion_thermal_id_table);
> +
> +static struct platform_driver orion_thermal_driver = {
> +	.probe = orion_thermal_probe,
> +	.remove = orion_thermal_exit,
> +	.driver = {
> +		.name = "orion_thermal",
> +		.owner = THIS_MODULE,
> +		.of_match_table = of_match_ptr(orion_thermal_id_table),
> +	},
> +};
> +
> +module_platform_driver(orion_thermal_driver);
> +
> +MODULE_AUTHOR("Nobuhiro Iwamatsu <[e-mail]>");
> +MODULE_DESCRIPTION("orion thermal driver");
> +MODULE_LICENSE("GPL");
>

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Eduardo Valentin Jan. 4, 2013, 9:47 a.m. UTC | #10

Hello again,

On 04-01-2013 11:40, Eduardo Valentin wrote:
> Hey Andrew,
>
> On 14-12-2012 13:03, Andrew Lunn wrote:
>> From: Nobuhiro Iwamatsu <[e-mail]>

<cut>

>
> I believe you forgot to request this memory. I suggest you do:
> +    thermal_dev->base_addr = devm_request_and_ioremap(&pdev->dev,
> res->start,
> +                          res);
>

small fix:

+    thermal_dev->base_addr = devm_request_and_ioremap(&pdev->dev,
+                                                      res);

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Andrew Lunn Jan. 4, 2013, 3:35 p.m. UTC | #11

On 04/01/13 10:40, Eduardo Valentin wrote:
> Hey Andrew,
>
> On 14-12-2012 13:03, Andrew Lunn wrote:
>> From: Nobuhiro Iwamatsu <[e-mail]>
>>
>> Some Orion SoC has thermal sensor.
>> This patch adds support for 88F6282 and 88F6283.
>>
>> Signed-off-by: Nobuhiro Iwamatsu <[e-mail]>
>> Signed-off-by: Andrew Lunn <[e-mail]>
>> ---
>> .../devicetree/bindings/thermal/orion-thermal.txt | 16 +++
>> drivers/thermal/Kconfig | 7 ++
>> drivers/thermal/Makefile | 1 +
>> drivers/thermal/orion_thermal.c | 133 ++++++++++++++++++++
>> 4 files changed, 157 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/thermal/orion-thermal.txt
>> create mode 100644 drivers/thermal/orion_thermal.c
>>
>> diff --git
>> a/Documentation/devicetree/bindings/thermal/orion-thermal.txt
>> b/Documentation/devicetree/bindings/thermal/orion-thermal.txt
>> new file mode 100644
>> index 0000000..5ce925d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/thermal/orion-thermal.txt
>> @@ -0,0 +1,16 @@
>> +* Orion Thermal
>> +
>> +This initial version is for Kirkwood 88F8262 & 88F6283 SoCs, however
>> +it is expected the driver will sometime in the future be expanded to
>> +also support Dove, using a different compatibility string.
>> +
>> +Required properties:
>> +- compatible : "marvell,kirkwood-thermal"
>> +- reg : Address range of the thermal registers
>> +
>> +Example:
>> +
>> + thermal@10078 {
>> + compatible = "marvell,kirkwood";
>> + reg = <0x10078 0x4>;
>> + };
>
> How do you differentiate if the SoC has the temperature sensor? On your
> patch description you are very clear saying that this supports only
> 88F8262 & 88F6283 SoCs.

Hi Eduardo

Thanks for the comments. I will address them in the next version.

We differentiate between the different SoCs by DT. Each has its own 
.dtsi file and we will put the node into only those which have the hardware.

Thanks
	Andrew
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Eduardo Valentin Jan. 7, 2013, 10:06 a.m. UTC | #12

On 04-01-2013 17:35, Andrew Lunn wrote:
> On 04/01/13 10:40, Eduardo Valentin wrote:
>> Hey Andrew,
>>
>> On 14-12-2012 13:03, Andrew Lunn wrote:
>>> From: Nobuhiro Iwamatsu <[e-mail]>
>>>
>>> Some Orion SoC has thermal sensor.
>>> This patch adds support for 88F6282 and 88F6283.
>>>
>>> Signed-off-by: Nobuhiro Iwamatsu <[e-mail]>
>>> Signed-off-by: Andrew Lunn <[e-mail]>
>>> ---
>>> .../devicetree/bindings/thermal/orion-thermal.txt | 16 +++
>>> drivers/thermal/Kconfig | 7 ++
>>> drivers/thermal/Makefile | 1 +
>>> drivers/thermal/orion_thermal.c | 133 ++++++++++++++++++++
>>> 4 files changed, 157 insertions(+)
>>> create mode 100644
>>> Documentation/devicetree/bindings/thermal/orion-thermal.txt
>>> create mode 100644 drivers/thermal/orion_thermal.c
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/thermal/orion-thermal.txt
>>> b/Documentation/devicetree/bindings/thermal/orion-thermal.txt
>>> new file mode 100644
>>> index 0000000..5ce925d
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/thermal/orion-thermal.txt
>>> @@ -0,0 +1,16 @@
>>> +* Orion Thermal
>>> +
>>> +This initial version is for Kirkwood 88F8262 & 88F6283 SoCs, however
>>> +it is expected the driver will sometime in the future be expanded to
>>> +also support Dove, using a different compatibility string.
>>> +
>>> +Required properties:
>>> +- compatible : "marvell,kirkwood-thermal"
>>> +- reg : Address range of the thermal registers
>>> +
>>> +Example:
>>> +
>>> + thermal@10078 {
>>> + compatible = "marvell,kirkwood";
>>> + reg = <0x10078 0x4>;
>>> + };
>>
>> How do you differentiate if the SoC has the temperature sensor? On your
>> patch description you are very clear saying that this supports only
>> 88F8262 & 88F6283 SoCs.
>
> Hi Eduardo
>
> Thanks for the comments. I will address them in the next version.
>

OK. Please put me on Cc so I will keep the review.

> We differentiate between the different SoCs by DT. Each has its own
> .dtsi file and we will put the node into only those which have the
> hardware.

Ok. That I understand, but my question was more into the difference 
between 88F6282 and 88F6283. Do you need to differentiate those two? Is 
there feature set which  is specific to one which is not present in the 
second ? Because the DT entries you have you won't differentiate them, 
besides you don't read any revision register to check the chip version. 
That's my point.

>
> Thanks
>      Andrew

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Andrew Lunn Jan. 7, 2013, 10:16 a.m. UTC | #13

> OK. Please put me on Cc so I will keep the review.
> 
> >We differentiate between the different SoCs by DT. Each has its own
> >.dtsi file and we will put the node into only those which have the
> >hardware.
> 
> Ok. That I understand, but my question was more into the difference
> between 88F6282 and 88F6283. Do you need to differentiate those two?

Hi Eduardo

No, there is no need to differentiate between these. As far as the
temperature sensor goes, they are identical. I've been testing on a
88F6282 and Nobuhiro Iwamatsu on a 88F6283.

	    Andrew
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/Documentation/devicetree/bindings/thermal/orion-thermal.txt b/Documentation/devicetree/bindings/thermal/orion-thermal.txt
new file mode 100644
index 0000000..5ce925d
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/orion-thermal.txt
@@ -0,0 +1,16 @@ 
+* Orion Thermal
+
+This initial version is for Kirkwood 88F8262 & 88F6283 SoCs, however
+it is expected the driver will sometime in the future be expanded to
+also support Dove, using a different compatibility string.
+
+Required properties:
+- compatible : "marvell,kirkwood-thermal"
+- reg : Address range of the thermal registers
+
+Example:
+
+	thermal@10078 {
+		compatible = "marvell,kirkwood";
+		reg = <0x10078 0x4>;
+	};
diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index e1cb6bd..3bba13f 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -55,3 +55,10 @@  config EXYNOS_THERMAL
 	help
 	  If you say yes here you get support for TMU (Thermal Managment
 	  Unit) on SAMSUNG EXYNOS series of SoC.
+
+config ORION_THERMAL
+	tristate "Temperature sensor on Marvel Orion SoCs"
+	depends on PLAT_ORION && THERMAL
+	help
+	  Support for the Orion thermal sensor driver into the Linux thermal
+	  framework. This currently supports only 88F6282 and 88F6283.
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 885550d..2fc64aa 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -6,4 +6,5 @@  obj-$(CONFIG_THERMAL)		+= thermal_sys.o
 obj-$(CONFIG_CPU_THERMAL)		+= cpu_cooling.o
 obj-$(CONFIG_SPEAR_THERMAL)		+= spear_thermal.o
 obj-$(CONFIG_RCAR_THERMAL)	+= rcar_thermal.o
+obj-$(CONFIG_ORION_THERMAL)         	+= orion_thermal.o
 obj-$(CONFIG_EXYNOS_THERMAL)		+= exynos_thermal.o
diff --git a/drivers/thermal/orion_thermal.c b/drivers/thermal/orion_thermal.c
new file mode 100644
index 0000000..e8a2a68
--- /dev/null
+++ b/drivers/thermal/orion_thermal.c
@@ -0,0 +1,133 @@ 
+/*
+ * Orion thermal sensor driver
+ *
+ * Copyright (C) 2012 Nobuhiro Iwamatsu <[e-mail]>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/thermal.h>
+
+#define THERMAL_VALID_OFFSET	9
+#define THERMAL_VALID_MASK	0x1
+#define THERMAL_TEMP_OFFSET	10
+#define THERMAL_TEMP_MASK	0x1FF
+
+/* Orion Thermal Sensor Dev Structure */
+struct orion_thermal_dev {
+	void __iomem *base_addr;
+};
+
+static int orion_get_temp(struct thermal_zone_device *thermal,
+			  unsigned long *temp)
+{
+	unsigned long reg;
+	struct orion_thermal_dev *thermal_dev = thermal->devdata;
+
+	reg = readl_relaxed(thermal_dev->base_addr);
+
+	/* Valid check */
+	if (!(reg >> THERMAL_VALID_OFFSET) & THERMAL_VALID_MASK) {
+		dev_info(&thermal->device,
+			 "Temperature sensor reading not valid\n");
+		return -EIO;
+	}
+
+	reg = (reg >> THERMAL_TEMP_OFFSET) & THERMAL_TEMP_MASK;
+	/* Calculate temperature. See Table 814 in 8262 hardware manual. */
+	*temp = ((322UL - reg) * 10000UL * 1000UL) / 13625UL;
+
+	return 0;
+}
+
+static struct thermal_zone_device_ops ops = {
+	.get_temp = orion_get_temp,
+};
+
+static int orion_thermal_probe(struct platform_device *pdev)
+{
+	struct thermal_zone_device *thermal = NULL;
+	struct orion_thermal_dev *thermal_dev;
+	struct resource *res;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "Failed to get platform resource\n");
+		return -ENODEV;
+	}
+
+	thermal_dev = devm_kzalloc(&pdev->dev, sizeof(*thermal_dev),
+				   GFP_KERNEL);
+	if (!thermal_dev) {
+		dev_err(&pdev->dev, "kzalloc fail\n");
+		return -ENOMEM;
+	}
+
+	thermal_dev->base_addr = devm_ioremap(&pdev->dev, res->start,
+					      resource_size(res));
+	if (!thermal_dev->base_addr) {
+		dev_err(&pdev->dev, "Failed to ioremap memory\n");
+		return -ENOMEM;
+	}
+
+	thermal = thermal_zone_device_register("orion_thermal", 0, 0,
+				   thermal_dev, &ops, 0, 0);
+	if (IS_ERR(thermal)) {
+		dev_err(&pdev->dev,
+			"Failed to register thermal zone device\n");
+		return  PTR_ERR(thermal);
+	}
+
+	platform_set_drvdata(pdev, thermal);
+
+	dev_info(&thermal->device,
+		 KBUILD_MODNAME ": Thermal sensor registered\n");
+
+	return 0;
+}
+
+static int orion_thermal_exit(struct platform_device *pdev)
+{
+	struct thermal_zone_device *orion_thermal = platform_get_drvdata(pdev);
+
+	thermal_zone_device_unregister(orion_thermal);
+	platform_set_drvdata(pdev, NULL);
+
+	return 0;
+}
+
+static const struct of_device_id orion_thermal_id_table[] = {
+	{ .compatible = "marvell,kirkwood-thermal" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, orion_thermal_id_table);
+
+static struct platform_driver orion_thermal_driver = {
+	.probe = orion_thermal_probe,
+	.remove = orion_thermal_exit,
+	.driver = {
+		.name = "orion_thermal",
+		.owner = THIS_MODULE,
+		.of_match_table = of_match_ptr(orion_thermal_id_table),
+	},
+};
+
+module_platform_driver(orion_thermal_driver);
+
+MODULE_AUTHOR("Nobuhiro Iwamatsu <[e-mail]>");
+MODULE_DESCRIPTION("orion thermal driver");
+MODULE_LICENSE("GPL");