[PATCH -next v3 0/2] media: cx231xx: Switch to use kmemdup() helper and add two macros

[PATCH -next v3 0/2] media: cx231xx: Switch to use kmemdup() helper and add two macros

[Jinjie Ruan]

Use kmemdup() helper instead of open-coding to simplify the code.

On the other hand, as Andrzej suggested, add EP5_BUF_SIZE and
EP5_TIMEOUT_MS macros to replace the magic constant 4096 and 2000.

Changes in v3:
- Adjust the second patch to be the first.
- Update the names of the macros.

Jinjie Ruan (2):
  media: cx231xx: Switch to use kmemdup() helper
  media: cx231xx: Add EP5_BUF_SIZE and EP5_TIMEOUT_MS macros

 drivers/media/usb/cx231xx/cx231xx-core.c | 5 ++---
 drivers/media/usb/cx231xx/cx231xx.h      | 3 +++
 2 files changed, 5 insertions(+), 3 deletions(-)

-- 
2.34.1

[PATCH -next v3 1/2] media: cx231xx: Switch to use kmemdup() helper

[Jinjie Ruan]

Use kmemdup() helper instead of open-coding to
simplify the code.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
v2:
- Adjust use kmemdup() helper to be the first patch as suggested.
---
 drivers/media/usb/cx231xx/cx231xx-core.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/usb/cx231xx/cx231xx-core.c b/drivers/media/usb/cx231xx/cx231xx-core.c
index 727e6268567f..746727ea1a4c 100644
--- a/drivers/media/usb/cx231xx/cx231xx-core.c
+++ b/drivers/media/usb/cx231xx/cx231xx-core.c
@@ -751,10 +751,9 @@ int cx231xx_ep5_bulkout(struct cx231xx *dev, u8 *firmware, u16 size)
 	int ret = -ENOMEM;
 	u32 *buffer;
 
-	buffer = kzalloc(4096, GFP_KERNEL);
+	buffer = kmemdup(firmware, 4096, GFP_KERNEL);
 	if (buffer == NULL)
 		return -ENOMEM;
-	memcpy(&buffer[0], firmware, 4096);
 
 	ret = usb_bulk_msg(dev->udev, usb_sndbulkpipe(dev->udev, 5),
 			buffer, 4096, &actlen, 2000);
-- 
2.34.1

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

[Jinjie Ruan]

Define EP5_BUF_SIZE and EP5_TIMEOUT_MS macros to make the code
more readable.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Suggested-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
---
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);
 
 	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;
-- 
2.34.1

Re: [PATCH -next v3 1/2] media: cx231xx: Switch to use kmemdup() helper

[Andrzej Pietrasiewicz]

Hi,

W dniu 3.09.2023 o 10:14, Jinjie Ruan pisze:
> Use kmemdup() helper instead of open-coding to
> simplify the code.
> 
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>

I have already reviewed this before, you can add my

Reviewed-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>

> ---
> v2:
> - Adjust use kmemdup() helper to be the first patch as suggested.
> ---
>   drivers/media/usb/cx231xx/cx231xx-core.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/media/usb/cx231xx/cx231xx-core.c b/drivers/media/usb/cx231xx/cx231xx-core.c
> index 727e6268567f..746727ea1a4c 100644
> --- a/drivers/media/usb/cx231xx/cx231xx-core.c
> +++ b/drivers/media/usb/cx231xx/cx231xx-core.c
> @@ -751,10 +751,9 @@ int cx231xx_ep5_bulkout(struct cx231xx *dev, u8 *firmware, u16 size)
>   	int ret = -ENOMEM;
>   	u32 *buffer;
>   
> -	buffer = kzalloc(4096, GFP_KERNEL);
> +	buffer = kmemdup(firmware, 4096, GFP_KERNEL);
>   	if (buffer == NULL)
>   		return -ENOMEM;
> -	memcpy(&buffer[0], firmware, 4096);
>   
>   	ret = usb_bulk_msg(dev->udev, usb_sndbulkpipe(dev->udev, 5),
>   			buffer, 4096, &actlen, 2000);

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

[Andrzej Pietrasiewicz]

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@huawei.com>
> Suggested-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> ---
> 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;

Re: [PATCH -next v3 1/2] media: cx231xx: Switch to use kmemdup() helper

[Ruan Jinjie]

On 2023/9/8 20:19, Andrzej Pietrasiewicz wrote:
> Hi,
> 
> W dniu 3.09.2023 o 10:14, Jinjie Ruan pisze:
>> Use kmemdup() helper instead of open-coding to
>> simplify the code.
>>
>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> 
> I have already reviewed this before, you can add my
> 
> Reviewed-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>

Thank you very much!

> 
>> ---
>> v2:
>> - Adjust use kmemdup() helper to be the first patch as suggested.
>> ---
>>   drivers/media/usb/cx231xx/cx231xx-core.c | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/usb/cx231xx/cx231xx-core.c
>> b/drivers/media/usb/cx231xx/cx231xx-core.c
>> index 727e6268567f..746727ea1a4c 100644
>> --- a/drivers/media/usb/cx231xx/cx231xx-core.c
>> +++ b/drivers/media/usb/cx231xx/cx231xx-core.c
>> @@ -751,10 +751,9 @@ int cx231xx_ep5_bulkout(struct cx231xx *dev, u8
>> *firmware, u16 size)
>>       int ret = -ENOMEM;
>>       u32 *buffer;
>>   -    buffer = kzalloc(4096, GFP_KERNEL);
>> +    buffer = kmemdup(firmware, 4096, GFP_KERNEL);
>>       if (buffer == NULL)
>>           return -ENOMEM;
>> -    memcpy(&buffer[0], firmware, 4096);
>>         ret = usb_bulk_msg(dev->udev, usb_sndbulkpipe(dev->udev, 5),
>>               buffer, 4096, &actlen, 2000);
> 

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

[Ruan Jinjie]

On 2023/9/8 20:26, Andrzej Pietrasiewicz wrote:
> 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@huawei.com>
>> Suggested-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
>> ---
>> 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?

Thank you! I'll replace them together.

> 
> 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;
>