Messages in this thread | | | Date | Thu, 18 Jan 2024 16:06:29 +0000 | From | Conor Dooley <> | Subject | Re: [PATCH v12 1/2] dt-bindings: adc: add AD7173 |
| |
On Thu, Jan 18, 2024 at 05:51:20PM +0200, Ceclan Dumitru wrote: > > > On 1/18/24 17:23, Conor Dooley wrote: > > On Thu, Jan 18, 2024 at 02:49:22PM +0200, Dumitru Ceclan wrote: > > ... > > >> + adi,clock-select: > >> + description: | > >> + Select the ADC clock source. Valid values are: > >> + int : Internal oscillator > >> + int-out : Internal oscillator with output on XTAL2 pin > >> + ext-clk : External clock input on XTAL2 pin > >> + xtal : External crystal on XTAL1 and XTAL2 pins > >> + > >> + $ref: /schemas/types.yaml#/definitions/string > >> + enum: > >> + - int > >> + - int-out > >> + - ext-clk > >> + - xtal > >> + default: int > > I am not a fan of properties like this one, that in my view reimplement > > things that are supported by the regular clocks properties. I've got > > some questions for you so I can understand whether or not this custom > > property is required. > > > > Whether or not the ext-clk or xtal is used is known based on > > clock-names - why is the custom property required to determine that?
> > If neither of those clocks are present, then the internal clock would be > > used. Choosing to use the internal clock if an external one is provided > > sounds to me like a software policy decision made by the operating > > system. > > If there was no int-out, sure. I considered that the choice between int > and int-out could be made here. So better for driver to choose int/int-out?
This part of my comments was specifically about choosing between use of the internal clock when ext-clk or xtal are provided, which I think excludes the possibility of using int-out, since the XTAL2 pin is an input.
There's 3 situations: - no external clock provided - ext-clk provided - xtal provided
For the former, you know you're in that state when no "clocks" property is present. The latter two you can differentiate based on "clock-names".
Choosing to use the internal clock if an external clock is provided seems to be a software policy decision, unless I am mistaken.
> > > > Finally, if the ADC has a clock output, why can that not be represented > > by making the ADC a clock-controller? > > > > Was not familiar with this/did not cross my mind. So if xtal/ext-clk is > present, the driver should detect it and disable the option for clock > output? (Common pin XTAL2)
Yeah, if those clocks are provided you would not register as a clock controller. If there is a user of the output clock, it should have its own "clocks" property that references the ADC's output.
Your dt-binding could also make clocks/clock-names & clock-controller mutually exclusive.
Cheers, Conor. [unhandled content-type:application/pgp-signature] | |