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

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

Posted by me@wantyapps.xyz 1 year, 10 months ago

From: Uri Arev <me@wantyapps.xyz>

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@wantyapps.xyz>
---
 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__);
 
 	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

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

Posted by Luiz Augusto von Dentz 1 year, 10 months ago

Hi Uri,

On Tue, Apr 2, 2024 at 11:56 AM <me@wantyapps.xyz> wrote:
>
> From: Uri Arev <me@wantyapps.xyz>
>
> 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@wantyapps.xyz>
> ---
>  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, "");

>
>         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
>


-- 
Luiz Augusto von Dentz

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

Posted by Uri Arev 1 year, 10 months ago

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@wantyapps.xyz> wrote:
> >
> > From: Uri Arev <me@wantyapps.xyz>
> >
> > 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@wantyapps.xyz>
> > ---
> >  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
> >

                 Uri Arev

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

Posted by Uri Arev 1 year, 10 months ago

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@wantyapps.xyz> wrote:
> > >
> > > From: Uri Arev <me@wantyapps.xyz>
> > >
> > > 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@wantyapps.xyz>
> > > ---
> > >  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