Re: [PATCH v4 1/2] phy-sun4i-usb: Add full support for usb0 phy / OTG
Hi,
On Sunday 31 May 2015 09:40 PM, Hans de Goede wrote:
The usb0 phy is connected to an OTG controller, and as such needs some special
handling:
1) It allows explicit control over the pullups, enable these on phy_init and
disable them on phy_exit.
2) It has bits to signal id and vbus detect to the musb-core, add support for
for monitoring id and vbus detect gpio-s for use in dual role mode, and set
these bits to the correct values for operating in host only mode when no
gpios are specified in the devicetree.
3) When in dual role mode the musb sunxi glue needs to know if the a host or
device cable is plugged in, so when in dual role mode register an extcon.
While updating the devicetree binding documentation also add documentation
for the sofar undocumented usage of regulators for vbus for all 3 phys.
merged this to linux-phy tree.
Cheers
Kishon
Signed-off-by: Hans de Goede hdego...@redhat.com
---
Changes in v2:
-Removed the sunxi specific phy functions, instead the id / vbus gpio polling
has been moved to the phy-sun4i-usb driver and their status is exported
through extcon for the sunxi-musb glue
Changes in v3:
-No changes
Changes in v4:
-Do not call regulator_disable in an unbalanced manner when an external vbus
is present
---
.../devicetree/bindings/phy/sun4i-usb-phy.txt | 18 +-
drivers/phy/Kconfig| 1 +
drivers/phy/phy-sun4i-usb.c| 273
-
3 files changed, 281 insertions(+), 11 deletions(-)
diff --git a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
index 16528b9..557fa99 100644
--- a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
+++ b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
@@ -23,6 +23,13 @@ Required properties:
* usb1_reset
* usb2_reset for sun4i, sun6i or sun7i
+Optional properties:
+- usb0_id_det-gpios : gpio phandle for reading the otg id pin value
+- usb0_vbus_det-gpios : gpio phandle for detecting the presence of usb0 vbus
+- usb0_vbus-supply : regulator phandle for controller usb0 vbus
+- usb1_vbus-supply : regulator phandle for controller usb1 vbus
+- usb2_vbus-supply : regulator phandle for controller usb2 vbus
+
Example:
usbphy: phy@0x01c13400 {
#phy-cells = 1;
@@ -32,6 +39,13 @@ Example:
reg-names = phy_ctrl, pmu1, pmu2;
clocks = usb_clk 8;
clock-names = usb_phy;
- resets = usb_clk 1, usb_clk 2;
- reset-names = usb1_reset, usb2_reset;
+ resets = usb_clk 0, usb_clk 1, usb_clk 2;
+ reset-names = usb0_reset, usb1_reset, usb2_reset;
+ pinctrl-names = default;
+ pinctrl-0 = usb0_id_detect_pin, usb0_vbus_detect_pin;
+ usb0_id_det-gpios = pio 7 19 GPIO_ACTIVE_HIGH; /* PH19 */
+ usb0_vbus_det-gpios = pio 7 22 GPIO_ACTIVE_HIGH; /* PH22 */
+ usb0_vbus-supply = reg_usb0_vbus;
+ usb1_vbus-supply = reg_usb1_vbus;
+ usb2_vbus-supply = reg_usb2_vbus;
};
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index a53bd5b..4614fba 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -173,6 +173,7 @@ config PHY_SUN4I_USB
tristate Allwinner sunxi SoC USB PHY driver
depends on ARCH_SUNXI HAS_IOMEM OF
depends on RESET_CONTROLLER
+ select EXTCON
select GENERIC_PHY
help
Enable this to support the transceiver that is part of Allwinner
diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index 91c5be4..b45d707 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -1,7 +1,7 @@
/*
* Allwinner sun4i USB phy driver
*
- * Copyright (C) 2014 Hans de Goede hdego...@redhat.com
+ * Copyright (C) 2014-2015 Hans de Goede hdego...@redhat.com
*
* Based on code from
* Allwinner Technology Co., Ltd. www.allwinnertech.com
@@ -23,17 +23,23 @@
#include linux/clk.h
#include linux/err.h
+#include linux/extcon.h
#include linux/io.h
+#include linux/interrupt.h
#include linux/kernel.h
#include linux/module.h
#include linux/mutex.h
#include linux/of.h
#include linux/of_address.h
+#include linux/of_gpio.h
#include linux/phy/phy.h
#include linux/phy/phy-sun4i-usb.h
#include linux/platform_device.h
#include linux/regulator/consumer.h
#include linux/reset.h
+#include linux/workqueue.h
+
+#define DRIVER_NAME sun4i-usb-phy
#define REG_ISCR 0x00
#define REG_PHYCTL 0x04
@@ -47,6 +53,17 @@
#define SUNXI_AHB_INCRX_ALIGN_EN BIT(8)
#define SUNXI_ULPI_BYPASS_EN BIT(0)
+/* ISCR, Interface Status and Control bits */
+#define ISCR_ID_PULLUP_EN(1 17)
+#define ISCR_DPDM_PULLUP_EN (1 16)
+/* sunxi has the phy id/vbus pins not connected, so we use
Re: [PATCH v4 1/2] phy-sun4i-usb: Add full support for usb0 phy / OTG
On Wednesday 15 July 2015 04:25 PM, Kishon Vijay Abraham I wrote:
Hi,
On Sunday 31 May 2015 09:40 PM, Hans de Goede wrote:
The usb0 phy is connected to an OTG controller, and as such needs some
special
handling:
1) It allows explicit control over the pullups, enable these on phy_init and
disable them on phy_exit.
2) It has bits to signal id and vbus detect to the musb-core, add support for
for monitoring id and vbus detect gpio-s for use in dual role mode, and set
these bits to the correct values for operating in host only mode when no
gpios are specified in the devicetree.
3) When in dual role mode the musb sunxi glue needs to know if the a host or
device cable is plugged in, so when in dual role mode register an extcon.
While updating the devicetree binding documentation also add documentation
for the sofar undocumented usage of regulators for vbus for all 3 phys.
merged this to linux-phy tree.
Please ignore. I merged the v5 of the patch series.
Cheers
Kishon
Cheers
Kishon
Signed-off-by: Hans de Goede hdego...@redhat.com
---
Changes in v2:
-Removed the sunxi specific phy functions, instead the id / vbus gpio polling
has been moved to the phy-sun4i-usb driver and their status is exported
through extcon for the sunxi-musb glue
Changes in v3:
-No changes
Changes in v4:
-Do not call regulator_disable in an unbalanced manner when an external vbus
is present
---
.../devicetree/bindings/phy/sun4i-usb-phy.txt | 18 +-
drivers/phy/Kconfig| 1 +
drivers/phy/phy-sun4i-usb.c| 273
-
3 files changed, 281 insertions(+), 11 deletions(-)
diff --git a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
index 16528b9..557fa99 100644
--- a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
+++ b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
@@ -23,6 +23,13 @@ Required properties:
* usb1_reset
* usb2_reset for sun4i, sun6i or sun7i
+Optional properties:
+- usb0_id_det-gpios : gpio phandle for reading the otg id pin value
+- usb0_vbus_det-gpios : gpio phandle for detecting the presence of usb0 vbus
+- usb0_vbus-supply : regulator phandle for controller usb0 vbus
+- usb1_vbus-supply : regulator phandle for controller usb1 vbus
+- usb2_vbus-supply : regulator phandle for controller usb2 vbus
+
Example:
usbphy: phy@0x01c13400 {
#phy-cells = 1;
@@ -32,6 +39,13 @@ Example:
reg-names = phy_ctrl, pmu1, pmu2;
clocks = usb_clk 8;
clock-names = usb_phy;
-resets = usb_clk 1, usb_clk 2;
-reset-names = usb1_reset, usb2_reset;
+resets = usb_clk 0, usb_clk 1, usb_clk 2;
+reset-names = usb0_reset, usb1_reset, usb2_reset;
+pinctrl-names = default;
+pinctrl-0 = usb0_id_detect_pin, usb0_vbus_detect_pin;
+usb0_id_det-gpios = pio 7 19 GPIO_ACTIVE_HIGH; /* PH19 */
+usb0_vbus_det-gpios = pio 7 22 GPIO_ACTIVE_HIGH; /* PH22 */
+usb0_vbus-supply = reg_usb0_vbus;
+usb1_vbus-supply = reg_usb1_vbus;
+usb2_vbus-supply = reg_usb2_vbus;
};
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index a53bd5b..4614fba 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -173,6 +173,7 @@ config PHY_SUN4I_USB
tristate Allwinner sunxi SoC USB PHY driver
depends on ARCH_SUNXI HAS_IOMEM OF
depends on RESET_CONTROLLER
+select EXTCON
select GENERIC_PHY
help
Enable this to support the transceiver that is part of Allwinner
diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index 91c5be4..b45d707 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -1,7 +1,7 @@
/*
* Allwinner sun4i USB phy driver
*
- * Copyright (C) 2014 Hans de Goede hdego...@redhat.com
+ * Copyright (C) 2014-2015 Hans de Goede hdego...@redhat.com
*
* Based on code from
* Allwinner Technology Co., Ltd. www.allwinnertech.com
@@ -23,17 +23,23 @@
#include linux/clk.h
#include linux/err.h
+#include linux/extcon.h
#include linux/io.h
+#include linux/interrupt.h
#include linux/kernel.h
#include linux/module.h
#include linux/mutex.h
#include linux/of.h
#include linux/of_address.h
+#include linux/of_gpio.h
#include linux/phy/phy.h
#include linux/phy/phy-sun4i-usb.h
#include linux/platform_device.h
#include linux/regulator/consumer.h
#include linux/reset.h
+#include linux/workqueue.h
+
+#define DRIVER_NAME sun4i-usb-phy
#define REG_ISCR0x00
#define REG_PHYCTL 0x04
@@ -47,6 +53,17 @@
#define SUNXI_AHB_INCRX_ALIGN_ENBIT(8)
#define SUNXI_ULPI_BYPASS_ENBIT(0)
+/* ISCR, Interface Status and Control bits */
+#define
Re: [PATCH v4 1/2] phy-sun4i-usb: Add full support for usb0 phy / OTG
Hi,
On Sunday 31 May 2015 09:40 PM, Hans de Goede wrote:
The usb0 phy is connected to an OTG controller, and as such needs some special
handling:
1) It allows explicit control over the pullups, enable these on phy_init and
disable them on phy_exit.
2) It has bits to signal id and vbus detect to the musb-core, add support for
for monitoring id and vbus detect gpio-s for use in dual role mode, and set
these bits to the correct values for operating in host only mode when no
gpios are specified in the devicetree.
3) When in dual role mode the musb sunxi glue needs to know if the a host or
device cable is plugged in, so when in dual role mode register an extcon.
While updating the devicetree binding documentation also add documentation
for the sofar undocumented usage of regulators for vbus for all 3 phys.
Signed-off-by: Hans de Goede hdego...@redhat.com
---
Changes in v2:
-Removed the sunxi specific phy functions, instead the id / vbus gpio polling
has been moved to the phy-sun4i-usb driver and their status is exported
through extcon for the sunxi-musb glue
Changes in v3:
-No changes
Changes in v4:
-Do not call regulator_disable in an unbalanced manner when an external vbus
is present
---
.../devicetree/bindings/phy/sun4i-usb-phy.txt | 18 +-
drivers/phy/Kconfig| 1 +
drivers/phy/phy-sun4i-usb.c| 273 -
3 files changed, 281 insertions(+), 11 deletions(-)
.
.
snip
.
.
diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index 91c5be4..b45d707 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -1,7 +1,7 @@
.
.
snip
.
.
static struct phy *sun4i_usb_phy_xlate(struct device *dev,
struct of_phandle_args *args)
{
@@ -240,13 +417,20 @@ static int sun4i_usb_phy_probe(struct platform_device
*pdev)
struct phy_provider *phy_provider;
bool dedicated_clocks;
struct resource *res;
- int i;
+ int i, ret;
data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
if (!data)
return -ENOMEM;
mutex_init(data-mutex);
+ INIT_DELAYED_WORK(data-detect, sun4i_usb_phy0_id_vbus_det_scan);
+ data-extcon_cable_names[0] = extcon_cable_name[EXTCON_USB_HOST];
+ data-extcon_cable_names[1] = extcon_cable_name[EXTCON_USB];
+ data-extcon_cable_names[2] = NULL;
+ data-extcon.name = DRIVER_NAME;
+ data-extcon.supported_cable = data-extcon_cable_names;
+ data-extcon.dev.parent = dev;
if (of_device_is_compatible(np, allwinner,sun5i-a13-usb-phy))
data-num_phys = 2;
@@ -269,6 +453,34 @@ static int sun4i_usb_phy_probe(struct platform_device
*pdev)
if (IS_ERR(data-base))
return PTR_ERR(data-base);
+ data-id_det_gpio = devm_gpiod_get(dev, usb0_id_det, GPIOD_IN);
+ if (IS_ERR(data-id_det_gpio)) {
+ if (PTR_ERR(data-id_det_gpio) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+ data-id_det_gpio = NULL;
+ }
+
+ data-vbus_det_gpio = devm_gpiod_get(dev, usb0_vbus_det, GPIOD_IN);
+ if (IS_ERR(data-vbus_det_gpio)) {
+ if (PTR_ERR(data-vbus_det_gpio) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+ data-vbus_det_gpio = NULL;
+ }
+
+ /* We either want both gpio pins or neither (when in host mode) */
+ if (!data-id_det_gpio != !data-vbus_det_gpio) {
+ dev_err(dev, failed to get id or vbus detect pin\n);
+ return -ENODEV;
+ }
+
+ if (data-id_det_gpio) {
+ ret = devm_extcon_dev_register(dev, data-extcon);
+ if (ret) {
+ dev_err(dev, failed to register extcon: %d\n, ret);
+ return ret;
+ }
+ }
+
for (i = 0; i data-num_phys; i++) {
struct sun4i_usb_phy *phy = data-phys + i;
char name[16];
@@ -318,12 +530,54 @@ static int sun4i_usb_phy_probe(struct platform_device
*pdev)
phy_set_drvdata(phy-phy, data-phys[i]);
}
+ 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_irq 0 || data-vbus_det_irq 0)
+ data-phy0_poll = true;
if polling is enabled, we shouldn't enable irq at all?
+
+ if (data-id_det_irq = 0) {
+ ret = devm_request_irq(dev, data-id_det_irq,
+ sun4i_usb_phy0_id_vbus_det_irq,
+ IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+ usb0-id-det, data);
+ if (ret) {
+ dev_err(dev, Err requesting id-det-irq: %d\n, ret);
+ return ret;
+ }
+ }
+
+ if (data-vbus_det_irq = 0) {
+
Re: [linux-sunxi] Re: [PATCH v4 1/2] phy-sun4i-usb: Add full support for usb0 phy / OTG
On 06/11/2015 06:33 PM, Chanwoo Choi wrote: Hi, On 06/11/2015 05:59 PM, Hans de Goede wrote: Hi, On 11-06-15 10:29, Chanwoo Choi wrote: Hi Hans, On 06/11/2015 05:21 PM, Hans de Goede wrote: Hi Chanwoo, Thanks for the quick review. On 11-06-15 10:07, Chanwoo Choi wrote: Hi Hans, I add the comment about extcon-related code. Firstly, I'd like you to implment the extcon driver for phy-sun4i-usb device in drivers/extcon/ directoryby using MFD No, just no, this is not what the MFD framework is for, the usb-phy in question here is not a multifunction device. The MFD framework is intended for true multi-function devices like i2c attached PMICs which have regulators, gpios, pwm, input (power button), chargers, power-supply, etc. That is NOT the case here. Also moving this to the MFD framework would very likely requiring the devicetree binding for the usb-phy to change which we cannot do as that would break the devicetree ABI. because there are both extcon provider driver and extcon client driver. I think that all extcon provider driver better to be included in drivers/extcon/ directory. extcon_set_cable_state() function should be handled in extcon provider driver which is incluced in drivers/extcon/ directory. I do not find this a compelling reason, there are plenty of subsystems where not all implementations of the subsystem class live in the subsystem directory, e.g. input and hwmon devices are often also found outside of the input and hwmon driver directories. There are difference on between input/hwmon and extcon. Because input and hwmon driver implement the only one type driver as provider driver. But, extcon implement the two type driver of both extcon provider and extcon client driver. The extcon is similiar with regulator and clock framework as resource. extcon provider driver to provider the event when the state of external connector is changed. - devm_extcon_dev_register() - e.g., almost extcon provider driver are included in 'drivers/extcon/' directory. I understand, but that does not change my first argument, that the usb-phy is not a MFD device. And although it may be desirable to keep extcon provider drivers in the drivers/extcon, there are no technical reasons to do so. The whole reason why Kishon asked me to start using the extcon framework is to avoid adding a private API to the phy-sun4i-usb code for notifying the musb-sunxi code about otg-id-pin status changes. Adding a separate driver for just the extcon bits means re-adding a private api to the phy-sun4i-usb code but this time for the extcon code, at which point we might just as well skip extcon and have the musb-sunxi glue code call directly into the phy-sun4i-usb code... Needing a private API for a separate extcn driver actually is a good argument to NOT have a separate extcon driver and keep the extcon code in the phy-sun4i-usb code, where as I see no technical arguments in favor of a separate extcon driver. There is one technical issue. The extcon_set_cable_state() should be handled by extcon provider driver. because extcon_set_cable_state() inform the extcon client driver of the event when detecting the change of h/w line (gpio line) or register of peripheral device. But, extcon client driver can now get the instance of extcon_dev structure by extcon_get_edev_by_phandle() and then can change the cable state by using the extcon_set_cable_state(). I think that these issue have to be protected by framework level. I fix wrong word. (protected - prevented) - I think that these issue have to be prevented by framework level. Thanks, Chanwoo Choi -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH v4 1/2] phy-sun4i-usb: Add full support for usb0 phy / OTG
Hi, On 06/11/2015 06:38 PM, Hans de Goede wrote: Hi, On 11-06-15 11:33, Chanwoo Choi wrote: Hi, On 06/11/2015 05:59 PM, Hans de Goede wrote: Hi, On 11-06-15 10:29, Chanwoo Choi wrote: Hi Hans, On 06/11/2015 05:21 PM, Hans de Goede wrote: Hi Chanwoo, Thanks for the quick review. On 11-06-15 10:07, Chanwoo Choi wrote: Hi Hans, I add the comment about extcon-related code. Firstly, I'd like you to implment the extcon driver for phy-sun4i-usb device in drivers/extcon/ directoryby using MFD No, just no, this is not what the MFD framework is for, the usb-phy in question here is not a multifunction device. The MFD framework is intended for true multi-function devices like i2c attached PMICs which have regulators, gpios, pwm, input (power button), chargers, power-supply, etc. That is NOT the case here. Also moving this to the MFD framework would very likely requiring the devicetree binding for the usb-phy to change which we cannot do as that would break the devicetree ABI. because there are both extcon provider driver and extcon client driver. I think that all extcon provider driver better to be included in drivers/extcon/ directory. extcon_set_cable_state() function should be handled in extcon provider driver which is incluced in drivers/extcon/ directory. I do not find this a compelling reason, there are plenty of subsystems where not all implementations of the subsystem class live in the subsystem directory, e.g. input and hwmon devices are often also found outside of the input and hwmon driver directories. There are difference on between input/hwmon and extcon. Because input and hwmon driver implement the only one type driver as provider driver. But, extcon implement the two type driver of both extcon provider and extcon client driver. The extcon is similiar with regulator and clock framework as resource. extcon provider driver to provider the event when the state of external connector is changed. - devm_extcon_dev_register() - e.g., almost extcon provider driver are included in 'drivers/extcon/' directory. I understand, but that does not change my first argument, that the usb-phy is not a MFD device. And although it may be desirable to keep extcon provider drivers in the drivers/extcon, there are no technical reasons to do so. The whole reason why Kishon asked me to start using the extcon framework is to avoid adding a private API to the phy-sun4i-usb code for notifying the musb-sunxi code about otg-id-pin status changes. Adding a separate driver for just the extcon bits means re-adding a private api to the phy-sun4i-usb code but this time for the extcon code, at which point we might just as well skip extcon and have the musb-sunxi glue code call directly into the phy-sun4i-usb code... Needing a private API for a separate extcn driver actually is a good argument to NOT have a separate extcon driver and keep the extcon code in the phy-sun4i-usb code, where as I see no technical arguments in favor of a separate extcon driver. There is one technical issue. The extcon_set_cable_state() should be handled by extcon provider driver. That is something which can be done regardless of where the extcon provider driver code is located in the kernel tree... because extcon_set_cable_state() inform the extcon client driver of the event when detecting the change of h/w line (gpio line) or register of peripheral device. But, extcon client driver can now get the instance of extcon_dev structure by extcon_get_edev_by_phandle() and then can change the cable state by using the extcon_set_cable_state(). I think that these issue have to be protected by framework level. Protecting this at the framework level would mean protecting it with code in drivers/extcon/extcon.c, that code will be used (and thus can protect things) regardless of where the extcon provider code lives. I really still see no technical reasons why all extcon provider code MUST be under drivers/extcon. OK. As you said, this issue shold be prevented on framework. I'm considering what is appropriate method to resolve this issue. Thanks, Chanwoo Choi -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH v4 1/2] phy-sun4i-usb: Add full support for usb0 phy / OTG
Hi, On 06/11/2015 05:59 PM, Hans de Goede wrote: Hi, On 11-06-15 10:29, Chanwoo Choi wrote: Hi Hans, On 06/11/2015 05:21 PM, Hans de Goede wrote: Hi Chanwoo, Thanks for the quick review. On 11-06-15 10:07, Chanwoo Choi wrote: Hi Hans, I add the comment about extcon-related code. Firstly, I'd like you to implment the extcon driver for phy-sun4i-usb device in drivers/extcon/ directoryby using MFD No, just no, this is not what the MFD framework is for, the usb-phy in question here is not a multifunction device. The MFD framework is intended for true multi-function devices like i2c attached PMICs which have regulators, gpios, pwm, input (power button), chargers, power-supply, etc. That is NOT the case here. Also moving this to the MFD framework would very likely requiring the devicetree binding for the usb-phy to change which we cannot do as that would break the devicetree ABI. because there are both extcon provider driver and extcon client driver. I think that all extcon provider driver better to be included in drivers/extcon/ directory. extcon_set_cable_state() function should be handled in extcon provider driver which is incluced in drivers/extcon/ directory. I do not find this a compelling reason, there are plenty of subsystems where not all implementations of the subsystem class live in the subsystem directory, e.g. input and hwmon devices are often also found outside of the input and hwmon driver directories. There are difference on between input/hwmon and extcon. Because input and hwmon driver implement the only one type driver as provider driver. But, extcon implement the two type driver of both extcon provider and extcon client driver. The extcon is similiar with regulator and clock framework as resource. extcon provider driver to provider the event when the state of external connector is changed. - devm_extcon_dev_register() - e.g., almost extcon provider driver are included in 'drivers/extcon/' directory. I understand, but that does not change my first argument, that the usb-phy is not a MFD device. And although it may be desirable to keep extcon provider drivers in the drivers/extcon, there are no technical reasons to do so. The whole reason why Kishon asked me to start using the extcon framework is to avoid adding a private API to the phy-sun4i-usb code for notifying the musb-sunxi code about otg-id-pin status changes. Adding a separate driver for just the extcon bits means re-adding a private api to the phy-sun4i-usb code but this time for the extcon code, at which point we might just as well skip extcon and have the musb-sunxi glue code call directly into the phy-sun4i-usb code... Needing a private API for a separate extcn driver actually is a good argument to NOT have a separate extcon driver and keep the extcon code in the phy-sun4i-usb code, where as I see no technical arguments in favor of a separate extcon driver. There is one technical issue. The extcon_set_cable_state() should be handled by extcon provider driver. because extcon_set_cable_state() inform the extcon client driver of the event when detecting the change of h/w line (gpio line) or register of peripheral device. But, extcon client driver can now get the instance of extcon_dev structure by extcon_get_edev_by_phandle() and then can change the cable state by using the extcon_set_cable_state(). I think that these issue have to be protected by framework level. Thanks, Chanwoo Choi -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH v4 1/2] phy-sun4i-usb: Add full support for usb0 phy / OTG
Hi, On 11-06-15 10:29, Chanwoo Choi wrote: Hi Hans, On 06/11/2015 05:21 PM, Hans de Goede wrote: Hi Chanwoo, Thanks for the quick review. On 11-06-15 10:07, Chanwoo Choi wrote: Hi Hans, I add the comment about extcon-related code. Firstly, I'd like you to implment the extcon driver for phy-sun4i-usb device in drivers/extcon/ directoryby using MFD No, just no, this is not what the MFD framework is for, the usb-phy in question here is not a multifunction device. The MFD framework is intended for true multi-function devices like i2c attached PMICs which have regulators, gpios, pwm, input (power button), chargers, power-supply, etc. That is NOT the case here. Also moving this to the MFD framework would very likely requiring the devicetree binding for the usb-phy to change which we cannot do as that would break the devicetree ABI. because there are both extcon provider driver and extcon client driver. I think that all extcon provider driver better to be included in drivers/extcon/ directory. extcon_set_cable_state() function should be handled in extcon provider driver which is incluced in drivers/extcon/ directory. I do not find this a compelling reason, there are plenty of subsystems where not all implementations of the subsystem class live in the subsystem directory, e.g. input and hwmon devices are often also found outside of the input and hwmon driver directories. There are difference on between input/hwmon and extcon. Because input and hwmon driver implement the only one type driver as provider driver. But, extcon implement the two type driver of both extcon provider and extcon client driver. The extcon is similiar with regulator and clock framework as resource. extcon provider driver to provider the event when the state of external connector is changed. - devm_extcon_dev_register() - e.g., almost extcon provider driver are included in 'drivers/extcon/' directory. I understand, but that does not change my first argument, that the usb-phy is not a MFD device. And although it may be desirable to keep extcon provider drivers in the drivers/extcon, there are no technical reasons to do so. The whole reason why Kishon asked me to start using the extcon framework is to avoid adding a private API to the phy-sun4i-usb code for notifying the musb-sunxi code about otg-id-pin status changes. Adding a separate driver for just the extcon bits means re-adding a private api to the phy-sun4i-usb code but this time for the extcon code, at which point we might just as well skip extcon and have the musb-sunxi glue code call directly into the phy-sun4i-usb code... Needing a private API for a separate extcn driver actually is a good argument to NOT have a separate extcon driver and keep the extcon code in the phy-sun4i-usb code, where as I see no technical arguments in favor of a separate extcon driver. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/2] phy-sun4i-usb: Add full support for usb0 phy / OTG
Hi,
On 11-06-15 11:42, Kishon Vijay Abraham I wrote:
Hi,
On Sunday 31 May 2015 09:40 PM, Hans de Goede wrote:
The usb0 phy is connected to an OTG controller, and as such needs some special
handling:
1) It allows explicit control over the pullups, enable these on phy_init and
disable them on phy_exit.
2) It has bits to signal id and vbus detect to the musb-core, add support for
for monitoring id and vbus detect gpio-s for use in dual role mode, and set
these bits to the correct values for operating in host only mode when no
gpios are specified in the devicetree.
3) When in dual role mode the musb sunxi glue needs to know if the a host or
device cable is plugged in, so when in dual role mode register an extcon.
While updating the devicetree binding documentation also add documentation
for the sofar undocumented usage of regulators for vbus for all 3 phys.
Signed-off-by: Hans de Goede hdego...@redhat.com
---
Changes in v2:
-Removed the sunxi specific phy functions, instead the id / vbus gpio polling
has been moved to the phy-sun4i-usb driver and their status is exported
through extcon for the sunxi-musb glue
Changes in v3:
-No changes
Changes in v4:
-Do not call regulator_disable in an unbalanced manner when an external vbus
is present
---
.../devicetree/bindings/phy/sun4i-usb-phy.txt | 18 +-
drivers/phy/Kconfig| 1 +
drivers/phy/phy-sun4i-usb.c| 273 -
3 files changed, 281 insertions(+), 11 deletions(-)
.
.
snip
.
.
diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index 91c5be4..b45d707 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -1,7 +1,7 @@
.
.
snip
.
.
static struct phy *sun4i_usb_phy_xlate(struct device *dev,
struct of_phandle_args *args)
{
@@ -240,13 +417,20 @@ static int sun4i_usb_phy_probe(struct platform_device
*pdev)
struct phy_provider *phy_provider;
bool dedicated_clocks;
struct resource *res;
-int i;
+int i, ret;
data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
if (!data)
return -ENOMEM;
mutex_init(data-mutex);
+INIT_DELAYED_WORK(data-detect, sun4i_usb_phy0_id_vbus_det_scan);
+data-extcon_cable_names[0] = extcon_cable_name[EXTCON_USB_HOST];
+data-extcon_cable_names[1] = extcon_cable_name[EXTCON_USB];
+data-extcon_cable_names[2] = NULL;
+data-extcon.name = DRIVER_NAME;
+data-extcon.supported_cable = data-extcon_cable_names;
+data-extcon.dev.parent = dev;
if (of_device_is_compatible(np, allwinner,sun5i-a13-usb-phy))
data-num_phys = 2;
@@ -269,6 +453,34 @@ static int sun4i_usb_phy_probe(struct platform_device
*pdev)
if (IS_ERR(data-base))
return PTR_ERR(data-base);
+data-id_det_gpio = devm_gpiod_get(dev, usb0_id_det, GPIOD_IN);
+if (IS_ERR(data-id_det_gpio)) {
+if (PTR_ERR(data-id_det_gpio) == -EPROBE_DEFER)
+return -EPROBE_DEFER;
+data-id_det_gpio = NULL;
+}
+
+data-vbus_det_gpio = devm_gpiod_get(dev, usb0_vbus_det, GPIOD_IN);
+if (IS_ERR(data-vbus_det_gpio)) {
+if (PTR_ERR(data-vbus_det_gpio) == -EPROBE_DEFER)
+return -EPROBE_DEFER;
+data-vbus_det_gpio = NULL;
+}
+
+/* We either want both gpio pins or neither (when in host mode) */
+if (!data-id_det_gpio != !data-vbus_det_gpio) {
+dev_err(dev, failed to get id or vbus detect pin\n);
+return -ENODEV;
+}
+
+if (data-id_det_gpio) {
+ret = devm_extcon_dev_register(dev, data-extcon);
+if (ret) {
+dev_err(dev, failed to register extcon: %d\n, ret);
+return ret;
+}
+}
+
for (i = 0; i data-num_phys; i++) {
struct sun4i_usb_phy *phy = data-phys + i;
char name[16];
@@ -318,12 +530,54 @@ static int sun4i_usb_phy_probe(struct platform_device
*pdev)
phy_set_drvdata(phy-phy, data-phys[i]);
}
+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_irq 0 || data-vbus_det_irq 0)
+data-phy0_poll = true;
if polling is enabled, we shouldn't enable irq at all?
Thanks for the review.
One some boards one of the gpio-s is irq capable and the other
is not, in which case the current code indeed enables both
irq handling for the one gpio which is irq capable and enables
polling. This is done this way deliberately as the irq path
has much better latency then polling and the 2 can co-exist.
+
+if (data-id_det_irq = 0) {
+ret = devm_request_irq(dev, data-id_det_irq,
+sun4i_usb_phy0_id_vbus_det_irq,
+IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+usb0-id-det, data);
+if (ret) {
+dev_err(dev, Err requesting id-det-irq: %d\n, ret);
+return ret;
+}
+}
+
+if
Re: [PATCH v4 1/2] phy-sun4i-usb: Add full support for usb0 phy / OTG
Hi,
On Thursday 11 June 2015 03:23 PM, Hans de Goede wrote:
Hi,
On 11-06-15 11:42, Kishon Vijay Abraham I wrote:
Hi,
On Sunday 31 May 2015 09:40 PM, Hans de Goede wrote:
The usb0 phy is connected to an OTG controller, and as such needs some special
handling:
1) It allows explicit control over the pullups, enable these on phy_init and
disable them on phy_exit.
2) It has bits to signal id and vbus detect to the musb-core, add support for
for monitoring id and vbus detect gpio-s for use in dual role mode, and set
these bits to the correct values for operating in host only mode when no
gpios are specified in the devicetree.
3) When in dual role mode the musb sunxi glue needs to know if the a host or
device cable is plugged in, so when in dual role mode register an extcon.
While updating the devicetree binding documentation also add documentation
for the sofar undocumented usage of regulators for vbus for all 3 phys.
Signed-off-by: Hans de Goede hdego...@redhat.com
---
Changes in v2:
-Removed the sunxi specific phy functions, instead the id / vbus gpio polling
has been moved to the phy-sun4i-usb driver and their status is exported
through extcon for the sunxi-musb glue
Changes in v3:
-No changes
Changes in v4:
-Do not call regulator_disable in an unbalanced manner when an external vbus
is present
---
.../devicetree/bindings/phy/sun4i-usb-phy.txt | 18 +-
drivers/phy/Kconfig| 1 +
drivers/phy/phy-sun4i-usb.c| 273
-
3 files changed, 281 insertions(+), 11 deletions(-)
.
.
snip
.
.
diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index 91c5be4..b45d707 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -1,7 +1,7 @@
.
.
snip
.
.
static struct phy *sun4i_usb_phy_xlate(struct device *dev,
struct of_phandle_args *args)
{
@@ -240,13 +417,20 @@ static int sun4i_usb_phy_probe(struct platform_device
*pdev)
struct phy_provider *phy_provider;
bool dedicated_clocks;
struct resource *res;
-int i;
+int i, ret;
data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
if (!data)
return -ENOMEM;
mutex_init(data-mutex);
+INIT_DELAYED_WORK(data-detect, sun4i_usb_phy0_id_vbus_det_scan);
+data-extcon_cable_names[0] = extcon_cable_name[EXTCON_USB_HOST];
+data-extcon_cable_names[1] = extcon_cable_name[EXTCON_USB];
+data-extcon_cable_names[2] = NULL;
+data-extcon.name = DRIVER_NAME;
+data-extcon.supported_cable = data-extcon_cable_names;
+data-extcon.dev.parent = dev;
if (of_device_is_compatible(np, allwinner,sun5i-a13-usb-phy))
data-num_phys = 2;
@@ -269,6 +453,34 @@ static int sun4i_usb_phy_probe(struct platform_device
*pdev)
if (IS_ERR(data-base))
return PTR_ERR(data-base);
+data-id_det_gpio = devm_gpiod_get(dev, usb0_id_det, GPIOD_IN);
+if (IS_ERR(data-id_det_gpio)) {
+if (PTR_ERR(data-id_det_gpio) == -EPROBE_DEFER)
+return -EPROBE_DEFER;
+data-id_det_gpio = NULL;
+}
+
+data-vbus_det_gpio = devm_gpiod_get(dev, usb0_vbus_det, GPIOD_IN);
+if (IS_ERR(data-vbus_det_gpio)) {
+if (PTR_ERR(data-vbus_det_gpio) == -EPROBE_DEFER)
+return -EPROBE_DEFER;
+data-vbus_det_gpio = NULL;
+}
+
+/* We either want both gpio pins or neither (when in host mode) */
+if (!data-id_det_gpio != !data-vbus_det_gpio) {
+dev_err(dev, failed to get id or vbus detect pin\n);
+return -ENODEV;
+}
+
+if (data-id_det_gpio) {
+ret = devm_extcon_dev_register(dev, data-extcon);
+if (ret) {
+dev_err(dev, failed to register extcon: %d\n, ret);
+return ret;
+}
+}
+
for (i = 0; i data-num_phys; i++) {
struct sun4i_usb_phy *phy = data-phys + i;
char name[16];
@@ -318,12 +530,54 @@ static int sun4i_usb_phy_probe(struct platform_device
*pdev)
phy_set_drvdata(phy-phy, data-phys[i]);
}
+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_irq 0 || data-vbus_det_irq 0)
+data-phy0_poll = true;
if polling is enabled, we shouldn't enable irq at all?
Thanks for the review.
One some boards one of the gpio-s is irq capable and the other
is not, in which case the current code indeed enables both
irq handling for the one gpio which is irq capable and enables
polling. This is done this way deliberately as the irq path
has much better latency then polling and the 2 can co-exist.
okay. Would be good to have this as a comment.
+
+if (data-id_det_irq = 0) {
+ret = devm_request_irq(dev, data-id_det_irq,
+sun4i_usb_phy0_id_vbus_det_irq,
+IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+usb0-id-det, data);
+
Re: [linux-sunxi] Re: [PATCH v4 1/2] phy-sun4i-usb: Add full support for usb0 phy / OTG
On Thursday 11 June 2015 06:05 PM, Hans de Goede wrote:
Hi,
On 11-06-15 13:16, Kishon Vijay Abraham I wrote:
Hi,
On Thursday 11 June 2015 03:23 PM, Hans de Goede wrote:
Hi,
On 11-06-15 11:42, Kishon Vijay Abraham I wrote:
Hi,
On Sunday 31 May 2015 09:40 PM, Hans de Goede wrote:
The usb0 phy is connected to an OTG controller, and as such needs some special
handling:
1) It allows explicit control over the pullups, enable these on phy_init and
disable them on phy_exit.
2) It has bits to signal id and vbus detect to the musb-core, add support for
for monitoring id and vbus detect gpio-s for use in dual role mode, and set
these bits to the correct values for operating in host only mode when no
gpios are specified in the devicetree.
3) When in dual role mode the musb sunxi glue needs to know if the a host or
device cable is plugged in, so when in dual role mode register an extcon.
While updating the devicetree binding documentation also add documentation
for the sofar undocumented usage of regulators for vbus for all 3 phys.
Signed-off-by: Hans de Goede hdego...@redhat.com
---
Changes in v2:
-Removed the sunxi specific phy functions, instead the id / vbus gpio polling
has been moved to the phy-sun4i-usb driver and their status is exported
through extcon for the sunxi-musb glue
Changes in v3:
-No changes
Changes in v4:
-Do not call regulator_disable in an unbalanced manner when an external vbus
is present
---
.../devicetree/bindings/phy/sun4i-usb-phy.txt | 18 +-
drivers/phy/Kconfig| 1 +
drivers/phy/phy-sun4i-usb.c| 273
-
3 files changed, 281 insertions(+), 11 deletions(-)
.
.
snip
.
.
diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index 91c5be4..b45d707 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -1,7 +1,7 @@
.
.
snip
.
.
static struct phy *sun4i_usb_phy_xlate(struct device *dev,
struct of_phandle_args *args)
{
@@ -240,13 +417,20 @@ static int sun4i_usb_phy_probe(struct platform_device
*pdev)
struct phy_provider *phy_provider;
bool dedicated_clocks;
struct resource *res;
-int i;
+int i, ret;
data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
if (!data)
return -ENOMEM;
mutex_init(data-mutex);
+INIT_DELAYED_WORK(data-detect, sun4i_usb_phy0_id_vbus_det_scan);
+data-extcon_cable_names[0] = extcon_cable_name[EXTCON_USB_HOST];
+data-extcon_cable_names[1] = extcon_cable_name[EXTCON_USB];
+data-extcon_cable_names[2] = NULL;
+data-extcon.name = DRIVER_NAME;
+data-extcon.supported_cable = data-extcon_cable_names;
+data-extcon.dev.parent = dev;
if (of_device_is_compatible(np, allwinner,sun5i-a13-usb-phy))
data-num_phys = 2;
@@ -269,6 +453,34 @@ static int sun4i_usb_phy_probe(struct platform_device
*pdev)
if (IS_ERR(data-base))
return PTR_ERR(data-base);
+data-id_det_gpio = devm_gpiod_get(dev, usb0_id_det, GPIOD_IN);
+if (IS_ERR(data-id_det_gpio)) {
+if (PTR_ERR(data-id_det_gpio) == -EPROBE_DEFER)
+return -EPROBE_DEFER;
+data-id_det_gpio = NULL;
+}
+
+data-vbus_det_gpio = devm_gpiod_get(dev, usb0_vbus_det, GPIOD_IN);
+if (IS_ERR(data-vbus_det_gpio)) {
+if (PTR_ERR(data-vbus_det_gpio) == -EPROBE_DEFER)
+return -EPROBE_DEFER;
+data-vbus_det_gpio = NULL;
+}
+
+/* We either want both gpio pins or neither (when in host mode) */
+if (!data-id_det_gpio != !data-vbus_det_gpio) {
+dev_err(dev, failed to get id or vbus detect pin\n);
+return -ENODEV;
+}
+
+if (data-id_det_gpio) {
+ret = devm_extcon_dev_register(dev, data-extcon);
+if (ret) {
+dev_err(dev, failed to register extcon: %d\n, ret);
+return ret;
+}
+}
+
for (i = 0; i data-num_phys; i++) {
struct sun4i_usb_phy *phy = data-phys + i;
char name[16];
@@ -318,12 +530,54 @@ static int sun4i_usb_phy_probe(struct platform_device
*pdev)
phy_set_drvdata(phy-phy, data-phys[i]);
}
+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_irq 0 || data-vbus_det_irq 0)
+data-phy0_poll = true;
if polling is enabled, we shouldn't enable irq at all?
Thanks for the review.
One some boards one of the gpio-s is irq capable and the other
is not, in which case the current code indeed enables both
irq handling for the one gpio which is irq capable and enables
polling. This is done this way deliberately as the irq path
has much better latency then polling and the 2 can co-exist.
okay. Would be good to have this as a comment.
+
+if (data-id_det_irq = 0) {
+ret = devm_request_irq(dev, data-id_det_irq,
+
Re: [linux-sunxi] Re: [PATCH v4 1/2] phy-sun4i-usb: Add full support for usb0 phy / OTG
Hi,
On 11-06-15 13:16, Kishon Vijay Abraham I wrote:
Hi,
On Thursday 11 June 2015 03:23 PM, Hans de Goede wrote:
Hi,
On 11-06-15 11:42, Kishon Vijay Abraham I wrote:
Hi,
On Sunday 31 May 2015 09:40 PM, Hans de Goede wrote:
The usb0 phy is connected to an OTG controller, and as such needs some special
handling:
1) It allows explicit control over the pullups, enable these on phy_init and
disable them on phy_exit.
2) It has bits to signal id and vbus detect to the musb-core, add support for
for monitoring id and vbus detect gpio-s for use in dual role mode, and set
these bits to the correct values for operating in host only mode when no
gpios are specified in the devicetree.
3) When in dual role mode the musb sunxi glue needs to know if the a host or
device cable is plugged in, so when in dual role mode register an extcon.
While updating the devicetree binding documentation also add documentation
for the sofar undocumented usage of regulators for vbus for all 3 phys.
Signed-off-by: Hans de Goede hdego...@redhat.com
---
Changes in v2:
-Removed the sunxi specific phy functions, instead the id / vbus gpio polling
has been moved to the phy-sun4i-usb driver and their status is exported
through extcon for the sunxi-musb glue
Changes in v3:
-No changes
Changes in v4:
-Do not call regulator_disable in an unbalanced manner when an external vbus
is present
---
.../devicetree/bindings/phy/sun4i-usb-phy.txt | 18 +-
drivers/phy/Kconfig| 1 +
drivers/phy/phy-sun4i-usb.c| 273
-
3 files changed, 281 insertions(+), 11 deletions(-)
.
.
snip
.
.
diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index 91c5be4..b45d707 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -1,7 +1,7 @@
.
.
snip
.
.
static struct phy *sun4i_usb_phy_xlate(struct device *dev,
struct of_phandle_args *args)
{
@@ -240,13 +417,20 @@ static int sun4i_usb_phy_probe(struct platform_device
*pdev)
struct phy_provider *phy_provider;
bool dedicated_clocks;
struct resource *res;
-int i;
+int i, ret;
data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
if (!data)
return -ENOMEM;
mutex_init(data-mutex);
+INIT_DELAYED_WORK(data-detect, sun4i_usb_phy0_id_vbus_det_scan);
+data-extcon_cable_names[0] = extcon_cable_name[EXTCON_USB_HOST];
+data-extcon_cable_names[1] = extcon_cable_name[EXTCON_USB];
+data-extcon_cable_names[2] = NULL;
+data-extcon.name = DRIVER_NAME;
+data-extcon.supported_cable = data-extcon_cable_names;
+data-extcon.dev.parent = dev;
if (of_device_is_compatible(np, allwinner,sun5i-a13-usb-phy))
data-num_phys = 2;
@@ -269,6 +453,34 @@ static int sun4i_usb_phy_probe(struct platform_device
*pdev)
if (IS_ERR(data-base))
return PTR_ERR(data-base);
+data-id_det_gpio = devm_gpiod_get(dev, usb0_id_det, GPIOD_IN);
+if (IS_ERR(data-id_det_gpio)) {
+if (PTR_ERR(data-id_det_gpio) == -EPROBE_DEFER)
+return -EPROBE_DEFER;
+data-id_det_gpio = NULL;
+}
+
+data-vbus_det_gpio = devm_gpiod_get(dev, usb0_vbus_det, GPIOD_IN);
+if (IS_ERR(data-vbus_det_gpio)) {
+if (PTR_ERR(data-vbus_det_gpio) == -EPROBE_DEFER)
+return -EPROBE_DEFER;
+data-vbus_det_gpio = NULL;
+}
+
+/* We either want both gpio pins or neither (when in host mode) */
+if (!data-id_det_gpio != !data-vbus_det_gpio) {
+dev_err(dev, failed to get id or vbus detect pin\n);
+return -ENODEV;
+}
+
+if (data-id_det_gpio) {
+ret = devm_extcon_dev_register(dev, data-extcon);
+if (ret) {
+dev_err(dev, failed to register extcon: %d\n, ret);
+return ret;
+}
+}
+
for (i = 0; i data-num_phys; i++) {
struct sun4i_usb_phy *phy = data-phys + i;
char name[16];
@@ -318,12 +530,54 @@ static int sun4i_usb_phy_probe(struct platform_device
*pdev)
phy_set_drvdata(phy-phy, data-phys[i]);
}
+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_irq 0 || data-vbus_det_irq 0)
+data-phy0_poll = true;
if polling is enabled, we shouldn't enable irq at all?
Thanks for the review.
One some boards one of the gpio-s is irq capable and the other
is not, in which case the current code indeed enables both
irq handling for the one gpio which is irq capable and enables
polling. This is done this way deliberately as the irq path
has much better latency then polling and the 2 can co-exist.
okay. Would be good to have this as a comment.
+
+if (data-id_det_irq = 0) {
+ret = devm_request_irq(dev, data-id_det_irq,
+sun4i_usb_phy0_id_vbus_det_irq,
+IRQF_TRIGGER_RISING |
Re: [linux-sunxi] Re: [PATCH v4 1/2] phy-sun4i-usb: Add full support for usb0 phy / OTG
Hi,
On 11-06-15 07:48, Kishon Vijay Abraham I wrote:
+Chanwoo
Hi,
On Sunday 31 May 2015 09:40 PM, Hans de Goede wrote:
The usb0 phy is connected to an OTG controller, and as such needs some special
handling:
1) It allows explicit control over the pullups, enable these on phy_init and
disable them on phy_exit.
2) It has bits to signal id and vbus detect to the musb-core, add support for
for monitoring id and vbus detect gpio-s for use in dual role mode, and set
these bits to the correct values for operating in host only mode when no
gpios are specified in the devicetree.
3) When in dual role mode the musb sunxi glue needs to know if the a host or
device cable is plugged in, so when in dual role mode register an extcon.
While updating the devicetree binding documentation also add documentation
for the sofar undocumented usage of regulators for vbus for all 3 phys.
Signed-off-by: Hans de Goede hdego...@redhat.com
---
Changes in v2:
-Removed the sunxi specific phy functions, instead the id / vbus gpio polling
has been moved to the phy-sun4i-usb driver and their status is exported
through extcon for the sunxi-musb glue
Changes in v3:
-No changes
Changes in v4:
-Do not call regulator_disable in an unbalanced manner when an external vbus
is present
---
.../devicetree/bindings/phy/sun4i-usb-phy.txt | 18 +-
drivers/phy/Kconfig| 1 +
drivers/phy/phy-sun4i-usb.c| 273 -
3 files changed, 281 insertions(+), 11 deletions(-)
diff --git a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
index 16528b9..557fa99 100644
--- a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
+++ b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
@@ -23,6 +23,13 @@ Required properties:
* usb1_reset
* usb2_reset for sun4i, sun6i or sun7i
+Optional properties:
+- usb0_id_det-gpios : gpio phandle for reading the otg id pin value
+- usb0_vbus_det-gpios : gpio phandle for detecting the presence of usb0 vbus
+- usb0_vbus-supply : regulator phandle for controller usb0 vbus
+- usb1_vbus-supply : regulator phandle for controller usb1 vbus
+- usb2_vbus-supply : regulator phandle for controller usb2 vbus
+
Example:
usbphy: phy@0x01c13400 {
#phy-cells = 1;
@@ -32,6 +39,13 @@ Example:
reg-names = phy_ctrl, pmu1, pmu2;
clocks = usb_clk 8;
clock-names = usb_phy;
-resets = usb_clk 1, usb_clk 2;
-reset-names = usb1_reset, usb2_reset;
+resets = usb_clk 0, usb_clk 1, usb_clk 2;
+reset-names = usb0_reset, usb1_reset, usb2_reset;
+pinctrl-names = default;
+pinctrl-0 = usb0_id_detect_pin, usb0_vbus_detect_pin;
+usb0_id_det-gpios = pio 7 19 GPIO_ACTIVE_HIGH; /* PH19 */
+usb0_vbus_det-gpios = pio 7 22 GPIO_ACTIVE_HIGH; /* PH22 */
+usb0_vbus-supply = reg_usb0_vbus;
+usb1_vbus-supply = reg_usb1_vbus;
+usb2_vbus-supply = reg_usb2_vbus;
};
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index a53bd5b..4614fba 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -173,6 +173,7 @@ config PHY_SUN4I_USB
tristate Allwinner sunxi SoC USB PHY driver
depends on ARCH_SUNXI HAS_IOMEM OF
depends on RESET_CONTROLLER
+select EXTCON
Avoid using 'select' on visible Kconfig symbols.
Ok, I'll do a v5 changing this to a depends on.
Also please split the patch to make the reviewing a bit easier.
There really is not much which can be split out, the iscr register
updating is only done from the id / vbus det scanning, so the 2 are
intertwined with each other.
Also I must say I'm unhappy with the fact that you request this now,
after this patch has been posted in its current form a long time
ago already. The changes in v3 and v4 were very minimal, the
patch has existed in this for since v2, so you've had a long long
time to request changes like this already.
In the mean time I've build a whole set of related changes (enabling
the use of otg on newer versions of the sunxi soc), so sorry I'm
not going to split this up now as that is going to be a big pain
with all the extra patches which are sitting on top, and as said
there really is not that much to split in the first place.
I'll send out a v5 of this patch together with a resend of all the
patches adding support for new models which sit on top of this one
(and which I've already send before).
Regards,
Hans
Thanks
Kishon
select GENERIC_PHY
help
Enable this to support the transceiver that is part of Allwinner
diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index 91c5be4..b45d707 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -1,7 +1,7 @@
/*
* Allwinner sun4i USB phy driver
*
- * Copyright (C) 2014 Hans de Goede hdego...@redhat.com
+ * Copyright (C) 2014-2015 Hans de Goede
Re: [PATCH v4 1/2] phy-sun4i-usb: Add full support for usb0 phy / OTG
Hi Hans,
I add the comment about extcon-related code.
Firstly,
I'd like you to implment the extcon driver for phy-sun4i-usb device
in drivers/extcon/ directoryby using MFD because there are both extcon
provider driver and extcon client driver. I think that all extcon
provider driver better to be included in drivers/extcon/ directory.
extcon_set_cable_state() function should be handled in extcon provider
driver which is incluced in drivers/extcon/ directory.
On 06/11/2015 02:48 PM, Kishon Vijay Abraham I wrote:
+Chanwoo
Hi,
On Sunday 31 May 2015 09:40 PM, Hans de Goede wrote:
The usb0 phy is connected to an OTG controller, and as such needs some
special
handling:
1) It allows explicit control over the pullups, enable these on phy_init and
disable them on phy_exit.
2) It has bits to signal id and vbus detect to the musb-core, add support for
for monitoring id and vbus detect gpio-s for use in dual role mode, and set
these bits to the correct values for operating in host only mode when no
gpios are specified in the devicetree.
3) When in dual role mode the musb sunxi glue needs to know if the a host or
device cable is plugged in, so when in dual role mode register an extcon.
While updating the devicetree binding documentation also add documentation
for the sofar undocumented usage of regulators for vbus for all 3 phys.
Signed-off-by: Hans de Goede hdego...@redhat.com
---
Changes in v2:
-Removed the sunxi specific phy functions, instead the id / vbus gpio polling
has been moved to the phy-sun4i-usb driver and their status is exported
through extcon for the sunxi-musb glue
Changes in v3:
-No changes
Changes in v4:
-Do not call regulator_disable in an unbalanced manner when an external vbus
is present
---
.../devicetree/bindings/phy/sun4i-usb-phy.txt | 18 +-
drivers/phy/Kconfig| 1 +
drivers/phy/phy-sun4i-usb.c| 273
-
3 files changed, 281 insertions(+), 11 deletions(-)
diff --git a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
index 16528b9..557fa99 100644
--- a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
+++ b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
@@ -23,6 +23,13 @@ Required properties:
* usb1_reset
* usb2_reset for sun4i, sun6i or sun7i
+Optional properties:
+- usb0_id_det-gpios : gpio phandle for reading the otg id pin value
+- usb0_vbus_det-gpios : gpio phandle for detecting the presence of usb0 vbus
+- usb0_vbus-supply : regulator phandle for controller usb0 vbus
+- usb1_vbus-supply : regulator phandle for controller usb1 vbus
+- usb2_vbus-supply : regulator phandle for controller usb2 vbus
+
Example:
usbphy: phy@0x01c13400 {
#phy-cells = 1;
@@ -32,6 +39,13 @@ Example:
reg-names = phy_ctrl, pmu1, pmu2;
clocks = usb_clk 8;
clock-names = usb_phy;
-resets = usb_clk 1, usb_clk 2;
-reset-names = usb1_reset, usb2_reset;
+resets = usb_clk 0, usb_clk 1, usb_clk 2;
+reset-names = usb0_reset, usb1_reset, usb2_reset;
+pinctrl-names = default;
+pinctrl-0 = usb0_id_detect_pin, usb0_vbus_detect_pin;
+usb0_id_det-gpios = pio 7 19 GPIO_ACTIVE_HIGH; /* PH19 */
+usb0_vbus_det-gpios = pio 7 22 GPIO_ACTIVE_HIGH; /* PH22 */
+usb0_vbus-supply = reg_usb0_vbus;
+usb1_vbus-supply = reg_usb1_vbus;
+usb2_vbus-supply = reg_usb2_vbus;
};
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index a53bd5b..4614fba 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -173,6 +173,7 @@ config PHY_SUN4I_USB
tristate Allwinner sunxi SoC USB PHY driver
depends on ARCH_SUNXI HAS_IOMEM OF
depends on RESET_CONTROLLER
+select EXTCON
Avoid using 'select' on visible Kconfig symbols.
Also please split the patch to make the reviewing a bit easier.
Thanks
Kishon
select GENERIC_PHY
help
Enable this to support the transceiver that is part of Allwinner
diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index 91c5be4..b45d707 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -1,7 +1,7 @@
/*
* Allwinner sun4i USB phy driver
*
- * Copyright (C) 2014 Hans de Goede hdego...@redhat.com
+ * Copyright (C) 2014-2015 Hans de Goede hdego...@redhat.com
*
* Based on code from
* Allwinner Technology Co., Ltd. www.allwinnertech.com
@@ -23,17 +23,23 @@
#include linux/clk.h
#include linux/err.h
+#include linux/extcon.h
#include linux/io.h
+#include linux/interrupt.h
#include linux/kernel.h
#include linux/module.h
#include linux/mutex.h
#include linux/of.h
#include linux/of_address.h
+#include linux/of_gpio.h
#include linux/phy/phy.h
#include
Re: [linux-sunxi] Re: [PATCH v4 1/2] phy-sun4i-usb: Add full support for usb0 phy / OTG
Hi Chanwoo,
Thanks for the quick review.
On 11-06-15 10:07, Chanwoo Choi wrote:
Hi Hans,
I add the comment about extcon-related code.
Firstly,
I'd like you to implment the extcon driver for phy-sun4i-usb device
in drivers/extcon/ directoryby using MFD
No, just no, this is not what the MFD framework is for, the usb-phy
in question here is not a multifunction device. The MFD framework
is intended for true multi-function devices like i2c attached
PMICs which have regulators, gpios, pwm, input (power button),
chargers, power-supply, etc. That is NOT the case here.
Also moving this to the MFD framework would very likely requiring
the devicetree binding for the usb-phy to change which we cannot
do as that would break the devicetree ABI.
because there are both extcon
provider driver and extcon client driver. I think that all extcon
provider driver better to be included in drivers/extcon/ directory.
extcon_set_cable_state() function should be handled in extcon provider
driver which is incluced in drivers/extcon/ directory.
I do not find this a compelling reason, there are plenty of subsystems
where not all implementations of the subsystem class live in the subsystem
directory, e.g. input and hwmon devices are often also found outside of
the input and hwmon driver directories.
On 06/11/2015 02:48 PM, Kishon Vijay Abraham I wrote:
+Chanwoo
Hi,
On Sunday 31 May 2015 09:40 PM, Hans de Goede wrote:
The usb0 phy is connected to an OTG controller, and as such needs some special
handling:
1) It allows explicit control over the pullups, enable these on phy_init and
disable them on phy_exit.
2) It has bits to signal id and vbus detect to the musb-core, add support for
for monitoring id and vbus detect gpio-s for use in dual role mode, and set
these bits to the correct values for operating in host only mode when no
gpios are specified in the devicetree.
3) When in dual role mode the musb sunxi glue needs to know if the a host or
device cable is plugged in, so when in dual role mode register an extcon.
While updating the devicetree binding documentation also add documentation
for the sofar undocumented usage of regulators for vbus for all 3 phys.
Signed-off-by: Hans de Goede hdego...@redhat.com
---
Changes in v2:
-Removed the sunxi specific phy functions, instead the id / vbus gpio polling
has been moved to the phy-sun4i-usb driver and their status is exported
through extcon for the sunxi-musb glue
Changes in v3:
-No changes
Changes in v4:
-Do not call regulator_disable in an unbalanced manner when an external vbus
is present
---
.../devicetree/bindings/phy/sun4i-usb-phy.txt | 18 +-
drivers/phy/Kconfig| 1 +
drivers/phy/phy-sun4i-usb.c| 273
-
3 files changed, 281 insertions(+), 11 deletions(-)
diff --git a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
index 16528b9..557fa99 100644
--- a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
+++ b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
@@ -23,6 +23,13 @@ Required properties:
* usb1_reset
* usb2_reset for sun4i, sun6i or sun7i
+Optional properties:
+- usb0_id_det-gpios : gpio phandle for reading the otg id pin value
+- usb0_vbus_det-gpios : gpio phandle for detecting the presence of usb0 vbus
+- usb0_vbus-supply : regulator phandle for controller usb0 vbus
+- usb1_vbus-supply : regulator phandle for controller usb1 vbus
+- usb2_vbus-supply : regulator phandle for controller usb2 vbus
+
Example:
usbphy: phy@0x01c13400 {
#phy-cells = 1;
@@ -32,6 +39,13 @@ Example:
reg-names = phy_ctrl, pmu1, pmu2;
clocks = usb_clk 8;
clock-names = usb_phy;
-resets = usb_clk 1, usb_clk 2;
-reset-names = usb1_reset, usb2_reset;
+resets = usb_clk 0, usb_clk 1, usb_clk 2;
+reset-names = usb0_reset, usb1_reset, usb2_reset;
+pinctrl-names = default;
+pinctrl-0 = usb0_id_detect_pin, usb0_vbus_detect_pin;
+usb0_id_det-gpios = pio 7 19 GPIO_ACTIVE_HIGH; /* PH19 */
+usb0_vbus_det-gpios = pio 7 22 GPIO_ACTIVE_HIGH; /* PH22 */
+usb0_vbus-supply = reg_usb0_vbus;
+usb1_vbus-supply = reg_usb1_vbus;
+usb2_vbus-supply = reg_usb2_vbus;
};
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index a53bd5b..4614fba 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -173,6 +173,7 @@ config PHY_SUN4I_USB
tristate Allwinner sunxi SoC USB PHY driver
depends on ARCH_SUNXI HAS_IOMEM OF
depends on RESET_CONTROLLER
+select EXTCON
Avoid using 'select' on visible Kconfig symbols.
Also please split the patch to make the reviewing a bit easier.
Thanks
Kishon
select GENERIC_PHY
help
Enable this to support the transceiver that is part of Allwinner
diff --git a/drivers/phy/phy-sun4i-usb.c
Re: [linux-sunxi] Re: [PATCH v4 1/2] phy-sun4i-usb: Add full support for usb0 phy / OTG
Hi Hans,
On 06/11/2015 05:21 PM, Hans de Goede wrote:
Hi Chanwoo,
Thanks for the quick review.
On 11-06-15 10:07, Chanwoo Choi wrote:
Hi Hans,
I add the comment about extcon-related code.
Firstly,
I'd like you to implment the extcon driver for phy-sun4i-usb device
in drivers/extcon/ directoryby using MFD
No, just no, this is not what the MFD framework is for, the usb-phy
in question here is not a multifunction device. The MFD framework
is intended for true multi-function devices like i2c attached
PMICs which have regulators, gpios, pwm, input (power button),
chargers, power-supply, etc. That is NOT the case here.
Also moving this to the MFD framework would very likely requiring
the devicetree binding for the usb-phy to change which we cannot
do as that would break the devicetree ABI.
because there are both extcon
provider driver and extcon client driver. I think that all extcon
provider driver better to be included in drivers/extcon/ directory.
extcon_set_cable_state() function should be handled in extcon provider
driver which is incluced in drivers/extcon/ directory.
I do not find this a compelling reason, there are plenty of subsystems
where not all implementations of the subsystem class live in the subsystem
directory, e.g. input and hwmon devices are often also found outside of
the input and hwmon driver directories.
There are difference on between input/hwmon and extcon.
Because input and hwmon driver implement the only one type driver as provider
driver.
But, extcon implement the two type driver of both extcon provider and extcon
client driver.
The extcon is similiar with regulator and clock framework as resource.
extcon provider driver to provider the event when the state of external
connector is changed.
- devm_extcon_dev_register()
- e.g., almost extcon provider driver are included in 'drivers/extcon/'
directory.
extcon client driver to receive the event when the state of external connector
is changed.
- extcon_register_notifier() or extcon_register_interest()(deprecated API)
- e.g., drivers/power/charger-manager.c, drivers/usb/dwc3/dwc3-omap.c etc.
Thanks,
Chanwoo Choi
On 06/11/2015 02:48 PM, Kishon Vijay Abraham I wrote:
+Chanwoo
Hi,
On Sunday 31 May 2015 09:40 PM, Hans de Goede wrote:
The usb0 phy is connected to an OTG controller, and as such needs some
special
handling:
1) It allows explicit control over the pullups, enable these on phy_init
and
disable them on phy_exit.
2) It has bits to signal id and vbus detect to the musb-core, add support
for
for monitoring id and vbus detect gpio-s for use in dual role mode, and set
these bits to the correct values for operating in host only mode when no
gpios are specified in the devicetree.
3) When in dual role mode the musb sunxi glue needs to know if the a host
or
device cable is plugged in, so when in dual role mode register an extcon.
While updating the devicetree binding documentation also add documentation
for the sofar undocumented usage of regulators for vbus for all 3 phys.
Signed-off-by: Hans de Goede hdego...@redhat.com
---
Changes in v2:
-Removed the sunxi specific phy functions, instead the id / vbus gpio
polling
has been moved to the phy-sun4i-usb driver and their status is exported
through extcon for the sunxi-musb glue
Changes in v3:
-No changes
Changes in v4:
-Do not call regulator_disable in an unbalanced manner when an external
vbus
is present
---
.../devicetree/bindings/phy/sun4i-usb-phy.txt | 18 +-
drivers/phy/Kconfig| 1 +
drivers/phy/phy-sun4i-usb.c| 273
-
3 files changed, 281 insertions(+), 11 deletions(-)
diff --git a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
index 16528b9..557fa99 100644
--- a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
+++ b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
@@ -23,6 +23,13 @@ Required properties:
* usb1_reset
* usb2_reset for sun4i, sun6i or sun7i
+Optional properties:
+- usb0_id_det-gpios : gpio phandle for reading the otg id pin value
+- usb0_vbus_det-gpios : gpio phandle for detecting the presence of usb0
vbus
+- usb0_vbus-supply : regulator phandle for controller usb0 vbus
+- usb1_vbus-supply : regulator phandle for controller usb1 vbus
+- usb2_vbus-supply : regulator phandle for controller usb2 vbus
+
Example:
usbphy: phy@0x01c13400 {
#phy-cells = 1;
@@ -32,6 +39,13 @@ Example:
reg-names = phy_ctrl, pmu1, pmu2;
clocks = usb_clk 8;
clock-names = usb_phy;
-resets = usb_clk 1, usb_clk 2;
-reset-names = usb1_reset, usb2_reset;
+resets = usb_clk 0, usb_clk 1, usb_clk 2;
+reset-names = usb0_reset, usb1_reset, usb2_reset;
+pinctrl-names = default;
+
Re: [PATCH v4 1/2] phy-sun4i-usb: Add full support for usb0 phy / OTG
Hi,
On 11-06-15 09:56, Hans de Goede wrote:
Hi,
On 11-06-15 07:48, Kishon Vijay Abraham I wrote:
+Chanwoo
Hi,
On Sunday 31 May 2015 09:40 PM, Hans de Goede wrote:
The usb0 phy is connected to an OTG controller, and as such needs some special
handling:
1) It allows explicit control over the pullups, enable these on phy_init and
disable them on phy_exit.
2) It has bits to signal id and vbus detect to the musb-core, add support for
for monitoring id and vbus detect gpio-s for use in dual role mode, and set
these bits to the correct values for operating in host only mode when no
gpios are specified in the devicetree.
3) When in dual role mode the musb sunxi glue needs to know if the a host or
device cable is plugged in, so when in dual role mode register an extcon.
While updating the devicetree binding documentation also add documentation
for the sofar undocumented usage of regulators for vbus for all 3 phys.
Signed-off-by: Hans de Goede hdego...@redhat.com
---
Changes in v2:
-Removed the sunxi specific phy functions, instead the id / vbus gpio polling
has been moved to the phy-sun4i-usb driver and their status is exported
through extcon for the sunxi-musb glue
Changes in v3:
-No changes
Changes in v4:
-Do not call regulator_disable in an unbalanced manner when an external vbus
is present
---
.../devicetree/bindings/phy/sun4i-usb-phy.txt | 18 +-
drivers/phy/Kconfig| 1 +
drivers/phy/phy-sun4i-usb.c| 273 -
3 files changed, 281 insertions(+), 11 deletions(-)
diff --git a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
index 16528b9..557fa99 100644
--- a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
+++ b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
@@ -23,6 +23,13 @@ Required properties:
* usb1_reset
* usb2_reset for sun4i, sun6i or sun7i
+Optional properties:
+- usb0_id_det-gpios : gpio phandle for reading the otg id pin value
+- usb0_vbus_det-gpios : gpio phandle for detecting the presence of usb0 vbus
+- usb0_vbus-supply : regulator phandle for controller usb0 vbus
+- usb1_vbus-supply : regulator phandle for controller usb1 vbus
+- usb2_vbus-supply : regulator phandle for controller usb2 vbus
+
Example:
usbphy: phy@0x01c13400 {
#phy-cells = 1;
@@ -32,6 +39,13 @@ Example:
reg-names = phy_ctrl, pmu1, pmu2;
clocks = usb_clk 8;
clock-names = usb_phy;
-resets = usb_clk 1, usb_clk 2;
-reset-names = usb1_reset, usb2_reset;
+resets = usb_clk 0, usb_clk 1, usb_clk 2;
+reset-names = usb0_reset, usb1_reset, usb2_reset;
+pinctrl-names = default;
+pinctrl-0 = usb0_id_detect_pin, usb0_vbus_detect_pin;
+usb0_id_det-gpios = pio 7 19 GPIO_ACTIVE_HIGH; /* PH19 */
+usb0_vbus_det-gpios = pio 7 22 GPIO_ACTIVE_HIGH; /* PH22 */
+usb0_vbus-supply = reg_usb0_vbus;
+usb1_vbus-supply = reg_usb1_vbus;
+usb2_vbus-supply = reg_usb2_vbus;
};
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index a53bd5b..4614fba 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -173,6 +173,7 @@ config PHY_SUN4I_USB
tristate Allwinner sunxi SoC USB PHY driver
depends on ARCH_SUNXI HAS_IOMEM OF
depends on RESET_CONTROLLER
+select EXTCON
Avoid using 'select' on visible Kconfig symbols.
Ok, I'll do a v5 changing this to a depends on.
Also please split the patch to make the reviewing a bit easier.
There really is not much which can be split out, the iscr register
updating is only done from the id / vbus det scanning, so the 2 are
intertwined with each other.
Also I must say I'm unhappy with the fact that you request this now,
after this patch has been posted in its current form a long time
ago already. The changes in v3 and v4 were very minimal, the
patch has existed in this for since v2, so you've had a long long
time to request changes like this already.
In the mean time I've build a whole set of related changes (enabling
the use of otg on newer versions of the sunxi soc), so sorry I'm
not going to split this up now as that is going to be a big pain
with all the extra patches which are sitting on top, and as said
there really is not that much to split in the first place.
I'll send out a v5 of this patch together with a resend of all the
patches adding support for new models which sit on top of this one
(and which I've already send before).
Correction: Since I need to rework the extcon bits for the new extcon
API in 4.2 anyways, I will split out the extcon bits, so you will
get 2 patches in v5, one adding the id / vbus det gpio scanning +
iscr register updating to reflect the detected status and one on
top adding extcon support so that the musb glue can respond
to id-pin changes.
Regards,
Hans
--
To unsubscribe from this list: send the line unsubscribe
Re: [PATCH v4 1/2] phy-sun4i-usb: Add full support for usb0 phy / OTG
+Chanwoo
Hi,
On Sunday 31 May 2015 09:40 PM, Hans de Goede wrote:
The usb0 phy is connected to an OTG controller, and as such needs some special
handling:
1) It allows explicit control over the pullups, enable these on phy_init and
disable them on phy_exit.
2) It has bits to signal id and vbus detect to the musb-core, add support for
for monitoring id and vbus detect gpio-s for use in dual role mode, and set
these bits to the correct values for operating in host only mode when no
gpios are specified in the devicetree.
3) When in dual role mode the musb sunxi glue needs to know if the a host or
device cable is plugged in, so when in dual role mode register an extcon.
While updating the devicetree binding documentation also add documentation
for the sofar undocumented usage of regulators for vbus for all 3 phys.
Signed-off-by: Hans de Goede hdego...@redhat.com
---
Changes in v2:
-Removed the sunxi specific phy functions, instead the id / vbus gpio polling
has been moved to the phy-sun4i-usb driver and their status is exported
through extcon for the sunxi-musb glue
Changes in v3:
-No changes
Changes in v4:
-Do not call regulator_disable in an unbalanced manner when an external vbus
is present
---
.../devicetree/bindings/phy/sun4i-usb-phy.txt | 18 +-
drivers/phy/Kconfig| 1 +
drivers/phy/phy-sun4i-usb.c| 273 -
3 files changed, 281 insertions(+), 11 deletions(-)
diff --git a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
index 16528b9..557fa99 100644
--- a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
+++ b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
@@ -23,6 +23,13 @@ Required properties:
* usb1_reset
* usb2_reset for sun4i, sun6i or sun7i
+Optional properties:
+- usb0_id_det-gpios : gpio phandle for reading the otg id pin value
+- usb0_vbus_det-gpios : gpio phandle for detecting the presence of usb0 vbus
+- usb0_vbus-supply : regulator phandle for controller usb0 vbus
+- usb1_vbus-supply : regulator phandle for controller usb1 vbus
+- usb2_vbus-supply : regulator phandle for controller usb2 vbus
+
Example:
usbphy: phy@0x01c13400 {
#phy-cells = 1;
@@ -32,6 +39,13 @@ Example:
reg-names = phy_ctrl, pmu1, pmu2;
clocks = usb_clk 8;
clock-names = usb_phy;
- resets = usb_clk 1, usb_clk 2;
- reset-names = usb1_reset, usb2_reset;
+ resets = usb_clk 0, usb_clk 1, usb_clk 2;
+ reset-names = usb0_reset, usb1_reset, usb2_reset;
+ pinctrl-names = default;
+ pinctrl-0 = usb0_id_detect_pin, usb0_vbus_detect_pin;
+ usb0_id_det-gpios = pio 7 19 GPIO_ACTIVE_HIGH; /* PH19 */
+ usb0_vbus_det-gpios = pio 7 22 GPIO_ACTIVE_HIGH; /* PH22 */
+ usb0_vbus-supply = reg_usb0_vbus;
+ usb1_vbus-supply = reg_usb1_vbus;
+ usb2_vbus-supply = reg_usb2_vbus;
};
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index a53bd5b..4614fba 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -173,6 +173,7 @@ config PHY_SUN4I_USB
tristate Allwinner sunxi SoC USB PHY driver
depends on ARCH_SUNXI HAS_IOMEM OF
depends on RESET_CONTROLLER
+ select EXTCON
Avoid using 'select' on visible Kconfig symbols.
Also please split the patch to make the reviewing a bit easier.
Thanks
Kishon
select GENERIC_PHY
help
Enable this to support the transceiver that is part of Allwinner
diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index 91c5be4..b45d707 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -1,7 +1,7 @@
/*
* Allwinner sun4i USB phy driver
*
- * Copyright (C) 2014 Hans de Goede hdego...@redhat.com
+ * Copyright (C) 2014-2015 Hans de Goede hdego...@redhat.com
*
* Based on code from
* Allwinner Technology Co., Ltd. www.allwinnertech.com
@@ -23,17 +23,23 @@
#include linux/clk.h
#include linux/err.h
+#include linux/extcon.h
#include linux/io.h
+#include linux/interrupt.h
#include linux/kernel.h
#include linux/module.h
#include linux/mutex.h
#include linux/of.h
#include linux/of_address.h
+#include linux/of_gpio.h
#include linux/phy/phy.h
#include linux/phy/phy-sun4i-usb.h
#include linux/platform_device.h
#include linux/regulator/consumer.h
#include linux/reset.h
+#include linux/workqueue.h
+
+#define DRIVER_NAME sun4i-usb-phy
#define REG_ISCR 0x00
#define REG_PHYCTL0x04
@@ -47,6 +53,17 @@
#define SUNXI_AHB_INCRX_ALIGN_EN BIT(8)
#define SUNXI_ULPI_BYPASS_EN BIT(0)
+/* ISCR, Interface Status and Control bits */
+#define ISCR_ID_PULLUP_EN (1 17)
+#define ISCR_DPDM_PULLUP_EN(1 16)
+/*
Re: [PATCH v4 1/2] phy-sun4i-usb: Add full support for usb0 phy / OTG
Hi,
On Mon, Jun 01, 2015 at 11:28:23AM +0200, Hans de Goede wrote:
On 01-06-15 11:22, Maxime Ripard wrote:
On Sun, May 31, 2015 at 06:10:25PM +0200, Hans de Goede wrote:
+ /* We either want both gpio pins or neither (when in host mode) */
+ if (!data-id_det_gpio != !data-vbus_det_gpio) {
+ dev_err(dev, failed to get id or vbus detect pin\n);
+ return -ENODEV;
+ }
The fact that the driver expects both to be set if one is should be in
the binding documentation.
That would be behavior description, and I've always been told that the
binding should only document the hardware description, not the behavior.
Yes, that's true, but here it looks more like there's no point of
having one without the other.
Pretty much like interrupt-parent and interrupt. A single one doesn't
really make much sense without the other.
And I don't think saying that you would need some additional
properties when in device mode rather than in host mode is really
documenting a behaviour. It does change things at a hardware level
too.
Also this may change in the future as we add support for more boards,
e.g. the cubieboard seems to be unable to turn vbus off ever, so vbus-det
will always read as 1 / high, making it rather useless. This is something
which I still need to figure out how to deal with.
We can always figure it out in due time.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH v4 1/2] phy-sun4i-usb: Add full support for usb0 phy / OTG
On Sun, May 31, 2015 at 06:10:25PM +0200, Hans de Goede wrote:
+ /* We either want both gpio pins or neither (when in host mode) */
+ if (!data-id_det_gpio != !data-vbus_det_gpio) {
+ dev_err(dev, failed to get id or vbus detect pin\n);
+ return -ENODEV;
+ }
The fact that the driver expects both to be set if one is should be in
the binding documentation.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH v4 1/2] phy-sun4i-usb: Add full support for usb0 phy / OTG
Hi,
On 01-06-15 11:22, Maxime Ripard wrote:
On Sun, May 31, 2015 at 06:10:25PM +0200, Hans de Goede wrote:
+ /* We either want both gpio pins or neither (when in host mode) */
+ if (!data-id_det_gpio != !data-vbus_det_gpio) {
+ dev_err(dev, failed to get id or vbus detect pin\n);
+ return -ENODEV;
+ }
The fact that the driver expects both to be set if one is should be in
the binding documentation.
That would be behavior description, and I've always been told that the
binding should only document the hardware description, not the behavior.
Also this may change in the future as we add support for more boards,
e.g. the cubieboard seems to be unable to turn vbus off ever, so vbus-det
will always read as 1 / high, making it rather useless. This is something
which I still need to figure out how to deal with.
Regards,
Hans
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 1/2] phy-sun4i-usb: Add full support for usb0 phy / OTG
The usb0 phy is connected to an OTG controller, and as such needs some special
handling:
1) It allows explicit control over the pullups, enable these on phy_init and
disable them on phy_exit.
2) It has bits to signal id and vbus detect to the musb-core, add support for
for monitoring id and vbus detect gpio-s for use in dual role mode, and set
these bits to the correct values for operating in host only mode when no
gpios are specified in the devicetree.
3) When in dual role mode the musb sunxi glue needs to know if the a host or
device cable is plugged in, so when in dual role mode register an extcon.
While updating the devicetree binding documentation also add documentation
for the sofar undocumented usage of regulators for vbus for all 3 phys.
Signed-off-by: Hans de Goede hdego...@redhat.com
---
Changes in v2:
-Removed the sunxi specific phy functions, instead the id / vbus gpio polling
has been moved to the phy-sun4i-usb driver and their status is exported
through extcon for the sunxi-musb glue
Changes in v3:
-No changes
Changes in v4:
-Do not call regulator_disable in an unbalanced manner when an external vbus
is present
---
.../devicetree/bindings/phy/sun4i-usb-phy.txt | 18 +-
drivers/phy/Kconfig| 1 +
drivers/phy/phy-sun4i-usb.c| 273 -
3 files changed, 281 insertions(+), 11 deletions(-)
diff --git a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
index 16528b9..557fa99 100644
--- a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
+++ b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
@@ -23,6 +23,13 @@ Required properties:
* usb1_reset
* usb2_reset for sun4i, sun6i or sun7i
+Optional properties:
+- usb0_id_det-gpios : gpio phandle for reading the otg id pin value
+- usb0_vbus_det-gpios : gpio phandle for detecting the presence of usb0 vbus
+- usb0_vbus-supply : regulator phandle for controller usb0 vbus
+- usb1_vbus-supply : regulator phandle for controller usb1 vbus
+- usb2_vbus-supply : regulator phandle for controller usb2 vbus
+
Example:
usbphy: phy@0x01c13400 {
#phy-cells = 1;
@@ -32,6 +39,13 @@ Example:
reg-names = phy_ctrl, pmu1, pmu2;
clocks = usb_clk 8;
clock-names = usb_phy;
- resets = usb_clk 1, usb_clk 2;
- reset-names = usb1_reset, usb2_reset;
+ resets = usb_clk 0, usb_clk 1, usb_clk 2;
+ reset-names = usb0_reset, usb1_reset, usb2_reset;
+ pinctrl-names = default;
+ pinctrl-0 = usb0_id_detect_pin, usb0_vbus_detect_pin;
+ usb0_id_det-gpios = pio 7 19 GPIO_ACTIVE_HIGH; /* PH19 */
+ usb0_vbus_det-gpios = pio 7 22 GPIO_ACTIVE_HIGH; /* PH22 */
+ usb0_vbus-supply = reg_usb0_vbus;
+ usb1_vbus-supply = reg_usb1_vbus;
+ usb2_vbus-supply = reg_usb2_vbus;
};
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index a53bd5b..4614fba 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -173,6 +173,7 @@ config PHY_SUN4I_USB
tristate Allwinner sunxi SoC USB PHY driver
depends on ARCH_SUNXI HAS_IOMEM OF
depends on RESET_CONTROLLER
+ select EXTCON
select GENERIC_PHY
help
Enable this to support the transceiver that is part of Allwinner
diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index 91c5be4..b45d707 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -1,7 +1,7 @@
/*
* Allwinner sun4i USB phy driver
*
- * Copyright (C) 2014 Hans de Goede hdego...@redhat.com
+ * Copyright (C) 2014-2015 Hans de Goede hdego...@redhat.com
*
* Based on code from
* Allwinner Technology Co., Ltd. www.allwinnertech.com
@@ -23,17 +23,23 @@
#include linux/clk.h
#include linux/err.h
+#include linux/extcon.h
#include linux/io.h
+#include linux/interrupt.h
#include linux/kernel.h
#include linux/module.h
#include linux/mutex.h
#include linux/of.h
#include linux/of_address.h
+#include linux/of_gpio.h
#include linux/phy/phy.h
#include linux/phy/phy-sun4i-usb.h
#include linux/platform_device.h
#include linux/regulator/consumer.h
#include linux/reset.h
+#include linux/workqueue.h
+
+#define DRIVER_NAME sun4i-usb-phy
#define REG_ISCR 0x00
#define REG_PHYCTL 0x04
@@ -47,6 +53,17 @@
#define SUNXI_AHB_INCRX_ALIGN_EN BIT(8)
#define SUNXI_ULPI_BYPASS_EN BIT(0)
+/* ISCR, Interface Status and Control bits */
+#define ISCR_ID_PULLUP_EN (1 17)
+#define ISCR_DPDM_PULLUP_EN(1 16)
+/* sunxi has the phy id/vbus pins not connected, so we use the force bits */
+#define ISCR_FORCE_ID_MASK (3 14)
+#define ISCR_FORCE_ID_LOW (2 14)
+#define ISCR_FORCE_ID_HIGH (3 14)
+#define
