Message ID | 20230420111759.2687001-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:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp256514vqo; Thu, 20 Apr 2023 04:23:26 -0700 (PDT) X-Google-Smtp-Source: AKy350Z4sC3odeYKn7tcDVjLkaSJaHqYDwyKqrMj6LXo173OBuJ7gJ5uPkeg+yJs2VFDAqkW04lh X-Received: by 2002:a05:6a20:3ca4:b0:f0:a92a:5088 with SMTP id b36-20020a056a203ca400b000f0a92a5088mr1765142pzj.44.1681989806017; Thu, 20 Apr 2023 04:23:26 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1681989806; cv=none; d=google.com; s=arc-20160816; b=CUeCzd2uT8UP0gibQJJnpZnOwml5hDveKd4phYPx/Kh2g+iBQsB40Vey8oydvLRqqV CnLD2xqeqFbOMxndlG7+NogERenCGvDiX+8K/5ZYJy5wRxgPwAw/oaeKI9zhhH+RHGjT foRgg091IA0gzPLiSj8WykL4weYcDEaVA7z/31gNKVO9h8LoFi1FxXoLDzTnH9q3GwRk fnoa3slJFr7mHT66uzCs/l3yHXMF2YxWUE+6g4y7+jNCYuTmTy41R0RpLvbvYWwWtEEG SOJybl5sEcE8kIUn+UInoQCp2EOoyhDGLbiemfyK7+damPEkNHIUOaCMGa4P1q9RVIFi B6dw== 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=RziL4H96Ec3j032prmOzYzjQ6nh/ZxH/lsLBU5oyee0=; b=pWHeZ9Jzd82LrZ86JdJVDd8SzrFaGXgx97T8B/6gE2kkwg7aDBVg9cuJuLinCJ1RzA yQofmy5AdLxLHcUXRENm2+ehvIQYTkmF5KfBEhT0ZNhxbUi0emnYUcAzregfu2QVVmWE TrSnK32kS/nb2jIU3CEXGMYQrxmJPpLiLUNLhH5OKW2L0SpoIxisI4thiUjemWNvQiO3 M3feNlwcePF++k4PnRivWPmKdaI/vOWMnHfJ+6OFG9COd1EkexK+DdLQJwenAATMwL+d 5CyMeZVuyxX3e5NhP+u69COpH5/VArMQ1G/9js4/OX29owcMPXRAxwsoKw9DxqQd3NEc HlbQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@9elements.com header.s=google header.b=aWaeNs6T; 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 c26-20020a630d1a000000b004dffd0e0409si1443483pgl.518.2023.04.20.04.23.13; Thu, 20 Apr 2023 04:23:25 -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=aWaeNs6T; 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 S232304AbjDTLVZ (ORCPT <rfc822;cjcooper78@gmail.com> + 99 others); Thu, 20 Apr 2023 07:21:25 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47202 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234264AbjDTLU7 (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 20 Apr 2023 07:20:59 -0400 Received: from mail-wm1-x331.google.com (mail-wm1-x331.google.com [IPv6:2a00:1450:4864:20::331]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E872A4C37 for <linux-kernel@vger.kernel.org>; Thu, 20 Apr 2023 04:19:02 -0700 (PDT) Received: by mail-wm1-x331.google.com with SMTP id o9-20020a05600c510900b003f17012276fso2873093wms.4 for <linux-kernel@vger.kernel.org>; Thu, 20 Apr 2023 04:19:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=9elements.com; s=google; t=1681989484; x=1684581484; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=RziL4H96Ec3j032prmOzYzjQ6nh/ZxH/lsLBU5oyee0=; b=aWaeNs6T1pRUJx87FY4CL5DqfD3UrRFGn9M6V1V3WjPfy7SkOPPEXzX5mnBI7VI6SW RdGHP6TAeHOb7kUKZyeBvkJrKIbLpEDG4mb2cCqtRi134fu3YHR1hQJqM4MnoNIixTS6 0PPKT60S8CUzPiTQdZWtJpiGCXYGlLM1vY4FTIXXwaE/F4iZoWqxP0ZLsNCOeaxEA7eX eqP2kOj53eQ/CCNjGp/yPPUauVDlmT3U07+mR3ONCVO7NoPGgAtXqQ4AYP8gcRYnfRgY K0PCR3b0xTkGlbXmdkbnezu8TLc83siwgYOvfVgEma9q/ri7PsN1FfMxnkhWtFqomGBW lD3Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1681989484; x=1684581484; 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=RziL4H96Ec3j032prmOzYzjQ6nh/ZxH/lsLBU5oyee0=; b=IyD1PAT3VQsnRL33ho83OUNtI7rdzuehY6xBjCsoEhNUr0b3VzmuOTxp1XEQjhwE51 YOkH3YMQ7yF1+UJ8x+3qwHSIFEZr2PAWRzXajlr4rQOoAiEsi1YPgt8jZI6Mz0iKCyP+ TTlZs/o7K0qVkiQ3Qwvn3PPZBLexmqgSAhPiFJZio6AG+j0Uja12Dx1RcAShZbjiOubZ WTaARkrCtanMqt0zlJzSQ0JznRRPA0ApRohvuCFgcbWD7Tb2iOM4+VubxsP91l4Uh4Zb feM3BawVnR9Tiaa5wo1MaArMXs6o9ESUrmydtFD/PAJmIW+grOdGOhYWlcjkWXY1NKi8 aIPg== X-Gm-Message-State: AAQBX9fdov9nbVH9EE1xwSi/5qkXW+9MGlxdurEICJr1AVnbpQ3GmYGA fmrJzmRosKXdFOnO6NYoi90aLw== X-Received: by 2002:a1c:7315:0:b0:3f1:70cf:a2d9 with SMTP id d21-20020a1c7315000000b003f170cfa2d9mr991160wmb.9.1681989484048; Thu, 20 Apr 2023 04:18:04 -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 h3-20020a5d5043000000b002c70ce264bfsm1680980wrt.76.2023.04.20.04.18.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 20 Apr 2023 04:18:03 -0700 (PDT) From: Naresh Solanki <naresh.solanki@9elements.com> X-Google-Original-From: Naresh Solanki <Naresh.Solanki@9elements.com> To: Jean Delvare <jdelvare@suse.com>, Guenter Roeck <linux@roeck-us.net>, Rob Herring <robh+dt@kernel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.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 v2 1/2] dt-bindings: hwmon: Add max6639 Date: Thu, 20 Apr 2023 13:17:57 +0200 Message-Id: <20230420111759.2687001-1-Naresh.Solanki@9elements.com> X-Mailer: git-send-email 2.39.1 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_NONE, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable 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: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1763694142465361005?= X-GMAIL-MSGID: =?utf-8?q?1763694142465361005?= |
Series |
[v2,1/2] dt-bindings: hwmon: Add max6639
|
|
Commit Message
Naresh Solanki
April 20, 2023, 11:17 a.m. UTC
From: Marcello Sylvester Bauer <sylv@sylv.io> Add Devicetree binding documentation for Maxim MAX6639 temperature monitor with PWM fan-speed controller. Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com> ... Changes in V2: - Update subject - Drop blank lines --- .../bindings/hwmon/maxim,max6639.yaml | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml base-commit: b8610e0fe08f1476895f141be60f8dd842f0adac
Comments
On 20/04/2023 13:17, Naresh Solanki wrote: > From: Marcello Sylvester Bauer <sylv@sylv.io> > > Add Devicetree binding documentation for Maxim MAX6639 temperature > monitor with PWM fan-speed controller. > > Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io> > Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com> > ... Wrong syntax. '...' do not mean anything, it's --- > Changes in V2: > - Update subject > - Drop blank lines > --- > .../bindings/hwmon/maxim,max6639.yaml | 52 +++++++++++++++++++ > 1 file changed, 52 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..1aaedfd7cee0 > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml > @@ -0,0 +1,52 @@ > +# 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 What is this device? 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 > + > + '#address-cells': > + const: 1 > + > + '#size-cells': > + const: 0 Why do you need these two properties? Anyway, the binding looks incomplete. Where are the supplies? Interrupts? > + > +required: > + - compatible > + - reg > + > +additionalProperties: false > + > +examples: > + - | > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + fan-controller@10 { > + compatible = "maxim,max6639"; > + reg = <0x10>; > + }; > + }; > +... > > base-commit: b8610e0fe08f1476895f141be60f8dd842f0adac Best regards, Krzysztof
Hi Krzysztof, On 21-04-2023 01:07 pm, Krzysztof Kozlowski wrote: > On 20/04/2023 13:17, Naresh Solanki wrote: >> From: Marcello Sylvester Bauer <sylv@sylv.io> >> >> Add Devicetree binding documentation for Maxim MAX6639 temperature >> monitor with PWM fan-speed controller. >> >> Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io> >> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com> >> ... > > Wrong syntax. '...' do not mean anything, it's --- Ack > >> Changes in V2: >> - Update subject >> - Drop blank lines >> --- >> .../bindings/hwmon/maxim,max6639.yaml | 52 +++++++++++++++++++ >> 1 file changed, 52 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..1aaedfd7cee0 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml >> @@ -0,0 +1,52 @@ >> +# 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 > > What is this device? fan controller? Yes Fan controller. Do you want me to update the title here as: "Maxim MAC6639 2 channel fan controller & temperature monitor" ? > >> + >> +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 >> + >> + '#address-cells': >> + const: 1 >> + >> + '#size-cells': >> + const: 0 > > Why do you need these two properties? Ack. Will remove them. > > Anyway, the binding looks incomplete. Where are the supplies? Interrupts? This patch just adds basic support to the existing platform driver. Intention is to be able to call the driver from DT with basic initialization from driver the existing driver. > >> + >> +required: >> + - compatible >> + - reg >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + i2c { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + fan-controller@10 { >> + compatible = "maxim,max6639"; >> + reg = <0x10>; >> + }; >> + }; >> +... >> >> base-commit: b8610e0fe08f1476895f141be60f8dd842f0adac > > Best regards, > Krzysztof > Regards, Naresh
On 24/04/2023 11:18, Naresh Solanki wrote: >>> Changes in V2: >>> - Update subject >>> - Drop blank lines >>> --- >>> .../bindings/hwmon/maxim,max6639.yaml | 52 +++++++++++++++++++ >>> 1 file changed, 52 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..1aaedfd7cee0 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml >>> @@ -0,0 +1,52 @@ >>> +# 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 >> >> What is this device? fan controller? > Yes Fan controller. > Do you want me to update the title here as: > "Maxim MAC6639 2 channel fan controller & temperature monitor" ? Enough would be: 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 >>> + >>> + '#address-cells': >>> + const: 1 >>> + >>> + '#size-cells': >>> + const: 0 >> >> Why do you need these two properties? > Ack. Will remove them. >> >> Anyway, the binding looks incomplete. Where are the supplies? Interrupts? > This patch just adds basic support to the existing platform driver. > Intention is to be able to call the driver from DT with basic > initialization from driver the existing driver. Bindings should be rather complete. Here the datasheet is accessible and few properties quite obvious, so I don't see a reason to skip them. Best regards, Krzysztof
Hi Krzysztof, On 24-04-2023 03:12 pm, Krzysztof Kozlowski wrote: > On 24/04/2023 11:18, Naresh Solanki wrote: > >>>> Changes in V2: >>>> - Update subject >>>> - Drop blank lines >>>> --- >>>> .../bindings/hwmon/maxim,max6639.yaml | 52 +++++++++++++++++++ >>>> 1 file changed, 52 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..1aaedfd7cee0 >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml >>>> @@ -0,0 +1,52 @@ >>>> +# 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 >>> >>> What is this device? fan controller? >> Yes Fan controller. >> Do you want me to update the title here as: >> "Maxim MAC6639 2 channel fan controller & temperature monitor" ? > > Enough would be: > Maxim MAX6639 Fan Controller Ack > > >> >>> >>>> + >>>> +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 >>>> + >>>> + '#address-cells': >>>> + const: 1 >>>> + >>>> + '#size-cells': >>>> + const: 0 >>> >>> Why do you need these two properties? >> Ack. Will remove them. >>> >>> Anyway, the binding looks incomplete. Where are the supplies? Interrupts? >> This patch just adds basic support to the existing platform driver. >> Intention is to be able to call the driver from DT with basic >> initialization from driver the existing driver. > > Bindings should be rather complete. Here the datasheet is accessible and > few properties quite obvious, so I don't see a reason to skip them. I agree with you. But currently the driver which is already merged doesn't has it implemented. And will be working on separate patch to include this later. > > > Best regards, > Krzysztof > Regards, Naresh.
Hi Krzysztof, On 24-04-2023 03:23 pm, Naresh Solanki wrote: > Hi Krzysztof, > > On 24-04-2023 03:12 pm, Krzysztof Kozlowski wrote: >> On 24/04/2023 11:18, Naresh Solanki wrote: >> >>>>> Changes in V2: >>>>> - Update subject >>>>> - Drop blank lines >>>>> --- >>>>> .../bindings/hwmon/maxim,max6639.yaml | 52 >>>>> +++++++++++++++++++ >>>>> 1 file changed, 52 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..1aaedfd7cee0 >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml >>>>> @@ -0,0 +1,52 @@ >>>>> +# 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 >>>> >>>> What is this device? fan controller? >>> Yes Fan controller. >>> Do you want me to update the title here as: >>> "Maxim MAC6639 2 channel fan controller & temperature monitor" ? >> >> Enough would be: >> Maxim MAX6639 Fan Controller > Ack >> >> >>> >>>> >>>>> + >>>>> +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 >>>>> + >>>>> + '#address-cells': >>>>> + const: 1 >>>>> + >>>>> + '#size-cells': >>>>> + const: 0 >>>> >>>> Why do you need these two properties? >>> Ack. Will remove them. >>>> >>>> Anyway, the binding looks incomplete. Where are the supplies? >>>> Interrupts? >>> This patch just adds basic support to the existing platform driver. >>> Intention is to be able to call the driver from DT with basic >>> initialization from driver the existing driver. >> >> Bindings should be rather complete. Here the datasheet is accessible and >> few properties quite obvious, so I don't see a reason to skip them. > I agree with you. But currently the driver which is already merged > doesn't has it implemented. > And will be working on separate patch to include this later. Please let me know if this approach is acceptable, or if there are any other suggestions or concerns that you have. Thanks, Naresh >> >> >> Best regards, >> Krzysztof >> > > Regards, > Naresh.
On 03/05/2023 10:26, Naresh Solanki wrote: > Hi Krzysztof, > > On 24-04-2023 03:23 pm, Naresh Solanki wrote: >> Hi Krzysztof, >> >> On 24-04-2023 03:12 pm, Krzysztof Kozlowski wrote: >>> On 24/04/2023 11:18, Naresh Solanki wrote: >>> >>>>>> Changes in V2: >>>>>> - Update subject >>>>>> - Drop blank lines >>>>>> --- >>>>>> .../bindings/hwmon/maxim,max6639.yaml | 52 >>>>>> +++++++++++++++++++ >>>>>> 1 file changed, 52 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..1aaedfd7cee0 >>>>>> --- /dev/null >>>>>> +++ b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml >>>>>> @@ -0,0 +1,52 @@ >>>>>> +# 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 >>>>> >>>>> What is this device? fan controller? >>>> Yes Fan controller. >>>> Do you want me to update the title here as: >>>> "Maxim MAC6639 2 channel fan controller & temperature monitor" ? >>> >>> Enough would be: >>> Maxim MAX6639 Fan Controller >> Ack >>> >>> >>>> >>>>> >>>>>> + >>>>>> +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 >>>>>> + >>>>>> + '#address-cells': >>>>>> + const: 1 >>>>>> + >>>>>> + '#size-cells': >>>>>> + const: 0 >>>>> >>>>> Why do you need these two properties? >>>> Ack. Will remove them. >>>>> >>>>> Anyway, the binding looks incomplete. Where are the supplies? >>>>> Interrupts? >>>> This patch just adds basic support to the existing platform driver. >>>> Intention is to be able to call the driver from DT with basic >>>> initialization from driver the existing driver. >>> >>> Bindings should be rather complete. Here the datasheet is accessible and >>> few properties quite obvious, so I don't see a reason to skip them. >> I agree with you. But currently the driver which is already merged >> doesn't has it implemented. >> And will be working on separate patch to include this later. > Please let me know if this approach is acceptable, or if there are any > other suggestions or concerns that you have. You are adding new bindings, so what does the driver has to do with it? Best regards, Krzysztof
Hi Krzysztof, On 03-05-2023 09:48 pm, Krzysztof Kozlowski wrote: > On 03/05/2023 10:26, Naresh Solanki wrote: >> Hi Krzysztof, >> >> On 24-04-2023 03:23 pm, Naresh Solanki wrote: >>> Hi Krzysztof, >>> >>> On 24-04-2023 03:12 pm, Krzysztof Kozlowski wrote: >>>> On 24/04/2023 11:18, Naresh Solanki wrote: >>>> >>>>>>> Changes in V2: >>>>>>> - Update subject >>>>>>> - Drop blank lines >>>>>>> --- >>>>>>> .../bindings/hwmon/maxim,max6639.yaml | 52 >>>>>>> +++++++++++++++++++ >>>>>>> 1 file changed, 52 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..1aaedfd7cee0 >>>>>>> --- /dev/null >>>>>>> +++ b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml >>>>>>> @@ -0,0 +1,52 @@ >>>>>>> +# 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 >>>>>> >>>>>> What is this device? fan controller? >>>>> Yes Fan controller. >>>>> Do you want me to update the title here as: >>>>> "Maxim MAC6639 2 channel fan controller & temperature monitor" ? >>>> >>>> Enough would be: >>>> Maxim MAX6639 Fan Controller >>> Ack >>>> >>>> >>>>> >>>>>> >>>>>>> + >>>>>>> +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 >>>>>>> + >>>>>>> + '#address-cells': >>>>>>> + const: 1 >>>>>>> + >>>>>>> + '#size-cells': >>>>>>> + const: 0 >>>>>> >>>>>> Why do you need these two properties? >>>>> Ack. Will remove them. >>>>>> >>>>>> Anyway, the binding looks incomplete. Where are the supplies? >>>>>> Interrupts? >>>>> This patch just adds basic support to the existing platform driver. >>>>> Intention is to be able to call the driver from DT with basic >>>>> initialization from driver the existing driver. >>>> >>>> Bindings should be rather complete. Here the datasheet is accessible and >>>> few properties quite obvious, so I don't see a reason to skip them. >>> I agree with you. But currently the driver which is already merged >>> doesn't has it implemented. >>> And will be working on separate patch to include this later. >> Please let me know if this approach is acceptable, or if there are any >> other suggestions or concerns that you have. > > You are adding new bindings, so what does the driver has to do with it? The reason for adding these new bindings is to enable the use of the driver on my machine. Without the compatible string, it would not be possible to use the driver. Currently, the driver initializes the device with defaults, which is good enough for my application. Also, as you previously pointed out, it uses the optional 'fan-supply' which will be included in the next patch revision. I hope this clarifies my reasoning. Could you kindly confirm if we can proceed with this approach? Best Regards, ~Naresh > > Best regards, > Krzysztof >
On 04/05/2023 11:47, Naresh Solanki wrote: > Hi Krzysztof, > > On 03-05-2023 09:48 pm, Krzysztof Kozlowski wrote: >> On 03/05/2023 10:26, Naresh Solanki wrote: >>> Hi Krzysztof, >>> >>> On 24-04-2023 03:23 pm, Naresh Solanki wrote: >>>> Hi Krzysztof, >>>> >>>> On 24-04-2023 03:12 pm, Krzysztof Kozlowski wrote: >>>>> On 24/04/2023 11:18, Naresh Solanki wrote: >>>>> >>>>>>>> Changes in V2: >>>>>>>> - Update subject >>>>>>>> - Drop blank lines >>>>>>>> --- >>>>>>>> .../bindings/hwmon/maxim,max6639.yaml | 52 >>>>>>>> +++++++++++++++++++ >>>>>>>> 1 file changed, 52 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..1aaedfd7cee0 >>>>>>>> --- /dev/null >>>>>>>> +++ b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml >>>>>>>> @@ -0,0 +1,52 @@ >>>>>>>> +# 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 >>>>>>> >>>>>>> What is this device? fan controller? >>>>>> Yes Fan controller. >>>>>> Do you want me to update the title here as: >>>>>> "Maxim MAC6639 2 channel fan controller & temperature monitor" ? >>>>> >>>>> Enough would be: >>>>> Maxim MAX6639 Fan Controller >>>> Ack >>>>> >>>>> >>>>>> >>>>>>> >>>>>>>> + >>>>>>>> +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 >>>>>>>> + >>>>>>>> + '#address-cells': >>>>>>>> + const: 1 >>>>>>>> + >>>>>>>> + '#size-cells': >>>>>>>> + const: 0 >>>>>>> >>>>>>> Why do you need these two properties? >>>>>> Ack. Will remove them. >>>>>>> >>>>>>> Anyway, the binding looks incomplete. Where are the supplies? >>>>>>> Interrupts? >>>>>> This patch just adds basic support to the existing platform driver. >>>>>> Intention is to be able to call the driver from DT with basic >>>>>> initialization from driver the existing driver. >>>>> >>>>> Bindings should be rather complete. Here the datasheet is accessible and >>>>> few properties quite obvious, so I don't see a reason to skip them. >>>> I agree with you. But currently the driver which is already merged >>>> doesn't has it implemented. >>>> And will be working on separate patch to include this later. >>> Please let me know if this approach is acceptable, or if there are any >>> other suggestions or concerns that you have. >> >> You are adding new bindings, so what does the driver has to do with it? > The reason for adding these new bindings is to enable the use of the > driver on my machine. Without the compatible string, it would not be > possible to use the driver. > > Currently, the driver initializes the device with defaults, which is > good enough for my application. Also, as you previously pointed out, it > uses the optional 'fan-supply' which will be included in the next patch > revision. > > I hope this clarifies my reasoning. Could you kindly confirm if we can > proceed with this approach? No, we cannot, because we asked you to fix things there. Your entire explanation about compatible and driver is not related to the comment you received: bindings should be complete. You argue that bindings do not have to be complete, because of something with driver. This is not related. Bindings are not for driver. Best regards, Krzysztof
Hi Krzysztof, On 04-05-2023 04:17 pm, Krzysztof Kozlowski wrote: > On 04/05/2023 11:47, Naresh Solanki wrote: >> Hi Krzysztof, >> >> On 03-05-2023 09:48 pm, Krzysztof Kozlowski wrote: >>> On 03/05/2023 10:26, Naresh Solanki wrote: >>>> Hi Krzysztof, >>>> >>>> On 24-04-2023 03:23 pm, Naresh Solanki wrote: >>>>> Hi Krzysztof, >>>>> >>>>> On 24-04-2023 03:12 pm, Krzysztof Kozlowski wrote: >>>>>> On 24/04/2023 11:18, Naresh Solanki wrote: >>>>>> >>>>>>>>> Changes in V2: >>>>>>>>> - Update subject >>>>>>>>> - Drop blank lines >>>>>>>>> --- >>>>>>>>> .../bindings/hwmon/maxim,max6639.yaml | 52 >>>>>>>>> +++++++++++++++++++ >>>>>>>>> 1 file changed, 52 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..1aaedfd7cee0 >>>>>>>>> --- /dev/null >>>>>>>>> +++ b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml >>>>>>>>> @@ -0,0 +1,52 @@ >>>>>>>>> +# 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 >>>>>>>> >>>>>>>> What is this device? fan controller? >>>>>>> Yes Fan controller. >>>>>>> Do you want me to update the title here as: >>>>>>> "Maxim MAC6639 2 channel fan controller & temperature monitor" ? >>>>>> >>>>>> Enough would be: >>>>>> Maxim MAX6639 Fan Controller >>>>> Ack >>>>>> >>>>>> >>>>>>> >>>>>>>> >>>>>>>>> + >>>>>>>>> +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 >>>>>>>>> + >>>>>>>>> + '#address-cells': >>>>>>>>> + const: 1 >>>>>>>>> + >>>>>>>>> + '#size-cells': >>>>>>>>> + const: 0 >>>>>>>> >>>>>>>> Why do you need these two properties? >>>>>>> Ack. Will remove them. >>>>>>>> >>>>>>>> Anyway, the binding looks incomplete. Where are the supplies? >>>>>>>> Interrupts? >>>>>>> This patch just adds basic support to the existing platform driver. >>>>>>> Intention is to be able to call the driver from DT with basic >>>>>>> initialization from driver the existing driver. >>>>>> >>>>>> Bindings should be rather complete. Here the datasheet is accessible and >>>>>> few properties quite obvious, so I don't see a reason to skip them. >>>>> I agree with you. But currently the driver which is already merged >>>>> doesn't has it implemented. >>>>> And will be working on separate patch to include this later. >>>> Please let me know if this approach is acceptable, or if there are any >>>> other suggestions or concerns that you have. >>> >>> You are adding new bindings, so what does the driver has to do with it? >> The reason for adding these new bindings is to enable the use of the >> driver on my machine. Without the compatible string, it would not be >> possible to use the driver. >> >> Currently, the driver initializes the device with defaults, which is >> good enough for my application. Also, as you previously pointed out, it >> uses the optional 'fan-supply' which will be included in the next patch >> revision. >> >> I hope this clarifies my reasoning. Could you kindly confirm if we can >> proceed with this approach? > > No, we cannot, because we asked you to fix things there. Your entire > explanation about compatible and driver is not related to the comment > you received: bindings should be complete. You argue that bindings do > not have to be complete, because of something with driver. This is not > related. Bindings are not for driver. I understand that complete bindings are important, but as the driver is already merged and functional, my immediate goal is to enable its use on my machine. I will work on a separate patch to include the interrupts in both binding & driver. > > Best regards, > Krzysztof >
On 04/05/2023 13:36, Naresh Solanki wrote: > Hi Krzysztof, > > On 04-05-2023 04:17 pm, Krzysztof Kozlowski wrote: >> On 04/05/2023 11:47, Naresh Solanki wrote: >>> Hi Krzysztof, >>> >>> On 03-05-2023 09:48 pm, Krzysztof Kozlowski wrote: >>>> On 03/05/2023 10:26, Naresh Solanki wrote: >>>>> Hi Krzysztof, >>>>> >>>>> On 24-04-2023 03:23 pm, Naresh Solanki wrote: >>>>>> Hi Krzysztof, >>>>>> >>>>>> On 24-04-2023 03:12 pm, Krzysztof Kozlowski wrote: >>>>>>> On 24/04/2023 11:18, Naresh Solanki wrote: >>>>>>> >>>>>>>>>> Changes in V2: >>>>>>>>>> - Update subject >>>>>>>>>> - Drop blank lines >>>>>>>>>> --- >>>>>>>>>> .../bindings/hwmon/maxim,max6639.yaml | 52 >>>>>>>>>> +++++++++++++++++++ >>>>>>>>>> 1 file changed, 52 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..1aaedfd7cee0 >>>>>>>>>> --- /dev/null >>>>>>>>>> +++ b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml >>>>>>>>>> @@ -0,0 +1,52 @@ >>>>>>>>>> +# 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 >>>>>>>>> >>>>>>>>> What is this device? fan controller? >>>>>>>> Yes Fan controller. >>>>>>>> Do you want me to update the title here as: >>>>>>>> "Maxim MAC6639 2 channel fan controller & temperature monitor" ? >>>>>>> >>>>>>> Enough would be: >>>>>>> Maxim MAX6639 Fan Controller >>>>>> Ack >>>>>>> >>>>>>> >>>>>>>> >>>>>>>>> >>>>>>>>>> + >>>>>>>>>> +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 >>>>>>>>>> + >>>>>>>>>> + '#address-cells': >>>>>>>>>> + const: 1 >>>>>>>>>> + >>>>>>>>>> + '#size-cells': >>>>>>>>>> + const: 0 >>>>>>>>> >>>>>>>>> Why do you need these two properties? >>>>>>>> Ack. Will remove them. >>>>>>>>> >>>>>>>>> Anyway, the binding looks incomplete. Where are the supplies? >>>>>>>>> Interrupts? >>>>>>>> This patch just adds basic support to the existing platform driver. >>>>>>>> Intention is to be able to call the driver from DT with basic >>>>>>>> initialization from driver the existing driver. >>>>>>> >>>>>>> Bindings should be rather complete. Here the datasheet is accessible and >>>>>>> few properties quite obvious, so I don't see a reason to skip them. >>>>>> I agree with you. But currently the driver which is already merged >>>>>> doesn't has it implemented. >>>>>> And will be working on separate patch to include this later. >>>>> Please let me know if this approach is acceptable, or if there are any >>>>> other suggestions or concerns that you have. >>>> >>>> You are adding new bindings, so what does the driver has to do with it? >>> The reason for adding these new bindings is to enable the use of the >>> driver on my machine. Without the compatible string, it would not be >>> possible to use the driver. >>> >>> Currently, the driver initializes the device with defaults, which is >>> good enough for my application. Also, as you previously pointed out, it >>> uses the optional 'fan-supply' which will be included in the next patch >>> revision. >>> >>> I hope this clarifies my reasoning. Could you kindly confirm if we can >>> proceed with this approach? >> >> No, we cannot, because we asked you to fix things there. Your entire >> explanation about compatible and driver is not related to the comment >> you received: bindings should be complete. You argue that bindings do >> not have to be complete, because of something with driver. This is not >> related. Bindings are not for driver. > > I understand that complete bindings are important, but as the driver is > already merged and functional, my immediate goal is to enable its use on > my machine. I will work on a separate patch to include the interrupts in > both binding & driver. I don't care about driver and did not comment about it. I don't understand why do you bring it here all'n over. Best regards, Krzysztof
On 5/4/23 04:36, Naresh Solanki wrote: > Hi Krzysztof, > [ ... ] >> >> No, we cannot, because we asked you to fix things there. Your entire >> explanation about compatible and driver is not related to the comment >> you received: bindings should be complete. You argue that bindings do >> not have to be complete, because of something with driver. This is not >> related. Bindings are not for driver. > > I understand that complete bindings are important, but as the driver is already merged and functional, my immediate goal is to enable its use on my machine. I will work on a separate patch to include the interrupts in both binding & driver. > As a follow-up, since it came up in a separate context: Bindings and driver are independent of each other. _Bindings_ are supposed to be complete. However, the existence of a property in the bindings description does not have to be reflected in the driver. FWIW, you _could_ have added the device to the list of trivial devices. The only really mandatory property is vdd, and every chip has that. All other properties are really about configuration and/or fan properties, and I don't even know how to describe fan properties (such as pulses per revolution, pwm parameters, fan speed limits, the relationship between pwm outputs and fan inputs, the relationship between fan speed input and pwm output, or fan spin-up requirements) in devicetree. Guenter
Hi Guenter, On Wed, 19 Jul 2023 at 23:10, Guenter Roeck <linux@roeck-us.net> wrote: > > On 5/4/23 04:36, Naresh Solanki wrote: > > Hi Krzysztof, > > > [ ... ] > >> > >> No, we cannot, because we asked you to fix things there. Your entire > >> explanation about compatible and driver is not related to the comment > >> you received: bindings should be complete. You argue that bindings do > >> not have to be complete, because of something with driver. This is not > >> related. Bindings are not for driver. > > > > I understand that complete bindings are important, but as the driver is already merged and functional, my immediate goal is to enable its use on my machine. I will work on a separate patch to include the interrupts in both binding & driver. > > > > As a follow-up, since it came up in a separate context: > > Bindings and driver are independent of each other. _Bindings_ > are supposed to be complete. However, the existence of a property > in the bindings description does not have to be reflected in > the driver. > > FWIW, you _could_ have added the device to the list of trivial > devices. The only really mandatory property is vdd, and every > chip has that. All other properties are really about configuration > and/or fan properties, and I don't even know how to describe fan > properties (such as pulses per revolution, pwm parameters, > fan speed limits, the relationship between pwm outputs > and fan inputs, the relationship between fan speed input > and pwm output, or fan spin-up requirements) in devicetree. Thanks for the inputs, Will work on it & keep you posted. Regards, Naresh > > Guenter >
diff --git a/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml new file mode 100644 index 000000000000..1aaedfd7cee0 --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml @@ -0,0 +1,52 @@ +# 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 + +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 + + '#address-cells': + const: 1 + + '#size-cells': + const: 0 + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + fan-controller@10 { + compatible = "maxim,max6639"; + reg = <0x10>; + }; + }; +...