linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/11] lib/string_helpers: Add a few string helpers
@ 2022-01-26  9:39 Lucas De Marchi
  2022-01-26  9:39 ` [PATCH v2 01/11] lib/string_helpers: Consolidate string helpers implementation Lucas De Marchi
                   ` (11 more replies)
  0 siblings, 12 replies; 24+ messages in thread
From: Lucas De Marchi @ 2022-01-26  9:39 UTC (permalink / raw)
  To: linux-kernel, intel-gfx, dri-devel, amd-gfx,
	linux-security-module, nouveau, netdev
  Cc: Alex Deucher, Andrew Morton, Andy Shevchenko, Andy Shevchenko,
	Ben Skeggs, Christian König, Chris Wilson, Daniel Vetter,
	David Airlie, David S. Miller, Emma Anholt, Francis Laniel,
	Greg Kroah-Hartman, Harry Wentland, Jakub Kicinski, Jani Nikula,
	Joonas Lahtinen, Julia Lawall, Kentaro Takeda, Leo Li,
	Petr Mladek, Rahul Lakkireddy, Raju Rangoju, Rasmus Villemoes,
	Rodrigo Vivi, Sakari Ailus, Sergey Senozhatsky, Steven Rostedt,
	Vishal Kulkarni

Add some helpers under lib/string_helpers.h so they can be used
throughout the kernel. When I started doing this there were 2 other
previous attempts I know of, not counting the iterations each of them
had:

1) https://lore.kernel.org/all/20191023131308.9420-1-jani.nikula@intel.com/
2) https://lore.kernel.org/all/20210215142137.64476-1-andriy.shevchenko@linux.intel.com/#t

Now there is also the v1 of this same patch series:
https://lore.kernel.org/all/20220119072450.2890107-1-lucas.demarchi@intel.com/

Going through the comments I tried to find some common ground and
justification for what is in here, addressing some of the concerns
raised.

a. This version should be a drop-in replacement for what is currently in
   the tree, with no change in behavior or binary size. For binary
   size what I checked was that the linked objects in the end have the
   same size (gcc 11). From comments in the previous attempts this seems
   also the case for earlier compiler versions

b. I didn't change the function name to choice_* as suggested by Andrew
   Morton in 20191023155619.43e0013f0c8c673a5c508c1e@linux-foundation.org
   because other people argumented in favor of shorter names for these
   simple helpers - if they are long and people simply not use due to
   that, we failed. However as pointed out in v1 of this patchseries,
   onoff(), yesno(), enabledisable(), enableddisabled() have some
   issues: the last 2 are hard to read and for the first 2 it would not
   be hard to have the symbol to clash with variable names.
   From comments in v1, most people were in favor (or at least not
   opposed) to using str_on_off(), str_yes_no(), str_enable_disable()
   and str_enabled_disabled().

c. Use string_helper.h for these helpers - pulling string.h in the
   compilations units was one of the concerns and I think re-using this
   already existing header is better than creating a new string-choice.h

d. One alternative to all of this suggested by Christian König
   (43456ba7-c372-84cc-4949-dcb817188e21@amd.com) would be to add a
   printk format. But besides the comment, he also seemed to like
   the common function. This brought the argument from others that the
   simple yesno()/enabledisable() already used in the code (or new
   renamed version) is easier to remember and use than e.g. %py[DOY]


Changes in v2:

  - Use str_ prefix and separate other words with underscore: it's a
    little bit longer, but should improve readability

  - Patches we re-split due to the rename: first patch adds all the new
    functions, then additional patches try to do one conversion at a
    time. While doing so, there were some fixes for issues already
    present along the way

  - Style suggestions from v1 were adopted

In v1 it was suggested to apply this in drm-misc. I will leave this to
maintainers to decide: maybe it would be simpler to merge the first
patches on drm-intel-next, wait for the back merge and merge the rest
through drm-misc - my fear is a big conflict with other work going in
drm-intel-next since the bulk of the rename is there.

I tried to figure out acks and reviews from v1 and apply them to how the
patches are now split.

thanks
Lucas De Marchi

Lucas De Marchi (11):
  lib/string_helpers: Consolidate string helpers implementation
  drm/i915: Fix trailing semicolon
  drm/i915: Use str_yes_no()
  drm/i915: Use str_enable_disable()
  drm/i915: Use str_enabled_disabled()
  drm/i915: Use str_on_off()
  drm/amd/display: Use str_yes_no()
  drm/gem: Sort includes alphabetically
  drm: Convert open-coded yes/no strings to yesno()
  tomoyo: Use str_yes_no()
  cxgb4: Use str_yes_no()

 drivers/gpu/drm/amd/amdgpu/atom.c             |   4 +-
 .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c |  14 +-
 drivers/gpu/drm/dp/drm_dp.c                   |   3 +-
 drivers/gpu/drm/drm_client_modeset.c          |   3 +-
 drivers/gpu/drm/drm_gem.c                     |  23 +-
 drivers/gpu/drm/i915/display/g4x_dp.c         |   6 +-
 .../gpu/drm/i915/display/intel_backlight.c    |   3 +-
 drivers/gpu/drm/i915/display/intel_ddi.c      |   4 +-
 drivers/gpu/drm/i915/display/intel_display.c  |  46 ++--
 .../drm/i915/display/intel_display_debugfs.c  |  74 +++---
 .../drm/i915/display/intel_display_power.c    |   4 +-
 .../drm/i915/display/intel_display_trace.h    |   9 +-
 drivers/gpu/drm/i915/display/intel_dp.c       |  20 +-
 drivers/gpu/drm/i915/display/intel_dpll.c     |   3 +-
 drivers/gpu/drm/i915/display/intel_dpll_mgr.c |   7 +-
 drivers/gpu/drm/i915/display/intel_dsi_vbt.c  |   7 +-
 drivers/gpu/drm/i915/display/intel_fbc.c      |   4 +-
 drivers/gpu/drm/i915/display/intel_fdi.c      |   8 +-
 drivers/gpu/drm/i915/display/intel_hdmi.c     |   3 +-
 drivers/gpu/drm/i915/display/intel_sprite.c   |   6 +-
 drivers/gpu/drm/i915/display/vlv_dsi_pll.c    |   3 +-
 .../gpu/drm/i915/gem/selftests/huge_pages.c   |   9 +-
 .../drm/i915/gem/selftests/i915_gem_context.c |   7 +-
 drivers/gpu/drm/i915/gt/intel_breadcrumbs.c   |   3 +-
 drivers/gpu/drm/i915/gt/intel_engine_cs.c     |  11 +-
 .../drm/i915/gt/intel_execlists_submission.c  |   7 +-
 drivers/gpu/drm/i915/gt/intel_gt_pm.c         |   3 +-
 drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c |  52 ++--
 drivers/gpu/drm/i915/gt/intel_rc6.c           |   5 +-
 drivers/gpu/drm/i915/gt/intel_reset.c         |   3 +-
 drivers/gpu/drm/i915/gt/intel_rps.c           |  13 +-
 drivers/gpu/drm/i915/gt/intel_sseu.c          |   9 +-
 drivers/gpu/drm/i915/gt/intel_sseu_debugfs.c  |  10 +-
 drivers/gpu/drm/i915/gt/selftest_timeline.c   |   3 +-
 drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c     |   5 +-
 drivers/gpu/drm/i915/gt/uc/intel_guc_log.c    |   5 +-
 drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c     |   6 +-
 drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c   |   4 +-
 drivers/gpu/drm/i915/gt/uc/intel_uc.c         |  14 +-
 drivers/gpu/drm/i915/gt/uc/intel_uc_debugfs.c |  20 +-
 drivers/gpu/drm/i915/i915_debugfs.c           |  17 +-
 drivers/gpu/drm/i915/i915_driver.c            |   4 +-
 drivers/gpu/drm/i915/i915_gpu_error.c         |   9 +-
 drivers/gpu/drm/i915/i915_params.c            |   5 +-
 drivers/gpu/drm/i915/i915_utils.h             |  21 +-
 drivers/gpu/drm/i915/intel_device_info.c      |   8 +-
 drivers/gpu/drm/i915/intel_dram.c             |  10 +-
 drivers/gpu/drm/i915/intel_pm.c               |  14 +-
 drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.c  |   4 +-
 drivers/gpu/drm/i915/selftests/i915_active.c  |   3 +-
 drivers/gpu/drm/i915/vlv_suspend.c            |   3 +-
 drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c |   5 +-
 drivers/gpu/drm/radeon/atom.c                 |   3 +-
 drivers/gpu/drm/v3d/v3d_debugfs.c             |  11 +-
 drivers/gpu/drm/virtio/virtgpu_debugfs.c      |   4 +-
 .../ethernet/chelsio/cxgb4/cxgb4_debugfs.c    | 249 ++++++++++--------
 include/linux/string_helpers.h                |  20 ++
 security/tomoyo/audit.c                       |   2 +-
 security/tomoyo/common.c                      |  19 +-
 security/tomoyo/common.h                      |   1 -
 60 files changed, 482 insertions(+), 373 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2022-02-01 21:25 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-26  9:39 [PATCH v2 00/11] lib/string_helpers: Add a few string helpers Lucas De Marchi
2022-01-26  9:39 ` [PATCH v2 01/11] lib/string_helpers: Consolidate string helpers implementation Lucas De Marchi
2022-01-26  9:39 ` [PATCH v2 02/11] drm/i915: Fix trailing semicolon Lucas De Marchi
2022-01-26 14:52   ` Jani Nikula
2022-01-26  9:39 ` [PATCH v2 03/11] drm/i915: Use str_yes_no() Lucas De Marchi
2022-02-01 21:25   ` [Intel-gfx] " Matt Roper
2022-01-26  9:39 ` [PATCH v2 04/11] drm/i915: Use str_enable_disable() Lucas De Marchi
2022-02-01 21:23   ` [Intel-gfx] " Matt Roper
2022-01-26  9:39 ` [PATCH v2 05/11] drm/i915: Use str_enabled_disabled() Lucas De Marchi
2022-02-01 21:21   ` [Intel-gfx] " Matt Roper
2022-01-26  9:39 ` [PATCH v2 06/11] drm/i915: Use str_on_off() Lucas De Marchi
2022-02-01 21:15   ` [Intel-gfx] " Matt Roper
2022-01-26  9:39 ` [PATCH v2 07/11] drm/amd/display: Use str_yes_no() Lucas De Marchi
2022-01-26 14:35   ` Harry Wentland
2022-01-26  9:39 ` [PATCH v2 08/11] drm/gem: Sort includes alphabetically Lucas De Marchi
2022-01-26 14:54   ` Jani Nikula
2022-01-26  9:39 ` [PATCH v2 09/11] drm: Convert open-coded yes/no strings to yesno() Lucas De Marchi
2022-01-26 10:12   ` Andy Shevchenko
2022-01-26 10:43     ` [Intel-gfx] " Lucas De Marchi
2022-01-26 12:15       ` Andy Shevchenko
2022-01-26 14:57   ` Jani Nikula
2022-01-26  9:39 ` [PATCH v2 10/11] tomoyo: Use str_yes_no() Lucas De Marchi
2022-01-26  9:39 ` [PATCH v2 11/11] cxgb4: " Lucas De Marchi
2022-01-26 10:22 ` [PATCH v2 00/11] lib/string_helpers: Add a few string helpers Andy Shevchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).