Re: [PATCH 2/2] ARM: dts: mvebu: add device tree for IIJ SA-W2 appliance

[INAGAKI Hiroshi]

Hi Andrew,

thank you for your reviews and detailed descriptions.

On 2023/02/23 23:43, Andrew Lunn wrote:
> > +		pcie {
> > +			status = "okay";
> > +
> > +			pcie@1,0 {
> > +				status = "okay";
> > +
> > +				/* Atheros AR9287 */
> > +				wifi@0,0 {
> > +					compatible = "pci168c,002e";
> > +					reg = <0000 0 0 0 0>;
> > +				};
> > +			};
> > +
> > +			pcie@3,0 {
> > +				status = "okay";
> > +
> > +				/* Qualcomm Atheros QCA9880 */
> > +				wifi@0,0 {
> > +					compatible = "qcom,ath10k";
> > +					reg = <0000 0 0 0 0>;
> > +				};
> > +			};
> > +		};
> > +	};
> These are not wrong, but they are also not needed. PCI devices should
> be discovered by enumeration, and you don't have any additional
> properties here, or phandles pointing to these nodes.
>
> I assume these are COTS wifi modules? By listing them here you are
> restricting some flexibility. The OEM could for example swap the
> modules around, and Linux would not care, but the DT would then be
> wrong. Or you could have a device with a different module because it
> is cheaper, and again, Linux would not care, but the DT would be
> wrong.

Got it. SA-W2 is not designed to allow users to swap cards under 
normal use, but certainly things like you said can happen...
I'll remove "wifi" nodes.

> I assume these are COTS wifi modules?

Yes, those are the modules manufactured by Silex Technology, Inc. [1][2].

[1]: https://www.silex.jp/products/wireless-module/sxpcegn.html
[2]: https://www.silex.jp/products/wireless-module/sxpceac.html

> > +&usb0 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&pmx_usb_pins>;
> > +	status = "okay";
> > +	#address-cells = <1>;
> > +	#size-cells = <0>;
> > +
> > +	/* SMSC USB2514B */
> > +	hub@1 {
> > +		compatible = "usb424,2514";
> > +		reg = <1>;
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		hub_port1: port@1 {
> > +			reg = <1>;
> > +			#trigger-source-cells = <0>;
> > +		};
> > +
> > +		hub_port2: port@2 {
> > +			reg = <2>;
> > +			#trigger-source-cells = <0>;
> > +		};
> > +	};
> > +};
> Same comment as PCI. However, it is likely that the USB hub is
> actually on the board, not a module, so it is a lot less likely to
> change.

Yes, that USB hub is on the PCB and wired to the SoC directly. But 
I'll keep it in mind...

> As i said, they are not wrong, so you don't need to remove them.
>
> 	Andrew

Regards,
Hiroshi