[PATCH] staging: iio: fix coding style

[PATCH] staging: iio: fix coding style

[Jimmy Picard]

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

Re: [PATCH] staging: iio: fix coding style

[Jonathan Cameron]

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);
>

Re: [PATCH] staging: iio: fix coding style

[Jonathan Cameron]

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);
>   }
>
>

Re: [PATCH] staging: iio: fix coding style

[Greg Kroah-Hartman]

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

[PATCH] staging: iio: fix coding style

[Jimmy P]

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

Re: [PATCH] staging: iio: fix coding style

[Jonathan Cameron]

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.

Re: [PATCH] staging: iio: fix coding style

[Joël Porquet]

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);
> >   }
> >
> >
> 

Re: [PATCH] staging: iio: fix coding style

[Dan O'Donovan]

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

Re: [PATCH] staging: iio: fix coding style

[Jonathan Cameron]

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);
>   }
>
>

[PATCH] staging: iio: fix coding style

[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