Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
In-Reply-To: <20180825093841.72a5d099@archlinux>
References: <4a7e33457617e6f0f7c9627fe20ddd3039e4fe82.1535103920.git.baolin.wang@linaro.org>
 <20180825093841.72a5d099@archlinux>
From:   Baolin Wang <baolin.wang@linaro.org>
Date:   Tue, 28 Aug 2018 11:14:09 +0800
Message-ID: <CAMz4kuJnEg4_hDx75w3md2J6E0gJ_2O+iCMe9W_F+EpxJVj-cw@mail.gmail.com>
Subject: Re: [PATCH 1/2] iio: adc: sc27xx: Add raw data support
To:     Jonathan Cameron <jic23@kernel.org>
Cc:     Rob Herring <robh+dt@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Hartmut Knaack <knaack.h@gmx.de>,
        Lars-Peter Clausen <lars@metafoo.de>,
        Peter Meerwald-Stadler <pmeerw@pmeerw.net>,
        freeman.liu@spreadtrum.com, Mark Brown <broonie@kernel.org>,
        DTML <devicetree@vger.kernel.org>, linux-iio@vger.kernel.org,
        LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Jonathan,

On 25 August 2018 at 16:38, Jonathan Cameron <jic23@kernel.org> wrote:
> On Fri, 24 Aug 2018 17:53:15 +0800
> Baolin Wang <baolin.wang@linaro.org> wrote:
>
>> The headset device will use channel 20 of ADC controller to detect events,
>> but it needs the raw ADC data to do conversion according to its own formula.
>>
>> Thus we should configure the channel mask separately and configure channel
>> 20 as IIO_CHAN_INFO_RAW, as well as adding raw data read support.
>
> So is this a general thing, i.e. that channel is 'meant' to be used for the
> headset, or just a one off for a particular board?
> If it is a general thing, than I'm fine with this (unlikely we'll break any
> other users), but if not we need to find a nicer way to do it.

Yes, it is a general thing. Now channel 20 is always used for the headset.

> I am a little unclear on how a channel would provide the voltage on it's pin
> but that could be wrong when used for a different purpose?
> If it's just a matter of unusual loading characteristics then perhaps that
> is valid, but I'd like to understand this a little.

On our platform, the headset will use this voltage value reading from
ADC to check the headset events: headset buttons, headset plugin and
so on.

>>
>> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
>> ---
>>  drivers/iio/adc/sc27xx_adc.c |   80 ++++++++++++++++++++++++------------------
>>  1 file changed, 45 insertions(+), 35 deletions(-)
>>
>> diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
>> index 2b60efe..153c311 100644
>> --- a/drivers/iio/adc/sc27xx_adc.c
>> +++ b/drivers/iio/adc/sc27xx_adc.c
>> @@ -273,6 +273,17 @@ static int sc27xx_adc_read_raw(struct iio_dev *indio_dev,
>>       int ret, tmp;
>>
>>       switch (mask) {
>> +     case IIO_CHAN_INFO_RAW:
>> +             mutex_lock(&indio_dev->mlock);
>> +             ret = sc27xx_adc_read(data, chan->channel, scale, &tmp);
>> +             mutex_unlock(&indio_dev->mlock);
>> +
>> +             if (ret)
>> +                     return ret;
>> +
>> +             *val = tmp;
>> +             return IIO_VAL_INT;
>> +
>>       case IIO_CHAN_INFO_PROCESSED:
>>               mutex_lock(&indio_dev->mlock);
>>               ret = sc27xx_adc_read_processed(data, chan->channel, scale,
>> @@ -315,48 +326,47 @@ static int sc27xx_adc_write_raw(struct iio_dev *indio_dev,
>>       .write_raw = &sc27xx_adc_write_raw,
>>  };
>>
>> -#define SC27XX_ADC_CHANNEL(index) {                          \
>> +#define SC27XX_ADC_CHANNEL(index, mask) {                    \
>>       .type = IIO_VOLTAGE,                                    \
>>       .channel = index,                                       \
>> -     .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |    \
>> -                           BIT(IIO_CHAN_INFO_SCALE),         \
>> +     .info_mask_separate = mask | BIT(IIO_CHAN_INFO_SCALE),  \
>>       .datasheet_name = "CH##index",                          \
>>       .indexed = 1,                                           \
>>  }
>>
>>  static const struct iio_chan_spec sc27xx_channels[] = {
>> -     SC27XX_ADC_CHANNEL(0),
>> -     SC27XX_ADC_CHANNEL(1),
>> -     SC27XX_ADC_CHANNEL(2),
>> -     SC27XX_ADC_CHANNEL(3),
>> -     SC27XX_ADC_CHANNEL(4),
>> -     SC27XX_ADC_CHANNEL(5),
>> -     SC27XX_ADC_CHANNEL(6),
>> -     SC27XX_ADC_CHANNEL(7),
>> -     SC27XX_ADC_CHANNEL(8),
>> -     SC27XX_ADC_CHANNEL(9),
>> -     SC27XX_ADC_CHANNEL(10),
>> -     SC27XX_ADC_CHANNEL(11),
>> -     SC27XX_ADC_CHANNEL(12),
>> -     SC27XX_ADC_CHANNEL(13),
>> -     SC27XX_ADC_CHANNEL(14),
>> -     SC27XX_ADC_CHANNEL(15),
>> -     SC27XX_ADC_CHANNEL(16),
>> -     SC27XX_ADC_CHANNEL(17),
>> -     SC27XX_ADC_CHANNEL(18),
>> -     SC27XX_ADC_CHANNEL(19),
>> -     SC27XX_ADC_CHANNEL(20),
>> -     SC27XX_ADC_CHANNEL(21),
>> -     SC27XX_ADC_CHANNEL(22),
>> -     SC27XX_ADC_CHANNEL(23),
>> -     SC27XX_ADC_CHANNEL(24),
>> -     SC27XX_ADC_CHANNEL(25),
>> -     SC27XX_ADC_CHANNEL(26),
>> -     SC27XX_ADC_CHANNEL(27),
>> -     SC27XX_ADC_CHANNEL(28),
>> -     SC27XX_ADC_CHANNEL(29),
>> -     SC27XX_ADC_CHANNEL(30),
>> -     SC27XX_ADC_CHANNEL(31),
>> +     SC27XX_ADC_CHANNEL(0, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(1, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(2, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(3, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(4, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(5, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(6, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(7, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(8, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(9, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(10, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(11, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(12, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(13, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(14, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(15, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(16, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(17, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(18, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(19, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(20, BIT(IIO_CHAN_INFO_RAW)),
>> +     SC27XX_ADC_CHANNEL(21, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(22, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(23, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(24, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(25, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(26, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(27, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(28, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(29, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(30, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(31, BIT(IIO_CHAN_INFO_PROCESSED)),
>>  };
>>
>>  static int sc27xx_adc_enable(struct sc27xx_adc_data *data)
>


-- 
Baolin Wang
Best Regards