All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 1/4] USB: Fix of_usb_get_dr_mode_by_phy with a shared phy block
@ 2016-06-05 14:59 ` Hans de Goede
  0 siblings, 0 replies; 28+ messages in thread
From: Hans de Goede @ 2016-06-05 14:59 UTC (permalink / raw)
  To: Bin Liu, Greg Kroah-Hartman, Kishon Vijay Abraham I
  Cc: Maxime Ripard, Chen-Yu Tsai, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
	Hans de Goede

Some SoCs have a single phy-hw-block with multiple phys, this is
modelled by a single phy dts node, so we end up with multiple
controller nodes with a phys property pointing to the phy-node
of the otg-phy.

Only one of these controllers typically is an otg controller, yet we
were checking the first controller who uses a phy from the block and
then end up looking for a dr_mode property in e.g. the ehci controller.

This commit fixes this by adding an arg0 parameter to
of_usb_get_dr_mode_by_phy and make of_usb_get_dr_mode_by_phy
check that this matches the phandle args[0] value when looking for
the otg controller.

Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
Changes in v2:
-Add a args0 parameter instead of looking for nodes with a dr_mode property
Changes in v3:
-No changes
---
 drivers/usb/common/common.c  | 31 ++++++++++++++++++-------------
 drivers/usb/phy/phy-am335x.c |  2 +-
 include/linux/usb/of.h       |  4 ++--
 3 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
index e3d0161..bd716f4 100644
--- a/drivers/usb/common/common.c
+++ b/drivers/usb/common/common.c
@@ -131,32 +131,37 @@ EXPORT_SYMBOL_GPL(usb_get_dr_mode);
  * of_usb_get_dr_mode_by_phy - Get dual role mode for the controller device
  * which is associated with the given phy device_node
  * @np:	Pointer to the given phy device_node
+ * @arg0: phandle args[0] for phy's with #phy-cells >= 1
  *
  * In dts a usb controller associates with phy devices.  The function gets
  * the string from property 'dr_mode' of the controller associated with the
  * given phy device node, and returns the correspondig enum usb_dr_mode.
  */
-enum usb_dr_mode of_usb_get_dr_mode_by_phy(struct device_node *phy_np)
+enum usb_dr_mode of_usb_get_dr_mode_by_phy(struct device_node *np, int arg0)
 {
 	struct device_node *controller = NULL;
-	struct device_node *phy;
+	struct of_phandle_args args;
 	const char *dr_mode;
 	int index;
 	int err;
+	bool found = false;
 
 	do {
 		controller = of_find_node_with_property(controller, "phys");
-		index = 0;
-		do {
-			phy = of_parse_phandle(controller, "phys", index);
-			of_node_put(phy);
-			if (phy == phy_np)
-				goto finish;
-			index++;
-		} while (phy);
-	} while (controller);
-
-finish:
+		for (index = 0; !found; index++) {
+			err = of_parse_phandle_with_args(controller, "phys",
+					"#phy-cells", index, &args);
+			if (err)
+				break;
+
+			if (args.np == np && (args.args_count == 0 ||
+					      args.args[0] == arg0))
+				found = true;
+
+			of_node_put(args.np);
+		}
+	} while (controller && !found);
+
 	err = of_property_read_string(controller, "dr_mode", &dr_mode);
 	of_node_put(controller);
 
diff --git a/drivers/usb/phy/phy-am335x.c b/drivers/usb/phy/phy-am335x.c
index a262a43..7e5aece 100644
--- a/drivers/usb/phy/phy-am335x.c
+++ b/drivers/usb/phy/phy-am335x.c
@@ -54,7 +54,7 @@ static int am335x_phy_probe(struct platform_device *pdev)
 		return am_phy->id;
 	}
 
-	am_phy->dr_mode = of_usb_get_dr_mode_by_phy(pdev->dev.of_node);
+	am_phy->dr_mode = of_usb_get_dr_mode_by_phy(pdev->dev.of_node, -1);
 
 	ret = usb_phy_gen_create_phy(dev, &am_phy->usb_phy_gen, NULL);
 	if (ret)
diff --git a/include/linux/usb/of.h b/include/linux/usb/of.h
index de3237f..5ff9032 100644
--- a/include/linux/usb/of.h
+++ b/include/linux/usb/of.h
@@ -12,7 +12,7 @@
 #include <linux/usb/phy.h>
 
 #if IS_ENABLED(CONFIG_OF)
-enum usb_dr_mode of_usb_get_dr_mode_by_phy(struct device_node *phy_np);
+enum usb_dr_mode of_usb_get_dr_mode_by_phy(struct device_node *np, int arg0);
 bool of_usb_host_tpl_support(struct device_node *np);
 int of_usb_update_otg_caps(struct device_node *np,
 			struct usb_otg_caps *otg_caps);
@@ -20,7 +20,7 @@ struct device_node *usb_of_get_child_node(struct device_node *parent,
 			int portnum);
 #else
 static inline enum usb_dr_mode
-of_usb_get_dr_mode_by_phy(struct device_node *phy_np)
+of_usb_get_dr_mode_by_phy(struct device_node *np, int arg0)
 {
 	return USB_DR_MODE_UNKNOWN;
 }
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 1/4] USB: Fix of_usb_get_dr_mode_by_phy with a shared phy block
@ 2016-06-05 14:59 ` Hans de Goede
  0 siblings, 0 replies; 28+ messages in thread
From: Hans de Goede @ 2016-06-05 14:59 UTC (permalink / raw)
  To: linux-arm-kernel

Some SoCs have a single phy-hw-block with multiple phys, this is
modelled by a single phy dts node, so we end up with multiple
controller nodes with a phys property pointing to the phy-node
of the otg-phy.

Only one of these controllers typically is an otg controller, yet we
were checking the first controller who uses a phy from the block and
then end up looking for a dr_mode property in e.g. the ehci controller.

This commit fixes this by adding an arg0 parameter to
of_usb_get_dr_mode_by_phy and make of_usb_get_dr_mode_by_phy
check that this matches the phandle args[0] value when looking for
the otg controller.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Add a args0 parameter instead of looking for nodes with a dr_mode property
Changes in v3:
-No changes
---
 drivers/usb/common/common.c  | 31 ++++++++++++++++++-------------
 drivers/usb/phy/phy-am335x.c |  2 +-
 include/linux/usb/of.h       |  4 ++--
 3 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
index e3d0161..bd716f4 100644
--- a/drivers/usb/common/common.c
+++ b/drivers/usb/common/common.c
@@ -131,32 +131,37 @@ EXPORT_SYMBOL_GPL(usb_get_dr_mode);
  * of_usb_get_dr_mode_by_phy - Get dual role mode for the controller device
  * which is associated with the given phy device_node
  * @np:	Pointer to the given phy device_node
+ * @arg0: phandle args[0] for phy's with #phy-cells >= 1
  *
  * In dts a usb controller associates with phy devices.  The function gets
  * the string from property 'dr_mode' of the controller associated with the
  * given phy device node, and returns the correspondig enum usb_dr_mode.
  */
-enum usb_dr_mode of_usb_get_dr_mode_by_phy(struct device_node *phy_np)
+enum usb_dr_mode of_usb_get_dr_mode_by_phy(struct device_node *np, int arg0)
 {
 	struct device_node *controller = NULL;
-	struct device_node *phy;
+	struct of_phandle_args args;
 	const char *dr_mode;
 	int index;
 	int err;
+	bool found = false;
 
 	do {
 		controller = of_find_node_with_property(controller, "phys");
-		index = 0;
-		do {
-			phy = of_parse_phandle(controller, "phys", index);
-			of_node_put(phy);
-			if (phy == phy_np)
-				goto finish;
-			index++;
-		} while (phy);
-	} while (controller);
-
-finish:
+		for (index = 0; !found; index++) {
+			err = of_parse_phandle_with_args(controller, "phys",
+					"#phy-cells", index, &args);
+			if (err)
+				break;
+
+			if (args.np == np && (args.args_count == 0 ||
+					      args.args[0] == arg0))
+				found = true;
+
+			of_node_put(args.np);
+		}
+	} while (controller && !found);
+
 	err = of_property_read_string(controller, "dr_mode", &dr_mode);
 	of_node_put(controller);
 
diff --git a/drivers/usb/phy/phy-am335x.c b/drivers/usb/phy/phy-am335x.c
index a262a43..7e5aece 100644
--- a/drivers/usb/phy/phy-am335x.c
+++ b/drivers/usb/phy/phy-am335x.c
@@ -54,7 +54,7 @@ static int am335x_phy_probe(struct platform_device *pdev)
 		return am_phy->id;
 	}
 
-	am_phy->dr_mode = of_usb_get_dr_mode_by_phy(pdev->dev.of_node);
+	am_phy->dr_mode = of_usb_get_dr_mode_by_phy(pdev->dev.of_node, -1);
 
 	ret = usb_phy_gen_create_phy(dev, &am_phy->usb_phy_gen, NULL);
 	if (ret)
diff --git a/include/linux/usb/of.h b/include/linux/usb/of.h
index de3237f..5ff9032 100644
--- a/include/linux/usb/of.h
+++ b/include/linux/usb/of.h
@@ -12,7 +12,7 @@
 #include <linux/usb/phy.h>
 
 #if IS_ENABLED(CONFIG_OF)
-enum usb_dr_mode of_usb_get_dr_mode_by_phy(struct device_node *phy_np);
+enum usb_dr_mode of_usb_get_dr_mode_by_phy(struct device_node *np, int arg0);
 bool of_usb_host_tpl_support(struct device_node *np);
 int of_usb_update_otg_caps(struct device_node *np,
 			struct usb_otg_caps *otg_caps);
@@ -20,7 +20,7 @@ struct device_node *usb_of_get_child_node(struct device_node *parent,
 			int portnum);
 #else
 static inline enum usb_dr_mode
-of_usb_get_dr_mode_by_phy(struct device_node *phy_np)
+of_usb_get_dr_mode_by_phy(struct device_node *np, int arg0)
 {
 	return USB_DR_MODE_UNKNOWN;
 }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 2/4] phy-sun4i-usb: Add support for peripheral-only mode
  2016-06-05 14:59 ` Hans de Goede
@ 2016-06-05 14:59     ` Hans de Goede
  -1 siblings, 0 replies; 28+ messages in thread
From: Hans de Goede @ 2016-06-05 14:59 UTC (permalink / raw)
  To: Bin Liu, Greg Kroah-Hartman, Kishon Vijay Abraham I
  Cc: Maxime Ripard, Chen-Yu Tsai, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
	Hans de Goede

Use the new of_usb_get_dr_mode_by_phy() function to get the dr_mode
from the musb controller node instead of assuming that having an id_det
gpio means otg mode, and not having one means host mode.

Implement peripheral-only mode by adding a sun4i_usb_phy0_get_id_det
helper which looks at the dr_mode, always registering our extcon and
always monitoring vbus.

If dr_mode is not specified in the dts, do not register phy0 as we then
do not know how to treat it. This is actually a good thing as this means
we will not be registering phy0 on devices where the otg controller is
not enabled in the devicetree.

Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
Changes in v2:
-Adjust for of_usb_get_dr_mode_by_phy now taking an args0 parameter
Changes in v3:
-Only toggle the phy vbus-det bit on id-change on systems without vbus-det
 when in otg mode
---
 drivers/phy/phy-sun4i-usb.c | 68 ++++++++++++++++++++++++++++++---------------
 1 file changed, 46 insertions(+), 22 deletions(-)

diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index bae54f7..e3cbaae 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -40,6 +40,7 @@
 #include <linux/power_supply.h>
 #include <linux/regulator/consumer.h>
 #include <linux/reset.h>
+#include <linux/usb/of.h>
 #include <linux/workqueue.h>
 
 #define REG_ISCR			0x00
@@ -109,6 +110,7 @@ struct sun4i_usb_phy_cfg {
 struct sun4i_usb_phy_data {
 	void __iomem *base;
 	const struct sun4i_usb_phy_cfg *cfg;
+	enum usb_dr_mode dr_mode;
 	struct mutex mutex;
 	struct sun4i_usb_phy {
 		struct phy *phy;
@@ -119,6 +121,7 @@ struct sun4i_usb_phy_data {
 		bool regulator_on;
 		int index;
 	} phys[MAX_PHYS];
+	int first_phy;
 	/* phy0 / otg related variables */
 	struct extcon_dev *extcon;
 	bool phy0_init;
@@ -285,16 +288,10 @@ static int sun4i_usb_phy_init(struct phy *_phy)
 		sun4i_usb_phy0_update_iscr(_phy, 0, ISCR_DPDM_PULLUP_EN);
 		sun4i_usb_phy0_update_iscr(_phy, 0, ISCR_ID_PULLUP_EN);
 
-		if (data->id_det_gpio) {
-			/* OTG mode, force ISCR and cable state updates */
-			data->id_det = -1;
-			data->vbus_det = -1;
-			queue_delayed_work(system_wq, &data->detect, 0);
-		} else {
-			/* Host only mode */
-			sun4i_usb_phy0_set_id_detect(_phy, 0);
-			sun4i_usb_phy0_set_vbus_detect(_phy, 1);
-		}
+		/* Force ISCR and cable state updates */
+		data->id_det = -1;
+		data->vbus_det = -1;
+		queue_delayed_work(system_wq, &data->detect, 0);
 	}
 
 	return 0;
@@ -319,6 +316,19 @@ static int sun4i_usb_phy_exit(struct phy *_phy)
 	return 0;
 }
 
+static int sun4i_usb_phy0_get_id_det(struct sun4i_usb_phy_data *data)
+{
+	switch (data->dr_mode) {
+	case USB_DR_MODE_OTG:
+		return gpiod_get_value_cansleep(data->id_det_gpio);
+	case USB_DR_MODE_HOST:
+		return 0;
+	case USB_DR_MODE_PERIPHERAL:
+	default:
+		return 1;
+	}
+}
+
 static int sun4i_usb_phy0_get_vbus_det(struct sun4i_usb_phy_data *data)
 {
 	if (data->vbus_det_gpio)
@@ -414,7 +424,10 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct work_struct *work)
 	struct phy *phy0 = data->phys[0].phy;
 	int id_det, vbus_det, id_notify = 0, vbus_notify = 0;
 
-	id_det = gpiod_get_value_cansleep(data->id_det_gpio);
+	if (phy0 == NULL)
+		return;
+
+	id_det = sun4i_usb_phy0_get_id_det(data);
 	vbus_det = sun4i_usb_phy0_get_vbus_det(data);
 
 	mutex_lock(&phy0->mutex);
@@ -430,7 +443,8 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct work_struct *work)
 		 * without vbus detection report vbus low for long enough for
 		 * the musb-ip to end the current device session.
 		 */
-		if (!sun4i_usb_phy0_have_vbus_det(data) && id_det == 0) {
+		if (data->dr_mode == USB_DR_MODE_OTG &&
+		    !sun4i_usb_phy0_have_vbus_det(data) && id_det == 0) {
 			sun4i_usb_phy0_set_vbus_detect(phy0, 0);
 			msleep(200);
 			sun4i_usb_phy0_set_vbus_detect(phy0, 1);
@@ -456,7 +470,8 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct work_struct *work)
 		 * without vbus detection report vbus low for long enough to
 		 * the musb-ip to end the current host session.
 		 */
-		if (!sun4i_usb_phy0_have_vbus_det(data) && id_det == 1) {
+		if (data->dr_mode == USB_DR_MODE_OTG &&
+		    !sun4i_usb_phy0_have_vbus_det(data) && id_det == 1) {
 			mutex_lock(&phy0->mutex);
 			sun4i_usb_phy0_set_vbus_detect(phy0, 0);
 			msleep(1000);
@@ -501,7 +516,8 @@ static struct phy *sun4i_usb_phy_xlate(struct device *dev,
 {
 	struct sun4i_usb_phy_data *data = dev_get_drvdata(dev);
 
-	if (args->args[0] >= data->cfg->num_phys)
+	if (args->args[0] < data->first_phy ||
+	    args->args[0] >= data->cfg->num_phys)
 		return ERR_PTR(-ENODEV);
 
 	return data->phys[args->args[0]].phy;
@@ -575,13 +591,17 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
 			return -EPROBE_DEFER;
 	}
 
-	/* vbus_det without id_det makes no sense, and is not supported */
-	if (sun4i_usb_phy0_have_vbus_det(data) && !data->id_det_gpio) {
-		dev_err(dev, "usb0_id_det missing or invalid\n");
-		return -ENODEV;
-	}
-
-	if (data->id_det_gpio) {
+	data->dr_mode = of_usb_get_dr_mode_by_phy(np, 0);
+	switch (data->dr_mode) {
+	case USB_DR_MODE_OTG:
+		/* otg without id_det makes no sense, and is not supported */
+		if (!data->id_det_gpio) {
+			dev_err(dev, "usb0_id_det missing or invalid\n");
+			return -ENODEV;
+		}
+		/* fall through */
+	case USB_DR_MODE_HOST:
+	case USB_DR_MODE_PERIPHERAL:
 		data->extcon = devm_extcon_dev_allocate(dev,
 							sun4i_usb_phy0_cable);
 		if (IS_ERR(data->extcon))
@@ -592,9 +612,13 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
 			dev_err(dev, "failed to register extcon: %d\n", ret);
 			return ret;
 		}
+		break;
+	default:
+		dev_info(dev, "dr_mode unknown, not registering usb phy0\n");
+		data->first_phy = 1;
 	}
 
-	for (i = 0; i < data->cfg->num_phys; i++) {
+	for (i = data->first_phy; i < data->cfg->num_phys; i++) {
 		struct sun4i_usb_phy *phy = data->phys + i;
 		char name[16];
 
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 2/4] phy-sun4i-usb: Add support for peripheral-only mode
@ 2016-06-05 14:59     ` Hans de Goede
  0 siblings, 0 replies; 28+ messages in thread
From: Hans de Goede @ 2016-06-05 14:59 UTC (permalink / raw)
  To: linux-arm-kernel

Use the new of_usb_get_dr_mode_by_phy() function to get the dr_mode
from the musb controller node instead of assuming that having an id_det
gpio means otg mode, and not having one means host mode.

Implement peripheral-only mode by adding a sun4i_usb_phy0_get_id_det
helper which looks at the dr_mode, always registering our extcon and
always monitoring vbus.

If dr_mode is not specified in the dts, do not register phy0 as we then
do not know how to treat it. This is actually a good thing as this means
we will not be registering phy0 on devices where the otg controller is
not enabled in the devicetree.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Adjust for of_usb_get_dr_mode_by_phy now taking an args0 parameter
Changes in v3:
-Only toggle the phy vbus-det bit on id-change on systems without vbus-det
 when in otg mode
---
 drivers/phy/phy-sun4i-usb.c | 68 ++++++++++++++++++++++++++++++---------------
 1 file changed, 46 insertions(+), 22 deletions(-)

diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index bae54f7..e3cbaae 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -40,6 +40,7 @@
 #include <linux/power_supply.h>
 #include <linux/regulator/consumer.h>
 #include <linux/reset.h>
+#include <linux/usb/of.h>
 #include <linux/workqueue.h>
 
 #define REG_ISCR			0x00
@@ -109,6 +110,7 @@ struct sun4i_usb_phy_cfg {
 struct sun4i_usb_phy_data {
 	void __iomem *base;
 	const struct sun4i_usb_phy_cfg *cfg;
+	enum usb_dr_mode dr_mode;
 	struct mutex mutex;
 	struct sun4i_usb_phy {
 		struct phy *phy;
@@ -119,6 +121,7 @@ struct sun4i_usb_phy_data {
 		bool regulator_on;
 		int index;
 	} phys[MAX_PHYS];
+	int first_phy;
 	/* phy0 / otg related variables */
 	struct extcon_dev *extcon;
 	bool phy0_init;
@@ -285,16 +288,10 @@ static int sun4i_usb_phy_init(struct phy *_phy)
 		sun4i_usb_phy0_update_iscr(_phy, 0, ISCR_DPDM_PULLUP_EN);
 		sun4i_usb_phy0_update_iscr(_phy, 0, ISCR_ID_PULLUP_EN);
 
-		if (data->id_det_gpio) {
-			/* OTG mode, force ISCR and cable state updates */
-			data->id_det = -1;
-			data->vbus_det = -1;
-			queue_delayed_work(system_wq, &data->detect, 0);
-		} else {
-			/* Host only mode */
-			sun4i_usb_phy0_set_id_detect(_phy, 0);
-			sun4i_usb_phy0_set_vbus_detect(_phy, 1);
-		}
+		/* Force ISCR and cable state updates */
+		data->id_det = -1;
+		data->vbus_det = -1;
+		queue_delayed_work(system_wq, &data->detect, 0);
 	}
 
 	return 0;
@@ -319,6 +316,19 @@ static int sun4i_usb_phy_exit(struct phy *_phy)
 	return 0;
 }
 
+static int sun4i_usb_phy0_get_id_det(struct sun4i_usb_phy_data *data)
+{
+	switch (data->dr_mode) {
+	case USB_DR_MODE_OTG:
+		return gpiod_get_value_cansleep(data->id_det_gpio);
+	case USB_DR_MODE_HOST:
+		return 0;
+	case USB_DR_MODE_PERIPHERAL:
+	default:
+		return 1;
+	}
+}
+
 static int sun4i_usb_phy0_get_vbus_det(struct sun4i_usb_phy_data *data)
 {
 	if (data->vbus_det_gpio)
@@ -414,7 +424,10 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct work_struct *work)
 	struct phy *phy0 = data->phys[0].phy;
 	int id_det, vbus_det, id_notify = 0, vbus_notify = 0;
 
-	id_det = gpiod_get_value_cansleep(data->id_det_gpio);
+	if (phy0 == NULL)
+		return;
+
+	id_det = sun4i_usb_phy0_get_id_det(data);
 	vbus_det = sun4i_usb_phy0_get_vbus_det(data);
 
 	mutex_lock(&phy0->mutex);
@@ -430,7 +443,8 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct work_struct *work)
 		 * without vbus detection report vbus low for long enough for
 		 * the musb-ip to end the current device session.
 		 */
-		if (!sun4i_usb_phy0_have_vbus_det(data) && id_det == 0) {
+		if (data->dr_mode == USB_DR_MODE_OTG &&
+		    !sun4i_usb_phy0_have_vbus_det(data) && id_det == 0) {
 			sun4i_usb_phy0_set_vbus_detect(phy0, 0);
 			msleep(200);
 			sun4i_usb_phy0_set_vbus_detect(phy0, 1);
@@ -456,7 +470,8 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct work_struct *work)
 		 * without vbus detection report vbus low for long enough to
 		 * the musb-ip to end the current host session.
 		 */
-		if (!sun4i_usb_phy0_have_vbus_det(data) && id_det == 1) {
+		if (data->dr_mode == USB_DR_MODE_OTG &&
+		    !sun4i_usb_phy0_have_vbus_det(data) && id_det == 1) {
 			mutex_lock(&phy0->mutex);
 			sun4i_usb_phy0_set_vbus_detect(phy0, 0);
 			msleep(1000);
@@ -501,7 +516,8 @@ static struct phy *sun4i_usb_phy_xlate(struct device *dev,
 {
 	struct sun4i_usb_phy_data *data = dev_get_drvdata(dev);
 
-	if (args->args[0] >= data->cfg->num_phys)
+	if (args->args[0] < data->first_phy ||
+	    args->args[0] >= data->cfg->num_phys)
 		return ERR_PTR(-ENODEV);
 
 	return data->phys[args->args[0]].phy;
@@ -575,13 +591,17 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
 			return -EPROBE_DEFER;
 	}
 
-	/* vbus_det without id_det makes no sense, and is not supported */
-	if (sun4i_usb_phy0_have_vbus_det(data) && !data->id_det_gpio) {
-		dev_err(dev, "usb0_id_det missing or invalid\n");
-		return -ENODEV;
-	}
-
-	if (data->id_det_gpio) {
+	data->dr_mode = of_usb_get_dr_mode_by_phy(np, 0);
+	switch (data->dr_mode) {
+	case USB_DR_MODE_OTG:
+		/* otg without id_det makes no sense, and is not supported */
+		if (!data->id_det_gpio) {
+			dev_err(dev, "usb0_id_det missing or invalid\n");
+			return -ENODEV;
+		}
+		/* fall through */
+	case USB_DR_MODE_HOST:
+	case USB_DR_MODE_PERIPHERAL:
 		data->extcon = devm_extcon_dev_allocate(dev,
 							sun4i_usb_phy0_cable);
 		if (IS_ERR(data->extcon))
@@ -592,9 +612,13 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
 			dev_err(dev, "failed to register extcon: %d\n", ret);
 			return ret;
 		}
+		break;
+	default:
+		dev_info(dev, "dr_mode unknown, not registering usb phy0\n");
+		data->first_phy = 1;
 	}
 
-	for (i = 0; i < data->cfg->num_phys; i++) {
+	for (i = data->first_phy; i < data->cfg->num_phys; i++) {
 		struct sun4i_usb_phy *phy = data->phys + i;
 		char name[16];
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 3/4] phy-sun4i-usb: Add workaround for missing Vbus det interrupts on A31
  2016-06-05 14:59 ` Hans de Goede
@ 2016-06-05 14:59     ` Hans de Goede
  -1 siblings, 0 replies; 28+ messages in thread
From: Hans de Goede @ 2016-06-05 14:59 UTC (permalink / raw)
  To: Bin Liu, Greg Kroah-Hartman, Kishon Vijay Abraham I
  Cc: Maxime Ripard, Chen-Yu Tsai, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
	Hans de Goede

The A31 companion pmic (axp221) does not generate vbus change interrupts
when the board is driving vbus, so we must poll when using the pmic for
vbus-det _and_ we're driving vbus.

Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
Changes in v2:
-No changes
Changes in v3:
-No changes
---
 drivers/phy/phy-sun4i-usb.c | 34 ++++++++++++++++++++++++----------
 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index e3cbaae..a7abae6 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -95,6 +95,7 @@
 
 enum sun4i_usb_phy_type {
 	sun4i_a10_phy,
+	sun6i_a31_phy,
 	sun8i_a33_phy,
 	sun8i_h3_phy,
 };
@@ -125,7 +126,6 @@ struct sun4i_usb_phy_data {
 	/* phy0 / otg related variables */
 	struct extcon_dev *extcon;
 	bool phy0_init;
-	bool phy0_poll;
 	struct gpio_desc *id_det_gpio;
 	struct gpio_desc *vbus_det_gpio;
 	struct power_supply *vbus_power_supply;
@@ -353,6 +353,24 @@ static bool sun4i_usb_phy0_have_vbus_det(struct sun4i_usb_phy_data *data)
 	return data->vbus_det_gpio || data->vbus_power_supply;
 }
 
+static bool sun4i_usb_phy0_poll(struct sun4i_usb_phy_data *data)
+{
+	if ((data->id_det_gpio && data->id_det_irq < 0) ||
+	    (data->vbus_det_gpio && data->vbus_det_irq < 0))
+		return true;
+
+	/*
+	 * The A31 companion pmic (axp221) does not generate vbus change
+	 * interrupts when the board is driving vbus, so we must poll
+	 * when using the pmic for vbus-det _and_ we're driving vbus.
+	 */
+	if (data->cfg->type == sun6i_a31_phy &&
+	    data->vbus_power_supply && data->phys[0].regulator_on)
+		return true;
+
+	return false;
+}
+
 static int sun4i_usb_phy_power_on(struct phy *_phy)
 {
 	struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
@@ -374,7 +392,7 @@ static int sun4i_usb_phy_power_on(struct phy *_phy)
 	phy->regulator_on = true;
 
 	/* We must report Vbus high within OTG_TIME_A_WAIT_VRISE msec. */
-	if (phy->index == 0 && data->vbus_det_gpio && data->phy0_poll)
+	if (phy->index == 0 && sun4i_usb_phy0_poll(data))
 		mod_delayed_work(system_wq, &data->detect, DEBOUNCE_TIME);
 
 	return 0;
@@ -395,7 +413,7 @@ static int sun4i_usb_phy_power_off(struct phy *_phy)
 	 * phy0 vbus typically slowly discharges, sometimes this causes the
 	 * Vbus gpio to not trigger an edge irq on Vbus off, so force a rescan.
 	 */
-	if (phy->index == 0 && data->vbus_det_gpio && !data->phy0_poll)
+	if (phy->index == 0 && !sun4i_usb_phy0_poll(data))
 		mod_delayed_work(system_wq, &data->detect, POLL_TIME);
 
 	return 0;
@@ -483,7 +501,7 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct work_struct *work)
 	if (vbus_notify)
 		extcon_set_cable_state_(data->extcon, EXTCON_USB, vbus_det);
 
-	if (data->phy0_poll)
+	if (sun4i_usb_phy0_poll(data))
 		queue_delayed_work(system_wq, &data->detect, POLL_TIME);
 }
 
@@ -668,11 +686,6 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
 	}
 
 	data->id_det_irq = gpiod_to_irq(data->id_det_gpio);
-	data->vbus_det_irq = gpiod_to_irq(data->vbus_det_gpio);
-	if ((data->id_det_gpio && data->id_det_irq < 0) ||
-	    (data->vbus_det_gpio && data->vbus_det_irq < 0))
-		data->phy0_poll = true;

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 3/4] phy-sun4i-usb: Add workaround for missing Vbus det interrupts on A31
@ 2016-06-05 14:59     ` Hans de Goede
  0 siblings, 0 replies; 28+ messages in thread
From: Hans de Goede @ 2016-06-05 14:59 UTC (permalink / raw)
  To: linux-arm-kernel

The A31 companion pmic (axp221) does not generate vbus change interrupts
when the board is driving vbus, so we must poll when using the pmic for
vbus-det _and_ we're driving vbus.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-No changes
Changes in v3:
-No changes
---
 drivers/phy/phy-sun4i-usb.c | 34 ++++++++++++++++++++++++----------
 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index e3cbaae..a7abae6 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -95,6 +95,7 @@
 
 enum sun4i_usb_phy_type {
 	sun4i_a10_phy,
+	sun6i_a31_phy,
 	sun8i_a33_phy,
 	sun8i_h3_phy,
 };
@@ -125,7 +126,6 @@ struct sun4i_usb_phy_data {
 	/* phy0 / otg related variables */
 	struct extcon_dev *extcon;
 	bool phy0_init;
-	bool phy0_poll;
 	struct gpio_desc *id_det_gpio;
 	struct gpio_desc *vbus_det_gpio;
 	struct power_supply *vbus_power_supply;
@@ -353,6 +353,24 @@ static bool sun4i_usb_phy0_have_vbus_det(struct sun4i_usb_phy_data *data)
 	return data->vbus_det_gpio || data->vbus_power_supply;
 }
 
+static bool sun4i_usb_phy0_poll(struct sun4i_usb_phy_data *data)
+{
+	if ((data->id_det_gpio && data->id_det_irq < 0) ||
+	    (data->vbus_det_gpio && data->vbus_det_irq < 0))
+		return true;
+
+	/*
+	 * The A31 companion pmic (axp221) does not generate vbus change
+	 * interrupts when the board is driving vbus, so we must poll
+	 * when using the pmic for vbus-det _and_ we're driving vbus.
+	 */
+	if (data->cfg->type == sun6i_a31_phy &&
+	    data->vbus_power_supply && data->phys[0].regulator_on)
+		return true;
+
+	return false;
+}
+
 static int sun4i_usb_phy_power_on(struct phy *_phy)
 {
 	struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
@@ -374,7 +392,7 @@ static int sun4i_usb_phy_power_on(struct phy *_phy)
 	phy->regulator_on = true;
 
 	/* We must report Vbus high within OTG_TIME_A_WAIT_VRISE msec. */
-	if (phy->index == 0 && data->vbus_det_gpio && data->phy0_poll)
+	if (phy->index == 0 && sun4i_usb_phy0_poll(data))
 		mod_delayed_work(system_wq, &data->detect, DEBOUNCE_TIME);
 
 	return 0;
@@ -395,7 +413,7 @@ static int sun4i_usb_phy_power_off(struct phy *_phy)
 	 * phy0 vbus typically slowly discharges, sometimes this causes the
 	 * Vbus gpio to not trigger an edge irq on Vbus off, so force a rescan.
 	 */
-	if (phy->index == 0 && data->vbus_det_gpio && !data->phy0_poll)
+	if (phy->index == 0 && !sun4i_usb_phy0_poll(data))
 		mod_delayed_work(system_wq, &data->detect, POLL_TIME);
 
 	return 0;
@@ -483,7 +501,7 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct work_struct *work)
 	if (vbus_notify)
 		extcon_set_cable_state_(data->extcon, EXTCON_USB, vbus_det);
 
-	if (data->phy0_poll)
+	if (sun4i_usb_phy0_poll(data))
 		queue_delayed_work(system_wq, &data->detect, POLL_TIME);
 }
 
@@ -668,11 +686,6 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
 	}
 
 	data->id_det_irq = gpiod_to_irq(data->id_det_gpio);
-	data->vbus_det_irq = gpiod_to_irq(data->vbus_det_gpio);
-	if ((data->id_det_gpio && data->id_det_irq < 0) ||
-	    (data->vbus_det_gpio && data->vbus_det_irq < 0))
-		data->phy0_poll = true;
-
 	if (data->id_det_irq >= 0) {
 		ret = devm_request_irq(dev, data->id_det_irq,
 				sun4i_usb_phy0_id_vbus_det_irq,
@@ -684,6 +697,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
 		}
 	}
 
+	data->vbus_det_irq = gpiod_to_irq(data->vbus_det_gpio);
 	if (data->vbus_det_irq >= 0) {
 		ret = devm_request_irq(dev, data->vbus_det_irq,
 				sun4i_usb_phy0_id_vbus_det_irq,
@@ -735,7 +749,7 @@ static const struct sun4i_usb_phy_cfg sun5i_a13_cfg = {
 
 static const struct sun4i_usb_phy_cfg sun6i_a31_cfg = {
 	.num_phys = 3,
-	.type = sun4i_a10_phy,
+	.type = sun6i_a31_phy,
 	.disc_thresh = 3,
 	.phyctl_offset = REG_PHYCTL_A10,
 	.dedicated_clocks = true,
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 4/4] musb: sunxi: Simplify dr_mode handling
  2016-06-05 14:59 ` Hans de Goede
@ 2016-06-05 14:59     ` Hans de Goede
  -1 siblings, 0 replies; 28+ messages in thread
From: Hans de Goede @ 2016-06-05 14:59 UTC (permalink / raw)
  To: Bin Liu, Greg Kroah-Hartman, Kishon Vijay Abraham I
  Cc: Maxime Ripard, Chen-Yu Tsai, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
	Hans de Goede

phy-sun4i-usb now has proper dr_mode handling, it always registers an
extcon, and sends a notify with the mode (even when in peripheral- /
host-only mode) at least once.

So we can simply the sunxi musb glue by always registering its extcon
notifier and relying on sunxi_musb_work() to enable vbus when in
host-only mode.

This also enables host- and peripheral-only mode with vbus monitoring.

Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
Changes in v2:
-No changes
Changes in v3:
-No changes
---
 drivers/usb/musb/sunxi.c | 68 ++++++++++++++++++------------------------------
 1 file changed, 25 insertions(+), 43 deletions(-)

diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
index e7d4617..b88a2f6 100644
--- a/drivers/usb/musb/sunxi.c
+++ b/drivers/usb/musb/sunxi.c
@@ -255,12 +255,10 @@ static int sunxi_musb_init(struct musb *musb)
 	writeb(SUNXI_MUSB_VEND0_PIO_MODE, musb->mregs + SUNXI_MUSB_VEND0);
 
 	/* Register notifier before calling phy_init() */
-	if (musb->port_mode == MUSB_PORT_MODE_DUAL_ROLE) {
-		ret = extcon_register_notifier(glue->extcon, EXTCON_USB_HOST,
-					       &glue->host_nb);
-		if (ret)
-			goto error_reset_assert;
-	}
+	ret = extcon_register_notifier(glue->extcon, EXTCON_USB_HOST,
+				       &glue->host_nb);
+	if (ret)
+		goto error_reset_assert;
 
 	ret = phy_init(glue->phy);
 	if (ret)
@@ -274,9 +272,8 @@ static int sunxi_musb_init(struct musb *musb)
 	return 0;
 
 error_unregister_notifier:
-	if (musb->port_mode == MUSB_PORT_MODE_DUAL_ROLE)
-		extcon_unregister_notifier(glue->extcon, EXTCON_USB_HOST,
-					   &glue->host_nb);
+	extcon_unregister_notifier(glue->extcon, EXTCON_USB_HOST,
+				   &glue->host_nb);
 error_reset_assert:
 	if (test_bit(SUNXI_MUSB_FL_HAS_RESET, &glue->flags))
 		reset_control_assert(glue->rst);
@@ -300,9 +297,8 @@ static int sunxi_musb_exit(struct musb *musb)
 
 	phy_exit(glue->phy);
 
-	if (musb->port_mode == MUSB_PORT_MODE_DUAL_ROLE)
-		extcon_unregister_notifier(glue->extcon, EXTCON_USB_HOST,
-					   &glue->host_nb);
+	extcon_unregister_notifier(glue->extcon, EXTCON_USB_HOST,
+				   &glue->host_nb);
 
 	if (test_bit(SUNXI_MUSB_FL_HAS_RESET, &glue->flags))
 		reset_control_assert(glue->rst);
@@ -314,25 +310,6 @@ static int sunxi_musb_exit(struct musb *musb)
 	return 0;
 }
 
-static int sunxi_set_mode(struct musb *musb, u8 mode)
-{
-	struct sunxi_glue *glue = dev_get_drvdata(musb->controller->parent);
-	int ret;
-
-	if (mode == MUSB_HOST) {
-		ret = phy_power_on(glue->phy);
-		if (ret)
-			return ret;
-
-		set_bit(SUNXI_MUSB_FL_PHY_ON, &glue->flags);
-		/* Stop musb work from turning vbus off again */
-		set_bit(SUNXI_MUSB_FL_VBUS_ON, &glue->flags);
-		musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE;
-	}
-
-	return 0;
-}
-
 static void sunxi_musb_enable(struct musb *musb)
 {
 	struct sunxi_glue *glue = dev_get_drvdata(musb->controller->parent);
@@ -579,7 +556,6 @@ static const struct musb_platform_ops sunxi_musb_ops = {
 	.exit		= sunxi_musb_exit,
 	.enable		= sunxi_musb_enable,
 	.disable	= sunxi_musb_disable,
-	.set_mode	= sunxi_set_mode,
 	.fifo_offset	= sunxi_musb_fifo_offset,
 	.ep_offset	= sunxi_musb_ep_offset,
 	.busctl_offset	= sunxi_musb_busctl_offset,
@@ -635,10 +611,6 @@ static int sunxi_musb_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	glue = devm_kzalloc(&pdev->dev, sizeof(*glue), GFP_KERNEL);
-	if (!glue)
-		return -ENOMEM;
-
 	memset(&pdata, 0, sizeof(pdata));
 	switch (usb_get_dr_mode(&pdev->dev)) {
 #if defined CONFIG_USB_MUSB_DUAL_ROLE || defined CONFIG_USB_MUSB_HOST
@@ -646,15 +618,13 @@ static int sunxi_musb_probe(struct platform_device *pdev)
 		pdata.mode = MUSB_PORT_MODE_HOST;
 		break;
 #endif
+#if defined CONFIG_USB_MUSB_DUAL_ROLE || defined CONFIG_USB_MUSB_GADGET
+	case USB_DR_MODE_PERIPHERAL:
+		pdata.mode = MUSB_PORT_MODE_GADGET;
+		break;
+#endif
 #ifdef CONFIG_USB_MUSB_DUAL_ROLE
 	case USB_DR_MODE_OTG:
-		glue->extcon = extcon_get_edev_by_phandle(&pdev->dev, 0);
-		if (IS_ERR(glue->extcon)) {
-			if (PTR_ERR(glue->extcon) == -EPROBE_DEFER)
-				return -EPROBE_DEFER;
-			dev_err(&pdev->dev, "Invalid or missing extcon\n");
-			return PTR_ERR(glue->extcon);
-		}
 		pdata.mode = MUSB_PORT_MODE_DUAL_ROLE;
 		break;
 #endif
@@ -665,6 +635,10 @@ static int sunxi_musb_probe(struct platform_device *pdev)
 	pdata.platform_ops	= &sunxi_musb_ops;
 	pdata.config		= &sunxi_musb_hdrc_config;
 
+	glue = devm_kzalloc(&pdev->dev, sizeof(*glue), GFP_KERNEL);
+	if (!glue)
+		return -ENOMEM;
+
 	glue->dev = &pdev->dev;
 	INIT_WORK(&glue->work, sunxi_musb_work);
 	glue->host_nb.notifier_call = sunxi_musb_host_notifier;
@@ -698,6 +672,14 @@ static int sunxi_musb_probe(struct platform_device *pdev)
 		}
 	}
 
+	glue->extcon = extcon_get_edev_by_phandle(&pdev->dev, 0);
+	if (IS_ERR(glue->extcon)) {
+		if (PTR_ERR(glue->extcon) == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
+		dev_err(&pdev->dev, "Invalid or missing extcon\n");
+		return PTR_ERR(glue->extcon);
+	}
+
 	glue->phy = devm_phy_get(&pdev->dev, "usb");
 	if (IS_ERR(glue->phy)) {
 		if (PTR_ERR(glue->phy) == -EPROBE_DEFER)
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 4/4] musb: sunxi: Simplify dr_mode handling
@ 2016-06-05 14:59     ` Hans de Goede
  0 siblings, 0 replies; 28+ messages in thread
From: Hans de Goede @ 2016-06-05 14:59 UTC (permalink / raw)
  To: linux-arm-kernel

phy-sun4i-usb now has proper dr_mode handling, it always registers an
extcon, and sends a notify with the mode (even when in peripheral- /
host-only mode) at least once.

So we can simply the sunxi musb glue by always registering its extcon
notifier and relying on sunxi_musb_work() to enable vbus when in
host-only mode.

This also enables host- and peripheral-only mode with vbus monitoring.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-No changes
Changes in v3:
-No changes
---
 drivers/usb/musb/sunxi.c | 68 ++++++++++++++++++------------------------------
 1 file changed, 25 insertions(+), 43 deletions(-)

diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
index e7d4617..b88a2f6 100644
--- a/drivers/usb/musb/sunxi.c
+++ b/drivers/usb/musb/sunxi.c
@@ -255,12 +255,10 @@ static int sunxi_musb_init(struct musb *musb)
 	writeb(SUNXI_MUSB_VEND0_PIO_MODE, musb->mregs + SUNXI_MUSB_VEND0);
 
 	/* Register notifier before calling phy_init() */
-	if (musb->port_mode == MUSB_PORT_MODE_DUAL_ROLE) {
-		ret = extcon_register_notifier(glue->extcon, EXTCON_USB_HOST,
-					       &glue->host_nb);
-		if (ret)
-			goto error_reset_assert;
-	}
+	ret = extcon_register_notifier(glue->extcon, EXTCON_USB_HOST,
+				       &glue->host_nb);
+	if (ret)
+		goto error_reset_assert;
 
 	ret = phy_init(glue->phy);
 	if (ret)
@@ -274,9 +272,8 @@ static int sunxi_musb_init(struct musb *musb)
 	return 0;
 
 error_unregister_notifier:
-	if (musb->port_mode == MUSB_PORT_MODE_DUAL_ROLE)
-		extcon_unregister_notifier(glue->extcon, EXTCON_USB_HOST,
-					   &glue->host_nb);
+	extcon_unregister_notifier(glue->extcon, EXTCON_USB_HOST,
+				   &glue->host_nb);
 error_reset_assert:
 	if (test_bit(SUNXI_MUSB_FL_HAS_RESET, &glue->flags))
 		reset_control_assert(glue->rst);
@@ -300,9 +297,8 @@ static int sunxi_musb_exit(struct musb *musb)
 
 	phy_exit(glue->phy);
 
-	if (musb->port_mode == MUSB_PORT_MODE_DUAL_ROLE)
-		extcon_unregister_notifier(glue->extcon, EXTCON_USB_HOST,
-					   &glue->host_nb);
+	extcon_unregister_notifier(glue->extcon, EXTCON_USB_HOST,
+				   &glue->host_nb);
 
 	if (test_bit(SUNXI_MUSB_FL_HAS_RESET, &glue->flags))
 		reset_control_assert(glue->rst);
@@ -314,25 +310,6 @@ static int sunxi_musb_exit(struct musb *musb)
 	return 0;
 }
 
-static int sunxi_set_mode(struct musb *musb, u8 mode)
-{
-	struct sunxi_glue *glue = dev_get_drvdata(musb->controller->parent);
-	int ret;
-
-	if (mode == MUSB_HOST) {
-		ret = phy_power_on(glue->phy);
-		if (ret)
-			return ret;
-
-		set_bit(SUNXI_MUSB_FL_PHY_ON, &glue->flags);
-		/* Stop musb work from turning vbus off again */
-		set_bit(SUNXI_MUSB_FL_VBUS_ON, &glue->flags);
-		musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE;
-	}
-
-	return 0;
-}
-
 static void sunxi_musb_enable(struct musb *musb)
 {
 	struct sunxi_glue *glue = dev_get_drvdata(musb->controller->parent);
@@ -579,7 +556,6 @@ static const struct musb_platform_ops sunxi_musb_ops = {
 	.exit		= sunxi_musb_exit,
 	.enable		= sunxi_musb_enable,
 	.disable	= sunxi_musb_disable,
-	.set_mode	= sunxi_set_mode,
 	.fifo_offset	= sunxi_musb_fifo_offset,
 	.ep_offset	= sunxi_musb_ep_offset,
 	.busctl_offset	= sunxi_musb_busctl_offset,
@@ -635,10 +611,6 @@ static int sunxi_musb_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	glue = devm_kzalloc(&pdev->dev, sizeof(*glue), GFP_KERNEL);
-	if (!glue)
-		return -ENOMEM;
-
 	memset(&pdata, 0, sizeof(pdata));
 	switch (usb_get_dr_mode(&pdev->dev)) {
 #if defined CONFIG_USB_MUSB_DUAL_ROLE || defined CONFIG_USB_MUSB_HOST
@@ -646,15 +618,13 @@ static int sunxi_musb_probe(struct platform_device *pdev)
 		pdata.mode = MUSB_PORT_MODE_HOST;
 		break;
 #endif
+#if defined CONFIG_USB_MUSB_DUAL_ROLE || defined CONFIG_USB_MUSB_GADGET
+	case USB_DR_MODE_PERIPHERAL:
+		pdata.mode = MUSB_PORT_MODE_GADGET;
+		break;
+#endif
 #ifdef CONFIG_USB_MUSB_DUAL_ROLE
 	case USB_DR_MODE_OTG:
-		glue->extcon = extcon_get_edev_by_phandle(&pdev->dev, 0);
-		if (IS_ERR(glue->extcon)) {
-			if (PTR_ERR(glue->extcon) == -EPROBE_DEFER)
-				return -EPROBE_DEFER;
-			dev_err(&pdev->dev, "Invalid or missing extcon\n");
-			return PTR_ERR(glue->extcon);
-		}
 		pdata.mode = MUSB_PORT_MODE_DUAL_ROLE;
 		break;
 #endif
@@ -665,6 +635,10 @@ static int sunxi_musb_probe(struct platform_device *pdev)
 	pdata.platform_ops	= &sunxi_musb_ops;
 	pdata.config		= &sunxi_musb_hdrc_config;
 
+	glue = devm_kzalloc(&pdev->dev, sizeof(*glue), GFP_KERNEL);
+	if (!glue)
+		return -ENOMEM;
+
 	glue->dev = &pdev->dev;
 	INIT_WORK(&glue->work, sunxi_musb_work);
 	glue->host_nb.notifier_call = sunxi_musb_host_notifier;
@@ -698,6 +672,14 @@ static int sunxi_musb_probe(struct platform_device *pdev)
 		}
 	}
 
+	glue->extcon = extcon_get_edev_by_phandle(&pdev->dev, 0);
+	if (IS_ERR(glue->extcon)) {
+		if (PTR_ERR(glue->extcon) == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
+		dev_err(&pdev->dev, "Invalid or missing extcon\n");
+		return PTR_ERR(glue->extcon);
+	}
+
 	glue->phy = devm_phy_get(&pdev->dev, "usb");
 	if (IS_ERR(glue->phy)) {
 		if (PTR_ERR(glue->phy) == -EPROBE_DEFER)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 4/4] musb: sunxi: Simplify dr_mode handling
  2016-06-05 14:59     ` Hans de Goede
@ 2016-06-08 10:23         ` Maxime Ripard
  -1 siblings, 0 replies; 28+ messages in thread
From: Maxime Ripard @ 2016-06-08 10:23 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Bin Liu, Greg Kroah-Hartman, Kishon Vijay Abraham I,
	Chen-Yu Tsai, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree

[-- Attachment #1: Type: text/plain, Size: 942 bytes --]

Hi,

On Sun, Jun 05, 2016 at 04:59:36PM +0200, Hans de Goede wrote:
> phy-sun4i-usb now has proper dr_mode handling, it always registers an
> extcon, and sends a notify with the mode (even when in peripheral- /
> host-only mode) at least once.
> 
> So we can simply the sunxi musb glue by always registering its extcon
> notifier and relying on sunxi_musb_work() to enable vbus when in
> host-only mode.
> 
> This also enables host- and peripheral-only mode with vbus monitoring.
> 
> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

It's been a bit painful to track all the patches needed so that it
applies properly, but I've finally been able to test it on a Sinlinx
SinA33 with peripheral-only mUSB, and it works like a charm.

You can add my Tested-by.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v3 4/4] musb: sunxi: Simplify dr_mode handling
@ 2016-06-08 10:23         ` Maxime Ripard
  0 siblings, 0 replies; 28+ messages in thread
From: Maxime Ripard @ 2016-06-08 10:23 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Sun, Jun 05, 2016 at 04:59:36PM +0200, Hans de Goede wrote:
> phy-sun4i-usb now has proper dr_mode handling, it always registers an
> extcon, and sends a notify with the mode (even when in peripheral- /
> host-only mode) at least once.
> 
> So we can simply the sunxi musb glue by always registering its extcon
> notifier and relying on sunxi_musb_work() to enable vbus when in
> host-only mode.
> 
> This also enables host- and peripheral-only mode with vbus monitoring.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

It's been a bit painful to track all the patches needed so that it
applies properly, but I've finally been able to test it on a Sinlinx
SinA33 with peripheral-only mUSB, and it works like a charm.

You can add my Tested-by.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160608/39078255/attachment.sig>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 4/4] musb: sunxi: Simplify dr_mode handling
  2016-06-08 10:23         ` Maxime Ripard
@ 2016-06-08 10:30           ` Hans de Goede
  -1 siblings, 0 replies; 28+ messages in thread
From: Hans de Goede @ 2016-06-08 10:30 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Bin Liu, Greg Kroah-Hartman, Kishon Vijay Abraham I,
	Chen-Yu Tsai, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree

Hi,

On 08-06-16 12:23, Maxime Ripard wrote:
> Hi,
>
> On Sun, Jun 05, 2016 at 04:59:36PM +0200, Hans de Goede wrote:
>> phy-sun4i-usb now has proper dr_mode handling, it always registers an
>> extcon, and sends a notify with the mode (even when in peripheral- /
>> host-only mode) at least once.
>>
>> So we can simply the sunxi musb glue by always registering its extcon
>> notifier and relying on sunxi_musb_work() to enable vbus when in
>> host-only mode.
>>
>> This also enables host- and peripheral-only mode with vbus monitoring.
>>
>> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>
> It's been a bit painful to track all the patches needed so that it
> applies properly, but I've finally been able to test it on a Sinlinx
> SinA33 with peripheral-only mUSB, and it works like a charm.
>
> You can add my Tested-by.

Great, thanks for testing.

This is the board which has an otg connector with vbus not connected,
right? Yet it does have a functional id-pin, right ?

In that case you should be able to put it in dual-role mode (only
specify the id-pin in the phy dts node, no vbus / vbus-monitoring)
and then it _should_ work in host mode if you use a powered hub.

I'm fine with putting in peripheral-only mode, but as said
dual-role might work with a powered hub.

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v3 4/4] musb: sunxi: Simplify dr_mode handling
@ 2016-06-08 10:30           ` Hans de Goede
  0 siblings, 0 replies; 28+ messages in thread
From: Hans de Goede @ 2016-06-08 10:30 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 08-06-16 12:23, Maxime Ripard wrote:
> Hi,
>
> On Sun, Jun 05, 2016 at 04:59:36PM +0200, Hans de Goede wrote:
>> phy-sun4i-usb now has proper dr_mode handling, it always registers an
>> extcon, and sends a notify with the mode (even when in peripheral- /
>> host-only mode) at least once.
>>
>> So we can simply the sunxi musb glue by always registering its extcon
>> notifier and relying on sunxi_musb_work() to enable vbus when in
>> host-only mode.
>>
>> This also enables host- and peripheral-only mode with vbus monitoring.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>
> It's been a bit painful to track all the patches needed so that it
> applies properly, but I've finally been able to test it on a Sinlinx
> SinA33 with peripheral-only mUSB, and it works like a charm.
>
> You can add my Tested-by.

Great, thanks for testing.

This is the board which has an otg connector with vbus not connected,
right? Yet it does have a functional id-pin, right ?

In that case you should be able to put it in dual-role mode (only
specify the id-pin in the phy dts node, no vbus / vbus-monitoring)
and then it _should_ work in host mode if you use a powered hub.

I'm fine with putting in peripheral-only mode, but as said
dual-role might work with a powered hub.

Regards,

Hans

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 1/4] USB: Fix of_usb_get_dr_mode_by_phy with a shared phy block
  2016-06-05 14:59 ` Hans de Goede
@ 2016-06-09 14:30     ` Bin Liu
  -1 siblings, 0 replies; 28+ messages in thread
From: Bin Liu @ 2016-06-09 14:30 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Greg Kroah-Hartman, Kishon Vijay Abraham I, Maxime Ripard,
	Chen-Yu Tsai, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree

Hi,

On Sun, Jun 05, 2016 at 04:59:33PM +0200, Hans de Goede wrote:
> Some SoCs have a single phy-hw-block with multiple phys, this is
> modelled by a single phy dts node, so we end up with multiple
> controller nodes with a phys property pointing to the phy-node
> of the otg-phy.
> 
> Only one of these controllers typically is an otg controller, yet we
> were checking the first controller who uses a phy from the block and
> then end up looking for a dr_mode property in e.g. the ehci controller.
> 
> This commit fixes this by adding an arg0 parameter to
> of_usb_get_dr_mode_by_phy and make of_usb_get_dr_mode_by_phy
> check that this matches the phandle args[0] value when looking for
> the otg controller.
> 
> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> Changes in v2:
> -Add a args0 parameter instead of looking for nodes with a dr_mode property
> Changes in v3:
> -No changes
> ---
>  drivers/usb/common/common.c  | 31 ++++++++++++++++++-------------
>  drivers/usb/phy/phy-am335x.c |  2 +-
>  include/linux/usb/of.h       |  4 ++--
>  3 files changed, 21 insertions(+), 16 deletions(-)

This breaks am335x.

[   17.433166] /ocp/usb@47400000/usb@47401000: could not get #phy-cells
for /ocp/usb@47400000/usb-phy@47401300
[   17.443627] /ocp/usb@47400000/usb@47401800: could not get #phy-cells
for /ocp/usb@47400000/usb-phy@47401b00
[   17.454005] am335x-phy-driver 47401300.usb-phy: dr_mode 0
[   17.460518] 47401300.usb-phy supply vcc not found, using dummy
regulator
[   17.469685] /ocp/usb@47400000/usb@47401000: could not get #phy-cells
for /ocp/usb@47400000/usb-phy@47401300
[   17.479998] /ocp/usb@47400000/usb@47401800: could not get #phy-cells
for /ocp/usb@47400000/usb-phy@47401b00
[   17.490342] am335x-phy-driver 47401b00.usb-phy: dr_mode 0

Regards,
-Bin.

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v3 1/4] USB: Fix of_usb_get_dr_mode_by_phy with a shared phy block
@ 2016-06-09 14:30     ` Bin Liu
  0 siblings, 0 replies; 28+ messages in thread
From: Bin Liu @ 2016-06-09 14:30 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Sun, Jun 05, 2016 at 04:59:33PM +0200, Hans de Goede wrote:
> Some SoCs have a single phy-hw-block with multiple phys, this is
> modelled by a single phy dts node, so we end up with multiple
> controller nodes with a phys property pointing to the phy-node
> of the otg-phy.
> 
> Only one of these controllers typically is an otg controller, yet we
> were checking the first controller who uses a phy from the block and
> then end up looking for a dr_mode property in e.g. the ehci controller.
> 
> This commit fixes this by adding an arg0 parameter to
> of_usb_get_dr_mode_by_phy and make of_usb_get_dr_mode_by_phy
> check that this matches the phandle args[0] value when looking for
> the otg controller.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -Add a args0 parameter instead of looking for nodes with a dr_mode property
> Changes in v3:
> -No changes
> ---
>  drivers/usb/common/common.c  | 31 ++++++++++++++++++-------------
>  drivers/usb/phy/phy-am335x.c |  2 +-
>  include/linux/usb/of.h       |  4 ++--
>  3 files changed, 21 insertions(+), 16 deletions(-)

This breaks am335x.

[   17.433166] /ocp/usb at 47400000/usb at 47401000: could not get #phy-cells
for /ocp/usb at 47400000/usb-phy at 47401300
[   17.443627] /ocp/usb at 47400000/usb at 47401800: could not get #phy-cells
for /ocp/usb at 47400000/usb-phy at 47401b00
[   17.454005] am335x-phy-driver 47401300.usb-phy: dr_mode 0
[   17.460518] 47401300.usb-phy supply vcc not found, using dummy
regulator
[   17.469685] /ocp/usb at 47400000/usb at 47401000: could not get #phy-cells
for /ocp/usb at 47400000/usb-phy at 47401300
[   17.479998] /ocp/usb at 47400000/usb at 47401800: could not get #phy-cells
for /ocp/usb at 47400000/usb-phy at 47401b00
[   17.490342] am335x-phy-driver 47401b00.usb-phy: dr_mode 0

Regards,
-Bin.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 1/4] USB: Fix of_usb_get_dr_mode_by_phy with a shared phy block
  2016-06-09 14:30     ` Bin Liu
@ 2016-06-09 14:51       ` Hans de Goede
  -1 siblings, 0 replies; 28+ messages in thread
From: Hans de Goede @ 2016-06-09 14:51 UTC (permalink / raw)
  To: Bin Liu, Greg Kroah-Hartman, Kishon Vijay Abraham I,
	Maxime Ripard, Chen-Yu Tsai, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree

Hi,

On 09-06-16 16:30, Bin Liu wrote:
> Hi,
>
> On Sun, Jun 05, 2016 at 04:59:33PM +0200, Hans de Goede wrote:
>> Some SoCs have a single phy-hw-block with multiple phys, this is
>> modelled by a single phy dts node, so we end up with multiple
>> controller nodes with a phys property pointing to the phy-node
>> of the otg-phy.
>>
>> Only one of these controllers typically is an otg controller, yet we
>> were checking the first controller who uses a phy from the block and
>> then end up looking for a dr_mode property in e.g. the ehci controller.
>>
>> This commit fixes this by adding an arg0 parameter to
>> of_usb_get_dr_mode_by_phy and make of_usb_get_dr_mode_by_phy
>> check that this matches the phandle args[0] value when looking for
>> the otg controller.
>>
>> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> ---
>> Changes in v2:
>> -Add a args0 parameter instead of looking for nodes with a dr_mode property
>> Changes in v3:
>> -No changes
>> ---
>>  drivers/usb/common/common.c  | 31 ++++++++++++++++++-------------
>>  drivers/usb/phy/phy-am335x.c |  2 +-
>>  include/linux/usb/of.h       |  4 ++--
>>  3 files changed, 21 insertions(+), 16 deletions(-)
>
> This breaks am335x.
>
> [   17.433166] /ocp/usb@47400000/usb@47401000: could not get #phy-cells
> for /ocp/usb@47400000/usb-phy@47401300
> [   17.443627] /ocp/usb@47400000/usb@47401800: could not get #phy-cells
> for /ocp/usb@47400000/usb-phy@47401b00
> [   17.454005] am335x-phy-driver 47401300.usb-phy: dr_mode 0
> [   17.460518] 47401300.usb-phy supply vcc not found, using dummy
> regulator
> [   17.469685] /ocp/usb@47400000/usb@47401000: could not get #phy-cells
> for /ocp/usb@47400000/usb-phy@47401300
> [   17.479998] /ocp/usb@47400000/usb@47401800: could not get #phy-cells
> for /ocp/usb@47400000/usb-phy@47401b00
> [   17.490342] am335x-phy-driver 47401b00.usb-phy: dr_mode 0

That is weird, quoting: drivers/phy/phy-core.c: _of_phy_get():

         ret = of_parse_phandle_with_args(np, "phys", "#phy-cells",
                 index, &args);
         if (ret)
                 return ERR_PTR(-ENODEV);

So if #phy-cells is not defined, then the phy core should not
be able to work with the dts files in question at all.

All my patch does is make the way of_usb_get_dr_mode_by_phy parses
phy-handles be identical to how phy-core.c does it.

I see that the usb0_phy node in arch/arm/boot/dts/am33xx.dtsi
indeed lacks a "#phy-cells = <0>;" line.

Quoting: Documentation/devicetree/bindings/phy/phy-bindings.txt

PHY device node
===============

Required Properties:
#phy-cells:     Number of cells in a PHY specifier;  The meaning of all those
                 cells is defined by the binding for the phy node. The PHY
                 provider can use the values in cells to find the appropriate
                 PHY.

So not having #phy-cells defined for a phy-node clearly is a bug.

A patch like the following should fix this:

diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index 52be48b..c4ff788d 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -557,6 +557,7 @@
  			};

  			usb0_phy: usb-phy@47401300 {
+				#phy-cells = <0>;
  				compatible = "ti,am335x-usb-phy";
  				reg = <0x47401300 0x100>;
  				reg-names = "phy";

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 1/4] USB: Fix of_usb_get_dr_mode_by_phy with a shared phy block
@ 2016-06-09 14:51       ` Hans de Goede
  0 siblings, 0 replies; 28+ messages in thread
From: Hans de Goede @ 2016-06-09 14:51 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 09-06-16 16:30, Bin Liu wrote:
> Hi,
>
> On Sun, Jun 05, 2016 at 04:59:33PM +0200, Hans de Goede wrote:
>> Some SoCs have a single phy-hw-block with multiple phys, this is
>> modelled by a single phy dts node, so we end up with multiple
>> controller nodes with a phys property pointing to the phy-node
>> of the otg-phy.
>>
>> Only one of these controllers typically is an otg controller, yet we
>> were checking the first controller who uses a phy from the block and
>> then end up looking for a dr_mode property in e.g. the ehci controller.
>>
>> This commit fixes this by adding an arg0 parameter to
>> of_usb_get_dr_mode_by_phy and make of_usb_get_dr_mode_by_phy
>> check that this matches the phandle args[0] value when looking for
>> the otg controller.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Changes in v2:
>> -Add a args0 parameter instead of looking for nodes with a dr_mode property
>> Changes in v3:
>> -No changes
>> ---
>>  drivers/usb/common/common.c  | 31 ++++++++++++++++++-------------
>>  drivers/usb/phy/phy-am335x.c |  2 +-
>>  include/linux/usb/of.h       |  4 ++--
>>  3 files changed, 21 insertions(+), 16 deletions(-)
>
> This breaks am335x.
>
> [   17.433166] /ocp/usb at 47400000/usb at 47401000: could not get #phy-cells
> for /ocp/usb at 47400000/usb-phy at 47401300
> [   17.443627] /ocp/usb at 47400000/usb at 47401800: could not get #phy-cells
> for /ocp/usb at 47400000/usb-phy at 47401b00
> [   17.454005] am335x-phy-driver 47401300.usb-phy: dr_mode 0
> [   17.460518] 47401300.usb-phy supply vcc not found, using dummy
> regulator
> [   17.469685] /ocp/usb at 47400000/usb at 47401000: could not get #phy-cells
> for /ocp/usb at 47400000/usb-phy at 47401300
> [   17.479998] /ocp/usb at 47400000/usb at 47401800: could not get #phy-cells
> for /ocp/usb at 47400000/usb-phy at 47401b00
> [   17.490342] am335x-phy-driver 47401b00.usb-phy: dr_mode 0

That is weird, quoting: drivers/phy/phy-core.c: _of_phy_get():

         ret = of_parse_phandle_with_args(np, "phys", "#phy-cells",
                 index, &args);
         if (ret)
                 return ERR_PTR(-ENODEV);

So if #phy-cells is not defined, then the phy core should not
be able to work with the dts files in question at all.

All my patch does is make the way of_usb_get_dr_mode_by_phy parses
phy-handles be identical to how phy-core.c does it.

I see that the usb0_phy node in arch/arm/boot/dts/am33xx.dtsi
indeed lacks a "#phy-cells = <0>;" line.

Quoting: Documentation/devicetree/bindings/phy/phy-bindings.txt

PHY device node
===============

Required Properties:
#phy-cells:     Number of cells in a PHY specifier;  The meaning of all those
                 cells is defined by the binding for the phy node. The PHY
                 provider can use the values in cells to find the appropriate
                 PHY.

So not having #phy-cells defined for a phy-node clearly is a bug.

A patch like the following should fix this:

diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index 52be48b..c4ff788d 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -557,6 +557,7 @@
  			};

  			usb0_phy: usb-phy at 47401300 {
+				#phy-cells = <0>;
  				compatible = "ti,am335x-usb-phy";
  				reg = <0x47401300 0x100>;
  				reg-names = "phy";

Regards,

Hans

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 1/4] USB: Fix of_usb_get_dr_mode_by_phy with a shared phy block
  2016-06-09 14:51       ` Hans de Goede
@ 2016-06-09 19:49           ` Bin Liu
  -1 siblings, 0 replies; 28+ messages in thread
From: Bin Liu @ 2016-06-09 19:49 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Greg Kroah-Hartman, Kishon Vijay Abraham I, Maxime Ripard,
	Chen-Yu Tsai, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree

Hi,

On Thu, Jun 09, 2016 at 04:51:45PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 09-06-16 16:30, Bin Liu wrote:
> >Hi,
> >
> >On Sun, Jun 05, 2016 at 04:59:33PM +0200, Hans de Goede wrote:
> >>Some SoCs have a single phy-hw-block with multiple phys, this is
> >>modelled by a single phy dts node, so we end up with multiple
> >>controller nodes with a phys property pointing to the phy-node
> >>of the otg-phy.
> >>
> >>Only one of these controllers typically is an otg controller, yet we
> >>were checking the first controller who uses a phy from the block and
> >>then end up looking for a dr_mode property in e.g. the ehci controller.
> >>
> >>This commit fixes this by adding an arg0 parameter to
> >>of_usb_get_dr_mode_by_phy and make of_usb_get_dr_mode_by_phy
> >>check that this matches the phandle args[0] value when looking for
> >>the otg controller.
> >>
> >>Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> >>---
> >>Changes in v2:
> >>-Add a args0 parameter instead of looking for nodes with a dr_mode property
> >>Changes in v3:
> >>-No changes
> >>---
> >> drivers/usb/common/common.c  | 31 ++++++++++++++++++-------------
> >> drivers/usb/phy/phy-am335x.c |  2 +-
> >> include/linux/usb/of.h       |  4 ++--
> >> 3 files changed, 21 insertions(+), 16 deletions(-)
> >
> >This breaks am335x.
> >
> >[   17.433166] /ocp/usb@47400000/usb@47401000: could not get #phy-cells
> >for /ocp/usb@47400000/usb-phy@47401300
> >[   17.443627] /ocp/usb@47400000/usb@47401800: could not get #phy-cells
> >for /ocp/usb@47400000/usb-phy@47401b00
> >[   17.454005] am335x-phy-driver 47401300.usb-phy: dr_mode 0
> >[   17.460518] 47401300.usb-phy supply vcc not found, using dummy
> >regulator
> >[   17.469685] /ocp/usb@47400000/usb@47401000: could not get #phy-cells
> >for /ocp/usb@47400000/usb-phy@47401300
> >[   17.479998] /ocp/usb@47400000/usb@47401800: could not get #phy-cells
> >for /ocp/usb@47400000/usb-phy@47401b00
> >[   17.490342] am335x-phy-driver 47401b00.usb-phy: dr_mode 0
> 
> That is weird, quoting: drivers/phy/phy-core.c: _of_phy_get():
> 
>         ret = of_parse_phandle_with_args(np, "phys", "#phy-cells",
>                 index, &args);
>         if (ret)
>                 return ERR_PTR(-ENODEV);
> 
> So if #phy-cells is not defined, then the phy core should not
> be able to work with the dts files in question at all.

am335x phy does not use phy framework, so those phy core api is not
called.

> 
> All my patch does is make the way of_usb_get_dr_mode_by_phy parses
> phy-handles be identical to how phy-core.c does it.
> 
> I see that the usb0_phy node in arch/arm/boot/dts/am33xx.dtsi
> indeed lacks a "#phy-cells = <0>;" line.
> 
> Quoting: Documentation/devicetree/bindings/phy/phy-bindings.txt
> 
> PHY device node
> ===============
> 
> Required Properties:
> #phy-cells:     Number of cells in a PHY specifier;  The meaning of all those
>                 cells is defined by the binding for the phy node. The PHY
>                 provider can use the values in cells to find the appropriate
>                 PHY.
> 
> So not having #phy-cells defined for a phy-node clearly is a bug.

am335x phy does not follow the bindings in phy/, but usb/phy/.

> 
> A patch like the following should fix this:
> 
> diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> index 52be48b..c4ff788d 100644
> --- a/arch/arm/boot/dts/am33xx.dtsi
> +++ b/arch/arm/boot/dts/am33xx.dtsi
> @@ -557,6 +557,7 @@
>  			};
> 
>  			usb0_phy: usb-phy@47401300 {
> +				#phy-cells = <0>;

This seems to be ok, but this patch would break the dt backward compatible.

>  				compatible = "ti,am335x-usb-phy";
>  				reg = <0x47401300 0x100>;
>  				reg-names = "phy";
> 
> Regards,
> 
> Hans

Regards,
-Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v3 1/4] USB: Fix of_usb_get_dr_mode_by_phy with a shared phy block
@ 2016-06-09 19:49           ` Bin Liu
  0 siblings, 0 replies; 28+ messages in thread
From: Bin Liu @ 2016-06-09 19:49 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Thu, Jun 09, 2016 at 04:51:45PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 09-06-16 16:30, Bin Liu wrote:
> >Hi,
> >
> >On Sun, Jun 05, 2016 at 04:59:33PM +0200, Hans de Goede wrote:
> >>Some SoCs have a single phy-hw-block with multiple phys, this is
> >>modelled by a single phy dts node, so we end up with multiple
> >>controller nodes with a phys property pointing to the phy-node
> >>of the otg-phy.
> >>
> >>Only one of these controllers typically is an otg controller, yet we
> >>were checking the first controller who uses a phy from the block and
> >>then end up looking for a dr_mode property in e.g. the ehci controller.
> >>
> >>This commit fixes this by adding an arg0 parameter to
> >>of_usb_get_dr_mode_by_phy and make of_usb_get_dr_mode_by_phy
> >>check that this matches the phandle args[0] value when looking for
> >>the otg controller.
> >>
> >>Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >>---
> >>Changes in v2:
> >>-Add a args0 parameter instead of looking for nodes with a dr_mode property
> >>Changes in v3:
> >>-No changes
> >>---
> >> drivers/usb/common/common.c  | 31 ++++++++++++++++++-------------
> >> drivers/usb/phy/phy-am335x.c |  2 +-
> >> include/linux/usb/of.h       |  4 ++--
> >> 3 files changed, 21 insertions(+), 16 deletions(-)
> >
> >This breaks am335x.
> >
> >[   17.433166] /ocp/usb at 47400000/usb at 47401000: could not get #phy-cells
> >for /ocp/usb at 47400000/usb-phy at 47401300
> >[   17.443627] /ocp/usb at 47400000/usb at 47401800: could not get #phy-cells
> >for /ocp/usb at 47400000/usb-phy at 47401b00
> >[   17.454005] am335x-phy-driver 47401300.usb-phy: dr_mode 0
> >[   17.460518] 47401300.usb-phy supply vcc not found, using dummy
> >regulator
> >[   17.469685] /ocp/usb at 47400000/usb at 47401000: could not get #phy-cells
> >for /ocp/usb at 47400000/usb-phy at 47401300
> >[   17.479998] /ocp/usb at 47400000/usb at 47401800: could not get #phy-cells
> >for /ocp/usb at 47400000/usb-phy at 47401b00
> >[   17.490342] am335x-phy-driver 47401b00.usb-phy: dr_mode 0
> 
> That is weird, quoting: drivers/phy/phy-core.c: _of_phy_get():
> 
>         ret = of_parse_phandle_with_args(np, "phys", "#phy-cells",
>                 index, &args);
>         if (ret)
>                 return ERR_PTR(-ENODEV);
> 
> So if #phy-cells is not defined, then the phy core should not
> be able to work with the dts files in question at all.

am335x phy does not use phy framework, so those phy core api is not
called.

> 
> All my patch does is make the way of_usb_get_dr_mode_by_phy parses
> phy-handles be identical to how phy-core.c does it.
> 
> I see that the usb0_phy node in arch/arm/boot/dts/am33xx.dtsi
> indeed lacks a "#phy-cells = <0>;" line.
> 
> Quoting: Documentation/devicetree/bindings/phy/phy-bindings.txt
> 
> PHY device node
> ===============
> 
> Required Properties:
> #phy-cells:     Number of cells in a PHY specifier;  The meaning of all those
>                 cells is defined by the binding for the phy node. The PHY
>                 provider can use the values in cells to find the appropriate
>                 PHY.
> 
> So not having #phy-cells defined for a phy-node clearly is a bug.

am335x phy does not follow the bindings in phy/, but usb/phy/.

> 
> A patch like the following should fix this:
> 
> diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> index 52be48b..c4ff788d 100644
> --- a/arch/arm/boot/dts/am33xx.dtsi
> +++ b/arch/arm/boot/dts/am33xx.dtsi
> @@ -557,6 +557,7 @@
>  			};
> 
>  			usb0_phy: usb-phy at 47401300 {
> +				#phy-cells = <0>;

This seems to be ok, but this patch would break the dt backward compatible.

>  				compatible = "ti,am335x-usb-phy";
>  				reg = <0x47401300 0x100>;
>  				reg-names = "phy";
> 
> Regards,
> 
> Hans

Regards,
-Bin.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 1/4] USB: Fix of_usb_get_dr_mode_by_phy with a shared phy block
  2016-06-09 19:49           ` Bin Liu
@ 2016-06-10  9:27             ` Hans de Goede
  -1 siblings, 0 replies; 28+ messages in thread
From: Hans de Goede @ 2016-06-10  9:27 UTC (permalink / raw)
  To: Bin Liu, Greg Kroah-Hartman, Kishon Vijay Abraham I,
	Maxime Ripard, Chen-Yu Tsai, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree

Hi,

On 09-06-16 21:49, Bin Liu wrote:
> Hi,
>
> On Thu, Jun 09, 2016 at 04:51:45PM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 09-06-16 16:30, Bin Liu wrote:
>>> Hi,
>>>
>>> On Sun, Jun 05, 2016 at 04:59:33PM +0200, Hans de Goede wrote:
>>>> Some SoCs have a single phy-hw-block with multiple phys, this is
>>>> modelled by a single phy dts node, so we end up with multiple
>>>> controller nodes with a phys property pointing to the phy-node
>>>> of the otg-phy.
>>>>
>>>> Only one of these controllers typically is an otg controller, yet we
>>>> were checking the first controller who uses a phy from the block and
>>>> then end up looking for a dr_mode property in e.g. the ehci controller.
>>>>
>>>> This commit fixes this by adding an arg0 parameter to
>>>> of_usb_get_dr_mode_by_phy and make of_usb_get_dr_mode_by_phy
>>>> check that this matches the phandle args[0] value when looking for
>>>> the otg controller.
>>>>
>>>> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>>> ---
>>>> Changes in v2:
>>>> -Add a args0 parameter instead of looking for nodes with a dr_mode property
>>>> Changes in v3:
>>>> -No changes
>>>> ---
>>>> drivers/usb/common/common.c  | 31 ++++++++++++++++++-------------
>>>> drivers/usb/phy/phy-am335x.c |  2 +-
>>>> include/linux/usb/of.h       |  4 ++--
>>>> 3 files changed, 21 insertions(+), 16 deletions(-)
>>>
>>> This breaks am335x.
>>>
>>> [   17.433166] /ocp/usb@47400000/usb@47401000: could not get #phy-cells
>>> for /ocp/usb@47400000/usb-phy@47401300
>>> [   17.443627] /ocp/usb@47400000/usb@47401800: could not get #phy-cells
>>> for /ocp/usb@47400000/usb-phy@47401b00
>>> [   17.454005] am335x-phy-driver 47401300.usb-phy: dr_mode 0
>>> [   17.460518] 47401300.usb-phy supply vcc not found, using dummy
>>> regulator
>>> [   17.469685] /ocp/usb@47400000/usb@47401000: could not get #phy-cells
>>> for /ocp/usb@47400000/usb-phy@47401300
>>> [   17.479998] /ocp/usb@47400000/usb@47401800: could not get #phy-cells
>>> for /ocp/usb@47400000/usb-phy@47401b00
>>> [   17.490342] am335x-phy-driver 47401b00.usb-phy: dr_mode 0
>>
>> That is weird, quoting: drivers/phy/phy-core.c: _of_phy_get():
>>
>>         ret = of_parse_phandle_with_args(np, "phys", "#phy-cells",
>>                 index, &args);
>>         if (ret)
>>                 return ERR_PTR(-ENODEV);
>>
>> So if #phy-cells is not defined, then the phy core should not
>> be able to work with the dts files in question at all.
>
> am335x phy does not use phy framework, so those phy core api is not
> called.
>
>>
>> All my patch does is make the way of_usb_get_dr_mode_by_phy parses
>> phy-handles be identical to how phy-core.c does it.
>>
>> I see that the usb0_phy node in arch/arm/boot/dts/am33xx.dtsi
>> indeed lacks a "#phy-cells = <0>;" line.
>>
>> Quoting: Documentation/devicetree/bindings/phy/phy-bindings.txt
>>
>> PHY device node
>> ===============
>>
>> Required Properties:
>> #phy-cells:     Number of cells in a PHY specifier;  The meaning of all those
>>                 cells is defined by the binding for the phy node. The PHY
>>                 provider can use the values in cells to find the appropriate
>>                 PHY.
>>
>> So not having #phy-cells defined for a phy-node clearly is a bug.
>
> am335x phy does not follow the bindings in phy/, but usb/phy/.

Ah, I did not know about those, that explains. I will send a v4 of
the patch which will keep compatibility with the usb/phy/ bindings
when the passed in arg0 == -1.

Note I will not be resending patch 2-4, please merge these as they
are (no changes are needed).

>>
>> A patch like the following should fix this:
>>
>> diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
>> index 52be48b..c4ff788d 100644
>> --- a/arch/arm/boot/dts/am33xx.dtsi
>> +++ b/arch/arm/boot/dts/am33xx.dtsi
>> @@ -557,6 +557,7 @@
>>  			};
>>
>>  			usb0_phy: usb-phy@47401300 {
>> +				#phy-cells = <0>;
>
> This seems to be ok, but this patch would break the dt backward compatible.

I understand, as said above I'll do a new version of the patch preserving
compatibility.

>>  				compatible = "ti,am335x-usb-phy";
>>  				reg = <0x47401300 0x100>;
>>  				reg-names = "phy";
>>

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v3 1/4] USB: Fix of_usb_get_dr_mode_by_phy with a shared phy block
@ 2016-06-10  9:27             ` Hans de Goede
  0 siblings, 0 replies; 28+ messages in thread
From: Hans de Goede @ 2016-06-10  9:27 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 09-06-16 21:49, Bin Liu wrote:
> Hi,
>
> On Thu, Jun 09, 2016 at 04:51:45PM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 09-06-16 16:30, Bin Liu wrote:
>>> Hi,
>>>
>>> On Sun, Jun 05, 2016 at 04:59:33PM +0200, Hans de Goede wrote:
>>>> Some SoCs have a single phy-hw-block with multiple phys, this is
>>>> modelled by a single phy dts node, so we end up with multiple
>>>> controller nodes with a phys property pointing to the phy-node
>>>> of the otg-phy.
>>>>
>>>> Only one of these controllers typically is an otg controller, yet we
>>>> were checking the first controller who uses a phy from the block and
>>>> then end up looking for a dr_mode property in e.g. the ehci controller.
>>>>
>>>> This commit fixes this by adding an arg0 parameter to
>>>> of_usb_get_dr_mode_by_phy and make of_usb_get_dr_mode_by_phy
>>>> check that this matches the phandle args[0] value when looking for
>>>> the otg controller.
>>>>
>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>> ---
>>>> Changes in v2:
>>>> -Add a args0 parameter instead of looking for nodes with a dr_mode property
>>>> Changes in v3:
>>>> -No changes
>>>> ---
>>>> drivers/usb/common/common.c  | 31 ++++++++++++++++++-------------
>>>> drivers/usb/phy/phy-am335x.c |  2 +-
>>>> include/linux/usb/of.h       |  4 ++--
>>>> 3 files changed, 21 insertions(+), 16 deletions(-)
>>>
>>> This breaks am335x.
>>>
>>> [   17.433166] /ocp/usb at 47400000/usb at 47401000: could not get #phy-cells
>>> for /ocp/usb at 47400000/usb-phy at 47401300
>>> [   17.443627] /ocp/usb at 47400000/usb at 47401800: could not get #phy-cells
>>> for /ocp/usb at 47400000/usb-phy at 47401b00
>>> [   17.454005] am335x-phy-driver 47401300.usb-phy: dr_mode 0
>>> [   17.460518] 47401300.usb-phy supply vcc not found, using dummy
>>> regulator
>>> [   17.469685] /ocp/usb at 47400000/usb at 47401000: could not get #phy-cells
>>> for /ocp/usb at 47400000/usb-phy at 47401300
>>> [   17.479998] /ocp/usb at 47400000/usb at 47401800: could not get #phy-cells
>>> for /ocp/usb at 47400000/usb-phy at 47401b00
>>> [   17.490342] am335x-phy-driver 47401b00.usb-phy: dr_mode 0
>>
>> That is weird, quoting: drivers/phy/phy-core.c: _of_phy_get():
>>
>>         ret = of_parse_phandle_with_args(np, "phys", "#phy-cells",
>>                 index, &args);
>>         if (ret)
>>                 return ERR_PTR(-ENODEV);
>>
>> So if #phy-cells is not defined, then the phy core should not
>> be able to work with the dts files in question at all.
>
> am335x phy does not use phy framework, so those phy core api is not
> called.
>
>>
>> All my patch does is make the way of_usb_get_dr_mode_by_phy parses
>> phy-handles be identical to how phy-core.c does it.
>>
>> I see that the usb0_phy node in arch/arm/boot/dts/am33xx.dtsi
>> indeed lacks a "#phy-cells = <0>;" line.
>>
>> Quoting: Documentation/devicetree/bindings/phy/phy-bindings.txt
>>
>> PHY device node
>> ===============
>>
>> Required Properties:
>> #phy-cells:     Number of cells in a PHY specifier;  The meaning of all those
>>                 cells is defined by the binding for the phy node. The PHY
>>                 provider can use the values in cells to find the appropriate
>>                 PHY.
>>
>> So not having #phy-cells defined for a phy-node clearly is a bug.
>
> am335x phy does not follow the bindings in phy/, but usb/phy/.

Ah, I did not know about those, that explains. I will send a v4 of
the patch which will keep compatibility with the usb/phy/ bindings
when the passed in arg0 == -1.

Note I will not be resending patch 2-4, please merge these as they
are (no changes are needed).

>>
>> A patch like the following should fix this:
>>
>> diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
>> index 52be48b..c4ff788d 100644
>> --- a/arch/arm/boot/dts/am33xx.dtsi
>> +++ b/arch/arm/boot/dts/am33xx.dtsi
>> @@ -557,6 +557,7 @@
>>  			};
>>
>>  			usb0_phy: usb-phy at 47401300 {
>> +				#phy-cells = <0>;
>
> This seems to be ok, but this patch would break the dt backward compatible.

I understand, as said above I'll do a new version of the patch preserving
compatibility.

>>  				compatible = "ti,am335x-usb-phy";
>>  				reg = <0x47401300 0x100>;
>>  				reg-names = "phy";
>>

Regards,

Hans

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 4/4] musb: sunxi: Simplify dr_mode handling
  2016-06-05 14:59     ` Hans de Goede
@ 2016-06-10 14:53         ` Bin Liu
  -1 siblings, 0 replies; 28+ messages in thread
From: Bin Liu @ 2016-06-10 14:53 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Greg Kroah-Hartman, Kishon Vijay Abraham I, Maxime Ripard,
	Chen-Yu Tsai, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree

Hi,

On Sun, Jun 05, 2016 at 04:59:36PM +0200, Hans de Goede wrote:
> phy-sun4i-usb now has proper dr_mode handling, it always registers an
> extcon, and sends a notify with the mode (even when in peripheral- /
> host-only mode) at least once.
> 
> So we can simply the sunxi musb glue by always registering its extcon
> notifier and relying on sunxi_musb_work() to enable vbus when in
> host-only mode.
> 
> This also enables host- and peripheral-only mode with vbus monitoring.
> 
> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> Changes in v2:
> -No changes
> Changes in v3:
> -No changes

Acked-by: Bin Liu <b-liu-l0cyMroinI0@public.gmane.org>

Regards,
-Bin.

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v3 4/4] musb: sunxi: Simplify dr_mode handling
@ 2016-06-10 14:53         ` Bin Liu
  0 siblings, 0 replies; 28+ messages in thread
From: Bin Liu @ 2016-06-10 14:53 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Sun, Jun 05, 2016 at 04:59:36PM +0200, Hans de Goede wrote:
> phy-sun4i-usb now has proper dr_mode handling, it always registers an
> extcon, and sends a notify with the mode (even when in peripheral- /
> host-only mode) at least once.
> 
> So we can simply the sunxi musb glue by always registering its extcon
> notifier and relying on sunxi_musb_work() to enable vbus when in
> host-only mode.
> 
> This also enables host- and peripheral-only mode with vbus monitoring.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -No changes
> Changes in v3:
> -No changes

Acked-by: Bin Liu <b-liu@ti.com>

Regards,
-Bin.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 4/4] musb: sunxi: Simplify dr_mode handling
  2016-06-08 10:30           ` Hans de Goede
@ 2016-06-15 19:30               ` Maxime Ripard
  -1 siblings, 0 replies; 28+ messages in thread
From: Maxime Ripard @ 2016-06-15 19:30 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Bin Liu, Greg Kroah-Hartman, Kishon Vijay Abraham I,
	Chen-Yu Tsai, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree

[-- Attachment #1: Type: text/plain, Size: 1656 bytes --]

Hi,

On Wed, Jun 08, 2016 at 12:30:20PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 08-06-16 12:23, Maxime Ripard wrote:
> >Hi,
> >
> >On Sun, Jun 05, 2016 at 04:59:36PM +0200, Hans de Goede wrote:
> >>phy-sun4i-usb now has proper dr_mode handling, it always registers an
> >>extcon, and sends a notify with the mode (even when in peripheral- /
> >>host-only mode) at least once.
> >>
> >>So we can simply the sunxi musb glue by always registering its extcon
> >>notifier and relying on sunxi_musb_work() to enable vbus when in
> >>host-only mode.
> >>
> >>This also enables host- and peripheral-only mode with vbus monitoring.
> >>
> >>Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> >
> >It's been a bit painful to track all the patches needed so that it
> >applies properly, but I've finally been able to test it on a Sinlinx
> >SinA33 with peripheral-only mUSB, and it works like a charm.
> >
> >You can add my Tested-by.
> 
> Great, thanks for testing.
> 
> This is the board which has an otg connector with vbus not connected,
> right? Yet it does have a functional id-pin, right ?

It's that one, yes.

> In that case you should be able to put it in dual-role mode (only
> specify the id-pin in the phy dts node, no vbus / vbus-monitoring)
> and then it _should_ work in host mode if you use a powered hub.
> 
> I'm fine with putting in peripheral-only mode, but as said
> dual-role might work with a powered hub.

Good point, I'll test that.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v3 4/4] musb: sunxi: Simplify dr_mode handling
@ 2016-06-15 19:30               ` Maxime Ripard
  0 siblings, 0 replies; 28+ messages in thread
From: Maxime Ripard @ 2016-06-15 19:30 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Wed, Jun 08, 2016 at 12:30:20PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 08-06-16 12:23, Maxime Ripard wrote:
> >Hi,
> >
> >On Sun, Jun 05, 2016 at 04:59:36PM +0200, Hans de Goede wrote:
> >>phy-sun4i-usb now has proper dr_mode handling, it always registers an
> >>extcon, and sends a notify with the mode (even when in peripheral- /
> >>host-only mode) at least once.
> >>
> >>So we can simply the sunxi musb glue by always registering its extcon
> >>notifier and relying on sunxi_musb_work() to enable vbus when in
> >>host-only mode.
> >>
> >>This also enables host- and peripheral-only mode with vbus monitoring.
> >>
> >>Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >
> >It's been a bit painful to track all the patches needed so that it
> >applies properly, but I've finally been able to test it on a Sinlinx
> >SinA33 with peripheral-only mUSB, and it works like a charm.
> >
> >You can add my Tested-by.
> 
> Great, thanks for testing.
> 
> This is the board which has an otg connector with vbus not connected,
> right? Yet it does have a functional id-pin, right ?

It's that one, yes.

> In that case you should be able to put it in dual-role mode (only
> specify the id-pin in the phy dts node, no vbus / vbus-monitoring)
> and then it _should_ work in host mode if you use a powered hub.
> 
> I'm fine with putting in peripheral-only mode, but as said
> dual-role might work with a powered hub.

Good point, I'll test that.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160615/4ee36279/attachment-0001.sig>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 2/4] phy-sun4i-usb: Add support for peripheral-only mode
  2016-06-05 14:59     ` Hans de Goede
@ 2016-06-17 13:12         ` Kishon Vijay Abraham I
  -1 siblings, 0 replies; 28+ messages in thread
From: Kishon Vijay Abraham I @ 2016-06-17 13:12 UTC (permalink / raw)
  To: Hans de Goede, Bin Liu, Greg Kroah-Hartman
  Cc: Maxime Ripard, Chen-Yu Tsai, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree



On Sunday 05 June 2016 08:29 PM, Hans de Goede wrote:
> Use the new of_usb_get_dr_mode_by_phy() function to get the dr_mode
> from the musb controller node instead of assuming that having an id_det
> gpio means otg mode, and not having one means host mode.
> 
> Implement peripheral-only mode by adding a sun4i_usb_phy0_get_id_det
> helper which looks at the dr_mode, always registering our extcon and
> always monitoring vbus.
> 
> If dr_mode is not specified in the dts, do not register phy0 as we then
> do not know how to treat it. This is actually a good thing as this means
> we will not be registering phy0 on devices where the otg controller is
> not enabled in the devicetree.
> 
> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Acked-by: Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org>
> ---
> Changes in v2:
> -Adjust for of_usb_get_dr_mode_by_phy now taking an args0 parameter
> Changes in v3:
> -Only toggle the phy vbus-det bit on id-change on systems without vbus-det
>  when in otg mode
> ---
>  drivers/phy/phy-sun4i-usb.c | 68 ++++++++++++++++++++++++++++++---------------
>  1 file changed, 46 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
> index bae54f7..e3cbaae 100644
> --- a/drivers/phy/phy-sun4i-usb.c
> +++ b/drivers/phy/phy-sun4i-usb.c
> @@ -40,6 +40,7 @@
>  #include <linux/power_supply.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/reset.h>
> +#include <linux/usb/of.h>
>  #include <linux/workqueue.h>
>  
>  #define REG_ISCR			0x00
> @@ -109,6 +110,7 @@ struct sun4i_usb_phy_cfg {
>  struct sun4i_usb_phy_data {
>  	void __iomem *base;
>  	const struct sun4i_usb_phy_cfg *cfg;
> +	enum usb_dr_mode dr_mode;
>  	struct mutex mutex;
>  	struct sun4i_usb_phy {
>  		struct phy *phy;
> @@ -119,6 +121,7 @@ struct sun4i_usb_phy_data {
>  		bool regulator_on;
>  		int index;
>  	} phys[MAX_PHYS];
> +	int first_phy;
>  	/* phy0 / otg related variables */
>  	struct extcon_dev *extcon;
>  	bool phy0_init;
> @@ -285,16 +288,10 @@ static int sun4i_usb_phy_init(struct phy *_phy)
>  		sun4i_usb_phy0_update_iscr(_phy, 0, ISCR_DPDM_PULLUP_EN);
>  		sun4i_usb_phy0_update_iscr(_phy, 0, ISCR_ID_PULLUP_EN);
>  
> -		if (data->id_det_gpio) {
> -			/* OTG mode, force ISCR and cable state updates */
> -			data->id_det = -1;
> -			data->vbus_det = -1;
> -			queue_delayed_work(system_wq, &data->detect, 0);
> -		} else {
> -			/* Host only mode */
> -			sun4i_usb_phy0_set_id_detect(_phy, 0);
> -			sun4i_usb_phy0_set_vbus_detect(_phy, 1);
> -		}
> +		/* Force ISCR and cable state updates */
> +		data->id_det = -1;
> +		data->vbus_det = -1;
> +		queue_delayed_work(system_wq, &data->detect, 0);
>  	}
>  
>  	return 0;
> @@ -319,6 +316,19 @@ static int sun4i_usb_phy_exit(struct phy *_phy)
>  	return 0;
>  }
>  
> +static int sun4i_usb_phy0_get_id_det(struct sun4i_usb_phy_data *data)
> +{
> +	switch (data->dr_mode) {
> +	case USB_DR_MODE_OTG:
> +		return gpiod_get_value_cansleep(data->id_det_gpio);
> +	case USB_DR_MODE_HOST:
> +		return 0;
> +	case USB_DR_MODE_PERIPHERAL:
> +	default:
> +		return 1;
> +	}
> +}
> +
>  static int sun4i_usb_phy0_get_vbus_det(struct sun4i_usb_phy_data *data)
>  {
>  	if (data->vbus_det_gpio)
> @@ -414,7 +424,10 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct work_struct *work)
>  	struct phy *phy0 = data->phys[0].phy;
>  	int id_det, vbus_det, id_notify = 0, vbus_notify = 0;
>  
> -	id_det = gpiod_get_value_cansleep(data->id_det_gpio);
> +	if (phy0 == NULL)
> +		return;
> +
> +	id_det = sun4i_usb_phy0_get_id_det(data);
>  	vbus_det = sun4i_usb_phy0_get_vbus_det(data);
>  
>  	mutex_lock(&phy0->mutex);
> @@ -430,7 +443,8 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct work_struct *work)
>  		 * without vbus detection report vbus low for long enough for
>  		 * the musb-ip to end the current device session.
>  		 */
> -		if (!sun4i_usb_phy0_have_vbus_det(data) && id_det == 0) {
> +		if (data->dr_mode == USB_DR_MODE_OTG &&
> +		    !sun4i_usb_phy0_have_vbus_det(data) && id_det == 0) {
>  			sun4i_usb_phy0_set_vbus_detect(phy0, 0);
>  			msleep(200);
>  			sun4i_usb_phy0_set_vbus_detect(phy0, 1);
> @@ -456,7 +470,8 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct work_struct *work)
>  		 * without vbus detection report vbus low for long enough to
>  		 * the musb-ip to end the current host session.
>  		 */
> -		if (!sun4i_usb_phy0_have_vbus_det(data) && id_det == 1) {
> +		if (data->dr_mode == USB_DR_MODE_OTG &&
> +		    !sun4i_usb_phy0_have_vbus_det(data) && id_det == 1) {
>  			mutex_lock(&phy0->mutex);
>  			sun4i_usb_phy0_set_vbus_detect(phy0, 0);
>  			msleep(1000);
> @@ -501,7 +516,8 @@ static struct phy *sun4i_usb_phy_xlate(struct device *dev,
>  {
>  	struct sun4i_usb_phy_data *data = dev_get_drvdata(dev);
>  
> -	if (args->args[0] >= data->cfg->num_phys)
> +	if (args->args[0] < data->first_phy ||
> +	    args->args[0] >= data->cfg->num_phys)
>  		return ERR_PTR(-ENODEV);
>  
>  	return data->phys[args->args[0]].phy;
> @@ -575,13 +591,17 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
>  			return -EPROBE_DEFER;
>  	}
>  
> -	/* vbus_det without id_det makes no sense, and is not supported */
> -	if (sun4i_usb_phy0_have_vbus_det(data) && !data->id_det_gpio) {
> -		dev_err(dev, "usb0_id_det missing or invalid\n");
> -		return -ENODEV;
> -	}
> -
> -	if (data->id_det_gpio) {
> +	data->dr_mode = of_usb_get_dr_mode_by_phy(np, 0);
> +	switch (data->dr_mode) {
> +	case USB_DR_MODE_OTG:
> +		/* otg without id_det makes no sense, and is not supported */
> +		if (!data->id_det_gpio) {
> +			dev_err(dev, "usb0_id_det missing or invalid\n");
> +			return -ENODEV;
> +		}
> +		/* fall through */
> +	case USB_DR_MODE_HOST:
> +	case USB_DR_MODE_PERIPHERAL:
>  		data->extcon = devm_extcon_dev_allocate(dev,
>  							sun4i_usb_phy0_cable);
>  		if (IS_ERR(data->extcon))
> @@ -592,9 +612,13 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
>  			dev_err(dev, "failed to register extcon: %d\n", ret);
>  			return ret;
>  		}
> +		break;
> +	default:
> +		dev_info(dev, "dr_mode unknown, not registering usb phy0\n");
> +		data->first_phy = 1;
>  	}
>  
> -	for (i = 0; i < data->cfg->num_phys; i++) {
> +	for (i = data->first_phy; i < data->cfg->num_phys; i++) {
>  		struct sun4i_usb_phy *phy = data->phys + i;
>  		char name[16];
>  
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v3 2/4] phy-sun4i-usb: Add support for peripheral-only mode
@ 2016-06-17 13:12         ` Kishon Vijay Abraham I
  0 siblings, 0 replies; 28+ messages in thread
From: Kishon Vijay Abraham I @ 2016-06-17 13:12 UTC (permalink / raw)
  To: linux-arm-kernel



On Sunday 05 June 2016 08:29 PM, Hans de Goede wrote:
> Use the new of_usb_get_dr_mode_by_phy() function to get the dr_mode
> from the musb controller node instead of assuming that having an id_det
> gpio means otg mode, and not having one means host mode.
> 
> Implement peripheral-only mode by adding a sun4i_usb_phy0_get_id_det
> helper which looks at the dr_mode, always registering our extcon and
> always monitoring vbus.
> 
> If dr_mode is not specified in the dts, do not register phy0 as we then
> do not know how to treat it. This is actually a good thing as this means
> we will not be registering phy0 on devices where the otg controller is
> not enabled in the devicetree.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> Changes in v2:
> -Adjust for of_usb_get_dr_mode_by_phy now taking an args0 parameter
> Changes in v3:
> -Only toggle the phy vbus-det bit on id-change on systems without vbus-det
>  when in otg mode
> ---
>  drivers/phy/phy-sun4i-usb.c | 68 ++++++++++++++++++++++++++++++---------------
>  1 file changed, 46 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
> index bae54f7..e3cbaae 100644
> --- a/drivers/phy/phy-sun4i-usb.c
> +++ b/drivers/phy/phy-sun4i-usb.c
> @@ -40,6 +40,7 @@
>  #include <linux/power_supply.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/reset.h>
> +#include <linux/usb/of.h>
>  #include <linux/workqueue.h>
>  
>  #define REG_ISCR			0x00
> @@ -109,6 +110,7 @@ struct sun4i_usb_phy_cfg {
>  struct sun4i_usb_phy_data {
>  	void __iomem *base;
>  	const struct sun4i_usb_phy_cfg *cfg;
> +	enum usb_dr_mode dr_mode;
>  	struct mutex mutex;
>  	struct sun4i_usb_phy {
>  		struct phy *phy;
> @@ -119,6 +121,7 @@ struct sun4i_usb_phy_data {
>  		bool regulator_on;
>  		int index;
>  	} phys[MAX_PHYS];
> +	int first_phy;
>  	/* phy0 / otg related variables */
>  	struct extcon_dev *extcon;
>  	bool phy0_init;
> @@ -285,16 +288,10 @@ static int sun4i_usb_phy_init(struct phy *_phy)
>  		sun4i_usb_phy0_update_iscr(_phy, 0, ISCR_DPDM_PULLUP_EN);
>  		sun4i_usb_phy0_update_iscr(_phy, 0, ISCR_ID_PULLUP_EN);
>  
> -		if (data->id_det_gpio) {
> -			/* OTG mode, force ISCR and cable state updates */
> -			data->id_det = -1;
> -			data->vbus_det = -1;
> -			queue_delayed_work(system_wq, &data->detect, 0);
> -		} else {
> -			/* Host only mode */
> -			sun4i_usb_phy0_set_id_detect(_phy, 0);
> -			sun4i_usb_phy0_set_vbus_detect(_phy, 1);
> -		}
> +		/* Force ISCR and cable state updates */
> +		data->id_det = -1;
> +		data->vbus_det = -1;
> +		queue_delayed_work(system_wq, &data->detect, 0);
>  	}
>  
>  	return 0;
> @@ -319,6 +316,19 @@ static int sun4i_usb_phy_exit(struct phy *_phy)
>  	return 0;
>  }
>  
> +static int sun4i_usb_phy0_get_id_det(struct sun4i_usb_phy_data *data)
> +{
> +	switch (data->dr_mode) {
> +	case USB_DR_MODE_OTG:
> +		return gpiod_get_value_cansleep(data->id_det_gpio);
> +	case USB_DR_MODE_HOST:
> +		return 0;
> +	case USB_DR_MODE_PERIPHERAL:
> +	default:
> +		return 1;
> +	}
> +}
> +
>  static int sun4i_usb_phy0_get_vbus_det(struct sun4i_usb_phy_data *data)
>  {
>  	if (data->vbus_det_gpio)
> @@ -414,7 +424,10 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct work_struct *work)
>  	struct phy *phy0 = data->phys[0].phy;
>  	int id_det, vbus_det, id_notify = 0, vbus_notify = 0;
>  
> -	id_det = gpiod_get_value_cansleep(data->id_det_gpio);
> +	if (phy0 == NULL)
> +		return;
> +
> +	id_det = sun4i_usb_phy0_get_id_det(data);
>  	vbus_det = sun4i_usb_phy0_get_vbus_det(data);
>  
>  	mutex_lock(&phy0->mutex);
> @@ -430,7 +443,8 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct work_struct *work)
>  		 * without vbus detection report vbus low for long enough for
>  		 * the musb-ip to end the current device session.
>  		 */
> -		if (!sun4i_usb_phy0_have_vbus_det(data) && id_det == 0) {
> +		if (data->dr_mode == USB_DR_MODE_OTG &&
> +		    !sun4i_usb_phy0_have_vbus_det(data) && id_det == 0) {
>  			sun4i_usb_phy0_set_vbus_detect(phy0, 0);
>  			msleep(200);
>  			sun4i_usb_phy0_set_vbus_detect(phy0, 1);
> @@ -456,7 +470,8 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct work_struct *work)
>  		 * without vbus detection report vbus low for long enough to
>  		 * the musb-ip to end the current host session.
>  		 */
> -		if (!sun4i_usb_phy0_have_vbus_det(data) && id_det == 1) {
> +		if (data->dr_mode == USB_DR_MODE_OTG &&
> +		    !sun4i_usb_phy0_have_vbus_det(data) && id_det == 1) {
>  			mutex_lock(&phy0->mutex);
>  			sun4i_usb_phy0_set_vbus_detect(phy0, 0);
>  			msleep(1000);
> @@ -501,7 +516,8 @@ static struct phy *sun4i_usb_phy_xlate(struct device *dev,
>  {
>  	struct sun4i_usb_phy_data *data = dev_get_drvdata(dev);
>  
> -	if (args->args[0] >= data->cfg->num_phys)
> +	if (args->args[0] < data->first_phy ||
> +	    args->args[0] >= data->cfg->num_phys)
>  		return ERR_PTR(-ENODEV);
>  
>  	return data->phys[args->args[0]].phy;
> @@ -575,13 +591,17 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
>  			return -EPROBE_DEFER;
>  	}
>  
> -	/* vbus_det without id_det makes no sense, and is not supported */
> -	if (sun4i_usb_phy0_have_vbus_det(data) && !data->id_det_gpio) {
> -		dev_err(dev, "usb0_id_det missing or invalid\n");
> -		return -ENODEV;
> -	}
> -
> -	if (data->id_det_gpio) {
> +	data->dr_mode = of_usb_get_dr_mode_by_phy(np, 0);
> +	switch (data->dr_mode) {
> +	case USB_DR_MODE_OTG:
> +		/* otg without id_det makes no sense, and is not supported */
> +		if (!data->id_det_gpio) {
> +			dev_err(dev, "usb0_id_det missing or invalid\n");
> +			return -ENODEV;
> +		}
> +		/* fall through */
> +	case USB_DR_MODE_HOST:
> +	case USB_DR_MODE_PERIPHERAL:
>  		data->extcon = devm_extcon_dev_allocate(dev,
>  							sun4i_usb_phy0_cable);
>  		if (IS_ERR(data->extcon))
> @@ -592,9 +612,13 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
>  			dev_err(dev, "failed to register extcon: %d\n", ret);
>  			return ret;
>  		}
> +		break;
> +	default:
> +		dev_info(dev, "dr_mode unknown, not registering usb phy0\n");
> +		data->first_phy = 1;
>  	}
>  
> -	for (i = 0; i < data->cfg->num_phys; i++) {
> +	for (i = data->first_phy; i < data->cfg->num_phys; i++) {
>  		struct sun4i_usb_phy *phy = data->phys + i;
>  		char name[16];
>  
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 3/4] phy-sun4i-usb: Add workaround for missing Vbus det interrupts on A31
  2016-06-05 14:59     ` Hans de Goede
@ 2016-06-17 13:12         ` Kishon Vijay Abraham I
  -1 siblings, 0 replies; 28+ messages in thread
From: Kishon Vijay Abraham I @ 2016-06-17 13:12 UTC (permalink / raw)
  To: Hans de Goede, Bin Liu, Greg Kroah-Hartman
  Cc: Maxime Ripard, Chen-Yu Tsai, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree



On Sunday 05 June 2016 08:29 PM, Hans de Goede wrote:
> The A31 companion pmic (axp221) does not generate vbus change interrupts
> when the board is driving vbus, so we must poll when using the pmic for
> vbus-det _and_ we're driving vbus.
> 
> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Acked-by: Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org>
> ---
> Changes in v2:
> -No changes
> Changes in v3:
> -No changes
> ---
>  drivers/phy/phy-sun4i-usb.c | 34 ++++++++++++++++++++++++----------
>  1 file changed, 24 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
> index e3cbaae..a7abae6 100644
> --- a/drivers/phy/phy-sun4i-usb.c
> +++ b/drivers/phy/phy-sun4i-usb.c
> @@ -95,6 +95,7 @@
>  
>  enum sun4i_usb_phy_type {
>  	sun4i_a10_phy,
> +	sun6i_a31_phy,
>  	sun8i_a33_phy,
>  	sun8i_h3_phy,
>  };
> @@ -125,7 +126,6 @@ struct sun4i_usb_phy_data {
>  	/* phy0 / otg related variables */
>  	struct extcon_dev *extcon;
>  	bool phy0_init;
> -	bool phy0_poll;
>  	struct gpio_desc *id_det_gpio;
>  	struct gpio_desc *vbus_det_gpio;
>  	struct power_supply *vbus_power_supply;
> @@ -353,6 +353,24 @@ static bool sun4i_usb_phy0_have_vbus_det(struct sun4i_usb_phy_data *data)
>  	return data->vbus_det_gpio || data->vbus_power_supply;
>  }
>  
> +static bool sun4i_usb_phy0_poll(struct sun4i_usb_phy_data *data)
> +{
> +	if ((data->id_det_gpio && data->id_det_irq < 0) ||
> +	    (data->vbus_det_gpio && data->vbus_det_irq < 0))
> +		return true;
> +
> +	/*
> +	 * The A31 companion pmic (axp221) does not generate vbus change
> +	 * interrupts when the board is driving vbus, so we must poll
> +	 * when using the pmic for vbus-det _and_ we're driving vbus.
> +	 */
> +	if (data->cfg->type == sun6i_a31_phy &&
> +	    data->vbus_power_supply && data->phys[0].regulator_on)
> +		return true;
> +
> +	return false;
> +}
> +
>  static int sun4i_usb_phy_power_on(struct phy *_phy)
>  {
>  	struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
> @@ -374,7 +392,7 @@ static int sun4i_usb_phy_power_on(struct phy *_phy)
>  	phy->regulator_on = true;
>  
>  	/* We must report Vbus high within OTG_TIME_A_WAIT_VRISE msec. */
> -	if (phy->index == 0 && data->vbus_det_gpio && data->phy0_poll)
> +	if (phy->index == 0 && sun4i_usb_phy0_poll(data))
>  		mod_delayed_work(system_wq, &data->detect, DEBOUNCE_TIME);
>  
>  	return 0;
> @@ -395,7 +413,7 @@ static int sun4i_usb_phy_power_off(struct phy *_phy)
>  	 * phy0 vbus typically slowly discharges, sometimes this causes the
>  	 * Vbus gpio to not trigger an edge irq on Vbus off, so force a rescan.
>  	 */
> -	if (phy->index == 0 && data->vbus_det_gpio && !data->phy0_poll)
> +	if (phy->index == 0 && !sun4i_usb_phy0_poll(data))
>  		mod_delayed_work(system_wq, &data->detect, POLL_TIME);
>  
>  	return 0;
> @@ -483,7 +501,7 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct work_struct *work)
>  	if (vbus_notify)
>  		extcon_set_cable_state_(data->extcon, EXTCON_USB, vbus_det);
>  
> -	if (data->phy0_poll)
> +	if (sun4i_usb_phy0_poll(data))
>  		queue_delayed_work(system_wq, &data->detect, POLL_TIME);
>  }
>  
> @@ -668,11 +686,6 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
>  	}
>  
>  	data->id_det_irq = gpiod_to_irq(data->id_det_gpio);
> -	data->vbus_det_irq = gpiod_to_irq(data->vbus_det_gpio);
> -	if ((data->id_det_gpio && data->id_det_irq < 0) ||
> -	    (data->vbus_det_gpio && data->vbus_det_irq < 0))
> -		data->phy0_poll = true;
> -
>  	if (data->id_det_irq >= 0) {
>  		ret = devm_request_irq(dev, data->id_det_irq,
>  				sun4i_usb_phy0_id_vbus_det_irq,
> @@ -684,6 +697,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> +	data->vbus_det_irq = gpiod_to_irq(data->vbus_det_gpio);
>  	if (data->vbus_det_irq >= 0) {
>  		ret = devm_request_irq(dev, data->vbus_det_irq,
>  				sun4i_usb_phy0_id_vbus_det_irq,
> @@ -735,7 +749,7 @@ static const struct sun4i_usb_phy_cfg sun5i_a13_cfg = {
>  
>  static const struct sun4i_usb_phy_cfg sun6i_a31_cfg = {
>  	.num_phys = 3,
> -	.type = sun4i_a10_phy,
> +	.type = sun6i_a31_phy,
>  	.disc_thresh = 3,
>  	.phyctl_offset = REG_PHYCTL_A10,
>  	.dedicated_clocks = true,
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v3 3/4] phy-sun4i-usb: Add workaround for missing Vbus det interrupts on A31
@ 2016-06-17 13:12         ` Kishon Vijay Abraham I
  0 siblings, 0 replies; 28+ messages in thread
From: Kishon Vijay Abraham I @ 2016-06-17 13:12 UTC (permalink / raw)
  To: linux-arm-kernel



On Sunday 05 June 2016 08:29 PM, Hans de Goede wrote:
> The A31 companion pmic (axp221) does not generate vbus change interrupts
> when the board is driving vbus, so we must poll when using the pmic for
> vbus-det _and_ we're driving vbus.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> Changes in v2:
> -No changes
> Changes in v3:
> -No changes
> ---
>  drivers/phy/phy-sun4i-usb.c | 34 ++++++++++++++++++++++++----------
>  1 file changed, 24 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
> index e3cbaae..a7abae6 100644
> --- a/drivers/phy/phy-sun4i-usb.c
> +++ b/drivers/phy/phy-sun4i-usb.c
> @@ -95,6 +95,7 @@
>  
>  enum sun4i_usb_phy_type {
>  	sun4i_a10_phy,
> +	sun6i_a31_phy,
>  	sun8i_a33_phy,
>  	sun8i_h3_phy,
>  };
> @@ -125,7 +126,6 @@ struct sun4i_usb_phy_data {
>  	/* phy0 / otg related variables */
>  	struct extcon_dev *extcon;
>  	bool phy0_init;
> -	bool phy0_poll;
>  	struct gpio_desc *id_det_gpio;
>  	struct gpio_desc *vbus_det_gpio;
>  	struct power_supply *vbus_power_supply;
> @@ -353,6 +353,24 @@ static bool sun4i_usb_phy0_have_vbus_det(struct sun4i_usb_phy_data *data)
>  	return data->vbus_det_gpio || data->vbus_power_supply;
>  }
>  
> +static bool sun4i_usb_phy0_poll(struct sun4i_usb_phy_data *data)
> +{
> +	if ((data->id_det_gpio && data->id_det_irq < 0) ||
> +	    (data->vbus_det_gpio && data->vbus_det_irq < 0))
> +		return true;
> +
> +	/*
> +	 * The A31 companion pmic (axp221) does not generate vbus change
> +	 * interrupts when the board is driving vbus, so we must poll
> +	 * when using the pmic for vbus-det _and_ we're driving vbus.
> +	 */
> +	if (data->cfg->type == sun6i_a31_phy &&
> +	    data->vbus_power_supply && data->phys[0].regulator_on)
> +		return true;
> +
> +	return false;
> +}
> +
>  static int sun4i_usb_phy_power_on(struct phy *_phy)
>  {
>  	struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
> @@ -374,7 +392,7 @@ static int sun4i_usb_phy_power_on(struct phy *_phy)
>  	phy->regulator_on = true;
>  
>  	/* We must report Vbus high within OTG_TIME_A_WAIT_VRISE msec. */
> -	if (phy->index == 0 && data->vbus_det_gpio && data->phy0_poll)
> +	if (phy->index == 0 && sun4i_usb_phy0_poll(data))
>  		mod_delayed_work(system_wq, &data->detect, DEBOUNCE_TIME);
>  
>  	return 0;
> @@ -395,7 +413,7 @@ static int sun4i_usb_phy_power_off(struct phy *_phy)
>  	 * phy0 vbus typically slowly discharges, sometimes this causes the
>  	 * Vbus gpio to not trigger an edge irq on Vbus off, so force a rescan.
>  	 */
> -	if (phy->index == 0 && data->vbus_det_gpio && !data->phy0_poll)
> +	if (phy->index == 0 && !sun4i_usb_phy0_poll(data))
>  		mod_delayed_work(system_wq, &data->detect, POLL_TIME);
>  
>  	return 0;
> @@ -483,7 +501,7 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct work_struct *work)
>  	if (vbus_notify)
>  		extcon_set_cable_state_(data->extcon, EXTCON_USB, vbus_det);
>  
> -	if (data->phy0_poll)
> +	if (sun4i_usb_phy0_poll(data))
>  		queue_delayed_work(system_wq, &data->detect, POLL_TIME);
>  }
>  
> @@ -668,11 +686,6 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
>  	}
>  
>  	data->id_det_irq = gpiod_to_irq(data->id_det_gpio);
> -	data->vbus_det_irq = gpiod_to_irq(data->vbus_det_gpio);
> -	if ((data->id_det_gpio && data->id_det_irq < 0) ||
> -	    (data->vbus_det_gpio && data->vbus_det_irq < 0))
> -		data->phy0_poll = true;
> -
>  	if (data->id_det_irq >= 0) {
>  		ret = devm_request_irq(dev, data->id_det_irq,
>  				sun4i_usb_phy0_id_vbus_det_irq,
> @@ -684,6 +697,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> +	data->vbus_det_irq = gpiod_to_irq(data->vbus_det_gpio);
>  	if (data->vbus_det_irq >= 0) {
>  		ret = devm_request_irq(dev, data->vbus_det_irq,
>  				sun4i_usb_phy0_id_vbus_det_irq,
> @@ -735,7 +749,7 @@ static const struct sun4i_usb_phy_cfg sun5i_a13_cfg = {
>  
>  static const struct sun4i_usb_phy_cfg sun6i_a31_cfg = {
>  	.num_phys = 3,
> -	.type = sun4i_a10_phy,
> +	.type = sun6i_a31_phy,
>  	.disc_thresh = 3,
>  	.phyctl_offset = REG_PHYCTL_A10,
>  	.dedicated_clocks = true,
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2016-06-17 13:12 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-05 14:59 [PATCH v3 1/4] USB: Fix of_usb_get_dr_mode_by_phy with a shared phy block Hans de Goede
2016-06-05 14:59 ` Hans de Goede
     [not found] ` <1465138776-6003-1-git-send-email-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2016-06-05 14:59   ` [PATCH v3 2/4] phy-sun4i-usb: Add support for peripheral-only mode Hans de Goede
2016-06-05 14:59     ` Hans de Goede
     [not found]     ` <1465138776-6003-2-git-send-email-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2016-06-17 13:12       ` Kishon Vijay Abraham I
2016-06-17 13:12         ` Kishon Vijay Abraham I
2016-06-05 14:59   ` [PATCH v3 3/4] phy-sun4i-usb: Add workaround for missing Vbus det interrupts on A31 Hans de Goede
2016-06-05 14:59     ` Hans de Goede
     [not found]     ` <1465138776-6003-3-git-send-email-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2016-06-17 13:12       ` Kishon Vijay Abraham I
2016-06-17 13:12         ` Kishon Vijay Abraham I
2016-06-05 14:59   ` [PATCH v3 4/4] musb: sunxi: Simplify dr_mode handling Hans de Goede
2016-06-05 14:59     ` Hans de Goede
     [not found]     ` <1465138776-6003-4-git-send-email-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2016-06-08 10:23       ` Maxime Ripard
2016-06-08 10:23         ` Maxime Ripard
2016-06-08 10:30         ` Hans de Goede
2016-06-08 10:30           ` Hans de Goede
     [not found]           ` <08af737c-f1f1-0966-0eca-24d4daa7423b-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2016-06-15 19:30             ` Maxime Ripard
2016-06-15 19:30               ` Maxime Ripard
2016-06-10 14:53       ` Bin Liu
2016-06-10 14:53         ` Bin Liu
2016-06-09 14:30   ` [PATCH v3 1/4] USB: Fix of_usb_get_dr_mode_by_phy with a shared phy block Bin Liu
2016-06-09 14:30     ` Bin Liu
2016-06-09 14:51     ` Hans de Goede
2016-06-09 14:51       ` Hans de Goede
     [not found]       ` <9a05c16a-e8a2-02b7-c093-1131e27bcdd4-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2016-06-09 19:49         ` Bin Liu
2016-06-09 19:49           ` Bin Liu
2016-06-10  9:27           ` Hans de Goede
2016-06-10  9:27             ` Hans de Goede

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.