mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Krzysztof Kozlowski <krzk@kernel.org>
To: Ryan Chen <ryan_chen@aspeedtech.com>,
	"benh@kernel.crashing.org" <benh@kernel.crashing.org>,
	"joel@jms.id.au" <joel@jms.id.au>,
	"andi.shyti@kernel.org" <andi.shyti@kernel.org>,
	"jk@codeconstruct.com.au" <jk@codeconstruct.com.au>,
	"robh@kernel.org" <robh@kernel.org>,
	"krzk+dt@kernel.org" <krzk+dt@kernel.org>,
	"conor+dt@kernel.org" <conor+dt@kernel.org>,
	"andrew@codeconstruct.com.au" <andrew@codeconstruct.com.au>,
	"p.zabel@pengutronix.de" <p.zabel@pengutronix.de>,
	"andriy.shevchenko@linux.intel.com"
	<andriy.shevchenko@linux.intel.com>,
	"naresh.solanki@9elements.com" <naresh.solanki@9elements.com>,
	"linux-i2c@vger.kernel.org" <linux-i2c@vger.kernel.org>,
	"openbmc@lists.ozlabs.org" <openbmc@lists.ozlabs.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-aspeed@lists.ozlabs.org" <linux-aspeed@lists.ozlabs.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v20 1/4] dt-bindings: i2c: Split AST2600 binding into a new YAML
Date: Wed, 29 Oct 2025 11:52:32 +0100	[thread overview]
Message-ID: <3c3287f6-1c5c-4c4d-9349-32665a5e1585@kernel.org> (raw)
In-Reply-To: <TY2PPF5CB9A1BE6597ECD46BD4CB7C5F09FF2FAA@TY2PPF5CB9A1BE6.apcprd06.prod.outlook.com>

On 29/10/2025 09:29, Ryan Chen wrote:
>> Subject: Re: [PATCH v20 1/4] dt-bindings: i2c: Split AST2600 binding into a new
>> YAML
>>
>> On 21/10/2025 03:35, Ryan Chen wrote:
>>> The AST2600 I2C controller is a new hardware design compared to the
>>> I2C controllers in previous ASPEED SoCs (e.g., AST2400, AST2500).
>>>
>>> It introduces new features such as:
>>>  - A redesigned register layout
>>>  - Separation between controller and target mode registers
>>>  - Transfer mode selection (byte, buffer, DMA)
>>>  - Support for a shared global register block for configuration
>>>
>>> Due to these fundamental differences, maintaining a separate
>>> devicetree binding file for AST2600 helps to clearly distinguish the
>>> hardware capabilities and configuration options from the older
>>> controllers.
>>>
>>> Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
>>> ---
>>>  .../devicetree/bindings/i2c/aspeed,i2c.yaml   |  3 +-
>>>  .../devicetree/bindings/i2c/ast2600-i2c.yaml  | 66
>>> +++++++++++++++++++
>>>  2 files changed, 67 insertions(+), 2 deletions(-)  create mode 100644
>>> Documentation/devicetree/bindings/i2c/ast2600-i2c.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
>>> b/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
>>> index 5b9bd2feda3b..d4e4f412feba 100644
>>> --- a/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
>>> +++ b/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
>>> @@ -4,7 +4,7 @@
>>>  $id: http://devicetree.org/schemas/i2c/aspeed,i2c.yaml#
>>>  $schema: http://devicetree.org/meta-schemas/core.yaml#
>>>
>>> -title: ASPEED I2C on the AST24XX, AST25XX, and AST26XX SoCs
>>> +title: ASPEED I2C on the AST24XX, AST25XX SoCs
>>>
>>>  maintainers:
>>>    - Rayn Chen <rayn_chen@aspeedtech.com> @@ -17,7 +17,6 @@
>>> properties:
>>>      enum:
>>>        - aspeed,ast2400-i2c-bus
>>>        - aspeed,ast2500-i2c-bus
>>> -      - aspeed,ast2600-i2c-bus
>>>
>>>    reg:
>>>      minItems: 1
>>> diff --git a/Documentation/devicetree/bindings/i2c/ast2600-i2c.yaml
>>> b/Documentation/devicetree/bindings/i2c/ast2600-i2c.yaml
>>
>> Why completely breaking naming? Please follow writing bindings carefully.
> 
> Will update 
> $id: "http://devicetree.org/schemas/i2c/aspeed,ast2600-i2c.yaml#"
>>
>>> new file mode 100644
>>> index 000000000000..6ddcec5decdc
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/i2c/ast2600-i2c.yaml
>>> @@ -0,0 +1,66 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/i2c/ast2600-i2c.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: ASPEED I2C on the AST26XX SoCs
>>> +
>>> +maintainers:
>>> +  - Ryan Chen <ryan_chen@aspeedtech.com>
>>> +
>>> +allOf:
>>> +  - $ref: /schemas/i2c/i2c-controller.yaml#
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - aspeed,ast2600-i2c-bus
>>> +
>>> +  reg:
>>> +    minItems: 1
>>
>> Why?
> 
> Will update as following.
> 
> reg:
>   minItems: 1
>   maxItems: 2


No. You changed nothing. Instead explain why this is flexible.

See writing bindings.


...


>>> +  bus-frequency:
>>> +    minimum: 500
>>> +    maximum: 4000000
>>> +    default: 100000
>>> +    description: frequency of the bus clock in Hz defaults to 100 kHz when
>> not
>>> +      specified
>>
>> Don't repeat constraints in free form text.
> 
> Will update
> clock-frequency:
>     description: Desired I2C bus frequency in Hz
>     default: 100000

Heh? You are making random set of changes like did not really read the
feedback. I not to repeat something. What is repeated? Constraints.
Where are the repeated "in free form text". What did you do? Dropped
constraints.

I don't know what to say.

Best regards,
Krzysztof

  reply	other threads:[~2025-10-29 10:52 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-21  1:35 [PATCH v20 0/4] Add ASPEED AST2600 I2C controller driver Ryan Chen
2025-10-21  1:35 ` [PATCH v20 1/4] dt-bindings: i2c: Split AST2600 binding into a new YAML Ryan Chen
2025-10-24  6:45   ` Krzysztof Kozlowski
2025-10-24  6:50     ` Krzysztof Kozlowski
2025-10-24  7:56       ` Jeremy Kerr
2025-10-24  8:06         ` Krzysztof Kozlowski
2025-10-24  8:35           ` Krzysztof Kozlowski
2025-10-24  8:41             ` Jeremy Kerr
2025-10-24 12:36               ` Ryan Chen
2025-10-24 12:43                 ` Krzysztof Kozlowski
2025-10-24  8:40           ` Jeremy Kerr
2025-10-24 12:30             ` Krzysztof Kozlowski
2025-10-24 12:47               ` Ryan Chen
2025-10-24 13:13                 ` Jeremy Kerr
2025-10-24  8:14   ` Krzysztof Kozlowski
2025-10-29  8:29     ` Ryan Chen
2025-10-29 10:52       ` Krzysztof Kozlowski [this message]
2025-11-11  4:57         ` Ryan Chen
2025-11-13  9:34           ` Ryan Chen
2025-11-13 10:40             ` Krzysztof Kozlowski
2025-11-14  6:17               ` Ryan Chen
2025-11-21  5:23                 ` Ryan Chen
2025-11-21  7:18                   ` Krzysztof Kozlowski
2025-11-21  7:22                     ` Ryan Chen
2025-10-21  1:35 ` [PATCH v20 2/4] dt-bindings: i2c: ast2600-i2c.yaml: Add global-regs and transfer-mode properties Ryan Chen
2025-10-21  1:35 ` [PATCH v20 3/4] i2c: ast2600: Add controller driver for new register layout Ryan Chen
2025-10-24  9:10   ` Philipp Zabel
2025-10-25  2:57     ` Ryan Chen
2025-10-21  1:35 ` [PATCH v20 4/4] i2c: ast2600: Add target mode support Ryan Chen

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=3c3287f6-1c5c-4c4d-9349-32665a5e1585@kernel.org \
    --to=krzk@kernel.org \
    --cc=andi.shyti@kernel.org \
    --cc=andrew@codeconstruct.com.au \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=benh@kernel.crashing.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=jk@codeconstruct.com.au \
    --cc=joel@jms.id.au \
    --cc=krzk+dt@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-aspeed@lists.ozlabs.org \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=naresh.solanki@9elements.com \
    --cc=openbmc@lists.ozlabs.org \
    --cc=p.zabel@pengutronix.de \
    --cc=robh@kernel.org \
    --cc=ryan_chen@aspeedtech.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®