LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Boris Brezillon <boris.brezillon@bootlin.com>
To: Paul Cercueil <paul@crapouillou.net>
Cc: Mark Rutland <mark.rutland@arm.com>,
	David Woodhouse <dwmw2@infradead.org>,
	linux-mtd@lists.infradead.org, Rob Herring <robh+dt@kernel.org>,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	Brian Norris <computersforpeace@gmail.com>,
	Richard Weinberger <richard@nod.at>,
	Boris Brezillon <boris.brezillon@bolin.com>,
	Marek Vasut <marek.vasut@gmail.com>
Subject: Re: [PATCH] mtd: nand: Add support for reading ooblayout from device tree
Date: Sat, 12 May 2018 20:02:19 +0200	[thread overview]
Message-ID: <20180512200219.6ded51a6@bbrezillon> (raw)
In-Reply-To: <1526146969.1594.0@smtp.crapouillou.net>

On Sat, 12 May 2018 19:42:49 +0200
Paul Cercueil <paul@crapouillou.net> wrote:

> >>  >>  My motivation is to get rid of this (move it to devicetree):
> >>  >>
> >>  >>   
> >> https://elixir.bootlin.com/linux/latest/source/arch/mips/jz4740/board-qi_lb60.c#L93  
> >>  >>  And enable the support of other boards with custom OOB layouts.  
> >>  >
> >>  > Can you list the different layouts you have? I'm pretty sure   
> >> there's a  
> >>  > pattern. Maybe we can even deduce the layout from the page size   
> >> or OOB  
> >>  > size.  
> >> 
> >>  This is the other layout I have for another ingenic device:
> >>  
> >> http://projects.qi-hardware.com/index.php/p/qi-kernel/source/tree/od-2011-09-18/arch/mips/jz4740/board-a320.c#L125
> >> 
> >>  Page size and OOB size are the same between these two devices.  
> > 
> > Indeed. Do you know if there are other kind of layouts in the wild?  
> 
> I'm getting a new board in a few weeks, I'll be able to check that out.
> 
> > Note that <layout-id> can be a string, so if each each board is
> > defining its own layout, you could specify the board name here.
> > Otherwise, if you just have those 2 patterns, you can just name them
> > "contiguous" and "interleaved".  
> 
> I don't like the idea of adding board-specific data inside the driver...
> I'd prefer to use the method I used in this patch, but inside the
> jz4740-nand driver, if you're OK with it.

Please don't. Encoding such detailed description in the DT has almost
always proven to be poor choice. Also, people are likely to get it
wrong, and then you'll have to fix all DTs, while, with a single unique
ID representing the layout, you can
- re-use existing layouts easily without having to describe everything
  again in the DT
- fix the driver without getting in trouble with people who claim
  that DT is a stable ABI and don't want to update their DT

So, please just pick user-friendly IDs and add layout definitions in
the driver. If you don't want to leak board info in the driver, then
don't name the layout with the board name. BTW, I still hope you'll only
have 2 kind of layouts (contiguous and interleaved).

  reply	other threads:[~2018-05-12 18:02 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20180512115551.56C6E20787@mail.bootlin.com>
2018-05-12 13:42 ` Boris Brezillon
2018-05-12 14:38   ` Paul Cercueil
2018-05-12 15:00     ` Boris Brezillon
2018-05-12 17:42       ` Paul Cercueil
2018-05-12 18:02         ` Boris Brezillon [this message]
2018-05-12 18:30           ` Paul Cercueil
2018-05-11 21:29 Paul Cercueil
2018-05-12  5:55 ` Boris Brezillon

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=20180512200219.6ded51a6@bbrezillon \
    --to=boris.brezillon@bootlin.com \
    --cc=boris.brezillon@bolin.com \
    --cc=computersforpeace@gmail.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dwmw2@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=marek.vasut@gmail.com \
    --cc=mark.rutland@arm.com \
    --cc=paul@crapouillou.net \
    --cc=richard@nod.at \
    --cc=robh+dt@kernel.org \
    --subject='Re: [PATCH] mtd: nand: Add support for reading ooblayout from device tree' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).