Hi Rob,
> > > > a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> > > > b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> > > > index 72d481c..2816789 100644
> > > > --- a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> > > > +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> > > > @@ -78,6 +78,19 @@ Required properties:
> > > > "fsl,imx8qm-clock"
> > > > "fsl,imx8qxp-clock"
> > > > followed by "fsl,scu-clk"
> > > > +- #clock-cells: Should be 0.
> > > > +- rsrc-id: Resource ID associated with this clock
> > > > +- clk-type: Type of this clock.
> > > > + Refer to <include/dt-bindings/firmware/imx/rsrc.h>
> for
> > > > + available clock types supported by SCU.
> > >
> > > Can't you just make these 2 values clock cells? I'm all for getting
> > > rid of made up clock numbers.
> > >
> >
> > Thanks for the agreement to remove clock IDs.
> >
> > The 2 values clock cell seems not the best approach for i.MX because
> > it still needs to define all clocks in the driver which is exactly we
> > want to avoid now due to some special HW characteristic:
>
> Why's that? You can walk the DT and extract the 2 cells for each clock present.
> That's not any different than walking child nodes here and getting the resource
> ids and type. That's not really fast, but if speed is really an issue we can
> consider addressing that in ways that extend rather than change the binding.
>
Due to searching the 'clocks' property of all device nodes indirectly to
exact the 2 cell value causes much troubles in driver implementation and
looks a bit weird and is very low efficiency ( the performance will also potentially
be affected by adding new unrelevant nodes which is bad), we found the below
alternative way to do the same 2 cell binding, but having no performance issue.
It's much similar to the ARM SCPI clock binding.
Documentation/devicetree/bindings/arm/arm,scpi.txt
Do you think if it's okay to you?
If we have to use 2 cell binding, we probably would prefer to use this way
as it can relief us a lot from indirectly searching the 'clocks' property.
If you're ok, please let me know, I will make it in V2 for the review.
enet0_clk: clock-enet0 {
compatible = "fsl,imx8qxp-clock", "fsl,scu-clk";
#clock-cells = <2>;
clock-indice = <IMX_SC_PM_CLK_PER>, // clock cell 2 value
<IMX_SC_PM_CLK_BYPASS>,
<IMX_SC_PM_CLK_MISC0>;
clock-output-names = "enet0_clk",
"enet0_bypass_clk",
"enet0_rgmii_clk";
// we can use the same resource id for clock cells 1 value
// or probably encoded in node@reg?
power-domains = <&pd IMX_SC_R_ENET_0>;
};
enet0_lpcg: clock-controller@5b230000 {
compatible = "fsl,imx8qxp-lpcg";
reg = <0x5b230000 0x10000>;
#clock-cells = <1>;
clocks = <&enet0_clk IMX_SC_R_ENET_0 IMX_SC_PM_CLK_PER>,
<&enet0_clk IMX_SC_R_ENET_0 IMX_SC_PM_CLK_PER>,
<&conn_axi_clk>, <&conn_ipg_clk>, <&conn_ipg_clk>;
bit-offset = <0 4 8 16 20>;
clock-output-names = "enet0_ipg_root_clk",
"enet0_tx_clk",
"enet0_ahb_clk",
"enet0_ipg_clk",
"enet0_ipg_s_clk";
power-domains = <&pd IMX_SC_R_ENET_0>;
};
fec1: ethernet@5b040000 {
reg = <0x5b040000 0x10000>;
interrupts = <GIC_SPI 258 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 256 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 257 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 259 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&enet0_lpcg 3>,
<&enet0_lpcg 2>,
<&enet0_lpcg 1>,
<&enet0_lpcg 0>;
clock-names = "ipg", "ahb", "enet_clk_ref", "ptp";
fsl,num-tx-queues=<3>;
fsl,num-rx-queues=<3>;
power-domains = <&pd IMX_SC_R_ENET_0>;
status = "disabled";
};
Regards
Dong Aisheng
