Re: [PATCH] tests/qtest: npcm7xx-emc-test: Skip checking MAC

[Patrick Venture]

On Thu, Oct 6, 2022 at 10:58 AM Patrick Venture  wrote:

>
>
> On Tue, Sep 20, 2022 at 12:00 AM Thomas Huth  wrote:
>
>> On 20/09/2022 00.37, Patrick Venture wrote:
>> >
>> >
>> > On Mon, Sep 19, 2022 at 5:44 AM Thomas Huth > > > 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 > wuhao...@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
>>
>>
>>

Re: [PATCH] tests/qtest: npcm7xx-emc-test: Skip checking MAC

[Patrick Venture]

On Tue, Sep 20, 2022 at 12:00 AM Thomas Huth  wrote:

> On 20/09/2022 00.37, Patrick Venture wrote:
> >
> >
> > On Mon, Sep 19, 2022 at 5:44 AM Thomas Huth  > > 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  wuhao...@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
>
>
>

Re: [PATCH] tests/qtest: npcm7xx-emc-test: Skip checking MAC

[Thomas Huth]

On 20/09/2022 00.37, Patrick Venture wrote:



On Mon, Sep 19, 2022 at 5:44 AM Thomas Huth > 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 mailto:wuhao...@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

Re: [PATCH] tests/qtest: npcm7xx-emc-test: Skip checking MAC

[Patrick Venture]

On Mon, Sep 19, 2022 at 5:44 AM Thomas Huth  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 
> > 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 
> > ---
> >   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
>
>

Re: [PATCH] tests/qtest: npcm7xx-emc-test: Skip checking MAC

[Thomas Huth]

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 
Change-Id: I02426e39bdab33ceedd42c49d233e8680d4ec058


What's that change-id good for?


Signed-off-by: Patrick Venture 
---
  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

[PATCH] tests/qtest: npcm7xx-emc-test: Skip checking MAC

[Patrick Venture]

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 
Change-Id: I02426e39bdab33ceedd42c49d233e8680d4ec058
Signed-off-by: Patrick Venture 
---
 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), ==,
-- 
2.37.2.789.g6183377224-goog