Re: [PATCH -next v3 2/2] media: cx231xx: Add EP5_BUF_SIZE and EP5_TIMEOUT_MS macros

[Andrzej Pietrasiewicz <andrzej.p@xxxxxxxxxxxxx>]

Hi,

W dniu 3.09.2023 o 10:14, Jinjie Ruan pisze:
> Define EP5_BUF_SIZE and EP5_TIMEOUT_MS macros to make the code
> more readable.
>
> Signed-off-by: Jinjie Ruan <ruanjinjie@xxxxxxxxxx>
> Suggested-by: Andrzej Pietrasiewicz <andrzej.p@xxxxxxxxxxxxx>
> ---
> v2:
> - Adjust the patch to be the second.
> - Adjust the macro name.
> ---
>   drivers/media/usb/cx231xx/cx231xx-core.c | 4 ++--
>   drivers/media/usb/cx231xx/cx231xx.h      | 3 +++
>   2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/usb/cx231xx/cx231xx-core.c b/drivers/media/usb/cx231xx/cx231xx-core.c
> index 746727ea1a4c..57a8b4780a7d 100644
> --- a/drivers/media/usb/cx231xx/cx231xx-core.c
> +++ b/drivers/media/usb/cx231xx/cx231xx-core.c
> @@ -751,12 +751,12 @@ int cx231xx_ep5_bulkout(struct cx231xx *dev, u8 *firmware, u16 size)
>   	int ret = -ENOMEM;
>   	u32 *buffer;
>   
> -	buffer = kmemdup(firmware, 4096, GFP_KERNEL);
> +	buffer = kmemdup(firmware, EP5_BUF_SIZE, GFP_KERNEL);
>   	if (buffer == NULL)
>   		return -ENOMEM;
>   
>   	ret = usb_bulk_msg(dev->udev, usb_sndbulkpipe(dev->udev, 5),
> -			buffer, 4096, &actlen, 2000);
> +			buffer, EP5_BUF_SIZE, &actlen, EP5_TIMEOUT_MS);
>   

$ git grep 4096 drivers/media/usb/cx231xx | sort | wc -l
6

4096 is a pretty popular constant in this driver. What about the other 4 places?

Andrzej

>   	if (ret)
>   		dev_err(dev->dev,
> diff --git a/drivers/media/usb/cx231xx/cx231xx.h b/drivers/media/usb/cx231xx/cx231xx.h
> index 6929e4d97067..74339a6a2f71 100644
> --- a/drivers/media/usb/cx231xx/cx231xx.h
> +++ b/drivers/media/usb/cx231xx/cx231xx.h
> @@ -120,6 +120,9 @@
>   #define CX23417_OSC_EN   8
>   #define CX23417_RESET    9
>   
> +#define EP5_BUF_SIZE     4096
> +#define EP5_TIMEOUT_MS   2000
> +
>   struct cx23417_fmt {
>   	u32   fourcc;          /* v4l2 format id */
>   	int   depth;