[coreboot] [PATCH] Kill 'pnp_ops', have all Super I/Os use their own 'ops'

Uwe Hermann uwe at hermann-uwe.de
Sat Oct 4 23:37:20 CEST 2008

On Sat, Oct 04, 2008 at 05:35:06PM +0200, Stefan Reinauer wrote:
> Uwe Hermann wrote:
> > Please correct me if I'm wrong, but I think pnp_ops will never be used
> > anyway (every Super I/O defines its own 'ops' struct)...
> Stupid question, but could we instead drop all the superio chips' own
> ops instead?
> Or are they really special? Do they have to be?

I'm also not entirely sure, but I guess they have to be special, yes.

This is what the generic (statically defined) 'pnp_ops' looks like:

struct device_operations pnp_ops = {
        .read_resources   = pnp_read_resources,
        .set_resources    = pnp_set_resources,
        .enable_resources = pnp_enable_resources,
        .enable           = pnp_enable,

So all operations use the standard PNP read/set/enable functions, and
there is no '.init' set.

Pretty much all Super I/Os, e.g. winbond/w83627ehg, use something like this:

static struct device_operations ops = {
        .read_resources   = pnp_read_resources,
        .set_resources    = w83627ehg_pnp_set_resources,
        .enable_resources = w83627ehg_pnp_enable_resources,
        .enable           = w83627ehg_pnp_enable,
        .init             = w83627ehg_init,

So, while '.read_resources' still uses the generic pnp_read_resources(),
all the other ones are specific for this Super I/O (especially so the
'.init' which is _always_ different for each Super I/O).

There is some level of code reuse here already (we can specify some
generic pnp_*() functions if this Superio I/O doesn't need special
treatment, or override the entries of the struct with specific ones
if required (e.g. w83627ehg_pnp_set_resources()).

But (AFAICT) we will never ever need the 'pnp_ops' in its above form,
at the very least the '.init' will always be a special one, so the
'pnp_ops' itself is useless. At least that's my current analysis, but
please correct me if I'm wrong.

I'm CC'ing Eric and YHLU, maybe they know the rationale behind pnp_ops?

Thanks, Uwe.
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org

More information about the coreboot mailing list