On Mon, Aug 12, 2019 at 02:41:55PM +0000, Aisheng Dong wrote:

quoted

From: Shawn Guo <shawnguo@kernel.org>
Sent: Monday, August 12, 2019 9:01 PM 
On Mon, Aug 05, 2019 at 11:27:20AM +0800, Dong Aisheng wrote:

quoted

quoted

quoted

+- compatible:                Should be one of:
+                       "fsl,imx8qxp-lpcg"
+                       "fsl,imx8qm-lpcg" followed by

"fsl,imx8qxp-lpcg".

quoted

quoted

quoted

+- reg:                       Address and length of the register set.
+- #clock-cells:              Should be 1. One LPCG supports multiple

clocks.

quoted

quoted

quoted

+- clocks:            Input parent clocks phandle array for each clock.
+- bit-offset:                An integer array indicating the bit offset

for each clock.

quoted

quoted

I guess that the driver should be able to figure bit offset from
'clock-indices' property.

Yes, it can be done in theory.
Then the binding may look like:
sdhc0_lpcg: clock-controller@5b200000 {
        ...
        #clock-cells = <1>;
        clocks = <&sdhc0_clk IMX_SC_PM_CLK_PER>,
                 <&conn_ipg_clk>, <&conn_axi_clk>;
        clock-indices = <0>, <16>, <20>;
        clock-output-names = "sdhc0_lpcg_per_clk",
                             "sdhc0_lpcg_ipg_clk",
                             "sdhc0_lpcg_ahb_clk";
        power-domains = <&pd IMX_SC_R_SDHC_0>; };

usdhc1: mmc@5b010000 {
        ...
        clocks = <&sdhc0_lpcg 16>,
                 <&sdhc0_lpcg 0>,
                 <&sdhc0_lpcg 20>;
        clock-names = "ipg", "per", "ahb"; };

However, after trying, i found  one limitation if using clock-indices
that users have to do a secondary search for the indices value from
clock names which is not very friendly.

Formerly from the clock output names, user can easily get the clock
index as they're in fixed orders as output names, so very easily to
use.
e.g.
clocks = <&sdhc0_lpcg 1>,
         <&sdhc0_lpcg 0>,
         <&sdhc0_lpcg 2>;

If using clock-indices, users have no way to know it's clock index
from clock output names order unless they do a secondary search from
the clock-indice array accordingly.
For example, for "sdhc0_lpcg_ahb_clk", user can easily know its
reference is <&sdhc0_lpcg 2>.
But if using clock-indice, we need search clock-indices array to find
its reference becomes <&sdhc0_lpcg 20>. So this seems like a drawback
if using clock-indices.

Shouldn't we have constant macro defined for those numbers, so that both
'clock-indices' and 'clocks' of client device can use?

I think we can do it.
Does below one look ok to you?
#define IMX_LPCG_ CLK_0	0
#define IMX_LPCG_ CLK_1	4
#define IMX_LPCG_ CLK_2	8
#define IMX_LPCG_ CLK_3	12
#define IMX_LPCG_ CLK_4	16
#define IMX_LPCG_ CLK_5	20
#define IMX_LPCG_ CLK_6	24
#define IMX_LPCG_ CLK_7	28

Looks fine to me, except the space in the middle of macro name, which
compiler will complain anyway :)

Shawn

The usage will look like:
<&sdhc0_lpcg IMX_LPCG_CLK_5>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel