Re: [PATCH 1/6] usb: chipidea: add a core function to setup ci_hdrc_platform_data

2014-11-16 Thread Peter Chen 
On Fri, Nov 14, 2014 at 04:34:55PM +0100, Antoine Tenart wrote:
 Peter,
 
 On Fri, Nov 14, 2014 at 09:16:55AM +0800, Peter Chen wrote:
  
  Ok, Antoine, I find this patch set may not have many benefits due to
  below reasons:
  - There is already function ci_get_platdata to do the similar things
  - If the PHY can't get from the DT, it will use devm_phy_get or
  devm_usb_get_phy to get, this code has already in core.
  
  I am apologized that I wanted you to do a patch for moving get PHY
  operation to core, it doesn't have many benefits currently
  - For non-dt user, the phy get function has already in core like I said
  above.
  - For dt generic phy user, it uses of_phy_get, the second parameter
  con_id may be different for platforms.
  - For dt usb phy, it uses devm_usb_get_phy_by_phandle, the second
  parameters phandle may be different for platforms.
 
 Ok.
 
  So, please rebase my next tree which your generic phy patch set has
  already in, and send your generic usb2 chipidea patch base on it.
 
 I just rebased my usb2 ci generic driver series and sent it to you and
 to the mailing lists. Since there is not reason from now to delay it, I
 expect it to be merged soon. I wouldn't want to miss yet another release.
 
 Thanks,
 

I hope soon, just your generic phy patch set changed lots of things, and
spent too much time on reviewing.

-- 

Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/6] usb: chipidea: add a core function to setup ci_hdrc_platform_data

2014-11-14 Thread Antoine Tenart 
Peter,

On Fri, Nov 14, 2014 at 09:16:55AM +0800, Peter Chen wrote:
 
 Ok, Antoine, I find this patch set may not have many benefits due to
 below reasons:
 - There is already function ci_get_platdata to do the similar things
 - If the PHY can't get from the DT, it will use devm_phy_get or
 devm_usb_get_phy to get, this code has already in core.
 
 I am apologized that I wanted you to do a patch for moving get PHY
 operation to core, it doesn't have many benefits currently
 - For non-dt user, the phy get function has already in core like I said
 above.
 - For dt generic phy user, it uses of_phy_get, the second parameter
 con_id may be different for platforms.
 - For dt usb phy, it uses devm_usb_get_phy_by_phandle, the second
 parameters phandle may be different for platforms.

Ok.

 So, please rebase my next tree which your generic phy patch set has
 already in, and send your generic usb2 chipidea patch base on it.

I just rebased my usb2 ci generic driver series and sent it to you and
to the mailing lists. Since there is not reason from now to delay it, I
expect it to be merged soon. I wouldn't want to miss yet another release.

Thanks,

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/6] usb: chipidea: add a core function to setup ci_hdrc_platform_data

2014-11-13 Thread Peter Chen 
On Thu, Oct 30, 2014 at 12:36:42PM +0100, Antoine Tenart wrote:
 Add a function into the chipidea core to help drivers setup the internal
 ci_hdrc_platform_data structure. This helps not duplicating common code.
 
 The ci_hdrc_get_platdata function only setup non filled members of the
 structure so that is is possible to give an already filled one. This is
 what the ci_pdata_default parameter is for.
 
 Signed-off-by: Antoine Tenart antoine.ten...@free-electrons.com
 ---
  drivers/usb/chipidea/core.c  | 129 
 +++
  include/linux/usb/chipidea.h |   2 +
  2 files changed, 131 insertions(+)
 
 diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
 index ba0ac2723098..0ad55c10a903 100644
 --- a/drivers/usb/chipidea/core.c
 +++ b/drivers/usb/chipidea/core.c
 @@ -535,6 +535,135 @@ static int ci_get_platdata(struct device *dev,
   return 0;
  }
  
 +/*
 + * Getting a PHY or an USB PHY is optional:
 + * If no PHY or USB PHY is found, or if their subsystems aren't enabled,
 + * PHY and/or USB PHY will be set to NULL. Otherwise returns an error.
 + */
 +static int ci_hdrc_get_phy(struct device *dev,
 +struct ci_hdrc_platform_data *ci_pdata)
 +{
 + ci_pdata-phy = devm_phy_get(dev, usb);
 + ci_pdata-usb_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
 +
 + if (PTR_ERR(ci_pdata-phy) == -EPROBE_DEFER ||
 + PTR_ERR(ci_pdata-usb_phy) == -EPROBE_DEFER)
 + return -EPROBE_DEFER;
 +
 + if (IS_ERR(ci_pdata-phy)) {
 + if (PTR_ERR(ci_pdata-phy) == -ENOSYS ||
 + PTR_ERR(ci_pdata-phy) == -ENODEV) {
 + ci_pdata-phy = NULL;
 + } else {
 + dev_err(dev, Could not get PHY: %ld\n,
 + PTR_ERR(ci_pdata-phy));
 + return PTR_ERR(ci_pdata-phy);
 + }
 + }
 +
 + if (IS_ERR(ci_pdata-usb_phy)) {
 + if (PTR_ERR(ci_pdata-usb_phy) == -ENXIO ||
 + PTR_ERR(ci_pdata-usb_phy) == -ENODEV) {
 + ci_pdata-usb_phy = NULL;
 + } else {
 + dev_err(dev, Could not get USB PHY: %ld\n,
 + PTR_ERR(ci_pdata-usb_phy));
 + return PTR_ERR(ci_pdata-usb_phy);
 + }
 + }
 +
 + return 0;
 +}
 +
 +static int ci_hdrc_get_usb_phy_mode(struct device *dev,
 + struct ci_hdrc_platform_data *ci_pdata)
 +{
 + if (!ci_pdata-phy_mode)
 + ci_pdata-phy_mode = of_usb_get_phy_mode(dev-of_node);
 +
 + if (!ci_pdata-dr_mode)
 + ci_pdata-dr_mode = of_usb_get_dr_mode(dev-of_node);
 +
 + if (of_usb_get_maximum_speed(dev-of_node) == USB_SPEED_FULL)
 + ci_pdata-flags |= CI_HDRC_FORCE_FULLSPEED;
 +
 + return 0;
 +}
 +

The things in above function are no relationship.

 +/*
 + * Getting a regulator is optional:
 + * If no regulator is found, or if the regulator subsystem isn't enabled,
 + * the regulator will be set to NULL. Otherwise returns an error.
 + */
 +static int ci_hdrc_get_regulator(struct device *dev,
 +  struct ci_hdrc_platform_data *ci_pdata)
 +{
 + ci_pdata-reg_vbus = devm_regulator_get(dev, vbus);
 +
 + if (IS_ERR(ci_pdata-reg_vbus)) {
 + if (PTR_ERR(ci_pdata-reg_vbus) == -EPROBE_DEFER)
 + return -EPROBE_DEFER;
 +
 + if (PTR_ERR(ci_pdata-reg_vbus) == -ENODEV) {
 + ci_pdata-reg_vbus = NULL;
 + } else {
 + dev_err(dev, Could not get regulator for vbus: %ld\n,
 + PTR_ERR(ci_pdata-reg_vbus));
 + return PTR_ERR(ci_pdata-reg_vbus);
 + }
 + }
 +
 + return 0;
 +}
 +
 +struct ci_hdrc_platform_data *ci_hdrc_get_platdata(struct device *dev,
 + struct ci_hdrc_platform_data *ci_pdata_default)
 +{
 + struct ci_hdrc_platform_data *ci_pdata;
 + int ret;
 +
 + if (!ci_pdata_default) {
 + ci_pdata = devm_kzalloc(dev, sizeof(*ci_pdata), GFP_KERNEL);
 + if (!ci_pdata)
 + return ERR_PTR(-ENOMEM);
 + } else {
 + ci_pdata = ci_pdata_default;
 + }
 +
 + if (!ci_pdata-name)
 + ci_pdata-name = dev_name(dev);
 +
 + if (!ci_pdata-phy  !ci_pdata-usb_phy) {
 + ret = ci_hdrc_get_phy(dev, ci_pdata);
 + if (ret)
 + return ERR_PTR(ret);
 + }
 +
 + if (ci_pdata-usb_phy) {
 + ret = ci_hdrc_get_usb_phy_mode(dev, ci_pdata);
 + if (ret)
 + return ERR_PTR(ret);
 + }
 +
 + if (ci_pdata-dr_mode == USB_DR_MODE_UNKNOWN)
 + ci_pdata-dr_mode = USB_DR_MODE_OTG;
 +
 + if (ci_pdata-dr_mode != USB_DR_MODE_PERIPHERAL) {
 + if (!ci_pdata-reg_vbus) {

[PATCH 1/6] usb: chipidea: add a core function to setup ci_hdrc_platform_data

2014-10-30 Thread Antoine Tenart 
Add a function into the chipidea core to help drivers setup the internal
ci_hdrc_platform_data structure. This helps not duplicating common code.

The ci_hdrc_get_platdata function only setup non filled members of the
structure so that is is possible to give an already filled one. This is
what the ci_pdata_default parameter is for.

Signed-off-by: Antoine Tenart antoine.ten...@free-electrons.com
---
 drivers/usb/chipidea/core.c  | 129 +++
 include/linux/usb/chipidea.h |   2 +
 2 files changed, 131 insertions(+)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index ba0ac2723098..0ad55c10a903 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -535,6 +535,135 @@ static int ci_get_platdata(struct device *dev,
return 0;
 }
 
+/*
+ * Getting a PHY or an USB PHY is optional:
+ * If no PHY or USB PHY is found, or if their subsystems aren't enabled,
+ * PHY and/or USB PHY will be set to NULL. Otherwise returns an error.
+ */
+static int ci_hdrc_get_phy(struct device *dev,
+  struct ci_hdrc_platform_data *ci_pdata)
+{
+   ci_pdata-phy = devm_phy_get(dev, usb);
+   ci_pdata-usb_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
+
+   if (PTR_ERR(ci_pdata-phy) == -EPROBE_DEFER ||
+   PTR_ERR(ci_pdata-usb_phy) == -EPROBE_DEFER)
+   return -EPROBE_DEFER;
+
+   if (IS_ERR(ci_pdata-phy)) {
+   if (PTR_ERR(ci_pdata-phy) == -ENOSYS ||
+   PTR_ERR(ci_pdata-phy) == -ENODEV) {
+   ci_pdata-phy = NULL;
+   } else {
+   dev_err(dev, Could not get PHY: %ld\n,
+   PTR_ERR(ci_pdata-phy));
+   return PTR_ERR(ci_pdata-phy);
+   }
+   }
+
+   if (IS_ERR(ci_pdata-usb_phy)) {
+   if (PTR_ERR(ci_pdata-usb_phy) == -ENXIO ||
+   PTR_ERR(ci_pdata-usb_phy) == -ENODEV) {
+   ci_pdata-usb_phy = NULL;
+   } else {
+   dev_err(dev, Could not get USB PHY: %ld\n,
+   PTR_ERR(ci_pdata-usb_phy));
+   return PTR_ERR(ci_pdata-usb_phy);
+   }
+   }
+
+   return 0;
+}
+
+static int ci_hdrc_get_usb_phy_mode(struct device *dev,
+   struct ci_hdrc_platform_data *ci_pdata)
+{
+   if (!ci_pdata-phy_mode)
+   ci_pdata-phy_mode = of_usb_get_phy_mode(dev-of_node);
+
+   if (!ci_pdata-dr_mode)
+   ci_pdata-dr_mode = of_usb_get_dr_mode(dev-of_node);
+
+   if (of_usb_get_maximum_speed(dev-of_node) == USB_SPEED_FULL)
+   ci_pdata-flags |= CI_HDRC_FORCE_FULLSPEED;
+
+   return 0;
+}
+
+/*
+ * Getting a regulator is optional:
+ * If no regulator is found, or if the regulator subsystem isn't enabled,
+ * the regulator will be set to NULL. Otherwise returns an error.
+ */
+static int ci_hdrc_get_regulator(struct device *dev,
+struct ci_hdrc_platform_data *ci_pdata)
+{
+   ci_pdata-reg_vbus = devm_regulator_get(dev, vbus);
+
+   if (IS_ERR(ci_pdata-reg_vbus)) {
+   if (PTR_ERR(ci_pdata-reg_vbus) == -EPROBE_DEFER)
+   return -EPROBE_DEFER;
+
+   if (PTR_ERR(ci_pdata-reg_vbus) == -ENODEV) {
+   ci_pdata-reg_vbus = NULL;
+   } else {
+   dev_err(dev, Could not get regulator for vbus: %ld\n,
+   PTR_ERR(ci_pdata-reg_vbus));
+   return PTR_ERR(ci_pdata-reg_vbus);
+   }
+   }
+
+   return 0;
+}
+
+struct ci_hdrc_platform_data *ci_hdrc_get_platdata(struct device *dev,
+   struct ci_hdrc_platform_data *ci_pdata_default)
+{
+   struct ci_hdrc_platform_data *ci_pdata;
+   int ret;
+
+   if (!ci_pdata_default) {
+   ci_pdata = devm_kzalloc(dev, sizeof(*ci_pdata), GFP_KERNEL);
+   if (!ci_pdata)
+   return ERR_PTR(-ENOMEM);
+   } else {
+   ci_pdata = ci_pdata_default;
+   }
+
+   if (!ci_pdata-name)
+   ci_pdata-name = dev_name(dev);
+
+   if (!ci_pdata-phy  !ci_pdata-usb_phy) {
+   ret = ci_hdrc_get_phy(dev, ci_pdata);
+   if (ret)
+   return ERR_PTR(ret);
+   }
+
+   if (ci_pdata-usb_phy) {
+   ret = ci_hdrc_get_usb_phy_mode(dev, ci_pdata);
+   if (ret)
+   return ERR_PTR(ret);
+   }
+
+   if (ci_pdata-dr_mode == USB_DR_MODE_UNKNOWN)
+   ci_pdata-dr_mode = USB_DR_MODE_OTG;
+
+   if (ci_pdata-dr_mode != USB_DR_MODE_PERIPHERAL) {
+   if (!ci_pdata-reg_vbus) {
+   ret = ci_hdrc_get_regulator(dev, ci_pdata);
+   if (ret)
+