Thread (25 messages) 25 messages, 6 authors, 2021-02-09

Re: [PATCH v3 3/5] arm64: dts: ti: Add support for AM642 SoC

From: Nishanth Menon <nm@ti.com>
Date: 2021-01-21 18:47:22
Also in: linux-arm-kernel

On 12:13-20210121, Suman Anna wrote:
On 1/21/21 11:46 AM, Nishanth Menon wrote:
quoted
On 11:25-20210121, Suman Anna wrote:
quoted
On 1/20/21 2:25 PM, Dave Gerlach wrote:
quoted
The AM642 SoC belongs to the K3 Multicore SoC architecture platform,
providing advanced system integration to enable applications such as
Motor Drives, PLC, Remote IO and IoT Gateways.

Some highlights of this SoC are:
* Dual Cortex-A53s in a single cluster, two clusters of dual Cortex-R5F
  MCUs, and a single Cortex-M4F.
* Two Gigabit Industrial Communication Subsystems (ICSSG).
* Integrated Ethernet switch supporting up to a total of two external
  ports.
* PCIe-GEN2x1L, USB3/USB2, 2xCAN-FD, eMMC and SD, UFS, OSPI memory
  controller, QSPI, I2C, eCAP/eQEP, ePWM, ADC, among other
  peripherals.
* Centralized System Controller for Security, Power, and Resource
  Management (DMSC).

See AM64X Technical Reference Manual (SPRUIM2, Nov 2020)
for further details: https://www.ti.com/lit/pdf/spruim2

Introduce basic support for the AM642 SoC to enable ramdisk or MMC
boot. Introduce the sdhci, i2c, spi, and uart MAIN domain periperhals
under cbass_main and the i2c, spi, and uart MCU domain periperhals
under cbass_mcu.

Signed-off-by: Faiz Abbas <redacted>
Signed-off-by: Aswath Govindraju <redacted>
Hmm, there are a few pieces contributed by me, so please do add

Signed-off-by: Suman Anna <redacted>
Sure, thanks..

[...]
quoted
quoted
+
+	sdhci0: mmc@fa10000 {
+		compatible = "ti,am64-sdhci-8bit";
Hmm, I tried booting this series on top of 5.11-rc1 + Nishanth's current
ti-k3-dts-next. So, boot of these patches using this baseline fails when using
MMC rootfs, but is ok when using initramfs. This particular compatible and the
corresponding driver change are only in linux-next coming through couple of
patches from the MMC subsystem.

I am not sure why we would be including stuff that's dependent on some other
patches being merged from a different sub-system? Strangely, this ought to be
caught by dtbs_check, but it is not throwing any errors.

IMHO, these should only be added if you have no other external dependencies
especially when you are applying on a 5.11-rc baseline. The MMC pull-requests
would not go through arm-soc either.
Yes, I am aware of this - this is no different from integration we have
done in the past as well.. intent is to get bindings in via subsystem
trees and dts changes via arm-soc. I always insist that basic ramdisk
boot always in the basic introduction tree. mmc, nfs are add-ons that
get added via subsystem tree and I host the dts changes - in this case
every dts node binding is fine with subsystems already queued in
linux-next. And this is no different from what I have noticed on other
ARM SoC maintainer trees as well.
Hmm, this is kinda counter-intuitive. When I see a dts node, I am expecting the
What is counter intutive about a -next branch be tested against
linux-next tree?
required driver functionality to have been in (or atleast the binding as per
documentation), and not having to need to pick additional patches.

If the intent is to verify/test everything against linux-next and not the
baseline tree, then I guess this works. But in general, this kinda goes against
the rules set in submitting patches. For example, see
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/submitting-patches.rst#n44

And sure enough, this is what I get when I run checkpatch against your tree.
Also read https://www.kernel.org/doc/html/v5.11-rc4/process/2.Process.html#next-trees

You should probably realize linux-next is an integral part of the
process for us now.
WARNING: DT compatible string "ti,am64-sdhci-8bit" appears un-documented --
check ./Documentation/devicetree/bindings/
#347: FILE: arch/arm64/boot/dts/ti/k3-am64-main.dtsi:298:
+		compatible = "ti,am64-sdhci-8bit";

WARNING: DT compatible string "ti,am64-sdhci-4bit" appears un-documented --
check ./Documentation/devicetree/bindings/
#365: FILE: arch/arm64/boot/dts/ti/k3-am64-main.dtsi:316:
+		compatible = "ti,am64-sdhci-4bit";

you are saying basically - wait a complete kernel cycle after a driver
is introduced before we can even test a driver SoC support introduced
without an user in the same kernel version.. which is a disaster and bit-rot

OR

Let the subsystem maintainers also carry the patches for dts - which is
going to be another disaster and creates all kind of avoidable merge
conflicts.

OR

I stage a rc1 and rc2 merge cycle - which makes no sense - these nodes
dont get activated without a compatible match, which gets enabled only
when the corresponding subsystem is merged - they dont break existing
functionality even when the subsystem is merged, it just increases
the functionality as it should. (not to mention that all my follow on
kernel merge trees will have to be rc2 based - since majority nodes
will be introduced there)

dts already has a pain point that we are trying to manage logically
here, this is not a MISRA-C ASIL-D process - follow and exact verbatim
word to word process, that is just plain ridiculous.

When rc1 comes together, which is what my next branch is for, things
should be cohesive - we dont introduce regressions and broken trees -
which is exactly what the -next process makes sure happens.


Now, if you want to launch a product with my -next branch - go ahead, I
don't intent it for current kernel version - you are on your own.

If there is a real risk of upstream next-breaking - speakup with an
real example - All I care about is keeping upstream functional and
useable.

I recheck the linux-next tree almost daily basis for consistency, and I
do appreciate the concern here (and passion) - point is, I think we
might be a bit of an over-reaction if we just look at the other options
in front of us - not to mention, maybe drop the entire idea of dt coming
in from ARM SoC - let the subsystem member create merge conflict and
duke it out.. I don't think any of us want to see that kind of mayhem.

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help