* [PATCH 0/3] Add support of New Amlogic temperature sensor for G12A SoCs @ 2019-06-04 14:47 Guillaume La Roque 2019-06-04 14:47 ` [PATCH 1/3] Documentation: dt-bindings: add the Amlogic Meson Temperature Sensor Guillaume La Roque ` (2 more replies) 0 siblings, 3 replies; 11+ messages in thread From: Guillaume La Roque @ 2019-06-04 14:47 UTC (permalink / raw) To: jic23, khilman Cc: linux-iio, devicetree, linux-kernel, linux-arm-kernel, linux-amlogic This patchs series add support of New Amlogic temperature sensor. This driver is based on IIO frameworks. formulas and calibration values come from amlogic. Dependencies : - patch 2: temperature sensor clock are needed [1] [1] https://lkml.kernel.org/r/20190412100221.26740-1-glaroque@baylibre.com Guillaume La Roque (3): Documentation: dt-bindings: add the Amlogic Meson Temperature Sensor arm64: dts: meson: g12a: add temperature sensor node iio: temperature: add a driver for the temperature sensor found in Amlogic Meson G12 SoCs .../iio/temperature/amlogic,meson-tsensor.txt | 31 ++ arch/arm64/boot/dts/amlogic/meson-g12a.dtsi | 22 + drivers/iio/temperature/Kconfig | 11 + drivers/iio/temperature/Makefile | 1 + drivers/iio/temperature/meson_tsensor.c | 416 ++++++++++++++++++ 5 files changed, 481 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/temperature/amlogic,meson-tsensor.txt create mode 100644 drivers/iio/temperature/meson_tsensor.c -- 2.17.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/3] Documentation: dt-bindings: add the Amlogic Meson Temperature Sensor 2019-06-04 14:47 [PATCH 0/3] Add support of New Amlogic temperature sensor for G12A SoCs Guillaume La Roque @ 2019-06-04 14:47 ` Guillaume La Roque 2019-06-06 19:16 ` Martin Blumenstingl 2019-06-08 13:08 ` Jonathan Cameron 2019-06-04 14:47 ` [PATCH 2/3] arm64: dts: meson: g12a: add temperature sensor node Guillaume La Roque 2019-06-04 14:47 ` [PATCH 3/3] iio: temperature: add a driver for the temperature sensor found in Amlogic Meson G12 SoCs Guillaume La Roque 2 siblings, 2 replies; 11+ messages in thread From: Guillaume La Roque @ 2019-06-04 14:47 UTC (permalink / raw) To: jic23, khilman Cc: linux-iio, devicetree, linux-kernel, linux-arm-kernel, linux-amlogic This adds the devicetree binding documentation for the Temperature Sensor found in the Amlogic Meson G12 SoCs. Currently only the G12A SoCs are supported. Signed-off-by: Guillaume La Roque <glaroque@baylibre.com> --- .../iio/temperature/amlogic,meson-tsensor.txt | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/temperature/amlogic,meson-tsensor.txt diff --git a/Documentation/devicetree/bindings/iio/temperature/amlogic,meson-tsensor.txt b/Documentation/devicetree/bindings/iio/temperature/amlogic,meson-tsensor.txt new file mode 100644 index 000000000000..d064db0e9cac --- /dev/null +++ b/Documentation/devicetree/bindings/iio/temperature/amlogic,meson-tsensor.txt @@ -0,0 +1,31 @@ +* Amlogic Meson Temperature Sensor + +Required properties: +- compatible: depending on the SoC and the position of the sensor, + this should be one of: + - "amlogic,meson-g12a-cpu-tsensor" for the CPU G12A SoC sensor + - "amlogic,meson-g12a-ddr-tsensor" for the DDR G12A SoC sensor + followed by the common : + - "amlogic,meson-g12a-tsensor" for G12A SoC family +- reg: the physical base address and length of the registers +- interrupts: the interrupt indicating end of sampling +- clocks: phandle identifier for the reference clock of temperature sensor +- #io-channel-cells: must be 1, see ../iio-bindings.txt +- amlogic,ao-secure: phandle to the ao-secure syscon + +Optional properties: +- amlogic,critical-temperature: temperature value in milli degrees Celsius + to set automatic reboot on too high temperature + +Example: + cpu_temp: temperature-sensor@ff634800 { + compatible = "amlogic,meson-g12a-cpu-tsensor", + "amlogic,meson-g12a-tsensor"; + reg = <0x0 0xff634800 0x0 0x50>; + interrupts = <GIC_SPI 35 IRQ_TYPE_EDGE_RISING>; + clocks = <&clkc CLKID_TS>; + status = "okay"; + #io-channel-cells = <1>; + amlogic,meson-ao-secure = <&sec_AO>; + amlogic,critical-temperature = <115000>; + }; -- 2.17.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] Documentation: dt-bindings: add the Amlogic Meson Temperature Sensor 2019-06-04 14:47 ` [PATCH 1/3] Documentation: dt-bindings: add the Amlogic Meson Temperature Sensor Guillaume La Roque @ 2019-06-06 19:16 ` Martin Blumenstingl 2019-06-08 13:10 ` Jonathan Cameron 2019-06-11 11:01 ` Neil Armstrong 2019-06-08 13:08 ` Jonathan Cameron 1 sibling, 2 replies; 11+ messages in thread From: Martin Blumenstingl @ 2019-06-06 19:16 UTC (permalink / raw) To: Guillaume La Roque Cc: devicetree, linux-iio, khilman, linux-kernel, jic23, linux-amlogic, linux-arm-kernel Hi Guillaume, thank you for working on this! On Tue, Jun 4, 2019 at 4:47 PM Guillaume La Roque <glaroque@baylibre.com> wrote: > > This adds the devicetree binding documentation for the Temperature > Sensor found in the Amlogic Meson G12 SoCs. > Currently only the G12A SoCs are supported. so G12B is not supported (yet)? > Signed-off-by: Guillaume La Roque <glaroque@baylibre.com> > --- > .../iio/temperature/amlogic,meson-tsensor.txt | 31 +++++++++++++++++++ > 1 file changed, 31 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/temperature/amlogic,meson-tsensor.txt > > diff --git a/Documentation/devicetree/bindings/iio/temperature/amlogic,meson-tsensor.txt b/Documentation/devicetree/bindings/iio/temperature/amlogic,meson-tsensor.txt > new file mode 100644 > index 000000000000..d064db0e9cac > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/temperature/amlogic,meson-tsensor.txt > @@ -0,0 +1,31 @@ > +* Amlogic Meson Temperature Sensor > + > +Required properties: > +- compatible: depending on the SoC and the position of the sensor, > + this should be one of: > + - "amlogic,meson-g12a-cpu-tsensor" for the CPU G12A SoC sensor > + - "amlogic,meson-g12a-ddr-tsensor" for the DDR G12A SoC sensor > + followed by the common : > + - "amlogic,meson-g12a-tsensor" for G12A SoC family > +- reg: the physical base address and length of the registers > +- interrupts: the interrupt indicating end of sampling > +- clocks: phandle identifier for the reference clock of temperature sensor > +- #io-channel-cells: must be 1, see ../iio-bindings.txt have you considered using the thermal framework [0] instead of the iio framework (see below)? > +- amlogic,ao-secure: phandle to the ao-secure syscon the driver has some "u_efuse_off" access. do we need to get some calibration values from the AO syscon or can we also fetch it from the eFuse? you can look at arch/arm/boot/dts/meson8.dtsi where I'm passing the temperature sensor calibration data to the SAR ADC (there's no dedicated temperature sensor IP block prior to G12A) while reading the data from the eFuse > +Optional properties: > +- amlogic,critical-temperature: temperature value in milli degrees Celsius > + to set automatic reboot on too high temperature as far as I can tell the thermal framework supports multiple trip points. I'm seeing this as a benefit because the hardware can raise interrupts at four different temperatures (defined by the driver) > +Example: > + cpu_temp: temperature-sensor@ff634800 { > + compatible = "amlogic,meson-g12a-cpu-tsensor", > + "amlogic,meson-g12a-tsensor"; > + reg = <0x0 0xff634800 0x0 0x50>; > + interrupts = <GIC_SPI 35 IRQ_TYPE_EDGE_RISING>; > + clocks = <&clkc CLKID_TS>; > + status = "okay"; as far as I know the dt-bindings should not have a status property in the examples Martin _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] Documentation: dt-bindings: add the Amlogic Meson Temperature Sensor 2019-06-06 19:16 ` Martin Blumenstingl @ 2019-06-08 13:10 ` Jonathan Cameron 2019-06-11 11:01 ` Neil Armstrong 1 sibling, 0 replies; 11+ messages in thread From: Jonathan Cameron @ 2019-06-08 13:10 UTC (permalink / raw) To: Martin Blumenstingl Cc: devicetree, linux-iio, khilman, linux-kernel, Guillaume La Roque, linux-amlogic, linux-arm-kernel On Thu, 6 Jun 2019 21:16:37 +0200 Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: > Hi Guillaume, > > thank you for working on this! On comment from me inline. Thanks, Jonathan > > On Tue, Jun 4, 2019 at 4:47 PM Guillaume La Roque <glaroque@baylibre.com> wrote: > > > > This adds the devicetree binding documentation for the Temperature > > Sensor found in the Amlogic Meson G12 SoCs. > > Currently only the G12A SoCs are supported. > so G12B is not supported (yet)? > > > Signed-off-by: Guillaume La Roque <glaroque@baylibre.com> > > --- > > .../iio/temperature/amlogic,meson-tsensor.txt | 31 +++++++++++++++++++ > > 1 file changed, 31 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/iio/temperature/amlogic,meson-tsensor.txt > > > > diff --git a/Documentation/devicetree/bindings/iio/temperature/amlogic,meson-tsensor.txt b/Documentation/devicetree/bindings/iio/temperature/amlogic,meson-tsensor.txt > > new file mode 100644 > > index 000000000000..d064db0e9cac > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/iio/temperature/amlogic,meson-tsensor.txt > > @@ -0,0 +1,31 @@ > > +* Amlogic Meson Temperature Sensor > > + > > +Required properties: > > +- compatible: depending on the SoC and the position of the sensor, > > + this should be one of: > > + - "amlogic,meson-g12a-cpu-tsensor" for the CPU G12A SoC sensor > > + - "amlogic,meson-g12a-ddr-tsensor" for the DDR G12A SoC sensor > > + followed by the common : > > + - "amlogic,meson-g12a-tsensor" for G12A SoC family > > +- reg: the physical base address and length of the registers > > +- interrupts: the interrupt indicating end of sampling > > +- clocks: phandle identifier for the reference clock of temperature sensor > > +- #io-channel-cells: must be 1, see ../iio-bindings.txt > have you considered using the thermal framework [0] instead of the iio > framework (see below)? I had the same thought. Right now, this doesn't look generic enough for IIO to make that much sense. I'll review anyway as may give some useful pointers even if you do move it over to thermal. > > > +- amlogic,ao-secure: phandle to the ao-secure syscon > the driver has some "u_efuse_off" access. do we need to get some > calibration values from the AO syscon or can we also fetch it from the > eFuse? you can look at arch/arm/boot/dts/meson8.dtsi where I'm passing > the temperature sensor calibration data to the SAR ADC (there's no > dedicated temperature sensor IP block prior to G12A) while reading the > data from the eFuse > > > +Optional properties: > > +- amlogic,critical-temperature: temperature value in milli degrees Celsius > > + to set automatic reboot on too high temperature > as far as I can tell the thermal framework supports multiple trip > points. I'm seeing this as a benefit because the hardware can raise > interrupts at four different temperatures (defined by the driver) > > > +Example: > > + cpu_temp: temperature-sensor@ff634800 { > > + compatible = "amlogic,meson-g12a-cpu-tsensor", > > + "amlogic,meson-g12a-tsensor"; > > + reg = <0x0 0xff634800 0x0 0x50>; > > + interrupts = <GIC_SPI 35 IRQ_TYPE_EDGE_RISING>; > > + clocks = <&clkc CLKID_TS>; > > + status = "okay"; > as far as I know the dt-bindings should not have a status property in > the examples > > > Martin _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] Documentation: dt-bindings: add the Amlogic Meson Temperature Sensor 2019-06-06 19:16 ` Martin Blumenstingl 2019-06-08 13:10 ` Jonathan Cameron @ 2019-06-11 11:01 ` Neil Armstrong 2019-06-11 17:57 ` Martin Blumenstingl 1 sibling, 1 reply; 11+ messages in thread From: Neil Armstrong @ 2019-06-11 11:01 UTC (permalink / raw) To: Martin Blumenstingl, Guillaume La Roque Cc: devicetree, linux-iio, khilman, linux-kernel, linux-arm-kernel, linux-amlogic, jic23 On 06/06/2019 21:16, Martin Blumenstingl wrote: > Hi Guillaume, > > thank you for working on this! > > On Tue, Jun 4, 2019 at 4:47 PM Guillaume La Roque <glaroque@baylibre.com> wrote: >> >> This adds the devicetree binding documentation for the Temperature >> Sensor found in the Amlogic Meson G12 SoCs. >> Currently only the G12A SoCs are supported. > so G12B is not supported (yet)? G12B is 95% similar as G12A, it will certainly use slighly different values. > >> Signed-off-by: Guillaume La Roque <glaroque@baylibre.com> >> --- >> .../iio/temperature/amlogic,meson-tsensor.txt | 31 +++++++++++++++++++ >> 1 file changed, 31 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/iio/temperature/amlogic,meson-tsensor.txt >> >> diff --git a/Documentation/devicetree/bindings/iio/temperature/amlogic,meson-tsensor.txt b/Documentation/devicetree/bindings/iio/temperature/amlogic,meson-tsensor.txt >> new file mode 100644 >> index 000000000000..d064db0e9cac >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/iio/temperature/amlogic,meson-tsensor.txt >> @@ -0,0 +1,31 @@ >> +* Amlogic Meson Temperature Sensor >> + >> +Required properties: >> +- compatible: depending on the SoC and the position of the sensor, >> + this should be one of: >> + - "amlogic,meson-g12a-cpu-tsensor" for the CPU G12A SoC sensor >> + - "amlogic,meson-g12a-ddr-tsensor" for the DDR G12A SoC sensor >> + followed by the common : >> + - "amlogic,meson-g12a-tsensor" for G12A SoC family >> +- reg: the physical base address and length of the registers >> +- interrupts: the interrupt indicating end of sampling >> +- clocks: phandle identifier for the reference clock of temperature sensor >> +- #io-channel-cells: must be 1, see ../iio-bindings.txt > have you considered using the thermal framework [0] instead of the iio > framework (see below)? Question: why thermal, and not hwmon ? what's the main difference ? > >> +- amlogic,ao-secure: phandle to the ao-secure syscon > the driver has some "u_efuse_off" access. do we need to get some > calibration values from the AO syscon or can we also fetch it from the > eFuse? you can look at arch/arm/boot/dts/meson8.dtsi where I'm passing > the temperature sensor calibration data to the SAR ADC (there's no > dedicated temperature sensor IP block prior to G12A) while reading the > data from the eFuse > >> +Optional properties: >> +- amlogic,critical-temperature: temperature value in milli degrees Celsius >> + to set automatic reboot on too high temperature > as far as I can tell the thermal framework supports multiple trip > points. I'm seeing this as a benefit because the hardware can raise > interrupts at four different temperatures (defined by the driver) Theoretically, but the implementation code differs a lot from the datasheet. > >> +Example: >> + cpu_temp: temperature-sensor@ff634800 { >> + compatible = "amlogic,meson-g12a-cpu-tsensor", >> + "amlogic,meson-g12a-tsensor"; >> + reg = <0x0 0xff634800 0x0 0x50>; >> + interrupts = <GIC_SPI 35 IRQ_TYPE_EDGE_RISING>; >> + clocks = <&clkc CLKID_TS>; >> + status = "okay"; > as far as I know the dt-bindings should not have a status property in > the examples > > > Martin > > _______________________________________________ > linux-amlogic mailing list > linux-amlogic@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-amlogic > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] Documentation: dt-bindings: add the Amlogic Meson Temperature Sensor 2019-06-11 11:01 ` Neil Armstrong @ 2019-06-11 17:57 ` Martin Blumenstingl 0 siblings, 0 replies; 11+ messages in thread From: Martin Blumenstingl @ 2019-06-11 17:57 UTC (permalink / raw) To: Neil Armstrong Cc: devicetree, linux-iio, khilman, linux-kernel, Guillaume La Roque, linux-arm-kernel, linux-amlogic, jic23 Hi Neil, On Tue, Jun 11, 2019 at 1:01 PM Neil Armstrong <narmstrong@baylibre.com> wrote: > > On 06/06/2019 21:16, Martin Blumenstingl wrote: > > Hi Guillaume, > > > > thank you for working on this! > > > > On Tue, Jun 4, 2019 at 4:47 PM Guillaume La Roque <glaroque@baylibre.com> wrote: > >> > >> This adds the devicetree binding documentation for the Temperature > >> Sensor found in the Amlogic Meson G12 SoCs. > >> Currently only the G12A SoCs are supported. > > so G12B is not supported (yet)? > > G12B is 95% similar as G12A, it will certainly use slighly different values. OK, thank you for clarifying this as far as I can tell Guillaume's code is already prepared for that (as there's a per-instance specific struct with settings for the specific instance) which is good to know > > > >> Signed-off-by: Guillaume La Roque <glaroque@baylibre.com> > >> --- > >> .../iio/temperature/amlogic,meson-tsensor.txt | 31 +++++++++++++++++++ > >> 1 file changed, 31 insertions(+) > >> create mode 100644 Documentation/devicetree/bindings/iio/temperature/amlogic,meson-tsensor.txt > >> > >> diff --git a/Documentation/devicetree/bindings/iio/temperature/amlogic,meson-tsensor.txt b/Documentation/devicetree/bindings/iio/temperature/amlogic,meson-tsensor.txt > >> new file mode 100644 > >> index 000000000000..d064db0e9cac > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/iio/temperature/amlogic,meson-tsensor.txt > >> @@ -0,0 +1,31 @@ > >> +* Amlogic Meson Temperature Sensor > >> + > >> +Required properties: > >> +- compatible: depending on the SoC and the position of the sensor, > >> + this should be one of: > >> + - "amlogic,meson-g12a-cpu-tsensor" for the CPU G12A SoC sensor > >> + - "amlogic,meson-g12a-ddr-tsensor" for the DDR G12A SoC sensor > >> + followed by the common : > >> + - "amlogic,meson-g12a-tsensor" for G12A SoC family > >> +- reg: the physical base address and length of the registers > >> +- interrupts: the interrupt indicating end of sampling > >> +- clocks: phandle identifier for the reference clock of temperature sensor > >> +- #io-channel-cells: must be 1, see ../iio-bindings.txt > > have you considered using the thermal framework [0] instead of the iio > > framework (see below)? > > Question: why thermal, and not hwmon ? what's the main difference ? this is what came to my mind why the thermal framework fits best (at least based on my current knowledge): a) the thermal-zones (see meson-gxm-khadas-vim2.dts for example) a "thermal-sensors" property. so for active (with a fan) or passive (by limiting the maximum frequency and thus the supply voltage) cooling we need a thermal device anyways b) the thermal bindings support multiple trip points. we can map them to one of the four interrupts which the IP block can generate c) defining a temperature where the chip will power off sounds like a use-case which may be implemented by other thermal IP blocks (in other words: maybe the thermal frameworks provides some generic property to replace the "amlogic,critical-temperature" property) d) as far as I know you can tell the thermal framework to create a hwmon device with only a couple (5?) lines of code as Guillaume has already shown we can implement c) with a custom property, but that's not limited to the underlying framework (IIO, hwmon, thermal, ...) use-case d) is not a strong one because I'm using iio-hwmon to create a hwmon device on the 32-bit SoCs. however, together with case a) using an IIO driver is going to be more difficult because currently there's "only" a "generic-adc-thermal" binding (but not a "generic-iio-temperature-thermal" binding) the initial driver version doesn't have to support everything I listed above. however, I believe with the thermal framework we don't limit ourselves to one use-case and can extend the driver in the future Martin _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] Documentation: dt-bindings: add the Amlogic Meson Temperature Sensor 2019-06-04 14:47 ` [PATCH 1/3] Documentation: dt-bindings: add the Amlogic Meson Temperature Sensor Guillaume La Roque 2019-06-06 19:16 ` Martin Blumenstingl @ 2019-06-08 13:08 ` Jonathan Cameron 1 sibling, 0 replies; 11+ messages in thread From: Jonathan Cameron @ 2019-06-08 13:08 UTC (permalink / raw) To: Guillaume La Roque Cc: devicetree, linux-iio, khilman, linux-kernel, linux-amlogic, linux-arm-kernel On Tue, 4 Jun 2019 16:47:12 +0200 Guillaume La Roque <glaroque@baylibre.com> wrote: > This adds the devicetree binding documentation for the Temperature > Sensor found in the Amlogic Meson G12 SoCs. > Currently only the G12A SoCs are supported. > > Signed-off-by: Guillaume La Roque <glaroque@baylibre.com> Hi Guillaume, I'm afraid we decided a month or so back that all new dt bindings proposed for IIO drivers should be in yaml format. Please reformat this appropriately for v2 and make sure to run make dt_bindings_check. There are a few examples in tree already, but we are deliberately not converting existing bindings too quickly to avoid overloading reviewers. Thanks, Jonathan > --- > .../iio/temperature/amlogic,meson-tsensor.txt | 31 +++++++++++++++++++ > 1 file changed, 31 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/temperature/amlogic,meson-tsensor.txt > > diff --git a/Documentation/devicetree/bindings/iio/temperature/amlogic,meson-tsensor.txt b/Documentation/devicetree/bindings/iio/temperature/amlogic,meson-tsensor.txt > new file mode 100644 > index 000000000000..d064db0e9cac > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/temperature/amlogic,meson-tsensor.txt > @@ -0,0 +1,31 @@ > +* Amlogic Meson Temperature Sensor > + > +Required properties: > +- compatible: depending on the SoC and the position of the sensor, > + this should be one of: > + - "amlogic,meson-g12a-cpu-tsensor" for the CPU G12A SoC sensor > + - "amlogic,meson-g12a-ddr-tsensor" for the DDR G12A SoC sensor > + followed by the common : > + - "amlogic,meson-g12a-tsensor" for G12A SoC family > +- reg: the physical base address and length of the registers > +- interrupts: the interrupt indicating end of sampling > +- clocks: phandle identifier for the reference clock of temperature sensor > +- #io-channel-cells: must be 1, see ../iio-bindings.txt > +- amlogic,ao-secure: phandle to the ao-secure syscon > + > +Optional properties: > +- amlogic,critical-temperature: temperature value in milli degrees Celsius > + to set automatic reboot on too high temperature > + > +Example: > + cpu_temp: temperature-sensor@ff634800 { > + compatible = "amlogic,meson-g12a-cpu-tsensor", > + "amlogic,meson-g12a-tsensor"; > + reg = <0x0 0xff634800 0x0 0x50>; > + interrupts = <GIC_SPI 35 IRQ_TYPE_EDGE_RISING>; > + clocks = <&clkc CLKID_TS>; > + status = "okay"; > + #io-channel-cells = <1>; > + amlogic,meson-ao-secure = <&sec_AO>; > + amlogic,critical-temperature = <115000>; > + }; _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/3] arm64: dts: meson: g12a: add temperature sensor node 2019-06-04 14:47 [PATCH 0/3] Add support of New Amlogic temperature sensor for G12A SoCs Guillaume La Roque 2019-06-04 14:47 ` [PATCH 1/3] Documentation: dt-bindings: add the Amlogic Meson Temperature Sensor Guillaume La Roque @ 2019-06-04 14:47 ` Guillaume La Roque 2019-06-04 14:47 ` [PATCH 3/3] iio: temperature: add a driver for the temperature sensor found in Amlogic Meson G12 SoCs Guillaume La Roque 2 siblings, 0 replies; 11+ messages in thread From: Guillaume La Roque @ 2019-06-04 14:47 UTC (permalink / raw) To: jic23, khilman Cc: linux-iio, devicetree, linux-kernel, linux-arm-kernel, linux-amlogic Add two temperature sensor the first near CPU and GPU, second near DDR Signed-off-by: Guillaume La Roque <glaroque@baylibre.com> --- arch/arm64/boot/dts/amlogic/meson-g12a.dtsi | 22 +++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi index 840dab606110..37f17087bdb1 100644 --- a/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi +++ b/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi @@ -1360,6 +1360,28 @@ }; }; + cpu_temp: temperature-sensor@34800 { + compatible = "amlogic,meson-g12a-cpu-tsensor", + "amlogic,meson-g12a-tsensor"; + reg = <0x0 0x34800 0x0 0x50>; + interrupts = <GIC_SPI 35 IRQ_TYPE_EDGE_RISING>; + clocks = <&clkc CLKID_TS>; + status = "okay"; + #io-channel-cells = <1>; + amlogic,ao-secure = <&sec_AO>; + }; + + ddr_temp: temperature-sensor@34c00 { + compatible = "amlogic,meson-g12a-ddr-tsensor", + "amlogic,meson-g12a-tsensor"; + reg = <0x0 0x34c00 0x0 0x50>; + interrupts = <GIC_SPI 36 IRQ_TYPE_EDGE_RISING>; + clocks = <&clkc CLKID_TS>; + status = "okay"; + #io-channel-cells = <1>; + amlogic,ao-secure = <&sec_AO>; + }; + usb2_phy0: phy@36000 { compatible = "amlogic,g12a-usb2-phy"; reg = <0x0 0x36000 0x0 0x2000>; -- 2.17.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/3] iio: temperature: add a driver for the temperature sensor found in Amlogic Meson G12 SoCs 2019-06-04 14:47 [PATCH 0/3] Add support of New Amlogic temperature sensor for G12A SoCs Guillaume La Roque 2019-06-04 14:47 ` [PATCH 1/3] Documentation: dt-bindings: add the Amlogic Meson Temperature Sensor Guillaume La Roque 2019-06-04 14:47 ` [PATCH 2/3] arm64: dts: meson: g12a: add temperature sensor node Guillaume La Roque @ 2019-06-04 14:47 ` Guillaume La Roque 2019-06-06 19:40 ` Martin Blumenstingl 2019-06-08 13:29 ` Jonathan Cameron 2 siblings, 2 replies; 11+ messages in thread From: Guillaume La Roque @ 2019-06-04 14:47 UTC (permalink / raw) To: jic23, khilman Cc: linux-iio, devicetree, linux-kernel, linux-arm-kernel, linux-amlogic The code is based on Amlogic source code. No public datasheet for this. Currently the G12A SoCs are supported. Supported features: - possibility to set an automatic reboot temperature Signed-off-by: Guillaume La Roque <glaroque@baylibre.com> --- drivers/iio/temperature/Kconfig | 11 + drivers/iio/temperature/Makefile | 1 + drivers/iio/temperature/meson_tsensor.c | 416 ++++++++++++++++++++++++ 3 files changed, 428 insertions(+) create mode 100644 drivers/iio/temperature/meson_tsensor.c diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig index 737faa0901fe..712a0062790d 100644 --- a/drivers/iio/temperature/Kconfig +++ b/drivers/iio/temperature/Kconfig @@ -34,6 +34,17 @@ config HID_SENSOR_TEMP To compile this driver as a module, choose M here: the module will be called hid-sensor-temperature. +config MESON_TSENSOR + tristate "Amlogic Meson temperature sensor Support" + default ARCH_MESON + depends on OF && ARCH_MESON + help + If you say yess here you get support for Meson Temperature sensor + for G12 SoC Family. + + This driver can also be built as a module. If so, the module will + be called meson_tsensor. + config MLX90614 tristate "MLX90614 contact-less infrared sensor" depends on I2C diff --git a/drivers/iio/temperature/Makefile b/drivers/iio/temperature/Makefile index baca4776ca0d..466d8c1c91d6 100644 --- a/drivers/iio/temperature/Makefile +++ b/drivers/iio/temperature/Makefile @@ -6,6 +6,7 @@ obj-$(CONFIG_HID_SENSOR_TEMP) += hid-sensor-temperature.o obj-$(CONFIG_MAXIM_THERMOCOUPLE) += maxim_thermocouple.o obj-$(CONFIG_MAX31856) += max31856.o +obj-$(CONFIG_MESON_TSENSOR) += meson_tsensor.o obj-$(CONFIG_MLX90614) += mlx90614.o obj-$(CONFIG_MLX90632) += mlx90632.o obj-$(CONFIG_TMP006) += tmp006.o diff --git a/drivers/iio/temperature/meson_tsensor.c b/drivers/iio/temperature/meson_tsensor.c new file mode 100644 index 000000000000..be0a8d073ba3 --- /dev/null +++ b/drivers/iio/temperature/meson_tsensor.c @@ -0,0 +1,416 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Amlogic Meson Temperature Sensor + * + * Copyright (C) 2017 Huan Biao <huan.biao@amlogic.com> + * Copyright (C) 2019 Guillaume La Roque <glaroque@baylibre.com> + * + * Register value to celsius temperature formulas: + * Read_Val m * U + * U = ---------, Uptat = --------- + * 2^16 1 + n * U + * + * Temperature = A * ( Uptat + u_efuse / 2^16 )- B + * + * A B m n : calibration parameters + * u_efuse : fused calibration value, it's a signed 16 bits value + */ + +#include <linux/bitfield.h> +#include <linux/clk.h> +#include <linux/iio/iio.h> +#include <linux/io.h> +#include <linux/mfd/syscon.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_address.h> +#include <linux/of_device.h> +#include <linux/platform_device.h> +#include <linux/regmap.h> + +#define TSENSOR_CFG_REG1 0x4 + #define TSENSOR_CFG_REG1_RSET_VBG BIT(12) + #define TSENSOR_CFG_REG1_RSET_ADC BIT(11) + #define TSENSOR_CFG_REG1_VCM_EN BIT(10) + #define TSENSOR_CFG_REG1_VBG_EN BIT(9) + #define TSENSOR_CFG_REG1_OUT_CTL BIT(6) + #define TSENSOR_CFG_REG1_FILTER_EN BIT(5) + #define TSENSOR_CFG_REG1_DEM_EN BIT(3) + #define TSENSOR_CFG_REG1_CH_SEL GENMASK(1, 0) + #define TSENSOR_CFG_REG1_ENABLE \ + (TSENSOR_CFG_REG1_FILTER_EN | \ + TSENSOR_CFG_REG1_VCM_EN | \ + TSENSOR_CFG_REG1_VBG_EN | \ + TSENSOR_CFG_REG1_DEM_EN | \ + TSENSOR_CFG_REG1_CH_SEL) + +#define TSENSOR_CFG_REG2 0x8 + #define TSENSOR_CFG_REG2_HITEMP_EN BIT(31) + #define TSENSOR_CFG_REG2_REBOOT_ALL_EN BIT(30) + #define TSENSOR_CFG_REG2_REBOOT_TIME GENMASK(25, 16) + #define TSENSOR_CFG_REG2_HITEMP_REBOOT_ENABLE \ + (TSENSOR_CFG_REG2_HITEMP_EN | \ + TSENSOR_CFG_REG2_REBOOT_ALL_EN | \ + TSENSOR_CFG_REG2_REBOOT_TIME) + #define TSENSOR_CFG_REG2_HITEMP_REBOOT_ENABLE_MASK \ + (GENMASK(31, 30) | GENMASK(25, 4)) + #define TSENSOR_CFG_REG2_HITEMP_REBOOT_REG_MASK \ + GENMASK(15, 4) + #define TSENSOR_CFG_REG2_HITEMP_REG_VAL(_reg_val) \ + (FIELD_PREP(TSENSOR_CFG_REG2_HITEMP_REBOOT_REG_MASK, \ + _reg_val) | \ + TSENSOR_CFG_REG2_HITEMP_REBOOT_ENABLE) + +#define TSENSOR_CFG_REG3 0xC +#define TSENSOR_CFG_REG4 0x10 +#define TSENSOR_CFG_REG5 0x14 +#define TSENSOR_CFG_REG6 0x18 +#define TSENSOR_CFG_REG7 0x1C +#define TSENSOR_CFG_REG8 0x20 + +#define TSENSOR_STAT0 0x40 + +#define TSENSOR_STAT9 0x64 + +#define TSENSOR_READ_TEMP_MASK GENMASK(15, 0) +#define TSENSOR_TEMP_MASK GENMASK(11, 0) + +#define TSENSOR_TRIM_SIGN_MASK BIT(15) +#define TSENSOR_TRIM_TEMP_MASK GENMASK(14, 0) +#define TSENSOR_TRIM_VERSION_MASK GENMASK(31, 24) + +#define TSENSOR_TRIM_VERSION(_version) \ + FIELD_GET(TSENSOR_TRIM_VERSION_MASK, _version) + +#define TSENSOR_TRIM_CALIB_VALID_MASK (GENMASK(3, 2) | BIT(7)) + +#define TSENSOR_CALIB_OFFSET 1 +#define TSENSOR_CALIB_SHIFT 4 + +static const struct iio_chan_spec temperature_channel[] = { + { + .type = IIO_TEMP, + .channel = 0, + .address = 0, + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), + }, +}; + +/** + * struct meson_tsensor_soc_data + * @A, B, m, n: calibration parameters + * This structure is required for configuration of meson tsensor driver. + */ +struct meson_tsensor_soc_data { + int A; + int B; + int m; + int n; +}; + +/** + * struct meson_tsensor_data + * @u_efuse_off: register offset to read fused calibration value + * @soc: calibration parameters structure pointer + * @regmap_config: regmap config for the device + * This structure is required for configuration of meson tsensor driver. + */ +struct meson_tsensor_data { + int u_efuse_off; + const struct meson_tsensor_soc_data *soc; + const struct regmap_config *regmap_config; +}; + +struct meson_tsensor { + int id; + const struct meson_tsensor_data *data; + struct regmap *regmap; + struct regmap *sec_ao_map; + struct clk *clk; + u32 trim_info; + void __iomem *base; + int reboot_temp; +}; + +/* + * tsensor treats temperature as a mapped temperature code. + * The temperature is converted differently depending on the calibration type. + */ +static u32 temp_to_code(struct meson_tsensor *priv, int temp) +{ + const struct meson_tsensor_soc_data *param = priv->data->soc; + s64 divisor, factor, uefuse; + u32 reg_code; + + uefuse = priv->trim_info & TSENSOR_TRIM_SIGN_MASK ? + ~(priv->trim_info & TSENSOR_TRIM_TEMP_MASK) + 1 : + (priv->trim_info & TSENSOR_TRIM_TEMP_MASK); + + factor = BIT(16) * (temp * 10 + param->B); + factor = div_s64(factor, param->A); + factor = factor + uefuse; + + factor = factor * 100; + + divisor = param->n * factor; + divisor = div_s64(divisor, BIT(16)); + divisor = param->m - divisor; + + reg_code = div_s64(factor, divisor); + reg_code = ((reg_code >> TSENSOR_CALIB_SHIFT) & TSENSOR_TEMP_MASK) + + TSENSOR_CALIB_OFFSET; + + return reg_code; +} + +/* + * Calculate a temperature value from a temperature code. + * The unit of the temperature is degree Celsius. + */ +static int code_to_temp(struct meson_tsensor *priv, int temp_code) +{ + const struct meson_tsensor_soc_data *param = priv->data->soc; + int temp; + s64 factor, Uptat, uefuse; + + uefuse = priv->trim_info & TSENSOR_TRIM_SIGN_MASK ? + ~(priv->trim_info & TSENSOR_TRIM_TEMP_MASK) + 1 : + (priv->trim_info & TSENSOR_TRIM_TEMP_MASK); + + factor = param->n * temp_code; + factor = div_s64(factor, 100); + + Uptat = temp_code * param->m; + Uptat = div_s64(Uptat, 100); + Uptat = Uptat * BIT(16); + Uptat = div_s64(Uptat, BIT(16) + factor); + + temp = (Uptat + uefuse) * param->A; + temp = div_s64(temp, BIT(16)); + temp = (temp - param->B) * 100; + + return temp; +} + +static int meson_tsensor_initialize(struct iio_dev *indio_dev) +{ + struct meson_tsensor *priv = iio_priv(indio_dev); + u32 reg_val; + int ret = 0; + int ver; + + regmap_read(priv->sec_ao_map, priv->data->u_efuse_off, + &priv->trim_info); + + ver = TSENSOR_TRIM_VERSION(priv->trim_info); + + if ((ver & TSENSOR_TRIM_CALIB_VALID_MASK) == 0) { + ret = -EINVAL; + dev_err(&indio_dev->dev, + "tsensor thermal calibration not supported: 0x%x!\n", + ver); + goto out; + } + + /* init the ts reboot soc function */ + if (priv->reboot_temp) { + /* register need value in celsius */ + reg_val = temp_to_code(priv, priv->reboot_temp / 1000); + regmap_update_bits(priv->regmap, TSENSOR_CFG_REG2, + TSENSOR_CFG_REG2_HITEMP_REBOOT_ENABLE_MASK, + TSENSOR_CFG_REG2_HITEMP_REG_VAL(reg_val)); + } + +out: + return ret; +} + +static int meson_tsensor_enable(struct iio_dev *indio_dev) +{ + struct meson_tsensor *priv = iio_priv(indio_dev); + + clk_prepare_enable(priv->clk); + regmap_update_bits(priv->regmap, TSENSOR_CFG_REG1, + TSENSOR_CFG_REG1_ENABLE, TSENSOR_CFG_REG1_ENABLE); + + return 0; +} + +static int meson_tsensor_disable(struct iio_dev *indio_dev) +{ + struct meson_tsensor *priv = iio_priv(indio_dev); + + regmap_update_bits(priv->regmap, TSENSOR_CFG_REG1, + TSENSOR_CFG_REG1_ENABLE, 0); + clk_disable(priv->clk); + + return 0; +} + +static int meson_tsensor_read(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, int *val, + int *val2, long mask) +{ + unsigned int tvalue; + struct meson_tsensor *priv = iio_priv(indio_dev); + + switch (mask) { + case IIO_CHAN_INFO_PROCESSED: + regmap_read(priv->regmap, TSENSOR_STAT0, &tvalue); + *val = code_to_temp(priv, + tvalue & TSENSOR_READ_TEMP_MASK); + + return IIO_VAL_INT; + default: + return -EINVAL; + } +} + +static const struct iio_info meson_tsensor_iio_info = { + .read_raw = &meson_tsensor_read, +}; + +static const struct regmap_config meson_tsensor_regmap_config_g12a = { + .reg_bits = 8, + .val_bits = 32, + .reg_stride = 4, + .max_register = TSENSOR_STAT9, +}; + +static const struct meson_tsensor_soc_data meson_tsensor_g12a = { + .A = 9411, + .B = 3159, + .m = 424, + .n = 324, +}; + +static const struct meson_tsensor_data meson_tsensor_g12a_cpu_param = { + .u_efuse_off = 0x128, + .soc = &meson_tsensor_g12a, + .regmap_config = &meson_tsensor_regmap_config_g12a, +}; + +static const struct meson_tsensor_data meson_tsensor_g12a_ddr_param = { + .u_efuse_off = 0xF0, + .soc = &meson_tsensor_g12a, + .regmap_config = &meson_tsensor_regmap_config_g12a, +}; + +static const struct of_device_id meson_tsensor_of_match[] = { + { + .compatible = "amlogic,meson-g12a-ddr-tsensor", + .data = &meson_tsensor_g12a_ddr_param, + }, + { + .compatible = "amlogic,meson-g12a-cpu-tsensor", + .data = &meson_tsensor_g12a_cpu_param, + }, + {}, +}; +MODULE_DEVICE_TABLE(of, meson_tsensor_of_match); + +static int meson_tsensor_probe(struct platform_device *pdev) +{ + struct meson_tsensor *priv; + struct iio_dev *indio_dev; + struct resource *res; + + int ret; + + indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*priv)); + if (!indio_dev) { + dev_err(&pdev->dev, "failed allocating iio device\n"); + return -ENOMEM; + } + + priv = iio_priv(indio_dev); + priv->data = of_device_get_match_data(&pdev->dev); + if (!priv->data) { + dev_err(&pdev->dev, "failed to get match data\n"); + return -ENODEV; + } + + indio_dev->channels = temperature_channel; + indio_dev->num_channels = ARRAY_SIZE(temperature_channel); + indio_dev->name = dev_name(&pdev->dev); + indio_dev->dev.parent = &pdev->dev; + indio_dev->dev.of_node = pdev->dev.of_node; + indio_dev->modes = INDIO_DIRECT_MODE; + indio_dev->info = &meson_tsensor_iio_info; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + priv->base = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(priv->base)) + return PTR_ERR(priv->base); + + priv->regmap = devm_regmap_init_mmio(&pdev->dev, priv->base, + priv->data->regmap_config); + if (IS_ERR(priv->regmap)) + return PTR_ERR(priv->regmap); + + priv->clk = devm_clk_get(&pdev->dev, NULL); + if (IS_ERR(priv->clk)) { + if (PTR_ERR(priv->clk) != -EPROBE_DEFER) + dev_err(&pdev->dev, "failed to get clock\n"); + return PTR_ERR(priv->clk); + } + + if (of_property_read_u32(pdev->dev.of_node, + "amlogic,critical-temperature", + &priv->reboot_temp)) { + priv->reboot_temp = 0; + } + + priv->sec_ao_map = syscon_regmap_lookup_by_phandle + (pdev->dev.of_node, "amlogic,ao-secure"); + if (IS_ERR(priv->sec_ao_map)) { + dev_err(&pdev->dev, "syscon regmap lookup failed.\n"); + return PTR_ERR(priv->sec_ao_map); + } + + ret = meson_tsensor_initialize(indio_dev); + if (ret) + return ret; + + ret = meson_tsensor_enable(indio_dev); + if (ret) + goto err; + + platform_set_drvdata(pdev, indio_dev); + ret = iio_device_register(indio_dev); + if (ret) + goto err_hw; + + return 0; + +err_hw: + meson_tsensor_disable(indio_dev); +err: + clk_unprepare(priv->clk); + + return ret; +} + +static int meson_tsensor_remove(struct platform_device *pdev) +{ + struct iio_dev *indio_dev = platform_get_drvdata(pdev); + + iio_device_unregister(indio_dev); + + return meson_tsensor_disable(indio_dev); +} + +static struct platform_driver meson_tsensor_driver = { + .probe = meson_tsensor_probe, + .remove = meson_tsensor_remove, + .driver = { + .name = "meson-tsensor", + .of_match_table = meson_tsensor_of_match, + }, +}; + +module_platform_driver(meson_tsensor_driver); + +MODULE_AUTHOR("Guillaume La Roque <glaroque@baylibre.com>"); +MODULE_DESCRIPTION("Amlogic Meson Temperature Sensor Driver"); +MODULE_LICENSE("GPL v2"); -- 2.17.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] iio: temperature: add a driver for the temperature sensor found in Amlogic Meson G12 SoCs 2019-06-04 14:47 ` [PATCH 3/3] iio: temperature: add a driver for the temperature sensor found in Amlogic Meson G12 SoCs Guillaume La Roque @ 2019-06-06 19:40 ` Martin Blumenstingl 2019-06-08 13:29 ` Jonathan Cameron 1 sibling, 0 replies; 11+ messages in thread From: Martin Blumenstingl @ 2019-06-06 19:40 UTC (permalink / raw) To: Guillaume La Roque Cc: devicetree, linux-iio, khilman, linux-kernel, jic23, linux-amlogic, linux-arm-kernel Hi Guillaume, below are my initial impressions. I will have a closer look once there's a decision whether this belongs to the IIO or thermal framework. On Tue, Jun 4, 2019 at 4:48 PM Guillaume La Roque <glaroque@baylibre.com> wrote: > > The code is based on Amlogic source code. No public datasheet for this. the public S922X datasheet from Hardkernel [0] has some documentation (starting at page 1106). [...] > +config MESON_TSENSOR > + tristate "Amlogic Meson temperature sensor Support" > + default ARCH_MESON > + depends on OF && ARCH_MESON depends on OF && (ARCH_MESON || COMPILE_TEST) so all the nice auto-builders / testing tools will speak up if someone tries to break your driver > + help > + If you say yess here you get support for Meson Temperature sensor s/yess/yes/ > + for G12 SoC Family. G12 (which I assume includes G12A and G12B) or G12A? [...] > diff --git a/drivers/iio/temperature/meson_tsensor.c b/drivers/iio/temperature/meson_tsensor.c > new file mode 100644 > index 000000000000..be0a8d073ba3 > --- /dev/null > +++ b/drivers/iio/temperature/meson_tsensor.c > @@ -0,0 +1,416 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Amlogic Meson Temperature Sensor > + * > + * Copyright (C) 2017 Huan Biao <huan.biao@amlogic.com> > + * Copyright (C) 2019 Guillaume La Roque <glaroque@baylibre.com> > + * > + * Register value to celsius temperature formulas: > + * Read_Val m * U > + * U = ---------, Uptat = --------- > + * 2^16 1 + n * U > + * > + * Temperature = A * ( Uptat + u_efuse / 2^16 )- B > + * > + * A B m n : calibration parameters > + * u_efuse : fused calibration value, it's a signed 16 bits value it's great to have this explained in the docs (instead of having to look it up in some out of tree driver, as it's not part of the datasheet) - thank you for this! > + */ > + > +#include <linux/bitfield.h> > +#include <linux/clk.h> > +#include <linux/iio/iio.h> > +#include <linux/io.h> > +#include <linux/mfd/syscon.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/of_address.h> > +#include <linux/of_device.h> > +#include <linux/platform_device.h> > +#include <linux/regmap.h> > + > +#define TSENSOR_CFG_REG1 0x4 > + #define TSENSOR_CFG_REG1_RSET_VBG BIT(12) > + #define TSENSOR_CFG_REG1_RSET_ADC BIT(11) > + #define TSENSOR_CFG_REG1_VCM_EN BIT(10) > + #define TSENSOR_CFG_REG1_VBG_EN BIT(9) > + #define TSENSOR_CFG_REG1_OUT_CTL BIT(6) > + #define TSENSOR_CFG_REG1_FILTER_EN BIT(5) > + #define TSENSOR_CFG_REG1_DEM_EN BIT(3) > + #define TSENSOR_CFG_REG1_CH_SEL GENMASK(1, 0) > + #define TSENSOR_CFG_REG1_ENABLE \ > + (TSENSOR_CFG_REG1_FILTER_EN | \ > + TSENSOR_CFG_REG1_VCM_EN | \ > + TSENSOR_CFG_REG1_VBG_EN | \ > + TSENSOR_CFG_REG1_DEM_EN | \ > + TSENSOR_CFG_REG1_CH_SEL) are all of these needed to enabled *and* disable the temperature sensor? TSENSOR_CFG_REG1_CH_SEL doesn't seem related when disabling the sensor reading (but I don't know since there's no documentation) > +#define TSENSOR_CFG_REG2 0x8 > + #define TSENSOR_CFG_REG2_HITEMP_EN BIT(31) > + #define TSENSOR_CFG_REG2_REBOOT_ALL_EN BIT(30) > + #define TSENSOR_CFG_REG2_REBOOT_TIME GENMASK(25, 16) > + #define TSENSOR_CFG_REG2_HITEMP_REBOOT_ENABLE \ > + (TSENSOR_CFG_REG2_HITEMP_EN | \ > + TSENSOR_CFG_REG2_REBOOT_ALL_EN | \ > + TSENSOR_CFG_REG2_REBOOT_TIME) the name mix between TSENSOR_CFG_REG2_HITEMP_REBOOT_ENABLE and TSENSOR_CFG_REG2_HITEMP_REBOOT_ENABLE_MASK confused me. personally I would drop the macros which just bit-wise or multiple other macros together > + #define TSENSOR_CFG_REG2_HITEMP_REBOOT_ENABLE_MASK \ > + (GENMASK(31, 30) | GENMASK(25, 4)) > + #define TSENSOR_CFG_REG2_HITEMP_REBOOT_REG_MASK \ > + GENMASK(15, 4) > + #define TSENSOR_CFG_REG2_HITEMP_REG_VAL(_reg_val) \ > + (FIELD_PREP(TSENSOR_CFG_REG2_HITEMP_REBOOT_REG_MASK, \ > + _reg_val) | \ > + TSENSOR_CFG_REG2_HITEMP_REBOOT_ENABLE) > + > +#define TSENSOR_CFG_REG3 0xC I like to use lower-case hex letters and I also pad them -> 0x0c in this case because we have for example 0x10 below) > +#define TSENSOR_CFG_REG4 0x10 > +#define TSENSOR_CFG_REG5 0x14 > +#define TSENSOR_CFG_REG6 0x18 > +#define TSENSOR_CFG_REG7 0x1C > +#define TSENSOR_CFG_REG8 0x20 > + > +#define TSENSOR_STAT0 0x40 > + > +#define TSENSOR_STAT9 0x64 > + > +#define TSENSOR_READ_TEMP_MASK GENMASK(15, 0) TSENSOR_STAT0_FILTER_OUT would match the naming from the datasheet > +#define TSENSOR_TEMP_MASK GENMASK(11, 0) > > +#define TSENSOR_TRIM_SIGN_MASK BIT(15) > +#define TSENSOR_TRIM_TEMP_MASK GENMASK(14, 0) > +#define TSENSOR_TRIM_VERSION_MASK GENMASK(31, 24) > + > +#define TSENSOR_TRIM_VERSION(_version) \ > + FIELD_GET(TSENSOR_TRIM_VERSION_MASK, _version) I would drop this and use the FIELD_GET directly where needed (it's only one occurrence anyways) [...] > +static int meson_tsensor_enable(struct iio_dev *indio_dev) > +{ > + struct meson_tsensor *priv = iio_priv(indio_dev); > + > + clk_prepare_enable(priv->clk); may return an error which you're discarding here > + regmap_update_bits(priv->regmap, TSENSOR_CFG_REG1, > + TSENSOR_CFG_REG1_ENABLE, TSENSOR_CFG_REG1_ENABLE); > + > + return 0; > +} > + > +static int meson_tsensor_disable(struct iio_dev *indio_dev) > +{ > + struct meson_tsensor *priv = iio_priv(indio_dev); > + > + regmap_update_bits(priv->regmap, TSENSOR_CFG_REG1, > + TSENSOR_CFG_REG1_ENABLE, 0); > + clk_disable(priv->clk); clk_disable_unprepare as you're calling clk_prepare_enable above? > + > + return 0; make it a void function instead? > +static const struct regmap_config meson_tsensor_regmap_config_g12a = { > + .reg_bits = 8, > + .val_bits = 32, > + .reg_stride = 4, > + .max_register = TSENSOR_STAT9, .fast_io = true if you ever need to ACK interrupts from the IRQ handler (IIRC fast_io will use a spinlock instead of mutex) [...] > +static const struct of_device_id meson_tsensor_of_match[] = { > + { > + .compatible = "amlogic,meson-g12a-ddr-tsensor", > + .data = &meson_tsensor_g12a_ddr_param, > + }, > + { > + .compatible = "amlogic,meson-g12a-cpu-tsensor", > + .data = &meson_tsensor_g12a_cpu_param, > + }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, meson_tsensor_of_match); I would move the of_device_id table above the platform_driver definition below of_device_get_match_data doesn't need the of_device_id as parameter (compared to it's predecessor) > +static int meson_tsensor_probe(struct platform_device *pdev) > +{ > + struct meson_tsensor *priv; > + struct iio_dev *indio_dev; > + struct resource *res; > + > + int ret; > + > + indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*priv)); > + if (!indio_dev) { > + dev_err(&pdev->dev, "failed allocating iio device\n"); > + return -ENOMEM; > + } > + > + priv = iio_priv(indio_dev); > + priv->data = of_device_get_match_data(&pdev->dev); > + if (!priv->data) { > + dev_err(&pdev->dev, "failed to get match data\n"); > + return -ENODEV; > + } > + > + indio_dev->channels = temperature_channel; > + indio_dev->num_channels = ARRAY_SIZE(temperature_channel); > + indio_dev->name = dev_name(&pdev->dev); > + indio_dev->dev.parent = &pdev->dev; > + indio_dev->dev.of_node = pdev->dev.of_node; > + indio_dev->modes = INDIO_DIRECT_MODE; > + indio_dev->info = &meson_tsensor_iio_info; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + priv->base = devm_ioremap_resource(&pdev->dev, res); you're only using priv->base in this function. consider dropping it from struct meson_tsensor [0] https://dn.odroid.com/S922X/ODROID-N2/Datasheet/S922X_Public_Datasheet_V0.2.pdf _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] iio: temperature: add a driver for the temperature sensor found in Amlogic Meson G12 SoCs 2019-06-04 14:47 ` [PATCH 3/3] iio: temperature: add a driver for the temperature sensor found in Amlogic Meson G12 SoCs Guillaume La Roque 2019-06-06 19:40 ` Martin Blumenstingl @ 2019-06-08 13:29 ` Jonathan Cameron 1 sibling, 0 replies; 11+ messages in thread From: Jonathan Cameron @ 2019-06-08 13:29 UTC (permalink / raw) To: Guillaume La Roque Cc: devicetree, linux-iio, khilman, linux-kernel, linux-amlogic, linux-arm-kernel On Tue, 4 Jun 2019 16:47:14 +0200 Guillaume La Roque <glaroque@baylibre.com> wrote: > The code is based on Amlogic source code. No public datasheet for this. > Currently the G12A SoCs are supported. > > Supported features: > - possibility to set an automatic reboot temperature > > Signed-off-by: Guillaume La Roque <glaroque@baylibre.com> A few comments inline. However, I'm still of the view this is probably better as a thermal driver than an IIO one. Jonathan > --- > drivers/iio/temperature/Kconfig | 11 + > drivers/iio/temperature/Makefile | 1 + > drivers/iio/temperature/meson_tsensor.c | 416 ++++++++++++++++++++++++ > 3 files changed, 428 insertions(+) > create mode 100644 drivers/iio/temperature/meson_tsensor.c > > diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig > index 737faa0901fe..712a0062790d 100644 > --- a/drivers/iio/temperature/Kconfig > +++ b/drivers/iio/temperature/Kconfig > @@ -34,6 +34,17 @@ config HID_SENSOR_TEMP > To compile this driver as a module, choose M here: the module > will be called hid-sensor-temperature. > > +config MESON_TSENSOR > + tristate "Amlogic Meson temperature sensor Support" > + default ARCH_MESON > + depends on OF && ARCH_MESON > + help > + If you say yess here you get support for Meson Temperature sensor > + for G12 SoC Family. > + > + This driver can also be built as a module. If so, the module will > + be called meson_tsensor. > + > config MLX90614 > tristate "MLX90614 contact-less infrared sensor" > depends on I2C > diff --git a/drivers/iio/temperature/Makefile b/drivers/iio/temperature/Makefile > index baca4776ca0d..466d8c1c91d6 100644 > --- a/drivers/iio/temperature/Makefile > +++ b/drivers/iio/temperature/Makefile > @@ -6,6 +6,7 @@ > obj-$(CONFIG_HID_SENSOR_TEMP) += hid-sensor-temperature.o > obj-$(CONFIG_MAXIM_THERMOCOUPLE) += maxim_thermocouple.o > obj-$(CONFIG_MAX31856) += max31856.o > +obj-$(CONFIG_MESON_TSENSOR) += meson_tsensor.o > obj-$(CONFIG_MLX90614) += mlx90614.o > obj-$(CONFIG_MLX90632) += mlx90632.o > obj-$(CONFIG_TMP006) += tmp006.o > diff --git a/drivers/iio/temperature/meson_tsensor.c b/drivers/iio/temperature/meson_tsensor.c > new file mode 100644 > index 000000000000..be0a8d073ba3 > --- /dev/null > +++ b/drivers/iio/temperature/meson_tsensor.c > @@ -0,0 +1,416 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Amlogic Meson Temperature Sensor > + * > + * Copyright (C) 2017 Huan Biao <huan.biao@amlogic.com> > + * Copyright (C) 2019 Guillaume La Roque <glaroque@baylibre.com> > + * > + * Register value to celsius temperature formulas: > + * Read_Val m * U > + * U = ---------, Uptat = --------- > + * 2^16 1 + n * U > + * > + * Temperature = A * ( Uptat + u_efuse / 2^16 )- B > + * > + * A B m n : calibration parameters > + * u_efuse : fused calibration value, it's a signed 16 bits value > + */ > + > +#include <linux/bitfield.h> > +#include <linux/clk.h> > +#include <linux/iio/iio.h> > +#include <linux/io.h> > +#include <linux/mfd/syscon.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/of_address.h> > +#include <linux/of_device.h> > +#include <linux/platform_device.h> > +#include <linux/regmap.h> > + > +#define TSENSOR_CFG_REG1 0x4 > + #define TSENSOR_CFG_REG1_RSET_VBG BIT(12) > + #define TSENSOR_CFG_REG1_RSET_ADC BIT(11) > + #define TSENSOR_CFG_REG1_VCM_EN BIT(10) > + #define TSENSOR_CFG_REG1_VBG_EN BIT(9) > + #define TSENSOR_CFG_REG1_OUT_CTL BIT(6) > + #define TSENSOR_CFG_REG1_FILTER_EN BIT(5) > + #define TSENSOR_CFG_REG1_DEM_EN BIT(3) > + #define TSENSOR_CFG_REG1_CH_SEL GENMASK(1, 0) > + #define TSENSOR_CFG_REG1_ENABLE \ > + (TSENSOR_CFG_REG1_FILTER_EN | \ > + TSENSOR_CFG_REG1_VCM_EN | \ > + TSENSOR_CFG_REG1_VBG_EN | \ > + TSENSOR_CFG_REG1_DEM_EN | \ > + TSENSOR_CFG_REG1_CH_SEL) > + > +#define TSENSOR_CFG_REG2 0x8 > + #define TSENSOR_CFG_REG2_HITEMP_EN BIT(31) > + #define TSENSOR_CFG_REG2_REBOOT_ALL_EN BIT(30) > + #define TSENSOR_CFG_REG2_REBOOT_TIME GENMASK(25, 16) > + #define TSENSOR_CFG_REG2_HITEMP_REBOOT_ENABLE \ > + (TSENSOR_CFG_REG2_HITEMP_EN | \ > + TSENSOR_CFG_REG2_REBOOT_ALL_EN | \ > + TSENSOR_CFG_REG2_REBOOT_TIME) > + #define TSENSOR_CFG_REG2_HITEMP_REBOOT_ENABLE_MASK \ > + (GENMASK(31, 30) | GENMASK(25, 4)) > + #define TSENSOR_CFG_REG2_HITEMP_REBOOT_REG_MASK \ > + GENMASK(15, 4) > + #define TSENSOR_CFG_REG2_HITEMP_REG_VAL(_reg_val) \ > + (FIELD_PREP(TSENSOR_CFG_REG2_HITEMP_REBOOT_REG_MASK, \ > + _reg_val) | \ > + TSENSOR_CFG_REG2_HITEMP_REBOOT_ENABLE) > + > +#define TSENSOR_CFG_REG3 0xC > +#define TSENSOR_CFG_REG4 0x10 > +#define TSENSOR_CFG_REG5 0x14 > +#define TSENSOR_CFG_REG6 0x18 > +#define TSENSOR_CFG_REG7 0x1C > +#define TSENSOR_CFG_REG8 0x20 > + > +#define TSENSOR_STAT0 0x40 > + > +#define TSENSOR_STAT9 0x64 > + > +#define TSENSOR_READ_TEMP_MASK GENMASK(15, 0) > +#define TSENSOR_TEMP_MASK GENMASK(11, 0) > + > +#define TSENSOR_TRIM_SIGN_MASK BIT(15) > +#define TSENSOR_TRIM_TEMP_MASK GENMASK(14, 0) > +#define TSENSOR_TRIM_VERSION_MASK GENMASK(31, 24) > + > +#define TSENSOR_TRIM_VERSION(_version) \ > + FIELD_GET(TSENSOR_TRIM_VERSION_MASK, _version) I'd just use the FIELD_GET directly inline. What it is doing is kind of obvious from the parameter. > + > +#define TSENSOR_TRIM_CALIB_VALID_MASK (GENMASK(3, 2) | BIT(7)) > + > +#define TSENSOR_CALIB_OFFSET 1 > +#define TSENSOR_CALIB_SHIFT 4 > + > +static const struct iio_chan_spec temperature_channel[] = { > + { > + .type = IIO_TEMP, > + .channel = 0, .channel doesn't do anything unless you set indexed. > + .address = 0, .address not used... > + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), > + }, > +}; > + > +/** > + * struct meson_tsensor_soc_data > + * @A, B, m, n: calibration parameters > + * This structure is required for configuration of meson tsensor driver. > + */ > +struct meson_tsensor_soc_data { > + int A; > + int B; > + int m; > + int n; > +}; > + > +/** > + * struct meson_tsensor_data > + * @u_efuse_off: register offset to read fused calibration value > + * @soc: calibration parameters structure pointer > + * @regmap_config: regmap config for the device > + * This structure is required for configuration of meson tsensor driver. > + */ > +struct meson_tsensor_data { > + int u_efuse_off; > + const struct meson_tsensor_soc_data *soc; > + const struct regmap_config *regmap_config; > +}; > + > +struct meson_tsensor { > + int id; > + const struct meson_tsensor_data *data; > + struct regmap *regmap; > + struct regmap *sec_ao_map; > + struct clk *clk; > + u32 trim_info; > + void __iomem *base; > + int reboot_temp; > +}; > + > +/* > + * tsensor treats temperature as a mapped temperature code. > + * The temperature is converted differently depending on the calibration type. > + */ > +static u32 temp_to_code(struct meson_tsensor *priv, int temp) > +{ > + const struct meson_tsensor_soc_data *param = priv->data->soc; > + s64 divisor, factor, uefuse; > + u32 reg_code; > + > + uefuse = priv->trim_info & TSENSOR_TRIM_SIGN_MASK ? > + ~(priv->trim_info & TSENSOR_TRIM_TEMP_MASK) + 1 : > + (priv->trim_info & TSENSOR_TRIM_TEMP_MASK); > + > + factor = BIT(16) * (temp * 10 + param->B); > + factor = div_s64(factor, param->A); > + factor = factor + uefuse; > + > + factor = factor * 100; > + > + divisor = param->n * factor; > + divisor = div_s64(divisor, BIT(16)); > + divisor = param->m - divisor; > + > + reg_code = div_s64(factor, divisor); > + reg_code = ((reg_code >> TSENSOR_CALIB_SHIFT) & TSENSOR_TEMP_MASK) + > + TSENSOR_CALIB_OFFSET; > + > + return reg_code; > +} > + > +/* > + * Calculate a temperature value from a temperature code. > + * The unit of the temperature is degree Celsius. > + */ > +static int code_to_temp(struct meson_tsensor *priv, int temp_code) > +{ > + const struct meson_tsensor_soc_data *param = priv->data->soc; > + int temp; > + s64 factor, Uptat, uefuse; > + > + uefuse = priv->trim_info & TSENSOR_TRIM_SIGN_MASK ? > + ~(priv->trim_info & TSENSOR_TRIM_TEMP_MASK) + 1 : > + (priv->trim_info & TSENSOR_TRIM_TEMP_MASK); Hmm. 2's complement from unsigned + a sign bit. uefuse = priv->trim_info & TSENSOR_TRIM_TEMP_MASK; if (priv->trim_info & TSENSOR_TRIM_SIGN_MASK) uefuse *= -1; Is probably going to do the same thing whilst being more readable. > + > + factor = param->n * temp_code; > + factor = div_s64(factor, 100); > + > + Uptat = temp_code * param->m; Why the capital U? Not really kernel style. > + Uptat = div_s64(Uptat, 100); > + Uptat = Uptat * BIT(16); > + Uptat = div_s64(Uptat, BIT(16) + factor); > + > + temp = (Uptat + uefuse) * param->A; > + temp = div_s64(temp, BIT(16)); > + temp = (temp - param->B) * 100; > + > + return temp; > +} > + > +static int meson_tsensor_initialize(struct iio_dev *indio_dev) > +{ > + struct meson_tsensor *priv = iio_priv(indio_dev); > + u32 reg_val; > + int ret = 0; > + int ver; > + > + regmap_read(priv->sec_ao_map, priv->data->u_efuse_off, > + &priv->trim_info); > + > + ver = TSENSOR_TRIM_VERSION(priv->trim_info); > + > + if ((ver & TSENSOR_TRIM_CALIB_VALID_MASK) == 0) { > + ret = -EINVAL; > + dev_err(&indio_dev->dev, > + "tsensor thermal calibration not supported: 0x%x!\n", > + ver); > + goto out; return -EINVAL; > + } > + > + /* init the ts reboot soc function */ > + if (priv->reboot_temp) { > + /* register need value in celsius */ > + reg_val = temp_to_code(priv, priv->reboot_temp / 1000); > + regmap_update_bits(priv->regmap, TSENSOR_CFG_REG2, > + TSENSOR_CFG_REG2_HITEMP_REBOOT_ENABLE_MASK, > + TSENSOR_CFG_REG2_HITEMP_REG_VAL(reg_val)); > + } > + return 0; > +out: > + return ret; > +} > + > +static int meson_tsensor_enable(struct iio_dev *indio_dev) > +{ > + struct meson_tsensor *priv = iio_priv(indio_dev); > + > + clk_prepare_enable(priv->clk); > + regmap_update_bits(priv->regmap, TSENSOR_CFG_REG1, > + TSENSOR_CFG_REG1_ENABLE, TSENSOR_CFG_REG1_ENABLE); > + > + return 0; Given neither this nor the follow up have any error paths, stop them having a return value and you can drop some error handling.. > +} > + > +static int meson_tsensor_disable(struct iio_dev *indio_dev) > +{ > + struct meson_tsensor *priv = iio_priv(indio_dev); > + > + regmap_update_bits(priv->regmap, TSENSOR_CFG_REG1, > + TSENSOR_CFG_REG1_ENABLE, 0); > + clk_disable(priv->clk); > + > + return 0; > +} > + > +static int meson_tsensor_read(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, int *val, > + int *val2, long mask) > +{ > + unsigned int tvalue; > + struct meson_tsensor *priv = iio_priv(indio_dev); > + > + switch (mask) { > + case IIO_CHAN_INFO_PROCESSED: > + regmap_read(priv->regmap, TSENSOR_STAT0, &tvalue); > + *val = code_to_temp(priv, > + tvalue & TSENSOR_READ_TEMP_MASK); Looks to me like this will fit on one line under 80 chars. > + > + return IIO_VAL_INT; > + default: > + return -EINVAL; > + } > +} > + > +static const struct iio_info meson_tsensor_iio_info = { > + .read_raw = &meson_tsensor_read, > +}; > + > +static const struct regmap_config meson_tsensor_regmap_config_g12a = { > + .reg_bits = 8, > + .val_bits = 32, > + .reg_stride = 4, > + .max_register = TSENSOR_STAT9, > +}; > + > +static const struct meson_tsensor_soc_data meson_tsensor_g12a = { > + .A = 9411, > + .B = 3159, > + .m = 424, > + .n = 324, > +}; > + > +static const struct meson_tsensor_data meson_tsensor_g12a_cpu_param = { > + .u_efuse_off = 0x128, > + .soc = &meson_tsensor_g12a, > + .regmap_config = &meson_tsensor_regmap_config_g12a, > +}; > + > +static const struct meson_tsensor_data meson_tsensor_g12a_ddr_param = { > + .u_efuse_off = 0xF0, > + .soc = &meson_tsensor_g12a, > + .regmap_config = &meson_tsensor_regmap_config_g12a, I'm going to guess there is support for other devices on it's way where the regmap_config and soc are different? If it is going to be a while I'd prefer you moved them into this param structure only when you need to. > +}; > + > +static const struct of_device_id meson_tsensor_of_match[] = { > + { > + .compatible = "amlogic,meson-g12a-ddr-tsensor", Given we don't do a match on the compatible without the type specified g12a-tsensor, is there any reason to have that in the binding? > + .data = &meson_tsensor_g12a_ddr_param, > + }, > + { > + .compatible = "amlogic,meson-g12a-cpu-tsensor", > + .data = &meson_tsensor_g12a_cpu_param, > + }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, meson_tsensor_of_match); > + > +static int meson_tsensor_probe(struct platform_device *pdev) > +{ > + struct meson_tsensor *priv; > + struct iio_dev *indio_dev; > + struct resource *res; > + > + int ret; > + > + indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*priv)); > + if (!indio_dev) { > + dev_err(&pdev->dev, "failed allocating iio device\n"); > + return -ENOMEM; > + } > + > + priv = iio_priv(indio_dev); > + priv->data = of_device_get_match_data(&pdev->dev); > + if (!priv->data) { > + dev_err(&pdev->dev, "failed to get match data\n"); > + return -ENODEV; > + } > + > + indio_dev->channels = temperature_channel; > + indio_dev->num_channels = ARRAY_SIZE(temperature_channel); > + indio_dev->name = dev_name(&pdev->dev); > + indio_dev->dev.parent = &pdev->dev; > + indio_dev->dev.of_node = pdev->dev.of_node; > + indio_dev->modes = INDIO_DIRECT_MODE; > + indio_dev->info = &meson_tsensor_iio_info; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + priv->base = devm_ioremap_resource(&pdev->dev, res); > + if (IS_ERR(priv->base)) > + return PTR_ERR(priv->base); > + > + priv->regmap = devm_regmap_init_mmio(&pdev->dev, priv->base, > + priv->data->regmap_config); > + if (IS_ERR(priv->regmap)) > + return PTR_ERR(priv->regmap); > + > + priv->clk = devm_clk_get(&pdev->dev, NULL); > + if (IS_ERR(priv->clk)) { > + if (PTR_ERR(priv->clk) != -EPROBE_DEFER) > + dev_err(&pdev->dev, "failed to get clock\n"); > + return PTR_ERR(priv->clk); > + } > + > + if (of_property_read_u32(pdev->dev.of_node, > + "amlogic,critical-temperature", > + &priv->reboot_temp)) { > + priv->reboot_temp = 0; > + } > + > + priv->sec_ao_map = syscon_regmap_lookup_by_phandle > + (pdev->dev.of_node, "amlogic,ao-secure"); > + if (IS_ERR(priv->sec_ao_map)) { > + dev_err(&pdev->dev, "syscon regmap lookup failed.\n"); > + return PTR_ERR(priv->sec_ao_map); > + } > + > + ret = meson_tsensor_initialize(indio_dev); > + if (ret) > + return ret; > + > + ret = meson_tsensor_enable(indio_dev); > + if (ret) > + goto err; > + > + platform_set_drvdata(pdev, indio_dev); > + ret = iio_device_register(indio_dev); > + if (ret) > + goto err_hw; > + > + return 0; > + > +err_hw: > + meson_tsensor_disable(indio_dev); > +err: > + clk_unprepare(priv->clk); > + > + return ret; > +} > + > +static int meson_tsensor_remove(struct platform_device *pdev) > +{ > + struct iio_dev *indio_dev = platform_get_drvdata(pdev); > + > + iio_device_unregister(indio_dev); > + > + return meson_tsensor_disable(indio_dev); Is there a missing clk_unprepare in here? One option to think about is whether to use devm_add_action_or_reset to move all cleanup into the device managed queue. That way you can drop the error handling in probe and get rid of remove entirely. > +} > + > +static struct platform_driver meson_tsensor_driver = { > + .probe = meson_tsensor_probe, > + .remove = meson_tsensor_remove, > + .driver = { > + .name = "meson-tsensor", > + .of_match_table = meson_tsensor_of_match, > + }, > +}; > + > +module_platform_driver(meson_tsensor_driver); > + > +MODULE_AUTHOR("Guillaume La Roque <glaroque@baylibre.com>"); > +MODULE_DESCRIPTION("Amlogic Meson Temperature Sensor Driver"); > +MODULE_LICENSE("GPL v2"); _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-06-11 17:58 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-06-04 14:47 [PATCH 0/3] Add support of New Amlogic temperature sensor for G12A SoCs Guillaume La Roque 2019-06-04 14:47 ` [PATCH 1/3] Documentation: dt-bindings: add the Amlogic Meson Temperature Sensor Guillaume La Roque 2019-06-06 19:16 ` Martin Blumenstingl 2019-06-08 13:10 ` Jonathan Cameron 2019-06-11 11:01 ` Neil Armstrong 2019-06-11 17:57 ` Martin Blumenstingl 2019-06-08 13:08 ` Jonathan Cameron 2019-06-04 14:47 ` [PATCH 2/3] arm64: dts: meson: g12a: add temperature sensor node Guillaume La Roque 2019-06-04 14:47 ` [PATCH 3/3] iio: temperature: add a driver for the temperature sensor found in Amlogic Meson G12 SoCs Guillaume La Roque 2019-06-06 19:40 ` Martin Blumenstingl 2019-06-08 13:29 ` Jonathan Cameron
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).