* [PATCH v3 0/2] hwmon: (amc6821) Add PWM polarity configuration with OF
@ 2025-03-31 15:52 Francesco Dolcini
2025-03-31 15:52 ` [PATCH v3 1/2] dt-bindings: hwmon: amc6821: add fan and PWM output Francesco Dolcini
2025-03-31 15:52 ` [PATCH v3 2/2] hwmon: (amc6821) Add PWM polarity configuration with OF Francesco Dolcini
0 siblings, 2 replies; 10+ messages in thread
From: Francesco Dolcini @ 2025-03-31 15:52 UTC (permalink / raw)
To: Jean Delvare, Guenter Roeck, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Farouk Bouabid, Quentin Schulz
Cc: Francesco Dolcini, linux-hwmon, devicetree, linux-kernel
From: Francesco Dolcini <francesco.dolcini@toradex.com>
Add support for configuring the PWM polarity of the amc6821 fan controller.
Francesco Dolcini (2):
dt-bindings: hwmon: amc6821: add fan and PWM output
hwmon: (amc6821) Add PWM polarity configuration with OF
.../devicetree/bindings/hwmon/ti,amc6821.yaml | 14 +++++-
drivers/hwmon/amc6821.c | 50 +++++++++++++++++--
2 files changed, 58 insertions(+), 6 deletions(-)
--
2.39.5
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 1/2] dt-bindings: hwmon: amc6821: add fan and PWM output
2025-03-31 15:52 [PATCH v3 0/2] hwmon: (amc6821) Add PWM polarity configuration with OF Francesco Dolcini
@ 2025-03-31 15:52 ` Francesco Dolcini
2025-03-31 16:01 ` Guenter Roeck
2025-04-01 6:13 ` Krzysztof Kozlowski
2025-03-31 15:52 ` [PATCH v3 2/2] hwmon: (amc6821) Add PWM polarity configuration with OF Francesco Dolcini
1 sibling, 2 replies; 10+ messages in thread
From: Francesco Dolcini @ 2025-03-31 15:52 UTC (permalink / raw)
To: Jean Delvare, Guenter Roeck, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Farouk Bouabid, Quentin Schulz
Cc: Francesco Dolcini, linux-hwmon, devicetree, linux-kernel
From: Francesco Dolcini <francesco.dolcini@toradex.com>
Add properties to describe the fan and the PWM controller output.
Link: https://www.ti.com/lit/gpn/amc6821
Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
v3:
- explicitly describe the fan, use standard PWM and FAN bindings
- pwm.yaml cannot be referenced, because of the $nodename pattern that is
enforced there
v2: https://lore.kernel.org/all/20250224180801.128685-2-francesco@dolcini.it/
- no changes
v1: https://lore.kernel.org/all/20250218165633.106867-2-francesco@dolcini.it/
---
.../devicetree/bindings/hwmon/ti,amc6821.yaml | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml b/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
index 5d33f1a23d03..94aca9c378e6 100644
--- a/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
+++ b/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
@@ -28,6 +28,13 @@ properties:
i2c-mux:
type: object
+ fan:
+ $ref: fan-common.yaml#
+ unevaluatedProperties: false
+
+ "#pwm-cells":
+ const: 3
+
required:
- compatible
- reg
@@ -50,9 +57,14 @@ examples:
#address-cells = <1>;
#size-cells = <0>;
- fan@18 {
+ fan_controller: fan@18 {
compatible = "ti,amc6821";
reg = <0x18>;
+ #pwm-cells = <3>;
+
+ fan {
+ pwms = <&fan_controller 0 40000 0>;
+ };
};
};
--
2.39.5
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 2/2] hwmon: (amc6821) Add PWM polarity configuration with OF
2025-03-31 15:52 [PATCH v3 0/2] hwmon: (amc6821) Add PWM polarity configuration with OF Francesco Dolcini
2025-03-31 15:52 ` [PATCH v3 1/2] dt-bindings: hwmon: amc6821: add fan and PWM output Francesco Dolcini
@ 2025-03-31 15:52 ` Francesco Dolcini
1 sibling, 0 replies; 10+ messages in thread
From: Francesco Dolcini @ 2025-03-31 15:52 UTC (permalink / raw)
To: Jean Delvare, Guenter Roeck, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Farouk Bouabid, Quentin Schulz
Cc: Francesco Dolcini, linux-hwmon, devicetree, linux-kernel
From: Francesco Dolcini <francesco.dolcini@toradex.com>
Add support to configure the PWM-Out pin polarity based on the device
tree. The binding would allow also to configure the PWM period, this is
currently not implemented by the driver.
The driver has a module option to set the PWM polarity (normal=0,
inverted=1), when specified it always takes the precedence over the DT.
Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
v3:
- configure PWM polarity using standard PWM DT properties
v2: https://lore.kernel.org/lkml/20250224180801.128685-3-francesco@dolcini.it/
- pwminv module parameter takes always the precedence over the DT property
v1: https://lore.kernel.org/all/20250218165633.106867-3-francesco@dolcini.it/
---
drivers/hwmon/amc6821.c | 50 ++++++++++++++++++++++++++++++++++++-----
1 file changed, 45 insertions(+), 5 deletions(-)
diff --git a/drivers/hwmon/amc6821.c b/drivers/hwmon/amc6821.c
index 1e3c6acd8974..9ad4e9b63138 100644
--- a/drivers/hwmon/amc6821.c
+++ b/drivers/hwmon/amc6821.c
@@ -23,9 +23,12 @@
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/of_platform.h>
+#include <linux/pwm.h>
#include <linux/regmap.h>
#include <linux/slab.h>
+#include <dt-bindings/pwm/pwm.h>
+
/*
* Addresses to scan.
*/
@@ -37,7 +40,7 @@ static const unsigned short normal_i2c[] = {0x18, 0x19, 0x1a, 0x2c, 0x2d, 0x2e,
* Insmod parameters
*/
-static int pwminv; /*Inverted PWM output. */
+static int pwminv = -1; /*Inverted PWM output. */
module_param(pwminv, int, 0444);
static int init = 1; /*Power-on initialization.*/
@@ -845,9 +848,43 @@ static int amc6821_detect(struct i2c_client *client, struct i2c_board_info *info
return 0;
}
-static int amc6821_init_client(struct amc6821_data *data)
+static enum pwm_polarity amc6821_pwm_polarity(struct i2c_client *client)
+{
+ enum pwm_polarity polarity = PWM_POLARITY_NORMAL;
+ struct of_phandle_args args;
+ struct device_node *fan_np;
+
+ /*
+ * For backward compatibility, the pwminv module parameter takes
+ * always the precedence over any other device description
+ */
+ if (pwminv == 0)
+ return PWM_POLARITY_NORMAL;
+ if (pwminv > 0)
+ return PWM_POLARITY_INVERSED;
+
+ fan_np = of_get_child_by_name(client->dev.of_node, "fan");
+ if (!fan_np)
+ return PWM_POLARITY_NORMAL;
+
+ if (of_parse_phandle_with_args(fan_np, "pwms", "#pwm-cells", 0, &args))
+ goto out;
+ of_node_put(args.np);
+
+ if (args.args_count != 3)
+ goto out;
+
+ if (args.args[2] & PWM_POLARITY_INVERTED)
+ polarity = PWM_POLARITY_INVERSED;
+out:
+ of_node_put(fan_np);
+ return polarity;
+}
+
+static int amc6821_init_client(struct i2c_client *client, struct amc6821_data *data)
{
struct regmap *regmap = data->regmap;
+ u32 regval;
int err;
if (init) {
@@ -864,11 +901,14 @@ static int amc6821_init_client(struct amc6821_data *data)
if (err)
return err;
+ regval = AMC6821_CONF1_START;
+ if (amc6821_pwm_polarity(client) == PWM_POLARITY_INVERSED)
+ regval |= AMC6821_CONF1_PWMINV;
+
err = regmap_update_bits(regmap, AMC6821_REG_CONF1,
AMC6821_CONF1_THERMOVIE | AMC6821_CONF1_FANIE |
AMC6821_CONF1_START | AMC6821_CONF1_PWMINV,
- AMC6821_CONF1_START |
- (pwminv ? AMC6821_CONF1_PWMINV : 0));
+ regval);
if (err)
return err;
}
@@ -916,7 +956,7 @@ static int amc6821_probe(struct i2c_client *client)
"Failed to initialize regmap\n");
data->regmap = regmap;
- err = amc6821_init_client(data);
+ err = amc6821_init_client(client, data);
if (err)
return err;
--
2.39.5
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: hwmon: amc6821: add fan and PWM output
2025-03-31 15:52 ` [PATCH v3 1/2] dt-bindings: hwmon: amc6821: add fan and PWM output Francesco Dolcini
@ 2025-03-31 16:01 ` Guenter Roeck
2025-03-31 16:16 ` Francesco Dolcini
2025-04-01 6:13 ` Krzysztof Kozlowski
1 sibling, 1 reply; 10+ messages in thread
From: Guenter Roeck @ 2025-03-31 16:01 UTC (permalink / raw)
To: Francesco Dolcini, Jean Delvare, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Farouk Bouabid, Quentin Schulz
Cc: Francesco Dolcini, linux-hwmon, devicetree, linux-kernel
On 3/31/25 08:52, Francesco Dolcini wrote:
> From: Francesco Dolcini <francesco.dolcini@toradex.com>
>
> Add properties to describe the fan and the PWM controller output.
>
> Link: https://www.ti.com/lit/gpn/amc6821
> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> ---
> v3:
> - explicitly describe the fan, use standard PWM and FAN bindings
> - pwm.yaml cannot be referenced, because of the $nodename pattern that is
> enforced there
> v2: https://lore.kernel.org/all/20250224180801.128685-2-francesco@dolcini.it/
> - no changes
> v1: https://lore.kernel.org/all/20250218165633.106867-2-francesco@dolcini.it/
> ---
> .../devicetree/bindings/hwmon/ti,amc6821.yaml | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml b/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
> index 5d33f1a23d03..94aca9c378e6 100644
> --- a/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
> @@ -28,6 +28,13 @@ properties:
> i2c-mux:
> type: object
>
> + fan:
> + $ref: fan-common.yaml#
> + unevaluatedProperties: false
> +
> + "#pwm-cells":
> + const: 3
> +
> required:
> - compatible
> - reg
> @@ -50,9 +57,14 @@ examples:
> #address-cells = <1>;
> #size-cells = <0>;
>
> - fan@18 {
> + fan_controller: fan@18 {
> compatible = "ti,amc6821";
> reg = <0x18>;
> + #pwm-cells = <3>;
> +
> + fan {
> + pwms = <&fan_controller 0 40000 0>;
There is no explanation for the parameters. I guess that the first is the index and the
second is the frequency. The index is not used and can be dropped (there is just a single
channel), and the frequency only makes sense if it can actually be configured.
Either case, the fields need to be documented.
Thanks,
Guenter
> + };
> };
> };
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: hwmon: amc6821: add fan and PWM output
2025-03-31 16:01 ` Guenter Roeck
@ 2025-03-31 16:16 ` Francesco Dolcini
2025-03-31 17:24 ` Guenter Roeck
0 siblings, 1 reply; 10+ messages in thread
From: Francesco Dolcini @ 2025-03-31 16:16 UTC (permalink / raw)
To: Guenter Roeck
Cc: Francesco Dolcini, Jean Delvare, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Farouk Bouabid, Quentin Schulz, Francesco Dolcini,
linux-hwmon, devicetree, linux-kernel
Hello Guenter,
On Mon, Mar 31, 2025 at 09:01:08AM -0700, Guenter Roeck wrote:
> On 3/31/25 08:52, Francesco Dolcini wrote:
> > From: Francesco Dolcini <francesco.dolcini@toradex.com>
> >
> > Add properties to describe the fan and the PWM controller output.
> >
> > Link: https://www.ti.com/lit/gpn/amc6821
> > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > ---
> > v3:
> > - explicitly describe the fan, use standard PWM and FAN bindings
> > - pwm.yaml cannot be referenced, because of the $nodename pattern that is
> > enforced there
> > v2: https://lore.kernel.org/all/20250224180801.128685-2-francesco@dolcini.it/
> > - no changes
> > v1: https://lore.kernel.org/all/20250218165633.106867-2-francesco@dolcini.it/
> > ---
> > .../devicetree/bindings/hwmon/ti,amc6821.yaml | 14 +++++++++++++-
> > 1 file changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml b/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
> > index 5d33f1a23d03..94aca9c378e6 100644
> > --- a/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
> > +++ b/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
> > @@ -28,6 +28,13 @@ properties:
> > i2c-mux:
> > type: object
> > + fan:
> > + $ref: fan-common.yaml#
> > + unevaluatedProperties: false
> > +
> > + "#pwm-cells":
> > + const: 3
> > +
> > required:
> > - compatible
> > - reg
> > @@ -50,9 +57,14 @@ examples:
> > #address-cells = <1>;
> > #size-cells = <0>;
> > - fan@18 {
> > + fan_controller: fan@18 {
> > compatible = "ti,amc6821";
> > reg = <0x18>;
> > + #pwm-cells = <3>;
> > +
> > + fan {
> > + pwms = <&fan_controller 0 40000 0>;
>
> There is no explanation for the parameters. I guess that the first is the index and the
> second is the frequency. The index is not used and can be dropped (there is just a single
> channel), and the frequency only makes sense if it can actually be configured.
The PWM period can be configured, the HW supports it, so I think is a
good idea to properly describe the HW now that I am modifying the
binding and not have the situation in which we need it in 1 month.
The instance is just one, it's correct, but there is an assumption
pretty much everywhere that these 3 #cells are
- PWM instance
- PWM period in ns
- PWM flags
I decided to keep the first cell there, with the PWM instance, for this
reason, even as you correctly wrote it would not be required here.
With that said, looking at existing binding, the description is almost
never present.
I would proceed this way for v4:
- keep the first cell as PWM instance
- add the description
Francesco
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: hwmon: amc6821: add fan and PWM output
2025-03-31 16:16 ` Francesco Dolcini
@ 2025-03-31 17:24 ` Guenter Roeck
0 siblings, 0 replies; 10+ messages in thread
From: Guenter Roeck @ 2025-03-31 17:24 UTC (permalink / raw)
To: Francesco Dolcini
Cc: Jean Delvare, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Farouk Bouabid, Quentin Schulz, Francesco Dolcini, linux-hwmon,
devicetree, linux-kernel
On 3/31/25 09:16, Francesco Dolcini wrote:
> Hello Guenter,
>
>
> On Mon, Mar 31, 2025 at 09:01:08AM -0700, Guenter Roeck wrote:
>> On 3/31/25 08:52, Francesco Dolcini wrote:
>>> From: Francesco Dolcini <francesco.dolcini@toradex.com>
>>>
>>> Add properties to describe the fan and the PWM controller output.
>>>
>>> Link: https://www.ti.com/lit/gpn/amc6821
>>> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
>>> ---
>>> v3:
>>> - explicitly describe the fan, use standard PWM and FAN bindings
>>> - pwm.yaml cannot be referenced, because of the $nodename pattern that is
>>> enforced there
>>> v2: https://lore.kernel.org/all/20250224180801.128685-2-francesco@dolcini.it/
>>> - no changes
>>> v1: https://lore.kernel.org/all/20250218165633.106867-2-francesco@dolcini.it/
>>> ---
>>> .../devicetree/bindings/hwmon/ti,amc6821.yaml | 14 +++++++++++++-
>>> 1 file changed, 13 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml b/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
>>> index 5d33f1a23d03..94aca9c378e6 100644
>>> --- a/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
>>> +++ b/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
>>> @@ -28,6 +28,13 @@ properties:
>>> i2c-mux:
>>> type: object
>>> + fan:
>>> + $ref: fan-common.yaml#
>>> + unevaluatedProperties: false
>>> +
>>> + "#pwm-cells":
>>> + const: 3
>>> +
>>> required:
>>> - compatible
>>> - reg
>>> @@ -50,9 +57,14 @@ examples:
>>> #address-cells = <1>;
>>> #size-cells = <0>;
>>> - fan@18 {
>>> + fan_controller: fan@18 {
>>> compatible = "ti,amc6821";
>>> reg = <0x18>;
>>> + #pwm-cells = <3>;
>>> +
>>> + fan {
>>> + pwms = <&fan_controller 0 40000 0>;
>>
>> There is no explanation for the parameters. I guess that the first is the index and the
>> second is the frequency. The index is not used and can be dropped (there is just a single
>> channel), and the frequency only makes sense if it can actually be configured.
>
> The PWM period can be configured, the HW supports it, so I think is a
> good idea to properly describe the HW now that I am modifying the
> binding and not have the situation in which we need it in 1 month.
>
> The instance is just one, it's correct, but there is an assumption
> pretty much everywhere that these 3 #cells are
> - PWM instance
> - PWM period in ns
> - PWM flags
>
> I decided to keep the first cell there, with the PWM instance, for this
> reason, even as you correctly wrote it would not be required here.
>
> With that said, looking at existing binding, the description is almost
> never present.
>
That is not a valid argument.
> I would proceed this way for v4:
> - keep the first cell as PWM instance
> - add the description
>
We just had another driver where we dropped the pwm instance number because
it is unnecessary. See commit a25633a00906. I very much prefer to keep it that
way if it is not needed. Everything else is just confusing. It is bad enough
that the sub-node is needed. We should not make it worse.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: hwmon: amc6821: add fan and PWM output
2025-03-31 15:52 ` [PATCH v3 1/2] dt-bindings: hwmon: amc6821: add fan and PWM output Francesco Dolcini
2025-03-31 16:01 ` Guenter Roeck
@ 2025-04-01 6:13 ` Krzysztof Kozlowski
2025-04-01 7:43 ` Francesco Dolcini
2025-04-01 23:08 ` Rob Herring
1 sibling, 2 replies; 10+ messages in thread
From: Krzysztof Kozlowski @ 2025-04-01 6:13 UTC (permalink / raw)
To: Francesco Dolcini
Cc: Jean Delvare, Guenter Roeck, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Farouk Bouabid, Quentin Schulz, Francesco Dolcini,
linux-hwmon, devicetree, linux-kernel
On Mon, Mar 31, 2025 at 05:52:28PM +0200, Francesco Dolcini wrote:
> From: Francesco Dolcini <francesco.dolcini@toradex.com>
>
> Add properties to describe the fan and the PWM controller output.
>
> Link: https://www.ti.com/lit/gpn/amc6821
> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> ---
> v3:
> - explicitly describe the fan, use standard PWM and FAN bindings
> - pwm.yaml cannot be referenced, because of the $nodename pattern that is
> enforced there
> v2: https://lore.kernel.org/all/20250224180801.128685-2-francesco@dolcini.it/
> - no changes
> v1: https://lore.kernel.org/all/20250218165633.106867-2-francesco@dolcini.it/
> ---
> .../devicetree/bindings/hwmon/ti,amc6821.yaml | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml b/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
> index 5d33f1a23d03..94aca9c378e6 100644
> --- a/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
> @@ -28,6 +28,13 @@ properties:
> i2c-mux:
> type: object
>
> + fan:
> + $ref: fan-common.yaml#
> + unevaluatedProperties: false
Why do you need the child, instead of referencing fan-common in the top
level?
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: hwmon: amc6821: add fan and PWM output
2025-04-01 6:13 ` Krzysztof Kozlowski
@ 2025-04-01 7:43 ` Francesco Dolcini
2025-04-01 12:33 ` Guenter Roeck
2025-04-01 23:08 ` Rob Herring
1 sibling, 1 reply; 10+ messages in thread
From: Francesco Dolcini @ 2025-04-01 7:43 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Francesco Dolcini, Jean Delvare, Guenter Roeck, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Farouk Bouabid, Quentin Schulz,
Francesco Dolcini, linux-hwmon, devicetree, linux-kernel
On Tue, Apr 01, 2025 at 08:13:14AM +0200, Krzysztof Kozlowski wrote:
> On Mon, Mar 31, 2025 at 05:52:28PM +0200, Francesco Dolcini wrote:
> > From: Francesco Dolcini <francesco.dolcini@toradex.com>
> >
> > Add properties to describe the fan and the PWM controller output.
> >
> > Link: https://www.ti.com/lit/gpn/amc6821
> > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > ---
> > v3:
> > - explicitly describe the fan, use standard PWM and FAN bindings
> > - pwm.yaml cannot be referenced, because of the $nodename pattern that is
> > enforced there
> > v2: https://lore.kernel.org/all/20250224180801.128685-2-francesco@dolcini.it/
> > - no changes
> > v1: https://lore.kernel.org/all/20250218165633.106867-2-francesco@dolcini.it/
> > ---
> > .../devicetree/bindings/hwmon/ti,amc6821.yaml | 14 +++++++++++++-
> > 1 file changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml b/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
> > index 5d33f1a23d03..94aca9c378e6 100644
> > --- a/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
> > +++ b/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
> > @@ -28,6 +28,13 @@ properties:
> > i2c-mux:
> > type: object
> >
> > + fan:
> > + $ref: fan-common.yaml#
> > + unevaluatedProperties: false
>
> Why do you need the child, instead of referencing fan-common in the top
> level?
Two small reasons.
First is that the amc6821 is a fan controller, and the fan is just
connected to it. So having the fan as a child seemed the right way to
describe it, and this is done like that in other hwmon binding.
.. but now that you asked I tried to move the fan-common to the top
level and it's not working.
I added
allOf:
- $ref: fan-common.yaml#
at top level, removed the fan child, and moved the pwms up one level in
the example
i2c {
#address-cells = <1>;
#size-cells = <0>;
fan_controller: fan@18 {
compatible = "ti,amc6821";
reg = <0x18>;
#pwm-cells = <2>;
pwms = <&fan_controller 40000 0>;
};
};
and the binding checker is not fine with it
$ make dt_binding_check DT_SCHEMA_FILES=ti,amc6821.yaml
SCHEMA Documentation/devicetree/bindings/processed-schema.json
CHKDT ./Documentation/devicetree/bindings
LINT ./Documentation/devicetree/bindings
DTEX Documentation/devicetree/bindings/hwmon/ti,amc6821.example.dts
DTC [C] Documentation/devicetree/bindings/hwmon/ti,amc6821.example.dtb
/home/francesco/Toradex/sources/linux/Documentation/devicetree/bindings/hwmon/ti,amc6821.example.dtb: fan@18: 'pwms' does not match any of the regexes: 'pinctrl-[0-9]+'
from schema $id: http://devicetree.org/schemas/hwmon/ti,amc6821.yaml#
Any suggestion?
Francesco
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: hwmon: amc6821: add fan and PWM output
2025-04-01 7:43 ` Francesco Dolcini
@ 2025-04-01 12:33 ` Guenter Roeck
0 siblings, 0 replies; 10+ messages in thread
From: Guenter Roeck @ 2025-04-01 12:33 UTC (permalink / raw)
To: Francesco Dolcini, Krzysztof Kozlowski
Cc: Jean Delvare, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Farouk Bouabid, Quentin Schulz, Francesco Dolcini, linux-hwmon,
devicetree, linux-kernel
On 4/1/25 00:43, Francesco Dolcini wrote:
> On Tue, Apr 01, 2025 at 08:13:14AM +0200, Krzysztof Kozlowski wrote:
>> On Mon, Mar 31, 2025 at 05:52:28PM +0200, Francesco Dolcini wrote:
>>> From: Francesco Dolcini <francesco.dolcini@toradex.com>
>>>
>>> Add properties to describe the fan and the PWM controller output.
>>>
>>> Link: https://www.ti.com/lit/gpn/amc6821
>>> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
>>> ---
>>> v3:
>>> - explicitly describe the fan, use standard PWM and FAN bindings
>>> - pwm.yaml cannot be referenced, because of the $nodename pattern that is
>>> enforced there
>>> v2: https://lore.kernel.org/all/20250224180801.128685-2-francesco@dolcini.it/
>>> - no changes
>>> v1: https://lore.kernel.org/all/20250218165633.106867-2-francesco@dolcini.it/
>>> ---
>>> .../devicetree/bindings/hwmon/ti,amc6821.yaml | 14 +++++++++++++-
>>> 1 file changed, 13 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml b/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
>>> index 5d33f1a23d03..94aca9c378e6 100644
>>> --- a/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
>>> +++ b/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
>>> @@ -28,6 +28,13 @@ properties:
>>> i2c-mux:
>>> type: object
>>>
>>> + fan:
>>> + $ref: fan-common.yaml#
>>> + unevaluatedProperties: false
>>
>> Why do you need the child, instead of referencing fan-common in the top
>> level?
>
> Two small reasons.
>
> First is that the amc6821 is a fan controller, and the fan is just
> connected to it. So having the fan as a child seemed the right way to
> describe it, and this is done like that in other hwmon binding.
>
> .. but now that you asked I tried to move the fan-common to the top
> level and it's not working.
>
> I added
>
> allOf:
> - $ref: fan-common.yaml#
>
> at top level, removed the fan child, and moved the pwms up one level in
> the example
>
> i2c {
> #address-cells = <1>;
> #size-cells = <0>;
>
> fan_controller: fan@18 {
> compatible = "ti,amc6821";
> reg = <0x18>;
> #pwm-cells = <2>;
> pwms = <&fan_controller 40000 0>;
devicetree wants to see #pwm-cells one level above pwms. Obviously you can't
put it under the i2c node, so you need the sub-level. Maybe there is a way
around it, but if so I don't know what it might be.
Guenter
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: hwmon: amc6821: add fan and PWM output
2025-04-01 6:13 ` Krzysztof Kozlowski
2025-04-01 7:43 ` Francesco Dolcini
@ 2025-04-01 23:08 ` Rob Herring
1 sibling, 0 replies; 10+ messages in thread
From: Rob Herring @ 2025-04-01 23:08 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Francesco Dolcini, Jean Delvare, Guenter Roeck,
Krzysztof Kozlowski, Conor Dooley, Farouk Bouabid, Quentin Schulz,
Francesco Dolcini, linux-hwmon, devicetree, linux-kernel
On Tue, Apr 01, 2025 at 08:13:14AM +0200, Krzysztof Kozlowski wrote:
> On Mon, Mar 31, 2025 at 05:52:28PM +0200, Francesco Dolcini wrote:
> > From: Francesco Dolcini <francesco.dolcini@toradex.com>
> >
> > Add properties to describe the fan and the PWM controller output.
> >
> > Link: https://www.ti.com/lit/gpn/amc6821
> > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > ---
> > v3:
> > - explicitly describe the fan, use standard PWM and FAN bindings
> > - pwm.yaml cannot be referenced, because of the $nodename pattern that is
> > enforced there
> > v2: https://lore.kernel.org/all/20250224180801.128685-2-francesco@dolcini.it/
> > - no changes
> > v1: https://lore.kernel.org/all/20250218165633.106867-2-francesco@dolcini.it/
> > ---
> > .../devicetree/bindings/hwmon/ti,amc6821.yaml | 14 +++++++++++++-
> > 1 file changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml b/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
> > index 5d33f1a23d03..94aca9c378e6 100644
> > --- a/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
> > +++ b/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
> > @@ -28,6 +28,13 @@ properties:
> > i2c-mux:
> > type: object
> >
> > + fan:
> > + $ref: fan-common.yaml#
> > + unevaluatedProperties: false
>
> Why do you need the child, instead of referencing fan-common in the top
> level?
Because shockingly a fan controller and a fan are 2 different pieces of
h/w. It took *years* for this advancement to come to fruition.
Rob
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-04-01 23:08 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-31 15:52 [PATCH v3 0/2] hwmon: (amc6821) Add PWM polarity configuration with OF Francesco Dolcini
2025-03-31 15:52 ` [PATCH v3 1/2] dt-bindings: hwmon: amc6821: add fan and PWM output Francesco Dolcini
2025-03-31 16:01 ` Guenter Roeck
2025-03-31 16:16 ` Francesco Dolcini
2025-03-31 17:24 ` Guenter Roeck
2025-04-01 6:13 ` Krzysztof Kozlowski
2025-04-01 7:43 ` Francesco Dolcini
2025-04-01 12:33 ` Guenter Roeck
2025-04-01 23:08 ` Rob Herring
2025-03-31 15:52 ` [PATCH v3 2/2] hwmon: (amc6821) Add PWM polarity configuration with OF Francesco Dolcini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).