* [PATCH RFC v2] mhi: Enable unique QRTR node ID support
@ 2021-02-26 9:19 Gokul Sriram Palanisamy
2021-02-26 9:19 ` [PATCH v2] bus: mhi: core: Add unique qrtr node id support Gokul Sriram Palanisamy
0 siblings, 1 reply; 12+ messages in thread
From: Gokul Sriram Palanisamy @ 2021-02-26 9:19 UTC (permalink / raw)
To: linux-arm-msm, inux-kernel
Cc: manivannan.sadhasivam, jhugo, hemantk, sricharan, gokulsri
On multi-mhi platforms, host WiFi driver and
QMI test driver needs to differntiate between
QMI packets received from multiple mhi devices.
With QCN9000 PCI cards, once SBL gets loaded, we
utilize ERRDBG2 register to write a unique value
per mhi device from device-tree that the device
utilizes to set a unique QRTR node ID and
instance ID for the QMI service. This helps QRTR
stack in differenting the packets in a multi-mhi
environment and can route them accordingly.
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.
Changes since v1:
- Addressed review comments by Jeffrey Hugo.
Gokul Sriram Palanisamy (1):
bus: mhi: core: Add unique qrtr node id support
drivers/bus/mhi/core/boot.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
--
2.7.4
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH v2] bus: mhi: core: Add unique qrtr node id support 2021-02-26 9:19 [PATCH RFC v2] mhi: Enable unique QRTR node ID support Gokul Sriram Palanisamy @ 2021-02-26 9:19 ` Gokul Sriram Palanisamy 0 siblings, 0 replies; 12+ messages in thread From: Gokul Sriram Palanisamy @ 2021-02-26 9:19 UTC (permalink / raw) To: linux-arm-msm, inux-kernel Cc: manivannan.sadhasivam, jhugo, hemantk, sricharan, gokulsri 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@codeaurora.org> --- 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); + write_lock_irq(&mhi_cntrl->pm_lock); mhi_cntrl->dev_state = MHI_STATE_RESET; write_unlock_irq(&mhi_cntrl->pm_lock); -- 2.7.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH RFC v2] mhi: Enable unique QRTR node ID support
@ 2021-02-26 10:42 Gokul Sriram Palanisamy
2021-02-26 10:42 ` [PATCH v2] bus: mhi: core: Add unique qrtr node id support Gokul Sriram Palanisamy
0 siblings, 1 reply; 12+ messages in thread
From: Gokul Sriram Palanisamy @ 2021-02-26 10:42 UTC (permalink / raw)
To: linux-arm-msm, linux-kernel
Cc: manivannan.sadhasivam, jhugo, hemantk, sricharan, gokulsri
On multi-mhi platforms, host WiFi driver and
QMI test driver needs to differntiate between
QMI packets received from multiple mhi devices.
With QCN9000 PCI cards, once SBL gets loaded, we
utilize ERRDBG2 register to write a unique value
per mhi device from device-tree that the device
utilizes to set a unique QRTR node ID and
instance ID for the QMI service. This helps QRTR
stack in differenting the packets in a multi-mhi
environment and can route them accordingly.
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.
Changes since v1:
- Addressed review comments by Jeffrey Hugo.
Gokul Sriram Palanisamy (1):
bus: mhi: core: Add unique qrtr node id support
drivers/bus/mhi/core/boot.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
--
2.7.4
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH v2] bus: mhi: core: Add unique qrtr node id support 2021-02-26 10:42 [PATCH RFC v2] mhi: Enable unique QRTR node ID support Gokul Sriram Palanisamy @ 2021-02-26 10:42 ` Gokul Sriram Palanisamy 2021-02-26 14:47 ` Jeffrey Hugo 2021-02-26 14:52 ` Manivannan Sadhasivam 0 siblings, 2 replies; 12+ messages in thread From: Gokul Sriram Palanisamy @ 2021-02-26 10:42 UTC (permalink / raw) To: linux-arm-msm, linux-kernel Cc: manivannan.sadhasivam, jhugo, hemantk, sricharan, gokulsri 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@codeaurora.org> --- 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); + write_lock_irq(&mhi_cntrl->pm_lock); mhi_cntrl->dev_state = MHI_STATE_RESET; write_unlock_irq(&mhi_cntrl->pm_lock); -- 2.7.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2] bus: mhi: core: Add unique qrtr node id support 2021-02-26 10:42 ` [PATCH v2] bus: mhi: core: Add unique qrtr node id support Gokul Sriram Palanisamy @ 2021-02-26 14:47 ` Jeffrey Hugo 2021-02-26 14:52 ` Manivannan Sadhasivam 1 sibling, 0 replies; 12+ messages in thread From: Jeffrey Hugo @ 2021-02-26 14:47 UTC (permalink / raw) To: Gokul Sriram Palanisamy, linux-arm-msm, linux-kernel Cc: manivannan.sadhasivam, hemantk, sricharan On 2/26/2021 3:42 AM, 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@codeaurora.org> > --- > 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); > + > write_lock_irq(&mhi_cntrl->pm_lock); > mhi_cntrl->dev_state = MHI_STATE_RESET; > write_unlock_irq(&mhi_cntrl->pm_lock); > NACK. Please see my comments on v1. -- Jeffrey Hugo Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] bus: mhi: core: Add unique qrtr node id support 2021-02-26 10:42 ` [PATCH v2] bus: mhi: core: Add unique qrtr node id support Gokul Sriram Palanisamy 2021-02-26 14:47 ` Jeffrey Hugo @ 2021-02-26 14:52 ` Manivannan Sadhasivam 2021-02-26 17:31 ` Bhaumik Bhatt 2021-03-01 11:14 ` Kalle Valo 1 sibling, 2 replies; 12+ messages in thread From: Manivannan Sadhasivam @ 2021-02-26 14:52 UTC (permalink / raw) To: Gokul Sriram Palanisamy Cc: linux-arm-msm, linux-kernel, jhugo, hemantk, sricharan 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@codeaurora.org> > --- > 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? Thanks, Mani > + > write_lock_irq(&mhi_cntrl->pm_lock); > mhi_cntrl->dev_state = MHI_STATE_RESET; > write_unlock_irq(&mhi_cntrl->pm_lock); > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] bus: mhi: core: Add unique qrtr node id support 2021-02-26 14:52 ` Manivannan Sadhasivam @ 2021-02-26 17:31 ` Bhaumik Bhatt 2021-02-27 8:25 ` gokulsri 2021-03-01 11:14 ` Kalle Valo 1 sibling, 1 reply; 12+ messages in thread From: Bhaumik Bhatt @ 2021-02-26 17:31 UTC (permalink / raw) To: Gokul Sriram Palanisamy Cc: Manivannan Sadhasivam, linux-arm-msm, linux-kernel, jhugo, hemantk, sricharan On 2021-02-26 06:52 AM, Manivannan Sadhasivam wrote: > 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@codeaurora.org> >> --- >> 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? > > Thanks, > Mani > >> + >> write_lock_irq(&mhi_cntrl->pm_lock); >> mhi_cntrl->dev_state = MHI_STATE_RESET; >> write_unlock_irq(&mhi_cntrl->pm_lock); >> -- >> 2.7.4 >> As others have stated, please refrain from adding protocol specific code (such as PCIe) in the MHI core driver. Please have this change in your controller. If there is access to BHI registers required prior to power up from MHI core, it is not exposed right now. We can talk about how you can achieve that, so you can do this write in your controller after mhi_prepare_for_power_up(). Thanks, Bhaumik --- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] bus: mhi: core: Add unique qrtr node id support 2021-02-26 17:31 ` Bhaumik Bhatt @ 2021-02-27 8:25 ` gokulsri 0 siblings, 0 replies; 12+ messages in thread From: gokulsri @ 2021-02-27 8:25 UTC (permalink / raw) To: bbhatt, Manivannan Sadhasivam, jhugo Cc: linux-arm-msm, linux-kernel, hemantk, sricharan, bjorn.andersson Hi On 2021-02-26 23:01, Bhaumik Bhatt wrote: > On 2021-02-26 06:52 AM, Manivannan Sadhasivam wrote: >> 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@codeaurora.org> >>> --- >>> 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? >> >> Thanks, >> Mani >> >>> + >>> write_lock_irq(&mhi_cntrl->pm_lock); >>> mhi_cntrl->dev_state = MHI_STATE_RESET; >>> write_unlock_irq(&mhi_cntrl->pm_lock); >>> -- >>> 2.7.4 >>> > > As others have stated, please refrain from adding protocol specific > code (such as PCIe) > in the MHI core driver. Please have this change in your controller. > > If there is access to BHI registers required prior to power up from > MHI core, it is not > exposed right now. We can talk about how you can achieve that, so you > can do this write > in your controller after mhi_prepare_for_power_up(). > > Thanks, > Bhaumik > --- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora > Forum, > a Linux Foundation Collaborative Project Thank you Jeffrey, Manivannan and Bhaumik. Adding Bjorn for his review and suggestions. Thanks, Gokul ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] bus: mhi: core: Add unique qrtr node id support 2021-02-26 14:52 ` Manivannan Sadhasivam 2021-02-26 17:31 ` Bhaumik Bhatt @ 2021-03-01 11:14 ` Kalle Valo 2021-03-01 18:17 ` Bhaumik Bhatt 1 sibling, 1 reply; 12+ messages in thread From: Kalle Valo @ 2021-03-01 11:14 UTC (permalink / raw) To: Manivannan Sadhasivam Cc: Gokul Sriram Palanisamy, linux-arm-msm, linux-kernel, jhugo, hemantk, sricharan, ath11k + ath11k list Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> 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@codeaurora.org> >> --- >> 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. -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] bus: mhi: core: Add unique qrtr node id support 2021-03-01 11:14 ` Kalle Valo @ 2021-03-01 18:17 ` Bhaumik Bhatt 2021-03-01 18:26 ` Kalle Valo 0 siblings, 1 reply; 12+ messages in thread From: Bhaumik Bhatt @ 2021-03-01 18:17 UTC (permalink / raw) To: Kalle Valo Cc: Manivannan Sadhasivam, Gokul Sriram Palanisamy, linux-arm-msm, linux-kernel, jhugo, hemantk, sricharan, ath11k, kvalo=codeaurora.org On 2021-03-01 03:14 AM, Kalle Valo wrote: > + ath11k list > > Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> 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@codeaurora.org> >>> --- >>> 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. Thanks, Bhaumik --- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] bus: mhi: core: Add unique qrtr node id support 2021-03-01 18:17 ` Bhaumik Bhatt @ 2021-03-01 18:26 ` Kalle Valo 2021-03-01 18:53 ` Bhaumik Bhatt 2021-03-01 18:56 ` Bhaumik Bhatt 0 siblings, 2 replies; 12+ messages in thread From: Kalle Valo @ 2021-03-01 18:26 UTC (permalink / raw) To: Bhaumik Bhatt Cc: jhugo, linux-arm-msm, linux-kernel, Gokul Sriram Palanisamy, Manivannan Sadhasivam, hemantk, sricharan, ath11k Bhaumik Bhatt <bbhatt@codeaurora.org> writes: > On 2021-03-01 03:14 AM, Kalle Valo wrote: >> + ath11k list >> >> Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> 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@codeaurora.org> >>>> --- >>>> 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] bus: mhi: core: Add unique qrtr node id support 2021-03-01 18:26 ` Kalle Valo @ 2021-03-01 18:53 ` Bhaumik Bhatt 2021-03-01 18:56 ` Bhaumik Bhatt 1 sibling, 0 replies; 12+ messages in thread From: Bhaumik Bhatt @ 2021-03-01 18:53 UTC (permalink / raw) To: Kalle Valo Cc: jhugo, linux-arm-msm, linux-kernel, Gokul Sriram Palanisamy, Manivannan Sadhasivam, hemantk, sricharan, ath11k, kvalo=codeaurora.org On 2021-03-01 10:26 AM, Kalle Valo wrote: > Bhaumik Bhatt <bbhatt@codeaurora.org> writes: > >> On 2021-03-01 03:14 AM, Kalle Valo wrote: >>> + ath11k list >>> >>> Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> 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@codeaurora.org> >>>>> --- >>>>> 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. Yes, I feel it would be better handled within ath11k. AFAIK, device (QCA/QCN) populates the BHI ERRDBG registers when it wants to communicate a certain problem/status to the host and it should not be used the other way round, where host writes a configuration cookie for the device to boot-up in a particular way. It feels hacky as of now unless an actual configuration register is used. As per BHI specification, these registers are permitted to be read-only for the host and Read/Write for device only. I also don't see any BHI configuration or general purpose registers that can be used to notify this cookie. If one is found, we can talk about how to use them and can introduce MHI patches for those. I suggest exploring alternatives to this. I think Hemant and are in agreement on this. Thanks, Bhaumik --- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] bus: mhi: core: Add unique qrtr node id support 2021-03-01 18:26 ` Kalle Valo 2021-03-01 18:53 ` Bhaumik Bhatt @ 2021-03-01 18:56 ` Bhaumik Bhatt 1 sibling, 0 replies; 12+ messages in thread From: Bhaumik Bhatt @ 2021-03-01 18:56 UTC (permalink / raw) To: Kalle Valo Cc: jhugo, linux-arm-msm, linux-kernel, Gokul Sriram Palanisamy, Manivannan Sadhasivam, hemantk, sricharan, ath11k Hi Kalle, On 2021-03-01 10:26 AM, Kalle Valo wrote: > Bhaumik Bhatt <bbhatt@codeaurora.org> writes: > >> On 2021-03-01 03:14 AM, Kalle Valo wrote: >>> + ath11k list >>> >>> Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> 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@codeaurora.org> >>>>> --- >>>>> 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. Yes, I feel it would be better handled within ath11k. AFAIK, device (QCA/QCN) populates the BHI ERRDBG registers when it wants to communicate a certain problem/status to the host and it should not be used the other way round, where host writes a configuration cookie for the device to boot-up in a particular way. It feels hacky as of now unless an actual configuration register is used. As per BHI specification, these registers are permitted to be read-only for the host and Read/Write for device only. I also don't see any BHI configuration or general purpose registers that can be used to notify this cookie. If one is found, we can talk about how to use them and can introduce MHI patches for those. I suggest exploring alternatives to this. I think Hemant and are in agreement on this. Thanks, Bhaumik --- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-03-01 19:00 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-02-26 9:19 [PATCH RFC v2] mhi: Enable unique QRTR node ID support Gokul Sriram Palanisamy 2021-02-26 9:19 ` [PATCH v2] bus: mhi: core: Add unique qrtr node id support Gokul Sriram Palanisamy 2021-02-26 10:42 [PATCH RFC v2] mhi: Enable unique QRTR node ID support Gokul Sriram Palanisamy 2021-02-26 10:42 ` [PATCH v2] bus: mhi: core: Add unique qrtr node id support Gokul Sriram Palanisamy 2021-02-26 14:47 ` Jeffrey Hugo 2021-02-26 14:52 ` Manivannan Sadhasivam 2021-02-26 17:31 ` Bhaumik Bhatt 2021-02-27 8:25 ` gokulsri 2021-03-01 11:14 ` Kalle Valo 2021-03-01 18:17 ` Bhaumik Bhatt 2021-03-01 18:26 ` Kalle Valo 2021-03-01 18:53 ` Bhaumik Bhatt 2021-03-01 18:56 ` Bhaumik Bhatt
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).