Re: [PATCH 1/1] usb: chipidea: support runtime power management for otg fsm mode
On Mon, Feb 09, 2015 at 11:52:11AM +0800, Li Jun wrote:
> From: Li Jun
>
> This patch adds runtime power management support for otg fsm mode, since
> A-device in a_idle state cannot detect data pulse irq after suspended, here
> enable wakeup by connection before suspend to make it can be resumed by DP;
> and handle wakeup from that state like SRP.
>
> Signed-off-by: Li Jun
> ---
> drivers/usb/chipidea/bits.h|1 +
> drivers/usb/chipidea/core.c| 35 +++
> drivers/usb/chipidea/otg_fsm.c | 22 +++---
> 3 files changed, 55 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/chipidea/bits.h b/drivers/usb/chipidea/bits.h
> index e69424d..3cb9bda 100644
> --- a/drivers/usb/chipidea/bits.h
> +++ b/drivers/usb/chipidea/bits.h
> @@ -63,6 +63,7 @@
> #define PORTSC_HSPBIT(9)
> #define PORTSC_PP BIT(12)
> #define PORTSC_PTC(0x0FUL << 16)
> +#define PORTSC_WKCN BIT(20)
> #define PORTSC_PHCD(d) ((d) ? BIT(22) : BIT(23))
> /* PTS and PTW for non lpm version only */
> #define PORTSC_PFSC BIT(24)
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 4b22d7c..6e18f40 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -861,6 +861,34 @@ static int ci_hdrc_remove(struct platform_device *pdev)
> }
>
> #ifdef CONFIG_PM
> +/* Prepare wakeup by SRP before suspend */
> +static void ci_otg_fsm_suspend_for_srp(struct ci_hdrc *ci)
> +{
> + if ((ci->fsm.otg->state == OTG_STATE_A_IDLE) &&
> + !hw_read_otgsc(ci, OTGSC_ID)) {
> + hw_write(ci, OP_PORTSC, PORTSC_W1C_BITS | PORTSC_PP,
> + PORTSC_PP);
> + hw_write(ci, OP_PORTSC, PORTSC_W1C_BITS | PORTSC_WKCN,
> + PORTSC_WKCN);
> + }
> +}
> +
> +/* Handle SRP when wakeup by data pulse */
> +static void ci_otg_fsm_wakeup_by_srp(struct ci_hdrc *ci)
> +{
> + /* if a_idle wakeup by data pulse, handle it like normal SRP */
Do we need this comment? From the name of function, it is SRP.
> + if ((ci->fsm.otg->state == OTG_STATE_A_IDLE) &&
> + (ci->fsm.a_bus_drop == 1) && (ci->fsm.a_bus_req == 0)) {
> + if (!hw_read_otgsc(ci, OTGSC_ID)) {
> + ci->fsm.a_srp_det = 1;
> + ci->fsm.a_bus_drop = 0;
> + } else {
> + ci->fsm.id = 1;
> + }
> + ci_otg_queue_work(ci);
> + }
> +}
> +
> static void ci_controller_suspend(struct ci_hdrc *ci)
> {
> disable_irq(ci->irq);
> @@ -894,6 +922,8 @@ static int ci_controller_resume(struct device *dev)
> pm_runtime_mark_last_busy(ci->dev);
> pm_runtime_put_autosuspend(ci->dev);
> enable_irq(ci->irq);
> + if (ci_otg_is_fsm_mode(ci))
> + ci_otg_fsm_wakeup_by_srp(ci);
> }
>
> return 0;
> @@ -921,6 +951,8 @@ static int ci_suspend(struct device *dev)
> }
>
> if (device_may_wakeup(dev)) {
> + if (ci_otg_is_fsm_mode(ci))
> + ci_otg_fsm_suspend_for_srp(ci);
Better have a blank line like below.
> usb_phy_set_wakeup(ci->usb_phy, true);
> enable_irq_wake(ci->irq);
> }
> @@ -963,6 +995,9 @@ static int ci_runtime_suspend(struct device *dev)
> return 0;
> }
>
> + if (ci_otg_is_fsm_mode(ci))
> + ci_otg_fsm_suspend_for_srp(ci);
> +
> usb_phy_set_wakeup(ci->usb_phy, true);
> ci_controller_suspend(ci);
>
> diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
> index 562e581..e3cf5be 100644
> --- a/drivers/usb/chipidea/otg_fsm.c
> +++ b/drivers/usb/chipidea/otg_fsm.c
> @@ -225,6 +225,9 @@ static void ci_otg_add_timer(struct ci_hdrc *ci, enum
> ci_otg_fsm_timer_index t)
> return;
> }
>
> + if (list_empty(active_timers))
> + pm_runtime_get(ci->dev);
> +
> timer->count = timer->expires;
> list_add_tail(&timer->list, active_timers);
>
> @@ -241,17 +244,22 @@ static void ci_otg_del_timer(struct ci_hdrc *ci, enum
> ci_otg_fsm_timer_index t)
> struct ci_otg_fsm_timer *tmp_timer, *del_tmp;
> struct ci_otg_fsm_timer *timer = ci->fsm_timer->timer_list[t];
> struct list_head *active_timers = &ci->fsm_timer->active_timers;
> + int flag = 0;
>
> if (t >= NUM_CI_OTG_FSM_TIMERS)
> return;
>
> list_for_each_entry_safe(tmp_timer, del_tmp, active_timers, list)
> - if (tmp_timer == timer)
> + if (tmp_timer == timer) {
> list_del(&timer->list);
> + flag = 1;
> + }
>
> /* Disable 1ms irq if there is no any active timer */
> - if (list_empty(ac
Re: [PATCH V5 6/8] USB: f81232: clarify f81232_ioctl()
Hello, Sergei Shtylyov 於 2015/2/6 下午 08:21 寫道: We extract TIOCGSERIAL section in f81232_ioctl() to f81232_get_serial_info() to make it clarify You're also changing 'ser.baud_rate' from 460800 to 115200. And explicitly overriding some previously initialized to 0 fields. F81232 max baudrate is only 115200bps, so I set it for 1.8432MHz/16 = 115200. We had add some closing time referenced from serial_core.c. The default value is: port->close_delay = HZ / 2; /* .5 seconds */ port->closing_wait= 30 * HZ;/* 30 seconds */ We had increasing close_delay about 10x to port->close_delay = 5 * HZ ; The f81232_set_mctrl() replace set_control_lines() to do MCR control so we clean-up the set_control_lines() function. I don't see where are you doing this... This text is my patch V5 5/8 second section. I had wrong operation of copy & paste. It's doesn't need for this patch, sorry for it. -- With Best Regards, Peter Hung -- 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 2/2] usb: musb: Fix getting a generic phy for musb_dsps
Hi Tony,
On 02/06/2015 10:53 PM, Tony Lindgren wrote:
* George Cherian [150206 05:05]:
Hi Tony,
You also need to add similar things in dsps_musb_reset();
Otherwise you might not recover from a BABBLE condition.
Thank I totally missed that, updated patch below.
Do you have some testcase that easily triggers BABBLE
on MUSB?
On a BBB or BBW you can connect a HUB with multiple device connected on HUB.
Then do a repeated Connect and Disconnect of the HUB, This should
trigger a BABBLE interrupt.
Not all HUB's might not lead you to a BABBLE condition.
Regards,
Tony
8< --
From: Tony Lindgren
Date: Wed, 4 Feb 2015 06:28:49 -0800
Subject: [PATCH] usb: musb: Fix getting a generic phy for musb_dsps
We still have a combination of legacy phys and generic phys in
use so we need to support both types of phy for musb_dsps.c.
Cc: Brian Hutchinson
Signed-off-by: Tony Lindgren
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -457,12 +457,27 @@ static int dsps_musb_init(struct musb *musb)
if (IS_ERR(musb->xceiv))
return PTR_ERR(musb->xceiv);
+ musb->phy = devm_phy_get(dev->parent, "usb2-phy");
+
/* Returns zero if e.g. not clocked */
rev = dsps_readl(reg_base, wrp->revision);
if (!rev)
return -ENODEV;
usb_phy_init(musb->xceiv);
+ if (IS_ERR(musb->phy)) {
+ musb->phy = NULL;
+ } else {
+ ret = phy_init(musb->phy);
+ if (ret < 0)
+ return ret;
+ ret = phy_power_on(musb->phy);
+ if (ret) {
+ phy_exit(musb->phy);
+ return ret;
+ }
+ }
+
setup_timer(&glue->timer, otg_timer, (unsigned long) musb);
/* Reset the musb */
@@ -502,6 +517,8 @@ static int dsps_musb_exit(struct musb *musb)
del_timer_sync(&glue->timer);
usb_phy_shutdown(musb->xceiv);
+ phy_power_off(musb->phy);
+ phy_exit(musb->phy);
debugfs_remove_recursive(glue->dbgfs_root);
return 0;
@@ -610,7 +627,7 @@ static int dsps_musb_reset(struct musb *musb)
struct device *dev = musb->controller;
struct dsps_glue *glue = dev_get_drvdata(dev->parent);
const struct dsps_musb_wrapper *wrp = glue->wrp;
- int session_restart = 0;
+ int session_restart = 0, error;
if (glue->sw_babble_enabled)
session_restart = sw_babble_control(musb);
@@ -624,8 +641,14 @@ static int dsps_musb_reset(struct musb *musb)
dsps_writel(musb->ctrl_base, wrp->control, (1 << wrp->reset));
usleep_range(100, 200);
usb_phy_shutdown(musb->xceiv);
+ error = phy_power_off(musb->phy);
+ if (error)
+ dev_err(dev, "phy shutdown failed: %i\n", error);
usleep_range(100, 200);
usb_phy_init(musb->xceiv);
+ error = phy_power_on(musb->phy);
+ if (error)
+ dev_err(dev, "phy powerup failed: %i\n", error);
session_restart = 1;
}
--
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 1/1] usb: chipidea: support runtime power management for otg fsm mode
From: Li Jun
This patch adds runtime power management support for otg fsm mode, since
A-device in a_idle state cannot detect data pulse irq after suspended, here
enable wakeup by connection before suspend to make it can be resumed by DP;
and handle wakeup from that state like SRP.
Signed-off-by: Li Jun
---
drivers/usb/chipidea/bits.h|1 +
drivers/usb/chipidea/core.c| 35 +++
drivers/usb/chipidea/otg_fsm.c | 22 +++---
3 files changed, 55 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/chipidea/bits.h b/drivers/usb/chipidea/bits.h
index e69424d..3cb9bda 100644
--- a/drivers/usb/chipidea/bits.h
+++ b/drivers/usb/chipidea/bits.h
@@ -63,6 +63,7 @@
#define PORTSC_HSPBIT(9)
#define PORTSC_PP BIT(12)
#define PORTSC_PTC(0x0FUL << 16)
+#define PORTSC_WKCN BIT(20)
#define PORTSC_PHCD(d) ((d) ? BIT(22) : BIT(23))
/* PTS and PTW for non lpm version only */
#define PORTSC_PFSC BIT(24)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 4b22d7c..6e18f40 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -861,6 +861,34 @@ static int ci_hdrc_remove(struct platform_device *pdev)
}
#ifdef CONFIG_PM
+/* Prepare wakeup by SRP before suspend */
+static void ci_otg_fsm_suspend_for_srp(struct ci_hdrc *ci)
+{
+ if ((ci->fsm.otg->state == OTG_STATE_A_IDLE) &&
+ !hw_read_otgsc(ci, OTGSC_ID)) {
+ hw_write(ci, OP_PORTSC, PORTSC_W1C_BITS | PORTSC_PP,
+ PORTSC_PP);
+ hw_write(ci, OP_PORTSC, PORTSC_W1C_BITS | PORTSC_WKCN,
+ PORTSC_WKCN);
+ }
+}
+
+/* Handle SRP when wakeup by data pulse */
+static void ci_otg_fsm_wakeup_by_srp(struct ci_hdrc *ci)
+{
+ /* if a_idle wakeup by data pulse, handle it like normal SRP */
+ if ((ci->fsm.otg->state == OTG_STATE_A_IDLE) &&
+ (ci->fsm.a_bus_drop == 1) && (ci->fsm.a_bus_req == 0)) {
+ if (!hw_read_otgsc(ci, OTGSC_ID)) {
+ ci->fsm.a_srp_det = 1;
+ ci->fsm.a_bus_drop = 0;
+ } else {
+ ci->fsm.id = 1;
+ }
+ ci_otg_queue_work(ci);
+ }
+}
+
static void ci_controller_suspend(struct ci_hdrc *ci)
{
disable_irq(ci->irq);
@@ -894,6 +922,8 @@ static int ci_controller_resume(struct device *dev)
pm_runtime_mark_last_busy(ci->dev);
pm_runtime_put_autosuspend(ci->dev);
enable_irq(ci->irq);
+ if (ci_otg_is_fsm_mode(ci))
+ ci_otg_fsm_wakeup_by_srp(ci);
}
return 0;
@@ -921,6 +951,8 @@ static int ci_suspend(struct device *dev)
}
if (device_may_wakeup(dev)) {
+ if (ci_otg_is_fsm_mode(ci))
+ ci_otg_fsm_suspend_for_srp(ci);
usb_phy_set_wakeup(ci->usb_phy, true);
enable_irq_wake(ci->irq);
}
@@ -963,6 +995,9 @@ static int ci_runtime_suspend(struct device *dev)
return 0;
}
+ if (ci_otg_is_fsm_mode(ci))
+ ci_otg_fsm_suspend_for_srp(ci);
+
usb_phy_set_wakeup(ci->usb_phy, true);
ci_controller_suspend(ci);
diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
index 562e581..e3cf5be 100644
--- a/drivers/usb/chipidea/otg_fsm.c
+++ b/drivers/usb/chipidea/otg_fsm.c
@@ -225,6 +225,9 @@ static void ci_otg_add_timer(struct ci_hdrc *ci, enum
ci_otg_fsm_timer_index t)
return;
}
+ if (list_empty(active_timers))
+ pm_runtime_get(ci->dev);
+
timer->count = timer->expires;
list_add_tail(&timer->list, active_timers);
@@ -241,17 +244,22 @@ static void ci_otg_del_timer(struct ci_hdrc *ci, enum
ci_otg_fsm_timer_index t)
struct ci_otg_fsm_timer *tmp_timer, *del_tmp;
struct ci_otg_fsm_timer *timer = ci->fsm_timer->timer_list[t];
struct list_head *active_timers = &ci->fsm_timer->active_timers;
+ int flag = 0;
if (t >= NUM_CI_OTG_FSM_TIMERS)
return;
list_for_each_entry_safe(tmp_timer, del_tmp, active_timers, list)
- if (tmp_timer == timer)
+ if (tmp_timer == timer) {
list_del(&timer->list);
+ flag = 1;
+ }
/* Disable 1ms irq if there is no any active timer */
- if (list_empty(active_timers))
+ if (list_empty(active_timers) && (flag == 1)) {
hw_write_otgsc(ci, OTGSC_1MSIE, 0);
+ pm_runtime_put(ci->dev);
+ }
}
/*
@@ -275,8 +283,10 @@ static inline int ci_otg_tick_timer(struct ci_hdrc *ci)
}
/* disable 1ms irq if t
Re: [PATCH 4/4] usb: phy: add phy-hi6220
On 9 February 2015 at 09:57, Peter Chen wrote:
>> >> +static int hi6220_phy_probe(struct platform_device *pdev)
>> >> +{
>> >> + struct hi6220_priv *priv;
>> >> + struct usb_otg *otg;
>> >> + struct device_node *np = pdev->dev.of_node;
>> >> + int ret, irq;
>> >> +
>> >> + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
>> >> + if (!priv)
>> >> + return -ENOMEM;
>> >> +
>> >> + otg = devm_kzalloc(&pdev->dev, sizeof(*otg), GFP_KERNEL);
>> >> + if (!otg)
>> >> + return -ENOMEM;
>> >> +
>> >> + priv->phy.dev = &pdev->dev;
>> >> + priv->phy.otg = otg;
>> >> + priv->phy.label = "hi6220";
>> >> + platform_set_drvdata(pdev, priv);
>> >> + otg->set_peripheral = mv_otg_set_peripheral;
>> >> +
>> >> + priv->gpio_vbus_det = of_get_named_gpio(np,
>> >> "hisilicon,gpio_vbus_det", 0);
>> >> + if (priv->gpio_vbus_det == -EPROBE_DEFER)
>> >> + return -EPROBE_DEFER;
>> >> + if (!gpio_is_valid(priv->gpio_vbus_det)) {
>> >> + dev_err(&pdev->dev, "invalid gpio %d\n",
>> >> priv->gpio_vbus_det);
>> >> + return -ENODEV;
>> >> + }
>> >> +
>> >> + priv->gpio_id_det = of_get_named_gpio(np, "hisilicon,gpio_id_det",
>> >> 0);
>> >> + if (priv->gpio_id_det == -EPROBE_DEFER)
>> >> + return -EPROBE_DEFER;
>> >> + if (!gpio_is_valid(priv->gpio_id_det)) {
>> >> + dev_err(&pdev->dev, "invalid gpio %d\n", priv->gpio_id_det);
>> >> + return -ENODEV;
>> >> + }
>> >> +
>> >> + priv->reg = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
>> >> + "hisilicon,peripheral-syscon");
>> >> + if (IS_ERR(priv->reg))
>> >> + priv->reg = NULL;
>> >
>> > You may differentiate -ENODEV and other errors, for other errors, you
>> > can show an error, and return directly.
>>
>> Here I want to set this property as optional, in case other platform
>> do not need this property.
>> So phy_setup also add protection if (priv->reg == NULL) return;
>>
>
> If syscon_regmap_lookup_by_phandle returns -EPROBE_DEFER, you may want
> to try later.
>
It should not.
syscon is postcore_initcall(syscon_init);
Thanks
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/6] drivers: usb: core: devio.c: remove assignment of variables in if conditions.
On Sat, Feb 07, 2015 at 10:55:01PM +0100, Bas Peters wrote:
> This patch removes assignment of variables in if conditions in
> accordance witht the CodingStyle.
%s/witht/with
>
> Signed-off-by: Bas Peters
> ---
> drivers/usb/core/devio.c | 15 ++-
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
> index 0b59731..ea3c737 100644
> --- a/drivers/usb/core/devio.c
> +++ b/drivers/usb/core/devio.c
> @@ -1081,7 +1081,8 @@ static int proc_bulk(struct usb_dev_state *ps, void
> __user *arg)
> ret = usbfs_increase_memory_usage(len1 + sizeof(struct urb));
> if (ret)
> return ret;
> - if (!(tbuf = kmalloc(len1, GFP_KERNEL))) {
> + tbuf = kmalloc(len1, GFP_KERNEL);
> + if (!tbuf) {
> ret = -ENOMEM;
> goto done;
> }
> @@ -1223,7 +1224,8 @@ static int proc_setintf(struct usb_dev_state *ps, void
> __user *arg)
>
> if (copy_from_user(&setintf, arg, sizeof(setintf)))
> return -EFAULT;
> - if ((ret = checkintf(ps, setintf.interface)))
> + ret = checkintf(ps, setintf.interface);
> + if (ret)
> return ret;
>
> destroy_async_on_interface(ps, setintf.interface);
> @@ -1392,7 +1394,8 @@ static int proc_do_submiturb(struct usb_dev_state *ps,
> struct usbdevfs_urb *uurb
> number_of_packets = uurb->number_of_packets;
> isofrmlen = sizeof(struct usbdevfs_iso_packet_desc) *
> number_of_packets;
> - if (!(isopkt = kmalloc(isofrmlen, GFP_KERNEL)))
> + isopkt = kmalloc(isofrmlen, GFP_KERNEL);
> + if (!isopkt)
> return -ENOMEM;
> if (copy_from_user(isopkt, iso_frame_desc, isofrmlen)) {
> ret = -EFAULT;
> @@ -1901,7 +1904,8 @@ static int proc_releaseinterface(struct usb_dev_state
> *ps, void __user *arg)
>
> if (get_user(ifnum, (unsigned int __user *)arg))
> return -EFAULT;
> - if ((ret = releaseintf(ps, ifnum)) < 0)
> + ret = releaseintf(ps, ifnum);
> + if (ret < 0)
> return ret;
> destroy_async_on_interface (ps, ifnum);
> return 0;
> @@ -1916,7 +1920,8 @@ static int proc_ioctl(struct usb_dev_state *ps, struct
> usbdevfs_ioctl *ctl)
> struct usb_driver *driver = NULL;
>
> /* alloc buffer */
> - if ((size = _IOC_SIZE(ctl->ioctl_code)) > 0) {
> + size = _IOC_SIZE(ctl->ioctl_code);
> + if (size > 0) {
> buf = kmalloc(size, GFP_KERNEL);
> if (buf == NULL)
> return -ENOMEM;
> --
> 2.1.0
>
--
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/4] usb: phy: add phy-hi6220-usb
On Sat, Feb 07, 2015 at 12:56:06PM +0800, Zhangfei Gao wrote:
> Add usb phy controller for hi6220 platform
>
> Signed-off-by: Zhangfei Gao
> ---
> drivers/usb/phy/Kconfig | 9 ++
> drivers/usb/phy/Makefile | 1 +
> drivers/usb/phy/phy-hi6220-usb.c | 297
> +++
> 3 files changed, 307 insertions(+)
> create mode 100644 drivers/usb/phy/phy-hi6220-usb.c
>
> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
> index c6d0c8e..405a3d0 100644
> --- a/drivers/usb/phy/Kconfig
> +++ b/drivers/usb/phy/Kconfig
> @@ -173,6 +173,15 @@ config USB_MXS_PHY
>
> MXS Phy is used by some of the i.MX SoCs, for example imx23/28/6x.
>
> +config USB_HI6220_PHY
> + tristate "hi6220 USB PHY support"
> + select USB_PHY
> + select MFD_SYSCON
> + help
> + Enable this to support the HISILICON HI6220 USB PHY.
> +
> + To compile this driver as a module, choose M here.
> +
> config USB_RCAR_PHY
> tristate "Renesas R-Car USB PHY support"
> depends on USB || USB_GADGET
> diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
> index 75f2bba..00172d3 100644
> --- a/drivers/usb/phy/Makefile
> +++ b/drivers/usb/phy/Makefile
> @@ -18,6 +18,7 @@ obj-$(CONFIG_SAMSUNG_USBPHY)+=
> phy-samsung-usb.o
> obj-$(CONFIG_TWL6030_USB)+= phy-twl6030-usb.o
> obj-$(CONFIG_USB_EHCI_TEGRA) += phy-tegra-usb.o
> obj-$(CONFIG_USB_GPIO_VBUS) += phy-gpio-vbus-usb.o
> +obj-$(CONFIG_USB_HI6220_PHY) += phy-hi6220-usb.o
> obj-$(CONFIG_USB_ISP1301)+= phy-isp1301.o
> obj-$(CONFIG_USB_MSM_OTG)+= phy-msm-usb.o
> obj-$(CONFIG_USB_MV_OTG) += phy-mv-usb.o
> diff --git a/drivers/usb/phy/phy-hi6220-usb.c
> b/drivers/usb/phy/phy-hi6220-usb.c
> new file mode 100644
> index 000..8092bca
> --- /dev/null
> +++ b/drivers/usb/phy/phy-hi6220-usb.c
> @@ -0,0 +1,297 @@
> +/*
> + * Copyright (c) 2015 Linaro Ltd.
> + * Copyright (c) 2015 Hisilicon Limited.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define SC_PERIPH_CTRL4 0x00c
> +
> +#define CTRL4_PICO_SIDDQ BIT(6)
> +#define CTRL4_PICO_OGDISABLE BIT(8)
> +#define CTRL4_PICO_VBUSVLDEXTBIT(10)
> +#define CTRL4_PICO_VBUSVLDEXTSEL BIT(11)
> +#define CTRL4_OTG_PHY_SELBIT(21)
> +
> +#define SC_PERIPH_CTRL5 0x010
> +
> +#define CTRL5_USBOTG_RES_SEL BIT(3)
> +#define CTRL5_PICOPHY_ACAENB BIT(4)
> +#define CTRL5_PICOPHY_BC_MODEBIT(5)
> +#define CTRL5_PICOPHY_CHRGSELBIT(6)
> +#define CTRL5_PICOPHY_VDATSRCEND BIT(7)
> +#define CTRL5_PICOPHY_VDATDETENB BIT(8)
> +#define CTRL5_PICOPHY_DCDENB BIT(9)
> +#define CTRL5_PICOPHY_IDDIG BIT(10)
> +
> +#define SC_PERIPH_CTRL8 0x018
> +#define SC_PERIPH_RSTEN0 0x300
> +#define SC_PERIPH_RSTDIS00x304
> +
> +#define RST0_USBOTG_BUS BIT(4)
> +#define RST0_POR_PICOPHY BIT(5)
> +#define RST0_USBOTG BIT(6)
> +#define RST0_USBOTG_32K BIT(7)
> +
> +#define EYE_PATTERN_PARA 0x7053348c
> +
> +struct hi6220_priv {
> + struct usb_phy phy;
> + struct delayed_work work;
> + struct regmap *reg;
> + struct clk *clk;
> + struct regulator *vcc;
> + int gpio_vbus;
> + int gpio_id;
> + enum usb_otg_state state;
> +};
> +
> +static void hi6220_start_periphrals(struct hi6220_priv *priv, bool on)
> +{
> + struct usb_otg *otg = priv->phy.otg;
> +
> + if (!otg->gadget)
> + return;
> +
> + if (on)
> + usb_gadget_connect(otg->gadget);
> + else
> + usb_gadget_disconnect(otg->gadget);
> +}
> +
> +static void hi6220_detect_work(struct work_struct *work)
> +{
> + struct hi6220_priv *priv =
> + container_of(work, struct hi6220_priv, work.work);
> + int gpio_id, gpio_vubs;
%s/gpio_vubs/gpio_vbus
> + enum usb_otg_state state;
> +
> + if (!gpio_is_valid(priv->gpio_id) || !gpio_is_valid(priv->gpio_vbus))
> + return;
> +
> + gpio_id = gpio_get_value_cansleep(priv->gpio_id);
> + gpio_vubs = gpio_get_value_cansleep(priv->gpio_vbus);
> +
> + if (gpio_vubs == 0) {
> + if (gpio_id == 1)
> + state = OTG_STATE_B_PERIPHERAL;
> + else
> + state = OTG_STATE_A_HOST;
> + } else {
> + state = OTG_STATE_A_HOST;
> + }
> +
> + if (priv->state != state) {
> + hi6220_s
Re: [PATCH 4/4] usb: phy: add phy-hi6220
On Fri, Feb 06, 2015 at 07:47:12PM +0800, Zhangfei Gao wrote:
> On 6 February 2015 at 16:41, Peter Chen wrote:
> > On Thu, Feb 05, 2015 at 10:47:00PM +0800, Zhangfei Gao wrote:
>
> >> @@ -18,6 +18,7 @@ obj-$(CONFIG_SAMSUNG_USBPHY)+=
> >> phy-samsung-usb.o
> >> obj-$(CONFIG_TWL6030_USB)+= phy-twl6030-usb.o
> >> obj-$(CONFIG_USB_EHCI_TEGRA) += phy-tegra-usb.o
> >> obj-$(CONFIG_USB_GPIO_VBUS) += phy-gpio-vbus-usb.o
> >> +obj-$(CONFIG_USB_HI6220_PHY) += phy-hi6220.o
> >
> > To align the naming method, phy-hi6220-usb is better.
> Sure,
>
>
> >> +enum usb_mode {
> >> + USB_EMPTY,
> >> + GADGET_DEVICE,
> >> + OTG_HOST,
> >> +};
> >
> > This usb_mode is a little strange, what state you would like to
> > use?
>
> it is internal state machine, to distinguish otg gadget mode and host mode.
> There are two gpio, we use gpio_vbus interrupt as well as gpio_id
> status to distinguish gadget or host.
>
> >> +static irqreturn_t hiusb_gpio_intr(int irq, void *data)
> >> +{
> >> + struct hi6220_priv *priv = (struct hi6220_priv *)data;
> >> +
> >> + /* add debounce time */
> >> + schedule_delayed_work(&priv->work, msecs_to_jiffies(100));
> >> + return IRQ_HANDLED;
> >> +}
> >> +
> >> +static int mv_otg_set_peripheral(struct usb_otg *otg,
> >
> > mv? You may want to use hi
> Yes, my bad.
>
>
> >> +static void hi6220_phy_setup(struct hi6220_priv *priv)
> >> +{
> >> + u32 val, mask;
> >> + int ret;
> >> +
> >> + if (priv->reg == NULL)
> >> + return;
> >> +
> >> + val = PERIPH_RSTDIS0_USBOTG_BUS | PERIPH_RSTDIS0_POR_PICOPHY |
> >> + PERIPH_RSTDIS0_USBOTG | PERIPH_RSTDIS0_USBOTG_32K;
> >> + mask = val;
> >> + ret = regmap_update_bits(priv->reg, SC_PERIPH_RSTDIS0, mask, val);
> >> + if (ret)
> >> + return;
> >> +
> >> + ret = regmap_read(priv->reg, SC_PERIPH_CTRL5, &val);
> >> + val = PERIPH_CTRL5_USBOTG_RES_SEL | PERIPH_CTRL5_PICOPHY_ACAENB;
> >> + mask = val | PERIPH_CTRL5_PICOPHY_BC_MODE;
> >> + ret = regmap_update_bits(priv->reg, SC_PERIPH_CTRL5, mask, val);
> >> + if (ret)
> >> + return;
> >> +
> >> + val = PERIPH_CTRL4_PICO_VBUSVLDEXT |
> >> PERIPH_CTRL4_PICO_VBUSVLDEXTSEL |
> >> +PERIPH_CTRL4_OTG_PHY_SEL;
> >> + mask = val | PERIPH_CTRL4_PICO_SIDDQ | PERIPH_CTRL4_PICO_OGDISABLE;
> >> + ret = regmap_update_bits(priv->reg, SC_PERIPH_CTRL4, mask, val);
> >> + if (ret)
> >> + return;
> >> +
> >> + ret = regmap_write(priv->reg, SC_PERIPH_CTRL8, EYE_PATTERN_PARA);
> >> + if (ret)
> >> + return;
> >> +}
> >> +
> >> +static int hi6220_phy_probe(struct platform_device *pdev)
> >> +{
> >> + struct hi6220_priv *priv;
> >> + struct usb_otg *otg;
> >> + struct device_node *np = pdev->dev.of_node;
> >> + int ret, irq;
> >> +
> >> + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> >> + if (!priv)
> >> + return -ENOMEM;
> >> +
> >> + otg = devm_kzalloc(&pdev->dev, sizeof(*otg), GFP_KERNEL);
> >> + if (!otg)
> >> + return -ENOMEM;
> >> +
> >> + priv->phy.dev = &pdev->dev;
> >> + priv->phy.otg = otg;
> >> + priv->phy.label = "hi6220";
> >> + platform_set_drvdata(pdev, priv);
> >> + otg->set_peripheral = mv_otg_set_peripheral;
> >> +
> >> + priv->gpio_vbus_det = of_get_named_gpio(np,
> >> "hisilicon,gpio_vbus_det", 0);
> >> + if (priv->gpio_vbus_det == -EPROBE_DEFER)
> >> + return -EPROBE_DEFER;
> >> + if (!gpio_is_valid(priv->gpio_vbus_det)) {
> >> + dev_err(&pdev->dev, "invalid gpio %d\n",
> >> priv->gpio_vbus_det);
> >> + return -ENODEV;
> >> + }
> >> +
> >> + priv->gpio_id_det = of_get_named_gpio(np, "hisilicon,gpio_id_det",
> >> 0);
> >> + if (priv->gpio_id_det == -EPROBE_DEFER)
> >> + return -EPROBE_DEFER;
> >> + if (!gpio_is_valid(priv->gpio_id_det)) {
> >> + dev_err(&pdev->dev, "invalid gpio %d\n", priv->gpio_id_det);
> >> + return -ENODEV;
> >> + }
> >> +
> >> + priv->reg = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
> >> + "hisilicon,peripheral-syscon");
> >> + if (IS_ERR(priv->reg))
> >> + priv->reg = NULL;
> >
> > You may differentiate -ENODEV and other errors, for other errors, you
> > can show an error, and return directly.
>
> Here I want to set this property as optional, in case other platform
> do not need this property.
> So phy_setup also add protection if (priv->reg == NULL) return;
>
If syscon_regmap_lookup_by_phandle returns -EPROBE_DEFER, you may want
to try later.
Peter
> >
> >> +
> >> + INIT_DELAYED_WORK(&priv->work, hi6220_detect_work);
> >> +
> >> + ret = devm_gpio_request_one(&pdev->dev, priv->gpio_vbus_det,
> >> + GPIOF_IN, "gpio_vbus_det");
> >> + if (ret < 0) {
> >> +
Re: [PATCH v2] usb: core: Remove unneeded #ifdef and associated dead code
On Sunday, February 08, 2015 08:36:38 PM Andreas Ruprecht wrote:
> In commit ceb6c9c862c8 ("USB / PM: Drop CONFIG_PM_RUNTIME from the
> USB core"), all occurrences of CONFIG_PM_RUNTIME in the USB core
> code were replaced by CONFIG_PM. This created the following structure
> of #ifdef blocks in drivers/usb/core/hub.c:
>
> [...]
> #ifdef CONFIG_PM
> #ifdef CONFIG_PM
> /* always on / undead */
> #else
> /* dead */
> #endif
> [...]
>
> This patch removes unnecessary inner "#ifdef CONFIG_PM" as well as
> the corresponding dead #else block. This inconsistency was found using
> the undertaker-checkpatch tool.
>
> Signed-off-by: Andreas Ruprecht
Applied, thanks!
> ---
> Changes to v1:
> - Better description of what was removed in the commit message
>
> drivers/usb/core/hub.c | 12
> 1 file changed, 12 deletions(-)
>
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 3e9c4d4..c362bbc 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -3452,8 +3452,6 @@ int usb_port_resume(struct usb_device *udev,
> pm_message_t msg)
> return status;
> }
>
> -#ifdef CONFIG_PM
> -
> int usb_remote_wakeup(struct usb_device *udev)
> {
> int status = 0;
> @@ -3512,16 +3510,6 @@ static int hub_handle_remote_wakeup(struct usb_hub
> *hub, unsigned int port,
> return connect_change;
> }
>
> -#else
> -
> -static int hub_handle_remote_wakeup(struct usb_hub *hub, unsigned int port,
> - u16 portstatus, u16 portchange)
> -{
> - return 0;
> -}
> -
> -#endif
> -
> static int check_ports_changed(struct usb_hub *hub)
> {
> int port1;
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
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: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")
On Mon, 19 Jan 2015, Alan Stern wrote:
...
If anyone (still?) cares about this bug, commit 3a9794d3 ("sd: Fix max
transfer length for 4k disks") fixes it, with no patches required.
-Kenny
--
Kenneth R. Crudup Sr. SW Engineer, Scott County Consulting, Silicon Valley
--
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 v2] usb: core: Remove unneeded #ifdef and associated dead code
In commit ceb6c9c862c8 ("USB / PM: Drop CONFIG_PM_RUNTIME from the
USB core"), all occurrences of CONFIG_PM_RUNTIME in the USB core
code were replaced by CONFIG_PM. This created the following structure
of #ifdef blocks in drivers/usb/core/hub.c:
[...]
#ifdef CONFIG_PM
#ifdef CONFIG_PM
/* always on / undead */
#else
/* dead */
#endif
[...]
This patch removes unnecessary inner "#ifdef CONFIG_PM" as well as
the corresponding dead #else block. This inconsistency was found using
the undertaker-checkpatch tool.
Signed-off-by: Andreas Ruprecht
---
Changes to v1:
- Better description of what was removed in the commit message
drivers/usb/core/hub.c | 12
1 file changed, 12 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 3e9c4d4..c362bbc 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3452,8 +3452,6 @@ int usb_port_resume(struct usb_device *udev, pm_message_t
msg)
return status;
}
-#ifdef CONFIG_PM
-
int usb_remote_wakeup(struct usb_device *udev)
{
int status = 0;
@@ -3512,16 +3510,6 @@ static int hub_handle_remote_wakeup(struct usb_hub *hub,
unsigned int port,
return connect_change;
}
-#else
-
-static int hub_handle_remote_wakeup(struct usb_hub *hub, unsigned int port,
- u16 portstatus, u16 portchange)
-{
- return 0;
-}
-
-#endif
-
static int check_ports_changed(struct usb_hub *hub)
{
int port1;
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] usb: gadget: udc-core: independent registration of gadgets and gadget drivers
Hi Alan,
On Thu, Jan 29, 2015 at 5:56 PM, Alan Stern wrote:
> On Thu, 29 Jan 2015, Ruslan Bilovol wrote:
>
>> Change behavior during registration of gadgets and
>> gadget drivers in udc-core. Instead of previous
>> approach when for successful probe of usb gadget driver
>> at least one usb gadget should be already registered
>> use another one where gadget drivers and gadgets
>> can be registered in udc-core independently.
>>
>> Independent registration of gadgets and gadget drivers
>> is useful for built-in into kernel gadget and gadget
>> driver case - because it's possible that gadget is
>> really probed only on late_init stage (due to deferred
>> probe) whereas gadget driver's probe is silently failed
>> on module_init stage due to no any UDC added.
>>
>> Also it is useful for modules case - now there is no
>> difference what module to insert first: gadget module
>> or gadget driver one.
>
> It's possible to do all this much more simply. In fact, I posted a
> patch some time ago to do exactly this (but I can't find a copy of it
> anywhere).
Unfortunately I didn't find your patch.
>
>> Signed-off-by: Ruslan Bilovol
>> ---
>> drivers/usb/gadget/udc/udc-core.c | 113
>> +++---
>> 1 file changed, 105 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/udc/udc-core.c
>> b/drivers/usb/gadget/udc/udc-core.c
>> index e31d574..4c9412b 100644
>> --- a/drivers/usb/gadget/udc/udc-core.c
>> +++ b/drivers/usb/gadget/udc/udc-core.c
>> @@ -43,13 +43,23 @@ struct usb_udc {
>> struct usb_gadget_driver*driver;
>> struct usb_gadget *gadget;
>> struct device dev;
>> + boolbind_by_name;
>> + struct list_headlist;
>> +};
>> +
>> +struct pending_gadget_driver {
>> + struct usb_gadget_driver*driver;
>> + char*udc_name;
>> struct list_headlist;
>> };
>
> You don't need all this stuff. What's the point of keeping track of
> names? If there are any unbound gadget drivers pending, a newly
> registered UDC should bind to the first one available.
It's because gadget driver may be bound to usb_gadget in two ways:
- standard way - in this case any available udc will be picked up
- by name of udc, in this case only matching udc will be picked up
Main feature of my path is not only deferred binding of gadget driver,
but also possibility to register/unregister udc at any time.
This is useful for user who can load, for example, udc module
if needed and unload it safely, not touching gadget driver.
Another example is USB device controllers that consist of pair of
HS+SS controllers, each one having its own udc driver. In this case
it's possible to switch SS/HS by registering/unregistering corresponding
udc and not touching gadget driver.
I did all of this inside of udc-core because it looks like to be best place for
udc <-> gadget driver housekeeping. Also it is verified on lot of combinations
of udc and gadget drivers that can be built-in or build as modules
Best regards,
Ruslan
>
> Just add a "pending" list_head into the usb_gadget_driver structure and
> forget about all the rest. (Or try to find my patch in the mailing
> list archives somehow see if you think it needs to be changed.)
>
> Alan Stern
>
--
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] usb: phy: am335x-control: check return value of bus_find_device
On 02/08/2015 04:29 PM, David Dueck wrote:
> This fixes a potential null pointer dereference.
>
> Signed-off-by: David Dueck
Acked-by: Sebastian Andrzej Siewior
Fixes: d4332013919a ("driver core: dev_get_drvdata: Don't check for NULL
dev")
Greg, this is a regression since d43320139 ("driver core:
dev_get_drvdata: Don't check for NULL dev"). I didn't check for NULL
after bus_find_device() because I knew that dev_get_drvdata() will do
it.
> ---
> drivers/usb/phy/phy-am335x-control.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/usb/phy/phy-am335x-control.c
> b/drivers/usb/phy/phy-am335x-control.c
> index 403fab7..7b3035f 100644
> --- a/drivers/usb/phy/phy-am335x-control.c
> +++ b/drivers/usb/phy/phy-am335x-control.c
> @@ -126,6 +126,9 @@ struct phy_control *am335x_get_phy_control(struct device
> *dev)
> return NULL;
>
> dev = bus_find_device(&platform_bus_type, NULL, node, match);
> + if (!dev)
> + return NULL;
> +
> ctrl_usb = dev_get_drvdata(dev)
> if (!ctrl_usb)
> return NULL;
>
Sebastian
--
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] usb: gadget: fix sparse warnings
Hi Felipe, On Thursday 05 February 2015 12:08:09 Felipe Balbi wrote: > On Thu, Feb 05, 2015 at 05:02:46PM +0200, Laurent Pinchart wrote: > > Hi Prabhakar, > > > > Thank you for the patch. > > > > On Thursday 05 February 2015 13:02:18 Lad Prabhakar wrote: > > > From: "Lad, Prabhakar" > > > > > > this patch fixes following sparse warnings: > > > > > > uvc_video.c:283:5: warning: symbol 'uvcg_video_pump' was not declared. > > > Should it be static? uvc_video.c:342:5: warning: symbol > > > 'uvcg_video_enable' > > > was not declared. Should it be static? uvc_video.c:381:5: warning: > > > symbol > > > 'uvcg_video_init' was not declared. Should it be static? > > > > > > Signed-off-by: Lad, Prabhakar > > > > Acked-by: Laurent Pinchart > > > > Felipe, could you please take this in your tree ? > > my tree is closed for v3.20. I'll pick it up once -rc1 is out That's good, I was targeting v3.21 too. How do you usually ensure that patches don't get lost, do you apply them to a n+1 branch straight away (which is what I was asking in my previous mail), rely on patchwork or some similar tool, or expect developers to ping you again when -rc1 is out ? -- Regards, Laurent Pinchart -- 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] usb: phy: am335x-control: check return value of bus_find_device
This fixes a potential null pointer dereference. Signed-off-by: David Dueck --- drivers/usb/phy/phy-am335x-control.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/usb/phy/phy-am335x-control.c b/drivers/usb/phy/phy-am335x-control.c index 403fab7..7b3035f 100644 --- a/drivers/usb/phy/phy-am335x-control.c +++ b/drivers/usb/phy/phy-am335x-control.c @@ -126,6 +126,9 @@ struct phy_control *am335x_get_phy_control(struct device *dev) return NULL; dev = bus_find_device(&platform_bus_type, NULL, node, match); + if (!dev) + return NULL; + ctrl_usb = dev_get_drvdata(dev); if (!ctrl_usb) return NULL; -- 2.2.2 -- 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 2/3] drivers: usb: storage: cypress_atacb.c: trivial checkpatch fixes
On 2/8/2015 1:42 AM, Bas Peters wrote:
Fixes errors thrown by checkpatch over a space issue and the
incorrect indentation of a switch statement.
Signed-off-by: Bas Peters
---
drivers/usb/storage/cypress_atacb.c | 17 -
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/storage/cypress_atacb.c
b/drivers/usb/storage/cypress_atacb.c
index 8514a2d..b3466d1 100644
--- a/drivers/usb/storage/cypress_atacb.c
+++ b/drivers/usb/storage/cypress_atacb.c
@@ -96,13 +96,13 @@ static void cypress_atacb_passthrough(struct scsi_cmnd
*srb, struct us_data *us)
if (save_cmnd[1] >> 5) /* MULTIPLE_COUNT */
goto invalid_fld;
/* check protocol */
- switch((save_cmnd[1] >> 1) & 0xf) {
- case 3: /*no DATA */
- case 4: /* PIO in */
- case 5: /* PIO out */
- break;
- default:
- goto invalid_fld;
+ switch ((save_cmnd[1] >> 1) & 0xf) {
+ case 3: /*no DATA */
Could also add space after /*, while at it.
+ case 4: /* PIO in */
+ case 5: /* PIO out */
+ break;
+ default:
+ goto invalid_fld;
}
/* first build the ATACB command */
[...]
WBR, Sergei
--
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 5/6] drivers: usb: core: hub.c: remove assignment of variables in if conditions.
On 2/8/2015 12:55 AM, Bas Peters wrote: As specified in the CodingStyle. Signed-off-by: Bas Peters --- drivers/usb/core/hub.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 82983d9..9afe8b0 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -671,8 +671,8 @@ resubmit: if (hub->quiescing) return; - if ((status = usb_submit_urb (hub->urb, GFP_ATOMIC)) != 0 - && status != -ENODEV && status != -EPERM) + status = usb_submit_urb (hub->urb, GFP_ATOMIC); checkpatch.pl should also complain about space before (. [...] WBR, Sergei -- 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 4/6] drivers: usb: core: hub.c: remove NULL initialization of static variables.
On 2/8/2015 12:55 AM, Bas Peters wrote: NULL Rather 0-. initialization of static variables is unnecessary as GCC kindly does this for us. It's rather the C run-time library that does this. Signed-off-by: Bas Peters [...] WBR, Sergei -- 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 3/6] drivers: usb: core: hcd.c: remove assignment of variables in if conditions.
Hello.
On 2/8/2015 12:55 AM, Bas Peters wrote:
This patch removes assignment of variables in if conditions,
as specified in CodingStyle.
Signed-off-by: Bas Peters
---
drivers/usb/core/hcd.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 11cee55..37c40d1 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
[...]
@@ -2733,7 +2736,8 @@ int usb_add_hcd(struct usb_hcd *hcd,
/* "reset" is misnamed; its role is now one-time init. the controller
* should already have been reset (and boot firmware kicked off etc).
*/
- if (hcd->driver->reset && (retval = hcd->driver->reset(hcd)) < 0) {
+ retval = hcd->driver->reset(hcd);
This will crash if 'hcd->driver->reset' is NULL (which is only checked
below).
+ if (hcd->driver->reset && retval < 0) {
It wasn't equivalent change anyway as the right part of && is only
executed if the left part is true.
WBR, Sergei
--
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: [PATCHv6 4/5] USB: gadget: atmel_usba_udc: Prepare for IRQ single edge support
On Sat, 7 Feb 2015 20:37:23 +0100
Sylvain Rochet wrote:
> Hello Nicolas,
>
>
> On Thu, Feb 05, 2015 at 06:19:55PM +0100, Nicolas Ferre wrote:
> > Le 22/01/2015 18:14, Boris Brezillon a écrit :
> > > On Thu, 22 Jan 2015 17:56:44 +0100
> > > Sylvain Rochet wrote:
> > >
> > > > -static const struct usba_udc_errata at91sam9g45_errata = {
> > > > +static const struct usba_udc_caps at91sam9g45_caps = {
> > > > .pulse_bias = at91sam9g45_pulse_bias,
> > > > + .irq_single_edge_support = true,
> >
> > Nope! at91sam9g45 doesn't have this property. You'll have to create
> > another compatible string and capabilities structure
> > ("atmel,at91sam9x5-udc")
>
> Oops.
>
>
> > But still, I don't know if it's the proper approach. The possibility to
> > trigger an IRQ on both edges or a single edge is a capacity of the gpio
> > controller, not the USBA IP. So, it's a little bit strange to have this
> > capability here.
>
> I agree.
Me too (even if I'm the one who proposed this approach in the first
place ;-)).
>
>
> > I don't know if it's actually feasible but trying to configure the IRQ
> > on a single edge, testing if it's accepted by the GPIO irq controller
> > and if not, falling back to a "both edge" pattern. Doesn't it look like
> > a way to workaround this issue nicely? Can you give it a try?
>
> Tried, it works, but it displays the following message from
> __irq_set_trigger() [1] during devm_request_threaded_irq(…,
> IRQF_TRIGGER_RISING, …) on boards which does not support single-edge
> IRQ:
>
> genirq: Setting trigger mode 1 for irq 176 failed (gpio_irq_type+0x0/0x34)
>
> Is it acceptable ?
IMHO it's not.
> If not, is udc->caps->irq_single_edge_support boolean acceptable ?
Do you mean keeping the current approach ?
If you do, then maybe you can rework a bit the way you detect the GPIO
controller you depends on: instead of linking this information to the
usba compatible string you could link it to the gpio controller
compatible string.
You can find the gpio controller node thanks to your "vbus-gpio"
property: use the phandle defined in this property to find the gpio
controller node, and once you have the device_node referencing the gpio
controller you can match it with your internal device_id table
(containing 2 entries: one for the at91rm9200 IP and the other for the
at91sam9x5 IP).
Another solution would be to add an irq_try_set_irq_type function that
would not complain when it fails to set the requested trigger.
Thomas, I know you did not follow the whole thread, but would you mind
adding this irq_try_set_irq_type function (here is a reference
implementation [1]), to prevent this error trace from happening when
we're just trying a configuration ?
> If not, I am ok to drop the feature, this is only a bonus.
That could be a short term solution, to get this series accepted. We
could then find a proper way to support that optimization.
Best Regards,
Boris
[1]http://code.bulix.org/z4bu9k-87846
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
