| Message ID | 20240223084332.100410-1-william.qiu@starfivetech.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v11] pwm: opencores: Add PWM driver support | expand |
> -----Original Message----- > From: William Qiu <william.qiu@starfivetech.com> > Sent: 2024年2月23日 16:44 > To: linux-kernel@vger.kernel.org; linux-pwm@vger.kernel.org > Cc: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>; Hal Feng > <hal.feng@starfivetech.com>; Philipp Zabel <p.zabel@pengutronix.de>; William > Qiu <william.qiu@starfivetech.com> > Subject: [PATCH v11] 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 | 232 > +++++++++++++++++++++++++++++++++++++++ > 4 files changed, 252 insertions(+) > create mode 100644 drivers/pwm/pwm-ocores.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 9ed4d3868539..12ea5e86fc23 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -16414,6 +16414,13 @@ F: Documentation/i2c/busses/i2c-ocores.rst > F: drivers/i2c/busses/i2c-ocores.c > F: include/linux/platform_data/i2c-ocores.h > > +OPENCORES PWM DRIVER > +M: William Qiu <william.qiu@starfivetech.com> > +M: Hal Feng <hal.feng@starfivetech.com> > +S: Supported > +F: Documentation/devicetree/bindings/pwm/opencores,pwm.yaml > +F: drivers/pwm/pwm-ocores.c > + > OPENRISC ARCHITECTURE > M: Jonas Bonn <jonas@southpole.se> > M: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi> > diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig index > 4b956d661755..d87e1bb350ba 100644 > --- a/drivers/pwm/Kconfig > +++ b/drivers/pwm/Kconfig > @@ -444,6 +444,18 @@ config PWM_NTXEC > controller found in certain e-book readers designed by the original > design manufacturer Netronix. > > +config PWM_OCORES > + tristate "OpenCores PWM support" > + depends on HAS_IOMEM && OF > + depends on COMMON_CLK && 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 > c5ec9e168ee7..517c4f643058 100644 > --- a/drivers/pwm/Makefile > +++ b/drivers/pwm/Makefile > @@ -40,6 +40,7 @@ obj-$(CONFIG_PWM_MICROCHIP_CORE) += > pwm-microchip-core.o > obj-$(CONFIG_PWM_MTK_DISP) += pwm-mtk-disp.o > obj-$(CONFIG_PWM_MXS) += pwm-mxs.o > obj-$(CONFIG_PWM_NTXEC) += pwm-ntxec.o > +obj-$(CONFIG_PWM_OCORES) += pwm-ocores.o > obj-$(CONFIG_PWM_OMAP_DMTIMER) += pwm-omap-dmtimer.o > obj-$(CONFIG_PWM_PCA9685) += pwm-pca9685.o > obj-$(CONFIG_PWM_PXA) += pwm-pxa.o > diff --git a/drivers/pwm/pwm-ocores.c b/drivers/pwm/pwm-ocores.c new file > mode 100644 index 000000000000..874bc630bf2d > --- /dev/null > +++ b/drivers/pwm/pwm-ocores.c > @@ -0,0 +1,232 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * OpenCores PWM Driver > + * > + * https://opencores.org/projects/ptc > + * > + * Copyright (C) 2018-2023 StarFive Technology Co., Ltd. > + * > + * Limitations: > + * - The hardware only do inverted polarity. > + * - The hardware minimum period / duty_cycle is (1 / pwm_apb clock > frequency) ns. > + * - The hardware maximum period / duty_cycle is (U32_MAX / pwm_apb clock > frequency) ns. > + */ > + > +#include <linux/clk.h> > +#include <linux/io.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/of_device.h> > +#include <linux/platform_device.h> > +#include <linux/pwm.h> > +#include <linux/reset.h> > +#include <linux/slab.h> > + > +/* OCPWM_CTRL register bits*/ > +#define REG_OCPWM_EN BIT(0) > +#define REG_OCPWM_ECLK BIT(1) > +#define REG_OCPWM_NEC BIT(2) > +#define REG_OCPWM_OE BIT(3) > +#define REG_OCPWM_SIGNLE BIT(4) > +#define REG_OCPWM_INTE BIT(5) > +#define REG_OCPWM_INT BIT(6) > +#define REG_OCPWM_CNTRRST BIT(7) > +#define REG_OCPWM_CAPTE BIT(8) > + > +struct ocores_pwm_device { > + struct pwm_chip chip; > + struct clk *clk; > + struct reset_control *rst; > + const struct ocores_pwm_data *data; > + void __iomem *regs; > + u32 clk_rate; /* PWM APB clock frequency */ }; > + > +struct ocores_pwm_data { > + void __iomem *(*get_ch_base)(void __iomem *base, unsigned int > +channel); }; > + > +static inline u32 ocores_readl(struct ocores_pwm_device *ddata, > + unsigned int channel, > + unsigned int offset) > +{ > + void __iomem *base = ddata->data->get_ch_base ? > + ddata->data->get_ch_base(ddata->regs, channel) : > ddata->regs; > + > + return readl(base + offset); > +} > + > +static inline void ocores_writel(struct ocores_pwm_device *ddata, > + unsigned int channel, > + unsigned int offset, u32 val) > +{ > + void __iomem *base = ddata->data->get_ch_base ? > + ddata->data->get_ch_base(ddata->regs, channel) : > ddata->regs; > + > + writel(val, base + offset); > +} > + > +static inline struct ocores_pwm_device *chip_to_ocores(struct pwm_chip > +*chip) { > + return container_of(chip, struct ocores_pwm_device, chip); } > + > +static void __iomem *starfive_jh71x0_get_ch_base(void __iomem *base, > + unsigned int channel) > +{ > + unsigned int offset = (channel > 3 ? 1 << 15 : 0) + (channel & 3) * > +0x10; > + > + return base + offset; > +} > + > +static int ocores_pwm_get_state(struct pwm_chip *chip, > + struct pwm_device *pwm, > + struct pwm_state *state) > +{ > + struct ocores_pwm_device *ddata = chip_to_ocores(chip); > + u32 period_data, duty_data, ctrl_data; > + > + period_data = ocores_readl(ddata, pwm->hwpwm, 0x8); > + duty_data = ocores_readl(ddata, pwm->hwpwm, 0x4); > + ctrl_data = ocores_readl(ddata, pwm->hwpwm, 0xC); > + > + state->period = DIV_ROUND_UP_ULL((u64)period_data * NSEC_PER_SEC, > ddata->clk_rate); > + state->duty_cycle = DIV_ROUND_UP_ULL((u64)duty_data * > NSEC_PER_SEC, ddata->clk_rate); > + state->polarity = PWM_POLARITY_INVERSED; > + state->enabled = (ctrl_data & REG_OCPWM_EN) ? true : false; > + > + return 0; > +} > + > +static int ocores_pwm_apply(struct pwm_chip *chip, > + struct pwm_device *pwm, > + const struct pwm_state *state) > +{ > + struct ocores_pwm_device *ddata = chip_to_ocores(chip); > + u32 ctrl_data = 0; > + u64 period_data, duty_data; > + > + if (state->polarity != PWM_POLARITY_INVERSED) > + return -EINVAL; > + > + ctrl_data = ocores_readl(ddata, pwm->hwpwm, 0xC); > + ocores_writel(ddata, pwm->hwpwm, 0xC, 0); > + > + period_data = DIV_ROUND_DOWN_ULL(state->period * ddata->clk_rate, > NSEC_PER_SEC); > + if (period_data <= U32_MAX) > + ocores_writel(ddata, pwm->hwpwm, 0x8, (u32)period_data); > + else > + return -EINVAL; > + > + duty_data = DIV_ROUND_DOWN_ULL(state->duty_cycle * > ddata->clk_rate, NSEC_PER_SEC); > + if (duty_data <= U32_MAX) > + ocores_writel(ddata, pwm->hwpwm, 0x4, (u32)duty_data); > + else > + return -EINVAL; > + > + ocores_writel(ddata, pwm->hwpwm, 0xC, 0); > + > + if (state->enabled) { > + ctrl_data = ocores_readl(ddata, pwm->hwpwm, 0xC); > + ocores_writel(ddata, pwm->hwpwm, 0xC, ctrl_data | > REG_OCPWM_EN | REG_OCPWM_OE); > + } > + > + return 0; > +} > + > +static const struct pwm_ops ocores_pwm_ops = { > + .get_state = ocores_pwm_get_state, > + .apply = ocores_pwm_apply, > +}; > + > +static const struct ocores_pwm_data jh7100_pwm_data = { > + .get_ch_base = starfive_jh71x0_get_ch_base, }; > + > +static const struct ocores_pwm_data jh7110_pwm_data = { > + .get_ch_base = starfive_jh71x0_get_ch_base, }; > + > +static const struct of_device_id ocores_pwm_of_match[] = { > + { .compatible = "opencores,pwm-v1" }, > + { .compatible = "starfive,jh7100-pwm", .data = &jh7100_pwm_data}, > + { .compatible = "starfive,jh7110-pwm", .data = &jh7110_pwm_data}, > + { /* sentinel */ } > +}; > +MODULE_DEVICE_TABLE(of, ocores_pwm_of_match); > + > +static void ocores_reset_control_assert(void *data) { > + reset_control_assert(data); > +} > + > +static int ocores_pwm_probe(struct platform_device *pdev) { > + const struct of_device_id *id; > + struct device *dev = &pdev->dev; > + struct ocores_pwm_device *ddata; > + struct pwm_chip *chip; > + int ret; > + > + id = of_match_device(ocores_pwm_of_match, dev); > + if (!id) > + return -EINVAL; > + > + ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL); > + if (!ddata) > + return -ENOMEM; > + > + ddata->data = id->data; > + chip = &ddata->chip; > + chip->dev = dev; > + chip->ops = &ocores_pwm_ops; > + chip->npwm = 8; > + > + ddata->regs = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(ddata->regs)) > + return dev_err_probe(dev, PTR_ERR(ddata->regs), > + "Unable to map IO resources\n"); > + > + ddata->clk = devm_clk_get_enabled(dev, NULL); > + if (IS_ERR(ddata->clk)) > + return dev_err_probe(dev, PTR_ERR(ddata->clk), > + "Unable to get pwm's clock\n"); > + > + ddata->rst = devm_reset_control_get_optional_exclusive(dev, NULL); > + if (IS_ERR(ddata->rst)) > + return dev_err_probe(dev, PTR_ERR(ddata->rst), > + "Unable to get pwm's reset\n"); > + > + reset_control_deassert(ddata->rst); > + > + ret = devm_add_action_or_reset(dev, ocores_reset_control_assert, > ddata->rst); > + if (ret) > + return ret; > + > + ddata->clk_rate = clk_get_rate(ddata->clk); > + if (ddata->clk_rate <= 0) > + return dev_err_probe(dev, ddata->clk_rate, > + "Unable to get clock's rate\n"); > + > + ret = devm_pwmchip_add(dev, chip); > + if (ret < 0) > + return dev_err_probe(dev, ret, "Could not register PWM chip\n"); > + > + platform_set_drvdata(pdev, ddata); > + > + return ret; > +} > + > +static struct platform_driver ocores_pwm_driver = { > + .probe = ocores_pwm_probe, > + .driver = { > + .name = "ocores-pwm", > + .of_match_table = ocores_pwm_of_match, > + }, > +}; > +module_platform_driver(ocores_pwm_driver); > + > +MODULE_AUTHOR("Jieqin Chen"); > +MODULE_AUTHOR("Hal Feng <hal.feng@starfivetech.com>"); > +MODULE_DESCRIPTION("OpenCores PWM PTC driver"); > MODULE_LICENSE("GPL"); > -- > 2.34.1 Hi 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
On Tue, Mar 05, 2024 at 06:12:23AM +0000, William Qiu wrote: > Could you please help me review this patch series to see if there is > anything that needs to be modified? If not, could you help me integrate > this patch into the mainline? Thanks. I know I'm behind on reviewing this driver. There are a few more and I still have your patch on my radar. New drivers require a big effort on my side for review---each revision takes easily >1h for me to comment. When I find time to review, I usually pick the oldest on https://patchwork.ozlabs.org/project/linux-pwm/list/ to reply. So as long as your patch appears there, it's not lost. So I ask you for some more patience. Best regards Uwe
> -----Original Message----- > From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > Sent: 2024年3月5日 18:39 > 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 v11] pwm: opencores: Add PWM driver support > > On Tue, Mar 05, 2024 at 06:12:23AM +0000, William Qiu wrote: > > Could you please help me review this patch series to see if there is > > anything that needs to be modified? If not, could you help me > > integrate this patch into the mainline? Thanks. > > I know I'm behind on reviewing this driver. There are a few more and I still have > your patch on my radar. New drivers require a big effort on my side for > review---each revision takes easily >1h for me to comment. > When I find time to review, I usually pick the oldest on > https://patchwork.ozlabs.org/project/linux-pwm/list/ to reply. So as long as your > patch appears there, it's not lost. > > So I ask you for some more patience. > > Best regards > Uwe > > -- > Pengutronix e.K. | Uwe Kleine-König > | > Industrial Linux Solutions | https://www.pengutronix.de/ | Hi Uwe, Sorry for that. I will wait so patiently. Thanks for taking time. Best Regards, William
Hello, thanks for your patience to wait for my review. On Fri, Feb 23, 2024 at 04:43:32PM +0800, William Qiu wrote: > Add driver for OpenCores PWM Controller. And add compatibility code > which based on StarFive SoC. > > Co-developed-by: Hal Feng <hal.feng@starfivetech.com> > Signed-off-by: Hal Feng <hal.feng@starfivetech.com> > Signed-off-by: William Qiu <william.qiu@starfivetech.com> > --- > MAINTAINERS | 7 ++ > drivers/pwm/Kconfig | 12 ++ > drivers/pwm/Makefile | 1 + > drivers/pwm/pwm-ocores.c | 232 +++++++++++++++++++++++++++++++++++++++ > 4 files changed, 252 insertions(+) > create mode 100644 drivers/pwm/pwm-ocores.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 9ed4d3868539..12ea5e86fc23 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -16414,6 +16414,13 @@ F: Documentation/i2c/busses/i2c-ocores.rst > F: drivers/i2c/busses/i2c-ocores.c > F: include/linux/platform_data/i2c-ocores.h > > +OPENCORES PWM DRIVER > +M: William Qiu <william.qiu@starfivetech.com> > +M: Hal Feng <hal.feng@starfivetech.com> > +S: Supported > +F: Documentation/devicetree/bindings/pwm/opencores,pwm.yaml > +F: drivers/pwm/pwm-ocores.c > + > OPENRISC ARCHITECTURE > M: Jonas Bonn <jonas@southpole.se> > M: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi> > diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig > index 4b956d661755..d87e1bb350ba 100644 > --- a/drivers/pwm/Kconfig > +++ b/drivers/pwm/Kconfig > @@ -444,6 +444,18 @@ config PWM_NTXEC > controller found in certain e-book readers designed by the original > design manufacturer Netronix. > > +config PWM_OCORES > + tristate "OpenCores PWM support" OpenCores PTC 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 c5ec9e168ee7..517c4f643058 100644 > --- a/drivers/pwm/Makefile > +++ b/drivers/pwm/Makefile > @@ -40,6 +40,7 @@ obj-$(CONFIG_PWM_MICROCHIP_CORE) += pwm-microchip-core.o > obj-$(CONFIG_PWM_MTK_DISP) += pwm-mtk-disp.o > obj-$(CONFIG_PWM_MXS) += pwm-mxs.o > obj-$(CONFIG_PWM_NTXEC) += pwm-ntxec.o > +obj-$(CONFIG_PWM_OCORES) += pwm-ocores.o > obj-$(CONFIG_PWM_OMAP_DMTIMER) += pwm-omap-dmtimer.o > obj-$(CONFIG_PWM_PCA9685) += pwm-pca9685.o > obj-$(CONFIG_PWM_PXA) += pwm-pxa.o > diff --git a/drivers/pwm/pwm-ocores.c b/drivers/pwm/pwm-ocores.c > new file mode 100644 > index 000000000000..874bc630bf2d > --- /dev/null > +++ b/drivers/pwm/pwm-ocores.c > @@ -0,0 +1,232 @@ > +// 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, ocores_pwm_readl is a tad longer (which is annoying), but IMHO the advantage of the longer name (no clash with other ocores IP drivers, being able to easily setup ftrace filtering for all functions in this driver) outweighs the shorter name. Can you please update accordingly. (There are a few more symbols that the same treatment.) > + 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 void __iomem *starfive_jh71x0_get_ch_base(void __iomem *base, > + unsigned int channel) > +{ > + unsigned int offset = (channel > 3 ? 1 << 15 : 0) + (channel & 3) * 0x10; offset = (channel & 4) << 13 | (channel & 3) << 4 results in the same offsets and can be compiled to more efficient code. (Well, at least on ARM, I suspect the same applies to riscv.) > + 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); Can you please give symbolic names to these offsets? > [...] > + > +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); The multiplication might overflow. Please use mul_u64_u32_div and in .probe assert that ddata->clk_rate <= NSEC_PER_SEC. > + if (period_data <= U32_MAX) > + ocores_writel(ddata, pwm->hwpwm, 0x8, (u32)period_data); > + else > + return -EINVAL; Please make this: if (period_data <= U32_MAX) period_data = U32_MAX; What happens if period_data == 0? I guess this is a problem and you should return -EINVAL in that case. > + 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); What is the effect on this one? I guess it disables the output? Is this necessary? Does updating the configuration complete the currently running period? Please document in the Limitations paragraph if there are possible glitches (e.g. when the period register is written but the duty_cycle register not yet) and if the current period is completed. > [...] > +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; This needs updating with the changes that got into 6.9-rc1. See ae8635e99c5cc752e204ab9ee8869ec54a9223f0 for a commit you might want to take as a template for the change needed here. > + chip->dev = dev; > + chip->ops = &ocores_pwm_ops; > + chip->npwm = 8; > + > + ddata->regs = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(ddata->regs)) > + return dev_err_probe(dev, PTR_ERR(ddata->regs), > + "Unable to map IO resources\n"); > + > + ddata->clk = devm_clk_get_enabled(dev, NULL); This member is only used here in .probe(). So it doesn't need to be stored in struct ocores_pwm_device. > + if (IS_ERR(ddata->clk)) > + return dev_err_probe(dev, PTR_ERR(ddata->clk), > + "Unable to get pwm's clock\n"); > + > + ddata->rst = devm_reset_control_get_optional_exclusive(dev, NULL); Same as for clk, rst is only used here. > + if (IS_ERR(ddata->rst)) > + return dev_err_probe(dev, PTR_ERR(ddata->rst), > + "Unable to get pwm's reset\n"); > + > + reset_control_deassert(ddata->rst); > + > + ret = devm_add_action_or_reset(dev, ocores_reset_control_assert, ddata->rst); > + if (ret) > + return ret; Please add a call to devm_clk_rate_exclusive_get() before storing the rate and relying on it not changing. > + ddata->clk_rate = clk_get_rate(ddata->clk); > + if (ddata->clk_rate <= 0) > + return dev_err_probe(dev, ddata->clk_rate, > + "Unable to get clock's rate\n"); > + > + ret = devm_pwmchip_add(dev, chip); > + if (ret < 0) > + return dev_err_probe(dev, ret, "Could not register PWM chip\n"); > + > + platform_set_drvdata(pdev, ddata); This is unused. > + return ret; Here ret is always 0, please use return 0 here. > +} > + > +static struct platform_driver ocores_pwm_driver = { > + .probe = ocores_pwm_probe, > + .driver = { > + .name = "ocores-pwm", > + .of_match_table = ocores_pwm_of_match, > + }, > +}; > +module_platform_driver(ocores_pwm_driver); > + > +MODULE_AUTHOR("Jieqin Chen"); > +MODULE_AUTHOR("Hal Feng <hal.feng@starfivetech.com>"); > +MODULE_DESCRIPTION("OpenCores PWM PTC driver"); The hardware unit is called PTC (PWM/Timer/Counter), so "OpenCores PTC PWM driver" would be more appropriate?! > +MODULE_LICENSE("GPL"); Best regards Uwe
Hello, Sorry for late reply. > -----Original Message----- > From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > Sent: 2024年4月14日 1:01 > 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 v11] pwm: opencores: Add PWM driver support > > Hello, > > thanks for your patience to wait for my review. > > On Fri, Feb 23, 2024 at 04:43:32PM +0800, William Qiu wrote: > > Add driver for OpenCores PWM Controller. And add compatibility code > > which based on StarFive SoC. > > > > Co-developed-by: Hal Feng <hal.feng@starfivetech.com> > > Signed-off-by: Hal Feng <hal.feng@starfivetech.com> > > Signed-off-by: William Qiu <william.qiu@starfivetech.com> > > --- > > MAINTAINERS | 7 ++ > > drivers/pwm/Kconfig | 12 ++ > > drivers/pwm/Makefile | 1 + > > drivers/pwm/pwm-ocores.c | 232 > > +++++++++++++++++++++++++++++++++++++++ > > 4 files changed, 252 insertions(+) > > create mode 100644 drivers/pwm/pwm-ocores.c > > > > diff --git a/MAINTAINERS b/MAINTAINERS index > > 9ed4d3868539..12ea5e86fc23 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -16414,6 +16414,13 @@ F: Documentation/i2c/busses/i2c-ocores.rst > > F: drivers/i2c/busses/i2c-ocores.c > > F: include/linux/platform_data/i2c-ocores.h > > > > +OPENCORES PWM DRIVER > > +M: William Qiu <william.qiu@starfivetech.com> > > +M: Hal Feng <hal.feng@starfivetech.com> > > +S: Supported > > +F: Documentation/devicetree/bindings/pwm/opencores,pwm.yaml > > +F: drivers/pwm/pwm-ocores.c > > + > > OPENRISC ARCHITECTURE > > M: Jonas Bonn <jonas@southpole.se> > > M: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi> > > diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig index > > 4b956d661755..d87e1bb350ba 100644 > > --- a/drivers/pwm/Kconfig > > +++ b/drivers/pwm/Kconfig > > @@ -444,6 +444,18 @@ config PWM_NTXEC > > controller found in certain e-book readers designed by the original > > design manufacturer Netronix. > > > > +config PWM_OCORES > > + tristate "OpenCores PWM support" > > OpenCores PTC PWM support? > Will update. > > + 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 > > c5ec9e168ee7..517c4f643058 100644 > > --- a/drivers/pwm/Makefile > > +++ b/drivers/pwm/Makefile > > @@ -40,6 +40,7 @@ obj-$(CONFIG_PWM_MICROCHIP_CORE) += > pwm-microchip-core.o > > obj-$(CONFIG_PWM_MTK_DISP) += pwm-mtk-disp.o > > obj-$(CONFIG_PWM_MXS) += pwm-mxs.o > > obj-$(CONFIG_PWM_NTXEC) += pwm-ntxec.o > > +obj-$(CONFIG_PWM_OCORES) += pwm-ocores.o > > obj-$(CONFIG_PWM_OMAP_DMTIMER) += pwm-omap-dmtimer.o > > obj-$(CONFIG_PWM_PCA9685) += pwm-pca9685.o > > obj-$(CONFIG_PWM_PXA) += pwm-pxa.o > > diff --git a/drivers/pwm/pwm-ocores.c b/drivers/pwm/pwm-ocores.c new > > file mode 100644 index 000000000000..874bc630bf2d > > --- /dev/null > > +++ b/drivers/pwm/pwm-ocores.c > > @@ -0,0 +1,232 @@ > > +// 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, > > ocores_pwm_readl is a tad longer (which is annoying), but IMHO the advantage > of the longer name (no clash with other ocores IP drivers, being able to easily > setup ftrace filtering for all functions in this > driver) outweighs the shorter name. Can you please update accordingly. > (There are a few more symbols that the same treatment.) > Will update. > > + 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 void __iomem *starfive_jh71x0_get_ch_base(void __iomem *base, > > + unsigned int channel) > > +{ > > + unsigned int offset = (channel > 3 ? 1 << 15 : 0) + (channel & 3) * > > +0x10; > > offset = (channel & 4) << 13 | (channel & 3) << 4 > > results in the same offsets and can be compiled to more efficient code. > (Well, at least on ARM, I suspect the same applies to riscv.) > I'll try it first, and update it later. > > + 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); > > Can you please give symbolic names to these offsets? > Will name it. > > [...] > > + > > +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); > > The multiplication might overflow. Please use mul_u64_u32_div and in .probe > assert that ddata->clk_rate <= NSEC_PER_SEC. > Will update. > > + if (period_data <= U32_MAX) > > + ocores_writel(ddata, pwm->hwpwm, 0x8, (u32)period_data); > > + else > > + return -EINVAL; > > Please make this: > > if (period_data <= U32_MAX) > period_data = U32_MAX; > > What happens if period_data == 0? I guess this is a problem and you should > return -EINVAL in that case. > Will fix. > > + 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); > > What is the effect on this one? I guess it disables the output? Is this necessary? > Does updating the configuration complete the currently running period? > > Please document in the Limitations paragraph if there are possible glitches (e.g. > when the period register is written but the duty_cycle register not yet) and if > the current period is completed. > Will check it. > > [...] > > +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; > > This needs updating with the changes that got into 6.9-rc1. See > ae8635e99c5cc752e204ab9ee8869ec54a9223f0 for a commit you might want > to take as a template for the change needed here. > Will fix. > > + chip->dev = dev; > > + chip->ops = &ocores_pwm_ops; > > + chip->npwm = 8; > > + > > + ddata->regs = devm_platform_ioremap_resource(pdev, 0); > > + if (IS_ERR(ddata->regs)) > > + return dev_err_probe(dev, PTR_ERR(ddata->regs), > > + "Unable to map IO resources\n"); > > + > > + ddata->clk = devm_clk_get_enabled(dev, NULL); > > This member is only used here in .probe(). So it doesn't need to be stored in > struct ocores_pwm_device. > Will drop it from struct ocores_pwm_device. > > + if (IS_ERR(ddata->clk)) > > + return dev_err_probe(dev, PTR_ERR(ddata->clk), > > + "Unable to get pwm's clock\n"); > > + > > + ddata->rst = devm_reset_control_get_optional_exclusive(dev, NULL); > > Same as for clk, rst is only used here. > ... > > + if (IS_ERR(ddata->rst)) > > + return dev_err_probe(dev, PTR_ERR(ddata->rst), > > + "Unable to get pwm's reset\n"); > > + > > + reset_control_deassert(ddata->rst); > > + > > + ret = devm_add_action_or_reset(dev, ocores_reset_control_assert, > ddata->rst); > > + if (ret) > > + return ret; > > Please add a call to devm_clk_rate_exclusive_get() before storing the rate and > relying on it not changing. > Will add. > > + ddata->clk_rate = clk_get_rate(ddata->clk); > > + if (ddata->clk_rate <= 0) > > + return dev_err_probe(dev, ddata->clk_rate, > > + "Unable to get clock's rate\n"); > > + > > + ret = devm_pwmchip_add(dev, chip); > > + if (ret < 0) > > + return dev_err_probe(dev, ret, "Could not register PWM chip\n"); > > + > > + platform_set_drvdata(pdev, ddata); > > This is unused. > Will drop. > > + return ret; > > Here ret is always 0, please use return 0 here. > Will update. > > +} > > + > > +static struct platform_driver ocores_pwm_driver = { > > + .probe = ocores_pwm_probe, > > + .driver = { > > + .name = "ocores-pwm", > > + .of_match_table = ocores_pwm_of_match, > > + }, > > +}; > > +module_platform_driver(ocores_pwm_driver); > > + > > +MODULE_AUTHOR("Jieqin Chen"); > > +MODULE_AUTHOR("Hal Feng <hal.feng@starfivetech.com>"); > > +MODULE_DESCRIPTION("OpenCores PWM PTC driver"); > > The hardware unit is called PTC (PWM/Timer/Counter), so "OpenCores PTC > PWM driver" would be more appropriate?! > Will update. > > +MODULE_LICENSE("GPL"); > > Best regards > Uwe > > -- > Pengutronix e.K. | Uwe Kleine-König > | > Industrial Linux Solutions | https://www.pengutronix.de/ | Thank you for taking time to review this patch and give helpful suggestions. Best regards, William
Hello William,
On Fri, Apr 19, 2024 at 08:10:18AM +0000, William Qiu wrote:
> Sorry for late reply.
I didn't hold my breath. And if there are no questions on your side (or
on mine that you want to answer), I don't expect an answer. Just sending
out a new revision when you find the time to do that is fine. No need
for excuses. (And compared to the time I needed to review your patch,
you're super fast :-)
Best regards
Uwe
diff --git a/MAINTAINERS b/MAINTAINERS index 9ed4d3868539..12ea5e86fc23 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -16414,6 +16414,13 @@ F: Documentation/i2c/busses/i2c-ocores.rst F: drivers/i2c/busses/i2c-ocores.c F: include/linux/platform_data/i2c-ocores.h +OPENCORES PWM DRIVER +M: William Qiu <william.qiu@starfivetech.com> +M: Hal Feng <hal.feng@starfivetech.com> +S: Supported +F: Documentation/devicetree/bindings/pwm/opencores,pwm.yaml +F: drivers/pwm/pwm-ocores.c + OPENRISC ARCHITECTURE M: Jonas Bonn <jonas@southpole.se> M: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig index 4b956d661755..d87e1bb350ba 100644 --- a/drivers/pwm/Kconfig +++ b/drivers/pwm/Kconfig @@ -444,6 +444,18 @@ config PWM_NTXEC controller found in certain e-book readers designed by the original design manufacturer Netronix. +config PWM_OCORES + tristate "OpenCores PWM support" + depends on HAS_IOMEM && OF + depends on COMMON_CLK && 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 c5ec9e168ee7..517c4f643058 100644 --- a/drivers/pwm/Makefile +++ b/drivers/pwm/Makefile @@ -40,6 +40,7 @@ obj-$(CONFIG_PWM_MICROCHIP_CORE) += pwm-microchip-core.o obj-$(CONFIG_PWM_MTK_DISP) += pwm-mtk-disp.o obj-$(CONFIG_PWM_MXS) += pwm-mxs.o obj-$(CONFIG_PWM_NTXEC) += pwm-ntxec.o +obj-$(CONFIG_PWM_OCORES) += pwm-ocores.o obj-$(CONFIG_PWM_OMAP_DMTIMER) += pwm-omap-dmtimer.o obj-$(CONFIG_PWM_PCA9685) += pwm-pca9685.o obj-$(CONFIG_PWM_PXA) += pwm-pxa.o diff --git a/drivers/pwm/pwm-ocores.c b/drivers/pwm/pwm-ocores.c new file mode 100644 index 000000000000..874bc630bf2d --- /dev/null +++ b/drivers/pwm/pwm-ocores.c @@ -0,0 +1,232 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * OpenCores PWM Driver + * + * https://opencores.org/projects/ptc + * + * Copyright (C) 2018-2023 StarFive Technology Co., Ltd. + * + * Limitations: + * - The hardware only do inverted polarity. + * - The hardware minimum period / duty_cycle is (1 / pwm_apb clock frequency) ns. + * - The hardware maximum period / duty_cycle is (U32_MAX / pwm_apb clock frequency) ns. + */ + +#include <linux/clk.h> +#include <linux/io.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/platform_device.h> +#include <linux/pwm.h> +#include <linux/reset.h> +#include <linux/slab.h> + +/* OCPWM_CTRL register bits*/ +#define REG_OCPWM_EN BIT(0) +#define REG_OCPWM_ECLK BIT(1) +#define REG_OCPWM_NEC BIT(2) +#define REG_OCPWM_OE BIT(3) +#define REG_OCPWM_SIGNLE BIT(4) +#define REG_OCPWM_INTE BIT(5) +#define REG_OCPWM_INT BIT(6) +#define REG_OCPWM_CNTRRST BIT(7) +#define REG_OCPWM_CAPTE BIT(8) + +struct ocores_pwm_device { + struct pwm_chip chip; + struct clk *clk; + struct reset_control *rst; + const struct ocores_pwm_data *data; + void __iomem *regs; + u32 clk_rate; /* PWM APB clock frequency */ +}; + +struct ocores_pwm_data { + void __iomem *(*get_ch_base)(void __iomem *base, unsigned int channel); +}; + +static inline u32 ocores_readl(struct ocores_pwm_device *ddata, + unsigned int channel, + unsigned int offset) +{ + void __iomem *base = ddata->data->get_ch_base ? + ddata->data->get_ch_base(ddata->regs, channel) : ddata->regs; + + return readl(base + offset); +} + +static inline void ocores_writel(struct ocores_pwm_device *ddata, + unsigned int channel, + unsigned int offset, u32 val) +{ + void __iomem *base = ddata->data->get_ch_base ? + ddata->data->get_ch_base(ddata->regs, channel) : ddata->regs; + + writel(val, base + offset); +} + +static inline struct ocores_pwm_device *chip_to_ocores(struct pwm_chip *chip) +{ + return container_of(chip, struct ocores_pwm_device, chip); +} + +static void __iomem *starfive_jh71x0_get_ch_base(void __iomem *base, + unsigned int channel) +{ + unsigned int offset = (channel > 3 ? 1 << 15 : 0) + (channel & 3) * 0x10; + + return base + offset; +} + +static int ocores_pwm_get_state(struct pwm_chip *chip, + struct pwm_device *pwm, + struct pwm_state *state) +{ + struct ocores_pwm_device *ddata = chip_to_ocores(chip); + u32 period_data, duty_data, ctrl_data; + + period_data = ocores_readl(ddata, pwm->hwpwm, 0x8); + duty_data = ocores_readl(ddata, pwm->hwpwm, 0x4); + ctrl_data = ocores_readl(ddata, pwm->hwpwm, 0xC); + + state->period = DIV_ROUND_UP_ULL((u64)period_data * NSEC_PER_SEC, ddata->clk_rate); + state->duty_cycle = DIV_ROUND_UP_ULL((u64)duty_data * NSEC_PER_SEC, ddata->clk_rate); + state->polarity = PWM_POLARITY_INVERSED; + state->enabled = (ctrl_data & REG_OCPWM_EN) ? true : false; + + return 0; +} + +static int ocores_pwm_apply(struct pwm_chip *chip, + struct pwm_device *pwm, + const struct pwm_state *state) +{ + struct ocores_pwm_device *ddata = chip_to_ocores(chip); + u32 ctrl_data = 0; + u64 period_data, duty_data; + + if (state->polarity != PWM_POLARITY_INVERSED) + return -EINVAL; + + ctrl_data = ocores_readl(ddata, pwm->hwpwm, 0xC); + ocores_writel(ddata, pwm->hwpwm, 0xC, 0); + + period_data = DIV_ROUND_DOWN_ULL(state->period * ddata->clk_rate, NSEC_PER_SEC); + if (period_data <= U32_MAX) + ocores_writel(ddata, pwm->hwpwm, 0x8, (u32)period_data); + else + return -EINVAL; + + duty_data = DIV_ROUND_DOWN_ULL(state->duty_cycle * ddata->clk_rate, NSEC_PER_SEC); + if (duty_data <= U32_MAX) + ocores_writel(ddata, pwm->hwpwm, 0x4, (u32)duty_data); + else + return -EINVAL; + + ocores_writel(ddata, pwm->hwpwm, 0xC, 0); + + if (state->enabled) { + ctrl_data = ocores_readl(ddata, pwm->hwpwm, 0xC); + ocores_writel(ddata, pwm->hwpwm, 0xC, ctrl_data | REG_OCPWM_EN | REG_OCPWM_OE); + } + + return 0; +} + +static const struct pwm_ops ocores_pwm_ops = { + .get_state = ocores_pwm_get_state, + .apply = ocores_pwm_apply, +}; + +static const struct ocores_pwm_data jh7100_pwm_data = { + .get_ch_base = starfive_jh71x0_get_ch_base, +}; + +static const struct ocores_pwm_data jh7110_pwm_data = { + .get_ch_base = starfive_jh71x0_get_ch_base, +}; + +static const struct of_device_id ocores_pwm_of_match[] = { + { .compatible = "opencores,pwm-v1" }, + { .compatible = "starfive,jh7100-pwm", .data = &jh7100_pwm_data}, + { .compatible = "starfive,jh7110-pwm", .data = &jh7110_pwm_data}, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, ocores_pwm_of_match); + +static void ocores_reset_control_assert(void *data) +{ + reset_control_assert(data); +} + +static int ocores_pwm_probe(struct platform_device *pdev) +{ + const struct of_device_id *id; + struct device *dev = &pdev->dev; + struct ocores_pwm_device *ddata; + struct pwm_chip *chip; + int ret; + + id = of_match_device(ocores_pwm_of_match, dev); + if (!id) + return -EINVAL; + + ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL); + if (!ddata) + return -ENOMEM; + + ddata->data = id->data; + chip = &ddata->chip; + chip->dev = dev; + chip->ops = &ocores_pwm_ops; + chip->npwm = 8; + + ddata->regs = devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(ddata->regs)) + return dev_err_probe(dev, PTR_ERR(ddata->regs), + "Unable to map IO resources\n"); + + ddata->clk = devm_clk_get_enabled(dev, NULL); + if (IS_ERR(ddata->clk)) + return dev_err_probe(dev, PTR_ERR(ddata->clk), + "Unable to get pwm's clock\n"); + + ddata->rst = devm_reset_control_get_optional_exclusive(dev, NULL); + if (IS_ERR(ddata->rst)) + return dev_err_probe(dev, PTR_ERR(ddata->rst), + "Unable to get pwm's reset\n"); + + reset_control_deassert(ddata->rst); + + ret = devm_add_action_or_reset(dev, ocores_reset_control_assert, ddata->rst); + if (ret) + return ret; + + ddata->clk_rate = clk_get_rate(ddata->clk); + if (ddata->clk_rate <= 0) + return dev_err_probe(dev, ddata->clk_rate, + "Unable to get clock's rate\n"); + + ret = devm_pwmchip_add(dev, chip); + if (ret < 0) + return dev_err_probe(dev, ret, "Could not register PWM chip\n"); + + platform_set_drvdata(pdev, ddata); + + return ret; +} + +static struct platform_driver ocores_pwm_driver = { + .probe = ocores_pwm_probe, + .driver = { + .name = "ocores-pwm", + .of_match_table = ocores_pwm_of_match, + }, +}; +module_platform_driver(ocores_pwm_driver); + +MODULE_AUTHOR("Jieqin Chen"); +MODULE_AUTHOR("Hal Feng <hal.feng@starfivetech.com>"); +MODULE_DESCRIPTION("OpenCores PWM PTC driver"); +MODULE_LICENSE("GPL");