<html xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:\65B0 \7D30 \660E \9AD4 ;
panose-1:2 2 5 0 0 0 0 0 0 0;}
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:"\@\65B0 \7D30 \660E \9AD4 ";
panose-1:2 1 6 1 0 1 1 1 1 1;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0cm;
font-size:12.0pt;
font-family:"\65B0 \7D30 \660E \9AD4 ",serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;
mso-ligatures:none;}
@page WordSection1
{size:612.0pt 792.0pt;
margin:72.0pt 90.0pt 72.0pt 90.0pt;}
div.WordSection1
{page:WordSection1;}
--></style>
</head>
<body lang="ZH-TW" link="blue" vlink="purple" style="word-wrap:break-word">
<div class="WordSection1">
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt">Hi Jeremy,<br>
<br>
> -----Original Message-----<br>
> From: Jeremy Kerr [<a href="mailto:jk@codeconstruct.com.au">mailto:jk@codeconstruct.com.au</a>]<br>
> Sent: Wednesday, August 9, 2023 8:08 AM<br>
> To: Dylan Hung <dylan_hung@aspeedtech.com>;<br>
> alexandre.belloni@bootlin.com; robh+dt@kernel.org;<br>
> krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org; joel@jms.id.au;<br>
> andrew@aj.id.au; p.zabel@pengutronix.de; linux-i3c@lists.infradead.org;<br>
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;<br>
> linux-aspeed@lists.ozlabs.org; linux-kernel@vger.kernel.org<br>
> Cc: BMC-SW <BMC-SW@aspeedtech.com>; kobedylan@gmail.com<br>
> Subject: Re: [PATCH 0/3] Add Aspeed AST2600 I3C support<br>
> <br>
> Hi Dylan,<br>
> <br>
> > This patch series introduces the necessary changes to enable I3C<br>
> > support for the Aspeed AST2600 I3C controller. Specifically, it<br>
> > addresses the missing pinctrl configuration and reset control for the<br>
> > I3C functionality.<br>
> <br>
> +1 for the pinctrl changes for the I3C1 and I3C2 controllers (I'll<br>
> review and ack separately). I have been testing on I3C3 and up, but just not<br>
> with the HVI3C on 1 & 2, hence no pinctrl definition there.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt">Thank you for your review. I3C1 and I3C2 can only operate in low voltage (1.0V/1.2V), which is why there are no HVI3C1 and HVI3C2 pinctrl definitions.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt"><br>
> <br>
> However, I don't think the other two are needed.<br>
> <br>
> For 2/3 and 3/3, you're adding a reset control for the global register block<br>
> within the per-controller driver, but we can already do that on a global basis<br>
> with the existing syscon device. Hence this earlier change:<br>
><o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt">I followed your recommendation and verified that it worked on my end.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt">Should I resend the pinctrl patch as a stand-alone submission?<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt"><br>
> <br>
> <a href="https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/dri">
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/dri</a><br>
> vers/mfd/syscon.c?id=7d1e3bd94828ad9fc86f55253cd6fec8edd65394<br>
> <br>
> For this, I have:<br>
> <br>
> &i3c {<br>
> i3c_global: i3c-global {<br>
> compatible = "aspeed,ast2600-i3c-global", "simple-mfd",<br>
> "syscon";<br>
> resets = <&syscon ASPEED_RESET_I3C_DMA>;<br>
> reg = <0x0 0x1000>;<br>
> };<br>
> <br>
> i3c2: i3c-master@4000 {<br>
> compatible = "aspeed,ast2600-i3c";<br>
> reg = <0x4000 0x1000>;<br>
> clocks = <&syscon ASPEED_CLK_GATE_I3C2CLK>;<br>
> pinctrl-names = "default";<br>
> pinctrl-0 = <&pinctrl_i3c3_default>;<br>
> interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;<br>
> aspeed,global-regs = <&i3c_global 2>;<br>
> status = "disabled";<br>
> };<br>
> <br>
> /* ... */<br>
> };<br>
> <br>
> - with no changes needed to any bindings. I haven't needed any other resets;<br>
> are there per-controller resets specified in the HW docs you have?<br>
> <br>
> Does that work for you? If you'd like to test, feel free to use my sample dts at:<br>
> <br>
> <br>
> <a href="https://github.com/CodeConstruct/linux/commit/05cac24705fa62d2176ecbb">
https://github.com/CodeConstruct/linux/commit/05cac24705fa62d2176ecbb</a><br>
> bf15d955cfe86e753<br>
> <br>
> Cheers,<br>
> <br>
> <br>
> Jeremy<o:p></o:p></span></p>
</div>
</div>
</body>
</html>