From: Paul Walmsley <paul@pwsan.com>
To: "Rafał Miłecki" <zajec5@gmail.com>
Cc: linux-mips@linux-mips.org, Ralf Baechle <ralf@linux-mips.org>,
Hauke Mehrtens <hauke@hauke-m.de>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Kumar Gala <galak@codeaurora.org>,
Olof Johansson <olof@lixom.net>, Arnd Bergmann <arnd@arndb.de>,
Sandeep Nair <sandeep_n@ti.com>,
linux-soc@vger.kernel.org
Subject: Re: [PATCH V3] MIPS: BCM47XX: Move NVRAM driver to the drivers/soc/
Date: Mon, 24 Nov 2014 10:02:27 +0000 (UTC) [thread overview]
Message-ID: <alpine.DEB.2.02.1411240910100.16047@utopia.booyaka.com> (raw)
In-Reply-To: <1416778509-31502-1-git-send-email-zajec5@gmail.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 2913 bytes --]
Hi Rafał.
On Sun, 23 Nov 2014, Rafał Miłecki wrote:
> After Broadcom switched from MIPS to ARM for their home routers we need
> to have NVRAM driver in some common place (not arch/mips/).
So this is a driver for a chunk of NVRAM that's embedded in the SoC,
hanging off an I/O bus?
Is this actually some kind of RAM, or is it flash, or something else?
> We were thinking about putting it in bus directory, however there are
> two possible buses for MIPS: drivers/ssb/ and drivers/bcma/. So this
> won't fit there neither.
> This is why I would like to move this driver to the drivers/soc/
Can't speak for anyone else, but I personally consider drivers/soc to be
primarily intended for so-called "integration" IP blocks. Those are used
for SoC control functions.
So low-level NVRAM drivers would probably go somewhere else. Here are
some likely possibilities, where it can live with others of its kind
(assuming it is something similar to RAM):
1. the PC "CMOS memory" NVRAM driver appears to be under drivers/char, as
drivers/char/nvram.c.
2. there's a generic SRAM driver, drivers/misc/sram.c
3. while people have put some of the eFuse code under drivers/soc, in my
opinion, the low-level fuse access code should really go under
drivers/misc/fuse.
...
Looking at arch/mips/bcm47xx/nvram.c: if the low-level NVRAM probe code
were moved elsewhere, the higher-level NVRAM "interpretation" functions
still remain: bcm47xx_nvram_getenv() and bcm47xx_nvram_gpio_pin(). Those
seem to be intended to parse device configuration data, yes? And this
device configuration data is organized this way by platform software
convention -- there's no hardware requirement to store data this way in
the NVRAM, right?
Unfortunately the way that we organize device data parsing code in the
kernel is quite frankly ... anarchic. In some kind of ideal world, we'd
have a standard place for device data storage and parsing functions, and
any combination of DT/ACPI/pdata/PCI/ISAPNP/whatever could be placed in
use, depending on the software environment. But instead we have
directories like drivers/acpi and drivers/of and drivers/base/platform and
drivers/pnp and drivers/platform/chrome.
So if the answer to the above two questions is "yes," meaning that this
NVRAM is used to store device probing data by software convention, then it
would seem to me that proposing some place like
drivers/devicedata/bcm47xx-nvram is a better bet (where "bcm47xx-nvram" is
just some kind of opaque token). Looking at those two functions, it
doesn't seem like there's really anything MIPS or Broadcom or NVRAM or
even SoC-specific about key-value pair parsing and GPIO device data? Or
am I missing something?
Actually, considering that bcm47xx_nvram_gpio_pin() isn't even used, can
we just drop it and save the hassle? :-)
- Paul
next prev parent reply other threads:[~2014-11-24 10:02 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-23 9:50 [PATCH V2] MIPS: BCM47XX: Move NVRAM driver to the drivers/soc/ Rafał Miłecki
2014-11-23 12:16 ` Hauke Mehrtens
2014-11-23 21:35 ` [PATCH V3] " Rafał Miłecki
2014-11-24 10:02 ` Paul Walmsley [this message]
2014-11-24 10:35 ` Rafał Miłecki
2014-11-25 17:50 ` Paul Walmsley
2014-11-25 18:22 ` Rafał Miłecki
2014-11-27 19:56 ` Paul Walmsley
2014-11-27 22:36 ` Rafał Miłecki
2014-11-28 17:07 ` Paul Walmsley
2014-11-28 17:16 ` Ralf Baechle
2014-12-04 6:43 ` Paul Walmsley
2014-12-04 7:28 ` Rafał Miłecki
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=alpine.DEB.2.02.1411240910100.16047@utopia.booyaka.com \
--to=paul@pwsan.com \
--cc=arnd@arndb.de \
--cc=galak@codeaurora.org \
--cc=gregkh@linuxfoundation.org \
--cc=hauke@hauke-m.de \
--cc=linux-mips@linux-mips.org \
--cc=linux-soc@vger.kernel.org \
--cc=olof@lixom.net \
--cc=ralf@linux-mips.org \
--cc=sandeep_n@ti.com \
--cc=zajec5@gmail.com \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.