All of lore.kernel.org
 help / color / mirror / Atom feed
* Questions related to some drivers
@ 2018-10-17 19:00 Giuliano Belinassi
  2018-10-18  6:53 ` Ardelean, Alexandru
  0 siblings, 1 reply; 14+ messages in thread
From: Giuliano Belinassi @ 2018-10-17 19:00 UTC (permalink / raw)
  To: linux-iio; +Cc: kernel-usp, renatogeh

Hello,

We are a student group and we want to contribute to the linux kernel. We
are thinking in put effort to move a driver from staging to the main tree.
We looked into three drivers:
  
  * adc/ad7780.c
  * adc/ad7816.c
  * impedance-analyzer/ad5933.c

Are these drivers still being worked on? The last commit referencing these
drivers were on march 2018, but they are still in staging since 2010.

We already know that we have to convert from the old api to the new one. It is
OK for us to work on this? Is there something else we can do?

Thank you

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Questions related to some drivers
  2018-10-17 19:00 Questions related to some drivers Giuliano Belinassi
@ 2018-10-18  6:53 ` Ardelean, Alexandru
  2018-10-18 11:41   ` Giuliano Belinassi
  0 siblings, 1 reply; 14+ messages in thread
From: Ardelean, Alexandru @ 2018-10-18  6:53 UTC (permalink / raw)
  To: giuliano.belinassi, linux-iio; +Cc: renatogeh, kernel-usp

SGV5LA0KDQpUaGFua3MgZm9yIHRoZSBub3RpZmljYXRpb24uDQoNClllcywgYWxsIHRoZXNlIDMg
ZHJpdmVycyBzaG91bGQgYmUgbW92ZWQgb3V0IG9mIHN0YWdpbmcuDQoNCldlIHdlcmUgcGxhbm5p
bmcgdG8gc3RhcnQgd29ya2luZyBvbiB0aGVtICh0byBtb3ZlIHRoZW0gb3V0IG9mIHN0YWdpbmcp
LA0KYnV0IHByaW9yaXRpZXMgYXJlIHNoaWZ0aW5nIGZyb20gb25lIHdlZWsgdG8gdGhlIG90aGVy
Lg0KU28sIGlmIHlvdSBndXlzIGhhdmUgdGhlIHRpbWUgYW5kIGFyZSB3aWxsaW5nIHRvIGRvIGl0
LCBwbGVhc2UgZ28gYWhlYWQsDQphbmQgYWRkIG1lLCBtaWNoYWVsLmhlbm5lcmljaEBhbmFsb2cu
Y29tIGFuZCBzdGVmYW4ucG9wYUBhbmFsb2cuY29tIHRvIHRoZQ0KQ0Mgd2hlbiBzZW5kaW5nIHBh
dGNoZXMsIGFuZCB3ZSB3aWxsIGFsc28gdGFrZSBhIGxvb2sgb3ZlciB5b3VyIHBhdGNoZXMuDQoN
CklmIHlvdSBoYXZlbid0IHRha2VuIGEgbG9vayBhbHJlYWR5LCBmZWVsIGZyZWUgdG8gYWxzbyB0
YWtlIGEgbG9vayBhdCBvdXINCndpa2k6DQpodHRwczovL3dpa2kuYW5hbG9nLmNvbS9yZXNvdXJj
ZXMvdG9vbHMtc29mdHdhcmUvbGludXgtZHJpdmVycy1hbGwNCkl0J3MgbW9yZSB0ZWNobmljYWwg
Zm9yIHRoZSBTVyBzaWRlOyBpdCBtaWdodCBoZWxwIHdpdGggc29tZSBnZW5lcmFsIGluZm8uDQpB
bmQgdGhlIGRhdGFzaGVldHMgc2hvdWxkIGJlIGF2YWlsYWJsZSBvbiB0aGUgYW5hbG9nLmNvbS88
cHJvZHVjdF9pZD4NCnBhZ2VzLg0KDQoNClRoYW5rcw0KQWxleA0KDQoNCk9uIFdlZCwgMjAxOC0x
MC0xNyBhdCAxNjowMCAtMDMwMCwgR2l1bGlhbm8gQmVsaW5hc3NpIHdyb3RlOg0KPiBIZWxsbywN
Cj4gDQo+IFdlIGFyZSBhIHN0dWRlbnQgZ3JvdXAgYW5kIHdlIHdhbnQgdG8gY29udHJpYnV0ZSB0
byB0aGUgbGludXgga2VybmVsLiBXZQ0KPiBhcmUgdGhpbmtpbmcgaW4gcHV0IGVmZm9ydCB0byBt
b3ZlIGEgZHJpdmVyIGZyb20gc3RhZ2luZyB0byB0aGUgbWFpbg0KPiB0cmVlLg0KPiBXZSBsb29r
ZWQgaW50byB0aHJlZSBkcml2ZXJzOg0KPiAgIA0KPiAgICogYWRjL2FkNzc4MC5jDQo+ICAgKiBh
ZGMvYWQ3ODE2LmMNCj4gICAqIGltcGVkYW5jZS1hbmFseXplci9hZDU5MzMuYw0KPiANCj4gQXJl
IHRoZXNlIGRyaXZlcnMgc3RpbGwgYmVpbmcgd29ya2VkIG9uPyBUaGUgbGFzdCBjb21taXQgcmVm
ZXJlbmNpbmcNCj4gdGhlc2UNCj4gZHJpdmVycyB3ZXJlIG9uIG1hcmNoIDIwMTgsIGJ1dCB0aGV5
IGFyZSBzdGlsbCBpbiBzdGFnaW5nIHNpbmNlIDIwMTAuDQo+IA0KPiBXZSBhbHJlYWR5IGtub3cg
dGhhdCB3ZSBoYXZlIHRvIGNvbnZlcnQgZnJvbSB0aGUgb2xkIGFwaSB0byB0aGUgbmV3IG9uZS4N
Cj4gSXQgaXMNCj4gT0sgZm9yIHVzIHRvIHdvcmsgb24gdGhpcz8gSXMgdGhlcmUgc29tZXRoaW5n
IGVsc2Ugd2UgY2FuIGRvPw0KPiANCj4gVGhhbmsgeW91DQo+IA0K

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Questions related to some drivers
  2018-10-18  6:53 ` Ardelean, Alexandru
@ 2018-10-18 11:41   ` Giuliano Belinassi
  2018-10-22 21:56     ` Giuliano Augusto Faulin Belinassi
  0 siblings, 1 reply; 14+ messages in thread
From: Giuliano Belinassi @ 2018-10-18 11:41 UTC (permalink / raw)
  To: Ardelean, Alexandru; +Cc: giuliano.belinassi, linux-iio, renatogeh, kernel-usp

On 10/18, Ardelean, Alexandru wrote:

Thank you for the reply :-). We will put effort into at least one driver to
remove it from staging.

> Hey,
> 
> Thanks for the notification.
> 
> Yes, all these 3 drivers should be moved out of staging.
> 
> We were planning to start working on them (to move them out of staging),
> but priorities are shifting from one week to the other.
> So, if you guys have the time and are willing to do it, please go ahead,
> and add me, michael.hennerich@analog.com and stefan.popa@analog.com to the
> CC when sending patches, and we will also take a look over your patches.
> 
> If you haven't taken a look already, feel free to also take a look at our
> wiki:
> https://wiki.analog.com/resources/tools-software/linux-drivers-all
> It's more technical for the SW side; it might help with some general info.
> And the datasheets should be available on the analog.com/<product_id>
> pages.
> 
> 
> Thanks
> Alex
> 
> 
> On Wed, 2018-10-17 at 16:00 -0300, Giuliano Belinassi wrote:
> > Hello,
> > 
> > We are a student group and we want to contribute to the linux kernel. We
> > are thinking in put effort to move a driver from staging to the main
> > tree.
> > We looked into three drivers:
> >   
> >   * adc/ad7780.c
> >   * adc/ad7816.c
> >   * impedance-analyzer/ad5933.c
> > 
> > Are these drivers still being worked on? The last commit referencing
> > these
> > drivers were on march 2018, but they are still in staging since 2010.
> > 
> > We already know that we have to convert from the old api to the new one.
> > It is
> > OK for us to work on this? Is there something else we can do?
> > 
> > Thank you
> > 
> 
> -- 
> You received this message because you are subscribed to the Google Groups "Kernel USP" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kernel-usp+unsubscribe@googlegroups.com.
> To post to this group, send email to kernel-usp@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kernel-usp/d0fc5a236a82d03fdbe4d05eb27ff3bd3ac36958.camel%40analog.com.
> For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Questions related to some drivers
  2018-10-18 11:41   ` Giuliano Belinassi
@ 2018-10-22 21:56     ` Giuliano Augusto Faulin Belinassi
  2018-10-23  9:51       ` Jonathan Cameron
  0 siblings, 1 reply; 14+ messages in thread
From: Giuliano Augusto Faulin Belinassi @ 2018-10-22 21:56 UTC (permalink / raw)
  To: giuliano.belinassi; +Cc: alexandru.Ardelean, linux-iio, renatogeh, kernel-usp

Hello,
I have some questions about the ad7780 driver.

  * What is the val2 In the line 96 (*val2 = chan->scan_type.realbits
- 1;)? How it is used? Is it related to the 24-bits ADC precision?
  * Why there is a subtraction in the line 99 ( *val -= (1 <<
(chan->scan_type.realbits - 1)); )? Is the *val initialized with 0? Is
that related to the formula described in the DATA OUTPUT CODING, at
page 12?
https://www.analog.com/media/en/technical-documentation/data-sheets/ad7780.pdf

With regards


On Thu, Oct 18, 2018 at 8:41 AM Giuliano Belinassi
<giuliano.belinassi@gmail.com> wrote:
>
> On 10/18, Ardelean, Alexandru wrote:
>
> Thank you for the reply :-). We will put effort into at least one driver to
> remove it from staging.
>
> > Hey,
> >
> > Thanks for the notification.
> >
> > Yes, all these 3 drivers should be moved out of staging.
> >
> > We were planning to start working on them (to move them out of staging),
> > but priorities are shifting from one week to the other.
> > So, if you guys have the time and are willing to do it, please go ahead,
> > and add me, michael.hennerich@analog.com and stefan.popa@analog.com to the
> > CC when sending patches, and we will also take a look over your patches.
> >
> > If you haven't taken a look already, feel free to also take a look at our
> > wiki:
> > https://wiki.analog.com/resources/tools-software/linux-drivers-all
> > It's more technical for the SW side; it might help with some general info.
> > And the datasheets should be available on the analog.com/<product_id>
> > pages.
> >
> >
> > Thanks
> > Alex
> >
> >
> > On Wed, 2018-10-17 at 16:00 -0300, Giuliano Belinassi wrote:
> > > Hello,
> > >
> > > We are a student group and we want to contribute to the linux kernel. We
> > > are thinking in put effort to move a driver from staging to the main
> > > tree.
> > > We looked into three drivers:
> > >
> > >   * adc/ad7780.c
> > >   * adc/ad7816.c
> > >   * impedance-analyzer/ad5933.c
> > >
> > > Are these drivers still being worked on? The last commit referencing
> > > these
> > > drivers were on march 2018, but they are still in staging since 2010.
> > >
> > > We already know that we have to convert from the old api to the new one.
> > > It is
> > > OK for us to work on this? Is there something else we can do?
> > >
> > > Thank you
> > >
> >
> > --
> > You received this message because you are subscribed to the Google Groups "Kernel USP" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to kernel-usp+unsubscribe@googlegroups.com.
> > To post to this group, send email to kernel-usp@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/kernel-usp/d0fc5a236a82d03fdbe4d05eb27ff3bd3ac36958.camel%40analog.com.
> > For more options, visit https://groups.google.com/d/optout.
>
> --
> You received this message because you are subscribed to the Google Groups "Kernel USP" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kernel-usp+unsubscribe@googlegroups.com.
> To post to this group, send email to kernel-usp@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kernel-usp/20181018114145.amqff57holmecxpo%40smtp.gmail.com.
> For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Questions related to some drivers
  2018-10-22 21:56     ` Giuliano Augusto Faulin Belinassi
@ 2018-10-23  9:51       ` Jonathan Cameron
  2018-11-05 18:46         ` Giuliano Augusto Faulin Belinassi
  0 siblings, 1 reply; 14+ messages in thread
From: Jonathan Cameron @ 2018-10-23  9:51 UTC (permalink / raw)
  To: Giuliano Augusto Faulin Belinassi
  Cc: giuliano.belinassi, alexandru.Ardelean, linux-iio, renatogeh, kernel-usp

On Mon, 22 Oct 2018 18:56:16 -0300
Giuliano Augusto Faulin Belinassi <giuliano.belinassi@usp.br> wrote:

> Hello,
> I have some questions about the ad7780 driver.
> 
>   * What is the val2 In the line 96 (*val2 = chan->scan_type.realbits
> - 1;)? How it is used? Is it related to the 24-bits ADC precision?

To understand that follow through what the code does when the type of a
read_raw is IIO_VAL_FRACTIONAL_LOG2; 

>   * Why there is a subtraction in the line 99 ( *val -= (1 <<
> (chan->scan_type.realbits - 1)); )? Is the *val initialized with 0? Is
> that related to the formula described in the DATA OUTPUT CODING, at
> page 12?
> https://www.analog.com/media/en/technical-documentation/data-sheets/ad7780.pdf

Yeah, that -= is definitely wrong.  Should just be an assignment to the
negative value.  This is telling userspace that the raw value should be
offset by half of it's maximum range before applying the scale to get
a reading in the IIO base units (here mV IIRC).

> 
> With regards
> 
> 
> On Thu, Oct 18, 2018 at 8:41 AM Giuliano Belinassi
> <giuliano.belinassi@gmail.com> wrote:
> >
> > On 10/18, Ardelean, Alexandru wrote:
> >
> > Thank you for the reply :-). We will put effort into at least one driver to
> > remove it from staging.
> >  
> > > Hey,
> > >
> > > Thanks for the notification.
> > >
> > > Yes, all these 3 drivers should be moved out of staging.
> > >
> > > We were planning to start working on them (to move them out of staging),
> > > but priorities are shifting from one week to the other.
> > > So, if you guys have the time and are willing to do it, please go ahead,
> > > and add me, michael.hennerich@analog.com and stefan.popa@analog.com to the
> > > CC when sending patches, and we will also take a look over your patches.
> > >
> > > If you haven't taken a look already, feel free to also take a look at our
> > > wiki:
> > > https://wiki.analog.com/resources/tools-software/linux-drivers-all
> > > It's more technical for the SW side; it might help with some general info.
> > > And the datasheets should be available on the analog.com/<product_id>
> > > pages.
> > >
> > >
> > > Thanks
> > > Alex
> > >
> > >
> > > On Wed, 2018-10-17 at 16:00 -0300, Giuliano Belinassi wrote:  
> > > > Hello,
> > > >
> > > > We are a student group and we want to contribute to the linux kernel. We
> > > > are thinking in put effort to move a driver from staging to the main
> > > > tree.
> > > > We looked into three drivers:
> > > >
> > > >   * adc/ad7780.c
> > > >   * adc/ad7816.c
> > > >   * impedance-analyzer/ad5933.c
> > > >
> > > > Are these drivers still being worked on? The last commit referencing
> > > > these
> > > > drivers were on march 2018, but they are still in staging since 2010.
> > > >
> > > > We already know that we have to convert from the old api to the new one.
> > > > It is
> > > > OK for us to work on this? Is there something else we can do?
> > > >
> > > > Thank you
> > > >  
> > >
> > > --
> > > You received this message because you are subscribed to the Google Groups "Kernel USP" group.
> > > To unsubscribe from this group and stop receiving emails from it, send an email to kernel-usp+unsubscribe@googlegroups.com.
> > > To post to this group, send email to kernel-usp@googlegroups.com.
> > > To view this discussion on the web visit https://groups.google.com/d/msgid/kernel-usp/d0fc5a236a82d03fdbe4d05eb27ff3bd3ac36958.camel%40analog.com.
> > > For more options, visit https://groups.google.com/d/optout.  
> >
> > --
> > You received this message because you are subscribed to the Google Groups "Kernel USP" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to kernel-usp+unsubscribe@googlegroups.com.
> > To post to this group, send email to kernel-usp@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/kernel-usp/20181018114145.amqff57holmecxpo%40smtp.gmail.com.
> > For more options, visit https://groups.google.com/d/optout.  

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Questions related to some drivers
  2018-10-23  9:51       ` Jonathan Cameron
@ 2018-11-05 18:46         ` Giuliano Augusto Faulin Belinassi
  2018-11-06 13:23           ` Ardelean, Alexandru
  0 siblings, 1 reply; 14+ messages in thread
From: Giuliano Augusto Faulin Belinassi @ 2018-11-05 18:46 UTC (permalink / raw)
  To: jic23
  Cc: giuliano.belinassi, alexandru.Ardelean, linux-iio, Renato Geh,
	kernel-usp

Hello,
Could you help us figure out what must be added/changed into the
staging/iio/adc/ad7780.c to remove it from staiging?

For instance, should we add an id field in ad7780_state struct that
represents ID1 and ID0 from the status bits (page 13 from datasheet)?
Or check the Status pattern bits PAT1, PAT0 for errors? Or something
else we can work on?

Thank you.
On Tue, Oct 23, 2018 at 6:51 AM Jonathan Cameron
<jic23@jic23.retrosnub.co.uk> wrote:
>
> On Mon, 22 Oct 2018 18:56:16 -0300
> Giuliano Augusto Faulin Belinassi <giuliano.belinassi@usp.br> wrote:
>
> > Hello,
> > I have some questions about the ad7780 driver.
> >
> >   * What is the val2 In the line 96 (*val2 = chan->scan_type.realbits
> > - 1;)? How it is used? Is it related to the 24-bits ADC precision?
>
> To understand that follow through what the code does when the type of a
> read_raw is IIO_VAL_FRACTIONAL_LOG2;
>
> >   * Why there is a subtraction in the line 99 ( *val -= (1 <<
> > (chan->scan_type.realbits - 1)); )? Is the *val initialized with 0? Is
> > that related to the formula described in the DATA OUTPUT CODING, at
> > page 12?
> > https://www.analog.com/media/en/technical-documentation/data-sheets/ad7780.pdf
>
> Yeah, that -= is definitely wrong.  Should just be an assignment to the
> negative value.  This is telling userspace that the raw value should be
> offset by half of it's maximum range before applying the scale to get
> a reading in the IIO base units (here mV IIRC).
>
> >
> > With regards
> >
> >
> > On Thu, Oct 18, 2018 at 8:41 AM Giuliano Belinassi
> > <giuliano.belinassi@gmail.com> wrote:
> > >
> > > On 10/18, Ardelean, Alexandru wrote:
> > >
> > > Thank you for the reply :-). We will put effort into at least one driver to
> > > remove it from staging.
> > >
> > > > Hey,
> > > >
> > > > Thanks for the notification.
> > > >
> > > > Yes, all these 3 drivers should be moved out of staging.
> > > >
> > > > We were planning to start working on them (to move them out of staging),
> > > > but priorities are shifting from one week to the other.
> > > > So, if you guys have the time and are willing to do it, please go ahead,
> > > > and add me, michael.hennerich@analog.com and stefan.popa@analog.com to the
> > > > CC when sending patches, and we will also take a look over your patches.
> > > >
> > > > If you haven't taken a look already, feel free to also take a look at our
> > > > wiki:
> > > > https://wiki.analog.com/resources/tools-software/linux-drivers-all
> > > > It's more technical for the SW side; it might help with some general info.
> > > > And the datasheets should be available on the analog.com/<product_id>
> > > > pages.
> > > >
> > > >
> > > > Thanks
> > > > Alex
> > > >
> > > >
> > > > On Wed, 2018-10-17 at 16:00 -0300, Giuliano Belinassi wrote:
> > > > > Hello,
> > > > >
> > > > > We are a student group and we want to contribute to the linux kernel. We
> > > > > are thinking in put effort to move a driver from staging to the main
> > > > > tree.
> > > > > We looked into three drivers:
> > > > >
> > > > >   * adc/ad7780.c
> > > > >   * adc/ad7816.c
> > > > >   * impedance-analyzer/ad5933.c
> > > > >
> > > > > Are these drivers still being worked on? The last commit referencing
> > > > > these
> > > > > drivers were on march 2018, but they are still in staging since 2010.
> > > > >
> > > > > We already know that we have to convert from the old api to the new one.
> > > > > It is
> > > > > OK for us to work on this? Is there something else we can do?
> > > > >
> > > > > Thank you
> > > > >
> > > >
> > > > --
> > > > You received this message because you are subscribed to the Google Groups "Kernel USP" group.
> > > > To unsubscribe from this group and stop receiving emails from it, send an email to kernel-usp+unsubscribe@googlegroups.com.
> > > > To post to this group, send email to kernel-usp@googlegroups.com.
> > > > To view this discussion on the web visit https://groups.google.com/d/msgid/kernel-usp/d0fc5a236a82d03fdbe4d05eb27ff3bd3ac36958.camel%40analog.com.
> > > > For more options, visit https://groups.google.com/d/optout.
> > >
> > > --
> > > You received this message because you are subscribed to the Google Groups "Kernel USP" group.
> > > To unsubscribe from this group and stop receiving emails from it, send an email to kernel-usp+unsubscribe@googlegroups.com.
> > > To post to this group, send email to kernel-usp@googlegroups.com.
> > > To view this discussion on the web visit https://groups.google.com/d/msgid/kernel-usp/20181018114145.amqff57holmecxpo%40smtp.gmail.com.
> > > For more options, visit https://groups.google.com/d/optout.
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Questions related to some drivers
  2018-11-05 18:46         ` Giuliano Augusto Faulin Belinassi
@ 2018-11-06 13:23           ` Ardelean, Alexandru
  2018-11-07 18:57             ` Renato Lui Geh
  0 siblings, 1 reply; 14+ messages in thread
From: Ardelean, Alexandru @ 2018-11-06 13:23 UTC (permalink / raw)
  To: giuliano.belinassi, jic23
  Cc: giuliano.belinassi, renatogeh, linux-iio, kernel-usp

T24gTW9uLCAyMDE4LTExLTA1IGF0IDE2OjQ2IC0wMjAwLCBHaXVsaWFubyBBdWd1c3RvIEZhdWxp
biBCZWxpbmFzc2kgd3JvdGU6DQo+IEhlbGxvLA0KPiBDb3VsZCB5b3UgaGVscCB1cyBmaWd1cmUg
b3V0IHdoYXQgbXVzdCBiZSBhZGRlZC9jaGFuZ2VkIGludG8gdGhlDQo+IHN0YWdpbmcvaWlvL2Fk
Yy9hZDc3ODAuYyB0byByZW1vdmUgaXQgZnJvbSBzdGFpZ2luZz8NCj4gDQo+IEZvciBpbnN0YW5j
ZSwgc2hvdWxkIHdlIGFkZCBhbiBpZCBmaWVsZCBpbiBhZDc3ODBfc3RhdGUgc3RydWN0IHRoYXQN
Cj4gcmVwcmVzZW50cyBJRDEgYW5kIElEMCBmcm9tIHRoZSBzdGF0dXMgYml0cyAocGFnZSAxMyBm
cm9tIGRhdGFzaGVldCk/DQo+IE9yIGNoZWNrIHRoZSBTdGF0dXMgcGF0dGVybiBiaXRzIFBBVDEs
IFBBVDAgZm9yIGVycm9ycz8gT3Igc29tZXRoaW5nDQo+IGVsc2Ugd2UgY2FuIHdvcmsgb24/DQo+
IA0KDQpIZXksDQoNCllvdSBkb24ndCBuZWVkIHRvIGNoZWNrIGFueSBwYXR0ZXJucywgdGhhdCdz
IGFscmVhZHkgYmVpbmcgZG9uZSBpbiB0aGUNCmRyaXZlciAoc2VlIGBwYXR0ZXJuYCAmIGBwYXR0
ZXJuX21hc2tgIGZpZWxkcykuIA0KQW5kIHNlZSBmdW5jdGlvbiBhZDc3ODBfcG9zdHByb2Nlc3Nf
c2FtcGxlKCkuDQpUaG91Z2ggSSB3aWxsIGFkbWl0IGl0IHdvdWxkIGhhdmUgYmVlbiBuaWNlciBp
ZiB0aG9zZSBwYXR0ZXJucyB3ZXJlDQpnZW5lcmF0ZWQgZnJvbSBBRDc3ODBfUEFUMSAmIEFENzc4
MF9QQVQwLiBTbyAoaWYgeW91IHdhbnQpLCB5b3UgY291bGQNCnVwZGF0ZSB0aG9zZSBwYXR0ZXJu
IGFzc2lnbm1lbnRzIHdpdGggc29tZSBtYWNyb3MgOyBmb3IgQUQ3MTd4IHlvdSBtYXkgbmVlZA0K
dG8gYWRkIEFENzE3MF9QQVQyIChCSVQoMikpDQoNCkkgYW0gbm90aWNpbmcgYSBzbWFsbCBidWcg
d2l0aCB0aGUgZHJpdmVyIG5vdw0KKGluIGFkNzc4MF9wb3N0cHJvY2Vzc19zYW1wbGUoKSkuDQpU
aGUgQUQ3NzgwX0dBSU4gYml0LWZpZWxkIChmb3IgQUQ3Nzh4KSBvdmVybGFwcyB3aXRoIEFENzE3
MF9QQVQyIChmb3INCkFENzE3eCkuIFNvLCBtYXliZSB5b3UgY291bGQgYWRkIGEgZmllbGQgY2Fs
bGVkIGBpc19hZDc3OHhgIGluDQpgYWQ3NzgwX2NoaXBfaW5mb2AgYW5kIGluaXRpYWxpemUgaXQg
dG8gdHJ1ZSBpbiB0aGUgYGFkNzc4MF9jaGlwX2luZm9fdGJsYA0KZm9yIEFENzc4MCAmIEFENzc4
MSA7IGFuZCBvbmx5IHBlcmZvcm0gdGhlIEFENzc4MF9HQUlOIGNoZWNrIGZvciB0aGVzZQ0KZGV2
aWNlcy4NCkFjY29yZGluZyB0byB0aGUgZGF0YXNoZWV0cyBBRDcxNzAgJiBBRDcxNzEgaGF2ZSBh
IGZpeGVkIGdhaW4gb2YgMS4NCg0KSSB3b3VsZCBpZ25vcmUgdGhlIElEMCAmIElEMSBmaWVsZHMg
OyB0aG9zZSBzZWVtIHRvIG92ZXJsYXAgYSBiaXQgYmV0d2Vlbg0KQUQ3MTd4ICYgQUQ3Nzh4LiBB
bmQgdGhleSdyZSBub3QgdmVyeSB1c2VmdWwgaW4gdGhlIGRyaXZlci4NCg0KRm9yIEFENzc4eCB5
b3UgY291bGQgYWxzbyBhZGQgc3VwcG9ydCBmb3IgdGhlIEdBSU4gJiBGSUxURVIgcGlucyB2aWEg
R1BJT3MsDQp0byBjb250cm9sIHRoZXNlIHNldHRpbmdzLiBBbmQgdGhlbiBpbiB0aGUgYWQ3Nzgw
X3Bvc3Rwcm9jZXNzX3NhbXBsZSgpDQpmdW5jdGlvbiBjaGVjayBpZiB0aGUgR1BJTyBzZXR0aW5n
cyAoc3RvcmVkIG9uIGEgZ3Bpb19kZXNjIG9uIGFkNzc4MF9zdGF0ZSkNCm1hdGNoIHRoZSBleHBl
Y3RlZCBHQUlOICYgRklMVEVSIGJpdCBzZXR0aW5ncyA7IGlmIG5vdCByZXR1cm4gLUVJTy4NCg0K
QWxsLWluLWFsbCwgdGhlIGRyaXZlciBpcyBhbG1vc3QgcmVhZHkgdG8gbW92ZSBvdXQgb2Ygc3Rh
Z2luZyAoZnJvbSBteQ0KcG9pbnQgb2YgdmlldykuDQpOb3Qgc3VyZSBpZiBhbnlvbmUgZWxzZSBo
YXMgYW55dGhpbmcgZWxzZSB0byBhZGQuDQpUaGUgYnVnIHNob3VsZCBiZSBmaXhlZCwgYW5kIHRo
ZSByZXN0IGlzIG5pY2UtdG8taGF2ZSwgYnV0IG5vdCBuZWNlc3NhcmlseQ0KYSBtdXN0IChhZ2Fp
biwgZnJvbSBteSBwb2ludCBvZiB2aWV3KSB0byBrZWVwIGl0IGluIHN0YWdpbmcuDQoNCkFsc28s
IGdlbmVyYWxseTogbWF5YmUgZG8gYSBjb21wYXJhdGl2ZSBzd2VlcCBiZXR3ZWVuIEFENzE3eCAm
IEFENzc4eA0KZGF0YXNoZWV0cyBhbmQgc2VlIGlmIHRoZXJlJ3Mgc29tZXRoaW5nIGxlZnQgdGhh
dCdzIGluY29ycmVjdCBmb3IgYW55IG9mDQp0aGVzZSBjaGlwcy4NCg0KVGhhbmtzDQpBbGV4DQoN
Cj4gVGhhbmsgeW91Lg0KPiBPbiBUdWUsIE9jdCAyMywgMjAxOCBhdCA2OjUxIEFNIEpvbmF0aGFu
IENhbWVyb24NCj4gPGppYzIzQGppYzIzLnJldHJvc251Yi5jby51az4gd3JvdGU6DQo+ID4gDQo+
ID4gT24gTW9uLCAyMiBPY3QgMjAxOCAxODo1NjoxNiAtMDMwMA0KPiA+IEdpdWxpYW5vIEF1Z3Vz
dG8gRmF1bGluIEJlbGluYXNzaSA8Z2l1bGlhbm8uYmVsaW5hc3NpQHVzcC5icj4gd3JvdGU6DQo+
ID4gDQo+ID4gPiBIZWxsbywNCj4gPiA+IEkgaGF2ZSBzb21lIHF1ZXN0aW9ucyBhYm91dCB0aGUg
YWQ3NzgwIGRyaXZlci4NCj4gPiA+IA0KPiA+ID4gICAqIFdoYXQgaXMgdGhlIHZhbDIgSW4gdGhl
IGxpbmUgOTYgKCp2YWwyID0gY2hhbi0+c2Nhbl90eXBlLnJlYWxiaXRzDQo+ID4gPiAtIDE7KT8g
SG93IGl0IGlzIHVzZWQ/IElzIGl0IHJlbGF0ZWQgdG8gdGhlIDI0LWJpdHMgQURDIHByZWNpc2lv
bj8NCj4gPiANCj4gPiBUbyB1bmRlcnN0YW5kIHRoYXQgZm9sbG93IHRocm91Z2ggd2hhdCB0aGUg
Y29kZSBkb2VzIHdoZW4gdGhlIHR5cGUgb2YgYQ0KPiA+IHJlYWRfcmF3IGlzIElJT19WQUxfRlJB
Q1RJT05BTF9MT0cyOw0KPiA+IA0KPiA+ID4gICAqIFdoeSB0aGVyZSBpcyBhIHN1YnRyYWN0aW9u
IGluIHRoZSBsaW5lIDk5ICggKnZhbCAtPSAoMSA8PA0KPiA+ID4gKGNoYW4tPnNjYW5fdHlwZS5y
ZWFsYml0cyAtIDEpKTsgKT8gSXMgdGhlICp2YWwgaW5pdGlhbGl6ZWQgd2l0aCAwPw0KPiA+ID4g
SXMNCj4gPiA+IHRoYXQgcmVsYXRlZCB0byB0aGUgZm9ybXVsYSBkZXNjcmliZWQgaW4gdGhlIERB
VEEgT1VUUFVUIENPRElORywgYXQNCj4gPiA+IHBhZ2UgMTI/DQo+ID4gPiANCmh0dHBzOi8vd3d3
LmFuYWxvZy5jb20vbWVkaWEvZW4vdGVjaG5pY2FsLWRvY3VtZW50YXRpb24vZGF0YS1zaGVldHMv
YWQ3NzgwLnBkZg0KPiA+IA0KPiA+IFllYWgsIHRoYXQgLT0gaXMgZGVmaW5pdGVseSB3cm9uZy4g
IFNob3VsZCBqdXN0IGJlIGFuIGFzc2lnbm1lbnQgdG8gdGhlDQo+ID4gbmVnYXRpdmUgdmFsdWUu
ICBUaGlzIGlzIHRlbGxpbmcgdXNlcnNwYWNlIHRoYXQgdGhlIHJhdyB2YWx1ZSBzaG91bGQgYmUN
Cj4gPiBvZmZzZXQgYnkgaGFsZiBvZiBpdCdzIG1heGltdW0gcmFuZ2UgYmVmb3JlIGFwcGx5aW5n
IHRoZSBzY2FsZSB0byBnZXQNCj4gPiBhIHJlYWRpbmcgaW4gdGhlIElJTyBiYXNlIHVuaXRzICho
ZXJlIG1WIElJUkMpLg0KPiA+IA0KPiA+ID4gDQo+ID4gPiBXaXRoIHJlZ2FyZHMNCj4gPiA+IA0K
PiA+ID4gDQo+ID4gPiBPbiBUaHUsIE9jdCAxOCwgMjAxOCBhdCA4OjQxIEFNIEdpdWxpYW5vIEJl
bGluYXNzaQ0KPiA+ID4gPGdpdWxpYW5vLmJlbGluYXNzaUBnbWFpbC5jb20+IHdyb3RlOg0KPiA+
ID4gPiANCj4gPiA+ID4gT24gMTAvMTgsIEFyZGVsZWFuLCBBbGV4YW5kcnUgd3JvdGU6DQo+ID4g
PiA+IA0KPiA+ID4gPiBUaGFuayB5b3UgZm9yIHRoZSByZXBseSA6LSkuIFdlIHdpbGwgcHV0IGVm
Zm9ydCBpbnRvIGF0IGxlYXN0IG9uZQ0KPiA+ID4gPiBkcml2ZXIgdG8NCj4gPiA+ID4gcmVtb3Zl
IGl0IGZyb20gc3RhZ2luZy4NCj4gPiA+ID4gDQo+ID4gPiA+ID4gSGV5LA0KPiA+ID4gPiA+IA0K
PiA+ID4gPiA+IFRoYW5rcyBmb3IgdGhlIG5vdGlmaWNhdGlvbi4NCj4gPiA+ID4gPiANCj4gPiA+
ID4gPiBZZXMsIGFsbCB0aGVzZSAzIGRyaXZlcnMgc2hvdWxkIGJlIG1vdmVkIG91dCBvZiBzdGFn
aW5nLg0KPiA+ID4gPiA+IA0KPiA+ID4gPiA+IFdlIHdlcmUgcGxhbm5pbmcgdG8gc3RhcnQgd29y
a2luZyBvbiB0aGVtICh0byBtb3ZlIHRoZW0gb3V0IG9mDQo+ID4gPiA+ID4gc3RhZ2luZyksDQo+
ID4gPiA+ID4gYnV0IHByaW9yaXRpZXMgYXJlIHNoaWZ0aW5nIGZyb20gb25lIHdlZWsgdG8gdGhl
IG90aGVyLg0KPiA+ID4gPiA+IFNvLCBpZiB5b3UgZ3V5cyBoYXZlIHRoZSB0aW1lIGFuZCBhcmUg
d2lsbGluZyB0byBkbyBpdCwgcGxlYXNlIGdvDQo+ID4gPiA+ID4gYWhlYWQsDQo+ID4gPiA+ID4g
YW5kIGFkZCBtZSwgbWljaGFlbC5oZW5uZXJpY2hAYW5hbG9nLmNvbSBhbmQgDQo+ID4gPiA+ID4g
c3RlZmFuLnBvcGFAYW5hbG9nLmNvbSB0byB0aGUNCj4gPiA+ID4gPiBDQyB3aGVuIHNlbmRpbmcg
cGF0Y2hlcywgYW5kIHdlIHdpbGwgYWxzbyB0YWtlIGEgbG9vayBvdmVyIHlvdXINCj4gPiA+ID4g
PiBwYXRjaGVzLg0KPiA+ID4gPiA+IA0KPiA+ID4gPiA+IElmIHlvdSBoYXZlbid0IHRha2VuIGEg
bG9vayBhbHJlYWR5LCBmZWVsIGZyZWUgdG8gYWxzbyB0YWtlIGENCj4gPiA+ID4gPiBsb29rIGF0
IG91cg0KPiA+ID4gPiA+IHdpa2k6DQo+ID4gPiA+ID4gDQpodHRwczovL3dpa2kuYW5hbG9nLmNv
bS9yZXNvdXJjZXMvdG9vbHMtc29mdHdhcmUvbGludXgtZHJpdmVycy1hbGwNCj4gPiA+ID4gPiBJ
dCdzIG1vcmUgdGVjaG5pY2FsIGZvciB0aGUgU1cgc2lkZTsgaXQgbWlnaHQgaGVscCB3aXRoIHNv
bWUNCj4gPiA+ID4gPiBnZW5lcmFsIGluZm8uDQo+ID4gPiA+ID4gQW5kIHRoZSBkYXRhc2hlZXRz
IHNob3VsZCBiZSBhdmFpbGFibGUgb24gdGhlDQo+ID4gPiA+ID4gYW5hbG9nLmNvbS88cHJvZHVj
dF9pZD4NCj4gPiA+ID4gPiBwYWdlcy4NCj4gPiA+ID4gPiANCj4gPiA+ID4gPiANCj4gPiA+ID4g
PiBUaGFua3MNCj4gPiA+ID4gPiBBbGV4DQo+ID4gPiA+ID4gDQo+ID4gPiA+ID4gDQo+ID4gPiA+
ID4gT24gV2VkLCAyMDE4LTEwLTE3IGF0IDE2OjAwIC0wMzAwLCBHaXVsaWFubyBCZWxpbmFzc2kg
d3JvdGU6DQo+ID4gPiA+ID4gPiBIZWxsbywNCj4gPiA+ID4gPiA+IA0KPiA+ID4gPiA+ID4gV2Ug
YXJlIGEgc3R1ZGVudCBncm91cCBhbmQgd2Ugd2FudCB0byBjb250cmlidXRlIHRvIHRoZSBsaW51
eA0KPiA+ID4gPiA+ID4ga2VybmVsLiBXZQ0KPiA+ID4gPiA+ID4gYXJlIHRoaW5raW5nIGluIHB1
dCBlZmZvcnQgdG8gbW92ZSBhIGRyaXZlciBmcm9tIHN0YWdpbmcgdG8gdGhlDQo+ID4gPiA+ID4g
PiBtYWluDQo+ID4gPiA+ID4gPiB0cmVlLg0KPiA+ID4gPiA+ID4gV2UgbG9va2VkIGludG8gdGhy
ZWUgZHJpdmVyczoNCj4gPiA+ID4gPiA+IA0KPiA+ID4gPiA+ID4gICAqIGFkYy9hZDc3ODAuYw0K
PiA+ID4gPiA+ID4gICAqIGFkYy9hZDc4MTYuYw0KPiA+ID4gPiA+ID4gICAqIGltcGVkYW5jZS1h
bmFseXplci9hZDU5MzMuYw0KPiA+ID4gPiA+ID4gDQo+ID4gPiA+ID4gPiBBcmUgdGhlc2UgZHJp
dmVycyBzdGlsbCBiZWluZyB3b3JrZWQgb24/IFRoZSBsYXN0IGNvbW1pdA0KPiA+ID4gPiA+ID4g
cmVmZXJlbmNpbmcNCj4gPiA+ID4gPiA+IHRoZXNlDQo+ID4gPiA+ID4gPiBkcml2ZXJzIHdlcmUg
b24gbWFyY2ggMjAxOCwgYnV0IHRoZXkgYXJlIHN0aWxsIGluIHN0YWdpbmcgc2luY2UNCj4gPiA+
ID4gPiA+IDIwMTAuDQo+ID4gPiA+ID4gPiANCj4gPiA+ID4gPiA+IFdlIGFscmVhZHkga25vdyB0
aGF0IHdlIGhhdmUgdG8gY29udmVydCBmcm9tIHRoZSBvbGQgYXBpIHRvIHRoZQ0KPiA+ID4gPiA+
ID4gbmV3IG9uZS4NCj4gPiA+ID4gPiA+IEl0IGlzDQo+ID4gPiA+ID4gPiBPSyBmb3IgdXMgdG8g
d29yayBvbiB0aGlzPyBJcyB0aGVyZSBzb21ldGhpbmcgZWxzZSB3ZSBjYW4gZG8/DQo+ID4gPiA+
ID4gPiANCj4gPiA+ID4gPiA+IFRoYW5rIHlvdQ0KPiA+ID4gPiA+ID4gDQo+ID4gPiA+ID4gDQo+
ID4gPiA+ID4gLS0NCj4gPiA+ID4gPiBZb3UgcmVjZWl2ZWQgdGhpcyBtZXNzYWdlIGJlY2F1c2Ug
eW91IGFyZSBzdWJzY3JpYmVkIHRvIHRoZQ0KPiA+ID4gPiA+IEdvb2dsZSBHcm91cHMgIktlcm5l
bCBVU1AiIGdyb3VwLg0KPiA+ID4gPiA+IFRvIHVuc3Vic2NyaWJlIGZyb20gdGhpcyBncm91cCBh
bmQgc3RvcCByZWNlaXZpbmcgZW1haWxzIGZyb20gaXQsDQo+ID4gPiA+ID4gc2VuZCBhbiBlbWFp
bCB0byBrZXJuZWwtdXNwK3Vuc3Vic2NyaWJlQGdvb2dsZWdyb3Vwcy5jb20uDQo+ID4gPiA+ID4g
VG8gcG9zdCB0byB0aGlzIGdyb3VwLCBzZW5kIGVtYWlsIHRvIGtlcm5lbC11c3BAZ29vZ2xlZ3Jv
dXBzLmNvbS4NCj4gPiA+ID4gPiBUbyB2aWV3IHRoaXMgZGlzY3Vzc2lvbiBvbiB0aGUgd2ViIHZp
c2l0IA0KPiA+ID4gPiA+IGh0dHBzOi8vZ3JvdXBzLmdvb2dsZS5jb20vZC9tc2dpZC9rZXJuZWwt
dXNwL2QwZmM1YTIzNmE4MmQwM2ZkYmU0ZDA1ZWIyN2ZmM2JkM2FjMzY5NTguY2FtZWwlNDBhbmFs
b2cuY29tDQo+ID4gPiA+ID4gLg0KPiA+ID4gPiA+IEZvciBtb3JlIG9wdGlvbnMsIHZpc2l0IGh0
dHBzOi8vZ3JvdXBzLmdvb2dsZS5jb20vZC9vcHRvdXQuDQo+ID4gPiA+IA0KPiA+ID4gPiAtLQ0K
PiA+ID4gPiBZb3UgcmVjZWl2ZWQgdGhpcyBtZXNzYWdlIGJlY2F1c2UgeW91IGFyZSBzdWJzY3Jp
YmVkIHRvIHRoZSBHb29nbGUNCj4gPiA+ID4gR3JvdXBzICJLZXJuZWwgVVNQIiBncm91cC4NCj4g
PiA+ID4gVG8gdW5zdWJzY3JpYmUgZnJvbSB0aGlzIGdyb3VwIGFuZCBzdG9wIHJlY2VpdmluZyBl
bWFpbHMgZnJvbSBpdCwNCj4gPiA+ID4gc2VuZCBhbiBlbWFpbCB0byBrZXJuZWwtdXNwK3Vuc3Vi
c2NyaWJlQGdvb2dsZWdyb3Vwcy5jb20uDQo+ID4gPiA+IFRvIHBvc3QgdG8gdGhpcyBncm91cCwg
c2VuZCBlbWFpbCB0byBrZXJuZWwtdXNwQGdvb2dsZWdyb3Vwcy5jb20uDQo+ID4gPiA+IFRvIHZp
ZXcgdGhpcyBkaXNjdXNzaW9uIG9uIHRoZSB3ZWIgdmlzaXQgDQo+ID4gPiA+IGh0dHBzOi8vZ3Jv
dXBzLmdvb2dsZS5jb20vZC9tc2dpZC9rZXJuZWwtdXNwLzIwMTgxMDE4MTE0MTQ1LmFtcWZmNTdo
b2xtZWN4cG8lNDBzbXRwLmdtYWlsLmNvbQ0KPiA+ID4gPiAuDQo+ID4gPiA+IEZvciBtb3JlIG9w
dGlvbnMsIHZpc2l0IGh0dHBzOi8vZ3JvdXBzLmdvb2dsZS5jb20vZC9vcHRvdXQuDQo+IA0KPiAN
Cg==

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Questions related to some drivers
  2018-11-06 13:23           ` Ardelean, Alexandru
@ 2018-11-07 18:57             ` Renato Lui Geh
  2018-11-08 14:04               ` Renato Lui Geh
  0 siblings, 1 reply; 14+ messages in thread
From: Renato Lui Geh @ 2018-11-07 18:57 UTC (permalink / raw)
  To: Ardelean, Alexandru
  Cc: giuliano.belinassi, jic23, giuliano.belinassi, renatogeh,
	linux-iio, kernel-usp

On 11/06, Ardelean, Alexandru wrote:
>Hey,
>
>You don't need to check any patterns, that's already being done in the
>driver (see `pattern` & `pattern_mask` fields).
>And see function ad7780_postprocess_sample().
>Though I will admit it would have been nicer if those patterns were
>generated from AD7780_PAT1 & AD7780_PAT0. So (if you want), you could
>update those pattern assignments with some macros ; for AD717x you may need
>to add AD7170_PAT2 (BIT(2))
>
>I am noticing a small bug with the driver now
>(in ad7780_postprocess_sample()).
>The AD7780_GAIN bit-field (for AD778x) overlaps with AD7170_PAT2 (for
>AD717x). So, maybe you could add a field called `is_ad778x` in
>`ad7780_chip_info` and initialize it to true in the `ad7780_chip_info_tbl`
>for AD7780 & AD7781 ; and only perform the AD7780_GAIN check for these
>devices.
>According to the datasheets AD7170 & AD7171 have a fixed gain of 1.
>
>I would ignore the ID0 & ID1 fields ; those seem to overlap a bit between
>AD717x & AD778x. And they're not very useful in the driver.
>

Hi,

Thank you for the clues on what to work on. We've just now sent a
patchset with the fixes you mentioned in the quoted block above.

>For AD778x you could also add support for the GAIN & FILTER pins via GPIOs,
>to control these settings. And then in the ad7780_postprocess_sample()
>function check if the GPIO settings (stored on a gpio_desc on ad7780_state)
>match the expected GAIN & FILTER bit settings ; if not return -EIO.
>
>All-in-all, the driver is almost ready to move out of staging (from my
>point of view).
>Not sure if anyone else has anything else to add.
>The bug should be fixed, and the rest is nice-to-have, but not necessarily
>a must (again, from my point of view) to keep it in staging.
>
>Also, generally: maybe do a comparative sweep between AD717x & AD778x
>datasheets and see if there's something left that's incorrect for any of
>these chips.

We'll take a look on these issues and study them further and will start
working on them once we're confident we understand them.

>Thanks
>Alex

Thanks,
Renato

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Questions related to some drivers
  2018-11-07 18:57             ` Renato Lui Geh
@ 2018-11-08 14:04               ` Renato Lui Geh
  2018-11-08 14:46                 ` Ardelean, Alexandru
  0 siblings, 1 reply; 14+ messages in thread
From: Renato Lui Geh @ 2018-11-08 14:04 UTC (permalink / raw)
  To: Ardelean, Alexandru
  Cc: giuliano.belinassi, jic23, giuliano.belinassi, renatogeh,
	linux-iio, kernel-usp

Hi,

On 11/06, Ardelean, Alexandru wrote:
>For AD778x you could also add support for the GAIN & FILTER pins via GPIOs,
>to control these settings. And then in the ad7780_postprocess_sample()
>function check if the GPIO settings (stored on a gpio_desc on ad7780_state)
>match the expected GAIN & FILTER bit settings ; if not return -EIO.

We're having some trouble with the GPIOs, and would like some insight on
how to proceed. Any help would be very much appreciated!

We're wondering if we should do something like this in ad7780.c's probe:

st->powerdown_gpio = devm_gpiod_get_optional(&spi->dev,
					     "powerdown",
					     GPIOD_OUT_LOW);

for both gain and filter. Taking a look at driver drivers/iio/adc/hx711.c
(another adc driver out of staging), we have:

hx711_data->gpiod_pd_sck = devm_gpiod_get(dev, "sck", GPIOD_OUT_LOW);

So we're assuming "sck" and "powerdown" are the pins we're looking for. Are we
correct to assume that these strings are compared with a table that map the
actual GPIO pins? So we'd have something like:

st->gain_gpio = devm_gpiod_get_optional(&spi->dev,
					"gain",
					GPIO_DOUT_LOW);

Where "gain" is the pin 4 or 5 on the AD778x
(https://www.analog.com/media/en/technical-documentation/data-sheets/ad7780.pdf)
chip.

Where can we find this table that maps these pin names to the actual pin
numbers? We found this link
https://www.kernel.org/doc/html/v4.17/driver-api/gpio/board.html that shows how
to declare table attributes, but we couldn't find this lookup table there.

Are we missing something? Out of curiosity, why do we have to pass a string
(e.g. powerdown, gain, sck, dout) instead of the pin number? We found somewhere
that they are names to functions. Are these functions implemented on the chip?

Thanks,
Renato

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Questions related to some drivers
  2018-11-08 14:04               ` Renato Lui Geh
@ 2018-11-08 14:46                 ` Ardelean, Alexandru
  2018-11-09 22:08                   ` Giuliano Augusto Faulin Belinassi
  2018-11-13 13:56                   ` Giuliano Augusto Faulin Belinassi
  0 siblings, 2 replies; 14+ messages in thread
From: Ardelean, Alexandru @ 2018-11-08 14:46 UTC (permalink / raw)
  To: renatogeh
  Cc: giuliano.belinassi, giuliano.belinassi, linux-iio, kernel-usp, jic23

T24gVGh1LCAyMDE4LTExLTA4IGF0IDEyOjA0IC0wMjAwLCBSZW5hdG8gTHVpIEdlaCB3cm90ZToN
Cj4gSGksDQo+IA0KDQpIZXksDQoNCj4gT24gMTEvMDYsIEFyZGVsZWFuLCBBbGV4YW5kcnUgd3Jv
dGU6DQo+ID4gRm9yIEFENzc4eCB5b3UgY291bGQgYWxzbyBhZGQgc3VwcG9ydCBmb3IgdGhlIEdB
SU4gJiBGSUxURVIgcGlucyB2aWENCj4gPiBHUElPcywNCj4gPiB0byBjb250cm9sIHRoZXNlIHNl
dHRpbmdzLiBBbmQgdGhlbiBpbiB0aGUgYWQ3NzgwX3Bvc3Rwcm9jZXNzX3NhbXBsZSgpDQo+ID4g
ZnVuY3Rpb24gY2hlY2sgaWYgdGhlIEdQSU8gc2V0dGluZ3MgKHN0b3JlZCBvbiBhIGdwaW9fZGVz
YyBvbg0KPiA+IGFkNzc4MF9zdGF0ZSkNCj4gPiBtYXRjaCB0aGUgZXhwZWN0ZWQgR0FJTiAmIEZJ
TFRFUiBiaXQgc2V0dGluZ3MgOyBpZiBub3QgcmV0dXJuIC1FSU8uDQo+IA0KPiBXZSdyZSBoYXZp
bmcgc29tZSB0cm91YmxlIHdpdGggdGhlIEdQSU9zLCBhbmQgd291bGQgbGlrZSBzb21lIGluc2ln
aHQgb24NCj4gaG93IHRvIHByb2NlZWQuIEFueSBoZWxwIHdvdWxkIGJlIHZlcnkgbXVjaCBhcHBy
ZWNpYXRlZCENCj4gDQo+IFdlJ3JlIHdvbmRlcmluZyBpZiB3ZSBzaG91bGQgZG8gc29tZXRoaW5n
IGxpa2UgdGhpcyBpbiBhZDc3ODAuYydzIHByb2JlOg0KPiANCj4gc3QtPnBvd2VyZG93bl9ncGlv
ID0gZGV2bV9ncGlvZF9nZXRfb3B0aW9uYWwoJnNwaS0+ZGV2LA0KPiAJCQkJCSAgICAgInBvd2Vy
ZG93biIsDQo+IAkJCQkJICAgICBHUElPRF9PVVRfTE9XKTsNCj4gDQoNClllcywgc29tZXRoaW5n
IGxpa2UgdGhpcy4NCg0KPiBmb3IgYm90aCBnYWluIGFuZCBmaWx0ZXIuIFRha2luZyBhIGxvb2sg
YXQgZHJpdmVyIGRyaXZlcnMvaWlvL2FkYy9oeDcxMS5jDQo+IChhbm90aGVyIGFkYyBkcml2ZXIg
b3V0IG9mIHN0YWdpbmcpLCB3ZSBoYXZlOg0KPiANCj4gaHg3MTFfZGF0YS0+Z3Bpb2RfcGRfc2Nr
ID0gZGV2bV9ncGlvZF9nZXQoZGV2LCAic2NrIiwgR1BJT0RfT1VUX0xPVyk7DQo+IA0KPiBTbyB3
ZSdyZSBhc3N1bWluZyAic2NrIiBhbmQgInBvd2VyZG93biIgYXJlIHRoZSBwaW5zIHdlJ3JlIGxv
b2tpbmcgZm9yLg0KPiBBcmUgd2UNCj4gY29ycmVjdCB0byBhc3N1bWUgdGhhdCB0aGVzZSBzdHJp
bmdzIGFyZSBjb21wYXJlZCB3aXRoIGEgdGFibGUgdGhhdCBtYXANCj4gdGhlDQo+IGFjdHVhbCBH
UElPIHBpbnM/IFNvIHdlJ2QgaGF2ZSBzb21ldGhpbmcgbGlrZToNCj4gDQo+IHN0LT5nYWluX2dw
aW8gPSBkZXZtX2dwaW9kX2dldF9vcHRpb25hbCgmc3BpLT5kZXYsDQo+IAkJCQkJImdhaW4iLA0K
PiAJCQkJCUdQSU9fRE9VVF9MT1cpOw0KDQpUaGlzIGlzIGV4YWN0bHkgYSBnb29kIGlkZWEgdG8g
ZG8gZm9yIGBnYWluYC4NCkFuZCBzaW1pbGFyIGZvciBgZmlsdGVyYA0KDQo+IA0KPiBXaGVyZSAi
Z2FpbiIgaXMgdGhlIHBpbiA0IG9yIDUgb24gdGhlIEFENzc4eA0KDQpUaGUgcGluIDQgb3IgNSBv
biB0aGUgQUQ3Nzh4IGFyZSBub3Qgd2hhdCB5b3UgZGVmaW5lIGhlcmUuDQpUaGUgR1BJT3MgeW91
IGRlZmluZS91c2UgaGVyZSBhcmUgb24gdGhlIGhvc3QtYm9hcmQuDQoNClNvLA0KDQpIb3N0LWJv
YXJkIFtkdC1lbnRyeV0gPC0tLS0+IEFENzc4eCBbSFctcGluXQ0KDQpXaGVyZToNCiogYGR0LWVu
dHJ5YCBpcyBkZWZpbnRlZCBpbiB0aGUgZGV2aWNlLXRyZWUgc29tZXRoaW5nIGxpa2U6DQogICBn
YWluLWdwaW8gPSA8JmdwaW8gR1BJT19OVU1CRVJfT05fSE9TVF9CT0FSRCBHUElPX0ZMQUdTPiAN
CiogR1BJT19OVU1CRVJfT05fSE9TVF9CT0FSRCBpcyBhIG51bWJlciwgd2hpY2ggdmFyaWVzIDsg
Zm9yIGV4YW1wbGUgZm9yDQpSYXNwYmVycnkgUGkgaXQgY291bGQgYmUgcGluIDI1IFtpbiB0aGUg
ZGV2aWNlIHRyZWVdOyANCiogR1BJT19GTEFHUyBpcyB1c3VhbGx5IDAgOyBidXQgY2FuIGJlIGFu
eSBvdGhlciBudW1lcmljIHZhbHVlIGRlZmluZWQNCmhlcmU6DQogIA0KaHR0cHM6Ly9naXRodWIu
Y29tL3RvcnZhbGRzL2xpbnV4L2Jsb2IvbWFzdGVyL2luY2x1ZGUvZHQtYmluZGluZ3MvZ3Bpby9n
cGlvLmgNCiogYEhXLXBpbmAgZG9lc24ndCBtYXR0ZXIsIGJlY2F1c2UgaXQncyBhIHBoeXNpY2Fs
IHBpbiB0aGF0IGNhbiBiZSBkZWZpbmVkDQphcyBhbnl0aGluZyBpbiBTVw0KDQo+ICgNCj4gaHR0
cHM6Ly93d3cuYW5hbG9nLmNvbS9tZWRpYS9lbi90ZWNobmljYWwtZG9jdW1lbnRhdGlvbi9kYXRh
LXNoZWV0cy9hZDc3ODAucGRmDQo+ICkNCj4gY2hpcC4NCj4gDQo+IFdoZXJlIGNhbiB3ZSBmaW5k
IHRoaXMgdGFibGUgdGhhdCBtYXBzIHRoZXNlIHBpbiBuYW1lcyB0byB0aGUgYWN0dWFsIHBpbg0K
PiBudW1iZXJzPyBXZSBmb3VuZCB0aGlzIGxpbmsNCg0KVGhlIGFjdHVhbCBwaW4gbnVtYmVyIGRv
ZXNuJ3QgbWF0dGVyIGluIHRoZSBkcml2ZXIgeW91IHdyaXRlLg0KSXQgbWF0dGVycyBpbiB0aGUg
ZGV2aWNlLXRyZWUgZm9yIHRoZSBib2FyZCB5b3UgdXNlIHRoZSBkcml2ZXIgW2FuZCBjaGlwXQ0K
b24uDQoNCklmIHlvdSB3ZXJlIHRvIHRlc3QgeW91ciBkcml2ZXIgY2hhbmdlIG9uIGEgUlBpIGJv
YXJkIGFuZCB5b3UgcGh5c2ljYWxseQ0KY29ubmVjdCB0aGlzIG9uIFBpbiAyNSAocGh5c2ljYWwp
IGFuZCBpbiB5b3VyIGRldmljZS10cmVlDQpHUElPX05VTUJFUl9PTl9IT1NUX0JPQVJEIGlzIHRo
ZSB2YWx1ZSAoaW4gU1cpIGZvciBQaW4gMjUsIHRoZW4gdGhhdCdzIHdoYXQNCm1hdHRlcnMgd2hl
biB5b3UgdGVzdC9ydW4gdGhlIGNvZGUuDQoNCkluIHRoZSBrZXJuZWwsIGEgcGluIG51bWJlcnMg
dGFibGUgZm9yIHRoZSBSUGkgY2FuIGJlIGZvdW5kIGhlcmU6DQoNCmh0dHBzOi8vZ2l0aHViLmNv
bS90b3J2YWxkcy9saW51eC9ibG9iL21hc3Rlci9hcmNoL2FybS9ib290L2R0cy9iY20yODN4LmR0
c2kjTDE4NQ0KDQpJIGtlZXAgcmVmZXJyaW5nIHRvIFJQaSBiZWNhdXNlIHdlJ3ZlIHVzZWQgaXQg
YSBiaXQgbW9yZSB0aGFuIG90aGVyIGJvYXJkcywNCmFuZCBhbHNvIGJlY2F1c2UgZm9yIFJQaSB1
c3VhbGx5IFBpbiAyNSBpbiBIVyBpcyBhbHNvIFBpbiAyNSBpbiBTVy4NCg0KDQo+IGh0dHBzOi8v
d3d3Lmtlcm5lbC5vcmcvZG9jL2h0bWwvdjQuMTcvZHJpdmVyLWFwaS9ncGlvL2JvYXJkLmh0bWwg
dGhhdA0KPiBzaG93cyBob3cNCj4gdG8gZGVjbGFyZSB0YWJsZSBhdHRyaWJ1dGVzLCBidXQgd2Ug
Y291bGRuJ3QgZmluZCB0aGlzIGxvb2t1cCB0YWJsZQ0KPiB0aGVyZS4NCj4gDQo+IEFyZSB3ZSBt
aXNzaW5nIHNvbWV0aGluZz8gT3V0IG9mIGN1cmlvc2l0eSwgd2h5IGRvIHdlIGhhdmUgdG8gcGFz
cyBhDQo+IHN0cmluZw0KPiAoZS5nLiBwb3dlcmRvd24sIGdhaW4sIHNjaywgZG91dCkgaW5zdGVh
ZCBvZiB0aGUgcGluIG51bWJlcj8gV2UgZm91bmQNCj4gc29tZXdoZXJlDQo+IHRoYXQgdGhleSBh
cmUgbmFtZXMgdG8gZnVuY3Rpb25zLiBBcmUgdGhlc2UgZnVuY3Rpb25zIGltcGxlbWVudGVkIG9u
IHRoZQ0KPiBjaGlwPw0KPiANCg0KU28sIHRoZSBzdHJpbmcgaW4gdGhlIGRyaXZlciwgd2lsbCBi
ZSB1c2VkIHRvIGxvb2t1cCB0aGUgcGh5c2ljYWwgcGluIGluDQp0aGUgZGV2aWNlLXRyZWUuDQpU
aGUgcGluIG51bWJlciBkaWZmZXJzIGZyb20gb25lIGhvc3QtYm9hcmQgdG8gYW5vdGhlciBbYXMg
SSd2ZSBzYWlkXSwgc28NCnRoZSBzdHJpbmcgaXMgYSB1bmlxdWUgaWRlbnRpZmllciBmb3IgdGhl
IGRyaXZlciwgd2hpY2ggcmVzb2x2ZXMgdG8gdGhlDQpwaHlzaWNhbCBwaW4gb24gdGhlIGJvYXJk
Lg0KDQpCeSB0aGUgd2F5OiBvbmUgd29yayBpdGVtIChtYXliZSBhZnRlciB0aGUgZHJpdmVyIGlz
IG1vdmVkIG91dCBvZiBzdGFnaW5nKQ0Kd291bGQgYmUgdG8gYWxzbyB3cml0ZSBhIGRldmljZS10
cmVlIGJpbmRpbmcgZG9jLg0KQW4gZXhhbXBsZToNCiANCmh0dHBzOi8vZ2l0aHViLmNvbS90b3J2
YWxkcy9saW51eC9ibG9iL21hc3Rlci9Eb2N1bWVudGF0aW9uL2RldmljZXRyZWUvYmluZGluZ3Mv
c291bmQvYWRpLGFkYXUxN3gxLnR4dA0KSXQncyBub3QgdGhlIGJlc3Qgb25lLCBhbmQgZmVlbCBm
cmVlIHRvIGxvb2sgZm9yIG90aGVycyBhcyB3ZWxsLCB0aGF0DQpkZWZpbmUgR1BJT3MuIEJ1dCBp
biB0aGF0IGZpbGUsIGlzIGFuIGV4YW1wbGUgb2YgaG93IHRvIGxpbmsgYSByZXNldC1ncGlvDQp0
aGF0IHdvdWxkIGJlIGNhbGxlZCBieSB0aGF0IGRyaXZlciB0byBwaHlzaWNhbGx5IHJlc2V0IHRo
ZSBkZXZpY2Ugd2hlbg0KcHJvYmVkICYgaW5pdGlhbGl6ZWQuDQoNClVzdWFsbHkgdGhlIGFyY2gv
YXJtWzY0XS9ib290L2R0cyBoYXZlIGEgbG90IG9mIGRldmljZS10cmVlcyB0aGF0IG1heSBoZWxw
DQpzaGFwZSBzb21lIHRoaW5raW5nIGFib3V0IGRldmljZS10cmVlcy4NCg0KSSdtIGhvcGluZyBJ
IGdvdCB0byBleHBsYWluIHRoaW5ncyBzb21ld2hhdC4NCkl0IGlzIGEgYml0IGxhdGUgaW4gdGhl
IGFmdGVybm9vbiBbZm9yIG1lXSwgYnV0IEkgdGhvdWdodCBJJ2QgZ2l2ZSBpdCBhIHRyeQ0KOikN
Cg0KSWYgdGhlcmUgYXJlIHN0aWxsIHF1ZXN0aW9ucywgZmVlbCBmcmVlIHRvIGFzay4NCkkgdGhp
bmsgSSBjYW4gZ2V0IHRvIHRoZW0gdG9tb3Jyb3cuDQoNClRoYW5rcw0KQWxleA0KDQo+IFRoYW5r
cywNCj4gUmVuYXRvDQo+IA0K

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Questions related to some drivers
  2018-11-08 14:46                 ` Ardelean, Alexandru
@ 2018-11-09 22:08                   ` Giuliano Augusto Faulin Belinassi
  2018-11-13 13:56                   ` Giuliano Augusto Faulin Belinassi
  1 sibling, 0 replies; 14+ messages in thread
From: Giuliano Augusto Faulin Belinassi @ 2018-11-09 22:08 UTC (permalink / raw)
  To: alexandru.Ardelean
  Cc: Renato Geh, giuliano.belinassi, linux-iio, Kernel USP, jic23

Hello. Sorry for the late reply

> On a more private note: do you guys have HW to test your code/changes on ?
No, we don't have it. I think it would be really nice if we could have
access to the
hardware to work on. :-)

> Jonathan did ask us about this in private, but I think the discussion has
> stalled a bit (time & priorities).
>
> We could send a few samples if there's anything you'd like to test this
> code on.

That would be really helpful :-)

> FWIW: we usually test quite a few of this simple ADC/DAC changes on
> Raspberry PI ; it's a pretty useful board for testing stuff.
On Thu, Nov 8, 2018 at 12:46 PM Ardelean, Alexandru
<alexandru.Ardelean@analog.com> wrote:
>
> On Thu, 2018-11-08 at 12:04 -0200, Renato Lui Geh wrote:
> > Hi,
> >
>
> Hey,
>
> > On 11/06, Ardelean, Alexandru wrote:
> > > For AD778x you could also add support for the GAIN & FILTER pins via
> > > GPIOs,
> > > to control these settings. And then in the ad7780_postprocess_sample()
> > > function check if the GPIO settings (stored on a gpio_desc on
> > > ad7780_state)
> > > match the expected GAIN & FILTER bit settings ; if not return -EIO.
> >
> > We're having some trouble with the GPIOs, and would like some insight on
> > how to proceed. Any help would be very much appreciated!
> >
> > We're wondering if we should do something like this in ad7780.c's probe:
> >
> > st->powerdown_gpio = devm_gpiod_get_optional(&spi->dev,
> >                                            "powerdown",
> >                                            GPIOD_OUT_LOW);
> >
>
> Yes, something like this.
>
> > for both gain and filter. Taking a look at driver drivers/iio/adc/hx711.c
> > (another adc driver out of staging), we have:
> >
> > hx711_data->gpiod_pd_sck = devm_gpiod_get(dev, "sck", GPIOD_OUT_LOW);
> >
> > So we're assuming "sck" and "powerdown" are the pins we're looking for.
> > Are we
> > correct to assume that these strings are compared with a table that map
> > the
> > actual GPIO pins? So we'd have something like:
> >
> > st->gain_gpio = devm_gpiod_get_optional(&spi->dev,
> >                                       "gain",
> >                                       GPIO_DOUT_LOW);
>
> This is exactly a good idea to do for `gain`.
> And similar for `filter`
>
> >
> > Where "gain" is the pin 4 or 5 on the AD778x
>
> The pin 4 or 5 on the AD778x are not what you define here.
> The GPIOs you define/use here are on the host-board.
>
> So,
>
> Host-board [dt-entry] <----> AD778x [HW-pin]
>
> Where:
> * `dt-entry` is definted in the device-tree something like:
>    gain-gpio = <&gpio GPIO_NUMBER_ON_HOST_BOARD GPIO_FLAGS>
> * GPIO_NUMBER_ON_HOST_BOARD is a number, which varies ; for example for
> Raspberry Pi it could be pin 25 [in the device tree];
> * GPIO_FLAGS is usually 0 ; but can be any other numeric value defined
> here:
>
> https://github.com/torvalds/linux/blob/master/include/dt-bindings/gpio/gpio.h
> * `HW-pin` doesn't matter, because it's a physical pin that can be defined
> as anything in SW
>
> > (
> > https://www.analog.com/media/en/technical-documentation/data-sheets/ad7780.pdf
> > )
> > chip.
> >
> > Where can we find this table that maps these pin names to the actual pin
> > numbers? We found this link
>
> The actual pin number doesn't matter in the driver you write.
> It matters in the device-tree for the board you use the driver [and chip]
> on.
>
> If you were to test your driver change on a RPi board and you physically
> connect this on Pin 25 (physical) and in your device-tree
> GPIO_NUMBER_ON_HOST_BOARD is the value (in SW) for Pin 25, then that's what
> matters when you test/run the code.
>
> In the kernel, a pin numbers table for the RPi can be found here:
>
> https://github.com/torvalds/linux/blob/master/arch/arm/boot/dts/bcm283x.dtsi#L185
>
> I keep referring to RPi because we've used it a bit more than other boards,
> and also because for RPi usually Pin 25 in HW is also Pin 25 in SW.
>
>
> > https://www.kernel.org/doc/html/v4.17/driver-api/gpio/board.html that
> > shows how
> > to declare table attributes, but we couldn't find this lookup table
> > there.
> >
> > Are we missing something? Out of curiosity, why do we have to pass a
> > string
> > (e.g. powerdown, gain, sck, dout) instead of the pin number? We found
> > somewhere
> > that they are names to functions. Are these functions implemented on the
> > chip?
> >
>
> So, the string in the driver, will be used to lookup the physical pin in
> the device-tree.
> The pin number differs from one host-board to another [as I've said], so
> the string is a unique identifier for the driver, which resolves to the
> physical pin on the board.
>
> By the way: one work item (maybe after the driver is moved out of staging)
> would be to also write a device-tree binding doc.
> An example:
>
> https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/sound/adi,adau17x1.txt
> It's not the best one, and feel free to look for others as well, that
> define GPIOs. But in that file, is an example of how to link a reset-gpio
> that would be called by that driver to physically reset the device when
> probed & initialized.
>
> Usually the arch/arm[64]/boot/dts have a lot of device-trees that may help
> shape some thinking about device-trees.
>
> I'm hoping I got to explain things somewhat.
> It is a bit late in the afternoon [for me], but I thought I'd give it a try
> :)
>
> If there are still questions, feel free to ask.
> I think I can get to them tomorrow.
>
> Thanks
> Alex
>
> > Thanks,
> > Renato
> >

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Questions related to some drivers
  2018-11-08 14:46                 ` Ardelean, Alexandru
  2018-11-09 22:08                   ` Giuliano Augusto Faulin Belinassi
@ 2018-11-13 13:56                   ` Giuliano Augusto Faulin Belinassi
  2018-11-14  9:55                     ` Ardelean, Alexandru
  1 sibling, 1 reply; 14+ messages in thread
From: Giuliano Augusto Faulin Belinassi @ 2018-11-13 13:56 UTC (permalink / raw)
  To: alexandru.Ardelean
  Cc: Renato Geh, giuliano.belinassi, linux-iio, Kernel USP, jic23

Hello,
     We have some doubts regarding the FILTER and GAIN pins. Should
these pins be set in userspace, and therefore there must be a
'ad7780_write_raw' function? Or should we use the device tree with a
value set on initialization?

Thank you
On Thu, Nov 8, 2018 at 12:46 PM Ardelean, Alexandru
<alexandru.Ardelean@analog.com> wrote:
>
> On Thu, 2018-11-08 at 12:04 -0200, Renato Lui Geh wrote:
> > Hi,
> >
>
> Hey,
>
> > On 11/06, Ardelean, Alexandru wrote:
> > > For AD778x you could also add support for the GAIN & FILTER pins via
> > > GPIOs,
> > > to control these settings. And then in the ad7780_postprocess_sample()
> > > function check if the GPIO settings (stored on a gpio_desc on
> > > ad7780_state)
> > > match the expected GAIN & FILTER bit settings ; if not return -EIO.
> >
> > We're having some trouble with the GPIOs, and would like some insight on
> > how to proceed. Any help would be very much appreciated!
> >
> > We're wondering if we should do something like this in ad7780.c's probe:
> >
> > st->powerdown_gpio = devm_gpiod_get_optional(&spi->dev,
> >                                            "powerdown",
> >                                            GPIOD_OUT_LOW);
> >
>
> Yes, something like this.
>
> > for both gain and filter. Taking a look at driver drivers/iio/adc/hx711.c
> > (another adc driver out of staging), we have:
> >
> > hx711_data->gpiod_pd_sck = devm_gpiod_get(dev, "sck", GPIOD_OUT_LOW);
> >
> > So we're assuming "sck" and "powerdown" are the pins we're looking for.
> > Are we
> > correct to assume that these strings are compared with a table that map
> > the
> > actual GPIO pins? So we'd have something like:
> >
> > st->gain_gpio = devm_gpiod_get_optional(&spi->dev,
> >                                       "gain",
> >                                       GPIO_DOUT_LOW);
>
> This is exactly a good idea to do for `gain`.
> And similar for `filter`
>
> >
> > Where "gain" is the pin 4 or 5 on the AD778x
>
> The pin 4 or 5 on the AD778x are not what you define here.
> The GPIOs you define/use here are on the host-board.
>
> So,
>
> Host-board [dt-entry] <----> AD778x [HW-pin]
>
> Where:
> * `dt-entry` is definted in the device-tree something like:
>    gain-gpio = <&gpio GPIO_NUMBER_ON_HOST_BOARD GPIO_FLAGS>
> * GPIO_NUMBER_ON_HOST_BOARD is a number, which varies ; for example for
> Raspberry Pi it could be pin 25 [in the device tree];
> * GPIO_FLAGS is usually 0 ; but can be any other numeric value defined
> here:
>
> https://github.com/torvalds/linux/blob/master/include/dt-bindings/gpio/gpio.h
> * `HW-pin` doesn't matter, because it's a physical pin that can be defined
> as anything in SW
>
> > (
> > https://www.analog.com/media/en/technical-documentation/data-sheets/ad7780.pdf
> > )
> > chip.
> >
> > Where can we find this table that maps these pin names to the actual pin
> > numbers? We found this link
>
> The actual pin number doesn't matter in the driver you write.
> It matters in the device-tree for the board you use the driver [and chip]
> on.
>
> If you were to test your driver change on a RPi board and you physically
> connect this on Pin 25 (physical) and in your device-tree
> GPIO_NUMBER_ON_HOST_BOARD is the value (in SW) for Pin 25, then that's what
> matters when you test/run the code.
>
> In the kernel, a pin numbers table for the RPi can be found here:
>
> https://github.com/torvalds/linux/blob/master/arch/arm/boot/dts/bcm283x.dtsi#L185
>
> I keep referring to RPi because we've used it a bit more than other boards,
> and also because for RPi usually Pin 25 in HW is also Pin 25 in SW.
>
>
> > https://www.kernel.org/doc/html/v4.17/driver-api/gpio/board.html that
> > shows how
> > to declare table attributes, but we couldn't find this lookup table
> > there.
> >
> > Are we missing something? Out of curiosity, why do we have to pass a
> > string
> > (e.g. powerdown, gain, sck, dout) instead of the pin number? We found
> > somewhere
> > that they are names to functions. Are these functions implemented on the
> > chip?
> >
>
> So, the string in the driver, will be used to lookup the physical pin in
> the device-tree.
> The pin number differs from one host-board to another [as I've said], so
> the string is a unique identifier for the driver, which resolves to the
> physical pin on the board.
>
> By the way: one work item (maybe after the driver is moved out of staging)
> would be to also write a device-tree binding doc.
> An example:
>
> https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/sound/adi,adau17x1.txt
> It's not the best one, and feel free to look for others as well, that
> define GPIOs. But in that file, is an example of how to link a reset-gpio
> that would be called by that driver to physically reset the device when
> probed & initialized.
>
> Usually the arch/arm[64]/boot/dts have a lot of device-trees that may help
> shape some thinking about device-trees.
>
> I'm hoping I got to explain things somewhat.
> It is a bit late in the afternoon [for me], but I thought I'd give it a try
> :)
>
> If there are still questions, feel free to ask.
> I think I can get to them tomorrow.
>
> Thanks
> Alex
>
> > Thanks,
> > Renato
> >

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Questions related to some drivers
  2018-11-13 13:56                   ` Giuliano Augusto Faulin Belinassi
@ 2018-11-14  9:55                     ` Ardelean, Alexandru
  2018-11-16 11:38                       ` Jonathan Cameron
  0 siblings, 1 reply; 14+ messages in thread
From: Ardelean, Alexandru @ 2018-11-14  9:55 UTC (permalink / raw)
  To: giuliano.belinassi
  Cc: giuliano.belinassi, renatogeh, linux-iio, kernel-usp, jic23

T24gVHVlLCAyMDE4LTExLTEzIGF0IDExOjU2IC0wMjAwLCBHaXVsaWFubyBBdWd1c3RvIEZhdWxp
biBCZWxpbmFzc2kgd3JvdGU6DQo+IEhlbGxvLA0KPiAgICAgIFdlIGhhdmUgc29tZSBkb3VidHMg
cmVnYXJkaW5nIHRoZSBGSUxURVIgYW5kIEdBSU4gcGlucy4gU2hvdWxkDQo+IHRoZXNlIHBpbnMg
YmUgc2V0IGluIHVzZXJzcGFjZSwgYW5kIHRoZXJlZm9yZSB0aGVyZSBtdXN0IGJlIGENCj4gJ2Fk
Nzc4MF93cml0ZV9yYXcnIGZ1bmN0aW9uPyBPciBzaG91bGQgd2UgdXNlIHRoZSBkZXZpY2UgdHJl
ZSB3aXRoIGENCj4gdmFsdWUgc2V0IG9uIGluaXRpYWxpemF0aW9uPw0KPiANCg0KU28sIGRlZmlu
aXRlbHkgYWRkIHRoZSBjb2RlIHRoYXQgaW5pdGlhbGl6ZXMgdGhlIHBpbnMgdmlhIGRldmljZS10
cmVlLg0KUXVpdGUgYSBmZXcgdXNlcnMgb2YgdGhlIGRyaXZlciBtYXkgb25seSB3YW50IGEgc2lu
Z2xlIGdhaW4gdmFsdWUgZm9yIHRoZQ0KZW50aXJlIGxpZmUtY3ljbGUgb2YgdGhlIGNoaXAgcnVu
bmluZyA7IGFuZCB0aGlzIGNhbiBiZSBuZWF0bHkgaW5pdGlhbGl6ZWQNCmZyb20gRFQuDQoNCkFk
ZGluZyBhICdhZDc3ODBfd3JpdGVfcmF3JyBmdW5jdGlvbiBmb3IgdXBkYXRpbmcgdGhlc2UgcGlu
cywgYWxzbyBzb3VuZHMNCmxpa2UgYSBuZWF0IGlkZWEuDQpUaGVuIHlvdSBjb3VsZCB1cGRhdGUg
dGhlIEdBSU4gYW5kIEZJTFRFUiBwaW5zIHZpYSB1c2VyLXNwYWNlIGNhbGxzLg0KSSBndWVzcyBp
dCdzIHVwIHRvIHlvdSBpZiB5b3Ugd2FudCB0byBkbyB0aGlzLiBCdXQgSSBkb24ndCBrbm93IGhv
dyBtYW55DQp1c2VycyBvZiB0aGUgZHJpdmVyIHdvdWxkIHJlcXVpcmUvd2FudCB0aGlzIGZsZXhp
YmlsaXR5IHdpdGggdGhpcw0KcGFydGljdWxhciBjaGlwIFt3ZSBoYXZlbid0IHJlY2VpdmVkIGFu
eSByZXF1ZXN0cyBmb3IgdGhpcyAoeWV0KSBBRkFJS10uDQoNCkRvaW5nIHRoaXMgaXMgYSBiaXQg
bW9yZSB3b3JrLCBiZWNhdXNlIHlvdSdsbCBoYXZlIHRvIGNyZWF0ZSBhIG5ldyBtYWNybw0KbGlr
ZSBBRF9TRF9DSEFOTkVMKCkgd2hpY2ggYWxsb3dzIHRvIHNldCBjdXN0b20gYml0L21hc2sgZmll
bGRzIFsxXSwgb3INCmp1c3QgY3JlYXRlIGEgQURfU0RfQ0hBTk5FTF9HQUlOKCkgb3IgQURfU0Rf
Q0hBTk5FTF9HQUlOX0ZJTFRFUigpIG1hY3JvLg0KWyBzZWUgaW5jbHVkZS9saW51eC9paW8vYWRj
L2FkX3NpZ21hX2RlbHRhLmggZm9yIEFEX1NEX0NIQU5ORUwoKSBdLg0KDQpJIGd1ZXNzIHRoZSBb
MV0gdmFyaWFudCB3b3VsZCBiZSBtb3JlIGZsZXhpYmxlL2ludGVyZXN0aW5nLg0KVGhlIEFENzc4
MCB1c2VzIHF1aXRlIGEgYml0IG9mIGxvZ2ljIGZyb20gdGhlIGdlbmVyaWMgYWRfc2lnbWFfZGVs
dGENCmRyaXZlci4NCg0KPiBUaGFuayB5b3UNCj4gT24gVGh1LCBOb3YgOCwgMjAxOCBhdCAxMjo0
NiBQTSBBcmRlbGVhbiwgQWxleGFuZHJ1DQo+IDxhbGV4YW5kcnUuQXJkZWxlYW5AYW5hbG9nLmNv
bT4gd3JvdGU6DQo+ID4gDQo+ID4gT24gVGh1LCAyMDE4LTExLTA4IGF0IDEyOjA0IC0wMjAwLCBS
ZW5hdG8gTHVpIEdlaCB3cm90ZToNCj4gPiA+IEhpLA0KPiA+ID4gDQo+ID4gDQo+ID4gSGV5LA0K
PiA+IA0KPiA+ID4gT24gMTEvMDYsIEFyZGVsZWFuLCBBbGV4YW5kcnUgd3JvdGU6DQo+ID4gPiA+
IEZvciBBRDc3OHggeW91IGNvdWxkIGFsc28gYWRkIHN1cHBvcnQgZm9yIHRoZSBHQUlOICYgRklM
VEVSIHBpbnMNCj4gPiA+ID4gdmlhDQo+ID4gPiA+IEdQSU9zLA0KPiA+ID4gPiB0byBjb250cm9s
IHRoZXNlIHNldHRpbmdzLiBBbmQgdGhlbiBpbiB0aGUNCj4gPiA+ID4gYWQ3NzgwX3Bvc3Rwcm9j
ZXNzX3NhbXBsZSgpDQo+ID4gPiA+IGZ1bmN0aW9uIGNoZWNrIGlmIHRoZSBHUElPIHNldHRpbmdz
IChzdG9yZWQgb24gYSBncGlvX2Rlc2Mgb24NCj4gPiA+ID4gYWQ3NzgwX3N0YXRlKQ0KPiA+ID4g
PiBtYXRjaCB0aGUgZXhwZWN0ZWQgR0FJTiAmIEZJTFRFUiBiaXQgc2V0dGluZ3MgOyBpZiBub3Qg
cmV0dXJuIC1FSU8uDQoNCkJ5IHRoZSB3YXk6IGxvb2tpbmcgYXQgdGhpcyBzdWdnZXN0aW9uIG5v
dywgSSdtIG5vdCBzdXJlIFtub3ddIGlmIGl0J3MgYQ0KZ29vZCBpZGVhIHRvIHJlYWQgR1BJT3Mg
aW4gdGhlIGFkNzc4MF9wb3N0cHJvY2Vzc19zYW1wbGUoKSBmdW5jdGlvbiBhcyB0aGF0DQpjb3Vs
ZCBzbG93IGRvd24gdGhlIGRhdGEtcmF0ZSB3aGVuIHJlYWRpbmcgZnJvbSB0aGUgQURDLg0KU28s
IEkgd291bGQgZHJvcCB0aGlzIGlkZWEuDQoNCj4gPiA+IA0KPiA+ID4gV2UncmUgaGF2aW5nIHNv
bWUgdHJvdWJsZSB3aXRoIHRoZSBHUElPcywgYW5kIHdvdWxkIGxpa2Ugc29tZSBpbnNpZ2h0DQo+
ID4gPiBvbg0KPiA+ID4gaG93IHRvIHByb2NlZWQuIEFueSBoZWxwIHdvdWxkIGJlIHZlcnkgbXVj
aCBhcHByZWNpYXRlZCENCj4gPiA+IA0KPiA+ID4gV2UncmUgd29uZGVyaW5nIGlmIHdlIHNob3Vs
ZCBkbyBzb21ldGhpbmcgbGlrZSB0aGlzIGluIGFkNzc4MC5jJ3MNCj4gPiA+IHByb2JlOg0KPiA+
ID4gDQo+ID4gPiBzdC0+cG93ZXJkb3duX2dwaW8gPSBkZXZtX2dwaW9kX2dldF9vcHRpb25hbCgm
c3BpLT5kZXYsDQo+ID4gPiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgInBvd2VyZG93biIsDQo+ID4gPiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgR1BJT0RfT1VUX0xPVyk7DQo+ID4gPiANCj4gPiANCj4gPiBZZXMsIHNvbWV0aGlu
ZyBsaWtlIHRoaXMuDQo+ID4gDQo+ID4gPiBmb3IgYm90aCBnYWluIGFuZCBmaWx0ZXIuIFRha2lu
ZyBhIGxvb2sgYXQgZHJpdmVyDQo+ID4gPiBkcml2ZXJzL2lpby9hZGMvaHg3MTEuYw0KPiA+ID4g
KGFub3RoZXIgYWRjIGRyaXZlciBvdXQgb2Ygc3RhZ2luZyksIHdlIGhhdmU6DQo+ID4gPiANCj4g
PiA+IGh4NzExX2RhdGEtPmdwaW9kX3BkX3NjayA9IGRldm1fZ3Bpb2RfZ2V0KGRldiwgInNjayIs
IEdQSU9EX09VVF9MT1cpOw0KPiA+ID4gDQo+ID4gPiBTbyB3ZSdyZSBhc3N1bWluZyAic2NrIiBh
bmQgInBvd2VyZG93biIgYXJlIHRoZSBwaW5zIHdlJ3JlIGxvb2tpbmcNCj4gPiA+IGZvci4NCj4g
PiA+IEFyZSB3ZQ0KPiA+ID4gY29ycmVjdCB0byBhc3N1bWUgdGhhdCB0aGVzZSBzdHJpbmdzIGFy
ZSBjb21wYXJlZCB3aXRoIGEgdGFibGUgdGhhdA0KPiA+ID4gbWFwDQo+ID4gPiB0aGUNCj4gPiA+
IGFjdHVhbCBHUElPIHBpbnM/IFNvIHdlJ2QgaGF2ZSBzb21ldGhpbmcgbGlrZToNCj4gPiA+IA0K
PiA+ID4gc3QtPmdhaW5fZ3BpbyA9IGRldm1fZ3Bpb2RfZ2V0X29wdGlvbmFsKCZzcGktPmRldiwN
Cj4gPiA+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgImdhaW4iLA0KPiA+
ID4gICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBHUElPX0RPVVRfTE9XKTsN
Cj4gPiANCj4gPiBUaGlzIGlzIGV4YWN0bHkgYSBnb29kIGlkZWEgdG8gZG8gZm9yIGBnYWluYC4N
Cj4gPiBBbmQgc2ltaWxhciBmb3IgYGZpbHRlcmANCj4gPiANCj4gPiA+IA0KPiA+ID4gV2hlcmUg
ImdhaW4iIGlzIHRoZSBwaW4gNCBvciA1IG9uIHRoZSBBRDc3OHgNCj4gPiANCj4gPiBUaGUgcGlu
IDQgb3IgNSBvbiB0aGUgQUQ3Nzh4IGFyZSBub3Qgd2hhdCB5b3UgZGVmaW5lIGhlcmUuDQo+ID4g
VGhlIEdQSU9zIHlvdSBkZWZpbmUvdXNlIGhlcmUgYXJlIG9uIHRoZSBob3N0LWJvYXJkLg0KPiA+
IA0KPiA+IFNvLA0KPiA+IA0KPiA+IEhvc3QtYm9hcmQgW2R0LWVudHJ5XSA8LS0tLT4gQUQ3Nzh4
IFtIVy1waW5dDQo+ID4gDQo+ID4gV2hlcmU6DQo+ID4gKiBgZHQtZW50cnlgIGlzIGRlZmludGVk
IGluIHRoZSBkZXZpY2UtdHJlZSBzb21ldGhpbmcgbGlrZToNCj4gPiAgICBnYWluLWdwaW8gPSA8
JmdwaW8gR1BJT19OVU1CRVJfT05fSE9TVF9CT0FSRCBHUElPX0ZMQUdTPg0KPiA+ICogR1BJT19O
VU1CRVJfT05fSE9TVF9CT0FSRCBpcyBhIG51bWJlciwgd2hpY2ggdmFyaWVzIDsgZm9yIGV4YW1w
bGUgZm9yDQo+ID4gUmFzcGJlcnJ5IFBpIGl0IGNvdWxkIGJlIHBpbiAyNSBbaW4gdGhlIGRldmlj
ZSB0cmVlXTsNCj4gPiAqIEdQSU9fRkxBR1MgaXMgdXN1YWxseSAwIDsgYnV0IGNhbiBiZSBhbnkg
b3RoZXIgbnVtZXJpYyB2YWx1ZSBkZWZpbmVkDQo+ID4gaGVyZToNCj4gPiANCj4gPiANCmh0dHBz
Oi8vZ2l0aHViLmNvbS90b3J2YWxkcy9saW51eC9ibG9iL21hc3Rlci9pbmNsdWRlL2R0LWJpbmRp
bmdzL2dwaW8vZ3Bpby5oDQo+ID4gKiBgSFctcGluYCBkb2Vzbid0IG1hdHRlciwgYmVjYXVzZSBp
dCdzIGEgcGh5c2ljYWwgcGluIHRoYXQgY2FuIGJlDQo+ID4gZGVmaW5lZA0KPiA+IGFzIGFueXRo
aW5nIGluIFNXDQo+ID4gDQo+ID4gPiAoDQo+ID4gPiANCmh0dHBzOi8vd3d3LmFuYWxvZy5jb20v
bWVkaWEvZW4vdGVjaG5pY2FsLWRvY3VtZW50YXRpb24vZGF0YS1zaGVldHMvYWQ3NzgwLnBkZg0K
PiA+ID4gKQ0KPiA+ID4gY2hpcC4NCj4gPiA+IA0KPiA+ID4gV2hlcmUgY2FuIHdlIGZpbmQgdGhp
cyB0YWJsZSB0aGF0IG1hcHMgdGhlc2UgcGluIG5hbWVzIHRvIHRoZSBhY3R1YWwNCj4gPiA+IHBp
bg0KPiA+ID4gbnVtYmVycz8gV2UgZm91bmQgdGhpcyBsaW5rDQo+ID4gDQo+ID4gVGhlIGFjdHVh
bCBwaW4gbnVtYmVyIGRvZXNuJ3QgbWF0dGVyIGluIHRoZSBkcml2ZXIgeW91IHdyaXRlLg0KPiA+
IEl0IG1hdHRlcnMgaW4gdGhlIGRldmljZS10cmVlIGZvciB0aGUgYm9hcmQgeW91IHVzZSB0aGUg
ZHJpdmVyIFthbmQNCj4gPiBjaGlwXQ0KPiA+IG9uLg0KPiA+IA0KPiA+IElmIHlvdSB3ZXJlIHRv
IHRlc3QgeW91ciBkcml2ZXIgY2hhbmdlIG9uIGEgUlBpIGJvYXJkIGFuZCB5b3UNCj4gPiBwaHlz
aWNhbGx5DQo+ID4gY29ubmVjdCB0aGlzIG9uIFBpbiAyNSAocGh5c2ljYWwpIGFuZCBpbiB5b3Vy
IGRldmljZS10cmVlDQo+ID4gR1BJT19OVU1CRVJfT05fSE9TVF9CT0FSRCBpcyB0aGUgdmFsdWUg
KGluIFNXKSBmb3IgUGluIDI1LCB0aGVuIHRoYXQncw0KPiA+IHdoYXQNCj4gPiBtYXR0ZXJzIHdo
ZW4geW91IHRlc3QvcnVuIHRoZSBjb2RlLg0KPiA+IA0KPiA+IEluIHRoZSBrZXJuZWwsIGEgcGlu
IG51bWJlcnMgdGFibGUgZm9yIHRoZSBSUGkgY2FuIGJlIGZvdW5kIGhlcmU6DQo+ID4gDQo+ID4g
DQpodHRwczovL2dpdGh1Yi5jb20vdG9ydmFsZHMvbGludXgvYmxvYi9tYXN0ZXIvYXJjaC9hcm0v
Ym9vdC9kdHMvYmNtMjgzeC5kdHNpI0wxODUNCj4gPiANCj4gPiBJIGtlZXAgcmVmZXJyaW5nIHRv
IFJQaSBiZWNhdXNlIHdlJ3ZlIHVzZWQgaXQgYSBiaXQgbW9yZSB0aGFuIG90aGVyDQo+ID4gYm9h
cmRzLA0KPiA+IGFuZCBhbHNvIGJlY2F1c2UgZm9yIFJQaSB1c3VhbGx5IFBpbiAyNSBpbiBIVyBp
cyBhbHNvIFBpbiAyNSBpbiBTVy4NCj4gPiANCj4gPiANCj4gPiA+IGh0dHBzOi8vd3d3Lmtlcm5l
bC5vcmcvZG9jL2h0bWwvdjQuMTcvZHJpdmVyLWFwaS9ncGlvL2JvYXJkLmh0bWwgdGhhdA0KPiA+
ID4gc2hvd3MgaG93DQo+ID4gPiB0byBkZWNsYXJlIHRhYmxlIGF0dHJpYnV0ZXMsIGJ1dCB3ZSBj
b3VsZG4ndCBmaW5kIHRoaXMgbG9va3VwIHRhYmxlDQo+ID4gPiB0aGVyZS4NCj4gPiA+IA0KPiA+
ID4gQXJlIHdlIG1pc3Npbmcgc29tZXRoaW5nPyBPdXQgb2YgY3VyaW9zaXR5LCB3aHkgZG8gd2Ug
aGF2ZSB0byBwYXNzIGENCj4gPiA+IHN0cmluZw0KPiA+ID4gKGUuZy4gcG93ZXJkb3duLCBnYWlu
LCBzY2ssIGRvdXQpIGluc3RlYWQgb2YgdGhlIHBpbiBudW1iZXI/IFdlIGZvdW5kDQo+ID4gPiBz
b21ld2hlcmUNCj4gPiA+IHRoYXQgdGhleSBhcmUgbmFtZXMgdG8gZnVuY3Rpb25zLiBBcmUgdGhl
c2UgZnVuY3Rpb25zIGltcGxlbWVudGVkIG9uDQo+ID4gPiB0aGUNCj4gPiA+IGNoaXA/DQo+ID4g
PiANCj4gPiANCj4gPiBTbywgdGhlIHN0cmluZyBpbiB0aGUgZHJpdmVyLCB3aWxsIGJlIHVzZWQg
dG8gbG9va3VwIHRoZSBwaHlzaWNhbCBwaW4NCj4gPiBpbg0KPiA+IHRoZSBkZXZpY2UtdHJlZS4N
Cj4gPiBUaGUgcGluIG51bWJlciBkaWZmZXJzIGZyb20gb25lIGhvc3QtYm9hcmQgdG8gYW5vdGhl
ciBbYXMgSSd2ZSBzYWlkXSwNCj4gPiBzbw0KPiA+IHRoZSBzdHJpbmcgaXMgYSB1bmlxdWUgaWRl
bnRpZmllciBmb3IgdGhlIGRyaXZlciwgd2hpY2ggcmVzb2x2ZXMgdG8gdGhlDQo+ID4gcGh5c2lj
YWwgcGluIG9uIHRoZSBib2FyZC4NCj4gPiANCj4gPiBCeSB0aGUgd2F5OiBvbmUgd29yayBpdGVt
IChtYXliZSBhZnRlciB0aGUgZHJpdmVyIGlzIG1vdmVkIG91dCBvZg0KPiA+IHN0YWdpbmcpDQo+
ID4gd291bGQgYmUgdG8gYWxzbyB3cml0ZSBhIGRldmljZS10cmVlIGJpbmRpbmcgZG9jLg0KPiA+
IEFuIGV4YW1wbGU6DQo+ID4gDQo+ID4gDQpodHRwczovL2dpdGh1Yi5jb20vdG9ydmFsZHMvbGlu
dXgvYmxvYi9tYXN0ZXIvRG9jdW1lbnRhdGlvbi9kZXZpY2V0cmVlL2JpbmRpbmdzL3NvdW5kL2Fk
aSxhZGF1MTd4MS50eHQNCj4gPiBJdCdzIG5vdCB0aGUgYmVzdCBvbmUsIGFuZCBmZWVsIGZyZWUg
dG8gbG9vayBmb3Igb3RoZXJzIGFzIHdlbGwsIHRoYXQNCj4gPiBkZWZpbmUgR1BJT3MuIEJ1dCBp
biB0aGF0IGZpbGUsIGlzIGFuIGV4YW1wbGUgb2YgaG93IHRvIGxpbmsgYSByZXNldC0NCj4gPiBn
cGlvDQo+ID4gdGhhdCB3b3VsZCBiZSBjYWxsZWQgYnkgdGhhdCBkcml2ZXIgdG8gcGh5c2ljYWxs
eSByZXNldCB0aGUgZGV2aWNlIHdoZW4NCj4gPiBwcm9iZWQgJiBpbml0aWFsaXplZC4NCj4gPiAN
Cj4gPiBVc3VhbGx5IHRoZSBhcmNoL2FybVs2NF0vYm9vdC9kdHMgaGF2ZSBhIGxvdCBvZiBkZXZp
Y2UtdHJlZXMgdGhhdCBtYXkNCj4gPiBoZWxwDQo+ID4gc2hhcGUgc29tZSB0aGlua2luZyBhYm91
dCBkZXZpY2UtdHJlZXMuDQo+ID4gDQo+ID4gSSdtIGhvcGluZyBJIGdvdCB0byBleHBsYWluIHRo
aW5ncyBzb21ld2hhdC4NCj4gPiBJdCBpcyBhIGJpdCBsYXRlIGluIHRoZSBhZnRlcm5vb24gW2Zv
ciBtZV0sIGJ1dCBJIHRob3VnaHQgSSdkIGdpdmUgaXQgYQ0KPiA+IHRyeQ0KPiA+IDopDQo+ID4g
DQo+ID4gSWYgdGhlcmUgYXJlIHN0aWxsIHF1ZXN0aW9ucywgZmVlbCBmcmVlIHRvIGFzay4NCj4g
PiBJIHRoaW5rIEkgY2FuIGdldCB0byB0aGVtIHRvbW9ycm93Lg0KPiA+IA0KPiA+IFRoYW5rcw0K
PiA+IEFsZXgNCj4gPiANCj4gPiA+IFRoYW5rcywNCj4gPiA+IFJlbmF0bw0KPiA+ID4gDQo+IA0K
PiANCg==

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Questions related to some drivers
  2018-11-14  9:55                     ` Ardelean, Alexandru
@ 2018-11-16 11:38                       ` Jonathan Cameron
  0 siblings, 0 replies; 14+ messages in thread
From: Jonathan Cameron @ 2018-11-16 11:38 UTC (permalink / raw)
  To: Ardelean, Alexandru
  Cc: giuliano.belinassi, giuliano.belinassi, renatogeh, linux-iio, kernel-usp

On Wed, 14 Nov 2018 09:55:08 +0000
"Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote:

> On Tue, 2018-11-13 at 11:56 -0200, Giuliano Augusto Faulin Belinassi wrote:
> > Hello,
> >      We have some doubts regarding the FILTER and GAIN pins. Should
> > these pins be set in userspace, and therefore there must be a
> > 'ad7780_write_raw' function? Or should we use the device tree with a
> > value set on initialization?
> >   
> 
> So, definitely add the code that initializes the pins via device-tree.
> Quite a few users of the driver may only want a single gain value for the
> entire life-cycle of the chip running ; and this can be neatly initialized
> from DT.

Actually I'll come down on the other side in that argument.  They are both
better set from just userspace, not in DT.  DT bindings for gain in particular
have always been controversial as it's is sometimes more of a policy decision
(and hence generally shouldn't be in DT) rather than one dictated by hardware.

However any defaults should then be conservative (minimum gain, widest filter)
so peoples till get a reasonable answer when not controlling these explicitly.

Now, if there is a 'strong' hardware reason for having a particular set of
value (typical one is the hardware is actually damaged if we go too far
out of range) then they can go in DT. Otherwise I would definitely prefer
userspace control only.

> 
> Adding a 'ad7780_write_raw' function for updating these pins, also sounds
> like a neat idea.
> Then you could update the GAIN and FILTER pins via user-space calls.
> I guess it's up to you if you want to do this. But I don't know how many
> users of the driver would require/want this flexibility with this
> particular chip [we haven't received any requests for this (yet) AFAIK].
> 
> Doing this is a bit more work, because you'll have to create a new macro
> like AD_SD_CHANNEL() which allows to set custom bit/mask fields [1], or
> just create a AD_SD_CHANNEL_GAIN() or AD_SD_CHANNEL_GAIN_FILTER() macro.
> [ see include/linux/iio/adc/ad_sigma_delta.h for AD_SD_CHANNEL() ].
> 
> I guess the [1] variant would be more flexible/interesting.
> The AD7780 uses quite a bit of logic from the generic ad_sigma_delta
> driver.
> 
> > Thank you
> > On Thu, Nov 8, 2018 at 12:46 PM Ardelean, Alexandru
> > <alexandru.Ardelean@analog.com> wrote:  
> > > 
> > > On Thu, 2018-11-08 at 12:04 -0200, Renato Lui Geh wrote:  
> > > > Hi,
> > > >   
> > > 
> > > Hey,
> > >   
> > > > On 11/06, Ardelean, Alexandru wrote:  
> > > > > For AD778x you could also add support for the GAIN & FILTER pins
> > > > > via
> > > > > GPIOs,
> > > > > to control these settings. And then in the
> > > > > ad7780_postprocess_sample()
> > > > > function check if the GPIO settings (stored on a gpio_desc on
> > > > > ad7780_state)
> > > > > match the expected GAIN & FILTER bit settings ; if not return -EIO.  
> 
> By the way: looking at this suggestion now, I'm not sure [now] if it's a
> good idea to read GPIOs in the ad7780_postprocess_sample() function as that
> could slow down the data-rate when reading from the ADC.
> So, I would drop this idea.

Should definitely avoid any actual hardware reads / writes as these may
well be on some gpio chip out on a slow bus.

> 
> > > > 
> > > > We're having some trouble with the GPIOs, and would like some insight
> > > > on
> > > > how to proceed. Any help would be very much appreciated!
> > > > 
> > > > We're wondering if we should do something like this in ad7780.c's
> > > > probe:
> > > > 
> > > > st->powerdown_gpio = devm_gpiod_get_optional(&spi->dev,
> > > >                                            "powerdown",
> > > >                                            GPIOD_OUT_LOW);
> > > >   
> > > 
> > > Yes, something like this.
> > >   
> > > > for both gain and filter. Taking a look at driver
> > > > drivers/iio/adc/hx711.c
> > > > (another adc driver out of staging), we have:
> > > > 
> > > > hx711_data->gpiod_pd_sck = devm_gpiod_get(dev, "sck", GPIOD_OUT_LOW);
> > > > 
> > > > So we're assuming "sck" and "powerdown" are the pins we're looking
> > > > for.
> > > > Are we
> > > > correct to assume that these strings are compared with a table that
> > > > map
> > > > the
> > > > actual GPIO pins? So we'd have something like:
> > > > 
> > > > st->gain_gpio = devm_gpiod_get_optional(&spi->dev,
> > > >                                       "gain",
> > > >                                       GPIO_DOUT_LOW);  
> > > 
> > > This is exactly a good idea to do for `gain`.
> > > And similar for `filter`
> > >   
> > > > 
> > > > Where "gain" is the pin 4 or 5 on the AD778x  
> > > 
> > > The pin 4 or 5 on the AD778x are not what you define here.
> > > The GPIOs you define/use here are on the host-board.
> > > 
> > > So,
> > > 
> > > Host-board [dt-entry] <----> AD778x [HW-pin]
> > > 
> > > Where:
> > > * `dt-entry` is definted in the device-tree something like:
> > >    gain-gpio = <&gpio GPIO_NUMBER_ON_HOST_BOARD GPIO_FLAGS>
> > > * GPIO_NUMBER_ON_HOST_BOARD is a number, which varies ; for example for
> > > Raspberry Pi it could be pin 25 [in the device tree];
> > > * GPIO_FLAGS is usually 0 ; but can be any other numeric value defined
> > > here:
> > > 
> > >   
> https://github.com/torvalds/linux/blob/master/include/dt-bindings/gpio/gpio.h
> > > * `HW-pin` doesn't matter, because it's a physical pin that can be
> > > defined
> > > as anything in SW
> > >   
> > > > (
> > > >   
> https://www.analog.com/media/en/technical-documentation/data-sheets/ad7780.pdf
> > > > )
> > > > chip.
> > > > 
> > > > Where can we find this table that maps these pin names to the actual
> > > > pin
> > > > numbers? We found this link  
> > > 
> > > The actual pin number doesn't matter in the driver you write.
> > > It matters in the device-tree for the board you use the driver [and
> > > chip]
> > > on.
> > > 
> > > If you were to test your driver change on a RPi board and you
> > > physically
> > > connect this on Pin 25 (physical) and in your device-tree
> > > GPIO_NUMBER_ON_HOST_BOARD is the value (in SW) for Pin 25, then that's
> > > what
> > > matters when you test/run the code.
> > > 
> > > In the kernel, a pin numbers table for the RPi can be found here:
> > > 
> > >   
> https://github.com/torvalds/linux/blob/master/arch/arm/boot/dts/bcm283x.dtsi#L185
> > > 
> > > I keep referring to RPi because we've used it a bit more than other
> > > boards,
> > > and also because for RPi usually Pin 25 in HW is also Pin 25 in SW.
> > > 
> > >   
> > > > https://www.kernel.org/doc/html/v4.17/driver-api/gpio/board.html that
> > > > shows how
> > > > to declare table attributes, but we couldn't find this lookup table
> > > > there.
> > > > 
> > > > Are we missing something? Out of curiosity, why do we have to pass a
> > > > string
> > > > (e.g. powerdown, gain, sck, dout) instead of the pin number? We found
> > > > somewhere
> > > > that they are names to functions. Are these functions implemented on
> > > > the
> > > > chip?
> > > >   
> > > 
> > > So, the string in the driver, will be used to lookup the physical pin
> > > in
> > > the device-tree.
> > > The pin number differs from one host-board to another [as I've said],
> > > so
> > > the string is a unique identifier for the driver, which resolves to the
> > > physical pin on the board.
> > > 
> > > By the way: one work item (maybe after the driver is moved out of
> > > staging)
> > > would be to also write a device-tree binding doc.
> > > An example:
> > > 
> > >   
> https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/sound/adi,adau17x1.txt
> > > It's not the best one, and feel free to look for others as well, that
> > > define GPIOs. But in that file, is an example of how to link a reset-
> > > gpio
> > > that would be called by that driver to physically reset the device when
> > > probed & initialized.
> > > 
> > > Usually the arch/arm[64]/boot/dts have a lot of device-trees that may
> > > help
> > > shape some thinking about device-trees.
> > > 
> > > I'm hoping I got to explain things somewhat.
> > > It is a bit late in the afternoon [for me], but I thought I'd give it a
> > > try
> > > :)
> > > 
> > > If there are still questions, feel free to ask.
> > > I think I can get to them tomorrow.
> > > 
> > > Thanks
> > > Alex
> > >   
> > > > Thanks,
> > > > Renato
> > > >   
> > 
> >   

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2018-11-16 21:50 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-17 19:00 Questions related to some drivers Giuliano Belinassi
2018-10-18  6:53 ` Ardelean, Alexandru
2018-10-18 11:41   ` Giuliano Belinassi
2018-10-22 21:56     ` Giuliano Augusto Faulin Belinassi
2018-10-23  9:51       ` Jonathan Cameron
2018-11-05 18:46         ` Giuliano Augusto Faulin Belinassi
2018-11-06 13:23           ` Ardelean, Alexandru
2018-11-07 18:57             ` Renato Lui Geh
2018-11-08 14:04               ` Renato Lui Geh
2018-11-08 14:46                 ` Ardelean, Alexandru
2018-11-09 22:08                   ` Giuliano Augusto Faulin Belinassi
2018-11-13 13:56                   ` Giuliano Augusto Faulin Belinassi
2018-11-14  9:55                     ` Ardelean, Alexandru
2018-11-16 11:38                       ` 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.