Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <e622b8bd-5490-5d8c-6209-c1ace0d1acf3@gmail.com>
Date:   Wed, 19 Oct 2022 13:58:38 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.13.1
Subject: Re: [PATCH 3/7] brcmfmac: add support for vendor-specific firmware
 api
Content-Language: en-US
To:     Kalle Valo <kvalo@kernel.org>
Cc:     arend.vanspriel@broadcom.com, linux-wireless@vger.kernel.org
References: <20220613091915.18884-1-arend.vanspriel@broadcom.com>
 <CAJ65rDxy3ERmDAV1BYH_BCUM3_GnzotWpp7yYO6OtA_CZVgbzw@mail.gmail.com>
 <87sfmlo9yi.fsf@kernel.org> <4e5c9cc1-a00e-c3de-3186-6d4cf5694339@gmail.com>
From:   Arend Van Spriel <aspriel@gmail.com>
In-Reply-To: <4e5c9cc1-a00e-c3de-3186-6d4cf5694339@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 7/29/2022 11:32 AM, Arend Van Spriel wrote:
> On 7/28/2022 11:36 AM, Kalle Valo wrote:
>> aspriel@gmail.com writes:
>>
>>> The driver is being used by multiple vendors who develop the firmware
>>> api independently. So far the firmware api as used by the driver has
>>> not diverged (yet). This change adds framework for supporting multiple
>>> firmware apis. The vendor-specific support code has to provide a number
>>> of callback operations. Right now it is only attach and detach callbacks
>>> so no real functionality as the api is still common. This code only
>>> adds WCC variant anyway, which is selected for all devices right now.
>>>
>>> Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
>>> Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
>>> Reviewed-by: Franky Lin <franky.lin@broadcom.com>
>>> Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
>>
>> [...]
>>
>>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/Kconfig
>>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/Kconfig
>>> @@ -8,6 +8,22 @@ config BRCMFMAC
>>>         interface support. If you choose to build a module, it'll be 
>>> called
>>>         brcmfmac.ko.
>>>
>>> +config BRCMFMAC_VENDOR_MODULES
>>> +    bool "Use vendor-specific modules"
>>> +    depends on BRCMFMAC = m
>>> +    help
>>> +      This option will build separate modules for the vendor-specific
>>> +      firmware support. If not selected the vendor-specific support
>>> +      will be build in brcmfmac.ko.
>>> +
>>> +config BRCMFMAC_VENDOR_WCC
>>> +    bool "Broadcom WCC"
>>> +    default y
>>> +    depends on BRCMFMAC
>>> +        help
>>> +          This option will allow the driver to communicate with devices
>>> +          shipped by Broadcom WCC division.
>>> +
>>
>> I'm not really a fan of these Kconfig options, I would rather have them
>> always enabled. Why do we need these options, what would be the use case
>> when user disables these?
> 
> I assume with "always enabled" you mean "drop these options". Obviously 
> I would prefer to keep them. The default will result in a single module 
> with all vendor support built-in, but this allows people to trim down 
> their configuration based on what they have. So the choices are:
> 
> 1) single module with all vendor support built-in
> 2) single module with partial vendor support built-in (as needed)
> 
>     allows users to select vendor for their specific device not carrying
>     stuff they don't need. If they have a Cypress/Infineon device they
>     only need support for that.
> 
> 3) separate vendor support modules loaded as needed during device probe
> 
>     build all (or some) vendor support modules and they are loaded into
>     memory only when they are needed for the device detected.

Hi Kalle,

I wanted to give this series another try, but the conversation above 
never came to any conclusion/consensus. Maybe good to finish this before 
resending?

Regards,
Arend