Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1331113 > unrolled thread

RE: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support

Started by"Sricharan" <sricharan@codeaurora.org>
First post2016-02-10 13:30 +0100
Last post2016-02-18 09:50 +0100
Articles 6 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  RE: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support "Sricharan" <sricharan@codeaurora.org> - 2016-02-10 13:30 +0100
    Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support Daniel Baluta <daniel.baluta@intel.com> - 2016-02-10 14:00 +0100
      RE: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support "Sricharan" <sricharan@codeaurora.org> - 2016-02-10 16:10 +0100
        Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support Michael Welling <mwelling@ieee.org> - 2016-02-10 17:40 +0100
          Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support Michael Welling <mwelling@ieee.org> - 2016-02-18 01:00 +0100
            RE: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support "Sricharan" <sricharan@codeaurora.org> - 2016-02-18 09:50 +0100

#1331113 — RE: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support

From"Sricharan" <sricharan@codeaurora.org>
Date2016-02-10 13:30 +0100
SubjectRE: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
Message-ID<r0C4a-28R-5@gated-at.bofh.it>
Hi,

> -----Original Message-----
> From: linux-arm-msm-owner@vger.kernel.org [mailto:linux-arm-msm-
> owner@vger.kernel.org] On Behalf Of Michael Welling
> Sent: Tuesday, February 09, 2016 12:47 AM
> To: Sricharan
> Cc: 'Wolfram Sang'; 'Daniel Baluta'; 'Jonathan Cameron'; 'Hartmut Knaack';
> 'Lars-Peter Clausen'; 'Peter Meerwald-Stadler'; 'Linux Kernel Mailing
List';
> linux-iio@vger.kernel.org; 'Lucas De Marchi'; 'Andy Gross'; 'Pramod
Gurav';
> 'Bjorn Andersson'; 'Guenter Roeck'; eibach@gdsys.de; linux-arm-
> msm@vger.kernel.org
> Subject: Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
> 
> On Tue, Feb 09, 2016 at 12:41:35AM +0530, Sricharan wrote:
> > Hi,
> >
> > > -----Original Message-----
> > > From: Michael Welling [mailto:mwelling79@gmail.com] On Behalf Of
> > > Michael Welling
> > > Sent: Monday, February 08, 2016 10:07 PM
> > > To: Wolfram Sang
> > > Cc: Daniel Baluta; Jonathan Cameron; Hartmut Knaack; Lars-Peter
> > > Clausen; Peter Meerwald-Stadler; Linux Kernel Mailing List;
> > linux-iio@vger.kernel.org;
> > > Lucas De Marchi; Andy Gross; Pramod Gurav; Bjorn Andersson; Guenter
> > > Roeck; eibach@gdsys.de; Sricharan R; linux-arm-msm@vger.kernel.org
> > > Subject: Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
> > >
> > > On Mon, Feb 08, 2016 at 11:25:00AM +0100, Wolfram Sang wrote:
> > > > On Fri, Feb 05, 2016 at 06:32:45PM -0600, Michael Welling wrote:
> > > > > On Fri, Feb 05, 2016 at 09:32:34PM +0200, Daniel Baluta wrote:
> > > > > > >> +static int ads1015_read_raw(struct iio_dev *indio_dev,
> > > > > > >> +                         struct iio_chan_spec const *chan,
> > > > > > >> +int
> > *val,
> > > > > > >> +                         int *val2, long mask) {
> > > > > > >> +     int ret, idx;
> > > > > > >> +     struct ads1015_data *data = iio_priv(indio_dev);
> > > > > > >> +
> > > > > > >> +     mutex_lock(&data->lock);
> > > > > > >> +     switch (mask) {
> > > > > > >> +     case IIO_CHAN_INFO_RAW:
> > > > > > >> +             if (iio_buffer_enabled(indio_dev)) {
> > > > > > >> +                     ret = -EBUSY;
> > > > > > >> +                     break;
> > > > > > >> +             }
> > > > > > >> +
> > > > > > >> +             ret = ads1015_set_power_state(data, true);
> > > > > > >> +             if (ret < 0)
> > > > > > >> +                     break;
> > > > > > >
> > > > > > > Just tested the driver on a Dragonboard 410C with a robotics
> > > > > > > mezzanine that I designed.
> > > > > > >
> > > > > > > The above ads1015_set_power_state(data, true) is always
> > > > > > > returning
> > -
> > > EINVAL.
> > > > > > >
> > > > > > > Any ideas why that would be happening?
> > > > > > > I think it may be the return from pm_runtime_get_sync?
> > > > > >
> > > > > > Can you confirm that pm_runtime_get_sync fails? Using some
> printk?
> > > > > >
> > > > > > Also adding printks in suspend/resume function would be helpful.
> > > > > > Do you have CONFIG_PM enabled?
> > > > > >
> > > > >
> > > > > Indeed it is the pm_runtime_get_sync that fails with a -EINVAL.
> > > > >
> > > > > > >
> > > > > > > When I comment out the break the readings come back but are
> > > > > > > not
> > > updated continually.
> > > > > > > If I read in_voltage0-voltage1_raw then in_voltage0_raw the
> > > > > > > value
> > is
> > > updated.
> > > > > >
> > > > > > I guess this is normal if set_power_state fails.
> > > > >
> > > > > The hwmod driver works fine BTW.
> > > > >
> > > > > My guess is there is an issue with the qup i2c driver seeing as
> > > > > it has worked on other system without issue.
> > > > >
> > > > > CC'd some the latest developer on the qup i2c driver.
> > > > >
> > > > > I2C guys have any ideas on this?
> > > > >
> > > >
> > > > Adding some more people who recently worked on this. Might be nice
> > > > to know which kernel version you are using.
> > > >
> >  Which i2c bus is this connected to ? I can give a try with 410c to
> > see why pm_runtime_get_sync from qup  fails.
> 
> It is on the lowspeed header. Here is my devicetree entry:
> 
>                  i2c@78b6000 {
>                  /* On Low speed expansion */
>                          label = "LS-I2C0";
>                          status = "okay";
> 
>                          pca: pca@40 {
>                                  compatible = "nxp,pca9685-pwm";
>                                  #pwm-cells = <2>;
>                                  reg = <0x40>;
>                          };
> 
>                          adc: adc@48 {
>                                  compatible = "ti,ads1015";
>                                  reg = <0x48>;
>                          };
>                  };

Whats the sequence in which the failure happens ?

I tested on DB410c by adding the DT entry that you mentioned above on
4.5-rc2 and rc3.
I see that the i2c transfers call from pca9685  during  pca9685_pwm_probe
did
go through and no failure from pm_runtime_get_sync

Regards,
 Sricharan

[toc] | [next] | [standalone]

#1331133

FromDaniel Baluta <daniel.baluta@intel.com>
Date2016-02-10 14:00 +0100
Message-ID<r0Cxd-2iO-13@gated-at.bofh.it>
In reply to#1331113
<snap headers>

>> > > > > > >> +static int ads1015_read_raw(struct iio_dev *indio_dev,
>> > > > > > >> +                         struct iio_chan_spec const *chan,
>> > > > > > >> +int
>> > *val,
>> > > > > > >> +                         int *val2, long mask) {
>> > > > > > >> +     int ret, idx;
>> > > > > > >> +     struct ads1015_data *data = iio_priv(indio_dev);
>> > > > > > >> +
>> > > > > > >> +     mutex_lock(&data->lock);
>> > > > > > >> +     switch (mask) {
>> > > > > > >> +     case IIO_CHAN_INFO_RAW:
>> > > > > > >> +             if (iio_buffer_enabled(indio_dev)) {
>> > > > > > >> +                     ret = -EBUSY;
>> > > > > > >> +                     break;
>> > > > > > >> +             }
>> > > > > > >> +
>> > > > > > >> +             ret = ads1015_set_power_state(data, true);
>> > > > > > >> +             if (ret < 0)
>> > > > > > >> +                     break;
>> > > > > > >
>> > > > > > > Just tested the driver on a Dragonboard 410C with a robotics
>> > > > > > > mezzanine that I designed.
>> > > > > > >
>> > > > > > > The above ads1015_set_power_state(data, true) is always
>> > > > > > > returning
>> > -
>> > > EINVAL.
>> > > > > > >
>> > > > > > > Any ideas why that would be happening?
>> > > > > > > I think it may be the return from pm_runtime_get_sync?
>> > > > > >
>> > > > > > Can you confirm that pm_runtime_get_sync fails? Using some
>> printk?
>> > > > > >
>> > > > > > Also adding printks in suspend/resume function would be helpful.
>> > > > > > Do you have CONFIG_PM enabled?
>> > > > > >
>> > > > >
>> > > > > Indeed it is the pm_runtime_get_sync that fails with a -EINVAL.
>> > > > >
>> > > > > > >
>> > > > > > > When I comment out the break the readings come back but are
>> > > > > > > not
>> > > updated continually.
>> > > > > > > If I read in_voltage0-voltage1_raw then in_voltage0_raw the
>> > > > > > > value
>> > is
>> > > updated.
>> > > > > >
>> > > > > > I guess this is normal if set_power_state fails.
>> > > > >
>> > > > > The hwmod driver works fine BTW.
>> > > > >
>> > > > > My guess is there is an issue with the qup i2c driver seeing as
>> > > > > it has worked on other system without issue.
>> > > > >
>> > > > > CC'd some the latest developer on the qup i2c driver.
>> > > > >
>> > > > > I2C guys have any ideas on this?
>> > > > >
>> > > >
>> > > > Adding some more people who recently worked on this. Might be nice
>> > > > to know which kernel version you are using.
>> > > >
>> >  Which i2c bus is this connected to ? I can give a try with 410c to
>> > see why pm_runtime_get_sync from qup  fails.
>>
>> It is on the lowspeed header. Here is my devicetree entry:
>>
>>                  i2c@78b6000 {
>>                  /* On Low speed expansion */
>>                          label = "LS-I2C0";
>>                          status = "okay";
>>
>>                          pca: pca@40 {
>>                                  compatible = "nxp,pca9685-pwm";
>>                                  #pwm-cells = <2>;
>>                                  reg = <0x40>;
>>                          };
>>
>>                          adc: adc@48 {
>>                                  compatible = "ti,ads1015";
>>                                  reg = <0x48>;
>>                          };
>>                  };
>
> Whats the sequence in which the failure happens ?
>
> I tested on DB410c by adding the DT entry that you mentioned above on
> 4.5-rc2 and rc3.
> I see that the i2c transfers call from pca9685  during  pca9685_pwm_probe
> did
> go through and no failure from pm_runtime_get_sync

Hi Sricharan,

Are you looking at pca9685_pwm_probe in drivers/pwm/pwm-pca9685.c right?

I'm asking this because this driver doesn't seem to support runtime
pm and there is no check for regmap_write/regmap_write return
code in the probe function.


Daniel.

[toc] | [prev] | [next] | [standalone]

#1331251

From"Sricharan" <sricharan@codeaurora.org>
Date2016-02-10 16:10 +0100
Message-ID<r0Ez0-3Nr-3@gated-at.bofh.it>
In reply to#1331133
<snip>...

> >> > > > >
> >> > > > > Indeed it is the pm_runtime_get_sync that fails with a -EINVAL.
> >> > > > >
> >> > > > > > >
> >> > > > > > > When I comment out the break the readings come back but
> >> > > > > > > are not
> >> > > updated continually.
> >> > > > > > > If I read in_voltage0-voltage1_raw then in_voltage0_raw
> >> > > > > > > the value
> >> > is
> >> > > updated.
> >> > > > > >
> >> > > > > > I guess this is normal if set_power_state fails.
> >> > > > >
> >> > > > > The hwmod driver works fine BTW.
> >> > > > >
> >> > > > > My guess is there is an issue with the qup i2c driver seeing
> >> > > > > as it has worked on other system without issue.
> >> > > > >
> >> > > > > CC'd some the latest developer on the qup i2c driver.
> >> > > > >
> >> > > > > I2C guys have any ideas on this?
> >> > > > >
> >> > > >
> >> > > > Adding some more people who recently worked on this. Might be
> >> > > > nice to know which kernel version you are using.
> >> > > >
> >> >  Which i2c bus is this connected to ? I can give a try with 410c to
> >> > see why pm_runtime_get_sync from qup  fails.
> >>
> >> It is on the lowspeed header. Here is my devicetree entry:
> >>
> >>                  i2c@78b6000 {
> >>                  /* On Low speed expansion */
> >>                          label = "LS-I2C0";
> >>                          status = "okay";
> >>
> >>                          pca: pca@40 {
> >>                                  compatible = "nxp,pca9685-pwm";
> >>                                  #pwm-cells = <2>;
> >>                                  reg = <0x40>;
> >>                          };
> >>
> >>                          adc: adc@48 {
> >>                                  compatible = "ti,ads1015";
> >>                                  reg = <0x48>;
> >>                          };
> >>                  };
> >
> > Whats the sequence in which the failure happens ?
> >
> > I tested on DB410c by adding the DT entry that you mentioned above on
> > 4.5-rc2 and rc3.
> > I see that the i2c transfers call from pca9685  during
> > pca9685_pwm_probe did go through and no failure from
> > pm_runtime_get_sync
> 
> Hi Sricharan,
> 
> Are you looking at pca9685_pwm_probe in drivers/pwm/pwm-pca9685.c
> right?
>
 Yes.
 
> I'm asking this because this driver doesn't seem to support runtime pm and
> there is no check for regmap_write/regmap_write return code in the probe
> function.
   Hmm to be clear, so it’s the pm_runtime_getsync from i2c-qup which fails right ?
   I was tracking that when there are i2c_xfers from pwm. I did not see any failures there.
   So wanted to know the correct sequence to reproduce.
  
Regards,
  Sricharan

[toc] | [prev] | [next] | [standalone]

#1331318

FromMichael Welling <mwelling@ieee.org>
Date2016-02-10 17:40 +0100
Message-ID<r0FY8-4CC-49@gated-at.bofh.it>
In reply to#1331251
On Wed, Feb 10, 2016 at 08:39:04PM +0530, Sricharan wrote:
> > Hi Sricharan,
> > 
> > Are you looking at pca9685_pwm_probe in drivers/pwm/pwm-pca9685.c
> > right?
> >
>  Yes.
>  
> > I'm asking this because this driver doesn't seem to support runtime pm and
> > there is no check for regmap_write/regmap_write return code in the probe
> > function.
>    Hmm to be clear, so it’s the pm_runtime_getsync from i2c-qup which fails right ?
>    I was tracking that when there are i2c_xfers from pwm. I did not see any failures there.
>    So wanted to know the correct sequence to reproduce.
>

The problem was discovered using the patch that this thread is on. The PWM driver does
not have the problem.

When the driver in this patch called pm_runtime_get_sync you got -EINVAL back.

> Regards,
>   Sricharan
>   
> 
>

[toc] | [prev] | [next] | [standalone]

#1336894

FromMichael Welling <mwelling@ieee.org>
Date2016-02-18 01:00 +0100
Message-ID<r3kaK-5hs-11@gated-at.bofh.it>
In reply to#1331318
On Wed, Feb 10, 2016 at 10:36:22AM -0600, Michael Welling wrote:
> On Wed, Feb 10, 2016 at 08:39:04PM +0530, Sricharan wrote:
> > > Hi Sricharan,
> > > 
> > > Are you looking at pca9685_pwm_probe in drivers/pwm/pwm-pca9685.c
> > > right?
> > >
> >  Yes.
> >  
> > > I'm asking this because this driver doesn't seem to support runtime pm and
> > > there is no check for regmap_write/regmap_write return code in the probe
> > > function.
> >    Hmm to be clear, so it’s the pm_runtime_getsync from i2c-qup which fails right ?
> >    I was tracking that when there are i2c_xfers from pwm. I did not see any failures there.
> >    So wanted to know the correct sequence to reproduce.
> >
> 
> The problem was discovered using the patch that this thread is on. The PWM driver does
> not have the problem.
> 
> When the driver in this patch called pm_runtime_get_sync you got -EINVAL back.

I noticed some patches for the QUP I2C driver in linux-next so I built against it.

The ADC driver now appears to work as desired.

root@dragonboard-410c:~# cat /sys/bus/iio/devices/iio\:device0/in_voltage0_raw 
287
root@dragonboard-410c:~# cat /sys/bus/iio/devices/iio\:device0/in_voltage1_raw 
269
root@dragonboard-410c:~# cat /sys/bus/iio/devices/iio\:device0/in_voltage2_raw 
270
root@dragonboard-410c:~# cat /sys/bus/iio/devices/iio\:device0/in_voltage3_raw 
271

> 
> > Regards,
> >   Sricharan
> >   
> > 
> >

[toc] | [prev] | [next] | [standalone]

#1337161

From"Sricharan" <sricharan@codeaurora.org>
Date2016-02-18 09:50 +0100
Message-ID<r3srE-2Xh-31@gated-at.bofh.it>
In reply to#1336894
Hi,

> On Wed, Feb 10, 2016 at 10:36:22AM -0600, Michael Welling wrote:
> > On Wed, Feb 10, 2016 at 08:39:04PM +0530, Sricharan wrote:
> > > > Hi Sricharan,
> > > >
> > > > Are you looking at pca9685_pwm_probe in drivers/pwm/pwm-
> pca9685.c
> > > > right?
> > > >
> > >  Yes.
> > >
> > > > I'm asking this because this driver doesn't seem to support
> > > > runtime pm and there is no check for regmap_write/regmap_write
> > > > return code in the probe function.
> > >    Hmm to be clear, so it’s the pm_runtime_getsync from i2c-qup which
> fails right ?
> > >    I was tracking that when there are i2c_xfers from pwm. I did not see
> any failures there.
> > >    So wanted to know the correct sequence to reproduce.
> > >
> >
> > The problem was discovered using the patch that this thread is on. The
> > PWM driver does not have the problem.
> >
> > When the driver in this patch called pm_runtime_get_sync you got -EINVAL
> back.
> 
> I noticed some patches for the QUP I2C driver in linux-next so I built against
> it.
> 
> The ADC driver now appears to work as desired.
> 
   Ok, I suppose those were changes merged last week to add qup V2 tags support, but
   not sure how that is helping to solve the pm_runtime issue here. 

Regards,
  Sricharan

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web