All of lore.kernel.org
 help / color / mirror / Atom feed
From: Laxman Dewangan <ldewangan@nvidia.com>
To: Jonathan Cameron <jic23@kernel.org>, Rob Herring <robh@kernel.org>
Cc: <corbet@lwn.net>, <lars@metafoo.de>, <devicetree@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <linux-doc@vger.kernel.org>,
	<linux-iio@vger.kernel.org>
Subject: Re: [PATCH 1/3] iio: adc: ina3221: Add DT binding details
Date: Fri, 3 Jun 2016 17:51:53 +0530	[thread overview]
Message-ID: <57517661.8090205@nvidia.com> (raw)
In-Reply-To: <c5a10e77-d326-9fd5-7c5f-2f5593295b1e@kernel.org>


On Friday 03 June 2016 05:41 PM, Jonathan Cameron wrote:
> On 03/06/16 12:48, Laxman Dewangan wrote:
>> On Friday 03 June 2016 03:49 PM, Jonathan Cameron wrote:
>>>>> +
>>>>> +enable-power-monitor:        Boolean, Enable power monitoring of the device.
>>> Is this the power good stuff?  description should be more detailed.
>> If there is no shunt resistance then we can not enable power monitor
>> on that rail. Device does not mandate to have shunt and so this is
>> based on platforms.
> The voltage shunt also becomes meaningless (unless you have other very
> small voltage drops to measure!). So drop that channel as well.
> Either it is there to do current measurement or it isn't.

OK. understood.


>>>>> +
>>>>> +enable-continuous-mode:        Boolean. Device support oneshot and continuous
>>>>> +                mode for the channel data conversion. Presence
>>>>> +                of this property will enable the continuous
>>>>> +                mode from boot.
>>> Is the difference between driver load time and the point where usespace can
>>> set it up significant enough to justify this?
>> We change the mode dynamically. If we have more core then goto the
>> continuous mode so that we can apply throttling if power consumption
>> is going more than requirement. If we are running single core then
>> change to oneshot mode.
> That's definitely a usespace or firmware decision, not a kernel one
> to my mind - unless I guess you an enforcing bringing this device up
> before firing up the additional cores?
>
> Then it's nasty but I can start to see some justification.

Yaah, this is FW decision. Our need on some platform was that start the 
driver with the continuous mode.
But I think that may be specific need. I can remove this and let user 
space can enable the continuous mode.



WARNING: multiple messages have this Message-ID (diff)
From: Laxman Dewangan <ldewangan@nvidia.com>
To: Jonathan Cameron <jic23@kernel.org>, Rob Herring <robh@kernel.org>
Cc: corbet@lwn.net, lars@metafoo.de, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
	linux-iio@vger.kernel.org
Subject: Re: [PATCH 1/3] iio: adc: ina3221: Add DT binding details
Date: Fri, 3 Jun 2016 17:51:53 +0530	[thread overview]
Message-ID: <57517661.8090205@nvidia.com> (raw)
In-Reply-To: <c5a10e77-d326-9fd5-7c5f-2f5593295b1e@kernel.org>


On Friday 03 June 2016 05:41 PM, Jonathan Cameron wrote:
> On 03/06/16 12:48, Laxman Dewangan wrote:
>> On Friday 03 June 2016 03:49 PM, Jonathan Cameron wrote:
>>>>> +
>>>>> +enable-power-monitor:        Boolean, Enable power monitoring of the device.
>>> Is this the power good stuff?  description should be more detailed.
>> If there is no shunt resistance then we can not enable power monitor
>> on that rail. Device does not mandate to have shunt and so this is
>> based on platforms.
> The voltage shunt also becomes meaningless (unless you have other very
> small voltage drops to measure!). So drop that channel as well.
> Either it is there to do current measurement or it isn't.

OK. understood.


>>>>> +
>>>>> +enable-continuous-mode:        Boolean. Device support oneshot and continuous
>>>>> +                mode for the channel data conversion. Presence
>>>>> +                of this property will enable the continuous
>>>>> +                mode from boot.
>>> Is the difference between driver load time and the point where usespace can
>>> set it up significant enough to justify this?
>> We change the mode dynamically. If we have more core then goto the
>> continuous mode so that we can apply throttling if power consumption
>> is going more than requirement. If we are running single core then
>> change to oneshot mode.
> That's definitely a usespace or firmware decision, not a kernel one
> to my mind - unless I guess you an enforcing bringing this device up
> before firing up the additional cores?
>
> Then it's nasty but I can start to see some justification.

Yaah, this is FW decision. Our need on some platform was that start the 
driver with the continuous mode.
But I think that may be specific need. I can remove this and let user 
space can enable the continuous mode.

  reply	other threads:[~2016-06-03 12:34 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-01 12:34 [PATCH 1/3] iio: adc: ina3221: Add DT binding details Laxman Dewangan
2016-06-01 12:34 ` Laxman Dewangan
2016-06-01 12:34 ` [PATCH 2/3] iio: adc: ina3221: Add support for IIO ADC driver for TI INA3221 Laxman Dewangan
2016-06-01 12:34   ` Laxman Dewangan
2016-06-03 10:06   ` Jonathan Cameron
2016-06-03 10:16     ` Jonathan Cameron
2016-06-03 11:31       ` Laxman Dewangan
2016-06-03 11:31         ` Laxman Dewangan
2016-06-03 12:04         ` Jonathan Cameron
2016-06-03 12:04           ` Jonathan Cameron
2016-06-03 12:03           ` Laxman Dewangan
2016-06-03 12:03             ` Laxman Dewangan
2016-06-03 11:26     ` Laxman Dewangan
2016-06-03 11:26       ` Laxman Dewangan
2016-06-03 12:09       ` Jonathan Cameron
2016-06-03 12:17         ` Laxman Dewangan
2016-06-03 12:17           ` Laxman Dewangan
2016-06-03 13:29     ` Guenter Roeck
2016-06-03 14:14       ` Laxman Dewangan
2016-06-03 14:14         ` Laxman Dewangan
2016-06-03 15:17         ` Andrew F. Davis
2016-06-03 15:17           ` Andrew F. Davis
2016-06-07 22:30           ` Guenter Roeck
2016-06-08 15:04             ` Andrew F. Davis
2016-06-08 15:04               ` Andrew F. Davis
2016-06-08 15:37               ` Laxman Dewangan
2016-06-08 15:37                 ` Laxman Dewangan
2016-06-01 12:34 ` [PATCH 3/3] iio: adc: ina3221: Add sysfs details " Laxman Dewangan
2016-06-01 12:34   ` Laxman Dewangan
2016-06-03 10:26   ` Jonathan Cameron
2016-06-03  2:07 ` [PATCH 1/3] iio: adc: ina3221: Add DT binding details Rob Herring
2016-06-03  2:07   ` Rob Herring
2016-06-03  9:02   ` Laxman Dewangan
2016-06-03  9:02     ` Laxman Dewangan
2016-06-03 10:19   ` Jonathan Cameron
2016-06-03 10:19     ` Jonathan Cameron
2016-06-03 11:48     ` Laxman Dewangan
2016-06-03 11:48       ` Laxman Dewangan
2016-06-03 12:11       ` Jonathan Cameron
2016-06-03 12:21         ` Laxman Dewangan [this message]
2016-06-03 12:21           ` Laxman Dewangan

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=57517661.8090205@nvidia.com \
    --to=ldewangan@nvidia.com \
    --cc=corbet@lwn.net \
    --cc=devicetree@vger.kernel.org \
    --cc=jic23@kernel.org \
    --cc=lars@metafoo.de \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robh@kernel.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.