Hi,

On Wednesday 14 August 2013 04:34 AM, Tomasz Figa wrote:

On Wednesday 14 of August 2013 00:19:28 Sylwester Nawrocki wrote:

quoted

W dniu 2013-08-13 14:05, Kishon Vijay Abraham I pisze:

quoted

On Tuesday 13 August 2013 05:07 PM, Tomasz Figa wrote:

quoted

On Tuesday 13 of August 2013 16:14:44 Kishon Vijay Abraham I wrote:

quoted

On Wednesday 31 July 2013 11:45 AM, Felipe Balbi wrote:

quoted

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

wrote:

quoted

quoted

quoted

quoted

quoted

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.

OK, so the PHY device name would have a fixed part, passed as
platform data of the controller and a variable part appended
by the PHY core, depending on the number of registered PHYs ?

Then same PHY names would be passed as the PHY provider driver's
platform data ?

Then if there are 2 instances of the above (same names in platform
data) how would be determined which PHY controller is linked to
which PHY supplier ?

I guess you want each device instance to have different PHY device
names already in platform data ? That might work. We probably will
be focused mostly on DT anyway. It seem without DT we are trying
to find some layer that would allow us to couple relevant devices
and overcome driver core inconvenience that it provides to means
to identify specific devices in advance. :) Your proposal sounds
reasonable, however I might be missing some details or corner cases.

quoted

quoted

What about slightly altering the concept of v9 to pass a pointer to
struct device instead of device name inside phy_init_data?

As Felipe said, we don't want to pass pointers in platform_data
to/from random subsystems. We pass data, passing pointers would
be a total mess IMHO.

Well, this is a total mess anyway... I don't really get the point of using 
PLATFORM_DEVID_AUTO. The only thing that comes to my mind is that you can 
use it if you don't care about the ID and so it can be assigned 
automatically.

However my understanding of the device ID is that it was supposed to 
provide a way to identify multiple instances of identical devices in a 
reliable way, to solve problems like the one we are trying to solve 
here...

So maybe let's stop solving an already solved problem and just state that 
you need to explicitly assign device ID to use this framework?

Felipe,
Can we have it the way I had in my v10 patch series till we find a better way?
I think this *non-dt* stuff shouldn't be blocking as most of the users are dt only?

Thanks
Kishon