mbox series

[v10,0/9] clk: clocking-wizard: driver updates

Message ID 1614172241-17326-1-git-send-email-shubhrajyoti.datta@xilinx.com
Headers show
Series clk: clocking-wizard: driver updates | expand

Message

Shubhrajyoti Datta Feb. 24, 2021, 1:10 p.m. UTC
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.

Changes in the v3:
Added the cover-letter.
Add patches for rate setting and fractional support
Add patches for warning.
Remove the driver from staging as suggested

v4:
Reorder the patches.
Merge the CLK_IS_BASIC patch.
Add the yaml form of binding document

v5:
Fix a mismerge

v6:
Fix the yaml warning
use poll timedout

v7:
Binding doc updates
Use common divisor function.

v8:
Fix Robs comments

v9:
Fix device tree warnings

v10:
Reorder the patches
Update the speed grade description.

[1] https://spinics.net/lists/linux-driver-devel/msg117326.html

Shubhrajyoti Datta (9):
  staging: clocking-wizard: Fix kernel-doc warning
  staging: clocking-wizard: Rename speed-grade to xlnx,speed-grade
  staging: clocking-wizard: Update the fixed factor divisors
  staging: clocking-wizard: Allow changing of parent rate for single
    output
  staging: clocking-wizard: Add support for dynamic reconfiguration
  staging: clocking-wizard: Add support for fractional support
  staging: clocking-wizard: Remove the hardcoding of the clock outputs
  dt-bindings: add documentation of xilinx clocking wizard
  clk: clock-wizard: Add the clockwizard to clk directory

 .../bindings/clock/xlnx,clocking-wizard.yaml       |  72 +++
 drivers/clk/Kconfig                                |   9 +
 drivers/clk/Makefile                               |   1 +
 drivers/clk/clk-xlnx-clock-wizard.c                | 636 +++++++++++++++++++++
 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, 718 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

Comments

Rob Herring (Arm) March 6, 2021, 8:20 p.m. UTC | #1
On Wed, Feb 24, 2021 at 06:40:40PM +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

>  v10:

>  Add nr-outputs

> 

>  .../bindings/clock/xlnx,clocking-wizard.yaml       | 72 ++++++++++++++++++++++

>  1 file changed, 72 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..280eb09

> --- /dev/null

> +++ b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml

> @@ -0,0 +1,72 @@

> +# 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


Not very specific. Only 1 version of this h/w?

> +

> +  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

> +

> +

> +  xlnx,speed-grade:

> +    $ref: /schemas/types.yaml#/definitions/uint32

> +    enum: [1, 2, 3]

> +    description:

> +      Speed grade of the device. Higher the speed grade faster is the FPGA device.


How does one decide what value?

> +

> +  nr-outputs:


xlnx,nr-outputs

> +    $ref: /schemas/types.yaml#/definitions/uint32

> +    enum: [1, 2, 3, 4, 5, 6, 7, 8]


minimum: 1
maximum: 8

> +    description:

> +      Number of outputs.

> +

> +required:

> +  - compatible

> +  - reg

> +  - "#clock-cells"

> +  - clocks

> +  - clock-names

> +  - xlnx,speed-grade

> +  - nr-outputs

> +

> +additionalProperties: false

> +

> +examples:

> +  - |

> +    wizard@b0000000  {


clock-controller@...

> +        compatible = "xlnx,clocking-wizard";

> +        reg = <0xb0000000 0x10000>;

> +        #clock-cells = <1>;

> +        xlnx,speed-grade = <1>;

> +        nr-outputs = <6>;

> +        clock-names = "clk_in1", "s_axi_aclk";

> +        clocks = <&clkc 15>, <&clkc 15>;

> +    };

> +...

> -- 

> 2.1.1

>
Shubhrajyoti Datta April 8, 2021, 10:26 a.m. UTC | #2
On Sun, Mar 7, 2021 at 1:50 AM Rob Herring <robh@kernel.org> wrote:
>

> On Wed, Feb 24, 2021 at 06:40:40PM +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

> >  v10:

> >  Add nr-outputs

> >

> >  .../bindings/clock/xlnx,clocking-wizard.yaml       | 72 ++++++++++++++++++++++

> >  1 file changed, 72 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..280eb09

> > --- /dev/null

> > +++ b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml

> > @@ -0,0 +1,72 @@

> > +# 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

>

> Not very specific. Only 1 version of this h/w?


Will fix in next version
>

> > +

> > +  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

> > +

> > +

> > +  xlnx,speed-grade:

> > +    $ref: /schemas/types.yaml#/definitions/uint32

> > +    enum: [1, 2, 3]

> > +    description:

> > +      Speed grade of the device. Higher the speed grade faster is the FPGA device.

>

> How does one decide what value?

This is a property of the FPGA fabric.
So  hdf/xsa  should tell that
>

> > +

> > +  nr-outputs:

>

> xlnx,nr-outputs

>

> > +    $ref: /schemas/types.yaml#/definitions/uint32

> > +    enum: [1, 2, 3, 4, 5, 6, 7, 8]

>

> minimum: 1

> maximum: 8

>

> > +    description:

> > +      Number of outputs.

> > +

> > +required:

> > +  - compatible

> > +  - reg

> > +  - "#clock-cells"

> > +  - clocks

> > +  - clock-names

> > +  - xlnx,speed-grade

> > +  - nr-outputs

> > +

> > +additionalProperties: false

> > +

> > +examples:

> > +  - |

> > +    wizard@b0000000  {

>

> clock-controller@...

will fix.
>

> > +        compatible = "xlnx,clocking-wizard";

> > +        reg = <0xb0000000 0x10000>;

> > +        #clock-cells = <1>;

> > +        xlnx,speed-grade = <1>;

> > +        nr-outputs = <6>;

> > +        clock-names = "clk_in1", "s_axi_aclk";

> > +        clocks = <&clkc 15>, <&clkc 15>;

> > +    };

> > +...

> > --

> > 2.1.1

> >
Michal Simek April 8, 2021, 10:40 a.m. UTC | #3
On 4/8/21 12:26 PM, Shubhrajyoti Datta wrote:
> On Sun, Mar 7, 2021 at 1:50 AM Rob Herring <robh@kernel.org> wrote:

>>

>> On Wed, Feb 24, 2021 at 06:40:40PM +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

>>>  v10:

>>>  Add nr-outputs

>>>

>>>  .../bindings/clock/xlnx,clocking-wizard.yaml       | 72 ++++++++++++++++++++++

>>>  1 file changed, 72 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..280eb09

>>> --- /dev/null

>>> +++ b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml

>>> @@ -0,0 +1,72 @@

>>> +# 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

>>

>> Not very specific. Only 1 version of this h/w?

> 

> Will fix in next version

>>

>>> +

>>> +  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

>>> +

>>> +

>>> +  xlnx,speed-grade:

>>> +    $ref: /schemas/types.yaml#/definitions/uint32

>>> +    enum: [1, 2, 3]

>>> +    description:

>>> +      Speed grade of the device. Higher the speed grade faster is the FPGA device.

>>

>> How does one decide what value?

> This is a property of the FPGA fabric.

> So  hdf/xsa  should tell that


Shubhrajyoti: Rob likely doesn't know what hdf/xsa is that's why it is
better to avoid it.

fpgas/pl part of SoC are tested for performance and different chips have
different speed grades. This is done for every chip and some chips are
faster/slower. Based on this speed grade is labeled. And there is no way
how to find at run time which speed grade your device has. That's why
there is a need to have property to identify this.

In designed tools it is your responsibility to select proper chip based
on your order and then this value is propagated in Xilinx standard way
via device tree generator to fill the right value for this property.

Thanks,
Michal
Stephen Boyd April 9, 2021, 11:32 p.m. UTC | #4
Quoting Michal Simek (2021-04-08 03:40:29)
> 

> 

> On 4/8/21 12:26 PM, Shubhrajyoti Datta wrote:

> > On Sun, Mar 7, 2021 at 1:50 AM Rob Herring <robh@kernel.org> wrote:

> >>

> >> On Wed, Feb 24, 2021 at 06:40:40PM +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

> >>>  v10:

> >>>  Add nr-outputs

> >>>

> >>>  .../bindings/clock/xlnx,clocking-wizard.yaml       | 72 ++++++++++++++++++++++

> >>>  1 file changed, 72 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..280eb09

> >>> --- /dev/null

> >>> +++ b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml

> >>> @@ -0,0 +1,72 @@

> >>> +# 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

> >>

> >> Not very specific. Only 1 version of this h/w?

> > 

> > Will fix in next version

> >>

> >>> +

> >>> +  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

> >>> +

> >>> +

> >>> +  xlnx,speed-grade:

> >>> +    $ref: /schemas/types.yaml#/definitions/uint32

> >>> +    enum: [1, 2, 3]

> >>> +    description:

> >>> +      Speed grade of the device. Higher the speed grade faster is the FPGA device.

> >>

> >> How does one decide what value?

> > This is a property of the FPGA fabric.

> > So  hdf/xsa  should tell that

> 

> Shubhrajyoti: Rob likely doesn't know what hdf/xsa is that's why it is

> better to avoid it.

> 

> fpgas/pl part of SoC are tested for performance and different chips have

> different speed grades. This is done for every chip and some chips are

> faster/slower. Based on this speed grade is labeled. And there is no way

> how to find at run time which speed grade your device has. That's why

> there is a need to have property to identify this.

> 

> In designed tools it is your responsibility to select proper chip based

> on your order and then this value is propagated in Xilinx standard way

> via device tree generator to fill the right value for this property.


The OPP framework and binding has support for speed grades via the
'supported-hw' property. I expect this speed-grade property could be
dropped and an opp table could be assigned to the clk controller node
for this speed grade by the DT author. Unfortunate that it isn't burned
somewhere into the device so that software can pick the right frequency
limits that way.
Shubhrajyoti Datta May 13, 2021, 6:48 a.m. UTC | #5
On Sat, Apr 10, 2021 at 5:02 AM Stephen Boyd <sboyd@kernel.org> wrote:
>

> Quoting Michal Simek (2021-04-08 03:40:29)

> >

> >

> > On 4/8/21 12:26 PM, Shubhrajyoti Datta wrote:

> > > On Sun, Mar 7, 2021 at 1:50 AM Rob Herring <robh@kernel.org> wrote:

> > >>

> > >> On Wed, Feb 24, 2021 at 06:40:40PM +0530, Shubhrajyoti Datta wrote:

> > >>> Add the devicetree binding for the xilinx clocking wizard.

> > >>>

> > >>> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>

> > >>> ---

...
> >

> > In designed tools it is your responsibility to select proper chip based

> > on your order and then this value is propagated in Xilinx standard way

> > via device tree generator to fill the right value for this property.

>

> The OPP framework and binding has support for speed grades via the

> 'supported-hw' property. I expect this speed-grade property could be

> dropped and an opp table could be assigned to the clk controller node

> for this speed grade by the DT author. Unfortunate that it isn't burned

> somewhere into the device so that software can pick the right frequency

> limits that way.


Rob let me know your opinion I will implement it in that way.