Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230628113934.824668-1-kiran.k@intel.com> <2a43ecd8-faf5-11f7-1a01-c114aa16354b@molgen.mpg.de>
 <PH0PR11MB758555E584EC6C8691133565F524A@PH0PR11MB7585.namprd11.prod.outlook.com>
In-Reply-To: <PH0PR11MB758555E584EC6C8691133565F524A@PH0PR11MB7585.namprd11.prod.outlook.com>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Wed, 28 Jun 2023 12:40:43 -0700
Message-ID: <CABBYNZJv=PHS5uiD7r1Wzgt4XC04dvjGpnAAH5gK-o15Y958LQ@mail.gmail.com>
Subject: Re: [PATCH v1] Bluetooth: btintel: Add support for GalePeak
To:     "K, Kiran" <kiran.k@intel.com>
Cc:     Paul Menzel <pmenzel@molgen.mpg.de>,
        "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>,
        "Srivatsa, Ravishankar" <ravishankar.srivatsa@intel.com>,
        "Tumkur Narayan, Chethan" <chethan.tumkur.narayan@intel.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Kiran,

On Wed, Jun 28, 2023 at 4:57=E2=80=AFAM K, Kiran <kiran.k@intel.com> wrote:
>
> Hi Paul,
>
> Thanks for the comments.
>
> >
> > Dear Kiran,
> >
> >
> > Thank you for your patch.
> >
> > Am 28.06.23 um 13:39 schrieb Kiran K:
> > > Hardware variant for GalePeak core (CNVi) is added.
> >
> > The code names are officially spelled with a space, I believe: Gale Pea=
k.
> >
> > Maybe also mention 0x1c.
>
> I will fix it in V2 version of patch.
> >
> > > Signed-off-by: Kiran K <kiran.k@intel.com>
> > > ---
> > >   drivers/bluetooth/btintel.c | 3 +++
> > >   1 file changed, 3 insertions(+)
> > >
> > > diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.=
c
> > > index dd1e48808ee2..f4d096639080 100644
> > > --- a/drivers/bluetooth/btintel.c
> > > +++ b/drivers/bluetooth/btintel.c
> > > @@ -480,6 +480,7 @@ static int btintel_version_info_tlv(struct hci_de=
v
> > *hdev,
> > >     case 0x18:      /* Slr */
> > >     case 0x19:      /* Slr-F */
> > >     case 0x1b:      /* Mgr */
> > > +   case 0x1c:      /* GaP */
> >
> > Why not add the full name in the comment? Maybe: Gale Peak (GaP).
> Ack.

I think it is about time to introduce proper definitions, or use an
enum, for these values. It would be also great to have btmon decoding
these values so you can include a HCI trace in the patch description.

> > >     case 0x1b:
> > > +   case 0x1c:
> > >             /* Display version information of TLV type */
> > >             btintel_version_info_tlv(hdev, &ver_tlv);
> > >
> >
> >
> > Kind regards,
> >
> > Paul
>
> Regards,
> Kiran


--=20
Luiz Augusto von Dentz