Thread (29 messages) 29 messages, 6 authors, 2017-03-13

Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for UniPhier SoC variants

From: Marek Vasut <marek.vasut@gmail.com>
Date: 2016-12-05 21:43:19
Also in: lkml

On 12/05/2016 09:51 PM, Dinh Nguyen wrote:
On Sun, Dec 4, 2016 at 10:22 PM, Marek Vasut [off-list ref] wrote:
quoted
On 12/05/2016 05:10 AM, Masahiro Yamada wrote:
quoted
Hi Marek,


2016-12-05 12:44 GMT+09:00 Marek Vasut [off-list ref]:
quoted
On 12/05/2016 04:30 AM, Masahiro Yamada wrote:
quoted
Hi Dinh,


2016-12-04 7:08 GMT+09:00 Dinh Nguyen [off-list ref]:
quoted
Hi,

On Fri, Dec 2, 2016 at 8:49 PM, Marek Vasut [off-list ref] wrote:
quoted
On 12/03/2016 03:41 AM, Masahiro Yamada wrote:
quoted
Hi Rob,
Hi!
quoted
2016-12-03 1:26 GMT+09:00 Rob Herring [off-list ref]:
quoted
quoted

(Plan A)
  "denali,socfpga-nand"           (for Altera SOCFPGA variant)
  "denali,uniphier-nand-v1"       (for old Socionext UniPhier family variant)
  "denali,uniphier-nand-v2"       (for new Socionext UniPhier family variant)

(Plan B)
  "altera,denali-nand"            (for Altera SOCFPGA variant)
  "socionext,denali-nand-v5a"     (for old Socionext UniPhier family variant)
  "socionext,denali-nand-v5b"     (for new Socionext UniPhier family variant)
quoted
Let the Altera folks worry about their stuff. At least for soft IP in
FPGA, it's a bit of a special case. The old string can remain as bad
as it is.

Hmm, I am not sure if this IP would fit in FPGA
(to use it along with NIOS-II?)

(even if it happened, nothing of this IP would be customizable on users' side.
When buying the IP, SoC vendors submit a list of desired features.
Denali (now Cadence) generates the RTL according to the configuration sheet.
The function is fixed at this point. So, generic compatible would be
useless anyway.)


If we are talking about SOCFPGA,
SOCFPGA is not only FPGA. Rather "SOC" + "FPGA".
It consists of two parts:
[1] SOC part  (Cortex-A9 + various hard-wired peripherals such UART,
USB, SD, NAND, ...)
[2] FPGA part (User design logic)

The Denali NAND controller is included in [1].
So, as far as we talk about the Denali on SOCFPGA,
it is as hard-wired as Intel, Socionext's ones.
That's correct, the Denali NAND IP in altera socfpga is a hardware
block. You can make it available to the fabric too, but by default
it's used by the ARM part of the chip, so for this discussion, you
can forget that the FPGA part exists altogether.

I would be in favor of plan B, since it seems to be the more often
taken approach. A nice example is ci-hdrc:

$ git grep compatible drivers/usb/chipidea/
quoted
quoted
I simply would do "socionext,uniphier-v5b-nand" (and v5a).
The fact that it is denali is part of the documentation.
Let me think about this.

Socionext bought two version of Denali IP,
and we are now re-using the newer one (v5b) for several SoCs.
Socionext has some more product lines other than Uniphier SoC family,
perhaps wider re-use might happen in the future.

At first, I included "uniphier" in compatible, but I am still wondering
if such a specific string is good or not.

Also, comments from Altera engineers are appreciated.
Sorry, it's taken me a while to add comments. My altera email is very spotty now
that the Intel merge is completed. Please use dinguyen@kernel.org for any future
communications.

Yes, everything that is said so far for the NAND controller on the
SoCFPGA is correct. I added the binding for the controller a while
back, but unfortunately, we never added the NAND interface to the
devkit, so we did not do much in terms of enabling it.

I think the only SoCFPGA board I know that has the NAND interface active is
the TRCom board, but I have never seen that board.

I don't have any strong opinions on this matter, just as long as the
original binding
"denali,denali-nand-dt" is kept, and I think Rob was ok with keeping
that binding.
I am proposing to add "altera,denali-nand" for Altera.
For what, do you need the generic compatible?
This IP has no default for it to fallback to.
IMO just for compatibility reasons with old DTs .
We generally contribute for
a "working driver" (at least, should be functional to some extent)
and "DT binding" bundled together.

However, Altera upstreamed the DT binding first
(then some parts of the DT binding turned out wrong),
but they did not upstream needed driver changes in the end.

So, the mainline driver has never worked on SOCFPGA, right?
Most likely it never worked, yes.
Right, looking through our downstream support, we may need to upstream a
few changes to make upstream driver work on SoCFPGA.
quoted
quoted
Removing "denali,denali-nand-dt" is not breakage at all,
so I do not owe anything to them, right?
I don't think I'm really qualified to answer this one. But, there is
drivers/mtd/nand/denali_dt.c , which handles this compatible string
and it's documented in
Documentation/devicetree/bindings/mtd/denali-nand.txt, so doesn't that
make it part of the ABI ? I think we should
at least keep it as a fallback, that should be pretty harmless.
I would like to propose "altr,denali-nand" as the binding we use to support the
driver going forward on SoCFPGA hardware. It's pretty much the same as
"altera,denali-nand", just with the correct vendor prefix.
Ah right, altr is the right prefix, thanks for pointing that out.
Still, wouldn't altr,socfpga-denali-nand be better ? I know it's
long, but it encodes the chip type , like ie. fsl,imx6q-usb .
If we can please keep, "denali,denali-nand-dt" only because SoCFPGA is using
this binding downstream, but I know that is a weak argument.
-- 
Best regards,
Marek Vasut

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help