All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rob Herring <robh@kernel.org>
To: Albert Yang <yangzh0906@thundersoft.com>
Cc: krzk@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 6/8] arm64: dts: bst: add support for Black Sesame Technologies C1200 CDCU1.0 board
Date: Wed, 2 Jul 2025 09:19:57 -0500	[thread overview]
Message-ID: <20250702141957.GA1416711-robh@kernel.org> (raw)
In-Reply-To: <20250702123133.3613126-1-yangzh0906@thundersoft.com>

On Wed, Jul 02, 2025 at 08:31:33PM +0800, Albert Yang wrote:
> Hi Krzysztof,
> 
> Thank you for your detailed review and feedback. I have addressed all the issues you mentioned:
> 
> > This is messed. SoB does not go to changelog. Apply your patch and look
> > at result - do you see SoB? No, because changelog is stripped.
> > submitting patches explains how this is supposed to look like.
> 
> Fixed. Moved Signed-off-by lines to the correct position in commit message, 
> outside of the changelog section.
> 
> > Nothing improved. I asked to follow DTS coding style in ordering.
> 
> Fixed. Reordered all nodes according to DTS coding style:
> - Root level nodes: alphabetically ordered (clk_mmc → cpus → psci → soc → timer)
> - SoC nodes: ordered by address (uart0@20008000 → mmc0@22200000 → gic@32800000)
> - Applied consistent ordering throughout the dtsi file
> 
> > l2-cache. Otherwise it is incomplete, so add the second one.
> 
> Fixed. Renamed l2-cache-1 to l2-cache as per standard naming convention.
> 
> > Why do you have multiple memory nodes, not one?
> 
> Fixed. Consolidated multiple memory nodes into a single memory node with 
> multiple reg entries as required by Device Tree specification:
> 
> Before (incorrect):
>   memory@800151000 { reg = <0x8 0x00151000 0x0 0x1000>; };
>   memory@800254000 { reg = <0x8 0x00254000 0x0 0x1000>; };
>   ...
> 
> After (correct):
>   memory@800151000 {
>     reg = <0x8 0x00151000 0x0 0x1000>,
>           <0x8 0x00254000 0x0 0x1000>,

These are very odd. Are these really main memory vs. some on chip SRAM 
or some other specific purpose? 

A 4KB block doesn't really work if the OS uses 16 or 64KB pages, but I 
guess that would be up to the OS to ignore them.

>           <0x8 0x10000000 0x0 0x30000000>,
>           <0x8 0xc0000000 0x1 0x0>,
>           <0xc 0x00000000 0x0 0x40000000>;
>   };

  reply	other threads:[~2025-07-02 14:19 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-28  8:54 [PATCH v1 0/9] arm64: Introduce Black Sesame Technologies C1200 SoC and CDCU1.0 board Albert Yang
2025-07-02  9:44 ` [PATCH v2 0/8] " Albert Yang
2025-07-02  9:44   ` [PATCH v2 1/8] dt-bindings: vendor-prefixes: Add Black Sesame Technologies Co., Ltd Albert Yang
2025-07-02 10:24     ` Krzysztof Kozlowski
2025-07-03  5:02       ` Albert Yang
2025-07-02  9:44   ` [PATCH v2 2/8] dt-bindings: arm: add Black Sesame Technologies (bst) SoC Albert Yang
2025-07-02  9:44   ` [PATCH v2 3/8] arm64: Kconfig: add ARCH_BST for bst silicons Albert Yang
2025-07-02 12:21     ` Krzysztof Kozlowski
2025-07-03  9:22       ` Albert Yang
2025-07-02  9:44   ` [PATCH v2 4/8] dt-bindings: mmc: add binding for BST DWCMSHC SDHCI controller Albert Yang
2025-07-02 13:28     ` Rob Herring (Arm)
2025-07-03  4:36       ` Albert Yang
2025-07-02 14:23     ` Rob Herring
2025-07-03  3:27       ` Albert Yang
2025-07-02  9:44   ` [PATCH v2 5/8] mmc: sdhci: add Black Sesame Technologies BST C1200 controller driver Albert Yang
2025-07-02 10:40     ` Arnd Bergmann
2025-07-11  5:55       ` Albert Yang
2025-07-11  6:55         ` Arnd Bergmann
2025-08-08  8:39           ` Albert Yang
2025-08-08  9:35             ` Arnd Bergmann
2025-07-02 10:47     ` Krzysztof Kozlowski
2025-07-02  9:44   ` [PATCH v2 6/8] arm64: dts: bst: add support for Black Sesame Technologies C1200 CDCU1.0 board and defconfig Albert Yang
2025-07-02 10:30     ` Krzysztof Kozlowski
2025-07-02 12:31       ` [PATCH v2 6/8] arm64: dts: bst: add support for Black Sesame Technologies C1200 CDCU1.0 board Albert Yang
2025-07-02 14:19         ` Rob Herring [this message]
2025-08-12  9:47           ` Albert Yang
2025-08-12 11:01           ` [PATCH v2 6/8] arm64: dts: bst: add support for Black Sesame Technologies C1200 CDCU1.0 board and defconfig Albert Yang
2025-07-02 12:15     ` Robin Murphy
2025-07-02  9:44   ` [PATCH v2 7/8] arm64: defconfig: enable BST C1200 DWCMSHC SDHCI controller Albert Yang
2025-07-02 10:25     ` Krzysztof Kozlowski
2025-07-02  9:44   ` [PATCH v2 8/8] MAINTAINERS: add and consolidate Black Sesame Technologies (BST) ARM SoC support Albert Yang

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=20250702141957.GA1416711-robh@kernel.org \
    --to=robh@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=krzk@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=yangzh0906@thundersoft.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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.