Hi,

> From: Zheng, Lv
> Sent: Wednesday, February 25, 2015 4:42 PM
> 
> Hi,
> 
> > From: Rafael J. Wysocki [mailto:r...@rjwysocki.net]
> > Sent: Wednesday, February 18, 2015 2:19 PM
> > To: Scot Doyle
> > Cc: Zheng, Lv; linux-a...@vger.kernel.org; linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH] ACPI / EC: Remove non-standard log emphasis
> >
> > On Sunday, February 15, 2015 07:43:08 PM Scot Doyle wrote:
> > > Remove unusual pr_info() visual emphasis introduced in ad479e7f47ca
> > > "ACPI / EC: Introduce STARTED/STOPPED flags to replace BLOCKED flag".
> > >
> > > Signed-off-by: Scot Doyle <lkm...@scotdoyle.com>
> >
> > Applied, thanks!
> >
> > > ---
> > >  drivers/acpi/ec.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
> > > index 982b67f..857175e 100644
> > > --- a/drivers/acpi/ec.c
> > > +++ b/drivers/acpi/ec.c
> > > @@ -680,7 +680,7 @@ static void acpi_ec_start(struct acpi_ec *ec, bool 
> > > resuming)
> > >           /* Enable GPE for event processing (SCI_EVT=1) */
> > >           if (!resuming)
> > >                   acpi_ec_submit_request(ec);
> > > -         pr_info("+++++ EC started +++++\n");
> > > +         pr_info("EC started\n");
> > >   }
> > >   spin_unlock_irqrestore(&ec->lock, flags);
> > >  }
> > > @@ -712,7 +712,7 @@ static void acpi_ec_stop(struct acpi_ec *ec, bool 
> > > suspending)
> > >                   acpi_ec_complete_request(ec);
> > >           clear_bit(EC_FLAGS_STARTED, &ec->flags);
> > >           clear_bit(EC_FLAGS_STOPPED, &ec->flags);
> > > -         pr_info("+++++ EC stopped +++++\n");
> > > +         pr_info("EC stopped\n");
> > >   }
> > >   spin_unlock_irqrestore(&ec->lock, flags);
> > >  }
> > >
> 
> I was using "+++++"/"#####"/"*****" to filter different EC log entries which 
> makes debugging easier.
> And, if we changed this from pr_info into pr_debug, then we will have nothing 
> in the suspend/resume logs for the EC device.
> While there are plenty of such log entries for each of other devices.
> So I really doubt what's the meaning of such a change.
> Could it improve anything or just make debugging more difficult for 
> developers?

IMO, it's better to introduce such kind of code in the head of ec.c:

#ifdef DEBUG_FILTER
#define EC_LOG_DEV "+++++"
#define EC_LOG_CMD "*****"
#define EC_LOG_EVT "#####"
...
#else
#define EC_LOG_DEV
#define EC_LOG_CMD
#define EC_LOG_EVT
#endif

And wrap the pr_info, pr_debug using filter enabled new macros - ec_info, 
ec_debug so that we won't see the filters in the normal dmesg output and the 
filters are still available for the developers.

Thanks and best regards
-Lv

> 
> Thanks and best regards
> -Lv
> 
> 
> >
> > --
> > I speak only for myself.
> > Rafael J. Wysocki, Intel Open Source Technology Center.
N�����r��y����b�X��ǧv�^�)޺{.n�+����{����zX����ܨ}���Ơz�&j:+v�������zZ+��+zf���h���~����i���z��w���?�����&�)ߢf��^jǫy�m��@A�a���
0��h���i

Reply via email to