Re: [PATCH v4 0/5] Binding and driver for gated-fixed-clocks

[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?

> 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@xxxxxxxxxx/
>
> 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