Hi Scott,

Please see my reply inline.

-----Original Message-----
From: Scott Wood <oss@buserror.net>
Sent: 2018年10月22日 13:21
To: Andy Tang <redacted>; mturquette@baylibre.com
Cc: sboyd@kernel.org; robh+dt@kernel.org; mark.rutland@arm.com;
linux-clk@vger.kernel.org; devicetree@vger.kernel.org
Subject: Re: [PATCH 2/3] powerpc: t102x: upgrade the legacy clock node

On Mon, 2018-10-22 at 01:34 +0000, Andy Tang wrote:

quoted

Hi Scott,

Please see my reply inline.

quoted

-----Original Message-----
From: Scott Wood <oss@buserror.net>
Sent: 2018年10月21日 7:54
To: Andy Tang <redacted>; mturquette@baylibre.com
Cc: sboyd@kernel.org; robh+dt@kernel.org; mark.rutland@arm.com;
linux-clk@vger.kernel.org; devicetree@vger.kernel.org
Subject: Re: [PATCH 2/3] powerpc: t102x: upgrade the legacy clock
node

On Wed, 2018-09-19 at 15:39 +0800, andy.tang@nxp.com wrote:

quoted

+clockgen: global-utilities@e1000 {
+	compatible = "fsl,qoriq-clockgen";

Where does this compatible string come from?

quoted

+		compatible = "fsl,t1023-clockgen";
 	};

And here you overwrite it with only the chip-specific compatible?

Is t1023 incompatible with both fsl,qoriq-clockgen-1.0 and
fsl,qoriq-clockgen- 2.0?  The existing dts says 2.0; is that wrong?

BTW, assuming it is 2.0 compatible and thus the use of
qoriq-clockgen2.dtsi is correct, the best course of action is
probably to to remove the legacy stuff from all fsl chips, rather
than introduce a new dtsi.
In fact it'd be nice to see it all removed in any case. :-)

qoriq-clockgen*.dtsi are used by legacy bindings. The contents are all
of legacy bindings.
To use new framework, I introduce a new dtsi which contains new
bindings and used for all PPC soc.
A chip-specific compatible is needed because driver will use it to get
chip- specific clock tree information.
The clock information was defined in driver not in dts in new
framework, remember?

I'm aware that a chip-specific compatible is required.  What is unusual (on
PPC) is providing *only* the chip-specific compatible.  I was expecting
something more along the lines of
https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fp
atchwork.ozlabs.org%2Fpatch%2F486565%2F&data=02%7C01%7Can
dy.tang%40nxp.com%7Ceaf46e43620b45a19eb408d637de20cb%7C686e
a1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636757824505667539
&sdata=T5yRSWtHYO2yLMjsZRIxZT%2BM82xuGQm2VGX7MRB8MJA%
3D&reserved=0

Granted, the driver requiring two different compatibles to be present is
odd, and was a convenience based on what was already in the device
trees, but the
fsl,qoriq-clockgen-2.0 compatible is part of the new binding (albeit an
optional part).  I guess I don't mind no longer relying on it, but at least
remove the "fsl,qoriq-clockgen" string.

After saw your previous patch, I plan to work out a similar patch. The fsl,qoriq-clockgen-2.0 and fsl,qoriq-clockgen-1.0 will be retained.

Thanks,
Andy

-Scott