[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20260116143600.5aacf12b@bootlin.com>
Date: Fri, 16 Jan 2026 14:36:00 +0100
From: Herve Codina <herve.codina@...tlin.com>
To: David Gibson <david@...son.dropbear.id.au>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Ayush Singh <ayush@...gleboard.org>,
Geert Uytterhoeven <geert@...ux-m68k.org>,
devicetree-compiler@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, devicetree-spec@...r.kernel.org, Hui Pu
<hui.pu@...ealthcare.com>, Ian Ray <ian.ray@...ealthcare.com>, Luca
Ceresoli <luca.ceresoli@...tlin.com>, Thomas Petazzoni
<thomas.petazzoni@...tlin.com>
Subject: Re: [RFC PATCH 10/77] tests: Add basic metadata tests
On Thu, 15 Jan 2026 11:50:56 +1100
David Gibson <david@...son.dropbear.id.au> wrote:
> On Mon, Jan 12, 2026 at 03:19:00PM +0100, Herve Codina wrote:
> > This first test is related to local phandle references (FDT_REF_LOCAL
> > dtb tag).
> >
> > The test pattern used is
> > - Generate a dts (xxx.dts.dts) from an input dts
> > - Check this generated dts against expected contents
> > - Generate a dtb (xxx.dtb) from the same input dts
> > - Check this generated dtb against expected contents
> > - Generate a dts (xxx.dtb.dts) from the generated dtb
> > - Check this generated dts against expected contents
> > - Generate a dtb (xxx.dtb.dts.dtb) from the generated dts
> > - Check this generated dtb, expect the same contents as for xxx.dtb
> >
> > Even if only one meta-data feature is currently tested in this tests
> > introduction, use a loop in order to ease future addition consisting in
> > adding new input dts as soon as new meta-data feature are supported.
>
> As a rule of tumb, I prefer tests to be introduced in the same patch
> that introduces the feature tested. Usually, I don't care that much,
> but in a giant series like this, it would really help review (the
> tests help to document the feature being added without switching
> between patches).
Hum ok but it is worth noting that a feature needs several patches to be
fully supported.
In order to ease the review (maybe I was wrong), I chose to have distinct
patches for modification related to new dts keywords and for modification
related to new dtb tag and tried to have patches as small as possible.
The last patch of a new feature was a patch adding test.
I am open to remove patches that just add tests. In that case tests will be
added in the last patch related to a new feature. You still need to switch
between patches in that case.
Further more, during iteration, the last patch of a new feature could be
modified just because the test part is present in this patch even if other
part of the patch itself is not impacted.
I mean:
- patch 1: related to dts keyword
- patch 2: related to dtb + test
Update patch 1 will imply an update to patch 2.
I am not sure that this will ease the review.
To avoid that, I can merge all patches related to feature into only one
patch. This patch can be quite huge and I am not sure that it will be easy
to review it.
Once again, I am open to merging patches. Let me know what do you prefer
in terms of patch granularity.
Best regards,
Hervé
Powered by blists - more mailing lists