All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeremy Kerr <jk@codeconstruct.com.au>
To: linux-aspeed@lists.ozlabs.org
Subject: [PATCH 0/3] Add Aspeed AST2600 I3C support
Date: Wed, 09 Aug 2023 08:07:58 +0800	[thread overview]
Message-ID: <d8d577e91779e045d5a2c7d701f65133a15b5d64.camel@codeconstruct.com.au> (raw)
In-Reply-To: <20230808154241.749641-1-dylan_hung@aspeedtech.com>

Hi Dylan,

> This patch series introduces the necessary changes to enable I3C
> support for the Aspeed AST2600 I3C controller. Specifically, it addresses the
> missing pinctrl configuration and reset control for the I3C
> functionality.

+1 for the pinctrl changes for the I3C1 and I3C2 controllers (I'll
review and ack separately). I have been testing on I3C3 and up, but just
not with the HVI3C on 1 & 2, hence no pinctrl definition there.

However, I don't think the other two are needed.

For 2/3 and 3/3, you're adding a reset control for the global register
block within the per-controller driver, but we can already do that on a
global basis with the existing syscon device. Hence this earlier change:

  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/mfd/syscon.c?id=7d1e3bd94828ad9fc86f55253cd6fec8edd65394

For this, I have:

    &i3c {
        i3c_global: i3c-global {
            compatible = "aspeed,ast2600-i3c-global", "simple-mfd", "syscon";
            resets = <&syscon ASPEED_RESET_I3C_DMA>;
            reg = <0x0 0x1000>;
        };

        i3c2: i3c-master at 4000 {
            compatible = "aspeed,ast2600-i3c";
            reg = <0x4000 0x1000>;
            clocks = <&syscon ASPEED_CLK_GATE_I3C2CLK>;
            pinctrl-names = "default";
            pinctrl-0 = <&pinctrl_i3c3_default>;
            interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
            aspeed,global-regs = <&i3c_global 2>;
            status = "disabled";
        };

        /* ... */
    };

- with no changes needed to any bindings. I haven't needed any other
resets; are there per-controller resets specified in the HW docs you
have?

Does that work for you? If you'd like to test, feel free to use my
sample dts at:

  https://github.com/CodeConstruct/linux/commit/05cac24705fa62d2176ecbbbf15d955cfe86e753

Cheers,


Jeremy

WARNING: multiple messages have this Message-ID (diff)
From: Jeremy Kerr <jk@codeconstruct.com.au>
To: Dylan Hung <dylan_hung@aspeedtech.com>,
	alexandre.belloni@bootlin.com,  robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
	joel@jms.id.au, andrew@aj.id.au, p.zabel@pengutronix.de,
	 linux-i3c@lists.infradead.org, devicetree@vger.kernel.org,
	 linux-arm-kernel@lists.infradead.org,
	linux-aspeed@lists.ozlabs.org,  linux-kernel@vger.kernel.org
Cc: BMC-SW@aspeedtech.com, kobedylan@gmail.com
Subject: Re: [PATCH 0/3] Add Aspeed AST2600 I3C support
Date: Wed, 09 Aug 2023 08:07:58 +0800	[thread overview]
Message-ID: <d8d577e91779e045d5a2c7d701f65133a15b5d64.camel@codeconstruct.com.au> (raw)
In-Reply-To: <20230808154241.749641-1-dylan_hung@aspeedtech.com>

Hi Dylan,

> This patch series introduces the necessary changes to enable I3C
> support for the Aspeed AST2600 I3C controller. Specifically, it addresses the
> missing pinctrl configuration and reset control for the I3C
> functionality.

+1 for the pinctrl changes for the I3C1 and I3C2 controllers (I'll
review and ack separately). I have been testing on I3C3 and up, but just
not with the HVI3C on 1 & 2, hence no pinctrl definition there.

However, I don't think the other two are needed.

For 2/3 and 3/3, you're adding a reset control for the global register
block within the per-controller driver, but we can already do that on a
global basis with the existing syscon device. Hence this earlier change:

  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/mfd/syscon.c?id=7d1e3bd94828ad9fc86f55253cd6fec8edd65394

For this, I have:

    &i3c {
        i3c_global: i3c-global {
            compatible = "aspeed,ast2600-i3c-global", "simple-mfd", "syscon";
            resets = <&syscon ASPEED_RESET_I3C_DMA>;
            reg = <0x0 0x1000>;
        };

        i3c2: i3c-master@4000 {
            compatible = "aspeed,ast2600-i3c";
            reg = <0x4000 0x1000>;
            clocks = <&syscon ASPEED_CLK_GATE_I3C2CLK>;
            pinctrl-names = "default";
            pinctrl-0 = <&pinctrl_i3c3_default>;
            interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
            aspeed,global-regs = <&i3c_global 2>;
            status = "disabled";
        };

        /* ... */
    };

- with no changes needed to any bindings. I haven't needed any other
resets; are there per-controller resets specified in the HW docs you
have?

Does that work for you? If you'd like to test, feel free to use my
sample dts at:

  https://github.com/CodeConstruct/linux/commit/05cac24705fa62d2176ecbbbf15d955cfe86e753

Cheers,


Jeremy

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

WARNING: multiple messages have this Message-ID (diff)
From: Jeremy Kerr <jk@codeconstruct.com.au>
To: Dylan Hung <dylan_hung@aspeedtech.com>,
	alexandre.belloni@bootlin.com,  robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
	joel@jms.id.au, andrew@aj.id.au, p.zabel@pengutronix.de,
	 linux-i3c@lists.infradead.org, devicetree@vger.kernel.org,
	 linux-arm-kernel@lists.infradead.org,
	linux-aspeed@lists.ozlabs.org,  linux-kernel@vger.kernel.org
Cc: BMC-SW@aspeedtech.com, kobedylan@gmail.com
Subject: Re: [PATCH 0/3] Add Aspeed AST2600 I3C support
Date: Wed, 09 Aug 2023 08:07:58 +0800	[thread overview]
Message-ID: <d8d577e91779e045d5a2c7d701f65133a15b5d64.camel@codeconstruct.com.au> (raw)
In-Reply-To: <20230808154241.749641-1-dylan_hung@aspeedtech.com>

Hi Dylan,

> This patch series introduces the necessary changes to enable I3C
> support for the Aspeed AST2600 I3C controller. Specifically, it addresses the
> missing pinctrl configuration and reset control for the I3C
> functionality.

+1 for the pinctrl changes for the I3C1 and I3C2 controllers (I'll
review and ack separately). I have been testing on I3C3 and up, but just
not with the HVI3C on 1 & 2, hence no pinctrl definition there.

However, I don't think the other two are needed.

For 2/3 and 3/3, you're adding a reset control for the global register
block within the per-controller driver, but we can already do that on a
global basis with the existing syscon device. Hence this earlier change:

  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/mfd/syscon.c?id=7d1e3bd94828ad9fc86f55253cd6fec8edd65394

For this, I have:

    &i3c {
        i3c_global: i3c-global {
            compatible = "aspeed,ast2600-i3c-global", "simple-mfd", "syscon";
            resets = <&syscon ASPEED_RESET_I3C_DMA>;
            reg = <0x0 0x1000>;
        };

        i3c2: i3c-master@4000 {
            compatible = "aspeed,ast2600-i3c";
            reg = <0x4000 0x1000>;
            clocks = <&syscon ASPEED_CLK_GATE_I3C2CLK>;
            pinctrl-names = "default";
            pinctrl-0 = <&pinctrl_i3c3_default>;
            interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
            aspeed,global-regs = <&i3c_global 2>;
            status = "disabled";
        };

        /* ... */
    };

- with no changes needed to any bindings. I haven't needed any other
resets; are there per-controller resets specified in the HW docs you
have?

Does that work for you? If you'd like to test, feel free to use my
sample dts at:

  https://github.com/CodeConstruct/linux/commit/05cac24705fa62d2176ecbbbf15d955cfe86e753

Cheers,


Jeremy

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Jeremy Kerr <jk@codeconstruct.com.au>
To: Dylan Hung <dylan_hung@aspeedtech.com>,
	alexandre.belloni@bootlin.com, robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
	joel@jms.id.au, andrew@aj.id.au, p.zabel@pengutronix.de,
	linux-i3c@lists.infradead.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-aspeed@lists.ozlabs.org, linux-kernel@vger.kernel.org
Cc: BMC-SW@aspeedtech.com, kobedylan@gmail.com
Subject: Re: [PATCH 0/3] Add Aspeed AST2600 I3C support
Date: Wed, 09 Aug 2023 08:07:58 +0800	[thread overview]
Message-ID: <d8d577e91779e045d5a2c7d701f65133a15b5d64.camel@codeconstruct.com.au> (raw)
In-Reply-To: <20230808154241.749641-1-dylan_hung@aspeedtech.com>

Hi Dylan,

> This patch series introduces the necessary changes to enable I3C
> support for the Aspeed AST2600 I3C controller. Specifically, it addresses the
> missing pinctrl configuration and reset control for the I3C
> functionality.

+1 for the pinctrl changes for the I3C1 and I3C2 controllers (I'll
review and ack separately). I have been testing on I3C3 and up, but just
not with the HVI3C on 1 & 2, hence no pinctrl definition there.

However, I don't think the other two are needed.

For 2/3 and 3/3, you're adding a reset control for the global register
block within the per-controller driver, but we can already do that on a
global basis with the existing syscon device. Hence this earlier change:

  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/mfd/syscon.c?id=7d1e3bd94828ad9fc86f55253cd6fec8edd65394

For this, I have:

    &i3c {
        i3c_global: i3c-global {
            compatible = "aspeed,ast2600-i3c-global", "simple-mfd", "syscon";
            resets = <&syscon ASPEED_RESET_I3C_DMA>;
            reg = <0x0 0x1000>;
        };

        i3c2: i3c-master@4000 {
            compatible = "aspeed,ast2600-i3c";
            reg = <0x4000 0x1000>;
            clocks = <&syscon ASPEED_CLK_GATE_I3C2CLK>;
            pinctrl-names = "default";
            pinctrl-0 = <&pinctrl_i3c3_default>;
            interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
            aspeed,global-regs = <&i3c_global 2>;
            status = "disabled";
        };

        /* ... */
    };

- with no changes needed to any bindings. I haven't needed any other
resets; are there per-controller resets specified in the HW docs you
have?

Does that work for you? If you'd like to test, feel free to use my
sample dts at:

  https://github.com/CodeConstruct/linux/commit/05cac24705fa62d2176ecbbbf15d955cfe86e753

Cheers,


Jeremy

  parent reply	other threads:[~2023-08-09  0:07 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-08 15:42 [PATCH 0/3] Add Aspeed AST2600 I3C support Dylan Hung
2023-08-08 15:42 ` Dylan Hung
2023-08-08 15:42 ` Dylan Hung
2023-08-08 15:42 ` Dylan Hung
2023-08-08 15:42 ` [PATCH 1/3] ARM: dts: pinctrl-aspeed-g6: Add I3C1 and I3C2 control pins Dylan Hung
2023-08-08 15:42   ` Dylan Hung
2023-08-08 15:42   ` Dylan Hung
2023-08-08 15:42   ` Dylan Hung
2023-08-08 15:42 ` [PATCH 2/3] dt-bindings: i3c: ast2600: Add resets and reset-names Dylan Hung
2023-08-08 15:42   ` Dylan Hung
2023-08-08 15:42   ` Dylan Hung
2023-08-08 15:42   ` Dylan Hung
2023-08-08 15:59   ` Krzysztof Kozlowski
2023-08-08 15:59     ` Krzysztof Kozlowski
2023-08-08 15:59     ` Krzysztof Kozlowski
2023-08-08 15:59     ` Krzysztof Kozlowski
2023-08-08 15:42 ` [PATCH 3/3] i3c: ast2600: Add reset deassertion for global registers Dylan Hung
2023-08-08 15:42   ` Dylan Hung
2023-08-08 15:42   ` Dylan Hung
2023-08-08 15:42   ` Dylan Hung
2023-08-08 16:00   ` Krzysztof Kozlowski
2023-08-08 16:00     ` Krzysztof Kozlowski
2023-08-08 16:00     ` Krzysztof Kozlowski
2023-08-08 16:00     ` Krzysztof Kozlowski
2023-08-09  0:07 ` Jeremy Kerr [this message]
2023-08-09  0:07   ` [PATCH 0/3] Add Aspeed AST2600 I3C support Jeremy Kerr
2023-08-09  0:07   ` Jeremy Kerr
2023-08-09  0:07   ` Jeremy Kerr
2023-08-09  0:28   ` Jeremy Kerr
2023-08-09  0:28     ` Jeremy Kerr
2023-08-09  0:28     ` Jeremy Kerr
2023-08-09  0:28     ` Jeremy Kerr
2023-08-09  2:46   ` Dylan Hung
2023-08-09  2:46     ` Dylan Hung
2023-08-09  2:46     ` Dylan Hung
2023-08-09  3:05   ` Dylan Hung
2023-08-09  3:12     ` Jeremy Kerr
2023-08-09  3:12       ` Jeremy Kerr
2023-08-09  3:12       ` Jeremy Kerr
2023-08-09  3:12       ` Jeremy Kerr

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=d8d577e91779e045d5a2c7d701f65133a15b5d64.camel@codeconstruct.com.au \
    --to=jk@codeconstruct.com.au \
    --cc=linux-aspeed@lists.ozlabs.org \
    /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.