On 04/08/2023 18:10, Guenter Roeck wrote: > On 8/4/23 08:48, Conor Dooley wrote: >> On Thu, Aug 03, 2023 at 04:43:59PM +0200, Naresh Solanki wrote: >>> From: Marcello Sylvester Bauer <sylv@xxxxxxx> >>> >>> Add binding documentation for Maxim MAX6639 fan-speed controller. >>> >>> Signed-off-by: Marcello Sylvester Bauer <sylv@xxxxxxx> >>> Signed-off-by: Naresh Solanki <Naresh.Solanki@xxxxxxxxxxxxx> >>> --- >>> Changes in V3: >>> - Update title >>> - Add pulses-per-revolution, supplies & interrupts >>> Changes in V2: >>> - Update subject >>> - Drop blank lines >>> --- >>> .../bindings/hwmon/maxim,max6639.yaml | 60 +++++++++++++++++++ >>> 1 file changed, 60 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml >>> new file mode 100644 >>> index 000000000000..b3292061ca58 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml >>> @@ -0,0 +1,60 @@ >>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/hwmon/maxim,max6639.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: Maxim MAX6639 Fan Controller >>> + >>> +maintainers: >>> + - Naresh Solanki <Naresh.Solanki@xxxxxxxxxxxxx> >>> + >>> +description: | >>> + The MAX6639 is a 2-channel temperature monitor with dual, automatic, PWM >>> + fan-speed controller. It monitors its own temperature and one external >>> + diode-connected transistor or the temperatures of two external diode-connected >>> + transistors, typically available in CPUs, FPGAs, or GPUs. >> >>> + fan-supply: >>> + description: Phandle to the regulator that provides power to the fan. >> >>> + pulses-per-revolution: >>> + description: >>> + Define the number of pulses per fan revolution for each tachometer >>> + input as an integer. >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + enum: [1, 2, 3, 4] >>> + default: 2 >> >> Apologies if I am digging up old wounds here, since there was quite a >> bit of back and forth on the last version, but these two newly added >> properties look to be common with the "pwm-fan" and with >> "adi,axi-fan-control". At what point should these live in a common >> schema instead? >> >> Otherwise, this looks okay to me, although I'll leave things to >> Krzysztof since he had a lot to say about the previous version. >> > > Rob has said that he won't accept any fan controller bindings without a generic > schema. At the same time he has said that he expects properties such as the > number of pulses per revolution to be attached to a 'fan' description, and he > wants pwm related properties of fan controllers to be modeled as pwm controllers. > And now we have a notion of a regulator providing power to the fan (which again > would be the fan controller, at least in cases where the fan controller > provides direct voltage to the fan). On top of that, this fan-supply property > should presumably, again, be part of a fan description and not be part of the > controller description. I don't think anyone knows how to make this all work > (I for sure don't), so it is very unlikely we'll see a generic fan controller > schema anytime soon. > > Given that neither fan-supply nor pulses-per-revolution is implemented in the > driver, and given that I am not aware of any fans which would have a value for > pulses-per-revolution other than 2, my personal suggestion would be to add the > chip to trivial devices and be done with it for the time being. Yeah, this also would work. Best regards, Krzysztof
