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:

quoted

Hi,

On Wed, Jul 31, 2013 at 11:14:32AM +0530, Kishon Vijay Abraham I wrote:

quoted

quoted

quoted

quoted

quoted

quoted

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