Received: by 10.213.65.68 with SMTP id h4csp895522imn; Tue, 20 Mar 2018 19:17:32 -0700 (PDT) X-Google-Smtp-Source: AG47ELtW7BvNvPEDKqsCNoOvYqvd3JfkrYQdyevJln//oD+nQxjOM7DMKWnZxrxqwtT/bVMy7oOQ X-Received: by 10.101.75.202 with SMTP id p10mr13864194pgr.339.1521598652241; Tue, 20 Mar 2018 19:17:32 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1521598652; cv=none; d=google.com; s=arc-20160816; b=TJ5Ccs0GzYaUGl5myWlE6N95Gl7PHV7Mp5iAZsfEfo6Oex60POTydYPTt+u5Q8BYqj +uNkgNYSta6ps1GqsE5Zeqwt5q38TgPQhGLhIlH3t/9yRkYop+MIBi83btR0ZlivixId TpSs+EYXMeK+BCLPUiZ31EvOz5HiQ+L85G0qmHWLp5BCBHHzkvcawgh2biVrIv90x0Jv EEdN5VuKs0UNNTRWsLp3FScfpAX37i6sYmnKz6ymPjAPBxXZap3mo+aDwgXshxKsrxVI lE44zaUTG9rG682NiyEKjjwd3ACb8Rrwr7219KvtKWvWLzbHUyBk3bE4aYUM89LBcwJl GWWw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:cc:to:subject:message-id:date:from :references:in-reply-to:mime-version:dkim-signature:dkim-signature :arc-authentication-results; bh=XAqv9zklEvzS7R3kTL4hRfrjBb40DSUMeFXFZPmS4Qw=; b=weGHXKj2JzeDHRZTKjSnuyWG2ulI4tCCZ5RjQfMuSv7c3nH6N8rCZbSHsE8Gy/VzsC kYPhKKVL2EfaMgdEfiQkIzeywahHULIPeVzTYVpJH0U51uMIYqcdvhIbiGi+WIwwYV6U 9vYEtgFFiyuve5r25jXmcQCaddK1kIInWpggsy9bz4HX9qGfvs2AoteoKmcMQ87WfjE0 a7qtEgXNHN2rMwLUEnsAvuGOwih3jdhkUYfuVfLsiYR5ZtOpMCt+aq0/TQzRw1TkSy3P +mbhTiwOFStZ+Tf6+dtZg5epBkz4aTHVt0sAfxGQ+vMdBov6FyU4nh/bqfsBILXx60yF KLQQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@google.com header.s=20161025 header.b=wVAFGFE/; dkim=fail header.i=@chromium.org header.s=google header.b=bbrxmoGP; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=chromium.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id j25si2045659pgn.592.2018.03.20.19.17.17; Tue, 20 Mar 2018 19:17:32 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=fail header.i=@google.com header.s=20161025 header.b=wVAFGFE/; dkim=fail header.i=@chromium.org header.s=google header.b=bbrxmoGP; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=chromium.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751688AbeCUCQR (ORCPT + 99 others); Tue, 20 Mar 2018 22:16:17 -0400 Received: from mail-vk0-f67.google.com ([209.85.213.67]:33099 "EHLO mail-vk0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751472AbeCUCQP (ORCPT ); Tue, 20 Mar 2018 22:16:15 -0400 Received: by mail-vk0-f67.google.com with SMTP id j85so2206620vke.0 for ; Tue, 20 Mar 2018 19:16:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=XAqv9zklEvzS7R3kTL4hRfrjBb40DSUMeFXFZPmS4Qw=; b=wVAFGFE/BIyPL9dQItCZewj7Yi4K0zz2DhtoFzS7z84JKLlq2RVzyXu2oa1OSJW76c kc5nRxMqI/DWZnBikZE0IFCFhzDZOY1ohggFTQDXSIASsQpwHpTvl63ZgxShiooWlTX+ oAo8XvxtdtO6h5Zm1A/La5NUa1i4onG+4IFvCEJVJ4r8KO9bobI0/GEhhykverkdYRow NZ+i3QVOrJxlh99jaL57UJme2LswV1IX2nDW3RuvajmXCH4sKhkZA09IIEE7T5isXXqG NtVWrZC1/TH9NbXMdOCTpsoxtHRW+T6J59SmDH62x86801bUceabHrJmZlngiqY+60yV 2A/Q== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=XAqv9zklEvzS7R3kTL4hRfrjBb40DSUMeFXFZPmS4Qw=; b=bbrxmoGPA/H9CZrFd6EcGuQNXyNWBrwJNbtRuvmcSkZY3NpOTjYHLTkmlH/fmYdyVJ kiB7F7uphMrbSVSfY4tqGVHzr1NCgjc9pTQXOeqUEPRbqwS2xI298R8n78xRyUP+akM8 5S6MIyJo0Gpyc6s5JY+4spnjS/s5sxLrSTJRQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=XAqv9zklEvzS7R3kTL4hRfrjBb40DSUMeFXFZPmS4Qw=; b=n/mTKOJlzXcix/lm5YGqfM6lUWZc2f7I4vTOknFUHn98ckX9z458IGvICK+T0uPZwP +tfhDfM1/jXaxbmmkjGEfRRrE4aU4dOJBtOogYbwVFlA8NMj8KyFxhL9V706Pi648EXA K/TLzw+KjdgFXQItqaDCuqXVVuFonBxQ0fptCtmoFqYGD4SV0lqzRdSULdaXOuh2qqwt hJvvx7jZIlgmnHIxfwQrw2D3yFtuYo/It48pDis8iMBmGpHQApHC07OtpEmLgIhfoMtE nNrSNbz6XjcxKpms08jr6sXjwXtLxhaTzjLsgiE0hkejJ+vKJdkLcFmdVY3/XOoIomwj 5Xww== X-Gm-Message-State: AElRT7FrtUC4PTBPZZ8m8Onwez7p8jJtnscKStIcukycG1rQIwkph2m6 8F7xpifjSQ0AMNg0cWLNLkz2Gu2brv9AqV1/zbOvhA== X-Received: by 10.31.103.69 with SMTP id m5mr2569333vki.160.1521598573705; Tue, 20 Mar 2018 19:16:13 -0700 (PDT) MIME-Version: 1.0 Received: by 10.31.180.195 with HTTP; Tue, 20 Mar 2018 19:16:12 -0700 (PDT) In-Reply-To: References: From: Doug Anderson Date: Tue, 20 Mar 2018 19:16:12 -0700 X-Google-Sender-Auth: PWj77u-i18DZAKlts2dhm9VnZDE Message-ID: Subject: Re: [PATCH 2/2] dt-bindings: regulator: add QCOM RPMh regulator bindings To: David Collins Cc: Mark Brown , Liam Girdwood , Rob Herring , Mark Rutland , linux-arm-msm@vger.kernel.org, Linux ARM , linux-arm@lists.infradead.org, devicetree@vger.kernel.org, LKML , Rajendra Nayak , sboyd@kernel.org Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi On Fri, Mar 16, 2018 at 6:09 PM, David Collins wrote: > Introduce bindings for RPMh regulator devices found on some > Qualcomm Technlogies, Inc. SoCs. These devices allow a given > processor within the SoC to make PMIC regulator requests which > are aggregated within the RPMh hardware block along with requests > from other processors in the SoC to determine the final PMIC > regulator hardware state. > > Signed-off-by: David Collins > --- > .../bindings/regulator/qcom,rpmh-regulator.txt | 246 +++++++++++++++++++++ > 1 file changed, 246 insertions(+) > create mode 100644 Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt > > diff --git a/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt b/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt > new file mode 100644 > index 0000000..2d86306 > --- /dev/null > +++ b/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt > @@ -0,0 +1,246 @@ > +Qualcomm Technologies, Inc. RPMh Regulators > + > +rpmh-regulator devices support PMIC regulator management via the VRM and XOB > +RPMh accelerators. The APPS processor communicates with these hardware blocks > +via an RSC using command packets. The VRM allows changing four parameters for a > +given regulator: enable state, output voltage, operating mode, and minimum > +headroom voltage. The XOB allows changing only a single parameter for a given > +regulator: its enable state. Somewhere in here can you give some context of what VRM and XOB stand for. From the other patch desc it's "voltage regulator manager (VRM) and oscillator buffer (XOB)", but nice to sprinkle that around the first time it's used in documents. ...and, ummmmm, what's an oscillator buffer? Is this really a regulator? It sounds a lot more like a clock enable knob. Are you sure this shouldn't be exposed through the common clock framework? Many other PMICs expose oscillator clocks through CCF. MAX77686 comes to mind. > +- regulator-name > + Usage: optional > + Value type: > + Definition: Specifies the name for this RPMh regulator. If not > + specified, then the regulator's name is equal to its subnode > + name. Probably don't need to include "regulator-name" since you say below "Other properties defined in regulator.txt may also be used" and this isn't anything special for your regulator. > +- regulator-min-microvolt > + Usage: required > + Value type: > + Definition: For VRM resources, this is the minimum supported voltage in > + microvolts. For XOB resources, this is the fixed output > + voltage. > + > +- regulator-max-microvolt > + Usage: required > + Value type: > + Definition: For VRM resources, this is the maximum supported voltage in > + microvolts. For XOB resources, this is the fixed output > + voltage. regulator-min-microvolt / regulator-max-microvolt are really required? What happens if you leave them off? In general the regulator framework supports this concept--it just lets you enable/disable without changing voltage. > +- qcom,regulator-initial-voltage > + Usage: optional; VRM regulators only > + Value type: > + Definition: Specifies the initial voltage in microvolts to request for a > + VRM regulator. Supported values are 0 to 8191000. The "supported values" here is a strange statement to make. Not all regulators will support all those voltages, right? Do you really need to list this here? > +- regulator-initial-mode > + Usage: optional; VRM regulators only > + Value type: > + Definition: Specifies the initial mode to request for a VRM regulator. > + Supported values are RPMH_REGULATOR_MODE_* which are defined > + in [1] (i.e. 0 to 4). Explicitly state whether this is allowed even if "regulator-allow-set-load" is not set. > +- regulator-allow-set-load > + Usage: optional > + Value type: > + Definition: Boolean flag indicating that the the mode of this regulator > + may be configured at runtime based upon consumer load needs. > + > +- qcom,allowed-modes It would be up to Mark Brown, but my guess is that he will say "please add this to the core". The regulator core already has the concept of modes and you're already using the standard core concepts in most places. Get rid of the special case code in your driver and add this to the core. > + Usage: required if regulator-allow-set-load is specified; > + VRM regulators only > + Value type: > + Definition: A list of integers specifying the PMIC regulator modes which > + can be configured at runtime based upon consumer load needs. > + Supported values are RPMH_REGULATOR_MODE_* which are defined > + in [1] (i.e. 0 to 4). Elements must be specified in order > + from lowest to highest value. As far as I can tell this sorting constraint should be removed. Really the constraints should be: * qcom,mode-threshold-currents should be sorted from lowest to highest * qcom,allowed-modes must match qcom,mode-threshold-currents Sure, today it happens that the numbers are always lower for lower currents, but it doesn't seem like something you'd have to encode into the dt. > +- qcom,headroom-voltage > + Usage: optional; VRM regulators only > + Value type: > + Definition: Specifies the headroom voltage in microvolts to request for > + a VRM regulator. RPMh hardware automatically ensures that > + the parent of this regulator outputs a voltage high enough > + to satisfy the requested headroom. Supported values are > + 0 to 511000. Is this just "regulator-microvolt-offset", but supported in hardware? > +- qcom,rpmh-resource-type > + Usage: optional > + Value type: > + Definition: RPMh accelerator type for this regulator. If not specified, > + then the default type associated with this regulator will be > + used. Supported values: "vrm" or "xob". I still don't have the big picture I guess, but it seems weird that you can override this. You're saying that someone might stick an oscillator buffer the place of something that's normally a regulator? ...or a regulator in the place of something that's usually an oscillator buffer? When would you use this? -Doug