leds: s3c24xx: Convert to use GPIO descriptors

Message ID 20200627001015.427376-1-linus.walleij@linaro.org
State Superseded
Headers show
Series
  • leds: s3c24xx: Convert to use GPIO descriptors
Related show

Commit Message

Linus Walleij June 27, 2020, 12:10 a.m.
This converts the s3c24xx LED driver to use GPIO descriptors
and also modify all board files to account for these changes
by registering the appropriate GPIO tables for each board.

The driver was using a custom flag to indicate open drain
(tristate) but this can be handled by standard descriptor
machine tables.

The driver was setting non-pull-up for the pin using the custom
S3C24xx GPIO API, but this is a custom pin control system used
by the S3C24xx and no generic GPIO function, so this has simply
been pushed back into the respective board files.

Cc: Ben Dooks <ben-linux@fluff.org>
Cc: Sylwester Nawrocki <sylvester.nawrocki@gmail.com>
Cc: Krzysztof Kozlowski <krzk@kernel.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

---
 arch/arm/mach-s3c24xx/common-smdk.c        | 67 +++++++++++++++-------
 arch/arm/mach-s3c24xx/mach-mini2440.c      | 63 +++++++++++++++++---
 arch/arm/mach-s3c24xx/mach-n30.c           | 54 +++++++++++++++--
 arch/arm/mach-s3c24xx/mach-qt2410.c        | 12 +++-
 arch/arm/mach-s3c24xx/mach-vr1000.c        | 38 +++++++++++-
 drivers/leds/leds-s3c24xx.c                | 37 +++---------
 include/linux/platform_data/leds-s3c24xx.h |  6 --
 7 files changed, 200 insertions(+), 77 deletions(-)

-- 
2.25.4

Comments

Krzysztof Kozlowski June 29, 2020, 6:40 p.m. | #1
On Sat, Jun 27, 2020 at 02:10:15AM +0200, Linus Walleij wrote:
> This converts the s3c24xx LED driver to use GPIO descriptors

> and also modify all board files to account for these changes

> by registering the appropriate GPIO tables for each board.

> 

> The driver was using a custom flag to indicate open drain

> (tristate) but this can be handled by standard descriptor

> machine tables.

> 

> The driver was setting non-pull-up for the pin using the custom

> S3C24xx GPIO API, but this is a custom pin control system used

> by the S3C24xx and no generic GPIO function, so this has simply

> been pushed back into the respective board files.

> 

> Cc: Ben Dooks <ben-linux@fluff.org>

> Cc: Sylwester Nawrocki <sylvester.nawrocki@gmail.com>

> Cc: Krzysztof Kozlowski <krzk@kernel.org>

> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

> ---

>  arch/arm/mach-s3c24xx/common-smdk.c        | 67 +++++++++++++++-------

>  arch/arm/mach-s3c24xx/mach-mini2440.c      | 63 +++++++++++++++++---

>  arch/arm/mach-s3c24xx/mach-n30.c           | 54 +++++++++++++++--

>  arch/arm/mach-s3c24xx/mach-qt2410.c        | 12 +++-

>  arch/arm/mach-s3c24xx/mach-vr1000.c        | 38 +++++++++++-

>  drivers/leds/leds-s3c24xx.c                | 37 +++---------

>  include/linux/platform_data/leds-s3c24xx.h |  6 --

>  7 files changed, 200 insertions(+), 77 deletions(-)

> 

> diff --git a/arch/arm/mach-s3c24xx/common-smdk.c b/arch/arm/mach-s3c24xx/common-smdk.c

> index 58e30cad386c..10fc804c4ec5 100644

> --- a/arch/arm/mach-s3c24xx/common-smdk.c

> +++ b/arch/arm/mach-s3c24xx/common-smdk.c

> @@ -14,6 +14,7 @@

>  #include <linux/timer.h>

>  #include <linux/init.h>

>  #include <linux/gpio.h>

> +#include <linux/gpio/machine.h>

>  #include <linux/device.h>

>  #include <linux/platform_device.h>

>  

> @@ -44,29 +45,53 @@

>  

>  /* LED devices */

>  

> +static struct gpiod_lookup_table smdk_led4_gpio_table = {

> +	.dev_id = "s3c24xx_led.0",

> +	.table = {

> +		GPIO_LOOKUP("GPF", 4, NULL, GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN),

> +		{ },

> +	},

> +};

> +

> +static struct gpiod_lookup_table smdk_led5_gpio_table = {

> +	.dev_id = "s3c24xx_led.1",

> +	.table = {

> +		GPIO_LOOKUP("GPF", 6, NULL, GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN),


The original code has here GPF(5).

> +		{ },

> +	},

> +};

> +

> +static struct gpiod_lookup_table smdk_led6_gpio_table = {

> +	.dev_id = "s3c24xx_led.2",

> +	.table = {

> +		GPIO_LOOKUP("GPF", 5, NULL, GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN),


Ditto, but GPF(6).

Rest looks fine. I will need Jacek's/Pavel's/Dan's ack to take it through
samsung-soc.

Best regards,
Krzysztof
Jacek Anaszewski June 29, 2020, 9:49 p.m. | #2
Hi Linus,

Thank you for the patch.

Besides what Krzysztof already pointed out, I have some
other nits below.

On 6/27/20 2:10 AM, Linus Walleij wrote:
> This converts the s3c24xx LED driver to use GPIO descriptors

> and also modify all board files to account for these changes

> by registering the appropriate GPIO tables for each board.

> 

> The driver was using a custom flag to indicate open drain

> (tristate) but this can be handled by standard descriptor

> machine tables.

> 

> The driver was setting non-pull-up for the pin using the custom

> S3C24xx GPIO API, but this is a custom pin control system used

> by the S3C24xx and no generic GPIO function, so this has simply

> been pushed back into the respective board files.

> 

> Cc: Ben Dooks <ben-linux@fluff.org>

> Cc: Sylwester Nawrocki <sylvester.nawrocki@gmail.com>

> Cc: Krzysztof Kozlowski <krzk@kernel.org>

> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

> ---

>   arch/arm/mach-s3c24xx/common-smdk.c        | 67 +++++++++++++++-------

>   arch/arm/mach-s3c24xx/mach-mini2440.c      | 63 +++++++++++++++++---

>   arch/arm/mach-s3c24xx/mach-n30.c           | 54 +++++++++++++++--

>   arch/arm/mach-s3c24xx/mach-qt2410.c        | 12 +++-

>   arch/arm/mach-s3c24xx/mach-vr1000.c        | 38 +++++++++++-

>   drivers/leds/leds-s3c24xx.c                | 37 +++---------

>   include/linux/platform_data/leds-s3c24xx.h |  6 --

>   7 files changed, 200 insertions(+), 77 deletions(-)

> 

> diff --git a/arch/arm/mach-s3c24xx/common-smdk.c b/arch/arm/mach-s3c24xx/common-smdk.c

> index 58e30cad386c..10fc804c4ec5 100644

> --- a/arch/arm/mach-s3c24xx/common-smdk.c

> +++ b/arch/arm/mach-s3c24xx/common-smdk.c

> @@ -14,6 +14,7 @@

>   #include <linux/timer.h>

>   #include <linux/init.h>

>   #include <linux/gpio.h>

> +#include <linux/gpio/machine.h>

>   #include <linux/device.h>

>   #include <linux/platform_device.h>

>   

> @@ -44,29 +45,53 @@

>   

>   /* LED devices */

>   

> +static struct gpiod_lookup_table smdk_led4_gpio_table = {

> +	.dev_id = "s3c24xx_led.0",

> +	.table = {

> +		GPIO_LOOKUP("GPF", 4, NULL, GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN),

> +		{ },

> +	},

> +};

> +

> +static struct gpiod_lookup_table smdk_led5_gpio_table = {

> +	.dev_id = "s3c24xx_led.1",

> +	.table = {

> +		GPIO_LOOKUP("GPF", 6, NULL, GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN),

> +		{ },

> +	},

> +};

> +

> +static struct gpiod_lookup_table smdk_led6_gpio_table = {

> +	.dev_id = "s3c24xx_led.2",

> +	.table = {

> +		GPIO_LOOKUP("GPF", 5, NULL, GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN),

> +		{ },

> +	},

> +};

> +

> +static struct gpiod_lookup_table smdk_led7_gpio_table = {

> +	.dev_id = "s3c24xx_led.3",

> +	.table = {

> +		GPIO_LOOKUP("GPF", 7, NULL, GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN),

> +		{ },

> +	},

> +};

> +

>   static struct s3c24xx_led_platdata smdk_pdata_led4 = {

> -	.gpio		= S3C2410_GPF(4),

> -	.flags		= S3C24XX_LEDF_ACTLOW | S3C24XX_LEDF_TRISTATE,

>   	.name		= "led4",

>   	.def_trigger	= "timer",

>   };

>   

>   static struct s3c24xx_led_platdata smdk_pdata_led5 = {

> -	.gpio		= S3C2410_GPF(5),

> -	.flags		= S3C24XX_LEDF_ACTLOW | S3C24XX_LEDF_TRISTATE,

>   	.name		= "led5",

>   	.def_trigger	= "nand-disk",

>   };

>   

>   static struct s3c24xx_led_platdata smdk_pdata_led6 = {

> -	.gpio		= S3C2410_GPF(6),

> -	.flags		= S3C24XX_LEDF_ACTLOW | S3C24XX_LEDF_TRISTATE,

>   	.name		= "led6",

>   };

>   

>   static struct s3c24xx_led_platdata smdk_pdata_led7 = {

> -	.gpio		= S3C2410_GPF(7),

> -	.flags		= S3C24XX_LEDF_ACTLOW | S3C24XX_LEDF_TRISTATE,

>   	.name		= "led7",

>   };

>   

> @@ -179,27 +204,25 @@ static struct platform_device __initdata *smdk_devs[] = {

>   	&smdk_led7,

>   };

>   

> -static const struct gpio smdk_led_gpios[] = {

> -	{ S3C2410_GPF(4), GPIOF_OUT_INIT_HIGH, NULL },

> -	{ S3C2410_GPF(5), GPIOF_OUT_INIT_HIGH, NULL },

> -	{ S3C2410_GPF(6), GPIOF_OUT_INIT_HIGH, NULL },

> -	{ S3C2410_GPF(7), GPIOF_OUT_INIT_HIGH, NULL },

> -};

> -

>   void __init smdk_machine_init(void)

>   {

> -	/* Configure the LEDs (even if we have no LED support)*/

> -

> -	int ret = gpio_request_array(smdk_led_gpios,

> -				     ARRAY_SIZE(smdk_led_gpios));

> -	if (!WARN_ON(ret < 0))

> -		gpio_free_array(smdk_led_gpios, ARRAY_SIZE(smdk_led_gpios));

> -

>   	if (machine_is_smdk2443())

>   		smdk_nand_info.twrph0 = 50;

>   

>   	s3c_nand_set_platdata(&smdk_nand_info);

>   

> +	/* Disable pull-up on the LED lines */

> +	s3c_gpio_setpull(S3C2410_GPF(4), S3C_GPIO_PULL_NONE);

> +	s3c_gpio_setpull(S3C2410_GPF(5), S3C_GPIO_PULL_NONE);

> +	s3c_gpio_setpull(S3C2410_GPF(6), S3C_GPIO_PULL_NONE);

> +	s3c_gpio_setpull(S3C2410_GPF(7), S3C_GPIO_PULL_NONE);

> +

> +	/* Add lookups for the lines */

> +	gpiod_add_lookup_table(&smdk_led4_gpio_table);

> +	gpiod_add_lookup_table(&smdk_led5_gpio_table);

> +	gpiod_add_lookup_table(&smdk_led6_gpio_table);

> +	gpiod_add_lookup_table(&smdk_led7_gpio_table);

> +

>   	platform_add_devices(smdk_devs, ARRAY_SIZE(smdk_devs));

>   

>   	s3c_pm_init();

> diff --git a/arch/arm/mach-s3c24xx/mach-mini2440.c b/arch/arm/mach-s3c24xx/mach-mini2440.c

> index 9035f868fb34..11add63b79c2 100644

> --- a/arch/arm/mach-s3c24xx/mach-mini2440.c

> +++ b/arch/arm/mach-s3c24xx/mach-mini2440.c

> @@ -402,37 +402,68 @@ static struct platform_device mini2440_button_device = {

>   

>   /* LEDS */

>   

> +static struct gpiod_lookup_table mini2440_led1_gpio_table = {

> +	.dev_id = "s3c24xx_led.1",

> +	.table = {

> +		GPIO_LOOKUP("GPB", 5, NULL, GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN),

> +		{ },

> +	},

> +};

> +

> +static struct gpiod_lookup_table mini2440_led2_gpio_table = {

> +	.dev_id = "s3c24xx_led.2",

> +	.table = {

> +		GPIO_LOOKUP("GPB", 6, NULL, GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN),

> +		{ },

> +	},

> +};

> +

> +static struct gpiod_lookup_table mini2440_led3_gpio_table = {

> +	.dev_id = "s3c24xx_led.3",

> +	.table = {

> +		GPIO_LOOKUP("GPB", 7, NULL, GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN),

> +		{ },

> +	},

> +};

> +

> +static struct gpiod_lookup_table mini2440_led4_gpio_table = {

> +	.dev_id = "s3c24xx_led.4",

> +	.table = {

> +		GPIO_LOOKUP("GPB", 8, NULL, GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN),

> +		{ },

> +	},

> +};

> +

> +static struct gpiod_lookup_table mini2440_led5_gpio_table = {


Why not backlight for consistency with pdata?

s/led5/led_backlight/

> +	.dev_id = "s3c24xx_led.5",


s/led.5/led_backlight/

> +	.table = {

> +		GPIO_LOOKUP("GPG", 4, NULL, GPIO_ACTIVE_HIGH),

> +		{ },

> +	},

> +};

> +

>   static struct s3c24xx_led_platdata mini2440_led1_pdata = {

>   	.name		= "led1",

> -	.gpio		= S3C2410_GPB(5),

> -	.flags		= S3C24XX_LEDF_ACTLOW | S3C24XX_LEDF_TRISTATE,

>   	.def_trigger	= "heartbeat",

>   };

>   

>   static struct s3c24xx_led_platdata mini2440_led2_pdata = {

>   	.name		= "led2",

> -	.gpio		= S3C2410_GPB(6),

> -	.flags		= S3C24XX_LEDF_ACTLOW | S3C24XX_LEDF_TRISTATE,

>   	.def_trigger	= "nand-disk",

>   };

>   

>   static struct s3c24xx_led_platdata mini2440_led3_pdata = {

>   	.name		= "led3",

> -	.gpio		= S3C2410_GPB(7),

> -	.flags		= S3C24XX_LEDF_ACTLOW | S3C24XX_LEDF_TRISTATE,

>   	.def_trigger	= "mmc0",

>   };

>   

>   static struct s3c24xx_led_platdata mini2440_led4_pdata = {

>   	.name		= "led4",

> -	.gpio		= S3C2410_GPB(8),

> -	.flags		= S3C24XX_LEDF_ACTLOW | S3C24XX_LEDF_TRISTATE,

>   	.def_trigger	= "",

>   };

>   

>   static struct s3c24xx_led_platdata mini2440_led_backlight_pdata = {

>   	.name		= "backlight",

> -	.gpio		= S3C2410_GPG(4),

>   	.def_trigger	= "backlight",

>   };

>   

> @@ -714,6 +745,20 @@ static void __init mini2440_init(void)

>   	i2c_register_board_info(0, mini2440_i2c_devs,

>   				ARRAY_SIZE(mini2440_i2c_devs));

>   

> +	/* Disable pull-up on the LED lines */

> +	s3c_gpio_setpull(S3C2410_GPB(5), S3C_GPIO_PULL_NONE);

> +	s3c_gpio_setpull(S3C2410_GPB(6), S3C_GPIO_PULL_NONE);

> +	s3c_gpio_setpull(S3C2410_GPB(7), S3C_GPIO_PULL_NONE);

> +	s3c_gpio_setpull(S3C2410_GPB(8), S3C_GPIO_PULL_NONE);

> +	s3c_gpio_setpull(S3C2410_GPG(4), S3C_GPIO_PULL_NONE);

> +

> +	/* Add lookups for the lines */

> +	gpiod_add_lookup_table(&mini2440_led1_gpio_table);

> +	gpiod_add_lookup_table(&mini2440_led2_gpio_table);

> +	gpiod_add_lookup_table(&mini2440_led3_gpio_table);

> +	gpiod_add_lookup_table(&mini2440_led4_gpio_table);

> +	gpiod_add_lookup_table(&mini2440_led5_gpio_table);

> +

>   	platform_add_devices(mini2440_devices, ARRAY_SIZE(mini2440_devices));

>   

>   	if (features.count)	/* the optional features */

> diff --git a/arch/arm/mach-s3c24xx/mach-n30.c b/arch/arm/mach-s3c24xx/mach-n30.c

> index d856f23939af..58a64f6d5fd0 100644

> --- a/arch/arm/mach-s3c24xx/mach-n30.c

> +++ b/arch/arm/mach-s3c24xx/mach-n30.c

> @@ -45,6 +45,7 @@

>   

>   #include <plat/cpu.h>

>   #include <plat/devs.h>

> +#include <plat/gpio-cfg.h>

>   #include <linux/platform_data/mmc-s3cmci.h>

>   #include <linux/platform_data/usb-s3c2410_udc.h>

>   #include <plat/samsung-time.h>

> @@ -246,17 +247,33 @@ static struct platform_device n35_button_device = {

>   };

>   

>   /* This is the bluetooth LED on the device. */

> +

> +static struct gpiod_lookup_table n30_blue_led_gpio_table = {

> +	.dev_id = "s3c24xx_led.1",

> +	.table = {

> +		GPIO_LOOKUP("GPG", 6, NULL, GPIO_ACTIVE_HIGH),

> +		{ },

> +	},

> +};

> +

>   static struct s3c24xx_led_platdata n30_blue_led_pdata = {

>   	.name		= "blue_led",

> -	.gpio		= S3C2410_GPG(6),

>   	.def_trigger	= "",

>   };

>   

>   /* This is the blue LED on the device. Originally used to indicate GPS activity

>    * by flashing. */

> +

> +static struct gpiod_lookup_table n35_blue_led_gpio_table = {

> +	.dev_id = "s3c24xx_led.1",

> +	.table = {

> +		GPIO_LOOKUP("GPD", 8, NULL, GPIO_ACTIVE_HIGH),

> +		{ },

> +	},

> +};

> +

>   static struct s3c24xx_led_platdata n35_blue_led_pdata = {

>   	.name		= "blue_led",

> -	.gpio		= S3C2410_GPD(8),

>   	.def_trigger	= "",

>   };

>   

> @@ -264,17 +281,30 @@ static struct s3c24xx_led_platdata n35_blue_led_pdata = {

>    * red, blinking green or solid green when the battery is low,

>    * charging or full respectively.  By driving GPD9 low, it's possible

>    * to force the LED to blink red, so call that warning LED.  */

> +

> +static struct gpiod_lookup_table n30_warning_led_gpio_table = {

> +	.dev_id = "s3c24xx_led.2",

> +	.table = {

> +		GPIO_LOOKUP("GPD", 9, NULL, GPIO_ACTIVE_LOW),

> +		{ },

> +	},

> +};

> +

>   static struct s3c24xx_led_platdata n30_warning_led_pdata = {

>   	.name		= "warning_led",

> -	.flags          = S3C24XX_LEDF_ACTLOW,

> -	.gpio		= S3C2410_GPD(9),

>   	.def_trigger	= "",

>   };

>   

> +static struct gpiod_lookup_table n35_warning_led_gpio_table = {

> +	.dev_id = "s3c24xx_led.2",

> +	.table = {

> +		GPIO_LOOKUP("GPD", 9, NULL, GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN),

> +		{ },

> +	},

> +};

> +

>   static struct s3c24xx_led_platdata n35_warning_led_pdata = {

>   	.name		= "warning_led",

> -	.flags          = S3C24XX_LEDF_ACTLOW | S3C24XX_LEDF_TRISTATE,

> -	.gpio		= S3C2410_GPD(9),

>   	.def_trigger	= "",

>   };

>   

> @@ -577,6 +607,12 @@ static void __init n30_init(void)

>   				      S3C2410_MISCCR_USBSUSPND0 |

>   				      S3C2410_MISCCR_USBSUSPND1, 0x0);

>   

> +		/* Disable pull-up and add GPIO tables */

> +		s3c_gpio_setpull(S3C2410_GPG(6), S3C_GPIO_PULL_NONE);

> +		s3c_gpio_setpull(S3C2410_GPD(9), S3C_GPIO_PULL_NONE);

> +		gpiod_add_lookup_table(&n30_blue_led_gpio_table);

> +		gpiod_add_lookup_table(&n30_warning_led_gpio_table);

> +

>   		platform_add_devices(n30_devices, ARRAY_SIZE(n30_devices));

>   	}

>   

> @@ -594,6 +630,12 @@ static void __init n30_init(void)

>   				      S3C2410_MISCCR_USBSUSPND1,

>   				      S3C2410_MISCCR_USBSUSPND0);

>   

> +		/* Disable pull-up and add GPIO tables */

> +		s3c_gpio_setpull(S3C2410_GPD(8), S3C_GPIO_PULL_NONE);

> +		s3c_gpio_setpull(S3C2410_GPD(9), S3C_GPIO_PULL_NONE);

> +		gpiod_add_lookup_table(&n35_blue_led_gpio_table);

> +		gpiod_add_lookup_table(&n35_warning_led_gpio_table);

> +

>   		platform_add_devices(n35_devices, ARRAY_SIZE(n35_devices));

>   	}

>   }

> diff --git a/arch/arm/mach-s3c24xx/mach-qt2410.c b/arch/arm/mach-s3c24xx/mach-qt2410.c

> index 5d48e5b6e738..ff9e3197309b 100644

> --- a/arch/arm/mach-s3c24xx/mach-qt2410.c

> +++ b/arch/arm/mach-s3c24xx/mach-qt2410.c

> @@ -177,9 +177,15 @@ static struct platform_device qt2410_cs89x0 = {

>   

>   /* LED */

>   

> +static struct gpiod_lookup_table qt2410_led_gpio_table = {

> +	.dev_id = "s3c24xx_led.0",

> +	.table = {

> +		GPIO_LOOKUP("GPB", 0, NULL, GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN),

> +		{ },

> +	},

> +};

> +

>   static struct s3c24xx_led_platdata qt2410_pdata_led = {

> -	.gpio		= S3C2410_GPB(0),

> -	.flags		= S3C24XX_LEDF_ACTLOW | S3C24XX_LEDF_TRISTATE,

>   	.name		= "led",

>   	.def_trigger	= "timer",

>   };

> @@ -338,6 +344,8 @@ static void __init qt2410_machine_init(void)

>   	s3c_i2c0_set_platdata(NULL);

>   

>   	gpiod_add_lookup_table(&qt2410_spi_gpiod_table);

> +	s3c_gpio_setpull(S3C2410_GPB(0), S3C_GPIO_PULL_NONE);

> +	gpiod_add_lookup_table(&qt2410_led_gpio_table);

>   	platform_add_devices(qt2410_devices, ARRAY_SIZE(qt2410_devices));

>   	s3c_pm_init();

>   }

> diff --git a/arch/arm/mach-s3c24xx/mach-vr1000.c b/arch/arm/mach-s3c24xx/mach-vr1000.c

> index 853e74f9b8b5..6a3fb2becc7c 100644

> --- a/arch/arm/mach-s3c24xx/mach-vr1000.c

> +++ b/arch/arm/mach-s3c24xx/mach-vr1000.c

> @@ -13,6 +13,7 @@

>   #include <linux/timer.h>

>   #include <linux/init.h>

>   #include <linux/gpio.h>

> +#include <linux/gpio/machine.h>

>   #include <linux/dm9000.h>

>   #include <linux/i2c.h>

>   

> @@ -40,6 +41,7 @@

>   

>   #include <plat/cpu.h>

>   #include <plat/devs.h>

> +#include <plat/gpio-cfg.h>

>   #include <plat/samsung-time.h>

>   

>   #include "bast.h"

> @@ -223,21 +225,42 @@ static struct platform_device vr1000_dm9k1 = {

>   

>   /* LEDS */

>   

> +static struct gpiod_lookup_table vr1000_led1_gpio_table = {

> +	.dev_id = "s3c24xx_led.1",

> +	.table = {

> +		GPIO_LOOKUP("GPB", 0, NULL, GPIO_ACTIVE_HIGH),

> +		{ },

> +	},

> +};

> +

> +static struct gpiod_lookup_table vr1000_led2_gpio_table = {

> +	.dev_id = "s3c24xx_led.2",

> +	.table = {

> +		GPIO_LOOKUP("GPB", 1, NULL, GPIO_ACTIVE_HIGH),

> +		{ },

> +	},

> +};

> +

> +static struct gpiod_lookup_table vr1000_led3_gpio_table = {

> +	.dev_id = "s3c24xx_led.3",

> +	.table = {

> +		GPIO_LOOKUP("GPB", 2, NULL, GPIO_ACTIVE_HIGH),

> +		{ },

> +	},

> +};

> +

>   static struct s3c24xx_led_platdata vr1000_led1_pdata = {

>   	.name		= "led1",

> -	.gpio		= S3C2410_GPB(0),

>   	.def_trigger	= "",

>   };

>   

>   static struct s3c24xx_led_platdata vr1000_led2_pdata = {

>   	.name		= "led2",

> -	.gpio		= S3C2410_GPB(1),

>   	.def_trigger	= "",

>   };

>   

>   static struct s3c24xx_led_platdata vr1000_led3_pdata = {

>   	.name		= "led3",

> -	.gpio		= S3C2410_GPB(2),

>   	.def_trigger	= "",

>   };

>   

> @@ -317,6 +340,15 @@ static void __init vr1000_init_time(void)

>   static void __init vr1000_init(void)

>   {

>   	s3c_i2c0_set_platdata(NULL);

> +

> +	/* Disable pull-up on LED lines and register GPIO lookups */

> +	s3c_gpio_setpull(S3C2410_GPB(0), S3C_GPIO_PULL_NONE);

> +	s3c_gpio_setpull(S3C2410_GPB(1), S3C_GPIO_PULL_NONE);

> +	s3c_gpio_setpull(S3C2410_GPB(2), S3C_GPIO_PULL_NONE);

> +	gpiod_add_lookup_table(&vr1000_led1_gpio_table);

> +	gpiod_add_lookup_table(&vr1000_led2_gpio_table);

> +	gpiod_add_lookup_table(&vr1000_led3_gpio_table);

> +

>   	platform_add_devices(vr1000_devices, ARRAY_SIZE(vr1000_devices));

>   

>   	i2c_register_board_info(0, vr1000_i2c_devs,

> diff --git a/drivers/leds/leds-s3c24xx.c b/drivers/leds/leds-s3c24xx.c

> index f8b8d6e313ee..3734173596bf 100644

> --- a/drivers/leds/leds-s3c24xx.c

> +++ b/drivers/leds/leds-s3c24xx.c

> @@ -11,19 +11,19 @@

>   #include <linux/kernel.h>

>   #include <linux/platform_device.h>

>   #include <linux/leds.h>

> -#include <linux/gpio.h>

> +#include <linux/gpio/consumer.h>

>   #include <linux/slab.h>

>   #include <linux/module.h>

>   #include <linux/platform_data/leds-s3c24xx.h>

>   

>   #include <mach/regs-gpio.h>

> -#include <plat/gpio-cfg.h>

>   

>   /* our context */

>   

>   struct s3c24xx_gpio_led {

>   	struct led_classdev		 cdev;

>   	struct s3c24xx_led_platdata	*pdata;

> +	struct gpio_desc		*gpiod;

>   };

>   

>   static inline struct s3c24xx_gpio_led *to_gpio(struct led_classdev *led_cdev)

> @@ -35,20 +35,9 @@ static void s3c24xx_led_set(struct led_classdev *led_cdev,

>   			    enum led_brightness value)

>   {

>   	struct s3c24xx_gpio_led *led = to_gpio(led_cdev);

> -	struct s3c24xx_led_platdata *pd = led->pdata;

> -	int state = (value ? 1 : 0) ^ (pd->flags & S3C24XX_LEDF_ACTLOW);

> +	int state = (value ? 1 : 0);


I'd drop this in favour on passing "!!value" directly to gpiod_set_value().

>   

> -	/* there will be a short delay between setting the output and

> -	 * going from output to input when using tristate. */

> -

> -	gpio_set_value(pd->gpio, state);

> -

> -	if (pd->flags & S3C24XX_LEDF_TRISTATE) {

> -		if (value)

> -			gpio_direction_output(pd->gpio, state);

> -		else

> -			gpio_direction_input(pd->gpio);

> -	}

> +	gpiod_set_value(led->gpiod, state);


Like this:

gpiod_set_value(led->gpiod, !!value);

>   }

>   

>   static int s3c24xx_led_probe(struct platform_device *dev)

> @@ -69,22 +58,12 @@ static int s3c24xx_led_probe(struct platform_device *dev)

>   

>   	led->pdata = pdata;

>   

> -	ret = devm_gpio_request(&dev->dev, pdata->gpio, "S3C24XX_LED");

> -	if (ret < 0)

> -		return ret;

> -

> -	/* no point in having a pull-up if we are always driving */

> -

> -	s3c_gpio_setpull(pdata->gpio, S3C_GPIO_PULL_NONE);

> -

> -	if (pdata->flags & S3C24XX_LEDF_TRISTATE)

> -		gpio_direction_input(pdata->gpio);

> -	else

> -		gpio_direction_output(pdata->gpio,

> -			pdata->flags & S3C24XX_LEDF_ACTLOW ? 1 : 0);

> +	/* Default to off */

> +	led->gpiod = devm_gpiod_get(&dev->dev, NULL, GPIOD_OUT_LOW);

> +	if (IS_ERR(led->gpiod))

> +		return PTR_ERR(led->gpiod);

>   

>   	/* register our new led device */

> -

>   	ret = devm_led_classdev_register(&dev->dev, &led->cdev);

>   	if (ret < 0)

>   		dev_err(&dev->dev, "led_classdev_register failed\n");

> diff --git a/include/linux/platform_data/leds-s3c24xx.h b/include/linux/platform_data/leds-s3c24xx.h

> index 5bbae85811e2..64f8d14876e0 100644

> --- a/include/linux/platform_data/leds-s3c24xx.h

> +++ b/include/linux/platform_data/leds-s3c24xx.h

> @@ -10,13 +10,7 @@

>   #ifndef __LEDS_S3C24XX_H

>   #define __LEDS_S3C24XX_H

>   

> -#define S3C24XX_LEDF_ACTLOW	(1<<0)		/* LED is on when GPIO low */

> -#define S3C24XX_LEDF_TRISTATE	(1<<1)		/* tristate to turn off */

> -

>   struct s3c24xx_led_platdata {

> -	unsigned int		 gpio;

> -	unsigned int		 flags;

> -

>   	char			*name;

>   	char			*def_trigger;

>   };

> 


-- 
Best regards,
Jacek Anaszewski
Linus Walleij July 8, 2020, 7:08 a.m. | #3
On Mon, Jun 29, 2020 at 11:49 PM Jacek Anaszewski
<jacek.anaszewski@gmail.com> wrote:

> > +static struct gpiod_lookup_table mini2440_led4_gpio_table = {

> > +     .dev_id = "s3c24xx_led.4",

> > +     .table = {

> > +             GPIO_LOOKUP("GPB", 8, NULL, GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN),

> > +             { },

> > +     },

> > +};

> > +

> > +static struct gpiod_lookup_table mini2440_led5_gpio_table = {

>

> Why not backlight for consistency with pdata?

>

> s/led5/led_backlight/


This platform boardfile is already using a confusion of "backlight"
and "led" for this device. The platform device is named s3c24xx_led
with .id 5 and gets named "s3c24xx_led.5". But I changed the
name of the lookup so it is a bit clearer.

> > +     .dev_id = "s3c24xx_led.5",

>

> s/led.5/led_backlight/


That will not work because this lookup is tied to the platform device
name, not the LED name, which I don't change in this patch.

I fixed the rest of the comments!

Thanks,
Linus Walleij

Patch

diff --git a/arch/arm/mach-s3c24xx/common-smdk.c b/arch/arm/mach-s3c24xx/common-smdk.c
index 58e30cad386c..10fc804c4ec5 100644
--- a/arch/arm/mach-s3c24xx/common-smdk.c
+++ b/arch/arm/mach-s3c24xx/common-smdk.c
@@ -14,6 +14,7 @@ 
 #include <linux/timer.h>
 #include <linux/init.h>
 #include <linux/gpio.h>
+#include <linux/gpio/machine.h>
 #include <linux/device.h>
 #include <linux/platform_device.h>
 
@@ -44,29 +45,53 @@ 
 
 /* LED devices */
 
+static struct gpiod_lookup_table smdk_led4_gpio_table = {
+	.dev_id = "s3c24xx_led.0",
+	.table = {
+		GPIO_LOOKUP("GPF", 4, NULL, GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN),
+		{ },
+	},
+};
+
+static struct gpiod_lookup_table smdk_led5_gpio_table = {
+	.dev_id = "s3c24xx_led.1",
+	.table = {
+		GPIO_LOOKUP("GPF", 6, NULL, GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN),
+		{ },
+	},
+};
+
+static struct gpiod_lookup_table smdk_led6_gpio_table = {
+	.dev_id = "s3c24xx_led.2",
+	.table = {
+		GPIO_LOOKUP("GPF", 5, NULL, GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN),
+		{ },
+	},
+};
+
+static struct gpiod_lookup_table smdk_led7_gpio_table = {
+	.dev_id = "s3c24xx_led.3",
+	.table = {
+		GPIO_LOOKUP("GPF", 7, NULL, GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN),
+		{ },
+	},
+};
+
 static struct s3c24xx_led_platdata smdk_pdata_led4 = {
-	.gpio		= S3C2410_GPF(4),
-	.flags		= S3C24XX_LEDF_ACTLOW | S3C24XX_LEDF_TRISTATE,
 	.name		= "led4",
 	.def_trigger	= "timer",
 };
 
 static struct s3c24xx_led_platdata smdk_pdata_led5 = {
-	.gpio		= S3C2410_GPF(5),
-	.flags		= S3C24XX_LEDF_ACTLOW | S3C24XX_LEDF_TRISTATE,
 	.name		= "led5",
 	.def_trigger	= "nand-disk",
 };
 
 static struct s3c24xx_led_platdata smdk_pdata_led6 = {
-	.gpio		= S3C2410_GPF(6),
-	.flags		= S3C24XX_LEDF_ACTLOW | S3C24XX_LEDF_TRISTATE,
 	.name		= "led6",
 };
 
 static struct s3c24xx_led_platdata smdk_pdata_led7 = {
-	.gpio		= S3C2410_GPF(7),
-	.flags		= S3C24XX_LEDF_ACTLOW | S3C24XX_LEDF_TRISTATE,
 	.name		= "led7",
 };
 
@@ -179,27 +204,25 @@  static struct platform_device __initdata *smdk_devs[] = {
 	&smdk_led7,
 };
 
-static const struct gpio smdk_led_gpios[] = {
-	{ S3C2410_GPF(4), GPIOF_OUT_INIT_HIGH, NULL },
-	{ S3C2410_GPF(5), GPIOF_OUT_INIT_HIGH, NULL },
-	{ S3C2410_GPF(6), GPIOF_OUT_INIT_HIGH, NULL },
-	{ S3C2410_GPF(7), GPIOF_OUT_INIT_HIGH, NULL },
-};
-
 void __init smdk_machine_init(void)
 {
-	/* Configure the LEDs (even if we have no LED support)*/
-
-	int ret = gpio_request_array(smdk_led_gpios,
-				     ARRAY_SIZE(smdk_led_gpios));
-	if (!WARN_ON(ret < 0))
-		gpio_free_array(smdk_led_gpios, ARRAY_SIZE(smdk_led_gpios));
-
 	if (machine_is_smdk2443())
 		smdk_nand_info.twrph0 = 50;
 
 	s3c_nand_set_platdata(&smdk_nand_info);
 
+	/* Disable pull-up on the LED lines */
+	s3c_gpio_setpull(S3C2410_GPF(4), S3C_GPIO_PULL_NONE);
+	s3c_gpio_setpull(S3C2410_GPF(5), S3C_GPIO_PULL_NONE);
+	s3c_gpio_setpull(S3C2410_GPF(6), S3C_GPIO_PULL_NONE);
+	s3c_gpio_setpull(S3C2410_GPF(7), S3C_GPIO_PULL_NONE);
+
+	/* Add lookups for the lines */
+	gpiod_add_lookup_table(&smdk_led4_gpio_table);
+	gpiod_add_lookup_table(&smdk_led5_gpio_table);
+	gpiod_add_lookup_table(&smdk_led6_gpio_table);
+	gpiod_add_lookup_table(&smdk_led7_gpio_table);
+
 	platform_add_devices(smdk_devs, ARRAY_SIZE(smdk_devs));
 
 	s3c_pm_init();
diff --git a/arch/arm/mach-s3c24xx/mach-mini2440.c b/arch/arm/mach-s3c24xx/mach-mini2440.c
index 9035f868fb34..11add63b79c2 100644
--- a/arch/arm/mach-s3c24xx/mach-mini2440.c
+++ b/arch/arm/mach-s3c24xx/mach-mini2440.c
@@ -402,37 +402,68 @@  static struct platform_device mini2440_button_device = {
 
 /* LEDS */
 
+static struct gpiod_lookup_table mini2440_led1_gpio_table = {
+	.dev_id = "s3c24xx_led.1",
+	.table = {
+		GPIO_LOOKUP("GPB", 5, NULL, GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN),
+		{ },
+	},
+};
+
+static struct gpiod_lookup_table mini2440_led2_gpio_table = {
+	.dev_id = "s3c24xx_led.2",
+	.table = {
+		GPIO_LOOKUP("GPB", 6, NULL, GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN),
+		{ },
+	},
+};
+
+static struct gpiod_lookup_table mini2440_led3_gpio_table = {
+	.dev_id = "s3c24xx_led.3",
+	.table = {
+		GPIO_LOOKUP("GPB", 7, NULL, GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN),
+		{ },
+	},
+};
+
+static struct gpiod_lookup_table mini2440_led4_gpio_table = {
+	.dev_id = "s3c24xx_led.4",
+	.table = {
+		GPIO_LOOKUP("GPB", 8, NULL, GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN),
+		{ },
+	},
+};
+
+static struct gpiod_lookup_table mini2440_led5_gpio_table = {
+	.dev_id = "s3c24xx_led.5",
+	.table = {
+		GPIO_LOOKUP("GPG", 4, NULL, GPIO_ACTIVE_HIGH),
+		{ },
+	},
+};
+
 static struct s3c24xx_led_platdata mini2440_led1_pdata = {
 	.name		= "led1",
-	.gpio		= S3C2410_GPB(5),
-	.flags		= S3C24XX_LEDF_ACTLOW | S3C24XX_LEDF_TRISTATE,
 	.def_trigger	= "heartbeat",
 };
 
 static struct s3c24xx_led_platdata mini2440_led2_pdata = {
 	.name		= "led2",
-	.gpio		= S3C2410_GPB(6),
-	.flags		= S3C24XX_LEDF_ACTLOW | S3C24XX_LEDF_TRISTATE,
 	.def_trigger	= "nand-disk",
 };
 
 static struct s3c24xx_led_platdata mini2440_led3_pdata = {
 	.name		= "led3",
-	.gpio		= S3C2410_GPB(7),
-	.flags		= S3C24XX_LEDF_ACTLOW | S3C24XX_LEDF_TRISTATE,
 	.def_trigger	= "mmc0",
 };
 
 static struct s3c24xx_led_platdata mini2440_led4_pdata = {
 	.name		= "led4",
-	.gpio		= S3C2410_GPB(8),
-	.flags		= S3C24XX_LEDF_ACTLOW | S3C24XX_LEDF_TRISTATE,
 	.def_trigger	= "",
 };
 
 static struct s3c24xx_led_platdata mini2440_led_backlight_pdata = {
 	.name		= "backlight",
-	.gpio		= S3C2410_GPG(4),
 	.def_trigger	= "backlight",
 };
 
@@ -714,6 +745,20 @@  static void __init mini2440_init(void)
 	i2c_register_board_info(0, mini2440_i2c_devs,
 				ARRAY_SIZE(mini2440_i2c_devs));
 
+	/* Disable pull-up on the LED lines */
+	s3c_gpio_setpull(S3C2410_GPB(5), S3C_GPIO_PULL_NONE);
+	s3c_gpio_setpull(S3C2410_GPB(6), S3C_GPIO_PULL_NONE);
+	s3c_gpio_setpull(S3C2410_GPB(7), S3C_GPIO_PULL_NONE);
+	s3c_gpio_setpull(S3C2410_GPB(8), S3C_GPIO_PULL_NONE);
+	s3c_gpio_setpull(S3C2410_GPG(4), S3C_GPIO_PULL_NONE);
+
+	/* Add lookups for the lines */
+	gpiod_add_lookup_table(&mini2440_led1_gpio_table);
+	gpiod_add_lookup_table(&mini2440_led2_gpio_table);
+	gpiod_add_lookup_table(&mini2440_led3_gpio_table);
+	gpiod_add_lookup_table(&mini2440_led4_gpio_table);
+	gpiod_add_lookup_table(&mini2440_led5_gpio_table);
+
 	platform_add_devices(mini2440_devices, ARRAY_SIZE(mini2440_devices));
 
 	if (features.count)	/* the optional features */
diff --git a/arch/arm/mach-s3c24xx/mach-n30.c b/arch/arm/mach-s3c24xx/mach-n30.c
index d856f23939af..58a64f6d5fd0 100644
--- a/arch/arm/mach-s3c24xx/mach-n30.c
+++ b/arch/arm/mach-s3c24xx/mach-n30.c
@@ -45,6 +45,7 @@ 
 
 #include <plat/cpu.h>
 #include <plat/devs.h>
+#include <plat/gpio-cfg.h>
 #include <linux/platform_data/mmc-s3cmci.h>
 #include <linux/platform_data/usb-s3c2410_udc.h>
 #include <plat/samsung-time.h>
@@ -246,17 +247,33 @@  static struct platform_device n35_button_device = {
 };
 
 /* This is the bluetooth LED on the device. */
+
+static struct gpiod_lookup_table n30_blue_led_gpio_table = {
+	.dev_id = "s3c24xx_led.1",
+	.table = {
+		GPIO_LOOKUP("GPG", 6, NULL, GPIO_ACTIVE_HIGH),
+		{ },
+	},
+};
+
 static struct s3c24xx_led_platdata n30_blue_led_pdata = {
 	.name		= "blue_led",
-	.gpio		= S3C2410_GPG(6),
 	.def_trigger	= "",
 };
 
 /* This is the blue LED on the device. Originally used to indicate GPS activity
  * by flashing. */
+
+static struct gpiod_lookup_table n35_blue_led_gpio_table = {
+	.dev_id = "s3c24xx_led.1",
+	.table = {
+		GPIO_LOOKUP("GPD", 8, NULL, GPIO_ACTIVE_HIGH),
+		{ },
+	},
+};
+
 static struct s3c24xx_led_platdata n35_blue_led_pdata = {
 	.name		= "blue_led",
-	.gpio		= S3C2410_GPD(8),
 	.def_trigger	= "",
 };
 
@@ -264,17 +281,30 @@  static struct s3c24xx_led_platdata n35_blue_led_pdata = {
  * red, blinking green or solid green when the battery is low,
  * charging or full respectively.  By driving GPD9 low, it's possible
  * to force the LED to blink red, so call that warning LED.  */
+
+static struct gpiod_lookup_table n30_warning_led_gpio_table = {
+	.dev_id = "s3c24xx_led.2",
+	.table = {
+		GPIO_LOOKUP("GPD", 9, NULL, GPIO_ACTIVE_LOW),
+		{ },
+	},
+};
+
 static struct s3c24xx_led_platdata n30_warning_led_pdata = {
 	.name		= "warning_led",
-	.flags          = S3C24XX_LEDF_ACTLOW,
-	.gpio		= S3C2410_GPD(9),
 	.def_trigger	= "",
 };
 
+static struct gpiod_lookup_table n35_warning_led_gpio_table = {
+	.dev_id = "s3c24xx_led.2",
+	.table = {
+		GPIO_LOOKUP("GPD", 9, NULL, GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN),
+		{ },
+	},
+};
+
 static struct s3c24xx_led_platdata n35_warning_led_pdata = {
 	.name		= "warning_led",
-	.flags          = S3C24XX_LEDF_ACTLOW | S3C24XX_LEDF_TRISTATE,
-	.gpio		= S3C2410_GPD(9),
 	.def_trigger	= "",
 };
 
@@ -577,6 +607,12 @@  static void __init n30_init(void)
 				      S3C2410_MISCCR_USBSUSPND0 |
 				      S3C2410_MISCCR_USBSUSPND1, 0x0);
 
+		/* Disable pull-up and add GPIO tables */
+		s3c_gpio_setpull(S3C2410_GPG(6), S3C_GPIO_PULL_NONE);
+		s3c_gpio_setpull(S3C2410_GPD(9), S3C_GPIO_PULL_NONE);
+		gpiod_add_lookup_table(&n30_blue_led_gpio_table);
+		gpiod_add_lookup_table(&n30_warning_led_gpio_table);
+
 		platform_add_devices(n30_devices, ARRAY_SIZE(n30_devices));
 	}
 
@@ -594,6 +630,12 @@  static void __init n30_init(void)
 				      S3C2410_MISCCR_USBSUSPND1,
 				      S3C2410_MISCCR_USBSUSPND0);
 
+		/* Disable pull-up and add GPIO tables */
+		s3c_gpio_setpull(S3C2410_GPD(8), S3C_GPIO_PULL_NONE);
+		s3c_gpio_setpull(S3C2410_GPD(9), S3C_GPIO_PULL_NONE);
+		gpiod_add_lookup_table(&n35_blue_led_gpio_table);
+		gpiod_add_lookup_table(&n35_warning_led_gpio_table);
+
 		platform_add_devices(n35_devices, ARRAY_SIZE(n35_devices));
 	}
 }
diff --git a/arch/arm/mach-s3c24xx/mach-qt2410.c b/arch/arm/mach-s3c24xx/mach-qt2410.c
index 5d48e5b6e738..ff9e3197309b 100644
--- a/arch/arm/mach-s3c24xx/mach-qt2410.c
+++ b/arch/arm/mach-s3c24xx/mach-qt2410.c
@@ -177,9 +177,15 @@  static struct platform_device qt2410_cs89x0 = {
 
 /* LED */
 
+static struct gpiod_lookup_table qt2410_led_gpio_table = {
+	.dev_id = "s3c24xx_led.0",
+	.table = {
+		GPIO_LOOKUP("GPB", 0, NULL, GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN),
+		{ },
+	},
+};
+
 static struct s3c24xx_led_platdata qt2410_pdata_led = {
-	.gpio		= S3C2410_GPB(0),
-	.flags		= S3C24XX_LEDF_ACTLOW | S3C24XX_LEDF_TRISTATE,
 	.name		= "led",
 	.def_trigger	= "timer",
 };
@@ -338,6 +344,8 @@  static void __init qt2410_machine_init(void)
 	s3c_i2c0_set_platdata(NULL);
 
 	gpiod_add_lookup_table(&qt2410_spi_gpiod_table);
+	s3c_gpio_setpull(S3C2410_GPB(0), S3C_GPIO_PULL_NONE);
+	gpiod_add_lookup_table(&qt2410_led_gpio_table);
 	platform_add_devices(qt2410_devices, ARRAY_SIZE(qt2410_devices));
 	s3c_pm_init();
 }
diff --git a/arch/arm/mach-s3c24xx/mach-vr1000.c b/arch/arm/mach-s3c24xx/mach-vr1000.c
index 853e74f9b8b5..6a3fb2becc7c 100644
--- a/arch/arm/mach-s3c24xx/mach-vr1000.c
+++ b/arch/arm/mach-s3c24xx/mach-vr1000.c
@@ -13,6 +13,7 @@ 
 #include <linux/timer.h>
 #include <linux/init.h>
 #include <linux/gpio.h>
+#include <linux/gpio/machine.h>
 #include <linux/dm9000.h>
 #include <linux/i2c.h>
 
@@ -40,6 +41,7 @@ 
 
 #include <plat/cpu.h>
 #include <plat/devs.h>
+#include <plat/gpio-cfg.h>
 #include <plat/samsung-time.h>
 
 #include "bast.h"
@@ -223,21 +225,42 @@  static struct platform_device vr1000_dm9k1 = {
 
 /* LEDS */
 
+static struct gpiod_lookup_table vr1000_led1_gpio_table = {
+	.dev_id = "s3c24xx_led.1",
+	.table = {
+		GPIO_LOOKUP("GPB", 0, NULL, GPIO_ACTIVE_HIGH),
+		{ },
+	},
+};
+
+static struct gpiod_lookup_table vr1000_led2_gpio_table = {
+	.dev_id = "s3c24xx_led.2",
+	.table = {
+		GPIO_LOOKUP("GPB", 1, NULL, GPIO_ACTIVE_HIGH),
+		{ },
+	},
+};
+
+static struct gpiod_lookup_table vr1000_led3_gpio_table = {
+	.dev_id = "s3c24xx_led.3",
+	.table = {
+		GPIO_LOOKUP("GPB", 2, NULL, GPIO_ACTIVE_HIGH),
+		{ },
+	},
+};
+
 static struct s3c24xx_led_platdata vr1000_led1_pdata = {
 	.name		= "led1",
-	.gpio		= S3C2410_GPB(0),
 	.def_trigger	= "",
 };
 
 static struct s3c24xx_led_platdata vr1000_led2_pdata = {
 	.name		= "led2",
-	.gpio		= S3C2410_GPB(1),
 	.def_trigger	= "",
 };
 
 static struct s3c24xx_led_platdata vr1000_led3_pdata = {
 	.name		= "led3",
-	.gpio		= S3C2410_GPB(2),
 	.def_trigger	= "",
 };
 
@@ -317,6 +340,15 @@  static void __init vr1000_init_time(void)
 static void __init vr1000_init(void)
 {
 	s3c_i2c0_set_platdata(NULL);
+
+	/* Disable pull-up on LED lines and register GPIO lookups */
+	s3c_gpio_setpull(S3C2410_GPB(0), S3C_GPIO_PULL_NONE);
+	s3c_gpio_setpull(S3C2410_GPB(1), S3C_GPIO_PULL_NONE);
+	s3c_gpio_setpull(S3C2410_GPB(2), S3C_GPIO_PULL_NONE);
+	gpiod_add_lookup_table(&vr1000_led1_gpio_table);
+	gpiod_add_lookup_table(&vr1000_led2_gpio_table);
+	gpiod_add_lookup_table(&vr1000_led3_gpio_table);
+
 	platform_add_devices(vr1000_devices, ARRAY_SIZE(vr1000_devices));
 
 	i2c_register_board_info(0, vr1000_i2c_devs,
diff --git a/drivers/leds/leds-s3c24xx.c b/drivers/leds/leds-s3c24xx.c
index f8b8d6e313ee..3734173596bf 100644
--- a/drivers/leds/leds-s3c24xx.c
+++ b/drivers/leds/leds-s3c24xx.c
@@ -11,19 +11,19 @@ 
 #include <linux/kernel.h>
 #include <linux/platform_device.h>
 #include <linux/leds.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/slab.h>
 #include <linux/module.h>
 #include <linux/platform_data/leds-s3c24xx.h>
 
 #include <mach/regs-gpio.h>
-#include <plat/gpio-cfg.h>
 
 /* our context */
 
 struct s3c24xx_gpio_led {
 	struct led_classdev		 cdev;
 	struct s3c24xx_led_platdata	*pdata;
+	struct gpio_desc		*gpiod;
 };
 
 static inline struct s3c24xx_gpio_led *to_gpio(struct led_classdev *led_cdev)
@@ -35,20 +35,9 @@  static void s3c24xx_led_set(struct led_classdev *led_cdev,
 			    enum led_brightness value)
 {
 	struct s3c24xx_gpio_led *led = to_gpio(led_cdev);
-	struct s3c24xx_led_platdata *pd = led->pdata;
-	int state = (value ? 1 : 0) ^ (pd->flags & S3C24XX_LEDF_ACTLOW);
+	int state = (value ? 1 : 0);
 
-	/* there will be a short delay between setting the output and
-	 * going from output to input when using tristate. */
-
-	gpio_set_value(pd->gpio, state);
-
-	if (pd->flags & S3C24XX_LEDF_TRISTATE) {
-		if (value)
-			gpio_direction_output(pd->gpio, state);
-		else
-			gpio_direction_input(pd->gpio);
-	}
+	gpiod_set_value(led->gpiod, state);
 }
 
 static int s3c24xx_led_probe(struct platform_device *dev)
@@ -69,22 +58,12 @@  static int s3c24xx_led_probe(struct platform_device *dev)
 
 	led->pdata = pdata;
 
-	ret = devm_gpio_request(&dev->dev, pdata->gpio, "S3C24XX_LED");
-	if (ret < 0)
-		return ret;
-
-	/* no point in having a pull-up if we are always driving */
-
-	s3c_gpio_setpull(pdata->gpio, S3C_GPIO_PULL_NONE);
-
-	if (pdata->flags & S3C24XX_LEDF_TRISTATE)
-		gpio_direction_input(pdata->gpio);
-	else
-		gpio_direction_output(pdata->gpio,
-			pdata->flags & S3C24XX_LEDF_ACTLOW ? 1 : 0);
+	/* Default to off */
+	led->gpiod = devm_gpiod_get(&dev->dev, NULL, GPIOD_OUT_LOW);
+	if (IS_ERR(led->gpiod))
+		return PTR_ERR(led->gpiod);
 
 	/* register our new led device */
-
 	ret = devm_led_classdev_register(&dev->dev, &led->cdev);
 	if (ret < 0)
 		dev_err(&dev->dev, "led_classdev_register failed\n");
diff --git a/include/linux/platform_data/leds-s3c24xx.h b/include/linux/platform_data/leds-s3c24xx.h
index 5bbae85811e2..64f8d14876e0 100644
--- a/include/linux/platform_data/leds-s3c24xx.h
+++ b/include/linux/platform_data/leds-s3c24xx.h
@@ -10,13 +10,7 @@ 
 #ifndef __LEDS_S3C24XX_H
 #define __LEDS_S3C24XX_H
 
-#define S3C24XX_LEDF_ACTLOW	(1<<0)		/* LED is on when GPIO low */
-#define S3C24XX_LEDF_TRISTATE	(1<<1)		/* tristate to turn off */
-
 struct s3c24xx_led_platdata {
-	unsigned int		 gpio;
-	unsigned int		 flags;
-
 	char			*name;
 	char			*def_trigger;
 };