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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <7d4a218d-8b8a-5a1d-eff8-e154bfde69be@gmail.com>
Date:   Fri, 24 Feb 2023 21:57:37 +0900
From:   INAGAKI Hiroshi <musashino.open@...il.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, gregory.clement@...tlin.com,
        sebastian.hesselbarth@...il.com, arnd@...db.de, olof@...om.net,
        soc@...nel.org, robh+dt@...nel.org,
        krzysztof.kozlowski+dt@...aro.org
Subject: Re: [PATCH 2/2] ARM: dts: mvebu: add device tree for IIJ SA-W2
 appliance

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