| Message ID | 1613623791-4598-1-git-send-email-shubhrajyoti.datta@xilinx.com |
|---|---|
| Headers | show |
| Series | clk: clk-wizard: clock-wizard: Driver updates | expand |
Hi Shubhrajyoti, Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> wrote on Thu, 18 Feb 2021 10:19:44 +0530: > In the thread [1] Greg suggested that we move the driver > to the clk from the staging. > Add patches to address the concerns regarding the fractional and > set rate support in the TODO. > > The patch set does the following > - Trivial fixes for kernel doc. > - Move the driver to the clk folder > - Add capability to set rate. > - Add fractional support. > - Add support for configurable outputs. > - Make the output names unique so that multiple instances > do not crib. I think we prefer to move "clean" drivers out of the staging tree rather than "to be fixed" code. So I would invert the order of the patches in this series to make more sense: * 3/7-7/7 (various fixes/improvements) * 1/7 (bindings) * 2/7 (move to clk) > Shubhrajyoti Datta (7): > dt-bindings: add documentation of xilinx clocking wizard > clk: clock-wizard: Add the clockwizard to clk directory > clk: clock-wizard: Fix kernel-doc warning > clk: clock-wizard: Add support for dynamic reconfiguration > clk: clock-wizard: Add support for fractional support > clk: clock-wizard: Remove the hardcoding of the clock outputs > clk: clock-wizard: Update the fixed factor divisors > > .../bindings/clock/xlnx,clocking-wizard.yaml | 65 ++ > drivers/clk/Kconfig | 9 + > drivers/clk/Makefile | 1 + > drivers/clk/clk-xlnx-clock-wizard.c | 689 +++++++++++++++++++++ > drivers/staging/Kconfig | 2 - > drivers/staging/Makefile | 1 - > drivers/staging/clocking-wizard/Kconfig | 10 - > drivers/staging/clocking-wizard/Makefile | 2 - > drivers/staging/clocking-wizard/TODO | 12 - > .../clocking-wizard/clk-xlnx-clock-wizard.c | 333 ---------- > drivers/staging/clocking-wizard/dt-binding.txt | 30 - > 11 files changed, 764 insertions(+), 390 deletions(-) > create mode 100644 Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml > create mode 100644 drivers/clk/clk-xlnx-clock-wizard.c > delete mode 100644 drivers/staging/clocking-wizard/Kconfig > delete mode 100644 drivers/staging/clocking-wizard/Makefile > delete mode 100644 drivers/staging/clocking-wizard/TODO > delete mode 100644 drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c > delete mode 100644 drivers/staging/clocking-wizard/dt-binding.txt > Thanks, Miquèl
Hi Shubhrajyoti, Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> wrote on Thu, 18 Feb 2021 10:19:45 +0530: > Add the devicetree binding for the xilinx clocking wizard. > > Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> > --- > v6: > Fix a yaml warning > v7: > Add vendor prefix speed-grade > v8: > Fix the warnings > v9: > Fix the warnings > > .../bindings/clock/xlnx,clocking-wizard.yaml | 65 ++++++++++++++++++++++ > 1 file changed, 65 insertions(+) > create mode 100644 Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml > > diff --git a/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml > new file mode 100644 > index 0000000..d209140 > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml > @@ -0,0 +1,65 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: "http://devicetree.org/schemas/clock/xlnx,clocking-wizard.yaml#" > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > + > +title: Xilinx clocking wizard > + > +maintainers: > + - Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> > + > +description: > + The clocking wizard is a soft ip clocking block of Xilinx versal. It > + reads required input clock frequencies from the devicetree and acts as clock > + clock output. > + > +properties: > + compatible: > + const: xlnx,clocking-wizard > + > + reg: > + maxItems: 1 > + > + "#clock-cells": > + const: 1 > + > + clocks: > + items: > + - description: clock input > + - description: axi clock > + > + clock-names: > + items: > + - const: clk_in1 > + - const: s_axi_aclk > + > + clock-output-names: true > + > + xlnx,speed-grade: > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [1, 2, 3] > + description: > + Speed grade of the device. A bit of explanation of what this describes would be welcome. Don't forget that binding are not tied to any driver implementation, these are supposed to be hardware description properties. > + > +required: > + - compatible > + - reg > + - "#clock-cells" > + - clocks > + - clock-names > + - xlnx,speed-grade > + > +additionalProperties: false > + > +examples: > + - | > + clock-controller { > + compatible = "xlnx,clocking-wizard"; > + reg = <0xb0000000 0x10000>; > + #clock-cells = <1>; > + xlnx,speed-grade = <1>; > + clock-names = "clk_in1", "s_axi_aclk"; > + clocks = <&clkc 15>, <&clkc 15>; > + }; > +... Thanks, Miquèl
Hi Shubhrajyoti, Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> wrote on Thu, 18 Feb 2021 10:19:50 +0530: > The number of output clocks are configurable in the hardware. > Currently the driver registers the maximum number of outputs. > Fix the same by registering only the outputs that are there. > > Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> > --- > v4: > Assign output in this patch > > drivers/clk/clk-xlnx-clock-wizard.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/clk/clk-xlnx-clock-wizard.c b/drivers/clk/clk-xlnx-clock-wizard.c > index ed3b0ef..d403a74 100644 > --- a/drivers/clk/clk-xlnx-clock-wizard.c > +++ b/drivers/clk/clk-xlnx-clock-wizard.c > @@ -473,6 +473,7 @@ static int clk_wzrd_probe(struct platform_device *pdev) > unsigned long rate; > const char *clk_name; > struct clk_wzrd *clk_wzrd; > + int outputs; > struct device_node *np = pdev->dev.of_node; > > clk_wzrd = devm_kzalloc(&pdev->dev, sizeof(*clk_wzrd), GFP_KERNEL); > @@ -541,6 +542,7 @@ static int clk_wzrd_probe(struct platform_device *pdev) > goto err_disable_clk; > } > > + outputs = of_property_count_strings(np, "clock-output-names"); A check on outputs validity is probably welcome. Also I usually prefer noutputs or nb_outputs for such variable name, which implies a number rather than an array, but this is personal taste. > /* register div */ > reg = (readl(clk_wzrd->base + WZRD_CLK_CFG_REG(0)) & > WZRD_DIVCLK_DIVIDE_MASK) >> WZRD_DIVCLK_DIVIDE_SHIFT; > @@ -562,7 +564,7 @@ static int clk_wzrd_probe(struct platform_device *pdev) > } > > /* register div per output */ > - for (i = WZRD_NUM_OUTPUTS - 1; i >= 0 ; i--) { > + for (i = outputs - 1; i >= 0 ; i--) { > const char *clkout_name; > > if (of_property_read_string_index(np, "clock-output-names", i, > @@ -593,7 +595,7 @@ static int clk_wzrd_probe(struct platform_device *pdev) > if (IS_ERR(clk_wzrd->clkout[i])) { > int j; > > - for (j = i + 1; j < WZRD_NUM_OUTPUTS; j++) > + for (j = i + 1; j < outputs; j++) > clk_unregister(clk_wzrd->clkout[j]); > dev_err(&pdev->dev, > "unable to register divider clock\n"); Thanks, Miquèl
On Thu, 18 Feb 2021 10:19:45 +0530, Shubhrajyoti Datta wrote: > Add the devicetree binding for the xilinx clocking wizard. > > Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> > --- > v6: > Fix a yaml warning > v7: > Add vendor prefix speed-grade > v8: > Fix the warnings > v9: > Fix the warnings > > .../bindings/clock/xlnx,clocking-wizard.yaml | 65 ++++++++++++++++++++++ > 1 file changed, 65 insertions(+) > create mode 100644 Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.example.dts:19.26-26.11: Warning (unit_address_vs_reg): /example-0/clock-controller: node has a reg or ranges property, but no unit name See https://patchwork.ozlabs.org/patch/1441521 This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit.
Quoting Miquel Raynal (2021-02-18 00:28:04) > Hi Shubhrajyoti, > > Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> wrote on Thu, 18 Feb > 2021 10:19:45 +0530: > > > Add the devicetree binding for the xilinx clocking wizard. > > > > Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> > > --- > > v6: > > Fix a yaml warning > > v7: > > Add vendor prefix speed-grade > > v8: > > Fix the warnings > > v9: > > Fix the warnings > > > > .../bindings/clock/xlnx,clocking-wizard.yaml | 65 ++++++++++++++++++++++ > > 1 file changed, 65 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml > > > > diff --git a/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml > > new file mode 100644 > > index 0000000..d209140 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml > > @@ -0,0 +1,65 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: "http://devicetree.org/schemas/clock/xlnx,clocking-wizard.yaml#" > > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > > + > > +title: Xilinx clocking wizard > > + > > +maintainers: > > + - Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> > > + > > +description: > > + The clocking wizard is a soft ip clocking block of Xilinx versal. It > > + reads required input clock frequencies from the devicetree and acts as clock > > + clock output. > > + > > +properties: > > + compatible: > > + const: xlnx,clocking-wizard > > + > > + reg: > > + maxItems: 1 > > + > > + "#clock-cells": > > + const: 1 > > + > > + clocks: > > + items: > > + - description: clock input > > + - description: axi clock > > + > > + clock-names: > > + items: > > + - const: clk_in1 > > + - const: s_axi_aclk > > + > > + clock-output-names: true > > + > > + xlnx,speed-grade: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + enum: [1, 2, 3] > > + description: > > + Speed grade of the device. > > A bit of explanation of what this describes would be welcome. > > Don't forget that binding are not tied to any driver implementation, > these are supposed to be hardware description properties. Would opp tables work for this?
Quoting Miquel Raynal (2021-02-18 00:37:15) > Hi Shubhrajyoti, > > Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> wrote on Thu, 18 Feb > 2021 10:19:50 +0530: > > > The number of output clocks are configurable in the hardware. > > Currently the driver registers the maximum number of outputs. > > Fix the same by registering only the outputs that are there. > > > > Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> > > --- > > v4: > > Assign output in this patch > > > > drivers/clk/clk-xlnx-clock-wizard.c | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/clk/clk-xlnx-clock-wizard.c b/drivers/clk/clk-xlnx-clock-wizard.c > > index ed3b0ef..d403a74 100644 > > --- a/drivers/clk/clk-xlnx-clock-wizard.c > > +++ b/drivers/clk/clk-xlnx-clock-wizard.c > > @@ -473,6 +473,7 @@ static int clk_wzrd_probe(struct platform_device *pdev) > > unsigned long rate; > > const char *clk_name; > > struct clk_wzrd *clk_wzrd; > > + int outputs; > > struct device_node *np = pdev->dev.of_node; > > > > clk_wzrd = devm_kzalloc(&pdev->dev, sizeof(*clk_wzrd), GFP_KERNEL); > > @@ -541,6 +542,7 @@ static int clk_wzrd_probe(struct platform_device *pdev) > > goto err_disable_clk; > > } > > > > + outputs = of_property_count_strings(np, "clock-output-names"); > > A check on outputs validity is probably welcome. > > Also I usually prefer noutputs or nb_outputs for such variable name, > which implies a number rather than an array, but this is personal taste. Ideally we get rid of clock-output-names and generate them at runtime instead based on some combination of device name and something else.
Hi Stephen, On Fri, Feb 19, 2021 at 6:55 AM Stephen Boyd <sboyd@kernel.org> wrote: > > Quoting Miquel Raynal (2021-02-18 00:37:15) > > Hi Shubhrajyoti, > > > > Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> wrote on Thu, 18 Feb > > 2021 10:19:50 +0530: > > > > > The number of output clocks are configurable in the hardware. > > > Currently the driver registers the maximum number of outputs. > > > Fix the same by registering only the outputs that are there. > > > > > > Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> > > > --- > > > v4: > > > Assign output in this patch > > > > > > drivers/clk/clk-xlnx-clock-wizard.c | 6 ++++-- > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/clk/clk-xlnx-clock-wizard.c b/drivers/clk/clk-xlnx-clock-wizard.c > > > index ed3b0ef..d403a74 100644 > > > --- a/drivers/clk/clk-xlnx-clock-wizard.c > > > +++ b/drivers/clk/clk-xlnx-clock-wizard.c > > > @@ -473,6 +473,7 @@ static int clk_wzrd_probe(struct platform_device *pdev) > > > unsigned long rate; > > > const char *clk_name; > > > struct clk_wzrd *clk_wzrd; > > > + int outputs; > > > struct device_node *np = pdev->dev.of_node; > > > > > > clk_wzrd = devm_kzalloc(&pdev->dev, sizeof(*clk_wzrd), GFP_KERNEL); > > > @@ -541,6 +542,7 @@ static int clk_wzrd_probe(struct platform_device *pdev) > > > goto err_disable_clk; > > > } > > > > > > + outputs = of_property_count_strings(np, "clock-output-names"); > > > > A check on outputs validity is probably welcome. > > > > Also I usually prefer noutputs or nb_outputs for such variable name, > > which implies a number rather than an array, but this is personal taste. > > Ideally we get rid of clock-output-names and generate them at runtime > instead based on some combination of device name and something else. Makes sense. However it may break the current binding. Do you think that shoud be okay?
Quoting Shubhrajyoti Datta (2021-02-21 22:47:26) > Hi Stephen, > > On Fri, Feb 19, 2021 at 6:55 AM Stephen Boyd <sboyd@kernel.org> wrote: > > > > Quoting Miquel Raynal (2021-02-18 00:37:15) > > > Hi Shubhrajyoti, > > > > > > Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> wrote on Thu, 18 Feb > > > 2021 10:19:50 +0530: > > > > > > > The number of output clocks are configurable in the hardware. > > > > Currently the driver registers the maximum number of outputs. > > > > Fix the same by registering only the outputs that are there. > > > > > > > > Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> > > > > --- > > > > v4: > > > > Assign output in this patch > > > > > > > > drivers/clk/clk-xlnx-clock-wizard.c | 6 ++++-- > > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/clk/clk-xlnx-clock-wizard.c b/drivers/clk/clk-xlnx-clock-wizard.c > > > > index ed3b0ef..d403a74 100644 > > > > --- a/drivers/clk/clk-xlnx-clock-wizard.c > > > > +++ b/drivers/clk/clk-xlnx-clock-wizard.c > > > > @@ -473,6 +473,7 @@ static int clk_wzrd_probe(struct platform_device *pdev) > > > > unsigned long rate; > > > > const char *clk_name; > > > > struct clk_wzrd *clk_wzrd; > > > > + int outputs; > > > > struct device_node *np = pdev->dev.of_node; > > > > > > > > clk_wzrd = devm_kzalloc(&pdev->dev, sizeof(*clk_wzrd), GFP_KERNEL); > > > > @@ -541,6 +542,7 @@ static int clk_wzrd_probe(struct platform_device *pdev) > > > > goto err_disable_clk; > > > > } > > > > > > > > + outputs = of_property_count_strings(np, "clock-output-names"); > > > > > > A check on outputs validity is probably welcome. > > > > > > Also I usually prefer noutputs or nb_outputs for such variable name, > > > which implies a number rather than an array, but this is personal taste. > > > > Ideally we get rid of clock-output-names and generate them at runtime > > instead based on some combination of device name and something else. > > Makes sense. However it may break the current binding. > Do you think that shoud be okay? I think it is OK given that the current binding is for the staging tree. The assumption is those bindings aren't stable.
On Tue, Feb 23, 2021 at 6:32 AM Stephen Boyd <sboyd@kernel.org> wrote: > > Quoting Shubhrajyoti Datta (2021-02-21 22:47:26) > > Hi Stephen, > > > > On Fri, Feb 19, 2021 at 6:55 AM Stephen Boyd <sboyd@kernel.org> wrote: > > > > > > Quoting Miquel Raynal (2021-02-18 00:37:15) > > > > Hi Shubhrajyoti, > > > > > > > > Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> wrote on Thu, 18 Feb > > > > 2021 10:19:50 +0530: > > > > > > > > > The number of output clocks are configurable in the hardware. > > > > > Currently the driver registers the maximum number of outputs. > > > > > Fix the same by registering only the outputs that are there. > > > > > > > > > > Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> > > > > > --- > > > > > v4: > > > > > Assign output in this patch > > > > > > > > > > drivers/clk/clk-xlnx-clock-wizard.c | 6 ++++-- > > > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/drivers/clk/clk-xlnx-clock-wizard.c b/drivers/clk/clk-xlnx-clock-wizard.c > > > > > index ed3b0ef..d403a74 100644 > > > > > --- a/drivers/clk/clk-xlnx-clock-wizard.c > > > > > +++ b/drivers/clk/clk-xlnx-clock-wizard.c > > > > > @@ -473,6 +473,7 @@ static int clk_wzrd_probe(struct platform_device *pdev) > > > > > unsigned long rate; > > > > > const char *clk_name; > > > > > struct clk_wzrd *clk_wzrd; > > > > > + int outputs; > > > > > struct device_node *np = pdev->dev.of_node; > > > > > > > > > > clk_wzrd = devm_kzalloc(&pdev->dev, sizeof(*clk_wzrd), GFP_KERNEL); > > > > > @@ -541,6 +542,7 @@ static int clk_wzrd_probe(struct platform_device *pdev) > > > > > goto err_disable_clk; > > > > > } > > > > > > > > > > + outputs = of_property_count_strings(np, "clock-output-names"); > > > > > > > > A check on outputs validity is probably welcome. > > > > > > > > Also I usually prefer noutputs or nb_outputs for such variable name, > > > > which implies a number rather than an array, but this is personal taste. > > > > > > Ideally we get rid of clock-output-names and generate them at runtime > > > instead based on some combination of device name and something else. > > > > Makes sense. However it may break the current binding. > > Do you think that shoud be okay? > > I think it is OK given that the current binding is for the staging tree. > The assumption is those bindings aren't stable. Updated it in next version.
On Fri, Feb 19, 2021 at 6:54 AM Stephen Boyd <sboyd@kernel.org> wrote: > > Quoting Miquel Raynal (2021-02-18 00:28:04) > > Hi Shubhrajyoti, > > > > Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> wrote on Thu, 18 Feb > > 2021 10:19:45 +0530: > > > > > Add the devicetree binding for the xilinx clocking wizard. > > > > > > Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> > > > --- > > > v6: > > > Fix a yaml warning > > > v7: > > > Add vendor prefix speed-grade > > > v8: > > > Fix the warnings > > > v9: > > > Fix the warnings > > > > > > .../bindings/clock/xlnx,clocking-wizard.yaml | 65 ++++++++++++++++++++++ > > > 1 file changed, 65 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml > > > > > > diff --git a/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml > > > new file mode 100644 > > > index 0000000..d209140 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml > > > @@ -0,0 +1,65 @@ > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > +$id: "http://devicetree.org/schemas/clock/xlnx,clocking-wizard.yaml#" > > > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > > > + > > > +title: Xilinx clocking wizard > > > + > > > +maintainers: > > > + - Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> > > > + > > > +description: > > > + The clocking wizard is a soft ip clocking block of Xilinx versal. It > > > + reads required input clock frequencies from the devicetree and acts as clock > > > + clock output. > > > + > > > +properties: > > > + compatible: > > > + const: xlnx,clocking-wizard > > > + > > > + reg: > > > + maxItems: 1 > > > + > > > + "#clock-cells": > > > + const: 1 > > > + > > > + clocks: > > > + items: > > > + - description: clock input > > > + - description: axi clock > > > + > > > + clock-names: > > > + items: > > > + - const: clk_in1 > > > + - const: s_axi_aclk > > > + > > > + clock-output-names: true > > > + > > > + xlnx,speed-grade: > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > + enum: [1, 2, 3] > > > + description: > > > + Speed grade of the device. > > > > A bit of explanation of what this describes would be welcome. > > > > Don't forget that binding are not tied to any driver implementation, > > these are supposed to be hardware description properties. > > Would opp tables work for this? This is the parameter is for speed of the fabric.
On Thu, Feb 18, 2021 at 1:59 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > Hi Shubhrajyoti, > > Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> wrote on Thu, 18 Feb > 2021 10:19:47 +0530: > > > Update description for the clocking wizard structure > > "Fix the clocking wizard main structure kernel documentation." ? will update in next version. > > > > > Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> > > --- > > drivers/clk/clk-xlnx-clock-wizard.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/clk/clk-xlnx-clock-wizard.c b/drivers/clk/clk-xlnx-clock-wizard.c > > index 1bab68e..fb2d555 100644 > > --- a/drivers/clk/clk-xlnx-clock-wizard.c > > +++ b/drivers/clk/clk-xlnx-clock-wizard.c > > @@ -40,7 +40,8 @@ enum clk_wzrd_int_clks { > > }; > > > > /** > > - * struct clk_wzrd: > > + * struct clk_wzrd - Clock wizard private data structure > > + * > > * @clk_data: Clock data > > * @nb: Notifier block > > * @base: Memory base > > > Thanks, > Miquèl
On Thu, Feb 18, 2021 at 1:54 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > Hi Shubhrajyoti, > > Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> wrote on Thu, 18 Feb > 2021 10:19:44 +0530: > > > In the thread [1] Greg suggested that we move the driver > > to the clk from the staging. > > Add patches to address the concerns regarding the fractional and > > set rate support in the TODO. > > > > The patch set does the following > > - Trivial fixes for kernel doc. > > - Move the driver to the clk folder > > - Add capability to set rate. > > - Add fractional support. > > - Add support for configurable outputs. > > - Make the output names unique so that multiple instances > > do not crib. > > I think we prefer to move "clean" drivers out of the staging tree > rather than "to be fixed" code. So I would invert the order of the > patches in this series to make more sense: > * 3/7-7/7 (various fixes/improvements) > * 1/7 (bindings) > * 2/7 (move to clk) > Will update in next version > > Shubhrajyoti Datta (7): > > dt-bindings: add documentation of xilinx clocking wizard > > clk: clock-wizard: Add the clockwizard to clk directory > > clk: clock-wizard: Fix kernel-doc warning > > clk: clock-wizard: Add support for dynamic reconfiguration > > clk: clock-wizard: Add support for fractional support > > clk: clock-wizard: Remove the hardcoding of the clock outputs > > clk: clock-wizard: Update the fixed factor divisors > > > > .../bindings/clock/xlnx,clocking-wizard.yaml | 65 ++ > > drivers/clk/Kconfig | 9 + > > drivers/clk/Makefile | 1 + > > drivers/clk/clk-xlnx-clock-wizard.c | 689 +++++++++++++++++++++ > > drivers/staging/Kconfig | 2 - > > drivers/staging/Makefile | 1 - > > drivers/staging/clocking-wizard/Kconfig | 10 - > > drivers/staging/clocking-wizard/Makefile | 2 - > > drivers/staging/clocking-wizard/TODO | 12 - > > .../clocking-wizard/clk-xlnx-clock-wizard.c | 333 ---------- > > drivers/staging/clocking-wizard/dt-binding.txt | 30 - > > 11 files changed, 764 insertions(+), 390 deletions(-) > > create mode 100644 Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml > > create mode 100644 drivers/clk/clk-xlnx-clock-wizard.c > > delete mode 100644 drivers/staging/clocking-wizard/Kconfig > > delete mode 100644 drivers/staging/clocking-wizard/Makefile > > delete mode 100644 drivers/staging/clocking-wizard/TODO > > delete mode 100644 drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c > > delete mode 100644 drivers/staging/clocking-wizard/dt-binding.txt > > > > Thanks, > Miquèl
Quoting Shubhrajyoti Datta (2021-02-24 06:10:08) > On Fri, Feb 19, 2021 at 6:54 AM Stephen Boyd <sboyd@kernel.org> wrote: > > > > > > + > > > > + xlnx,speed-grade: > > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > > + enum: [1, 2, 3] > > > > + description: > > > > + Speed grade of the device. > > > > > > A bit of explanation of what this describes would be welcome. > > > > > > Don't forget that binding are not tied to any driver implementation, > > > these are supposed to be hardware description properties. > > > > Would opp tables work for this? > This is the parameter is for speed of the fabric. Ok. Yes or no? Is it configuring the speed of the fabric? Sounds like assigned-clock-rates or assigned-interconnect-bandwidth or something like that.
Hi Stephen, On Wed, Mar 3, 2021 at 4:37 AM Stephen Boyd <sboyd@kernel.org> wrote: > > Quoting Shubhrajyoti Datta (2021-02-24 06:10:08) > > On Fri, Feb 19, 2021 at 6:54 AM Stephen Boyd <sboyd@kernel.org> wrote: > > > > > > > > + > > > > > + xlnx,speed-grade: > > > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > > > + enum: [1, 2, 3] > > > > > + description: > > > > > + Speed grade of the device. > > > > > > > > A bit of explanation of what this describes would be welcome. > > > > > > > > Don't forget that binding are not tied to any driver implementation, > > > > these are supposed to be hardware description properties. > > > > > > Would opp tables work for this? > > This is the parameter is for speed of the fabric. > > Ok. Yes or no? Is it configuring the speed of the fabric? Sounds like > assigned-clock-rates or assigned-interconnect-bandwidth or something > like that. I do not think we could use opp tables. Xilinx has products where we have partly FPGA and patrly PS(hardware is not programmable) so the ip could be in PL and the processor in PS. The speed grade influences a variety of timing parameters in the FPGA, including fabric (slice), multiplier/DSP48x, BlockRAM, I/O, and other resources parameters. Basically the timing of the fabric is determined by it. Also we are notifying the speed grade to the driver no configuration is done. We are telling which speed-grade fabric we are running on. There is no correlation between these numbers. It is really a relative metric of performance within a specific family.