diff mbox series

[2/2,v1] backlight: Add Kinetic KTD253 backlight driver

Message ID 20200720203506.3883129-2-linus.walleij@linaro.org
State New
Headers show
Series None | expand

Commit Message

Linus Walleij July 20, 2020, 8:35 p.m. UTC
The Kinetic KTD253 backlight driver is controlled with a
single GPIO line, but still supports a range of brightness
settings by sending fast pulses on the line.

This is based off the source code release for the Samsung
GT-S7710 mobile phone.

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

---
 MAINTAINERS                                |   6 +
 drivers/video/backlight/Kconfig            |   8 +
 drivers/video/backlight/Makefile           |   1 +
 drivers/video/backlight/ktd253-backlight.c | 254 +++++++++++++++++++++
 4 files changed, 269 insertions(+)
 create mode 100644 drivers/video/backlight/ktd253-backlight.c

-- 
2.26.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Comments

Sam Ravnborg July 21, 2020, 8:49 a.m. UTC | #1
Hi Linus.

On Mon, Jul 20, 2020 at 10:35:06PM +0200, Linus Walleij wrote:
> The Kinetic KTD253 backlight driver is controlled with a

> single GPIO line, but still supports a range of brightness

> settings by sending fast pulses on the line.

> 

> This is based off the source code release for the Samsung

> GT-S7710 mobile phone.

> 

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


In -next there is a few updated to backlight stuff that this driver
could benefit from.
My comments in the following assumes you have the latest -next.

	Sam

> ---

>  MAINTAINERS                                |   6 +

>  drivers/video/backlight/Kconfig            |   8 +

>  drivers/video/backlight/Makefile           |   1 +

>  drivers/video/backlight/ktd253-backlight.c | 254 +++++++++++++++++++++

>  4 files changed, 269 insertions(+)

>  create mode 100644 drivers/video/backlight/ktd253-backlight.c

> 

> diff --git a/MAINTAINERS b/MAINTAINERS

> index b4a43a9e7fbc..ea6fcc5bb79e 100644

> --- a/MAINTAINERS

> +++ b/MAINTAINERS

> @@ -9610,6 +9610,12 @@ F:	Documentation/admin-guide/auxdisplay/ks0108.rst

>  F:	drivers/auxdisplay/ks0108.c

>  F:	include/linux/ks0108.h

>  

> +KTD253 BACKLIGHT DRIVER

> +M:	Linus Walleij <linus.walleij@linaro.org>

> +S:	Maintained

> +F:	Documentation/devicetree/bindings/leds/backlight/kinetic,ktd253.yaml

> +F:	drivers/video/backlight/ktd253-backlight.c

> +

>  L3MDEV

>  M:	David Ahern <dsahern@kernel.org>

>  L:	netdev@vger.kernel.org

> diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig

> index 7d22d7377606..6a74c60707b4 100644

> --- a/drivers/video/backlight/Kconfig

> +++ b/drivers/video/backlight/Kconfig

> @@ -190,6 +190,14 @@ config BACKLIGHT_IPAQ_MICRO

>  	  computers. Say yes if you have one of the h3100/h3600/h3700

>  	  machines.

>  

> +config BACKLIGHT_KTD253

> +	tristate "Backlight Driver for Kinetic KTD253"

> +	depends on GPIOLIB || COMPILE_TEST

> +	help

> +	  Say y to enabled the backlight driver for the Kinetic KTD253

> +	  which is a 1-wire GPIO-controlled backlight found in some mobile

> +	  phones.

> +

>  config BACKLIGHT_LM3533

>  	tristate "Backlight Driver for LM3533"

>  	depends on MFD_LM3533

> diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile

> index 0c1a1524627a..d50cd12574ae 100644

> --- a/drivers/video/backlight/Makefile

> +++ b/drivers/video/backlight/Makefile

> @@ -36,6 +36,7 @@ obj-$(CONFIG_BACKLIGHT_GPIO)		+= gpio_backlight.o

>  obj-$(CONFIG_BACKLIGHT_HP680)		+= hp680_bl.o

>  obj-$(CONFIG_BACKLIGHT_HP700)		+= jornada720_bl.o

>  obj-$(CONFIG_BACKLIGHT_IPAQ_MICRO)	+= ipaq_micro_bl.o

> +obj-$(CONFIG_BACKLIGHT_KTD253)		+= ktd253-backlight.o

>  obj-$(CONFIG_BACKLIGHT_LM3533)		+= lm3533_bl.o

>  obj-$(CONFIG_BACKLIGHT_LM3630A)		+= lm3630a_bl.o

>  obj-$(CONFIG_BACKLIGHT_LM3639)		+= lm3639_bl.o

> diff --git a/drivers/video/backlight/ktd253-backlight.c b/drivers/video/backlight/ktd253-backlight.c

> new file mode 100644

> index 000000000000..d460d1fef329

> --- /dev/null

> +++ b/drivers/video/backlight/ktd253-backlight.c

> @@ -0,0 +1,254 @@

> +// SPDX-License-Identifier: GPL-2.0-only

> +/*

> + * Backlight driver for the Kinetic KTD253

> + * Based on code and know-how from the Samsung GT-S7710

> + * Gareth Phillips <gareth.phillips@samsung.com>

> + */

> +#include <linux/backlight.h>

> +#include <linux/err.h>

> +#include <linux/fb.h>

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

> +#include <linux/init.h>

> +#include <linux/kernel.h>

> +#include <linux/module.h>

> +#include <linux/of.h>

> +#include <linux/platform_device.h>

> +#include <linux/property.h>

> +#include <linux/slab.h>

> +#include <linux/delay.h>

> +#include <linux/limits.h>


In drm land these needs to be sorted.
I do not think backlight demands it.

> +

> +/* Current ratio is n/32 from 1/32 to 32/32 */

> +#define KTD253_MIN_RATIO 1

> +#define KTD253_MAX_RATIO 32

> +#define KTD253_DEFAULT_RATIO 13

> +

> +/* With the table we use this is 24/32 current ratio actually */

> +#define KTD253_MAX_BRIGHTNESS 255

> +#define KTD253_DEFAULT_BRIGHTNESS 160

> +

> +#define KTD253_T_LOW_NS (200 + 10) /* Additional 10ns as safety factor */

> +#define KTD253_T_HIGH_NS (200 + 10) /* Additional 10ns as safety factor */

> +#define KTD253_T_OFF_MS 3

> +

> +struct ktd253_backlight {

> +	struct device *dev;

> +	struct gpio_desc *gpiod;

> +	u16 ratio;


> +	unsigned int brightness;

brightness is not used - delete.
> +};


I had expected to see a backlight pointer in the above structure.
Like we do in most drivers.

> +

> +/*

> + * The following table is used to convert brightness level to the LED

> + * Current Ratio expressed as (full current) /(n * 32).

> + * i.e. 1 = 1/32 full current. Zero indicates LED is powered off.

> + * The table is intended to allow the brightness level to be "tuned"

> + * to compensate for non-linearity of brightness relative to current.

> + */

> +static const u16 ktd253_brightness_to_current_ratio[] = {

> +	0,      /* (0/32) KTD253_BACKLIGHT_OFF */

> +	39,     /* (1/32) KTD253_MIN_RATIO */

> +	58,     /* (2/32) */

> +	67,     /* (3/32) */

> +	76,     /* (4/32) */

> +	85,     /* (5/32) */

> +	94,     /* (6/32) */

> +	104,    /* (7/32) */

> +	113,    /* (8/32) */

> +	122,    /* (9/32) */

> +	131,    /* (10/32) */

> +	145,    /* (11/32) */

> +	159,    /* (12/32) */

> +	169,    /* (13/32) */

> +	179,    /* (14/32) */

> +	189,    /* (15/32) */

> +	196,    /* (16/32) */

> +	203,    /* (17/32) */

> +	210,    /* (18/32) */

> +	217,    /* (19/32) */

> +	224,    /* (20/32) */

> +	231,    /* (21/32) */

> +	238,    /* (22/32) */

> +	245,    /* (23/32) */

> +	255,    /* (24/32) */

> +	300,    /* (25/32) */

> +	300,    /* (26/32) */

> +	300,    /* (27/32) */

> +	300,    /* (28/32) */

> +	300,    /* (29/32) */

> +	300,    /* (30/32) */

> +	300,    /* (31/32) */

> +	300     /* (32/32) KTD253_MAX_RATIO */

> +

> +};

> +

> +/* Inspired by gpio_bl.c */

> +static int ktd253_backlight_get_next_brightness(struct backlight_device *bl)

> +{

> +	int brightness = bl->props.brightness;

> +

> +	if (bl->props.power != FB_BLANK_UNBLANK ||

> +	    bl->props.fb_blank != FB_BLANK_UNBLANK ||

> +	    bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))

> +		brightness = 0;

> +

> +	return brightness;

> +}

> +

> +static int ktd253_backlight_update_status(struct backlight_device *bl)

> +{

> +	struct ktd253_backlight *ktd253 = bl_get_data(bl);

> +	int brightness = ktd253_backlight_get_next_brightness(bl);

Use backligt_get_brightness() to get the brightness.
Then you can delete ktd253_backlight_get_next_brightness()

> +	u16 target_ratio;

> +	u16 current_ratio = ktd253->ratio;

> +	unsigned long flags;

> +

> +	dev_dbg(ktd253->dev, "new brightness: %d\n", brightness);

> +

> +	/* Look up the current ratio */

> +	for (target_ratio = KTD253_MAX_RATIO; target_ratio > 0; target_ratio--) {

> +		if (brightness > ktd253_brightness_to_current_ratio[target_ratio - 1])

> +			break;

> +	}

> +

> +	dev_dbg(ktd253->dev, "new ratio: %d/32\n", target_ratio);

Maybe only one print with both brightness and ratio?

> +

> +	if (target_ratio == current_ratio)

> +		/* This is already right */

> +		return 0;

> +

> +	if (target_ratio == 0) {

> +		gpiod_set_value_cansleep(ktd253->gpiod, 0);

> +		/*

> +		 * We need to keep the GPIO low for at least this long

> +		 * to actually switch the KTD253 off.

> +		 */

> +		msleep(KTD253_T_OFF_MS);

> +		ktd253->ratio = 0;

> +		return 0;

> +	}

> +

> +	if (current_ratio == 0) {

> +		gpiod_set_value_cansleep(ktd253->gpiod, 1);

> +		ndelay(KTD253_T_HIGH_NS);

> +		/* We always fall back to this when we power on */

> +		current_ratio = KTD253_MAX_RATIO;

> +	}

> +

> +	/*

> +	 * WARNING:

> +	 * The loop to set the correct current level is performed

> +	 * with interrupts disabled as it is timing critical.

> +	 * The maximum number of cycles of the loop is 32

> +	 * so the time taken will be (T_LOW_NS + T_HIGH_NS + loop_time) * 32,

> +	 */

> +	local_irq_save(flags);

> +	while (current_ratio != target_ratio) {

> +		/*

> +		 * These GPIO operations absolutely can NOT sleep so no

> +		 * _cansleep suffixes, and no using GPIO expanders on

> +		 * slow buses for this!

> +		 */

> +		gpiod_set_value(ktd253->gpiod, 0);

> +		ndelay(KTD253_T_LOW_NS);

> +		gpiod_set_value(ktd253->gpiod, 1);

> +		ndelay(KTD253_T_HIGH_NS);

> +		/* After 1/32 we loop back to 32/32 */

> +		if (current_ratio == KTD253_MIN_RATIO)

> +			current_ratio = KTD253_MAX_RATIO;

> +		else

> +			current_ratio--;

> +	}

> +	local_irq_restore(flags);

> +	ktd253->ratio = current_ratio;

> +

> +	dev_dbg(ktd253->dev, "new ratio set to %d/32\n", target_ratio);

> +

> +	return 0;

> +}

> +

> +static const struct backlight_ops ktd253_backlight_ops = {

> +	.options	= BL_CORE_SUSPENDRESUME,

> +	.update_status	= ktd253_backlight_update_status,

> +};

> +

> +static int ktd253_backlight_probe(struct platform_device *pdev)

> +{

> +	struct device *dev = &pdev->dev;

> +	struct backlight_device *bl;

> +	struct ktd253_backlight *ktd253;

> +	u32 max_brightness;

> +	u32 brightness;

> +	int ret;

> +

> +	ktd253 = devm_kzalloc(dev, sizeof(*ktd253), GFP_KERNEL);

> +	if (!ktd253)

> +		return -ENOMEM;

> +	ktd253->dev = dev;

> +

> +	ret = device_property_read_u32(dev, "max-brightness", &max_brightness);

> +	if (ret)

> +		max_brightness = KTD253_MAX_BRIGHTNESS;

> +

> +	ret = device_property_read_u32(dev, "default-brightness", &brightness);

> +	if (ret)

> +		brightness = KTD253_DEFAULT_BRIGHTNESS;

> +

> +	if (brightness)

> +		/* This will be the default ratio when the KTD253 is enabled */

> +		ktd253->ratio = KTD253_MAX_RATIO;

> +	else

> +		ktd253->ratio = 0;

> +

> +	ktd253->gpiod = devm_gpiod_get(dev, NULL,

> +				       brightness ? GPIOD_OUT_HIGH :

> +				       GPIOD_OUT_LOW);

> +	if (IS_ERR(ktd253->gpiod)) {

> +		ret = PTR_ERR(ktd253->gpiod);

> +		if (ret != -EPROBE_DEFER)

> +			dev_err(dev, "gpio line missing or invalid.\n");

> +		return ret;

> +	}

> +	gpiod_set_consumer_name(ktd253->gpiod, dev_name(dev));

> +

> +	bl = devm_backlight_device_register(dev, dev_name(dev), dev, ktd253,

> +					    &ktd253_backlight_ops, NULL);

> +	if (IS_ERR(bl)) {

> +		dev_err(dev, "failed to register backlight\n");

> +		return PTR_ERR(bl);

> +	}

> +	bl->props.max_brightness = max_brightness;

> +	/* When we just enable the GPIO line we set max brightness */

> +	if (brightness) {

> +		bl->props.brightness = brightness;

> +		bl->props.power = FB_BLANK_UNBLANK;

> +		ktd253_backlight_update_status(bl);

> +	} else {

> +		bl->props.brightness = 0;

> +		bl->props.power = FB_BLANK_POWERDOWN;

> +	}

Pass a backlight_properties to devm_backlight_device_register.
So this is correct at init time.

FB_BLANK_* are constant used by the fb_blank icotl - and should not be
used here.
Do not assign props.power - as there is no change in power state to
report.

In other words:
Init backlight_properties with:
- max_brightness
- brightness
- Type (RAW)
Call devm_backlight_device_register()

Then unconditionally call backlight_update_status()
(Not the local variant, go via backlight core)

The above is my understandig - but let the backlight people chime in
too.

I would love if we could make it simpler to register a backlight
device...

	Sam

> +

> +	platform_set_drvdata(pdev, bl);

> +

> +	return 0;

> +}

> +

> +static const struct of_device_id ktd253_backlight_of_match[] = {

> +	{ .compatible = "kinetic,ktd253" },

> +	{ /* sentinel */ }

> +};

> +MODULE_DEVICE_TABLE(of, ktd253_backlight_of_match);

> +

> +static struct platform_driver ktd253_backlight_driver = {

> +	.driver = {

> +		.name = "ktd253-backlight",

> +		.of_match_table = ktd253_backlight_of_match,

> +	},

> +	.probe		= ktd253_backlight_probe,

> +};

> +module_platform_driver(ktd253_backlight_driver);

> +

> +MODULE_AUTHOR("Linus Walleij <linus.walleij@linaro.org>");

> +MODULE_DESCRIPTION("Kinetic KTD253 Backlight Driver");

> +MODULE_LICENSE("GPL");

> +MODULE_ALIAS("platform:ktd253-backlight");

> -- 

> 2.26.2

> 

> _______________________________________________

> dri-devel mailing list

> dri-devel@lists.freedesktop.org

> https://lists.freedesktop.org/mailman/listinfo/dri-devel

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index b4a43a9e7fbc..ea6fcc5bb79e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9610,6 +9610,12 @@  F:	Documentation/admin-guide/auxdisplay/ks0108.rst
 F:	drivers/auxdisplay/ks0108.c
 F:	include/linux/ks0108.h
 
+KTD253 BACKLIGHT DRIVER
+M:	Linus Walleij <linus.walleij@linaro.org>
+S:	Maintained
+F:	Documentation/devicetree/bindings/leds/backlight/kinetic,ktd253.yaml
+F:	drivers/video/backlight/ktd253-backlight.c
+
 L3MDEV
 M:	David Ahern <dsahern@kernel.org>
 L:	netdev@vger.kernel.org
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index 7d22d7377606..6a74c60707b4 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -190,6 +190,14 @@  config BACKLIGHT_IPAQ_MICRO
 	  computers. Say yes if you have one of the h3100/h3600/h3700
 	  machines.
 
+config BACKLIGHT_KTD253
+	tristate "Backlight Driver for Kinetic KTD253"
+	depends on GPIOLIB || COMPILE_TEST
+	help
+	  Say y to enabled the backlight driver for the Kinetic KTD253
+	  which is a 1-wire GPIO-controlled backlight found in some mobile
+	  phones.
+
 config BACKLIGHT_LM3533
 	tristate "Backlight Driver for LM3533"
 	depends on MFD_LM3533
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index 0c1a1524627a..d50cd12574ae 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -36,6 +36,7 @@  obj-$(CONFIG_BACKLIGHT_GPIO)		+= gpio_backlight.o
 obj-$(CONFIG_BACKLIGHT_HP680)		+= hp680_bl.o
 obj-$(CONFIG_BACKLIGHT_HP700)		+= jornada720_bl.o
 obj-$(CONFIG_BACKLIGHT_IPAQ_MICRO)	+= ipaq_micro_bl.o
+obj-$(CONFIG_BACKLIGHT_KTD253)		+= ktd253-backlight.o
 obj-$(CONFIG_BACKLIGHT_LM3533)		+= lm3533_bl.o
 obj-$(CONFIG_BACKLIGHT_LM3630A)		+= lm3630a_bl.o
 obj-$(CONFIG_BACKLIGHT_LM3639)		+= lm3639_bl.o
diff --git a/drivers/video/backlight/ktd253-backlight.c b/drivers/video/backlight/ktd253-backlight.c
new file mode 100644
index 000000000000..d460d1fef329
--- /dev/null
+++ b/drivers/video/backlight/ktd253-backlight.c
@@ -0,0 +1,254 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Backlight driver for the Kinetic KTD253
+ * Based on code and know-how from the Samsung GT-S7710
+ * Gareth Phillips <gareth.phillips@samsung.com>
+ */
+#include <linux/backlight.h>
+#include <linux/err.h>
+#include <linux/fb.h>
+#include <linux/gpio/consumer.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
+#include <linux/limits.h>
+
+/* Current ratio is n/32 from 1/32 to 32/32 */
+#define KTD253_MIN_RATIO 1
+#define KTD253_MAX_RATIO 32
+#define KTD253_DEFAULT_RATIO 13
+
+/* With the table we use this is 24/32 current ratio actually */
+#define KTD253_MAX_BRIGHTNESS 255
+#define KTD253_DEFAULT_BRIGHTNESS 160
+
+#define KTD253_T_LOW_NS (200 + 10) /* Additional 10ns as safety factor */
+#define KTD253_T_HIGH_NS (200 + 10) /* Additional 10ns as safety factor */
+#define KTD253_T_OFF_MS 3
+
+struct ktd253_backlight {
+	struct device *dev;
+	struct gpio_desc *gpiod;
+	u16 ratio;
+	unsigned int brightness;
+};
+
+/*
+ * The following table is used to convert brightness level to the LED
+ * Current Ratio expressed as (full current) /(n * 32).
+ * i.e. 1 = 1/32 full current. Zero indicates LED is powered off.
+ * The table is intended to allow the brightness level to be "tuned"
+ * to compensate for non-linearity of brightness relative to current.
+ */
+static const u16 ktd253_brightness_to_current_ratio[] = {
+	0,      /* (0/32) KTD253_BACKLIGHT_OFF */
+	39,     /* (1/32) KTD253_MIN_RATIO */
+	58,     /* (2/32) */
+	67,     /* (3/32) */
+	76,     /* (4/32) */
+	85,     /* (5/32) */
+	94,     /* (6/32) */
+	104,    /* (7/32) */
+	113,    /* (8/32) */
+	122,    /* (9/32) */
+	131,    /* (10/32) */
+	145,    /* (11/32) */
+	159,    /* (12/32) */
+	169,    /* (13/32) */
+	179,    /* (14/32) */
+	189,    /* (15/32) */
+	196,    /* (16/32) */
+	203,    /* (17/32) */
+	210,    /* (18/32) */
+	217,    /* (19/32) */
+	224,    /* (20/32) */
+	231,    /* (21/32) */
+	238,    /* (22/32) */
+	245,    /* (23/32) */
+	255,    /* (24/32) */
+	300,    /* (25/32) */
+	300,    /* (26/32) */
+	300,    /* (27/32) */
+	300,    /* (28/32) */
+	300,    /* (29/32) */
+	300,    /* (30/32) */
+	300,    /* (31/32) */
+	300     /* (32/32) KTD253_MAX_RATIO */
+
+};
+
+/* Inspired by gpio_bl.c */
+static int ktd253_backlight_get_next_brightness(struct backlight_device *bl)
+{
+	int brightness = bl->props.brightness;
+
+	if (bl->props.power != FB_BLANK_UNBLANK ||
+	    bl->props.fb_blank != FB_BLANK_UNBLANK ||
+	    bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
+		brightness = 0;
+
+	return brightness;
+}
+
+static int ktd253_backlight_update_status(struct backlight_device *bl)
+{
+	struct ktd253_backlight *ktd253 = bl_get_data(bl);
+	int brightness = ktd253_backlight_get_next_brightness(bl);
+	u16 target_ratio;
+	u16 current_ratio = ktd253->ratio;
+	unsigned long flags;
+
+	dev_dbg(ktd253->dev, "new brightness: %d\n", brightness);
+
+	/* Look up the current ratio */
+	for (target_ratio = KTD253_MAX_RATIO; target_ratio > 0; target_ratio--) {
+		if (brightness > ktd253_brightness_to_current_ratio[target_ratio - 1])
+			break;
+	}
+
+	dev_dbg(ktd253->dev, "new ratio: %d/32\n", target_ratio);
+
+	if (target_ratio == current_ratio)
+		/* This is already right */
+		return 0;
+
+	if (target_ratio == 0) {
+		gpiod_set_value_cansleep(ktd253->gpiod, 0);
+		/*
+		 * We need to keep the GPIO low for at least this long
+		 * to actually switch the KTD253 off.
+		 */
+		msleep(KTD253_T_OFF_MS);
+		ktd253->ratio = 0;
+		return 0;
+	}
+
+	if (current_ratio == 0) {
+		gpiod_set_value_cansleep(ktd253->gpiod, 1);
+		ndelay(KTD253_T_HIGH_NS);
+		/* We always fall back to this when we power on */
+		current_ratio = KTD253_MAX_RATIO;
+	}
+
+	/*
+	 * WARNING:
+	 * The loop to set the correct current level is performed
+	 * with interrupts disabled as it is timing critical.
+	 * The maximum number of cycles of the loop is 32
+	 * so the time taken will be (T_LOW_NS + T_HIGH_NS + loop_time) * 32,
+	 */
+	local_irq_save(flags);
+	while (current_ratio != target_ratio) {
+		/*
+		 * These GPIO operations absolutely can NOT sleep so no
+		 * _cansleep suffixes, and no using GPIO expanders on
+		 * slow buses for this!
+		 */
+		gpiod_set_value(ktd253->gpiod, 0);
+		ndelay(KTD253_T_LOW_NS);
+		gpiod_set_value(ktd253->gpiod, 1);
+		ndelay(KTD253_T_HIGH_NS);
+		/* After 1/32 we loop back to 32/32 */
+		if (current_ratio == KTD253_MIN_RATIO)
+			current_ratio = KTD253_MAX_RATIO;
+		else
+			current_ratio--;
+	}
+	local_irq_restore(flags);
+	ktd253->ratio = current_ratio;
+
+	dev_dbg(ktd253->dev, "new ratio set to %d/32\n", target_ratio);
+
+	return 0;
+}
+
+static const struct backlight_ops ktd253_backlight_ops = {
+	.options	= BL_CORE_SUSPENDRESUME,
+	.update_status	= ktd253_backlight_update_status,
+};
+
+static int ktd253_backlight_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct backlight_device *bl;
+	struct ktd253_backlight *ktd253;
+	u32 max_brightness;
+	u32 brightness;
+	int ret;
+
+	ktd253 = devm_kzalloc(dev, sizeof(*ktd253), GFP_KERNEL);
+	if (!ktd253)
+		return -ENOMEM;
+	ktd253->dev = dev;
+
+	ret = device_property_read_u32(dev, "max-brightness", &max_brightness);
+	if (ret)
+		max_brightness = KTD253_MAX_BRIGHTNESS;
+
+	ret = device_property_read_u32(dev, "default-brightness", &brightness);
+	if (ret)
+		brightness = KTD253_DEFAULT_BRIGHTNESS;
+
+	if (brightness)
+		/* This will be the default ratio when the KTD253 is enabled */
+		ktd253->ratio = KTD253_MAX_RATIO;
+	else
+		ktd253->ratio = 0;
+
+	ktd253->gpiod = devm_gpiod_get(dev, NULL,
+				       brightness ? GPIOD_OUT_HIGH :
+				       GPIOD_OUT_LOW);
+	if (IS_ERR(ktd253->gpiod)) {
+		ret = PTR_ERR(ktd253->gpiod);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "gpio line missing or invalid.\n");
+		return ret;
+	}
+	gpiod_set_consumer_name(ktd253->gpiod, dev_name(dev));
+
+	bl = devm_backlight_device_register(dev, dev_name(dev), dev, ktd253,
+					    &ktd253_backlight_ops, NULL);
+	if (IS_ERR(bl)) {
+		dev_err(dev, "failed to register backlight\n");
+		return PTR_ERR(bl);
+	}
+	bl->props.max_brightness = max_brightness;
+	/* When we just enable the GPIO line we set max brightness */
+	if (brightness) {
+		bl->props.brightness = brightness;
+		bl->props.power = FB_BLANK_UNBLANK;
+		ktd253_backlight_update_status(bl);
+	} else {
+		bl->props.brightness = 0;
+		bl->props.power = FB_BLANK_POWERDOWN;
+	}
+
+	platform_set_drvdata(pdev, bl);
+
+	return 0;
+}
+
+static const struct of_device_id ktd253_backlight_of_match[] = {
+	{ .compatible = "kinetic,ktd253" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, ktd253_backlight_of_match);
+
+static struct platform_driver ktd253_backlight_driver = {
+	.driver = {
+		.name = "ktd253-backlight",
+		.of_match_table = ktd253_backlight_of_match,
+	},
+	.probe		= ktd253_backlight_probe,
+};
+module_platform_driver(ktd253_backlight_driver);
+
+MODULE_AUTHOR("Linus Walleij <linus.walleij@linaro.org>");
+MODULE_DESCRIPTION("Kinetic KTD253 Backlight Driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:ktd253-backlight");