Re: [PATCH v3 2/5] dt-bindings: phy: Add ARTPEC-8 PCIe phy

Bjorn Helgaas
2022-06-21T21:14:01Z

On Mon, Jun 20, 2022 at 05:38:21PM +0900, Wangseok Lee wrote:
> On 17/06/2022 07:59, Krzysztof Kozlowski wrote:
> > On 13/06/2022 18:29, Wangseok Lee wrote:
> >> Add description to support Axis, ARTPEC-8 SoC.
> >> ARTPEC-8 is the SoC platform of Axis Communications
> >> and PCIe phy is designed based on SAMSUNG PHY.
> >
> > No improvements here. On v2 I gave you link pointing to specific
> > paragraph of our documentation which you need to apply - wrong wrapping.
> > Is there something unclear here?
> >
> > Please
> > do
> > not
> > wrap
> > in
> > different
> > style.
>
> I think i misunderstood your review comment.

Krzysztof was pointing out that your commit log:

Add description to support Axis, ARTPEC-8 SoC.
ARTPEC-8 is the SoC platform of Axis Communications
and PCIe phy is designed based on SAMSUNG PHY.

only fills about 50 columns, and if you run "git log", most commit logs
fill about 75 columns so that when git adds 4 spaces of indentation, they
fit nicely in an 80-column terminal and take advantage of the whole width.

It's easier to read when all the commit logs are roughly the same
width. So please wrap yours to something like this:

Add description to support Axis, ARTPEC-8 SoC. ARTPEC-8 is the SoC
platform of Axis Communications and PCIe PHY is designed based on Samsung
PHY.

The PCI driver the commit log is:

Add support Axis, ARTPEC-8 SoC.
ARTPEC-8 is the SoC platform of Axis Communications.

This is based on arm64 and support GEN4 & 2lane.
This PCIe controller is based on DesignWare Hardware core and uses DesignWa
re core functions to implement the driver.

"pcie-artpec6. c" supports artpec6 and artpec7 H/W.
artpec8 can not be expanded because H/W configuration is completely differe
nt from artpec6/7.
phy and sub controller are different.

This should be similarly rewrapped to fill 75 columns. The short lines are
a signal to the reader that "this is the last line of a paragraph, so
expect a new paragraph to follow."

But in commit logs, paragraphs are typically separated by blank lines, so a
short line followed not by a blank line but by text that *could* fit on the
previous short line is a confusing signal.

This similar to the Wikipedia style:
https://en.wikipedia.org/wiki/Wikipedia:Manual_of_Style/Layout#Paragraphs

The PCI driver commit log should also join "DesignWare" and "different",
which are currently split across lines.

> I will modify it as below.
> s/SAMSUNG PHY/Samsung phy

"PHY" is typically all caps in English text, e.g., see examples here:
https://en.wikipedia.org/wiki/Physical_layer#PHY

Bjorn