[PATCH v2 1/1] arm64: Add DTS support for FSL's LS1012A SoC

[Stuart Yoder]

> -----Original Message-----
> From: Bhaskar U
> Sent: Friday, September 30, 2016 4:13 PM
> To: Stuart Yoder <stuart.yoder at nxp.com>; Shawn Guo <shawnguo at kernel.org>
> Cc: devicetree at vger.kernel.org; oss at buserror.net; Prabhakar Kushwaha <prabhakar.kushwaha at nxp.com>;
> linux-devel at gforge.freescale.net; Pratiyush Srivastava <pratiyush.srivastava at nxp.com>; linux-arm-
> kernel at lists.infradead.org
> Subject: RE: [PATCH v2 1/1] arm64: Add DTS support for FSL's LS1012A SoC
> 
> 
> 
> >-----Original Message-----
> >From: Stuart Yoder
> >Sent: Friday, September 30, 2016 7:26 PM
> >To: Bhaskar U <bhaskar.upadhaya at nxp.com>; Shawn Guo
> ><shawnguo at kernel.org>
> >Cc: devicetree at vger.kernel.org; oss at buserror.net; Prabhakar Kushwaha
> ><prabhakar.kushwaha at nxp.com>; linux-devel at gforge.freescale.net; Pratiyush
> >Srivastava <pratiyush.srivastava at nxp.com>; linux-arm-
> >kernel at lists.infradead.org
> >Subject: RE: [PATCH v2 1/1] arm64: Add DTS support for FSL's LS1012A SoC
> >
> >> >> +&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".

> >>
> >> >
> >> >> +	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.
> >>
> >> >
> >> >> +
> >> >> +	qflash0: s25fs512s at 0 {
> >> >> +		compatible = "spansion,m25p80";
> >> >> +		#address-cells = <1>;
> >> >> +		#size-cells = <1>;
> >> >> +		spi-max-frequency = <20000000>;
> >> >> +		m25p,fast-read;
> >> >> +		reg = <0>;
> >> >> +	};
> >> >> +};
> >> >> +
> >> >> +&i2c0 {
> >> >> +	status = "okay";
> >> >> +
> >> >> +	codec: sgtl5000 at 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).

> >
> >> >
> >> >> +};
> >> >> +
> >> >> +&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