On 20 October 2016 13:55:12 BST, Lars-Peter Clausen <lars@xxxxxxxxxx> wrote: >On 10/20/2016 11:25 AM, Peter Rosin wrote: >> Hi! >> >> These two drivers share the fact that they wrap another iio channel, >> and I use the first in combination with the second, which is why I'm >> submitting them as a pair. >> >> The first driver is a simple wrapper converting an iio dpot into an >> iio dac. It only changes the unit and scale. It also does not add any >> fancy iio buffer support that I don't need. I suppose that can be >> added. By someone else :-) >> >> Please look over the scale conversion, notably for the fractional >log2 >> case that I don't need myself, so is untested. Maybe I should just >> remove it? >> >> Also, is there some agreed-upon way to dig out the maximum value from >> an iio channel? If so, "dpot-dac,max-ohms" can be eliminated from the >> dt bindings, which would have been nice... > >Yes, this is something we could really use. In a sense it exists for >the >devices with buffer-capable channels where there is the real_bits field >which tells us the data width of the channel. But a dedicated mechanism >for >querying the maximum (and minimum) valid code seems like a useful >feature. >Not only for in-kernel clients, but also for userspace. This was something that was addressed by the rather ancient patch series i posted that added an available call back which provided info on range and values for all info mask elements. Series got buried by there being a lot of precursors but quite a few of those have merged since. Hmm Google won't let me find it on my phone. Was a while back now. Will try to get on pc with decent email archive later and dig out a reference. > >> >> I'm also wondering if I'm somehow abusing the regulator? I only added >> it to get rid of a "dpot-dac,max-voltage" thing from the dt bindings. >> It feels right though, but maybe I should do more with it than check >> its voltage? What? > >Enable the regulator when it is in use? > >> >> The second driver (the envelope detector) is more involved. It also >> explains why I need the dpot-dac driver. I wanted the envelope >> detector to be generic and work with any dac, but I had a dpot... >> >> The envelope detector was previously discussed late last year [1], >> and this is what I came up with instead of that mess. >> >> There are a couple of things to be said about the envelope detector, >> one question is where it should live? I placed it in the adc >directory, >> but maybe it deserves an iio directory of its own? I'm also a bit >> worried that the name is a wee bit too generic. But what is a good >> name? I don't want it to be too long like dac-comp-envelope-detector >> and something like dac-comp-env-det is just unreadable. Naming is >> difficult... And suggestions? > >Yeah, it is a bit tricky. It is a envelope detector built from discrete >components, but of course there are many more ways to build one. If you >have >a codename for your platform you could use this for the DT compatible >string, like 'vendor,foobar-envelope-detector'. > >> >> Another thing is that I'm not 100% satisfied with the fact that you >> have to decide at instantiation if you are going to invert the search >> or not (search from below). But in order for that to be selectable >> at runtime with a channel attribute of some sort, I need to be able >> to rebind the interrupt to the other edge and I want to do that >> without releasing the irq and grabbing it again (someone might >> otherwise steal the irq, making the driver lose the irq all >together). >> I don't see any API to change the irq trigger condition. Is there >> such a thing? >> >> Anyway, despite all the above questions and remarks, this works for >> me. Please consider applying. > >In general this series looks really good, good and clear implementation >as >well as documentation. A few minor bits here and there, but that is >normal. -- Sent from my Android device with K-9 Mail. Please excuse my brevity. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
