[v2] bus: mhi: host: pci_generic: Use unique 'mhi_pci_dev_info' for product families

[PATCH v2] bus: mhi: host: pci_generic: Use unique 'mhi_pci_dev_info' for product families

Posted by Manivannan Sadhasivam 1 year, 5 months ago

Currently, a single 'mhi_pci_dev_info' is shared across different product
families. Even though it makes the device functional, it misleads the users
by sharing the common product name.

For instance, below message will be printed for Foxconn SDX62 modem during
boot:

"MHI PCI device found: foxconn-sdx65"

But this is quite misleading to the users since the actual modem plugged in
could be 'T99W373' which is based on SDX62.

So fix this issue by using a unique 'mhi_pci_dev_info' for product
families. This allows us to specify a unique product name for each product
family. Also, once this name is exposed to client drivers, they may use
this name to identify the modems and use any modem specific configuration.

Modems of unknown product families are not impacted by this change.

CC: Slark Xiao <slark_xiao@163.com>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---

Changes in v2:

* Used 'mhi_foxconn_t99w175_info' for HP variant as well

 drivers/bus/mhi/host/pci_generic.c | 78 ++++++++++++++++++++++--------
 1 file changed, 59 insertions(+), 19 deletions(-)

diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
index 08844ee79654..35ae7cd0711f 100644
--- a/drivers/bus/mhi/host/pci_generic.c
+++ b/drivers/bus/mhi/host/pci_generic.c
@@ -419,8 +419,10 @@ static const struct mhi_controller_config modem_foxconn_sdx55_config = {
 	.event_cfg = mhi_foxconn_sdx55_events,
 };
 
-static const struct mhi_pci_dev_info mhi_foxconn_sdx24_info = {
-	.name = "foxconn-sdx24",
+static const struct mhi_pci_dev_info mhi_foxconn_sdx55_info = {
+	.name = "foxconn-sdx55",
+	.fw = "qcom/sdx55m/sbl1.mbn",
+	.edl = "qcom/sdx55m/edl.mbn",
 	.config = &modem_foxconn_sdx55_config,
 	.bar_num = MHI_PCI_DEFAULT_BAR_NUM,
 	.dma_data_width = 32,
@@ -428,8 +430,8 @@ static const struct mhi_pci_dev_info mhi_foxconn_sdx24_info = {
 	.sideband_wake = false,
 };
 
-static const struct mhi_pci_dev_info mhi_foxconn_sdx55_info = {
-	.name = "foxconn-sdx55",
+static const struct mhi_pci_dev_info mhi_foxconn_t99w175_info = {
+	.name = "foxconn-t99w175",
 	.fw = "qcom/sdx55m/sbl1.mbn",
 	.edl = "qcom/sdx55m/edl.mbn",
 	.config = &modem_foxconn_sdx55_config,
@@ -439,8 +441,46 @@ static const struct mhi_pci_dev_info mhi_foxconn_sdx55_info = {
 	.sideband_wake = false,
 };
 
-static const struct mhi_pci_dev_info mhi_foxconn_sdx65_info = {
-	.name = "foxconn-sdx65",
+static const struct mhi_pci_dev_info mhi_foxconn_dw5930e_info = {
+	.name = "foxconn-dw5930e",
+	.fw = "qcom/sdx55m/sbl1.mbn",
+	.edl = "qcom/sdx55m/edl.mbn",
+	.config = &modem_foxconn_sdx55_config,
+	.bar_num = MHI_PCI_DEFAULT_BAR_NUM,
+	.dma_data_width = 32,
+	.mru_default = 32768,
+	.sideband_wake = false,
+};
+
+static const struct mhi_pci_dev_info mhi_foxconn_t99w368_info = {
+	.name = "foxconn-t99w368",
+	.config = &modem_foxconn_sdx55_config,
+	.bar_num = MHI_PCI_DEFAULT_BAR_NUM,
+	.dma_data_width = 32,
+	.mru_default = 32768,
+	.sideband_wake = false,
+};
+
+static const struct mhi_pci_dev_info mhi_foxconn_t99w373_info = {
+	.name = "foxconn-t99w373",
+	.config = &modem_foxconn_sdx55_config,
+	.bar_num = MHI_PCI_DEFAULT_BAR_NUM,
+	.dma_data_width = 32,
+	.mru_default = 32768,
+	.sideband_wake = false,
+};
+
+static const struct mhi_pci_dev_info mhi_foxconn_t99w510_info = {
+	.name = "foxconn-t99w510",
+	.config = &modem_foxconn_sdx55_config,
+	.bar_num = MHI_PCI_DEFAULT_BAR_NUM,
+	.dma_data_width = 32,
+	.mru_default = 32768,
+	.sideband_wake = false,
+};
+
+static const struct mhi_pci_dev_info mhi_foxconn_dw5932e_info = {
+	.name = "foxconn-dw5932e",
 	.config = &modem_foxconn_sdx55_config,
 	.bar_num = MHI_PCI_DEFAULT_BAR_NUM,
 	.dma_data_width = 32,
@@ -646,40 +686,40 @@ static const struct pci_device_id mhi_pci_id_table[] = {
 		.driver_data = (kernel_ulong_t) &mhi_quectel_em1xx_info },
 	/* T99W175 (sdx55), Both for eSIM and Non-eSIM */
 	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0ab),
-		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx55_info },
+		.driver_data = (kernel_ulong_t) &mhi_foxconn_t99w175_info },
 	/* DW5930e (sdx55), With eSIM, It's also T99W175 */
 	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0b0),
-		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx55_info },
+		.driver_data = (kernel_ulong_t) &mhi_foxconn_dw5930e_info },
 	/* DW5930e (sdx55), Non-eSIM, It's also T99W175 */
 	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0b1),
-		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx55_info },
+		.driver_data = (kernel_ulong_t) &mhi_foxconn_dw5930e_info },
 	/* T99W175 (sdx55), Based on Qualcomm new baseline */
 	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0bf),
-		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx55_info },
+		.driver_data = (kernel_ulong_t) &mhi_foxconn_t99w175_info },
 	/* T99W175 (sdx55) */
 	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0c3),
-		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx55_info },
+		.driver_data = (kernel_ulong_t) &mhi_foxconn_t99w175_info },
 	/* T99W368 (sdx65) */
 	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0d8),
-		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx65_info },
+		.driver_data = (kernel_ulong_t) &mhi_foxconn_t99w368_info },
 	/* T99W373 (sdx62) */
 	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0d9),
-		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx65_info },
+		.driver_data = (kernel_ulong_t) &mhi_foxconn_t99w373_info },
 	/* T99W510 (sdx24), variant 1 */
 	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0f0),
-		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx24_info },
+		.driver_data = (kernel_ulong_t) &mhi_foxconn_t99w510_info },
 	/* T99W510 (sdx24), variant 2 */
 	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0f1),
-		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx24_info },
+		.driver_data = (kernel_ulong_t) &mhi_foxconn_t99w510_info },
 	/* T99W510 (sdx24), variant 3 */
 	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0f2),
-		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx24_info },
+		.driver_data = (kernel_ulong_t) &mhi_foxconn_t99w510_info },
 	/* DW5932e-eSIM (sdx62), With eSIM */
 	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0f5),
-		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx65_info },
+		.driver_data = (kernel_ulong_t) &mhi_foxconn_dw5932e_info },
 	/* DW5932e (sdx62), Non-eSIM */
 	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0f9),
-		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx65_info },
+		.driver_data = (kernel_ulong_t) &mhi_foxconn_dw5932e_info },
 	/* MV31-W (Cinterion) */
 	{ PCI_DEVICE(PCI_VENDOR_ID_THALES, 0x00b3),
 		.driver_data = (kernel_ulong_t) &mhi_mv31_info },
@@ -694,7 +734,7 @@ static const struct pci_device_id mhi_pci_id_table[] = {
 		.driver_data = (kernel_ulong_t) &mhi_mv32_info },
 	/* T99W175 (sdx55), HP variant */
 	{ PCI_DEVICE(0x03f0, 0x0a6c),
-		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx55_info },
+		.driver_data = (kernel_ulong_t) &mhi_foxconn_t99w175_info },
 	{  }
 };
 MODULE_DEVICE_TABLE(pci, mhi_pci_id_table);
-- 
2.25.1

Re: [PATCH v2] bus: mhi: host: pci_generic: Use unique 'mhi_pci_dev_info' for product families

Posted by Manivannan Sadhasivam 1 year, 5 months ago

On Wed, Jun 26, 2024 at 11:02:37AM +0530, Manivannan Sadhasivam wrote:
> Currently, a single 'mhi_pci_dev_info' is shared across different product
> families. Even though it makes the device functional, it misleads the users
> by sharing the common product name.
> 
> For instance, below message will be printed for Foxconn SDX62 modem during
> boot:
> 
> "MHI PCI device found: foxconn-sdx65"
> 
> But this is quite misleading to the users since the actual modem plugged in
> could be 'T99W373' which is based on SDX62.
> 
> So fix this issue by using a unique 'mhi_pci_dev_info' for product
> families. This allows us to specify a unique product name for each product
> family. Also, once this name is exposed to client drivers, they may use
> this name to identify the modems and use any modem specific configuration.
> 
> Modems of unknown product families are not impacted by this change.
> 
> CC: Slark Xiao <slark_xiao@163.com>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Applied to mhi-next!

- Mani

> ---
> 
> Changes in v2:
> 
> * Used 'mhi_foxconn_t99w175_info' for HP variant as well
> 
>  drivers/bus/mhi/host/pci_generic.c | 78 ++++++++++++++++++++++--------
>  1 file changed, 59 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
> index 08844ee79654..35ae7cd0711f 100644
> --- a/drivers/bus/mhi/host/pci_generic.c
> +++ b/drivers/bus/mhi/host/pci_generic.c
> @@ -419,8 +419,10 @@ static const struct mhi_controller_config modem_foxconn_sdx55_config = {
>  	.event_cfg = mhi_foxconn_sdx55_events,
>  };
>  
> -static const struct mhi_pci_dev_info mhi_foxconn_sdx24_info = {
> -	.name = "foxconn-sdx24",
> +static const struct mhi_pci_dev_info mhi_foxconn_sdx55_info = {
> +	.name = "foxconn-sdx55",
> +	.fw = "qcom/sdx55m/sbl1.mbn",
> +	.edl = "qcom/sdx55m/edl.mbn",
>  	.config = &modem_foxconn_sdx55_config,
>  	.bar_num = MHI_PCI_DEFAULT_BAR_NUM,
>  	.dma_data_width = 32,
> @@ -428,8 +430,8 @@ static const struct mhi_pci_dev_info mhi_foxconn_sdx24_info = {
>  	.sideband_wake = false,
>  };
>  
> -static const struct mhi_pci_dev_info mhi_foxconn_sdx55_info = {
> -	.name = "foxconn-sdx55",
> +static const struct mhi_pci_dev_info mhi_foxconn_t99w175_info = {
> +	.name = "foxconn-t99w175",
>  	.fw = "qcom/sdx55m/sbl1.mbn",
>  	.edl = "qcom/sdx55m/edl.mbn",
>  	.config = &modem_foxconn_sdx55_config,
> @@ -439,8 +441,46 @@ static const struct mhi_pci_dev_info mhi_foxconn_sdx55_info = {
>  	.sideband_wake = false,
>  };
>  
> -static const struct mhi_pci_dev_info mhi_foxconn_sdx65_info = {
> -	.name = "foxconn-sdx65",
> +static const struct mhi_pci_dev_info mhi_foxconn_dw5930e_info = {
> +	.name = "foxconn-dw5930e",
> +	.fw = "qcom/sdx55m/sbl1.mbn",
> +	.edl = "qcom/sdx55m/edl.mbn",
> +	.config = &modem_foxconn_sdx55_config,
> +	.bar_num = MHI_PCI_DEFAULT_BAR_NUM,
> +	.dma_data_width = 32,
> +	.mru_default = 32768,
> +	.sideband_wake = false,
> +};
> +
> +static const struct mhi_pci_dev_info mhi_foxconn_t99w368_info = {
> +	.name = "foxconn-t99w368",
> +	.config = &modem_foxconn_sdx55_config,
> +	.bar_num = MHI_PCI_DEFAULT_BAR_NUM,
> +	.dma_data_width = 32,
> +	.mru_default = 32768,
> +	.sideband_wake = false,
> +};
> +
> +static const struct mhi_pci_dev_info mhi_foxconn_t99w373_info = {
> +	.name = "foxconn-t99w373",
> +	.config = &modem_foxconn_sdx55_config,
> +	.bar_num = MHI_PCI_DEFAULT_BAR_NUM,
> +	.dma_data_width = 32,
> +	.mru_default = 32768,
> +	.sideband_wake = false,
> +};
> +
> +static const struct mhi_pci_dev_info mhi_foxconn_t99w510_info = {
> +	.name = "foxconn-t99w510",
> +	.config = &modem_foxconn_sdx55_config,
> +	.bar_num = MHI_PCI_DEFAULT_BAR_NUM,
> +	.dma_data_width = 32,
> +	.mru_default = 32768,
> +	.sideband_wake = false,
> +};
> +
> +static const struct mhi_pci_dev_info mhi_foxconn_dw5932e_info = {
> +	.name = "foxconn-dw5932e",
>  	.config = &modem_foxconn_sdx55_config,
>  	.bar_num = MHI_PCI_DEFAULT_BAR_NUM,
>  	.dma_data_width = 32,
> @@ -646,40 +686,40 @@ static const struct pci_device_id mhi_pci_id_table[] = {
>  		.driver_data = (kernel_ulong_t) &mhi_quectel_em1xx_info },
>  	/* T99W175 (sdx55), Both for eSIM and Non-eSIM */
>  	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0ab),
> -		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx55_info },
> +		.driver_data = (kernel_ulong_t) &mhi_foxconn_t99w175_info },
>  	/* DW5930e (sdx55), With eSIM, It's also T99W175 */
>  	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0b0),
> -		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx55_info },
> +		.driver_data = (kernel_ulong_t) &mhi_foxconn_dw5930e_info },
>  	/* DW5930e (sdx55), Non-eSIM, It's also T99W175 */
>  	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0b1),
> -		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx55_info },
> +		.driver_data = (kernel_ulong_t) &mhi_foxconn_dw5930e_info },
>  	/* T99W175 (sdx55), Based on Qualcomm new baseline */
>  	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0bf),
> -		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx55_info },
> +		.driver_data = (kernel_ulong_t) &mhi_foxconn_t99w175_info },
>  	/* T99W175 (sdx55) */
>  	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0c3),
> -		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx55_info },
> +		.driver_data = (kernel_ulong_t) &mhi_foxconn_t99w175_info },
>  	/* T99W368 (sdx65) */
>  	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0d8),
> -		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx65_info },
> +		.driver_data = (kernel_ulong_t) &mhi_foxconn_t99w368_info },
>  	/* T99W373 (sdx62) */
>  	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0d9),
> -		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx65_info },
> +		.driver_data = (kernel_ulong_t) &mhi_foxconn_t99w373_info },
>  	/* T99W510 (sdx24), variant 1 */
>  	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0f0),
> -		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx24_info },
> +		.driver_data = (kernel_ulong_t) &mhi_foxconn_t99w510_info },
>  	/* T99W510 (sdx24), variant 2 */
>  	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0f1),
> -		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx24_info },
> +		.driver_data = (kernel_ulong_t) &mhi_foxconn_t99w510_info },
>  	/* T99W510 (sdx24), variant 3 */
>  	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0f2),
> -		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx24_info },
> +		.driver_data = (kernel_ulong_t) &mhi_foxconn_t99w510_info },
>  	/* DW5932e-eSIM (sdx62), With eSIM */
>  	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0f5),
> -		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx65_info },
> +		.driver_data = (kernel_ulong_t) &mhi_foxconn_dw5932e_info },
>  	/* DW5932e (sdx62), Non-eSIM */
>  	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0f9),
> -		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx65_info },
> +		.driver_data = (kernel_ulong_t) &mhi_foxconn_dw5932e_info },
>  	/* MV31-W (Cinterion) */
>  	{ PCI_DEVICE(PCI_VENDOR_ID_THALES, 0x00b3),
>  		.driver_data = (kernel_ulong_t) &mhi_mv31_info },
> @@ -694,7 +734,7 @@ static const struct pci_device_id mhi_pci_id_table[] = {
>  		.driver_data = (kernel_ulong_t) &mhi_mv32_info },
>  	/* T99W175 (sdx55), HP variant */
>  	{ PCI_DEVICE(0x03f0, 0x0a6c),
> -		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx55_info },
> +		.driver_data = (kernel_ulong_t) &mhi_foxconn_t99w175_info },
>  	{  }
>  };
>  MODULE_DEVICE_TABLE(pci, mhi_pci_id_table);
> -- 
> 2.25.1
> 

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

Re:[PATCH v2] bus: mhi: host: pci_generic: Use unique 'mhi_pci_dev_info' for product families

Posted by Slark Xiao 1 year, 5 months ago

At 2024-06-26 13:32:37, "Manivannan Sadhasivam" <manivannan.sadhasivam@linaro.org> wrote:
>Currently, a single 'mhi_pci_dev_info' is shared across different product
>families. Even though it makes the device functional, it misleads the users
>by sharing the common product name.
>
>For instance, below message will be printed for Foxconn SDX62 modem during
>boot:
>
>"MHI PCI device found: foxconn-sdx65"
>
>But this is quite misleading to the users since the actual modem plugged in
>could be 'T99W373' which is based on SDX62.
>
>So fix this issue by using a unique 'mhi_pci_dev_info' for product
>families. This allows us to specify a unique product name for each product
>family. Also, once this name is exposed to client drivers, they may use
>this name to identify the modems and use any modem specific configuration.
>
>Modems of unknown product families are not impacted by this change.
>
>CC: Slark Xiao <slark_xiao@163.com>
>Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Reviewed-by: Slark Xiao <slark_xiao@163.com>

>---
>
>Changes in v2:
>
>* Used 'mhi_foxconn_t99w175_info' for HP variant as well
>
> drivers/bus/mhi/host/pci_generic.c | 78 ++++++++++++++++++++++--------
> 1 file changed, 59 insertions(+), 19 deletions(-)
>
>diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
>index 08844ee79654..35ae7cd0711f 100644
>--- a/drivers/bus/mhi/host/pci_generic.c
>+++ b/drivers/bus/mhi/host/pci_generic.c
>@@ -419,8 +419,10 @@ static const struct mhi_controller_config modem_foxconn_sdx55_config = {
> 	.event_cfg = mhi_foxconn_sdx55_events,
> };
> 
>-static const struct mhi_pci_dev_info mhi_foxconn_sdx24_info = {
>-	.name = "foxconn-sdx24",
>+static const struct mhi_pci_dev_info mhi_foxconn_sdx55_info = {
>+	.name = "foxconn-sdx55",
>+	.fw = "qcom/sdx55m/sbl1.mbn",
>+	.edl = "qcom/sdx55m/edl.mbn",
> 	.config = &modem_foxconn_sdx55_config,
> 	.bar_num = MHI_PCI_DEFAULT_BAR_NUM,
> 	.dma_data_width = 32,
>@@ -428,8 +430,8 @@ static const struct mhi_pci_dev_info mhi_foxconn_sdx24_info = {
> 	.sideband_wake = false,
> };

Seems nobody use this mhi_foxconn_sdx55_info.  Please help remove it.

> 
>-static const struct mhi_pci_dev_info mhi_foxconn_sdx55_info = {
>-	.name = "foxconn-sdx55",
>+static const struct mhi_pci_dev_info mhi_foxconn_t99w175_info = {
>+	.name = "foxconn-t99w175",
> 	.fw = "qcom/sdx55m/sbl1.mbn",
> 	.edl = "qcom/sdx55m/edl.mbn",
> 	.config = &modem_foxconn_sdx55_config,
>@@ -439,8 +441,46 @@ static const struct mhi_pci_dev_info mhi_foxconn_sdx55_info = {
> 	.sideband_wake = false,
> };
> 
>-static const struct mhi_pci_dev_info mhi_foxconn_sdx65_info = {
>-	.name = "foxconn-sdx65",
>+static const struct mhi_pci_dev_info mhi_foxconn_dw5930e_info = {
>+	.name = "foxconn-dw5930e",
>+	.fw = "qcom/sdx55m/sbl1.mbn",
>+	.edl = "qcom/sdx55m/edl.mbn",
>+	.config = &modem_foxconn_sdx55_config,
>+	.bar_num = MHI_PCI_DEFAULT_BAR_NUM,
>+	.dma_data_width = 32,
>+	.mru_default = 32768,
>+	.sideband_wake = false,
>+};
>+
>+static const struct mhi_pci_dev_info mhi_foxconn_t99w368_info = {
>+	.name = "foxconn-t99w368",
>+	.config = &modem_foxconn_sdx55_config,
>+	.bar_num = MHI_PCI_DEFAULT_BAR_NUM,
>+	.dma_data_width = 32,
>+	.mru_default = 32768,
>+	.sideband_wake = false,
>+};
>+
>+static const struct mhi_pci_dev_info mhi_foxconn_t99w373_info = {
>+	.name = "foxconn-t99w373",
>+	.config = &modem_foxconn_sdx55_config,
>+	.bar_num = MHI_PCI_DEFAULT_BAR_NUM,
>+	.dma_data_width = 32,
>+	.mru_default = 32768,
>+	.sideband_wake = false,
>+};
>+
>+static const struct mhi_pci_dev_info mhi_foxconn_t99w510_info = {
>+	.name = "foxconn-t99w510",
>+	.config = &modem_foxconn_sdx55_config,
>+	.bar_num = MHI_PCI_DEFAULT_BAR_NUM,
>+	.dma_data_width = 32,
>+	.mru_default = 32768,
>+	.sideband_wake = false,
>+};
>+
>+static const struct mhi_pci_dev_info mhi_foxconn_dw5932e_info = {
>+	.name = "foxconn-dw5932e",
> 	.config = &modem_foxconn_sdx55_config,
> 	.bar_num = MHI_PCI_DEFAULT_BAR_NUM,
> 	.dma_data_width = 32,
>@@ -646,40 +686,40 @@ static const struct pci_device_id mhi_pci_id_table[] = {
> 		.driver_data = (kernel_ulong_t) &mhi_quectel_em1xx_info },
> 	/* T99W175 (sdx55), Both for eSIM and Non-eSIM */
> 	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0ab),
>-		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx55_info },
>+		.driver_data = (kernel_ulong_t) &mhi_foxconn_t99w175_info },
> 	/* DW5930e (sdx55), With eSIM, It's also T99W175 */
> 	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0b0),
>-		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx55_info },
>+		.driver_data = (kernel_ulong_t) &mhi_foxconn_dw5930e_info },
> 	/* DW5930e (sdx55), Non-eSIM, It's also T99W175 */
> 	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0b1),
>-		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx55_info },
>+		.driver_data = (kernel_ulong_t) &mhi_foxconn_dw5930e_info },
> 	/* T99W175 (sdx55), Based on Qualcomm new baseline */
> 	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0bf),
>-		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx55_info },
>+		.driver_data = (kernel_ulong_t) &mhi_foxconn_t99w175_info },
> 	/* T99W175 (sdx55) */
> 	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0c3),
>-		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx55_info },
>+		.driver_data = (kernel_ulong_t) &mhi_foxconn_t99w175_info },
> 	/* T99W368 (sdx65) */
> 	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0d8),
>-		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx65_info },
>+		.driver_data = (kernel_ulong_t) &mhi_foxconn_t99w368_info },
> 	/* T99W373 (sdx62) */
> 	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0d9),
>-		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx65_info },
>+		.driver_data = (kernel_ulong_t) &mhi_foxconn_t99w373_info },
> 	/* T99W510 (sdx24), variant 1 */
> 	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0f0),
>-		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx24_info },
>+		.driver_data = (kernel_ulong_t) &mhi_foxconn_t99w510_info },
> 	/* T99W510 (sdx24), variant 2 */
> 	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0f1),
>-		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx24_info },
>+		.driver_data = (kernel_ulong_t) &mhi_foxconn_t99w510_info },
> 	/* T99W510 (sdx24), variant 3 */
> 	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0f2),
>-		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx24_info },
>+		.driver_data = (kernel_ulong_t) &mhi_foxconn_t99w510_info },
> 	/* DW5932e-eSIM (sdx62), With eSIM */
> 	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0f5),
>-		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx65_info },
>+		.driver_data = (kernel_ulong_t) &mhi_foxconn_dw5932e_info },
> 	/* DW5932e (sdx62), Non-eSIM */
> 	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0f9),
>-		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx65_info },
>+		.driver_data = (kernel_ulong_t) &mhi_foxconn_dw5932e_info },
> 	/* MV31-W (Cinterion) */
> 	{ PCI_DEVICE(PCI_VENDOR_ID_THALES, 0x00b3),
> 		.driver_data = (kernel_ulong_t) &mhi_mv31_info },
>@@ -694,7 +734,7 @@ static const struct pci_device_id mhi_pci_id_table[] = {
> 		.driver_data = (kernel_ulong_t) &mhi_mv32_info },
> 	/* T99W175 (sdx55), HP variant */
> 	{ PCI_DEVICE(0x03f0, 0x0a6c),
>-		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx55_info },
>+		.driver_data = (kernel_ulong_t) &mhi_foxconn_t99w175_info },
> 	{  }
> };
> MODULE_DEVICE_TABLE(pci, mhi_pci_id_table);
>-- 
>2.25.1

Re: [PATCH v2] bus: mhi: host: pci_generic: Use unique 'mhi_pci_dev_info' for product families

Posted by Manivannan Sadhasivam 1 year, 5 months ago

On Wed, Jun 26, 2024 at 08:16:28PM +0800, Slark Xiao wrote:
> 
> At 2024-06-26 13:32:37, "Manivannan Sadhasivam" <manivannan.sadhasivam@linaro.org> wrote:
> >Currently, a single 'mhi_pci_dev_info' is shared across different product
> >families. Even though it makes the device functional, it misleads the users
> >by sharing the common product name.
> >
> >For instance, below message will be printed for Foxconn SDX62 modem during
> >boot:
> >
> >"MHI PCI device found: foxconn-sdx65"
> >
> >But this is quite misleading to the users since the actual modem plugged in
> >could be 'T99W373' which is based on SDX62.
> >
> >So fix this issue by using a unique 'mhi_pci_dev_info' for product
> >families. This allows us to specify a unique product name for each product
> >family. Also, once this name is exposed to client drivers, they may use
> >this name to identify the modems and use any modem specific configuration.
> >
> >Modems of unknown product families are not impacted by this change.
> >
> >CC: Slark Xiao <slark_xiao@163.com>
> >Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> 
> Reviewed-by: Slark Xiao <slark_xiao@163.com>
> 
> >---
> >
> >Changes in v2:
> >
> >* Used 'mhi_foxconn_t99w175_info' for HP variant as well
> >
> > drivers/bus/mhi/host/pci_generic.c | 78 ++++++++++++++++++++++--------
> > 1 file changed, 59 insertions(+), 19 deletions(-)
> >
> >diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
> >index 08844ee79654..35ae7cd0711f 100644
> >--- a/drivers/bus/mhi/host/pci_generic.c
> >+++ b/drivers/bus/mhi/host/pci_generic.c
> >@@ -419,8 +419,10 @@ static const struct mhi_controller_config modem_foxconn_sdx55_config = {
> > 	.event_cfg = mhi_foxconn_sdx55_events,
> > };
> > 
> >-static const struct mhi_pci_dev_info mhi_foxconn_sdx24_info = {
> >-	.name = "foxconn-sdx24",
> >+static const struct mhi_pci_dev_info mhi_foxconn_sdx55_info = {
> >+	.name = "foxconn-sdx55",
> >+	.fw = "qcom/sdx55m/sbl1.mbn",
> >+	.edl = "qcom/sdx55m/edl.mbn",
> > 	.config = &modem_foxconn_sdx55_config,
> > 	.bar_num = MHI_PCI_DEFAULT_BAR_NUM,
> > 	.dma_data_width = 32,
> >@@ -428,8 +430,8 @@ static const struct mhi_pci_dev_info mhi_foxconn_sdx24_info = {
> > 	.sideband_wake = false,
> > };
> 
> Seems nobody use this mhi_foxconn_sdx55_info.  Please help remove it.
> 

No, it is used by a Qcom modem.

- Mani

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