[PATCH 0/4] Qualcomm MSM8660/APQ8060 EBI2 and ethernet support

[PATCH 0/4] Qualcomm MSM8660/APQ8060 EBI2 and ethernet support

[Linus Walleij]

This is basically a resend of the earlier patches, restricted to
the ARM SoC parts. (drivers/soc usually merge through ARM SoC).

Mark, how do you want to proceed? Are you queueing all of these
for v4.9?

Linus Walleij (4):
  soc: qcom: add an EBI2 device tree bindings
  soc: qcom: add EBI2 driver
  ARM: dts: add EBI2 to the Qualcomm MSM8660 DTSI
  ARM: dts: add SMSC ethernet on the APQ8060 Dragonboard

 .../devicetree/bindings/soc/qcom/qcom,ebi2.txt     | 134 ++++++++
 arch/arm/boot/dts/qcom-apq8060-dragonboard.dts     | 130 ++++++++
 arch/arm/boot/dts/qcom-msm8660.dtsi                |  12 +
 drivers/soc/qcom/Kconfig                           |   8 +
 drivers/soc/qcom/Makefile                          |   1 +
 drivers/soc/qcom/ebi2.c                            | 371 +++++++++++++++++++++
 6 files changed, 656 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,ebi2.txt
 create mode 100644 drivers/soc/qcom/ebi2.c

-- 
2.7.4

[PATCH 1/4] soc: qcom: add an EBI2 device tree bindings

[Linus Walleij]

This adds device tree bindings for the External Bus Interface 2, EBI2
to the Qualcomm SoC drivers.

Cc: devicetree at vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 .../devicetree/bindings/soc/qcom/qcom,ebi2.txt     | 134 +++++++++++++++++++++
 1 file changed, 134 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,ebi2.txt

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,ebi2.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,ebi2.txt
new file mode 100644
index 000000000000..b848a60352ca
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,ebi2.txt
@@ -0,0 +1,134 @@
+Qualcomm External Bus Interface 2 (EBI2)
+
+The EBI2 contains two peripheral blocks: XMEM and LCDC. The XMEM handles any
+external memory (such as NAND or other memory-mapped peripherals) whereas
+LCDC handles LCD displays.
+
+As it says it connects devices to an external bus interface, meaning address
+lines (up to 9 address lines so can only address 1KiB external memory space),
+data lines (16 bits), OE (output enable), ADV (address valid, used on some
+NOR flash memories), WE (write enable). This on top of 6 different chip selects
+(CS0 thru CS5) so that in theory 6 different devices can be connected.
+
+Apparently this bus is clocked at 64MHz. It has dedicated pins on the package
+and the bus can only come out on these pins, however if some of the pins are
+unused they can be left unconnected or remuxed to be used as GPIO or in some
+cases other orthogonal functions as well.
+
+Also CS1 and CS2 has -A and -B signals. Why they have that is unclear to me.
+
+The chip selects have the following memory range assignments. This region of
+memory is referred to as "Chip Peripheral SS FPB0" and is 168MB big.
+
+Chip Select                     Physical address base
+CS0 GPIO134                     0x1a800000-0x1b000000 (8MB)
+CS1 GPIO39 (A) / GPIO123 (B)    0x1b000000-0x1b800000 (8MB)
+CS2 GPIO40 (A) / GPIO124 (B)    0x1b800000-0x1c000000 (8MB)
+CS3 GPIO133                     0x1d000000-0x25000000 (128 MB)
+CS4 GPIO132                     0x1c800000-0x1d000000 (8MB)
+CS5 GPIO131                     0x1c000000-0x1c800000 (8MB)
+
+The APQ8060 Qualcomm Application Processor User Guide, 80-N7150-14 Rev. A,
+August 6, 2012 contains some incomplete documentation of the EBI2.
+
+FIXME: the manual mentions "write precharge cycles" and "precharge cycles".
+We have not been able to figure out which bit fields these correspond to
+in the hardware, or what valid values exist. The current hypothesis is that
+this is something just used on the FAST chip selects and that the SLOW
+chip selects are understood fully. There is also a "byte device enable"
+flag somewhere for 8bit memories.
+
+FIXME: The chipselects have SLOW and FAST configuration registers. It's a bit
+unclear what this means, if they are mutually exclusive or can be used
+together, or if some chip selects are hardwired to be FAST and others are SLOW
+by design.
+
+The XMEM registers are totally undocumented but could be partially decoded
+because the Cypress AN49576 Antioch Westbridge apparently has suspiciously
+similar register layout, see: http://www.cypress.com/file/105771/download
+
+Required properties:
+- compatible: should be "qcom,ebi2"
+- #address-cells: shoule be <1>
+- #size-cells: should be <1>
+- ranges: boolean should be present
+- reg: two ranges of registers: EBI2 config and XMEM config areas
+- reg-names: should be "ebi2", "xmem"
+- clocks: two clocks, EBI_2X and EBI
+- clock-names: shoule be "ebi2x", "ebi2"
+
+Optional subnodes:
+- Nodes inside the EBI2 will be considered chipselect nodes. For each
+  chipselect under the EBI2 node you should have the following required
+  properties:
+
+Required subnode properties:
+- chipselect: <N> where N is the chipselect configured in this node
+- address-cells: should be <1>
+- size-cells: should be <1>
+- ranges: bool should be present
+
+Optional subnode properties for SLOW chip selects:
+- xmem-recovery-cycles: recovery cycles is the time the memory continues to
+  drive the data bus after OE is de-asserted, in order to avoid contention on
+  the data bus. They are inserted when reading one CS and switching to another
+  CS or read followed by write on the same CS. Valid values 0 thru 15. Minimum
+  value is actually 1, so a value of 0 will still yield 1 recovery cycle.
+- xmem-write-hold-cycles: write hold cycles, these are extra cycles inserted
+  after every write minimum 1. The data out is driven from the time WE is
+  asserted until CS is asserted. With a hold of 1 (value = 0), the CS stays
+  active for 1 extra cycle etc. Valid values 0 thru 15.
+- xmem-write-delta-cycles: initial latency for write cycles inserted for the
+  first write to a page or burst memory. Valid values 0 thru 255.
+- xmem-read-delta-cycles: initial latency for read cycles inserted for the
+  first read to a page or burst memory. Valid values 0 thru 255.
+- xmem-write-wait-cycles: number of wait cycles for every write access, 0=1
+  cycle. Valid values 0 thru 15.
+- xmem-read-wait-cycles: number of wait cycles for every read access, 0=1
+  cycle. Valid values 0 thru 15.
+
+Optional subnode properties for FAST chip selects:
+- xmem-address-hold-enable: this is a boolean property stating that we shall
+  holde the address for an extra cycle to meet hold time requirements with ADV
+  assertion.
+- xmem-adv-to-oe-recovery-cycles: the number of cycles elapsed before an OE
+  assertion, with respect to the cycle where ADV (address valid) is asserted.
+  2 means 2 cycles between ADV and OE. Valid values 0, 1, 2 or 3.
+- xmem-read-hold-cycles: the length in cycles of the first segment of a read
+  transfer. For a single read trandfer this will be the time from CS assertion
+  to OE assertion. Valid values 0 thru 15.
+
+Example:
+
+ebi2 at 1a100000 {
+	compatible = "qcom,ebi2";
+	#address-cells = <1>;
+	#size-cells = <1>;
+	ranges;
+	reg = <0x1a100000 0x1000>, <0x1a110000 0x1000>;
+	reg-names = "ebi2", "xmem";
+	clocks = <&gcc EBI2_2X_CLK>, <&gcc EBI2_CLK>;
+	clock-names = "ebi2x", "ebi2";
+	/* Make sure to set up the pin control for the EBI2 */
+	pinctrl-names = "default";
+	pinctrl-0 = <&foo_ebi2_pins>;
+
+	cs2 at 1b800000 {
+		chipselect = <2>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+		xmem-recovery-cycles = <0>;
+		xmem-write-hold-cycles = <3>;
+		xmem-write-delta-cycles = <31>;
+		xmem-read-delta-cycles = <28>;
+		xmem-write-wait-cycles = <9>;
+		xmem-read-wait-cycles = <9>;
+
+		foo-ebi2 at 1b800000 {
+			compatible = "foo";
+			reg = <0x1b800000 0x100>;
+			(...)
+		};
+	};
+};
-- 
2.7.4

[PATCH 1/4] soc: qcom: add an EBI2 device tree bindings

[Arnd Bergmann]

On Monday, August 8, 2016 11:24:02 PM CEST Linus Walleij wrote:
> +
> +Required subnode properties:
> +- chipselect: <N> where N is the chipselect configured in this node
> +- address-cells: should be <1>
> +- size-cells: should be <1>
> +- ranges: bool should be present
> 

I think this should use a proper ranges property that translates
the address space of the external bus into the parent bus.

Just use #address-cells=<2> and make the first cell the cs number,
as the other similar drivers do, and get rid of the chipselect
property.

	Arnd

[PATCH 1/4] soc: qcom: add an EBI2 device tree bindings

[Linus Walleij]

On Mon, Aug 8, 2016 at 11:32 PM, Arnd Bergmann <arnd@arndb.de> wrote:

> On Monday, August 8, 2016 11:24:02 PM CEST Linus Walleij wrote:
>> +
>> +Required subnode properties:
>> +- chipselect: <N> where N is the chipselect configured in this node
>> +- address-cells: should be <1>
>> +- size-cells: should be <1>
>> +- ranges: bool should be present
>>
>
> I think this should use a proper ranges property that translates
> the address space of the external bus into the parent bus.
>
> Just use #address-cells=<2> and make the first cell the cs number,
> as the other similar drivers do, and get rid of the chipselect
> property.

Rob Herring also wrote:

>> +Required subnode properties:
>> +- chipselect: <N> where N is the chipselect configured in this node
>
> Follow the other external buses that put the CS# in the reg property.

Also the same comment from Mark Rutland on IRC.

The following discussion is only about the CS-in-reg property problem.
The ranges issue is not fixed but that is an orthogonal thing I'm working
on.

There is a problem. Look at this cut-down example where I just have
the "qcom,xmem-recovery-cycles" set up for the CS2 chip select:

ebi2 at 1a100000 {
       compatible = "qcom,msm8660-ebi2";
       #address-cells = <1>;
       #size-cells = <1>;
       (...)
       cs2 at 1b800000 {
               chipselect = <2>;
               #address-cells = <1>;
               #size-cells = <1>;
               qcom,xmem-recovery-cycles = <5>;
               foo-ebi2 at 1b800000 {
                       compatible = "foo";
                       reg = <0x1b800000 0x100>;
                       (...)
               };
               bar-ebi2 at 1b800000 {
                       compatible = "bar";
                       reg = <0x1ba00000 0x100>;
                       (...)
               };
       };
};

So two devices "foo" and "bar" on the EBI2 bus, in the old
scheme.

So I need to encode the CS in the first cell of the reg for
foo-ebi2, and get rid of chipselect = <2> fine:

ebi2 at 1a100000 {
       compatible = "qcom,msm8660-ebi2";
       #address-cells = <1>;
       #size-cells = <1>;
       (...)
       cs2 at 1b800000 {
               #address-cells = <2>;
               #size-cells = <1>;
               qcom,xmem-recovery-cycles = <5>;
               foo-ebi2 at 1b800000 {
                       compatible = "foo";
                       reg = <2 0x1b800000 0x100>;
                       (...)
               };
               bar-ebi2 at 1ba00000 {
                       compatible = "bar";
                       reg = <2 0x1ba00000 0x100>;
                       (...)
               };
       };
};

This is not looking good at all. First: the configuration settings for
the chipselect (i.e. all devices below it, both "foo" and "bar" are
just floating in space. When parsing the tree how should you know
what chipselect to set up the stuff on? Shall I check the child nodes
first value in the reg property and then make a majority vote on what
chipselect they apply to or what? That doesn't make sense.

So I assume doing away with the chipselect node altogether would
be prefered. But then: where do I put stuff like "qcom,xmem-recovery-cycles"
that apply to the whole chipselect, not just a single subdevice on that
chipselect? I certainly cannot encode it in the reg since it needs
to be the same for all devices and it's not about addressing.

The only thing I can reasonably come up with would be this:

ebi2 at 1a100000 {
       compatible = "qcom,msm8660-ebi2";
       #address-cells = <2>;
       #size-cells = <1>;
       qcom,xmem-recovery-cycles = <0>, <0>, <5>, <0>, <0>, <0>;

       (...)
       foo-ebi2 at 1b800000 {
              compatible = "foo";
               reg = <2 0x1b800000 0x100>;
               (...)
       };
       bar-ebi2 at 1ba00000 {
               compatible = "bar";
               reg = <2 0x1ba00000 0x100>;
               (...)
       };
};

So the chip select settings are shoehorned into an array in the top
node that is then indexed to find the settings for cs0, cs1 etc.

There will be 6 such arrays for the different per-cs settings.

Is this what you want? I kind of thought the hierarchical model
was nice since each device was below its chipselect node, but
I understand that it breaks the pattern of other similar bus drivers.

So are you fine with the array solution?

Yours,
Linus Walleij

[PATCH 1/4] soc: qcom: add an EBI2 device tree bindings

[Arnd Bergmann]

On Thursday 18 August 2016, Linus Walleij wrote:
> On Mon, Aug 8, 2016 at 11:32 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Monday, August 8, 2016 11:24:02 PM CEST Linus Walleij wrote:

> 
> So two devices "foo" and "bar" on the EBI2 bus, in the old
> scheme.
> 
> So I need to encode the CS in the first cell of the reg for
> foo-ebi2, and get rid of chipselect = <2> fine:
> 
> ebi2 at 1a100000 {
>        compatible = "qcom,msm8660-ebi2";
>        #address-cells = <1>;
>        #size-cells = <1>;
>        (...)
>        cs2 at 1b800000 {
>                #address-cells = <2>;
>                #size-cells = <1>;
>                qcom,xmem-recovery-cycles = <5>;
>                foo-ebi2 at 1b800000 {
>                        compatible = "foo";
>                        reg = <2 0x1b800000 0x100>;
>                        (...)
>                };
>                bar-ebi2 at 1ba00000 {
>                        compatible = "bar";
>                        reg = <2 0x1ba00000 0x100>;
>                        (...)
>                };
>        };
> };
> 
> This is not looking good at all. First: the configuration settings for
> the chipselect (i.e. all devices below it, both "foo" and "bar" are
> just floating in space. When parsing the tree how should you know
> what chipselect to set up the stuff on? Shall I check the child nodes
> first value in the reg property and then make a majority vote on what
> chipselect they apply to or what? That doesn't make sense.

I would have one node per chip-select and then put the devices on
that CS below it, with #address-cells=1 again.

> So I assume doing away with the chipselect node altogether would
> be prefered. But then: where do I put stuff like "qcom,xmem-recovery-cycles"
> that apply to the whole chipselect, not just a single subdevice on that
> chipselect? I certainly cannot encode it in the reg since it needs
> to be the same for all devices and it's not about addressing.
> 
> The only thing I can reasonably come up with would be this:
> 
> ebi2 at 1a100000 {
>        compatible = "qcom,msm8660-ebi2";
>        #address-cells = <2>;
>        #size-cells = <1>;
>        qcom,xmem-recovery-cycles = <0>, <0>, <5>, <0>, <0>, <0>;

No, better put the settings into one device per cs.

> So the chip select settings are shoehorned into an array in the top
> node that is then indexed to find the settings for cs0, cs1 etc.
> 
> There will be 6 such arrays for the different per-cs settings.
> 
> Is this what you want? I kind of thought the hierarchical model
> was nice since each device was below its chipselect node, but
> I understand that it breaks the pattern of other similar bus drivers.

Nothing wrong with having a hierarchy here, what I'm interested
in is making the addressing reflect what the hardware actually
does. With the empty "ranges", it looks like the entire 32-bit
address is getting exposed to the external bus, which is usually
not how things work: instead, each "cs" line gets raised by an I/O
operation on a particular CPU address range, and that range should
be part of the "ranges" propert of the main bus node, and the
externally visible addresses should be the translated addresses
in the child bus representation in DT.

	Arnd

[PATCH 1/4] soc: qcom: add an EBI2 device tree bindings

[Rob Herring]

On Mon, Aug 29, 2016 at 6:51 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday 18 August 2016, Linus Walleij wrote:
>> On Mon, Aug 8, 2016 at 11:32 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> > On Monday, August 8, 2016 11:24:02 PM CEST Linus Walleij wrote:
>
>>
>> So two devices "foo" and "bar" on the EBI2 bus, in the old
>> scheme.
>>
>> So I need to encode the CS in the first cell of the reg for
>> foo-ebi2, and get rid of chipselect = <2> fine:
>>
>> ebi2 at 1a100000 {
>>        compatible = "qcom,msm8660-ebi2";
>>        #address-cells = <1>;
>>        #size-cells = <1>;
>>        (...)
>>        cs2 at 1b800000 {
>>                #address-cells = <2>;
>>                #size-cells = <1>;
>>                qcom,xmem-recovery-cycles = <5>;
>>                foo-ebi2 at 1b800000 {
>>                        compatible = "foo";
>>                        reg = <2 0x1b800000 0x100>;
>>                        (...)
>>                };
>>                bar-ebi2 at 1ba00000 {
>>                        compatible = "bar";
>>                        reg = <2 0x1ba00000 0x100>;
>>                        (...)
>>                };
>>        };
>> };
>>
>> This is not looking good at all. First: the configuration settings for
>> the chipselect (i.e. all devices below it, both "foo" and "bar" are
>> just floating in space. When parsing the tree how should you know
>> what chipselect to set up the stuff on? Shall I check the child nodes
>> first value in the reg property and then make a majority vote on what
>> chipselect they apply to or what? That doesn't make sense.
>
> I would have one node per chip-select and then put the devices on
> that CS below it, with #address-cells=1 again.

Actually, how I've been guiding folks is 2 levels of nodes.

eim {
  #address-cells = <3>;
  cs at 0,0 {
    compatible = "bar,some-device";
    foo,eim-timing-prop = <0>;
  };

This is how most bindings have been done. There's not really any point
to 3 levels other than keeping CS properties separate, but they don't
need to be if they are properly prefixed. The timing for a device is a
property of the device even though we express them in terms of the
controller.

>> So I assume doing away with the chipselect node altogether would
>> be prefered. But then: where do I put stuff like "qcom,xmem-recovery-cycles"
>> that apply to the whole chipselect, not just a single subdevice on that
>> chipselect? I certainly cannot encode it in the reg since it needs
>> to be the same for all devices and it's not about addressing.
>>
>> The only thing I can reasonably come up with would be this:
>>
>> ebi2 at 1a100000 {
>>        compatible = "qcom,msm8660-ebi2";
>>        #address-cells = <2>;
>>        #size-cells = <1>;
>>        qcom,xmem-recovery-cycles = <0>, <0>, <5>, <0>, <0>, <0>;
>
> No, better put the settings into one device per cs.

Yes.

>
>> So the chip select settings are shoehorned into an array in the top
>> node that is then indexed to find the settings for cs0, cs1 etc.
>>
>> There will be 6 such arrays for the different per-cs settings.
>>
>> Is this what you want? I kind of thought the hierarchical model
>> was nice since each device was below its chipselect node, but
>> I understand that it breaks the pattern of other similar bus drivers.
>
> Nothing wrong with having a hierarchy here, what I'm interested
> in is making the addressing reflect what the hardware actually
> does. With the empty "ranges", it looks like the entire 32-bit
> address is getting exposed to the external bus, which is usually
> not how things work: instead, each "cs" line gets raised by an I/O
> operation on a particular CPU address range, and that range should
> be part of the "ranges" propert of the main bus node, and the
> externally visible addresses should be the translated addresses
> in the child bus representation in DT.

Yes.

[PATCH 1/4] soc: qcom: add an EBI2 device tree bindings

[Linus Walleij]

On Mon, Aug 29, 2016 at 2:24 PM, Rob Herring <robh+dt@kernel.org> wrote:
> On Mon, Aug 29, 2016 at 6:51 AM, Arnd Bergmann <arnd@arndb.de> wrote:

>> I would have one node per chip-select and then put the devices on
>> that CS below it, with #address-cells=1 again.
>
> Actually, how I've been guiding folks is 2 levels of nodes.
>
> eim {
>   #address-cells = <3>;
>   cs at 0,0 {
>     compatible = "bar,some-device";
>     foo,eim-timing-prop = <0>;
>   };
>
> This is how most bindings have been done. There's not really any point
> to 3 levels other than keeping CS properties separate, but they don't
> need to be if they are properly prefixed. The timing for a device is a
> property of the device even though we express them in terms of the
> controller.

This is what I'm doing in the most recent version from aug 24 I think.
http://marc.info/?l=linux-arm-kernel&m=147202889622540&w=2

>>> ebi2 at 1a100000 {
>>>        compatible = "qcom,msm8660-ebi2";
>>>        #address-cells = <2>;
>>>        #size-cells = <1>;
>>>        qcom,xmem-recovery-cycles = <0>, <0>, <5>, <0>, <0>, <0>;
>>
>> No, better put the settings into one device per cs.
>
> Yes.

Gnah. Each chipselect can (in theory) house several memory-mapped
devices at different offsets. Like two ethernet controllers. It gets a bit
weird.

This is how it currently looks (v2):

ebi2 at 1a100000 {
    compatible = "qcom,apq8060-ebi2";
    #address-cells = <2>;
    #size-cells = <1>;
    ranges = <0 0x0 0x1a800000 0x00800000>,
        <1 0x0 0x1b000000 0x00800000>,
        <2 0x0 0x1b800000 0x00800000>,
        <3 0x0 0x1d000000 0x00800000>,
        <4 0x0 0x1c800000 0x00800000>,
        <5 0x0 0x1c000000 0x00800000>;
    reg = <0x1a100000 0x1000>, <0x1a110000 0x1000>;
    reg-names = "ebi2", "xmem";
    qcom,xmem-recovery-cycles = <0>, <0>, <0>, <0>, <0>, <0>;
    qcom,xmem-write-hold-cycles = <0>, <0>, <3>, <0>, <0>, <0>;
    qcom,xmem-write-delta-cycles = <0>, <0>, <31>, <0>, <0>, <0>;
    qcom,xmem-read-delta-cycles = <0>, <0>, <28>, <0>, <0>, <0>;
    qcom,xmem-write-wait-cycles = <0>, <0>, <9>, <0>, <0>, <0>;
    qcom,xmem-read-wait-cycles = <0>, <0>, <9>, <0>, <0>, <0>;

    foo-ebi2 at 1b800000 {
        compatible = "foo";
        reg = <2 0x0 0x100>;
        (...)
    };
};

That is very close to how
Documentation/devicetree/bindings/bus/imx-weim.txt
drivers/bus/imx-weim.c
does things, just with the additional property arrays.

By looping over the subnodes and inspecting the first address cell of
them I can figure out what chipselects need to be turned on, and
the settings for those chipselects I need to activate are taken from
those 6 qcom,xmem* arrays of timings.

So if I reintroduce the cs nodes how do you imagine it'd look? Like this?

ebi2 at 1a100000 {
    compatible = "qcom,apq8060-ebi2";
    #address-cells = <1>;
    #size-cells = <1>;
    reg = <0x1a100000 0x1000>, <0x1a110000 0x1000>;
    reg-names = "ebi2", "xmem";

   cs at 2 {
       compatible = "bar,some-device";
       #address-cells = <1>;
       #size-cells = <1>;
       ranges = <0x0 0x1b800000 0x00800000>;
       chipselect = <2>;
       foo,eim-timing-prop = <0>;
       qcom,xmem-recovery-cycles = <0>;
       qcom,xmem-write-hold-cycles = <3>;
       qcom,xmem-write-delta-cycles = <31>;
       qcom,xmem-read-delta-cycles = <28>;
       qcom,xmem-write-wait-cycles = <9>;
       qcom,xmem-read-wait-cycles = <9>;

       foo-ebi2 at 0 {
           compatible = "foo";
           reg = <0x0 0x100>;
           (...)
       };

       bar-ebi2 at 100 {
           compatible = "bar";
           reg = <0x100 0x100>;
           (...)
       };

    };
};

Note: the chipselect goes out of the address cell, into the old
chipselect = <n> property, because we have no other way to know
which chipselect the settings apply to! (It's not like we can parse
the subnodes and make a majority decision...)

I kind of think both are non-perfect but the first one is the lesser
evil.

Yours,
Linus Walleij

[PATCH 1/4] soc: qcom: add an EBI2 device tree bindings

[Arnd Bergmann]

On Monday 29 August 2016, Linus Walleij wrote:
> Gnah. Each chipselect can (in theory) house several memory-mapped
> devices at different offsets. Like two ethernet controllers. It gets a bit
> weird.

How theoretical is that setup though? From looking at other external
bus interfaces that we support, I can't find anyone actually doing
this.

	Arnd

[PATCH 1/4] soc: qcom: add an EBI2 device tree bindings

[Linus Walleij]

On Mon, Aug 29, 2016 at 3:42 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday 29 August 2016, Linus Walleij wrote:
>> Gnah. Each chipselect can (in theory) house several memory-mapped
>> devices at different offsets. Like two ethernet controllers. It gets a bit
>> weird.
>
> How theoretical is that setup though? From looking at other external
> bus interfaces that we support, I can't find anyone actually doing
> this.

I only have this one device on EBI2.

However I suspect it could be common to use e.g. two
NOR flash chips in the same CS.

Yours,
Linus Walleij

[PATCH 1/4] soc: qcom: add an EBI2 device tree bindings

[Rob Herring]

On Mon, Aug 29, 2016 at 8:13 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Mon, Aug 29, 2016 at 2:24 PM, Rob Herring <robh+dt@kernel.org> wrote:
>> On Mon, Aug 29, 2016 at 6:51 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>
>>> I would have one node per chip-select and then put the devices on
>>> that CS below it, with #address-cells=1 again.
>>
>> Actually, how I've been guiding folks is 2 levels of nodes.
>>
>> eim {
>>   #address-cells = <3>;
>>   cs at 0,0 {
>>     compatible = "bar,some-device";
>>     foo,eim-timing-prop = <0>;
>>   };
>>
>> This is how most bindings have been done. There's not really any point
>> to 3 levels other than keeping CS properties separate, but they don't
>> need to be if they are properly prefixed. The timing for a device is a
>> property of the device even though we express them in terms of the
>> controller.
>
> This is what I'm doing in the most recent version from aug 24 I think.
> http://marc.info/?l=linux-arm-kernel&m=147202889622540&w=2
>
>>>> ebi2 at 1a100000 {
>>>>        compatible = "qcom,msm8660-ebi2";
>>>>        #address-cells = <2>;
>>>>        #size-cells = <1>;
>>>>        qcom,xmem-recovery-cycles = <0>, <0>, <5>, <0>, <0>, <0>;
>>>
>>> No, better put the settings into one device per cs.
>>
>> Yes.
>
> Gnah. Each chipselect can (in theory) house several memory-mapped
> devices at different offsets. Like two ethernet controllers. It gets a bit
> weird.

Then perhaps that is a case for a middle CS node. Or you can have 2
nodes on same CS with different offsets:

dev at 0,0 {};
dev at 0,10000 {};
dev at 1,0 {};

I guess either timing props get duplicated or you designate the first
node has them (2 devices on the same CS need to have the same timing
or have s/w setup per access).

> This is how it currently looks (v2):
>
> ebi2 at 1a100000 {
>     compatible = "qcom,apq8060-ebi2";
>     #address-cells = <2>;
>     #size-cells = <1>;
>     ranges = <0 0x0 0x1a800000 0x00800000>,
>         <1 0x0 0x1b000000 0x00800000>,
>         <2 0x0 0x1b800000 0x00800000>,
>         <3 0x0 0x1d000000 0x00800000>,
>         <4 0x0 0x1c800000 0x00800000>,
>         <5 0x0 0x1c000000 0x00800000>;
>     reg = <0x1a100000 0x1000>, <0x1a110000 0x1000>;
>     reg-names = "ebi2", "xmem";
>     qcom,xmem-recovery-cycles = <0>, <0>, <0>, <0>, <0>, <0>;
>     qcom,xmem-write-hold-cycles = <0>, <0>, <3>, <0>, <0>, <0>;
>     qcom,xmem-write-delta-cycles = <0>, <0>, <31>, <0>, <0>, <0>;
>     qcom,xmem-read-delta-cycles = <0>, <0>, <28>, <0>, <0>, <0>;
>     qcom,xmem-write-wait-cycles = <0>, <0>, <9>, <0>, <0>, <0>;
>     qcom,xmem-read-wait-cycles = <0>, <0>, <9>, <0>, <0>, <0>;
>
>     foo-ebi2 at 1b800000 {
>         compatible = "foo";
>         reg = <2 0x0 0x100>;
>         (...)
>     };
> };
>
> That is very close to how
> Documentation/devicetree/bindings/bus/imx-weim.txt
> drivers/bus/imx-weim.c
> does things, just with the additional property arrays.

Maybe so, but I'm trying to bring some consistency here, so I'd
suggest looking at newer ones like atmel IIRC.

> By looping over the subnodes and inspecting the first address cell of
> them I can figure out what chipselects need to be turned on, and
> the settings for those chipselects I need to activate are taken from
> those 6 qcom,xmem* arrays of timings.
>
> So if I reintroduce the cs nodes how do you imagine it'd look? Like this?

Between the 3 choices, what I've said is obviously my 1st choice. This
one would be 2nd (though chipselect should be part of the address
translation as Arnd suggested). The one above would be last.

> ebi2 at 1a100000 {
>     compatible = "qcom,apq8060-ebi2";
>     #address-cells = <1>;
>     #size-cells = <1>;
>     reg = <0x1a100000 0x1000>, <0x1a110000 0x1000>;
>     reg-names = "ebi2", "xmem";
>
>    cs at 2 {
>        compatible = "bar,some-device";
>        #address-cells = <1>;
>        #size-cells = <1>;
>        ranges = <0x0 0x1b800000 0x00800000>;
>        chipselect = <2>;
>        foo,eim-timing-prop = <0>;
>        qcom,xmem-recovery-cycles = <0>;
>        qcom,xmem-write-hold-cycles = <3>;
>        qcom,xmem-write-delta-cycles = <31>;
>        qcom,xmem-read-delta-cycles = <28>;
>        qcom,xmem-write-wait-cycles = <9>;
>        qcom,xmem-read-wait-cycles = <9>;
>
>        foo-ebi2 at 0 {
>            compatible = "foo";
>            reg = <0x0 0x100>;
>            (...)
>        };
>
>        bar-ebi2 at 100 {
>            compatible = "bar";
>            reg = <0x100 0x100>;
>            (...)
>        };
>
>     };
> };
>
> Note: the chipselect goes out of the address cell, into the old
> chipselect = <n> property, because we have no other way to know
> which chipselect the settings apply to! (It's not like we can parse
> the subnodes and make a majority decision...)
>
> I kind of think both are non-perfect but the first one is the lesser
> evil.
>
> Yours,
> Linus Walleij

[PATCH 1/4] soc: qcom: add an EBI2 device tree bindings

[Linus Walleij]

On Mon, Aug 29, 2016 at 4:06 PM, Rob Herring <robh+dt@kernel.org> wrote:
> [Me]
>> That is very close to how
>> Documentation/devicetree/bindings/bus/imx-weim.txt
>> drivers/bus/imx-weim.c
>> does things, just with the additional property arrays.
>
> Maybe so, but I'm trying to bring some consistency here, so I'd
> suggest looking at newer ones like atmel IIRC.

The Uniphier bindings also look like this and it's the most recent
bus driver ...
Documentation/devicetree/bindings/bus/uniphier-system-bus.txt

But OK it's probably best to follow an example so I get it the way
you prefer. Which binding and driver is that?

There's no Atmel stuff in Documentation/devicetree/bindings/bus/
nor drivers/bus from what I can tell, so just point out what you're
referring to here and I'll do my best to follow that example.

Yours,
Linus Walleij

[PATCH 1/4] soc: qcom: add an EBI2 device tree bindings

[Rob Herring]

On Mon, Aug 29, 2016 at 12:21 PM, Linus Walleij
<linus.walleij@linaro.org> wrote:
> On Mon, Aug 29, 2016 at 4:06 PM, Rob Herring <robh+dt@kernel.org> wrote:
>> [Me]
>>> That is very close to how
>>> Documentation/devicetree/bindings/bus/imx-weim.txt
>>> drivers/bus/imx-weim.c
>>> does things, just with the additional property arrays.
>>
>> Maybe so, but I'm trying to bring some consistency here, so I'd
>> suggest looking at newer ones like atmel IIRC.
>
> The Uniphier bindings also look like this and it's the most recent
> bus driver ...
> Documentation/devicetree/bindings/bus/uniphier-system-bus.txt
>
> But OK it's probably best to follow an example so I get it the way
> you prefer. Which binding and driver is that?
>
> There's no Atmel stuff in Documentation/devicetree/bindings/bus/
> nor drivers/bus from what I can tell, so just point out what you're
> referring to here and I'll do my best to follow that example.

Argg, that's because it is in a different spot:

Documentation/devicetree/bindings/memory-controllers/atmel,ebi.txt

Time to clean things up... bindings/bus is supposed to be internal,
on-chip buses.

Rob

[PATCH 1/4] soc: qcom: add an EBI2 device tree bindings

[Linus Walleij]

On Mon, Aug 29, 2016 at 1:51 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday 18 August 2016, Linus Walleij wrote:

>> This is not looking good at all. First: the configuration settings for
>> the chipselect (i.e. all devices below it, both "foo" and "bar" are
>> just floating in space. When parsing the tree how should you know
>> what chipselect to set up the stuff on? Shall I check the child nodes
>> first value in the reg property and then make a majority vote on what
>> chipselect they apply to or what? That doesn't make sense.
>
> I would have one node per chip-select and then put the devices on
> that CS below it, with #address-cells=1 again.

Hm I already sent out a new version I think...
http://marc.info/?l=linux-arm-kernel&m=147202889722541&w=2
http://marc.info/?l=linux-arm-kernel&m=147202889622540&w=2
http://marc.info/?l=linux-arm-kernel&m=147202890622543&w=2
http://marc.info/?l=linux-arm-kernel&m=147202894222555&w=2
http://marc.info/?l=linux-arm-kernel&m=147202898022571&w=2

(OK I know, no patience this guy...)

Check it and see what you think.

>> ebi2 at 1a100000 {
>>        compatible = "qcom,msm8660-ebi2";
>>        #address-cells = <2>;
>>        #size-cells = <1>;
>>        qcom,xmem-recovery-cycles = <0>, <0>, <5>, <0>, <0>, <0>;
>
> No, better put the settings into one device per cs.

Yeah sigh I already made a version like this...

> Nothing wrong with having a hierarchy here, what I'm interested
> in is making the addressing reflect what the hardware actually
> does. With the empty "ranges", it looks like the entire 32-bit
> address is getting exposed to the external bus, which is usually
> not how things work: instead, each "cs" line gets raised by an I/O
> operation on a particular CPU address range, and that range should
> be part of the "ranges" propert of the main bus node, and the
> externally visible addresses should be the translated addresses
> in the child bus representation in DT.

I have fixed the ranges too in the new version.

Yours,
Linus Walleij

[PATCH 1/4] soc: qcom: add an EBI2 device tree bindings

[Rob Herring]

On Mon, Aug 08, 2016 at 11:24:02PM +0200, Linus Walleij wrote:
> This adds device tree bindings for the External Bus Interface 2, EBI2
> to the Qualcomm SoC drivers.
> 
> Cc: devicetree at vger.kernel.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  .../devicetree/bindings/soc/qcom/qcom,ebi2.txt     | 134 +++++++++++++++++++++
>  1 file changed, 134 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,ebi2.txt

Please address my previous comments:

http://lists.infradead.org/pipermail/linux-arm-kernel/2016-July/443826.html

[PATCH 2/4] soc: qcom: add EBI2 driver

[Linus Walleij]

This adds a driver for the Qualcomm External Bus Interface EBI2
found in the MSM8660 and APQ8060 SoCs (at least).

This was tested with the SMSC9112 ethernet on the APQ8060
Dragonboard sitting on top of the SLOW CS2.

Some of my understanding if very vague and based on guesses and
extrapolations: the documentation in APQ8060 Qualcomm Application
Processor User Guide 80-N7150-14 Rev. A describes select features but
does not document the register bit fields.

However I had to do something like this to provide a serious driver,
I have marked the guesses with /* GUESS */

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/soc/qcom/Kconfig  |   8 +
 drivers/soc/qcom/Makefile |   1 +
 drivers/soc/qcom/ebi2.c   | 371 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 380 insertions(+)
 create mode 100644 drivers/soc/qcom/ebi2.c

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index 461b387d03cc..1f56baa30807 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -76,3 +76,11 @@ config QCOM_WCNSS_CTRL
 	help
 	  Client driver for the WCNSS_CTRL SMD channel, used to download nv
 	  firmware to a newly booted WCNSS chip.
+
+config QCOM_EBI2
+	bool "Qualcomm External Bus Interface 2 (EBI2)"
+	default y if ARCH_MSM8X60
+	help
+	  Say y here to enable support for the Qualcomm External Bus
+	  Interface 2, which can be used to connect things like NAND Flash,
+	  SRAM, ethernet adapters, FPGAs and LCD displays.
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index fdd664edf0bd..8d5a8d738d0a 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -7,3 +7,4 @@ obj-$(CONFIG_QCOM_SMEM_STATE) += smem_state.o
 obj-$(CONFIG_QCOM_SMP2P)	+= smp2p.o
 obj-$(CONFIG_QCOM_SMSM)	+= smsm.o
 obj-$(CONFIG_QCOM_WCNSS_CTRL) += wcnss_ctrl.o
+obj-$(CONFIG_QCOM_EBI2)	+= ebi2.o
diff --git a/drivers/soc/qcom/ebi2.c b/drivers/soc/qcom/ebi2.c
new file mode 100644
index 000000000000..f90c5d13fb5a
--- /dev/null
+++ b/drivers/soc/qcom/ebi2.c
@@ -0,0 +1,371 @@
+/*
+ * Qualcomm External Bus Interface 2 (EBI2) driver
+ * an older version of the Qualcomm Parallel Interface Controller (QPIC)
+ *
+ * Copyright (C) 2016 Linaro Ltd.
+ *
+ * Author: Linus Walleij <linus.walleij@linaro.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2, as
+ * published by the Free Software Foundation.
+ *
+ * See the device tree bindings for this block for more details on the
+ * hardware.
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/bitops.h>
+
+/* Guessed bit placement CS2 and CS3 are certain */
+/* What about CS1A, CS1B, CS2A, CS2B? */
+#define EBI2_CS0_ENABLE BIT(2) /* GUESS */
+#define EBI2_CS1_ENABLE BIT(3) /* GUESS */
+#define EBI2_CS2_ENABLE BIT(4)
+#define EBI2_CS3_ENABLE BIT(5)
+#define EBI2_CS4_ENABLE BIT(6) /* GUESS */
+#define EBI2_CS5_ENABLE BIT(7) /* GUESS */
+#define EBI2_CSN_MASK BIT(2)|BIT(3)|BIT(4)|BIT(5)|BIT(6)|BIT(7)
+
+#define EBI2_XMEM_CFG 0x0000 /* Power management etc */
+
+/*
+ * SLOW CSn CFG
+ *
+ * Bits 31-28: RECOVERY recovery cycles (0 = 1, 1 = 2 etc) this is the time the
+ *             memory continues to drive the data bus after OE is de-asserted.
+ *             Inserted when reading one CS and switching to another CS or read
+ *             followed by write on the same CS. Valid values 0 thru 15.
+ * Bits 27-24: WR_HOLD write hold cycles, these are extra cycles inserted after
+ *             every write minimum 1. The data out is driven from the time WE is
+ *             asserted until CS is asserted. With a hold of 1, the CS stays
+ *             active for 1 extra cycle etc. Valid values 0 thru 15.
+ * Bits 23-16: WR_DELTA initial latency for write cycles inserted for the first
+ *             write to a page or burst memory
+ * Bits 15-8:  RD_DELTA initial latency for read cycles inserted for the first
+ *             read to a page or burst memory
+ * Bits 7-4:   WR_WAIT number of wait cycles for every write access, 0=1 cycle
+ *             so 1 thru 16 cycles.
+ * Bits 3-0:   RD_WAIT number of wait cycles for every read access, 0=1 cycle
+ *             so 1 thru 16 cycles.
+ */
+#define EBI2_XMEM_CS0_SLOW_CFG 0x0008 /* GUESS */
+#define EBI2_XMEM_CS1_SLOW_CFG 0x000C /* GUESS */
+#define EBI2_XMEM_CS2_SLOW_CFG 0x0010
+#define EBI2_XMEM_CS3_SLOW_CFG 0x0014
+#define EBI2_XMEM_CS4_SLOW_CFG 0x0018 /* GUESS */
+#define EBI2_XMEM_CS5_SLOW_CFG 0x001C /* GUESS */
+
+#define EBI2_XMEM_RECOVERY_SHIFT	28
+#define EBI2_XMEM_WR_HOLD_SHIFT		24
+#define EBI2_XMEM_WR_DELTA_SHIFT	16
+#define EBI2_XMEM_RD_DELTA_SHIFT	8
+#define EBI2_XMEM_WR_WAIT_SHIFT		4
+#define EBI2_XMEM_RD_WAIT_SHIFT		0
+
+/*
+ * FAST CSn CFG
+ * Bits 31-28: ?
+ * Bits 27-24: RD_HOLD: the length in cycles of the first segment of a read
+ *             transfer. For a single read trandfer this will be the time
+ *             from CS assertion to OE assertion.
+ * Bits 18-24: ?
+ * Bits 17-16: ADV_OE_RECOVERY, the number of cycles elapsed before an OE
+ *             assertion, with respect to the cycle where ADV is asserted.
+ *             2 means 2 cycles between ADV and OE. Values 0, 1, 2 or 3.
+ * Bits 5:     ADDR_HOLD_ENA, The address is held for an extra cycle to meet
+ *             hold time requirements with ADV assertion.
+ *
+ * The manual mentions "write precharge cycles" and "precharge cycles".
+ * We have not been able to figure out which bit fields these correspond to
+ * in the hardware, or what valid values exist. The current hypothesis is that
+ * this is something just used on the FAST chip selects. There is also a "byte
+ * device enable" flag somewhere for 8bit memories.
+ */
+#define EBI2_XMEM_CS0_FAST_CFG 0x0028 /* GUESS */
+#define EBI2_XMEM_CS1_FAST_CFG 0x002C /* GUESS */
+#define EBI2_XMEM_CS2_FAST_CFG 0x0030 /* GUESS */
+#define EBI2_XMEM_CS3_FAST_CFG 0x0034
+#define EBI2_XMEM_CS4_FAST_CFG 0x0038 /* GUESS */
+#define EBI2_XMEM_CS5_FAST_CFG 0x003C /* GUESS */
+
+#define EBI2_XMEM_RD_HOLD_SHIFT		24
+#define EBI2_XMEM_ADV_OE_RECOVERY_SHIFT	16
+#define EBI2_XMEM_ADDR_HOLD_ENA_SHIFT	5
+
+/**
+ * struct cs_data - struct with info on a chipselect setting
+ * @enable_mask: mask to enable the chipselect in the EBI2 config
+ * @slow_cfg0: offset to XMEMC slow CS config
+ * @fast_cfg1: offset to XMEMC fast CS config
+ */
+struct cs_data {
+	u32 enable_mask;
+	u16 slow_cfg;
+	u16 fast_cfg;
+};
+
+static const struct cs_data cs_info[] = {
+	{
+		/* CS0 */
+		.enable_mask = EBI2_CS0_ENABLE,
+		.slow_cfg = EBI2_XMEM_CS0_SLOW_CFG,
+		.fast_cfg = EBI2_XMEM_CS0_FAST_CFG,
+	},
+	{
+		/* CS1 */
+		.enable_mask = EBI2_CS1_ENABLE,
+		.slow_cfg = EBI2_XMEM_CS1_SLOW_CFG,
+		.fast_cfg = EBI2_XMEM_CS1_FAST_CFG,
+	},
+	{
+		/* CS2 */
+		.enable_mask = EBI2_CS2_ENABLE,
+		.slow_cfg = EBI2_XMEM_CS2_SLOW_CFG,
+		.fast_cfg = EBI2_XMEM_CS2_FAST_CFG,
+	},
+	{
+		/* CS3 */
+		.enable_mask = EBI2_CS3_ENABLE,
+		.slow_cfg = EBI2_XMEM_CS3_SLOW_CFG,
+		.fast_cfg = EBI2_XMEM_CS3_FAST_CFG,
+	},
+	{
+		/* CS4 */
+		.enable_mask = EBI2_CS4_ENABLE,
+		.slow_cfg = EBI2_XMEM_CS4_SLOW_CFG,
+		.fast_cfg = EBI2_XMEM_CS4_FAST_CFG,
+	},
+	{
+		/* CS5 */
+		.enable_mask = EBI2_CS5_ENABLE,
+		.slow_cfg = EBI2_XMEM_CS5_SLOW_CFG,
+		.fast_cfg = EBI2_XMEM_CS5_FAST_CFG,
+	},
+};
+
+/**
+ * struct ebi2_xmem_prop - describes an XMEM config property
+ * @prop: the device tree binding name
+ * @max: maximum value for the property
+ * @slowreg: true if this property is in the SLOW CS config register
+ * else it is assumed to be in the FAST config register
+ * @shift: the bit field start in the SLOW or FAST register for this
+ * property
+ */
+struct ebi2_xmem_prop {
+	const char *prop;
+	u32 max;
+	bool slowreg;
+	u16 shift;
+};
+
+static const struct ebi2_xmem_prop xmem_props[] = {
+	{
+		.prop = "xmem-recovery-cycles",
+		.max = 15,
+		.slowreg = true,
+		.shift = EBI2_XMEM_RECOVERY_SHIFT,
+	},
+	{
+		.prop = "xmem-write-hold-cycles",
+		.max = 15,
+		.slowreg = true,
+		.shift = EBI2_XMEM_WR_HOLD_SHIFT,
+	},
+	{
+		.prop = "xmem-write-delta-cycles",
+		.max = 255,
+		.slowreg = true,
+		.shift = EBI2_XMEM_WR_DELTA_SHIFT,
+	},
+	{
+		.prop = "xmem-read-delta-cycles",
+		.max = 255,
+		.slowreg = true,
+		.shift = EBI2_XMEM_RD_DELTA_SHIFT,
+	},
+	{
+		.prop = "xmem-write-wait-cycles",
+		.max = 15,
+		.slowreg = true,
+		.shift = EBI2_XMEM_WR_WAIT_SHIFT,
+	},
+	{
+		.prop = "xmem-read-wait-cycles",
+		.max = 15,
+		.slowreg = true,
+		.shift = EBI2_XMEM_RD_WAIT_SHIFT,
+	},
+	{
+		.prop = "xmem-address-hold-enable",
+		.max = 1, /* boolean prop */
+		.slowreg = false,
+		.shift = EBI2_XMEM_ADDR_HOLD_ENA_SHIFT,
+	},
+	{
+		.prop = "xmem-adv-to-oe-recovery-cycles",
+		.max = 3,
+		.slowreg = false,
+		.shift = EBI2_XMEM_ADV_OE_RECOVERY_SHIFT,
+	},
+	{
+		.prop = "xmem-read-hold-cycles",
+		.max = 15,
+		.slowreg = false,
+		.shift = EBI2_XMEM_RD_HOLD_SHIFT,
+	},
+};
+
+static int qcom_ebi2_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct device_node *cs;
+	struct device *dev = &pdev->dev;
+	struct resource *res;
+	void __iomem *ebi2_base;
+	void __iomem *ebi2_xmem;
+	int ret;
+	struct clk *clk;
+	u32 val;
+	u32 csval;
+
+	clk = devm_clk_get(dev, "ebi2x");
+	if (IS_ERR(clk)) {
+		dev_err(dev, "could not get EBI2X clk (%li)\n", PTR_ERR(clk));
+		return PTR_ERR(clk);
+	}
+
+	ret = clk_prepare_enable(clk);
+	if (ret) {
+		dev_err(dev, "could not enable EBI2X clk (%d)\n", ret);
+		return ret;
+	}
+
+	clk = devm_clk_get(dev, "ebi2");
+	if (IS_ERR(clk)) {
+		dev_err(dev, "could not get EBI2 clk\n");
+		return PTR_ERR(clk);
+	}
+
+	ret = clk_prepare_enable(clk);
+	if (ret) {
+		dev_err(dev, "could not enable EBI2 clk\n");
+		return ret;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	ebi2_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(ebi2_base))
+		return PTR_ERR(ebi2_base);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	ebi2_xmem = devm_ioremap_resource(dev, res);
+	if (IS_ERR(ebi2_xmem))
+		return PTR_ERR(ebi2_xmem);
+
+	/* Allegedly this turns the power save mode off */
+	writel_relaxed(0UL, ebi2_xmem + EBI2_XMEM_CFG);
+
+	/* Disable all chipselects */
+	csval = readl_relaxed(ebi2_base);
+	csval &= ~EBI2_CSN_MASK;
+	writel_relaxed(csval, ebi2_base);
+
+	/* Walk over the child nodes, one for each chip select */
+	for_each_available_child_of_node(np, cs) {
+		const struct cs_data *csd;
+		u32 slowcfg, fastcfg;
+		u32 csindex;
+		int i;
+
+		ret = of_property_read_u32(cs, "chipselect", &csindex);
+		if (ret)
+			/* Invalid node or whatever */
+			continue;
+		if (csindex > 5) {
+			dev_err(dev,
+				"invalid chipselect %u, we only support 0-5\n",
+				csindex);
+			continue;
+		}
+		csd = &cs_info[csindex];
+		csval |= csd->enable_mask;
+		writel_relaxed(csval, ebi2_base);
+		dev_info(dev, "enabled CS%u\n", csindex);
+
+		/* Next set up the XMEMC */
+		slowcfg = 0;
+		fastcfg = 0;
+
+		for (i = 0; i < ARRAY_SIZE(xmem_props); i++) {
+			const struct ebi2_xmem_prop *xp = &xmem_props[i];
+
+			/* First check boolean props */
+			if (xp->max == 1) {
+				if (of_property_read_bool(cs, xp->prop)) {
+					if (xp->slowreg)
+						slowcfg |= BIT(xp->shift);
+					else
+						fastcfg |= BIT(xp->shift);
+					dev_info(dev, "set %s flag\n", xp->prop);
+				}
+				continue;
+			}
+
+			ret = of_property_read_u32(cs, xp->prop, &val);
+			if (ret)
+				continue;
+
+			/* We're dealing with an u32 */
+			if (val > xp->max) {
+				dev_err(dev,
+					"too high value for %s: %u, capped at %u\n",
+					xp->prop, val, xp->max);
+				val = xp->max;
+			}
+			if (xp->slowreg)
+				slowcfg |= (val << xp->shift);
+			else
+				fastcfg |= (val << xp->shift);
+			dev_info(dev, "set %s to %u\n", xp->prop, val);
+		}
+
+		dev_info(dev, "CS%u: SLOW CFG 0x%08x, FAST CFG 0x%08x\n",
+			 csindex, slowcfg, fastcfg);
+
+		if (slowcfg)
+			writel_relaxed(slowcfg, ebi2_xmem + csd->slow_cfg);
+		if (fastcfg)
+			writel_relaxed(fastcfg, ebi2_xmem + csd->fast_cfg);
+
+		/* Now the CS is online: time to populate the children */
+		ret = of_platform_default_populate(cs, NULL, dev);
+		if (ret)
+			dev_err(dev, "failed to populate CS%u\n", val);
+	}
+
+	return 0;
+}
+
+static const struct of_device_id qcom_ebi2_of_match[] = {
+	{ .compatible = "qcom,ebi2", },
+	{ }
+};
+
+static struct platform_driver qcom_ebi2_driver = {
+	.probe = qcom_ebi2_probe,
+	.driver = {
+		.name = "qcom-ebi2",
+		.of_match_table = qcom_ebi2_of_match,
+	},
+};
+builtin_platform_driver(qcom_ebi2_driver);
-- 
2.7.4

[PATCH 2/4] soc: qcom: add EBI2 driver

[Arnd Bergmann]

On Monday, August 8, 2016 11:24:03 PM CEST Linus Walleij wrote:
> This adds a driver for the Qualcomm External Bus Interface EBI2
> found in the MSM8660 and APQ8060 SoCs (at least).
> 
> This was tested with the SMSC9112 ethernet on the APQ8060
> Dragonboard sitting on top of the SLOW CS2.
> 
> Some of my understanding if very vague and based on guesses and
> extrapolations: the documentation in APQ8060 Qualcomm Application
> Processor User Guide 80-N7150-14 Rev. A describes select features but
> does not document the register bit fields.
> 
> However I had to do something like this to provide a serious driver,
> I have marked the guesses with /* GUESS */
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/soc/qcom/Kconfig  |   8 +
>  drivers/soc/qcom/Makefile |   1 +
>  drivers/soc/qcom/ebi2.c   | 371 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 380 insertions(+)
>  create mode 100644 drivers/soc/qcom/ebi2.c
> 

This should probably live in drivers/memory, or possibly drivers/bus.

	Arnd

[PATCH 2/4] soc: qcom: add EBI2 driver

[Stephen Boyd]

On 08/08/2016 02:24 PM, Linus Walleij wrote:
> ---
>  drivers/soc/qcom/Kconfig  |   8 +
>  drivers/soc/qcom/Makefile |   1 +
>  drivers/soc/qcom/ebi2.c   | 371 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 380 insertions(+)

drivers/bus seems ok to me.

>  create mode 100644 drivers/soc/qcom/ebi2.c
>
> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> index 461b387d03cc..1f56baa30807 100644
> --- a/drivers/soc/qcom/Kconfig
> +++ b/drivers/soc/qcom/Kconfig
> @@ -76,3 +76,11 @@ config QCOM_WCNSS_CTRL
>  	help
>  	  Client driver for the WCNSS_CTRL SMD channel, used to download nv
>  	  firmware to a newly booted WCNSS chip.
> +
> +config QCOM_EBI2
> +	bool "Qualcomm External Bus Interface 2 (EBI2)"
> +	default y if ARCH_MSM8X60

Please don't do any default. I've been trying to get rid of ARCH_MSM8X60
as a Kconfig symbol for some time.

> +	help
> +	  Say y here to enable support for the Qualcomm External Bus
> +	  Interface 2, which can be used to connect things like NAND Flash,
> +	  SRAM, ethernet adapters, FPGAs and LCD displays.
> diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
> index fdd664edf0bd..8d5a8d738d0a 100644
> --- a/drivers/soc/qcom/Makefile
> +++ b/drivers/soc/qcom/Makefile
> @@ -7,3 +7,4 @@ obj-$(CONFIG_QCOM_SMEM_STATE) += smem_state.o
>  obj-$(CONFIG_QCOM_SMP2P)	+= smp2p.o
>  obj-$(CONFIG_QCOM_SMSM)	+= smsm.o
>  obj-$(CONFIG_QCOM_WCNSS_CTRL) += wcnss_ctrl.o
> +obj-$(CONFIG_QCOM_EBI2)	+= ebi2.o
> diff --git a/drivers/soc/qcom/ebi2.c b/drivers/soc/qcom/ebi2.c
> new file mode 100644
> index 000000000000..f90c5d13fb5a
> --- /dev/null
> +++ b/drivers/soc/qcom/ebi2.c
> @@ -0,0 +1,371 @@
> +/*
> + * Qualcomm External Bus Interface 2 (EBI2) driver
> + * an older version of the Qualcomm Parallel Interface Controller (QPIC)
> + *
> + * Copyright (C) 2016 Linaro Ltd.
> + *
> + * Author: Linus Walleij <linus.walleij@linaro.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2, as
> + * published by the Free Software Foundation.
> + *
> + * See the device tree bindings for this block for more details on the
> + * hardware.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/slab.h>
> +#include <linux/platform_device.h>
> +#include <linux/bitops.h>
> +
> +/* Guessed bit placement CS2 and CS3 are certain */
> +/* What about CS1A, CS1B, CS2A, CS2B? */
> +#define EBI2_CS0_ENABLE BIT(2) /* GUESS */
> +#define EBI2_CS1_ENABLE BIT(3) /* GUESS */
> +#define EBI2_CS2_ENABLE BIT(4)
> +#define EBI2_CS3_ENABLE BIT(5)
> +#define EBI2_CS4_ENABLE BIT(6) /* GUESS */
> +#define EBI2_CS5_ENABLE BIT(7) /* GUESS */
> +#define EBI2_CSN_MASK BIT(2)|BIT(3)|BIT(4)|BIT(5)|BIT(6)|BIT(7)

CS0, CS1, CS4, CS5 are 2 bits wide.

> +
> +#define EBI2_XMEM_CFG 0x0000 /* Power management etc */
> +
> +/*
> + * SLOW CSn CFG
> + *
> + * Bits 31-28: RECOVERY recovery cycles (0 = 1, 1 = 2 etc) this is the time the
> + *             memory continues to drive the data bus after OE is de-asserted.
> + *             Inserted when reading one CS and switching to another CS or read
> + *             followed by write on the same CS. Valid values 0 thru 15.
> + * Bits 27-24: WR_HOLD write hold cycles, these are extra cycles inserted after
> + *             every write minimum 1. The data out is driven from the time WE is
> + *             asserted until CS is asserted. With a hold of 1, the CS stays
> + *             active for 1 extra cycle etc. Valid values 0 thru 15.
> + * Bits 23-16: WR_DELTA initial latency for write cycles inserted for the first
> + *             write to a page or burst memory
> + * Bits 15-8:  RD_DELTA initial latency for read cycles inserted for the first
> + *             read to a page or burst memory
> + * Bits 7-4:   WR_WAIT number of wait cycles for every write access, 0=1 cycle
> + *             so 1 thru 16 cycles.
> + * Bits 3-0:   RD_WAIT number of wait cycles for every read access, 0=1 cycle
> + *             so 1 thru 16 cycles.
> + */
> +#define EBI2_XMEM_CS0_SLOW_CFG 0x0008 /* GUESS */
> +#define EBI2_XMEM_CS1_SLOW_CFG 0x000C /* GUESS */
> +#define EBI2_XMEM_CS2_SLOW_CFG 0x0010
> +#define EBI2_XMEM_CS3_SLOW_CFG 0x0014
> +#define EBI2_XMEM_CS4_SLOW_CFG 0x0018 /* GUESS */
> +#define EBI2_XMEM_CS5_SLOW_CFG 0x001C /* GUESS */

Guesses are correct.

> +
> +#define EBI2_XMEM_RECOVERY_SHIFT	28
> +#define EBI2_XMEM_WR_HOLD_SHIFT		24
> +#define EBI2_XMEM_WR_DELTA_SHIFT	16
> +#define EBI2_XMEM_RD_DELTA_SHIFT	8
> +#define EBI2_XMEM_WR_WAIT_SHIFT		4
> +#define EBI2_XMEM_RD_WAIT_SHIFT		0
> +
> +/*
> + * FAST CSn CFG
> + * Bits 31-28: ?
> + * Bits 27-24: RD_HOLD: the length in cycles of the first segment of a read
> + *             transfer. For a single read trandfer this will be the time
> + *             from CS assertion to OE assertion.
> + * Bits 18-24: ?
> + * Bits 17-16: ADV_OE_RECOVERY, the number of cycles elapsed before an OE
> + *             assertion, with respect to the cycle where ADV is asserted.
> + *             2 means 2 cycles between ADV and OE. Values 0, 1, 2 or 3.
> + * Bits 5:     ADDR_HOLD_ENA, The address is held for an extra cycle to meet
> + *             hold time requirements with ADV assertion.
> + *
> + * The manual mentions "write precharge cycles" and "precharge cycles".
> + * We have not been able to figure out which bit fields these correspond to
> + * in the hardware, or what valid values exist. The current hypothesis is that
> + * this is something just used on the FAST chip selects. There is also a "byte
> + * device enable" flag somewhere for 8bit memories.
> + */
> +#define EBI2_XMEM_CS0_FAST_CFG 0x0028 /* GUESS */
> +#define EBI2_XMEM_CS1_FAST_CFG 0x002C /* GUESS */
> +#define EBI2_XMEM_CS2_FAST_CFG 0x0030 /* GUESS */
> +#define EBI2_XMEM_CS3_FAST_CFG 0x0034
> +#define EBI2_XMEM_CS4_FAST_CFG 0x0038 /* GUESS */
> +#define EBI2_XMEM_CS5_FAST_CFG 0x003C /* GUESS */

Guesses are correct.

> +
> +#define EBI2_XMEM_RD_HOLD_SHIFT		24
> +#define EBI2_XMEM_ADV_OE_RECOVERY_SHIFT	16
> +#define EBI2_XMEM_ADDR_HOLD_ENA_SHIFT	5
> +
> +/**
> + * struct cs_data - struct with info on a chipselect setting
> + * @enable_mask: mask to enable the chipselect in the EBI2 config
> + * @slow_cfg0: offset to XMEMC slow CS config
> + * @fast_cfg1: offset to XMEMC fast CS config
> + */
> +struct cs_data {
> +	u32 enable_mask;
> +	u16 slow_cfg;
> +	u16 fast_cfg;
> +};
> +
> +static const struct cs_data cs_info[] = {
> +	{
> +		/* CS0 */
> +		.enable_mask = EBI2_CS0_ENABLE,
> +		.slow_cfg = EBI2_XMEM_CS0_SLOW_CFG,
> +		.fast_cfg = EBI2_XMEM_CS0_FAST_CFG,
> +	},
> +	{
> +		/* CS1 */
> +		.enable_mask = EBI2_CS1_ENABLE,
> +		.slow_cfg = EBI2_XMEM_CS1_SLOW_CFG,
> +		.fast_cfg = EBI2_XMEM_CS1_FAST_CFG,
> +	},
> +	{
> +		/* CS2 */
> +		.enable_mask = EBI2_CS2_ENABLE,
> +		.slow_cfg = EBI2_XMEM_CS2_SLOW_CFG,
> +		.fast_cfg = EBI2_XMEM_CS2_FAST_CFG,
> +	},
> +	{
> +		/* CS3 */
> +		.enable_mask = EBI2_CS3_ENABLE,
> +		.slow_cfg = EBI2_XMEM_CS3_SLOW_CFG,
> +		.fast_cfg = EBI2_XMEM_CS3_FAST_CFG,
> +	},
> +	{
> +		/* CS4 */
> +		.enable_mask = EBI2_CS4_ENABLE,
> +		.slow_cfg = EBI2_XMEM_CS4_SLOW_CFG,
> +		.fast_cfg = EBI2_XMEM_CS4_FAST_CFG,
> +	},
> +	{
> +		/* CS5 */
> +		.enable_mask = EBI2_CS5_ENABLE,
> +		.slow_cfg = EBI2_XMEM_CS5_SLOW_CFG,
> +		.fast_cfg = EBI2_XMEM_CS5_FAST_CFG,
> +	},
> +};
> +
> +/**
> + * struct ebi2_xmem_prop - describes an XMEM config property
> + * @prop: the device tree binding name
> + * @max: maximum value for the property
> + * @slowreg: true if this property is in the SLOW CS config register
> + * else it is assumed to be in the FAST config register
> + * @shift: the bit field start in the SLOW or FAST register for this
> + * property
> + */
> +struct ebi2_xmem_prop {
> +	const char *prop;
> +	u32 max;
> +	bool slowreg;
> +	u16 shift;
> +};
> +
> +static const struct ebi2_xmem_prop xmem_props[] = {
> +	{
> +		.prop = "xmem-recovery-cycles",
> +		.max = 15,
> +		.slowreg = true,
> +		.shift = EBI2_XMEM_RECOVERY_SHIFT,
> +	},
> +	{
> +		.prop = "xmem-write-hold-cycles",
> +		.max = 15,
> +		.slowreg = true,
> +		.shift = EBI2_XMEM_WR_HOLD_SHIFT,
> +	},
> +	{
> +		.prop = "xmem-write-delta-cycles",
> +		.max = 255,
> +		.slowreg = true,
> +		.shift = EBI2_XMEM_WR_DELTA_SHIFT,
> +	},
> +	{
> +		.prop = "xmem-read-delta-cycles",
> +		.max = 255,
> +		.slowreg = true,
> +		.shift = EBI2_XMEM_RD_DELTA_SHIFT,
> +	},
> +	{
> +		.prop = "xmem-write-wait-cycles",
> +		.max = 15,
> +		.slowreg = true,
> +		.shift = EBI2_XMEM_WR_WAIT_SHIFT,
> +	},
> +	{
> +		.prop = "xmem-read-wait-cycles",
> +		.max = 15,
> +		.slowreg = true,
> +		.shift = EBI2_XMEM_RD_WAIT_SHIFT,
> +	},
> +	{
> +		.prop = "xmem-address-hold-enable",
> +		.max = 1, /* boolean prop */
> +		.slowreg = false,
> +		.shift = EBI2_XMEM_ADDR_HOLD_ENA_SHIFT,
> +	},
> +	{
> +		.prop = "xmem-adv-to-oe-recovery-cycles",
> +		.max = 3,
> +		.slowreg = false,
> +		.shift = EBI2_XMEM_ADV_OE_RECOVERY_SHIFT,
> +	},
> +	{
> +		.prop = "xmem-read-hold-cycles",
> +		.max = 15,
> +		.slowreg = false,
> +		.shift = EBI2_XMEM_RD_HOLD_SHIFT,
> +	},
> +};
> +
> +static int qcom_ebi2_probe(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct device_node *cs;
> +	struct device *dev = &pdev->dev;
> +	struct resource *res;
> +	void __iomem *ebi2_base;
> +	void __iomem *ebi2_xmem;
> +	int ret;
> +	struct clk *clk;
> +	u32 val;
> +	u32 csval;
> +
> +	clk = devm_clk_get(dev, "ebi2x");
> +	if (IS_ERR(clk)) {
> +		dev_err(dev, "could not get EBI2X clk (%li)\n", PTR_ERR(clk));
> +		return PTR_ERR(clk);

This could be noisy on probe defer...

> +	}
> +
> +	ret = clk_prepare_enable(clk);
> +	if (ret) {
> +		dev_err(dev, "could not enable EBI2X clk (%d)\n", ret);
> +		return ret;
> +	}
> +
> +	clk = devm_clk_get(dev, "ebi2");
> +	if (IS_ERR(clk)) {
> +		dev_err(dev, "could not get EBI2 clk\n");
> +		return PTR_ERR(clk);
> +	}
> +
> +	ret = clk_prepare_enable(clk);
> +	if (ret) {
> +		dev_err(dev, "could not enable EBI2 clk\n");
> +		return ret;

clk_disable_unprepare ebi2x clk? Or perhaps get both clks first and then
try to enable both in succession.

> +	}
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	ebi2_base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(ebi2_base))
> +		return PTR_ERR(ebi2_base);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +	ebi2_xmem = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(ebi2_xmem))
> +		return PTR_ERR(ebi2_xmem);
> +
> +	/* Allegedly this turns the power save mode off */
> +	writel_relaxed(0UL, ebi2_xmem + EBI2_XMEM_CFG);
> +
> +	/* Disable all chipselects */
> +	csval = readl_relaxed(ebi2_base);
> +	csval &= ~EBI2_CSN_MASK;
> +	writel_relaxed(csval, ebi2_base);
> +
> +	/* Walk over the child nodes, one for each chip select */
> +	for_each_available_child_of_node(np, cs) {
> +		const struct cs_data *csd;
> +		u32 slowcfg, fastcfg;
> +		u32 csindex;
> +		int i;
> +
> +		ret = of_property_read_u32(cs, "chipselect", &csindex);
> +		if (ret)
> +			/* Invalid node or whatever */
> +			continue;
> +		if (csindex > 5) {
> +			dev_err(dev,
> +				"invalid chipselect %u, we only support 0-5\n",
> +				csindex);
> +			continue;
> +		}
> +		csd = &cs_info[csindex];
> +		csval |= csd->enable_mask;
> +		writel_relaxed(csval, ebi2_base);
> +		dev_info(dev, "enabled CS%u\n", csindex);

dev_dbg?

> +
> +		/* Next set up the XMEMC */
> +		slowcfg = 0;
> +		fastcfg = 0;
> +
> +		for (i = 0; i < ARRAY_SIZE(xmem_props); i++) {
> +			const struct ebi2_xmem_prop *xp = &xmem_props[i];
> +
> +			/* First check boolean props */
> +			if (xp->max == 1) {
> +				if (of_property_read_bool(cs, xp->prop)) {
> +					if (xp->slowreg)
> +						slowcfg |= BIT(xp->shift);
> +					else
> +						fastcfg |= BIT(xp->shift);
> +					dev_info(dev, "set %s flag\n", xp->prop);

dev_dbg?

> +				}
> +				continue;
> +			}
> +
> +			ret = of_property_read_u32(cs, xp->prop, &val);
> +			if (ret)
> +				continue;
> +
> +			/* We're dealing with an u32 */
> +			if (val > xp->max) {
> +				dev_err(dev,
> +					"too high value for %s: %u, capped at %u\n",
> +					xp->prop, val, xp->max);
> +				val = xp->max;
> +			}
> +			if (xp->slowreg)
> +				slowcfg |= (val << xp->shift);
> +			else
> +				fastcfg |= (val << xp->shift);
> +			dev_info(dev, "set %s to %u\n", xp->prop, val);

dev_dbg?

> +		}
> +
> +		dev_info(dev, "CS%u: SLOW CFG 0x%08x, FAST CFG 0x%08x\n",
> +			 csindex, slowcfg, fastcfg);

dev_dbg?

> +
> +		if (slowcfg)
> +			writel_relaxed(slowcfg, ebi2_xmem + csd->slow_cfg);
> +		if (fastcfg)
> +			writel_relaxed(fastcfg, ebi2_xmem + csd->fast_cfg);

The documentation just speaks of config0 and config1, but I guess if
slow and fast is meaningful to you then it's ok.

> +
> +		/* Now the CS is online: time to populate the children */
> +		ret = of_platform_default_populate(cs, NULL, dev);
> +		if (ret)
> +			dev_err(dev, "failed to populate CS%u\n", val);
> +	}
> +
> +	return 0;
> +}
> +

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 2/4] soc: qcom: add EBI2 driver

[Linus Walleij]

On Tue, Aug 9, 2016 at 1:33 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 08/08/2016 02:24 PM, Linus Walleij wrote:

> drivers/bus seems ok to me.

Moved to drivers/bus.

>> +config QCOM_EBI2
>> +     bool "Qualcomm External Bus Interface 2 (EBI2)"
>> +     default y if ARCH_MSM8X60
>
> Please don't do any default. I've been trying to get rid of ARCH_MSM8X60
> as a Kconfig symbol for some time.

OK cut it, let's defconfig it in or something.

>> +/* Guessed bit placement CS2 and CS3 are certain */
>> +/* What about CS1A, CS1B, CS2A, CS2B? */
>> +#define EBI2_CS0_ENABLE BIT(2) /* GUESS */
>> +#define EBI2_CS1_ENABLE BIT(3) /* GUESS */
>> +#define EBI2_CS2_ENABLE BIT(4)
>> +#define EBI2_CS3_ENABLE BIT(5)
>> +#define EBI2_CS4_ENABLE BIT(6) /* GUESS */
>> +#define EBI2_CS5_ENABLE BIT(7) /* GUESS */
>> +#define EBI2_CSN_MASK BIT(2)|BIT(3)|BIT(4)|BIT(5)|BIT(6)|BIT(7)
>
> CS0, CS1, CS4, CS5 are 2 bits wide.

I guess that reads:
CS0 bit 0,1
CS1 bit 2,3
CS2 bit 4
CS3 bit 5
CS4 bit 6,7
CS5 bit 8,9

And the register is 32bits wide. Augmenting the code like this.

>> +/*
>> + * FAST CSn CFG
>> + * Bits 31-28: ?
>> + * Bits 27-24: RD_HOLD: the length in cycles of the first segment of a read
>> + *             transfer. For a single read trandfer this will be the time
>> + *             from CS assertion to OE assertion.
>> + * Bits 18-24: ?
>> + * Bits 17-16: ADV_OE_RECOVERY, the number of cycles elapsed before an OE
>> + *             assertion, with respect to the cycle where ADV is asserted.
>> + *             2 means 2 cycles between ADV and OE. Values 0, 1, 2 or 3.
>> + * Bits 5:     ADDR_HOLD_ENA, The address is held for an extra cycle to meet
>> + *             hold time requirements with ADV assertion.
>> + *
>> + * The manual mentions "write precharge cycles" and "precharge cycles".
>> + * We have not been able to figure out which bit fields these correspond to
>> + * in the hardware, or what valid values exist. The current hypothesis is that
>> + * this is something just used on the FAST chip selects. There is also a "byte
>> + * device enable" flag somewhere for 8bit memories.
>> + */
>> +#define EBI2_XMEM_CS0_FAST_CFG 0x0028 /* GUESS */
>> +#define EBI2_XMEM_CS1_FAST_CFG 0x002C /* GUESS */
>> +#define EBI2_XMEM_CS2_FAST_CFG 0x0030 /* GUESS */
>> +#define EBI2_XMEM_CS3_FAST_CFG 0x0034
>> +#define EBI2_XMEM_CS4_FAST_CFG 0x0038 /* GUESS */
>> +#define EBI2_XMEM_CS5_FAST_CFG 0x003C /* GUESS */
>
> Guesses are correct.

Thanks, feel more secure. You don't happen to know about the
uses of bits 31-28 or bits 18-24 in these registers?

>> +     clk = devm_clk_get(dev, "ebi2x");
>> +     if (IS_ERR(clk)) {
>> +             dev_err(dev, "could not get EBI2X clk (%li)\n", PTR_ERR(clk));
>> +             return PTR_ERR(clk);
>
> This could be noisy on probe defer...

OK skipping it.

>> +     ret = clk_prepare_enable(clk);
>> +     if (ret) {
>> +             dev_err(dev, "could not enable EBI2 clk\n");
>> +             return ret;
>
> clk_disable_unprepare ebi2x clk? Or perhaps get both clks first and then
> try to enable both in succession.

OK will handle the errorpath better.

>> +             dev_info(dev, "enabled CS%u\n", csindex);
>
> dev_dbg?

OK. Once everything works I will take a swep and dev_dbg()
a bunch of stuff.

>> +             if (slowcfg)
>> +                     writel_relaxed(slowcfg, ebi2_xmem + csd->slow_cfg);
>> +             if (fastcfg)
>> +                     writel_relaxed(fastcfg, ebi2_xmem + csd->fast_cfg);
>
> The documentation just speaks of config0 and config1, but I guess if
> slow and fast is meaningful to you then it's ok.

That comes from the very similar hardware
Cypress AN49576 Antioch Westbridge
http://www.cypress.com/file/105771/download
they call then "Fast_CSn_CFG0" and "Slow_CSn_CFG1"
respectively.

Yours,
Linus Walleij

[PATCH 2/4] soc: qcom: add EBI2 driver

[Lothar Waßmann]

Hi,

On Mon,  8 Aug 2016 23:24:03 +0200 Linus Walleij wrote:
> This adds a driver for the Qualcomm External Bus Interface EBI2
> found in the MSM8660 and APQ8060 SoCs (at least).
> 
> This was tested with the SMSC9112 ethernet on the APQ8060
> Dragonboard sitting on top of the SLOW CS2.
> 
> Some of my understanding if very vague and based on guesses and
> extrapolations: the documentation in APQ8060 Qualcomm Application
> Processor User Guide 80-N7150-14 Rev. A describes select features but
> does not document the register bit fields.
> 
> However I had to do something like this to provide a serious driver,
> I have marked the guesses with /* GUESS */
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/soc/qcom/Kconfig  |   8 +
>  drivers/soc/qcom/Makefile |   1 +
>  drivers/soc/qcom/ebi2.c   | 371 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 380 insertions(+)
>  create mode 100644 drivers/soc/qcom/ebi2.c
> 
> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> index 461b387d03cc..1f56baa30807 100644
> --- a/drivers/soc/qcom/Kconfig
> +++ b/drivers/soc/qcom/Kconfig
> @@ -76,3 +76,11 @@ config QCOM_WCNSS_CTRL
>  	help
>  	  Client driver for the WCNSS_CTRL SMD channel, used to download nv
>  	  firmware to a newly booted WCNSS chip.
> +
> +config QCOM_EBI2
> +	bool "Qualcomm External Bus Interface 2 (EBI2)"
> +	default y if ARCH_MSM8X60
> +	help
> +	  Say y here to enable support for the Qualcomm External Bus
> +	  Interface 2, which can be used to connect things like NAND Flash,
> +	  SRAM, ethernet adapters, FPGAs and LCD displays.
> diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
> index fdd664edf0bd..8d5a8d738d0a 100644
> --- a/drivers/soc/qcom/Makefile
> +++ b/drivers/soc/qcom/Makefile
> @@ -7,3 +7,4 @@ obj-$(CONFIG_QCOM_SMEM_STATE) += smem_state.o
>  obj-$(CONFIG_QCOM_SMP2P)	+= smp2p.o
>  obj-$(CONFIG_QCOM_SMSM)	+= smsm.o
>  obj-$(CONFIG_QCOM_WCNSS_CTRL) += wcnss_ctrl.o
> +obj-$(CONFIG_QCOM_EBI2)	+= ebi2.o
> diff --git a/drivers/soc/qcom/ebi2.c b/drivers/soc/qcom/ebi2.c
> new file mode 100644
> index 000000000000..f90c5d13fb5a
> --- /dev/null
> +++ b/drivers/soc/qcom/ebi2.c
> @@ -0,0 +1,371 @@
> +/*
> + * Qualcomm External Bus Interface 2 (EBI2) driver
> + * an older version of the Qualcomm Parallel Interface Controller (QPIC)
> + *
> + * Copyright (C) 2016 Linaro Ltd.
> + *
> + * Author: Linus Walleij <linus.walleij@linaro.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2, as
> + * published by the Free Software Foundation.
> + *
> + * See the device tree bindings for this block for more details on the
> + * hardware.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/slab.h>
> +#include <linux/platform_device.h>
> +#include <linux/bitops.h>
> +
> +/* Guessed bit placement CS2 and CS3 are certain */
> +/* What about CS1A, CS1B, CS2A, CS2B? */
> +#define EBI2_CS0_ENABLE BIT(2) /* GUESS */
> +#define EBI2_CS1_ENABLE BIT(3) /* GUESS */
> +#define EBI2_CS2_ENABLE BIT(4)
> +#define EBI2_CS3_ENABLE BIT(5)
> +#define EBI2_CS4_ENABLE BIT(6) /* GUESS */
> +#define EBI2_CS5_ENABLE BIT(7) /* GUESS */
> +#define EBI2_CSN_MASK BIT(2)|BIT(3)|BIT(4)|BIT(5)|BIT(6)|BIT(7)
> +
missing '()' around expression, otherwise...

> +	csval &= ~EBI2_CSN_MASK;
...this will be '~BIT(2) | BIT(3) | BIT(4) | BIT(5) | BIT(6) | BIT(7)'
which is most likely NOT what's intended.

(Also spaces around the '|' operator would improve readability)


Lothar Wa?mann

[PATCH 2/4] soc: qcom: add EBI2 driver

[Linus Walleij]

On Thu, Aug 18, 2016 at 2:07 PM, Lothar Wa?mann <LW@karo-electronics.de> wrote:

>> +#define EBI2_CSN_MASK BIT(2)|BIT(3)|BIT(4)|BIT(5)|BIT(6)|BIT(7)
>> +
> missing '()' around expression, otherwise...
>
>> +     csval &= ~EBI2_CSN_MASK;
> ...this will be '~BIT(2) | BIT(3) | BIT(4) | BIT(5) | BIT(6) | BIT(7)'
> which is most likely NOT what's intended.
>
> (Also spaces around the '|' operator would improve readability)

Sorry for not mentioning in my latest v2 revision: I solved this
by using GENMASK(0,9) (yeah it also turns out to be more
than 8 bits of CS...)

Yours,
Linus Walleij

[PATCH 3/4] ARM: dts: add EBI2 to the Qualcomm MSM8660 DTSI

[Linus Walleij]

This adds the external bus interface EBI2 to the MSM8660 device
tree, albeit with status = "disabled" so that devices actually
using EBI2 can turn it on if needed.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 arch/arm/boot/dts/qcom-msm8660.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-msm8660.dtsi b/arch/arm/boot/dts/qcom-msm8660.dtsi
index 8c65e0d82559..e1cbc9d2f3f3 100644
--- a/arch/arm/boot/dts/qcom-msm8660.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8660.dtsi
@@ -141,6 +141,18 @@
 			};
 		};
 
+		ebi2 at 1a100000 {
+			compatible = "qcom,ebi2";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges;
+			reg = <0x1a100000 0x1000>, <0x1a110000 0x1000>;
+			reg-names = "ebi2", "xmem";
+			clocks = <&gcc EBI2_2X_CLK>, <&gcc EBI2_CLK>;
+			clock-names = "ebi2x", "ebi2";
+			status = "disabled";
+		};
+
 		qcom,ssbi at 500000 {
 			compatible = "qcom,ssbi";
 			reg = <0x500000 0x1000>;
-- 
2.7.4

[PATCH 4/4] ARM: dts: add SMSC ethernet on the APQ8060 Dragonboard

[Linus Walleij]

The SMSC9112 ethernet controller is connected to chip select 2
on the EBI2 bus on the APQ8060 Dragonboard. We set this up by
activating EBI2, creating a chipselect entry as a subnode, and then
putting the ethernet controller in a subnode of the chipselect.

After the chipselect is configured, the SMSC device will be
instantiated.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 arch/arm/boot/dts/qcom-apq8060-dragonboard.dts | 130 +++++++++++++++++++++++++
 1 file changed, 130 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-apq8060-dragonboard.dts b/arch/arm/boot/dts/qcom-apq8060-dragonboard.dts
index 0abc93e5bb00..3604608afe1e 100644
--- a/arch/arm/boot/dts/qcom-apq8060-dragonboard.dts
+++ b/arch/arm/boot/dts/qcom-apq8060-dragonboard.dts
@@ -51,6 +51,29 @@
 			regulator-boot-on;
 		};
 
+		/* GPIO controlled ethernet power regulator */
+		dragon_veth: xc622a331mrg {
+			compatible = "regulator-fixed";
+			regulator-name = "XC6222A331MR-G";
+			regulator-min-microvolt = <3300000>;
+			regulator-max-microvolt = <3300000>;
+			vin-supply = <&vph>;
+			gpio = <&pm8058_gpio 40 GPIO_ACTIVE_HIGH>;
+			enable-active-high;
+			pinctrl-names = "default";
+			pinctrl-0 = <&dragon_veth_gpios>;
+			regulator-always-on;
+		};
+
+		/* VDDvario fixed regulator */
+		dragon_vario: nds332p {
+			compatible = "regulator-fixed";
+			regulator-name = "NDS332P";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+			vin-supply = <&pm8058_s3>;
+		};
+
 		/* This is a levelshifter for SDCC5 */
 		dragon_vio_txb: txb0104rgyr {
 			compatible = "regulator-fixed";
@@ -167,6 +190,41 @@
 					bias-pull-up;
 				};
 			};
+
+			dragon_ebi2_pins: ebi2 {
+				/*
+				 * Pins used by EBI2 on the Dragonboard, actually only
+				 * only CS2 is used by a real peripheral. CS0 is just
+				 * routed to a test point.
+				 */
+				mux0 {
+					/*
+					 * Pins used by EBI2 on the Dragonboard, actually only
+					 * only CS2 is used by a real peripheral. CS0 is just
+					 * routed to a test point.
+					 */
+					pins =
+					    /* "gpio39", CS1A_N this is not good to mux */
+					    "gpio40", /* CS2A_N */
+					    "gpio134"; /* CS0_N testpoint TP29 */
+					function = "ebi2cs";
+				};
+				mux1 {
+					pins =
+					    /* EBI2_ADDR_7 downto EBI2_ADDR_0 address bus */
+					    "gpio123", "gpio124", "gpio125", "gpio126",
+					    "gpio127", "gpio128", "gpio129", "gpio130",
+					    /* EBI2_DATA_15 downto EBI2_DATA_0 data bus */
+					    "gpio135", "gpio136", "gpio137", "gpio138",
+					    "gpio139", "gpio140", "gpio141", "gpio142",
+					    "gpio143", "gpio144", "gpio145", "gpio146",
+					    "gpio147", "gpio148", "gpio149", "gpio150",
+					    "gpio151", /* EBI2_OE_N */
+					    "gpio153", /* EBI2_ADV */
+					    "gpio157"; /* EBI2_WE_N */
+					function = "ebi2";
+				};
+			};
 		};
 
 		qcom,ssbi at 500000 {
@@ -201,6 +259,15 @@
 				};
 
 				gpio at 150 {
+					dragon_ethernet_gpios: ethernet-gpios {
+						pinconf {
+							pins = "gpio7";
+							function = "normal";
+							input-enable;
+							bias-disable;
+							power-source = <PM8058_GPIO_S3>;
+						};
+					};
 					dragon_bmp085_gpios: bmp085-gpios {
 						pinconf {
 							pins = "gpio16";
@@ -238,6 +305,15 @@
 							power-source = <PM8058_GPIO_S3>;
 						};
 					};
+					dragon_veth_gpios: veth-gpios {
+						pinconf {
+							pins = "gpio40";
+							function = "normal";
+							bias-disable;
+							drive-push-pull;
+							// power-source = <PM8058_GPIO_S3>;
+						};
+					};
 				};
 			};
 		};
@@ -283,6 +359,60 @@
 			};
 		};
 
+		ebi2 at 1a100000 {
+			/* The EBI2 will instantiate first, then populate its children */
+			status = "ok";
+			pinctrl-names = "default";
+			pinctrl-0 = <&dragon_ebi2_pins>;
+
+			cs2 at 1b800000 {
+				chipselect = <2>;
+				#address-cells = <1>;
+				#size-cells = <1>;
+				ranges;
+				/*
+				 * SLOW chipselect config
+				 * Delay 9 cycles (140ns at 64MHz) between SMSC LAN9221
+				 * Ethernet controller reads and writes.
+				 */
+				xmem-recovery-cycles = <0>;
+				xmem-write-hold-cycles = <3>;
+				xmem-write-delta-cycles = <31>;
+				xmem-read-delta-cycles = <28>;
+				xmem-write-wait-cycles = <9>;
+				xmem-read-wait-cycles = <9>;
+
+				/*
+				 * An on-board SMSC LAN9221 chip for "debug ethernet",
+				 * which is actually just an ordinary ethernet on the
+				 * EBI2. This has a 25MHz chrystal next to it, so no
+				 * clocking is needed.
+				 */
+				ethernet-ebi2 at 1b800000 {
+					compatible = "smsc,lan9221", "smsc,lan9115";
+					reg = <0x1b800000 0x100>;
+					/*
+					 * GPIO7 has interrupt 198 on the PM8058
+					 * The second interrupt is the PME interrupt
+					 * for network wakeup, connected to the TLMM.
+					 */
+					interrupts-extended = <&pmicintc 198 IRQ_TYPE_EDGE_FALLING>,
+							    <&tlmm 29 IRQ_TYPE_EDGE_RISING>;
+					reset-gpios = <&tlmm 30 GPIO_ACTIVE_LOW>;
+					vdd33a-supply = <&dragon_veth>;
+					vddvario-supply = <&dragon_vario>;
+					pinctrl-names = "default";
+					pinctrl-0 = <&dragon_ethernet_gpios>;
+					phy-mode = "mii";
+					reg-io-width = <2>;
+					smsc,force-external-phy;
+					/* IRQ on edge falling = active low */
+					smsc,irq-active-low;
+					smsc,irq-push-pull;
+				};
+			};
+		};
+
 		rpm at 104000 {
 			/*
 			 * Set up of the PMIC RPM regulators for this board
-- 
2.7.4