| Message ID | 20240624081906.1399447-2-jacobe.zang@wesion.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | Add AP6275P wireless support | expand |
On 24/06/2024 10:19, Jacobe Zang wrote: > Khadas Edge2 uses the PCI-e Ampak AP6275P 2T2R Wi-Fi 6 module. > > Co-developed-by: Muhammed Efe Cetin <efectn@protonmail.com> > Signed-off-by: Muhammed Efe Cetin <efectn@protonmail.com> > Signed-off-by: Jacobe Zang <jacobe.zang@wesion.com> > --- > .../boot/dts/rockchip/rk3588s-khadas-edge2.dts | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > 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? > + 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? > + }; > + }; > + No need for this blank line. > }; > > &pwm11 { Best regards, Krzysztof
>> 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 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? References: https://lore.kernel.org/all/20220629155956.1138955-15-nfraprado@collabora.com/ --- Best Regards Jacobe
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
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>; + + wifi: wifi@0,0 { + reg = <0x410000 0 0 0 0>; + clocks = <&hym8563>; + clock-names = "32k"; + }; + }; + }; &pwm11 {