Re: [PATCH 1/3] ARM: dts: imx6qdl: add TQMa6{S,Q,QP} SoM

[Philipp Zabel <p.zabel@pengutronix.de>]

Hi Shawn,

On Sat, 2019-10-26 at 17:34 +0800, Shawn Guo wrote:
> On Fri, Oct 11, 2019 at 04:36:49PM +0200, Philipp Zabel wrote:
> > From: Markus Niebel <Markus.Niebel@tq-group.com>
> > 
> > Add device trees for TQMa6S, TQMa6Q, and TQMa6QP embedded modules.
> > The A and B SoM variants are for hardware revisions that differ in
> > how the I2C devices are connected. For details, see [1].
> 
> It looks like a perfect case to be handled by DT overlay.
> Did you consider of using that?

I don't think using DT overlays is feasible. The EEPROM that could tell
us which variant we are running on is on the I2C bus that is different
between the two variants.

[...]
> > +&ecspi1 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&pinctrl_ecspi1>;
> > +	fsl,spi-num-chipselects = <1>;
> 
> Obsolete property.
> 
> > +	cs-gpios = <&gpio3 19 0>;
> 
> GPIO_ACTIVE_HIGH
> 
> > +	status = "okay";
> > +
> > +	flash: m25p80@0 {
> 
> Node name should be generic, while label can be specific.
> 
> > +		status = "okay";
> 
> Not really needed.
> 
[...]
> > +&iomuxc {
> > +	tqma6 {
> 
> Drop this container node.
> 
[...]
> > +		pinctrl_i2c1_tqma6: i2c1-tqma6grp {
> 
> The '_tqma6' suffix isn't really useful.
> 
[...]
> > +&pmic {
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&pinctrl_pmic>;
> > +		interrupt-parent = <&gpio6>;
> > +		interrupts = <10 8>;
> 
> IRQ_TYPE_LEVEL_LOW
> 
[...]
> > +	pmic: pf0100@8 {
> 
> Node name should be generic, while label can be specific.
> 
> > +		compatible = "fsl,pfuze100";
> > +		reg = <0x08>;
> > +	};
> > +
> > +	sensor0: lm75@48 {
> 
> Ditto
> 
> > +		compatible = "lm75";
> > +		reg = <0x48>;
> > +	};
> > +
> > +	eeprom0: m24c64@50 {
> 
> Ditto

Thank you, I'll fix these.

regards
Philipp