mbox series

[v4,0/5] Binding and driver for gated-fixed-clocks

Message ID 20240906082511.2963890-1-heiko@sntech.de (mailing list archive)
Headers show
Series Binding and driver for gated-fixed-clocks | expand

Message

Heiko Stuebner Sept. 6, 2024, 8:25 a.m. UTC
Rockchip boards with PCIe3 controllers inside the soc (rk3568, rk3588) have
external oscillators on the board to generate the needed 100MHz reference
clock the PCIe3 controller needs.

Often these clock generators need supplies to be enabled to run.

Modelling this clock has taken a number of shapes:
- The rk3568 Rock-3a modelled the generator-regulator as "phy-supply" [0]
  &pcie30phy {
  	phy-supply = <&vcc3v3_pi6c_03>;
  	status = "okay";
  };
  which is of course not part of the binding

- On the Rock-5-ITX the supply of the clock generator is controlled by
  the same gpio as the regulator supplying the the port connected to the
  pcie30x4 controller, so if this controller probes first, both
  controllers will just run. But if the pcie30x2 controller probes first
  (which has a different supply), the controller will stall at the first
  dbi read.

There are other types too, where an 25MHz oscillator supplies a PLL
chip like the diodes,pi6c557 used on Theobroma Jaguar and Tiger boards.

As we established in v1 [1], these are essentially different types, so
this series attempts to solve the first case of "voltage controlled
oscillators" as Stephen called them.


With the discussion in v2, gated-fixed-clock was deemed one possible
nice naming, so I did go with that.
Stephen also suggested reusing more of clk-gpio to not re-implement the
gpio handling wrt. sleeping and non-sleeping gpios.

Though instead of exporting masses of structs and ops, gated-fixed-clock
is quite close to the other gpio-clocks, so I've put it into the clk-gpio
file.

changes in v4:
- fix example node-name in binding (Rob)
- add Rob's and Conor's Reviewed-by
- which -> with in patch 2 message (Diederik)
- store rate as unsigned long (with a temporary u32 to make
  of_property_read_u32 happy) (Stephen)
- add static to struct clk_ops (Stephen)
- match table sentinel (Stephen)
- some formatting (Stephen)

changes in v3:
- rename to gated-fixed-clock (Conor)
- move into clk-gpio
- some tiny cleanups to the existing clk-gpio drivers

changes in v2:
- drop the Diodes PLLs for now, to get the first variant right
- rename stuff to voltage-oscillator / clk_vco as suggested by Stephen
- require vdd-supply in the binding
- enable-gpios stays optional, as they often are tied to vdd-supply
- drop deprecated elements that were left in from the fixed clock binding


[0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts#n605
[1] https://lore.kernel.org/linux-clk/b3c450a94bcb4ad0bc5b3c7ee8712cb8.sboyd@kernel.org/

Heiko Stuebner (5):
  dt-bindings: clocks: add binding for gated-fixed-clocks
  clk: clk-gpio: update documentation for gpio-gate clock
  clk: clk-gpio: use dev_err_probe for gpio-get failure
  clk: clk-gpio: add driver for gated-fixed-clocks
  arm64: dts: rockchip: fix the pcie refclock oscillator on Rock 5 ITX

 .../bindings/clock/gated-fixed-clock.yaml     |  49 +++++
 .../boot/dts/rockchip/rk3588-rock-5-itx.dts   |  38 +++-
 drivers/clk/clk-gpio.c                        | 206 ++++++++++++++++--
 3 files changed, 277 insertions(+), 16 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/gated-fixed-clock.yaml

Comments

Dragan Simic Oct. 13, 2024, 7:58 p.m. UTC | #1
Hello Heiko,

On 2024-09-06 10:25, Heiko Stuebner wrote:
> Rockchip boards with PCIe3 controllers inside the soc (rk3568, rk3588) 
> have
> external oscillators on the board to generate the needed 100MHz 
> reference
> clock the PCIe3 controller needs.
> 
> Often these clock generators need supplies to be enabled to run.
> 
> Modelling this clock has taken a number of shapes:
> - The rk3568 Rock-3a modelled the generator-regulator as "phy-supply" 
> [0]
>   &pcie30phy {
>   	phy-supply = <&vcc3v3_pi6c_03>;
>   	status = "okay";
>   };
>   which is of course not part of the binding
> 
> - On the Rock-5-ITX the supply of the clock generator is controlled by
>   the same gpio as the regulator supplying the the port connected to 
> the
>   pcie30x4 controller, so if this controller probes first, both
>   controllers will just run. But if the pcie30x2 controller probes 
> first
>   (which has a different supply), the controller will stall at the 
> first
>   dbi read.
> 
> There are other types too, where an 25MHz oscillator supplies a PLL
> chip like the diodes,pi6c557 used on Theobroma Jaguar and Tiger boards.
> 
> As we established in v1 [1], these are essentially different types, so
> this series attempts to solve the first case of "voltage controlled
> oscillators" as Stephen called them.
> 
> With the discussion in v2, gated-fixed-clock was deemed one possible
> nice naming, so I did go with that.

Thanks, I find "gated-fixed-clock" a much better choice.

> Stephen also suggested reusing more of clk-gpio to not re-implement the
> gpio handling wrt. sleeping and non-sleeping gpios.
> 
> Though instead of exporting masses of structs and ops, 
> gated-fixed-clock
> is quite close to the other gpio-clocks, so I've put it into the 
> clk-gpio
> file.

Just checking, what's the current state of this patch series?
Would another review help with getting it accepted?

> changes in v4:
> - fix example node-name in binding (Rob)
> - add Rob's and Conor's Reviewed-by
> - which -> with in patch 2 message (Diederik)
> - store rate as unsigned long (with a temporary u32 to make
>   of_property_read_u32 happy) (Stephen)
> - add static to struct clk_ops (Stephen)
> - match table sentinel (Stephen)
> - some formatting (Stephen)
> 
> changes in v3:
> - rename to gated-fixed-clock (Conor)
> - move into clk-gpio
> - some tiny cleanups to the existing clk-gpio drivers
> 
> changes in v2:
> - drop the Diodes PLLs for now, to get the first variant right
> - rename stuff to voltage-oscillator / clk_vco as suggested by Stephen
> - require vdd-supply in the binding
> - enable-gpios stays optional, as they often are tied to vdd-supply
> - drop deprecated elements that were left in from the fixed clock 
> binding
> 
> [0] 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts#n605
> [1] 
> https://lore.kernel.org/linux-clk/b3c450a94bcb4ad0bc5b3c7ee8712cb8.sboyd@kernel.org/
> 
> Heiko Stuebner (5):
>   dt-bindings: clocks: add binding for gated-fixed-clocks
>   clk: clk-gpio: update documentation for gpio-gate clock
>   clk: clk-gpio: use dev_err_probe for gpio-get failure
>   clk: clk-gpio: add driver for gated-fixed-clocks
>   arm64: dts: rockchip: fix the pcie refclock oscillator on Rock 5 ITX
> 
>  .../bindings/clock/gated-fixed-clock.yaml     |  49 +++++
>  .../boot/dts/rockchip/rk3588-rock-5-itx.dts   |  38 +++-
>  drivers/clk/clk-gpio.c                        | 206 ++++++++++++++++--
>  3 files changed, 277 insertions(+), 16 deletions(-)
>  create mode 100644
> Documentation/devicetree/bindings/clock/gated-fixed-clock.yaml
Heiko Stuebner Oct. 13, 2024, 8:27 p.m. UTC | #2
Am Sonntag, 13. Oktober 2024, 21:58:41 CEST schrieb Dragan Simic:
> Hello Heiko,
> 
> On 2024-09-06 10:25, Heiko Stuebner wrote:
> > Rockchip boards with PCIe3 controllers inside the soc (rk3568, rk3588) 
> > have
> > external oscillators on the board to generate the needed 100MHz 
> > reference
> > clock the PCIe3 controller needs.
> > 
> > Often these clock generators need supplies to be enabled to run.
> > 
> > Modelling this clock has taken a number of shapes:
> > - The rk3568 Rock-3a modelled the generator-regulator as "phy-supply" 
> > [0]
> >   &pcie30phy {
> >   	phy-supply = <&vcc3v3_pi6c_03>;
> >   	status = "okay";
> >   };
> >   which is of course not part of the binding
> > 
> > - On the Rock-5-ITX the supply of the clock generator is controlled by
> >   the same gpio as the regulator supplying the the port connected to 
> > the
> >   pcie30x4 controller, so if this controller probes first, both
> >   controllers will just run. But if the pcie30x2 controller probes 
> > first
> >   (which has a different supply), the controller will stall at the 
> > first
> >   dbi read.
> > 
> > There are other types too, where an 25MHz oscillator supplies a PLL
> > chip like the diodes,pi6c557 used on Theobroma Jaguar and Tiger boards.
> > 
> > As we established in v1 [1], these are essentially different types, so
> > this series attempts to solve the first case of "voltage controlled
> > oscillators" as Stephen called them.
> > 
> > With the discussion in v2, gated-fixed-clock was deemed one possible
> > nice naming, so I did go with that.
> 
> Thanks, I find "gated-fixed-clock" a much better choice.
> 
> > Stephen also suggested reusing more of clk-gpio to not re-implement the
> > gpio handling wrt. sleeping and non-sleeping gpios.
> > 
> > Though instead of exporting masses of structs and ops, 
> > gated-fixed-clock
> > is quite close to the other gpio-clocks, so I've put it into the 
> > clk-gpio
> > file.
> 
> Just checking, what's the current state of this patch series?
> Would another review help with getting it accepted?

I guess me needing to ping Stephen to look at it now that the
merge window is done ;-) .

In the previous version he sounded ok with the naming, so hopefully
it'll just need a tiny ping.


Heiko
Heiko Stuebner Oct. 22, 2024, 2:12 p.m. UTC | #3
On Fri, 6 Sep 2024 10:25:06 +0200, Heiko Stuebner wrote:
> Rockchip boards with PCIe3 controllers inside the soc (rk3568, rk3588) have
> external oscillators on the board to generate the needed 100MHz reference
> clock the PCIe3 controller needs.
> 
> Often these clock generators need supplies to be enabled to run.
> 
> Modelling this clock has taken a number of shapes:
> - The rk3568 Rock-3a modelled the generator-regulator as "phy-supply" [0]
>   &pcie30phy {
>   	phy-supply = <&vcc3v3_pi6c_03>;
>   	status = "okay";
>   };
>   which is of course not part of the binding
> 
> [...]

Applied, thanks!

[5/5] arm64: dts: rockchip: fix the pcie refclock oscillator on Rock 5 ITX
      commit: e684f02492f99d6f6f037a35a613607339cf8e8f

Best regards,