* [PATCH 06/19] ARM: mach-shmobile: Initiliaze the new sh_mipi_dsi_info channel field
From: Laurent Pinchart @ 2012-08-16 13:00 UTC (permalink / raw)
To: linux-fbdev
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
arch/arm/mach-shmobile/board-ag5evm.c | 1 +
arch/arm/mach-shmobile/board-ap4evb.c | 1 +
2 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-shmobile/board-ag5evm.c b/arch/arm/mach-shmobile/board-ag5evm.c
index afae573..5523bc8 100644
--- a/arch/arm/mach-shmobile/board-ag5evm.c
+++ b/arch/arm/mach-shmobile/board-ag5evm.c
@@ -258,6 +258,7 @@ static struct sh_mobile_lcdc_info lcdc0_info;
static struct sh_mipi_dsi_info mipidsi0_info = {
.data_format = MIPI_RGB888,
.lcd_chan = &lcdc0_info.ch[0],
+ .channel = LCDC_CHAN_MAINLCD,
.lane = 2,
.vsynw_offset = 20,
.clksrc = 1,
diff --git a/arch/arm/mach-shmobile/board-ap4evb.c b/arch/arm/mach-shmobile/board-ap4evb.c
index ace6024..5fbd176 100644
--- a/arch/arm/mach-shmobile/board-ap4evb.c
+++ b/arch/arm/mach-shmobile/board-ap4evb.c
@@ -532,6 +532,7 @@ static struct sh_mobile_lcdc_info lcdc_info;
static struct sh_mipi_dsi_info mipidsi0_info = {
.data_format = MIPI_RGB888,
.lcd_chan = &lcdc_info.ch[0],
+ .channel = LCDC_CHAN_MAINLCD,
.lane = 2,
.vsynw_offset = 17,
.phyctrl = 0x6 << 8,
--
1.7.8.6
^ permalink raw reply related
* [PATCH 07/19] fbdev: sh_mipi_dsi: Use the sh_mipi_dsi_info channel field
From: Laurent Pinchart @ 2012-08-16 13:00 UTC (permalink / raw)
To: linux-fbdev
Get the LCDC channel selector from the sh_mipi_dsi_info channel field
directly instead of accessing the LCDC platform data through the
lcd_chan field.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/video/sh_mipi_dsi.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/video/sh_mipi_dsi.c b/drivers/video/sh_mipi_dsi.c
index 3951fda..7f6ce65 100644
--- a/drivers/video/sh_mipi_dsi.c
+++ b/drivers/video/sh_mipi_dsi.c
@@ -369,7 +369,7 @@ static int sh_mipi_setup(struct sh_mipi *mipi, struct sh_mipi_dsi_info *pdata)
/* setup LCD panel */
/* cf. drivers/video/omap/lcd_mipid.c */
- sh_mipi_dcs(ch->chan, MIPI_DCS_EXIT_SLEEP_MODE);
+ sh_mipi_dcs(pdata->channel, MIPI_DCS_EXIT_SLEEP_MODE);
msleep(120);
/*
* [7] - Page Address Mode
@@ -381,11 +381,11 @@ static int sh_mipi_setup(struct sh_mipi *mipi, struct sh_mipi_dsi_info *pdata)
* [1] - Flip Horizontal
* [0] - Flip Vertical
*/
- sh_mipi_dcs_param(ch->chan, MIPI_DCS_SET_ADDRESS_MODE, 0x00);
+ sh_mipi_dcs_param(pdata->channel, MIPI_DCS_SET_ADDRESS_MODE, 0x00);
/* cf. set_data_lines() */
- sh_mipi_dcs_param(ch->chan, MIPI_DCS_SET_PIXEL_FORMAT,
+ sh_mipi_dcs_param(pdata->channel, MIPI_DCS_SET_PIXEL_FORMAT,
pixfmt << 4);
- sh_mipi_dcs(ch->chan, MIPI_DCS_SET_DISPLAY_ON);
+ sh_mipi_dcs(pdata->channel, MIPI_DCS_SET_DISPLAY_ON);
/* Enable timeout counters */
iowrite32(0x00000f00, base + DSICTRL);
--
1.7.8.6
^ permalink raw reply related
* [PATCH 08/19] fbdev: sh_mipi_dsi: Use the LCDC entity default mode
From: Laurent Pinchart @ 2012-08-16 13:00 UTC (permalink / raw)
To: linux-fbdev
Configure the MIPI-DSI transmitter using the LCDC entity default mode
instead of accessing the mode through the LCDC platform data.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/video/sh_mipi_dsi.c | 43 ++++++++++++++++++++++---------------------
1 files changed, 22 insertions(+), 21 deletions(-)
diff --git a/drivers/video/sh_mipi_dsi.c b/drivers/video/sh_mipi_dsi.c
index 7f6ce65..f88b204 100644
--- a/drivers/video/sh_mipi_dsi.c
+++ b/drivers/video/sh_mipi_dsi.c
@@ -127,9 +127,10 @@ static void sh_mipi_shutdown(struct platform_device *pdev)
sh_mipi_dsi_enable(mipi, false);
}
-static int sh_mipi_setup(struct sh_mipi *mipi, struct sh_mipi_dsi_info *pdata)
+static int sh_mipi_setup(struct sh_mipi *mipi, const struct fb_videomode *mode)
{
void __iomem *base = mipi->base;
+ struct sh_mipi_dsi_info *pdata = mipi->pdev->dev.platform_data;
struct sh_mobile_lcdc_chan_cfg *ch = pdata->lcd_chan;
u32 pctype, datatype, pixfmt, linelength, vmctr2;
u32 tmp, top, bottom, delay, div;
@@ -146,77 +147,77 @@ static int sh_mipi_setup(struct sh_mipi *mipi, struct sh_mipi_dsi_info *pdata)
pctype = 0;
datatype = MIPI_DSI_PACKED_PIXEL_STREAM_24;
pixfmt = MIPI_DCS_PIXEL_FMT_24BIT;
- linelength = ch->lcd_modes[0].xres * 3;
+ linelength = mode->xres * 3;
yuv = false;
break;
case MIPI_RGB565:
pctype = 1;
datatype = MIPI_DSI_PACKED_PIXEL_STREAM_16;
pixfmt = MIPI_DCS_PIXEL_FMT_16BIT;
- linelength = ch->lcd_modes[0].xres * 2;
+ linelength = mode->xres * 2;
yuv = false;
break;
case MIPI_RGB666_LP:
pctype = 2;
datatype = MIPI_DSI_PIXEL_STREAM_3BYTE_18;
pixfmt = MIPI_DCS_PIXEL_FMT_24BIT;
- linelength = ch->lcd_modes[0].xres * 3;
+ linelength = mode->xres * 3;
yuv = false;
break;
case MIPI_RGB666:
pctype = 3;
datatype = MIPI_DSI_PACKED_PIXEL_STREAM_18;
pixfmt = MIPI_DCS_PIXEL_FMT_18BIT;
- linelength = (ch->lcd_modes[0].xres * 18 + 7) / 8;
+ linelength = (mode->xres * 18 + 7) / 8;
yuv = false;
break;
case MIPI_BGR888:
pctype = 8;
datatype = MIPI_DSI_PACKED_PIXEL_STREAM_24;
pixfmt = MIPI_DCS_PIXEL_FMT_24BIT;
- linelength = ch->lcd_modes[0].xres * 3;
+ linelength = mode->xres * 3;
yuv = false;
break;
case MIPI_BGR565:
pctype = 9;
datatype = MIPI_DSI_PACKED_PIXEL_STREAM_16;
pixfmt = MIPI_DCS_PIXEL_FMT_16BIT;
- linelength = ch->lcd_modes[0].xres * 2;
+ linelength = mode->xres * 2;
yuv = false;
break;
case MIPI_BGR666_LP:
pctype = 0xa;
datatype = MIPI_DSI_PIXEL_STREAM_3BYTE_18;
pixfmt = MIPI_DCS_PIXEL_FMT_24BIT;
- linelength = ch->lcd_modes[0].xres * 3;
+ linelength = mode->xres * 3;
yuv = false;
break;
case MIPI_BGR666:
pctype = 0xb;
datatype = MIPI_DSI_PACKED_PIXEL_STREAM_18;
pixfmt = MIPI_DCS_PIXEL_FMT_18BIT;
- linelength = (ch->lcd_modes[0].xres * 18 + 7) / 8;
+ linelength = (mode->xres * 18 + 7) / 8;
yuv = false;
break;
case MIPI_YUYV:
pctype = 4;
datatype = MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR16;
pixfmt = MIPI_DCS_PIXEL_FMT_16BIT;
- linelength = ch->lcd_modes[0].xres * 2;
+ linelength = mode->xres * 2;
yuv = true;
break;
case MIPI_UYVY:
pctype = 5;
datatype = MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR16;
pixfmt = MIPI_DCS_PIXEL_FMT_16BIT;
- linelength = ch->lcd_modes[0].xres * 2;
+ linelength = mode->xres * 2;
yuv = true;
break;
case MIPI_YUV420_L:
pctype = 6;
datatype = MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR12;
pixfmt = MIPI_DCS_PIXEL_FMT_12BIT;
- linelength = (ch->lcd_modes[0].xres * 12 + 7) / 8;
+ linelength = (mode->xres * 12 + 7) / 8;
yuv = true;
break;
case MIPI_YUV420:
@@ -224,7 +225,7 @@ static int sh_mipi_setup(struct sh_mipi *mipi, struct sh_mipi_dsi_info *pdata)
datatype = MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR12;
pixfmt = MIPI_DCS_PIXEL_FMT_12BIT;
/* Length of U/V line */
- linelength = (ch->lcd_modes[0].xres + 1) / 2;
+ linelength = (mode->xres + 1) / 2;
yuv = true;
break;
default:
@@ -293,7 +294,7 @@ static int sh_mipi_setup(struct sh_mipi *mipi, struct sh_mipi_dsi_info *pdata)
*/
iowrite32(0x00000006, mipi->linkbase + DTCTR);
/* VSYNC width = 2 (<< 17) */
- iowrite32((ch->lcd_modes[0].vsync_len << pdata->vsynw_offset) |
+ iowrite32((mode->vsync_len << pdata->vsynw_offset) |
(pdata->clksrc << 16) | (pctype << 12) | datatype,
mipi->linkbase + VMCTR1);
@@ -327,7 +328,7 @@ static int sh_mipi_setup(struct sh_mipi *mipi, struct sh_mipi_dsi_info *pdata)
top = linelength << 16; /* RGBLEN */
bottom = 0x00000001;
if (pdata->flags & SH_MIPI_DSI_HSABM) /* HSALEN */
- bottom = (pdata->lane * ch->lcd_modes[0].hsync_len) - 10;
+ bottom = (pdata->lane * mode->hsync_len) - 10;
iowrite32(top | bottom , mipi->linkbase + VMLEN1);
/*
@@ -347,18 +348,18 @@ static int sh_mipi_setup(struct sh_mipi *mipi, struct sh_mipi_dsi_info *pdata)
div = 2;
if (pdata->flags & SH_MIPI_DSI_HFPBM) { /* HBPLEN */
- top = ch->lcd_modes[0].hsync_len + ch->lcd_modes[0].left_margin;
+ top = mode->hsync_len + mode->left_margin;
top = ((pdata->lane * top / div) - 10) << 16;
}
if (pdata->flags & SH_MIPI_DSI_HBPBM) { /* HFPLEN */
- bottom = ch->lcd_modes[0].right_margin;
+ bottom = mode->right_margin;
bottom = (pdata->lane * bottom / div) - 12;
}
- bpp = linelength / ch->lcd_modes[0].xres; /* byte / pixel */
+ bpp = linelength / mode->xres; /* byte / pixel */
if ((pdata->lane / div) > bpp) {
- tmp = ch->lcd_modes[0].xres / bpp; /* output cycle */
- tmp = ch->lcd_modes[0].xres - tmp; /* (input - output) cycle */
+ tmp = mode->xres / bpp; /* output cycle */
+ tmp = mode->xres - tmp; /* (input - output) cycle */
delay = (pdata->lane * tmp);
}
@@ -405,7 +406,7 @@ static int mipi_display_on(struct sh_mobile_lcdc_entity *entity)
if (ret < 0)
goto mipi_display_on_fail1;
- ret = sh_mipi_setup(mipi, pdata);
+ ret = sh_mipi_setup(mipi, &entity->def_mode);
if (ret < 0)
goto mipi_display_on_fail2;
--
1.7.8.6
^ permalink raw reply related
* [PATCH 09/19] fbdev: sh_mipi_dsi: Remove last reference to LCDC platform data
From: Laurent Pinchart @ 2012-08-16 13:00 UTC (permalink / raw)
To: linux-fbdev
The format check that references LCDC platform data isn't needed, as the
mismatch between LCDC and MIPI-DSI platform it detects would result in a
badly displayed image, which isn't worse than a complete failure to
setup the transmitter.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/video/sh_mipi_dsi.c | 18 ------------------
1 files changed, 0 insertions(+), 18 deletions(-)
diff --git a/drivers/video/sh_mipi_dsi.c b/drivers/video/sh_mipi_dsi.c
index f88b204..f496229 100644
--- a/drivers/video/sh_mipi_dsi.c
+++ b/drivers/video/sh_mipi_dsi.c
@@ -131,10 +131,8 @@ static int sh_mipi_setup(struct sh_mipi *mipi, const struct fb_videomode *mode)
{
void __iomem *base = mipi->base;
struct sh_mipi_dsi_info *pdata = mipi->pdev->dev.platform_data;
- struct sh_mobile_lcdc_chan_cfg *ch = pdata->lcd_chan;
u32 pctype, datatype, pixfmt, linelength, vmctr2;
u32 tmp, top, bottom, delay, div;
- bool yuv;
int bpp;
/*
@@ -148,77 +146,66 @@ static int sh_mipi_setup(struct sh_mipi *mipi, const struct fb_videomode *mode)
datatype = MIPI_DSI_PACKED_PIXEL_STREAM_24;
pixfmt = MIPI_DCS_PIXEL_FMT_24BIT;
linelength = mode->xres * 3;
- yuv = false;
break;
case MIPI_RGB565:
pctype = 1;
datatype = MIPI_DSI_PACKED_PIXEL_STREAM_16;
pixfmt = MIPI_DCS_PIXEL_FMT_16BIT;
linelength = mode->xres * 2;
- yuv = false;
break;
case MIPI_RGB666_LP:
pctype = 2;
datatype = MIPI_DSI_PIXEL_STREAM_3BYTE_18;
pixfmt = MIPI_DCS_PIXEL_FMT_24BIT;
linelength = mode->xres * 3;
- yuv = false;
break;
case MIPI_RGB666:
pctype = 3;
datatype = MIPI_DSI_PACKED_PIXEL_STREAM_18;
pixfmt = MIPI_DCS_PIXEL_FMT_18BIT;
linelength = (mode->xres * 18 + 7) / 8;
- yuv = false;
break;
case MIPI_BGR888:
pctype = 8;
datatype = MIPI_DSI_PACKED_PIXEL_STREAM_24;
pixfmt = MIPI_DCS_PIXEL_FMT_24BIT;
linelength = mode->xres * 3;
- yuv = false;
break;
case MIPI_BGR565:
pctype = 9;
datatype = MIPI_DSI_PACKED_PIXEL_STREAM_16;
pixfmt = MIPI_DCS_PIXEL_FMT_16BIT;
linelength = mode->xres * 2;
- yuv = false;
break;
case MIPI_BGR666_LP:
pctype = 0xa;
datatype = MIPI_DSI_PIXEL_STREAM_3BYTE_18;
pixfmt = MIPI_DCS_PIXEL_FMT_24BIT;
linelength = mode->xres * 3;
- yuv = false;
break;
case MIPI_BGR666:
pctype = 0xb;
datatype = MIPI_DSI_PACKED_PIXEL_STREAM_18;
pixfmt = MIPI_DCS_PIXEL_FMT_18BIT;
linelength = (mode->xres * 18 + 7) / 8;
- yuv = false;
break;
case MIPI_YUYV:
pctype = 4;
datatype = MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR16;
pixfmt = MIPI_DCS_PIXEL_FMT_16BIT;
linelength = mode->xres * 2;
- yuv = true;
break;
case MIPI_UYVY:
pctype = 5;
datatype = MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR16;
pixfmt = MIPI_DCS_PIXEL_FMT_16BIT;
linelength = mode->xres * 2;
- yuv = true;
break;
case MIPI_YUV420_L:
pctype = 6;
datatype = MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR12;
pixfmt = MIPI_DCS_PIXEL_FMT_12BIT;
linelength = (mode->xres * 12 + 7) / 8;
- yuv = true;
break;
case MIPI_YUV420:
pctype = 7;
@@ -226,16 +213,11 @@ static int sh_mipi_setup(struct sh_mipi *mipi, const struct fb_videomode *mode)
pixfmt = MIPI_DCS_PIXEL_FMT_12BIT;
/* Length of U/V line */
linelength = (mode->xres + 1) / 2;
- yuv = true;
break;
default:
return -EINVAL;
}
- if ((yuv && ch->interface_type != YUV422) ||
- (!yuv && ch->interface_type != RGB24))
- return -EINVAL;
-
if (!pdata->lane)
return -EINVAL;
--
1.7.8.6
^ permalink raw reply related
* [PATCH 10/19] ARM: mach-shmobile: Remove the unused sh_mipi_dsi_info lcd_chan field
From: Laurent Pinchart @ 2012-08-16 13:00 UTC (permalink / raw)
To: linux-fbdev
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
arch/arm/mach-shmobile/board-ag5evm.c | 3 ---
arch/arm/mach-shmobile/board-ap4evb.c | 3 ---
2 files changed, 0 insertions(+), 6 deletions(-)
diff --git a/arch/arm/mach-shmobile/board-ag5evm.c b/arch/arm/mach-shmobile/board-ag5evm.c
index 5523bc8..7d6b96b 100644
--- a/arch/arm/mach-shmobile/board-ag5evm.c
+++ b/arch/arm/mach-shmobile/board-ag5evm.c
@@ -253,11 +253,8 @@ sh_mipi_set_dot_clock_pck_err:
return ret;
}
-static struct sh_mobile_lcdc_info lcdc0_info;
-
static struct sh_mipi_dsi_info mipidsi0_info = {
.data_format = MIPI_RGB888,
- .lcd_chan = &lcdc0_info.ch[0],
.channel = LCDC_CHAN_MAINLCD,
.lane = 2,
.vsynw_offset = 20,
diff --git a/arch/arm/mach-shmobile/board-ap4evb.c b/arch/arm/mach-shmobile/board-ap4evb.c
index 5fbd176..09f4e0a 100644
--- a/arch/arm/mach-shmobile/board-ap4evb.c
+++ b/arch/arm/mach-shmobile/board-ap4evb.c
@@ -527,11 +527,8 @@ static struct resource mipidsi0_resources[] = {
},
};
-static struct sh_mobile_lcdc_info lcdc_info;
-
static struct sh_mipi_dsi_info mipidsi0_info = {
.data_format = MIPI_RGB888,
- .lcd_chan = &lcdc_info.ch[0],
.channel = LCDC_CHAN_MAINLCD,
.lane = 2,
.vsynw_offset = 17,
--
1.7.8.6
^ permalink raw reply related
* [PATCH 11/19] fbdev: sh_mipi_dsi: Remove the unused sh_mipi_dsi_info lcd_chan field
From: Laurent Pinchart @ 2012-08-16 13:00 UTC (permalink / raw)
To: linux-fbdev
The field is unused, remove it from the structure.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
include/video/sh_mipi_dsi.h | 3 ---
1 files changed, 0 insertions(+), 3 deletions(-)
diff --git a/include/video/sh_mipi_dsi.h b/include/video/sh_mipi_dsi.h
index eae71d9..a01f197 100644
--- a/include/video/sh_mipi_dsi.h
+++ b/include/video/sh_mipi_dsi.h
@@ -25,8 +25,6 @@ enum sh_mipi_dsi_data_fmt {
MIPI_YUV420,
};
-struct sh_mobile_lcdc_chan_cfg;
-
#define SH_MIPI_DSI_HSABM (1 << 0)
#define SH_MIPI_DSI_HBPBM (1 << 1)
#define SH_MIPI_DSI_HFPBM (1 << 2)
@@ -47,7 +45,6 @@ struct sh_mobile_lcdc_chan_cfg;
struct sh_mipi_dsi_info {
enum sh_mipi_dsi_data_fmt data_format;
- struct sh_mobile_lcdc_chan_cfg *lcd_chan;
int channel;
int lane;
unsigned long flags;
--
1.7.8.6
^ permalink raw reply related
* [PATCH 12/19] fbdev: sh_mobile_lcdc: Store the backlight brightness internally
From: Laurent Pinchart @ 2012-08-16 13:00 UTC (permalink / raw)
To: linux-fbdev
There's no need to query the hardware for the currenty brightness value
through a platform data callback when we can cache the value internally
in the LCDC driver.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/video/sh_mobile_lcdcfb.c | 3 ++-
drivers/video/sh_mobile_lcdcfb.h | 1 +
2 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/video/sh_mobile_lcdcfb.c b/drivers/video/sh_mobile_lcdcfb.c
index c169581..dd8dd41 100644
--- a/drivers/video/sh_mobile_lcdcfb.c
+++ b/drivers/video/sh_mobile_lcdcfb.c
@@ -2282,6 +2282,7 @@ static int sh_mobile_lcdc_update_bl(struct backlight_device *bdev)
bdev->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
brightness = 0;
+ ch->bl_brightness = brightness;
return ch->cfg->bl_info.set_brightness(brightness);
}
@@ -2289,7 +2290,7 @@ static int sh_mobile_lcdc_get_brightness(struct backlight_device *bdev)
{
struct sh_mobile_lcdc_chan *ch = bl_get_data(bdev);
- return ch->cfg->bl_info.get_brightness();
+ return ch->bl_brightness;
}
static int sh_mobile_lcdc_check_fb(struct backlight_device *bdev,
diff --git a/drivers/video/sh_mobile_lcdcfb.h b/drivers/video/sh_mobile_lcdcfb.h
index 0f92f65..f839ade 100644
--- a/drivers/video/sh_mobile_lcdcfb.h
+++ b/drivers/video/sh_mobile_lcdcfb.h
@@ -94,6 +94,7 @@ struct sh_mobile_lcdc_chan {
/* Backlight */
struct backlight_device *bl;
+ unsigned int bl_brightness;
/* FB */
struct fb_info *info;
--
1.7.8.6
^ permalink raw reply related
* [PATCH 13/19] ARM: mach-shmobile: mackerel: Removed unused get_brightness callback
From: Laurent Pinchart @ 2012-08-16 13:00 UTC (permalink / raw)
To: linux-fbdev
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
arch/arm/mach-shmobile/board-mackerel.c | 6 ------
1 files changed, 0 insertions(+), 6 deletions(-)
diff --git a/arch/arm/mach-shmobile/board-mackerel.c b/arch/arm/mach-shmobile/board-mackerel.c
index b577f7c..10d9a63 100644
--- a/arch/arm/mach-shmobile/board-mackerel.c
+++ b/arch/arm/mach-shmobile/board-mackerel.c
@@ -365,11 +365,6 @@ static int mackerel_set_brightness(int brightness)
return 0;
}
-static int mackerel_get_brightness(void)
-{
- return gpio_get_value(GPIO_PORT31);
-}
-
static const struct sh_mobile_meram_cfg lcd_meram_cfg = {
.icb[0] = {
.meram_size = 0x40,
@@ -398,7 +393,6 @@ static struct sh_mobile_lcdc_info lcdc_info = {
.name = "sh_mobile_lcdc_bl",
.max_brightness = 1,
.set_brightness = mackerel_set_brightness,
- .get_brightness = mackerel_get_brightness,
},
.meram_cfg = &lcd_meram_cfg,
}
--
1.7.8.6
^ permalink raw reply related
* [PATCH 14/19] sh: ap325rxa: Remove unused get_brightness LCDC callback
From: Laurent Pinchart @ 2012-08-16 13:00 UTC (permalink / raw)
To: linux-fbdev
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
arch/sh/boards/mach-ap325rxa/setup.c | 6 ------
1 files changed, 0 insertions(+), 6 deletions(-)
diff --git a/arch/sh/boards/mach-ap325rxa/setup.c b/arch/sh/boards/mach-ap325rxa/setup.c
index f33ebf4..8a5ea1a 100644
--- a/arch/sh/boards/mach-ap325rxa/setup.c
+++ b/arch/sh/boards/mach-ap325rxa/setup.c
@@ -171,11 +171,6 @@ static int ap320_wvga_set_brightness(int brightness)
return 0;
}
-static int ap320_wvga_get_brightness(void)
-{
- return gpio_get_value(GPIO_PTS3);
-}
-
static void ap320_wvga_power_on(void)
{
msleep(100);
@@ -224,7 +219,6 @@ static struct sh_mobile_lcdc_info lcdc_info = {
.name = "sh_mobile_lcdc_bl",
.max_brightness = 1,
.set_brightness = ap320_wvga_set_brightness,
- .get_brightness = ap320_wvga_get_brightness,
},
}
};
--
1.7.8.6
^ permalink raw reply related
* [PATCH 15/19] sh: ecovec24: Remove unused get_brightness LCDC callback
From: Laurent Pinchart @ 2012-08-16 13:00 UTC (permalink / raw)
To: linux-fbdev
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
arch/sh/boards/mach-ecovec24/setup.c | 6 ------
1 files changed, 0 insertions(+), 6 deletions(-)
diff --git a/arch/sh/boards/mach-ecovec24/setup.c b/arch/sh/boards/mach-ecovec24/setup.c
index 4158d70..d9b0a76 100644
--- a/arch/sh/boards/mach-ecovec24/setup.c
+++ b/arch/sh/boards/mach-ecovec24/setup.c
@@ -319,11 +319,6 @@ static int ecovec24_set_brightness(int brightness)
return 0;
}
-static int ecovec24_get_brightness(void)
-{
- return gpio_get_value(GPIO_PTR1);
-}
-
static struct sh_mobile_lcdc_info lcdc_info = {
.ch[0] = {
.interface_type = RGB18,
@@ -337,7 +332,6 @@ static struct sh_mobile_lcdc_info lcdc_info = {
.name = "sh_mobile_lcdc_bl",
.max_brightness = 1,
.set_brightness = ecovec24_set_brightness,
- .get_brightness = ecovec24_get_brightness,
},
}
};
--
1.7.8.6
^ permalink raw reply related
* [PATCH 16/19] fbdev: sh_mobile_lcdc: Remove unused get_brightness pdata callback
From: Laurent Pinchart @ 2012-08-16 13:00 UTC (permalink / raw)
To: linux-fbdev
The callback isn't used anymore, remove it.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
include/video/sh_mobile_lcdc.h | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/include/video/sh_mobile_lcdc.h b/include/video/sh_mobile_lcdc.h
index ff43ffc..2605fa8 100644
--- a/include/video/sh_mobile_lcdc.h
+++ b/include/video/sh_mobile_lcdc.h
@@ -163,7 +163,6 @@ struct sh_mobile_lcdc_bl_info {
const char *name;
int max_brightness;
int (*set_brightness)(int brightness);
- int (*get_brightness)(void);
};
struct sh_mobile_lcdc_overlay_cfg {
--
1.7.8.6
^ permalink raw reply related
* [PATCH 17/19] ARM: mach-shmobile: ag5evm: Use the backlight API for brightness control
From: Laurent Pinchart @ 2012-08-16 13:00 UTC (permalink / raw)
To: linux-fbdev
Don't hook up brightness control in the display on/off operations, use
the backlight API instead.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
arch/arm/mach-shmobile/board-ag5evm.c | 45 +++++++++++++++++++++-----------
1 files changed, 29 insertions(+), 16 deletions(-)
diff --git a/arch/arm/mach-shmobile/board-ag5evm.c b/arch/arm/mach-shmobile/board-ag5evm.c
index 7d6b96b..826ac13 100644
--- a/arch/arm/mach-shmobile/board-ag5evm.c
+++ b/arch/arm/mach-shmobile/board-ag5evm.c
@@ -281,28 +281,38 @@ static unsigned char lcd_backlight_seq[3][2] = {
{ 0x03, 0x01 },
};
-static void lcd_backlight_on(void)
+static int lcd_backlight_set_brightness(int brightness)
{
- struct i2c_adapter *a;
+ struct i2c_adapter *adap;
struct i2c_msg msg;
- int k;
+ unsigned int i;
+ int ret;
+
+ if (brightness = 0) {
+ /* Reset the chip */
+ gpio_set_value(GPIO_PORT235, 0);
+ mdelay(24);
+ gpio_set_value(GPIO_PORT235, 1);
+ return 0;
+ }
+
+ adap = i2c_get_adapter(1);
+ if (adap = NULL)
+ return -ENODEV;
- a = i2c_get_adapter(1);
- for (k = 0; a && k < 3; k++) {
+ for (i = 0; i < ARRAY_SIZE(lcd_backlight_seq); i++) {
msg.addr = 0x6d;
- msg.buf = &lcd_backlight_seq[k][0];
+ msg.buf = &lcd_backlight_seq[i][0];
msg.len = 2;
msg.flags = 0;
- if (i2c_transfer(a, &msg, 1) != 1)
+
+ ret = i2c_transfer(adap, &msg, 1);
+ if (ret < 0)
break;
}
-}
-static void lcd_backlight_reset(void)
-{
- gpio_set_value(GPIO_PORT235, 0);
- mdelay(24);
- gpio_set_value(GPIO_PORT235, 1);
+ i2c_put_adapter(adap);
+ return ret < 0 ? ret : 0;
}
/* LCDC0 */
@@ -334,8 +344,11 @@ static struct sh_mobile_lcdc_info lcdc0_info = {
.panel_cfg = {
.width = 44,
.height = 79,
- .display_on = lcd_backlight_on,
- .display_off = lcd_backlight_reset,
+ },
+ .bl_info = {
+ .name = "sh_mobile_lcdc_bl",
+ .max_brightness = 1,
+ .set_brightness = lcd_backlight_set_brightness,
},
.tx_dev = &mipidsi0_device,
}
@@ -545,7 +558,7 @@ static void __init ag5evm_init(void)
/* LCD backlight controller */
gpio_request(GPIO_PORT235, NULL); /* RESET */
gpio_direction_output(GPIO_PORT235, 0);
- lcd_backlight_reset();
+ lcd_backlight_set_brightness(0);
/* enable SDHI0 on CN15 [SD I/F] */
gpio_request(GPIO_FN_SDHIWP0, NULL);
--
1.7.8.6
^ permalink raw reply related
* [PATCH 18/19] sh: kfr2r09: Use the backlight API for brightness control
From: Laurent Pinchart @ 2012-08-16 13:00 UTC (permalink / raw)
To: linux-fbdev
Don't hook up brightness control in the display on/off operations, use
the backlight API instead.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
arch/sh/boards/mach-kfr2r09/lcd_wqvga.c | 16 +++-------------
arch/sh/boards/mach-kfr2r09/setup.c | 7 +++++--
arch/sh/include/mach-kfr2r09/mach/kfr2r09.h | 6 ++----
3 files changed, 10 insertions(+), 19 deletions(-)
diff --git a/arch/sh/boards/mach-kfr2r09/lcd_wqvga.c b/arch/sh/boards/mach-kfr2r09/lcd_wqvga.c
index c148b36..c620503 100644
--- a/arch/sh/boards/mach-kfr2r09/lcd_wqvga.c
+++ b/arch/sh/boards/mach-kfr2r09/lcd_wqvga.c
@@ -283,7 +283,7 @@ void kfr2r09_lcd_start(void *sohandle, struct sh_mobile_lcdc_sys_bus_ops *so)
#define MAIN_MLED4 0x40
#define MAIN_MSW 0x80
-static int kfr2r09_lcd_backlight(int on)
+int kfr2r09_lcd_set_brightness(int brightness)
{
struct i2c_adapter *a;
struct i2c_msg msg;
@@ -295,7 +295,7 @@ static int kfr2r09_lcd_backlight(int on)
return -ENODEV;
buf[0] = 0x00;
- if (on)
+ if (brightness)
buf[1] = CTRL_CPSW | CTRL_C10 | CTRL_CKSW;
else
buf[1] = 0;
@@ -309,7 +309,7 @@ static int kfr2r09_lcd_backlight(int on)
return -ENODEV;
buf[0] = 0x01;
- if (on)
+ if (brightness)
buf[1] = MAIN_MSW | MAIN_MLED4 | 0x0c;
else
buf[1] = 0;
@@ -324,13 +324,3 @@ static int kfr2r09_lcd_backlight(int on)
return 0;
}
-
-void kfr2r09_lcd_on(void)
-{
- kfr2r09_lcd_backlight(1);
-}
-
-void kfr2r09_lcd_off(void)
-{
- kfr2r09_lcd_backlight(0);
-}
diff --git a/arch/sh/boards/mach-kfr2r09/setup.c b/arch/sh/boards/mach-kfr2r09/setup.c
index 158c917..ecf40b8 100644
--- a/arch/sh/boards/mach-kfr2r09/setup.c
+++ b/arch/sh/boards/mach-kfr2r09/setup.c
@@ -156,8 +156,11 @@ static struct sh_mobile_lcdc_info kfr2r09_sh_lcdc_info = {
.height = 58,
.setup_sys = kfr2r09_lcd_setup,
.start_transfer = kfr2r09_lcd_start,
- .display_on = kfr2r09_lcd_on,
- .display_off = kfr2r09_lcd_off,
+ },
+ .bl_info = {
+ .name = "sh_mobile_lcdc_bl",
+ .max_brightness = 1,
+ .set_brightness = kfr2r09_lcd_set_brightness,
},
.sys_bus_cfg = {
.ldmt2r = 0x07010904,
diff --git a/arch/sh/include/mach-kfr2r09/mach/kfr2r09.h b/arch/sh/include/mach-kfr2r09/mach/kfr2r09.h
index ba3d93d..c20c9e5 100644
--- a/arch/sh/include/mach-kfr2r09/mach/kfr2r09.h
+++ b/arch/sh/include/mach-kfr2r09/mach/kfr2r09.h
@@ -4,15 +4,13 @@
#include <video/sh_mobile_lcdc.h>
#if defined(CONFIG_FB_SH_MOBILE_LCDC) || defined(CONFIG_FB_SH_MOBILE_LCDC_MODULE)
-void kfr2r09_lcd_on(void);
-void kfr2r09_lcd_off(void);
+int kfr2r09_lcd_set_brightness(int brightness);
int kfr2r09_lcd_setup(void *sys_ops_handle,
struct sh_mobile_lcdc_sys_bus_ops *sys_ops);
void kfr2r09_lcd_start(void *sys_ops_handle,
struct sh_mobile_lcdc_sys_bus_ops *sys_ops);
#else
-static void kfr2r09_lcd_on(void) {}
-static void kfr2r09_lcd_off(void) {}
+static int kfr2r09_lcd_set_brightness(int brightness) {}
static int kfr2r09_lcd_setup(void *sys_ops_handle,
struct sh_mobile_lcdc_sys_bus_ops *sys_ops)
{
--
1.7.8.6
^ permalink raw reply related
* [PATCH 19/19] fbdev: sh_mobile_lcdc: Make sh_mobile_lcdc_sys_bus_ops static
From: Laurent Pinchart @ 2012-08-16 13:00 UTC (permalink / raw)
To: linux-fbdev
The structure isn't used outside of its compilation unit, make it
static.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/video/sh_mobile_lcdcfb.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/video/sh_mobile_lcdcfb.c b/drivers/video/sh_mobile_lcdcfb.c
index dd8dd41..e78fe4b 100644
--- a/drivers/video/sh_mobile_lcdcfb.c
+++ b/drivers/video/sh_mobile_lcdcfb.c
@@ -438,7 +438,7 @@ static unsigned long lcdc_sys_read_data(void *handle)
return lcdc_read(ch->lcdc, _LDDRDR) & LDDRDR_DRD_MASK;
}
-struct sh_mobile_lcdc_sys_bus_ops sh_mobile_lcdc_sys_bus_ops = {
+static struct sh_mobile_lcdc_sys_bus_ops sh_mobile_lcdc_sys_bus_ops = {
lcdc_sys_write_index,
lcdc_sys_write_data,
lcdc_sys_read_data,
--
1.7.8.6
^ permalink raw reply related
* Re: [PATCH 6/6] OMAPDSS: VENC: Maintian copy of video output polarity in private data
From: Tomi Valkeinen @ 2012-08-16 13:09 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <502CE727.6070906@ti.com>
[-- Attachment #1: Type: text/plain, Size: 4329 bytes --]
On Thu, 2012-08-16 at 17:57 +0530, Archit Taneja wrote:
> On Thursday 16 August 2012 05:08 PM, Tomi Valkeinen wrote:
> > On Thu, 2012-08-16 at 13:06 +0530, Archit Taneja wrote:
> >> The VENC driver currently relies on the omap_dss_device struct to configure the
> >> video output polarity. This makes the VENC interface driver dependent on the
> >> omap_dss_device struct.
> >>
> >> Make the VENC driver data maintain it's own polarity field. A panel driver
> >> is expected to call omapdss_venc_set_vid_out_polarity() before enabling the
> >> interface.
> >>
> >> Signed-off-by: Archit Taneja <archit@ti.com>
> >> ---
> >> drivers/video/omap2/dss/dss.h | 2 ++
> >> drivers/video/omap2/dss/venc.c | 13 ++++++++++++-
> >> drivers/video/omap2/dss/venc_panel.c | 6 ++++++
> >> 3 files changed, 20 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
> >> index c17d298..b2cf5530 100644
> >> --- a/drivers/video/omap2/dss/dss.h
> >> +++ b/drivers/video/omap2/dss/dss.h
> >> @@ -479,6 +479,8 @@ u32 omapdss_venc_get_wss(struct omap_dss_device *dssdev);
> >> int omapdss_venc_set_wss(struct omap_dss_device *dssdev, u32 wss);
> >> void omapdss_venc_set_type(struct omap_dss_device *dssdev,
> >> enum omap_dss_venc_type type);
> >> +void omapdss_venc_set_vid_out_polarity(struct omap_dss_device *dssdev,
> >> + enum omap_dss_signal_level vid_out_pol);
> >> int venc_panel_init(void);
> >> void venc_panel_exit(void);
> >>
> >> diff --git a/drivers/video/omap2/dss/venc.c b/drivers/video/omap2/dss/venc.c
> >> index 2d90fcf..8cb372f 100644
> >> --- a/drivers/video/omap2/dss/venc.c
> >> +++ b/drivers/video/omap2/dss/venc.c
> >> @@ -303,6 +303,7 @@ static struct {
> >>
> >> struct omap_video_timings timings;
> >> enum omap_dss_venc_type type;
> >> + enum omap_dss_signal_level vid_out_pol;
> >> } venc;
> >>
> >> static inline void venc_write_reg(int idx, u32 val)
> >> @@ -447,7 +448,7 @@ static int venc_power_on(struct omap_dss_device *dssdev)
> >> else /* S-Video */
> >> l |= (1 << 0) | (1 << 2);
> >>
> >> - if (dssdev->phy.venc.invert_polarity == false)
> >> + if (venc.vid_out_pol == OMAPDSS_SIG_ACTIVE_HIGH)
> >> l |= 1 << 3;
> >
> > Are you sure this is correct? I know practically nothing about analog
> > TV, but the TRM doesn't seem to say much about that bit, except it can
> > be used to "invert the video output". It doesn't say there's an
> > active/inactive level for the signal.
>
> Well, the code works :), I'm also not totally sure about whether it
You could put there APPLE and ORANGE enum values, and it'd still work =)
> should be represented as a 2-level signal, it seemed like it would be
> nicer to give it a signal level rather than keeping it as a bool, which
> could vary for other SoC's?
It may be really a bool. TRM says "This may be used to
correct for inversion in an external video amplifier". I don't think
there are any digital on/off signals in analog video output, so I'm
guessing it's really inverting (some parts of) the analog signal. If so,
a boolean invert field sounds correct to me.
Actually, check this out:
http://books.google.fi/books?id=P6BlcWaizHUC&pg=PT81&lpg=PT81&dq=composite+video+polarity&source=bl&ots=-gsl0Exv5R&sig=gMylEnoV9ozRwM4RX2iQFqhRpP8&hl=en&sa=X&ei=0u8sUIe3KYXh4QTf9YDQBA&redir_esc=y#v=onepage&q=composite%20video%20polarity&f=false
A monster url, here's a tinyurl version: http://tinyurl.com/clceb6t
2.16 section there shows signal polarities. I'm not sure if it's the
same one that we're discussing, but sounds like it.
I don't think ACTIVE_LOW/HIGH fits into that kind of polarity. Perhaps a
bool is not quite right for it either, as I'm not sure there's a
"normal" polarity. But I'd go forward with out current bool, and fix it
when somebody who understands this stuff tells us what to do =).
> I am considering not to pass this via the panel driver for now, I don't
> know if all VENC IPs needs to do this, maybe it's okay to leave this
> dssdev reference for now?
I don't know if other VENC IPs support this or not, but the TRM refers
to external amplifier, so it sounds like a thing that would exist on
other IPs also.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v4 1/3] Runtime Interpreted Power Sequences
From: Mark Brown @ 2012-08-16 14:14 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Stephen Warren, Thierry Reding, Simon Glass, Grant Likely,
Rob Herring, Anton Vorontsov, David Woodhouse, Arnd Bergmann,
Leela Krishna Amudala, linux-tegra, linux-kernel, linux-fbdev,
devicetree-discuss, linux-doc
In-Reply-To: <1345097337-24170-2-git-send-email-acourbot@nvidia.com>
On Thu, Aug 16, 2012 at 03:08:55PM +0900, Alexandre Courbot wrote:
> + power-off-sequence {
> + step0 {
> + gpio = "enable";
> + disable;
I'd change the name to "reset" or something in the example - avoids any
confusion with the action.
> +#ifdef CONFIG_REGULATOR
> + case POWER_SEQ_REGULATOR:
> + if (pdata->regulator.enable)
> + err = regulator_enable(step->resource->regulator);
> + else
> + err = regulator_disable(step->resource->regulator);
> + break;
The regulator and GPIO APIs should stub themselves out adequately to not
need the ifdefs at least for regulator I'd use the stubs since...
> + /*
> + * should never happen unless the sequence includes a step which
> + * type does not have support compiled in
> + */
> + default:
> + return -EINVAL;
...this probably isn't what's meant. It might also be nice to have
support for bulk_enable() but that's definitely something that could be
added later.
> + err = power_seq_step_run(&seq->steps[cpt]);
> + if (err) {
> + dev_err(dev, "error %d while running power sequence!\n",
> + err);
> + return err;
I'd also log at least the step number, it'll make diagnostics easier.
^ permalink raw reply
* Re: [PATCH 0/7] HID: picoLCD updates
From: Bruno Prémont @ 2012-08-16 16:30 UTC (permalink / raw)
To: Jiri Kosina; +Cc: linux-input, linux-kernel, linux-fbdev
In-Reply-To: <alpine.LNX.2.00.1208152329580.7026@pobox.suse.cz>
On Wed, 15 August 2012 Jiri Kosina <jkosina@suse.cz> wrote:
> On Wed, 15 Aug 2012, Bruno Prémont wrote:
> > > I see. Alan Stern has fixed a huge pile of things in this area in 3.6-rc1.
> > > I have expected all of those to actually be on theoretical problems not
> > > ever having happened in the wild, but it might be that you are actually
> > > chasing on of those.
> > >
> > > Could you please retest with latest Linus' tree (or at least eb055fd0560b)
> > > to see whether this hasn't actually been fixed already by Alan's series?
> >
> > I've started trying that out, it seems Alan's work improved things.
> >
> > For the first few attempts I have not seen SLAB corruptions, though after
> > a few rounds I hit accumulation of the following messages:
> > [ 297.174828] hid-picolcd 0003:04D8:C002.0003: output queue full
> > [ 297.181098] hid-picolcd 0003:04D8:C002.0003: output queue full
> > [ 297.187820] hid-picolcd 0003:04D8:C002.0003: output queue full
> > [ 297.194087] hid-picolcd 0003:04D8:C002.0003: output queue full
> >
> > with sporadically in between:
> > [ 292.668019] hid-picolcd 0003:04D8:C002.0003: usb_submit_urb(out) failed: -1
> >
> > At first glance I think the queue filling up and never draining is caused
> > by hid_hw_stop() stalling the queue and the time between both being just too
> > short.
>
> I don't really understand this explanation. Once usb_kill_urb() returns,
> the URB should be available for future use (and therefore all queues
> completely drained).
I won't have time today to check, though my guess is that on each
echo $usb-id > bind; echo $usb-id > unbind
under /sys/bus/hid/drivers/hid-picolcd/ the USB urb queue fills a bit does
not get cleared.
Is usb_kill_urb() called when unbinding just the specific hid driver? If so
my short timing between bind/unbind must be triggering something else...
Otherwise I'm missing something as at first time I got no "output queue full"
messages, but as I repeated the bind/unbind sequences the prints per bind/unbind
iteration increased in number.
Anyhow, on Friday evening/week-end I will continue digging and report back with my
findings.
Thanks,
Bruno
^ permalink raw reply
* Re: [PATCH 0/7] HID: picoLCD updates
From: Jiri Kosina @ 2012-08-16 16:47 UTC (permalink / raw)
To: Bruno Prémont; +Cc: linux-input, linux-kernel, linux-fbdev
In-Reply-To: <20120816183021.68c37f37@neptune.home>
On Thu, 16 Aug 2012, Bruno Prémont wrote:
> > I don't really understand this explanation. Once usb_kill_urb() returns,
> > the URB should be available for future use (and therefore all queues
> > completely drained).
>
> I won't have time today to check, though my guess is that on each
> echo $usb-id > bind; echo $usb-id > unbind
> under /sys/bus/hid/drivers/hid-picolcd/ the USB urb queue fills a bit does
> not get cleared.
>
> Is usb_kill_urb() called when unbinding just the specific hid driver?
Yup, through hid_hw_stop() -> usbhid_stop().
> If so my short timing between bind/unbind must be triggering something
> else...
>
> Otherwise I'm missing something as at first time I got no "output queue full"
> messages, but as I repeated the bind/unbind sequences the prints per bind/unbind
> iteration increased in number.
>
> Anyhow, on Friday evening/week-end I will continue digging and report back with my
> findings.
Thank you, Bruno.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v4 1/3] Runtime Interpreted Power Sequences
From: Stephen Warren @ 2012-08-16 18:38 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Stephen Warren, Thierry Reding, Simon Glass, Grant Likely,
Rob Herring, Mark Brown, Anton Vorontsov, David Woodhouse,
Arnd Bergmann, Leela Krishna Amudala, linux-tegra, linux-kernel,
linux-fbdev, devicetree-discuss, linux-doc
In-Reply-To: <1345097337-24170-2-git-send-email-acourbot@nvidia.com>
On 08/16/2012 12:08 AM, Alexandre Courbot wrote:
> Some device drivers (panel backlights especially) need to follow precise
> sequences for powering on and off, involving gpios, regulators, PWMs
> with a precise powering order and delays to respect between each steps.
> These sequences are board-specific, and do not belong to a particular
> driver - therefore they have been performed by board-specific hook
> functions to far.
>
> With the advent of the device tree and of ARM kernels that are not
> board-tied, we cannot rely on these board-specific hooks anymore but
> need a way to implement these sequences in a portable manner. This patch
> introduces a simple interpreter that can execute such power sequences
> encoded either as platform data or within the device tree.
> diff --git a/Documentation/devicetree/bindings/power_seq/power_seq.txt b/Documentation/devicetree/bindings/power_seq/power_seq.txt
> +Specifying Power Sequences in the Device Tree
> +======================> +In the device tree, power sequences are specified as sub-nodes of the device
> +node and reference resources declared by that device.
> +
> +For an introduction about runtime interpreted power sequences, see
> +Documentation/power/power_seq.txt and include/linux/power_seq.h.
Device tree bindings shouldn't reference Linux documentation; the
bindings are supposed to be OS-agnostic.
> +Power Sequences Structure
> +-------------------------
> +Power sequences are sub-nodes that are named such as the device driver can find
> +them. The driver's documentation should list the sequence names it recognizes.
That's a little roundabout. That might be better as simply:
Valid power sequence names are defined by each device's binding. For a
power sequence named "foo", a node named "foo-power-sequence" defines
that sequence.
> +Inside a power sequence node are sub-nodes that describe the different steps
> +of the sequence. Each step must be named sequentially, with the first step
> +named step0, the second step1, etc. Failure to follow this rule will result in a
> +parsing error.
Node names shouldn't be interpreted by the code; nodes should all be
named after the type of object the represent. Hence, every step should
be named just "step" for example.
The node's unit address (@0) should be used to distinguish the nodes.
This requires reg properties within each node to match the unit address,
and hence #address-cells and #size-cells properties in the power
sequence itself.
Note that this somewhat conflicts with accessing the top-level power
sequence by name too; perhaps that should be re-thought too. I must
admit this DT rule kinda sucks.
> +Power Sequences Steps
> +---------------------
> +Every step of a sequence describes an action to be performed on a resource. It
> +generally includes a property named after the resource type, and which value
> +references the resource to be used. Depending on the resource type, additional
> +properties can be defined to control the action to be performed.
I think you need to add a property that indicates what type of resource
each step applies to. Sure, this is implicit in that if a "gpio"
property exists, the step is a GPIO step, but in order to make that
work, you'd have to search each node for each possible resource type's
property name. It'd be far better to read a single type="gpio" property,
then parse the step based on that.
> +Example
> +-------
> +Here are example sequences declared within a backlight device that use all the
> +supported resources types:
> +
> + backlight {
> + compatible = "pwm-backlight";
> + ...
> +
> + /* resources used by the sequences */
> + pwms = <&pwm 2 5000000>;
> + pwm-names = "backlight";
> + power-supply = <&backlight_reg>;
> + enable-gpio = <&gpio 28 0>;
> +
> + power-on-sequence {
> + step0 {
> + regulator = "power";
> + enable;
> + };
> + step1 {
> + delay = <10000>;
> + };
> + step2 {
> + pwm = "backlight";
> + enable;
> + };
> + step3 {
> + gpio = "enable";
> + enable;
> + };
> + };
> +
> + power-off-sequence {
> + step0 {
> + gpio = "enable";
> + disable;
> + };
> + step1 {
> + pwm = "backlight";
> + disable;
> + };
> + step2 {
> + delay = <10000>;
> + };
> + step3 {
> + regulator = "power";
> + disable;
> + };
> + };
> + };
I notice that for clocks, pwms, and interrupts, the initial step of the
lookup is via a single property that lists all know resources of that
type. Regulators and GPIOs don't follow this style though. Using the
same mechanism for power-sequences would yield something like the
following, which would avoid the "node names must be significant" issue
I mention above, although it does make everything rather more wordy.
[start my proposal]
> backlight {
> compatible = "pwm-backlight";
>
> /* resources used by the sequences */
> pwms = <&pwm 2 5000000>;
> pwm-names = "backlight";
> power-supply = <&backlight_reg>;
> bl-enable-gpio = <&gpio 28 0>;
> pwr-seqs = <&bl_on &bl_off>;
> pwr-seq-names = "on", "off";
>
> #address-cells = <1>;
> #size-cells = <0>;
>
> bl_on: pwr-seq@0 {
> reg = <0>;
> #address-cells = <1>;
> #size-cells = <0>;
> step@0 {
> reg = <0>;
> type = "regulator";
> regulator = "power";
> enable;
> };
> step@1 {
> reg = <1>;
> type = "delay";
> delay = <10000>;
> };
> step@2 {
> reg = <2>;
> type = "pwm";
> pwm = "backlight";
> enable;
> };
> step@3 {
> reg = <3>;
> type = "gpio";
> gpio = "bl-enable";
> enable;
> };
> };
>
> bl_off: pwr-seq@1 {
> reg = <1>;
> #address-cells = <1>;
> #size-cells = <0>;
> step@0 {
> reg = <0>;
> type = "gpio";
> gpio = "bl-enable";
> disable;
> };
> step@1 {
> reg = <1>;
> type = "pwm";
> pwm = "backlight";
> disable;
> };
> step@2 {
> reg = <2>;
> type = "delay";
> delay = <10000>;
> };
> step@3 {
> reg = <3>;
> type = "regulator";
> regulator = "power";
> disable;
> };
> };
> };
>
[end my proposal]
> diff --git a/Documentation/power/power_seq.txt b/Documentation/power/power_seq.txt
> +Usage by Drivers and Resources Management
> +-----------------------------------------
> +Power sequences make use of resources that must be properly allocated and
> +managed. The power_seq_build() function builds a power sequence from the
> +platform data. It also takes care of resolving and allocating the resources
> +referenced by the sequence if needed:
> +
> + struct power_seq *power_seq_build(struct device *dev, struct list_head *ress,
> + struct platform_power_seq *pseq);
> +
> +The 'dev' argument is the device in the name of which the resources are to be
> +allocated.
> +
> +The 'ress' argument is a list to which the resolved resources are appended. This
> +avoids allocating a resource referenced in several power sequences multiple
> +times.
I see in other parts of the thread, there has been discussion re:
needing the separate ress parameter, and requiring the driver to pass
this in to multiple power_seq_build calls.
The way the pinctrl subsystem solved this was to have a single function
that parsed all pinctrl setting (c.f. all power sequences) at once, and
return a single handle. Later, the driver passes this handle
pinctrl_lookup_state(), along with the requested state (c.f. sequence
name) to search for, and finally passes that handle to
pinctrl_select_state(). Doing something similar here would result in:
struct power_seqs *power_seq_get(struct device *dev);
void power_seq_put(struct power_seqs *seqs);
struct power_seq *power_seq_lookup(struct power_seqs *seqs,
const char *name);
int power_seq_executestruct power_seq *seq);
struct power_seqs *devm_power_seq_get(struct device *dev);
void devm_power_seq_put(struct power_seqs *seqs);
> +On success, the function returns a devm allocated resolved sequence that is
Perhaps the function should be named devm_power_seq_build(), in order to
make this obvious to people who only read the client code, and not this
documentation.
> +ready to be passed to power_seq_run(). In case of failure, and error code is
> +returned.
> +
> +A resolved power sequence returned by power_seq_build can be run by
> +power_run_run():
> +
> + int power_seq_run(power_seq *seq);
> +
> +It returns 0 if the sequence has successfully been run, or an error code if a
> +problem occured.
> +
> +There is no need to explicitly free the resources used by the sequence as they
> +are devm-allocated.
^ permalink raw reply
* Re: [PATCH v4 2/3] pwm_backlight: use power sequences
From: Stephen Warren @ 2012-08-16 18:42 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Stephen Warren, Thierry Reding, Simon Glass, Grant Likely,
Rob Herring, Mark Brown, Anton Vorontsov, David Woodhouse,
Arnd Bergmann, Leela Krishna Amudala, linux-tegra, linux-kernel,
linux-fbdev, devicetree-discuss, linux-doc
In-Reply-To: <1345097337-24170-3-git-send-email-acourbot@nvidia.com>
On 08/16/2012 12:08 AM, Alexandre Courbot wrote:
> Make use of the power sequences specified in the device tree or platform
> data to control how the backlight is powered on and off.
> +++ b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
> Required properties:
> - compatible: "pwm-backlight"
> - - pwms: OF device-tree PWM specification (see PWM binding[0])
> - brightness-levels: Array of distinct brightness levels. Typically these
> are in the range from 0 to 255, but any range starting at 0 will do.
> The actual brightness level (PWM duty cycle) will be interpolated
> @@ -10,19 +9,72 @@ Required properties:
> last value in the array represents a 100% duty cycle (brightest).
> - default-brightness-level: the default brightness level (index into the
> array defined by the "brightness-levels" property)
> + - pwms: OF device-tree PWM specification (see PWM binding[0]). Exactly one PWM
> + must be specified
There's probably no need to move that entry just to change it?
> Optional properties:
> - - pwm-names: a list of names for the PWM devices specified in the
> - "pwms" property (see PWM binding[0])
> + - *-supply: regulators used within a power sequence
> + - *-gpio: GPIOs used within a power sequence
I don't think these really warrant mentioning here; such properties are
part of the standard regulator and GPIO bindings, and are required as a
side-effect of a power sequence using a resource of those types, rather
than being something actively defined or needed directly by the
pwm-backlight binding.
> + - pwm-names: name for the PWM device specified in the "pwms" property (see PWM
> + binding[0]). Necessary if power sequences are used
> + - power-on-sequence: Power sequence (see Power sequences[1]) used to bring the
> + backlight on. This sequence must reference the PWM specified in the pwms
> + property by its name. It can also reference other resources supported by
> + the power sequences mechanism
> + - power-off-sequence: Power sequence (see Power sequences[1]) used to bring
> + the backlight off. This sequence must reference the PWM specified in the
> + pwms property by its name. It can also reference other resources supported
> + by the power sequences mechanism
For these two, I would personally simply say that the pwm-backlight
binding requires that two power sequences named "on" and "off" must
exists, and are to defined according to [1].
> [0]: Documentation/devicetree/bindings/pwm/pwm.txt
> +[1]: Documentation/devicetree/bindings/power_seq/power_seq.txt
^ permalink raw reply
* Re: [PATCH v4 3/3] tegra: add pwm backlight device tree nodes
From: Stephen Warren @ 2012-08-16 18:45 UTC (permalink / raw)
To: Alexandre Courbot
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Mark Brown, Stephen Warren,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Anton Vorontsov,
linux-tegra-u79uwXL29TY76Z2rM5mHXA, David Woodhouse,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ
In-Reply-To: <1345097337-24170-4-git-send-email-acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
On 08/16/2012 12:08 AM, Alexandre Courbot wrote:
> +++ b/arch/arm/boot/dts/tegra20-ventana.dts
> + backlight_reg: fixedregulator@176 {
> + compatible = "regulator-fixed";
> + regulator-name = "backlight_regulator";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + gpio = <&gpio 176 0>;
> + startup-delay-us = <0>;
> + enable-active-high;
> + regulator-boot-off;
> + };
Rather than add that as a separate node at the top-level, I think just
add another sub-node to the "regulators" node. Oh, in fact it's already
there in next-20120816; you just need to add a label.
> +++ b/arch/arm/boot/dts/tegra20.dtsi
> - pwm {
> + pwm: pwm {
> compatible = "nvidia,tegra20-pwm";
> reg = <0x7000a000 0x100>;
It's pretty trivial I know, but I'd almost be tempted to make that a
separate patch so that it could be cherry-picked somewhere without a
tegra20-ventana.dts. But, perhaps that's silly.
^ permalink raw reply
* Re: [PATCH v4 1/3] Runtime Interpreted Power Sequences
From: Mark Brown @ 2012-08-16 18:47 UTC (permalink / raw)
To: Stephen Warren
Cc: Alexandre Courbot, Stephen Warren, Thierry Reding, Simon Glass,
Grant Likely, Rob Herring, Anton Vorontsov, David Woodhouse,
Arnd Bergmann, Leela Krishna Amudala,
linux-tegra-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-doc-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <502D3E29.1010501-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
On Thu, Aug 16, 2012 at 12:38:33PM -0600, Stephen Warren wrote:
> Note that this somewhat conflicts with accessing the top-level power
> sequence by name too; perhaps that should be re-thought too. I must
> admit this DT rule kinda sucks.
Given that currently the information there is useless and ignored is
there any reason we can't just change the rule? It'd be rather more
constructive...
^ permalink raw reply
* Re: [PATCH v4 1/3] Runtime Interpreted Power Sequences
From: Stephen Warren @ 2012-08-16 18:57 UTC (permalink / raw)
To: Mark Brown
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Stephen Warren,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Anton Vorontsov,
Alexandre Courbot, linux-tegra-u79uwXL29TY76Z2rM5mHXA,
David Woodhouse, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ
In-Reply-To: <20120816184703.GP15365-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>
On 08/16/2012 12:47 PM, Mark Brown wrote:
> On Thu, Aug 16, 2012 at 12:38:33PM -0600, Stephen Warren wrote:
>
>> Note that this somewhat conflicts with accessing the top-level power
>> sequence by name too; perhaps that should be re-thought too. I must
>> admit this DT rule kinda sucks.
>
> Given that currently the information there is useless and ignored is
> there any reason we can't just change the rule? It'd be rather more
> constructive...
I'll start a new thread on that topic and see. I would simplify matters
a lot...
^ permalink raw reply
* Re: [PATCH v4 1/3] Runtime Interpreted Power Sequences
From: Stephen Warren @ 2012-08-16 19:35 UTC (permalink / raw)
To: Mark Brown
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Stephen Warren,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Anton Vorontsov,
Alexandre Courbot, linux-tegra-u79uwXL29TY76Z2rM5mHXA,
David Woodhouse
In-Reply-To: <502D428F.1010901-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
On 08/16/2012 12:57 PM, Stephen Warren wrote:
> On 08/16/2012 12:47 PM, Mark Brown wrote:
>> On Thu, Aug 16, 2012 at 12:38:33PM -0600, Stephen Warren wrote:
>>
>>> Note that this somewhat conflicts with accessing the top-level power
>>> sequence by name too; perhaps that should be re-thought too. I must
>>> admit this DT rule kinda sucks.
>>
>> Given that currently the information there is useless and ignored is
>> there any reason we can't just change the rule? It'd be rather more
>> constructive...
>
> I'll start a new thread on that topic and see. I would simplify matters
> a lot...
In case anyone wants to follow it, it's at:
https://lists.ozlabs.org/pipermail/devicetree-discuss/2012-August/018502.html
^ permalink raw reply
* Re: [PATCH v4 1/3] Runtime Interpreted Power Sequences
From: Thierry Reding @ 2012-08-16 19:49 UTC (permalink / raw)
To: Stephen Warren
Cc: Alexandre Courbot, Stephen Warren, Simon Glass, Grant Likely,
Rob Herring, Mark Brown, Anton Vorontsov, David Woodhouse,
Arnd Bergmann, Leela Krishna Amudala, linux-tegra, linux-kernel,
linux-fbdev, devicetree-discuss, linux-doc
In-Reply-To: <502D3E29.1010501@wwwdotorg.org>
[-- Attachment #1: Type: text/plain, Size: 2258 bytes --]
On Thu, Aug 16, 2012 at 12:38:33PM -0600, Stephen Warren wrote:
> On 08/16/2012 12:08 AM, Alexandre Courbot wrote:
> > diff --git a/Documentation/power/power_seq.txt b/Documentation/power/power_seq.txt
>
> > +Usage by Drivers and Resources Management
> > +-----------------------------------------
> > +Power sequences make use of resources that must be properly allocated and
> > +managed. The power_seq_build() function builds a power sequence from the
> > +platform data. It also takes care of resolving and allocating the resources
> > +referenced by the sequence if needed:
> > +
> > + struct power_seq *power_seq_build(struct device *dev, struct list_head *ress,
> > + struct platform_power_seq *pseq);
> > +
> > +The 'dev' argument is the device in the name of which the resources are to be
> > +allocated.
> > +
> > +The 'ress' argument is a list to which the resolved resources are appended. This
> > +avoids allocating a resource referenced in several power sequences multiple
> > +times.
>
> I see in other parts of the thread, there has been discussion re:
> needing the separate ress parameter, and requiring the driver to pass
> this in to multiple power_seq_build calls.
>
> The way the pinctrl subsystem solved this was to have a single function
> that parsed all pinctrl setting (c.f. all power sequences) at once, and
> return a single handle. Later, the driver passes this handle
> pinctrl_lookup_state(), along with the requested state (c.f. sequence
> name) to search for, and finally passes that handle to
> pinctrl_select_state(). Doing something similar here would result in:
>
> struct power_seqs *power_seq_get(struct device *dev);
> void power_seq_put(struct power_seqs *seqs);
> struct power_seq *power_seq_lookup(struct power_seqs *seqs,
> const char *name);
> int power_seq_executestruct power_seq *seq);
>
> struct power_seqs *devm_power_seq_get(struct device *dev);
> void devm_power_seq_put(struct power_seqs *seqs);
Hehe, this looks very much like what I had in mind when I proposed this
during the last version of this series. The nice thing about this is
that the API is very much in line with how other subsystems work.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox