Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1480461 > unrolled thread

[PATCH v2 2/2] i2c: aspeed: added documentation for Aspeed I2C driver

Started byBrendan Higgins <brendanhiggins@google.com>
First post2016-09-10 04:00 +0200
Last post2016-09-23 08:10 +0200
Articles 6 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH v2 2/2] i2c: aspeed: added documentation for Aspeed I2C driver Brendan Higgins <brendanhiggins@google.com> - 2016-09-10 04:00 +0200
    Re: [PATCH v2 2/2] i2c: aspeed: added documentation for Aspeed I2C  driver Rob Herring <robh@kernel.org> - 2016-09-19 23:40 +0200
      Re: [PATCH v2 2/2] i2c: aspeed: added documentation for Aspeed I2C driver Brendan Higgins <brendanhiggins@google.com> - 2016-09-20 01:30 +0200
        Re: [PATCH v2 2/2] i2c: aspeed: added documentation for Aspeed I2C driver Brendan Higgins <brendanhiggins@google.com> - 2016-09-20 20:10 +0200
          Re: [PATCH v2 2/2] i2c: aspeed: added documentation for Aspeed I2C driver Brendan Higgins <brendanhiggins@google.com> - 2016-09-21 01:10 +0200
        Re: [PATCH v2 2/2] i2c: aspeed: added documentation for Aspeed I2C driver Joel Stanley <joel@jms.id.au> - 2016-09-23 08:10 +0200

#1480461 — [PATCH v2 2/2] i2c: aspeed: added documentation for Aspeed I2C driver

FromBrendan Higgins <brendanhiggins@google.com>
Date2016-09-10 04:00 +0200
Subject[PATCH v2 2/2] i2c: aspeed: added documentation for Aspeed I2C driver
Message-ID<sfFKh-2aV-7@gated-at.bofh.it>
Added device tree binding documentation for Aspeed I2C controller and
busses.

Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
---
Changes for v2:
  - None
---
 .../devicetree/bindings/i2c/i2c-aspeed.txt         | 63 ++++++++++++++++++++++
 1 file changed, 63 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-aspeed.txt

diff --git a/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
new file mode 100644
index 0000000..df68f2a
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
@@ -0,0 +1,63 @@
+Device tree configuration for the I2C controller and busses on the AST24XX
+and AST25XX SoCs.
+
+Controller:
+
+	Required Properties:
+	- #address-cells	: should be 1
+	- #size-cells 		: should be 1
+	- #interrupt-cells 	: should be 1
+	- compatible 		: should be "aspeed,ast2400-i2c-controller"
+				  or "aspeed,ast2500-i2c-controller"
+	- reg			: address start and range of controller
+	- ranges		: defines address offset and range for busses
+	- interrupts		: interrupt number
+	- clocks		: root clock of bus, should reference the APB
+				  clock
+	- clock-ranges		: specifies that child busses can inherit clocks
+	- interrupt-controller	: denotes that the controller receives and fires
+				  new interrupts for child busses
+
+Bus:
+
+	Required Properties:
+	- #address-cells	: should be 1
+	- #size-cells		: should be 0
+	- reg			: address offset and range of bus
+	- compatible		: should be "aspeed,ast2400-i2c-bus"
+				  or "aspeed,ast2500-i2c-bus"
+	- bus			: the bus's number
+	- interrupts		: interrupt number
+
+	Optional Properties:
+	- clock-frequency	: frequency of the bus clock in Hz
+				  defaults to 100 kHz when not specified
+
+Example:
+
+i2c: i2c@1e78a000 {
+	#address-cells = <1>;
+	#size-cells = <1>;
+	#interrupt-cells = <1>;
+
+	compatible = "aspeed,ast2400-i2c-controller";
+	reg = <0x1e78a000 0x40>;
+	ranges = <0 0x1e78a000 0x1000>;
+	interrupts = <12>;
+	clocks = <&clk_apb>;
+	clock-ranges;
+	interrupt-controller;
+
+	i2c0: i2c-bus@40 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reg = <0x40 0x40>;
+		compatible = "aspeed,ast2400-i2c-bus";
+		bus = <0>;
+		clock-frequency = <100000>;
+		status = "disabled";
+		interrupts = <0>;
+		interrupt-parent = <&i2c>;
+	};
+};
+
-- 
2.8.0.rc3.226.g39d4020

[toc] | [next] | [standalone]


#1486890 — Re: [PATCH v2 2/2] i2c: aspeed: added documentation for Aspeed I2C driver

FromRob Herring <robh@kernel.org>
Date2016-09-19 23:40 +0200
SubjectRe: [PATCH v2 2/2] i2c: aspeed: added documentation for Aspeed I2C driver
Message-ID<sjesa-2tY-33@gated-at.bofh.it>
In reply to#1480461
On Fri, Sep 09, 2016 at 06:55:51PM -0700, Brendan Higgins wrote:
> Added device tree binding documentation for Aspeed I2C controller and
> busses.
> 
> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> ---
> Changes for v2:
>   - None
> ---
>  .../devicetree/bindings/i2c/i2c-aspeed.txt         | 63 ++++++++++++++++++++++
>  1 file changed, 63 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
> new file mode 100644
> index 0000000..df68f2a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
> @@ -0,0 +1,63 @@
> +Device tree configuration for the I2C controller and busses on the AST24XX
> +and AST25XX SoCs.
> +
> +Controller:
> +
> +	Required Properties:
> +	- #address-cells	: should be 1
> +	- #size-cells 		: should be 1
> +	- #interrupt-cells 	: should be 1
> +	- compatible 		: should be "aspeed,ast2400-i2c-controller"
> +				  or "aspeed,ast2500-i2c-controller"
> +	- reg			: address start and range of controller
> +	- ranges		: defines address offset and range for busses
> +	- interrupts		: interrupt number
> +	- clocks		: root clock of bus, should reference the APB
> +				  clock
> +	- clock-ranges		: specifies that child busses can inherit clocks
> +	- interrupt-controller	: denotes that the controller receives and fires
> +				  new interrupts for child busses
> +
> +Bus:
> +
> +	Required Properties:
> +	- #address-cells	: should be 1
> +	- #size-cells		: should be 0
> +	- reg			: address offset and range of bus
> +	- compatible		: should be "aspeed,ast2400-i2c-bus"
> +				  or "aspeed,ast2500-i2c-bus"
> +	- bus			: the bus's number

Don't use indexes. The reg property is enough to id which bus is which.

[toc] | [prev] | [next] | [standalone]


#1486948

FromBrendan Higgins <brendanhiggins@google.com>
Date2016-09-20 01:30 +0200
Message-ID<sjgaC-3Aa-25@gated-at.bofh.it>
In reply to#1486890
Addressed in v3.

Thanks!

On Mon, Sep 19, 2016 at 2:35 PM, Rob Herring <robh@kernel.org> wrote:
> On Fri, Sep 09, 2016 at 06:55:51PM -0700, Brendan Higgins wrote:
>> Added device tree binding documentation for Aspeed I2C controller and
>> busses.
>>
>> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
>> ---
>> Changes for v2:
>>   - None
>> ---
>>  .../devicetree/bindings/i2c/i2c-aspeed.txt         | 63 ++++++++++++++++++++++
>>  1 file changed, 63 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
>>
>> diff --git a/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
>> new file mode 100644
>> index 0000000..df68f2a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
>> @@ -0,0 +1,63 @@
>> +Device tree configuration for the I2C controller and busses on the AST24XX
>> +and AST25XX SoCs.
>> +
>> +Controller:
>> +
>> +     Required Properties:
>> +     - #address-cells        : should be 1
>> +     - #size-cells           : should be 1
>> +     - #interrupt-cells      : should be 1
>> +     - compatible            : should be "aspeed,ast2400-i2c-controller"
>> +                               or "aspeed,ast2500-i2c-controller"
>> +     - reg                   : address start and range of controller
>> +     - ranges                : defines address offset and range for busses
>> +     - interrupts            : interrupt number
>> +     - clocks                : root clock of bus, should reference the APB
>> +                               clock
>> +     - clock-ranges          : specifies that child busses can inherit clocks
>> +     - interrupt-controller  : denotes that the controller receives and fires
>> +                               new interrupts for child busses
>> +
>> +Bus:
>> +
>> +     Required Properties:
>> +     - #address-cells        : should be 1
>> +     - #size-cells           : should be 0
>> +     - reg                   : address offset and range of bus
>> +     - compatible            : should be "aspeed,ast2400-i2c-bus"
>> +                               or "aspeed,ast2500-i2c-bus"
>> +     - bus                   : the bus's number
>
> Don't use indexes. The reg property is enough to id which bus is which.

[toc] | [prev] | [next] | [standalone]


#1487558

FromBrendan Higgins <brendanhiggins@google.com>
Date2016-09-20 20:10 +0200
Message-ID<sjxEt-6rD-15@gated-at.bofh.it>
In reply to#1486948
(sorry if you get a duplicate, I forgot to send plain text)

First off, someone pointed out to me that the mapping that I used
between addresses and bus numbers is not actually valid for busses
8-14.

This could be fixed by checking the offset, but I am wondering if that
is the right way to do it. It seems like this is not completely
trivial so maybe this should be specified in the device tree? If that
is the case, should I do this as another reg entry or go back to the
way I was doing it before?

On Mon, Sep 19, 2016 at 4:26 PM, Brendan Higgins
<brendanhiggins@google.com> wrote:
> Addressed in v3.
>
> Thanks!
>
> On Mon, Sep 19, 2016 at 2:35 PM, Rob Herring <robh@kernel.org> wrote:
>> On Fri, Sep 09, 2016 at 06:55:51PM -0700, Brendan Higgins wrote:
>>> Added device tree binding documentation for Aspeed I2C controller and
>>> busses.
>>>
>>> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
>>> ---
>>> Changes for v2:
>>>   - None
>>> ---
>>>  .../devicetree/bindings/i2c/i2c-aspeed.txt         | 63 ++++++++++++++++++++++
>>>  1 file changed, 63 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
>>> new file mode 100644
>>> index 0000000..df68f2a
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
>>> @@ -0,0 +1,63 @@
>>> +Device tree configuration for the I2C controller and busses on the AST24XX
>>> +and AST25XX SoCs.
>>> +
>>> +Controller:
>>> +
>>> +     Required Properties:
>>> +     - #address-cells        : should be 1
>>> +     - #size-cells           : should be 1
>>> +     - #interrupt-cells      : should be 1
>>> +     - compatible            : should be "aspeed,ast2400-i2c-controller"
>>> +                               or "aspeed,ast2500-i2c-controller"
>>> +     - reg                   : address start and range of controller
>>> +     - ranges                : defines address offset and range for busses
>>> +     - interrupts            : interrupt number
>>> +     - clocks                : root clock of bus, should reference the APB
>>> +                               clock
>>> +     - clock-ranges          : specifies that child busses can inherit clocks
>>> +     - interrupt-controller  : denotes that the controller receives and fires
>>> +                               new interrupts for child busses
>>> +
>>> +Bus:
>>> +
>>> +     Required Properties:
>>> +     - #address-cells        : should be 1
>>> +     - #size-cells           : should be 0
>>> +     - reg                   : address offset and range of bus
>>> +     - compatible            : should be "aspeed,ast2400-i2c-bus"
>>> +                               or "aspeed,ast2500-i2c-bus"
>>> +     - bus                   : the bus's number
>>
>> Don't use indexes. The reg property is enough to id which bus is which.

[toc] | [prev] | [next] | [standalone]


#1487712

FromBrendan Higgins <brendanhiggins@google.com>
Date2016-09-21 01:10 +0200
Message-ID<sjCkN-13s-9@gated-at.bofh.it>
In reply to#1487558
To be clear, these bus numbers are defined as part of the hardware
specification. I am not sure if that was clear before.

On Tue, Sep 20, 2016 at 11:08 AM, Brendan Higgins
<brendanhiggins@google.com> wrote:
> (sorry if you get a duplicate, I forgot to send plain text)
>
> First off, someone pointed out to me that the mapping that I used
> between addresses and bus numbers is not actually valid for busses
> 8-14.
>
> This could be fixed by checking the offset, but I am wondering if that
> is the right way to do it. It seems like this is not completely
> trivial so maybe this should be specified in the device tree? If that
> is the case, should I do this as another reg entry or go back to the
> way I was doing it before?
>
> On Mon, Sep 19, 2016 at 4:26 PM, Brendan Higgins
> <brendanhiggins@google.com> wrote:
>> Addressed in v3.
>>
>> Thanks!
>>
>> On Mon, Sep 19, 2016 at 2:35 PM, Rob Herring <robh@kernel.org> wrote:
>>> On Fri, Sep 09, 2016 at 06:55:51PM -0700, Brendan Higgins wrote:
>>>> Added device tree binding documentation for Aspeed I2C controller and
>>>> busses.
>>>>
>>>> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
>>>> ---
>>>> Changes for v2:
>>>>   - None
>>>> ---
>>>>  .../devicetree/bindings/i2c/i2c-aspeed.txt         | 63 ++++++++++++++++++++++
>>>>  1 file changed, 63 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
>>>> new file mode 100644
>>>> index 0000000..df68f2a
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
>>>> @@ -0,0 +1,63 @@
>>>> +Device tree configuration for the I2C controller and busses on the AST24XX
>>>> +and AST25XX SoCs.
>>>> +
>>>> +Controller:
>>>> +
>>>> +     Required Properties:
>>>> +     - #address-cells        : should be 1
>>>> +     - #size-cells           : should be 1
>>>> +     - #interrupt-cells      : should be 1
>>>> +     - compatible            : should be "aspeed,ast2400-i2c-controller"
>>>> +                               or "aspeed,ast2500-i2c-controller"
>>>> +     - reg                   : address start and range of controller
>>>> +     - ranges                : defines address offset and range for busses
>>>> +     - interrupts            : interrupt number
>>>> +     - clocks                : root clock of bus, should reference the APB
>>>> +                               clock
>>>> +     - clock-ranges          : specifies that child busses can inherit clocks
>>>> +     - interrupt-controller  : denotes that the controller receives and fires
>>>> +                               new interrupts for child busses
>>>> +
>>>> +Bus:
>>>> +
>>>> +     Required Properties:
>>>> +     - #address-cells        : should be 1
>>>> +     - #size-cells           : should be 0
>>>> +     - reg                   : address offset and range of bus
>>>> +     - compatible            : should be "aspeed,ast2400-i2c-bus"
>>>> +                               or "aspeed,ast2500-i2c-bus"
>>>> +     - bus                   : the bus's number
>>>
>>> Don't use indexes. The reg property is enough to id which bus is which.

[toc] | [prev] | [next] | [standalone]


#1489784

FromJoel Stanley <joel@jms.id.au>
Date2016-09-23 08:10 +0200
Message-ID<skrQl-8qj-1@gated-at.bofh.it>
In reply to#1486948
Hi Brendan,

On Wed, Sep 21, 2016 at 3:35 AM, Brendan Higgins
<brendanhiggins@google.com> wrote:
> First off, someone pointed out to me that the mapping that I used between
> addresses and bus numbers is not actually valid for busses 8-14.
>
> This could be fixed by checking the offset, but I am wondering if that is
> the right way to do it. It seems like this is not completely trivial so
> maybe this should be specified in the device tree? If that is the case,
> should I do this as another reg entry or go back to the way I was doing it
> before?

I don't see an alternative way to derive these numbers. As you
mention, the block of SRAM in the middle of the IP screwing up the
linear mapping.

I suggest going back to what you have in v2, with a mention in the
commit message as to why the bus property is necessary.

Cheers,

Joel

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web