On Tue, Apr 02, 2024 at 07:12:23PM +0300, Uri Arev wrote:
> Hi Luiz, thanks for taking the time to review my patch.
>
> On Tue, Apr 02, 2024 at 12:06:30PM -0400, Luiz Augusto von Dentz wrote:
> > Hi Uri,
> >
> > On Tue, Apr 2, 2024 at 11:56 AM <me@xxxxxxxxxxxxx> wrote:
> > >
> > > From: Uri Arev <me@xxxxxxxxxxxxx>
> > >
> > > This fixes the following CHECKs, WARNINGs, and ERRORs reported in
> > > hci_intel.c
> > >
> > > Reported by checkpatch.pl:
> > > -----------
> > > hci_intel.c
> > > -----------
> > > WARNING: Prefer using '"%s...", __func__' to using 'intel_setup', this function's name, in a string
> > > + bt_dev_dbg(hdev, "start intel_setup");
> > >
> > > ERROR: code indent should use tabs where possible
> > > + /* Check for supported iBT hardware variants of this firmware$
> > >
> > > ERROR: code indent should use tabs where possible
> > > + * loading method.$
> > >
> > > ERROR: code indent should use tabs where possible
> > > + *$
> > >
> > > ERROR: code indent should use tabs where possible
> > > + * This check has been put in place to ensure correct forward$
> > >
> > > ERROR: code indent should use tabs where possible
> > > + * compatibility options when newer hardware variants come along$
> > >
> > > ERROR: code indent should use tabs where possible
> > > + */$
> > >
> > > CHECK: No space is necessary after a cast
> > > + duration = (unsigned long long) ktime_to_ns(delta) >> 10;
> > >
> > > CHECK: No space is necessary after a cast
> > > + duration = (unsigned long long) ktime_to_ns(delta) >> 10;
> > >
> > > WARNING: Missing a blank line after declarations
> > > + int err = PTR_ERR(intel->rx_skb);
> > > + bt_dev_err(hu->hdev, "Frame reassembly failed (%d)", err);
> > >
> > > Signed-off-by: Uri Arev <me@xxxxxxxxxxxxx>
> > > ---
> > > drivers/bluetooth/hci_intel.c | 19 ++++++++++---------
> > > 1 file changed, 10 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/bluetooth/hci_intel.c b/drivers/bluetooth/hci_intel.c
> > > index 78afb9a348e7..44731de1a2d6 100644
> > > --- a/drivers/bluetooth/hci_intel.c
> > > +++ b/drivers/bluetooth/hci_intel.c
> > > @@ -537,7 +537,7 @@ static int intel_setup(struct hci_uart *hu)
> > > int speed_change = 0;
> > > int err;
> > >
> > > - bt_dev_dbg(hdev, "start intel_setup");
> > > + bt_dev_dbg(hdev, "start %s", __func__);
> >
> > We probably don't need __func__ either since this is normally calls
> > pr_debug which already has support for printing function names if the
> > user wants to, so I just leave as bt_dev_dbg(hdev, "");
> >
> Thanks for the comment, will revise. I still don't know most of the
> debugging functions in the kernel, as I am a relatively new hacker.
>
> > >
> > > hu->hdev->set_diag = btintel_set_diag;
> > > hu->hdev->set_bdaddr = btintel_set_bdaddr;
> > > @@ -591,12 +591,12 @@ static int intel_setup(struct hci_uart *hu)
> > > return -EINVAL;
> > > }
> > >
> > > - /* Check for supported iBT hardware variants of this firmware
> > > - * loading method.
> > > - *
> > > - * This check has been put in place to ensure correct forward
> > > - * compatibility options when newer hardware variants come along.
> > > - */
> > > + /* Check for supported iBT hardware variants of this firmware
> > > + * loading method.
> > > + *
> > > + * This check has been put in place to ensure correct forward
> > > + * compatibility options when newer hardware variants come along.
> > > + */
> > > switch (ver.hw_variant) {
> > > case 0x0b: /* LnP */
> > > case 0x0c: /* WsP */
> > > @@ -777,7 +777,7 @@ static int intel_setup(struct hci_uart *hu)
> > >
> > > rettime = ktime_get();
> > > delta = ktime_sub(rettime, calltime);
> > > - duration = (unsigned long long) ktime_to_ns(delta) >> 10;
> > > + duration = (unsigned long long)ktime_to_ns(delta) >> 10;
> > >
> > > bt_dev_info(hdev, "Firmware loaded in %llu usecs", duration);
> > >
> > > @@ -822,7 +822,7 @@ static int intel_setup(struct hci_uart *hu)
> > >
> > > rettime = ktime_get();
> > > delta = ktime_sub(rettime, calltime);
> > > - duration = (unsigned long long) ktime_to_ns(delta) >> 10;
> > > + duration = (unsigned long long)ktime_to_ns(delta) >> 10;
> > >
> > > bt_dev_info(hdev, "Device booted in %llu usecs", duration);
> > >
> > > @@ -977,6 +977,7 @@ static int intel_recv(struct hci_uart *hu, const void *data, int count)
> > > ARRAY_SIZE(intel_recv_pkts));
> > > if (IS_ERR(intel->rx_skb)) {
> > > int err = PTR_ERR(intel->rx_skb);
> > > +
> > > bt_dev_err(hu->hdev, "Frame reassembly failed (%d)", err);
> > > intel->rx_skb = NULL;
> > > return err;
> > > --
> > > 2.44.0
> > >
Hey Luiz, I sent a v2 that adds the above suggestion and fixes gitlint.
Uri Arev
