Re: [flashrom] [PATCH] Allow DMI supported board enables with subsystem ID zero.

[Carl-Daniel Hailfinger]

On 29.01.2010 01:40, Michael Karcher wrote:
> --- a/board_enable.c
> +++ b/board_enable.c
> @@ -1312,7 +1312,8 @@ static struct board_pciid_enable *board_match_pci_card_ids(void)
>    struct board_pciid_enable *board = board_pciid_enables;
>
> for (; board->vendor_name; board++) {
> if (!board->first_card_vendor || !board->first_card_device)
> if (!board->first_card_vendor || 
>     (!board->first_card_device && !board->dmi_pattern))
> continue;
>
> if (!pci_card_find(board->first_vendor, board->first_device,

This code still requires a nonzero subsystem vendor even if a DMI
pattern is present. If that is intentional (I thought DMI would
eliminate subsystem vendor+device requirement), the patch is
Acked-by: Carl-Daniel Hailfinger

Regards,
Carl-Daniel

[PATCH] Allow DMI supported board enables with subsystem ID zero.

[Luc Verhaegen]

On Thu, Feb 04, 2010 at 01:58:53AM +0100, Carl-Daniel Hailfinger wrote:
> On 29.01.2010 01:40, Michael Karcher wrote:
> > --- a/board_enable.c
> > +++ b/board_enable.c
> > @@ -1312,7 +1312,8 @@ static struct board_pciid_enable *board_match_pci_card_ids(void)
> >    struct board_pciid_enable *board = board_pciid_enables;
> >
> > for (; board->vendor_name; board++) {
> > if (!board->first_card_vendor || !board->first_card_device)
> > if (!board->first_card_vendor || 
> >     (!board->first_card_device && !board->dmi_pattern))
> > continue;
> >
> > if (!pci_card_find(board->first_vendor, board->first_device,
>
> This code still requires a nonzero subsystem vendor even if a DMI
> pattern is present. If that is intentional (I thought DMI would
> eliminate subsystem vendor+device requirement), the patch is
> Acked-by: Carl-Daniel Hailfinger
>
> Regards,
> Carl-Daniel

It probably should be an || there.

Luc Verhaegen.

[PATCH] Allow DMI supported board enables with subsystem ID zero.

[Carl-Daniel Hailfinger]

On 04.02.2010 04:07, Luc Verhaegen wrote:
> On Thu, Feb 04, 2010 at 01:58:53AM +0100, Carl-Daniel Hailfinger wrote:
> > On 29.01.2010 01:40, Michael Karcher wrote:
> > > --- a/board_enable.c
> > > +++ b/board_enable.c
> > > @@ -1312,7 +1312,8 @@ static struct board_pciid_enable *board_match_pci_card_ids(void)
> > >    struct board_pciid_enable *board = board_pciid_enables;
> > >
> > > for (; board->vendor_name; board++) {
> > > if (!board->first_card_vendor || !board->first_card_device)
> > > if (!board->first_card_vendor || 
> > >     (!board->first_card_device && !board->dmi_pattern))
> > > continue;
> > >
> > > if (!pci_card_find(board->first_vendor, board->first_device,
> >
> > This code still requires a nonzero subsystem vendor even if a DMI
> > pattern is present. If that is intentional (I thought DMI would
> > eliminate subsystem vendor+device requirement), the patch is
> > Acked-by: Carl-Daniel Hailfinger
> >
> > Regards,
> > Carl-Daniel
>
> It probably should be an || there.

No, that would invert the logic and make DMI an absolute requirement for
all boards.

Regards,
Carl-Daniel

[PATCH] Allow DMI supported board enables with subsystem ID zero.

[Luc Verhaegen]

On Thu, Feb 04, 2010 at 04:44:16AM +0100, Carl-Daniel Hailfinger wrote:
> On 04.02.2010 04:07, Luc Verhaegen wrote:
> > On Thu, Feb 04, 2010 at 01:58:53AM +0100, Carl-Daniel Hailfinger wrote:
> > > On 29.01.2010 01:40, Michael Karcher wrote:
> > > > --- a/board_enable.c
> > > > +++ b/board_enable.c
> > > > @@ -1312,7 +1312,8 @@ static struct board_pciid_enable *board_match_pci_card_ids(void)
> > > >    struct board_pciid_enable *board = board_pciid_enables;
> > > >
> > > > for (; board->vendor_name; board++) {
> > > > if (!board->first_card_vendor || !board->first_card_device)
> > > > if (!board->first_card_vendor || 
> > > >     (!board->first_card_device && !board->dmi_pattern))
> > > > continue;
> > > >
> > > > if (!pci_card_find(board->first_vendor, board->first_device,
> > >
> > > This code still requires a nonzero subsystem vendor even if a DMI
> > > pattern is present. If that is intentional (I thought DMI would
> > > eliminate subsystem vendor+device requirement), the patch is
> > > Acked-by: Carl-Daniel Hailfinger
> > >
> > > Regards,
> > > Carl-Daniel
> >
> > It probably should be an || there.
>
> No, that would invert the logic and make DMI an absolute requirement for
> all boards.
>
> Regards,
> Carl-Daniel

Yes, coin just dropped too reading your answer to stepan. :)

Luc Verhaegen.

[PATCH] (fixed) Allow DMI supported board enables with subsystem ID zero.

[Michael Karcher]

This is needed for the Intel SE440BX-2 as well as the Asus P5A.

Thanks to Carl-Daniel Hailfinger for pointing out wrong logic in
the patch.

Zach: please retry with this one instead of the previous version of
this patch. You can unpatch the previous version by
patch -R < oldpatch.diff

Signed-off-by: Michael Karcher 
---
 board_enable.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/board_enable.c b/board_enable.c
index 40db653..66afab4 100644
--- a/board_enable.c
+++ b/board_enable.c
@@ -1312,7 +1312,8 @@ static struct board_pciid_enable *board_match_pci_card_ids(void)
    struct board_pciid_enable *board = board_pciid_enables;

for (; board->vendor_name; board++) {
-		if (!board->first_card_vendor || !board->first_card_device)
+		if (!board->first_card_vendor || 
+		    (!board->first_card_device && !board->dmi_pattern))
    		continue;

if (!pci_card_find(board->first_vendor, board->first_device,

[PATCH] Allow DMI supported board enables with subsystem ID zero.

[Michael Karcher]

Am Donnerstag, den 04.02.2010, 01:58 +0100 schrieb Carl-Daniel
Hailfinger:
> On 29.01.2010 01:40, Michael Karcher wrote:
> > --- a/board_enable.c
> > +++ b/board_enable.c
> > @@ -1312,7 +1312,8 @@ static struct board_pciid_enable *board_match_pci_card_ids(void)
> >    struct board_pciid_enable *board = board_pciid_enables;
> >
> > for (; board->vendor_name; board++) {
> > if (!board->first_card_vendor || !board->first_card_device)
> > if (!board->first_card_vendor || 
> >     (!board->first_card_device && !board->dmi_pattern))
> > continue;
> >
> > if (!pci_card_find(board->first_vendor, board->first_device,
>
> This code still requires a nonzero subsystem vendor even if a DMI
> pattern is present.

That condition is completely wrong. I meant (!board->first_card_vendor
&& !board->dmi_pattern). The idea is that a zero subsystem ID is fine
when matching with DMI, but the zero subsystem ID is still matched by
pci_card_find.
> If that is intentional (I thought DMI would
> eliminate subsystem vendor+device requirement),

No, DMI was not meant to make softer matching in PCI space, just to make
stronger matches. The patch is just to avoid bogus rejects on boards
with no subsystem IDs (meaning subsystem ID zero) at all.

Regards,
  Michael Karcher