* [PATCH] brcmfmac: expose firmware config files through modinfo
@ 2020-07-01 15:31 matthias.bgg
2020-07-01 15:38 ` Hans de Goede
2020-07-14 9:37 ` Kalle Valo
0 siblings, 2 replies; 7+ messages in thread
From: matthias.bgg @ 2020-07-01 15:31 UTC (permalink / raw)
To: Jakub Kicinski, Kalle Valo, David S . Miller, hdegoede
Cc: Pali Rohár, Guenter Roeck, Chi-Hsien Lin, Franky Lin,
Chung-Hsien Hsu, Jean-Philippe Brucker, Double Lo, Frank Kao,
linux-wireless, brcm80211-dev-list.pdl, Arend van Spriel,
Gustavo A . R . Silva, netdev, Rafał Miłecki,
Hante Meuleman, Wright Feng, Matthias Brugger,
Saravanan Shanmugham, brcm80211-dev-list, linux-kernel,
Ulf Hansson, Soeren Moch
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.
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),
--
2.27.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] brcmfmac: expose firmware config files through modinfo
2020-07-01 15:31 [PATCH] brcmfmac: expose firmware config files through modinfo matthias.bgg
@ 2020-07-01 15:38 ` Hans de Goede
2020-07-01 15:46 ` Matthias Brugger
2020-07-14 9:37 ` Kalle Valo
1 sibling, 1 reply; 7+ messages in thread
From: Hans de Goede @ 2020-07-01 15:38 UTC (permalink / raw)
To: matthias.bgg, Jakub Kicinski, Kalle Valo, David S . Miller
Cc: Pali Rohár, Guenter Roeck, Chi-Hsien Lin, Franky Lin,
Chung-Hsien Hsu, Jean-Philippe Brucker, Double Lo, Frank Kao,
linux-wireless, brcm80211-dev-list.pdl, Arend van Spriel,
Gustavo A . R . Silva, netdev, Rafał Miłecki,
Hante Meuleman, Wright Feng, Matthias Brugger,
Saravanan Shanmugham, brcm80211-dev-list, linux-kernel,
Ulf Hansson, Soeren Moch
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),
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] brcmfmac: expose firmware config files through modinfo
2020-07-01 15:38 ` Hans de Goede
@ 2020-07-01 15:46 ` Matthias Brugger
2020-07-02 18:00 ` Hans de Goede
0 siblings, 1 reply; 7+ messages in thread
From: Matthias Brugger @ 2020-07-01 15:46 UTC (permalink / raw)
To: Hans de Goede, matthias.bgg, Jakub Kicinski, Kalle Valo,
David S . Miller
Cc: Pali Rohár, Guenter Roeck, Chi-Hsien Lin, Franky Lin,
Chung-Hsien Hsu, Jean-Philippe Brucker, Double Lo, Frank Kao,
linux-wireless, brcm80211-dev-list.pdl, Arend van Spriel,
Gustavo A . R . Silva, netdev, Rafał Miłecki,
Hante Meuleman, Wright Feng, Saravanan Shanmugham,
brcm80211-dev-list, linux-kernel, Ulf Hansson, Soeren Moch
Hi Hans,
On 01/07/2020 17:38, Hans de Goede wrote:
> 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'm aware of this, and I agree.
>
> 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 ?
>
Yes exactly.
> 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.
>
Another path we could go is add a wildcard string instead, for example:
MODULE_FIRMWARE("brcm/brcmfmac43455-sdio.*.txt");
AFAIK there is no driver in the kernel that does this. I checked with our dracut
developer and right now dracut can't cope with that. But he will try to
implement that in the future.
So my idea was to maintain that list for now and switch to the wildcard approach
once we have dracut support that.
Regards,
Matthias
> 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),
>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] brcmfmac: expose firmware config files through modinfo
2020-07-01 15:46 ` Matthias Brugger
@ 2020-07-02 18:00 ` Hans de Goede
2020-07-03 14:01 ` Matthias Brugger
0 siblings, 1 reply; 7+ messages in thread
From: Hans de Goede @ 2020-07-02 18:00 UTC (permalink / raw)
To: Matthias Brugger, matthias.bgg, Jakub Kicinski, Kalle Valo,
David S . Miller
Cc: Pali Rohár, Guenter Roeck, Chi-Hsien Lin, Franky Lin,
Chung-Hsien Hsu, Jean-Philippe Brucker, Double Lo, Frank Kao,
linux-wireless, brcm80211-dev-list.pdl, Arend van Spriel,
Gustavo A . R . Silva, netdev, Rafał Miłecki,
Hante Meuleman, Wright Feng, Saravanan Shanmugham,
brcm80211-dev-list, linux-kernel, Ulf Hansson, Soeren Moch
Hi,
On 7/1/20 5:46 PM, Matthias Brugger wrote:
> Hi Hans,
>
> On 01/07/2020 17:38, Hans de Goede wrote:
>> 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'm aware of this, and I agree.
>
>>
>> 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 ?
>>
>
> Yes exactly.
>
>> 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.
>>
>
> Another path we could go is add a wildcard string instead, for example:
> MODULE_FIRMWARE("brcm/brcmfmac43455-sdio.*.txt");
I was thinking about the same lines, but I'm afraid some user-space
utils may blow up if we introduce this, which is why I did not suggest
it in my previous email.
> AFAIK there is no driver in the kernel that does this. I checked with our dracut
> developer and right now dracut can't cope with that.
Can't cope as in tries to add "/lib/firmware/brcm/brcmfmac43455-sdio.*.txt"
and then skips it (as it does for other missing firmware files); or can't
cope as in blows-up and aborts without leaving a valid initrd behind.
If is the former, that is fine, if it is the latter that is a problem.
> But he will try to
> implement that in the future.
>
> So my idea was to maintain that list for now and switch to the wildcard approach
> once we have dracut support that.
So lets assume that the wildcard approach is ok and any initrd tools looking at
the MODULE_FIRMWARE metadata either accidentally do what we want; or fail
gracefully. Then if we temporarily add the long MODULE_FIRMWARE list now, those
which fail gracefully will start doing the right thing (except they add too
much firmware), and later on we cannot remove all the non wildcard
MODULE_FIRMWARE list entries because that will cause a regression.
Because of this I'm not a fan of temporarily fixing this like this. Using wifi
inside the initrd is very much a cornercase anyways, so I think users can
use a workaround by dropping an /etc/dracut.conf.d file adding the necessary
config file for now.
As for the long run, I was thinking that even with regular firmware files
we are adding too much firmware to host-specific initrds since we add all
the firmwares listed with MODULE_FIRMWARE, and typically only a few are
actually necessary.
We could modify the firmware_loader code under drivers/base/firmware_loader
to keep a list of all files loaded since boot; and export that somewhere
under /sys, then dracut could use that list in host-only mode and we get
a smaller initrd. One challenge with this approach though is firmware files
which are necessary for a new kernel, but not used by the running kernel ...
I'm afraid I do not have a good answer to that.
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),
>>>
>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] brcmfmac: expose firmware config files through modinfo
2020-07-02 18:00 ` Hans de Goede
@ 2020-07-03 14:01 ` Matthias Brugger
2020-07-03 14:10 ` Hans de Goede
0 siblings, 1 reply; 7+ messages in thread
From: Matthias Brugger @ 2020-07-03 14:01 UTC (permalink / raw)
To: Hans de Goede, Matthias Brugger, matthias.bgg, Jakub Kicinski,
Kalle Valo, David S . Miller
Cc: Pali Rohár, Guenter Roeck, Chi-Hsien Lin, Franky Lin,
Chung-Hsien Hsu, Jean-Philippe Brucker, Double Lo, Frank Kao,
linux-wireless, brcm80211-dev-list.pdl, Arend van Spriel,
Gustavo A . R . Silva, netdev, Rafał Miłecki,
Hante Meuleman, Wright Feng, Saravanan Shanmugham,
brcm80211-dev-list, linux-kernel, Ulf Hansson, Soeren Moch
On 02/07/2020 20:00, Hans de Goede wrote:
> Hi,
>
> On 7/1/20 5:46 PM, Matthias Brugger wrote:
>> Hi Hans,
>>
>> On 01/07/2020 17:38, Hans de Goede wrote:
>>> 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'm aware of this, and I agree.
>>
>>>
>>> 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 ?
>>>
>>
>> Yes exactly.
>>
>>> 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.
>>>
>>
>> Another path we could go is add a wildcard string instead, for example:
>> MODULE_FIRMWARE("brcm/brcmfmac43455-sdio.*.txt");
>
> I was thinking about the same lines, but I'm afraid some user-space
> utils may blow up if we introduce this, which is why I did not suggest
> it in my previous email.
>
>> AFAIK there is no driver in the kernel that does this. I checked with our dracut
>> developer and right now dracut can't cope with that.
>
> Can't cope as in tries to add "/lib/firmware/brcm/brcmfmac43455-sdio.*.txt"
> and then skips it (as it does for other missing firmware files); or can't
> cope as in blows-up and aborts without leaving a valid initrd behind.
>
> If is the former, that is fine, if it is the latter that is a problem.
>
>> But he will try to
>> implement that in the future.
>>
>> So my idea was to maintain that list for now and switch to the wildcard approach
>> once we have dracut support that.
>
> So lets assume that the wildcard approach is ok and any initrd tools looking at
> the MODULE_FIRMWARE metadata either accidentally do what we want; or fail
> gracefully. Then if we temporarily add the long MODULE_FIRMWARE list now, those
> which fail gracefully will start doing the right thing (except they add too
> much firmware), and later on we cannot remove all the non wildcard
> MODULE_FIRMWARE list entries because that will cause a regression.
>
> Because of this I'm not a fan of temporarily fixing this like this. Using wifi
> inside the initrd is very much a cornercase anyways, so I think users can
> use a workaround by dropping an /etc/dracut.conf.d file adding the necessary
> config file for now.
>
> As for the long run, I was thinking that even with regular firmware files
> we are adding too much firmware to host-specific initrds since we add all
> the firmwares listed with MODULE_FIRMWARE, and typically only a few are
> actually necessary.
>
> We could modify the firmware_loader code under drivers/base/firmware_loader
> to keep a list of all files loaded since boot; and export that somewhere
> under /sys, then dracut could use that list in host-only mode and we get
> a smaller initrd. One challenge with this approach though is firmware files
> which are necessary for a new kernel, but not used by the running kernel ...
> I'm afraid I do not have a good answer to that.
>
That would work for creating a new minimal initrd from a working image. But it
would not help in bootstrapping an image. My understanding is that for
bootstrapping an image we will need to support wildcards in MODULE_FIRMWARE()
strings.
Regards,
Matthias
> 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),
>>>>
>>>
>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] brcmfmac: expose firmware config files through modinfo
2020-07-03 14:01 ` Matthias Brugger
@ 2020-07-03 14:10 ` Hans de Goede
0 siblings, 0 replies; 7+ messages in thread
From: Hans de Goede @ 2020-07-03 14:10 UTC (permalink / raw)
To: Matthias Brugger, Matthias Brugger, matthias.bgg, Jakub Kicinski,
Kalle Valo, David S . Miller
Cc: Pali Rohár, Guenter Roeck, Chi-Hsien Lin, Franky Lin,
Chung-Hsien Hsu, Jean-Philippe Brucker, Double Lo, Frank Kao,
linux-wireless, brcm80211-dev-list.pdl, Arend van Spriel,
Gustavo A . R . Silva, netdev, Rafał Miłecki,
Hante Meuleman, Wright Feng, Saravanan Shanmugham,
brcm80211-dev-list, linux-kernel, Ulf Hansson, Soeren Moch
Hi,
On 7/3/20 4:01 PM, Matthias Brugger wrote:
>
>
> On 02/07/2020 20:00, Hans de Goede wrote:
>> Hi,
>>
>> On 7/1/20 5:46 PM, Matthias Brugger wrote:
>>> Hi Hans,
>>>
>>> On 01/07/2020 17:38, Hans de Goede wrote:
>>>> 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'm aware of this, and I agree.
>>>
>>>>
>>>> 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 ?
>>>>
>>>
>>> Yes exactly.
>>>
>>>> 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.
>>>>
>>>
>>> Another path we could go is add a wildcard string instead, for example:
>>> MODULE_FIRMWARE("brcm/brcmfmac43455-sdio.*.txt");
>>
>> I was thinking about the same lines, but I'm afraid some user-space
>> utils may blow up if we introduce this, which is why I did not suggest
>> it in my previous email.
>>
>>> AFAIK there is no driver in the kernel that does this. I checked with our dracut
>>> developer and right now dracut can't cope with that.
>>
>> Can't cope as in tries to add "/lib/firmware/brcm/brcmfmac43455-sdio.*.txt"
>> and then skips it (as it does for other missing firmware files); or can't
>> cope as in blows-up and aborts without leaving a valid initrd behind.
>>
>> If is the former, that is fine, if it is the latter that is a problem.
>>
>>> But he will try to
>>> implement that in the future.
>>>
>>> So my idea was to maintain that list for now and switch to the wildcard approach
>>> once we have dracut support that.
>>
>> So lets assume that the wildcard approach is ok and any initrd tools looking at
>> the MODULE_FIRMWARE metadata either accidentally do what we want; or fail
>> gracefully. Then if we temporarily add the long MODULE_FIRMWARE list now, those
>> which fail gracefully will start doing the right thing (except they add too
>> much firmware), and later on we cannot remove all the non wildcard
>> MODULE_FIRMWARE list entries because that will cause a regression.
>>
>> Because of this I'm not a fan of temporarily fixing this like this. Using wifi
>> inside the initrd is very much a cornercase anyways, so I think users can
>> use a workaround by dropping an /etc/dracut.conf.d file adding the necessary
>> config file for now.
>>
>> As for the long run, I was thinking that even with regular firmware files
>> we are adding too much firmware to host-specific initrds since we add all
>> the firmwares listed with MODULE_FIRMWARE, and typically only a few are
>> actually necessary.
>>
>> We could modify the firmware_loader code under drivers/base/firmware_loader
>> to keep a list of all files loaded since boot; and export that somewhere
>> under /sys, then dracut could use that list in host-only mode and we get
>> a smaller initrd. One challenge with this approach though is firmware files
>> which are necessary for a new kernel, but not used by the running kernel ...
>> I'm afraid I do not have a good answer to that.
>>
>
> That would work for creating a new minimal initrd from a working image. But it
> would not help in bootstrapping an image. My understanding is that for
> bootstrapping an image we will need to support wildcards in MODULE_FIRMWARE()
> strings.
Yes, I agree.
Regards,
Hans
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] brcmfmac: expose firmware config files through modinfo
2020-07-01 15:31 [PATCH] brcmfmac: expose firmware config files through modinfo matthias.bgg
2020-07-01 15:38 ` Hans de Goede
@ 2020-07-14 9:37 ` Kalle Valo
1 sibling, 0 replies; 7+ messages in thread
From: Kalle Valo @ 2020-07-14 9:37 UTC (permalink / raw)
To: matthias.bgg
Cc: Jakub Kicinski, David S . Miller, hdegoede, Pali Rohár,
Guenter Roeck, Chi-Hsien Lin, Franky Lin, Chung-Hsien Hsu,
Jean-Philippe Brucker, Double Lo, Frank Kao, linux-wireless,
brcm80211-dev-list.pdl, Arend van Spriel, Gustavo A . R . Silva,
netdev, Rafał Miłecki, Hante Meuleman, Wright Feng,
Matthias Brugger, Saravanan Shanmugham, brcm80211-dev-list,
linux-kernel, Ulf Hansson, Soeren Moch
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.
>
> Signed-off-by: Matthias Brugger <mbrugger@suse.com>
I agree with Hans, this does not look a good solution for the problem.
Patch set to Changes Requested.
--
https://patchwork.kernel.org/patch/11636715/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-07-14 9:37 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-01 15:31 [PATCH] brcmfmac: expose firmware config files through modinfo matthias.bgg
2020-07-01 15:38 ` Hans de Goede
2020-07-01 15:46 ` Matthias Brugger
2020-07-02 18:00 ` Hans de Goede
2020-07-03 14:01 ` Matthias Brugger
2020-07-03 14:10 ` Hans de Goede
2020-07-14 9:37 ` Kalle Valo
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).