On 2/25/2022 11:20 AM, Manikanta Pubbisetty wrote:
On 1/28/2022 3:46 PM, Kalle Valo wrote:
Manikanta Pubbisetty <quic_mpubbise@xxxxxxxxxxx> writes:
Unlike other ATH11K PCIe devices which are enumerated by APSS
processor (Application Processor SubSystem), WCN6750 gets
enumerated by the WPSS Q6 processor (Wireless Processor SubSystem);
In simple terms, though WCN6750 is PCIe device, it is not attached
to the APSS processor, APSS will not know of such a device being
present in the system and therefore WCN6750 will be registered as
a platform device to the kernel core like other supported AHB
devices.
WCN6750 uses both AHB and PCI APIs for it's operation, it uses
AHB APIs for device probe/boot and PCI APIs for device setup
and register accesses; Because of this nature, it is referred
as a hybrid bus device.
Refactor PCI code to support hybrid bus devices like WCN6750.
Tested-on: WCN6750 hw1.0 AHB WLAN.MSL.1.0.1-00573-QCAMSLSWPLZ-1
Tested-on: WCN6855 hw2.0 PCI
WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1
Tested-on: QCN9074 hw1.0 PCI WLAN.HK.2.5.0.1-01100-QCAHKSWPL_SILICONZ-1
Tested-on: IPQ8074 hw2.0 AHB WLAN.HK.2.4.0.1-00192-QCAHKSWPL_SILICONZ-1
Signed-off-by: Manikanta Pubbisetty <quic_mpubbise@xxxxxxxxxxx>
[...]
--- a/drivers/net/wireless/ath/ath11k/Makefile
+++ b/drivers/net/wireless/ath/ath11k/Makefile
@@ -29,7 +29,7 @@ obj-$(CONFIG_ATH11K_AHB) += ath11k_ahb.o
ath11k_ahb-y += ahb.o
obj-$(CONFIG_ATH11K_PCI) += ath11k_pci.o
-ath11k_pci-y += mhi.o pci.o
+ath11k_pci-y += mhi.o pci.o pci_cmn.o
So the end result looks like this:
obj-$(CONFIG_ATH11K_AHB) += ath11k_ahb.o
ath11k_ahb-y += ahb.o pci_cmn.o
obj-$(CONFIG_ATH11K_PCI) += ath11k_pci.o
ath11k_pci-y += mhi.o pci.o pci_cmn.o
Linking pci_cmn.o to both ath11k_pci.ko and ath11k_ahb.ko looks wrong.
Does that even compile if ath11k is linked to the kernel, eg. with
allyesconfig?
I did try compiling the kernel with allyesconfig after your comment,
compilation went through without any hiccups.
One way to solve is to link pci_cmn.o to ath11k.ko. But for another
approach, for a long time I have been thinking about what's the point to
have separate ath11k_pci.ko and ath11k_ahb.ko modules?,They are very
small anyway compared to ath11k.ko. So my ideais that should we have
just one ath11k.ko module, it contains all AHB and PCI code as well, and
ath11k_pci.ko and ath11k_ahb.ko would not be created anymore. It would
simplify things a bit, especially here.
Thoughts?
I see some concerns going with single module combining both AHB and PCI
modules into ath11k.ko
1) AHB and PCI drivers make use of completely different kernel
frameworks, for example AHB driver needs remoteproc APIs for booting and
require CONFIG_REMOTEPROC to be compiled in to the kernel. Similarly,
PCI driver needs MHI APIs and also dependent on CONFIG_PCI. Both MHI and
PCI bus frameworks need to be compiled for PCI to work. If we club all
of this into single module, I see that unnecessarily additional modules
will be compiled into the kernel which IMO is not so good idea.
2) Secondly, there is high chance of writing bad code all over the
driver. For example, there are chances that developers put AHB/PCI
specific code all over the driver creating a big mess.
Though this can be avoided with stringent code review, but why to
give the chance.
Though AHB and PCI drivers are smaller in size, IMHO let AHB and PCI be
independent drivers, code looks cleaner and properly segregated by
keeping them as it is today.
Regarding the compilation of PCI common code, shall we move it into
ath11k.ko? What is your opinion on this.
Hi Kalle,
Any thoughts about the idea proposed?
Thanks,
Manikanta
