Message-Id: <20171120182409.27348-1-mcgrof@kernel.org>
Date:   Mon, 20 Nov 2017 10:23:46 -0800
From:   "Luis R. Rodriguez" <mcgrof@...nel.org>
To:     gregkh@...uxfoundation.org
Cc:     akpm@...ux-foundation.org, keescook@...omium.org,
        mfuzzey@...keon.com, zohar@...ux.vnet.ibm.com, dhowells@...hat.com,
        pali.rohar@...il.com, tiwai@...e.de, arend.vanspriel@...adcom.com,
        zajec5@...il.com, nbroeking@...com, markivx@...eaurora.org,
        stephen.boyd@...aro.org, broonie@...nel.org,
        dmitry.torokhov@...il.com, dwmw2@...radead.org,
        torvalds@...ux-foundation.org, Abhay_Salunke@...l.com,
        bjorn.andersson@...aro.org, jewalt@...innovations.com,
        linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        "Luis R. Rodriguez" <mcgrof@...nel.org>
Subject: [PATCH v2 00/23] firmware: cleanup for v4.16

Here's a series of cleanups for the firmware loader for consideration for
v4.16, *after* the traditional yearly American turkey massacre. This
depends on the few fixes I just posted for consideration for v4.15 [0].

While discussing the cleanup patch I posted last for the firmware API [1],
Greg suggested making the new data structure we use an opaque pointer so
we can avoid modifying so many call sites needing the new parameter. This
was a good suggestion, but we already have our own private opaque data
structure, it was just used for storing a few variables. We also have
our own fw_priv data structure, its however a complete misnomer given
it actually is only used now for the old sysfs firmware loading facility.

The naming mishap happened given the sysfs loader was the default loader back
in the hay day. This is no longer the case, we always directly load firmware
from the filesystem first, and only rely on the sysfs loader mechanism as
a fallback, if its enabled.

So we rename the old fw_priv to reflect its for sysfs only, and re-use the name
now for our existing opaque private data structure, which we'll extend for the
original proposed cleanup.

IMHO this ends up tidying up the code even more and makes the code *much*
easier to read.

While doing this work I also realized how with a small amount of work we
could end up testing the 3 different functional kernel builds possible for
the firmware loader with *one* kernel build with a few debugfs knobs. This
adds this as well and also paves the way to enable us to later split the
sysfs firmware loader onto its own file.

Worth discussing is how the debugfs knobs also reveal how two kernel build
configs could potentially just later end up as module params, if we're happy
to to live with implicating the fallback mechanism always when the firmware
loader is enabled. This would simplfy the kernel build considerably. I'm not
aware of anyone disabling the syfs fallback mechanism completely, given its
only a fallback mechanism now and we have a way to avoid it completely as
well for sync calls, so this very well may be worth a direction worth
evaluating forward

0-day has been happy with this. I've also hammered testing the API on the 3
different kernel builds possible, no regressions were found, and also addressed
ensuring that the selftests still works on older kernels as expected. On older
kernels we run the typical test of two scripts as usual, using whatever
whatever defaults your kernel provides and doing our best with that.

Comments and questions are, as always, welcome. 

[0] https://lkml.kernel.org/r/20171120174535.27000-1-mcgrof@kernel.org
[1] https://lkml.kernel.org/r/20170914225422.31034-1-mcgrof@kernel.org

Luis R. Rodriguez (23):
  firmware: rename struct firmware_priv to struct fw_sysfs
  firmware: rename struct firmware_buf to struct fw_priv
  firmware: rename struct fw_priv->fw_id to fw_name
  firmware: move core data structures to the top of file
  firmware: remove duplicate fw_state_aborted()
  firmware: remove unused __fw_state_is_done()
  firmware: use static inlines for state machine checking
  firmware: rename sysfs state checks with sysfs prefix
  firmware: use static inline for to_fw_priv()
  firmware: add helper to copy built-in data to pre-alloc buffer
  firmware: provide helper for FW_OPT_USERHELPER
  firmware: replace #ifdef over FW_OPT_FALLBACK with function checks
  test_firmware: wrap sysfs timeout test into helper
  test_firmware: wrap basic sysfs fallback tests into helper
  test_firmware: wrap custom sysfs load tests into helper
  test_firmware: enable custom fallback testing on limited kernel
    configs
  test_firmware: replace syfs fallback check with kconfig_has helper
  firmware: enable to split firmware_class into separate target files
  firmware: add debug facility to emulate forcing sysfs fallback
  firmware: add debug facility to emulate disabling sysfs fallback
  test_firmware: add a library for shared helpers
  test_firmware: test the 3 firmware kernel configs using debugfs
  firmware: cleanup - group and document up private firmware parameters

 drivers/base/Kconfig                               |   6 +
 drivers/base/Makefile                              |   2 +
 drivers/base/firmware_debug.c                      |  39 +
 drivers/base/firmware_debug.h                      |  58 ++
 .../base/{firmware_class.c => firmware_loader.c}   | 981 +++++++++++++--------
 tools/testing/selftests/firmware/Makefile          |   2 +-
 tools/testing/selftests/firmware/config            |   5 +
 tools/testing/selftests/firmware/fw_fallback.sh    | 207 +++--
 tools/testing/selftests/firmware/fw_filesystem.sh  |  61 +-
 tools/testing/selftests/firmware/fw_lib.sh         | 174 ++++
 tools/testing/selftests/firmware/fw_run_tests.sh   |  67 ++
 11 files changed, 1064 insertions(+), 538 deletions(-)
 create mode 100644 drivers/base/firmware_debug.c
 create mode 100644 drivers/base/firmware_debug.h
 rename drivers/base/{firmware_class.c => firmware_loader.c} (64%)
 create mode 100755 tools/testing/selftests/firmware/fw_lib.sh
 create mode 100755 tools/testing/selftests/firmware/fw_run_tests.sh

-- 
2.15.0

Powered by blists - more mailing lists