| Message ID | 20230915062926.2460502-3-Delphine_CC_Chiu@wiwynn.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | hwmon: max31790: support to config PWM as TACH | expand |
Yo, On Fri, Sep 15, 2023 at 02:29:24PM +0800, Delphine CC Chiu wrote: > Add dt-bindings for the MAXIM MAX31790. > > Signed-off-by: Delphine CC Chiu <Delphine_CC_Chiu@wiwynn.com> > --- > Changelog: > v2 - Add dt-bindings for the MAXIM MAX31790. > --- > .../bindings/hwmon/maxim,max31790.yaml | 59 +++++++++++++++++++ > MAINTAINERS | 6 ++ > 2 files changed, 65 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwmon/maxim,max31790.yaml > > diff --git a/Documentation/devicetree/bindings/hwmon/maxim,max31790.yaml b/Documentation/devicetree/bindings/hwmon/maxim,max31790.yaml > new file mode 100644 > index 000000000000..2bd455b36b3f > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwmon/maxim,max31790.yaml > @@ -0,0 +1,59 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > + > +$id: http://devicetree.org/schemas/hwmon/maxim,max31790.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Maxim max31790 > + > +maintainers: > + - Delphine CC Chiu <Delphine_CC_Chiu@wiwynn.com> > + > +description: | > + The MAX31790 controls the speeds of up to six fans using > + six independent PWM outputs. The desired fan speeds (or PWM duty cycles) > + are written through the I2C interface. > + The outputs drive “4-wire” fans directly, or can be used to modulate > + the fan’s power terminals using an external pass transistor. > + > + Datasheets: > + https://datasheets.maximintegrated.com/en/ds/MAX31790.pdf > + > +properties: > + compatible: > + enum: > + - maxim,max31790 > + > + reg: > + maxItems: 1 > + > + pwm-as-tach: I don't see any other users of this in-tree, so you'd need a vendor prefix. That said, I'm once bitten, twice shy about fan related properties in hwmon, so I would definitely like Rob to comment on this whole binding. > + description: | > + There are 6 PWM output channel in MAX31790 that allows to be configured > + as a TACH input by setting the Fan Configuration register. > + Config PWM output channels in the array as tachometer inputs. > + $ref: /schemas/types.yaml#/definitions/uint32-array > + minItems: 1 > + maxItems: 6 > + items: > + enum: [1, 2, 3, 4, 5, 6] > + > +required: > + - compatible > + - reg > + > +additionalProperties: false > + > +examples: > + - | > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + pwm@20 { > + compatible = "maxim,max31790"; > + reg = <0x20>; > + pwm-as-tach = <2 5>; This would be <2>, <5>; no? > + }; > + }; > diff --git a/MAINTAINERS b/MAINTAINERS > index c8fdd0d03907..97e13b6bf51d 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -1371,6 +1371,12 @@ F: Documentation/devicetree/bindings/hwmon/adi,max31760.yaml > F: Documentation/hwmon/max31760.rst > F: drivers/hwmon/max31760.c > > +ANALOG DEVICES INC MAX31790 DRIVER > +M: Delphine CC Chiu <Delphine_CC_Chiu@wiwynn.com> > +S: Odd Fixes This is a pretty odd status for something you're newly adding. How come it's not going to be maintained? Thanks, Conor.
On Fri, Sep 15, 2023 at 9:50 AM Conor Dooley <conor@kernel.org> wrote: > > Yo, > > On Fri, Sep 15, 2023 at 02:29:24PM +0800, Delphine CC Chiu wrote: > > Add dt-bindings for the MAXIM MAX31790. > > > > Signed-off-by: Delphine CC Chiu <Delphine_CC_Chiu@wiwynn.com> > > --- > > Changelog: > > v2 - Add dt-bindings for the MAXIM MAX31790. > > --- > > .../bindings/hwmon/maxim,max31790.yaml | 59 +++++++++++++++++++ > > MAINTAINERS | 6 ++ > > 2 files changed, 65 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/hwmon/maxim,max31790.yaml > > > > diff --git a/Documentation/devicetree/bindings/hwmon/maxim,max31790.yaml b/Documentation/devicetree/bindings/hwmon/maxim,max31790.yaml > > new file mode 100644 > > index 000000000000..2bd455b36b3f > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/hwmon/maxim,max31790.yaml > > @@ -0,0 +1,59 @@ > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > + > > +$id: http://devicetree.org/schemas/hwmon/maxim,max31790.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Maxim max31790 > > + > > +maintainers: > > + - Delphine CC Chiu <Delphine_CC_Chiu@wiwynn.com> > > + > > +description: | > > + The MAX31790 controls the speeds of up to six fans using > > + six independent PWM outputs. The desired fan speeds (or PWM duty cycles) > > + are written through the I2C interface. > > + The outputs drive “4-wire” fans directly, or can be used to modulate > > + the fan’s power terminals using an external pass transistor. > > + > > + Datasheets: > > + https://datasheets.maximintegrated.com/en/ds/MAX31790.pdf > > + > > +properties: > > + compatible: > > + enum: > > + - maxim,max31790 > > + > > + reg: > > + maxItems: 1 > > + > > + pwm-as-tach: > > I don't see any other users of this in-tree, so you'd need a vendor > prefix. That said, I'm once bitten, twice shy about fan related > properties in hwmon, so I would definitely like Rob to comment on this > whole binding. Please see this[1] and comment on it to ensure it meets your needs. Otherwise, omit any fan related properties for now. Rob [1] https://lore.kernel.org/all/20230830123202.3408318-2-billy_tsai@aspeedtech.com/
> > Yo, > > > > On Fri, Sep 15, 2023 at 02:29:24PM +0800, Delphine CC Chiu wrote: > > > Add dt-bindings for the MAXIM MAX31790. > > > > > > Signed-off-by: Delphine CC Chiu <Delphine_CC_Chiu@wiwynn.com> > > > --- > > > Changelog: > > > v2 - Add dt-bindings for the MAXIM MAX31790. > > > --- > > > .../bindings/hwmon/maxim,max31790.yaml | 59 > +++++++++++++++++++ > > > MAINTAINERS | 6 ++ > > > 2 files changed, 65 insertions(+) > > > create mode 100644 > > > Documentation/devicetree/bindings/hwmon/maxim,max31790.yaml > > > > > > diff --git > > > a/Documentation/devicetree/bindings/hwmon/maxim,max31790.yaml > > > b/Documentation/devicetree/bindings/hwmon/maxim,max31790.yaml > > > new file mode 100644 > > > index 000000000000..2bd455b36b3f > > > --- /dev/null > > > +++ > b/Documentation/devicetree/bindings/hwmon/maxim,max31790.yaml > > > @@ -0,0 +1,59 @@ > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2 > > > +--- > > > + > > > +$id: > > > +http://de/ > > > > +vicetree.org%2Fschemas%2Fhwmon%2Fmaxim%2Cmax31790.yaml%23&dat > a=05%7 > > > > +C01%7CRicky_CX_Wu%40wiwynn.com%7C5cef527f44254a3b08d708dbb5ff > 64b4%7 > > > > +Cda6e0628fc834caf9dd273061cbab167%7C0%7C0%7C63830388164599049 > 9%7CUn > > > > +known%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI > 6Ik1 > > > > +haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=Wp%2Bu9QyFyjv8bh > ZRGoK8gaj > > > +S7hikXbxNoehdOzLBD%2FI%3D&reserved=0 > > > +$schema: > > > +http://de/ > > > > +vicetree.org%2Fmeta-schemas%2Fcore.yaml%23&data=05%7C01%7CRicky_ > CX_ > > > > +Wu%40wiwynn.com%7C5cef527f44254a3b08d708dbb5ff64b4%7Cda6e0628 > fc834c > > > > +af9dd273061cbab167%7C0%7C0%7C638303881645990499%7CUnknown%7 > CTWFpbGZ > > > > +sb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6 > Mn > > > > +0%3D%7C3000%7C%7C%7C&sdata=uyqp2bRI%2BTWNwiloKf76R6TiL61OiW > 2aqxgZkN > > > +%2B78mg%3D&reserved=0 > > > + > > > +title: Maxim max31790 > > > + > > > +maintainers: > > > + - Delphine CC Chiu <Delphine_CC_Chiu@wiwynn.com> > > > + > > > +description: | > > > + The MAX31790 controls the speeds of up to six fans using > > > + six independent PWM outputs. The desired fan speeds (or PWM duty > cycles) > > > + are written through the I2C interface. > > > + The outputs drive “4-wire” fans directly, or can be used to > > > +modulate > > > + the fan’s power terminals using an external pass transistor. > > > + > > > + Datasheets: > > > + > > > + https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2F > > > + > datasheets.maximintegrated.com%2Fen%2Fds%2FMAX31790.pdf&data=05% > 7C > > > + > 01%7CRicky_CX_Wu%40wiwynn.com%7C5cef527f44254a3b08d708dbb5ff64 > b4%7 > > > + > Cda6e0628fc834caf9dd273061cbab167%7C0%7C0%7C638303881645990499 > %7CU > > > + > nknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI > 6I > > > + > k1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=KrmGiJ2VFoJE5%2 > B%2FeyW > > > + hrCqIXbcSMZk5ToCiiUHUQCRs%3D&reserved=0 > > > + > > > +properties: > > > + compatible: > > > + enum: > > > + - maxim,max31790 > > > + > > > + reg: > > > + maxItems: 1 > > > + > > > + pwm-as-tach: > > > > I don't see any other users of this in-tree, so you'd need a vendor > > prefix. That said, I'm once bitten, twice shy about fan related > > properties in hwmon, so I would definitely like Rob to comment on this > > whole binding. > > Please see this[1] and comment on it to ensure it meets your needs. > Otherwise, omit any fan related properties for now. > This property could only be used in max31790 driver. Would it be ok if we add vendor prefix like "maxim, pwm-as-tach"? > Rob > > [1] > https://lore.ker/ > nel.org%2Fall%2F20230830123202.3408318-2-billy_tsai%40aspeedtech.com > %2F&data=05%7C01%7CRicky_CX_Wu%40wiwynn.com%7C5cef527f44254a3 > b08d708dbb5ff64b4%7Cda6e0628fc834caf9dd273061cbab167%7C0%7C0%7 > C638303881645990499%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw > MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C > %7C&sdata=1dre05mnoY9Y2%2FdQ2%2B2nVq6wRufembfxEHAMg1BXsMc%3 > D&reserved=0 > > +examples: > > + - | > > + i2c { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + pwm@20 { > > + compatible = "maxim,max31790"; > > + reg = <0x20>; > > + pwm-as-tach = <2 5>; > > This would be <2>, <5>; no? > I refer to the other binding documents in hwmon and most of them were using the format like <2 5> as an array. > > diff --git a/MAINTAINERS b/MAINTAINERS index > > c8fdd0d03907..97e13b6bf51d 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -1371,6 +1371,12 @@ F: > Documentation/devicetree/bindings/hwmon/adi,max31760.yaml > > F: Documentation/hwmon/max31760.rst > > F: drivers/hwmon/max31760.c > > > > +ANALOG DEVICES INC MAX31790 DRIVER > > +M: Delphine CC Chiu <Delphine_CC_Chiu@wiwynn.com> > > +S: Odd Fixes > > This is a pretty odd status for something you're newly adding. > How come it's not going to be maintained? > We are not the authors of this driver but we want to add a feature to config PWM as TACH that was descripted in the datasheet of MAX31790. Should we set the status to maintained?
On Fri, Sep 22, 2023 at 02:33:06AM +0000, Delphine_CC_Chiu/WYHQ/Wiwynn wrote: > > > On Fri, Sep 15, 2023 at 02:29:24PM +0800, Delphine CC Chiu wrote: > > > > + pwm-as-tach: > > > > > > I don't see any other users of this in-tree, so you'd need a vendor > > > prefix. That said, I'm once bitten, twice shy about fan related > > > properties in hwmon, so I would definitely like Rob to comment on this > > > whole binding. > > > > Please see this[1] and comment on it to ensure it meets your needs. > > Otherwise, omit any fan related properties for now. > > > This property could only be used in max31790 driver. Would it be ok if we add > vendor prefix like "maxim, pwm-as-tach"? I think the answer to this is a pretty straightforward no. The goal is to create a set of common fan properties that works for multiple usecases, not create one specifically for each user... > > > +examples: > > > + - | > > > + i2c { > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + > > > + pwm@20 { > > > + compatible = "maxim,max31790"; > > > + reg = <0x20>; > > > + pwm-as-tach = <2 5>; > > > > This would be <2>, <5>; no? > > > I refer to the other binding documents in hwmon and most of them were using > the format like <2 5> as an array. Which also makes this moot, since it'll be going away. > > > diff --git a/MAINTAINERS b/MAINTAINERS index > > > c8fdd0d03907..97e13b6bf51d 100644 > > > --- a/MAINTAINERS > > > +++ b/MAINTAINERS > > > @@ -1371,6 +1371,12 @@ F: > > Documentation/devicetree/bindings/hwmon/adi,max31760.yaml > > > F: Documentation/hwmon/max31760.rst > > > F: drivers/hwmon/max31760.c > > > > > > +ANALOG DEVICES INC MAX31790 DRIVER > > > +M: Delphine CC Chiu <Delphine_CC_Chiu@wiwynn.com> > > > +S: Odd Fixes > > > > This is a pretty odd status for something you're newly adding. > > How come it's not going to be maintained? > > > We are not the authors of this driver but we want to add a feature to > config PWM as TACH that was descripted in the datasheet of MAX31790. > Should we set the status to maintained? It's really up to you. I just found it curious & wanted to ask why it was that way. Thanks, Conor.
On 9/22/23 02:53, Conor Dooley wrote: > On Fri, Sep 22, 2023 at 02:33:06AM +0000, Delphine_CC_Chiu/WYHQ/Wiwynn wrote: >>>> On Fri, Sep 15, 2023 at 02:29:24PM +0800, Delphine CC Chiu wrote: > >>>>> + pwm-as-tach: >>>> >>>> I don't see any other users of this in-tree, so you'd need a vendor >>>> prefix. That said, I'm once bitten, twice shy about fan related >>>> properties in hwmon, so I would definitely like Rob to comment on this >>>> whole binding. >>> >>> Please see this[1] and comment on it to ensure it meets your needs. >>> Otherwise, omit any fan related properties for now. >>> >> This property could only be used in max31790 driver. Would it be ok if we add >> vendor prefix like "maxim, pwm-as-tach"? > > I think the answer to this is a pretty straightforward no. The goal is > to create a set of common fan properties that works for multiple > usecases, not create one specifically for each user... > Another chip with configurable channel configuration is nct7802, where individual channels can be configured as temperature or voltage sensor. We are using sensor-type to select the mode in that driver. Maybe something similar would make sense / be acceptable here. >>>> +examples: >>>> + - | >>>> + i2c { >>>> + #address-cells = <1>; >>>> + #size-cells = <0>; >>>> + >>>> + pwm@20 { >>>> + compatible = "maxim,max31790"; >>>> + reg = <0x20>; >>>> + pwm-as-tach = <2 5>; >>> >>> This would be <2>, <5>; no? >>> >> I refer to the other binding documents in hwmon and most of them were using >> the format like <2 5> as an array. > > Which also makes this moot, since it'll be going away. > >>>> diff --git a/MAINTAINERS b/MAINTAINERS index >>>> c8fdd0d03907..97e13b6bf51d 100644 >>>> --- a/MAINTAINERS >>>> +++ b/MAINTAINERS >>>> @@ -1371,6 +1371,12 @@ F: >>> Documentation/devicetree/bindings/hwmon/adi,max31760.yaml >>>> F: Documentation/hwmon/max31760.rst >>>> F: drivers/hwmon/max31760.c >>>> >>>> +ANALOG DEVICES INC MAX31790 DRIVER >>>> +M: Delphine CC Chiu <Delphine_CC_Chiu@wiwynn.com> >>>> +S: Odd Fixes >>> >>> This is a pretty odd status for something you're newly adding. >>> How come it's not going to be maintained? >>> >> We are not the authors of this driver but we want to add a feature to >> config PWM as TACH that was descripted in the datasheet of MAX31790. >> Should we set the status to maintained? > > It's really up to you. I just found it curious & wanted to ask why it > was that way. > It is misleading because it downgrades the driver from "supported" (like all other hwmon drivers) to "odd fixes". Guenter
diff --git a/Documentation/devicetree/bindings/hwmon/maxim,max31790.yaml b/Documentation/devicetree/bindings/hwmon/maxim,max31790.yaml new file mode 100644 index 000000000000..2bd455b36b3f --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/maxim,max31790.yaml @@ -0,0 +1,59 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- + +$id: http://devicetree.org/schemas/hwmon/maxim,max31790.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Maxim max31790 + +maintainers: + - Delphine CC Chiu <Delphine_CC_Chiu@wiwynn.com> + +description: | + The MAX31790 controls the speeds of up to six fans using + six independent PWM outputs. The desired fan speeds (or PWM duty cycles) + are written through the I2C interface. + The outputs drive “4-wire” fans directly, or can be used to modulate + the fan’s power terminals using an external pass transistor. + + Datasheets: + https://datasheets.maximintegrated.com/en/ds/MAX31790.pdf + +properties: + compatible: + enum: + - maxim,max31790 + + reg: + maxItems: 1 + + pwm-as-tach: + description: | + There are 6 PWM output channel in MAX31790 that allows to be configured + as a TACH input by setting the Fan Configuration register. + Config PWM output channels in the array as tachometer inputs. + $ref: /schemas/types.yaml#/definitions/uint32-array + minItems: 1 + maxItems: 6 + items: + enum: [1, 2, 3, 4, 5, 6] + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + pwm@20 { + compatible = "maxim,max31790"; + reg = <0x20>; + pwm-as-tach = <2 5>; + }; + }; diff --git a/MAINTAINERS b/MAINTAINERS index c8fdd0d03907..97e13b6bf51d 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1371,6 +1371,12 @@ F: Documentation/devicetree/bindings/hwmon/adi,max31760.yaml F: Documentation/hwmon/max31760.rst F: drivers/hwmon/max31760.c +ANALOG DEVICES INC MAX31790 DRIVER +M: Delphine CC Chiu <Delphine_CC_Chiu@wiwynn.com> +S: Odd Fixes +F: Documentation/devicetree/bindings/hwmon/maxim,max31790.yaml +F: drivers/hwmon/max31790.c + ANALOGBITS PLL LIBRARIES M: Paul Walmsley <paul.walmsley@sifive.com> S: Supported
Add dt-bindings for the MAXIM MAX31790. Signed-off-by: Delphine CC Chiu <Delphine_CC_Chiu@wiwynn.com> --- Changelog: v2 - Add dt-bindings for the MAXIM MAX31790. --- .../bindings/hwmon/maxim,max31790.yaml | 59 +++++++++++++++++++ MAINTAINERS | 6 ++ 2 files changed, 65 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/maxim,max31790.yaml