On Sun, Nov 26, 2023 at 04:04:38PM +0000, Jonathan Cameron wrote: > On Sun, 26 Nov 2023 11:24:56 +0000 > Conor Dooley <conor@xxxxxxxxxx> wrote: > > 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@xxxxxxxxxx> wrote: > > > > On Thu, Nov 16, 2023 at 04:01:43PM +0100, Krzysztof Kozlowski wrote: > > > > > On 15/11/2023 14:44, marius.cristea@xxxxxxxxxxxxx wrote: > > > > > > From: Marius Cristea <marius.cristea@xxxxxxxxxxxxx> > > > > > > +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? > > Yes. > > > If that is what you are suggesting, that seems pragmatic to me. > > If a use case to do something else comes along later, then we can > be smarter, but I'd like to keep it simple initially at least. Sounds good to me, thanks Jonathan. Seems like a good compromise of depicting the hardware accurately and not overcomplicating the driver implementation. Marius, I completely forgot to get in touch with you about this - give me a shout on teams if there's anything outstanding that I can help you with.
Attachment:
signature.asc
Description: PGP signature
