| Message ID | 20240720120048.36758-6-hpausten@protonmail.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | clk: clocking-wizard: add user clock monitor support | expand |
Quoting Harry Austen (2024-07-20 05:01:53) > Xilinx clocking wizard IP core supports monitoring of up to four > optional user clock inputs, with a corresponding interrupt for > notification in change of clock state (stop, underrun, overrun or > glitch). Give userspace access to this monitor logic through use of the > UIO framework. > > Use presence of the user monitor interrupt description in devicetree to > indicate whether or not the UIO device should be registered. Also, this > functionality is only supported from v6.0 onwards, so add indication of > support to the device match data, in order to be tied to the utilised > compatible string. > > Signed-off-by: Harry Austen <hpausten@protonmail.com> > --- > diff --git a/drivers/clk/xilinx/Kconfig b/drivers/clk/xilinx/Kconfig > index 051756953558b..907a435694687 100644 > --- a/drivers/clk/xilinx/Kconfig > +++ b/drivers/clk/xilinx/Kconfig > @@ -21,6 +21,7 @@ config COMMON_CLK_XLNX_CLKWZRD > tristate "Xilinx Clocking Wizard" > depends on OF > depends on HAS_IOMEM > + depends on UIO If I have a pre-v6.0 device I probably don't want UIO though. Perhaps you should use the auxiliary bus framework to register a device that is otherwise unused and then have the uio driver live in drivers/uio and match that device made here. I think you can have 'imply UIO' if you like to put a weak Kconfig dependency. > help > Support for the Xilinx Clocking Wizard IP core clock generator. > Adds support for clocking wizard and compatible. > diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c > index 7b262d73310fe..2d419e8ad4419 100644 > --- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c > +++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c > @@ -1165,6 +1209,17 @@ static int clk_wzrd_probe(struct platform_device *pdev) > return -EINVAL; > } > > + data = device_get_match_data(&pdev->dev); > + if (data && data->supports_monitor) { > + irq = platform_get_irq(pdev, 0); > + if (irq > 0) { > + ret = clk_wzrd_setup_monitor(&pdev->dev, irq, > + platform_get_resource(pdev, IORESOURCE_IO, 0)); Any reason this can't be ret = clk_wzrd_setup_monitor(pdev); if (ret) return ret; and then all the surrounding code be moved into the function, including the dev_err_probe()? > + if (ret) > + return dev_err_probe(&pdev->dev, ret, "failed to setup monitor\n"); > + } > + } > + > ret = of_property_read_u32(np, "xlnx,nr-outputs", &nr_outputs);
On Wed Jul 24, 2024 at 12:29 AM BST, Stephen Boyd wrote: > Quoting Harry Austen (2024-07-20 05:01:53) > > Xilinx clocking wizard IP core supports monitoring of up to four > > optional user clock inputs, with a corresponding interrupt for > > notification in change of clock state (stop, underrun, overrun or > > glitch). Give userspace access to this monitor logic through use of the > > UIO framework. > > > > Use presence of the user monitor interrupt description in devicetree to > > indicate whether or not the UIO device should be registered. Also, this > > functionality is only supported from v6.0 onwards, so add indication of > > support to the device match data, in order to be tied to the utilised > > compatible string. > > > > Signed-off-by: Harry Austen <hpausten@protonmail.com> > > --- > > diff --git a/drivers/clk/xilinx/Kconfig b/drivers/clk/xilinx/Kconfig > > index 051756953558b..907a435694687 100644 > > --- a/drivers/clk/xilinx/Kconfig > > +++ b/drivers/clk/xilinx/Kconfig > > @@ -21,6 +21,7 @@ config COMMON_CLK_XLNX_CLKWZRD > > tristate "Xilinx Clocking Wizard" > > depends on OF > > depends on HAS_IOMEM > > + depends on UIO > > If I have a pre-v6.0 device I probably don't want UIO though. Perhaps > you should use the auxiliary bus framework to register a device that is > otherwise unused and then have the uio driver live in drivers/uio and > match that device made here. I think you can have 'imply UIO' if you > like to put a weak Kconfig dependency. Yeah I wasn't particularly happy about the UIO usage here. It seems like a nice way to leave control up to the user in userspace though, rather than adding a bunch of device attributes to read status registers, configure interrupts etc. but I agree the dependency isn't good here. Will look into your suggestion for v2. > > > help > > Support for the Xilinx Clocking Wizard IP core clock generator. > > Adds support for clocking wizard and compatible. > > diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c > > index 7b262d73310fe..2d419e8ad4419 100644 > > --- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c > > +++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c > > @@ -1165,6 +1209,17 @@ static int clk_wzrd_probe(struct platform_device *pdev) > > return -EINVAL; > > } > > > > + data = device_get_match_data(&pdev->dev); > > + if (data && data->supports_monitor) { > > + irq = platform_get_irq(pdev, 0); > > + if (irq > 0) { > > + ret = clk_wzrd_setup_monitor(&pdev->dev, irq, > > + platform_get_resource(pdev, IORESOURCE_IO, 0)); > > Any reason this can't be > > ret = clk_wzrd_setup_monitor(pdev); > if (ret) > return ret; > > and then all the surrounding code be moved into the function, including > the dev_err_probe()? That makes a lot more sense. Don't know what I was doing. Thanks! > > > + if (ret) > > + return dev_err_probe(&pdev->dev, ret, "failed to setup monitor\n"); > > + } > > + } > > + > > ret = of_property_read_u32(np, "xlnx,nr-outputs", &nr_outputs);
diff --git a/drivers/clk/xilinx/Kconfig b/drivers/clk/xilinx/Kconfig index 051756953558b..907a435694687 100644 --- a/drivers/clk/xilinx/Kconfig +++ b/drivers/clk/xilinx/Kconfig @@ -21,6 +21,7 @@ config COMMON_CLK_XLNX_CLKWZRD tristate "Xilinx Clocking Wizard" depends on OF depends on HAS_IOMEM + depends on UIO help Support for the Xilinx Clocking Wizard IP core clock generator. Adds support for clocking wizard and compatible. diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c index 7b262d73310fe..2d419e8ad4419 100644 --- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c +++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c @@ -20,10 +20,13 @@ #include <linux/overflow.h> #include <linux/err.h> #include <linux/iopoll.h> +#include <linux/uio_driver.h> #define WZRD_NUM_OUTPUTS 7 #define WZRD_ACLK_MAX_FREQ 250000000UL +#define WZRD_INTR_ENABLE 0x10 + #define WZRD_CLK_CFG_REG(v, n) (0x200 + 0x130 * (v) + 4 * (n)) #define WZRD_CLKOUT0_FRAC_EN BIT(18) @@ -171,8 +174,9 @@ struct clk_wzrd_divider { spinlock_t *lock; /* divider lock */ }; -struct versal_clk_data { +struct clk_wzrd_data { bool is_versal; + bool supports_monitor; }; #define to_clk_wzrd(_nb) container_of(_nb, struct clk_wzrd, nb) @@ -958,16 +962,55 @@ static int __maybe_unused clk_wzrd_resume(struct device *dev) static SIMPLE_DEV_PM_OPS(clk_wzrd_dev_pm_ops, clk_wzrd_suspend, clk_wzrd_resume); -static const struct versal_clk_data versal_data = { - .is_versal = true, +static const struct clk_wzrd_data version_6_0_data = { + .is_versal = false, + .supports_monitor = true, +}; + +static const struct clk_wzrd_data versal_data = { + .is_versal = true, + .supports_monitor = true, }; +static int clk_wzrd_irqcontrol(struct uio_info *info, s32 irq_on) +{ + if (irq_on) + iowrite32(GENMASK(15, 0), info->mem[0].internal_addr + WZRD_INTR_ENABLE); + else + iowrite32(0, info->mem[0].internal_addr + WZRD_INTR_ENABLE); + + return 0; +} + +static int clk_wzrd_setup_monitor(struct device *dev, int irq, const struct resource *res) +{ + struct clk_wzrd *clk_wzrd = dev_get_drvdata(dev); + struct uio_info *info; + + info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL); + if (!info) + return -ENOMEM; + + info->name = KBUILD_MODNAME; + info->version = "0.0.1"; + + info->mem[0].name = "user monitor"; + info->mem[0].memtype = UIO_MEM_PHYS; + info->mem[0].addr = res->start; + info->mem[0].size = WZRD_INTR_ENABLE; + info->mem[0].internal_addr = clk_wzrd->base; + + info->irq = irq; + info->irqcontrol = clk_wzrd_irqcontrol; + return devm_uio_register_device(dev, info); +} + static int clk_wzrd_register_output_clocks(struct device *dev, int nr_outputs) { const char *clkout_name, *clk_name, *clk_mul_name; struct clk_wzrd *clk_wzrd = dev_get_drvdata(dev); u32 regl, regh, edge, regld, reghd, edged, div; - const struct versal_clk_data *data; + const struct clk_wzrd_data *data; unsigned long flags = 0; bool is_versal = false; void __iomem *ctrl_reg; @@ -1127,10 +1170,11 @@ static int clk_wzrd_register_output_clocks(struct device *dev, int nr_outputs) static int clk_wzrd_probe(struct platform_device *pdev) { struct device_node *np = pdev->dev.of_node; + const struct clk_wzrd_data *data; struct clk_wzrd *clk_wzrd; unsigned long rate; int nr_outputs; - int ret; + int irq, ret; clk_wzrd = devm_kzalloc(&pdev->dev, sizeof(*clk_wzrd), GFP_KERNEL); if (!clk_wzrd) @@ -1165,6 +1209,17 @@ static int clk_wzrd_probe(struct platform_device *pdev) return -EINVAL; } + data = device_get_match_data(&pdev->dev); + if (data && data->supports_monitor) { + irq = platform_get_irq(pdev, 0); + if (irq > 0) { + ret = clk_wzrd_setup_monitor(&pdev->dev, irq, + platform_get_resource(pdev, IORESOURCE_IO, 0)); + if (ret) + return dev_err_probe(&pdev->dev, ret, "failed to setup monitor\n"); + } + } + ret = of_property_read_u32(np, "xlnx,nr-outputs", &nr_outputs); if (ret || nr_outputs > WZRD_NUM_OUTPUTS) return -EINVAL; @@ -1208,7 +1263,7 @@ static const struct of_device_id clk_wzrd_ids[] = { { .compatible = "xlnx,versal-clk-wizard", .data = &versal_data }, { .compatible = "xlnx,clocking-wizard" }, { .compatible = "xlnx,clocking-wizard-v5.2" }, - { .compatible = "xlnx,clocking-wizard-v6.0" }, + { .compatible = "xlnx,clocking-wizard-v6.0", .data = &version_6_0_data }, { }, }; MODULE_DEVICE_TABLE(of, clk_wzrd_ids);
Xilinx clocking wizard IP core supports monitoring of up to four optional user clock inputs, with a corresponding interrupt for notification in change of clock state (stop, underrun, overrun or glitch). Give userspace access to this monitor logic through use of the UIO framework. Use presence of the user monitor interrupt description in devicetree to indicate whether or not the UIO device should be registered. Also, this functionality is only supported from v6.0 onwards, so add indication of support to the device match data, in order to be tied to the utilised compatible string. Signed-off-by: Harry Austen <hpausten@protonmail.com> --- drivers/clk/xilinx/Kconfig | 1 + drivers/clk/xilinx/clk-xlnx-clock-wizard.c | 67 ++++++++++++++++++++-- 2 files changed, 62 insertions(+), 6 deletions(-)