Messages in this thread | | | Subject | Re: [PATCH v2 10/15] soc: qcom: ipa: use new module_firmware_crashed() | From | Alex Elder <> | Date | Tue, 19 May 2020 17:34:13 -0500 |
| |
On 5/15/20 4:28 PM, Luis Chamberlain wrote: > This makes use of the new module_firmware_crashed() to help > annotate when firmware for device drivers crash. When firmware > crashes devices can sometimes become unresponsive, and recovery > sometimes requires a driver unload / reload and in the worst cases > a reboot. > > Using a taint flag allows us to annotate when this happens clearly.
I don't fully understand what this is meant to do, so I can't fully assess whether it's the right thing to do.
But in this particular place in the IPA code, the *modem* has crashed. And the IPA driver is not responsible for modem firmware, remoteproc is.
The IPA driver *can* be responsible for loading some other firmware, but even in that case, it only happens on initial boot, and it's basically assumed to never crash.
So regardless of whether this module_firmware_crashed() call is appropriate in some places, I believe it should not be used here.
-Alex
> > Cc: Alex Elder <elder@kernel.org> > Signed-off-by: Luis Chamberlain <mcgrof@kernel.org> > --- > drivers/net/ipa/ipa_modem.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/net/ipa/ipa_modem.c b/drivers/net/ipa/ipa_modem.c > index ed10818dd99f..1790b87446ed 100644 > --- a/drivers/net/ipa/ipa_modem.c > +++ b/drivers/net/ipa/ipa_modem.c > @@ -285,6 +285,7 @@ static void ipa_modem_crashed(struct ipa *ipa) > struct device *dev = &ipa->pdev->dev; > int ret; > > + module_firmware_crashed(); > ipa_endpoint_modem_pause_all(ipa, true); > > ipa_endpoint_modem_hol_block_clear_all(ipa); >
| |