| Message ID | 20231115095609.39883-11-jaewon02.kim@samsung.com |
|---|---|
| State | New |
| Headers | show |
| Series | Introduce ExynosAutov920 SoC and SADK board | expand |
On 15/11/2023 10:56, Jaewon Kim wrote: > ExynosAutov920 GPIO has a different register structure. > In the existing Exynos series, EINT control register enumerated after > a specific offset (e.g EXYNOS_GPIO_ECON_OFFSET). > However, in ExynosAutov920 SoC, the register that controls EINT belongs > to each GPIO group, and each GPIO group has 0x1000 align. > > This is a structure to protect the GPIO group with S2MPU in VM environment, > and will only be applied in ExynosAuto series SoCs. Checkpatch points some warnings: CHECK: Alignment should match open parenthesis CHECK: Lines should not end with a '(' CHECK: Macro argument reuse 'reg' - possible side-effects? Best regards, Krzysztof
On 23. 11. 15. 21:42, Krzysztof Kozlowski wrote: > On 15/11/2023 10:56, Jaewon Kim wrote: >> ExynosAutov920 GPIO has a different register structure. >> In the existing Exynos series, EINT control register enumerated after >> a specific offset (e.g EXYNOS_GPIO_ECON_OFFSET). >> However, in ExynosAutov920 SoC, the register that controls EINT belongs >> to each GPIO group, and each GPIO group has 0x1000 align. >> >> This is a structure to protect the GPIO group with S2MPU in VM environment, >> and will only be applied in ExynosAuto series SoCs. > Checkpatch points some warnings: > > CHECK: Alignment should match open parenthesis > CHECK: Lines should not end with a '(' > CHECK: Macro argument reuse 'reg' - possible side-effects? I don`t know this happens. When I did the checkpatch, there were no problems as shown below. --- ./scripts/checkpatch.pl 0010-pinctrl-samsung-add-exynosautov920-pinctrl.patch total: 0 errors, 0 warnings, 416 lines checked 0010-pinctrl-samsung-add-exynosautov920-pinctrl.patch has no obvious style problems and is ready for submission. --- > > Best regards, > Krzysztof > > Thanks Jaewon Kim
Hi all, I sent it again because the mail format was broken due to the HTML part. On 23. 11. 15. 21:28, Krzysztof Kozlowski wrote: > On 15/11/2023 10:56, Jaewon Kim wrote: >> ExynosAutov920 GPIO has a different register structure. >> In the existing Exynos series, EINT control register enumerated after >> a specific offset (e.g EXYNOS_GPIO_ECON_OFFSET). >> However, in ExynosAutov920 SoC, the register that controls EINT belongs >> to each GPIO group, and each GPIO group has 0x1000 align. >> >> This is a structure to protect the GPIO group with S2MPU in VM environment, >> and will only be applied in ExynosAuto series SoCs. >> >> Example) >> ------------------------------------------------- >> | original | ExynosAutov920 | >> |-----------------------------------------------| >> | 0x0 GPIO_CON | 0x0 GPIO_CON | >> | 0x4 GPIO_DAT | 0x4 GPIO_DAT | >> | 0x8 GPIO_PUD | 0x8 GPIO_PUD | >> | 0xc GPIO_DRV | 0xc GPIO_DRV | >> | 0x700 EINT_CON | 0x18 EINT_CON | >> | 0x800 EINT_FLTCON | 0x1c EINT_FLTCON0 | >> | 0x900 EINT_MASK | 0x20 EINT_FLTCON1 | >> | 0xa00 EINT_PEND | 0x24 EINT_MASK | >> | | 0x28 EINT_PEND | >> ------------------------------------------------- >> >> Pinctrl data for ExynosAutoV920 SoC. >> - GPA0,GPA1 (10): External wake up interrupt >> - GPQ0 (2): SPMI (PMIC I/F) >> - GPB0,GPB1,GPB2,GPB3,GPB4,GPB5,GPB6 (47): I2S Audio >> - GPH0,GPH1,GPH2,GPH3,GPH4,GPH5,GPH6,GPH8 (49): PCIE, UFS, Ethernet >> - GPG0,GPG1,GPG2,GPG3,GPG4,GPG5 (29): General purpose >> - GPP0,GPP1,GPP2,GPP3,GPP4,GPP5,GPP6,GPP7,GPP8,GPP9,GPP10 (77): USI >> >> Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com> >> --- >> .../pinctrl/samsung/pinctrl-exynos-arm64.c | 140 ++++++++++++++++++ >> drivers/pinctrl/samsung/pinctrl-exynos.c | 102 ++++++++++++- >> drivers/pinctrl/samsung/pinctrl-exynos.h | 27 ++++ >> drivers/pinctrl/samsung/pinctrl-samsung.c | 5 + >> drivers/pinctrl/samsung/pinctrl-samsung.h | 13 ++ >> 5 files changed, 280 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c >> index cb965cf93705..cf86722a70a3 100644 >> --- a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c >> +++ b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c >> @@ -796,3 +796,143 @@ const struct samsung_pinctrl_of_match_data fsd_of_data __initconst = { >> .ctrl = fsd_pin_ctrl, >> .num_ctrl = ARRAY_SIZE(fsd_pin_ctrl), >> }; >> + >> +/* pin banks of exynosautov920 pin-controller 0 (ALIVE) */ >> +static struct samsung_pin_bank_data exynosautov920_pin_banks0[] = { > So you created patch from some downstream code? No, please work on > upstream. Take upstream code and customize it to your needs. That way > you won't introduce same mistakes fixes years ago. > > Missing const. I did not work on downstream source. Kernel version is different, and there are numerous SoCs, So bringing the patch form the downstream make conflicts. Don`t worry about it. Only GPIO struct was copied in the downstream code and the 'const' was missing. Anyway, I will add const in next version. > > ... > >> @@ -31,6 +31,7 @@ >> #define EXYNOS7_WKUP_EMASK_OFFSET 0x900 >> #define EXYNOS7_WKUP_EPEND_OFFSET 0xA00 >> #define EXYNOS_SVC_OFFSET 0xB08 >> +#define EXYNOSAUTOV920_SVC_OFFSET 0xF008 >> > ... > >> #ifdef CONFIG_PINCTRL_S3C64XX >> { .compatible = "samsung,s3c64xx-pinctrl", >> diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.h b/drivers/pinctrl/samsung/pinctrl-samsung.h >> index 9b3db50adef3..cbb78178651b 100644 >> --- a/drivers/pinctrl/samsung/pinctrl-samsung.h >> +++ b/drivers/pinctrl/samsung/pinctrl-samsung.h >> @@ -122,6 +122,9 @@ struct samsung_pin_bank_type { >> * @eint_type: type of the external interrupt supported by the bank. >> * @eint_mask: bit mask of pins which support EINT function. >> * @eint_offset: SoC-specific EINT register or interrupt offset of bank. >> + * @mask_offset: SoC-specific EINT mask register offset of bank. >> + * @pend_offset: SoC-specific EINT pend register offset of bank. >> + * @combine: EINT register is adjacent to the GPIO control register. > I don't understand it. Adjacent? Are you sure? GPIO control register has > 0xF004 (EXYNOSAUTOV920_SVC_OFFSET + 0x4)? Anyway, this does not scale. > What if next revision comes with not-adjacent. There will be > "combine_plus"? Also name confuses me - combine means together. > > Also your first map of registers does not have it adjacent... I think I should have added more information about new architect. ------------------------------------------------- | original | ExynosAutov920 | |------------------------------------------------| | 0x0 GPA_CON | 0x0 GPA_CON | | 0x4 GPA_DAT | 0x4 GPA_DAT | | 0x8 GPA_PUD | 0x8 GPA_PUD | | 0xc GPA_DRV | 0xc GPA_DRV | |-----------------------| 0x18 EINT_GPA_CON | | 0x20 GPB_CON | 0x1c EINT_GPA_FLTCON0| | 0x24 GPB_DAT | 0x20 EINT_GPA_FLTCON1| | 0x28 GPB_PUD | 0x24 EINT_GPA_MASK | | 0x2c GPB_DRV | 0x28 EINT_GPA_PEND | |-----------------------|------------------------| | 0x700 EINT_GPA_CON | 0x1000 GPB_CON | | 0x704 EINT_GPB_CON | 0x1004 GPB_DAT | |-----------------------| 0x1008 GPB_PUD | | 0x800 EINT_GPA_FLTCON | 0x100c GPB_DRV | | 0x804 EINT_GPB_FLTCON | 0x1018 EINT_GPB_CON | |-----------------------| 0x101c EINT_GPB_FLTCON0| | 0x900 EINT_GPA_MASK | 0x1020 EINT_GPB_FLTCON1| | 0x904 EINT_GPB_MASK | 0x1024 EINT_GPB_MASK | |-----------------------| 0x1028 EINT_GPB_PEND | | 0xa00 EINT_GPA_PEND |------------------------| | 0xa04 EINT_GPB_PEND | | |-----------------------|------------------------| | 0xb08 SVC | 0xf008 SVC | -------------------------------------------------| The reason why I chose variable name 'combine' is that EINT registers was separated from gpio control address. However, in exynosautov920 EINT registers combined with GPx group. So I chose "combine" word. If there is more reasonable name, i will change it. And I will also change the description of the variable. EINT registers related to the entire group(e.g SVC) were at the end of the GPIO block 0xb00 and now it has been moved to 0xf000. > > Anyway first patch is to rework driver to support new register layout. > Second patch is to add new variant. Okay, I will divide the patch in the next version. > > Best regards, > Krzysztof > > Thanks Jaewon Kim
On 16/11/2023 04:50, Jaewon Kim wrote: > > On 23. 11. 15. 21:42, Krzysztof Kozlowski wrote: >> On 15/11/2023 10:56, Jaewon Kim wrote: >>> ExynosAutov920 GPIO has a different register structure. >>> In the existing Exynos series, EINT control register enumerated after >>> a specific offset (e.g EXYNOS_GPIO_ECON_OFFSET). >>> However, in ExynosAutov920 SoC, the register that controls EINT belongs >>> to each GPIO group, and each GPIO group has 0x1000 align. >>> >>> This is a structure to protect the GPIO group with S2MPU in VM environment, >>> and will only be applied in ExynosAuto series SoCs. >> Checkpatch points some warnings: >> >> CHECK: Alignment should match open parenthesis >> CHECK: Lines should not end with a '(' >> CHECK: Macro argument reuse 'reg' - possible side-effects? > > I don`t know this happens. > > When I did the checkpatch, there were no problems as shown below. Didn't you miss some arguments? Lime --strict? Best regards, Krzysztof
On 16/11/2023 06:39, Jaewon Kim wrote: > On 23. 11. 15. 21:28, Krzysztof Kozlowski wrote: > >> On 15/11/2023 10:56, Jaewon Kim wrote: >>> ExynosAutov920 GPIO has a different register structure. >>> In the existing Exynos series, EINT control register enumerated after >>> a specific offset (e.g EXYNOS_GPIO_ECON_OFFSET). >>> However, in ExynosAutov920 SoC, the register that controls EINT belongs >>> to each GPIO group, and each GPIO group has 0x1000 align. >>> >>> This is a structure to protect the GPIO group with S2MPU in VM environment, >>> and will only be applied in ExynosAuto series SoCs. >>> >>> Example) >>> ------------------------------------------------- >>> | original | ExynosAutov920 | >>> |-----------------------------------------------| >>> | 0x0 GPIO_CON | 0x0 GPIO_CON | >>> | 0x4 GPIO_DAT | 0x4 GPIO_DAT | >>> | 0x8 GPIO_PUD | 0x8 GPIO_PUD | >>> | 0xc GPIO_DRV | 0xc GPIO_DRV | >>> | 0x700 EINT_CON | 0x18 EINT_CON | >>> | 0x800 EINT_FLTCON | 0x1c EINT_FLTCON0 | >>> | 0x900 EINT_MASK | 0x20 EINT_FLTCON1 | >>> | 0xa00 EINT_PEND | 0x24 EINT_MASK | >>> | | 0x28 EINT_PEND | >>> ------------------------------------------------- >>> >>> Pinctrl data for ExynosAutoV920 SoC. >>> - GPA0,GPA1 (10): External wake up interrupt >>> - GPQ0 (2): SPMI (PMIC I/F) >>> - GPB0,GPB1,GPB2,GPB3,GPB4,GPB5,GPB6 (47): I2S Audio >>> - GPH0,GPH1,GPH2,GPH3,GPH4,GPH5,GPH6,GPH8 (49): PCIE, UFS, Ethernet >>> - GPG0,GPG1,GPG2,GPG3,GPG4,GPG5 (29): General purpose >>> - GPP0,GPP1,GPP2,GPP3,GPP4,GPP5,GPP6,GPP7,GPP8,GPP9,GPP10 (77): USI >>> >>> Signed-off-by: Jaewon Kim<jaewon02.kim@samsung.com> >>> --- >>> .../pinctrl/samsung/pinctrl-exynos-arm64.c | 140 ++++++++++++++++++ >>> drivers/pinctrl/samsung/pinctrl-exynos.c | 102 ++++++++++++- >>> drivers/pinctrl/samsung/pinctrl-exynos.h | 27 ++++ >>> drivers/pinctrl/samsung/pinctrl-samsung.c | 5 + >>> drivers/pinctrl/samsung/pinctrl-samsung.h | 13 ++ >>> 5 files changed, 280 insertions(+), 7 deletions(-) >>> >>> diff --git a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c >>> index cb965cf93705..cf86722a70a3 100644 >>> --- a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c >>> +++ b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c >>> @@ -796,3 +796,143 @@ const struct samsung_pinctrl_of_match_data fsd_of_data __initconst = { >>> .ctrl = fsd_pin_ctrl, >>> .num_ctrl = ARRAY_SIZE(fsd_pin_ctrl), >>> }; >>> + >>> +/* pin banks of exynosautov920 pin-controller 0 (ALIVE) */ >>> +static struct samsung_pin_bank_data exynosautov920_pin_banks0[] = { >> So you created patch from some downstream code? No, please work on >> upstream. Take upstream code and customize it to your needs. That way >> you won't introduce same mistakes fixes years ago. >> >> Missing const. > > Thanks for the guide. > > I didn`t work on downstream source, but when I copy/paste > > the struct enumerations from downstream, it seemed like That's what I am talking about. Don't do like this. We fixed several things in Linux kernel, so copying unfixed code is wasting of everyone's time. Don't work on downstream. Don't copy anything from downstream. You *MUST CUSTOMIZE* upstream file, not downstream. > > 'const' was missing. > >> >> ... >> >>> @@ -31,6 +31,7 @@ >>> #define EXYNOS7_WKUP_EMASK_OFFSET 0x900 >>> #define EXYNOS7_WKUP_EPEND_OFFSET 0xA00 >>> #define EXYNOS_SVC_OFFSET 0xB08 >>> +#define EXYNOSAUTOV920_SVC_OFFSET 0xF008 >>> >> ... >> >>> #ifdef CONFIG_PINCTRL_S3C64XX >>> { .compatible = "samsung,s3c64xx-pinctrl", >>> diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.h b/drivers/pinctrl/samsung/pinctrl-samsung.h >>> index 9b3db50adef3..cbb78178651b 100644 >>> --- a/drivers/pinctrl/samsung/pinctrl-samsung.h >>> +++ b/drivers/pinctrl/samsung/pinctrl-samsung.h >>> @@ -122,6 +122,9 @@ struct samsung_pin_bank_type { >>> * @eint_type: type of the external interrupt supported by the bank. >>> * @eint_mask: bit mask of pins which support EINT function. >>> * @eint_offset: SoC-specific EINT register or interrupt offset of bank. >>> + * @mask_offset: SoC-specific EINT mask register offset of bank. >>> + * @pend_offset: SoC-specific EINT pend register offset of bank. >>> + * @combine: EINT register is adjacent to the GPIO control register. >> I don't understand it. Adjacent? Are you sure? GPIO control register has >> 0xF004 (EXYNOSAUTOV920_SVC_OFFSET + 0x4)? Anyway, this does not scale. >> What if next revision comes with not-adjacent. There will be >> "combine_plus"? Also name confuses me - combine means together. >> >> Also your first map of registers does not have it adjacent... > > I think I should have added a little more information about new struct. > > ------------------------------------------------- > | original | ExynosAutov920 | > |-----------------------------------------------| > | 0x0 GPA_CON | 0x0 GPA_CON | > | 0x4 GPA_DAT | 0x4 GPA_DAT | > | 0x8 GPA_PUD | 0x8 GPA_PUD | > | 0xc GPA_DRV | 0xc GPA_DRV | > |----------------------| 0x18 EINT_GPA_CON | > | 0x20 GPB_CON | 0x1c EINT_GPA_FLTCON0| > | 0x4 GPB_DAT | 0x20 EINT_GPA_FLTCON1| > | 0x28 GPB_PUD | 0x24 EINT_GPA_MASK | > | 0x2c GPB_DRV | 0x28 EINT_GPA_PEND | > |----------------------|------------------------| > | 0x700 EINT_GPA_CON | 0x1000 GPA_CON | > | 0x704 EINT_GPB_CON | 0x1004 GPA_DAT | > |----------------------| 0x1008 GPA_PUD | > | 0x800 EINT_GPA_FLTCON| 0x100c GPA_DRV | > | 0x804 EINT_GPB_FLTCON| 0x1018 EINT_GPA_CON | > |----------------------| 0x101c EINT_GPA_FLTCON0| > | 0x900 EINT_GPA_MASK | 0x1020 EINT_GPA_FLTCON1| > | 0x904 EINT_GPB_MASK | 0x1024 EINT_GPA_MASK | > |----------------------| 0x1028 EINT_GPA_PEND | > | 0xa00 EINT_GPA_PEND |------------------------| > | 0xa04 EINT_GPB_PEND | | > ------------------------------------------------| > | 0xb08 SVC | 0xf008 SVC | > ------------------------------------------------- > > The reason why I chose variable name 'combine' is that EINT registers was > separated from gpio control address. However, in exynosautov920 EINT > registers combined with GPx group. So I chose "combine" word. What does it mean "the GPx group"? Combined means the same place, the same register. I could imagine offset is 0x4, what I wrote last time. Is the offset 0x4? > Is another reasonable word, I will change it. Why you cannot store the offset? > > EINT registers related to the entire group(e.g SVC) were at the end of > the GPIO block and are now moved to 0xf000. So not in the same register, not combined? Best regards, Krzysztof
On 23. 11. 16. 20:21, Krzysztof Kozlowski wrote: > On 16/11/2023 06:39, Jaewon Kim wrote: >> On 23. 11. 15. 21:28, Krzysztof Kozlowski wrote: >> >>> On 15/11/2023 10:56, Jaewon Kim wrote: >>>> ExynosAutov920 GPIO has a different register structure. >>>> In the existing Exynos series, EINT control register enumerated after >>>> a specific offset (e.g EXYNOS_GPIO_ECON_OFFSET). >>>> However, in ExynosAutov920 SoC, the register that controls EINT belongs >>>> to each GPIO group, and each GPIO group has 0x1000 align. >>>> >>>> This is a structure to protect the GPIO group with S2MPU in VM environment, >>>> and will only be applied in ExynosAuto series SoCs. >>>> >>>> Example) >>>> ------------------------------------------------- >>>> | original | ExynosAutov920 | >>>> |-----------------------------------------------| >>>> | 0x0 GPIO_CON | 0x0 GPIO_CON | >>>> | 0x4 GPIO_DAT | 0x4 GPIO_DAT | >>>> | 0x8 GPIO_PUD | 0x8 GPIO_PUD | >>>> | 0xc GPIO_DRV | 0xc GPIO_DRV | >>>> | 0x700 EINT_CON | 0x18 EINT_CON | >>>> | 0x800 EINT_FLTCON | 0x1c EINT_FLTCON0 | >>>> | 0x900 EINT_MASK | 0x20 EINT_FLTCON1 | >>>> | 0xa00 EINT_PEND | 0x24 EINT_MASK | >>>> | | 0x28 EINT_PEND | >>>> ------------------------------------------------- >>>> >>>> Pinctrl data for ExynosAutoV920 SoC. >>>> - GPA0,GPA1 (10): External wake up interrupt >>>> - GPQ0 (2): SPMI (PMIC I/F) >>>> - GPB0,GPB1,GPB2,GPB3,GPB4,GPB5,GPB6 (47): I2S Audio >>>> - GPH0,GPH1,GPH2,GPH3,GPH4,GPH5,GPH6,GPH8 (49): PCIE, UFS, Ethernet >>>> - GPG0,GPG1,GPG2,GPG3,GPG4,GPG5 (29): General purpose >>>> - GPP0,GPP1,GPP2,GPP3,GPP4,GPP5,GPP6,GPP7,GPP8,GPP9,GPP10 (77): USI >>>> >>>> Signed-off-by: Jaewon Kim<jaewon02.kim@samsung.com> >>>> --- >>>> .../pinctrl/samsung/pinctrl-exynos-arm64.c | 140 ++++++++++++++++++ >>>> drivers/pinctrl/samsung/pinctrl-exynos.c | 102 ++++++++++++- >>>> drivers/pinctrl/samsung/pinctrl-exynos.h | 27 ++++ >>>> drivers/pinctrl/samsung/pinctrl-samsung.c | 5 + >>>> drivers/pinctrl/samsung/pinctrl-samsung.h | 13 ++ >>>> 5 files changed, 280 insertions(+), 7 deletions(-) >>>> >>>> diff --git a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c >>>> index cb965cf93705..cf86722a70a3 100644 >>>> --- a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c >>>> +++ b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c >>>> @@ -796,3 +796,143 @@ const struct samsung_pinctrl_of_match_data fsd_of_data __initconst = { >>>> .ctrl = fsd_pin_ctrl, >>>> .num_ctrl = ARRAY_SIZE(fsd_pin_ctrl), >>>> }; >>>> + >>>> +/* pin banks of exynosautov920 pin-controller 0 (ALIVE) */ >>>> +static struct samsung_pin_bank_data exynosautov920_pin_banks0[] = { >>> So you created patch from some downstream code? No, please work on >>> upstream. Take upstream code and customize it to your needs. That way >>> you won't introduce same mistakes fixes years ago. >>> >>> Missing const. >> Thanks for the guide. >> >> I didn`t work on downstream source, but when I copy/paste >> >> the struct enumerations from downstream, it seemed like > That's what I am talking about. Don't do like this. > > We fixed several things in Linux kernel, so copying unfixed code is > wasting of everyone's time. Don't work on downstream. Don't copy > anything from downstream. You *MUST CUSTOMIZE* upstream file, not > downstream. Got it. I will not copy from downstream code. > > >> 'const' was missing. >> >>> ... >>> >>>> @@ -31,6 +31,7 @@ >>>> #define EXYNOS7_WKUP_EMASK_OFFSET 0x900 >>>> #define EXYNOS7_WKUP_EPEND_OFFSET 0xA00 >>>> #define EXYNOS_SVC_OFFSET 0xB08 >>>> +#define EXYNOSAUTOV920_SVC_OFFSET 0xF008 >>>> >>> ... >>> >>>> #ifdef CONFIG_PINCTRL_S3C64XX >>>> { .compatible = "samsung,s3c64xx-pinctrl", >>>> diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.h b/drivers/pinctrl/samsung/pinctrl-samsung.h >>>> index 9b3db50adef3..cbb78178651b 100644 >>>> --- a/drivers/pinctrl/samsung/pinctrl-samsung.h >>>> +++ b/drivers/pinctrl/samsung/pinctrl-samsung.h >>>> @@ -122,6 +122,9 @@ struct samsung_pin_bank_type { >>>> * @eint_type: type of the external interrupt supported by the bank. >>>> * @eint_mask: bit mask of pins which support EINT function. >>>> * @eint_offset: SoC-specific EINT register or interrupt offset of bank. >>>> + * @mask_offset: SoC-specific EINT mask register offset of bank. >>>> + * @pend_offset: SoC-specific EINT pend register offset of bank. >>>> + * @combine: EINT register is adjacent to the GPIO control register. >>> I don't understand it. Adjacent? Are you sure? GPIO control register has >>> 0xF004 (EXYNOSAUTOV920_SVC_OFFSET + 0x4)? Anyway, this does not scale. >>> What if next revision comes with not-adjacent. There will be >>> "combine_plus"? Also name confuses me - combine means together. >>> >>> Also your first map of registers does not have it adjacent... >> I think I should have added a little more information about new struct. >> >> ------------------------------------------------- >> | original | ExynosAutov920 | >> |-----------------------------------------------| >> | 0x0 GPA_CON | 0x0 GPA_CON | >> | 0x4 GPA_DAT | 0x4 GPA_DAT | >> | 0x8 GPA_PUD | 0x8 GPA_PUD | >> | 0xc GPA_DRV | 0xc GPA_DRV | >> |----------------------| 0x18 EINT_GPA_CON | >> | 0x20 GPB_CON | 0x1c EINT_GPA_FLTCON0| >> | 0x4 GPB_DAT | 0x20 EINT_GPA_FLTCON1| >> | 0x28 GPB_PUD | 0x24 EINT_GPA_MASK | >> | 0x2c GPB_DRV | 0x28 EINT_GPA_PEND | >> |----------------------|------------------------| >> | 0x700 EINT_GPA_CON | 0x1000 GPA_CON | >> | 0x704 EINT_GPB_CON | 0x1004 GPA_DAT | >> |----------------------| 0x1008 GPA_PUD | >> | 0x800 EINT_GPA_FLTCON| 0x100c GPA_DRV | >> | 0x804 EINT_GPB_FLTCON| 0x1018 EINT_GPA_CON | >> |----------------------| 0x101c EINT_GPA_FLTCON0| >> | 0x900 EINT_GPA_MASK | 0x1020 EINT_GPA_FLTCON1| >> | 0x904 EINT_GPB_MASK | 0x1024 EINT_GPA_MASK | >> |----------------------| 0x1028 EINT_GPA_PEND | >> | 0xa00 EINT_GPA_PEND |------------------------| >> | 0xa04 EINT_GPB_PEND | | >> ------------------------------------------------| >> | 0xb08 SVC | 0xf008 SVC | >> ------------------------------------------------- >> >> The reason why I chose variable name 'combine' is that EINT registers was >> separated from gpio control address. However, in exynosautov920 EINT >> registers combined with GPx group. So I chose "combine" word. > What does it mean "the GPx group"? Combined means the same place, the > same register. I could imagine offset is 0x4, what I wrote last time. > > Is the offset 0x4? > > >> Is another reasonable word, I will change it. > > Why you cannot store the offset? > >> EINT registers related to the entire group(e.g SVC) were at the end of >> the GPIO block and are now moved to 0xf000. > So not in the same register, not combined? > Okay, Instead of the word combine, I will think of a better word in next version. Thanks Jaewon Kim
On 23. 11. 16. 20:17, Krzysztof Kozlowski wrote: > On 16/11/2023 04:50, Jaewon Kim wrote: >> On 23. 11. 15. 21:42, Krzysztof Kozlowski wrote: >>> On 15/11/2023 10:56, Jaewon Kim wrote: >>>> ExynosAutov920 GPIO has a different register structure. >>>> In the existing Exynos series, EINT control register enumerated after >>>> a specific offset (e.g EXYNOS_GPIO_ECON_OFFSET). >>>> However, in ExynosAutov920 SoC, the register that controls EINT belongs >>>> to each GPIO group, and each GPIO group has 0x1000 align. >>>> >>>> This is a structure to protect the GPIO group with S2MPU in VM environment, >>>> and will only be applied in ExynosAuto series SoCs. >>> Checkpatch points some warnings: >>> >>> CHECK: Alignment should match open parenthesis >>> CHECK: Lines should not end with a '(' >>> CHECK: Macro argument reuse 'reg' - possible side-effects? >> I don`t know this happens. >> >> When I did the checkpatch, there were no problems as shown below. > Didn't you miss some arguments? Lime --strict? > From next time, I will check with --strict option. Thanks Krzysztof. Thanks Jaewon Kim
On 17/11/2023 08:36, Jaewon Kim wrote: >>> The reason why I chose variable name 'combine' is that EINT registers was >>> separated from gpio control address. However, in exynosautov920 EINT >>> registers combined with GPx group. So I chose "combine" word. >> What does it mean "the GPx group"? Combined means the same place, the >> same register. I could imagine offset is 0x4, what I wrote last time. >> >> Is the offset 0x4? >> >> >>> Is another reasonable word, I will change it. >> >> Why you cannot store the offset? >> >>> EINT registers related to the entire group(e.g SVC) were at the end of >>> the GPIO block and are now moved to 0xf000. >> So not in the same register, not combined? >> > Okay, > > Instead of the word combine, I will think of a better word in next version. I want to know answer to: "Why you cannot store the offset?" Best regards, Krzysztof
On 23. 11. 17. 19:48, Krzysztof Kozlowski wrote: > On 17/11/2023 08:36, Jaewon Kim wrote: >>>> The reason why I chose variable name 'combine' is that EINT registers was >>>> separated from gpio control address. However, in exynosautov920 EINT >>>> registers combined with GPx group. So I chose "combine" word. >>> What does it mean "the GPx group"? Combined means the same place, the >>> same register. I could imagine offset is 0x4, what I wrote last time. >>> >>> Is the offset 0x4? If you are asking about the offset of GPIO control register and EINT control register, 0x4 is correct. There is no empty space between the two register. 0x0 CON 0x4 DAT 0x8 PUD 0xc DRV 0x10 CONPDN 0x14 PUDPDN 0x18 EINT_CON 0x1c EINT_FLTCON 0x20 or 0x24 EINT_MASK (The size of FLTCON register depending on the number of gpio) 0x24 or 0x28 EINT_PEND >>> >>> >>>> Is another reasonable word, I will change it. >>> Why you cannot store the offset? >>> >>>> EINT registers related to the entire group(e.g SVC) were at the end of >>>> the GPIO block and are now moved to 0xf000. >>> So not in the same register, not combined? >>> >> Okay, >> >> Instead of the word combine, I will think of a better word in next version. > I want to know answer to: > > "Why you cannot store the offset?" > I did not understand exactly what you said, but if i guess,, you want to get rid of the offs because the value of the offs is always the same? #define EXYNOSV920_PIN_BANK_EINTG(pins, reg, id, offs, mask_offs, pend_offs) Thanks Jaewon Kim
On 18/11/2023 08:43, Jaewon Kim wrote: > > On 23. 11. 17. 19:48, Krzysztof Kozlowski wrote: >> On 17/11/2023 08:36, Jaewon Kim wrote: >>>>> The reason why I chose variable name 'combine' is that EINT registers was >>>>> separated from gpio control address. However, in exynosautov920 EINT >>>>> registers combined with GPx group. So I chose "combine" word. >>>> What does it mean "the GPx group"? Combined means the same place, the >>>> same register. I could imagine offset is 0x4, what I wrote last time. >>>> >>>> Is the offset 0x4? > > If you are asking about the offset of GPIO control register and EINT > control register, 0x4 is correct. > > There is no empty space between the two register. > > > 0x0 CON > > 0x4 DAT > > 0x8 PUD > > 0xc DRV > > 0x10 CONPDN > > 0x14 PUDPDN > > 0x18 EINT_CON > > 0x1c EINT_FLTCON > > 0x20 or 0x24 EINT_MASK (The size of FLTCON register depending on the > number of gpio) > > 0x24 or 0x28 EINT_PEND > > >>>> >>>> >>>>> Is another reasonable word, I will change it. >>>> Why you cannot store the offset? >>>> >>>>> EINT registers related to the entire group(e.g SVC) were at the end of >>>>> the GPIO block and are now moved to 0xf000. >>>> So not in the same register, not combined? >>>> >>> Okay, >>> >>> Instead of the word combine, I will think of a better word in next version. >> I want to know answer to: >> >> "Why you cannot store the offset?" >> > I did not understand exactly what you said, but if i guess,, > > you want to get rid of the offs because the value of the offs is always > the same? > > #define EXYNOSV920_PIN_BANK_EINTG(pins, reg, id, offs, mask_offs, pend_offs) I meant that it looks possible to store the offset and use it directly, instead of storing bool telling that offset is different. Best regards, Krzysztof
diff --git a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c index cb965cf93705..cf86722a70a3 100644 --- a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c +++ b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c @@ -796,3 +796,143 @@ const struct samsung_pinctrl_of_match_data fsd_of_data __initconst = { .ctrl = fsd_pin_ctrl, .num_ctrl = ARRAY_SIZE(fsd_pin_ctrl), }; + +/* pin banks of exynosautov920 pin-controller 0 (ALIVE) */ +static struct samsung_pin_bank_data exynosautov920_pin_banks0[] = { + EXYNOSV920_PIN_BANK_EINTW(8, 0x0000, "gpa0", 0x18, 0x24, 0x28), + EXYNOSV920_PIN_BANK_EINTW(2, 0x1000, "gpa1", 0x18, 0x20, 0x24), + EXYNOS850_PIN_BANK_EINTN(2, 0x2000, "gpq0"), +}; + +/* pin banks of exynosautov920 pin-controller 1 (AUD) */ +static struct samsung_pin_bank_data exynosautov920_pin_banks1[] = { + EXYNOSV920_PIN_BANK_EINTG(7, 0x0000, "gpb0", 0x18, 0x24, 0x28), + EXYNOSV920_PIN_BANK_EINTG(6, 0x1000, "gpb1", 0x18, 0x24, 0x28), + EXYNOSV920_PIN_BANK_EINTG(8, 0x2000, "gpb2", 0x18, 0x24, 0x28), + EXYNOSV920_PIN_BANK_EINTG(8, 0x3000, "gpb3", 0x18, 0x24, 0x28), + EXYNOSV920_PIN_BANK_EINTG(8, 0x4000, "gpb4", 0x18, 0x24, 0x28), + EXYNOSV920_PIN_BANK_EINTG(5, 0x5000, "gpb5", 0x18, 0x24, 0x28), + EXYNOSV920_PIN_BANK_EINTG(5, 0x6000, "gpb6", 0x18, 0x24, 0x28), +}; + +/* pin banks of exynosautov920 pin-controller 2 (HSI0) */ +static struct samsung_pin_bank_data exynosautov920_pin_banks2[] = { + EXYNOSV920_PIN_BANK_EINTG(6, 0x0000, "gph0", 0x18, 0x24, 0x28), + EXYNOSV920_PIN_BANK_EINTG(2, 0x1000, "gph1", 0x18, 0x20, 0x24), +}; + +/* pin banks of exynosautov920 pin-controller 3 (HSI1) */ +static struct samsung_pin_bank_data exynosautov920_pin_banks3[] = { + EXYNOSV920_PIN_BANK_EINTG(7, 0x000, "gph8", 0x18, 0x24, 0x28), +}; + +/* pin banks of exynosautov920 pin-controller 4 (HSI2) */ +static struct samsung_pin_bank_data exynosautov920_pin_banks4[] = { + EXYNOSV920_PIN_BANK_EINTG(8, 0x0000, "gph3", 0x18, 0x24, 0x28), + EXYNOSV920_PIN_BANK_EINTG(7, 0x1000, "gph4", 0x18, 0x24, 0x28), + EXYNOSV920_PIN_BANK_EINTG(8, 0x2000, "gph5", 0x18, 0x24, 0x28), + EXYNOSV920_PIN_BANK_EINTG(7, 0x3000, "gph6", 0x18, 0x24, 0x28), +}; + +/* pin banks of exynosautov920 pin-controller 5 (HSI2UFS) */ +static struct samsung_pin_bank_data exynosautov920_pin_banks5[] = { + EXYNOSV920_PIN_BANK_EINTG(4, 0x000, "gph2", 0x18, 0x20, 0x24), +}; + +/* pin banks of exynosautov920 pin-controller 6 (PERIC0) */ +static struct samsung_pin_bank_data exynosautov920_pin_banks6[] = { + EXYNOSV920_PIN_BANK_EINTG(8, 0x0000, "gpp0", 0x18, 0x24, 0x28), + EXYNOSV920_PIN_BANK_EINTG(8, 0x1000, "gpp1", 0x18, 0x24, 0x28), + EXYNOSV920_PIN_BANK_EINTG(8, 0x2000, "gpp2", 0x18, 0x24, 0x28), + EXYNOSV920_PIN_BANK_EINTG(5, 0x3000, "gpg0", 0x18, 0x24, 0x28), + EXYNOSV920_PIN_BANK_EINTG(8, 0x4000, "gpp3", 0x18, 0x24, 0x28), + EXYNOSV920_PIN_BANK_EINTG(4, 0x5000, "gpp4", 0x18, 0x20, 0x24), + EXYNOSV920_PIN_BANK_EINTG(4, 0x6000, "gpg2", 0x18, 0x20, 0x24), + EXYNOSV920_PIN_BANK_EINTG(4, 0x7000, "gpg5", 0x18, 0x20, 0x24), + EXYNOSV920_PIN_BANK_EINTG(3, 0x8000, "gpg3", 0x18, 0x20, 0x24), + EXYNOSV920_PIN_BANK_EINTG(5, 0x9000, "gpg4", 0x18, 0x24, 0x28), +}; + +/* pin banks of exynosautov920 pin-controller 7 (PERIC1) */ +static struct samsung_pin_bank_data exynosautov920_pin_banks7[] = { + EXYNOSV920_PIN_BANK_EINTG(8, 0x0000, "gpp5", 0x18, 0x24, 0x28), + EXYNOSV920_PIN_BANK_EINTG(5, 0x1000, "gpp6", 0x18, 0x24, 0x28), + EXYNOSV920_PIN_BANK_EINTG(4, 0x2000, "gpp10", 0x18, 0x20, 0x24), + EXYNOSV920_PIN_BANK_EINTG(8, 0x3000, "gpp7", 0x18, 0x24, 0x28), + EXYNOSV920_PIN_BANK_EINTG(4, 0x4000, "gpp8", 0x18, 0x20, 0x24), + EXYNOSV920_PIN_BANK_EINTG(4, 0x5000, "gpp11", 0x18, 0x20, 0x24), + EXYNOSV920_PIN_BANK_EINTG(4, 0x6000, "gpp9", 0x18, 0x20, 0x24), + EXYNOSV920_PIN_BANK_EINTG(4, 0x7000, "gpp12", 0x18, 0x20, 0x24), + EXYNOSV920_PIN_BANK_EINTG(8, 0x8000, "gpg1", 0x18, 0x24, 0x28), +}; + +static const struct samsung_retention_data exynosautov920_retention_data __initconst = { + .regs = NULL, + .nr_regs = 0, + .value = 0, + .refcnt = &exynos_shared_retention_refcnt, + .init = exynos_retention_init, +}; + +const struct samsung_pin_ctrl exynosautov920_pin_ctrl[] = { + { + /* pin-controller instance 0 ALIVE data */ + .pin_banks = exynosautov920_pin_banks0, + .nr_banks = ARRAY_SIZE(exynosautov920_pin_banks0), + .eint_wkup_init = exynos_eint_wkup_init, + .suspend = exynos_pinctrl_suspend, + .resume = exynos_pinctrl_resume, + .retention_data = &exynosautov920_retention_data, + }, { + /* pin-controller instance 1 AUD data */ + .pin_banks = exynosautov920_pin_banks1, + .nr_banks = ARRAY_SIZE(exynosautov920_pin_banks1), + }, { + /* pin-controller instance 2 HSI0 data */ + .pin_banks = exynosautov920_pin_banks2, + .nr_banks = ARRAY_SIZE(exynosautov920_pin_banks2), + .eint_gpio_init = exynos_eint_gpio_init, + .suspend = exynos_pinctrl_suspend, + .resume = exynos_pinctrl_resume, + }, { + /* pin-controller instance 2 HSI1 data */ + .pin_banks = exynosautov920_pin_banks3, + .nr_banks = ARRAY_SIZE(exynosautov920_pin_banks3), + .eint_gpio_init = exynos_eint_gpio_init, + .suspend = exynos_pinctrl_suspend, + .resume = exynos_pinctrl_resume, + }, { + /* pin-controller instance 2 HSI2 data */ + .pin_banks = exynosautov920_pin_banks4, + .nr_banks = ARRAY_SIZE(exynosautov920_pin_banks4), + .eint_gpio_init = exynos_eint_gpio_init, + .suspend = exynos_pinctrl_suspend, + .resume = exynos_pinctrl_resume, + }, { + /* pin-controller instance 2 HSI2UFS data */ + .pin_banks = exynosautov920_pin_banks5, + .nr_banks = ARRAY_SIZE(exynosautov920_pin_banks5), + .eint_gpio_init = exynos_eint_gpio_init, + .suspend = exynos_pinctrl_suspend, + .resume = exynos_pinctrl_resume, + }, { + /* pin-controller instance 2 PERIC0 data */ + .pin_banks = exynosautov920_pin_banks6, + .nr_banks = ARRAY_SIZE(exynosautov920_pin_banks6), + .eint_gpio_init = exynos_eint_gpio_init, + .suspend = exynos_pinctrl_suspend, + .resume = exynos_pinctrl_resume, + }, { + /* pin-controller instance 2 PERIC1 data */ + .pin_banks = exynosautov920_pin_banks7, + .nr_banks = ARRAY_SIZE(exynosautov920_pin_banks7), + .eint_gpio_init = exynos_eint_gpio_init, + .suspend = exynos_pinctrl_suspend, + .resume = exynos_pinctrl_resume, + }, +}; + +const struct samsung_pinctrl_of_match_data exynosautov920_of_data __initconst = { + .ctrl = exynosautov920_pin_ctrl, + .num_ctrl = ARRAY_SIZE(exynosautov920_pin_ctrl), +}; diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.c b/drivers/pinctrl/samsung/pinctrl-exynos.c index 6b58ec84e34b..b1bf44ee09db 100644 --- a/drivers/pinctrl/samsung/pinctrl-exynos.c +++ b/drivers/pinctrl/samsung/pinctrl-exynos.c @@ -56,6 +56,9 @@ static void exynos_irq_mask(struct irq_data *irqd) unsigned int mask; unsigned long flags; + if (bank->combine) + reg_mask = bank->mask_offset; + raw_spin_lock_irqsave(&bank->slock, flags); mask = readl(bank->eint_base + reg_mask); @@ -72,6 +75,9 @@ static void exynos_irq_ack(struct irq_data *irqd) struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(irqd); unsigned long reg_pend = our_chip->eint_pend + bank->eint_offset; + if (bank->combine) + reg_pend = bank->pend_offset; + writel(1 << irqd->hwirq, bank->eint_base + reg_pend); } @@ -95,6 +101,9 @@ static void exynos_irq_unmask(struct irq_data *irqd) if (irqd_get_trigger_type(irqd) & IRQ_TYPE_LEVEL_MASK) exynos_irq_ack(irqd); + if (bank->combine) + reg_mask = bank->mask_offset; + raw_spin_lock_irqsave(&bank->slock, flags); mask = readl(bank->eint_base + reg_mask); @@ -221,6 +230,22 @@ static const struct exynos_irq_chip exynos_gpio_irq_chip __initconst = { /* eint_wake_mask_value not used */ }; +static const struct exynos_irq_chip exynosautov920_gpio_irq_chip __initconst = { + .chip = { + .name = "exynosautov920_gpio_irq_chip", + .irq_unmask = exynos_irq_unmask, + .irq_mask = exynos_irq_mask, + .irq_ack = exynos_irq_ack, + .irq_set_type = exynos_irq_set_type, + .irq_request_resources = exynos_irq_request_resources, + .irq_release_resources = exynos_irq_release_resources, + }, + .eint_con = 0, + .eint_mask = 0, + .eint_pend = 0, + /* eint_wake_mask_value not used */ +}; + static int exynos_eint_irq_map(struct irq_domain *h, unsigned int virq, irq_hw_number_t hw) { @@ -247,7 +272,10 @@ static irqreturn_t exynos_eint_gpio_irq(int irq, void *data) unsigned int svc, group, pin; int ret; - svc = readl(bank->eint_base + EXYNOS_SVC_OFFSET); + if (bank->combine) + svc = readl(bank->eint_base + EXYNOSAUTOV920_SVC_OFFSET); + else + svc = readl(bank->eint_base + EXYNOS_SVC_OFFSET); group = EXYNOS_SVC_GROUP(svc); pin = svc & EXYNOS_SVC_NUM_MASK; @@ -297,8 +325,12 @@ __init int exynos_eint_gpio_init(struct samsung_pinctrl_drv_data *d) if (bank->eint_type != EINT_TYPE_GPIO) continue; - bank->irq_chip = devm_kmemdup(dev, &exynos_gpio_irq_chip, - sizeof(*bank->irq_chip), GFP_KERNEL); + if (bank->combine) + bank->irq_chip = devm_kmemdup(dev, &exynosautov920_gpio_irq_chip, + sizeof(*bank->irq_chip), GFP_KERNEL); + else + bank->irq_chip = devm_kmemdup(dev, &exynos_gpio_irq_chip, + sizeof(*bank->irq_chip), GFP_KERNEL); if (!bank->irq_chip) { ret = -ENOMEM; goto err_domains; @@ -456,6 +488,22 @@ static const struct exynos_irq_chip exynos7_wkup_irq_chip __initconst = { .set_eint_wakeup_mask = exynos_pinctrl_set_eint_wakeup_mask, }; +static const struct exynos_irq_chip exynosautov920_wkup_irq_chip __initconst = { + .chip = { + .name = "exynosautov920_wkup_irq_chip", + .irq_unmask = exynos_irq_unmask, + .irq_mask = exynos_irq_mask, + .irq_ack = exynos_irq_ack, + .irq_set_type = exynos_irq_set_type, + .irq_set_wake = exynos_wkup_irq_set_wake, + .irq_request_resources = exynos_irq_request_resources, + .irq_release_resources = exynos_irq_release_resources, + }, + .eint_wake_mask_value = &eint_wake_mask_value, + .eint_wake_mask_reg = EXYNOS5433_EINT_WAKEUP_MASK, + .set_eint_wakeup_mask = exynos_pinctrl_set_eint_wakeup_mask, +}; + /* list of external wakeup controllers supported */ static const struct of_device_id exynos_wkup_irq_ids[] = { { .compatible = "samsung,s5pv210-wakeup-eint", @@ -468,6 +516,8 @@ static const struct of_device_id exynos_wkup_irq_ids[] = { .data = &exynos7_wkup_irq_chip }, { .compatible = "samsung,exynosautov9-wakeup-eint", .data = &exynos7_wkup_irq_chip }, + { .compatible = "samsung,exynosautov920-wakeup-eint", + .data = &exynosautov920_wkup_irq_chip }, { } }; @@ -655,6 +705,20 @@ static void exynos_pinctrl_suspend_bank( pr_debug("%s: save mask %#010x\n", bank->name, save->eint_mask); } +static void exynosautov920_pinctrl_suspend_bank( + struct samsung_pinctrl_drv_data *drvdata, + struct samsung_pin_bank *bank) +{ + struct exynos_eint_gpio_save *save = bank->soc_priv; + void __iomem *regs = bank->eint_base; + + save->eint_con = readl(regs + bank->eint_offset); + save->eint_mask = readl(regs + bank->mask_offset); + + pr_debug("%s: save con %#010x\n", bank->name, save->eint_con); + pr_debug("%s: save mask %#010x\n", bank->name, save->eint_mask); +} + void exynos_pinctrl_suspend(struct samsung_pinctrl_drv_data *drvdata) { struct samsung_pin_bank *bank = drvdata->pin_banks; @@ -662,8 +726,12 @@ void exynos_pinctrl_suspend(struct samsung_pinctrl_drv_data *drvdata) int i; for (i = 0; i < drvdata->nr_banks; ++i, ++bank) { - if (bank->eint_type == EINT_TYPE_GPIO) - exynos_pinctrl_suspend_bank(drvdata, bank); + if (bank->eint_type == EINT_TYPE_GPIO) { + if (bank->combine) + exynosautov920_pinctrl_suspend_bank(drvdata, bank); + else + exynos_pinctrl_suspend_bank(drvdata, bank); + } else if (bank->eint_type == EINT_TYPE_WKUP) { if (!irq_chip) { irq_chip = bank->irq_chip; @@ -704,14 +772,34 @@ static void exynos_pinctrl_resume_bank( + bank->eint_offset); } +static void exynosautov920_pinctrl_resume_bank( + struct samsung_pinctrl_drv_data *drvdata, + struct samsung_pin_bank *bank) +{ + struct exynos_eint_gpio_save *save = bank->soc_priv; + void __iomem *regs = bank->eint_base; + + pr_debug("%s: con %#010x => %#010x\n", bank->name, + readl(regs + bank->eint_offset), save->eint_con); + pr_debug("%s: mask %#010x => %#010x\n", bank->name, + readl(regs + bank->mask_offset), save->eint_mask); + + writel(save->eint_con, regs + bank->eint_offset); + writel(save->eint_mask, regs + bank->mask_offset); +} + void exynos_pinctrl_resume(struct samsung_pinctrl_drv_data *drvdata) { struct samsung_pin_bank *bank = drvdata->pin_banks; int i; for (i = 0; i < drvdata->nr_banks; ++i, ++bank) - if (bank->eint_type == EINT_TYPE_GPIO) - exynos_pinctrl_resume_bank(drvdata, bank); + if (bank->eint_type == EINT_TYPE_GPIO) { + if (bank->combine) + exynosautov920_pinctrl_resume_bank(drvdata, bank); + else + exynos_pinctrl_resume_bank(drvdata, bank); + } } static void exynos_retention_enable(struct samsung_pinctrl_drv_data *drvdata) diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.h b/drivers/pinctrl/samsung/pinctrl-exynos.h index 3ac52c2cf998..210952c7a5aa 100644 --- a/drivers/pinctrl/samsung/pinctrl-exynos.h +++ b/drivers/pinctrl/samsung/pinctrl-exynos.h @@ -31,6 +31,7 @@ #define EXYNOS7_WKUP_EMASK_OFFSET 0x900 #define EXYNOS7_WKUP_EPEND_OFFSET 0xA00 #define EXYNOS_SVC_OFFSET 0xB08 +#define EXYNOSAUTOV920_SVC_OFFSET 0xF008 /* helpers to access interrupt service register */ #define EXYNOS_SVC_GROUP_SHIFT 3 @@ -140,6 +141,32 @@ .name = id \ } +#define EXYNOSV920_PIN_BANK_EINTG(pins, reg, id, offs, mask_offs, pend_offs) \ + { \ + .type = &exynos850_bank_type_off, \ + .pctl_offset = reg, \ + .nr_pins = pins, \ + .eint_type = EINT_TYPE_GPIO, \ + .eint_offset = (reg + offs), \ + .mask_offset = (reg + mask_offs), \ + .pend_offset = (reg + pend_offs), \ + .combine = true, \ + .name = id \ + } + +#define EXYNOSV920_PIN_BANK_EINTW(pins, reg, id, offs, mask_offs, pend_offs) \ + { \ + .type = &exynos850_bank_type_alive, \ + .pctl_offset = reg, \ + .nr_pins = pins, \ + .eint_type = EINT_TYPE_WKUP, \ + .eint_offset = (reg + offs), \ + .mask_offset = (reg + mask_offs), \ + .pend_offset = (reg + pend_offs), \ + .combine = true, \ + .name = id \ + } + /** * struct exynos_weint_data: irq specific data for all the wakeup interrupts * generated by the external wakeup interrupt controller. diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.c b/drivers/pinctrl/samsung/pinctrl-samsung.c index 79babbb39ced..adf2d0cff438 100644 --- a/drivers/pinctrl/samsung/pinctrl-samsung.c +++ b/drivers/pinctrl/samsung/pinctrl-samsung.c @@ -1106,6 +1106,9 @@ samsung_pinctrl_get_soc_data(struct samsung_pinctrl_drv_data *d, bank->eint_type = bdata->eint_type; bank->eint_mask = bdata->eint_mask; bank->eint_offset = bdata->eint_offset; + bank->mask_offset = bdata->mask_offset; + bank->pend_offset = bdata->pend_offset; + bank->combine = bdata->combine; bank->name = bdata->name; raw_spin_lock_init(&bank->slock); @@ -1321,6 +1324,8 @@ static const struct of_device_id samsung_pinctrl_dt_match[] = { .data = &exynosautov9_of_data }, { .compatible = "tesla,fsd-pinctrl", .data = &fsd_of_data }, + { .compatible = "samsung,exynosautov920-pinctrl", + .data = &exynosautov920_of_data }, #endif #ifdef CONFIG_PINCTRL_S3C64XX { .compatible = "samsung,s3c64xx-pinctrl", diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.h b/drivers/pinctrl/samsung/pinctrl-samsung.h index 9b3db50adef3..cbb78178651b 100644 --- a/drivers/pinctrl/samsung/pinctrl-samsung.h +++ b/drivers/pinctrl/samsung/pinctrl-samsung.h @@ -122,6 +122,9 @@ struct samsung_pin_bank_type { * @eint_type: type of the external interrupt supported by the bank. * @eint_mask: bit mask of pins which support EINT function. * @eint_offset: SoC-specific EINT register or interrupt offset of bank. + * @mask_offset: SoC-specific EINT mask register offset of bank. + * @pend_offset: SoC-specific EINT pend register offset of bank. + * @combine: EINT register is adjacent to the GPIO control register. * @name: name to be prefixed for each pin in this pin bank. */ struct samsung_pin_bank_data { @@ -133,6 +136,9 @@ struct samsung_pin_bank_data { enum eint_type eint_type; u32 eint_mask; u32 eint_offset; + u32 mask_offset; + u32 pend_offset; + bool combine; const char *name; }; @@ -147,6 +153,9 @@ struct samsung_pin_bank_data { * @eint_type: type of the external interrupt supported by the bank. * @eint_mask: bit mask of pins which support EINT function. * @eint_offset: SoC-specific EINT register or interrupt offset of bank. + * @mask_offset: SoC-specific EINT mask register offset of bank. + * @pend_offset: SoC-specific EINT pend register offset of bank. + * @combine: EINT register is adjacent to the GPIO control register. * @name: name to be prefixed for each pin in this pin bank. * @id: id of the bank, propagated to the pin range. * @pin_base: starting pin number of the bank. @@ -170,6 +179,9 @@ struct samsung_pin_bank { enum eint_type eint_type; u32 eint_mask; u32 eint_offset; + u32 mask_offset; + u32 pend_offset; + bool combine; const char *name; u32 id; @@ -350,6 +362,7 @@ extern const struct samsung_pinctrl_of_match_data exynos7_of_data; extern const struct samsung_pinctrl_of_match_data exynos7885_of_data; extern const struct samsung_pinctrl_of_match_data exynos850_of_data; extern const struct samsung_pinctrl_of_match_data exynosautov9_of_data; +extern const struct samsung_pinctrl_of_match_data exynosautov920_of_data; extern const struct samsung_pinctrl_of_match_data fsd_of_data; extern const struct samsung_pinctrl_of_match_data s3c64xx_of_data; extern const struct samsung_pinctrl_of_match_data s3c2412_of_data;
ExynosAutov920 GPIO has a different register structure. In the existing Exynos series, EINT control register enumerated after a specific offset (e.g EXYNOS_GPIO_ECON_OFFSET). However, in ExynosAutov920 SoC, the register that controls EINT belongs to each GPIO group, and each GPIO group has 0x1000 align. This is a structure to protect the GPIO group with S2MPU in VM environment, and will only be applied in ExynosAuto series SoCs. Example) ------------------------------------------------- | original | ExynosAutov920 | |-----------------------------------------------| | 0x0 GPIO_CON | 0x0 GPIO_CON | | 0x4 GPIO_DAT | 0x4 GPIO_DAT | | 0x8 GPIO_PUD | 0x8 GPIO_PUD | | 0xc GPIO_DRV | 0xc GPIO_DRV | | 0x700 EINT_CON | 0x18 EINT_CON | | 0x800 EINT_FLTCON | 0x1c EINT_FLTCON0 | | 0x900 EINT_MASK | 0x20 EINT_FLTCON1 | | 0xa00 EINT_PEND | 0x24 EINT_MASK | | | 0x28 EINT_PEND | ------------------------------------------------- Pinctrl data for ExynosAutoV920 SoC. - GPA0,GPA1 (10): External wake up interrupt - GPQ0 (2): SPMI (PMIC I/F) - GPB0,GPB1,GPB2,GPB3,GPB4,GPB5,GPB6 (47): I2S Audio - GPH0,GPH1,GPH2,GPH3,GPH4,GPH5,GPH6,GPH8 (49): PCIE, UFS, Ethernet - GPG0,GPG1,GPG2,GPG3,GPG4,GPG5 (29): General purpose - GPP0,GPP1,GPP2,GPP3,GPP4,GPP5,GPP6,GPP7,GPP8,GPP9,GPP10 (77): USI Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com> --- .../pinctrl/samsung/pinctrl-exynos-arm64.c | 140 ++++++++++++++++++ drivers/pinctrl/samsung/pinctrl-exynos.c | 102 ++++++++++++- drivers/pinctrl/samsung/pinctrl-exynos.h | 27 ++++ drivers/pinctrl/samsung/pinctrl-samsung.c | 5 + drivers/pinctrl/samsung/pinctrl-samsung.h | 13 ++ 5 files changed, 280 insertions(+), 7 deletions(-)