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

[Vincent Mailhol <mailhol@kernel.org>]

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