Re: [PATCHv3 0/4] drivers/base: bugfix for supplier<-consumer ordering in device_kset

On Fri, Jul 6, 2018 at 4:36 PM Lukas Wunner [off-list ref] wrote:

[cc += Kishon Vijay Abraham]

On Thu, Jul 05, 2018 at 11:18:28AM +0200, Rafael J. Wysocki wrote:

quoted

OK, so calling devices_kset_move_last() from really_probe() clearly is
a mistake.

I'm not really sure what the intention of it was as the changelog of
commit 52cdbdd49853d doesn't really explain that (why would it be
insufficient without that change?)

It seems 52cdbdd49853d fixed an issue with boards which have an MMC
whose reset pin needs to be driven high on shutdown, lest the MMC
won't be found on the next boot.

The boards' devicetrees use a kludge wherein the reset pin is modelled
as a regulator.  The regulator is enabled when the MMC probes and
disabled on driver unbind and shutdown.  As a result, the pin is driven
low on shutdown and the MMC is not found on the next boot.

To fix this, another kludge was invented wherein the GPIO expander
driving the reset pin unconditionally drives all its pins high on
shutdown, see pcf857x_shutdown() in drivers/gpio/gpio-pcf857x.c
(commit adc284755055, "gpio: pcf857x: restore the initial line state
of all pcf lines").

For this kludge to work, the GPIO expander's ->shutdown hook needs to
be executed after the MMC expander's ->shutdown hook.

Commit 52cdbdd49853d achieved that by reordering devices_kset according
to the probe order.  Apparently the MMC probes after the GPIO expander,
possibly because it returns -EPROBE_DEFER if the vmmc regulator isn't
available yet, see mmc_regulator_get_supply().

Note, I'm just piecing the information together from git history,
I'm not responsible for these kludges.  (I'm innocent!)

Thanks for your exploration, very clearly. I had tried, but failed
since these commits are contributed with different authors. I am not
familiar with ARM and dts, so had thought
really_probe()->devices_kset_move_last() is used to address a very
popular "supplier<-consumer" order issue in smart phone, based on the
configuration hard-coded in "bios(or counterpart in ARM).

@Pingfan Liu, if you just remove the call to devices_kset_move_last()
from really_probe(), does the issue go away?

Yes, it goes away.

If so, it might be best to remove that call and model the dependency
with a call to device_link_add() in mmc_regulator_get_supply().
Another idea would be to automatically add a device_link in the
driver core if -EPROBE_DEFER is returned.

Maybe the first one is better, as it is already used by other drivers.

Thanks,
Pingfan