Re: [PATCH v2 1/5] arm64: dts: rockchip: Add AP6275P wireless support to Khadas Edge 2

From: Krzysztof Kozlowski <krzk@kernel.org>
To: Jacobe Zang <jacobe.zang@wesion.com>,
	"arend.vanspriel@broadcom.com" <arend.vanspriel@broadcom.com>
Cc: "kvalo@kernel.org" <kvalo@kernel.org>,
	"duoming@zju.edu.cn" <duoming@zju.edu.cn>,
	"bhelgaas@google.com" <bhelgaas@google.com>,
	"minipli@grsecurity.net" <minipli@grsecurity.net>,
	"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
	"brcm80211@lists.linux.dev" <brcm80211@lists.linux.dev>,
	"brcm80211-dev-list.pdl@broadcom.com"
	<brcm80211-dev-list.pdl@broadcom.com>,
	"megi@xff.cz" <megi@xff.cz>, "robh@kernel.org" <robh@kernel.org>,
	"efectn@protonmail.com" <efectn@protonmail.com>,
	"krzk+dt@kernel.org" <krzk+dt@kernel.org>,
	"conor+dt@kernel.org" <conor+dt@kernel.org>,
	"heiko@sntech.de" <heiko@sntech.de>, Nick Xie <nick@khadas.com>,
	"jagan@edgeble.ai" <jagan@edgeble.ai>,
	"dsimic@manjaro.org" <dsimic@manjaro.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-rockchip@lists.infradead.org"
	<linux-rockchip@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 1/5] arm64: dts: rockchip: Add AP6275P wireless support to Khadas Edge 2
Date: Mon, 1 Jul 2024 10:41:30 +0200	[thread overview]
Message-ID: <3fe026a2-a8d6-4688-863f-1237e71945ea@kernel.org> (raw)
In-Reply-To: <TYZPR03MB7001D4DE507C919802B2F7F380D52@TYZPR03MB7001.apcprd03.prod.outlook.com>

On 25/06/2024 10:04, Jacobe Zang wrote:
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-khadas-edge2.dts b/arch/arm64/boot/dts/rockchip/rk3588s-khadas-edge2.dts
>>> index 3b6286461a746..f674deb6f7da8 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3588s-khadas-edge2.dts
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-khadas-edge2.dts
>>> @@ -356,6 +356,22 @@ &pcie2x1l2 {
>>>         reset-gpios = <&gpio3 RK_PD1 GPIO_ACTIVE_HIGH>;
>>>         vpcie3v3-supply = <&vcc3v3_pcie_wl>;
>>>         status = "okay";
>>> +
>>> +     pcie@0,0 {
>>> +             reg = <0x400000 0 0 0 0>;
>>> +             #address-cells = <3>;
>>> +             #size-cells = <2>;
>>> +             ranges;
>>> +             device_type = "pci";
>>> +             bus-range = <0x40 0x4f>;
>>
>> Isn't bus-range a property of PCI host bridge, so the parent? This is a
>> PCI device, right?
>>
>>> +
>>> +             wifi: wifi@0,0 {
>>
>> Binding does not say anything about this. Rockchip PCI controller is the
>> PCI host bridge, isn't it? Then the pci@0,0 is the child, so what is this?
> 
> The host bridge is the parent of pcie@0,0. And pcie@0,0 is Bridge1, so the

Do you want to say Rockchip PCI is PCI-PCI bridge? Bindings do not allow it.

> wifi@0,0 as a device under the Bridge1.
> 
>>
>>> +                     reg = <0x410000 0 0 0 0>;
>>> +                     clocks = <&hym8563>;
>>> +                     clock-names = "32k";
>>
>> 1. Bindings are before the users.
>> 2. Where is the compatible? Are you sure this validates?
> 
> Before, the compatible is "pci14e4,449d", but when I checkpatch the warning
> said that "pci14e4" was not documented, so I remove the compatible which 
> doesn't affect the Wi-Fi function. I have tried to add "pci14e4" to 
> vendor-prefixes.yaml but was refused. So whether should I add the compatible 
> with warning? 

I talk about dtbs_check, not checkpatch. That checkpatch warning does
not matter, obviously.

Best regards,
Krzysztof