[PATCH 1/7] dt-bindings: Add DT bindings documentation for Allwinner Thermal Sensor Controller

[PATCH 1/7] dt-bindings: Add DT bindings documentation for Allwinner Thermal Sensor Controller

[Emmanuel Vadot]

This patch adds documentation for Device-Tree bindings for the Allwinner
Thermal Sensor Controller found on the H3, H5 and A64 SoCs

Signed-off-by: Emmanuel Vadot <manu@freebsd.org>
---
 .../bindings/thermal/allwinner-thermal.txt    | 41 +++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/allwinner-thermal.txt

diff --git a/Documentation/devicetree/bindings/thermal/allwinner-thermal.txt b/Documentation/devicetree/bindings/thermal/allwinner-thermal.txt
new file mode 100644
index 000000000000..5810d44cf495
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/allwinner-thermal.txt
@@ -0,0 +1,41 @@
+* Thermal Sensor Controller on Allwinner SoCs
+
+Required properties:
+- compatible : should be "allwinner,<name>-ths"
+   "allwinner,sun8i-h3-ths": found on H3 and H2+ SoCs
+   "allwinner,sun50i-h5-ths": found on H5 SoC
+   "allwinner,sun50i-a64-ths": found on H5 SoC
+- reg : physical base address of the controller and length of memory mapped
+	region.
+- interrupts : The interrupt number to the cpu. The interrupt specifier format
+	       depends on the interrupt controller.
+- clocks : Must contain an entry for each entry in clock-names.
+- clock-names : Shall be "apb" for the bus, and "ths" for
+		the peripheral clock.
+- resets : Must contain an entry for each entry in reset-names.
+	   See ../reset/reset.txt for details.
+- reset-names : Must be "apb".
+- #thermal-sensor-cells : Depend on the SoC
+   For H3 should be 0
+   For H5 should be 1
+   For A64 should be 2
+   See ./thermal.txt for a description.
+- nvmem-cells : Phandle to the calibration data
+- nvmem-cell-names = Should be "ths-calib"
+
+Example:
+
+ths: thermal_sensor@1c25000 {
+	compatible = "allwinner,sun8i-h3-ths";
+	reg = <0x01c25000 0x100>;
+	interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
+	clocks = <&ccu CLK_BUS_THS>, <&ccu CLK_THS>;
+	clock-names = "apb", "ths";
+	resets = <&ccu RST_BUS_THS>;
+	reset-names = "apb";
+	#thermal-sensor-cells = <0>;
+	status = "disabled";
+
+	nvmem-cells = <&ths_calib>;
+	nvmem-cell-names = "ths-calib";
+};
-- 
2.18.0

Re: [PATCH 1/7] dt-bindings: Add DT bindings documentation for Allwinner Thermal Sensor Controller

[Rob Herring]

On Sat, Aug 04, 2018 at 09:03:49AM +0200, Emmanuel Vadot wrote:
> This patch adds documentation for Device-Tree bindings for the Allwinner
> Thermal Sensor Controller found on the H3, H5 and A64 SoCs
> 
> Signed-off-by: Emmanuel Vadot <manu@freebsd.org>
> ---
>  .../bindings/thermal/allwinner-thermal.txt    | 41 +++++++++++++++++++
>  1 file changed, 41 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/thermal/allwinner-thermal.txt
> 
> diff --git a/Documentation/devicetree/bindings/thermal/allwinner-thermal.txt b/Documentation/devicetree/bindings/thermal/allwinner-thermal.txt
> new file mode 100644
> index 000000000000..5810d44cf495
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/allwinner-thermal.txt
> @@ -0,0 +1,41 @@
> +* Thermal Sensor Controller on Allwinner SoCs
> +
> +Required properties:
> +- compatible : should be "allwinner,<name>-ths"
> +   "allwinner,sun8i-h3-ths": found on H3 and H2+ SoCs
> +   "allwinner,sun50i-h5-ths": found on H5 SoC
> +   "allwinner,sun50i-a64-ths": found on H5 SoC
> +- reg : physical base address of the controller and length of memory mapped
> +	region.
> +- interrupts : The interrupt number to the cpu. The interrupt specifier format
> +	       depends on the interrupt controller.

Just need to say how many entries (and order if more than one).

> +- clocks : Must contain an entry for each entry in clock-names.
> +- clock-names : Shall be "apb" for the bus, and "ths" for
> +		the peripheral clock.
> +- resets : Must contain an entry for each entry in reset-names.
> +	   See ../reset/reset.txt for details.
> +- reset-names : Must be "apb".
> +- #thermal-sensor-cells : Depend on the SoC
> +   For H3 should be 0
> +   For H5 should be 1
> +   For A64 should be 2
> +   See ./thermal.txt for a description.
> +- nvmem-cells : Phandle to the calibration data
> +- nvmem-cell-names = Should be "ths-calib"
> +
> +Example:
> +
> +ths: thermal_sensor@1c25000 {

thermal-sensor@...

> +	compatible = "allwinner,sun8i-h3-ths";
> +	reg = <0x01c25000 0x100>;
> +	interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
> +	clocks = <&ccu CLK_BUS_THS>, <&ccu CLK_THS>;
> +	clock-names = "apb", "ths";
> +	resets = <&ccu RST_BUS_THS>;
> +	reset-names = "apb";
> +	#thermal-sensor-cells = <0>;
> +	status = "disabled";
> +
> +	nvmem-cells = <&ths_calib>;
> +	nvmem-cell-names = "ths-calib";
> +};
> -- 
> 2.18.0
> 

Re: [PATCH 1/7] dt-bindings: Add DT bindings documentation for Allwinner Thermal Sensor Controller

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 501 bytes --]

On Sat, Aug 04, 2018 at 09:03:49AM +0200, Emmanuel Vadot wrote:
> This patch adds documentation for Device-Tree bindings for the Allwinner
> Thermal Sensor Controller found on the H3, H5 and A64 SoCs
> 
> Signed-off-by: Emmanuel Vadot <manu@freebsd.org>

I'm not going to merge a binding for a device that doesn't have any
driver implemented at the moment in Linux.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/7] dt-bindings: Add DT bindings documentation for Allwinner Thermal Sensor Controller

[Rob Herring]

On Mon, Aug 20, 2018 at 5:17 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Sat, Aug 04, 2018 at 09:03:49AM +0200, Emmanuel Vadot wrote:
> > This patch adds documentation for Device-Tree bindings for the Allwinner
> > Thermal Sensor Controller found on the H3, H5 and A64 SoCs
> >
> > Signed-off-by: Emmanuel Vadot <manu@freebsd.org>
>
> I'm not going to merge a binding for a device that doesn't have any
> driver implemented at the moment in Linux.

I'll take it then. Linux is not the only DT client.

Rob

Re: [PATCH 1/7] dt-bindings: Add DT bindings documentation for Allwinner Thermal Sensor Controller

[Emmanuel Vadot]

On Mon, 20 Aug 2018 07:41:22 -0600
Rob Herring <robh+dt@kernel.org> wrote:

> On Mon, Aug 20, 2018 at 5:17 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> >
> > On Sat, Aug 04, 2018 at 09:03:49AM +0200, Emmanuel Vadot wrote:
> > > This patch adds documentation for Device-Tree bindings for the Allwinner
> > > Thermal Sensor Controller found on the H3, H5 and A64 SoCs
> > >
> > > Signed-off-by: Emmanuel Vadot <manu@freebsd.org>
> >
> > I'm not going to merge a binding for a device that doesn't have any
> > driver implemented at the moment in Linux.
> 
> I'll take it then. Linux is not the only DT client.
> 
> Rob

 Thanks Rob, I'll send a V2 shortly with the modification you asked and
some minor others documentation corrections.

-- 
Emmanuel Vadot <manu@bidouilliste.com> <manu@freebsd.org>

Re: [PATCH 1/7] dt-bindings: Add DT bindings documentation for Allwinner Thermal Sensor Controller

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1132 bytes --]

On Mon, Aug 20, 2018 at 07:41:22AM -0600, Rob Herring wrote:
> On Mon, Aug 20, 2018 at 5:17 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> >
> > On Sat, Aug 04, 2018 at 09:03:49AM +0200, Emmanuel Vadot wrote:
> > > This patch adds documentation for Device-Tree bindings for the Allwinner
> > > Thermal Sensor Controller found on the H3, H5 and A64 SoCs
> > >
> > > Signed-off-by: Emmanuel Vadot <manu@freebsd.org>
> >
> > I'm not going to merge a binding for a device that doesn't have any
> > driver implemented at the moment in Linux.
> 
> I'll take it then. Linux is not the only DT client.

Then don't complain if we ever have to break the ABI. We never tested
that hardware, never had any code running on it, and it *will* cause
some issues. For example, the calibration data have never been used
and how they should be represented have never been described, since no
one ever actually tried to use it.

And sure, Linux is not the only DT client. Just like FreeBSD isn't.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/7] dt-bindings: Add DT bindings documentation for Allwinner Thermal Sensor Controller

[Emmanuel Vadot]

On Mon, 20 Aug 2018 16:07:37 +0200
Maxime Ripard <maxime.ripard@bootlin.com> wrote:

> On Mon, Aug 20, 2018 at 07:41:22AM -0600, Rob Herring wrote:
> > On Mon, Aug 20, 2018 at 5:17 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > >
> > > On Sat, Aug 04, 2018 at 09:03:49AM +0200, Emmanuel Vadot wrote:
> > > > This patch adds documentation for Device-Tree bindings for the Allwinner
> > > > Thermal Sensor Controller found on the H3, H5 and A64 SoCs
> > > >
> > > > Signed-off-by: Emmanuel Vadot <manu@freebsd.org>
> > >
> > > I'm not going to merge a binding for a device that doesn't have any
> > > driver implemented at the moment in Linux.
> > 
> > I'll take it then. Linux is not the only DT client.
> 
> Then don't complain if we ever have to break the ABI. We never tested
> that hardware, never had any code running on it, and it *will* cause
> some issues. For example, the calibration data have never been used
> and how they should be represented have never been described, since no
> one ever actually tried to use it.
> 
> And sure, Linux is not the only DT client. Just like FreeBSD isn't.
> 
> Maxime

 Maxime,

 What kind of data would you need for me to make things better ?
 We have the driver in FreeBSD for almost two years now, whne I started
to see how to upstream it I noticed a lot of problems and spent 4 or 5
days to try on all the SoCs mentionned in the serie, the only SoC I
didn't include it the A83T as for some reason I couldn't make the
driver work.
 I just hope that you understand that we cannot wait for Linux to have
a driver to have some bindings.

 Cheers,

-- 
Emmanuel Vadot <manu@bidouilliste.com> <manu@freebsd.org>

Re: [PATCH 1/7] dt-bindings: Add DT bindings documentation for Allwinner Thermal Sensor Controller

[Rob Herring]

On Mon, Aug 20, 2018 at 9:07 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Mon, Aug 20, 2018 at 07:41:22AM -0600, Rob Herring wrote:
> > On Mon, Aug 20, 2018 at 5:17 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > >
> > > On Sat, Aug 04, 2018 at 09:03:49AM +0200, Emmanuel Vadot wrote:
> > > > This patch adds documentation for Device-Tree bindings for the Allwinner
> > > > Thermal Sensor Controller found on the H3, H5 and A64 SoCs
> > > >
> > > > Signed-off-by: Emmanuel Vadot <manu@freebsd.org>
> > >
> > > I'm not going to merge a binding for a device that doesn't have any
> > > driver implemented at the moment in Linux.
> >
> > I'll take it then. Linux is not the only DT client.
>
> Then don't complain if we ever have to break the ABI.

My only complaint is when breaking ABIs and not being clear about
doing so. Otherwise, it is up to platform maintainers to not upset
their users (including if some of those users are FreeBSD).

> We never tested
> that hardware, never had any code running on it, and it *will* cause
> some issues. For example, the calibration data have never been used
> and how they should be represented have never been described, since no
> one ever actually tried to use it.

Then perhaps that part of the binding should be omitted for now. But I
don't see how if this works for FreeBSD how it wouldn't for Linux.

Rob

Re: [PATCH 1/7] dt-bindings: Add DT bindings documentation for Allwinner Thermal Sensor Controller

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2156 bytes --]

Hi,

On Mon, Aug 20, 2018 at 04:27:15PM +0200, Emmanuel Vadot wrote:
> On Mon, 20 Aug 2018 16:07:37 +0200
> Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> 
> > On Mon, Aug 20, 2018 at 07:41:22AM -0600, Rob Herring wrote:
> > > On Mon, Aug 20, 2018 at 5:17 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > >
> > > > On Sat, Aug 04, 2018 at 09:03:49AM +0200, Emmanuel Vadot wrote:
> > > > > This patch adds documentation for Device-Tree bindings for the Allwinner
> > > > > Thermal Sensor Controller found on the H3, H5 and A64 SoCs
> > > > >
> > > > > Signed-off-by: Emmanuel Vadot <manu@freebsd.org>
> > > >
> > > > I'm not going to merge a binding for a device that doesn't have any
> > > > driver implemented at the moment in Linux.
> > > 
> > > I'll take it then. Linux is not the only DT client.
> > 
> > Then don't complain if we ever have to break the ABI. We never tested
> > that hardware, never had any code running on it, and it *will* cause
> > some issues. For example, the calibration data have never been used
> > and how they should be represented have never been described, since no
> > one ever actually tried to use it.
> > 
> > And sure, Linux is not the only DT client. Just like FreeBSD isn't.
> 
>  What kind of data would you need for me to make things better ?
>  We have the driver in FreeBSD for almost two years now, whne I started
> to see how to upstream it I noticed a lot of problems and spent 4 or 5
> days to try on all the SoCs mentionned in the serie, the only SoC I
> didn't include it the A83T as for some reason I couldn't make the
> driver work.

Do you have a link to that driver? How and how much did you test it?

Are you using the calibration data stored in the SID?

> I just hope that you understand that we cannot wait for Linux to
> have a driver to have some bindings.

Just like I'm sure you can understand that just merging the DT bits
without testing anything caused some troubles in the past, and I don't
want to discover it in a year from now.

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/7] dt-bindings: Add DT bindings documentation for Allwinner Thermal Sensor Controller

[Emmanuel Vadot]

 Hi,

On Fri, 24 Aug 2018 16:58:40 +0200
Maxime Ripard <maxime.ripard@bootlin.com> wrote:

> Hi,
> 
> On Mon, Aug 20, 2018 at 04:27:15PM +0200, Emmanuel Vadot wrote:
> > On Mon, 20 Aug 2018 16:07:37 +0200
> > Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > 
> > > On Mon, Aug 20, 2018 at 07:41:22AM -0600, Rob Herring wrote:
> > > > On Mon, Aug 20, 2018 at 5:17 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > >
> > > > > On Sat, Aug 04, 2018 at 09:03:49AM +0200, Emmanuel Vadot wrote:
> > > > > > This patch adds documentation for Device-Tree bindings for the Allwinner
> > > > > > Thermal Sensor Controller found on the H3, H5 and A64 SoCs
> > > > > >
> > > > > > Signed-off-by: Emmanuel Vadot <manu@freebsd.org>
> > > > >
> > > > > I'm not going to merge a binding for a device that doesn't have any
> > > > > driver implemented at the moment in Linux.
> > > > 
> > > > I'll take it then. Linux is not the only DT client.
> > > 
> > > Then don't complain if we ever have to break the ABI. We never tested
> > > that hardware, never had any code running on it, and it *will* cause
> > > some issues. For example, the calibration data have never been used
> > > and how they should be represented have never been described, since no
> > > one ever actually tried to use it.
> > > 
> > > And sure, Linux is not the only DT client. Just like FreeBSD isn't.
> > 
> >  What kind of data would you need for me to make things better ?
> >  We have the driver in FreeBSD for almost two years now, whne I started
> > to see how to upstream it I noticed a lot of problems and spent 4 or 5
> > days to try on all the SoCs mentionned in the serie, the only SoC I
> > didn't include it the A83T as for some reason I couldn't make the
> > driver work.
> 
> Do you have a link to that driver? 

 https://github.com/freebsd/freebsd/blob/master/sys/arm/allwinner/aw_thermal.c

> How and how much did you test it?

 Using a cheap IR thermometer as it's all I can afford.
 Then running multiples cpufreq-a53 process and comparing the result.
 Did the same without using the calibration data, result where closer
when I used them.

> Are you using the calibration data stored in the SID?

 Yes, which is why the serie contain the nvmem cell for the SID.

> 
> > I just hope that you understand that we cannot wait for Linux to
> > have a driver to have some bindings.
> 
> Just like I'm sure you can understand that just merging the DT bits
> without testing anything caused some troubles in the past, and I don't
> want to discover it in a year from now.

 I do, I don't like wrong DT info as much as you do I think. I had
the unfortunate event to find that the sun4i-a10-timer compatible used
in every soc is plain wrong for !A10 and !A13, patches comming soon.
 I want those bits merged but I also want that they reflect reality,
this is not a hard driver to do, NetBSD also have one I think, maybe
OpenBSD do to.

> Maxime
> 
> -- 
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com


-- 
Emmanuel Vadot <manu@bidouilliste.com> <manu@freebsd.org>

Re: [PATCH 1/7] dt-bindings: Add DT bindings documentation for Allwinner Thermal Sensor Controller

[Eduardo Valentin]

On Fri, Aug 24, 2018 at 09:59:21PM +0200, Emmanuel Vadot wrote:
> 
>  Hi,
> 
> On Fri, 24 Aug 2018 16:58:40 +0200
> Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> 
> > Hi,
> > 
> > On Mon, Aug 20, 2018 at 04:27:15PM +0200, Emmanuel Vadot wrote:
> > > On Mon, 20 Aug 2018 16:07:37 +0200
> > > Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > 
> > > > On Mon, Aug 20, 2018 at 07:41:22AM -0600, Rob Herring wrote:
> > > > > On Mon, Aug 20, 2018 at 5:17 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > >
> > > > > > On Sat, Aug 04, 2018 at 09:03:49AM +0200, Emmanuel Vadot wrote:
> > > > > > > This patch adds documentation for Device-Tree bindings for the Allwinner
> > > > > > > Thermal Sensor Controller found on the H3, H5 and A64 SoCs
> > > > > > >
> > > > > > > Signed-off-by: Emmanuel Vadot <manu@freebsd.org>
> > > > > >
> > > > > > I'm not going to merge a binding for a device that doesn't have any
> > > > > > driver implemented at the moment in Linux.
> > > > > 
> > > > > I'll take it then. Linux is not the only DT client.
> > > > 
> > > > Then don't complain if we ever have to break the ABI. We never tested
> > > > that hardware, never had any code running on it, and it *will* cause
> > > > some issues. For example, the calibration data have never been used
> > > > and how they should be represented have never been described, since no
> > > > one ever actually tried to use it.
> > > > 
> > > > And sure, Linux is not the only DT client. Just like FreeBSD isn't.
> > > 
> > >  What kind of data would you need for me to make things better ?
> > >  We have the driver in FreeBSD for almost two years now, whne I started
> > > to see how to upstream it I noticed a lot of problems and spent 4 or 5
> > > days to try on all the SoCs mentionned in the serie, the only SoC I
> > > didn't include it the A83T as for some reason I couldn't make the
> > > driver work.
> > 
> > Do you have a link to that driver? 
> 
>  https://github.com/freebsd/freebsd/blob/master/sys/arm/allwinner/aw_thermal.c
> 
> > How and how much did you test it?
> 
>  Using a cheap IR thermometer as it's all I can afford.
>  Then running multiples cpufreq-a53 process and comparing the result.
>  Did the same without using the calibration data, result where closer
> when I used them.
> 
> > Are you using the calibration data stored in the SID?
> 
>  Yes, which is why the serie contain the nvmem cell for the SID.
> 
> > 
> > > I just hope that you understand that we cannot wait for Linux to
> > > have a driver to have some bindings.
> > 
> > Just like I'm sure you can understand that just merging the DT bits
> > without testing anything caused some troubles in the past, and I don't
> > want to discover it in a year from now.
> 
>  I do, I don't like wrong DT info as much as you do I think. I had
> the unfortunate event to find that the sun4i-a10-timer compatible used
> in every soc is plain wrong for !A10 and !A13, patches comming soon.
>  I want those bits merged but I also want that they reflect reality,
> this is not a hard driver to do, NetBSD also have one I think, maybe
> OpenBSD do to.


I would prefer to see a series of DT bindings + driver. Can you send
your next iteration with the driver so we can review both together?

> 
> > Maxime
> > 
> > -- 
> > Maxime Ripard, Bootlin
> > Embedded Linux and Kernel engineering
> > https://bootlin.com
> 
> 
> -- 
> Emmanuel Vadot <manu@bidouilliste.com> <manu@freebsd.org>

Re: [PATCH 1/7] dt-bindings: Add DT bindings documentation for Allwinner Thermal Sensor Controller

[Mark Kettenis]

Hi Emmanuel,

I think the following bit in your proposed binding is wrong:

+- #thermal-sensor-cells : Depend on the SoC
+   For H3 should be 0
+   For H5 should be 1
+   For A64 should be 2
+   See ./thermal.txt for a description.

I think what you're trying to say here is that H3 has a single sensor
and that H5 and A64 have more sensors.  But even though the number of
sensors is different for H5 and A64 (two for H3, three for A64) you
need only a single cell to describe which sensor to read.  So
#thermal-sensors-cells should be 1 for both H5 and A64.

As far as I can see this proposed binding is compatible with the
OpenBSD's sxitemp(4) driver.  That driver also supports the R40 with
"allwinner,sun8i-r40-ths" as the compatible string.  The R40 has two
sensors so #thermal-sensor-cells should be 1 for this SoC as well.

Cheers,

Mark

Re: [PATCH 1/7] dt-bindings: Add DT bindings documentation for Allwinner Thermal Sensor Controller

[Emmanuel Vadot]

 Hi Mark,

On Sat, 25 Aug 2018 13:07:40 +0200 (CEST)
Mark Kettenis <mark.kettenis@xs4all.nl> wrote:

> Hi Emmanuel,
> 
> I think the following bit in your proposed binding is wrong:
> 
> +- #thermal-sensor-cells : Depend on the SoC
> +   For H3 should be 0
> +   For H5 should be 1
> +   For A64 should be 2
> +   See ./thermal.txt for a description.
> 
> I think what you're trying to say here is that H3 has a single sensor
> and that H5 and A64 have more sensors.  But even though the number of
> sensors is different for H5 and A64 (two for H3, three for A64) you
> need only a single cell to describe which sensor to read.  So
> #thermal-sensors-cells should be 1 for both H5 and A64.

 You might be right, this part wasn't really clear for me reading the
thermal binding docs. Can someone double-confirm that ?

> As far as I can see this proposed binding is compatible with the
> OpenBSD's sxitemp(4) driver. 

 Go to know.

> That driver also supports the R40 with
> "allwinner,sun8i-r40-ths" as the compatible string.  The R40 has two
> sensors so #thermal-sensor-cells should be 1 for this SoC as well.

 I don't have R40 board, I guess the best thing will be for you to send
a patch to update the binding docs and dts when this serie will be
merged/acked.

> Cheers,
> 
> Mark

 Cheers,

-- 
Emmanuel Vadot <manu@bidouilliste.com> <manu@freebsd.org>

Re: [PATCH 1/7] dt-bindings: Add DT bindings documentation for Allwinner Thermal Sensor Controller

[Emmanuel Vadot]

 Hi,

On Fri, 24 Aug 2018 16:03:40 -0700
Eduardo Valentin <edubezval@gmail.com> wrote:

> On Fri, Aug 24, 2018 at 09:59:21PM +0200, Emmanuel Vadot wrote:
> > 
> >  Hi,
> > 
> > On Fri, 24 Aug 2018 16:58:40 +0200
> > Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > 
> > > Hi,
> > > 
> > > On Mon, Aug 20, 2018 at 04:27:15PM +0200, Emmanuel Vadot wrote:
> > > > On Mon, 20 Aug 2018 16:07:37 +0200
> > > > Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > 
> > > > > On Mon, Aug 20, 2018 at 07:41:22AM -0600, Rob Herring wrote:
> > > > > > On Mon, Aug 20, 2018 at 5:17 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > > >
> > > > > > > On Sat, Aug 04, 2018 at 09:03:49AM +0200, Emmanuel Vadot wrote:
> > > > > > > > This patch adds documentation for Device-Tree bindings for the Allwinner
> > > > > > > > Thermal Sensor Controller found on the H3, H5 and A64 SoCs
> > > > > > > >
> > > > > > > > Signed-off-by: Emmanuel Vadot <manu@freebsd.org>
> > > > > > >
> > > > > > > I'm not going to merge a binding for a device that doesn't have any
> > > > > > > driver implemented at the moment in Linux.
> > > > > > 
> > > > > > I'll take it then. Linux is not the only DT client.
> > > > > 
> > > > > Then don't complain if we ever have to break the ABI. We never tested
> > > > > that hardware, never had any code running on it, and it *will* cause
> > > > > some issues. For example, the calibration data have never been used
> > > > > and how they should be represented have never been described, since no
> > > > > one ever actually tried to use it.
> > > > > 
> > > > > And sure, Linux is not the only DT client. Just like FreeBSD isn't.
> > > > 
> > > >  What kind of data would you need for me to make things better ?
> > > >  We have the driver in FreeBSD for almost two years now, whne I started
> > > > to see how to upstream it I noticed a lot of problems and spent 4 or 5
> > > > days to try on all the SoCs mentionned in the serie, the only SoC I
> > > > didn't include it the A83T as for some reason I couldn't make the
> > > > driver work.
> > > 
> > > Do you have a link to that driver? 
> > 
> >  https://github.com/freebsd/freebsd/blob/master/sys/arm/allwinner/aw_thermal.c
> > 
> > > How and how much did you test it?
> > 
> >  Using a cheap IR thermometer as it's all I can afford.
> >  Then running multiples cpufreq-a53 process and comparing the result.
> >  Did the same without using the calibration data, result where closer
> > when I used them.
> > 
> > > Are you using the calibration data stored in the SID?
> > 
> >  Yes, which is why the serie contain the nvmem cell for the SID.
> > 
> > > 
> > > > I just hope that you understand that we cannot wait for Linux to
> > > > have a driver to have some bindings.
> > > 
> > > Just like I'm sure you can understand that just merging the DT bits
> > > without testing anything caused some troubles in the past, and I don't
> > > want to discover it in a year from now.
> > 
> >  I do, I don't like wrong DT info as much as you do I think. I had
> > the unfortunate event to find that the sun4i-a10-timer compatible used
> > in every soc is plain wrong for !A10 and !A13, patches comming soon.
> >  I want those bits merged but I also want that they reflect reality,
> > this is not a hard driver to do, NetBSD also have one I think, maybe
> > OpenBSD do to.
> 
> 
> I would prefer to see a series of DT bindings + driver. Can you send
> your next iteration with the driver so we can review both together?

 You mean a link to the FreeBSD driver (and OpenBSD since they have one
too) in the cover letter ?

> > 
> > > Maxime
> > > 
> > > -- 
> > > Maxime Ripard, Bootlin
> > > Embedded Linux and Kernel engineering
> > > https://bootlin.com
> > 
> > 
> > -- 
> > Emmanuel Vadot <manu@bidouilliste.com> <manu@freebsd.org>
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


-- 
Emmanuel Vadot <manu@bidouilliste.com> <manu@freebsd.org>

Re: [PATCH 1/7] dt-bindings: Add DT bindings documentation for Allwinner Thermal Sensor Controller

[Emmanuel Vadot]

On Sat, 25 Aug 2018 17:42:48 +0200
Emmanuel Vadot <manu@bidouilliste.com> wrote:

> 
>  Hi Mark,
> 
> On Sat, 25 Aug 2018 13:07:40 +0200 (CEST)
> Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> 
> > Hi Emmanuel,
> > 
> > I think the following bit in your proposed binding is wrong:
> > 
> > +- #thermal-sensor-cells : Depend on the SoC
> > +   For H3 should be 0
> > +   For H5 should be 1
> > +   For A64 should be 2
> > +   See ./thermal.txt for a description.
> > 
> > I think what you're trying to say here is that H3 has a single sensor
> > and that H5 and A64 have more sensors.  But even though the number of
> > sensors is different for H5 and A64 (two for H3, three for A64) you
> > need only a single cell to describe which sensor to read.  So
> > #thermal-sensors-cells should be 1 for both H5 and A64.
> 
>  You might be right, this part wasn't really clear for me reading the
> thermal binding docs. Can someone double-confirm that ?

 Re-reading the doc with this information and now it's clear, I'll
correct this in the new serie.

> > As far as I can see this proposed binding is compatible with the
> > OpenBSD's sxitemp(4) driver. 
> 
>  Go to know.
> 
> > That driver also supports the R40 with
> > "allwinner,sun8i-r40-ths" as the compatible string.  The R40 has two
> > sensors so #thermal-sensor-cells should be 1 for this SoC as well.
> 
>  I don't have R40 board, I guess the best thing will be for you to send
> a patch to update the binding docs and dts when this serie will be
> merged/acked.
> 
> > Cheers,
> > 
> > Mark
> 
>  Cheers,
> 
> -- 
> Emmanuel Vadot <manu@bidouilliste.com> <manu@freebsd.org>
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


-- 
Emmanuel Vadot <manu@bidouilliste.com> <manu@freebsd.org>

Re: [PATCH 1/7] dt-bindings: Add DT bindings documentation for Allwinner Thermal Sensor Controller

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 4066 bytes --]

On Sat, Aug 25, 2018 at 05:43:52PM +0200, Emmanuel Vadot wrote:
> 
>  Hi,
> 
> On Fri, 24 Aug 2018 16:03:40 -0700
> Eduardo Valentin <edubezval@gmail.com> wrote:
> 
> > On Fri, Aug 24, 2018 at 09:59:21PM +0200, Emmanuel Vadot wrote:
> > > 
> > >  Hi,
> > > 
> > > On Fri, 24 Aug 2018 16:58:40 +0200
> > > Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > 
> > > > Hi,
> > > > 
> > > > On Mon, Aug 20, 2018 at 04:27:15PM +0200, Emmanuel Vadot wrote:
> > > > > On Mon, 20 Aug 2018 16:07:37 +0200
> > > > > Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > 
> > > > > > On Mon, Aug 20, 2018 at 07:41:22AM -0600, Rob Herring wrote:
> > > > > > > On Mon, Aug 20, 2018 at 5:17 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > > > >
> > > > > > > > On Sat, Aug 04, 2018 at 09:03:49AM +0200, Emmanuel Vadot wrote:
> > > > > > > > > This patch adds documentation for Device-Tree bindings for the Allwinner
> > > > > > > > > Thermal Sensor Controller found on the H3, H5 and A64 SoCs
> > > > > > > > >
> > > > > > > > > Signed-off-by: Emmanuel Vadot <manu@freebsd.org>
> > > > > > > >
> > > > > > > > I'm not going to merge a binding for a device that doesn't have any
> > > > > > > > driver implemented at the moment in Linux.
> > > > > > > 
> > > > > > > I'll take it then. Linux is not the only DT client.
> > > > > > 
> > > > > > Then don't complain if we ever have to break the ABI. We never tested
> > > > > > that hardware, never had any code running on it, and it *will* cause
> > > > > > some issues. For example, the calibration data have never been used
> > > > > > and how they should be represented have never been described, since no
> > > > > > one ever actually tried to use it.
> > > > > > 
> > > > > > And sure, Linux is not the only DT client. Just like FreeBSD isn't.
> > > > > 
> > > > >  What kind of data would you need for me to make things better ?
> > > > >  We have the driver in FreeBSD for almost two years now, whne I started
> > > > > to see how to upstream it I noticed a lot of problems and spent 4 or 5
> > > > > days to try on all the SoCs mentionned in the serie, the only SoC I
> > > > > didn't include it the A83T as for some reason I couldn't make the
> > > > > driver work.
> > > > 
> > > > Do you have a link to that driver? 
> > > 
> > >  https://github.com/freebsd/freebsd/blob/master/sys/arm/allwinner/aw_thermal.c
> > > 
> > > > How and how much did you test it?
> > > 
> > >  Using a cheap IR thermometer as it's all I can afford.
> > >  Then running multiples cpufreq-a53 process and comparing the result.
> > >  Did the same without using the calibration data, result where closer
> > > when I used them.
> > > 
> > > > Are you using the calibration data stored in the SID?
> > > 
> > >  Yes, which is why the serie contain the nvmem cell for the SID.
> > > 
> > > > 
> > > > > I just hope that you understand that we cannot wait for Linux to
> > > > > have a driver to have some bindings.
> > > > 
> > > > Just like I'm sure you can understand that just merging the DT bits
> > > > without testing anything caused some troubles in the past, and I don't
> > > > want to discover it in a year from now.
> > > 
> > >  I do, I don't like wrong DT info as much as you do I think. I had
> > > the unfortunate event to find that the sun4i-a10-timer compatible used
> > > in every soc is plain wrong for !A10 and !A13, patches comming soon.
> > >  I want those bits merged but I also want that they reflect reality,
> > > this is not a hard driver to do, NetBSD also have one I think, maybe
> > > OpenBSD do to.
> > 
> > 
> > I would prefer to see a series of DT bindings + driver. Can you send
> > your next iteration with the driver so we can review both together?
> 
>  You mean a link to the FreeBSD driver (and OpenBSD since they have one
> too) in the cover letter ?

That would definitely be valuable, yes, thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/7] dt-bindings: Add DT bindings documentation for Allwinner Thermal Sensor Controller

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 3432 bytes --]

On Fri, Aug 24, 2018 at 09:59:21PM +0200, Emmanuel Vadot wrote:
> 
>  Hi,
> 
> On Fri, 24 Aug 2018 16:58:40 +0200
> Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> 
> > Hi,
> > 
> > On Mon, Aug 20, 2018 at 04:27:15PM +0200, Emmanuel Vadot wrote:
> > > On Mon, 20 Aug 2018 16:07:37 +0200
> > > Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > 
> > > > On Mon, Aug 20, 2018 at 07:41:22AM -0600, Rob Herring wrote:
> > > > > On Mon, Aug 20, 2018 at 5:17 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > >
> > > > > > On Sat, Aug 04, 2018 at 09:03:49AM +0200, Emmanuel Vadot wrote:
> > > > > > > This patch adds documentation for Device-Tree bindings for the Allwinner
> > > > > > > Thermal Sensor Controller found on the H3, H5 and A64 SoCs
> > > > > > >
> > > > > > > Signed-off-by: Emmanuel Vadot <manu@freebsd.org>
> > > > > >
> > > > > > I'm not going to merge a binding for a device that doesn't have any
> > > > > > driver implemented at the moment in Linux.
> > > > > 
> > > > > I'll take it then. Linux is not the only DT client.
> > > > 
> > > > Then don't complain if we ever have to break the ABI. We never tested
> > > > that hardware, never had any code running on it, and it *will* cause
> > > > some issues. For example, the calibration data have never been used
> > > > and how they should be represented have never been described, since no
> > > > one ever actually tried to use it.
> > > > 
> > > > And sure, Linux is not the only DT client. Just like FreeBSD isn't.
> > > 
> > >  What kind of data would you need for me to make things better ?
> > >  We have the driver in FreeBSD for almost two years now, whne I started
> > > to see how to upstream it I noticed a lot of problems and spent 4 or 5
> > > days to try on all the SoCs mentionned in the serie, the only SoC I
> > > didn't include it the A83T as for some reason I couldn't make the
> > > driver work.
> > 
> > Do you have a link to that driver? 
> 
>  https://github.com/freebsd/freebsd/blob/master/sys/arm/allwinner/aw_thermal.c
> 
> > How and how much did you test it?
> 
>  Using a cheap IR thermometer as it's all I can afford.
>  Then running multiples cpufreq-a53 process and comparing the result.
>  Did the same without using the calibration data, result where closer
> when I used them.

Ok.

> > Are you using the calibration data stored in the SID?
> 
>  Yes, which is why the serie contain the nvmem cell for the SID.

Ok.

> > 
> > > I just hope that you understand that we cannot wait for Linux to
> > > have a driver to have some bindings.
> > 
> > Just like I'm sure you can understand that just merging the DT bits
> > without testing anything caused some troubles in the past, and I don't
> > want to discover it in a year from now.
> 
>  I do, I don't like wrong DT info as much as you do I think. I had
> the unfortunate event to find that the sun4i-a10-timer compatible used
> in every soc is plain wrong for !A10 and !A13, patches comming soon.
>  I want those bits merged but I also want that they reflect reality,
> this is not a hard driver to do, NetBSD also have one I think, maybe
> OpenBSD do to.

I guess these one went un-noticed because they are not the default
source of time keeping and events in the kernel.

Thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]