Re: [PATCH v4] staging: qlge: emit debug and dump at same level

[Joe Perches <joe@perches.com>]

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.