2024-02-22 18:01:25

by Mukesh Ojha

[permalink] [raw]
Subject: [PATCH vRFC 0/8] Refactor and rename request firmware API

This came as an idea, when i was looking for nowarn version
of request_firmware_into_buf() but ended up doing this.

I am keeping this as RFC, as i am not sure whether we are
open for such changes in API or some refactors.
Please provide feedback.

Mukesh Ojha (8):
firmware_loader: Refactor request firmware lower level functions
treewide: rename firmware_request_nowarn()
treewide: rename firmware_request_platform()
treewide: rename firmware_request_cache()
firmware: Convert minor inline function to macro
firmware: Move module template to the bottom
firmware: remove prototype of fw_cache_piggyback_on_request()
firmware: FW_OPT_UEVENT for all request_firmware family functions

.../firmware/fallback-mechanisms.rst | 4 +-
.../driver-api/firmware/lookup-order.rst | 2 +-
.../driver-api/firmware/request_firmware.rst | 14 +-
drivers/accel/ivpu/ivpu_fw.c | 2 +-
drivers/base/firmware_loader/main.c | 226 ++++++++----------
drivers/bluetooth/btbcm.c | 2 +-
drivers/bluetooth/btintel.c | 4 +-
drivers/bluetooth/hci_bcm4377.c | 4 +-
drivers/crypto/ccp/sev-dev.c | 6 +-
drivers/crypto/inside-secure/safexcel.c | 4 +-
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 2 +-
drivers/gpu/drm/nouveau/nvkm/core/firmware.c | 2 +-
drivers/input/touchscreen/chipone_icn8505.c | 2 +-
drivers/input/touchscreen/silead.c | 4 +-
drivers/media/tuners/si2157.c | 2 +-
drivers/memory/brcmstb_dpfe.c | 2 +-
drivers/net/ethernet/intel/ice/ice_main.c | 2 +-
drivers/net/wireless/ath/ath10k/core.c | 2 +-
drivers/net/wireless/ath/ath10k/testmode.c | 2 +-
drivers/net/wireless/ath/ath11k/core.c | 2 +-
drivers/net/wireless/ath/ath12k/core.c | 2 +-
.../broadcom/brcm80211/brcmfmac/firmware.c | 2 +-
drivers/net/wireless/intel/iwlwifi/fw/pnvm.c | 2 +-
.../net/wireless/intel/iwlwifi/iwl-dbg-tlv.c | 2 +-
.../net/wireless/mediatek/mt76/mt7615/mcu.c | 2 +-
.../wireless/mediatek/mt76/mt76x0/usb_mcu.c | 2 +-
drivers/net/wireless/mediatek/mt7601u/mcu.c | 2 +-
drivers/net/wireless/realtek/rtw89/fw.c | 2 +-
drivers/net/wireless/silabs/wfx/fwio.c | 2 +-
drivers/usb/host/xhci-pci-renesas.c | 2 +-
include/linux/firmware.h | 10 +-
lib/test_firmware.c | 2 +-
sound/pci/hda/cs35l41_hda.c | 2 +-
sound/pci/hda/cs35l56_hda.c | 2 +-
sound/soc/codecs/wm_adsp.c | 2 +-
sound/soc/sof/fw-file-profile.c | 4 +-
36 files changed, 152 insertions(+), 180 deletions(-)

--
2.43.0.254.ga26002b62827



2024-02-22 18:02:31

by Mukesh Ojha

[permalink] [raw]
Subject: [PATCH vRFC 2/8] treewide: rename firmware_request_nowarn()

Rename firmware_request_nowarn() to align with
other request_firmware family functions.

Signed-off-by: Mukesh Ojha <[email protected]>
---
Documentation/driver-api/firmware/request_firmware.rst | 4 ++--
drivers/accel/ivpu/ivpu_fw.c | 2 +-
drivers/base/firmware_loader/main.c | 6 +++---
drivers/bluetooth/btbcm.c | 2 +-
drivers/bluetooth/btintel.c | 4 ++--
drivers/bluetooth/hci_bcm4377.c | 4 ++--
drivers/crypto/ccp/sev-dev.c | 6 +++---
drivers/crypto/inside-secure/safexcel.c | 4 ++--
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 2 +-
drivers/gpu/drm/nouveau/nvkm/core/firmware.c | 2 +-
drivers/input/touchscreen/silead.c | 2 +-
drivers/media/tuners/si2157.c | 2 +-
drivers/memory/brcmstb_dpfe.c | 2 +-
drivers/net/ethernet/intel/ice/ice_main.c | 2 +-
drivers/net/wireless/ath/ath10k/core.c | 2 +-
drivers/net/wireless/ath/ath10k/testmode.c | 2 +-
drivers/net/wireless/ath/ath11k/core.c | 2 +-
drivers/net/wireless/ath/ath12k/core.c | 2 +-
drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c | 2 +-
drivers/net/wireless/intel/iwlwifi/fw/pnvm.c | 2 +-
drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c | 2 +-
drivers/net/wireless/mediatek/mt76/mt7615/mcu.c | 2 +-
drivers/net/wireless/mediatek/mt76/mt76x0/usb_mcu.c | 2 +-
drivers/net/wireless/realtek/rtw89/fw.c | 2 +-
drivers/net/wireless/silabs/wfx/fwio.c | 2 +-
drivers/usb/host/xhci-pci-renesas.c | 2 +-
include/linux/firmware.h | 4 ++--
sound/pci/hda/cs35l41_hda.c | 2 +-
sound/pci/hda/cs35l56_hda.c | 2 +-
sound/soc/codecs/wm_adsp.c | 2 +-
sound/soc/sof/fw-file-profile.c | 4 ++--
31 files changed, 41 insertions(+), 41 deletions(-)

diff --git a/Documentation/driver-api/firmware/request_firmware.rst b/Documentation/driver-api/firmware/request_firmware.rst
index 0d6ea0329995..0201334bc308 100644
--- a/Documentation/driver-api/firmware/request_firmware.rst
+++ b/Documentation/driver-api/firmware/request_firmware.rst
@@ -20,10 +20,10 @@ request_firmware
.. kernel-doc:: drivers/base/firmware_loader/main.c
:functions: request_firmware

-firmware_request_nowarn
+request_firmware_nowarn
-----------------------
.. kernel-doc:: drivers/base/firmware_loader/main.c
- :functions: firmware_request_nowarn
+ :functions: request_firmware_nowarn

firmware_request_platform
-------------------------
diff --git a/drivers/accel/ivpu/ivpu_fw.c b/drivers/accel/ivpu/ivpu_fw.c
index 6576232f3e67..11c27fe860f2 100644
--- a/drivers/accel/ivpu/ivpu_fw.c
+++ b/drivers/accel/ivpu/ivpu_fw.c
@@ -76,7 +76,7 @@ static int ivpu_fw_request(struct ivpu_device *vdev)
if (fw_names[i].gen != ivpu_hw_gen(vdev))
continue;

- ret = firmware_request_nowarn(&vdev->fw->file, fw_names[i].name, vdev->drm.dev);
+ ret = request_firmware_nowarn(&vdev->fw->file, fw_names[i].name, vdev->drm.dev);
if (!ret) {
vdev->fw->name = fw_names[i].name;
return 0;
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index 645d658facae..cb681f4069b8 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -975,7 +975,7 @@ request_firmware(const struct firmware **firmware_p, const char *name,
EXPORT_SYMBOL(request_firmware);

/**
- * firmware_request_nowarn() - request for an optional fw module
+ * request_firmware_nowarn() - request for an optional fw module
* @firmware: pointer to firmware image
* @name: name of firmware file
* @device: device for which firmware is being loaded
@@ -987,13 +987,13 @@ EXPORT_SYMBOL(request_firmware);
* therefore up to the driver to check for the return value of this call and to
* decide when to inform the users of errors.
**/
-int firmware_request_nowarn(const struct firmware **firmware, const char *name,
+int request_firmware_nowarn(const struct firmware **firmware, const char *name,
struct device *device)
{
return _request_firmware(firmware, name, device, NULL, 0, 0,
FW_OPT_UEVENT | FW_OPT_NO_WARN);
}
-EXPORT_SYMBOL_GPL(firmware_request_nowarn);
+EXPORT_SYMBOL_GPL(request_firmware_nowarn);

/**
* request_firmware_direct() - load firmware directly without usermode helper
diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
index 0a5445ac5e1b..94ebf6113c44 100644
--- a/drivers/bluetooth/btbcm.c
+++ b/drivers/bluetooth/btbcm.c
@@ -672,7 +672,7 @@ int btbcm_initialize(struct hci_dev *hdev, bool *fw_load_done, bool use_autobaud
fw_name_count++;

for (i = 0; i < fw_name_count; i++) {
- err = firmware_request_nowarn(&fw, fw_name[i], &hdev->dev);
+ err = request_firmware_nowarn(&fw, fw_name[i], &hdev->dev);
if (err == 0) {
bt_dev_info(hdev, "%s '%s' Patch",
hw_name ? hw_name : "BCM", fw_name[i]);
diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index cdc5c08824a0..6ec90754a76f 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -2057,7 +2057,7 @@ static int btintel_download_fw(struct hci_dev *hdev,
return -EINVAL;
}

- err = firmware_request_nowarn(&fw, fwname, &hdev->dev);
+ err = request_firmware_nowarn(&fw, fwname, &hdev->dev);
if (err < 0) {
if (!btintel_test_flag(hdev, INTEL_BOOTLOADER)) {
/* Firmware has already been loaded */
@@ -2246,7 +2246,7 @@ static int btintel_prepare_fw_download_tlv(struct hci_dev *hdev,
}

btintel_get_fw_name_tlv(ver, fwname, sizeof(fwname), "sfi");
- err = firmware_request_nowarn(&fw, fwname, &hdev->dev);
+ err = request_firmware_nowarn(&fw, fwname, &hdev->dev);
if (err < 0) {
if (!btintel_test_flag(hdev, INTEL_BOOTLOADER)) {
/* Firmware has already been loaded */
diff --git a/drivers/bluetooth/hci_bcm4377.c b/drivers/bluetooth/hci_bcm4377.c
index 9a7243d5db71..fd06707a6709 100644
--- a/drivers/bluetooth/hci_bcm4377.c
+++ b/drivers/bluetooth/hci_bcm4377.c
@@ -1207,10 +1207,10 @@ static const struct firmware *bcm4377_request_blob(struct bcm4377_data *bcm4377,
dev_dbg(&bcm4377->pdev->dev, "Trying to load firmware: '%s' or '%s'\n",
name0, name1);

- ret = firmware_request_nowarn(&fw, name0, &bcm4377->pdev->dev);
+ ret = request_firmware_nowarn(&fw, name0, &bcm4377->pdev->dev);
if (!ret)
return fw;
- ret = firmware_request_nowarn(&fw, name1, &bcm4377->pdev->dev);
+ ret = request_firmware_nowarn(&fw, name1, &bcm4377->pdev->dev);
if (!ret)
return fw;

diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index e4d3f45242f6..a2ded1f9d940 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -761,9 +761,9 @@ static int sev_get_firmware(struct device *dev,
*
* Fall-back to using generic name: sev.fw
*/
- if ((firmware_request_nowarn(firmware, fw_name_specific, dev) >= 0) ||
- (firmware_request_nowarn(firmware, fw_name_subset, dev) >= 0) ||
- (firmware_request_nowarn(firmware, SEV_FW_FILE, dev) >= 0))
+ if ((request_firmware_nowarn(firmware, fw_name_specific, dev) >= 0) ||
+ (request_firmware_nowarn(firmware, fw_name_subset, dev) >= 0) ||
+ (request_firmware_nowarn(firmware, SEV_FW_FILE, dev) >= 0))
return 0;

return -ENOENT;
diff --git a/drivers/crypto/inside-secure/safexcel.c b/drivers/crypto/inside-secure/safexcel.c
index f5c1912aa564..791a7ff25371 100644
--- a/drivers/crypto/inside-secure/safexcel.c
+++ b/drivers/crypto/inside-secure/safexcel.c
@@ -429,7 +429,7 @@ static int eip197_load_firmwares(struct safexcel_crypto_priv *priv)
retry_fw:
for (i = 0; i < FW_NB; i++) {
snprintf(fw_path, 37, "inside-secure/%s/%s", dir, fw_name[i]);
- ret = firmware_request_nowarn(&fw[i], fw_path, priv->dev);
+ ret = request_firmware_nowarn(&fw[i], fw_path, priv->dev);
if (ret) {
if (minifw || priv->data->version != EIP197B_MRVL)
goto release_fw;
@@ -437,7 +437,7 @@ static int eip197_load_firmwares(struct safexcel_crypto_priv *priv)
/* Fallback to the old firmware location for the
* EIP197b.
*/
- ret = firmware_request_nowarn(&fw[i], fw_name[i],
+ ret = request_firmware_nowarn(&fw[i], fw_name[i],
priv->dev);
if (ret)
goto release_fw;
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
index 362639162ed6..205cf027870f 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -785,7 +785,7 @@ static int try_firmware_load(struct intel_uc_fw *uc_fw, const struct firmware **
struct device *dev = gt->i915->drm.dev;
int err;

- err = firmware_request_nowarn(fw, uc_fw->file_selected.path, dev);
+ err = request_firmware_nowarn(fw, uc_fw->file_selected.path, dev);

if (err)
return err;
diff --git a/drivers/gpu/drm/nouveau/nvkm/core/firmware.c b/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
index adc60b25f8e6..febfabb17405 100644
--- a/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
+++ b/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
@@ -91,7 +91,7 @@ nvkm_firmware_get(const struct nvkm_subdev *subdev, const char *fwname, int ver,
else
snprintf(f, sizeof(f), "nvidia/%s/%s.bin", cname, fwname);

- if (!firmware_request_nowarn(fw, f, device->dev)) {
+ if (!request_firmware_nowarn(fw, f, device->dev)) {
nvkm_debug(subdev, "firmware \"%s\" loaded - %zu byte(s)\n",
f, (*fw)->size);
return 0;
diff --git a/drivers/input/touchscreen/silead.c b/drivers/input/touchscreen/silead.c
index 62f562ad5026..52477e450b02 100644
--- a/drivers/input/touchscreen/silead.c
+++ b/drivers/input/touchscreen/silead.c
@@ -426,7 +426,7 @@ static int silead_ts_load_fw(struct i2c_client *client)
* succeeds we apply an (optional) set of alternative min/max values from the
* "silead,efi-fw-min-max" property.
*/
- error = firmware_request_nowarn(&fw, data->fw_name, dev);
+ error = request_firmware_nowarn(&fw, data->fw_name, dev);
if (error) {
error = firmware_request_platform(&fw, data->fw_name, dev);
if (error) {
diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index def06c262ea2..64fbb24c0758 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -98,7 +98,7 @@ static int si2157_load_firmware(struct dvb_frontend *fe,
struct si2157_cmd cmd;

/* request the firmware, this will block and timeout */
- ret = firmware_request_nowarn(&fw, fw_name, &client->dev);
+ ret = request_firmware_nowarn(&fw, fw_name, &client->dev);
if (ret)
return ret;

diff --git a/drivers/memory/brcmstb_dpfe.c b/drivers/memory/brcmstb_dpfe.c
index 5028467b2dc9..a6e6412416d8 100644
--- a/drivers/memory/brcmstb_dpfe.c
+++ b/drivers/memory/brcmstb_dpfe.c
@@ -648,7 +648,7 @@ static int brcmstb_dpfe_download_firmware(struct brcmstb_dpfe_priv *priv)
if (!priv->dpfe_api->fw_name)
return -ENODEV;

- ret = firmware_request_nowarn(&fw, priv->dpfe_api->fw_name, dev);
+ ret = request_firmware_nowarn(&fw, priv->dpfe_api->fw_name, dev);
/*
* Defer the firmware download if the firmware file couldn't be found.
* The root file system may not be available yet.
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index dd4a9bc0dfdc..46d9e0f1bbf4 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -4428,7 +4428,7 @@ static void ice_request_fw(struct ice_pf *pf)
* and warning messages for other errors.
*/
if (opt_fw_filename) {
- err = firmware_request_nowarn(&firmware, opt_fw_filename, dev);
+ err = request_firmware_nowarn(&firmware, opt_fw_filename, dev);
if (err) {
kfree(opt_fw_filename);
goto dflt_pkg_load;
diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 0032f8aa892f..dc57a4319ae7 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -943,7 +943,7 @@ static const struct firmware *ath10k_fetch_fw_file(struct ath10k *ar,
dir = ".";

snprintf(filename, sizeof(filename), "%s/%s", dir, file);
- ret = firmware_request_nowarn(&fw, filename, ar->dev);
+ ret = request_firmware_nowarn(&fw, filename, ar->dev);
ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot fw request '%s': %d\n",
filename, ret);

diff --git a/drivers/net/wireless/ath/ath10k/testmode.c b/drivers/net/wireless/ath/ath10k/testmode.c
index 7a9b9bbcdbfc..b9f976786a7c 100644
--- a/drivers/net/wireless/ath/ath10k/testmode.c
+++ b/drivers/net/wireless/ath/ath10k/testmode.c
@@ -146,7 +146,7 @@ static int ath10k_tm_fetch_utf_firmware_api_1(struct ath10k *ar,
ar->hw_params.fw.dir, ATH10K_FW_UTF_FILE);

/* load utf firmware image */
- ret = firmware_request_nowarn(&fw_file->firmware, filename, ar->dev);
+ ret = request_firmware_nowarn(&fw_file->firmware, filename, ar->dev);
ath10k_dbg(ar, ATH10K_DBG_TESTMODE, "testmode fw request '%s': %d\n",
filename, ret);

diff --git a/drivers/net/wireless/ath/ath11k/core.c b/drivers/net/wireless/ath/ath11k/core.c
index 0c6ecbb9a066..9d800a8415d4 100644
--- a/drivers/net/wireless/ath/ath11k/core.c
+++ b/drivers/net/wireless/ath/ath11k/core.c
@@ -1084,7 +1084,7 @@ const struct firmware *ath11k_core_firmware_request(struct ath11k_base *ab,

ath11k_core_create_firmware_path(ab, file, path, sizeof(path));

- ret = firmware_request_nowarn(&fw, path, ab->dev);
+ ret = request_firmware_nowarn(&fw, path, ab->dev);
if (ret)
return ERR_PTR(ret);

diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
index 6c01b282fcd3..d160791019c8 100644
--- a/drivers/net/wireless/ath/ath12k/core.c
+++ b/drivers/net/wireless/ath/ath12k/core.c
@@ -137,7 +137,7 @@ const struct firmware *ath12k_core_firmware_request(struct ath12k_base *ab,

ath12k_core_create_firmware_path(ab, file, path, sizeof(path));

- ret = firmware_request_nowarn(&fw, path, ab->dev);
+ ret = request_firmware_nowarn(&fw, path, ab->dev);
if (ret)
return ERR_PTR(ret);

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
index 83f8ed7d00f9..b0451d97b8e2 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
@@ -659,7 +659,7 @@ static int brcmf_fw_request_firmware(const struct firmware **fw,
if (!alt_path)
goto fallback;

- ret = firmware_request_nowarn(fw, alt_path, fwctx->dev);
+ ret = request_firmware_nowarn(fw, alt_path, fwctx->dev);
kfree(alt_path);
if (ret == 0)
return ret;
diff --git a/drivers/net/wireless/intel/iwlwifi/fw/pnvm.c b/drivers/net/wireless/intel/iwlwifi/fw/pnvm.c
index 650e4bde9c17..8b3676e87e07 100644
--- a/drivers/net/wireless/intel/iwlwifi/fw/pnvm.c
+++ b/drivers/net/wireless/intel/iwlwifi/fw/pnvm.c
@@ -231,7 +231,7 @@ static int iwl_pnvm_get_from_fs(struct iwl_trans *trans, u8 **data, size_t *len)

iwl_pnvm_get_fs_name(trans, pnvm_name, sizeof(pnvm_name));

- ret = firmware_request_nowarn(&pnvm, pnvm_name, trans->dev);
+ ret = request_firmware_nowarn(&pnvm, pnvm_name, trans->dev);
if (ret) {
IWL_DEBUG_FW(trans, "PNVM file %s not found %d\n",
pnvm_name, ret);
diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c b/drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c
index 3b14f6476743..dc139849454f 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c
@@ -503,7 +503,7 @@ void iwl_dbg_tlv_load_bin(struct device *dev, struct iwl_trans *trans)
trans->trans_cfg->device_family <= IWL_DEVICE_FAMILY_8000)
return;

- res = firmware_request_nowarn(&fw, yoyo_bin, dev);
+ res = request_firmware_nowarn(&fw, yoyo_bin, dev);
IWL_DEBUG_FW(trans, "%s %s\n", res ? "didn't load" : "loaded", yoyo_bin);

if (res)
diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
index ae34d019e588..eceaee233993 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
@@ -1224,7 +1224,7 @@ static int mt7615_load_patch(struct mt7615_dev *dev, u32 addr, const char *name)
const struct firmware *fw = NULL;
int len, ret, sem;

- ret = firmware_request_nowarn(&fw, name, dev->mt76.dev);
+ ret = request_firmware_nowarn(&fw, name, dev->mt76.dev);
if (ret)
return ret;

diff --git a/drivers/net/wireless/mediatek/mt76/mt76x0/usb_mcu.c b/drivers/net/wireless/mediatek/mt76/mt76x0/usb_mcu.c
index 6dc1f51f5658..3e1b0db7fd10 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x0/usb_mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x0/usb_mcu.c
@@ -72,7 +72,7 @@ static int mt76x0_get_firmware(struct mt76x02_dev *dev,
/* try to load mt7610e fw if available
* otherwise fall back to mt7610u one
*/
- err = firmware_request_nowarn(fw, MT7610E_FIRMWARE, dev->mt76.dev);
+ err = request_firmware_nowarn(fw, MT7610E_FIRMWARE, dev->mt76.dev);
if (err) {
dev_info(dev->mt76.dev, "%s not found, switching to %s",
MT7610E_FIRMWARE, MT7610U_FIRMWARE);
diff --git a/drivers/net/wireless/realtek/rtw89/fw.c b/drivers/net/wireless/realtek/rtw89/fw.c
index 09684cea9731..d3b103419862 100644
--- a/drivers/net/wireless/realtek/rtw89/fw.c
+++ b/drivers/net/wireless/realtek/rtw89/fw.c
@@ -1206,7 +1206,7 @@ static int rtw89_load_firmware_req(struct rtw89_dev *rtwdev,
}

if (nowarn)
- ret = firmware_request_nowarn(&req->firmware, fw_name, rtwdev->dev);
+ ret = request_firmware_nowarn(&req->firmware, fw_name, rtwdev->dev);
else
ret = request_firmware(&req->firmware, fw_name, rtwdev->dev);

diff --git a/drivers/net/wireless/silabs/wfx/fwio.c b/drivers/net/wireless/silabs/wfx/fwio.c
index 52c7f560b062..3cdc8548a619 100644
--- a/drivers/net/wireless/silabs/wfx/fwio.c
+++ b/drivers/net/wireless/silabs/wfx/fwio.c
@@ -105,7 +105,7 @@ static int get_firmware(struct wfx_dev *wdev, u32 keyset_chip,

snprintf(filename, sizeof(filename), "%s_%02X.sec",
wdev->pdata.file_fw, keyset_chip);
- ret = firmware_request_nowarn(fw, filename, wdev->dev);
+ ret = request_firmware_nowarn(fw, filename, wdev->dev);
if (ret) {
dev_info(wdev->dev, "can't load %s, falling back to %s.sec\n",
filename, wdev->pdata.file_fw);
diff --git a/drivers/usb/host/xhci-pci-renesas.c b/drivers/usb/host/xhci-pci-renesas.c
index 93f8b355bc70..1335e92b76fa 100644
--- a/drivers/usb/host/xhci-pci-renesas.c
+++ b/drivers/usb/host/xhci-pci-renesas.c
@@ -603,7 +603,7 @@ int renesas_xhci_check_request_fw(struct pci_dev *pdev,
return has_rom ? 0 : err;

pci_dev_get(pdev);
- err = firmware_request_nowarn(&fw, fw_name, &pdev->dev);
+ err = request_firmware_nowarn(&fw, fw_name, &pdev->dev);
pci_dev_put(pdev);
if (err) {
if (has_rom) {
diff --git a/include/linux/firmware.h b/include/linux/firmware.h
index 0311858b46ce..5523c9fdc92d 100644
--- a/include/linux/firmware.h
+++ b/include/linux/firmware.h
@@ -97,7 +97,7 @@ static inline bool firmware_request_builtin(struct firmware *fw,
#if IS_REACHABLE(CONFIG_FW_LOADER)
int request_firmware(const struct firmware **fw, const char *name,
struct device *device);
-int firmware_request_nowarn(const struct firmware **fw, const char *name,
+int request_firmware_nowarn(const struct firmware **fw, const char *name,
struct device *device);
int firmware_request_platform(const struct firmware **fw, const char *name,
struct device *device);
@@ -122,7 +122,7 @@ static inline int request_firmware(const struct firmware **fw,
return -EINVAL;
}

-static inline int firmware_request_nowarn(const struct firmware **fw,
+static inline int request_firmware_nowarn(const struct firmware **fw,
const char *name,
struct device *device)
{
diff --git a/sound/pci/hda/cs35l41_hda.c b/sound/pci/hda/cs35l41_hda.c
index d3fa6e136744..a93dcae244ac 100644
--- a/sound/pci/hda/cs35l41_hda.c
+++ b/sound/pci/hda/cs35l41_hda.c
@@ -164,7 +164,7 @@ static int cs35l41_request_firmware_file(struct cs35l41_hda *cs35l41,
s++;
}

- ret = firmware_request_nowarn(firmware, *filename, cs35l41->dev);
+ ret = request_firmware_nowarn(firmware, *filename, cs35l41->dev);
if (ret != 0) {
dev_dbg(cs35l41->dev, "Failed to request '%s'\n", *filename);
kfree(*filename);
diff --git a/sound/pci/hda/cs35l56_hda.c b/sound/pci/hda/cs35l56_hda.c
index b61e1de8c4bf..0390fd6b6eaa 100644
--- a/sound/pci/hda/cs35l56_hda.c
+++ b/sound/pci/hda/cs35l56_hda.c
@@ -422,7 +422,7 @@ static int cs35l56_hda_request_firmware_file(struct cs35l56_hda *cs35l56,
s++;
}

- ret = firmware_request_nowarn(firmware, *filename, cs35l56->base.dev);
+ ret = request_firmware_nowarn(firmware, *filename, cs35l56->base.dev);
if (ret) {
dev_dbg(cs35l56->base.dev, "Failed to request '%s'\n", *filename);
kfree(*filename);
diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
index c01e31175015..072e3e3606e7 100644
--- a/sound/soc/codecs/wm_adsp.c
+++ b/sound/soc/codecs/wm_adsp.c
@@ -772,7 +772,7 @@ static int wm_adsp_request_firmware_file(struct wm_adsp *dsp,
s++;
}

- ret = firmware_request_nowarn(firmware, *filename, cs_dsp->dev);
+ ret = request_firmware_nowarn(firmware, *filename, cs_dsp->dev);
if (ret != 0) {
adsp_dbg(dsp, "Failed to request '%s'\n", *filename);
kfree(*filename);
diff --git a/sound/soc/sof/fw-file-profile.c b/sound/soc/sof/fw-file-profile.c
index 138a1ca2c4a8..819fc846d498 100644
--- a/sound/soc/sof/fw-file-profile.c
+++ b/sound/soc/sof/fw-file-profile.c
@@ -26,7 +26,7 @@ static int sof_test_firmware_file(struct device *dev,
if (!fw_filename)
return -ENOMEM;

- ret = firmware_request_nowarn(&fw, fw_filename, dev);
+ ret = request_firmware_nowarn(&fw, fw_filename, dev);
if (ret < 0) {
dev_dbg(dev, "Failed to open firmware file: %s\n", fw_filename);
kfree(fw_filename);
@@ -78,7 +78,7 @@ static int sof_test_topology_file(struct device *dev,
if (!tplg_filename)
return -ENOMEM;

- ret = firmware_request_nowarn(&fw, tplg_filename, dev);
+ ret = request_firmware_nowarn(&fw, tplg_filename, dev);
if (!ret)
release_firmware(fw);
else
--
2.43.0.254.ga26002b62827


2024-02-22 18:02:49

by Mukesh Ojha

[permalink] [raw]
Subject: [PATCH vRFC 8/8] firmware: FW_OPT_UEVENT for all request_firmware family functions

Currently, FW_OPT_UEVENT is used in almost every request_firmware
family function apart from request_firmware_nowait(), however,
request_firmware_nowait() uses much lower level api which does
not assume FW_OPT_UEVENT to be a default flag.

Apply FW_OPT_UEVENT as default flag.

Signed-off-by: Mukesh Ojha <[email protected]>
---
drivers/base/firmware_loader/main.c | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index 91a45767a3a7..4650fef9b713 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -985,7 +985,8 @@ _request_firmware(const struct firmware **firmware_p, const char *name,

/* Need to pin this module until return */
__module_get(THIS_MODULE);
- ret = __request_firmware(firmware_p, name, device, NULL, 0, 0, opt_flags);
+ ret = __request_firmware(firmware_p, name, device, NULL, 0, 0,
+ FW_OPT_UEVENT | opt_flags);
module_put(THIS_MODULE);
return ret;
}
@@ -1013,8 +1014,7 @@ int
request_firmware(const struct firmware **firmware_p, const char *name,
struct device *device)
{
- return _request_firmware(firmware_p, name, device, NULL, 0, 0,
- FW_OPT_UEVENT);
+ return _request_firmware(firmware_p, name, device, NULL, 0, 0, 0);
}
EXPORT_SYMBOL(request_firmware);

@@ -1034,8 +1034,7 @@ EXPORT_SYMBOL(request_firmware);
int request_firmware_nowarn(const struct firmware **firmware, const char *name,
struct device *device)
{
- return _request_firmware(firmware, name, device, NULL, 0, 0,
- FW_OPT_UEVENT | FW_OPT_NO_WARN);
+ return _request_firmware(firmware, name, device, NULL, 0, 0, FW_OPT_NO_WARN);
}
EXPORT_SYMBOL_GPL(request_firmware_nowarn);

@@ -1054,8 +1053,7 @@ int request_firmware_direct(const struct firmware **firmware_p,
const char *name, struct device *device)
{
return _request_firmware(firmware_p, name, device, NULL, 0, 0,
- FW_OPT_UEVENT | FW_OPT_NO_WARN |
- FW_OPT_NOFALLBACK_SYSFS);
+ FW_OPT_NO_WARN | FW_OPT_NOFALLBACK_SYSFS);
}
EXPORT_SYMBOL_GPL(request_firmware_direct);

@@ -1073,7 +1071,7 @@ int request_firmware_platform(const struct firmware **firmware,
const char *name, struct device *device)
{
return _request_firmware(firmware, name, device, NULL, 0, 0,
- FW_OPT_UEVENT | FW_OPT_FALLBACK_PLATFORM);
+ FW_OPT_FALLBACK_PLATFORM);
}
EXPORT_SYMBOL_GPL(request_firmware_platform);

@@ -1124,7 +1122,7 @@ request_firmware_into_buf(const struct firmware **firmware_p, const char *name,
return -EOPNOTSUPP;

return _request_firmware(firmware_p, name, device, buf, size, 0,
- FW_OPT_UEVENT | FW_OPT_NOCACHE);
+ FW_OPT_NOCACHE);
}
EXPORT_SYMBOL(request_firmware_into_buf);

@@ -1149,8 +1147,7 @@ request_partial_firmware_into_buf(const struct firmware **firmware_p,
return -EOPNOTSUPP;

return _request_firmware(firmware_p, name, device, buf, size, offset,
- FW_OPT_UEVENT | FW_OPT_NOCACHE |
- FW_OPT_PARTIAL);
+ FW_OPT_NOCACHE | FW_OPT_PARTIAL);
}
EXPORT_SYMBOL(request_partial_firmware_into_buf);

--
2.43.0.254.ga26002b62827


2024-02-22 18:03:01

by Mukesh Ojha

[permalink] [raw]
Subject: [PATCH vRFC 6/8] firmware: Move module template to the bottom

Normally, module template should be kept at the
bottom of the file. Let's do the same for firmware
module as well.

Signed-off-by: Mukesh Ojha <[email protected]>
---
drivers/base/firmware_loader/main.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index 642b4d5c4375..1d752965d311 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -44,10 +44,6 @@
#include "firmware.h"
#include "fallback.h"

-MODULE_AUTHOR("Manuel Estrada Sainz");
-MODULE_DESCRIPTION("Multi purpose firmware loading support");
-MODULE_LICENSE("GPL");
-
struct firmware_cache {
/* firmware_buf instance will be added into the below list */
spinlock_t lock;
@@ -1633,3 +1629,7 @@ static void __exit firmware_class_exit(void)

fs_initcall(firmware_class_init);
module_exit(firmware_class_exit);
+
+MODULE_AUTHOR("Manuel Estrada Sainz");
+MODULE_DESCRIPTION("Multi purpose firmware loading support");
+MODULE_LICENSE("GPL");
--
2.43.0.254.ga26002b62827


2024-02-22 18:03:18

by Mukesh Ojha

[permalink] [raw]
Subject: [PATCH vRFC 7/8] firmware: remove prototype of fw_cache_piggyback_on_request()

We should always try to avoid using prototype of a static
function like fw_cache_piggyback_on_request() firmware
main file. Move the relevant code in such a way that avoid
the prototype usage.

Signed-off-by: Mukesh Ojha <[email protected]>
---
drivers/base/firmware_loader/main.c | 109 ++++++++++++++--------------
1 file changed, 54 insertions(+), 55 deletions(-)

diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index 1d752965d311..91a45767a3a7 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -100,8 +100,6 @@ static inline int fw_state_wait(struct fw_priv *fw_priv)
return __fw_state_wait_common(fw_priv, MAX_SCHEDULE_TIMEOUT);
}

-static void fw_cache_piggyback_on_request(struct fw_priv *fw_priv);
-
static struct fw_priv *__allocate_fw_priv(const char *fw_name,
struct firmware_cache *fwc,
void *dbuf,
@@ -672,7 +670,61 @@ static int fw_add_devm_name(struct device *dev, const char *name)

return 0;
}
+
+static struct fw_cache_entry *alloc_fw_cache_entry(const char *name)
+{
+ struct fw_cache_entry *fce;
+
+ fce = kzalloc(sizeof(*fce), GFP_ATOMIC);
+ if (!fce)
+ goto exit;
+
+ fce->name = kstrdup_const(name, GFP_ATOMIC);
+ if (!fce->name) {
+ kfree(fce);
+ fce = NULL;
+ goto exit;
+ }
+exit:
+ return fce;
+}
+
+static int __fw_entry_found(const char *name)
+{
+ struct firmware_cache *fwc = &fw_cache;
+ struct fw_cache_entry *fce;
+
+ list_for_each_entry(fce, &fwc->fw_names, list) {
+ if (!strcmp(fce->name, name))
+ return 1;
+ }
+ return 0;
+}
+
+static void fw_cache_piggyback_on_request(struct fw_priv *fw_priv)
+{
+ const char *name = fw_priv->fw_name;
+ struct firmware_cache *fwc = fw_priv->fwc;
+ struct fw_cache_entry *fce;
+
+ spin_lock(&fwc->name_lock);
+ if (__fw_entry_found(name))
+ goto found;
+
+ fce = alloc_fw_cache_entry(name);
+ if (fce) {
+ list_add(&fce->list, &fwc->fw_names);
+ kref_get(&fw_priv->ref);
+ pr_debug("%s: fw: %s\n", __func__, name);
+ }
+found:
+ spin_unlock(&fwc->name_lock);
+}
+
#else
+static void fw_cache_piggyback_on_request(struct fw_priv *fw_priv)
+{
+}
static bool fw_cache_is_setup(struct device *dev, const char *name)
{
return false;
@@ -1285,56 +1337,6 @@ static int uncache_firmware(const char *fw_name)
return -EINVAL;
}

-static struct fw_cache_entry *alloc_fw_cache_entry(const char *name)
-{
- struct fw_cache_entry *fce;
-
- fce = kzalloc(sizeof(*fce), GFP_ATOMIC);
- if (!fce)
- goto exit;
-
- fce->name = kstrdup_const(name, GFP_ATOMIC);
- if (!fce->name) {
- kfree(fce);
- fce = NULL;
- goto exit;
- }
-exit:
- return fce;
-}
-
-static int __fw_entry_found(const char *name)
-{
- struct firmware_cache *fwc = &fw_cache;
- struct fw_cache_entry *fce;
-
- list_for_each_entry(fce, &fwc->fw_names, list) {
- if (!strcmp(fce->name, name))
- return 1;
- }
- return 0;
-}
-
-static void fw_cache_piggyback_on_request(struct fw_priv *fw_priv)
-{
- const char *name = fw_priv->fw_name;
- struct firmware_cache *fwc = fw_priv->fwc;
- struct fw_cache_entry *fce;
-
- spin_lock(&fwc->name_lock);
- if (__fw_entry_found(name))
- goto found;
-
- fce = alloc_fw_cache_entry(name);
- if (fce) {
- list_add(&fce->list, &fwc->fw_names);
- kref_get(&fw_priv->ref);
- pr_debug("%s: fw: %s\n", __func__, name);
- }
-found:
- spin_unlock(&fwc->name_lock);
-}
-
static void free_fw_cache_entry(struct fw_cache_entry *fce)
{
kfree_const(fce->name);
@@ -1563,9 +1565,6 @@ static inline void unregister_fw_pm_ops(void)
unregister_pm_notifier(&fw_cache.pm_notify);
}
#else
-static void fw_cache_piggyback_on_request(struct fw_priv *fw_priv)
-{
-}
static inline int register_fw_pm_ops(void)
{
return 0;
--
2.43.0.254.ga26002b62827


2024-02-22 18:04:35

by Mukesh Ojha

[permalink] [raw]
Subject: [PATCH vRFC 1/8] firmware_loader: Refactor request firmware lower level functions

Refactor low level api's to do code reuse.

Signed-off-by: Mukesh Ojha <[email protected]>
---
drivers/base/firmware_loader/main.c | 88 +++++++++++------------------
1 file changed, 34 insertions(+), 54 deletions(-)

diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index da8ca01d011c..645d658facae 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -739,7 +739,7 @@ int assign_fw(struct firmware *fw, struct device *device)
* or a negative error code
*/
static int
-_request_firmware_prepare(struct firmware **firmware_p, const char *name,
+__request_firmware_prepare(struct firmware **firmware_p, const char *name,
struct device *device, void *dbuf, size_t size,
size_t offset, u32 opt_flags)
{
@@ -851,9 +851,9 @@ static void fw_log_firmware_info(const struct firmware *fw, const char *name,

/* called from request_firmware() and request_firmware_work_func() */
static int
-_request_firmware(const struct firmware **firmware_p, const char *name,
- struct device *device, void *buf, size_t size,
- size_t offset, u32 opt_flags)
+__request_firmware(const struct firmware **firmware_p, const char *name,
+ struct device *device, void *buf, size_t size,
+ size_t offset, u32 opt_flags)
{
struct firmware *fw = NULL;
struct cred *kern_cred = NULL;
@@ -869,7 +869,7 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
goto out;
}

- ret = _request_firmware_prepare(&fw, name, device, buf, size,
+ ret = __request_firmware_prepare(&fw, name, device, buf, size,
offset, opt_flags);
if (ret <= 0) /* error or already assigned */
goto out;
@@ -932,6 +932,19 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
return ret;
}

+static int
+_request_firmware(const struct firmware **firmware_p, const char *name,
+ struct device *device, void *buf, size_t size,
+ size_t offset, u32 opt_flags)
+{
+ int ret;
+
+ /* Need to pin this module until return */
+ __module_get(THIS_MODULE);
+ ret = __request_firmware(firmware_p, name, device, NULL, 0, 0, opt_flags);
+ module_put(THIS_MODULE);
+ return ret;
+}
/**
* request_firmware() - send firmware request and wait for it
* @firmware_p: pointer to firmware image
@@ -956,14 +969,8 @@ int
request_firmware(const struct firmware **firmware_p, const char *name,
struct device *device)
{
- int ret;
-
- /* Need to pin this module until return */
- __module_get(THIS_MODULE);
- ret = _request_firmware(firmware_p, name, device, NULL, 0, 0,
- FW_OPT_UEVENT);
- module_put(THIS_MODULE);
- return ret;
+ return _request_firmware(firmware_p, name, device, NULL, 0, 0,
+ FW_OPT_UEVENT);
}
EXPORT_SYMBOL(request_firmware);

@@ -983,14 +990,8 @@ EXPORT_SYMBOL(request_firmware);
int firmware_request_nowarn(const struct firmware **firmware, const char *name,
struct device *device)
{
- int ret;
-
- /* Need to pin this module until return */
- __module_get(THIS_MODULE);
- ret = _request_firmware(firmware, name, device, NULL, 0, 0,
- FW_OPT_UEVENT | FW_OPT_NO_WARN);
- module_put(THIS_MODULE);
- return ret;
+ return _request_firmware(firmware, name, device, NULL, 0, 0,
+ FW_OPT_UEVENT | FW_OPT_NO_WARN);
}
EXPORT_SYMBOL_GPL(firmware_request_nowarn);

@@ -1008,14 +1009,9 @@ EXPORT_SYMBOL_GPL(firmware_request_nowarn);
int request_firmware_direct(const struct firmware **firmware_p,
const char *name, struct device *device)
{
- int ret;
-
- __module_get(THIS_MODULE);
- ret = _request_firmware(firmware_p, name, device, NULL, 0, 0,
- FW_OPT_UEVENT | FW_OPT_NO_WARN |
- FW_OPT_NOFALLBACK_SYSFS);
- module_put(THIS_MODULE);
- return ret;
+ return _request_firmware(firmware_p, name, device, NULL, 0, 0,
+ FW_OPT_UEVENT | FW_OPT_NO_WARN |
+ FW_OPT_NOFALLBACK_SYSFS);
}
EXPORT_SYMBOL_GPL(request_firmware_direct);

@@ -1032,14 +1028,8 @@ EXPORT_SYMBOL_GPL(request_firmware_direct);
int firmware_request_platform(const struct firmware **firmware,
const char *name, struct device *device)
{
- int ret;
-
- /* Need to pin this module until return */
- __module_get(THIS_MODULE);
- ret = _request_firmware(firmware, name, device, NULL, 0, 0,
- FW_OPT_UEVENT | FW_OPT_FALLBACK_PLATFORM);
- module_put(THIS_MODULE);
- return ret;
+ return _request_firmware(firmware, name, device, NULL, 0, 0,
+ FW_OPT_UEVENT | FW_OPT_FALLBACK_PLATFORM);
}
EXPORT_SYMBOL_GPL(firmware_request_platform);

@@ -1086,16 +1076,11 @@ int
request_firmware_into_buf(const struct firmware **firmware_p, const char *name,
struct device *device, void *buf, size_t size)
{
- int ret;
-
if (fw_cache_is_setup(device, name))
return -EOPNOTSUPP;

- __module_get(THIS_MODULE);
- ret = _request_firmware(firmware_p, name, device, buf, size, 0,
- FW_OPT_UEVENT | FW_OPT_NOCACHE);
- module_put(THIS_MODULE);
- return ret;
+ return _request_firmware(firmware_p, name, device, buf, size, 0,
+ FW_OPT_UEVENT | FW_OPT_NOCACHE);
}
EXPORT_SYMBOL(request_firmware_into_buf);

@@ -1116,17 +1101,12 @@ request_partial_firmware_into_buf(const struct firmware **firmware_p,
const char *name, struct device *device,
void *buf, size_t size, size_t offset)
{
- int ret;
-
if (fw_cache_is_setup(device, name))
return -EOPNOTSUPP;

- __module_get(THIS_MODULE);
- ret = _request_firmware(firmware_p, name, device, buf, size, offset,
- FW_OPT_UEVENT | FW_OPT_NOCACHE |
- FW_OPT_PARTIAL);
- module_put(THIS_MODULE);
- return ret;
+ return _request_firmware(firmware_p, name, device, buf, size, offset,
+ FW_OPT_UEVENT | FW_OPT_NOCACHE |
+ FW_OPT_PARTIAL);
}
EXPORT_SYMBOL(request_partial_firmware_into_buf);

@@ -1162,8 +1142,8 @@ static void request_firmware_work_func(struct work_struct *work)

fw_work = container_of(work, struct firmware_work, work);

- _request_firmware(&fw, fw_work->name, fw_work->device, NULL, 0, 0,
- fw_work->opt_flags);
+ __request_firmware(&fw, fw_work->name, fw_work->device, NULL, 0, 0,
+ fw_work->opt_flags);
fw_work->cont(fw, fw_work->context);
put_device(fw_work->device); /* taken in request_firmware_nowait() */

--
2.43.0.254.ga26002b62827


2024-02-22 18:04:48

by Mukesh Ojha

[permalink] [raw]
Subject: [PATCH vRFC 3/8] treewide: rename firmware_request_platform()

Rename firmware_request_platform() to request_firmware_platform()
to be more concrete and align with the name of other request
firmware family functions.

Signed-off-by: Mukesh Ojha <[email protected]>
---
Documentation/driver-api/firmware/fallback-mechanisms.rst | 4 ++--
Documentation/driver-api/firmware/lookup-order.rst | 2 +-
Documentation/driver-api/firmware/request_firmware.rst | 4 ++--
drivers/base/firmware_loader/main.c | 6 +++---
drivers/input/touchscreen/chipone_icn8505.c | 2 +-
drivers/input/touchscreen/silead.c | 2 +-
include/linux/firmware.h | 4 ++--
lib/test_firmware.c | 2 +-
8 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/Documentation/driver-api/firmware/fallback-mechanisms.rst b/Documentation/driver-api/firmware/fallback-mechanisms.rst
index 5f04c3bcdf0c..f888fceb65ba 100644
--- a/Documentation/driver-api/firmware/fallback-mechanisms.rst
+++ b/Documentation/driver-api/firmware/fallback-mechanisms.rst
@@ -212,7 +212,7 @@ of firmware for some of the system's integrated peripheral devices and
the peripheral's Linux device-driver needs to access this firmware.

Device drivers which need such firmware can use the
-firmware_request_platform() function for this, note that this is a
+request_firmware_platform() function for this, note that this is a
separate fallback mechanism from the other fallback mechanisms and
this does not use the sysfs interface.

@@ -245,7 +245,7 @@ To register this array with the efi-embedded-fw code, a driver needs to:

4. Add "select EFI_EMBEDDED_FIRMWARE if EFI_STUB" to its Kconfig entry.

-The firmware_request_platform() function will always first try to load firmware
+The request_firmware_platform() function will always first try to load firmware
with the specified name directly from the disk, so the EFI embedded-fw can
always be overridden by placing a file under /lib/firmware.

diff --git a/Documentation/driver-api/firmware/lookup-order.rst b/Documentation/driver-api/firmware/lookup-order.rst
index 6064672a782e..c72bc8efb734 100644
--- a/Documentation/driver-api/firmware/lookup-order.rst
+++ b/Documentation/driver-api/firmware/lookup-order.rst
@@ -13,7 +13,7 @@ a driver issues a firmware API call.
* The ''Direct filesystem lookup'' is performed next, if found we
return it immediately
* The ''Platform firmware fallback'' is performed next, but only when
- firmware_request_platform() is used, if found we return it immediately
+ request_firmware_platform() is used, if found we return it immediately
* If no firmware has been found and the fallback mechanism was enabled
the sysfs interface is created. After this either a kobject uevent
is issued or the custom firmware loading is relied upon for firmware
diff --git a/Documentation/driver-api/firmware/request_firmware.rst b/Documentation/driver-api/firmware/request_firmware.rst
index 0201334bc308..3d6df9922c4b 100644
--- a/Documentation/driver-api/firmware/request_firmware.rst
+++ b/Documentation/driver-api/firmware/request_firmware.rst
@@ -25,10 +25,10 @@ request_firmware_nowarn
.. kernel-doc:: drivers/base/firmware_loader/main.c
:functions: request_firmware_nowarn

-firmware_request_platform
+request_firmware_platform
-------------------------
.. kernel-doc:: drivers/base/firmware_loader/main.c
- :functions: firmware_request_platform
+ :functions: request_firmware_platform

request_firmware_direct
-----------------------
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index cb681f4069b8..9411ad7968cb 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -1016,7 +1016,7 @@ int request_firmware_direct(const struct firmware **firmware_p,
EXPORT_SYMBOL_GPL(request_firmware_direct);

/**
- * firmware_request_platform() - request firmware with platform-fw fallback
+ * request_firmware_platform() - request firmware with platform-fw fallback
* @firmware: pointer to firmware image
* @name: name of firmware file
* @device: device for which firmware is being loaded
@@ -1025,13 +1025,13 @@ EXPORT_SYMBOL_GPL(request_firmware_direct);
* direct filesystem lookup fails, it will fallback to looking for a copy of the
* requested firmware embedded in the platform's main (e.g. UEFI) firmware.
**/
-int firmware_request_platform(const struct firmware **firmware,
+int request_firmware_platform(const struct firmware **firmware,
const char *name, struct device *device)
{
return _request_firmware(firmware, name, device, NULL, 0, 0,
FW_OPT_UEVENT | FW_OPT_FALLBACK_PLATFORM);
}
-EXPORT_SYMBOL_GPL(firmware_request_platform);
+EXPORT_SYMBOL_GPL(request_firmware_platform);

/**
* firmware_request_cache() - cache firmware for suspend so resume can use it
diff --git a/drivers/input/touchscreen/chipone_icn8505.c b/drivers/input/touchscreen/chipone_icn8505.c
index b56954830b33..be1d7530a968 100644
--- a/drivers/input/touchscreen/chipone_icn8505.c
+++ b/drivers/input/touchscreen/chipone_icn8505.c
@@ -288,7 +288,7 @@ static int icn8505_upload_fw(struct icn8505_data *icn8505)
* we may need it at resume. Having loaded it once will make the
* firmware class code cache it at suspend/resume.
*/
- error = firmware_request_platform(&fw, icn8505->firmware_name, dev);
+ error = request_firmware_platform(&fw, icn8505->firmware_name, dev);
if (error) {
dev_err(dev, "Firmware request error %d\n", error);
return error;
diff --git a/drivers/input/touchscreen/silead.c b/drivers/input/touchscreen/silead.c
index 52477e450b02..e3fb27eeca40 100644
--- a/drivers/input/touchscreen/silead.c
+++ b/drivers/input/touchscreen/silead.c
@@ -428,7 +428,7 @@ static int silead_ts_load_fw(struct i2c_client *client)
*/
error = request_firmware_nowarn(&fw, data->fw_name, dev);
if (error) {
- error = firmware_request_platform(&fw, data->fw_name, dev);
+ error = request_firmware_platform(&fw, data->fw_name, dev);
if (error) {
dev_err(dev, "Firmware request error %d\n", error);
return error;
diff --git a/include/linux/firmware.h b/include/linux/firmware.h
index 5523c9fdc92d..93e6a37352a8 100644
--- a/include/linux/firmware.h
+++ b/include/linux/firmware.h
@@ -99,7 +99,7 @@ int request_firmware(const struct firmware **fw, const char *name,
struct device *device);
int request_firmware_nowarn(const struct firmware **fw, const char *name,
struct device *device);
-int firmware_request_platform(const struct firmware **fw, const char *name,
+int request_firmware_platform(const struct firmware **fw, const char *name,
struct device *device);
int request_firmware_nowait(
struct module *module, bool uevent,
@@ -129,7 +129,7 @@ static inline int request_firmware_nowarn(const struct firmware **fw,
return -EINVAL;
}

-static inline int firmware_request_platform(const struct firmware **fw,
+static inline int request_firmware_platform(const struct firmware **fw,
const char *name,
struct device *device)
{
diff --git a/lib/test_firmware.c b/lib/test_firmware.c
index 9cfdcd6d21db..4aeb25bea3b4 100644
--- a/lib/test_firmware.c
+++ b/lib/test_firmware.c
@@ -730,7 +730,7 @@ static ssize_t trigger_request_platform_store(struct device *dev,
efi_embedded_fw_checked = true;

pr_info("loading '%s'\n", name);
- rc = firmware_request_platform(&firmware, name, dev);
+ rc = request_firmware_platform(&firmware, name, dev);
if (rc) {
pr_info("load of '%s' failed: %d\n", name, rc);
goto out;
--
2.43.0.254.ga26002b62827


2024-02-22 18:04:54

by Mukesh Ojha

[permalink] [raw]
Subject: [PATCH vRFC 5/8] firmware: Convert minor inline function to macro

Convert to_fw_priv() inline function to macro.
No functional change.

Signed-off-by: Mukesh Ojha <[email protected]>
---
drivers/base/firmware_loader/main.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index 2823dcd7153a..642b4d5c4375 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -80,13 +80,9 @@ struct fw_name_devm {
const char *name;
};

-static inline struct fw_priv *to_fw_priv(struct kref *ref)
-{
- return container_of(ref, struct fw_priv, ref);
-}
-
#define FW_LOADER_NO_CACHE 0
#define FW_LOADER_START_CACHE 1
+#define to_fw_priv(d) container_of(d, struct fw_priv, d)

/* fw_lock could be moved to 'struct fw_sysfs' but since it is just
* guarding for corner cases a global lock should be OK */
--
2.43.0.254.ga26002b62827


2024-02-22 18:15:47

by Mukesh Ojha

[permalink] [raw]
Subject: [PATCH vRFC 4/8] treewide: rename firmware_request_cache()

Rename firmware_request_cache() to request_firmware_cache()
to be more concrete and align with the name of other request
firmware family functions.

Signed-off-by: Mukesh Ojha <[email protected]>
---
Documentation/driver-api/firmware/request_firmware.rst | 6 +++---
drivers/base/firmware_loader/main.c | 6 +++---
drivers/net/wireless/mediatek/mt7601u/mcu.c | 2 +-
include/linux/firmware.h | 2 +-
4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/Documentation/driver-api/firmware/request_firmware.rst b/Documentation/driver-api/firmware/request_firmware.rst
index 3d6df9922c4b..6c5b91705bb3 100644
--- a/Documentation/driver-api/firmware/request_firmware.rst
+++ b/Documentation/driver-api/firmware/request_firmware.rst
@@ -60,13 +60,13 @@ Special optimizations on reboot
Some devices have an optimization in place to enable the firmware to be
retained during system reboot. When such optimizations are used the driver
author must ensure the firmware is still available on resume from suspend,
-this can be done with firmware_request_cache() instead of requesting for the
+this can be done with request_firmware_cache() instead of requesting for the
firmware to be loaded.

-firmware_request_cache()
+request_firmware_cache()
------------------------
.. kernel-doc:: drivers/base/firmware_loader/main.c
- :functions: firmware_request_cache
+ :functions: request_firmware_cache

request firmware API expected driver use
========================================
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index 9411ad7968cb..2823dcd7153a 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -1034,7 +1034,7 @@ int request_firmware_platform(const struct firmware **firmware,
EXPORT_SYMBOL_GPL(request_firmware_platform);

/**
- * firmware_request_cache() - cache firmware for suspend so resume can use it
+ * request_firmware_cache() - cache firmware for suspend so resume can use it
* @name: name of firmware file
* @device: device for which firmware should be cached for
*
@@ -1045,7 +1045,7 @@ EXPORT_SYMBOL_GPL(request_firmware_platform);
* situations. This helper is not compatible with drivers which use
* request_firmware_into_buf() or request_firmware_nowait() with no uevent set.
**/
-int firmware_request_cache(struct device *device, const char *name)
+int request_firmware_cache(struct device *device, const char *name)
{
int ret;

@@ -1055,7 +1055,7 @@ int firmware_request_cache(struct device *device, const char *name)

return ret;
}
-EXPORT_SYMBOL_GPL(firmware_request_cache);
+EXPORT_SYMBOL_GPL(request_firmware_cache);

/**
* request_firmware_into_buf() - load firmware into a previously allocated buffer
diff --git a/drivers/net/wireless/mediatek/mt7601u/mcu.c b/drivers/net/wireless/mediatek/mt7601u/mcu.c
index 1b5cc271a9e1..cee36dc1e9eb 100644
--- a/drivers/net/wireless/mediatek/mt7601u/mcu.c
+++ b/drivers/net/wireless/mediatek/mt7601u/mcu.c
@@ -414,7 +414,7 @@ static int mt7601u_load_firmware(struct mt7601u_dev *dev)
MT_USB_DMA_CFG_TX_BULK_EN));

if (firmware_running(dev))
- return firmware_request_cache(dev->dev, MT7601U_FIRMWARE);
+ return request_firmware_cache(dev->dev, MT7601U_FIRMWARE);

ret = request_firmware(&fw, MT7601U_FIRMWARE, dev->dev);
if (ret)
diff --git a/include/linux/firmware.h b/include/linux/firmware.h
index 93e6a37352a8..4449837ea0c6 100644
--- a/include/linux/firmware.h
+++ b/include/linux/firmware.h
@@ -196,6 +196,6 @@ static inline void firmware_upload_unregister(struct fw_upload *fw_upload)

#endif

-int firmware_request_cache(struct device *device, const char *name);
+int request_firmware_cache(struct device *device, const char *name);

#endif
--
2.43.0.254.ga26002b62827


2024-02-23 06:21:41

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH vRFC 3/8] treewide: rename firmware_request_platform()

On Thu, Feb 22, 2024 at 11:30:28PM +0530, Mukesh Ojha wrote:
> Rename firmware_request_platform() to request_firmware_platform()
> to be more concrete and align with the name of other request
> firmware family functions.

Sorry, but no, it should be "noun_verb" for public functions.

Yes, we mess this up a lot, but keeping the namespace this way works out
better for global symbols, so "firmware_*" is best please.

thanks,

greg k-h

2024-02-23 06:22:30

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH vRFC 5/8] firmware: Convert minor inline function to macro

On Thu, Feb 22, 2024 at 11:30:30PM +0530, Mukesh Ojha wrote:
> Convert to_fw_priv() inline function to macro.

Why?

> No functional change.

You lost good error messages if you pass in the wrong pointer :(

There is a good reason to convert this type of function to a macro, but
you aren't using that here, so I'm not going to tell you why it would be
ok :)

thanks,

greg k-h

2024-02-23 15:16:12

by Luis Chamberlain

[permalink] [raw]
Subject: Re: [PATCH vRFC 3/8] treewide: rename firmware_request_platform()

On Fri, Feb 23, 2024 at 07:21:31AM +0100, Greg KH wrote:
> On Thu, Feb 22, 2024 at 11:30:28PM +0530, Mukesh Ojha wrote:
> > Rename firmware_request_platform() to request_firmware_platform()
> > to be more concrete and align with the name of other request
> > firmware family functions.
>
> Sorry, but no, it should be "noun_verb" for public functions.

News to me, do we have this documented somewhere?

> Yes, we mess this up a lot, but keeping the namespace this way works out
> better for global symbols, so "firmware_*" is best please.

We should certainly stick to *one* pattern, for the better, and it
occurs to me we could further review this with a coccinelle python
script for public functions, checking the first two elements of a public
function for noun and verb.

Luis

2024-02-23 15:33:50

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH vRFC 3/8] treewide: rename firmware_request_platform()

On Fri, Feb 23, 2024 at 07:15:45AM -0800, Luis Chamberlain wrote:
> On Fri, Feb 23, 2024 at 07:21:31AM +0100, Greg KH wrote:
> > On Thu, Feb 22, 2024 at 11:30:28PM +0530, Mukesh Ojha wrote:
> > > Rename firmware_request_platform() to request_firmware_platform()
> > > to be more concrete and align with the name of other request
> > > firmware family functions.
> >
> > Sorry, but no, it should be "noun_verb" for public functions.
>
> News to me, do we have this documented somewhere?

Not really, but searching makes it nicer.

And yes, I violated this in the past in places, and have regretted it...

> > Yes, we mess this up a lot, but keeping the namespace this way works out
> > better for global symbols, so "firmware_*" is best please.
>
> We should certainly stick to *one* pattern, for the better, and it
> occurs to me we could further review this with a coccinelle python
> script for public functions, checking the first two elements of a public
> function for noun and verb.

Changing the existing function names for no real reason isn't probably a
good idea, nor worth it. The firmware_* function prefix is good, let's
keep it please.

If you really wanted to be picky, we should make them part of a module
namespace too :)

thanks,

greg k-h

2024-02-23 19:43:06

by Luis Chamberlain

[permalink] [raw]
Subject: Re: [PATCH vRFC 3/8] treewide: rename firmware_request_platform()

On Fri, Feb 23, 2024 at 04:33:40PM +0100, Greg KH wrote:
> On Fri, Feb 23, 2024 at 07:15:45AM -0800, Luis Chamberlain wrote:
> > On Fri, Feb 23, 2024 at 07:21:31AM +0100, Greg KH wrote:
> > > On Thu, Feb 22, 2024 at 11:30:28PM +0530, Mukesh Ojha wrote:
> > > > Rename firmware_request_platform() to request_firmware_platform()
> > > > to be more concrete and align with the name of other request
> > > > firmware family functions.
> > >
> > > Sorry, but no, it should be "noun_verb" for public functions.
> >
> > News to me, do we have this documented somewhere?
>
> Not really, but searching makes it nicer.
>
> And yes, I violated this in the past in places, and have regretted it...

Care to share a few examples of regret?

> > > Yes, we mess this up a lot, but keeping the namespace this way works out
> > > better for global symbols, so "firmware_*" is best please.
> >
> > We should certainly stick to *one* pattern, for the better, and it
> > occurs to me we could further review this with a coccinelle python
> > script for public functions, checking the first two elements of a public
> > function for noun and verb.
>
> Changing the existing function names for no real reason isn't probably a
> good idea, nor worth it. The firmware_* function prefix is good, let's
> keep it please.
>
> If you really wanted to be picky, we should make them part of a module
> namespace too :)

Sticking to *any* convention is good, so longa as we decide on one, it
used to be hard to have tidy rules to do this sort of stuff but with
coccinelle we should be able to grow these rules and ensure we stick
to it for the good for new stuff.

Luis

2024-02-24 05:36:34

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH vRFC 3/8] treewide: rename firmware_request_platform()

On Fri, Feb 23, 2024 at 11:42:35AM -0800, Luis Chamberlain wrote:
> On Fri, Feb 23, 2024 at 04:33:40PM +0100, Greg KH wrote:
> > On Fri, Feb 23, 2024 at 07:15:45AM -0800, Luis Chamberlain wrote:
> > > On Fri, Feb 23, 2024 at 07:21:31AM +0100, Greg KH wrote:
> > > > On Thu, Feb 22, 2024 at 11:30:28PM +0530, Mukesh Ojha wrote:
> > > > > Rename firmware_request_platform() to request_firmware_platform()
> > > > > to be more concrete and align with the name of other request
> > > > > firmware family functions.
> > > >
> > > > Sorry, but no, it should be "noun_verb" for public functions.
> > >
> > > News to me, do we have this documented somewhere?
> >
> > Not really, but searching makes it nicer.
> >
> > And yes, I violated this in the past in places, and have regretted it...
>
> Care to share a few examples of regret?

get_device()
put_device()
kill_device()

vs. a saner:
kobject_get()
kobject_put()
kobject_del()

Learn from the mistakes of my youth please :)

thanks,

greg k-h

2024-02-26 10:55:51

by Mukesh Ojha

[permalink] [raw]
Subject: Re: [PATCH vRFC 3/8] treewide: rename firmware_request_platform()



On 2/24/2024 11:06 AM, Greg KH wrote:
> On Fri, Feb 23, 2024 at 11:42:35AM -0800, Luis Chamberlain wrote:
>> On Fri, Feb 23, 2024 at 04:33:40PM +0100, Greg KH wrote:
>>> On Fri, Feb 23, 2024 at 07:15:45AM -0800, Luis Chamberlain wrote:
>>>> On Fri, Feb 23, 2024 at 07:21:31AM +0100, Greg KH wrote:
>>>>> On Thu, Feb 22, 2024 at 11:30:28PM +0530, Mukesh Ojha wrote:
>>>>>> Rename firmware_request_platform() to request_firmware_platform()
>>>>>> to be more concrete and align with the name of other request
>>>>>> firmware family functions.
>>>>>
>>>>> Sorry, but no, it should be "noun_verb" for public functions.
>>>>
>>>> News to me, do we have this documented somewhere?
>>>
>>> Not really, but searching makes it nicer.
>>>
>>> And yes, I violated this in the past in places, and have regretted it...
>>
>> Care to share a few examples of regret?
>
> get_device()
> put_device()
> kill_device()
>
> vs. a saner:
> kobject_get()
> kobject_put()
> kobject_del()
>
> Learn from the mistakes of my youth please :)

Thanks for the history.,
In that case, should we fix this verb_noun cases ?

request_firmware()
request_firmware_into_buf()
request_firmware_nowarn()
request_firmware_direct()
request_firmware_cache()
request_partial_firmware_into_buf()
release_firmware()

-Mukesh

>
> thanks,
>
> greg k-h

2024-02-26 13:10:08

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH vRFC 3/8] treewide: rename firmware_request_platform()

On Mon, Feb 26, 2024 at 04:22:09PM +0530, Mukesh Ojha wrote:
>
>
> On 2/24/2024 11:06 AM, Greg KH wrote:
> > On Fri, Feb 23, 2024 at 11:42:35AM -0800, Luis Chamberlain wrote:
> > > On Fri, Feb 23, 2024 at 04:33:40PM +0100, Greg KH wrote:
> > > > On Fri, Feb 23, 2024 at 07:15:45AM -0800, Luis Chamberlain wrote:
> > > > > On Fri, Feb 23, 2024 at 07:21:31AM +0100, Greg KH wrote:
> > > > > > On Thu, Feb 22, 2024 at 11:30:28PM +0530, Mukesh Ojha wrote:
> > > > > > > Rename firmware_request_platform() to request_firmware_platform()
> > > > > > > to be more concrete and align with the name of other request
> > > > > > > firmware family functions.
> > > > > >
> > > > > > Sorry, but no, it should be "noun_verb" for public functions.
> > > > >
> > > > > News to me, do we have this documented somewhere?
> > > >
> > > > Not really, but searching makes it nicer.
> > > >
> > > > And yes, I violated this in the past in places, and have regretted it...
> > >
> > > Care to share a few examples of regret?
> >
> > get_device()
> > put_device()
> > kill_device()
> >
> > vs. a saner:
> > kobject_get()
> > kobject_put()
> > kobject_del()
> >
> > Learn from the mistakes of my youth please :)
>
> Thanks for the history.,
> In that case, should we fix this verb_noun cases ?
>
> request_firmware()
> request_firmware_into_buf()
> request_firmware_nowarn()
> request_firmware_direct()
> request_firmware_cache()
> request_partial_firmware_into_buf()
> release_firmware()

That would provide consistency, right?

thanks,

greg k-h

2024-02-26 13:23:14

by Mukesh Ojha

[permalink] [raw]
Subject: Re: [PATCH vRFC 3/8] treewide: rename firmware_request_platform()



On 2/26/2024 6:39 PM, Greg KH wrote:
> On Mon, Feb 26, 2024 at 04:22:09PM +0530, Mukesh Ojha wrote:
>>
>>
>> On 2/24/2024 11:06 AM, Greg KH wrote:
>>> On Fri, Feb 23, 2024 at 11:42:35AM -0800, Luis Chamberlain wrote:
>>>> On Fri, Feb 23, 2024 at 04:33:40PM +0100, Greg KH wrote:
>>>>> On Fri, Feb 23, 2024 at 07:15:45AM -0800, Luis Chamberlain wrote:
>>>>>> On Fri, Feb 23, 2024 at 07:21:31AM +0100, Greg KH wrote:
>>>>>>> On Thu, Feb 22, 2024 at 11:30:28PM +0530, Mukesh Ojha wrote:
>>>>>>>> Rename firmware_request_platform() to request_firmware_platform()
>>>>>>>> to be more concrete and align with the name of other request
>>>>>>>> firmware family functions.
>>>>>>>
>>>>>>> Sorry, but no, it should be "noun_verb" for public functions.
>>>>>>
>>>>>> News to me, do we have this documented somewhere?
>>>>>
>>>>> Not really, but searching makes it nicer.
>>>>>
>>>>> And yes, I violated this in the past in places, and have regretted it...
>>>>
>>>> Care to share a few examples of regret?
>>>
>>> get_device()
>>> put_device()
>>> kill_device()
>>>
>>> vs. a saner:
>>> kobject_get()
>>> kobject_put()
>>> kobject_del()
>>>
>>> Learn from the mistakes of my youth please :)
>>
>> Thanks for the history.,
>> In that case, should we fix this verb_noun cases ?
>>
>> request_firmware()
>> request_firmware_into_buf()
>> request_firmware_nowarn()
>> request_firmware_direct()
>> request_firmware_cache()
>> request_partial_firmware_into_buf()
>> release_firmware()
>
> That would provide consistency, right?

Yes, Below names look better..

firmware_request()
firmware_request_into_buf()
firmware_request_nowarn()
firmware_request_direct()
firmware_request_cache()
firmware_request_partial_into_buf()
firmware_release()

@Luis/Others, Can we do this change ?

-Mukesh
>
> thanks,
>
> greg k-h

2024-02-26 18:06:48

by Luis Chamberlain

[permalink] [raw]
Subject: Re: [PATCH vRFC 3/8] treewide: rename firmware_request_platform()

On Mon, Feb 26, 2024 at 06:52:49PM +0530, Mukesh Ojha wrote:
>
>
> On 2/26/2024 6:39 PM, Greg KH wrote:
> > On Mon, Feb 26, 2024 at 04:22:09PM +0530, Mukesh Ojha wrote:
> > >
> > >
> > > On 2/24/2024 11:06 AM, Greg KH wrote:
> > > > On Fri, Feb 23, 2024 at 11:42:35AM -0800, Luis Chamberlain wrote:
> > > > > On Fri, Feb 23, 2024 at 04:33:40PM +0100, Greg KH wrote:
> > > > > > On Fri, Feb 23, 2024 at 07:15:45AM -0800, Luis Chamberlain wrote:
> > > > > > > On Fri, Feb 23, 2024 at 07:21:31AM +0100, Greg KH wrote:
> > > > > > > > On Thu, Feb 22, 2024 at 11:30:28PM +0530, Mukesh Ojha wrote:
> > > > > > > > > Rename firmware_request_platform() to request_firmware_platform()
> > > > > > > > > to be more concrete and align with the name of other request
> > > > > > > > > firmware family functions.
> > > > > > > >
> > > > > > > > Sorry, but no, it should be "noun_verb" for public functions.
> > > > > > >
> > > > > > > News to me, do we have this documented somewhere?
> > > > > >
> > > > > > Not really, but searching makes it nicer.
> > > > > >
> > > > > > And yes, I violated this in the past in places, and have regretted it...
> > > > >
> > > > > Care to share a few examples of regret?
> > > >
> > > > get_device()
> > > > put_device()
> > > > kill_device()
> > > >
> > > > vs. a saner:
> > > > kobject_get()
> > > > kobject_put()
> > > > kobject_del()
> > > >
> > > > Learn from the mistakes of my youth please :)
> > >
> > > Thanks for the history.,
> > > In that case, should we fix this verb_noun cases ?
> > >
> > > request_firmware()
> > > request_firmware_into_buf()
> > > request_firmware_nowarn()
> > > request_firmware_direct()
> > > request_firmware_cache()
> > > request_partial_firmware_into_buf()
> > > release_firmware()
> >
> > That would provide consistency, right?
>
> Yes, Below names look better..
>
> firmware_request()
> firmware_request_into_buf()
> firmware_request_nowarn()
> firmware_request_direct()
> firmware_request_cache()
> firmware_request_partial_into_buf()
> firmware_release()
>
> @Luis/Others, Can we do this change ?

Go for it. I just also think we might as well document from the learnt
lessons, and our preference, instead of making this just one developer's
personal preference because the moon made them feel a different way than
two years ago. From my part it is best we *strive* to stick to one
convention, whatever it is. As for the *why* to document this, I suspect
it allows easier namespace grep'ing for symbols related to one thing or
another, as to why it shoudl go first, I'll let Greg chime in.

Long term I see value in having anything we decide to stick to, to make it
easier for debugging heuristics.

Luis