[PATCH] fix MSR code in flashrom

[Stefan Reinauer]

See patch... With this flashrom won't try to open /dev/cpu/x/msr on
non-linux OSes...

Stefan

[Carl-Daniel Hailfinger]

On 11.08.2009 15:00, Stefan Reinauer wrote:
> See patch... With this flashrom won't try to open /dev/cpu/x/msr on
> non-linux OSes...
>
> Fix up MSR handling in flashrom to support more OSes than Linux.
>
> Signed-off-by: Stefan Reinauer

I'm not totally happy with the abstraction and think getting MSR access
should be part of getting IO permissions, but this is a good step forward.

Acked-by: Carl-Daniel Hailfinger

It would be nice if you could address the one comment below, either with
an updated patch or a second fixup patch.

Regards,
Carl-Daniel
> Index: chipset_enable.c
>
> --- chipset_enable.c	(revision 676)
> +++ chipset_enable.c	(working copy)
> @@ -527,92 +527,35 @@
>  static int enable_flash_cs5536(struct pci_dev *dev, const char *name)
>  {
>  #define MSR_RCONF_DEFAULT	0x1808
>  #define MSR_NORF_CTL		0x51400018
> int fd_msr;
> unsigned char buf[8];
> msr_t msr;
> fd_msr = open("/dev/cpu/0/msr", O_RDWR);
> if (fd_msr == -1) {
> perror("open(/dev/cpu/0/msr)");
> printf("Cannot operate on MSR. Did you run 'modprobe msr'?\n");
> return -1;
> }
> /* Geode only has a single core */
> setup_cpu_msr(0);

Missing error handling for the setup case. rdmsr and wrmsr don't have to
be checked after setup was successful.

[Stefan Reinauer]

On 8/11/09 3:43 PM, Carl-Daniel Hailfinger wrote:
> On 11.08.2009 15:00, Stefan Reinauer wrote:
> > See patch... With this flashrom won't try to open /dev/cpu/x/msr on
> > non-linux OSes...
> >
> > Fix up MSR handling in flashrom to support more OSes than Linux.
> >
> > Signed-off-by: Stefan Reinauer
>
> I'm not totally happy with the abstraction and think getting MSR access
> should be part of getting IO permissions, but this is a good step forward.

I considered that. BUT: Why make code run every time, if it's only
needed on 1% or less of the machines.

Stefan

[Carl-Daniel Hailfinger]

On 11.08.2009 15:59, Stefan Reinauer wrote:
> On 8/11/09 3:43 PM, Carl-Daniel Hailfinger wrote:
> > On 11.08.2009 15:00, Stefan Reinauer wrote:
> > > See patch... With this flashrom won't try to open /dev/cpu/x/msr on
> > > non-linux OSes...
> > >
> > > Fix up MSR handling in flashrom to support more OSes than Linux.
> > >
> > > Signed-off-by: Stefan Reinauer
> >
> > I'm not totally happy with the abstraction and think getting MSR access
> > should be part of getting IO permissions, but this is a good step forward.
>
> I considered that. BUT: Why make code run every time, if it's only
> needed on 1% or less of the machines.

True. Go ahead.

Regards,
Carl-Daniel

[Carl-Daniel Hailfinger]

On 11.08.2009 15:43, Carl-Daniel Hailfinger wrote:
> On 11.08.2009 15:00, Stefan Reinauer wrote:
> > See patch... With this flashrom won't try to open /dev/cpu/x/msr on
> > non-linux OSes...
> >
> > Fix up MSR handling in flashrom to support more OSes than Linux.
> >
> > Signed-off-by: Stefan Reinauer
>
> Acked-by: Carl-Daniel Hailfinger

Committed by Stefan with changes in r677.

Regards,
Carl-Daniel