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: <0b277149-867b-8acf-30d8-2cd68ba24c99@gmail.com>
Date:   Fri, 6 Jan 2023 17:21:24 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.6.1
Subject: Re: [PATCH v2] brcmfmac: Prefer DT board type over DMI board type
Content-Language: en-US
To:     "Ivan T. Ivanov" <iivanov@suse.de>, marcan@marcan.st
Cc:     franky.lin@broadcom.com, hante.meuleman@broadcom.com,
        rmk+kernel@armlinux.org.uk, stefan.wahren@i2se.com,
        pbrobinson@gmail.com, jforbes@fedoraproject.org, kvalo@kernel.org,
        davem@davemloft.net, devicetree@vger.kernel.org,
        edumazet@google.com, krzysztof.kozlowski+dt@linaro.org,
        kuba@kernel.org, pabeni@redhat.com, robh+dt@kernel.org,
        linux-arm-kernel@lists.infradead.org,
        linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
        brcm80211-dev-list.pdl@broadcom.com,
        SHA-cyfmac-dev-list@infineon.com, stable@vger.kernel.org
References: <20230106131905.81854-1-iivanov@suse.de>
From:   Arend Van Spriel <aspriel@gmail.com>
In-Reply-To: <20230106131905.81854-1-iivanov@suse.de>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 1/6/2023 2:19 PM, Ivan T. Ivanov wrote:
> The introduction of support for Apple board types inadvertently changed
> the precedence order, causing hybrid SMBIOS+DT platforms to look up the
> firmware using the DMI information instead of the device tree compatible
> to generate the board type. Revert back to the old behavior,
> as affected platforms use firmwares named after the DT compatible.
> 
> Fixes: 7682de8b3351 ("wifi: brcmfmac: of: Fetch Apple properties")
> 
> [1] https://bugzilla.opensuse.org/show_bug.cgi?id=1206697#c13
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Ivan T. Ivanov <iivanov@suse.de>

Looks good to me. I do have a question about the devicetree node for 
brcmfmac. The driver does a compatible check against 
"brcm,bcm4329-fmac". I actually expect all devicetree specifications to 
use this. That said I noticed the check for it in brcmf_of_probe() 
should be moved so it is the first check done.

Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Reviewed-by: Hector Martin <marcan@marcan.st>
> ---
> Changes since v1
> Rewrite commit message according feedback.
> https://lore.kernel.org/all/20230106072746.29516-1-iivanov@suse.de/
> 
>   drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
> index a83699de01ec..fdd0c9abc1a1 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
> @@ -79,7 +79,8 @@ void brcmf_of_probe(struct device *dev, enum brcmf_bus_type bus_type,
>   	/* Apple ARM64 platforms have their own idea of board type, passed in
>   	 * via the device tree. They also have an antenna SKU parameter
>   	 */
> -	if (!of_property_read_string(np, "brcm,board-type", &prop))
> +	err = of_property_read_string(np, "brcm,board-type", &prop);
> +	if (!err)
>   		settings->board_type = prop;
>   
>   	if (!of_property_read_string(np, "apple,antenna-sku", &prop))
> @@ -87,7 +88,7 @@ void brcmf_of_probe(struct device *dev, enum brcmf_bus_type bus_type,
>   
>   	/* Set board-type to the first string of the machine compatible prop */
>   	root = of_find_node_by_path("/");
> -	if (root && !settings->board_type) {
> +	if (root && err) {
>   		char *board_type;
>   		const char *tmp;
>