-----Original Message-----
From: Bhaskar U
Sent: Friday, September 30, 2016 4:13 PM
To: Stuart Yoder <redacted>; Shawn Guo <shawnguo@kernel.org>
Cc: devicetree@vger.kernel.org; oss@buserror.net; Prabhakar Kushwaha <redacted>;
linux-devel@gforge.freescale.net; Pratiyush Srivastava [off-list ref]; linux-arm-
kernel@lists.infradead.org
Subject: RE: [PATCH v2 1/1] arm64: Add DTS support for FSL's LS1012A SoC

quoted

-----Original Message-----
From: Stuart Yoder
Sent: Friday, September 30, 2016 7:26 PM
To: Bhaskar U <redacted>; Shawn Guo
[off-list ref]
Cc: devicetree@vger.kernel.org; oss@buserror.net; Prabhakar Kushwaha
[off-list ref]; linux-devel@gforge.freescale.net; Pratiyush
Srivastava [off-list ref]; linux-arm-
kernel@lists.infradead.org
Subject: RE: [PATCH v2 1/1] arm64: Add DTS support for FSL's LS1012A SoC

quoted

quoted

quoted

+&qspi {
+	num-cs = <2>;
+	bus-num = <0>;
+	status = "disabled";

Why is it being disabled?

Ok, will change like below.
status = "okay";

The comment was not "change this to okay".  The question is why is this disabled?
Can you explain why it was disabled?   Should it have been disasbled?  Is qspi
working and tested on this board?

The intension of putting the status in disabled state is that the qspi functionality is not tested with
the up-streamed kernel.
Yes qspi is working and tested on this board with 4.1 kernel version.

If the qspi is supported and you have a high degree of confidence that the
node is correct based on testing on 4.1, I would say keep the node and
mark it as "ok".

quoted

quoted

quoted

quoted

+	fsl,ddr-sampling-point = <4>;

I do not find the bindings for this property, neither how driver supports it.

Yes the QSPI DDR mode is not yet up-streamed, so  I will remove this property

as of now.

quoted

quoted

quoted

+
+	qflash0: s25fs512s@0 {
+		compatible = "spansion,m25p80";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		spi-max-frequency = <20000000>;
+		m25p,fast-read;
+		reg = <0>;
+	};
+};
+
+&i2c0 {
+	status = "okay";
+
+	codec: sgtl5000@a {
+		#sound-dai-cells = <0>;
+		compatible = "fsl,sgtl5000";
+		reg = <0xa>;
+		VDDA-supply = <&reg_1p8v>;
+		VDDIO-supply = <&reg_1p8v>;
+		clocks = <&sys_mclk>;
+	};
+};
+
+&duart0 {
+	status = "okay";
+};
+
+&esdhc0 {
+	status = "disabled";

We prefer to disable devices which have board level options by
default in <soc>.dtsi, and enable them per availability in <board>.dts.

Ok , will make the status as okay i.e. status = "okay";

Again, the feedback was not "set this to okay".  Why was esdhc0 set to "disabled"
here in the first place?  Was there a reason?

The comment is that if there are certain boards where esdhc0 is not available,
then fsl-ls1012a.dtsi should set this to "disabled" and board .dts files should
override it.

esdhc0 is not there on this board so shall we mark the status in disabled state ?

If it's not there then it should be disabled, but it should be set to disabled
in the dtsi file (not the dts).

quoted

quoted

quoted

quoted

+};
+
+&esdhc1 {
+	status = "disabled";
+};
+
+&sai2 {
+	status = "disabled";
+};

Same comment for the above nodes.  The fsl-ls1012a.dtsi should set them to
disabled and any .dts file should override with "ok" if applicable.

esdhc1 is not there on the board, so shall we keep the status of esdhc1 in disabled state ?

If it's not there on the board, then yes, mark as disabled.

sai2 is working and tested on this board, so shall we put the sai2 status as  "okay" ?
Earlier when we kept sai2 status as disabled, by that time sai2 was not tested but now it is working
fine.

If the board supports it and it's been tested, then yes mark as "okay".

Stuart