RE: Cpufreq for opteron

From: paul . devriendt
Date: Fri Aug 22 2003 - 15:19:27 EST


> > > +#ifdef DEBUG
> > > ... deletia
> > > +#endif
> >
> > There's a *lot* of this in this driver. Does it really need
> that much
> > debugging info ? Additionally, the combination of dprintk, tprintk,
> > printk (KERN_DEBUG is really messy, and kind of defeats the point
> > of having these macros. If they're not going to be consistent, don't
> > use them at all.
>
> Yep, I do not like those ?printk's too. Anyway, I killed most #ifdef
> DEBUG, and converted it to BUG_ON(). That makes driver shorter and
> easier to read. Hopefully not much new hardware will be buggy.

I am not really expecting to see a lot of buggy hardware. Hopefully !

I am, however, expecting to see BIOS problems. This code has been tested
internally on a few machines, and every single one of the had some form
or error in the BIOS. Even the AMD internal only development platforms
had problems Some of this stuff was defined kind of late, and went through
several revisions.

There are many debug prints in the code, plus additional code that is
enabled when DEBUG/TRACE are defined. This is all there, based on the
experience of debugging these early machines in house.

I really need all that debug code there so that as new buggy machines
appear in the field, I can just have people email me the log file, and
that ought to be sufficient for me to figure out what is wrong with the
BIOS - I can then report the problem to the machine vendor, and perhaps
offer a workaround.

Without the debug/trace code there, I have to fall back to "please put
the machine in a box and mail it to me" instead of "email me the log file".

I know the debug code is ugly ... but, I am expecting to need it. In the
next rev of the driver, when hardware is publicly for sale, we have some
degree of stability, etc ... then great. But, for now, releasing a driver
that has only been tested on prototype hardware ... and removing the
debug code. Ouch.

Please leave all the debug/trace code in there. I do not care if you
want to change the method of invoking it. But, I fear it is going to be
needed, however it is invoked.

Paul.

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