public inbox for linux-clk@vger.kernel.org
 help / color / mirror / Atom feed
From: Stephen Boyd <sboyd@kernel.org>
To: Audra Mitchell <audra@redhat.com>,
	Donald Zickus <dzickus@redhat.com>,
	msalter@redhat.com
Cc: Nico Pache <npache@redhat.com>,
	KUnit Development <kunit-dev@googlegroups.com>,
	linux-clk@vger.kernel.org, Shuah Khan <skhan@linuxfoundation.org>,
	Audra Mitchell <aubaker@redhat.com>
Subject: Re: [Bug Report] Multiple S390x KUNIT clk failures
Date: Wed, 29 May 2024 12:39:34 -0700	[thread overview]
Message-ID: <076c65b6247cc0ddbae792f8f414be89.sboyd@kernel.org> (raw)
In-Reply-To: <7ee38bb411e20721c1d1ebdb0455a626885e1cb8.camel@redhat.com>

Quoting msalter@redhat.com (2024-05-28 15:53:48)
> On Tue, 2024-05-28 at 16:49 -0400, Audra Mitchell wrote:
> > 
> > I spent some time last week or so working on debugging these failures and I 
> > believe I have found the problem. I reached out to Malk Salter for advice on 
> > the best way to move forward with a fix on Friday the 17th, but he was on 
> > PTO for the last week. I was waiting for his reply before I replied to this 
> > thread. 
> > 
> > Also as a side note, I also ran into the same issue as Stephen with running
> > the kunit tests on s390 QEMU. I did not pursue resolving that issue and
> > instead just compiled the test as a module. 
> > 
> > For clarity, this is what I sent to Mark and were I believe the failure is
> > occurring:
> > 
> > The tests create a pretend clk-gate and use a "fake_reg" to emulate
> > the expected behavior of the clk_gate->reg. I added some debug
> > statements to the driver and noticed that the reg changes after
> > initialization to -1. I also noticed that we call this to read the
> > data in the clk-gate->reg:
> > 
> > static inline u32 clk_gate_readl(struct clk_gate *gate)
> > {
> >         if (gate->flags & CLK_GATE_BIG_ENDIAN)
> >                 return ioread32be(gate->reg);
> > 
> >         return readl(gate->reg);
> > }
> > 
> > However, it does not look like ioread32be is defined for s390, so
> 
> It is defined. arch/s390/include/asm/io.h defines:
> 
>    #define __raw_readl  zpci_read_u32
> 
> and then includes include/asm-generic/io.h which has:
> 
> static inline u32 readl(const volatile void __iomem *addr)
> {
>         u32 val;
> 
>         log_read_mmio(32, addr, _THIS_IP_, _RET_IP_);
>         __io_br();
>         val = __le32_to_cpu((__le32 __force)__raw_readl(addr));
>         __io_ar(val);
>         log_post_read_mmio(val, 32, addr, _THIS_IP_, _RET_IP_);
>         return val;
> }
> ...
> static inline u32 ioread32be(const volatile void __iomem *addr)
> {
>         return swab32(readl(addr));
> }
> 
> which should do the right thing (s390 being BE and readl() is for 32-bit LE reads).
> 
> But I don't know the s390 compiler or ISA, so I'm not sure where the zpci_load
> is coming from.
> 

So the problem is that the zpci_read_u32() fails and returns -1?

This test isn't the best because it uses fakes iomem and architectures
may not like that. We really need to implement something in KUnit core
to allocate a fake iomem region and then plumb that through all the
architectures so that the iomem functions like readl, writel, etc. go a
different direction when the pointer is for the fake region.

Probably the best thing to do in the short term here is to prevent this
test from running on S390 via Kconfig.

  reply	other threads:[~2024-05-29 19:39 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-14  1:08 [Bug Report] Multiple S390x KUNIT clk failures Nico Pache
2024-05-14  3:38 ` Stephen Boyd
2024-05-14  7:14   ` Nico Pache
2024-05-14 22:04     ` Stephen Boyd
     [not found]       ` <CAK18DXZyEHZ=1TC52kQQ89gscFLph0e_4zB_bt=DTwR-A=0UPA@mail.gmail.com>
2024-05-28 18:49         ` Donald Zickus
2024-05-28 20:49           ` Audra Mitchell
2024-05-28 22:53             ` msalter
2024-05-29 19:39               ` Stephen Boyd [this message]
2024-05-30 20:12                 ` Audra Mitchell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=076c65b6247cc0ddbae792f8f414be89.sboyd@kernel.org \
    --to=sboyd@kernel.org \
    --cc=aubaker@redhat.com \
    --cc=audra@redhat.com \
    --cc=dzickus@redhat.com \
    --cc=kunit-dev@googlegroups.com \
    --cc=linux-clk@vger.kernel.org \
    --cc=msalter@redhat.com \
    --cc=npache@redhat.com \
    --cc=skhan@linuxfoundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox