From: Francesco Dolcini <francesco.dolcini@toradex.com>
Add property to describe the PWM-Out pin polarity.
Link: https://www.ti.com/lit/gpn/amc6821
Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
v2: no changes
v1: https://lore.kernel.org/all/20250218165633.106867-2-francesco@dolcini.it/
---
Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml b/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
index 5d33f1a23d03..11604aa41b3e 100644
--- a/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
+++ b/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
@@ -28,6 +28,14 @@ properties:
i2c-mux:
type: object
+ ti,pwm-inverted:
+ type: boolean
+ description:
+ Set to make the PWM-Out pin go high (with an external pull-up resistor)
+ for 100% duty cycle (suitable for driving the fan using a NMOS device),
+ when not set the PWM-Out pin goes low for 100% duty cycle (suitable for
+ driving the fan using a PMOS device).
+
required:
- compatible
- reg
--
2.39.5On Mon, Feb 24, 2025 at 07:08:00PM +0100, Francesco Dolcini wrote: > From: Francesco Dolcini <francesco.dolcini@toradex.com> > > Add property to describe the PWM-Out pin polarity. Why doesn't the invert support in the pwm binding work for you? Yes, I read the discussion, but don't remember the conclusion and you need to justify it here. > > Link: https://www.ti.com/lit/gpn/amc6821 > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com> > --- > v2: no changes > v1: https://lore.kernel.org/all/20250218165633.106867-2-francesco@dolcini.it/ > --- > Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml b/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml > index 5d33f1a23d03..11604aa41b3e 100644 > --- a/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml > +++ b/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml > @@ -28,6 +28,14 @@ properties: > i2c-mux: > type: object > > + ti,pwm-inverted: > + type: boolean > + description: > + Set to make the PWM-Out pin go high (with an external pull-up resistor) > + for 100% duty cycle (suitable for driving the fan using a NMOS device), > + when not set the PWM-Out pin goes low for 100% duty cycle (suitable for > + driving the fan using a PMOS device). > + > required: > - compatible > - reg > -- > 2.39.5 >
On Wed, Feb 26, 2025 at 07:49:22AM -0600, Rob Herring wrote:
> On Mon, Feb 24, 2025 at 07:08:00PM +0100, Francesco Dolcini wrote:
> > From: Francesco Dolcini <francesco.dolcini@toradex.com>
> >
> > Add property to describe the PWM-Out pin polarity.
>
> Why doesn't the invert support in the pwm binding work for you? Yes, I
> read the discussion, but don't remember the conclusion and you need to
> justify it here.
This chip is not a PWM controller, it is a FAN controller.
The HW has a PWM pin output that is used to control the fan, but the
device is not modelled as a PWM controller (correctly, given that is not
such a device) and the OS does not control the PWM, the chip reads the
temperature and decide the PWM duty cycle accordingly in an autonomous
way.
It's just the same that was done in commit ed39ff506adb ("dt-bindings:
hwmon: Document adt7475 pwm-active-state property").
Francesco
Hello Rob and all, On Wed, Feb 26, 2025 at 02:58:06PM +0100, Francesco Dolcini wrote: > On Wed, Feb 26, 2025 at 07:49:22AM -0600, Rob Herring wrote: > > On Mon, Feb 24, 2025 at 07:08:00PM +0100, Francesco Dolcini wrote: > > > From: Francesco Dolcini <francesco.dolcini@toradex.com> > > > > > > Add property to describe the PWM-Out pin polarity. > > > > Why doesn't the invert support in the pwm binding work for you? Yes, I > > read the discussion, but don't remember the conclusion and you need to > > justify it here. > > This chip is not a PWM controller, it is a FAN controller. > > The HW has a PWM pin output that is used to control the fan, but the > device is not modelled as a PWM controller (correctly, given that is not > such a device) and the OS does not control the PWM, the chip reads the > temperature and decide the PWM duty cycle accordingly in an autonomous > way. Can you advise on how to move this forward? Is my explanation good enough or some more clarification is needed? Should I send a v3 incorporating such a comment into the commit message? Anything else? Thanks, Francesco
On 3/19/25 03:12, Francesco Dolcini wrote:
> Hello Rob and all,
>
> On Wed, Feb 26, 2025 at 02:58:06PM +0100, Francesco Dolcini wrote:
>> On Wed, Feb 26, 2025 at 07:49:22AM -0600, Rob Herring wrote:
>>> On Mon, Feb 24, 2025 at 07:08:00PM +0100, Francesco Dolcini wrote:
>>>> From: Francesco Dolcini <francesco.dolcini@toradex.com>
>>>>
>>>> Add property to describe the PWM-Out pin polarity.
>>>
>>> Why doesn't the invert support in the pwm binding work for you? Yes, I
>>> read the discussion, but don't remember the conclusion and you need to
>>> justify it here.
>>
>> This chip is not a PWM controller, it is a FAN controller.
>>
>> The HW has a PWM pin output that is used to control the fan, but the
>> device is not modelled as a PWM controller (correctly, given that is not
>> such a device) and the OS does not control the PWM, the chip reads the
>> temperature and decide the PWM duty cycle accordingly in an autonomous
>> way.
>
> Can you advise on how to move this forward? Is my explanation good
> enough or some more clarification is needed? Should I send a v3
> incorporating such a comment into the commit message? Anything else?
>
DT maintainers insist that pwm properties are described using pwm cells.
That does not mean that the driver has to implement a pwm controller
(and I would resist doing that, because it is pointless), just that
the chip's pwm properties are described this way. That is indeed a deviation
from older devicetree files, where "inverted" properties were acceptable.
I don't know if there is a means to avoid that. Some devicetree files
don't mention pwm in the property name. See nxp,inverted-out or
atmel,lcdcon-backlight-inverted for examples. I suspect that is no longer
acceptable, though. The easiest would probably be to define optional
minimal pwm bindings for the chip. Unless I am missing something, that
would just be the pwm polarity, so you would have a single pwm cell.
Something like
'#pwm-cells':
const: 1
description: |
Number of cells in a PWM specifier.
- cell 0: The PWM polarity: 0 or PWM_POLARITY_INVERTED
and then something like
fan_controller: fan@18 {
compatible = "ti,amc6821";
reg = <0x18>;
#pwm-cells = <1>;
pwms = <&fan_controller PWM_POLARITY_INVERTED>;
};
That may require some tweaking though; I think #pwm-cells may apply to the
children of a property, not to the property itself.
Guenter
© 2016 - 2025 Red Hat, Inc.