Bhaumik Bhatt <bbhatt@xxxxxxxxxxxxxx> writes: > On 2021-03-01 03:14 AM, Kalle Valo wrote: >> + ath11k list >> >> Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx> writes: >> >>> On Fri, Feb 26, 2021 at 04:12:49PM +0530, Gokul Sriram Palanisamy >>> wrote: >>>> On platforms with two or more identical mhi >>>> devices, qmi service will run with identical >>>> qrtr-node-id. Because of this identical ID, >>>> host qrtr-lookup cannot register more than one >>>> qmi service with identical node ID. Ultimately, >>>> only one qmi service will be avilable for the >>>> underlying drivers to communicate with. >>>> >>>> On QCN9000, it implements a unique qrtr-node-id >>>> and qmi instance ID using a unique instance ID >>>> written to a debug register from host driver >>>> soon after SBL is loaded. >>>> >>>> This change generates a unique instance ID from >>>> PCIe domain number and bus number, writes to the >>>> given debug register just after SBL is loaded so >>>> that it is available for FW when the QMI service >>>> is spawned. >>>> >>>> sample: >>>> root@OpenWrt:/# qrtr-lookup >>>> Service Version Instance Node Port >>>> 15 1 0 8 1 Test service >>>> 69 1 8 8 2 ATH10k WLAN firmware service >>>> 15 1 0 24 1 Test service >>>> 69 1 24 24 2 ATH10k WLAN firmware service >>>> >>>> Here 8 and 24 on column 3 (QMI Instance ID) >>>> and 4 (QRTR Node ID) are the node IDs that >>>> is unique per mhi device. >>>> >>>> Signed-off-by: Gokul Sriram Palanisamy <gokulsri@xxxxxxxxxxxxxx> >>>> --- >>>> drivers/bus/mhi/core/boot.c | 14 ++++++++++++++ >>>> 1 file changed, 14 insertions(+) >>>> >>>> diff --git a/drivers/bus/mhi/core/boot.c >>>> b/drivers/bus/mhi/core/boot.c >>>> index c2546bf..5e5dad5 100644 >>>> --- a/drivers/bus/mhi/core/boot.c >>>> +++ b/drivers/bus/mhi/core/boot.c >>>> @@ -16,8 +16,12 @@ >>>> #include <linux/random.h> >>>> #include <linux/slab.h> >>>> #include <linux/wait.h> >>>> +#include <linux/pci.h> >>>> #include "internal.h" >>>> >>>> +#define QRTR_INSTANCE_MASK 0x000000FF >>>> +#define QRTR_INSTANCE_SHIFT 0 >>>> + >>>> /* Setup RDDM vector table for RDDM transfer and program RXVEC */ >>>> void mhi_rddm_prepare(struct mhi_controller *mhi_cntrl, >>>> struct image_info *img_info) >>>> @@ -391,6 +395,9 @@ void mhi_fw_load_handler(struct mhi_controller >>>> *mhi_cntrl) >>>> const struct firmware *firmware = NULL; >>>> struct image_info *image_info; >>>> struct device *dev = &mhi_cntrl->mhi_dev->dev; >>>> + struct pci_dev *pci_dev = to_pci_dev(mhi_cntrl->cntrl_dev); >>>> + struct pci_bus *bus = pci_dev->bus; >>>> + uint32_t instance; >>>> const char *fw_name; >>>> void *buf; >>>> dma_addr_t dma_addr; >>>> @@ -466,6 +473,13 @@ void mhi_fw_load_handler(struct >>>> mhi_controller *mhi_cntrl) >>>> return; >>>> } >>>> >>>> + instance = ((pci_domain_nr(bus) & 0xF) << 4) | (bus->number & 0xF); >>>> + instance &= QRTR_INSTANCE_MASK; >>>> + >>>> + mhi_write_reg_field(mhi_cntrl, mhi_cntrl->bhi, >>>> + BHI_ERRDBG2, QRTR_INSTANCE_MASK, >>>> + QRTR_INSTANCE_SHIFT, instance); >>> >>> You cannot not do this in MHI stack. Why can't you do this in the >>> MHI controller >>> specific to QCN9000? And btw, is QCN9000 supported in mainline? >> >> I'm not sure what QCN9000 means but I'm guessing it's QCN9074. We have >> initial QCN9074 support in ath11k but there are some issues still so >> it's not enabled by default (yet): >> >> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=ath-next&id=4e80946197a83a6115e308334618449b77696d6a >> >> And I suspect we have this same qrtr issue with any ath11k PCI device, >> including QCA6390, so this is not a QCN9074 specific problem. >> >> BTW Gokul, please always CC the ath11k list when submitting patches >> which are related to ath11k. > > QRTR sits on top of MHI so shouldn't this be handled outside of MHI > after MHI is operational? We cannot allow PCI code in MHI core driver > but this can be handled pre or post MHI power-up in whatever way you > desire that does not have to directly involve MHI. Sure, makes sense. I was just replying to Mani's question about status of QCN9000 upstream support. So should we handle this within ath11k, is that the right approach? I also suspect that for QCN9074 and QCA6390 we have to do this a bit differently, so it would be easier to handle the differences between devices (and firmware versions) inside ath11k. -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
