LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v4] staging: qlge: emit debug and dump at same level
@ 2020-02-24  8:24 Kaaira Gupta
  2020-02-24 13:38 ` Joe Perches
  2020-03-04  6:49 ` Greg Kroah-Hartman
  0 siblings, 2 replies; 7+ messages in thread
From: Kaaira Gupta @ 2020-02-24  8:24 UTC (permalink / raw)
  To: Manish Chopra, GR-Linux-NIC-Dev, Greg Kroah-Hartman, netdev,
	devel, linux-kernel

Simplify code in ql_mpi_core_to_log() by calling print_hex_dump()
instead of existing functions so that the debug and dump are
emitted at the same KERN_<LEVEL>

Signed-off-by: Kaaira Gupta <kgupta@es.iitr.ac.in>
---

changes since v1: make code of ql_mpi_core_to_log() simpler.
changes since v2: directly call the function instead of defining a
macro.
changes since v3: Remove prefix string.

----
---
 drivers/staging/qlge/qlge_dbg.c | 23 +++--------------------
 1 file changed, 3 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/qlge/qlge_dbg.c b/drivers/staging/qlge/qlge_dbg.c
index c7af2548d119..0c5f6859e645 100644
--- a/drivers/staging/qlge/qlge_dbg.c
+++ b/drivers/staging/qlge/qlge_dbg.c
@@ -1324,27 +1324,10 @@ void ql_mpi_core_to_log(struct work_struct *work)
 {
 	struct ql_adapter *qdev =
 		container_of(work, struct ql_adapter, mpi_core_to_log.work);
-	u32 *tmp, count;
-	int i;
 
-	count = sizeof(struct ql_mpi_coredump) / sizeof(u32);
-	tmp = (u32 *)qdev->mpi_coredump;
-	netif_printk(qdev, drv, KERN_DEBUG, qdev->ndev,
-		     "Core is dumping to log file!\n");
-
-	for (i = 0; i < count; i += 8) {
-		pr_err("%.08x: %.08x %.08x %.08x %.08x %.08x "
-			"%.08x %.08x %.08x\n", i,
-			tmp[i + 0],
-			tmp[i + 1],
-			tmp[i + 2],
-			tmp[i + 3],
-			tmp[i + 4],
-			tmp[i + 5],
-			tmp[i + 6],
-			tmp[i + 7]);
-		msleep(5);
-	}
+	print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_OFFSET,
+		       32, 4, qdev->mpi_coredump,
+		       sizeof(*qdev->mpi_coredump), false);
 }
 
 #ifdef QL_REG_DUMP
-- 
2.17.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4] staging: qlge: emit debug and dump at same level
  2020-02-24  8:24 [PATCH v4] staging: qlge: emit debug and dump at same level Kaaira Gupta
@ 2020-02-24 13:38 ` Joe Perches
  2020-02-24 16:47   ` Kaaira Gupta
  2020-03-04  6:49 ` Greg Kroah-Hartman
  1 sibling, 1 reply; 7+ messages in thread
From: Joe Perches @ 2020-02-24 13:38 UTC (permalink / raw)
  To: Kaaira Gupta, Manish Chopra, GR-Linux-NIC-Dev,
	Greg Kroah-Hartman, netdev, devel, linux-kernel

On Mon, 2020-02-24 at 13:54 +0530, Kaaira Gupta wrote:
> Simplify code in ql_mpi_core_to_log() by calling print_hex_dump()
> instead of existing functions so that the debug and dump are
> emitted at the same KERN_<LEVEL>
[]
> diff --git a/drivers/staging/qlge/qlge_dbg.c b/drivers/staging/qlge/qlge_dbg.c
[]
> @@ -1324,27 +1324,10 @@ void ql_mpi_core_to_log(struct work_struct *work)
>  {
>  	struct ql_adapter *qdev =
>  		container_of(work, struct ql_adapter, mpi_core_to_log.work);
> -	u32 *tmp, count;
> -	int i;
>  
> -	count = sizeof(struct ql_mpi_coredump) / sizeof(u32);
> -	tmp = (u32 *)qdev->mpi_coredump;
> -	netif_printk(qdev, drv, KERN_DEBUG, qdev->ndev,
> -		     "Core is dumping to log file!\n");

There is no real need to delete this line.

And if you really want to, it'd be better to mention
the removal in the commit message description.

As is for this change, there is no "debug" and "dump"
as the commit message description shows, just "dump".





^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4] staging: qlge: emit debug and dump at same level
  2020-02-24 13:38 ` Joe Perches
@ 2020-02-24 16:47   ` Kaaira Gupta
  2020-02-24 16:54     ` Joe Perches
  0 siblings, 1 reply; 7+ messages in thread
From: Kaaira Gupta @ 2020-02-24 16:47 UTC (permalink / raw)
  To: Joe Perches, Manish Chopra, GR-Linux-NIC-Dev, Greg Kroah-Hartman,
	netdev, devel, linux-kernel

On Mon, Feb 24, 2020 at 05:38:09AM -0800, Joe Perches wrote:
> On Mon, 2020-02-24 at 13:54 +0530, Kaaira Gupta wrote:
> > Simplify code in ql_mpi_core_to_log() by calling print_hex_dump()
> > instead of existing functions so that the debug and dump are
> > emitted at the same KERN_<LEVEL>
> []
> > diff --git a/drivers/staging/qlge/qlge_dbg.c b/drivers/staging/qlge/qlge_dbg.c
> []
> > @@ -1324,27 +1324,10 @@ void ql_mpi_core_to_log(struct work_struct *work)
> >  {
> >  	struct ql_adapter *qdev =
> >  		container_of(work, struct ql_adapter, mpi_core_to_log.work);
> > -	u32 *tmp, count;
> > -	int i;
> >  
> > -	count = sizeof(struct ql_mpi_coredump) / sizeof(u32);
> > -	tmp = (u32 *)qdev->mpi_coredump;
> > -	netif_printk(qdev, drv, KERN_DEBUG, qdev->ndev,
> > -		     "Core is dumping to log file!\n");
> 
> There is no real need to delete this line.
> 
> And if you really want to, it'd be better to mention
> the removal in the commit message description.
> 
> As is for this change, there is no "debug" and "dump"
> as the commit message description shows, just "dump".

This patch has already been added to the tree, if I amend the commit now
using git rebase, won't it affect the upstream as the SHA-1 of the
commit and it's children will change?

> 
> 
> 
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4] staging: qlge: emit debug and dump at same level
  2020-02-24 16:47   ` Kaaira Gupta
@ 2020-02-24 16:54     ` Joe Perches
  2020-02-24 17:18       ` Kaaira Gupta
  0 siblings, 1 reply; 7+ messages in thread
From: Joe Perches @ 2020-02-24 16:54 UTC (permalink / raw)
  To: Kaaira Gupta, Manish Chopra, GR-Linux-NIC-Dev,
	Greg Kroah-Hartman, netdev, devel, linux-kernel

On Mon, 2020-02-24 at 22:17 +0530, Kaaira Gupta wrote:
> On Mon, Feb 24, 2020 at 05:38:09AM -0800, Joe Perches wrote:
> > On Mon, 2020-02-24 at 13:54 +0530, Kaaira Gupta wrote:
> > > Simplify code in ql_mpi_core_to_log() by calling print_hex_dump()
> > > instead of existing functions so that the debug and dump are
> > > emitted at the same KERN_<LEVEL>
> > []
> > > diff --git a/drivers/staging/qlge/qlge_dbg.c b/drivers/staging/qlge/qlge_dbg.c
> > []
> > > @@ -1324,27 +1324,10 @@ void ql_mpi_core_to_log(struct work_struct *work)
> > >  {
> > >  	struct ql_adapter *qdev =
> > >  		container_of(work, struct ql_adapter, mpi_core_to_log.work);
> > > -	u32 *tmp, count;
> > > -	int i;
> > >  
> > > -	count = sizeof(struct ql_mpi_coredump) / sizeof(u32);
> > > -	tmp = (u32 *)qdev->mpi_coredump;
> > > -	netif_printk(qdev, drv, KERN_DEBUG, qdev->ndev,
> > > -		     "Core is dumping to log file!\n");
> > 
> > There is no real need to delete this line.
> > 
> > And if you really want to, it'd be better to mention
> > the removal in the commit message description.
> > 
> > As is for this change, there is no "debug" and "dump"
> > as the commit message description shows, just "dump".
> 
> This patch has already been added to the tree,

What tree is that?
It's not in -next as of right now.

>  if I amend the commit now
> using git rebase, won't it affect the upstream as the SHA-1 of the
> commit and it's children will change?

You are sending patches not pull requests.

If it's really in an actual tree that people
care about, send another patch putting the
netif_printk back.



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4] staging: qlge: emit debug and dump at same level
  2020-02-24 16:54     ` Joe Perches
@ 2020-02-24 17:18       ` Kaaira Gupta
  2020-02-24 17:30         ` Joe Perches
  0 siblings, 1 reply; 7+ messages in thread
From: Kaaira Gupta @ 2020-02-24 17:18 UTC (permalink / raw)
  To: Joe Perches, Manish Chopra, GR-Linux-NIC-Dev, Greg Kroah-Hartman,
	netdev, devel, linux-kernel

On Mon, Feb 24, 2020 at 08:54:43AM -0800, Joe Perches wrote:
> On Mon, 2020-02-24 at 22:17 +0530, Kaaira Gupta wrote:
> > On Mon, Feb 24, 2020 at 05:38:09AM -0800, Joe Perches wrote:
> > > On Mon, 2020-02-24 at 13:54 +0530, Kaaira Gupta wrote:
> > > > Simplify code in ql_mpi_core_to_log() by calling print_hex_dump()
> > > > instead of existing functions so that the debug and dump are
> > > > emitted at the same KERN_<LEVEL>
> > > []
> > > > diff --git a/drivers/staging/qlge/qlge_dbg.c b/drivers/staging/qlge/qlge_dbg.c
> > > []
> > > > @@ -1324,27 +1324,10 @@ void ql_mpi_core_to_log(struct work_struct *work)
> > > >  {
> > > >  	struct ql_adapter *qdev =
> > > >  		container_of(work, struct ql_adapter, mpi_core_to_log.work);
> > > > -	u32 *tmp, count;
> > > > -	int i;
> > > >  
> > > > -	count = sizeof(struct ql_mpi_coredump) / sizeof(u32);
> > > > -	tmp = (u32 *)qdev->mpi_coredump;
> > > > -	netif_printk(qdev, drv, KERN_DEBUG, qdev->ndev,
> > > > -		     "Core is dumping to log file!\n");
> > > 
> > > There is no real need to delete this line.
> > > 
> > > And if you really want to, it'd be better to mention
> > > the removal in the commit message description.
> > > 
> > > As is for this change, there is no "debug" and "dump"
> > > as the commit message description shows, just "dump".
> > 
> > This patch has already been added to the tree,
> 
> What tree is that?
> It's not in -next as of right now.

Its in staging-next right now.
This is the link: https://lore.kernel.org/driverdev-devel/cba75ee4d88afdf118631510ad0f971e42c1a31c.camel@perches.com/

> 
> >  if I amend the commit now
> > using git rebase, won't it affect the upstream as the SHA-1 of the
> > commit and it's children will change?
> 
> You are sending patches not pull requests.
> 
> If it's really in an actual tree that people
> care about, send another patch putting the
> netif_printk back.

I'll submit a patch, but can you please explain me why this function is
still needed when we are already using print_hex_dump()?

> 
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4] staging: qlge: emit debug and dump at same level
  2020-02-24 17:18       ` Kaaira Gupta
@ 2020-02-24 17:30         ` Joe Perches
  0 siblings, 0 replies; 7+ messages in thread
From: Joe Perches @ 2020-02-24 17:30 UTC (permalink / raw)
  To: Kaaira Gupta, Manish Chopra, GR-Linux-NIC-Dev,
	Greg Kroah-Hartman, netdev, devel, linux-kernel

On Mon, 2020-02-24 at 22:48 +0530, Kaaira Gupta wrote:
> On Mon, Feb 24, 2020 at 08:54:43AM -0800, Joe Perches wrote:
> > On Mon, 2020-02-24 at 22:17 +0530, Kaaira Gupta wrote:
> > > On Mon, Feb 24, 2020 at 05:38:09AM -0800, Joe Perches wrote:
> > > > On Mon, 2020-02-24 at 13:54 +0530, Kaaira Gupta wrote:
> > > > > Simplify code in ql_mpi_core_to_log() by calling print_hex_dump()
> > > > > instead of existing functions so that the debug and dump are
> > > > > emitted at the same KERN_<LEVEL>
> > > > []
> > > > > diff --git a/drivers/staging/qlge/qlge_dbg.c b/drivers/staging/qlge/qlge_dbg.c
> > > > []
> > > > > @@ -1324,27 +1324,10 @@ void ql_mpi_core_to_log(struct work_struct *work)
> > > > >  {
> > > > >  	struct ql_adapter *qdev =
> > > > >  		container_of(work, struct ql_adapter, mpi_core_to_log.work);
> > > > > -	u32 *tmp, count;
> > > > > -	int i;
> > > > >  
> > > > > -	count = sizeof(struct ql_mpi_coredump) / sizeof(u32);
> > > > > -	tmp = (u32 *)qdev->mpi_coredump;
> > > > > -	netif_printk(qdev, drv, KERN_DEBUG, qdev->ndev,
> > > > > -		     "Core is dumping to log file!\n");
> > > > 
> > > > There is no real need to delete this line.
> > > > 
> > > > And if you really want to, it'd be better to mention
> > > > the removal in the commit message description.
> > > > 
> > > > As is for this change, there is no "debug" and "dump"
> > > > as the commit message description shows, just "dump".
> > > 
> > > This patch has already been added to the tree,
> > 
> > What tree is that?
> > It's not in -next as of right now.
> 
> Its in staging-next right now.
> This is the link: https://lore.kernel.org/driverdev-devel/cba75ee4d88afdf118631510ad0f971e42c1a31c.camel@perches.com/
> 
> > >  if I amend the commit now
> > > using git rebase, won't it affect the upstream as the SHA-1 of the
> > > commit and it's children will change?
> > 
> > You are sending patches not pull requests.
> > 
> > If it's really in an actual tree that people
> > care about, send another patch putting the
> > netif_printk back.
> 
> I'll submit a patch, but can you please explain me why this function is
> still needed when we are already using print_hex_dump()?

Your commit message did not match the code.

You are changing the code and the output without explanation.

It's fine to both change the code and the output when appropriate.
it's not fine to do so by misstating what you are doing.



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4] staging: qlge: emit debug and dump at same level
  2020-02-24  8:24 [PATCH v4] staging: qlge: emit debug and dump at same level Kaaira Gupta
  2020-02-24 13:38 ` Joe Perches
@ 2020-03-04  6:49 ` Greg Kroah-Hartman
  1 sibling, 0 replies; 7+ messages in thread
From: Greg Kroah-Hartman @ 2020-03-04  6:49 UTC (permalink / raw)
  To: Kaaira Gupta; +Cc: Manish Chopra, GR-Linux-NIC-Dev, netdev, devel, linux-kernel

On Mon, Feb 24, 2020 at 01:54:48PM +0530, Kaaira Gupta wrote:
> Simplify code in ql_mpi_core_to_log() by calling print_hex_dump()
> instead of existing functions so that the debug and dump are
> emitted at the same KERN_<LEVEL>
> 
> Signed-off-by: Kaaira Gupta <kgupta@es.iitr.ac.in>
> ---
> 
> changes since v1: make code of ql_mpi_core_to_log() simpler.
> changes since v2: directly call the function instead of defining a
> macro.
> changes since v3: Remove prefix string.
> 
> ----
> ---
>  drivers/staging/qlge/qlge_dbg.c | 23 +++--------------------
>  1 file changed, 3 insertions(+), 20 deletions(-)

Does not apply to my tree :(

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-03-04  6:49 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-24  8:24 [PATCH v4] staging: qlge: emit debug and dump at same level Kaaira Gupta
2020-02-24 13:38 ` Joe Perches
2020-02-24 16:47   ` Kaaira Gupta
2020-02-24 16:54     ` Joe Perches
2020-02-24 17:18       ` Kaaira Gupta
2020-02-24 17:30         ` Joe Perches
2020-03-04  6:49 ` Greg Kroah-Hartman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).