| Message ID | 1712805329-46158-3-git-send-email-quic_qianyu@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add sysfs entry to EDL mode | expand |
On 4/10/2024 9:15 PM, Qiang Yu wrote: > Add mhi_pci_generic_edl_trigger as edl_trigger for some devices (eg. SDX65) > to enter EDL mode by writing the 0xEDEDEDED cookie to the channel 91 > doorbell register and forcing an SOC reset afterwards. > > Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com> > --- > drivers/bus/mhi/host/pci_generic.c | 50 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 50 insertions(+) > > diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c > index 51639bf..a529815 100644 > --- a/drivers/bus/mhi/host/pci_generic.c > +++ b/drivers/bus/mhi/host/pci_generic.c > @@ -27,12 +27,23 @@ > #define PCI_VENDOR_ID_THALES 0x1269 > #define PCI_VENDOR_ID_QUECTEL 0x1eac > > +#define MHI_EDL_DB 91 > +#define MHI_EDL_COOKIE 0xEDEDEDED > + > +/* Device can enter EDL by first setting edl cookie then issuing inband reset*/ > +#define MHI_PCI_GENERIC_EDL_TRIGGER BIT(0) > + > +#define CHDBOFF 0x18 This is already in drivers/bus/mhi/common.h why duplicate it here? > +#define CHDBOFF_CHDBOFF_MASK 0xFFFFFFFF > +#define CHDBOFF_CHDBOFF_SHIFT 0 > + > /** > * struct mhi_pci_dev_info - MHI PCI device specific information > * @config: MHI controller configuration > * @name: name of the PCI module > * @fw: firmware path (if any) > * @edl: emergency download mode firmware path (if any) > + * @edl_trigger: each bit represents a different way to enter EDL > * @bar_num: PCI base address register to use for MHI MMIO register space > * @dma_data_width: DMA transfer word size (32 or 64 bits) > * @mru_default: default MRU size for MBIM network packets > @@ -44,6 +55,7 @@ struct mhi_pci_dev_info { > const char *name; > const char *fw; > const char *edl; > + unsigned int edl_trigger; > unsigned int bar_num; > unsigned int dma_data_width; > unsigned int mru_default; > @@ -292,6 +304,7 @@ static const struct mhi_pci_dev_info mhi_qcom_sdx75_info = { > .name = "qcom-sdx75m", > .fw = "qcom/sdx75m/xbl.elf", > .edl = "qcom/sdx75m/edl.mbn", > + .edl_trigger = MHI_PCI_GENERIC_EDL_TRIGGER, > .config = &modem_qcom_v2_mhiv_config, > .bar_num = MHI_PCI_DEFAULT_BAR_NUM, > .dma_data_width = 32, > @@ -302,6 +315,7 @@ static const struct mhi_pci_dev_info mhi_qcom_sdx65_info = { > .name = "qcom-sdx65m", > .fw = "qcom/sdx65m/xbl.elf", > .edl = "qcom/sdx65m/edl.mbn", > + .edl_trigger = MHI_PCI_GENERIC_EDL_TRIGGER, > .config = &modem_qcom_v1_mhiv_config, > .bar_num = MHI_PCI_DEFAULT_BAR_NUM, > .dma_data_width = 32, > @@ -312,6 +326,7 @@ static const struct mhi_pci_dev_info mhi_qcom_sdx55_info = { > .name = "qcom-sdx55m", > .fw = "qcom/sdx55m/sbl1.mbn", > .edl = "qcom/sdx55m/edl.mbn", > + .edl_trigger = MHI_PCI_GENERIC_EDL_TRIGGER, > .config = &modem_qcom_v1_mhiv_config, > .bar_num = MHI_PCI_DEFAULT_BAR_NUM, > .dma_data_width = 32, > @@ -928,6 +943,38 @@ static void health_check(struct timer_list *t) > mod_timer(&mhi_pdev->health_check_timer, jiffies + HEALTH_CHECK_PERIOD); > } > > +static int mhi_pci_generic_edl_trigger(struct mhi_controller *mhi_cntrl) > +{ > + int ret; > + u32 val; > + void __iomem *edl_db; > + void __iomem *base = mhi_cntrl->regs; It looks like this file follows reverse christmas tree, but this function does not. you should fix it. > + > + ret = mhi_device_get_sync(mhi_cntrl->mhi_dev); > + if (ret) { > + dev_err(mhi_cntrl->cntrl_dev, "Wake up device fail before trigger EDL\n"); > + return ret; > + } > + > + pm_wakeup_event(&mhi_cntrl->mhi_dev->dev, 0); > + mhi_cntrl->runtime_get(mhi_cntrl); > + > + mhi_cntrl->read_reg(mhi_cntrl, base + CHDBOFF, &val); > + val = (val & CHDBOFF_CHDBOFF_MASK) >> CHDBOFF_CHDBOFF_SHIFT; > + > + edl_db = base + val + (8 * MHI_EDL_DB); > + > + mhi_cntrl->write_reg(mhi_cntrl, edl_db + 4, upper_32_bits(MHI_EDL_COOKIE)); > + mhi_cntrl->write_reg(mhi_cntrl, edl_db, lower_32_bits(MHI_EDL_COOKIE)); > + > + mhi_soc_reset(mhi_cntrl); > + > + mhi_cntrl->runtime_put(mhi_cntrl); > + mhi_device_put(mhi_cntrl->mhi_dev); > + > + return 0; > +} > + > static int mhi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) > { > const struct mhi_pci_dev_info *info = (struct mhi_pci_dev_info *) id->driver_data; > @@ -962,6 +1009,9 @@ static int mhi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) > mhi_cntrl->runtime_put = mhi_pci_runtime_put; > mhi_cntrl->mru = info->mru_default; > > + if (info->edl_trigger & MHI_PCI_GENERIC_EDL_TRIGGER) > + mhi_cntrl->edl_trigger = mhi_pci_generic_edl_trigger; > + > if (info->sideband_wake) { > mhi_cntrl->wake_get = mhi_pci_wake_get_nop; > mhi_cntrl->wake_put = mhi_pci_wake_put_nop;
On 4/11/2024 10:46 PM, Jeffrey Hugo wrote: > On 4/10/2024 9:15 PM, Qiang Yu wrote: >> Add mhi_pci_generic_edl_trigger as edl_trigger for some devices (eg. >> SDX65) >> to enter EDL mode by writing the 0xEDEDEDED cookie to the channel 91 >> doorbell register and forcing an SOC reset afterwards. >> >> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com> >> --- >> drivers/bus/mhi/host/pci_generic.c | 50 >> ++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 50 insertions(+) >> >> diff --git a/drivers/bus/mhi/host/pci_generic.c >> b/drivers/bus/mhi/host/pci_generic.c >> index 51639bf..a529815 100644 >> --- a/drivers/bus/mhi/host/pci_generic.c >> +++ b/drivers/bus/mhi/host/pci_generic.c >> @@ -27,12 +27,23 @@ >> #define PCI_VENDOR_ID_THALES 0x1269 >> #define PCI_VENDOR_ID_QUECTEL 0x1eac >> +#define MHI_EDL_DB 91 >> +#define MHI_EDL_COOKIE 0xEDEDEDED >> + >> +/* Device can enter EDL by first setting edl cookie then issuing >> inband reset*/ >> +#define MHI_PCI_GENERIC_EDL_TRIGGER BIT(0) >> + >> +#define CHDBOFF 0x18 > > This is already in drivers/bus/mhi/common.h why duplicate it here? I only see common.h be included in ep/internal.h host/internal.h and host/trace.h. So I thought it can only be used by MHI stack. Can we include common.h in pci_generic.c? > >> +#define CHDBOFF_CHDBOFF_MASK 0xFFFFFFFF >> +#define CHDBOFF_CHDBOFF_SHIFT 0 >> + >> /** >> * struct mhi_pci_dev_info - MHI PCI device specific information >> * @config: MHI controller configuration >> * @name: name of the PCI module >> * @fw: firmware path (if any) >> * @edl: emergency download mode firmware path (if any) >> + * @edl_trigger: each bit represents a different way to enter EDL >> * @bar_num: PCI base address register to use for MHI MMIO register >> space >> * @dma_data_width: DMA transfer word size (32 or 64 bits) >> * @mru_default: default MRU size for MBIM network packets >> @@ -44,6 +55,7 @@ struct mhi_pci_dev_info { >> const char *name; >> const char *fw; >> const char *edl; >> + unsigned int edl_trigger; >> unsigned int bar_num; >> unsigned int dma_data_width; >> unsigned int mru_default; >> @@ -292,6 +304,7 @@ static const struct mhi_pci_dev_info >> mhi_qcom_sdx75_info = { >> .name = "qcom-sdx75m", >> .fw = "qcom/sdx75m/xbl.elf", >> .edl = "qcom/sdx75m/edl.mbn", >> + .edl_trigger = MHI_PCI_GENERIC_EDL_TRIGGER, >> .config = &modem_qcom_v2_mhiv_config, >> .bar_num = MHI_PCI_DEFAULT_BAR_NUM, >> .dma_data_width = 32, >> @@ -302,6 +315,7 @@ static const struct mhi_pci_dev_info >> mhi_qcom_sdx65_info = { >> .name = "qcom-sdx65m", >> .fw = "qcom/sdx65m/xbl.elf", >> .edl = "qcom/sdx65m/edl.mbn", >> + .edl_trigger = MHI_PCI_GENERIC_EDL_TRIGGER, >> .config = &modem_qcom_v1_mhiv_config, >> .bar_num = MHI_PCI_DEFAULT_BAR_NUM, >> .dma_data_width = 32, >> @@ -312,6 +326,7 @@ static const struct mhi_pci_dev_info >> mhi_qcom_sdx55_info = { >> .name = "qcom-sdx55m", >> .fw = "qcom/sdx55m/sbl1.mbn", >> .edl = "qcom/sdx55m/edl.mbn", >> + .edl_trigger = MHI_PCI_GENERIC_EDL_TRIGGER, >> .config = &modem_qcom_v1_mhiv_config, >> .bar_num = MHI_PCI_DEFAULT_BAR_NUM, >> .dma_data_width = 32, >> @@ -928,6 +943,38 @@ static void health_check(struct timer_list *t) >> mod_timer(&mhi_pdev->health_check_timer, jiffies + >> HEALTH_CHECK_PERIOD); >> } >> +static int mhi_pci_generic_edl_trigger(struct mhi_controller >> *mhi_cntrl) >> +{ >> + int ret; >> + u32 val; >> + void __iomem *edl_db; >> + void __iomem *base = mhi_cntrl->regs; > > It looks like this file follows reverse christmas tree, but this > function does not. you should fix it. Will fix it in next version patch. > >> + >> + ret = mhi_device_get_sync(mhi_cntrl->mhi_dev); >> + if (ret) { >> + dev_err(mhi_cntrl->cntrl_dev, "Wake up device fail before >> trigger EDL\n"); >> + return ret; >> + } >> + >> + pm_wakeup_event(&mhi_cntrl->mhi_dev->dev, 0); >> + mhi_cntrl->runtime_get(mhi_cntrl); >> + >> + mhi_cntrl->read_reg(mhi_cntrl, base + CHDBOFF, &val); >> + val = (val & CHDBOFF_CHDBOFF_MASK) >> CHDBOFF_CHDBOFF_SHIFT; >> + >> + edl_db = base + val + (8 * MHI_EDL_DB); >> + >> + mhi_cntrl->write_reg(mhi_cntrl, edl_db + 4, >> upper_32_bits(MHI_EDL_COOKIE)); >> + mhi_cntrl->write_reg(mhi_cntrl, edl_db, >> lower_32_bits(MHI_EDL_COOKIE)); >> + >> + mhi_soc_reset(mhi_cntrl); >> + >> + mhi_cntrl->runtime_put(mhi_cntrl); >> + mhi_device_put(mhi_cntrl->mhi_dev); >> + >> + return 0; >> +} >> + >> static int mhi_pci_probe(struct pci_dev *pdev, const struct >> pci_device_id *id) >> { >> const struct mhi_pci_dev_info *info = (struct mhi_pci_dev_info >> *) id->driver_data; >> @@ -962,6 +1009,9 @@ static int mhi_pci_probe(struct pci_dev *pdev, >> const struct pci_device_id *id) >> mhi_cntrl->runtime_put = mhi_pci_runtime_put; >> mhi_cntrl->mru = info->mru_default; >> + if (info->edl_trigger & MHI_PCI_GENERIC_EDL_TRIGGER) >> + mhi_cntrl->edl_trigger = mhi_pci_generic_edl_trigger; >> + >> if (info->sideband_wake) { >> mhi_cntrl->wake_get = mhi_pci_wake_get_nop; >> mhi_cntrl->wake_put = mhi_pci_wake_put_nop; >
On 4/12/2024 1:13 AM, Qiang Yu wrote: > > On 4/11/2024 10:46 PM, Jeffrey Hugo wrote: >> On 4/10/2024 9:15 PM, Qiang Yu wrote: >>> Add mhi_pci_generic_edl_trigger as edl_trigger for some devices (eg. >>> SDX65) >>> to enter EDL mode by writing the 0xEDEDEDED cookie to the channel 91 >>> doorbell register and forcing an SOC reset afterwards. >>> >>> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com> >>> --- >>> drivers/bus/mhi/host/pci_generic.c | 50 >>> ++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 50 insertions(+) >>> >>> diff --git a/drivers/bus/mhi/host/pci_generic.c >>> b/drivers/bus/mhi/host/pci_generic.c >>> index 51639bf..a529815 100644 >>> --- a/drivers/bus/mhi/host/pci_generic.c >>> +++ b/drivers/bus/mhi/host/pci_generic.c >>> @@ -27,12 +27,23 @@ >>> #define PCI_VENDOR_ID_THALES 0x1269 >>> #define PCI_VENDOR_ID_QUECTEL 0x1eac >>> +#define MHI_EDL_DB 91 >>> +#define MHI_EDL_COOKIE 0xEDEDEDED >>> + >>> +/* Device can enter EDL by first setting edl cookie then issuing >>> inband reset*/ >>> +#define MHI_PCI_GENERIC_EDL_TRIGGER BIT(0) >>> + >>> +#define CHDBOFF 0x18 >> >> This is already in drivers/bus/mhi/common.h why duplicate it here? > > I only see common.h be included in ep/internal.h host/internal.h and > host/trace.h. So I thought it can only be used by MHI stack. Can we > include common.h in pci_generic.c? Up to Mani, but duplicating the definition seems like it would result in maintence overhead over time. An alternative to including the header might be a new API between MHI and controller which allow the setting of a CHDB to a specific value. >> >>> +#define CHDBOFF_CHDBOFF_MASK 0xFFFFFFFF >>> +#define CHDBOFF_CHDBOFF_SHIFT 0 >>> + >>> /** >>> * struct mhi_pci_dev_info - MHI PCI device specific information >>> * @config: MHI controller configuration >>> * @name: name of the PCI module >>> * @fw: firmware path (if any) >>> * @edl: emergency download mode firmware path (if any) >>> + * @edl_trigger: each bit represents a different way to enter EDL >>> * @bar_num: PCI base address register to use for MHI MMIO register >>> space >>> * @dma_data_width: DMA transfer word size (32 or 64 bits) >>> * @mru_default: default MRU size for MBIM network packets >>> @@ -44,6 +55,7 @@ struct mhi_pci_dev_info { >>> const char *name; >>> const char *fw; >>> const char *edl; >>> + unsigned int edl_trigger; >>> unsigned int bar_num; >>> unsigned int dma_data_width; >>> unsigned int mru_default; >>> @@ -292,6 +304,7 @@ static const struct mhi_pci_dev_info >>> mhi_qcom_sdx75_info = { >>> .name = "qcom-sdx75m", >>> .fw = "qcom/sdx75m/xbl.elf", >>> .edl = "qcom/sdx75m/edl.mbn", >>> + .edl_trigger = MHI_PCI_GENERIC_EDL_TRIGGER, >>> .config = &modem_qcom_v2_mhiv_config, >>> .bar_num = MHI_PCI_DEFAULT_BAR_NUM, >>> .dma_data_width = 32, >>> @@ -302,6 +315,7 @@ static const struct mhi_pci_dev_info >>> mhi_qcom_sdx65_info = { >>> .name = "qcom-sdx65m", >>> .fw = "qcom/sdx65m/xbl.elf", >>> .edl = "qcom/sdx65m/edl.mbn", >>> + .edl_trigger = MHI_PCI_GENERIC_EDL_TRIGGER, >>> .config = &modem_qcom_v1_mhiv_config, >>> .bar_num = MHI_PCI_DEFAULT_BAR_NUM, >>> .dma_data_width = 32, >>> @@ -312,6 +326,7 @@ static const struct mhi_pci_dev_info >>> mhi_qcom_sdx55_info = { >>> .name = "qcom-sdx55m", >>> .fw = "qcom/sdx55m/sbl1.mbn", >>> .edl = "qcom/sdx55m/edl.mbn", >>> + .edl_trigger = MHI_PCI_GENERIC_EDL_TRIGGER, >>> .config = &modem_qcom_v1_mhiv_config, >>> .bar_num = MHI_PCI_DEFAULT_BAR_NUM, >>> .dma_data_width = 32, >>> @@ -928,6 +943,38 @@ static void health_check(struct timer_list *t) >>> mod_timer(&mhi_pdev->health_check_timer, jiffies + >>> HEALTH_CHECK_PERIOD); >>> } >>> +static int mhi_pci_generic_edl_trigger(struct mhi_controller >>> *mhi_cntrl) >>> +{ >>> + int ret; >>> + u32 val; >>> + void __iomem *edl_db; >>> + void __iomem *base = mhi_cntrl->regs; >> >> It looks like this file follows reverse christmas tree, but this >> function does not. you should fix it. > > Will fix it in next version patch. >> >>> + >>> + ret = mhi_device_get_sync(mhi_cntrl->mhi_dev); >>> + if (ret) { >>> + dev_err(mhi_cntrl->cntrl_dev, "Wake up device fail before >>> trigger EDL\n"); >>> + return ret; >>> + } >>> + >>> + pm_wakeup_event(&mhi_cntrl->mhi_dev->dev, 0); >>> + mhi_cntrl->runtime_get(mhi_cntrl); >>> + >>> + mhi_cntrl->read_reg(mhi_cntrl, base + CHDBOFF, &val); >>> + val = (val & CHDBOFF_CHDBOFF_MASK) >> CHDBOFF_CHDBOFF_SHIFT; >>> + >>> + edl_db = base + val + (8 * MHI_EDL_DB); >>> + >>> + mhi_cntrl->write_reg(mhi_cntrl, edl_db + 4, >>> upper_32_bits(MHI_EDL_COOKIE)); >>> + mhi_cntrl->write_reg(mhi_cntrl, edl_db, >>> lower_32_bits(MHI_EDL_COOKIE)); >>> + >>> + mhi_soc_reset(mhi_cntrl); >>> + >>> + mhi_cntrl->runtime_put(mhi_cntrl); >>> + mhi_device_put(mhi_cntrl->mhi_dev); >>> + >>> + return 0; >>> +} >>> + >>> static int mhi_pci_probe(struct pci_dev *pdev, const struct >>> pci_device_id *id) >>> { >>> const struct mhi_pci_dev_info *info = (struct mhi_pci_dev_info >>> *) id->driver_data; >>> @@ -962,6 +1009,9 @@ static int mhi_pci_probe(struct pci_dev *pdev, >>> const struct pci_device_id *id) >>> mhi_cntrl->runtime_put = mhi_pci_runtime_put; >>> mhi_cntrl->mru = info->mru_default; >>> + if (info->edl_trigger & MHI_PCI_GENERIC_EDL_TRIGGER) >>> + mhi_cntrl->edl_trigger = mhi_pci_generic_edl_trigger; >>> + >>> if (info->sideband_wake) { >>> mhi_cntrl->wake_get = mhi_pci_wake_get_nop; >>> mhi_cntrl->wake_put = mhi_pci_wake_put_nop; >>
On Fri, Apr 12, 2024 at 08:16:52AM -0600, Jeffrey Hugo wrote: > On 4/12/2024 1:13 AM, Qiang Yu wrote: > > > > On 4/11/2024 10:46 PM, Jeffrey Hugo wrote: > > > On 4/10/2024 9:15 PM, Qiang Yu wrote: > > > > Add mhi_pci_generic_edl_trigger as edl_trigger for some devices > > > > (eg. SDX65) > > > > to enter EDL mode by writing the 0xEDEDEDED cookie to the channel 91 > > > > doorbell register and forcing an SOC reset afterwards. > > > > > > > > Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com> > > > > --- > > > > drivers/bus/mhi/host/pci_generic.c | 50 > > > > ++++++++++++++++++++++++++++++++++++++ > > > > 1 file changed, 50 insertions(+) > > > > > > > > diff --git a/drivers/bus/mhi/host/pci_generic.c > > > > b/drivers/bus/mhi/host/pci_generic.c > > > > index 51639bf..a529815 100644 > > > > --- a/drivers/bus/mhi/host/pci_generic.c > > > > +++ b/drivers/bus/mhi/host/pci_generic.c > > > > @@ -27,12 +27,23 @@ > > > > #define PCI_VENDOR_ID_THALES 0x1269 > > > > #define PCI_VENDOR_ID_QUECTEL 0x1eac > > > > +#define MHI_EDL_DB 91 > > > > +#define MHI_EDL_COOKIE 0xEDEDEDED > > > > + > > > > +/* Device can enter EDL by first setting edl cookie then > > > > issuing inband reset*/ > > > > +#define MHI_PCI_GENERIC_EDL_TRIGGER BIT(0) > > > > + > > > > +#define CHDBOFF 0x18 > > > > > > This is already in drivers/bus/mhi/common.h why duplicate it here? > > > > I only see common.h be included in ep/internal.h host/internal.h and > > host/trace.h. So I thought it can only be used by MHI stack. Can we > > include common.h in pci_generic.c? > > Up to Mani, but duplicating the definition seems like it would result in > maintence overhead over time. An alternative to including the header might > be a new API between MHI and controller which allow the setting of a CHDB to > a specific value. > +1 to the new API suggestion. - Mani > > > > > > > +#define CHDBOFF_CHDBOFF_MASK 0xFFFFFFFF > > > > +#define CHDBOFF_CHDBOFF_SHIFT 0 > > > > + > > > > /** > > > > * struct mhi_pci_dev_info - MHI PCI device specific information > > > > * @config: MHI controller configuration > > > > * @name: name of the PCI module > > > > * @fw: firmware path (if any) > > > > * @edl: emergency download mode firmware path (if any) > > > > + * @edl_trigger: each bit represents a different way to enter EDL > > > > * @bar_num: PCI base address register to use for MHI MMIO > > > > register space > > > > * @dma_data_width: DMA transfer word size (32 or 64 bits) > > > > * @mru_default: default MRU size for MBIM network packets > > > > @@ -44,6 +55,7 @@ struct mhi_pci_dev_info { > > > > const char *name; > > > > const char *fw; > > > > const char *edl; > > > > + unsigned int edl_trigger; > > > > unsigned int bar_num; > > > > unsigned int dma_data_width; > > > > unsigned int mru_default; > > > > @@ -292,6 +304,7 @@ static const struct mhi_pci_dev_info > > > > mhi_qcom_sdx75_info = { > > > > .name = "qcom-sdx75m", > > > > .fw = "qcom/sdx75m/xbl.elf", > > > > .edl = "qcom/sdx75m/edl.mbn", > > > > + .edl_trigger = MHI_PCI_GENERIC_EDL_TRIGGER, > > > > .config = &modem_qcom_v2_mhiv_config, > > > > .bar_num = MHI_PCI_DEFAULT_BAR_NUM, > > > > .dma_data_width = 32, > > > > @@ -302,6 +315,7 @@ static const struct mhi_pci_dev_info > > > > mhi_qcom_sdx65_info = { > > > > .name = "qcom-sdx65m", > > > > .fw = "qcom/sdx65m/xbl.elf", > > > > .edl = "qcom/sdx65m/edl.mbn", > > > > + .edl_trigger = MHI_PCI_GENERIC_EDL_TRIGGER, > > > > .config = &modem_qcom_v1_mhiv_config, > > > > .bar_num = MHI_PCI_DEFAULT_BAR_NUM, > > > > .dma_data_width = 32, > > > > @@ -312,6 +326,7 @@ static const struct mhi_pci_dev_info > > > > mhi_qcom_sdx55_info = { > > > > .name = "qcom-sdx55m", > > > > .fw = "qcom/sdx55m/sbl1.mbn", > > > > .edl = "qcom/sdx55m/edl.mbn", > > > > + .edl_trigger = MHI_PCI_GENERIC_EDL_TRIGGER, > > > > .config = &modem_qcom_v1_mhiv_config, > > > > .bar_num = MHI_PCI_DEFAULT_BAR_NUM, > > > > .dma_data_width = 32, > > > > @@ -928,6 +943,38 @@ static void health_check(struct timer_list *t) > > > > mod_timer(&mhi_pdev->health_check_timer, jiffies + > > > > HEALTH_CHECK_PERIOD); > > > > } > > > > +static int mhi_pci_generic_edl_trigger(struct mhi_controller > > > > *mhi_cntrl) > > > > +{ > > > > + int ret; > > > > + u32 val; > > > > + void __iomem *edl_db; > > > > + void __iomem *base = mhi_cntrl->regs; > > > > > > It looks like this file follows reverse christmas tree, but this > > > function does not. you should fix it. > > > > Will fix it in next version patch. > > > > > > > + > > > > + ret = mhi_device_get_sync(mhi_cntrl->mhi_dev); > > > > + if (ret) { > > > > + dev_err(mhi_cntrl->cntrl_dev, "Wake up device fail > > > > before trigger EDL\n"); > > > > + return ret; > > > > + } > > > > + > > > > + pm_wakeup_event(&mhi_cntrl->mhi_dev->dev, 0); > > > > + mhi_cntrl->runtime_get(mhi_cntrl); > > > > + > > > > + mhi_cntrl->read_reg(mhi_cntrl, base + CHDBOFF, &val); > > > > + val = (val & CHDBOFF_CHDBOFF_MASK) >> CHDBOFF_CHDBOFF_SHIFT; > > > > + > > > > + edl_db = base + val + (8 * MHI_EDL_DB); > > > > + > > > > + mhi_cntrl->write_reg(mhi_cntrl, edl_db + 4, > > > > upper_32_bits(MHI_EDL_COOKIE)); > > > > + mhi_cntrl->write_reg(mhi_cntrl, edl_db, > > > > lower_32_bits(MHI_EDL_COOKIE)); > > > > + > > > > + mhi_soc_reset(mhi_cntrl); > > > > + > > > > + mhi_cntrl->runtime_put(mhi_cntrl); > > > > + mhi_device_put(mhi_cntrl->mhi_dev); > > > > + > > > > + return 0; > > > > +} > > > > + > > > > static int mhi_pci_probe(struct pci_dev *pdev, const struct > > > > pci_device_id *id) > > > > { > > > > const struct mhi_pci_dev_info *info = (struct > > > > mhi_pci_dev_info *) id->driver_data; > > > > @@ -962,6 +1009,9 @@ static int mhi_pci_probe(struct pci_dev > > > > *pdev, const struct pci_device_id *id) > > > > mhi_cntrl->runtime_put = mhi_pci_runtime_put; > > > > mhi_cntrl->mru = info->mru_default; > > > > + if (info->edl_trigger & MHI_PCI_GENERIC_EDL_TRIGGER) > > > > + mhi_cntrl->edl_trigger = mhi_pci_generic_edl_trigger; > > > > + > > > > if (info->sideband_wake) { > > > > mhi_cntrl->wake_get = mhi_pci_wake_get_nop; > > > > mhi_cntrl->wake_put = mhi_pci_wake_put_nop; > > > > >
On 4/13/2024 1:09 AM, Manivannan Sadhasivam wrote: > On Fri, Apr 12, 2024 at 08:16:52AM -0600, Jeffrey Hugo wrote: >> On 4/12/2024 1:13 AM, Qiang Yu wrote: >>> On 4/11/2024 10:46 PM, Jeffrey Hugo wrote: >>>> On 4/10/2024 9:15 PM, Qiang Yu wrote: >>>>> Add mhi_pci_generic_edl_trigger as edl_trigger for some devices >>>>> (eg. SDX65) >>>>> to enter EDL mode by writing the 0xEDEDEDED cookie to the channel 91 >>>>> doorbell register and forcing an SOC reset afterwards. >>>>> >>>>> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com> >>>>> --- >>>>> drivers/bus/mhi/host/pci_generic.c | 50 >>>>> ++++++++++++++++++++++++++++++++++++++ >>>>> 1 file changed, 50 insertions(+) >>>>> >>>>> diff --git a/drivers/bus/mhi/host/pci_generic.c >>>>> b/drivers/bus/mhi/host/pci_generic.c >>>>> index 51639bf..a529815 100644 >>>>> --- a/drivers/bus/mhi/host/pci_generic.c >>>>> +++ b/drivers/bus/mhi/host/pci_generic.c >>>>> @@ -27,12 +27,23 @@ >>>>> #define PCI_VENDOR_ID_THALES 0x1269 >>>>> #define PCI_VENDOR_ID_QUECTEL 0x1eac >>>>> +#define MHI_EDL_DB 91 >>>>> +#define MHI_EDL_COOKIE 0xEDEDEDED >>>>> + >>>>> +/* Device can enter EDL by first setting edl cookie then >>>>> issuing inband reset*/ >>>>> +#define MHI_PCI_GENERIC_EDL_TRIGGER BIT(0) >>>>> + >>>>> +#define CHDBOFF 0x18 >>>> This is already in drivers/bus/mhi/common.h why duplicate it here? >>> I only see common.h be included in ep/internal.h host/internal.h and >>> host/trace.h. So I thought it can only be used by MHI stack. Can we >>> include common.h in pci_generic.c? >> Up to Mani, but duplicating the definition seems like it would result in >> maintence overhead over time. An alternative to including the header might >> be a new API between MHI and controller which allow the setting of a CHDB to >> a specific value. >> > +1 to the new API suggestion. > > - Mani OK, will add a new API in MHI to allow controller get CHDB address. >>>>> +#define CHDBOFF_CHDBOFF_MASK 0xFFFFFFFF >>>>> +#define CHDBOFF_CHDBOFF_SHIFT 0 >>>>> + >>>>> /** >>>>> * struct mhi_pci_dev_info - MHI PCI device specific information >>>>> * @config: MHI controller configuration >>>>> * @name: name of the PCI module >>>>> * @fw: firmware path (if any) >>>>> * @edl: emergency download mode firmware path (if any) >>>>> + * @edl_trigger: each bit represents a different way to enter EDL >>>>> * @bar_num: PCI base address register to use for MHI MMIO >>>>> register space >>>>> * @dma_data_width: DMA transfer word size (32 or 64 bits) >>>>> * @mru_default: default MRU size for MBIM network packets >>>>> @@ -44,6 +55,7 @@ struct mhi_pci_dev_info { >>>>> const char *name; >>>>> const char *fw; >>>>> const char *edl; >>>>> + unsigned int edl_trigger; >>>>> unsigned int bar_num; >>>>> unsigned int dma_data_width; >>>>> unsigned int mru_default; >>>>> @@ -292,6 +304,7 @@ static const struct mhi_pci_dev_info >>>>> mhi_qcom_sdx75_info = { >>>>> .name = "qcom-sdx75m", >>>>> .fw = "qcom/sdx75m/xbl.elf", >>>>> .edl = "qcom/sdx75m/edl.mbn", >>>>> + .edl_trigger = MHI_PCI_GENERIC_EDL_TRIGGER, >>>>> .config = &modem_qcom_v2_mhiv_config, >>>>> .bar_num = MHI_PCI_DEFAULT_BAR_NUM, >>>>> .dma_data_width = 32, >>>>> @@ -302,6 +315,7 @@ static const struct mhi_pci_dev_info >>>>> mhi_qcom_sdx65_info = { >>>>> .name = "qcom-sdx65m", >>>>> .fw = "qcom/sdx65m/xbl.elf", >>>>> .edl = "qcom/sdx65m/edl.mbn", >>>>> + .edl_trigger = MHI_PCI_GENERIC_EDL_TRIGGER, >>>>> .config = &modem_qcom_v1_mhiv_config, >>>>> .bar_num = MHI_PCI_DEFAULT_BAR_NUM, >>>>> .dma_data_width = 32, >>>>> @@ -312,6 +326,7 @@ static const struct mhi_pci_dev_info >>>>> mhi_qcom_sdx55_info = { >>>>> .name = "qcom-sdx55m", >>>>> .fw = "qcom/sdx55m/sbl1.mbn", >>>>> .edl = "qcom/sdx55m/edl.mbn", >>>>> + .edl_trigger = MHI_PCI_GENERIC_EDL_TRIGGER, >>>>> .config = &modem_qcom_v1_mhiv_config, >>>>> .bar_num = MHI_PCI_DEFAULT_BAR_NUM, >>>>> .dma_data_width = 32, >>>>> @@ -928,6 +943,38 @@ static void health_check(struct timer_list *t) >>>>> mod_timer(&mhi_pdev->health_check_timer, jiffies + >>>>> HEALTH_CHECK_PERIOD); >>>>> } >>>>> +static int mhi_pci_generic_edl_trigger(struct mhi_controller >>>>> *mhi_cntrl) >>>>> +{ >>>>> + int ret; >>>>> + u32 val; >>>>> + void __iomem *edl_db; >>>>> + void __iomem *base = mhi_cntrl->regs; >>>> It looks like this file follows reverse christmas tree, but this >>>> function does not. you should fix it. >>> Will fix it in next version patch. >>>>> + >>>>> + ret = mhi_device_get_sync(mhi_cntrl->mhi_dev); >>>>> + if (ret) { >>>>> + dev_err(mhi_cntrl->cntrl_dev, "Wake up device fail >>>>> before trigger EDL\n"); >>>>> + return ret; >>>>> + } >>>>> + >>>>> + pm_wakeup_event(&mhi_cntrl->mhi_dev->dev, 0); >>>>> + mhi_cntrl->runtime_get(mhi_cntrl); >>>>> + >>>>> + mhi_cntrl->read_reg(mhi_cntrl, base + CHDBOFF, &val); >>>>> + val = (val & CHDBOFF_CHDBOFF_MASK) >> CHDBOFF_CHDBOFF_SHIFT; >>>>> + >>>>> + edl_db = base + val + (8 * MHI_EDL_DB); >>>>> + >>>>> + mhi_cntrl->write_reg(mhi_cntrl, edl_db + 4, >>>>> upper_32_bits(MHI_EDL_COOKIE)); >>>>> + mhi_cntrl->write_reg(mhi_cntrl, edl_db, >>>>> lower_32_bits(MHI_EDL_COOKIE)); >>>>> + >>>>> + mhi_soc_reset(mhi_cntrl); >>>>> + >>>>> + mhi_cntrl->runtime_put(mhi_cntrl); >>>>> + mhi_device_put(mhi_cntrl->mhi_dev); >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> static int mhi_pci_probe(struct pci_dev *pdev, const struct >>>>> pci_device_id *id) >>>>> { >>>>> const struct mhi_pci_dev_info *info = (struct >>>>> mhi_pci_dev_info *) id->driver_data; >>>>> @@ -962,6 +1009,9 @@ static int mhi_pci_probe(struct pci_dev >>>>> *pdev, const struct pci_device_id *id) >>>>> mhi_cntrl->runtime_put = mhi_pci_runtime_put; >>>>> mhi_cntrl->mru = info->mru_default; >>>>> + if (info->edl_trigger & MHI_PCI_GENERIC_EDL_TRIGGER) >>>>> + mhi_cntrl->edl_trigger = mhi_pci_generic_edl_trigger; >>>>> + >>>>> if (info->sideband_wake) { >>>>> mhi_cntrl->wake_get = mhi_pci_wake_get_nop; >>>>> mhi_cntrl->wake_put = mhi_pci_wake_put_nop; >>
diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c index 51639bf..a529815 100644 --- a/drivers/bus/mhi/host/pci_generic.c +++ b/drivers/bus/mhi/host/pci_generic.c @@ -27,12 +27,23 @@ #define PCI_VENDOR_ID_THALES 0x1269 #define PCI_VENDOR_ID_QUECTEL 0x1eac +#define MHI_EDL_DB 91 +#define MHI_EDL_COOKIE 0xEDEDEDED + +/* Device can enter EDL by first setting edl cookie then issuing inband reset*/ +#define MHI_PCI_GENERIC_EDL_TRIGGER BIT(0) + +#define CHDBOFF 0x18 +#define CHDBOFF_CHDBOFF_MASK 0xFFFFFFFF +#define CHDBOFF_CHDBOFF_SHIFT 0 + /** * struct mhi_pci_dev_info - MHI PCI device specific information * @config: MHI controller configuration * @name: name of the PCI module * @fw: firmware path (if any) * @edl: emergency download mode firmware path (if any) + * @edl_trigger: each bit represents a different way to enter EDL * @bar_num: PCI base address register to use for MHI MMIO register space * @dma_data_width: DMA transfer word size (32 or 64 bits) * @mru_default: default MRU size for MBIM network packets @@ -44,6 +55,7 @@ struct mhi_pci_dev_info { const char *name; const char *fw; const char *edl; + unsigned int edl_trigger; unsigned int bar_num; unsigned int dma_data_width; unsigned int mru_default; @@ -292,6 +304,7 @@ static const struct mhi_pci_dev_info mhi_qcom_sdx75_info = { .name = "qcom-sdx75m", .fw = "qcom/sdx75m/xbl.elf", .edl = "qcom/sdx75m/edl.mbn", + .edl_trigger = MHI_PCI_GENERIC_EDL_TRIGGER, .config = &modem_qcom_v2_mhiv_config, .bar_num = MHI_PCI_DEFAULT_BAR_NUM, .dma_data_width = 32, @@ -302,6 +315,7 @@ static const struct mhi_pci_dev_info mhi_qcom_sdx65_info = { .name = "qcom-sdx65m", .fw = "qcom/sdx65m/xbl.elf", .edl = "qcom/sdx65m/edl.mbn", + .edl_trigger = MHI_PCI_GENERIC_EDL_TRIGGER, .config = &modem_qcom_v1_mhiv_config, .bar_num = MHI_PCI_DEFAULT_BAR_NUM, .dma_data_width = 32, @@ -312,6 +326,7 @@ static const struct mhi_pci_dev_info mhi_qcom_sdx55_info = { .name = "qcom-sdx55m", .fw = "qcom/sdx55m/sbl1.mbn", .edl = "qcom/sdx55m/edl.mbn", + .edl_trigger = MHI_PCI_GENERIC_EDL_TRIGGER, .config = &modem_qcom_v1_mhiv_config, .bar_num = MHI_PCI_DEFAULT_BAR_NUM, .dma_data_width = 32, @@ -928,6 +943,38 @@ static void health_check(struct timer_list *t) mod_timer(&mhi_pdev->health_check_timer, jiffies + HEALTH_CHECK_PERIOD); } +static int mhi_pci_generic_edl_trigger(struct mhi_controller *mhi_cntrl) +{ + int ret; + u32 val; + void __iomem *edl_db; + void __iomem *base = mhi_cntrl->regs; + + ret = mhi_device_get_sync(mhi_cntrl->mhi_dev); + if (ret) { + dev_err(mhi_cntrl->cntrl_dev, "Wake up device fail before trigger EDL\n"); + return ret; + } + + pm_wakeup_event(&mhi_cntrl->mhi_dev->dev, 0); + mhi_cntrl->runtime_get(mhi_cntrl); + + mhi_cntrl->read_reg(mhi_cntrl, base + CHDBOFF, &val); + val = (val & CHDBOFF_CHDBOFF_MASK) >> CHDBOFF_CHDBOFF_SHIFT; + + edl_db = base + val + (8 * MHI_EDL_DB); + + mhi_cntrl->write_reg(mhi_cntrl, edl_db + 4, upper_32_bits(MHI_EDL_COOKIE)); + mhi_cntrl->write_reg(mhi_cntrl, edl_db, lower_32_bits(MHI_EDL_COOKIE)); + + mhi_soc_reset(mhi_cntrl); + + mhi_cntrl->runtime_put(mhi_cntrl); + mhi_device_put(mhi_cntrl->mhi_dev); + + return 0; +} + static int mhi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) { const struct mhi_pci_dev_info *info = (struct mhi_pci_dev_info *) id->driver_data; @@ -962,6 +1009,9 @@ static int mhi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) mhi_cntrl->runtime_put = mhi_pci_runtime_put; mhi_cntrl->mru = info->mru_default; + if (info->edl_trigger & MHI_PCI_GENERIC_EDL_TRIGGER) + mhi_cntrl->edl_trigger = mhi_pci_generic_edl_trigger; + if (info->sideband_wake) { mhi_cntrl->wake_get = mhi_pci_wake_get_nop; mhi_cntrl->wake_put = mhi_pci_wake_put_nop;
Add mhi_pci_generic_edl_trigger as edl_trigger for some devices (eg. SDX65) to enter EDL mode by writing the 0xEDEDEDED cookie to the channel 91 doorbell register and forcing an SOC reset afterwards. Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com> --- drivers/bus/mhi/host/pci_generic.c | 50 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+)