This patch series has some Enhancements and Bug Fixes to chipidea USB
driver related to Zynq platform.
Manish Narani (3):
usb: chipidea: Add support for VBUS control with PHY
usb: chipidea: Use usb2 phy for Zynq platform
usb: chipidea: Check usb_phy exists before using it
Piyush Mehta (3):
usb: chipidea: Fix return value handling
usb: chipidea: core: Add return value function check
usb: chipidea: udc: Add xilinx revision support
drivers/usb/chipidea/ci_hdrc_usb2.c | 5 +++++
drivers/usb/chipidea/core.c | 26 +++++++++++++++++++-------
drivers/usb/chipidea/host.c | 9 +++++++++
drivers/usb/chipidea/otg.c | 6 +++++-
drivers/usb/chipidea/otg_fsm.c | 9 +++++++++
drivers/usb/chipidea/udc.c | 3 ++-
include/linux/usb/chipidea.h | 1 +
7 files changed, 50 insertions(+), 9 deletions(-)
--
2.1.1
Some platforms make use of VBUS control over PHY which
means controller driver has to access PHY registers to
turn on/off VBUS line.This patch adds support for
such platforms in chipidea.
Signed-off-by: Subbaraya Sundeep Bhatta <[email protected]>
Signed-off-by: Michal Simek <[email protected]>
Signed-off-by: Manish Narani <[email protected]>
---
drivers/usb/chipidea/ci_hdrc_usb2.c | 1 +
drivers/usb/chipidea/host.c | 9 +++++++++
drivers/usb/chipidea/otg_fsm.c | 7 +++++++
include/linux/usb/chipidea.h | 1 +
4 files changed, 18 insertions(+)
diff --git a/drivers/usb/chipidea/ci_hdrc_usb2.c b/drivers/usb/chipidea/ci_hdrc_usb2.c
index 89e1d82..dc86b12 100644
--- a/drivers/usb/chipidea/ci_hdrc_usb2.c
+++ b/drivers/usb/chipidea/ci_hdrc_usb2.c
@@ -30,6 +30,7 @@ static const struct ci_hdrc_platform_data ci_default_pdata = {
static const struct ci_hdrc_platform_data ci_zynq_pdata = {
.capoffset = DEF_CAPOFFSET,
+ .flags = CI_HDRC_PHY_VBUS_CONTROL,
};
static const struct ci_hdrc_platform_data ci_zevio_pdata = {
diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
index e86d13c..578968d 100644
--- a/drivers/usb/chipidea/host.c
+++ b/drivers/usb/chipidea/host.c
@@ -63,6 +63,14 @@ static int ehci_ci_portpower(struct usb_hcd *hcd, int portnum, bool enable)
priv->enabled = enable;
}
+ if (ci->platdata->flags & CI_HDRC_PHY_VBUS_CONTROL &&
+ ci->usb_phy && ci->usb_phy->set_vbus) {
+ if (enable)
+ ci->usb_phy->set_vbus(ci->usb_phy, 1);
+ else
+ ci->usb_phy->set_vbus(ci->usb_phy, 0);
+ }
+
if (enable && (ci->platdata->phy_mode == USBPHY_INTERFACE_MODE_HSIC)) {
/*
* Marvell 28nm HSIC PHY requires forcing the port to HS mode.
@@ -71,6 +79,7 @@ static int ehci_ci_portpower(struct usb_hcd *hcd, int portnum, bool enable)
hw_port_test_set(ci, 5);
hw_port_test_set(ci, 0);
}
+
return 0;
};
diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
index 6ed4b00..2f7f94d 100644
--- a/drivers/usb/chipidea/otg_fsm.c
+++ b/drivers/usb/chipidea/otg_fsm.c
@@ -471,6 +471,10 @@ static void ci_otg_drv_vbus(struct otg_fsm *fsm, int on)
return;
}
}
+
+ if (ci->platdata->flags & CI_HDRC_PHY_VBUS_CONTROL)
+ ci->usb_phy->set_vbus(ci->usb_phy, 1);
+
/* Disable data pulse irq */
hw_write_otgsc(ci, OTGSC_DPIE, 0);
@@ -480,6 +484,9 @@ static void ci_otg_drv_vbus(struct otg_fsm *fsm, int on)
if (ci->platdata->reg_vbus)
regulator_disable(ci->platdata->reg_vbus);
+ if (ci->platdata->flags & CI_HDRC_PHY_VBUS_CONTROL)
+ ci->usb_phy->set_vbus(ci->usb_phy, 0);
+
fsm->a_bus_drop = 1;
fsm->a_bus_req = 0;
}
diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
index edf3342..ee38835 100644
--- a/include/linux/usb/chipidea.h
+++ b/include/linux/usb/chipidea.h
@@ -62,6 +62,7 @@ struct ci_hdrc_platform_data {
#define CI_HDRC_REQUIRES_ALIGNED_DMA BIT(13)
#define CI_HDRC_IMX_IS_HSIC BIT(14)
#define CI_HDRC_PMQOS BIT(15)
+#define CI_HDRC_PHY_VBUS_CONTROL BIT(16)
enum usb_dr_mode dr_mode;
#define CI_HDRC_CONTROLLER_RESET_EVENT 0
#define CI_HDRC_CONTROLLER_STOPPED_EVENT 1
--
2.1.1
From: Piyush Mehta <[email protected]>
Issue: Adding a dTD to a Primed Endpoint May Not Get Recognized with
revision 2.20a.
There is an issue with the add dTD tripwire semaphore (ATDTW bit in
USBCMD register) that can cause the controller to ignore a dTD that is
added to a primed endpoint. When this happens, the software can read
the tripwire bit and the status bit at '1' even though the endpoint is
unprimed.
This issue observed with the Windows host machine.
Workaround:
The software must implement a periodic cycle, and check for each dTD
pending on execution (Active = 1), if the endpoint is primed. It can do
this by reading the corresponding bits in the ENDPTPRIME and ENDPTSTAT
registers. If these bits are read at 0, the software needs to re-prime
the endpoint by writing 1 to the corresponding bit in the ENDPTPRIME
register.
Added conditional revision check of 2.20[CI_REVISION_22] along with 2.40.
Signed-off-by: Piyush Mehta <[email protected]>
Signed-off-by: Manish Narani <[email protected]>
---
drivers/usb/chipidea/udc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 8834ca6..b440205 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -680,7 +680,8 @@ static int _hardware_dequeue(struct ci_hw_ep *hwep, struct ci_hw_req *hwreq)
if ((TD_STATUS_ACTIVE & tmptoken) != 0) {
int n = hw_ep_bit(hwep->num, hwep->dir);
- if (ci->rev == CI_REVISION_24)
+ if (ci->rev == CI_REVISION_24 ||
+ ci->rev == CI_REVISION_22)
if (!hw_read(ci, OP_ENDPTSTAT, BIT(n)))
reprime_dtd(ci, hwep, node);
hwreq->req.status = -EALREADY;
--
2.1.1
From: Piyush Mehta <[email protected]>
Add return value validation for function phy exit and phy power off.
Addresses-Coverity: "USELESS_CALL"
Signed-off-by: Piyush Mehta <[email protected]>
Signed-off-by: Manish Narani <[email protected]>
---
drivers/usb/chipidea/core.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 676346f..37f619e 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -324,7 +324,8 @@ static int _ci_usb_phy_init(struct ci_hdrc *ci)
ret = phy_power_on(ci->phy);
if (ret) {
- phy_exit(ci->phy);
+ if (phy_exit(ci->phy) < 0)
+ dev_dbg(ci->dev, "phy exit failed\r\n");
return ret;
}
} else {
@@ -341,12 +342,20 @@ static int _ci_usb_phy_init(struct ci_hdrc *ci)
*/
static void ci_usb_phy_exit(struct ci_hdrc *ci)
{
+ int ret;
+
if (ci->platdata->flags & CI_HDRC_OVERRIDE_PHY_CONTROL)
return;
if (ci->phy) {
- phy_power_off(ci->phy);
- phy_exit(ci->phy);
+ ret = phy_power_off(ci->phy);
+ if (ret < 0)
+ dev_dbg(ci->dev, "phy poweroff failed\r\n");
+
+ ret = phy_exit(ci->phy);
+ if (ret < 0)
+ dev_dbg(ci->dev, "phy exit failed\r\n");
+
} else {
usb_phy_shutdown(ci->usb_phy);
}
--
2.1.1
usb_phy and usb_phy->set_vbus may not be present all the times
based on PHY driver used. So check for it.
Signed-off-by: Subbaraya Sundeep Bhatta <[email protected]>
Signed-off-by: Michal Simek <[email protected]>
Signed-off-by: Manish Narani <[email protected]>
---
drivers/usb/chipidea/otg_fsm.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
index 2f7f94d..5f8f5d2 100644
--- a/drivers/usb/chipidea/otg_fsm.c
+++ b/drivers/usb/chipidea/otg_fsm.c
@@ -472,7 +472,8 @@ static void ci_otg_drv_vbus(struct otg_fsm *fsm, int on)
}
}
- if (ci->platdata->flags & CI_HDRC_PHY_VBUS_CONTROL)
+ if (ci->platdata->flags & CI_HDRC_PHY_VBUS_CONTROL &&
+ ci->usb_phy && ci->usb_phy->set_vbus)
ci->usb_phy->set_vbus(ci->usb_phy, 1);
/* Disable data pulse irq */
@@ -484,7 +485,8 @@ static void ci_otg_drv_vbus(struct otg_fsm *fsm, int on)
if (ci->platdata->reg_vbus)
regulator_disable(ci->platdata->reg_vbus);
- if (ci->platdata->flags & CI_HDRC_PHY_VBUS_CONTROL)
+ if (ci->platdata->flags & CI_HDRC_PHY_VBUS_CONTROL &&
+ ci->usb_phy && ci->usb_phy->set_vbus)
ci->usb_phy->set_vbus(ci->usb_phy, 0);
fsm->a_bus_drop = 1;
--
2.1.1
From: Piyush Mehta <[email protected]>
API was neither captured nor checked.Fixed it by capturing the
return value and then checking for any error.
Addresses-Coverity: "Calling without checking return"
Addresses-Coverity: "CHECKED_RETURN"
Signed-off-by: Piyush Mehta <[email protected]>
Signed-off-by: Manish Narani <[email protected]>
---
drivers/usb/chipidea/core.c | 11 +++++++----
drivers/usb/chipidea/otg.c | 6 +++++-
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 2b18f50..676346f 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -700,13 +700,16 @@ static int ci_get_platdata(struct device *dev,
if (usb_get_maximum_speed(dev) == USB_SPEED_FULL)
platdata->flags |= CI_HDRC_FORCE_FULLSPEED;
- of_property_read_u32(dev->of_node, "phy-clkgate-delay-us",
- &platdata->phy_clkgate_delay_us);
+ if (of_property_read_u32(dev->of_node, "phy-clkgate-delay-us",
+ &platdata->phy_clkgate_delay_us))
+ dev_dbg(dev, "Missing phy-clkgate-delay-us property\n");
platdata->itc_setting = 1;
- of_property_read_u32(dev->of_node, "itc-setting",
- &platdata->itc_setting);
+ if (of_property_read_u32(dev->of_node, "itc-setting",
+ &platdata->itc_setting))
+ dev_dbg(dev, "Missing itc-setting property\n");
+
ret = of_property_read_u32(dev->of_node, "ahb-burst-config",
&platdata->ahb_burst_config);
diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
index 8dd5928..d527d9d 100644
--- a/drivers/usb/chipidea/otg.c
+++ b/drivers/usb/chipidea/otg.c
@@ -168,6 +168,7 @@ static int hw_wait_vbus_lower_bsv(struct ci_hdrc *ci)
static void ci_handle_id_switch(struct ci_hdrc *ci)
{
enum ci_role role = ci_otg_role(ci);
+ int ret;
if (role != ci->role) {
dev_dbg(ci->dev, "switching from %s to %s\n",
@@ -193,7 +194,10 @@ static void ci_handle_id_switch(struct ci_hdrc *ci)
*/
hw_wait_vbus_lower_bsv(ci);
- ci_role_start(ci, role);
+ ret = ci_role_start(ci, role);
+ if (ret < 0)
+ dev_dbg(ci->dev, "switching err %d\n", ret);
+
/* vbus change may have already occurred */
if (role == CI_ROLE_GADGET)
ci_handle_vbus_change(ci);
--
2.1.1
On 21-08-24 22:46:13, Manish Narani wrote:
> Some platforms make use of VBUS control over PHY which
> means controller driver has to access PHY registers to
> turn on/off VBUS line.This patch adds support for
> such platforms in chipidea.
>
> Signed-off-by: Subbaraya Sundeep Bhatta <[email protected]>
> Signed-off-by: Michal Simek <[email protected]>
> Signed-off-by: Manish Narani <[email protected]>
> ---
> drivers/usb/chipidea/ci_hdrc_usb2.c | 1 +
> drivers/usb/chipidea/host.c | 9 +++++++++
> drivers/usb/chipidea/otg_fsm.c | 7 +++++++
> include/linux/usb/chipidea.h | 1 +
> 4 files changed, 18 insertions(+)
>
> diff --git a/drivers/usb/chipidea/ci_hdrc_usb2.c b/drivers/usb/chipidea/ci_hdrc_usb2.c
> index 89e1d82..dc86b12 100644
> --- a/drivers/usb/chipidea/ci_hdrc_usb2.c
> +++ b/drivers/usb/chipidea/ci_hdrc_usb2.c
> @@ -30,6 +30,7 @@ static const struct ci_hdrc_platform_data ci_default_pdata = {
>
> static const struct ci_hdrc_platform_data ci_zynq_pdata = {
> .capoffset = DEF_CAPOFFSET,
> + .flags = CI_HDRC_PHY_VBUS_CONTROL,
> };
>
> static const struct ci_hdrc_platform_data ci_zevio_pdata = {
> diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
> index e86d13c..578968d 100644
> --- a/drivers/usb/chipidea/host.c
> +++ b/drivers/usb/chipidea/host.c
> @@ -63,6 +63,14 @@ static int ehci_ci_portpower(struct usb_hcd *hcd, int portnum, bool enable)
> priv->enabled = enable;
> }
>
> + if (ci->platdata->flags & CI_HDRC_PHY_VBUS_CONTROL &&
> + ci->usb_phy && ci->usb_phy->set_vbus) {
Indent is needed. Otherwise:
Acked-by: Peter Chen <[email protected]>
Peter
> + if (enable)
> + ci->usb_phy->set_vbus(ci->usb_phy, 1);
> + else
> + ci->usb_phy->set_vbus(ci->usb_phy, 0);
> + }
> +
> if (enable && (ci->platdata->phy_mode == USBPHY_INTERFACE_MODE_HSIC)) {
> /*
> * Marvell 28nm HSIC PHY requires forcing the port to HS mode.
> @@ -71,6 +79,7 @@ static int ehci_ci_portpower(struct usb_hcd *hcd, int portnum, bool enable)
> hw_port_test_set(ci, 5);
> hw_port_test_set(ci, 0);
> }
> +
> return 0;
> };
>
> diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
> index 6ed4b00..2f7f94d 100644
> --- a/drivers/usb/chipidea/otg_fsm.c
> +++ b/drivers/usb/chipidea/otg_fsm.c
> @@ -471,6 +471,10 @@ static void ci_otg_drv_vbus(struct otg_fsm *fsm, int on)
> return;
> }
> }
> +
> + if (ci->platdata->flags & CI_HDRC_PHY_VBUS_CONTROL)
> + ci->usb_phy->set_vbus(ci->usb_phy, 1);
> +
> /* Disable data pulse irq */
> hw_write_otgsc(ci, OTGSC_DPIE, 0);
>
> @@ -480,6 +484,9 @@ static void ci_otg_drv_vbus(struct otg_fsm *fsm, int on)
> if (ci->platdata->reg_vbus)
> regulator_disable(ci->platdata->reg_vbus);
>
> + if (ci->platdata->flags & CI_HDRC_PHY_VBUS_CONTROL)
> + ci->usb_phy->set_vbus(ci->usb_phy, 0);
> +
> fsm->a_bus_drop = 1;
> fsm->a_bus_req = 0;
> }
> diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
> index edf3342..ee38835 100644
> --- a/include/linux/usb/chipidea.h
> +++ b/include/linux/usb/chipidea.h
> @@ -62,6 +62,7 @@ struct ci_hdrc_platform_data {
> #define CI_HDRC_REQUIRES_ALIGNED_DMA BIT(13)
> #define CI_HDRC_IMX_IS_HSIC BIT(14)
> #define CI_HDRC_PMQOS BIT(15)
> +#define CI_HDRC_PHY_VBUS_CONTROL BIT(16)
> enum usb_dr_mode dr_mode;
> #define CI_HDRC_CONTROLLER_RESET_EVENT 0
> #define CI_HDRC_CONTROLLER_STOPPED_EVENT 1
> --
> 2.1.1
>
--
Thanks,
Peter Chen
On 21-08-24 22:46:15, Manish Narani wrote:
> usb_phy and usb_phy->set_vbus may not be present all the times
> based on PHY driver used. So check for it.
Please squash this one with the first one.
Peter
>
> Signed-off-by: Subbaraya Sundeep Bhatta <[email protected]>
> Signed-off-by: Michal Simek <[email protected]>
> Signed-off-by: Manish Narani <[email protected]>
> ---
> drivers/usb/chipidea/otg_fsm.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
> index 2f7f94d..5f8f5d2 100644
> --- a/drivers/usb/chipidea/otg_fsm.c
> +++ b/drivers/usb/chipidea/otg_fsm.c
> @@ -472,7 +472,8 @@ static void ci_otg_drv_vbus(struct otg_fsm *fsm, int on)
> }
> }
>
> - if (ci->platdata->flags & CI_HDRC_PHY_VBUS_CONTROL)
> + if (ci->platdata->flags & CI_HDRC_PHY_VBUS_CONTROL &&
> + ci->usb_phy && ci->usb_phy->set_vbus)
> ci->usb_phy->set_vbus(ci->usb_phy, 1);
>
> /* Disable data pulse irq */
> @@ -484,7 +485,8 @@ static void ci_otg_drv_vbus(struct otg_fsm *fsm, int on)
> if (ci->platdata->reg_vbus)
> regulator_disable(ci->platdata->reg_vbus);
>
> - if (ci->platdata->flags & CI_HDRC_PHY_VBUS_CONTROL)
> + if (ci->platdata->flags & CI_HDRC_PHY_VBUS_CONTROL &&
> + ci->usb_phy && ci->usb_phy->set_vbus)
> ci->usb_phy->set_vbus(ci->usb_phy, 0);
>
> fsm->a_bus_drop = 1;
> --
> 2.1.1
>
--
Thanks,
Peter Chen
On 21-08-24 22:46:17, Manish Narani wrote:
> From: Piyush Mehta <[email protected]>
>
> Add return value validation for function phy exit and phy power off.
>
> Addresses-Coverity: "USELESS_CALL"
> Signed-off-by: Piyush Mehta <[email protected]>
> Signed-off-by: Manish Narani <[email protected]>
> ---
> drivers/usb/chipidea/core.c | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 676346f..37f619e 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -324,7 +324,8 @@ static int _ci_usb_phy_init(struct ci_hdrc *ci)
>
> ret = phy_power_on(ci->phy);
> if (ret) {
> - phy_exit(ci->phy);
> + if (phy_exit(ci->phy) < 0)
> + dev_dbg(ci->dev, "phy exit failed\r\n");
You may not need to debug message, the phy_exit shows error message.
You may return 'ret' for phy_exit for error.
> return ret;
> }
> } else {
> @@ -341,12 +342,20 @@ static int _ci_usb_phy_init(struct ci_hdrc *ci)
> */
> static void ci_usb_phy_exit(struct ci_hdrc *ci)
> {
> + int ret;
> +
> if (ci->platdata->flags & CI_HDRC_OVERRIDE_PHY_CONTROL)
> return;
>
> if (ci->phy) {
> - phy_power_off(ci->phy);
> - phy_exit(ci->phy);
> + ret = phy_power_off(ci->phy);
> + if (ret < 0)
> + dev_dbg(ci->dev, "phy poweroff failed\r\n");
> +
> + ret = phy_exit(ci->phy);
> + if (ret < 0)
> + dev_dbg(ci->dev, "phy exit failed\r\n");
> +
Ditto
> } else {
> usb_phy_shutdown(ci->usb_phy);
> }
> --
> 2.1.1
>
--
Thanks,
Peter Chen
On 21-08-24 22:46:18, Manish Narani wrote:
> From: Piyush Mehta <[email protected]>
>
> Issue: Adding a dTD to a Primed Endpoint May Not Get Recognized with
> revision 2.20a.
>
> There is an issue with the add dTD tripwire semaphore (ATDTW bit in
> USBCMD register) that can cause the controller to ignore a dTD that is
> added to a primed endpoint. When this happens, the software can read
> the tripwire bit and the status bit at '1' even though the endpoint is
> unprimed.
> This issue observed with the Windows host machine.
>
> Workaround:
> The software must implement a periodic cycle, and check for each dTD
> pending on execution (Active = 1), if the endpoint is primed. It can do
> this by reading the corresponding bits in the ENDPTPRIME and ENDPTSTAT
> registers. If these bits are read at 0, the software needs to re-prime
> the endpoint by writing 1 to the corresponding bit in the ENDPTPRIME
> register.
>
> Added conditional revision check of 2.20[CI_REVISION_22] along with 2.40.
>
> Signed-off-by: Piyush Mehta <[email protected]>
> Signed-off-by: Manish Narani <[email protected]>
> ---
> drivers/usb/chipidea/udc.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index 8834ca6..b440205 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -680,7 +680,8 @@ static int _hardware_dequeue(struct ci_hw_ep *hwep, struct ci_hw_req *hwreq)
> if ((TD_STATUS_ACTIVE & tmptoken) != 0) {
> int n = hw_ep_bit(hwep->num, hwep->dir);
>
> - if (ci->rev == CI_REVISION_24)
> + if (ci->rev == CI_REVISION_24 ||
> + ci->rev == CI_REVISION_22)
Add indent for it, otherwise:
Acked-by: Peter Chen <[email protected]>
> if (!hw_read(ci, OP_ENDPTSTAT, BIT(n)))
> reprime_dtd(ci, hwep, node);
> hwreq->req.status = -EALREADY;
> --
> 2.1.1
>
--
Thanks,
Peter Chen
On 21-08-24 22:46:18, Manish Narani wrote:
> From: Piyush Mehta <[email protected]>
>
> Issue: Adding a dTD to a Primed Endpoint May Not Get Recognized with
> revision 2.20a.
>
> There is an issue with the add dTD tripwire semaphore (ATDTW bit in
> USBCMD register) that can cause the controller to ignore a dTD that is
> added to a primed endpoint. When this happens, the software can read
> the tripwire bit and the status bit at '1' even though the endpoint is
> unprimed.
> This issue observed with the Windows host machine.
>
> Workaround:
> The software must implement a periodic cycle, and check for each dTD
> pending on execution (Active = 1), if the endpoint is primed. It can do
> this by reading the corresponding bits in the ENDPTPRIME and ENDPTSTAT
> registers. If these bits are read at 0, the software needs to re-prime
> the endpoint by writing 1 to the corresponding bit in the ENDPTPRIME
> register.
>
> Added conditional revision check of 2.20[CI_REVISION_22] along with 2.40.
>
> Signed-off-by: Piyush Mehta <[email protected]>
> Signed-off-by: Manish Narani <[email protected]>
> ---
> drivers/usb/chipidea/udc.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index 8834ca6..b440205 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -680,7 +680,8 @@ static int _hardware_dequeue(struct ci_hw_ep *hwep, struct ci_hw_req *hwreq)
> if ((TD_STATUS_ACTIVE & tmptoken) != 0) {
> int n = hw_ep_bit(hwep->num, hwep->dir);
>
> - if (ci->rev == CI_REVISION_24)
> + if (ci->rev == CI_REVISION_24 ||
> + ci->rev == CI_REVISION_22)
Besides, please change subject a little, it is for specific IP version,
but not related to SoC platforms.
> if (!hw_read(ci, OP_ENDPTSTAT, BIT(n)))
> reprime_dtd(ci, hwep, node);
> hwreq->req.status = -EALREADY;
> --
> 2.1.1
>
--
Thanks,
Peter Chen
On 21-08-24 22:46:12, Manish Narani wrote:
> This patch series has some Enhancements and Bug Fixes to chipidea USB
> driver related to Zynq platform.
>
> Manish Narani (3):
> usb: chipidea: Add support for VBUS control with PHY
> usb: chipidea: Use usb2 phy for Zynq platform
> usb: chipidea: Check usb_phy exists before using it
>
> Piyush Mehta (3):
> usb: chipidea: Fix return value handling
> usb: chipidea: core: Add return value function check
> usb: chipidea: udc: Add xilinx revision support
>
> drivers/usb/chipidea/ci_hdrc_usb2.c | 5 +++++
> drivers/usb/chipidea/core.c | 26 +++++++++++++++++++-------
> drivers/usb/chipidea/host.c | 9 +++++++++
> drivers/usb/chipidea/otg.c | 6 +++++-
> drivers/usb/chipidea/otg_fsm.c | 9 +++++++++
> drivers/usb/chipidea/udc.c | 3 ++-
> include/linux/usb/chipidea.h | 1 +
> 7 files changed, 50 insertions(+), 9 deletions(-)
>
I am afraid I can't find [4/6] at my INBOX, comments below:
From: Piyush Mehta <[email protected]>
API was neither captured nor checked.Fixed it by capturing the
return value and then checking for any error.
Addresses-Coverity: "Calling without checking return"
Addresses-Coverity: "CHECKED_RETURN"
Signed-off-by: Piyush Mehta <[email protected]>
Signed-off-by: Manish Narani <[email protected]>
---
drivers/usb/chipidea/core.c | 11 +++++++----
drivers/usb/chipidea/otg.c | 6 +++++-
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 2b18f50..676346f 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -700,13 +700,16 @@ static int ci_get_platdata(struct device *dev,
if (usb_get_maximum_speed(dev) == USB_SPEED_FULL)
platdata->flags |= CI_HDRC_FORCE_FULLSPEED;
- of_property_read_u32(dev->of_node, "phy-clkgate-delay-us",
- &platdata->phy_clkgate_delay_us);
+ if (of_property_read_u32(dev->of_node, "phy-clkgate-delay-us",
+ &platdata->phy_clkgate_delay_us))
+ dev_dbg(dev, "Missing phy-clkgate-delay-us property\n");
platdata->itc_setting = 1;
- of_property_read_u32(dev->of_node, "itc-setting",
- &platdata->itc_setting);
+ if (of_property_read_u32(dev->of_node, "itc-setting",
+ &platdata->itc_setting))
+ dev_dbg(dev, "Missing itc-setting property\n");
+
You may check how to handle this for other properties in this function.
ret = of_property_read_u32(dev->of_node, "ahb-burst-config",
&platdata->ahb_burst_config);
diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
index 8dd5928..d527d9d 100644
--- a/drivers/usb/chipidea/otg.c
+++ b/drivers/usb/chipidea/otg.c
@@ -168,6 +168,7 @@ static int hw_wait_vbus_lower_bsv(struct ci_hdrc *ci)
static void ci_handle_id_switch(struct ci_hdrc *ci)
{
enum ci_role role = ci_otg_role(ci);
+ int ret;
if (role != ci->role) {
dev_dbg(ci->dev, "switching from %s to %s\n",
@@ -193,7 +194,10 @@ static void ci_handle_id_switch(struct ci_hdrc *ci)
*/
hw_wait_vbus_lower_bsv(ci);
- ci_role_start(ci, role);
+ ret = ci_role_start(ci, role);
+ if (ret < 0)
+ dev_dbg(ci->dev, "switching err %d\n", ret);
+
You may use dev_err for it.
/* vbus change may have already occurred */
if (role == CI_ROLE_GADGET)
ci_handle_vbus_change(ci);
--
Thanks,
Peter Chen
Manish Narani <[email protected]> 于2021年8月25日周三 上午1:23写道：
>
> Some platforms make use of VBUS control over PHY which
> means controller driver has to access PHY registers to
> turn on/off VBUS line.This patch adds support for
> such platforms in chipidea.
>
> Signed-off-by: Subbaraya Sundeep Bhatta <[email protected]>
> Signed-off-by: Michal Simek <[email protected]>
> Signed-off-by: Manish Narani <[email protected]>
> ---
> drivers/usb/chipidea/ci_hdrc_usb2.c | 1 +
> drivers/usb/chipidea/host.c | 9 +++++++++
> drivers/usb/chipidea/otg_fsm.c | 7 +++++++
> include/linux/usb/chipidea.h | 1 +
> 4 files changed, 18 insertions(+)
>
> diff --git a/drivers/usb/chipidea/ci_hdrc_usb2.c b/drivers/usb/chipidea/ci_hdrc_usb2.c
> index 89e1d82..dc86b12 100644
> --- a/drivers/usb/chipidea/ci_hdrc_usb2.c
> +++ b/drivers/usb/chipidea/ci_hdrc_usb2.c
> @@ -30,6 +30,7 @@ static const struct ci_hdrc_platform_data ci_default_pdata = {
>
> static const struct ci_hdrc_platform_data ci_zynq_pdata = {
> .capoffset = DEF_CAPOFFSET,
> + .flags = CI_HDRC_PHY_VBUS_CONTROL,
> };
>
> static const struct ci_hdrc_platform_data ci_zevio_pdata = {
> diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
> index e86d13c..578968d 100644
> --- a/drivers/usb/chipidea/host.c
> +++ b/drivers/usb/chipidea/host.c
> @@ -63,6 +63,14 @@ static int ehci_ci_portpower(struct usb_hcd *hcd, int portnum, bool enable)
> priv->enabled = enable;
> }
>
> + if (ci->platdata->flags & CI_HDRC_PHY_VBUS_CONTROL &&
> + ci->usb_phy && ci->usb_phy->set_vbus) {
> + if (enable)
> + ci->usb_phy->set_vbus(ci->usb_phy, 1);
> + else
> + ci->usb_phy->set_vbus(ci->usb_phy, 0);
> + }
> +
Can existing API usb_phy_vbus_on/off() work for you? I think that
helper will make it simple so you don't need a new flag and those
checks.
Li Jun
> if (enable && (ci->platdata->phy_mode == USBPHY_INTERFACE_MODE_HSIC)) {
> /*
> * Marvell 28nm HSIC PHY requires forcing the port to HS mode.
> @@ -71,6 +79,7 @@ static int ehci_ci_portpower(struct usb_hcd *hcd, int portnum, bool enable)
> hw_port_test_set(ci, 5);
> hw_port_test_set(ci, 0);
> }
> +
> return 0;
> };
>
> diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
> index 6ed4b00..2f7f94d 100644
> --- a/drivers/usb/chipidea/otg_fsm.c
> +++ b/drivers/usb/chipidea/otg_fsm.c
> @@ -471,6 +471,10 @@ static void ci_otg_drv_vbus(struct otg_fsm *fsm, int on)
> return;
> }
> }
> +
> + if (ci->platdata->flags & CI_HDRC_PHY_VBUS_CONTROL)
> + ci->usb_phy->set_vbus(ci->usb_phy, 1);
> +
> /* Disable data pulse irq */
> hw_write_otgsc(ci, OTGSC_DPIE, 0);
>
> @@ -480,6 +484,9 @@ static void ci_otg_drv_vbus(struct otg_fsm *fsm, int on)
> if (ci->platdata->reg_vbus)
> regulator_disable(ci->platdata->reg_vbus);
>
> + if (ci->platdata->flags & CI_HDRC_PHY_VBUS_CONTROL)
> + ci->usb_phy->set_vbus(ci->usb_phy, 0);
> +
> fsm->a_bus_drop = 1;
> fsm->a_bus_req = 0;
> }
> diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
> index edf3342..ee38835 100644
> --- a/include/linux/usb/chipidea.h
> +++ b/include/linux/usb/chipidea.h
> @@ -62,6 +62,7 @@ struct ci_hdrc_platform_data {
> #define CI_HDRC_REQUIRES_ALIGNED_DMA BIT(13)
> #define CI_HDRC_IMX_IS_HSIC BIT(14)
> #define CI_HDRC_PMQOS BIT(15)
> +#define CI_HDRC_PHY_VBUS_CONTROL BIT(16)
> enum usb_dr_mode dr_mode;
> #define CI_HDRC_CONTROLLER_RESET_EVENT 0
> #define CI_HDRC_CONTROLLER_STOPPED_EVENT 1
> --
> 2.1.1
>