diff mbox series

[v2,4/5] arm64: dts: rockchip: Add AP6275P wireless support to Khadas Edge 2

Message ID 20240617071112.3133101-5-jacobe.zang@wesion.com (mailing list archive)
State New
Headers show
Series arm64: dts: Add board support for Khadas Edge2 | expand

Commit Message

Jacobe Zang June 17, 2024, 7:11 a.m. UTC
Khadas Edge2 uses the PCI-e Ampak AP6275P 2T2R Wi-Fi 6 module.

Signed-off-by: Jacobe Zang <jacobe.zang@wesion.com>
---
 .../boot/dts/rockchip/rk3588s-khadas-edge2.dts  | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Comments

Alexey Charkov June 17, 2024, 9:44 a.m. UTC | #1
On 17/06/2024 10:11, Jacobe Zang wrote:
> Khadas Edge2 uses the PCI-e Ampak AP6275P 2T2R Wi-Fi 6 module.
>
> Signed-off-by: Jacobe Zang <jacobe.zang@wesion.com>
> ---
>   .../boot/dts/rockchip/rk3588s-khadas-edge2.dts  | 17 +++++++++++++++++
>   1 file changed, 17 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-khadas-edge2.dts b/arch/arm64/boot/dts/rockchip/rk3588s-khadas-edge2.dts
> index 233bab17bffd2..7d7cc3e76838c 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588s-khadas-edge2.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-khadas-edge2.dts
> @@ -365,6 +365,23 @@ &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>;
> +
> +		wifi: wifi@0,0 {
> +			compatible = "pci14e4,449d";

This doesn't seem to be listed in the bindings, nor is there a mainline 
driver that would match either this compatible or PCI ID 14e4:449d. 
Maybe submit either or both of those first, to make sure they are 
reviewed and acceptable for mainline inclusion, before this change lands 
in DTS and becomes part of the ABI?

I'm also wondering why would adding a DT node for a PCI device be needed 
in the first place, given that PCI supports device discovery? Does it 
require some sort of boot-time fixup by the bootloader? If so, it might 
be helpful to state that in comments.

Best regards,

Alexey
Jacobe Zang June 17, 2024, 10:57 a.m. UTC | #2
>I'm also wondering why would adding a DT node for a PCI device be needed
>in the first place, given that PCI supports device discovery?

In fact, I learn that PCIe bus devices do not need compatible to probe just now... Before sending this patch, I committed the code that added "pci14e4,449d" to vendor-prefix.yaml and net/wireless/brcm,brcm4329-fmac.yaml. Now I know the reason why my addition was rejected. By the way, except for the compatible binding, is there any other binding that I should remove??

---
Best Regards
Jacobe
Alexey Charkov June 17, 2024, 11:17 a.m. UTC | #3
On Mon, Jun 17, 2024 at 2:58 PM Jacobe Zang <jacobe.zang@wesion.com> wrote:
>
> >I'm also wondering why would adding a DT node for a PCI device be needed
> >in the first place, given that PCI supports device discovery?
>
> In fact, I learn that PCIe bus devices do not need compatible to probe just now... Before sending this patch, I committed the code that added "pci14e4,449d" to vendor-prefix.yaml and net/wireless/brcm,brcm4329-fmac.yaml. Now I know the reason why my addition was rejected. By the way, except for the compatible binding, is there any other binding that I should remove??

If your PCI bridge is functioning properly and if your WiFi adapter is
connected and physically enabled (in terms of power and RFKILL) I
believe it should be automatically discovered and you should see it in
lspci. No additional DT nodes needed - but check if you need any
additional DT property somewhere to keep the HYM8563 clock enabled.
I'm not sure your pcie@0,0 node is needed either.

Then it's up to the driver to recognize your adapter by its PCI ID and
attach. I guess you'd need to extend the hardware IDs table in the
brcmfmac driver for it to attach - similar to [1]

[1] https://github.com/armbian/build/blob/main/patch/kernel/archive/rockchip-rk3588-6.10/0801-wireless-add-bcm43752.patch

Best regards,
Alexey
Sebastian Reichel June 18, 2024, 12:10 a.m. UTC | #4
Hi,

On Mon, Jun 17, 2024 at 03:17:50PM GMT, Alexey Charkov wrote:
> On Mon, Jun 17, 2024 at 2:58 PM Jacobe Zang <jacobe.zang@wesion.com> wrote:
> >
> > >I'm also wondering why would adding a DT node for a PCI device be needed
> > >in the first place, given that PCI supports device discovery?
> >
> > In fact, I learn that PCIe bus devices do not need compatible to probe just now... Before sending this patch, I committed the code that added "pci14e4,449d" to vendor-prefix.yaml and net/wireless/brcm,brcm4329-fmac.yaml. Now I know the reason why my addition was rejected. By the way, except for the compatible binding, is there any other binding that I should remove??
> 
> If your PCI bridge is functioning properly and if your WiFi adapter is
> connected and physically enabled (in terms of power and RFKILL) I
> believe it should be automatically discovered and you should see it in
> lspci. No additional DT nodes needed - but check if you need any
> additional DT property somewhere to keep the HYM8563 clock enabled.
> I'm not sure your pcie@0,0 node is needed either.
> 
> Then it's up to the driver to recognize your adapter by its PCI ID and
> attach. I guess you'd need to extend the hardware IDs table in the
> brcmfmac driver for it to attach - similar to [1]
> 
> [1] https://github.com/armbian/build/blob/main/patch/kernel/archive/rockchip-rk3588-6.10/0801-wireless-add-bcm43752.patch

RK3588 EVB1 has the same WLAN/BT module and I started looking
into adding support at a low priority. I think this is very
similar to the Qualcomm chip Bartosz Golaszewski worked on and
thus should get a pwrseq driver, which handles the enable GPIOs,
regulators and clocks. If any of them are missing the PCIe device
is not discovered. Note, that this will actually require describing
the PCIe device in DT to reference the pwrseq device.

Additionally the brcmfmac need to be extended to actually have
working wlan.

I put some links and notes about this here:

https://gitlab.collabora.com/hardware-enablement/rockchip-3588/linux/-/issues/1

Greetings,

-- Sebastian
Jacobe Zang June 18, 2024, 3:27 a.m. UTC | #5
> If your PCI bridge is functioning properly and if your WiFi adapter is
> connected and physically enabled (in terms of power and RFKILL) I
> believe it should be automatically discovered and you should see it in
> lspci.

I'm sure that wifi@0,0 node is not needed. 

> I'm not sure your pcie@0,0 node is needed either.

I think pcie@0,0 node is useful. I have tried to comment it, after that Wi-Fi works properly but bluetooth can't work.

---
Best Regards
Jacobe
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-khadas-edge2.dts b/arch/arm64/boot/dts/rockchip/rk3588s-khadas-edge2.dts
index 233bab17bffd2..7d7cc3e76838c 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s-khadas-edge2.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588s-khadas-edge2.dts
@@ -365,6 +365,23 @@  &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>;
+
+		wifi: wifi@0,0 {
+			compatible = "pci14e4,449d";
+			reg = <0x410000 0 0 0 0>;
+			clocks = <&hym8563>;
+			clock-names = "32k";
+		};
+	};
+
 };
 
 &pwm11 {