| Message ID | 1606404547-10737-4-git-send-email-loic.poulain@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | mhi: pci_generic: Misc improvements | expand |
On 11/26/2020 8:29 AM, Loic Poulain wrote: > Ensure that the device is hard-reset on remove to restore its initial > state and avoid further issues on subsequent probe. > > This has been tested with Telit FN980m module. > > Signed-off-by: Loic Poulain <loic.poulain@linaro.org> > --- > drivers/bus/mhi/pci_generic.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/bus/mhi/pci_generic.c b/drivers/bus/mhi/pci_generic.c > index d3896ef..4363676 100644 > --- a/drivers/bus/mhi/pci_generic.c > +++ b/drivers/bus/mhi/pci_generic.c > @@ -15,6 +15,8 @@ > > #define MHI_PCI_DEFAULT_BAR_NUM 0 > > +#define DEV_RESET_REG (0xB0) > + > /** > * struct mhi_pci_dev_info - MHI PCI device specific information > * @config: MHI controller configuration > @@ -166,6 +168,11 @@ static void mhi_pci_status_cb(struct mhi_controller *mhi_cntrl, > /* Nothing to do for now */ > } > > +static inline void mhi_pci_reset(struct mhi_controller *mhi_cntrl) > +{ > + writel(1, mhi_cntrl->regs + DEV_RESET_REG); > +} > + > static int mhi_pci_claim(struct mhi_controller *mhi_cntrl, > unsigned int bar_num, u64 dma_mask) > { > @@ -329,6 +336,10 @@ static void mhi_pci_remove(struct pci_dev *pdev) > mhi_power_down(mhi_cntrl, true); > mhi_unprepare_after_power_down(mhi_cntrl); > mhi_unregister_controller(mhi_cntrl); > + > + /* MHI-layer reset could not be enough, always hard-reset the device */ > + mhi_pci_reset(mhi_cntrl); > + > mhi_free_controller(mhi_cntrl); > } > > Did you miss my eariler comment asking to make this functionality common? Also, I caution you to think carefully about this. Its possible doing this type of reset can take down the PCI link, and the host would need to have PCI hotplug to recover. If not, the device will become inaccessable until the host reboots. -- Jeffrey Hugo Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
On 11/27/2020 10:34 AM, Jeffrey Hugo wrote: > On 11/26/2020 8:29 AM, Loic Poulain wrote: >> Ensure that the device is hard-reset on remove to restore its initial >> state and avoid further issues on subsequent probe. >> >> This has been tested with Telit FN980m module. >> >> Signed-off-by: Loic Poulain <loic.poulain@linaro.org> >> --- >> drivers/bus/mhi/pci_generic.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/drivers/bus/mhi/pci_generic.c >> b/drivers/bus/mhi/pci_generic.c >> index d3896ef..4363676 100644 >> --- a/drivers/bus/mhi/pci_generic.c >> +++ b/drivers/bus/mhi/pci_generic.c >> @@ -15,6 +15,8 @@ >> #define MHI_PCI_DEFAULT_BAR_NUM 0 >> +#define DEV_RESET_REG (0xB0) >> + >> /** >> * struct mhi_pci_dev_info - MHI PCI device specific information >> * @config: MHI controller configuration >> @@ -166,6 +168,11 @@ static void mhi_pci_status_cb(struct >> mhi_controller *mhi_cntrl, >> /* Nothing to do for now */ >> } >> +static inline void mhi_pci_reset(struct mhi_controller *mhi_cntrl) >> +{ >> + writel(1, mhi_cntrl->regs + DEV_RESET_REG); >> +} >> + >> static int mhi_pci_claim(struct mhi_controller *mhi_cntrl, >> unsigned int bar_num, u64 dma_mask) >> { >> @@ -329,6 +336,10 @@ static void mhi_pci_remove(struct pci_dev *pdev) >> mhi_power_down(mhi_cntrl, true); >> mhi_unprepare_after_power_down(mhi_cntrl); >> mhi_unregister_controller(mhi_cntrl); >> + >> + /* MHI-layer reset could not be enough, always hard-reset the >> device */ >> + mhi_pci_reset(mhi_cntrl); >> + >> mhi_free_controller(mhi_cntrl); >> } >> > > Did you miss my eariler comment asking to make this functionality common? Sorry, I missed your reply to my reply. > > Also, I caution you to think carefully about this. Its possible doing > this type of reset can take down the PCI link, and the host would need > to have PCI hotplug to recover. If not, the device will become > inaccessable until the host reboots. > -- Jeffrey Hugo Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
diff --git a/drivers/bus/mhi/pci_generic.c b/drivers/bus/mhi/pci_generic.c index d3896ef..4363676 100644 --- a/drivers/bus/mhi/pci_generic.c +++ b/drivers/bus/mhi/pci_generic.c @@ -15,6 +15,8 @@ #define MHI_PCI_DEFAULT_BAR_NUM 0 +#define DEV_RESET_REG (0xB0) + /** * struct mhi_pci_dev_info - MHI PCI device specific information * @config: MHI controller configuration @@ -166,6 +168,11 @@ static void mhi_pci_status_cb(struct mhi_controller *mhi_cntrl, /* Nothing to do for now */ } +static inline void mhi_pci_reset(struct mhi_controller *mhi_cntrl) +{ + writel(1, mhi_cntrl->regs + DEV_RESET_REG); +} + static int mhi_pci_claim(struct mhi_controller *mhi_cntrl, unsigned int bar_num, u64 dma_mask) { @@ -329,6 +336,10 @@ static void mhi_pci_remove(struct pci_dev *pdev) mhi_power_down(mhi_cntrl, true); mhi_unprepare_after_power_down(mhi_cntrl); mhi_unregister_controller(mhi_cntrl); + + /* MHI-layer reset could not be enough, always hard-reset the device */ + mhi_pci_reset(mhi_cntrl); + mhi_free_controller(mhi_cntrl); }
Ensure that the device is hard-reset on remove to restore its initial state and avoid further issues on subsequent probe. This has been tested with Telit FN980m module. Signed-off-by: Loic Poulain <loic.poulain@linaro.org> --- drivers/bus/mhi/pci_generic.c | 11 +++++++++++ 1 file changed, 11 insertions(+) -- 2.7.4