Skip to contentSkip to conversation

pwm: intel: Add PWM driver for a new SoC

PATCH Superseded Rahul Tanwar 3 participants 6 messages Aug 21, 2020 – Aug 22, 2020

Subsystems (2)

v9 of 15 Superseded Latest is v15 →

lore
Share & tools
lore
#0 lore raw

v9 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.

v9:
- Address code quality related review concerns (Andy Shevchenko)
- Use devm_add_action_or_reset() instead of explicit unwind calls.

v8:
- Remove fan related optional properties usage, keep them as default. If needed, change pwm-fan driver separately in future to add them as generic properties.

v7:
- Address code quality related review concerns.
- Rename fan related property to pwm-*.
- Fix one make dt_binding_check reported error.

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     |  44 ++++
 drivers/pwm/Kconfig                                |  11 +
 drivers/pwm/Makefile                               |   1 +
 drivers/pwm/pwm-intel-lgm.c                        | 241 +++++++++++++++++++++
 4 files changed, 297 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
#1 lore raw

PATCH v9 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     | 44 ++++++++++++++++++++++
 1 file changed, 44 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/intel,lgm-pwm.yaml
Documentation/devicetree/bindings/pwm/intel,lgm-pwm.yaml new file +44 -0
new file mode 100644
index 000000000000..11a606536169
@@ -0,0 +1,44 @@
+# 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
+
+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
#2 lore raw

PATCH v9 2/2 Add PWM fan controller driver for LGM SoC

In reply to Rahul Tanwar

Intel 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 | 241 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 253 insertions(+)
 create mode 100644 drivers/pwm/pwm-intel-lgm.c
drivers/pwm/Kconfig +11 -0
index 7dbcf6973d33..4949c51fe90b 100644
@@ -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 HAS_IOMEM
+	depends on (OF && 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
index 2c2ba0a03557..e9431b151694 100644
@@ -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 +241 -0

241 lines not on this page · raw message

Signature · 1 line
2.11.0
#3 lore raw

Re: [PATCH v9 2/2] Add PWM fan controller driver for LGM SoC

Quoted 4 lines · Rahul Tanwar

Intel 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/pwm-intel-lgm.c line 20

Quoted 5 lines
drivers/pwm/pwm-intel-lgm.c new file +4 -0
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/module.h>
 
+#include <linux/of_device.h>

We haven't settle this yet...

Quoted 3 lines
drivers/pwm/pwm-intel-lgm.c new file +3 -0
+#include <linux/pwm.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
Signature · 2 lines
With Best Regards,
Andy Shevchenko
#4 lore raw

Re: [PATCH v9 2/2] Add PWM fan controller driver for LGM SoC

Hi Andy,

Quoted 5 lines · Andy Shevchenko
Quoted 4 lines · Rahul Tanwar

Intel 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.

Quoted 7 lines

...

drivers/pwm/pwm-intel-lgm.c line 20

Quoted 4 lines
drivers/pwm/pwm-intel-lgm.c new file +4 -0
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/module.h>
+#include <linux/of_device.h>

We haven't settle this yet...

I investigated more about it. I was getting build error because we were relying on of_device.h for including platform_device.h. You are right that we are not using anything from of_device.h. So i removed of_device.h from driver and added include <linux/platform_device.h> & build is ok.

Regarding mod_devicetable.h header, it gets included indirectly from <linux/pwm.h> which includes of.h which includes mod_devicetable.h. So i think no point including it again in the driver.

Regards,
Rahul

Quoted 3 lines
Quoted 3 lines
drivers/pwm/pwm-intel-lgm.c new file +3 -0
+#include <linux/pwm.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
#5 lore raw

Re: [PATCH v9 2/2] Add PWM fan controller driver for LGM SoC

Quoted 2 lines · Rahul
Quoted 1 line · Andy Shevchenko

On Fri, Aug 21, 2020 at 05:32:11PM +0800, Rahul Tanwar wrote:

...

Quoted 6 lines
Quoted 5 lines

drivers/pwm/pwm-intel-lgm.c line 20

Quoted 4 lines
drivers/pwm/pwm-intel-lgm.c new file +4 -0
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/module.h>
+#include <linux/of_device.h>

We haven't settle this yet...

Quoted 8 lines

I investigated more about it. I was getting build error because we were relying on of_device.h for including platform_device.h. You are right that we are not using anything from of_device.h. So i removed of_device.h from driver and added include <linux/platform_device.h> & build is ok.

Regarding mod_devicetable.h header, it gets included indirectly from <linux/pwm.h> which includes of.h which includes mod_devicetable.h. So i think no point including it again in the driver.

Ideally you should include all headers you have direct users of. mod_devicetable.h is exactly one and I doubt the pwm.h should include it, but it's another story.

There are, of course, some cases when not all required because there is a guarantee that upper one includes lower one. For example, if you use bitops.h there is no need to include bits.h.

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
drivers/pwm/pwm-intel-lgm.c new file +3 -0
+#include <linux/pwm.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
Signature · 2 lines
With Best Regards,
Andy Shevchenko