Hi Krzysztof, > -----Original Message----- > From: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxxxxx> > Sent: Friday, October 8, 2021 2:55 AM > To: Chanho Park <chanho61.park@xxxxxxxxxxx>; Rob Herring > <robh+dt@xxxxxxxxxx>; Linus Walleij <linus.walleij@xxxxxxxxxx> > Cc: Tomasz Figa <tomasz.figa@xxxxxxxxx>; Sylwester Nawrocki > <s.nawrocki@xxxxxxxxxxx>; linux-samsung-soc@xxxxxxxxxxxxxxx; > devicetree@xxxxxxxxxxxxxxx > Subject: Re: [PATCH 1/3] pinctrl: samsung: support ExynosAutov9 SoC > pinctrl > > On 07/10/2021 14:08, Chanho Park wrote: > > Add pinctrl data for ExynosAuto v9 SoC. > > > > - GPA0, GPA1: 10, External wake up interrupt > > - GPQ0: 2, XbootLDO, Speedy PMIC I/F > > - GPB0, GPB1, GPB2, GPB3: 29, I2S 7 CH > > - GPF0, GPF1, GPF2, GPF3,GPF4, GPF5, GPF6, GPF8: 52, FSYS > > - GPG0, GPG1, GPG2, GPG3: 25, GPIO x 24, SMPL_INT > > - GPP0, GPP1, GPP2, GPP3, GPP4, GPP5: 48, USI 12 CH > > > > Signed-off-by: Chanho Park <chanho61.park@xxxxxxxxxxx> > > Thanks Chanho for the patches. It's awesome to see this work upstreamed! > > Few comments below. Thanks for your review. > > > --- > > .../bindings/pinctrl/samsung-pinctrl.txt | 1 + > > .../pinctrl/samsung/pinctrl-exynos-arm64.c | 108 ++++++++++++++++++ > > drivers/pinctrl/samsung/pinctrl-samsung.c | 2 + > > drivers/pinctrl/samsung/pinctrl-samsung.h | 1 + > > 4 files changed, 112 insertions(+) > > > > diff --git > > a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt > > b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt > > index e7a1b1880375..b8b475967ff9 100644 > > --- a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt > > +++ b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt > > @@ -23,6 +23,7 @@ Required Properties: > > - "samsung,exynos5433-pinctrl": for Exynos5433 compatible pin- > controller. > > - "samsung,exynos7-pinctrl": for Exynos7 compatible pin-controller. > > - "samsung,exynos850-pinctrl": for Exynos850 compatible pin-controller. > > + - "samsung,exynosautov9-pinctrl": for ExynosAutov9 compatible pin- > controller. > > > > - reg: Base address of the pin controller hardware module and length of > > the address space it occupies. > > diff --git a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c > > b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c > > index fe5f6046fbd5..3bf18e844402 100644 > > --- a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c > > +++ b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c > > @@ -538,3 +538,111 @@ const struct samsung_pinctrl_of_match_data > exynos850_of_data __initconst = { > > .ctrl = exynos850_pin_ctrl, > > .num_ctrl = ARRAY_SIZE(exynos850_pin_ctrl), > > }; > > + > > +/* pin banks of exynosautov9 pin-controller 0 (ALIVE) */ static > > +struct samsung_pin_bank_data exynosautov9_pin_banks0[] = { > > This and below should be static const and __initconst. Okay. I'll put both. > > > + EXYNOS850_PIN_BANK_EINTW(8, 0x000, "gpa0", 0x00), > > + EXYNOS850_PIN_BANK_EINTW(2, 0x020, "gpa1", 0x04), > > + EXYNOS850_PIN_BANK_EINTN(2, 0x040, "gpq0"), }; > > + > > +/* pin banks of exynosautov9 pin-controller 1 (AUD) */ static struct > > +samsung_pin_bank_data exynosautov9_pin_banks1[] = { > > + EXYNOS850_PIN_BANK_EINTG(5, 0x000, "gpb0", 0x00), > > + EXYNOS850_PIN_BANK_EINTG(8, 0x020, "gpb1", 0x04), > > + EXYNOS850_PIN_BANK_EINTG(8, 0x040, "gpb2", 0x08), > > + EXYNOS850_PIN_BANK_EINTG(8, 0x060, "gpb3", 0x0C), }; > > + > > +/* pin banks of exynosautov9 pin-controller 2 (FSYS0) */ static > > +struct samsung_pin_bank_data exynosautov9_pin_banks2[] = { > > + EXYNOS850_PIN_BANK_EINTG(6, 0x000, "gpf0", 0x00), > > + EXYNOS850_PIN_BANK_EINTG(6, 0x020, "gpf1", 0x04), }; > > + > > +/* pin banks of exynosautov9 pin-controller 3 (FSYS1) */ static > > +struct samsung_pin_bank_data exynosautov9_pin_banks3[] = { > > + EXYNOS850_PIN_BANK_EINTG(6, 0x000, "gpf8", 0x00), }; > > + > > +/* pin banks of exynosautov9 pin-controller 4 (FSYS2) */ static > > +struct samsung_pin_bank_data exynosautov9_pin_banks4[] = { > > + EXYNOS850_PIN_BANK_EINTG(4, 0x000, "gpf2", 0x00), > > + EXYNOS850_PIN_BANK_EINTG(8, 0x020, "gpf3", 0x04), > > + EXYNOS850_PIN_BANK_EINTG(7, 0x040, "gpf4", 0x08), > > + EXYNOS850_PIN_BANK_EINTG(8, 0x060, "gpf5", 0x0C), > > + EXYNOS850_PIN_BANK_EINTG(7, 0x080, "gpf6", 0x10), }; > > + > > +/* pin banks of exynosautov9 pin-controller 5 (PERIC0) */ static > > +struct samsung_pin_bank_data exynosautov9_pin_banks5[] = { > > + EXYNOS850_PIN_BANK_EINTG(8, 0x000, "gpp0", 0x00), > > + EXYNOS850_PIN_BANK_EINTG(8, 0x020, "gpp1", 0x04), > > + EXYNOS850_PIN_BANK_EINTG(8, 0x040, "gpp2", 0x08), > > + EXYNOS850_PIN_BANK_EINTG(5, 0x060, "gpg0", 0x0C), }; > > + > > +/* pin banks of exynosautov9 pin-controller 6 (PERIC1) */ static > > +struct samsung_pin_bank_data exynosautov9_pin_banks6[] = { > > + EXYNOS850_PIN_BANK_EINTG(8, 0x000, "gpp3", 0x00), > > + EXYNOS850_PIN_BANK_EINTG(8, 0x020, "gpp4", 0x04), > > + EXYNOS850_PIN_BANK_EINTG(8, 0x040, "gpp5", 0x08), > > + EXYNOS850_PIN_BANK_EINTG(8, 0x060, "gpg1", 0x0C), > > + EXYNOS850_PIN_BANK_EINTG(8, 0x080, "gpg2", 0x10), > > + EXYNOS850_PIN_BANK_EINTG(4, 0x0A0, "gpg3", 0x14), }; > > + > > +const struct samsung_pin_ctrl exynosautov9_pin_ctrl[] = { > > __initconst at the end, please. Will do as well. Best Regards, Chanho Park