Re: [PATCH 07/12] usb: usbtmc: Add ioctl USBTMC488_IOCTL_WAIT_SRQ

2018-05-28 Thread Oliver Neukum
Am Montag, den 28.05.2018, 10:59 + schrieb guido@kiener-
muenchen.de:
> > No, the problem is that you will underflow io->mutex
> > 
> 
> Don't worry. The function usbtmc488_ioctl_wait_srq is called by
> usbtmc_ioctl which already locks the mutex. See
> https://elixir.bootlin.com/linux/v4.17-rc7/source/drivers/usb/class/usbtmc.c#L1189
> So the mutex is just unlocked here to allow other threads to still communicate
> with the device.

Hi,

thanks I had overlooked that.

Regards
Oliver

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 02/12] usb: usbtmc: Support Read Status Byte with SRQ per file handle

2018-05-28 Thread Oliver Neukum
Am Donnerstag, den 24.05.2018, 12:31 + schrieb guido@kiener-
muenchen.de:
> Zitat von Oliver Neukum :
> 
> > Am Montag, den 21.05.2018, 21:00 + schrieb guido@kiener-
> > muenchen.de:
> > > 
> > > I looked for a race here, but I do not find a race between open and 
> > > release,
> > > since a refcount of "file_data->data->kref" is always hold by
> > > usbtmc_probe/disconnect.
> > > 
> > > However I see a race between usbtmc_open and usbtmc_disconnect. Are these
> > > callback functions called mutual exclusive?
> > 
> > No, they are not.
> 
> In the meantime I found these conflictive hints:
> 
> https://github.com/torvalds/linux/commit/52a749992ca6a0fd304609af40ed3bfd6cef4660
> and
> https://elixir.bootlin.com/linux/v4.17-rc6/source/include/linux/usb.h#L1164
> 
> What do you think?
> My current feeling is that open/disconnect is mutual exclusive.
> We also could verify what really happens.

Ok, I remember.

You are safe, if and only if you share the USB minor number space.

Regards
Oliver

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Fwd: usb: uas: device reset most the time while enumeration- usb3.0

2018-05-28 Thread Oliver Neukum
Am Donnerstag, den 24.05.2018, 20:13 +0530 schrieb Tushar Nimkar:
> We have built SCSI as module will it cause any problem to enable
> CONFIG_SCSI_SCAN_ASYNC ?

No, that should work. But the failure is bizzare. You say
synchronous scanning fails, but async scan works?

Regards
Oliver

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] usb: dwc3: of_simple: don't call pm_runtime_set_active()

2018-05-28 Thread Johan Hovold
On Mon, May 28, 2018 at 05:36:14PM +0300, Roger Quadros wrote:
> Don't call pm_runtime_set_active() as it will prevent the device
> from being activated in the next pm_runtime_get_sync() call.
> 
> Also call pm_runtime_get_sync() before of_platform_populate().

This paragraph describes what you do, but not why do it.

> Signed-off-by: Roger Quadros 
> ---
>  drivers/usb/dwc3/dwc3-of-simple.c | 7 +++
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-of-simple.c 
> b/drivers/usb/dwc3/dwc3-of-simple.c
> index e98d221..2cbb5c0 100644
> --- a/drivers/usb/dwc3/dwc3-of-simple.c
> +++ b/drivers/usb/dwc3/dwc3-of-simple.c
> @@ -121,6 +121,9 @@ static int dwc3_of_simple_probe(struct platform_device 
> *pdev)
>   if (ret)
>   goto err_resetc_assert;
>  
> + pm_runtime_enable(dev);
> + pm_runtime_get_sync(dev);

This breaks runtime pm as you now get a second round of clock enables
which are never balanced on runtime suspend (the clocks are first
enabled in dwc3_of_simple_clk_init() above and with your change again in
dwc3_of_simple_runtime_resume()).

On the other hand, we currently return from probe() with a positive RPM
count so perhaps the RPM callbacks can just be removed altogether (i.e.
unless some other entity drops that count at some point before
remove()).

>   ret = of_platform_populate(np, NULL, NULL, dev);
>   if (ret) {
>   for (i = 0; i < simple->num_clocks; i++) {
> @@ -131,10 +134,6 @@ static int dwc3_of_simple_probe(struct platform_device 
> *pdev)
>   goto err_resetc_assert;
>   }
>  
> - pm_runtime_set_active(dev);
> - pm_runtime_enable(dev);
> - pm_runtime_get_sync(dev);
> -
>   return 0;
>  
>  err_resetc_assert:

Also note that there's currently a use-after-free in remove(), where
pm_runtime_put_sync() is called after the clocks have been put.
Something like the below (untested) patch should fix it.

Johan


>From 35c384c31010c344d403c26fc0a1dde0fd68ef4a Mon Sep 17 00:00:00 2001
From: Johan Hovold 
Date: Mon, 28 May 2018 17:31:45 +0200
Subject: [PATCH] usb: dwc3: of-simple: fix use-after-free on remove

The clocks have already been explicitly disabled and put as part of
remove() so the runtime suspend callback must not be run when balancing
the runtime PM usage count before returning.

Fixes: 16adc674d0d6 ("usb: dwc3: add generic OF glue layer")
Signed-off-by: Johan Hovold 
---
 drivers/usb/dwc3/dwc3-of-simple.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/dwc3-of-simple.c 
b/drivers/usb/dwc3/dwc3-of-simple.c
index cb2ee96fd3e8..b9c869cd6585 100644
--- a/drivers/usb/dwc3/dwc3-of-simple.c
+++ b/drivers/usb/dwc3/dwc3-of-simple.c
@@ -165,8 +165,9 @@ static int dwc3_of_simple_remove(struct platform_device 
*pdev)
 
reset_control_put(simple->resets);
 
-   pm_runtime_put_sync(dev);
+   pm_runtime_put_noidle(dev);
pm_runtime_disable(dev);
+   pm_runtime_set_suspended(dev);
 
return 0;
 }
-- 
2.17.0




--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] usb: musb: remove an unused variable

2018-05-28 Thread Arnd Bergmann
After the only users of this variable got removed, we now get a
warning about 'otg' being unused:

drivers/usb/musb/da8xx.c: In function 'da8xx_musb_interrupt':
drivers/usb/musb/da8xx.c:226:19: error: unused variable 'otg' 
[-Werror=unused-variable]

Fixes: d2852f2d3e6d ("usb: musb: remove references to default_a of struct 
usb_otg")
Signed-off-by: Arnd Bergmann 
---
 drivers/usb/musb/da8xx.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
index 0e5929e81d26..1c023c0091c4 100644
--- a/drivers/usb/musb/da8xx.c
+++ b/drivers/usb/musb/da8xx.c
@@ -223,7 +223,6 @@ static irqreturn_t da8xx_musb_interrupt(int irq, void *hci)
 {
struct musb *musb = hci;
void __iomem*reg_base = musb->ctrl_base;
-   struct usb_otg  *otg = musb->xceiv->otg;
unsigned long   flags;
irqreturn_t ret = IRQ_NONE;
u32 status;
-- 
2.9.0

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/2] usb: dwc3: of_simple: don't call pm_runtime_set_active()

2018-05-28 Thread Roger Quadros
Don't call pm_runtime_set_active() as it will prevent the device
from being activated in the next pm_runtime_get_sync() call.

Also call pm_runtime_get_sync() before of_platform_populate().

Signed-off-by: Roger Quadros 
---
 drivers/usb/dwc3/dwc3-of-simple.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-of-simple.c 
b/drivers/usb/dwc3/dwc3-of-simple.c
index e98d221..2cbb5c0 100644
--- a/drivers/usb/dwc3/dwc3-of-simple.c
+++ b/drivers/usb/dwc3/dwc3-of-simple.c
@@ -121,6 +121,9 @@ static int dwc3_of_simple_probe(struct platform_device 
*pdev)
if (ret)
goto err_resetc_assert;
 
+   pm_runtime_enable(dev);
+   pm_runtime_get_sync(dev);
+
ret = of_platform_populate(np, NULL, NULL, dev);
if (ret) {
for (i = 0; i < simple->num_clocks; i++) {
@@ -131,10 +134,6 @@ static int dwc3_of_simple_probe(struct platform_device 
*pdev)
goto err_resetc_assert;
}
 
-   pm_runtime_set_active(dev);
-   pm_runtime_enable(dev);
-   pm_runtime_get_sync(dev);
-
return 0;
 
 err_resetc_assert:
-- 
cheers,
-roger

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/2] usb: dwc3: of-simple: Don't fail if no clock entries

2018-05-28 Thread Roger Quadros
of_count_phandle_with_args() returns -ENOENT (-2) if
there are no clock entries. Don't fail in such a case.

Signed-off-by: Roger Quadros 
---
 drivers/usb/dwc3/dwc3-of-simple.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-of-simple.c 
b/drivers/usb/dwc3/dwc3-of-simple.c
index cb2ee96..e98d221 100644
--- a/drivers/usb/dwc3/dwc3-of-simple.c
+++ b/drivers/usb/dwc3/dwc3-of-simple.c
@@ -36,11 +36,11 @@ static int dwc3_of_simple_clk_init(struct dwc3_of_simple 
*simple, int count)
struct device_node  *np = dev->of_node;
int i;
 
-   simple->num_clocks = count;
-
-   if (!count)
+   if (count <= 0)
return 0;
 
+   simple->num_clocks = count;
+
simple->clks = devm_kcalloc(dev, simple->num_clocks,
sizeof(struct clk *), GFP_KERNEL);
if (!simple->clks)
-- 
cheers,
-roger

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 2/3] platform: x86: intel_cht_int33fe: Fix dependencies

2018-05-28 Thread Andy Shevchenko
On Wed, 2018-05-23 at 17:37 +0300, Heikki Krogerus wrote:
> The driver will not probe unless bq24190 is loaded, so
> making it a dependency.
> 

Acked-by: Andy Shevchenko 

assuming it will go through some other tree.

> Signed-off-by: Heikki Krogerus 
> Cc: Wolfram Sang 
> Cc: Darren Hart 
> Cc: Andy Shevchenko 
> ---
>  drivers/i2c/busses/Kconfig   | 3 +--
>  drivers/platform/x86/Kconfig | 4 ++--
>  2 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 99edffae27f6..4f8df2ec87b1 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -202,8 +202,7 @@ config I2C_CHT_WC
>  
> Note this controller is hooked up to a TI bq24292i charger-
> IC,
> combined with a FUSB302 Type-C port-controller as such it
> is advised
> -   to also select CONFIG_CHARGER_BQ24190=m and
> CONFIG_TYPEC_FUSB302=m
> -   (the fusb302 driver currently is in drivers/staging).
> +   to also select CONFIG_TYPEC_FUSB302=m.
>  
>  config I2C_NFORCE2
>   tristate "Nvidia nForce2, nForce3 and nForce4"
> diff --git a/drivers/platform/x86/Kconfig
> b/drivers/platform/x86/Kconfig
> index 566644bb496a..f27cb186437d 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -866,6 +866,7 @@ config ACPI_CMPC
>  config INTEL_CHT_INT33FE
>   tristate "Intel Cherry Trail ACPI INT33FE Driver"
>   depends on X86 && ACPI && I2C && REGULATOR
> + depends on CHARGER_BQ24190=y || (CHARGER_BQ24190=m && m)
>   ---help---
> This driver add support for the INT33FE ACPI device found
> on
> some Intel Cherry Trail devices.
> @@ -877,8 +878,7 @@ config INTEL_CHT_INT33FE
> i2c drivers for these chips can bind to the them.
>  
> If you enable this driver it is advised to also select
> -   CONFIG_TYPEC_FUSB302=m, CONFIG_CHARGER_BQ24190=m and
> -   CONFIG_BATTERY_MAX17042=m.
> +   CONFIG_TYPEC_FUSB302=m and CONFIG_BATTERY_MAX17042=m.
>  
>  config INTEL_INT0002_VGPIO
>   tristate "Intel ACPI INT0002 Virtual GPIO driver"

-- 
Andy Shevchenko 
Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5] usb: gadget: udc: renesas_usb3: Add register of usb role switch

2018-05-28 Thread Heikki Krogerus
On Fri, May 25, 2018 at 04:12:56PM +0900, Yoshihiro Shimoda wrote:
> @@ -2573,6 +2635,12 @@ static void renesas_usb3_init_ram(struct renesas_usb3 
> *usb3, struct device *dev,
>   EXTCON_NONE,
>  };
>  
> +static struct usb_role_switch_desc renesas_usb3_role_switch_desc = {

You can constify that.

> + .set = renesas_usb3_role_switch_set,
> + .get = renesas_usb3_role_switch_get,
> + .allow_userspace_control = true,
> +};
> +
>  static int renesas_usb3_probe(struct platform_device *pdev)
>  {
>   struct renesas_usb3 *usb3;

Thanks,

-- 
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5] usb: gadget: udc: renesas_usb3: Add register of usb role switch

2018-05-28 Thread Andy Shevchenko
On Fri, May 25, 2018 at 10:12 AM, Yoshihiro Shimoda
 wrote:

> -static void usb3_set_mode(struct renesas_usb3 *usb3, bool host)

Wouldn't be better to choose another name for a new function?

> +   struct renesas_usb3 *usb3 = container_of(work, struct renesas_usb3,
> +role_work);

Matter of style, though I would rather put entire container_of() on
the next line (see for the existing style in the module and use it).

> +   /* This device_attach() might sleep */
> +   if (device_attach(host) < 0)
> +   dev_err(dev, "device_attach(usb3_port) failed\n");

can't be "usb3_port" part derived from the host variable somehow and
to some extend?

> +   usb3->role_sw = usb_role_switch_register(>dev,
> +   _usb3_role_switch_desc);
> +   if (!IS_ERR(usb3->role_sw)) {

> +   usb3->host_dev = usb_of_get_companion_dev(>dev);

Hmm... Can it possible return -EPROBE_DEFER? If so, would it be better
to use other approach to handle it?

> +   if (IS_ERR_OR_NULL(usb3->host_dev)) {
> +   /* If not found, this driver will not use a role sw */
> +   usb_role_switch_unregister(usb3->role_sw);
> +   usb3->role_sw = NULL;
> +   }
> +   } else {
> +   usb3->role_sw = NULL;
> +   }


-- 
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 07/12] usb: usbtmc: Add ioctl USBTMC488_IOCTL_WAIT_SRQ

2018-05-28 Thread guido


Zitat von Oliver Neukum :


Am Donnerstag, den 24.05.2018, 12:59 + schrieb guido@kiener-
muenchen.de:

Zitat von Oliver Neukum :

> Am Donnerstag, den 17.05.2018, 19:03 +0200 schrieb Guido Kiener:
> > +static int usbtmc488_ioctl_wait_srq(struct usbtmc_file_data *file_data,
> > +   unsigned int __user *arg)
> > +{
> > +   struct usbtmc_device_data *data = file_data->data;
> > +   struct device *dev = >intf->dev;
> > +   int rv;
> > +   unsigned int timeout;
> > +   unsigned long expire;
> > +
> > +   if (!data->iin_ep_present) {
> > +   dev_dbg(dev, "no interrupt endpoint present\n");
> > +   return -EFAULT;
> > +   }
> > +
> > +   if (get_user(timeout, arg))
> > +   return -EFAULT;
> > +
> > +   expire = msecs_to_jiffies(timeout);
> > +
> > +   mutex_unlock(>io_mutex);
>
> There is such a thing as threads sharing file descriptors.
> That leads to the question what happens to the mutex if this
> ioctl() is called multiple times.
>
>Regards
>Oliver

Multiple threads can wait with the same or different file descriptors.
When an SRQ interrupt occurs, all threads and file descriptors are
informed concurrently with wake_up_interruptible_all(>waitq);
The "_all" is already fixed in 02/12.


No, the problem is that you will underflow io->mutex


Don't worry. The function usbtmc488_ioctl_wait_srq is called by
usbtmc_ioctl which already locks the mutex. See
https://elixir.bootlin.com/linux/v4.17-rc7/source/drivers/usb/class/usbtmc.c#L1189
So the mutex is just unlocked here to allow other threads to still communicate
with the device.

Regards,

Guido

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 1/3] usb: gadget: ccid: add support for USB CCID Gadget Device

2018-05-28 Thread Andrzej Pietrasiewicz
W dniu 28.05.2018 o 11:32, Marcus Folkesson pisze:
> Hi Andrzej,
> 
> Thank you for reviewing.
> 
> On Mon, May 28, 2018 at 11:12:27AM +0200, Andrzej Pietrasiewicz wrote:
>> W dniu 28.05.2018 o 10:38, Marcus Folkesson pisze:
>>> Hi Andrzej,
>>>
>>> On Mon, May 28, 2018 at 09:04:51AM +0200, Andrzej Pietrasiewicz wrote:
 Mi Marcus,

 W dniu 26.05.2018 o 23:19, Marcus Folkesson pisze:
> Chip Card Interface Device (CCID) protocol is a USB protocol that
> allows a smartcard device to be connected to a computer via a card
> reader using a standard USB interface, without the need for each 
> manufacturer
> of smartcards to provide its own reader or protocol.
>
> This gadget driver makes Linux show up as a CCID device to the host and 
> let a
> userspace daemon act as the smartcard.
>
> This is useful when the Linux gadget itself should act as a cryptographic
> device or forward APDUs to an embedded smartcard device.
>
> Signed-off-by: Marcus Folkesson 
> ---

> 
> +config USB_CONFIGFS_CCID
> + bool "Chip Card Interface Device (CCID)"
> + depends on USB_CONFIGFS
> + select USB_F_CCID
> + help
> +   The CCID function driver provides generic emulation of a
> +   Chip Card Interface Device (CCID).
> +
> +   You will need a user space server talking to /dev/ccidg*,
> +   since the kernel itself does not implement CCID/TPDU/APDU
> +   protocol.

 Your function needs a userspace daemon to work.
 It seems you want to use FunctionFS for such a purpose
 instead of creating a new function.

 Andrzej
>>>
> +   since the kernel itself does not implement CCID/TPDU/APDU
>>> Oops, the driver does handle CCID.
>>
>> Which parts of code do this handling?
> 
> My bad, I was thinking about the USB descriptors and endpoints setup.
> That is of cause not part of the CCID protocol.
> 
>>
>> Is there any kind of state machine usual for protocols?
>> If the protocol is stateless then isn't it just a data format then?
> 
> The protocol is stateless.
> 
>>
>> Which part of this handling must be done in kernel and why?
>>
>> Does the said handling do anything other than forwarding the
>> traffic between USB and a character device?
> 
> No, it forward the CCID messages to the character device to be handled
> by the application.
> 
>>

My opinion is: this wants to be done with FunctionFS.

Andrzej
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 1/3] usb: gadget: ccid: add support for USB CCID Gadget Device

2018-05-28 Thread Marcus Folkesson
Hi Andrzej,

Thank you for reviewing.

On Mon, May 28, 2018 at 11:12:27AM +0200, Andrzej Pietrasiewicz wrote:
> W dniu 28.05.2018 o 10:38, Marcus Folkesson pisze:
> > Hi Andrzej,
> > 
> > On Mon, May 28, 2018 at 09:04:51AM +0200, Andrzej Pietrasiewicz wrote:
> >> Mi Marcus,
> >>
> >> W dniu 26.05.2018 o 23:19, Marcus Folkesson pisze:
> >>> Chip Card Interface Device (CCID) protocol is a USB protocol that
> >>> allows a smartcard device to be connected to a computer via a card
> >>> reader using a standard USB interface, without the need for each 
> >>> manufacturer
> >>> of smartcards to provide its own reader or protocol.
> >>>
> >>> This gadget driver makes Linux show up as a CCID device to the host and 
> >>> let a
> >>> userspace daemon act as the smartcard.
> >>>
> >>> This is useful when the Linux gadget itself should act as a cryptographic
> >>> device or forward APDUs to an embedded smartcard device.
> >>>
> >>> Signed-off-by: Marcus Folkesson 
> >>> ---
> >>
> >>>
> >>> +config USB_CONFIGFS_CCID
> >>> + bool "Chip Card Interface Device (CCID)"
> >>> + depends on USB_CONFIGFS
> >>> + select USB_F_CCID
> >>> + help
> >>> +   The CCID function driver provides generic emulation of a
> >>> +   Chip Card Interface Device (CCID).
> >>> +
> >>> +   You will need a user space server talking to /dev/ccidg*,
> >>> +   since the kernel itself does not implement CCID/TPDU/APDU
> >>> +   protocol.
> >>
> >> Your function needs a userspace daemon to work.
> >> It seems you want to use FunctionFS for such a purpose
> >> instead of creating a new function.
> >>
> >> Andrzej
> > 
> >>> +   since the kernel itself does not implement CCID/TPDU/APDU
> > Oops, the driver does handle CCID.
> 
> Which parts of code do this handling?

My bad, I was thinking about the USB descriptors and endpoints setup.
That is of cause not part of the CCID protocol.

> 
> Is there any kind of state machine usual for protocols?
> If the protocol is stateless then isn't it just a data format then?

The protocol is stateless.

> 
> Which part of this handling must be done in kernel and why?
> 
> Does the said handling do anything other than forwarding the
> traffic between USB and a character device?

No, it forward the CCID messages to the character device to be handled
by the application.

> 
> What is the character device used for? I know: read, write and poll.
> But why? To do what?

It is used for the application to fetch, interpret and then perform actions 
depending on
commands.

> 
> > 
> > Well, yes, It needs an application that perform the "smartcard operations", 
> > such as
> > generate keys or sign data, as this depends on how it should be used.
> > 
> > The actual smartcard operations could for example be in software,
> > use a crypto engine in SoC or external HSM (Hardware Security Module).
> > 
> > Without the application, the gadget shows up as a smart card reader
> > with an unconnected smartcard.
> > 
> 
> Does showing up as anything require anything other than merely
> providing USB descriptors?

I guess.

> 
> Andrzej

Thank you,
Marcus
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 1/3] usb: gadget: ccid: add support for USB CCID Gadget Device

2018-05-28 Thread Andrzej Pietrasiewicz
W dniu 28.05.2018 o 10:38, Marcus Folkesson pisze:
> Hi Andrzej,
> 
> On Mon, May 28, 2018 at 09:04:51AM +0200, Andrzej Pietrasiewicz wrote:
>> Mi Marcus,
>>
>> W dniu 26.05.2018 o 23:19, Marcus Folkesson pisze:
>>> Chip Card Interface Device (CCID) protocol is a USB protocol that
>>> allows a smartcard device to be connected to a computer via a card
>>> reader using a standard USB interface, without the need for each 
>>> manufacturer
>>> of smartcards to provide its own reader or protocol.
>>>
>>> This gadget driver makes Linux show up as a CCID device to the host and let 
>>> a
>>> userspace daemon act as the smartcard.
>>>
>>> This is useful when the Linux gadget itself should act as a cryptographic
>>> device or forward APDUs to an embedded smartcard device.
>>>
>>> Signed-off-by: Marcus Folkesson 
>>> ---
>>
>>>
>>> +config USB_CONFIGFS_CCID
>>> +   bool "Chip Card Interface Device (CCID)"
>>> +   depends on USB_CONFIGFS
>>> +   select USB_F_CCID
>>> +   help
>>> + The CCID function driver provides generic emulation of a
>>> + Chip Card Interface Device (CCID).
>>> +
>>> + You will need a user space server talking to /dev/ccidg*,
>>> + since the kernel itself does not implement CCID/TPDU/APDU
>>> + protocol.
>>
>> Your function needs a userspace daemon to work.
>> It seems you want to use FunctionFS for such a purpose
>> instead of creating a new function.
>>
>> Andrzej
> 
>>> + since the kernel itself does not implement CCID/TPDU/APDU
> Oops, the driver does handle CCID.

Which parts of code do this handling?

Is there any kind of state machine usual for protocols?
If the protocol is stateless then isn't it just a data format then?

Which part of this handling must be done in kernel and why?

Does the said handling do anything other than forwarding the
traffic between USB and a character device?

What is the character device used for? I know: read, write and poll.
But why? To do what?

> 
> Well, yes, It needs an application that perform the "smartcard operations", 
> such as
> generate keys or sign data, as this depends on how it should be used.
> 
> The actual smartcard operations could for example be in software,
> use a crypto engine in SoC or external HSM (Hardware Security Module).
> 
> Without the application, the gadget shows up as a smart card reader
> with an unconnected smartcard.
> 

Does showing up as anything require anything other than merely
providing USB descriptors?

Andrzej
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] net: qmi_wwan: Add Netgear Aircard 779S

2018-05-28 Thread Bjørn Mork
Josh Hill  writes:

> Add support for Netgear Aircard 779S
>
> Signed-off-by: Josh Hill 

Acked-by: Bjørn Mork 

Please queue this for stable too.  Thanks.


Bjørn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 1/3] usb: gadget: ccid: add support for USB CCID Gadget Device

2018-05-28 Thread Marcus Folkesson
Hi Andrzej,

On Mon, May 28, 2018 at 09:04:51AM +0200, Andrzej Pietrasiewicz wrote:
> Mi Marcus,
> 
> W dniu 26.05.2018 o 23:19, Marcus Folkesson pisze:
> > Chip Card Interface Device (CCID) protocol is a USB protocol that
> > allows a smartcard device to be connected to a computer via a card
> > reader using a standard USB interface, without the need for each 
> > manufacturer
> > of smartcards to provide its own reader or protocol.
> > 
> > This gadget driver makes Linux show up as a CCID device to the host and let 
> > a
> > userspace daemon act as the smartcard.
> > 
> > This is useful when the Linux gadget itself should act as a cryptographic
> > device or forward APDUs to an embedded smartcard device.
> > 
> > Signed-off-by: Marcus Folkesson 
> > ---
> 
> >   
> > +config USB_CONFIGFS_CCID
> > +   bool "Chip Card Interface Device (CCID)"
> > +   depends on USB_CONFIGFS
> > +   select USB_F_CCID
> > +   help
> > + The CCID function driver provides generic emulation of a
> > + Chip Card Interface Device (CCID).
> > +
> > + You will need a user space server talking to /dev/ccidg*,
> > + since the kernel itself does not implement CCID/TPDU/APDU
> > + protocol.
> 
> Your function needs a userspace daemon to work.
> It seems you want to use FunctionFS for such a purpose
> instead of creating a new function.
> 
> Andrzej

> > + since the kernel itself does not implement CCID/TPDU/APDU
Oops, the driver does handle CCID.

Well, yes, It needs an application that perform the "smartcard operations", 
such as
generate keys or sign data, as this depends on how it should be used.

The actual smartcard operations could for example be in software,
use a crypto engine in SoC or external HSM (Hardware Security Module).

Without the application, the gadget shows up as a smart card reader
with an unconnected smartcard.

I guess it could be accomplished with FunctionFS as well.

Best regards
Marcus Folkesson

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Serdev: USB device and sysdev probing ala i2c

2018-05-28 Thread Johan Hovold
On Fri, May 25, 2018 at 10:17:20AM -0500, Rob Herring wrote:
> On Fri, May 25, 2018 at 7:08 AM, Johan Hovold  wrote:
> > On Thu, May 24, 2018 at 11:49:24AM -0500, Rob Herring wrote:
> >> On Thu, May 24, 2018 at 7:18 AM, Ricardo Ribalda Delgado
> >>  wrote:
> >> > Hi Johan,
> >> >
> >> > On Thu, May 24, 2018 at 2:07 PM Johan Hovold  wrote:
> >
> >> >> Serdev currently only supports device tree and ACPI. Using out-of-tree
> >> >> code, you could load a device tree fragment during runtime to describe
> >> >> your serial bus (or you just amend the device tree).
> >> >
> >> >> Using device tree overlays would have the benefit of being able to
> >> >> describe associated resources (e.g. reset gpios) which a simple
> >> >> compatible string (or equivalent) would not.
> >> >
> >> >> But there are examples where a simple compatible string would do, for
> >> >> example an existing CEC device presenting itself as a generic USB CDC
> >> >> device (hopefully with a dedicated VID/PID so that no user-space
> >> >> configuration is needed at all).
> >
> >> The fundamental problem here is you need a parent device node to apply
> >> a DT overlay to and a USB device hotplugged has no DT device node. The
> >> system you are running on may not even have a DT (like a PC). If you
> >> have an overlay of the downstream devices, they have to be a child of
> >> something for the overlay to apply to. We could just create virtual
> >> device nodes for the purposes of applying overlays to. Another option
> >> would be allowing multiple DTs. Then you aren't even using overlays
> >> (what's the point of an overlay when a system has no real DT to begin
> >> with). That also would mean they are completely independent from any
> >> real DT or other instances (you may want to plug in multiple of the
> >> same device).
> >
> > Right, there's more (than just a DT overlay loader) that needs to be in
> > place before this could be used for the generic hotplug case (and even
> > more than that if this was to be usable for ACPI systems).
> >
> > I think generating DT nodes during enumeration is preferable to having
> > detached trees, if only to deal with the case where a loaded overlay do
> > overlap with the "real" static tree.
> 
> Generating nodes effectively means we're implementing the full USB
> tree as defined for OpenFirmware[1]. I'd prefer to not go there. That
> seems a bit pointless as for most of the devices we don't care and
> there's really nothing related to USB we care about. We just need to
> describe downstream functions. The USB device (or interface) just
> happens to be the controller/provider for those functions.
> 
> There should only be overlap with a real tree if devices are soldered
> onto a board or use a custom connector and you have other sideband
> signals.

Right, we may not want to do it, but having too many mechanisms for
doing the same thing can get messy (but so are overlays to begin with, I
guess).

What we have today (and with the USB serial device tree patches) gets us
a long way by covering static setups. And before supporting the generic
hotplug case with sideband signals (where the descriptive power of
device-tree overlays would be handy) it may be better to focus on the
subset where just a compatible string (or equivalent) would do.

The USB CEC device I mentioned above would be a good example; and an
always-on or DTR/RTS power-controlled GPS could be another.

> > Another problem is that we need to deregister any tty device and
> > essentially reprobe the underlying serial controller whenever user space
> > tells us what can be found on the other end of wire in order to register
> > a serdev controller and device instead. IIRC this is something we would
> > get for free if using DT overlays (i.e. any device with a modified DT
> > node would get removed and readded, or at least notified that something
> > changed).
> 
> I did some experiments in this area. Marcel wanted to make all BT
> drivers serdev only and then make the BT ldisc just create serdev
> devices. And all this would be transparent to existing BT userspace.
> As part of this I modified registration to register both serdev ctrlr
> and tty char device and allow adding slave devices later. It's up on
> my serdev-ldisc-v2 branch in all its hacky glory.

Yeah, I remember you mentioning something about that, but I never dared
to look at the code. ;)

> I don't think we get anything for free with overlays. I think we only
> handle platform devices and only new nodes added. There are notifiers
> for modifications, but if no one is listening modifications won't have
> any effect.

Ok, then using DT overlays sounds like too much work for too little gain
in any case. The only use case that comes to mind for hotplug + sideband
would be to allow people to tinker with evaluation kits (e.g. for a new
GPS) on a PC, but they would probably be better off using an embedded
system such as the BBB 

Re: 4.16 issue with mbim modem and ping with size > 14552 bytes

2018-05-28 Thread Greg KH
On Mon, May 28, 2018 at 09:58:01AM +0200, Daniele Palmas wrote:
> 2018-05-25 0:54 GMT+02:00 Daniele Palmas :
> > Hi Greg,
> >
> > 2018-05-24 17:53 GMT+02:00 Greg KH :
> >> On Thu, May 24, 2018 at 05:04:49PM +0200, Daniele Palmas wrote:
> >>> Hello,
> >>>
> >>> I have an issue with an USB mbim modem when trying to send with ping
> >>> more than 14552 bytes: it looks like to me a kernel issue, but not at
> >>> the cdc_mbim or cdc_ncm level, anyway not sure, so I'm reporting the
> >>> issue.
> >>>
> >>> My kernel is 4.16. The device is the following:
> >>
> >> Does older kernels work, or is this something that has always been
> >> there?
> >>
> >
> > Not tested yet, I'm going to do.
> >
> 
> So, ping with more than 14552 was working properly until v4.5-rc7:
> starting from v4.5 I'm not even able to make a data connection with
> mbim since things fail badly with the following:
> 
> [  259.551836] [ cut here ]
> [  259.551848] WARNING: CPU: 2 PID: 0 at
> /home/kernel/COD/linux/net/sched/sch_generic.c:303
> dev_watchdog+0x237/0x240()
> [  259.551860] NETDEV WATCHDOG: wwp0s20u7i2 (cdc_mbim): transmit queue
> 0 timed out
> [  259.551861] Modules linked in: cdc_mbim cdc_wdm cdc_ncm usbnet mii
> intel_rapl x86_pkg_temp_thermal intel_powerclamp coretemp kvm
> snd_hda_codec_realtek snd_hda_codec_hdmi snd_hda_codec_generic
> irqbypass crct10dif_pclmul snd_hda_intel snd_hda_codec snd_hda_core
> crc32_pclmul snd_hwdep ghash_clmulni_intel aesni_intel aes_x86_64
> snd_pcm snd_seq_midi lrw snd_seq_midi_event gf128mul input_leds
> snd_rawmidi snd_seq glue_helper snd_seq_device ablk_helper snd_timer
> cryptd snd soundcore shpchp serio_raw mac_hid lpc_ich 8250_fintek
> mei_me mei parport_pc ppdev lp parport autofs4 hid_generic usbhid hid
> i915 i2c_algo_bit drm_kms_helper syscopyarea sysfillrect psmouse
> sysimgblt ahci fb_sys_fops e1000e libahci drm ptp pps_core wmi fjes
> video
> [  259.551889] CPU: 2 PID: 0 Comm: swapper/2 Not tainted
> 4.5.0-040500-generic #201603140130
> [  259.551890] Hardware name: LENOVO 10A6A0J5IX/SHARKBAY, BIOS
> FBKT79AUS 04/17/2014
> [  259.551891]  0286 108c91d75cf5b65f 88021eb03d98
> 813e0233
> [  259.551893]  88021eb03de0 81d816a0 88021eb03dd0
> 81080e72
> [  259.551894]   8800cee81880 0002
> 8800a19f8000
> [  259.551895] Call Trace:
> [  259.551896][] dump_stack+0x63/0x90
> [  259.551903]  [] warn_slowpath_common+0x82/0xc0
> [  259.551904]  [] warn_slowpath_fmt+0x5c/0x80
> [  259.551907]  [] dev_watchdog+0x237/0x240
> [  259.551909]  [] ? qdisc_rcu_free+0x40/0x40
> [  259.551910]  [] call_timer_fn+0x35/0x120
> [  259.551911]  [] ? qdisc_rcu_free+0x40/0x40
> [  259.551912]  [] run_timer_softirq+0x246/0x2f0
> [  259.551914]  [] __do_softirq+0xf6/0x280
> [  259.551916]  [] irq_exit+0xa3/0xb0
> [  259.551919]  [] smp_apic_timer_interrupt+0x42/0x50
> [  259.551920]  [] apic_timer_interrupt+0x82/0x90
> [  259.551922][] ? cpuidle_enter_state+0x11d/0x2c0
> [  259.551925]  [] cpuidle_enter+0x17/0x20
> [  259.551928]  [] call_cpuidle+0x2a/0x40
> [  259.551929]  [] cpu_startup_entry+0x295/0x350
> [  259.551931]  [] start_secondary+0x15e/0x190
> [  259.551933] ---[ end trace 6f8d3c1a1b02644d ]---
> 
> but this is probably something different, cause with v4.16 data
> connection works fine.
> 
> If this is not helpful I guess I need to bisect.

Bisection would be best.  It looks like you narrowed things down really
well already, bisection should go very quickly.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: 4.16 issue with mbim modem and ping with size > 14552 bytes

2018-05-28 Thread Daniele Palmas
2018-05-25 0:54 GMT+02:00 Daniele Palmas :
> Hi Greg,
>
> 2018-05-24 17:53 GMT+02:00 Greg KH :
>> On Thu, May 24, 2018 at 05:04:49PM +0200, Daniele Palmas wrote:
>>> Hello,
>>>
>>> I have an issue with an USB mbim modem when trying to send with ping
>>> more than 14552 bytes: it looks like to me a kernel issue, but not at
>>> the cdc_mbim or cdc_ncm level, anyway not sure, so I'm reporting the
>>> issue.
>>>
>>> My kernel is 4.16. The device is the following:
>>
>> Does older kernels work, or is this something that has always been
>> there?
>>
>
> Not tested yet, I'm going to do.
>

So, ping with more than 14552 was working properly until v4.5-rc7:
starting from v4.5 I'm not even able to make a data connection with
mbim since things fail badly with the following:

[  259.551836] [ cut here ]
[  259.551848] WARNING: CPU: 2 PID: 0 at
/home/kernel/COD/linux/net/sched/sch_generic.c:303
dev_watchdog+0x237/0x240()
[  259.551860] NETDEV WATCHDOG: wwp0s20u7i2 (cdc_mbim): transmit queue
0 timed out
[  259.551861] Modules linked in: cdc_mbim cdc_wdm cdc_ncm usbnet mii
intel_rapl x86_pkg_temp_thermal intel_powerclamp coretemp kvm
snd_hda_codec_realtek snd_hda_codec_hdmi snd_hda_codec_generic
irqbypass crct10dif_pclmul snd_hda_intel snd_hda_codec snd_hda_core
crc32_pclmul snd_hwdep ghash_clmulni_intel aesni_intel aes_x86_64
snd_pcm snd_seq_midi lrw snd_seq_midi_event gf128mul input_leds
snd_rawmidi snd_seq glue_helper snd_seq_device ablk_helper snd_timer
cryptd snd soundcore shpchp serio_raw mac_hid lpc_ich 8250_fintek
mei_me mei parport_pc ppdev lp parport autofs4 hid_generic usbhid hid
i915 i2c_algo_bit drm_kms_helper syscopyarea sysfillrect psmouse
sysimgblt ahci fb_sys_fops e1000e libahci drm ptp pps_core wmi fjes
video
[  259.551889] CPU: 2 PID: 0 Comm: swapper/2 Not tainted
4.5.0-040500-generic #201603140130
[  259.551890] Hardware name: LENOVO 10A6A0J5IX/SHARKBAY, BIOS
FBKT79AUS 04/17/2014
[  259.551891]  0286 108c91d75cf5b65f 88021eb03d98
813e0233
[  259.551893]  88021eb03de0 81d816a0 88021eb03dd0
81080e72
[  259.551894]   8800cee81880 0002
8800a19f8000
[  259.551895] Call Trace:
[  259.551896][] dump_stack+0x63/0x90
[  259.551903]  [] warn_slowpath_common+0x82/0xc0
[  259.551904]  [] warn_slowpath_fmt+0x5c/0x80
[  259.551907]  [] dev_watchdog+0x237/0x240
[  259.551909]  [] ? qdisc_rcu_free+0x40/0x40
[  259.551910]  [] call_timer_fn+0x35/0x120
[  259.551911]  [] ? qdisc_rcu_free+0x40/0x40
[  259.551912]  [] run_timer_softirq+0x246/0x2f0
[  259.551914]  [] __do_softirq+0xf6/0x280
[  259.551916]  [] irq_exit+0xa3/0xb0
[  259.551919]  [] smp_apic_timer_interrupt+0x42/0x50
[  259.551920]  [] apic_timer_interrupt+0x82/0x90
[  259.551922][] ? cpuidle_enter_state+0x11d/0x2c0
[  259.551925]  [] cpuidle_enter+0x17/0x20
[  259.551928]  [] call_cpuidle+0x2a/0x40
[  259.551929]  [] cpu_startup_entry+0x295/0x350
[  259.551931]  [] start_secondary+0x15e/0x190
[  259.551933] ---[ end trace 6f8d3c1a1b02644d ]---

but this is probably something different, cause with v4.16 data
connection works fine.

If this is not helpful I guess I need to bisect.

Thanks,
Daniele

>> I ask, as my mobile provider does horrible things to large packet sizes.
>> So much so that I have to set the mtu to 1280 just to get things to work
>> properly when tethering my phone through to my laptop.  So this might be
>> a network provider issue :)
>>
>
> Yeah, I thought the same, so I tried the same scenario with Windows 10
> but it is working fine.
>
> Thanks,
> Daniele
>
>> thanks,
>>
>> greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 2/3] Documentation: usb: add documentation for USB CCID Gadget Device

2018-05-28 Thread Marcus Folkesson
Hi Randy,

On Sun, May 27, 2018 at 04:36:24PM -0700, Randy Dunlap wrote:
> Hi,
> 
> I have a few documentation comments below...
> 
> On 05/26/2018 02:19 PM, Marcus Folkesson wrote:
> > Add documentation to give a brief description on how to use the
> > CCID Gadget Device.
> > This includes a description for all attributes followed by an example on
> > how to setup the device with ConfigFS.
> > 
> > Signed-off-by: Marcus Folkesson 
> > ---
> >  Documentation/usb/gadget_ccid.rst | 267 
> > ++
> >  1 file changed, 267 insertions(+)
> >  create mode 100644 Documentation/usb/gadget_ccid.rst
> > 
> > diff --git a/Documentation/usb/gadget_ccid.rst 
> > b/Documentation/usb/gadget_ccid.rst
> > new file mode 100644
> > index ..5ac806b14604
> > --- /dev/null
> > +++ b/Documentation/usb/gadget_ccid.rst
> > @@ -0,0 +1,267 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +
> > +
> > +CCID Gadget
> > +
> > +
> > +:Author: Marcus Folkesson 
> > +
> > +Introduction
> > +
> > +
> > +The CCID Gadget will present itself as a CCID device to the host system.
> > +The device supports two endpoints for now; BULK IN and BULK OUT.
> > +These endpoints is exposed to userspace via /dev/ccidg*.
> 
>are exposed
> 
> > +
> > +All CCID commands are sent on the BULK-OUT endpoint. Each command sent to 
> > the CCID
> > +has an associated ending response. Some commands can also have intermediate
> > +responses. The response is sent on the BULK-IN endpoint.
> > +See Figure 3-3 in the CCID Specification [1]_ for more details.
> > +
> > +The CCID commands must be handled in userspace since the driver is only 
> > working
> > +as a transport layer for the TPDUs.
> > +
> > +
> > +CCID Commands
> > +--
> > +
> > +All CCID commands begins with a 10 bytes header followed by an optional
> 
> with a 10-byte header
> (or maybe that's a locale difference)
> 
> > +data field depending on message type.
> > +
> > +++--+---+--+
> > +| Offset | Field| Size  | Description  |
> > +++==+===+==+
> > +| 0  | bMessageType | 1 | Type of message  |
> > +++--+---+--+
> > +| 1  | dwLength | 4 | Message specific data length |
> > +||  |   |  |
> > +++--+---+--+
> > +| 5  | bSlot| 1 | Identifies the slot number   |
> > +||  |   | for this command |
> > +++--+---+--+
> > +| 6  | bSeq | 1 | Sequence number for command  |
> > +++--+---+--+
> > +| 7  | ...  | 3 | Fields depends on message type   |
> > +++--+---+--+
> > +| 10 | abData   | array | Message specific data (OPTIONAL) |
> > +++--+---+--+
> > +
> > +
> > +Multiple CCID gadgets
> > +--
> > +
> > +It is possible to create multiple instances of the CCID gadget, however,
> > +a much more flexible way is to create one gadget and set the `nslots` 
> > attribute
> > +to the number of desired CCID devices.
> > +
> > +All CCID commands specifies which slot that is the receiver in the `bSlot` 
> > field
> 
>  specify which slot is the receiver
> 
> > +of the CCID header.
> > +
> > +Usage
> > +=
> > +
> > +Access from userspace
> > +--
> > +All communication is by read(2) and write(2) to the corresponding 
> > /dev/ccidg* device.
> > +Only one filedescriptor is allowed to be open to the device at a time.
> 
> file descriptor
> 
> > +
> > +The buffer size provided to read(2) **must be at least** 522 (10 bytes 
> > header + 512 bytes payload)
> > +bytes as we are working with whole commands.
> > +
> > +The buffer size provided to write(2) **may not exceed** 522 (10 bytes 
> > header + 512 bytes payload)
> > +bytes as we are working with whole commands.
> > +
> > +
> > +Configuration with configfs
> > +
> > +
> > +ConfigFS is used to create and configure the CCID gadget.
> > +In order to get a device to work as intended, a few attributes must
> > +be considered.
> > +
> > +The attributes is described below followed by an example.
> 
>   are
> 
> > +
> > +features
> > +~
> > +
> > +The `feature` attribute writes to the dwFeatures field in the class 
> > descriptor.
> > +See Table 5.1-1 Smart Card Device Descriptors in the CCID Specification 
> 

Re: [PATCH v2 1/3] usb: gadget: ccid: add support for USB CCID Gadget Device

2018-05-28 Thread Andrzej Pietrasiewicz
Mi Marcus,

W dniu 26.05.2018 o 23:19, Marcus Folkesson pisze:
> Chip Card Interface Device (CCID) protocol is a USB protocol that
> allows a smartcard device to be connected to a computer via a card
> reader using a standard USB interface, without the need for each manufacturer
> of smartcards to provide its own reader or protocol.
> 
> This gadget driver makes Linux show up as a CCID device to the host and let a
> userspace daemon act as the smartcard.
> 
> This is useful when the Linux gadget itself should act as a cryptographic
> device or forward APDUs to an embedded smartcard device.
> 
> Signed-off-by: Marcus Folkesson 
> ---

>   
> +config USB_CONFIGFS_CCID
> + bool "Chip Card Interface Device (CCID)"
> + depends on USB_CONFIGFS
> + select USB_F_CCID
> + help
> +   The CCID function driver provides generic emulation of a
> +   Chip Card Interface Device (CCID).
> +
> +   You will need a user space server talking to /dev/ccidg*,
> +   since the kernel itself does not implement CCID/TPDU/APDU
> +   protocol.

Your function needs a userspace daemon to work.
It seems you want to use FunctionFS for such a purpose
instead of creating a new function.

Andrzej
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCHv2 1/2] usb: hub: Per-port setting to use old enumeration scheme

2018-05-28 Thread Nicolas Boichat
The "old" enumeration scheme is considerably faster (it takes
~244ms instead of ~356ms to get the descriptor).

It is currently only possible to use the old scheme globally
(/sys/module/usbcore/parameters/old_scheme_first), which is not
desirable as the new scheme was introduced to increase compatibility
with more devices.

However, in our case, we care about time-to-active for a specific
USB device (which we make the firmware for), on a specific port
(that is pogo-pin based: not a standard USB port). This new
sysfs option makes it possible to use the old scheme on a single
port only.

Signed-off-by: Nicolas Boichat 
---

Changes since v1:
 - Added documentation in Documentation/ABI
 - Updated timing in commit message to account for recent improvement
   in USB core (74072bae88fb3b)

 Documentation/ABI/testing/sysfs-bus-usb | 18 ++
 drivers/usb/core/hub.c  | 13 +
 drivers/usb/core/hub.h  |  1 +
 drivers/usb/core/port.c | 23 +++
 include/linux/usb.h |  7 +++
 5 files changed, 58 insertions(+), 4 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-usb 
b/Documentation/ABI/testing/sysfs-bus-usb
index c6e9b30f05b13..a31a66d62cbba 100644
--- a/Documentation/ABI/testing/sysfs-bus-usb
+++ b/Documentation/ABI/testing/sysfs-bus-usb
@@ -189,6 +189,24 @@ Description:
The file will read "hotplug", "wired" and "not used" if the
information is available, and "unknown" otherwise.
 
+What:  /sys/bus/usb/devices/.../(hub interface)/portX/quirks
+Date:  May 2018
+Contact:   Nicolas Boichat 
+Description:
+   In some cases, we care about time-to-active for devices
+   connected on a specific port (e.g. non-standard USB port like
+   pogo pins), where the device to be connected is known in
+   advance, and behaves well according to the specification.
+   This attribute is a bit-field that controls the behavior of
+   a specific port:
+- Bit 0 of this field selects the "old" enumeration scheme,
+  as it is considerably faster (it only causes one USB reset
+  instead of 2).
+  The old enumeration scheme can also be selected globally
+  using /sys/module/usbcore/parameters/old_scheme_first, but
+  it is often not desirable as the new scheme was introduced to
+  increase compatibility with more devices.
+
 What:  /sys/bus/usb/devices/.../(hub 
interface)/portX/over_current_count
 Date:  February 2018
 Contact:   Richard Leitner 
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index c2d993d3816f0..f900f66a62856 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2636,7 +2636,7 @@ static unsigned hub_is_wusb(struct usb_hub *hub)
 #define SET_ADDRESS_TRIES  2
 #define GET_DESCRIPTOR_TRIES   2
 #define SET_CONFIG_TRIES   (2 * (use_both_schemes + 1))
-#define USE_NEW_SCHEME(i)  ((i) / 2 == (int)old_scheme_first)
+#define USE_NEW_SCHEME(i, scheme)  ((i) / 2 == (int)scheme)
 
 #define HUB_ROOT_RESET_TIME60  /* times are in msec */
 #define HUB_SHORT_RESET_TIME   10
@@ -2651,12 +2651,16 @@ static unsigned hub_is_wusb(struct usb_hub *hub)
  * enumeration failures, so disable this enumeration scheme for USB3
  * devices.
  */
-static bool use_new_scheme(struct usb_device *udev, int retry)
+static bool use_new_scheme(struct usb_device *udev, int retry,
+  struct usb_port *port_dev)
 {
+   int old_scheme_first_port =
+   port_dev->quirks & USB_PORT_QUIRK_OLD_SCHEME;
+
if (udev->speed >= USB_SPEED_SUPER)
return false;
 
-   return USE_NEW_SCHEME(retry);
+   return USE_NEW_SCHEME(retry, old_scheme_first_port || old_scheme_first);
 }
 
 /* Is a USB 3.0 port in the Inactive or Compliance Mode state?
@@ -4392,6 +4396,7 @@ hub_port_init(struct usb_hub *hub, struct usb_device 
*udev, int port1,
 {
struct usb_device   *hdev = hub->hdev;
struct usb_hcd  *hcd = bus_to_hcd(hdev->bus);
+   struct usb_port *port_dev = hub->ports[port1 - 1];
int retries, operations, retval, i;
unsigneddelay = HUB_SHORT_RESET_TIME;
enum usb_device_speed   oldspeed = udev->speed;
@@ -4513,7 +4518,7 @@ hub_port_init(struct usb_hub *hub, struct usb_device 
*udev, int port1,
for (retries = 0; retries < GET_DESCRIPTOR_TRIES; (++retries, 
msleep(100))) {
bool did_new_scheme = false;
 
-   if (use_new_scheme(udev, retry_counter)) {
+   if (use_new_scheme(udev, retry_counter, port_dev)) {
struct usb_device_descriptor *buf;

[PATCH] usb: hub: Per-port setting to reduce TRSTRCY to 10 ms

2018-05-28 Thread Nicolas Boichat
Currently, the USB hub core waits for 50 ms after enumerating the
device. This was added to help "some high speed devices" to
enumerate (b789696af8 "[PATCH] USB: relax usbcore reset timings").

On some devices, the time-to-active is important, so we provide
a per-port option to reduce the time to what the USB specification
requires: 10 ms.

Signed-off-by: Nicolas Boichat 
---

Reduces enumeration time by ~40ms in conjunction with previous
patch/quirk (or ~80ms on its own).

 Documentation/ABI/testing/sysfs-bus-usb | 4 
 drivers/usb/core/hub.c  | 6 +-
 include/linux/usb.h | 3 +++
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-usb 
b/Documentation/ABI/testing/sysfs-bus-usb
index a31a66d62cbba..08d456e07b538 100644
--- a/Documentation/ABI/testing/sysfs-bus-usb
+++ b/Documentation/ABI/testing/sysfs-bus-usb
@@ -206,6 +206,10 @@ Description:
   using /sys/module/usbcore/parameters/old_scheme_first, but
   it is often not desirable as the new scheme was introduced to
   increase compatibility with more devices.
+- Bit 1 reduces TRSTRCY to the 10 ms that are required by the
+  USB 2.0 specification, instead of the 50 ms that are normally
+  used to help make enumeration work better on some high speed
+  devices.
 
 What:  /sys/bus/usb/devices/.../(hub 
interface)/portX/over_current_count
 Date:  February 2018
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index f900f66a62856..26c2438d28893 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2879,7 +2879,11 @@ static int hub_port_reset(struct usb_hub *hub, int port1,
 done:
if (status == 0) {
/* TRSTRCY = 10 ms; plus some extra */
-   msleep(10 + 40);
+   if (port_dev->quirks & USB_PORT_QUIRK_FAST_ENUM)
+   usleep_range(1, 12000);
+   else
+   msleep(10 + 40);
+
if (udev) {
struct usb_hcd *hcd = bus_to_hcd(udev->bus);
 
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 2ade17992ed66..4cdd515a4385f 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -496,6 +496,9 @@ enum usb_port_connect_type {
 /* For the given port, prefer the old (faster) enumeration scheme. */
 #define USB_PORT_QUIRK_OLD_SCHEME  BIT(0)
 
+/* Decrease TRSTRCY to 10ms during device enumeration. */
+#define USB_PORT_QUIRK_FAST_ENUM   BIT(1)
+
 /*
  * USB 2.0 Link Power Management (LPM) parameters.
  */
-- 
2.17.0.921.gf22659ad46-goog

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html