Messages in this thread | ![/](/images/icornerl.gif) | | Date | Wed, 31 Jul 2013 09:15:38 +0300 | From | Felipe Balbi <> | Subject | Re: [PATCH 01/15] drivers: phy: add generic PHY framework |
| |
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 ;-)
-- balbi [unhandled content-type:application/pgp-signature] | ![\](/images/icornerr.gif) |