On Tuesday 27 October 2009 09:38:41 am Bjorn Helgaas wrote:
> On Monday 26 October 2009 07:02:55 pm ykzhao wrote:
> > In fact you mention two issues about the two patches:
> >    1: Load a PNP driver for it to register the IPMI system interface.
> > This is about the first patch.
> >    2. coding style( for example: comments, the definition about some
> > variables).
> > 
> > For the first issue: Before I start the first patch, I consider using
> > the PNP device driver. But I find that it is so complex because of the
> > following two points:
> >     1. One is that we can't register the IPMI system interface if the
> > boot option of "pnpacpi=off" is added. This will also depend on the PNP
> > module.
> 
> This is not a problem.  It is perfectly acceptable for the IPMI driver
> to depend on PNP and PNPACPI in order to claim an ACPI device.  If the
> users boots with "pnpacpi=off", we just won't find an IPMI device.
> That is the way it works for TPM devices and serial devices described
> by ACPI, and IPMI should work the same way.
> 
> >     2. The second is that there exist so many cases about the IPMI
> > IO/memory resource definition. Maybe there exist both IO/memory resource
> > definition for one IPMI device. In such case we can't know which should
> > be selected. At the same time we have similar issues about the interrupt
> > type. So I decide to parse the IO/memory/interrupt resource
> > independently.
> 
> This doesn't make any sense.  The fact that an IPMI device might have
> a variety of IO/memory/IRQ resources is orthogonal to the question of
> whether you should use pnp_register_driver() or acpi_walk_namespace().
> 
> PNPACPI parses the IPMI device resources for every ACPI device,
> including the IPMI device, before we even know whether there will be
> a PNP driver for the device.  It's much easier to look at the PNP
> resources and figure out which to use than it is to use
> acpi_walk_resources() manually.
> 
> The main point is that ipmi_si_intf.c is a device driver, and it should
> use the normal driver registration mechanisms.  I think it would be
> simplest and clearest to make a few PNP enhancements so it could use
> pnp_register_driver(), but even using acpi_bus_register_driver() would
> be fine.  Using acpi_walk_namespace() to do everything by hand is just
> completely wrong.

Somebody asked me to clarify this, so here's some explanation that
might make this more clear.

He pointed out:
> The IPMI driver "stack" is split into three levels.  The middle
> layer, ipmi_msghandler, routes commands/responses/etc between
> upper and lower levels.  It has no clue about hardware OR device
> files.  It must be loaded first.
> 
> Lowest level is ipmi_si_intf, and talks directly to hardware, and
> it cares about memory/ports/IRQs/etc.  It only talks to ipmi_msghandler.
> Embedded in it are the three subdrivers (BT, KCS, SMIC) that
> do actual bit twiddling.
> 
> The top layer presents a character device file.  It connects only
> to ipmi_msghandler.
> 
> So device file registration stuff occurs in an entirely different
> source module than ACPI/PNP lookup.

I'm not concerned with the top or middle layers or the device file
registration.  The bottom layer talks directly to hardware, and that
makes it a Linux driver.  It has to use the right I/O ports, MMIO
regions, IRQs, etc., so it avoids conflicts with other devices in
the system.

The fact that your patch uses acpi_walk_namespace() to find the
device means that ipmi_si_intf.c can be talking to a device, but the
rest of the system doesn't know ipmi_si_intf.c "owns" it.  So another
driver B could come along and correctly use acpi_bus_register_driver()
with the IPMI IDs.  The Linux ACPI core knows nothing about the fact
that ipmi_si_intf.c thinks it "owns" the IPMI device, so it will call
the driver B "add" method.  Now we have two drivers that both think
they own the device.  This leads to chaos.

Bjorn

------------------------------------------------------------------------------
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay 
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference
_______________________________________________
Openipmi-developer mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openipmi-developer

Reply via email to