| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 16 Aug 2016 10:46:02 +0200
From: Krzysztof Kozlowski <k.kozlowski@...sung.com>
To: Chanwoo Choi <cw00.choi@...sung.com>, kgene@...nel.org,
robh+dt@...nel.org, mark.rutland@....com, catalin.marinas@....com,
will.deacon@....com, devicetree@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org,
linux-samsung-soc@...r.kernel.org, linux-kernel@...r.kernel.org
Cc: krzk@...nel.org, jh80.chung@...sung.com, sw0312.kim@...sung.com,
jy0922.shim@...sung.com, inki.dae@...sung.com,
jonghwa3.lee@...sung.com, beomho.seo@...sung.com,
jaewon02.kim@...sung.com, human.hwang@...sung.com,
ideal.song@...sung.com, ingi2.kim@...sung.com,
m.szyprowski@...sung.com, a.hajda@...sung.com,
s.nawrocki@...sung.com, chanwoo@...nel.org,
Linus Walleij <linus.walleij@...aro.org>,
Tomasz Figa <tomasz.figa@...il.com>, linux-gpio@...r.kernel.org
Subject: Re: [PATCH 4/7] pinctrl: samsung: Add GPFx support of Exynos5433
On 08/16/2016 08:27 AM, Chanwoo Choi wrote:
> From: Joonyoung Shim <jy0922.shim@...sung.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@...aro.org>
> Cc: Rob Herring <robh+dt@...nel.org>
> Cc: Mark Rutland <mark.rutland@....com>
> Cc: Tomasz Figa <tomasz.figa@...il.com>
> Cc: Krzysztof Kozlowski <k.kozlowski@...sung.com>
> Cc: Sylwester Nawrocki <s.nawrocki@...sung.com>
> Cc: Kukjin Kim <kgene@...nel.org>
> Cc: linux-gpio@...r.kernel.org
> Signed-off-by: Joonyoung Shim <jy0922.shim@...sung.com>
> Signed-off-by: Chanwoo Choi <cw00.choi@...sung.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;
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
Powered by blists - more mailing lists