Message ID | 1522959594-3411-1-git-send-email-ulf.hansson@linaro.org |
---|---|
Headers | show |
Series | mmc: sdio: Enable SW reset of SDIO cards | expand |
Hi Ulf, On 05-04-18 22:19, Ulf Hansson wrote: > It's rather common that SDIO func devices becomes loaded with a new firmware as > a part of the SDIO func driver being probed. However, in some special scenarios > the SDIO func device needs a SW reset, as to start running the new firmware. > > More importantly, a full power cycle doesn't work, as that would reset also the > firmware, thus the existing mmc_hw_reset() API can't be used to deal with these > scenarios. > > Therefore this series suggest to add a new API, mmc_sw_reset(), which resets > and re-initialize the SDIO card. A couple of the patches in the series are > mostly re-factorings making generic improvements to the related code. > > For more background to this series, feel free to digest the discussions from > the submitted patch: https://patchwork.kernel.org/patch/9857175/ > > It should be noted, at this point this series has only be compile tested. Help > with tests and deployment of using the new API is greatly appreciated. Thank you for this series, I've taken a quick look and it looks good, but the proof is in the pudding. Quentin can you test this on an ARM board with an ESP8089 sometime the coming week? Regards, Hans > > Kind regards > Ulf Hansson > > Ulf Hansson (5): > mmc: core: Re-factor some code for SDIO re-initialization > mmc: core: Rename ->reset() bus ops to ->hw_reset() > mmc: core: Export a function mmc_sw_reset() to allow soft reset of > cards > mmc: core: Share internal function to set initial signal voltage > mmc: core: Implement ->sw_reset bus ops for SDIO > > drivers/mmc/core/core.c | 49 +++++++++++++++++++++++++++++++++--------- > drivers/mmc/core/core.h | 4 +++- > drivers/mmc/core/mmc.c | 4 ++-- > drivers/mmc/core/sd.c | 4 ++-- > drivers/mmc/core/sdio.c | 56 +++++++++++++++++++++++++++++------------------- > include/linux/mmc/core.h | 1 + > 6 files changed, 81 insertions(+), 37 deletions(-) > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ulf On Fri, Apr 06, 2018 at 10:41:10AM +0200, Hans de Goede wrote: > Hi Ulf, > > On 05-04-18 22:19, Ulf Hansson wrote: > > It's rather common that SDIO func devices becomes loaded with a new firmware as > > a part of the SDIO func driver being probed. However, in some special scenarios > > the SDIO func device needs a SW reset, as to start running the new firmware. > > > > More importantly, a full power cycle doesn't work, as that would reset also the > > firmware, thus the existing mmc_hw_reset() API can't be used to deal with these > > scenarios. > > > > Therefore this series suggest to add a new API, mmc_sw_reset(), which resets > > and re-initialize the SDIO card. A couple of the patches in the series are > > mostly re-factorings making generic improvements to the related code. > > > > For more background to this series, feel free to digest the discussions from > > the submitted patch: https://patchwork.kernel.org/patch/9857175/ > > > > It should be noted, at this point this series has only be compile tested. Help > > with tests and deployment of using the new API is greatly appreciated. > > Thank you for this series, I've taken a quick look and it looks good, > but the proof is in the pudding. Quentin can you test this on an ARM > board with an ESP8089 sometime the coming week? > Indeed, thank you for this series, it's highly appreciated. It's on my TODO list. If you haven't heard of me by Tuesday next week, please ping me. Thanks, Quentin > Regards, > > Hans > > > > > > Kind regards > > Ulf Hansson > > > > Ulf Hansson (5): > > mmc: core: Re-factor some code for SDIO re-initialization > > mmc: core: Rename ->reset() bus ops to ->hw_reset() > > mmc: core: Export a function mmc_sw_reset() to allow soft reset of > > cards > > mmc: core: Share internal function to set initial signal voltage > > mmc: core: Implement ->sw_reset bus ops for SDIO > > > > drivers/mmc/core/core.c | 49 +++++++++++++++++++++++++++++++++--------- > > drivers/mmc/core/core.h | 4 +++- > > drivers/mmc/core/mmc.c | 4 ++-- > > drivers/mmc/core/sd.c | 4 ++-- > > drivers/mmc/core/sdio.c | 56 +++++++++++++++++++++++++++++------------------- > > include/linux/mmc/core.h | 1 + > > 6 files changed, 81 insertions(+), 37 deletions(-) > >
Hi Ulf and Hans, On Fri, Apr 06, 2018 at 10:41:10AM +0200, Hans de Goede wrote: > Hi Ulf, > > On 05-04-18 22:19, Ulf Hansson wrote: > > It's rather common that SDIO func devices becomes loaded with a new firmware as > > a part of the SDIO func driver being probed. However, in some special scenarios > > the SDIO func device needs a SW reset, as to start running the new firmware. > > > > More importantly, a full power cycle doesn't work, as that would reset also the > > firmware, thus the existing mmc_hw_reset() API can't be used to deal with these > > scenarios. > > > > Therefore this series suggest to add a new API, mmc_sw_reset(), which resets > > and re-initialize the SDIO card. A couple of the patches in the series are > > mostly re-factorings making generic improvements to the related code. > > > > For more background to this series, feel free to digest the discussions from > > the submitted patch: https://patchwork.kernel.org/patch/9857175/ > > > > It should be noted, at this point this series has only be compile tested. Help > > with tests and deployment of using the new API is greatly appreciated. > > Thank you for this series, I've taken a quick look and it looks good, > but the proof is in the pudding. Quentin can you test this on an ARM > board with an ESP8089 sometime the coming week? > I applied your patches on top of next-20180406. I may need some help to get the ESP8089 driver to work. Note that I'm very new to MMC and SDIO, just so you know (and so you can adapt your answer). It's filling my kernel log buffer with a lot of these[1]. I've basically replaced Hans's mmc_force_detect_change() by your mmc_sw_reset() in esp_sdio_probe() of the ESP8089 driver and that's it. Here is my old patch series for ESP8089[2]. I can't say if it's an error in your patch series or me using it the wrong way. Could you help me debug this so that we can make the "weird" SDIO devices such as the ESP8089 work in the kernel :)? I'll try to mount my rootfs from something else than an initramfs so that it may solve my problem of dmesg not showing the whole log. [1] http://code.bulix.org/eechlb-318686 [2] https://lkml.org/lkml/2017/7/21/386 Let me know how I can be helpful, Thanks, Quentin
Hi, On 14-04-18 13:25, Quentin Schulz wrote: > Hi Ulf and Hans, > > On Fri, Apr 06, 2018 at 10:41:10AM +0200, Hans de Goede wrote: >> Hi Ulf, >> >> On 05-04-18 22:19, Ulf Hansson wrote: >>> It's rather common that SDIO func devices becomes loaded with a new firmware as >>> a part of the SDIO func driver being probed. However, in some special scenarios >>> the SDIO func device needs a SW reset, as to start running the new firmware. >>> >>> More importantly, a full power cycle doesn't work, as that would reset also the >>> firmware, thus the existing mmc_hw_reset() API can't be used to deal with these >>> scenarios. >>> >>> Therefore this series suggest to add a new API, mmc_sw_reset(), which resets >>> and re-initialize the SDIO card. A couple of the patches in the series are >>> mostly re-factorings making generic improvements to the related code. >>> >>> For more background to this series, feel free to digest the discussions from >>> the submitted patch: https://patchwork.kernel.org/patch/9857175/ >>> >>> It should be noted, at this point this series has only be compile tested. Help >>> with tests and deployment of using the new API is greatly appreciated. >> >> Thank you for this series, I've taken a quick look and it looks good, >> but the proof is in the pudding. Quentin can you test this on an ARM >> board with an ESP8089 sometime the coming week? >> > > I applied your patches on top of next-20180406. > > I may need some help to get the ESP8089 driver to work. Note that I'm > very new to MMC and SDIO, just so you know (and so you can adapt your > answer). > > It's filling my kernel log buffer with a lot of these[1]. I've basically > replaced Hans's mmc_force_detect_change() by your mmc_sw_reset() in > esp_sdio_probe() of the ESP8089 driver and that's it. Here is my old > patch series for ESP8089[2]. Previously, after my mmc_force_detect_change() the driver would see a disconnect/remove and then a new connect/probe, this will no longer happen, now after the mmc_sw_reset() you should let probe continue at there point where previously the second probe call would start. > I can't say if it's an error in your patch series or me using it the > wrong way. Well if you just replaced my mmc_force_detect_change() with mmc_sw_reset() that is not enough, see above. After that it might very well be that mc_sw_reset() needs some work too... Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Hans, On Sat, Apr 14, 2018 at 04:41:24PM +0200, Hans de Goede wrote: > Hi, > > On 14-04-18 13:25, Quentin Schulz wrote: > > Hi Ulf and Hans, > > > > On Fri, Apr 06, 2018 at 10:41:10AM +0200, Hans de Goede wrote: > > > Hi Ulf, > > > > > > On 05-04-18 22:19, Ulf Hansson wrote: > > > > It's rather common that SDIO func devices becomes loaded with a new firmware as > > > > a part of the SDIO func driver being probed. However, in some special scenarios > > > > the SDIO func device needs a SW reset, as to start running the new firmware. > > > > > > > > More importantly, a full power cycle doesn't work, as that would reset also the > > > > firmware, thus the existing mmc_hw_reset() API can't be used to deal with these > > > > scenarios. > > > > > > > > Therefore this series suggest to add a new API, mmc_sw_reset(), which resets > > > > and re-initialize the SDIO card. A couple of the patches in the series are > > > > mostly re-factorings making generic improvements to the related code. > > > > > > > > For more background to this series, feel free to digest the discussions from > > > > the submitted patch: https://patchwork.kernel.org/patch/9857175/ > > > > > > > > It should be noted, at this point this series has only be compile tested. Help > > > > with tests and deployment of using the new API is greatly appreciated. > > > > > > Thank you for this series, I've taken a quick look and it looks good, > > > but the proof is in the pudding. Quentin can you test this on an ARM > > > board with an ESP8089 sometime the coming week? > > > > > > > I applied your patches on top of next-20180406. > > > > I may need some help to get the ESP8089 driver to work. Note that I'm > > very new to MMC and SDIO, just so you know (and so you can adapt your > > answer). > > > > It's filling my kernel log buffer with a lot of these[1]. I've basically > > replaced Hans's mmc_force_detect_change() by your mmc_sw_reset() in > > esp_sdio_probe() of the ESP8089 driver and that's it. Here is my old > > patch series for ESP8089[2]. > > Previously, after my mmc_force_detect_change() the driver would > see a disconnect/remove and then a new connect/probe, this will > no longer happen, now after the mmc_sw_reset() you should let > probe continue at there point where previously the second probe > call would start. It's what I understood of it so I moved pieces specific to the second probe and put it after mmc_sw_reset(). I forgot to mention this modification in my previous mail. Anyway, the trace happens before reaching the second part of the probe is what I understood from the log attached in the previous mail. It looks like it's happening in mmc_sw_reset(). Am I wrongly assuming? > > I can't say if it's an error in your patch series or me using it the > > wrong way. > > Well if you just replaced my mmc_force_detect_change() with > mmc_sw_reset() that is not enough, see above. After that it might > very well be that mc_sw_reset() needs some work too... Best regards, Quentin
Hi, On 14-04-18 17:57, Quentin Schulz wrote: > Hi Hans, > > On Sat, Apr 14, 2018 at 04:41:24PM +0200, Hans de Goede wrote: >> Hi, >> >> On 14-04-18 13:25, Quentin Schulz wrote: >>> Hi Ulf and Hans, >>> >>> On Fri, Apr 06, 2018 at 10:41:10AM +0200, Hans de Goede wrote: >>>> Hi Ulf, >>>> >>>> On 05-04-18 22:19, Ulf Hansson wrote: >>>>> It's rather common that SDIO func devices becomes loaded with a new firmware as >>>>> a part of the SDIO func driver being probed. However, in some special scenarios >>>>> the SDIO func device needs a SW reset, as to start running the new firmware. >>>>> >>>>> More importantly, a full power cycle doesn't work, as that would reset also the >>>>> firmware, thus the existing mmc_hw_reset() API can't be used to deal with these >>>>> scenarios. >>>>> >>>>> Therefore this series suggest to add a new API, mmc_sw_reset(), which resets >>>>> and re-initialize the SDIO card. A couple of the patches in the series are >>>>> mostly re-factorings making generic improvements to the related code. >>>>> >>>>> For more background to this series, feel free to digest the discussions from >>>>> the submitted patch: https://patchwork.kernel.org/patch/9857175/ >>>>> >>>>> It should be noted, at this point this series has only be compile tested. Help >>>>> with tests and deployment of using the new API is greatly appreciated. >>>> >>>> Thank you for this series, I've taken a quick look and it looks good, >>>> but the proof is in the pudding. Quentin can you test this on an ARM >>>> board with an ESP8089 sometime the coming week? >>>> >>> >>> I applied your patches on top of next-20180406. >>> >>> I may need some help to get the ESP8089 driver to work. Note that I'm >>> very new to MMC and SDIO, just so you know (and so you can adapt your >>> answer). >>> >>> It's filling my kernel log buffer with a lot of these[1]. I've basically >>> replaced Hans's mmc_force_detect_change() by your mmc_sw_reset() in >>> esp_sdio_probe() of the ESP8089 driver and that's it. Here is my old >>> patch series for ESP8089[2]. >> >> Previously, after my mmc_force_detect_change() the driver would >> see a disconnect/remove and then a new connect/probe, this will >> no longer happen, now after the mmc_sw_reset() you should let >> probe continue at there point where previously the second probe >> call would start. > > It's what I understood of it so I moved pieces specific to the second > probe and put it after mmc_sw_reset(). I forgot to mention this > modification in my previous mail. Ah ok, yes that is how it should be used, so if it does not work then, then we need some more work on this. > Anyway, the trace happens before reaching the second part of the probe > is what I understood from the log attached in the previous mail. > It looks like it's happening in mmc_sw_reset(). Am I wrongly assuming? I have not looked into this at all, I just wanted to point out how the new function should be used. I'm sorry but I'm currently totally swamped with other stuff, so I don't have time to look into this. Hopefully Ulf can make some time to look at the errors you are getting. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 14 April 2018 at 13:25, Quentin Schulz <quentin.schulz@bootlin.com> wrote: > Hi Ulf and Hans, > > On Fri, Apr 06, 2018 at 10:41:10AM +0200, Hans de Goede wrote: >> Hi Ulf, >> >> On 05-04-18 22:19, Ulf Hansson wrote: >> > It's rather common that SDIO func devices becomes loaded with a new firmware as >> > a part of the SDIO func driver being probed. However, in some special scenarios >> > the SDIO func device needs a SW reset, as to start running the new firmware. >> > >> > More importantly, a full power cycle doesn't work, as that would reset also the >> > firmware, thus the existing mmc_hw_reset() API can't be used to deal with these >> > scenarios. >> > >> > Therefore this series suggest to add a new API, mmc_sw_reset(), which resets >> > and re-initialize the SDIO card. A couple of the patches in the series are >> > mostly re-factorings making generic improvements to the related code. >> > >> > For more background to this series, feel free to digest the discussions from >> > the submitted patch: https://patchwork.kernel.org/patch/9857175/ >> > >> > It should be noted, at this point this series has only be compile tested. Help >> > with tests and deployment of using the new API is greatly appreciated. >> >> Thank you for this series, I've taken a quick look and it looks good, >> but the proof is in the pudding. Quentin can you test this on an ARM >> board with an ESP8089 sometime the coming week? >> > > I applied your patches on top of next-20180406. > > I may need some help to get the ESP8089 driver to work. Note that I'm > very new to MMC and SDIO, just so you know (and so you can adapt your > answer). > > It's filling my kernel log buffer with a lot of these[1]. I've basically > replaced Hans's mmc_force_detect_change() by your mmc_sw_reset() in > esp_sdio_probe() of the ESP8089 driver and that's it. Here is my old > patch series for ESP8089[2]. > > I can't say if it's an error in your patch series or me using it the > wrong way. > > Could you help me debug this so that we can make the "weird" SDIO > devices such as the ESP8089 work in the kernel :)? > > I'll try to mount my rootfs from something else than an initramfs so > that it may solve my problem of dmesg not showing the whole log. > > [1] http://code.bulix.org/eechlb-318686 Unless I am misstaken, it looks like the messages comes from a WARN_ON(!host->claimed); in mmc_wait_for_cmd(). Did you call sdio_claim_host() before calling mmc_sw_reset()? And then of course you need to release the host when you are ready executing the required operations, sdio_release_host(). > [2] https://lkml.org/lkml/2017/7/21/386 > > Let me know how I can be helpful, > Thanks, > Quentin Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 23 April 2018 at 11:50, Ulf Hansson <ulf.hansson@linaro.org> wrote: > On 14 April 2018 at 13:25, Quentin Schulz <quentin.schulz@bootlin.com> wrote: >> Hi Ulf and Hans, >> >> On Fri, Apr 06, 2018 at 10:41:10AM +0200, Hans de Goede wrote: >>> Hi Ulf, >>> >>> On 05-04-18 22:19, Ulf Hansson wrote: >>> > It's rather common that SDIO func devices becomes loaded with a new firmware as >>> > a part of the SDIO func driver being probed. However, in some special scenarios >>> > the SDIO func device needs a SW reset, as to start running the new firmware. >>> > >>> > More importantly, a full power cycle doesn't work, as that would reset also the >>> > firmware, thus the existing mmc_hw_reset() API can't be used to deal with these >>> > scenarios. >>> > >>> > Therefore this series suggest to add a new API, mmc_sw_reset(), which resets >>> > and re-initialize the SDIO card. A couple of the patches in the series are >>> > mostly re-factorings making generic improvements to the related code. >>> > >>> > For more background to this series, feel free to digest the discussions from >>> > the submitted patch: https://patchwork.kernel.org/patch/9857175/ >>> > >>> > It should be noted, at this point this series has only be compile tested. Help >>> > with tests and deployment of using the new API is greatly appreciated. >>> >>> Thank you for this series, I've taken a quick look and it looks good, >>> but the proof is in the pudding. Quentin can you test this on an ARM >>> board with an ESP8089 sometime the coming week? >>> >> >> I applied your patches on top of next-20180406. >> >> I may need some help to get the ESP8089 driver to work. Note that I'm >> very new to MMC and SDIO, just so you know (and so you can adapt your >> answer). >> >> It's filling my kernel log buffer with a lot of these[1]. I've basically >> replaced Hans's mmc_force_detect_change() by your mmc_sw_reset() in >> esp_sdio_probe() of the ESP8089 driver and that's it. Here is my old >> patch series for ESP8089[2]. >> >> I can't say if it's an error in your patch series or me using it the >> wrong way. >> >> Could you help me debug this so that we can make the "weird" SDIO >> devices such as the ESP8089 work in the kernel :)? >> >> I'll try to mount my rootfs from something else than an initramfs so >> that it may solve my problem of dmesg not showing the whole log. >> >> [1] http://code.bulix.org/eechlb-318686 > > Unless I am misstaken, it looks like the messages comes from a > WARN_ON(!host->claimed); in mmc_wait_for_cmd(). > > Did you call sdio_claim_host() before calling mmc_sw_reset()? And then > of course you need to release the host when you are ready executing > the required operations, sdio_release_host(). > >> [2] https://lkml.org/lkml/2017/7/21/386 >> >> Let me know how I can be helpful, >> Thanks, >> Quentin Quentin, ping! Just tell me if there is anything else I can help out with! Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ulf, On Wed, May 02, 2018 at 11:26:30AM +0200, Ulf Hansson wrote: > On 23 April 2018 at 11:50, Ulf Hansson <ulf.hansson@linaro.org> wrote: > > On 14 April 2018 at 13:25, Quentin Schulz <quentin.schulz@bootlin.com> wrote: > >> Hi Ulf and Hans, > >> > >> On Fri, Apr 06, 2018 at 10:41:10AM +0200, Hans de Goede wrote: > >>> Hi Ulf, > >>> > >>> On 05-04-18 22:19, Ulf Hansson wrote: > >>> > It's rather common that SDIO func devices becomes loaded with a new firmware as > >>> > a part of the SDIO func driver being probed. However, in some special scenarios > >>> > the SDIO func device needs a SW reset, as to start running the new firmware. > >>> > > >>> > More importantly, a full power cycle doesn't work, as that would reset also the > >>> > firmware, thus the existing mmc_hw_reset() API can't be used to deal with these > >>> > scenarios. > >>> > > >>> > Therefore this series suggest to add a new API, mmc_sw_reset(), which resets > >>> > and re-initialize the SDIO card. A couple of the patches in the series are > >>> > mostly re-factorings making generic improvements to the related code. > >>> > > >>> > For more background to this series, feel free to digest the discussions from > >>> > the submitted patch: https://patchwork.kernel.org/patch/9857175/ > >>> > > >>> > It should be noted, at this point this series has only be compile tested. Help > >>> > with tests and deployment of using the new API is greatly appreciated. > >>> > >>> Thank you for this series, I've taken a quick look and it looks good, > >>> but the proof is in the pudding. Quentin can you test this on an ARM > >>> board with an ESP8089 sometime the coming week? > >>> > >> > >> I applied your patches on top of next-20180406. > >> > >> I may need some help to get the ESP8089 driver to work. Note that I'm > >> very new to MMC and SDIO, just so you know (and so you can adapt your > >> answer). > >> > >> It's filling my kernel log buffer with a lot of these[1]. I've basically > >> replaced Hans's mmc_force_detect_change() by your mmc_sw_reset() in > >> esp_sdio_probe() of the ESP8089 driver and that's it. Here is my old > >> patch series for ESP8089[2]. > >> > >> I can't say if it's an error in your patch series or me using it the > >> wrong way. > >> > >> Could you help me debug this so that we can make the "weird" SDIO > >> devices such as the ESP8089 work in the kernel :)? > >> > >> I'll try to mount my rootfs from something else than an initramfs so > >> that it may solve my problem of dmesg not showing the whole log. > >> > >> [1] http://code.bulix.org/eechlb-318686 > > > > Unless I am misstaken, it looks like the messages comes from a > > WARN_ON(!host->claimed); in mmc_wait_for_cmd(). > > > > Did you call sdio_claim_host() before calling mmc_sw_reset()? And then > > of course you need to release the host when you are ready executing > > the required operations, sdio_release_host(). > > > >> [2] https://lkml.org/lkml/2017/7/21/386 > >> > >> Let me know how I can be helpful, > >> Thanks, > >> Quentin > > Quentin, ping! > > Just tell me if there is anything else I can help out with! > Thanks for the ping, I'll have a look tomorrow evening or this week-end. Looks weird to me that using this patch in the core resulted in an unbalanced sdio_claim/release_host() while it was working with Hans's hack. I'll let you know if I'm getting lost :) Thanks, Quentin > Kind regards > Uffe
Hi Ulf, On Wed, May 02, 2018 at 11:26:30AM +0200, Ulf Hansson wrote: > On 23 April 2018 at 11:50, Ulf Hansson <ulf.hansson@linaro.org> wrote: > > On 14 April 2018 at 13:25, Quentin Schulz <quentin.schulz@bootlin.com> wrote: > >> Hi Ulf and Hans, > >> > >> On Fri, Apr 06, 2018 at 10:41:10AM +0200, Hans de Goede wrote: > >>> Hi Ulf, > >>> > >>> On 05-04-18 22:19, Ulf Hansson wrote: > >>> > It's rather common that SDIO func devices becomes loaded with a new firmware as > >>> > a part of the SDIO func driver being probed. However, in some special scenarios > >>> > the SDIO func device needs a SW reset, as to start running the new firmware. > >>> > > >>> > More importantly, a full power cycle doesn't work, as that would reset also the > >>> > firmware, thus the existing mmc_hw_reset() API can't be used to deal with these > >>> > scenarios. > >>> > > >>> > Therefore this series suggest to add a new API, mmc_sw_reset(), which resets > >>> > and re-initialize the SDIO card. A couple of the patches in the series are > >>> > mostly re-factorings making generic improvements to the related code. > >>> > > >>> > For more background to this series, feel free to digest the discussions from > >>> > the submitted patch: https://patchwork.kernel.org/patch/9857175/ > >>> > > >>> > It should be noted, at this point this series has only be compile tested. Help > >>> > with tests and deployment of using the new API is greatly appreciated. > >>> > >>> Thank you for this series, I've taken a quick look and it looks good, > >>> but the proof is in the pudding. Quentin can you test this on an ARM > >>> board with an ESP8089 sometime the coming week? > >>> > >> > >> I applied your patches on top of next-20180406. > >> > >> I may need some help to get the ESP8089 driver to work. Note that I'm > >> very new to MMC and SDIO, just so you know (and so you can adapt your > >> answer). > >> > >> It's filling my kernel log buffer with a lot of these[1]. I've basically > >> replaced Hans's mmc_force_detect_change() by your mmc_sw_reset() in > >> esp_sdio_probe() of the ESP8089 driver and that's it. Here is my old > >> patch series for ESP8089[2]. > >> > >> I can't say if it's an error in your patch series or me using it the > >> wrong way. > >> > >> Could you help me debug this so that we can make the "weird" SDIO > >> devices such as the ESP8089 work in the kernel :)? > >> > >> I'll try to mount my rootfs from something else than an initramfs so > >> that it may solve my problem of dmesg not showing the whole log. > >> > >> [1] http://code.bulix.org/eechlb-318686 > > > > Unless I am misstaken, it looks like the messages comes from a > > WARN_ON(!host->claimed); in mmc_wait_for_cmd(). > > > > Did you call sdio_claim_host() before calling mmc_sw_reset()? And then > > of course you need to release the host when you are ready executing > > the required operations, sdio_release_host(). > > That was it, thanks. Now, I go pass mmc_sw_reset and the init of the driver can continue but I'm now hitting a timeout waiting for a completion. I'll need definitely to put more time into it to debug it now. Next week has two holidays in France though, I'll try my best to find time to work on it next week but can't promise much work will be done. If anyone wants to help meanwhile, here is the code: https://github.com/QSchulz/linux/tree/mmc_sw_reset/esp8089 This is the bootlog of the tablet: http://code.bulix.org/t7iken-328962 Hope we can get this sorted out somehow quickly so we can just put this issue of weird SDIO devices behind us once and for all :) (well, until the next weird ones :D). Thanks, Quentin > >> [2] https://lkml.org/lkml/2017/7/21/386 > >> > >> Let me know how I can be helpful, > >> Thanks, > >> Quentin > > Quentin, ping! > > Just tell me if there is anything else I can help out with! > > Kind regards > Uffe
On 2018/5/6 22:01, Quentin Schulz wrote: > Hi Ulf, > > On Wed, May 02, 2018 at 11:26:30AM +0200, Ulf Hansson wrote: >> On 23 April 2018 at 11:50, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>> On 14 April 2018 at 13:25, Quentin Schulz <quentin.schulz@bootlin.com> wrote: >>>> Hi Ulf and Hans, >>>> >>>> On Fri, Apr 06, 2018 at 10:41:10AM +0200, Hans de Goede wrote: >>>>> Hi Ulf, >>>>> >>>>> On 05-04-18 22:19, Ulf Hansson wrote: >>>>>> It's rather common that SDIO func devices becomes loaded with a new firmware as >>>>>> a part of the SDIO func driver being probed. However, in some special scenarios >>>>>> the SDIO func device needs a SW reset, as to start running the new firmware. >>>>>> >>>>>> More importantly, a full power cycle doesn't work, as that would reset also the >>>>>> firmware, thus the existing mmc_hw_reset() API can't be used to deal with these >>>>>> scenarios. >>>>>> >>>>>> Therefore this series suggest to add a new API, mmc_sw_reset(), which resets >>>>>> and re-initialize the SDIO card. A couple of the patches in the series are >>>>>> mostly re-factorings making generic improvements to the related code. >>>>>> >>>>>> For more background to this series, feel free to digest the discussions from >>>>>> the submitted patch: https://patchwork.kernel.org/patch/9857175/ >>>>>> >>>>>> It should be noted, at this point this series has only be compile tested. Help >>>>>> with tests and deployment of using the new API is greatly appreciated. >>>>> >>>>> Thank you for this series, I've taken a quick look and it looks good, >>>>> but the proof is in the pudding. Quentin can you test this on an ARM >>>>> board with an ESP8089 sometime the coming week? >>>>> >>>> >>>> I applied your patches on top of next-20180406. >>>> >>>> I may need some help to get the ESP8089 driver to work. Note that I'm >>>> very new to MMC and SDIO, just so you know (and so you can adapt your >>>> answer). >>>> >>>> It's filling my kernel log buffer with a lot of these[1]. I've basically >>>> replaced Hans's mmc_force_detect_change() by your mmc_sw_reset() in >>>> esp_sdio_probe() of the ESP8089 driver and that's it. Here is my old >>>> patch series for ESP8089[2]. >>>> >>>> I can't say if it's an error in your patch series or me using it the >>>> wrong way. >>>> >>>> Could you help me debug this so that we can make the "weird" SDIO >>>> devices such as the ESP8089 work in the kernel :)? >>>> >>>> I'll try to mount my rootfs from something else than an initramfs so >>>> that it may solve my problem of dmesg not showing the whole log. >>>> >>>> [1] http://code.bulix.org/eechlb-318686 >>> >>> Unless I am misstaken, it looks like the messages comes from a >>> WARN_ON(!host->claimed); in mmc_wait_for_cmd(). >>> >>> Did you call sdio_claim_host() before calling mmc_sw_reset()? And then >>> of course you need to release the host when you are ready executing >>> the required operations, sdio_release_host(). >>> > > That was it, thanks. > > Now, I go pass mmc_sw_reset and the init of the driver can continue but > I'm now hitting a timeout waiting for a completion. > > I'll need definitely to put more time into it to debug it now. Next week > has two holidays in France though, I'll try my best to find time to work > on it next week but can't promise much work will be done. > > If anyone wants to help meanwhile, here is the code: > https://github.com/QSchulz/linux/tree/mmc_sw_reset/esp8089 > > This is the bootlog of the tablet: > http://code.bulix.org/t7iken-328962 Could you try: drivers/net/wireless/espressif/esp8089/sdio_sif_esp.c -543,21 +530,61 @@ static int esp_sdio_probe(struct sdio_func *func, sdio_claim_host(func); + sdio_release_irq(func); mmc_sw_reset(host); sdio_release_host(func); > > Hope we can get this sorted out somehow quickly so we can just put this > issue of weird SDIO devices behind us once and for all :) (well, until > the next weird ones :D). > > Thanks, > Quentin > >>>> [2] https://lkml.org/lkml/2017/7/21/386 >>>> >>>> Let me know how I can be helpful, >>>> Thanks, >>>> Quentin >> >> Quentin, ping! >> >> Just tell me if there is anything else I can help out with! >> >> Kind regards >> Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Shawn, On Mon, May 07, 2018 at 03:29:31PM +0800, Shawn Lin wrote: > On 2018/5/6 22:01, Quentin Schulz wrote: > > Hi Ulf, > > > > On Wed, May 02, 2018 at 11:26:30AM +0200, Ulf Hansson wrote: > > > On 23 April 2018 at 11:50, Ulf Hansson <ulf.hansson@linaro.org> wrote: > > > > On 14 April 2018 at 13:25, Quentin Schulz <quentin.schulz@bootlin.com> wrote: > > > > > Hi Ulf and Hans, > > > > > > > > > > On Fri, Apr 06, 2018 at 10:41:10AM +0200, Hans de Goede wrote: > > > > > > Hi Ulf, > > > > > > > > > > > > On 05-04-18 22:19, Ulf Hansson wrote: > > > > > > > It's rather common that SDIO func devices becomes loaded with a new firmware as > > > > > > > a part of the SDIO func driver being probed. However, in some special scenarios > > > > > > > the SDIO func device needs a SW reset, as to start running the new firmware. > > > > > > > > > > > > > > More importantly, a full power cycle doesn't work, as that would reset also the > > > > > > > firmware, thus the existing mmc_hw_reset() API can't be used to deal with these > > > > > > > scenarios. > > > > > > > > > > > > > > Therefore this series suggest to add a new API, mmc_sw_reset(), which resets > > > > > > > and re-initialize the SDIO card. A couple of the patches in the series are > > > > > > > mostly re-factorings making generic improvements to the related code. > > > > > > > > > > > > > > For more background to this series, feel free to digest the discussions from > > > > > > > the submitted patch: https://patchwork.kernel.org/patch/9857175/ > > > > > > > > > > > > > > It should be noted, at this point this series has only be compile tested. Help > > > > > > > with tests and deployment of using the new API is greatly appreciated. > > > > > > > > > > > > Thank you for this series, I've taken a quick look and it looks good, > > > > > > but the proof is in the pudding. Quentin can you test this on an ARM > > > > > > board with an ESP8089 sometime the coming week? > > > > > > > > > > > > > > > > I applied your patches on top of next-20180406. > > > > > > > > > > I may need some help to get the ESP8089 driver to work. Note that I'm > > > > > very new to MMC and SDIO, just so you know (and so you can adapt your > > > > > answer). > > > > > > > > > > It's filling my kernel log buffer with a lot of these[1]. I've basically > > > > > replaced Hans's mmc_force_detect_change() by your mmc_sw_reset() in > > > > > esp_sdio_probe() of the ESP8089 driver and that's it. Here is my old > > > > > patch series for ESP8089[2]. > > > > > > > > > > I can't say if it's an error in your patch series or me using it the > > > > > wrong way. > > > > > > > > > > Could you help me debug this so that we can make the "weird" SDIO > > > > > devices such as the ESP8089 work in the kernel :)? > > > > > > > > > > I'll try to mount my rootfs from something else than an initramfs so > > > > > that it may solve my problem of dmesg not showing the whole log. > > > > > > > > > > [1] http://code.bulix.org/eechlb-318686 > > > > > > > > Unless I am misstaken, it looks like the messages comes from a > > > > WARN_ON(!host->claimed); in mmc_wait_for_cmd(). > > > > > > > > Did you call sdio_claim_host() before calling mmc_sw_reset()? And then > > > > of course you need to release the host when you are ready executing > > > > the required operations, sdio_release_host(). > > > > > > > > That was it, thanks. > > > > Now, I go pass mmc_sw_reset and the init of the driver can continue but > > I'm now hitting a timeout waiting for a completion. > > > > I'll need definitely to put more time into it to debug it now. Next week > > has two holidays in France though, I'll try my best to find time to work > > on it next week but can't promise much work will be done. > > > > If anyone wants to help meanwhile, here is the code: > > https://github.com/QSchulz/linux/tree/mmc_sw_reset/esp8089 > > > > This is the bootlog of the tablet: > > http://code.bulix.org/t7iken-328962 > > Could you try: > > drivers/net/wireless/espressif/esp8089/sdio_sif_esp.c > -543,21 +530,61 @@ static int esp_sdio_probe(struct sdio_func *func, > sdio_claim_host(func); > + sdio_release_irq(func); > mmc_sw_reset(host); > sdio_release_host(func); > I don't have the tablet with me at work, I'll let you know the output of this modification in about 9 hours. I think you're offering this patch because of the "sif sif_enable_irq failed" debug message which calls sdio_claim_irq and fails. Here sdio_claim_irq() fails at https://elixir.bootlin.com/linux/latest/source/drivers/mmc/core/sdio_irq.c#L294 In the code of the ESP8089 driver, failing to claim the IRQ isn't blocking and I don't think it was disabled or something was done in SDIO subsystem on this IRQ or its handler. Anyway, anything's worth trying :) I'll let you know, thanks, Quentin > > > > Hope we can get this sorted out somehow quickly so we can just put this > > issue of weird SDIO devices behind us once and for all :) (well, until > > the next weird ones :D). > > > > Thanks, > > Quentin > > > > > > > [2] https://lkml.org/lkml/2017/7/21/386 > > > > > > > > > > Let me know how I can be helpful, > > > > > Thanks, > > > > > Quentin > > > > > > Quentin, ping! > > > > > > Just tell me if there is anything else I can help out with! > > > > > > Kind regards > > > Uffe >
On 2018/5/7 15:36, Quentin Schulz wrote: > Hi Shawn, > > On Mon, May 07, 2018 at 03:29:31PM +0800, Shawn Lin wrote: >> On 2018/5/6 22:01, Quentin Schulz wrote: >>> Hi Ulf, >>> >>> On Wed, May 02, 2018 at 11:26:30AM +0200, Ulf Hansson wrote: >>>> On 23 April 2018 at 11:50, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>>>> On 14 April 2018 at 13:25, Quentin Schulz <quentin.schulz@bootlin.com> wrote: >>>>>> Hi Ulf and Hans, >>>>>> >>>>>> On Fri, Apr 06, 2018 at 10:41:10AM +0200, Hans de Goede wrote: >>>>>>> Hi Ulf, >>>>>>> >>>>>>> On 05-04-18 22:19, Ulf Hansson wrote: >>>>>>>> It's rather common that SDIO func devices becomes loaded with a new firmware as >>>>>>>> a part of the SDIO func driver being probed. However, in some special scenarios >>>>>>>> the SDIO func device needs a SW reset, as to start running the new firmware. >>>>>>>> >>>>>>>> More importantly, a full power cycle doesn't work, as that would reset also the >>>>>>>> firmware, thus the existing mmc_hw_reset() API can't be used to deal with these >>>>>>>> scenarios. >>>>>>>> >>>>>>>> Therefore this series suggest to add a new API, mmc_sw_reset(), which resets >>>>>>>> and re-initialize the SDIO card. A couple of the patches in the series are >>>>>>>> mostly re-factorings making generic improvements to the related code. >>>>>>>> >>>>>>>> For more background to this series, feel free to digest the discussions from >>>>>>>> the submitted patch: https://patchwork.kernel.org/patch/9857175/ >>>>>>>> >>>>>>>> It should be noted, at this point this series has only be compile tested. Help >>>>>>>> with tests and deployment of using the new API is greatly appreciated. >>>>>>> >>>>>>> Thank you for this series, I've taken a quick look and it looks good, >>>>>>> but the proof is in the pudding. Quentin can you test this on an ARM >>>>>>> board with an ESP8089 sometime the coming week? >>>>>>> >>>>>> >>>>>> I applied your patches on top of next-20180406. >>>>>> >>>>>> I may need some help to get the ESP8089 driver to work. Note that I'm >>>>>> very new to MMC and SDIO, just so you know (and so you can adapt your >>>>>> answer). >>>>>> >>>>>> It's filling my kernel log buffer with a lot of these[1]. I've basically >>>>>> replaced Hans's mmc_force_detect_change() by your mmc_sw_reset() in >>>>>> esp_sdio_probe() of the ESP8089 driver and that's it. Here is my old >>>>>> patch series for ESP8089[2]. >>>>>> >>>>>> I can't say if it's an error in your patch series or me using it the >>>>>> wrong way. >>>>>> >>>>>> Could you help me debug this so that we can make the "weird" SDIO >>>>>> devices such as the ESP8089 work in the kernel :)? >>>>>> >>>>>> I'll try to mount my rootfs from something else than an initramfs so >>>>>> that it may solve my problem of dmesg not showing the whole log. >>>>>> >>>>>> [1] http://code.bulix.org/eechlb-318686 >>>>> >>>>> Unless I am misstaken, it looks like the messages comes from a >>>>> WARN_ON(!host->claimed); in mmc_wait_for_cmd(). >>>>> >>>>> Did you call sdio_claim_host() before calling mmc_sw_reset()? And then >>>>> of course you need to release the host when you are ready executing >>>>> the required operations, sdio_release_host(). >>>>> >>> >>> That was it, thanks. >>> >>> Now, I go pass mmc_sw_reset and the init of the driver can continue but >>> I'm now hitting a timeout waiting for a completion. >>> >>> I'll need definitely to put more time into it to debug it now. Next week >>> has two holidays in France though, I'll try my best to find time to work >>> on it next week but can't promise much work will be done. >>> >>> If anyone wants to help meanwhile, here is the code: >>> https://github.com/QSchulz/linux/tree/mmc_sw_reset/esp8089 >>> >>> This is the bootlog of the tablet: >>> http://code.bulix.org/t7iken-328962 >> >> Could you try: >> >> drivers/net/wireless/espressif/esp8089/sdio_sif_esp.c >> -543,21 +530,61 @@ static int esp_sdio_probe(struct sdio_func *func, >> sdio_claim_host(func); >> + sdio_release_irq(func); >> mmc_sw_reset(host); >> sdio_release_host(func); >> > > I don't have the tablet with me at work, I'll let you know the output of > this modification in about 9 hours. > > I think you're offering this patch because of the "sif sif_enable_irq > failed" debug message which calls sdio_claim_irq and fails. > Here sdio_claim_irq() fails at > https://elixir.bootlin.com/linux/latest/source/drivers/mmc/core/sdio_irq.c#L294 Right. I think the best thing to do here is to play whac-a-mole. :) > > In the code of the ESP8089 driver, failing to claim the IRQ isn't > blocking and I don't think it was disabled or something was done in SDIO > subsystem on this IRQ or its handler. Anyway, anything's worth trying :) > > I'll let you know, thanks, > > Quentin > >>> >>> Hope we can get this sorted out somehow quickly so we can just put this >>> issue of weird SDIO devices behind us once and for all :) (well, until >>> the next weird ones :D). >>> >>> Thanks, >>> Quentin >>> >>>>>> [2] https://lkml.org/lkml/2017/7/21/386 >>>>>> >>>>>> Let me know how I can be helpful, >>>>>> Thanks, >>>>>> Quentin >>>> >>>> Quentin, ping! >>>> >>>> Just tell me if there is anything else I can help out with! >>>> >>>> Kind regards >>>> Uffe >> -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Shawn, On Mon, May 07, 2018 at 03:29:31PM +0800, Shawn Lin wrote: > On 2018/5/6 22:01, Quentin Schulz wrote: > > Hi Ulf, > > > > On Wed, May 02, 2018 at 11:26:30AM +0200, Ulf Hansson wrote: > > > On 23 April 2018 at 11:50, Ulf Hansson <ulf.hansson@linaro.org> wrote: > > > > On 14 April 2018 at 13:25, Quentin Schulz <quentin.schulz@bootlin.com> wrote: > > > > > Hi Ulf and Hans, > > > > > > > > > > On Fri, Apr 06, 2018 at 10:41:10AM +0200, Hans de Goede wrote: > > > > > > Hi Ulf, > > > > > > > > > > > > On 05-04-18 22:19, Ulf Hansson wrote: > > > > > > > It's rather common that SDIO func devices becomes loaded with a new firmware as > > > > > > > a part of the SDIO func driver being probed. However, in some special scenarios > > > > > > > the SDIO func device needs a SW reset, as to start running the new firmware. > > > > > > > > > > > > > > More importantly, a full power cycle doesn't work, as that would reset also the > > > > > > > firmware, thus the existing mmc_hw_reset() API can't be used to deal with these > > > > > > > scenarios. > > > > > > > > > > > > > > Therefore this series suggest to add a new API, mmc_sw_reset(), which resets > > > > > > > and re-initialize the SDIO card. A couple of the patches in the series are > > > > > > > mostly re-factorings making generic improvements to the related code. > > > > > > > > > > > > > > For more background to this series, feel free to digest the discussions from > > > > > > > the submitted patch: https://patchwork.kernel.org/patch/9857175/ > > > > > > > > > > > > > > It should be noted, at this point this series has only be compile tested. Help > > > > > > > with tests and deployment of using the new API is greatly appreciated. > > > > > > > > > > > > Thank you for this series, I've taken a quick look and it looks good, > > > > > > but the proof is in the pudding. Quentin can you test this on an ARM > > > > > > board with an ESP8089 sometime the coming week? > > > > > > > > > > > > > > > > I applied your patches on top of next-20180406. > > > > > > > > > > I may need some help to get the ESP8089 driver to work. Note that I'm > > > > > very new to MMC and SDIO, just so you know (and so you can adapt your > > > > > answer). > > > > > > > > > > It's filling my kernel log buffer with a lot of these[1]. I've basically > > > > > replaced Hans's mmc_force_detect_change() by your mmc_sw_reset() in > > > > > esp_sdio_probe() of the ESP8089 driver and that's it. Here is my old > > > > > patch series for ESP8089[2]. > > > > > > > > > > I can't say if it's an error in your patch series or me using it the > > > > > wrong way. > > > > > > > > > > Could you help me debug this so that we can make the "weird" SDIO > > > > > devices such as the ESP8089 work in the kernel :)? > > > > > > > > > > I'll try to mount my rootfs from something else than an initramfs so > > > > > that it may solve my problem of dmesg not showing the whole log. > > > > > > > > > > [1] http://code.bulix.org/eechlb-318686 > > > > > > > > Unless I am misstaken, it looks like the messages comes from a > > > > WARN_ON(!host->claimed); in mmc_wait_for_cmd(). > > > > > > > > Did you call sdio_claim_host() before calling mmc_sw_reset()? And then > > > > of course you need to release the host when you are ready executing > > > > the required operations, sdio_release_host(). > > > > > > > > That was it, thanks. > > > > Now, I go pass mmc_sw_reset and the init of the driver can continue but > > I'm now hitting a timeout waiting for a completion. > > > > I'll need definitely to put more time into it to debug it now. Next week > > has two holidays in France though, I'll try my best to find time to work > > on it next week but can't promise much work will be done. > > > > If anyone wants to help meanwhile, here is the code: > > https://github.com/QSchulz/linux/tree/mmc_sw_reset/esp8089 > > > > This is the bootlog of the tablet: > > http://code.bulix.org/t7iken-328962 > > Could you try: > > drivers/net/wireless/espressif/esp8089/sdio_sif_esp.c > -543,21 +530,61 @@ static int esp_sdio_probe(struct sdio_func *func, > sdio_claim_host(func); > + sdio_release_irq(func); > mmc_sw_reset(host); > sdio_release_host(func); > You rock :) That made it work despite my wrong assumptions. Thanks a lot! We've got ESP8089 working without a dirty hack \o/ (well at least not on the core side :) ). Ulf, I'm very happy (and relieved) to give my Tested-by: Quentin Schulz <quentin.schulz@bootlin.com> Thanks a ton for the patch series, I can now go back to cleaning up the WiFi driver :) Quentin > > > > Hope we can get this sorted out somehow quickly so we can just put this > > issue of weird SDIO devices behind us once and for all :) (well, until > > the next weird ones :D). > > > > Thanks, > > Quentin > > > > > > > [2] https://lkml.org/lkml/2017/7/21/386 > > > > > > > > > > Let me know how I can be helpful, > > > > > Thanks, > > > > > Quentin > > > > > > Quentin, ping! > > > > > > Just tell me if there is anything else I can help out with! > > > > > > Kind regards > > > Uffe >
On 2018/5/8 1:32, Quentin Schulz wrote: > Shawn, > > On Mon, May 07, 2018 at 03:29:31PM +0800, Shawn Lin wrote: >> On 2018/5/6 22:01, Quentin Schulz wrote: >>> Hi Ulf, >>> >>> On Wed, May 02, 2018 at 11:26:30AM +0200, Ulf Hansson wrote: >>>> On 23 April 2018 at 11:50, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>>>> On 14 April 2018 at 13:25, Quentin Schulz <quentin.schulz@bootlin.com> wrote: >>>>>> Hi Ulf and Hans, >>>>>> >>>>>> On Fri, Apr 06, 2018 at 10:41:10AM +0200, Hans de Goede wrote: >>>>>>> Hi Ulf, >>>>>>> >>>>>>> On 05-04-18 22:19, Ulf Hansson wrote: >>>>>>>> It's rather common that SDIO func devices becomes loaded with a new firmware as >>>>>>>> a part of the SDIO func driver being probed. However, in some special scenarios >>>>>>>> the SDIO func device needs a SW reset, as to start running the new firmware. >>>>>>>> >>>>>>>> More importantly, a full power cycle doesn't work, as that would reset also the >>>>>>>> firmware, thus the existing mmc_hw_reset() API can't be used to deal with these >>>>>>>> scenarios. >>>>>>>> >>>>>>>> Therefore this series suggest to add a new API, mmc_sw_reset(), which resets >>>>>>>> and re-initialize the SDIO card. A couple of the patches in the series are >>>>>>>> mostly re-factorings making generic improvements to the related code. >>>>>>>> >>>>>>>> For more background to this series, feel free to digest the discussions from >>>>>>>> the submitted patch: https://patchwork.kernel.org/patch/9857175/ >>>>>>>> >>>>>>>> It should be noted, at this point this series has only be compile tested. Help >>>>>>>> with tests and deployment of using the new API is greatly appreciated. >>>>>>> >>>>>>> Thank you for this series, I've taken a quick look and it looks good, >>>>>>> but the proof is in the pudding. Quentin can you test this on an ARM >>>>>>> board with an ESP8089 sometime the coming week? >>>>>>> >>>>>> >>>>>> I applied your patches on top of next-20180406. >>>>>> >>>>>> I may need some help to get the ESP8089 driver to work. Note that I'm >>>>>> very new to MMC and SDIO, just so you know (and so you can adapt your >>>>>> answer). >>>>>> >>>>>> It's filling my kernel log buffer with a lot of these[1]. I've basically >>>>>> replaced Hans's mmc_force_detect_change() by your mmc_sw_reset() in >>>>>> esp_sdio_probe() of the ESP8089 driver and that's it. Here is my old >>>>>> patch series for ESP8089[2]. >>>>>> >>>>>> I can't say if it's an error in your patch series or me using it the >>>>>> wrong way. >>>>>> >>>>>> Could you help me debug this so that we can make the "weird" SDIO >>>>>> devices such as the ESP8089 work in the kernel :)? >>>>>> >>>>>> I'll try to mount my rootfs from something else than an initramfs so >>>>>> that it may solve my problem of dmesg not showing the whole log. >>>>>> >>>>>> [1] http://code.bulix.org/eechlb-318686 >>>>> >>>>> Unless I am misstaken, it looks like the messages comes from a >>>>> WARN_ON(!host->claimed); in mmc_wait_for_cmd(). >>>>> >>>>> Did you call sdio_claim_host() before calling mmc_sw_reset()? And then >>>>> of course you need to release the host when you are ready executing >>>>> the required operations, sdio_release_host(). >>>>> >>> >>> That was it, thanks. >>> >>> Now, I go pass mmc_sw_reset and the init of the driver can continue but >>> I'm now hitting a timeout waiting for a completion. >>> >>> I'll need definitely to put more time into it to debug it now. Next week >>> has two holidays in France though, I'll try my best to find time to work >>> on it next week but can't promise much work will be done. >>> >>> If anyone wants to help meanwhile, here is the code: >>> https://github.com/QSchulz/linux/tree/mmc_sw_reset/esp8089 >>> >>> This is the bootlog of the tablet: >>> http://code.bulix.org/t7iken-328962 >> >> Could you try: >> >> drivers/net/wireless/espressif/esp8089/sdio_sif_esp.c >> -543,21 +530,61 @@ static int esp_sdio_probe(struct sdio_func *func, >> sdio_claim_host(func); >> + sdio_release_irq(func); >> mmc_sw_reset(host); >> sdio_release_host(func); >> > > You rock :) That made it work despite my wrong assumptions. Thanks a > lot! > > We've got ESP8089 working without a dirty hack \o/ (well at least not on > the core side :) ). > Good to know that. :) The whole series looks good to me, Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com> > Ulf, I'm very happy (and relieved) to give my > > Tested-by: Quentin Schulz <quentin.schulz@bootlin.com> > > Thanks a ton for the patch series, I can now go back to cleaning up the > WiFi driver :) > > Quentin > >>> >>> Hope we can get this sorted out somehow quickly so we can just put this >>> issue of weird SDIO devices behind us once and for all :) (well, until >>> the next weird ones :D). >>> >>> Thanks, >>> Quentin >>> >>>>>> [2] https://lkml.org/lkml/2017/7/21/386 >>>>>> >>>>>> Let me know how I can be helpful, >>>>>> Thanks, >>>>>> Quentin >>>> >>>> Quentin, ping! >>>> >>>> Just tell me if there is anything else I can help out with! >>>> >>>> Kind regards >>>> Uffe >> -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On 07-05-18 19:32, Quentin Schulz wrote: > Shawn, > > On Mon, May 07, 2018 at 03:29:31PM +0800, Shawn Lin wrote: >> On 2018/5/6 22:01, Quentin Schulz wrote: >>> Hi Ulf, >>> >>> On Wed, May 02, 2018 at 11:26:30AM +0200, Ulf Hansson wrote: >>>> On 23 April 2018 at 11:50, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>>>> On 14 April 2018 at 13:25, Quentin Schulz <quentin.schulz@bootlin.com> wrote: >>>>>> Hi Ulf and Hans, >>>>>> >>>>>> On Fri, Apr 06, 2018 at 10:41:10AM +0200, Hans de Goede wrote: >>>>>>> Hi Ulf, >>>>>>> >>>>>>> On 05-04-18 22:19, Ulf Hansson wrote: >>>>>>>> It's rather common that SDIO func devices becomes loaded with a new firmware as >>>>>>>> a part of the SDIO func driver being probed. However, in some special scenarios >>>>>>>> the SDIO func device needs a SW reset, as to start running the new firmware. >>>>>>>> >>>>>>>> More importantly, a full power cycle doesn't work, as that would reset also the >>>>>>>> firmware, thus the existing mmc_hw_reset() API can't be used to deal with these >>>>>>>> scenarios. >>>>>>>> >>>>>>>> Therefore this series suggest to add a new API, mmc_sw_reset(), which resets >>>>>>>> and re-initialize the SDIO card. A couple of the patches in the series are >>>>>>>> mostly re-factorings making generic improvements to the related code. >>>>>>>> >>>>>>>> For more background to this series, feel free to digest the discussions from >>>>>>>> the submitted patch: https://patchwork.kernel.org/patch/9857175/ >>>>>>>> >>>>>>>> It should be noted, at this point this series has only be compile tested. Help >>>>>>>> with tests and deployment of using the new API is greatly appreciated. >>>>>>> >>>>>>> Thank you for this series, I've taken a quick look and it looks good, >>>>>>> but the proof is in the pudding. Quentin can you test this on an ARM >>>>>>> board with an ESP8089 sometime the coming week? >>>>>>> >>>>>> >>>>>> I applied your patches on top of next-20180406. >>>>>> >>>>>> I may need some help to get the ESP8089 driver to work. Note that I'm >>>>>> very new to MMC and SDIO, just so you know (and so you can adapt your >>>>>> answer). >>>>>> >>>>>> It's filling my kernel log buffer with a lot of these[1]. I've basically >>>>>> replaced Hans's mmc_force_detect_change() by your mmc_sw_reset() in >>>>>> esp_sdio_probe() of the ESP8089 driver and that's it. Here is my old >>>>>> patch series for ESP8089[2]. >>>>>> >>>>>> I can't say if it's an error in your patch series or me using it the >>>>>> wrong way. >>>>>> >>>>>> Could you help me debug this so that we can make the "weird" SDIO >>>>>> devices such as the ESP8089 work in the kernel :)? >>>>>> >>>>>> I'll try to mount my rootfs from something else than an initramfs so >>>>>> that it may solve my problem of dmesg not showing the whole log. >>>>>> >>>>>> [1] http://code.bulix.org/eechlb-318686 >>>>> >>>>> Unless I am misstaken, it looks like the messages comes from a >>>>> WARN_ON(!host->claimed); in mmc_wait_for_cmd(). >>>>> >>>>> Did you call sdio_claim_host() before calling mmc_sw_reset()? And then >>>>> of course you need to release the host when you are ready executing >>>>> the required operations, sdio_release_host(). >>>>> >>> >>> That was it, thanks. >>> >>> Now, I go pass mmc_sw_reset and the init of the driver can continue but >>> I'm now hitting a timeout waiting for a completion. >>> >>> I'll need definitely to put more time into it to debug it now. Next week >>> has two holidays in France though, I'll try my best to find time to work >>> on it next week but can't promise much work will be done. >>> >>> If anyone wants to help meanwhile, here is the code: >>> https://github.com/QSchulz/linux/tree/mmc_sw_reset/esp8089 >>> >>> This is the bootlog of the tablet: >>> http://code.bulix.org/t7iken-328962 >> >> Could you try: >> >> drivers/net/wireless/espressif/esp8089/sdio_sif_esp.c >> -543,21 +530,61 @@ static int esp_sdio_probe(struct sdio_func *func, >> sdio_claim_host(func); >> + sdio_release_irq(func); >> mmc_sw_reset(host); >> sdio_release_host(func); >> > > You rock :) That made it work despite my wrong assumptions. Thanks a > lot! > > We've got ESP8089 working without a dirty hack \o/ (well at least not on > the core side :) ). > > Ulf, I'm very happy (and relieved) to give my > > Tested-by: Quentin Schulz <quentin.schulz@bootlin.com> > > Thanks a ton for the patch series, I can now go back to cleaning up the > WiFi driver :) Cool, I'm glad that this means we can now finally get the esp8089 driver on its way to the mainline kernel. A big "thank you' to all involved. Regards, Hans > > Quentin > >>> >>> Hope we can get this sorted out somehow quickly so we can just put this >>> issue of weird SDIO devices behind us once and for all :) (well, until >>> the next weird ones :D). >>> >>> Thanks, >>> Quentin >>> >>>>>> [2] https://lkml.org/lkml/2017/7/21/386 >>>>>> >>>>>> Let me know how I can be helpful, >>>>>> Thanks, >>>>>> Quentin >>>> >>>> Quentin, ping! >>>> >>>> Just tell me if there is anything else I can help out with! >>>> >>>> Kind regards >>>> Uffe >> -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[...] >> > Now, I go pass mmc_sw_reset and the init of the driver can continue but >> > I'm now hitting a timeout waiting for a completion. >> > >> > I'll need definitely to put more time into it to debug it now. Next week >> > has two holidays in France though, I'll try my best to find time to work >> > on it next week but can't promise much work will be done. >> > >> > If anyone wants to help meanwhile, here is the code: >> > https://github.com/QSchulz/linux/tree/mmc_sw_reset/esp8089 >> > >> > This is the bootlog of the tablet: >> > http://code.bulix.org/t7iken-328962 >> >> Could you try: >> >> drivers/net/wireless/espressif/esp8089/sdio_sif_esp.c >> -543,21 +530,61 @@ static int esp_sdio_probe(struct sdio_func *func, >> sdio_claim_host(func); >> + sdio_release_irq(func); >> mmc_sw_reset(host); >> sdio_release_host(func); >> > > You rock :) That made it work despite my wrong assumptions. Thanks a > lot! > > We've got ESP8089 working without a dirty hack \o/ (well at least not on > the core side :) ). > > Ulf, I'm very happy (and relieved) to give my > > Tested-by: Quentin Schulz <quentin.schulz@bootlin.com> > > Thanks a ton for the patch series, I can now go back to cleaning up the > WiFi driver :) > > Quentin Quentin, Shawn - thanks! I have queued up the series for 4.18. [...] Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html