From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 1DDB1CE8359 for ; Mon, 30 Sep 2024 15:02:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=QEEcFRuNeOsc9RpybwffNB+gcq4G4WuH/O5ahZb9Ihs=; b=GqnhHSFu2Nlbge51OA4MSLrG55 726Bxst4GLQskTyTSRmBc2nalVAnG2J4baxvo1DRKigstw7/Ned8OghAFQOSduheHW1qkpgUILEIK 3z1zkVL+JAUO4QHKx3x7E+mlgkHdgaEHQ5nYvHj2FdAiqYDKUzsCZFmoRfvWW7/r+gQkAdX8dvaRi ot/ynjovNxzh3vS0aqD0y4v8p6voeQNwXAHzLMFuBZEGlm5pL7pD8AFoi6GIl+Vq5ihQDpKb9ikuC HA+1Kzdp2RFH/dpMeuLUlGyeibhf4b2bEsXtOW3SMKNj3VoZrF3ix96JssxBDZSYxEcwPF6ysuRPP Db5rdRUQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1svHuo-000000000K7-05QS; Mon, 30 Sep 2024 15:02:22 +0000 Received: from nyc.source.kernel.org ([147.75.193.91]) by bombadil.infradead.org with esmtps (Exim 4.98 #2 (Red Hat Linux)) id 1svHtY-0000000009U-0Z6l for linux-arm-kernel@lists.infradead.org; Mon, 30 Sep 2024 15:01:08 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by nyc.source.kernel.org (Postfix) with ESMTP id A120DA41D53; Mon, 30 Sep 2024 15:00:54 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 26C62C4CEC7; Mon, 30 Sep 2024 15:00:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1727708462; bh=eohE6QCObep88wykExC/gfzu1IpD4jbaR8BG3GtP1Ac=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=qiAWCsUkUogRQgFUOjRzTChQGzUNcImiaZO0kaN2QxvNiOThYXbpKlI5fo+ExMfZS /90bIfUh+bwgt7TeLd+UHumn8FxBpuBgE+mOoUjeoPprSrY7f+3p/i6Wc8LZJHogxi qdbLegcP3cZHBhe5j6wGxUYFz95qJxUabVs4skRK3xYR4eDg27xeppTz+gd7MDGiDl Pm5D700V/MnUdEODmqIyLFmAf4rr8bQ5PfZH+ulN25Dmp/OBeb1Yg3kBI95ftlSKTm D/1AA8AmvV2ukVDBTB1uR6RvvwEiL/qNqYSffHlXuXRkAK1EsVGHDhX9orLfLrteKz nBhM9Azr5pm1w== Date: Mon, 30 Sep 2024 16:00:57 +0100 From: Conor Dooley To: Andrei Stefanescu Cc: Linus Walleij , Bartosz Golaszewski , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Chester Lin , Matthias Brugger , Greg Kroah-Hartman , "Rafael J. Wysocki" , linux-gpio@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, NXP S32 Linux Team , Christophe Lizzi , Alberto Ruiz , Enric Balletbo Subject: Re: [PATCH v4 2/4] dt-bindings: gpio: add support for NXP S32G2/S32G3 SoCs Message-ID: <20240930-shortness-unedited-650f7996e912@spud> References: <20240926143122.1385658-1-andrei.stefanescu@oss.nxp.com> <20240926143122.1385658-3-andrei.stefanescu@oss.nxp.com> <20240926-apricot-unfasten-5577c54a3e2f@spud> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="ks3nvjHvnjWNbDgx" Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240930_080106_637866_E532BAB5 X-CRM114-Status: GOOD ( 42.10 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org --ks3nvjHvnjWNbDgx Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Sep 27, 2024 at 10:13:54AM +0300, Andrei Stefanescu wrote: > Hi Conor, >=20 > Thank you very much for the prompt review! >=20 > On 26/09/2024 18:38, Conor Dooley wrote: > > On Thu, Sep 26, 2024 at 05:31:19PM +0300, Andrei Stefanescu wrote: > >> Add support for the GPIO driver of the NXP S32G2/S32G3 SoCs. > >> > >> Signed-off-by: Phu Luu An > >> Signed-off-by: Larisa Grigore > >> Signed-off-by: Ghennadi Procopciuc > >> Signed-off-by: Andrei Stefanescu > >=20 > > What's up with this SoB chain? You're the author what did > > the other 3 people do? Are they missing co-developed-by tags? >=20 > Yes, thank you for suggesting it! I will also add Co-developed-by tags > for them. In the end it should look like this: >=20 > Co-developed-by: Phu Luu An > Signed-off-by: Phu Luu An > Co-developed-by: Larisa Grigore > Signed-off-by: Larisa Grigore > Co-developed-by: Ghennadi Procopciuc > Signed-off-by: Ghennadi Procopciuc > Signed-off-by: Andrei Stefanescu >=20 > >> + > >> +examples: > >> + - | > >> + #include > >> + #include > >> + > >> + gpio@4009d700 { > >> + compatible =3D "nxp,s32g2-siul2-gpio"; > >> + reg =3D <0x4009d700 0x10>, > >> + <0x44011700 0x18>, > >> + <0x4009d740 0x10>, > >> + <0x44011740 0x18>, > >> + <0x44010010 0xb4>, > >> + <0x44011078 0x80>; > >=20 > > Huh, I only noticed this now. Are you sure that this is a correct > > representation of this device, and it is not really part of some syscon? > > The "random" nature of the addresses and the tiny sizes of the > > reservations make it seem that way. What other devices are in these > > regions? Thanks for your answer to my second question, but I think you missed this part here ^^^ > >=20 > > Additionally, it looks like "opads0" and "ipads0" are in a different > > region to their "1" equivalents. Should this really be represented as > > two disctint GPIO controllers? >=20 > I will add a bit more context regarding the hardware. >=20 > The hardware module which implements pinctrl & GPIO is called SIUL2. > For both S32G2 and S32G3 we have the same version of the module and=20 > it is integrated in the same way. Each SoC has two SIUL2 instances which > mostly have the same register types and only differ in the number > of pads associated to them: >=20 > - SIUL2_0 mapped at address 0x4009c000, handling pins 0 - 101 > - SIUL2_1 mapped at address 0x44010000, handling pins 112 - 190 >=20 > There are multiple registers for the SIUL2 modules which are important > for pinctrl & GPIO: >=20 > - MSCR (Multiplexed Signal Configuration Register) > It configures the function of a pin and some > pinconf properties: > - input buffer > - output buffer > - open-drain > - pull-up/pull-down > - slew rate > Function 0 means the pin is to be used as a GPIO. >=20 > - IMCR (Input Multiplexed Signal and Configuration Register) > If the signal on this pad is to be read by another hardware > module, this register is similar to a multiplexer, its value > configures which pad the hardware will link to the module. > As an example let's consider the I2C0 SDA line. It has one > IMCR associated to it. Below are its possible pins and > corresponding IMCR values: > pin 16 <- 2 > pin 24 <- 7 > pin 31 <- 3 > pin 122 <- 4=20 > (Note that MSCR122 is part of SIUL2_1 but the IMCR for > I2C0_SDA is part of SIUL2_0) > pin 153 <- 5 > pin 161 <- 6 > The IMCR values should be aligned with the function bits in the > MSCR bits. If we want to use pin 122 for I2C0_SDA we will configure > the function bits in MSCR122 and write the value 4 to the I2C0_SDA > IMCR.=20 >=20 > - PGPDO/PGPDI Parallel GPIO Pad Data Out/In > 16 bit registers where each bit(besides some gaps) represents > a GPIO's output/input value >=20 > - DISR0, DIRER0, IREER0, IFEER0 > These registers are used for: status, enable, rising/falling edge > configuration for interrupts. We have 32 interrupts called EIRQ and > each interrupt has one or more pads associated with it (controlled > by an IMCR register per EIRQ). >=20 > However, one important thing to note is that even though there are > EIRQs for SIUL2_0 pads, all the interrupt registers mentioned above > are only present in SIUL2_1. >=20 > Because of mixed pins (I2C0_SDA in the example above with the MSCR > in SIUL2_1 for pad 122 and the IMCR in SIUL2_0) and the interrupt > configuration registers in SIUL2_1 we decided to have a single > driver instance. >=20 > >=20 > >=20 > > Cheers, > > Conor. > >=20 >=20 > Best regards, > Andrei >=20 --ks3nvjHvnjWNbDgx Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEABYIAB0WIQRh246EGq/8RLhDjO14tDGHoIJi0gUCZvq9KAAKCRB4tDGHoIJi 0vmHAQDZkEsvWqk9bF65pY+mP7V4lxNXaDOc2Y3zZQ2x4yhxPwEAlTF5ibAwpY74 4llBQ4d9tup2Szwhs4An5BnJEm0SSwA= =BZuW -----END PGP SIGNATURE----- --ks3nvjHvnjWNbDgx--