[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <34a76874.acd9.1905479d200.Coremail.slark_xiao@163.com>
Date: Wed, 26 Jun 2024 20:16:28 +0800 (CST)
From: "Slark Xiao" <slark_xiao@....com>
To: "Manivannan Sadhasivam" <manivannan.sadhasivam@...aro.org>
Cc: mhi@...ts.linux.dev, linux-arm-msm@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re:[PATCH v2] bus: mhi: host: pci_generic: Use unique
'mhi_pci_dev_info' for product families
At 2024-06-26 13:32:37, "Manivannan Sadhasivam" <manivannan.sadhasivam@...aro.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@....com>
>Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
Reviewed-by: Slark Xiao <slark_xiao@....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
Powered by blists - more mailing lists