| Message ID | 20241027141907.503946-1-prosunofficial@gmail.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [linux-next] ice: use string choice helpers | expand |
On 10/27/24 15:19, R Sundar wrote: > Use string choice helpers for better readability. > > Reported-by: kernel test robot <lkp@intel.com> > Reported-by: Julia Lawall <julia.lawall@inria.fr> > Closes: https://lore.kernel.org/r/202410121553.SRNFzc2M-lkp@intel.com/ > Signed-off-by: R Sundar <prosunofficial@gmail.com> > --- thanks, this indeed covers all "enabled/disabled" cases, so: Acked-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> for future submissions for Intel Ethernet drivers please use the iwl-next (or iwl-net) target trees. There are also other cases that we could cover ON/OFF etc > > 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/net/ethernet/intel/ice/ice_ptp_hw.c:396:4-22: opportunity for str_enabled_disabled(dw24 . ts_pll_enable) > drivers/net/ethernet/intel/ice/ice_ptp_hw.c:474:4-22: opportunity for str_enabled_disabled(dw24 . ts_pll_enable) > > vim +396 drivers/net/ethernet/intel/ice/ice_ptp_hw.c > > > drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c > index da88c6ccfaeb..d8d3395e49c3 100644 > --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c > +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c > @@ -393,7 +393,7 @@ static int ice_cfg_cgu_pll_e82x(struct ice_hw *hw, > > /* Log the current clock configuration */ > ice_debug(hw, ICE_DBG_PTP, "Current CGU configuration -- %s, clk_src %s, clk_freq %s, PLL %s\n", > - dw24.ts_pll_enable ? "enabled" : "disabled", > + str_enabled_disabled(dw24.ts_pll_enable), > ice_clk_src_str(dw24.time_ref_sel), > ice_clk_freq_str(dw9.time_ref_freq_sel), > bwm_lf.plllock_true_lock_cri ? "locked" : "unlocked"); perhaps locked/unlocked could be added into string_choices.h > @@ -471,7 +471,7 @@ static int ice_cfg_cgu_pll_e82x(struct ice_hw *hw, > > /* Log the current clock configuration */ > ice_debug(hw, ICE_DBG_PTP, "New CGU configuration -- %s, clk_src %s, clk_freq %s, PLL %s\n", > - dw24.ts_pll_enable ? "enabled" : "disabled", > + str_enabled_disabled(dw24.ts_pll_enable), > ice_clk_src_str(dw24.time_ref_sel), > ice_clk_freq_str(dw9.time_ref_freq_sel), > bwm_lf.plllock_true_lock_cri ? "locked" : "unlocked"); > @@ -548,7 +548,7 @@ static int ice_cfg_cgu_pll_e825c(struct ice_hw *hw, > > /* Log the current clock configuration */ > ice_debug(hw, ICE_DBG_PTP, "Current CGU configuration -- %s, clk_src %s, clk_freq %s, PLL %s\n", > - dw24.ts_pll_enable ? "enabled" : "disabled", > + str_enabled_disabled(dw24.ts_pll_enable), > ice_clk_src_str(dw23.time_ref_sel), > ice_clk_freq_str(dw9.time_ref_freq_sel), > ro_lock.plllock_true_lock_cri ? "locked" : "unlocked"); > @@ -653,7 +653,7 @@ static int ice_cfg_cgu_pll_e825c(struct ice_hw *hw, > > /* Log the current clock configuration */ > ice_debug(hw, ICE_DBG_PTP, "New CGU configuration -- %s, clk_src %s, clk_freq %s, PLL %s\n", > - dw24.ts_pll_enable ? "enabled" : "disabled", > + str_enabled_disabled(dw24.ts_pll_enable), > ice_clk_src_str(dw23.time_ref_sel), > ice_clk_freq_str(dw9.time_ref_freq_sel), > ro_lock.plllock_true_lock_cri ? "locked" : "unlocked");
On 28/10/24 15:24, Przemek Kitszel wrote: > On 10/27/24 15:19, R Sundar wrote: >> Use string choice helpers for better readability. >> >> Reported-by: kernel test robot <lkp@intel.com> >> Reported-by: Julia Lawall <julia.lawall@inria.fr> >> Closes: https://lore.kernel.org/r/202410121553.SRNFzc2M-lkp@intel.com/ >> Signed-off-by: R Sundar <prosunofficial@gmail.com> >> --- > > thanks, this indeed covers all "enabled/disabled" cases, so: > Acked-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> > Hi, Thanks for comments. > for future submissions for Intel Ethernet drivers please use the > iwl-next (or iwl-net) target trees. > Sure. Noted. > There are also other cases that we could cover ON/OFF etc > >> >> 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/net/ethernet/intel/ice/ice_ptp_hw.c:396:4-22: opportunity >>>> for str_enabled_disabled(dw24 . ts_pll_enable) >> drivers/net/ethernet/intel/ice/ice_ptp_hw.c:474:4-22: opportunity >> for str_enabled_disabled(dw24 . ts_pll_enable) >> >> vim +396 drivers/net/ethernet/intel/ice/ice_ptp_hw.c >> >> >> drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c >> b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c >> index da88c6ccfaeb..d8d3395e49c3 100644 >> --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c >> +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c >> @@ -393,7 +393,7 @@ static int ice_cfg_cgu_pll_e82x(struct ice_hw *hw, >> /* Log the current clock configuration */ >> ice_debug(hw, ICE_DBG_PTP, "Current CGU configuration -- %s, >> clk_src %s, clk_freq %s, PLL %s\n", >> - dw24.ts_pll_enable ? "enabled" : "disabled", >> + str_enabled_disabled(dw24.ts_pll_enable), >> ice_clk_src_str(dw24.time_ref_sel), >> ice_clk_freq_str(dw9.time_ref_freq_sel), >> bwm_lf.plllock_true_lock_cri ? "locked" : "unlocked"); > > perhaps locked/unlocked could be added into string_choices.h > Sure, Can I add locked/unlocked changes in linux-next repository and use suggested-by Tag? >> @@ -471,7 +471,7 @@ static int ice_cfg_cgu_pll_e82x(struct ice_hw *hw, >> /* Log the current clock configuration */ >> ice_debug(hw, ICE_DBG_PTP, "New CGU configuration -- %s, clk_src >> %s, clk_freq %s, PLL %s\n", >> - dw24.ts_pll_enable ? "enabled" : "disabled", >> + str_enabled_disabled(dw24.ts_pll_enable), >> ice_clk_src_str(dw24.time_ref_sel), >> ice_clk_freq_str(dw9.time_ref_freq_sel), >> bwm_lf.plllock_true_lock_cri ? "locked" : "unlocked"); >> @@ -548,7 +548,7 @@ static int ice_cfg_cgu_pll_e825c(struct ice_hw *hw, >> /* Log the current clock configuration */ >> ice_debug(hw, ICE_DBG_PTP, "Current CGU configuration -- %s, >> clk_src %s, clk_freq %s, PLL %s\n", >> - dw24.ts_pll_enable ? "enabled" : "disabled", >> + str_enabled_disabled(dw24.ts_pll_enable), >> ice_clk_src_str(dw23.time_ref_sel), >> ice_clk_freq_str(dw9.time_ref_freq_sel), >> ro_lock.plllock_true_lock_cri ? "locked" : "unlocked"); >> @@ -653,7 +653,7 @@ static int ice_cfg_cgu_pll_e825c(struct ice_hw *hw, >> /* Log the current clock configuration */ >> ice_debug(hw, ICE_DBG_PTP, "New CGU configuration -- %s, clk_src >> %s, clk_freq %s, PLL %s\n", >> - dw24.ts_pll_enable ? "enabled" : "disabled", >> + str_enabled_disabled(dw24.ts_pll_enable), >> ice_clk_src_str(dw23.time_ref_sel), >> ice_clk_freq_str(dw9.time_ref_freq_sel), >> ro_lock.plllock_true_lock_cri ? "locked" : "unlocked"); >
On 10/29/24 17:37, R Sundar wrote: > On 28/10/24 15:24, Przemek Kitszel wrote: >> On 10/27/24 15:19, R Sundar wrote: >>> Use string choice helpers for better readability. >>> bwm_lf.plllock_true_lock_cri ? "locked" : "unlocked"); >> >> perhaps locked/unlocked could be added into string_choices.h >> > > Sure, Can I add locked/unlocked changes in linux-next repository and use > suggested-by Tag? sure, that's way to go but please first check if there are any other users (despite this driver) > > >>> @@ -471,7 +471,7 @@ static int ice_cfg_cgu_pll_e82x(struct ice_hw *hw, >>> /* Log the current clock configuration */ >>> ice_debug(hw, ICE_DBG_PTP, "New CGU configuration -- %s, >>> clk_src %s, clk_freq %s, PLL %s\n", >>> - dw24.ts_pll_enable ? "enabled" : "disabled", >>> + str_enabled_disabled(dw24.ts_pll_enable), >>> ice_clk_src_str(dw24.time_ref_sel), >>> ice_clk_freq_str(dw9.time_ref_freq_sel), >>> bwm_lf.plllock_true_lock_cri ? "locked" : "unlocked"); >>> @@ -548,7 +548,7 @@ static int ice_cfg_cgu_pll_e825c(struct ice_hw *hw, >>> /* Log the current clock configuration */ >>> ice_debug(hw, ICE_DBG_PTP, "Current CGU configuration -- %s, >>> clk_src %s, clk_freq %s, PLL %s\n", >>> - dw24.ts_pll_enable ? "enabled" : "disabled", >>> + str_enabled_disabled(dw24.ts_pll_enable), >>> ice_clk_src_str(dw23.time_ref_sel), >>> ice_clk_freq_str(dw9.time_ref_freq_sel), >>> ro_lock.plllock_true_lock_cri ? "locked" : "unlocked"); >>> @@ -653,7 +653,7 @@ static int ice_cfg_cgu_pll_e825c(struct ice_hw *hw, >>> /* Log the current clock configuration */ >>> ice_debug(hw, ICE_DBG_PTP, "New CGU configuration -- %s, >>> clk_src %s, clk_freq %s, PLL %s\n", >>> - dw24.ts_pll_enable ? "enabled" : "disabled", >>> + str_enabled_disabled(dw24.ts_pll_enable), >>> ice_clk_src_str(dw23.time_ref_sel), >>> ice_clk_freq_str(dw9.time_ref_freq_sel), >>> ro_lock.plllock_true_lock_cri ? "locked" : "unlocked"); >> >
On Mon, 28 Oct 2024, Przemek Kitszel wrote: > On 10/27/24 15:19, R Sundar wrote: > > Use string choice helpers for better readability. > > > > Reported-by: kernel test robot <lkp@intel.com> > > Reported-by: Julia Lawall <julia.lawall@inria.fr> > > Closes: https://lore.kernel.org/r/202410121553.SRNFzc2M-lkp@intel.com/ > > Signed-off-by: R Sundar <prosunofficial@gmail.com> > > --- > > thanks, this indeed covers all "enabled/disabled" cases, so: > Acked-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> > > for future submissions for Intel Ethernet drivers please use the > iwl-next (or iwl-net) target trees. > > There are also other cases that we could cover ON/OFF etc I counted the number of occurrences of various cases. Here are the results for at least 5 occurrences. I converted everything to lowercase and put the two strings in alphabetical order. julia " " "\n ": 5 " (full)" "": 5 " (last)" "": 5 " csc" "": 5 " recoverable" "": 5 "" ".5": 5 "" "1": 5 "" ":" systemlow: 5 "" "\"": 5 "" "_backup": 5 "" "auto-": 5 "" "non": 5 "" "t": 5 "" # x " ": 5 "->" "<-": 5 "070701" "070702": 5 "2.4g" "5g": 5 "2g" dpk->bp[path][kidx].band == 1 ? "5g" : "6g": 5 "80m" dpk->bp[path][kidx].bw == rtw89_channel_width_40 ? "40m" : "20m": 5 "aborted" "completed": 5 "active" "disabled": 5 "anode" "sectors": 5 "assert" "deassert": 5 "attach" "detach": 5 "basic rate" "edr rate": 5 "bulk" "isoc": 5 "client" "server": 5 "closed" "open": 5 "correctable" "uncorrectable": 5 "dedicated" "shared": 5 "fcp" "nvme": 5 "fixed" "roll": 5 "full duplex" "half duplex": 5 "full" "high": 5 "gsi" "smi": 5 "hit" "not hit": 5 "ht20" "ht40": 5 "init" "rt": 5 "ips off" "ips on": 5 "lps off" "lps on": 5 "mc" "uc": 5 "migration" "recovery": 5 "none" "tx": 5 "off!!" "on!!": 5 "pause" "resume": 5 "rf_1t1r" "rf_2t2r": 5 "running" "stopped": 5 "set" "unset": 5 "veb" "vepa": 5 kern_emerg kern_info: 5 " " "\n": 6 " -- kernel too old?" "": 6 " and " "": 6 " flr" "": 6 " iaa" "": 6 " int" "": 6 " pcd" "": 6 " periodic" "": 6 " x4" "": 6 "" ")": 6 "" "*not* ": 6 "" ", ibss": 6 "" ".": 6 "" ":": 6 "" "_flip": 6 "" "amps, ": 6 "" "i": 6 "" "no": 6 "" "pkgpwrl1, ": 6 "" "pkgpwrl2, ": 6 "" "prochot, ": 6 "" "thermstatus, ": 6 "" "vr-therm, ": 6 "(not available)" "(success)": 6 "1" "2": 6 "20m" "40m": 6 "32" "64": 6 "???" "dma write": 6 "active/passive" "passive only": 6 "analog" "digital": 6 "aura" "pool": 6 "beacon" "probe response": 6 "c-vlan" "vlan": 6 "cancelled" "initiated": 6 "capture" "playback": 6 "cc1" "cc2": 6 "decoder" "encoder": 6 "downlink" "uplink": 6 "exmode" "prmode": 6 "found" "lost": 6 "gdt" "ldt": 6 "get" "set": 6 "group" "user": 6 "host" "peripheral": 6 "ipv4" "ipv6": 6 "is" "not": 6 "kill" "on": 6 "ns" "us": 6 "reading" "writing": 6 "recv" "send": 6 " ..." "": 7 " overflow" "": 7 " suspend" "": 7 "" ", nowayout": 7 "" "...": 7 "" "c": 7 "" (#x " "): 7 "" column_sep: 7 "active" "idle": 7 "add" "del": 7 "add" "remove": 7 "autodetected" "insmod option": 7 "capture" "output": 7 "ce" "ue": 7 "dual" "single": 7 "enter" "exit": 7 "fail:" "pass:": 7 "gate" "ungate": 7 "intx" "msi": 7 "private" "shared": 7 "read error" "write error": 7 "read from" "write to": 7 "ro" "rw": 7 kern_debug kern_err: 7 " async" "": 8 " fatal" "": 8 " sof" "": 8 " x16" "": 8 "" "a": 8 "" "b": 8 "10" "100": 8 "40m" "80m": 8 "active" "passive": 8 "bypass" "ram b": 8 "connected" "disconnected": 8 "dcs" "generic": 8 "done" "failed": 8 "dst" "src": 8 "entering" "leaving": 8 "failed" "ok": 8 "failed" "pass": 8 "fast" "slow": 8 "hard" "soft": 8 "invalid" "valid": 8 "kernel" "user": 8 "local" "remote": 8 "primary" "secondary": 8 "ram" "unknown": 8 "restart" "start": 8 "runtime" "system": 8 " err" "": 9 "" "-": 9 "" "/s": 9 "" ": ": 9 "" "[": 9 "" "]": 9 "" "d": 9 "2.4" "5.2": 9 "absent" "present": 9 "fail" "pass": 9 "locked" "unlocked": 9 "offline" "online": 9 "r" "w": 9 "struct" "union": 9 "failed" "success": 53 "" "un": 55 "down" "up": 56 "high" "low": 57 "" "s": 65 "full" "half": 84 "" ",": 86 "not supported" "supported": 91 "" "not ": 96
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c index da88c6ccfaeb..d8d3395e49c3 100644 --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c @@ -393,7 +393,7 @@ static int ice_cfg_cgu_pll_e82x(struct ice_hw *hw, /* Log the current clock configuration */ ice_debug(hw, ICE_DBG_PTP, "Current CGU configuration -- %s, clk_src %s, clk_freq %s, PLL %s\n", - dw24.ts_pll_enable ? "enabled" : "disabled", + str_enabled_disabled(dw24.ts_pll_enable), ice_clk_src_str(dw24.time_ref_sel), ice_clk_freq_str(dw9.time_ref_freq_sel), bwm_lf.plllock_true_lock_cri ? "locked" : "unlocked"); @@ -471,7 +471,7 @@ static int ice_cfg_cgu_pll_e82x(struct ice_hw *hw, /* Log the current clock configuration */ ice_debug(hw, ICE_DBG_PTP, "New CGU configuration -- %s, clk_src %s, clk_freq %s, PLL %s\n", - dw24.ts_pll_enable ? "enabled" : "disabled", + str_enabled_disabled(dw24.ts_pll_enable), ice_clk_src_str(dw24.time_ref_sel), ice_clk_freq_str(dw9.time_ref_freq_sel), bwm_lf.plllock_true_lock_cri ? "locked" : "unlocked"); @@ -548,7 +548,7 @@ static int ice_cfg_cgu_pll_e825c(struct ice_hw *hw, /* Log the current clock configuration */ ice_debug(hw, ICE_DBG_PTP, "Current CGU configuration -- %s, clk_src %s, clk_freq %s, PLL %s\n", - dw24.ts_pll_enable ? "enabled" : "disabled", + str_enabled_disabled(dw24.ts_pll_enable), ice_clk_src_str(dw23.time_ref_sel), ice_clk_freq_str(dw9.time_ref_freq_sel), ro_lock.plllock_true_lock_cri ? "locked" : "unlocked"); @@ -653,7 +653,7 @@ static int ice_cfg_cgu_pll_e825c(struct ice_hw *hw, /* Log the current clock configuration */ ice_debug(hw, ICE_DBG_PTP, "New CGU configuration -- %s, clk_src %s, clk_freq %s, PLL %s\n", - dw24.ts_pll_enable ? "enabled" : "disabled", + str_enabled_disabled(dw24.ts_pll_enable), ice_clk_src_str(dw23.time_ref_sel), ice_clk_freq_str(dw9.time_ref_freq_sel), ro_lock.plllock_true_lock_cri ? "locked" : "unlocked");