[PATCH] nvram buffering/error logging port to 2.6
Paul Mackerras
paulus at samba.org
Thu Nov 13 22:39:39 EST 2003
Jake,
> Here is the patch again w/ a symlink for /proc/rtas to
> /proc/ppc64/rtas.
>
> Per Olof's suggestion I also changed the names of all the symbols to
> have the "nvram" portion of the name in the front.
Mostly looks fine. A few comments:
* Please add a comment somewhere explaining why it is necessary to
have the kernel write the error log to nvram, i.e. what the
circumstances are under which Bad Things would happen if we relied
on userspace to do it (and what those Bad Things are).
* We are going to have to separate the general /dev/nvram support from
the methods for reading/writing nvram via RTAS, because the G5
powermacs have nvram but don't use RTAS. This is not directly a
criticism of the patch, but if you felt like reworking it to make
this separation (and maybe put function pointers for nvram_read /
nvram_write into ppc_md) that would be great.
* With the kernel parsing the partitions in the nvram, I wonder if we
should be exporting that information to userspace somehow?
* In future, as a way of reducing the bulk of the code, could we
consider having userspace do the repartitioning to create the
ppc64,linux partition instead of the kernel? That is, could we have
rtasd (or something similar) create the partition ahead of time so
that the kernel would only have to read the partitioning? Or is
there some scenario where this would be impossible?
* The nvram_read and nvram_write routines could be rewritten to be a
bit shorter and neater. In particular we should be able to do it
with just one loop containing the rtas_call call, rather than an if
and a loop each with a call to rtas_call. And we shouldn't need to
do a mod operation. Like this:
ssize_t nvram_read(char *buf, size_t count, loff_t *index)
{
unsigned int i;
unsigned long len, done;
unsigned long flags;
char *p = buf;
if (*index >= rtas_nvram_size)
return 0;
i = *index;
if (i + count > rtas_nvram_size)
count = rtas_nvram_size - i;
spin_lock_irqsave(&nvram_lock, flags);
for (; count != 0; count -= len) {
len = count;
if (len > NVRW_CNT)
len = NVRW_CNT;
if ((rtas_call(nvram_fetch, 3, 2, &done, i, __pa(nvram_buf),
len) != 0) || done != len) {
spin_unlock_irqrestore(&nvram_lock, flags);
return -EIO;
}
memcpy(p, nvram_buf, len);
p += len;
i += len;
}
spin_unlock_irqrestore(&nvram_lock, flags);
*index = i;
return p - buf;
}
Regards,
Paul.
** Sent via the linuxppc64-dev mail list. See http://lists.linuxppc.org/
More information about the Linuxppc64-dev
mailing list