Re: [PATCH 00/14] mips: dts: ralink: mt7621: improve DTS style

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2024-03-17 17:29, Krzysztof Kozlowski wrote:
On 17/03/2024 16:22, Justin Swartz wrote:
On 2024-03-17 17:10, Krzysztof Kozlowski wrote:
On 16/03/2024 16:49, Sergio Paracuellos wrote:
On Sat, Mar 16, 2024 at 5:54 AM Justin Swartz
<justin.swartz@xxxxxxxxxxxxxxxx> wrote:

This set of patches was created with the intention of cleaning up
arch/mips/boot/dts/ralink/mt7621.dtsi so that it is aligned with
the Devicetree Sources (DTS) Coding Style [1] [2] guide.

[1] Documentation/devicetree/bindings/dts-coding-style.rst

[2] https://docs.kernel.org/devicetree/bindings/dts-coding-style.html

Justin Swartz (14):
  mips: dts: ralink: mt7621: reorder cpu node attributes
  mips: dts: ralink: mt7621: reorder cpuintc node attributes
  mips: dts: ralink: mt7621: reorder mmc regulator attributes
  mips: dts: ralink: mt7621: reorder sysc node attributes
  mips: dts: ralink: mt7621: reorder gpio node attributes
  mips: dts: ralink: mt7621: reorder i2c node attributes
  mips: dts: ralink: mt7621: reorder spi0 node attributes
  mips: dts: ralink: mt7621: move pinctrl and sort its children
  mips: dts: ralink: mt7621: reorder mmc node attributes
  mips: dts: ralink: mt7621: reorder gic node attributes
  mips: dts: ralink: mt7621: reorder ethernet node attributes and
kids
  mips: dts: ralink: mt7621: reorder pcie node attributes and
children
  mips: dts: ralink: mt7621: reorder pci?_phy attributes

These are all simple cleanups for the same file. It's one patch, not
15.

I agree these are all simple cleanups.

Even though the cleanup pattern was the same, or very similar,
for each node affected, the intention was to isolate each change
to a single node (or a grouping of nodes of that seemed logical
to me) so that if anyone had any objections, the discussion would
be easier to follow in subthreads identifiable by patch names (and

Objections to what? Coding style? Coding style is defined so you either
implement it or not... and even if someone disagrees with one line swap,
why it cannot be done like for every contribution: inline?

I had been asked to include empty lines when I had left them out when
I had contributed a patch regarding the serial nodes, which resulted in
a second version of that patch.


Organize your patches how described in submitting patches: one per
logical change. Logical change is to reorder all properties in one file,
without functional impact.

If I had accidentally deleted or modified an attribute in the process
of cleanup, this could have had a functional impact. It's easier to
notice this sort of omission when the wall of text you're confronted
with is as small as possible, and not multiple pages long.


But if there're no objections and it lessens the burden on
maintainers upstream to have less patches to apply, then I have no
problem combining them into a single patch.


Yeah, one review response instead of 14 responses... One commit in the
history instead of 14.

I agree that 1 commit vs 14 is better.

But for future reference: is it not enough for the Reviewed-by: trailer
to be sent in response to the cover letter of a patch set if a reviewer
has looked at the entire set?

Regards
Justin




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux