Skip to contentSkip to conversation

Add PWM support to EN7581

PATCH Superseded Lorenzo Bianconi 3 participants 10 messages Aug 10, 2024 – Aug 11, 2024

Subsystems (2)
Device tree bindings PWM

v1 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Lorenzo Bianconi

author
#0 lore raw

cover Add PWM support to EN7581

Introduce driver for PWM module available on EN7581 SoC.

Benjamin Larsson (1):
pwm: airoha: Add support for EN7581 SoC

Christian Marangi (1):
dt-bindings: pwm: Document Airoha EN7581 PWM

 .../bindings/pwm/airoha,en7581-pwm.yaml       |  42 ++
 drivers/pwm/Kconfig                           |  10 +
 drivers/pwm/Makefile                          |   1 +
 drivers/pwm/pwm-airoha.c                      | 403 ++++++++++++++++++
 4 files changed, 456 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/airoha,en7581-pwm.yaml
 create mode 100644 drivers/pwm/pwm-airoha.c
Signature · 1 line
2.46.0
author
#1 lore raw

PATCH 1/2 dt-bindings: pwm: Document Airoha EN7581 PWM

From: Christian Marangi <[email protected]>

Document required property for the Airoha EN7581 PWM. The device requires 3 different address for the sgpio, flash and cycle config.

Signed-off-by Christian Marangi <[email protected]>
Signed-off-by Lorenzo Bianconi <[email protected]>
---
 .../bindings/pwm/airoha,en7581-pwm.yaml       | 42 +++++++++++++++++++
 1 file changed, 42 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/airoha,en7581-pwm.yaml
Documentation/devicetree/bindings/pwm/airoha,en7581-pwm.yaml new file +42 -0
new file mode 100644
index 000000000000..52470668f90e
@@ -0,0 +1,42 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/airoha,en7581-pwm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Airoha EN7581 PWM
+
+maintainers:
+  - Christian Marangi <[email protected]>
+
+allOf:
+  - $ref: pwm.yaml#
+
+properties:
+  compatible:
+    const: airoha,en7581-pwm
+
+  reg:
+    items:
+      - description: sgpio config address
+      - description: flash config address
+      - description: cycle config address
+
+  "#pwm-cells":
+    const: 3
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    pwm@1fbf0224 {
+        compatible = "airoha,en7581-pwm";
+        reg = <0x1fbf0224 0x10>,
+              <0x1fbf0238 0x28>,
+              <0x1fbf0298 0x8>;
+        #pwm-cells = <3>;
+    };
Signature · 1 line
2.46.0
#2 lore raw

Re: [PATCH 1/2] dt-bindings: pwm: Document Airoha EN7581 PWM

Quoted 4 lines · Lorenzo Bianconi
Documentation/devicetree/bindings/pwm/airoha,en7581-pwm.yaml new file +4 -0
+
+required:
+  - compatible
+  - reg

Documentation/devicetree/bindings/pwm/airoha,en7581-pwm.yaml line 40

Quoted 10 lines
Documentation/devicetree/bindings/pwm/airoha,en7581-pwm.yaml new file +10 -0
+
+additionalProperties: false
+
+examples:
+  - |
+    pwm@1fbf0224 {
+        compatible = "airoha,en7581-pwm";
+        reg = <0x1fbf0224 0x10>,
+              <0x1fbf0238 0x28>,
+              <0x1fbf0298 0x8>;

These look almost continuous, so I wonder what's in between? E.g. between 0x1fbf0224+10=0x1fbf0234 and 0x1fbf0238?

Rest looks good.

Best regards,
Krzysztof

author
#3 lore raw

Re: [PATCH 1/2] dt-bindings: pwm: Document Airoha EN7581 PWM

Quoted 10 lines · Krzysztof Kozlowski
Quoted 9 lines · Lorenzo Bianconi
Documentation/devicetree/bindings/pwm/airoha,en7581-pwm.yaml new file +9 -0
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
Quoted 8 lines

Documentation/devicetree/bindings/pwm/airoha,en7581-pwm.yaml line 40

Quoted 5 lines
Documentation/devicetree/bindings/pwm/airoha,en7581-pwm.yaml new file +5 -0
+    pwm@1fbf0224 {
+        compatible = "airoha,en7581-pwm";
+        reg = <0x1fbf0224 0x10>,
+              <0x1fbf0238 0x28>,
+              <0x1fbf0298 0x8>;

These look almost continuous, so I wonder what's in between? E.g. between 0x1fbf0224+10=0x1fbf0234 and 0x1fbf0238?

register 0x1fbf0234 will be used by pinctrl driver (I will post it soon) for pwm muxing. The issue here is clock, pinctrl, gpio, pwm and (future) serdes registers are all interleaved in a non-regular fashion in the following IO space:

- <0x1fa20000 - 0x1fa20384>
- <0x1fb00000 - 0x1fb0096c>
- <0x1fbf0200 - 0x1fbf02bc>

So in order to avoid conflicts we need a sparse mapping.

Regards,
Lorenzo

Quoted 6 lines

Rest looks good.

Best regards,
Krzysztof

Reviewed-by on v1 1/2
#4 lore raw

Re: [PATCH 1/2] dt-bindings: pwm: Document Airoha EN7581 PWM

In reply to Lorenzo Bianconi
Quoted 7 lines · Lorenzo Bianconi

From: Christian Marangi <[email protected]>

Document required property for the Airoha EN7581 PWM. The device requires 3 different address for the sgpio, flash and cycle config.

Signed-off-by Christian Marangi <[email protected]>
Signed-off-by Lorenzo Bianconi <[email protected]>
Reviewed-by Krzysztof Kozlowski <[email protected]>

Best regards,
Krzysztof

author
#5 lore raw

PATCH 2/2 pwm: airoha: Add support for EN7581 SoC

In reply to Lorenzo Bianconi

From: Benjamin Larsson <[email protected]>

Introduce driver for PWM module available on EN7581 SoC.

Signed-off-by Benjamin Larsson <[email protected]>
Co-developed-by Lorenzo Bianconi <[email protected]>
Signed-off-by Lorenzo Bianconi <[email protected]>
---
 drivers/pwm/Kconfig      |  10 +
 drivers/pwm/Makefile     |   1 +
 drivers/pwm/pwm-airoha.c | 403 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 414 insertions(+)
 create mode 100644 drivers/pwm/pwm-airoha.c
drivers/pwm/Kconfig +10 -0
index 1dd7921194f5..0d702b394ea7 100644
@@ -47,6 +47,16 @@ config PWM_AB8500
 	  To compile this driver as a module, choose M here: the module
 	  will be called pwm-ab8500.
 
+config PWM_AIROHA
+	tristate "Airoha PWM support"
+	depends on ARCH_AIROHA || COMPILE_TEST
+	depends on OF
+	help
+	  Generic PWM framework driver for Airoha SoC.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called pwm-airoha.
+
 config PWM_APPLE
 	tristate "Apple SoC PWM support"
 	depends on ARCH_APPLE || COMPILE_TEST
drivers/pwm/Makefile +1 -0
index 90913519f11a..cef216753dd6 100644
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_PWM)		+= core.o
 obj-$(CONFIG_PWM_AB8500)	+= pwm-ab8500.o
+obj-$(CONFIG_PWM_AIROHA)	+= pwm-airoha.o
 obj-$(CONFIG_PWM_APPLE)		+= pwm-apple.o
 obj-$(CONFIG_PWM_ATMEL)		+= pwm-atmel.o
 obj-$(CONFIG_PWM_ATMEL_HLCDC_PWM)	+= pwm-atmel-hlcdc.o
drivers/pwm/pwm-airoha.c new file +403 -0

403 lines not on this page · raw message

Signature · 1 line
2.46.0
#6 lore raw

Re: [PATCH 2/2] pwm: airoha: Add support for EN7581 SoC

Quoted 8 lines · Lorenzo Bianconi

From: Benjamin Larsson <[email protected]>

Introduce driver for PWM module available on EN7581 SoC.

Signed-off-by Benjamin Larsson <[email protected]>
Co-developed-by Lorenzo Bianconi <[email protected]>
Signed-off-by Lorenzo Bianconi <[email protected]>

---

...

Quoted 5 lines
drivers/pwm/pwm-airoha.c new file +5 -0
+
+static void airoha_pwm_config_flash_map(struct airoha_pwm *pc,
+					unsigned int hwpwm, int index)
+{
+	u32 addr, mask, val;

drivers/pwm/pwm-airoha.c line 262

Quoted 10 lines
drivers/pwm/pwm-airoha.c new file +10 -0
+
+	if (hwpwm < PWM_NUM_GPIO) {
+		addr = REG_GPIO_FLASH_MAP(hwpwm / 8);
+	} else {
+		addr = REG_SIPO_FLASH_MAP(hwpwm / 8);
+		hwpwm -= PWM_NUM_GPIO;
+	}
+
+	if (index < 0) {
+		/* Change of waveform takes effect immediately but

This and other comments should be not netdev-style but general Linux style.

Quoted 5 lines
drivers/pwm/pwm-airoha.c new file +5 -0
+		 * disabling has some delay so to prevent glitching
+		 * only the enable bit is touched when disabling
+		 */
+		airoha_pwm_flash_clear(pc, addr, GPIO_FLASH_EN(hwpwm % 8));
+		return;

...

Quoted 7 lines
drivers/pwm/pwm-airoha.c new file +7 -0
+
+static const struct of_device_id airoha_pwm_of_match[] = {
+	{ .compatible = "airoha,en7581-pwm" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, airoha_pwm_of_match);
+

drivers/pwm/pwm-airoha.c line 398

Quoted 10 lines
drivers/pwm/pwm-airoha.c new file +10 -0
+static struct platform_driver airoha_pwm_driver = {
+	.driver = {
+		.name = "airoha-pwm",
+		.of_match_table = airoha_pwm_of_match,
+	},
+	.probe = airoha_pwm_probe,
+};
+module_platform_driver(airoha_pwm_driver);
+
+MODULE_ALIAS("platform:airoha-pwm");

You should not need MODULE_ALIAS() in normal cases. If you need it, usually it means your device ID table is wrong (e.g. misses either entries or MODULE_DEVICE_TABLE()). MODULE_ALIAS() is not a substitute for incomplete ID table.

Especially that it does not match compatible, so you cannot use excuse module autoloading does not work for given OF node...

Best regards,
Krzysztof

author
#7 lore raw

Re: [PATCH 2/2] pwm: airoha: Add support for EN7581 SoC

Quoted 22 lines · Lorenzo Bianconi
Quoted 8 lines

From: Benjamin Larsson <[email protected]>

Introduce driver for PWM module available on EN7581 SoC.

Signed-off-by Benjamin Larsson <[email protected]>
Co-developed-by Lorenzo Bianconi <[email protected]>
Signed-off-by Lorenzo Bianconi <[email protected]>

---

...

Quoted 9 lines
drivers/pwm/pwm-airoha.c new file +9 -0
+
+static void airoha_pwm_config_flash_map(struct airoha_pwm *pc,
+					unsigned int hwpwm, int index)
+{
+	u32 addr, mask, val;
+
+	if (hwpwm < PWM_NUM_GPIO) {
+		addr = REG_GPIO_FLASH_MAP(hwpwm / 8);
+	} else {
Quoted 8 lines

drivers/pwm/pwm-airoha.c line 262

Quoted 6 lines
drivers/pwm/pwm-airoha.c new file +6 -0
+		addr = REG_SIPO_FLASH_MAP(hwpwm / 8);
+		hwpwm -= PWM_NUM_GPIO;
+	}
+
+	if (index < 0) {
+		/* Change of waveform takes effect immediately but

This and other comments should be not netdev-style but general Linux style.

ack, I will fix them in v2.

Quoted 33 lines
Quoted 6 lines
drivers/pwm/pwm-airoha.c new file +5 -0
+		 * disabling has some delay so to prevent glitching
+		 * only the enable bit is touched when disabling
+		 */
+		airoha_pwm_flash_clear(pc, addr, GPIO_FLASH_EN(hwpwm % 8));
+		return;

...

Quoted 7 lines
drivers/pwm/pwm-airoha.c new file +7 -0
+
+static const struct of_device_id airoha_pwm_of_match[] = {
+	{ .compatible = "airoha,en7581-pwm" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, airoha_pwm_of_match);
+

drivers/pwm/pwm-airoha.c line 398

Quoted 10 lines
drivers/pwm/pwm-airoha.c new file +10 -0
+static struct platform_driver airoha_pwm_driver = {
+	.driver = {
+		.name = "airoha-pwm",
+		.of_match_table = airoha_pwm_of_match,
+	},
+	.probe = airoha_pwm_probe,
+};
+module_platform_driver(airoha_pwm_driver);
+
+MODULE_ALIAS("platform:airoha-pwm");

You should not need MODULE_ALIAS() in normal cases. If you need it, usually it means your device ID table is wrong (e.g. misses either entries or MODULE_DEVICE_TABLE()). MODULE_ALIAS() is not a substitute for incomplete ID table.

Quoted 2 lines

Especially that it does not match compatible, so you cannot use excuse module autoloading does not work for given OF node...

ack, I will fix it in v2.

Regards,
Lorenzo

Quoted 5 lines

Best regards,
Krzysztof

build bot
#8 lore raw

Re: [PATCH 2/2] pwm: airoha: Add support for EN7581 SoC

In reply to Lorenzo Bianconi
kernel test robot noticed the following build errors:

Hi Lorenzo,

kernel test robot noticed the following build errors:

[auto build test ERROR on robh/for-next]
[also build test ERROR on linus/master v6.11-rc2 next-20240809] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Lorenzo-Bianconi/dt-bindings-pwm-Document-Airoha-EN7581-PWM/20240810-143716
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/a03f5ea9291e39eab303696eb03fdd44cf04e8d9.1723264979.git.lorenzo%40kernel.org
patch subject: [PATCH 2/2] pwm: airoha: Add support for EN7581 SoC
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20240811/[email protected]/config)
compiler: alpha-linux-gcc (GCC) 13.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240811/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

drivers/pwm/pwm-airoha.c: In function 'airoha_pwm_config_waveform':

>> drivers/pwm/pwm-airoha.c:239:26: error: implicit declaration of function '__bf_shf' [-Werror=implicit-function-declaration]
     239 |         val = (period << __bf_shf(mask)) & mask;
         |                          ^~~~~~~~

cc1: some warnings being treated as errors

vim +/__bf_shf +239 drivers/pwm/pwm-airoha.c

231
232 static void airoha_pwm_config_waveform(struct airoha_pwm *pc, int index,
233 u32 duty, u32 period)
234 {
235 u32 mask, val;
236
237 /* Configure frequency divisor */
238 mask = WAVE_GEN_CYCLE_MASK(index % 4);

 > 239		val = (period << __bf_shf(mask)) & mask;

240 airoha_pwm_cycle_rmw(pc, REG_CYCLE_CFG_VALUE(index / 4), mask, val);
241
242 /* Configure duty cycle */
243 duty = ((DUTY_FULL - duty) << 8) | duty;
244 mask = GPIO_FLASH_PRD_MASK(index % 2);
245 val = (duty << __bf_shf(mask)) & mask;
246 airoha_pwm_flash_rmw(pc, REG_GPIO_FLASH_PRD_SET(index / 2), mask, val);
247 }
248

Signature · 2 lines
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
build bot
#9 lore raw

Re: [PATCH 2/2] pwm: airoha: Add support for EN7581 SoC

In reply to Lorenzo Bianconi
kernel test robot noticed the following build errors:

Hi Lorenzo,

kernel test robot noticed the following build errors:

[auto build test ERROR on robh/for-next]
[also build test ERROR on linus/master thierry-reding-pwm/for-next v6.11-rc2 next-20240809] [If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Lorenzo-Bianconi/dt-bindings-pwm-Document-Airoha-EN7581-PWM/20240810-143716
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/a03f5ea9291e39eab303696eb03fdd44cf04e8d9.1723264979.git.lorenzo%40kernel.org
patch subject: [PATCH 2/2] pwm: airoha: Add support for EN7581 SoC
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20240811/[email protected]/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project f86594788ce93b696675c94f54016d27a6c21d18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240811/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

In file included from drivers/pwm/pwm-airoha.c:7:
In file included from include/linux/io.h:14:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]

     548 |         val = __raw_readb(PCI_IOBASE + addr);
         |                           ~~~~~~~~~~ ^
   include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     561 |         val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
      37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
         |                                                   ^
   In file included from drivers/pwm/pwm-airoha.c:7:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     574 |         val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
      35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
         |                                                   ^
   In file included from drivers/pwm/pwm-airoha.c:7:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     585 |         __raw_writeb(value, PCI_IOBASE + addr);
         |                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     595 |         __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     605 |         __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
>> drivers/pwm/pwm-airoha.c:239:19: error: call to undeclared function '__bf_shf'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     239 |         val = (period << __bf_shf(mask)) & mask;
         |                          ^
   drivers/pwm/pwm-airoha.c:271:24: error: call to undeclared function '__bf_shf'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     271 |         val = ((index & 7) << __bf_shf(mask)) & mask;
         |                               ^

6 warnings and 2 errors generated.

vim +/__bf_shf +239 drivers/pwm/pwm-airoha.c

231
232 static void airoha_pwm_config_waveform(struct airoha_pwm *pc, int index,
233 u32 duty, u32 period)
234 {
235 u32 mask, val;
236
237 /* Configure frequency divisor */
238 mask = WAVE_GEN_CYCLE_MASK(index % 4);

 > 239		val = (period << __bf_shf(mask)) & mask;

240 airoha_pwm_cycle_rmw(pc, REG_CYCLE_CFG_VALUE(index / 4), mask, val);
241
242 /* Configure duty cycle */
243 duty = ((DUTY_FULL - duty) << 8) | duty;
244 mask = GPIO_FLASH_PRD_MASK(index % 2);
245 val = (duty << __bf_shf(mask)) & mask;
246 airoha_pwm_flash_rmw(pc, REG_GPIO_FLASH_PRD_SET(index / 2), mask, val);
247 }
248

Signature · 2 lines
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki