On Tuesday 13 of August 2013 16:14:44 Kishon Vijay Abraham I wrote: > Hi, > > On Wednesday 31 July 2013 11:45 AM, Felipe Balbi wrote: > > Hi, > > > > On Wed, Jul 31, 2013 at 11:14:32AM +0530, Kishon Vijay Abraham I wrote: > >>>>>>> IMHO we need a lookup method for PHYs, just like for clocks, > >>>>>>> regulators, PWMs or even i2c busses because there are complex > >>>>>>> cases > >>>>>>> when passing just a name using platform data will not work. I > >>>>>>> would > >>>>>>> second what Stephen said [1] and define a structure doing things > >>>>>>> in a > >>>>>>> DT-like way. > >>>>>>> > >>>>>>> Example; > >>>>>>> > >>>>>>> [platform code] > >>>>>>> > >>>>>>> static const struct phy_lookup my_phy_lookup[] = { > >>>>>>> > >>>>>>> PHY_LOOKUP("s3c-hsotg.0", "otg", "samsung-usbphy.1", "phy.2"), > >>>>>> > >>>>>> The only problem here is that if *PLATFORM_DEVID_AUTO* is used > >>>>>> while > >>>>>> creating the device, the ids in the device name would change and > >>>>>> PHY_LOOKUP wont be useful. > >>>>> > >>>>> I don't think this is a problem. All the existing lookup methods > >>>>> already > >>>>> use ID to identify devices (see regulators, clkdev, PWMs, i2c, > >>>>> ...). You > >>>>> can simply add a requirement that the ID must be assigned manually, > >>>>> without using PLATFORM_DEVID_AUTO to use PHY lookup. > >>>> > >>>> And I'm saying that this idea, of using a specific name and id, is > >>>> frought with fragility and will break in the future in various ways > >>>> when > >>>> devices get added to systems, making these strings constantly have > >>>> to be > >>>> kept up to date with different board configurations. > >>>> > >>>> People, NEVER, hardcode something like an id. The fact that this > >>>> happens today with the clock code, doesn't make it right, it makes > >>>> the > >>>> clock code wrong. Others have already said that this is wrong there > >>>> as > >>>> well, as systems change and dynamic ids get used more and more. > >>>> > >>>> Let's not repeat the same mistakes of the past just because we > >>>> refuse to > >>>> learn from them... > >>>> > >>>> So again, the "find a phy by a string" functions should be removed, > >>>> the > >>>> device id should be automatically created by the phy core just to > >>>> make > >>>> things unique in sysfs, and no driver code should _ever_ be reliant > >>>> on > >>>> the number that is being created, and the pointer to the phy > >>>> structure > >>>> should be used everywhere instead. > >>>> > >>>> With those types of changes, I will consider merging this subsystem, > >>>> but > >>>> without them, sorry, I will not. > >>> > >>> I'll agree with Greg here, the very fact that we see people trying to > >>> add a requirement of *NOT* using PLATFORM_DEVID_AUTO already points > >>> to a > >>> big problem in the framework. > >>> > >>> The fact is that if we don't allow PLATFORM_DEVID_AUTO we will end up > >>> adding similar infrastructure to the driver themselves to make sure > >>> we > >>> don't end up with duplicate names in sysfs in case we have multiple > >>> instances of the same IP in the SoC (or several of the same PCIe > >>> card). > >>> I really don't want to go back to that. > >> > >> If we are using PLATFORM_DEVID_AUTO, then I dont see any way we can > >> give the correct binding information to the PHY framework. I think we > >> can drop having this non-dt support in PHY framework? I see only one > >> platform (OMAP3) going to be needing this non-dt support and we can > >> use the USB PHY library for it.> > > you shouldn't drop support for non-DT platform, in any case we lived > > without DT (and still do) for years. Gotta find a better way ;-) > > hmm.. > > how about passing the device names of PHY in platform data of the > controller? It should be deterministic as the PHY framework assigns its > own id and we *don't* want to add any requirement that the ID must be > assigned manually without using PLATFORM_DEVID_AUTO. We can get rid of > *phy_init_data* in the v10 patch series. What about slightly altering the concept of v9 to pass a pointer to struct device instead of device name inside phy_init_data? Best regards, Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html