Discussion:
[PATCH 0/6] Add DT support for at91_udc and atmel_lcdfb
Sam Ravnborg
2017-07-20 20:01:03 UTC
Permalink
A few more patches on my journey to add DT support
for the at91sam9263ek board.

Review / feedback / testing would be appreciated!

The at91_udc driver has seen only limited testing.
Any hints how I can test this are appreciated.
I tried to connect the board to my linux box,
but nothing happened.
The USB gadget core adds a few devices for which
I did not manage to dig up any reasons:
$ devinfo shows:
udc0

And there is also a hidden usbgadget device that shows
the following info:
$ devinfo usbgadget
Parameters:
manufacturer: barebox (type: string)
product: 0x0000 (type: uint32)
productname: Atmel at91sam9263ek (type: string)
vendor: 0x0000 (type: uint32)



The atmel_lcdfb driver in the linux kernel
used device data to define if the device
had an intensity bit.
This is with this patchset encoded in the
lcd-wiring mode.
See further details in the "add DT support" patch.

Tested on the at91sam9263ek board.
(with DT support added - patches will be sent later).

atmel_lcdfb_core is shared between atmel_lcdfb and atmel_hlcdfb.
But only the compatible related to the first are included,
as I was not certain how this would work with the hlcdfb variants.
We can always add remaining compatible entires is someone steps
up and test it.

The work is done on top of master but I do not expect
any problems adding these to -next.

Sam


Sam Ravnborg (6):
at91_udc: add DT support
atmel_lcdfb: move dmacon, lcdcon2 to local data
atmel_lcdfb: move lcd_wiring_mode, have_intensity_bit to local data
atmel_lcdfb: define power_control gpio in platform_data
atmel_lcdfb: move pdata init to a separate function
atmel_lcdfb: add DT support

arch/arm/boards/at91sam9261ek/init.c | 35 +----
arch/arm/boards/at91sam9263ek/init.c | 26 +---
arch/arm/boards/at91sam9m10ihd/init.c | 26 +---
arch/arm/boards/at91sam9n12ek/init.c | 28 +---
drivers/usb/gadget/at91_udc.c | 56 ++++++--
drivers/video/atmel_hlcdfb.c | 5 +-
drivers/video/atmel_lcdfb.c | 19 ++-
drivers/video/atmel_lcdfb.h | 8 +-
drivers/video/atmel_lcdfb_core.c | 247 ++++++++++++++++++++++++++++++----
include/video/atmel_lcdc.h | 3 +-
10 files changed, 295 insertions(+), 158 deletions(-)
Sam Ravnborg
2017-07-20 20:05:23 UTC
Permalink
Copy lcd_wiring_mode and have_intensity_bit to
atmel_lcdfb_info to minimize dependency on the
atmel_lcdfb_platform_data.

Signed-off-by: Sam Ravnborg <***@ravnborg.org>
---
drivers/video/atmel_lcdfb.h | 2 ++
drivers/video/atmel_lcdfb_core.c | 9 +++++----
2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/video/atmel_lcdfb.h b/drivers/video/atmel_lcdfb.h
index 1be9cccd9..90992df02 100644
--- a/drivers/video/atmel_lcdfb.h
+++ b/drivers/video/atmel_lcdfb.h
@@ -23,6 +23,8 @@ struct atmel_lcdfb_info {
unsigned int smem_len;
unsigned int lcdcon2;
unsigned int dmacon;
+ unsigned int lcd_wiring_mode;
+ bool have_intensity_bit;
struct clk *bus_clk;
struct clk *lcdc_clk;

diff --git a/drivers/video/atmel_lcdfb_core.c b/drivers/video/atmel_lcdfb_core.c
index a79c31ae4..cdeb927ad 100644
--- a/drivers/video/atmel_lcdfb_core.c
+++ b/drivers/video/atmel_lcdfb_core.c
@@ -69,7 +69,6 @@ static int atmel_lcdfb_check_var(struct fb_info *info)
{
struct device_d *dev = &info->dev;
struct atmel_lcdfb_info *sinfo = info->priv;
- struct atmel_lcdfb_platform_data *pdata = sinfo->pdata;
struct fb_videomode *mode = info->mode;
unsigned long clk_value_khz;

@@ -126,11 +125,11 @@ static int atmel_lcdfb_check_var(struct fb_info *info)
break;
case 16:
/* Older SOCs use IBGR:555 rather than BGR:565. */
- if (pdata->have_intensity_bit)
+ if (sinfo->have_intensity_bit)
info->green.length = 5;
else
info->green.length = 6;
- if (pdata->lcd_wiring_mode == ATMEL_LCDC_WIRING_RGB) {
+ if (sinfo->lcd_wiring_mode == ATMEL_LCDC_WIRING_RGB) {
/* RGB:5X5 mode */
info->red.offset = info->green.length + 5;
info->blue.offset = 0;
@@ -147,7 +146,7 @@ static int atmel_lcdfb_check_var(struct fb_info *info)
info->transp.length = 8;
/* fall through */
case 24:
- if (pdata->lcd_wiring_mode == ATMEL_LCDC_WIRING_RGB) {
+ if (sinfo->lcd_wiring_mode == ATMEL_LCDC_WIRING_RGB) {
/* RGB:888 mode */
info->red.offset = 16;
info->blue.offset = 0;
@@ -261,6 +260,8 @@ int atmel_lcdc_register(struct device_d *dev, struct atmel_lcdfb_devdata *data)
sinfo->guard_time = pdata->guard_time;
sinfo->lcdcon2 = pdata->default_lcdcon2;
sinfo->dmacon = pdata->default_dmacon;
+ sinfo->lcd_wiring_mode = pdata->lcd_wiring_mode;
+ sinfo->have_intensity_bit = pdata->have_intensity_bit;
iores = dev_request_mem_resource(dev, 0);
if (IS_ERR(iores))
return PTR_ERR(iores);
--
2.12.0
Sam Ravnborg
2017-07-20 20:05:25 UTC
Permalink
Keep atmel_lcdc_register() readable by separating out
pdata handling in a helper function

Signed-off-by: Sam Ravnborg <***@ravnborg.org>
---
drivers/video/atmel_lcdfb_core.c | 65 +++++++++++++++++++++++++---------------
1 file changed, 41 insertions(+), 24 deletions(-)

diff --git a/drivers/video/atmel_lcdfb_core.c b/drivers/video/atmel_lcdfb_core.c
index cc065397d..87bddade4 100644
--- a/drivers/video/atmel_lcdfb_core.c
+++ b/drivers/video/atmel_lcdfb_core.c
@@ -278,31 +278,23 @@ static int power_control_init(struct device_d *dev,
return ret;
}

-int atmel_lcdc_register(struct device_d *dev, struct atmel_lcdfb_devdata *data)
+static int lcdfb_pdata_init(struct device_d *dev, struct atmel_lcdfb_info *sinfo)
{
- struct resource *iores;
- struct atmel_lcdfb_info *sinfo;
- struct atmel_lcdfb_platform_data *pdata = dev->platform_data;
- int ret = 0;
- int gpio;
+ struct atmel_lcdfb_platform_data *pdata;
struct fb_info *info;
+ bool active_low;
+ int gpio;
+ int ret;

- if (!pdata) {
- dev_err(dev, "missing platform_data\n");
- return -EINVAL;
- }
-
- sinfo = xzalloc(sizeof(*sinfo));
+ pdata = dev->platform_data;

/* If gpio == 0 (default in pdata) then we assume no power control */
gpio = pdata->gpio_power_control;
if (gpio == 0)
gpio = -1;

- ret = power_control_init(dev,
- sinfo,
- gpio,
- pdata->gpio_power_control_active_low);
+ active_low = pdata->gpio_power_control_active_low;
+ ret = power_control_init(dev, sinfo, gpio, active_low);
if (ret)
goto err;

@@ -311,23 +303,48 @@ int atmel_lcdc_register(struct device_d *dev, struct atmel_lcdfb_devdata *data)
sinfo->dmacon = pdata->default_dmacon;
sinfo->lcd_wiring_mode = pdata->lcd_wiring_mode;
sinfo->have_intensity_bit = pdata->have_intensity_bit;
+
+ info = &sinfo->info;
+ info->modes.modes = pdata->mode_list;
+ info->modes.num_modes = pdata->num_modes;
+ info->mode = &info->modes.modes[0];
+ info->xres = info->mode->xres;
+ info->yres = info->mode->yres;
+ info->bits_per_pixel = pdata->default_bpp;
+
+err:
+ return ret;
+}
+
+int atmel_lcdc_register(struct device_d *dev, struct atmel_lcdfb_devdata *data)
+{
+ struct resource *iores;
+ struct atmel_lcdfb_info *sinfo;
+ struct fb_info *info;
+ int ret = 0;
+
iores = dev_request_mem_resource(dev, 0);
if (IS_ERR(iores))
return PTR_ERR(iores);
- sinfo->mmio = IOMEM(iores->start);

+ sinfo = xzalloc(sizeof(*sinfo));
sinfo->dev_data = data;
+ sinfo->mmio = IOMEM(iores->start);

- /* just init */
info = &sinfo->info;
info->priv = sinfo;
info->fbops = &atmel_lcdc_ops;
- info->modes.modes = pdata->mode_list;
- info->modes.num_modes = pdata->num_modes;
- info->mode = &info->modes.modes[0];
- info->xres = info->mode->xres;
- info->yres = info->mode->yres;
- info->bits_per_pixel = pdata->default_bpp;
+
+ if (dev->platform_data) {
+ ret = lcdfb_pdata_init(dev, sinfo);
+ if (ret) {
+ dev_err(dev, "failed to init lcdfb from pdata\n");
+ goto err;
+ }
+ } else {
+ dev_err(dev, "missing platform_data\n");
+ return -EINVAL;
+ }

/* Enable LCDC Clocks */
sinfo->bus_clk = clk_get(dev, "hck1");
--
2.12.0
Sam Ravnborg
2017-07-20 20:05:21 UTC
Permalink
Based on the linux kernel version of the same driver.
Needs to adjust clock names as the clock names used in the device tree
does not match the clocknames used in platform_data.
The clocknames in the device tree are not unique, so it was
not an option to rename the clocks.

It boots and the driver is discovered - no further testing done.

$ devinfo fff78000.gadget
Resources:
num: 0
name: /ahb/apb/***@fff78000
start: 0xfff78000
size: 0x00004000
Driver: at91_udc
Bus: platform
Parameters:
vbus: 1 (type: bool)
Device node: /ahb/apb/***@fff78000
***@fff78000 {
compatible = "atmel,at91sam9263-udc";
reg = <0xfff78000 0x4000>;
interrupts = <0x18 0x4 0x2>;
clocks = <0x26 0x27>;
clock-names = "pclk", "hclk";
status = "okay";
atmel,vbus-gpio = <0x28 0x19 0x0>;
};

Signed-off-by: Sam Ravnborg <***@ravnborg.org>
---
drivers/usb/gadget/at91_udc.c | 56 ++++++++++++++++++++++++++++++++++---------
1 file changed, 45 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/gadget/at91_udc.c b/drivers/usb/gadget/at91_udc.c
index 5f6bebc73..18427114d 100644
--- a/drivers/usb/gadget/at91_udc.c
+++ b/drivers/usb/gadget/at91_udc.c
@@ -21,7 +21,7 @@
#include <clock.h>
#include <usb/ch9.h>
#include <usb/gadget.h>
-#include <gpio.h>
+#include <of_gpio.h>

#include <linux/list.h>
#include <linux/clk.h>
@@ -1375,6 +1375,22 @@ static void at91_udc_gadget_poll(struct usb_gadget *gadget)
at91_udc_irq(udc);
}

+static void __init at91udc_of_init(struct at91_udc *udc, struct device_node *np)
+{
+ enum of_gpio_flags flags;
+ struct at91_udc_data *board;
+
+ board = &udc->board;
+
+ board->vbus_pin = of_get_named_gpio_flags(np, "atmel,vbus-gpio", 0,
+ &flags);
+ board->vbus_active_low = (flags & OF_GPIO_ACTIVE_LOW) ? 1 : 0;
+
+ board->pullup_pin = of_get_named_gpio_flags(np, "atmel,pullup-gpio", 0,
+ &flags);
+ board->pullup_active_low = (flags & OF_GPIO_ACTIVE_LOW) ? 1 : 0;
+}
+
/*-------------------------------------------------------------------------*/

static int __init at91udc_probe(struct device_d *dev)
@@ -1382,18 +1398,29 @@ static int __init at91udc_probe(struct device_d *dev)
struct resource *iores;
struct at91_udc *udc = &controller;
int retval;
-
- if (!dev->platform_data) {
- /* small (so we copy it) but critical! */
- DBG(udc, "missing platform_data\n");
- return -ENODEV;
- }
+ const char *iclk_name;
+ const char *fclk_name;

/* init software state */
udc->dev = dev;
- udc->board = *(struct at91_udc_data *) dev->platform_data;
udc->enabled = 0;

+ if (dev->platform_data) {
+ /* small (so we copy it) */
+ udc->board = *(struct at91_udc_data *)dev->platform_data;
+ iclk_name = "udc_clk";
+ fclk_name = "udpck";
+ } else {
+ if (!IS_ENABLED(CONFIG_OFDEVICE) || !dev->device_node) {
+ dev_err(dev, "no DT and no platform_data\n");
+ return -ENODEV;
+ }
+
+ at91udc_of_init(udc, dev->device_node);
+ iclk_name = "pclk";
+ fclk_name = "hclk";
+ }
+
/* rm9200 needs manual D+ pullup; off by default */
if (cpu_is_at91rm9200()) {
if (udc->board.pullup_pin <= 0) {
@@ -1435,8 +1462,8 @@ static int __init at91udc_probe(struct device_d *dev)
udc_reinit(udc);

/* get interface and function clocks */
- udc->iclk = clk_get(dev, "udc_clk");
- udc->fclk = clk_get(dev, "udpck");
+ udc->iclk = clk_get(dev, iclk_name);
+ udc->fclk = clk_get(dev, fclk_name);
if (IS_ERR(udc->iclk) || IS_ERR(udc->fclk)) {
DBG(udc, "clocks missing\n");
retval = -ENODEV;
@@ -1491,10 +1518,17 @@ fail0:
DBG(udc, "%s probe failed, %d\n", driver_name, retval);
return retval;
}
-
+static const struct of_device_id at91_udc_dt_ids[] = {
+ { .compatible = "atmel,at91rm9200-udc" },
+ { .compatible = "atmel,at91sam9260-udc" },
+ { .compatible = "atmel,at91sam9261-udc" },
+ { .compatible = "atmel,at91sam9263-udc" },
+ { /* sentinel */ }
+};

static struct driver_d at91_udc_driver = {
.name = driver_name,
.probe = at91udc_probe,
+ .of_compatible = DRV_OF_COMPAT(at91_udc_dt_ids),
};
device_platform_driver(at91_udc_driver);
--
2.12.0
Sam Ravnborg
2017-07-20 20:05:24 UTC
Permalink
Simplify board specific code by specifying the power_control
gpio direct in platform data.
Move registration of the GPIO to the driver so we no longer
need to duplicate this for each board.

As an intended side-effect there is no longer
any references to platform_data outside atmel_lcdc_register()
so remove it from struct atmel_lcdfb_info

The implementation assumes that GPIO=0 is the same as no power control.
This prevents us from using any GPIO=0 for power control,
but this is not considered a problem for current users.
Future DT users will not have this limitation.

This commit include a fix so we will actually power
down if requested. Previously this was hardcoded to ON.

Signed-off-by: Sam Ravnborg <***@ravnborg.org>
---
arch/arm/boards/at91sam9261ek/init.c | 35 ++-------------------
arch/arm/boards/at91sam9263ek/init.c | 26 +--------------
arch/arm/boards/at91sam9m10ihd/init.c | 26 +--------------
arch/arm/boards/at91sam9n12ek/init.c | 28 ++---------------
drivers/video/atmel_lcdfb.h | 4 ++-
drivers/video/atmel_lcdfb_core.c | 59 ++++++++++++++++++++++++++++++++---
include/video/atmel_lcdc.h | 3 +-
7 files changed, 65 insertions(+), 116 deletions(-)

diff --git a/arch/arm/boards/at91sam9261ek/init.c b/arch/arm/boards/at91sam9261ek/init.c
index 72716b818..58f253b1a 100644
--- a/arch/arm/boards/at91sam9261ek/init.c
+++ b/arch/arm/boards/at91sam9261ek/init.c
@@ -158,22 +158,6 @@ static void ek_add_device_udc(void) {}
* LCD Controller
*/
#if defined(CONFIG_DRIVER_VIDEO_ATMEL)
-static int ek_gpio_request_output(int gpio, const char *name)
-{
- int ret;
-
- ret = gpio_request(gpio, name);
- if (ret) {
- pr_err("%s: can not request gpio %d (%d)\n", name, gpio, ret);
- return ret;
- }
-
- ret = gpio_direction_output(gpio, 1);
- if (ret)
- pr_err("%s: can not configure gpio %d as output (%d)\n", name, gpio, ret);
- return ret;
-}
-
/* TFT */
static struct fb_videomode at91_tft_vga_modes[] = {
{
@@ -195,35 +179,20 @@ static struct fb_videomode at91_tft_vga_modes[] = {
| ATMEL_LCDC_DISTYPE_TFT \
| ATMEL_LCDC_CLKMOD_ALWAYSACTIVE)

-static void at91_lcdc_tft_power_control(int on)
-{
- if (on)
- gpio_set_value(AT91_PIN_PA12, 0); /* power up */
- else
- gpio_set_value(AT91_PIN_PA12, 1); /* power down */
-}
-
static struct atmel_lcdfb_platform_data ek_lcdc_data = {
.lcdcon_is_backlight = true,
.default_bpp = 16,
.default_dmacon = ATMEL_LCDC_DMAEN,
.default_lcdcon2 = AT91SAM9261_DEFAULT_TFT_LCDCON2,
.guard_time = 1,
- .atmel_lcdfb_power_control = at91_lcdc_tft_power_control,
+ .gpio_power_control = AT91_PIN_PA12,
+ .gpio_power_control_active_low = true,
.mode_list = at91_tft_vga_modes,
.num_modes = ARRAY_SIZE(at91_tft_vga_modes),
};

-static int at91_lcdc_gpio(void)
-{
- return ek_gpio_request_output(AT91_PIN_PA12, "lcdc_tft_power");
-}
-
static void ek_add_device_lcdc(void)
{
- if (at91_lcdc_gpio())
- return;
-
if (machine_is_at91sam9g10ek())
ek_lcdc_data.lcd_wiring_mode = ATMEL_LCDC_WIRING_RGB;

diff --git a/arch/arm/boards/at91sam9263ek/init.c b/arch/arm/boards/at91sam9263ek/init.c
index 682449b7b..b71cc5517 100644
--- a/arch/arm/boards/at91sam9263ek/init.c
+++ b/arch/arm/boards/at91sam9263ek/init.c
@@ -156,22 +156,6 @@ static void ek_add_device_udc(void) {}
* LCD Controller
*/
#if defined(CONFIG_DRIVER_VIDEO_ATMEL)
-static int ek_gpio_request_output(int gpio, const char *name)
-{
- int ret;
-
- ret = gpio_request(gpio, name);
- if (ret) {
- pr_err("%s: can not request gpio %d (%d)\n", name, gpio, ret);
- return ret;
- }
-
- ret = gpio_direction_output(gpio, 1);
- if (ret)
- pr_err("%s: can not configure gpio %d as output (%d)\n", name, gpio, ret);
- return ret;
-}
-
static struct fb_videomode at91_tft_vga_modes[] = {
{
.name = "TX09D50VM1CCA @ 60",
@@ -192,11 +176,6 @@ static struct fb_videomode at91_tft_vga_modes[] = {
| ATMEL_LCDC_DISTYPE_TFT \
| ATMEL_LCDC_CLKMOD_ALWAYSACTIVE)

-static void at91_lcdc_power_control(int on)
-{
- gpio_set_value(AT91_PIN_PA30, on);
-}
-
/* Driver datas */
static struct atmel_lcdfb_platform_data ek_lcdc_data = {
.lcdcon_is_backlight = true,
@@ -204,16 +183,13 @@ static struct atmel_lcdfb_platform_data ek_lcdc_data = {
.default_dmacon = ATMEL_LCDC_DMAEN,
.default_lcdcon2 = AT91SAM9263_DEFAULT_LCDCON2,
.guard_time = 1,
- .atmel_lcdfb_power_control = at91_lcdc_power_control,
+ .gpio_power_control = AT91_PIN_PA30,
.mode_list = at91_tft_vga_modes,
.num_modes = ARRAY_SIZE(at91_tft_vga_modes),
};

static void ek_add_device_lcdc(void)
{
- if (ek_gpio_request_output(AT91_PIN_PA30, "lcdc_power"))
- return;
-
at91_add_device_lcdc(&ek_lcdc_data);
}

diff --git a/arch/arm/boards/at91sam9m10ihd/init.c b/arch/arm/boards/at91sam9m10ihd/init.c
index dcd93c10b..de601d53b 100644
--- a/arch/arm/boards/at91sam9m10ihd/init.c
+++ b/arch/arm/boards/at91sam9m10ihd/init.c
@@ -177,22 +177,6 @@ static int at91sam9m10g45ek_mem_init(void)
mem_initcall(at91sam9m10g45ek_mem_init);

#if defined(CONFIG_DRIVER_VIDEO_ATMEL)
-static int ek_gpio_request_output(int gpio, const char *name)
-{
- int ret;
-
- ret = gpio_request(gpio, name);
- if (ret) {
- pr_err("%s: can not request gpio %d (%d)\n", name, gpio, ret);
- return ret;
- }
-
- ret = gpio_direction_output(gpio, 1);
- if (ret)
- pr_err("%s: can not configure gpio %d as output (%d)\n", name, gpio, ret);
- return ret;
-}
-
static struct fb_videomode at91fb_default_monspecs[] = {
{
.name = "MULTEK",
@@ -213,11 +197,6 @@ static struct fb_videomode at91fb_default_monspecs[] = {
| ATMEL_LCDC_DISTYPE_TFT \
| ATMEL_LCDC_CLKMOD_ALWAYSACTIVE)

-static void at91_lcdc_power_control(int on)
-{
- gpio_set_value(AT91_PIN_PE6, on);
-}
-
/* Driver datas */
static struct atmel_lcdfb_platform_data ek_lcdc_data = {
.lcdcon_is_backlight = true,
@@ -226,16 +205,13 @@ static struct atmel_lcdfb_platform_data ek_lcdc_data = {
.default_lcdcon2 = AT91SAM9G45_DEFAULT_LCDCON2,
.guard_time = 9,
.lcd_wiring_mode = ATMEL_LCDC_WIRING_RGB,
- .atmel_lcdfb_power_control = at91_lcdc_power_control,
+ .gpio_power_control = AT91_PIN_PE6,
.mode_list = at91fb_default_monspecs,
.num_modes = ARRAY_SIZE(at91fb_default_monspecs),
};

static void ek_add_device_lcd(void)
{
- if (ek_gpio_request_output(AT91_PIN_PE6, "lcdc_power"))
- return;
-
at91_add_device_lcdc(&ek_lcdc_data);
}
#else
diff --git a/arch/arm/boards/at91sam9n12ek/init.c b/arch/arm/boards/at91sam9n12ek/init.c
index b9431b2ee..bc3fb8e08 100644
--- a/arch/arm/boards/at91sam9n12ek/init.c
+++ b/arch/arm/boards/at91sam9n12ek/init.c
@@ -127,23 +127,6 @@ static void __init ek_add_device_ks8851(void) {}
#endif /* CONFIG_DRIVER_NET_KS8851_MLL */

#if defined(CONFIG_DRIVER_VIDEO_ATMEL_HLCD)
-static int ek_gpio_request_output(int gpio, const char *name)
-{
- int ret;
-
- ret = gpio_request(gpio, name);
- if (ret) {
- pr_err("%s: can not request gpio %d (%d)\n", name, gpio, ret);
- return ret;
- }
-
- ret = gpio_direction_output(gpio, 1);
- if (ret)
- pr_err("%s: can not configure gpio %d as output (%d)\n", name, gpio, ret);
- return ret;
-}
-
-
/*
* LCD Controller
*/
@@ -166,11 +149,6 @@ static struct fb_videomode at91_tft_vga_modes[] = {
/* Default output mode is TFT 24 bit */
#define BPP_OUT_DEFAULT_LCDCFG5 (LCDC_LCDCFG5_MODE_OUTPUT_24BPP)

-static void at91_lcdc_power_control(int on)
-{
- gpio_set_value(AT91_PIN_PC25, !on);
-}
-
/* Driver datas */
static struct atmel_lcdfb_platform_data ek_lcdc_data = {
.lcdcon_is_backlight = true,
@@ -179,16 +157,14 @@ static struct atmel_lcdfb_platform_data ek_lcdc_data = {
.default_lcdcon2 = BPP_OUT_DEFAULT_LCDCFG5,
.guard_time = 9,
.lcd_wiring_mode = ATMEL_LCDC_WIRING_RGB,
- .atmel_lcdfb_power_control = at91_lcdc_power_control,
+ .gpio_power_control = AT91_PIN_PC25,
+ .gpio_power_control_active_low = true,
.mode_list = at91_tft_vga_modes,
.num_modes = ARRAY_SIZE(at91_tft_vga_modes),
};

static void ek_add_device_lcdc(void)
{
- if (ek_gpio_request_output(AT91_PIN_PC25, "lcdc_power"))
- return;
-
at91_add_device_lcdc(&ek_lcdc_data);
}
#else
diff --git a/drivers/video/atmel_lcdfb.h b/drivers/video/atmel_lcdfb.h
index 90992df02..a011d4201 100644
--- a/drivers/video/atmel_lcdfb.h
+++ b/drivers/video/atmel_lcdfb.h
@@ -25,10 +25,12 @@ struct atmel_lcdfb_info {
unsigned int dmacon;
unsigned int lcd_wiring_mode;
bool have_intensity_bit;
+
+ int gpio_power_control;
+ bool gpio_power_control_active_low;
struct clk *bus_clk;
struct clk *lcdc_clk;

- struct atmel_lcdfb_platform_data *pdata;
struct atmel_lcdfb_devdata *dev_data;
void *dma_desc;
};
diff --git a/drivers/video/atmel_lcdfb_core.c b/drivers/video/atmel_lcdfb_core.c
index cdeb927ad..cc065397d 100644
--- a/drivers/video/atmel_lcdfb_core.c
+++ b/drivers/video/atmel_lcdfb_core.c
@@ -19,6 +19,7 @@
*/

#include <common.h>
+#include <gpio.h>
#include <dma.h>
#include <io.h>
#include <linux/err.h>
@@ -39,13 +40,17 @@ static void atmel_lcdfb_stop_clock(struct atmel_lcdfb_info *sinfo)
clk_disable(sinfo->lcdc_clk);
}

-static void atmel_lcdc_power_controller(struct fb_info *fb_info, int i)
+static void atmel_lcdc_power_controller(struct fb_info *fb_info, int on)
{
struct atmel_lcdfb_info *sinfo = fb_info->priv;
- struct atmel_lcdfb_platform_data *pdata = sinfo->pdata;

- if (pdata->atmel_lcdfb_power_control)
- pdata->atmel_lcdfb_power_control(1);
+ if (sinfo->gpio_power_control < 0)
+ return;
+
+ if (sinfo->gpio_power_control_active_low)
+ gpio_set_value(sinfo->gpio_power_control, !on);
+ else
+ gpio_set_value(sinfo->gpio_power_control, on);
}

/**
@@ -242,12 +247,44 @@ static struct fb_ops atmel_lcdc_ops = {
.fb_disable = atmel_lcdc_disable_controller,
};

+static int power_control_init(struct device_d *dev,
+ struct atmel_lcdfb_info *sinfo,
+ int gpio,
+ bool active_low)
+{
+ int ret;
+ const char *name = "lcdc_power";
+
+ sinfo->gpio_power_control = gpio;
+ sinfo->gpio_power_control_active_low = active_low;
+
+ /* If no GPIO specified then stop */
+ if (!gpio_is_valid(gpio))
+ return 0;
+
+ ret = gpio_request(gpio, name);
+ if (ret) {
+ dev_err(dev, "%s: can not request gpio %d (%d)\n",
+ name, gpio, ret);
+ return ret;
+ }
+ ret = gpio_direction_output(gpio, 1);
+ if (ret) {
+ dev_err(dev, "%s: can not configure gpio %d as output (%d)\n",
+ name, gpio, ret);
+ return ret;
+ }
+
+ return ret;
+}
+
int atmel_lcdc_register(struct device_d *dev, struct atmel_lcdfb_devdata *data)
{
struct resource *iores;
struct atmel_lcdfb_info *sinfo;
struct atmel_lcdfb_platform_data *pdata = dev->platform_data;
int ret = 0;
+ int gpio;
struct fb_info *info;

if (!pdata) {
@@ -256,7 +293,19 @@ int atmel_lcdc_register(struct device_d *dev, struct atmel_lcdfb_devdata *data)
}

sinfo = xzalloc(sizeof(*sinfo));
- sinfo->pdata = pdata;
+
+ /* If gpio == 0 (default in pdata) then we assume no power control */
+ gpio = pdata->gpio_power_control;
+ if (gpio == 0)
+ gpio = -1;
+
+ ret = power_control_init(dev,
+ sinfo,
+ gpio,
+ pdata->gpio_power_control_active_low);
+ if (ret)
+ goto err;
+
sinfo->guard_time = pdata->guard_time;
sinfo->lcdcon2 = pdata->default_lcdcon2;
sinfo->dmacon = pdata->default_dmacon;
diff --git a/include/video/atmel_lcdc.h b/include/video/atmel_lcdc.h
index 132ee598a..07a30e2e6 100644
--- a/include/video/atmel_lcdc.h
+++ b/include/video/atmel_lcdc.h
@@ -44,7 +44,8 @@ struct atmel_lcdfb_platform_data {
u8 lcd_wiring_mode;
unsigned int default_lcdcon2;
unsigned int default_dmacon;
- void (*atmel_lcdfb_power_control)(int on);
+ int gpio_power_control;
+ bool gpio_power_control_active_low;
struct fb_videomode *mode_list;
unsigned num_modes;
--
2.12.0
Sam Ravnborg
2017-07-20 20:05:22 UTC
Permalink
Copy dmacon + lcdcon2 to atmel_lcdfb_info to minimize
dependency on the atmel_lcdfb_platform_data.

Signed-off-by: Sam Ravnborg <***@ravnborg.org>
---
drivers/video/atmel_hlcdfb.c | 5 ++---
drivers/video/atmel_lcdfb.c | 7 ++-----
drivers/video/atmel_lcdfb.h | 2 ++
drivers/video/atmel_lcdfb_core.c | 2 ++
4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/video/atmel_hlcdfb.c b/drivers/video/atmel_hlcdfb.c
index f7aab7f45..5d130f598 100644
--- a/drivers/video/atmel_hlcdfb.c
+++ b/drivers/video/atmel_hlcdfb.c
@@ -178,7 +178,6 @@ static u32 atmel_hlcdfb_get_rgbmode(struct fb_info *info)
static void atmel_hlcdfb_setup_core_base(struct fb_info *info)
{
struct atmel_lcdfb_info *sinfo = info->priv;
- struct atmel_lcdfb_platform_data *pdata = sinfo->pdata;
struct fb_videomode *mode = info->mode;
unsigned long value;
unsigned long clk_value_khz;
@@ -205,8 +204,8 @@ static void atmel_hlcdfb_setup_core_base(struct fb_info *info)
lcdc_writel(sinfo, ATMEL_LCDC_LCDCFG0, value);

/* Initialize control register 5 */
- /* In 9x5, the default_lcdcon2 will use for LCDCFG5 */
- value = pdata->default_lcdcon2;
+ /* In 9x5, the lcdcon2 will use for LCDCFG5 */
+ value = sinfo->lcdcon2;
value |= (sinfo->guard_time << LCDC_LCDCFG5_GUARDTIME_OFFSET)
| LCDC_LCDCFG5_DISPDLY
| LCDC_LCDCFG5_VSPDLYS;
diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
index a0e41d10c..770cf0497 100644
--- a/drivers/video/atmel_lcdfb.c
+++ b/drivers/video/atmel_lcdfb.c
@@ -81,9 +81,7 @@ static void atmel_lcdfb_stop(struct atmel_lcdfb_info *sinfo, u32 flags)

static void atmel_lcdfb_start(struct atmel_lcdfb_info *sinfo)
{
- struct atmel_lcdfb_platform_data *pdata = sinfo->pdata;
-
- lcdc_writel(sinfo, ATMEL_LCDC_DMACON, pdata->default_dmacon);
+ lcdc_writel(sinfo, ATMEL_LCDC_DMACON, sinfo->dmacon);
lcdc_writel(sinfo, ATMEL_LCDC_PWRCON,
(sinfo->guard_time << ATMEL_LCDC_GUARDT_OFFSET)
| ATMEL_LCDC_PWR);
@@ -123,7 +121,6 @@ static void atmel_lcdfb_limit_screeninfo(struct fb_videomode *mode)
static void atmel_lcdfb_setup_core(struct fb_info *info)
{
struct atmel_lcdfb_info *sinfo = info->priv;
- struct atmel_lcdfb_platform_data *pdata = sinfo->pdata;
struct fb_videomode *mode = info->mode;
unsigned long clk_value_khz;
unsigned long pix_factor = 2;
@@ -159,7 +156,7 @@ static void atmel_lcdfb_setup_core(struct fb_info *info)
}

/* Initialize control register 2 */
- value = pdata->default_lcdcon2;
+ value = sinfo->lcdcon2;

if (!(mode->sync & FB_SYNC_HOR_HIGH_ACT))
value |= ATMEL_LCDC_INVLINE_INVERTED;
diff --git a/drivers/video/atmel_lcdfb.h b/drivers/video/atmel_lcdfb.h
index ea4c7e647..1be9cccd9 100644
--- a/drivers/video/atmel_lcdfb.h
+++ b/drivers/video/atmel_lcdfb.h
@@ -21,6 +21,8 @@ struct atmel_lcdfb_info {

unsigned int guard_time;
unsigned int smem_len;
+ unsigned int lcdcon2;
+ unsigned int dmacon;
struct clk *bus_clk;
struct clk *lcdc_clk;

diff --git a/drivers/video/atmel_lcdfb_core.c b/drivers/video/atmel_lcdfb_core.c
index f6c5d7c05..a79c31ae4 100644
--- a/drivers/video/atmel_lcdfb_core.c
+++ b/drivers/video/atmel_lcdfb_core.c
@@ -259,6 +259,8 @@ int atmel_lcdc_register(struct device_d *dev, struct atmel_lcdfb_devdata *data)
sinfo = xzalloc(sizeof(*sinfo));
sinfo->pdata = pdata;
sinfo->guard_time = pdata->guard_time;
+ sinfo->lcdcon2 = pdata->default_lcdcon2;
+ sinfo->dmacon = pdata->default_dmacon;
iores = dev_request_mem_resource(dev, 0);
if (IS_ERR(iores))
return PTR_ERR(iores);
--
2.12.0
Sam Ravnborg
2017-07-20 20:05:26 UTC
Permalink
Some boards use "have_intensity_bit". To support this the
syntax for lcd_wiring_mode was extended to include this info.
This is an extension compared to the documented bindings,
and an extension the kernel does not support (yet).

The binding documents that there can be more than one gpio to
power on/off the display but current implmentation supports only
one gpio. There are no users that requires more than one gpio
so this is good enough.

The clk name used for hclk differs from device trees and
platform data. We could rename the clocl used in platform data
but the name "hclk" is not unique.

Configure clockname based on configuration method.
Devicetree => hclk
Platform data => hck1

Signed-off-by: Sam Ravnborg <***@ravnborg.org>
---
drivers/video/atmel_lcdfb.c | 12 ++++
drivers/video/atmel_lcdfb_core.c | 130 +++++++++++++++++++++++++++++++++++++--
2 files changed, 138 insertions(+), 4 deletions(-)

diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
index 770cf0497..7c05e857b 100644
--- a/drivers/video/atmel_lcdfb.c
+++ b/drivers/video/atmel_lcdfb.c
@@ -243,8 +243,20 @@ static int atmel_lcdc_probe(struct device_d *dev)
return atmel_lcdc_register(dev, &atmel_lcdfb_data);
}

+static __maybe_unused struct of_device_id atmel_lcdfb_compatible[] = {
+ { .compatible = "atmel,at91sam9261-lcdc", },
+ { .compatible = "atmel,at91sam9263-lcdc", },
+ { .compatible = "atmel,at91sam9g10-lcdc", },
+ { .compatible = "atmel,at91sam9g45-lcdc", },
+ { .compatible = "atmel,at91sam9g45es-lcdc", },
+ { .compatible = "atmel,at91sam9rl-lcdc", },
+ { .compatible = "atmel,at32ap-lcdc", },
+ { /* sentinel */ }
+};
+
static struct driver_d atmel_lcdc_driver = {
.name = "atmel_lcdfb",
.probe = atmel_lcdc_probe,
+ .of_compatible = DRV_OF_COMPAT(atmel_lcdfb_compatible),
};
device_platform_driver(atmel_lcdc_driver);
diff --git a/drivers/video/atmel_lcdfb_core.c b/drivers/video/atmel_lcdfb_core.c
index 87bddade4..45b0c63d0 100644
--- a/drivers/video/atmel_lcdfb_core.c
+++ b/drivers/video/atmel_lcdfb_core.c
@@ -19,6 +19,7 @@
*/

#include <common.h>
+#include <of_gpio.h>
#include <gpio.h>
#include <dma.h>
#include <io.h>
@@ -278,6 +279,118 @@ static int power_control_init(struct device_d *dev,
return ret;
}

+/*
+ * Syntax: atmel,lcd-wiring-mode: lcd wiring mode "RGB", "BRG", "IRGB", "IBRG"
+ * The optional "I" indicates that green has an intensity bit as used by some
+ * older displays
+ */
+static int of_get_wiring_mode(struct device_node *np,
+ struct atmel_lcdfb_info *sinfo)
+{
+ const char *mode;
+ int ret;
+
+ ret = of_property_read_string(np, "atmel,lcd-wiring-mode", &mode);
+ if (ret < 0) {
+ /* Not present, use defaults */
+ sinfo->lcd_wiring_mode = ATMEL_LCDC_WIRING_BGR;
+ sinfo->have_intensity_bit = false;
+ return 0;
+ }
+
+ if (!strcasecmp(mode, "BGR")) {
+ sinfo->lcd_wiring_mode = ATMEL_LCDC_WIRING_BGR;
+ sinfo->have_intensity_bit = false;
+ } else if (!strcasecmp(mode, "RGB")) {
+ sinfo->lcd_wiring_mode = ATMEL_LCDC_WIRING_RGB;
+ sinfo->have_intensity_bit = false;
+ } else if (!strcasecmp(mode, "IBGR")) {
+ sinfo->lcd_wiring_mode = ATMEL_LCDC_WIRING_BGR;
+ sinfo->have_intensity_bit = true;
+ } else if (!strcasecmp(mode, "IRGB")) {
+ sinfo->lcd_wiring_mode = ATMEL_LCDC_WIRING_RGB;
+ sinfo->have_intensity_bit = true;
+ } else {
+ return -ENODEV;
+ }
+ return 0;
+}
+
+static int of_get_power_control(struct device_d *dev,
+ struct device_node *np,
+ struct atmel_lcdfb_info *sinfo)
+{
+ enum of_gpio_flags flags;
+ bool active_low;
+ int gpio;
+
+ gpio = of_get_named_gpio_flags(np, "atmel,power-control-gpio", 0, &flags);
+ if (!gpio_is_valid(gpio)) {
+ /* No power control - ignore */
+ return 0;
+ }
+ active_low = (flags & OF_GPIO_ACTIVE_LOW ? true : false);
+ return power_control_init(dev, sinfo, gpio, active_low);
+}
+
+static int lcdfb_of_init(struct device_d *dev, struct atmel_lcdfb_info *sinfo)
+{
+ struct fb_info *info = &sinfo->info;
+ struct display_timings *modes;
+ struct device_node *display;
+ int ret;
+
+ /* Required properties */
+ display = of_parse_phandle(dev->device_node, "display", 0);
+ if (!display) {
+ dev_err(dev, "no display phandle\n");
+ return -ENOENT;
+ }
+ ret = of_property_read_u32(display, "atmel,guard-time", &sinfo->guard_time);
+ if (ret < 0) {
+ dev_err(dev, "failed to get atmel,guard-time property\n");
+ goto err;
+ }
+ ret = of_property_read_u32(display, "atmel,lcdcon2", &sinfo->lcdcon2);
+ if (ret < 0) {
+ dev_err(dev, "failed to get atmel,lcdcon2 property\n");
+ goto err;
+ }
+ ret = of_property_read_u32(display, "atmel,dmacon", &sinfo->dmacon);
+ if (ret < 0) {
+ dev_err(dev, "failed to get atmel,dmacon property\n");
+ goto err;
+ }
+ ret = of_property_read_u32(display, "bits-per-pixel", &info->bits_per_pixel);
+ if (ret < 0) {
+ dev_err(dev, "failed to get bits-per-pixel property\n");
+ goto err;
+ }
+ modes = of_get_display_timings(display);
+ if (IS_ERR(modes)) {
+ dev_err(dev, "unable to parse display timings\n");
+ ret = PTR_ERR(modes);
+ goto err;
+ }
+ info->modes.modes = modes->modes;
+ info->modes.num_modes = modes->num_modes;
+
+ /* Optional properties */
+ ret = of_get_wiring_mode(display, sinfo);
+ if (ret < 0) {
+ dev_err(dev, "failed to get atmel,lcd-wiring-mode property\n");
+ goto err;
+ }
+ ret = of_get_power_control(dev, display, sinfo);
+ if (ret < 0) {
+ dev_err(dev, "failed to get power control gpio\n");
+ goto err;
+ }
+ return 0;
+err:
+ return ret;
+}
+
static int lcdfb_pdata_init(struct device_d *dev, struct atmel_lcdfb_info *sinfo)
{
struct atmel_lcdfb_platform_data *pdata;
@@ -318,8 +431,9 @@ err:

int atmel_lcdc_register(struct device_d *dev, struct atmel_lcdfb_devdata *data)
{
- struct resource *iores;
struct atmel_lcdfb_info *sinfo;
+ const char *bus_clk_name;
+ struct resource *iores;
struct fb_info *info;
int ret = 0;

@@ -341,13 +455,21 @@ int atmel_lcdc_register(struct device_d *dev, struct atmel_lcdfb_devdata *data)
dev_err(dev, "failed to init lcdfb from pdata\n");
goto err;
}
+ bus_clk_name = "hck1";
} else {
- dev_err(dev, "missing platform_data\n");
- return -EINVAL;
+ if (!IS_ENABLED(CONFIG_OFDEVICE) || !dev->device_node)
+ return -EINVAL;
+
+ ret = lcdfb_of_init(dev, sinfo);
+ if (ret) {
+ dev_err(dev, "failed to init lcdfb from DT\n");
+ goto err;
+ }
+ bus_clk_name = "hclk";
}

/* Enable LCDC Clocks */
- sinfo->bus_clk = clk_get(dev, "hck1");
+ sinfo->bus_clk = clk_get(dev, bus_clk_name);
if (IS_ERR(sinfo->bus_clk)) {
ret = PTR_ERR(sinfo->bus_clk);
goto err;
--
2.12.0
Loading...