diff mbox series

[v7,2/6] thermal: sy7636a: Add thermal driver for sy7636a

Message ID 20210708115804.212-2-alistair@alistair23.me
State New
Headers show
Series [v7,1/6] mfd: sy7636a: Initial commit | expand

Commit Message

Alistair July 8, 2021, 11:58 a.m. UTC
From: Lars Ivar Miljeteig <lars.ivar.miljeteig@remarkable.no>


Add thermal driver to enable kernel based polling
and shutdown of device for temperatures out of spec

Signed-off-by: Alistair Francis <alistair@alistair23.me>

---
 drivers/thermal/Kconfig           |   7 ++
 drivers/thermal/Makefile          |   1 +
 drivers/thermal/sy7636a_thermal.c | 107 ++++++++++++++++++++++++++++++
 3 files changed, 115 insertions(+)
 create mode 100644 drivers/thermal/sy7636a_thermal.c

-- 
2.31.1

Comments

Lee Jones July 20, 2021, 3:02 p.m. UTC | #1
On Thu, 08 Jul 2021, Alistair Francis wrote:

> From: Lars Ivar Miljeteig <lars.ivar.miljeteig@remarkable.no>

> 

> Add thermal driver to enable kernel based polling

> and shutdown of device for temperatures out of spec

> 

> Signed-off-by: Alistair Francis <alistair@alistair23.me>

> ---

>  drivers/thermal/Kconfig           |   7 ++

>  drivers/thermal/Makefile          |   1 +

>  drivers/thermal/sy7636a_thermal.c | 107 ++++++++++++++++++++++++++++++

>  3 files changed, 115 insertions(+)

>  create mode 100644 drivers/thermal/sy7636a_thermal.c

> 

> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig

> index d7f44deab5b1..7112c63d9021 100644

> --- a/drivers/thermal/Kconfig

> +++ b/drivers/thermal/Kconfig

> @@ -450,6 +450,13 @@ depends on (ARCH_STI || ARCH_STM32) && OF

>  source "drivers/thermal/st/Kconfig"

>  endmenu

>  

> +config SY7636A_THERMAL

> +	tristate "SY7636A thermal management"

> +	depends on MFD_SY7636A

> +	help

> +	  Enable the sy7636a thermal driver, which supports the

> +	  temperature sensor embedded in Silabs SY7636A chip.

> +

>  source "drivers/thermal/tegra/Kconfig"

>  

>  config GENERIC_ADC_THERMAL

> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile

> index 82fc3e616e54..2e1aca8a0a09 100644

> --- a/drivers/thermal/Makefile

> +++ b/drivers/thermal/Makefile

> @@ -51,6 +51,7 @@ obj-$(CONFIG_DA9062_THERMAL)	+= da9062-thermal.o

>  obj-y				+= intel/

>  obj-$(CONFIG_TI_SOC_THERMAL)	+= ti-soc-thermal/

>  obj-y				+= st/

> +obj-$(CONFIG_SY7636A_THERMAL)	+= sy7636a_thermal.o

>  obj-$(CONFIG_QCOM_TSENS)	+= qcom/

>  obj-y				+= tegra/

>  obj-$(CONFIG_HISI_THERMAL)     += hisi_thermal.o

> diff --git a/drivers/thermal/sy7636a_thermal.c b/drivers/thermal/sy7636a_thermal.c

> new file mode 100644

> index 000000000000..705a16fb1045

> --- /dev/null

> +++ b/drivers/thermal/sy7636a_thermal.c

> @@ -0,0 +1,107 @@

> +// SPDX-License-Identifier: GPL-2.0+

> +//

> +// Functions to access SY3686A power management chip temperature

> +//

> +// Copyright (C) 2019 reMarkable AS - http://www.remarkable.com/

> +//

> +// Authors: Lars Ivar Miljeteig <lars.ivar.miljeteig@remarkable.com>

> +//          Alistair Francis <alistair@alistair23.me>

> +

> +#include <linux/module.h>

> +#include <linux/platform_device.h>

> +#include <linux/thermal.h>

> +

> +#include <linux/mfd/sy7636a.h>

> +

> +struct sy7636a_data {

> +	struct sy7636a *sy7636a;

> +	struct thermal_zone_device *thermal_zone_dev;

> +};

> +

> +static int sy7636a_get_temp(void *arg, int *res)

> +{

> +	unsigned int reg_val, mode_ctr;

> +	int ret;

> +	struct sy7636a_data *data = arg;

> +	bool isVoltageActive;

> +

> +	ret = regmap_read(data->sy7636a->regmap,

> +			SY7636A_REG_OPERATION_MODE_CRL, &mode_ctr);

> +	if (ret)

> +		return ret;

> +

> +	isVoltageActive = mode_ctr & SY7636A_OPERATION_MODE_CRL_ONOFF;

> +

> +	if (!isVoltageActive) {

> +		ret = regmap_write(data->sy7636a->regmap,

> +				SY7636A_REG_OPERATION_MODE_CRL,

> +				mode_ctr | SY7636A_OPERATION_MODE_CRL_ONOFF);

> +		if (ret)

> +			return ret;

> +	}

> +

> +	ret = regmap_read(data->sy7636a->regmap,

> +			SY7636A_REG_TERMISTOR_READOUT, &reg_val);

> +	if (ret)

> +		return ret;

> +

> +	if (!isVoltageActive) {

> +		ret = regmap_write(data->sy7636a->regmap,

> +				SY7636A_REG_OPERATION_MODE_CRL,

> +				mode_ctr);

> +		if (ret)

> +			return ret;

> +	}

> +

> +	*res = *((signed char*)&reg_val);

> +	*res *= 1000;

> +

> +	return ret;

> +}

> +

> +static const struct thermal_zone_of_device_ops ops = {

> +	.get_temp	= sy7636a_get_temp,

> +};

> +

> +static int sy7636a_thermal_probe(struct platform_device *pdev)

> +{

> +	struct sy7636a *sy7636a = dev_get_drvdata(pdev->dev.parent);

> +	struct sy7636a_data *data;

> +

> +	if (!sy7636a)

> +		return -EPROBE_DEFER;


How is this possible?

> +	data = devm_kzalloc(&pdev->dev, sizeof(struct sy7636a_data), GFP_KERNEL);


sizeof(*data)

> +	if (!data)

> +		return -ENOMEM;

> +

> +	platform_set_drvdata(pdev, data);

> +

> +	data->sy7636a = sy7636a;

> +	data->thermal_zone_dev = devm_thermal_zone_of_sensor_register(

> +			pdev->dev.parent,

> +			0,

> +			data,


Why don't you just pass in your initial ddata?

> +			&ops);

> +

> +	return PTR_ERR_OR_ZERO(data->thermal_zone_dev);

> +}

> +

> +static const struct platform_device_id sy7636a_thermal_id_table[] = {

> +	{ "sy7636a-thermal", },

> +	{ }

> +};

> +MODULE_DEVICE_TABLE(platform, sy7636a_thermal_id_table);

> +

> +static struct platform_driver sy7636a_thermal_driver = {

> +	.driver = {

> +		.name = "sy7636a-thermal",

> +	},

> +	.probe = sy7636a_thermal_probe,

> +	.id_table = sy7636a_thermal_id_table,

> +};

> +module_platform_driver(sy7636a_thermal_driver);

> +

> +MODULE_AUTHOR("Lars Ivar Miljeteig <lars.ivar.miljeteig@remarkable.com>");

> +MODULE_DESCRIPTION("SY7636A thermal driver");

> +MODULE_LICENSE("GPL v2");


-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
Alistair Francis July 28, 2021, 8:23 a.m. UTC | #2
On Wed, Jul 21, 2021 at 1:02 AM Lee Jones <lee.jones@linaro.org> wrote:
>

> On Thu, 08 Jul 2021, Alistair Francis wrote:

>

> > From: Lars Ivar Miljeteig <lars.ivar.miljeteig@remarkable.no>

> >

> > Add thermal driver to enable kernel based polling

> > and shutdown of device for temperatures out of spec

> >

> > Signed-off-by: Alistair Francis <alistair@alistair23.me>

> > ---

> >  drivers/thermal/Kconfig           |   7 ++

> >  drivers/thermal/Makefile          |   1 +

> >  drivers/thermal/sy7636a_thermal.c | 107 ++++++++++++++++++++++++++++++

> >  3 files changed, 115 insertions(+)

> >  create mode 100644 drivers/thermal/sy7636a_thermal.c

> >

> > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig

> > index d7f44deab5b1..7112c63d9021 100644

> > --- a/drivers/thermal/Kconfig

> > +++ b/drivers/thermal/Kconfig

> > @@ -450,6 +450,13 @@ depends on (ARCH_STI || ARCH_STM32) && OF

> >  source "drivers/thermal/st/Kconfig"

> >  endmenu

> >

> > +config SY7636A_THERMAL

> > +     tristate "SY7636A thermal management"

> > +     depends on MFD_SY7636A

> > +     help

> > +       Enable the sy7636a thermal driver, which supports the

> > +       temperature sensor embedded in Silabs SY7636A chip.

> > +

> >  source "drivers/thermal/tegra/Kconfig"

> >

> >  config GENERIC_ADC_THERMAL

> > diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile

> > index 82fc3e616e54..2e1aca8a0a09 100644

> > --- a/drivers/thermal/Makefile

> > +++ b/drivers/thermal/Makefile

> > @@ -51,6 +51,7 @@ obj-$(CONFIG_DA9062_THERMAL)        += da9062-thermal.o

> >  obj-y                                += intel/

> >  obj-$(CONFIG_TI_SOC_THERMAL) += ti-soc-thermal/

> >  obj-y                                += st/

> > +obj-$(CONFIG_SY7636A_THERMAL)        += sy7636a_thermal.o

> >  obj-$(CONFIG_QCOM_TSENS)     += qcom/

> >  obj-y                                += tegra/

> >  obj-$(CONFIG_HISI_THERMAL)     += hisi_thermal.o

> > diff --git a/drivers/thermal/sy7636a_thermal.c b/drivers/thermal/sy7636a_thermal.c

> > new file mode 100644

> > index 000000000000..705a16fb1045

> > --- /dev/null

> > +++ b/drivers/thermal/sy7636a_thermal.c

> > @@ -0,0 +1,107 @@

> > +// SPDX-License-Identifier: GPL-2.0+

> > +//

> > +// Functions to access SY3686A power management chip temperature

> > +//

> > +// Copyright (C) 2019 reMarkable AS - http://www.remarkable.com/

> > +//

> > +// Authors: Lars Ivar Miljeteig <lars.ivar.miljeteig@remarkable.com>

> > +//          Alistair Francis <alistair@alistair23.me>

> > +

> > +#include <linux/module.h>

> > +#include <linux/platform_device.h>

> > +#include <linux/thermal.h>

> > +

> > +#include <linux/mfd/sy7636a.h>

> > +

> > +struct sy7636a_data {

> > +     struct sy7636a *sy7636a;

> > +     struct thermal_zone_device *thermal_zone_dev;

> > +};

> > +

> > +static int sy7636a_get_temp(void *arg, int *res)

> > +{

> > +     unsigned int reg_val, mode_ctr;

> > +     int ret;

> > +     struct sy7636a_data *data = arg;

> > +     bool isVoltageActive;

> > +

> > +     ret = regmap_read(data->sy7636a->regmap,

> > +                     SY7636A_REG_OPERATION_MODE_CRL, &mode_ctr);

> > +     if (ret)

> > +             return ret;

> > +

> > +     isVoltageActive = mode_ctr & SY7636A_OPERATION_MODE_CRL_ONOFF;

> > +

> > +     if (!isVoltageActive) {

> > +             ret = regmap_write(data->sy7636a->regmap,

> > +                             SY7636A_REG_OPERATION_MODE_CRL,

> > +                             mode_ctr | SY7636A_OPERATION_MODE_CRL_ONOFF);

> > +             if (ret)

> > +                     return ret;

> > +     }

> > +

> > +     ret = regmap_read(data->sy7636a->regmap,

> > +                     SY7636A_REG_TERMISTOR_READOUT, &reg_val);

> > +     if (ret)

> > +             return ret;

> > +

> > +     if (!isVoltageActive) {

> > +             ret = regmap_write(data->sy7636a->regmap,

> > +                             SY7636A_REG_OPERATION_MODE_CRL,

> > +                             mode_ctr);

> > +             if (ret)

> > +                     return ret;

> > +     }

> > +

> > +     *res = *((signed char*)&reg_val);

> > +     *res *= 1000;

> > +

> > +     return ret;

> > +}

> > +

> > +static const struct thermal_zone_of_device_ops ops = {

> > +     .get_temp       = sy7636a_get_temp,

> > +};

> > +

> > +static int sy7636a_thermal_probe(struct platform_device *pdev)

> > +{

> > +     struct sy7636a *sy7636a = dev_get_drvdata(pdev->dev.parent);

> > +     struct sy7636a_data *data;

> > +

> > +     if (!sy7636a)

> > +             return -EPROBE_DEFER;

>

> How is this possible?


I'm not sure, I have removed this.

>

> > +     data = devm_kzalloc(&pdev->dev, sizeof(struct sy7636a_data), GFP_KERNEL);

>

> sizeof(*data)

>

> > +     if (!data)

> > +             return -ENOMEM;

> > +

> > +     platform_set_drvdata(pdev, data);

> > +

> > +     data->sy7636a = sy7636a;

> > +     data->thermal_zone_dev = devm_thermal_zone_of_sensor_register(

> > +                     pdev->dev.parent,

> > +                     0,

> > +                     data,

>

> Why don't you just pass in your initial ddata?


I'm not sure what you mean, which data?

Alistair
Lee Jones Aug. 2, 2021, 7:55 a.m. UTC | #3
On Wed, 28 Jul 2021, Alistair Francis wrote:

> On Wed, Jul 21, 2021 at 1:02 AM Lee Jones <lee.jones@linaro.org> wrote:

> >

> > On Thu, 08 Jul 2021, Alistair Francis wrote:

> >

> > > From: Lars Ivar Miljeteig <lars.ivar.miljeteig@remarkable.no>

> > >

> > > Add thermal driver to enable kernel based polling

> > > and shutdown of device for temperatures out of spec

> > >

> > > Signed-off-by: Alistair Francis <alistair@alistair23.me>

> > > ---

> > >  drivers/thermal/Kconfig           |   7 ++

> > >  drivers/thermal/Makefile          |   1 +

> > >  drivers/thermal/sy7636a_thermal.c | 107 ++++++++++++++++++++++++++++++

> > >  3 files changed, 115 insertions(+)

> > >  create mode 100644 drivers/thermal/sy7636a_thermal.c

> > >

> > > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig

> > > index d7f44deab5b1..7112c63d9021 100644

> > > --- a/drivers/thermal/Kconfig

> > > +++ b/drivers/thermal/Kconfig

> > > @@ -450,6 +450,13 @@ depends on (ARCH_STI || ARCH_STM32) && OF

> > >  source "drivers/thermal/st/Kconfig"

> > >  endmenu

> > >

> > > +config SY7636A_THERMAL

> > > +     tristate "SY7636A thermal management"

> > > +     depends on MFD_SY7636A

> > > +     help

> > > +       Enable the sy7636a thermal driver, which supports the

> > > +       temperature sensor embedded in Silabs SY7636A chip.

> > > +

> > >  source "drivers/thermal/tegra/Kconfig"

> > >

> > >  config GENERIC_ADC_THERMAL

> > > diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile

> > > index 82fc3e616e54..2e1aca8a0a09 100644

> > > --- a/drivers/thermal/Makefile

> > > +++ b/drivers/thermal/Makefile

> > > @@ -51,6 +51,7 @@ obj-$(CONFIG_DA9062_THERMAL)        += da9062-thermal.o

> > >  obj-y                                += intel/

> > >  obj-$(CONFIG_TI_SOC_THERMAL) += ti-soc-thermal/

> > >  obj-y                                += st/

> > > +obj-$(CONFIG_SY7636A_THERMAL)        += sy7636a_thermal.o

> > >  obj-$(CONFIG_QCOM_TSENS)     += qcom/

> > >  obj-y                                += tegra/

> > >  obj-$(CONFIG_HISI_THERMAL)     += hisi_thermal.o

> > > diff --git a/drivers/thermal/sy7636a_thermal.c b/drivers/thermal/sy7636a_thermal.c

> > > new file mode 100644

> > > index 000000000000..705a16fb1045

> > > --- /dev/null

> > > +++ b/drivers/thermal/sy7636a_thermal.c

> > > @@ -0,0 +1,107 @@

> > > +// SPDX-License-Identifier: GPL-2.0+

> > > +//

> > > +// Functions to access SY3686A power management chip temperature

> > > +//

> > > +// Copyright (C) 2019 reMarkable AS - http://www.remarkable.com/

> > > +//

> > > +// Authors: Lars Ivar Miljeteig <lars.ivar.miljeteig@remarkable.com>

> > > +//          Alistair Francis <alistair@alistair23.me>

> > > +

> > > +#include <linux/module.h>

> > > +#include <linux/platform_device.h>

> > > +#include <linux/thermal.h>

> > > +

> > > +#include <linux/mfd/sy7636a.h>

> > > +

> > > +struct sy7636a_data {

> > > +     struct sy7636a *sy7636a;

> > > +     struct thermal_zone_device *thermal_zone_dev;

> > > +};

> > > +

> > > +static int sy7636a_get_temp(void *arg, int *res)

> > > +{

> > > +     unsigned int reg_val, mode_ctr;

> > > +     int ret;

> > > +     struct sy7636a_data *data = arg;

> > > +     bool isVoltageActive;

> > > +

> > > +     ret = regmap_read(data->sy7636a->regmap,

> > > +                     SY7636A_REG_OPERATION_MODE_CRL, &mode_ctr);

> > > +     if (ret)

> > > +             return ret;

> > > +

> > > +     isVoltageActive = mode_ctr & SY7636A_OPERATION_MODE_CRL_ONOFF;

> > > +

> > > +     if (!isVoltageActive) {

> > > +             ret = regmap_write(data->sy7636a->regmap,

> > > +                             SY7636A_REG_OPERATION_MODE_CRL,

> > > +                             mode_ctr | SY7636A_OPERATION_MODE_CRL_ONOFF);

> > > +             if (ret)

> > > +                     return ret;

> > > +     }

> > > +

> > > +     ret = regmap_read(data->sy7636a->regmap,

> > > +                     SY7636A_REG_TERMISTOR_READOUT, &reg_val);

> > > +     if (ret)

> > > +             return ret;

> > > +

> > > +     if (!isVoltageActive) {

> > > +             ret = regmap_write(data->sy7636a->regmap,

> > > +                             SY7636A_REG_OPERATION_MODE_CRL,

> > > +                             mode_ctr);

> > > +             if (ret)

> > > +                     return ret;

> > > +     }

> > > +

> > > +     *res = *((signed char*)&reg_val);

> > > +     *res *= 1000;

> > > +

> > > +     return ret;

> > > +}

> > > +

> > > +static const struct thermal_zone_of_device_ops ops = {

> > > +     .get_temp       = sy7636a_get_temp,

> > > +};

> > > +

> > > +static int sy7636a_thermal_probe(struct platform_device *pdev)

> > > +{

> > > +     struct sy7636a *sy7636a = dev_get_drvdata(pdev->dev.parent);

> > > +     struct sy7636a_data *data;

> > > +

> > > +     if (!sy7636a)

> > > +             return -EPROBE_DEFER;

> >

> > How is this possible?

> 

> I'm not sure, I have removed this.

> 

> >

> > > +     data = devm_kzalloc(&pdev->dev, sizeof(struct sy7636a_data), GFP_KERNEL);

> >

> > sizeof(*data)

> >

> > > +     if (!data)

> > > +             return -ENOMEM;

> > > +

> > > +     platform_set_drvdata(pdev, data);

> > > +

> > > +     data->sy7636a = sy7636a;

> > > +     data->thermal_zone_dev = devm_thermal_zone_of_sensor_register(

> > > +                     pdev->dev.parent,

> > > +                     0,

> > > +                     data,

> >

> > Why don't you just pass in your initial ddata?

> 

> I'm not sure what you mean, which data?


sy7636a

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
Daniel Lezcano Aug. 14, 2021, 11:08 a.m. UTC | #4
On 08/07/2021 13:58, Alistair Francis wrote:
> From: Lars Ivar Miljeteig <lars.ivar.miljeteig@remarkable.no>

> 

> Add thermal driver to enable kernel based polling

> and shutdown of device for temperatures out of spec

> 

> Signed-off-by: Alistair Francis <alistair@alistair23.me>

> ---

>  drivers/thermal/Kconfig           |   7 ++

>  drivers/thermal/Makefile          |   1 +

>  drivers/thermal/sy7636a_thermal.c | 107 ++++++++++++++++++++++++++++++

>  3 files changed, 115 insertions(+)

>  create mode 100644 drivers/thermal/sy7636a_thermal.c

> 

> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig

> index d7f44deab5b1..7112c63d9021 100644

> --- a/drivers/thermal/Kconfig

> +++ b/drivers/thermal/Kconfig

> @@ -450,6 +450,13 @@ depends on (ARCH_STI || ARCH_STM32) && OF

>  source "drivers/thermal/st/Kconfig"

>  endmenu

>  

> +config SY7636A_THERMAL

> +	tristate "SY7636A thermal management"

> +	depends on MFD_SY7636A

> +	help

> +	  Enable the sy7636a thermal driver, which supports the

> +	  temperature sensor embedded in Silabs SY7636A chip.

> +

>  source "drivers/thermal/tegra/Kconfig"

>  

>  config GENERIC_ADC_THERMAL

> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile

> index 82fc3e616e54..2e1aca8a0a09 100644

> --- a/drivers/thermal/Makefile

> +++ b/drivers/thermal/Makefile

> @@ -51,6 +51,7 @@ obj-$(CONFIG_DA9062_THERMAL)	+= da9062-thermal.o

>  obj-y				+= intel/

>  obj-$(CONFIG_TI_SOC_THERMAL)	+= ti-soc-thermal/

>  obj-y				+= st/

> +obj-$(CONFIG_SY7636A_THERMAL)	+= sy7636a_thermal.o

>  obj-$(CONFIG_QCOM_TSENS)	+= qcom/

>  obj-y				+= tegra/

>  obj-$(CONFIG_HISI_THERMAL)     += hisi_thermal.o

> diff --git a/drivers/thermal/sy7636a_thermal.c b/drivers/thermal/sy7636a_thermal.c

> new file mode 100644

> index 000000000000..705a16fb1045

> --- /dev/null

> +++ b/drivers/thermal/sy7636a_thermal.c

> @@ -0,0 +1,107 @@

> +// SPDX-License-Identifier: GPL-2.0+

> +//

> +// Functions to access SY3686A power management chip temperature

> +//

> +// Copyright (C) 2019 reMarkable AS - http://www.remarkable.com/

> +//

> +// Authors: Lars Ivar Miljeteig <lars.ivar.miljeteig@remarkable.com>

> +//          Alistair Francis <alistair@alistair23.me>


// SPDX-License-Identifier: GPL-2.0+
/*
 * Functions to access SY3686A power management chip temperature
 * ...
 *
 */

> +#include <linux/module.h>

> +#include <linux/platform_device.h>

> +#include <linux/thermal.h>

> +

> +#include <linux/mfd/sy7636a.h>

> +

> +struct sy7636a_data {

> +	struct sy7636a *sy7636a;


Only regmap is used AFAICT, so storing the regmap pointer is enough and
the structure is not needed neither the code associated with it.

> +	struct thermal_zone_device *thermal_zone_dev;


This field is only used to check the return code of the
thermal_zone_device_register function. It can be local to the function
below.

> +};

> +

> +static int sy7636a_get_temp(void *arg, int *res)

> +{

> +	unsigned int reg_val, mode_ctr;


Is it possible the sensor returns negative values ?

> +	int ret;

> +	struct sy7636a_data *data = arg;

> +	bool isVoltageActive;

> +	ret = regmap_read(data->sy7636a->regmap,

> +			SY7636A_REG_OPERATION_MODE_CRL, &mode_ctr);

> +	if (ret)

> +		return ret;

> +

> +	isVoltageActive = mode_ctr & SY7636A_OPERATION_MODE_CRL_ONOFF;

> +

> +	if (!isVoltageActive) {

> +		ret = regmap_write(data->sy7636a->regmap,

> +				SY7636A_REG_OPERATION_MODE_CRL,

> +				mode_ctr | SY7636A_OPERATION_MODE_CRL_ONOFF);


What is this conditional block ? Please add a comment to explain why
these 'isVoltageActive' test are needed.

> +		if (ret)

> +			return ret;

> +	}

> +

> +	ret = regmap_read(data->sy7636a->regmap,

> +			SY7636A_REG_TERMISTOR_READOUT, &reg_val);

> +	if (ret)

> +		return ret;

> +

> +	if (!isVoltageActive) {

> +		ret = regmap_write(data->sy7636a->regmap,

> +				SY7636A_REG_OPERATION_MODE_CRL,

> +				mode_ctr);

> +		if (ret)

> +			return ret;

> +	}

> +

> +	*res = *((signed char*)&reg_val);


Please revisit this.

> +	*res *= 1000;

> +

> +	return ret;

> +}

> +

> +static const struct thermal_zone_of_device_ops ops = {

> +	.get_temp	= sy7636a_get_temp,

> +};

> +

> +static int sy7636a_thermal_probe(struct platform_device *pdev)

> +{

> +	struct sy7636a *sy7636a = dev_get_drvdata(pdev->dev.parent);

> +	struct sy7636a_data *data;

> +

> +	if (!sy7636a)

> +		return -EPROBE_DEFER;

> +

> +	data = devm_kzalloc(&pdev->dev, sizeof(struct sy7636a_data), GFP_KERNEL);

> +	if (!data)

> +		return -ENOMEM;

> +

> +	platform_set_drvdata(pdev, data);

> +

> +	data->sy7636a = sy7636a;

> +	data->thermal_zone_dev = devm_thermal_zone_of_sensor_register(

> +			pdev->dev.parent,

> +			0,

> +			data,

> +			&ops);

> +

> +	return PTR_ERR_OR_ZERO(data->thermal_zone_dev);

> +}

> +

> +static const struct platform_device_id sy7636a_thermal_id_table[] = {

> +	{ "sy7636a-thermal", },

> +	{ }

> +};

> +MODULE_DEVICE_TABLE(platform, sy7636a_thermal_id_table);

> +

> +static struct platform_driver sy7636a_thermal_driver = {

> +	.driver = {

> +		.name = "sy7636a-thermal",

> +	},

> +	.probe = sy7636a_thermal_probe,

> +	.id_table = sy7636a_thermal_id_table,

> +};

> +module_platform_driver(sy7636a_thermal_driver);

> +

> +MODULE_AUTHOR("Lars Ivar Miljeteig <lars.ivar.miljeteig@remarkable.com>");

> +MODULE_DESCRIPTION("SY7636A thermal driver");

> +MODULE_LICENSE("GPL v2");

> 



-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
diff mbox series

Patch

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index d7f44deab5b1..7112c63d9021 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -450,6 +450,13 @@  depends on (ARCH_STI || ARCH_STM32) && OF
 source "drivers/thermal/st/Kconfig"
 endmenu
 
+config SY7636A_THERMAL
+	tristate "SY7636A thermal management"
+	depends on MFD_SY7636A
+	help
+	  Enable the sy7636a thermal driver, which supports the
+	  temperature sensor embedded in Silabs SY7636A chip.
+
 source "drivers/thermal/tegra/Kconfig"
 
 config GENERIC_ADC_THERMAL
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 82fc3e616e54..2e1aca8a0a09 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -51,6 +51,7 @@  obj-$(CONFIG_DA9062_THERMAL)	+= da9062-thermal.o
 obj-y				+= intel/
 obj-$(CONFIG_TI_SOC_THERMAL)	+= ti-soc-thermal/
 obj-y				+= st/
+obj-$(CONFIG_SY7636A_THERMAL)	+= sy7636a_thermal.o
 obj-$(CONFIG_QCOM_TSENS)	+= qcom/
 obj-y				+= tegra/
 obj-$(CONFIG_HISI_THERMAL)     += hisi_thermal.o
diff --git a/drivers/thermal/sy7636a_thermal.c b/drivers/thermal/sy7636a_thermal.c
new file mode 100644
index 000000000000..705a16fb1045
--- /dev/null
+++ b/drivers/thermal/sy7636a_thermal.c
@@ -0,0 +1,107 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+//
+// Functions to access SY3686A power management chip temperature
+//
+// Copyright (C) 2019 reMarkable AS - http://www.remarkable.com/
+//
+// Authors: Lars Ivar Miljeteig <lars.ivar.miljeteig@remarkable.com>
+//          Alistair Francis <alistair@alistair23.me>
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/thermal.h>
+
+#include <linux/mfd/sy7636a.h>
+
+struct sy7636a_data {
+	struct sy7636a *sy7636a;
+	struct thermal_zone_device *thermal_zone_dev;
+};
+
+static int sy7636a_get_temp(void *arg, int *res)
+{
+	unsigned int reg_val, mode_ctr;
+	int ret;
+	struct sy7636a_data *data = arg;
+	bool isVoltageActive;
+
+	ret = regmap_read(data->sy7636a->regmap,
+			SY7636A_REG_OPERATION_MODE_CRL, &mode_ctr);
+	if (ret)
+		return ret;
+
+	isVoltageActive = mode_ctr & SY7636A_OPERATION_MODE_CRL_ONOFF;
+
+	if (!isVoltageActive) {
+		ret = regmap_write(data->sy7636a->regmap,
+				SY7636A_REG_OPERATION_MODE_CRL,
+				mode_ctr | SY7636A_OPERATION_MODE_CRL_ONOFF);
+		if (ret)
+			return ret;
+	}
+
+	ret = regmap_read(data->sy7636a->regmap,
+			SY7636A_REG_TERMISTOR_READOUT, &reg_val);
+	if (ret)
+		return ret;
+
+	if (!isVoltageActive) {
+		ret = regmap_write(data->sy7636a->regmap,
+				SY7636A_REG_OPERATION_MODE_CRL,
+				mode_ctr);
+		if (ret)
+			return ret;
+	}
+
+	*res = *((signed char*)&reg_val);
+	*res *= 1000;
+
+	return ret;
+}
+
+static const struct thermal_zone_of_device_ops ops = {
+	.get_temp	= sy7636a_get_temp,
+};
+
+static int sy7636a_thermal_probe(struct platform_device *pdev)
+{
+	struct sy7636a *sy7636a = dev_get_drvdata(pdev->dev.parent);
+	struct sy7636a_data *data;
+
+	if (!sy7636a)
+		return -EPROBE_DEFER;
+
+	data = devm_kzalloc(&pdev->dev, sizeof(struct sy7636a_data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, data);
+
+	data->sy7636a = sy7636a;
+	data->thermal_zone_dev = devm_thermal_zone_of_sensor_register(
+			pdev->dev.parent,
+			0,
+			data,
+			&ops);
+
+	return PTR_ERR_OR_ZERO(data->thermal_zone_dev);
+}
+
+static const struct platform_device_id sy7636a_thermal_id_table[] = {
+	{ "sy7636a-thermal", },
+	{ }
+};
+MODULE_DEVICE_TABLE(platform, sy7636a_thermal_id_table);
+
+static struct platform_driver sy7636a_thermal_driver = {
+	.driver = {
+		.name = "sy7636a-thermal",
+	},
+	.probe = sy7636a_thermal_probe,
+	.id_table = sy7636a_thermal_id_table,
+};
+module_platform_driver(sy7636a_thermal_driver);
+
+MODULE_AUTHOR("Lars Ivar Miljeteig <lars.ivar.miljeteig@remarkable.com>");
+MODULE_DESCRIPTION("SY7636A thermal driver");
+MODULE_LICENSE("GPL v2");