Hi Conor, >>>>> >>>>> 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 ^^^ >> >> Reading it again, I think you might have answered my first question, >> though not explicitly. The regions in question do both pinctrl and gpio, >> but you have chosen to represent it has lots of mini register regions, >> rather than as a simple-mfd type device - which I think would be the >> correct representation. . > > Yes, SIUL2 is mostly used for pinctrl and GPIO. The only other uses case is > to register a nvmem device for the first two registers in the SIUL2 MIDR1/MIDR2 > (MCU ID Register) which tell us information about the SoC (revision, > SRAM size and so on). > > I will convert the SIUL2 node into a simple-mfd device and switch the > GPIO and pinctrl drivers to use the syscon regmap in v5. I replied in the other patch series https://lore.kernel.org/all/a924bbb6-96ec-40be-9d82-a76b2ab73afd@xxxxxxxxxxx/ that I actually decided to unify the pinctrl&GPIO drivers instead of making them mfd_cells. I have a question regarding the NVMEM driver that I mentioned earlier. I haven't yet created a patch series to upstream it but I wanted to discuss about it here since it relates to SIUL2 and, in the future, we would like to upstream it as well. We register a NVMEM driver for the first two registers of SIUL2 which can then be read by other drivers to get information about the SoC. I think there are two options for integrating it: - Separate it from the pinctrl&GPIO driver as if it were part of a different IP. This would look something like this in the device tree /* SIUL2_0 base address is 0x4009c000 */ /* SIUL2_1 base address is 0x44010000 */ nvmem1@4009c000 { /* The registers are 32bit wide but start at offset 0x4 */ reg = <0x4009c000 0xc>; [..] }; pinctrl-gpio@4009c010 { reg = <0x4009c010 0xb84>, /* SIUL2_0 32bit registers */ <0x4009d700 0x50>, /* SIUL2_0 16bit registers */ <0x44010010 0x11f0>, /* SIUL2_1 32bit registers */ <0x4401170c 0x4c>, /* SIUL2_1 16bit registers */ [..] }; nvmem2@0x44010000 { reg = <0x44010000 0xc>; [..] } - have the nvmem as an mfd cell and the pinctrl&GPIO as another mfd cell The first option keeps the nvmem completely separated from pinctrl&GPIO but it makes the pinctrl&GPIO node start at an "odd" address. The second one more accurately represents the hardware (since the functionality is part of the same hardware block) but I am not sure if adding the mfd layer would add any benefit since the two functionalities don't have any shared resources in common. What do you think? Best regards, Andrei > > Best regards, > Andrei > > >> Cheers, >> Conor. >> >>> >>>>> >>>>> 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? >>>> >>>> I will add a bit more context regarding the hardware. >>>> >>>> The hardware module which implements pinctrl & GPIO is called SIUL2. >>>> For both S32G2 and S32G3 we have the same version of the module and >>>> 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: >>>> >>>> - SIUL2_0 mapped at address 0x4009c000, handling pins 0 - 101 >>>> - SIUL2_1 mapped at address 0x44010000, handling pins 112 - 190 >>>> >>>> There are multiple registers for the SIUL2 modules which are important >>>> for pinctrl & GPIO: >>>> >>>> - 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. >>>> >>>> - 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 >>>> (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. >>>> >>>> - PGPDO/PGPDI Parallel GPIO Pad Data Out/In >>>> 16 bit registers where each bit(besides some gaps) represents >>>> a GPIO's output/input value >>>> >>>> - 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). >>>> >>>> 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. >>>> >>>> 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. >>>> >>>>> >>>>> >>>>> Cheers, >>>>> Conor. >>>>> >>>> >>>> Best regards, >>>> Andrei >>>> >> >> > >
