[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAJQfnxGz9dJCU2YwwKYZLpZ73L5YS6=8yac8K5BUfPBjeeM4rg@mail.gmail.com>
Date: Mon, 22 Apr 2024 16:25:49 +0800
From: Archie Pusaka <apusaka@...gle.com>
To: Paul Menzel <pmenzel@...gen.mpg.de>
Cc: Archie Pusaka <apusaka@...omium.org>, linux-bluetooth@...r.kernel.org,
Luiz Augusto von Dentz <luiz.dentz@...il.com>, Johan Hedberg <johan.hedberg@...il.com>,
Marcel Holtmann <marcel@...tmann.org>, chromeos-bluetooth-upstreaming@...omium.org,
Abhishek Pandit-Subedi <abhishekpandit@...gle.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Bluetooth: Populate hci_set_hw_info for Intel and Realtek
Hi Paul,
On Mon, 22 Apr 2024 at 15:32, Paul Menzel <pmenzel@...gen.mpg.de> wrote:
>
> Dear Archie,
>
>
> Thank you for your patch.
>
> Am 22.04.24 um 09:24 schrieb Archie Pusaka:
> > From: Archie Pusaka <apusaka@...omium.org>
> >
> > The hardware information surfaced via debugfs might be usable by the
> > userspace to set some configuration knobs. This patch sets the hw_info
> > for Intel and Realtek chipsets.
>
> Could you please add an example paste to the commit message?
>
Sure, I will add those.
> > Signed-off-by: Archie Pusaka <apusaka@...omium.org>
> > Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@...gle.com>
> >
> > ---
> >
> > drivers/bluetooth/btintel.c | 9 +++++++++
> > drivers/bluetooth/btrtl.c | 7 +++++++
> > 2 files changed, 16 insertions(+)
> >
> > diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> > index a19ebe47bd951..dc48352166a52 100644
> > --- a/drivers/bluetooth/btintel.c
> > +++ b/drivers/bluetooth/btintel.c
> > @@ -2956,6 +2956,11 @@ static int btintel_setup_combined(struct hci_dev *hdev)
> > err = -EINVAL;
> > }
> >
> > + hci_set_hw_info(hdev,
> > + "INTEL platform=%u variant=%u revision=%u",
> > + ver.hw_platform, ver.hw_variant,
> > + ver.hw_revision);
> > +
> > goto exit_error;
> > }
> >
> > @@ -3060,6 +3065,10 @@ static int btintel_setup_combined(struct hci_dev *hdev)
> > break;
> > }
> >
> > + hci_set_hw_info(hdev, "INTEL platform=%u variant=%u",
> > + INTEL_HW_PLATFORM(ver_tlv.cnvi_bt),
> > + INTEL_HW_VARIANT(ver_tlv.cnvi_bt));
> > +
>
> Why does it need to be added at two places?
>
Intel put the HW information into two structs, the "intel_version ver"
and "intel_version_tlv ver_tlv".
I don't know the history of the two structs, but that requires us to
have the hw info set in two places.
At most only one of them is executed though.
> > exit_error:
> > kfree_skb(skb);
> >
> > diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c
> > index cc50de69e8dc9..4f1e37b4f7802 100644
> > --- a/drivers/bluetooth/btrtl.c
> > +++ b/drivers/bluetooth/btrtl.c
> > @@ -1339,6 +1339,13 @@ int btrtl_setup_realtek(struct hci_dev *hdev)
> >
> > btrtl_set_quirks(hdev, btrtl_dev);
> >
> > + hci_set_hw_info(hdev,
> > + "RTL lmp_subver=%u hci_rev=%u hci_ver=%u hci_bus=%u",
> > + btrtl_dev->ic_info->lmp_subver,
> > + btrtl_dev->ic_info->hci_rev,
> > + btrtl_dev->ic_info->hci_ver,
> > + btrtl_dev->ic_info->hci_bus);
> > +
> > btrtl_free(btrtl_dev);
> > return ret;
> > }
>
>
> Kind regards,
>
> Paul
Thanks!
Archie
Powered by blists - more mailing lists