* [PATCH 0/2] Staging: iio: cdc: Use BIT macro and remove dev_info
@ 2015-09-25 17:23 Shraddha Barke
2015-09-25 17:23 ` [PATCH 1/2] Staging: iio: cdc: Prefer using the BIT macro Shraddha Barke
2015-09-25 17:23 ` [PATCH 2/2] Staging: iio: cdc: Remove unnecessary dev_info Shraddha Barke
0 siblings, 2 replies; 9+ messages in thread
From: Shraddha Barke @ 2015-09-25 17:23 UTC (permalink / raw
To: Lars-Peter Clausen, Jonathan Cameron
Cc: Hartmut Knaack, linux-iio, devel, Shraddha Barke
These patches use the BIT macro and remove dev_info
Shraddha Barke (2):
Staging: iio: cdc: Prefer using the BIT macro
Staging: iio: cdc: Remove unnecessary dev_info
drivers/staging/iio/cdc/ad7746.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
--
2.1.4
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] Staging: iio: cdc: Prefer using the BIT macro
2015-09-25 17:23 [PATCH 0/2] Staging: iio: cdc: Use BIT macro and remove dev_info Shraddha Barke
@ 2015-09-25 17:23 ` Shraddha Barke
2015-09-26 17:14 ` Lars-Peter Clausen
2015-09-25 17:23 ` [PATCH 2/2] Staging: iio: cdc: Remove unnecessary dev_info Shraddha Barke
1 sibling, 1 reply; 9+ messages in thread
From: Shraddha Barke @ 2015-09-25 17:23 UTC (permalink / raw
To: Lars-Peter Clausen, Jonathan Cameron
Cc: Hartmut Knaack, linux-iio, devel, Shraddha Barke
Replace bit shifting on 1 with the BIT(x) macro
Signed-off-by: Shraddha Barke <shraddha.6596@gmail.com>
---
drivers/staging/iio/cdc/ad7746.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
index 10fa372..55cc8bb 100644
--- a/drivers/staging/iio/cdc/ad7746.c
+++ b/drivers/staging/iio/cdc/ad7746.c
@@ -68,12 +68,12 @@
#define AD7746_VTSETUP_VTCHOP (1 << 0)
/* Excitation Setup Register Bit Designations (AD7746_REG_EXC_SETUP) */
-#define AD7746_EXCSETUP_CLKCTRL (1 << 7)
-#define AD7746_EXCSETUP_EXCON (1 << 6)
-#define AD7746_EXCSETUP_EXCB (1 << 5)
-#define AD7746_EXCSETUP_NEXCB (1 << 4)
-#define AD7746_EXCSETUP_EXCA (1 << 3)
-#define AD7746_EXCSETUP_NEXCA (1 << 2)
+#define AD7746_EXCSETUP_CLKCTRL BIT(7)
+#define AD7746_EXCSETUP_EXCON BIT(6)
+#define AD7746_EXCSETUP_EXCB BIT(5)
+#define AD7746_EXCSETUP_NEXCB BIT(4)
+#define AD7746_EXCSETUP_EXCA BIT(3)
+#define AD7746_EXCSETUP_NEXCA BIT(2)
#define AD7746_EXCSETUP_EXCLVL(x) (((x) & 0x3) << 0)
/* Config Register Bit Designations (AD7746_REG_CFG) */
--
2.1.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] Staging: iio: cdc: Remove unnecessary dev_info
2015-09-25 17:23 [PATCH 0/2] Staging: iio: cdc: Use BIT macro and remove dev_info Shraddha Barke
2015-09-25 17:23 ` [PATCH 1/2] Staging: iio: cdc: Prefer using the BIT macro Shraddha Barke
@ 2015-09-25 17:23 ` Shraddha Barke
2015-09-26 17:14 ` Lars-Peter Clausen
1 sibling, 1 reply; 9+ messages in thread
From: Shraddha Barke @ 2015-09-25 17:23 UTC (permalink / raw
To: Lars-Peter Clausen, Jonathan Cameron
Cc: Hartmut Knaack, linux-iio, devel, Shraddha Barke
Remove dev_info as the information can be obtained by other means
Signed-off-by: Shraddha Barke <shraddha.6596@gmail.com>
---
drivers/staging/iio/cdc/ad7746.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
index 55cc8bb..ca7d33f 100644
--- a/drivers/staging/iio/cdc/ad7746.c
+++ b/drivers/staging/iio/cdc/ad7746.c
@@ -753,8 +753,6 @@ static int ad7746_probe(struct i2c_client *client,
if (ret)
return ret;
- dev_info(&client->dev, "%s capacitive sensor registered\n", id->name);
-
return 0;
}
--
2.1.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] Staging: iio: cdc: Prefer using the BIT macro
2015-09-25 17:23 ` [PATCH 1/2] Staging: iio: cdc: Prefer using the BIT macro Shraddha Barke
@ 2015-09-26 17:14 ` Lars-Peter Clausen
2015-09-27 13:46 ` Jonathan Cameron
0 siblings, 1 reply; 9+ messages in thread
From: Lars-Peter Clausen @ 2015-09-26 17:14 UTC (permalink / raw
To: Shraddha Barke, Jonathan Cameron; +Cc: Hartmut Knaack, linux-iio, devel
On 09/25/2015 07:23 PM, Shraddha Barke wrote:
> Replace bit shifting on 1 with the BIT(x) macro
>
> Signed-off-by: Shraddha Barke <shraddha.6596@gmail.com>
Acked-by: Lars-Peter Clausen <lars@metafoo.de>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] Staging: iio: cdc: Remove unnecessary dev_info
2015-09-25 17:23 ` [PATCH 2/2] Staging: iio: cdc: Remove unnecessary dev_info Shraddha Barke
@ 2015-09-26 17:14 ` Lars-Peter Clausen
2015-09-27 13:47 ` Jonathan Cameron
0 siblings, 1 reply; 9+ messages in thread
From: Lars-Peter Clausen @ 2015-09-26 17:14 UTC (permalink / raw
To: Shraddha Barke, Jonathan Cameron; +Cc: Hartmut Knaack, linux-iio, devel
On 09/25/2015 07:23 PM, Shraddha Barke wrote:
> Remove dev_info as the information can be obtained by other means
>
> Signed-off-by: Shraddha Barke <shraddha.6596@gmail.com>
Acked-by: Lars-Peter Clausen <lars@metafoo.de>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] Staging: iio: cdc: Prefer using the BIT macro
2015-09-26 17:14 ` Lars-Peter Clausen
@ 2015-09-27 13:46 ` Jonathan Cameron
2015-09-27 19:39 ` Shraddha Barke
0 siblings, 1 reply; 9+ messages in thread
From: Jonathan Cameron @ 2015-09-27 13:46 UTC (permalink / raw
To: Lars-Peter Clausen, Shraddha Barke; +Cc: Hartmut Knaack, linux-iio, devel
On 26/09/15 18:14, Lars-Peter Clausen wrote:
> On 09/25/2015 07:23 PM, Shraddha Barke wrote:
>> Replace bit shifting on 1 with the BIT(x) macro
>>
>> Signed-off-by: Shraddha Barke <shraddha.6596@gmail.com>
>
> Acked-by: Lars-Peter Clausen <lars@metafoo.de>
>
This is a good little patch, so applied to the togreg branch
of iio.git. This will get initially pushed out as testing
and the branch will get rebased onto Greg KH's staging-next
branch sometime soon.
However, I think there are some other places the BIT macro
can be correctly used in this driver if you are willing to take
it further.
Note that you can only do these by checking the datasheet to be
sure we aren't writing part of a large field in the register!
In the status register the bottom for bits are single bit fields.
#define AD7746_STATUS_EXCERR (1 << 3)
#define AD7746_STATUS_RDY (1 << 2)
#define AD7746_STATUS_RDYVT (1 << 1)
#define AD7746_STATUS_RDYCAP (1 << 0)
As are the cap setup register bits
#define AD7746_CAPSETUP_CAPEN (1 << 7)
#define AD7746_CAPSETUP_CIN2 (1 << 6) /* AD7746 only */
#define AD7746_CAPSETUP_CAPDIFF (1 << 5)
#define AD7746_CAPSETUP_CACHOP (1 << 0)
Some of the vt setup register are also single bits but as that register
is a more complex mess and some other bits are hard to define macros
for perhaps leave it alone. It's another one arguing in favour of
a ZERO(bit) macro but I fear persuading people of that will be an uphill
struggle :(
Anyhow the two registers mentioned above are a clear cut as the one you've
done so perhaps tidy those up as well?
Thanks,
Jonathan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] Staging: iio: cdc: Remove unnecessary dev_info
2015-09-26 17:14 ` Lars-Peter Clausen
@ 2015-09-27 13:47 ` Jonathan Cameron
0 siblings, 0 replies; 9+ messages in thread
From: Jonathan Cameron @ 2015-09-27 13:47 UTC (permalink / raw
To: Lars-Peter Clausen, Shraddha Barke; +Cc: Hartmut Knaack, linux-iio, devel
On 26/09/15 18:14, Lars-Peter Clausen wrote:
> On 09/25/2015 07:23 PM, Shraddha Barke wrote:
>> Remove dev_info as the information can be obtained by other means
>>
>> Signed-off-by: Shraddha Barke <shraddha.6596@gmail.com>
>
> Acked-by: Lars-Peter Clausen <lars@metafoo.de>
Applied, thanks Shraddha,
Keep up the good work, these drivers have been in staging way too long!
Jonathan
>
> --
> 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] 9+ messages in thread
* Re: [PATCH 1/2] Staging: iio: cdc: Prefer using the BIT macro
2015-09-27 13:46 ` Jonathan Cameron
@ 2015-09-27 19:39 ` Shraddha Barke
2015-09-29 17:36 ` Jonathan Cameron
0 siblings, 1 reply; 9+ messages in thread
From: Shraddha Barke @ 2015-09-27 19:39 UTC (permalink / raw
To: Jonathan Cameron
Cc: Lars-Peter Clausen, Shraddha Barke, Hartmut Knaack, linux-iio,
devel
On Sun, 27 Sep 2015, Jonathan Cameron wrote:
> On 26/09/15 18:14, Lars-Peter Clausen wrote:
>> On 09/25/2015 07:23 PM, Shraddha Barke wrote:
>>> Replace bit shifting on 1 with the BIT(x) macro
>>>
>>> Signed-off-by: Shraddha Barke <shraddha.6596@gmail.com>
>>
>> Acked-by: Lars-Peter Clausen <lars@metafoo.de>
>>
> This is a good little patch, so applied to the togreg branch
> of iio.git. This will get initially pushed out as testing
> and the branch will get rebased onto Greg KH's staging-next
> branch sometime soon.
>
> However, I think there are some other places the BIT macro
> can be correctly used in this driver if you are willing to take
> it further.
>
> Note that you can only do these by checking the datasheet to be
> sure we aren't writing part of a large field in the register!
>
> In the status register the bottom for bits are single bit fields.
>
>
> #define AD7746_STATUS_EXCERR (1 << 3)
> #define AD7746_STATUS_RDY (1 << 2)
> #define AD7746_STATUS_RDYVT (1 << 1)
> #define AD7746_STATUS_RDYCAP (1 << 0)
>
> As are the cap setup register bits
>
> #define AD7746_CAPSETUP_CAPEN (1 << 7)
> #define AD7746_CAPSETUP_CIN2 (1 << 6) /* AD7746 only */
> #define AD7746_CAPSETUP_CAPDIFF (1 << 5)
> #define AD7746_CAPSETUP_CACHOP (1 << 0)
>
> Some of the vt setup register are also single bits but as that register
> is a more complex mess and some other bits are hard to define macros
> for perhaps leave it alone. It's another one arguing in favour of
> a ZERO(bit) macro but I fear persuading people of that will be an uphill
> struggle :(
>
> Anyhow the two registers mentioned above are a clear cut as the one you've
> done so perhaps tidy those up as well?
>
Yes sure. But after checking I found out that AD7746_STATUS_EXCERR macro
is not used at all. I'm sure there are other such macros in this driver.
Wouldn't it be better to get rid of them?
Thanks,
Shraddha
> Thanks,
>
> Jonathan
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] Staging: iio: cdc: Prefer using the BIT macro
2015-09-27 19:39 ` Shraddha Barke
@ 2015-09-29 17:36 ` Jonathan Cameron
0 siblings, 0 replies; 9+ messages in thread
From: Jonathan Cameron @ 2015-09-29 17:36 UTC (permalink / raw
To: Shraddha Barke; +Cc: Lars-Peter Clausen, Hartmut Knaack, linux-iio, devel
On 27/09/15 20:39, Shraddha Barke wrote:
>
>
> On Sun, 27 Sep 2015, Jonathan Cameron wrote:
>
>> On 26/09/15 18:14, Lars-Peter Clausen wrote:
>>> On 09/25/2015 07:23 PM, Shraddha Barke wrote:
>>>> Replace bit shifting on 1 with the BIT(x) macro
>>>>
>>>> Signed-off-by: Shraddha Barke <shraddha.6596@gmail.com>
>>>
>>> Acked-by: Lars-Peter Clausen <lars@metafoo.de>
>>>
>> This is a good little patch, so applied to the togreg branch
>> of iio.git. This will get initially pushed out as testing
>> and the branch will get rebased onto Greg KH's staging-next
>> branch sometime soon.
>>
>> However, I think there are some other places the BIT macro
>> can be correctly used in this driver if you are willing to take
>> it further.
>>
>> Note that you can only do these by checking the datasheet to be
>> sure we aren't writing part of a large field in the register!
>>
>> In the status register the bottom for bits are single bit fields.
>>
>>
>> #define AD7746_STATUS_EXCERR (1 << 3)
>> #define AD7746_STATUS_RDY (1 << 2)
>> #define AD7746_STATUS_RDYVT (1 << 1)
>> #define AD7746_STATUS_RDYCAP (1 << 0)
>>
>> As are the cap setup register bits
>>
>> #define AD7746_CAPSETUP_CAPEN (1 << 7)
>> #define AD7746_CAPSETUP_CIN2 (1 << 6) /* AD7746 only */
>> #define AD7746_CAPSETUP_CAPDIFF (1 << 5)
>> #define AD7746_CAPSETUP_CACHOP (1 << 0)
>>
>> Some of the vt setup register are also single bits but as that register
>> is a more complex mess and some other bits are hard to define macros
>> for perhaps leave it alone. It's another one arguing in favour of
>> a ZERO(bit) macro but I fear persuading people of that will be an uphill
>> struggle :(
>>
>> Anyhow the two registers mentioned above are a clear cut as the one you've
>> done so perhaps tidy those up as well?
>>
> Yes sure. But after checking I found out that AD7746_STATUS_EXCERR
> macro is not used at all. I'm sure there are other such macros in
> this driver. Wouldn't it be better to get rid of them?
In general yes, but always worth taking a quick look at the datasheet
to see if they are something the driver SHOULD be using.
Sometimes it is argued that these act as documentation, but that in
my view is only really valid if the datasheet isn't publicly available.
J
> Thanks,
>
> Shraddha
>
>> Thanks,
>>
>> Jonathan
>>
>>
> --
> 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] 9+ messages in thread
end of thread, other threads:[~2015-09-29 17:36 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-25 17:23 [PATCH 0/2] Staging: iio: cdc: Use BIT macro and remove dev_info Shraddha Barke
2015-09-25 17:23 ` [PATCH 1/2] Staging: iio: cdc: Prefer using the BIT macro Shraddha Barke
2015-09-26 17:14 ` Lars-Peter Clausen
2015-09-27 13:46 ` Jonathan Cameron
2015-09-27 19:39 ` Shraddha Barke
2015-09-29 17:36 ` Jonathan Cameron
2015-09-25 17:23 ` [PATCH 2/2] Staging: iio: cdc: Remove unnecessary dev_info Shraddha Barke
2015-09-26 17:14 ` Lars-Peter Clausen
2015-09-27 13:47 ` 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.