[PATCH v16] pwm: opencores: Add PWM driver support

[PATCH v16] 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 | 241 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 261 insertions(+)
 create mode 100644 drivers/pwm/pwm-ocores.c

diff --git a/MAINTAINERS b/MAINTAINERS
index a097afd76ded..5db90c1d9854 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17373,6 +17373,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 0915c1e7df16..33819cb585be 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -471,6 +471,18 @@ config PWM_NTXEC
 	  controller found in certain e-book readers designed by the original
 	  design manufacturer Netronix.
 
+config PWM_OCORES
+	tristate "OpenCores PTC 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 9081e0c0e9e0..e55997490dcb 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -42,6 +42,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..1ba29f4fe0a6
--- /dev/null
+++ b/drivers/pwm/pwm-ocores.c
@@ -0,0 +1,241 @@
+// 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 supports inverted polarity.
+ * - The hardware minimum period / duty_cycle is (1 / pwm_apb clock frequency).
+ * - The hardware maximum period / duty_cycle is (U32_MAX / pwm_apb clock frequency).
+ * - The output is set to a low level immediately when disabled.
+ * - When configuration changes are done, they get active immediately without resetting
+ *   the counter. This might result in one period affected by both old and new settings.
+ */
+
+#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>
+
+/* OpenCores Register offsets */
+#define REG_OCPWM_CNTR    0x0
+#define REG_OCPWM_HRC     0x4
+#define REG_OCPWM_LRC     0x8
+#define REG_OCPWM_CTRL    0xC
+
+/* OCPWM_CTRL register bits*/
+#define REG_OCPWM_CNTR_EN      BIT(0)
+#define REG_OCPWM_CNTR_ECLK    BIT(1)
+#define REG_OCPWM_CNTR_NEC     BIT(2)
+#define REG_OCPWM_CNTR_OE      BIT(3)
+#define REG_OCPWM_CNTR_SIGNLE  BIT(4)
+#define REG_OCPWM_CNTR_INTE    BIT(5)
+#define REG_OCPWM_CNTR_INT     BIT(6)
+#define REG_OCPWM_CNTR_RST     BIT(7)
+#define REG_OCPWM_CNTR_CAPTE   BIT(8)
+
+struct ocores_pwm_data {
+	void __iomem *(*get_ch_base)(void __iomem *base, unsigned int channel);
+};
+
+struct ocores_pwm_device {
+	const struct ocores_pwm_data *data;
+	void __iomem *regs;
+	u32 clk_rate; /* PWM APB clock frequency */
+};
+
+static inline u32 ocores_pwm_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_pwm_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 pwmchip_get_drvdata(chip);
+}
+
+static void __iomem *ocores_pwm_get_ch_base(void __iomem *base,
+					    unsigned int channel)
+{
+	unsigned int offset = (channel & 4) << 13 | (channel & 3) << 4;
+
+	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_pwm_readl(ddata, pwm->hwpwm, REG_OCPWM_LRC);
+	duty_data = ocores_pwm_readl(ddata, pwm->hwpwm, REG_OCPWM_HRC);
+	ctrl_data = ocores_pwm_readl(ddata, pwm->hwpwm, REG_OCPWM_CTRL);
+
+	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_CNTR_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;
+
+	period_data = mul_u64_u32_div(state->period, ddata->clk_rate, NSEC_PER_SEC);
+	if (!period_data)
+		return -EINVAL;
+
+	if (period_data > U32_MAX)
+		period_data = U32_MAX;
+
+	ocores_pwm_writel(ddata, pwm->hwpwm, REG_OCPWM_LRC, period_data);
+
+	duty_data = mul_u64_u32_div(state->duty_cycle, ddata->clk_rate, NSEC_PER_SEC);
+	if (!duty_data)
+		return -EINVAL;
+
+	if (duty_data > U32_MAX)
+		duty_data = U32_MAX;
+
+	ocores_pwm_writel(ddata, pwm->hwpwm, REG_OCPWM_HRC, duty_data);
+
+	ctrl_data = ocores_pwm_readl(ddata, pwm->hwpwm, REG_OCPWM_CTRL);
+	if (state->enabled)
+		ocores_pwm_writel(ddata, pwm->hwpwm, REG_OCPWM_CTRL,
+				  ctrl_data | REG_OCPWM_CNTR_EN | REG_OCPWM_CNTR_OE);
+	else
+		ocores_pwm_writel(ddata, pwm->hwpwm, REG_OCPWM_CTRL,
+				  ctrl_data & ~(REG_OCPWM_CNTR_EN | REG_OCPWM_CNTR_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 starfive_pwm_data = {
+	.get_ch_base = ocores_pwm_get_ch_base,
+};
+
+static const struct of_device_id ocores_pwm_of_match[] = {
+	{ .compatible = "opencores,pwm-v1" },
+	{ .compatible = "starfive,jh7100-pwm", .data = &starfive_pwm_data},
+	{ .compatible = "starfive,jh7110-pwm", .data = &starfive_pwm_data},
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, ocores_pwm_of_match);
+
+static void ocores_pwm_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;
+	struct clk *clk;
+	struct reset_control *rst;
+	int ret;
+
+	id = of_device_get_match_data(&pdev->dev);
+	if (!id)
+		return -ENODEV;
+
+	chip = devm_pwmchip_alloc(&pdev->dev, 8, sizeof(*ddata));
+	if (IS_ERR(chip))
+		return -ENOMEM;
+
+	ddata = chip_to_ocores(chip);
+	ddata->data = id->data;
+	chip->ops = &ocores_pwm_ops;
+
+	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");
+
+	clk = devm_clk_get_enabled(dev, NULL);
+	if (IS_ERR(clk))
+		return dev_err_probe(dev, PTR_ERR(clk),
+				     "Unable to get pwm's clock\n");
+
+	ret = devm_clk_rate_exclusive_get(dev, clk);
+	if (ret)
+		return ret;
+
+	rst = devm_reset_control_get_optional_exclusive(dev, NULL);
+	if (IS_ERR(rst))
+		return dev_err_probe(dev, PTR_ERR(rst),
+				     "Unable to get pwm's reset\n");
+
+	ret = reset_control_deassert(rst);
+	if (ret)
+		return ret;
+
+	ret = devm_add_action_or_reset(dev, ocores_pwm_reset_control_assert, rst);
+	if (ret)
+		return ret;
+
+	ddata->clk_rate = clk_get_rate(clk);
+	if (ddata->clk_rate > NSEC_PER_SEC)
+		return -EINVAL;
+
+	ret = devm_pwmchip_add(dev, chip);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Could not register PWM chip\n");
+
+	return 0;
+}
+
+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 PTC PWM driver");
+MODULE_LICENSE("GPL");
-- 
2.43.0

RE: [PATCH v16] pwm: opencores: Add PWM driver support

[William Qiu]

> -----Original Message-----
> From: William Qiu
> Sent: 2024年10月28日 9:46
> To: linux-kernel@vger.kernel.org; linux-pwm@vger.kernel.org
> Cc: Uwe Kleine-König <ukleinek@kernel.org>; Hal Feng
> <hal.feng@starfivetech.com>; Philipp Zabel <p.zabel@pengutronix.de>; William
> Qiu <william.qiu@starfivetech.com>
> Subject: [PATCH v16] pwm: opencores: Add PWM driver support
> 
> 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 | 241
> +++++++++++++++++++++++++++++++++++++++
>  4 files changed, 261 insertions(+)
>  create mode 100644 drivers/pwm/pwm-ocores.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a097afd76ded..5db90c1d9854 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -17373,6 +17373,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
> 0915c1e7df16..33819cb585be 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -471,6 +471,18 @@ config PWM_NTXEC
>  	  controller found in certain e-book readers designed by the original
>  	  design manufacturer Netronix.
> 
> +config PWM_OCORES
> +	tristate "OpenCores PTC 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
> 9081e0c0e9e0..e55997490dcb 100644
> --- a/drivers/pwm/Makefile
> +++ b/drivers/pwm/Makefile
> @@ -42,6 +42,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..1ba29f4fe0a6
> --- /dev/null
> +++ b/drivers/pwm/pwm-ocores.c
> @@ -0,0 +1,241 @@
> +// 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 supports inverted polarity.
> + * - The hardware minimum period / duty_cycle is (1 / pwm_apb clock
> frequency).
> + * - The hardware maximum period / duty_cycle is (U32_MAX / pwm_apb clock
> frequency).
> + * - The output is set to a low level immediately when disabled.
> + * - When configuration changes are done, they get active immediately without
> resetting
> + *   the counter. This might result in one period affected by both old and new
> settings.
> + */
> +
> +#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>
> +
> +/* OpenCores Register offsets */
> +#define REG_OCPWM_CNTR    0x0
> +#define REG_OCPWM_HRC     0x4
> +#define REG_OCPWM_LRC     0x8
> +#define REG_OCPWM_CTRL    0xC
> +
> +/* OCPWM_CTRL register bits*/
> +#define REG_OCPWM_CNTR_EN      BIT(0)
> +#define REG_OCPWM_CNTR_ECLK    BIT(1)
> +#define REG_OCPWM_CNTR_NEC     BIT(2)
> +#define REG_OCPWM_CNTR_OE      BIT(3)
> +#define REG_OCPWM_CNTR_SIGNLE  BIT(4)
> +#define REG_OCPWM_CNTR_INTE    BIT(5)
> +#define REG_OCPWM_CNTR_INT     BIT(6)
> +#define REG_OCPWM_CNTR_RST     BIT(7)
> +#define REG_OCPWM_CNTR_CAPTE   BIT(8)
> +
> +struct ocores_pwm_data {
> +	void __iomem *(*get_ch_base)(void __iomem *base, unsigned int
> +channel); };
> +
> +struct ocores_pwm_device {
> +	const struct ocores_pwm_data *data;
> +	void __iomem *regs;
> +	u32 clk_rate; /* PWM APB clock frequency */ };
> +
> +static inline u32 ocores_pwm_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_pwm_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 pwmchip_get_drvdata(chip);
> +}
> +
> +static void __iomem *ocores_pwm_get_ch_base(void __iomem *base,
> +					    unsigned int channel)
> +{
> +	unsigned int offset = (channel & 4) << 13 | (channel & 3) << 4;
> +
> +	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_pwm_readl(ddata, pwm->hwpwm,
> REG_OCPWM_LRC);
> +	duty_data = ocores_pwm_readl(ddata, pwm->hwpwm,
> REG_OCPWM_HRC);
> +	ctrl_data = ocores_pwm_readl(ddata, pwm->hwpwm,
> REG_OCPWM_CTRL);
> +
> +	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_CNTR_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;
> +
> +	period_data = mul_u64_u32_div(state->period, ddata->clk_rate,
> NSEC_PER_SEC);
> +	if (!period_data)
> +		return -EINVAL;
> +
> +	if (period_data > U32_MAX)
> +		period_data = U32_MAX;
> +
> +	ocores_pwm_writel(ddata, pwm->hwpwm, REG_OCPWM_LRC,
> period_data);
> +
> +	duty_data = mul_u64_u32_div(state->duty_cycle, ddata->clk_rate,
> NSEC_PER_SEC);
> +	if (!duty_data)
> +		return -EINVAL;
> +
> +	if (duty_data > U32_MAX)
> +		duty_data = U32_MAX;
> +
> +	ocores_pwm_writel(ddata, pwm->hwpwm, REG_OCPWM_HRC,
> duty_data);
> +
> +	ctrl_data = ocores_pwm_readl(ddata, pwm->hwpwm,
> REG_OCPWM_CTRL);
> +	if (state->enabled)
> +		ocores_pwm_writel(ddata, pwm->hwpwm, REG_OCPWM_CTRL,
> +				  ctrl_data | REG_OCPWM_CNTR_EN |
> REG_OCPWM_CNTR_OE);
> +	else
> +		ocores_pwm_writel(ddata, pwm->hwpwm, REG_OCPWM_CTRL,
> +				  ctrl_data & ~(REG_OCPWM_CNTR_EN |
> REG_OCPWM_CNTR_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 starfive_pwm_data = {
> +	.get_ch_base = ocores_pwm_get_ch_base, };
> +
> +static const struct of_device_id ocores_pwm_of_match[] = {
> +	{ .compatible = "opencores,pwm-v1" },
> +	{ .compatible = "starfive,jh7100-pwm", .data = &starfive_pwm_data},
> +	{ .compatible = "starfive,jh7110-pwm", .data = &starfive_pwm_data},
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, ocores_pwm_of_match);
> +
> +static void ocores_pwm_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;
> +	struct clk *clk;
> +	struct reset_control *rst;
> +	int ret;
> +
> +	id = of_device_get_match_data(&pdev->dev);
> +	if (!id)
> +		return -ENODEV;
> +
> +	chip = devm_pwmchip_alloc(&pdev->dev, 8, sizeof(*ddata));
> +	if (IS_ERR(chip))
> +		return -ENOMEM;
> +
> +	ddata = chip_to_ocores(chip);
> +	ddata->data = id->data;
> +	chip->ops = &ocores_pwm_ops;
> +
> +	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");
> +
> +	clk = devm_clk_get_enabled(dev, NULL);
> +	if (IS_ERR(clk))
> +		return dev_err_probe(dev, PTR_ERR(clk),
> +				     "Unable to get pwm's clock\n");
> +
> +	ret = devm_clk_rate_exclusive_get(dev, clk);
> +	if (ret)
> +		return ret;
> +
> +	rst = devm_reset_control_get_optional_exclusive(dev, NULL);
> +	if (IS_ERR(rst))
> +		return dev_err_probe(dev, PTR_ERR(rst),
> +				     "Unable to get pwm's reset\n");
> +
> +	ret = reset_control_deassert(rst);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_add_action_or_reset(dev, ocores_pwm_reset_control_assert,
> rst);
> +	if (ret)
> +		return ret;
> +
> +	ddata->clk_rate = clk_get_rate(clk);
> +	if (ddata->clk_rate > NSEC_PER_SEC)
> +		return -EINVAL;
> +
> +	ret = devm_pwmchip_add(dev, chip);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "Could not register PWM chip\n");
> +
> +	return 0;
> +}
> +
> +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 PTC PWM driver");
> MODULE_LICENSE("GPL");
> --
> 2.43.0

Hi Uwe,

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 mainline? Thanks.

Thanks for taking time to review this patch series.
 
Best Regards,
William

Re: [PATCH v16] pwm: opencores: Add PWM driver support

[Uwe Kleine-König]

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

Hello,

On Mon, Oct 28, 2024 at 09:46:09AM +0800, William Qiu wrote:
> +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;
> +
> +	period_data = mul_u64_u32_div(state->period, ddata->clk_rate, NSEC_PER_SEC);
> +	if (!period_data)
> +		return -EINVAL;
> +
> +	if (period_data > U32_MAX)
> +		period_data = U32_MAX;
> +
> +	ocores_pwm_writel(ddata, pwm->hwpwm, REG_OCPWM_LRC, period_data);
> +
> +	duty_data = mul_u64_u32_div(state->duty_cycle, ddata->clk_rate, NSEC_PER_SEC);
> +	if (!duty_data)
> +		return -EINVAL;

I can understand that period_data == 0 is an error, but duty_data == 0
could/should just work?!

> +
> +	if (duty_data > U32_MAX)
> +		duty_data = U32_MAX;
> +
> +	ocores_pwm_writel(ddata, pwm->hwpwm, REG_OCPWM_HRC, duty_data);
> +
> +	ctrl_data = ocores_pwm_readl(ddata, pwm->hwpwm, REG_OCPWM_CTRL);
> +	if (state->enabled)
> +		ocores_pwm_writel(ddata, pwm->hwpwm, REG_OCPWM_CTRL,
> +				  ctrl_data | REG_OCPWM_CNTR_EN | REG_OCPWM_CNTR_OE);
> +	else
> +		ocores_pwm_writel(ddata, pwm->hwpwm, REG_OCPWM_CTRL,
> +				  ctrl_data & ~(REG_OCPWM_CNTR_EN | REG_OCPWM_CNTR_OE));
> +
> +	return 0;
> +}
> [...]
> +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;
> +	struct clk *clk;
> +	struct reset_control *rst;
> +	int ret;
> +
> +	id = of_device_get_match_data(&pdev->dev);
> +	if (!id)
> +		return -ENODEV;
> +
> +	chip = devm_pwmchip_alloc(&pdev->dev, 8, sizeof(*ddata));
> +	if (IS_ERR(chip))
> +		return -ENOMEM;
> +
> +	ddata = chip_to_ocores(chip);
> +	ddata->data = id->data;
> +	chip->ops = &ocores_pwm_ops;
> +
> +	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");
> +
> +	clk = devm_clk_get_enabled(dev, NULL);
> +	if (IS_ERR(clk))
> +		return dev_err_probe(dev, PTR_ERR(clk),
> +				     "Unable to get pwm's clock\n");
> +
> +	ret = devm_clk_rate_exclusive_get(dev, clk);
> +	if (ret)
> +		return ret;
> +
> +	rst = devm_reset_control_get_optional_exclusive(dev, NULL);
> +	if (IS_ERR(rst))
> +		return dev_err_probe(dev, PTR_ERR(rst),
> +				     "Unable to get pwm's reset\n");
> +
> +	ret = reset_control_deassert(rst);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_add_action_or_reset(dev, ocores_pwm_reset_control_assert, rst);
> +	if (ret)
> +		return ret;
> +
> +	ddata->clk_rate = clk_get_rate(clk);
> +	if (ddata->clk_rate > NSEC_PER_SEC)
> +		return -EINVAL;

Missing error messages for the three checks above.

> +	ret = devm_pwmchip_add(dev, chip);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "Could not register PWM chip\n");
> +
> +	return 0;
> +}

Best regards
Uwe

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

RE: [PATCH v16] pwm: opencores: Add PWM driver support

[William Qiu]

> -----Original Message-----
> From: Uwe Kleine-König <ukleinek@kernel.org>
> Sent: 2024年12月20日 5:00
> To: William Qiu <william.qiu@starfivetech.com>
> Cc: linux-kernel@vger.kernel.org; linux-pwm@vger.kernel.org; Hal Feng
> <hal.feng@starfivetech.com>; Philipp Zabel <p.zabel@pengutronix.de>
> Subject: Re: [PATCH v16] pwm: opencores: Add PWM driver support
> 
> Hello,
> 
> On Mon, Oct 28, 2024 at 09:46:09AM +0800, William Qiu wrote:
> > +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;
> > +
> > +	period_data = mul_u64_u32_div(state->period, ddata->clk_rate,
> NSEC_PER_SEC);
> > +	if (!period_data)
> > +		return -EINVAL;
> > +
> > +	if (period_data > U32_MAX)
> > +		period_data = U32_MAX;
> > +
> > +	ocores_pwm_writel(ddata, pwm->hwpwm, REG_OCPWM_LRC,
> period_data);
> > +
> > +	duty_data = mul_u64_u32_div(state->duty_cycle, ddata->clk_rate,
> NSEC_PER_SEC);
> > +	if (!duty_data)
> > +		return -EINVAL;
> 
> I can understand that period_data == 0 is an error, but duty_data == 0
> could/should just work?!
> 
It means no need to check whether the duty is valid? 
> > +
> > +	if (duty_data > U32_MAX)
> > +		duty_data = U32_MAX;
> > +
> > +	ocores_pwm_writel(ddata, pwm->hwpwm, REG_OCPWM_HRC,
> duty_data);
> > +
> > +	ctrl_data = ocores_pwm_readl(ddata, pwm->hwpwm,
> REG_OCPWM_CTRL);
> > +	if (state->enabled)
> > +		ocores_pwm_writel(ddata, pwm->hwpwm, REG_OCPWM_CTRL,
> > +				  ctrl_data | REG_OCPWM_CNTR_EN |
> REG_OCPWM_CNTR_OE);
> > +	else
> > +		ocores_pwm_writel(ddata, pwm->hwpwm, REG_OCPWM_CTRL,
> > +				  ctrl_data & ~(REG_OCPWM_CNTR_EN |
> REG_OCPWM_CNTR_OE));
> > +
> > +	return 0;
> > +}
> > [...]
> > +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;
> > +	struct clk *clk;
> > +	struct reset_control *rst;
> > +	int ret;
> > +
> > +	id = of_device_get_match_data(&pdev->dev);
> > +	if (!id)
> > +		return -ENODEV;
> > +
> > +	chip = devm_pwmchip_alloc(&pdev->dev, 8, sizeof(*ddata));
> > +	if (IS_ERR(chip))
> > +		return -ENOMEM;
> > +
> > +	ddata = chip_to_ocores(chip);
> > +	ddata->data = id->data;
> > +	chip->ops = &ocores_pwm_ops;
> > +
> > +	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");
> > +
> > +	clk = devm_clk_get_enabled(dev, NULL);
> > +	if (IS_ERR(clk))
> > +		return dev_err_probe(dev, PTR_ERR(clk),
> > +				     "Unable to get pwm's clock\n");
> > +
> > +	ret = devm_clk_rate_exclusive_get(dev, clk);
> > +	if (ret)
> > +		return ret;
> > +
> > +	rst = devm_reset_control_get_optional_exclusive(dev, NULL);
> > +	if (IS_ERR(rst))
> > +		return dev_err_probe(dev, PTR_ERR(rst),
> > +				     "Unable to get pwm's reset\n");
> > +
> > +	ret = reset_control_deassert(rst);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = devm_add_action_or_reset(dev, ocores_pwm_reset_control_assert,
> rst);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ddata->clk_rate = clk_get_rate(clk);
> > +	if (ddata->clk_rate > NSEC_PER_SEC)
> > +		return -EINVAL;
> 
> Missing error messages for the three checks above.
> 
Will add.
> > +	ret = devm_pwmchip_add(dev, chip);
> > +	if (ret < 0)
> > +		return dev_err_probe(dev, ret, "Could not register PWM chip\n");
> > +
> > +	return 0;
> > +}
> 
> Best regards
> Uwe

Re: [PATCH v16] pwm: opencores: Add PWM driver support

[Uwe Kleine-König]

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

Hello William,

On Mon, Dec 23, 2024 at 02:47:59AM +0000, William Qiu wrote:
> > > +	duty_data = mul_u64_u32_div(state->duty_cycle, ddata->clk_rate,
> > NSEC_PER_SEC);
> > > +	if (!duty_data)
> > > +		return -EINVAL;
> > 
> > I can understand that period_data == 0 is an error, but duty_data == 0
> > could/should just work?!
>
> It means no need to check whether the duty is valid? 

No, it means that I expect that duty_data == 0 is a valid setting and
most controllers support it.

Best regards
Uwe

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

RE: [PATCH v16] pwm: opencores: Add PWM driver support

[William Qiu]

> -----Original Message-----
> From: Uwe Kleine-König <ukleinek@kernel.org>
> Sent: 2024年12月24日 1:12
> To: William Qiu <william.qiu@starfivetech.com>
> Cc: linux-kernel@vger.kernel.org; linux-pwm@vger.kernel.org; Hal Feng
> <hal.feng@starfivetech.com>; Philipp Zabel <p.zabel@pengutronix.de>
> Subject: Re: [PATCH v16] pwm: opencores: Add PWM driver support
> 
> Hello William,
> 
> On Mon, Dec 23, 2024 at 02:47:59AM +0000, William Qiu wrote:
> > > > +	duty_data = mul_u64_u32_div(state->duty_cycle, ddata->clk_rate,
> > > NSEC_PER_SEC);
> > > > +	if (!duty_data)
> > > > +		return -EINVAL;
> > >
> > > I can understand that period_data == 0 is an error, but duty_data ==
> > > 0 could/should just work?!
> >
> > It means no need to check whether the duty is valid?
> 
> No, it means that I expect that duty_data == 0 is a valid setting and most
> controllers support it.
> 
> Best regards
> Uwe

So we just need to check duty < 0?

Best Regards,
William

Re: [PATCH v16] pwm: opencores: Add PWM driver support

[Uwe Kleine-König]

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

On Tue, Dec 24, 2024 at 01:53:03AM +0000, William Qiu wrote:
> > -----Original Message-----
> > From: Uwe Kleine-König <ukleinek@kernel.org>
> > Sent: 2024年12月24日 1:12
> > To: William Qiu <william.qiu@starfivetech.com>
> > Cc: linux-kernel@vger.kernel.org; linux-pwm@vger.kernel.org; Hal Feng
> > <hal.feng@starfivetech.com>; Philipp Zabel <p.zabel@pengutronix.de>
> > Subject: Re: [PATCH v16] pwm: opencores: Add PWM driver support
> > 
> > Hello William,
> > 
> > On Mon, Dec 23, 2024 at 02:47:59AM +0000, William Qiu wrote:
> > > > > +	duty_data = mul_u64_u32_div(state->duty_cycle, ddata->clk_rate,
> > > > NSEC_PER_SEC);
> > > > > +	if (!duty_data)
> > > > > +		return -EINVAL;
> > > >
> > > > I can understand that period_data == 0 is an error, but duty_data ==
> > > > 0 could/should just work?!
> > >
> > > It means no need to check whether the duty is valid?
> > 
> > No, it means that I expect that duty_data == 0 is a valid setting and most
> > controllers support it.
> > 
> > Best regards
> > Uwe
> 
> So we just need to check duty < 0?

I'm not sure I understand this question. You can assume that
state->duty_cycle is >= 0 in the driver callback.

If the hardware doesn't support duty_cycle == 0, add a comment and
proper error handling. (i.e. return with -EINVAL if the requested
duty_cycle is too low to be implemented.)

Best regards
Uwe

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

RE: [PATCH v16] pwm: opencores: Add PWM driver support

[William Qiu]

> -----Original Message-----
> From: Uwe Kleine-König <ukleinek@kernel.org>
> Sent: 2024年12月27日 16:38
> To: William Qiu <william.qiu@starfivetech.com>
> Cc: linux-kernel@vger.kernel.org; linux-pwm@vger.kernel.org; Hal Feng
> <hal.feng@starfivetech.com>; Philipp Zabel <p.zabel@pengutronix.de>
> Subject: Re: [PATCH v16] pwm: opencores: Add PWM driver support
> 
> On Tue, Dec 24, 2024 at 01:53:03AM +0000, William Qiu wrote:
> > > -----Original Message-----
> > > From: Uwe Kleine-König <ukleinek@kernel.org>
> > > Sent: 2024年12月24日 1:12
> > > To: William Qiu <william.qiu@starfivetech.com>
> > > Cc: linux-kernel@vger.kernel.org; linux-pwm@vger.kernel.org; Hal
> > > Feng <hal.feng@starfivetech.com>; Philipp Zabel
> > > <p.zabel@pengutronix.de>
> > > Subject: Re: [PATCH v16] pwm: opencores: Add PWM driver support
> > >
> > > Hello William,
> > >
> > > On Mon, Dec 23, 2024 at 02:47:59AM +0000, William Qiu wrote:
> > > > > > +	duty_data = mul_u64_u32_div(state->duty_cycle,
> > > > > > +ddata->clk_rate,
> > > > > NSEC_PER_SEC);
> > > > > > +	if (!duty_data)
> > > > > > +		return -EINVAL;
> > > > >
> > > > > I can understand that period_data == 0 is an error, but
> > > > > duty_data ==
> > > > > 0 could/should just work?!
> > > >
> > > > It means no need to check whether the duty is valid?
> > >
> > > No, it means that I expect that duty_data == 0 is a valid setting
> > > and most controllers support it.
> > >
> > > Best regards
> > > Uwe
> >
> > So we just need to check duty < 0?
> 
> I'm not sure I understand this question. You can assume that
> state->duty_cycle is >= 0 in the driver callback.
> 
> If the hardware doesn't support duty_cycle == 0, add a comment and proper
> error handling. (i.e. return with -EINVAL if the requested duty_cycle is too low
> to be implemented.)
> 
> Best regards
> Uwe

I see. I'll check it and send v17 soon.

Thanks,
William

Re: [PATCH v16] pwm: opencores: Add PWM driver support

[Maud Spierings]

Hello William,


I tested this patch on my deepcomputing fml13v01 with a pwm-backlight 
and I am getting an oops:

[   10.308290] Unable to handle kernel access to user memory without 
uaccess routines at virtual address 0000000200000001
[   10.319038] Oops [#1]
[   10.321318] Modules linked in: pwm_bl(+) sfctemp designware_i2s 
snd_soc_spdif_tx dm_mod configfs ip_tables x_tables vs_drm 
drm_dma_helper stmmac pcs_xpcs pcie_starfive phy_jh7110_usb 
phy_jh7110_pcie phy_jh
7110_dphy_rx phy_generic overlay jh7110_trng jh7110_tdm jh7110_pwmdac 
inno_hdmi drm_kms_helper i2c_hid_of i2c_hid hid_multitouch hci_uart drm 
drm_panel_orientation_quirks clk_starfive_jh7110_vout clk_starfive
_jh7110_stg clk_starfive_jh7110_isp clk_starfive_jh7110_aon 
clk_starfive_jh7100_audio cdns3_starfive cdns3 cdns_usb_common roles 
btintel bridge stp llc bluetooth libaes ecdh_generic ecc backlight efivarfs
[   10.375483] CPU: 3 UID: 0 PID: 307 Comm: (udev-worker) Not tainted 
6.13.0-rc4-00032-ga27a3464e6ab-dirty #16
[   10.385227] Hardware name: Unknown Unknown Product/Unknown Product, 
BIOS 2021.10 10/01/2021
[   10.393576] epc : ocores_pwm_get_state+0x22/0xca
[   10.398204]  ra : pwm_get_state_hw+0x10c/0x132
[   10.402651] epc : ffffffff804f8d48 ra : ffffffff804f81f4 sp : 
ffffffc600f1b5c0
[   10.409875]  gp : ffffffff814f8658 tp : ffffffd6c3efe400 t0 : 
0000000000000000
[   10.417096]  t1 : 0000000000000040 t2 : ffffffff814a0498 s0 : 
ffffffc600f1b600
[   10.424317]  s1 : ffffffd6c0b9b800 a0 : ffffffc6007b0000 a1 : 
ffffffd6c0b9bbb8
[   10.431539]  a2 : ffffffc600f1b690 a3 : 0000000000000000 a4 : 
0000000000000000
[   10.438760]  a5 : 0000000200000001 a6 : ffffffd6c07fd918 a7 : 
0000000000000000
[   10.445982]  s2 : ffffffff80e6b2c0 s3 : ffffffd6c0b9bec0 s4 : 
ffffffc600f1b690
[   10.453204]  s5 : ffffffd6c0b9bbb8 s6 : 0000000000000000 s7 : 
ffffffd6c0b9bb78
[   10.460426]  s8 : ffffffc600f1bc90 s9 : 0000000000000000 s10: 
ffffffc600f1bc40
[   10.467648]  s11: 0000000000000001 t3 : 0000000000000000 t4 : 
0000000000000002
[   10.474868]  t5 : 0000000000000000 t6 : 0000000000000402
[   10.480179] status: 0000000200000120 badaddr: 0000000200000001 cause: 
000000000000000d
[   10.488096] [<ffffffff804f8d48>] ocores_pwm_get_state+0x22/0xca
[   10.494019] [<ffffffff804f81f4>] pwm_get_state_hw+0x10c/0x132
[   10.499768] [<ffffffff804f8338>] pwm_request_from_chip.part.0+0x11e/0x182
[   10.506556] [<ffffffff804f8640>] of_pwm_xlate_with_flags+0x26/0x64
[   10.512739] [<ffffffff804f7084>] of_pwm_get+0x146/0x1ea
[   10.517967] [<ffffffff804f84e8>] pwm_get+0x14c/0x1d4
[   10.522934] [<ffffffff804f8582>] devm_pwm_get+0x12/0x4e
[   10.528166] [<ffffffff018ab2ae>] pwm_backlight_probe+0x102/0x84a [pwm_bl]
[   10.534963] [<ffffffff805a05c4>] platform_probe+0x4e/0xb2
[   10.540369] [<ffffffff8059e0b0>] really_probe+0x84/0x22a
[   10.545684] [<ffffffff8059e2b2>] __driver_probe_device+0x5c/0xdc
[   10.551694] [<ffffffff8059e3f4>] driver_probe_device+0x2c/0xb2
[   10.557529] [<ffffffff8059e574>] __driver_attach+0x6c/0x11a
[   10.563104] [<ffffffff8059c1c4>] bus_for_each_dev+0x62/0xb0
[   10.568679] [<ffffffff8059daf2>] driver_attach+0x1a/0x22
[   10.573993] [<ffffffff8059d3e8>] bus_add_driver+0xce/0x1d6
[   10.579481] [<ffffffff8059f114>] driver_register+0x40/0xda
[   10.584970] [<ffffffff805a034c>] __platform_driver_register+0x1c/0x24
[   10.591415] [<ffffffff018b1020>] 
pwm_backlight_driver_init+0x20/0x1000 [pwm_bl]
[   10.598730] [<ffffffff8000f12c>] do_one_initcall+0x5c/0x1aa
[   10.604306] [<ffffffff8009f992>] do_init_module+0x52/0x1b2
[   10.609796] [<ffffffff800a10f2>] load_module+0x1374/0x177e
[   10.615284] [<ffffffff800a16c0>] init_module_from_file+0x76/0xae
[   10.621292] [<ffffffff800a18e4>] __riscv_sys_finit_module+0x1b4/0x31c
[   10.627735] [<ffffffff80983cf4>] do_trap_ecall_u+0x1a8/0x1d4
[   10.633399] [<ffffffff8098d8a2>] 
_new_vmalloc_restore_context_a0+0xc2/0xce
[   10.640285] Code: ec4e 0080 3983 0785 8aae 8a32 b783 0009 b503 0089 
(639c) c399
[   10.647772] ---[ end trace 0000000000000000 ]---


kind regards,
Maud Spierings

RE: [PATCH v16] pwm: opencores: Add PWM driver support

[William Qiu]

> -----Original Message-----
> From: Maud Spierings <maud_spierings@hotmail.com>
> Sent: 2025年1月5日 22:51
> To: William Qiu <william.qiu@starfivetech.com>
> Cc: Hal Feng <hal.feng@starfivetech.com>; linux-kernel@vger.kernel.org;
> linux-pwm@vger.kernel.org; p.zabel@pengutronix.de; ukleinek@kernel.org
> Subject: Re: [PATCH v16] pwm: opencores: Add PWM driver support
> 
> Hello William,
> 
> 
> I tested this patch on my deepcomputing fml13v01 with a pwm-backlight and I
> am getting an oops:
> 
> [   10.308290] Unable to handle kernel access to user memory without
> uaccess routines at virtual address 0000000200000001 [   10.319038] Oops
> [#1] [   10.321318] Modules linked in: pwm_bl(+) sfctemp designware_i2s
> snd_soc_spdif_tx dm_mod configfs ip_tables x_tables vs_drm drm_dma_helper
> stmmac pcs_xpcs pcie_starfive phy_jh7110_usb phy_jh7110_pcie phy_jh
> 7110_dphy_rx phy_generic overlay jh7110_trng jh7110_tdm jh7110_pwmdac
> inno_hdmi drm_kms_helper i2c_hid_of i2c_hid hid_multitouch hci_uart drm
> drm_panel_orientation_quirks clk_starfive_jh7110_vout clk_starfive
> _jh7110_stg clk_starfive_jh7110_isp clk_starfive_jh7110_aon
> clk_starfive_jh7100_audio cdns3_starfive cdns3 cdns_usb_common roles btintel
> bridge stp llc bluetooth libaes ecdh_generic ecc backlight efivarfs
> [   10.375483] CPU: 3 UID: 0 PID: 307 Comm: (udev-worker) Not tainted
> 6.13.0-rc4-00032-ga27a3464e6ab-dirty #16 [   10.385227] Hardware name:
> Unknown Unknown Product/Unknown Product, BIOS 2021.10 10/01/2021
> [   10.393576] epc : ocores_pwm_get_state+0x22/0xca [   10.398204]  ra :
> pwm_get_state_hw+0x10c/0x132 [   10.402651] epc : ffffffff804f8d48 ra :
> ffffffff804f81f4 sp :
> ffffffc600f1b5c0
> [   10.409875]  gp : ffffffff814f8658 tp : ffffffd6c3efe400 t0 :
> 0000000000000000
> [   10.417096]  t1 : 0000000000000040 t2 : ffffffff814a0498 s0 :
> ffffffc600f1b600
> [   10.424317]  s1 : ffffffd6c0b9b800 a0 : ffffffc6007b0000 a1 :
> ffffffd6c0b9bbb8
> [   10.431539]  a2 : ffffffc600f1b690 a3 : 0000000000000000 a4 :
> 0000000000000000
> [   10.438760]  a5 : 0000000200000001 a6 : ffffffd6c07fd918 a7 :
> 0000000000000000
> [   10.445982]  s2 : ffffffff80e6b2c0 s3 : ffffffd6c0b9bec0 s4 :
> ffffffc600f1b690
> [   10.453204]  s5 : ffffffd6c0b9bbb8 s6 : 0000000000000000 s7 :
> ffffffd6c0b9bb78
> [   10.460426]  s8 : ffffffc600f1bc90 s9 : 0000000000000000 s10:
> ffffffc600f1bc40
> [   10.467648]  s11: 0000000000000001 t3 : 0000000000000000 t4 :
> 0000000000000002
> [   10.474868]  t5 : 0000000000000000 t6 : 0000000000000402
> [   10.480179] status: 0000000200000120 badaddr: 0000000200000001
> cause:
> 000000000000000d
> [   10.488096] [<ffffffff804f8d48>] ocores_pwm_get_state+0x22/0xca
> [   10.494019] [<ffffffff804f81f4>] pwm_get_state_hw+0x10c/0x132
> [   10.499768] [<ffffffff804f8338>]
> pwm_request_from_chip.part.0+0x11e/0x182
> [   10.506556] [<ffffffff804f8640>] of_pwm_xlate_with_flags+0x26/0x64
> [   10.512739] [<ffffffff804f7084>] of_pwm_get+0x146/0x1ea [   10.517967]
> [<ffffffff804f84e8>] pwm_get+0x14c/0x1d4 [   10.522934] [<ffffffff804f8582>]
> devm_pwm_get+0x12/0x4e [   10.528166] [<ffffffff018ab2ae>]
> pwm_backlight_probe+0x102/0x84a [pwm_bl] [   10.534963]
> [<ffffffff805a05c4>] platform_probe+0x4e/0xb2 [   10.540369]
> [<ffffffff8059e0b0>] really_probe+0x84/0x22a [   10.545684]
> [<ffffffff8059e2b2>] __driver_probe_device+0x5c/0xdc [   10.551694]
> [<ffffffff8059e3f4>] driver_probe_device+0x2c/0xb2 [   10.557529]
> [<ffffffff8059e574>] __driver_attach+0x6c/0x11a [   10.563104]
> [<ffffffff8059c1c4>] bus_for_each_dev+0x62/0xb0 [   10.568679]
> [<ffffffff8059daf2>] driver_attach+0x1a/0x22 [   10.573993]
> [<ffffffff8059d3e8>] bus_add_driver+0xce/0x1d6 [   10.579481]
> [<ffffffff8059f114>] driver_register+0x40/0xda [   10.584970]
> [<ffffffff805a034c>] __platform_driver_register+0x1c/0x24
> [   10.591415] [<ffffffff018b1020>]
> pwm_backlight_driver_init+0x20/0x1000 [pwm_bl] [   10.598730]
> [<ffffffff8000f12c>] do_one_initcall+0x5c/0x1aa [   10.604306]
> [<ffffffff8009f992>] do_init_module+0x52/0x1b2 [   10.609796]
> [<ffffffff800a10f2>] load_module+0x1374/0x177e [   10.615284]
> [<ffffffff800a16c0>] init_module_from_file+0x76/0xae [   10.621292]
> [<ffffffff800a18e4>] __riscv_sys_finit_module+0x1b4/0x31c
> [   10.627735] [<ffffffff80983cf4>] do_trap_ecall_u+0x1a8/0x1d4
> [   10.633399] [<ffffffff8098d8a2>]
> _new_vmalloc_restore_context_a0+0xc2/0xce
> [   10.640285] Code: ec4e 0080 3983 0785 8aae 8a32 b783 0009 b503 0089
> (639c) c399
> [   10.647772] ---[ end trace 0000000000000000 ]---
> 
> 
> kind regards,
> Maud Spierings


Hi Maud,
Thanks for your email, I'll fix it in next version.

Best Regards,
William