Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1469449 > unrolled thread
| Started by | Mirza Krak <mirza.krak@gmail.com> |
|---|---|
| First post | 2016-08-24 15:40 +0200 |
| Last post | 2016-08-31 11:30 +0200 |
| Articles | 15 — 4 participants |
Back to article view | Back to linux.kernel
[PATCH v2 0/6] Add support for Tegra GMI bus controller Mirza Krak <mirza.krak@gmail.com> - 2016-08-24 15:40 +0200
[PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller Mirza Krak <mirza.krak@gmail.com> - 2016-08-24 15:40 +0200
Re: [PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller Jon Hunter <jonathanh@nvidia.com> - 2016-08-24 18:00 +0200
Re: [PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller Mirza Krak <mirza.krak@gmail.com> - 2016-08-24 22:00 +0200
Re: [PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller Mirza Krak <mirza.krak@gmail.com> - 2016-08-26 07:00 +0200
Re: [PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller Jon Hunter <jonathanh@nvidia.com> - 2016-08-26 09:30 +0200
Re: [PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller Mirza Krak <mirza.krak@gmail.com> - 2016-08-29 09:40 +0200
Re: [PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller Rob Herring <robh@kernel.org> - 2016-08-30 19:10 +0200
Re: [PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller Mirza Krak <mirza.krak@gmail.com> - 2016-08-31 13:30 +0200
Re: [PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller Jon Hunter <jonathanh@nvidia.com> - 2016-09-06 12:40 +0200
Re: [PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller Jon Hunter <jonathanh@nvidia.com> - 2016-09-06 12:40 +0200
Re: [PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller Marcel Ziswiler <marcel.ziswiler@toradex.com> - 2016-08-31 08:40 +0200
Re: [PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller Mirza Krak <mirza.krak@gmail.com> - 2016-08-31 11:30 +0200
Re: [PATCH v2 0/6] Add support for Tegra GMI bus controller Marcel Ziswiler <marcel.ziswiler@toradex.com> - 2016-08-30 17:10 +0200
Re: [PATCH v2 0/6] Add support for Tegra GMI bus controller Mirza Krak <mirza.krak@gmail.com> - 2016-08-31 11:30 +0200
| From | Mirza Krak <mirza.krak@gmail.com> |
|---|---|
| Date | 2016-08-24 15:40 +0200 |
| Subject | [PATCH v2 0/6] Add support for Tegra GMI bus controller |
| Message-ID | <s9Gzo-2Q5-5@gated-at.bofh.it> |
From: Mirza Krak <mirza.krak@gmail.com> Hi. This is a follow up to my previous RFC to add support for Tegra GMI bus controller. I have tested this series on a Tegra30 using a Colibri T30 SOM on a custom carrier board which has multiple CAN controllers (SJA1000) connected to the GMI bus. I have rebased on top of latest tegra/for-next in V2. Also see individual patches for changes in V2. See below links for previous discussions. Comments on RFC: https://marc.info/?l=linux-clk&m=146893557629903&w=2 https://marc.info/?l=linux-tegra&m=146893541829801&w=2 https://marc.info/?l=linux-tegra&m=146893542429814&w=2 Comments on V1: https://marc.info/?l=linux-arm-kernel&m=147051551821122&w=2 https://marc.info/?l=linux-arm-kernel&m=147051553121150&w=2 https://marc.info/?l=linux-arm-kernel&m=147194856600627&w=2 https://marc.info/?l=linux-arm-kernel&m=147072742432211&w=2 Mirza Krak (6): clk: tegra: add TEGRA20_CLK_NOR to init table clk: tegra: add TEGRA30_CLK_NOR to init table dt/bindings: Add bindings for Tegra GMI controller ARM: tegra: Add Tegra30 GMI support ARM: tegra: Add Tegra20 GMI support bus: Add support for Tegra Generic Memory Interface .../devicetree/bindings/bus/nvidia,tegra20-gmi.txt | 132 ++++++++++++ arch/arm/boot/dts/tegra20.dtsi | 13 ++ arch/arm/boot/dts/tegra30.dtsi | 12 ++ drivers/bus/Kconfig | 8 + drivers/bus/Makefile | 1 + drivers/bus/tegra-gmi.c | 231 +++++++++++++++++++++ drivers/clk/tegra/clk-tegra20.c | 1 + drivers/clk/tegra/clk-tegra30.c | 1 + 8 files changed, 399 insertions(+) create mode 100644 Documentation/devicetree/bindings/bus/nvidia,tegra20-gmi.txt create mode 100644 drivers/bus/tegra-gmi.c -- 2.1.4
[toc] | [next] | [standalone]
| From | Mirza Krak <mirza.krak@gmail.com> |
|---|---|
| Date | 2016-08-24 15:40 +0200 |
| Subject | [PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller |
| Message-ID | <s9Gzo-2Q5-21@gated-at.bofh.it> |
| In reply to | #1469449 |
From: Mirza Krak <mirza.krak@gmail.com>
Document the devicetree bindings for the Generic Memory Interface (GMI)
bus driver found on Tegra SOCs.
Signed-off-by: Mirza Krak <mirza.krak@gmail.com>
---
Changes in v2:
- Updated examples and some information based on comments from Jon Hunter.
.../devicetree/bindings/bus/nvidia,tegra20-gmi.txt | 132 +++++++++++++++++++++
1 file changed, 132 insertions(+)
create mode 100644 Documentation/devicetree/bindings/bus/nvidia,tegra20-gmi.txt
diff --git a/Documentation/devicetree/bindings/bus/nvidia,tegra20-gmi.txt b/Documentation/devicetree/bindings/bus/nvidia,tegra20-gmi.txt
new file mode 100644
index 0000000..8c1e15f
--- /dev/null
+++ b/Documentation/devicetree/bindings/bus/nvidia,tegra20-gmi.txt
@@ -0,0 +1,132 @@
+Device tree bindings for NVIDIA Tegra Generic Memory Interface bus
+
+The Generic Memory Interface bus enables memory transfers between internal and
+external memory. Can be used to attach various high speed devices such as
+synchronous/asynchronous NOR, FPGA, UARTS and more.
+
+The actual devices are instantiated from the child nodes of a GMI node.
+
+Required properties:
+ - compatible : Should contain one of the following:
+ For Tegra20 must contain "nvidia,tegra20-gmi".
+ For Tegra30 must contain "nvidia,tegra30-gmi".
+ - reg: Should contain GMI controller registers location and length.
+ - clocks: Must contain an entry for each entry in clock-names.
+ - clock-names: Must include the following entries: "gmi"
+ - resets : Must contain an entry for each entry in reset-names.
+ - reset-names : Must include the following entries: "gmi"
+ - #address-cells: The number of cells used to represent physical base
+ addresses in the GMI address space. Should be 1.
+ - #size-cells: The number of cells used to represent the size of an address
+ range in the GMI address space. Should be 1.
+ - ranges: Must be set up to reflect the memory layout with three integer values
+ for each chip-select line in use (only one entry is supported, see below
+ comments):
+ <cs-number> <physical address of mapping> <size>
+
+Note that the GMI controller does not have any internal chip-select address
+decoding, because of that chip-selects either need to be managed via software
+or by employing external chip-select decoding logic.
+
+If external chip-select logic is used to support multiple devices it is assumed
+that the devices use the same timing and so are probably the same type. It also
+assumes that they can fit in the 256MB address range. In this case only one
+child device is supported which represents the active chip-select line, see
+examples for more insight.
+
+Required child cs node properties:
+ - reg: First entry should contain the active chip-select number
+
+Optional child cs node properties:
+
+ - nvidia,snor-data-width-32bit: Use 32bit data-bus, default is 16bit.
+ - nvidia,snor-mux-mode: Enable address/data MUX mode.
+ - nvidia,snor-rdy-active-before-data: Assert RDY signal one cycle before data.
+ If omitted it will be asserted with data.
+ - nvidia,snor-rdy-inv: RDY signal is active high
+ - nvidia,snor-adv-inv: ADV signal is active high
+ - nvidia,snor-oe-inv: WE/OE signal is active high
+ - nvidia,snor-cs-inv: CS signal is active high
+
+ Note that there is some special handling for the timing values.
+ From Tegra TRM:
+ Programming 0 means 1 clock cycle: actual cycle = programmed cycle + 1
+
+ - nvidia,snor-muxed-width: Number of cycles MUX address/data asserted on the
+ bus. Valid values are 0-15, default is 1
+ - nvidia,snor-hold-width: Number of cycles CE stays asserted after the
+ de-assertion of WR_N (in case of SLAVE/MASTER Request) or OE_N
+ (in case of MASTER Request). Valid values are 0-15, default is 1
+ - nvidia,snor-adv-width: Number of cycles during which ADV stays asserted.
+ Valid values are 0-15, default is 1.
+ - nvidia,snor-ce-width: Number of cycles before CE is asserted.
+ Valid values are 0-15, default is 4
+ - nvidia,snor-we-width: Number of cycles during which WE stays asserted.
+ Valid values are 0-15, default is 1
+ - nvidia,snor-oe-width: Number of cycles during which OE stays asserted.
+ Valid values are 0-255, default is 1
+ - nvidia,snor-wait-width: Number of cycles before READY is asserted.
+ Valid values are 0-255, default is 3
+
+Example with two SJA1000 CAN controllers connected to the GMI bus. We wrap the
+controllers with a simple-bus node since they are all connected to the same
+chip-select (CS4), in this example external address decoding is provided:
+
+gmi@70090000 {
+ compatible = "nvidia,tegra20-gmi";
+ reg = <0x70009000 0x1000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ clocks = <&tegra_car TEGRA20_CLK_NOR>;
+ clock-names = "gmi";
+ resets = <&tegra_car 42>;
+ reset-names = "gmi";
+ ranges = <4 0x48000000 0x7ffffff>;
+
+ status = "disabled";
+
+ bus@4 {
+ compatible = "simple-bus";
+ reg = <4>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 4 0x40100>;
+
+ nvidia,snor-mux-mode;
+ nvidia,snor-adv-inv;
+
+ can@0 {
+ reg = <0 0x100>;
+ ...
+ };
+
+ can@40000 {
+ reg = <0x40000 0x100>;
+ ...
+ };
+ };
+};
+
+Example with one SJA1000 CAN controller connected to the GMI bus
+on CS4:
+
+gmi@70090000 {
+ compatible = "nvidia,tegra20-gmi";
+ reg = <0x70009000 0x1000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ clocks = <&tegra_car TEGRA20_CLK_NOR>;
+ clock-names = "gmi";
+ resets = <&tegra_car 42>;
+ reset-names = "gmi";
+ ranges = <4 0x48000000 0x7ffffff>;
+
+ status = "disabled";
+
+ can@4 {
+ reg = <4 0x100>;
+ ...
+ nvidia,snor-mux-mode;
+ nvidia,snor-adv-inv;
+ };
+};
--
2.1.4
[toc] | [prev] | [next] | [standalone]
| From | Jon Hunter <jonathanh@nvidia.com> |
|---|---|
| Date | 2016-08-24 18:00 +0200 |
| Subject | Re: [PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller |
| Message-ID | <s9IKR-4jv-21@gated-at.bofh.it> |
| In reply to | #1469454 |
On 24/08/16 14:37, Mirza Krak wrote:
> From: Mirza Krak <mirza.krak@gmail.com>
>
> Document the devicetree bindings for the Generic Memory Interface (GMI)
> bus driver found on Tegra SOCs.
>
> Signed-off-by: Mirza Krak <mirza.krak@gmail.com>
> ---
> Changes in v2:
> - Updated examples and some information based on comments from Jon Hunter.
>
> .../devicetree/bindings/bus/nvidia,tegra20-gmi.txt | 132 +++++++++++++++++++++
> 1 file changed, 132 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/bus/nvidia,tegra20-gmi.txt
>
> diff --git a/Documentation/devicetree/bindings/bus/nvidia,tegra20-gmi.txt b/Documentation/devicetree/bindings/bus/nvidia,tegra20-gmi.txt
> new file mode 100644
> index 0000000..8c1e15f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/bus/nvidia,tegra20-gmi.txt
> @@ -0,0 +1,132 @@
> +Device tree bindings for NVIDIA Tegra Generic Memory Interface bus
> +
> +The Generic Memory Interface bus enables memory transfers between internal and
> +external memory. Can be used to attach various high speed devices such as
> +synchronous/asynchronous NOR, FPGA, UARTS and more.
> +
> +The actual devices are instantiated from the child nodes of a GMI node.
> +
> +Required properties:
> + - compatible : Should contain one of the following:
> + For Tegra20 must contain "nvidia,tegra20-gmi".
> + For Tegra30 must contain "nvidia,tegra30-gmi".
> + - reg: Should contain GMI controller registers location and length.
> + - clocks: Must contain an entry for each entry in clock-names.
> + - clock-names: Must include the following entries: "gmi"
> + - resets : Must contain an entry for each entry in reset-names.
> + - reset-names : Must include the following entries: "gmi"
> + - #address-cells: The number of cells used to represent physical base
> + addresses in the GMI address space. Should be 1.
> + - #size-cells: The number of cells used to represent the size of an address
> + range in the GMI address space. Should be 1.
> + - ranges: Must be set up to reflect the memory layout with three integer values
> + for each chip-select line in use (only one entry is supported, see below
> + comments):
> + <cs-number> <physical address of mapping> <size>
> +
> +Note that the GMI controller does not have any internal chip-select address
> +decoding, because of that chip-selects either need to be managed via software
> +or by employing external chip-select decoding logic.
> +
> +If external chip-select logic is used to support multiple devices it is assumed
> +that the devices use the same timing and so are probably the same type. It also
> +assumes that they can fit in the 256MB address range. In this case only one
> +child device is supported which represents the active chip-select line, see
> +examples for more insight.
> +
> +Required child cs node properties:
> + - reg: First entry should contain the active chip-select number
> +
> +Optional child cs node properties:
> +
> + - nvidia,snor-data-width-32bit: Use 32bit data-bus, default is 16bit.
> + - nvidia,snor-mux-mode: Enable address/data MUX mode.
> + - nvidia,snor-rdy-active-before-data: Assert RDY signal one cycle before data.
> + If omitted it will be asserted with data.
> + - nvidia,snor-rdy-inv: RDY signal is active high
> + - nvidia,snor-adv-inv: ADV signal is active high
> + - nvidia,snor-oe-inv: WE/OE signal is active high
> + - nvidia,snor-cs-inv: CS signal is active high
> +
> + Note that there is some special handling for the timing values.
> + From Tegra TRM:
> + Programming 0 means 1 clock cycle: actual cycle = programmed cycle + 1
> +
> + - nvidia,snor-muxed-width: Number of cycles MUX address/data asserted on the
> + bus. Valid values are 0-15, default is 1
> + - nvidia,snor-hold-width: Number of cycles CE stays asserted after the
> + de-assertion of WR_N (in case of SLAVE/MASTER Request) or OE_N
> + (in case of MASTER Request). Valid values are 0-15, default is 1
> + - nvidia,snor-adv-width: Number of cycles during which ADV stays asserted.
> + Valid values are 0-15, default is 1.
> + - nvidia,snor-ce-width: Number of cycles before CE is asserted.
> + Valid values are 0-15, default is 4
> + - nvidia,snor-we-width: Number of cycles during which WE stays asserted.
> + Valid values are 0-15, default is 1
> + - nvidia,snor-oe-width: Number of cycles during which OE stays asserted.
> + Valid values are 0-255, default is 1
> + - nvidia,snor-wait-width: Number of cycles before READY is asserted.
> + Valid values are 0-255, default is 3
> +
> +Example with two SJA1000 CAN controllers connected to the GMI bus. We wrap the
> +controllers with a simple-bus node since they are all connected to the same
> +chip-select (CS4), in this example external address decoding is provided:
> +
> +gmi@70090000 {
> + compatible = "nvidia,tegra20-gmi";
> + reg = <0x70009000 0x1000>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + clocks = <&tegra_car TEGRA20_CLK_NOR>;
> + clock-names = "gmi";
> + resets = <&tegra_car 42>;
> + reset-names = "gmi";
> + ranges = <4 0x48000000 0x7ffffff>;
> +
> + status = "disabled";
> +
> + bus@4 {
> + compatible = "simple-bus";
> + reg = <4>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0 4 0x40100>;
Does this work? I tried to add an example like this and I got ...
Warning (reg_format): "reg" property in /gmi@70009000/bus@4 has invalid
length (4 bytes) (#address-cells == 1, #size-cells == 1)
I am wondering if we should just following the arm,pl172 example and
have ...
cs4 {
compatible = "simple-bus";
#address-cells = <1>;
#size-cells = <1>;
ranges;
nvidia,snor-cs = <4>;
nvidia,snor-mux-mode;
nvidia,snor-adv-inv;
can@0 {
reg = <0 0x100>;
...
};
...
};
Cheers
Jon
--
nvpublic
[toc] | [prev] | [next] | [standalone]
| From | Mirza Krak <mirza.krak@gmail.com> |
|---|---|
| Date | 2016-08-24 22:00 +0200 |
| Subject | Re: [PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller |
| Message-ID | <s9Mv7-6UZ-23@gated-at.bofh.it> |
| In reply to | #1469558 |
2016-08-24 17:56 GMT+02:00 Jon Hunter <jonathanh@nvidia.com>:
+
>> +Example with two SJA1000 CAN controllers connected to the GMI bus. We wrap the
>> +controllers with a simple-bus node since they are all connected to the same
>> +chip-select (CS4), in this example external address decoding is provided:
>> +
>> +gmi@70090000 {
>> + compatible = "nvidia,tegra20-gmi";
>> + reg = <0x70009000 0x1000>;
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + clocks = <&tegra_car TEGRA20_CLK_NOR>;
>> + clock-names = "gmi";
>> + resets = <&tegra_car 42>;
>> + reset-names = "gmi";
>> + ranges = <4 0x48000000 0x7ffffff>;
>> +
>> + status = "disabled";
>> +
>> + bus@4 {
>> + compatible = "simple-bus";
>> + reg = <4>;
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + ranges = <0 4 0x40100>;
>
> Does this work? I tried to add an example like this and I got ...
>
> Warning (reg_format): "reg" property in /gmi@70009000/bus@4 has invalid
> length (4 bytes) (#address-cells == 1, #size-cells == 1)
Shoot, to get rid of the warning it should be
reg = <4 0 >;
But it works either way.
>
> I am wondering if we should just following the arm,pl172 example and
> have ...
>
> cs4 {
> compatible = "simple-bus";
> #address-cells = <1>;
> #size-cells = <1>;
> ranges;
>
> nvidia,snor-cs = <4>;
> nvidia,snor-mux-mode;
> nvidia,snor-adv-inv;
>
> can@0 {
> reg = <0 0x100>;
> ...
> };
>
> ...
> };
>
That means to go back to V1 really (almost :)). Which I do not mind.
Will give it a test run.
But I am a little hesitant if will be any better/cleaner. In your example above:
can@0 {
reg = <0 0x100>;
...
};
Would this really translate correctly? In the pl172 example they have
multiple ranges and address with "flash@0,0" which a range defined in
parent node. "can@0" does not have valid match in parent node in our
example. So I probably need add some more logic for it to properly
translate.
I have an idea which is following:
gmi@70090000 {
status = "okay";
#address-cells = <2>;
#size-cells = <1>;
ranges = <4 0 0x48000000 0x00040000>;
cs4 {
compatible = "simple-bus";
#address-cells = <2>;
#size-cells = <1>;
ranges;
nvidia,snor-cs = <4>;
nvidia,snor-mux-mode;
nvidia,snor-adv-inv;
can@0 {
compatible = "nxp,sja1000";
reg = <4 0 0x100>;
...
};
can@40000 {
compatible = "nxp,sja1000";
reg = <4 0x40000 0x100>;
...
};
};
};
Do not know if above will work at all (not able to test at current
location), anyway I will play around with it some more and get back to
you.
Best Regards
Mirza
[toc] | [prev] | [next] | [standalone]
| From | Mirza Krak <mirza.krak@gmail.com> |
|---|---|
| Date | 2016-08-26 07:00 +0200 |
| Subject | Re: [PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller |
| Message-ID | <sahpf-2mA-3@gated-at.bofh.it> |
| In reply to | #1469675 |
2016-08-24 21:54 GMT+02:00 Mirza Krak <mirza.krak@gmail.com>:
> 2016-08-24 17:56 GMT+02:00 Jon Hunter <jonathanh@nvidia.com>:
> +
>>> +Example with two SJA1000 CAN controllers connected to the GMI bus. We wrap the
>>> +controllers with a simple-bus node since they are all connected to the same
>>> +chip-select (CS4), in this example external address decoding is provided:
>>> +
>>> +gmi@70090000 {
>>> + compatible = "nvidia,tegra20-gmi";
>>> + reg = <0x70009000 0x1000>;
>>> + #address-cells = <1>;
>>> + #size-cells = <1>;
>>> + clocks = <&tegra_car TEGRA20_CLK_NOR>;
>>> + clock-names = "gmi";
>>> + resets = <&tegra_car 42>;
>>> + reset-names = "gmi";
>>> + ranges = <4 0x48000000 0x7ffffff>;
>>> +
>>> + status = "disabled";
>>> +
>>> + bus@4 {
>>> + compatible = "simple-bus";
>>> + reg = <4>;
>>> + #address-cells = <1>;
>>> + #size-cells = <1>;
>>> + ranges = <0 4 0x40100>;
>>
>> Does this work? I tried to add an example like this and I got ...
>>
>> Warning (reg_format): "reg" property in /gmi@70009000/bus@4 has invalid
>> length (4 bytes) (#address-cells == 1, #size-cells == 1)
>
> Shoot, to get rid of the warning it should be
>
> reg = <4 0 >;
>
> But it works either way.
>
>>
>> I am wondering if we should just following the arm,pl172 example and
>> have ...
>>
>> cs4 {
>> compatible = "simple-bus";
>> #address-cells = <1>;
>> #size-cells = <1>;
>> ranges;
>>
>> nvidia,snor-cs = <4>;
>> nvidia,snor-mux-mode;
>> nvidia,snor-adv-inv;
>>
>> can@0 {
>> reg = <0 0x100>;
>> ...
>> };
>>
>> ...
>> };
>>
>
> That means to go back to V1 really (almost :)). Which I do not mind.
> Will give it a test run.
>
> But I am a little hesitant if will be any better/cleaner. In your example above:
>
> can@0 {
> reg = <0 0x100>;
> ...
> };
>
> Would this really translate correctly? In the pl172 example they have
> multiple ranges and address with "flash@0,0" which a range defined in
> parent node. "can@0" does not have valid match in parent node in our
> example. So I probably need add some more logic for it to properly
> translate.
>
> I have an idea which is following:
>
> gmi@70090000 {
> status = "okay";
> #address-cells = <2>;
> #size-cells = <1>;
> ranges = <4 0 0x48000000 0x00040000>;
>
> cs4 {
> compatible = "simple-bus";
> #address-cells = <2>;
> #size-cells = <1>;
> ranges;
>
> nvidia,snor-cs = <4>;
> nvidia,snor-mux-mode;
> nvidia,snor-adv-inv;
>
> can@0 {
> compatible = "nxp,sja1000";
> reg = <4 0 0x100>;
> ...
> };
>
>
> can@40000 {
> compatible = "nxp,sja1000";
> reg = <4 0x40000 0x100>;
> ...
> };
> };
> };
>
> Do not know if above will work at all (not able to test at current
> location), anyway I will play around with it some more and get back to
> you.
Gave above a test run and it works like a charm. Are we happy with that?
Best Regards
Mirza
[toc] | [prev] | [next] | [standalone]
| From | Jon Hunter <jonathanh@nvidia.com> |
|---|---|
| Date | 2016-08-26 09:30 +0200 |
| Subject | Re: [PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller |
| Message-ID | <sajKp-3YE-7@gated-at.bofh.it> |
| In reply to | #1470541 |
On 26/08/16 05:53, Mirza Krak wrote:
...
>> I have an idea which is following:
>>
>> gmi@70090000 {
>> status = "okay";
>> #address-cells = <2>;
>> #size-cells = <1>;
>> ranges = <4 0 0x48000000 0x00040000>;
>>
>> cs4 {
>> compatible = "simple-bus";
>> #address-cells = <2>;
>> #size-cells = <1>;
>> ranges;
>>
>> nvidia,snor-cs = <4>;
>> nvidia,snor-mux-mode;
>> nvidia,snor-adv-inv;
>>
>> can@0 {
>> compatible = "nxp,sja1000";
>> reg = <4 0 0x100>;
>> ...
>> };
>>
>>
>> can@40000 {
>> compatible = "nxp,sja1000";
>> reg = <4 0x40000 0x100>;
>> ...
>> };
>> };
>> };
>>
>> Do not know if above will work at all (not able to test at current
>> location), anyway I will play around with it some more and get back to
>> you.
>
> Gave above a test run and it works like a charm. Are we happy with that?
Does it not work with #address-cells = <1>? Seems odd to have the cs in
the reg for the device.
Cheers
Jon
--
nvpublic
[toc] | [prev] | [next] | [standalone]
| From | Mirza Krak <mirza.krak@gmail.com> |
|---|---|
| Date | 2016-08-29 09:40 +0200 |
| Subject | Re: [PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller |
| Message-ID | <sbpkJ-4p3-11@gated-at.bofh.it> |
| In reply to | #1470572 |
2016-08-26 9:25 GMT+02:00 Jon Hunter <jonathanh@nvidia.com>:
>
> On 26/08/16 05:53, Mirza Krak wrote:
>
> ...
>
>>> I have an idea which is following:
>>>
>>> gmi@70090000 {
>>> status = "okay";
>>> #address-cells = <2>;
>>> #size-cells = <1>;
>>> ranges = <4 0 0x48000000 0x00040000>;
>>>
>>> cs4 {
>>> compatible = "simple-bus";
>>> #address-cells = <2>;
>>> #size-cells = <1>;
>>> ranges;
>>>
>>> nvidia,snor-cs = <4>;
>>> nvidia,snor-mux-mode;
>>> nvidia,snor-adv-inv;
>>>
>>> can@0 {
>>> compatible = "nxp,sja1000";
>>> reg = <4 0 0x100>;
>>> ...
>>> };
>>>
>>>
>>> can@40000 {
>>> compatible = "nxp,sja1000";
>>> reg = <4 0x40000 0x100>;
>>> ...
>>> };
>>> };
>>> };
>>>
>>> Do not know if above will work at all (not able to test at current
>>> location), anyway I will play around with it some more and get back to
>>> you.
>>
>> Gave above a test run and it works like a charm. Are we happy with that?
>
> Does it not work with #address-cells = <1>? Seems odd to have the cs in
> the reg for the device.
>
No it does not work with #address-cells = <1> with the current
structure that we have.
With #address-cells = <2>, we can state that this device is on
chip-select 4 and on this specific offset of chip-select 4 (that is
the second address cell). I do not see how we can specify this with
only one address cell.
And is it really that odd? Looking at other drivers they all use the
same metod, even the arm,pl172 that you where referring to as a base
for our implementation.
Best Regards
Mirza
[toc] | [prev] | [next] | [standalone]
| From | Rob Herring <robh@kernel.org> |
|---|---|
| Date | 2016-08-30 19:10 +0200 |
| Subject | Re: [PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller |
| Message-ID | <sbUHT-7Jg-17@gated-at.bofh.it> |
| In reply to | #1469675 |
On Wed, Aug 24, 2016 at 09:54:47PM +0200, Mirza Krak wrote:
> 2016-08-24 17:56 GMT+02:00 Jon Hunter <jonathanh@nvidia.com>:
> +
> >> +Example with two SJA1000 CAN controllers connected to the GMI bus. We wrap the
> >> +controllers with a simple-bus node since they are all connected to the same
> >> +chip-select (CS4), in this example external address decoding is provided:
> >> +
> >> +gmi@70090000 {
> >> + compatible = "nvidia,tegra20-gmi";
> >> + reg = <0x70009000 0x1000>;
> >> + #address-cells = <1>;
> >> + #size-cells = <1>;
> >> + clocks = <&tegra_car TEGRA20_CLK_NOR>;
> >> + clock-names = "gmi";
> >> + resets = <&tegra_car 42>;
> >> + reset-names = "gmi";
> >> + ranges = <4 0x48000000 0x7ffffff>;
> >> +
> >> + status = "disabled";
> >> +
> >> + bus@4 {
> >> + compatible = "simple-bus";
> >> + reg = <4>;
> >> + #address-cells = <1>;
> >> + #size-cells = <1>;
> >> + ranges = <0 4 0x40100>;
> >
> > Does this work? I tried to add an example like this and I got ...
> >
> > Warning (reg_format): "reg" property in /gmi@70009000/bus@4 has invalid
> > length (4 bytes) (#address-cells == 1, #size-cells == 1)
>
> Shoot, to get rid of the warning it should be
>
> reg = <4 0 >;
>
> But it works either way.
The CS node should have #address-cells=2 with the first being CS# and
the second being the offset (often 0).
>
> >
> > I am wondering if we should just following the arm,pl172 example and
> > have ...
> >
> > cs4 {
> > compatible = "simple-bus";
> > #address-cells = <1>;
> > #size-cells = <1>;
> > ranges;
Empty ranges is typically wrong and due to laziness...
This should have the CS# in it.
> >
> > nvidia,snor-cs = <4>;
> > nvidia,snor-mux-mode;
> > nvidia,snor-adv-inv;
> >
> > can@0 {
> > reg = <0 0x100>;
This can be 1 cell with just the offset.
> > ...
> > };
> >
> > ...
> > };
> >
>
> That means to go back to V1 really (almost :)). Which I do not mind.
> Will give it a test run.
>
> But I am a little hesitant if will be any better/cleaner. In your example above:
>
> can@0 {
> reg = <0 0x100>;
> ...
> };
>
> Would this really translate correctly? In the pl172 example they have
> multiple ranges and address with "flash@0,0" which a range defined in
> parent node. "can@0" does not have valid match in parent node in our
> example. So I probably need add some more logic for it to properly
> translate.
pl172 has several things I don't like, so don't follow it. Mainly those
are custom CS property and 3 levels of nodes. I'm fine with 3 levels if
there is more than one device, but otherwise 2 levels with timing
properties in the child device node.
>
> I have an idea which is following:
>
> gmi@70090000 {
> status = "okay";
> #address-cells = <2>;
> #size-cells = <1>;
> ranges = <4 0 0x48000000 0x00040000>;
>
> cs4 {
cs@4,0
> compatible = "simple-bus";
> #address-cells = <2>;
1 cell here.
> #size-cells = <1>;
> ranges;
Fill this in to drop the 2nd cell on child addresses and just have the
offset.
>
> nvidia,snor-cs = <4>;
NAK, no custom CS properties.
> nvidia,snor-mux-mode;
> nvidia,snor-adv-inv;
>
> can@0 {
> compatible = "nxp,sja1000";
> reg = <4 0 0x100>;
> ...
> };
>
>
> can@40000 {
> compatible = "nxp,sja1000";
> reg = <4 0x40000 0x100>;
> ...
> };
> };
> };
>
> Do not know if above will work at all (not able to test at current
> location), anyway I will play around with it some more and get back to
> you.
>
> Best Regards
> Mirza
[toc] | [prev] | [next] | [standalone]
| From | Mirza Krak <mirza.krak@gmail.com> |
|---|---|
| Date | 2016-08-31 13:30 +0200 |
| Subject | Re: [PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller |
| Message-ID | <scbSq-1Nd-35@gated-at.bofh.it> |
| In reply to | #1472651 |
2016-08-30 19:06 GMT+02:00 Rob Herring <robh@kernel.org>:
> On Wed, Aug 24, 2016 at 09:54:47PM +0200, Mirza Krak wrote:
>> 2016-08-24 17:56 GMT+02:00 Jon Hunter <jonathanh@nvidia.com>:
>> +
>> >> +Example with two SJA1000 CAN controllers connected to the GMI bus. We wrap the
>> >> +controllers with a simple-bus node since they are all connected to the same
>> >> +chip-select (CS4), in this example external address decoding is provided:
>> >> +
>> >> +gmi@70090000 {
>> >> + compatible = "nvidia,tegra20-gmi";
>> >> + reg = <0x70009000 0x1000>;
>> >> + #address-cells = <1>;
>> >> + #size-cells = <1>;
>> >> + clocks = <&tegra_car TEGRA20_CLK_NOR>;
>> >> + clock-names = "gmi";
>> >> + resets = <&tegra_car 42>;
>> >> + reset-names = "gmi";
>> >> + ranges = <4 0x48000000 0x7ffffff>;
>> >> +
>> >> + status = "disabled";
>> >> +
>> >> + bus@4 {
>> >> + compatible = "simple-bus";
>> >> + reg = <4>;
>> >> + #address-cells = <1>;
>> >> + #size-cells = <1>;
>> >> + ranges = <0 4 0x40100>;
>> >
>> > Does this work? I tried to add an example like this and I got ...
>> >
>> > Warning (reg_format): "reg" property in /gmi@70009000/bus@4 has invalid
>> > length (4 bytes) (#address-cells == 1, #size-cells == 1)
>>
>> Shoot, to get rid of the warning it should be
>>
>> reg = <4 0 >;
>>
>> But it works either way.
>
> The CS node should have #address-cells=2 with the first being CS# and
> the second being the offset (often 0).
>
>>
>> >
>> > I am wondering if we should just following the arm,pl172 example and
>> > have ...
>> >
>> > cs4 {
>> > compatible = "simple-bus";
>> > #address-cells = <1>;
>> > #size-cells = <1>;
>> > ranges;
>
> Empty ranges is typically wrong and due to laziness...
>
> This should have the CS# in it.
>
>> >
>> > nvidia,snor-cs = <4>;
>> > nvidia,snor-mux-mode;
>> > nvidia,snor-adv-inv;
>> >
>> > can@0 {
>> > reg = <0 0x100>;
>
> This can be 1 cell with just the offset.
>
>> > ...
>> > };
>> >
>> > ...
>> > };
>> >
>>
>> That means to go back to V1 really (almost :)). Which I do not mind.
>> Will give it a test run.
>>
>> But I am a little hesitant if will be any better/cleaner. In your example above:
>>
>> can@0 {
>> reg = <0 0x100>;
>> ...
>> };
>>
>> Would this really translate correctly? In the pl172 example they have
>> multiple ranges and address with "flash@0,0" which a range defined in
>> parent node. "can@0" does not have valid match in parent node in our
>> example. So I probably need add some more logic for it to properly
>> translate.
>
> pl172 has several things I don't like, so don't follow it. Mainly those
> are custom CS property and 3 levels of nodes. I'm fine with 3 levels if
> there is more than one device, but otherwise 2 levels with timing
> properties in the child device node.
>
>
>>
>> I have an idea which is following:
>>
>> gmi@70090000 {
>> status = "okay";
>> #address-cells = <2>;
>> #size-cells = <1>;
>> ranges = <4 0 0x48000000 0x00040000>;
>>
>> cs4 {
>
> cs@4,0
>
>> compatible = "simple-bus";
>> #address-cells = <2>;
>
> 1 cell here.
>
>> #size-cells = <1>;
>> ranges;
>
> Fill this in to drop the 2nd cell on child addresses and just have the
> offset.
>
>>
>> nvidia,snor-cs = <4>;
>
> NAK, no custom CS properties.
>
>> nvidia,snor-mux-mode;
>> nvidia,snor-adv-inv;
>>
>> can@0 {
>> compatible = "nxp,sja1000";
>> reg = <4 0 0x100>;
>> ...
>> };
>>
>>
>> can@40000 {
>> compatible = "nxp,sja1000";
>> reg = <4 0x40000 0x100>;
>> ...
>> };
>> };
>> };
>>
Thank you for your review Rob.
Taking your comments in to account I end up with this:
gmi@70090000 {
compatible = "nvidia,tegra20-gmi";
reg = <0x70009000 0x1000>;
#address-cells = <2>;
#size-cells = <1>;
clocks = <&tegra_car TEGRA20_CLK_NOR>;
clock-names = "gmi";
resets = <&tegra_car 42>;
reset-names = "gmi";
ranges = <4 0 0xd0000000 0xfffffff>;
status = "okay";
bus@4,0 {
compatible = "simple-bus";
#address-cells = <1>;
#size-cells = <1>;
ranges = <0 4 0 0x40000>;
nvidia,snor-mux-mode;
nvidia,snor-adv-inv;
can@0 {
reg = <0 0x100>;
...
};
can@40000 {
reg = <0x40000 0x100>;
...
};
};
};
Have I understood you correct?
Also wanted to verify the example case where you only have on device
connected to one CS#, from what I see in other implementations it
seems OK to put the CS# in the reg property in that case. Is this
correct?
Example with one SJA1000 CAN controller connected to the GMI bus
on CS4:
gmi@70090000 {
compatible = "nvidia,tegra20-gmi";
reg = <0x70009000 0x1000>;
#address-cells = <2>;
#size-cells = <1>;
clocks = <&tegra_car TEGRA20_CLK_NOR>;
clock-names = "gmi";
resets = <&tegra_car 42>;
reset-names = "gmi";
ranges = <4 0 0xd0000000 0xfffffff>;
status = "okay";
can@4,0 {
reg = <4 0 0x100>;
nvidia,snor-mux-mode;
nvidia,snor-adv-inv;
...
};
};
Jon, to be able to handle both cases in the driver we would first
attempt to decode the CS# from the ranges property, and fallback to
reg property if no ranges are defined. Does that sound reasonable?
Best Regards
Mirza
[toc] | [prev] | [next] | [standalone]
| From | Jon Hunter <jonathanh@nvidia.com> |
|---|---|
| Date | 2016-09-06 12:40 +0200 |
| Subject | Re: [PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller |
| Message-ID | <selXj-PI-13@gated-at.bofh.it> |
| In reply to | #1473339 |
On 31/08/16 12:22, Mirza Krak wrote:
...
> Taking your comments in to account I end up with this:
>
> gmi@70090000 {
> compatible = "nvidia,tegra20-gmi";
> reg = <0x70009000 0x1000>;
> #address-cells = <2>;
Does this need to be 2? Seems simpler if this is 1.
Cheers
Jon
--
nvpublic
[toc] | [prev] | [next] | [standalone]
| From | Jon Hunter <jonathanh@nvidia.com> |
|---|---|
| Date | 2016-09-06 12:40 +0200 |
| Subject | Re: [PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller |
| Message-ID | <selXj-PI-7@gated-at.bofh.it> |
| In reply to | #1473339 |
On 31/08/16 12:22, Mirza Krak wrote:
> 2016-08-30 19:06 GMT+02:00 Rob Herring <robh@kernel.org>:
...
>>> nvidia,snor-cs = <4>;
>>
>> NAK, no custom CS properties.
Ok, so ...
> gmi@70090000 {
> compatible = "nvidia,tegra20-gmi";
> reg = <0x70009000 0x1000>;
> #address-cells = <2>;
> #size-cells = <1>;
> clocks = <&tegra_car TEGRA20_CLK_NOR>;
> clock-names = "gmi";
> resets = <&tegra_car 42>;
> reset-names = "gmi";
> ranges = <4 0 0xd0000000 0xfffffff>;
>
> status = "okay";
>
> bus@4,0 {
> compatible = "simple-bus";
> #address-cells = <1>;
> #size-cells = <1>;
> ranges = <0 4 0 0x40000>;
>
> nvidia,snor-mux-mode;
> nvidia,snor-adv-inv;
>
> can@0 {
> reg = <0 0x100>;
> ...
> };
>
> can@40000 {
> reg = <0x40000 0x100>;
> ...
> };
> };
> };
>
> Have I understood you correct?
>
> Also wanted to verify the example case where you only have on device
> connected to one CS#, from what I see in other implementations it
> seems OK to put the CS# in the reg property in that case. Is this
> correct?
>
> Example with one SJA1000 CAN controller connected to the GMI bus
> on CS4:
>
> gmi@70090000 {
> compatible = "nvidia,tegra20-gmi";
> reg = <0x70009000 0x1000>;
> #address-cells = <2>;
> #size-cells = <1>;
> clocks = <&tegra_car TEGRA20_CLK_NOR>;
> clock-names = "gmi";
> resets = <&tegra_car 42>;
> reset-names = "gmi";
> ranges = <4 0 0xd0000000 0xfffffff>;
>
> status = "okay";
>
> can@4,0 {
> reg = <4 0 0x100>;
> nvidia,snor-mux-mode;
> nvidia,snor-adv-inv;
> ...
> };
> };
>
> Jon, to be able to handle both cases in the driver we would first
> attempt to decode the CS# from the ranges property, and fallback to
> reg property if no ranges are defined. Does that sound reasonable?
Given the above examples that may be supported, is there a
better/simpler way to extract the CS# than what Mirza is proposing? For
example, from the node-name unit-address?
Cheers
Jon
--
nvpublic
[toc] | [prev] | [next] | [standalone]
| From | Marcel Ziswiler <marcel.ziswiler@toradex.com> |
|---|---|
| Date | 2016-08-31 08:40 +0200 |
| Subject | Re: [PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller |
| Message-ID | <sc7lM-7js-27@gated-at.bofh.it> |
| In reply to | #1469454 |
On Wed, 2016-08-24 at 15:37 +0200, Mirza Krak wrote:
> From: Mirza Krak <mirza.krak@gmail.com>
>
> Document the devicetree bindings for the Generic Memory Interface
> (GMI)
> bus driver found on Tegra SOCs.
>
> Signed-off-by: Mirza Krak <mirza.krak@gmail.com>
> ---
> Changes in v2:
> - Updated examples and some information based on comments from Jon
> Hunter.
>
> .../devicetree/bindings/bus/nvidia,tegra20-gmi.txt | 132
> +++++++++++++++++++++
> 1 file changed, 132 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/bus/nvidia,tegra20-gmi.txt
>
> diff --git a/Documentation/devicetree/bindings/bus/nvidia,tegra20-
> gmi.txt b/Documentation/devicetree/bindings/bus/nvidia,tegra20-
> gmi.txt
> new file mode 100644
> index 0000000..8c1e15f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/bus/nvidia,tegra20-gmi.txt
> @@ -0,0 +1,132 @@
> +Device tree bindings for NVIDIA Tegra Generic Memory Interface bus
> +
> +The Generic Memory Interface bus enables memory transfers between
> internal and
> +external memory. Can be used to attach various high speed devices
> such as
> +synchronous/asynchronous NOR, FPGA, UARTS and more.
> +
> +The actual devices are instantiated from the child nodes of a GMI
> node.
> +
> +Required properties:
> + - compatible : Should contain one of the following:
> + For Tegra20 must contain "nvidia,tegra20-gmi".
> + For Tegra30 must contain "nvidia,tegra30-gmi".
> + - reg: Should contain GMI controller registers location and length.
> + - clocks: Must contain an entry for each entry in clock-names.
> + - clock-names: Must include the following entries: "gmi"
> + - resets : Must contain an entry for each entry in reset-names.
> + - reset-names : Must include the following entries: "gmi"
> + - #address-cells: The number of cells used to represent physical
> base
> + addresses in the GMI address space. Should be 1.
> + - #size-cells: The number of cells used to represent the size of an
> address
> + range in the GMI address space. Should be 1.
> + - ranges: Must be set up to reflect the memory layout with three
> integer values
> + for each chip-select line in use (only one entry is supported,
> see below
> + comments):
> + <cs-number> <physical address of mapping> <size>
> +
> +Note that the GMI controller does not have any internal chip-select
> address
> +decoding, because of that chip-selects either need to be managed via
> software
> +or by employing external chip-select decoding logic.
> +
> +If external chip-select logic is used to support multiple devices it
> is assumed
> +that the devices use the same timing and so are probably the same
> type. It also
> +assumes that they can fit in the 256MB address range. In this case
> only one
> +child device is supported which represents the active chip-select
> line, see
> +examples for more insight.
> +
> +Required child cs node properties:
> + - reg: First entry should contain the active chip-select number
> +
> +Optional child cs node properties:
> +
> + - nvidia,snor-data-width-32bit: Use 32bit data-bus, default is
> 16bit.
> + - nvidia,snor-mux-mode: Enable address/data MUX mode.
> + - nvidia,snor-rdy-active-before-data: Assert RDY signal one cycle
> before data.
> + If omitted it will be asserted with data.
> + - nvidia,snor-rdy-inv: RDY signal is active high
> + - nvidia,snor-adv-inv: ADV signal is active high
> + - nvidia,snor-oe-inv: WE/OE signal is active high
> + - nvidia,snor-cs-inv: CS signal is active high
> +
> + Note that there is some special handling for the timing values.
> + From Tegra TRM:
> + Programming 0 means 1 clock cycle: actual cycle = programmed cycle
> + 1
> +
> + - nvidia,snor-muxed-width: Number of cycles MUX address/data
> asserted on the
> + bus. Valid values are 0-15, default is 1
> + - nvidia,snor-hold-width: Number of cycles CE stays asserted after
> the
> + de-assertion of WR_N (in case of SLAVE/MASTER Request) or OE_N
> + (in case of MASTER Request). Valid values are 0-15, default is 1
> + - nvidia,snor-adv-width: Number of cycles during which ADV stays
> asserted.
> + Valid values are 0-15, default is 1.
> + - nvidia,snor-ce-width: Number of cycles before CE is asserted.
> + Valid values are 0-15, default is 4
> + - nvidia,snor-we-width: Number of cycles during which WE stays
> asserted.
> + Valid values are 0-15, default is 1
> + - nvidia,snor-oe-width: Number of cycles during which OE stays
> asserted.
> + Valid values are 0-255, default is 1
> + - nvidia,snor-wait-width: Number of cycles before READY is
> asserted.
> + Valid values are 0-255, default is 3
> +
> +Example with two SJA1000 CAN controllers connected to the GMI bus.
> We wrap the
> +controllers with a simple-bus node since they are all connected to
> the same
> +chip-select (CS4), in this example external address decoding is
> provided:
> +
> +gmi@70090000 {
It's actually 70009000.
> + compatible = "nvidia,tegra20-gmi";
> + reg = <0x70009000 0x1000>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + clocks = <&tegra_car TEGRA20_CLK_NOR>;
> + clock-names = "gmi";
> + resets = <&tegra_car 42>;
> + reset-names = "gmi";
> + ranges = <4 0x48000000 0x7ffffff>;
> +
> + status = "disabled";
I guess in an example one could even set this to okay.
> +
> + bus@4 {
> + compatible = "simple-bus";
> + reg = <4>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0 4 0x40100>;
> +
> + nvidia,snor-mux-mode;
> + nvidia,snor-adv-inv;
> +
> + can@0 {
> + reg = <0 0x100>;
> + ...
> + };
> +
> + can@40000 {
> + reg = <0x40000 0x100>;
> + ...
> + };
> + };
> +};
> +
> +Example with one SJA1000 CAN controller connected to the GMI bus
> +on CS4:
> +
> +gmi@70090000 {
Same here.
> + compatible = "nvidia,tegra20-gmi";
> + reg = <0x70009000 0x1000>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + clocks = <&tegra_car TEGRA20_CLK_NOR>;
> + clock-names = "gmi";
> + resets = <&tegra_car 42>;
> + reset-names = "gmi";
> + ranges = <4 0x48000000 0x7ffffff>;
> +
> + status = "disabled";
Same here.
> +
> + can@4 {
> + reg = <4 0x100>;
> + ...
> + nvidia,snor-mux-mode;
> + nvidia,snor-adv-inv;
> + };
> +};
> --
> 2.1.4
[toc] | [prev] | [next] | [standalone]
| From | Mirza Krak <mirza.krak@gmail.com> |
|---|---|
| Date | 2016-08-31 11:30 +0200 |
| Subject | Re: [PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller |
| Message-ID | <sca0h-B0-19@gated-at.bofh.it> |
| In reply to | #1473063 |
2016-08-30 17:02 GMT+02:00 Marcel Ziswiler <marcel.ziswiler@toradex.com>:
> On Wed, 2016-08-24 at 15:37 +0200, Mirza Krak wrote:
>> From: Mirza Krak <mirza.krak@gmail.com>
>>
>> Document the devicetree bindings for the Generic Memory Interface
>> (GMI)
>> bus driver found on Tegra SOCs.
>>
>> Signed-off-by: Mirza Krak <mirza.krak@gmail.com>
>> ---
>> Changes in v2:
>> - Updated examples and some information based on comments from Jon
>> Hunter.
>>
>> .../devicetree/bindings/bus/nvidia,tegra20-gmi.txt | 132
>> +++++++++++++++++++++
>> 1 file changed, 132 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/bus/nvidia,tegra20-gmi.txt
>>
>> diff --git a/Documentation/devicetree/bindings/bus/nvidia,tegra20-
>> gmi.txt b/Documentation/devicetree/bindings/bus/nvidia,tegra20-
>> gmi.txt
>> new file mode 100644
>> index 0000000..8c1e15f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/bus/nvidia,tegra20-gmi.txt
>> @@ -0,0 +1,132 @@
>> +Device tree bindings for NVIDIA Tegra Generic Memory Interface bus
>> +
>> +The Generic Memory Interface bus enables memory transfers between
>> internal and
>> +external memory. Can be used to attach various high speed devices
>> such as
>> +synchronous/asynchronous NOR, FPGA, UARTS and more.
>> +
>> +The actual devices are instantiated from the child nodes of a GMI
>> node.
>> +
>> +Required properties:
>> + - compatible : Should contain one of the following:
>> + For Tegra20 must contain "nvidia,tegra20-gmi".
>> + For Tegra30 must contain "nvidia,tegra30-gmi".
>> + - reg: Should contain GMI controller registers location and length.
>> + - clocks: Must contain an entry for each entry in clock-names.
>> + - clock-names: Must include the following entries: "gmi"
>> + - resets : Must contain an entry for each entry in reset-names.
>> + - reset-names : Must include the following entries: "gmi"
>> + - #address-cells: The number of cells used to represent physical
>> base
>> + addresses in the GMI address space. Should be 1.
>> + - #size-cells: The number of cells used to represent the size of an
>> address
>> + range in the GMI address space. Should be 1.
>> + - ranges: Must be set up to reflect the memory layout with three
>> integer values
>> + for each chip-select line in use (only one entry is supported,
>> see below
>> + comments):
>> + <cs-number> <physical address of mapping> <size>
>> +
>> +Note that the GMI controller does not have any internal chip-select
>> address
>> +decoding, because of that chip-selects either need to be managed via
>> software
>> +or by employing external chip-select decoding logic.
>> +
>> +If external chip-select logic is used to support multiple devices it
>> is assumed
>> +that the devices use the same timing and so are probably the same
>> type. It also
>> +assumes that they can fit in the 256MB address range. In this case
>> only one
>> +child device is supported which represents the active chip-select
>> line, see
>> +examples for more insight.
>> +
>> +Required child cs node properties:
>> + - reg: First entry should contain the active chip-select number
>> +
>> +Optional child cs node properties:
>> +
>> + - nvidia,snor-data-width-32bit: Use 32bit data-bus, default is
>> 16bit.
>> + - nvidia,snor-mux-mode: Enable address/data MUX mode.
>> + - nvidia,snor-rdy-active-before-data: Assert RDY signal one cycle
>> before data.
>> + If omitted it will be asserted with data.
>> + - nvidia,snor-rdy-inv: RDY signal is active high
>> + - nvidia,snor-adv-inv: ADV signal is active high
>> + - nvidia,snor-oe-inv: WE/OE signal is active high
>> + - nvidia,snor-cs-inv: CS signal is active high
>> +
>> + Note that there is some special handling for the timing values.
>> + From Tegra TRM:
>> + Programming 0 means 1 clock cycle: actual cycle = programmed cycle
>> + 1
>> +
>> + - nvidia,snor-muxed-width: Number of cycles MUX address/data
>> asserted on the
>> + bus. Valid values are 0-15, default is 1
>> + - nvidia,snor-hold-width: Number of cycles CE stays asserted after
>> the
>> + de-assertion of WR_N (in case of SLAVE/MASTER Request) or OE_N
>> + (in case of MASTER Request). Valid values are 0-15, default is 1
>> + - nvidia,snor-adv-width: Number of cycles during which ADV stays
>> asserted.
>> + Valid values are 0-15, default is 1.
>> + - nvidia,snor-ce-width: Number of cycles before CE is asserted.
>> + Valid values are 0-15, default is 4
>> + - nvidia,snor-we-width: Number of cycles during which WE stays
>> asserted.
>> + Valid values are 0-15, default is 1
>> + - nvidia,snor-oe-width: Number of cycles during which OE stays
>> asserted.
>> + Valid values are 0-255, default is 1
>> + - nvidia,snor-wait-width: Number of cycles before READY is
>> asserted.
>> + Valid values are 0-255, default is 3
>> +
>> +Example with two SJA1000 CAN controllers connected to the GMI bus.
>> We wrap the
>> +controllers with a simple-bus node since they are all connected to
>> the same
>> +chip-select (CS4), in this example external address decoding is
>> provided:
>> +
>> +gmi@70090000 {
>
> It's actually 70009000.
I actually noticed this all ready, so should be fixed in the upcoming V3.
Thank you for your review.
Best Regards
Mirza
[toc] | [prev] | [next] | [standalone]
| From | Marcel Ziswiler <marcel.ziswiler@toradex.com> |
|---|---|
| Date | 2016-08-30 17:10 +0200 |
| Message-ID | <sbSPL-6x2-1@gated-at.bofh.it> |
| In reply to | #1469449 |
Hi Mirza Sorry, I long since wanted to give you some feedback on this as well. BTW: Thank you very much for taking this on! On Wed, 2016-08-24 at 15:37 +0200, Mirza Krak wrote: > From: Mirza Krak <mirza.krak@gmail.com> > > Hi. > > This is a follow up to my previous RFC to add support for Tegra GMI > bus > controller. > > I have tested this series on a Tegra30 using a Colibri T30 SOM on a > custom > carrier board which has multiple CAN controllers (SJA1000) connected > to the > GMI bus. We once did a nice GMI-Memory Board which mates with the extension connector X3 of our V3.x Colibri Evaluation boards and allows testing SRAM access not only in muxed but also in non-muxed mode albeit 16-bit only. I took your driver for a spin both on Colibri T20 as well as Colibri T30 both in muxed as well as non-muxed mode and it passed all tests being both manual devmem2 type reads/writes as well as memtester runs on the full 128K SRAM giving it the physical address using the -p argument. So you may add the following to the whole series: Tested-by: Marcel Ziswiler <marcel.ziswiler@toradex.com> Tested-on: Colibri T20/T30 on EvalBoard V3.x and GMI-Memory Board I will leave further comments on the individual patches. BTW: Of course for non-muxed mode I also had to adjust the pin muxing as they default to muxed. > I have rebased on top of latest tegra/for-next in V2. Also see > individual > patches for changes in V2. > > See below links for previous discussions. > > Comments on RFC: > https://marc.info/?l=linux-clk&m=146893557629903&w=2 > https://marc.info/?l=linux-tegra&m=146893541829801&w=2 > https://marc.info/?l=linux-tegra&m=146893542429814&w=2 > > Comments on V1: > https://marc.info/?l=linux-arm-kernel&m=147051551821122&w=2 > https://marc.info/?l=linux-arm-kernel&m=147051553121150&w=2 > https://marc.info/?l=linux-arm-kernel&m=147194856600627&w=2 > https://marc.info/?l=linux-arm-kernel&m=147072742432211&w=2 > > > Mirza Krak (6): > clk: tegra: add TEGRA20_CLK_NOR to init table > clk: tegra: add TEGRA30_CLK_NOR to init table > dt/bindings: Add bindings for Tegra GMI controller > ARM: tegra: Add Tegra30 GMI support > ARM: tegra: Add Tegra20 GMI support > bus: Add support for Tegra Generic Memory Interface > > .../devicetree/bindings/bus/nvidia,tegra20-gmi.txt | 132 > ++++++++++++ > arch/arm/boot/dts/tegra20.dtsi | 13 ++ > arch/arm/boot/dts/tegra30.dtsi | 12 ++ > drivers/bus/Kconfig | 8 + > drivers/bus/Makefile | 1 + > drivers/bus/tegra-gmi.c | 231 > +++++++++++++++++++++ > drivers/clk/tegra/clk-tegra20.c | 1 + > drivers/clk/tegra/clk-tegra30.c | 1 + > 8 files changed, 399 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/bus/nvidia,tegra20-gmi.txt > create mode 100644 drivers/bus/tegra-gmi.c > > -- > 2.1.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-clk" > in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Cheers Marcel
[toc] | [prev] | [next] | [standalone]
| From | Mirza Krak <mirza.krak@gmail.com> |
|---|---|
| Date | 2016-08-31 11:30 +0200 |
| Message-ID | <sca0h-B0-7@gated-at.bofh.it> |
| In reply to | #1472539 |
Hi Marcel. 2016-08-30 17:01 GMT+02:00 Marcel Ziswiler <marcel.ziswiler@toradex.com>: > Hi Mirza > > Sorry, I long since wanted to give you some feedback on this as well. > > BTW: Thank you very much for taking this on! It has been and still is a fun project. So gladly doing it. > > On Wed, 2016-08-24 at 15:37 +0200, Mirza Krak wrote: >> From: Mirza Krak <mirza.krak@gmail.com> >> >> Hi. >> >> This is a follow up to my previous RFC to add support for Tegra GMI >> bus >> controller. >> >> I have tested this series on a Tegra30 using a Colibri T30 SOM on a >> custom >> carrier board which has multiple CAN controllers (SJA1000) connected >> to the >> GMI bus. > > We once did a nice GMI-Memory Board which mates with the extension > connector X3 of our V3.x Colibri Evaluation boards and allows testing > SRAM access not only in muxed but also in non-muxed mode albeit 16-bit > only. I took your driver for a spin both on Colibri T20 as well as > Colibri T30 both in muxed as well as non-muxed mode and it passed all > tests being both manual devmem2 type reads/writes as well as memtester > runs on the full 128K SRAM giving it the physical address using the -p > argument. > > So you may add the following to the whole series: > > Tested-by: Marcel Ziswiler <marcel.ziswiler@toradex.com> > Tested-on: Colibri T20/T30 on EvalBoard V3.x and GMI-Memory Board Thank you very much for testing. Will add your tags in the upcoming V3. Best Regards Mirza
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web