* [PATCH] staging: media: as102: replace custom dprintk() with dev_dbg() @ 2014-05-17 13:16 Martin Kepplinger 2014-05-17 13:59 ` Antti Palosaari 0 siblings, 1 reply; 10+ messages in thread From: Martin Kepplinger @ 2014-05-17 13:16 UTC (permalink / raw) To: gregkh; +Cc: m.chehab, linux-media, devel, linux-kernel, Martin Kepplinger don't reinvent dev_dbg(). use the common kernel coding style. Signed-off-by: Martin Kepplinger <martink@posteo.de> --- this applies to next-20140516. drivers/staging/media/as102/as102_drv.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/staging/media/as102/as102_drv.c b/drivers/staging/media/as102/as102_drv.c index 09d64cd..99c3ed93 100644 --- a/drivers/staging/media/as102/as102_drv.c +++ b/drivers/staging/media/as102/as102_drv.c @@ -74,7 +74,8 @@ static void as102_stop_stream(struct as102_dev_t *dev) return; if (as10x_cmd_stop_streaming(bus_adap) < 0) - dprintk(debug, "as10x_cmd_stop_streaming failed\n"); + dev_dbg(&dev->bus_adap.usb_dev->dev, + "as10x_cmd_stop_streaming failed\n"); mutex_unlock(&dev->bus_adap.lock); } @@ -112,14 +113,16 @@ static int as10x_pid_filter(struct as102_dev_t *dev, int ret = -EFAULT; if (mutex_lock_interruptible(&dev->bus_adap.lock)) { - dprintk(debug, "mutex_lock_interruptible(lock) failed !\n"); + dev_dbg(&dev->bus_adap.usb_dev->dev, + "amutex_lock_interruptible(lock) failed !\n"); return -EBUSY; } switch (onoff) { case 0: ret = as10x_cmd_del_PID_filter(bus_adap, (uint16_t) pid); - dprintk(debug, "DEL_PID_FILTER([%02d] 0x%04x) ret = %d\n", + dev_dbg(&dev->bus_adap.usb_dev->dev, + "DEL_PID_FILTER([%02d] 0x%04x) ret = %d\n", index, pid, ret); break; case 1: @@ -131,7 +134,7 @@ static int as10x_pid_filter(struct as102_dev_t *dev, filter.pid = pid; ret = as10x_cmd_add_PID_filter(bus_adap, &filter); - dprintk(debug, + dev_dbg(&dev->bus_adap.usb_dev->dev, "ADD_PID_FILTER([%02d -> %02d], 0x%04x) ret = %d\n", index, filter.idx, filter.pid, ret); break; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] staging: media: as102: replace custom dprintk() with dev_dbg() 2014-05-17 13:16 [PATCH] staging: media: as102: replace custom dprintk() with dev_dbg() Martin Kepplinger @ 2014-05-17 13:59 ` Antti Palosaari 2014-05-17 16:05 ` [PATCHv2] " Martin Kepplinger 0 siblings, 1 reply; 10+ messages in thread From: Antti Palosaari @ 2014-05-17 13:59 UTC (permalink / raw) To: Martin Kepplinger, gregkh; +Cc: m.chehab, linux-media, devel, linux-kernel you forget to remove debug parameter itself. Antti On 05/17/2014 04:16 PM, Martin Kepplinger wrote: > don't reinvent dev_dbg(). use the common kernel coding style. > > Signed-off-by: Martin Kepplinger <martink@posteo.de> > --- > this applies to next-20140516. > > drivers/staging/media/as102/as102_drv.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/drivers/staging/media/as102/as102_drv.c b/drivers/staging/media/as102/as102_drv.c > index 09d64cd..99c3ed93 100644 > --- a/drivers/staging/media/as102/as102_drv.c > +++ b/drivers/staging/media/as102/as102_drv.c > @@ -74,7 +74,8 @@ static void as102_stop_stream(struct as102_dev_t *dev) > return; > > if (as10x_cmd_stop_streaming(bus_adap) < 0) > - dprintk(debug, "as10x_cmd_stop_streaming failed\n"); > + dev_dbg(&dev->bus_adap.usb_dev->dev, > + "as10x_cmd_stop_streaming failed\n"); > > mutex_unlock(&dev->bus_adap.lock); > } > @@ -112,14 +113,16 @@ static int as10x_pid_filter(struct as102_dev_t *dev, > int ret = -EFAULT; > > if (mutex_lock_interruptible(&dev->bus_adap.lock)) { > - dprintk(debug, "mutex_lock_interruptible(lock) failed !\n"); > + dev_dbg(&dev->bus_adap.usb_dev->dev, > + "amutex_lock_interruptible(lock) failed !\n"); > return -EBUSY; > } > > switch (onoff) { > case 0: > ret = as10x_cmd_del_PID_filter(bus_adap, (uint16_t) pid); > - dprintk(debug, "DEL_PID_FILTER([%02d] 0x%04x) ret = %d\n", > + dev_dbg(&dev->bus_adap.usb_dev->dev, > + "DEL_PID_FILTER([%02d] 0x%04x) ret = %d\n", > index, pid, ret); > break; > case 1: > @@ -131,7 +134,7 @@ static int as10x_pid_filter(struct as102_dev_t *dev, > filter.pid = pid; > > ret = as10x_cmd_add_PID_filter(bus_adap, &filter); > - dprintk(debug, > + dev_dbg(&dev->bus_adap.usb_dev->dev, > "ADD_PID_FILTER([%02d -> %02d], 0x%04x) ret = %d\n", > index, filter.idx, filter.pid, ret); > break; > -- http://palosaari.fi/ ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCHv2] staging: media: as102: replace custom dprintk() with dev_dbg() 2014-05-17 13:59 ` Antti Palosaari @ 2014-05-17 16:05 ` Martin Kepplinger 2014-05-17 17:21 ` Antti Palosaari 0 siblings, 1 reply; 10+ messages in thread From: Martin Kepplinger @ 2014-05-17 16:05 UTC (permalink / raw) To: gregkh, crope Cc: m.chehab, linux-media, devel, linux-kernel, Martin Kepplinger don't reinvent dev_dbg(). remove dprintk() in as102_drv.c. use the common kernel coding style. Signed-off-by: Martin Kepplinger <martink@posteo.de> --- this applies to next-20140516. any more suggestions? more cleanup can be done when dprintk() is completely gone. drivers/staging/media/as102/as102_drv.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/drivers/staging/media/as102/as102_drv.c b/drivers/staging/media/as102/as102_drv.c index 09d64cd..e0ee618 100644 --- a/drivers/staging/media/as102/as102_drv.c +++ b/drivers/staging/media/as102/as102_drv.c @@ -31,10 +31,6 @@ #include "as102_fw.h" #include "dvbdev.h" -int as102_debug; -module_param_named(debug, as102_debug, int, 0644); -MODULE_PARM_DESC(debug, "Turn on/off debugging (default: off)"); - int dual_tuner; module_param_named(dual_tuner, dual_tuner, int, 0644); MODULE_PARM_DESC(dual_tuner, "Activate Dual-Tuner config (default: off)"); @@ -74,7 +70,8 @@ static void as102_stop_stream(struct as102_dev_t *dev) return; if (as10x_cmd_stop_streaming(bus_adap) < 0) - dprintk(debug, "as10x_cmd_stop_streaming failed\n"); + dev_dbg(&dev->bus_adap.usb_dev->dev, + "as10x_cmd_stop_streaming failed\n"); mutex_unlock(&dev->bus_adap.lock); } @@ -112,14 +109,16 @@ static int as10x_pid_filter(struct as102_dev_t *dev, int ret = -EFAULT; if (mutex_lock_interruptible(&dev->bus_adap.lock)) { - dprintk(debug, "mutex_lock_interruptible(lock) failed !\n"); + dev_dbg(&dev->bus_adap.usb_dev->dev, + "amutex_lock_interruptible(lock) failed !\n"); return -EBUSY; } switch (onoff) { case 0: ret = as10x_cmd_del_PID_filter(bus_adap, (uint16_t) pid); - dprintk(debug, "DEL_PID_FILTER([%02d] 0x%04x) ret = %d\n", + dev_dbg(&dev->bus_adap.usb_dev->dev, + "DEL_PID_FILTER([%02d] 0x%04x) ret = %d\n", index, pid, ret); break; case 1: @@ -131,7 +130,7 @@ static int as10x_pid_filter(struct as102_dev_t *dev, filter.pid = pid; ret = as10x_cmd_add_PID_filter(bus_adap, &filter); - dprintk(debug, + dev_dbg(&dev->bus_adap.usb_dev->dev, "ADD_PID_FILTER([%02d -> %02d], 0x%04x) ret = %d\n", index, filter.idx, filter.pid, ret); break; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCHv2] staging: media: as102: replace custom dprintk() with dev_dbg() 2014-05-17 16:05 ` [PATCHv2] " Martin Kepplinger @ 2014-05-17 17:21 ` Antti Palosaari 2014-05-17 17:52 ` Martin Kepplinger 2014-05-17 19:22 ` Dan Carpenter 0 siblings, 2 replies; 10+ messages in thread From: Antti Palosaari @ 2014-05-17 17:21 UTC (permalink / raw) To: Martin Kepplinger, gregkh, Devin Heitmueller Cc: m.chehab, linux-media, devel, linux-kernel On 05/17/2014 07:05 PM, Martin Kepplinger wrote: > don't reinvent dev_dbg(). remove dprintk() in as102_drv.c. > use the common kernel coding style. > > Signed-off-by: Martin Kepplinger <martink@posteo.de> Reviewed-by: Antti Palosaari <crope@iki.fi> > --- > this applies to next-20140516. any more suggestions? > more cleanup can be done when dprintk() is completely gone. Do you have the device? I am a bit reluctant patching that driver without any testing as it has happened too many times something has gone totally broken. IIRC Devin said it is in staging because of style issues and nothing more. Is that correct? regards Antti -- http://palosaari.fi/ ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCHv2] staging: media: as102: replace custom dprintk() with dev_dbg() 2014-05-17 17:21 ` Antti Palosaari @ 2014-05-17 17:52 ` Martin Kepplinger 2014-05-17 18:43 ` Gianluca Gennari 2014-05-17 19:22 ` Dan Carpenter 1 sibling, 1 reply; 10+ messages in thread From: Martin Kepplinger @ 2014-05-17 17:52 UTC (permalink / raw) To: Antti Palosaari, gregkh, Devin Heitmueller Cc: m.chehab, linux-media, devel, linux-kernel Am 2014-05-17 19:21, schrieb Antti Palosaari: > On 05/17/2014 07:05 PM, Martin Kepplinger wrote: >> don't reinvent dev_dbg(). remove dprintk() in as102_drv.c. >> use the common kernel coding style. >> >> Signed-off-by: Martin Kepplinger <martink@posteo.de> > > Reviewed-by: Antti Palosaari <crope@iki.fi> > >> --- >> this applies to next-20140516. any more suggestions? >> more cleanup can be done when dprintk() is completely gone. > > Do you have the device? I am a bit reluctant patching that driver > without any testing as it has happened too many times something has gone > totally broken. I don't have the device and will, at most, change such style issues. > > IIRC Devin said it is in staging because of style issues and nothing > more. Is that correct? I haven't heard anything. A TODO file would help. > > regards > Antti > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCHv2] staging: media: as102: replace custom dprintk() with dev_dbg() 2014-05-17 17:52 ` Martin Kepplinger @ 2014-05-17 18:43 ` Gianluca Gennari 0 siblings, 0 replies; 10+ messages in thread From: Gianluca Gennari @ 2014-05-17 18:43 UTC (permalink / raw) To: Martin Kepplinger, Antti Palosaari, gregkh, Devin Heitmueller Cc: m.chehab, linux-media, devel, linux-kernel Il 17/05/2014 19:52, Martin Kepplinger ha scritto: > Am 2014-05-17 19:21, schrieb Antti Palosaari: >> On 05/17/2014 07:05 PM, Martin Kepplinger wrote: >>> don't reinvent dev_dbg(). remove dprintk() in as102_drv.c. >>> use the common kernel coding style. >>> >>> Signed-off-by: Martin Kepplinger <martink@posteo.de> >> >> Reviewed-by: Antti Palosaari <crope@iki.fi> >> >>> --- >>> this applies to next-20140516. any more suggestions? >>> more cleanup can be done when dprintk() is completely gone. >> >> Do you have the device? I am a bit reluctant patching that driver >> without any testing as it has happened too many times something has gone >> totally broken. > I don't have the device and will, at most, change such style issues. > >> >> IIRC Devin said it is in staging because of style issues and nothing >> more. Is that correct? > I haven't heard anything. A TODO file would help. Hi Antti, Martin, if I remember correctly, the main issue with this driver is that the device does not work anymore after a reboot: it needs a power cycle to start working again. Probably this issue is enough to keep the driver in staging. Regards, Gianluca > >> >> regards >> Antti >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-media" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCHv2] staging: media: as102: replace custom dprintk() with dev_dbg() 2014-05-17 17:21 ` Antti Palosaari 2014-05-17 17:52 ` Martin Kepplinger @ 2014-05-17 19:22 ` Dan Carpenter 1 sibling, 0 replies; 10+ messages in thread From: Dan Carpenter @ 2014-05-17 19:22 UTC (permalink / raw) To: Antti Palosaari Cc: Martin Kepplinger, gregkh, Devin Heitmueller, devel, linux-media, linux-kernel, m.chehab On Sat, May 17, 2014 at 08:21:03PM +0300, Antti Palosaari wrote: > On 05/17/2014 07:05 PM, Martin Kepplinger wrote: > >don't reinvent dev_dbg(). remove dprintk() in as102_drv.c. > >use the common kernel coding style. > > > >Signed-off-by: Martin Kepplinger <martink@posteo.de> > > Reviewed-by: Antti Palosaari <crope@iki.fi> > > >--- > >this applies to next-20140516. any more suggestions? > >more cleanup can be done when dprintk() is completely gone. > > Do you have the device? I am a bit reluctant patching that driver > without any testing as it has happened too many times something has > gone totally broken. Looking through the log the only time I see breakage is build breakage on allyesconfig. 1ec9a35 [media] staging: as102: Add missing function argument This was a compile warning and it definitely should have been caught before the code was submitted or merged, but it wasn't something people would hit in real life. regards, dan carpenter ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] staging: media: as102: replace custom dprintk() with dev_dbg()
@ 2014-08-04 9:10 Dan Carpenter
2014-08-04 10:17 ` [PATCHv2] " Martin Kepplinger
0 siblings, 1 reply; 10+ messages in thread
From: Dan Carpenter @ 2014-08-04 9:10 UTC (permalink / raw)
To: Martin Kepplinger; +Cc: gregkh, devel, linux-media, linux-kernel, m.chehab
On Sun, Aug 03, 2014 at 04:54:21PM +0200, Martin Kepplinger wrote:
> @@ -447,6 +457,13 @@ static uint8_t as102_fe_get_code_rate(fe_code_rate_t arg)
> static void as102_fe_copy_tune_parameters(struct as10x_tune_args *tune_args,
> struct dtv_frontend_properties *params)
> {
> + struct dvb_frontend *fe;
> + struct as102_dev_t *dev;
> +
> + fe = container_of(params, struct dvb_frontend, dtv_property_cache);
> + dev = (struct as102_dev_t *) fe->tuner_priv;
> + if (dev == NULL)
> + dev_err(&dev->bus_adap.usb_dev->dev, "No device found\n");
NULL dereference in printing error message. I think smatch or
coccinelle would detect this although I haven't tried either.
This is the typical bug for this kind of patch.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCHv2] staging: media: as102: replace custom dprintk() with dev_dbg() 2014-08-04 9:10 [PATCH] " Dan Carpenter @ 2014-08-04 10:17 ` Martin Kepplinger 2014-08-04 10:40 ` Dan Carpenter 0 siblings, 1 reply; 10+ messages in thread From: Martin Kepplinger @ 2014-08-04 10:17 UTC (permalink / raw) To: dan.carpenter Cc: gregkh, devel, linux-media, linux-kernel, m.chehab, Martin Kepplinger remove dprintk() and replace it with dev_dbg() in order to use the common kernel coding style. Signed-off-by: Martin Kepplinger <martink@posteo.de> --- Thanks Dan. And since it continues to succeed if (dev == NULL), differntiate if (dev) or not. drivers/staging/media/as102/as102_drv.c | 15 +++--- drivers/staging/media/as102/as102_drv.h | 7 --- drivers/staging/media/as102/as102_fe.c | 73 ++++++++++++++++++++------- drivers/staging/media/as102/as102_usb_drv.c | 41 ++++++++------- 4 files changed, 86 insertions(+), 50 deletions(-) diff --git a/drivers/staging/media/as102/as102_drv.c b/drivers/staging/media/as102/as102_drv.c index 09d64cd..e0ee618 100644 --- a/drivers/staging/media/as102/as102_drv.c +++ b/drivers/staging/media/as102/as102_drv.c @@ -31,10 +31,6 @@ #include "as102_fw.h" #include "dvbdev.h" -int as102_debug; -module_param_named(debug, as102_debug, int, 0644); -MODULE_PARM_DESC(debug, "Turn on/off debugging (default: off)"); - int dual_tuner; module_param_named(dual_tuner, dual_tuner, int, 0644); MODULE_PARM_DESC(dual_tuner, "Activate Dual-Tuner config (default: off)"); @@ -74,7 +70,8 @@ static void as102_stop_stream(struct as102_dev_t *dev) return; if (as10x_cmd_stop_streaming(bus_adap) < 0) - dprintk(debug, "as10x_cmd_stop_streaming failed\n"); + dev_dbg(&dev->bus_adap.usb_dev->dev, + "as10x_cmd_stop_streaming failed\n"); mutex_unlock(&dev->bus_adap.lock); } @@ -112,14 +109,16 @@ static int as10x_pid_filter(struct as102_dev_t *dev, int ret = -EFAULT; if (mutex_lock_interruptible(&dev->bus_adap.lock)) { - dprintk(debug, "mutex_lock_interruptible(lock) failed !\n"); + dev_dbg(&dev->bus_adap.usb_dev->dev, + "amutex_lock_interruptible(lock) failed !\n"); return -EBUSY; } switch (onoff) { case 0: ret = as10x_cmd_del_PID_filter(bus_adap, (uint16_t) pid); - dprintk(debug, "DEL_PID_FILTER([%02d] 0x%04x) ret = %d\n", + dev_dbg(&dev->bus_adap.usb_dev->dev, + "DEL_PID_FILTER([%02d] 0x%04x) ret = %d\n", index, pid, ret); break; case 1: @@ -131,7 +130,7 @@ static int as10x_pid_filter(struct as102_dev_t *dev, filter.pid = pid; ret = as10x_cmd_add_PID_filter(bus_adap, &filter); - dprintk(debug, + dev_dbg(&dev->bus_adap.usb_dev->dev, "ADD_PID_FILTER([%02d -> %02d], 0x%04x) ret = %d\n", index, filter.idx, filter.pid, ret); break; diff --git a/drivers/staging/media/as102/as102_drv.h b/drivers/staging/media/as102/as102_drv.h index a06837d..49d0c42 100644 --- a/drivers/staging/media/as102/as102_drv.h +++ b/drivers/staging/media/as102/as102_drv.h @@ -27,17 +27,10 @@ #define DRIVER_FULL_NAME "Abilis Systems as10x usb driver" #define DRIVER_NAME "as10x_usb" -extern int as102_debug; #define debug as102_debug extern struct usb_driver as102_usb_driver; extern int elna_enable; -#define dprintk(debug, args...) \ - do { if (debug) { \ - pr_debug("%s: ", __func__); \ - printk(args); \ - } } while (0) - #define AS102_DEVICE_MAJOR 192 #define AS102_USB_BUF_SIZE 512 diff --git a/drivers/staging/media/as102/as102_fe.c b/drivers/staging/media/as102/as102_fe.c index b686b76..4c4c47d 100644 --- a/drivers/staging/media/as102/as102_fe.c +++ b/drivers/staging/media/as102/as102_fe.c @@ -46,7 +46,8 @@ static int as102_fe_set_frontend(struct dvb_frontend *fe) /* send abilis command: SET_TUNE */ ret = as10x_cmd_set_tune(&dev->bus_adap, &tune_args); if (ret != 0) - dprintk(debug, "as10x_cmd_set_tune failed. (err = %d)\n", ret); + dev_dbg(&dev->bus_adap.usb_dev->dev, + "as10x_cmd_set_tune failed. (err = %d)\n", ret); mutex_unlock(&dev->bus_adap.lock); @@ -82,10 +83,17 @@ static int as102_fe_get_tune_settings(struct dvb_frontend *fe, struct dvb_frontend_tune_settings *settings) { #if 0 - dprintk(debug, "step_size = %d\n", settings->step_size); - dprintk(debug, "max_drift = %d\n", settings->max_drift); - dprintk(debug, "min_delay_ms = %d -> %d\n", settings->min_delay_ms, - 1000); + struct as102_dev_t *dev; + + dev = (struct as102_dev_t *) fe->tuner_priv; + if (dev == NULL) + return -EINVAL; + dev_dbg(&dev->bus_adap.usb_dev->dev, + "step_size = %d\n", settings->step_size); + dev_dbg(&dev->bus_adap.usb_dev->dev, + "max_drift = %d\n", settings->max_drift); + dev_dbg(&dev->bus_adap.usb_dev->dev, + "min_delay_ms = %d -> %d\n", settings->min_delay_ms, 1000); #endif settings->min_delay_ms = 1000; @@ -110,7 +118,8 @@ static int as102_fe_read_status(struct dvb_frontend *fe, fe_status_t *status) /* send abilis command: GET_TUNE_STATUS */ ret = as10x_cmd_get_tune_status(&dev->bus_adap, &tstate); if (ret < 0) { - dprintk(debug, "as10x_cmd_get_tune_status failed (err = %d)\n", + dev_dbg(&dev->bus_adap.usb_dev->dev, + "as10x_cmd_get_tune_status failed (err = %d)\n", ret); goto out; } @@ -133,7 +142,8 @@ static int as102_fe_read_status(struct dvb_frontend *fe, fe_status_t *status) *status = TUNE_STATUS_NOT_TUNED; } - dprintk(debug, "tuner status: 0x%02x, strength %d, per: %d, ber: %d\n", + dev_dbg(&dev->bus_adap.usb_dev->dev, + "tuner status: 0x%02x, strength %d, per: %d, ber: %d\n", tstate.tune_state, tstate.signal_strength, tstate.PER, tstate.BER); @@ -141,10 +151,10 @@ static int as102_fe_read_status(struct dvb_frontend *fe, fe_status_t *status) if (as10x_cmd_get_demod_stats(&dev->bus_adap, (struct as10x_demod_stats *) &dev->demod_stats) < 0) { memset(&dev->demod_stats, 0, sizeof(dev->demod_stats)); - dprintk(debug, - "as10x_cmd_get_demod_stats failed (probably not tuned)\n"); + dev_dbg(&dev->bus_adap.usb_dev->dev, + "as10x_cmd_get_demod_stats failed (probably not tuned)\n"); } else { - dprintk(debug, + dev_dbg(&dev->bus_adap.usb_dev->dev, "demod status: fc: 0x%08x, bad fc: 0x%08x, " "bytes corrected: 0x%08x , MER: 0x%04x\n", dev->demod_stats.frame_count, @@ -447,6 +457,15 @@ static uint8_t as102_fe_get_code_rate(fe_code_rate_t arg) static void as102_fe_copy_tune_parameters(struct as10x_tune_args *tune_args, struct dtv_frontend_properties *params) { + struct dvb_frontend *fe; + struct as102_dev_t *dev; + + fe = container_of(params, struct dvb_frontend, dtv_property_cache); + dev = (struct as102_dev_t *) fe->tuner_priv; + if (dev == NULL) + pr_err("as102: No device found\n"); + else + dev_err(&dev->bus_adap.usb_dev->dev, "No device found\n"); /* set frequency */ tune_args->freq = params->frequency / 1000; @@ -531,10 +550,18 @@ static void as102_fe_copy_tune_parameters(struct as10x_tune_args *tune_args, break; } - dprintk(debug, "tuner parameters: freq: %d bw: 0x%02x gi: 0x%02x\n", + if (dev) { + dev_dbg(&dev->bus_adap.usb_dev->dev, + "tuner parameters: freq: %d bw: 0x%02x gi: 0x%02x\n", + params->frequency, + tune_args->bandwidth, + tune_args->guard_interval); + } else { + pr_debug("as102: tuner parameters: freq: %d bw: 0x%02x gi: 0x%02x\n", params->frequency, tune_args->bandwidth, tune_args->guard_interval); + } /* * Detect a hierarchy selection @@ -556,14 +583,24 @@ static void as102_fe_copy_tune_parameters(struct as10x_tune_args *tune_args, as102_fe_get_code_rate(params->code_rate_LP); } - dprintk(debug, + if (dev) { + dev_dbg(&dev->bus_adap.usb_dev->dev, "\thierarchy: 0x%02x selected: %s code_rate_%s: 0x%02x\n", - tune_args->hierarchy, - tune_args->hier_select == HIER_HIGH_PRIORITY ? - "HP" : "LP", - tune_args->hier_select == HIER_HIGH_PRIORITY ? - "HP" : "LP", - tune_args->code_rate); + tune_args->hierarchy, + tune_args->hier_select == HIER_HIGH_PRIORITY ? + "HP" : "LP", + tune_args->hier_select == HIER_HIGH_PRIORITY ? + "HP" : "LP", + tune_args->code_rate); + } else { + pr_debug("as102: \thierarchy: 0x%02x selected: %s code_rate_%s: 0x%02x\n", + tune_args->hierarchy, + tune_args->hier_select == HIER_HIGH_PRIORITY ? + "HP" : "LP", + tune_args->hier_select == HIER_HIGH_PRIORITY ? + "HP" : "LP", + tune_args->code_rate); + } } else { tune_args->code_rate = as102_fe_get_code_rate(params->code_rate_HP); diff --git a/drivers/staging/media/as102/as102_usb_drv.c b/drivers/staging/media/as102/as102_usb_drv.c index e6f6278..86f83b9 100644 --- a/drivers/staging/media/as102/as102_usb_drv.c +++ b/drivers/staging/media/as102/as102_usb_drv.c @@ -104,21 +104,22 @@ static int as102_usb_xfer_cmd(struct as10x_bus_adapter_t *bus_adap, send_buf, send_buf_len, USB_CTRL_SET_TIMEOUT /* 200 */); if (ret < 0) { - dprintk(debug, "usb_control_msg(send) failed, err %i\n", - ret); + dev_dbg(&bus_adap->usb_dev->dev, + "usb_control_msg(send) failed, err %i\n", ret); return ret; } if (ret != send_buf_len) { - dprintk(debug, "only wrote %d of %d bytes\n", - ret, send_buf_len); + dev_dbg(&bus_adap->usb_dev->dev, + "only wrote %d of %d bytes\n", ret, send_buf_len); return -1; } } if (recv_buf != NULL) { #ifdef TRACE - dprintk(debug, "want to read: %d bytes\n", recv_buf_len); + dev_dbg(bus_adap->usb_dev->dev, + "want to read: %d bytes\n", recv_buf_len); #endif ret = usb_control_msg(bus_adap->usb_dev, usb_rcvctrlpipe(bus_adap->usb_dev, 0), @@ -130,12 +131,13 @@ static int as102_usb_xfer_cmd(struct as10x_bus_adapter_t *bus_adap, recv_buf, recv_buf_len, USB_CTRL_GET_TIMEOUT /* 200 */); if (ret < 0) { - dprintk(debug, "usb_control_msg(recv) failed, err %i\n", - ret); + dev_dbg(&bus_adap->usb_dev->dev, + "usb_control_msg(recv) failed, err %i\n", ret); return ret; } #ifdef TRACE - dprintk(debug, "read %d bytes\n", recv_buf_len); + dev_dbg(bus_adap->usb_dev->dev, + "read %d bytes\n", recv_buf_len); #endif } @@ -153,13 +155,14 @@ static int as102_send_ep1(struct as10x_bus_adapter_t *bus_adap, usb_sndbulkpipe(bus_adap->usb_dev, 1), send_buf, send_buf_len, &actual_len, 200); if (ret) { - dprintk(debug, "usb_bulk_msg(send) failed, err %i\n", ret); + dev_dbg(&bus_adap->usb_dev->dev, + "usb_bulk_msg(send) failed, err %i\n", ret); return ret; } if (actual_len != send_buf_len) { - dprintk(debug, "only wrote %d of %d bytes\n", - actual_len, send_buf_len); + dev_dbg(&bus_adap->usb_dev->dev, "only wrote %d of %d bytes\n", + actual_len, send_buf_len); return -1; } return ret ? ret : actual_len; @@ -177,13 +180,14 @@ static int as102_read_ep2(struct as10x_bus_adapter_t *bus_adap, usb_rcvbulkpipe(bus_adap->usb_dev, 2), recv_buf, recv_buf_len, &actual_len, 200); if (ret) { - dprintk(debug, "usb_bulk_msg(recv) failed, err %i\n", ret); + dev_dbg(&bus_adap->usb_dev->dev, + "usb_bulk_msg(recv) failed, err %i\n", ret); return ret; } if (actual_len != recv_buf_len) { - dprintk(debug, "only read %d of %d bytes\n", - actual_len, recv_buf_len); + dev_dbg(&bus_adap->usb_dev->dev, "only read %d of %d bytes\n", + actual_len, recv_buf_len); return -1; } return ret ? ret : actual_len; @@ -211,7 +215,8 @@ static int as102_submit_urb_stream(struct as102_dev_t *dev, struct urb *urb) err = usb_submit_urb(urb, GFP_ATOMIC); if (err) - dprintk(debug, "%s: usb_submit_urb failed\n", __func__); + dev_dbg(&urb->dev->dev, + "%s: usb_submit_urb failed\n", __func__); return err; } @@ -256,7 +261,8 @@ static int as102_alloc_usb_stream_buffer(struct as102_dev_t *dev) GFP_KERNEL, &dev->dma_addr); if (!dev->stream) { - dprintk(debug, "%s: usb_buffer_alloc failed\n", __func__); + dev_dbg(&dev->bus_adap.usb_dev->dev, + "%s: usb_buffer_alloc failed\n", __func__); return -ENOMEM; } @@ -268,7 +274,8 @@ static int as102_alloc_usb_stream_buffer(struct as102_dev_t *dev) urb = usb_alloc_urb(0, GFP_ATOMIC); if (urb == NULL) { - dprintk(debug, "%s: usb_alloc_urb failed\n", __func__); + dev_dbg(&dev->bus_adap.usb_dev->dev, + "%s: usb_alloc_urb failed\n", __func__); as102_free_usb_stream_buffer(dev); return -ENOMEM; } -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCHv2] staging: media: as102: replace custom dprintk() with dev_dbg() 2014-08-04 10:17 ` [PATCHv2] " Martin Kepplinger @ 2014-08-04 10:40 ` Dan Carpenter 2014-08-04 11:12 ` Joe Perches 0 siblings, 1 reply; 10+ messages in thread From: Dan Carpenter @ 2014-08-04 10:40 UTC (permalink / raw) To: Martin Kepplinger; +Cc: gregkh, devel, linux-media, linux-kernel, m.chehab On thinking about it some more the proper way to redo this would have been to remove the bogus check. Also a couple trivial things below. On Mon, Aug 04, 2014 at 12:17:14PM +0200, Martin Kepplinger wrote: > @@ -82,10 +83,17 @@ static int as102_fe_get_tune_settings(struct dvb_frontend *fe, > struct dvb_frontend_tune_settings *settings) { > > #if 0 > - dprintk(debug, "step_size = %d\n", settings->step_size); > - dprintk(debug, "max_drift = %d\n", settings->max_drift); > - dprintk(debug, "min_delay_ms = %d -> %d\n", settings->min_delay_ms, > - 1000); > + struct as102_dev_t *dev; > + > + dev = (struct as102_dev_t *) fe->tuner_priv; > + if (dev == NULL) > + return -EINVAL; This doesn't make sense. Why are we crapping out just because we can't print some useless debug info? Debugging code annoys me because of stuff like this. Debugging code is buggier than normal code because it never gets tested. It complicates the code. People think they are helping when they add lots of debug code, but normally they are just making the situation worse. Also it bloats the kernel a bit. Just remove this whole block because it is ifdef 0. > + dev_dbg(&dev->bus_adap.usb_dev->dev, > + "step_size = %d\n", settings->step_size); > + dev_dbg(&dev->bus_adap.usb_dev->dev, > + "max_drift = %d\n", settings->max_drift); > + dev_dbg(&dev->bus_adap.usb_dev->dev, > + "min_delay_ms = %d -> %d\n", settings->min_delay_ms, 1000); > #endif > > settings->min_delay_ms = 1000; > @@ -141,10 +151,10 @@ static int as102_fe_read_status(struct dvb_frontend *fe, fe_status_t *status) > if (as10x_cmd_get_demod_stats(&dev->bus_adap, > (struct as10x_demod_stats *) &dev->demod_stats) < 0) { > memset(&dev->demod_stats, 0, sizeof(dev->demod_stats)); > - dprintk(debug, > - "as10x_cmd_get_demod_stats failed (probably not tuned)\n"); > + dev_dbg(&dev->bus_adap.usb_dev->dev, > + "as10x_cmd_get_demod_stats failed (probably not tuned)\n"); This isn't indented correctly. > } else { > - dprintk(debug, > + dev_dbg(&dev->bus_adap.usb_dev->dev, > "demod status: fc: 0x%08x, bad fc: 0x%08x, " > "bytes corrected: 0x%08x , MER: 0x%04x\n", > dev->demod_stats.frame_count, > @@ -447,6 +457,15 @@ static uint8_t as102_fe_get_code_rate(fe_code_rate_t arg) > static void as102_fe_copy_tune_parameters(struct as10x_tune_args *tune_args, > struct dtv_frontend_properties *params) > { > + struct dvb_frontend *fe; > + struct as102_dev_t *dev; > + > + fe = container_of(params, struct dvb_frontend, dtv_property_cache); > + dev = (struct as102_dev_t *) fe->tuner_priv; > + if (dev == NULL) > + pr_err("as102: No device found\n"); This condition is never true. Don't add tests if you know the answer. > + else > + dev_err(&dev->bus_adap.usb_dev->dev, "No device found\n"); > > /* set frequency */ > tune_args->freq = params->frequency / 1000; > @@ -531,10 +550,18 @@ static void as102_fe_copy_tune_parameters(struct as10x_tune_args *tune_args, > break; > } > > - dprintk(debug, "tuner parameters: freq: %d bw: 0x%02x gi: 0x%02x\n", > + if (dev) { > + dev_dbg(&dev->bus_adap.usb_dev->dev, > + "tuner parameters: freq: %d bw: 0x%02x gi: 0x%02x\n", > + params->frequency, > + tune_args->bandwidth, > + tune_args->guard_interval); > + } else { > + pr_debug("as102: tuner parameters: freq: %d bw: 0x%02x gi: 0x%02x\n", > params->frequency, > tune_args->bandwidth, > tune_args->guard_interval); > + } This isn't indented correctly. I wish checkpatch.pl would catch that... Anyway, the else side can be removed as explained earlier. regards, dan carpenter ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCHv2] staging: media: as102: replace custom dprintk() with dev_dbg() 2014-08-04 10:40 ` Dan Carpenter @ 2014-08-04 11:12 ` Joe Perches 0 siblings, 0 replies; 10+ messages in thread From: Joe Perches @ 2014-08-04 11:12 UTC (permalink / raw) To: Dan Carpenter Cc: Martin Kepplinger, gregkh, devel, linux-media, linux-kernel, m.chehab On Mon, 2014-08-04 at 13:40 +0300, Dan Carpenter wrote: > On Mon, Aug 04, 2014 at 12:17:14PM +0200, Martin Kepplinger wrote: [] > > + if (dev) { > > + dev_dbg(&dev->bus_adap.usb_dev->dev, > > + "tuner parameters: freq: %d bw: 0x%02x gi: 0x%02x\n", > > + params->frequency, > > + tune_args->bandwidth, > > + tune_args->guard_interval); > > + } else { > > + pr_debug("as102: tuner parameters: freq: %d bw: 0x%02x gi: 0x%02x\n", > > params->frequency, > > tune_args->bandwidth, > > tune_args->guard_interval); > > + } [] > This isn't indented correctly. I wish checkpatch.pl would catch that... > Anyway, the else side can be removed as explained earlier. checkpatch doesn't warn on any of: $ cat t.c static int func(void **bar) { bool b; /* test */ int a[2] = { 3, 6 }; if (b) { b = a[0] == a[1]; } return a[b]; } $ I think it'd be better if it would one day but it doesn't seem as simple as it seems for checkpatch to do it. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2014-08-04 11:12 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-05-17 13:16 [PATCH] staging: media: as102: replace custom dprintk() with dev_dbg() Martin Kepplinger 2014-05-17 13:59 ` Antti Palosaari 2014-05-17 16:05 ` [PATCHv2] " Martin Kepplinger 2014-05-17 17:21 ` Antti Palosaari 2014-05-17 17:52 ` Martin Kepplinger 2014-05-17 18:43 ` Gianluca Gennari 2014-05-17 19:22 ` Dan Carpenter -- strict thread matches above, loose matches on Subject: below -- 2014-08-04 9:10 [PATCH] " Dan Carpenter 2014-08-04 10:17 ` [PATCHv2] " Martin Kepplinger 2014-08-04 10:40 ` Dan Carpenter 2014-08-04 11:12 ` Joe Perches
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).