[-next,v2,0/2] media: cx231xx: Add two macros and switch to use kmemdup() helper

Message ID 20230822111832.822367-1-ruanjinjie@huawei.com (mailing list archive)
Headers
Series media: cx231xx: Add two macros and switch to use kmemdup() helper |

Message

Jinjie Ruan Aug. 22, 2023, 11:18 a.m. UTC
  As Andrzej suggested, add BUF_SIZE and TIMEOUT_MS macros to replace
the magic constant 4096 and 2000.

On the other hand, use kmemdup() helper instead of open-coding to
simplify the code.

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

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

Comments

Andrzej Pietrasiewicz Aug. 31, 2023, 7:05 p.m. UTC | #1
Hi Jinjie,

W dniu 22.08.2023 o 13:18, Jinjie Ruan pisze:
> As Andrzej suggested, add BUF_SIZE and TIMEOUT_MS macros to replace
> the magic constant 4096 and 2000.
> 
> On the other hand, use kmemdup() helper instead of open-coding to
> simplify the code.
> 

Sorry about the delay.

I think I'd prefer the kmemdup() patch as the first one so that it does not
depend on the patch adding the macros. And then the one adding the macros
becomes optional.

Speaking about the latter, maybe BUF_SIZE and TIMEOUT_MS are a bit too vague?
No strong opinion, though. Also, BUF_SIZE is suspiciously identical to PAGE_SIZE 
on some/many architectures. Any thoughts about it?

Regards,

Andrzej

> Jinjie Ruan (2):
>    media: cx231xx: Add BUF_SIZE and TIMEOUT_MS macros
>    media: cx231xx: Switch to use kmemdup() helper
> 
>   drivers/media/usb/cx231xx/cx231xx-core.c | 7 +++----
>   drivers/media/usb/cx231xx/cx231xx.h      | 3 +++
>   2 files changed, 6 insertions(+), 4 deletions(-)
>
  
Jinjie Ruan Sept. 3, 2023, 7:16 a.m. UTC | #2
On 2023/9/1 3:05, Andrzej Pietrasiewicz wrote:
> Hi Jinjie,
> 
> W dniu 22.08.2023 o 13:18, Jinjie Ruan pisze:
>> As Andrzej suggested, add BUF_SIZE and TIMEOUT_MS macros to replace
>> the magic constant 4096 and 2000.
>>
>> On the other hand, use kmemdup() helper instead of open-coding to
>> simplify the code.
>>
> 
> Sorry about the delay.
> 
> I think I'd prefer the kmemdup() patch as the first one so that it does not
> depend on the patch adding the macros. And then the one adding the macros
> becomes optional.
> 
> Speaking about the latter, maybe BUF_SIZE and TIMEOUT_MS are a bit too
> vague?
> No strong opinion, though. Also, BUF_SIZE is suspiciously identical to
> PAGE_SIZE on some/many architectures. Any thoughts about it?

So just use the PAGE_SIZE macro?

> 
> Regards,
> 
> Andrzej
> 
>> Jinjie Ruan (2):
>>    media: cx231xx: Add BUF_SIZE and TIMEOUT_MS macros
>>    media: cx231xx: Switch to use kmemdup() helper
>>
>>   drivers/media/usb/cx231xx/cx231xx-core.c | 7 +++----
>>   drivers/media/usb/cx231xx/cx231xx.h      | 3 +++
>>   2 files changed, 6 insertions(+), 4 deletions(-)
>>
>
  
Jinjie Ruan Sept. 3, 2023, 7:47 a.m. UTC | #3
On 2023/9/1 3:05, Andrzej Pietrasiewicz wrote:
> Hi Jinjie,
> 
> W dniu 22.08.2023 o 13:18, Jinjie Ruan pisze:
>> As Andrzej suggested, add BUF_SIZE and TIMEOUT_MS macros to replace
>> the magic constant 4096 and 2000.
>>
>> On the other hand, use kmemdup() helper instead of open-coding to
>> simplify the code.
>>
> 
> Sorry about the delay.
> 
> I think I'd prefer the kmemdup() patch as the first one so that it does not
> depend on the patch adding the macros. And then the one adding the macros
> becomes optional.
> 
> Speaking about the latter, maybe BUF_SIZE and TIMEOUT_MS are a bit too
> vague?
> No strong opinion, though. Also, BUF_SIZE is suspiciously identical to
> PAGE_SIZE on some/many architectures. Any thoughts about it?

PAGE_SIZE is configurable, which can be 2^12, 2^14, 2^16, such as ARM64:

/* PAGE_SHIFT determines the page size */
#define PAGE_SHIFT>----->-------CONFIG_ARM64_PAGE_SHIFT
#define PAGE_SIZE>------>-------(_AC(1, UL) << PAGE_SHIFT)

> 
> Regards,
> 
> Andrzej
> 
>> Jinjie Ruan (2):
>>    media: cx231xx: Add BUF_SIZE and TIMEOUT_MS macros
>>    media: cx231xx: Switch to use kmemdup() helper
>>
>>   drivers/media/usb/cx231xx/cx231xx-core.c | 7 +++----
>>   drivers/media/usb/cx231xx/cx231xx.h      | 3 +++
>>   2 files changed, 6 insertions(+), 4 deletions(-)
>>
>