From: Kukjin Kim <kgene.kim@samsung.com>
To: 'Vasily Khoruzhick' <anarsoul@gmail.com>,
'Lars-Peter Clausen' <lars@metafoo.de>,
'Ben Dooks' <ben-linux@fluff.org>,
'Russell King' <linux@arm.linux.org.uk>,
linux-samsung-soc@vger.k
Subject: RE: [PATCH v5] ARM: s3c244x: Fix mess with gpio {set,get}_pull callbacks
Date: Wed, 01 Dec 2010 16:26:31 +0900 [thread overview]
Message-ID: <001301cb9129$15f1b5c0$41d52140$%kim@samsung.com> (raw)
In-Reply-To: <1291147949-18812-1-git-send-email-anarsoul@gmail.com>
Vasily Khoruzhick wrote:
>
> Currently the {set,get}_pull callbacks of the s3c24xx_gpiocfg_default
> structure
> are initalized via s3c_gpio_{get,set}pull_1up. This results in a linker
> error when only CONFIG_CPU_S3C2442 is selected:
>
> arch/arm/plat-s3c24xx/built-in.o:(.data+0x13f4): undefined reference to
> `s3c_gpio_getpull_1up'
> arch/arm/plat-s3c24xx/built-in.o:(.data+0x13f8): undefined reference to
> `s3c_gpio_setpull_1up'
>
> The s3c2442 has pulldowns instead of pullups compared to the s3c2440.
> The method of controlling them is the same though.
> So this patch modifies the existing s3c_gpio_{get,set}pull_1up helper
> functions
> to take an additional parameter deciding whether the pin has a pullup or
> pulldown.
> The s3c_gpio_{get,set}pull_1{down,up} functions then wrap that functions
> passing
> either S3C_GPIO_PULL_UP or S3C_GPIO_PULL_DOWN.
>
> Furthermore this patch sets up the s3c24xx_gpiocfg_default.{get,set}_pull
> fields
> in the s3c244{0,2}_map_io function to the new pulldown helper functions.
>
> Based on patch from "Lars-Peter Clausen" <lars@metafoo.de>
>
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> ---
> v2: adapt patch for 2.6.37-rc1
> v3: restore default pull callbacks, add default pull callbacks for s3c2442
> v4: remove default pull callbacks, set them in per-soc map_io function
> instead.
> v5: fix obvious mistake in setpull_1
> arch/arm/mach-s3c2440/Kconfig | 1 +
> arch/arm/mach-s3c2440/s3c2440.c | 11 ++++-
> arch/arm/mach-s3c2440/s3c2442.c | 14 ++++++
> arch/arm/plat-s3c24xx/cpu.c | 8 ++--
> arch/arm/plat-s3c24xx/gpiolib.c | 2 -
> arch/arm/plat-s3c24xx/include/plat/s3c244x.h | 7 +++-
> arch/arm/plat-samsung/gpio-config.c | 45
++++++++++++++++--
> --
> .../plat-samsung/include/plat/gpio-cfg-helpers.h | 11 +++++
> 8 files changed, 80 insertions(+), 19 deletions(-)
>
> diff --git a/arch/arm/mach-s3c2440/Kconfig b/arch/arm/mach-s3c2440/Kconfig
> index ff024a6..478fae0 100644
> --- a/arch/arm/mach-s3c2440/Kconfig
> +++ b/arch/arm/mach-s3c2440/Kconfig
> @@ -18,6 +18,7 @@ config CPU_S3C2440
> config CPU_S3C2442
> bool
> select CPU_ARM920T
> + select S3C_GPIO_PULL_DOWN
> select S3C2410_CLOCK
> select S3C2410_GPIO
> select S3C2410_PM if PM
> diff --git a/arch/arm/mach-s3c2440/s3c2440.c b/arch/arm/mach-
> s3c2440/s3c2440.c
> index d50f3ae..f7663f7 100644
> --- a/arch/arm/mach-s3c2440/s3c2440.c
> +++ b/arch/arm/mach-s3c2440/s3c2440.c
> @@ -46,9 +46,6 @@ int __init s3c2440_init(void)
> {
> printk("S3C2440: Initialising architecture\n");
>
> - s3c24xx_gpiocfg_default.set_pull = s3c_gpio_setpull_1up;
> - s3c24xx_gpiocfg_default.get_pull = s3c_gpio_getpull_1up;
> -
> /* change irq for watchdog */
>
> s3c_device_wdt.resource[1].start = IRQ_S3C2440_WDT;
> @@ -58,3 +55,11 @@ int __init s3c2440_init(void)
>
> return sysdev_register(&s3c2440_sysdev);
> }
> +
> +void __init s3c2440_map_io(void)
> +{
> + s3c244x_map_io();
> +
> + s3c24xx_gpiocfg_default.set_pull = s3c_gpio_setpull_1up;
> + s3c24xx_gpiocfg_default.get_pull = s3c_gpio_getpull_1up;
> +}
How about following...
+void __init s3c2440_map_io(void)
+{
+ s3c24xx_gpiocfg_default.set_pull = s3c_gpio_setpull_1up;
+ s3c24xx_gpiocfg_default.get_pull = s3c_gpio_getpull_1up;
+
+ s3c244x_map_io();
+}
> diff --git a/arch/arm/mach-s3c2440/s3c2442.c b/arch/arm/mach-
> s3c2440/s3c2442.c
> index 188ad1e..ecf8135 100644
> --- a/arch/arm/mach-s3c2440/s3c2442.c
> +++ b/arch/arm/mach-s3c2440/s3c2442.c
> @@ -32,6 +32,7 @@
> #include <linux/interrupt.h>
> #include <linux/ioport.h>
> #include <linux/mutex.h>
> +#include <linux/gpio.h>
> #include <linux/clk.h>
> #include <linux/io.h>
>
> @@ -43,6 +44,11 @@
>
> #include <plat/clock.h>
> #include <plat/cpu.h>
> +#include <plat/s3c244x.h>
> +
No need this empty line.
> +#include <plat/gpio-core.h>
> +#include <plat/gpio-cfg.h>
> +#include <plat/gpio-cfg-helpers.h>
>
> /* S3C2442 extended clock support */
>
> @@ -163,3 +169,11 @@ int __init s3c2442_init(void)
>
> return sysdev_register(&s3c2442_sysdev);
> }
> +
> +void __init s3c2442_map_io(void)
> +{
> + s3c244x_map_io();
> +
> + s3c24xx_gpiocfg_default.set_pull = s3c_gpio_setpull_1down;
> + s3c24xx_gpiocfg_default.get_pull = s3c_gpio_getpull_1down;
> +}
Same as above.
> diff --git a/arch/arm/plat-s3c24xx/cpu.c b/arch/arm/plat-s3c24xx/cpu.c
> index 76d0858..4a10c0f 100644
> --- a/arch/arm/plat-s3c24xx/cpu.c
> +++ b/arch/arm/plat-s3c24xx/cpu.c
> @@ -88,7 +88,7 @@ static struct cpu_table cpu_ids[] __initdata = {
> {
> .idcode = 0x32440000,
> .idmask = 0xffffffff,
> - .map_io = s3c244x_map_io,
> + .map_io = s3c2440_map_io,
> .init_clocks = s3c244x_init_clocks,
> .init_uarts = s3c244x_init_uarts,
> .init = s3c2440_init,
> @@ -97,7 +97,7 @@ static struct cpu_table cpu_ids[] __initdata = {
> {
> .idcode = 0x32440001,
> .idmask = 0xffffffff,
> - .map_io = s3c244x_map_io,
> + .map_io = s3c2440_map_io,
> .init_clocks = s3c244x_init_clocks,
> .init_uarts = s3c244x_init_uarts,
> .init = s3c2440_init,
> @@ -106,7 +106,7 @@ static struct cpu_table cpu_ids[] __initdata = {
> {
> .idcode = 0x32440aaa,
> .idmask = 0xffffffff,
> - .map_io = s3c244x_map_io,
> + .map_io = s3c2442_map_io,
> .init_clocks = s3c244x_init_clocks,
> .init_uarts = s3c244x_init_uarts,
> .init = s3c2442_init,
> @@ -115,7 +115,7 @@ static struct cpu_table cpu_ids[] __initdata = {
> {
> .idcode = 0x32440aab,
> .idmask = 0xffffffff,
> - .map_io = s3c244x_map_io,
> + .map_io = s3c2442_map_io,
> .init_clocks = s3c244x_init_clocks,
> .init_uarts = s3c244x_init_uarts,
> .init = s3c2442_init,
> diff --git a/arch/arm/plat-s3c24xx/gpiolib.c b/arch/arm/plat-
> s3c24xx/gpiolib.c
> index 24c6f5a..243b641 100644
> --- a/arch/arm/plat-s3c24xx/gpiolib.c
> +++ b/arch/arm/plat-s3c24xx/gpiolib.c
> @@ -82,8 +82,6 @@ static struct s3c_gpio_cfg s3c24xx_gpiocfg_banka = {
> struct s3c_gpio_cfg s3c24xx_gpiocfg_default = {
> .set_config = s3c_gpio_setcfg_s3c24xx,
> .get_config = s3c_gpio_getcfg_s3c24xx,
> - .set_pull = s3c_gpio_setpull_1up,
> - .get_pull = s3c_gpio_getpull_1up,
Yeah, however, in my opinion, need to add following during gpiolib_init().
@@ -222,6 +222,11 @@ static __init int s3c24xx_gpiolib_init(void)
if (!chip->config)
chip->config = &s3c24xx_gpiocfg_default;
+ if (!chip->config->set_pull)
+ chip->config->set_pull = s3c_gpio_setpull_1up;
+ if (!chip->config->get_pull)
+ chip->config->get_pull = s3c_gpio_getpull_1up;
+
s3c_gpiolib_add(chip);
}
> };
>
> struct s3c_gpio_chip s3c24xx_gpios[] = {
> diff --git a/arch/arm/plat-s3c24xx/include/plat/s3c244x.h b/arch/arm/plat-
> s3c24xx/include/plat/s3c244x.h
> index 307248d..89e8d0a 100644
> --- a/arch/arm/plat-s3c24xx/include/plat/s3c244x.h
> +++ b/arch/arm/plat-s3c24xx/include/plat/s3c244x.h
> @@ -21,17 +21,22 @@ extern void s3c244x_init_clocks(int xtal);
> #else
> #define s3c244x_init_clocks NULL
> #define s3c244x_init_uarts NULL
> -#define s3c244x_map_io NULL
> #endif
>
> #ifdef CONFIG_CPU_S3C2440
> extern int s3c2440_init(void);
> +
> +extern void s3c2440_map_io(void);
> #else
> #define s3c2440_init NULL
> +#define s3c2440_map_io NULL
> #endif
>
> #ifdef CONFIG_CPU_S3C2442
> extern int s3c2442_init(void);
> +
> +extern void s3c2442_map_io(void);
> #else
> #define s3c2442_init NULL
> +#define s3c2442_map_io NULL
> #endif
> diff --git a/arch/arm/plat-samsung/gpio-config.c b/arch/arm/plat-
> samsung/gpio-config.c
> index b732b77..304ce7e 100644
> --- a/arch/arm/plat-samsung/gpio-config.c
> +++ b/arch/arm/plat-samsung/gpio-config.c
> @@ -280,16 +280,15 @@ s3c_gpio_pull_t s3c_gpio_getpull_updown(struct
> s3c_gpio_chip *chip,
> }
> #endif
>
> -#ifdef CONFIG_S3C_GPIO_PULL_UP
> -int s3c_gpio_setpull_1up(struct s3c_gpio_chip *chip,
> - unsigned int off, s3c_gpio_pull_t pull)
> +#if defined(CONFIG_S3C_GPIO_PULL_UP) ||
defined(CONFIG_S3C_GPIO_PULL_DOWN)
> +static int s3c_gpio_setpull_1(struct s3c_gpio_chip *chip,
> + unsigned int off, s3c_gpio_pull_t pull,
> + s3c_gpio_pull_t updown)
> {
> void __iomem *reg = chip->base + 0x08;
> u32 pup = __raw_readl(reg);
>
> - pup = __raw_readl(reg);
> -
> - if (pup == S3C_GPIO_PULL_UP)
> + if (pull == updown)
> pup &= ~(1 << off);
> else if (pup == S3C_GPIO_PULL_NONE)
pull
> pup |= (1 << off);
> @@ -300,17 +299,45 @@ int s3c_gpio_setpull_1up(struct s3c_gpio_chip *chip,
> return 0;
> }
>
> -s3c_gpio_pull_t s3c_gpio_getpull_1up(struct s3c_gpio_chip *chip,
> - unsigned int off)
> +static s3c_gpio_pull_t s3c_gpio_getpull_1(struct s3c_gpio_chip *chip,
> + unsigned int off, s3c_gpio_pull_t
updown)
> {
> void __iomem *reg = chip->base + 0x08;
> u32 pup = __raw_readl(reg);
>
> pup &= (1 << off);
> - return pup ? S3C_GPIO_PULL_NONE : S3C_GPIO_PULL_UP;
> + return pup ? S3C_GPIO_PULL_NONE : updown;
> +}
> +#endif /* CONFIG_S3C_GPIO_PULL_UP || CONFIG_S3C_GPIO_PULL_DOWN */
> +
> +#ifdef CONFIG_S3C_GPIO_PULL_UP
> +s3c_gpio_pull_t s3c_gpio_getpull_1up(struct s3c_gpio_chip *chip,
> + unsigned int off)
> +{
> + return s3c_gpio_getpull_1(chip, off, S3C_GPIO_PULL_UP);
> +}
> +
> +int s3c_gpio_setpull_1up(struct s3c_gpio_chip *chip,
> + unsigned int off, s3c_gpio_pull_t pull)
> +{
> + return s3c_gpio_setpull_1(chip, off, pull, S3C_GPIO_PULL_UP);
> }
> #endif /* CONFIG_S3C_GPIO_PULL_UP */
>
> +#ifdef CONFIG_S3C_GPIO_PULL_DOWN
> +s3c_gpio_pull_t s3c_gpio_getpull_1down(struct s3c_gpio_chip *chip,
> + unsigned int off)
> +{
> + return s3c_gpio_getpull_1(chip, off, S3C_GPIO_PULL_DOWN);
> +}
> +
> +int s3c_gpio_setpull_1down(struct s3c_gpio_chip *chip,
> + unsigned int off, s3c_gpio_pull_t pull)
> +{
> + return s3c_gpio_setpull_1(chip, off, pull, S3C_GPIO_PULL_DOWN);
> +}
> +#endif /* CONFIG_S3C_GPIO_PULL_DOWN */
> +
How about to separate functions between PULL_UP and PULL_DOWN like
following...
#ifdef CONFIG_S3C_GPIO_PULL_UP
s3c_gpio_pull_t s3c_gpio_getpull_1up(struct s3c_gpio_chip *chip,
unsigned int off)
{
void __iomem *reg = chip->base + 0x08;
u32 pup = __raw_readl(reg);
pup &= (1 << off);
return pup ? S3C_GPIO_PULL_NONE : S3C_GPIO_PULL_UP;
}
int s3c_gpio_setpull_1up(struct s3c_gpio_chip *chip,
unsigned int off, s3c_gpio_pull_t pull)
{
void __iomem *reg = chip->base + 0x08;
u32 pup = __raw_readl(reg);
if (pull == S3C_GPIO_PULL_UP)
pup &= ~(1 << off);
else if (pull == S3C_GPIO_PULL_NONE)
pup |= (1 << off);
else
return -EINVAL;
__raw_writel(pup, reg);
return 0;
}
#endif /* CONFIG_S3C_GPIO_PULL_UP */
#ifdef CONFIG_S3C_GPIO_PULL_DOWN
s3c_gpio_pull_t s3c_gpio_getpull_1down(struct s3c_gpio_chip *chip,
unsigned int off)
{
void __iomem *reg = chip->base + 0x08;
u32 pdown = __raw_readl(reg);
pdown &= (1 << off);
return pdown ? S3C_GPIO_PULL_NONE : S3C_GPIO_PULL_DOWN;
}
int s3c_gpio_setpull_1down(struct s3c_gpio_chip *chip,
unsigned int off, s3c_gpio_pull_t pull)
{
void __iomem *reg = chip->base + 0x08;
u32 pdown = __raw_readl(reg);
if (pull == S3C_GPIO_PULL_DOWN)
pdown &= ~(1 << off);
else if (pull == S3C_GPIO_PULL_NONE)
pdown |= (1 << off);
else
return -EINVAL;
__raw_writel(pdown, reg);
return 0;
}
#endif /* CONFIG_S3C_GPIO_PULL_DOWN */
> #ifdef CONFIG_S5P_GPIO_DRVSTR
> s5p_gpio_drvstr_t s5p_gpio_get_drvstr(unsigned int pin)
> {
> diff --git a/arch/arm/plat-samsung/include/plat/gpio-cfg-helpers.h
> b/arch/arm/plat-samsung/include/plat/gpio-cfg-helpers.h
> index 8fd65d8..0d2c570 100644
> --- a/arch/arm/plat-samsung/include/plat/gpio-cfg-helpers.h
> +++ b/arch/arm/plat-samsung/include/plat/gpio-cfg-helpers.h
> @@ -210,6 +210,17 @@ extern s3c_gpio_pull_t s3c_gpio_getpull_1up(struct
> s3c_gpio_chip *chip,
> unsigned int off);
>
> /**
> + * s3c_gpio_getpull_1down() - Get configuration for choice of down or
none
> + * @chip: The gpio chip that the GPIO pin belongs to
> + * @off: The offset to the pin to get the configuration of.
> + *
> + * This helper function reads the state of the pull-down resistor for the
> + * given GPIO in the same case as s3c_gpio_setpull_1down.
> +*/
> +extern s3c_gpio_pull_t s3c_gpio_getpull_1down(struct s3c_gpio_chip *chip,
> + unsigned int off);
> +
> +/**
> * s3c_gpio_setpull_s3c2443() - Pull configuration for s3c2443.
> * @chip: The gpio chip that is being configured.
> * @off: The offset for the GPIO being configured.
> --
Thanks.
Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.
next prev parent reply other threads:[~2010-12-01 7:26 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-09-20 22:00 [PATCH] ARM: s3c2442: Setup gpio {set,get}_pull callbacks Lars-Peter Clausen
2010-09-20 22:00 ` [PATCH] ARM: s3c24xx: Set ARCH_NR_GPIOS according to the selected SoC types Lars-Peter Clausen
2010-09-20 23:41 ` [PATCH] ARM: s3c2442: Setup gpio {set,get}_pull callbacks Ben Dooks
2010-09-21 5:28 ` Abdoulaye Walsimou GAYE
2010-11-06 7:54 ` [PATCH v2] " Vasily Khoruzhick
[not found] ` <4CD57DC1.4050403@embtoolkit.org>
2010-11-08 9:08 ` Vasily Khoruzhick
2010-11-08 20:26 ` [PATCH v3] " Vasily Khoruzhick
2010-11-08 21:41 ` Abdoulaye Walsimou GAYE
2010-11-29 9:56 ` Vasily Khoruzhick
2010-11-29 10:15 ` Kukjin Kim
2010-11-29 10:26 ` [PATCH RESEND " Vasily Khoruzhick
2010-11-30 6:18 ` Kukjin Kim
2010-11-30 11:53 ` [PATCH RESEND v3] ARM: s3c2442: Setup gpio {set, get}_pull callbacks Lars-Peter Clausen
2010-11-30 12:01 ` Vasily Khoruzhick
2010-11-30 13:12 ` [PATCH RESEND v3] ARM: s3c2442: Setup gpio {set,get}_pull callbacks Lars-Peter Clausen
2010-11-30 14:33 ` Vasily Khoruzhick
2010-11-30 14:53 ` Lars-Peter Clausen
2010-11-30 15:01 ` Vasily Khoruzhick
2010-11-30 19:46 ` [PATCH v4] ARM: s3c244x: Fix mess with " Vasily Khoruzhick
[not found] ` <1291146383-16215-1-git-send-email-anarsoul@gmail.com>
2010-11-30 19:59 ` Lars-Peter Clausen
2010-11-30 20:05 ` Vasily Khoruzhick
2010-11-30 20:12 ` [PATCH v5] " Vasily Khoruzhick
2010-12-01 7:26 ` Kukjin Kim [this message]
2010-12-01 10:17 ` Vasily Khoruzhick
2010-12-01 12:05 ` Kukjin Kim
2010-12-01 12:16 ` Vasily Khoruzhick
2010-12-01 5:28 ` [PATCH v4] " Kukjin Kim
2010-12-01 6:19 ` Vasily Khoruzhick
2010-12-01 6:29 ` [PATCH v6] " Vasily Khoruzhick
2010-12-01 5:19 ` [PATCH RESEND v3] ARM: s3c2442: Setup " Kukjin Kim
2010-11-29 10:28 ` [PATCH " Vasily Khoruzhick
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='001301cb9129$15f1b5c0$41d52140$%kim@samsung.com' \
--to=kgene.kim@samsung.com \
--cc=anarsoul@gmail.com \
--cc=ben-linux@fluff.org \
--cc=lars@metafoo.de \
--cc=linux-samsung-soc@vger.k \
--cc=linux@arm.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox