[PATCH ath-next] wifi: ath9k: simplify eeprom format strings

[PATCH ath-next] wifi: ath9k: simplify eeprom format strings

[Rosen Penev]

devm is already used here so might as well simplify the whole function
with devm_kasprintf.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/net/wireless/ath/ath9k/ath9k.h                |  2 +-
 drivers/net/wireless/ath/ath9k/ath9k_pci_owl_loader.c | 11 +----------
 drivers/net/wireless/ath/ath9k/init.c                 |  7 +++----
 drivers/net/wireless/ath/ath9k/rng.c                  |  4 ++--
 4 files changed, 7 insertions(+), 17 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 6e38aa7351e3..60aac2c50409 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -1076,7 +1076,7 @@ struct ath_softc {
 #ifdef CONFIG_ATH9K_HWRNG
 	struct hwrng rng_ops;
 	u32 rng_last;
-	char rng_name[sizeof("ath9k_65535")];
+	const char *rng_name;
 #endif
 };
 
diff --git a/drivers/net/wireless/ath/ath9k/ath9k_pci_owl_loader.c b/drivers/net/wireless/ath/ath9k/ath9k_pci_owl_loader.c
index fe1013a3a588..c4f8d1f98369 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k_pci_owl_loader.c
+++ b/drivers/net/wireless/ath/ath9k/ath9k_pci_owl_loader.c
@@ -140,19 +140,10 @@ static void owl_fw_cb(const struct firmware *fw, void *context)
 static const char *owl_get_eeprom_name(struct pci_dev *pdev)
 {
 	struct device *dev = &pdev->dev;
-	char *eeprom_name;
 
 	dev_dbg(dev, "using auto-generated eeprom filename\n");
 
-	eeprom_name = devm_kzalloc(dev, EEPROM_FILENAME_LEN, GFP_KERNEL);
-	if (!eeprom_name)
-		return NULL;
-
-	/* this should match the pattern used in ath9k/init.c */
-	scnprintf(eeprom_name, EEPROM_FILENAME_LEN, "ath9k-eeprom-pci-%s.bin",
-		  dev_name(dev));
-
-	return eeprom_name;
+	return devm_kasprintf(dev, GFP_KERNEL, "ath9k-eeprom-pci-%s.bin", dev_name(dev));
 }
 
 static void owl_nvmem_work(struct work_struct *work)
diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index ee951493e993..04903b5c611f 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -625,7 +625,7 @@ static int ath9k_of_init(struct ath_softc *sc)
 	struct ath_hw *ah = sc->sc_ah;
 	struct ath_common *common = ath9k_hw_common(ah);
 	enum ath_bus_type bus_type = common->bus_ops->ath_bus_type;
-	char eeprom_name[100];
+	const char *eeprom_name;
 	int ret;
 
 	if (!of_device_is_available(np))
@@ -635,9 +635,8 @@ static int ath9k_of_init(struct ath_softc *sc)
 
 	if (of_property_read_bool(np, "qca,no-eeprom")) {
 		/* ath9k-eeprom-<bus>-<id>.bin */
-		scnprintf(eeprom_name, sizeof(eeprom_name),
-			  "ath9k-eeprom-%s-%s.bin",
-			  ath_bus_type_to_string(bus_type), dev_name(ah->dev));
+		eeprom_name = devm_kasprintf(ah->dev, GFP_KERNEL, "ath9k-eeprom-%s-%s.bin",
+					     ath_bus_type_to_string(bus_type), dev_name(ah->dev));
 
 		ret = ath9k_eeprom_request(sc, eeprom_name);
 		if (ret)
diff --git a/drivers/net/wireless/ath/ath9k/rng.c b/drivers/net/wireless/ath/ath9k/rng.c
index e1def77591c6..7d549d18f605 100644
--- a/drivers/net/wireless/ath/ath9k/rng.c
+++ b/drivers/net/wireless/ath/ath9k/rng.c
@@ -103,8 +103,8 @@ void ath9k_rng_start(struct ath_softc *sc)
 	if (!AR_SREV_9300_20_OR_LATER(ah))
 		return;
 
-	snprintf(sc->rng_name, sizeof(sc->rng_name), "ath9k_%u",
-		 (atomic_inc_return(&serial) - 1) & U16_MAX);
+	sc->rng_name = devm_kasprintf(sc->dev, GFP_KERNEL, "ath9k_%u",
+				      (atomic_inc_return(&serial) - 1) & U16_MAX);
 	sc->rng_ops.name = sc->rng_name;
 	sc->rng_ops.read = ath9k_rng_read;
 	sc->rng_ops.quality = 320;
-- 
2.50.1

Re: [PATCH ath-next] wifi: ath9k: simplify eeprom format strings

[Toke Høiland-Jørgensen]

Rosen Penev <rosenp@gmail.com> writes:

> devm is already used here so might as well simplify the whole function
> with devm_kasprintf.
>
> Signed-off-by: Rosen Penev <rosenp@gmail.com>

(sorry for not getting around to replying to this before)

> ---
>  drivers/net/wireless/ath/ath9k/ath9k.h                |  2 +-
>  drivers/net/wireless/ath/ath9k/ath9k_pci_owl_loader.c | 11 +----------
>  drivers/net/wireless/ath/ath9k/init.c                 |  7 +++----
>  drivers/net/wireless/ath/ath9k/rng.c                  |  4 ++--
>  4 files changed, 7 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
> index 6e38aa7351e3..60aac2c50409 100644
> --- a/drivers/net/wireless/ath/ath9k/ath9k.h
> +++ b/drivers/net/wireless/ath/ath9k/ath9k.h
> @@ -1076,7 +1076,7 @@ struct ath_softc {
>  #ifdef CONFIG_ATH9K_HWRNG
>  	struct hwrng rng_ops;
>  	u32 rng_last;
> -	char rng_name[sizeof("ath9k_65535")];
> +	const char *rng_name;

Changing this fixed buffer to a devm-managed pointer makes no sense: it
doesn't help with any lifetime issues, and just adds overhead.

>  #endif
>  };
>  
> diff --git a/drivers/net/wireless/ath/ath9k/ath9k_pci_owl_loader.c b/drivers/net/wireless/ath/ath9k/ath9k_pci_owl_loader.c
> index fe1013a3a588..c4f8d1f98369 100644
> --- a/drivers/net/wireless/ath/ath9k/ath9k_pci_owl_loader.c
> +++ b/drivers/net/wireless/ath/ath9k/ath9k_pci_owl_loader.c
> @@ -140,19 +140,10 @@ static void owl_fw_cb(const struct firmware *fw, void *context)
>  static const char *owl_get_eeprom_name(struct pci_dev *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> -	char *eeprom_name;
>  
>  	dev_dbg(dev, "using auto-generated eeprom filename\n");
>  
> -	eeprom_name = devm_kzalloc(dev, EEPROM_FILENAME_LEN, GFP_KERNEL);
> -	if (!eeprom_name)
> -		return NULL;
> -
> -	/* this should match the pattern used in ath9k/init.c */
> -	scnprintf(eeprom_name, EEPROM_FILENAME_LEN, "ath9k-eeprom-pci-%s.bin",
> -		  dev_name(dev));
> -
> -	return eeprom_name;
> +	return devm_kasprintf(dev, GFP_KERNEL, "ath9k-eeprom-pci-%s.bin", dev_name(dev));

This change sort of makes sense, given that devm is already used. But
really, it shouldn't be; the name is only used to pass it to
request_firmware_nowait(), so the devm management is totally
superfluous. Better to change it to an on-stack buffer like the other
call instead (maybe just by getting rid of the owl_get_eeprom_name()
helper entirely, and moving the scnprintf() into the caller).

>  }
>  
>  static void owl_nvmem_work(struct work_struct *work)
> diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
> index ee951493e993..04903b5c611f 100644
> --- a/drivers/net/wireless/ath/ath9k/init.c
> +++ b/drivers/net/wireless/ath/ath9k/init.c
> @@ -625,7 +625,7 @@ static int ath9k_of_init(struct ath_softc *sc)
>  	struct ath_hw *ah = sc->sc_ah;
>  	struct ath_common *common = ath9k_hw_common(ah);
>  	enum ath_bus_type bus_type = common->bus_ops->ath_bus_type;
> -	char eeprom_name[100];
> +	const char *eeprom_name;

Changing an on-stack buffer to a devm-managed one also doesn't make any
sense.

-Toke