pwm: intel: Add PWM driver for a new SoC
Subsystems (2)
v6 cover pwm: intel: Add PWM driver for a new SoC
Patch 1 adds dt binding document in YAML format. Patch 2 add PWM fan controller driver for LGM SoC.
v6:
- Readjust .apply op as per review feedback.
- Add back pwm-cells property to resolve make dt_binding_check error. pwm-cells is a required property for PWM driver.
- Add back fan related optional properties.
v5:
- Address below review concerns from Uwe Kleine-K�nig.
* Improve comments about Limitations.
* Use return value of regmap_update_bits if container function returns error code.
* Modify .apply op to have strict checking for fixed period supported by PWM HW.
* Use u64 as type when use min_t for duty_cycle.
* Add reset_control_assert() in failure case in probe where it was missing earlier.
- Remove fan specific optional properties from pwm dt binding document (Rob Herring)
v4:
- Address below review concerns from Uwe Kleine-K�nig.
* Improve notes and limitations comments.
* Add common prefixes for all #defines.
* Modify/Improve logic in .apply & .get_state ops as advised.
* Skip error messages in probe when error is -EPROBE_DEFER.
* Add dependencies in Kconfig (OF & HAS_IOMEM) and add select REGMAP_MMIO.
* Address other code quality related review concerns.
- Fix make dt_binding_check reported error in YAML file.
v3:
- Address below review concerns from Uwe Kleine-K�nig.
* Remove fan rpm calibration task from the driver.
* Modify apply op as per the review feedback.
* Add roundup & round down where necessary.
* Address other misc code quality related review concerns.
* Use devm_reset_control_get_exclusive(). (Philipp Zabel)
* Improve dt binding document.
v2:
- Address below review concerns from Uwe Kleine-K�nig.
* Add notes and limitations about PWM HW.
* Rename all functions and structure to lgm_pwm_*
* Readjust space aligninment in structure fields to single space.
* Switch to using apply instead of config/enable/disable.
* Address other code quality related concerns.
* Rebase to 5.8-rc1.
- Address review concerns in dt binding YAML from Rob Herring.
v1:
- Initial version.
Rahul Tanwar (2):
Add DT bindings YAML schema for PWM fan controller of LGM SoC Add PWM fan controller driver for LGM SoC
.../devicetree/bindings/pwm/intel,lgm-pwm.yaml | 54 +++++ drivers/pwm/Kconfig | 11 + drivers/pwm/Makefile | 1 + drivers/pwm/pwm-intel-lgm.c | 268 +++++++++++++++++++++ 4 files changed, 334 insertions(+) create mode 100644 Documentation/devicetree/bindings/pwm/intel,lgm-pwm.yaml create mode 100644 drivers/pwm/pwm-intel-lgm.c
Signature · 1 line
2.11.0
PATCH v6 1/2 Add DT bindings YAML schema for PWM fan controller of LGM SoC
Intel's LGM(Lightning Mountain) SoC contains a PWM fan controller which is only used to control the fan attached to the system. This PWM controller does not have any other consumer other than fan. Add DT bindings documentation for this PWM fan controller.
--- .../devicetree/bindings/pwm/intel,lgm-pwm.yaml | 54 ++++++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 Documentation/devicetree/bindings/pwm/intel,lgm-pwm.yaml
Documentation/devicetree/bindings/pwm/intel,lgm-pwm.yaml new file +54 -0
@@ -0,0 +1,54 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pwm/intel,lgm-pwm.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: LGM SoC PWM fan controller + +maintainers: + - Rahul Tanwar <[email protected]> + +properties: + compatible: + const: intel,lgm-pwm + + reg: + maxItems: 1 + + "#pwm-cells": + const: 2 + + clocks: + maxItems: 1 + + resets: + maxItems: 1 + + intel,fan-wire: + $ref: '/schemas/types.yaml#/definitions/uint32' + description: Specifies fan mode. Default when unspecified is 2. + + intel,max-rpm: + $ref: '/schemas/types.yaml#/definitions/uint32' + description: + Specifies maximum RPM of fan attached to the system. + Default when unspecified is 4000. + ++required: + - compatible + - reg + - clocks + - resets + +additionalProperties: false + +examples: + - | + pwm: pwm@e0d00000 { + compatible = "intel,lgm-pwm"; + reg = <0xe0d00000 0x30>; + #pwm-cells = <2>; + clocks = <&cgu0 126>; + resets = <&rcu0 0x30 21>; + };
Signature · 1 line
2.11.0
PATCH v6 2/2 Add PWM fan controller driver for LGM SoC
In reply to Rahul TanwarIntel Lightning Mountain(LGM) SoC contains a PWM fan controller. This PWM controller does not have any other consumer, it is a dedicated PWM controller for fan attached to the system. Add driver for this PWM fan controller.
--- drivers/pwm/Kconfig | 11 ++ drivers/pwm/Makefile | 1 + drivers/pwm/pwm-intel-lgm.c | 268 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 280 insertions(+) create mode 100644 drivers/pwm/pwm-intel-lgm.c
drivers/pwm/Kconfig +11 -0
@@ -232,6 +232,17 @@ config PWM_IMX_TPM To compile this driver as a module, choose M here: the module will be called pwm-imx-tpm. +config PWM_INTEL_LGM + tristate "Intel LGM PWM support" + depends on OF && HAS_IOMEM + depends on X86 || COMPILE_TEST + select REGMAP_MMIO + help + Generic PWM fan controller driver for LGM SoC. + + To compile this driver as a module, choose M here: the module + will be called pwm-intel-lgm. + config PWM_IQS620A tristate "Azoteq IQS620A PWM support" depends on MFD_IQS62X || COMPILE_TEST
drivers/pwm/Makefile +1 -0
@@ -20,6 +20,7 @@ obj-$(CONFIG_PWM_IMG) += pwm-img.o obj-$(CONFIG_PWM_IMX1) += pwm-imx1.o obj-$(CONFIG_PWM_IMX27) += pwm-imx27.o obj-$(CONFIG_PWM_IMX_TPM) += pwm-imx-tpm.o +obj-$(CONFIG_PWM_INTEL_LGM) += pwm-intel-lgm.o obj-$(CONFIG_PWM_IQS620A) += pwm-iqs620a.o obj-$(CONFIG_PWM_JZ4740) += pwm-jz4740.o obj-$(CONFIG_PWM_LP3943) += pwm-lp3943.o
drivers/pwm/pwm-intel-lgm.c new file +268 -0
268 lines not on this page · raw message
Signature · 1 line
2.11.0
Rob Herring
maintainerRe: [PATCH v6 1/2] Add DT bindings YAML schema for PWM fan controller of LGM SoC
In reply to Rahul TanwarQuoted 11 lines · Rahul Tanwar
Intel's LGM(Lightning Mountain) SoC contains a PWM fan controller which is only used to control the fan attached to the system. This PWM controller does not have any other consumer other than fan. Add DT bindings documentation for this PWM fan controller.
Signed-off-by Rahul Tanwar <[email protected]>--- .../devicetree/bindings/pwm/intel,lgm-pwm.yaml | 54 ++++++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 Documentation/devicetree/bindings/pwm/intel,lgm-pwm.yaml
My bot found errors running 'make dt_binding_check' on your patch:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pwm/intel,lgm-pwm.yaml: Additional properties are not allowed ('+required' was unexpected) /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pwm/intel,lgm-pwm.yaml: Additional properties are not allowed ('+required' was unexpected) Documentation/devicetree/bindings/Makefile:20: recipe for target 'Documentation/devicetree/bindings/pwm/intel,lgm-pwm.example.dts' failed make[1]: *** [Documentation/devicetree/bindings/pwm/intel,lgm-pwm.example.dts] Error 1
make[1]: *** Waiting for unfinished jobs.... /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pwm/intel,lgm-pwm.yaml: ignoring, error in schema:
warning: no schema found in file: ./Documentation/devicetree/bindings/pwm/intel,lgm-pwm.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pwm/intel,lgm-pwm.yaml: ignoring, error in schema:
warning: no schema found in file: ./Documentation/devicetree/bindings/pwm/intel,lgm-pwm.yaml
Makefile:1347: recipe for target 'dt_binding_check' failed
make: *** [dt_binding_check] Error 2
See https://patchwork.ozlabs.org/patch/1337629
If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure dt-schema is up to date:
pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade
Please check and re-submit.
Rob Herring
maintainerRe: [PATCH v6 1/2] Add DT bindings YAML schema for PWM fan controller of LGM SoC
In reply to Rahul TanwarQuoted 43 lines · Rahul Tanwar
Intel's LGM(Lightning Mountain) SoC contains a PWM fan controller which is only used to control the fan attached to the system. This PWM controller does not have any other consumer other than fan. Add DT bindings documentation for this PWM fan controller.
Signed-off-by Rahul Tanwar <[email protected]>--- .../devicetree/bindings/pwm/intel,lgm-pwm.yaml | 54 ++++++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 Documentation/devicetree/bindings/pwm/intel,lgm-pwm.yamlDocumentation/devicetree/bindings/pwm/intel,lgm-pwm.yaml new file +26
-0@@ -0,0 +1,54 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pwm/intel,lgm-pwm.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: LGM SoC PWM fan controller + +maintainers: + - Rahul Tanwar <[email protected]> + +properties: + compatible: + const: intel,lgm-pwm + + reg: + maxItems: 1 + + "#pwm-cells": + const: 2 + + clocks: + maxItems: 1 + + resets: + maxItems: 1
Documentation/devicetree/bindings/pwm/intel,lgm-pwm.yaml line 36
Quoted 10 lines
Documentation/devicetree/bindings/pwm/intel,lgm-pwm.yaml new file +10
-0+ + intel,fan-wire: + $ref: '/schemas/types.yaml#/definitions/uint32' + description: Specifies fan mode. Default when unspecified is 2. + + intel,max-rpm: + $ref: '/schemas/types.yaml#/definitions/uint32' + description: + Specifies maximum RPM of fan attached to the system. + Default when unspecified is 4000.
Again, these are properties of a fan, not the pwm controller. They belong in a fan node.
Quoted 21 lines
Documentation/devicetree/bindings/pwm/intel,lgm-pwm.yaml new file +18
-0+ ++required: + - compatible + - reg + - clocks + - resets + +additionalProperties: false + +examples: + - | + pwm: pwm@e0d00000 { + compatible = "intel,lgm-pwm"; + reg = <0xe0d00000 0x30>; + #pwm-cells = <2>; + clocks = <&cgu0 126>; + resets = <&rcu0 0x30 21>; + };Signature · 1 line
2.11.0
Re: [PATCH v6 2/2] Add PWM fan controller driver for LGM SoC
In reply to Rahul TanwarHello,
I only found a two minor issues this round, see below.
drivers/pwm/pwm-intel-lgm.c line 77
Quoted 10 lines · Rahul Tanwar
drivers/pwm/pwm-intel-lgm.c new file +10
-0+static int lgm_pwm_apply (struct pwm_chip *chip,struct pwm_device *pwm, +const struct pwm_state *state) +{ +struct lgm_pwm_chip *pc =to_lgm_pwm_chip (chip); +u32 duty_cycle, val; +int ret; + +/* +* HW only supports only NORMAL polarity +* HW supports fixed period
there are too many "only"s here. What about:
/* * The hardware only supports * normal polarity and fixed period. */
?
Quoted 10 lines
drivers/pwm/pwm-intel-lgm.c new file +10
-0+*/ +if (state->polarity !=PWM_POLARITY_NORMAL || + state->period < pc->period) +return -EINVAL ; + +if (!state->enabled) { + ret =lgm_pwm_enable (chip,0 ); +return ret; + } +
drivers/pwm/pwm-intel-lgm.c line 97
Quoted 10 lines
drivers/pwm/pwm-intel-lgm.c new file +10
-0+ duty_cycle =min_t (u64, state->duty_cycle, pc->period); + val = duty_cycle *LGM_PWM_MAX_DUTY_CYCLE / pc->period; + + ret =regmap_update_bits (pc->regmap,LGM_PWM_FAN_CON0 ,LGM_PWM_FAN_DC_MSK , +FIELD_PREP (LGM_PWM_FAN_DC_MSK , val)); +if (ret) +return ret; + +if (state->enabled) + ret =lgm_pwm_enable (chip,1 );
You can do this unconditionally, if state->enabled is false the function returns a few lines above already.
drivers/pwm/pwm-intel-lgm.c line 86
Quoted 3 lines
drivers/pwm/pwm-intel-lgm.c new file +3
-0+ +return ret; +}
The rest looks fine.
Best regards
Uwe
Signature · 2 lines
Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ |
Re: [PATCH v6 1/2] Add DT bindings YAML schema for PWM fan controller of LGM SoC
In reply to Rob HerringHi Rob,
Quoted 48 lines · Rob Herring
Quoted 47 lines · Rahul Tanwar
Intel's LGM(Lightning Mountain) SoC contains a PWM fan controller which is only used to control the fan attached to the system. This PWM controller does not have any other consumer other than fan. Add DT bindings documentation for this PWM fan controller.
Signed-off-by Rahul Tanwar <[email protected]>--- .../devicetree/bindings/pwm/intel,lgm-pwm.yaml | 54 ++++++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 Documentation/devicetree/bindings/pwm/intel,lgm-pwm.yamlDocumentation/devicetree/bindings/pwm/intel,lgm-pwm.yaml new file +30
-0@@ -0,0 +1,54 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pwm/intel,lgm-pwm.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: LGM SoC PWM fan controller + +maintainers: + - Rahul Tanwar <[email protected]> + +properties: + compatible: + const: intel,lgm-pwm + + reg: + maxItems: 1 + + "#pwm-cells": + const: 2 + + clocks: + maxItems: 1 + + resets: + maxItems: 1 + + intel,fan-wire: + $ref: '/schemas/types.yaml#/definitions/uint32' + description: Specifies fan mode. Default when unspecified is 2.
Quoted 8 lines
Documentation/devicetree/bindings/pwm/intel,lgm-pwm.yaml line 36
Quoted 6 lines
Documentation/devicetree/bindings/pwm/intel,lgm-pwm.yaml new file +6
-0+ + intel,max-rpm: + $ref: '/schemas/types.yaml#/definitions/uint32' + description: + Specifies maximum RPM of fan attached to the system. + Default when unspecified is 4000.Again, these are properties of a fan, not the pwm controller. They belong in a fan node.
Our PWM controller is actually a PWM fan controller dedicated for controlling fan. I am looking for some suggestions from you on how to handle fan related optional properties in such a scenario.
Should i create a separate child node for fan with PWM node being the parent? Is that what you are suggesting? Thanks.
Regards,
Rahul
Re: [PATCH v6 1/2] Add DT bindings YAML schema for PWM fan controller of LGM SoC
Hello Rahul,
Quoted 6 lines · Rahul
Our PWM controller is actually a PWM fan controller dedicated for controlling fan. I am looking for some suggestions from you on how to handle fan related optional properties in such a scenario.
Should i create a separate child node for fan with PWM node being the parent? Is that what you are suggesting? Thanks.
What is the problem of just using pwm-fan?
Best regards
Uwe
Signature · 2 lines
Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ |