Message ID | 20230803144401.1151065-1-Naresh.Solanki@9elements.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:9f41:0:b0:3e4:2afc:c1 with SMTP id v1csp1216318vqx; Thu, 3 Aug 2023 08:13:21 -0700 (PDT) X-Google-Smtp-Source: APBJJlHNG+YUtM6wa8GZ17WNHk5443Hi1M1p0/r2Qq5fmO+7D0zZq5zfKGbuPGxghqhPNsD0zxtD X-Received: by 2002:a05:6808:347:b0:3a7:1e40:6ce9 with SMTP id j7-20020a056808034700b003a71e406ce9mr15800257oie.26.1691075600949; Thu, 03 Aug 2023 08:13:20 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1691075600; cv=none; d=google.com; s=arc-20160816; b=CvohZhox0TC9PnKLlcm4dYGrDTCLqmC9wedMf4sz6hZwE9QiT0O1Fr5FygxBxAz2FF w9sDXDq6dQzevc1q1R+/Fbusg/h5BXdjUCrEAL1vJ5g7nVR2obDM0bgbWpd3uQiDwm4U bAoNoTV/v9oNgtDQ8sTC0E6Oxdqp9yFsiIytQU/USOibbEJuBrYHma/olhieg5KonIKF MFyToZD/dV5Qjt6YJs3qkkx3jJNG7TLUOaaTrRN7xh0qcD2iDfyW4Nc6KW+USXvorqHT WSJ8u2eoIVjKNJZ+XyJc0YTthbckNls47yawSJMoTmm9hWWWWuTq/Qjiy5PZXO71qTpW dxOg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=jJQoP+OlQsep7JicKgaIdwHZRhxScdcgkq1/BfxKseY=; fh=8ak6QauB5x4MIqFRh8zurj8EoiWd7T4rMNZqNkpWqB8=; b=s/LZN6XPCXqEdLopv00b93lU3dKK07BzltK3IcoVNRoApv4bevN6ziU4MxGKcqJmZY lPjT9vA59Fylxj9tOP3Fjm0rPpzm+X3WLNP2YW1wJ98Q0ptVuYmlzHJlSelUa4YAf7VN Z/xT8Ip/A3nXHYcszXLU8dmkae6T2wbSk21XwUqtXYNPNDs9BHvQGquKu9u/XevCq/j+ qRUJC3ntquJVx3NbDEGDrvLv/oWDvxMLhVixz/F+pJkCdrIGHMMxkZs/tIZlk4OiFqfX Q1ECawRubPthV2W+QiuL36wdexfQJpzaFmXyAeSKZsBHxyQ49bdFptvYoM4XEnYjAEZ+ 6/uA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@9elements.com header.s=google header.b=QaXj4huY; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=9elements.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id k189-20020a6384c6000000b0055ffed90cc9si10669721pgd.609.2023.08.03.08.13.07; Thu, 03 Aug 2023 08:13:20 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@9elements.com header.s=google header.b=QaXj4huY; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=9elements.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236126AbjHCOof (ORCPT <rfc822;guoshuai5156@gmail.com> + 99 others); Thu, 3 Aug 2023 10:44:35 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54308 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235814AbjHCOoY (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 3 Aug 2023 10:44:24 -0400 Received: from mail-ej1-x62f.google.com (mail-ej1-x62f.google.com [IPv6:2a00:1450:4864:20::62f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8AD6C4233 for <linux-kernel@vger.kernel.org>; Thu, 3 Aug 2023 07:44:07 -0700 (PDT) Received: by mail-ej1-x62f.google.com with SMTP id a640c23a62f3a-99bdcade7fbso144313966b.1 for <linux-kernel@vger.kernel.org>; Thu, 03 Aug 2023 07:44:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=9elements.com; s=google; t=1691073845; x=1691678645; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=jJQoP+OlQsep7JicKgaIdwHZRhxScdcgkq1/BfxKseY=; b=QaXj4huY5XwJDj5dgKYPotJmlo6vd/UT1VwxEm6+zSboBzFWXaftbcixNgcNP/0Bhe jcJr4LgYdkNdTdy4P4pXBCUtbiBDxwzDWSLY4wb2rC8rdCTqrNXcvwnrrlpb3QIgCdGK FZlyfOwdf9XbCsqjr17Fqdppk3LAXTphgv3h498VIF5cD2Kt5YRa01zYumasyM9q+2cX eJfcPAOVACXJD7Upx/stiZmeKaLx04qF8vMCSo5Q5NxjvI6JP+TMY3fBSkc8brHQ7hKI WiX9O6K9UXzARZsILMIATPYro5c9VSsXLQ4gjIJf4gX8G9XiJ0knPECRhz5i9uDKEwKG uawQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1691073845; x=1691678645; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=jJQoP+OlQsep7JicKgaIdwHZRhxScdcgkq1/BfxKseY=; b=Z7pFhQZxcWpEK+q5AruS2ZlTmUbmiEaip45X7/en9Ed+h6vH6obxmDq6DI79vljmtB kLis+Ctnb6ShL758wWOS4hz4aKE5o7EO5t+MSG5oa6BmLe4etmFkyl1V2HsoZ7/haykz l0eqywNlq5iYvZB/65WkamjYVAJVFSwAjvMmo42tPZNo/Fby7/K2kh5Nk+3NAHy1sqPA NG0aQM65mg1ImqdvnDRn8Q86qgswnhk/h9ehIW6uScxWiDXw61emfZTL5IbXgtybaZin dbZNLjh+1A7lzK35Howxnw8VNK18OFgfLDRVGye7hP2vQ/36hTzOMEYc+YBwM4WGClG8 iECg== X-Gm-Message-State: ABy/qLY/95fo8vKwc1iAwlvHbCO19BdDhZvxJ1ewYZ6FLXa/9bpT5oqS z7w2+bySCec0eb63QvfA2I76Ag== X-Received: by 2002:a17:906:77d6:b0:992:b3a3:81f4 with SMTP id m22-20020a17090677d600b00992b3a381f4mr8361117ejn.50.1691073845548; Thu, 03 Aug 2023 07:44:05 -0700 (PDT) Received: from stroh80.sec.9e.network (ip-078-094-000-051.um19.pools.vodafone-ip.de. [78.94.0.51]) by smtp.gmail.com with ESMTPSA id t26-20020a1709064f1a00b0099bd86f9248sm10629859eju.63.2023.08.03.07.44.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 03 Aug 2023 07:44:05 -0700 (PDT) From: Naresh Solanki <naresh.solanki@9elements.com> X-Google-Original-From: Naresh Solanki <Naresh.Solanki@9elements.com> To: Guenter Roeck <linux@roeck-us.net>, krzysztof.kozlowski+dt@linaro.org, Jean Delvare <jdelvare@suse.com>, Rob Herring <robh+dt@kernel.org>, Conor Dooley <conor+dt@kernel.org>, Naresh Solanki <Naresh.Solanki@9elements.com> Cc: Marcello Sylvester Bauer <sylv@sylv.io>, linux-hwmon@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v3 1/2] dt-bindings: hwmon: Add MAX6639 Date: Thu, 3 Aug 2023 16:43:59 +0200 Message-ID: <20230803144401.1151065-1-Naresh.Solanki@9elements.com> X-Mailer: git-send-email 2.41.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_BLOCKED, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1773221289289435874 X-GMAIL-MSGID: 1773221289289435874 |
Series |
[v3,1/2] dt-bindings: hwmon: Add MAX6639
|
|
Commit Message
Naresh Solanki
Aug. 3, 2023, 2:43 p.m. UTC
From: Marcello Sylvester Bauer <sylv@sylv.io> Add binding documentation for Maxim MAX6639 fan-speed controller. Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com> --- 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 base-commit: cb7022b8976e3c4d12cea2e7bb820a2944e2fd7b
Comments
On Thu, Aug 03, 2023 at 04:43:59PM +0200, Naresh Solanki wrote: > From: Marcello Sylvester Bauer <sylv@sylv.io> > > Add binding documentation for Maxim MAX6639 fan-speed controller. > > Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io> > Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com> > --- > 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@9elements.com> > + > +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. Thanks, Conor.
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@sylv.io> >> >> Add binding documentation for Maxim MAX6639 fan-speed controller. >> >> Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io> >> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com> >> --- >> 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@9elements.com> >> + >> +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. Guenter
On 03/08/2023 16:43, Naresh Solanki wrote: > From: Marcello Sylvester Bauer <sylv@sylv.io> > > Add binding documentation for Maxim MAX6639 fan-speed controller. > > Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io> > Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com> > --- ... > +examples: > + - | > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + fan-controller@10 { > + compatible = "maxim,max6639"; > + reg = <0x10>; I wished the examples were a bit more complete (not only 40% of your properties). Well, that's not a stopper, so: Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
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@sylv.io> >>> >>> Add binding documentation for Maxim MAX6639 fan-speed controller. >>> >>> Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io> >>> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com> >>> --- >>> 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@9elements.com> >>> + >>> +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
On Thu, Aug 03, 2023 at 04:43:59PM +0200, Naresh Solanki wrote: > From: Marcello Sylvester Bauer <sylv@sylv.io> > > Add binding documentation for Maxim MAX6639 fan-speed controller. > > Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io> > Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Applied to hwmon-next. Thanks, Guenter > --- > 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 > > > base-commit: cb7022b8976e3c4d12cea2e7bb820a2944e2fd7b > > 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@9elements.com> > + > +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. > + > + Datasheets: > + https://datasheets.maximintegrated.com/en/ds/MAX6639-MAX6639F.pdf > + > +properties: > + compatible: > + enum: > + - maxim,max6639 > + > + reg: > + maxItems: 1 > + > + fan-supply: > + description: Phandle to the regulator that provides power to the fan. > + > + interrupts: > + maxItems: 1 > + > + 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 > + > +required: > + - compatible > + - reg > + > +additionalProperties: false > + > +examples: > + - | > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + fan-controller@10 { > + compatible = "maxim,max6639"; > + reg = <0x10>; > + }; > + }; > +...
On Fri, Aug 04, 2023 at 09:10:37AM -0700, 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@sylv.io> > > > > > > Add binding documentation for Maxim MAX6639 fan-speed controller. > > > > > > Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io> > > > Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com> > > > --- > > > 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@9elements.com> > > > + > > > +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. I thought what was done earlier in this series was somewhat close. And there are some bindings that already look pretty close to what a common binding should. But it seems no one wants to worry about more than their 1 device. In case it's not clear, as-is, this binding is a NAK for me. > 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. I'm fine with that too. Just keep kicking that can... Rob
On 8/10/23 16:11, Rob Herring wrote: > On Fri, Aug 04, 2023 at 09:10:37AM -0700, 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@sylv.io> >>>> >>>> Add binding documentation for Maxim MAX6639 fan-speed controller. >>>> >>>> Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io> >>>> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com> >>>> --- >>>> 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@9elements.com> >>>> + >>>> +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. > > I thought what was done earlier in this series was somewhat close. And > there are some bindings that already look pretty close to what a common > binding should. But it seems no one wants to worry about more than their > 1 device. > > In case it's not clear, as-is, this binding is a NAK for me. > Ok, I'll drop it. Guenter >> 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. > > I'm fine with that too. Just keep kicking that can... > > Rob
Hi Rob, On Fri, 11 Aug 2023 at 04:41, Rob Herring <robh@kernel.org> wrote: > > On Fri, Aug 04, 2023 at 09:10:37AM -0700, 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@sylv.io> > > > > > > > > Add binding documentation for Maxim MAX6639 fan-speed controller. > > > > > > > > Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io> > > > > Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com> > > > > --- > > > > 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@9elements.com> > > > > + > > > > +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. > > I thought what was done earlier in this series was somewhat close. And > there are some bindings that already look pretty close to what a common > binding should. But it seems no one wants to worry about more than their > 1 device. The DT binding for common fan properties is: https://lore.kernel.org/lkml/20221130144626.GA2647609@roeck-us.net/t/#m15ce07c3c43c46506acc389bf24d616646e05653 I wasn't sure on how to address properties for DC controlled fans. Regards, Naresh > > In case it's not clear, as-is, this binding is a NAK for me. > > > 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. > > I'm fine with that too. Just keep kicking that can... > > Rob
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@9elements.com> + +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. + + Datasheets: + https://datasheets.maximintegrated.com/en/ds/MAX6639-MAX6639F.pdf + +properties: + compatible: + enum: + - maxim,max6639 + + reg: + maxItems: 1 + + fan-supply: + description: Phandle to the regulator that provides power to the fan. + + interrupts: + maxItems: 1 + + 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 + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + fan-controller@10 { + compatible = "maxim,max6639"; + reg = <0x10>; + }; + }; +...