Linux-USB Archive on lore.kernel.org
 help / color / Atom feed
From: Krzysztof Kozlowski <krzk@kernel.org>
To: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: "Serge Semin" <fancer.lancer@gmail.com>,
	"Mathias Nyman" <mathias.nyman@intel.com>,
	"Felipe Balbi" <balbi@kernel.org>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Rob Herring" <robh+dt@kernel.org>,
	"Jason Cooper" <jason@lakedaemon.net>,
	"Andrew Lunn" <andrew@lunn.ch>,
	"Gregory Clement" <gregory.clement@bootlin.com>,
	"Sebastian Hesselbarth" <sebastian.hesselbarth@gmail.com>,
	"Kukjin Kim" <kgene@kernel.org>,
	"Santosh Shilimkar" <ssantosh@kernel.org>,
	"Shawn Guo" <shawnguo@kernel.org>, "Li Yang" <leoyang.li@nxp.com>,
	"Benoît Cousson" <bcousson@baylibre.com>,
	"Tony Lindgren" <tony@atomide.com>,
	"Patrice Chotard" <patrice.chotard@st.com>,
	"Maxime Ripard" <mripard@kernel.org>,
	"Chen-Yu Tsai" <wens@csie.org>, "Wei Xu" <xuwei5@hisilicon.com>,
	"Andy Gross" <agross@kernel.org>,
	"Bjorn Andersson" <bjorn.andersson@linaro.org>,
	"Alexey Malahov" <Alexey.Malahov@baikalelectronics.ru>,
	"Pavel Parkhomenko" <Pavel.Parkhomenko@baikalelectronics.ru>,
	"Manu Gautam" <mgautam@codeaurora.org>,
	"Roger Quadros" <rogerq@ti.com>,
	"Lad Prabhakar" <prabhakar.mahadev-lad.rj@bp.renesas.com>,
	"Yoshihiro Shimoda" <yoshihiro.shimoda.uh@renesas.com>,
	"Neil Armstrong" <narmstrong@baylibre.com>,
	"Kevin Hilman" <khilman@baylibre.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-snps-arc@lists.infradead.org, linux-mips@vger.kernel.org,
	linuxppc-dev@lists.ozlabs.org, linux-usb@vger.kernel.org,
	devicetree@vger.kernel.org,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-samsung-soc@vger.kernel.org"
	<linux-samsung-soc@vger.kernel.org>,
	linux-omap@vger.kernel.org, linux-arm-msm@vger.kernel.org
Subject: Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name
Date: Wed, 14 Oct 2020 22:04:32 +0200
Message-ID: <CAJKOXPcHi_=jea=0YrPNo4dh6k03+63Tc2Uo+sd0u8+XPdQjOw@mail.gmail.com> (raw)
In-Reply-To: <20201014171640.bup52mgaz4jvhtsy@mobilestation>

On Wed, 14 Oct 2020 at 19:16, Serge Semin
<Sergey.Semin@baikalelectronics.ru> wrote:
>
> On Wed, Oct 14, 2020 at 12:33:25PM +0200, Krzysztof Kozlowski wrote:
> > On Wed, 14 Oct 2020 at 12:23, Serge Semin
> > <Sergey.Semin@baikalelectronics.ru> wrote:
> > >
> > > In accordance with the DWC USB3 bindings the corresponding node name is
> > > suppose to comply with Generic USB HCD DT schema, which requires the USB
> > > nodes to have the name acceptable by the regexp: "^usb(@.*)?" . But a lot
> > > of the DWC USB3-compatible nodes defined in the ARM/ARM64 DTS files have
> > > name as "^dwc3@.*" or "^usb[1-3]@.*" or even "^dwusb@.*", which will cause
> > > the dtbs_check procedure failure. Let's fix the nodes naming to be
> > > compatible with the DWC USB3 DT schema to make dtbs_check happy.
> > >
> > > Note we don't change the DWC USB3-compatible nodes names of
> > > arch/arm64/boot/dts/apm/{apm-storm.dtsi,apm-shadowcat.dtsi} since the
> > > in-source comment says that the nodes name need to be preserved as
> > > "^dwusb@.*" for some backward compatibility.
> > >
> > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > >
> > > ---
> > >
> > > Please, test the patch out to make sure it doesn't brake the dependent DTS
> > > files. I did only a manual grepping of the possible nodes dependencies.
> >
>
> > 1. It is you who should compare the decompiled DTS, not us. For example:
> > $ for i in dts-old/*/*dtb dts-old/*/*/*dtb; do echo $i; crosc64
> > scripts/dtc/dtx_diff ${i} dts-new/${i#dts-old/} ; done
> >
> > $ for i in dts-old/*/*dtb dts-old/*/*/*dtb; do echo $i; crosc64
> > fdtdump ${i} > ${i}.fdt ; crosc64 fdtdump dts-new/${i#dts-old/} >
> > dts-new/${i#dts-old/}.fdt ; diff -ubB ${i}.fdt
> > dts-new/${i#dts-old/}.fdt ; done
>
> So basically you suggest first to compile the old and new dts files, then to
> de-compile them, then diff old and new fdt's, and visually compare the results.
> Personally it isn't that much better than what I did, since each old and new
> dtbs will for sure differ due to the node names change suggested in this patch.
> So it will lead to the visual debugging too, which isn't that effective. But
> your approach is still more demonstrative to make sure that I didn't loose any
> nodes redefinition, since in the occasion the old and new de-compiled nodes will
> differ not only by the node names but with an additional old named node.

My suggestion is to compare the entire, effective DTS after all
inclusions. Maybe you did it already, I don't know. The point is that
when you change node names in DTSI but you miss one in DTS, you end up
with two nodes. This is much easier to spot with dtxdiff or with
fdtdump (which behaves better for node moves).

Indeed it is still a visual comparison - if you have any ideas how to
automate it (e.g. ignore phandle changes), please share. It would
solve my testings as well. But asking others to test because you do
not want to check it is not the best way to handle such changes.

>
> So to speak thanks for suggesting it. I'll try it to validate the proposed
> changes.
>
> Two questions:
> 1) Any advise of a good inliner/command to compile all dtbs at once? Of course I
> can get all the updated dtsi'es, then find out all the dts'es which include
> them, then directly use dtc to compile the found dts'es... On the other hand I
> can just compile all dts'es, then compare old and new ones. The diff of the
> non-modified dtb'es will be just empty...

make dtbs
touch your dts or git stash pop
make dtbs
compare
diff for all unchanged will be simply empty, so easy to spot

> 2) What crosc64 is?

Ah, just an alias for cross compiling + ccache + kbuild out. I just
copied you my helpers, so you need to tweak them.

>
> >
> > 2. Split it per arm architectures (and proper subject prefix - not
> > "arch") and subarchitectures so maintainers can pick it up.
>
> Why? The changes are simple and can be formatted as a single patch. I've seen
> tons of patches submitted like that, accepted and then merged. What you suggest
> is just much more work, which I don't see quite required.

DTS changes go separate between arm64 and arm. There is nothing
unusual here - all changes are submitted like this.
Second topic is to split by subarchitectures which is necessary if you
want it to be picked up by maintainers. It also makes it easier to
review. Sure, without split ber subarchitectures this could be picked
up by SoC folks but you did not even CC them. So if you do not want to
split it per subarchitectures for maintainers and you do not CC SoC,
then how do you believe this should be picked up? Out of the regular
patch submission way? That's not how the changes are handled.

>
> >
> > 3. The subject title could be more accurate - there is no fix here
> > because there was no errors in the first place. Requirement of DWC
> > node names comes recently, so it is more alignment with dtschema.
> > Otherwise automatic-pickup-stable-bot might want to pick up... and it
> > should not go to stable.
>
> Actually it is a fix, because the USB DT nodes should have been named with "usb"
> prefix in the first place. Legacy DWC USB3 bindings didn't define the nodes
> naming, but implied to be "usb"-prefixed by the USB HCD schema. The Qualcomm
> DWC3 schema should have defined the sub-nodes as "dwc3@"-prefixed, which was
> wrong in the first place.

Not following the naming convention of DT spec which was loosely
enforced is not an error which should be "fixed". Simply wrong title.
This is an alignment with dtschema or correcting naming convention.
Not fixing errors.

>
> Regarding automatic-pickup-stable-bot if it exists I don't think it scans all the
> emails, but most likely the stable@vger.kernel.org list only or the emails
> having the "Fixes:" tag. If I am wrong please give me a link to the bot sources
> or refer to a doc where I can read about the way it works, to take it into
> account in future commits. Just to note I submitted patches which did some fixes,
> had the word "fix" in the subject but weren't selected to be backported to the
> stable kernel.

You mixed up bots. The regular stable bot picks commits with cc-stable
or with "Fixes". The auto-pickup bot picks all commits (not emails...
why would it look at emails?) looking like a fix. Wording could be one
of the hints used in the heuristic. Anyway, this is not a fix,
regardless of autosel, so the wording is not correct.

Just Google for AUTOSEL. You can then ask Sasha for sources...

> Anyway I don't really care that much about the subject text using the word "fix"
> or some else. So if you suggest some better alternative, I'd be glad to consider
> it.

I already did. One example is: alignment with dtschema.

Best regards,
Krzysztof

  reply index

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-14 10:13 [PATCH 00/20] dt-bindings: usb: Add generic USB HCD, xHCI, DWC USB3 DT schema Serge Semin
2020-10-14 10:13 ` [PATCH 01/20] dt-bindings: usb: usb-hcd: Convert generic USB properties to " Serge Semin
2020-10-16 18:33   ` Rob Herring
2020-10-14 10:13 ` [PATCH 02/20] dt-bindings: usb: usb-hcd: Add "otg-rev" property restriction Serge Semin
2020-10-16 18:33   ` Rob Herring
2020-10-14 10:13 ` [PATCH 03/20] dt-bindings: usb: usb-hcd: Add "ulpi/serial/hsic" PHY types Serge Semin
2020-10-16 18:34   ` Rob Herring
2020-10-14 10:13 ` [PATCH 04/20] dt-bindings: usb: usb-hcd: Add "tpl-support" property Serge Semin
2020-10-14 13:27   ` Rob Herring
2020-10-14 17:27     ` Serge Semin
2020-10-16 15:44       ` Rob Herring
2020-10-16 18:47   ` Rob Herring
2020-10-14 10:13 ` [PATCH 05/20] dt-bindings: usb: usb-hcd: Add generic "usb-phy" property Serge Semin
2020-10-16 18:47   ` Rob Herring
2020-10-14 10:13 ` [PATCH 06/20] dt-bindings: usb: Convert xHCI bindings to DT schema Serge Semin
2020-10-16 18:49   ` Rob Herring
2020-10-14 10:13 ` [PATCH 07/20] dt-bindings: usb: xhci: Add Broadcom STB v2 compatible device Serge Semin
2020-10-15 16:39   ` Florian Fainelli
2020-10-16 18:49   ` Rob Herring
2020-10-14 10:13 ` [PATCH 08/20] dt-bindings: usb: renesas-xhci: Refer to the usb-xhci.yaml file Serge Semin
2020-10-16 18:50   ` Rob Herring
2020-10-20  4:42   ` Yoshihiro Shimoda
2020-10-14 10:13 ` [PATCH 09/20] dt-bindings: usb: Convert DWC USB3 bindings to DT schema Serge Semin
2020-10-14 13:32   ` Rob Herring
2020-10-14 17:37     ` Serge Semin
2020-10-14 10:13 ` [PATCH 10/20] dt-bindings: usb: dwc3: Add interrupt-names property support Serge Semin
2020-10-16 18:50   ` Rob Herring
2020-10-14 10:13 ` [PATCH 11/20] dt-bindings: usb: dwc3: Add synopsys,dwc3 compatible string Serge Semin
2020-10-14 20:18   ` Krzysztof Kozlowski
2020-10-14 21:35     ` Serge Semin
2020-10-16 18:53       ` Rob Herring
2020-10-19 16:22         ` Serge Semin
2020-10-14 10:13 ` [PATCH 12/20] dt-bindings: usb: dwc3: Add Tx De-emphasis restrictions Serge Semin
2020-10-16 18:54   ` Rob Herring
2020-10-14 10:13 ` [PATCH 13/20] dt-bindings: usb: dwc3: Add Frame Length Adj restrictions Serge Semin
2020-10-16 18:54   ` Rob Herring
2020-10-14 10:13 ` [PATCH 14/20] dt-bindings: usb: meson-g12a-usb: Fix FL-adj property value Serge Semin
2020-10-16 18:54   ` Rob Herring
2020-10-16 20:46   ` Martin Blumenstingl
2020-10-14 10:13 ` [PATCH 15/20] dt-bindings: usb: meson-g12a-usb: Validate DWC2/DWC3 sub-nodes Serge Semin
2020-10-16 18:54   ` Rob Herring
2020-10-16 20:47   ` Martin Blumenstingl
2020-10-14 10:13 ` [PATCH 16/20] dt-bindings: usb: keystone-dwc3: Validate DWC3 sub-node Serge Semin
2020-10-16 18:55   ` Rob Herring
2020-10-14 10:13 ` [PATCH 17/20] dt-bindings: usb: qcom,dwc3: " Serge Semin
2020-10-16 18:55   ` Rob Herring
2020-10-14 10:14 ` [PATCH 18/20] arch: dts: Fix EHCI/OHCI DT nodes name Serge Semin
2020-10-14 18:00   ` Florian Fainelli
2020-10-14 18:11     ` Serge Semin
2020-10-14 18:41       ` Florian Fainelli
2020-10-14 21:21         ` Serge Semin
2020-10-14 18:56   ` Paul Cercueil
2020-10-14 19:12     ` Alexey Brodkin
2020-10-15  8:05   ` Amelie DELAUNAY
2020-10-15 16:37   ` Florian Fainelli
2020-10-16  7:08   ` Alexandre Torgue
2020-10-16  8:52     ` Serge Semin
2020-10-14 10:14 ` [PATCH 19/20] arch: dts: Fix xHCI " Serge Semin
2020-10-14 10:14 ` [PATCH 20/20] arch: dts: Fix DWC USB3 " Serge Semin
2020-10-14 10:33   ` Krzysztof Kozlowski
2020-10-14 17:16     ` Serge Semin
2020-10-14 20:04       ` Krzysztof Kozlowski [this message]
2020-10-14 23:51         ` Serge Semin
2020-10-15  6:14           ` Krzysztof Kozlowski
2020-10-15 14:15             ` Serge Semin
2020-10-14 14:09   ` Felipe Balbi
2020-10-14 14:37     ` Serge Semin
2020-10-14 18:35       ` Rob Herring
2020-10-14 21:22         ` Serge Semin
2020-10-15 10:15       ` Felipe Balbi
2020-10-15 13:53         ` Serge Semin
2020-10-15 14:04         ` Maxime Ripard
2020-10-14 17:39 ` [PATCH 00/20] dt-bindings: usb: Add generic USB HCD, xHCI, DWC USB3 DT schema Serge Semin

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='CAJKOXPcHi_=jea=0YrPNo4dh6k03+63Tc2Uo+sd0u8+XPdQjOw@mail.gmail.com' \
    --to=krzk@kernel.org \
    --cc=Alexey.Malahov@baikalelectronics.ru \
    --cc=Pavel.Parkhomenko@baikalelectronics.ru \
    --cc=Sergey.Semin@baikalelectronics.ru \
    --cc=agross@kernel.org \
    --cc=andrew@lunn.ch \
    --cc=balbi@kernel.org \
    --cc=bcousson@baylibre.com \
    --cc=bjorn.andersson@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=fancer.lancer@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=gregory.clement@bootlin.com \
    --cc=jason@lakedaemon.net \
    --cc=kgene@kernel.org \
    --cc=khilman@baylibre.com \
    --cc=leoyang.li@nxp.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=linux-snps-arc@lists.infradead.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mathias.nyman@intel.com \
    --cc=mgautam@codeaurora.org \
    --cc=mripard@kernel.org \
    --cc=narmstrong@baylibre.com \
    --cc=patrice.chotard@st.com \
    --cc=prabhakar.mahadev-lad.rj@bp.renesas.com \
    --cc=robh+dt@kernel.org \
    --cc=rogerq@ti.com \
    --cc=sebastian.hesselbarth@gmail.com \
    --cc=shawnguo@kernel.org \
    --cc=ssantosh@kernel.org \
    --cc=tony@atomide.com \
    --cc=wens@csie.org \
    --cc=xuwei5@hisilicon.com \
    --cc=yoshihiro.shimoda.uh@renesas.com \
    /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

Linux-USB Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-usb/0 linux-usb/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-usb linux-usb/ https://lore.kernel.org/linux-usb \
		linux-usb@vger.kernel.org
	public-inbox-index linux-usb

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-usb


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git