lkml.org 
[lkml]   [1999]   [Jul]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: PATCH: /dev/nvram cleanups.
Hi Philipp.

>>> Why Riley's solution is bad should be pretty obvious.

>> I'm still waiting to hear a valid reason why it might be bad...

> ok, here is what I think are the reasons:

> 1. it's slow.

Rephrase that - it needs polishing to speed it up. I can agree with
that - indeed, I said as much in an earlier post. As I also pointed
out at the same time, the code snippet was written to highlight the
method of auto-detecting how much CMOS in that chip, and not as an
example of speedy code.

Indeed, my experience has been that examples of speedy code usually
hide the method behind what they're trying to do behind all the fluff
needed to get the speed, so I would be VERY wary of a snippet that
claimed to do both...

> 2. it's specific to PC-style RTCs in that it requires their
> wraparound and the seconds register at a certain location
> (so for example several separate chunks of NVRAM would be
> hard to integrate).

As I see it, if there's more than one chunk of NVRAM, it's because
they are in separate chips, and each chip would need to be probed
separately, so that isn't an issue.

> 3. it requires rewriting the existing nvram driver(s ?)

It doesn't actually - all that changes is the amount of ram the
current nvram driver can refer to, and that is at worst a case of
changing what's currently a constant into a global variable. The
rest of the driver would stay as it is.

Remember, the current nvram driver just blindly assumes that only
50 bytes of CMOS RAM exist. With the auto-detection in place, it
would instead know the amount of RAM present, something which is
generally agreed to be a good thing, at least if the comments on this
list are any guide.

>>>>> Is there any reason that wouldn't be the case ?

>>>> I quote from the `man 3 sleep` manpage...

>>> You want to run this code out of userspace ?

>> Who said anything about using userspace?

> last I looked you would not normally use sleep() in the kernel.

For the purpose of an example designed to CLEARLY show the basic
method, the use of sleep(1) was ideal, but is should have been
read as "at this point, do whatever is appropriate until the second
counter has changed". Taken in that context, it says exactly what it
needed to, neither more nor less.

>> Perhaps I can emphasise a point I believe I made in a previous
>> post, which is that the code snippet I wrote was to show the
>> ALGORITHM that I was suggesting be used.

> If you have a guaranteed wrapping behaviour...

All the chips I have seen and used have that behaviour, mainly because
it's cheaper for the manufacturers to implement them that way. Because
of that, I would be highly surprised to see such change any time soon.

> ...you can get all the (1<<n)+RTC_SECONDS values at one time,
> making the routine take one second regardless of the NVRAM's
> size.

That comes under the heading of being an optimisation to the algorithm
rather than being a separate algorithm. Remember, the snippet was
designed to show the method, not to be a world speed champion that hid
all the details of how it worked.

>> My quote from the sleep(3) manpage was to point out the problems
>> that need to be considered when doing anything like that, as
>> they are best summarised by the text therein.

> You'd avoid the rather ugly assumption sleep(1) always makes
> your secound counter increment while just looping until its
> value changes. That way you could get your routine to take .5
> seconds on average.

Add in the optimisation you refer to above, and one can get the
routine to take 0.5 seconds TOTAL.

Alternatively, just put the two steps at opposite ends of the kernel's
internal initialisation procedure and have it effectively take 0 time
due to the fact that even on the fastest processor, the kernel takes
rather more than a second to initialise...

However, none of this makes the slightest change to the basic method.

>>>> A. Have the kernel perform the various parts of this
>>>> test at different points, with at least a second's
>>>> worth of other initialisations between each one. I
>>>> would see this as being VERY difficult to guarantee
>>>> as being done correctly since it would depend on
>>>> the speed of the processor as to how much needs to
>>>> be done between each check.

>>>> and you may not set the RTC in between.

>> Both true and irrelevant.

> It means you may not take a timer interrupt while you are
> waiting, which is not exactly what I would call irrelevant.

Why does it mean that? The kernel's timer interrupt routine never
reads the RTC anyway - in fact, apart from setting the system clock
during the boot-up procedure, nothing in the kernel's boot process
touches the RTC or CMOS at all.

>>>> B. Have the kernel fork a separate thread that did the
>>>> measurement and set a static variable appropriately,
>>>> then exits, with the main kernel spending the time
>>>> starting up various other drivers, and at the end of
>>>> the kernel's initialisation, it makes use of the
>>>> resultant value to initialise /dev/nvram as needed.

>>> ditto.

>> Again, ditto.

> I'm really waiting for your timer-interrupt-less kernel to run
> this code (the chance it would not work is very slow, granted).

I'm still waiting for you to point out where in the timer interrupt
routine the RTC or CMOS is changed.

You're not by any chance getting confused with the system timer chip,
which generates the 100 Hz (1024 Hz on Alpha) timer tick interrupts,
are you? If so, that's a completely separate subsystem with no
connection to the RTC and CMOS.

>>>> 3. All valid locations in the chip's addressing space that
>>>> are not occupied by RTC registers are occupied by CMOS
>>>> RAM locations that are distinct from each other.

>>>> I'm not aware of any chip where that isn't the case, but that
>>>> doesn't say that there isn't one...

>>> If you want to go into strange RTC chips, there's lots more for
>>> you:

>> Thanks for confirming my suspicion that suchlike chips do indeed
>> exist. That basically means that ANY auto-sizing algorithm needs
>> to be able to detect suchlike locations AFTER first detecting the
>> chip's address bus size.

> I am sorry, I just wanted to say I really don't know what
> undocumented weirdnesses are in all the RTC chips around (and
> that I don't want to), not that I actually know of any specific
> problems.

In other words, you're just inventing problems so you can ask how we
deal with them !!!

For reference, I've spent the best part of the last DECADE working
with various RTC chips, and I have yet to come across one that the
above method would not handle. I've met the following variations:

1. External address pins or one or more internal address
registers.

2. RTC registers occupying from 8 to 16 addresses.

3. RTC registers only or with CMOS as well.

4. Where CMOS is present, RTC registers at the bottom or
top of the address map.

5. Where CMOS is present, from 16 to 1010 bytes of CMOS. It is
for this reason that I used a 16-bit value for the limit
counter.

I have not yet met any where the RTC chip includes CMOS in which there
were any non-register locations in the chip that were not valid CMOS
locations, but I would not be surprised to meet such a chip.

One final point: Certainly as far as ix86 systems are concerned, the
manufacturers will in general use chips that remain compatible with
those used on earlier systems, primarily because it's cheaper for them
to do so - if they start using non-standard chips, they have to go to
the expense of obtaining custom drivers for those chips as used in
their systems for the Windows family of operating systems, otherwise
their machines just won't sell. As a result, it is a reasonable
assumption to state that such will in fact happen.

Best wishes from Riley.

+----------------------------------------------------------------------+
| There is something frustrating about the quality and speed of Linux |
| development, ie., the quality is too high and the speed is too high, |
| in other words, I can implement this XXXX feature, but I bet someone |
| else has already done so and is just about to release their patch. |
+----------------------------------------------------------------------+
* ftp://ftp.MemAlpha.cx/pub/rhw/Linux
* http://www.MemAlpha.cx/kernel.versions.html

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 13:53    [from the cache]
©2003-2011 Jasper Spaans