[PATCH v10 0/4] StarFive's Pulse Width Modulation driver support

[PATCH v10 0/4] StarFive's Pulse Width Modulation driver support

[William Qiu]

Hi,

This patchset adds initial rudimentary support for the StarFive
Pulse Width Modulation controller driver. And this driver will
be used in StarFive's VisionFive 2 board.The first patch add
Documentations for the device and Patch 2 adds device probe for
the module.

Changes v9->v10:
- Rebased to v6.7rc6.
- Dropped unuseful dependency.
- Added error handling.

Changes v8->v9:
- Rebased to v6.7rc4.
- Updated the bindings format.
- Dropped removed() interface.

Changes v7->v8:
- Rebased to v6.7rc3.
- Changed compatible to "opencores,pwm-v1"
- Adjusted the clock unprepare order.
- Followed dt-bindings Coding style.

Changes v6->v7:
- Rebased to v6.6.
- Added dependency architecture.
- Adopted new rounding algorithm.
- Added limitation descripton.
- Used function interfaces instead of macro definitions.
- Followed the linux coding style.

Changes v5->v6:
- Rebased to v6.6rc5.
- Changed driver into a generic OpenCores driver.
- Modified dt-bindings description into OpenCores.
- Uesd the StarFive compatible string to parameterize.

Changes v4->v5:
- Rebased to v6.6rc2.
- Updated macro definition indent.
- Replaced the clock initializes the interface.
- Fixed patch description.

Changes v3->v4:
- Rebased to v6.5rc7.
- Sorted the header files in alphabetic order.
- Changed iowrite32() to writel().
- Added a way to turn off.
- Modified polarity inversion implementation.
- Added 7100 support.
- Added dts patches.
- Used the various helpers in linux/math.h.
- Corrected formatting problems.
- Renamed dtbinding  to 'starfive,jh7100-pwm.yaml'.
- Dropped the redundant code.

Changes v2->v3:
- Fixed some formatting issues.

Changes v1->v2:
- Renamed the dt-binding 'pwm-starfive.yaml' to 'starfive,jh7110-pwm.yaml'.
- Dropped the compatible's Items.
- Dropped the unuse defines.
- Modified the code to follow the Linux coding style.
- Changed return value to dev_err_probe.
- Dropped the unnecessary local variable.

The patch series is based on v6.7rc6.

William Qiu (4):
  dt-bindings: pwm: Add bindings for OpenCores PWM Controller
  pwm: opencores: Add PWM driver support
  riscv: dts: starfive: jh7100: Add PWM node and pins configuration
  riscv: dts: starfive: jh7110: Add PWM node and pins configuration

 .../bindings/pwm/opencores,pwm.yaml           |  55 +++++
 MAINTAINERS                                   |   7 +
 .../boot/dts/starfive/jh7100-common.dtsi      |  24 ++
 arch/riscv/boot/dts/starfive/jh7100.dtsi      |   9 +
 .../jh7110-starfive-visionfive-2.dtsi         |  22 ++
 arch/riscv/boot/dts/starfive/jh7110.dtsi      |   9 +
 drivers/pwm/Kconfig                           |  12 +
 drivers/pwm/Makefile                          |   1 +
 drivers/pwm/pwm-ocores.c                      | 233 ++++++++++++++++++
 9 files changed, 372 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/opencores,pwm.yaml
 create mode 100644 drivers/pwm/pwm-ocores.c

--
2.34.1

[PATCH v10 1/4] dt-bindings: pwm: Add bindings for OpenCores PWM Controller

[William Qiu]

Add bindings for OpenCores PWM Controller.

Signed-off-by: William Qiu <william.qiu@starfivetech.com>
Reviewed-by: Hal Feng <hal.feng@starfivetech.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
---
 .../bindings/pwm/opencores,pwm.yaml           | 55 +++++++++++++++++++
 1 file changed, 55 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/opencores,pwm.yaml

diff --git a/Documentation/devicetree/bindings/pwm/opencores,pwm.yaml b/Documentation/devicetree/bindings/pwm/opencores,pwm.yaml
new file mode 100644
index 000000000000..0b85dd861dfd
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/opencores,pwm.yaml
@@ -0,0 +1,55 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/opencores,pwm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: OpenCores PWM controller
+
+maintainers:
+  - William Qiu <william.qiu@starfivetech.com>
+
+description:
+  The OpenCores PTC ip core contains a PWM controller. When operating in PWM
+  mode, the PTC core generates binary signal with user-programmable low and
+  high periods. All PTC counters and registers are 32-bit.
+
+allOf:
+  - $ref: pwm.yaml#
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - starfive,jh7100-pwm
+          - starfive,jh7110-pwm
+      - const: opencores,pwm-v1
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+  "#pwm-cells":
+    const: 3
+
+required:
+  - compatible
+  - reg
+  - clocks
+
+additionalProperties: false
+
+examples:
+  - |
+    pwm@12490000 {
+        compatible = "starfive,jh7110-pwm", "opencores,pwm-v1";
+        reg = <0x12490000 0x10000>;
+        clocks = <&clkgen 181>;
+        resets = <&rstgen 109>;
+        #pwm-cells = <3>;
+    };
-- 
2.34.1

[PATCH v10 2/4] pwm: opencores: Add PWM driver support

[William Qiu]

Add driver for OpenCores PWM Controller. And add compatibility code
which based on StarFive SoC.

Co-developed-by: Hal Feng <hal.feng@starfivetech.com>
Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
Signed-off-by: William Qiu <william.qiu@starfivetech.com>
---
 MAINTAINERS              |   7 ++
 drivers/pwm/Kconfig      |  12 ++
 drivers/pwm/Makefile     |   1 +
 drivers/pwm/pwm-ocores.c | 233 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 253 insertions(+)
 create mode 100644 drivers/pwm/pwm-ocores.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 9104430e148e..6a6c355150e7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16145,6 +16145,13 @@ F:	Documentation/i2c/busses/i2c-ocores.rst
 F:	drivers/i2c/busses/i2c-ocores.c
 F:	include/linux/platform_data/i2c-ocores.h

+OPENCORES PWM DRIVER
+M:	William Qiu <william.qiu@starfivetech.com>
+M:	Hal Feng <hal.feng@starfivetech.com>
+S:	Supported
+F:	Documentation/devicetree/bindings/pwm/opencores,pwm.yaml
+F:	drivers/pwm/pwm-ocores.c
+
 OPENRISC ARCHITECTURE
 M:	Jonas Bonn <jonas@southpole.se>
 M:	Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 4b956d661755..d87e1bb350ba 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -444,6 +444,18 @@ config PWM_NTXEC
 	  controller found in certain e-book readers designed by the original
 	  design manufacturer Netronix.

+config PWM_OCORES
+	tristate "OpenCores PWM support"
+	depends on HAS_IOMEM && OF
+	depends on COMMON_CLK
+	depends on ARCH_STARFIVE || COMPILE_TEST
+	help
+	  If you say yes to this option, support will be included for the
+	  OpenCores PWM. For details see https://opencores.org/projects/ptc.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called pwm-ocores.
+
 config PWM_OMAP_DMTIMER
 	tristate "OMAP Dual-Mode Timer PWM support"
 	depends on OF
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index c5ec9e168ee7..517c4f643058 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -40,6 +40,7 @@ obj-$(CONFIG_PWM_MICROCHIP_CORE)	+= pwm-microchip-core.o
 obj-$(CONFIG_PWM_MTK_DISP)	+= pwm-mtk-disp.o
 obj-$(CONFIG_PWM_MXS)		+= pwm-mxs.o
 obj-$(CONFIG_PWM_NTXEC)		+= pwm-ntxec.o
+obj-$(CONFIG_PWM_OCORES)	+= pwm-ocores.o
 obj-$(CONFIG_PWM_OMAP_DMTIMER)	+= pwm-omap-dmtimer.o
 obj-$(CONFIG_PWM_PCA9685)	+= pwm-pca9685.o
 obj-$(CONFIG_PWM_PXA)		+= pwm-pxa.o
diff --git a/drivers/pwm/pwm-ocores.c b/drivers/pwm/pwm-ocores.c
new file mode 100644
index 000000000000..dfb5a186da71
--- /dev/null
+++ b/drivers/pwm/pwm-ocores.c
@@ -0,0 +1,233 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * OpenCores PWM Driver
+ *
+ * https://opencores.org/projects/ptc
+ *
+ * Copyright (C) 2018-2023 StarFive Technology Co., Ltd.
+ *
+ * Limitations:
+ * - The hardware only do inverted polarity.
+ * - The hardware minimum period / duty_cycle is (1 / pwm_apb clock frequency) ns.
+ * - The hardware maximum period / duty_cycle is (U32_MAX / pwm_apb clock frequency) ns.
+ */
+
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pwm.h>
+#include <linux/reset.h>
+#include <linux/slab.h>
+
+/* OCPWM_CTRL register bits*/
+#define REG_OCPWM_EN      BIT(0)
+#define REG_OCPWM_ECLK    BIT(1)
+#define REG_OCPWM_NEC     BIT(2)
+#define REG_OCPWM_OE      BIT(3)
+#define REG_OCPWM_SIGNLE  BIT(4)
+#define REG_OCPWM_INTE    BIT(5)
+#define REG_OCPWM_INT     BIT(6)
+#define REG_OCPWM_CNTRRST BIT(7)
+#define REG_OCPWM_CAPTE   BIT(8)
+
+struct ocores_pwm_device {
+	struct pwm_chip chip;
+	struct clk *clk;
+	struct reset_control *rst;
+	const struct ocores_pwm_data *data;
+	void __iomem *regs;
+	u32 clk_rate; /* PWM APB clock frequency */
+};
+
+struct ocores_pwm_data {
+	void __iomem *(*get_ch_base)(void __iomem *base, unsigned int channel);
+};
+
+static inline u32 ocores_readl(struct ocores_pwm_device *ddata,
+			       unsigned int channel,
+			       unsigned int offset)
+{
+	void __iomem *base = ddata->data->get_ch_base ?
+			     ddata->data->get_ch_base(ddata->regs, channel) : ddata->regs;
+
+	return readl(base + offset);
+}
+
+static inline void ocores_writel(struct ocores_pwm_device *ddata,
+				 unsigned int channel,
+				 unsigned int offset, u32 val)
+{
+	void __iomem *base = ddata->data->get_ch_base ?
+			     ddata->data->get_ch_base(ddata->regs, channel) : ddata->regs;
+
+	writel(val, base + offset);
+}
+
+static inline struct ocores_pwm_device *chip_to_ocores(struct pwm_chip *chip)
+{
+	return container_of(chip, struct ocores_pwm_device, chip);
+}
+
+static void __iomem *starfive_jh71x0_get_ch_base(void __iomem *base,
+						 unsigned int channel)
+{
+	unsigned int offset = (channel > 3 ? 1 << 15 : 0) + (channel & 3) * 0x10;
+
+	return base + offset;
+}
+
+static int ocores_pwm_get_state(struct pwm_chip *chip,
+				struct pwm_device *pwm,
+				struct pwm_state *state)
+{
+	struct ocores_pwm_device *ddata = chip_to_ocores(chip);
+	u32 period_data, duty_data, ctrl_data;
+
+	period_data = ocores_readl(ddata, pwm->hwpwm, 0x8);
+	duty_data = ocores_readl(ddata, pwm->hwpwm, 0x4);
+	ctrl_data = ocores_readl(ddata, pwm->hwpwm, 0xC);
+
+	state->period = DIV_ROUND_UP_ULL((u64)period_data * NSEC_PER_SEC, ddata->clk_rate);
+	state->duty_cycle = DIV_ROUND_UP_ULL((u64)duty_data * NSEC_PER_SEC, ddata->clk_rate);
+	state->polarity = PWM_POLARITY_INVERSED;
+	state->enabled = (ctrl_data & REG_OCPWM_EN) ? true : false;
+
+	return 0;
+}
+
+static int ocores_pwm_apply(struct pwm_chip *chip,
+			    struct pwm_device *pwm,
+			    const struct pwm_state *state)
+{
+	struct ocores_pwm_device *ddata = chip_to_ocores(chip);
+	u32 ctrl_data = 0;
+	u64 period_data, duty_data;
+
+	if (state->polarity != PWM_POLARITY_INVERSED)
+		return -EINVAL;
+
+	ctrl_data = ocores_readl(ddata, pwm->hwpwm, 0xC);
+	ocores_writel(ddata, pwm->hwpwm, 0xC, 0);
+
+	period_data = DIV_ROUND_DOWN_ULL(state->period * ddata->clk_rate, NSEC_PER_SEC);
+	if (period_data <= U32_MAX)
+		ocores_writel(ddata, pwm->hwpwm, 0x8, (u32)period_data);
+	else
+		return -EINVAL;
+
+	duty_data = DIV_ROUND_DOWN_ULL(state->duty_cycle * ddata->clk_rate, NSEC_PER_SEC);
+	if (duty_data <= U32_MAX)
+		ocores_writel(ddata, pwm->hwpwm, 0x4, (u32)duty_data);
+	else
+		return -EINVAL;
+
+	ocores_writel(ddata, pwm->hwpwm, 0xC, 0);
+
+	if (state->enabled) {
+		ctrl_data = ocores_readl(ddata, pwm->hwpwm, 0xC);
+		ocores_writel(ddata, pwm->hwpwm, 0xC, ctrl_data | REG_OCPWM_EN | REG_OCPWM_OE);
+	}
+
+	return 0;
+}
+
+static const struct pwm_ops ocores_pwm_ops = {
+	.get_state	= ocores_pwm_get_state,
+	.apply		= ocores_pwm_apply,
+};
+
+static const struct ocores_pwm_data jh7100_pwm_data = {
+	.get_ch_base = starfive_jh71x0_get_ch_base,
+};
+
+static const struct ocores_pwm_data jh7110_pwm_data = {
+	.get_ch_base = starfive_jh71x0_get_ch_base,
+};
+
+static const struct of_device_id ocores_pwm_of_match[] = {
+	{ .compatible = "opencores,pwm-v1" },
+	{ .compatible = "starfive,jh7100-pwm", .data = &jh7100_pwm_data},
+	{ .compatible = "starfive,jh7110-pwm", .data = &jh7110_pwm_data},
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, ocores_pwm_of_match);
+
+static void ocores_reset_control_assert(void *data)
+{
+	reset_control_assert(data);
+}
+
+static int ocores_pwm_probe(struct platform_device *pdev)
+{
+	const struct of_device_id *id;
+	struct device *dev = &pdev->dev;
+	struct ocores_pwm_device *ddata;
+	struct pwm_chip *chip;
+	int ret;
+
+	id = of_match_device(ocores_pwm_of_match, dev);
+	if (!id)
+		return -EINVAL;
+
+	ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
+	if (!ddata)
+		return -ENOMEM;
+
+	ddata->data = id->data;
+	chip = &ddata->chip;
+	chip->dev = dev;
+	chip->ops = &ocores_pwm_ops;
+	chip->npwm = 8;
+	chip->of_pwm_n_cells = 3;
+
+	ddata->regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(ddata->regs))
+		return dev_err_probe(dev, PTR_ERR(ddata->regs),
+				     "Unable to map IO resources\n");
+
+	ddata->clk = devm_clk_get_enabled(dev, NULL);
+	if (IS_ERR(ddata->clk))
+		return dev_err_probe(dev, PTR_ERR(ddata->clk),
+				     "Unable to get pwm's clock\n");
+
+	ddata->rst = devm_reset_control_get_optional_exclusive(dev, NULL);
+	if (IS_ERR(ddata->rst))
+		return dev_err_probe(dev, PTR_ERR(ddata->rst),
+				     "Unable to get pwm's reset\n");
+
+	reset_control_deassert(ddata->rst);
+
+	ret = devm_add_action_or_reset(dev, ocores_reset_control_assert, ddata->rst);
+	if (ret)
+		return ret;
+
+	ddata->clk_rate = clk_get_rate(ddata->clk);
+	if (ddata->clk_rate <= 0)
+		return dev_err_probe(dev, ddata->clk_rate,
+				     "Unable to get clock's rate\n");
+
+	ret = devm_pwmchip_add(dev, chip);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Could not register PWM chip\n");
+
+	platform_set_drvdata(pdev, ddata);
+
+	return ret;
+}
+
+static struct platform_driver ocores_pwm_driver = {
+	.probe = ocores_pwm_probe,
+	.driver = {
+		.name = "ocores-pwm",
+		.of_match_table = ocores_pwm_of_match,
+	},
+};
+module_platform_driver(ocores_pwm_driver);
+
+MODULE_AUTHOR("Jieqin Chen");
+MODULE_AUTHOR("Hal Feng <hal.feng@starfivetech.com>");
+MODULE_DESCRIPTION("OpenCores PWM PTC driver");
+MODULE_LICENSE("GPL");
--
2.34.1

[PATCH v10 3/4] riscv: dts: starfive: jh7100: Add PWM node and pins configuration

[William Qiu]

Add OpenCores PWM controller node and add PWM pins configuration
on VisionFive 1 board.

Signed-off-by: William Qiu <william.qiu@starfivetech.com>
---
 .../boot/dts/starfive/jh7100-common.dtsi      | 24 +++++++++++++++++++
 arch/riscv/boot/dts/starfive/jh7100.dtsi      |  9 +++++++
 2 files changed, 33 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
index b93ce351a90f..11876906cc05 100644
--- a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
@@ -84,6 +84,24 @@ GPO_I2C2_PAD_SDA_OEN,
 		};
 	};
 
+	pwm_pins: pwm-0 {
+		pwm-pins {
+			pinmux = <GPIOMUX(7,
+				  GPO_PWM_PAD_OUT_BIT0,
+				  GPO_PWM_PAD_OE_N_BIT0,
+				  GPI_NONE)>,
+				 <GPIOMUX(5,
+				  GPO_PWM_PAD_OUT_BIT1,
+				  GPO_PWM_PAD_OE_N_BIT1,
+				  GPI_NONE)>;
+			bias-disable;
+			drive-strength = <35>;
+			input-disable;
+			input-schmitt-disable;
+			slew-rate = <0>;
+		};
+	};
+
 	uart3_pins: uart3-0 {
 		rx-pins {
 			pinmux = <GPIOMUX(13, GPO_LOW, GPO_DISABLE,
@@ -154,6 +172,12 @@ &osc_aud {
 	clock-frequency = <27000000>;
 };
 
+&pwm {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pwm_pins>;
+	status = "okay";
+};
+
 &uart3 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart3_pins>;
diff --git a/arch/riscv/boot/dts/starfive/jh7100.dtsi b/arch/riscv/boot/dts/starfive/jh7100.dtsi
index e68cafe7545f..4f5eb2f60856 100644
--- a/arch/riscv/boot/dts/starfive/jh7100.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7100.dtsi
@@ -280,6 +280,15 @@ watchdog@12480000 {
 				 <&rstgen JH7100_RSTN_WDT>;
 		};
 
+		pwm: pwm@12490000 {
+			compatible = "starfive,jh7100-pwm", "opencores,pwm-v1";
+			reg = <0x0 0x12490000 0x0 0x10000>;
+			clocks = <&clkgen JH7100_CLK_PWM_APB>;
+			resets = <&rstgen JH7100_RSTN_PWM_APB>;
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
+
 		sfctemp: temperature-sensor@124a0000 {
 			compatible = "starfive,jh7100-temp";
 			reg = <0x0 0x124a0000 0x0 0x10000>;
-- 
2.34.1

[PATCH v10 4/4] riscv: dts: starfive: jh7110: Add PWM node and pins configuration

[William Qiu]

Add OpenCores PWM controller node and add PWM pins configuration
on VisionFive 2 board.

Signed-off-by: William Qiu <william.qiu@starfivetech.com>
---
 .../jh7110-starfive-visionfive-2.dtsi         | 22 +++++++++++++++++++
 arch/riscv/boot/dts/starfive/jh7110.dtsi      |  9 ++++++++
 2 files changed, 31 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
index b89e9791efa7..e08af8a830ab 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
@@ -323,6 +323,12 @@ reserved-data@600000 {
 	};
 };
 
+&pwm {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pwm_pins>;
+	status = "okay";
+};
+
 &spi0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&spi0_pins>;
@@ -513,6 +519,22 @@ GPOEN_ENABLE,
 		};
 	};
 
+	pwm_pins: pwm-0 {
+		pwm-pins {
+			pinmux = <GPIOMUX(46, GPOUT_SYS_PWM_CHANNEL0,
+					      GPOEN_SYS_PWM0_CHANNEL0,
+					      GPI_NONE)>,
+				 <GPIOMUX(59, GPOUT_SYS_PWM_CHANNEL1,
+					      GPOEN_SYS_PWM0_CHANNEL1,
+					      GPI_NONE)>;
+			bias-disable;
+			drive-strength = <12>;
+			input-disable;
+			input-schmitt-disable;
+			slew-rate = <0>;
+		};
+	};
+
 	spi0_pins: spi0-0 {
 		mosi-pins {
 			pinmux = <GPIOMUX(52, GPOUT_SYS_SPI0_TXD,
diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
index 45213cdf50dc..1b782f2c1395 100644
--- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
@@ -829,6 +829,15 @@ i2stx1: i2s@120c0000 {
 			status = "disabled";
 		};
 
+		pwm: pwm@120d0000 {
+			compatible = "starfive,jh7110-pwm", "opencores,pwm-v1";
+			reg = <0x0 0x120d0000 0x0 0x10000>;
+			clocks = <&syscrg JH7110_SYSCLK_PWM_APB>;
+			resets = <&syscrg JH7110_SYSRST_PWM_APB>;
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
+
 		sfctemp: temperature-sensor@120e0000 {
 			compatible = "starfive,jh7110-temp";
 			reg = <0x0 0x120e0000 0x0 0x10000>;
-- 
2.34.1

Re: [PATCH v10 3/4] riscv: dts: starfive: jh7100: Add PWM node and pins configuration

[Emil Renner Berthing]

William Qiu wrote:
> Add OpenCores PWM controller node and add PWM pins configuration
> on VisionFive 1 board.
>
> Signed-off-by: William Qiu <william.qiu@starfivetech.com>

Sorry, I thought I already sent my review. This looks good.

Reviewed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>

> ---
>  .../boot/dts/starfive/jh7100-common.dtsi      | 24 +++++++++++++++++++
>  arch/riscv/boot/dts/starfive/jh7100.dtsi      |  9 +++++++
>  2 files changed, 33 insertions(+)
>
> diff --git a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
> index b93ce351a90f..11876906cc05 100644
> --- a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
> @@ -84,6 +84,24 @@ GPO_I2C2_PAD_SDA_OEN,
>  		};
>  	};
>
> +	pwm_pins: pwm-0 {
> +		pwm-pins {
> +			pinmux = <GPIOMUX(7,
> +				  GPO_PWM_PAD_OUT_BIT0,
> +				  GPO_PWM_PAD_OE_N_BIT0,
> +				  GPI_NONE)>,
> +				 <GPIOMUX(5,
> +				  GPO_PWM_PAD_OUT_BIT1,
> +				  GPO_PWM_PAD_OE_N_BIT1,
> +				  GPI_NONE)>;
> +			bias-disable;
> +			drive-strength = <35>;
> +			input-disable;
> +			input-schmitt-disable;
> +			slew-rate = <0>;
> +		};
> +	};
> +
>  	uart3_pins: uart3-0 {
>  		rx-pins {
>  			pinmux = <GPIOMUX(13, GPO_LOW, GPO_DISABLE,
> @@ -154,6 +172,12 @@ &osc_aud {
>  	clock-frequency = <27000000>;
>  };
>
> +&pwm {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pwm_pins>;
> +	status = "okay";
> +};
> +
>  &uart3 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&uart3_pins>;
> diff --git a/arch/riscv/boot/dts/starfive/jh7100.dtsi b/arch/riscv/boot/dts/starfive/jh7100.dtsi
> index e68cafe7545f..4f5eb2f60856 100644
> --- a/arch/riscv/boot/dts/starfive/jh7100.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7100.dtsi
> @@ -280,6 +280,15 @@ watchdog@12480000 {
>  				 <&rstgen JH7100_RSTN_WDT>;
>  		};
>
> +		pwm: pwm@12490000 {
> +			compatible = "starfive,jh7100-pwm", "opencores,pwm-v1";
> +			reg = <0x0 0x12490000 0x0 0x10000>;
> +			clocks = <&clkgen JH7100_CLK_PWM_APB>;
> +			resets = <&rstgen JH7100_RSTN_PWM_APB>;
> +			#pwm-cells = <3>;
> +			status = "disabled";
> +		};
> +
>  		sfctemp: temperature-sensor@124a0000 {
>  			compatible = "starfive,jh7100-temp";
>  			reg = <0x0 0x124a0000 0x0 0x10000>;
> --
> 2.34.1
>

Re: [PATCH v10 4/4] riscv: dts: starfive: jh7110: Add PWM node and pins configuration

[Emil Renner Berthing]

William Qiu wrote:
> Add OpenCores PWM controller node and add PWM pins configuration
> on VisionFive 2 board.
>
> Signed-off-by: William Qiu <william.qiu@starfivetech.com>

Reviewed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>

> ---
>  .../jh7110-starfive-visionfive-2.dtsi         | 22 +++++++++++++++++++
>  arch/riscv/boot/dts/starfive/jh7110.dtsi      |  9 ++++++++
>  2 files changed, 31 insertions(+)
>
> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> index b89e9791efa7..e08af8a830ab 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> @@ -323,6 +323,12 @@ reserved-data@600000 {
>  	};
>  };
>
> +&pwm {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pwm_pins>;
> +	status = "okay";
> +};
> +
>  &spi0 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&spi0_pins>;
> @@ -513,6 +519,22 @@ GPOEN_ENABLE,
>  		};
>  	};
>
> +	pwm_pins: pwm-0 {
> +		pwm-pins {
> +			pinmux = <GPIOMUX(46, GPOUT_SYS_PWM_CHANNEL0,
> +					      GPOEN_SYS_PWM0_CHANNEL0,
> +					      GPI_NONE)>,
> +				 <GPIOMUX(59, GPOUT_SYS_PWM_CHANNEL1,
> +					      GPOEN_SYS_PWM0_CHANNEL1,
> +					      GPI_NONE)>;
> +			bias-disable;
> +			drive-strength = <12>;
> +			input-disable;
> +			input-schmitt-disable;
> +			slew-rate = <0>;
> +		};
> +	};
> +
>  	spi0_pins: spi0-0 {
>  		mosi-pins {
>  			pinmux = <GPIOMUX(52, GPOUT_SYS_SPI0_TXD,
> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> index 45213cdf50dc..1b782f2c1395 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> @@ -829,6 +829,15 @@ i2stx1: i2s@120c0000 {
>  			status = "disabled";
>  		};
>
> +		pwm: pwm@120d0000 {
> +			compatible = "starfive,jh7110-pwm", "opencores,pwm-v1";
> +			reg = <0x0 0x120d0000 0x0 0x10000>;
> +			clocks = <&syscrg JH7110_SYSCLK_PWM_APB>;
> +			resets = <&syscrg JH7110_SYSRST_PWM_APB>;
> +			#pwm-cells = <3>;
> +			status = "disabled";
> +		};
> +
>  		sfctemp: temperature-sensor@120e0000 {
>  			compatible = "starfive,jh7110-temp";
>  			reg = <0x0 0x120e0000 0x0 0x10000>;
> --
> 2.34.1
>

Re: [PATCH v10 2/4] pwm: opencores: Add PWM driver support

[William Qiu]

On 2023/12/22 17:45, William Qiu wrote:
> Add driver for OpenCores PWM Controller. And add compatibility code
> which based on StarFive SoC.
> 
> Co-developed-by: Hal Feng <hal.feng@starfivetech.com>
> Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
> ---
>  MAINTAINERS              |   7 ++
>  drivers/pwm/Kconfig      |  12 ++
>  drivers/pwm/Makefile     |   1 +
>  drivers/pwm/pwm-ocores.c | 233 +++++++++++++++++++++++++++++++++++++++
>  4 files changed, 253 insertions(+)
>  create mode 100644 drivers/pwm/pwm-ocores.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 9104430e148e..6a6c355150e7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -16145,6 +16145,13 @@ F:	Documentation/i2c/busses/i2c-ocores.rst
>  F:	drivers/i2c/busses/i2c-ocores.c
>  F:	include/linux/platform_data/i2c-ocores.h
> 
> +OPENCORES PWM DRIVER
> +M:	William Qiu <william.qiu@starfivetech.com>
> +M:	Hal Feng <hal.feng@starfivetech.com>
> +S:	Supported
> +F:	Documentation/devicetree/bindings/pwm/opencores,pwm.yaml
> +F:	drivers/pwm/pwm-ocores.c
> +
>  OPENRISC ARCHITECTURE
>  M:	Jonas Bonn <jonas@southpole.se>
>  M:	Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> index 4b956d661755..d87e1bb350ba 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -444,6 +444,18 @@ config PWM_NTXEC
>  	  controller found in certain e-book readers designed by the original
>  	  design manufacturer Netronix.
> 
> +config PWM_OCORES
> +	tristate "OpenCores PWM support"
> +	depends on HAS_IOMEM && OF
> +	depends on COMMON_CLK
> +	depends on ARCH_STARFIVE || COMPILE_TEST
> +	help
> +	  If you say yes to this option, support will be included for the
> +	  OpenCores PWM. For details see https://opencores.org/projects/ptc.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called pwm-ocores.
> +
>  config PWM_OMAP_DMTIMER
>  	tristate "OMAP Dual-Mode Timer PWM support"
>  	depends on OF
> diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
> index c5ec9e168ee7..517c4f643058 100644
> --- a/drivers/pwm/Makefile
> +++ b/drivers/pwm/Makefile
> @@ -40,6 +40,7 @@ obj-$(CONFIG_PWM_MICROCHIP_CORE)	+= pwm-microchip-core.o
>  obj-$(CONFIG_PWM_MTK_DISP)	+= pwm-mtk-disp.o
>  obj-$(CONFIG_PWM_MXS)		+= pwm-mxs.o
>  obj-$(CONFIG_PWM_NTXEC)		+= pwm-ntxec.o
> +obj-$(CONFIG_PWM_OCORES)	+= pwm-ocores.o
>  obj-$(CONFIG_PWM_OMAP_DMTIMER)	+= pwm-omap-dmtimer.o
>  obj-$(CONFIG_PWM_PCA9685)	+= pwm-pca9685.o
>  obj-$(CONFIG_PWM_PXA)		+= pwm-pxa.o
> diff --git a/drivers/pwm/pwm-ocores.c b/drivers/pwm/pwm-ocores.c
> new file mode 100644
> index 000000000000..dfb5a186da71
> --- /dev/null
> +++ b/drivers/pwm/pwm-ocores.c
> @@ -0,0 +1,233 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * OpenCores PWM Driver
> + *
> + * https://opencores.org/projects/ptc
> + *
> + * Copyright (C) 2018-2023 StarFive Technology Co., Ltd.
> + *
> + * Limitations:
> + * - The hardware only do inverted polarity.
> + * - The hardware minimum period / duty_cycle is (1 / pwm_apb clock frequency) ns.
> + * - The hardware maximum period / duty_cycle is (U32_MAX / pwm_apb clock frequency) ns.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/pwm.h>
> +#include <linux/reset.h>
> +#include <linux/slab.h>
> +
> +/* OCPWM_CTRL register bits*/
> +#define REG_OCPWM_EN      BIT(0)
> +#define REG_OCPWM_ECLK    BIT(1)
> +#define REG_OCPWM_NEC     BIT(2)
> +#define REG_OCPWM_OE      BIT(3)
> +#define REG_OCPWM_SIGNLE  BIT(4)
> +#define REG_OCPWM_INTE    BIT(5)
> +#define REG_OCPWM_INT     BIT(6)
> +#define REG_OCPWM_CNTRRST BIT(7)
> +#define REG_OCPWM_CAPTE   BIT(8)
> +
> +struct ocores_pwm_device {
> +	struct pwm_chip chip;
> +	struct clk *clk;
> +	struct reset_control *rst;
> +	const struct ocores_pwm_data *data;
> +	void __iomem *regs;
> +	u32 clk_rate; /* PWM APB clock frequency */
> +};
> +
> +struct ocores_pwm_data {
> +	void __iomem *(*get_ch_base)(void __iomem *base, unsigned int channel);
> +};
> +
> +static inline u32 ocores_readl(struct ocores_pwm_device *ddata,
> +			       unsigned int channel,
> +			       unsigned int offset)
> +{
> +	void __iomem *base = ddata->data->get_ch_base ?
> +			     ddata->data->get_ch_base(ddata->regs, channel) : ddata->regs;
> +
> +	return readl(base + offset);
> +}
> +
> +static inline void ocores_writel(struct ocores_pwm_device *ddata,
> +				 unsigned int channel,
> +				 unsigned int offset, u32 val)
> +{
> +	void __iomem *base = ddata->data->get_ch_base ?
> +			     ddata->data->get_ch_base(ddata->regs, channel) : ddata->regs;
> +
> +	writel(val, base + offset);
> +}
> +
> +static inline struct ocores_pwm_device *chip_to_ocores(struct pwm_chip *chip)
> +{
> +	return container_of(chip, struct ocores_pwm_device, chip);
> +}
> +
> +static void __iomem *starfive_jh71x0_get_ch_base(void __iomem *base,
> +						 unsigned int channel)
> +{
> +	unsigned int offset = (channel > 3 ? 1 << 15 : 0) + (channel & 3) * 0x10;
> +
> +	return base + offset;
> +}
> +
> +static int ocores_pwm_get_state(struct pwm_chip *chip,
> +				struct pwm_device *pwm,
> +				struct pwm_state *state)
> +{
> +	struct ocores_pwm_device *ddata = chip_to_ocores(chip);
> +	u32 period_data, duty_data, ctrl_data;
> +
> +	period_data = ocores_readl(ddata, pwm->hwpwm, 0x8);
> +	duty_data = ocores_readl(ddata, pwm->hwpwm, 0x4);
> +	ctrl_data = ocores_readl(ddata, pwm->hwpwm, 0xC);
> +
> +	state->period = DIV_ROUND_UP_ULL((u64)period_data * NSEC_PER_SEC, ddata->clk_rate);
> +	state->duty_cycle = DIV_ROUND_UP_ULL((u64)duty_data * NSEC_PER_SEC, ddata->clk_rate);
> +	state->polarity = PWM_POLARITY_INVERSED;
> +	state->enabled = (ctrl_data & REG_OCPWM_EN) ? true : false;
> +
> +	return 0;
> +}
> +
> +static int ocores_pwm_apply(struct pwm_chip *chip,
> +			    struct pwm_device *pwm,
> +			    const struct pwm_state *state)
> +{
> +	struct ocores_pwm_device *ddata = chip_to_ocores(chip);
> +	u32 ctrl_data = 0;
> +	u64 period_data, duty_data;
> +
> +	if (state->polarity != PWM_POLARITY_INVERSED)
> +		return -EINVAL;
> +
> +	ctrl_data = ocores_readl(ddata, pwm->hwpwm, 0xC);
> +	ocores_writel(ddata, pwm->hwpwm, 0xC, 0);
> +
> +	period_data = DIV_ROUND_DOWN_ULL(state->period * ddata->clk_rate, NSEC_PER_SEC);
> +	if (period_data <= U32_MAX)
> +		ocores_writel(ddata, pwm->hwpwm, 0x8, (u32)period_data);
> +	else
> +		return -EINVAL;
> +
> +	duty_data = DIV_ROUND_DOWN_ULL(state->duty_cycle * ddata->clk_rate, NSEC_PER_SEC);
> +	if (duty_data <= U32_MAX)
> +		ocores_writel(ddata, pwm->hwpwm, 0x4, (u32)duty_data);
> +	else
> +		return -EINVAL;
> +
> +	ocores_writel(ddata, pwm->hwpwm, 0xC, 0);
> +
> +	if (state->enabled) {
> +		ctrl_data = ocores_readl(ddata, pwm->hwpwm, 0xC);
> +		ocores_writel(ddata, pwm->hwpwm, 0xC, ctrl_data | REG_OCPWM_EN | REG_OCPWM_OE);
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct pwm_ops ocores_pwm_ops = {
> +	.get_state	= ocores_pwm_get_state,
> +	.apply		= ocores_pwm_apply,
> +};
> +
> +static const struct ocores_pwm_data jh7100_pwm_data = {
> +	.get_ch_base = starfive_jh71x0_get_ch_base,
> +};
> +
> +static const struct ocores_pwm_data jh7110_pwm_data = {
> +	.get_ch_base = starfive_jh71x0_get_ch_base,
> +};
> +
> +static const struct of_device_id ocores_pwm_of_match[] = {
> +	{ .compatible = "opencores,pwm-v1" },
> +	{ .compatible = "starfive,jh7100-pwm", .data = &jh7100_pwm_data},
> +	{ .compatible = "starfive,jh7110-pwm", .data = &jh7110_pwm_data},
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, ocores_pwm_of_match);
> +
> +static void ocores_reset_control_assert(void *data)
> +{
> +	reset_control_assert(data);
> +}
> +
> +static int ocores_pwm_probe(struct platform_device *pdev)
> +{
> +	const struct of_device_id *id;
> +	struct device *dev = &pdev->dev;
> +	struct ocores_pwm_device *ddata;
> +	struct pwm_chip *chip;
> +	int ret;
> +
> +	id = of_match_device(ocores_pwm_of_match, dev);
> +	if (!id)
> +		return -EINVAL;
> +
> +	ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
> +	if (!ddata)
> +		return -ENOMEM;
> +
> +	ddata->data = id->data;
> +	chip = &ddata->chip;
> +	chip->dev = dev;
> +	chip->ops = &ocores_pwm_ops;
> +	chip->npwm = 8;
> +	chip->of_pwm_n_cells = 3;
> +
> +	ddata->regs = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(ddata->regs))
> +		return dev_err_probe(dev, PTR_ERR(ddata->regs),
> +				     "Unable to map IO resources\n");
> +
> +	ddata->clk = devm_clk_get_enabled(dev, NULL);
> +	if (IS_ERR(ddata->clk))
> +		return dev_err_probe(dev, PTR_ERR(ddata->clk),
> +				     "Unable to get pwm's clock\n");
> +
> +	ddata->rst = devm_reset_control_get_optional_exclusive(dev, NULL);
> +	if (IS_ERR(ddata->rst))
> +		return dev_err_probe(dev, PTR_ERR(ddata->rst),
> +				     "Unable to get pwm's reset\n");
> +
> +	reset_control_deassert(ddata->rst);
> +
> +	ret = devm_add_action_or_reset(dev, ocores_reset_control_assert, ddata->rst);
> +	if (ret)
> +		return ret;
> +
> +	ddata->clk_rate = clk_get_rate(ddata->clk);
> +	if (ddata->clk_rate <= 0)
> +		return dev_err_probe(dev, ddata->clk_rate,
> +				     "Unable to get clock's rate\n");
> +
> +	ret = devm_pwmchip_add(dev, chip);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "Could not register PWM chip\n");
> +
> +	platform_set_drvdata(pdev, ddata);
> +
> +	return ret;
> +}
> +
> +static struct platform_driver ocores_pwm_driver = {
> +	.probe = ocores_pwm_probe,
> +	.driver = {
> +		.name = "ocores-pwm",
> +		.of_match_table = ocores_pwm_of_match,
> +	},
> +};
> +module_platform_driver(ocores_pwm_driver);
> +
> +MODULE_AUTHOR("Jieqin Chen");
> +MODULE_AUTHOR("Hal Feng <hal.feng@starfivetech.com>");
> +MODULE_DESCRIPTION("OpenCores PWM PTC driver");
> +MODULE_LICENSE("GPL");
> --
> 2.34.1
> 


Hi Thierry Reding,

Could you please help me review this patch series to see if there is
anything that needs to be modified? If not, could you help me integrate
this patch into the main line? Thanks.
Thanks for taking time to review this patch series.
 
Best Regards,
William

Re: [PATCH v10 2/4] pwm: opencores: Add PWM driver support

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 640 bytes --]

Hello William,

On Wed, Jan 03, 2024 at 03:15:31PM +0800, William Qiu wrote:
> Could you please help me review this patch series to see if there is
> anything that needs to be modified? If not, could you help me integrate
> this patch into the main line? Thanks.
> Thanks for taking time to review this patch series.

It's on my radar. Actually your patch set is on the top of my list. I
won't promise a timely review, but I plan to do it this week.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v10 2/4] pwm: opencores: Add PWM driver support

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1045 bytes --]

Hello again,

On Fri, Dec 22, 2023 at 05:45:46PM +0800, William Qiu wrote:
> +static const struct ocores_pwm_data jh7100_pwm_data = {
> +	.get_ch_base = starfive_jh71x0_get_ch_base,
> +};
> +
> +static const struct ocores_pwm_data jh7110_pwm_data = {
> +	.get_ch_base = starfive_jh71x0_get_ch_base,
> +};
> +
> +static const struct of_device_id ocores_pwm_of_match[] = {
> +	{ .compatible = "opencores,pwm-v1" },
> +	{ .compatible = "starfive,jh7100-pwm", .data = &jh7100_pwm_data},
> +	{ .compatible = "starfive,jh7110-pwm", .data = &jh7110_pwm_data},
> +	{ /* sentinel */ }

Looking at the binding

	compatible = "opencores,pwm-v1";

isn't a valid configuration. If that is indeed the case and you always
have either starfive,jh7100-pwm or starfive,jh7110-pwm, you can drop the
logic to only use starfive_jh71x0_get_ch_base conditionally.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v10 1/4] dt-bindings: pwm: Add bindings for OpenCores PWM Controller

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 676 bytes --]

Hello William,

On Fri, Dec 22, 2023 at 05:45:45PM +0800, William Qiu wrote:
> Add bindings for OpenCores PWM Controller.
> 
> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
> Reviewed-by: Hal Feng <hal.feng@starfivetech.com>
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Looks fine to me. I'll assume you reiterate the series for patch #2 and
so I will mark this patch as deferred in patchwork.

Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v10 3/4] riscv: dts: starfive: jh7100: Add PWM node and pins configuration

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 863 bytes --]

Hello Emil,

On Sun, Dec 24, 2023 at 02:49:34AM -0800, Emil Renner Berthing wrote:
> William Qiu wrote:
> > Add OpenCores PWM controller node and add PWM pins configuration
> > on VisionFive 1 board.
> >
> > Signed-off-by: William Qiu <william.qiu@starfivetech.com>
> 
> Sorry, I thought I already sent my review. This looks good.
> 
> Reviewed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>

Is this also an implicit Ack to take this patch via the pwm tree once
the earlier patches are ready? Or do you want to take it via your tree?
(Maybe already now together with the binding? If so, you can assume my
Reviewed-by to be an implicit Ack for that.)

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

RE: [PATCH v10 2/4] pwm: opencores: Add PWM driver support

[William Qiu]

> -----Original Message-----
> From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Sent: 2024年1月5日 6:41
> To: William Qiu <william.qiu@starfivetech.com>
> Cc: devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-riscv@lists.infradead.org; linux-pwm@vger.kernel.org; Emil Renner
> Berthing <kernel@esmil.dk>; Rob Herring <robh+dt@kernel.org>; Thierry
> Reding <thierry.reding@gmail.com>; Philipp Zabel <p.zabel@pengutronix.de>;
> Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley
> <conor+dt@kernel.org>; Hal Feng <hal.feng@starfivetech.com>; Paul
> Walmsley <paul.walmsley@sifive.com>; Palmer Dabbelt
> <palmer@dabbelt.com>; Albert Ou <aou@eecs.berkeley.edu>
> Subject: Re: [PATCH v10 2/4] pwm: opencores: Add PWM driver support
> 
> Hello again,
> 
> On Fri, Dec 22, 2023 at 05:45:46PM +0800, William Qiu wrote:
> > +static const struct ocores_pwm_data jh7100_pwm_data = {
> > +	.get_ch_base = starfive_jh71x0_get_ch_base, };
> > +
> > +static const struct ocores_pwm_data jh7110_pwm_data = {
> > +	.get_ch_base = starfive_jh71x0_get_ch_base, };
> > +
> > +static const struct of_device_id ocores_pwm_of_match[] = {
> > +	{ .compatible = "opencores,pwm-v1" },
> > +	{ .compatible = "starfive,jh7100-pwm", .data = &jh7100_pwm_data},
> > +	{ .compatible = "starfive,jh7110-pwm", .data = &jh7110_pwm_data},
> > +	{ /* sentinel */ }
> 
> Looking at the binding
> 
> 	compatible = "opencores,pwm-v1";
> 
> isn't a valid configuration. If that is indeed the case and you always have either
> starfive,jh7100-pwm or starfive,jh7110-pwm, you can drop the logic to only use
> starfive_jh71x0_get_ch_base conditionally.
> 
> Best regards
> Uwe
> 
> --
> Pengutronix e.K.                           | Uwe Kleine-König
> |
> Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Hi Uwe,

I just upload code for OpenCores.. And I need to add the starfive_jh71x0_get_ch_base
to compatibility with jh71x0. So even the "opencores,pwm-v1" isn't a valid
configuration, but I still need to add it for OpenCores.

Best Regards,
William

Re: [PATCH v10 3/4] riscv: dts: starfive: jh7100: Add PWM node and pins configuration

[Emil Renner Berthing]

Uwe Kleine-König wrote:
> Hello Emil,
>
> On Sun, Dec 24, 2023 at 02:49:34AM -0800, Emil Renner Berthing wrote:
> > William Qiu wrote:
> > > Add OpenCores PWM controller node and add PWM pins configuration
> > > on VisionFive 1 board.
> > >
> > > Signed-off-by: William Qiu <william.qiu@starfivetech.com>
> >
> > Sorry, I thought I already sent my review. This looks good.
> >
> > Reviewed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
>
> Is this also an implicit Ack to take this patch via the pwm tree once
> the earlier patches are ready? Or do you want to take it via your tree?
> (Maybe already now together with the binding? If so, you can assume my
> Reviewed-by to be an implicit Ack for that.)

Yes, sorry. This is also meant to be an Ack from me.

I imagined the dt patches would go through Conor's riscv-dt-for-next branch,
but the pwm tree is certainly also fine by.

/Emil

Re: [PATCH v10 3/4] riscv: dts: starfive: jh7100: Add PWM node and pins configuration

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1362 bytes --]

On Fri, Jan 05, 2024 at 05:18:45AM -0800, Emil Renner Berthing wrote:
> Uwe Kleine-König wrote:
> > Hello Emil,
> >
> > On Sun, Dec 24, 2023 at 02:49:34AM -0800, Emil Renner Berthing wrote:
> > > William Qiu wrote:
> > > > Add OpenCores PWM controller node and add PWM pins configuration
> > > > on VisionFive 1 board.
> > > >
> > > > Signed-off-by: William Qiu <william.qiu@starfivetech.com>
> > >
> > > Sorry, I thought I already sent my review. This looks good.
> > >
> > > Reviewed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
> >
> > Is this also an implicit Ack to take this patch via the pwm tree once
> > the earlier patches are ready? Or do you want to take it via your tree?
> > (Maybe already now together with the binding? If so, you can assume my
> > Reviewed-by to be an implicit Ack for that.)
> 
> Yes, sorry. This is also meant to be an Ack from me.
> 
> I imagined the dt patches would go through Conor's riscv-dt-for-next branch,
> but the pwm tree is certainly also fine by.

idk, I prefer things to go as MAINTAINERS indicates, in case something
is determined to be wrong in the cycle where the patch is in the "wrong"
tree.

I suppose I could take the binding though, since I am CCed on every
binding patch under the sun... I'd rather an explicit ack in that case
though.

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v10 1/4] dt-bindings: pwm: Add bindings for OpenCores PWM Controller

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 926 bytes --]

Hello Conor,

On Thu, Jan 04, 2024 at 11:43:13PM +0100, Uwe Kleine-König wrote:
> On Fri, Dec 22, 2023 at 05:45:45PM +0800, William Qiu wrote:
> > Add bindings for OpenCores PWM Controller.
> > 
> > Signed-off-by: William Qiu <william.qiu@starfivetech.com>
> > Reviewed-by: Hal Feng <hal.feng@starfivetech.com>
> > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> 
> Looks fine to me. I'll assume you reiterate the series for patch #2 and
> so I will mark this patch as deferred in patchwork.
> 
> Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

If you want, pick this patch up that it goes along with the dts changes.

To make this formal:

Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v10 1/4] dt-bindings: pwm: Add bindings for OpenCores PWM Controller

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 919 bytes --]

On Mon, Jan 08, 2024 at 10:03:49PM +0100, Uwe Kleine-König wrote:
> Hello Conor,
> 
> On Thu, Jan 04, 2024 at 11:43:13PM +0100, Uwe Kleine-König wrote:
> > On Fri, Dec 22, 2023 at 05:45:45PM +0800, William Qiu wrote:
> > > Add bindings for OpenCores PWM Controller.
> > > 
> > > Signed-off-by: William Qiu <william.qiu@starfivetech.com>
> > > Reviewed-by: Hal Feng <hal.feng@starfivetech.com>
> > > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> > 
> > Looks fine to me. I'll assume you reiterate the series for patch #2 and
> > so I will mark this patch as deferred in patchwork.
> > 
> > Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> 
> If you want, pick this patch up that it goes along with the dts changes.
> 
> To make this formal:
> 
> Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Cool. I'll do that after the merge window closes :)

Thanks!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: (subset) [PATCH v10 0/4] StarFive's Pulse Width Modulation driver support

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

On Fri, 22 Dec 2023 17:45:44 +0800, William Qiu wrote:
> This patchset adds initial rudimentary support for the StarFive
> Pulse Width Modulation controller driver. And this driver will
> be used in StarFive's VisionFive 2 board.The first patch add
> Documentations for the device and Patch 2 adds device probe for
> the module.
> 
> Changes v9->v10:
> - Rebased to v6.7rc6.
> - Dropped unuseful dependency.
> - Added error handling.
> 
> [...]

Applied to riscv-dt-for-next, thanks!

[1/4] dt-bindings: pwm: Add bindings for OpenCores PWM Controller
      https://git.kernel.org/conor/c/2529085831b0
[3/4] riscv: dts: starfive: jh7100: Add PWM node and pins configuration
      https://git.kernel.org/conor/c/26c3112c10f8
[4/4] riscv: dts: starfive: jh7110: Add PWM node and pins configuration
      https://git.kernel.org/conor/c/92df97487208

Thanks,
Conor.

Re: (subset) [PATCH v10 0/4] StarFive's Pulse Width Modulation driver support

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1240 bytes --]

On Mon, Jan 22, 2024 at 04:50:25PM +0000, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> On Fri, 22 Dec 2023 17:45:44 +0800, William Qiu wrote:
> > This patchset adds initial rudimentary support for the StarFive
> > Pulse Width Modulation controller driver. And this driver will
> > be used in StarFive's VisionFive 2 board.The first patch add
> > Documentations for the device and Patch 2 adds device probe for
> > the module.
> > 
> > Changes v9->v10:
> > - Rebased to v6.7rc6.
> > - Dropped unuseful dependency.
> > - Added error handling.
> > 
> > [...]
> 
> Applied to riscv-dt-for-next, thanks!
> 
> [1/4] dt-bindings: pwm: Add bindings for OpenCores PWM Controller
>       https://git.kernel.org/conor/c/2529085831b0
> [3/4] riscv: dts: starfive: jh7100: Add PWM node and pins configuration
>       https://git.kernel.org/conor/c/26c3112c10f8
> [4/4] riscv: dts: starfive: jh7110: Add PWM node and pins configuration
>       https://git.kernel.org/conor/c/92df97487208

Something went super wrong here with the CC list that b4 generated for
me, there was a bunch of utf8 encoding crap in the middle of it. Perhaps
I should update the version of b4 that I have been running...

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]