Re: [coreboot] r1040 - coreboot-v3/superio/winbond/w83627thg

[Peter Stuge]

[e-mail] wrote:
> Fix/finish the Winbond W83627THF/THG dts.

..
> +++ coreboot-v3/superio/winbond/w83627thg/dts	2008-11-17 20:40:39 UTC (rev 1040)
> @@ -20,8 +20,23 @@
>
> {
>    device_operations = "w83627thg_ops";
>
> /* To override any of these, put the over-ride in mainboard dts. */
>
> /* Floppy */
>
> floppydev = "0";
>
> floppyenable = "0";
>
> floppyio = "0x3f0";
>
> floppyirq = "6";
>
> floppydrq = "2";

I would like to veto this style - it's not the dts way, it's the v2
way.

Sorry if there is already consensus on this issue, but I would really
like something more dtsy than that for the pnps.

//Peter

r1040 - coreboot-v3/superio/winbond/w83627thg

[Myles Watson]

> -----Original Message-----
> From: [e-mail] [mailto:[e-mail]]
> On Behalf Of Peter Stuge
> Sent: Monday, November 17, 2008 1:49 PM
> To: [e-mail]
> Subject: Re: [coreboot] r1040 - coreboot-v3/superio/winbond/w83627thg
>
> [e-mail] wrote:
> > Fix/finish the Winbond W83627THF/THG dts.
>
> ..
> > +++ coreboot-v3/superio/winbond/w83627thg/dts	2008-11-17 20:40:39

UTC
> (rev 1040)
> > @@ -20,8 +20,23 @@
> >
> > {
> >    device_operations = "w83627thg_ops";
> >
> > /* To override any of these, put the over-ride in mainboard dts. */
> >
> > /* Floppy */
> >
> > floppydev = "0";
> >
> > floppyenable = "0";
> >
> > floppyio = "0x3f0";
> >
> > floppyirq = "6";
> >
> > floppydrq = "2";
>
> I would like to veto this style - it's not the dts way, it's the v2
> way.

I implemented two different ways, and this one was the one that was
accepted.  No one liked the idea of having a generic dts for PNP devices,
and no one liked the idea of having a specific dts for each function of
every SuperIO.
> Sorry if there is already consensus on this issue, but I would really
> like something more dtsy than that for the pnps.

Suggestions?  Code?

Thanks,
Myles

r1040 - coreboot-v3/superio/winbond/w83627thg

[ron minnich]

On Mon, Nov 17, 2008 at 1:19 PM, Myles Watson  wrote:
> Suggestions?  Code?

I am in the process of (slowly) iterating towards something I believe
you all are going to like. I don't like to say "please wait a few
days" but in this one case I'm going to say it.

I hope to have it ready tomorrow night or wed. It's a lot more think
than coding, so I'm taking it slowly, but I do believe you will like
it. I hope to drop an RFC on the list tomorrow.

SO, don't look at this; instead please review my not-yet-reviewed kontron patch.

ron

r1040 - coreboot-v3/superio/winbond/w83627thg

[Uwe Hermann]

On Mon, Nov 17, 2008 at 09:48:31PM +0100, Peter Stuge wrote:
> > /* Floppy */
> > floppydev = "0";
> > floppyenable = "0";
> > floppyio = "0x3f0";
> > floppyirq = "6";
> > floppydrq = "2";
>
> I would like to veto this style -

I agree that this is not ideal, I don't like it either. Especially
encoding LDN names into variable names is crap, we have to find a better
way to do that (arrays or something, so that the functions in superio.c
can be dropped in favor of one common, nice function that iterates over
an array and that we _don't_ have to rewrite for every superio.c).

This stuff below should be totally unnecessary later:

[...]
        /* Keyboard */
        pnp_dev_info[IT8716F_KBCK].enable = conf->kbenable;
        pnp_dev_info[IT8716F_KBCK].io0.val = conf->kbio;
        pnp_dev_info[IT8716F_KBCK].io1.val = conf->kbio2;
        pnp_dev_info[IT8716F_KBCK].irq0.val = conf->kbirq;

/* PS/2 mouse */
        pnp_dev_info[IT8716F_KBCM].enable = conf->mouseenable;
        pnp_dev_info[IT8716F_KBCM].irq0.val = conf->mouseirq;

/* GPIO */
        pnp_dev_info[IT8716F_GPIO].enable = conf->gpioenable;
        [...]

I'm not saying I know the perfect way to achieve this, but we should
try to find a good solution here.
> it's not the dts way, it's the v2 way.

Can you explain what you refer to as "dts way"? How should it look like?
> Sorry if there is already consensus on this issue, but I would really
> like something more dtsy than that for the pnps.

Yep.

Uwe.

r1040 - coreboot-v3/superio/winbond/w83627thg

[Peter Stuge]

Uwe Hermann wrote:
> I'm not saying I know the perfect way to achieve this, but we
> should try to find a good solution here.

We all agree. \o/
> > it's not the dts way, it's the v2 way.
>
> Can you explain what you refer to as "dts way"? How should it
> look like?

Don't have anything. Just know that those options were redundant.

Curious about Ron's idea. :)

//Peter