Re: [PATCH 02/15] usb: phy: omap-usb2: use the new generic PHY framework

[Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org>]

On Thursday 18 July 2013 12:51 PM, Greg KH wrote:
> On Thu, Jul 18, 2013 at 12:16:11PM +0530, Kishon Vijay Abraham I wrote:
>> Used the generic PHY framework API to create the PHY. Now the power off and
>> power on are done in omap_usb_power_off and omap_usb_power_on respectively.
>>
>> However using the old USB PHY library cannot be completely removed
>> because OTG is intertwined with PHY and moving to the new framework
>> will break OTG. Once we have a separate OTG state machine, we
>> can get rid of the USB PHY library.
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org>
>> Reviewed-by: Sylwester Nawrocki <s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> Acked-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
>> ---
>>  drivers/usb/phy/Kconfig         |    1 +
>>  drivers/usb/phy/phy-omap-usb2.c |   45 +++++++++++++++++++++++++++++++++++----
>>  2 files changed, 42 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
>> index 3622fff..cc55993 100644
>> --- a/drivers/usb/phy/Kconfig
>> +++ b/drivers/usb/phy/Kconfig
>> @@ -75,6 +75,7 @@ config OMAP_CONTROL_USB
>>  config OMAP_USB2
>>  	tristate "OMAP USB2 PHY Driver"
>>  	depends on ARCH_OMAP2PLUS
>> +	depends on GENERIC_PHY
>>  	select OMAP_CONTROL_USB
>>  	help
>>  	  Enable this to support the transceiver that is part of SOC. This
>> diff --git a/drivers/usb/phy/phy-omap-usb2.c b/drivers/usb/phy/phy-omap-usb2.c
>> index 844ab68..751b30f 100644
>> --- a/drivers/usb/phy/phy-omap-usb2.c
>> +++ b/drivers/usb/phy/phy-omap-usb2.c
>> @@ -28,6 +28,7 @@
>>  #include <linux/pm_runtime.h>
>>  #include <linux/delay.h>
>>  #include <linux/usb/omap_control_usb.h>
>> +#include <linux/phy/phy.h>
>>  
>>  /**
>>   * omap_usb2_set_comparator - links the comparator present in the sytem with
>> @@ -119,10 +120,36 @@ static int omap_usb2_suspend(struct usb_phy *x, int suspend)
>>  	return 0;
>>  }
>>  
>> +static int omap_usb_power_off(struct phy *x)
>> +{
>> +	struct omap_usb *phy = phy_get_drvdata(x);
>> +
>> +	omap_control_usb_phy_power(phy->control_dev, 0);
>> +
>> +	return 0;
>> +}
>> +
>> +static int omap_usb_power_on(struct phy *x)
>> +{
>> +	struct omap_usb *phy = phy_get_drvdata(x);
>> +
>> +	omap_control_usb_phy_power(phy->control_dev, 1);
>> +
>> +	return 0;
>> +}
>> +
>> +static struct phy_ops ops = {
>> +	.power_on	= omap_usb_power_on,
>> +	.power_off	= omap_usb_power_off,
>> +	.owner		= THIS_MODULE,
>> +};
>> +
>>  static int omap_usb2_probe(struct platform_device *pdev)
>>  {
>>  	struct omap_usb			*phy;
>> +	struct phy			*generic_phy;
>>  	struct usb_otg			*otg;
>> +	struct phy_provider		*phy_provider;
>>  
>>  	phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
>>  	if (!phy) {
>> @@ -144,6 +171,11 @@ static int omap_usb2_probe(struct platform_device *pdev)
>>  	phy->phy.otg		= otg;
>>  	phy->phy.type		= USB_PHY_TYPE_USB2;
>>  
>> +	phy_provider = devm_of_phy_provider_register(phy->dev,
>> +			of_phy_simple_xlate);
>> +	if (IS_ERR(phy_provider))
>> +		return PTR_ERR(phy_provider);
>> +
>>  	phy->control_dev = omap_get_control_dev();
>>  	if (IS_ERR(phy->control_dev)) {
>>  		dev_dbg(&pdev->dev, "Failed to get control device\n");
>> @@ -159,6 +191,15 @@ static int omap_usb2_probe(struct platform_device *pdev)
>>  	otg->start_srp		= omap_usb_start_srp;
>>  	otg->phy		= &phy->phy;
>>  
>> +	platform_set_drvdata(pdev, phy);
>> +	pm_runtime_enable(phy->dev);
>> +
>> +	generic_phy = devm_phy_create(phy->dev, 0, &ops, "omap-usb2");
>> +	if (IS_ERR(generic_phy))
>> +		return PTR_ERR(generic_phy);
> 
> So, if I have two of these controllers in my system, I can't create the
> second phy because the name for it will be identical to the first?
> That's why the phy core should handle the id, and not rely on the
> drivers to set it, as they have no idea how many they have in the
> system.

hmm.. for such cases I'll have something like PLATFORM_DEVID_AUTO.

Thanks
Kishon