| Message ID | 20240628073626.1447288-1-slark_xiao@163.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3,1/3] bus: mhi: host: Add Foxconn SDX72 related support | expand |
At 2024-06-28 22:38:57, "Jeffrey Hugo" <quic_jhugo@quicinc.com> wrote: >On 6/28/2024 1:36 AM, Slark Xiao wrote: >> 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)". > >Weird indentation My mistake. Will be corrected in next. > >> In oder to fix this issue, we decide to use the modem name > >"order" > >> 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 <slark_xiao@163.com> >> --- >> 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 > >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. > >> * >> * 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; > >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? > >> }; >> >> /**
On Sat, Jun 29, 2024 at 04:03:28PM +0800, Slark Xiao wrote: > > At 2024-06-28 22:38:57, "Jeffrey Hugo" <quic_jhugo@quicinc.com> wrote: > >On 6/28/2024 1:36 AM, Slark Xiao wrote: > >> 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)". > > > > >Weird indentation > > My mistake. Will be corrected in next. > > > > >> In oder to fix this issue, we decide to use the modem name > > > >"order" > > > >> 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 <slark_xiao@163.com> > >> --- > >> 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 > > > > >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' > > > >> * > >> * 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; > > > > >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
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 * * 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; }; /**
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)". In oder to fix this issue, we decide to use the modem name 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 <slark_xiao@163.com> --- drivers/bus/mhi/host/pci_generic.c | 1 + include/linux/mhi.h | 2 ++ 2 files changed, 3 insertions(+)