| Message ID | 20241027140537.503264-1-prosunofficial@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [linux-next] drm/amdgpu: use string choice helpers | expand |
On Sun, Oct 27, 2024 at 10:18 AM R Sundar <prosunofficial@gmail.com> wrote: > > Use string choice helpers for better readability. I personally find this less readable, but if this is the preferred method going forward, I'm fine to take the patch. Alex > > Reported-by: kernel test robot <lkp@intel.com> > Reported-by: Julia Lawall <julia.lawall@inria.fr> > Closes: https://lore.kernel.org/r/202410161814.I6p2Nnux-lkp@intel.com/ > Signed-off-by: R Sundar <prosunofficial@gmail.com> > --- > > reported in linux repository. > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master > > cocci warnings: (new ones prefixed by >>) > >> drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c:145:8-12: opportunity for str_read_write(read) > > vim +145 drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c > > drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c > index 35fee3e8cde2..8cd69836dd99 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c > @@ -200,7 +200,7 @@ static int amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap, u32 eeprom_addr, > dev_err_ratelimited(&i2c_adap->dev, > "maddr:0x%04X size:0x%02X:quirk max_%s_len must be > %d", > eeprom_addr, buf_size, > - read ? "read" : "write", EEPROM_OFFSET_SIZE); > + str_read_write(read), EEPROM_OFFSET_SIZE); > return -EINVAL; > } > > -- > 2.34.1 >
On 29/10/24 19:11, Alex Deucher wrote: > On Sun, Oct 27, 2024 at 10:18 AM R Sundar <prosunofficial@gmail.com> wrote: >> >> Use string choice helpers for better readability. > > I personally find this less readable, but if this is the preferred > method going forward, I'm fine to take the patch. > > Alex > Hi, Thanks for the comments. I came across this comments in string_choices.h files, where str_read_write() helpers are present. Using these helpers offers the following benefits: 1) Reducing the hardcoding of strings, which makes the code more elegant through these simple literal-meaning helpers. 2) Unifying the output, which prevents the same string from being printed in various forms, such as enable/disable, enabled/disabled, en/dis. 3) Deduping by the linker, which results in a smaller binary file. Kindly, I'm leaving the decision to maintainers. Thanks, Sundar >> >> Reported-by: kernel test robot <lkp@intel.com> >> Reported-by: Julia Lawall <julia.lawall@inria.fr> >> Closes: https://lore.kernel.org/r/202410161814.I6p2Nnux-lkp@intel.com/ >> Signed-off-by: R Sundar <prosunofficial@gmail.com> >> --- >> >> reported in linux repository. >> >> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master >> >> cocci warnings: (new ones prefixed by >>) >>>> drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c:145:8-12: opportunity for str_read_write(read) >> >> vim +145 drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c >> >> drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c >> index 35fee3e8cde2..8cd69836dd99 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c >> @@ -200,7 +200,7 @@ static int amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap, u32 eeprom_addr, >> dev_err_ratelimited(&i2c_adap->dev, >> "maddr:0x%04X size:0x%02X:quirk max_%s_len must be > %d", >> eeprom_addr, buf_size, >> - read ? "read" : "write", EEPROM_OFFSET_SIZE); >> + str_read_write(read), EEPROM_OFFSET_SIZE); >> return -EINVAL; >> } >> >> -- >> 2.34.1 >>
On Tue, Oct 29, 2024 at 1:05 PM R Sundar <prosunofficial@gmail.com> wrote: > > On 29/10/24 19:11, Alex Deucher wrote: > > On Sun, Oct 27, 2024 at 10:18 AM R Sundar <prosunofficial@gmail.com> wrote: > >> > >> Use string choice helpers for better readability. > > > > I personally find this less readable, but if this is the preferred > > method going forward, I'm fine to take the patch. > > > > Alex > > > > Hi, > > Thanks for the comments. > > I came across this comments in string_choices.h files, where > str_read_write() helpers are present. > > Using these helpers offers the following benefits: > > 1) Reducing the hardcoding of strings, which makes the code more > elegant through these simple literal-meaning helpers. > > 2) Unifying the output, which prevents the same string from being > printed in various forms, such as enable/disable, enabled/disabled, > en/dis. > 3) Deduping by the linker, which results in a smaller binary file. > Thanks. I've applied it. Alex > Kindly, I'm leaving the decision to maintainers. > > Thanks, > Sundar > > >> > >> Reported-by: kernel test robot <lkp@intel.com> > >> Reported-by: Julia Lawall <julia.lawall@inria.fr> > >> Closes: https://lore.kernel.org/r/202410161814.I6p2Nnux-lkp@intel.com/ > >> Signed-off-by: R Sundar <prosunofficial@gmail.com> > >> --- > >> > >> reported in linux repository. > >> > >> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master > >> > >> cocci warnings: (new ones prefixed by >>) > >>>> drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c:145:8-12: opportunity for str_read_write(read) > >> > >> vim +145 drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c > >> > >> drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c > >> index 35fee3e8cde2..8cd69836dd99 100644 > >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c > >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c > >> @@ -200,7 +200,7 @@ static int amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap, u32 eeprom_addr, > >> dev_err_ratelimited(&i2c_adap->dev, > >> "maddr:0x%04X size:0x%02X:quirk max_%s_len must be > %d", > >> eeprom_addr, buf_size, > >> - read ? "read" : "write", EEPROM_OFFSET_SIZE); > >> + str_read_write(read), EEPROM_OFFSET_SIZE); > >> return -EINVAL; > >> } > >> > >> -- > >> 2.34.1 > >> >
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c index 35fee3e8cde2..8cd69836dd99 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c @@ -200,7 +200,7 @@ static int amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap, u32 eeprom_addr, dev_err_ratelimited(&i2c_adap->dev, "maddr:0x%04X size:0x%02X:quirk max_%s_len must be > %d", eeprom_addr, buf_size, - read ? "read" : "write", EEPROM_OFFSET_SIZE); + str_read_write(read), EEPROM_OFFSET_SIZE); return -EINVAL; }