| Message ID | 20240803105702.9621-7-hpausten@protonmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | clk: clocking-wizard: add user clock monitor support | expand |
[AMD Official Use Only - AMD Internal Distribution Only] > -----Original Message----- > From: Harry Austen <hpausten@protonmail.com> > Sent: Saturday, August 3, 2024 4:28 PM > To: Michael Turquette <mturquette@baylibre.com>; Stephen Boyd > <sboyd@kernel.org>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski > <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Simek, Michal > <michal.simek@amd.com>; Greg Kroah-Hartman > <gregkh@linuxfoundation.org> > Cc: Datta, Shubhrajyoti <shubhrajyoti.datta@amd.com>; Dave Ertman > <david.m.ertman@intel.com>; Ira Weiny <ira.weiny@intel.com>; linux- > clk@vger.kernel.org; devicetree@vger.kernel.org; linux- > kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; Harry Austen > <hpausten@protonmail.com> > Subject: [PATCH v2 6/9] clk: clocking-wizard: add user clock monitor support > > Caution: This message originated from an External Source. Use proper > caution when opening attachments, clicking links, or responding. > > > 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 access to this monitor > logic through use of an auxiliary device. > > Use presence of the user monitor interrupt description in devicetree to > indicate whether or not the auxiliary 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> > --- > v1 -> v2: > - Remove direct UIO dependency by utilising auxiliary device > - Move some logic from probe into clk_wzrd_setup_monitor for tidiness > > drivers/clk/xilinx/Kconfig | 1 + > drivers/clk/xilinx/clk-xlnx-clock-wizard.c | 60 ++++++++++++++++++++-- > 2 files changed, 56 insertions(+), 5 deletions(-) > > diff --git a/drivers/clk/xilinx/Kconfig b/drivers/clk/xilinx/Kconfig index > 051756953558b..87f507bd9b6f3 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 > + select AUXILIARY_BUS > 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 1f8023d24029f..557e11017faf9 100644 > --- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c > +++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c > @@ -8,6 +8,7 @@ > * > */ > > +#include <linux/auxiliary_bus.h> > #include <linux/bitfield.h> > #include <linux/platform_device.h> > #include <linux/clk.h> > @@ -129,6 +130,7 @@ enum clk_wzrd_int_clks { > * @axi_clk: Handle to input clock 's_axi_aclk' > * @clks_internal: Internal clocks > * @speed_grade: Speed grade of the device > + * @adev: User clock monitor auxiliary device > * @suspended: Flag indicating power state of the device > */ > struct clk_wzrd { > @@ -139,6 +141,7 @@ struct clk_wzrd { > struct clk_hw *clks_internal[wzrd_clk_int_max]; > unsigned int speed_grade; > bool suspended; > + struct auxiliary_device adev; > struct clk_hw_onecell_data clk_data; }; > > @@ -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,58 @@ 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, > }; The clocking wizard monitor support is a design choice. This will enable it for all the designs. > > +static const struct clk_wzrd_data versal_data = { > + .is_versal = true, > + .supports_monitor = true, > +}; Same here. > + > +static void clk_wzrd_unregister_adev(void *_adev) { > + struct auxiliary_device *adev = _adev; > + > + auxiliary_device_delete(adev); > + auxiliary_device_uninit(adev); > +} > + > +static int clk_wzrd_setup_monitor(struct platform_device *pdev) { > + struct device *dev = &pdev->dev; > + const struct clk_wzrd_data *data = device_get_match_data(dev); > + struct clk_wzrd *clk_wzrd = dev_get_drvdata(dev); > + struct auxiliary_device *adev = &clk_wzrd->adev; > + int ret; > + > + if (!data || !data->supports_monitor) > + return 0; > + > + adev->name = "clk-mon"; > + adev->dev.parent = dev; > + adev->dev.platform_data = (__force void *)clk_wzrd->base; > + > + ret = auxiliary_device_init(adev); > + if (ret) > + return ret; > + > + ret = auxiliary_device_add(adev); > + if (ret) { > + auxiliary_device_uninit(adev); > + return ret; > + } > + > + return devm_add_action_or_reset(dev, clk_wzrd_unregister_adev, > +adev); } > + > 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; > @@ -1170,6 +1216,10 @@ static int clk_wzrd_probe(struct platform_device > *pdev) > return -EINVAL; > } > > + ret = clk_wzrd_setup_monitor(pdev); > + if (ret) > + return dev_err_probe(&pdev->dev, ret, "failed to setup > + monitor\n"); > + > ret = clk_wzrd_register_output_clocks(&pdev->dev, nr_outputs); > if (ret) > return ret; > @@ -1204,7 +1254,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); > -- > 2.46.0 >
On Mon Aug 19, 2024 at 1:39 PM BST, Datta, Shubhrajyoti wrote: > [AMD Official Use Only - AMD Internal Distribution Only] > > > -----Original Message----- ... > > > > -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, > > }; > > The clocking wizard monitor support is a design choice. > This will enable it for all the designs. But the interrupt still has to be described in devicetree for the auxiliary/UIO device to be registered. The interrupt is only used by the core for the clock monitor functionality, so if that functionality is not built into the hardware, then the interrupt description should be omitted. Does that not sound sensible to you? > > > > > +static const struct clk_wzrd_data versal_data = { > > + .is_versal = true, > > + .supports_monitor = true, > > +}; > > > Same here. Same reasoning as above. Thanks for the review! Harry > > > + > > +static void clk_wzrd_unregister_adev(void *_adev) { > > + struct auxiliary_device *adev = _adev; > > + > > + auxiliary_device_delete(adev); > > + auxiliary_device_uninit(adev); > > +} > > + > > +static int clk_wzrd_setup_monitor(struct platform_device *pdev) { > > + struct device *dev = &pdev->dev; > > + const struct clk_wzrd_data *data = device_get_match_data(dev); > > + struct clk_wzrd *clk_wzrd = dev_get_drvdata(dev); > > + struct auxiliary_device *adev = &clk_wzrd->adev; > > + int ret; > > + > > + if (!data || !data->supports_monitor) > > + return 0; > > + > > + adev->name = "clk-mon"; > > + adev->dev.parent = dev; > > + adev->dev.platform_data = (__force void *)clk_wzrd->base; > > + > > + ret = auxiliary_device_init(adev); > > + if (ret) > > + return ret; > > + > > + ret = auxiliary_device_add(adev); > > + if (ret) { > > + auxiliary_device_uninit(adev); > > + return ret; > > + } > > + > > + return devm_add_action_or_reset(dev, clk_wzrd_unregister_adev, > > +adev); } > > + > > 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; > > @@ -1170,6 +1216,10 @@ static int clk_wzrd_probe(struct platform_device > > *pdev) > > return -EINVAL; > > } > > > > + ret = clk_wzrd_setup_monitor(pdev); > > + if (ret) > > + return dev_err_probe(&pdev->dev, ret, "failed to setup > > + monitor\n"); > > + > > ret = clk_wzrd_register_output_clocks(&pdev->dev, nr_outputs); > > if (ret) > > return ret; > > @@ -1204,7 +1254,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); > > -- > > 2.46.0 > >
diff --git a/drivers/clk/xilinx/Kconfig b/drivers/clk/xilinx/Kconfig index 051756953558b..87f507bd9b6f3 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 + select AUXILIARY_BUS 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 1f8023d24029f..557e11017faf9 100644 --- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c +++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c @@ -8,6 +8,7 @@ * */ +#include <linux/auxiliary_bus.h> #include <linux/bitfield.h> #include <linux/platform_device.h> #include <linux/clk.h> @@ -129,6 +130,7 @@ enum clk_wzrd_int_clks { * @axi_clk: Handle to input clock 's_axi_aclk' * @clks_internal: Internal clocks * @speed_grade: Speed grade of the device + * @adev: User clock monitor auxiliary device * @suspended: Flag indicating power state of the device */ struct clk_wzrd { @@ -139,6 +141,7 @@ struct clk_wzrd { struct clk_hw *clks_internal[wzrd_clk_int_max]; unsigned int speed_grade; bool suspended; + struct auxiliary_device adev; struct clk_hw_onecell_data clk_data; }; @@ -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,58 @@ 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 void clk_wzrd_unregister_adev(void *_adev) +{ + struct auxiliary_device *adev = _adev; + + auxiliary_device_delete(adev); + auxiliary_device_uninit(adev); +} + +static int clk_wzrd_setup_monitor(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + const struct clk_wzrd_data *data = device_get_match_data(dev); + struct clk_wzrd *clk_wzrd = dev_get_drvdata(dev); + struct auxiliary_device *adev = &clk_wzrd->adev; + int ret; + + if (!data || !data->supports_monitor) + return 0; + + adev->name = "clk-mon"; + adev->dev.parent = dev; + adev->dev.platform_data = (__force void *)clk_wzrd->base; + + ret = auxiliary_device_init(adev); + if (ret) + return ret; + + ret = auxiliary_device_add(adev); + if (ret) { + auxiliary_device_uninit(adev); + return ret; + } + + return devm_add_action_or_reset(dev, clk_wzrd_unregister_adev, adev); +} + 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; @@ -1170,6 +1216,10 @@ static int clk_wzrd_probe(struct platform_device *pdev) return -EINVAL; } + ret = clk_wzrd_setup_monitor(pdev); + if (ret) + return dev_err_probe(&pdev->dev, ret, "failed to setup monitor\n"); + ret = clk_wzrd_register_output_clocks(&pdev->dev, nr_outputs); if (ret) return ret; @@ -1204,7 +1254,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 access to this monitor logic through use of an auxiliary device. Use presence of the user monitor interrupt description in devicetree to indicate whether or not the auxiliary 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> --- v1 -> v2: - Remove direct UIO dependency by utilising auxiliary device - Move some logic from probe into clk_wzrd_setup_monitor for tidiness drivers/clk/xilinx/Kconfig | 1 + drivers/clk/xilinx/clk-xlnx-clock-wizard.c | 60 ++++++++++++++++++++-- 2 files changed, 56 insertions(+), 5 deletions(-)