LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: "hui.liu" <hui.liu@mediatek.com>
Cc: Jonathan Cameron <Jonathan.Cameron@Huawei.com>,
	<robh+dt@kernel.org>, <lars@metafoo.de>, <pmeerw@pmeerw.net>,
	<srv_heupstream@mediatek.com>, <zhiyong.tao@mediatek.com>,
	<chun-hung.wu@mediatek.com>, <yingjoe.chen@mediatek.com>,
	<seiya.wang@mediatek.com>, <matthias.bgg@gmail.com>,
	<s.hauer@pengutronix.de>, <devicetree@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-iio@vger.kernel.org>, <linux-mediatek@lists.infradead.org>
Subject: Re: [PATCH v1 1/2] iio: mtk-auxadc: add support IIO_CHAN_INFO_RAW case
Date: Sat, 14 Aug 2021 17:10:07 +0100	[thread overview]
Message-ID: <20210814171007.6892ae94@jic23-huawei> (raw)
In-Reply-To: <042625639032bffe73b60a5c6274511e58e34ef4.camel@mediatek.com>

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

> On Thu, 2021-08-12 at 19:07 +0100, Jonathan Cameron wrote:
> > On Thu, 12 Aug 2021 13:48:43 +0800
> > Hui Liu <hui.liu@mediatek.com> wrote:
> >   
> > > 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?)

> 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
> 
> Thanks.
> 
> > > 
> > > Signed-off-by: Hui Liu <hui.liu@mediatek.com>
> > > ---
> > >  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_PROCESSED), \
> > >  }
> > >  
> > >  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) {  
> > 
> >   


  parent reply	other threads:[~2021-08-14 16:08 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-12  5:48 [PATCH v1 0/2] AUXADC: Mediatek auxadc driver Hui Liu
2021-08-12  5:48 ` [PATCH v1 1/2] iio: mtk-auxadc: add support IIO_CHAN_INFO_RAW case Hui Liu
2021-08-12 18:07   ` Jonathan Cameron
     [not found]     ` <042625639032bffe73b60a5c6274511e58e34ef4.camel@mediatek.com>
2021-08-14 16:10       ` Jonathan Cameron [this message]
     [not found]         ` <470d4a7fbbcbcaa35aa9dcbaab6a2b77f98dc528.camel@mediatek.com>
2021-08-17 15:54           ` Jonathan Cameron
2021-08-12  5:48 ` [PATCH v1 2/2] iio: mtk-auxadc: update case IIO_CHAN_INFO_PROCESSED Hui Liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210814171007.6892ae94@jic23-huawei \
    --to=jic23@kernel.org \
    --cc=Jonathan.Cameron@Huawei.com \
    --cc=chun-hung.wu@mediatek.com \
    --cc=devicetree@vger.kernel.org \
    --cc=hui.liu@mediatek.com \
    --cc=lars@metafoo.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=matthias.bgg@gmail.com \
    --cc=pmeerw@pmeerw.net \
    --cc=robh+dt@kernel.org \
    --cc=s.hauer@pengutronix.de \
    --cc=seiya.wang@mediatek.com \
    --cc=srv_heupstream@mediatek.com \
    --cc=yingjoe.chen@mediatek.com \
    --cc=zhiyong.tao@mediatek.com \
    --subject='Re: [PATCH v1 1/2] iio: mtk-auxadc: add support IIO_CHAN_INFO_RAW case' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).