net/can/gs_usb: increase max interface to 255

[PATCH] net/can/gs_usb: increase max interface to 255

Posted by Celeste Liu 4 months, 1 week ago

This issue was found by Runcheng Lu when develop HSCanT USB to CAN FD
converter[1]. The original developers may have only 3 intefaces device to
test so they write 3 here and wait for future change.

During the HSCanT development, we actually used 4 interfaces, so the
limitation of 3 is not enough now. But just increase one is not
future-proofed. Since the channel type in gs_host_frame is u8, use 255
as max interface number should be safe.

[1]: https://github.com/cherry-embedded/HSCanT-hardware

Reported-by: Runcheng Lu <runcheng.lu@hpmicro.com>
Signed-off-by: Celeste Liu <uwu@coelacanthus.name>
---
 drivers/net/can/usb/gs_usb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index c9482d6e947b0c7b033dc4f0c35f5b111e1bfd92..35fc257c19e57c1f33e03e7c86ea908d22400254 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -290,9 +290,9 @@ struct gs_host_frame {
 #define GS_NAPI_WEIGHT 32
 
 /* Maximum number of interfaces the driver supports per device.
- * Current hardware only supports 3 interfaces. The future may vary.
+ * The channel number type of gs_host_frame is u8, so max interfaces can be 255.
  */
-#define GS_MAX_INTF 3
+#define GS_MAX_INTF 255
 
 struct gs_tx_context {
 	struct gs_can *dev;

---
base-commit: e5f0a698b34ed76002dc5cff3804a61c80233a7a
change-id: 20250929-gs-usb-max-if-a304c83243e5

Best regards,
-- 
Celeste Liu <uwu@coelacanthus.name>

Re: [PATCH] net/can/gs_usb: increase max interface to 255

Posted by Vincent Mailhol 4 months, 1 week ago

Hi Celeste,

Thank you for your patch. Here are my comments.

On 9/29/25 8:10 PM, Celeste Liu wrote:
> This issue was found by Runcheng Lu when develop HSCanT USB to CAN FD
> converter[1]. The original developers may have only 3 intefaces device to
> test so they write 3 here and wait for future change.
> 
> During the HSCanT development, we actually used 4 interfaces, so the
> limitation of 3 is not enough now. But just increase one is not
> future-proofed. Since the channel type in gs_host_frame is u8, use 255
> as max interface number should be safe.
> 
> [1]: https://github.com/cherry-embedded/HSCanT-hardware
> 
> Reported-by: Runcheng Lu <runcheng.lu@hpmicro.com>

If you have any links where Runcheng reported the issue, you can add it here as:

  Closes: <URL to the bug report>

(Ignore if such message does not exist).

Also add a Fixes tag so that the fix can be backported to stable.

> Signed-off-by: Celeste Liu <uwu@coelacanthus.name>
> ---
>  drivers/net/can/usb/gs_usb.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
> index c9482d6e947b0c7b033dc4f0c35f5b111e1bfd92..35fc257c19e57c1f33e03e7c86ea908d22400254 100644
> --- a/drivers/net/can/usb/gs_usb.c
> +++ b/drivers/net/can/usb/gs_usb.c
> @@ -290,9 +290,9 @@ struct gs_host_frame {
>  #define GS_NAPI_WEIGHT 32
>  
>  /* Maximum number of interfaces the driver supports per device.
> - * Current hardware only supports 3 interfaces. The future may vary.
> + * The channel number type of gs_host_frame is u8, so max interfaces can be 255.
>   */
> -#define GS_MAX_INTF 3
> +#define GS_MAX_INTF 255

After doing this, you are left with an array of 255 pointers in struct gs_usb
(which represents 2 kilobytes of memory on a 64 bits machine). You also have two
loops iterating from 0 to 255. This is a bit of a waste of both space and
processing power.

It is better to use a flexible array member, like this:

-----8<-----
diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index c9482d6e947b..459a956ac0d9 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -324,7 +324,6 @@ struct gs_can {

 /* usb interface struct */
 struct gs_usb {
-       struct gs_can *canch[GS_MAX_INTF];
        struct usb_anchor rx_submitted;
        struct usb_device *udev;

@@ -336,9 +335,11 @@ struct gs_usb {

        unsigned int hf_size_rx;
        u8 active_channels;
+       u8 channel_cnt;

        unsigned int pipe_in;
        unsigned int pipe_out;
+       struct gs_can *canch[] __counted_by(channel_cnt);
 };

 /* 'allocate' a tx context.
----->8-----

Then all the instances of GS_MAX_INTF are replaced by gs_usb->channel_cnt except
from the check on dfconf.icount which can be replaced by a:

  type_max(typeof(gs_usb->channel_cnt))


Yours sincerely,
Vincent Mailhol

Re: [PATCH] net/can/gs_usb: increase max interface to 255

Posted by Celeste Liu 4 months, 1 week ago

On 2025-09-29 22:52, Vincent Mailhol wrote:
> Hi Celeste,
> 
> Thank you for your patch. Here are my comments.
> 
> On 9/29/25 8:10 PM, Celeste Liu wrote:
>> This issue was found by Runcheng Lu when develop HSCanT USB to CAN FD
>> converter[1]. The original developers may have only 3 intefaces device to
>> test so they write 3 here and wait for future change.
>>
>> During the HSCanT development, we actually used 4 interfaces, so the
>> limitation of 3 is not enough now. But just increase one is not
>> future-proofed. Since the channel type in gs_host_frame is u8, use 255
>> as max interface number should be safe.
>>
>> [1]: https://github.com/cherry-embedded/HSCanT-hardware
>>
>> Reported-by: Runcheng Lu <runcheng.lu@hpmicro.com>
> 
> If you have any links where Runcheng reported the issue, you can add it here as:
> 
>   Closes: <URL to the bug report>
> 
> (Ignore if such message does not exist).

Yeah. No report is public to Internet.

> 
> Also add a Fixes tag so that the fix can be backported to stable.

Added in v2.

> 
>> Signed-off-by: Celeste Liu <uwu@coelacanthus.name>
>> ---
>>  drivers/net/can/usb/gs_usb.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
>> index c9482d6e947b0c7b033dc4f0c35f5b111e1bfd92..35fc257c19e57c1f33e03e7c86ea908d22400254 100644
>> --- a/drivers/net/can/usb/gs_usb.c
>> +++ b/drivers/net/can/usb/gs_usb.c
>> @@ -290,9 +290,9 @@ struct gs_host_frame {
>>  #define GS_NAPI_WEIGHT 32
>>  
>>  /* Maximum number of interfaces the driver supports per device.
>> - * Current hardware only supports 3 interfaces. The future may vary.
>> + * The channel number type of gs_host_frame is u8, so max interfaces can be 255.
>>   */
>> -#define GS_MAX_INTF 3
>> +#define GS_MAX_INTF 255
> 
> After doing this, you are left with an array of 255 pointers in struct gs_usb
> (which represents 2 kilobytes of memory on a 64 bits machine). You also have two
> loops iterating from 0 to 255. This is a bit of a waste of both space and
> processing power.
> 
> It is better to use a flexible array member, like this:

Has done in v2.

> 
> -----8<-----
> diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
> index c9482d6e947b..459a956ac0d9 100644
> --- a/drivers/net/can/usb/gs_usb.c
> +++ b/drivers/net/can/usb/gs_usb.c
> @@ -324,7 +324,6 @@ struct gs_can {
> 
>  /* usb interface struct */
>  struct gs_usb {
> -       struct gs_can *canch[GS_MAX_INTF];
>         struct usb_anchor rx_submitted;
>         struct usb_device *udev;
> 
> @@ -336,9 +335,11 @@ struct gs_usb {
> 
>         unsigned int hf_size_rx;
>         u8 active_channels;
> +       u8 channel_cnt;
> 
>         unsigned int pipe_in;
>         unsigned int pipe_out;
> +       struct gs_can *canch[] __counted_by(channel_cnt);
>  };
> 
>  /* 'allocate' a tx context.
> ----->8-----
> 
> Then all the instances of GS_MAX_INTF are replaced by gs_usb->channel_cnt except
> from the check on dfconf.icount which can be replaced by a:
> 
>   type_max(typeof(gs_usb->channel_cnt))
> 
> 
> Yours sincerely,
> Vincent Mailhol
>