On 25 November 2014 at 18:50, Paul Walmsley <paul@xxxxxxxxx> wrote: >> I don't think NVRAM can be treated as a standard char device. Also, in >> my V1 I tried moving it to the drivers/misc/, but then drivers/soc/ >> was suggested as a better place, see Arnd's reply: >> http://www.linux-mips.org/archives/linux-mips/2014-11/msg00238.html > > Yeah. It depends on who is going to merge the patch. If you can persuade > someone else to merge it in drivers/soc, then it doesn't really matter > what I think. I'm looking for the solution that will satisfy most ppl. I understand your arguments against drivers/soc/, but on the other hand I have no idea where else this driver could go. >> > 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? >> >> This bcm47xx_nvram driver has two ways of reading NVRAM content: >> 1) Using a standard MTD API to access "nvram" partition. In such case >> flash driver is a low-level thing if you call it so. > > OK, so just to confirm, you are referring to these drivers: > > drivers/mtd/nand/bcm47xxnflash/* > drivers/mtd/devices/bcm47xxsflash.c > drivers/ssb/driver_mipscore.c (for pflash) > drivers/bcma/driver_mips.c (for pflash) > > ? Yes, these are drivers for Broadcom flash memories that implement (more or less directly) MTD API. There is also a new SPI-NOR based driver I'm trying to upstream (it was already sent, needs accepting some more spi-nor changes first). > Just out of curiosity, the nvram.c code seems to contain some code to work > around cases where the flash size is larger than the MMIO aperture, and to > truncate the copy. Is there some way to program the flash controller to > point the aperture at a different section of the flash? Er, not really. We assume NVRAM content is not bigger than NVRAM_SPACE (0x8000), but that's all. If NVRAM can be read using MMIO, then it's always fully available. There isn't any documented way of reprogramming flash content mapping into memory. >> 2) Using memory-mapped flash access window. In such case there isn't >> any extra low-level driver. > > Could you point me at the software entity that configures the > memory-mapped flash access window and programs the flash controller to use > one of {p,n,s}flash? Or is that done by some code external to the kernel, > like the bootloader? It's done by the CFE (bootloader). You can find CFE source in some tarballs published by Asus in their GPL firmware packages. I never really focused on CFE source, never tried to compiling it or re-installing. >> The "nvram" partition is required by the bootloader (CFE). It contains >> some important info like CPU clock, RAM configuration, switch ports >> layout. Then it's used by system drivers (like Ethernet driver bgmac) >> to get info about some particular hardware parts (like PHY address, >> MAC, etc.). > > So it's not used by the on-chip boot-ROM? Sounds like it's just software > convention, then? In other words, if one used a different bootloader, > like u-boot, and passed a DT or something like that to the kernel, this > wouldn't be needed. The presence and format of this flash is part of a > specific hardware/software platform, external to the SoC hardware itself, > that Broadcom suggests. It's analogous to ARM ATAGS - > arch/arm/kernel/atags_parse.c. Does all this match your understanding? > (I'm not advocating using a different bootloader or device data format - I > think it's important to preserve compatibility with CPE - I am just trying > to understand how this area of flash is used.) I guess yes, you could probably use different bootloader and hardcode all important settings into it (e.g. using DTB). I guess DT is older than CFE, but Broadcom decided to invent own solution called NVRAM anyway. This is a bit messy, because it actually stores hardware details (CPU, RAM, switch) as well as user settings (e.g. LEDs behavior). I can't say why Broadcom decided to implement it this way. We all would love to see Broadcom shipping devices with U-Boot and DT! ;) >> > 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? :-) >> >> We need this function to easily find GPIO responsible for "something". >> For example during switch initialization we need to reset it toggling >> GPIO. NVRAM contains info which GPIO should be toggled, e.g.: >> gpio4=robo_reset >> (robo means roboswitch) >> >> bcm47xx_nvram_gpio_pin is needed by out-of-tree "b53" driver that some >> tried to push upstream, but was rejected because of API. >> >> I'll also send a patch to USB host driver soon that will require >> bcm47xx_nvram_gpio_pin. > > OK thanks for the context. > > It sounds to me like this code is a combination of three > pieces: > > 1. code that autoprobes the size of the "nvram" partition in the Broadcom > platform flash, by reading various locations in the MMIO flash aperture, > configured by some other system entity That's right, on MIPS we simply detect flash type (drivers/ssb & driver/bcma) and using that we init NVRAM passing memory offset where the flash is mapped. > 2. code that shadow copies the device data from the MMIO flash aperture > into system RAM > > 3. code that parses the CPE-generated device data and returns it to other > drivers > > Does that sound accurate? Correct (s/CPE/CFE). -- Rafał
