linux-kernel - Re: [PATCH v3 1/2] dt-bindings: iio: adc: adding support for PAC193X

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20231126-nineteen-clumsy-701ac4145ba8@spud>
Date:   Sun, 26 Nov 2023 11:24:56 +0000
From:   Conor Dooley <conor@...nel.org>
To:     Jonathan Cameron <jic23@...nel.org>
Cc:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        marius.cristea@...rochip.com, lars@...afoo.de, robh+dt@...nel.org,
        jdelvare@...e.com, linux@...ck-us.net, linux-hwmon@...r.kernel.org,
        krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org,
        linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/2] dt-bindings: iio: adc: adding support for PAC193X

On Sat, Nov 25, 2023 at 07:47:54PM +0000, Jonathan Cameron wrote:
> On Thu, 16 Nov 2023 18:21:33 +0000
> Conor Dooley <conor@...nel.org> wrote:
> > On Thu, Nov 16, 2023 at 04:01:43PM +0100, Krzysztof Kozlowski wrote:
> > > On 15/11/2023 14:44, marius.cristea@...rochip.com wrote:  
> > > > From: Marius Cristea <marius.cristea@...rochip.com>

> > > > +patternProperties:
> > > > +  "^channel@[1-4]+$":
> > > > +    type: object
> > > > +    $ref: adc.yaml
> > > > +    description: Represents the external channels which are connected to the ADC.
> > > > +
> > > > +    properties:
> > > > +      reg:
> > > > +        items:
> > > > +          minimum: 1
> > > > +          maximum: 4
> > > > +
> > > > +      shunt-resistor-micro-ohms:
> > > > +        description: |
> > > > +          Value in micro Ohms of the shunt resistor connected between
> > > > +          the SENSE+ and SENSE- inputs, across which the current is measured. Value
> > > > +          is needed to compute the scaling of the measured current.
> > > > +
> > > > +    required:
> > > > +      - reg
> > > > +      - shunt-resistor-micro-ohms
> > > > +
> > > > +    unevaluatedProperties: false
> > > > +
> > > > +required:
> > > > +  - compatible
> > > > +  - reg
> > > > +  - "#address-cells"
> > > > +  - "#size-cells"
> > > > +
> > > > +allOf:
> > > > +  - if:
> > > > +      properties:
> > > > +        compatible:
> > > > +          contains:
> > > > +            const: interrupts  
> > > 
> > > 
> > > I don't understand what do you want to say here. I am also 100% sure you
> > > did not test it on a real case (maybe example passes but nothing more).  
> > 
> > As far as I understand, the same pin on the device is used for both an
> > output or an input depending on the configuration. As an input, it is
> > the "slow-io" control, and as an output it is an interrupt.
> > I think Marius is trying to convey that either this pin can be in
> > exclusively one state or another.
> > 
> > _However_ I am not sure that that is really the right thing to do - they
> > might well be mutually exclusive modes, but I think the decision can be
> > made at runtime, rather than at devicetree creation time. Say for
> > example the GPIO controller this is connected to is capable of acting as
> > an interrupt controller. Unless I am misunderstanding the runtime
> > configurability of this hardware, I think it is possible to actually
> > provide a "slow-io-gpios" and an interrupt property & let the operating
> > system decide at runtime which mode it wants to work in.
> 
> I'll admit I've long forgotten what was going on here, but based just on
> this bit of text I agree. There is nothing 'stopping' us having a pin
> uses as either / or / both interrupt and gpio.
> 
> It'll be a bit messy to support in the driver as IIRC there are some sanity
> checks that limit combinations on IRQs and output GPIOS.  Can't remember
> how bad the dance to navigate it safely is.
> 
> First version I'd just say pick one option if both are provided and
> don't support configuring it at runtime.

Just to be clear, you are suggesting having the
"microchip,slow-io-gpios" and "interrupts" properties in the binding,
but the driver will just (for example) put that pin into alert mode
always & leave it there?
If that is what you are suggesting, that seems pragmatic to me.

Cheers,
Conor.


Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)