| Message ID | 20170901065818.2037-2-fe@dev.tdt.de (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Fri, Sep 01, 2017 at 08:58:18AM +0200, Florian Eckert wrote: > Document the devicetree bindings for the ltq-cputemp > > Signed-off-by: Florian Eckert <fe@dev.tdt.de> I see nothing special with the bindings, and they are in line with other lantiq bindings, so I am taking the risk of accepting this patch (and the matching driver) without waiting for explicit approval from Rob. Rob, scream at me if that is not ok. Thanks, Guenter > --- > Documentation/devicetree/bindings/hwmon/ltq-cputemp.txt | 10 ++++++++++ > 1 file changed, 10 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwmon/ltq-cputemp.txt > > diff --git a/Documentation/devicetree/bindings/hwmon/ltq-cputemp.txt b/Documentation/devicetree/bindings/hwmon/ltq-cputemp.txt > new file mode 100644 > index 000000000000..33fd00a987c7 > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwmon/ltq-cputemp.txt > @@ -0,0 +1,10 @@ > +Lantiq cpu temperatur sensor > + > +Requires node properties: > +- compatible value : > + "lantiq,cputemp" > + > +Example: > + cputemp@0 { > + compatible = "lantiq,cputemp"; > + }; > -- > 2.11.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Sep 01, 2017 at 07:26:22AM -0700, Guenter Roeck wrote: > On Fri, Sep 01, 2017 at 08:58:18AM +0200, Florian Eckert wrote: > > Document the devicetree bindings for the ltq-cputemp > > > > Signed-off-by: Florian Eckert <fe@dev.tdt.de> > > I see nothing special with the bindings, and they are in line with other lantiq > bindings, so I am taking the risk of accepting this patch (and the matching > driver) without waiting for explicit approval from Rob. Rob, scream at me if > that is not ok. Perhaps the existing one was not well reviewed. > > Thanks, > Guenter > > > --- > > Documentation/devicetree/bindings/hwmon/ltq-cputemp.txt | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/hwmon/ltq-cputemp.txt > > > > diff --git a/Documentation/devicetree/bindings/hwmon/ltq-cputemp.txt b/Documentation/devicetree/bindings/hwmon/ltq-cputemp.txt > > new file mode 100644 > > index 000000000000..33fd00a987c7 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/hwmon/ltq-cputemp.txt > > @@ -0,0 +1,10 @@ > > +Lantiq cpu temperatur sensor s/temperatur/temperature/ > > + > > +Requires node properties: > > +- compatible value : > > + "lantiq,cputemp" Kind of non-specific. How is this device even accessed without any other property? > > + > > +Example: > > + cputemp@0 { unit-address without reg property is not valid. > > + compatible = "lantiq,cputemp"; > > + }; > > -- > > 2.11.0 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Rob >> > --- /dev/null >> > +++ b/Documentation/devicetree/bindings/hwmon/ltq-cputemp.txt >> > @@ -0,0 +1,10 @@ >> > +Lantiq cpu temperatur sensor > > s/temperatur/temperature/ Will update this in a follow up page based on the old one. So no v4? > >> > + >> > +Requires node properties: >> > +- compatible value : >> > + "lantiq,cputemp" > > Kind of non-specific. How is this device even accessed without any > other > property? It does not need any further properties. If this is set in the device tree then the driver is loaded. After loading the temperature could be read from "/sys/class/hwmon". Let me know what should i do to get this fixed? Thanks for feedback Florian -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Sep 12, 2017 at 11:20:08AM -0500, Rob Herring wrote: > On Fri, Sep 01, 2017 at 07:26:22AM -0700, Guenter Roeck wrote: > > On Fri, Sep 01, 2017 at 08:58:18AM +0200, Florian Eckert wrote: > > > Document the devicetree bindings for the ltq-cputemp > > > > > > Signed-off-by: Florian Eckert <fe@dev.tdt.de> > > > > I see nothing special with the bindings, and they are in line with other lantiq > > bindings, so I am taking the risk of accepting this patch (and the matching > > driver) without waiting for explicit approval from Rob. Rob, scream at me if > > that is not ok. > > Perhaps the existing one was not well reviewed. > Bummer. Should teach me to never accept DT patches without your review. I owe you a beer or two. Guenter > > > > Thanks, > > Guenter > > > > > --- > > > Documentation/devicetree/bindings/hwmon/ltq-cputemp.txt | 10 ++++++++++ > > > 1 file changed, 10 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/hwmon/ltq-cputemp.txt > > > > > > diff --git a/Documentation/devicetree/bindings/hwmon/ltq-cputemp.txt b/Documentation/devicetree/bindings/hwmon/ltq-cputemp.txt > > > new file mode 100644 > > > index 000000000000..33fd00a987c7 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/hwmon/ltq-cputemp.txt > > > @@ -0,0 +1,10 @@ > > > +Lantiq cpu temperatur sensor > > s/temperatur/temperature/ > > > > + > > > +Requires node properties: > > > +- compatible value : > > > + "lantiq,cputemp" > > Kind of non-specific. How is this device even accessed without any other > property? > > > > + > > > +Example: > > > + cputemp@0 { > > unit-address without reg property is not valid. > > > > + compatible = "lantiq,cputemp"; > > > + }; > > > -- > > > 2.11.0 > > > > > > -- > > > To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in > > > the body of a message to majordomo@vger.kernel.org > > > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Sep 13, 2017 at 07:36:16AM +0200, Florian Eckert wrote: > Hello Rob > > >>> --- /dev/null > >>> +++ b/Documentation/devicetree/bindings/hwmon/ltq-cputemp.txt > >>> @@ -0,0 +1,10 @@ > >>> +Lantiq cpu temperatur sensor > > > >s/temperatur/temperature/ > > Will update this in a follow up page based on the old one. So no v4? > Please send a follow-up patch. Thanks, Guenter > > > >>> + > >>> +Requires node properties: > >>> +- compatible value : > >>> + "lantiq,cputemp" > > > >Kind of non-specific. How is this device even accessed without any other > >property? > > It does not need any further properties. If this is set in the device tree > then the driver is loaded. > After loading the temperature could be read from "/sys/class/hwmon". > > Let me know what should i do to get this fixed? > > Thanks for feedback > > Florian > -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>> > >> >>> + >> >>> +Requires node properties: >> >>> +- compatible value : >> >>> + "lantiq,cputemp" >> > >> >Kind of non-specific. How is this device even accessed without any other >> >property? >> >> It does not need any further properties. If this is set in the device >> tree >> then the driver is loaded. >> After loading the temperature could be read from "/sys/class/hwmon". >> Let me know what should i do to get this fixed? >> What about with this is this OK from your side or do I have do to something? So I only update "s/temperatur/temperature/" with an follow-up patch based the current linux-next tree? Thanks Florian -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Sep 13, 2017 at 04:42:33PM +0200, Florian Eckert wrote: > >>> > >>>>> + > >>>>> +Requires node properties: > >>>>> +- compatible value : > >>>>> + "lantiq,cputemp" > >>> > >>>Kind of non-specific. How is this device even accessed without any other > >>>property? > >> > >>It does not need any further properties. If this is set in the device > >>tree > >>then the driver is loaded. > >>After loading the temperature could be read from "/sys/class/hwmon". > >>Let me know what should i do to get this fixed? > >> > > What about with this is this OK from your side or do I have do to something? > So I only update "s/temperatur/temperature/" with an follow-up patch based > the current linux-next tree? > Also s/cputemp@0/cputemp/ if I understand Rob's comment correctly. Question for Rob: The driver checks the SOC version and bails out if the version is not vr9 v1.2. Should that be expressed in DT ? Guenter > Thanks > > Florian > > -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Sep 13, 2017 at 12:36 AM, Florian Eckert <fe@dev.tdt.de> wrote: > Hello Rob > >>> > --- /dev/null >>> > +++ b/Documentation/devicetree/bindings/hwmon/ltq-cputemp.txt >>> > @@ -0,0 +1,10 @@ >>> > +Lantiq cpu temperatur sensor >> >> >> s/temperatur/temperature/ > > > Will update this in a follow up page based on the old one. So no v4? > >> >>> > + >>> > +Requires node properties: >>> > +- compatible value : >>> > + "lantiq,cputemp" >> >> >> Kind of non-specific. How is this device even accessed without any other >> property? > > > It does not need any further properties. If this is set in the device tree > then the driver is loaded. DT is not the only way to instantiate drivers. What I meant is how do you access the hardware? That should be evident from the binding and it is not. Looking at the driver, you have some memory mapped system control registers which get ioremapped in arch/mips/lantiq/xway/sysctrl.c and accesses thru some platform specific macros. That is not the ideal way to do things as we use syscon and regmap for such things. But that's all mostly kernel details not so relevant to the DT binding. For DT, I'd expect this is a child node of the sysctrl block with a reg property value of <0x40 4> (along with any other child devices). You could also not even put this in DT and the system controller can have it's own driver that instantiates the child device for this driver. Rob -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Rob >>>> > + >>>> > +Requires node properties: >>>> > +- compatible value : >>>> > + "lantiq,cputemp" >>> >>> >>> Kind of non-specific. How is this device even accessed without any >>> other >>> property? >> >> >> It does not need any further properties. If this is set in the device >> tree >> then the driver is loaded. > > DT is not the only way to instantiate drivers. > > What I meant is how do you access the hardware? That should be evident > from the binding and it is not. Agree with our statement. > > Looking at the driver, you have some memory mapped system control > registers which get ioremapped in arch/mips/lantiq/xway/sysctrl.c and > accesses thru some platform specific macros. That is not the ideal way > to do things as we use syscon and regmap for such things. But that's > all mostly kernel details not so relevant to the DT binding. For lanitq xrx200 this is all i have. So if i have to use syscon and regmap i am also not sure how to handle this. > For DT, I'd expect this is a child node of the sysctrl block with a > reg property value of <0x40 4> (along with any other child devices). > You could also not even put this in DT and the system controller can > have it's own driver that instantiates the child device for this > driver. Yes this would be the best practice. But the hardware designer for what ever reason placed the Register for the temperature sensors into the CGU (Clock Generation Unit) section! And the Register is also shared with some other stuff which is not only assign for temperature stuff! I am not sure how to handle this in the device tree. This is a Register description extract from the data sheet GPHY Configuration Register 01 This register configures the booting options of GPHY1 IP. Offset 0x0040 Reset Value 0x01FC0000 31: RES 30: 100FX_H 29: 100FX_F 28: 10BT_F 27: 10BT_H 26: 100BT_F 25: 100BT_H 24: 1000BT_F 23: 1000BT_H 22: RES 21: RES 19: TEMP_PD <--- NEEDED Power down the Temperature Sensor 18: TEMP_HL <--- NEEDED Indicate temperature higher than 128 C 17: TEMP <--- NEEDED Value 16: TEMP <--- NEEDED Value 15: TEMP <--- NEEDED Value 14: TEMP <--- NEEDED Value 13: TEMP <--- NEEDED Value 12: TEMP <--- NEEDED Value 11: TEMP <--- NEEDED Value 10: TEMP <--- NEEDED Value 09: TEMP <--- NEEDED Value 08: RES 07: RES 06: SPI_Delay 05: SPI_Delay 04: AHB_EnD 03: DMA_OR 02: RES 01: RES 00: RES And that is a dts tree from LEDE/Openwrt for lantiq URL: https://github.com/lede-project/source/blob/c88770c766fdc5599efc4672bca230017f52e8e4/target/linux/lantiq/dts/vr9.dtsi#L54 Extraction: sram@1F000000 { #address-cells = <1>; #size-cells = <1>; compatible = "lantiq,sram", "simple-bus"; reg = <0x1F000000 0x800000>; ranges = <0x0 0x1F000000 0x7FFFFF>; eiu0: eiu@101000 { #interrupt-cells = <1>; interrupt-controller; compatible = "lantiq,eiu-xway"; reg = <0x101000 0x1000>; interrupt-parent = <&icu0>; lantiq,eiu-irqs = <166 135 66 40 41 42>; }; pmu0: pmu@102000 { compatible = "lantiq,pmu-xway"; reg = <0x102000 0x1000>; }; cgu0: cgu@103000 { compatible = "lantiq,cgu-xway"; reg = <0x103000 0x1000>; -> This is the place to add the binding? }; Sorry for the noise but i am unsure how to do this. Thanks for help Florian -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Sep 14, 2017 at 2:06 AM, Florian Eckert <fe@dev.tdt.de> wrote: > Hello Rob > >>>>> > + >>>>> > +Requires node properties: >>>>> > +- compatible value : >>>>> > + "lantiq,cputemp" >>>> >>>> >>>> >>>> Kind of non-specific. How is this device even accessed without any other >>>> property? >>> >>> >>> >>> It does not need any further properties. If this is set in the device >>> tree >>> then the driver is loaded. >> >> >> DT is not the only way to instantiate drivers. >> >> What I meant is how do you access the hardware? That should be evident >> from the binding and it is not. > > > Agree with our statement. > >> >> Looking at the driver, you have some memory mapped system control >> registers which get ioremapped in arch/mips/lantiq/xway/sysctrl.c and >> accesses thru some platform specific macros. That is not the ideal way >> to do things as we use syscon and regmap for such things. But that's >> all mostly kernel details not so relevant to the DT binding. > > > For lanitq xrx200 this is all i have. So if i have to use syscon and regmap > i am also not sure how to handle this. > >> For DT, I'd expect this is a child node of the sysctrl block with a >> reg property value of <0x40 4> (along with any other child devices). >> You could also not even put this in DT and the system controller can >> have it's own driver that instantiates the child device for this >> driver. > > > Yes this would be the best practice. But the hardware designer for what ever > reason > placed the Register for the temperature sensors into the CGU (Clock > Generation Unit) section! > And the Register is also shared with some other stuff which is not only > assign for temperature > stuff! I am not sure how to handle this in the device tree. This is quite common and there are plenty of examples. Look for bindings with "syscon". > > This is a Register description extract from the data sheet > > GPHY Configuration Register 01 > This register configures the booting options of GPHY1 IP. > > Offset 0x0040 > Reset Value 0x01FC0000 > > 31: RES > 30: 100FX_H > 29: 100FX_F > 28: 10BT_F > 27: 10BT_H > 26: 100BT_F > 25: 100BT_H > 24: 1000BT_F > 23: 1000BT_H > 22: RES > 21: RES > 19: TEMP_PD <--- NEEDED Power down the Temperature Sensor > 18: TEMP_HL <--- NEEDED Indicate temperature higher than 128 C > 17: TEMP <--- NEEDED Value > 16: TEMP <--- NEEDED Value > 15: TEMP <--- NEEDED Value > 14: TEMP <--- NEEDED Value > 13: TEMP <--- NEEDED Value > 12: TEMP <--- NEEDED Value > 11: TEMP <--- NEEDED Value > 10: TEMP <--- NEEDED Value > 09: TEMP <--- NEEDED Value > 08: RES > 07: RES > 06: SPI_Delay > 05: SPI_Delay > 04: AHB_EnD > 03: DMA_OR > 02: RES > 01: RES > 00: RES > > And that is a dts tree from LEDE/Openwrt for lantiq > URL: > https://github.com/lede-project/source/blob/c88770c766fdc5599efc4672bca230017f52e8e4/target/linux/lantiq/dts/vr9.dtsi#L54 > > Extraction: > sram@1F000000 { > #address-cells = <1>; > #size-cells = <1>; > compatible = "lantiq,sram", "simple-bus"; > reg = <0x1F000000 0x800000>; > ranges = <0x0 0x1F000000 0x7FFFFF>; > > eiu0: eiu@101000 { > #interrupt-cells = <1>; > interrupt-controller; > compatible = "lantiq,eiu-xway"; > reg = <0x101000 0x1000>; > interrupt-parent = <&icu0>; > lantiq,eiu-irqs = <166 135 66 40 41 42>; > }; > > pmu0: pmu@102000 { > compatible = "lantiq,pmu-xway"; > reg = <0x102000 0x1000>; > }; > > cgu0: cgu@103000 { > compatible = "lantiq,cgu-xway"; > reg = <0x103000 0x1000>; > -> This is the place to add the binding? Yes. > }; > > Sorry for the noise but i am unsure how to do this. > Thanks for help > > Florian -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/hwmon/ltq-cputemp.txt b/Documentation/devicetree/bindings/hwmon/ltq-cputemp.txt new file mode 100644 index 000000000000..33fd00a987c7 --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/ltq-cputemp.txt @@ -0,0 +1,10 @@ +Lantiq cpu temperatur sensor + +Requires node properties: +- compatible value : + "lantiq,cputemp" + +Example: + cputemp@0 { + compatible = "lantiq,cputemp"; + };
Document the devicetree bindings for the ltq-cputemp Signed-off-by: Florian Eckert <fe@dev.tdt.de> --- Documentation/devicetree/bindings/hwmon/ltq-cputemp.txt | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/ltq-cputemp.txt