spear_adc driver help needed ?
Hello Jonathan,
I've been working on a SPEAr600 platform recently, which uses the spear_adc driver, and was about to start working on moving the driver out of staging... only to discover in 4.12-rc1 that you already moved it out of staging.
In the commit log that moves it out of staging, you're saying:
There are some unanswered questions due to disagreements between the code and various datasheets (including between different datasheets for the same part).
I don't think that is necessarily a reason to keep it in staging however. I'm partly posting this patch inorder to reignite debate and with a bit of luck find someone who has one of these to test!
What are the unanswered questions you are talking about?
Also, I am currently using the driver on a SPEAr600 device, so I can do some testing.
The driver has support for the SPEAr3xx, but it isn't used anywhere:
there is no compatible string for it, and nowhere is old-style probing used for "spear-adc".
So I'm thinking of:
- Dropping SPEAr3xx support, because it's unused. This will also remove the ugly checks done on the compatible string all over the place.
- Moving to a "base + offset" style of register accesses, rather than the struct-based accesses.
What do you think ?
Best regards,
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Jonathan Cameron
maintainerQuoted 6 lines · Thomas Petazzoni
Hello Jonathan,
I've been working on a SPEAr600 platform recently, which uses the
spear_adcdriver, and was about to start working on moving the driver out of staging... only to discover in 4.12-rc1 that you already moved it out of staging.
Yeah, this one hit the question of:
Is it is good enough to move it or should we bin it as no improvements are really happening and no one seems to have one?
I went with the it's good enough option being an optimist.
Quoted 7 lines
In the commit log that moves it out of staging, you're saying:
There are some unanswered questions due to disagreements between the code and various datasheets (including between different datasheets for the same part).
Quoted 5 lines
I don't think that is necessarily a reason to keep it in staging however. I'm partly posting this patch inorder to reignite debate and with a bit of luck find someone who has one of these to test!
What are the unanswered questions you are talking about?
These all came up in the previous attempt to move the driver out of staging back in 2014
http://www.spinics.net/lists/linux-iio/msg12443.html
In particular the results of Harmut tracking down a datahsheet and going through some elements of it in detail.
Quoted 3 lines
Also, I am currently using the driver on a SPEAr600 device, so I can do some testing.
Cool.
Quoted 4 lines
The driver has support for the SPEAr3xx, but it isn't used anywhere:
there is no compatible string for it, and nowhere is old-style probing used for "spear-adc".
Stefan (cc'd) might know who is likely to be using it...
However after a bit of digging in the history, his original patch said that it was untested and proper support might eventually be added.
Quoted 6 lines
So I'm thinking of:
- Dropping SPEAr3xx support, because it's unused. This will also remove the ugly checks done on the compatible string all over the place.
I agree.
Quoted 3 lines
- Moving to a "base + offset" style of register accesses, rather than the struct-based accesses.
Yeah, with the 3xx stuff gone that would tidy things up further.
Only exception would be if someone rapidly jumps up and down and says they use the 3xx support as is or have working patches they were aiming to submit to make it work.
Thanks for working on this. I'd forgotten that stuff was even in there!
Quoted 2 lines
What do you think ?
Would be great to tidy this up, particularly as it sounds like there
are actually users. Sometimes I wonder on some of the stuff I've
lifted out of staging over the years ;)
Jonathan
Quoted 5 lines
Best regards,
Thomas
Quoted 38 lines · Jonathan Cameron
Quoted 6 lines · Thomas Petazzoni
Hello Jonathan,
I've been working on a SPEAr600 platform recently, which uses the
spear_adcdriver, and was about to start working on moving the driver out of staging... only to discover in 4.12-rc1 that you already moved it out of staging.Yeah, this one hit the question of:
Is it is good enough to move it or should we bin it as no improvements are really happening and no one seems to have one?I went with the it's good enough option being an optimist.
Quoted 14 lines
In the commit log that moves it out of staging, you're saying:
There are some unanswered questions due to disagreements between
the code
and various datasheets (including between different datasheets
for the same
part).
I don't think that is necessarily a reason to keep it in
staging however.
I'm partly posting this patch inorder to reignite debate and with
a bit
of luck find someone who has one of these to test!Quoted 1 line
What are the unanswered questions you are talking about?
These all came up in the previous attempt to move the driver out of staging back in 2014
http://www.spinics.net/lists/linux-iio/msg12443.html
In particular the results of Harmut tracking down a datahsheet and going through some elements of it in detail.Quoted 3 lines
Also, I am currently using the driver on a SPEAr600 device, so I can do some testing.
Cool.
Quoted 1 line
Quoted 7 lines
Quoted 3 lines
The driver has support for the SPEAr3xx, but it isn't used anywhere:
there is no compatible string for it, and nowhere is old-style probing used for "spear-adc".Stefan (cc'd) might know who is likely to be using it...
However after a bit of digging in the history, his original patch said that it was untested and proper support might eventually be added.
This is also my understanding (that's so looong ago, sorry). Somebody suggested / requested to add support support for SPEAr3xx to this driver as well. I definitely never tested it on this platform and don't remember anybody who did.
Quoted 16 lines
Quoted 6 lines
So I'm thinking of:
- Dropping SPEAr3xx support, because it's unused. This will also remove the ugly checks done on the compatible string all over the place.
I agree.
Quoted 3 lines
- Moving to a "base + offset" style of register accesses, rather than the struct-based accesses.
Yeah, with the 3xx stuff gone that would tidy things up further.
Only exception would be if someone rapidly jumps up and down and says they use the 3xx support as is or have working patches they were aiming to submit to make it work.
Quoted 6 lines
Thanks for working on this. I'd forgotten that stuff was even in there!
Quoted 2 lines
What do you think ?
Would be great to tidy this up, particularly as it sounds like there
are actually users. Sometimes I wonder on some of the stuff I've
lifted out of staging over the years ;)
From the history above, its quite likely that this driver has never been tested / used on SPEAr3xx. So I vote for dropping this support.
Thanks,
Stefan