| Message ID | 20221013094838.1529153-2-Naresh.Solanki@9elements.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add devicetree support for max6639 | expand |
On Thu, Oct 13, 2022 at 11:48:36AM +0200, Naresh Solanki wrote: > Add common fan properties bindings to a schema. > > Bindings for fan controllers can reference the common schema for the > fan > > child nodes: > > patternProperties: > "^fan@[0-2]": > type: object > $ref: fan-common.yaml# > > Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com> > --- > .../devicetree/bindings/hwmon/fan-common.yaml | 48 +++++++++++++++++++ > 1 file changed, 48 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwmon/fan-common.yaml > > diff --git a/Documentation/devicetree/bindings/hwmon/fan-common.yaml b/Documentation/devicetree/bindings/hwmon/fan-common.yaml > new file mode 100644 > index 000000000000..224f5013c93f > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwmon/fan-common.yaml > @@ -0,0 +1,48 @@ > +# SPDX-License-Identifier: GPL-2.0-or-later OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/hwmon/fan-common.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Common fan properties > + > +maintainers: > + - Naresh Solanki <naresh.solanki@9elements.com> > + > +properties: > + max-rpm: > + description: > + Max RPM supported by fan. > + $ref: /schemas/types.yaml#/definitions/uint32 > + > + pulses-per-revolution: > + description: > + The number of pulse from fan sensor per revolution. > + $ref: /schemas/types.yaml#/definitions/uint32 > + > + default-rpm: > + description: > + Target RPM the fan should be configured during driver probe. So if we unload and reload the driver module, it should go back to the default? I think it is really, 'target RPM if not already configured' which could be keep the setting from a register (e.g. what the bootloader set) or perhaps you already have temperature information to use... > + $ref: /schemas/types.yaml#/definitions/uint32 > + pwm-frequency: > + description: > + PWM frequency for fan in Hertz(Hz). > + $ref: /schemas/types.yaml#/definitions/uint32 > + > + pwm-polarity-inverse: > + description: > + Inverse PWM polarity for fan. > + type: boolean As I said before, the PWM binding handles these 2 settings. Use it. Yes, it's a bit of an overkill when the child is the consumer of the parent. Until some 'clever' h/w engineer decides to use one of the PWMs for something else like a backlight. Rob
On 24-10-2022 09:48 pm, Rob Herring wrote: > So if we unload and reload the driver module, it should go back to the > default? This is RPM to be set during probe if desired. > > I think it is really, 'target RPM if not already configured' which could > be keep the setting from a register (e.g. what the bootloader set) or > perhaps you already have temperature information to use... Yes. missed it. It should be target-rpm will correct this. in next version. > >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + pwm-frequency: >> + description: >> + PWM frequency for fan in Hertz(Hz). >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + >> + pwm-polarity-inverse: >> + description: >> + Inverse PWM polarity for fan. >> + type: boolean > As I said before, the PWM binding handles these 2 settings. Use it. Yes, > it's a bit of an overkill when the child is the consumer of the parent. > Until some 'clever' h/w engineer decides to use one of the PWMs for > something else like a backlight. I would like you to consider this as something recommended by fan datasheet for the given fan instance. This info can be used by fan controller driver to configure PWM source/provider accordingly. If you still feel that may not make sense then I'll remove this property. Thanks, Naresh
On Tue, Oct 25, 2022 at 4:16 AM Naresh Solanki <naresh.solanki@9elements.com> wrote: > > > > On 24-10-2022 09:48 pm, Rob Herring wrote: > > So if we unload and reload the driver module, it should go back to the > > default? > This is RPM to be set during probe if desired. > > > > I think it is really, 'target RPM if not already configured' which could > > be keep the setting from a register (e.g. what the bootloader set) or > > perhaps you already have temperature information to use... > Yes. missed it. It should be target-rpm will correct this. in next version. > > > >> + $ref: /schemas/types.yaml#/definitions/uint32 > >> + pwm-frequency: > >> + description: > >> + PWM frequency for fan in Hertz(Hz). > >> + $ref: /schemas/types.yaml#/definitions/uint32 > >> + > >> + pwm-polarity-inverse: > >> + description: > >> + Inverse PWM polarity for fan. > >> + type: boolean > > As I said before, the PWM binding handles these 2 settings. Use it. Yes, > > it's a bit of an overkill when the child is the consumer of the parent. > > Until some 'clever' h/w engineer decides to use one of the PWMs for > > something else like a backlight. > I would like you to consider this as something recommended by fan > datasheet for the given fan instance. > This info can be used by fan controller driver to configure PWM > source/provider accordingly. > > If you still feel that may not make sense then I'll remove this property. You evidently don't understand my comments. My suggestion is to do this: fanc: fan-controller { #pwm-cells = <3>; ... fan { pwms = <&fanc 0 500000 PWM_POLARITY_INVERTED>; ... }; }; 0 is PWM number and 500000 is the PWM frequency. The 3rd cell are per consumer flags. See pwm.txt for more details. Rob
Hi Rob, On 26-10-2022 07:07 pm, Rob Herring wrote: > fanc: fan-controller { > #pwm-cells = <3>; > ... > > fan { > pwms = <&fanc 0 500000 PWM_POLARITY_INVERTED>; > ... > }; > }; > > 0 is PWM number and 500000 is the PWM frequency. The 3rd cell are per > consumer flags. See pwm.txt for more details. Did the implementation & while testing getting the below err: [63.626505] max6639 166-002e: failed to create device link to 166-002e
On Mon, Oct 31, 2022 at 01:35:09PM +0530, Naresh Solanki wrote: > Hi Rob, > > On 26-10-2022 07:07 pm, Rob Herring wrote: > > fanc: fan-controller { > > #pwm-cells = <3>; > > ... > > > > fan { > > pwms = <&fanc 0 500000 PWM_POLARITY_INVERTED>; > > ... > > }; > > }; > > > > 0 is PWM number and 500000 is the PWM frequency. The 3rd cell are per > > consumer flags. See pwm.txt for more details. > > Did the implementation & while testing getting the below err: > [63.626505] max6639 166-002e: failed to create device link to 166-002e Does turning off fw_devlink help (fw_devlink=off)? Rob
Hi Rob,
On 02-11-2022 12:14 am, Rob Herring wrote:
> Does turning off fw_devlink help (fw_devlink=off)?
This didn't bring any difference for the error.
Failing due to same consumer & supplier.
Returning from here:
https://github.com/torvalds/linux/blob/master/drivers/base/core.c#L702
Also this can cause return:
https://github.com/torvalds/linux/blob/master/drivers/base/core.c#L732
Regards,
Naresh
Hi Rob, On 02-11-2022 12:14 am, Rob Herring wrote: > On Mon, Oct 31, 2022 at 01:35:09PM +0530, Naresh Solanki wrote: >> Hi Rob, >> >> On 26-10-2022 07:07 pm, Rob Herring wrote: >>> fanc: fan-controller { >>> #pwm-cells = <3>; >>> ... >>> >>> fan { >>> pwms = <&fanc 0 500000 PWM_POLARITY_INVERTED>; >>> ... >>> }; >>> }; >>> >>> 0 is PWM number and 500000 is the PWM frequency. The 3rd cell are per >>> consumer flags. See pwm.txt for more details. >> >> Did the implementation & while testing getting the below err: >> [63.626505] max6639 166-002e: failed to create device link to 166-002e > > Does turning off fw_devlink help (fw_devlink=off)? Will supplier == consumer, device link creation fails. Not sure what is best approach but not creating device link in this scenario help & for that below additional changes needed in pwm core. diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c index 4527f09a5c50..afea51c49138 100644 --- a/drivers/pwm/core.c +++ b/drivers/pwm/core.c @@ -730,6 +730,12 @@ static struct device_link *pwm_device_link_add(struct device *dev, return NULL; } + /* + * Do not attempt to create link if consumer itself is supplier. + */ + if (dev == pwm->chip->dev) + return 0; + dl = device_link_add(dev, pwm->chip->dev, DL_FLAG_AUTOREMOVE_CONSUMER); if (!dl) { dev_err(dev, "failed to create device link to %s\n", Regards, Naresh
diff --git a/Documentation/devicetree/bindings/hwmon/fan-common.yaml b/Documentation/devicetree/bindings/hwmon/fan-common.yaml new file mode 100644 index 000000000000..224f5013c93f --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/fan-common.yaml @@ -0,0 +1,48 @@ +# SPDX-License-Identifier: GPL-2.0-or-later OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/hwmon/fan-common.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Common fan properties + +maintainers: + - Naresh Solanki <naresh.solanki@9elements.com> + +properties: + max-rpm: + description: + Max RPM supported by fan. + $ref: /schemas/types.yaml#/definitions/uint32 + + pulses-per-revolution: + description: + The number of pulse from fan sensor per revolution. + $ref: /schemas/types.yaml#/definitions/uint32 + + default-rpm: + description: + Target RPM the fan should be configured during driver probe. + $ref: /schemas/types.yaml#/definitions/uint32 + + pwm-frequency: + description: + PWM frequency for fan in Hertz(Hz). + $ref: /schemas/types.yaml#/definitions/uint32 + + pwm-polarity-inverse: + description: + Inverse PWM polarity for fan. + type: boolean + + label: + description: + Optional fan label + + fan-supply: + description: + Power supply for fan. + +additionalProperties: true + +...
Add common fan properties bindings to a schema. Bindings for fan controllers can reference the common schema for the fan child nodes: patternProperties: "^fan@[0-2]": type: object $ref: fan-common.yaml# Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com> --- .../devicetree/bindings/hwmon/fan-common.yaml | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/fan-common.yaml base-commit: 0cf46a653bdae56683fece68dc50340f7520e6c4