Re: Re: [PATCH v3 2/3] bus: mhi: host: Add name for mhi_controller

On Sat, Jun 29, 2024 at 04:03:28PM +0800, Slark Xiao wrote:

At 2024-06-28 22:38:57, "Jeffrey Hugo" [off-list ref] wrote:

quoted

On 6/28/2024 1:36 AM, Slark Xiao wrote:

quoted

  For SDX72 MBIM mode, it starts data mux id from 112 instead of 0.
  This would lead to device can't ping outside successfully.
  Also MBIM side would report "bad packet session (112)".

quoted

Weird indentation

My mistake. Will be corrected in next.

quoted

quoted

  In oder to fix this issue, we decide to use the modem name

"order"

quoted

to do a match in client driver side. Then client driver could
set a corresponding mux_id value for this modem product.

Signed-off-by: Slark Xiao <redacted>
---
  drivers/bus/mhi/host/pci_generic.c | 1 +
  include/linux/mhi.h                | 2 ++
  2 files changed, 3 insertions(+)

diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
index 1fb1c2f2fe12..14a11880bcea 100644
--- a/drivers/bus/mhi/host/pci_generic.c
+++ b/drivers/bus/mhi/host/pci_generic.c

@@ -1086,6 +1086,7 @@ static int mhi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
  	mhi_cntrl->runtime_get = mhi_pci_runtime_get;
  	mhi_cntrl->runtime_put = mhi_pci_runtime_put;
  	mhi_cntrl->mru = info->mru_default;
+	mhi_cntrl->name = info->name;
  
  	if (info->edl_trigger)
  		mhi_cntrl->edl_trigger = mhi_pci_generic_edl_trigger;

diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index b573f15762f8..86aa4f52842c 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h

@@ -361,6 +361,7 @@ struct mhi_controller_config {
   * @wake_set: Device wakeup set flag
   * @irq_flags: irq flags passed to request_irq (optional)
   * @mru: the default MRU for the MHI device
+ * @name: name of the modem

quoted

Why restrict this to modems?  There are plenty of other MHI devices

Actually all MHI devices could be called modems. I don't think this is
a wrong name.

No, not all MHI controllers are modems. This driver is a generic driver for MHI
controllers. So use below description:

	'Product or device name of the MHI controller'

quoted

quoted

   *
   * Fields marked as (required) need to be populated by the controller driver
   * before calling mhi_register_controller(). For the fields marked as (optional)

@@ -445,6 +446,7 @@ struct mhi_controller {
  	bool wake_set;
  	unsigned long irq_flags;
  	u32 mru;
+	const char *name;

quoted

Please run pahole

Emm, just checked,  there are 3 holes:
    u32                        M3;                   /*   312     4 */
    /* XXX 4 bytes hole, try to pack */
...
    bool                       wake_set;             /*   526     1 */
    /* XXX 1 byte hole, try to pack */
...
    u32                        mru;                  /*   536     4 */
    /* XXX 4 bytes hole, try to pack */

I will put 'const char *name' above 'u32 mru' to avoid the last hole.
Is this okay?

Just put it at the top.

- Mani

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