From: "Rakesh Pillai" <pillair@codeaurora.org>
To: "'Doug Anderson'" <dianders@chromium.org>
Cc: "'Abhishek Kumar'" <kuabhs@chromium.org>,
"'Kalle Valo'" <kvalo@codeaurora.org>,
"'LKML'" <linux-kernel@vger.kernel.org>,
"'ath10k'" <ath10k@lists.infradead.org>,
"'Brian Norris'" <briannorris@chromium.org>,
"'linux-wireless'" <linux-wireless@vger.kernel.org>,
"'David S. Miller'" <davem@davemloft.net>,
"'Jakub Kicinski'" <kuba@kernel.org>,
"'netdev'" <netdev@vger.kernel.org>
Subject: RE: [PATCH v2 1/1] ath10k: add option for chip-id based BDF selection
Date: Thu, 3 Dec 2020 17:03:02 +0530 [thread overview]
Message-ID: <004301d6c968$12ef1b10$38cd5130$@codeaurora.org> (raw)
In-Reply-To: <CAD=FV=WQPMnor3oTefDHd6JP6UmpyBo7UsOJ1Sg4Ly1otxr6hw@mail.gmail.com>
> -----Original Message-----
> From: Doug Anderson <dianders@chromium.org>
> Sent: Tuesday, December 1, 2020 12:49 AM
> To: Rakesh Pillai <pillair@codeaurora.org>
> Cc: Abhishek Kumar <kuabhs@chromium.org>; Kalle Valo
> <kvalo@codeaurora.org>; LKML <linux-kernel@vger.kernel.org>; ath10k
> <ath10k@lists.infradead.org>; Brian Norris <briannorris@chromium.org>;
> linux-wireless <linux-wireless@vger.kernel.org>; David S. Miller
> <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>; netdev
> <netdev@vger.kernel.org>
> Subject: Re: [PATCH v2 1/1] ath10k: add option for chip-id based BDF
> selection
>
> Hi,
>
> On Tue, Nov 24, 2020 at 7:44 PM Rakesh Pillai <pillair@codeaurora.org>
> wrote:
> >
> > > > I missed on reviewing this change. Also I agree with Doug that this is not
> > > the change I was looking for.
> > > >
> > > > The argument "with_variant" can be renamed to "with_extra_params".
> > > There is no need for any new argument to this function.
> > > > Case 1: with_extra_params=0, ar->id.bdf_ext[0] = 0 -> The
> default
> > > name will be used (bus=snoc,qmi_board_id=0xab)
> > > > Case 2: with_extra_params=1, ar->id.bdf_ext[0] = 0 ->
> > > bus=snoc,qmi_board_id=0xab,qmi_chip_id=0xcd
> > > > Case 3: with_extra_params=1, ar->id.bdf_ext[0] = "xyz" ->
> > > bus=snoc,qmi_board_id=0xab,qmi_chip_id=0xcd,variant=xyz
> > > >
> > > > ar->id.bdf_ext[0] depends on the DT entry for variant field.
> > >
> > > I'm confused about your suggestion. Maybe you can help clarify. Are
> > > you suggesting:
> > >
> > > a) Only two calls to ath10k_core_create_board_name()
> > >
> > > I'm pretty sure this will fail in some cases. Specifically consider
> > > the case where the device tree has a "variant" defined but the BRD
> > > file only has one entry for (board-id) and one for (board-id +
> > > chip-id) but no entry for (board-id + chip-id + variant). If you are
> > > only making two calls then I don't think you'll pick the right one.
> > >
> > > Said another way...
> > >
> > > If the device tree has a variant:
> > > 1. We should prefer a BRD entry that has board-id + chip-id + variant
> > > 2. If #1 isn't there, we should prefer a BRD entry that has board-id + chip-
> id
> > > 3. If #1 and #2 aren't there we fall back to a BRD entry that has board-id.
> > >
> > > ...without 3 calls to ath10k_core_create_board_name() we can't handle
> > > all 3 cases.
> >
> > This can be handled by two calls to ath10k_core_create_board_name
> > 1) ath10k_core_create_board_name(ar, boardname, sizeof(boardname),
> true) : As per my suggestions, this can result in two possible board names
> > a) If DT have the "variant" node, it outputs the #1 from your suggestion
> (1. We should prefer a BRD entry that has board-id + chip-id + variant)
> > b) If DT does not have the "variant" node, it outputs the #2 from your
> suggestion (2. If #1 isn't there, we should prefer a BRD entry that has board-
> id + chip-id)
> >
> > 2) ath10k_core_create_board_name(ar, boardname, sizeof(boardname),
> false) : This is the second call to this function and outputs the #3 from your
> suggestion (3. If #1 and #2 aren't there we fall back to a BRD entry that has
> board-id)
>
> What I'm trying to say is this. Imagine that:
>
> a) the device tree has the "variant" property.
>
> b) the BRD file has two entries, one for "board-id" (1) and one for
> "board-id + chip-id" (2). It doesn't have one for "board-id + chip-id
> + variant" (3).
>
> With your suggestion we'll see the "variant" property in the device
> tree. That means we'll search for (1) and (3). (3) isn't there, so
> we'll pick (1). ...but we really should have picked (2), right?
Do we expect board-2.bin to not be populated with the bdf with variant field (if its necessary ?)
Seems fine for me, if we have 2 fallback names if that is needed.
>
> -Doug
next prev parent reply other threads:[~2020-12-03 11:39 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-12 20:09 [PATCH v2 0/1] This patch address comments on patch v1 Abhishek Kumar
2020-11-12 20:09 ` [PATCH v2 1/1] ath10k: add option for chip-id based BDF selection Abhishek Kumar
2020-11-24 0:56 ` Doug Anderson
2020-11-24 9:18 ` Rakesh Pillai
2020-11-24 16:26 ` Doug Anderson
2020-11-25 3:44 ` Rakesh Pillai
2020-11-30 19:18 ` Doug Anderson
2020-12-03 11:33 ` Rakesh Pillai [this message]
2020-12-03 15:33 ` Doug Anderson
2020-12-07 18:14 ` Abhishek Kumar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='004301d6c968$12ef1b10$38cd5130$@codeaurora.org' \
--to=pillair@codeaurora.org \
--cc=ath10k@lists.infradead.org \
--cc=briannorris@chromium.org \
--cc=davem@davemloft.net \
--cc=dianders@chromium.org \
--cc=kuabhs@chromium.org \
--cc=kuba@kernel.org \
--cc=kvalo@codeaurora.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).