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

[PATCH v7 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 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.6.

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

 .../bindings/pwm/opencores,pwm.yaml           |  56 +++++
 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                      | 236 ++++++++++++++++++
 9 files changed, 376 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/opencores,pwm.yaml
 create mode 100644 drivers/pwm/pwm-ocores.c

--
2.34.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v7 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 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.6.

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

 .../bindings/pwm/opencores,pwm.yaml           |  56 +++++
 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                      | 236 ++++++++++++++++++
 9 files changed, 376 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/opencores,pwm.yaml
 create mode 100644 drivers/pwm/pwm-ocores.c

--
2.34.1

[PATCH v7 1/4] dt-bindings: pwm: Add OpenCores PWM module

[William Qiu]

Add documentation to describe OpenCores Pulse Width Modulation
controller driver.

Signed-off-by: William Qiu <william.qiu@starfivetech.com>
Reviewed-by: Hal Feng <hal.feng@starfivetech.com>
---
 .../bindings/pwm/opencores,pwm.yaml           | 56 +++++++++++++++++++
 1 file changed, 56 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..8f776bbc1112
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/opencores,pwm.yaml
@@ -0,0 +1,56 @@
+# 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:
+  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:
+    oneOf:
+      - items:
+          - enum:
+              - starfive,jh7100-pwm
+              - starfive,jh7110-pwm
+          - const: opencores,pwm
+
+  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";
+        reg = <0x12490000 0x10000>;
+        clocks = <&clkgen 181>;
+        resets = <&rstgen 109>;
+        #pwm-cells = <3>;
+    };
-- 
2.34.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v7 1/4] dt-bindings: pwm: Add OpenCores PWM module

[William Qiu]

Add documentation to describe OpenCores Pulse Width Modulation
controller driver.

Signed-off-by: William Qiu <william.qiu@starfivetech.com>
Reviewed-by: Hal Feng <hal.feng@starfivetech.com>
---
 .../bindings/pwm/opencores,pwm.yaml           | 56 +++++++++++++++++++
 1 file changed, 56 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..8f776bbc1112
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/opencores,pwm.yaml
@@ -0,0 +1,56 @@
+# 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:
+  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:
+    oneOf:
+      - items:
+          - enum:
+              - starfive,jh7100-pwm
+              - starfive,jh7110-pwm
+          - const: opencores,pwm
+
+  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";
+        reg = <0x12490000 0x10000>;
+        clocks = <&clkgen 181>;
+        resets = <&rstgen 109>;
+        #pwm-cells = <3>;
+    };
-- 
2.34.1

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

[William Qiu]

Add Pulse Width Modulation driver support for OpenCores.

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 | 236 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 256 insertions(+)
 create mode 100644 drivers/pwm/pwm-ocores.c

diff --git a/MAINTAINERS b/MAINTAINERS
index dd5de540ec0b..1861067cf385 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16006,6 +16006,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-ocores.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 8ebcddf91f7b..a9db2d7ab294 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -434,6 +434,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 && RESET_CONTROLLER
+	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 c822389c2a24..542b98202153 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -39,6 +39,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..4b98a152da94
--- /dev/null
+++ b/drivers/pwm/pwm-ocores.c
@@ -0,0 +1,236 @@
+// 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" },
+	{ .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 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(dev, NULL);
+	if (IS_ERR(ddata->clk))
+		return dev_err_probe(dev, PTR_ERR(ddata->clk),
+				     "Unable to get pwm's clock\n");
+
+	ret = clk_prepare_enable(ddata->clk);
+	if (ret)
+		return dev_err_probe(dev, ret, "Clock enable failed\n");
+
+	ddata->rst = devm_reset_control_get_optional_exclusive(dev, NULL);
+	reset_control_deassert(ddata->rst);
+
+	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) {
+		dev_err_probe(dev, ret, "Could not register PWM chip\n");
+		clk_disable_unprepare(ddata->clk);
+		reset_control_assert(ddata->rst);
+	}
+
+	platform_set_drvdata(pdev, ddata);
+
+	return ret;
+}
+
+static void ocores_pwm_remove(struct platform_device *dev)
+{
+	struct ocores_pwm_device *ddata = platform_get_drvdata(dev);
+
+	reset_control_assert(ddata->rst);
+	clk_disable_unprepare(ddata->clk);
+}
+
+static struct platform_driver ocores_pwm_driver = {
+	.probe = ocores_pwm_probe,
+	.remove_new = ocores_pwm_remove,
+	.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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[William Qiu]

Add Pulse Width Modulation driver support for OpenCores.

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 | 236 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 256 insertions(+)
 create mode 100644 drivers/pwm/pwm-ocores.c

diff --git a/MAINTAINERS b/MAINTAINERS
index dd5de540ec0b..1861067cf385 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16006,6 +16006,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-ocores.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 8ebcddf91f7b..a9db2d7ab294 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -434,6 +434,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 && RESET_CONTROLLER
+	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 c822389c2a24..542b98202153 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -39,6 +39,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..4b98a152da94
--- /dev/null
+++ b/drivers/pwm/pwm-ocores.c
@@ -0,0 +1,236 @@
+// 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" },
+	{ .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 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(dev, NULL);
+	if (IS_ERR(ddata->clk))
+		return dev_err_probe(dev, PTR_ERR(ddata->clk),
+				     "Unable to get pwm's clock\n");
+
+	ret = clk_prepare_enable(ddata->clk);
+	if (ret)
+		return dev_err_probe(dev, ret, "Clock enable failed\n");
+
+	ddata->rst = devm_reset_control_get_optional_exclusive(dev, NULL);
+	reset_control_deassert(ddata->rst);
+
+	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) {
+		dev_err_probe(dev, ret, "Could not register PWM chip\n");
+		clk_disable_unprepare(ddata->clk);
+		reset_control_assert(ddata->rst);
+	}
+
+	platform_set_drvdata(pdev, ddata);
+
+	return ret;
+}
+
+static void ocores_pwm_remove(struct platform_device *dev)
+{
+	struct ocores_pwm_device *ddata = platform_get_drvdata(dev);
+
+	reset_control_assert(ddata->rst);
+	clk_disable_unprepare(ddata->clk);
+}
+
+static struct platform_driver ocores_pwm_driver = {
+	.probe = ocores_pwm_probe,
+	.remove_new = ocores_pwm_remove,
+	.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 v7 3/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>
Reviewed-by: Hal Feng <hal.feng@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 2c02358abd71..823f298c3f4c 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
@@ -268,6 +268,12 @@ reserved-data@600000 {
 	};
 };
 
+&pwm {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pwm_pins>;
+	status = "okay";
+};
+
 &spi0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&spi0_pins>;
@@ -402,6 +408,22 @@ GPOEN_SYS_SDIO1_DATA3,
 		};
 	};
 
+	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 e85464c328d0..74630feff7a8 100644
--- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
@@ -736,6 +736,15 @@ spi6: spi@120a0000 {
 			status = "disabled";
 		};
 
+		pwm: pwm@120d0000 {
+			compatible = "starfive,jh7110-pwm", "opencores,pwm";
+			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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v7 3/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>
Reviewed-by: Hal Feng <hal.feng@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 2c02358abd71..823f298c3f4c 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
@@ -268,6 +268,12 @@ reserved-data@600000 {
 	};
 };
 
+&pwm {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pwm_pins>;
+	status = "okay";
+};
+
 &spi0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&spi0_pins>;
@@ -402,6 +408,22 @@ GPOEN_SYS_SDIO1_DATA3,
 		};
 	};
 
+	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 e85464c328d0..74630feff7a8 100644
--- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
@@ -736,6 +736,15 @@ spi6: spi@120a0000 {
 			status = "disabled";
 		};
 
+		pwm: pwm@120d0000 {
+			compatible = "starfive,jh7110-pwm", "opencores,pwm";
+			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

[PATCH v7 4/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>
Reviewed-by: Hal Feng <hal.feng@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 35ab54fb235f..9a7192b0684c 100644
--- a/arch/riscv/boot/dts/starfive/jh7100.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7100.dtsi
@@ -274,6 +274,15 @@ watchdog@12480000 {
 				 <&rstgen JH7100_RSTN_WDT>;
 		};
 
+		pwm: pwm@12490000 {
+			compatible = "starfive,jh7100-pwm", "opencores,pwm";
+			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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v7 4/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>
Reviewed-by: Hal Feng <hal.feng@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 35ab54fb235f..9a7192b0684c 100644
--- a/arch/riscv/boot/dts/starfive/jh7100.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7100.dtsi
@@ -274,6 +274,15 @@ watchdog@12480000 {
 				 <&rstgen JH7100_RSTN_WDT>;
 		};
 
+		pwm: pwm@12490000 {
+			compatible = "starfive,jh7100-pwm", "opencores,pwm";
+			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 v7 1/4] dt-bindings: pwm: Add OpenCores PWM module

[Krzysztof Kozlowski]

On 10/11/2023 07:20, William Qiu wrote:
> Add documentation to describe OpenCores Pulse Width Modulation
> controller driver.
> 
> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
> Reviewed-by: Hal Feng <hal.feng@starfivetech.com>
> ---
>  .../bindings/pwm/opencores,pwm.yaml           | 56 +++++++++++++++++++
>  1 file changed, 56 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..8f776bbc1112
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pwm/opencores,pwm.yaml
> @@ -0,0 +1,56 @@
> +# 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:
> +  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.

Wrap at 80 (as Coding Style asks)

> +
> +allOf:
> +  - $ref: pwm.yaml#
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +          - enum:
> +              - starfive,jh7100-pwm
> +              - starfive,jh7110-pwm
> +          - const: opencores,pwm

That's a very, very generic compatible. Are you sure, 100% sure, that
all designs from OpenCores from now till next 100 years will be 100%
compatible?


Best regards,
Krzysztof

Re: [PATCH v7 1/4] dt-bindings: pwm: Add OpenCores PWM module

[Krzysztof Kozlowski]

On 10/11/2023 07:20, William Qiu wrote:
> Add documentation to describe OpenCores Pulse Width Modulation
> controller driver.
> 
> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
> Reviewed-by: Hal Feng <hal.feng@starfivetech.com>
> ---
>  .../bindings/pwm/opencores,pwm.yaml           | 56 +++++++++++++++++++
>  1 file changed, 56 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..8f776bbc1112
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pwm/opencores,pwm.yaml
> @@ -0,0 +1,56 @@
> +# 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:
> +  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.

Wrap at 80 (as Coding Style asks)

> +
> +allOf:
> +  - $ref: pwm.yaml#
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +          - enum:
> +              - starfive,jh7100-pwm
> +              - starfive,jh7110-pwm
> +          - const: opencores,pwm

That's a very, very generic compatible. Are you sure, 100% sure, that
all designs from OpenCores from now till next 100 years will be 100%
compatible?


Best regards,
Krzysztof


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v7 1/4] dt-bindings: pwm: Add OpenCores PWM module

[Krzysztof Kozlowski]

On 10/11/2023 07:20, William Qiu wrote:
> Add documentation to describe OpenCores Pulse Width Modulation
> controller driver.

Please describe the hardware, not the driver.

Best regards,
Krzysztof

Re: [PATCH v7 1/4] dt-bindings: pwm: Add OpenCores PWM module

[Krzysztof Kozlowski]

On 10/11/2023 07:20, William Qiu wrote:
> Add documentation to describe OpenCores Pulse Width Modulation
> controller driver.

Please describe the hardware, not the driver.

Best regards,
Krzysztof


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Krzysztof Kozlowski]

On 10/11/2023 07:20, William Qiu wrote:
> Add Pulse Width Modulation driver support for OpenCores.

What is OpenCores? Why all your commit messages lack basic explanation
of the hardware you are working on?

> 


> +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" },
> +	{ .compatible = "starfive,jh7100-pwm", .data = &jh7100_pwm_data},
> +	{ .compatible = "starfive,jh7110-pwm", .data = &jh7110_pwm_data},

Your bindings say something entirely else.

I don't understand what is happening with this patchset.


> +	{ /* sentinel */ }
> +};

...

> +	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(dev, NULL);
> +	if (IS_ERR(ddata->clk))
> +		return dev_err_probe(dev, PTR_ERR(ddata->clk),
> +				     "Unable to get pwm's clock\n");
> +
> +	ret = clk_prepare_enable(ddata->clk);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Clock enable failed\n");

dev_clk_get_enabled() or whatever the API is called

> +
> +	ddata->rst = devm_reset_control_get_optional_exclusive(dev, NULL);
> +	reset_control_deassert(ddata->rst);
> +
> +	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) {
> +		dev_err_probe(dev, ret, "Could not register PWM chip\n");
> +		clk_disable_unprepare(ddata->clk);
> +		reset_control_assert(ddata->rst);

return dev_err_probe

> +	}
> +
> +	platform_set_drvdata(pdev, ddata);
> +
> +	return ret;
> +}
> +
> +static void ocores_pwm_remove(struct platform_device *dev)
> +{
> +	struct ocores_pwm_device *ddata = platform_get_drvdata(dev);
> +
> +	reset_control_assert(ddata->rst);
> +	clk_disable_unprepare(ddata->clk);

You have confusing order of cleanups. It's like random, once clock then
reset, in other place reset then clock.

Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

On 10/11/2023 07:20, William Qiu wrote:
> Add Pulse Width Modulation driver support for OpenCores.

What is OpenCores? Why all your commit messages lack basic explanation
of the hardware you are working on?

> 


> +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" },
> +	{ .compatible = "starfive,jh7100-pwm", .data = &jh7100_pwm_data},
> +	{ .compatible = "starfive,jh7110-pwm", .data = &jh7110_pwm_data},

Your bindings say something entirely else.

I don't understand what is happening with this patchset.


> +	{ /* sentinel */ }
> +};

...

> +	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(dev, NULL);
> +	if (IS_ERR(ddata->clk))
> +		return dev_err_probe(dev, PTR_ERR(ddata->clk),
> +				     "Unable to get pwm's clock\n");
> +
> +	ret = clk_prepare_enable(ddata->clk);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Clock enable failed\n");

dev_clk_get_enabled() or whatever the API is called

> +
> +	ddata->rst = devm_reset_control_get_optional_exclusive(dev, NULL);
> +	reset_control_deassert(ddata->rst);
> +
> +	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) {
> +		dev_err_probe(dev, ret, "Could not register PWM chip\n");
> +		clk_disable_unprepare(ddata->clk);
> +		reset_control_assert(ddata->rst);

return dev_err_probe

> +	}
> +
> +	platform_set_drvdata(pdev, ddata);
> +
> +	return ret;
> +}
> +
> +static void ocores_pwm_remove(struct platform_device *dev)
> +{
> +	struct ocores_pwm_device *ddata = platform_get_drvdata(dev);
> +
> +	reset_control_assert(ddata->rst);
> +	clk_disable_unprepare(ddata->clk);

You have confusing order of cleanups. It's like random, once clock then
reset, in other place reset then clock.

Best regards,
Krzysztof


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[kernel test robot]

Hi William,

kernel test robot noticed the following build warnings:

[auto build test WARNING on robh/for-next]
[also build test WARNING on thierry-reding-pwm/for-next linus/master v6.6 next-20231110]
[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/William-Qiu/dt-bindings-pwm-Add-OpenCores-PWM-module/20231110-142301
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/20231110062039.103339-3-william.qiu%40starfivetech.com
patch subject: [PATCH v7 2/4] pwm: opencores: Add PWM driver support
reproduce: (https://download.01.org/0day-ci/archive/20231111/202311112141.UCOa6ZV5-lkp@intel.com/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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311112141.UCOa6ZV5-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> Warning: MAINTAINERS references a file that doesn't exist: Documentation/devicetree/bindings/pwm/opencores,pwm-ocores.yaml

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

Re: [PATCH v7 1/4] dt-bindings: pwm: Add OpenCores PWM module

[William Qiu]

On 2023/11/10 20:24, Krzysztof Kozlowski wrote:
> On 10/11/2023 07:20, William Qiu wrote:
>> Add documentation to describe OpenCores Pulse Width Modulation
>> controller driver.
>> 
>> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
>> Reviewed-by: Hal Feng <hal.feng@starfivetech.com>
>> ---
>>  .../bindings/pwm/opencores,pwm.yaml           | 56 +++++++++++++++++++
>>  1 file changed, 56 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..8f776bbc1112
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pwm/opencores,pwm.yaml
>> @@ -0,0 +1,56 @@
>> +# 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:
>> +  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.
> 
> Wrap at 80 (as Coding Style asks)
> 
Will update.
>> +
>> +allOf:
>> +  - $ref: pwm.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    oneOf:
>> +      - items:
>> +          - enum:
>> +              - starfive,jh7100-pwm
>> +              - starfive,jh7110-pwm
>> +          - const: opencores,pwm
> 
> That's a very, very generic compatible. Are you sure, 100% sure, that
> all designs from OpenCores from now till next 100 years will be 100%
> compatible?
> 
My description is not accurate enough, this is OpenCores PTC IP, and PWM
is one of those modes, so it might be better to replace compatible with
"opencores, ptc-pwm"

What do you think?

Best Regrads,
William
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v7 1/4] dt-bindings: pwm: Add OpenCores PWM module

[William Qiu]

On 2023/11/10 20:24, Krzysztof Kozlowski wrote:
> On 10/11/2023 07:20, William Qiu wrote:
>> Add documentation to describe OpenCores Pulse Width Modulation
>> controller driver.
>> 
>> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
>> Reviewed-by: Hal Feng <hal.feng@starfivetech.com>
>> ---
>>  .../bindings/pwm/opencores,pwm.yaml           | 56 +++++++++++++++++++
>>  1 file changed, 56 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..8f776bbc1112
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pwm/opencores,pwm.yaml
>> @@ -0,0 +1,56 @@
>> +# 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:
>> +  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.
> 
> Wrap at 80 (as Coding Style asks)
> 
Will update.
>> +
>> +allOf:
>> +  - $ref: pwm.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    oneOf:
>> +      - items:
>> +          - enum:
>> +              - starfive,jh7100-pwm
>> +              - starfive,jh7110-pwm
>> +          - const: opencores,pwm
> 
> That's a very, very generic compatible. Are you sure, 100% sure, that
> all designs from OpenCores from now till next 100 years will be 100%
> compatible?
> 
My description is not accurate enough, this is OpenCores PTC IP, and PWM
is one of those modes, so it might be better to replace compatible with
"opencores, ptc-pwm"

What do you think?

Best Regrads,
William
> 
> Best regards,
> Krzysztof
> 

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v7 1/4] dt-bindings: pwm: Add OpenCores PWM module

[William Qiu]

On 2023/11/10 20:24, Krzysztof Kozlowski wrote:
> On 10/11/2023 07:20, William Qiu wrote:
>> Add documentation to describe OpenCores Pulse Width Modulation
>> controller driver.
> 
> Please describe the hardware, not the driver.
> 
Will modify the description.

Thanks.

Best Regards,
William
> Best regards,
> Krzysztof
> 

Re: [PATCH v7 1/4] dt-bindings: pwm: Add OpenCores PWM module

[William Qiu]

On 2023/11/10 20:24, Krzysztof Kozlowski wrote:
> On 10/11/2023 07:20, William Qiu wrote:
>> Add documentation to describe OpenCores Pulse Width Modulation
>> controller driver.
> 
> Please describe the hardware, not the driver.
> 
Will modify the description.

Thanks.

Best Regards,
William
> Best regards,
> Krzysztof
> 

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[William Qiu]

On 2023/11/10 20:27, Krzysztof Kozlowski wrote:
> On 10/11/2023 07:20, William Qiu wrote:
>> Add Pulse Width Modulation driver support for OpenCores.
> 
> What is OpenCores? Why all your commit messages lack basic explanation
> of the hardware you are working on?
> 
Will modify.
>> 
> 
> 
>> +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" },
>> +	{ .compatible = "starfive,jh7100-pwm", .data = &jh7100_pwm_data},
>> +	{ .compatible = "starfive,jh7110-pwm", .data = &jh7110_pwm_data},
> 
> Your bindings say something entirely else.
> 
> I don't understand what is happening with this patchset.
> 
Will update.
> 
>> +	{ /* sentinel */ }
>> +};
> 
> ...
> 
>> +	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(dev, NULL);
>> +	if (IS_ERR(ddata->clk))
>> +		return dev_err_probe(dev, PTR_ERR(ddata->clk),
>> +				     "Unable to get pwm's clock\n");
>> +
>> +	ret = clk_prepare_enable(ddata->clk);
>> +	if (ret)
>> +		return dev_err_probe(dev, ret, "Clock enable failed\n");
> 
> dev_clk_get_enabled() or whatever the API is called
>
You mean change devm_clk_get_enabled() instead of devm_clk_get()?
>> +
>> +	ddata->rst = devm_reset_control_get_optional_exclusive(dev, NULL);
>> +	reset_control_deassert(ddata->rst);
>> +
>> +	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) {
>> +		dev_err_probe(dev, ret, "Could not register PWM chip\n");
>> +		clk_disable_unprepare(ddata->clk);
>> +		reset_control_assert(ddata->rst);
> 
> return dev_err_probe
> 
Will update.
>> +	}
>> +
>> +	platform_set_drvdata(pdev, ddata);
>> +
>> +	return ret;
>> +}
>> +
>> +static void ocores_pwm_remove(struct platform_device *dev)
>> +{
>> +	struct ocores_pwm_device *ddata = platform_get_drvdata(dev);
>> +
>> +	reset_control_assert(ddata->rst);
>> +	clk_disable_unprepare(ddata->clk);
> 
> You have confusing order of cleanups. It's like random, once clock then
> reset, in other place reset then clock.
> 
Will change it into a reasonable order.

Thank you for spending time on this patchset.

Best Regards,
William
> Best regards,
> Krzysztof
> 

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

[William Qiu]

On 2023/11/10 20:27, Krzysztof Kozlowski wrote:
> On 10/11/2023 07:20, William Qiu wrote:
>> Add Pulse Width Modulation driver support for OpenCores.
> 
> What is OpenCores? Why all your commit messages lack basic explanation
> of the hardware you are working on?
> 
Will modify.
>> 
> 
> 
>> +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" },
>> +	{ .compatible = "starfive,jh7100-pwm", .data = &jh7100_pwm_data},
>> +	{ .compatible = "starfive,jh7110-pwm", .data = &jh7110_pwm_data},
> 
> Your bindings say something entirely else.
> 
> I don't understand what is happening with this patchset.
> 
Will update.
> 
>> +	{ /* sentinel */ }
>> +};
> 
> ...
> 
>> +	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(dev, NULL);
>> +	if (IS_ERR(ddata->clk))
>> +		return dev_err_probe(dev, PTR_ERR(ddata->clk),
>> +				     "Unable to get pwm's clock\n");
>> +
>> +	ret = clk_prepare_enable(ddata->clk);
>> +	if (ret)
>> +		return dev_err_probe(dev, ret, "Clock enable failed\n");
> 
> dev_clk_get_enabled() or whatever the API is called
>
You mean change devm_clk_get_enabled() instead of devm_clk_get()?
>> +
>> +	ddata->rst = devm_reset_control_get_optional_exclusive(dev, NULL);
>> +	reset_control_deassert(ddata->rst);
>> +
>> +	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) {
>> +		dev_err_probe(dev, ret, "Could not register PWM chip\n");
>> +		clk_disable_unprepare(ddata->clk);
>> +		reset_control_assert(ddata->rst);
> 
> return dev_err_probe
> 
Will update.
>> +	}
>> +
>> +	platform_set_drvdata(pdev, ddata);
>> +
>> +	return ret;
>> +}
>> +
>> +static void ocores_pwm_remove(struct platform_device *dev)
>> +{
>> +	struct ocores_pwm_device *ddata = platform_get_drvdata(dev);
>> +
>> +	reset_control_assert(ddata->rst);
>> +	clk_disable_unprepare(ddata->clk);
> 
> You have confusing order of cleanups. It's like random, once clock then
> reset, in other place reset then clock.
> 
Will change it into a reasonable order.

Thank you for spending time on this patchset.

Best Regards,
William
> Best regards,
> Krzysztof
> 

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v7 1/4] dt-bindings: pwm: Add OpenCores PWM module

[Krzysztof Kozlowski]

On 13/11/2023 10:42, William Qiu wrote:
> Will update.
>>> +
>>> +allOf:
>>> +  - $ref: pwm.yaml#
>>> +
>>> +properties:
>>> +  compatible:
>>> +    oneOf:
>>> +      - items:
>>> +          - enum:
>>> +              - starfive,jh7100-pwm
>>> +              - starfive,jh7110-pwm
>>> +          - const: opencores,pwm
>>
>> That's a very, very generic compatible. Are you sure, 100% sure, that
>> all designs from OpenCores from now till next 100 years will be 100%
>> compatible?
>>
> My description is not accurate enough, this is OpenCores PTC IP, and PWM
> is one of those modes, so it might be better to replace compatible with
> "opencores, ptc-pwm"
> 
> What do you think?

Sorry, maybe this answers maybe doesn't. What is "PTC"?

Best regards,
Krzysztof

Re: [PATCH v7 1/4] dt-bindings: pwm: Add OpenCores PWM module

[Krzysztof Kozlowski]

On 13/11/2023 10:42, William Qiu wrote:
> Will update.
>>> +
>>> +allOf:
>>> +  - $ref: pwm.yaml#
>>> +
>>> +properties:
>>> +  compatible:
>>> +    oneOf:
>>> +      - items:
>>> +          - enum:
>>> +              - starfive,jh7100-pwm
>>> +              - starfive,jh7110-pwm
>>> +          - const: opencores,pwm
>>
>> That's a very, very generic compatible. Are you sure, 100% sure, that
>> all designs from OpenCores from now till next 100 years will be 100%
>> compatible?
>>
> My description is not accurate enough, this is OpenCores PTC IP, and PWM
> is one of those modes, so it might be better to replace compatible with
> "opencores, ptc-pwm"
> 
> What do you think?

Sorry, maybe this answers maybe doesn't. What is "PTC"?

Best regards,
Krzysztof


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v7 1/4] dt-bindings: pwm: Add OpenCores PWM module

[Conor Dooley]

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

On Mon, Nov 13, 2023 at 09:07:15PM +0100, Krzysztof Kozlowski wrote:
> On 13/11/2023 10:42, William Qiu wrote:
> > Will update.
> >>> +
> >>> +allOf:
> >>> +  - $ref: pwm.yaml#
> >>> +
> >>> +properties:
> >>> +  compatible:
> >>> +    oneOf:
> >>> +      - items:
> >>> +          - enum:
> >>> +              - starfive,jh7100-pwm
> >>> +              - starfive,jh7110-pwm
> >>> +          - const: opencores,pwm
> >>
> >> That's a very, very generic compatible. Are you sure, 100% sure, that
> >> all designs from OpenCores from now till next 100 years will be 100%
> >> compatible?
> >>
> > My description is not accurate enough, this is OpenCores PTC IP, and PWM
> > is one of those modes, so it might be better to replace compatible with
> > "opencores, ptc-pwm"
> > 
> > What do you think?
> 
> Sorry, maybe this answers maybe doesn't. What is "PTC"?

"pwm timer counter". AFAIU, the IP can be configured to provide all 3.
I think that William pointed out on an earlier revision that they have
only implemented the pwm on their hardware.
I don't think putting in "ptc" is a sufficient differentiator though, as
clearly there could be several different versions of "ptc-pwm" that have
the same concern about "all designs from OpenCores for now till the next
100 years" being compatible.

Cheers.
Conor.

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

Re: [PATCH v7 1/4] dt-bindings: pwm: Add OpenCores PWM module

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 1348 bytes --]

On Mon, Nov 13, 2023 at 09:07:15PM +0100, Krzysztof Kozlowski wrote:
> On 13/11/2023 10:42, William Qiu wrote:
> > Will update.
> >>> +
> >>> +allOf:
> >>> +  - $ref: pwm.yaml#
> >>> +
> >>> +properties:
> >>> +  compatible:
> >>> +    oneOf:
> >>> +      - items:
> >>> +          - enum:
> >>> +              - starfive,jh7100-pwm
> >>> +              - starfive,jh7110-pwm
> >>> +          - const: opencores,pwm
> >>
> >> That's a very, very generic compatible. Are you sure, 100% sure, that
> >> all designs from OpenCores from now till next 100 years will be 100%
> >> compatible?
> >>
> > My description is not accurate enough, this is OpenCores PTC IP, and PWM
> > is one of those modes, so it might be better to replace compatible with
> > "opencores, ptc-pwm"
> > 
> > What do you think?
> 
> Sorry, maybe this answers maybe doesn't. What is "PTC"?

"pwm timer counter". AFAIU, the IP can be configured to provide all 3.
I think that William pointed out on an earlier revision that they have
only implemented the pwm on their hardware.
I don't think putting in "ptc" is a sufficient differentiator though, as
clearly there could be several different versions of "ptc-pwm" that have
the same concern about "all designs from OpenCores for now till the next
100 years" being compatible.

Cheers.
Conor.

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

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v7 1/4] dt-bindings: pwm: Add OpenCores PWM module

[William Qiu]

On 2023/11/14 4:17, Conor Dooley wrote:
> On Mon, Nov 13, 2023 at 09:07:15PM +0100, Krzysztof Kozlowski wrote:
>> On 13/11/2023 10:42, William Qiu wrote:
>> > Will update.
>> >>> +
>> >>> +allOf:
>> >>> +  - $ref: pwm.yaml#
>> >>> +
>> >>> +properties:
>> >>> +  compatible:
>> >>> +    oneOf:
>> >>> +      - items:
>> >>> +          - enum:
>> >>> +              - starfive,jh7100-pwm
>> >>> +              - starfive,jh7110-pwm
>> >>> +          - const: opencores,pwm
>> >>
>> >> That's a very, very generic compatible. Are you sure, 100% sure, that
>> >> all designs from OpenCores from now till next 100 years will be 100%
>> >> compatible?
>> >>
>> > My description is not accurate enough, this is OpenCores PTC IP, and PWM
>> > is one of those modes, so it might be better to replace compatible with
>> > "opencores, ptc-pwm"
>> > 
>> > What do you think?
>> 
>> Sorry, maybe this answers maybe doesn't. What is "PTC"?
> 
> "pwm timer counter". AFAIU, the IP can be configured to provide all 3.
> I think that William pointed out on an earlier revision that they have
> only implemented the pwm on their hardware.
> I don't think putting in "ptc" is a sufficient differentiator though, as
> clearly there could be several different versions of "ptc-pwm" that have
> the same concern about "all designs from OpenCores for now till the next
> 100 years" being compatible.
> 
> Cheers.
> Conor.

Hi，Conor and Krzysztof,

After discussion and review of materials, we plan to use "opencores,ptc-pwm-v1"
as this version of compatible, so that it can also be compatible in the future.

What do you think?


Best regards,
William

Re: [PATCH v7 1/4] dt-bindings: pwm: Add OpenCores PWM module

[William Qiu]

On 2023/11/14 4:17, Conor Dooley wrote:
> On Mon, Nov 13, 2023 at 09:07:15PM +0100, Krzysztof Kozlowski wrote:
>> On 13/11/2023 10:42, William Qiu wrote:
>> > Will update.
>> >>> +
>> >>> +allOf:
>> >>> +  - $ref: pwm.yaml#
>> >>> +
>> >>> +properties:
>> >>> +  compatible:
>> >>> +    oneOf:
>> >>> +      - items:
>> >>> +          - enum:
>> >>> +              - starfive,jh7100-pwm
>> >>> +              - starfive,jh7110-pwm
>> >>> +          - const: opencores,pwm
>> >>
>> >> That's a very, very generic compatible. Are you sure, 100% sure, that
>> >> all designs from OpenCores from now till next 100 years will be 100%
>> >> compatible?
>> >>
>> > My description is not accurate enough, this is OpenCores PTC IP, and PWM
>> > is one of those modes, so it might be better to replace compatible with
>> > "opencores, ptc-pwm"
>> > 
>> > What do you think?
>> 
>> Sorry, maybe this answers maybe doesn't. What is "PTC"?
> 
> "pwm timer counter". AFAIU, the IP can be configured to provide all 3.
> I think that William pointed out on an earlier revision that they have
> only implemented the pwm on their hardware.
> I don't think putting in "ptc" is a sufficient differentiator though, as
> clearly there could be several different versions of "ptc-pwm" that have
> the same concern about "all designs from OpenCores for now till the next
> 100 years" being compatible.
> 
> Cheers.
> Conor.

Hi，Conor and Krzysztof,

After discussion and review of materials, we plan to use "opencores,ptc-pwm-v1"
as this version of compatible, so that it can also be compatible in the future.

What do you think?


Best regards,
William

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v7 1/4] dt-bindings: pwm: Add OpenCores PWM module

[Conor Dooley]

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

On Wed, Nov 22, 2023 at 03:03:36PM +0800, William Qiu wrote:
> 
> 
> On 2023/11/14 4:17, Conor Dooley wrote:
> > On Mon, Nov 13, 2023 at 09:07:15PM +0100, Krzysztof Kozlowski wrote:
> >> On 13/11/2023 10:42, William Qiu wrote:
> >> > Will update.
> >> >>> +
> >> >>> +allOf:
> >> >>> +  - $ref: pwm.yaml#
> >> >>> +
> >> >>> +properties:
> >> >>> +  compatible:
> >> >>> +    oneOf:
> >> >>> +      - items:
> >> >>> +          - enum:
> >> >>> +              - starfive,jh7100-pwm
> >> >>> +              - starfive,jh7110-pwm
> >> >>> +          - const: opencores,pwm
> >> >>
> >> >> That's a very, very generic compatible. Are you sure, 100% sure, that
> >> >> all designs from OpenCores from now till next 100 years will be 100%
> >> >> compatible?
> >> >>
> >> > My description is not accurate enough, this is OpenCores PTC IP, and PWM
> >> > is one of those modes, so it might be better to replace compatible with
> >> > "opencores, ptc-pwm"
> >> > 
> >> > What do you think?
> >> 
> >> Sorry, maybe this answers maybe doesn't. What is "PTC"?
> > 
> > "pwm timer counter". AFAIU, the IP can be configured to provide all 3.
> > I think that William pointed out on an earlier revision that they have
> > only implemented the pwm on their hardware.
> > I don't think putting in "ptc" is a sufficient differentiator though, as
> > clearly there could be several different versions of "ptc-pwm" that have
> > the same concern about "all designs from OpenCores for now till the next
> > 100 years" being compatible.

Perhaps noting what "ptc" stands for in the description field would be a
good idea.

> After discussion and review of materials, we plan to use "opencores,ptc-pwm-v1"
> as this version of compatible, so that it can also be compatible in the future.
> 
> What do you think?

Do we know that it is actually "v1" of the IP? I would suggest using the
version that actually matches the version of the IP that you are using
in your SoC.

Thanks,
Conor.

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

Re: [PATCH v7 1/4] dt-bindings: pwm: Add OpenCores PWM module

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 2019 bytes --]

On Wed, Nov 22, 2023 at 03:03:36PM +0800, William Qiu wrote:
> 
> 
> On 2023/11/14 4:17, Conor Dooley wrote:
> > On Mon, Nov 13, 2023 at 09:07:15PM +0100, Krzysztof Kozlowski wrote:
> >> On 13/11/2023 10:42, William Qiu wrote:
> >> > Will update.
> >> >>> +
> >> >>> +allOf:
> >> >>> +  - $ref: pwm.yaml#
> >> >>> +
> >> >>> +properties:
> >> >>> +  compatible:
> >> >>> +    oneOf:
> >> >>> +      - items:
> >> >>> +          - enum:
> >> >>> +              - starfive,jh7100-pwm
> >> >>> +              - starfive,jh7110-pwm
> >> >>> +          - const: opencores,pwm
> >> >>
> >> >> That's a very, very generic compatible. Are you sure, 100% sure, that
> >> >> all designs from OpenCores from now till next 100 years will be 100%
> >> >> compatible?
> >> >>
> >> > My description is not accurate enough, this is OpenCores PTC IP, and PWM
> >> > is one of those modes, so it might be better to replace compatible with
> >> > "opencores, ptc-pwm"
> >> > 
> >> > What do you think?
> >> 
> >> Sorry, maybe this answers maybe doesn't. What is "PTC"?
> > 
> > "pwm timer counter". AFAIU, the IP can be configured to provide all 3.
> > I think that William pointed out on an earlier revision that they have
> > only implemented the pwm on their hardware.
> > I don't think putting in "ptc" is a sufficient differentiator though, as
> > clearly there could be several different versions of "ptc-pwm" that have
> > the same concern about "all designs from OpenCores for now till the next
> > 100 years" being compatible.

Perhaps noting what "ptc" stands for in the description field would be a
good idea.

> After discussion and review of materials, we plan to use "opencores,ptc-pwm-v1"
> as this version of compatible, so that it can also be compatible in the future.
> 
> What do you think?

Do we know that it is actually "v1" of the IP? I would suggest using the
version that actually matches the version of the IP that you are using
in your SoC.

Thanks,
Conor.

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

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v7 1/4] dt-bindings: pwm: Add OpenCores PWM module

[William Qiu]

On 2023/11/23 1:36, Conor Dooley wrote:
> On Wed, Nov 22, 2023 at 03:03:36PM +0800, William Qiu wrote:
>> 
>> 
>> On 2023/11/14 4:17, Conor Dooley wrote:
>> > On Mon, Nov 13, 2023 at 09:07:15PM +0100, Krzysztof Kozlowski wrote:
>> >> On 13/11/2023 10:42, William Qiu wrote:
>> >> > Will update.
>> >> >>> +
>> >> >>> +allOf:
>> >> >>> +  - $ref: pwm.yaml#
>> >> >>> +
>> >> >>> +properties:
>> >> >>> +  compatible:
>> >> >>> +    oneOf:
>> >> >>> +      - items:
>> >> >>> +          - enum:
>> >> >>> +              - starfive,jh7100-pwm
>> >> >>> +              - starfive,jh7110-pwm
>> >> >>> +          - const: opencores,pwm
>> >> >>
>> >> >> That's a very, very generic compatible. Are you sure, 100% sure, that
>> >> >> all designs from OpenCores from now till next 100 years will be 100%
>> >> >> compatible?
>> >> >>
>> >> > My description is not accurate enough, this is OpenCores PTC IP, and PWM
>> >> > is one of those modes, so it might be better to replace compatible with
>> >> > "opencores, ptc-pwm"
>> >> > 
>> >> > What do you think?
>> >> 
>> >> Sorry, maybe this answers maybe doesn't. What is "PTC"?
>> > 
>> > "pwm timer counter". AFAIU, the IP can be configured to provide all 3.
>> > I think that William pointed out on an earlier revision that they have
>> > only implemented the pwm on their hardware.
>> > I don't think putting in "ptc" is a sufficient differentiator though, as
>> > clearly there could be several different versions of "ptc-pwm" that have
>> > the same concern about "all designs from OpenCores for now till the next
>> > 100 years" being compatible.
> 
> Perhaps noting what "ptc" stands for in the description field would be a
> good idea.
> 
I will add.
>> After discussion and review of materials, we plan to use "opencores,ptc-pwm-v1"
>> as this version of compatible, so that it can also be compatible in the future.
>> 
>> What do you think?
> 
> Do we know that it is actually "v1" of the IP? I would suggest using the
> version that actually matches the version of the IP that you are using
> in your SoC.
> 
> Thanks,
> Conor.

There is no version list on their official website, so it is not certain whether
it is v1, but at least the driver is the first version.

What do you think is the best way?

Thanks,
William

Re: [PATCH v7 1/4] dt-bindings: pwm: Add OpenCores PWM module

[William Qiu]

On 2023/11/23 1:36, Conor Dooley wrote:
> On Wed, Nov 22, 2023 at 03:03:36PM +0800, William Qiu wrote:
>> 
>> 
>> On 2023/11/14 4:17, Conor Dooley wrote:
>> > On Mon, Nov 13, 2023 at 09:07:15PM +0100, Krzysztof Kozlowski wrote:
>> >> On 13/11/2023 10:42, William Qiu wrote:
>> >> > Will update.
>> >> >>> +
>> >> >>> +allOf:
>> >> >>> +  - $ref: pwm.yaml#
>> >> >>> +
>> >> >>> +properties:
>> >> >>> +  compatible:
>> >> >>> +    oneOf:
>> >> >>> +      - items:
>> >> >>> +          - enum:
>> >> >>> +              - starfive,jh7100-pwm
>> >> >>> +              - starfive,jh7110-pwm
>> >> >>> +          - const: opencores,pwm
>> >> >>
>> >> >> That's a very, very generic compatible. Are you sure, 100% sure, that
>> >> >> all designs from OpenCores from now till next 100 years will be 100%
>> >> >> compatible?
>> >> >>
>> >> > My description is not accurate enough, this is OpenCores PTC IP, and PWM
>> >> > is one of those modes, so it might be better to replace compatible with
>> >> > "opencores, ptc-pwm"
>> >> > 
>> >> > What do you think?
>> >> 
>> >> Sorry, maybe this answers maybe doesn't. What is "PTC"?
>> > 
>> > "pwm timer counter". AFAIU, the IP can be configured to provide all 3.
>> > I think that William pointed out on an earlier revision that they have
>> > only implemented the pwm on their hardware.
>> > I don't think putting in "ptc" is a sufficient differentiator though, as
>> > clearly there could be several different versions of "ptc-pwm" that have
>> > the same concern about "all designs from OpenCores for now till the next
>> > 100 years" being compatible.
> 
> Perhaps noting what "ptc" stands for in the description field would be a
> good idea.
> 
I will add.
>> After discussion and review of materials, we plan to use "opencores,ptc-pwm-v1"
>> as this version of compatible, so that it can also be compatible in the future.
>> 
>> What do you think?
> 
> Do we know that it is actually "v1" of the IP? I would suggest using the
> version that actually matches the version of the IP that you are using
> in your SoC.
> 
> Thanks,
> Conor.

There is no version list on their official website, so it is not certain whether
it is v1, but at least the driver is the first version.

What do you think is the best way?

Thanks,
William

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v7 1/4] dt-bindings: pwm: Add OpenCores PWM module

[Conor Dooley]

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

On Fri, Nov 24, 2023 at 03:38:41PM +0800, William Qiu wrote:
> 
> 
> On 2023/11/23 1:36, Conor Dooley wrote:
> > On Wed, Nov 22, 2023 at 03:03:36PM +0800, William Qiu wrote:
> >> 
> >> 
> >> On 2023/11/14 4:17, Conor Dooley wrote:
> >> > On Mon, Nov 13, 2023 at 09:07:15PM +0100, Krzysztof Kozlowski wrote:
> >> >> On 13/11/2023 10:42, William Qiu wrote:
> >> >> > Will update.
> >> >> >>> +
> >> >> >>> +allOf:
> >> >> >>> +  - $ref: pwm.yaml#
> >> >> >>> +
> >> >> >>> +properties:
> >> >> >>> +  compatible:
> >> >> >>> +    oneOf:
> >> >> >>> +      - items:
> >> >> >>> +          - enum:
> >> >> >>> +              - starfive,jh7100-pwm
> >> >> >>> +              - starfive,jh7110-pwm
> >> >> >>> +          - const: opencores,pwm
> >> >> >>
> >> >> >> That's a very, very generic compatible. Are you sure, 100% sure, that
> >> >> >> all designs from OpenCores from now till next 100 years will be 100%
> >> >> >> compatible?
> >> >> >>
> >> >> > My description is not accurate enough, this is OpenCores PTC IP, and PWM
> >> >> > is one of those modes, so it might be better to replace compatible with
> >> >> > "opencores, ptc-pwm"
> >> >> > 
> >> >> > What do you think?
> >> >> 
> >> >> Sorry, maybe this answers maybe doesn't. What is "PTC"?
> >> > 
> >> > "pwm timer counter". AFAIU, the IP can be configured to provide all 3.
> >> > I think that William pointed out on an earlier revision that they have
> >> > only implemented the pwm on their hardware.
> >> > I don't think putting in "ptc" is a sufficient differentiator though, as
> >> > clearly there could be several different versions of "ptc-pwm" that have
> >> > the same concern about "all designs from OpenCores for now till the next
> >> > 100 years" being compatible.
> > 
> > Perhaps noting what "ptc" stands for in the description field would be a
> > good idea.
> > 
> I will add.
> >> After discussion and review of materials, we plan to use "opencores,ptc-pwm-v1"
> >> as this version of compatible, so that it can also be compatible in the future.
> >> 
> >> What do you think?
> > 
> > Do we know that it is actually "v1" of the IP? I would suggest using the
> > version that actually matches the version of the IP that you are using
> > in your SoC.
> > 
> > Thanks,
> > Conor.
> 
> There is no version list on their official website, so it is not certain whether
> it is v1, but at least the driver is the first version.
> 
> What do you think is the best way?

I don't have an account, so I cannot open the "ptc_spec.pdf at this link:
https://opencores.org/projects/ptc/downloads
but I would take whatever documentation you have for the spec and see
what it says as the revision on the front cover.

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

Re: [PATCH v7 1/4] dt-bindings: pwm: Add OpenCores PWM module

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 2754 bytes --]

On Fri, Nov 24, 2023 at 03:38:41PM +0800, William Qiu wrote:
> 
> 
> On 2023/11/23 1:36, Conor Dooley wrote:
> > On Wed, Nov 22, 2023 at 03:03:36PM +0800, William Qiu wrote:
> >> 
> >> 
> >> On 2023/11/14 4:17, Conor Dooley wrote:
> >> > On Mon, Nov 13, 2023 at 09:07:15PM +0100, Krzysztof Kozlowski wrote:
> >> >> On 13/11/2023 10:42, William Qiu wrote:
> >> >> > Will update.
> >> >> >>> +
> >> >> >>> +allOf:
> >> >> >>> +  - $ref: pwm.yaml#
> >> >> >>> +
> >> >> >>> +properties:
> >> >> >>> +  compatible:
> >> >> >>> +    oneOf:
> >> >> >>> +      - items:
> >> >> >>> +          - enum:
> >> >> >>> +              - starfive,jh7100-pwm
> >> >> >>> +              - starfive,jh7110-pwm
> >> >> >>> +          - const: opencores,pwm
> >> >> >>
> >> >> >> That's a very, very generic compatible. Are you sure, 100% sure, that
> >> >> >> all designs from OpenCores from now till next 100 years will be 100%
> >> >> >> compatible?
> >> >> >>
> >> >> > My description is not accurate enough, this is OpenCores PTC IP, and PWM
> >> >> > is one of those modes, so it might be better to replace compatible with
> >> >> > "opencores, ptc-pwm"
> >> >> > 
> >> >> > What do you think?
> >> >> 
> >> >> Sorry, maybe this answers maybe doesn't. What is "PTC"?
> >> > 
> >> > "pwm timer counter". AFAIU, the IP can be configured to provide all 3.
> >> > I think that William pointed out on an earlier revision that they have
> >> > only implemented the pwm on their hardware.
> >> > I don't think putting in "ptc" is a sufficient differentiator though, as
> >> > clearly there could be several different versions of "ptc-pwm" that have
> >> > the same concern about "all designs from OpenCores for now till the next
> >> > 100 years" being compatible.
> > 
> > Perhaps noting what "ptc" stands for in the description field would be a
> > good idea.
> > 
> I will add.
> >> After discussion and review of materials, we plan to use "opencores,ptc-pwm-v1"
> >> as this version of compatible, so that it can also be compatible in the future.
> >> 
> >> What do you think?
> > 
> > Do we know that it is actually "v1" of the IP? I would suggest using the
> > version that actually matches the version of the IP that you are using
> > in your SoC.
> > 
> > Thanks,
> > Conor.
> 
> There is no version list on their official website, so it is not certain whether
> it is v1, but at least the driver is the first version.
> 
> What do you think is the best way?

I don't have an account, so I cannot open the "ptc_spec.pdf at this link:
https://opencores.org/projects/ptc/downloads
but I would take whatever documentation you have for the spec and see
what it says as the revision on the front cover.

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

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv