diff mbox series

[v5,6/8] dt-bindings: i2c: iproc: add "brcm, iproc-nic-i2c" compatible string

Message ID 20190214175725.60462-7-ray.jui@broadcom.com
State Not Applicable, archived
Headers show
Series iProc I2C slave mode and NIC mode | expand

Checks

Context Check Description
robh/checkpatch success

Commit Message

Ray Jui Feb. 14, 2019, 5:57 p.m. UTC
From: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>

Update iProc I2C binding document to add new compatible string
"brcm,iproc-nic-i2c". Optional property "brcm,ape-hsls-addr-mask" is
also added that allows configuration of the host view into the APE's
address for "brcm,iproc-nic-i2c"

Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
Signed-off-by: Ray Jui <ray.jui@broadcom.com>
---
 Documentation/devicetree/bindings/i2c/brcm,iproc-i2c.txt | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Rob Herring Feb. 14, 2019, 10:26 p.m. UTC | #1
On Thu, Feb 14, 2019 at 11:58 AM Ray Jui <ray.jui@broadcom.com> wrote:
>
> From: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
>
> Update iProc I2C binding document to add new compatible string
> "brcm,iproc-nic-i2c". Optional property "brcm,ape-hsls-addr-mask" is
> also added that allows configuration of the host view into the APE's
> address for "brcm,iproc-nic-i2c"
>
> Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
> ---
>  Documentation/devicetree/bindings/i2c/brcm,iproc-i2c.txt | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)

Reviewed-by: Rob Herring <robh@kernel.org>
Wolfram Sang March 27, 2019, 10:24 p.m. UTC | #2
> Update iProc I2C binding document to add new compatible string
> "brcm,iproc-nic-i2c". Optional property "brcm,ape-hsls-addr-mask" is
> also added that allows configuration of the host view into the APE's
> address for "brcm,iproc-nic-i2c"

I don't know the platform, but wouldn't it be more DT-like to describe
the APE in DT and derive the mask from that information? Custom bindings
with values which are directly poked into a register usually raise my
eyebrow.
Ray Jui April 1, 2019, 9:43 p.m. UTC | #3
Hi Wolfram,

On 3/27/2019 3:24 PM, Wolfram Sang wrote:
> 
>> Update iProc I2C binding document to add new compatible string
>> "brcm,iproc-nic-i2c". Optional property "brcm,ape-hsls-addr-mask" is
>> also added that allows configuration of the host view into the APE's
>> address for "brcm,iproc-nic-i2c"
> 
> I don't know the platform, but wouldn't it be more DT-like to describe
> the APE in DT and derive the mask from that information? Custom bindings
> with values which are directly poked into a register usually raise my
> eyebrow.
> 

Note APE is a co-processor that is not visible from the Linux running
from the host processor.

"brcm,iproc-nic-i2c" here is introduced to allow the I2C port from APE
to be completely owned by the host CPU, to meet the requirement of
certain use cases. At the same time, the control of the I2C port from
APE will be disabled.

The "brcm,ape-hsls-addr-mask" defines the address translation and be
programmed into some configuration registers to allow the host to
directly access the I2C registers of APE. Note those configuration
registers are owned by the host, and that address is not APE's address
space nor the host is intending to take over the control of APE.

Therefore, I think it makes way more sense to use an address mask type
of DT property here.

Thanks,

Ray
Wolfram Sang April 2, 2019, 10:17 a.m. UTC | #4
> Note APE is a co-processor that is not visible from the Linux running
> from the host processor.
> 
> "brcm,iproc-nic-i2c" here is introduced to allow the I2C port from APE
> to be completely owned by the host CPU, to meet the requirement of
> certain use cases. At the same time, the control of the I2C port from
> APE will be disabled.
> 
> The "brcm,ape-hsls-addr-mask" defines the address translation and be
> programmed into some configuration registers to allow the host to
> directly access the I2C registers of APE. Note those configuration
> registers are owned by the host, and that address is not APE's address
> space nor the host is intending to take over the control of APE.
> 
> Therefore, I think it makes way more sense to use an address mask type
> of DT property here.

Thanks for the explanation! Well, since Rob is already OK with it, then
so am I.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/i2c/brcm,iproc-i2c.txt b/Documentation/devicetree/bindings/i2c/brcm,iproc-i2c.txt
index 7a32bf81bfa9..d12cc33cca6c 100644
--- a/Documentation/devicetree/bindings/i2c/brcm,iproc-i2c.txt
+++ b/Documentation/devicetree/bindings/i2c/brcm,iproc-i2c.txt
@@ -3,7 +3,7 @@  Broadcom iProc I2C controller
 Required properties:
 
 - compatible:
-    Must be "brcm,iproc-i2c"
+    Must be "brcm,iproc-i2c" or "brcm,iproc-nic-i2c"
 
 - reg:
     Define the base and range of the I/O address space that contain the iProc
@@ -26,6 +26,10 @@  Optional properties:
     case, this property should be left unspecified, and driver will fall back
     to polling mode
 
+- brcm,ape-hsls-addr-mask:
+    Required for "brcm,iproc-nic-i2c". Host view of address mask into the
+    'APE' co-processor. Value must be unsigned, 32-bit
+
 Example:
 	i2c0: i2c@18008000 {
 		compatible = "brcm,iproc-i2c";