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@xxxxxxxxxxx>
> > > > ---
> > > > 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;
> > >
>
>
--
மணிவண்ணன் சதாசிவம்
