Re: [PATCH 2/2] mmc: Add mmc_force_detect_change_begin / _end functions

[Frieder Schrempf]

Hi Quentin,

On 09.10.2018 09:52, Quentin Schulz wrote:

Hi Frieder,

On Mon, Oct 08, 2018 at 11:53:21AM +0200, Frieder Schrempf wrote:

Hi,

On 27.09.2018 10:14, Maxime Ripard wrote:

On Wed, Sep 26, 2018 at 10:19:22PM +0200, Hans de Goede wrote:

On 26-09-18 16:44, Frieder Schrempf wrote:

Hi,

On Fri, Feb 09, 2018 at 03:01:00PM +0100, Ulf Hansson wrote:

[...]


I'd like to know if any progress has been made on that problem

(I may

have missed patches).
Had you had the time to look at the issue?


I have looked at the issue, but not manage to cook some patches

for it.


However, it's on my top of my TODO list for mmc. No promises, but
perhaps and hopefully I manage to get something posted during the
coming release cycle.


I would be interested in a ESP8089 driver in mainline and that's why I want to 
pick up this discussion.

What is the current status of the "mmc_reprobe_device" implementation, that 
Hans was explaining and Ulf wanted to provide some months ago?


Ulf did eventually write a new way to deal with this and then Quentin
did manage to get the esp8089 driver to work with it, the new function
to use for this is added by this commit:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/mmc/core?id=1433269c4d2461be1f36db5dbb453976b38996ff

I'm not sure what the status of upstreaming the ep8089 driver is now
that we've this in place.

Quentin, do you have a version of the esp8089 driver somewhere
which will work correctly with the new mmc_sw_reset() function?

Also what is the status of adding this driver to say staging?


IIRC, we tried to get it into staging, and we got told that it was too
nice for staging at this point. So we're basically stuck somewhere
between staging and !staging, with the driver being too nice for the
former, and not nice enough for the latter :)


Ok, and is there someone willing to continue upstreaming the driver? Maybe
someone can rebase and resend the latest approach?

After all it looks like a lot of work has already been done.



There's clean up to do. It's time consuming but shouldn't be too hard to
do.

Then, we stressed the driver with an iperf test and it crashes very
often so we first need to identify if it happened with the "original"
driver before Icenowy's, Hans's and my clean-up. If it happened, since
we don't have a datasheet, it might be not that easy to fix. If it
didn't happen, then we have to find out where I cleaned up too much :D

I'm not currently working on this topic so anyone willing to take over
the work is free to do so.


Thanks a lot for providing the details. It is currently not clear on our 
side, if the project with the ESP8089 will happen, but if that's the 
case we might pick up the work.


Thanks
Frieder
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/2] mmc: Add mmc_force_detect_change_begin / _end functions

[Quentin Schulz]

Hi Frieder,

On Mon, Oct 08, 2018 at 11:53:21AM +0200, Frieder Schrempf wrote:
> Hi,
> 
> On 27.09.2018 10:14, Maxime Ripard wrote:
> > On Wed, Sep 26, 2018 at 10:19:22PM +0200, Hans de Goede wrote:
> > > On 26-09-18 16:44, Frieder Schrempf wrote:
> > > > Hi,
> > > > 
> > > > On Fri, Feb 09, 2018 at 03:01:00PM +0100, Ulf Hansson wrote:
> > > > > [...]
> > > > > 
> > > > > > > > I'd like to know if any progress has been made on that problem
> > > > (I may
> > > > > > > > have missed patches).
> > > > > > > > Had you had the time to look at the issue?
> > > > > > > 
> > > > > > > I have looked at the issue, but not manage to cook some patches
> > > > for it.
> > > > > > > 
> > > > > > > However, it's on my top of my TODO list for mmc. No promises, but
> > > > > > > perhaps and hopefully I manage to get something posted during the
> > > > > > > coming release cycle.
> > > > 
> > > > I would be interested in a ESP8089 driver in mainline and that's why I 
> > > > want to pick up this discussion.
> > > > 
> > > > What is the current status of the "mmc_reprobe_device" implementation, 
> > > > that Hans was explaining and Ulf wanted to provide some months ago?
> > > 
> > > Ulf did eventually write a new way to deal with this and then Quentin
> > > did manage to get the esp8089 driver to work with it, the new function
> > > to use for this is added by this commit:
> > > 
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/mmc/core?id=1433269c4d2461be1f36db5dbb453976b38996ff
> > > 
> > > I'm not sure what the status of upstreaming the ep8089 driver is now
> > > that we've this in place.
> > > 
> > > Quentin, do you have a version of the esp8089 driver somewhere
> > > which will work correctly with the new mmc_sw_reset() function?
> > > 
> > > Also what is the status of adding this driver to say staging?
> > 
> > IIRC, we tried to get it into staging, and we got told that it was too
> > nice for staging at this point. So we're basically stuck somewhere
> > between staging and !staging, with the driver being too nice for the
> > former, and not nice enough for the latter :)
> 
> Ok, and is there someone willing to continue upstreaming the driver? Maybe
> someone can rebase and resend the latest approach?
> 
> After all it looks like a lot of work has already been done.
> 

There's clean up to do. It's time consuming but shouldn't be too hard to
do.

Then, we stressed the driver with an iperf test and it crashes very
often so we first need to identify if it happened with the "original"
driver before Icenowy's, Hans's and my clean-up. If it happened, since
we don't have a datasheet, it might be not that easy to fix. If it
didn't happen, then we have to find out where I cleaned up too much :D

I'm not currently working on this topic so anyone willing to take over
the work is free to do so.

Thanks,
Quentin


signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/2] mmc: Add mmc_force_detect_change_begin / _end functions

[Frieder Schrempf]

Hi,

On 27.09.2018 10:14, Maxime Ripard wrote:

On Wed, Sep 26, 2018 at 10:19:22PM +0200, Hans de Goede wrote:

On 26-09-18 16:44, Frieder Schrempf wrote:

Hi,

On Fri, Feb 09, 2018 at 03:01:00PM +0100, Ulf Hansson wrote:

[...]


I'd like to know if any progress has been made on that problem

(I may

have missed patches).
Had you had the time to look at the issue?


I have looked at the issue, but not manage to cook some patches

for it.


However, it's on my top of my TODO list for mmc. No promises, but
perhaps and hopefully I manage to get something posted during the
coming release cycle.


I would be interested in a ESP8089 driver in mainline and that's why I want to 
pick up this discussion.

What is the current status of the "mmc_reprobe_device" implementation, that 
Hans was explaining and Ulf wanted to provide some months ago?


Ulf did eventually write a new way to deal with this and then Quentin
did manage to get the esp8089 driver to work with it, the new function
to use for this is added by this commit:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/mmc/core?id=1433269c4d2461be1f36db5dbb453976b38996ff

I'm not sure what the status of upstreaming the ep8089 driver is now
that we've this in place.

Quentin, do you have a version of the esp8089 driver somewhere
which will work correctly with the new mmc_sw_reset() function?

Also what is the status of adding this driver to say staging?


IIRC, we tried to get it into staging, and we got told that it was too
nice for staging at this point. So we're basically stuck somewhere
between staging and !staging, with the driver being too nice for the
former, and not nice enough for the latter :)


Ok, and is there someone willing to continue upstreaming the driver? 
Maybe someone can rebase and resend the latest approach?


After all it looks like a lot of work has already been done.

Thanks,
Frieder
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/2] mmc: Add mmc_force_detect_change_begin / _end functions

[Maxime Ripard]

On Wed, Sep 26, 2018 at 10:19:22PM +0200, Hans de Goede wrote:
> On 26-09-18 16:44, Frieder Schrempf wrote:
> > Hi,
> > 
> > On Fri, Feb 09, 2018 at 03:01:00PM +0100, Ulf Hansson wrote:
> > > [...]
> > > 
> > > >> > I'd like to know if any progress has been made on that problem
> > (I may
> > > >> > have missed patches).
> > > >> > Had you had the time to look at the issue?
> > > >>
> > > >> I have looked at the issue, but not manage to cook some patches
> > for it.
> > > >>
> > > >> However, it's on my top of my TODO list for mmc. No promises, but
> > > >> perhaps and hopefully I manage to get something posted during the
> > > >> coming release cycle.
> > 
> > I would be interested in a ESP8089 driver in mainline and that's why I want 
> > to pick up this discussion.
> > 
> > What is the current status of the "mmc_reprobe_device" implementation, that 
> > Hans was explaining and Ulf wanted to provide some months ago?
> 
> Ulf did eventually write a new way to deal with this and then Quentin
> did manage to get the esp8089 driver to work with it, the new function
> to use for this is added by this commit:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/mmc/core?id=1433269c4d2461be1f36db5dbb453976b38996ff
> 
> I'm not sure what the status of upstreaming the ep8089 driver is now
> that we've this in place.
> 
> Quentin, do you have a version of the esp8089 driver somewhere
> which will work correctly with the new mmc_sw_reset() function?
> 
> Also what is the status of adding this driver to say staging?

IIRC, we tried to get it into staging, and we got told that it was too
nice for staging at this point. So we're basically stuck somewhere
between staging and !staging, with the driver being too nice for the
former, and not nice enough for the latter :)

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/2] mmc: Add mmc_force_detect_change_begin / _end functions

[Hans de Goede]

Hi,

On 26-09-18 16:44, Frieder Schrempf wrote:

Hi,

On Fri, Feb 09, 2018 at 03:01:00PM +0100, Ulf Hansson wrote:

[...]

>> > I'd like to know if any progress has been made on that problem 

(I may

>> > have missed patches).
>> > Had you had the time to look at the issue?
>>
>> I have looked at the issue, but not manage to cook some patches 

for it.

>>
>> However, it's on my top of my TODO list for mmc. No promises, but
>> perhaps and hopefully I manage to get something posted during the
>> coming release cycle.


I would be interested in a ESP8089 driver in mainline and that's why I want to 
pick up this discussion.

What is the current status of the "mmc_reprobe_device" implementation, that 
Hans was explaining and Ulf wanted to provide some months ago?


Ulf did eventually write a new way to deal with this and then Quentin
did manage to get the esp8089 driver to work with it, the new function
to use for this is added by this commit:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/mmc/core?id=1433269c4d2461be1f36db5dbb453976b38996ff

I'm not sure what the status of upstreaming the ep8089 driver is now
that we've this in place.

Quentin, do you have a version of the esp8089 driver somewhere
which will work correctly with the new mmc_sw_reset() function?

Also what is the status of adding this driver to say staging?

Regards,

Hans





BTW, I am not on any of the mailing lists involved, so I tried to recreate the 
proper mail headers manually for the reply to the correct thread.

Thanks,
Frieder


>>
>
> Cool! If you ever need some testing, I'd be glad to test your patches
> (even if they are in a draft/RFC state).
>
> Also, when you send patches, I'd appreciate being Cc'ed so that I can
> put my Tested-by :) Thanks!

Absolutely! I appreciate it.

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/2] mmc: Add mmc_force_detect_change_begin / _end functions

[Frieder Schrempf]

Hi,

On Fri, Feb 09, 2018 at 03:01:00PM +0100, Ulf Hansson wrote:

[...]

>> > I'd like to know if any progress has been made on that problem 

(I may

>> > have missed patches).
>> > Had you had the time to look at the issue?
>>
>> I have looked at the issue, but not manage to cook some patches 

for it.

>>
>> However, it's on my top of my TODO list for mmc. No promises, but
>> perhaps and hopefully I manage to get something posted during the
>> coming release cycle.


I would be interested in a ESP8089 driver in mainline and that's why I 
want to pick up this discussion.


What is the current status of the "mmc_reprobe_device" implementation, 
that Hans was explaining and Ulf wanted to provide some months ago?


BTW, I am not on any of the mailing lists involved, so I tried to 
recreate the proper mail headers manually for the reply to the correct 
thread.


Thanks,
Frieder


>>
>
> Cool! If you ever need some testing, I'd be glad to test your patches
> (even if they are in a draft/RFC state).
>
> Also, when you send patches, I'd appreciate being Cc'ed so that I can
> put my Tested-by :) Thanks!

Absolutely! I appreciate it.

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/2] mmc: Add mmc_force_detect_change_begin / _end functions

[Quentin Schulz]

Hi Uffe,

On Fri, Feb 09, 2018 at 03:01:00PM +0100, Ulf Hansson wrote:
> [...]
> 
> >> > I'd like to know if any progress has been made on that problem (I may
> >> > have missed patches).
> >> > Had you had the time to look at the issue?
> >>
> >> I have looked at the issue, but not manage to cook some patches for it.
> >>
> >> However, it's on my top of my TODO list for mmc. No promises, but
> >> perhaps and hopefully I manage to get something posted during the
> >> coming release cycle.
> >>
> >
> > Cool! If you ever need some testing, I'd be glad to test your patches
> > (even if they are in a draft/RFC state).
> >
> > Also, when you send patches, I'd appreciate being Cc'ed so that I can
> > put my Tested-by :) Thanks!
> 
> Absolutely! I appreciate it.
> 

Any news? Can I help in any way?

Best regards,
Quentin


signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/2] mmc: Add mmc_force_detect_change_begin / _end functions

[Ulf Hansson]

[...]

>> > I'd like to know if any progress has been made on that problem (I may
>> > have missed patches).
>> > Had you had the time to look at the issue?
>>
>> I have looked at the issue, but not manage to cook some patches for it.
>>
>> However, it's on my top of my TODO list for mmc. No promises, but
>> perhaps and hopefully I manage to get something posted during the
>> coming release cycle.
>>
>
> Cool! If you ever need some testing, I'd be glad to test your patches
> (even if they are in a draft/RFC state).
>
> Also, when you send patches, I'd appreciate being Cc'ed so that I can
> put my Tested-by :) Thanks!

Absolutely! I appreciate it.

Br
Uffe
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/2] mmc: Add mmc_force_detect_change_begin / _end functions

[Quentin Schulz]

Hi Ulf,

On Thu, Feb 08, 2018 at 10:31:39PM +0100, Ulf Hansson wrote:
> On 8 February 2018 at 15:59, Quentin Schulz  
> wrote:
> > Hi Ulf,
> >
> > On Wed, Aug 30, 2017 at 03:43:49PM +0200, Ulf Hansson wrote:
> >> On 30 August 2017 at 14:44, Hans de Goede  wrote:
> >> > Hi,
> >> >
> >> >
> >> > On 21-07-17 16:35, Quentin Schulz wrote:
> >> >>
> >> >> From: Hans de Goede 
> >> >>
> >> >> Some sdio devices have a multiple stage bring-up process. Specifically
> >> >> the esp8089 (for which an out of tree driver is available) loads 
> >> >> firmware
> >> >> on the first call to its sdio-drivers' probe function and then resets
> >> >> the device causing it to reboot from its RAM with the new firmware.
> >> >>
> >> >> When this sdio device reboots it comes back up in 1 bit 400 KHz mode
> >> >> again, and we need to walk through the whole ios negatiation and sdio
> >> >> setup
> >> >> again.
> >> >>
> >> >> There are 2 problems with this:
> >> >>
> >> >> 1) Typically these devices are soldered onto some (ARM) tablet / SBC
> >> >> PCB and as such are described in devicetree as "non-removable", which
> >> >> causes the mmc-core to scan them only once and not poll for the device
> >> >> dropping of the bus. Normally this is the right thing todo but in the
> >> >> eso8089 example we need the mmc-core to notice the module has 
> >> >> disconnected
> >> >> (since it is now in 1 bit mode again it will not talk to the host in 4 
> >> >> bit
> >> >> mode). This can be worked around by using "broken-cd" in devicetree
> >> >> instead of "non-removable", but that is not a proper fix since the 
> >> >> device
> >> >> really is non-removable.
> >> >>
> >> >> 2) When the mmc-core detects the device has disconnected it will 
> >> >> poweroff
> >> >> the device, causing the RAM loaded firmware to be lost. This can be 
> >> >> worked
> >> >> around in devicetree by using regulator-always-on (and avoiding the use 
> >> >> of
> >> >> mmc-pwrseq), but again that is more of a hack then a proper fix.
> >> >>
> >> >> This commmit fixes 1) by adding a mmc_force_detect_change function which
> >> >> will cause scanning for device removal / insertion until a new device is
> >> >> detected. 2) Is fixed by a keep_power flag to the 
> >> >> mmc_force_detect_change
> >> >> function which when set causes the mmc-core to keep the power to the
> >> >> device
> >> >> on during the rescan.
> >> >>
> >> >> Cc: Icenowy Zheng 
> >> >> Cc: Maxime Ripard 
> >> >> Cc: Chen-Yu Tsai 
> >> >> Signed-off-by: Hans de Goede 
> >> >
> >> >
> >> > So when I posted this patch quite a while back, there was some discussion
> >> > about this and a consensus that this is not the right solution.
> >> >
> >> > So first of all lets describe the problem:
> >> >
> >> > The esp8089 sdio wifi chip is really an ARM core with a wifi phy
> >> > connected to it (as many wifi chipsets are).
> >> >
> >> > But this one comes up in some really generic sdio capable boot-loader
> >> > mode and we need to feed it firmware and then reboot it into the
> >> > new firmware.
> >> >
> >> > The reboot is where the problems happens. It seems to fallback
> >> > from the negotiated 4 wire sdio mode to single wire spi mode then.
> >> >
> >> > The out of tree version of the driver deals with this by not setting
> >> > the non-removable flag as well as setting the broken_cd flag so that
> >> > the mmc core polls the device, after the reboot the poll fails
> >> > because the mmc-controller and the esp8089 are using a different
> >> > amount of wires so the mmc-cmd the poll uses times out.
> >> >
> >> > After which the esp8089 drivers remove function gets called, and
> >> > the mmc stack re-discovers the esp8089 by restarting the whole
> >> > number of wires (and speed) used negotiation. After which the
> >> > esp8089 driver's probe function gets called (again) and on
> >> > firmware loading is has set a global flag, so now it actually
> >> > acts as a wifi driver rather then trying to load the firmware
> >> > a second time.
> >> >
> >> > Since I did not want to rely on broken_cd polling I came up
> >> > with the hack which is this patch.
> >> >
> >> > So when this patch was first discussed we came to the conclusion
> >> > that what we really need is some sort of mmc_reprobe_device
> >> > function which the driver can call from probe which will
> >> > redo the number of wires (and speed) used negotiation,
> >> > while keeping the sdio_function device as is so that probe can
> >> > simply continue after this and we also don't need the ugly
> >> > global flag.
> >> >
> >> > The idea would be for this function to be some wrapper
> >> > around mmc_init_card() which resets the ios settings as is
> >> > normally done on remove and then call mmc_init_card()
> >> > passing in the existing card the same way as is done
> >> > one resume, so that the 

Re: [PATCH 2/2] mmc: Add mmc_force_detect_change_begin / _end functions

[Ulf Hansson]

On 8 February 2018 at 15:59, Quentin Schulz  wrote:
> Hi Ulf,
>
> On Wed, Aug 30, 2017 at 03:43:49PM +0200, Ulf Hansson wrote:
>> On 30 August 2017 at 14:44, Hans de Goede  wrote:
>> > Hi,
>> >
>> >
>> > On 21-07-17 16:35, Quentin Schulz wrote:
>> >>
>> >> From: Hans de Goede 
>> >>
>> >> Some sdio devices have a multiple stage bring-up process. Specifically
>> >> the esp8089 (for which an out of tree driver is available) loads firmware
>> >> on the first call to its sdio-drivers' probe function and then resets
>> >> the device causing it to reboot from its RAM with the new firmware.
>> >>
>> >> When this sdio device reboots it comes back up in 1 bit 400 KHz mode
>> >> again, and we need to walk through the whole ios negatiation and sdio
>> >> setup
>> >> again.
>> >>
>> >> There are 2 problems with this:
>> >>
>> >> 1) Typically these devices are soldered onto some (ARM) tablet / SBC
>> >> PCB and as such are described in devicetree as "non-removable", which
>> >> causes the mmc-core to scan them only once and not poll for the device
>> >> dropping of the bus. Normally this is the right thing todo but in the
>> >> eso8089 example we need the mmc-core to notice the module has disconnected
>> >> (since it is now in 1 bit mode again it will not talk to the host in 4 bit
>> >> mode). This can be worked around by using "broken-cd" in devicetree
>> >> instead of "non-removable", but that is not a proper fix since the device
>> >> really is non-removable.
>> >>
>> >> 2) When the mmc-core detects the device has disconnected it will poweroff
>> >> the device, causing the RAM loaded firmware to be lost. This can be worked
>> >> around in devicetree by using regulator-always-on (and avoiding the use of
>> >> mmc-pwrseq), but again that is more of a hack then a proper fix.
>> >>
>> >> This commmit fixes 1) by adding a mmc_force_detect_change function which
>> >> will cause scanning for device removal / insertion until a new device is
>> >> detected. 2) Is fixed by a keep_power flag to the mmc_force_detect_change
>> >> function which when set causes the mmc-core to keep the power to the
>> >> device
>> >> on during the rescan.
>> >>
>> >> Cc: Icenowy Zheng 
>> >> Cc: Maxime Ripard 
>> >> Cc: Chen-Yu Tsai 
>> >> Signed-off-by: Hans de Goede 
>> >
>> >
>> > So when I posted this patch quite a while back, there was some discussion
>> > about this and a consensus that this is not the right solution.
>> >
>> > So first of all lets describe the problem:
>> >
>> > The esp8089 sdio wifi chip is really an ARM core with a wifi phy
>> > connected to it (as many wifi chipsets are).
>> >
>> > But this one comes up in some really generic sdio capable boot-loader
>> > mode and we need to feed it firmware and then reboot it into the
>> > new firmware.
>> >
>> > The reboot is where the problems happens. It seems to fallback
>> > from the negotiated 4 wire sdio mode to single wire spi mode then.
>> >
>> > The out of tree version of the driver deals with this by not setting
>> > the non-removable flag as well as setting the broken_cd flag so that
>> > the mmc core polls the device, after the reboot the poll fails
>> > because the mmc-controller and the esp8089 are using a different
>> > amount of wires so the mmc-cmd the poll uses times out.
>> >
>> > After which the esp8089 drivers remove function gets called, and
>> > the mmc stack re-discovers the esp8089 by restarting the whole
>> > number of wires (and speed) used negotiation. After which the
>> > esp8089 driver's probe function gets called (again) and on
>> > firmware loading is has set a global flag, so now it actually
>> > acts as a wifi driver rather then trying to load the firmware
>> > a second time.
>> >
>> > Since I did not want to rely on broken_cd polling I came up
>> > with the hack which is this patch.
>> >
>> > So when this patch was first discussed we came to the conclusion
>> > that what we really need is some sort of mmc_reprobe_device
>> > function which the driver can call from probe which will
>> > redo the number of wires (and speed) used negotiation,
>> > while keeping the sdio_function device as is so that probe can
>> > simply continue after this and we also don't need the ugly
>> > global flag.
>> >
>> > The idea would be for this function to be some wrapper
>> > around mmc_init_card() which resets the ios settings as is
>> > normally done on remove and then call mmc_init_card()
>> > passing in the existing card the same way as is done
>> > one resume, so that the existing card / sdio_function
>> > devices get reused.
>> >
>> > IIRC Ulf would look into writing this mmc_reprobe_device
>> > function and then I would test it with the esp8089, but
>> > Ulf never got around to writing the function and I ended
>> > up working on other things too.
>>
>> Thanks for summary!
>>
>> Just to let you know, I 

Re: [PATCH 2/2] mmc: Add mmc_force_detect_change_begin / _end functions

[Quentin Schulz]

Hi Ulf,

On Wed, Aug 30, 2017 at 03:43:49PM +0200, Ulf Hansson wrote:
> On 30 August 2017 at 14:44, Hans de Goede  wrote:
> > Hi,
> >
> >
> > On 21-07-17 16:35, Quentin Schulz wrote:
> >>
> >> From: Hans de Goede 
> >>
> >> Some sdio devices have a multiple stage bring-up process. Specifically
> >> the esp8089 (for which an out of tree driver is available) loads firmware
> >> on the first call to its sdio-drivers' probe function and then resets
> >> the device causing it to reboot from its RAM with the new firmware.
> >>
> >> When this sdio device reboots it comes back up in 1 bit 400 KHz mode
> >> again, and we need to walk through the whole ios negatiation and sdio
> >> setup
> >> again.
> >>
> >> There are 2 problems with this:
> >>
> >> 1) Typically these devices are soldered onto some (ARM) tablet / SBC
> >> PCB and as such are described in devicetree as "non-removable", which
> >> causes the mmc-core to scan them only once and not poll for the device
> >> dropping of the bus. Normally this is the right thing todo but in the
> >> eso8089 example we need the mmc-core to notice the module has disconnected
> >> (since it is now in 1 bit mode again it will not talk to the host in 4 bit
> >> mode). This can be worked around by using "broken-cd" in devicetree
> >> instead of "non-removable", but that is not a proper fix since the device
> >> really is non-removable.
> >>
> >> 2) When the mmc-core detects the device has disconnected it will poweroff
> >> the device, causing the RAM loaded firmware to be lost. This can be worked
> >> around in devicetree by using regulator-always-on (and avoiding the use of
> >> mmc-pwrseq), but again that is more of a hack then a proper fix.
> >>
> >> This commmit fixes 1) by adding a mmc_force_detect_change function which
> >> will cause scanning for device removal / insertion until a new device is
> >> detected. 2) Is fixed by a keep_power flag to the mmc_force_detect_change
> >> function which when set causes the mmc-core to keep the power to the
> >> device
> >> on during the rescan.
> >>
> >> Cc: Icenowy Zheng 
> >> Cc: Maxime Ripard 
> >> Cc: Chen-Yu Tsai 
> >> Signed-off-by: Hans de Goede 
> >
> >
> > So when I posted this patch quite a while back, there was some discussion
> > about this and a consensus that this is not the right solution.
> >
> > So first of all lets describe the problem:
> >
> > The esp8089 sdio wifi chip is really an ARM core with a wifi phy
> > connected to it (as many wifi chipsets are).
> >
> > But this one comes up in some really generic sdio capable boot-loader
> > mode and we need to feed it firmware and then reboot it into the
> > new firmware.
> >
> > The reboot is where the problems happens. It seems to fallback
> > from the negotiated 4 wire sdio mode to single wire spi mode then.
> >
> > The out of tree version of the driver deals with this by not setting
> > the non-removable flag as well as setting the broken_cd flag so that
> > the mmc core polls the device, after the reboot the poll fails
> > because the mmc-controller and the esp8089 are using a different
> > amount of wires so the mmc-cmd the poll uses times out.
> >
> > After which the esp8089 drivers remove function gets called, and
> > the mmc stack re-discovers the esp8089 by restarting the whole
> > number of wires (and speed) used negotiation. After which the
> > esp8089 driver's probe function gets called (again) and on
> > firmware loading is has set a global flag, so now it actually
> > acts as a wifi driver rather then trying to load the firmware
> > a second time.
> >
> > Since I did not want to rely on broken_cd polling I came up
> > with the hack which is this patch.
> >
> > So when this patch was first discussed we came to the conclusion
> > that what we really need is some sort of mmc_reprobe_device
> > function which the driver can call from probe which will
> > redo the number of wires (and speed) used negotiation,
> > while keeping the sdio_function device as is so that probe can
> > simply continue after this and we also don't need the ugly
> > global flag.
> >
> > The idea would be for this function to be some wrapper
> > around mmc_init_card() which resets the ios settings as is
> > normally done on remove and then call mmc_init_card()
> > passing in the existing card the same way as is done
> > one resume, so that the existing card / sdio_function
> > devices get reused.
> >
> > IIRC Ulf would look into writing this mmc_reprobe_device
> > function and then I would test it with the esp8089, but
> > Ulf never got around to writing the function and I ended
> > up working on other things too.
> 
> Thanks for summary!
> 
> Just to let you know, I haven't forgot about this problem. I am
> planning for a major update of the SDIO for power management support,
> within a not too far future.
> The issue described above, is then also one of 

Re: [PATCH 2/2] mmc: Add mmc_force_detect_change_begin / _end functions

[Ulf Hansson]

On 30 August 2017 at 14:44, Hans de Goede  wrote:
> Hi,
>
>
> On 21-07-17 16:35, Quentin Schulz wrote:
>>
>> From: Hans de Goede 
>>
>> Some sdio devices have a multiple stage bring-up process. Specifically
>> the esp8089 (for which an out of tree driver is available) loads firmware
>> on the first call to its sdio-drivers' probe function and then resets
>> the device causing it to reboot from its RAM with the new firmware.
>>
>> When this sdio device reboots it comes back up in 1 bit 400 KHz mode
>> again, and we need to walk through the whole ios negatiation and sdio
>> setup
>> again.
>>
>> There are 2 problems with this:
>>
>> 1) Typically these devices are soldered onto some (ARM) tablet / SBC
>> PCB and as such are described in devicetree as "non-removable", which
>> causes the mmc-core to scan them only once and not poll for the device
>> dropping of the bus. Normally this is the right thing todo but in the
>> eso8089 example we need the mmc-core to notice the module has disconnected
>> (since it is now in 1 bit mode again it will not talk to the host in 4 bit
>> mode). This can be worked around by using "broken-cd" in devicetree
>> instead of "non-removable", but that is not a proper fix since the device
>> really is non-removable.
>>
>> 2) When the mmc-core detects the device has disconnected it will poweroff
>> the device, causing the RAM loaded firmware to be lost. This can be worked
>> around in devicetree by using regulator-always-on (and avoiding the use of
>> mmc-pwrseq), but again that is more of a hack then a proper fix.
>>
>> This commmit fixes 1) by adding a mmc_force_detect_change function which
>> will cause scanning for device removal / insertion until a new device is
>> detected. 2) Is fixed by a keep_power flag to the mmc_force_detect_change
>> function which when set causes the mmc-core to keep the power to the
>> device
>> on during the rescan.
>>
>> Cc: Icenowy Zheng 
>> Cc: Maxime Ripard 
>> Cc: Chen-Yu Tsai 
>> Signed-off-by: Hans de Goede 
>
>
> So when I posted this patch quite a while back, there was some discussion
> about this and a consensus that this is not the right solution.
>
> So first of all lets describe the problem:
>
> The esp8089 sdio wifi chip is really an ARM core with a wifi phy
> connected to it (as many wifi chipsets are).
>
> But this one comes up in some really generic sdio capable boot-loader
> mode and we need to feed it firmware and then reboot it into the
> new firmware.
>
> The reboot is where the problems happens. It seems to fallback
> from the negotiated 4 wire sdio mode to single wire spi mode then.
>
> The out of tree version of the driver deals with this by not setting
> the non-removable flag as well as setting the broken_cd flag so that
> the mmc core polls the device, after the reboot the poll fails
> because the mmc-controller and the esp8089 are using a different
> amount of wires so the mmc-cmd the poll uses times out.
>
> After which the esp8089 drivers remove function gets called, and
> the mmc stack re-discovers the esp8089 by restarting the whole
> number of wires (and speed) used negotiation. After which the
> esp8089 driver's probe function gets called (again) and on
> firmware loading is has set a global flag, so now it actually
> acts as a wifi driver rather then trying to load the firmware
> a second time.
>
> Since I did not want to rely on broken_cd polling I came up
> with the hack which is this patch.
>
> So when this patch was first discussed we came to the conclusion
> that what we really need is some sort of mmc_reprobe_device
> function which the driver can call from probe which will
> redo the number of wires (and speed) used negotiation,
> while keeping the sdio_function device as is so that probe can
> simply continue after this and we also don't need the ugly
> global flag.
>
> The idea would be for this function to be some wrapper
> around mmc_init_card() which resets the ios settings as is
> normally done on remove and then call mmc_init_card()
> passing in the existing card the same way as is done
> one resume, so that the existing card / sdio_function
> devices get reused.
>
> IIRC Ulf would look into writing this mmc_reprobe_device
> function and then I would test it with the esp8089, but
> Ulf never got around to writing the function and I ended
> up working on other things too.

Thanks for summary!

Just to let you know, I haven't forgot about this problem. I am
planning for a major update of the SDIO for power management support,
within a not too far future.
The issue described above, is then also one of the things I also plan
to look into.

However, if there anybody that wants to hack on this, feel free to do it!

Kind regards
Uffe
___
devel mailing list
de...@linuxdriverproject.org

Re: [PATCH 2/2] mmc: Add mmc_force_detect_change_begin / _end functions

[Hans de Goede]

Hi,

On 21-07-17 16:35, Quentin Schulz wrote:

From: Hans de Goede 

Some sdio devices have a multiple stage bring-up process. Specifically
the esp8089 (for which an out of tree driver is available) loads firmware
on the first call to its sdio-drivers' probe function and then resets
the device causing it to reboot from its RAM with the new firmware.

When this sdio device reboots it comes back up in 1 bit 400 KHz mode
again, and we need to walk through the whole ios negatiation and sdio setup
again.

There are 2 problems with this:

1) Typically these devices are soldered onto some (ARM) tablet / SBC
PCB and as such are described in devicetree as "non-removable", which
causes the mmc-core to scan them only once and not poll for the device
dropping of the bus. Normally this is the right thing todo but in the
eso8089 example we need the mmc-core to notice the module has disconnected
(since it is now in 1 bit mode again it will not talk to the host in 4 bit
mode). This can be worked around by using "broken-cd" in devicetree
instead of "non-removable", but that is not a proper fix since the device
really is non-removable.

2) When the mmc-core detects the device has disconnected it will poweroff
the device, causing the RAM loaded firmware to be lost. This can be worked
around in devicetree by using regulator-always-on (and avoiding the use of
mmc-pwrseq), but again that is more of a hack then a proper fix.

This commmit fixes 1) by adding a mmc_force_detect_change function which
will cause scanning for device removal / insertion until a new device is
detected. 2) Is fixed by a keep_power flag to the mmc_force_detect_change
function which when set causes the mmc-core to keep the power to the device
on during the rescan.

Cc: Icenowy Zheng 
Cc: Maxime Ripard 
Cc: Chen-Yu Tsai 
Signed-off-by: Hans de Goede 


So when I posted this patch quite a while back, there was some discussion
about this and a consensus that this is not the right solution.

So first of all lets describe the problem:

The esp8089 sdio wifi chip is really an ARM core with a wifi phy
connected to it (as many wifi chipsets are).

But this one comes up in some really generic sdio capable boot-loader
mode and we need to feed it firmware and then reboot it into the
new firmware.

The reboot is where the problems happens. It seems to fallback
from the negotiated 4 wire sdio mode to single wire spi mode then.

The out of tree version of the driver deals with this by not setting
the non-removable flag as well as setting the broken_cd flag so that
the mmc core polls the device, after the reboot the poll fails
because the mmc-controller and the esp8089 are using a different
amount of wires so the mmc-cmd the poll uses times out.

After which the esp8089 drivers remove function gets called, and
the mmc stack re-discovers the esp8089 by restarting the whole
number of wires (and speed) used negotiation. After which the
esp8089 driver's probe function gets called (again) and on
firmware loading is has set a global flag, so now it actually
acts as a wifi driver rather then trying to load the firmware
a second time.

Since I did not want to rely on broken_cd polling I came up
with the hack which is this patch.

So when this patch was first discussed we came to the conclusion
that what we really need is some sort of mmc_reprobe_device
function which the driver can call from probe which will
redo the number of wires (and speed) used negotiation,
while keeping the sdio_function device as is so that probe can
simply continue after this and we also don't need the ugly
global flag.

The idea would be for this function to be some wrapper
around mmc_init_card() which resets the ios settings as is
normally done on remove and then call mmc_init_card()
passing in the existing card the same way as is done
one resume, so that the existing card / sdio_function
devices get reused.

IIRC Ulf would look into writing this mmc_reprobe_device
function and then I would test it with the esp8089, but
Ulf never got around to writing the function and I ended
up working on other things too.

I HTH.

Regards,

Hans






---
  drivers/mmc/core/core.c  | 47 ++-
  include/linux/mmc/host.h |  7 +++
  2 files changed, 49 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 26431267a3e2..103badde910b 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1620,8 +1620,11 @@ int mmc_select_drive_strength(struct mmc_card *card, 
unsigned int max_dtr,
   */
  void mmc_power_up(struct mmc_host *host, u32 ocr)
  {
-   if (host->ios.power_mode == MMC_POWER_ON)
+   if (host->ios.power_mode == MMC_POWER_ON) {
+   if (host->ios.clock == 0)
+   goto set_clock;
return;
+   }
  
  	mmc_pwrseq_pre_power_on(host);
  
@@ -1646,6 

Re: [PATCH 2/2] mmc: Add mmc_force_detect_change_begin / _end functions

[Shawn Lin]

invite Jack from expressif

在 2017/7/21 22:35, Quentin Schulz 写道:

From: Hans de Goede 

Some sdio devices have a multiple stage bring-up process. Specifically
the esp8089 (for which an out of tree driver is available) loads firmware
on the first call to its sdio-drivers' probe function and then resets
the device causing it to reboot from its RAM with the new firmware.



Nice to see finally someone get into here!

I was bringing up ESP8089 for rockchip platforms 4 yeas ago with
Jack from espressif, the ESP8089 RD team, face 2 face. And I forgot
most the details but it seems indeed the limitation of RAM size so that
it has to use 2 stages boot-up method.

I hople Jack could give some suggestion or details about this.



When this sdio device reboots it comes back up in 1 bit 400 KHz mode
again, and we need to walk through the whole ios negatiation and sdio setup
again.

There are 2 problems with this:

1) Typically these devices are soldered onto some (ARM) tablet / SBC
PCB and as such are described in devicetree as "non-removable", which
causes the mmc-core to scan them only once and not poll for the device
dropping of the bus. Normally this is the right thing todo but in the
eso8089 example we need the mmc-core to notice the module has disconnected
(since it is now in 1 bit mode again it will not talk to the host in 4 bit
mode). This can be worked around by using "broken-cd" in devicetree
instead of "non-removable", but that is not a proper fix since the device
really is non-removable.

2) When the mmc-core detects the device has disconnected it will poweroff
the device, causing the RAM loaded firmware to be lost. This can be worked
around in devicetree by using regulator-always-on (and avoiding the use of
mmc-pwrseq), but again that is more of a hack then a proper fix.

This commmit fixes 1) by adding a mmc_force_detect_change function which
will cause scanning for device removal / insertion until a new device is
detected. 2) Is fixed by a keep_power flag to the mmc_force_detect_change
function which when set causes the mmc-core to keep the power to the device
on during the rescan.

Cc: Icenowy Zheng 
Cc: Maxime Ripard 
Cc: Chen-Yu Tsai 
Signed-off-by: Hans de Goede 
---
 drivers/mmc/core/core.c  | 47 ++-
 include/linux/mmc/host.h |  7 +++
 2 files changed, 49 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 26431267a3e2..103badde910b 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1620,8 +1620,11 @@ int mmc_select_drive_strength(struct mmc_card *card, 
unsigned int max_dtr,
  */
 void mmc_power_up(struct mmc_host *host, u32 ocr)
 {
-   if (host->ios.power_mode == MMC_POWER_ON)
+   if (host->ios.power_mode == MMC_POWER_ON) {
+   if (host->ios.clock == 0)
+   goto set_clock;
return;
+   }

mmc_pwrseq_pre_power_on(host);

@@ -1646,6 +1649,7 @@ void mmc_power_up(struct mmc_host *host, u32 ocr)

mmc_pwrseq_post_power_on(host);

+set_clock:
host->ios.clock = host->f_init;

host->ios.power_mode = MMC_POWER_ON;
@@ -1663,6 +1667,11 @@ void mmc_power_off(struct mmc_host *host)
if (host->ios.power_mode == MMC_POWER_OFF)
return;

+   if (host->rescan_keep_power) {
+   mmc_set_clock(host, 0);
+   return;
+   }
+
mmc_pwrseq_power_off(host);

host->ios.clock = 0;
@@ -1804,6 +1813,27 @@ void mmc_detect_change(struct mmc_host *host, unsigned 
long delay)
 }
 EXPORT_SYMBOL(mmc_detect_change);

+/**
+ * mmc_force_detect_change - force rescanning of a MMC socket even if
+ *   it is non-removable
+ * @host: host to rescan
+ * @delay: optional delay to wait before detection (jiffies)
+ * @keep_power: if set do not turn of vdd / call pwrseq_off during rescan
+ *
+ * MMC drivers which need non-removable sdio devices to be rescanned
+ * (e.g. because the device reboots its fw after a firmware upload),
+ * can call this to force scanning the MMC socket for changes, even
+ * if it is non-removable.
+ */
+void mmc_force_detect_change(struct mmc_host *host, unsigned long delay,
+bool keep_power)
+{
+   host->rescan_force = 1;
+   host->rescan_keep_power = keep_power;
+   _mmc_detect_change(host, delay, false);
+}
+EXPORT_SYMBOL(mmc_force_detect_change);
+
 void mmc_init_erase(struct mmc_card *card)
 {
unsigned int sz;
@@ -2566,7 +2596,8 @@ void mmc_rescan(struct work_struct *work)
return;

/* If there is a non-removable card registered, only scan once */
-   if (!mmc_card_is_removable(host) && host->rescan_entered)
+   if (!mmc_card_is_removable(host) && host->rescan_entered &&
+