[PATCH] USB: phy: twl6030: convert platform driver to use dev_groups
Platform drivers now have the option to have the platform core create
and remove any needed sysfs attribute files. So take advantage of that
and do not register "by hand" any sysfs files.
Cc: Felipe Balbi
Signed-off-by: Greg Kroah-Hartman
---
drivers/usb/phy/phy-twl6030-usb.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/phy/phy-twl6030-usb.c
b/drivers/usb/phy/phy-twl6030-usb.c
index dade34d70419..bfebf1f2e991 100644
--- a/drivers/usb/phy/phy-twl6030-usb.c
+++ b/drivers/usb/phy/phy-twl6030-usb.c
@@ -196,6 +196,12 @@ static ssize_t vbus_show(struct device *dev,
}
static DEVICE_ATTR_RO(vbus);
+static struct attribute *twl6030_attrs[] = {
+ &dev_attr_vbus.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(twl6030);
+
static irqreturn_t twl6030_usb_irq(int irq, void *_twl)
{
struct twl6030_usb *twl = _twl;
@@ -361,8 +367,6 @@ static int twl6030_usb_probe(struct platform_device *pdev)
}
platform_set_drvdata(pdev, twl);
- if (device_create_file(&pdev->dev, &dev_attr_vbus))
- dev_warn(&pdev->dev, "could not create sysfs file\n");
INIT_WORK(&twl->set_vbus_work, otg_set_vbus_work);
INIT_DELAYED_WORK(&twl->get_status_work, twl6030_status_work);
@@ -373,7 +377,6 @@ static int twl6030_usb_probe(struct platform_device *pdev)
if (status < 0) {
dev_err(&pdev->dev, "can't get IRQ %d, err %d\n",
twl->irq1, status);
- device_remove_file(twl->dev, &dev_attr_vbus);
return status;
}
@@ -384,7 +387,6 @@ static int twl6030_usb_probe(struct platform_device *pdev)
dev_err(&pdev->dev, "can't get IRQ %d, err %d\n",
twl->irq2, status);
free_irq(twl->irq1, twl);
- device_remove_file(twl->dev, &dev_attr_vbus);
return status;
}
@@ -408,7 +410,6 @@ static int twl6030_usb_remove(struct platform_device *pdev)
free_irq(twl->irq1, twl);
free_irq(twl->irq2, twl);
regulator_put(twl->usb3v3);
- device_remove_file(twl->dev, &dev_attr_vbus);
cancel_work_sync(&twl->set_vbus_work);
return 0;
@@ -426,6 +427,7 @@ static struct platform_driver twl6030_usb_driver = {
.driver = {
.name = "twl6030_usb",
.of_match_table = of_match_ptr(twl6030_usb_id_table),
+ .dev_groups = twl6030_groups,
},
};
--
2.22.0
[PATCH] USB: phy: fsl-usb: convert platform driver to use dev_groups
Platform drivers now have the option to have the platform core create
and remove any needed sysfs attribute files. So take advantage of that
and do not register "by hand" any sysfs files.
Cc: Felipe Balbi
Signed-off-by: Greg Kroah-Hartman
---
drivers/usb/phy/phy-fsl-usb.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c
index f7c96d209eda..dae2f21d3276 100644
--- a/drivers/usb/phy/phy-fsl-usb.c
+++ b/drivers/usb/phy/phy-fsl-usb.c
@@ -1043,6 +1043,11 @@ static ssize_t show_fsl_usb2_otg_state(struct device
*dev,
static DEVICE_ATTR(fsl_usb2_otg_state, S_IRUGO, show_fsl_usb2_otg_state, NULL);
+static struct attribute *fsl_otg_attrs[] = {
+ &dev_attr_fsl_usb2_otg_state.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(fsl_otg_attrs);
/* Char driver interface to control some OTG input */
@@ -1132,10 +1137,6 @@ static int fsl_otg_probe(struct platform_device *pdev)
return ret;
}
- ret = device_create_file(&pdev->dev, &dev_attr_fsl_usb2_otg_state);
- if (ret)
- dev_warn(&pdev->dev, "Can't register sysfs attribute\n");
-
return ret;
}
@@ -1152,8 +1153,6 @@ static int fsl_otg_remove(struct platform_device *pdev)
kfree(fsl_otg_dev->phy.otg);
kfree(fsl_otg_dev);
- device_remove_file(&pdev->dev, &dev_attr_fsl_usb2_otg_state);
-
unregister_chrdev(FSL_OTG_MAJOR, FSL_OTG_NAME);
if (pdata->exit)
@@ -1168,6 +1167,7 @@ struct platform_driver fsl_otg_driver = {
.driver = {
.name = driver_name,
.owner = THIS_MODULE,
+ .dev_groups = fsl_otg_groups,
},
};
--
2.22.0
Re: [PATCH] USB: usbip: convert platform driver to use dev_groups
On Mon, Aug 05, 2019 at 02:22:18PM -0600, shuah wrote:
> On 8/5/19 1:36 PM, Greg Kroah-Hartman wrote:
> > Platform drivers now have the option to have the platform core create
> > and remove any needed sysfs attribute files. So take advantage of that
> > and do not register "by hand" any sysfs files.
> >
> > Cc: Valentina Manea
> > Cc: Shuah Khan
> > Signed-off-by: Greg Kroah-Hartman
> > ---
> > drivers/usb/usbip/vudc.h | 2 +-
> > drivers/usb/usbip/vudc_dev.c | 9 -
> > drivers/usb/usbip/vudc_main.c | 1 +
> > drivers/usb/usbip/vudc_sysfs.c | 7 ++-
> > 4 files changed, 8 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/usb/usbip/vudc.h b/drivers/usb/usbip/vudc.h
> > index cf968192e59f..1bd4bc005829 100644
> > --- a/drivers/usb/usbip/vudc.h
> > +++ b/drivers/usb/usbip/vudc.h
> > @@ -115,7 +115,7 @@ struct vudc_device {
> > struct list_head dev_entry;
> > };
> > -extern const struct attribute_group vudc_attr_group;
> > +extern const struct attribute_group *vudc_groups[];
> > /* visible everywhere */
> > diff --git a/drivers/usb/usbip/vudc_dev.c b/drivers/usb/usbip/vudc_dev.c
> > index a72c17ff1c6a..c8eeabdd9b56 100644
> > --- a/drivers/usb/usbip/vudc_dev.c
> > +++ b/drivers/usb/usbip/vudc_dev.c
> > @@ -616,18 +616,10 @@ int vudc_probe(struct platform_device *pdev)
> > if (ret < 0)
> > goto err_add_udc;
> > - ret = sysfs_create_group(&pdev->dev.kobj, &vudc_attr_group);
> > - if (ret) {
> > - dev_err(&udc->pdev->dev, "create sysfs files\n");
> > - goto err_sysfs;
> > - }
> > -
> > platform_set_drvdata(pdev, udc);
> > return ret;
> > -err_sysfs:
> > - usb_del_gadget_udc(&udc->gadget);
> > err_add_udc:
> > cleanup_vudc_hw(udc);
> > err_init_vudc_hw:
> > @@ -640,7 +632,6 @@ int vudc_remove(struct platform_device *pdev)
> > {
> > struct vudc *udc = platform_get_drvdata(pdev);
> > - sysfs_remove_group(&pdev->dev.kobj, &vudc_attr_group);
> > usb_del_gadget_udc(&udc->gadget);
> > cleanup_vudc_hw(udc);
> > kfree(udc);
> > diff --git a/drivers/usb/usbip/vudc_main.c b/drivers/usb/usbip/vudc_main.c
> > index 390733e6937e..678faa82598c 100644
> > --- a/drivers/usb/usbip/vudc_main.c
> > +++ b/drivers/usb/usbip/vudc_main.c
> > @@ -22,6 +22,7 @@ static struct platform_driver vudc_driver = {
> > .remove = vudc_remove,
> > .driver = {
> > .name = GADGET_NAME,
> > + .dev_groups = vudc_groups,
> > },
> > };
> > diff --git a/drivers/usb/usbip/vudc_sysfs.c b/drivers/usb/usbip/vudc_sysfs.c
> > index 6dcd3ff655c3..100f680c572a 100644
> > --- a/drivers/usb/usbip/vudc_sysfs.c
> > +++ b/drivers/usb/usbip/vudc_sysfs.c
> > @@ -215,7 +215,12 @@ static struct bin_attribute *dev_bin_attrs[] = {
> > NULL,
> > };
> > -const struct attribute_group vudc_attr_group = {
> > +static const struct attribute_group vudc_attr_group = {
> > .attrs = dev_attrs,
> > .bin_attrs = dev_bin_attrs,
> > };
> > +
> > +const struct attribute_group *vudc_groups[] = {
> > + &vudc_attr_group,
> > + NULL,
> > +};
> >
>
> Looks good to me.
>
> Acked-by: Shuah Khan
Thanks for the review!
greg k-h
Re: [PATCH v8 00/11] add USB GPIO based connection detection driver
On Mon, 2019-08-05 at 12:06 +0200, Linus Walleij wrote: > On Wed, Jul 24, 2019 at 10:51 AM Chunfeng Yun > wrote: > > > Because the USB Connector is introduced and the requirement of > > usb-connector.txt binding, the old way using extcon to support > > USB Dual-Role switch is now deprecated, meanwhile there is no > > available common driver when use Type-B connector, typically > > using an input GPIO to detect USB ID pin. > > However while this was going on, > drivers/extcon/extcon-fsa9480.c was merged and that detects > not only GPIO on the USB port but multiplexed usecases such > as UART over the USB micro PHY (and no that UART is not > a USB UART, but an actual RX/TX over D-/D+). > > That driver also measure a whole slew of funny resistance > values on the ID pin, that is how it does its job. I look into the spec of fsa9480, it indeed detect many USB accessories. But this driver is used for simplest cases that only uses micro receptacle with id-pin/vbus-pin > > But for just "hey I'm plugged in" we can surely keep this > ID on GPIO detection in the USB subsystem. Sorry, you mean provide a common API? could you please describe more clearer about your suggestion? Introducing a single driver using usb_role_switch will help to keep transparent for USB controller driver, no matter it uses type-c or micro Thanks a lot > > I just get a bit insecure about how we should ideally > handle these "funny-PHY's". > > Yours, > Linus Walleij
RE: [PATCH 2/5] usb: chipidea: add role switch class support
> >
> > You may use connect bit at portsc since VBUS may not connect to SoC.
>
> By this way, disconnect can be decided but connect is a problem in current
> driver,
> as controller was stopped in low power mode so can't detect connection from
> host,
> unless we also update this behavior too.
>
For connection, if current role is USB_ROLE_NONE, you may start device mode.
Peter
> Li Jun
> >
> > Peter
> >
> > > Li Jun
> > > >
> > > > Peter
> > > >
> > > > > Li Jun
> > > > >
> > > > > >
> > > > > > Peter
> > > > > >
> > > > > > > Signed-off-by: Li Jun
> > > > > > > ---
> > > > > > > drivers/usb/chipidea/ci.h | 2 +
> > > > > > > drivers/usb/chipidea/core.c | 125
> > > > > > > ++--
> > > > > > > drivers/usb/chipidea/otg.c | 13 +
> > > > > > > 3 files changed, 111 insertions(+), 29 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/usb/chipidea/ci.h
> > > > > > > b/drivers/usb/chipidea/ci.h index 82b86cd..5e2f0bc 100644
> > > > > > > --- a/drivers/usb/chipidea/ci.h
> > > > > > > +++ b/drivers/usb/chipidea/ci.h
> > > > > > > @@ -212,6 +212,7 @@ struct ci_hdrc {
> > > > > > > ktime_t
> > > > hr_timeouts[NUM_OTG_FSM_TIMERS];
> > > > > > > unsignedenabled_otg_timer_bits;
> > > > > > > enum otg_fsm_timer next_otg_timer;
> > > > > > > + struct usb_role_switch *role_switch;
> > > > > > > struct work_struct work;
> > > > > > > struct workqueue_struct *wq;
> > > > > > >
> > > > > > > @@ -244,6 +245,7 @@ struct ci_hdrc {
> > > > > > > struct dentry *debugfs;
> > > > > > > boolid_event;
> > > > > > > boolb_sess_valid_event;
> > > > > > > + boolrole_switch_event;
> > > > > > > boolimx28_write_fix;
> > > > > > > boolsupports_runtime_pm;
> > > > > > > boolin_lpm;
> > > > > > > diff --git a/drivers/usb/chipidea/core.c
> > > > > > > b/drivers/usb/chipidea/core.c index bc24c5b..1bcf6f6 100644
> > > > > > > --- a/drivers/usb/chipidea/core.c
> > > > > > > +++ b/drivers/usb/chipidea/core.c
> > > > > > > @@ -587,6 +587,47 @@ static irqreturn_t ci_irq(int irq, void
> > > > > > > *data)
> > > > > > > return ret;
> > > > > > > }
> > > > > > >
> > > > > > > +static int ci_usb_role_switch_set(struct device *dev, enum
> > > > > > > +usb_role
> > > > > > > +role) {
> > > > > > > + struct ci_hdrc *ci = dev_get_drvdata(dev);
> > > > > > > + unsigned long flags;
> > > > > > > +
> > > > > > > + if (ci->role == role || role == USB_ROLE_NONE)
> > > > > > > + return 0;
> > > > > > > +
> > > > > > > + spin_lock_irqsave(&ci->lock, flags);
> > > > > > > + ci->role_switch_event = true;
> > > > > > > + if (ci->role == USB_ROLE_NONE) {
> > > > > > > + if (role == USB_ROLE_DEVICE)
> > > > > > > + ci->role = USB_ROLE_HOST;
> > > > > > > + else
> > > > > > > + ci->role = USB_ROLE_DEVICE;
> > > > > > > + }
> > > > > > > + spin_unlock_irqrestore(&ci->lock, flags);
> > > > > > > +
> > > > > > > + ci_otg_queue_work(ci);
> > > > > > > +
> > > > > > > + return 0;
> > > > > > > +}
> > > > > > > +
> > > > > > > +static enum usb_role ci_usb_role_switch_get(struct device *dev) {
> > > > > > > + struct ci_hdrc *ci = dev_get_drvdata(dev);
> > > > > > > + unsigned long flags;
> > > > > > > + enum usb_role role;
> > > > > > > +
> > > > > > > + spin_lock_irqsave(&ci->lock, flags);
> > > > > > > + role = ci->role;
> > > > > > > + spin_unlock_irqrestore(&ci->lock, flags);
> > > > > > > +
> > > > > > > + return role;
> > > > > > > +}
> > > > > > > +
> > > > > > > +static struct usb_role_switch_desc ci_role_switch = {
> > > > > > > + .set = ci_usb_role_switch_set,
> > > > > > > + .get = ci_usb_role_switch_get, };
> > > > > > > +
> > > > > > > static int ci_cable_notifier(struct notifier_block *nb, unsigned
> > > > > > > long event,
> > > > > > > void *ptr) { @@ -689,6 +730,9
> > > > > > > @@ static int ci_get_platdata(struct device *dev,
> > > > > > > if (of_find_property(dev->of_node,
> > > > > > > "non-zero-ttctrl-ttha", NULL))
> > > > > > > platdata->flags |=
> > > > > > > CI_HDRC_SET_NON_ZERO_TTHA;
> > > > > > >
> > > > > > > + if (device_property_read_bool(dev, "usb-role-switch"))
> > > > > > > + ci_role_switch.fwnode = dev->fwnode;
> > > > > > > +
> > > > > > > ext_id = ERR_PTR(-ENODEV);
> > > > > > > ext_vbus = ERR_PTR(-ENODEV);
> > > > > > > if (of_property_read_bool(dev->of_node, "extcon")) {
> > > > > > > @@
> > > > > > > -908,6
> > > > >
RE: [PATCH 2/5] usb: chipidea: add role switch class support
> -Original Message-
> From: Peter Chen
> Sent: 2019年8月6日 15:52
> To: Jun Li ; Peter Chen
> Cc: Greg Kroah-Hartman ; dl-linux-imx
> ; USB list
> Subject: RE: [PATCH 2/5] usb: chipidea: add role switch class support
>
>
> > >
> > > You may use connect bit at portsc since VBUS may not connect to SoC.
> >
> > By this way, disconnect can be decided but connect is a problem in
> > current driver, as controller was stopped in low power mode so can't
> > detect connection from host, unless we also update this behavior too.
> >
>
> For connection, if current role is USB_ROLE_NONE, you may start device mode.
This is assuming set role only can be called one time between disconnect
and connect to host, this may not always true, but this can be resolved,
I think we need handle the case device mode is started but connection does
not happen, so the gadget need be stopped and enter low power mode again,
if this approach is OK to you, I will go in this direction for my v2.
Li Jun
>
> Peter
>
> > Li Jun
> > >
> > > Peter
> > >
> > > > Li Jun
> > > > >
> > > > > Peter
> > > > >
> > > > > > Li Jun
> > > > > >
> > > > > > >
> > > > > > > Peter
> > > > > > >
> > > > > > > > Signed-off-by: Li Jun
> > > > > > > > ---
> > > > > > > > drivers/usb/chipidea/ci.h | 2 +
> > > > > > > > drivers/usb/chipidea/core.c | 125
> > > > > > > > ++--
> > > > > > > > drivers/usb/chipidea/otg.c | 13 +
> > > > > > > > 3 files changed, 111 insertions(+), 29 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/drivers/usb/chipidea/ci.h
> > > > > > > > b/drivers/usb/chipidea/ci.h index 82b86cd..5e2f0bc 100644
> > > > > > > > --- a/drivers/usb/chipidea/ci.h
> > > > > > > > +++ b/drivers/usb/chipidea/ci.h
> > > > > > > > @@ -212,6 +212,7 @@ struct ci_hdrc {
> > > > > > > > ktime_t
> > > > > hr_timeouts[NUM_OTG_FSM_TIMERS];
> > > > > > > > unsignedenabled_otg_timer_bits;
> > > > > > > > enum otg_fsm_timer next_otg_timer;
> > > > > > > > + struct usb_role_switch *role_switch;
> > > > > > > > struct work_struct work;
> > > > > > > > struct workqueue_struct *wq;
> > > > > > > >
> > > > > > > > @@ -244,6 +245,7 @@ struct ci_hdrc {
> > > > > > > > struct dentry *debugfs;
> > > > > > > > boolid_event;
> > > > > > > > boolb_sess_valid_event;
> > > > > > > > + boolrole_switch_event;
> > > > > > > > boolimx28_write_fix;
> > > > > > > > boolsupports_runtime_pm;
> > > > > > > > boolin_lpm;
> > > > > > > > diff --git a/drivers/usb/chipidea/core.c
> > > > > > > > b/drivers/usb/chipidea/core.c index bc24c5b..1bcf6f6
> > > > > > > > 100644
> > > > > > > > --- a/drivers/usb/chipidea/core.c
> > > > > > > > +++ b/drivers/usb/chipidea/core.c
> > > > > > > > @@ -587,6 +587,47 @@ static irqreturn_t ci_irq(int irq, void
> > > > > > > > *data)
> > > > > > > > return ret;
> > > > > > > > }
> > > > > > > >
> > > > > > > > +static int ci_usb_role_switch_set(struct device *dev,
> > > > > > > > +enum usb_role
> > > > > > > > +role) {
> > > > > > > > + struct ci_hdrc *ci = dev_get_drvdata(dev);
> > > > > > > > + unsigned long flags;
> > > > > > > > +
> > > > > > > > + if (ci->role == role || role == USB_ROLE_NONE)
> > > > > > > > + return 0;
> > > > > > > > +
> > > > > > > > + spin_lock_irqsave(&ci->lock, flags);
> > > > > > > > + ci->role_switch_event = true;
> > > > > > > > + if (ci->role == USB_ROLE_NONE) {
> > > > > > > > + if (role == USB_ROLE_DEVICE)
> > > > > > > > + ci->role = USB_ROLE_HOST;
> > > > > > > > + else
> > > > > > > > + ci->role = USB_ROLE_DEVICE;
> > > > > > > > + }
> > > > > > > > + spin_unlock_irqrestore(&ci->lock, flags);
> > > > > > > > +
> > > > > > > > + ci_otg_queue_work(ci);
> > > > > > > > +
> > > > > > > > + return 0;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static enum usb_role ci_usb_role_switch_get(struct device
> > > > > > > > *dev) {
> > > > > > > > + struct ci_hdrc *ci = dev_get_drvdata(dev);
> > > > > > > > + unsigned long flags;
> > > > > > > > + enum usb_role role;
> > > > > > > > +
> > > > > > > > + spin_lock_irqsave(&ci->lock, flags);
> > > > > > > > + role = ci->role;
> > > > > > > > + spin_unlock_irqrestore(&ci->lock, flags);
> > > > > > > > +
> > > > > > > > + return role;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static struct usb_role_switch_desc ci_role_switch = {
> > > > > > > > + .set = ci_usb_role_switch_set,
> > > > > > > > + .get = ci_usb_role_switch_get, };
> > > > > > > >
[PATCH] USB: Move wusbcore and UWB to staging as it is obsolete
The UWB and wusbcore code is long obsolete, so let us just move the code
out of the real part of the kernel and into the drivers/staging/
location with plans to remove it entirely in a few releases.
Signed-off-by: Greg Kroah-Hartman
---
MAINTAINERS | 15 +++---
drivers/Kconfig | 2 --
drivers/Makefile | 1 -
drivers/staging/Kconfig | 3 ++
drivers/staging/Makefile | 2 ++
drivers/{ => staging}/uwb/Kconfig | 0
drivers/{ => staging}/uwb/Makefile| 0
drivers/staging/uwb/TODO | 8 ++
drivers/{ => staging}/uwb/address.c | 0
drivers/{ => staging}/uwb/allocator.c | 2 +-
drivers/{ => staging}/uwb/beacon.c| 0
drivers/{ => staging}/uwb/driver.c| 0
drivers/{ => staging}/uwb/drp-avail.c | 0
drivers/{ => staging}/uwb/drp-ie.c| 2 +-
drivers/{ => staging}/uwb/drp.c | 0
drivers/{ => staging}/uwb/est.c | 0
drivers/{ => staging}/uwb/hwa-rc.c| 6 ++--
drivers/{ => staging}/uwb/i1480/Makefile | 0
drivers/{ => staging}/uwb/i1480/dfu/Makefile | 0
drivers/{ => staging}/uwb/i1480/dfu/dfu.c | 2 +-
.../{ => staging}/uwb/i1480/dfu/i1480-dfu.h | 2 +-
drivers/{ => staging}/uwb/i1480/dfu/mac.c | 2 +-
drivers/{ => staging}/uwb/i1480/dfu/phy.c | 2 +-
drivers/{ => staging}/uwb/i1480/dfu/usb.c | 6 ++--
drivers/{ => staging}/uwb/i1480/i1480-est.c | 2 +-
drivers/{ => staging}/uwb/ie-rcv.c| 0
drivers/{ => staging}/uwb/ie.c| 0
.../staging/uwb/include}/debug-cmd.h | 0
.../staging/uwb/include}/spec.h | 0
.../uwb => drivers/staging/uwb/include}/umc.h | 0
.../staging/uwb/include}/whci.h | 0
drivers/{ => staging}/uwb/lc-dev.c| 0
drivers/{ => staging}/uwb/lc-rc.c | 0
drivers/{ => staging}/uwb/neh.c | 0
drivers/{ => staging}/uwb/pal.c | 2 +-
drivers/{ => staging}/uwb/radio.c | 2 +-
drivers/{ => staging}/uwb/reset.c | 0
drivers/{ => staging}/uwb/rsv.c | 2 +-
drivers/{ => staging}/uwb/scan.c | 0
drivers/{ => staging}/uwb/umc-bus.c | 2 +-
drivers/{ => staging}/uwb/umc-dev.c | 2 +-
drivers/{ => staging}/uwb/umc-drv.c | 2 +-
drivers/{ => staging}/uwb/uwb-debug.c | 3 +-
drivers/{ => staging}/uwb/uwb-internal.h | 2 +-
{include/linux => drivers/staging/uwb}/uwb.h | 2 +-
drivers/{ => staging}/uwb/uwbd.c | 0
drivers/{ => staging}/uwb/whc-rc.c| 6 ++--
drivers/{ => staging}/uwb/whci.c | 4 +--
.../staging/wusbcore/Documentation}/wusb-cbaf | 0
.../Documentation}/wusb-design-overview.rst | 0
drivers/{usb => staging}/wusbcore/Kconfig | 1 +
drivers/{usb => staging}/wusbcore/Makefile| 2 ++
drivers/staging/wusbcore/TODO | 8 ++
drivers/{usb => staging}/wusbcore/cbaf.c | 6 ++--
drivers/{usb => staging}/wusbcore/crypto.c| 4 +--
drivers/{usb => staging}/wusbcore/dev-sysfs.c | 0
.../{usb => staging}/wusbcore/devconnect.c| 0
drivers/staging/wusbcore/host/Kconfig | 28 +++
drivers/staging/wusbcore/host/Makefile| 3 ++
.../{usb => staging/wusbcore}/host/hwa-hc.c | 4 +--
.../wusbcore}/host/whci/Makefile | 0
.../{usb => staging/wusbcore}/host/whci/asl.c | 4 +--
.../wusbcore}/host/whci/debug.c | 2 +-
.../{usb => staging/wusbcore}/host/whci/hcd.c | 4 +--
.../{usb => staging/wusbcore}/host/whci/hw.c | 4 +--
.../wusbcore}/host/whci/init.c| 4 +--
.../{usb => staging/wusbcore}/host/whci/int.c | 4 +--
.../{usb => staging/wusbcore}/host/whci/pzl.c | 4 +--
.../wusbcore}/host/whci/qset.c| 4 +--
.../wusbcore}/host/whci/whcd.h| 4 +--
.../wusbcore}/host/whci/whci-hc.h | 0
.../wusbcore}/host/whci/wusb.c| 4 +--
.../staging/wusbcore/include}/association.h | 0
.../staging/wusbcore/include}/wusb-wa.h | 0
.../staging/wusbcore/include}/wusb.h | 2 +-
drivers/{usb => staging}/wusbcore/mmc.c | 2 +-
drivers/{usb => staging}/wusbcore/pal.c | 0
.../{usb => staging}/wusbcore/reservation.c | 2 +-
drivers/{usb => staging}/wusbcore/rh.c| 0
drivers/{usb => staging}/wusbcore/security.c | 0
drivers/{usb => staging}/wusbcore/wa-hc.c | 0
drivers/{usb => staging}/wusbcore/wa-hc.h | 6 ++--
drivers/{usb => staging}/wusbcore/wa-nep.c| 0
drivers/{usb => staging}/wusbcore/wa-rpipe.c | 0
drivers/{usb => staging}/wusbcore/wa-xfer.c | 0
drivers/{usb => staging}/wusbcore/wusbhc.c| 0
drivers/{usb => staging}/wusbcore/wusbhc.h| 4 +--
drivers/usb/Kconfig
Re: KASAN: slab-out-of-bounds Write in lg4ff_init
Am Montag, den 05.08.2019, 16:53 +0200 schrieb Andrey Konovalov: > On Mon, Aug 5, 2019 at 4:34 PM Oliver Neukum wrote: > > > > Am Montag, den 05.08.2019, 05:38 -0700 schrieb syzbot: > > > Hello, > > > > > > syzbot found the following crash on: > > > > > > HEAD commit:e96407b4 usb-fuzzer: main usb gadget fuzzer driver > > > git tree: https://github.com/google/kasan.git usb-fuzzer > > > console output: https://syzkaller.appspot.com/x/log.txt?x=144c21dc60 > > > kernel config: https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e > > > dashboard link: > > > https://syzkaller.appspot.com/bug?extid=94e2b9e9c7d1dd332345 > > > compiler: gcc (GCC) 9.0.0 20181231 (experimental) > > > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=169e854260 > > > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=10ec826260 > > > > > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > > > Reported-by: syzbot+94e2b9e9c7d1dd332...@syzkaller.appspotmail.com > > > > > > usb 1-1: config 0 interface 0 altsetting 0 has 1 endpoint descriptor, > > > different from the interface descriptor's value: 9 > > > usb 1-1: New USB device found, idVendor=046d, idProduct=c298, bcdDevice= > > > 0.00 > > > usb 1-1: New USB device strings: Mfr=0, Product=0, SerialNumber=0 > > > usb 1-1: config 0 descriptor?? > > > logitech 0003:046D:C298.0001: unknown main item tag 0x0 > > > logitech 0003:046D:C298.0001: unknown main item tag 0x0 > > > logitech 0003:046D:C298.0001: hidraw0: USB HID v0.00 Device [HID > > > 046d:c298] > > > on usb-dummy_hcd.0-1/input0 > > > BUG: KASAN: slab-out-of-bounds in set_bit > > > include/asm-generic/bitops-instrumented.h:28 [inline] > > > > #syz test: https://github.com/google/kasan.git e96407b4 > > > > From 7e7f8ce9108b69613f8bb4ff2f95c258e22c3228 Mon Sep 17 00:00:00 2001 > > From: Oliver Neukum > > Date: Mon, 5 Aug 2019 16:14:47 +0200 > > Subject: [PATCH] hid-lg4ff: sanity check for offsets of FF effects > > > > Malicious devices could provide huge offsets which would lead > > to setting bits in random kernel memory. Adding a sanity check. > > > > Reported-by: syzbot+94e2b9e9c7d1dd332...@syzkaller.appspotmail.com > > Signed-off-by: Oliver Neukum > > --- > > drivers/hid/hid-lg4ff.c | 6 +- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/hid/hid-lg4ff.c b/drivers/hid/hid-lg4ff.c > > index cefba038520c..f9572750d889 100644 > > --- a/drivers/hid/hid-lg4ff.c > > +++ b/drivers/hid/hid-lg4ff.c > > @@ -1327,8 +1327,12 @@ int lg4ff_init(struct hid_device *hid) > > } > > > > /* Set supported force feedback capabilities */ > > + error = -ENODEV; > > for (j = 0; lg4ff_devices[i].ff_effects[j] >= 0; j++) > > - set_bit(lg4ff_devices[i].ff_effects[j], dev->ffbit); > > + if (lg4ff_devices[i].ff_effects[j] <= 15) > > Can't ff_effects have one of the FF_CONSTANT, FF_PERIODIC, etc. > values? Those are 0x50, 0x51, ... Or maybe I'm just misunderstanding > something. Are those ff_effects provided by the device? You are correct. It is a bitmap. Next patch is in the works. Reagrds Oliver
Re: [PATCH] USB: Move wusbcore and UWB to staging as it is obsolete
On Tue, 2019-08-06 at 12:15 +0200, Greg Kroah-Hartman wrote: > The UWB and wusbcore code is long obsolete, so let us just move the code > out of the real part of the kernel and into the drivers/staging/ > location with plans to remove it entirely in a few releases. [] > MAINTAINERS | 15 +++--- [] > diff --git a/MAINTAINERS b/MAINTAINERS [] > @@ -3800,14 +3800,9 @@ F: scripts/sign-file.c > F: scripts/extract-cert.c > > CERTIFIED WIRELESS USB (WUSB) SUBSYSTEM: > -L: linux-usb@vger.kernel.org > +L: de...@driverdev.osuosl.org > S: Orphan Better to mark this as obsolete so checkpatch emits a message saying "no unnecessary modifications"
Re: KASAN: slab-out-of-bounds Write in lg4ff_init
Am Montag, den 05.08.2019, 05:38 -0700 schrieb syzbot: > Hello, > > syzbot found the following crash on: > > HEAD commit:e96407b4 usb-fuzzer: main usb gadget fuzzer driver > git tree: https://github.com/google/kasan.git usb-fuzzer > console output: https://syzkaller.appspot.com/x/log.txt?x=144c21dc60 > kernel config: https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e > dashboard link: https://syzkaller.appspot.com/bug?extid=94e2b9e9c7d1dd332345 > compiler: gcc (GCC) 9.0.0 20181231 (experimental) > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=169e854260 > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=10ec826260 > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > Reported-by: syzbot+94e2b9e9c7d1dd332...@syzkaller.appspotmail.com > > usb 1-1: config 0 interface 0 altsetting 0 has 1 endpoint descriptor, > different from the interface descriptor's value: 9 > usb 1-1: New USB device found, idVendor=046d, idProduct=c298, bcdDevice= > 0.00 > usb 1-1: New USB device strings: Mfr=0, Product=0, SerialNumber=0 > usb 1-1: config 0 descriptor?? > logitech 0003:046D:C298.0001: unknown main item tag 0x0 > logitech 0003:046D:C298.0001: unknown main item tag 0x0 > logitech 0003:046D:C298.0001: hidraw0: USB HID v0.00 Device [HID 046d:c298] > on usb-dummy_hcd.0-1/input0 > BUG: KASAN: slab-out-of-bounds in set_bit > include/asm-generic/bitops-instrumented.h:28 [inline] #syz test: https://github.com/google/kasan.git e96407b4 >From 90b712f3e9b9a45996eb0dfe5f489a4502c9f843 Mon Sep 17 00:00:00 2001 From: Oliver Neukum Date: Mon, 5 Aug 2019 16:14:47 +0200 Subject: [PATCH] hid-lg4ff: sanity check for offsets of FF effects Malicious devices could provide huge offsets which would lead to setting bits in random kernel memory. Adding a sanity check. Reported-by: syzbot+94e2b9e9c7d1dd332...@syzkaller.appspotmail.com Signed-off-by: Oliver Neukum --- drivers/hid/hid-lg4ff.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/hid/hid-lg4ff.c b/drivers/hid/hid-lg4ff.c index cefba038520c..9e63da793a0d 100644 --- a/drivers/hid/hid-lg4ff.c +++ b/drivers/hid/hid-lg4ff.c @@ -1327,8 +1327,12 @@ int lg4ff_init(struct hid_device *hid) } /* Set supported force feedback capabilities */ + error = -ENODEV; for (j = 0; lg4ff_devices[i].ff_effects[j] >= 0; j++) - set_bit(lg4ff_devices[i].ff_effects[j], dev->ffbit); + if (lg4ff_devices[i].ff_effects[j] < FF_CNT) + set_bit(lg4ff_devices[i].ff_effects[j], dev->ffbit); + else + goto err_init; error = input_ff_create_memless(dev, NULL, lg4ff_play); -- 2.16.4
Re: KASAN: slab-out-of-bounds Write in lg4ff_init
Hello, syzbot has tested the proposed patch but the reproducer still triggered crash: KASAN: slab-out-of-bounds Write in lg4ff_init logitech 0003:046D:C298.0001: unknown main item tag 0x0 logitech 0003:046D:C298.0001: unknown main item tag 0x0 logitech 0003:046D:C298.0001: hidraw0: USB HID v0.00 Device [HID 046d:c298] on usb-dummy_hcd.2-1/input0 == BUG: KASAN: slab-out-of-bounds in set_bit include/asm-generic/bitops-instrumented.h:28 [inline] BUG: KASAN: slab-out-of-bounds in lg4ff_init+0x878/0x1800 drivers/hid/hid-lg4ff.c:1333 Write of size 8 at addr 8881d5d81dc0 by task kworker/0:0/5 CPU: 0 PID: 5 Comm: kworker/0:0 Not tainted 5.3.0-rc2+ #1 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Workqueue: usb_hub_wq hub_event Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0xca/0x13e lib/dump_stack.c:113 print_address_description+0x6a/0x32c mm/kasan/report.c:351 __kasan_report.cold+0x1a/0x33 mm/kasan/report.c:482 kasan_report+0xe/0x12 mm/kasan/common.c:612 check_memory_region_inline mm/kasan/generic.c:185 [inline] check_memory_region+0x128/0x190 mm/kasan/generic.c:192 set_bit include/asm-generic/bitops-instrumented.h:28 [inline] lg4ff_init+0x878/0x1800 drivers/hid/hid-lg4ff.c:1333 lg_probe+0x3b3/0x890 drivers/hid/hid-lg.c:850 hid_device_probe+0x2be/0x3f0 drivers/hid/hid-core.c:2209 really_probe+0x281/0x650 drivers/base/dd.c:548 driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 __device_attach+0x217/0x360 drivers/base/dd.c:882 bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 device_add+0xae6/0x16f0 drivers/base/core.c:2114 hid_add_device+0x33c/0x990 drivers/hid/hid-core.c:2365 usbhid_probe+0xa81/0xfa0 drivers/hid/usbhid/hid-core.c:1386 usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361 really_probe+0x281/0x650 drivers/base/dd.c:548 driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 __device_attach+0x217/0x360 drivers/base/dd.c:882 bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 device_add+0xae6/0x16f0 drivers/base/core.c:2114 usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023 generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210 usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266 really_probe+0x281/0x650 drivers/base/dd.c:548 driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 __device_attach+0x217/0x360 drivers/base/dd.c:882 bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 device_add+0xae6/0x16f0 drivers/base/core.c:2114 usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536 hub_port_connect drivers/usb/core/hub.c:5098 [inline] hub_port_connect_change drivers/usb/core/hub.c:5213 [inline] port_event drivers/usb/core/hub.c:5359 [inline] hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441 process_one_work+0x92b/0x1530 kernel/workqueue.c:2269 worker_thread+0x96/0xe20 kernel/workqueue.c:2415 kthread+0x318/0x420 kernel/kthread.c:255 ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352 Allocated by task 5: save_stack+0x1b/0x80 mm/kasan/common.c:69 set_track mm/kasan/common.c:77 [inline] __kasan_kmalloc mm/kasan/common.c:487 [inline] __kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:460 kmalloc include/linux/slab.h:552 [inline] kzalloc include/linux/slab.h:748 [inline] hidraw_connect+0x4b/0x3e0 drivers/hid/hidraw.c:513 hid_connect+0x5c7/0xbb0 drivers/hid/hid-core.c:1885 hid_hw_start drivers/hid/hid-core.c:1981 [inline] hid_hw_start+0xa2/0x130 drivers/hid/hid-core.c:1972 lg_probe+0x2a4/0x890 drivers/hid/hid-lg.c:806 hid_device_probe+0x2be/0x3f0 drivers/hid/hid-core.c:2209 really_probe+0x281/0x650 drivers/base/dd.c:548 driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 __device_attach+0x217/0x360 drivers/base/dd.c:882 bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 device_add+0xae6/0x16f0 drivers/base/core.c:2114 hid_add_device+0x33c/0x990 drivers/hid/hid-core.c:2365 usbhid_probe+0xa81/0xfa0 drivers/hid/usbhid/hid-core.c:1386 usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361 really_probe+0x281/0x650 drivers/base/dd.c:548 driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 __device_attach+0x217/0x360 drivers/base/dd.c:882 bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 device_add+0xae6/0x16f0 drivers/base/core.c:2114 usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:20
[PATCH] usb: setup authorized_default using usb_bus_notify
Currently, the authorized_default and interface_authorized_default
attributes for HCD are set up after the uevent has been sent to userland.
This creates a race condition where userland may fail to access this
file when processing the event. Move the appending of these attributes
earlier relying on the usb_bus_notify dispatcher.
Signed-off-by: Thiébaud Weksteen
---
drivers/usb/core/hcd.c | 123 ---
drivers/usb/core/sysfs.c | 121 ++
drivers/usb/core/usb.h | 5 ++
3 files changed, 126 insertions(+), 123 deletions(-)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 9320787ac2e6..2ccbc2f83570 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -103,11 +103,6 @@ static DEFINE_SPINLOCK(hcd_urb_unlink_lock);
/* wait queue for synchronous unlinks */
DECLARE_WAIT_QUEUE_HEAD(usb_kill_urb_queue);
-static inline int is_root_hub(struct usb_device *udev)
-{
- return (udev->parent == NULL);
-}
-
/*-*/
/*
@@ -880,101 +875,6 @@ static int usb_rh_urb_dequeue(struct usb_hcd *hcd, struct
urb *urb, int status)
}
-
-/*
- * Show & store the current value of authorized_default
- */
-static ssize_t authorized_default_show(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- struct usb_device *rh_usb_dev = to_usb_device(dev);
- struct usb_bus *usb_bus = rh_usb_dev->bus;
- struct usb_hcd *hcd;
-
- hcd = bus_to_hcd(usb_bus);
- return snprintf(buf, PAGE_SIZE, "%u\n", hcd->dev_policy);
-}
-
-static ssize_t authorized_default_store(struct device *dev,
- struct device_attribute *attr,
- const char *buf, size_t size)
-{
- ssize_t result;
- unsigned val;
- struct usb_device *rh_usb_dev = to_usb_device(dev);
- struct usb_bus *usb_bus = rh_usb_dev->bus;
- struct usb_hcd *hcd;
-
- hcd = bus_to_hcd(usb_bus);
- result = sscanf(buf, "%u\n", &val);
- if (result == 1) {
- hcd->dev_policy = val <= USB_DEVICE_AUTHORIZE_INTERNAL ?
- val : USB_DEVICE_AUTHORIZE_ALL;
- result = size;
- } else {
- result = -EINVAL;
- }
- return result;
-}
-static DEVICE_ATTR_RW(authorized_default);
-
-/*
- * interface_authorized_default_show - show default authorization status
- * for USB interfaces
- *
- * note: interface_authorized_default is the default value
- * for initializing the authorized attribute of interfaces
- */
-static ssize_t interface_authorized_default_show(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- struct usb_device *usb_dev = to_usb_device(dev);
- struct usb_hcd *hcd = bus_to_hcd(usb_dev->bus);
-
- return sprintf(buf, "%u\n", !!HCD_INTF_AUTHORIZED(hcd));
-}
-
-/*
- * interface_authorized_default_store - store default authorization status
- * for USB interfaces
- *
- * note: interface_authorized_default is the default value
- * for initializing the authorized attribute of interfaces
- */
-static ssize_t interface_authorized_default_store(struct device *dev,
- struct device_attribute *attr, const char *buf, size_t count)
-{
- struct usb_device *usb_dev = to_usb_device(dev);
- struct usb_hcd *hcd = bus_to_hcd(usb_dev->bus);
- int rc = count;
- bool val;
-
- if (strtobool(buf, &val) != 0)
- return -EINVAL;
-
- if (val)
- set_bit(HCD_FLAG_INTF_AUTHORIZED, &hcd->flags);
- else
- clear_bit(HCD_FLAG_INTF_AUTHORIZED, &hcd->flags);
-
- return rc;
-}
-static DEVICE_ATTR_RW(interface_authorized_default);
-
-/* Group all the USB bus attributes */
-static struct attribute *usb_bus_attrs[] = {
- &dev_attr_authorized_default.attr,
- &dev_attr_interface_authorized_default.attr,
- NULL,
-};
-
-static const struct attribute_group usb_bus_attr_group = {
- .name = NULL, /* we want them in the same directory */
- .attrs = usb_bus_attrs,
-};
-
-
-
/*-*/
/**
@@ -2894,32 +2794,11 @@ int usb_add_hcd(struct usb_hcd *hcd,
if (retval != 0)
goto err_register_root_hub;
- retval = sysfs_create_group(&rhdev->dev.kobj, &usb_bus_attr_group);
- if (retval < 0) {
- printk(KERN_ERR "Cannot register USB bus sysfs attributes:
%d\n",
- retval);
- goto error_create_attr_group;
- }
if (hcd->uses_new_polling && HCD_POLL_RH(hcd))
usb_hcd_poll_rh_status(hcd);
return retval;
-error_create_attr_group:
- clear_bit(HCD_FLAG_RH_RUNNING, &hcd->flags);
- if (HC_IS_RUNNING(hcd
[PATCH net-next 3/5] r8152: use alloc_pages for rx buffer
Replace kmalloc_node() with alloc_pages() for rx buffer.
Signed-off-by: Hayes Wang
---
drivers/net/usb/r8152.c | 31 +++
1 file changed, 11 insertions(+), 20 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 44d832ceb516..401e56112365 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -698,8 +698,8 @@ struct rx_agg {
struct list_head list, info_list;
struct urb *urb;
struct r8152 *context;
+ struct page *page;
void *buffer;
- void *head;
};
struct tx_agg {
@@ -1476,7 +1476,7 @@ static void free_rx_agg(struct r8152 *tp, struct rx_agg
*agg)
list_del(&agg->info_list);
usb_free_urb(agg->urb);
- kfree(agg->buffer);
+ __free_pages(agg->page, get_order(tp->rx_buf_sz));
kfree(agg);
atomic_dec(&tp->rx_count);
@@ -1487,26 +1487,17 @@ static struct rx_agg *alloc_rx_agg(struct r8152 *tp,
gfp_t mflags)
struct net_device *netdev = tp->netdev;
int node = netdev->dev.parent ? dev_to_node(netdev->dev.parent) : -1;
struct rx_agg *rx_agg;
+ unsigned int order = get_order(tp->rx_buf_sz);
rx_agg = kmalloc_node(sizeof(*rx_agg), mflags, node);
if (rx_agg) {
unsigned long flags;
- u8 *buf;
- buf = kmalloc_node(tp->rx_buf_sz, mflags, node);
- if (!buf)
+ rx_agg->page = alloc_pages(mflags, order);
+ if (!rx_agg->page)
goto free_rx;
- if (buf != rx_agg_align(buf)) {
- kfree(buf);
- buf = kmalloc_node(tp->rx_buf_sz + RX_ALIGN, mflags,
- node);
- if (!buf)
- goto free_rx;
- }
-
- rx_agg->buffer = buf;
- rx_agg->head = rx_agg_align(buf);
+ rx_agg->buffer = page_address(rx_agg->page);
rx_agg->urb = usb_alloc_urb(0, mflags);
if (!rx_agg->urb)
@@ -1526,7 +1517,7 @@ static struct rx_agg *alloc_rx_agg(struct r8152 *tp,
gfp_t mflags)
return rx_agg;
free_buf:
- kfree(rx_agg->buffer);
+ __free_pages(rx_agg->page, order);
free_rx:
kfree(rx_agg);
return NULL;
@@ -2007,8 +1998,8 @@ static int rx_bottom(struct r8152 *tp, int budget)
if (urb->actual_length < ETH_ZLEN)
goto submit;
- rx_desc = agg->head;
- rx_data = agg->head;
+ rx_desc = agg->buffer;
+ rx_data = agg->buffer;
len_used += sizeof(struct rx_desc);
while (urb->actual_length > len_used) {
@@ -2055,7 +2046,7 @@ static int rx_bottom(struct r8152 *tp, int budget)
find_next_rx:
rx_data = rx_agg_align(rx_data + pkt_len + ETH_FCS_LEN);
rx_desc = (struct rx_desc *)rx_data;
- len_used = (int)(rx_data - (u8 *)agg->head);
+ len_used = (int)(rx_data - (u8 *)agg->buffer);
len_used += sizeof(struct rx_desc);
}
@@ -2166,7 +2157,7 @@ int r8152_submit_rx(struct r8152 *tp, struct rx_agg *agg,
gfp_t mem_flags)
return 0;
usb_fill_bulk_urb(agg->urb, tp->udev, usb_rcvbulkpipe(tp->udev, 1),
- agg->head, tp->rx_buf_sz,
+ agg->buffer, tp->rx_buf_sz,
(usb_complete_t)read_bulk_callback, agg);
ret = usb_submit_urb(agg->urb, mem_flags);
--
2.21.0
[PATCH net-next 5/5] r8152: change rx_frag_head_sz and rx_max_agg_num dynamically
Let rx_frag_head_sz and rx_max_agg_num could be modified dynamically
through the sysfs.
Signed-off-by: Hayes Wang
---
drivers/net/usb/r8152.c | 119 ++--
1 file changed, 115 insertions(+), 4 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 1615900c8592..0b4d439f603a 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -26,7 +26,7 @@
#include
/* Information for net-next */
-#define NETNEXT_VERSION"09"
+#define NETNEXT_VERSION"10"
/* Information for net */
#define NET_VERSION"10"
@@ -756,6 +756,9 @@ struct r8152 {
u32 tx_qlen;
u32 coalesce;
u32 rx_buf_sz;
+ u32 rx_frag_head_sz;
+ u32 rx_max_agg_num;
+
u16 ocp_base;
u16 speed;
u8 *intr_buff;
@@ -1992,7 +1995,7 @@ static struct rx_agg *rtl_get_free_rx(struct r8152 *tp,
gfp_t mflags)
spin_unlock_irqrestore(&tp->rx_lock, flags);
- if (!agg_free && atomic_read(&tp->rx_count) < RTL8152_MAX_RX_AGG)
+ if (!agg_free && atomic_read(&tp->rx_count) < tp->rx_max_agg_num)
agg_free = alloc_rx_agg(tp, mflags);
return agg_free;
@@ -2072,10 +2075,10 @@ static int rx_bottom(struct r8152 *tp, int budget)
pkt_len -= ETH_FCS_LEN;
rx_data += sizeof(struct rx_desc);
- if (!agg_next || RTL8152_RXFG_HEADSZ > pkt_len)
+ if (!agg_next || tp->rx_frag_head_sz > pkt_len)
rx_frag_head_sz = pkt_len;
else
- rx_frag_head_sz = RTL8152_RXFG_HEADSZ;
+ rx_frag_head_sz = tp->rx_frag_head_sz;
skb = napi_alloc_skb(napi, rx_frag_head_sz);
if (!skb) {
@@ -5383,6 +5386,101 @@ static u8 rtl_get_version(struct usb_interface *intf)
return version;
}
+static ssize_t rx_frag_head_sz_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct usb_interface *intf = to_usb_interface(dev);
+ struct r8152 *tp = usb_get_intfdata(intf);
+
+ sprintf(buf, "%u\n", tp->rx_frag_head_sz);
+
+ return strlen(buf);
+}
+
+static ssize_t rx_frag_head_sz_store(struct device *dev,
+struct device_attribute *attr,
+const char *buf, size_t count)
+{
+ struct usb_interface *intf;
+ u32 rx_frag_head_sz;
+ struct r8152 *tp;
+
+ intf = to_usb_interface(dev);
+ tp = usb_get_intfdata(intf);
+
+ if (sscanf(buf, "%u\n", &rx_frag_head_sz) != 1)
+ return -EINVAL;
+
+ if (rx_frag_head_sz < ETH_ZLEN)
+ return -EINVAL;
+
+ if (test_bit(RTL8152_UNPLUG, &tp->flags))
+ return -ENODEV;
+
+ if (tp->rx_frag_head_sz != rx_frag_head_sz) {
+ napi_disable(&tp->napi);
+ tp->rx_frag_head_sz = rx_frag_head_sz;
+ napi_enable(&tp->napi);
+ }
+
+ return count;
+}
+
+static DEVICE_ATTR_RW(rx_frag_head_sz);
+
+static ssize_t rx_max_agg_num_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct usb_interface *intf = to_usb_interface(dev);
+ struct r8152 *tp = usb_get_intfdata(intf);
+
+ sprintf(buf, "%u\n", tp->rx_max_agg_num);
+
+ return strlen(buf);
+}
+
+static ssize_t rx_max_agg_num_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct usb_interface *intf;
+ u32 rx_max_agg_num;
+ struct r8152 *tp;
+
+ intf = to_usb_interface(dev);
+ tp = usb_get_intfdata(intf);
+
+ if (sscanf(buf, "%u\n", &rx_max_agg_num) != 1)
+ return -EINVAL;
+
+ if (rx_max_agg_num < (RTL8152_MAX_RX * 2))
+ return -EINVAL;
+
+ if (test_bit(RTL8152_UNPLUG, &tp->flags))
+ return -ENODEV;
+
+ if (tp->rx_max_agg_num != rx_max_agg_num) {
+ napi_disable(&tp->napi);
+ tp->rx_max_agg_num = rx_max_agg_num;
+ napi_enable(&tp->napi);
+ }
+
+ return count;
+}
+
+static DEVICE_ATTR_RW(rx_max_agg_num);
+
+static struct attribute *rtk_adv_attrs[] = {
+ &dev_attr_rx_frag_head_sz.attr,
+ &dev_attr_rx_max_agg_num.attr,
+ NULL
+};
+
+static struct attribute_group rtk_adv_grp = {
+ .name = "rtl_adv",
+ .attrs = rtk_adv_attrs,
+};
+
static int rtl8152_probe(struct usb_interface *intf,
const struct usb_device_id *id)
{
@@ -5487,6 +5585,9 @@ static int rtl8152_probe(struct usb_interface *intf,
tp->speed = tp->mii.supports_gmii ? SPEED_1000 : SPEED_100;
tp->duplex = DUPLEX_FULL;
+
[PATCH net-next 2/5] r8152: replace array with linking list for rx information
The original method uses an array to store the rx information. The
new one uses a list to link each rx structure. Then, it is possible
to increase/decrease the number of rx structure dynamically.
Signed-off-by: Hayes Wang
---
drivers/net/usb/r8152.c | 187
1 file changed, 132 insertions(+), 55 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 0f07ed05ab34..44d832ceb516 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
#include
/* Information for net-next */
@@ -694,7 +695,7 @@ struct tx_desc {
struct r8152;
struct rx_agg {
- struct list_head list;
+ struct list_head list, info_list;
struct urb *urb;
struct r8152 *context;
void *buffer;
@@ -719,7 +720,7 @@ struct r8152 {
struct net_device *netdev;
struct urb *intr_urb;
struct tx_agg tx_info[RTL8152_MAX_TX];
- struct rx_agg rx_info[RTL8152_MAX_RX];
+ struct list_head rx_info;
struct list_head rx_done, tx_free;
struct sk_buff_head tx_queue, rx_queue;
spinlock_t rx_lock, tx_lock;
@@ -744,6 +745,8 @@ struct r8152 {
void (*autosuspend_en)(struct r8152 *tp, bool enable);
} rtl_ops;
+ atomic_t rx_count;
+
int intr_interval;
u32 saved_wolopts;
u32 msg_enable;
@@ -1468,19 +1471,86 @@ static inline void *tx_agg_align(void *data)
return (void *)ALIGN((uintptr_t)data, TX_ALIGN);
}
+static void free_rx_agg(struct r8152 *tp, struct rx_agg *agg)
+{
+ list_del(&agg->info_list);
+
+ usb_free_urb(agg->urb);
+ kfree(agg->buffer);
+ kfree(agg);
+
+ atomic_dec(&tp->rx_count);
+}
+
+static struct rx_agg *alloc_rx_agg(struct r8152 *tp, gfp_t mflags)
+{
+ struct net_device *netdev = tp->netdev;
+ int node = netdev->dev.parent ? dev_to_node(netdev->dev.parent) : -1;
+ struct rx_agg *rx_agg;
+
+ rx_agg = kmalloc_node(sizeof(*rx_agg), mflags, node);
+ if (rx_agg) {
+ unsigned long flags;
+ u8 *buf;
+
+ buf = kmalloc_node(tp->rx_buf_sz, mflags, node);
+ if (!buf)
+ goto free_rx;
+
+ if (buf != rx_agg_align(buf)) {
+ kfree(buf);
+ buf = kmalloc_node(tp->rx_buf_sz + RX_ALIGN, mflags,
+ node);
+ if (!buf)
+ goto free_rx;
+ }
+
+ rx_agg->buffer = buf;
+ rx_agg->head = rx_agg_align(buf);
+
+ rx_agg->urb = usb_alloc_urb(0, mflags);
+ if (!rx_agg->urb)
+ goto free_buf;
+
+ rx_agg->context = tp;
+
+ INIT_LIST_HEAD(&rx_agg->list);
+ INIT_LIST_HEAD(&rx_agg->info_list);
+ spin_lock_irqsave(&tp->rx_lock, flags);
+ list_add_tail(&rx_agg->info_list, &tp->rx_info);
+ spin_unlock_irqrestore(&tp->rx_lock, flags);
+
+ atomic_inc(&tp->rx_count);
+ }
+
+ return rx_agg;
+
+free_buf:
+ kfree(rx_agg->buffer);
+free_rx:
+ kfree(rx_agg);
+ return NULL;
+}
+
static void free_all_mem(struct r8152 *tp)
{
+ struct list_head *cursor, *next;
+ unsigned long flags;
int i;
- for (i = 0; i < RTL8152_MAX_RX; i++) {
- usb_free_urb(tp->rx_info[i].urb);
- tp->rx_info[i].urb = NULL;
+ spin_lock_irqsave(&tp->rx_lock, flags);
- kfree(tp->rx_info[i].buffer);
- tp->rx_info[i].buffer = NULL;
- tp->rx_info[i].head = NULL;
+ list_for_each_safe(cursor, next, &tp->rx_info) {
+ struct rx_agg *agg;
+
+ agg = list_entry(cursor, struct rx_agg, info_list);
+ free_rx_agg(tp, agg);
}
+ spin_unlock_irqrestore(&tp->rx_lock, flags);
+
+ WARN_ON(unlikely(atomic_read(&tp->rx_count)));
+
for (i = 0; i < RTL8152_MAX_TX; i++) {
usb_free_urb(tp->tx_info[i].urb);
tp->tx_info[i].urb = NULL;
@@ -1503,46 +1573,28 @@ static int alloc_all_mem(struct r8152 *tp)
struct usb_interface *intf = tp->intf;
struct usb_host_interface *alt = intf->cur_altsetting;
struct usb_host_endpoint *ep_intr = alt->endpoint + 2;
- struct urb *urb;
int node, i;
- u8 *buf;
node = netdev->dev.parent ? dev_to_node(netdev->dev.parent) : -1;
spin_lock_init(&tp->rx_lock);
spin_lock_init(&tp->tx_lock);
+ INIT_LIST_HEAD(&tp->rx_info);
INIT_LIST_HEAD(&tp->tx_free);
INIT_LIST_HEAD(&tp->rx_done);
skb_queue_head_init(&tp->tx_queue);
skb_queue_head_init(&tp->rx_queue);
+ atomic_set(&tp->rx_count, 0);
for (i = 0; i < RTL8152_MAX_RX;
[PATCH net-next 4/5] r8152: support skb_add_rx_frag
Use skb_add_rx_frag() to reduce the memory copy for rx data.
Use a new list of rx_used to store the rx buffer which couldn't be
reused yet.
Besides, the total number of rx buffer may be increased or decreased
dynamically. And it is limited by RTL8152_MAX_RX_AGG.
Signed-off-by: Hayes Wang
---
drivers/net/usb/r8152.c | 122 +++-
1 file changed, 108 insertions(+), 14 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 401e56112365..1615900c8592 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -584,6 +584,9 @@ enum rtl_register_content {
#define TX_ALIGN 4
#define RX_ALIGN 8
+#define RTL8152_MAX_RX_AGG (10 * RTL8152_MAX_RX)
+#define RTL8152_RXFG_HEADSZ256
+
#define INTR_LINK 0x0004
#define RTL8152_REQT_READ 0xc0
@@ -720,7 +723,7 @@ struct r8152 {
struct net_device *netdev;
struct urb *intr_urb;
struct tx_agg tx_info[RTL8152_MAX_TX];
- struct list_head rx_info;
+ struct list_head rx_info, rx_used;
struct list_head rx_done, tx_free;
struct sk_buff_head tx_queue, rx_queue;
spinlock_t rx_lock, tx_lock;
@@ -1476,7 +1479,7 @@ static void free_rx_agg(struct r8152 *tp, struct rx_agg
*agg)
list_del(&agg->info_list);
usb_free_urb(agg->urb);
- __free_pages(agg->page, get_order(tp->rx_buf_sz));
+ put_page(agg->page);
kfree(agg);
atomic_dec(&tp->rx_count);
@@ -1493,7 +1496,7 @@ static struct rx_agg *alloc_rx_agg(struct r8152 *tp,
gfp_t mflags)
if (rx_agg) {
unsigned long flags;
- rx_agg->page = alloc_pages(mflags, order);
+ rx_agg->page = alloc_pages(mflags | __GFP_COMP, order);
if (!rx_agg->page)
goto free_rx;
@@ -1951,6 +1954,50 @@ static u8 r8152_rx_csum(struct r8152 *tp, struct rx_desc
*rx_desc)
return checksum;
}
+static inline bool rx_count_exceed(struct r8152 *tp)
+{
+ return atomic_read(&tp->rx_count) > RTL8152_MAX_RX;
+}
+
+static inline int agg_offset(struct rx_agg *agg, void *addr)
+{
+ return (int)(addr - agg->buffer);
+}
+
+static struct rx_agg *rtl_get_free_rx(struct r8152 *tp, gfp_t mflags)
+{
+ struct list_head *cursor, *next;
+ struct rx_agg *agg_free = NULL;
+ unsigned long flags;
+
+ spin_lock_irqsave(&tp->rx_lock, flags);
+
+ list_for_each_safe(cursor, next, &tp->rx_used) {
+ struct rx_agg *agg;
+
+ agg = list_entry(cursor, struct rx_agg, list);
+
+ if (page_count(agg->page) == 1) {
+ if (!agg_free) {
+ list_del_init(cursor);
+ agg_free = agg;
+ continue;
+ } else if (rx_count_exceed(tp)) {
+ list_del_init(cursor);
+ free_rx_agg(tp, agg);
+ }
+ break;
+ }
+ }
+
+ spin_unlock_irqrestore(&tp->rx_lock, flags);
+
+ if (!agg_free && atomic_read(&tp->rx_count) < RTL8152_MAX_RX_AGG)
+ agg_free = alloc_rx_agg(tp, mflags);
+
+ return agg_free;
+}
+
static int rx_bottom(struct r8152 *tp, int budget)
{
unsigned long flags;
@@ -1986,7 +2033,7 @@ static int rx_bottom(struct r8152 *tp, int budget)
list_for_each_safe(cursor, next, &rx_queue) {
struct rx_desc *rx_desc;
- struct rx_agg *agg;
+ struct rx_agg *agg, *agg_next;
int len_used = 0;
struct urb *urb;
u8 *rx_data;
@@ -1998,6 +2045,8 @@ static int rx_bottom(struct r8152 *tp, int budget)
if (urb->actual_length < ETH_ZLEN)
goto submit;
+ agg_next = rtl_get_free_rx(tp, GFP_ATOMIC);
+
rx_desc = agg->buffer;
rx_data = agg->buffer;
len_used += sizeof(struct rx_desc);
@@ -2005,7 +2054,7 @@ static int rx_bottom(struct r8152 *tp, int budget)
while (urb->actual_length > len_used) {
struct net_device *netdev = tp->netdev;
struct net_device_stats *stats = &netdev->stats;
- unsigned int pkt_len;
+ unsigned int pkt_len, rx_frag_head_sz;
struct sk_buff *skb;
/* limite the skb numbers for rx_queue */
@@ -2023,22 +2072,37 @@ static int rx_bottom(struct r8152 *tp, int budget)
pkt_len -= ETH_FCS_LEN;
rx_data += sizeof(struct rx_desc);
- skb = napi_alloc_skb(napi, pkt_len);
+ if (!agg_next || RTL8152_RXFG_HEADSZ > pkt_len)
+ rx_frag_head_sz = pkt_len;
+
[PATCH net-next 1/5] r8152: separate the rx buffer size
The different chips may accept different rx buffer sizes. The RTL8152
supports 16K bytes, and RTL8153 support 32K bytes.
Signed-off-by: Hayes Wang
---
drivers/net/usb/r8152.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 39e0768d734d..0f07ed05ab34 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -749,6 +749,7 @@ struct r8152 {
u32 msg_enable;
u32 tx_qlen;
u32 coalesce;
+ u32 rx_buf_sz;
u16 ocp_base;
u16 speed;
u8 *intr_buff;
@@ -1516,13 +1517,13 @@ static int alloc_all_mem(struct r8152 *tp)
skb_queue_head_init(&tp->rx_queue);
for (i = 0; i < RTL8152_MAX_RX; i++) {
- buf = kmalloc_node(agg_buf_sz, GFP_KERNEL, node);
+ buf = kmalloc_node(tp->rx_buf_sz, GFP_KERNEL, node);
if (!buf)
goto err1;
if (buf != rx_agg_align(buf)) {
kfree(buf);
- buf = kmalloc_node(agg_buf_sz + RX_ALIGN, GFP_KERNEL,
+ buf = kmalloc_node(tp->rx_buf_sz + RX_ALIGN, GFP_KERNEL,
node);
if (!buf)
goto err1;
@@ -2113,7 +2114,7 @@ int r8152_submit_rx(struct r8152 *tp, struct rx_agg *agg,
gfp_t mem_flags)
return 0;
usb_fill_bulk_urb(agg->urb, tp->udev, usb_rcvbulkpipe(tp->udev, 1),
- agg->head, agg_buf_sz,
+ agg->head, tp->rx_buf_sz,
(usb_complete_t)read_bulk_callback, agg);
ret = usb_submit_urb(agg->urb, mem_flags);
@@ -2447,7 +2448,7 @@ static void r8153_set_rx_early_timeout(struct r8152 *tp)
static void r8153_set_rx_early_size(struct r8152 *tp)
{
- u32 ocp_data = agg_buf_sz - rx_reserved_size(tp->netdev->mtu);
+ u32 ocp_data = tp->rx_buf_sz - rx_reserved_size(tp->netdev->mtu);
switch (tp->version) {
case RTL_VER_03:
@@ -5115,6 +5116,7 @@ static int rtl_ops_init(struct r8152 *tp)
ops->in_nway= rtl8152_in_nway;
ops->hw_phy_cfg = r8152b_hw_phy_cfg;
ops->autosuspend_en = rtl_runtime_suspend_enable;
+ tp->rx_buf_sz = 16 * 1024;
break;
case RTL_VER_03:
@@ -5132,6 +5134,7 @@ static int rtl_ops_init(struct r8152 *tp)
ops->in_nway= rtl8153_in_nway;
ops->hw_phy_cfg = r8153_hw_phy_cfg;
ops->autosuspend_en = rtl8153_runtime_enable;
+ tp->rx_buf_sz = 32 * 1024;
break;
case RTL_VER_08:
@@ -5147,6 +5150,7 @@ static int rtl_ops_init(struct r8152 *tp)
ops->in_nway= rtl8153_in_nway;
ops->hw_phy_cfg = r8153b_hw_phy_cfg;
ops->autosuspend_en = rtl8153b_runtime_enable;
+ tp->rx_buf_sz = 32 * 1024;
break;
default:
--
2.21.0
[PATCH net-next 0/5] RX improve
The different chips use different rx buffer size. Use skb_add_rx_frag() to reduce memory copy for RX. Hayes Wang (5): r8152: separate the rx buffer size r8152: replace array with linking list for rx information r8152: use alloc_pages for rx buffer r8152: support skb_add_rx_frag r8152: change rx_frag_head_sz and rx_max_agg_num dynamically drivers/net/usb/r8152.c | 415 +--- 1 file changed, 346 insertions(+), 69 deletions(-) -- 2.21.0
Re: [PATCH] USB: Move wusbcore and UWB to staging as it is obsolete
On Tue, Aug 06, 2019 at 03:29:40AM -0700, Joe Perches wrote: > On Tue, 2019-08-06 at 12:15 +0200, Greg Kroah-Hartman wrote: > > The UWB and wusbcore code is long obsolete, so let us just move the code > > out of the real part of the kernel and into the drivers/staging/ > > location with plans to remove it entirely in a few releases. > [] > > MAINTAINERS | 15 +++--- > [] > > diff --git a/MAINTAINERS b/MAINTAINERS > [] > > @@ -3800,14 +3800,9 @@ F: scripts/sign-file.c > > F: scripts/extract-cert.c > > > > CERTIFIED WIRELESS USB (WUSB) SUBSYSTEM: > > -L: linux-usb@vger.kernel.org > > +L: de...@driverdev.osuosl.org > > S: Orphan > > Better to mark this as obsolete so checkpatch emits > a message saying "no unnecessary modifications" > > Ah, good point, will do that as an add-on patch after this. thanks, greg k-h
[RESEND][PATCH] usb: musb: sunxi: propagate devicetree node to glue pdev
In order for devicetree nodes to be correctly associated with attached devices, the controller node needs to be propagated to the glue device. Signed-off-by: Mans Rullgard --- drivers/usb/musb/sunxi.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c index 832a41f9ee7d..a72665fbf111 100644 --- a/drivers/usb/musb/sunxi.c +++ b/drivers/usb/musb/sunxi.c @@ -781,6 +781,8 @@ static int sunxi_musb_probe(struct platform_device *pdev) pinfo.name = "musb-hdrc"; pinfo.id= PLATFORM_DEVID_AUTO; pinfo.parent= &pdev->dev; + pinfo.fwnode= of_fwnode_handle(pdev->dev.of_node); + pinfo.of_node_reused = true; pinfo.res = pdev->resource; pinfo.num_res = pdev->num_resources; pinfo.data = &pdata; -- 2.22.0
Re: [PATCH] usb: setup authorized_default using usb_bus_notify
On Tue, Aug 06, 2019 at 01:00:50PM +0200, Thiébaud Weksteen wrote: > Currently, the authorized_default and interface_authorized_default > attributes for HCD are set up after the uevent has been sent to userland. > This creates a race condition where userland may fail to access this > file when processing the event. Move the appending of these attributes > earlier relying on the usb_bus_notify dispatcher. > > Signed-off-by: Thiébaud Weksteen > --- > drivers/usb/core/hcd.c | 123 --- > drivers/usb/core/sysfs.c | 121 ++ > drivers/usb/core/usb.h | 5 ++ > 3 files changed, 126 insertions(+), 123 deletions(-) And does this solve the issue you reported yesterday? If so, I'll be glad to backport this to the older stable kernels as well. thanks, greg k-h
[PATCH v4 0/2] usbip: Implement SG support
There are bugs on vhci with usb 3.0 storage device. In USB, each SG
list entry buffer should be divisible by the bulk max packet size.
But with native SG support, this problem doesn't matter because the
SG buffer is treated as contiguous buffer. But without native SG
support, USB storage driver breaks SG list into several URBs and the
error occurs because of a buffer size of URB that cannot be divided
by the bulk max packet size. The error situation is as follows.
When USB Storage driver requests 31.5 KB data and has SG list which
has 3584 bytes buffer followed by 7 4096 bytes buffer for some
reason. USB Storage driver splits this SG list into several URBs
because VHCI doesn't support SG and sends them separately. So the
first URB buffer size is 3584 bytes. When receiving data from device,
USB 3.0 device sends data packet of 1024 bytes size because the max
packet size of BULK pipe is 1024 bytes. So device sends 4096 bytes.
But the first URB buffer has only 3584 bytes buffer size. So host
controller terminates the transfer even though there is more data to
receive. So, vhci needs to support SG transfer to prevent this error.
In this patch, vhci supports SG regardless of whether the server's
host controller supports SG or not, because stub driver splits SG
list into several URBs if the server's host controller doesn't
support SG.
To support SG, vhci_map_urb_for_dma() sets URB_DMA_MAP_SG flag in
urb->transfer_flags if URB has SG list and this flag will tell stub
driver to use SG list.
vhci sends each SG list entry to stub driver. Then, stub driver sees
the total length of the buffer and allocates SG table and pages
according to the total buffer length calling sgl_alloc(). After stub
driver receives completed URB, it again sends each SG list entry to
vhci.
If the server's host controller doesn't support SG, stub driver
breaks a single SG request into several URBs and submits them to
the server's host controller. When all the split URBs are completed,
stub driver reassembles the URBs into a single return command and
sends it to vhci.
Alan fixed vhci bug with the USB 3.0 storage device by modifying
USB storage driver.
("usb-storage: Set virt_boundary_mask to avoid SG overflows")
But the fundamental solution of it is to add SG support to vhci.
This patch works well with the USB 3.0 storage devices without Alan's
patch, and we can revert Alan's patch if it causes some troubles.
Suwan Kim (2):
usbip: Skip DMA mapping and unmapping for urb at vhci
usbip: Implement SG support to vhci-hcd and stub driver
drivers/usb/usbip/stub.h | 7 +-
drivers/usb/usbip/stub_main.c| 55 ++---
drivers/usb/usbip/stub_rx.c | 204 ++-
drivers/usb/usbip/stub_tx.c | 99 +++
drivers/usb/usbip/usbip_common.c | 59 ++---
drivers/usb/usbip/vhci_hcd.c | 34 +-
drivers/usb/usbip/vhci_tx.c | 63 --
7 files changed, 395 insertions(+), 126 deletions(-)
--
2.20.1
[PATCH v4 1/2] usbip: Skip DMA mapping and unmapping for urb at vhci
vhci doesn’t do DMA for remote device. Actually, the real DMA
operation is done by network card driver. vhci just passes virtual
address of the buffer to the network stack, so vhci doesn’t use and
need dma address of the buffer of the URB.
But HCD provides DMA mapping and unmapping function by default.
Moreover, it causes unnecessary DMA mapping and unmapping which
will be done again at the NIC driver and it wastes CPU cycles.
So, implement map_urb_for_dma and unmap_urb_for_dma function for
vhci in order to skip the DMA mapping and unmapping procedure.
When it comes to supporting SG for vhci, it is useful to use native
SG list (urb->num_sgs) instead of mapped SG list because DMA mapping
fnuction can adjust the number of SG list (urb->num_mapped_sgs).
And vhci_map_urb_for_dma() prevents isoc pipe from using SG as
hcd_map_urb_for_dma() does.
Signed-off-by: Suwan Kim
---
v3 - v4:
- Replace WARN_ON() with pr_err() in the error path.
v2 - v3
- Move setting URB_DMA_MAP_SG flag to the patch 2.
- Prevent isoc pipe from using SG buffer.
v1 - v2
- Add setting URB_DMA_MAP_SG flag in urb->transfer_flags to tell
stub driver to use SG buffer.
---
drivers/usb/usbip/vhci_hcd.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
index 000ab7225717..429e4e989f38 100644
--- a/drivers/usb/usbip/vhci_hcd.c
+++ b/drivers/usb/usbip/vhci_hcd.c
@@ -1288,6 +1288,22 @@ static int vhci_free_streams(struct usb_hcd *hcd, struct
usb_device *udev,
return 0;
}
+static int vhci_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb,
+ gfp_t mem_flags)
+{
+ if (usb_endpoint_xfer_isoc(&urb->ep->desc) && urb->num_sgs) {
+ pr_err("SG is not supported for isochronous transfer\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static void vhci_unmap_urb_for_dma(struct usb_hcd *hcd, struct urb *urb)
+{
+ dev_dbg(hcd->self.controller, "vhci does not unmap urb for dma\n");
+}
+
static const struct hc_driver vhci_hc_driver = {
.description= driver_name,
.product_desc = driver_desc,
@@ -1304,6 +1320,9 @@ static const struct hc_driver vhci_hc_driver = {
.get_frame_number = vhci_get_frame_number,
+ .map_urb_for_dma = vhci_map_urb_for_dma,
+ .unmap_urb_for_dma = vhci_unmap_urb_for_dma,
+
.hub_status_data = vhci_hub_status,
.hub_control= vhci_hub_control,
.bus_suspend= vhci_bus_suspend,
--
2.20.1
[PATCH v4 2/2] usbip: Implement SG support to vhci-hcd and stub driver
There are bugs on vhci with usb 3.0 storage device. In USB, each SG list entry buffer should be divisible by the bulk max packet size. But with native SG support, this problem doesn't matter because the SG buffer is treated as contiguous buffer. But without native SG support, USB storage driver breaks SG list into several URBs and the error occurs because of a buffer size of URB that cannot be divided by the bulk max packet size. The error situation is as follows. When USB Storage driver requests 31.5 KB data and has SG list which has 3584 bytes buffer followed by 7 4096 bytes buffer for some reason. USB Storage driver splits this SG list into several URBs because VHCI doesn't support SG and sends them separately. So the first URB buffer size is 3584 bytes. When receiving data from device, USB 3.0 device sends data packet of 1024 bytes size because the max packet size of BULK pipe is 1024 bytes. So device sends 4096 bytes. But the first URB buffer has only 3584 bytes buffer size. So host controller terminates the transfer even though there is more data to receive. So, vhci needs to support SG transfer to prevent this error. In this patch, vhci supports SG regardless of whether the server's host controller supports SG or not, because stub driver splits SG list into several URBs if the server's host controller doesn't support SG. To support SG, vhci_map_urb_for_dma() sets URB_DMA_MAP_SG flag in urb->transfer_flags if URB has SG list and this flag will tell stub driver to use SG list. vhci sends each SG list entry to stub driver. Then, stub driver sees the total length of the buffer and allocates SG table and pages according to the total buffer length calling sgl_alloc(). After stub driver receives completed URB, it again sends each SG list entry to vhci. If the server's host controller doesn't support SG, stub driver breaks a single SG request into several URBs and submits them to the server's host controller. When all the split URBs are completed, stub driver reassembles the URBs into a single return command and sends it to vhci. Moreover, in the situation where vhci supports SG, but stub driver does not, or vice versa, usbip works normally. Because there is no protocol modification, there is no problem in communication between server and client even if the one has a kernel without SG support. In the case of vhci supports SG and stub driver doesn't, because vhci sends only the total length of the buffer to stub driver as it did before the patch applied, stub driver only needs to allocate the required length of buffers regardless of whether vhci supports SG or not. If stub driver needs to send data buffer to vhci because of IN pipe, stub driver also sends only total length of buffer as metadata and then sends real data as vhci does. Then vhci receive data from stub driver and store it to the corresponding buffer of SG list entry. In the case of stub driver that supports SG, buffer is allocated by sgl_alloc(). However, stub driver that does not support SG allocates buffer using only kmalloc(). Therefore, if vhci supports SG and stub driver doesn't, stub driver has to allocate buffer with kmalloc() as much as the total length of SG buffer which is quite huge when vhci sends SG request, so it has big overhead in buffer allocation. And for the case of stub driver supports SG and vhci doesn't, since the USB storage driver checks that vhci doesn't support SG and sends the request to stub driver by splitting the SG list into multiple URBs, stub driver allocates a buffer with kmalloc() as it did before this patch. VUDC also works well with this patch. Tests are done with two USB gadget created by CONFIGFS USB gadget. Both use the BULK pipe. 1. Serial gadget 2. Mass storage gadget * Serial gadget test Serial gadget on the host sends and receives data using cat command on the /dev/ttyGS. The client uses minicom to communicate with the serial gadget. * Mass storage gadget test After connecting the gadget with vhci, use "dd" to test read and write operation on the client side. Read - dd if=/dev/sd iflag=direct of=/dev/null bs=1G count=1 Write - dd if= iflag=direct of=/dev/sd bs=1G count=1 Signed-off-by: Suwan Kim --- v3 - v4 - Rewrite the description about the vhci bug with USB 3.0 storage device. - Add the description about the test with VUDC. - Fix the error message in stub_recv_cmd_unlink(). v2 - v3 - Rewrite the commit log to make it more clear. - Add the description about the mismatch situation in commit log. - Run chechpatch.pl and fix errors with coding style and typos. - Fix the error path of usbip_recv_xbuff() to stop receiving data after TCP error occurs. - Consolidate the duplicated error path in usbip_recv_xbuff() and vhci_send_cmd_submit(). - Undo the unnecessary changes * Undo the deletion of empty line in stub_send_ret_submit() * Move memset() lines in stub_send_ret_submit() to the original position. v1 - v2 - Add the logic that splits single SG req
Re: KASAN: use-after-free Read in device_release_driver_internal
Am Donnerstag, den 01.08.2019, 14:47 -0400 schrieb Alan Stern:
>
> I think this must be caused by an unbalanced refcount. That is,
> something must drop one more reference to the device than it takes.
> That would explain why the invalid access occurs inside a single
> bus_remove_device() call, between the klist_del() and
> device_release_driver().
>
> The kernel log indicates that the device was probed by rndis_wlan,
> rndis_host, and cdc_acm, all of which got errors because of the
> device's bogus descriptors. Probably one of them is messing up the
> refcount.
Hi,
you made me look at cdc-acm. I suspect
cae2bc768d176bfbdad7035bbcc3cdc973eb7984 ("usb: cdc-acm: Decrement tty port's
refcount if probe() fail")
is buggy decrementing the refcount on the interface in destroy()
even before the refcount is increased.
Unfortunately I cannot tell from the bug report how many and which
interfaces the emulated test device has. Hence it is unclear to me,
when exactly probe() would fail cdc-acm.
If you agree. I am attaching a putative fix.
Regards
Oliver
From 6b31904e6cf75f89441e308b9e428a1de7728fd8 Mon Sep 17 00:00:00 2001
From: Oliver Neukum
Date: Tue, 6 Aug 2019 14:34:27 +0200
Subject: [PATCH] usb: cdc-acm: make sure a refcount is taken early enough
destroy() will decrement the refcount on the interface, so that
it needs to be taken so early that it never undercounts.
Signed-off-by: Oliver Neukum
---
drivers/usb/class/cdc-acm.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 183b41753c98..28e3de775ada 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -1301,10 +1301,6 @@ static int acm_probe(struct usb_interface *intf,
tty_port_init(&acm->port);
acm->port.ops = &acm_port_ops;
- minor = acm_alloc_minor(acm);
- if (minor < 0)
- goto alloc_fail1;
-
ctrlsize = usb_endpoint_maxp(epctrl);
readsize = usb_endpoint_maxp(epread) *
(quirks == SINGLE_RX_URB ? 1 : 2);
@@ -1312,6 +1308,13 @@ static int acm_probe(struct usb_interface *intf,
acm->writesize = usb_endpoint_maxp(epwrite) * 20;
acm->control = control_interface;
acm->data = data_interface;
+
+ usb_get_intf(acm->control); /* undone in destroy() */
+
+ minor = acm_alloc_minor(acm);
+ if (minor < 0)
+ goto alloc_fail1;
+
acm->minor = minor;
acm->dev = usb_dev;
if (h.usb_cdc_acm_descriptor)
@@ -1458,7 +1461,6 @@ static int acm_probe(struct usb_interface *intf,
usb_driver_claim_interface(&acm_driver, data_interface, acm);
usb_set_intfdata(data_interface, acm);
- usb_get_intf(control_interface);
tty_dev = tty_port_register_device(&acm->port, acm_tty_driver, minor,
&control_interface->dev);
if (IS_ERR(tty_dev)) {
--
2.16.4
Re: KMSAN: kernel-usb-infoleak in pcan_usb_pro_send_req
Am Dienstag, den 30.07.2019, 02:38 -0700 schrieb syzbot: > Hello, > > syzbot found the following crash on: > > HEAD commit:41550654 [UPSTREAM] KVM: x86: degrade WARN to pr_warn_rate.. > git tree: kmsan > console output: https://syzkaller.appspot.com/x/log.txt?x=13e95183a0 > kernel config: https://syzkaller.appspot.com/x/.config?x=40511ad0c5945201 > dashboard link: https://syzkaller.appspot.com/bug?extid=513e4d0985298538bf9b > compiler: clang version 9.0.0 (/home/glider/llvm/clang > 80fee25776c2fb61e74c1ecb1a523375c2500b69) > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=17eafa1ba0 > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=17b87983a0 > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > Reported-by: syzbot+513e4d0985298538b...@syzkaller.appspotmail.com #syz test: https://github.com/google/kasan.git 41550654 >From 6de76fa3df8aedc7a76dc0ecdea8308e38d4dccc Mon Sep 17 00:00:00 2001 From: Oliver Neukum Date: Tue, 6 Aug 2019 14:41:52 +0200 Subject: [PATCH] pcan_usb_fd: zero out the common command buffer Lest we leak kernel memory to a device we better zero out buffers. Signed-off-by: Oliver Neukum --- drivers/net/can/usb/peak_usb/pcan_usb_fd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c index 34761c3a6286..47cc1ff5b88e 100644 --- a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c +++ b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c @@ -841,7 +841,7 @@ static int pcan_usb_fd_init(struct peak_usb_device *dev) goto err_out; /* allocate command buffer once for all for the interface */ - pdev->cmd_buffer_addr = kmalloc(PCAN_UFD_CMD_BUFFER_SIZE, + pdev->cmd_buffer_addr = kzalloc(PCAN_UFD_CMD_BUFFER_SIZE, GFP_KERNEL); if (!pdev->cmd_buffer_addr) goto err_out_1; -- 2.16.4
Re: Re: KMSAN: kernel-usb-infoleak in pcan_usb_pro_send_req
Am Dienstag, den 30.07.2019, 02:38 -0700 schrieb syzbot: Hello, syzbot found the following crash on: HEAD commit:41550654 [UPSTREAM] KVM: x86: degrade WARN to pr_warn_rate.. git tree: kmsan console output: https://syzkaller.appspot.com/x/log.txt?x=13e95183a0 kernel config: https://syzkaller.appspot.com/x/.config?x=40511ad0c5945201 dashboard link: https://syzkaller.appspot.com/bug?extid=513e4d0985298538bf9b compiler: clang version 9.0.0 (/home/glider/llvm/clang 80fee25776c2fb61e74c1ecb1a523375c2500b69) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=17eafa1ba0 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=17b87983a0 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+513e4d0985298538b...@syzkaller.appspotmail.com #syz test: https://github.com/google/kasan.git 41550654 KMSAN bugs can only be tested on https://github.com/google/kmsan.git tree because KMSAN tool is not upstreamed yet. See https://goo.gl/tpsmEJ#kmsan-bugs for details. From 6de76fa3df8aedc7a76dc0ecdea8308e38d4dccc Mon Sep 17 00:00:00 2001 From: Oliver Neukum Date: Tue, 6 Aug 2019 14:41:52 +0200 Subject: [PATCH] pcan_usb_fd: zero out the common command buffer Lest we leak kernel memory to a device we better zero out buffers. Signed-off-by: Oliver Neukum --- drivers/net/can/usb/peak_usb/pcan_usb_fd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c index 34761c3a6286..47cc1ff5b88e 100644 --- a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c +++ b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c @@ -841,7 +841,7 @@ static int pcan_usb_fd_init(struct peak_usb_device *dev) goto err_out; /* allocate command buffer once for all for the interface */ - pdev->cmd_buffer_addr = kmalloc(PCAN_UFD_CMD_BUFFER_SIZE, + pdev->cmd_buffer_addr = kzalloc(PCAN_UFD_CMD_BUFFER_SIZE, GFP_KERNEL); if (!pdev->cmd_buffer_addr) goto err_out_1; -- 2.16.4
Re: KMSAN: kernel-usb-infoleak in pcan_usb_pro_send_req
On Tue, Aug 6, 2019 at 2:45 PM Oliver Neukum wrote: > > Am Dienstag, den 30.07.2019, 02:38 -0700 schrieb syzbot: > > Hello, > > > > syzbot found the following crash on: > > > > HEAD commit:41550654 [UPSTREAM] KVM: x86: degrade WARN to pr_warn_rate.. > > git tree: kmsan > > console output: https://syzkaller.appspot.com/x/log.txt?x=13e95183a0 > > kernel config: https://syzkaller.appspot.com/x/.config?x=40511ad0c5945201 > > dashboard link: https://syzkaller.appspot.com/bug?extid=513e4d0985298538bf9b > > compiler: clang version 9.0.0 (/home/glider/llvm/clang > > 80fee25776c2fb61e74c1ecb1a523375c2500b69) > > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=17eafa1ba0 > > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=17b87983a0 > > > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > > Reported-by: syzbot+513e4d0985298538b...@syzkaller.appspotmail.com > > #syz test: https://github.com/google/kasan.git 41550654 Hi Oliver, For KMSAN bugs you'll need to use the kmsan tree (syz test: https://github.com/google/kmsan.git COMMIT_ID). I've updated the testing instructions some time ago to reflect this. Sorry for the complexity, this is caused by USB fuzzing and KMSAN not being upstream yet. Thanks! > > From 6de76fa3df8aedc7a76dc0ecdea8308e38d4dccc Mon Sep 17 00:00:00 2001 > From: Oliver Neukum > Date: Tue, 6 Aug 2019 14:41:52 +0200 > Subject: [PATCH] pcan_usb_fd: zero out the common command buffer > > Lest we leak kernel memory to a device we better zero out buffers. > > Signed-off-by: Oliver Neukum > --- > drivers/net/can/usb/peak_usb/pcan_usb_fd.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c > b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c > index 34761c3a6286..47cc1ff5b88e 100644 > --- a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c > +++ b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c > @@ -841,7 +841,7 @@ static int pcan_usb_fd_init(struct peak_usb_device *dev) > goto err_out; > > /* allocate command buffer once for all for the interface */ > - pdev->cmd_buffer_addr = kmalloc(PCAN_UFD_CMD_BUFFER_SIZE, > + pdev->cmd_buffer_addr = kzalloc(PCAN_UFD_CMD_BUFFER_SIZE, > GFP_KERNEL); > if (!pdev->cmd_buffer_addr) > goto err_out_1; > -- > 2.16.4 > > > > -- > You received this message because you are subscribed to the Google Groups > "syzkaller-bugs" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to syzkaller-bugs+unsubscr...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/syzkaller-bugs/1565095525.8136.22.camel%40suse.com.
Re: [PATCH] USB: typec: ucsi_ccg: convert i2c driver to use dev_groups
On Mon, Aug 05, 2019 at 09:36:36PM +0200, Greg Kroah-Hartman wrote:
> The driver core now supports the option to automatically create and
> remove any needed sysfs attribute files for a driver when the device is
> bound/removed from it. Convert the uscsi_ccg code to use that instead
> of trying to create sysfs files "by hand".
>
> Cc: Heikki Krogerus
> Cc: Ajay Gupta
> Cc: Wolfram Sang
> Cc: Wei Yongjun
> Signed-off-by: Greg Kroah-Hartman
Reviewed-by: Heikki Krogerus
> ---
> drivers/usb/typec/ucsi/ucsi_ccg.c | 13 +++--
> 1 file changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c
> b/drivers/usb/typec/ucsi/ucsi_ccg.c
> index f7a79a23ebed..e283a42e4f06 100644
> --- a/drivers/usb/typec/ucsi/ucsi_ccg.c
> +++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
> @@ -1104,14 +1104,11 @@ static ssize_t do_flash_store(struct device *dev,
>
> static DEVICE_ATTR_WO(do_flash);
>
> -static struct attribute *ucsi_ccg_sysfs_attrs[] = {
> +static struct attribute *ucsi_ccg_attrs[] = {
> &dev_attr_do_flash.attr,
> NULL,
> };
> -
> -static struct attribute_group ucsi_ccg_attr_group = {
> - .attrs = ucsi_ccg_sysfs_attrs,
> -};
> +ATTRIBUTE_GROUPS(ucsi_ccg);
>
> static int ucsi_ccg_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> @@ -1189,10 +1186,6 @@ static int ucsi_ccg_probe(struct i2c_client *client,
>
> i2c_set_clientdata(client, uc);
>
> - status = sysfs_create_group(&uc->dev->kobj, &ucsi_ccg_attr_group);
> - if (status)
> - dev_err(uc->dev, "cannot create sysfs group: %d\n", status);
> -
> pm_runtime_set_active(uc->dev);
> pm_runtime_enable(uc->dev);
> pm_runtime_idle(uc->dev);
> @@ -1209,7 +1202,6 @@ static int ucsi_ccg_remove(struct i2c_client *client)
> ucsi_unregister_ppm(uc->ucsi);
> pm_runtime_disable(uc->dev);
> free_irq(uc->irq, uc);
> - sysfs_remove_group(&uc->dev->kobj, &ucsi_ccg_attr_group);
>
> return 0;
> }
> @@ -1270,6 +1262,7 @@ static struct i2c_driver ucsi_ccg_driver = {
> .driver = {
> .name = "ucsi_ccg",
> .pm = &ucsi_ccg_pm,
> + .dev_groups = ucsi_ccg_groups,
> },
> .probe = ucsi_ccg_probe,
> .remove = ucsi_ccg_remove,
> --
> 2.22.0
thanks,
--
heikki
Re: KMSAN: kernel-usb-infoleak in pcan_usb_pro_send_req
Am Dienstag, den 30.07.2019, 02:38 -0700 schrieb syzbot: > Hello, > > syzbot found the following crash on: > > HEAD commit:41550654 [UPSTREAM] KVM: x86: degrade WARN to pr_warn_rate.. > git tree: kmsan > console output: https://syzkaller.appspot.com/x/log.txt?x=13e95183a0 > kernel config: https://syzkaller.appspot.com/x/.config?x=40511ad0c5945201 > dashboard link: https://syzkaller.appspot.com/bug?extid=513e4d0985298538bf9b > compiler: clang version 9.0.0 (/home/glider/llvm/clang > 80fee25776c2fb61e74c1ecb1a523375c2500b69) > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=17eafa1ba0 > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=17b87983a0 > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > Reported-by: syzbot+513e4d0985298538b...@syzkaller.appspotmail.com #syz test: https://github.com/google/kmsan.git 41550654 >From 6de76fa3df8aedc7a76dc0ecdea8308e38d4dccc Mon Sep 17 00:00:00 2001 From: Oliver Neukum Date: Tue, 6 Aug 2019 14:41:52 +0200 Subject: [PATCH] pcan_usb_fd: zero out the common command buffer Lest we leak kernel memory to a device we better zero out buffers. Signed-off-by: Oliver Neukum --- drivers/net/can/usb/peak_usb/pcan_usb_fd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c index 34761c3a6286..47cc1ff5b88e 100644 --- a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c +++ b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c @@ -841,7 +841,7 @@ static int pcan_usb_fd_init(struct peak_usb_device *dev) goto err_out; /* allocate command buffer once for all for the interface */ - pdev->cmd_buffer_addr = kmalloc(PCAN_UFD_CMD_BUFFER_SIZE, + pdev->cmd_buffer_addr = kzalloc(PCAN_UFD_CMD_BUFFER_SIZE, GFP_KERNEL); if (!pdev->cmd_buffer_addr) goto err_out_1;
KASAN: use-after-free Read in dvb_usb_device_exit (2)
Hello, syzbot found the following crash on: HEAD commit:e96407b4 usb-fuzzer: main usb gadget fuzzer driver git tree: https://github.com/google/kasan.git usb-fuzzer console output: https://syzkaller.appspot.com/x/log.txt?x=114fd9aa60 kernel config: https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e dashboard link: https://syzkaller.appspot.com/bug?extid=c58e976e022432ee60b4 compiler: gcc (GCC) 9.0.0 20181231 (experimental) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=173ee42c60 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=16d9442c60 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+c58e976e022432ee6...@syzkaller.appspotmail.com input: TeVii S421 PCI as /devices/platform/dummy_hcd.0/usb1/1-1/rc/rc0/input5 dvb-usb: schedule remote query interval to 150 msecs. dw2102: su3000_power_ctrl: 0, initialized 1 dvb-usb: TeVii S421 PCI successfully initialized and connected. usb 1-1: USB disconnect, device number 2 == BUG: KASAN: use-after-free in dvb_usb_device_exit+0x19a/0x1a0 drivers/media/usb/dvb-usb/dvb-usb-init.c:305 Read of size 8 at addr 8881d50468e8 by task kworker/1:1/22 CPU: 1 PID: 22 Comm: kworker/1:1 Not tainted 5.3.0-rc2+ #25 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Workqueue: usb_hub_wq hub_event Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0xca/0x13e lib/dump_stack.c:113 print_address_description+0x6a/0x32c mm/kasan/report.c:351 __kasan_report.cold+0x1a/0x33 mm/kasan/report.c:482 kasan_report+0xe/0x12 mm/kasan/common.c:612 dvb_usb_device_exit+0x19a/0x1a0 drivers/media/usb/dvb-usb/dvb-usb-init.c:305 usb_unbind_interface+0x1bd/0x8a0 drivers/usb/core/driver.c:423 __device_release_driver drivers/base/dd.c:1120 [inline] device_release_driver_internal+0x404/0x4c0 drivers/base/dd.c:1151 bus_remove_device+0x2dc/0x4a0 drivers/base/bus.c:556 device_del+0x420/0xb10 drivers/base/core.c:2288 usb_disable_device+0x211/0x690 drivers/usb/core/message.c:1237 usb_disconnect+0x284/0x8d0 drivers/usb/core/hub.c:2199 hub_port_connect drivers/usb/core/hub.c:4949 [inline] hub_port_connect_change drivers/usb/core/hub.c:5213 [inline] port_event drivers/usb/core/hub.c:5359 [inline] hub_event+0x1454/0x3640 drivers/usb/core/hub.c:5441 process_one_work+0x92b/0x1530 kernel/workqueue.c:2269 process_scheduled_works kernel/workqueue.c:2331 [inline] worker_thread+0x7ab/0xe20 kernel/workqueue.c:2417 kthread+0x318/0x420 kernel/kthread.c:255 ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352 Allocated by task 22: save_stack+0x1b/0x80 mm/kasan/common.c:69 set_track mm/kasan/common.c:77 [inline] __kasan_kmalloc mm/kasan/common.c:487 [inline] __kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:460 slab_post_alloc_hook mm/slab.h:520 [inline] slab_alloc_node mm/slub.c:2766 [inline] slab_alloc mm/slub.c:2774 [inline] __kmalloc_track_caller+0xc8/0x2a0 mm/slub.c:4331 kmemdup+0x23/0x50 mm/util.c:120 kmemdup include/linux/string.h:432 [inline] dw2102_probe+0x627/0xc40 drivers/media/usb/dvb-usb/dw2102.c:2372 usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361 really_probe+0x281/0x650 drivers/base/dd.c:548 driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 __device_attach+0x217/0x360 drivers/base/dd.c:882 bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 device_add+0xae6/0x16f0 drivers/base/core.c:2114 usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023 generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210 usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266 really_probe+0x281/0x650 drivers/base/dd.c:548 driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 __device_attach+0x217/0x360 drivers/base/dd.c:882 bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 device_add+0xae6/0x16f0 drivers/base/core.c:2114 usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536 hub_port_connect drivers/usb/core/hub.c:5098 [inline] hub_port_connect_change drivers/usb/core/hub.c:5213 [inline] port_event drivers/usb/core/hub.c:5359 [inline] hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441 process_one_work+0x92b/0x1530 kernel/workqueue.c:2269 worker_thread+0x96/0xe20 kernel/workqueue.c:2415 kthread+0x318/0x420 kernel/kthread.c:255 ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352 Freed by task 22: save_stack+0x1b/0x80 mm/kasan/common.c:69 set_track mm/kasan/common.c:77 [inline] __kasan_slab_free+0x130/0x180 mm/kasan/common.c:449 slab_free_hook mm/slub.c:1423 [inline] slab_free_freelist_hook mm/slub.c:1470 [inline] slab_free mm/slub.c:3012 [inline] kfree+0xe4/0x2f0 mm/slub.c:3953
KASAN: use-after-free Read in hiddev_ioctl
Hello, syzbot found the following crash on: HEAD commit:e96407b4 usb-fuzzer: main usb gadget fuzzer driver git tree: https://github.com/google/kasan.git usb-fuzzer console output: https://syzkaller.appspot.com/x/log.txt?x=1732258a60 kernel config: https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e dashboard link: https://syzkaller.appspot.com/bug?extid=5e9ed50a49eb77802d0e compiler: gcc (GCC) 9.0.0 20181231 (experimental) Unfortunately, I don't have any reproducer for this crash yet. IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+5e9ed50a49eb77802...@syzkaller.appspotmail.com == BUG: KASAN: use-after-free in __mutex_lock_common kernel/locking/mutex.c:912 [inline] BUG: KASAN: use-after-free in __mutex_lock+0xf23/0x1360 kernel/locking/mutex.c:1077 Read of size 8 at addr 8881cf955468 by task syz-executor.1/19529 CPU: 0 PID: 19529 Comm: syz-executor.1 Not tainted 5.3.0-rc2+ #25 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0xca/0x13e lib/dump_stack.c:113 print_address_description+0x6a/0x32c mm/kasan/report.c:351 __kasan_report.cold+0x1a/0x33 mm/kasan/report.c:482 kasan_report+0xe/0x12 mm/kasan/common.c:612 __mutex_lock_common kernel/locking/mutex.c:912 [inline] __mutex_lock+0xf23/0x1360 kernel/locking/mutex.c:1077 hiddev_ioctl+0xea/0x1550 drivers/hid/usbhid/hiddev.c:607 vfs_ioctl fs/ioctl.c:46 [inline] file_ioctl fs/ioctl.c:509 [inline] do_vfs_ioctl+0xd2d/0x1330 fs/ioctl.c:696 ksys_ioctl+0x9b/0xc0 fs/ioctl.c:713 __do_sys_ioctl fs/ioctl.c:720 [inline] __se_sys_ioctl fs/ioctl.c:718 [inline] __x64_sys_ioctl+0x6f/0xb0 fs/ioctl.c:718 do_syscall_64+0xb7/0x580 arch/x86/entry/common.c:296 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x459829 Code: fd b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 cb b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00 RSP: 002b:7f0ec5dd7c78 EFLAGS: 0246 ORIG_RAX: 0010 RAX: ffda RBX: 0003 RCX: 00459829 RDX: 2380 RSI: 81044804 RDI: 0005 RBP: 0075c070 R08: R09: R10: R11: 0246 R12: 7f0ec5dd86d4 R13: 004c2249 R14: 004d55f8 R15: Allocated by task 2777: save_stack+0x1b/0x80 mm/kasan/common.c:69 set_track mm/kasan/common.c:77 [inline] __kasan_kmalloc mm/kasan/common.c:487 [inline] __kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:460 kmalloc include/linux/slab.h:552 [inline] kzalloc include/linux/slab.h:748 [inline] hiddev_connect+0x242/0x5b0 drivers/hid/usbhid/hiddev.c:900 hid_connect+0x239/0xbb0 drivers/hid/hid-core.c:1882 hid_hw_start drivers/hid/hid-core.c:1981 [inline] hid_hw_start+0xa2/0x130 drivers/hid/hid-core.c:1972 appleir_probe+0x13e/0x1a0 drivers/hid/hid-appleir.c:308 hid_device_probe+0x2be/0x3f0 drivers/hid/hid-core.c:2209 really_probe+0x281/0x650 drivers/base/dd.c:548 driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 __device_attach+0x217/0x360 drivers/base/dd.c:882 bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 device_add+0xae6/0x16f0 drivers/base/core.c:2114 hid_add_device+0x33c/0x990 drivers/hid/hid-core.c:2365 usbhid_probe+0xa81/0xfa0 drivers/hid/usbhid/hid-core.c:1386 usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361 really_probe+0x281/0x650 drivers/base/dd.c:548 driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 __device_attach+0x217/0x360 drivers/base/dd.c:882 bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 device_add+0xae6/0x16f0 drivers/base/core.c:2114 usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023 generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210 usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266 really_probe+0x281/0x650 drivers/base/dd.c:548 driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 __device_attach+0x217/0x360 drivers/base/dd.c:882 bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 device_add+0xae6/0x16f0 drivers/base/core.c:2114 usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536 hub_port_connect drivers/usb/core/hub.c:5098 [inline] hub_port_connect_change drivers/usb/core/hub.c:5213 [inline] port_event drivers/usb/core/hub.c:5359 [inline] hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441 process_one_work+0x92b/0x1530 kernel/workqueue.c:2269 proc
KASAN: slab-out-of-bounds Read in usbhid_close
Hello, syzbot found the following crash on: HEAD commit:e96407b4 usb-fuzzer: main usb gadget fuzzer driver git tree: https://github.com/google/kasan.git usb-fuzzer console output: https://syzkaller.appspot.com/x/log.txt?x=117a9f4260 kernel config: https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e dashboard link: https://syzkaller.appspot.com/bug?extid=3268ee512f866a903602 compiler: gcc (GCC) 9.0.0 20181231 (experimental) Unfortunately, I don't have any reproducer for this crash yet. IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+3268ee512f866a903...@syzkaller.appspotmail.com == BUG: KASAN: slab-out-of-bounds in __lock_acquire+0x302a/0x3b50 kernel/locking/lockdep.c:3753 Read of size 8 at addr 8881ceab68a0 by task syz-executor.0/3352 CPU: 1 PID: 3352 Comm: syz-executor.0 Not tainted 5.3.0-rc2+ #25 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0xca/0x13e lib/dump_stack.c:113 print_address_description+0x6a/0x32c mm/kasan/report.c:351 __kasan_report.cold+0x1a/0x33 mm/kasan/report.c:482 kasan_report+0xe/0x12 mm/kasan/common.c:612 __lock_acquire+0x302a/0x3b50 kernel/locking/lockdep.c:3753 lock_acquire+0x127/0x320 kernel/locking/lockdep.c:4412 __raw_spin_lock_irq include/linux/spinlock_api_smp.h:128 [inline] _raw_spin_lock_irq+0x2d/0x40 kernel/locking/spinlock.c:167 spin_lock_irq include/linux/spinlock.h:363 [inline] usbhid_close+0x51/0x210 drivers/hid/usbhid/hid-core.c:740 hid_hw_close+0xa8/0xd0 drivers/hid/hid-core.c:2046 drop_ref.part.0+0x32/0xe0 drivers/hid/hidraw.c:337 drop_ref drivers/hid/hidraw.c:360 [inline] hidraw_release+0x34f/0x440 drivers/hid/hidraw.c:356 __fput+0x2d7/0x840 fs/file_table.c:280 task_work_run+0x13f/0x1c0 kernel/task_work.c:113 exit_task_work include/linux/task_work.h:22 [inline] do_exit+0x8ef/0x2c50 kernel/exit.c:878 do_group_exit+0x125/0x340 kernel/exit.c:982 get_signal+0x466/0x23d0 kernel/signal.c:2728 do_signal+0x88/0x14e0 arch/x86/kernel/signal.c:815 exit_to_usermode_loop+0x1a2/0x200 arch/x86/entry/common.c:159 prepare_exit_to_usermode arch/x86/entry/common.c:194 [inline] syscall_return_slowpath arch/x86/entry/common.c:274 [inline] do_syscall_64+0x45f/0x580 arch/x86/entry/common.c:299 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x459829 Code: Bad RIP value. RSP: 002b:7f123439dcf8 EFLAGS: 0246 ORIG_RAX: 00ca RAX: fe00 RBX: 0075bf28 RCX: 00459829 RDX: RSI: 0080 RDI: 0075bf28 RBP: 0075bf20 R08: R09: R10: R11: 0246 R12: 0075bf2c R13: 7ffe9281699f R14: 7f123439e9c0 R15: 0075bf2c Allocated by task 0: (stack is not available) Freed by task 0: (stack is not available) The buggy address belongs to the object at 8881ceab6880 which belongs to the cache shmem_inode_cache of size 1168 The buggy address is located 32 bytes inside of 1168-byte region [8881ceab6880, 8881ceab6d10) The buggy address belongs to the page: page:ea00073aad00 refcount:1 mapcount:0 mapping:8881da115180 index:0x0 compound_mapcount: 0 flags: 0x2010200(slab|head) raw: 02010200 dead0100 dead0122 8881da115180 raw: 800c000c 0001 page dumped because: kasan: bad access detected Memory state around the buggy address: 8881ceab6780: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 8881ceab6800: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc 8881ceab6880: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc ^ 8881ceab6900: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc 8881ceab6980: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc == --- This bug is generated by a bot. It may contain errors. See https://goo.gl/tpsmEJ for more information about syzbot. syzbot engineers can be reached at syzkal...@googlegroups.com. syzbot will keep track of this bug report. See: https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
Re: [PATCH v8 00/11] add USB GPIO based connection detection driver
On Tue, Aug 6, 2019 at 9:48 AM Chunfeng Yun wrote: > On Mon, 2019-08-05 at 12:06 +0200, Linus Walleij wrote: > > On Wed, Jul 24, 2019 at 10:51 AM Chunfeng Yun > > wrote: > > But for just "hey I'm plugged in" we can surely keep this > > ID on GPIO detection in the USB subsystem. > > Sorry, you mean provide a common API? could you please describe more > clearer about your suggestion? Sorry I am not suggesting anything, this code is fine. But: > > I just get a bit insecure about how we should ideally > > handle these "funny-PHY's". I am more thinking about which subsystem these things really belong in. But let's keep it like this for the simple GPIO case. Acked-by: Linus Walleij Yours, Linus Walleij
[PATCH] xhci: Prevent deadlock when xhci adapter breaks during init
The system can hit a deadlock if xhci adapter breaks while initializing.
The deadlock is between two threads: thread 1 is tearing down the
adapter and is stuck in usb_unlocked_disable_lpm waiting to lock the
hcd->handwidth_mutex. Thread 2 is holding this mutex (while still trying
to add a usb device), but is stuck in xhci_endpoint_reset waiting for a
stop or config command to complete. A reboot is required to resolve.
It turns out when calling xhci_queue_stop_endpoint and
xhci_queue_configure_endpoint in xhci_endpoint_reset, the return code is
not checked for errors. If the timing is right and the adapter dies just
before either of these commands get issued, we hang indefinitely waiting
for a completion on a command that didn't get issued.
This wasn't a problem before the following fix because we didn't send
commands in xhci_endpoint_reset:
commit f5249461b504 ("xhci: Clear the host side toggle manually when endpoint
is soft reset")
With the patch I am submitting, a duration test which breaks adapters
during initialization (and which deadlocks with the standard kernel) runs
without issue.
Fixes: f5249461b504 ("xhci: Clear the host side toggle manually when endpoint
is soft reset")
Signed-off-by: Bill Kuzeja
---
drivers/usb/host/xhci.c | 22 +++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 248cd7a..835708d 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3132,7 +3132,16 @@ static void xhci_endpoint_reset(struct usb_hcd *hcd,
xhci_free_command(xhci, cfg_cmd);
goto cleanup;
}
- xhci_queue_stop_endpoint(xhci, stop_cmd, udev->slot_id, ep_index, 0);
+
+ if (xhci_queue_stop_endpoint(xhci, stop_cmd, udev->slot_id,
+ ep_index, 0) < 0) {
+ spin_unlock_irqrestore(&xhci->lock, flags);
+ xhci_free_command(xhci, cfg_cmd);
+ xhci_warn(xhci, "%s: stop_cmd xhci_queue_stop_endpoint "
+ "returns error, exiting\n", __func__);
+ goto cleanup;
+ }
+
xhci_ring_cmd_db(xhci);
spin_unlock_irqrestore(&xhci->lock, flags);
@@ -3146,8 +3155,15 @@ static void xhci_endpoint_reset(struct usb_hcd *hcd,
ctrl_ctx, ep_flag, ep_flag);
xhci_endpoint_copy(xhci, cfg_cmd->in_ctx, vdev->out_ctx, ep_index);
- xhci_queue_configure_endpoint(xhci, cfg_cmd, cfg_cmd->in_ctx->dma,
- udev->slot_id, false);
+ if (xhci_queue_configure_endpoint(xhci, cfg_cmd, cfg_cmd->in_ctx->dma,
+ udev->slot_id, false) < 0) {
+ spin_unlock_irqrestore(&xhci->lock, flags);
+ xhci_free_command(xhci, cfg_cmd);
+ xhci_warn(xhci, "%s: cfg_cmd xhci_queue_configure_endpoint "
+ "returns error, exiting\n", __func__);
+ goto cleanup;
+ }
+
xhci_ring_cmd_db(xhci);
spin_unlock_irqrestore(&xhci->lock, flags);
--
1.8.3.1
[PATCH] xhci: Prevent deadlock when xhci adapter breaks during init
The system can hit a deadlock if xhci adapter breaks while initializing.
The deadlock is between two threads: thread 1 is tearing down the
adapter and is stuck in usb_unlocked_disable_lpm waiting to lock the
hcd->handwidth_mutex. Thread 2 is holding this mutex (while still trying
to add a usb device), but is stuck in xhci_endpoint_reset waiting for a
stop or config command to complete. A reboot is required to resolve.
It turns out when calling xhci_queue_stop_endpoint and
xhci_queue_configure_endpoint in xhci_endpoint_reset, the return code is
not checked for errors. If the timing is right and the adapter dies just
before either of these commands get issued, we hang indefinitely waiting
for a completion on a command that didn't get issued.
This wasn't a problem before the following fix because we didn't send
commands in xhci_endpoint_reset:
commit f5249461b504 ("xhci: Clear the host side toggle manually when endpoint
is soft reset")
With the patch I am submitting, a duration test which breaks adapters
during initialization (and which deadlocks with the standard kernel) runs
without issue.
Fixes: f5249461b504 ("xhci: Clear the host side toggle manually when endpoint
is soft reset")
Signed-off-by: Bill Kuzeja
---
drivers/usb/host/xhci.c | 22 +++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 248cd7a..835708d 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3132,7 +3132,16 @@ static void xhci_endpoint_reset(struct usb_hcd *hcd,
xhci_free_command(xhci, cfg_cmd);
goto cleanup;
}
- xhci_queue_stop_endpoint(xhci, stop_cmd, udev->slot_id, ep_index, 0);
+
+ if (xhci_queue_stop_endpoint(xhci, stop_cmd, udev->slot_id,
+ ep_index, 0) < 0) {
+ spin_unlock_irqrestore(&xhci->lock, flags);
+ xhci_free_command(xhci, cfg_cmd);
+ xhci_warn(xhci, "%s: stop_cmd xhci_queue_stop_endpoint "
+ "returns error, exiting\n", __func__);
+ goto cleanup;
+ }
+
xhci_ring_cmd_db(xhci);
spin_unlock_irqrestore(&xhci->lock, flags);
@@ -3146,8 +3155,15 @@ static void xhci_endpoint_reset(struct usb_hcd *hcd,
ctrl_ctx, ep_flag, ep_flag);
xhci_endpoint_copy(xhci, cfg_cmd->in_ctx, vdev->out_ctx, ep_index);
- xhci_queue_configure_endpoint(xhci, cfg_cmd, cfg_cmd->in_ctx->dma,
- udev->slot_id, false);
+ if (xhci_queue_configure_endpoint(xhci, cfg_cmd, cfg_cmd->in_ctx->dma,
+ udev->slot_id, false) < 0) {
+ spin_unlock_irqrestore(&xhci->lock, flags);
+ xhci_free_command(xhci, cfg_cmd);
+ xhci_warn(xhci, "%s: cfg_cmd xhci_queue_configure_endpoint "
+ "returns error, exiting\n", __func__);
+ goto cleanup;
+ }
+
xhci_ring_cmd_db(xhci);
spin_unlock_irqrestore(&xhci->lock, flags);
--
1.8.3.1
Re: WARNING in __iforce_usb_xmit/usb_submit_urb
Am Montag, den 05.08.2019, 04:58 -0700 schrieb syzbot: > Hello, > > syzbot found the following crash on: > > HEAD commit:e96407b4 usb-fuzzer: main usb gadget fuzzer driver > git tree: https://github.com/google/kasan.git usb-fuzzer > console output: https://syzkaller.appspot.com/x/log.txt?x=10809e0c60 > kernel config: https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e > dashboard link: https://syzkaller.appspot.com/bug?extid=5efc10c005014d061a74 > compiler: gcc (GCC) 9.0.0 20181231 (experimental) > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=15e40b1a60 > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=174a69d860 > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > Reported-by: syzbot+5efc10c005014d061...@syzkaller.appspotmail.com #syz test: https://github.com/google/kasan.git e96407b4 >From 06be579ae09483c7c723067f4e5bf938ad302bda Mon Sep 17 00:00:00 2001 From: Oliver Neukum Date: Tue, 6 Aug 2019 15:33:35 +0200 Subject: [PATCH] iforce: add sanity checks The endpoint type should also be checked before a device is accepted. Reported-by: syzbot+5efc10c005014d061...@syzkaller.appspotmail.com Signed-off-by: Oliver Neukum --- drivers/input/joystick/iforce/iforce-usb.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/input/joystick/iforce/iforce-usb.c b/drivers/input/joystick/iforce/iforce-usb.c index 29abfeeef9a5..a481a226166c 100644 --- a/drivers/input/joystick/iforce/iforce-usb.c +++ b/drivers/input/joystick/iforce/iforce-usb.c @@ -203,6 +203,11 @@ static int iforce_usb_probe(struct usb_interface *intf, epirq = &interface->endpoint[0].desc; epout = &interface->endpoint[1].desc; + if (!usb_endpoint_is_int_in(epirq)) + return -ENODEV; + if (!usb_endpoint_is_int_out(epout)) + return -ENODEV; + iforce_usb = kzalloc(sizeof(*iforce_usb), GFP_KERNEL); if (!iforce_usb) goto fail; -- 2.16.4
Re: WARNING in __iforce_usb_xmit/usb_submit_urb
Hello, syzbot has tested the proposed patch and the reproducer did not trigger crash: Reported-and-tested-by: syzbot+5efc10c005014d061...@syzkaller.appspotmail.com Tested on: commit: e96407b4 usb-fuzzer: main usb gadget fuzzer driver git tree: https://github.com/google/kasan.git kernel config: https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e compiler: gcc (GCC) 9.0.0 20181231 (experimental) patch: https://syzkaller.appspot.com/x/patch.diff?x=13554e2660 Note: testing is done by a robot and is best-effort only.
Re: KMSAN: kernel-usb-infoleak in pcan_usb_pro_send_req
On Tue, 6 Aug 2019, Andrey Konovalov wrote: > On Tue, Aug 6, 2019 at 2:45 PM Oliver Neukum wrote: > > > > Am Dienstag, den 30.07.2019, 02:38 -0700 schrieb syzbot: > > > Hello, > > > > > > syzbot found the following crash on: > > > > > > HEAD commit:41550654 [UPSTREAM] KVM: x86: degrade WARN to > > > pr_warn_rate.. > > > git tree: kmsan > > > console output: https://syzkaller.appspot.com/x/log.txt?x=13e95183a0 > > > kernel config: https://syzkaller.appspot.com/x/.config?x=40511ad0c5945201 > > > dashboard link: > > > https://syzkaller.appspot.com/bug?extid=513e4d0985298538bf9b > > > compiler: clang version 9.0.0 (/home/glider/llvm/clang > > > 80fee25776c2fb61e74c1ecb1a523375c2500b69) > > > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=17eafa1ba0 > > > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=17b87983a0 > > > > > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > > > Reported-by: syzbot+513e4d0985298538b...@syzkaller.appspotmail.com > > > > #syz test: https://github.com/google/kasan.git 41550654 > > Hi Oliver, > > For KMSAN bugs you'll need to use the kmsan tree (syz test: > https://github.com/google/kmsan.git COMMIT_ID). I've updated the > testing instructions some time ago to reflect this. Sorry for the > complexity, this is caused by USB fuzzing and KMSAN not being upstream > yet. Maybe you can fix the "git tree:" header in the bug report. It should say "https://github.com/google/kmsan.git"; instead of just "kmsan". Alan Stern
Re: KMSAN: kernel-usb-infoleak in pcan_usb_pro_send_req
On Tue, Aug 6, 2019 at 3:59 PM Alan Stern wrote: > > On Tue, 6 Aug 2019, Andrey Konovalov wrote: > > > On Tue, Aug 6, 2019 at 2:45 PM Oliver Neukum wrote: > > > > > > Am Dienstag, den 30.07.2019, 02:38 -0700 schrieb syzbot: > > > > Hello, > > > > > > > > syzbot found the following crash on: > > > > > > > > HEAD commit:41550654 [UPSTREAM] KVM: x86: degrade WARN to > > > > pr_warn_rate.. > > > > git tree: kmsan > > > > console output: https://syzkaller.appspot.com/x/log.txt?x=13e95183a0 > > > > kernel config: > > > > https://syzkaller.appspot.com/x/.config?x=40511ad0c5945201 > > > > dashboard link: > > > > https://syzkaller.appspot.com/bug?extid=513e4d0985298538bf9b > > > > compiler: clang version 9.0.0 (/home/glider/llvm/clang > > > > 80fee25776c2fb61e74c1ecb1a523375c2500b69) > > > > syz repro: > > > > https://syzkaller.appspot.com/x/repro.syz?x=17eafa1ba0 > > > > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=17b87983a0 > > > > > > > > IMPORTANT: if you fix the bug, please add the following tag to the > > > > commit: > > > > Reported-by: syzbot+513e4d0985298538b...@syzkaller.appspotmail.com > > > > > > #syz test: https://github.com/google/kasan.git 41550654 > > > > Hi Oliver, > > > > For KMSAN bugs you'll need to use the kmsan tree (syz test: > > https://github.com/google/kmsan.git COMMIT_ID). I've updated the > > testing instructions some time ago to reflect this. Sorry for the > > complexity, this is caused by USB fuzzing and KMSAN not being upstream > > yet. > > Maybe you can fix the "git tree:" header in the bug report. It should > say "https://github.com/google/kmsan.git"; instead of just "kmsan". Makes sense, will do, thanks! > > Alan Stern >
Re: KASAN: use-after-free Read in device_release_driver_internal
On Tue, 6 Aug 2019, Oliver Neukum wrote:
> Am Donnerstag, den 01.08.2019, 14:47 -0400 schrieb Alan Stern:
> >
> > I think this must be caused by an unbalanced refcount. That is,
> > something must drop one more reference to the device than it takes.
> > That would explain why the invalid access occurs inside a single
> > bus_remove_device() call, between the klist_del() and
> > device_release_driver().
> >
> > The kernel log indicates that the device was probed by rndis_wlan,
> > rndis_host, and cdc_acm, all of which got errors because of the
> > device's bogus descriptors. Probably one of them is messing up the
> > refcount.
>
> Hi,
>
> you made me look at cdc-acm. I suspect
>
> cae2bc768d176bfbdad7035bbcc3cdc973eb7984 ("usb: cdc-acm: Decrement tty port's
> refcount if probe() fail")
>
> is buggy decrementing the refcount on the interface in destroy()
> even before the refcount is increased.
>
> Unfortunately I cannot tell from the bug report how many and which
> interfaces the emulated test device has. Hence it is unclear to me,
> when exactly probe() would fail cdc-acm.
Only one interface (numbered 234!).
> If you agree. I am attaching a putative fix.
Your patch adds a line saying:
> + usb_get_intf(acm->control); /* undone in destroy() */
but I don't see any destroy() function in that source file. Did you
mean acm_port_destruct()?
In any case, I don't know if this missing "get" would cause the
problem, but it might well.
Alan Stern
Re: possible deadlock in usb_deregister_dev
Am Montag, den 05.08.2019, 04:58 -0700 schrieb syzbot:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit:e96407b4 usb-fuzzer: main usb gadget fuzzer driver
> git tree: https://github.com/google/kasan.git usb-fuzzer
> console output: https://syzkaller.appspot.com/x/log.txt?x=13b5bc8a60
> kernel config: https://syzkaller.appspot.com/x/.config?x=792eb47789f57810
> dashboard link: https://syzkaller.appspot.com/bug?extid=a64a382964bf6c71a9c0
> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
>
> Unfortunately, I don't have any reproducer for this crash yet.
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+a64a382964bf6c71a...@syzkaller.appspotmail.com
#syz test: https://github.com/google/kasan.git e96407b4
>From 973e82b3f583113e4d7fe5cd2f918a16022c4e38 Mon Sep 17 00:00:00 2001
From: Oliver Neukum
Date: Tue, 6 Aug 2019 16:17:54 +0200
Subject: [PATCH] usb: iowarrior: fix deadlock on disconnect
We have to drop the mutex before we close() upon disconnect()
as close() needs the lock. This is safe to do by dropping the
mutex as intfdata is already set to NULL, so open() will fail.
Fixes: 03f36e885fc26 ("USB: open disconnect race in iowarrior")
Reported-by: syzbot+a64a382964bf6c71a...@syzkaller.appspotmail.com
Signed-off-by: Oliver Neukum
---
drivers/usb/misc/iowarrior.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/misc/iowarrior.c b/drivers/usb/misc/iowarrior.c
index ba05dd80a020..f5bed9f29e56 100644
--- a/drivers/usb/misc/iowarrior.c
+++ b/drivers/usb/misc/iowarrior.c
@@ -866,19 +866,20 @@ static void iowarrior_disconnect(struct usb_interface
*interface)
dev = usb_get_intfdata(interface);
mutex_lock(&iowarrior_open_disc_lock);
usb_set_intfdata(interface, NULL);
+ /* prevent device read, write and ioctl */
+ dev->present = 0;
minor = dev->minor;
+ mutex_unlock(&iowarrior_open_disc_lock);
+ /* give back our minor - this will call close() locks need to be
dropped at this point*/
- /* give back our minor */
usb_deregister_dev(interface, &iowarrior_class);
mutex_lock(&dev->mutex);
/* prevent device read, write and ioctl */
- dev->present = 0;
mutex_unlock(&dev->mutex);
- mutex_unlock(&iowarrior_open_disc_lock);
if (dev->opened) {
/* There is a process that holds a filedescriptor to the device
,
--
2.16.4
Re: Re: possible deadlock in usb_deregister_dev
Am Montag, den 05.08.2019, 04:58 -0700 schrieb syzbot:
Hello,
syzbot found the following crash on:
HEAD commit:e96407b4 usb-fuzzer: main usb gadget fuzzer driver
git tree: https://github.com/google/kasan.git usb-fuzzer
console output: https://syzkaller.appspot.com/x/log.txt?x=13b5bc8a60
kernel config:
https://syzkaller.appspot.com/x/.config?x=792eb47789f57810
dashboard link:
https://syzkaller.appspot.com/bug?extid=a64a382964bf6c71a9c0
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
Unfortunately, I don't have any reproducer for this crash yet.
IMPORTANT: if you fix the bug, please add the following tag to the
commit:
Reported-by: syzbot+a64a382964bf6c71a...@syzkaller.appspotmail.com
#syz test: https://github.com/google/kasan.git e96407b4
This crash does not have a reproducer. I cannot test it.
From 973e82b3f583113e4d7fe5cd2f918a16022c4e38 Mon Sep 17 00:00:00 2001
From: Oliver Neukum
Date: Tue, 6 Aug 2019 16:17:54 +0200
Subject: [PATCH] usb: iowarrior: fix deadlock on disconnect
We have to drop the mutex before we close() upon disconnect()
as close() needs the lock. This is safe to do by dropping the
mutex as intfdata is already set to NULL, so open() will fail.
Fixes: 03f36e885fc26 ("USB: open disconnect race in iowarrior")
Reported-by: syzbot+a64a382964bf6c71a...@syzkaller.appspotmail.com
Signed-off-by: Oliver Neukum
---
drivers/usb/misc/iowarrior.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/misc/iowarrior.c b/drivers/usb/misc/iowarrior.c
index ba05dd80a020..f5bed9f29e56 100644
--- a/drivers/usb/misc/iowarrior.c
+++ b/drivers/usb/misc/iowarrior.c
@@ -866,19 +866,20 @@ static void iowarrior_disconnect(struct
usb_interface *interface)
dev = usb_get_intfdata(interface);
mutex_lock(&iowarrior_open_disc_lock);
usb_set_intfdata(interface, NULL);
+ /* prevent device read, write and ioctl */
+ dev->present = 0;
minor = dev->minor;
+ mutex_unlock(&iowarrior_open_disc_lock);
+ /* give back our minor - this will call close() locks need to be
dropped at this point*/
- /* give back our minor */
usb_deregister_dev(interface, &iowarrior_class);
mutex_lock(&dev->mutex);
/* prevent device read, write and ioctl */
- dev->present = 0;
mutex_unlock(&dev->mutex);
- mutex_unlock(&iowarrior_open_disc_lock);
if (dev->opened) {
/* There is a process that holds a filedescriptor to the device
,
--
2.16.4
Re: KASAN: use-after-free Read in device_release_driver_internal
Am Dienstag, den 06.08.2019, 10:19 -0400 schrieb Alan Stern:
> On Tue, 6 Aug 2019, Oliver Neukum wrote:
>
> > Am Donnerstag, den 01.08.2019, 14:47 -0400 schrieb Alan Stern:
> > >
> > > I think this must be caused by an unbalanced refcount. That is,
> > > something must drop one more reference to the device than it takes.
> > > That would explain why the invalid access occurs inside a single
> > > bus_remove_device() call, between the klist_del() and
> > > device_release_driver().
> > >
> > > The kernel log indicates that the device was probed by rndis_wlan,
> > > rndis_host, and cdc_acm, all of which got errors because of the
> > > device's bogus descriptors. Probably one of them is messing up the
> > > refcount.
> >
> > Hi,
> >
> > you made me look at cdc-acm. I suspect
> >
> > cae2bc768d176bfbdad7035bbcc3cdc973eb7984 ("usb: cdc-acm: Decrement tty
> > port's refcount if probe() fail")
> >
> > is buggy decrementing the refcount on the interface in destroy()
> > even before the refcount is increased.
> >
> > Unfortunately I cannot tell from the bug report how many and which
> > interfaces the emulated test device has. Hence it is unclear to me,
> > when exactly probe() would fail cdc-acm.
>
> Only one interface (numbered 234!).
Yes. cdc-acm went into the look_for_collapsed_interface code path.
But I cannot tell whether it proceeded to made_compressed_probe
(Yes, I know the code makes extensive use of "goto")
> > If you agree. I am attaching a putative fix.
>
> Your patch adds a line saying:
>
> > + usb_get_intf(acm->control); /* undone in destroy() */
>
> but I don't see any destroy() function in that source file. Did you
> mean acm_port_destruct()?
Yes, sorry
> In any case, I don't know if this missing "get" would cause the
> problem, but it might well.
Then let's wait for the result.
Regards
Oliver
Re: KMSAN: kernel-usb-infoleak in pcan_usb_pro_send_req
Hello, syzbot has tested the proposed patch and the reproducer did not trigger crash: Reported-and-tested-by: syzbot+513e4d0985298538b...@syzkaller.appspotmail.com Tested on: commit: 41550654 [UPSTREAM] KVM: x86: degrade WARN to pr_warn_rate.. git tree: https://github.com/google/kmsan.git kernel config: https://syzkaller.appspot.com/x/.config?x=40511ad0c5945201 compiler: clang version 9.0.0 (/home/glider/llvm/clang 80fee25776c2fb61e74c1ecb1a523375c2500b69) patch: https://syzkaller.appspot.com/x/patch.diff?x=1170f88c60 Note: testing is done by a robot and is best-effort only.
[PATCH 01/12] USB: add support for dev_groups to struct usb_driver
Now that the driver core supports dev_groups for individual drivers,
expose that pointer to struct usb_driver to make it easier for USB
drivers to also use it.
Signed-off-by: Greg Kroah-Hartman
---
drivers/usb/core/driver.c | 1 +
include/linux/usb.h | 3 +++
2 files changed, 4 insertions(+)
diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
index ebcadaad89d1..687fc5df4c17 100644
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -954,6 +954,7 @@ int usb_register_driver(struct usb_driver *new_driver,
struct module *owner,
new_driver->drvwrap.driver.remove = usb_unbind_interface;
new_driver->drvwrap.driver.owner = owner;
new_driver->drvwrap.driver.mod_name = mod_name;
+ new_driver->drvwrap.driver.dev_groups = new_driver->dev_groups;
spin_lock_init(&new_driver->dynids.lock);
INIT_LIST_HEAD(&new_driver->dynids.list);
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 83d35d993e8c..af4eb6419ae8 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -1151,6 +1151,8 @@ struct usbdrv_wrap {
* @id_table: USB drivers use ID table to support hotplugging.
* Export this with MODULE_DEVICE_TABLE(usb,...). This must be set
* or your driver's probe function will never get called.
+ * @dev_groups: Attributes attached to the device that will be created once it
+ * is bound to the driver.
* @dynids: used internally to hold the list of dynamically added device
* ids for this driver.
* @drvwrap: Driver-model core structure wrapper.
@@ -1198,6 +1200,7 @@ struct usb_driver {
int (*post_reset)(struct usb_interface *intf);
const struct usb_device_id *id_table;
+ const struct attribute_group **dev_groups;
struct usb_dynids dynids;
struct usbdrv_wrap drvwrap;
--
2.22.0
[PATCH 00/12] USB: dev_groups support for usb drivers
Now that the driver core supports the ability for individual drivers to have attribute groups added/removed when a device is bound/unbound to a driver automatically, we should take advantage of that in the USB subsystem. This patch series adds dev_groups support to struct usb_driver and struct usb_device_driver (needed for the usbip driver), and then converts a number of individual USB drivers over to use this api. Greg Kroah-Hartman (12): USB: add support for dev_groups to struct usb_driver USB: add support for dev_groups to struct usb_device_driver USB: atm: cxacru: convert to use dev_groups USB: ueagle-atm: convert to use dev_groups USB: usblp: convert to use dev_groups USB: usbtmc: convert to use dev_groups USB: cypress_cy7c63: convert to use dev_groups USB: cytherm: convert to use dev_groups USB: lvstest: convert to use dev_groups USB: trancevibrator: convert to use dev_groups USB: usbsevseg: convert to use dev_groups USB: usbip: convert to use dev_groups drivers/usb/atm/cxacru.c | 58 drivers/usb/atm/ueagle-atm.c | 16 ++-- drivers/usb/class/usblp.c | 13 --- drivers/usb/class/usbtmc.c| 13 ++- drivers/usb/core/driver.c | 2 + drivers/usb/misc/cypress_cy7c63.c | 29 -- drivers/usb/misc/cytherm.c| 64 +-- drivers/usb/misc/lvstest.c| 19 ++--- drivers/usb/misc/trancevibrator.c | 15 drivers/usb/misc/usbsevseg.c | 17 ++-- drivers/usb/usbip/stub_dev.c | 50 include/linux/usb.h | 6 +++ 12 files changed, 104 insertions(+), 198 deletions(-) -- 2.22.0
[PATCH 11/12] USB: usbsevseg: convert to use dev_groups
USB drivers now support the ability for the driver core to handle the
creation and removal of device-specific sysfs files in a race-free
manner. Take advantage of that by converting the driver to use this by
moving the sysfs attributes into a group and assigning the dev_groups
pointer to it.
Signed-off-by: Greg Kroah-Hartman
---
drivers/usb/misc/usbsevseg.c | 17 +++--
1 file changed, 3 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/misc/usbsevseg.c b/drivers/usb/misc/usbsevseg.c
index 1923d5b6d9c9..551074f5b7ad 100644
--- a/drivers/usb/misc/usbsevseg.c
+++ b/drivers/usb/misc/usbsevseg.c
@@ -316,7 +316,7 @@ MYDEV_ATTR_SIMPLE_UNSIGNED(powered, update_display_powered);
MYDEV_ATTR_SIMPLE_UNSIGNED(mode_msb, update_display_mode);
MYDEV_ATTR_SIMPLE_UNSIGNED(mode_lsb, update_display_mode);
-static struct attribute *dev_attrs[] = {
+static struct attribute *sevseg_attrs[] = {
&dev_attr_powered.attr,
&dev_attr_text.attr,
&dev_attr_textmode.attr,
@@ -325,10 +325,7 @@ static struct attribute *dev_attrs[] = {
&dev_attr_mode_lsb.attr,
NULL
};
-
-static const struct attribute_group dev_attr_grp = {
- .attrs = dev_attrs,
-};
+ATTRIBUTE_GROUPS(sevseg);
static int sevseg_probe(struct usb_interface *interface,
const struct usb_device_id *id)
@@ -354,17 +351,9 @@ static int sevseg_probe(struct usb_interface *interface,
mydev->mode_msb = 0x06; /* 6 characters */
mydev->mode_lsb = 0x3f; /* scanmode for 6 chars */
- rc = sysfs_create_group(&interface->dev.kobj, &dev_attr_grp);
- if (rc)
- goto error;
-
dev_info(&interface->dev, "USB 7 Segment device now attached\n");
return 0;
-error:
- usb_set_intfdata(interface, NULL);
- usb_put_dev(mydev->udev);
- kfree(mydev);
error_mem:
return rc;
}
@@ -374,7 +363,6 @@ static void sevseg_disconnect(struct usb_interface
*interface)
struct usb_sevsegdev *mydev;
mydev = usb_get_intfdata(interface);
- sysfs_remove_group(&interface->dev.kobj, &dev_attr_grp);
usb_set_intfdata(interface, NULL);
usb_put_dev(mydev->udev);
kfree(mydev);
@@ -423,6 +411,7 @@ static struct usb_driver sevseg_driver = {
.resume = sevseg_resume,
.reset_resume = sevseg_reset_resume,
.id_table = id_table,
+ .dev_groups = sevseg_groups,
.supports_autosuspend = 1,
};
--
2.22.0
[PATCH 12/12] USB: usbip: convert to use dev_groups
USB drivers now support the ability for the driver core to handle the
creation and removal of device-specific sysfs files in a race-free
manner. Take advantage of that by converting the driver to use this by
moving the sysfs attributes into a group and assigning the dev_groups
pointer to it.
Cc: Valentina Manea
Cc: Shuah Khan
Signed-off-by: Greg Kroah-Hartman
---
drivers/usb/usbip/stub_dev.c | 50 ++--
1 file changed, 8 insertions(+), 42 deletions(-)
diff --git a/drivers/usb/usbip/stub_dev.c b/drivers/usb/usbip/stub_dev.c
index 7931e6cecc70..2305d425e6c9 100644
--- a/drivers/usb/usbip/stub_dev.c
+++ b/drivers/usb/usbip/stub_dev.c
@@ -106,38 +106,13 @@ static ssize_t usbip_sockfd_store(struct device *dev,
struct device_attribute *a
}
static DEVICE_ATTR_WO(usbip_sockfd);
-static int stub_add_files(struct device *dev)
-{
- int err = 0;
-
- err = device_create_file(dev, &dev_attr_usbip_status);
- if (err)
- goto err_status;
-
- err = device_create_file(dev, &dev_attr_usbip_sockfd);
- if (err)
- goto err_sockfd;
-
- err = device_create_file(dev, &dev_attr_usbip_debug);
- if (err)
- goto err_debug;
-
- return 0;
-
-err_debug:
- device_remove_file(dev, &dev_attr_usbip_sockfd);
-err_sockfd:
- device_remove_file(dev, &dev_attr_usbip_status);
-err_status:
- return err;
-}
-
-static void stub_remove_files(struct device *dev)
-{
- device_remove_file(dev, &dev_attr_usbip_status);
- device_remove_file(dev, &dev_attr_usbip_sockfd);
- device_remove_file(dev, &dev_attr_usbip_debug);
-}
+static struct attribute *usbip_attrs[] = {
+ &dev_attr_usbip_status.attr,
+ &dev_attr_usbip_sockfd.attr,
+ &dev_attr_usbip_debug.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(usbip);
static void stub_shutdown_connection(struct usbip_device *ud)
{
@@ -379,17 +354,8 @@ static int stub_probe(struct usb_device *udev)
goto err_port;
}
- rc = stub_add_files(&udev->dev);
- if (rc) {
- dev_err(&udev->dev, "stub_add_files for %s\n", udev_busid);
- goto err_files;
- }
-
return 0;
-err_files:
- usb_hub_release_port(udev->parent, udev->portnum,
-(struct usb_dev_state *) udev);
err_port:
dev_set_drvdata(&udev->dev, NULL);
usb_put_dev(udev);
@@ -457,7 +423,6 @@ static void stub_disconnect(struct usb_device *udev)
/*
* NOTE: rx/tx threads are invoked for each usb_device.
*/
- stub_remove_files(&udev->dev);
/* release port */
rc = usb_hub_release_port(udev->parent, udev->portnum,
@@ -526,4 +491,5 @@ struct usb_device_driver stub_driver = {
.resume = stub_resume,
#endif
.supports_autosuspend = 0,
+ .dev_groups = usbip_groups,
};
--
2.22.0
[PATCH 10/12] USB: trancevibrator: convert to use dev_groups
USB drivers now support the ability for the driver core to handle the
creation and removal of device-specific sysfs files in a race-free
manner. Take advantage of that by converting the driver to use this by
moving the sysfs attributes into a group and assigning the dev_groups
pointer to it.
Cc: Ding Xiang
Signed-off-by: Greg Kroah-Hartman
---
drivers/usb/misc/trancevibrator.c | 15 +++
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/misc/trancevibrator.c
b/drivers/usb/misc/trancevibrator.c
index ac357ce2d1a6..a3dfc77578ea 100644
--- a/drivers/usb/misc/trancevibrator.c
+++ b/drivers/usb/misc/trancevibrator.c
@@ -71,9 +71,14 @@ static ssize_t speed_store(struct device *dev, struct
device_attribute *attr,
}
return count;
}
-
static DEVICE_ATTR_RW(speed);
+static struct attribute *tv_attrs[] = {
+ &dev_attr_speed.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(tv);
+
static int tv_probe(struct usb_interface *interface,
const struct usb_device_id *id)
{
@@ -89,15 +94,9 @@ static int tv_probe(struct usb_interface *interface,
dev->udev = usb_get_dev(udev);
usb_set_intfdata(interface, dev);
- retval = device_create_file(&interface->dev, &dev_attr_speed);
- if (retval)
- goto error_create_file;
return 0;
-error_create_file:
- usb_put_dev(udev);
- usb_set_intfdata(interface, NULL);
error:
kfree(dev);
return retval;
@@ -108,7 +107,6 @@ static void tv_disconnect(struct usb_interface *interface)
struct trancevibrator *dev;
dev = usb_get_intfdata (interface);
- device_remove_file(&interface->dev, &dev_attr_speed);
usb_set_intfdata(interface, NULL);
usb_put_dev(dev->udev);
kfree(dev);
@@ -120,6 +118,7 @@ static struct usb_driver tv_driver = {
.probe =tv_probe,
.disconnect = tv_disconnect,
.id_table = id_table,
+ .dev_groups = tv_groups,
};
module_usb_driver(tv_driver);
--
2.22.0
[PATCH 02/12] USB: add support for dev_groups to struct usb_device_driver
Now that the driver core supports dev_groups for individual drivers,
expose that pointer to struct usb_device_driver to make it easier for USB
drivers to also use it.
Yes, users of usb_device_driver are much rare, but there are instances
already that use custom sysfs files, so adding this support will make
things easier for those drivers. usbip is one example, hubs might be
another one.
Signed-off-by: Greg Kroah-Hartman
---
drivers/usb/core/driver.c | 1 +
include/linux/usb.h | 3 +++
2 files changed, 4 insertions(+)
diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
index 687fc5df4c17..2b27d232d7a7 100644
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -892,6 +892,7 @@ int usb_register_device_driver(struct usb_device_driver
*new_udriver,
new_udriver->drvwrap.driver.probe = usb_probe_device;
new_udriver->drvwrap.driver.remove = usb_unbind_device;
new_udriver->drvwrap.driver.owner = owner;
+ new_udriver->drvwrap.driver.dev_groups = new_udriver->dev_groups;
retval = driver_register(&new_udriver->drvwrap.driver);
diff --git a/include/linux/usb.h b/include/linux/usb.h
index af4eb6419ae8..57f667cad3ec 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -1224,6 +1224,8 @@ struct usb_driver {
* module is being unloaded.
* @suspend: Called when the device is going to be suspended by the system.
* @resume: Called when the device is being resumed by the system.
+ * @dev_groups: Attributes attached to the device that will be created once it
+ * is bound to the driver.
* @drvwrap: Driver-model core structure wrapper.
* @supports_autosuspend: if set to 0, the USB core will not allow autosuspend
* for devices bound to this driver.
@@ -1238,6 +1240,7 @@ struct usb_device_driver {
int (*suspend) (struct usb_device *udev, pm_message_t message);
int (*resume) (struct usb_device *udev, pm_message_t message);
+ const struct attribute_group **dev_groups;
struct usbdrv_wrap drvwrap;
unsigned int supports_autosuspend:1;
};
--
2.22.0
[PATCH 03/12] USB: atm: cxacru: convert to use dev_groups
USB drivers now support the ability for the driver core to handle the
creation and removal of device-specific sysfs files in a race-free
manner. Take advantage of that by converting the driver to use this by
moving the sysfs attributes into a group and assigning the dev_groups
pointer to it.
Signed-off-by: Greg Kroah-Hartman
---
drivers/usb/atm/cxacru.c | 58 +++-
1 file changed, 33 insertions(+), 25 deletions(-)
diff --git a/drivers/usb/atm/cxacru.c b/drivers/usb/atm/cxacru.c
index e57a2be8754a..5d41f85a7445 100644
--- a/drivers/usb/atm/cxacru.c
+++ b/drivers/usb/atm/cxacru.c
@@ -539,6 +539,37 @@ CXACRU_SET_##_action(
adsl_config);
CXACRU_ALL_FILES(INIT);
+static struct attribute *cxacru_attrs[] = {
+ &dev_attr_adsl_config.attr,
+ &dev_attr_adsl_state.attr,
+ &dev_attr_adsl_controller_version.attr,
+ &dev_attr_adsl_headend_environment.attr,
+ &dev_attr_adsl_headend.attr,
+ &dev_attr_modulation.attr,
+ &dev_attr_line_startable.attr,
+ &dev_attr_downstream_hec_errors.attr,
+ &dev_attr_upstream_hec_errors.attr,
+ &dev_attr_downstream_fec_errors.attr,
+ &dev_attr_upstream_fec_errors.attr,
+ &dev_attr_downstream_crc_errors.attr,
+ &dev_attr_upstream_crc_errors.attr,
+ &dev_attr_startup_attempts.attr,
+ &dev_attr_downstream_bits_per_frame.attr,
+ &dev_attr_upstream_bits_per_frame.attr,
+ &dev_attr_transmitter_power.attr,
+ &dev_attr_downstream_attenuation.attr,
+ &dev_attr_upstream_attenuation.attr,
+ &dev_attr_downstream_snr_margin.attr,
+ &dev_attr_upstream_snr_margin.attr,
+ &dev_attr_mac_address.attr,
+ &dev_attr_line_status.attr,
+ &dev_attr_link_status.attr,
+ &dev_attr_upstream_rate.attr,
+ &dev_attr_downstream_rate.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(cxacru);
+
/* the following three functions are stolen from drivers/usb/core/message.c */
static void cxacru_blocking_completion(struct urb *urb)
{
@@ -736,17 +767,6 @@ static int cxacru_card_status(struct cxacru_data *instance)
return 0;
}
-static void cxacru_remove_device_files(struct usbatm_data *usbatm_instance,
- struct atm_dev *atm_dev)
-{
- struct usb_interface *intf = usbatm_instance->usb_intf;
-
- #define CXACRU_DEVICE_REMOVE_FILE(_name) \
- device_remove_file(&intf->dev, &dev_attr_##_name);
- CXACRU_ALL_FILES(REMOVE);
- #undef CXACRU_DEVICE_REMOVE_FILE
-}
-
static int cxacru_atm_start(struct usbatm_data *usbatm_instance,
struct atm_dev *atm_dev)
{
@@ -765,13 +785,6 @@ static int cxacru_atm_start(struct usbatm_data
*usbatm_instance,
return ret;
}
- #define CXACRU_DEVICE_CREATE_FILE(_name) \
- ret = device_create_file(&intf->dev, &dev_attr_##_name); \
- if (unlikely(ret)) \
- goto fail_sysfs;
- CXACRU_ALL_FILES(CREATE);
- #undef CXACRU_DEVICE_CREATE_FILE
-
/* start ADSL */
mutex_lock(&instance->adsl_state_serialize);
ret = cxacru_cm(instance, CM_REQUEST_CHIP_ADSL_LINE_START, NULL, 0,
NULL, 0);
@@ -804,11 +817,6 @@ static int cxacru_atm_start(struct usbatm_data
*usbatm_instance,
if (start_polling)
cxacru_poll_status(&instance->poll_work.work);
return 0;
-
-fail_sysfs:
- usb_err(usbatm_instance, "cxacru_atm_start: device_create_file failed
(%d)\n", ret);
- cxacru_remove_device_files(usbatm_instance, atm_dev);
- return ret;
}
static void cxacru_poll_status(struct work_struct *work)
@@ -1332,7 +1340,6 @@ static struct usbatm_driver cxacru_driver = {
.heavy_init = cxacru_heavy_init,
.unbind = cxacru_unbind,
.atm_start = cxacru_atm_start,
- .atm_stop = cxacru_remove_device_files,
.bulk_in= CXACRU_EP_DATA,
.bulk_out = CXACRU_EP_DATA,
.rx_padding = 3,
@@ -1364,7 +1371,8 @@ static struct usb_driver cxacru_usb_driver = {
.name = cxacru_driver_name,
.probe = cxacru_usb_probe,
.disconnect = usbatm_usb_disconnect,
- .id_table = cxacru_usb_ids
+ .id_table = cxacru_usb_ids,
+ .dev_groups = cxacru_groups,
};
module_usb_driver(cxacru_usb_driver);
--
2.22.0
[PATCH 04/12] USB: ueagle-atm: convert to use dev_groups
USB drivers now support the ability for the driver core to handle the
creation and removal of device-specific sysfs files in a race-free
manner. Take advantage of that by converting the driver to use this by
moving the sysfs attributes into a group and assigning the dev_groups
pointer to it.
Cc: Matthieu CASTET
Cc: Stanislaw Gruszka
Signed-off-by: Greg Kroah-Hartman
---
drivers/usb/atm/ueagle-atm.c | 16
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/drivers/usb/atm/ueagle-atm.c b/drivers/usb/atm/ueagle-atm.c
index 8faa51b1a520..8b0ea8c70d73 100644
--- a/drivers/usb/atm/ueagle-atm.c
+++ b/drivers/usb/atm/ueagle-atm.c
@@ -2458,7 +2458,7 @@ static int claim_interface(struct usb_device *usb_dev,
return ret;
}
-static struct attribute *attrs[] = {
+static struct attribute *uea_attrs[] = {
&dev_attr_stat_status.attr,
&dev_attr_stat_mflags.attr,
&dev_attr_stat_human_status.attr,
@@ -2479,9 +2479,7 @@ static struct attribute *attrs[] = {
&dev_attr_stat_firmid.attr,
NULL,
};
-static const struct attribute_group attr_grp = {
- .attrs = attrs,
-};
+ATTRIBUTE_GROUPS(uea);
static int uea_bind(struct usbatm_data *usbatm, struct usb_interface *intf,
const struct usb_device_id *id)
@@ -2550,18 +2548,12 @@ static int uea_bind(struct usbatm_data *usbatm, struct
usb_interface *intf,
}
}
- ret = sysfs_create_group(&intf->dev.kobj, &attr_grp);
- if (ret < 0)
- goto error;
-
ret = uea_boot(sc);
if (ret < 0)
- goto error_rm_grp;
+ goto error;
return 0;
-error_rm_grp:
- sysfs_remove_group(&intf->dev.kobj, &attr_grp);
error:
kfree(sc);
return ret;
@@ -2571,7 +2563,6 @@ static void uea_unbind(struct usbatm_data *usbatm, struct
usb_interface *intf)
{
struct uea_softc *sc = usbatm->driver_data;
- sysfs_remove_group(&intf->dev.kobj, &attr_grp);
uea_stop(sc);
kfree(sc);
}
@@ -2721,6 +2712,7 @@ static struct usb_driver uea_driver = {
.id_table = uea_ids,
.probe = uea_probe,
.disconnect = uea_disconnect,
+ .dev_groups = uea_groups,
};
MODULE_DEVICE_TABLE(usb, uea_ids);
--
2.22.0
[PATCH 05/12] USB: usblp: convert to use dev_groups
USB drivers now support the ability for the driver core to handle the
creation and removal of device-specific sysfs files in a race-free
manner. Take advantage of that by converting the driver to use this by
moving the sysfs attributes into a group and assigning the dev_groups
pointer to it.
Cc: Pete Zaitcev
Signed-off-by: Greg Kroah-Hartman
---
drivers/usb/class/usblp.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/class/usblp.c b/drivers/usb/class/usblp.c
index 407a7a6198a2..7fea4999d352 100644
--- a/drivers/usb/class/usblp.c
+++ b/drivers/usb/class/usblp.c
@@ -1082,6 +1082,12 @@ static ssize_t ieee1284_id_show(struct device *dev,
struct device_attribute *att
static DEVICE_ATTR_RO(ieee1284_id);
+static struct attribute *usblp_attrs[] = {
+ &dev_attr_ieee1284_id.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(usblp);
+
static int usblp_probe(struct usb_interface *intf,
const struct usb_device_id *id)
{
@@ -1156,9 +1162,6 @@ static int usblp_probe(struct usb_interface *intf,
/* Retrieve and store the device ID string. */
usblp_cache_device_id_string(usblp);
- retval = device_create_file(&intf->dev, &dev_attr_ieee1284_id);
- if (retval)
- goto abort_intfdata;
#ifdef DEBUG
usblp_check_status(usblp, 0);
@@ -1189,7 +1192,6 @@ static int usblp_probe(struct usb_interface *intf,
abort_intfdata:
usb_set_intfdata(intf, NULL);
- device_remove_file(&intf->dev, &dev_attr_ieee1284_id);
abort:
kfree(usblp->readbuf);
kfree(usblp->statusbuf);
@@ -1360,8 +1362,6 @@ static void usblp_disconnect(struct usb_interface *intf)
BUG();
}
- device_remove_file(&intf->dev, &dev_attr_ieee1284_id);
-
mutex_lock(&usblp_mutex);
mutex_lock(&usblp->mut);
usblp->present = 0;
@@ -1421,6 +1421,7 @@ static struct usb_driver usblp_driver = {
.suspend = usblp_suspend,
.resume = usblp_resume,
.id_table = usblp_ids,
+ .dev_groups = usblp_groups,
.supports_autosuspend = 1,
};
--
2.22.0
[PATCH 06/12] USB: usbtmc: convert to use dev_groups
USB drivers now support the ability for the driver core to handle the
creation and removal of device-specific sysfs files in a race-free
manner. Take advantage of that by converting the driver to use this by
moving the sysfs attributes into a group and assigning the dev_groups
pointer to it.
Cc: Guido Kiener
Cc: Steve Bayless
Signed-off-by: Greg Kroah-Hartman
---
drivers/usb/class/usbtmc.c | 13 +++--
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 4942122b2346..7ff831f2fd21 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -1836,17 +1836,14 @@ capability_attribute(device_capabilities);
capability_attribute(usb488_interface_capabilities);
capability_attribute(usb488_device_capabilities);
-static struct attribute *capability_attrs[] = {
+static struct attribute *usbtmc_attrs[] = {
&dev_attr_interface_capabilities.attr,
&dev_attr_device_capabilities.attr,
&dev_attr_usb488_interface_capabilities.attr,
&dev_attr_usb488_device_capabilities.attr,
NULL,
};
-
-static const struct attribute_group capability_attr_grp = {
- .attrs = capability_attrs,
-};
+ATTRIBUTE_GROUPS(usbtmc);
static int usbtmc_ioctl_indicator_pulse(struct usbtmc_device_data *data)
{
@@ -2383,9 +2380,6 @@ static int usbtmc_probe(struct usb_interface *intf,
retcode = get_capabilities(data);
if (retcode)
dev_err(&intf->dev, "can't read capabilities\n");
- else
- retcode = sysfs_create_group(&intf->dev.kobj,
-&capability_attr_grp);
if (data->iin_ep_present) {
/* allocate int urb */
@@ -2432,7 +2426,6 @@ static int usbtmc_probe(struct usb_interface *intf,
return 0;
error_register:
- sysfs_remove_group(&intf->dev.kobj, &capability_attr_grp);
usbtmc_free_int(data);
err_put:
kref_put(&data->kref, usbtmc_delete);
@@ -2445,7 +2438,6 @@ static void usbtmc_disconnect(struct usb_interface *intf)
struct list_head *elem;
usb_deregister_dev(intf, &usbtmc_class);
- sysfs_remove_group(&intf->dev.kobj, &capability_attr_grp);
mutex_lock(&data->io_mutex);
data->zombie = 1;
wake_up_interruptible_all(&data->waitq);
@@ -2551,6 +2543,7 @@ static struct usb_driver usbtmc_driver = {
.resume = usbtmc_resume,
.pre_reset = usbtmc_pre_reset,
.post_reset = usbtmc_post_reset,
+ .dev_groups = usbtmc_groups,
};
module_usb_driver(usbtmc_driver);
--
2.22.0
[PATCH 07/12] USB: cypress_cy7c63: convert to use dev_groups
USB drivers now support the ability for the driver core to handle the
creation and removal of device-specific sysfs files in a race-free
manner. Take advantage of that by converting the driver to use this by
moving the sysfs attributes into a group and assigning the dev_groups
pointer to it.
Signed-off-by: Greg Kroah-Hartman
---
drivers/usb/misc/cypress_cy7c63.c | 29 -
1 file changed, 8 insertions(+), 21 deletions(-)
diff --git a/drivers/usb/misc/cypress_cy7c63.c
b/drivers/usb/misc/cypress_cy7c63.c
index 9d780b77314b..14faec51d7a5 100644
--- a/drivers/usb/misc/cypress_cy7c63.c
+++ b/drivers/usb/misc/cypress_cy7c63.c
@@ -183,6 +183,7 @@ static ssize_t port0_show(struct device *dev,
{
return read_port(dev, attr, buf, 0, CYPRESS_READ_PORT_ID0);
}
+static DEVICE_ATTR_RW(port0);
/* attribute callback handler (read) */
static ssize_t port1_show(struct device *dev,
@@ -190,11 +191,14 @@ static ssize_t port1_show(struct device *dev,
{
return read_port(dev, attr, buf, 1, CYPRESS_READ_PORT_ID1);
}
-
-static DEVICE_ATTR_RW(port0);
-
static DEVICE_ATTR_RW(port1);
+static struct attribute *cypress_attrs[] = {
+ &dev_attr_port0.attr,
+ &dev_attr_port1.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(cypress);
static int cypress_probe(struct usb_interface *interface,
const struct usb_device_id *id)
@@ -212,26 +216,11 @@ static int cypress_probe(struct usb_interface *interface,
/* save our data pointer in this interface device */
usb_set_intfdata(interface, dev);
- /* create device attribute files */
- retval = device_create_file(&interface->dev, &dev_attr_port0);
- if (retval)
- goto error;
- retval = device_create_file(&interface->dev, &dev_attr_port1);
- if (retval)
- goto error;
-
/* let the user know that the device is now attached */
dev_info(&interface->dev,
"Cypress CY7C63xxx device now attached\n");
return 0;
-error:
- device_remove_file(&interface->dev, &dev_attr_port0);
- device_remove_file(&interface->dev, &dev_attr_port1);
- usb_set_intfdata(interface, NULL);
- usb_put_dev(dev->udev);
- kfree(dev);
-
error_mem:
return retval;
}
@@ -242,9 +231,6 @@ static void cypress_disconnect(struct usb_interface
*interface)
dev = usb_get_intfdata(interface);
- /* remove device attribute files */
- device_remove_file(&interface->dev, &dev_attr_port0);
- device_remove_file(&interface->dev, &dev_attr_port1);
/* the intfdata can be set to NULL only after the
* device files have been removed */
usb_set_intfdata(interface, NULL);
@@ -262,6 +248,7 @@ static struct usb_driver cypress_driver = {
.probe = cypress_probe,
.disconnect = cypress_disconnect,
.id_table = cypress_table,
+ .dev_groups = cypress_groups,
};
module_usb_driver(cypress_driver);
--
2.22.0
[PATCH 08/12] USB: cytherm: convert to use dev_groups
USB drivers now support the ability for the driver core to handle the
creation and removal of device-specific sysfs files in a race-free
manner. Take advantage of that by converting the driver to use this by
moving the sysfs attributes into a group and assigning the dev_groups
pointer to it.
Signed-off-by: Greg Kroah-Hartman
---
drivers/usb/misc/cytherm.c | 64 +++---
1 file changed, 19 insertions(+), 45 deletions(-)
diff --git a/drivers/usb/misc/cytherm.c b/drivers/usb/misc/cytherm.c
index 8b15ab5e1450..3e3802aaefa3 100644
--- a/drivers/usb/misc/cytherm.c
+++ b/drivers/usb/misc/cytherm.c
@@ -36,20 +36,6 @@ struct usb_cytherm {
};
-/* local function prototypes */
-static int cytherm_probe(struct usb_interface *interface,
-const struct usb_device_id *id);
-static void cytherm_disconnect(struct usb_interface *interface);
-
-
-/* usb specific object needed to register this driver with the usb subsystem */
-static struct usb_driver cytherm_driver = {
- .name = "cytherm",
- .probe =cytherm_probe,
- .disconnect = cytherm_disconnect,
- .id_table = id_table,
-};
-
/* Vendor requests */
/* They all operate on one byte at a time */
#define PING 0x00
@@ -304,6 +290,15 @@ static ssize_t port1_store(struct device *dev, struct
device_attribute *attr, co
}
static DEVICE_ATTR_RW(port1);
+static struct attribute *cytherm_attrs[] = {
+ &dev_attr_brightness.attr,
+ &dev_attr_temp.attr,
+ &dev_attr_button.attr,
+ &dev_attr_port0.attr,
+ &dev_attr_port1.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(cytherm);
static int cytherm_probe(struct usb_interface *interface,
const struct usb_device_id *id)
@@ -322,34 +317,10 @@ static int cytherm_probe(struct usb_interface *interface,
dev->brightness = 0xFF;
- retval = device_create_file(&interface->dev, &dev_attr_brightness);
- if (retval)
- goto error;
- retval = device_create_file(&interface->dev, &dev_attr_temp);
- if (retval)
- goto error;
- retval = device_create_file(&interface->dev, &dev_attr_button);
- if (retval)
- goto error;
- retval = device_create_file(&interface->dev, &dev_attr_port0);
- if (retval)
- goto error;
- retval = device_create_file(&interface->dev, &dev_attr_port1);
- if (retval)
- goto error;
-
dev_info (&interface->dev,
"Cypress thermometer device now attached\n");
return 0;
-error:
- device_remove_file(&interface->dev, &dev_attr_brightness);
- device_remove_file(&interface->dev, &dev_attr_temp);
- device_remove_file(&interface->dev, &dev_attr_button);
- device_remove_file(&interface->dev, &dev_attr_port0);
- device_remove_file(&interface->dev, &dev_attr_port1);
- usb_set_intfdata (interface, NULL);
- usb_put_dev(dev->udev);
- kfree(dev);
+
error_mem:
return retval;
}
@@ -360,12 +331,6 @@ static void cytherm_disconnect(struct usb_interface
*interface)
dev = usb_get_intfdata (interface);
- device_remove_file(&interface->dev, &dev_attr_brightness);
- device_remove_file(&interface->dev, &dev_attr_temp);
- device_remove_file(&interface->dev, &dev_attr_button);
- device_remove_file(&interface->dev, &dev_attr_port0);
- device_remove_file(&interface->dev, &dev_attr_port1);
-
/* first remove the files, then NULL the pointer */
usb_set_intfdata (interface, NULL);
@@ -376,6 +341,15 @@ static void cytherm_disconnect(struct usb_interface
*interface)
dev_info(&interface->dev, "Cypress thermometer now disconnected\n");
}
+/* usb specific object needed to register this driver with the usb subsystem */
+static struct usb_driver cytherm_driver = {
+ .name = "cytherm",
+ .probe =cytherm_probe,
+ .disconnect = cytherm_disconnect,
+ .id_table = id_table,
+ .dev_groups = cytherm_groups,
+};
+
module_usb_driver(cytherm_driver);
MODULE_AUTHOR(DRIVER_AUTHOR);
--
2.22.0
[PATCH 09/12] USB: lvstest: convert to use dev_groups
USB drivers now support the ability for the driver core to handle the
creation and removal of device-specific sysfs files in a race-free
manner. Take advantage of that by converting the driver to use this by
moving the sysfs attributes into a group and assigning the dev_groups
pointer to it.
Signed-off-by: Greg Kroah-Hartman
---
drivers/usb/misc/lvstest.c | 19 ---
1 file changed, 4 insertions(+), 15 deletions(-)
diff --git a/drivers/usb/misc/lvstest.c b/drivers/usb/misc/lvstest.c
index e5c03c6d16e9..407fe7570f3b 100644
--- a/drivers/usb/misc/lvstest.c
+++ b/drivers/usb/misc/lvstest.c
@@ -310,7 +310,7 @@ static ssize_t enable_compliance_store(struct device *dev,
}
static DEVICE_ATTR_WO(enable_compliance);
-static struct attribute *lvs_attributes[] = {
+static struct attribute *lvs_attrs[] = {
&dev_attr_get_dev_desc.attr,
&dev_attr_u1_timeout.attr,
&dev_attr_u2_timeout.attr,
@@ -321,10 +321,7 @@ static struct attribute *lvs_attributes[] = {
&dev_attr_enable_compliance.attr,
NULL
};
-
-static const struct attribute_group lvs_attr_group = {
- .attrs = lvs_attributes,
-};
+ATTRIBUTE_GROUPS(lvs);
static void lvs_rh_work(struct work_struct *work)
{
@@ -439,12 +436,6 @@ static int lvs_rh_probe(struct usb_interface *intf,
INIT_WORK(&lvs->rh_work, lvs_rh_work);
- ret = sysfs_create_group(&intf->dev.kobj, &lvs_attr_group);
- if (ret < 0) {
- dev_err(&intf->dev, "Failed to create sysfs node %d\n", ret);
- goto free_urb;
- }
-
pipe = usb_rcvintpipe(hdev, endpoint->bEndpointAddress);
maxp = usb_maxpacket(hdev, pipe, usb_pipeout(pipe));
usb_fill_int_urb(lvs->urb, hdev, pipe, &lvs->buffer[0], maxp,
@@ -453,13 +444,11 @@ static int lvs_rh_probe(struct usb_interface *intf,
ret = usb_submit_urb(lvs->urb, GFP_KERNEL);
if (ret < 0) {
dev_err(&intf->dev, "couldn't submit lvs urb %d\n", ret);
- goto sysfs_remove;
+ goto free_urb;
}
return ret;
-sysfs_remove:
- sysfs_remove_group(&intf->dev.kobj, &lvs_attr_group);
free_urb:
usb_free_urb(lvs->urb);
return ret;
@@ -469,7 +458,6 @@ static void lvs_rh_disconnect(struct usb_interface *intf)
{
struct lvs_rh *lvs = usb_get_intfdata(intf);
- sysfs_remove_group(&intf->dev.kobj, &lvs_attr_group);
usb_poison_urb(lvs->urb); /* used in scheduled work */
flush_work(&lvs->rh_work);
usb_free_urb(lvs->urb);
@@ -479,6 +467,7 @@ static struct usb_driver lvs_driver = {
.name = "lvs",
.probe =lvs_rh_probe,
.disconnect = lvs_rh_disconnect,
+ .dev_groups = lvs_groups,
};
module_usb_driver(lvs_driver);
--
2.22.0
Re: KASAN: use-after-free Read in device_release_driver_internal
On Tue, Aug 6, 2019 at 2:36 PM Oliver Neukum wrote:
>
> Am Donnerstag, den 01.08.2019, 14:47 -0400 schrieb Alan Stern:
> >
> > I think this must be caused by an unbalanced refcount. That is,
> > something must drop one more reference to the device than it takes.
> > That would explain why the invalid access occurs inside a single
> > bus_remove_device() call, between the klist_del() and
> > device_release_driver().
> >
> > The kernel log indicates that the device was probed by rndis_wlan,
> > rndis_host, and cdc_acm, all of which got errors because of the
> > device's bogus descriptors. Probably one of them is messing up the
> > refcount.
>
> Hi,
>
> you made me look at cdc-acm. I suspect
>
> cae2bc768d176bfbdad7035bbcc3cdc973eb7984 ("usb: cdc-acm: Decrement tty port's
> refcount if probe() fail")
>
> is buggy decrementing the refcount on the interface in destroy()
> even before the refcount is increased.
>
> Unfortunately I cannot tell from the bug report how many and which
> interfaces the emulated test device has. Hence it is unclear to me,
> when exactly probe() would fail cdc-acm.
>
> If you agree. I am attaching a putative fix.
Let's see if it fixes the issue.
#syz fix: https://github.com/google/kasan.git 6a3599ce
>
> Regards
> Oliver
From 6b31904e6cf75f89441e308b9e428a1de7728fd8 Mon Sep 17 00:00:00 2001
From: Oliver Neukum
Date: Tue, 6 Aug 2019 14:34:27 +0200
Subject: [PATCH] usb: cdc-acm: make sure a refcount is taken early enough
destroy() will decrement the refcount on the interface, so that
it needs to be taken so early that it never undercounts.
Signed-off-by: Oliver Neukum
---
drivers/usb/class/cdc-acm.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 183b41753c98..28e3de775ada 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -1301,10 +1301,6 @@ static int acm_probe(struct usb_interface *intf,
tty_port_init(&acm->port);
acm->port.ops = &acm_port_ops;
- minor = acm_alloc_minor(acm);
- if (minor < 0)
- goto alloc_fail1;
-
ctrlsize = usb_endpoint_maxp(epctrl);
readsize = usb_endpoint_maxp(epread) *
(quirks == SINGLE_RX_URB ? 1 : 2);
@@ -1312,6 +1308,13 @@ static int acm_probe(struct usb_interface *intf,
acm->writesize = usb_endpoint_maxp(epwrite) * 20;
acm->control = control_interface;
acm->data = data_interface;
+
+ usb_get_intf(acm->control); /* undone in destroy() */
+
+ minor = acm_alloc_minor(acm);
+ if (minor < 0)
+ goto alloc_fail1;
+
acm->minor = minor;
acm->dev = usb_dev;
if (h.usb_cdc_acm_descriptor)
@@ -1458,7 +1461,6 @@ static int acm_probe(struct usb_interface *intf,
usb_driver_claim_interface(&acm_driver, data_interface, acm);
usb_set_intfdata(data_interface, acm);
- usb_get_intf(control_interface);
tty_dev = tty_port_register_device(&acm->port, acm_tty_driver, minor,
&control_interface->dev);
if (IS_ERR(tty_dev)) {
--
2.16.4
Re: [PATCH v4 1/2] usbip: Skip DMA mapping and unmapping for urb at vhci
On 8/6/19 6:31 AM, Suwan Kim wrote:
vhci doesn’t do DMA for remote device. Actually, the real DMA
operation is done by network card driver. vhci just passes virtual
address of the buffer to the network stack, so vhci doesn’t use and
need dma address of the buffer of the URB.
But HCD provides DMA mapping and unmapping function by default.
Moreover, it causes unnecessary DMA mapping and unmapping which
will be done again at the NIC driver and it wastes CPU cycles.
So, implement map_urb_for_dma and unmap_urb_for_dma function for
vhci in order to skip the DMA mapping and unmapping procedure.
When it comes to supporting SG for vhci, it is useful to use native
SG list (urb->num_sgs) instead of mapped SG list because DMA mapping
fnuction can adjust the number of SG list (urb->num_mapped_sgs).
And vhci_map_urb_for_dma() prevents isoc pipe from using SG as
hcd_map_urb_for_dma() does.
Signed-off-by: Suwan Kim
---
v3 - v4:
- Replace WARN_ON() with pr_err() in the error path.
v2 - v3
- Move setting URB_DMA_MAP_SG flag to the patch 2.
- Prevent isoc pipe from using SG buffer.
v1 - v2
- Add setting URB_DMA_MAP_SG flag in urb->transfer_flags to tell
stub driver to use SG buffer.
---
drivers/usb/usbip/vhci_hcd.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
index 000ab7225717..429e4e989f38 100644
--- a/drivers/usb/usbip/vhci_hcd.c
+++ b/drivers/usb/usbip/vhci_hcd.c
@@ -1288,6 +1288,22 @@ static int vhci_free_streams(struct usb_hcd *hcd, struct
usb_device *udev,
return 0;
}
+static int vhci_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb,
+ gfp_t mem_flags)
+{
+ if (usb_endpoint_xfer_isoc(&urb->ep->desc) && urb->num_sgs) {
+ pr_err("SG is not supported for isochronous transfer\n");
Any reason to not use dev_err()?
Looks good otherwise.
thanks,
-- Shuah
Re: [PATCH v4 2/2] usbip: Implement SG support to vhci-hcd and stub driver
On 8/6/19 6:31 AM, Suwan Kim wrote: There are bugs on vhci with usb 3.0 storage device. In USB, each SG list entry buffer should be divisible by the bulk max packet size. But with native SG support, this problem doesn't matter because the SG buffer is treated as contiguous buffer. But without native SG support, USB storage driver breaks SG list into several URBs and the error occurs because of a buffer size of URB that cannot be divided by the bulk max packet size. The error situation is as follows. When USB Storage driver requests 31.5 KB data and has SG list which has 3584 bytes buffer followed by 7 4096 bytes buffer for some reason. USB Storage driver splits this SG list into several URBs because VHCI doesn't support SG and sends them separately. So the first URB buffer size is 3584 bytes. When receiving data from device, USB 3.0 device sends data packet of 1024 bytes size because the max packet size of BULK pipe is 1024 bytes. So device sends 4096 bytes. But the first URB buffer has only 3584 bytes buffer size. So host controller terminates the transfer even though there is more data to receive. So, vhci needs to support SG transfer to prevent this error. In this patch, vhci supports SG regardless of whether the server's host controller supports SG or not, because stub driver splits SG list into several URBs if the server's host controller doesn't support SG. To support SG, vhci_map_urb_for_dma() sets URB_DMA_MAP_SG flag in urb->transfer_flags if URB has SG list and this flag will tell stub driver to use SG list. vhci sends each SG list entry to stub driver. Then, stub driver sees the total length of the buffer and allocates SG table and pages according to the total buffer length calling sgl_alloc(). After stub driver receives completed URB, it again sends each SG list entry to vhci. If the server's host controller doesn't support SG, stub driver breaks a single SG request into several URBs and submits them to the server's host controller. When all the split URBs are completed, stub driver reassembles the URBs into a single return command and sends it to vhci. Moreover, in the situation where vhci supports SG, but stub driver does not, or vice versa, usbip works normally. Because there is no protocol modification, there is no problem in communication between server and client even if the one has a kernel without SG support. In the case of vhci supports SG and stub driver doesn't, because vhci sends only the total length of the buffer to stub driver as it did before the patch applied, stub driver only needs to allocate the required length of buffers regardless of whether vhci supports SG or not. If stub driver needs to send data buffer to vhci because of IN pipe, stub driver also sends only total length of buffer as metadata and then sends real data as vhci does. Then vhci receive data from stub driver and store it to the corresponding buffer of SG list entry. In the case of stub driver that supports SG, buffer is allocated by sgl_alloc(). However, stub driver that does not support SG allocates buffer using only kmalloc(). Therefore, if vhci supports SG and stub driver doesn't, stub driver has to allocate buffer with kmalloc() as much as the total length of SG buffer which is quite huge when vhci sends SG request, so it has big overhead in buffer allocation. And for the case of stub driver supports SG and vhci doesn't, since the USB storage driver checks that vhci doesn't support SG and sends the request to stub driver by splitting the SG list into multiple URBs, stub driver allocates a buffer with kmalloc() as it did before this patch. VUDC also works well with this patch. Tests are done with two USB gadget created by CONFIGFS USB gadget. Both use the BULK pipe. 1. Serial gadget 2. Mass storage gadget * Serial gadget test Serial gadget on the host sends and receives data using cat command on the /dev/ttyGS. The client uses minicom to communicate with the serial gadget. * Mass storage gadget test After connecting the gadget with vhci, use "dd" to test read and write operation on the client side. Read - dd if=/dev/sd iflag=direct of=/dev/null bs=1G count=1 Write - dd if= iflag=direct of=/dev/sd bs=1G count=1 Thanks for the test results. Were you able to test with USB lowspeed devices? thanks, -- Shuah
Re: [PATCH v4 1/2] usbip: Skip DMA mapping and unmapping for urb at vhci
On Tue, Aug 06, 2019 at 09:11:30AM -0600, shuah wrote:
> On 8/6/19 6:31 AM, Suwan Kim wrote:
> > vhci doesn’t do DMA for remote device. Actually, the real DMA
> > operation is done by network card driver. vhci just passes virtual
> > address of the buffer to the network stack, so vhci doesn’t use and
> > need dma address of the buffer of the URB.
> >
> > But HCD provides DMA mapping and unmapping function by default.
> > Moreover, it causes unnecessary DMA mapping and unmapping which
> > will be done again at the NIC driver and it wastes CPU cycles.
> > So, implement map_urb_for_dma and unmap_urb_for_dma function for
> > vhci in order to skip the DMA mapping and unmapping procedure.
> >
> > When it comes to supporting SG for vhci, it is useful to use native
> > SG list (urb->num_sgs) instead of mapped SG list because DMA mapping
> > fnuction can adjust the number of SG list (urb->num_mapped_sgs).
> > And vhci_map_urb_for_dma() prevents isoc pipe from using SG as
> > hcd_map_urb_for_dma() does.
> >
> > Signed-off-by: Suwan Kim
> > ---
> > v3 - v4:
> > - Replace WARN_ON() with pr_err() in the error path.
> >
> > v2 - v3
> > - Move setting URB_DMA_MAP_SG flag to the patch 2.
> > - Prevent isoc pipe from using SG buffer.
> >
> > v1 - v2
> > - Add setting URB_DMA_MAP_SG flag in urb->transfer_flags to tell
> > stub driver to use SG buffer.
> > ---
> > drivers/usb/usbip/vhci_hcd.c | 19 +++
> > 1 file changed, 19 insertions(+)
> >
> > diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
> > index 000ab7225717..429e4e989f38 100644
> > --- a/drivers/usb/usbip/vhci_hcd.c
> > +++ b/drivers/usb/usbip/vhci_hcd.c
> > @@ -1288,6 +1288,22 @@ static int vhci_free_streams(struct usb_hcd *hcd,
> > struct usb_device *udev,
> > return 0;
> > }
> > +static int vhci_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb,
> > + gfp_t mem_flags)
> > +{
> > + if (usb_endpoint_xfer_isoc(&urb->ep->desc) && urb->num_sgs) {
> > + pr_err("SG is not supported for isochronous transfer\n");
>
> Any reason to not use dev_err()?
Because some codes in vhci_hcd.c use pr_err().There is no other
reason. However, dev_err() seems more appropriate than pr_err().
I will replace pr_err() with dev_err(urb->dev->dev, "SG is ...")
Is it ok?
Regards
Suwan Kim
Re: KASAN: use-after-free Read in device_release_driver_internal
Am Dienstag, den 06.08.2019, 10:19 -0400 schrieb Alan Stern: > In any case, I don't know if this missing "get" would cause the > problem, but it might well. Hi, upon further thought, this should be automated. Checking for refcount leaks is KASAN's job. In particular, refcounts should not * decrease in probe() * increase in disconnect() * change in case probe() fails Regards Oliver
Re: [PATCH v4 1/2] usbip: Skip DMA mapping and unmapping for urb at vhci
On 8/6/19 9:32 AM, Suwan Kim wrote:
On Tue, Aug 06, 2019 at 09:11:30AM -0600, shuah wrote:
On 8/6/19 6:31 AM, Suwan Kim wrote:
vhci doesn’t do DMA for remote device. Actually, the real DMA
operation is done by network card driver. vhci just passes virtual
address of the buffer to the network stack, so vhci doesn’t use and
need dma address of the buffer of the URB.
But HCD provides DMA mapping and unmapping function by default.
Moreover, it causes unnecessary DMA mapping and unmapping which
will be done again at the NIC driver and it wastes CPU cycles.
So, implement map_urb_for_dma and unmap_urb_for_dma function for
vhci in order to skip the DMA mapping and unmapping procedure.
When it comes to supporting SG for vhci, it is useful to use native
SG list (urb->num_sgs) instead of mapped SG list because DMA mapping
fnuction can adjust the number of SG list (urb->num_mapped_sgs).
And vhci_map_urb_for_dma() prevents isoc pipe from using SG as
hcd_map_urb_for_dma() does.
Signed-off-by: Suwan Kim
---
v3 - v4:
- Replace WARN_ON() with pr_err() in the error path.
v2 - v3
- Move setting URB_DMA_MAP_SG flag to the patch 2.
- Prevent isoc pipe from using SG buffer.
v1 - v2
- Add setting URB_DMA_MAP_SG flag in urb->transfer_flags to tell
stub driver to use SG buffer.
---
drivers/usb/usbip/vhci_hcd.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
index 000ab7225717..429e4e989f38 100644
--- a/drivers/usb/usbip/vhci_hcd.c
+++ b/drivers/usb/usbip/vhci_hcd.c
@@ -1288,6 +1288,22 @@ static int vhci_free_streams(struct usb_hcd *hcd, struct
usb_device *udev,
return 0;
}
+static int vhci_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb,
+ gfp_t mem_flags)
+{
+ if (usb_endpoint_xfer_isoc(&urb->ep->desc) && urb->num_sgs) {
+ pr_err("SG is not supported for isochronous transfer\n");
Any reason to not use dev_err()?
Because some codes in vhci_hcd.c use pr_err().There is no other
reason. However, dev_err() seems more appropriate than pr_err().
I will replace pr_err() with dev_err(urb->dev->dev, "SG is ...")
Is it ok?
Please. This way we will have the device information. pr_err()
won't us that. In general I prefer dev_* if dev is available in
the code path, which is the case here.
thanks,
-- Shuah
Re: [PATCH v4 2/2] usbip: Implement SG support to vhci-hcd and stub driver
On Tue, Aug 06, 2019 at 09:13:54AM -0600, shuah wrote: > On 8/6/19 6:31 AM, Suwan Kim wrote: > > There are bugs on vhci with usb 3.0 storage device. In USB, each SG > > list entry buffer should be divisible by the bulk max packet size. > > But with native SG support, this problem doesn't matter because the > > SG buffer is treated as contiguous buffer. But without native SG > > support, USB storage driver breaks SG list into several URBs and the > > error occurs because of a buffer size of URB that cannot be divided > > by the bulk max packet size. The error situation is as follows. > > > > When USB Storage driver requests 31.5 KB data and has SG list which > > has 3584 bytes buffer followed by 7 4096 bytes buffer for some > > reason. USB Storage driver splits this SG list into several URBs > > because VHCI doesn't support SG and sends them separately. So the > > first URB buffer size is 3584 bytes. When receiving data from device, > > USB 3.0 device sends data packet of 1024 bytes size because the max > > packet size of BULK pipe is 1024 bytes. So device sends 4096 bytes. > > But the first URB buffer has only 3584 bytes buffer size. So host > > controller terminates the transfer even though there is more data to > > receive. So, vhci needs to support SG transfer to prevent this error. > > > > In this patch, vhci supports SG regardless of whether the server's > > host controller supports SG or not, because stub driver splits SG > > list into several URBs if the server's host controller doesn't > > support SG. > > > > To support SG, vhci_map_urb_for_dma() sets URB_DMA_MAP_SG flag in > > urb->transfer_flags if URB has SG list and this flag will tell stub > > driver to use SG list. > > > > vhci sends each SG list entry to stub driver. Then, stub driver sees > > the total length of the buffer and allocates SG table and pages > > according to the total buffer length calling sgl_alloc(). After stub > > driver receives completed URB, it again sends each SG list entry to > > vhci. > > > > If the server's host controller doesn't support SG, stub driver > > breaks a single SG request into several URBs and submits them to > > the server's host controller. When all the split URBs are completed, > > stub driver reassembles the URBs into a single return command and > > sends it to vhci. > > > > Moreover, in the situation where vhci supports SG, but stub driver > > does not, or vice versa, usbip works normally. Because there is no > > protocol modification, there is no problem in communication between > > server and client even if the one has a kernel without SG support. > > > > In the case of vhci supports SG and stub driver doesn't, because > > vhci sends only the total length of the buffer to stub driver as > > it did before the patch applied, stub driver only needs to allocate > > the required length of buffers regardless of whether vhci supports > > SG or not. > > > > If stub driver needs to send data buffer to vhci because of IN pipe, > > stub driver also sends only total length of buffer as metadata and > > then sends real data as vhci does. Then vhci receive data from stub > > driver and store it to the corresponding buffer of SG list entry. > > > > In the case of stub driver that supports SG, buffer is allocated by > > sgl_alloc(). However, stub driver that does not support SG allocates > > buffer using only kmalloc(). Therefore, if vhci supports SG and stub > > driver doesn't, stub driver has to allocate buffer with kmalloc() as > > much as the total length of SG buffer which is quite huge when vhci > > sends SG request, so it has big overhead in buffer allocation. > > > > And for the case of stub driver supports SG and vhci doesn't, since > > the USB storage driver checks that vhci doesn't support SG and sends > > the request to stub driver by splitting the SG list into multiple > > URBs, stub driver allocates a buffer with kmalloc() as it did before > > this patch. > > > > VUDC also works well with this patch. Tests are done with two USB > > gadget created by CONFIGFS USB gadget. Both use the BULK pipe. > > > > 1. Serial gadget > > 2. Mass storage gadget > > > > * Serial gadget test > > > > Serial gadget on the host sends and receives data using cat command > > on the /dev/ttyGS. The client uses minicom to communicate with > > the serial gadget. > > > > * Mass storage gadget test > > > > After connecting the gadget with vhci, use "dd" to test read and > > write operation on the client side. > > > > Read - dd if=/dev/sd iflag=direct of=/dev/null bs=1G count=1 > > Write - dd if= iflag=direct of=/dev/sd bs=1G count=1 > > > > Thanks for the test results. > > Were you able to test with USB lowspeed devices? I tested USB3 super-speed device and USB2 high-speed device. But I don't have full/low speed device that uses BULK transfer. In USB spec, low-speed device doesn't support BULK transfer. Do I add test description about the USB3 super-speed and USB2 high-speed device t
Re: [PATCH v4 1/2] usbip: Skip DMA mapping and unmapping for urb at vhci
On Tue, Aug 06, 2019 at 09:38:54AM -0600, shuah wrote:
> On 8/6/19 9:32 AM, Suwan Kim wrote:
> > On Tue, Aug 06, 2019 at 09:11:30AM -0600, shuah wrote:
> > > On 8/6/19 6:31 AM, Suwan Kim wrote:
> > > > vhci doesn’t do DMA for remote device. Actually, the real DMA
> > > > operation is done by network card driver. vhci just passes virtual
> > > > address of the buffer to the network stack, so vhci doesn’t use and
> > > > need dma address of the buffer of the URB.
> > > >
> > > > But HCD provides DMA mapping and unmapping function by default.
> > > > Moreover, it causes unnecessary DMA mapping and unmapping which
> > > > will be done again at the NIC driver and it wastes CPU cycles.
> > > > So, implement map_urb_for_dma and unmap_urb_for_dma function for
> > > > vhci in order to skip the DMA mapping and unmapping procedure.
> > > >
> > > > When it comes to supporting SG for vhci, it is useful to use native
> > > > SG list (urb->num_sgs) instead of mapped SG list because DMA mapping
> > > > fnuction can adjust the number of SG list (urb->num_mapped_sgs).
> > > > And vhci_map_urb_for_dma() prevents isoc pipe from using SG as
> > > > hcd_map_urb_for_dma() does.
> > > >
> > > > Signed-off-by: Suwan Kim
> > > > ---
> > > > v3 - v4:
> > > > - Replace WARN_ON() with pr_err() in the error path.
> > > >
> > > > v2 - v3
> > > > - Move setting URB_DMA_MAP_SG flag to the patch 2.
> > > > - Prevent isoc pipe from using SG buffer.
> > > >
> > > > v1 - v2
> > > > - Add setting URB_DMA_MAP_SG flag in urb->transfer_flags to tell
> > > > stub driver to use SG buffer.
> > > > ---
> > > >drivers/usb/usbip/vhci_hcd.c | 19 +++
> > > >1 file changed, 19 insertions(+)
> > > >
> > > > diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
> > > > index 000ab7225717..429e4e989f38 100644
> > > > --- a/drivers/usb/usbip/vhci_hcd.c
> > > > +++ b/drivers/usb/usbip/vhci_hcd.c
> > > > @@ -1288,6 +1288,22 @@ static int vhci_free_streams(struct usb_hcd
> > > > *hcd, struct usb_device *udev,
> > > > return 0;
> > > >}
> > > > +static int vhci_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb,
> > > > + gfp_t mem_flags)
> > > > +{
> > > > + if (usb_endpoint_xfer_isoc(&urb->ep->desc) && urb->num_sgs) {
> > > > + pr_err("SG is not supported for isochronous
> > > > transfer\n");
> > >
> > > Any reason to not use dev_err()?
> >
> > Because some codes in vhci_hcd.c use pr_err().There is no other
> > reason. However, dev_err() seems more appropriate than pr_err().
> > I will replace pr_err() with dev_err(urb->dev->dev, "SG is ...")
> > Is it ok?
> >
>
> Please. This way we will have the device information. pr_err()
> won't us that. In general I prefer dev_* if dev is available in
> the code path, which is the case here.
Ok. I will resend v5.
Regards
Suwan Kim
Re: [PATCH v4 2/2] usbip: Implement SG support to vhci-hcd and stub driver
On 8/6/19 9:48 AM, Suwan Kim wrote: On Tue, Aug 06, 2019 at 09:13:54AM -0600, shuah wrote: On 8/6/19 6:31 AM, Suwan Kim wrote: There are bugs on vhci with usb 3.0 storage device. In USB, each SG list entry buffer should be divisible by the bulk max packet size. But with native SG support, this problem doesn't matter because the SG buffer is treated as contiguous buffer. But without native SG support, USB storage driver breaks SG list into several URBs and the error occurs because of a buffer size of URB that cannot be divided by the bulk max packet size. The error situation is as follows. When USB Storage driver requests 31.5 KB data and has SG list which has 3584 bytes buffer followed by 7 4096 bytes buffer for some reason. USB Storage driver splits this SG list into several URBs because VHCI doesn't support SG and sends them separately. So the first URB buffer size is 3584 bytes. When receiving data from device, USB 3.0 device sends data packet of 1024 bytes size because the max packet size of BULK pipe is 1024 bytes. So device sends 4096 bytes. But the first URB buffer has only 3584 bytes buffer size. So host controller terminates the transfer even though there is more data to receive. So, vhci needs to support SG transfer to prevent this error. In this patch, vhci supports SG regardless of whether the server's host controller supports SG or not, because stub driver splits SG list into several URBs if the server's host controller doesn't support SG. To support SG, vhci_map_urb_for_dma() sets URB_DMA_MAP_SG flag in urb->transfer_flags if URB has SG list and this flag will tell stub driver to use SG list. vhci sends each SG list entry to stub driver. Then, stub driver sees the total length of the buffer and allocates SG table and pages according to the total buffer length calling sgl_alloc(). After stub driver receives completed URB, it again sends each SG list entry to vhci. If the server's host controller doesn't support SG, stub driver breaks a single SG request into several URBs and submits them to the server's host controller. When all the split URBs are completed, stub driver reassembles the URBs into a single return command and sends it to vhci. Moreover, in the situation where vhci supports SG, but stub driver does not, or vice versa, usbip works normally. Because there is no protocol modification, there is no problem in communication between server and client even if the one has a kernel without SG support. In the case of vhci supports SG and stub driver doesn't, because vhci sends only the total length of the buffer to stub driver as it did before the patch applied, stub driver only needs to allocate the required length of buffers regardless of whether vhci supports SG or not. If stub driver needs to send data buffer to vhci because of IN pipe, stub driver also sends only total length of buffer as metadata and then sends real data as vhci does. Then vhci receive data from stub driver and store it to the corresponding buffer of SG list entry. In the case of stub driver that supports SG, buffer is allocated by sgl_alloc(). However, stub driver that does not support SG allocates buffer using only kmalloc(). Therefore, if vhci supports SG and stub driver doesn't, stub driver has to allocate buffer with kmalloc() as much as the total length of SG buffer which is quite huge when vhci sends SG request, so it has big overhead in buffer allocation. And for the case of stub driver supports SG and vhci doesn't, since the USB storage driver checks that vhci doesn't support SG and sends the request to stub driver by splitting the SG list into multiple URBs, stub driver allocates a buffer with kmalloc() as it did before this patch. VUDC also works well with this patch. Tests are done with two USB gadget created by CONFIGFS USB gadget. Both use the BULK pipe. 1. Serial gadget 2. Mass storage gadget * Serial gadget test Serial gadget on the host sends and receives data using cat command on the /dev/ttyGS. The client uses minicom to communicate with the serial gadget. * Mass storage gadget test After connecting the gadget with vhci, use "dd" to test read and write operation on the client side. Read - dd if=/dev/sd iflag=direct of=/dev/null bs=1G count=1 Write - dd if= iflag=direct of=/dev/sd bs=1G count=1 Thanks for the test results. Were you able to test with USB lowspeed devices? I tested USB3 super-speed device and USB2 high-speed device. But I don't have full/low speed device that uses BULK transfer. In USB spec, low-speed device doesn't support BULK transfer. I think I mentioned earlier that the reason to test with lowspeed is make sure there are no regressions due this change. You aren't testing BULK transfer, you are looking for any regressions. Do I add test description about the USB3 super-speed and USB2 high-speed device to the commit log? Please do. thanks, -- Shuah
Re: possible deadlock in open_rio
On Thu, 1 Aug 2019, syzbot wrote: > Hello, > > syzbot found the following crash on: > > HEAD commit:7f7867ff usb-fuzzer: main usb gadget fuzzer driver > git tree: https://github.com/google/kasan.git usb-fuzzer > console output: https://syzkaller.appspot.com/x/log.txt?x=136b6aec60 > kernel config: https://syzkaller.appspot.com/x/.config?x=792eb47789f57810 > dashboard link: https://syzkaller.appspot.com/bug?extid=7bbcbe9c9ff0cd49592a > compiler: gcc (GCC) 9.0.0 20181231 (experimental) > > Unfortunately, I don't have any reproducer for this crash yet. > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > Reported-by: syzbot+7bbcbe9c9ff0cd495...@syzkaller.appspotmail.com > > == > WARNING: possible circular locking dependency detected > 5.3.0-rc2+ #23 Not tainted > -- Andrey: This should be completely reproducible, since it's a simple ABBA locking violation. Maybe just introducing a time delay (to avoid races and give the open() call time to run) between the gadget creation and gadget removal would be enough to do it. Is there any way you can test this? Alan Stern
[PATCH] usbfs: Add ioctls for runtime power management
It has been requested that usbfs should implement runtime power
management, instead of forcing the device to remain at full power as
long as the device file is open. This patch introduces that new
feature.
It does so by adding three new usbfs ioctls:
USBDEVFS_FORBID_SUSPEND: Prevents the device from going into
runtime suspend (and causes a resume if the device is already
suspended).
USBDEVFS_ALLOW_SUSPEND: Allows the device to go into runtime
suspend. Some time may elapse before the device actually is
suspended, depending on things like the autosuspend delay.
USBDEVFS_WAIT_FOR_RESUME: Blocks until the call is interrupted
by a signal or at least one runtime resume has occurred since
the most recent ALLOW_SUSPEND ioctl call (which may mean
immediately, even if the device is currently suspended). In
the latter case, the device is prevented from suspending again
just as if FORBID_SUSPEND was called before the ioctl returns.
For backward compatibility, when the device file is first opened
runtime suspends are forbidden. The userspace program can then allow
suspends whenever it wants, and either resume the device directly (by
forbidding suspends again) or wait for a resume from some other source
(such as a remote wakeup). URBs submitted to a suspended device will
fail or will complete with an appropriate error code.
This combination of ioctls is sufficient for user programs to have
nearly the same degree of control over a device's runtime power
behavior as kernel drivers do.
Still lacking is documentation for the new ioctls. I intend to add it
later, after the existing documentation for the usbfs userspace API is
straightened out into a reasonable form.
Suggested-by: Mayuresh Kulkarni
Signed-off-by: Alan Stern
---
[as1905]
drivers/usb/core/devio.c | 96 --
drivers/usb/core/generic.c|5 +
drivers/usb/core/usb.h|3 +
include/uapi/linux/usbdevice_fs.h |3 +
4 files changed, 102 insertions(+), 5 deletions(-)
Index: usb-devel/drivers/usb/core/devio.c
===
--- usb-devel.orig/drivers/usb/core/devio.c
+++ usb-devel/drivers/usb/core/devio.c
@@ -48,6 +48,9 @@
#define USB_DEVICE_MAX (USB_MAXBUS * 128)
#define USB_SG_SIZE16384 /* split-size for large txs */
+/* Mutual exclusion for ps->list in resume vs. release and remove */
+static DEFINE_MUTEX(usbfs_mutex);
+
struct usb_dev_state {
struct list_head list; /* state list */
struct usb_device *dev;
@@ -57,14 +60,17 @@ struct usb_dev_state {
struct list_head async_completed;
struct list_head memory_list;
wait_queue_head_t wait; /* wake up if a request completed */
+ wait_queue_head_t wait_for_resume; /* wake up upon runtime resume */
unsigned int discsignr;
struct pid *disc_pid;
const struct cred *cred;
sigval_t disccontext;
unsigned long ifclaimed;
u32 disabled_bulk_eps;
- bool privileges_dropped;
unsigned long interface_allowed_mask;
+ int not_yet_resumed;
+ bool suspend_allowed;
+ bool privileges_dropped;
};
struct usb_memory {
@@ -694,9 +700,7 @@ static void driver_disconnect(struct usb
destroy_async_on_interface(ps, ifnum);
}
-/* The following routines are merely placeholders. There is no way
- * to inform a user task about suspend or resumes.
- */
+/* We don't care about suspend/resume of claimed interfaces */
static int driver_suspend(struct usb_interface *intf, pm_message_t msg)
{
return 0;
@@ -707,12 +711,32 @@ static int driver_resume(struct usb_inte
return 0;
}
+/* The following routines apply to the entire device, not interfaces */
+void usbfs_notify_suspend(struct usb_device *udev)
+{
+ /* We don't need to handle this */
+}
+
+void usbfs_notify_resume(struct usb_device *udev)
+{
+ struct usb_dev_state *ps;
+
+ /* Protect against simultaneous remove or release */
+ mutex_lock(&usbfs_mutex);
+ list_for_each_entry(ps, &udev->filelist, list) {
+ WRITE_ONCE(ps->not_yet_resumed, 0);
+ wake_up_all(&ps->wait_for_resume);
+ }
+ mutex_unlock(&usbfs_mutex);
+}
+
struct usb_driver usbfs_driver = {
.name = "usbfs",
.probe =driver_probe,
.disconnect = driver_disconnect,
.suspend = driver_suspend,
.resume = driver_resume,
+ .supports_autosuspend = 1,
};
static int claimintf(struct usb_dev_state *ps, unsigned int ifnum)
@@ -997,9 +1021,12 @@ static int usbdev_open(struct inode *ino
INIT_LIST_HEAD(&ps->async_completed);
INIT_LIST_HEAD(&ps->memory_list);
init_waitqueue_head(&ps->wait);
+ init_waitqueue_head(&ps->wait_for_resume);
Re: [PATCH net-next 2/5] r8152: replace array with linking list for rx information
On Tue, 6 Aug 2019 19:18:01 +0800, Hayes Wang wrote:
> The original method uses an array to store the rx information. The
> new one uses a list to link each rx structure. Then, it is possible
> to increase/decrease the number of rx structure dynamically.
>
> Signed-off-by: Hayes Wang
> ---
> drivers/net/usb/r8152.c | 187
> 1 file changed, 132 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index 0f07ed05ab34..44d832ceb516 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -22,6 +22,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> /* Information for net-next */
> @@ -694,7 +695,7 @@ struct tx_desc {
> struct r8152;
>
> struct rx_agg {
> - struct list_head list;
> + struct list_head list, info_list;
> struct urb *urb;
> struct r8152 *context;
> void *buffer;
> @@ -719,7 +720,7 @@ struct r8152 {
> struct net_device *netdev;
> struct urb *intr_urb;
> struct tx_agg tx_info[RTL8152_MAX_TX];
> - struct rx_agg rx_info[RTL8152_MAX_RX];
> + struct list_head rx_info;
> struct list_head rx_done, tx_free;
> struct sk_buff_head tx_queue, rx_queue;
> spinlock_t rx_lock, tx_lock;
> @@ -744,6 +745,8 @@ struct r8152 {
> void (*autosuspend_en)(struct r8152 *tp, bool enable);
> } rtl_ops;
>
> + atomic_t rx_count;
I wonder what the advantage of rx_count being atomic is, perhpas it
could be protected by the same lock as the list head?
> int intr_interval;
> u32 saved_wolopts;
> u32 msg_enable;
> @@ -1468,19 +1471,86 @@ static inline void *tx_agg_align(void *data)
> return (void *)ALIGN((uintptr_t)data, TX_ALIGN);
> }
>
> +static void free_rx_agg(struct r8152 *tp, struct rx_agg *agg)
> +{
> + list_del(&agg->info_list);
> +
> + usb_free_urb(agg->urb);
> + kfree(agg->buffer);
> + kfree(agg);
> +
> + atomic_dec(&tp->rx_count);
> +}
> +
> +static struct rx_agg *alloc_rx_agg(struct r8152 *tp, gfp_t mflags)
> +{
> + struct net_device *netdev = tp->netdev;
> + int node = netdev->dev.parent ? dev_to_node(netdev->dev.parent) : -1;
> + struct rx_agg *rx_agg;
> +
> + rx_agg = kmalloc_node(sizeof(*rx_agg), mflags, node);
> + if (rx_agg) {
nit: you could possibly save the indentation by returning early here
> + unsigned long flags;
> + u8 *buf;
> +
> + buf = kmalloc_node(tp->rx_buf_sz, mflags, node);
> + if (!buf)
> + goto free_rx;
> +
> + if (buf != rx_agg_align(buf)) {
> + kfree(buf);
> + buf = kmalloc_node(tp->rx_buf_sz + RX_ALIGN, mflags,
> +node);
> + if (!buf)
> + goto free_rx;
> + }
> +
> + rx_agg->buffer = buf;
> + rx_agg->head = rx_agg_align(buf);
> +
> + rx_agg->urb = usb_alloc_urb(0, mflags);
> + if (!rx_agg->urb)
> + goto free_buf;
> +
> + rx_agg->context = tp;
> +
> + INIT_LIST_HEAD(&rx_agg->list);
> + INIT_LIST_HEAD(&rx_agg->info_list);
> + spin_lock_irqsave(&tp->rx_lock, flags);
> + list_add_tail(&rx_agg->info_list, &tp->rx_info);
> + spin_unlock_irqrestore(&tp->rx_lock, flags);
> +
> + atomic_inc(&tp->rx_count);
> + }
> +
> + return rx_agg;
> +
> +free_buf:
> + kfree(rx_agg->buffer);
> +free_rx:
> + kfree(rx_agg);
> + return NULL;
> +}
> +
> static void free_all_mem(struct r8152 *tp)
> {
> + struct list_head *cursor, *next;
> + unsigned long flags;
> int i;
>
> - for (i = 0; i < RTL8152_MAX_RX; i++) {
> - usb_free_urb(tp->rx_info[i].urb);
> - tp->rx_info[i].urb = NULL;
> + spin_lock_irqsave(&tp->rx_lock, flags);
>
> - kfree(tp->rx_info[i].buffer);
> - tp->rx_info[i].buffer = NULL;
> - tp->rx_info[i].head = NULL;
> + list_for_each_safe(cursor, next, &tp->rx_info) {
nit: list_for_each_entry_safe, perhaps?
> + struct rx_agg *agg;
> +
> + agg = list_entry(cursor, struct rx_agg, info_list);
> + free_rx_agg(tp, agg);
> }
>
> + spin_unlock_irqrestore(&tp->rx_lock, flags);
> +
> + WARN_ON(unlikely(atomic_read(&tp->rx_count)));
nit: WARN_ON() has an unlikely() already built in
> for (i = 0; i < RTL8152_MAX_TX; i++) {
> usb_free_urb(tp->tx_info[i].urb);
> tp->tx_info[i].urb = NULL;
> @@ -1503,46 +1573,28 @@ static int alloc_all_mem(struct r8152 *tp)
> struct usb_interface *intf = tp->intf;
> struct usb_host_interface *alt = intf->cur_altsetting;
> struct usb_host_endpoint *ep_intr = alt->endpoint + 2;
> - struct urb *urb;
> int n
Re: [PATCH 05/14] gpio: lpc32xx: allow building on non-lpc32xx targets
Hi Arnd,
On Wed, Jul 31, 2019 at 4:00 PM Arnd Bergmann wrote:
>
> The driver uses hardwire MMIO addresses instead of the data
> that is passed in device tree. Change it over to only
> hardcode the register offset values and allow compile-testing.
>
> Signed-off-by: Arnd Bergmann
> ---
> drivers/gpio/Kconfig| 8 +
> drivers/gpio/Makefile | 2 +-
> drivers/gpio/gpio-lpc32xx.c | 63 -
> 3 files changed, 50 insertions(+), 23 deletions(-)
>
[...]
> diff --git a/drivers/gpio/gpio-lpc32xx.c b/drivers/gpio/gpio-lpc32xx.c
> index 24885b3db3d5..548f7cb69386 100644
> --- a/drivers/gpio/gpio-lpc32xx.c
> +++ b/drivers/gpio/gpio-lpc32xx.c
[...]
> @@ -498,6 +509,10 @@ static int lpc32xx_gpio_probe(struct platform_device
> *pdev)
> {
> int i;
>
> + gpio_reg_base = devm_platform_ioremap_resource(pdev, 0);
> + if (gpio_reg_base)
> + return -ENXIO;
The probe function will always return an error.
Please replace the previous 2 lines with:
if (IS_ERR(gpio_reg_base))
return PTR_ERR(gpio_reg_base);
You can add my acked-by and tested-by in the v2 patch.
Acked-by: Sylvain Lemieux
Tested-by: Sylvain Lemieux
> +
> for (i = 0; i < ARRAY_SIZE(lpc32xx_gpiochip); i++) {
> if (pdev->dev.of_node) {
> lpc32xx_gpiochip[i].chip.of_xlate = lpc32xx_of_xlate;
> @@ -527,3 +542,7 @@ static struct platform_driver lpc32xx_gpio_driver = {
> };
>
> module_platform_driver(lpc32xx_gpio_driver);
> +
> +MODULE_AUTHOR("Kevin Wells ");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("GPIO driver for LPC32xx SoC");
> --
> 2.20.0
>
Sylvain
Re: [PATCH 07/14] net: lpc-enet: move phy setup into platform code
On Wed, Jul 31, 2019 at 4:01 PM Arnd Bergmann wrote:
>
> Setting the phy mode requires touching a platform specific
> register, which prevents us from building the driver without
> its header files.
>
> Move it into a separate function in arch/arm/mach/lpc32xx
> to hide the core registers from the network driver.
>
> Signed-off-by: Arnd Bergmann
> ---
> arch/arm/mach-lpc32xx/common.c | 12
> drivers/net/ethernet/nxp/lpc_eth.c | 12 +---
> include/linux/soc/nxp/lpc32xx-misc.h | 5 +
> 3 files changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm/mach-lpc32xx/common.c b/arch/arm/mach-lpc32xx/common.c
> index f648324d5fb4..a475339333c1 100644
> --- a/arch/arm/mach-lpc32xx/common.c
> +++ b/arch/arm/mach-lpc32xx/common.c
> @@ -63,6 +63,18 @@ u32 lpc32xx_return_iram(void __iomem **mapbase, dma_addr_t
> *dmaaddr)
> }
> EXPORT_SYMBOL_GPL(lpc32xx_return_iram);
>
> +void lpc32xx_set_phy_interface_mode(phy_interface_t mode)
> +{
> + u32 tmp = __raw_readl(LPC32XX_CLKPWR_MACCLK_CTRL);
> + tmp &= ~LPC32XX_CLKPWR_MACCTRL_PINS_MSK;
> + if (mode == PHY_INTERFACE_MODE_MII)
> + tmp |= LPC32XX_CLKPWR_MACCTRL_USE_MII_PINS;
> + else
> + tmp |= LPC32XX_CLKPWR_MACCTRL_USE_RMII_PINS;
> + __raw_writel(tmp, LPC32XX_CLKPWR_MACCLK_CTRL);
> +}
> +EXPORT_SYMBOL_GPL(lpc32xx_set_phy_interface_mode);
> +
> static struct map_desc lpc32xx_io_desc[] __initdata = {
> {
> .virtual= (unsigned
> long)IO_ADDRESS(LPC32XX_AHB0_START),
> diff --git a/drivers/net/ethernet/nxp/lpc_eth.c
> b/drivers/net/ethernet/nxp/lpc_eth.c
> index bcdd0adcfb0c..0893b77c385d 100644
> --- a/drivers/net/ethernet/nxp/lpc_eth.c
> +++ b/drivers/net/ethernet/nxp/lpc_eth.c
> @@ -20,9 +20,6 @@
> #include
> #include
>
> -#include
> -#include
> -
> #define MODNAME "lpc-eth"
> #define DRV_VERSION "1.00"
>
> @@ -1237,16 +1234,9 @@ static int lpc_eth_drv_probe(struct platform_device
> *pdev)
> dma_addr_t dma_handle;
> struct resource *res;
> int irq, ret;
> - u32 tmp;
>
> /* Setup network interface for RMII or MII mode */
> - tmp = __raw_readl(LPC32XX_CLKPWR_MACCLK_CTRL);
> - tmp &= ~LPC32XX_CLKPWR_MACCTRL_PINS_MSK;
> - if (lpc_phy_interface_mode(dev) == PHY_INTERFACE_MODE_MII)
> - tmp |= LPC32XX_CLKPWR_MACCTRL_USE_MII_PINS;
> - else
> - tmp |= LPC32XX_CLKPWR_MACCTRL_USE_RMII_PINS;
> - __raw_writel(tmp, LPC32XX_CLKPWR_MACCLK_CTRL);
> + lpc32xx_set_phy_interface_mode(lpc_phy_interface_mode(dev));
>
> /* Get platform resources */
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> diff --git a/include/linux/soc/nxp/lpc32xx-misc.h
> b/include/linux/soc/nxp/lpc32xx-misc.h
> index f232e1a1bcdc..af4f82f6cf3b 100644
> --- a/include/linux/soc/nxp/lpc32xx-misc.h
> +++ b/include/linux/soc/nxp/lpc32xx-misc.h
> @@ -9,9 +9,11 @@
> #define __SOC_LPC32XX_MISC_H
>
> #include
> +#include
>
> #ifdef CONFIG_ARCH_LPC32XX
> extern u32 lpc32xx_return_iram(void __iomem **mapbase, dma_addr_t *dmaaddr);
> +extern void lpc32xx_set_phy_interface_mode(phy_interface_t mode);
> #else
> static inline u32 lpc32xx_return_iram(void __iomem **mapbase, dma_addr_t
> *dmaaddr)
> {
> @@ -19,6 +21,9 @@ static inline u32 lpc32xx_return_iram(void __iomem
> **mapbase, dma_addr_t *dmaadd
> *dmaaddr = 0;
> return 0;
> }
> +static inline void lpc32xx_set_phy_interface_mode(phy_interface_t mode)
> +{
> +}
> #endif
>
> #endif /* __SOC_LPC32XX_MISC_H */
> --
> 2.20.0
>
Re: [PATCH 07/14] net: lpc-enet: move phy setup into platform code
Acked-by: Sylvain Lemieux
On Wed, Jul 31, 2019 at 4:01 PM Arnd Bergmann wrote:
>
> Setting the phy mode requires touching a platform specific
> register, which prevents us from building the driver without
> its header files.
>
> Move it into a separate function in arch/arm/mach/lpc32xx
> to hide the core registers from the network driver.
>
> Signed-off-by: Arnd Bergmann
> ---
> arch/arm/mach-lpc32xx/common.c | 12
> drivers/net/ethernet/nxp/lpc_eth.c | 12 +---
> include/linux/soc/nxp/lpc32xx-misc.h | 5 +
> 3 files changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm/mach-lpc32xx/common.c b/arch/arm/mach-lpc32xx/common.c
> index f648324d5fb4..a475339333c1 100644
> --- a/arch/arm/mach-lpc32xx/common.c
> +++ b/arch/arm/mach-lpc32xx/common.c
> @@ -63,6 +63,18 @@ u32 lpc32xx_return_iram(void __iomem **mapbase, dma_addr_t
> *dmaaddr)
> }
> EXPORT_SYMBOL_GPL(lpc32xx_return_iram);
>
> +void lpc32xx_set_phy_interface_mode(phy_interface_t mode)
> +{
> + u32 tmp = __raw_readl(LPC32XX_CLKPWR_MACCLK_CTRL);
> + tmp &= ~LPC32XX_CLKPWR_MACCTRL_PINS_MSK;
> + if (mode == PHY_INTERFACE_MODE_MII)
> + tmp |= LPC32XX_CLKPWR_MACCTRL_USE_MII_PINS;
> + else
> + tmp |= LPC32XX_CLKPWR_MACCTRL_USE_RMII_PINS;
> + __raw_writel(tmp, LPC32XX_CLKPWR_MACCLK_CTRL);
> +}
> +EXPORT_SYMBOL_GPL(lpc32xx_set_phy_interface_mode);
> +
> static struct map_desc lpc32xx_io_desc[] __initdata = {
> {
> .virtual= (unsigned
> long)IO_ADDRESS(LPC32XX_AHB0_START),
> diff --git a/drivers/net/ethernet/nxp/lpc_eth.c
> b/drivers/net/ethernet/nxp/lpc_eth.c
> index bcdd0adcfb0c..0893b77c385d 100644
> --- a/drivers/net/ethernet/nxp/lpc_eth.c
> +++ b/drivers/net/ethernet/nxp/lpc_eth.c
> @@ -20,9 +20,6 @@
> #include
> #include
>
> -#include
> -#include
> -
> #define MODNAME "lpc-eth"
> #define DRV_VERSION "1.00"
>
> @@ -1237,16 +1234,9 @@ static int lpc_eth_drv_probe(struct platform_device
> *pdev)
> dma_addr_t dma_handle;
> struct resource *res;
> int irq, ret;
> - u32 tmp;
>
> /* Setup network interface for RMII or MII mode */
> - tmp = __raw_readl(LPC32XX_CLKPWR_MACCLK_CTRL);
> - tmp &= ~LPC32XX_CLKPWR_MACCTRL_PINS_MSK;
> - if (lpc_phy_interface_mode(dev) == PHY_INTERFACE_MODE_MII)
> - tmp |= LPC32XX_CLKPWR_MACCTRL_USE_MII_PINS;
> - else
> - tmp |= LPC32XX_CLKPWR_MACCTRL_USE_RMII_PINS;
> - __raw_writel(tmp, LPC32XX_CLKPWR_MACCLK_CTRL);
> + lpc32xx_set_phy_interface_mode(lpc_phy_interface_mode(dev));
>
> /* Get platform resources */
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> diff --git a/include/linux/soc/nxp/lpc32xx-misc.h
> b/include/linux/soc/nxp/lpc32xx-misc.h
> index f232e1a1bcdc..af4f82f6cf3b 100644
> --- a/include/linux/soc/nxp/lpc32xx-misc.h
> +++ b/include/linux/soc/nxp/lpc32xx-misc.h
> @@ -9,9 +9,11 @@
> #define __SOC_LPC32XX_MISC_H
>
> #include
> +#include
>
> #ifdef CONFIG_ARCH_LPC32XX
> extern u32 lpc32xx_return_iram(void __iomem **mapbase, dma_addr_t *dmaaddr);
> +extern void lpc32xx_set_phy_interface_mode(phy_interface_t mode);
> #else
> static inline u32 lpc32xx_return_iram(void __iomem **mapbase, dma_addr_t
> *dmaaddr)
> {
> @@ -19,6 +21,9 @@ static inline u32 lpc32xx_return_iram(void __iomem
> **mapbase, dma_addr_t *dmaadd
> *dmaaddr = 0;
> return 0;
> }
> +static inline void lpc32xx_set_phy_interface_mode(phy_interface_t mode)
> +{
> +}
> #endif
>
> #endif /* __SOC_LPC32XX_MISC_H */
> --
> 2.20.0
>
Re: [PATCH 08/14] net: lpc-enet: allow compile testing
Acked-by: Sylvain Lemieux On Wed, Jul 31, 2019 at 4:01 PM Arnd Bergmann wrote: > > The lpc-enet driver can now be built on all platforms, so > allow compile testing as well. > > Signed-off-by: Arnd Bergmann > --- > drivers/net/ethernet/nxp/Kconfig | 2 +- > drivers/net/ethernet/nxp/lpc_eth.c | 1 + > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/nxp/Kconfig > b/drivers/net/ethernet/nxp/Kconfig > index 261f107e2be0..418afb84c84b 100644 > --- a/drivers/net/ethernet/nxp/Kconfig > +++ b/drivers/net/ethernet/nxp/Kconfig > @@ -1,7 +1,7 @@ > # SPDX-License-Identifier: GPL-2.0-only > config LPC_ENET > tristate "NXP ethernet MAC on LPC devices" > -depends on ARCH_LPC32XX > +depends on ARCH_LPC32XX || COMPILE_TEST > select PHYLIB > help > Say Y or M here if you want to use the NXP ethernet MAC included on > diff --git a/drivers/net/ethernet/nxp/lpc_eth.c > b/drivers/net/ethernet/nxp/lpc_eth.c > index 0893b77c385d..34fdf2100772 100644 > --- a/drivers/net/ethernet/nxp/lpc_eth.c > +++ b/drivers/net/ethernet/nxp/lpc_eth.c > @@ -14,6 +14,7 @@ > #include > #include > #include > +#include > #include > #include > #include > -- > 2.20.0 >
Re: [PATCH 10/14] ARM: lpc32xx: clean up header files
Acked-by: Sylvain Lemieux
On Wed, Jul 31, 2019 at 4:03 PM Arnd Bergmann wrote:
>
> All device drivers have stopped relying on mach/*.h headers,
> so move the remaining headers into arch/arm/mach-lpc32xx/lpc32xx.h
> to prepare for multiplatform builds.
>
> The mach/entry-macro.S file has been unused for a long time now
> and can simply get removed.
>
> Signed-off-by: Arnd Bergmann
> ---
> arch/arm/mach-lpc32xx/common.c| 3 +-
> .../mach-lpc32xx/include/mach/entry-macro.S | 28 ---
> arch/arm/mach-lpc32xx/include/mach/hardware.h | 25 -
> .../mach-lpc32xx/include/mach/uncompress.h| 4 +--
> .../{include/mach/platform.h => lpc32xx.h}| 18 ++--
> arch/arm/mach-lpc32xx/pm.c| 3 +-
> arch/arm/mach-lpc32xx/serial.c| 3 +-
> arch/arm/mach-lpc32xx/suspend.S | 3 +-
> 8 files changed, 21 insertions(+), 66 deletions(-)
> delete mode 100644 arch/arm/mach-lpc32xx/include/mach/entry-macro.S
> delete mode 100644 arch/arm/mach-lpc32xx/include/mach/hardware.h
> rename arch/arm/mach-lpc32xx/{include/mach/platform.h => lpc32xx.h} (98%)
>
> diff --git a/arch/arm/mach-lpc32xx/common.c b/arch/arm/mach-lpc32xx/common.c
> index a475339333c1..304ea61a0716 100644
> --- a/arch/arm/mach-lpc32xx/common.c
> +++ b/arch/arm/mach-lpc32xx/common.c
> @@ -13,8 +13,7 @@
> #include
> #include
>
> -#include
> -#include
> +#include "lpc32xx.h"
> #include "common.h"
>
> /*
> diff --git a/arch/arm/mach-lpc32xx/include/mach/entry-macro.S
> b/arch/arm/mach-lpc32xx/include/mach/entry-macro.S
> deleted file mode 100644
> index eec0f5f7e722..
> --- a/arch/arm/mach-lpc32xx/include/mach/entry-macro.S
> +++ /dev/null
> @@ -1,28 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-or-later */
> -/*
> - * arch/arm/mach-lpc32xx/include/mach/entry-macro.S
> - *
> - * Author: Kevin Wells
> - *
> - * Copyright (C) 2010 NXP Semiconductors
> - */
> -
> -#include
> -#include
> -
> -#define LPC32XX_INTC_MASKED_STATUS_OFS 0x8
> -
> - .macro get_irqnr_preamble, base, tmp
> - ldr \base, =IO_ADDRESS(LPC32XX_MIC_BASE)
> - .endm
> -
> -/*
> - * Return IRQ number in irqnr. Also return processor Z flag status in CPSR
> - * as set if an interrupt is pending.
> - */
> - .macro get_irqnr_and_base, irqnr, irqstat, base, tmp
> - ldr \irqstat, [\base, #LPC32XX_INTC_MASKED_STATUS_OFS]
> - clz \irqnr, \irqstat
> - rsb \irqnr, \irqnr, #31
> - teq \irqstat, #0
> - .endm
> diff --git a/arch/arm/mach-lpc32xx/include/mach/hardware.h
> b/arch/arm/mach-lpc32xx/include/mach/hardware.h
> deleted file mode 100644
> index 4866f096ffce..
> --- a/arch/arm/mach-lpc32xx/include/mach/hardware.h
> +++ /dev/null
> @@ -1,25 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-or-later */
> -/*
> - * arch/arm/mach-lpc32xx/include/mach/hardware.h
> - *
> - * Copyright (c) 2005 MontaVista Software, Inc.
> - */
> -
> -#ifndef __ASM_ARCH_HARDWARE_H
> -#define __ASM_ARCH_HARDWARE_H
> -
> -/*
> - * Start of virtual addresses for IO devices
> - */
> -#define IO_BASE0xF000
> -
> -/*
> - * This macro relies on fact that for all HW i/o addresses bits 20-23 are 0
> - */
> -#define IO_ADDRESS(x) IOMEM(x) & 0xff00) >> 4) | ((x) & 0xf)) |\
> -IO_BASE)
> -
> -#define io_p2v(x) ((void __iomem *) (unsigned long) IO_ADDRESS(x))
> -#define io_v2p(x) x) & 0x0ff0) << 4) | ((x) & 0x000f))
> -
> -#endif
> diff --git a/arch/arm/mach-lpc32xx/include/mach/uncompress.h
> b/arch/arm/mach-lpc32xx/include/mach/uncompress.h
> index a568812a0b91..74b7aa0da0e4 100644
> --- a/arch/arm/mach-lpc32xx/include/mach/uncompress.h
> +++ b/arch/arm/mach-lpc32xx/include/mach/uncompress.h
> @@ -12,15 +12,13 @@
>
> #include
>
> -#include
> -#include
> -
> /*
> * Uncompress output is hardcoded to standard UART 5
> */
>
> #define UART_FIFO_CTL_TX_RESET (1 << 2)
> #define UART_STATUS_TX_MT (1 << 6)
> +#define LPC32XX_UART5_BASE 0x4009
>
> #define _UARTREG(x)(void __iomem *)(LPC32XX_UART5_BASE + (x))
>
> diff --git a/arch/arm/mach-lpc32xx/include/mach/platform.h
> b/arch/arm/mach-lpc32xx/lpc32xx.h
> similarity index 98%
> rename from arch/arm/mach-lpc32xx/include/mach/platform.h
> rename to arch/arm/mach-lpc32xx/lpc32xx.h
> index 1c53790444fc..5eeb884a1993 100644
> --- a/arch/arm/mach-lpc32xx/include/mach/platform.h
> +++ b/arch/arm/mach-lpc32xx/lpc32xx.h
> @@ -7,8 +7,8 @@
> * Copyright (C) 2010 NXP Semiconductors
> */
>
> -#ifndef __ASM_ARCH_PLATFORM_H
> -#define __ASM_ARCH_PLATFORM_H
> +#ifndef __ARM_LPC32XX_H
> +#define __ARM_LPC32XX_H
>
> #define _SBF(f, v) ((v) << (f))
> #define _BIT(n)_SBF(n, 1)
> @@ -700,4 +700,18 @@
> #define LPC32XX_USB_OTG_DEV_CLOCK_ON _BIT(1)
> #define LPC32XX_USB_OTG_HOST_CLOCK_ON _BIT(0)
>
> +/*
> +
Re: [PATCH net-next 2/5] r8152: replace array with linking list for rx information
On Tue, 6 Aug 2019 12:53:42 -0700, Jakub Kicinski wrote:
> > @@ -744,6 +745,8 @@ struct r8152 {
> > void (*autosuspend_en)(struct r8152 *tp, bool enable);
> > } rtl_ops;
> >
> > + atomic_t rx_count;
>
> I wonder what the advantage of rx_count being atomic is, perhpas it
> could be protected by the same lock as the list head?
Okay, I see you're using it to test the queue length without the lock in
a later patch.
Re: [PATCH net-next 4/5] r8152: support skb_add_rx_frag
On Tue, 6 Aug 2019 19:18:03 +0800, Hayes Wang wrote:
> Use skb_add_rx_frag() to reduce the memory copy for rx data.
>
> Use a new list of rx_used to store the rx buffer which couldn't be
> reused yet.
>
> Besides, the total number of rx buffer may be increased or decreased
> dynamically. And it is limited by RTL8152_MAX_RX_AGG.
>
> Signed-off-by: Hayes Wang
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index 401e56112365..1615900c8592 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -584,6 +584,9 @@ enum rtl_register_content {
> #define TX_ALIGN 4
> #define RX_ALIGN 8
>
> +#define RTL8152_MAX_RX_AGG (10 * RTL8152_MAX_RX)
> +#define RTL8152_RXFG_HEADSZ 256
> +
> #define INTR_LINK0x0004
>
> #define RTL8152_REQT_READ0xc0
> @@ -720,7 +723,7 @@ struct r8152 {
> struct net_device *netdev;
> struct urb *intr_urb;
> struct tx_agg tx_info[RTL8152_MAX_TX];
> - struct list_head rx_info;
> + struct list_head rx_info, rx_used;
I don't see where entries on the rx_used list get freed when driver is
unloaded, could you explain how that's taken care of?
> struct list_head rx_done, tx_free;
> struct sk_buff_head tx_queue, rx_queue;
> spinlock_t rx_lock, tx_lock;
> @@ -1476,7 +1479,7 @@ static void free_rx_agg(struct r8152 *tp, struct rx_agg
> *agg)
> list_del(&agg->info_list);
>
> usb_free_urb(agg->urb);
> - __free_pages(agg->page, get_order(tp->rx_buf_sz));
> + put_page(agg->page);
> kfree(agg);
>
> atomic_dec(&tp->rx_count);
> @@ -1493,7 +1496,7 @@ static struct rx_agg *alloc_rx_agg(struct r8152 *tp,
> gfp_t mflags)
> if (rx_agg) {
> unsigned long flags;
>
> - rx_agg->page = alloc_pages(mflags, order);
> + rx_agg->page = alloc_pages(mflags | __GFP_COMP, order);
> if (!rx_agg->page)
> goto free_rx;
>
> @@ -1951,6 +1954,50 @@ static u8 r8152_rx_csum(struct r8152 *tp, struct
> rx_desc *rx_desc)
> return checksum;
> }
>
> +static inline bool rx_count_exceed(struct r8152 *tp)
> +{
> + return atomic_read(&tp->rx_count) > RTL8152_MAX_RX;
> +}
> +
> +static inline int agg_offset(struct rx_agg *agg, void *addr)
> +{
> + return (int)(addr - agg->buffer);
> +}
> +
> +static struct rx_agg *rtl_get_free_rx(struct r8152 *tp, gfp_t mflags)
> +{
> + struct list_head *cursor, *next;
> + struct rx_agg *agg_free = NULL;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&tp->rx_lock, flags);
> +
> + list_for_each_safe(cursor, next, &tp->rx_used) {
> + struct rx_agg *agg;
> +
> + agg = list_entry(cursor, struct rx_agg, list);
> +
> + if (page_count(agg->page) == 1) {
> + if (!agg_free) {
> + list_del_init(cursor);
> + agg_free = agg;
> + continue;
> + } else if (rx_count_exceed(tp)) {
nit: else unnecessary after continue
> + list_del_init(cursor);
> + free_rx_agg(tp, agg);
> + }
> + break;
> + }
> + }
> +
> + spin_unlock_irqrestore(&tp->rx_lock, flags);
> +
> + if (!agg_free && atomic_read(&tp->rx_count) < RTL8152_MAX_RX_AGG)
> + agg_free = alloc_rx_agg(tp, mflags);
> +
> + return agg_free;
> +}
> +
> static int rx_bottom(struct r8152 *tp, int budget)
> {
> unsigned long flags;
Re: [PATCH net-next 5/5] r8152: change rx_frag_head_sz and rx_max_agg_num dynamically
On Tue, 6 Aug 2019 19:18:04 +0800, Hayes Wang wrote: > Let rx_frag_head_sz and rx_max_agg_num could be modified dynamically > through the sysfs. > > Signed-off-by: Hayes Wang Please don't expose those via sysfs. Ethtool's copybreak and descriptor count should be applicable here, I think.
PARTNERSHIP REQUEST,
Dear Friend, I need you to please let me know if there are fast growing investments in your country in which i can invest money in. I have access to a huge amount of money, which i want to invest in your country, i want to know if you can be an agent/partner to me and i will give you a commission of 30% only If you agree to assist me, i will like to know if the commission is ok for you, also i would love to know more about you too. Get Back to me without delay if you are interested Yours Faithfully Simon Beron.
RE: [PATCH 2/5] usb: chipidea: add role switch class support
> > > > > > > > > > > > You may use connect bit at portsc since VBUS may not connect to SoC. > > > > > > By this way, disconnect can be decided but connect is a problem in > > > current driver, as controller was stopped in low power mode so can't > > > detect connection from host, unless we also update this behavior too. > > > > > > > For connection, if current role is USB_ROLE_NONE, you may start device mode. > > This is assuming set role only can be called one time between disconnect and > connect to host, this may not always true, but this can be resolved, I think > we need > handle the case device mode is started but connection does not happen, so the > gadget need be stopped and enter low power mode again, if this approach is OK > to > you, I will go in this direction for my v2. > After thinking more, I think Type-C tcpm code should set usb_role correctly, it knows physical connection status well, and there are already USB_ROLE_NONE references at tcpm now. Depending on USB device driver workaround to know USB_ROLE_NONE is not a good solution. Look at another USB role driver, intel-xhci-usb-role-switch.c, it could also get USB_ROLE_NONE state. Peter
Re: EHSET USB testing
Hi Everyone, (Pinging Mathias regarding xHCI support of the USB 2.0 test modes) On Mon, Aug 05, 2019 at 02:07:23PM +0800, Peter Chen wrote: > On Sun, Aug 4, 2019 at 10:45 AM Evan Gates wrote: > > > > I'm trying to get my device to pass the EHSET tests. I found the thread > > "Using EHSET module" from March. I'm having similar issues. > > > > I don't have access to the PID VID board that the lab uses. Instead I'm > > using another computer setup as a mass storage gadget but set the VID:PID > > to 1a0a:0102 (TEST_J). If that sounds like a problem or there is another, > > better way to do this please let me know. > > > > > The usb-storage driver bound to your device first. Try building a > > > kernel without that driver and then it shoudl bind to the other driver. > > > > > > Or manually bind the ehset driver to your device through sysfs. Read up > > > on the documentation for the "new_id" and "bind" and "unbind" sysfs > > > files for how to do that. > > > > I did this. I was able to unbind from usb-storage but binding to > > usb_ehset_test failed with error -32 (AFAICT EPIPE). I tried both the > > usb_ehset_test/bind and usb_ehset_test/new_id methods. In both cases > > I got the same error. > > > > I did another build without usb-storage. Now I don't have to go through > > the unbind step but I still get the same error while probing. > > > > [ 296.089877] usb 1-1: new high-speed USB device number 2 using > > xhci_hcd > > [ 296.271081] usb_ehset_test: probe of 1-1:1.0 failed with error > > -32 > > > > I notice that it says "using xhci_hcd." Is that a problem? Does it > > need to be ehci? I tried a build without xhci but that caused other > > problems for me. > > > > 1) Can I use a computer in device mode to present a VID:PID to get into > > EHSET mode? If so how should I do that? > > > > Afaik, you can't. No Host computer will act as USB device. You may try > to configure your box as USB device mode, for gadget driver, you could > using legacy g_zero module, or source_sink function using configfs. > > > 2) What else do I need to do in order to get my box into EHSET mode? > Only thing is your box need to be at host mode. For testing USB2 for xHCI, > the mainline code should not support TEST_SINGLE_STEP_SET_FEATURE > at my last access. Today I was able to get access to a PID/VID test fixture that is used in USB 2.0 EHSET testing. Some of the major findings include: 1. The only test mode that works properly is TEST_HS_HOST_PORT_SUSPEND_RESUME. All other probes of the ehset module fail with errno -32 as shown above. This test mode is likely the only one to work because it does not set the PORT_FEAT_TEST feature for the appropriate port, but rather sets and clears the PORT_FEAT_SUSPEND feature. 2. The same issue was observed on not only our custom hardware, but also a Dell XPS 13 running Ubuntu 18.04 with kernel version 4.15. I've attached the dmesg logs with xhci_hcd dynamic debug enabled for both hosts. The logs were captured while testing the TEST_PACKET test mode, but the logs looked similar for every other rest mode besides HS_HOST_PORT_SUSPEND_RESUME. Mathias, I would appreciate your feedback on the attached dmesg logs with xhci_hcd dynamic debug enabled. If you have a moment, could you please take a look at the logs and let me know if there are any clues as to why the test mode is not working with xhci_hcd? I've also attached the lsusb output for this particular EHSET test fixture made by Allion. I've been digging into the xHCI specification to see what details it might contain about the USB 2.0 test modes we are trying to use. Section 4.19.6 describes the proper sequence for enabling the USB 2.0 port test modes. This requires setting the appropriate bits in the USB 2.0 PORTPMSC register, which is described in full detail in section 5.4.9.2. It's clear that xHCI should support the EHSET test modes, but we're not able to set the appropriate port features for some reason. My next thought is to double-check the logic of the ehset driver as it relates to xhci_hcd. I would like to make sure that we are enabling the USB 2.0 test modes according to the xHCI spec. I'm concerned that we might be experiencing an error because we aren't setting the test mode acording to section 4.19.6 of the xHCI specification. Thanks in advance for reviewing the logs, Mathias! I appreciate any time and feedback you may be able to provide. Let me know if you have any further questions or need more information from me. I look forward to hearing from you soon! Cheers, Rob Weber [110706.181186] xhci_hcd :00:14.0: // Ding dong! [110706.181229] xhci_hcd :00:14.0: Slot 27 output ctx = 0x182032000 (dma) [110706.181234] xhci_hcd :00:14.0: Slot 27 input ctx = 0x106a32000 (dma) [110706.181254] xhci_hcd :00:14.0: Set slot id 27 dcbaa entry a827a8c9 to 0x182032000 [110706.261110] usb 1-1.1: new high-speed USB device number 27 using xhci_hcd [110706
RE: [PATCH net-next 2/5] r8152: replace array with linking list for rx information
Jakub Kicinski [mailto:jakub.kicin...@netronome.com]
[...]
> > static int rtl_stop_rx(struct r8152 *tp)
> > {
> > - int i;
> > + struct list_head *cursor, *next, tmp_list;
> > + unsigned long flags;
> > +
> > + INIT_LIST_HEAD(&tmp_list);
> >
> > - for (i = 0; i < RTL8152_MAX_RX; i++)
> > - usb_kill_urb(tp->rx_info[i].urb);
> > + /* The usb_kill_urb() couldn't be used in atomic.
> > +* Therefore, move the list of rx_info to a tmp one.
> > +* Then, list_for_each_safe could be used without
> > +* spin lock.
> > +*/
>
> Would you mind explaining in a little more detail why taking the
> entries from the list for a brief period of time is safe?
Usually, it needs the spin lock before accessing the entry
of the list "tp->rx_info". However, for some reasons,
if we want to access the entry without spin lock, we
cloud move all entries to a local list temporally. Then,
we could make sure no other one could access the entries
included in the temporal local list.
For this case, when I move all entries to a temporal
local list, no other one could access them. Therefore,
I could access the entries included in the temporal local
list without spin lock.
Best Regards,
Hayes
RE: [PATCH net-next 4/5] r8152: support skb_add_rx_frag
Jakub Kicinski [mailto:jakub.kicin...@netronome.com]
> Sent: Wednesday, August 07, 2019 6:08 AM
[...]
> > #define RTL8152_REQT_READ 0xc0
> > @@ -720,7 +723,7 @@ struct r8152 {
> > struct net_device *netdev;
> > struct urb *intr_urb;
> > struct tx_agg tx_info[RTL8152_MAX_TX];
> > - struct list_head rx_info;
> > + struct list_head rx_info, rx_used;
>
> I don't see where entries on the rx_used list get freed when driver is
> unloaded, could you explain how that's taken care of?
When the driver is unloaded, all rx_agg would be freed from
info_list list.
The info_list includes all rx_agg buffers which may be idle
or be busy. The rx_done and rx_use are used to determine
the status of rx_agg buffer included in info_list.
info_list: the rx_agg buffer would be inserted in this list
when it is allocated.
rx_done: the rx_agg buffer is ready (contains rx data). Or
it needs to be resubmitted.
rx_use: the rx_agg buffer is busy and couldn't be submitted
yet.
Best Regards,
Hayes
RE: EHSET USB testing
> Hi Everyone, > > (Pinging Mathias regarding xHCI support of the USB 2.0 test modes) > > On Mon, Aug 05, 2019 at 02:07:23PM +0800, Peter Chen wrote: > > On Sun, Aug 4, 2019 at 10:45 AM Evan Gates wrote: > > > > > > I'm trying to get my device to pass the EHSET tests. I found the > > > thread "Using EHSET module" from March. I'm having similar issues. > > > > > > I don't have access to the PID VID board that the lab uses. Instead > > > I'm using another computer setup as a mass storage gadget but set > > > the VID:PID to 1a0a:0102 (TEST_J). If that sounds like a problem or > > > there is another, better way to do this please let me know. > > > > > > > The usb-storage driver bound to your device first. Try building a > > > > kernel without that driver and then it shoudl bind to the other driver. > > > > > > > > Or manually bind the ehset driver to your device through sysfs. > > > > Read up on the documentation for the "new_id" and "bind" and > > > > "unbind" sysfs files for how to do that. > > > > > > I did this. I was able to unbind from usb-storage but binding to > > > usb_ehset_test failed with error -32 (AFAICT EPIPE). I tried both > > > the usb_ehset_test/bind and usb_ehset_test/new_id methods. In both > > > cases I got the same error. > > > > > > I did another build without usb-storage. Now I don't have to go > > > through the unbind step but I still get the same error while probing. > > > > > > [ 296.089877] usb 1-1: new high-speed USB device number 2 using > xhci_hcd > > > [ 296.271081] usb_ehset_test: probe of 1-1:1.0 failed with > > > error -32 > > > > > > I notice that it says "using xhci_hcd." Is that a problem? Does it > > > need to be ehci? I tried a build without xhci but that caused other > > > problems for me. > > > > > > 1) Can I use a computer in device mode to present a VID:PID to get > > > into EHSET mode? If so how should I do that? > > > > > > > Afaik, you can't. No Host computer will act as USB device. You may try > > to configure your box as USB device mode, for gadget driver, you could > > using legacy g_zero module, or source_sink function using configfs. > > > > > 2) What else do I need to do in order to get my box into EHSET mode? > > Only thing is your box need to be at host mode. For testing USB2 for > > xHCI, the mainline code should not support > > TEST_SINGLE_STEP_SET_FEATURE at my last access. > > Today I was able to get access to a PID/VID test fixture that is used in USB > 2.0 > EHSET testing. Some of the major findings include: > > 1. The only test mode that works properly is >TEST_HS_HOST_PORT_SUSPEND_RESUME. All other probes of the ehset > module >fail with errno -32 as shown above. This test mode is likely the only >one to work because it does not set the PORT_FEAT_TEST feature for the >appropriate port, but rather sets and clears the PORT_FEAT_SUSPEND feature. > > 2. The same issue was observed on not only our custom hardware, but also >a Dell XPS 13 running Ubuntu 18.04 with kernel version 4.15. I've >attached the dmesg logs with xhci_hcd dynamic debug enabled for both >hosts. The logs were captured while testing the TEST_PACKET test >mode, but the logs looked similar for every other rest mode besides >HS_HOST_PORT_SUSPEND_RESUME. > > > Mathias, I would appreciate your feedback on the attached dmesg logs with > xhci_hcd dynamic debug enabled. If you have a moment, could you please take a > look at the logs and let me know if there are any clues as to why the test > mode is > not working with xhci_hcd? I've also attached the lsusb output for this > particular > EHSET test fixture made by Allion. > > I've been digging into the xHCI specification to see what details it might > contain > about the USB 2.0 test modes we are trying to use. > Section 4.19.6 describes the proper sequence for enabling the USB 2.0 port > test > modes. This requires setting the appropriate bits in the USB > 2.0 PORTPMSC register, which is described in full detail in section 5.4.9.2. > It's clear > that xHCI should support the EHSET test modes, but we're not able to set the > appropriate port features for some reason. > > My next thought is to double-check the logic of the ehset driver as it > relates to > xhci_hcd. I would like to make sure that we are enabling the USB 2.0 test > modes > according to the xHCI spec. I'm concerned that we might be experiencing an > error > because we aren't setting the test mode acording to section 4.19.6 of the xHCI > specification. > > Thanks in advance for reviewing the logs, Mathias! I appreciate any time and > feedback you may be able to provide. Let me know if you have any further > questions or need more information from me. I look forward to hearing from you > soon! > You may not enter test mode at all. Check flow: xhci_hub_control->xhci_enter_test_mode. Peter
