On 2020-12-13 13:16, Jonathan Cameron wrote: > On Sun, 13 Dec 2020 00:22:17 +0100 > Peter Rosin <peda@xxxxxxxxxx> wrote: > >> On 2020-12-12 13:26, Linus Walleij wrote: >>> On Mon, Nov 2, 2020 at 12:22 AM Linus Walleij <linus.walleij@xxxxxxxxxx> wrote: >>> >>>> It happens that an ADC will only provide raw or processed >>>> voltage conversion channels. (adc/ab8500-gpadc.c). >>>> On the Samsung GT-I9070 this is used for a light sensor >>>> and current sense amplifier so we need to think of something. >>>> >>>> The idea is to allow processed channels and scale them >>>> with 1/1 and then the rescaler can modify the result >>>> on top. >>>> >>>> Cc: Peter Rosin <peda@xxxxxxxxxx> >>>> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx> >>> >>> Did we reach any conclusion on this? I really need to use >>> the rescaler on an ADC that only handles processed channels... >>> >>> I'm sorry that I can't make this ADC disappear :D >> >> Hi! >> >> My conclusion was that the patch is buggy since it presents inconsistent >> information. That needs to be fixed one way or the other. If the offending >> information cannot be filtered out for some reason, I don't know what to >> do. Details in my previous comment [1]. BTW, I still do not know the answer >> to the .read_avail question at the end of that message, and I don't have >> time to dig into it. Sorry. > > Unless I'm missing something, I think it presents no information unless > we strangely have a driver providing read_avail for _RAW but only > _PROCESSED channels which is a bug. I'm not that bothered about > missing information in this particular, somewhat obscure, corner case. > > So I think we should take the patch as it stands. It's missed the > merge window now anyway unfortunately. So Peter, I would suggest we > take this and perhaps revisit to tidy up loose corners when we all have > more time. My concern was a driver with a raw channel, including read_avail, providing raw sample values but that no easy conversion existed to get from that to the processed values. One option for the driver in that case would be to provide these raw values, but then have no scaling info. I.e. the way I see it, it is perfectly reasonable for a driver to provide raw with read_avail, no scaling but also processed values. And that gets transformed by the rescaler into the processed values being presented as raw, with rescaling added on top, but with the read_avail info for this new raw channel being completely wrong. For the intended driver (ab8500-gpadc) this is not the case (it has no read_avail for its raw channel). But it does have a raw channel, so adding read_avail seems easy and I can easily see other drivers already doing it. Haven't checked that though... But if you say that this never happens, fine. Otherwise, since it's too late for the merge window anyway, the patch might as well be updated such that the rescaler blocks the read_avail channel in this situation, if it exists. Cheers, Peter
