[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZmD2c0rNz10KsPA0@smile.fi.intel.com>
Date: Thu, 6 Jun 2024 02:36:19 +0300
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Chia-I Wu <olvaffe@...il.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Dan Williams <dan.j.williams@...el.com>,
AKASHI Takahiro <takahiro.akashi@...aro.org>,
Dave Jiang <dave.jiang@...el.com>,
Alison Schofield <alison.schofield@...el.com>,
Baoquan He <bhe@...hat.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4] resource: add a simple test for walk_iomem_res_desc()
On Wed, Jun 05, 2024 at 03:52:26PM -0700, Chia-I Wu wrote:
> On Wed, Jun 5, 2024 at 3:10 PM Andy Shevchenko
> <andriy.shevchenko@...ux.intel.com> wrote:
> > On Wed, Jun 05, 2024 at 02:28:26PM -0700, Chia-I Wu wrote:
> > > This mainly tests that find_next_iomem_res() does not miss resources.
...
> > > + /* build the resource tree */
> > > + res[0] = DEFINE_RES_NAMED(root.start + 0x0000, 0x1000, "SYSRAM 1",
> > > + IORESOURCE_SYSTEM_RAM);
> > > + res[1] = DEFINE_RES_NAMED(root.start + 0x1000, 0x1000, "OTHER", 0);
> > > +
> > > + res[2] = DEFINE_RES_NAMED(root.start + 0x3000, 0x1000, "NESTED", 0);
> > > + res[3] = DEFINE_RES_NAMED(root.start + 0x3800, 0x0400, "SYSRAM 2",
> > > + IORESOURCE_SYSTEM_RAM);
> >
> > ...here is overlap with the previous resource.
> >
> > And here is the gap to the next one, in case we make that overlapping gone.
> >
> > > + res[4] = DEFINE_RES_NAMED(root.start + 0x4000, 0x1000, "SYSRAM 3",
> > > + IORESOURCE_SYSTEM_RAM);
> >
> > It wasn't the case in previous data. Please, elaborate what's going on here?
> The test data is chosen to be
>
> first interval: a matching resource (res[0])
> second interval: a non-matching resource (res[1])
> third interval: a hole
> fourth interval: a matching resource (res[3]) nested in a
> non-matching resource (res[2])
> fifth interval: a matching resource (res[4])
>
> The idea hasn't changed between revisions.
>
> res[3] went from a half of res[2] to a quarter of res[2] in v4. I
> guess it causes confusion if it is not viewed as a nested resource.
Okay, so far it's correct data from testing p.o.v.
Maybe you can add a comment on top explaining this layout?
...
> > And rather sending one version per 12h, take your time and think more about
> > test data. What are we testing? Are the testing data correct? Shouldn't we also
> > have negative test cases?
> The current choice of test data covers the most common patterns. Do
> you have other patterns you want to cover? I am new to the resource
> code and that's why I am largely reactive to review feedback.
Nope, seems okay to have what is there for the starter. Later on we might add
more if required. Just got confused.
--
With Best Regards,
Andy Shevchenko
Powered by blists - more mailing lists