Re: [PATCH] Bluetooth: hci_intel: Fix multiple issues reported by checkpatch.pl

[Uri Arev]

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