| Message ID | 20241106023916.440767-2-j2anfernee@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: adc: add Nuvoton NCT720x ADC driver | expand |
On 06/11/2024 09:39, Eason Yang wrote: > This adds a binding specification for the Nuvoton NCT7201/NCT7202 > family of ADCs. > > Signed-off-by: Eason Yang <j2anfernee@gmail.com> > --- > .../bindings/iio/adc/nuvoton,nct720x.yaml | 47 +++++++++++++++++++ > MAINTAINERS | 1 + > 2 files changed, 48 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > > diff --git a/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml b/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > new file mode 100644 > index 000000000000..3052039af10e > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > @@ -0,0 +1,47 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iio/adc/nuvoton,nct720x.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Nuvoton nct7202 and similar ADCs > + > +maintainers: > + - Eason Yang <yhyang2@nuvoton.com> > + > +description: | > + Family of ADCs with i2c interface. > + > +properties: > + compatible: > + enum: > + - nuvoton,nct7201 > + - nuvoton,nct7202 > + > + reg: > + maxItems: 1 > + > + read-vin-data-size: Is it generic property or vendor property? I tried to find in the https://github.com/torvalds/linux/tree/master/Documentation/devicetree/bindings , but it seems this property hasn't been used on other devices. If it is vendor property, then I think it should include a vendor prefix. For examples: https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/adi%2Cad7780.yaml#L50 https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/fsl%2Cvf610-adc.yaml#L42 https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/st%2Cstmpe-adc.yaml#L22 > + description: number of data bits per read vin > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [8, 16] > + > +required: > + - compatible > + - reg > + - read-vin-data-size > + > +additionalProperties: false > + > +examples: > + - | > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + nct7202@1d { I think the Node name should follow https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation For some examples that were merged before https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/adi%2Cad7091r5.yaml#L102 https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/maxim%2Cmax1238.yaml#L73 https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/ti%2Cadc081c.yaml#L49 > + compatible = "nuvoton,nct7202"; > + reg = <0x1d>; > + read-vin-data-size = <8>; > + }; > + }; > diff --git a/MAINTAINERS b/MAINTAINERS > index 91d0609db61b..68570c58e7aa 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -2746,6 +2746,7 @@ L: openbmc@lists.ozlabs.org (moderated for non-subscribers) > S: Supported > F: Documentation/devicetree/bindings/*/*/*npcm* > F: Documentation/devicetree/bindings/*/*npcm* > +F: Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > F: Documentation/devicetree/bindings/rtc/nuvoton,nct3018y.yaml > F: arch/arm/boot/dts/nuvoton/nuvoton-npcm* > F: arch/arm/mach-npcm/
Dear Chanh Nguyen, Thank you for your response. Chanh Nguyen <chanh@amperemail.onmicrosoft.com> 於 2024年11月6日 週三 下午12:58寫道： > > > > On 06/11/2024 09:39, Eason Yang wrote: > > This adds a binding specification for the Nuvoton NCT7201/NCT7202 > > family of ADCs. > > > > Signed-off-by: Eason Yang <j2anfernee@gmail.com> > > --- > > .../bindings/iio/adc/nuvoton,nct720x.yaml | 47 +++++++++++++++++++ > > MAINTAINERS | 1 + > > 2 files changed, 48 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > > > > diff --git a/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml b/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > > new file mode 100644 > > index 000000000000..3052039af10e > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > > @@ -0,0 +1,47 @@ > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/iio/adc/nuvoton,nct720x.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Nuvoton nct7202 and similar ADCs > > + > > +maintainers: > > + - Eason Yang <yhyang2@nuvoton.com> > > + > > +description: | > > + Family of ADCs with i2c interface. > > + > > +properties: > > + compatible: > > + enum: > > + - nuvoton,nct7201 > > + - nuvoton,nct7202 > > + > > + reg: > > + maxItems: 1 > > + > > + read-vin-data-size: > > Is it generic property or vendor property? I tried to find in the > https://github.com/torvalds/linux/tree/master/Documentation/devicetree/bindings > , but it seems this property hasn't been used on other devices. > > If it is vendor property, then I think it should include a vendor > prefix. For examples: > > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/adi%2Cad7780.yaml#L50 > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/fsl%2Cvf610-adc.yaml#L42 > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/st%2Cstmpe-adc.yaml#L22 > > I would add a vendor prefix for it. > > + description: number of data bits per read vin > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + enum: [8, 16] > > + > > +required: > > + - compatible > > + - reg > > + - read-vin-data-size > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + i2c { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + nct7202@1d { > > I think the Node name should follow > https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > > > For some examples that were merged before > > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/adi%2Cad7091r5.yaml#L102 > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/maxim%2Cmax1238.yaml#L73 > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/ti%2Cadc081c.yaml#L49 > I would change it for the node naming. > > + compatible = "nuvoton,nct7202"; > > + reg = <0x1d>; > > + read-vin-data-size = <8>; > > + }; > > + }; > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 91d0609db61b..68570c58e7aa 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -2746,6 +2746,7 @@ L: openbmc@lists.ozlabs.org (moderated for non-subscribers) > > S: Supported > > F: Documentation/devicetree/bindings/*/*/*npcm* > > F: Documentation/devicetree/bindings/*/*npcm* > > +F: Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > > F: Documentation/devicetree/bindings/rtc/nuvoton,nct3018y.yaml > > F: arch/arm/boot/dts/nuvoton/nuvoton-npcm* > > F: arch/arm/mach-npcm/ >
On Wed, Nov 06, 2024 at 11:58:06AM +0700, Chanh Nguyen wrote: > > > On 06/11/2024 09:39, Eason Yang wrote: > > This adds a binding specification for the Nuvoton NCT7201/NCT7202 > > family of ADCs. > > > > Signed-off-by: Eason Yang <j2anfernee@gmail.com> > > --- > > .../bindings/iio/adc/nuvoton,nct720x.yaml | 47 +++++++++++++++++++ > > MAINTAINERS | 1 + > > 2 files changed, 48 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > > > > diff --git a/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml b/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > > new file mode 100644 > > index 000000000000..3052039af10e > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > > @@ -0,0 +1,47 @@ > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/iio/adc/nuvoton,nct720x.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Nuvoton nct7202 and similar ADCs > > + > > +maintainers: > > + - Eason Yang <yhyang2@nuvoton.com> > > + > > +description: | > > + Family of ADCs with i2c interface. > > + > > +properties: > > + compatible: > > + enum: > > + - nuvoton,nct7201 > > + - nuvoton,nct7202 > > + > > + reg: > > + maxItems: 1 > > + > > + read-vin-data-size: > > Is it generic property or vendor property? I tried to find in the > https://github.com/torvalds/linux/tree/master/Documentation/devicetree/bindings > , but it seems this property hasn't been used on other devices. > > If it is vendor property, then I think it should include a vendor prefix. > For examples: > > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/adi%2Cad7780.yaml#L50 > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/fsl%2Cvf610-adc.yaml#L42 > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/st%2Cstmpe-adc.yaml#L22 An explanation of why it cannot be determined from the compatible would also be good. Two compatibles and two values makes me a little suspicious!
Dear Conor Dooley, Thank you for your response. NCT7201 supports 8 voltage monitor inputs while NCT7202 supports 12 voltage monitor inputs. NCT7201 and NCT7202 provide SMBus to access the internal register, and support SMBus byte write and byte/word read protocols. Conor Dooley <conor@kernel.org> 於 2024年11月7日 週四 上午12:13寫道： > > On Wed, Nov 06, 2024 at 11:58:06AM +0700, Chanh Nguyen wrote: > > > > > > On 06/11/2024 09:39, Eason Yang wrote: > > > This adds a binding specification for the Nuvoton NCT7201/NCT7202 > > > family of ADCs. > > > > > > Signed-off-by: Eason Yang <j2anfernee@gmail.com> > > > --- > > > .../bindings/iio/adc/nuvoton,nct720x.yaml | 47 +++++++++++++++++++ > > > MAINTAINERS | 1 + > > > 2 files changed, 48 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > > > > > > diff --git a/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml b/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > > > new file mode 100644 > > > index 000000000000..3052039af10e > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > > > @@ -0,0 +1,47 @@ > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/iio/adc/nuvoton,nct720x.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: Nuvoton nct7202 and similar ADCs > > > + > > > +maintainers: > > > + - Eason Yang <yhyang2@nuvoton.com> > > > + > > > +description: | > > > + Family of ADCs with i2c interface. > > > + > > > +properties: > > > + compatible: > > > + enum: > > > + - nuvoton,nct7201 > > > + - nuvoton,nct7202 > > > + > > > + reg: > > > + maxItems: 1 > > > + > > > + read-vin-data-size: > > > > Is it generic property or vendor property? I tried to find in the > > https://github.com/torvalds/linux/tree/master/Documentation/devicetree/bindings > > , but it seems this property hasn't been used on other devices. > > > > If it is vendor property, then I think it should include a vendor prefix. > > For examples: > > > > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/adi%2Cad7780.yaml#L50 > > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/fsl%2Cvf610-adc.yaml#L42 > > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/st%2Cstmpe-adc.yaml#L22 > > An explanation of why it cannot be determined from the compatible would > also be good. Two compatibles and two values makes me a little > suspicious!
On Wed, 6 Nov 2024 17:22:35 +0800 Yu-Hsian Yang <j2anfernee@gmail.com> wrote: > Dear Chanh Nguyen, > > Thank you for your response. > > Chanh Nguyen <chanh@amperemail.onmicrosoft.com> 於 2024年11月6日 週三 下午12:58寫道： > > > > > > > > On 06/11/2024 09:39, Eason Yang wrote: > > > This adds a binding specification for the Nuvoton NCT7201/NCT7202 > > > family of ADCs. > > > > > > Signed-off-by: Eason Yang <j2anfernee@gmail.com> > > > --- > > > .../bindings/iio/adc/nuvoton,nct720x.yaml | 47 +++++++++++++++++++ > > > MAINTAINERS | 1 + > > > 2 files changed, 48 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > > > > > > diff --git a/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml b/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > > > new file mode 100644 > > > index 000000000000..3052039af10e > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > > > @@ -0,0 +1,47 @@ > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/iio/adc/nuvoton,nct720x.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: Nuvoton nct7202 and similar ADCs > > > + > > > +maintainers: > > > + - Eason Yang <yhyang2@nuvoton.com> > > > + > > > +description: | > > > + Family of ADCs with i2c interface. > > > + > > > +properties: > > > + compatible: > > > + enum: > > > + - nuvoton,nct7201 > > > + - nuvoton,nct7202 > > > + > > > + reg: > > > + maxItems: 1 > > > + > > > + read-vin-data-size: > > > > Is it generic property or vendor property? I tried to find in the > > https://github.com/torvalds/linux/tree/master/Documentation/devicetree/bindings > > , but it seems this property hasn't been used on other devices. > > > > If it is vendor property, then I think it should include a vendor > > prefix. For examples: > > > > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/adi%2Cad7780.yaml#L50 > > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/fsl%2Cvf610-adc.yaml#L42 > > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/st%2Cstmpe-adc.yaml#L22 > > > > > > I would add a vendor prefix for it. Why do we want this at all? Is this device sufficiently high performance that Linux will ever want to trade of resolution against sampling speed? If so that seems like a policy control that belongs in userspace. Note that to support that in IIO I would want a strong justification for why we dno't just set it to 16 always. We just go for maximum resolution in the vast majority of drivers that support control of this. > > > > + description: number of data bits per read vin > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > + enum: [8, 16] > > > + > > > +required: > > > + - compatible > > > + - reg > > > + - read-vin-data-size > > > + > > > +additionalProperties: false > > > + > > > +examples: > > > + - | > > > + i2c { > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + > > > + nct7202@1d { > > > > I think the Node name should follow > > https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > > > > > > For some examples that were merged before > > > > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/adi%2Cad7091r5.yaml#L102 > > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/maxim%2Cmax1238.yaml#L73 > > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/ti%2Cadc081c.yaml#L49 > > > > I would change it for the node naming. > > > > + compatible = "nuvoton,nct7202"; > > > + reg = <0x1d>; > > > + read-vin-data-size = <8>; > > > + }; > > > + }; > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > index 91d0609db61b..68570c58e7aa 100644 > > > --- a/MAINTAINERS > > > +++ b/MAINTAINERS > > > @@ -2746,6 +2746,7 @@ L: openbmc@lists.ozlabs.org (moderated for non-subscribers) > > > S: Supported > > > F: Documentation/devicetree/bindings/*/*/*npcm* > > > F: Documentation/devicetree/bindings/*/*npcm* > > > +F: Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > > > F: Documentation/devicetree/bindings/rtc/nuvoton,nct3018y.yaml > > > F: arch/arm/boot/dts/nuvoton/nuvoton-npcm* > > > F: arch/arm/mach-npcm/ > >
On Wed, 6 Nov 2024 10:39:15 +0800 Eason Yang <j2anfernee@gmail.com> wrote: > This adds a binding specification for the Nuvoton NCT7201/NCT7202 > family of ADCs. > > Signed-off-by: Eason Yang <j2anfernee@gmail.com> Hi Eason, > --- > .../bindings/iio/adc/nuvoton,nct720x.yaml | 47 +++++++++++++++++++ > MAINTAINERS | 1 + > 2 files changed, 48 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > > diff --git a/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml b/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > new file mode 100644 > index 000000000000..3052039af10e > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml Already raised in another review I think, but pick a part number and name after that. Wild card get broken far too often for them to be usable like this. > @@ -0,0 +1,47 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iio/adc/nuvoton,nct720x.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Nuvoton nct7202 and similar ADCs > + > +maintainers: > + - Eason Yang <yhyang2@nuvoton.com> > + > +description: | > + Family of ADCs with i2c interface. > + > +properties: > + compatible: > + enum: > + - nuvoton,nct7201 > + - nuvoton,nct7202 > + > + reg: > + maxItems: 1 > + > + read-vin-data-size: > + description: number of data bits per read vin > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [8, 16] > + > +required: > + - compatible > + - reg > + - read-vin-data-size If you do keep this, then pick a sensible default (16) and drop the required. > + > +additionalProperties: false > + > +examples: > + - | > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + nct7202@1d { > + compatible = "nuvoton,nct7202"; > + reg = <0x1d>; > + read-vin-data-size = <8>; > + }; > + }; > diff --git a/MAINTAINERS b/MAINTAINERS > index 91d0609db61b..68570c58e7aa 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -2746,6 +2746,7 @@ L: openbmc@lists.ozlabs.org (moderated for non-subscribers) > S: Supported > F: Documentation/devicetree/bindings/*/*/*npcm* > F: Documentation/devicetree/bindings/*/*npcm* > +F: Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > F: Documentation/devicetree/bindings/rtc/nuvoton,nct3018y.yaml > F: arch/arm/boot/dts/nuvoton/nuvoton-npcm* > F: arch/arm/mach-npcm/
On Sat, 9 Nov 2024 13:42:28 +0000 Jonathan Cameron <jic23@kernel.org> wrote: > On Wed, 6 Nov 2024 17:22:35 +0800 > Yu-Hsian Yang <j2anfernee@gmail.com> wrote: > > > Dear Chanh Nguyen, > > > > Thank you for your response. > > > > Chanh Nguyen <chanh@amperemail.onmicrosoft.com> 於 2024年11月6日 週三 下午12:58寫道： > > > > > > > > > > > > On 06/11/2024 09:39, Eason Yang wrote: > > > > This adds a binding specification for the Nuvoton NCT7201/NCT7202 > > > > family of ADCs. > > > > > > > > Signed-off-by: Eason Yang <j2anfernee@gmail.com> > > > > --- > > > > .../bindings/iio/adc/nuvoton,nct720x.yaml | 47 +++++++++++++++++++ > > > > MAINTAINERS | 1 + > > > > 2 files changed, 48 insertions(+) > > > > create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > > > > > > > > diff --git a/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml b/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > > > > new file mode 100644 > > > > index 000000000000..3052039af10e > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > > > > @@ -0,0 +1,47 @@ > > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > > > +%YAML 1.2 > > > > +--- > > > > +$id: http://devicetree.org/schemas/iio/adc/nuvoton,nct720x.yaml# > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > + > > > > +title: Nuvoton nct7202 and similar ADCs > > > > + > > > > +maintainers: > > > > + - Eason Yang <yhyang2@nuvoton.com> > > > > + > > > > +description: | > > > > + Family of ADCs with i2c interface. > > > > + > > > > +properties: > > > > + compatible: > > > > + enum: > > > > + - nuvoton,nct7201 > > > > + - nuvoton,nct7202 > > > > + > > > > + reg: > > > > + maxItems: 1 > > > > + > > > > + read-vin-data-size: > > > > > > Is it generic property or vendor property? I tried to find in the > > > https://github.com/torvalds/linux/tree/master/Documentation/devicetree/bindings > > > , but it seems this property hasn't been used on other devices. > > > > > > If it is vendor property, then I think it should include a vendor > > > prefix. For examples: > > > > > > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/adi%2Cad7780.yaml#L50 > > > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/fsl%2Cvf610-adc.yaml#L42 > > > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/st%2Cstmpe-adc.yaml#L22 > > > > > > > > > > I would add a vendor prefix for it. > > Why do we want this at all? Is this device sufficiently high > performance that Linux will ever want to trade of resolution against > sampling speed? > > If so that seems like a policy control that belongs in userspace. Note > that to support that in IIO I would want a strong justification for why we dno't > just set it to 16 always. We just go for maximum resolution in the vast majority > of drivers that support control of this. I'd misunderstood what this is. It's a control no what the i2c word size is. Do we actually care about supporting rubbish i2c controllers? How many can't do a word access? If you do it should be detected from the controller rather than in DT. > > > > > > > > + description: number of data bits per read vin > > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > > + enum: [8, 16] > > > > + > > > > +required: > > > > + - compatible > > > > + - reg > > > > + - read-vin-data-size > > > > + > > > > +additionalProperties: false > > > > + > > > > +examples: > > > > + - | > > > > + i2c { > > > > + #address-cells = <1>; > > > > + #size-cells = <0>; > > > > + > > > > + nct7202@1d { > > > > > > I think the Node name should follow > > > https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > > > > > > > > > For some examples that were merged before > > > > > > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/adi%2Cad7091r5.yaml#L102 > > > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/maxim%2Cmax1238.yaml#L73 > > > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/ti%2Cadc081c.yaml#L49 > > > > > > > I would change it for the node naming. > > > > > > + compatible = "nuvoton,nct7202"; > > > > + reg = <0x1d>; > > > > + read-vin-data-size = <8>; > > > > + }; > > > > + }; > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > > index 91d0609db61b..68570c58e7aa 100644 > > > > --- a/MAINTAINERS > > > > +++ b/MAINTAINERS > > > > @@ -2746,6 +2746,7 @@ L: openbmc@lists.ozlabs.org (moderated for non-subscribers) > > > > S: Supported > > > > F: Documentation/devicetree/bindings/*/*/*npcm* > > > > F: Documentation/devicetree/bindings/*/*npcm* > > > > +F: Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > > > > F: Documentation/devicetree/bindings/rtc/nuvoton,nct3018y.yaml > > > > F: arch/arm/boot/dts/nuvoton/nuvoton-npcm* > > > > F: arch/arm/mach-npcm/ > > > > >
Dear Jonathan Cameron, For property read-vin-data-size, we have a internal discussion. For Nuvoton NCT7201/NCT7202 chip, Take an example as to Vin1: The VIN reading supports Byte read (One Byte) and Word read (Two Byte) For Byte read: First read Index 00h to get VIN1 MSB, then read Index 0Fh Bit 3~7 to get VIN1 LSB. Index 0Fh is a shared LSB for all VINs. For Word read: Read Index 00h and get 2 Byte (VIN1 MSB and VIN1 LSB). We would refer your suggestion, we declare a property named "nvuoton,read-vin-data-size" with default value 16 for user to use. Jonathan Cameron <jic23@kernel.org> 於 2024年11月9日 週六 下午10:29寫道： > > On Sat, 9 Nov 2024 13:42:28 +0000 > Jonathan Cameron <jic23@kernel.org> wrote: > > > On Wed, 6 Nov 2024 17:22:35 +0800 > > Yu-Hsian Yang <j2anfernee@gmail.com> wrote: > > > > > Dear Chanh Nguyen, > > > > > > Thank you for your response. > > > > > > Chanh Nguyen <chanh@amperemail.onmicrosoft.com> 於 2024年11月6日 週三 下午12:58寫道： > > > > > > > > > > > > > > > > On 06/11/2024 09:39, Eason Yang wrote: > > > > > This adds a binding specification for the Nuvoton NCT7201/NCT7202 > > > > > family of ADCs. > > > > > > > > > > Signed-off-by: Eason Yang <j2anfernee@gmail.com> > > > > > --- > > > > > .../bindings/iio/adc/nuvoton,nct720x.yaml | 47 +++++++++++++++++++ > > > > > MAINTAINERS | 1 + > > > > > 2 files changed, 48 insertions(+) > > > > > create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml b/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > > > > > new file mode 100644 > > > > > index 000000000000..3052039af10e > > > > > --- /dev/null > > > > > +++ b/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > > > > > @@ -0,0 +1,47 @@ > > > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > > > > +%YAML 1.2 > > > > > +--- > > > > > +$id: http://devicetree.org/schemas/iio/adc/nuvoton,nct720x.yaml# > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > > + > > > > > +title: Nuvoton nct7202 and similar ADCs > > > > > + > > > > > +maintainers: > > > > > + - Eason Yang <yhyang2@nuvoton.com> > > > > > + > > > > > +description: | > > > > > + Family of ADCs with i2c interface. > > > > > + > > > > > +properties: > > > > > + compatible: > > > > > + enum: > > > > > + - nuvoton,nct7201 > > > > > + - nuvoton,nct7202 > > > > > + > > > > > + reg: > > > > > + maxItems: 1 > > > > > + > > > > > + read-vin-data-size: > > > > > > > > Is it generic property or vendor property? I tried to find in the > > > > https://github.com/torvalds/linux/tree/master/Documentation/devicetree/bindings > > > > , but it seems this property hasn't been used on other devices. > > > > > > > > If it is vendor property, then I think it should include a vendor > > > > prefix. For examples: > > > > > > > > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/adi%2Cad7780.yaml#L50 > > > > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/fsl%2Cvf610-adc.yaml#L42 > > > > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/st%2Cstmpe-adc.yaml#L22 > > > > > > > > > > > > > > I would add a vendor prefix for it. > > > > Why do we want this at all? Is this device sufficiently high > > performance that Linux will ever want to trade of resolution against > > sampling speed? > > > > If so that seems like a policy control that belongs in userspace. Note > > that to support that in IIO I would want a strong justification for why we dno't > > just set it to 16 always. We just go for maximum resolution in the vast majority > > of drivers that support control of this. > I'd misunderstood what this is. It's a control no what the i2c word size is. > Do we actually care about supporting rubbish i2c controllers? How many > can't do a word access? > > If you do it should be detected from the controller rather than in DT. > > > > > > > > > > > > > + description: number of data bits per read vin > > > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > > > + enum: [8, 16] > > > > > + > > > > > +required: > > > > > + - compatible > > > > > + - reg > > > > > + - read-vin-data-size > > > > > + > > > > > +additionalProperties: false > > > > > + > > > > > +examples: > > > > > + - | > > > > > + i2c { > > > > > + #address-cells = <1>; > > > > > + #size-cells = <0>; > > > > > + > > > > > + nct7202@1d { > > > > > > > > I think the Node name should follow > > > > https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > > > > > > > > > > > > For some examples that were merged before > > > > > > > > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/adi%2Cad7091r5.yaml#L102 > > > > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/maxim%2Cmax1238.yaml#L73 > > > > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/ti%2Cadc081c.yaml#L49 > > > > > > > > > > I would change it for the node naming. > > > > > > > > + compatible = "nuvoton,nct7202"; > > > > > + reg = <0x1d>; > > > > > + read-vin-data-size = <8>; > > > > > + }; > > > > > + }; > > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > > > index 91d0609db61b..68570c58e7aa 100644 > > > > > --- a/MAINTAINERS > > > > > +++ b/MAINTAINERS > > > > > @@ -2746,6 +2746,7 @@ L: openbmc@lists.ozlabs.org (moderated for non-subscribers) > > > > > S: Supported > > > > > F: Documentation/devicetree/bindings/*/*/*npcm* > > > > > F: Documentation/devicetree/bindings/*/*npcm* > > > > > +F: Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > > > > > F: Documentation/devicetree/bindings/rtc/nuvoton,nct3018y.yaml > > > > > F: arch/arm/boot/dts/nuvoton/nuvoton-npcm* > > > > > F: arch/arm/mach-npcm/ > > > > > > > > >
On Mon, 11 Nov 2024 15:45:03 +0800 Yu-Hsian Yang <j2anfernee@gmail.com> wrote: > Dear Jonathan Cameron, > > For property read-vin-data-size, we have a internal discussion. > > For Nuvoton NCT7201/NCT7202 chip, > Take an example as to Vin1: > The VIN reading supports Byte read (One Byte) and Word read (Two Byte) > > For Byte read: > First read Index 00h to get VIN1 MSB, then read Index 0Fh Bit 3~7 to > get VIN1 LSB. > Index 0Fh is a shared LSB for all VINs. > > For Word read: > Read Index 00h and get 2 Byte (VIN1 MSB and VIN1 LSB). > > We would refer your suggestion, > we declare a property named "nvuoton,read-vin-data-size" with default value 16 > for user to use. Thanks for the info. If the i2c controller allows word read then the right thing is to always use it. Just check for I2C_FUNC_SMBUS_READ_WORD_DATA with i2c_check_functionality() If it's supported use i2c_smbus_read_word_swapped() if not, do the i2c_smbus_read_byte() approach. We don't need to want this in DT as it is a property of the smbus controller, not this device. Jonathan > > Jonathan Cameron <jic23@kernel.org> 於 2024年11月9日 週六 下午10:29寫道： > > > > On Sat, 9 Nov 2024 13:42:28 +0000 > > Jonathan Cameron <jic23@kernel.org> wrote: > > > > > On Wed, 6 Nov 2024 17:22:35 +0800 > > > Yu-Hsian Yang <j2anfernee@gmail.com> wrote: > > > > > > > Dear Chanh Nguyen, > > > > > > > > Thank you for your response. > > > > > > > > Chanh Nguyen <chanh@amperemail.onmicrosoft.com> 於 2024年11月6日 週三 下午12:58寫道： > > > > > > > > > > > > > > > > > > > > On 06/11/2024 09:39, Eason Yang wrote: > > > > > > This adds a binding specification for the Nuvoton NCT7201/NCT7202 > > > > > > family of ADCs. > > > > > > > > > > > > Signed-off-by: Eason Yang <j2anfernee@gmail.com> > > > > > > --- > > > > > > .../bindings/iio/adc/nuvoton,nct720x.yaml | 47 +++++++++++++++++++ > > > > > > MAINTAINERS | 1 + > > > > > > 2 files changed, 48 insertions(+) > > > > > > create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml b/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > > > > > > new file mode 100644 > > > > > > index 000000000000..3052039af10e > > > > > > --- /dev/null > > > > > > +++ b/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > > > > > > @@ -0,0 +1,47 @@ > > > > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > > > > > +%YAML 1.2 > > > > > > +--- > > > > > > +$id: http://devicetree.org/schemas/iio/adc/nuvoton,nct720x.yaml# > > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > > > + > > > > > > +title: Nuvoton nct7202 and similar ADCs > > > > > > + > > > > > > +maintainers: > > > > > > + - Eason Yang <yhyang2@nuvoton.com> > > > > > > + > > > > > > +description: | > > > > > > + Family of ADCs with i2c interface. > > > > > > + > > > > > > +properties: > > > > > > + compatible: > > > > > > + enum: > > > > > > + - nuvoton,nct7201 > > > > > > + - nuvoton,nct7202 > > > > > > + > > > > > > + reg: > > > > > > + maxItems: 1 > > > > > > + > > > > > > + read-vin-data-size: > > > > > > > > > > Is it generic property or vendor property? I tried to find in the > > > > > https://github.com/torvalds/linux/tree/master/Documentation/devicetree/bindings > > > > > , but it seems this property hasn't been used on other devices. > > > > > > > > > > If it is vendor property, then I think it should include a vendor > > > > > prefix. For examples: > > > > > > > > > > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/adi%2Cad7780.yaml#L50 > > > > > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/fsl%2Cvf610-adc.yaml#L42 > > > > > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/st%2Cstmpe-adc.yaml#L22 > > > > > > > > > > > > > > > > > > I would add a vendor prefix for it. > > > > > > Why do we want this at all? Is this device sufficiently high > > > performance that Linux will ever want to trade of resolution against > > > sampling speed? > > > > > > If so that seems like a policy control that belongs in userspace. Note > > > that to support that in IIO I would want a strong justification for why we dno't > > > just set it to 16 always. We just go for maximum resolution in the vast majority > > > of drivers that support control of this. > > I'd misunderstood what this is. It's a control no what the i2c word size is. > > Do we actually care about supporting rubbish i2c controllers? How many > > can't do a word access? > > > > If you do it should be detected from the controller rather than in DT. > > > > > > > > > > > > > > > > > > + description: number of data bits per read vin > > > > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > > > > + enum: [8, 16] > > > > > > + > > > > > > +required: > > > > > > + - compatible > > > > > > + - reg > > > > > > + - read-vin-data-size > > > > > > + > > > > > > +additionalProperties: false > > > > > > + > > > > > > +examples: > > > > > > + - | > > > > > > + i2c { > > > > > > + #address-cells = <1>; > > > > > > + #size-cells = <0>; > > > > > > + > > > > > > + nct7202@1d { > > > > > > > > > > I think the Node name should follow > > > > > https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > > > > > > > > > > > > > > > For some examples that were merged before > > > > > > > > > > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/adi%2Cad7091r5.yaml#L102 > > > > > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/maxim%2Cmax1238.yaml#L73 > > > > > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/ti%2Cadc081c.yaml#L49 > > > > > > > > > > > > > I would change it for the node naming. > > > > > > > > > > + compatible = "nuvoton,nct7202"; > > > > > > + reg = <0x1d>; > > > > > > + read-vin-data-size = <8>; > > > > > > + }; > > > > > > + }; > > > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > > > > index 91d0609db61b..68570c58e7aa 100644 > > > > > > --- a/MAINTAINERS > > > > > > +++ b/MAINTAINERS > > > > > > @@ -2746,6 +2746,7 @@ L: openbmc@lists.ozlabs.org (moderated for non-subscribers) > > > > > > S: Supported > > > > > > F: Documentation/devicetree/bindings/*/*/*npcm* > > > > > > F: Documentation/devicetree/bindings/*/*npcm* > > > > > > +F: Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > > > > > > F: Documentation/devicetree/bindings/rtc/nuvoton,nct3018y.yaml > > > > > > F: arch/arm/boot/dts/nuvoton/nuvoton-npcm* > > > > > > F: arch/arm/mach-npcm/ > > > > > > > > > > > > >
diff --git a/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml b/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml new file mode 100644 index 000000000000..3052039af10e --- /dev/null +++ b/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml @@ -0,0 +1,47 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/adc/nuvoton,nct720x.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Nuvoton nct7202 and similar ADCs + +maintainers: + - Eason Yang <yhyang2@nuvoton.com> + +description: | + Family of ADCs with i2c interface. + +properties: + compatible: + enum: + - nuvoton,nct7201 + - nuvoton,nct7202 + + reg: + maxItems: 1 + + read-vin-data-size: + description: number of data bits per read vin + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [8, 16] + +required: + - compatible + - reg + - read-vin-data-size + +additionalProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + nct7202@1d { + compatible = "nuvoton,nct7202"; + reg = <0x1d>; + read-vin-data-size = <8>; + }; + }; diff --git a/MAINTAINERS b/MAINTAINERS index 91d0609db61b..68570c58e7aa 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2746,6 +2746,7 @@ L: openbmc@lists.ozlabs.org (moderated for non-subscribers) S: Supported F: Documentation/devicetree/bindings/*/*/*npcm* F: Documentation/devicetree/bindings/*/*npcm* +F: Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml F: Documentation/devicetree/bindings/rtc/nuvoton,nct3018y.yaml F: arch/arm/boot/dts/nuvoton/nuvoton-npcm* F: arch/arm/mach-npcm/
This adds a binding specification for the Nuvoton NCT7201/NCT7202 family of ADCs. Signed-off-by: Eason Yang <j2anfernee@gmail.com> --- .../bindings/iio/adc/nuvoton,nct720x.yaml | 47 +++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 48 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml