On Thu, Jan 18, 2024 at 7:49 AM Krishna Kurapati <quic_kriskura@xxxxxxxxxxx> wrote: > > Recent commit [1] added support for changing max segment size of the NCM > interface via configfs. But the value of segment size value stored in > ncm_opts need to be converted to little endian before saving it in > ecm_desc. Also while initialising the value of segment size in opts during > instance allocation, the value ETH_FRAME_LEN needs to be assigned directly > without any conversion as ETH_FRAME_LEN and the variable max_segment_size > are native endian. The current implementaion modifies it into little endian > thus breaking things for big endian targets. > > Fix endianness while assigning these variables. > While at it, fix up some stray spaces in comments added in code. > > [1]: https://lore.kernel.org/all/20231221153216.18657-1-quic_kriskura@xxxxxxxxxxx/ > > Fixes: 1900daeefd3e ("usb: gadget: ncm: Add support to update wMaxSegmentSize via configfs") > Signed-off-by: Krishna Kurapati <quic_kriskura@xxxxxxxxxxx> > --- > Since the patch was tested on ARM based QC devices, no issues were seen > as these devices were little endian. Thanks to Maciej Żenczykowski for > pointing it out offline over ACK that the patch breaks functionality for > big endian devices. > > drivers/usb/gadget/function/f_ncm.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/usb/gadget/function/f_ncm.c b/drivers/usb/gadget/function/f_ncm.c > index a1575a0ca568..ca5d5f564998 100644 > --- a/drivers/usb/gadget/function/f_ncm.c > +++ b/drivers/usb/gadget/function/f_ncm.c > @@ -105,8 +105,8 @@ static inline struct f_ncm *func_to_ncm(struct usb_function *f) > > /* > * Although max mtu as dictated by u_ether is 15412 bytes, setting > - * max_segment_sizeto 15426 would not be efficient. If user chooses segment > - * size to be (>= 8192), then we can't aggregate more than one buffer in each > + * max_segment_size to 15426 would not be efficient. If user chooses segment > + * size to be (>= 8192), then we can't aggregate more than one buffer in each > * NTB (assuming each packet coming from network layer is >= 8192 bytes) as ep > * maxpacket limit is 16384. So let max_segment_size be limited to 8000 to allow > * at least 2 packets to be aggregated reducing wastage of NTB buffer space > @@ -1489,7 +1489,7 @@ static int ncm_bind(struct usb_configuration *c, struct usb_function *f) > ncm_data_intf.bInterfaceNumber = status; > ncm_union_desc.bSlaveInterface0 = status; > > - ecm_desc.wMaxSegmentSize = ncm_opts->max_segment_size; > + ecm_desc.wMaxSegmentSize = cpu_to_le16(ncm_opts->max_segment_size); > > status = -ENODEV; > > @@ -1685,7 +1685,7 @@ static struct usb_function_instance *ncm_alloc_inst(void) > kfree(opts); > return ERR_CAST(net); > } > - opts->max_segment_size = cpu_to_le16(ETH_FRAME_LEN); > + opts->max_segment_size = ETH_FRAME_LEN; > INIT_LIST_HEAD(&opts->ncm_os_desc.ext_prop); > > descs[0] = &opts->ncm_os_desc; > -- > 2.42.0 > Reviewed-by: Maciej Żenczykowski <maze@xxxxxxxxxx>
