> From: Leonard Crestez
> Sent: Monday, May 20, 2019 7:45 PM
>
> On 30.04.2019 20:35, Aisheng Dong wrote:
> > MX8QM and MX8QXP LPCG Clocks are mostly the same except they may
> > reside in different subsystems across CPUs and also vary a bit on the
> availability.
> >
> > Same as SCU clock, we want to move the clock definition into device
> > tree which can fully decouple the dependency of Clock ID definition
> > from device tree and make us be able to write a fully generic lpcg clock
> driver.
> >
> > And we can also use the existence of clock nodes in device tree to
> > address the device and clock availability differences across different SoCs.
> >
> > diff --git a/Documentation/devicetree/bindings/clock/imx8qxp-lpcg.txt
> > b/Documentation/devicetree/bindings/clock/imx8qxp-lpcg.txt
> >
> > Required properties:
> > +- compatible: Should be one of:
> > + "fsl,imx8qxp-lpcg"
> > + "fsl,imx8qm-lpcg" followed by "fsl,imx8qxp-lpcg".
> > +- reg: Address and length of the register set.
> > +- #clock-cells: Should be 1. One LPCG supports multiple clocks.
> > +- clocks: Input parent clocks phandle array for each clock.
> > +- bit-offset: An integer array indicating the bit offset for each clock.
> > +- hw-autogate: Boolean array indicating whether supports HW
> autogate for
> > + each clock.
> > +- clock-output-names: Shall be the corresponding names of the outputs.
> > + NOTE this property must be specified in the same order
> > + as the clock bit-offset and hw-autogate property.
>
> Splitting the LPCG areas is good but describing "bit-offset" and similar inside
> devicetree seems excessively generic.
>
> Perhaps we could have many smaller imx8qxp-lpcg-sdhc, imx8qxp-lpcg-enet
> etc where the actual clks inside each node are still defined in driver code.
>
If that way, we would have too many more compatible strings per clocks
and we have to add more for new SoCs which I'd like to avoid.
> > usdhc1: mmc@5b010000 {
> > @@ -44,8 +66,8 @@ usdhc1: mmc@5b010000 {
> > interrupt-parent = <&gic>;
> > interrupts = <GIC_SPI 232 IRQ_TYPE_LEVEL_HIGH>;
> > reg = <0x5b010000 0x10000>;
> > - clocks = <&conn_lpcg IMX8QXP_CONN_LPCG_SDHC0_IPG_CLK>,
> > - <&conn_lpcg IMX8QXP_CONN_LPCG_SDHC0_PER_CLK>,
> > - <&conn_lpcg IMX8QXP_CONN_LPCG_SDHC0_HCLK>;
> > + clocks = <&sdhc0_lpcg 1>,
> > + <&sdhc0_lpcg 0>,
> > + <&sdhc0_lpcg 2>;
>
> This is less readable, can't we keep symbolic constants?
I'm scared to define more macros for device clocks.
It's usually a one time job and could be referenced easily by looking into
the definition of sdhc0_lpcg in DT.
So less readable may not be a real problem.
Regards
Dong Aisheng
