[superiotool] patch for smsc mec1308

[David Hendricks]

Hi,
Attached is a patch to add support for the SMSC MEC1308 embedded controller.

Unfortunately, the device ID it conflicts with another SMSC Super IO chip,
the FDC37M81x. However, since superiotool does not really support the
FDC37M81x in any really useful capacity, I think it's fair to just disable
it for now.

Signed-off-by: David Hendricks

[Stefan Reinauer]

Acked-by: Stefan Reinauer

On Wed, Feb 16, 2011 at 4:11 PM, David Hendricks  wrote:
> Hi,
> Attached is a patch to add support for the SMSC MEC1308 embedded controller.
> Unfortunately, the device ID it conflicts with another SMSC Super IO chip,
> the FDC37M81x. However, since superiotool does not really support the
> FDC37M81x in any really useful capacity, I think it's fair to just disable
> it for now.
> Signed-off-by: David Hendricks

[Carl-Daniel Hailfinger]

Auf 17.02.2011 01:11, David Hendricks schrieb:
> Attached is a patch to add support for the SMSC MEC1308 embedded controller.
>
> Unfortunately, the device ID it conflicts with another SMSC Super IO chip,
> the FDC37M81x. However, since superiotool does not really support the
> FDC37M81x in any really useful capacity, I think it's fair to just disable
> it for now.

Unless I'm mistaken, this patch brings the MEC1308 to the level of
support the FDC37M81x had before your patch. I was just looking at the
line count per chip, so maybe there is some key information I'm missing.
Could you please enlighten me what "not really supported" means in this
context? Thanks!

Regards,
Carl-Daniel

[David Hendricks]

On Wed, Feb 16, 2011 at 5:09 PM, Carl-Daniel Hailfinger <
[e-mail]> wrote:
> Auf 17.02.2011 01:11, David Hendricks schrieb:
> > Attached is a patch to add support for the SMSC MEC1308 embedded
>
> controller.
> > Unfortunately, the device ID it conflicts with another SMSC Super IO
>
> chip,
> > the FDC37M81x. However, since superiotool does not really support the
> > FDC37M81x in any really useful capacity, I think it's fair to just
>
> disable
> > it for now.
>
> Unless I'm mistaken, this patch brings the MEC1308 to the level of
> support the FDC37M81x had before your patch. I was just looking at the
> line count per chip, so maybe there is some key information I'm missing.
> Could you please enlighten me what "not really supported" means in this
> context? Thanks!

Huh, odd... I actually wrote the patch over a year ago, and was probably
using the obsolete branch (before it got put back into
coreboot/util). Clearly the FDC37M81x has support now. Should we just leave
both of them enabled?

(apologies for the hasty commit, i had an immediate use case)
> Regards,
> Carl-Daniel

[Carl-Daniel Hailfinger]

Auf 17.02.2011 02:38, David Hendricks schrieb:
> On Wed, Feb 16, 2011 at 5:09 PM, Carl-Daniel Hailfinger <
> [e-mail]> wrote:
> > Auf 17.02.2011 01:11, David Hendricks schrieb:
> > > Attached is a patch to add support for the SMSC MEC1308 embedded
> >
> > controller.
> > > Unfortunately, the device ID it conflicts with another SMSC Super IO
> >
> > chip,
> > > the FDC37M81x. However, since superiotool does not really support the
> > > FDC37M81x in any really useful capacity, I think it's fair to just
> >
> > disable
> > > it for now.
> >
> > Unless I'm mistaken, this patch brings the MEC1308 to the level of
> > support the FDC37M81x had before your patch. I was just looking at the
> > line count per chip, so maybe there is some key information I'm missing.
> > Could you please enlighten me what "not really supported" means in this
> > context? Thanks!
>
> Huh, odd... I actually wrote the patch over a year ago, and was probably
> using the obsolete branch (before it got put back into
> coreboot/util).

Ah, that explains it.
> Clearly the FDC37M81x has support now. Should we just leave
> both of them enabled?

That, or maybe we can use some ID/revision field (or something
documented to be zero on one chip and documented to be 1 on the other
chip) to differentiate between them?
> (apologies for the hasty commit, i had an immediate use case)

No problem, I was just surprised by the reasoning. You did explain it,
and the good thing is that you can test any further differentiators for
the code.

Regards,
Carl-Daniel

[David Hendricks]

On Wed, Feb 16, 2011 at 5:43 PM, Carl-Daniel Hailfinger <
[e-mail]> wrote:
> > Clearly the FDC37M81x has support now. Should we just leave
> > both of them enabled?
>
> That, or maybe we can use some ID/revision field (or something
> documented to be zero on one chip and documented to be 1 on the other
> chip) to differentiate between them?

I couldn't find a FDC37M81x datasheet with those details, and the code
documents global register 0x21 as NANA.

I kinda brushed it off as one area where superiotool probably needs more
sophisticated probing methods.

On Wed, Feb 16, 2011 at 5:43 PM, Carl-Daniel Hailfinger <
[e-mail]> wrote:
> No problem, I was just surprised by the reasoning. You did explain it,
> and the good thing is that you can test any further differentiators for
> the code.

The ITE code uses a 16-bit identifier for this info... I wish the SMSC code
could do the same :-/