Hi, On 05/26/2014 04:22 PM, Mark Brown wrote: > On Mon, May 26, 2014 at 01:12:43PM +0200, Hans de Goede wrote: >> On 05/26/2014 12:38 PM, Mark Brown wrote: >>> On Sun, May 25, 2014 at 09:20:52PM +0200, Hans de Goede wrote: > >>> until we've powered up and enumerated. The only time that there's a >>> problem and would need to specify exactly what the device is in the DTS >>> is if we need the custom sequence prior to being able to do that at >>> which point I don't see much option. > >> Specified != driver available. Determining whether or not we should power >> up based purely on there being a compatible string is not going to work, as >> even when using simple powerup we still need compatible strings for non >> powerup settings like sdio signal drive strength, oob irq, etc. > > If there are compatible strings but not one we've heard of then we're > back in the situation where we may as well not bother powering up in the > first place since we've no idea what to do with the device once it's > powered. If we fall down a list of compatibles and decide that the > only thing we know about the device is that we can power it up (this is > distinct from the case where we probe the device) I'd expect the device > to be turned off. We won't know if we have a valid driver until we power on the device, and probe it, not all sdio drivers will have compatibles, actually most of them won't since sdio is a discoverable bus. > >> So the only way this will work is if we delay powerup until we've a driver >> available, which may be never if the module is not build, or whatever, >> and without powerup the user is never going to figure out he is missing >> a module. Basically adding a driver flag for blacklisting from the simple >> power up logic means inserting an unnecessary initialization ordering issue, >> which we really don't need as each ordering dependency is usually one too many. > > I'm afraid I'm really not seeing a substantial problem either way here, > powering up the device isn't going to help us find a driver for it and > the UI around reporting that the device is there without a driver should > hopefully be unaffected by its power state. How can the UI be unaffected if we cannot tell userspace that there is a device there and what its vendor / product ids are ? We need to power up the device before it will respond to probes... > >>> I don't understand why not powering the device up would be a sensible >>> default or why other OSs would also choose to implement things that way. > >> Because if we are not 100% sure that our simple power up logic will do the >> job properly (i.e. in the right order), then the SAFE thing to do is to >> not power up. > > So long as we've got a clear way of saying that we might need to do > something special I don't think we should have an issue either way; > it's mostly a case of how we specify. > >> What if a user takes an older distro kernel, where the driver does not >> set the opt-out flag you're suggesting since at that time it did not >> have its own power logic, then tries to boot that with a dtb file written >> for a newer kernel (where the driver does have the opt out flag), and we >> start powering up things in the wrong order and let the magic smoke out >> of various components on the board ? > > Conversely what if someone fixes a bug in the standard power up sequence > in a newer version of the kernel but then tries to run older software? > There's plenty of ways things can go wrong with this stuff. > >> Also note that this is a perfectly standard use of compatibles, compatibles >> are typically used to indicate which driver should be used, in this case >> the compatible indicates that the simple powerup driver should be used, >> where as if another powerup driver should be used another compatbile will >> be there instead. > > We don't typically actually bind multiple compatibles for a single > device. We've got a bunch of options we can choose from but we > generally pick the one that matches best and ignore the others. > >> Where as what you're suggesting is to always pick driver foo, unless >> driver bar is available and has a special flag saying to not use foo, this >> is a whole new way to use compatibles really, and not one which I think >> we want to introduce. > > I'm not sure I'm buying the idea that we have a powerup driver that's > meaningfully not part of the main device driver. Well, if we merge some variant of Olof's code, we will have a powerup driver that is part of the mmc core, and thus not of the sdio function driver. >>> Well, if things aren't going to work either way for these devices >>> without extra stuff it seems it doesn't much matter but it helps the >>> simple case to have things default to working. > >> The simple case still needs a child node describing the needed resources, >> adding a compatible = "simple-sdio-powerup" to that at the same as creating >> the child node in the first place really is no extra effort at all. > > From where I'm sitting it's more effort since instead of just putting > the device in there (and possibly also some other devices that are > software compatible) we have to put in another compatible string which > is really just a boolean flag to be used in conjunction with the others. > That's harder to think about and we clearly don't want to go through the > compatible list, decide that we don't know how to handle the device > except power it up so go and do that. > > If it were done as something like "set boolean property X or > powerup-method = Y in the card description" or whatever it'd seem a bit > annoying but not a big deal, I think it's the fact that it's getting put > into the compatible list that's really concerning me. Ok, so lets switch it over to a boolean, options for the name I see are: linux,mmc-host-powerup (opt in to powerup being dealt with by the mmc core, implementation specific) simple-powerup (platform neutral opt in to say just enable all resources and be done with it) custom-powerup (platform neutral opt out version of simple-powerup) linux,custom-powerup (same, but consider this something linux specific) I think that it may be best to go with one of the 2 linux prefixed options. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
