[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <69a0a8fa-854c-4498-a0cb-6176d8d7dd98@kernel.org>
Date: Mon, 29 Sep 2025 23:52:41 +0900
From: Vincent Mailhol <mailhol@...nel.org>
To: Celeste Liu <uwu@...lacanthus.name>,
Marc Kleine-Budde <mkl@...gutronix.de>
Cc: Maximilian Schneider <max@...neidersoft.net>,
Henrik Brix Andersen <henrik@...xandersen.dk>,
Wolfgang Grandegger <wg@...ndegger.com>, linux-can@...r.kernel.org,
linux-kernel@...r.kernel.org, Runcheng Lu <runcheng.lu@...icro.com>
Subject: Re: [PATCH] net/can/gs_usb: increase max interface to 255
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@...icro.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@...lacanthus.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
Powered by blists - more mailing lists