[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