diff mbox

[v2,1/2] dt-bindings: Add Tegra186 BPMP I2C binding

Message ID 20170127083939.20393-2-thierry.reding@gmail.com
State Superseded
Headers show

Commit Message

Thierry Reding Jan. 27, 2017, 8:39 a.m. UTC
From: Stephen Warren <swarren@nvidia.com>

In Tegra186, the BPMP (Boot and Power Management Processor) owns certain
HW devices, such as the I2C controller for the power management I2C bus.
Software running on other CPUs must perform IPC to the BPMP in order to
execute transactions on that I2C bus. This binding describes an I2C bus
that is accessed in such a fashion.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Tom Warren <twarren@nvidia.com>
Acked-by: Jon Hunter <jonathanh@nvidia.com>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 .../bindings/i2c/nvidia,tegra186-bpmp-i2c.txt      | 42 ++++++++++++++++++++++
 1 file changed, 42 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt

Comments

Wolfram Sang Jan. 27, 2017, 4:09 p.m. UTC | #1
> +- nvidia,bpmp-bus-id:
> +    Single-cell integer.
> +    Indicates the I2C bus number this DT node represent, as defined by the
> +    BPMP firmware.

I'd like to get an ack from Rob for this one if vendor-specific bindings
are still the way to go to encode firmware data. I simply don't know.

> +
> +Example:
> +
> +bpmp {
> +	...
> +
> +	i2c {
> +		compatible = "nvidia,tegra186-bpmp-i2c";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		nvidia,bpmp-bus-id = <5>;
> +	};
> +};
Thierry Reding Jan. 27, 2017, 10:19 p.m. UTC | #2
On Fri, Jan 27, 2017 at 09:39:38AM +0100, Thierry Reding wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> In Tegra186, the BPMP (Boot and Power Management Processor) owns certain
> HW devices, such as the I2C controller for the power management I2C bus.
> Software running on other CPUs must perform IPC to the BPMP in order to
> execute transactions on that I2C bus. This binding describes an I2C bus
> that is accessed in such a fashion.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Tom Warren <twarren@nvidia.com>
> Acked-by: Jon Hunter <jonathanh@nvidia.com>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  .../bindings/i2c/nvidia,tegra186-bpmp-i2c.txt      | 42 ++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt

Rob, sorry for not Cc'ing you earlier on this, but I had thought this
binding had already been reviewed and acked since it is merged in
U-Boot.

Wolfram was concerned in particular about the nvidia,bpmp-bus-id
property below.

Can you give this a quick look, please?

Thanks,
Thierry

> diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt b/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt
> new file mode 100644
> index 000000000000..ab240e10debc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt
> @@ -0,0 +1,42 @@
> +NVIDIA Tegra186 BPMP I2C controller
> +
> +In Tegra186, the BPMP (Boot and Power Management Processor) owns certain HW
> +devices, such as the I2C controller for the power management I2C bus. Software
> +running on other CPUs must perform IPC to the BPMP in order to execute
> +transactions on that I2C bus. This binding describes an I2C bus that is
> +accessed in such a fashion.
> +
> +The BPMP I2C node must be located directly inside the main BPMP node. See
> +../firmware/nvidia,tegra186-bpmp.txt for details of the BPMP binding.
> +
> +This node represents an I2C controller. See ../i2c/i2c.txt for details of the
> +core I2C binding.
> +
> +Required properties:
> +- compatible:
> +    Array of strings.
> +    One of:
> +    - "nvidia,tegra186-bpmp-i2c".
> +- #address-cells: Address cells for I2C device address.
> +    Single-cell integer.
> +    Must be <1>.
> +- #size-cells:
> +    Single-cell integer.
> +    Must be <0>.
> +- nvidia,bpmp-bus-id:
> +    Single-cell integer.
> +    Indicates the I2C bus number this DT node represent, as defined by the
> +    BPMP firmware.
> +
> +Example:
> +
> +bpmp {
> +	...
> +
> +	i2c {
> +		compatible = "nvidia,tegra186-bpmp-i2c";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		nvidia,bpmp-bus-id = <5>;
> +	};
> +};
> -- 
> 2.11.0
>
Rob Herring Jan. 27, 2017, 10:52 p.m. UTC | #3
On Fri, Jan 27, 2017 at 4:19 PM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> On Fri, Jan 27, 2017 at 09:39:38AM +0100, Thierry Reding wrote:
>> From: Stephen Warren <swarren@nvidia.com>
>>
>> In Tegra186, the BPMP (Boot and Power Management Processor) owns certain
>> HW devices, such as the I2C controller for the power management I2C bus.
>> Software running on other CPUs must perform IPC to the BPMP in order to
>> execute transactions on that I2C bus. This binding describes an I2C bus
>> that is accessed in such a fashion.
>>
>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>> Signed-off-by: Tom Warren <twarren@nvidia.com>
>> Acked-by: Jon Hunter <jonathanh@nvidia.com>
>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>> ---
>>  .../bindings/i2c/nvidia,tegra186-bpmp-i2c.txt      | 42 ++++++++++++++++++++++
>>  1 file changed, 42 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt
>
> Rob, sorry for not Cc'ing you earlier on this, but I had thought this
> binding had already been reviewed and acked since it is merged in
> U-Boot.

Ha! Good one. :(

>
> Wolfram was concerned in particular about the nvidia,bpmp-bus-id
> property below.
>
> Can you give this a quick look, please?
>
> Thanks,
> Thierry
>
>> diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt b/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt
>> new file mode 100644
>> index 000000000000..ab240e10debc
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt
>> @@ -0,0 +1,42 @@
>> +NVIDIA Tegra186 BPMP I2C controller
>> +
>> +In Tegra186, the BPMP (Boot and Power Management Processor) owns certain HW
>> +devices, such as the I2C controller for the power management I2C bus. Software
>> +running on other CPUs must perform IPC to the BPMP in order to execute
>> +transactions on that I2C bus. This binding describes an I2C bus that is
>> +accessed in such a fashion.
>> +
>> +The BPMP I2C node must be located directly inside the main BPMP node. See
>> +../firmware/nvidia,tegra186-bpmp.txt for details of the BPMP binding.
>> +
>> +This node represents an I2C controller. See ../i2c/i2c.txt for details of the
>> +core I2C binding.
>> +
>> +Required properties:
>> +- compatible:
>> +    Array of strings.
>> +    One of:
>> +    - "nvidia,tegra186-bpmp-i2c".
>> +- #address-cells: Address cells for I2C device address.
>> +    Single-cell integer.
>> +    Must be <1>.
>> +- #size-cells:
>> +    Single-cell integer.
>> +    Must be <0>.
>> +- nvidia,bpmp-bus-id:
>> +    Single-cell integer.
>> +    Indicates the I2C bus number this DT node represent, as defined by the
>> +    BPMP firmware.

This seems okay to me given it's a pretty specific use and dictated by
the firmware. If there were multiple, then perhaps we should use reg,
but that may collide with other BPMP child nodes.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Warren Jan. 28, 2017, 5:42 a.m. UTC | #4
On 01/27/2017 03:19 PM, Thierry Reding wrote:
> On Fri, Jan 27, 2017 at 09:39:38AM +0100, Thierry Reding wrote:
>> From: Stephen Warren <swarren@nvidia.com>
>>
>> In Tegra186, the BPMP (Boot and Power Management Processor) owns certain
>> HW devices, such as the I2C controller for the power management I2C bus.
>> Software running on other CPUs must perform IPC to the BPMP in order to
>> execute transactions on that I2C bus. This binding describes an I2C bus
>> that is accessed in such a fashion.
>>
>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>> Signed-off-by: Tom Warren <twarren@nvidia.com>
>> Acked-by: Jon Hunter <jonathanh@nvidia.com>
>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>> ---
>>  .../bindings/i2c/nvidia,tegra186-bpmp-i2c.txt      | 42 ++++++++++++++++++++++
>>  1 file changed, 42 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt
>
> Rob, sorry for not Cc'ing you earlier on this, but I had thought this
> binding had already been reviewed and acked since it is merged in
> U-Boot.

I'm pretty sure all the DT binding patches for Tegra186 that I submitted 
were ack'd by Rob. They were certainly all posted to relevant Linux 
lists and discussed and ack'd by someone before I pushed them into 
U-Boot. Probably 2nd quarter last year, or maybe 3rd.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Thierry Reding Jan. 30, 2017, 6:47 a.m. UTC | #5
On Fri, Jan 27, 2017 at 04:52:37PM -0600, Rob Herring wrote:
> On Fri, Jan 27, 2017 at 4:19 PM, Thierry Reding
> <thierry.reding@gmail.com> wrote:
> > On Fri, Jan 27, 2017 at 09:39:38AM +0100, Thierry Reding wrote:
> >> From: Stephen Warren <swarren@nvidia.com>
> >>
> >> In Tegra186, the BPMP (Boot and Power Management Processor) owns certain
> >> HW devices, such as the I2C controller for the power management I2C bus.
> >> Software running on other CPUs must perform IPC to the BPMP in order to
> >> execute transactions on that I2C bus. This binding describes an I2C bus
> >> that is accessed in such a fashion.
> >>
> >> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> >> Reviewed-by: Simon Glass <sjg@chromium.org>
> >> Signed-off-by: Tom Warren <twarren@nvidia.com>
> >> Acked-by: Jon Hunter <jonathanh@nvidia.com>
> >> Signed-off-by: Thierry Reding <treding@nvidia.com>
> >> ---
> >>  .../bindings/i2c/nvidia,tegra186-bpmp-i2c.txt      | 42 ++++++++++++++++++++++
> >>  1 file changed, 42 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt
> >
> > Rob, sorry for not Cc'ing you earlier on this, but I had thought this
> > binding had already been reviewed and acked since it is merged in
> > U-Boot.
> 
> Ha! Good one. :(

So I went looking for earlier discussion on this patch because I
distinctly remembered you being part of it, and found this:

	https://patchwork.ozlabs.org/patch/650385/

You already gave an Acked-by, but I failed to pick it up for some
reason. Given what you said below I'm going to assume that it still
applies.

Wolfram, do you want me to resend with Rob's Acked-by from back in
July or do you want to add it yourself when applying?

Thanks,
Thierry

> > Wolfram was concerned in particular about the nvidia,bpmp-bus-id
> > property below.
> >
> > Can you give this a quick look, please?
> >
> > Thanks,
> > Thierry
> >
> >> diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt b/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt
> >> new file mode 100644
> >> index 000000000000..ab240e10debc
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt
> >> @@ -0,0 +1,42 @@
> >> +NVIDIA Tegra186 BPMP I2C controller
> >> +
> >> +In Tegra186, the BPMP (Boot and Power Management Processor) owns certain HW
> >> +devices, such as the I2C controller for the power management I2C bus. Software
> >> +running on other CPUs must perform IPC to the BPMP in order to execute
> >> +transactions on that I2C bus. This binding describes an I2C bus that is
> >> +accessed in such a fashion.
> >> +
> >> +The BPMP I2C node must be located directly inside the main BPMP node. See
> >> +../firmware/nvidia,tegra186-bpmp.txt for details of the BPMP binding.
> >> +
> >> +This node represents an I2C controller. See ../i2c/i2c.txt for details of the
> >> +core I2C binding.
> >> +
> >> +Required properties:
> >> +- compatible:
> >> +    Array of strings.
> >> +    One of:
> >> +    - "nvidia,tegra186-bpmp-i2c".
> >> +- #address-cells: Address cells for I2C device address.
> >> +    Single-cell integer.
> >> +    Must be <1>.
> >> +- #size-cells:
> >> +    Single-cell integer.
> >> +    Must be <0>.
> >> +- nvidia,bpmp-bus-id:
> >> +    Single-cell integer.
> >> +    Indicates the I2C bus number this DT node represent, as defined by the
> >> +    BPMP firmware.
> 
> This seems okay to me given it's a pretty specific use and dictated by
> the firmware. If there were multiple, then perhaps we should use reg,
> but that may collide with other BPMP child nodes.
> 
> Rob
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Thierry Reding Jan. 30, 2017, 6:50 a.m. UTC | #6
On Fri, Jan 27, 2017 at 10:42:27PM -0700, Stephen Warren wrote:
> On 01/27/2017 03:19 PM, Thierry Reding wrote:
> > On Fri, Jan 27, 2017 at 09:39:38AM +0100, Thierry Reding wrote:
> > > From: Stephen Warren <swarren@nvidia.com>
> > > 
> > > In Tegra186, the BPMP (Boot and Power Management Processor) owns certain
> > > HW devices, such as the I2C controller for the power management I2C bus.
> > > Software running on other CPUs must perform IPC to the BPMP in order to
> > > execute transactions on that I2C bus. This binding describes an I2C bus
> > > that is accessed in such a fashion.
> > > 
> > > Signed-off-by: Stephen Warren <swarren@nvidia.com>
> > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > > Signed-off-by: Tom Warren <twarren@nvidia.com>
> > > Acked-by: Jon Hunter <jonathanh@nvidia.com>
> > > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > > ---
> > >  .../bindings/i2c/nvidia,tegra186-bpmp-i2c.txt      | 42 ++++++++++++++++++++++
> > >  1 file changed, 42 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt
> > 
> > Rob, sorry for not Cc'ing you earlier on this, but I had thought this
> > binding had already been reviewed and acked since it is merged in
> > U-Boot.
> 
> I'm pretty sure all the DT binding patches for Tegra186 that I submitted
> were ack'd by Rob. They were certainly all posted to relevant Linux lists
> and discussed and ack'd by someone before I pushed them into U-Boot.
> Probably 2nd quarter last year, or maybe 3rd.

Yes, I thought I had seen an Acked-by myself, and I know that you're
very diligent about this. And true enough, it's in patchwork. I probably
missed it because I was cherry-picking it directly from U-Boot where the
Acked-by wasn't present.

Thierry
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt b/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt
new file mode 100644
index 000000000000..ab240e10debc
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt
@@ -0,0 +1,42 @@ 
+NVIDIA Tegra186 BPMP I2C controller
+
+In Tegra186, the BPMP (Boot and Power Management Processor) owns certain HW
+devices, such as the I2C controller for the power management I2C bus. Software
+running on other CPUs must perform IPC to the BPMP in order to execute
+transactions on that I2C bus. This binding describes an I2C bus that is
+accessed in such a fashion.
+
+The BPMP I2C node must be located directly inside the main BPMP node. See
+../firmware/nvidia,tegra186-bpmp.txt for details of the BPMP binding.
+
+This node represents an I2C controller. See ../i2c/i2c.txt for details of the
+core I2C binding.
+
+Required properties:
+- compatible:
+    Array of strings.
+    One of:
+    - "nvidia,tegra186-bpmp-i2c".
+- #address-cells: Address cells for I2C device address.
+    Single-cell integer.
+    Must be <1>.
+- #size-cells:
+    Single-cell integer.
+    Must be <0>.
+- nvidia,bpmp-bus-id:
+    Single-cell integer.
+    Indicates the I2C bus number this DT node represent, as defined by the
+    BPMP firmware.
+
+Example:
+
+bpmp {
+	...
+
+	i2c {
+		compatible = "nvidia,tegra186-bpmp-i2c";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		nvidia,bpmp-bus-id = <5>;
+	};
+};