Re: [PATCH] brcmfmac: expose firmware config files through modinfo

From: Hans de Goede <hdegoede@redhat.com>
To: matthias.bgg@kernel.org, Jakub Kicinski <kuba@kernel.org>,
	Kalle Valo <kvalo@codeaurora.org>,
	"David S . Miller" <davem@davemloft.net>
Cc: "Pali Rohár" <pali@kernel.org>,
	"Guenter Roeck" <linux@roeck-us.net>,
	"Chi-Hsien Lin" <chi-hsien.lin@cypress.com>,
	"Franky Lin" <franky.lin@broadcom.com>,
	"Chung-Hsien Hsu" <stanley.hsu@cypress.com>,
	"Jean-Philippe Brucker" <jean-philippe@linaro.org>,
	"Double Lo" <double.lo@cypress.com>,
	"Frank Kao" <frank.kao@cypress.com>,
	linux-wireless@vger.kernel.org,
	brcm80211-dev-list.pdl@broadcom.com,
	"Arend van Spriel" <arend.vanspriel@broadcom.com>,
	"Gustavo A . R . Silva" <gustavo@embeddedor.com>,
	netdev@vger.kernel.org, "Rafał Miłecki" <rafal@milecki.pl>,
	"Hante Meuleman" <hante.meuleman@broadcom.com>,
	"Wright Feng" <wright.feng@cypress.com>,
	"Matthias Brugger" <mbrugger@suse.com>,
	"Saravanan Shanmugham" <saravanan.shanmugham@cypress.com>,
	brcm80211-dev-list@cypress.com, linux-kernel@vger.kernel.org,
	"Ulf Hansson" <ulf.hansson@linaro.org>,
	"Soeren Moch" <smoch@web.de>
Subject: Re: [PATCH] brcmfmac: expose firmware config files through modinfo
Date: Wed, 1 Jul 2020 17:38:25 +0200	[thread overview]
Message-ID: <338e3cff-dfa0-c588-cf53-a160d75af2ee@redhat.com> (raw)
In-Reply-To: <20200701153123.25602-1-matthias.bgg@kernel.org>

Hi,

On 7/1/20 5:31 PM, matthias.bgg@kernel.org wrote:
> From: Matthias Brugger <mbrugger@suse.com>
> 
> Apart from a firmware binary the chip needs a config file used by the
> FW. Add the config files to modinfo so that they can be read by
> userspace.

The configfile firmware filename is dynamically generated, just adding the list
of all currently shipped ones is not really helpful and this is going to get
out of sync with what we actually have in linux-firmware.

I must honestly say that I'm not a fan of this, I guess you are trying to
get some tool which builds a minimal image, such as an initrd generator
to add these files to the image ?

I do not immediately have a better idea, but IMHO the solution
this patch proposes is not a good one, so nack from me for this change.

Regards,

Hans

> 
> Signed-off-by: Matthias Brugger <mbrugger@suse.com>
> 
> ---
> 
>   .../wireless/broadcom/brcm80211/brcmfmac/sdio.c  | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> index 310d8075f5d7..ba18df6d8d94 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> @@ -624,6 +624,22 @@ BRCMF_FW_DEF(4359, "brcmfmac4359-sdio");
>   BRCMF_FW_DEF(4373, "brcmfmac4373-sdio");
>   BRCMF_FW_DEF(43012, "brcmfmac43012-sdio");
>   
> +/* firmware config files */
> +MODULE_FIRMWARE(BRCMF_FW_DEFAULT_PATH "brcm/brcmfmac4330-sdio.Prowise-PT301.txt");
> +MODULE_FIRMWARE(BRCMF_FW_DEFAULT_PATH "brcm/brcmfmac43340-sdio.meegopad-t08.txt");
> +MODULE_FIRMWARE(BRCMF_FW_DEFAULT_PATH "brcm/brcmfmac43340-sdio.pov-tab-p1006w-data.txt");
> +MODULE_FIRMWARE(BRCMF_FW_DEFAULT_PATH "brcm/brcmfmac43362-sdio.cubietech,cubietruck.txt");
> +MODULE_FIRMWARE(BRCMF_FW_DEFAULT_PATH "brcm/brcmfmac43430a0-sdio.jumper-ezpad-mini3.txt");
> +MODULE_FIRMWARE(BRCMF_FW_DEFAULT_PATH "brcm/brcmfmac43430a0-sdio.ONDA-V80 PLUS.txt");
> +MODULE_FIRMWARE(BRCMF_FW_DEFAULT_PATH "brcm/brcmfmac43430-sdio.AP6212.txt");
> +MODULE_FIRMWARE(BRCMF_FW_DEFAULT_PATH "brcm/brcmfmac43430-sdio.Hampoo-D2D3_Vi8A1.txt");
> +MODULE_FIRMWARE(BRCMF_FW_DEFAULT_PATH "brcm/brcmfmac43430-sdio.MUR1DX.txt");
> +MODULE_FIRMWARE(BRCMF_FW_DEFAULT_PATH "brcm/brcmfmac43430-sdio.raspberrypi,3-model-b.txt");
> +MODULE_FIRMWARE(BRCMF_FW_DEFAULT_PATH "brcm/brcmfmac43455-sdio.MINIX-NEO Z83-4.txt");
> +MODULE_FIRMWARE(BRCMF_FW_DEFAULT_PATH "brcm/brcmfmac43455-sdio.raspberrypi,3-model-b-plus.txt");
> +MODULE_FIRMWARE(BRCMF_FW_DEFAULT_PATH "brcm/brcmfmac43455-sdio.raspberrypi,4-model-b.txt");
> +MODULE_FIRMWARE(BRCMF_FW_DEFAULT_PATH "brcm/brcmfmac4356-pcie.gpd-win-pocket.txt");
> +
>   static const struct brcmf_firmware_mapping brcmf_sdio_fwnames[] = {
>   	BRCMF_FW_ENTRY(BRCM_CC_43143_CHIP_ID, 0xFFFFFFFF, 43143),
>   	BRCMF_FW_ENTRY(BRCM_CC_43241_CHIP_ID, 0x0000001F, 43241B0),
> 