Message ID | 20240214092504.1237402-1-naresh.solanki@9elements.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel+bounces-64955-ouuuleilei=gmail.com@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7300:bc8a:b0:106:860b:bbdd with SMTP id dn10csp1090728dyb; Wed, 14 Feb 2024 01:25:52 -0800 (PST) X-Forwarded-Encrypted: i=3; AJvYcCURhpk15AMAO/uvSGdmH9mE+9l+oNB/+t0peAoOgCPa9RNlb9+2i8ZWHHqE5+Zp/Wqq4UdUyGZLMjqFxUXJMr480o43Ng== X-Google-Smtp-Source: AGHT+IEaGXEVq3A8zBx5RgQnQ8a8TllVkbkrpaK5Ado/mhRJFi85i9eELN5m/I1D8+Vymoah3Myx X-Received: by 2002:a0c:cc05:0:b0:68e:fb5e:3ba3 with SMTP id r5-20020a0ccc05000000b0068efb5e3ba3mr1891449qvk.11.1707902752644; Wed, 14 Feb 2024 01:25:52 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1707902752; cv=pass; d=google.com; s=arc-20160816; b=KjTp0YccAJMrsGDU97IqDo8M6GMFoqmFpC1oEVrsGvEX4orkko4nk0TgTFMRMaWWd1 gCdn9CGklxH6WJQGU6C2BLzD338oMYTyHz6lro7ay8IPfutNE2nmGQio7Y+JYn+BLRTk ddWKO9yOANjv079S+5DfRJREUm0wfDrIt8xq/MLL6X24rTflG98ngbOANKx0L67q8GUo 4rSXANRckafGD8K3T2uo2pVkegKM5yfPWywosUFB82Q/eBNhxiBefyHdtM7j0KWuySlY DsMCbQpjAsEgcM+IfUV/nULahJ9dqlY0Wso6NmvrYHgm+pmtYzzjVHQ6M0OA05Lt0eB6 yQUg== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:message-id:date:subject:cc:to :from:dkim-signature; bh=y041iHqKwu8te/0ASvv6dCCwr8Scx9NdIC+ftOWbyRk=; fh=t6Q6MXOBcVM45Z5JOQgC53CEPhvBC2gEMOcRoYZqkSg=; b=JCsgnCujb4z4s3jNqV+svZhFFwItgSXIGWZQ/wbBj8r1T7/sUiQRqpH3yLLdoJW2aW 6CnGrVD7YdYT2Uppda9ALPclANRMDKBzdmaq82+MqU2tNJu7JovIlCJ9K8scAu8AMXJ8 sw4kyIGZhNWaGm2wHqxSm+Mqu+tNBu2O7CkN67CZ6p12gQzgolUY9QPzxlbBwRhdaip3 dQiILOIU8QgIWjOzMbXfMBwXV6zlvm0EaJ1cTYG7k2CGlCHfyBw1ui2WDfZ6gHsBvWVz DweBn5nt1yKTl2Kr8DPvqTe+UgyN7Rdv2Rqd4Bi0xW0tnPbQO27BHfblYXgEHO4xs5iu CYUQ==; dara=google.com ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@9elements.com header.s=google header.b=K9+wPc5O; arc=pass (i=1 spf=pass spfdomain=9elements.com dkim=pass dkdomain=9elements.com dmarc=pass fromdomain=9elements.com); spf=pass (google.com: domain of linux-kernel+bounces-64955-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-64955-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=9elements.com X-Forwarded-Encrypted: i=2; AJvYcCXS4pgYwioRDt1DlOiQ/V+CODOedoK4+AQQTyG/JA7lym0QzMVLTd5g40uM8v2R32UF4zZwYVMDH+NIRfD4C4VKHQLOAQ== Received: from ny.mirrors.kernel.org (ny.mirrors.kernel.org. [2604:1380:45d1:ec00::1]) by mx.google.com with ESMTPS id if7-20020a0562141c4700b0068efcc3453dsi1582174qvb.67.2024.02.14.01.25.52 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 14 Feb 2024 01:25:52 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-64955-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1; Authentication-Results: mx.google.com; dkim=pass header.i=@9elements.com header.s=google header.b=K9+wPc5O; arc=pass (i=1 spf=pass spfdomain=9elements.com dkim=pass dkdomain=9elements.com dmarc=pass fromdomain=9elements.com); spf=pass (google.com: domain of linux-kernel+bounces-64955-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-64955-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=9elements.com Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ny.mirrors.kernel.org (Postfix) with ESMTPS id 7006C1C22169 for <ouuuleilei@gmail.com>; Wed, 14 Feb 2024 09:25:52 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 900881643E; Wed, 14 Feb 2024 09:25:21 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=9elements.com header.i=@9elements.com header.b="K9+wPc5O" Received: from mail-wr1-f54.google.com (mail-wr1-f54.google.com [209.85.221.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0E70512B69 for <linux-kernel@vger.kernel.org>; Wed, 14 Feb 2024 09:25:15 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.54 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1707902718; cv=none; b=HMPV5c9hUOZZtMFcyBt0QVwwoiwiCEljaNFHOOtqI9b3wZIimswc9X8xIAwkdMKSwcyhG4Dzj2HKTFZ1qt7GaPaIcT5i4IlUc/dPpcRWRmGBLpeFJETb/4N7u90VRghngzzFXOAOor+i2foJUT/71Iu3ySO6OnHStPLyDWZ7vgA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1707902718; c=relaxed/simple; bh=TLbgnEAQe8QDK0qYRbwk6kDfqKEpbc9popgQQqa1quc=; h=From:To:Cc:Subject:Date:Message-ID:MIME-Version; b=sB7u1HKNssoqg5Zrf1g+3qXurxQEcwzxWMenqI1VDINkfqwLtXUVF82vPwi317vnxPv+iLsYbx7SxjilU/DiuJ8qS/hpYANtzE/ABcBaPz4jHkQ3UySN4Bm6lUExMLrFeW4PGlYHvlPLGtxZslurrZkCuAZlQ56rvvZy3GcssaY= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=9elements.com; spf=pass smtp.mailfrom=9elements.com; dkim=pass (2048-bit key) header.d=9elements.com header.i=@9elements.com header.b=K9+wPc5O; arc=none smtp.client-ip=209.85.221.54 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=9elements.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=9elements.com Received: by mail-wr1-f54.google.com with SMTP id ffacd0b85a97d-33b401fd72bso3465903f8f.3 for <linux-kernel@vger.kernel.org>; Wed, 14 Feb 2024 01:25:15 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=9elements.com; s=google; t=1707902714; x=1708507514; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=y041iHqKwu8te/0ASvv6dCCwr8Scx9NdIC+ftOWbyRk=; b=K9+wPc5OG/5e5hKH+3L+NHXEIysK4FDawD86LzWMeGXdd/hVglUUyQC0qMdAVgeILC jy+iEMKChSBhYGpUUPQ+cXzYCdL7CPLgBepRmpqoNyOu7+aOzfW5c9v/mkH+TMxkOnEK p0Rzxyd2PmiQ7QJlmubY/N4x1uSYp0eHL1LyL51WULHCF8HAq0cos1/9DSAQ2ymdqAXu 5UpQDSCVzmFXe6+ty6foSiPN1WyItALurOuzz87QGgEVuN1L1Vq1SUAzN9gvcfmPVG/i 7aJnbsVJgFtsF0kX/+VOx25ed/piZJNk4tHl7d11oPeywPLso1fbM2fTlr/XTBMQd9tl zoxA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1707902714; x=1708507514; 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=y041iHqKwu8te/0ASvv6dCCwr8Scx9NdIC+ftOWbyRk=; b=MwbEj2ovct7l/IvDhklnFthcwXpNMPz8yq9Q7odepneuh7FsxfBQp5Of/oniKBUFIC owLt+fHLYBOrNRORtX4dC1z3kDQBYxYEivSLEzgaaTssF6piEmZrfKywJwA+I1qjZ+k2 vpxZva+R93qInUy1URSqu1Q/JDuNs1/3CNefTd142pviWD7aR86Xpl+oGf/fiTFlPMLe vdsQRStplbv9s4qrjBzC/kFFuXD+V1QHMdLgN6sJgaHqifrGrbD00Xh/UwCDqFRtWca4 8EiVZ4AVrlpOlgfkKRo4j30JO2iLTCYaliINTmQziHVPI2KGSPo42RbjHFSXntEpOb6V aE3Q== X-Forwarded-Encrypted: i=1; AJvYcCX3MFVfT9dTZe+ceHJMT2sF3YHOcpYBKt5lRDAyLKeFVWxanlIm7bEawCeFEZiG2g+TxSPlCNqOG6TooXvwphe1X+JrKQst+EWNqCD2 X-Gm-Message-State: AOJu0YzgSpqoqnpJiiOvxk6Ygq8kC5ORnHC07920CpltcobAcKQ/nEed kPFqFhXm8LBLNKAj8anaik19wwXqGiH6vQQqXw5MU/NpyNXwtvlu3HpHeI3GGD4= X-Received: by 2002:a5d:51c7:0:b0:33b:5f40:323 with SMTP id n7-20020a5d51c7000000b0033b5f400323mr1101863wrv.51.1707902714217; Wed, 14 Feb 2024 01:25:14 -0800 (PST) X-Forwarded-Encrypted: i=1; AJvYcCUWFyWps/fHcpSSH0HQANSkGfvWsqyVSJ9Q7MtVLH+UpedJsKJn1mpMI8+0buoRgMBl5hkD/N7xMLb0c4pzRk7lFUl/rCu+SUXWJxAdIFR4qB9CWgrXvs6h/lwzlizPzUoVBX5gnE4er6oaiQnd/LwuJHLoFXY5W2af2mRwsGtMFDYFTxGJwelRur3m8E7ERQwdHPhZgApFJVakP8OpL7dI4wfzzaP5ZJUWOPw3zIG6AMNfNxe7VLTXv/grKY56+KDSxj/SR9tdXT94mmaSm6GB9mKJn/sjzUbPPTkhXpD/xfgrcwvOssDfwzjISs/XlW/d2ZoxofHEzHSRYXXd74BvWzwx 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 en8-20020a056000420800b0033b7d9c14desm9114882wrb.5.2024.02.14.01.25.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 14 Feb 2024 01:25:13 -0800 (PST) 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>, Conor Dooley <conor+dt@kernel.org>, Naresh Solanki <naresh.solanki@9elements.com> Cc: mazziesaccount@gmail.com, linux-hwmon@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v2] dt-bindings: hwmon: tda38640: Add interrupt & regulator properties Date: Wed, 14 Feb 2024 14:55:03 +0530 Message-ID: <20240214092504.1237402-1-naresh.solanki@9elements.com> X-Mailer: git-send-email 2.42.0 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: <linux-kernel.vger.kernel.org> List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1789153700429733154 X-GMAIL-MSGID: 1790865836441932328 |
Series |
[v2] dt-bindings: hwmon: tda38640: Add interrupt & regulator properties
|
|
Commit Message
Naresh Solanki
Feb. 14, 2024, 9:25 a.m. UTC
Add properties for interrupt & regulator.
Also update example.
Signed-off-by: Naresh Solanki <naresh.solanki@9elements.com>
---
Changes in v2:
1. Remove TEST=..
2. Update regulator subnode property as vout0
3. Restore commented line in example
4. blank line after interrupts property in example.
---
.../hwmon/pmbus/infineon,tda38640.yaml | 28 +++++++++++++++++++
1 file changed, 28 insertions(+)
base-commit: 7e90b5c295ec1e47c8ad865429f046970c549a66
Comments
On Wed, Feb 14, 2024 at 02:55:03PM +0530, Naresh Solanki wrote: > Add properties for interrupt & regulator. > Also update example. I feel like a broken record. Your patches need to explain _why_ you're doing what you're doing. I can read the diff and see this, but I do not know what the justification for it is. /30 seconds later I really am a broken record, to quote from v1: | Feeling like a broken record, given I am leaving the same comments on | multiple patches. The commit message needs to explain why you're doing | something. I can read the diff and see what you did! https://lore.kernel.org/all/20240126-fleshed-subdued-36bae813e2ba@spud/ The patch itself does look better than the v1, with one minor comment below. Thanks, Conor. > Signed-off-by: Naresh Solanki <naresh.solanki@9elements.com> > > --- > Changes in v2: > 1. Remove TEST=.. > 2. Update regulator subnode property as vout0 > 3. Restore commented line in example > 4. blank line after interrupts property in example. > --- > .../hwmon/pmbus/infineon,tda38640.yaml | 28 +++++++++++++++++++ > 1 file changed, 28 insertions(+) > > diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/infineon,tda38640.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/infineon,tda38640.yaml > index ded1c115764b..a93b3f86ee87 100644 > --- a/Documentation/devicetree/bindings/hwmon/pmbus/infineon,tda38640.yaml > +++ b/Documentation/devicetree/bindings/hwmon/pmbus/infineon,tda38640.yaml > @@ -30,6 +30,23 @@ properties: > unconnected(has internal pull-down). > type: boolean > > + interrupts: > + maxItems: 1 > + > + regulators: > + type: object > + description: > + list of regulators provided by this controller. > + > + properties: > + vout0: Why "vout0" if there's only one output? Is it called that in the documentation? I had a quick check but only saw it called "vout". Are there other related devices that would have multiple regulators that might end up sharing the binding? Thanks, Conor. > + $ref: /schemas/regulator/regulator.yaml# > + type: object > + > + unevaluatedProperties: false > + > + additionalProperties: false > + > required: > - compatible > - reg > @@ -38,6 +55,7 @@ additionalProperties: false > > examples: > - | > + #include <dt-bindings/interrupt-controller/irq.h> > i2c { > #address-cells = <1>; > #size-cells = <0>; > @@ -45,5 +63,15 @@ examples: > tda38640@40 { > compatible = "infineon,tda38640"; > reg = <0x40>; > + > + interrupt-parent = <&smb_pex_cpu0_event>; > + interrupts = <10 IRQ_TYPE_LEVEL_LOW>; > + > + regulators { > + pvnn_main_cpu0: vout0 { > + regulator-name = "pvnn_main_cpu0"; > + regulator-enable-ramp-delay = <200>; > + }; > + }; > }; > }; > > base-commit: 7e90b5c295ec1e47c8ad865429f046970c549a66 > -- > 2.42.0 >
On 2/14/24 09:51, Conor Dooley wrote: > On Wed, Feb 14, 2024 at 02:55:03PM +0530, Naresh Solanki wrote: >> Add properties for interrupt & regulator. >> Also update example. > > I feel like a broken record. Your patches need to explain _why_ you're > doing what you're doing. I can read the diff and see this, but I do not > know what the justification for it is. > > /30 seconds later > I really am a broken record, to quote from v1: > | Feeling like a broken record, given I am leaving the same comments on > | multiple patches. The commit message needs to explain why you're doing > | something. I can read the diff and see what you did! > > https://lore.kernel.org/all/20240126-fleshed-subdued-36bae813e2ba@spud/ > > The patch itself does look better than the v1, with one minor comment > below. > > Thanks, > Conor. > >> Signed-off-by: Naresh Solanki <naresh.solanki@9elements.com> >> >> --- >> Changes in v2: >> 1. Remove TEST=.. >> 2. Update regulator subnode property as vout0 >> 3. Restore commented line in example >> 4. blank line after interrupts property in example. >> --- >> .../hwmon/pmbus/infineon,tda38640.yaml | 28 +++++++++++++++++++ >> 1 file changed, 28 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/infineon,tda38640.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/infineon,tda38640.yaml >> index ded1c115764b..a93b3f86ee87 100644 >> --- a/Documentation/devicetree/bindings/hwmon/pmbus/infineon,tda38640.yaml >> +++ b/Documentation/devicetree/bindings/hwmon/pmbus/infineon,tda38640.yaml >> @@ -30,6 +30,23 @@ properties: >> unconnected(has internal pull-down). >> type: boolean >> >> + interrupts: >> + maxItems: 1 >> + >> + regulators: >> + type: object >> + description: >> + list of regulators provided by this controller. >> + >> + properties: >> + vout0: > > Why "vout0" if there's only one output? Is it called that in the > documentation? I had a quick check but only saw it called "vout". > Are there other related devices that would have multiple regulators > that might end up sharing the binding? > Primarily because that is what the PMBus core generates for the driver because no one including me was aware that this is unacceptable for single-output drivers. We now have commit 88b5970e92d0 ("hwmon: (pmbus/core) Add helper macro to define single pmbus regulator"). I guess we can update the tda38640 driver to use the new macro to register vout instead of vout0. Guenter
On Wed, Feb 14, 2024 at 10:48:52AM -0800, Guenter Roeck wrote: > On 2/14/24 09:51, Conor Dooley wrote: > > On Wed, Feb 14, 2024 at 02:55:03PM +0530, Naresh Solanki wrote: > > > Add properties for interrupt & regulator. > > > Also update example. > > > > I feel like a broken record. Your patches need to explain _why_ you're > > doing what you're doing. I can read the diff and see this, but I do not > > know what the justification for it is. > > > > /30 seconds later > > I really am a broken record, to quote from v1: > > | Feeling like a broken record, given I am leaving the same comments on > > | multiple patches. The commit message needs to explain why you're doing > > | something. I can read the diff and see what you did! > > > > https://lore.kernel.org/all/20240126-fleshed-subdued-36bae813e2ba@spud/ > > > > The patch itself does look better than the v1, with one minor comment > > below. > > > > Thanks, > > Conor. > > > > > Signed-off-by: Naresh Solanki <naresh.solanki@9elements.com> > > > > > > --- > > > Changes in v2: > > > 1. Remove TEST=.. > > > 2. Update regulator subnode property as vout0 > > > 3. Restore commented line in example > > > 4. blank line after interrupts property in example. > > > --- > > > .../hwmon/pmbus/infineon,tda38640.yaml | 28 +++++++++++++++++++ > > > 1 file changed, 28 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/infineon,tda38640.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/infineon,tda38640.yaml > > > index ded1c115764b..a93b3f86ee87 100644 > > > --- a/Documentation/devicetree/bindings/hwmon/pmbus/infineon,tda38640.yaml > > > +++ b/Documentation/devicetree/bindings/hwmon/pmbus/infineon,tda38640.yaml > > > @@ -30,6 +30,23 @@ properties: > > > unconnected(has internal pull-down). > > > type: boolean > > > + interrupts: > > > + maxItems: 1 > > > + > > > + regulators: > > > + type: object > > > + description: > > > + list of regulators provided by this controller. > > > + > > > + properties: > > > + vout0: > > > > Why "vout0" if there's only one output? Is it called that in the > > documentation? I had a quick check but only saw it called "vout". > > Are there other related devices that would have multiple regulators > > that might end up sharing the binding? > > > > Primarily because that is what the PMBus core generates for the driver > because no one including me was aware that this is unacceptable > for single-output drivers. Is it unacceptable? If you're implying that I am saying it is, that's not what I was doing here - I'm just wondering why it was chosen. Numbering when there's only one seems odd, so I was just looking for the rationale. > We now have commit 88b5970e92d0 ("hwmon: > (pmbus/core) Add helper macro to define single pmbus regulator"). > I guess we can update the tda38640 driver to use the new macro > to register vout instead of vout0.
On 2/14/24 11:55, Conor Dooley wrote: [ ... ] >>> Why "vout0" if there's only one output? Is it called that in the >>> documentation? I had a quick check but only saw it called "vout". >>> Are there other related devices that would have multiple regulators >>> that might end up sharing the binding? >>> >> >> Primarily because that is what the PMBus core generates for the driver >> because no one including me was aware that this is unacceptable >> for single-output drivers. > > Is it unacceptable? If you're implying that I am saying it is, that's > not what I was doing here - I'm just wondering why it was chosen. > Numbering when there's only one seems odd, so I was just looking for the > rationale. > Given the tendency of corporate speak (aka "this was a good attempt" for a complete screwup), and since this did come up before, I did interpret it along that line. My apologies if that was not the idea. Still, I really don't know how to resolve this for existing PMBus drivers which do register "vout0" even if there is only a single output regulator. Guenter
On Wed, Feb 14, 2024 at 05:17:04PM -0800, Guenter Roeck wrote: > On 2/14/24 11:55, Conor Dooley wrote: > [ ... ] > > > > Why "vout0" if there's only one output? Is it called that in the > > > > documentation? I had a quick check but only saw it called "vout". > > > > Are there other related devices that would have multiple regulators > > > > that might end up sharing the binding? > > > > > > > > > > Primarily because that is what the PMBus core generates for the driver > > > because no one including me was aware that this is unacceptable > > > for single-output drivers. > > > > Is it unacceptable? If you're implying that I am saying it is, that's > > not what I was doing here - I'm just wondering why it was chosen. > > Numbering when there's only one seems odd, so I was just looking for the > > rationale. > > > > Given the tendency of corporate speak (aka "this was a good attempt" for > a complete screwup), and since this did come up before, I did interpret > it along that line. My apologies if that was not the idea. I'm not gonna go and decree that "vout0" is unacceptable, if it was called that in documentation that I had missed or was convention, I was just gonna say "okay, that sounds reasonable to me". > Still, I really don't know how to resolve this for existing PMBus drivers > which do register "vout0" even if there is only a single output regulator. I had a quick look at that series, none of the devices that I checked out there seem to have documented regulators at all. Some of the devices were only documented in trivial-devices.yaml. Relying on the naming of undocumented child nodes is a bug in those drivers & I guess nobody cares about dtbs_check complaints for those platforms. The example that was linked in the other thread doesn't even use a valid compatible :( https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/aspeed/aspeed-bmc-delta-ahe50dc.dts?id=8d3dea210042f54b952b481838c1e7dfc4ec751d#n21 I guess it uses the i2c device ids to probe on that platform, or have I missed something there? Cheers, Conor.
On 2/15/24 03:48, Conor Dooley wrote: > On Wed, Feb 14, 2024 at 05:17:04PM -0800, Guenter Roeck wrote: >> On 2/14/24 11:55, Conor Dooley wrote: >> [ ... ] >>>>> Why "vout0" if there's only one output? Is it called that in the >>>>> documentation? I had a quick check but only saw it called "vout". >>>>> Are there other related devices that would have multiple regulators >>>>> that might end up sharing the binding? >>>>> >>>> >>>> Primarily because that is what the PMBus core generates for the driver >>>> because no one including me was aware that this is unacceptable >>>> for single-output drivers. >>> >>> Is it unacceptable? If you're implying that I am saying it is, that's >>> not what I was doing here - I'm just wondering why it was chosen. >>> Numbering when there's only one seems odd, so I was just looking for the >>> rationale. >>> >> >> Given the tendency of corporate speak (aka "this was a good attempt" for >> a complete screwup), and since this did come up before, I did interpret >> it along that line. My apologies if that was not the idea. > > I'm not gonna go and decree that "vout0" is unacceptable, if it was > called that in documentation that I had missed or was convention, I was > just gonna say "okay, that sounds reasonable to me". > "convention" only if lack of awareness how regulators are supposed to be named is a convention. >> Still, I really don't know how to resolve this for existing PMBus drivers >> which do register "vout0" even if there is only a single output regulator. > > I had a quick look at that series, none of the devices that I checked > out there seem to have documented regulators at all. Some of the devices > were only documented in trivial-devices.yaml. Relying on the naming of > undocumented child nodes is a bug in those drivers & I guess nobody cares > about dtbs_check complaints for those platforms. The example that was > linked in the other thread doesn't even use a valid compatible :( > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/aspeed/aspeed-bmc-delta-ahe50dc.dts?id=8d3dea210042f54b952b481838c1e7dfc4ec751d#n21 > I guess it uses the i2c device ids to probe on that platform, or have > I missed something there? > I think that is correct. If I recall correctly, the I2C subsystem no longer searches for compatible drivers by only looking at the device id in the compatible node, so I guess one has to list "lm25066" instead of "ti,lm25066" as compatible to get a match in the i2c subsystem. That is of course completely wrong. Guenter
On Sat, Feb 17, 2024 at 07:57:43AM -0800, Guenter Roeck wrote: > On 2/15/24 03:48, Conor Dooley wrote: > > On Wed, Feb 14, 2024 at 05:17:04PM -0800, Guenter Roeck wrote: > > > On 2/14/24 11:55, Conor Dooley wrote: > > > [ ... ] > > > > > > Why "vout0" if there's only one output? Is it called that in the > > > > > > documentation? I had a quick check but only saw it called "vout". > > > > > > Are there other related devices that would have multiple regulators > > > > > > that might end up sharing the binding? > > > > > > > > > > > > > > > > Primarily because that is what the PMBus core generates for the driver > > > > > because no one including me was aware that this is unacceptable > > > > > for single-output drivers. > > > > > > > > Is it unacceptable? If you're implying that I am saying it is, that's > > > > not what I was doing here - I'm just wondering why it was chosen. > > > > Numbering when there's only one seems odd, so I was just looking for the > > > > rationale. > > > > > > > > > > Given the tendency of corporate speak (aka "this was a good attempt" for > > > a complete screwup), and since this did come up before, I did interpret > > > it along that line. My apologies if that was not the idea. > > > > I'm not gonna go and decree that "vout0" is unacceptable, if it was > > called that in documentation that I had missed or was convention, I was > > just gonna say "okay, that sounds reasonable to me". > > > > "convention" only if lack of awareness how regulators are supposed to be named > is a convention. They're "supposed" to be named whatever the binding says they are named, but as we've discovered none of these devices actually have bindings that allow regulators in the first place. I think they should be called whatever they're called in the documentation for the device, which in this case was "vout". > > > Still, I really don't know how to resolve this for existing PMBus drivers > > > which do register "vout0" even if there is only a single output regulator. > > > > I had a quick look at that series, none of the devices that I checked > > out there seem to have documented regulators at all. Some of the devices > > were only documented in trivial-devices.yaml. Relying on the naming of > > undocumented child nodes is a bug in those drivers & I guess nobody cares > > about dtbs_check complaints for those platforms. The example that was > > linked in the other thread doesn't even use a valid compatible :( > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/aspeed/aspeed-bmc-delta-ahe50dc.dts?id=8d3dea210042f54b952b481838c1e7dfc4ec751d#n21 > > I guess it uses the i2c device ids to probe on that platform, or have > > I missed something there? > > > > I think that is correct. If I recall correctly, the I2C subsystem no longer > searches for compatible drivers by only looking at the device id in the > compatible node, so I guess one has to list "lm25066" instead of "ti,lm25066" > as compatible to get a match in the i2c subsystem. That is of course > completely wrong. If the driver is probing based on i2c_device_id matching, is it correct to use DT to probe the regulators? (I don't know, that's not some sort of rhetorical question).
On 2/17/24 12:30, Conor Dooley wrote: > On Sat, Feb 17, 2024 at 07:57:43AM -0800, Guenter Roeck wrote: >> On 2/15/24 03:48, Conor Dooley wrote: >>> On Wed, Feb 14, 2024 at 05:17:04PM -0800, Guenter Roeck wrote: >>>> On 2/14/24 11:55, Conor Dooley wrote: >>>> [ ... ] >>>>>>> Why "vout0" if there's only one output? Is it called that in the >>>>>>> documentation? I had a quick check but only saw it called "vout". >>>>>>> Are there other related devices that would have multiple regulators >>>>>>> that might end up sharing the binding? >>>>>>> >>>>>> >>>>>> Primarily because that is what the PMBus core generates for the driver >>>>>> because no one including me was aware that this is unacceptable >>>>>> for single-output drivers. >>>>> >>>>> Is it unacceptable? If you're implying that I am saying it is, that's >>>>> not what I was doing here - I'm just wondering why it was chosen. >>>>> Numbering when there's only one seems odd, so I was just looking for the >>>>> rationale. >>>>> >>>> >>>> Given the tendency of corporate speak (aka "this was a good attempt" for >>>> a complete screwup), and since this did come up before, I did interpret >>>> it along that line. My apologies if that was not the idea. >>> >>> I'm not gonna go and decree that "vout0" is unacceptable, if it was >>> called that in documentation that I had missed or was convention, I was >>> just gonna say "okay, that sounds reasonable to me". >>> >> >> "convention" only if lack of awareness how regulators are supposed to be named >> is a convention. > > They're "supposed" to be named whatever the binding says they are named, > but as we've discovered none of these devices actually have bindings > that allow regulators in the first place. I think they should be called > whatever they're called in the documentation for the device, which in > this case was "vout". > I would agree. I'd even submit a yaml file extension for the affected drivers to address that, but I realize that I am notoriously bad with doing that, so I won't even try. >>>> Still, I really don't know how to resolve this for existing PMBus drivers >>>> which do register "vout0" even if there is only a single output regulator. >>> >>> I had a quick look at that series, none of the devices that I checked >>> out there seem to have documented regulators at all. Some of the devices >>> were only documented in trivial-devices.yaml. Relying on the naming of >>> undocumented child nodes is a bug in those drivers & I guess nobody cares >>> about dtbs_check complaints for those platforms. The example that was >>> linked in the other thread doesn't even use a valid compatible :( >>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/aspeed/aspeed-bmc-delta-ahe50dc.dts?id=8d3dea210042f54b952b481838c1e7dfc4ec751d#n21 >>> I guess it uses the i2c device ids to probe on that platform, or have >>> I missed something there? >>> >> >> I think that is correct. If I recall correctly, the I2C subsystem no longer >> searches for compatible drivers by only looking at the device id in the >> compatible node, so I guess one has to list "lm25066" instead of "ti,lm25066" >> as compatible to get a match in the i2c subsystem. That is of course >> completely wrong. > > If the driver is probing based on i2c_device_id matching, is it correct to > use DT to probe the regulators? (I don't know, that's not some sort of > rhetorical question). Looking into the lm25066 driver, it actually does support the "ti,lm25066" compatible. Given that, I don't know why the dts file doesn't use it, especially since the dts file was created after the compatible entry was added to the lm25066 driver. Guenter
diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/infineon,tda38640.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/infineon,tda38640.yaml index ded1c115764b..a93b3f86ee87 100644 --- a/Documentation/devicetree/bindings/hwmon/pmbus/infineon,tda38640.yaml +++ b/Documentation/devicetree/bindings/hwmon/pmbus/infineon,tda38640.yaml @@ -30,6 +30,23 @@ properties: unconnected(has internal pull-down). type: boolean + interrupts: + maxItems: 1 + + regulators: + type: object + description: + list of regulators provided by this controller. + + properties: + vout0: + $ref: /schemas/regulator/regulator.yaml# + type: object + + unevaluatedProperties: false + + additionalProperties: false + required: - compatible - reg @@ -38,6 +55,7 @@ additionalProperties: false examples: - | + #include <dt-bindings/interrupt-controller/irq.h> i2c { #address-cells = <1>; #size-cells = <0>; @@ -45,5 +63,15 @@ examples: tda38640@40 { compatible = "infineon,tda38640"; reg = <0x40>; + + interrupt-parent = <&smb_pex_cpu0_event>; + interrupts = <10 IRQ_TYPE_LEVEL_LOW>; + + regulators { + pvnn_main_cpu0: vout0 { + regulator-name = "pvnn_main_cpu0"; + regulator-enable-ramp-delay = <200>; + }; + }; }; };