All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] tests/qtest: npcm7xx-emc-test: Skip checking MAC
@ 2022-09-06 16:31 Patrick Venture
  2022-09-19 12:44 ` Thomas Huth
  0 siblings, 1 reply; 6+ messages in thread
From: Patrick Venture @ 2022-09-06 16:31 UTC (permalink / raw)
  To: thuth, lvivier
  Cc: hskinnemoen, kfting, qemu-arm, qemu-devel, Patrick Venture, Hao Wu

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
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), ==,
-- 
2.37.2.789.g6183377224-goog



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] tests/qtest: npcm7xx-emc-test: Skip checking MAC
  2022-09-06 16:31 [PATCH] tests/qtest: npcm7xx-emc-test: Skip checking MAC Patrick Venture
@ 2022-09-19 12:44 ` Thomas Huth
  2022-09-19 22:37   ` Patrick Venture
  0 siblings, 1 reply; 6+ messages in thread
From: Thomas Huth @ 2022-09-19 12:44 UTC (permalink / raw)
  To: Patrick Venture, lvivier
  Cc: hskinnemoen, kfting, qemu-arm, qemu-devel, Hao Wu

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



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] tests/qtest: npcm7xx-emc-test: Skip checking MAC
  2022-09-19 12:44 ` Thomas Huth
@ 2022-09-19 22:37   ` Patrick Venture
  2022-09-20  7:00     ` Thomas Huth
  0 siblings, 1 reply; 6+ messages in thread
From: Patrick Venture @ 2022-09-19 22:37 UTC (permalink / raw)
  To: Thomas Huth
  Cc: Laurent Vivier, Havard Skinnemoen, CS20 KFTing, qemu-arm,
	QEMU Developers, Hao Wu

[-- Attachment #1: Type: text/plain, Size: 1903 bytes --]

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
>
>

[-- Attachment #2: Type: text/html, Size: 2931 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] tests/qtest: npcm7xx-emc-test: Skip checking MAC
  2022-09-19 22:37   ` Patrick Venture
@ 2022-09-20  7:00     ` Thomas Huth
  2022-10-06 17:58       ` Patrick Venture
  0 siblings, 1 reply; 6+ messages in thread
From: Thomas Huth @ 2022-09-20  7:00 UTC (permalink / raw)
  To: Patrick Venture
  Cc: Laurent Vivier, Havard Skinnemoen, CS20 KFTing, qemu-arm,
	QEMU Developers, Hao Wu

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




^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] tests/qtest: npcm7xx-emc-test: Skip checking MAC
  2022-09-20  7:00     ` Thomas Huth
@ 2022-10-06 17:58       ` Patrick Venture
  2022-10-06 17:59         ` Patrick Venture
  0 siblings, 1 reply; 6+ messages in thread
From: Patrick Venture @ 2022-10-06 17:58 UTC (permalink / raw)
  To: Thomas Huth
  Cc: Laurent Vivier, Havard Skinnemoen, CS20 KFTing, qemu-arm,
	QEMU Developers, Hao Wu

[-- Attachment #1: Type: text/plain, Size: 1655 bytes --]

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
>
>
>

[-- Attachment #2: Type: text/html, Size: 2655 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] tests/qtest: npcm7xx-emc-test: Skip checking MAC
  2022-10-06 17:58       ` Patrick Venture
@ 2022-10-06 17:59         ` Patrick Venture
  0 siblings, 0 replies; 6+ messages in thread
From: Patrick Venture @ 2022-10-06 17:59 UTC (permalink / raw)
  To: Thomas Huth
  Cc: Laurent Vivier, Havard Skinnemoen, CS20 KFTing, qemu-arm,
	QEMU Developers, Hao Wu

[-- Attachment #1: Type: text/plain, Size: 1913 bytes --]

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
>>
>>
>>

[-- Attachment #2: Type: text/html, Size: 3378 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-10-06 18:37 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-06 16:31 [PATCH] tests/qtest: npcm7xx-emc-test: Skip checking MAC Patrick Venture
2022-09-19 12:44 ` Thomas Huth
2022-09-19 22:37   ` Patrick Venture
2022-09-20  7:00     ` Thomas Huth
2022-10-06 17:58       ` Patrick Venture
2022-10-06 17:59         ` Patrick Venture

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.