| Message ID | 20230822111832.822367-1-ruanjinjie@huawei.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | media: cx231xx: Add two macros and switch to use kmemdup() helper | expand |
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(-) >
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(-) >> >
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(-) >> >