Hi,

On Tuesday 23 July 2013 08:07 PM, Alan Stern wrote:

On Tue, 23 Jul 2013, Tomasz Figa wrote:

quoted

On Tuesday 23 of July 2013 09:29:32 Tomasz Figa wrote:

quoted

Hi Alan,

Thanks for helping to clarify the issues here.

quoted

quoted

quoted

Okay.  Are PHYs _always_ platform devices?

They can be i2c, spi or any other device types as well.

In those other cases, presumably there is no platform data associated
with the PHY since it isn't a platform device.  Then how does the
kernel know which controller is attached to the PHY?  Is this spelled
out in platform data associated with the PHY's i2c/spi/whatever parent?

Yes. I think we could use i2c_board_info for passing platform data.

quoted

quoted

quoted

quoted

quoted

	PHY.  Currently this information is represented by name or 

ID

quoted

quoted

quoted

quoted

	strings embedded in platform data.

right. It's embedded in the platform data of the controller.

It must also be embedded in the PHY's platform data somehow.
Otherwise, how would the kernel know which PHY to use?

By using a PHY lookup as Stephen and I suggested in our previous
replies. Without any extra data in platform data. (I have even posted a
code example.)

I don't understand, because I don't know what "a PHY lookup" does.

It is how the PHY framework finds a PHY, when the controller (say USB)requests
a PHY from the PHY framework.

quoted

quoted

quoted

In this case, it doesn't matter where the platform_device structures
are created or where the driver source code is.  Let's take a simple
example.  Suppose the system design includes a PHY named "foo".  Then
the board file could contain:

struct phy_info { ... } phy_foo;
EXPORT_SYMBOL_GPL(phy_foo);

and a header file would contain:

extern struct phy_info phy_foo;

The PHY supplier could then call phy_create(&phy_foo), and the PHY
client could call phy_find(&phy_foo).  Or something like that; make up
your own structure tags and function names.

It's still possible to have conflicts, but now two PHYs with the same
name (or a misspelled name somewhere) will cause an error at link
time.

This is incorrect, sorry. First of all it's a layering violation - you
export random driver-specific symbols from one driver to another. Then

No, that's not what I said.  Neither the PHY driver nor the controller
driver exports anything to the other.  Instead, both drivers use data
exported by the board file.

I think instead we can use the same data while creating the platform data of
the controller and the PHY.
The PHY driver while creating the PHY (using PHY framework) will also pass the
*data* it actually got from the platform data to the framework.
The PHY user driver (USB), while requesting for the PHY (from the PHY
framework) will pass the *data* it got from its platform data.
The PHY framework can do a comparison of the *data* pointers it has and return
the appropriate PHY to the controller.

quoted

quoted

imagine 4 SoCs - A, B, C, D. There are two PHY types PHY1 and PHY2 and
there are two types of consumer drivers (e.g. USB host controllers). Now
consider following mapping:

SoC	PHY	consumer
A	PHY1	HOST1
B	PHY1	HOST2
C	PHY2	HOST1
D	PHY2	HOST2

So we have to be able to use any of the PHYs with any of the host
drivers. This means you would have to export symbol with the same name
from both PHY drivers, which obviously would not work in this case,
because having both drivers enabled (in a multiplatform aware
configuration) would lead to linking conflict.

You're right; the scheme was too simple.  Instead, the board file must
export two types of data structures, one for PHYs and one for
controllers.  Like this:

struct phy_info {
	/* Info for the controller attached to this PHY */
	struct controller_info	*hinfo;
};

struct controller_info {
	/* Info for the PHY which this controller is attached to */
	struct phy_info		*pinfo;
};

The board file for SoC A would contain:

struct phy_info phy1 = {&host1);
EXPORT_SYMBOL(phy1);
struct controller_info host1 = {&phy1};
EXPORT_SYMBOL(host1);

The board file for SoC B would contain:

struct phy_info phy1 = {&host2);
EXPORT_SYMBOL(phy1);
struct controller_info host2 = {&phy1};
EXPORT_SYMBOL(host2);

I meant something like this
struct phy_info {
	const char *name;
};

struct phy_platform_data {
	.
	.
	struct phy_info *info;
};

struct usb_controller_platform_data {
	.
	.
	struct phy_info *info;
};

struct phy_info phy_info;

While creating the phy device
	struct phy_platform_data phy_data;
	phy_data.info = &info;
	platform_device_add_data(pdev, &phy_data, sizeof(*phy_data))
	platform_device_add();

While creating the controller device
	struct usb_controller_platform_data controller_data;
	controller_data.info = &info;
	platform_device_add_data(pdev, &controller_data, sizeof(*controller_data))
	platform_device_add();

Then modify PHY framework API phy create
	phy_create((struct device *dev, const struct phy_ops *ops,
        void *priv)  {//API changed to take void pointer instead of label
		. //existing implementation
		.
		phy->priv = priv;
	}

	struct phy *phy_get(struct device *dev, const char *string, void *priv) {
//API changed to take an additional pointer
		phy_lookup(priv)
	}

	static struct phy *phy_lookup(void *priv) {
		.
		.
		if (phy->priv=priv) //instead of string comparison, we'll use pointer
			return phy;
	}

PHY driver should be like
	phy_create((dev, ops, pdata->info);

The controller driver would do
	phy_get(dev, NULL, pdata->info);

Now the PHY framework will check for a match of *priv* pointer and return the PHY.

I think this should be possible?

Thanks
Kishon