[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2eafa89c-7c95-4bc1-85cb-a6d7417dcea8@linaro.org>
Date: Fri, 13 Oct 2023 10:35:49 +0200
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Jonathan Cameron <Jonathan.Cameron@...wei.com>,
Ivan Mikhaylov <fr0st61te@...il.com>
Cc: Jonathan Cameron <jic23@...nel.org>,
Lars-Peter Clausen <lars@...afoo.de>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>, linux-iio@...r.kernel.org,
linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH v5 1/2] dt-bindings: adc: provide max34408/9 device tree
binding document
On 13/10/2023 10:19, Jonathan Cameron wrote:
> On Thu, 12 Oct 2023 19:27:33 +0300
> Ivan Mikhaylov <fr0st61te@...il.com> wrote:
>
>> On Thu, 2023-10-12 at 08:40 +0100, Jonathan Cameron wrote:
>>> On Tue, 10 Oct 2023 23:22:48 +0300
>>> Ivan Mikhaylov <fr0st61te@...il.com> wrote:
>>>
>>>> On Tue, 2023-10-10 at 15:40 +0100, Jonathan Cameron wrote:
>>>>> On Sun, 8 Oct 2023 02:48:37 +0300
>>>>> Ivan Mikhaylov <fr0st61te@...il.com> wrote:
>>>>>
>>>>>> The hardware binding for i2c current monitoring device with
>>>>>> overcurrent
>>>>>> control.
>>>>>>
>>>>>> Signed-off-by: Ivan Mikhaylov <fr0st61te@...il.com>
>>>>>> ---
>>>>>> .../bindings/iio/adc/maxim,max34408.yaml | 141
>>>>>> ++++++++++++++++++
>>>>>> 1 file changed, 141 insertions(+)
>>>>>> create mode 100644
>>>>>> Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
>>>>>>
>>>>>> diff --git
>>>>>> a/Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
>>>>>> b/Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
>>>>>> new file mode 100644
>>>>>> index 000000000000..9749f1fd1802
>>>>>> --- /dev/null
>>>>>> +++
>>>>>> b/Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
>>>>>> @@ -0,0 +1,141 @@
>>>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>>>> +%YAML 1.2
>>>>>> +---
>>>>>> +$id:
>>>>>> http://devicetree.org/schemas/iio/adc/maxim,max34408.yaml#
>>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>>> +
>>>>>> +title: Two- and four-channel current monitors with overcurrent
>>>>>> control
>>>>>> +
>>>>>> +maintainers:
>>>>>> + - Ivan Mikhaylov <fr0st61te@...il.com>
>>>>>> +
>>>>>> +description: |
>>>>>> + The MAX34408/MAX34409 are two- and four-channel current
>>>>>> monitors
>>>>>> that are
>>>>>> + configured and monitored with a standard I2C/SMBus serial
>>>>>> interface. Each
>>>>>> + unidirectional current sensor offers precision high-side
>>>>>> operation with a
>>>>>> + low full-scale sense voltage. The devices automatically
>>>>>> sequence
>>>>>> through
>>>>>> + two or four channels and collect the current-sense samples
>>>>>> and
>>>>>> average them
>>>>>> + to reduce the effect of impulse noise. The raw ADC samples
>>>>>> are
>>>>>> compared to
>>>>>> + user-programmable digital thresholds to indicate overcurrent
>>>>>> conditions.
>>>>>> + Overcurrent conditions trigger a hardware output to provide
>>>>>> an
>>>>>> immediate
>>>>>> + indication to shut down any necessary external circuitry.
>>>>>> +
>>>>>> + Specifications about the devices can be found at:
>>>>>> +
>>>>>> https://www.analog.com/media/en/technical-documentation/data-sheets/MAX34408-MAX34409.pdf
>>>>>> +
>>>>>> +properties:
>>>>>> + compatible:
>>>>>> + enum:
>>>>>> + - maxim,max34408
>>>>>> + - maxim,max34409
>>>>>> +
>>>>>> + "#address-cells":
>>>>>> + const: 1
>>>>>> +
>>>>>> + "#size-cells":
>>>>>> + const: 0
>>>>>> +
>>>>>> + reg:
>>>>>> + maxItems: 1
>>>>>> +
>>>>>> + interrupts:
>>>>>> + maxItems: 1
>>>>>> +
>>>>>> + powerdown-gpios:
>>>>>> + description:
>>>>>> + Shutdown Output. Open-drain output. This output
>>>>>> transitions
>>>>>> to high impedance
>>>>>> + when any of the digital comparator thresholds are
>>>>>> exceeded
>>>>>> as long as the ENA
>>>>>> + pin is high.
>>>>>> + maxItems: 1
>>>>>> +
>>>>>> + shtdn-enable-gpios:
>>>>>
>>>>> I guess the review crossed with you sending v5. There is some
>>>>> feedback on v4 you need
>>>>> to address here.
>>>>
>>>> Jonathan, I thought I did, I've changed ena to powerdown-gpios from
>>>> Krzysztof's comments but about this one pin I'm still not sure, it
>>>> looks like *-enable-gpios (like in *-enable-gpios pins in
>>>> iio/frequency/adi,adf4377.yaml) pin or is it not? Or maybe any
>>>> other
>>>> suggestions about naming of this one?
>>>>
>>>> Thanks.
>>>
>>> shutdown-gpios and make the sense (active high / low) such that
>>> setting
>>> it results in teh device being shut down.
>>> Or treat it as an enable and enable-gpios
>>>
>>> Something that indicates both shutdown and enable is confusing ;)
>>>
>>> Jonathan
>>
>>
>> Jonathan, then I make these changes:
>>
>> powerdown-gpios: -> output-enable:
> Needs to retain the gpios bit as we want the standard gpio stuff to pick
> them up. I'm not that keen on output-enable-gpios though. The activity
> here is very much 'shutdown because of error or not enabled' I think.
> So perhaps we flip the sense and document that it needs to be active low?
>
>> shtdn-enable-gpios: -> enable-gpios:
>>
>> Is it ok?
>
> Conor, Rob, Krzysztof - you probably have a better insight into this than
> I do.
>
"enable-gpios" are for turning on a specific feature, not powering
on/off entire device. For example to enable regulator output.
"powerdown-gpios" are for turning device on/off.
I don't know what do you have in your device.
Best regards,
Krzysztof
Powered by blists - more mailing lists