[LinuxBIOS] [RFC] v3: dts fixes and questions

[Uwe Hermann]

Here's a patch which streamlines the dts contents a bit:

- Every "thing" should have an "enable;" line (or am I missing some
   reason why they shouldn't?)

- Every "thing" should have the "enable;" as first entry, if it has one.

- Constructors should come before other stuff, but after the "enable;".

Comments?

Does the order in which variables are defined (e.g. ide0_enable) and in
which the constructors are called matter? I.e. is there a semantic
difference between

{
        ide0_enable = "0";
        ide1_enable = "0";
        constructor = "i82371eb_constructors";
 };

and

{
        constructor = "i82371eb_constructors";
        ide0_enable = "0";
        ide1_enable = "0";
 };

?

See patch for my proposal, but please note that it may be incorrent,
that's why I'm asking...

Uwe.

[ron minnich]

On 6/30/07, Uwe Hermann  wrote:
> Here's a patch which streamlines the dts contents a bit:
> Every "thing" should have an "enable;" line (or am I missing some
> reason why they shouldn't?)

no, the enable is part of the device struct that is
device-independent, see device.h. You are right that every device
should have this, so it is device-independent.
> Every "thing" should have the "enable;" as first entry, if it has one.
>
> Constructors should come before other stuff, but after the "enable;".

no, see above. Not needed.
> Does the order in which variables are defined (e.g. ide0_enable) and in
> which the constructors are called matter? I.e. is there a semantic
> difference between
>
> {
>         ide0_enable = "0";
>         ide1_enable = "0";
>         constructor = "i82371eb_constructors";
>  };
>
> and
>
> {
>         constructor = "i82371eb_constructors";
>         ide0_enable = "0";
>         ide1_enable = "0";
>  };

no, it does not matter. The order in which the dtc generates these in
order declared, but people should not depend on that.

thanks for your patch. The enabled; that you are adding to dts is
fine, but hang on, stefan has another idea.

ron

[Stefan Reinauer]

* Uwe Hermann  [070630 21:38]:
> Here's a patch which streamlines the dts contents a bit:
> Every "thing" should have an "enable;" line (or am I missing some
> reason why they shouldn't?)

If every device node gets an enabled; entry, the meaning of enabled is
bogus and we can leave it away. So adding enabled to all devices is a
bad idea in my opinion.

Ron explained to me that there might be some devices that need their
bars set up so the bridge they hang off can be set up correctly, but the
device itself should not be enabled. Hope I get it right.

So instead of adding enabled; to all device nodes, we should add
disabled to those that we do not want enabled. What do you think?

[Peter Stuge]

On Sat, Jun 30, 2007 at 11:43:20PM +0200, Stefan Reinauer wrote:
> So instead of adding enabled; to all device nodes, we should add
> disabled to those that we do not want enabled. What do you think?

We should definately optimize for the common case whatever it is.

//Peter

[ron minnich]

On 6/30/07, Peter Stuge  wrote:
> On Sat, Jun 30, 2007 at 11:43:20PM +0200, Stefan Reinauer wrote:
> > So instead of adding enabled; to all device nodes, we should add
> > disabled to those that we do not want enabled. What do you think?
>
> We should definately optimize for the common case whatever it is.

What I can do is look for a disable property in each node. If it is
NOT found, emit
.enabled = 1;
If it is found,
.enabled = 0;

sound ok?

ron

[Peter Stuge]

On Sat, Jun 30, 2007 at 04:27:27PM -0700, ron minnich wrote:
> What I can do is look for a disable property in each node. If it is
> NOT found, emit
> .enabled = 1;
> If it is found,
> .enabled = 0;
>
> sound ok?

Yes sir! As long as enabled is more common than disabled.

//Peter

[Uwe Hermann]

On Sat, Jun 30, 2007 at 04:27:27PM -0700, ron minnich wrote:
> On 6/30/07, Peter Stuge  wrote:
> > On Sat, Jun 30, 2007 at 11:43:20PM +0200, Stefan Reinauer wrote:
> > > So instead of adding enabled; to all device nodes, we should add
> > > disabled to those that we do not want enabled. What do you think?
> >
> > We should definately optimize for the common case whatever it is.
>
> What I can do is look for a disable property in each node. If it is
> NOT found, emit
> .enabled = 1;
> If it is found,
> .enabled = 0;
>
> sound ok?

Why not, instead of 'enabled;', use
  enabled="0"
or 
  enabled="1"
then? That has the benefit that you don't have to know what the default
is (enabled or disabled), you know everything you need just by looking
at the dts itself.

Uwe.

[ron minnich]

On 6/30/07, Uwe Hermann  wrote:
> On Sat, Jun 30, 2007 at 04:27:27PM -0700, ron minnich wrote:
> > On 6/30/07, Peter Stuge  wrote:
> > > On Sat, Jun 30, 2007 at 11:43:20PM +0200, Stefan Reinauer wrote:
> > > > So instead of adding enabled; to all device nodes, we should add
> > > > disabled to those that we do not want enabled. What do you think?
> > >
> > > We should definately optimize for the common case whatever it is.
> >
> > What I can do is look for a disable property in each node. If it is
> > NOT found, emit
> > .enabled = 1;
> > If it is found,
> > .enabled = 0;
> >
> > sound ok?
>
> Why not, instead of 'enabled;', use
>   enabled="0"
> or
>   enabled="1"
> then? That has the benefit that you don't have to know what the default
> is (enabled or disabled), you know everything you need just by looking
> at the dts itself.
> Uwe.
>
> http://www.hermann-uwe.de  | http://www.holsham-traders.de
> http://www.crazy-hacks.org | http://www.unmaintained-free-software.org
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.6 (GNU/Linux)
>
> iD8DBQFGhvBIXdVoV3jWIbQRAgQoAJ0dDBxSH2BlcI7u0ldL93XOtM57/gCbB7nD
> 120zDwDQEbqpwuyLjU8xfAE=
> =hPcG
> -----END PGP SIGNATURE-----

[ron minnich]

Sorry about the lats empty message. Gmail did something, I know not what.

On 6/30/07, Uwe Hermann  wrote:
> On Sat, Jun 30, 2007 at 04:27:27PM -0700, ron minnich wrote:
> > What I can do is look for a disable property in each node. If it is
> > NOT found, emit
> > .enabled = 1;
> > If it is found,
> > .enabled = 0;
> >
> > sound ok?
>
> Why not, instead of 'enabled;', use
>   enabled="0"
> or
>   enabled="1"
> then? That has the benefit that you don't have to know what the default
> is (enabled or disabled), you know everything you need just by looking
> at the dts itself.

yeah, that works. I will take the magic enabled out of the .y, and
will require that people set that property for each node; I will catch
a missing set at flatten tree time. I think that is reasonable.

This will get done next week, I'm out of time for a bit. Or someone
else can write it, which is fine too.

ron

[Stefan Reinauer]

* ron minnich  [070701 05:48]:
> > then? That has the benefit that you don't have to know what the default
> > is (enabled or disabled), you know everything you need just by looking
> > at the dts itself.
>
> yeah, that works. I will take the magic enabled out of the .y, and
> will require that people set that property for each node; I will catch
> a missing set at flatten tree time. I think that is reasonable.

Please dont do that. I think it is reasonable to assume that a device
that you specify is enabled. Otherwise why would you specify it?

I dont like overspecifying completely obvious things in description files.

Stefan

[Uwe Hermann]

On Mon, Jul 02, 2007 at 12:38:32AM +0200, Stefan Reinauer wrote:
> ron minnich  [070701 05:48]:
> > > then? That has the benefit that you don't have to know what the default
> > > is (enabled or disabled), you know everything you need just by looking
> > > at the dts itself.
> >
> > yeah, that works. I will take the magic enabled out of the .y, and
> > will require that people set that property for each node; I will catch
> > a missing set at flatten tree time. I think that is reasonable.
>
> Please dont do that. I think it is reasonable to assume that a device
> that you specify is enabled. Otherwise why would you specify it?
>
> I dont like overspecifying completely obvious things in description files.

Yeah, I think you're right.

So we have two options:

1. disable="1"
 2. enable="0"

for devices which should be disabled. I think option 1 is a bit
clearer. Thoughts?

Uwe.

[Corey Osgood]

Uwe Hermann wrote:
> On Mon, Jul 02, 2007 at 12:38:32AM +0200, Stefan Reinauer wrote:
> > ron minnich  [070701 05:48]:
> > > > then? That has the benefit that you don't have to know what the default
> > > > is (enabled or disabled), you know everything you need just by looking
> > > > at the dts itself.
> > >
> > > yeah, that works. I will take the magic enabled out of the .y, and
> > > will require that people set that property for each node; I will catch
> > > a missing set at flatten tree time. I think that is reasonable.
> >
> > Please dont do that. I think it is reasonable to assume that a device
> > that you specify is enabled. Otherwise why would you specify it?
> >
> > I dont like overspecifying completely obvious things in description files.
>
> Yeah, I think you're right.
>
> So we have two options:
> disable="1"
> enable="0"
>
> for devices which should be disabled. I think option 1 is a bit
> clearer. Thoughts?
>
> Uwe.

Agreed. Default to hardware enabled unless explicitly disabled, and
clearly state that somewhere, seems the best route to me.

-Corey

[ron minnich]

On 7/2/07, Corey Osgood  wrote:
> Agreed. Default to hardware enabled unless explicitly disabled, and
> clearly state that somewhere, seems the best route to me.

Will do. Look for it in a few days.

ron