Received-SPF: pass (google.com: domain of linux-kernel+bounces-30302-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <b96d5bfc-cc38-44c7-a88f-e7ac5e4eb71d@gmail.com>
Date: Thu, 18 Jan 2024 17:51:20 +0200
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v12 1/2] dt-bindings: adc: add AD7173
To: Conor Dooley <conor@kernel.org>
Cc: linus.walleij@linaro.org, brgl@bgdev.pl, andy@kernel.org,
 linux-gpio@vger.kernel.org, Lars-Peter Clausen <lars@metafoo.de>,
 Jonathan Cameron <jic23@kernel.org>, Rob Herring <robh+dt@kernel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
 Conor Dooley <conor+dt@kernel.org>, Michael Walle <michael@walle.cc>,
 Andy Shevchenko <andy.shevchenko@gmail.com>, Arnd Bergmann <arnd@arndb.de>,
 ChiaEn Wu <chiaen_wu@richtek.com>, Niklas Schnelle <schnelle@linux.ibm.com>,
 =?UTF-8?Q?Leonard_G=C3=B6hrs?= <l.goehrs@pengutronix.de>,
 Mike Looijmans <mike.looijmans@topic.nl>, Haibo Chen <haibo.chen@nxp.com>,
 Hugo Villeneuve <hvilleneuve@dimonoff.com>,
 David Lechner <dlechner@baylibre.com>,
 Ceclan Dumitru <dumitru.ceclan@analog.com>, linux-iio@vger.kernel.org,
 devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20240118125001.12809-1-mitrutzceclan@gmail.com>
 <20240118-lunar-anthem-31bf3b9b351d@spud>
Content-Language: en-US
From: Ceclan Dumitru <mitrutzceclan@gmail.com>
In-Reply-To: <20240118-lunar-anthem-31bf3b9b351d@spud>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit


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?

> 
> 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)