| Message ID | 20220906163138.2831353-1-venture@google.com |
|---|---|
| State | New |
| Headers | show |
| Series | tests/qtest: npcm7xx-emc-test: Skip checking MAC | expand |
On 06/09/2022 18.31, Patrick Venture wrote: > The register tests walks all the registers to verify they are initially > 0 when appropriate. However, if the MAC address is set in the register > space, this should not be checked against 0. > > Reviewed-by: Hao Wu <wuhaotsh@google.com> > Change-Id: I02426e39bdab33ceedd42c49d233e8680d4ec058 What's that change-id good for? > Signed-off-by: Patrick Venture <venture@google.com> > --- > tests/qtest/npcm7xx_emc-test.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/tests/qtest/npcm7xx_emc-test.c b/tests/qtest/npcm7xx_emc-test.c > index 7c435ac915..207d8515b7 100644 > --- a/tests/qtest/npcm7xx_emc-test.c > +++ b/tests/qtest/npcm7xx_emc-test.c > @@ -378,7 +378,8 @@ static void test_init(gconstpointer test_data) > > #undef CHECK_REG > > - for (i = 0; i < NUM_CAMML_REGS; ++i) { > + /* Skip over the MAC address registers, which is BASE+0 */ > + for (i = 1; i < NUM_CAMML_REGS; ++i) { > g_assert_cmpuint(emc_read(qts, mod, REG_CAMM_BASE + i * 2), ==, > 0); > g_assert_cmpuint(emc_read(qts, mod, REG_CAML_BASE + i * 2), ==, Basically ack, but one question: Where should that non-zero MAC address come from / when did you hit a problem here? If QEMU is started without any mac settings at all (like it is done here), the register never contains a non-zero value, does it? Thomas
On Mon, Sep 19, 2022 at 5:44 AM Thomas Huth <thuth@redhat.com> wrote: > On 06/09/2022 18.31, Patrick Venture wrote: > > The register tests walks all the registers to verify they are initially > > 0 when appropriate. However, if the MAC address is set in the register > > space, this should not be checked against 0. > > > > Reviewed-by: Hao Wu <wuhaotsh@google.com> > > Change-Id: I02426e39bdab33ceedd42c49d233e8680d4ec058 > > What's that change-id good for? > Oops, sorry about that. I can send out a v2 without it, or during application someone can nicely trim it? :) > > > Signed-off-by: Patrick Venture <venture@google.com> > > --- > > tests/qtest/npcm7xx_emc-test.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/tests/qtest/npcm7xx_emc-test.c > b/tests/qtest/npcm7xx_emc-test.c > > index 7c435ac915..207d8515b7 100644 > > --- a/tests/qtest/npcm7xx_emc-test.c > > +++ b/tests/qtest/npcm7xx_emc-test.c > > @@ -378,7 +378,8 @@ static void test_init(gconstpointer test_data) > > > > #undef CHECK_REG > > > > - for (i = 0; i < NUM_CAMML_REGS; ++i) { > > + /* Skip over the MAC address registers, which is BASE+0 */ > > + for (i = 1; i < NUM_CAMML_REGS; ++i) { > > g_assert_cmpuint(emc_read(qts, mod, REG_CAMM_BASE + i * 2), ==, > > 0); > > g_assert_cmpuint(emc_read(qts, mod, REG_CAML_BASE + i * 2), ==, > > Basically ack, but one question: Where should that non-zero MAC address > come > from / when did you hit a problem here? If QEMU is started without any mac > settings at all (like it is done here), the register never contains a > non-zero value, does it? > So, there's a bug in the emc device presently where that value isn't set when it should be. I have that bug fixed, but for whatever reason, probably not enough caffeine, I didn't bundle the two patches together. > > Thomas > >
On 20/09/2022 00.37, Patrick Venture wrote: > > > On Mon, Sep 19, 2022 at 5:44 AM Thomas Huth <thuth@redhat.com > <mailto:thuth@redhat.com>> wrote: > > On 06/09/2022 18.31, Patrick Venture wrote: > > The register tests walks all the registers to verify they are initially > > 0 when appropriate. However, if the MAC address is set in the register > > space, this should not be checked against 0. > > > > Reviewed-by: Hao Wu <wuhaotsh@google.com <mailto:wuhaotsh@google.com>> > > Change-Id: I02426e39bdab33ceedd42c49d233e8680d4ec058 > > What's that change-id good for? > > > Oops, sorry about that. I can send out a v2 without it, or during > application someone can nicely trim it? :) I can take the patch through my qtest branch - I'll drop the line there. > Basically ack, but one question: Where should that non-zero MAC address > come > from / when did you hit a problem here? If QEMU is started without any mac > settings at all (like it is done here), the register never contains a > non-zero value, does it? > > > So, there's a bug in the emc device presently where that value isn't set > when it should be. I have that bug fixed, but for whatever reason, probably > not enough caffeine, I didn't bundle the two patches together. OK, makes sense now, thanks for the explanation! Thomas
On Tue, Sep 20, 2022 at 12:00 AM Thomas Huth <thuth@redhat.com> wrote: > On 20/09/2022 00.37, Patrick Venture wrote: > > > > > > On Mon, Sep 19, 2022 at 5:44 AM Thomas Huth <thuth@redhat.com > > <mailto:thuth@redhat.com>> wrote: > > > > On 06/09/2022 18.31, Patrick Venture wrote: > > > The register tests walks all the registers to verify they are > initially > > > 0 when appropriate. However, if the MAC address is set in the > register > > > space, this should not be checked against 0. > > > > > > Reviewed-by: Hao Wu <wuhaotsh@google.com <mailto: > wuhaotsh@google.com>> > > > Change-Id: I02426e39bdab33ceedd42c49d233e8680d4ec058 > > > > What's that change-id good for? > > > > > > Oops, sorry about that. I can send out a v2 without it, or during > > application someone can nicely trim it? :) > > I can take the patch through my qtest branch - I'll drop the line there. > > > Basically ack, but one question: Where should that non-zero MAC > address > > come > > from / when did you hit a problem here? If QEMU is started without > any mac > > settings at all (like it is done here), the register never contains a > > non-zero value, does it? > > > > > > So, there's a bug in the emc device presently where that value isn't set > > when it should be. I have that bug fixed, but for whatever reason, > probably > > not enough caffeine, I didn't bundle the two patches together. > > OK, makes sense now, thanks for the explanation! > The follow-on patch was just applied to arm.next, so I wanted to check if this was applied to your .next or if you wanted a v2. > > Thomas > > >
On Thu, Oct 6, 2022 at 10:58 AM Patrick Venture <venture@google.com> wrote: > > > On Tue, Sep 20, 2022 at 12:00 AM Thomas Huth <thuth@redhat.com> wrote: > >> On 20/09/2022 00.37, Patrick Venture wrote: >> > >> > >> > On Mon, Sep 19, 2022 at 5:44 AM Thomas Huth <thuth@redhat.com >> > <mailto:thuth@redhat.com>> wrote: >> > >> > On 06/09/2022 18.31, Patrick Venture wrote: >> > > The register tests walks all the registers to verify they are >> initially >> > > 0 when appropriate. However, if the MAC address is set in the >> register >> > > space, this should not be checked against 0. >> > > >> > > Reviewed-by: Hao Wu <wuhaotsh@google.com <mailto: >> wuhaotsh@google.com>> >> > > Change-Id: I02426e39bdab33ceedd42c49d233e8680d4ec058 >> > >> > What's that change-id good for? >> > >> > >> > Oops, sorry about that. I can send out a v2 without it, or during >> > application someone can nicely trim it? :) >> >> I can take the patch through my qtest branch - I'll drop the line there. >> >> > Basically ack, but one question: Where should that non-zero MAC >> address >> > come >> > from / when did you hit a problem here? If QEMU is started without >> any mac >> > settings at all (like it is done here), the register never contains >> a >> > non-zero value, does it? >> > >> > >> > So, there's a bug in the emc device presently where that value isn't >> set >> > when it should be. I have that bug fixed, but for whatever reason, >> probably >> > not enough caffeine, I didn't bundle the two patches together. >> >> OK, makes sense now, thanks for the explanation! >> > > The follow-on patch was just applied to arm.next, so I wanted to check if > this was applied to your .next or if you wanted a v2. > Nevermind, sorry for the spam - I already saw it in a PULL but forgot to update my internal tracking. Thanks! > > >> >> Thomas >> >> >>
diff --git a/tests/qtest/npcm7xx_emc-test.c b/tests/qtest/npcm7xx_emc-test.c index 7c435ac915..207d8515b7 100644 --- a/tests/qtest/npcm7xx_emc-test.c +++ b/tests/qtest/npcm7xx_emc-test.c @@ -378,7 +378,8 @@ static void test_init(gconstpointer test_data) #undef CHECK_REG - for (i = 0; i < NUM_CAMML_REGS; ++i) { + /* Skip over the MAC address registers, which is BASE+0 */ + for (i = 1; i < NUM_CAMML_REGS; ++i) { g_assert_cmpuint(emc_read(qts, mod, REG_CAMM_BASE + i * 2), ==, 0); g_assert_cmpuint(emc_read(qts, mod, REG_CAML_BASE + i * 2), ==,