Linux Media Controller development
 help / color / mirror / Atom feed
From: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
To: Antti Palosaari <crope@iki.fi>
Cc: Linux Media Mailing List <linux-media@vger.kernel.org>,
	Mauro Carvalho Chehab <mchehab@infradead.org>,
	Hans Verkuil <hans.verkuil@cisco.com>,
	Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	Ramakrishnan Muthukrishnan <ramakrmu@cisco.com>,
	Takashi Iwai <tiwai@suse.de>,
	Matthias Schwarzott <zzam@gentoo.org>,
	Peter Senna Tschudin <peter.senna@gmail.com>
Subject: Re: [PATCH 3/7] [media] cx231xx: Cleanup printk at the driver
Date: Sat, 1 Nov 2014 12:08:58 -0200	[thread overview]
Message-ID: <20141101120858.47dc0bd7@recife.lan> (raw)
In-Reply-To: <5454E7A8.40707@iki.fi>

Em Sat, 01 Nov 2014 16:01:12 +0200
Antti Palosaari <crope@iki.fi> escreveu:

> On 11/01/2014 03:38 PM, Mauro Carvalho Chehab wrote:
> > There are lots of debug printks printed with pr_info. Also, the
> > printk's data are not too coherent:
> >
> > - there are duplicated driver name at the print format;
> > - function name format string differs from function to function;
> > - long strings broken into multiple lines;
> > - some printks just produce ugly reports, being almost useless
> >    as-is.
> >
> > Do a cleanup on that.
> >
> > Still, there are much to be done in order to do a better printk
> > job on this driver, but, at least it will now be a way less
> > verbose, if debug printks are disabled, and some logs might
> > actually be useful.
> 
> As you do that kind of cleanup, why don't just use a bit more time and 
> do it properly using dev_foo() logging. Basically all device drivers 
> should use dev_foo() logging, it prints module name, bus number etc. 
> automatically in a standard manner. pr_foo() is worse, which should be 
> only used for cases where pointer to device is not available (like library).

Please notice that my only goal with this series is to be able to
check if analog/digital is still working after Mattias's i2c mux
patchsets. I don't have much time right now for a more complete
cleanup.

Several of the printks are done before creating the cx231xx device
struct. See the places where cx231xx_errdev() were called before
patch 1/7.

The cx231xx probing is complex. Not sure if it is possible
to convert everything to dev_foo() and mixing pr_foo with dev_foo()
seems to be worse.

I may revisit it some other time and try to evaluate the impact of
doing such change when I have more spare time.

Regards,
Mauro

  reply	other threads:[~2014-11-01 14:09 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1414849139-29609-1-git-send-email-mchehab@osg.samsung.com>
     [not found] ` <cover.1414849031.git.mchehab@osg.samsung.com>
2014-11-01 13:38   ` [PATCH 1/7] [media] cx231xx: get rid of driver-defined printk macros Mauro Carvalho Chehab
2014-11-01 13:38   ` [PATCH 2/7] [media] cx231xx: Fix identation Mauro Carvalho Chehab
2014-11-01 13:38   ` [PATCH 3/7] [media] cx231xx: Cleanup printk at the driver Mauro Carvalho Chehab
2014-11-01 14:01     ` Antti Palosaari
2014-11-01 14:08       ` Mauro Carvalho Chehab [this message]
2014-11-02 11:46       ` Mauro Carvalho Chehab
2014-11-01 13:38   ` [PATCH 4/7] [media] cx25840: Don't report an error if max size is adjusted Mauro Carvalho Chehab
2014-11-01 13:38   ` [PATCH 5/7] [media] cx25840: convert max_buf_size var to lowercase Mauro Carvalho Chehab
2014-11-01 13:38   ` [PATCH 6/7] [media] cx231xx-i2c: fix i2c_scan modprobe parameter Mauro Carvalho Chehab
2014-11-01 16:46     ` Matthias Schwarzott
2014-11-01 20:19       ` [PATCH] cx231xx: use 1 byte read for i2c scan Matthias Schwarzott
2014-11-02  1:24       ` [PATCH 6/7] [media] cx231xx-i2c: fix i2c_scan modprobe parameter Mauro Carvalho Chehab
2014-11-01 13:38   ` [PATCH 7/7] [media] cx231xx: disable I2C errors during i2c_scan Mauro Carvalho Chehab

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141101120858.47dc0bd7@recife.lan \
    --to=mchehab@osg.samsung.com \
    --cc=crope@iki.fi \
    --cc=hans.verkuil@cisco.com \
    --cc=laurent.pinchart+renesas@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@infradead.org \
    --cc=peter.senna@gmail.com \
    --cc=ramakrmu@cisco.com \
    --cc=sakari.ailus@linux.intel.com \
    --cc=tiwai@suse.de \
    --cc=zzam@gentoo.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox