[PATCH] mhi: pci_generic: add support for sc8280xp-crd SDX55 variant

[PATCH] mhi: pci_generic: add support for sc8280xp-crd SDX55 variant

[Johan Hovold]

The SC8280XP Compute Reference Design (CRD) has an on-PCB SDX55 modem
which uses MBIM.

The exact channel configuration is not known but the Foxconn SDX55
configuration allows the modem to be used so reuse that one for now.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/bus/mhi/host/pci_generic.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
index caa4ce28cf9e..f9f4345d472d 100644
--- a/drivers/bus/mhi/host/pci_generic.c
+++ b/drivers/bus/mhi/host/pci_generic.c
@@ -542,6 +542,9 @@ static const struct mhi_pci_dev_info mhi_telit_fn990_info = {
 static const struct pci_device_id mhi_pci_id_table[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x0304),
 		.driver_data = (kernel_ulong_t) &mhi_qcom_sdx24_info },
+	/* SC8280XP-CRD (sdx55) */
+	{ PCI_DEVICE_SUB(PCI_VENDOR_ID_QCOM, 0x0306, PCI_VENDOR_ID_QCOM, 0x010c),
+		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx55_info },
 	/* EM919x (sdx55), use the same vid:pid as qcom-sdx55m */
 	{ PCI_DEVICE_SUB(PCI_VENDOR_ID_QCOM, 0x0306, 0x18d7, 0x0200),
 		.driver_data = (kernel_ulong_t) &mhi_sierra_em919x_info },
-- 
2.37.3

Re: [PATCH] mhi: pci_generic: add support for sc8280xp-crd SDX55 variant

[Manivannan Sadhasivam]

On Mon, Oct 31, 2022 at 09:34:29AM +0100, Johan Hovold wrote:
> The SC8280XP Compute Reference Design (CRD) has an on-PCB SDX55 modem
> which uses MBIM.
> 
> The exact channel configuration is not known but the Foxconn SDX55
> configuration allows the modem to be used so reuse that one for now.
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  drivers/bus/mhi/host/pci_generic.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
> index caa4ce28cf9e..f9f4345d472d 100644
> --- a/drivers/bus/mhi/host/pci_generic.c
> +++ b/drivers/bus/mhi/host/pci_generic.c
> @@ -542,6 +542,9 @@ static const struct mhi_pci_dev_info mhi_telit_fn990_info = {
>  static const struct pci_device_id mhi_pci_id_table[] = {
>  	{ PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x0304),
>  		.driver_data = (kernel_ulong_t) &mhi_qcom_sdx24_info },
> +	/* SC8280XP-CRD (sdx55) */

Platform device name is not needed.

> +	{ PCI_DEVICE_SUB(PCI_VENDOR_ID_QCOM, 0x0306, PCI_VENDOR_ID_QCOM, 0x010c),

Sub ID looks wierd. Did you test with mhi_qcom_sdx55_info? If it didn't
work, what was the issue?

Thanks,
Mani

> +		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx55_info },
>  	/* EM919x (sdx55), use the same vid:pid as qcom-sdx55m */
>  	{ PCI_DEVICE_SUB(PCI_VENDOR_ID_QCOM, 0x0306, 0x18d7, 0x0200),
>  		.driver_data = (kernel_ulong_t) &mhi_sierra_em919x_info },
> -- 
> 2.37.3
> 
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH] mhi: pci_generic: add support for sc8280xp-crd SDX55 variant

[Johan Hovold]

On Tue, Nov 01, 2022 at 05:12:58PM +0530, Manivannan Sadhasivam wrote:
> On Mon, Oct 31, 2022 at 09:34:29AM +0100, Johan Hovold wrote:
> > The SC8280XP Compute Reference Design (CRD) has an on-PCB SDX55 modem
> > which uses MBIM.
> > 
> > The exact channel configuration is not known but the Foxconn SDX55
> > configuration allows the modem to be used so reuse that one for now.
> > 
> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> > ---
> >  drivers/bus/mhi/host/pci_generic.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
> > index caa4ce28cf9e..f9f4345d472d 100644
> > --- a/drivers/bus/mhi/host/pci_generic.c
> > +++ b/drivers/bus/mhi/host/pci_generic.c
> > @@ -542,6 +542,9 @@ static const struct mhi_pci_dev_info mhi_telit_fn990_info = {
> >  static const struct pci_device_id mhi_pci_id_table[] = {
> >  	{ PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x0304),
> >  		.driver_data = (kernel_ulong_t) &mhi_qcom_sdx24_info },
> > +	/* SC8280XP-CRD (sdx55) */
> 
> Platform device name is not needed.

Ok, only added it because the other entries had a description.
 
> > +	{ PCI_DEVICE_SUB(PCI_VENDOR_ID_QCOM, 0x0306, PCI_VENDOR_ID_QCOM, 0x010c),
> 
> Sub ID looks wierd. Did you test with mhi_qcom_sdx55_info? If it didn't
> work, what was the issue?

I obviously added the new entry because the default match did not work.

I haven't had time to look at this since before the summer, but based on
my notes and a quick test now it seems that the QMI channel is not
supported and ModemManager takes forever to enumerate with a bunch of
failed transactions every second:

  ModemManager[469]: [/dev/wwan0qmi0] Checking version info (45 retries)...
  ...
  ModemManager[469]: transaction 0x2 aborted, but message is not abortable
  ...
  ModemManager[469]: transaction 0x2e aborted, but message is not abortable
  ...
  ModemManager[469]: <warn>  [plugin/generic] could not grab port wwan0qmi0: Cannot add port 'wwan/wwan0qmi0', unhandled port type

According to my notes I could connect to the provider after the (~45 s)
timeout, but no data was coming through with the default config which I
attributed to the differences in the data channel configuration (i.e.
"IP_HW0" and "IP_HW0_MBIM" are handled by different drivers).
 
> > +		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx55_info },
> >  	/* EM919x (sdx55), use the same vid:pid as qcom-sdx55m */
> >  	{ PCI_DEVICE_SUB(PCI_VENDOR_ID_QCOM, 0x0306, 0x18d7, 0x0200),
> >  		.driver_data = (kernel_ulong_t) &mhi_sierra_em919x_info },
> > -- 
> > 2.37.3

Johan

Re: [PATCH] mhi: pci_generic: add support for sc8280xp-crd SDX55 variant

[Manivannan Sadhasivam]

On Wed, Nov 02, 2022 at 02:00:47PM +0100, Johan Hovold wrote:
> On Tue, Nov 01, 2022 at 05:12:58PM +0530, Manivannan Sadhasivam wrote:
> > On Mon, Oct 31, 2022 at 09:34:29AM +0100, Johan Hovold wrote:
> > > The SC8280XP Compute Reference Design (CRD) has an on-PCB SDX55 modem
> > > which uses MBIM.
> > > 
> > > The exact channel configuration is not known but the Foxconn SDX55
> > > configuration allows the modem to be used so reuse that one for now.
> > > 
> > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> > > ---
> > >  drivers/bus/mhi/host/pci_generic.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
> > > index caa4ce28cf9e..f9f4345d472d 100644
> > > --- a/drivers/bus/mhi/host/pci_generic.c
> > > +++ b/drivers/bus/mhi/host/pci_generic.c
> > > @@ -542,6 +542,9 @@ static const struct mhi_pci_dev_info mhi_telit_fn990_info = {
> > >  static const struct pci_device_id mhi_pci_id_table[] = {
> > >  	{ PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x0304),
> > >  		.driver_data = (kernel_ulong_t) &mhi_qcom_sdx24_info },
> > > +	/* SC8280XP-CRD (sdx55) */
> > 
> > Platform device name is not needed.
> 
> Ok, only added it because the other entries had a description.
>  

Those descriptions are for the modems itself, like Telit FN980m. But
SC8280XP-CRD is the platform that makes use of *some* sdx55 based modem. So it
is not required.

> > > +	{ PCI_DEVICE_SUB(PCI_VENDOR_ID_QCOM, 0x0306, PCI_VENDOR_ID_QCOM, 0x010c),
> > 
> > Sub ID looks wierd. Did you test with mhi_qcom_sdx55_info? If it didn't
> > work, what was the issue?
> 
> I obviously added the new entry because the default match did not work.
> 
> I haven't had time to look at this since before the summer, but based on
> my notes and a quick test now it seems that the QMI channel is not
> supported and ModemManager takes forever to enumerate with a bunch of
> failed transactions every second:
> 
>   ModemManager[469]: [/dev/wwan0qmi0] Checking version info (45 retries)...
>   ...
>   ModemManager[469]: transaction 0x2 aborted, but message is not abortable
>   ...
>   ModemManager[469]: transaction 0x2e aborted, but message is not abortable
>   ...
>   ModemManager[469]: <warn>  [plugin/generic] could not grab port wwan0qmi0: Cannot add port 'wwan/wwan0qmi0', unhandled port type
> 
> According to my notes I could connect to the provider after the (~45 s)
> timeout, but no data was coming through with the default config which I
> attributed to the differences in the data channel configuration (i.e.
> "IP_HW0" and "IP_HW0_MBIM" are handled by different drivers).
>  

Okay, this makes it clear.

Thanks,
Mani

> > > +		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx55_info },
> > >  	/* EM919x (sdx55), use the same vid:pid as qcom-sdx55m */
> > >  	{ PCI_DEVICE_SUB(PCI_VENDOR_ID_QCOM, 0x0306, 0x18d7, 0x0200),
> > >  		.driver_data = (kernel_ulong_t) &mhi_sierra_em919x_info },
> > > -- 
> > > 2.37.3
> 
> Johan
> 

-- 
மணிவண்ணன் சதாசிவம்