RE: [PATCH v13 08/10] usbip: exporting devices: change to usbip_list.c
Hello,
> > + rc = get_importable_devices(host, sockfd);
> > if (rc < 0) {
> > err("failed to get device list from %s", host);
> > + close(sockfd);
> > return -1;
> > }
> Why not just close before if?
I will move before if.
> After fixing suggestion about placement of close() function you may
> add my:
>
> Reviewed-by: Krzysztof Opasiak
Sure.
> In addition I think that this patch should be send separately before
> this whole series.
After this set is re-arranged. I will remind it.
I may include this patch because (it can be said that) this caused by
introducing export request.
Thank you for your help,
n.iwata
//
> -Original Message-
> From: Krzysztof Opasiak [mailto:k.opas...@samsung.com]
> Sent: Saturday, December 03, 2016 12:53 AM
> To: fx IWATA NOBUO; valentina.mane...@gmail.com; shuah...@samsung.com;
> gre...@linuxfoundation.org; linux-usb@vger.kernel.org
> Cc: fx MICHIMURA TADAO
> Subject: Re: [PATCH v13 08/10] usbip: exporting devices: change to
> usbip_list.c
>
>
>
> On 11/22/2016 07:48 AM, Nobuo Iwata wrote:
> > Correction to wording inconsistency around import and export in
> > usbip_list.c regarding output title, help and function names.
> >
> > 'exported' was used for devices bound in remote and to be attached
> > with 'import' request. This patch set uses pre-defined 'export'
> > request to connect device.
> >
> > To avoid mixed usage of 'export', 'importable' is used for devices to
> > be attached with 'import' request.
> >
> > The word 'imported' has already been used in output of port operation.
> > It is consistent to this patch.
> >
>
> After fixing suggestion about placement of close() function you may add
> my:
>
> Reviewed-by: Krzysztof Opasiak
>
> In addition I think that this patch should be send separately before this
> whole series.
>
> > Signed-off-by: Nobuo Iwata
> > ---
> > tools/usb/usbip/src/usbip_list.c | 17 +
> > 1 file changed, 9 insertions(+), 8 deletions(-)
> >
> > diff --git a/tools/usb/usbip/src/usbip_list.c
> > b/tools/usb/usbip/src/usbip_list.c
> > index f1b38e8..cf69f31 100644
> > --- a/tools/usb/usbip/src/usbip_list.c
> > +++ b/tools/usb/usbip/src/usbip_list.c
> > @@ -44,7 +44,7 @@
> > static const char usbip_list_usage_string[] =
> > "usbip list [-p|--parsable] \n"
> > "-p, --parsable Parsable list format\n"
> > - "-r, --remote=List the exportable USB devices on
> \n"
> > + "-r, --remote=List the importable USB devices on
> \n"
> > "-l, --localList the local USB devices\n";
> >
> > void usbip_list_usage(void)
> > @@ -52,7 +52,7 @@ void usbip_list_usage(void)
> > printf("usage: %s", usbip_list_usage_string); }
> >
> > -static int get_exported_devices(char *host, int sockfd)
> > +static int get_importable_devices(char *host, int sockfd)
> > {
> > char product_name[100];
> > char class_name[100];
> > @@ -82,14 +82,14 @@ static int get_exported_devices(char *host, int
> sockfd)
> > return -1;
> > }
> > PACK_OP_DEVLIST_REPLY(0, &reply);
> > - dbg("exportable devices: %d\n", reply.ndev);
> > + dbg("importable devices: %d\n", reply.ndev);
> >
> > if (reply.ndev == 0) {
> > - info("no exportable devices found on %s", host);
> > + info("no importable devices found on %s", host);
> > return 0;
> > }
> >
> > - printf("Exportable USB devices\n");
> > + printf("Importable USB devices\n");
> > printf("==\n");
> > printf(" - %s\n", host);
> >
> > @@ -134,7 +134,7 @@ static int get_exported_devices(char *host, int
> sockfd)
> > return 0;
> > }
> >
> > -static int list_exported_devices(char *host)
> > +static int list_importable_devices(char *host)
> > {
> > int rc;
> > int sockfd;
> > @@ -147,9 +147,10 @@ static int list_exported_devices(char *host)
> > }
> > dbg("connected to %s:%s", host, usbip_port_string);
> >
> > - rc = get_exported_devices(host, sockfd);
> > + rc = get_importable_devices(host, sockfd);
> > if (rc < 0) {
> > err("failed to get device list from %s", host);
> > + close(sockfd);
> > return -1;
> > }
>
> Why not just close before if?
>
> Best regards,
> --
> Krzysztof Opasiak
> Samsung R&D Institute Poland
> Samsung Electronics
--
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: mtu3: fix U3 port link issue
the issue is introduced when @is_u3_ip is used in mtu3_device_enabe()
before initialized in mtu3_mem_alloc(), so get global IP information
at first before used by following functins.
Signed-off-by: Chunfeng Yun
---
drivers/usb/mtu3/mtu3_core.c | 18 ++
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/mtu3/mtu3_core.c b/drivers/usb/mtu3/mtu3_core.c
index 520e55a..af3e531 100644
--- a/drivers/usb/mtu3/mtu3_core.c
+++ b/drivers/usb/mtu3/mtu3_core.c
@@ -481,21 +481,14 @@ static int mtu3_mem_alloc(struct mtu3 *mtu)
void __iomem *mbase = mtu->mac_base;
struct mtu3_ep *ep_array;
int in_ep_num, out_ep_num;
- u32 cap_epinfo, cap_dev;
+ u32 cap_epinfo;
int ret;
int i;
- mtu->hw_version = mtu3_readl(mtu->ippc_base, U3D_SSUSB_HW_ID);
-
- cap_dev = mtu3_readl(mtu->ippc_base, U3D_SSUSB_IP_DEV_CAP);
- mtu->is_u3_ip = !!SSUSB_IP_DEV_U3_PORT_NUM(cap_dev);
-
cap_epinfo = mtu3_readl(mbase, U3D_CAP_EPINFO);
in_ep_num = CAP_TX_EP_NUM(cap_epinfo);
out_ep_num = CAP_RX_EP_NUM(cap_epinfo);
- dev_info(mtu->dev, "IP version 0x%x(%s IP)\n", mtu->hw_version,
- mtu->is_u3_ip ? "U3" : "U2");
dev_info(mtu->dev, "fifosz/epnum: Tx=%#x/%d, Rx=%#x/%d\n",
mtu3_readl(mbase, U3D_CAP_EPNTXFFSZ), in_ep_num,
mtu3_readl(mbase, U3D_CAP_EPNRXFFSZ), out_ep_num);
@@ -730,8 +723,17 @@ irqreturn_t mtu3_irq(int irq, void *data)
static int mtu3_hw_init(struct mtu3 *mtu)
{
+ u32 cap_dev;
int ret;
+ mtu->hw_version = mtu3_readl(mtu->ippc_base, U3D_SSUSB_HW_ID);
+
+ cap_dev = mtu3_readl(mtu->ippc_base, U3D_SSUSB_IP_DEV_CAP);
+ mtu->is_u3_ip = !!SSUSB_IP_DEV_U3_PORT_NUM(cap_dev);
+
+ dev_info(mtu->dev, "IP version 0x%x(%s IP)\n", mtu->hw_version,
+ mtu->is_u3_ip ? "U3" : "U2");
+
mtu3_device_reset(mtu);
ret = mtu3_device_enable(mtu);
--
1.7.9.5
--
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: mtu3: enable auto switch from U3 to U2
inform mac2 to build U2 link automatically after U3 detect fail without software setting soft_connect. Signed-off-by: Chunfeng Yun --- drivers/usb/mtu3/mtu3_core.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/usb/mtu3/mtu3_core.c b/drivers/usb/mtu3/mtu3_core.c index af3e531..ca05021 100644 --- a/drivers/usb/mtu3/mtu3_core.c +++ b/drivers/usb/mtu3/mtu3_core.c @@ -568,6 +568,8 @@ static void mtu3_regs_init(struct mtu3 *mtu) SW_U1_REQUEST_ENABLE | SW_U2_REQUEST_ENABLE); /* device responses to u3_exit from host automatically */ mtu3_clrbits(mbase, U3D_LTSSM_CTRL, SOFT_U3_EXIT_EN); + /* automatically build U2 link when U3 detect fail */ + mtu3_setbits(mbase, U3D_USB2_TEST_MODE, U2U3_AUTO_SWITCH); } mtu3_set_speed(mtu); -- 1.7.9.5 -- 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 v13 00/10] usbip: exporting devices
Hello, > That doesn't tell me much. I am looking to see in the above where are > the server and client located. Sorry. The 'goal' section is added in later version. I will add reference to ASCII diagram in section 2. Also improve diagram in section 2. NEW) - dedicates devices from device(stub)-side (client) (server) +--+ +--+ device--+ STUB | | application/VHCI | +--+ +--+ 1) usbipa ... start daemon = = = 2) usbip list --local 3) usbip connect --- export a device --> = = = 4) usbip disconnect --- un-export a device ---> > >> Is this something that could be solved by opening ports in the > >> firewall? > > Usually, http(80) and https(443) from inside is opened for internet > > Access. > You can open other ports if need be and if you have access to firewall. > Doesn't making sure port 3240 isn't blocked on the firewall help? If it's the firewall of personal workstation, it's possible. > > The firewall is in between internet and As far as I know, corporate firewall is prohibited to open port access from outside by system administrator. For home network, I think rarely open port from outside. Enterprise/SOHO/Home Firewall/Proxy/Router Internet EXISTING) APP# usbip attach ---(passed)> DEV# usbipd DEV# usbipd (blocked)|| <- APP# usbip attach NEW) DEV# usbip connect --(passed)> DEV# usbipa APP# usbipa (blocked)|| <- APP# usbip connect > I want to understand how a different server (system that doesn't have > the USB device physically attached) can be authorized to export > devices that don't belong to it. 1) Connection level security TCP wrappers allows and/or denies network access. It is enabled when the daemons are compiled with ./configure --with-tcp-wrappers. When the daemons are running with SSL or Secure WebSocket tunneling proxy, the proxy can use client authentication with certificate files. 2) Device level security Udev rules can allow only known devices. To identify whether a device is remote, the local bus-id (KERNEL parameter in the rule) will be found in the last column of /sys/devices/platform/vhci_hcd/status[.N]. When device is found, the port number of USB/IP can be found in the first column of the matched line. The udev script can finish the connection using detach operation with the port number. I know that it's problem that the daemon accept any connected remote devices unless something special described above. User (or administrator) using new daemon must be careful. While applying the system, I learned server edition linux is different from workstation. In workstation edition, most of all devices are plug- and-playable. But, in server edition, there's less USB configuration. So administrator will do something to enable USB device. > Something about this model doesn't sound right. If I have system that > sits behind a firewall, I don't want USB devices visible to anybody > and everybody. I don't see anything in this series that disallows > that, short of saying don't enable USB/IP. The current (bind-and-attach) operation and new (connect) operation is independent. New operation is available when administrator starts new daemon. Please, let me know if you have comments to goal #2. --- 1-2) To improve usability of operations When two Linux machines are in a small distance, it's OK to bind (makes importable) at device side machine and attach (import) at application side. If application is as cloud service or in blade server, it's not practical to attach from application side. It's useful to connect (export) from device side. This patch set adds the new operation to connect from device side. --- > I would like to know why there is a need to change the existing server- > client model. I still would like to see a concrete answer why the > current model doesn't work. I don't have reason other than direction of connection from outside from firewall. My motivation is to utilize USB/IP as a platform of IoT. Linux is major of server OS and various small linux node is distributed everywhere. USB devices are most easy-to-use for the small node. USB/IP is useful to serve USB devices of distributed linux nodes as if they are local devices without any modification to applications. My goal is add flexibility for the IoT system. As a user of USB/IP, I think new way (connect) helps to handle many USB devices of many distributed linux nodes. > btw: could you please re-run get_maintainers - my email address in the > MAINTAINERS file changed a while ago. I think your email might be > outdated. I did. Sorry for my late response. I took time to write the answer. Thank you for taking time, n.iwata // > -Original Message- > From: Shuah Khan [mailto:shua
Re: [RFC][PATCH 1/3 v2] usb: dwc2: Add extcon support to dwc2 driver
On Tue, Dec 6, 2016 at 7:54 PM, Chanwoo Choi wrote: > Hi John, > > I give a some guide for extcon API. > This patch uses the deprecated extcon API (extcon_get_cable_state_). > So, I recommend that you better to use following extcon API: > - extcon_get_cable_state_() -> extcon_get_state() > - extcon_register_notifier() -> devm_extcon_register_notifier() Many thanks for the pointers! I really appreciate it! I'll rework the driver accordingly (if JohnY doesn't conclude that the extcon support here is unnecessary). thanks -john -- 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: [RFC][PATCH 1/3 v2] usb: dwc2: Add extcon support to dwc2 driver
Hi John,
I give a some guide for extcon API.
This patch uses the deprecated extcon API (extcon_get_cable_state_).
So, I recommend that you better to use following extcon API:
- extcon_get_cable_state_() -> extcon_get_state()
- extcon_register_notifier() -> devm_extcon_register_notifier()
Best Regards,
Chanwoo Choi
On 2016년 12월 06일 17:06, John Stultz wrote:
> This patch wires up extcon support to the dwc2 driver
> so that devices that use a modern generic phy driver
> and don't use the usb-phy infrastructure can still
> signal connect/disconnect events.
>
> Cc: Wei Xu
> Cc: Guodong Xu
> Cc: Amit Pundir
> Cc: Rob Herring
> Cc: John Youn
> Cc: Douglas Anderson
> Cc: Chen Yu
> Cc: Kishon Vijay Abraham I
> Cc: Felipe Balbi
> Cc: Greg Kroah-Hartman
> Cc: linux-usb@vger.kernel.org
> Signed-off-by: John Stultz
> ---
> v2:
> * Move extcon logic from generic phy to dwc2 driver, as
> suggested by Kishon.
> ---
> arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 11 +
> drivers/usb/dwc2/core.h | 16
> drivers/usb/dwc2/core_intr.c | 25 +++
> drivers/usb/dwc2/hcd.c| 23 +
> drivers/usb/dwc2/platform.c | 41
> +++
> 5 files changed, 116 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> index 17839db..8a86a11 100644
> --- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> +++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> @@ -732,6 +732,16 @@
> regulator-always-on;
> };
>
> + usb_vbus: usb-vbus {
> + compatible = "linux,extcon-usb-gpio";
> + id-gpio = <&gpio2 6 1>;
> + };
> +
> + usb_id: usb-id {
> + compatible = "linux,extcon-usb-gpio";
> + id-gpio = <&gpio2 5 1>;
> + };
> +
> usb_phy: usbphy {
> compatible = "hisilicon,hi6220-usb-phy";
> #phy-cells = <0>;
> @@ -745,6 +755,7 @@
> phys = <&usb_phy>;
> phy-names = "usb2-phy";
> clocks = <&sys_ctrl HI6220_USBOTG_HCLK>;
> + extcon = <&usb_vbus>, <&usb_id>;
> clock-names = "otg";
> dr_mode = "otg";
> g-use-dma;
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index 2a21a04..4cfce62 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -623,6 +623,13 @@ struct dwc2_hregs_backup {
> bool valid;
> };
>
> +struct dwc2_extcon {
> + struct notifier_block nb;
> + struct extcon_dev *extcon;
> + int state;
> +};
> +
> +
> /*
> * Constants related to high speed periodic scheduling
> *
> @@ -996,6 +1003,10 @@ struct dwc2_hsotg {
> u32 g_np_g_tx_fifo_sz;
> u32 g_tx_fifo_sz[MAX_EPS_CHANNELS];
> #endif /* CONFIG_USB_DWC2_PERIPHERAL || CONFIG_USB_DWC2_DUAL_ROLE */
> +
> + struct dwc2_extcon extcon_vbus;
> + struct dwc2_extcon extcon_id;
> + struct delayed_work extcon_work;
> };
>
> /* Reasons for halting a host channel */
> @@ -1041,6 +1052,11 @@ extern void dwc2_flush_rx_fifo(struct dwc2_hsotg
> *hsotg);
> extern void dwc2_enable_global_interrupts(struct dwc2_hsotg *hcd);
> extern void dwc2_disable_global_interrupts(struct dwc2_hsotg *hcd);
>
> +extern int dwc2_extcon_vbus_notifier(struct notifier_block *nb,
> + unsigned long event, void *ptr);
> +extern int dwc2_extcon_id_notifier(struct notifier_block *nb,
> + unsigned long event, void *ptr);
> +
> /* This function should be called on every hardware interrupt. */
> extern irqreturn_t dwc2_handle_common_intr(int irq, void *dev);
>
> diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c
> index d85c5c9..d4fa938 100644
> --- a/drivers/usb/dwc2/core_intr.c
> +++ b/drivers/usb/dwc2/core_intr.c
> @@ -479,6 +479,31 @@ static void dwc2_handle_usb_suspend_intr(struct
> dwc2_hsotg *hsotg)
> }
> }
>
> +
> +
> +int dwc2_extcon_vbus_notifier(struct notifier_block *nb,
> + unsigned long event, void *ptr)
> +{
> + struct dwc2_extcon *vbus = container_of(nb, struct dwc2_extcon, nb);
> + struct dwc2_hsotg *hsotg = container_of(vbus, struct dwc2_hsotg,
> + extcon_vbus);
> +
> + schedule_delayed_work(&hsotg->extcon_work, msecs_to_jiffies(100));
> + return NOTIFY_DONE;
> +}
> +
> +int dwc2_extcon_id_notifier(struct notifier_block *nb,
> + unsigned long event, void *ptr)
> +{
> + struct dwc2_extcon *id = container_of(nb, struct dwc2_extcon, nb);
> + struct dwc2_hsotg *hsotg = container_of(id, struct
Re: [RFC][PATCH] HACK: usb: dwc2: Workaround case where GOTGCTL state is wrong
On 12/6/2016 5:48 PM, John Stultz wrote:
> Hey John,
> Just wanted to send this by you, as it seems something is
> slightly off with the GOTGCTL state when removing a otg adapter
> cable. The following seems to work around the issue I'm seeing.
>
> Let me know if you have any thoughts on this.
> thanks
> -john
>
>
> When removing a USB-A to USB-otg adapter cable, we get a change
> status irq, and then in dwc2_conn_id_status_change, we
> erroniously see the GOTGCTL_CONID_B flag set. This causes us to
This is the correct behavior for an OTG controller. When you unplug a
cable or plug in the B end of a cable, the ID pin floats, indicating
it is a B-Device.
When you plug in an A-cable, which is what your adapter is, it will
ground the pin, meaning A-device.
> get stuck in the "while (!dwc2_is_device_mode(hsotg))" loop,
> spitting out "Waiting for Peripheral Mode, Mode=Host" warnings
> until it fails out many seconds later.
This is weird. Once the ID pin goes to B, the core should become a
peripheral and this should be reflected in the status registers.
>
> This patch works around the issue by re-reading the GOTGCTL
> state to check if the GOTGCTL_CONID_B is still set and if not
> restarting the change status logic.
This also seems weird. The connector id status shouldn't go back to A,
assuming you've left the cable unplugged.
Is the controller supposed to work in both peripheral and host modes?
>
> I suspect this isn't the best solution, but it seems to work
> well for me.
>
The workaround seems fine, but still, this indicates that something
wrong is going on somwhere.
You can add my ack:
Acked-by: John Youn
Regards,
John
> Feedback would be greatly appreciated!
>
> Cc: Wei Xu
> Cc: Guodong Xu
> Cc: Amit Pundir
> Cc: Rob Herring
> Cc: John Youn
> Cc: Douglas Anderson
> Cc: Chen Yu
> Cc: Kishon Vijay Abraham I
> Cc: Felipe Balbi
> Cc: Greg Kroah-Hartman
> Cc: linux-usb@vger.kernel.org
> Signed-off-by: John Stultz
> ---
> drivers/usb/dwc2/hcd.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index 143da47..6d6802a 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -3203,7 +3203,7 @@ static void dwc2_conn_id_status_change(struct
> work_struct *work)
> dev_dbg(hsotg->dev, "gotgctl=%0x\n", gotgctl);
> dev_dbg(hsotg->dev, "gotgctl.b.conidsts=%d\n",
> !!(gotgctl & GOTGCTL_CONID_B));
> -
> +again:
> /* B-Device connector (Device Mode) */
> if (gotgctl & GOTGCTL_CONID_B) {
> /* Wait for switch to device mode */
> @@ -3219,6 +3219,9 @@ static void dwc2_conn_id_status_change(struct
> work_struct *work)
>dwc2_is_host_mode(hsotg) ? "Host" :
>"Peripheral");
> usleep_range(2, 4);
> + gotgctl = dwc2_readl(hsotg->regs + GOTGCTL);
> + if (!(gotgctl & GOTGCTL_CONID_B))
> + goto again;
> if (++count > 250)
> break;
> }
>
--
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 03/12] power_supply: axp288_charger: Replace the extcon API
Hi Sebastian, On 2016년 12월 07일 12:05, Sebastian Reichel wrote: > Hi Chanwoo, > > On Tue, Dec 06, 2016 at 09:26:14AM +0900, Chanwoo Choi wrote: >> Could you please review and pick the patch3/4 for power-supply driver? > > Patches look fine. As I expect the merge window to open next week I > would rather not queue this for 4.10 and instead do it once 4.10-rc1 > has been tagged. > > -- Sebastian > Thanks for your pick up. -- Best Regards, 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: [PATCH 03/12] power_supply: axp288_charger: Replace the extcon API
Hi Chanwoo, On Tue, Dec 06, 2016 at 09:26:14AM +0900, Chanwoo Choi wrote: > Could you please review and pick the patch3/4 for power-supply driver? Patches look fine. As I expect the merge window to open next week I would rather not queue this for 4.10 and instead do it once 4.10-rc1 has been tagged. -- Sebastian signature.asc Description: PGP signature
[RFC][PATCH] HACK: usb: dwc2: Workaround case where GOTGCTL state is wrong
Hey John,
Just wanted to send this by you, as it seems something is
slightly off with the GOTGCTL state when removing a otg adapter
cable. The following seems to work around the issue I'm seeing.
Let me know if you have any thoughts on this.
thanks
-john
When removing a USB-A to USB-otg adapter cable, we get a change
status irq, and then in dwc2_conn_id_status_change, we
erroniously see the GOTGCTL_CONID_B flag set. This causes us to
get stuck in the "while (!dwc2_is_device_mode(hsotg))" loop,
spitting out "Waiting for Peripheral Mode, Mode=Host" warnings
until it fails out many seconds later.
This patch works around the issue by re-reading the GOTGCTL
state to check if the GOTGCTL_CONID_B is still set and if not
restarting the change status logic.
I suspect this isn't the best solution, but it seems to work
well for me.
Feedback would be greatly appreciated!
Cc: Wei Xu
Cc: Guodong Xu
Cc: Amit Pundir
Cc: Rob Herring
Cc: John Youn
Cc: Douglas Anderson
Cc: Chen Yu
Cc: Kishon Vijay Abraham I
Cc: Felipe Balbi
Cc: Greg Kroah-Hartman
Cc: linux-usb@vger.kernel.org
Signed-off-by: John Stultz
---
drivers/usb/dwc2/hcd.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 143da47..6d6802a 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -3203,7 +3203,7 @@ static void dwc2_conn_id_status_change(struct work_struct
*work)
dev_dbg(hsotg->dev, "gotgctl=%0x\n", gotgctl);
dev_dbg(hsotg->dev, "gotgctl.b.conidsts=%d\n",
!!(gotgctl & GOTGCTL_CONID_B));
-
+again:
/* B-Device connector (Device Mode) */
if (gotgctl & GOTGCTL_CONID_B) {
/* Wait for switch to device mode */
@@ -3219,6 +3219,9 @@ static void dwc2_conn_id_status_change(struct work_struct
*work)
dwc2_is_host_mode(hsotg) ? "Host" :
"Peripheral");
usleep_range(2, 4);
+ gotgctl = dwc2_readl(hsotg->regs + GOTGCTL);
+ if (!(gotgctl & GOTGCTL_CONID_B))
+ goto again;
if (++count > 250)
break;
}
--
2.7.4
--
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: [RFC][PATCH 1/3 v2] usb: dwc2: Add extcon support to dwc2 driver
On Tue, Dec 6, 2016 at 4:35 PM, John Stultz wrote: > On Tue, Dec 6, 2016 at 4:26 PM, John Youn wrote: >> On 12/6/2016 4:05 PM, John Stultz wrote: >>> On Tue, Dec 6, 2016 at 3:17 PM, John Youn wrote: Also, do you really need this at all? Wasn't your system previously able to detect the ID pin change correctly via the connection id status change interrupt? This would only be needed if that were not the case. >>> >>> So it can be made work w/o this, but we needed other hacks because the >>> usb-gadget disconnect logic never triggered when the cable was >>> unplugged. The controller would jump over to host mode, then when we >>> re-plugged in the usb-gadget cable, it would fail often as we never >>> got a disconnect signal. That's why earlier I was using this hack to >>> force gadget disconnect before the reset was called: >>> https://lkml.org/lkml/2016/10/20/26 >> >> Other than the triggering WARN_ON() in fifo init, is there any other >> negative effects? > > Well, when we see the WARN_ON, it doesn't connect into usb-gadget > mode. I had to unplug and re-plug the cable. > (The hack I linked to above avoids this, but I suspect its not correct). > > Also Amit Pundir had mentioned earlier that the UDC sysfs state > doesn't get reported correctly since it doesn't register the > usb-gadget as unplugged until the cable is re-inserted. Actually, this is still an outstanding issue, as /sys/class/udc/f72c.usb/state seems to be stuck at "configured" no matter the state of the cable, even with my patches. I'll need to look further at the details there. thanks -john -- 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: [RFC][PATCH 1/3 v2] usb: dwc2: Add extcon support to dwc2 driver
On Tue, Dec 6, 2016 at 4:26 PM, John Youn wrote: > On 12/6/2016 4:05 PM, John Stultz wrote: >> On Tue, Dec 6, 2016 at 3:17 PM, John Youn wrote: >>> Also, do you really need this at all? Wasn't your system previously >>> able to detect the ID pin change correctly via the connection id >>> status change interrupt? This would only be needed if that were not >>> the case. >> >> So it can be made work w/o this, but we needed other hacks because the >> usb-gadget disconnect logic never triggered when the cable was >> unplugged. The controller would jump over to host mode, then when we >> re-plugged in the usb-gadget cable, it would fail often as we never >> got a disconnect signal. That's why earlier I was using this hack to >> force gadget disconnect before the reset was called: >> https://lkml.org/lkml/2016/10/20/26 > > Other than the triggering WARN_ON() in fifo init, is there any other > negative effects? Well, when we see the WARN_ON, it doesn't connect into usb-gadget mode. I had to unplug and re-plug the cable. (The hack I linked to above avoids this, but I suspect its not correct). Also Amit Pundir had mentioned earlier that the UDC sysfs state doesn't get reported correctly since it doesn't register the usb-gadget as unplugged until the cable is re-inserted. > We are revisiting this fifo init code and I think the fifo init is not > necessary for USB_RESET purposes. This should get rid of a race > condition where the EP's are not disabled before attempting to > initialize their FIFO's. Which should get rid of the WARN_ON(). > > If this is the only issue, then this will probably resolve it. (Basically that and the two suspend fixes I sent along in this patchset :). I'd be happy to test anything you're playing with. thanks -john -- 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: [RFC][PATCH 1/3 v2] usb: dwc2: Add extcon support to dwc2 driver
On 12/6/2016 4:05 PM, John Stultz wrote:
> On Tue, Dec 6, 2016 at 3:17 PM, John Youn wrote:
>> On 12/6/2016 12:06 AM, John Stultz wrote:
>>> This patch wires up extcon support to the dwc2 driver
>>> so that devices that use a modern generic phy driver
>>> and don't use the usb-phy infrastructure can still
>>> signal connect/disconnect events.
>>>
>>> Cc: Wei Xu
>>> Cc: Guodong Xu
>>> Cc: Amit Pundir
>>> Cc: Rob Herring
>>> Cc: John Youn
>>> Cc: Douglas Anderson
>>> Cc: Chen Yu
>>> Cc: Kishon Vijay Abraham I
>>> Cc: Felipe Balbi
>>> Cc: Greg Kroah-Hartman
>>> Cc: linux-usb@vger.kernel.org
>>> Signed-off-by: John Stultz
>>> ---
>>> v2:
>>> * Move extcon logic from generic phy to dwc2 driver, as
>>> suggested by Kishon.
>>> ---
>>> arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 11 +
>>> drivers/usb/dwc2/core.h | 16
>>> drivers/usb/dwc2/core_intr.c | 25 +++
>>> drivers/usb/dwc2/hcd.c| 23 +
>>> drivers/usb/dwc2/platform.c | 41
>>> +++
>>> 5 files changed, 116 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
>>> b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
>>> index 17839db..8a86a11 100644
>>> --- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
>>> +++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
>>> @@ -732,6 +732,16 @@
>>> regulator-always-on;
>>> };
>>>
>>> + usb_vbus: usb-vbus {
>>> + compatible = "linux,extcon-usb-gpio";
>>> + id-gpio = <&gpio2 6 1>;
>>> + };
>>> +
>>> + usb_id: usb-id {
>>> + compatible = "linux,extcon-usb-gpio";
>>> + id-gpio = <&gpio2 5 1>;
>>> + };
>>> +
>>
>> So you are using extcon-usb-gpio driver to detect both the ID pin and
>> VBUS status correct? Do you need the VBUS one? It doesn't look like
>> you are using it.
>
> Not at the moment. Apologies for my ignorance, I'm not totally
> familiar with the usage of the vbus vs id pins, so I'm somewhat
> following what I was seeing from other drivers. I know with a usb OTG
> to usb A adapter, you get the vbus signal but not the id signal, but I
> don't quite see what should be done differently in that case (as it
> seems to work ok).
Hi John,
The ID pin indicates which end of the cable is connected to the
controller port, determining whether it should take the role of an
A-device or B-device. If this is not visible to the controller (thus
the controller would not give CONNIDSTSCHNG interrupt), that is why
you would need to hook up the extcon module.
I'm thinking this shouldn't be needed for you since you can see this
interrupt.
>
>> Also, do you really need this at all? Wasn't your system previously
>> able to detect the ID pin change correctly via the connection id
>> status change interrupt? This would only be needed if that were not
>> the case.
>
> So it can be made work w/o this, but we needed other hacks because the
> usb-gadget disconnect logic never triggered when the cable was
> unplugged. The controller would jump over to host mode, then when we
> re-plugged in the usb-gadget cable, it would fail often as we never
> got a disconnect signal. That's why earlier I was using this hack to
> force gadget disconnect before the reset was called:
> https://lkml.org/lkml/2016/10/20/26
Other than the triggering WARN_ON() in fifo init, is there any other
negative effects?
We are revisiting this fifo init code and I think the fifo init is not
necessary for USB_RESET purposes. This should get rid of a race
condition where the EP's are not disabled before attempting to
initialize their FIFO's. Which should get rid of the WARN_ON().
If this is the only issue, then this will probably resolve it.
Regards,
John
--
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: [RFC][PATCH 1/3 v2] usb: dwc2: Add extcon support to dwc2 driver
On Tue, Dec 6, 2016 at 3:17 PM, John Youn wrote:
> On 12/6/2016 12:06 AM, John Stultz wrote:
>> This patch wires up extcon support to the dwc2 driver
>> so that devices that use a modern generic phy driver
>> and don't use the usb-phy infrastructure can still
>> signal connect/disconnect events.
>>
>> Cc: Wei Xu
>> Cc: Guodong Xu
>> Cc: Amit Pundir
>> Cc: Rob Herring
>> Cc: John Youn
>> Cc: Douglas Anderson
>> Cc: Chen Yu
>> Cc: Kishon Vijay Abraham I
>> Cc: Felipe Balbi
>> Cc: Greg Kroah-Hartman
>> Cc: linux-usb@vger.kernel.org
>> Signed-off-by: John Stultz
>> ---
>> v2:
>> * Move extcon logic from generic phy to dwc2 driver, as
>> suggested by Kishon.
>> ---
>> arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 11 +
>> drivers/usb/dwc2/core.h | 16
>> drivers/usb/dwc2/core_intr.c | 25 +++
>> drivers/usb/dwc2/hcd.c| 23 +
>> drivers/usb/dwc2/platform.c | 41
>> +++
>> 5 files changed, 116 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
>> b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
>> index 17839db..8a86a11 100644
>> --- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
>> +++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
>> @@ -732,6 +732,16 @@
>> regulator-always-on;
>> };
>>
>> + usb_vbus: usb-vbus {
>> + compatible = "linux,extcon-usb-gpio";
>> + id-gpio = <&gpio2 6 1>;
>> + };
>> +
>> + usb_id: usb-id {
>> + compatible = "linux,extcon-usb-gpio";
>> + id-gpio = <&gpio2 5 1>;
>> + };
>> +
>
> So you are using extcon-usb-gpio driver to detect both the ID pin and
> VBUS status correct? Do you need the VBUS one? It doesn't look like
> you are using it.
Not at the moment. Apologies for my ignorance, I'm not totally
familiar with the usage of the vbus vs id pins, so I'm somewhat
following what I was seeing from other drivers. I know with a usb OTG
to usb A adapter, you get the vbus signal but not the id signal, but I
don't quite see what should be done differently in that case (as it
seems to work ok).
> Also, do you really need this at all? Wasn't your system previously
> able to detect the ID pin change correctly via the connection id
> status change interrupt? This would only be needed if that were not
> the case.
So it can be made work w/o this, but we needed other hacks because the
usb-gadget disconnect logic never triggered when the cable was
unplugged. The controller would jump over to host mode, then when we
re-plugged in the usb-gadget cable, it would fail often as we never
got a disconnect signal. That's why earlier I was using this hack to
force gadget disconnect before the reset was called:
https://lkml.org/lkml/2016/10/20/26
>> usb_phy: usbphy {
>> compatible = "hisilicon,hi6220-usb-phy";
>> #phy-cells = <0>;
>> @@ -745,6 +755,7 @@
>> phys = <&usb_phy>;
>> phy-names = "usb2-phy";
>> clocks = <&sys_ctrl HI6220_USBOTG_HCLK>;
>> + extcon = <&usb_vbus>, <&usb_id>;
>
> You should document what should be set for "extcon" in
> /Documentation/devicetree/bindings/usb/dwc2.txt. And make that a
> separate commit before using the binding.
Yes. I've already split it out, and added bindings documentation in my
tree. I included this here to make it easier to see what all was
involved w/o having to go through a bunch of patches.
>> clock-names = "otg";
>> dr_mode = "otg";
>> g-use-dma;
>> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
>> index 2a21a04..4cfce62 100644
>> --- a/drivers/usb/dwc2/core.h
>> +++ b/drivers/usb/dwc2/core.h
>> @@ -623,6 +623,13 @@ struct dwc2_hregs_backup {
>> bool valid;
>> };
>>
>> +struct dwc2_extcon {
>> + struct notifier_block nb;
>> + struct extcon_dev *extcon;
>> + int state;
>> +};
>> +
>> +
>
> Don't use multiple blank lines. Please run "checkpatch.pl --strict"
> and fix other issues it reports, if possible.
Yes. Apologies. I noticed this once I sent it and fixed it up in my
tree (as well as a few other extra whitespace lines elsewhere).
> [snip]
>
>> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
>> index 8e1728b..2e4545f 100644
>> --- a/drivers/usb/dwc2/platform.c
>> +++ b/drivers/usb/dwc2/platform.c
>> @@ -46,6 +46,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include
>>
>> @@ -543,6 +544,7 @@ static int dwc2_driver_probe(struct platform_device *dev)
>> struct dwc2_core_params defparams;
>> struct dwc2_hsotg *hsotg;
>> struct resource *res;
>> + struct extcon_dev *ext_id, *ext_vbus;
>>
RE: [PATCH v13 05/10] usbip: exporting devices: modifications to daemon
Hello, > how about usbip_meta_driver_set(type)? Good. I'd like to confirm the 'type' parameter. #define USBIP_DRIVER_TYPE_HOST 0 #define USBIP_DRIVER_TYPE_DEVICE 1 Then usbip_meta_driver_init should not be included. It must be usbip_meta_driver_set(USBIP_DRIVER_TYPE_HOST). Thank you, n.iwata // > -Original Message- > From: Krzysztof Opasiak [mailto:k.opas...@samsung.com] > Sent: Tuesday, December 06, 2016 6:51 PM > To: fx IWATA NOBUO; valentina.mane...@gmail.com; shuah...@samsung.com; > gre...@linuxfoundation.org; linux-usb@vger.kernel.org > Cc: fx MICHIMURA TADAO > Subject: Re: [PATCH v13 05/10] usbip: exporting devices: modifications to > daemon > > Hi, > > On 12/06/2016 05:40 AM, fx IWATA NOBUO wrote: > > Hello, > > > >>> usbip_driver_ has widely used as function names and file > >>> names for host side abstraction. > >>> If they were usbip_host_, I can use > >>> usbip_driver_open/close for both host(stub/vudc) and vhci. > >> > >> usbip_host() is not correct name to use for both stub and vudc > >> as from USB point of view stub is on a host but vudc is on a device > >> side > > > > OK. > > > >> maybe a kind of usbipd_backed_init() would be more suitable? > > > > Naming problem again but I recognize usbip_open_driver() is worse than > > connect. > > I think the word 'backend' has wide meaning and more strict word is > > better. > > > > init driver = &host_driver; NA > > ( )driver = &device_driver; NA > > open usbip_driver_open usbip_vhci_driver_open > > close usbip_driver_close usbip_vhci_driver_close > > > > Here, I'd like to use word 'driver'. > > I searched analogy meta_, super_ in kernel. > > > > How about usbip_meta_driver_init/open/close()? > > Sounds good. Let's try. > > > > usbip_update_driver() is t totally unrelated to what this > assignment really does. > >> So as above. I suggest to call it usbipd_backend() instead of driver. > > > > Please, let me know good verb representing 'driver = &device_driver'. > > how about usbip_meta_driver_set(type)? > > Best regards, > -- > Krzysztof Opasiak > Samsung R&D Institute Poland > Samsung Electronics -- 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: [RFC][PATCH 1/3 v2] usb: dwc2: Add extcon support to dwc2 driver
On 12/6/2016 12:06 AM, John Stultz wrote:
> This patch wires up extcon support to the dwc2 driver
> so that devices that use a modern generic phy driver
> and don't use the usb-phy infrastructure can still
> signal connect/disconnect events.
>
> Cc: Wei Xu
> Cc: Guodong Xu
> Cc: Amit Pundir
> Cc: Rob Herring
> Cc: John Youn
> Cc: Douglas Anderson
> Cc: Chen Yu
> Cc: Kishon Vijay Abraham I
> Cc: Felipe Balbi
> Cc: Greg Kroah-Hartman
> Cc: linux-usb@vger.kernel.org
> Signed-off-by: John Stultz
> ---
> v2:
> * Move extcon logic from generic phy to dwc2 driver, as
> suggested by Kishon.
> ---
> arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 11 +
> drivers/usb/dwc2/core.h | 16
> drivers/usb/dwc2/core_intr.c | 25 +++
> drivers/usb/dwc2/hcd.c| 23 +
> drivers/usb/dwc2/platform.c | 41
> +++
> 5 files changed, 116 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> index 17839db..8a86a11 100644
> --- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> +++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> @@ -732,6 +732,16 @@
> regulator-always-on;
> };
>
> + usb_vbus: usb-vbus {
> + compatible = "linux,extcon-usb-gpio";
> + id-gpio = <&gpio2 6 1>;
> + };
> +
> + usb_id: usb-id {
> + compatible = "linux,extcon-usb-gpio";
> + id-gpio = <&gpio2 5 1>;
> + };
> +
So you are using extcon-usb-gpio driver to detect both the ID pin and
VBUS status correct? Do you need the VBUS one? It doesn't look like
you are using it.
Also, do you really need this at all? Wasn't your system previously
able to detect the ID pin change correctly via the connection id
status change interrupt? This would only be needed if that were not
the case.
> usb_phy: usbphy {
> compatible = "hisilicon,hi6220-usb-phy";
> #phy-cells = <0>;
> @@ -745,6 +755,7 @@
> phys = <&usb_phy>;
> phy-names = "usb2-phy";
> clocks = <&sys_ctrl HI6220_USBOTG_HCLK>;
> + extcon = <&usb_vbus>, <&usb_id>;
You should document what should be set for "extcon" in
/Documentation/devicetree/bindings/usb/dwc2.txt. And make that a
separate commit before using the binding.
> clock-names = "otg";
> dr_mode = "otg";
> g-use-dma;
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index 2a21a04..4cfce62 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -623,6 +623,13 @@ struct dwc2_hregs_backup {
> bool valid;
> };
>
> +struct dwc2_extcon {
> + struct notifier_block nb;
> + struct extcon_dev *extcon;
> + int state;
> +};
> +
> +
Don't use multiple blank lines. Please run "checkpatch.pl --strict"
and fix other issues it reports, if possible.
[snip]
> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
> index 8e1728b..2e4545f 100644
> --- a/drivers/usb/dwc2/platform.c
> +++ b/drivers/usb/dwc2/platform.c
> @@ -46,6 +46,7 @@
> #include
> #include
> #include
> +#include
>
> #include
>
> @@ -543,6 +544,7 @@ static int dwc2_driver_probe(struct platform_device *dev)
> struct dwc2_core_params defparams;
> struct dwc2_hsotg *hsotg;
> struct resource *res;
> + struct extcon_dev *ext_id, *ext_vbus;
> int retval;
>
> match = of_match_device(dwc2_of_match_table, &dev->dev);
> @@ -620,6 +622,45 @@ static int dwc2_driver_probe(struct platform_device *dev)
> if (retval)
> goto error;
>
> + ext_id = ERR_PTR(-ENODEV);
> + ext_vbus = ERR_PTR(-ENODEV);
> + if (of_property_read_bool(dev->dev.of_node, "extcon")) {
You can omit this check since it's done in the api function.
> + /* Each one of them is not mandatory */
> + ext_vbus = extcon_get_edev_by_phandle(&dev->dev, 0);
> + if (IS_ERR(ext_vbus) && PTR_ERR(ext_vbus) != -ENODEV)
> + return PTR_ERR(ext_vbus);
> +
> + ext_id = extcon_get_edev_by_phandle(&dev->dev, 1);
> + if (IS_ERR(ext_id) && PTR_ERR(ext_id) != -ENODEV)
> + return PTR_ERR(ext_id);
> + }
Ensure when you initialize hsotg->extcon_vbus/id that they are either
valid and set or NULL so that you don't have to do IS_ERR() all the
time.
Regards,
John
--
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: udc: atmel: Fix check in usba_ep_enable()
Hi Felipe,
I realize I sent this patch to your old @ti.com email address. Do you
want me to resend it?
Regards,
Boris
On Tue, 6 Dec 2016 22:59:43 +0100
Boris Brezillon wrote:
> desc->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK is not necessarily
> equal to ep->index and that's perfectly fine. The usba endpoint index is
> just an internal identifier used by the driver to know which registers
> to use for a USB endpoint.
>
> Enforcing this constraint is not only useless, but can also lead to
> errors since nothing guarantees that the endpoint number and index are
> matching when an endpoint is selected for a specific descriptor, thus
> leading to errors at ->enable() time when it's already too late to choose
> another endpoint.
>
> Signed-off-by: Boris Brezillon
> ---
> Hi,
>
> I intentionally didn't add the Cc stable and Fixes tags because this
> bug dates back to the drivers creation, and I fear the index <->
> epnum constraint was actually required at that time.
>
> Note that I discovered this bug thanks to the WARN_ON_ONCE() in
> usb_ep_queue() [1] which was introduced in 4.5.
> It might appear that this problem was silently ignored before that
> (with part of the usba_ep_enable() code being skipped without any
> notice).
>
> Regards,
>
> Boris
>
> [1]http://lxr.free-electrons.com/source/drivers/usb/gadget/udc/core.c#L264
> ---
> drivers/usb/gadget/udc/atmel_usba_udc.c | 7 ++-
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c
> b/drivers/usb/gadget/udc/atmel_usba_udc.c
> index bb1f6c8f0f01..981d2639d413 100644
> --- a/drivers/usb/gadget/udc/atmel_usba_udc.c
> +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
> @@ -531,11 +531,8 @@ usba_ep_enable(struct usb_ep *_ep, const struct
> usb_endpoint_descriptor *desc)
>
> maxpacket = usb_endpoint_maxp(desc) & 0x7ff;
>
> - if (((desc->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK) != ep->index)
> - || ep->index == 0
> - || desc->bDescriptorType != USB_DT_ENDPOINT
> - || maxpacket == 0
> - || maxpacket > ep->fifo_size) {
> + if (ep->index == 0 || desc->bDescriptorType != USB_DT_ENDPOINT ||
> + maxpacket == 0 || maxpacket > ep->fifo_size) {
> DBG(DBG_ERR, "ep_enable: Invalid argument");
> return -EINVAL;
> }
--
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: no USB rootfs after wake
On Tue, 6 Dec 2016, Steffen Dirkwinkel wrote: > Hi all, > > I've been trying to debug issues with my rootfs on usb 3 flash drives > and suspending to ram. > > Symptoms: > > If I suspend and wake while my laptop is plugged into a charger > everything works perfectly. It looks like my laptop keeps usb power on > if a charger is plugged in. > > Without a charger the laptop locks up after waking due to ext4-fs errors. > > I'm not sure wether this is an issue with usb-persist power management > or something else. > > There are other people running into similar issues: > https://forum.manjaro.org/t/wake-up-problem-usb-3-0-after-suspend/10709 > https://bugzilla.kernel.org/show_bug.cgi?id=30912# > > Things I have tried: > - increasing retry count in drivers/usb/core/hub.c check_port_resume_type How high did you make it? Did you go as high as 1? > - http://askubuntu.com/questions/505779/suspending-with-root-on-usb The explanation on that askubuntu page doesn't make a lot of sense. USB devices are detected during resume before kernel threads get woken up. The error messages listed in the log suggest that the connection to the device simply did not get re-established during the resume. > Things I might try: > - debug log to another device (would usb to serial work?) Maybe, but it's unlikely. > - inject 5v usb power using a powered hub? > - anything you suggest You should enable dynamic debugging in usbcore: echo 'module usbcore =p' >/sys/kernel/debug/dynamic_debug/control You'll probably have to use a network console to collect the debugging messages. Another thing you could try is connecting that USB-3 flash drive to a normal PC running Linux (so not as the root fs), and see what happens when you suspend and resume the system. 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
[PATCH] usb: gadget: udc: atmel: Fix check in usba_ep_enable()
desc->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK is not necessarily
equal to ep->index and that's perfectly fine. The usba endpoint index is
just an internal identifier used by the driver to know which registers
to use for a USB endpoint.
Enforcing this constraint is not only useless, but can also lead to
errors since nothing guarantees that the endpoint number and index are
matching when an endpoint is selected for a specific descriptor, thus
leading to errors at ->enable() time when it's already too late to choose
another endpoint.
Signed-off-by: Boris Brezillon
---
Hi,
I intentionally didn't add the Cc stable and Fixes tags because this
bug dates back to the drivers creation, and I fear the index <->
epnum constraint was actually required at that time.
Note that I discovered this bug thanks to the WARN_ON_ONCE() in
usb_ep_queue() [1] which was introduced in 4.5.
It might appear that this problem was silently ignored before that
(with part of the usba_ep_enable() code being skipped without any
notice).
Regards,
Boris
[1]http://lxr.free-electrons.com/source/drivers/usb/gadget/udc/core.c#L264
---
drivers/usb/gadget/udc/atmel_usba_udc.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c
b/drivers/usb/gadget/udc/atmel_usba_udc.c
index bb1f6c8f0f01..981d2639d413 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -531,11 +531,8 @@ usba_ep_enable(struct usb_ep *_ep, const struct
usb_endpoint_descriptor *desc)
maxpacket = usb_endpoint_maxp(desc) & 0x7ff;
- if (((desc->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK) != ep->index)
- || ep->index == 0
- || desc->bDescriptorType != USB_DT_ENDPOINT
- || maxpacket == 0
- || maxpacket > ep->fifo_size) {
+ if (ep->index == 0 || desc->bDescriptorType != USB_DT_ENDPOINT ||
+ maxpacket == 0 || maxpacket > ep->fifo_size) {
DBG(DBG_ERR, "ep_enable: Invalid argument");
return -EINVAL;
}
--
2.7.4
--
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
no USB rootfs after wake
Hi all, I've been trying to debug issues with my rootfs on usb 3 flash drives and suspending to ram. Symptoms: If I suspend and wake while my laptop is plugged into a charger everything works perfectly. It looks like my laptop keeps usb power on if a charger is plugged in. Without a charger the laptop locks up after waking due to ext4-fs errors. I'm not sure wether this is an issue with usb-persist power management or something else. There are other people running into similar issues: https://forum.manjaro.org/t/wake-up-problem-usb-3-0-after-suspend/10709 https://bugzilla.kernel.org/show_bug.cgi?id=30912# Things I have tried: - increasing retry count in drivers/usb/core/hub.c check_port_resume_type - http://askubuntu.com/questions/505779/suspending-with-root-on-usb Things I might try: - debug log to another device (would usb to serial work?) - inject 5v usb power using a powered hub? - anything you suggest Steffen Dirkwinkel -- 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: net/gadget: slab-out-of-bounds write in dev_config
On Tue, Dec 6, 2016 at 4:30 PM, Alan Stern wrote:
> On Tue, 6 Dec 2016, Andrey Konovalov wrote:
>
>> Hi!
>>
>> I've got the following error report while running the syzkaller fuzzer.
>>
>> ep0_write() doesn't check the length, so a user can cause an
>> out-of-bounds with both size and data controlled.
>> There's a comment which says "IN DATA+STATUS caller makes len <=
>> wLength". While I'm not exactly sure what that means, the length seems
>> to be passed unmodified directly from dev_config().
>
> You're right about the comment being misleading. It looks like
> somebody forgot to actually do the check.
>
>> This doesn't seem to be a critical security issue since gadgetfs can't
>> be mounted from a user namespace.
>>
>> On commit 3c49de52d5647cda8b42c4255cf8a29d1e22eff5 (Dec 2).
>>
>> ==
>> BUG: KASAN: slab-out-of-bounds in dev_config+0x86f/0x1190 at addr
>> 88003c47e160
>> Write of size 65537 by task syz-executor0/6356
>> CPU: 3 PID: 6356 Comm: syz-executor0 Not tainted 4.9.0-rc7+ #19
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>> 88003c107ad8 81f96aba 3dc11ef0 110007820eee
>> ed0007820ee6 88003dc11f00 41b58ab3 8598b4c8
>> 81f96828 813fb4a0 88003b6eadc0 88003c107738
>> Call Trace:
>> [< inline >] __dump_stack lib/dump_stack.c:15
>> [] dump_stack+0x292/0x398 lib/dump_stack.c:51
>> [] kasan_object_err+0x1c/0x70 mm/kasan/report.c:159
>> [< inline >] print_address_description mm/kasan/report.c:197
>> [] kasan_report_error+0x1f0/0x4e0 mm/kasan/report.c:286
>> [] kasan_report+0x35/0x40 mm/kasan/report.c:306
>> [< inline >] check_memory_region_inline mm/kasan/kasan.c:308
>> [] check_memory_region+0x139/0x190 mm/kasan/kasan.c:315
>> [] kasan_check_write+0x14/0x20 mm/kasan/kasan.c:326
>> [< inline >] copy_from_user arch/x86/include/asm/uaccess.h:689
>> [< inline >] ep0_write drivers/usb/gadget/legacy/inode.c:1135
>> [] dev_config+0x86f/0x1190
>> drivers/usb/gadget/legacy/inode.c:1759
>> [] __vfs_write+0x5d5/0x760 fs/read_write.c:510
>> [] vfs_write+0x170/0x4e0 fs/read_write.c:560
>> [< inline >] SYSC_write fs/read_write.c:607
>> [] SyS_write+0xfb/0x230 fs/read_write.c:599
>> [] entry_SYSCALL_64_fastpath+0x1f/0xc2
>
> How does this patch work out?
I believe it fixes the issue.
Thanks!
>
> Alan Stern
>
>
>
> Index: usb-4.x/drivers/usb/gadget/legacy/inode.c
> ===
> --- usb-4.x.orig/drivers/usb/gadget/legacy/inode.c
> +++ usb-4.x/drivers/usb/gadget/legacy/inode.c
> @@ -1126,7 +1126,7 @@ ep0_write (struct file *fd, const char _
> /* data and/or status stage for control request */
> } else if (dev->state == STATE_DEV_SETUP) {
>
> - /* IN DATA+STATUS caller makes len <= wLength */
> + len = min(len, (size_t) dev->setup_wLength);
> if (dev->setup_in) {
> retval = setup_req (dev->gadget->ep0, dev->req, len);
> if (retval == 0) {
>
--
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: usb/gadget: use-after-free in gadgetfs_setup
[CC: list drastically trimmed] On Tue, 6 Dec 2016, Andrey Konovalov wrote: > On Tue, Dec 6, 2016 at 1:28 PM, Andrey Konovalov > wrote: > > On Mon, Dec 5, 2016 at 8:31 PM, Alan Stern > > wrote: > >> On Mon, 5 Dec 2016, Andrey Konovalov wrote: > >> > >>> Hi! > >>> > >>> I've got the following error report while running the syzkaller fuzzer. > >>> > >>> On commit 3c49de52d5647cda8b42c4255cf8a29d1e22eff5 (Dec 2). > >>> > >>> BUG: KASAN: use-after-free in gadgetfs_setup+0x208a/0x20e0 at addr > >>> 88003dfe5bf2 > >> Can you test whether the patch below fixes this problem? > > > > Hi Alan, > > > > Yes, I believe it does. > > It also seems to fix the warnings in dummy_free_request() I've been getting. > > It seems that I was wrong. Still see both use-after-free and warnings. You posted three messages about possibly related problems: use-after-free in gadgetfs_setup (this one), GPF in usb_gadget_unregister_driver, warning in dummy_free_request. Are you saying the patch below didn't fix any of them? And in any case, is there any way you can post the series of system calls that syzkaller makes so we can tell what went wrong? Alan Stern > >> Index: usb-4.x/drivers/usb/gadget/legacy/inode.c > >> === > >> --- usb-4.x.orig/drivers/usb/gadget/legacy/inode.c > >> +++ usb-4.x/drivers/usb/gadget/legacy/inode.c > >> @@ -1762,6 +1762,10 @@ dev_config (struct file *fd, const char > >> } > >> spin_unlock_irq(&dev->lock); > >> > >> + /* Registered but not yet bound to a UDC driver? */ > >> + if (dev->gadget_registered) > >> + return -EIO; > >> + > >> if (len < (USB_DT_CONFIG_SIZE + USB_DT_DEVICE_SIZE + 4)) > >> return -EINVAL; > >> -- 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] add dlink dwm-158 to usb-serial-option
On Tue, Dec 06, 2016 at 09:18:40PM +0100, Giuseppe Lippolis wrote:
> > Any chance you can resend this in a format we can apply it in (tabs
> properly
> > used, no line-wrap, correct signed-off-by, good subject and changelog
> text,
> > etc.)?
>
> Adding registration for 3G modem DWM-158 in usb-serial-option
>
> Signed-off-by: Giuseppe Lippolis
>
> diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
> index 9894e34..386b687 100644
> --- a/drivers/usb/serial/option.c
> +++ b/drivers/usb/serial/option.c
> @@ -1989,6 +1989,7 @@ static const struct usb_device_id option_ids[] = {
> { USB_DEVICE_AND_INTERFACE_INFO(0x2001, 0x7d02, 0xff, 0x00, 0x00) },
> { USB_DEVICE_AND_INTERFACE_INFO(0x2001, 0x7d03, 0xff, 0x02, 0x01) },
> { USB_DEVICE_AND_INTERFACE_INFO(0x2001, 0x7d03, 0xff, 0x00, 0x00) },
> + { USB_DEVICE_INTERFACE_CLASS(0x2001, 0x7d04, 0xff) },
> /* D-Link DWM-158 */
> { USB_DEVICE_INTERFACE_CLASS(0x2001, 0x7e19, 0xff),
> /* D-Link DWM-221 B1 */
> .driver_info = (kernel_ulong_t)&net_intf4_blacklist },
> { USB_DEVICE_AND_INTERFACE_INFO(0x07d1, 0x3e01, 0xff, 0xff, 0xff) },
> /* D-Link DWM-152/C1 */
>
> Is it now ok?
Your tabs are getting converted to spaces by your email client, you can
not use the web gmail client and cut-and-paste, sorry.
greg k-h
--
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 RESEND] usb: gadget: udc-core: Rescan pending list on driver unbind
Krzysztof: Your patch doesn't address a bug that has been present in usb_add_gadget_udc_release() for some time. If the bind callback fails with any error other than -EPROBE_DEFER, the driver remains unbound to the gadget but it is not added back on to the gadget_driver_pending_list. As a result, when the driver is unregistered, the list_del() call will cause problems. There's a good reason not to add the driver back on to the list: If it's not working then we don't want to keep trying to probe it over and over again. I think the best thing to do would be list_del_init(&driver->pending), so that at least unregistration won't cause an oops. There's another thing. If the bind callback fails, why not continue the loop in check_pending_gadget_drivers()? Maybe another pending driver will succeed. (Of course, to do this would require using list_for_each_entry_safe instead of list_for_each_entry.) 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
AW: [PATCH] add dlink dwm-158 to usb-serial-option
> Any chance you can resend this in a format we can apply it in (tabs
properly
> used, no line-wrap, correct signed-off-by, good subject and changelog
text,
> etc.)?
Adding registration for 3G modem DWM-158 in usb-serial-option
Signed-off-by: Giuseppe Lippolis
diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
index 9894e34..386b687 100644
--- a/drivers/usb/serial/option.c
+++ b/drivers/usb/serial/option.c
@@ -1989,6 +1989,7 @@ static const struct usb_device_id option_ids[] = {
{ USB_DEVICE_AND_INTERFACE_INFO(0x2001, 0x7d02, 0xff, 0x00, 0x00) },
{ USB_DEVICE_AND_INTERFACE_INFO(0x2001, 0x7d03, 0xff, 0x02, 0x01) },
{ USB_DEVICE_AND_INTERFACE_INFO(0x2001, 0x7d03, 0xff, 0x00, 0x00) },
+ { USB_DEVICE_INTERFACE_CLASS(0x2001, 0x7d04, 0xff) },
/* D-Link DWM-158 */
{ USB_DEVICE_INTERFACE_CLASS(0x2001, 0x7e19, 0xff),
/* D-Link DWM-221 B1 */
.driver_info = (kernel_ulong_t)&net_intf4_blacklist },
{ USB_DEVICE_AND_INTERFACE_INFO(0x07d1, 0x3e01, 0xff, 0xff, 0xff) },
/* D-Link DWM-152/C1 */
Is it now ok?
Bye.
--
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] add dlink dwm-158 to usb-serial-option
On Tue, Dec 06, 2016 at 08:55:02PM +0100, Giuseppe Lippolis wrote:
> I all,
> This patch will add the 3G modem dwm-158 found inside the DWR-512 from
> Dlink.
> The modem have 2 cdc_ether interface and 4 usb-serial.
>
> I tested the patch in the current lede system and compiled on the latest
> linux tree.
>
> diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
> index 9894e34..386b687 100644
> --- a/drivers/usb/serial/option.c
> +++ b/drivers/usb/serial/option.c
> @@ -1989,6 +1989,7 @@ static const struct usb_device_id option_ids[] = {
> { USB_DEVICE_AND_INTERFACE_INFO(0x2001, 0x7d02, 0xff, 0x00, 0x00) },
> { USB_DEVICE_AND_INTERFACE_INFO(0x2001, 0x7d03, 0xff, 0x02, 0x01) },
> { USB_DEVICE_AND_INTERFACE_INFO(0x2001, 0x7d03, 0xff, 0x00, 0x00) },
> + { USB_DEVICE_INTERFACE_CLASS(0x2001, 0x7d04, 0xff) },
> /* D-Link DWM-158 */
> { USB_DEVICE_INTERFACE_CLASS(0x2001, 0x7e19, 0xff),
> /* D-Link DWM-221 B1 */
> .driver_info = (kernel_ulong_t)&net_intf4_blacklist },
> { USB_DEVICE_AND_INTERFACE_INFO(0x07d1, 0x3e01, 0xff, 0xff, 0xff) },
> /* D-Link DWM-152/C1 */
>
Any chance you can resend this in a format we can apply it in (tabs
properly used, no line-wrap, correct signed-off-by, good subject and
changelog text, etc.)?
thanks,
greg k-h
--
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] add dlink dwm-158 to usb-serial-option
I all,
This patch will add the 3G modem dwm-158 found inside the DWR-512 from
Dlink.
The modem have 2 cdc_ether interface and 4 usb-serial.
I tested the patch in the current lede system and compiled on the latest
linux tree.
diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
index 9894e34..386b687 100644
--- a/drivers/usb/serial/option.c
+++ b/drivers/usb/serial/option.c
@@ -1989,6 +1989,7 @@ static const struct usb_device_id option_ids[] = {
{ USB_DEVICE_AND_INTERFACE_INFO(0x2001, 0x7d02, 0xff, 0x00, 0x00) },
{ USB_DEVICE_AND_INTERFACE_INFO(0x2001, 0x7d03, 0xff, 0x02, 0x01) },
{ USB_DEVICE_AND_INTERFACE_INFO(0x2001, 0x7d03, 0xff, 0x00, 0x00) },
+ { USB_DEVICE_INTERFACE_CLASS(0x2001, 0x7d04, 0xff) },
/* D-Link DWM-158 */
{ USB_DEVICE_INTERFACE_CLASS(0x2001, 0x7e19, 0xff),
/* D-Link DWM-221 B1 */
.driver_info = (kernel_ulong_t)&net_intf4_blacklist },
{ USB_DEVICE_AND_INTERFACE_INFO(0x07d1, 0x3e01, 0xff, 0xff, 0xff) },
/* D-Link DWM-152/C1 */
Result on the target:
T: Bus=01 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#= 2 Spd=480 MxCh= 0
D: Ver= 2.00 Cls=02(comm.) Sub=00 Prot=00 MxPS=64 #Cfgs= 1
P: Vendor=2001 ProdID=7d04 Rev= 3.00
S: Manufacturer=D-Link,Inc
S: Product=D-Link DWM-158
C:* #Ifs= 7 Cfg#= 1 Atr=a0 MxPwr=500mA
A: FirstIf#= 0 IfCount= 2 Cls=02(comm.) Sub=06 Prot=00
I:* If#= 0 Alt= 0 #EPs= 1 Cls=02(comm.) Sub=06 Prot=00 Driver=cdc_ether
E: Ad=88(I) Atr=03(Int.) MxPS= 64 Ivl=125us
I: If#= 1 Alt= 0 #EPs= 0 Cls=0a(data ) Sub=00 Prot=00 Driver=cdc_ether
I:* If#= 1 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=00 Driver=cdc_ether
E: Ad=81(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=01(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
I:* If#= 2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=02 Prot=01 Driver=option
E: Ad=87(I) Atr=03(Int.) MxPS= 64 Ivl=125us
E: Ad=82(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=02(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
I:* If#= 3 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
E: Ad=83(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=03(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
I:* If#= 4 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
E: Ad=84(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=04(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
I:* If#= 5 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
E: Ad=85(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=05(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
I:* If#= 6 Alt= 0 #EPs= 2 Cls=08(stor.) Sub=06 Prot=50 Driver=(none)
E: Ad=86(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=06(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
Here the relevant lsusb info:
Bus 001 Device 002: ID 2001:7d04 D-Link Corp.
Device Descriptor:
bLength18
bDescriptorType 1
bcdUSB 2.00
bDeviceClass2 Communications
bDeviceSubClass 0
bDeviceProtocol 0
bMaxPacketSize064
idVendor 0x2001 D-Link Corp.
idProduct 0x7d04
bcdDevice3.00
iManufacturer 10 D-Link,Inc
iProduct 11 D-Link DWM-158
iSerial 0
bNumConfigurations 1
Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength 229
bNumInterfaces 7
bConfigurationValue 1
iConfiguration 0
bmAttributes 0xa0
(Bus Powered)
Remote Wakeup
MaxPower 500mA
Interface Association:
bLength 8
bDescriptorType11
bFirstInterface 0
bInterfaceCount 2
bFunctionClass 2 Communications
bFunctionSubClass 6 Ethernet Networking
bFunctionProtocol 0
iFunction 2 COM(comm_if)
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber0
bAlternateSetting 0
bNumEndpoints 1
bInterfaceClass 2 Communications
bInterfaceSubClass 6 Ethernet Networking
bInterfaceProtocol 0
iInterface 2 COM(comm_if)
CDC Header:
bcdCDC 1.10
CDC Union:
bMasterInterface0
bSlaveInterface 1
CDC Ethernet:
iMacAddress 1 0200FFAA
bmEthernetStatistics0x7f18
wMaxSegmentSize 1514
wNumberMCFilters0x
bNumberPowerFilters 16
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x88 EP 8 IN
bmAttributes3
Transfer TypeInterrupt
Synch Type None
Usage Type Data
wMaxPacketSize 0x0040 1x 64 bytes
bInterval 1
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber1
Re: USB stops working if a malfunctioning USB device is connected
On 7 December 2016 at 01:09, Alan Stern wrote: > On Tue, 6 Dec 2016, PrasannaKumar Muralidharan wrote: > >> > The issue did not appear for a day with 4.9-rc5. Will check for a >> > couple more days and get back. >> >> I have been running 4.9-rc5 for quite some time. Did not observe the >> crash so far but some times my USB port stops recognising the device. >> After rebooting the system the USB port starts working. >> >> No OOPS or message related to this appeared in dmesg output. Is there >> a way to collect more info? > > You can turn on dynamic debugging for USB: > > echo 'module usbcore =p' >/sys/kernel/debug/dynamic_debug/control > > Alan Stern > Okay. Let me try to get more info and get back. Thanks, PrasannaKumar -- 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 v9 1/2] usb: dwc2: assert phy reset when waking up in rk3288 platform
On 12/6/2016 4:00 AM, Ayaka wrote:
> Hello John
> I still waiting them be merged, but I still can't find it at next-20161206.
>
Can you resubmit this fixing the checkpatch issues?
You can add my ack:
Acked-by: John Youn
Regards,
John
> 從我的 iPad 傳送
>
>> John Youn 於 2016年10月25日 上午9:30 寫道：
>>
>>> On 10/23/2016 2:33 AM, ayaka wrote:
>>>
>>>
>>>> On 10/22/2016 03:27 AM, John Youn wrote:
>>>>> On 10/20/2016 11:38 AM, Randy Li wrote:
>>>>> On the rk3288 USB host-only port (the one that's not the OTG-enabled
>>>>> port) the PHY can get into a bad state when a wakeup is asserted (not
>>>>> just a wakeup from full system suspend but also a wakeup from
>>>>> autosuspend).
>>>>>
>>>>> We can get the PHY out of its bad state by asserting its "port reset",
>>>>> but unfortunately that seems to assert a reset onto the USB bus so it
>>>>> could confuse things if we don't actually deenumerate / reenumerate the
>>>>> device.
>>>>>
>>>>> We can also get the PHY out of its bad state by fully resetting it using
>>>>> the reset from the CRU (clock reset unit) in chip, which does a more full
>>>>> reset. The CRU-based reset appears to actually cause devices on the bus
>>>>> to be removed and reinserted, which fixes the problem (albeit in a hacky
>>>>> way).
>>>>>
>>>>> It's unfortunate that we need to do a full re-enumeration of devices at
>>>>> wakeup time, but this is better than alternative of letting the bus get
>>>>> wedged.
>>>>>
>>>>> Signed-off-by: Randy Li
>>>>> ---
>>>>> drivers/usb/dwc2/core.h | 1 +
>>>>> drivers/usb/dwc2/core_intr.c | 11 +++
>>>>> drivers/usb/dwc2/platform.c | 9 +
>>>>> 3 files changed, 21 insertions(+)
>>>>>
>>>>> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
>>>>> index 2a21a04..e91ddbc 100644
>>>>> --- a/drivers/usb/dwc2/core.h
>>>>> +++ b/drivers/usb/dwc2/core.h
>>>>> @@ -859,6 +859,7 @@ struct dwc2_hsotg {
>>>>> unsigned int ll_hw_enabled:1;
>>>>>
>>>>> struct phy *phy;
>>>>> +struct work_struct phy_rst_work;
>>>>> struct usb_phy *uphy;
>>>>> struct dwc2_hsotg_plat *plat;
>>>>> struct regulator_bulk_data
>>>>> supplies[ARRAY_SIZE(dwc2_hsotg_supply_names)];
>>>>> diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c
>>>>> index d85c5c9..c3d2168 100644
>>>>> --- a/drivers/usb/dwc2/core_intr.c
>>>>> +++ b/drivers/usb/dwc2/core_intr.c
>>>>> @@ -345,6 +345,7 @@ static void dwc2_handle_session_req_intr(struct
>>>>> dwc2_hsotg *hsotg)
>>>>> static void dwc2_handle_wakeup_detected_intr(struct dwc2_hsotg *hsotg)
>>>>> {
>>>>> int ret;
>>>>> +struct device_node *np = hsotg->dev->of_node;
>>>>>
>>>>> /* Clear interrupt */
>>>>> dwc2_writel(GINTSTS_WKUPINT, hsotg->regs + GINTSTS);
>>>>> @@ -379,6 +380,16 @@ static void dwc2_handle_wakeup_detected_intr(struct
>>>>> dwc2_hsotg *hsotg)
>>>>> /* Restart the Phy Clock */
>>>>> pcgcctl &= ~PCGCTL_STOPPCLK;
>>>>> dwc2_writel(pcgcctl, hsotg->regs + PCGCTL);
>>>>> +
>>>>> +/*
>>>>> + * It is a quirk in Rockchip RK3288, causing by
>>>>> + * a hardware bug. This will propagate out and
>>>>> + * eventually we'll re-enumerate the device.
>>>>> + * Not great but the best we can do
>>>>> + */
>>>>> +if (of_device_is_compatible(np, "rockchip,rk3288-usb"))
>>>>> +schedule_work(&hsotg->phy_rst_work);
>>>>> +
>>>>> mod_timer(&hsotg->wkp_timer,
>>>>>jiffies + msecs_to_jiffies(71));
>>>>> } else {
>>>>> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
>>>>> index 8e1728b..65953cf 100644
>>>>> --- a/drivers/
Re: USB stops working if a malfunctioning USB device is connected
On Tue, 6 Dec 2016, PrasannaKumar Muralidharan wrote: > > The issue did not appear for a day with 4.9-rc5. Will check for a > > couple more days and get back. > > I have been running 4.9-rc5 for quite some time. Did not observe the > crash so far but some times my USB port stops recognising the device. > After rebooting the system the USB port starts working. > > No OOPS or message related to this appeared in dmesg output. Is there > a way to collect more info? You can turn on dynamic debugging for USB: echo 'module usbcore =p' >/sys/kernel/debug/dynamic_debug/control 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: USB stops working if a malfunctioning USB device is connected
> The issue did not appear for a day with 4.9-rc5. Will check for a > couple more days and get back. I have been running 4.9-rc5 for quite some time. Did not observe the crash so far but some times my USB port stops recognising the device. After rebooting the system the USB port starts working. No OOPS or message related to this appeared in dmesg output. Is there a way to collect more info? Thanks, PrasannaKumar -- 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: [BUG 4.9] New led trigger usbport gets the kernel to panic
On 6 December 2016 at 18:26, Pavel Machek wrote: > On Fri 2016-12-02 09:48:18, Ralph Sennhauser wrote: >> On Thu, 1 Dec 2016 17:56:07 +0100 >> Rafał Miłecki wrote: >> >> > On 12/01/2016 03:28 PM, Ralph Sennhauser wrote: >> > > Below the oops with your debug patch applied. >> > > >> > > (...) >> > > >> > > root@wrt1900acs:/# cd sys/class/leds/pca963x\:shelby\:white\:usb2/ >> > > root@wrt1900acs:/sys/devices/platform/soc/soc:internal-regs/f1011000.i2c/i2c-0/0-0068/leds/pca963x:shelby:white:usb2# >> > > echo usbport > trigger [ 124.727665] leds >> > > pca963x:shelby:white:usb2: [usbport_trig_add_port] port:dd708240 >> > > [ 124.735266] leds pca963x:shelby:white:usb2: >> > > [usbport_trig_add_port] port->port_name:usb1-port1 >> > > port->data:dd708140 [ 124.745671] leds pca963x:shelby:white:usb2: >> > > [usbport_trig_add_port] port:dd7083c0 [ 124.753194] leds >> > > pca963x:shelby:white:usb2: [usbport_trig_add_port] >> > > port->port_name:usb2-port1 port->data:dd708140 [ 124.763594] leds >> > > pca963x:shelby:white:usb2: [usbport_trig_add_port] port:dd708300 >> > > [ 124.771114] leds pca963x:shelby:white:usb2: >> > > [usbport_trig_add_port] port->port_name:usb3-port1 >> > > port->data:dd708140 >> > > root@wrt1900acs:/sys/devices/platform/soc/soc:internal-regs/f1011000.i2c/i2c-0/0-0068/leds/pca963x:shelby:white:usb2# >> > > echo 1 > ports/usb2-port1[ 171.649751] leds >> > > pca963x:shelby:white:usb2: [usbport_trig_port_store] buf:1 >> > > [ 171.649751] size:2 >> > > >> > > [ 171.660160] leds pca963x:shelby:white:usb2: >> > > [usbport_trig_port_store] port:dd7083c0 [ 171.668103] leds >> > > pca963x:shelby:white:usb2: [usbport_trig_port_store] >> > > port->port_name:usb2-port1 port->data:dd708140 [ 171.678678] >> > > [usbport_trig_update_count] usbport_data->count:0 [ 171.684457] >> > > [usbport_trig_update_count] usbport_data->count:0 [ 171.690253] >> > > Unable to handle kernel NULL pointer dereference at virtual address >> > > >> > >> > Oh, so this happens a bit later than I expected or I could read from >> > the backtrace. Anyway this debugging was still helpful, I think I can >> > see a possible problem. >> > >> > So most likely the crash happens at the: >> > led_cdev->brightness_set(led_cdev, ...); >> > call. I'm not sure what I was thinking when writing that code. It >> > looks wrong. >> > >> > The thing is some LEDs (drivers) don't provide brightness_set op. >> > It's fine, we should just use brightness_set_blocking op when that >> > happens. Of course kernel has proper helpers for that, we don't have >> > to worry about the choice of op or scheduling the work. I have no >> > idea why I didn't use a proper helper in the first place. >> > >> > So we should simply replace above call with one of following ones: >> > 1) led_set_brightness(led_cdev, ...); >> > 2) led_set_brightness_nosleep(led_cdev, ...); >> > 3) led_set_brightness_sync(led_cdev, ...); >> > >> > I still have to check which one is correct. In theory we don't deal >> > blinking at this point so we shouldn't need to use led_set_brightness. >> > >> > led_set_brightness_nosleep looks like the most likely correct choice. >> > >> > led_set_brightness_sync requires brightness_set_blocking which is not >> > always present so most likely we don't want this one. >> > >> > >> > If you have some free time and you want to play with this, please >> > replace led_cdev->brightness_set >> > with >> > led_set_brightness_nosleep >> > and give it a try. This should fix crashes for you. >> > >> > I'll look at this again during next days. >> >> Hi Rafał >> >> I just gave 2) led_set_brightness_nosleep a try. The kernel no longer >> crashes and the led does work as expected. > > Do you have any patch that needs to be applied? Yes, it has been pushed: https://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id=89778ba335e302a450932ce5b703c1ee6216e949 When sending it I didn't Cc linux-leds as get_maintainer.pl didn't pick it. Should we add MAINTAINERS entry for ledtrig-usbport.c maybe? -- Rafał -- 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: [BUG 4.9] New led trigger usbport gets the kernel to panic
On Fri 2016-12-02 09:48:18, Ralph Sennhauser wrote: > On Thu, 1 Dec 2016 17:56:07 +0100 > Rafał Miłecki wrote: > > > On 12/01/2016 03:28 PM, Ralph Sennhauser wrote: > > > Below the oops with your debug patch applied. > > > > > > (...) > > > > > > root@wrt1900acs:/# cd sys/class/leds/pca963x\:shelby\:white\:usb2/ > > > root@wrt1900acs:/sys/devices/platform/soc/soc:internal-regs/f1011000.i2c/i2c-0/0-0068/leds/pca963x:shelby:white:usb2# > > > echo usbport > trigger [ 124.727665] leds > > > pca963x:shelby:white:usb2: [usbport_trig_add_port] port:dd708240 > > > [ 124.735266] leds pca963x:shelby:white:usb2: > > > [usbport_trig_add_port] port->port_name:usb1-port1 > > > port->data:dd708140 [ 124.745671] leds pca963x:shelby:white:usb2: > > > [usbport_trig_add_port] port:dd7083c0 [ 124.753194] leds > > > pca963x:shelby:white:usb2: [usbport_trig_add_port] > > > port->port_name:usb2-port1 port->data:dd708140 [ 124.763594] leds > > > pca963x:shelby:white:usb2: [usbport_trig_add_port] port:dd708300 > > > [ 124.771114] leds pca963x:shelby:white:usb2: > > > [usbport_trig_add_port] port->port_name:usb3-port1 > > > port->data:dd708140 > > > root@wrt1900acs:/sys/devices/platform/soc/soc:internal-regs/f1011000.i2c/i2c-0/0-0068/leds/pca963x:shelby:white:usb2# > > > echo 1 > ports/usb2-port1[ 171.649751] leds > > > pca963x:shelby:white:usb2: [usbport_trig_port_store] buf:1 > > > [ 171.649751] size:2 > > > > > > [ 171.660160] leds pca963x:shelby:white:usb2: > > > [usbport_trig_port_store] port:dd7083c0 [ 171.668103] leds > > > pca963x:shelby:white:usb2: [usbport_trig_port_store] > > > port->port_name:usb2-port1 port->data:dd708140 [ 171.678678] > > > [usbport_trig_update_count] usbport_data->count:0 [ 171.684457] > > > [usbport_trig_update_count] usbport_data->count:0 [ 171.690253] > > > Unable to handle kernel NULL pointer dereference at virtual address > > > > > > > Oh, so this happens a bit later than I expected or I could read from > > the backtrace. Anyway this debugging was still helpful, I think I can > > see a possible problem. > > > > So most likely the crash happens at the: > > led_cdev->brightness_set(led_cdev, ...); > > call. I'm not sure what I was thinking when writing that code. It > > looks wrong. > > > > The thing is some LEDs (drivers) don't provide brightness_set op. > > It's fine, we should just use brightness_set_blocking op when that > > happens. Of course kernel has proper helpers for that, we don't have > > to worry about the choice of op or scheduling the work. I have no > > idea why I didn't use a proper helper in the first place. > > > > So we should simply replace above call with one of following ones: > > 1) led_set_brightness(led_cdev, ...); > > 2) led_set_brightness_nosleep(led_cdev, ...); > > 3) led_set_brightness_sync(led_cdev, ...); > > > > I still have to check which one is correct. In theory we don't deal > > blinking at this point so we shouldn't need to use led_set_brightness. > > > > led_set_brightness_nosleep looks like the most likely correct choice. > > > > led_set_brightness_sync requires brightness_set_blocking which is not > > always present so most likely we don't want this one. > > > > > > If you have some free time and you want to play with this, please > > replace led_cdev->brightness_set > > with > > led_set_brightness_nosleep > > and give it a try. This should fix crashes for you. > > > > I'll look at this again during next days. > > Hi Rafał > > I just gave 2) led_set_brightness_nosleep a try. The kernel no longer > crashes and the led does work as expected. Do you have any patch that needs to be applied? -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: usb/gadget: use-after-free in gadgetfs_setup
On Tue, Dec 6, 2016 at 1:28 PM, Andrey Konovalov wrote: > On Mon, Dec 5, 2016 at 8:31 PM, Alan Stern wrote: >> On Mon, 5 Dec 2016, Andrey Konovalov wrote: >> >>> Hi! >>> >>> I've got the following error report while running the syzkaller fuzzer. >>> >>> On commit 3c49de52d5647cda8b42c4255cf8a29d1e22eff5 (Dec 2). >>> >>> BUG: KASAN: use-after-free in gadgetfs_setup+0x208a/0x20e0 at addr >>> 88003dfe5bf2 >>> Read of size 2 by task syz-executor0/22994 >>> CPU: 3 PID: 22994 Comm: syz-executor0 Not tainted 4.9.0-rc7+ #16 >>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 >>> 88006df06a18 81f96aba e0528500 11000dbe0cd6 >>> ed000dbe0cce 88006df068f0 41b58ab3 8598b4c8 >>> 81f96828 11000dbe0ccd 88006df06708 88006df06748 >>> Call Trace: >>> [ 201.343209] [< inline >] __dump_stack lib/dump_stack.c:15 >>> [ 201.343209] [] dump_stack+0x292/0x398 >>> lib/dump_stack.c:51 >>> [] kasan_object_err+0x1c/0x70 mm/kasan/report.c:159 >>> [< inline >] print_address_description mm/kasan/report.c:197 >>> [] kasan_report_error+0x1f0/0x4e0 mm/kasan/report.c:286 >>> [< inline >] kasan_report mm/kasan/report.c:306 >>> [] __asan_report_load_n_noabort+0x3a/0x40 >>> mm/kasan/report.c:337 >>> [< inline >] config_buf drivers/usb/gadget/legacy/inode.c:1298 >>> [] gadgetfs_setup+0x208a/0x20e0 >>> drivers/usb/gadget/legacy/inode.c:1368 >>> [] dummy_timer+0x11f0/0x36d0 >>> drivers/usb/gadget/udc/dummy_hcd.c:1858 >>> [] call_timer_fn+0x241/0x800 kernel/time/timer.c:1308 >>> [< inline >] expire_timers kernel/time/timer.c:1348 >>> [] __run_timers+0xa06/0xec0 kernel/time/timer.c:1641 >>> [] run_timer_softirq+0x21/0x80 kernel/time/timer.c:1654 >>> [] __do_softirq+0x2fb/0xb63 kernel/softirq.c:284 >>> [< inline >] invoke_softirq kernel/softirq.c:364 >>> [] irq_exit+0x19e/0x1d0 kernel/softirq.c:405 >>> [< inline >] exiting_irq arch/x86/include/asm/apic.h:659 >>> [] smp_apic_timer_interrupt+0x7b/0xa0 >>> arch/x86/kernel/apic/apic.c:960 >>> [] apic_timer_interrupt+0x8c/0xa0 >>> arch/x86/entry/entry_64.S:489 >>> [ 201.343209] [] ? >>> _raw_spin_unlock_irqrestore+0x119/0x1a0 >>> [] try_to_wake_up+0x174/0x1160 kernel/sched/core.c:2099 >>> [< inline >] wake_up_process kernel/sched/core.c:2165 >>> [] wake_up_q+0x8a/0xe0 kernel/sched/core.c:469 >>> [] futex_wake+0x5be/0x6c0 kernel/futex.c:1453 >>> [] do_futex+0x11bd/0x1f00 kernel/futex.c:3219 >>> [< inline >] SYSC_futex kernel/futex.c:3275 >>> [] SyS_futex+0x2fc/0x400 kernel/futex.c:3243 >>> [] entry_SYSCALL_64_fastpath+0x1f/0xc2 >> >> Can you test whether the patch below fixes this problem? > > Hi Alan, > > Yes, I believe it does. > It also seems to fix the warnings in dummy_free_request() I've been getting. It seems that I was wrong. Still see both use-after-free and warnings. > > Thanks! > >> >> Alan Stern >> >> >> >> Index: usb-4.x/drivers/usb/gadget/legacy/inode.c >> === >> --- usb-4.x.orig/drivers/usb/gadget/legacy/inode.c >> +++ usb-4.x/drivers/usb/gadget/legacy/inode.c >> @@ -1762,6 +1762,10 @@ dev_config (struct file *fd, const char >> } >> spin_unlock_irq(&dev->lock); >> >> + /* Registered but not yet bound to a UDC driver? */ >> + if (dev->gadget_registered) >> + return -EIO; >> + >> if (len < (USB_DT_CONFIG_SIZE + USB_DT_DEVICE_SIZE + 4)) >> return -EINVAL; >> >> -- 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: net/gadget: slab-out-of-bounds write in dev_config
On Tue, 6 Dec 2016, Andrey Konovalov wrote:
> Hi!
>
> I've got the following error report while running the syzkaller fuzzer.
>
> ep0_write() doesn't check the length, so a user can cause an
> out-of-bounds with both size and data controlled.
> There's a comment which says "IN DATA+STATUS caller makes len <=
> wLength". While I'm not exactly sure what that means, the length seems
> to be passed unmodified directly from dev_config().
You're right about the comment being misleading. It looks like
somebody forgot to actually do the check.
> This doesn't seem to be a critical security issue since gadgetfs can't
> be mounted from a user namespace.
>
> On commit 3c49de52d5647cda8b42c4255cf8a29d1e22eff5 (Dec 2).
>
> ==
> BUG: KASAN: slab-out-of-bounds in dev_config+0x86f/0x1190 at addr
> 88003c47e160
> Write of size 65537 by task syz-executor0/6356
> CPU: 3 PID: 6356 Comm: syz-executor0 Not tainted 4.9.0-rc7+ #19
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> 88003c107ad8 81f96aba 3dc11ef0 110007820eee
> ed0007820ee6 88003dc11f00 41b58ab3 8598b4c8
> 81f96828 813fb4a0 88003b6eadc0 88003c107738
> Call Trace:
> [< inline >] __dump_stack lib/dump_stack.c:15
> [] dump_stack+0x292/0x398 lib/dump_stack.c:51
> [] kasan_object_err+0x1c/0x70 mm/kasan/report.c:159
> [< inline >] print_address_description mm/kasan/report.c:197
> [] kasan_report_error+0x1f0/0x4e0 mm/kasan/report.c:286
> [] kasan_report+0x35/0x40 mm/kasan/report.c:306
> [< inline >] check_memory_region_inline mm/kasan/kasan.c:308
> [] check_memory_region+0x139/0x190 mm/kasan/kasan.c:315
> [] kasan_check_write+0x14/0x20 mm/kasan/kasan.c:326
> [< inline >] copy_from_user arch/x86/include/asm/uaccess.h:689
> [< inline >] ep0_write drivers/usb/gadget/legacy/inode.c:1135
> [] dev_config+0x86f/0x1190
> drivers/usb/gadget/legacy/inode.c:1759
> [] __vfs_write+0x5d5/0x760 fs/read_write.c:510
> [] vfs_write+0x170/0x4e0 fs/read_write.c:560
> [< inline >] SYSC_write fs/read_write.c:607
> [] SyS_write+0xfb/0x230 fs/read_write.c:599
> [] entry_SYSCALL_64_fastpath+0x1f/0xc2
How does this patch work out?
Alan Stern
Index: usb-4.x/drivers/usb/gadget/legacy/inode.c
===
--- usb-4.x.orig/drivers/usb/gadget/legacy/inode.c
+++ usb-4.x/drivers/usb/gadget/legacy/inode.c
@@ -1126,7 +1126,7 @@ ep0_write (struct file *fd, const char _
/* data and/or status stage for control request */
} else if (dev->state == STATE_DEV_SETUP) {
- /* IN DATA+STATUS caller makes len <= wLength */
+ len = min(len, (size_t) dev->setup_wLength);
if (dev->setup_in) {
retval = setup_req (dev->gadget->ep0, dev->req, len);
if (retval == 0) {
--
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] usb: musb: Add a quirk to preserve MUSB_DEVCTL during suspend
On Monday 05 December 2016 10:17 PM, Bin Liu wrote: > On Fri, Dec 02, 2016 at 03:23:50PM +0100, Alexandre Bailon wrote: >> On 11/29/2016 06:56 PM, Bin Liu wrote: >>> On Tue, Nov 29, 2016 at 06:41:04PM +0100, Alexandre Bailon wrote: On 11/29/2016 05:16 PM, Bin Liu wrote: > Hi, > > On Mon, Nov 28, 2016 at 05:26:20PM +0100, Alexandre Bailon wrote: >> On da8xx, VBUS is not maintained during suspend when musb is in host >> mode. >> On resume, all the connected devices will be disconnected and then will >> be enumerated again. >> This happens because MUSB_DEVCTL is cleared during suspend. >> Add a quirk to preserve MUSB_DEVCTL during a suspend. > > Will preserving MUSB_DEVCTL prevent the device getting disconnected? Yes. Preserving MUSB_DEVCTL keeps VBUS on though the PHY is off during suspend. >>> >>> VBUS will be on, but does it prevent disconnecting the usb device on >>> resume? I don't have a DA8xx to test, but I doubt it, since the PHY is >>> off. >> Actually, the PHY is not really off. > > I guess the PHY should be off when the system is suspended for an > aggressive power saving. > > Sekhar, any comments? No, nothing from my side. I haven't really played with this side of DA850 at all. For your reference, this is what chapter 10.11.1 referenced by Alexandre says: " The USB modules can be clock gated using the PSC; however, this does not power down/clock gate the PHY logic. You can put the USB2.0 PHY and OTG module in the lowest power state, when not in use, by writing to the USB0PHYPWDN and the USB0OTGPWRDN bits in the Chip Configuration 2 Register (CFGCHIP2) in the System Configuration (SYSCFG) Module chapter. " It is little bit ambiguous on if USB0PHYPWDN and USB0OTGPWRDN do really power down the PHY. I would just program the bits suggested by the manual and assume that the hardware reaches the safest low power state it can reach. Thanks, Sekhar -- 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] usb: musb: Add a quirk to preserve MUSB_DEVCTL during suspend
On Tue, Dec 06, 2016 at 06:29:31PM +0530, Sekhar Nori wrote: > On Monday 05 December 2016 10:17 PM, Bin Liu wrote: > > On Fri, Dec 02, 2016 at 03:23:50PM +0100, Alexandre Bailon wrote: > >> On 11/29/2016 06:56 PM, Bin Liu wrote: > >>> On Tue, Nov 29, 2016 at 06:41:04PM +0100, Alexandre Bailon wrote: > On 11/29/2016 05:16 PM, Bin Liu wrote: > > Hi, > > > > On Mon, Nov 28, 2016 at 05:26:20PM +0100, Alexandre Bailon wrote: > >> On da8xx, VBUS is not maintained during suspend when musb is in host > >> mode. > >> On resume, all the connected devices will be disconnected and then will > >> be enumerated again. > >> This happens because MUSB_DEVCTL is cleared during suspend. > >> Add a quirk to preserve MUSB_DEVCTL during a suspend. > > > > Will preserving MUSB_DEVCTL prevent the device getting disconnected? > Yes. Preserving MUSB_DEVCTL keeps VBUS on though the PHY is off during > suspend. > >>> > >>> VBUS will be on, but does it prevent disconnecting the usb device on > >>> resume? I don't have a DA8xx to test, but I doubt it, since the PHY is > >>> off. > >> Actually, the PHY is not really off. > > > > I guess the PHY should be off when the system is suspended for an > > aggressive power saving. > > > > Sekhar, any comments? > > No, nothing from my side. I haven't really played with this side of > DA850 at all. Ok, I wanted to know the power requirement in system suspend - do we want to power off the PHY to save power as much as possible, or leave the PHY on to not disconnect enumerated devices. > > For your reference, this is what chapter 10.11.1 referenced by Alexandre > says: > > " > The USB modules can be clock gated using the PSC; however, this does not > power down/clock gate the PHY logic. You can put the USB2.0 PHY and OTG > module in the lowest power state, when not in use, by writing to the > USB0PHYPWDN and the USB0OTGPWRDN bits in the Chip Configuration 2 > Register (CFGCHIP2) in the System Configuration (SYSCFG) Module chapter. > " It sounds like the PHY cannot be turned off on DA8xx? > > It is little bit ambiguous on if USB0PHYPWDN and USB0OTGPWRDN do really > power down the PHY. > > I would just program the bits suggested by the manual and assume that > the hardware reaches the safest low power state it can reach. > > Thanks, > Sekhar Regards, -Bin. -- 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
net/gadget: slab-out-of-bounds write in dev_config
Hi! I've got the following error report while running the syzkaller fuzzer. ep0_write() doesn't check the length, so a user can cause an out-of-bounds with both size and data controlled. There's a comment which says "IN DATA+STATUS caller makes len <= wLength". While I'm not exactly sure what that means, the length seems to be passed unmodified directly from dev_config(). This doesn't seem to be a critical security issue since gadgetfs can't be mounted from a user namespace. On commit 3c49de52d5647cda8b42c4255cf8a29d1e22eff5 (Dec 2). == BUG: KASAN: slab-out-of-bounds in dev_config+0x86f/0x1190 at addr 88003c47e160 Write of size 65537 by task syz-executor0/6356 CPU: 3 PID: 6356 Comm: syz-executor0 Not tainted 4.9.0-rc7+ #19 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 88003c107ad8 81f96aba 3dc11ef0 110007820eee ed0007820ee6 88003dc11f00 41b58ab3 8598b4c8 81f96828 813fb4a0 88003b6eadc0 88003c107738 Call Trace: [< inline >] __dump_stack lib/dump_stack.c:15 [] dump_stack+0x292/0x398 lib/dump_stack.c:51 [] kasan_object_err+0x1c/0x70 mm/kasan/report.c:159 [< inline >] print_address_description mm/kasan/report.c:197 [] kasan_report_error+0x1f0/0x4e0 mm/kasan/report.c:286 [] kasan_report+0x35/0x40 mm/kasan/report.c:306 [< inline >] check_memory_region_inline mm/kasan/kasan.c:308 [] check_memory_region+0x139/0x190 mm/kasan/kasan.c:315 [] kasan_check_write+0x14/0x20 mm/kasan/kasan.c:326 [< inline >] copy_from_user arch/x86/include/asm/uaccess.h:689 [< inline >] ep0_write drivers/usb/gadget/legacy/inode.c:1135 [] dev_config+0x86f/0x1190 drivers/usb/gadget/legacy/inode.c:1759 [] __vfs_write+0x5d5/0x760 fs/read_write.c:510 [] vfs_write+0x170/0x4e0 fs/read_write.c:560 [< inline >] SYSC_write fs/read_write.c:607 [] SyS_write+0xfb/0x230 fs/read_write.c:599 [] entry_SYSCALL_64_fastpath+0x1f/0xc2 Object at 88003c47e038, in cache kmalloc-1024 size: 1024 Allocated: PID = 4565 [ 43.417154] [] save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:57 [ 43.417154] [] save_stack+0x43/0xd0 mm/kasan/kasan.c:495 [ 43.417154] [< inline >] set_track mm/kasan/kasan.c:507 [ 43.417154] [] kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:598 [ 43.417154] [] kmem_cache_alloc_trace+0x82/0x270 mm/slub.c:2735 [ 43.417154] [< inline >] kmalloc include/linux/slab.h:490 [ 43.417154] [< inline >] kzalloc include/linux/slab.h:636 [ 43.417154] [< inline >] dev_new drivers/usb/gadget/legacy/inode.c:170 [ 43.417154] [] gadgetfs_fill_super+0x24a/0x540 drivers/usb/gadget/legacy/inode.c:1987 [ 43.417154] [] mount_single+0xf1/0x160 fs/super.c:1146 [ 43.417154] [] gadgetfs_mount+0x2c/0x40 drivers/usb/gadget/legacy/inode.c:2013 [ 43.417154] [] mount_fs+0x97/0x2e0 fs/super.c:1177 [ 43.417154] [] vfs_kern_mount.part.24+0x67/0x2f0 fs/namespace.c:954 [ 43.417154] [< inline >] vfs_kern_mount fs/namespace.c:2433 [ 43.417154] [< inline >] do_new_mount fs/namespace.c:2436 [ 43.417154] [] do_mount+0x418/0x2da0 fs/namespace.c:2758 [ 43.417154] [< inline >] SYSC_mount fs/namespace.c:2974 [ 43.417154] [] SyS_mount+0xab/0x120 fs/namespace.c:2951 [ 43.417154] [] entry_SYSCALL_64_fastpath+0x1f/0xc2 Freed: PID = 0 (stack is not available) Memory state around the buggy address: 88003c47e100: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 88003c47e180: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >88003c47e200: 00 00 00 00 00 00 00 00 00 00 00 00 fc fc fc fc ^ 88003c47e280: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc 88003c47e300: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc == -- 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] Fixed to checkpatch errors to usbtmc.c
Hello all,
I will solve a checkpatch.pl script errors.
drivers/usb/class/usbtmc.c:719: ERROR: else should follow close brace '}'
drivers/usb/class/usbtmc.c:735: ERROR: space required after that ',' (ctx:VxV)
drivers/usb/class/usbtmc.c:735: ERROR: space required after that ',' (ctx:VxV)
drivers/usb/class/usbtmc.c:735: ERROR: space required after that ',' (ctx:VxV)
drivers/usb/class/usbtmc.c:746: ERROR: else should follow close brace '}'
drivers/usb/class/usbtmc.c:752: ERROR: space required after that ',' (ctx:VxV)
drivers/usb/class/usbtmc.c:752: ERROR: space required after that ',' (ctx:VxV)
drivers/usb/class/usbtmc.c:1403: ERROR: space required before the open
parenthesis '('
total: 8 errors, 25 warnings, 1558 lines checked
Regards,
Signed-off-by: Ozgur Karatas
---
drivers/usb/class/usbtmc.c | 12 +---
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index a6c1fae..3b29871 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -715,8 +715,7 @@ static ssize_t usbtmc_read(struct file *filp, char __user
*buf,
/* Remove padding if it exists */
if (actual > remaining)
actual = remaining;
- }
- else {
+ } else {
if (this_part > n_characters)
this_part = n_characters;
/* Remove padding if it exists */
@@ -732,7 +731,7 @@ static ssize_t usbtmc_read(struct file *filp, char __user
*buf,
if ((buffer[8] & 0x01) && (actual >= n_characters))
remaining = 0;
- dev_dbg(dev, "Bulk-IN header: remaining(%zu), buf(%p),
buffer(%p) done(%zu)\n", remaining,buf,buffer,done);
+ dev_dbg(dev, "Bulk-IN header: remaining(%zu), buf(%p),
buffer(%p) done(%zu)\n", remaining, buf, buffer, done);
/* Copy buffer to user space */
@@ -742,14 +741,13 @@ static ssize_t usbtmc_read(struct file *filp, char __user
*buf,
goto exit;
}
done += actual;
- }
- else {
+ } else {
if (actual > remaining)
actual = remaining;
remaining -= actual;
- dev_dbg(dev, "Bulk-IN header cont: actual(%u),
done(%zu), remaining(%zu), buf(%p), buffer(%p)\n", actual, done,
remaining,buf,buffer);
+ dev_dbg(dev, "Bulk-IN header cont: actual(%u),
done(%zu), remaining(%zu), buf(%p), buffer(%p)\n", actual, done, remaining,
buf, buffer);
/* Copy buffer to user space */
if (copy_to_user(buf + done, buffer, actual)) {
@@ -1400,7 +1398,7 @@ static int usbtmc_probe(struct usb_interface *intf,
dev_dbg(&intf->dev, "Trying to find if device Vendor 0x%04X Product
0x%04X has the RIGOL quirk\n",
le16_to_cpu(data->usb_dev->descriptor.idVendor),
le16_to_cpu(data->usb_dev->descriptor.idProduct));
- for(n = 0; usbtmc_id_quirk[n].idVendor > 0; n++) {
+ for (n = 0; usbtmc_id_quirk[n].idVendor > 0; n++) {
if ((usbtmc_id_quirk[n].idVendor ==
le16_to_cpu(data->usb_dev->descriptor.idVendor)) &&
(usbtmc_id_quirk[n].idProduct ==
le16_to_cpu(data->usb_dev->descriptor.idProduct))) {
dev_dbg(&intf->dev, "Setting this device as having the
RIGOL quirk\n");
--
2.1.4
--
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 RESEND] usb: gadget: udc-core: Rescan pending list on driver unbind
Since:
commit 855ed04a3758 ("usb: gadget: udc-core: independent registration
of gadgets and gadget drivers")
if we load gadget module but there is no free udc available
then it will be stored on a pending gadgets list.
$ modprobe g_zero.ko
$ modprobe g_ether.ko
[] udc-core: couldn't find an available UDC - added [g_ether] to list
of pending drivers
We scan this list each time when new UDC appears in system.
But we can get a free UDC each time after gadget unbind.
This commit add scanning of that list directly after unbinding
gadget from udc.
Thanks to this, when we unload first gadget:
$ rmmod g_zero.ko
gadget which is pending is automatically
attached to that UDC (if name matches).
Signed-off-by: Krzysztof Opasiak
---
Changes since v1:
- remove unused variable driver
---
drivers/usb/gadget/udc/core.c | 45 +--
1 file changed, 31 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
index 9483489080f6..73292459acd6 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -1080,6 +1080,24 @@ static void usb_udc_nop_release(struct device *dev)
dev_vdbg(dev, "%s\n", __func__);
}
+/* should be called with udc_lock held */
+static int check_pending_gadget_drivers(struct usb_udc *udc)
+{
+ struct usb_gadget_driver *driver;
+ int ret = 0;
+
+ list_for_each_entry(driver, &gadget_driver_pending_list, pending)
+ if (!driver->udc_name || strcmp(driver->udc_name,
+ dev_name(&udc->dev)) == 0) {
+ ret = udc_bind_to_driver(udc, driver);
+ if (ret != -EPROBE_DEFER)
+ list_del(&driver->pending);
+ break;
+ }
+
+ return ret;
+}
+
/**
* usb_add_gadget_udc_release - adds a new gadget to the udc class driver list
* @parent: the parent device to this udc. Usually the controller driver's
@@ -1093,7 +,6 @@ int usb_add_gadget_udc_release(struct device *parent,
struct usb_gadget *gadget,
void (*release)(struct device *dev))
{
struct usb_udc *udc;
- struct usb_gadget_driver *driver;
int ret = -ENOMEM;
udc = kzalloc(sizeof(*udc), GFP_KERNEL);
@@ -1136,17 +1153,9 @@ int usb_add_gadget_udc_release(struct device *parent,
struct usb_gadget *gadget,
udc->vbus = true;
/* pick up one of pending gadget drivers */
- list_for_each_entry(driver, &gadget_driver_pending_list, pending) {
- if (!driver->udc_name || strcmp(driver->udc_name,
- dev_name(&udc->dev)) == 0) {
- ret = udc_bind_to_driver(udc, driver);
- if (ret != -EPROBE_DEFER)
- list_del(&driver->pending);
- if (ret)
- goto err5;
- break;
- }
- }
+ ret = check_pending_gadget_drivers(udc);
+ if (ret)
+ goto err5;
mutex_unlock(&udc_lock);
@@ -1352,14 +1361,22 @@ int usb_gadget_unregister_driver(struct
usb_gadget_driver *driver)
return -EINVAL;
mutex_lock(&udc_lock);
- list_for_each_entry(udc, &udc_list, list)
+ list_for_each_entry(udc, &udc_list, list) {
if (udc->driver == driver) {
usb_gadget_remove_driver(udc);
usb_gadget_set_state(udc->gadget,
- USB_STATE_NOTATTACHED);
+USB_STATE_NOTATTACHED);
+
+ /* Maybe there is someone waiting for this UDC? */
+ check_pending_gadget_drivers(udc);
+ /*
+* For now we ignore bind errors as probably it's
+* not a valid reason to fail other's gadget unbind
+*/
ret = 0;
break;
}
+ }
if (ret) {
list_del(&driver->pending);
--
2.9.3
--
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 RESEND] usb: gadget: udc-core: Rescan pending list on driver unbind
Hi Krzysztof,
[auto build test WARNING on usb/usb-testing]
[also build test WARNING on v4.9-rc8 next-20161205]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Krzysztof-Opasiak/usb-gadget-udc-core-Rescan-pending-list-on-driver-unbind/20161206-185131
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
usb-testing
config: x86_64-randconfig-s0-12061826 (attached as .config)
compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
drivers/usb/gadget/udc/core.c: In function 'usb_add_gadget_udc_release':
>> drivers/usb/gadget/udc/core.c:1114: warning: unused variable 'driver'
At top level:
cc1: warning: unrecognized command line option "-Wno-maybe-uninitialized"
drivers/usb/gadget/udc/core.o: warning: objtool:
usb_gadget_map_request_by_dev()+0x90: function has unreachable instruction
vim +/driver +1114 drivers/usb/gadget/udc/core.c
60068fb97 drivers/usb/gadget/udc/core.c Krzysztof Opasiak 2016-12-06 1098
return ret;
60068fb97 drivers/usb/gadget/udc/core.c Krzysztof Opasiak 2016-12-06 1099
}
60068fb97 drivers/usb/gadget/udc/core.c Krzysztof Opasiak 2016-12-06 1100
2ccea03a8 drivers/usb/gadget/udc-core.c Felipe Balbi 2011-06-28 1101
/**
792bfcf7a drivers/usb/gadget/udc-core.c Felipe Balbi 2013-02-26 1102
* usb_add_gadget_udc_release - adds a new gadget to the udc class driver list
792bfcf7a drivers/usb/gadget/udc-core.c Felipe Balbi 2013-02-26 1103
* @parent: the parent device to this udc. Usually the controller driver's
792bfcf7a drivers/usb/gadget/udc-core.c Felipe Balbi 2013-02-26 1104
* device.
792bfcf7a drivers/usb/gadget/udc-core.c Felipe Balbi 2013-02-26 1105
* @gadget: the gadget to be added to the list.
792bfcf7a drivers/usb/gadget/udc-core.c Felipe Balbi 2013-02-26 1106
* @release: a gadget release function.
2ccea03a8 drivers/usb/gadget/udc-core.c Felipe Balbi 2011-06-28 1107
*
2ccea03a8 drivers/usb/gadget/udc-core.c Felipe Balbi 2011-06-28 1108
* Returns zero on success, negative errno otherwise.
2ccea03a8 drivers/usb/gadget/udc-core.c Felipe Balbi 2011-06-28 1109
*/
792bfcf7a drivers/usb/gadget/udc-core.c Felipe Balbi 2013-02-26 1110
int usb_add_gadget_udc_release(struct device *parent, struct usb_gadget *gadget,
792bfcf7a drivers/usb/gadget/udc-core.c Felipe Balbi 2013-02-26
void (*release)(struct device *dev))
2ccea03a8 drivers/usb/gadget/udc-core.c Felipe Balbi 2011-06-28 1112
{
2ccea03a8 drivers/usb/gadget/udc-core.c Felipe Balbi 2011-06-28 1113
struct usb_udc *udc;
855ed04a3 drivers/usb/gadget/udc/udc-core.c Ruslan Bilovol2015-11-23 @1114
struct usb_gadget_driver *driver;
2ccea03a8 drivers/usb/gadget/udc-core.c Felipe Balbi 2011-06-28 1115
int ret = -ENOMEM;
2ccea03a8 drivers/usb/gadget/udc-core.c Felipe Balbi 2011-06-28 1116
2ccea03a8 drivers/usb/gadget/udc-core.c Felipe Balbi 2011-06-28 1117
udc = kzalloc(sizeof(*udc), GFP_KERNEL);
2ccea03a8 drivers/usb/gadget/udc-core.c Felipe Balbi 2011-06-28 1118
if (!udc)
2ccea03a8 drivers/usb/gadget/udc-core.c Felipe Balbi 2011-06-28 1119
goto err1;
2ccea03a8 drivers/usb/gadget/udc-core.c Felipe Balbi 2011-06-28 1120
f07bd56bb drivers/usb/gadget/udc-core.c Felipe Balbi 2013-01-24 1121
dev_set_name(&gadget->dev, "gadget");
5702f7537 drivers/usb/gadget/udc-core.c Felipe Balbi 2013-07-17 1122
INIT_WORK(&gadget->work, usb_gadget_state_work);
:: The code at line 1114 was first introduced by commit
:: 855ed04a3758b205e84b269f92d26ab36ed8e2f7 usb: gadget: udc-core:
independent registration of gadgets and gadget drivers
:: TO: Ruslan Bilovol
:: CC: Felipe Balbi
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
Re: usb/gadget: use-after-free in gadgetfs_setup
On Mon, Dec 5, 2016 at 8:31 PM, Alan Stern wrote: > On Mon, 5 Dec 2016, Andrey Konovalov wrote: > >> Hi! >> >> I've got the following error report while running the syzkaller fuzzer. >> >> On commit 3c49de52d5647cda8b42c4255cf8a29d1e22eff5 (Dec 2). >> >> BUG: KASAN: use-after-free in gadgetfs_setup+0x208a/0x20e0 at addr >> 88003dfe5bf2 >> Read of size 2 by task syz-executor0/22994 >> CPU: 3 PID: 22994 Comm: syz-executor0 Not tainted 4.9.0-rc7+ #16 >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 >> 88006df06a18 81f96aba e0528500 11000dbe0cd6 >> ed000dbe0cce 88006df068f0 41b58ab3 8598b4c8 >> 81f96828 11000dbe0ccd 88006df06708 88006df06748 >> Call Trace: >> [ 201.343209] [< inline >] __dump_stack lib/dump_stack.c:15 >> [ 201.343209] [] dump_stack+0x292/0x398 >> lib/dump_stack.c:51 >> [] kasan_object_err+0x1c/0x70 mm/kasan/report.c:159 >> [< inline >] print_address_description mm/kasan/report.c:197 >> [] kasan_report_error+0x1f0/0x4e0 mm/kasan/report.c:286 >> [< inline >] kasan_report mm/kasan/report.c:306 >> [] __asan_report_load_n_noabort+0x3a/0x40 >> mm/kasan/report.c:337 >> [< inline >] config_buf drivers/usb/gadget/legacy/inode.c:1298 >> [] gadgetfs_setup+0x208a/0x20e0 >> drivers/usb/gadget/legacy/inode.c:1368 >> [] dummy_timer+0x11f0/0x36d0 >> drivers/usb/gadget/udc/dummy_hcd.c:1858 >> [] call_timer_fn+0x241/0x800 kernel/time/timer.c:1308 >> [< inline >] expire_timers kernel/time/timer.c:1348 >> [] __run_timers+0xa06/0xec0 kernel/time/timer.c:1641 >> [] run_timer_softirq+0x21/0x80 kernel/time/timer.c:1654 >> [] __do_softirq+0x2fb/0xb63 kernel/softirq.c:284 >> [< inline >] invoke_softirq kernel/softirq.c:364 >> [] irq_exit+0x19e/0x1d0 kernel/softirq.c:405 >> [< inline >] exiting_irq arch/x86/include/asm/apic.h:659 >> [] smp_apic_timer_interrupt+0x7b/0xa0 >> arch/x86/kernel/apic/apic.c:960 >> [] apic_timer_interrupt+0x8c/0xa0 >> arch/x86/entry/entry_64.S:489 >> [ 201.343209] [] ? >> _raw_spin_unlock_irqrestore+0x119/0x1a0 >> [] try_to_wake_up+0x174/0x1160 kernel/sched/core.c:2099 >> [< inline >] wake_up_process kernel/sched/core.c:2165 >> [] wake_up_q+0x8a/0xe0 kernel/sched/core.c:469 >> [] futex_wake+0x5be/0x6c0 kernel/futex.c:1453 >> [] do_futex+0x11bd/0x1f00 kernel/futex.c:3219 >> [< inline >] SYSC_futex kernel/futex.c:3275 >> [] SyS_futex+0x2fc/0x400 kernel/futex.c:3243 >> [] entry_SYSCALL_64_fastpath+0x1f/0xc2 > > Can you test whether the patch below fixes this problem? Hi Alan, Yes, I believe it does. It also seems to fix the warnings in dummy_free_request() I've been getting. Thanks! > > Alan Stern > > > > Index: usb-4.x/drivers/usb/gadget/legacy/inode.c > === > --- usb-4.x.orig/drivers/usb/gadget/legacy/inode.c > +++ usb-4.x/drivers/usb/gadget/legacy/inode.c > @@ -1762,6 +1762,10 @@ dev_config (struct file *fd, const char > } > spin_unlock_irq(&dev->lock); > > + /* Registered but not yet bound to a UDC driver? */ > + if (dev->gadget_registered) > + return -EIO; > + > if (len < (USB_DT_CONFIG_SIZE + USB_DT_DEVICE_SIZE + 4)) > return -EINVAL; > > -- 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] usbip: fix warning in vhci_hcd_probe/lockdep_init_map
On Mon, Dec 5, 2016 at 9:00 PM, Shuah Khan wrote: > Hi Andrey, > > On 12/05/2016 12:56 PM, Shuah Khan wrote: >> vhci_hcd calls sysfs_create_group() with dynamically allocated sysfs >> attributes triggering the lock-class key not persistent warning. Call >> sysfs_attr_init() for dynamically allocated sysfs attributes to fix it. >> >> vhci_hcd vhci_hcd: USB/IP Virtual Host Controller >> vhci_hcd vhci_hcd: new USB bus registered, assigned bus number 2 >> BUG: key 88006a7e8d18 not in .data! >> [ cut here ] >> WARNING: CPU: 0 PID: 1 at kernel/locking/lockdep.c:3131 >> lockdep_init_map+0x60c/0x770 >> DEBUG_LOCKS_WARN_ON(1)[1.567044] Modules linked in: >> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.9.0-rc7+ #58 >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 >> 88006bce6eb8 81f96c8a 0a02 11000d79cd6a >> ed000d79cd62 00046bce6ed8 41b58ab3 8598af40 >> 81f969f8 41b58ab3 0200 >> Call Trace: >> [< inline >] __dump_stack lib/dump_stack.c:15 >> [] dump_stack+0x292/0x398 lib/dump_stack.c:51 >> [] __warn+0x19f/0x1e0 kernel/panic.c:550 >> [] warn_slowpath_fmt+0xc5/0x110 kernel/panic.c:565 >> [] lockdep_init_map+0x60c/0x770 >> kernel/locking/lockdep.c:3131 >> [] __kernfs_create_file+0x114/0x2a0 fs/kernfs/file.c:954 >> [] sysfs_add_file_mode_ns+0x225/0x520 fs/sysfs/file.c:305 >> [< inline >] create_files fs/sysfs/group.c:64 >> [] internal_create_group+0x239/0x8f0 fs/sysfs/group.c:134 >> [] sysfs_create_group+0x1f/0x30 fs/sysfs/group.c:156 >> [] vhci_start+0x5b4/0x7a0 drivers/usb/usbip/vhci_hcd.c:978 >> [] usb_add_hcd+0x8da/0x1c60 drivers/usb/core/hcd.c:2867 >> [] vhci_hcd_probe+0x97/0x130 >> drivers/usb/usbip/vhci_hcd.c:1103 >> --- >> --- >> ---[ end trace c33c7b202cf3aac8 ]--- >> >> Signed-off-by: Shuah Khan >> Reported-by: Andrey Konovalov > > Here is the fix. Fixed the warning I reproduced on my system. > Let me know if it works for you. Hi Shuah, This fixes the warning I've been seeing. Thanks! > > thanks, > -- Shuah > >> --- >> drivers/usb/usbip/vhci_sysfs.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/usb/usbip/vhci_sysfs.c b/drivers/usb/usbip/vhci_sysfs.c >> index c404017..b96e5b1 100644 >> --- a/drivers/usb/usbip/vhci_sysfs.c >> +++ b/drivers/usb/usbip/vhci_sysfs.c >> @@ -361,6 +361,7 @@ static void set_status_attr(int id) >> status->attr.attr.name = status->name; >> status->attr.attr.mode = S_IRUGO; >> status->attr.show = status_show; >> + sysfs_attr_init(&status->attr.attr); >> } >> >> static int init_status_attrs(void) >> > -- 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 v9 1/2] usb: dwc2: assert phy reset when waking up in rk3288 platform
Hello John
I still waiting them be merged, but I still can't find it at next-20161206.
從我的 iPad 傳送
> John Youn 於 2016年10月25日 上午9:30 寫道：
>
>> On 10/23/2016 2:33 AM, ayaka wrote:
>>
>>
>>> On 10/22/2016 03:27 AM, John Youn wrote:
>>>> On 10/20/2016 11:38 AM, Randy Li wrote:
>>>> On the rk3288 USB host-only port (the one that's not the OTG-enabled
>>>> port) the PHY can get into a bad state when a wakeup is asserted (not
>>>> just a wakeup from full system suspend but also a wakeup from
>>>> autosuspend).
>>>>
>>>> We can get the PHY out of its bad state by asserting its "port reset",
>>>> but unfortunately that seems to assert a reset onto the USB bus so it
>>>> could confuse things if we don't actually deenumerate / reenumerate the
>>>> device.
>>>>
>>>> We can also get the PHY out of its bad state by fully resetting it using
>>>> the reset from the CRU (clock reset unit) in chip, which does a more full
>>>> reset. The CRU-based reset appears to actually cause devices on the bus
>>>> to be removed and reinserted, which fixes the problem (albeit in a hacky
>>>> way).
>>>>
>>>> It's unfortunate that we need to do a full re-enumeration of devices at
>>>> wakeup time, but this is better than alternative of letting the bus get
>>>> wedged.
>>>>
>>>> Signed-off-by: Randy Li
>>>> ---
>>>> drivers/usb/dwc2/core.h | 1 +
>>>> drivers/usb/dwc2/core_intr.c | 11 +++
>>>> drivers/usb/dwc2/platform.c | 9 +
>>>> 3 files changed, 21 insertions(+)
>>>>
>>>> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
>>>> index 2a21a04..e91ddbc 100644
>>>> --- a/drivers/usb/dwc2/core.h
>>>> +++ b/drivers/usb/dwc2/core.h
>>>> @@ -859,6 +859,7 @@ struct dwc2_hsotg {
>>>> unsigned int ll_hw_enabled:1;
>>>>
>>>> struct phy *phy;
>>>> +struct work_struct phy_rst_work;
>>>> struct usb_phy *uphy;
>>>> struct dwc2_hsotg_plat *plat;
>>>> struct regulator_bulk_data
>>>> supplies[ARRAY_SIZE(dwc2_hsotg_supply_names)];
>>>> diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c
>>>> index d85c5c9..c3d2168 100644
>>>> --- a/drivers/usb/dwc2/core_intr.c
>>>> +++ b/drivers/usb/dwc2/core_intr.c
>>>> @@ -345,6 +345,7 @@ static void dwc2_handle_session_req_intr(struct
>>>> dwc2_hsotg *hsotg)
>>>> static void dwc2_handle_wakeup_detected_intr(struct dwc2_hsotg *hsotg)
>>>> {
>>>> int ret;
>>>> +struct device_node *np = hsotg->dev->of_node;
>>>>
>>>> /* Clear interrupt */
>>>> dwc2_writel(GINTSTS_WKUPINT, hsotg->regs + GINTSTS);
>>>> @@ -379,6 +380,16 @@ static void dwc2_handle_wakeup_detected_intr(struct
>>>> dwc2_hsotg *hsotg)
>>>> /* Restart the Phy Clock */
>>>> pcgcctl &= ~PCGCTL_STOPPCLK;
>>>> dwc2_writel(pcgcctl, hsotg->regs + PCGCTL);
>>>> +
>>>> +/*
>>>> + * It is a quirk in Rockchip RK3288, causing by
>>>> + * a hardware bug. This will propagate out and
>>>> + * eventually we'll re-enumerate the device.
>>>> + * Not great but the best we can do
>>>> + */
>>>> +if (of_device_is_compatible(np, "rockchip,rk3288-usb"))
>>>> +schedule_work(&hsotg->phy_rst_work);
>>>> +
>>>> mod_timer(&hsotg->wkp_timer,
>>>>jiffies + msecs_to_jiffies(71));
>>>> } else {
>>>> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
>>>> index 8e1728b..65953cf 100644
>>>> --- a/drivers/usb/dwc2/platform.c
>>>> +++ b/drivers/usb/dwc2/platform.c
>>>> @@ -366,6 +366,14 @@ int dwc2_lowlevel_hw_disable(struct dwc2_hsotg *hsotg)
>>>> return ret;
>>>> }
>>>>
>>>> +/* Only used to reset usb phy at interrupter runtime */
>>>> +static void dwc2_reset_phy_work(struct work_struct *data)
>>>> +{
>>>> +struct dwc2_hsotg *hsotg = container_of(data, struct dwc2
Re: [PATCH 1/2] Add DT bindings documentation for Synopsys UDC driver
Hi Rob,
On Tue, Dec 6, 2016 at 4:34 AM, Rob Herring wrote:
> On Wed, Nov 30, 2016 at 11:35:09AM +0530, Raviteja Garimella wrote:
>> This patch adds documentation for Synopsis Designware Cores AHB
>> Subsystem Device Controller (UDC).
>>
>> Signed-off-by: Raviteja Garimella
>> ---
>> .../devicetree/bindings/usb/snps,dw-ahb-udc.txt| 29
>> ++
>> 1 file changed, 29 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/usb/snps,dw-ahb-udc.txt
>>
>> diff --git a/Documentation/devicetree/bindings/usb/snps,dw-ahb-udc.txt
>> b/Documentation/devicetree/bindings/usb/snps,dw-ahb-udc.txt
>> new file mode 100644
>> index 000..64e1fbf
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/usb/snps,dw-ahb-udc.txt
>> @@ -0,0 +1,29 @@
>> +Synopsys USB Device controller.
>> +
>> +The device node is used for Synopsys Designware Cores AHB
>> +Subsystem Device Controller (UDC).
>> +
>> +Required properties:
>> + - compatible: should be "snps,dw-ahbudc"
>
> Needs an SoC specific compatible string.
This will be changed. I am working on using amd5536udc.c driver
which's already in Kernel tree and can be used for this UDC(as per
review comments from Felipe/John).
>
>> + - reg: Offset and length of UDC register set
>> + - interrupts: description of interrupt line
>> + - phys: phandle to phy node.
>> + - phy-names: name of phy node. Must be usb2drd.
>
> A name is pointless when there's only 1 phy. Is this a device or dual
> role device(DRD)?
This is DRD phy that's is connected to a Host Controller and a Device
Controller.
>
>> + - extcon: phandle to the extcon device
>
> I don't think extcon should be required. If this is UDC only, I'm not
> sure why you'd need it.
This Phy will be initialized in Host/Device mode based on the external
connector that's plugged in. That's reason for having extcon node.
>
>> +
>> +Example:
>> +
>> + usbdrd_phy: phy@6501c000 {
>> + #phy-cells = <0>;
>> + compatible = "brcm,ns2-drd-phy";
>> + reg = <0x6600 0x1000>,
>> + }
>> +
>> + udc_dwc: usb@664e {
>> + compatible = "snps,dw-ahb-udc";
>
> Doesn't match above.
This will be changed.
>
>> + reg = <0x664e 0x2000>;
>> + interrupts = ;
>> + phys = <&usbdrd_phy>;
>> + phy-names = "usb2drd";
>> + extcon = <&usbdrd_phy>";
>
> You are already describing the phy connection, you shouldn't need both.
"extcon_get_edev_by_phandle" requires extcon phandle. I see that's the
only way to get the extcon device, since generic Phy device doesn't
have any extcon member.
The current driver will not go as-is after the suggestion I got in UDC
driver review (to use amd5536udc driver). I will work on the changes
and submit the patches once again.
Thanks,
Ravi
>
>> + };
>> --
>> 2.1.0
>>
--
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 RESEND] usb: gadget: udc-core: Rescan pending list on driver unbind
Since:
commit 855ed04a3758 ("usb: gadget: udc-core: independent registration
of gadgets and gadget drivers")
if we load gadget module but there is no free udc available
then it will be stored on a pending gadgets list.
$ modprobe g_zero.ko
$ modprobe g_ether.ko
[] udc-core: couldn't find an available UDC - added [g_ether] to list
of pending drivers
We scan this list each time when new UDC appears in system.
But we can get a free UDC each time after gadget unbind.
This commit add scanning of that list directly after unbinding
gadget from udc.
Thanks to this, when we unload first gadget:
$ rmmod g_zero.ko
gadget which is pending is automatically
attached to that UDC (if name matches).
Signed-off-by: Krzysztof Opasiak
Reviewed-by: Marek Szyprowski
---
Resend this patch as it didn't reach linux-usb mailing list due to problems
with company smtp server.
drivers/usb/gadget/udc/core.c | 44 ++-
1 file changed, 31 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
index 9483489080f6..6c8f6ea8eb96 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -1080,6 +1080,24 @@ static void usb_udc_nop_release(struct device *dev)
dev_vdbg(dev, "%s\n", __func__);
}
+/* should be called with udc_lock held */
+static int check_pending_gadget_drivers(struct usb_udc *udc)
+{
+ struct usb_gadget_driver *driver;
+ int ret = 0;
+
+ list_for_each_entry(driver, &gadget_driver_pending_list, pending)
+ if (!driver->udc_name || strcmp(driver->udc_name,
+ dev_name(&udc->dev)) == 0) {
+ ret = udc_bind_to_driver(udc, driver);
+ if (ret != -EPROBE_DEFER)
+ list_del(&driver->pending);
+ break;
+ }
+
+ return ret;
+}
+
/**
* usb_add_gadget_udc_release - adds a new gadget to the udc class driver list
* @parent: the parent device to this udc. Usually the controller driver's
@@ -1136,17 +1154,9 @@ int usb_add_gadget_udc_release(struct device *parent,
struct usb_gadget *gadget,
udc->vbus = true;
/* pick up one of pending gadget drivers */
- list_for_each_entry(driver, &gadget_driver_pending_list, pending) {
- if (!driver->udc_name || strcmp(driver->udc_name,
- dev_name(&udc->dev)) == 0) {
- ret = udc_bind_to_driver(udc, driver);
- if (ret != -EPROBE_DEFER)
- list_del(&driver->pending);
- if (ret)
- goto err5;
- break;
- }
- }
+ ret = check_pending_gadget_drivers(udc);
+ if (ret)
+ goto err5;
mutex_unlock(&udc_lock);
@@ -1352,14 +1362,22 @@ int usb_gadget_unregister_driver(struct
usb_gadget_driver *driver)
return -EINVAL;
mutex_lock(&udc_lock);
- list_for_each_entry(udc, &udc_list, list)
+ list_for_each_entry(udc, &udc_list, list) {
if (udc->driver == driver) {
usb_gadget_remove_driver(udc);
usb_gadget_set_state(udc->gadget,
- USB_STATE_NOTATTACHED);
+USB_STATE_NOTATTACHED);
+
+ /* Maybe there is someone waiting for this UDC? */
+ check_pending_gadget_drivers(udc);
+ /*
+* For now we ignore bind errors as probably it's
+* not a valid reason to fail other's gadget unbind
+*/
ret = 0;
break;
}
+ }
if (ret) {
list_del(&driver->pending);
--
2.9.3
--
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 v13 07/10] usbip: exporting devices: new application-side daemon
Sorry for spam but previous mail didn't reach linux-usb due to some
problems with my company mail server.
On 12/06/2016 10:48 AM, Krzysztof Opasiak wrote:
> Hi,
>
> On 12/06/2016 09:44 AM, fx IWATA NOBUO wrote:
>> Hello,
>>
>>> sizeof(rhist), sizeof(rserv)
>>
>> I will fix them.
>>
>>> BTW.
>>> The read_record() function is really weird and probably it should be
>>> also refactored. We pass 3 arrays but pass size only for 2 of them and
>>> size of third argument is hard coded inside this function:(
>>
>> Comment "the 3rd part without it being truncated to an acceptable
>> length" at read_record of original code.
>>
>> But I will add busid_len and modify the comment.
>> Also I will check port operation's output will not be changed.
>>
+ if (!ret &&
+ !strncmp(host, rhost, NI_MAXHOST) &&
+ !strncmp(busid, rbusid, SYSFS_BUS_ID_SIZE)) {
+ return vhci_driver->idev + i;
+ }
>>>
>>> Is there any reason why you don't check port here?
>>
>> Remote port number will be different in each connection.
>> This function checks if remote device of remote host is already imported.
>> So it's not checked.
>
> You are absolutely right here. Thank you for your explanation.
>
>>
>> I will refactor read_record() that it can omit if the pointer is NULL.
>>
>>> I also thing that it's better to ensure that strings are \0 terminated
>>> rather than using strncmp all the time.
>>
>> getnameinfo(3), getaddrinfo(3) always set NULL terminator and NI_MAX_
>> includes the NULL.
>>
>> So I will make read_record() always set NULL terminator.
>> Also add comment at read_record().
>
> Please do this in a separate series with fixes and improvements.
>
>>
+ memset(&req, 0, sizeof(req));
+ memset(&reply, 0, sizeof(reply));
>>>
>>> As before. You don't need to memset() reply.
>>
>> I will remove it.
>>
+ if (!error)
+ reply.returncode = 0;
+ else
+ reply.returncode = -1;
>>>
>>> As I wrote in first patch. This may end up in compiler warning
>>
>> The field return code will be removed.
>>
+ rc = usbip_vhci_create_record(host, port, req.udev.busid,
>>> rhport);
>>>
>>> This call should be moved to import_device() as we may endup with
>>> inconsistent_state() if there is a comminication error.
>>
>> Yes!
>> It must be match with the result of vhci_attach_device().
>>
+ usbip_vhci_delete_record(rhport);
>>>
>>> I think that this function should be called from unimport_device()
>>>
>>> Now this function can be ommited if there is some communication error and
>>> we may endup in inconsistent state.
>>
>> I will fix to match the status of vhci_detach_device().
>>
>> I found original usbip_attach() and usbip_detach() have same problem.
>> I will fix them patch 6/10: refactoring of them.
>
> Please do this in a separate series so it may be applied as is without
> discussing device exporting feature.
>
>>
+int usbip_recv_pdu(int connfd, char *host, char *port) {
--snip--
+}
>>> This one is a copy-paste from usbip_recv_pdu() in usbipd_dev.c with
>>> only values changed in switch. How about sharing this code and add
>>> only callback which will be used based on received op_code?
>>
>> usbip_refresh_device_list(), usbip_vhci_refresh_device_list() must be
>> wrapped as same as open, close.
>>
>> Is usbip_meta_refresh_device_list() acceptable?
>
> Sounds good. Let's try.
>
> Best regards,
>
--
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
--
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 v13 05/10] usbip: exporting devices: modifications to daemon
Hi, On 12/06/2016 05:40 AM, fx IWATA NOBUO wrote: > Hello, > >>> usbip_driver_ has widely used as function names and file >>> names for host side abstraction. >>> If they were usbip_host_, I can use usbip_driver_open/close >>> for both host(stub/vudc) and vhci. >> >> usbip_host() is not correct name to use for both stub and vudc as from >> USB point of view stub is on a host but vudc is on a device side > > OK. > >> maybe a kind of usbipd_backed_init() would be more suitable? > > Naming problem again but I recognize usbip_open_driver() is worse than > connect. > I think the word 'backend' has wide meaning and more strict word is > better. > > init driver = &host_driver; NA > ( )driver = &device_driver; NA > open usbip_driver_open usbip_vhci_driver_open > close usbip_driver_close usbip_vhci_driver_close > > Here, I'd like to use word 'driver'. > I searched analogy meta_, super_ in kernel. > > How about usbip_meta_driver_init/open/close()? Sounds good. Let's try. > usbip_update_driver() is t totally unrelated to what this assignment really does. >> So as above. I suggest to call it usbipd_backend() instead of driver. > > Please, let me know good verb representing 'driver = &device_driver'. how about usbip_meta_driver_set(type)? Best regards, -- Krzysztof Opasiak Samsung R&D Institute Poland Samsung Electronics -- 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 v13 07/10] usbip: exporting devices: new application-side daemon
Hello,
> sizeof(rhist), sizeof(rserv)
I will fix them.
> BTW.
> The read_record() function is really weird and probably it should be
> also refactored. We pass 3 arrays but pass size only for 2 of them and
> size of third argument is hard coded inside this function:(
Comment "the 3rd part without it being truncated to an acceptable
length" at read_record of original code.
But I will add busid_len and modify the comment.
Also I will check port operation's output will not be changed.
> > + if (!ret &&
> > + !strncmp(host, rhost, NI_MAXHOST) &&
> > + !strncmp(busid, rbusid, SYSFS_BUS_ID_SIZE)) {
> > + return vhci_driver->idev + i;
> > + }
>
> Is there any reason why you don't check port here?
Remote port number will be different in each connection.
This function checks if remote device of remote host is already imported.
So it's not checked.
I will refactor read_record() that it can omit if the pointer is NULL.
> I also thing that it's better to ensure that strings are \0 terminated
> rather than using strncmp all the time.
getnameinfo(3), getaddrinfo(3) always set NULL terminator and NI_MAX_
includes the NULL.
So I will make read_record() always set NULL terminator.
Also add comment at read_record().
> > + memset(&req, 0, sizeof(req));
> > + memset(&reply, 0, sizeof(reply));
>
> As before. You don't need to memset() reply.
I will remove it.
> > + if (!error)
> > + reply.returncode = 0;
> > + else
> > + reply.returncode = -1;
>
> As I wrote in first patch. This may end up in compiler warning
The field return code will be removed.
> > + rc = usbip_vhci_create_record(host, port, req.udev.busid,
> rhport);
>
> This call should be moved to import_device() as we may endup with
> inconsistent_state() if there is a comminication error.
Yes!
It must be match with the result of vhci_attach_device().
> > + usbip_vhci_delete_record(rhport);
>
> I think that this function should be called from unimport_device()
>
> Now this function can be ommited if there is some communication error and
> we may endup in inconsistent state.
I will fix to match the status of vhci_detach_device().
I found original usbip_attach() and usbip_detach() have same problem.
I will fix them patch 6/10: refactoring of them.
> > +int usbip_recv_pdu(int connfd, char *host, char *port) {
> > --snip--
> > +}
> This one is a copy-paste from usbip_recv_pdu() in usbipd_dev.c with
> only values changed in switch. How about sharing this code and add
> only callback which will be used based on received op_code?
usbip_refresh_device_list(), usbip_vhci_refresh_device_list() must be
wrapped as same as open, close.
Is usbip_meta_refresh_device_list() acceptable?
Thank you for reviewing,
n.iwata
//
> -Original Message-
> From: Krzysztof Opasiak [mailto:k.opas...@samsung.com]
> Sent: Saturday, December 03, 2016 12:46 AM
> To: fx IWATA NOBUO; valentina.mane...@gmail.com; shuah...@samsung.com;
> gre...@linuxfoundation.org; linux-usb@vger.kernel.org
> Cc: fx MICHIMURA TADAO
> Subject: Re: [PATCH v13 07/10] usbip: exporting devices: new
> application-side daemon
>
>
>
> On 11/22/2016 07:48 AM, Nobuo Iwata wrote:
> > This patch is the new application(vhci)-side daemon specific code.
> >
> > The daemons are consisting three files.
> > usbip.c : common code.
> > usbip_dev.c: device(stub)-side specific code.
> > usbip_app.c: application(vhci)-side specific code - this patch.
> >
> > Here, device-side daemon is EXISTING-1 and application-side daemon is
> > NEW-1 in figure below.
> >
> > EXISTING) - invites devices from application(vhci)-side
> > +--+
> +--+
> > device--+ STUB | | application/VHCI |
> > +--+
> +--+
> > 1) usbipd ... start daemon
> > = = =
> > 2) usbip list --local
> > 3) usbip bind
> > <--- list bound devices --- 4) usbip list --remote
> > <--- import a device -- 5) usbip attach
> > = = =
> >X disconnected 6) usbip detach
> > 7) usbip unbind
> >
> > NEW) - dedicates devices from device(stb)-side
> > +--+
> +--+
> > device--+ STUB | | application/VHCI |
> > +--+
> +--+
> > 1) usbipa ... start
> > daemon = = =
> > 2) usbip list --local
> > 3) usbip connect --- export a device -->
> > = = =
> > 4) usbip disconnect --- un-export a device --->
> >
> > Signed-off-by: Nobuo Iwata
> > ---
> > tools/usb/usbip/libsrc/vhci_driver.c | 19 +++
> > tools/usb/usbip/libsrc/vhci_driver.h | 1 +
> > tools/usb/usbip/src/Makefile.am | 7 +-
> > tools/usb/usbip/src/usbipd.c | 12 +-
> > tools/usb/usbip/src/usbipd_app.c | 242
> +++
> > 5 files changed, 279 i
[RFC][PATCH 1/3 v2] usb: dwc2: Add extcon support to dwc2 driver
This patch wires up extcon support to the dwc2 driver
so that devices that use a modern generic phy driver
and don't use the usb-phy infrastructure can still
signal connect/disconnect events.
Cc: Wei Xu
Cc: Guodong Xu
Cc: Amit Pundir
Cc: Rob Herring
Cc: John Youn
Cc: Douglas Anderson
Cc: Chen Yu
Cc: Kishon Vijay Abraham I
Cc: Felipe Balbi
Cc: Greg Kroah-Hartman
Cc: linux-usb@vger.kernel.org
Signed-off-by: John Stultz
---
v2:
* Move extcon logic from generic phy to dwc2 driver, as
suggested by Kishon.
---
arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 11 +
drivers/usb/dwc2/core.h | 16
drivers/usb/dwc2/core_intr.c | 25 +++
drivers/usb/dwc2/hcd.c| 23 +
drivers/usb/dwc2/platform.c | 41 +++
5 files changed, 116 insertions(+)
diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
index 17839db..8a86a11 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
@@ -732,6 +732,16 @@
regulator-always-on;
};
+ usb_vbus: usb-vbus {
+ compatible = "linux,extcon-usb-gpio";
+ id-gpio = <&gpio2 6 1>;
+ };
+
+ usb_id: usb-id {
+ compatible = "linux,extcon-usb-gpio";
+ id-gpio = <&gpio2 5 1>;
+ };
+
usb_phy: usbphy {
compatible = "hisilicon,hi6220-usb-phy";
#phy-cells = <0>;
@@ -745,6 +755,7 @@
phys = <&usb_phy>;
phy-names = "usb2-phy";
clocks = <&sys_ctrl HI6220_USBOTG_HCLK>;
+ extcon = <&usb_vbus>, <&usb_id>;
clock-names = "otg";
dr_mode = "otg";
g-use-dma;
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 2a21a04..4cfce62 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -623,6 +623,13 @@ struct dwc2_hregs_backup {
bool valid;
};
+struct dwc2_extcon {
+ struct notifier_block nb;
+ struct extcon_dev *extcon;
+ int state;
+};
+
+
/*
* Constants related to high speed periodic scheduling
*
@@ -996,6 +1003,10 @@ struct dwc2_hsotg {
u32 g_np_g_tx_fifo_sz;
u32 g_tx_fifo_sz[MAX_EPS_CHANNELS];
#endif /* CONFIG_USB_DWC2_PERIPHERAL || CONFIG_USB_DWC2_DUAL_ROLE */
+
+ struct dwc2_extcon extcon_vbus;
+ struct dwc2_extcon extcon_id;
+ struct delayed_work extcon_work;
};
/* Reasons for halting a host channel */
@@ -1041,6 +1052,11 @@ extern void dwc2_flush_rx_fifo(struct dwc2_hsotg *hsotg);
extern void dwc2_enable_global_interrupts(struct dwc2_hsotg *hcd);
extern void dwc2_disable_global_interrupts(struct dwc2_hsotg *hcd);
+extern int dwc2_extcon_vbus_notifier(struct notifier_block *nb,
+ unsigned long event, void *ptr);
+extern int dwc2_extcon_id_notifier(struct notifier_block *nb,
+ unsigned long event, void *ptr);
+
/* This function should be called on every hardware interrupt. */
extern irqreturn_t dwc2_handle_common_intr(int irq, void *dev);
diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c
index d85c5c9..d4fa938 100644
--- a/drivers/usb/dwc2/core_intr.c
+++ b/drivers/usb/dwc2/core_intr.c
@@ -479,6 +479,31 @@ static void dwc2_handle_usb_suspend_intr(struct dwc2_hsotg
*hsotg)
}
}
+
+
+int dwc2_extcon_vbus_notifier(struct notifier_block *nb,
+ unsigned long event, void *ptr)
+{
+ struct dwc2_extcon *vbus = container_of(nb, struct dwc2_extcon, nb);
+ struct dwc2_hsotg *hsotg = container_of(vbus, struct dwc2_hsotg,
+ extcon_vbus);
+
+ schedule_delayed_work(&hsotg->extcon_work, msecs_to_jiffies(100));
+ return NOTIFY_DONE;
+}
+
+int dwc2_extcon_id_notifier(struct notifier_block *nb,
+ unsigned long event, void *ptr)
+{
+ struct dwc2_extcon *id = container_of(nb, struct dwc2_extcon, nb);
+ struct dwc2_hsotg *hsotg = container_of(id, struct dwc2_hsotg,
+ extcon_id);
+ schedule_delayed_work(&hsotg->extcon_work, msecs_to_jiffies(100));
+
+ return NOTIFY_DONE;
+}
+
+
#define GINTMSK_COMMON (GINTSTS_WKUPINT | GINTSTS_SESSREQINT | \
GINTSTS_CONIDSTSCHNG | GINTSTS_OTGINT |\
GINTSTS_MODEMIS | GINTSTS_DISCONNINT | \
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index df5a065..61eea70 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/u
[RFC][PATCH 3/3 v2] usb: dwc2: Force port resume on switching to device mode
From: Chen Yu
We've seen failures when switching between host and gadget mode,
which was diagnosed as being caused due to the bus being
auto-suspended when we switched.
So this patch forces a port resume when switching to device
mode if the bus is suspended.
Cc: Wei Xu
Cc: Guodong Xu
Cc: Amit Pundir
Cc: Rob Herring
Cc: John Youn
Cc: Douglas Anderson
Cc: Chen Yu
Cc: Felipe Balbi
Cc: Greg Kroah-Hartman
Cc: linux-usb@vger.kernel.org
Signed-off-by: Chen Yu
Signed-off-by: John Stultz
---
drivers/usb/dwc2/hcd.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 75ddfa3..0221534 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -55,6 +55,9 @@
#include "core.h"
#include "hcd.h"
+
+static void dwc2_port_resume(struct dwc2_hsotg *hsotg);
+
/*
* =
* Host Core Layer Functions
@@ -3205,6 +3208,11 @@ static void dwc2_conn_id_status_change(struct
work_struct *work)
if (gotgctl & GOTGCTL_CONID_B) {
/* Wait for switch to device mode */
dev_dbg(hsotg->dev, "connId B\n");
+ if (hsotg->bus_suspended) {
+ dev_info(hsotg->dev,
+ "Do port resume before switching to device
mode\n");
+ dwc2_port_resume(hsotg);
+ }
while (!dwc2_is_device_mode(hsotg)) {
dev_info(hsotg->dev,
"Waiting for Peripheral Mode, Mode=%s\n",
--
2.7.4
--
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
[RFC][PATCH 0/3 v2] Try to connect HiKey's usb extcon to dwc2 driver
After feedback from Kishon and John, I've reworked my efforts to add extcon support to the phy-hi6220-usb driver and instead have added the extcon support to the dwc2 driver directly. This avoids odd interactions trying to wire the generic phy to the otg gadget structure to send proper connect/disconnect notifications to the hcd core. Since this is my first stab at moving it to the dwc2 driver, I suspect there is further improvements possible, so please let me know if there's anything folks would like to see changed. In this series, I also re-added an older patch to force port resuming when transitioning from host -> otg mode, as that was catching me as the bus would suspend if there were no devices plugged into the host ports and then would not work when replugging in the gadget cable. If there is a better way to handle this bus resuming logic in gadget mode, please let me know. Feedback and guidance would be greatly appreciated! thanks -john Cc: Wei Xu Cc: Guodong Xu Cc: Amit Pundir Cc: Rob Herring Cc: John Youn Cc: Douglas Anderson Cc: Chen Yu Cc: Kishon Vijay Abraham I Cc: Felipe Balbi Cc: Greg Kroah-Hartman Cc: linux-usb@vger.kernel.org Chen Yu (1): usb: dwc2: Force port resume on switching to device mode John Stultz (2): usb: dwc2: Add extcon support to dwc2 driver usb: dwc2: Avoid suspending if we're in gadget mode arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 11 + drivers/usb/dwc2/core.h | 16 drivers/usb/dwc2/core_intr.c | 25 +++ drivers/usb/dwc2/hcd.c| 34 + drivers/usb/dwc2/platform.c | 41 +++ 5 files changed, 127 insertions(+) -- 2.7.4 -- 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
[RFC][PATCH 2/3 v2] usb: dwc2: Avoid suspending if we're in gadget mode
I've found when booting HiKey with the usb gadget cable attached if I then try to connect via adb, I get an infinite spew of: dwc2 f72c.usb: dwc2_hsotg_ep_sethalt(ep ffc0790ecb18 ep1out, 0) dwc2 f72c.usb: dwc2_hsotg_ep_sethalt(ep ffc0790eca18 ep1in, 0) It seems that the usb autosuspend is suspending the bus shortly after bootup when the gadget cable is attached. So when adbd then tries to use the device, it doesn't work and it then tries to restart it over and over via the ep_sethalt calls (via FUNCTIONFS_CLEAR_HALT ioctl). Chen Yu suggested this patch to avoid suspending if we're in device mode, and it avoids the problem. This doesn't remove the need for the previous patch, to resume the port when we switch to gadget mode from host mode. But it does seem to resolve the issue. Cc: Wei Xu Cc: Guodong Xu Cc: Amit Pundir Cc: Rob Herring Cc: John Youn Cc: Douglas Anderson Cc: Chen Yu Cc: Felipe Balbi Cc: Greg Kroah-Hartman Cc: linux-usb@vger.kernel.org Suggested-by: Chen Yu Signed-off-by: John Stultz --- drivers/usb/dwc2/hcd.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index 61eea70..75ddfa3 100644 --- a/drivers/usb/dwc2/hcd.c +++ b/drivers/usb/dwc2/hcd.c @@ -4386,6 +4386,9 @@ static int _dwc2_hcd_suspend(struct usb_hcd *hcd) if (!HCD_HW_ACCESSIBLE(hcd)) goto unlock; + if (hsotg->op_state == OTG_STATE_B_PERIPHERAL) + goto unlock; + if (!hsotg->core_params->hibernation) goto skip_power_saving; -- 2.7.4 -- 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
