Re: [PATCH] usb: Ignore endpoints in non-zero altsettings

2023-10-29 Thread Marek Vasut

On 10/29/23 16:50, Hector Martin wrote:

On 29/10/2023 21.04, Marek Vasut wrote:

On 10/29/23 08:24, Hector Martin wrote:

We currently do not really handle altsettings properly, and no driver
uses them. Ignore the respective endpoint descriptors for secondary
altsettings, to avoid creating duplicate endpoint records in the
interface.

This will have to be revisited if/when we have a driver that needs
altsettings to work properly.

Signed-off-by: Hector Martin 
---
   common/usb.c | 9 +
   1 file changed, 9 insertions(+)

diff --git a/common/usb.c b/common/usb.c
index aad13fd9c557..90f72fda00bc 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -463,6 +463,15 @@ static int usb_parse_config(struct usb_device *dev,
puts("Endpoint descriptor out of order!\n");
break;
}
+   if (if_desc->num_altsetting > 1) {
+   /*
+* Ignore altsettings, which can trigger 
duplicate
+* endpoint errors below. Revisit this when some
+* driver actually needs altsettings with 
differing
+* endpoint setups.
+*/


How do you trigger this error ?



Plug in a device with altsettings, like most sound cards or UVC devices.


Please add that to the commit message.


Re: [PATCH] usb: Ignore endpoints in non-zero altsettings

2023-10-29 Thread Hector Martin
On 29/10/2023 21.04, Marek Vasut wrote:
> On 10/29/23 08:24, Hector Martin wrote:
>> We currently do not really handle altsettings properly, and no driver
>> uses them. Ignore the respective endpoint descriptors for secondary
>> altsettings, to avoid creating duplicate endpoint records in the
>> interface.
>>
>> This will have to be revisited if/when we have a driver that needs
>> altsettings to work properly.
>>
>> Signed-off-by: Hector Martin 
>> ---
>>   common/usb.c | 9 +
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/common/usb.c b/common/usb.c
>> index aad13fd9c557..90f72fda00bc 100644
>> --- a/common/usb.c
>> +++ b/common/usb.c
>> @@ -463,6 +463,15 @@ static int usb_parse_config(struct usb_device *dev,
>>  puts("Endpoint descriptor out of order!\n");
>>  break;
>>  }
>> +if (if_desc->num_altsetting > 1) {
>> +/*
>> + * Ignore altsettings, which can trigger 
>> duplicate
>> + * endpoint errors below. Revisit this when some
>> + * driver actually needs altsettings with 
>> differing
>> + * endpoint setups.
>> + */
> 
> How do you trigger this error ?
> 

Plug in a device with altsettings, like most sound cards or UVC devices.

- Hector



Re: [PATCH] usb: Ignore endpoints in non-zero altsettings

2023-10-29 Thread Marek Vasut

On 10/29/23 08:24, Hector Martin wrote:

We currently do not really handle altsettings properly, and no driver
uses them. Ignore the respective endpoint descriptors for secondary
altsettings, to avoid creating duplicate endpoint records in the
interface.

This will have to be revisited if/when we have a driver that needs
altsettings to work properly.

Signed-off-by: Hector Martin 
---
  common/usb.c | 9 +
  1 file changed, 9 insertions(+)

diff --git a/common/usb.c b/common/usb.c
index aad13fd9c557..90f72fda00bc 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -463,6 +463,15 @@ static int usb_parse_config(struct usb_device *dev,
puts("Endpoint descriptor out of order!\n");
break;
}
+   if (if_desc->num_altsetting > 1) {
+   /*
+* Ignore altsettings, which can trigger 
duplicate
+* endpoint errors below. Revisit this when some
+* driver actually needs altsettings with 
differing
+* endpoint setups.
+*/


How do you trigger this error ?


Re: [PATCH] usb: Ignore endpoints in non-zero altsettings

2023-10-29 Thread Neal Gompa
On Sun, Oct 29, 2023 at 3:25 AM Hector Martin  wrote:
>
> We currently do not really handle altsettings properly, and no driver
> uses them. Ignore the respective endpoint descriptors for secondary
> altsettings, to avoid creating duplicate endpoint records in the
> interface.
>
> This will have to be revisited if/when we have a driver that needs
> altsettings to work properly.
>
> Signed-off-by: Hector Martin 
> ---
>  common/usb.c | 9 +
>  1 file changed, 9 insertions(+)
>
> diff --git a/common/usb.c b/common/usb.c
> index aad13fd9c557..90f72fda00bc 100644
> --- a/common/usb.c
> +++ b/common/usb.c
> @@ -463,6 +463,15 @@ static int usb_parse_config(struct usb_device *dev,
> puts("Endpoint descriptor out of order!\n");
> break;
> }
> +   if (if_desc->num_altsetting > 1) {
> +   /*
> +* Ignore altsettings, which can trigger 
> duplicate
> +* endpoint errors below. Revisit this when 
> some
> +* driver actually needs altsettings with 
> differing
> +* endpoint setups.
> +*/
> +   break;
> +   }
> epno = dev->config.if_desc[ifno].no_of_ep;
> if_desc = >config.if_desc[ifno];
> if (epno >= USB_MAXENDPOINTS) {
>
> ---
> base-commit: 8ad1c9c26f7740806a162818b790d4a72f515b7e
> change-id: 20231029-usb-fixes-4-ba6931acf217
>

Irritating, but makes sense.

Reviewed-by: Neal Gompa 



--
真実はいつも一つ!/ Always, there's only one truth!


[PATCH] usb: Ignore endpoints in non-zero altsettings

2023-10-29 Thread Hector Martin
We currently do not really handle altsettings properly, and no driver
uses them. Ignore the respective endpoint descriptors for secondary
altsettings, to avoid creating duplicate endpoint records in the
interface.

This will have to be revisited if/when we have a driver that needs
altsettings to work properly.

Signed-off-by: Hector Martin 
---
 common/usb.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/common/usb.c b/common/usb.c
index aad13fd9c557..90f72fda00bc 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -463,6 +463,15 @@ static int usb_parse_config(struct usb_device *dev,
puts("Endpoint descriptor out of order!\n");
break;
}
+   if (if_desc->num_altsetting > 1) {
+   /*
+* Ignore altsettings, which can trigger 
duplicate
+* endpoint errors below. Revisit this when some
+* driver actually needs altsettings with 
differing
+* endpoint setups.
+*/
+   break;
+   }
epno = dev->config.if_desc[ifno].no_of_ep;
if_desc = >config.if_desc[ifno];
if (epno >= USB_MAXENDPOINTS) {

---
base-commit: 8ad1c9c26f7740806a162818b790d4a72f515b7e
change-id: 20231029-usb-fixes-4-ba6931acf217

Best regards,
-- 
Hector Martin