* [PATCH] staging: iio: fix coding style
@ 2014-04-09 18:09 Joel Porquet
2014-04-12 17:28 ` Jonathan Cameron
2014-06-14 15:21 ` Jonathan Cameron
0 siblings, 2 replies; 10+ messages in thread
From: Joel Porquet @ 2014-04-09 18:09 UTC (permalink / raw)
To: jic23, gregkh, jg1.han; +Cc: linux-iio, devel, linux-kernel, Joel Porquet
As suggested by checkpatch.pl, use dev_info() instead of
printk(KERN_INFO ...) to print message.
Signed-off-by: Joel Porquet <joel@porquet.org>
---
Only tested by compilation.
drivers/staging/iio/trigger/iio-trig-periodic-rtc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/iio/trigger/iio-trig-periodic-rtc.c b/drivers/staging/iio/trigger/iio-trig-periodic-rtc.c
index 48a6afa..38ecb4b 100644
--- a/drivers/staging/iio/trigger/iio-trig-periodic-rtc.c
+++ b/drivers/staging/iio/trigger/iio-trig-periodic-rtc.c
@@ -33,7 +33,8 @@ static int iio_trig_periodic_rtc_set_state(struct iio_trigger *trig, bool state)
struct iio_prtc_trigger_info *trig_info = iio_trigger_get_drvdata(trig);
if (trig_info->frequency == 0)
return -EINVAL;
- printk(KERN_INFO "trigger frequency is %d\n", trig_info->frequency);
+ dev_info(&trig_info->rtc->dev, "trigger frequency is %d\n",
+ trig_info->frequency);
return rtc_irq_set_state(trig_info->rtc, &trig_info->task, state);
}
--
1.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] staging: iio: fix coding style
2014-04-09 18:09 [PATCH] staging: iio: fix coding style Joel Porquet
@ 2014-04-12 17:28 ` Jonathan Cameron
2014-04-14 9:40 ` Dan O'Donovan
2014-04-14 13:59 ` Joël Porquet
2014-06-14 15:21 ` Jonathan Cameron
1 sibling, 2 replies; 10+ messages in thread
From: Jonathan Cameron @ 2014-04-12 17:28 UTC (permalink / raw)
To: Joel Porquet, gregkh, jg1.han; +Cc: linux-iio, devel, linux-kernel
On 09/04/14 19:09, Joel Porquet wrote:
> As suggested by checkpatch.pl, use dev_info() instead of
> printk(KERN_INFO ...) to print message.
>
> Signed-off-by: Joel Porquet <joel@porquet.org>
> ---
> Only tested by compilation.
> drivers/staging/iio/trigger/iio-trig-periodic-rtc.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/iio/trigger/iio-trig-periodic-rtc.c b/drivers/staging/iio/trigger/iio-trig-periodic-rtc.c
> index 48a6afa..38ecb4b 100644
> --- a/drivers/staging/iio/trigger/iio-trig-periodic-rtc.c
> +++ b/drivers/staging/iio/trigger/iio-trig-periodic-rtc.c
> @@ -33,7 +33,8 @@ static int iio_trig_periodic_rtc_set_state(struct iio_trigger *trig, bool state)
> struct iio_prtc_trigger_info *trig_info = iio_trigger_get_drvdata(trig);
> if (trig_info->frequency == 0)
> return -EINVAL;
> - printk(KERN_INFO "trigger frequency is %d\n", trig_info->frequency);
> + dev_info(&trig_info->rtc->dev, "trigger frequency is %d\n",
> + trig_info->frequency);
The principle is good, but why make the error message us the underlying rtc device?
Going to lead to a rather unhelpful error message.
Perhaps the iio_trigger structures device element would make more sense?
Might not be terribly informative, but will at least come from the right
subsystem.
Also, I think we will be dropping this driver entirely at some point.
It was a dodgy hack that perhaps made sense at the time, but now a high
resolution timer is going to give better results.
> return rtc_irq_set_state(trig_info->rtc, &trig_info->task, state);
> }
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] staging: iio: fix coding style
2014-04-12 17:28 ` Jonathan Cameron
@ 2014-04-14 9:40 ` Dan O'Donovan
2014-04-14 13:59 ` Joël Porquet
1 sibling, 0 replies; 10+ messages in thread
From: Dan O'Donovan @ 2014-04-14 9:40 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Joel Porquet, gregkh, jg1.han, linux-iio, devel, linux-kernel
On Sat, 2014-04-12 at 18:28 +0100, Jonathan Cameron wrote:
> On 09/04/14 19:09, Joel Porquet wrote:
> > As suggested by checkpatch.pl, use dev_info() instead of
> > printk(KERN_INFO ...) to print message.
> >
> > Signed-off-by: Joel Porquet <joel@porquet.org>
> > ---
> > Only tested by compilation.
> > drivers/staging/iio/trigger/iio-trig-periodic-rtc.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/iio/trigger/iio-trig-periodic-rtc.c b/drivers/staging/iio/trigger/iio-trig-periodic-rtc.c
> > index 48a6afa..38ecb4b 100644
> > --- a/drivers/staging/iio/trigger/iio-trig-periodic-rtc.c
> > +++ b/drivers/staging/iio/trigger/iio-trig-periodic-rtc.c
> > @@ -33,7 +33,8 @@ static int iio_trig_periodic_rtc_set_state(struct iio_trigger *trig, bool state)
> > struct iio_prtc_trigger_info *trig_info = iio_trigger_get_drvdata(trig);
> > if (trig_info->frequency == 0)
> > return -EINVAL;
> > - printk(KERN_INFO "trigger frequency is %d\n", trig_info->frequency);
> > + dev_info(&trig_info->rtc->dev, "trigger frequency is %d\n",
> > + trig_info->frequency);
> The principle is good, but why make the error message us the underlying rtc device?
> Going to lead to a rather unhelpful error message.
>
> Perhaps the iio_trigger structures device element would make more sense?
> Might not be terribly informative, but will at least come from the right
> subsystem.
>
> Also, I think we will be dropping this driver entirely at some point.
> It was a dodgy hack that perhaps made sense at the time, but now a high
> resolution timer is going to give better results.
I've been using this patch from Denis Ciocca which implements a hr-timer
trigger, and it has worked well for me:
http://comments.gmane.org/gmane.linux.kernel.iio/9698
It would be great to see this added to the kernel.
> > return rtc_irq_set_state(trig_info->rtc, &trig_info->task, state);
> > }
> >
> >
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" 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: [PATCH] staging: iio: fix coding style
2014-04-12 17:28 ` Jonathan Cameron
2014-04-14 9:40 ` Dan O'Donovan
@ 2014-04-14 13:59 ` Joël Porquet
2014-04-14 16:10 ` Jonathan Cameron
1 sibling, 1 reply; 10+ messages in thread
From: Joël Porquet @ 2014-04-14 13:59 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: gregkh, jg1.han, linux-iio, devel, linux-kernel
On Saturday, April 12, 2014 06:28:07 PM Jonathan Cameron wrote:
>
> On 09/04/14 19:09, Joel Porquet wrote:
> > As suggested by checkpatch.pl, use dev_info() instead of
> > printk(KERN_INFO ...) to print message.
> >
> > Signed-off-by: Joel Porquet <joel@porquet.org>
> > ---
> > Only tested by compilation.
> > drivers/staging/iio/trigger/iio-trig-periodic-rtc.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/iio/trigger/iio-trig-periodic-rtc.c b/drivers/staging/iio/trigger/iio-trig-periodic-rtc.c
> > index 48a6afa..38ecb4b 100644
> > --- a/drivers/staging/iio/trigger/iio-trig-periodic-rtc.c
> > +++ b/drivers/staging/iio/trigger/iio-trig-periodic-rtc.c
> > @@ -33,7 +33,8 @@ static int iio_trig_periodic_rtc_set_state(struct iio_trigger *trig, bool state)
> > struct iio_prtc_trigger_info *trig_info = iio_trigger_get_drvdata(trig);
> > if (trig_info->frequency == 0)
> > return -EINVAL;
> > - printk(KERN_INFO "trigger frequency is %d\n", trig_info->frequency);
> > + dev_info(&trig_info->rtc->dev, "trigger frequency is %d\n",
> > + trig_info->frequency);
> The principle is good, but why make the error message us the underlying rtc device?
> Going to lead to a rather unhelpful error message.
>
> Perhaps the iio_trigger structures device element would make more sense?
> Might not be terribly informative, but will at least come from the right
> subsystem.
>
> Also, I think we will be dropping this driver entirely at some point.
> It was a dodgy hack that perhaps made sense at the time, but now a high
> resolution timer is going to give better results.
OK, thanks for the feedback!
Does that still make sense that I resubmit another patch (using iio_trigger-dev instead)?
Or should I just drop this patch altogether since you seem to say that patching this driver is not really worth it anyway?
> > return rtc_irq_set_state(trig_info->rtc, &trig_info->task, state);
> > }
> >
> >
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] staging: iio: fix coding style
2014-04-14 13:59 ` Joël Porquet
@ 2014-04-14 16:10 ` Jonathan Cameron
0 siblings, 0 replies; 10+ messages in thread
From: Jonathan Cameron @ 2014-04-14 16:10 UTC (permalink / raw)
To: Joël Porquet; +Cc: gregkh, jg1.han, linux-iio, devel, linux-kernel
On April 14, 2014 2:59:32 PM GMT+01:00, "Joël Porquet" <joel@porquet.org> wrote:
>
>
>On Saturday, April 12, 2014 06:28:07 PM Jonathan Cameron wrote:
>>
>> On 09/04/14 19:09, Joel Porquet wrote:
>> > As suggested by checkpatch.pl, use dev_info() instead of
>> > printk(KERN_INFO ...) to print message.
>> >
>> > Signed-off-by: Joel Porquet <joel@porquet.org>
>> > ---
>> > Only tested by compilation.
>> > drivers/staging/iio/trigger/iio-trig-periodic-rtc.c | 3 ++-
>> > 1 file changed, 2 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/staging/iio/trigger/iio-trig-periodic-rtc.c
>b/drivers/staging/iio/trigger/iio-trig-periodic-rtc.c
>> > index 48a6afa..38ecb4b 100644
>> > --- a/drivers/staging/iio/trigger/iio-trig-periodic-rtc.c
>> > +++ b/drivers/staging/iio/trigger/iio-trig-periodic-rtc.c
>> > @@ -33,7 +33,8 @@ static int iio_trig_periodic_rtc_set_state(struct
>iio_trigger *trig, bool state)
>> > struct iio_prtc_trigger_info *trig_info =
>iio_trigger_get_drvdata(trig);
>> > if (trig_info->frequency == 0)
>> > return -EINVAL;
>> > - printk(KERN_INFO "trigger frequency is %d\n",
>trig_info->frequency);
>> > + dev_info(&trig_info->rtc->dev, "trigger frequency is %d\n",
>> > + trig_info->frequency);
>> The principle is good, but why make the error message us the
>underlying rtc device?
>> Going to lead to a rather unhelpful error message.
>>
>> Perhaps the iio_trigger structures device element would make more
>sense?
>> Might not be terribly informative, but will at least come from the
>right
>> subsystem.
>>
>> Also, I think we will be dropping this driver entirely at some point.
>> It was a dodgy hack that perhaps made sense at the time, but now a
>high
>> resolution timer is going to give better results.
>
>OK, thanks for the feedback!
>
>Does that still make sense that I resubmit another patch (using
>iio_trigger-dev instead)?
>Or should I just drop this patch altogether since you seem to say that
>patching this driver is not really worth it anyway?
Up to you. Nothing wrong with setting a good example even in code we aim ultimately
drop the code! I doubt we will do it for a cycle or two yet
>
>> > return rtc_irq_set_state(trig_info->rtc, &trig_info->task,
>state);
>> > }
>> >
>> >
>>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] staging: iio: fix coding style
2014-04-09 18:09 [PATCH] staging: iio: fix coding style Joel Porquet
2014-04-12 17:28 ` Jonathan Cameron
@ 2014-06-14 15:21 ` Jonathan Cameron
1 sibling, 0 replies; 10+ messages in thread
From: Jonathan Cameron @ 2014-06-14 15:21 UTC (permalink / raw)
To: Joel Porquet, gregkh, jg1.han; +Cc: linux-iio, devel, linux-kernel
On 09/04/14 19:09, Joel Porquet wrote:
> As suggested by checkpatch.pl, use dev_info() instead of
> printk(KERN_INFO ...) to print message.
>
> Signed-off-by: Joel Porquet <joel@porquet.org>
This line has been removed by another patch in the meantime.
> ---
> Only tested by compilation.
> drivers/staging/iio/trigger/iio-trig-periodic-rtc.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/iio/trigger/iio-trig-periodic-rtc.c b/drivers/staging/iio/trigger/iio-trig-periodic-rtc.c
> index 48a6afa..38ecb4b 100644
> --- a/drivers/staging/iio/trigger/iio-trig-periodic-rtc.c
> +++ b/drivers/staging/iio/trigger/iio-trig-periodic-rtc.c
> @@ -33,7 +33,8 @@ static int iio_trig_periodic_rtc_set_state(struct iio_trigger *trig, bool state)
> struct iio_prtc_trigger_info *trig_info = iio_trigger_get_drvdata(trig);
> if (trig_info->frequency == 0)
> return -EINVAL;
> - printk(KERN_INFO "trigger frequency is %d\n", trig_info->frequency);
> + dev_info(&trig_info->rtc->dev, "trigger frequency is %d\n",
> + trig_info->frequency);
> return rtc_irq_set_state(trig_info->rtc, &trig_info->task, state);
> }
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] staging: iio: fix coding style
@ 2014-06-12 9:42 Jimmy P
2014-06-12 16:10 ` Greg Kroah-Hartman
0 siblings, 1 reply; 10+ messages in thread
From: Jimmy P @ 2014-06-12 9:42 UTC (permalink / raw)
To: Jonathan Cameron, Greg Kroah-Hartman, Sachin Kamat
Cc: Jimmy P, linux-iio, devel, linux-kernel
This patch fixes coding style errors reported by checkpatch.pl for
lines that was over 80 chars long. The macro value shoud be put in () as
well.
Signed-off-by: Jimmy P <jimmyp11f155@gmail.com>
---
drivers/staging/iio/frequency/ad5930.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/iio/frequency/ad5930.c b/drivers/staging/iio/frequency/ad5930.c
index a4aeee6..95a3550 100644
--- a/drivers/staging/iio/frequency/ad5930.c
+++ b/drivers/staging/iio/frequency/ad5930.c
@@ -21,7 +21,7 @@
#define DRV_NAME "ad5930"
-#define value_mask (u16)0xf000
+#define value_mask ((u16)0xf000)
#define addr_shift 12
/* Register format: 4 bits addr + 12 bits value */
@@ -52,12 +52,15 @@ static ssize_t ad5930_set_parameter(struct device *dev,
config->control = (config->control & ~value_mask);
config->incnum = (config->control & ~value_mask) | (1 << addr_shift);
- config->frqdelt[0] = (config->control & ~value_mask) | (2 << addr_shift);
+ config->frqdelt[0] = (config->control & ~value_mask) |
+ (2 << addr_shift);
config->frqdelt[1] = (config->control & ~value_mask) | 3 << addr_shift;
config->incitvl = (config->control & ~value_mask) | 4 << addr_shift;
config->buritvl = (config->control & ~value_mask) | 8 << addr_shift;
- config->strtfrq[0] = (config->control & ~value_mask) | 0xc << addr_shift;
- config->strtfrq[1] = (config->control & ~value_mask) | 0xd << addr_shift;
+ config->strtfrq[0] = (config->control & ~value_mask) |
+ 0xc << addr_shift;
+ config->strtfrq[1] = (config->control & ~value_mask) |
+ 0xd << addr_shift;
xfer.len = len;
xfer.tx_buf = config;
--
2.0.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] staging: iio: fix coding style
2014-06-12 9:42 Jimmy P
@ 2014-06-12 16:10 ` Greg Kroah-Hartman
0 siblings, 0 replies; 10+ messages in thread
From: Greg Kroah-Hartman @ 2014-06-12 16:10 UTC (permalink / raw)
To: Jimmy P; +Cc: Jonathan Cameron, Sachin Kamat, linux-iio, devel, linux-kernel
On Thu, Jun 12, 2014 at 05:42:43PM +0800, Jimmy P wrote:
> This patch fixes coding style errors reported by checkpatch.pl for
> lines that was over 80 chars long. The macro value shoud be put in () as
> well.
>
> Signed-off-by: Jimmy P <jimmyp11f155@gmail.com>
We need a "real" or "full" name here for the signed-off-by: line, please
read Documentation/SubmittingPatches for the full details as to what
this line means and why we need that.
Also, this patch does multiple things, please break it up into different
patches, each only doing one specific thing.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] staging: iio: fix coding style
@ 2014-06-13 5:56 Jimmy Picard
2014-06-14 15:15 ` Jonathan Cameron
0 siblings, 1 reply; 10+ messages in thread
From: Jimmy Picard @ 2014-06-13 5:56 UTC (permalink / raw)
To: Jonathan Cameron, Greg Kroah-Hartman
Cc: Jimmy Picard, linux-iio, devel, linux-kernel
This patch fixes coding style reported by checkpatch.pl that missing
a blank line after declarations.
Signed-off-by: Jimmy Picard <jimmyp11f155@gmail.com>
---
drivers/staging/iio/iio_simple_dummy.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/staging/iio/iio_simple_dummy.c b/drivers/staging/iio/iio_simple_dummy.c
index fd334a0..bf78e6f 100644
--- a/drivers/staging/iio/iio_simple_dummy.c
+++ b/drivers/staging/iio/iio_simple_dummy.c
@@ -550,6 +550,7 @@ error_ret:
static __init int iio_dummy_init(void)
{
int i, ret;
+
if (instances > 10) {
instances = 1;
return -EINVAL;
@@ -577,6 +578,7 @@ module_init(iio_dummy_init);
static __exit void iio_dummy_exit(void)
{
int i;
+
for (i = 0; i < instances; i++)
iio_dummy_remove(i);
kfree(iio_dummy_devs);
--
2.0.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH] staging: iio: fix coding style
2014-06-13 5:56 Jimmy Picard
@ 2014-06-14 15:15 ` Jonathan Cameron
0 siblings, 0 replies; 10+ messages in thread
From: Jonathan Cameron @ 2014-06-14 15:15 UTC (permalink / raw)
To: Jimmy Picard, Greg Kroah-Hartman; +Cc: linux-iio, devel, linux-kernel
On 13/06/14 06:56, Jimmy Picard wrote:
> This patch fixes coding style reported by checkpatch.pl that missing
> a blank line after declarations.
>
> Signed-off-by: Jimmy Picard <jimmyp11f155@gmail.com>
Hmm. This is in the trivial enough to be marginally annoying
category of basically noise. Normally I'd only take this as
part of a larger series doing something more interesting, but
what the heck.
Applied to the togreg branch of iio.git
J
> ---
> drivers/staging/iio/iio_simple_dummy.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/staging/iio/iio_simple_dummy.c b/drivers/staging/iio/iio_simple_dummy.c
> index fd334a0..bf78e6f 100644
> --- a/drivers/staging/iio/iio_simple_dummy.c
> +++ b/drivers/staging/iio/iio_simple_dummy.c
> @@ -550,6 +550,7 @@ error_ret:
> static __init int iio_dummy_init(void)
> {
> int i, ret;
> +
> if (instances > 10) {
> instances = 1;
> return -EINVAL;
> @@ -577,6 +578,7 @@ module_init(iio_dummy_init);
> static __exit void iio_dummy_exit(void)
> {
> int i;
> +
> for (i = 0; i < instances; i++)
> iio_dummy_remove(i);
> kfree(iio_dummy_devs);
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2014-06-14 15:19 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-09 18:09 [PATCH] staging: iio: fix coding style Joel Porquet
2014-04-12 17:28 ` Jonathan Cameron
2014-04-14 9:40 ` Dan O'Donovan
2014-04-14 13:59 ` Joël Porquet
2014-04-14 16:10 ` Jonathan Cameron
2014-06-14 15:21 ` Jonathan Cameron
2014-06-12 9:42 Jimmy P
2014-06-12 16:10 ` Greg Kroah-Hartman
2014-06-13 5:56 Jimmy Picard
2014-06-14 15:15 ` Jonathan Cameron
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.