On Thu, 26 Aug 2021 10:27:34 +0800
hui.liu [off-list ref] wrote:

Hi Jonathan,

Our internal Kernel module CCCI use _RAW case, we described in the
previous mail. Could you give your suggestion?

Fix your internal module?  I would have assumed it would want
calibrated values if available, so it seems a little odd to use _RAW.
If there is a good reason for it doing so that you can talk about here
then perhaps we can consider it.

As a general rule we don't put any significant effort into supporting
out of tree drivers so I'm not very keen on adding _RAW on the basis
one uses it.

Jonathan

Thanks.
Hui

quoted

On Tue, 2021-08-17 at 16:54 +0100, Jonathan Cameron wrote:  

quoted

On Tue, 17 Aug 2021 16:37:52 +0800
hui.liu [off-list ref] wrote:
  

quoted

On Sat, 2021-08-14 at 17:10 +0100, Jonathan Cameron wrote:  

quoted

On Fri, 13 Aug 2021 11:46:24 +0800 hui.liu <  
hui.liu@mediatek.com>   
wrote:
    

quoted

On Thu, 2021-08-12 at 19:07 +0100, Jonathan Cameron wrote:    

quoted

On Thu, 12 Aug 2021 13:48:43 +0800 Hui Liu 
[off-list ref] wrote:
      

quoted

Add support IIO_CHAN_INFO_RAW case.      

Why?

We almost never support both RAW and PROCESSED as
userspace 
should be fine to use either.  There are a few reasons
we've 
let drivers do this but I would like know why it matters
to 
you and it definitely needs to be in the patch description.
      

Hi Jonathan,

1. To support ADC consumers' different types of requirement:
some
consumers want to call iio_read_channel_raw to get raw data,
the 
others use iio_read_channel_processed to get voltage.  

Give an example of the consumer using the raw channel readback 
(without acess to any scaling information?)
    

quoted

2. In our origin driver, if consumer call 
iio_read_channel_processed, read back value is raw data.

Could we use SCALE instead of PROCESSED in patch for next 
version, or what's your suggestion?  

That would unfortunately be a userspace ABI change.  We can
add 
interfaces but taking them away is normally a problem :(

Your reasons here are fine, subject to information on what 
consumer cares about having _RAW, please resend the patch with 
this information added to the description.

Thanks,

Jonathan  

1. We found afe/iio-rescale.c, dac/dpot-dac.c and
multiplexer/iio- 
mux.c call iio_read_channel_raw to get raw data. If they use our
ADC 
driver, I think we should support _RAW case.
If we support _RAW case, we will add more information in v2 
description.  

iio-rescale has recently gained support for processed.
  

https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/commit/drivers/iio/afe/iio-rescale.c?h=testing&id=53ebee9499805add3eef630d998c40812e6a1c39

quoted

quoted

dpot-dac is a rather obscure special case, so I doubt you actually 
have one of those.
If iio-mux is relevant then we should add processed support to
that 
driver as well.

I would rather see those users of the interface fixed than us
having 
to tweak lots of drivers to provide _raw when it isn't appropriate
for 
that piece of hardware.

Jonathan
  

Hi Jonathan,

For we have an internal use scenario:
Our internal Kernel module CCCI(Communication interface between AP
Core and Modem) would like get raw data from ADC, but CCCI driver
have not do upstream now.

If we no need add _RAW, we will only change _PROCESSED readback value
in the next version of patch.
  

quoted

quoted

2. Since we change _PROCESSED readback value from raw data to 
voltage, our consumer will make the changes synchronously.
  

quoted

quoted

Thanks.
    

quoted

quoted

Signed-off-by: Hui Liu <redacted>
---
 drivers/iio/adc/mt6577_auxadc.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/mt6577_auxadc.c 

b/drivers/iio/adc/mt6577_auxadc.c index 
79c1dd68b909..e995d43287b2 100644

--- a/drivers/iio/adc/mt6577_auxadc.c
+++ b/drivers/iio/adc/mt6577_auxadc.c

@@ -60,7 +60,8 @@ static const struct

mtk_auxadc_compatible 
mt6765_compat = {
 		.type = IIO_VOLTAGE,				
   
 \
 		.indexed = 1,					
   
 \
 		.channel = (idx),				
    \
-		.info_mask_separate =
BIT(IIO_CHAN_INFO_PROCESSED), \
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	
   
 \
+				      BIT(IIO_CHAN_INFO_PROCESS
ED), \
 }
 
 static const struct iio_chan_spec 
mt6577_auxadc_iio_channels[] = { @@ -181,6 +182,19 @@
static 
int mt6577_auxadc_read_raw(struct iio_dev *indio_dev,
 	struct mt6577_auxadc_device *adc_dev = 
iio_priv(indio_dev);
 
 	switch (info) {
+	case IIO_CHAN_INFO_RAW:
+		*val = mt6577_auxadc_read(indio_dev, chan);
+		if (*val < 0) {
+			dev_notice(indio_dev->dev.parent,
+				"failed to sample data on
channel[%d]\n",
+				chan->channel);
+			return *val;
+		}
+		if (adc_dev->dev_comp->sample_data_cali)
+			*val = mt_auxadc_get_cali_data(*val,
true);
+
+		return IIO_VAL_INT;
+
 	case IIO_CHAN_INFO_PROCESSED:
 		*val = mt6577_auxadc_read(indio_dev, chan);
 		if (*val < 0) {      

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel