From: Marcello Sylvester Bauer <[email protected]>
Add binding documentation for Maxim MAX6639 fan-speed controller.
Signed-off-by: Marcello Sylvester Bauer <[email protected]>
Signed-off-by: Naresh Solanki <[email protected]>
---
Changes in V3:
- Update title
- Add pulses-per-revolution, supplies & interrupts
Changes in V2:
- Update subject
- Drop blank lines
---
.../bindings/hwmon/maxim,max6639.yaml | 60 +++++++++++++++++++
1 file changed, 60 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
diff --git a/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
new file mode 100644
index 000000000000..b3292061ca58
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
@@ -0,0 +1,60 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hwmon/maxim,max6639.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Maxim MAX6639 Fan Controller
+
+maintainers:
+ - Naresh Solanki <[email protected]>
+
+description: |
+ The MAX6639 is a 2-channel temperature monitor with dual, automatic, PWM
+ fan-speed controller. It monitors its own temperature and one external
+ diode-connected transistor or the temperatures of two external diode-connected
+ transistors, typically available in CPUs, FPGAs, or GPUs.
+
+ Datasheets:
+ https://datasheets.maximintegrated.com/en/ds/MAX6639-MAX6639F.pdf
+
+properties:
+ compatible:
+ enum:
+ - maxim,max6639
+
+ reg:
+ maxItems: 1
+
+ fan-supply:
+ description: Phandle to the regulator that provides power to the fan.
+
+ interrupts:
+ maxItems: 1
+
+ pulses-per-revolution:
+ description:
+ Define the number of pulses per fan revolution for each tachometer
+ input as an integer.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [1, 2, 3, 4]
+ default: 2
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ fan-controller@10 {
+ compatible = "maxim,max6639";
+ reg = <0x10>;
+ };
+ };
+...
base-commit: cb7022b8976e3c4d12cea2e7bb820a2944e2fd7b
--
2.41.0
Use maxim,max6639 as compatible string for the driver.
Signed-off-by: Naresh Solanki <[email protected]>
---
Changes in V3:
- None
Changes in V2:
- None, Updated DT patch
---
drivers/hwmon/max6639.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/hwmon/max6639.c b/drivers/hwmon/max6639.c
index caf527154fca..aa7f21ab2395 100644
--- a/drivers/hwmon/max6639.c
+++ b/drivers/hwmon/max6639.c
@@ -618,11 +618,17 @@ MODULE_DEVICE_TABLE(i2c, max6639_id);
static DEFINE_SIMPLE_DEV_PM_OPS(max6639_pm_ops, max6639_suspend, max6639_resume);
+static const struct of_device_id max6639_of_match[] = {
+ { .compatible = "maxim,max6639", },
+ { },
+};
+
static struct i2c_driver max6639_driver = {
.class = I2C_CLASS_HWMON,
.driver = {
.name = "max6639",
.pm = pm_sleep_ptr(&max6639_pm_ops),
+ .of_match_table = max6639_of_match,
},
.probe = max6639_probe,
.id_table = max6639_id,
--
2.41.0
On Thu, Aug 03, 2023 at 04:43:59PM +0200, Naresh Solanki wrote:
> From: Marcello Sylvester Bauer <[email protected]>
>
> Add binding documentation for Maxim MAX6639 fan-speed controller.
>
> Signed-off-by: Marcello Sylvester Bauer <[email protected]>
> Signed-off-by: Naresh Solanki <[email protected]>
> ---
> Changes in V3:
> - Update title
> - Add pulses-per-revolution, supplies & interrupts
> Changes in V2:
> - Update subject
> - Drop blank lines
> ---
> .../bindings/hwmon/maxim,max6639.yaml | 60 +++++++++++++++++++
> 1 file changed, 60 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
>
> diff --git a/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
> new file mode 100644
> index 000000000000..b3292061ca58
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
> @@ -0,0 +1,60 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/maxim,max6639.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Maxim MAX6639 Fan Controller
> +
> +maintainers:
> + - Naresh Solanki <[email protected]>
> +
> +description: |
> + The MAX6639 is a 2-channel temperature monitor with dual, automatic, PWM
> + fan-speed controller. It monitors its own temperature and one external
> + diode-connected transistor or the temperatures of two external diode-connected
> + transistors, typically available in CPUs, FPGAs, or GPUs.
> + fan-supply:
> + description: Phandle to the regulator that provides power to the fan.
> + pulses-per-revolution:
> + description:
> + Define the number of pulses per fan revolution for each tachometer
> + input as an integer.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [1, 2, 3, 4]
> + default: 2
Apologies if I am digging up old wounds here, since there was quite a
bit of back and forth on the last version, but these two newly added
properties look to be common with the "pwm-fan" and with
"adi,axi-fan-control". At what point should these live in a common
schema instead?
Otherwise, this looks okay to me, although I'll leave things to
Krzysztof since he had a lot to say about the previous version.
Thanks,
Conor.
On 8/4/23 08:48, Conor Dooley wrote:
> On Thu, Aug 03, 2023 at 04:43:59PM +0200, Naresh Solanki wrote:
>> From: Marcello Sylvester Bauer <[email protected]>
>>
>> Add binding documentation for Maxim MAX6639 fan-speed controller.
>>
>> Signed-off-by: Marcello Sylvester Bauer <[email protected]>
>> Signed-off-by: Naresh Solanki <[email protected]>
>> ---
>> Changes in V3:
>> - Update title
>> - Add pulses-per-revolution, supplies & interrupts
>> Changes in V2:
>> - Update subject
>> - Drop blank lines
>> ---
>> .../bindings/hwmon/maxim,max6639.yaml | 60 +++++++++++++++++++
>> 1 file changed, 60 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
>> new file mode 100644
>> index 000000000000..b3292061ca58
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
>> @@ -0,0 +1,60 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/hwmon/maxim,max6639.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Maxim MAX6639 Fan Controller
>> +
>> +maintainers:
>> + - Naresh Solanki <[email protected]>
>> +
>> +description: |
>> + The MAX6639 is a 2-channel temperature monitor with dual, automatic, PWM
>> + fan-speed controller. It monitors its own temperature and one external
>> + diode-connected transistor or the temperatures of two external diode-connected
>> + transistors, typically available in CPUs, FPGAs, or GPUs.
>
>> + fan-supply:
>> + description: Phandle to the regulator that provides power to the fan.
>
>> + pulses-per-revolution:
>> + description:
>> + Define the number of pulses per fan revolution for each tachometer
>> + input as an integer.
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + enum: [1, 2, 3, 4]
>> + default: 2
>
> Apologies if I am digging up old wounds here, since there was quite a
> bit of back and forth on the last version, but these two newly added
> properties look to be common with the "pwm-fan" and with
> "adi,axi-fan-control". At what point should these live in a common
> schema instead?
>
> Otherwise, this looks okay to me, although I'll leave things to
> Krzysztof since he had a lot to say about the previous version.
>
Rob has said that he won't accept any fan controller bindings without a generic
schema. At the same time he has said that he expects properties such as the
number of pulses per revolution to be attached to a 'fan' description, and he
wants pwm related properties of fan controllers to be modeled as pwm controllers.
And now we have a notion of a regulator providing power to the fan (which again
would be the fan controller, at least in cases where the fan controller
provides direct voltage to the fan). On top of that, this fan-supply property
should presumably, again, be part of a fan description and not be part of the
controller description. I don't think anyone knows how to make this all work
(I for sure don't), so it is very unlikely we'll see a generic fan controller
schema anytime soon.
Given that neither fan-supply nor pulses-per-revolution is implemented in the
driver, and given that I am not aware of any fans which would have a value for
pulses-per-revolution other than 2, my personal suggestion would be to add the
chip to trivial devices and be done with it for the time being.
Guenter
On 03/08/2023 16:43, Naresh Solanki wrote:
> From: Marcello Sylvester Bauer <[email protected]>
>
> Add binding documentation for Maxim MAX6639 fan-speed controller.
>
> Signed-off-by: Marcello Sylvester Bauer <[email protected]>
> Signed-off-by: Naresh Solanki <[email protected]>
> ---
...
> +examples:
> + - |
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + fan-controller@10 {
> + compatible = "maxim,max6639";
> + reg = <0x10>;
I wished the examples were a bit more complete (not only 40% of your
properties). Well, that's not a stopper, so:
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Best regards,
Krzysztof
On 04/08/2023 18:10, Guenter Roeck wrote:
> On 8/4/23 08:48, Conor Dooley wrote:
>> On Thu, Aug 03, 2023 at 04:43:59PM +0200, Naresh Solanki wrote:
>>> From: Marcello Sylvester Bauer <[email protected]>
>>>
>>> Add binding documentation for Maxim MAX6639 fan-speed controller.
>>>
>>> Signed-off-by: Marcello Sylvester Bauer <[email protected]>
>>> Signed-off-by: Naresh Solanki <[email protected]>
>>> ---
>>> Changes in V3:
>>> - Update title
>>> - Add pulses-per-revolution, supplies & interrupts
>>> Changes in V2:
>>> - Update subject
>>> - Drop blank lines
>>> ---
>>> .../bindings/hwmon/maxim,max6639.yaml | 60 +++++++++++++++++++
>>> 1 file changed, 60 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
>>> new file mode 100644
>>> index 000000000000..b3292061ca58
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
>>> @@ -0,0 +1,60 @@
>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/hwmon/maxim,max6639.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Maxim MAX6639 Fan Controller
>>> +
>>> +maintainers:
>>> + - Naresh Solanki <[email protected]>
>>> +
>>> +description: |
>>> + The MAX6639 is a 2-channel temperature monitor with dual, automatic, PWM
>>> + fan-speed controller. It monitors its own temperature and one external
>>> + diode-connected transistor or the temperatures of two external diode-connected
>>> + transistors, typically available in CPUs, FPGAs, or GPUs.
>>
>>> + fan-supply:
>>> + description: Phandle to the regulator that provides power to the fan.
>>
>>> + pulses-per-revolution:
>>> + description:
>>> + Define the number of pulses per fan revolution for each tachometer
>>> + input as an integer.
>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>> + enum: [1, 2, 3, 4]
>>> + default: 2
>>
>> Apologies if I am digging up old wounds here, since there was quite a
>> bit of back and forth on the last version, but these two newly added
>> properties look to be common with the "pwm-fan" and with
>> "adi,axi-fan-control". At what point should these live in a common
>> schema instead?
>>
>> Otherwise, this looks okay to me, although I'll leave things to
>> Krzysztof since he had a lot to say about the previous version.
>>
>
> Rob has said that he won't accept any fan controller bindings without a generic
> schema. At the same time he has said that he expects properties such as the
> number of pulses per revolution to be attached to a 'fan' description, and he
> wants pwm related properties of fan controllers to be modeled as pwm controllers.
> And now we have a notion of a regulator providing power to the fan (which again
> would be the fan controller, at least in cases where the fan controller
> provides direct voltage to the fan). On top of that, this fan-supply property
> should presumably, again, be part of a fan description and not be part of the
> controller description. I don't think anyone knows how to make this all work
> (I for sure don't), so it is very unlikely we'll see a generic fan controller
> schema anytime soon.
>
> Given that neither fan-supply nor pulses-per-revolution is implemented in the
> driver, and given that I am not aware of any fans which would have a value for
> pulses-per-revolution other than 2, my personal suggestion would be to add the
> chip to trivial devices and be done with it for the time being.
Yeah, this also would work.
Best regards,
Krzysztof
On Thu, Aug 03, 2023 at 04:43:59PM +0200, Naresh Solanki wrote:
> From: Marcello Sylvester Bauer <[email protected]>
>
> Add binding documentation for Maxim MAX6639 fan-speed controller.
>
> Signed-off-by: Marcello Sylvester Bauer <[email protected]>
> Signed-off-by: Naresh Solanki <[email protected]>
> Reviewed-by: Krzysztof Kozlowski <[email protected]>
Applied to hwmon-next.
Thanks,
Guenter
> ---
> Changes in V3:
> - Update title
> - Add pulses-per-revolution, supplies & interrupts
> Changes in V2:
> - Update subject
> - Drop blank lines
> ---
> .../bindings/hwmon/maxim,max6639.yaml | 60 +++++++++++++++++++
> 1 file changed, 60 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
>
>
> base-commit: cb7022b8976e3c4d12cea2e7bb820a2944e2fd7b
>
> diff --git a/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
> new file mode 100644
> index 000000000000..b3292061ca58
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
> @@ -0,0 +1,60 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/maxim,max6639.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Maxim MAX6639 Fan Controller
> +
> +maintainers:
> + - Naresh Solanki <[email protected]>
> +
> +description: |
> + The MAX6639 is a 2-channel temperature monitor with dual, automatic, PWM
> + fan-speed controller. It monitors its own temperature and one external
> + diode-connected transistor or the temperatures of two external diode-connected
> + transistors, typically available in CPUs, FPGAs, or GPUs.
> +
> + Datasheets:
> + https://datasheets.maximintegrated.com/en/ds/MAX6639-MAX6639F.pdf
> +
> +properties:
> + compatible:
> + enum:
> + - maxim,max6639
> +
> + reg:
> + maxItems: 1
> +
> + fan-supply:
> + description: Phandle to the regulator that provides power to the fan.
> +
> + interrupts:
> + maxItems: 1
> +
> + pulses-per-revolution:
> + description:
> + Define the number of pulses per fan revolution for each tachometer
> + input as an integer.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [1, 2, 3, 4]
> + default: 2
> +
> +required:
> + - compatible
> + - reg
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + fan-controller@10 {
> + compatible = "maxim,max6639";
> + reg = <0x10>;
> + };
> + };
> +...
On Thu, Aug 03, 2023 at 04:44:00PM +0200, Naresh Solanki wrote:
> Use maxim,max6639 as compatible string for the driver.
>
> Signed-off-by: Naresh Solanki <[email protected]>
Applied to hwmon-next, but please consider updating your e-mail
addresses to either all say [email protected] or
[email protected] to avoid nuisance checkpatch
warnings in the future.
Thanks,
Guenter
> ---
> Changes in V3:
> - None
> Changes in V2:
> - None, Updated DT patch
> ---
> drivers/hwmon/max6639.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/hwmon/max6639.c b/drivers/hwmon/max6639.c
> index caf527154fca..aa7f21ab2395 100644
> --- a/drivers/hwmon/max6639.c
> +++ b/drivers/hwmon/max6639.c
> @@ -618,11 +618,17 @@ MODULE_DEVICE_TABLE(i2c, max6639_id);
>
> static DEFINE_SIMPLE_DEV_PM_OPS(max6639_pm_ops, max6639_suspend, max6639_resume);
>
> +static const struct of_device_id max6639_of_match[] = {
> + { .compatible = "maxim,max6639", },
> + { },
> +};
> +
> static struct i2c_driver max6639_driver = {
> .class = I2C_CLASS_HWMON,
> .driver = {
> .name = "max6639",
> .pm = pm_sleep_ptr(&max6639_pm_ops),
> + .of_match_table = max6639_of_match,
> },
> .probe = max6639_probe,
> .id_table = max6639_id,
On Fri, Aug 04, 2023 at 09:10:37AM -0700, Guenter Roeck wrote:
> On 8/4/23 08:48, Conor Dooley wrote:
> > On Thu, Aug 03, 2023 at 04:43:59PM +0200, Naresh Solanki wrote:
> > > From: Marcello Sylvester Bauer <[email protected]>
> > >
> > > Add binding documentation for Maxim MAX6639 fan-speed controller.
> > >
> > > Signed-off-by: Marcello Sylvester Bauer <[email protected]>
> > > Signed-off-by: Naresh Solanki <[email protected]>
> > > ---
> > > Changes in V3:
> > > - Update title
> > > - Add pulses-per-revolution, supplies & interrupts
> > > Changes in V2:
> > > - Update subject
> > > - Drop blank lines
> > > ---
> > > .../bindings/hwmon/maxim,max6639.yaml | 60 +++++++++++++++++++
> > > 1 file changed, 60 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
> > > new file mode 100644
> > > index 000000000000..b3292061ca58
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
> > > @@ -0,0 +1,60 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/hwmon/maxim,max6639.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Maxim MAX6639 Fan Controller
> > > +
> > > +maintainers:
> > > + - Naresh Solanki <[email protected]>
> > > +
> > > +description: |
> > > + The MAX6639 is a 2-channel temperature monitor with dual, automatic, PWM
> > > + fan-speed controller. It monitors its own temperature and one external
> > > + diode-connected transistor or the temperatures of two external diode-connected
> > > + transistors, typically available in CPUs, FPGAs, or GPUs.
> >
> > > + fan-supply:
> > > + description: Phandle to the regulator that provides power to the fan.
> >
> > > + pulses-per-revolution:
> > > + description:
> > > + Define the number of pulses per fan revolution for each tachometer
> > > + input as an integer.
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > + enum: [1, 2, 3, 4]
> > > + default: 2
> >
> > Apologies if I am digging up old wounds here, since there was quite a
> > bit of back and forth on the last version, but these two newly added
> > properties look to be common with the "pwm-fan" and with
> > "adi,axi-fan-control". At what point should these live in a common
> > schema instead?
> >
> > Otherwise, this looks okay to me, although I'll leave things to
> > Krzysztof since he had a lot to say about the previous version.
> >
>
> Rob has said that he won't accept any fan controller bindings without a generic
> schema. At the same time he has said that he expects properties such as the
> number of pulses per revolution to be attached to a 'fan' description, and he
> wants pwm related properties of fan controllers to be modeled as pwm controllers.
> And now we have a notion of a regulator providing power to the fan (which again
> would be the fan controller, at least in cases where the fan controller
> provides direct voltage to the fan). On top of that, this fan-supply property
> should presumably, again, be part of a fan description and not be part of the
> controller description. I don't think anyone knows how to make this all work
> (I for sure don't), so it is very unlikely we'll see a generic fan controller
> schema anytime soon.
I thought what was done earlier in this series was somewhat close. And
there are some bindings that already look pretty close to what a common
binding should. But it seems no one wants to worry about more than their
1 device.
In case it's not clear, as-is, this binding is a NAK for me.
> Given that neither fan-supply nor pulses-per-revolution is implemented in the
> driver, and given that I am not aware of any fans which would have a value for
> pulses-per-revolution other than 2, my personal suggestion would be to add the
> chip to trivial devices and be done with it for the time being.
I'm fine with that too. Just keep kicking that can...
Rob
On 8/10/23 16:11, Rob Herring wrote:
> On Fri, Aug 04, 2023 at 09:10:37AM -0700, Guenter Roeck wrote:
>> On 8/4/23 08:48, Conor Dooley wrote:
>>> On Thu, Aug 03, 2023 at 04:43:59PM +0200, Naresh Solanki wrote:
>>>> From: Marcello Sylvester Bauer <[email protected]>
>>>>
>>>> Add binding documentation for Maxim MAX6639 fan-speed controller.
>>>>
>>>> Signed-off-by: Marcello Sylvester Bauer <[email protected]>
>>>> Signed-off-by: Naresh Solanki <[email protected]>
>>>> ---
>>>> Changes in V3:
>>>> - Update title
>>>> - Add pulses-per-revolution, supplies & interrupts
>>>> Changes in V2:
>>>> - Update subject
>>>> - Drop blank lines
>>>> ---
>>>> .../bindings/hwmon/maxim,max6639.yaml | 60 +++++++++++++++++++
>>>> 1 file changed, 60 insertions(+)
>>>> create mode 100644 Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
>>>> new file mode 100644
>>>> index 000000000000..b3292061ca58
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
>>>> @@ -0,0 +1,60 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/hwmon/maxim,max6639.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Maxim MAX6639 Fan Controller
>>>> +
>>>> +maintainers:
>>>> + - Naresh Solanki <[email protected]>
>>>> +
>>>> +description: |
>>>> + The MAX6639 is a 2-channel temperature monitor with dual, automatic, PWM
>>>> + fan-speed controller. It monitors its own temperature and one external
>>>> + diode-connected transistor or the temperatures of two external diode-connected
>>>> + transistors, typically available in CPUs, FPGAs, or GPUs.
>>>
>>>> + fan-supply:
>>>> + description: Phandle to the regulator that provides power to the fan.
>>>
>>>> + pulses-per-revolution:
>>>> + description:
>>>> + Define the number of pulses per fan revolution for each tachometer
>>>> + input as an integer.
>>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>>> + enum: [1, 2, 3, 4]
>>>> + default: 2
>>>
>>> Apologies if I am digging up old wounds here, since there was quite a
>>> bit of back and forth on the last version, but these two newly added
>>> properties look to be common with the "pwm-fan" and with
>>> "adi,axi-fan-control". At what point should these live in a common
>>> schema instead?
>>>
>>> Otherwise, this looks okay to me, although I'll leave things to
>>> Krzysztof since he had a lot to say about the previous version.
>>>
>>
>> Rob has said that he won't accept any fan controller bindings without a generic
>> schema. At the same time he has said that he expects properties such as the
>> number of pulses per revolution to be attached to a 'fan' description, and he
>> wants pwm related properties of fan controllers to be modeled as pwm controllers.
>> And now we have a notion of a regulator providing power to the fan (which again
>> would be the fan controller, at least in cases where the fan controller
>> provides direct voltage to the fan). On top of that, this fan-supply property
>> should presumably, again, be part of a fan description and not be part of the
>> controller description. I don't think anyone knows how to make this all work
>> (I for sure don't), so it is very unlikely we'll see a generic fan controller
>> schema anytime soon.
>
> I thought what was done earlier in this series was somewhat close. And
> there are some bindings that already look pretty close to what a common
> binding should. But it seems no one wants to worry about more than their
> 1 device.
>
> In case it's not clear, as-is, this binding is a NAK for me.
>
Ok, I'll drop it.
Guenter
>> Given that neither fan-supply nor pulses-per-revolution is implemented in the
>> driver, and given that I am not aware of any fans which would have a value for
>> pulses-per-revolution other than 2, my personal suggestion would be to add the
>> chip to trivial devices and be done with it for the time being.
>
> I'm fine with that too. Just keep kicking that can...
>
> Rob
Hi Rob,
On Fri, 11 Aug 2023 at 04:41, Rob Herring <[email protected]> wrote:
>
> On Fri, Aug 04, 2023 at 09:10:37AM -0700, Guenter Roeck wrote:
> > On 8/4/23 08:48, Conor Dooley wrote:
> > > On Thu, Aug 03, 2023 at 04:43:59PM +0200, Naresh Solanki wrote:
> > > > From: Marcello Sylvester Bauer <[email protected]>
> > > >
> > > > Add binding documentation for Maxim MAX6639 fan-speed controller.
> > > >
> > > > Signed-off-by: Marcello Sylvester Bauer <[email protected]>
> > > > Signed-off-by: Naresh Solanki <[email protected]>
> > > > ---
> > > > Changes in V3:
> > > > - Update title
> > > > - Add pulses-per-revolution, supplies & interrupts
> > > > Changes in V2:
> > > > - Update subject
> > > > - Drop blank lines
> > > > ---
> > > > .../bindings/hwmon/maxim,max6639.yaml | 60 +++++++++++++++++++
> > > > 1 file changed, 60 insertions(+)
> > > > create mode 100644 Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
> > > > new file mode 100644
> > > > index 000000000000..b3292061ca58
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
> > > > @@ -0,0 +1,60 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/hwmon/maxim,max6639.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Maxim MAX6639 Fan Controller
> > > > +
> > > > +maintainers:
> > > > + - Naresh Solanki <[email protected]>
> > > > +
> > > > +description: |
> > > > + The MAX6639 is a 2-channel temperature monitor with dual, automatic, PWM
> > > > + fan-speed controller. It monitors its own temperature and one external
> > > > + diode-connected transistor or the temperatures of two external diode-connected
> > > > + transistors, typically available in CPUs, FPGAs, or GPUs.
> > >
> > > > + fan-supply:
> > > > + description: Phandle to the regulator that provides power to the fan.
> > >
> > > > + pulses-per-revolution:
> > > > + description:
> > > > + Define the number of pulses per fan revolution for each tachometer
> > > > + input as an integer.
> > > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > > + enum: [1, 2, 3, 4]
> > > > + default: 2
> > >
> > > Apologies if I am digging up old wounds here, since there was quite a
> > > bit of back and forth on the last version, but these two newly added
> > > properties look to be common with the "pwm-fan" and with
> > > "adi,axi-fan-control". At what point should these live in a common
> > > schema instead?
> > >
> > > Otherwise, this looks okay to me, although I'll leave things to
> > > Krzysztof since he had a lot to say about the previous version.
> > >
> >
> > Rob has said that he won't accept any fan controller bindings without a generic
> > schema. At the same time he has said that he expects properties such as the
> > number of pulses per revolution to be attached to a 'fan' description, and he
> > wants pwm related properties of fan controllers to be modeled as pwm controllers.
> > And now we have a notion of a regulator providing power to the fan (which again
> > would be the fan controller, at least in cases where the fan controller
> > provides direct voltage to the fan). On top of that, this fan-supply property
> > should presumably, again, be part of a fan description and not be part of the
> > controller description. I don't think anyone knows how to make this all work
> > (I for sure don't), so it is very unlikely we'll see a generic fan controller
> > schema anytime soon.
>
> I thought what was done earlier in this series was somewhat close. And
> there are some bindings that already look pretty close to what a common
> binding should. But it seems no one wants to worry about more than their
> 1 device.
The DT binding for common fan properties is:
https://lore.kernel.org/lkml/[email protected]/t/#m15ce07c3c43c46506acc389bf24d616646e05653
I wasn't sure on how to address properties for DC controlled fans.
Regards,
Naresh
>
> In case it's not clear, as-is, this binding is a NAK for me.
>
> > Given that neither fan-supply nor pulses-per-revolution is implemented in the
> > driver, and given that I am not aware of any fans which would have a value for
> > pulses-per-revolution other than 2, my personal suggestion would be to add the
> > chip to trivial devices and be done with it for the time being.
>
> I'm fine with that too. Just keep kicking that can...
>
> Rob