* [PATCH 4/7] pinctrl: samsung: Add GPFx support of Exynos5433 [not found] <1471328843-26653-1-git-send-email-cw00.choi@samsung.com> @ 2016-08-16 6:27 ` Chanwoo Choi 2016-08-16 6:42 ` Tomasz Figa ` (3 more replies) 0 siblings, 4 replies; 7+ messages in thread From: Chanwoo Choi @ 2016-08-16 6:27 UTC (permalink / raw) To: k.kozlowski, kgene, robh+dt, mark.rutland, catalin.marinas, will.deacon, devicetree, linux-arm-kernel, linux-samsung-soc, linux-kernel Cc: krzk, cw00.choi, jh80.chung, sw0312.kim, jy0922.shim, inki.dae, jonghwa3.lee, beomho.seo, jaewon02.kim, human.hwang, ideal.song, ingi2.kim, m.szyprowski, a.hajda, s.nawrocki, chanwoo, Linus Walleij, Tomasz Figa, linux-gpio From: Joonyoung Shim <jy0922.shim@samsung.com> This patch add the support of GPFx pin of Exynos5433 SoC. Exynos5433 has different memory map of GPFx from previous Exynos SoC. Exynos GPIO has following register to control gpio funciton. Usually, all registers of GPIO are included in same domain. - CON / DAT / PUD / DRV / CONPDN / PUDPDN - EINT_CON/ EINT_FLTCON0, EINT_FLTCON1 / EINT_MASK / EINT_PEND But, GPFx are included in two domain as following. So, this patch supports the GPFx pin which handle the on separate two domains. - ALIVE domain : CON / DAT / PUD / DRV / CONPDN / PUDPDN - IMEM domain : EINT_CON/ EINT_FLTCON0, EINT_FLTCON1 / EINT_MASK / EINT_PEND Cc: Linus Walleij <linus.walleij@linaro.org> Cc: Rob Herring <robh+dt@kernel.org> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Tomasz Figa <tomasz.figa@gmail.com> Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com> Cc: Sylwester Nawrocki <s.nawrocki@samsung.com> Cc: Kukjin Kim <kgene@kernel.org> Cc: linux-gpio@vger.kernel.org Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com> --- .../bindings/pinctrl/samsung-pinctrl.txt | 1 + drivers/pinctrl/samsung/pinctrl-exynos.c | 5 +++ drivers/pinctrl/samsung/pinctrl-exynos.h | 11 ++++++ drivers/pinctrl/samsung/pinctrl-samsung.c | 43 ++++++++++++++++++---- drivers/pinctrl/samsung/pinctrl-samsung.h | 5 +++ 5 files changed, 57 insertions(+), 8 deletions(-) diff --git a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt index 6db16b90873a..807fba1f829f 100644 --- a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt +++ b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt @@ -19,6 +19,7 @@ Required Properties: - "samsung,exynos5260-pinctrl": for Exynos5260 compatible pin-controller. - "samsung,exynos5410-pinctrl": for Exynos5410 compatible pin-controller. - "samsung,exynos5420-pinctrl": for Exynos5420 compatible pin-controller. + - "samsung,exynos5433-pinctrl": for Exynos5433 compatible pin-controller. - "samsung,exynos7-pinctrl": for Exynos7 compatible pin-controller. - reg: Base address of the pin controller hardware module and length of diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.c b/drivers/pinctrl/samsung/pinctrl-exynos.c index 051b5bf701a8..4f95983e0cdd 100644 --- a/drivers/pinctrl/samsung/pinctrl-exynos.c +++ b/drivers/pinctrl/samsung/pinctrl-exynos.c @@ -1350,6 +1350,11 @@ static const struct samsung_pin_bank_data exynos5433_pin_banks0[] = { EXYNOS_PIN_BANK_EINTW(8, 0x020, "gpa1", 0x04), EXYNOS_PIN_BANK_EINTW(8, 0x040, "gpa2", 0x08), EXYNOS_PIN_BANK_EINTW(8, 0x060, "gpa3", 0x0c), + EXYNOS_PIN_BANK_EINTW_EXT(8, 0x020, "gpf1", 0x1004), + EXYNOS_PIN_BANK_EINTW_EXT(4, 0x040, "gpf2", 0x1008), + EXYNOS_PIN_BANK_EINTW_EXT(4, 0x060, "gpf3", 0x100c), + EXYNOS_PIN_BANK_EINTW_EXT(8, 0x080, "gpf4", 0x1010), + EXYNOS_PIN_BANK_EINTW_EXT(8, 0x0a0, "gpf5", 0x1014), }; /* pin banks of exynos5433 pin-controller - AUD */ diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.h b/drivers/pinctrl/samsung/pinctrl-exynos.h index 0f0f7cedb2dc..4b737b6c434d 100644 --- a/drivers/pinctrl/samsung/pinctrl-exynos.h +++ b/drivers/pinctrl/samsung/pinctrl-exynos.h @@ -79,6 +79,17 @@ .name = id \ } +#define EXYNOS_PIN_BANK_EINTW_EXT(pins, reg, id, offs) \ + { \ + .type = &bank_type_off, \ + .pctl_offset = reg, \ + .nr_pins = pins, \ + .eint_type = EINT_TYPE_WKUP, \ + .eint_offset = offs, \ + .eint_ext = 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 513fe6b23248..57e22085c2db 100644 --- a/drivers/pinctrl/samsung/pinctrl-samsung.c +++ b/drivers/pinctrl/samsung/pinctrl-samsung.c @@ -338,6 +338,7 @@ static void pin_to_reg_bank(struct samsung_pinctrl_drv_data *drvdata, struct samsung_pin_bank **bank) { struct samsung_pin_bank *b; + void __iomem *virt_base = drvdata->virt_base; b = drvdata->pin_banks; @@ -345,7 +346,10 @@ static void pin_to_reg_bank(struct samsung_pinctrl_drv_data *drvdata, ((b->pin_base + b->nr_pins - 1) < pin)) b++; - *reg = drvdata->virt_base + b->pctl_offset; + if (b->eint_ext) + virt_base = drvdata->ext_base; + + *reg = virt_base + b->pctl_offset; *offset = pin - b->pin_base; if (bank) *bank = b; @@ -523,10 +527,12 @@ static void samsung_gpio_set_value(struct gpio_chip *gc, { struct samsung_pin_bank *bank = gpiochip_get_data(gc); const struct samsung_pin_bank_type *type = bank->type; + void __iomem *virt_base = bank->eint_ext ? + bank->drvdata->ext_base : bank->drvdata->virt_base; void __iomem *reg; u32 data; - reg = bank->drvdata->virt_base + bank->pctl_offset; + reg = virt_base + bank->pctl_offset; data = readl(reg + type->reg_offset[PINCFG_TYPE_DAT]); data &= ~(1 << offset); @@ -553,8 +559,10 @@ static int samsung_gpio_get(struct gpio_chip *gc, unsigned offset) u32 data; struct samsung_pin_bank *bank = gpiochip_get_data(gc); const struct samsung_pin_bank_type *type = bank->type; + void __iomem *virt_base = bank->eint_ext ? + bank->drvdata->ext_base : bank->drvdata->virt_base; - reg = bank->drvdata->virt_base + bank->pctl_offset; + reg = virt_base + bank->pctl_offset; data = readl(reg + type->reg_offset[PINCFG_TYPE_DAT]); data >>= offset; @@ -574,6 +582,7 @@ static int samsung_gpio_set_direction(struct gpio_chip *gc, const struct samsung_pin_bank_type *type; struct samsung_pin_bank *bank; struct samsung_pinctrl_drv_data *drvdata; + void __iomem *virt_base; void __iomem *reg; u32 data, mask, shift; @@ -581,7 +590,8 @@ static int samsung_gpio_set_direction(struct gpio_chip *gc, type = bank->type; drvdata = bank->drvdata; - reg = drvdata->virt_base + bank->pctl_offset + + virt_base = bank->eint_ext ? drvdata->ext_base : drvdata->virt_base; + reg = virt_base + bank->pctl_offset + type->reg_offset[PINCFG_TYPE_FUNC]; mask = (1 << type->fld_width[PINCFG_TYPE_FUNC]) - 1; @@ -1007,6 +1017,7 @@ 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->eint_ext = bdata->eint_ext; bank->name = bdata->name; spin_lock_init(&bank->slock); @@ -1065,6 +1076,14 @@ static int samsung_pinctrl_probe(struct platform_device *pdev) if (IS_ERR(drvdata->virt_base)) return PTR_ERR(drvdata->virt_base); + res = platform_get_resource(pdev, IORESOURCE_MEM, 1); + if (res) { + drvdata->ext_base = + devm_ioremap(dev, res->start, resource_size(res)); + if (!drvdata->ext_base) + return -ENXIO; + } + res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); if (res) drvdata->irq = res->start; @@ -1102,16 +1121,20 @@ static int samsung_pinctrl_probe(struct platform_device *pdev) static void samsung_pinctrl_suspend_dev( struct samsung_pinctrl_drv_data *drvdata) { - void __iomem *virt_base = drvdata->virt_base; + void __iomem *virt_base; int i; for (i = 0; i < drvdata->nr_banks; i++) { struct samsung_pin_bank *bank = &drvdata->pin_banks[i]; - void __iomem *reg = virt_base + bank->pctl_offset; + void __iomem *reg; const u8 *offs = bank->type->reg_offset; const u8 *widths = bank->type->fld_width; enum pincfg_type type; + virt_base = bank->eint_ext ? + drvdata->ext_base : drvdata->virt_base; + reg = virt_base + bank->pctl_offset; + /* Registers without a powerdown config aren't lost */ if (!widths[PINCFG_TYPE_CON_PDN]) continue; @@ -1148,7 +1171,7 @@ static void samsung_pinctrl_suspend_dev( */ static void samsung_pinctrl_resume_dev(struct samsung_pinctrl_drv_data *drvdata) { - void __iomem *virt_base = drvdata->virt_base; + void __iomem *virt_base; int i; if (drvdata->resume) @@ -1156,11 +1179,15 @@ static void samsung_pinctrl_resume_dev(struct samsung_pinctrl_drv_data *drvdata) for (i = 0; i < drvdata->nr_banks; i++) { struct samsung_pin_bank *bank = &drvdata->pin_banks[i]; - void __iomem *reg = virt_base + bank->pctl_offset; + void __iomem *reg; const u8 *offs = bank->type->reg_offset; const u8 *widths = bank->type->fld_width; enum pincfg_type type; + virt_base = bank->eint_ext ? + drvdata->ext_base : drvdata->virt_base; + reg = virt_base + bank->pctl_offset; + /* Registers without a powerdown config aren't lost */ if (!widths[PINCFG_TYPE_CON_PDN]) continue; diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.h b/drivers/pinctrl/samsung/pinctrl-samsung.h index cd31bfaf62cb..3005135f4565 100644 --- a/drivers/pinctrl/samsung/pinctrl-samsung.h +++ b/drivers/pinctrl/samsung/pinctrl-samsung.h @@ -131,6 +131,7 @@ struct samsung_pin_bank_data { enum eint_type eint_type; u32 eint_mask; u32 eint_offset; + bool eint_ext; const char *name; }; @@ -163,6 +164,7 @@ struct samsung_pin_bank { enum eint_type eint_type; u32 eint_mask; u32 eint_offset; + bool eint_ext; const char *name; u32 pin_base; @@ -201,6 +203,8 @@ struct samsung_pin_ctrl { * struct samsung_pinctrl_drv_data: wrapper for holding driver data together. * @node: global list node * @virt_base: register base address of the controller. + * @ext_base: external register base address of the controller. + * @ext_base: external register base address of the controller. * @dev: device instance representing the controller. * @irq: interrpt number used by the controller to notify gpio interrupts. * @ctrl: pin controller instance managed by the driver. @@ -216,6 +220,7 @@ struct samsung_pin_ctrl { struct samsung_pinctrl_drv_data { struct list_head node; void __iomem *virt_base; + void __iomem *ext_base; struct device *dev; int irq; -- 1.9.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 4/7] pinctrl: samsung: Add GPFx support of Exynos5433 2016-08-16 6:27 ` [PATCH 4/7] pinctrl: samsung: Add GPFx support of Exynos5433 Chanwoo Choi @ 2016-08-16 6:42 ` Tomasz Figa [not found] ` <1471328843-26653-5-git-send-email-cw00.choi-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> ` (2 subsequent siblings) 3 siblings, 0 replies; 7+ messages in thread From: Tomasz Figa @ 2016-08-16 6:42 UTC (permalink / raw) To: Chanwoo Choi Cc: Mark Rutland, Krzysztof Kozłowski, chanwoo, Catalin Marinas, Linus Walleij, Will Deacon, Andrzej Hajda, Sylwester Nawrocki, Beomho Seo, Marek Szyprowski, Jonghwa Lee, linux-samsung-soc@vger.kernel.org, Joonyoung Shim, Krzysztof Kozlowski, Jaehoon Chung, Kukjin Kim, devicetree, InKi Dae, linux-gpio@vger.kernel.org, Rob Herring, ingi2.kim Hi, 2016-08-16 15:27 GMT+09:00 Chanwoo Choi <cw00.choi@samsung.com>: > From: Joonyoung Shim <jy0922.shim@samsung.com> > > This patch add the support of GPFx pin of Exynos5433 SoC. Exynos5433 has > different memory map of GPFx from previous Exynos SoC. Exynos GPIO has > following register to control gpio funciton. Usually, all registers of GPIO > are included in same domain. > - CON / DAT / PUD / DRV / CONPDN / PUDPDN > - EINT_CON/ EINT_FLTCON0, EINT_FLTCON1 / EINT_MASK / EINT_PEND > > But, GPFx are included in two domain as following. So, this patch supports > the GPFx pin which handle the on separate two domains. > - ALIVE domain : CON / DAT / PUD / DRV / CONPDN / PUDPDN > - IMEM domain : EINT_CON/ EINT_FLTCON0, EINT_FLTCON1 / EINT_MASK / EINT_PEND I'm afraid I don't get anything from the description above. Could you describe the layout of registers in memory map and IRQ routing of the pins? Best regards, Tomasz ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <1471328843-26653-5-git-send-email-cw00.choi-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>]
* Re: [PATCH 4/7] pinctrl: samsung: Add GPFx support of Exynos5433 [not found] ` <1471328843-26653-5-git-send-email-cw00.choi-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> @ 2016-08-16 8:46 ` Krzysztof Kozlowski 0 siblings, 0 replies; 7+ messages in thread From: Krzysztof Kozlowski @ 2016-08-16 8:46 UTC (permalink / raw) To: Chanwoo Choi, kgene-DgEjT+Ai2ygdnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA Cc: krzk-DgEjT+Ai2ygdnm+yROfE0A, jh80.chung-Sze3O3UU22JBDgjK7y7TUQ, sw0312.kim-Sze3O3UU22JBDgjK7y7TUQ, jy0922.shim-Sze3O3UU22JBDgjK7y7TUQ, inki.dae-Sze3O3UU22JBDgjK7y7TUQ, jonghwa3.lee-Sze3O3UU22JBDgjK7y7TUQ, beomho.seo-Sze3O3UU22JBDgjK7y7TUQ, jaewon02.kim-Sze3O3UU22JBDgjK7y7TUQ, human.hwang-Sze3O3UU22JBDgjK7y7TUQ, ideal.song-Sze3O3UU22JBDgjK7y7TUQ, ingi2.kim-Sze3O3UU22JBDgjK7y7TUQ, m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ, a.hajda-Sze3O3UU22JBDgjK7y7TUQ, s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ, chanwoo-DgEjT+Ai2ygdnm+yROfE0A, Linus Walleij, Tomasz Figa, linux-gpio-u79uwXL29TY76Z2rM5mHXA On 08/16/2016 08:27 AM, Chanwoo Choi wrote: > From: Joonyoung Shim <jy0922.shim-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> > > This patch add the support of GPFx pin of Exynos5433 SoC. Exynos5433 has > different memory map of GPFx from previous Exynos SoC. Exynos GPIO has > following register to control gpio funciton. Usually, all registers of GPIO > are included in same domain. > - CON / DAT / PUD / DRV / CONPDN / PUDPDN > - EINT_CON/ EINT_FLTCON0, EINT_FLTCON1 / EINT_MASK / EINT_PEND > > But, GPFx are included in two domain as following. So, this patch supports > the GPFx pin which handle the on separate two domains. > - ALIVE domain : CON / DAT / PUD / DRV / CONPDN / PUDPDN > - IMEM domain : EINT_CON/ EINT_FLTCON0, EINT_FLTCON1 / EINT_MASK / EINT_PEND > > Cc: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> > Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> > Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> > Cc: Tomasz Figa <tomasz.figa-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > Cc: Krzysztof Kozlowski <k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> > Cc: Sylwester Nawrocki <s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> > Cc: Kukjin Kim <kgene-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> > Cc: linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > Signed-off-by: Joonyoung Shim <jy0922.shim-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> > Signed-off-by: Chanwoo Choi <cw00.choi-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> > --- > .../bindings/pinctrl/samsung-pinctrl.txt | 1 + > drivers/pinctrl/samsung/pinctrl-exynos.c | 5 +++ > drivers/pinctrl/samsung/pinctrl-exynos.h | 11 ++++++ > drivers/pinctrl/samsung/pinctrl-samsung.c | 43 ++++++++++++++++++---- > drivers/pinctrl/samsung/pinctrl-samsung.h | 5 +++ > 5 files changed, 57 insertions(+), 8 deletions(-) > > diff --git a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt > index 6db16b90873a..807fba1f829f 100644 > --- a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt > +++ b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt > @@ -19,6 +19,7 @@ Required Properties: > - "samsung,exynos5260-pinctrl": for Exynos5260 compatible pin-controller. > - "samsung,exynos5410-pinctrl": for Exynos5410 compatible pin-controller. > - "samsung,exynos5420-pinctrl": for Exynos5420 compatible pin-controller. > + - "samsung,exynos5433-pinctrl": for Exynos5433 compatible pin-controller. > - "samsung,exynos7-pinctrl": for Exynos7 compatible pin-controller. > > - reg: Base address of the pin controller hardware module and length of > diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.c b/drivers/pinctrl/samsung/pinctrl-exynos.c > index 051b5bf701a8..4f95983e0cdd 100644 > --- a/drivers/pinctrl/samsung/pinctrl-exynos.c > +++ b/drivers/pinctrl/samsung/pinctrl-exynos.c > @@ -1350,6 +1350,11 @@ static const struct samsung_pin_bank_data exynos5433_pin_banks0[] = { > EXYNOS_PIN_BANK_EINTW(8, 0x020, "gpa1", 0x04), > EXYNOS_PIN_BANK_EINTW(8, 0x040, "gpa2", 0x08), > EXYNOS_PIN_BANK_EINTW(8, 0x060, "gpa3", 0x0c), > + EXYNOS_PIN_BANK_EINTW_EXT(8, 0x020, "gpf1", 0x1004), > + EXYNOS_PIN_BANK_EINTW_EXT(4, 0x040, "gpf2", 0x1008), > + EXYNOS_PIN_BANK_EINTW_EXT(4, 0x060, "gpf3", 0x100c), > + EXYNOS_PIN_BANK_EINTW_EXT(8, 0x080, "gpf4", 0x1010), > + EXYNOS_PIN_BANK_EINTW_EXT(8, 0x0a0, "gpf5", 0x1014), > }; > > /* pin banks of exynos5433 pin-controller - AUD */ > diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.h b/drivers/pinctrl/samsung/pinctrl-exynos.h > index 0f0f7cedb2dc..4b737b6c434d 100644 > --- a/drivers/pinctrl/samsung/pinctrl-exynos.h > +++ b/drivers/pinctrl/samsung/pinctrl-exynos.h > @@ -79,6 +79,17 @@ > .name = id \ > } > > +#define EXYNOS_PIN_BANK_EINTW_EXT(pins, reg, id, offs) \ > + { \ > + .type = &bank_type_off, \ > + .pctl_offset = reg, \ > + .nr_pins = pins, \ > + .eint_type = EINT_TYPE_WKUP, \ > + .eint_offset = offs, \ > + .eint_ext = 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 513fe6b23248..57e22085c2db 100644 > --- a/drivers/pinctrl/samsung/pinctrl-samsung.c > +++ b/drivers/pinctrl/samsung/pinctrl-samsung.c > @@ -338,6 +338,7 @@ static void pin_to_reg_bank(struct samsung_pinctrl_drv_data *drvdata, > struct samsung_pin_bank **bank) > { > struct samsung_pin_bank *b; > + void __iomem *virt_base = drvdata->virt_base; > > b = drvdata->pin_banks; > > @@ -345,7 +346,10 @@ static void pin_to_reg_bank(struct samsung_pinctrl_drv_data *drvdata, > ((b->pin_base + b->nr_pins - 1) < pin)) > b++; > > - *reg = drvdata->virt_base + b->pctl_offset; > + if (b->eint_ext) > + virt_base = drvdata->ext_base; > + > + *reg = virt_base + b->pctl_offset; > *offset = pin - b->pin_base; > if (bank) > *bank = b; > @@ -523,10 +527,12 @@ static void samsung_gpio_set_value(struct gpio_chip *gc, > { > struct samsung_pin_bank *bank = gpiochip_get_data(gc); > const struct samsung_pin_bank_type *type = bank->type; > + void __iomem *virt_base = bank->eint_ext ? > + bank->drvdata->ext_base : bank->drvdata->virt_base; I would prefer to avoid '?' operator because it makes the line difficult to read. However I wonder whether this check here and later is needed at all. It obfuscates the code so how about: 1. In pinctrl-samsung always access virt_base as it was before. The virt_base in probe will be in domain of GPF_CON registers. 2. Rename ext_base to eint_base 3. Always assign eint_base in probe(): if (exynos5433) eint_base = iomap()... else eint_base = virt_base; 4. in pinctrl-exynos.c access the eint_base. No need of scattered if() through various calls and simpler flow. > void __iomem *reg; > u32 data; > > - reg = bank->drvdata->virt_base + bank->pctl_offset; > + reg = virt_base + bank->pctl_offset; > > data = readl(reg + type->reg_offset[PINCFG_TYPE_DAT]); > data &= ~(1 << offset); > @@ -553,8 +559,10 @@ static int samsung_gpio_get(struct gpio_chip *gc, unsigned offset) > u32 data; > struct samsung_pin_bank *bank = gpiochip_get_data(gc); > const struct samsung_pin_bank_type *type = bank->type; > + void __iomem *virt_base = bank->eint_ext ? > + bank->drvdata->ext_base : bank->drvdata->virt_base; > > - reg = bank->drvdata->virt_base + bank->pctl_offset; > + reg = virt_base + bank->pctl_offset; > > data = readl(reg + type->reg_offset[PINCFG_TYPE_DAT]); > data >>= offset; > @@ -574,6 +582,7 @@ static int samsung_gpio_set_direction(struct gpio_chip *gc, > const struct samsung_pin_bank_type *type; > struct samsung_pin_bank *bank; > struct samsung_pinctrl_drv_data *drvdata; > + void __iomem *virt_base; > void __iomem *reg; > u32 data, mask, shift; > > @@ -581,7 +590,8 @@ static int samsung_gpio_set_direction(struct gpio_chip *gc, > type = bank->type; > drvdata = bank->drvdata; > > - reg = drvdata->virt_base + bank->pctl_offset + > + virt_base = bank->eint_ext ? drvdata->ext_base : drvdata->virt_base; > + reg = virt_base + bank->pctl_offset + > type->reg_offset[PINCFG_TYPE_FUNC]; > > mask = (1 << type->fld_width[PINCFG_TYPE_FUNC]) - 1; > @@ -1007,6 +1017,7 @@ 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->eint_ext = bdata->eint_ext; > bank->name = bdata->name; > > spin_lock_init(&bank->slock); > @@ -1065,6 +1076,14 @@ static int samsung_pinctrl_probe(struct platform_device *pdev) > if (IS_ERR(drvdata->virt_base)) > return PTR_ERR(drvdata->virt_base); > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 1); Additional reg should be documented in bindings documentation. Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 4/7] pinctrl: samsung: Add GPFx support of Exynos5433 2016-08-16 6:27 ` [PATCH 4/7] pinctrl: samsung: Add GPFx support of Exynos5433 Chanwoo Choi 2016-08-16 6:42 ` Tomasz Figa [not found] ` <1471328843-26653-5-git-send-email-cw00.choi-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> @ 2016-08-18 19:00 ` Rob Herring 2016-08-19 9:07 ` Chanwoo Choi 3 siblings, 0 replies; 7+ messages in thread From: Rob Herring @ 2016-08-18 19:00 UTC (permalink / raw) To: Chanwoo Choi Cc: mark.rutland, k.kozlowski, chanwoo, catalin.marinas, Linus Walleij, will.deacon, Tomasz Figa, a.hajda, s.nawrocki, beomho.seo, m.szyprowski, jonghwa3.lee, linux-samsung-soc, jy0922.shim, krzk, jh80.chung, kgene, devicetree, inki.dae, linux-gpio, ingi2.kim, linux-arm-kernel, jaewon02.kim, sw0312.kim, linux-kernel, human.hwang, ideal.song On Tue, Aug 16, 2016 at 03:27:20PM +0900, Chanwoo Choi wrote: > From: Joonyoung Shim <jy0922.shim@samsung.com> > > This patch add the support of GPFx pin of Exynos5433 SoC. Exynos5433 has > different memory map of GPFx from previous Exynos SoC. Exynos GPIO has > following register to control gpio funciton. Usually, all registers of GPIO > are included in same domain. > - CON / DAT / PUD / DRV / CONPDN / PUDPDN > - EINT_CON/ EINT_FLTCON0, EINT_FLTCON1 / EINT_MASK / EINT_PEND > > But, GPFx are included in two domain as following. So, this patch supports > the GPFx pin which handle the on separate two domains. > - ALIVE domain : CON / DAT / PUD / DRV / CONPDN / PUDPDN > - IMEM domain : EINT_CON/ EINT_FLTCON0, EINT_FLTCON1 / EINT_MASK / EINT_PEND > > Cc: Linus Walleij <linus.walleij@linaro.org> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: Tomasz Figa <tomasz.figa@gmail.com> > Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com> > Cc: Sylwester Nawrocki <s.nawrocki@samsung.com> > Cc: Kukjin Kim <kgene@kernel.org> > Cc: linux-gpio@vger.kernel.org > Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com> > Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com> > --- > .../bindings/pinctrl/samsung-pinctrl.txt | 1 + Acked-by: Rob Herring <robh@kernel.org> > drivers/pinctrl/samsung/pinctrl-exynos.c | 5 +++ > drivers/pinctrl/samsung/pinctrl-exynos.h | 11 ++++++ > drivers/pinctrl/samsung/pinctrl-samsung.c | 43 ++++++++++++++++++---- > drivers/pinctrl/samsung/pinctrl-samsung.h | 5 +++ > 5 files changed, 57 insertions(+), 8 deletions(-) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 4/7] pinctrl: samsung: Add GPFx support of Exynos5433 2016-08-16 6:27 ` [PATCH 4/7] pinctrl: samsung: Add GPFx support of Exynos5433 Chanwoo Choi ` (2 preceding siblings ...) 2016-08-18 19:00 ` Rob Herring @ 2016-08-19 9:07 ` Chanwoo Choi 2016-08-19 11:31 ` Tomasz Figa 3 siblings, 1 reply; 7+ messages in thread From: Chanwoo Choi @ 2016-08-19 9:07 UTC (permalink / raw) To: Tomasz Figa Cc: mark.rutland, k.kozlowski, chanwoo, catalin.marinas, Linus Walleij, will.deacon, a.hajda, s.nawrocki, beomho.seo, m.szyprowski, jonghwa3.lee, linux-samsung-soc, jy0922.shim, krzk, jh80.chung, kgene, devicetree, inki.dae, linux-gpio, robh+dt, ingi2.kim, linux-arm-kernel, jaewon02.kim, sw0312.kim, linux-kernel, human.hwang, ideal.song Hi Tomasz Figa, Due to wrong setting of email client, your reply is deleted on my email client at the company. I'm so sorry. So, I add your comment on below and then I reply the detailed description. On 2016년 08월 16일 15:27, Chanwoo Choi wrote: > From: Joonyoung Shim <jy0922.shim@samsung.com> > > This patch add the support of GPFx pin of Exynos5433 SoC. Exynos5433 has > different memory map of GPFx from previous Exynos SoC. Exynos GPIO has > following register to control gpio funciton. Usually, all registers of GPIO > are included in same domain. > - CON / DAT / PUD / DRV / CONPDN / PUDPDN > - EINT_CON/ EINT_FLTCON0, EINT_FLTCON1 / EINT_MASK / EINT_PEND > > But, GPFx are included in two domain as following. So, this patch supports > the GPFx pin which handle the on separate two domains. > - ALIVE domain : CON / DAT / PUD / DRV / CONPDN / PUDPDN > - IMEM domain : EINT_CON/ EINT_FLTCON0, EINT_FLTCON1 / EINT_MASK / EINT_PEND --------- I'm afraid I don't get anything from the description above. Could you describe the layout of registers in memory map and IRQ routing of the pins? Best regards, Tomasz ---------- On this patch, I'm sorry that I described the wrong information about GFP1~5. I explained the memory map of GPF[1-5] the oppositely. Following compositions are correct. - ALIVE : WEINT_FLTCONx, WEINT_MASK, WEING_PEND - IMEM : CON, DAT, PUD, DRV, CONPDN, PUDPDN And, I add the memory map for GPF[1~5][2] and the wakeup interrupt information[1]. [1] Memory map for GPF1~5 [ALIVE] WEINT_GPA0_CON : 0x1058_0000 (ALIVE) + (0x0700 = 0x0700 + 0x0) WEINT_GPA1_CON : 0x1058_0000 (ALIVE) + (0x0704 = 0x0700 + 0x4) WEINT_GPA2_CON : 0x1058_0000 (ALIVE) + (0x0708 = 0x0700 + 0x8) WEINT_GPA3_CON : 0x1058_0000 (ALIVE) + (0x070C = 0x0700 + 0xC) WEINT_GPF1_CON : 0x1058_0000 (ALIVE) + (0x1704 = 0x0700 + 0x1004) WEINT_GPF2_CON : 0x1058_0000 (ALIVE) + (0x1708 = 0x0700 + 0x1008) WEINT_GPF3_CON : 0x1058_0000 (ALIVE) + (0x170C = 0x0700 + 0x100C) WEINT_GPF4_CON : 0x1058_0000 (ALIVE) + (0x1710 = 0x0700 + 0x10010) WEINT_GPF5_CON : 0x1058_0000 (ALIVE) + (0x1714 = 0x0700 + 0x1014) WEINT_GPF[x]_MASK : 0x1058_0000 (ALIVE) + (0x1900 + (x * 4)) WEINT_GPF[x]_PEND : 0x1058_0000 (ALIVE) + (0x1A00 + (x * 4)) (x : 1 ~ 5) [IMEM] GPF1_CON : 0X1109_0000 (IMEM) + 0x0020 GPF1_DAT : 0X1109_0000 (IMEM) + 0x0024 GPF1_PUD : 0X1109_0000 (IMEM) + 0x0028 GPF1_DRV : 0X1109_0000 (IMEM) + 0x002C GPF1_CONPDN : 0X1109_0000 (IMEM) + 0x0030 GPF1_PUDPDN : 0X1109_0000 (IMEM) + 0x0034 GPF2_CON : 0X1109_0000 (IMEM) + 0x0040 ... GPF3_CON : 0X1109_0000 (IMEM) + 0x0060 ... GPF4_CON : 0X1109_0000 (IMEM) + 0x0080 ... GPF5_CON : 0X1109_0000 (IMEM) + 0x00A0 [2] Interrput pin information - the total number of wakeup external IRQ is 64. ---------------------------------------------------------------------------------- domain| gpio : nr | wakeup interrput name | SPI number | ----------------------------------------------------------------------------------- ALIVE | GPA0 : 8 | INTREQ__EINT[0~7] | SPI[0] ~ SPI[7] | ALIVE | GPA1 : 8 | INTREQ__EINT[8~15] | SPI[8] ~ SPI[15] | ALIVE | GPA2 : 8 | INTREQ__EINT_16_63 | SPI[16] | ALIVE | GPA3 : 8 | INTREQ__EINT_16_63 | SPI[16] | ----------------------------------------------------------------------------------- ALIVE | GPF1 : 8 | INTREQ__EINT_16_63 | SPI[16] | ALIVE | GPF2 : 4 | INTREQ__EINT_16_63 | SPI[16] | ALIVE | GPF3 : 4 | INTREQ__EINT_16_63 | SPI[16] | ALIVE | GPF4 : 8 | INTREQ__EINT_16_63 | SPI[16] | ALIVE | GPF5 : 8 | INTREQ__EINT_16_63 | SPI[16] | ----------------------------------------------------------------------------------- In summary, If gpf[1-5] handle the CON/DAT/PUD/DRV register, the driver will use the drvdata->ext_base (IMEM base address) instead of drvdata->virt_base(ALIVE base address) because the CON/DAT/PUD/DRV register of gpf[1-5] are included in the IMEM domain. If gpf[1-5] handle the WEINT_* register, the driver will use the drvdata->virt_base(ALIVE base address) because the WEINT_* registers of gpf[1-5] are included in the ALIVE domain. Best Regards, Chanwoo Choi > > Cc: Linus Walleij <linus.walleij@linaro.org> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: Tomasz Figa <tomasz.figa@gmail.com> > Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com> > Cc: Sylwester Nawrocki <s.nawrocki@samsung.com> > Cc: Kukjin Kim <kgene@kernel.org> > Cc: linux-gpio@vger.kernel.org > Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com> > Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com> > --- > .../bindings/pinctrl/samsung-pinctrl.txt | 1 + > drivers/pinctrl/samsung/pinctrl-exynos.c | 5 +++ > drivers/pinctrl/samsung/pinctrl-exynos.h | 11 ++++++ > drivers/pinctrl/samsung/pinctrl-samsung.c | 43 ++++++++++++++++++---- > drivers/pinctrl/samsung/pinctrl-samsung.h | 5 +++ > 5 files changed, 57 insertions(+), 8 deletions(-) > > diff --git a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt > index 6db16b90873a..807fba1f829f 100644 > --- a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt > +++ b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt > @@ -19,6 +19,7 @@ Required Properties: > - "samsung,exynos5260-pinctrl": for Exynos5260 compatible pin-controller. > - "samsung,exynos5410-pinctrl": for Exynos5410 compatible pin-controller. > - "samsung,exynos5420-pinctrl": for Exynos5420 compatible pin-controller. > + - "samsung,exynos5433-pinctrl": for Exynos5433 compatible pin-controller. > - "samsung,exynos7-pinctrl": for Exynos7 compatible pin-controller. > > - reg: Base address of the pin controller hardware module and length of > diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.c b/drivers/pinctrl/samsung/pinctrl-exynos.c > index 051b5bf701a8..4f95983e0cdd 100644 > --- a/drivers/pinctrl/samsung/pinctrl-exynos.c > +++ b/drivers/pinctrl/samsung/pinctrl-exynos.c > @@ -1350,6 +1350,11 @@ static const struct samsung_pin_bank_data exynos5433_pin_banks0[] = { > EXYNOS_PIN_BANK_EINTW(8, 0x020, "gpa1", 0x04), > EXYNOS_PIN_BANK_EINTW(8, 0x040, "gpa2", 0x08), > EXYNOS_PIN_BANK_EINTW(8, 0x060, "gpa3", 0x0c), > + EXYNOS_PIN_BANK_EINTW_EXT(8, 0x020, "gpf1", 0x1004), > + EXYNOS_PIN_BANK_EINTW_EXT(4, 0x040, "gpf2", 0x1008), > + EXYNOS_PIN_BANK_EINTW_EXT(4, 0x060, "gpf3", 0x100c), > + EXYNOS_PIN_BANK_EINTW_EXT(8, 0x080, "gpf4", 0x1010), > + EXYNOS_PIN_BANK_EINTW_EXT(8, 0x0a0, "gpf5", 0x1014), > }; > > /* pin banks of exynos5433 pin-controller - AUD */ > diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.h b/drivers/pinctrl/samsung/pinctrl-exynos.h > index 0f0f7cedb2dc..4b737b6c434d 100644 > --- a/drivers/pinctrl/samsung/pinctrl-exynos.h > +++ b/drivers/pinctrl/samsung/pinctrl-exynos.h > @@ -79,6 +79,17 @@ > .name = id \ > } > > +#define EXYNOS_PIN_BANK_EINTW_EXT(pins, reg, id, offs) \ > + { \ > + .type = &bank_type_off, \ > + .pctl_offset = reg, \ > + .nr_pins = pins, \ > + .eint_type = EINT_TYPE_WKUP, \ > + .eint_offset = offs, \ > + .eint_ext = 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 513fe6b23248..57e22085c2db 100644 > --- a/drivers/pinctrl/samsung/pinctrl-samsung.c > +++ b/drivers/pinctrl/samsung/pinctrl-samsung.c > @@ -338,6 +338,7 @@ static void pin_to_reg_bank(struct samsung_pinctrl_drv_data *drvdata, > struct samsung_pin_bank **bank) > { > struct samsung_pin_bank *b; > + void __iomem *virt_base = drvdata->virt_base; > > b = drvdata->pin_banks; > > @@ -345,7 +346,10 @@ static void pin_to_reg_bank(struct samsung_pinctrl_drv_data *drvdata, > ((b->pin_base + b->nr_pins - 1) < pin)) > b++; > > - *reg = drvdata->virt_base + b->pctl_offset; > + if (b->eint_ext) > + virt_base = drvdata->ext_base; > + > + *reg = virt_base + b->pctl_offset; > *offset = pin - b->pin_base; > if (bank) > *bank = b; > @@ -523,10 +527,12 @@ static void samsung_gpio_set_value(struct gpio_chip *gc, > { > struct samsung_pin_bank *bank = gpiochip_get_data(gc); > const struct samsung_pin_bank_type *type = bank->type; > + void __iomem *virt_base = bank->eint_ext ? > + bank->drvdata->ext_base : bank->drvdata->virt_base; > void __iomem *reg; > u32 data; > > - reg = bank->drvdata->virt_base + bank->pctl_offset; > + reg = virt_base + bank->pctl_offset; > > data = readl(reg + type->reg_offset[PINCFG_TYPE_DAT]); > data &= ~(1 << offset); > @@ -553,8 +559,10 @@ static int samsung_gpio_get(struct gpio_chip *gc, unsigned offset) > u32 data; > struct samsung_pin_bank *bank = gpiochip_get_data(gc); > const struct samsung_pin_bank_type *type = bank->type; > + void __iomem *virt_base = bank->eint_ext ? > + bank->drvdata->ext_base : bank->drvdata->virt_base; > > - reg = bank->drvdata->virt_base + bank->pctl_offset; > + reg = virt_base + bank->pctl_offset; > > data = readl(reg + type->reg_offset[PINCFG_TYPE_DAT]); > data >>= offset; > @@ -574,6 +582,7 @@ static int samsung_gpio_set_direction(struct gpio_chip *gc, > const struct samsung_pin_bank_type *type; > struct samsung_pin_bank *bank; > struct samsung_pinctrl_drv_data *drvdata; > + void __iomem *virt_base; > void __iomem *reg; > u32 data, mask, shift; > > @@ -581,7 +590,8 @@ static int samsung_gpio_set_direction(struct gpio_chip *gc, > type = bank->type; > drvdata = bank->drvdata; > > - reg = drvdata->virt_base + bank->pctl_offset + > + virt_base = bank->eint_ext ? drvdata->ext_base : drvdata->virt_base; > + reg = virt_base + bank->pctl_offset + > type->reg_offset[PINCFG_TYPE_FUNC]; > > mask = (1 << type->fld_width[PINCFG_TYPE_FUNC]) - 1; > @@ -1007,6 +1017,7 @@ 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->eint_ext = bdata->eint_ext; > bank->name = bdata->name; > > spin_lock_init(&bank->slock); > @@ -1065,6 +1076,14 @@ static int samsung_pinctrl_probe(struct platform_device *pdev) > if (IS_ERR(drvdata->virt_base)) > return PTR_ERR(drvdata->virt_base); > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 1); > + if (res) { > + drvdata->ext_base = > + devm_ioremap(dev, res->start, resource_size(res)); > + if (!drvdata->ext_base) > + return -ENXIO; > + } > + > res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); > if (res) > drvdata->irq = res->start; > @@ -1102,16 +1121,20 @@ static int samsung_pinctrl_probe(struct platform_device *pdev) > static void samsung_pinctrl_suspend_dev( > struct samsung_pinctrl_drv_data *drvdata) > { > - void __iomem *virt_base = drvdata->virt_base; > + void __iomem *virt_base; > int i; > > for (i = 0; i < drvdata->nr_banks; i++) { > struct samsung_pin_bank *bank = &drvdata->pin_banks[i]; > - void __iomem *reg = virt_base + bank->pctl_offset; > + void __iomem *reg; > const u8 *offs = bank->type->reg_offset; > const u8 *widths = bank->type->fld_width; > enum pincfg_type type; > > + virt_base = bank->eint_ext ? > + drvdata->ext_base : drvdata->virt_base; > + reg = virt_base + bank->pctl_offset; > + > /* Registers without a powerdown config aren't lost */ > if (!widths[PINCFG_TYPE_CON_PDN]) > continue; > @@ -1148,7 +1171,7 @@ static void samsung_pinctrl_suspend_dev( > */ > static void samsung_pinctrl_resume_dev(struct samsung_pinctrl_drv_data *drvdata) > { > - void __iomem *virt_base = drvdata->virt_base; > + void __iomem *virt_base; > int i; > > if (drvdata->resume) > @@ -1156,11 +1179,15 @@ static void samsung_pinctrl_resume_dev(struct samsung_pinctrl_drv_data *drvdata) > > for (i = 0; i < drvdata->nr_banks; i++) { > struct samsung_pin_bank *bank = &drvdata->pin_banks[i]; > - void __iomem *reg = virt_base + bank->pctl_offset; > + void __iomem *reg; > const u8 *offs = bank->type->reg_offset; > const u8 *widths = bank->type->fld_width; > enum pincfg_type type; > > + virt_base = bank->eint_ext ? > + drvdata->ext_base : drvdata->virt_base; > + reg = virt_base + bank->pctl_offset; > + > /* Registers without a powerdown config aren't lost */ > if (!widths[PINCFG_TYPE_CON_PDN]) > continue; > diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.h b/drivers/pinctrl/samsung/pinctrl-samsung.h > index cd31bfaf62cb..3005135f4565 100644 > --- a/drivers/pinctrl/samsung/pinctrl-samsung.h > +++ b/drivers/pinctrl/samsung/pinctrl-samsung.h > @@ -131,6 +131,7 @@ struct samsung_pin_bank_data { > enum eint_type eint_type; > u32 eint_mask; > u32 eint_offset; > + bool eint_ext; > const char *name; > }; > > @@ -163,6 +164,7 @@ struct samsung_pin_bank { > enum eint_type eint_type; > u32 eint_mask; > u32 eint_offset; > + bool eint_ext; > const char *name; > > u32 pin_base; > @@ -201,6 +203,8 @@ struct samsung_pin_ctrl { > * struct samsung_pinctrl_drv_data: wrapper for holding driver data together. > * @node: global list node > * @virt_base: register base address of the controller. > + * @ext_base: external register base address of the controller. > + * @ext_base: external register base address of the controller. > * @dev: device instance representing the controller. > * @irq: interrpt number used by the controller to notify gpio interrupts. > * @ctrl: pin controller instance managed by the driver. > @@ -216,6 +220,7 @@ struct samsung_pin_ctrl { > struct samsung_pinctrl_drv_data { > struct list_head node; > void __iomem *virt_base; > + void __iomem *ext_base; > struct device *dev; > int irq; > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 4/7] pinctrl: samsung: Add GPFx support of Exynos5433 2016-08-19 9:07 ` Chanwoo Choi @ 2016-08-19 11:31 ` Tomasz Figa [not found] ` <CA+Ln22EaUmdrfLXJfYPAtCQ1DY4Y5iLWr-tqG67-P9hSXKMC6w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 7+ messages in thread From: Tomasz Figa @ 2016-08-19 11:31 UTC (permalink / raw) To: Chanwoo Choi Cc: Krzysztof Kozłowski, Kukjin Kim, Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon, devicetree, linux-arm-kernel, linux-samsung-soc@vger.kernel.org, linux-kernel, Krzysztof Kozlowski, Jaehoon Chung, sw0312.kim, Joonyoung Shim, InKi Dae, Jonghwa Lee, Beomho Seo, jaewon02.kim Hi Chanwoo, 2016-08-19 18:07 GMT+09:00 Chanwoo Choi <cw00.choi@samsung.com>: > Hi Tomasz Figa, > > Due to wrong setting of email client, > your reply is deleted on my email client at the company. I used Gmail (in plain text mode) to reply, was that related? > I'm so sorry. So, I add your comment on below and then > I reply the detailed description. No problem. Thanks for description. > > On 2016년 08월 16일 15:27, Chanwoo Choi wrote: >> From: Joonyoung Shim <jy0922.shim@samsung.com> >> >> This patch add the support of GPFx pin of Exynos5433 SoC. Exynos5433 has >> different memory map of GPFx from previous Exynos SoC. Exynos GPIO has >> following register to control gpio funciton. Usually, all registers of GPIO >> are included in same domain. >> - CON / DAT / PUD / DRV / CONPDN / PUDPDN >> - EINT_CON/ EINT_FLTCON0, EINT_FLTCON1 / EINT_MASK / EINT_PEND >> >> But, GPFx are included in two domain as following. So, this patch supports >> the GPFx pin which handle the on separate two domains. >> - ALIVE domain : CON / DAT / PUD / DRV / CONPDN / PUDPDN >> - IMEM domain : EINT_CON/ EINT_FLTCON0, EINT_FLTCON1 / EINT_MASK / EINT_PEND > > --------- > I'm afraid I don't get anything from the description above. Could you > describe the layout of registers in memory map and IRQ routing of the > pins? > > Best regards, > Tomasz > ---------- > > On this patch, I'm sorry that I described the wrong information about GFP1~5. > I explained the memory map of GPF[1-5] the oppositely. Following compositions > are correct. > - ALIVE : WEINT_FLTCONx, WEINT_MASK, WEING_PEND > - IMEM : CON, DAT, PUD, DRV, CONPDN, PUDPDN > And, I add the memory map for GPF[1~5][2] and the wakeup interrupt information[1]. > > [1] Memory map for GPF1~5 > [ALIVE] > WEINT_GPA0_CON : 0x1058_0000 (ALIVE) + (0x0700 = 0x0700 + 0x0) > WEINT_GPA1_CON : 0x1058_0000 (ALIVE) + (0x0704 = 0x0700 + 0x4) > WEINT_GPA2_CON : 0x1058_0000 (ALIVE) + (0x0708 = 0x0700 + 0x8) > WEINT_GPA3_CON : 0x1058_0000 (ALIVE) + (0x070C = 0x0700 + 0xC) > > WEINT_GPF1_CON : 0x1058_0000 (ALIVE) + (0x1704 = 0x0700 + 0x1004) > WEINT_GPF2_CON : 0x1058_0000 (ALIVE) + (0x1708 = 0x0700 + 0x1008) > WEINT_GPF3_CON : 0x1058_0000 (ALIVE) + (0x170C = 0x0700 + 0x100C) > WEINT_GPF4_CON : 0x1058_0000 (ALIVE) + (0x1710 = 0x0700 + 0x10010) > WEINT_GPF5_CON : 0x1058_0000 (ALIVE) + (0x1714 = 0x0700 + 0x1014) > > WEINT_GPF[x]_MASK : 0x1058_0000 (ALIVE) + (0x1900 + (x * 4)) > WEINT_GPF[x]_PEND : 0x1058_0000 (ALIVE) + (0x1A00 + (x * 4)) > (x : 1 ~ 5) > > [IMEM] > GPF1_CON : 0X1109_0000 (IMEM) + 0x0020 > GPF1_DAT : 0X1109_0000 (IMEM) + 0x0024 > GPF1_PUD : 0X1109_0000 (IMEM) + 0x0028 > GPF1_DRV : 0X1109_0000 (IMEM) + 0x002C > GPF1_CONPDN : 0X1109_0000 (IMEM) + 0x0030 > GPF1_PUDPDN : 0X1109_0000 (IMEM) + 0x0034 > > GPF2_CON : 0X1109_0000 (IMEM) + 0x0040 > ... > GPF3_CON : 0X1109_0000 (IMEM) + 0x0060 > ... > GPF4_CON : 0X1109_0000 (IMEM) + 0x0080 > ... > GPF5_CON : 0X1109_0000 (IMEM) + 0x00A0 > > [2] Interrput pin information > - the total number of wakeup external IRQ is 64. > ---------------------------------------------------------------------------------- > domain| gpio : nr | wakeup interrput name | SPI number | > ----------------------------------------------------------------------------------- > ALIVE | GPA0 : 8 | INTREQ__EINT[0~7] | SPI[0] ~ SPI[7] | > ALIVE | GPA1 : 8 | INTREQ__EINT[8~15] | SPI[8] ~ SPI[15] | > ALIVE | GPA2 : 8 | INTREQ__EINT_16_63 | SPI[16] | > ALIVE | GPA3 : 8 | INTREQ__EINT_16_63 | SPI[16] | > ----------------------------------------------------------------------------------- > ALIVE | GPF1 : 8 | INTREQ__EINT_16_63 | SPI[16] | > ALIVE | GPF2 : 4 | INTREQ__EINT_16_63 | SPI[16] | > ALIVE | GPF3 : 4 | INTREQ__EINT_16_63 | SPI[16] | > ALIVE | GPF4 : 8 | INTREQ__EINT_16_63 | SPI[16] | > ALIVE | GPF5 : 8 | INTREQ__EINT_16_63 | SPI[16] | > ----------------------------------------------------------------------------------- > > In summary, > If gpf[1-5] handle the CON/DAT/PUD/DRV register, > the driver will use the drvdata->ext_base (IMEM base address) > instead of drvdata->virt_base(ALIVE base address) > because the CON/DAT/PUD/DRV register of gpf[1-5] are included > in the IMEM domain. > > If gpf[1-5] handle the WEINT_* register, > the driver will use the drvdata->virt_base(ALIVE base address) > because the WEINT_* registers of gpf[1-5] are included in the ALIVE domain. Okay, so Krzysztof's suggestion doesn't apply because it's not the eint base which is displaced, but the pinctrl base. I'd suggest the following solution then: - make samsung_pinctrl_drv_data::virt_base an array and save there all mapped iomem resources, - add unsigned pctl_base_res_idx to samsung_pin_bank_data that would be the index of iomem resource into which the samsung_pin_bank_data::pctl_offset is an offfset, Existing EXYNOS_PIN_BANK* macros don't need to be changed, because the field would be 0 by default. Then only the new bank type macro would have another argument that would be the resource index, - replace samsung_pin_bank::pctl_offset with void __iomem *pctl_base and precalculate the addresses at probe time for each bank (pctl_base = virt_base[pctl_base_res_idx] + samsung_pin_bank_data::pctl_offset). Since currently there is only a problem with pctl_base and eint_base seems to be only one, EINT code can simply use virt_base[0] all the time for now. Also you should document the second regs entry in the DT binding. What do you think? Best regards, Tomasz ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <CA+Ln22EaUmdrfLXJfYPAtCQ1DY4Y5iLWr-tqG67-P9hSXKMC6w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH 4/7] pinctrl: samsung: Add GPFx support of Exynos5433 [not found] ` <CA+Ln22EaUmdrfLXJfYPAtCQ1DY4Y5iLWr-tqG67-P9hSXKMC6w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2016-08-24 12:53 ` Chanwoo Choi 0 siblings, 0 replies; 7+ messages in thread From: Chanwoo Choi @ 2016-08-24 12:53 UTC (permalink / raw) To: Tomasz Figa Cc: Krzysztof Kozłowski, Kukjin Kim, Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon, devicetree, linux-arm-kernel, linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel, Krzysztof Kozlowski, Jaehoon Chung, sw0312.kim, Joonyoung Shim, InKi Dae, Jonghwa Lee, Beomho Seo, jaewon02.kim-Sze3O3UU22JBDgjK7y7TUQ Hi Tomasz, I'm sorry for delay reply. On 2016년 08월 19일 20:31, Tomasz Figa wrote: > Hi Chanwoo, > > 2016-08-19 18:07 GMT+09:00 Chanwoo Choi <cw00.choi-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>: >> Hi Tomasz Figa, >> >> Due to wrong setting of email client, >> your reply is deleted on my email client at the company. > > I used Gmail (in plain text mode) to reply, was that related? The mistake depend on my filer setting of mail client. > >> I'm so sorry. So, I add your comment on below and then >> I reply the detailed description. > > No problem. Thanks for description. > >> >> On 2016년 08월 16일 15:27, Chanwoo Choi wrote: >>> From: Joonyoung Shim <jy0922.shim-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> >>> >>> This patch add the support of GPFx pin of Exynos5433 SoC. Exynos5433 has >>> different memory map of GPFx from previous Exynos SoC. Exynos GPIO has >>> following register to control gpio funciton. Usually, all registers of GPIO >>> are included in same domain. >>> - CON / DAT / PUD / DRV / CONPDN / PUDPDN >>> - EINT_CON/ EINT_FLTCON0, EINT_FLTCON1 / EINT_MASK / EINT_PEND >>> >>> But, GPFx are included in two domain as following. So, this patch supports >>> the GPFx pin which handle the on separate two domains. >>> - ALIVE domain : CON / DAT / PUD / DRV / CONPDN / PUDPDN >>> - IMEM domain : EINT_CON/ EINT_FLTCON0, EINT_FLTCON1 / EINT_MASK / EINT_PEND >> >> --------- >> I'm afraid I don't get anything from the description above. Could you >> describe the layout of registers in memory map and IRQ routing of the >> pins? >> >> Best regards, >> Tomasz >> ---------- >> >> On this patch, I'm sorry that I described the wrong information about GFP1~5. >> I explained the memory map of GPF[1-5] the oppositely. Following compositions >> are correct. >> - ALIVE : WEINT_FLTCONx, WEINT_MASK, WEING_PEND >> - IMEM : CON, DAT, PUD, DRV, CONPDN, PUDPDN >> And, I add the memory map for GPF[1~5][2] and the wakeup interrupt information[1]. >> >> [1] Memory map for GPF1~5 >> [ALIVE] >> WEINT_GPA0_CON : 0x1058_0000 (ALIVE) + (0x0700 = 0x0700 + 0x0) >> WEINT_GPA1_CON : 0x1058_0000 (ALIVE) + (0x0704 = 0x0700 + 0x4) >> WEINT_GPA2_CON : 0x1058_0000 (ALIVE) + (0x0708 = 0x0700 + 0x8) >> WEINT_GPA3_CON : 0x1058_0000 (ALIVE) + (0x070C = 0x0700 + 0xC) >> >> WEINT_GPF1_CON : 0x1058_0000 (ALIVE) + (0x1704 = 0x0700 + 0x1004) >> WEINT_GPF2_CON : 0x1058_0000 (ALIVE) + (0x1708 = 0x0700 + 0x1008) >> WEINT_GPF3_CON : 0x1058_0000 (ALIVE) + (0x170C = 0x0700 + 0x100C) >> WEINT_GPF4_CON : 0x1058_0000 (ALIVE) + (0x1710 = 0x0700 + 0x10010) >> WEINT_GPF5_CON : 0x1058_0000 (ALIVE) + (0x1714 = 0x0700 + 0x1014) >> >> WEINT_GPF[x]_MASK : 0x1058_0000 (ALIVE) + (0x1900 + (x * 4)) >> WEINT_GPF[x]_PEND : 0x1058_0000 (ALIVE) + (0x1A00 + (x * 4)) >> (x : 1 ~ 5) >> >> [IMEM] >> GPF1_CON : 0X1109_0000 (IMEM) + 0x0020 >> GPF1_DAT : 0X1109_0000 (IMEM) + 0x0024 >> GPF1_PUD : 0X1109_0000 (IMEM) + 0x0028 >> GPF1_DRV : 0X1109_0000 (IMEM) + 0x002C >> GPF1_CONPDN : 0X1109_0000 (IMEM) + 0x0030 >> GPF1_PUDPDN : 0X1109_0000 (IMEM) + 0x0034 >> >> GPF2_CON : 0X1109_0000 (IMEM) + 0x0040 >> ... >> GPF3_CON : 0X1109_0000 (IMEM) + 0x0060 >> ... >> GPF4_CON : 0X1109_0000 (IMEM) + 0x0080 >> ... >> GPF5_CON : 0X1109_0000 (IMEM) + 0x00A0 >> >> [2] Interrput pin information >> - the total number of wakeup external IRQ is 64. >> ---------------------------------------------------------------------------------- >> domain| gpio : nr | wakeup interrput name | SPI number | >> ----------------------------------------------------------------------------------- >> ALIVE | GPA0 : 8 | INTREQ__EINT[0~7] | SPI[0] ~ SPI[7] | >> ALIVE | GPA1 : 8 | INTREQ__EINT[8~15] | SPI[8] ~ SPI[15] | >> ALIVE | GPA2 : 8 | INTREQ__EINT_16_63 | SPI[16] | >> ALIVE | GPA3 : 8 | INTREQ__EINT_16_63 | SPI[16] | >> ----------------------------------------------------------------------------------- >> ALIVE | GPF1 : 8 | INTREQ__EINT_16_63 | SPI[16] | >> ALIVE | GPF2 : 4 | INTREQ__EINT_16_63 | SPI[16] | >> ALIVE | GPF3 : 4 | INTREQ__EINT_16_63 | SPI[16] | >> ALIVE | GPF4 : 8 | INTREQ__EINT_16_63 | SPI[16] | >> ALIVE | GPF5 : 8 | INTREQ__EINT_16_63 | SPI[16] | >> ----------------------------------------------------------------------------------- >> >> In summary, >> If gpf[1-5] handle the CON/DAT/PUD/DRV register, >> the driver will use the drvdata->ext_base (IMEM base address) >> instead of drvdata->virt_base(ALIVE base address) >> because the CON/DAT/PUD/DRV register of gpf[1-5] are included >> in the IMEM domain. >> >> If gpf[1-5] handle the WEINT_* register, >> the driver will use the drvdata->virt_base(ALIVE base address) >> because the WEINT_* registers of gpf[1-5] are included in the ALIVE domain. > > Okay, so Krzysztof's suggestion doesn't apply because it's not the > eint base which is displaced, but the pinctrl base. I'd suggest the > following solution then: > > - make samsung_pinctrl_drv_data::virt_base an array and save there all > mapped iomem resources, > > - add unsigned pctl_base_res_idx to samsung_pin_bank_data that would > be the index of iomem resource into which the > samsung_pin_bank_data::pctl_offset is an offfset, Existing > EXYNOS_PIN_BANK* macros don't need to be changed, because the field > would be 0 by default. Then only the new bank type macro would have > another argument that would be the resource index, > > - replace samsung_pin_bank::pctl_offset with void __iomem *pctl_base > and precalculate the addresses at probe time for each bank (pctl_base > = virt_base[pctl_base_res_idx] + samsung_pin_bank_data::pctl_offset). > Since currently there is only a problem with pctl_base and eint_base > seems to be only one, EINT code can simply use virt_base[0] all the > time for now. > > Also you should document the second regs entry in the DT binding. > > What do you think? I understand. I suggest the one thing. I think that we need to add the 'eint_base_idx' for WEINT registers which is handled on pinctrl-exynos.c because the composition of gpio registers might be exchanged against the Exynos5433's GPFx. "drvdata->virt_base[pctl_res_idx]" will be used on pinctrl-samsung.c. "drvdata->virt_base[eint_res_idx]" will be used on pinctrl-exynos.c. As your suggestion, I make a patch and this patch is well working. I'll send the new patch for samsung pinctrl driver on v2 patchset. -- Best Regards, Chanwoo Choi -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-08-24 12:53 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1471328843-26653-1-git-send-email-cw00.choi@samsung.com>
2016-08-16 6:27 ` [PATCH 4/7] pinctrl: samsung: Add GPFx support of Exynos5433 Chanwoo Choi
2016-08-16 6:42 ` Tomasz Figa
[not found] ` <1471328843-26653-5-git-send-email-cw00.choi-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
2016-08-16 8:46 ` Krzysztof Kozlowski
2016-08-18 19:00 ` Rob Herring
2016-08-19 9:07 ` Chanwoo Choi
2016-08-19 11:31 ` Tomasz Figa
[not found] ` <CA+Ln22EaUmdrfLXJfYPAtCQ1DY4Y5iLWr-tqG67-P9hSXKMC6w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-08-24 12:53 ` Chanwoo Choi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox