Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Kalle Valo <kvalo@kernel.org>
To:     Doug Anderson <dianders@chromium.org>
Cc:     Abhishek Kumar <kuabhs@chromium.org>,
        Rakesh Pillai <pillair@codeaurora.org>,
        LKML <linux-kernel@vger.kernel.org>,
        linux-wireless <linux-wireless@vger.kernel.org>,
        "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        ath10k <ath10k@lists.infradead.org>,
        netdev <netdev@vger.kernel.org>
Subject: Re: [PATCH 1/2] ath10k: search for default BDF name provided in DT
References: <20220107200417.1.Ie4dcc45b0bf365077303c596891d460d716bb4c5@changeid>
        <CAD=FV=W5fHP8K-PcoYWxYHiDWnPUVQQzOzw=REbuJSSqGeVVfg@mail.gmail.com>
Date:   Thu, 10 Mar 2022 12:05:53 +0200
In-Reply-To: <CAD=FV=W5fHP8K-PcoYWxYHiDWnPUVQQzOzw=REbuJSSqGeVVfg@mail.gmail.com>
        (Doug Anderson's message of "Fri, 7 Jan 2022 13:22:45 -0800")
Message-ID: <87sfrqqfzy.fsf@kernel.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

Doug Anderson <dianders@chromium.org> writes:

> Hi,
>
> On Fri, Jan 7, 2022 at 12:05 PM Abhishek Kumar <kuabhs@chromium.org> wrote:
>>
>> There can be cases where the board-2.bin does not contain
>> any BDF matching the chip-id+board-id+variant combination.
>> This causes the wlan probe to fail and renders wifi unusable.
>> For e.g. if the board-2.bin has default BDF as:
>> bus=snoc,qmi-board-id=67 but for some reason the board-id
>> on the wlan chip is not programmed and read 0xff as the
>> default value. In such cases there won't be any matching BDF
>> because the board-2.bin will be searched with following:
>> bus=snoc,qmi-board-id=ff

I just checked, in ath10k-firmware WCN3990/hw1.0/board-2.bin we have
that entry:

BoardNames[1]: 'bus=snoc,qmi-board-id=ff'

>> To address these scenarios, there can be an option to provide
>> fallback default BDF name in the device tree. If none of the
>> BDF names match then the board-2.bin file can be searched with
>> default BDF names assigned in the device tree.
>>
>> The default BDF name can be set as:
>> wifi@a000000 {
>>         status = "okay";
>>         qcom,ath10k-default-bdf = "bus=snoc,qmi-board-id=67";
>
> Rather than add a new device tree property, wouldn't it be good enough
> to leverage the existing variant?

I'm not thrilled either adding this to Device Tree, we should keep the
bindings as simple as possible.

> Right now I think that the board file contains:
>
> 'bus=snoc,qmi-board-id=67.bin'
> 'bus=snoc,qmi-board-id=67,qmi-chip-id=320,variant=GO_LAZOR.bin'
> 'bus=snoc,qmi-board-id=67,qmi-chip-id=320,variant=GO_POMPOM.bin'
> 'bus=snoc,qmi-board-id=67,qmi-chip-id=4320,variant=GO_LAZOR.bin'
> 'bus=snoc,qmi-board-id=67,qmi-chip-id=4320,variant=GO_POMPOM.bin'
>
> ...and, on lazor for instance, we have:
>
> qcom,ath10k-calibration-variant = "GO_LAZOR";
>
> The problem you're trying to solve is that on some early lazor
> prototype hardware we didn't have the "board-id" programmed we could
> get back 0xff from the hardware. As I understand it 0xff always means
> "unprogrammed".
>
> It feels like you could just have a special case such that if the
> hardware reports board ID of 0xff and you don't get a "match" that you
> could just treat 0xff as a wildcard. That means that you'd see the
> "variant" in the device tree and pick one of the "GO_LAZOR" entries.
>
> Anyway, I guess it's up to the people who spend more time in this file
> which they'd prefer, but that seems like it'd be simple and wouldn't
> require a bindings addition...

In ath11k we need something similar for that I have been thinking like
this:

'bus=snoc,qmi-board-id=67,qmi-chip-id=320,variant=GO_LAZOR'

'bus=snoc,qmi-board-id=67,qmi-chip-id=320'

'bus=snoc,qmi-board-id=67'

'bus=snoc'

Ie. we drop one attribute at a time and try to find a suitable board
file. Though I'm not sure if it's possible to find a board file which
works with many different hardware, but the principle would be at least
that. Would something like that work in your case?

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches