From: thierry.reding@gmail•com (Thierry Reding)
To: linux-arm-kernel@lists•infradead.org
Subject: [RFC 3/6] dt/bindings: Add bindings for Tegra20/30 NOR bus driver
Date: Mon, 25 Jul 2016 16:15:44 +0200 [thread overview]
Message-ID: <20160725141544.GN21170@ulmo.ba.sec> (raw)
In-Reply-To: <CALw8SCWEFVu__8Gyr5q995h+=oMQEz1VUNB=Ne7MRb0O-tZXqg@mail.gmail.com>
On Mon, Jul 25, 2016 at 03:16:28PM +0200, Mirza Krak wrote:
> 2016-07-25 13:30 GMT+02:00 Thierry Reding <thierry.reding@gmail•com>:
[...]
> > The above suggests that one of the CAN controllers gets mapped to an
> > address 0x48000000 and the other gets mapped to 0x48040000. But why do
> > we even need a chip-select at all in that case? If both chips don't use
> > any overlapping memory region, what good does the chip-select do?
>
> If we take a look on similar controllers found on others SOCs they
> usually define an address range / chip-select.
>
> Example (weim):
> ranges = <
> 0 0 0x10000000 0x02000000
> 1 0 0x12000000 0x01000000
> 2 0 0x13000000 0x01000000
> 3 0 0x14000000 0x01000000
> 4 0 0x15000000 0x01000000
> 5 0 0x16000000 0x01000000
> >;
>
> Which means that you all ready have an address mapped to PIN function.
>
> But Tegra GMI controller is a first for me, where you do not have this
> kind of setup in hardware. Usually you also have a timing register /
> chip-select so that you can connect different chip types.
>
> The lack of hardware support do decode an address to a chip-select PIN
> function, we have implemented this our self externally.
>
> We actually have 6 different chips connected to the GMI bus and the
> "ranges" would be:
> ranges = <
> 0 0 0x48000000 0x00000100
> 1 0 0x48040000 0x00000100
> 2 0 0x48080000 0x00000100
> 3 0 0x480A0000 0x00000100
> 4 0 0x480C0000 0x00000100
> 5 0 0x480E0000 0x00000100
> >;
>
> And this not nothing complicated, small number of AND gates and that is it.
>
> The chip-select signal is necessary for the access characteristics, so
> we need to translate an address to an chip-select so that the chip
> knows the host CPU wants to talk to it.
>
> Do not know if I made anything more clear here :).
Yes, that clarifies many things. The presence of an external, address-
based chip-select is essential information in order to describe this
setup properly.
Given that the external chip select is entirely invisible to software, I
think a more accurate description of your setup would be:
gmi at 70090000 {
...
/* for the chip select */
#address-cells = <1>;
#size-cells = <0>;
/*
* Technically this could be used to translate the range from
* 0x48000000 to 0x4fffffff into a different range, but that
* no longer works because of the #address-cells. Does this
* matter?
*/
ranges;
bus at 0 {
compatible = "simple-bus";
reg = <0>;
#address-cells = <1>;
#size-cells = <1>;
can at 48000000 {
reg = <0x48000000 0x100>;
...
};
can at 48040000 {
reg = <0x48040000 0x100>;
...
};
};
};
That omits any reference to the external chip select, which I think
makes sense because it's something that software is completely unaware
of.
Perhaps one important question: does your setup use the GMI's CS lines
in any way? Or does it simply get ignored?
If it gets ignored, I suppose one could encode this as a special case:
gmi at 70090000 {
...
/* simple address translation */
#address-cells = <1>;
#size-cells = <1>;
ranges = <0x48000000 0x48000000 0x08000000>;
can at 48000000 {
...
reg = <0x48000000 0x100>;
...
};
can at 48040000 {
...
reg = <0x48000000 0x100>;
...
};
};
We could use that special case in order to make the driver behave in a
special way that suits your setup, namely without explicit chip-select.
The case where #size-cells = <0> could be treated as the explicit chip-
select case, that somebody could implement in the future if they need
it. All other cases could be treated as meaning that the chip-select is
automatically handled externally (like in your setup) and we simply
statically configure the controller with chip select 0 (kind of in the
way your current patch series does).
Rob, what do you think about the above?
> > Also, it seems to me that you'd have to program the SNOR_CONFIG_0
> > register in order to select a specific chip, but I don't see anything in
> > the driver access that register after the initial write of the register.
>
> This is only setup at probe.
>
> >
> > I would've expected this to require some sort of infrastructure to allow
> > devices connected to the GMI controller to acquire the bus via some API
> > to select their chip.
>
> Yes, ultimately you would need some sort of infrastructure to allow
> devices to acquire the GMI bus if you want to solve this in software.
> But at the moment I do not see such an infrastructure in place, and is
> it feasible to add one specifically for the GMI controller? If one
> such infrastructure was in place we would need to modify all the
> drivers that want to use to include Tegra specific infrastructure to
> access the GMI bus?
>
> Since my knowledge is limited it hard for me to comment on this, maybe
> there is a simple way of doing this?
I don't think there's a simple way to do this. In order to properly
implement it we'd need to implement a generic infrastructure for chip
selects so that drivers such as the one for your CAN controller can be
written without tying them specifically to the Tegra GMI controller.
>From what you and Jon were saying it sounds like the drivers are
completely agnostic of any chip-select, so conversion won't be easy.
But technically if these chips take a chip-select as input then it's
always possible to hook them up to a controller that doesn't do this
automatic translation of address to chip-select, so eventually some
setup is bound to come along where they'd need explicit chip-select
handling as well.
I don't think it's fair to require you to implement this infrastructure
if you don't actually need it. At the same time I want to be cautious
and make sure we keep the driver and binding flexible enough to allow
us to implement explicit chip-selects should we later need them.
Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160725/374df929/attachment.sig>
next prev parent reply other threads:[~2016-07-25 14:15 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-19 13:36 [RFC 0/6] Add support for Tegra20/30 NOR bus controller Mirza Krak
2016-07-19 13:36 ` [RFC 1/6] clk: tegra: add TEGRA20_CLK_NOR to init table Mirza Krak
2016-07-25 11:17 ` Thierry Reding
2016-07-25 12:28 ` Mirza Krak
2016-07-25 13:23 ` Thierry Reding
2016-07-19 13:36 ` [RFC 2/6] clk: tegra: add TEGRA30_CLK_NOR " Mirza Krak
2016-07-19 13:36 ` [RFC 3/6] dt/bindings: Add bindings for Tegra20/30 NOR bus driver Mirza Krak
2016-07-20 12:44 ` Rob Herring
2016-07-20 19:28 ` Mirza Krak
2016-07-21 10:26 ` Jon Hunter
2016-07-25 11:36 ` Thierry Reding
2016-07-25 13:20 ` Mirza Krak
2016-07-25 13:27 ` Thierry Reding
2016-07-25 13:33 ` Mirza Krak
2016-07-21 9:56 ` Jon Hunter
2016-07-21 20:10 ` Mirza Krak
2016-07-22 9:32 ` Jon Hunter
2016-07-22 19:07 ` Mirza Krak
2016-07-25 8:14 ` Jon Hunter
2016-07-25 12:10 ` Thierry Reding
2016-07-25 13:09 ` Jon Hunter
2016-07-25 13:32 ` Thierry Reding
2016-07-25 11:59 ` Thierry Reding
2016-07-25 13:30 ` Mirza Krak
2016-07-25 13:39 ` Thierry Reding
2016-07-25 13:50 ` Mirza Krak
2016-07-25 13:36 ` Jon Hunter
2016-07-25 13:49 ` Thierry Reding
2016-07-25 11:30 ` Thierry Reding
2016-07-25 13:16 ` Mirza Krak
2016-07-25 14:15 ` Thierry Reding [this message]
2016-07-25 14:38 ` Mirza Krak
2016-07-25 15:01 ` Jon Hunter
2016-07-25 15:34 ` Thierry Reding
2016-07-25 19:59 ` Mirza Krak
2016-07-26 8:32 ` Thierry Reding
2016-07-28 9:29 ` Mirza Krak
2016-07-19 13:36 ` [RFC 4/6] ARM: tegra: Add Tegra30 NOR support Mirza Krak
2016-07-19 13:36 ` [RFC 5/6] ARM: tegra: Add Tegra20 " Mirza Krak
2016-07-19 13:36 ` [RFC 6/6] bus: Add support for Tegra NOR controller Mirza Krak
2016-07-21 10:15 ` Jon Hunter
2016-07-21 20:42 ` Mirza Krak
2016-07-22 9:38 ` Jon Hunter
2016-07-22 19:18 ` Mirza Krak
2016-07-25 8:19 ` Jon Hunter
2016-07-25 10:57 ` Thierry Reding
2016-07-21 15:12 ` Jon Hunter
2016-07-21 21:41 ` Mirza Krak
2016-07-25 11:14 ` Thierry Reding
2016-07-25 12:17 ` Mirza Krak
2016-07-25 13:41 ` Thierry Reding
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160725141544.GN21170@ulmo.ba.sec \
--to=thierry.reding@gmail$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox