On 8/23/24 19:16, Ajay.Kathat@xxxxxxxxxxxxx wrote: > Hi Marek, > > On 8/21/24 11:36, Marek Vasut wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >> >> In case the hardware is not initialized, do not operate it during >> suspend/resume cycle, the hardware is already off so there is no >> reason to access it. >> >> In fact, wilc_sdio_enable_interrupt() in the resume callback does >> interfere with the same call when initializing the hardware after >> resume and makes such initialization after resume fail. Fix this >> by not operating uninitialized hardware during suspend/resume. > > Is this behavior observed then power-save is enabled when interface is not up. > Ideally registers read/write commands should pass as soon the wilc module is > up. But anyway, it is good have this check to avoid these commands. if > possible, please add the similar check for wilc_spi_suspend/resume() to have > similar behavior. It looks like there is a hole and that no PM ops are implemented in the upstream driver on spi side. That may be a miss from me when I sent the power management series a few months ago. I'll fix that. So for this change: Reviewed-by: Alexis Lothoré <alexis.lothore@xxxxxxxxxxx> > >> >> Signed-off-by: Marek Vasut <marex@xxxxxxx> >> --- >> Cc: Ajay Singh <ajay.kathat@xxxxxxxxxxxxx> >> Cc: Alexis Lothoré <alexis.lothore@xxxxxxxxxxx> >> Cc: Claudiu Beznea <claudiu.beznea@xxxxxxxxx> >> Cc: Kalle Valo <kvalo@xxxxxxxxxx> >> Cc: Marek Vasut <marex@xxxxxxx> >> Cc: linux-wireless@xxxxxxxxxxxxxxx >> --- >> drivers/net/wireless/microchip/wilc1000/sdio.c | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/drivers/net/wireless/microchip/wilc1000/sdio.c b/drivers/net/wireless/microchip/wilc1000/sdio.c >> index 0043f7a0fdf97..7999aeb76901f 100644 >> --- a/drivers/net/wireless/microchip/wilc1000/sdio.c >> +++ b/drivers/net/wireless/microchip/wilc1000/sdio.c >> @@ -977,6 +977,9 @@ static int wilc_sdio_suspend(struct device *dev) >> >> dev_info(dev, "sdio suspend\n"); >> >> + if (!wilc->initialized) >> + return 0; >> + >> if (!IS_ERR(wilc->rtc_clk)) >> clk_disable_unprepare(wilc->rtc_clk); >> >> @@ -999,6 +1002,10 @@ static int wilc_sdio_resume(struct device *dev) >> struct wilc *wilc = sdio_get_drvdata(func); >> >> dev_info(dev, "sdio resume\n"); >> + >> + if (!wilc->initialized) >> + return 0; >> + >> wilc_sdio_init(wilc, true); >> wilc_sdio_enable_interrupt(wilc); >> >> -- >> 2.43.0 >> > -- Alexis Lothoré, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
