RE: [PATCH 0/5] mpt fusion: Add logging support

[James Bottomley <James.Bottomley@SteelEye.com>]

On Fri, 2007-07-27 at 16:16 -0600, Moore, Eric wrote:
> On Friday, July 27, 2007 10:21 AM, wrote: 
> > 
> > The way your module parameter works is slightly counter intuitive.  On
> > all our other drivers, you can write a value into 
> > 
> > /sys/module/<module>/parameters/<debug parameter>
> > 
> > And have it acted on immediately.  In yours, it seems only to work
> > before the host is probed (because after that, the value in the ioc
> > structure is what's used).
> 
> not true,  the debug parameter can be configured prior to the host being
> probed.

That's what I just said ... if you mean can be configured *after* the
host being probed, then I think the parameter needs a
module_param_call() so you can intercept the set and update the ioc
structures accordingly.

>     We have a command line option called mpt_debug_level, that
> can set the debug level from mptbase.ko.  That way you can enable
> certain debug during probe time prior to the loading of
> mptsas/mptfc/mptspi. Once those upper drivers are loaded, you can toggle
> off and on the debug via the shost_attrib. This is explained in
> mptdebug.h.

Yes, but my point was that most other module parameters are settable
from /sys as well ... this one has some strange rules.

> > 
> > The other question is are you really sure you actually want per host
> > debugging?  is the added flexibility in being able to turn it 
> > on and off
> > per host worth the problems of explaining to the users where 
> > to find the
> > parameter?  I've got to bet that 95% of the installations only have a
> > single fusion card anyway.  would it not be simpler just to have a
> > global module parameter that can be set and acted on from 
> > /sys/modules?
> > 
> 
> I like having the added flexibility, and potential customers may agree.
> Our driver stack support multiple bus protocols, unlike other vendors,
> and some customers may ship fibre, sas, and spi in a single systems..
> For my personal use, I like being able to have per host debugging, as I
> have multiple cards in my systems.    There are several cases I've
> debugged two controller case, when boot OS is on one controller, and the
> debug efforts on another, in that case I only want to concern myself
> with the debug in question, not boot OS.  The method of debug usesage is
> in mptdebug.h, so I would think people would look there, and figure it
> out.  I also have a script below that sets all the host debug sas chips.
> Does this sound reasonable? If not, let me know.

OK fair enough ... I'm just pointing out it's non standard.

I really think the module parameter has to be hooked to act as a global
setting, but otherwise, this looks fine.

James