* [PATCH 3/3] fbdev: sh_mobile_lcdc: Compute clock pattern using divider denominator
From: Laurent Pinchart @ 2011-07-15 21:52 UTC (permalink / raw)
To: linux-fbdev
The clock divider pattern is computed based on the dot clock register
value which stores the divider denumerator. However, when using a 1:1
divider ratio, the register is programmed with a value that must not be
interpreted as a denominator. This results in a shift left operation
with a value of 32, which produces undefined behaviour.
Compute the clock pattern using the divider denominator, not the dot
clock register value.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/video/sh_mobile_lcdcfb.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/video/sh_mobile_lcdcfb.c b/drivers/video/sh_mobile_lcdcfb.c
index 32f884e..856f6a9 100644
--- a/drivers/video/sh_mobile_lcdcfb.c
+++ b/drivers/video/sh_mobile_lcdcfb.c
@@ -481,13 +481,15 @@ static int sh_mobile_lcdc_start(struct sh_mobile_lcdc_priv *priv)
if (!m)
continue;
+ /* FIXME: sh7724 can only use 42, 48, 54 and 60 for the divider
+ * denominator.
+ */
+ lcdc_write_chan(ch, LDDCKPAT1R, 0);
+ lcdc_write_chan(ch, LDDCKPAT2R, (1 << (m/2)) - 1);
+
if (m = 1)
m = LDDCKR_MOSEL;
tmp |= m << (lcdc_chan_is_sublcd(ch) ? 8 : 0);
-
- /* FIXME: sh7724 can only use 42, 48, 54 and 60 for the divider denominator */
- lcdc_write_chan(ch, LDDCKPAT1R, 0);
- lcdc_write_chan(ch, LDDCKPAT2R, (1 << (m/2)) - 1);
}
lcdc_write(priv, _LDDCKR, tmp);
--
1.7.3.4
^ permalink raw reply related
* [PATCH 2/3] fbdev: sh_mobile_lcdc: Don't acknowlege interrupts unintentionally
From: Laurent Pinchart @ 2011-07-15 21:52 UTC (permalink / raw)
To: linux-fbdev
The LDINTR register caries both interrupt enable and interrupt status
bits. When setting or clearing interrupt enable bits, write all status
bits to 1 to avoid acknowledging interrupts by mistake.
When acknowledging interrupts, write 1 to all non-triggered interrupt
bits to avoid losing interrupts.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/video/sh_mobile_lcdcfb.c | 20 ++++++++------------
1 files changed, 8 insertions(+), 12 deletions(-)
diff --git a/drivers/video/sh_mobile_lcdcfb.c b/drivers/video/sh_mobile_lcdcfb.c
index 7d77651..32f884e 100644
--- a/drivers/video/sh_mobile_lcdcfb.c
+++ b/drivers/video/sh_mobile_lcdcfb.c
@@ -318,19 +318,13 @@ static irqreturn_t sh_mobile_lcdc_irq(int irq, void *data)
{
struct sh_mobile_lcdc_priv *priv = data;
struct sh_mobile_lcdc_chan *ch;
- unsigned long tmp;
unsigned long ldintr;
int is_sub;
int k;
- /* acknowledge interrupt */
- ldintr = tmp = lcdc_read(priv, _LDINTR);
- /*
- * disable further VSYNC End IRQs, preserve all other enabled IRQs,
- * write 0 to bits 0-6 to ack all triggered IRQs.
- */
- tmp &= ~LDINTR_STATUS_MASK & ~LDINTR_VEE;
- lcdc_write(priv, _LDINTR, tmp);
+ /* Acknowledge interrupts and disable further VSYNC End IRQs. */
+ ldintr = lcdc_read(priv, _LDINTR);
+ lcdc_write(priv, _LDINTR, (ldintr ^ LDINTR_STATUS_MASK) & ~LDINTR_VEE);
/* figure out if this interrupt is for main or sub lcd */
is_sub = (lcdc_read(priv, _LDSR) & LDSR_MSS) ? 1 : 0;
@@ -342,7 +336,7 @@ static irqreturn_t sh_mobile_lcdc_irq(int irq, void *data)
if (!ch->enabled)
continue;
- /* Frame Start */
+ /* Frame End */
if (ldintr & LDINTR_FS) {
if (is_sub = lcdc_chan_is_sublcd(ch)) {
ch->frame_end = 1;
@@ -971,9 +965,11 @@ static int sh_mobile_wait_for_vsync(struct fb_info *info)
unsigned long ldintr;
int ret;
- /* Enable VSync End interrupt */
+ /* Enable VSync End interrupt and be careful not to acknowledge any
+ * pending interrupt.
+ */
ldintr = lcdc_read(ch->lcdc, _LDINTR);
- ldintr |= LDINTR_VEE;
+ ldintr |= LDINTR_VEE | LDINTR_STATUS_MASK;
lcdc_write(ch->lcdc, _LDINTR, ldintr);
ret = wait_for_completion_interruptible_timeout(&ch->vsync_completion,
--
1.7.3.4
^ permalink raw reply related
* [PATCH 1/3] fbdev: sh_mobile_lcdc: Replace hardcoded register values with macros
From: Laurent Pinchart @ 2011-07-15 21:52 UTC (permalink / raw)
To: linux-fbdev
Instead of hardcoding register values through the driver, define macros
for individual register bits using the register name and the bit name,
and use the macros.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/video/sh_mobile_lcdcfb.c | 204 +++++++++++++++++---------------------
include/video/sh_mobile_lcdc.h | 135 +++++++++++++++++++++----
2 files changed, 209 insertions(+), 130 deletions(-)
diff --git a/drivers/video/sh_mobile_lcdcfb.c b/drivers/video/sh_mobile_lcdcfb.c
index e9b80bc..7d77651 100644
--- a/drivers/video/sh_mobile_lcdcfb.c
+++ b/drivers/video/sh_mobile_lcdcfb.c
@@ -32,20 +32,6 @@
#define SIDE_B_OFFSET 0x1000
#define MIRROR_OFFSET 0x2000
-/* shared registers */
-#define _LDDCKR 0x410
-#define _LDDCKSTPR 0x414
-#define _LDINTR 0x468
-#define _LDSR 0x46c
-#define _LDCNT1R 0x470
-#define _LDCNT2R 0x474
-#define _LDRCNTR 0x478
-#define _LDDDSR 0x47c
-#define _LDDWD0R 0x800
-#define _LDDRDR 0x840
-#define _LDDWAR 0x900
-#define _LDDRAR 0x904
-
/* shared registers and their order for context save/restore */
static int lcdc_shared_regs[] = {
_LDDCKR,
@@ -98,22 +84,6 @@ static unsigned long lcdc_offs_sublcd[NR_CH_REGS] = {
[LDPMR] = 0x63c,
};
-#define START_LCDC 0x00000001
-#define LCDC_RESET 0x00000100
-#define DISPLAY_BEU 0x00000008
-#define LCDC_ENABLE 0x00000001
-#define LDINTR_FE 0x00000400
-#define LDINTR_VSE 0x00000200
-#define LDINTR_VEE 0x00000100
-#define LDINTR_FS 0x00000004
-#define LDINTR_VSS 0x00000002
-#define LDINTR_VES 0x00000001
-#define LDRCNTR_SRS 0x00020000
-#define LDRCNTR_SRC 0x00010000
-#define LDRCNTR_MRS 0x00000002
-#define LDRCNTR_MRC 0x00000001
-#define LDSR_MRS 0x00000100
-
static const struct fb_videomode default_720p = {
.name = "HDMI 720p",
.xres = 1280,
@@ -218,33 +188,36 @@ static void lcdc_sys_write_index(void *handle, unsigned long data)
{
struct sh_mobile_lcdc_chan *ch = handle;
- lcdc_write(ch->lcdc, _LDDWD0R, data | 0x10000000);
- lcdc_wait_bit(ch->lcdc, _LDSR, 2, 0);
- lcdc_write(ch->lcdc, _LDDWAR, 1 | (lcdc_chan_is_sublcd(ch) ? 2 : 0));
- lcdc_wait_bit(ch->lcdc, _LDSR, 2, 0);
+ lcdc_write(ch->lcdc, _LDDWD0R, data | LDDWDxR_WDACT);
+ lcdc_wait_bit(ch->lcdc, _LDSR, LDSR_AS, 0);
+ lcdc_write(ch->lcdc, _LDDWAR, LDDWAR_WA |
+ (lcdc_chan_is_sublcd(ch) ? 2 : 0));
+ lcdc_wait_bit(ch->lcdc, _LDSR, LDSR_AS, 0);
}
static void lcdc_sys_write_data(void *handle, unsigned long data)
{
struct sh_mobile_lcdc_chan *ch = handle;
- lcdc_write(ch->lcdc, _LDDWD0R, data | 0x11000000);
- lcdc_wait_bit(ch->lcdc, _LDSR, 2, 0);
- lcdc_write(ch->lcdc, _LDDWAR, 1 | (lcdc_chan_is_sublcd(ch) ? 2 : 0));
- lcdc_wait_bit(ch->lcdc, _LDSR, 2, 0);
+ lcdc_write(ch->lcdc, _LDDWD0R, data | LDDWDxR_WDACT | LDDWDxR_RSW);
+ lcdc_wait_bit(ch->lcdc, _LDSR, LDSR_AS, 0);
+ lcdc_write(ch->lcdc, _LDDWAR, LDDWAR_WA |
+ (lcdc_chan_is_sublcd(ch) ? 2 : 0));
+ lcdc_wait_bit(ch->lcdc, _LDSR, LDSR_AS, 0);
}
static unsigned long lcdc_sys_read_data(void *handle)
{
struct sh_mobile_lcdc_chan *ch = handle;
- lcdc_write(ch->lcdc, _LDDRDR, 0x01000000);
- lcdc_wait_bit(ch->lcdc, _LDSR, 2, 0);
- lcdc_write(ch->lcdc, _LDDRAR, 1 | (lcdc_chan_is_sublcd(ch) ? 2 : 0));
+ lcdc_write(ch->lcdc, _LDDRDR, LDDRDR_RSR);
+ lcdc_wait_bit(ch->lcdc, _LDSR, LDSR_AS, 0);
+ lcdc_write(ch->lcdc, _LDDRAR, LDDRAR_RA |
+ (lcdc_chan_is_sublcd(ch) ? 2 : 0));
udelay(1);
- lcdc_wait_bit(ch->lcdc, _LDSR, 2, 0);
+ lcdc_wait_bit(ch->lcdc, _LDSR, LDSR_AS, 0);
- return lcdc_read(ch->lcdc, _LDDRDR) & 0x3ffff;
+ return lcdc_read(ch->lcdc, _LDDRDR) & LDDRDR_DRD_MASK;
}
struct sh_mobile_lcdc_sys_bus_ops sh_mobile_lcdc_sys_bus_ops = {
@@ -323,13 +296,13 @@ static void sh_mobile_lcdc_deferred_io(struct fb_info *info,
if (bcfg->start_transfer)
bcfg->start_transfer(bcfg->board_data, ch,
&sh_mobile_lcdc_sys_bus_ops);
- lcdc_write_chan(ch, LDSM2R, 1);
+ lcdc_write_chan(ch, LDSM2R, LDSM2R_OSTRG);
dma_unmap_sg(info->dev, ch->sglist, nr_pages, DMA_TO_DEVICE);
} else {
if (bcfg->start_transfer)
bcfg->start_transfer(bcfg->board_data, ch,
&sh_mobile_lcdc_sys_bus_ops);
- lcdc_write_chan(ch, LDSM2R, 1);
+ lcdc_write_chan(ch, LDSM2R, LDSM2R_OSTRG);
}
}
@@ -356,11 +329,11 @@ static irqreturn_t sh_mobile_lcdc_irq(int irq, void *data)
* disable further VSYNC End IRQs, preserve all other enabled IRQs,
* write 0 to bits 0-6 to ack all triggered IRQs.
*/
- tmp &= 0xffffff00 & ~LDINTR_VEE;
+ tmp &= ~LDINTR_STATUS_MASK & ~LDINTR_VEE;
lcdc_write(priv, _LDINTR, tmp);
/* figure out if this interrupt is for main or sub lcd */
- is_sub = (lcdc_read(priv, _LDSR) & (1 << 10)) ? 1 : 0;
+ is_sub = (lcdc_read(priv, _LDSR) & LDSR_MSS) ? 1 : 0;
/* wake up channel and disable clocks */
for (k = 0; k < ARRAY_SIZE(priv->ch); k++) {
@@ -395,16 +368,17 @@ static void sh_mobile_lcdc_start_stop(struct sh_mobile_lcdc_priv *priv,
/* start or stop the lcdc */
if (start)
- lcdc_write(priv, _LDCNT2R, tmp | START_LCDC);
+ lcdc_write(priv, _LDCNT2R, tmp | LDCNT2R_DO);
else
- lcdc_write(priv, _LDCNT2R, tmp & ~START_LCDC);
+ lcdc_write(priv, _LDCNT2R, tmp & ~LDCNT2R_DO);
/* wait until power is applied/stopped on all channels */
for (k = 0; k < ARRAY_SIZE(priv->ch); k++)
if (lcdc_read(priv, _LDCNT2R) & priv->ch[k].enabled)
while (1) {
- tmp = lcdc_read_chan(&priv->ch[k], LDPMR) & 3;
- if (start && tmp = 3)
+ tmp = lcdc_read_chan(&priv->ch[k], LDPMR)
+ & LDPMR_LPS;
+ if (start && tmp = LDPMR_LPS)
break;
if (!start && tmp = 0)
break;
@@ -422,13 +396,13 @@ static void sh_mobile_lcdc_geometry(struct sh_mobile_lcdc_chan *ch)
u32 tmp;
tmp = ch->ldmt1r_value;
- tmp |= (var->sync & FB_SYNC_VERT_HIGH_ACT) ? 0 : 1 << 28;
- tmp |= (var->sync & FB_SYNC_HOR_HIGH_ACT) ? 0 : 1 << 27;
- tmp |= (ch->cfg.flags & LCDC_FLAGS_DWPOL) ? 1 << 26 : 0;
- tmp |= (ch->cfg.flags & LCDC_FLAGS_DIPOL) ? 1 << 25 : 0;
- tmp |= (ch->cfg.flags & LCDC_FLAGS_DAPOL) ? 1 << 24 : 0;
- tmp |= (ch->cfg.flags & LCDC_FLAGS_HSCNT) ? 1 << 17 : 0;
- tmp |= (ch->cfg.flags & LCDC_FLAGS_DWCNT) ? 1 << 16 : 0;
+ tmp |= (var->sync & FB_SYNC_VERT_HIGH_ACT) ? 0 : LDMT1R_VPOL;
+ tmp |= (var->sync & FB_SYNC_HOR_HIGH_ACT) ? 0 : LDMT1R_HPOL;
+ tmp |= (ch->cfg.flags & LCDC_FLAGS_DWPOL) ? LDMT1R_DWPOL : 0;
+ tmp |= (ch->cfg.flags & LCDC_FLAGS_DIPOL) ? LDMT1R_DIPOL : 0;
+ tmp |= (ch->cfg.flags & LCDC_FLAGS_DAPOL) ? LDMT1R_DAPOL : 0;
+ tmp |= (ch->cfg.flags & LCDC_FLAGS_HSCNT) ? LDMT1R_HSCNT : 0;
+ tmp |= (ch->cfg.flags & LCDC_FLAGS_DWCNT) ? LDMT1R_DWCNT : 0;
lcdc_write_chan(ch, LDMT1R, tmp);
/* setup SYS bus */
@@ -486,8 +460,8 @@ static int sh_mobile_lcdc_start(struct sh_mobile_lcdc_priv *priv)
}
/* reset */
- lcdc_write(priv, _LDCNT2R, lcdc_read(priv, _LDCNT2R) | LCDC_RESET);
- lcdc_wait_bit(priv, _LDCNT2R, LCDC_RESET, 0);
+ lcdc_write(priv, _LDCNT2R, lcdc_read(priv, _LDCNT2R) | LDCNT2R_BR);
+ lcdc_wait_bit(priv, _LDCNT2R, LDCNT2R_BR, 0);
/* enable LCDC channels */
tmp = lcdc_read(priv, _LDCNT2R);
@@ -496,7 +470,7 @@ static int sh_mobile_lcdc_start(struct sh_mobile_lcdc_priv *priv)
lcdc_write(priv, _LDCNT2R, tmp);
/* read data from external memory, avoid using the BEU for now */
- lcdc_write(priv, _LDCNT2R, lcdc_read(priv, _LDCNT2R) & ~DISPLAY_BEU);
+ lcdc_write(priv, _LDCNT2R, lcdc_read(priv, _LDCNT2R) & ~LDCNT2R_MD);
/* stop the lcdc first */
sh_mobile_lcdc_start_stop(priv, 0);
@@ -514,7 +488,7 @@ static int sh_mobile_lcdc_start(struct sh_mobile_lcdc_priv *priv)
continue;
if (m = 1)
- m = 1 << 6;
+ m = LDDCKR_MOSEL;
tmp |= m << (lcdc_chan_is_sublcd(ch) ? 8 : 0);
/* FIXME: sh7724 can only use 42, 48, 54 and 60 for the divider denominator */
@@ -554,20 +528,21 @@ static int sh_mobile_lcdc_start(struct sh_mobile_lcdc_priv *priv)
/* word and long word swap */
ldddsr = lcdc_read(priv, _LDDDSR);
if (priv->ch[0].info->var.nonstd)
- lcdc_write(priv, _LDDDSR, ldddsr | 7);
+ ldddsr |= LDDDSR_LS | LDDDSR_WS | LDDDSR_BS;
else {
switch (bpp) {
case 16:
- lcdc_write(priv, _LDDDSR, ldddsr | 6);
+ ldddsr |= LDDDSR_LS | LDDDSR_WS;
break;
case 24:
- lcdc_write(priv, _LDDDSR, ldddsr | 7);
+ ldddsr |= LDDDSR_LS | LDDDSR_WS | LDDDSR_BS;
break;
case 32:
- lcdc_write(priv, _LDDDSR, ldddsr | 4);
+ ldddsr |= LDDDSR_LS;
break;
}
}
+ lcdc_write(priv, _LDDDSR, ldddsr);
for (k = 0; k < ARRAY_SIZE(priv->ch); k++) {
unsigned long base_addr_y;
@@ -580,28 +555,29 @@ static int sh_mobile_lcdc_start(struct sh_mobile_lcdc_priv *priv)
/* set bpp format in PKF[4:0] */
tmp = lcdc_read_chan(ch, LDDFR);
- tmp &= ~0x0003031f;
+ tmp &= ~(LDDFR_CF0 | LDDFR_CC | LDDFR_YF_MASK | LDDFR_PKF_MASK);
if (ch->info->var.nonstd) {
tmp |= (ch->info->var.nonstd << 16);
switch (ch->info->var.bits_per_pixel) {
case 12:
break;
case 16:
- tmp |= (0x1 << 8);
+ tmp |= LDDFR_YF_422;
break;
case 24:
- tmp |= (0x2 << 8);
+ tmp |= LDDFR_YF_444;
break;
}
} else {
switch (ch->info->var.bits_per_pixel) {
case 16:
- tmp |= 0x03;
+ tmp |= LDDFR_PKF_RGB16;
break;
case 24:
- tmp |= 0x0b;
+ tmp |= LDDFR_PKF_RGB24;
break;
case 32:
+ tmp |= LDDFR_PKF_ARGB32;
break;
}
}
@@ -672,14 +648,14 @@ static int sh_mobile_lcdc_start(struct sh_mobile_lcdc_priv *priv)
/* setup deferred io if SYS bus */
tmp = ch->cfg.sys_bus_cfg.deferred_io_msec;
- if (ch->ldmt1r_value & (1 << 12) && tmp) {
+ if (ch->ldmt1r_value & LDMT1R_IFM && tmp) {
ch->defio.deferred_io = sh_mobile_lcdc_deferred_io;
ch->defio.delay = msecs_to_jiffies(tmp);
ch->info->fbdefio = &ch->defio;
fb_deferred_io_init(ch->info);
/* one-shot mode */
- lcdc_write_chan(ch, LDSM1R, 1);
+ lcdc_write_chan(ch, LDSM1R, LDSM1R_OS);
/* enable "Frame End Interrupt Enable" bit */
lcdc_write(priv, _LDINTR, LDINTR_FE);
@@ -691,7 +667,7 @@ static int sh_mobile_lcdc_start(struct sh_mobile_lcdc_priv *priv)
}
/* display output */
- lcdc_write(priv, _LDCNT1R, LCDC_ENABLE);
+ lcdc_write(priv, _LDCNT1R, LDCNT1R_DE);
/* start the lcdc */
sh_mobile_lcdc_start_stop(priv, 1);
@@ -780,42 +756,42 @@ static void sh_mobile_lcdc_stop(struct sh_mobile_lcdc_priv *priv)
static int sh_mobile_lcdc_check_interface(struct sh_mobile_lcdc_chan *ch)
{
- int ifm, miftyp;
-
- switch (ch->cfg.interface_type) {
- case RGB8: ifm = 0; miftyp = 0; break;
- case RGB9: ifm = 0; miftyp = 4; break;
- case RGB12A: ifm = 0; miftyp = 5; break;
- case RGB12B: ifm = 0; miftyp = 6; break;
- case RGB16: ifm = 0; miftyp = 7; break;
- case RGB18: ifm = 0; miftyp = 10; break;
- case RGB24: ifm = 0; miftyp = 11; break;
- case SYS8A: ifm = 1; miftyp = 0; break;
- case SYS8B: ifm = 1; miftyp = 1; break;
- case SYS8C: ifm = 1; miftyp = 2; break;
- case SYS8D: ifm = 1; miftyp = 3; break;
- case SYS9: ifm = 1; miftyp = 4; break;
- case SYS12: ifm = 1; miftyp = 5; break;
- case SYS16A: ifm = 1; miftyp = 7; break;
- case SYS16B: ifm = 1; miftyp = 8; break;
- case SYS16C: ifm = 1; miftyp = 9; break;
- case SYS18: ifm = 1; miftyp = 10; break;
- case SYS24: ifm = 1; miftyp = 11; break;
- default: goto bad;
+ int interface_type = ch->cfg.interface_type;
+
+ switch (interface_type) {
+ case RGB8:
+ case RGB9:
+ case RGB12A:
+ case RGB12B:
+ case RGB16:
+ case RGB18:
+ case RGB24:
+ case SYS8A:
+ case SYS8B:
+ case SYS8C:
+ case SYS8D:
+ case SYS9:
+ case SYS12:
+ case SYS16A:
+ case SYS16B:
+ case SYS16C:
+ case SYS18:
+ case SYS24:
+ break;
+ default:
+ return -EINVAL;
}
/* SUBLCD only supports SYS interface */
if (lcdc_chan_is_sublcd(ch)) {
- if (ifm = 0)
- goto bad;
- else
- ifm = 0;
+ if (!(interface_type & LDMT1R_IFM))
+ return -EINVAL;
+
+ interface_type &= ~LDMT1R_IFM;
}
- ch->ldmt1r_value = (ifm << 12) | miftyp;
+ ch->ldmt1r_value = interface_type;
return 0;
- bad:
- return -EINVAL;
}
static int sh_mobile_lcdc_setup_clocks(struct platform_device *pdev,
@@ -823,18 +799,24 @@ static int sh_mobile_lcdc_setup_clocks(struct platform_device *pdev,
struct sh_mobile_lcdc_priv *priv)
{
char *str;
- int icksel;
switch (clock_source) {
- case LCDC_CLK_BUS: str = "bus_clk"; icksel = 0; break;
- case LCDC_CLK_PERIPHERAL: str = "peripheral_clk"; icksel = 1; break;
- case LCDC_CLK_EXTERNAL: str = NULL; icksel = 2; break;
+ case LCDC_CLK_BUS:
+ str = "bus_clk";
+ priv->lddckr = LDDCKR_ICKSEL_BUS;
+ break;
+ case LCDC_CLK_PERIPHERAL:
+ str = "peripheral_clk";
+ priv->lddckr = LDDCKR_ICKSEL_MIPI;
+ break;
+ case LCDC_CLK_EXTERNAL:
+ str = NULL;
+ priv->lddckr = LDDCKR_ICKSEL_HDMI;
+ break;
default:
return -EINVAL;
}
- priv->lddckr = icksel << 16;
-
if (str) {
priv->dot_clk = clk_get(&pdev->dev, str);
if (IS_ERR(priv->dot_clk)) {
@@ -1476,12 +1458,12 @@ static int __devinit sh_mobile_lcdc_probe(struct platform_device *pdev)
switch (pdata->ch[i].chan) {
case LCDC_CHAN_MAINLCD:
- ch->enabled = 1 << 1;
+ ch->enabled = LDCNT2R_ME;
ch->reg_offs = lcdc_offs_mainlcd;
j++;
break;
case LCDC_CHAN_SUBLCD:
- ch->enabled = 1 << 2;
+ ch->enabled = LDCNT2R_SE;
ch->reg_offs = lcdc_offs_sublcd;
j++;
break;
diff --git a/include/video/sh_mobile_lcdc.h b/include/video/sh_mobile_lcdc.h
index d964e68..8101b72 100644
--- a/include/video/sh_mobile_lcdc.h
+++ b/include/video/sh_mobile_lcdc.h
@@ -4,26 +4,123 @@
#include <linux/fb.h>
#include <video/sh_mobile_meram.h>
+/* Register definitions */
+#define _LDDCKR 0x410
+#define LDDCKR_ICKSEL_BUS (0 << 16)
+#define LDDCKR_ICKSEL_MIPI (1 << 16)
+#define LDDCKR_ICKSEL_HDMI (2 << 16)
+#define LDDCKR_ICKSEL_EXT (3 << 16)
+#define LDDCKR_ICKSEL_MASK (7 << 16)
+#define LDDCKR_MOSEL (1 << 6)
+#define _LDDCKSTPR 0x414
+#define _LDINTR 0x468
+#define LDINTR_FE (1 << 10)
+#define LDINTR_VSE (1 << 9)
+#define LDINTR_VEE (1 << 8)
+#define LDINTR_FS (1 << 2)
+#define LDINTR_VSS (1 << 1)
+#define LDINTR_VES (1 << 0)
+#define LDINTR_STATUS_MASK (0xff << 0)
+#define _LDSR 0x46c
+#define LDSR_MSS (1 << 10)
+#define LDSR_MRS (1 << 8)
+#define LDSR_AS (1 << 1)
+#define _LDCNT1R 0x470
+#define LDCNT1R_DE (1 << 0)
+#define _LDCNT2R 0x474
+#define LDCNT2R_BR (1 << 8)
+#define LDCNT2R_MD (1 << 3)
+#define LDCNT2R_SE (1 << 2)
+#define LDCNT2R_ME (1 << 1)
+#define LDCNT2R_DO (1 << 0)
+#define _LDRCNTR 0x478
+#define LDRCNTR_SRS (1 << 17)
+#define LDRCNTR_SRC (1 << 16)
+#define LDRCNTR_MRS (1 << 1)
+#define LDRCNTR_MRC (1 << 0)
+#define _LDDDSR 0x47c
+#define LDDDSR_LS (1 << 2)
+#define LDDDSR_WS (1 << 1)
+#define LDDDSR_BS (1 << 0)
+
+#define LDMT1R_VPOL (1 << 28)
+#define LDMT1R_HPOL (1 << 27)
+#define LDMT1R_DWPOL (1 << 26)
+#define LDMT1R_DIPOL (1 << 25)
+#define LDMT1R_DAPOL (1 << 24)
+#define LDMT1R_HSCNT (1 << 17)
+#define LDMT1R_DWCNT (1 << 16)
+#define LDMT1R_IFM (1 << 12)
+#define LDMT1R_MIFTYP_RGB8 (0x0 << 0)
+#define LDMT1R_MIFTYP_RGB9 (0x4 << 0)
+#define LDMT1R_MIFTYP_RGB12A (0x5 << 0)
+#define LDMT1R_MIFTYP_RGB12B (0x6 << 0)
+#define LDMT1R_MIFTYP_RGB16 (0x7 << 0)
+#define LDMT1R_MIFTYP_RGB18 (0xa << 0)
+#define LDMT1R_MIFTYP_RGB24 (0xb << 0)
+#define LDMT1R_MIFTYP_YCBCR (0xf << 0)
+#define LDMT1R_MIFTYP_SYS8A (0x0 << 0)
+#define LDMT1R_MIFTYP_SYS8B (0x1 << 0)
+#define LDMT1R_MIFTYP_SYS8C (0x2 << 0)
+#define LDMT1R_MIFTYP_SYS8D (0x3 << 0)
+#define LDMT1R_MIFTYP_SYS9 (0x4 << 0)
+#define LDMT1R_MIFTYP_SYS12 (0x5 << 0)
+#define LDMT1R_MIFTYP_SYS16A (0x7 << 0)
+#define LDMT1R_MIFTYP_SYS16B (0x8 << 0)
+#define LDMT1R_MIFTYP_SYS16C (0x9 << 0)
+#define LDMT1R_MIFTYP_SYS18 (0xa << 0)
+#define LDMT1R_MIFTYP_SYS24 (0xb << 0)
+#define LDMT1R_MIFTYP_MASK (0xf << 0)
+
+#define LDDFR_CF1 (1 << 18)
+#define LDDFR_CF0 (1 << 17)
+#define LDDFR_CC (1 << 16)
+#define LDDFR_YF_420 (0 << 8)
+#define LDDFR_YF_422 (1 << 8)
+#define LDDFR_YF_444 (2 << 8)
+#define LDDFR_YF_MASK (3 << 8)
+#define LDDFR_PKF_ARGB32 (0x00 << 0)
+#define LDDFR_PKF_RGB16 (0x03 << 0)
+#define LDDFR_PKF_RGB24 (0x0b << 0)
+#define LDDFR_PKF_MASK (0x1f << 0)
+
+#define LDSM1R_OS (1 << 0)
+
+#define LDSM2R_OSTRG (1 << 0)
+
+#define LDPMR_LPS (3 << 0)
+
+#define _LDDWD0R 0x800
+#define LDDWDxR_WDACT (1 << 28)
+#define LDDWDxR_RSW (1 << 24)
+#define _LDDRDR 0x840
+#define LDDRDR_RSR (1 << 24)
+#define LDDRDR_DRD_MASK (0x3ffff << 0)
+#define _LDDWAR 0x900
+#define LDDWAR_WA (1 << 0)
+#define _LDDRAR 0x904
+#define LDDRAR_RA (1 << 0)
+
enum {
- RGB8, /* 24bpp, 8:8:8 */
- RGB9, /* 18bpp, 9:9 */
- RGB12A, /* 24bpp, 12:12 */
- RGB12B, /* 12bpp */
- RGB16, /* 16bpp */
- RGB18, /* 18bpp */
- RGB24, /* 24bpp */
- YUV422, /* 16bpp */
- SYS8A, /* 24bpp, 8:8:8 */
- SYS8B, /* 18bpp, 8:8:2 */
- SYS8C, /* 18bpp, 2:8:8 */
- SYS8D, /* 16bpp, 8:8 */
- SYS9, /* 18bpp, 9:9 */
- SYS12, /* 24bpp, 12:12 */
- SYS16A, /* 16bpp */
- SYS16B, /* 18bpp, 16:2 */
- SYS16C, /* 18bpp, 2:16 */
- SYS18, /* 18bpp */
- SYS24, /* 24bpp */
+ RGB8 = LDMT1R_MIFTYP_RGB8, /* 24bpp, 8:8:8 */
+ RGB9 = LDMT1R_MIFTYP_RGB9, /* 18bpp, 9:9 */
+ RGB12A = LDMT1R_MIFTYP_RGB12A, /* 24bpp, 12:12 */
+ RGB12B = LDMT1R_MIFTYP_RGB12B, /* 12bpp */
+ RGB16 = LDMT1R_MIFTYP_RGB16, /* 16bpp */
+ RGB18 = LDMT1R_MIFTYP_RGB18, /* 18bpp */
+ RGB24 = LDMT1R_MIFTYP_RGB24, /* 24bpp */
+ YUV422 = LDMT1R_MIFTYP_YCBCR, /* 16bpp */
+ SYS8A = LDMT1R_IFM | LDMT1R_MIFTYP_SYS8A, /* 24bpp, 8:8:8 */
+ SYS8B = LDMT1R_IFM | LDMT1R_MIFTYP_SYS8B, /* 18bpp, 8:8:2 */
+ SYS8C = LDMT1R_IFM | LDMT1R_MIFTYP_SYS8C, /* 18bpp, 2:8:8 */
+ SYS8D = LDMT1R_IFM | LDMT1R_MIFTYP_SYS8D, /* 16bpp, 8:8 */
+ SYS9 = LDMT1R_IFM | LDMT1R_MIFTYP_SYS9, /* 18bpp, 9:9 */
+ SYS12 = LDMT1R_IFM | LDMT1R_MIFTYP_SYS12, /* 24bpp, 12:12 */
+ SYS16A = LDMT1R_IFM | LDMT1R_MIFTYP_SYS16A, /* 16bpp */
+ SYS16B = LDMT1R_IFM | LDMT1R_MIFTYP_SYS16B, /* 18bpp, 16:2 */
+ SYS16C = LDMT1R_IFM | LDMT1R_MIFTYP_SYS16C, /* 18bpp, 2:16 */
+ SYS18 = LDMT1R_IFM | LDMT1R_MIFTYP_SYS18, /* 18bpp */
+ SYS24 = LDMT1R_IFM | LDMT1R_MIFTYP_SYS24, /* 24bpp */
};
enum { LCDC_CHAN_DISABLED = 0,
--
1.7.3.4
^ permalink raw reply related
* [PATCH 0/3] sh_mobile_lcdc cleanup and fixes
From: Laurent Pinchart @ 2011-07-15 21:52 UTC (permalink / raw)
To: linux-fbdev
Hi everybody,
When trying to understand the sh_mobile_lcdc driver, I found it hard to read
statements that include hardcoded register values. I thus wrote a patch that
replace them with macros, making the code more readable.
While doing so, I found two potential issues in the driver. See patches 2 and
3 for detailed explanations and fixes.
Laurent Pinchart (3):
fbdev: sh_mobile_lcdc: Replace hardcoded register values with macros
fbdev: sh_mobile_lcdc: Don't acknowlege interrupts unintentionally
fbdev: sh_mobile_lcdc: Compute clock pattern using divider
denominator
drivers/video/sh_mobile_lcdcfb.c | 232 +++++++++++++++++---------------------
include/video/sh_mobile_lcdc.h | 135 +++++++++++++++++++---
2 files changed, 222 insertions(+), 145 deletions(-)
--
Best regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 2/2 v2] ARM: mach-shmobile: use GPIO interrupt also for
From: Guennadi Liakhovetski @ 2011-07-15 12:34 UTC (permalink / raw)
To: Magnus Damm; +Cc: linux-mmc, linux-sh, Magnus Damm
In-Reply-To: <CANqRtoTqppKKRyOnLYQqA=wEm--OpyNNe4vDw9-EjOYZ6vJkEg@mail.gmail.com>
On Fri, 15 Jul 2011, Magnus Damm wrote:
> On Fri, Jul 15, 2011 at 7:59 PM, Guennadi Liakhovetski
> <g.liakhovetski@gmx.de> wrote:
> > When we switch to transaction-based runtime PM on SDHI / TMIO MMC,
> > also card eject events will have to be detected by the platform.
> > This patch prepares mackerel to this switch.
> >
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > ---
> >
> > v2: reconfiguring the pin for card insertion and ejection is no longer
> > needed.
>
> Hi Guennadi,
>
> Thanks for your work on this. I may be reading the code wrong, or
> perhaps my stack of patches is not the same as yours, but I can't see
> any code dealing with hotplug for SDHI2? It looks to me that your
> patch is updating the comment about PORT162 and IRQ0 but does nothing
> about it on the software side. I sort of assume that we want to have
> hotplug support via IRQ on all SDHI channels if possible. But maybe
> that's not possible?
Maybe;-) I've spent quite a bit of time trying to make SDHI2 work
according to the same scheme, but so far I failed. With a kernel, patched
to produce GPIO interrupts from SDHI2 card insertion, as soon as I insert
a card the board locks up hard without a single character on any of the
consoles... So, I'm afraid, we have to leave SDHI2 alone for now.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply
* Re: [PATCH 2/2 v2] ARM: mach-shmobile: use GPIO interrupt also for
From: Magnus Damm @ 2011-07-15 11:25 UTC (permalink / raw)
To: Guennadi Liakhovetski; +Cc: linux-mmc, linux-sh, Magnus Damm
In-Reply-To: <Pine.LNX.4.64.1107151257220.22613@axis700.grange>
On Fri, Jul 15, 2011 at 7:59 PM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> When we switch to transaction-based runtime PM on SDHI / TMIO MMC,
> also card eject events will have to be detected by the platform.
> This patch prepares mackerel to this switch.
>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
>
> v2: reconfiguring the pin for card insertion and ejection is no longer
> needed.
Hi Guennadi,
Thanks for your work on this. I may be reading the code wrong, or
perhaps my stack of patches is not the same as yours, but I can't see
any code dealing with hotplug for SDHI2? It looks to me that your
patch is updating the comment about PORT162 and IRQ0 but does nothing
about it on the software side. I sort of assume that we want to have
hotplug support via IRQ on all SDHI channels if possible. But maybe
that's not possible?
Cheers,
/ magnus
^ permalink raw reply
* Re: [PATCH 1/2] sh-mobile: enable both edges GPIO interrupts on sh7372
From: Magnus Damm @ 2011-07-15 11:14 UTC (permalink / raw)
To: Guennadi Liakhovetski; +Cc: linux-mmc, linux-sh, Magnus Damm
In-Reply-To: <Pine.LNX.4.64.1107151256380.22613@axis700.grange>
On Fri, Jul 15, 2011 at 7:58 PM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> From: Magnus Damm <damm@opensource.se>
>
> IRQ-capable GPIOs on sh7372 can be configured to produce interrupts on
> both edges.
>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Cc: Magnus Damm <damm@opensource.se>
> ---
> drivers/sh/intc/chip.c | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
Acked-by: Magnus Damm <damm@opensource.se>
^ permalink raw reply
* [PATCH 2/2 v2] ARM: mach-shmobile: use GPIO interrupt also for card
From: Guennadi Liakhovetski @ 2011-07-15 10:59 UTC (permalink / raw)
To: linux-mmc; +Cc: linux-sh, Magnus Damm
In-Reply-To: <Pine.LNX.4.64.1107151254300.22613@axis700.grange>
When we switch to transaction-based runtime PM on SDHI / TMIO MMC,
also card eject events will have to be detected by the platform.
This patch prepares mackerel to this switch.
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
v2: reconfiguring the pin for card insertion and ejection is no longer
needed.
arch/arm/mach-shmobile/board-mackerel.c | 48 +++++++++++++++++++++++++-----
1 files changed, 40 insertions(+), 8 deletions(-)
diff --git a/arch/arm/mach-shmobile/board-mackerel.c b/arch/arm/mach-shmobile/board-mackerel.c
index 1b30195..9addaae 100644
--- a/arch/arm/mach-shmobile/board-mackerel.c
+++ b/arch/arm/mach-shmobile/board-mackerel.c
@@ -45,6 +45,7 @@
#include <linux/tca6416_keypad.h>
#include <linux/usb/r8a66597.h>
#include <linux/usb/renesas_usbhs.h>
+#include <linux/workqueue.h>
#include <video/sh_mobile_hdmi.h>
#include <video/sh_mobile_lcdc.h>
@@ -990,7 +991,7 @@ static struct platform_device fsi_ak4643_device = {
/*
* The card detect pin of the top SD/MMC slot (CN7) is active low and is
- * connected to GPIO A22 of SH7372 (GPIO_PORT41).
+ * connected to GPIO A22 of SH7372 (GPIO_PORT41 / IRQ8).
*/
static int slot_cn7_get_cd(struct platform_device *pdev)
{
@@ -998,12 +999,30 @@ static int slot_cn7_get_cd(struct platform_device *pdev)
}
/* SDHI0 */
-static irqreturn_t mackerel_sdhi0_gpio_cd(int irq, void *arg)
+struct sdhi_card_detect {
+ struct delayed_work work;
+ struct sh_mobile_sdhi_info *info;
+ int gpio_irq;
+};
+
+static void sdhi_cd_work(struct work_struct *work)
+{
+ struct sdhi_card_detect *cd = container_of(work, struct sdhi_card_detect, work.work);
+ irq_set_irq_type(cd->gpio_irq, IRQ_TYPE_EDGE_BOTH);
+}
+
+static irqreturn_t mackerel_sdhi_gpio_cd(int irq, void *arg)
{
- struct device *dev = arg;
- struct sh_mobile_sdhi_info *info = dev->platform_data;
+ struct sdhi_card_detect *cd = arg;
+ struct sh_mobile_sdhi_info *info = cd->info;
struct tmio_mmc_data *pdata = info->pdata;
+ if (irq != cd->gpio_irq)
+ return IRQ_NONE;
+
+ irq_set_irq_type(irq, IRQ_TYPE_NONE);
+
+ schedule_delayed_work(&cd->work, msecs_to_jiffies(200));
tmio_mmc_cd_wakeup(pdata);
return IRQ_HANDLED;
@@ -1015,6 +1034,12 @@ static struct sh_mobile_sdhi_info sdhi0_info = {
.tmio_caps = MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ,
};
+static struct sdhi_card_detect sdhi0_cd = {
+ .work = __DELAYED_WORK_INITIALIZER(sdhi0_cd.work, sdhi_cd_work),
+ .info = &sdhi0_info,
+ .gpio_irq = evt2irq(0x3340),
+};
+
static struct resource sdhi0_resources[] = {
[0] = {
.name = "SDHI0",
@@ -1092,7 +1117,7 @@ static struct platform_device sdhi1_device = {
/*
* The card detect pin of the top SD/MMC slot (CN23) is active low and is
- * connected to GPIO SCIFB_SCK of SH7372 (GPIO_PORT162).
+ * connected to GPIO SCIFB_SCK of SH7372 (GPIO_PORT162 / IRQ0).
*/
static int slot_cn23_get_cd(struct platform_device *pdev)
{
@@ -1500,12 +1525,19 @@ static void __init mackerel_init(void)
gpio_request(GPIO_FN_SDHID0_1, NULL);
gpio_request(GPIO_FN_SDHID0_0, NULL);
- ret = request_irq(evt2irq(0x3340), mackerel_sdhi0_gpio_cd,
- IRQF_TRIGGER_FALLING, "sdhi0 cd", &sdhi0_device.dev);
+ /*
+ * If the driver probes with a card plugged in, the native SDHICD0 IRQ
+ * will trigger, when the runtime PM brings the interface up, and the
+ * card will be detected. This interrupt is needed if there is no card
+ * during probing and runtime PM turns the interface power off.
+ */
+ ret = request_irq(sdhi0_cd.gpio_irq, mackerel_sdhi_gpio_cd,
+ IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+ "sdhi0 cd", &sdhi0_cd);
if (!ret)
sdhi0_info.tmio_flags |= TMIO_MMC_HAS_COLD_CD;
else
- pr_err("Cannot get IRQ #%d: %d\n", evt2irq(0x3340), ret);
+ pr_err("Cannot get IRQ #%d: %d\n", sdhi0_cd.gpio_irq, ret);
#if !defined(CONFIG_MMC_SH_MMCIF) && !defined(CONFIG_MMC_SH_MMCIF_MODULE)
/* enable SDHI1 */
--
1.7.2.5
^ permalink raw reply related
* [PATCH 1/2] sh-mobile: enable both edges GPIO interrupts on sh7372
From: Guennadi Liakhovetski @ 2011-07-15 10:58 UTC (permalink / raw)
To: linux-mmc; +Cc: linux-sh, Magnus Damm
In-Reply-To: <Pine.LNX.4.64.1107151254300.22613@axis700.grange>
From: Magnus Damm <damm@opensource.se>
IRQ-capable GPIOs on sh7372 can be configured to produce interrupts on
both edges.
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Magnus Damm <damm@opensource.se>
---
drivers/sh/intc/chip.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/sh/intc/chip.c b/drivers/sh/intc/chip.c
index f33e2dd..33b2ed4 100644
--- a/drivers/sh/intc/chip.c
+++ b/drivers/sh/intc/chip.c
@@ -186,6 +186,9 @@ static unsigned char intc_irq_sense_table[IRQ_TYPE_SENSE_MASK + 1] = {
!defined(CONFIG_CPU_SUBTYPE_SH7709)
[IRQ_TYPE_LEVEL_HIGH] = VALID(3),
#endif
+#if defined(CONFIG_ARCH_SH7372)
+ [IRQ_TYPE_EDGE_BOTH] = VALID(4),
+#endif
};
static int intc_set_type(struct irq_data *data, unsigned int type)
--
1.7.2.5
^ permalink raw reply related
* [PATCH 0/2] use both-edge GPIO interrupt for SDHI0 on mackerel
From: Guennadi Liakhovetski @ 2011-07-15 10:58 UTC (permalink / raw)
To: linux-mmc; +Cc: linux-sh, Magnus Damm
In-Reply-To: <Pine.LNX.4.64.1107140123070.30737@axis700.grange>
As correctly pointed out by Magnus, IRQ-capable GPIOs on sh7372 can
produce interrupts on both edges. These patches extend the intc driver and
use this for SDHI0 hotplug detection on mackerel.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply
* Re: [PATCH 0/2] mmc: tmio: two bug-fixes
From: Guennadi Liakhovetski @ 2011-07-15 8:46 UTC (permalink / raw)
To: Chris Ball
Cc: linux-mmc, linux-sh, Magnus Damm, Rafael J. Wysocki, Ian Molton,
Simon Horman
In-Reply-To: <m2pqlcslx4.fsf@bob.laptop.org>
Hi Chris
On Thu, 14 Jul 2011, Chris Ball wrote:
> Hi Guennadi,
>
> On Thu, Jul 14 2011, Guennadi Liakhovetski wrote:
> > There go two more bug-fixes for tmio on top of my earlier patches from
> > today, so, should still be easy enough to trace. So, to pull them
> > together, the previous ones were
> >
> > [PATCH v3] mmc: tmio: fix recursive spinlock, don't schedule with
> > interrupts disabled
> > [PATCH v3] mmc: tmio: maximize power saving
> >
> > And now follow
> >
> > [PATCH 1/2] mmc: tmio: fix a recently introduced bug in DMA code
> > [PATCH 2/2] mmc: tmio: fix a deadlock
>
> Pushed these four plus "mmc: sh_mmcif: maximize power saving" to
> mmc-next for 3.1. Thanks!
Thanks for a fast reaction.
> By the way, feel free to send me a MAINTAINERS update taking over
> TMIO if you'd like to; you've been doing the work. :-)
Thanks for the offer, I appreciate it! I am not the only active tmio mmc
contributor, but yes, I could take that role upon me, if noone objects.
Would be good to hear, what Ian thinks? Is he maybe still planning to
actively working on the driver? Would he like to stay listed as a
co-maintainer? If I am to maintain the driver alone, it would be good to
have access to some non-sh-mobile hardware, using it too, at least for
regression testing. Any idea where to get anything like that?
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply
* Re: [PATCH 6/6] clk: Add initial WM831x clock driver
From: Ryan Mallon @ 2011-07-15 5:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110715050503.GL32716@opensource.wolfsonmicro.com>
On 15/07/11 15:05, Mark Brown wrote:
> On Thu, Jul 14, 2011 at 08:53:39PM -0600, Grant Likely wrote:
>> On Mon, Jul 11, 2011 at 11:53:57AM +0900, Mark Brown wrote:
>>> + if (clkdata->xtal_ena)
>>> + return 0;
>>> + else
>>> + return -EPERM;
>>> +}
>> Nit: return clkdata->xtal_ena ? 0 : -EPERM;
>> Just makes for more concise code.
> I have an extremely strong dislike of the ternery operator, I find it
> does nothing for legibility.
>
I prefer this:
if (!clkdata->xtal_ena)
return -EPERM;
return 0;
}
Which makes the error check clear and makes the success case visible
right at the bottom of the function.
~Ryan
^ permalink raw reply
* Re: [PATCH 6/6] clk: Add initial WM831x clock driver
From: Mark Brown @ 2011-07-15 5:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110715025339.GO2927@ponder.secretlab.ca>
On Thu, Jul 14, 2011 at 08:53:39PM -0600, Grant Likely wrote:
> On Mon, Jul 11, 2011 at 11:53:57AM +0900, Mark Brown wrote:
> > @@ -10,6 +10,7 @@ config GENERIC_CLK_BUILD_TEST
> > depends on EXPERIMENTAL && GENERIC_CLK
> > select GENERIC_CLK_FIXED
> > select GENERIC_CLK_GATE
> > + select GENERIC_CLK_WM831X if MFD_WM831X=y
> Hmmm, this could get unwieldy in a hurry.
It's not really any hassle, we've got a one of these in ASoC. The list
gets long but if you keep it sorted it's not an issue for merges and
otherwise it's just long not complicated. The ability to get build
coverage is *really* useful.
> > +static int wm831x_xtal_enable(struct clk_hw *hw)
> > +{
> > + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk,
> > + xtal_hw);
> This container of is used 10 times. A static inline would be
> reasonable.
Not quite - it's used in three different variants (one for each of the
clocks).
> > + if (clkdata->xtal_ena)
> > + return 0;
> > + else
> > + return -EPERM;
> > +}
> Nit: return clkdata->xtal_ena ? 0 : -EPERM;
> Just makes for more concise code.
I have an extremely strong dislike of the ternery operator, I find it
does nothing for legibility.
> > + if (!clk_register(wm831x->dev, &wm831x_clkout_ops, &clkdata->clkout_hw,
> > + "clkout")) {
> > + ret = -EINVAL;
> > + goto err_fll;
> > + }
> How common will this pattern be? Is there need for a
> clk_register_many() variant?
I dunno, I think a lot of the SoCs may be doing one clk per device. But
equally well it's not like it'd be hard if someone starts working on the
API again.
^ permalink raw reply
* Re: [PATCH 6/6] clk: Add initial WM831x clock driver
From: Grant Likely @ 2011-07-15 2:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1310352837-4277-6-git-send-email-broonie@opensource.wolfsonmicro.com>
On Mon, Jul 11, 2011 at 11:53:57AM +0900, Mark Brown wrote:
> The WM831x and WM832x series of PMICs contain a flexible clocking
> subsystem intended to provide always on and system core clocks. It
> features:
>
> - A 32.768kHz crystal oscillator which can optionally be used to pass
> through an externally generated clock.
> - A FLL which can be clocked from either the 32.768kHz oscillator or
> the CLKIN pin.
> - A CLKOUT pin which can bring out either the oscillator or the FLL
> output.
> - The 32.768kHz clock can also optionally be brought out on the GPIO
> pins of the device.
>
> This driver fully supports the 32.768kHz oscillator and CLKOUT. The FLL
> is supported only in AUTO mode, the full flexibility of the FLL cannot
> currently be used. The use of clock references other than the internal
> oscillator is not currently supported, and since clk_set_parent() is not
> implemented in the generic clock API the clock tree configuration cannot
> be changed at runtime.
>
> Due to a lack of access to systems where the core SoC has been converted
> to use the generic clock API this driver has been compile tested only.
Generally seems okay. Minor comments below.
g.
>
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
> MAINTAINERS | 1 +
> drivers/clk/Kconfig | 5 +
> drivers/clk/Makefile | 1 +
> drivers/clk/clk-wm831x.c | 389 ++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 396 insertions(+), 0 deletions(-)
> create mode 100644 drivers/clk/clk-wm831x.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ae563fa..c234756 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6969,6 +6969,7 @@ T: git git://opensource.wolfsonmicro.com/linux-2.6-audioplus
> W: http://opensource.wolfsonmicro.com/content/linux-drivers-wolfson-devices
> S: Supported
> F: Documentation/hwmon/wm83??
> +F: drivers/clk/clk-wm83*.c
> F: drivers/leds/leds-wm83*.c
> F: drivers/mfd/wm8*.c
> F: drivers/power/wm83*.c
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index 1fd0070..7f6eec2 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -10,6 +10,7 @@ config GENERIC_CLK_BUILD_TEST
> depends on EXPERIMENTAL && GENERIC_CLK
> select GENERIC_CLK_FIXED
> select GENERIC_CLK_GATE
> + select GENERIC_CLK_WM831X if MFD_WM831X=y
Hmmm, this could get unwieldy in a hurry.
> help
> Enable all possible generic clock drivers. This is only
> useful for improving build coverage, it is not useful for
> @@ -22,3 +23,7 @@ config GENERIC_CLK_FIXED
> config GENERIC_CLK_GATE
> bool
> depends on GENERIC_CLK
> +
> +config GENERIC_CLK_WM831X
> + tristate
> + depends on GENERIC_CLK && MFD_WM831X
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index d186446..6628ad5 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -3,3 +3,4 @@ obj-$(CONFIG_CLKDEV_LOOKUP) += clkdev.o
> obj-$(CONFIG_GENERIC_CLK) += clk.o
> obj-$(CONFIG_GENERIC_CLK_FIXED) += clk-fixed.o
> obj-$(CONFIG_GENERIC_CLK_GATE) += clk-gate.o
> +obj-$(CONFIG_GENERIC_CLK_WM831X) += clk-wm831x.o
> diff --git a/drivers/clk/clk-wm831x.c b/drivers/clk/clk-wm831x.c
> new file mode 100644
> index 0000000..82782f1
> --- /dev/null
> +++ b/drivers/clk/clk-wm831x.c
> @@ -0,0 +1,389 @@
> +/*
> + * WM831x clock control
> + *
> + * Copyright 2011 Wolfson Microelectronics PLC.
> + *
> + * Author: Mark Brown <broonie@opensource.wolfsonmicro.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + *
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/platform_device.h>
> +#include <linux/mfd/wm831x/core.h>
> +
> +struct wm831x_clk {
> + struct wm831x *wm831x;
> + struct clk_hw xtal_hw;
> + struct clk_hw fll_hw;
> + struct clk_hw clkout_hw;
> + bool xtal_ena;
> +};
> +
> +static int wm831x_xtal_enable(struct clk_hw *hw)
> +{
> + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk,
> + xtal_hw);
This container of is used 10 times. A static inline would be
reasonable.
> +
> + if (clkdata->xtal_ena)
> + return 0;
> + else
> + return -EPERM;
> +}
Nit: return clkdata->xtal_ena ? 0 : -EPERM;
Just makes for more concise code.
> +
> +static unsigned long wm831x_xtal_recalc_rate(struct clk_hw *hw)
> +{
> + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk,
> + xtal_hw);
> +
> + if (clkdata->xtal_ena)
> + return 32768;
> + else
> + return 0;
> +}
> +
> +static long wm831x_xtal_round_rate(struct clk_hw *hw, unsigned long rate)
> +{
> + return wm831x_xtal_recalc_rate(hw);
> +}
> +
> +static const struct clk_hw_ops wm831x_xtal_ops = {
> + .enable = wm831x_xtal_enable,
> + .recalc_rate = wm831x_xtal_recalc_rate,
> + .round_rate = wm831x_xtal_round_rate,
> +};
> +
> +static const unsigned long wm831x_fll_auto_rates[] = {
> + 2048000,
> + 11289600,
> + 12000000,
> + 12288000,
> + 19200000,
> + 22579600,
> + 24000000,
> + 24576000,
> +};
> +
> +static bool wm831x_fll_enabled(struct wm831x *wm831x)
> +{
> + int ret;
> +
> + ret = wm831x_reg_read(wm831x, WM831X_FLL_CONTROL_1);
> + if (ret < 0) {
> + dev_err(wm831x->dev, "Unable to read FLL_CONTROL_1: %d\n",
> + ret);
> + return true;
> + }
> +
> + if (ret & WM831X_FLL_ENA)
> + return true;
> + else
> + return false;
similarly, return (ret & WM831X_FLL_ENA) != 0;
> +}
> +
> +static int wm831x_fll_prepare(struct clk_hw *hw)
> +{
> + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk,
> + fll_hw);
> + struct wm831x *wm831x = clkdata->wm831x;
> + int ret;
> +
> + ret = wm831x_set_bits(wm831x, WM831X_FLL_CONTROL_2,
> + WM831X_FLL_ENA, WM831X_FLL_ENA);
> + if (ret != 0)
> + dev_crit(wm831x->dev, "Failed to enable FLL: %d\n", ret);
> +
> + return ret;
> +}
> +
> +static void wm831x_fll_unprepare(struct clk_hw *hw)
> +{
> + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk,
> + fll_hw);
> + struct wm831x *wm831x = clkdata->wm831x;
> + int ret;
> +
> + ret = wm831x_set_bits(wm831x, WM831X_FLL_CONTROL_2, WM831X_FLL_ENA, 0);
> + if (ret != 0)
> + dev_crit(wm831x->dev, "Failed to disaable FLL: %d\n", ret);
> +}
> +
> +static unsigned long wm831x_fll_recalc_rate(struct clk_hw *hw)
> +{
> + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk,
> + fll_hw);
> + struct wm831x *wm831x = clkdata->wm831x;
> + int ret;
> +
> + ret = wm831x_reg_read(wm831x, WM831X_CLOCK_CONTROL_2);
> + if (ret < 0) {
> + dev_err(wm831x->dev, "Unable to read CLOCK_CONTROL_2: %d\n",
> + ret);
> + return 0;
> + }
> +
> + if (ret & WM831X_FLL_AUTO)
> + return wm831x_fll_auto_rates[ret & WM831X_FLL_AUTO_FREQ_MASK];
> +
> + dev_err(wm831x->dev, "FLL only supported in AUTO mode\n");
> + return 0;
> +}
> +
> +static int wm831x_fll_set_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long *parent_rate)
> +{
> + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk,
> + fll_hw);
> + struct wm831x *wm831x = clkdata->wm831x;
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(wm831x_fll_auto_rates); i++)
> + if (wm831x_fll_auto_rates[i] = rate)
> + break;
> + if (i = ARRAY_SIZE(wm831x_fll_auto_rates))
> + return -EINVAL;
> +
> + if (wm831x_fll_enabled(wm831x))
> + return -EPERM;
> +
> + return wm831x_set_bits(wm831x, WM831X_CLOCK_CONTROL_2,
> + WM831X_FLL_AUTO_FREQ_MASK, i);
> +}
> +
> +static struct clk *wm831x_fll_get_parent(struct clk_hw *hw)
> +{
> + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk,
> + fll_hw);
> + struct wm831x *wm831x = clkdata->wm831x;
> + int ret;
> +
> + /* AUTO mode is always clocked from the crystal */
> + ret = wm831x_reg_read(wm831x, WM831X_CLOCK_CONTROL_2);
> + if (ret < 0) {
> + dev_err(wm831x->dev, "Unable to read CLOCK_CONTROL_2: %d\n",
> + ret);
> + return NULL;
> + }
> +
> + if (ret & WM831X_FLL_AUTO)
> + return clkdata->xtal_hw.clk;
> +
> + ret = wm831x_reg_read(wm831x, WM831X_FLL_CONTROL_5);
> + if (ret < 0) {
> + dev_err(wm831x->dev, "Unable to read FLL_CONTROL_5: %d\n",
> + ret);
> + return NULL;
> + }
> +
> + switch (ret & WM831X_FLL_CLK_SRC_MASK) {
> + case 0:
> + return clkdata->xtal_hw.clk;
> + case 1:
> + dev_warn(wm831x->dev,
> + "FLL clocked from CLKIN not yet supported\n");
> + return NULL;
> + default:
> + dev_err(wm831x->dev, "Unsupported FLL clock source %d\n",
> + ret & WM831X_FLL_CLK_SRC_MASK);
> + return NULL;
> + }
> +}
> +
> +static const struct clk_hw_ops wm831x_fll_ops = {
> + .prepare = wm831x_fll_prepare,
> + .unprepare = wm831x_fll_unprepare,
> + .recalc_rate = wm831x_fll_recalc_rate,
> + .set_rate = wm831x_fll_set_rate,
> + .get_parent = wm831x_fll_get_parent,
> +};
> +
> +static int wm831x_clkout_prepare(struct clk_hw *hw)
> +{
> + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk,
> + clkout_hw);
> + struct wm831x *wm831x = clkdata->wm831x;
> + int ret;
> +
> + ret = wm831x_reg_unlock(wm831x);
> + if (ret != 0) {
> + dev_crit(wm831x->dev, "Failed to lock registers: %d\n", ret);
> + return ret;
> + }
> +
> + ret = wm831x_set_bits(wm831x, WM831X_CLOCK_CONTROL_1,
> + WM831X_CLKOUT_ENA, WM831X_CLKOUT_ENA);
> + if (ret != 0)
> + dev_crit(wm831x->dev, "Failed to enable CLKOUT: %d\n", ret);
> +
> + wm831x_reg_lock(wm831x);
> +
> + return ret;
> +}
> +
> +static void wm831x_clkout_unprepare(struct clk_hw *hw)
> +{
> + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk,
> + clkout_hw);
> + struct wm831x *wm831x = clkdata->wm831x;
> + int ret;
> +
> + ret = wm831x_reg_unlock(wm831x);
> + if (ret != 0) {
> + dev_crit(wm831x->dev, "Failed to lock registers: %d\n", ret);
> + return;
> + }
> +
> + ret = wm831x_set_bits(wm831x, WM831X_CLOCK_CONTROL_1,
> + WM831X_CLKOUT_ENA, 0);
> + if (ret != 0)
> + dev_crit(wm831x->dev, "Failed to disable CLKOUT: %d\n", ret);
> +
> + wm831x_reg_lock(wm831x);
> +}
> +
> +static unsigned long wm831x_clkout_recalc_rate(struct clk_hw *hw)
> +{
> + return clk_get_rate(clk_get_parent(hw->clk));
> +}
> +
> +static long wm831x_clkout_round_rate(struct clk_hw *hw, unsigned long rate)
> +{
> + return clk_round_rate(clk_get_parent(hw->clk), rate);
> +}
> +
> +static int wm831x_clkout_set_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long *parent_rate)
> +{
> + *parent_rate = rate;
> + return CLK_SET_RATE_PROPAGATE;
> +}
> +
> +static struct clk *wm831x_clkout_get_parent(struct clk_hw *hw)
> +{
> + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk,
> + clkout_hw);
> + struct wm831x *wm831x = clkdata->wm831x;
> + int ret;
> +
> + ret = wm831x_reg_read(wm831x, WM831X_CLOCK_CONTROL_1);
> + if (ret < 0) {
> + dev_err(wm831x->dev, "Unable to read CLOCK_CONTROL_1: %d\n",
> + ret);
> + return NULL;
> + }
> +
> + if (ret & WM831X_CLKOUT_SRC)
> + return clkdata->xtal_hw.clk;
> + else
> + return clkdata->fll_hw.clk;
> +}
> +
> +static const struct clk_hw_ops wm831x_clkout_ops = {
> + .prepare = wm831x_clkout_prepare,
> + .unprepare = wm831x_clkout_unprepare,
> + .recalc_rate = wm831x_clkout_recalc_rate,
> + .round_rate = wm831x_clkout_round_rate,
> + .set_rate = wm831x_clkout_set_rate,
> + .get_parent = wm831x_clkout_get_parent,
> +};
> +
> +static __devinit int wm831x_clk_probe(struct platform_device *pdev)
> +{
> + struct wm831x *wm831x = dev_get_drvdata(pdev->dev.parent);
> + struct wm831x_clk *clkdata;
> + int ret;
> +
> + clkdata = kzalloc(sizeof(*clkdata), GFP_KERNEL);
> + if (!clkdata)
> + return -ENOMEM;
> +
> + /* XTAL_ENA can only be set via OTP/InstantConfig so just read once */
> + ret = wm831x_reg_read(wm831x, WM831X_CLOCK_CONTROL_2);
> + if (ret < 0) {
> + dev_err(wm831x->dev, "Unable to read CLOCK_CONTROL_2: %d\n",
> + ret);
> + goto err_alloc;
> + }
> + clkdata->xtal_ena = ret & WM831X_XTAL_ENA;
> +
> + if (!clk_register(wm831x->dev, &wm831x_xtal_ops, &clkdata->xtal_hw,
> + "xtal")) {
> + ret = -EINVAL;
> + goto err_alloc;
> + }
> +
> + if (!clk_register(wm831x->dev, &wm831x_fll_ops, &clkdata->fll_hw,
> + "fll")) {
> + ret = -EINVAL;
> + goto err_xtal;
> + }
> +
> + if (!clk_register(wm831x->dev, &wm831x_clkout_ops, &clkdata->clkout_hw,
> + "clkout")) {
> + ret = -EINVAL;
> + goto err_fll;
> + }
How common will this pattern be? Is there need for a
clk_register_many() variant?
> +
> + dev_set_drvdata(&pdev->dev, clkdata);
> +
> + return 0;
> +
> +err_fll:
> + clk_unregister(clkdata->fll_hw.clk);
> +err_xtal:
> + clk_unregister(clkdata->xtal_hw.clk);
> +err_alloc:
> + kfree(clkdata);
> + return ret;
> +}
> +
> +static __devexit int wm831x_clk_remove(struct platform_device *pdev)
> +{
> + struct wm831x_clk *clkdata = dev_get_drvdata(&pdev->dev);
> +
> + clk_unregister(clkdata->clkout_hw.clk);
> + clk_unregister(clkdata->fll_hw.clk);
> + clk_unregister(clkdata->xtal_hw.clk);
> + kfree(clkdata);
> +
> + return 0;
> +}
> +
> +static struct platform_driver wm831x_clk_driver = {
> + .probe = wm831x_clk_probe,
> + .remove = __devexit_p(wm831x_clk_remove),
> + .driver = {
> + .name = "wm831x-clk",
> + .owner = THIS_MODULE,
> + },
> +};
> +
> +static int __init wm831x_clk_init(void)
> +{
> + int ret;
> +
> + ret = platform_driver_register(&wm831x_clk_driver);
> + if (ret != 0)
> + pr_err("Failed to register WM831x clock driver: %d\n", ret);
> +
> + return ret;
> +}
Driver registration is very well implemented and debugged. Just do
"return platform_driver_register();"
> +module_init(wm831x_clk_init);
> +
> +static void __exit wm831x_clk_exit(void)
> +{
> + platform_driver_unregister(&wm831x_clk_driver);
> +}
> +module_exit(wm831x_clk_exit);
> +
> +/* Module information */
> +MODULE_AUTHOR("Mark Brown <broonie@opensource.wolfsonmicro.com>");
> +MODULE_DESCRIPTION("WM831x clock driver");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:wm831x-clk");
> --
> 1.7.5.4
>
^ permalink raw reply
* Re: [PATCH 1/6] clk: Prototype and document clk_register()
From: Grant Likely @ 2011-07-15 2:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1310352837-4277-1-git-send-email-broonie@opensource.wolfsonmicro.com>
On Mon, Jul 11, 2011 at 11:53:52AM +0900, Mark Brown wrote:
> This allows the compiler to ensure drivers are using the correct prototype.
>
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
Acked-by: Grant Likely <grant.likely@secretlab.ca>
> ---
> include/linux/clk.h | 12 ++++++++++++
> 1 files changed, 12 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/clk.h b/include/linux/clk.h
> index 7c26135..2ca4f66 100644
> --- a/include/linux/clk.h
> +++ b/include/linux/clk.h
> @@ -136,6 +136,18 @@ extern struct clk_hw_ops clk_gate_ops;
>
> #endif /* CONFIG_GENERIC_CLK_GATE */
>
> +/**
> + * clk_register - register and initialize a new clock
> + *
> + * @ops: ops for the new clock
> + * @hw: struct clk_hw to be passed to the ops of the new clock
> + * @name: name to use for the new clock
> + *
> + * Register a new clock with the clk subsytem. Returns either a
> + * struct clk for the new clock or a NULL pointer.
> + */
> +struct clk *clk_register(struct clk_hw_ops *ops, struct clk_hw *hw,
> + const char *name);
>
> #else /* !CONFIG_GENERIC_CLK */
>
> --
> 1.7.5.4
>
^ permalink raw reply
* Re: [PATCH 0/3] PM / Domains / shmobile fixes
From: Rafael J. Wysocki @ 2011-07-14 19:34 UTC (permalink / raw)
To: Magnus Damm; +Cc: Linux PM mailing list, LKML, Paul Mundt, linux-sh
In-Reply-To: <CANqRtoTLO=DqehwanGdE+rgxL_ofa5YCJjsx47doHzwZDP4Ocw@mail.gmail.com>
On Thursday, July 14, 2011, Magnus Damm wrote:
> On Thu, Jul 14, 2011 at 6:52 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > Hi,
> >
> > The following three patches fix a couple of issues in the code currently
> > in my pm-domains branch at:
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/rafael/suspend-2.6.git pm-domains
> >
> > [1/3] - Use genpd_queue_power_off_work() for queuing up the powering off
> > of A4LC (this avoids attempting to queue up a work item while it is
> > pending).
> >
> > [2/3] - Make the generic PM domains code react to -EBUSY returned from a
> > PM domain's .power_down() callback (needed for [3/3]).
> >
> > [3/3] - Return -EBUSY from the A4LC's .power_down() callback to indicate that
> > the domain hasn't been powered down on purpose and remove the confusing
> > (and now redundant) pm_genpd_poweron(A4LC) from pd_power_down_a3rv().
>
> All patches above look great, thanks a lot for your help!
>
> Acked-by: Magnus Damm <damm@opensource.se>
Thanks!
Rafael
^ permalink raw reply
* Re: [PATCH 0/2] mmc: tmio: two bug-fixes
From: Chris Ball @ 2011-07-14 19:27 UTC (permalink / raw)
To: Guennadi Liakhovetski
Cc: linux-mmc, linux-sh, Magnus Damm, Rafael J. Wysocki, Ian Molton,
Simon Horman
In-Reply-To: <Pine.LNX.4.64.1107141828470.10688@axis700.grange>
Hi Guennadi,
On Thu, Jul 14 2011, Guennadi Liakhovetski wrote:
> There go two more bug-fixes for tmio on top of my earlier patches from
> today, so, should still be easy enough to trace. So, to pull them
> together, the previous ones were
>
> [PATCH v3] mmc: tmio: fix recursive spinlock, don't schedule with
> interrupts disabled
> [PATCH v3] mmc: tmio: maximize power saving
>
> And now follow
>
> [PATCH 1/2] mmc: tmio: fix a recently introduced bug in DMA code
> [PATCH 2/2] mmc: tmio: fix a deadlock
Pushed these four plus "mmc: sh_mmcif: maximize power saving" to
mmc-next for 3.1. Thanks!
By the way, feel free to send me a MAINTAINERS update taking over
TMIO if you'd like to; you've been doing the work. :-)
- Chris.
--
Chris Ball <cjb@laptop.org> <http://printf.net/>
One Laptop Per Child
^ permalink raw reply
* [PATCH 2/2] mmc: tmio: fix a deadlock
From: Guennadi Liakhovetski @ 2011-07-14 16:39 UTC (permalink / raw)
To: linux-mmc
Cc: linux-sh, Magnus Damm, Rafael J. Wysocki, Chris Ball, Ian Molton,
Simon Horman
In-Reply-To: <Pine.LNX.4.64.1107141828470.10688@axis700.grange>
Currently the tmio-mmc driver contains a recursive runtime PM method
invocation, which leads to a deadlock on a mutex. Avoid it by taking
care not to request DMA too early.
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
drivers/mmc/host/tmio_mmc.h | 5 +++++
drivers/mmc/host/tmio_mmc_dma.c | 5 ++++-
drivers/mmc/host/tmio_mmc_pio.c | 4 ++--
3 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 551d1ef..60ba335 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -109,6 +109,7 @@ static inline void tmio_mmc_kunmap_atomic(struct scatterlist *sg,
#if defined(CONFIG_MMC_SDHI) || defined(CONFIG_MMC_SDHI_MODULE)
void tmio_mmc_start_dma(struct tmio_mmc_host *host, struct mmc_data *data);
+void tmio_mmc_enable_dma(struct tmio_mmc_host *host, bool enable);
void tmio_mmc_request_dma(struct tmio_mmc_host *host, struct tmio_mmc_data *pdata);
void tmio_mmc_release_dma(struct tmio_mmc_host *host);
#else
@@ -117,6 +118,10 @@ static inline void tmio_mmc_start_dma(struct tmio_mmc_host *host,
{
}
+static inline void tmio_mmc_enable_dma(struct tmio_mmc_host *host, bool enable)
+{
+}
+
static inline void tmio_mmc_request_dma(struct tmio_mmc_host *host,
struct tmio_mmc_data *pdata)
{
diff --git a/drivers/mmc/host/tmio_mmc_dma.c b/drivers/mmc/host/tmio_mmc_dma.c
index 7e86662..2aa616d 100644
--- a/drivers/mmc/host/tmio_mmc_dma.c
+++ b/drivers/mmc/host/tmio_mmc_dma.c
@@ -22,8 +22,11 @@
#define TMIO_MMC_MIN_DMA_LEN 8
-static void tmio_mmc_enable_dma(struct tmio_mmc_host *host, bool enable)
+void tmio_mmc_enable_dma(struct tmio_mmc_host *host, bool enable)
{
+ if (!host->chan_tx || !host->chan_rx)
+ return;
+
#if defined(CONFIG_SUPERH) || defined(CONFIG_ARCH_SHMOBILE)
/* Switch DMA mode on or off - SuperH specific? */
sd_ctrl_write16(host, CTL_DMA_ENABLE, enable ? 2 : 0);
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 221ffb7..1f16357 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -984,7 +984,7 @@ int tmio_mmc_host_resume(struct device *dev)
if (host->pm_global) {
/* Runtime PM resume callback didn't run */
tmio_mmc_reset(host);
- tmio_mmc_request_dma(host, host->pdata);
+ tmio_mmc_enable_dma(host, true);
host->pm_global = false;
}
@@ -1007,7 +1007,7 @@ int tmio_mmc_host_runtime_resume(struct device *dev)
struct tmio_mmc_data *pdata = host->pdata;
tmio_mmc_reset(host);
- tmio_mmc_request_dma(host, host->pdata);
+ tmio_mmc_enable_dma(host, true);
if (pdata->power) {
/* Only entered after a card-insert interrupt */
--
1.7.2.5
^ permalink raw reply related
* [PATCH 1/2] mmc: tmio: fix a recently introduced bug in DMA code
From: Guennadi Liakhovetski @ 2011-07-14 16:39 UTC (permalink / raw)
To: linux-mmc
Cc: linux-sh, Magnus Damm, Rafael J. Wysocki, Chris Ball, Ian Molton,
Simon Horman
In-Reply-To: <Pine.LNX.4.64.1107141828470.10688@axis700.grange>
A recent commit "mmc: tmio: Share register access functions" has swapped
arguments of a macro and broken DMA with TMIO MMC. This patch fixes the
arguments back.
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
drivers/mmc/host/tmio_mmc_dma.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/mmc/host/tmio_mmc_dma.c b/drivers/mmc/host/tmio_mmc_dma.c
index f24a029..7e86662 100644
--- a/drivers/mmc/host/tmio_mmc_dma.c
+++ b/drivers/mmc/host/tmio_mmc_dma.c
@@ -26,7 +26,7 @@ static void tmio_mmc_enable_dma(struct tmio_mmc_host *host, bool enable)
{
#if defined(CONFIG_SUPERH) || defined(CONFIG_ARCH_SHMOBILE)
/* Switch DMA mode on or off - SuperH specific? */
- sd_ctrl_write16(host, enable ? 2 : 0, CTL_DMA_ENABLE);
+ sd_ctrl_write16(host, CTL_DMA_ENABLE, enable ? 2 : 0);
#endif
}
--
1.7.2.5
^ permalink raw reply related
* [PATCH 0/2] mmc: tmio: two bug-fixes
From: Guennadi Liakhovetski @ 2011-07-14 16:39 UTC (permalink / raw)
To: linux-mmc
Cc: linux-sh, Magnus Damm, Rafael J. Wysocki, Chris Ball, Ian Molton,
Simon Horman
So, Chris, I lied:-)
There go two more bug-fixes for tmio on top of my earlier patches from
today, so, should still be easy enough to trace. So, to pull them
together, the previous ones were
[PATCH v3] mmc: tmio: fix recursive spinlock, don't schedule with interrupts disabled
[PATCH v3] mmc: tmio: maximize power saving
And now follow
[PATCH 1/2] mmc: tmio: fix a recently introduced bug in DMA code
[PATCH 2/2] mmc: tmio: fix a deadlock
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply
* Re: [PATCH v2] mmc: tmio: fix recursive spinlock, don't schedule
From: Guennadi Liakhovetski @ 2011-07-14 10:26 UTC (permalink / raw)
To: Chris Ball
Cc: linux-mmc, linux-sh, Ian Molton, Kuninori Morimoto, Magnus Damm
In-Reply-To: <m2oc0y2pk6.fsf@bob.laptop.org>
Hi Chris
On Wed, 13 Jul 2011, Chris Ball wrote:
> Hi Guennadi,
>
> On Mon, Jun 20 2011, Guennadi Liakhovetski wrote:
> > Calling mmc_request_done() under a spinlock with interrupts disabled
> > leads to a recursive spin-lock on request retry path and to
> > scheduling in atomic context. This patch fixes both these problems
> > by moving mmc_request_done() to the scheduler workqueue.
> >
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > ---
> >
> > This is a bug-fix: without it the system Oopses with LOCKDEP enabled, so,
> > it should really go in 3.0. OTOH it is pretty intrusine and non-trivial,
> > so, reviews and tests are highly appreciated! Also, unfortunately, I
> > wasn't able to test it well enough with SDIO, because the driver for the
> > only SDIO card, that I have, reproducibly crashes the kernel:
>
> Having trouble working out how to apply this -- for example, in this hunk:
Right, I've been developing on top of other trees. As you probably have
seen, a couple of minutes ago I've updated my two patches and re-posted
them:
[PATCH v3] mmc: tmio: fix recursive spinlock, don't schedule with interrupts disabled
[PATCH v3] mmc: tmio: maximize power saving
That's it for this merge window for tmio from me so far;-) The outstanding
sh-mmcif patch
[PATCH/RFC] mmc: sh_mmcif: maximize power saving
is no longer an RFC, should have no conflicts and requires no updates.
Thanks
Guennadi
>
> > @@ -618,7 +631,8 @@ irqreturn_t tmio_mmc_irq(int irq, void *devid)
> > if (ireg & (TMIO_STAT_CARD_INSERT | TMIO_STAT_CARD_REMOVE)) {
> > tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_CARD_INSERT |
> > TMIO_STAT_CARD_REMOVE);
> > - mmc_detect_change(host->mmc, msecs_to_jiffies(100));
> > + if (!work_pending(&host->mmc->detect.work))
> > + mmc_detect_change(host->mmc, msecs_to_jiffies(100));
> > }
> >
> > /* CRC and other errors */
>
> In mmc-next there's a "goto out;" after the mmc_detect_change call, which
> looks like it's always been there. Am I missing a patch this depends on?
>
> (It'd be a good time to get a full set of tmio patches for 3.1 pulled
> together, if you can do that.)
>
> Thanks!
>
> - Chris.
> --
> Chris Ball <cjb@laptop.org> <http://printf.net/>
> One Laptop Per Child
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply
* [PATCH v3] mmc: tmio: maximize power saving
From: Guennadi Liakhovetski @ 2011-07-14 10:16 UTC (permalink / raw)
To: linux-mmc
Cc: linux-sh, Magnus Damm, Rafael J. Wysocki, Chris Ball, Ian Molton,
Simon Horman
In-Reply-To: <Pine.LNX.4.64.1107140125460.30737@axis700.grange>
This patch uses runtime PM to allow the system to power down the MMC
controller, when the MMC closk is switched off.
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
v3: updated on top of current mmc-next plus my earlier patch "mmc: tmio:
fix recursive spinlock, don't schedule with interrupts disabled"
v2: Primarily tested on mackerel, with runtime PM and system-wide suspend,
updated to the current core and sh-mobile runtime PM code. To avoid
regression should go in after "ARM: mach-shmobile: use GPIO interrupt also
for card eject on SDHI0 on mackerel":
http://article.gmane.org/gmane.linux.kernel.mmc/9230
drivers/mmc/host/tmio_mmc.h | 2 +
drivers/mmc/host/tmio_mmc_pio.c | 64 ++++++++++++++++++++++----------------
2 files changed, 39 insertions(+), 27 deletions(-)
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index a2d6f9f..551d1ef 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -53,6 +53,8 @@ struct tmio_mmc_host {
void (*set_clk_div)(struct platform_device *host, int state);
int pm_error;
+ /* recognise system-wide suspend in runtime PM methods */
+ bool pm_global;
/* pio related stuff */
struct scatterlist *sg_ptr;
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index a2f76ad..221ffb7 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -546,6 +546,7 @@ out:
irqreturn_t tmio_mmc_irq(int irq, void *devid)
{
struct tmio_mmc_host *host = devid;
+ struct mmc_host *mmc = host->mmc;
struct tmio_mmc_data *pdata = host->pdata;
unsigned int ireg, irq_mask, status;
unsigned int sdio_ireg, sdio_irq_mask, sdio_status;
@@ -567,13 +568,13 @@ irqreturn_t tmio_mmc_irq(int irq, void *devid)
if (sdio_ireg && !host->sdio_irq_enabled) {
pr_warning("tmio_mmc: Spurious SDIO IRQ, disabling! 0x%04x 0x%04x 0x%04x\n",
sdio_status, sdio_irq_mask, sdio_ireg);
- tmio_mmc_enable_sdio_irq(host->mmc, 0);
+ tmio_mmc_enable_sdio_irq(mmc, 0);
goto out;
}
- if (host->mmc->caps & MMC_CAP_SDIO_IRQ &&
+ if (mmc->caps & MMC_CAP_SDIO_IRQ &&
sdio_ireg & TMIO_SDIO_STAT_IOIRQ)
- mmc_signal_sdio_irq(host->mmc);
+ mmc_signal_sdio_irq(mmc);
if (sdio_ireg)
goto out;
@@ -586,7 +587,9 @@ irqreturn_t tmio_mmc_irq(int irq, void *devid)
if (ireg & (TMIO_STAT_CARD_INSERT | TMIO_STAT_CARD_REMOVE)) {
tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_CARD_INSERT |
TMIO_STAT_CARD_REMOVE);
- if (!work_pending(&host->mmc->detect.work))
+ if ((((ireg & TMIO_STAT_CARD_REMOVE) && mmc->card) ||
+ ((ireg & TMIO_STAT_CARD_INSERT) && !mmc->card)) &&
+ !work_pending(&mmc->detect.work))
mmc_detect_change(host->mmc, msecs_to_jiffies(100));
goto out;
}
@@ -743,33 +746,30 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
spin_unlock_irqrestore(&host->lock, flags);
- if (ios->clock)
- tmio_mmc_set_clock(host, ios->clock);
-
- /* Power sequence - OFF -> UP -> ON */
- if (ios->power_mode = MMC_POWER_UP) {
- if ((pdata->flags & TMIO_MMC_HAS_COLD_CD) && !pdata->power) {
+ /*
+ * pdata->power = false only if COLD_CD is available, otherwise only
+ * in short time intervals during probing or resuming
+ */
+ if (ios->power_mode = MMC_POWER_ON && ios->clock) {
+ if (!pdata->power) {
pm_runtime_get_sync(&host->pdev->dev);
pdata->power = true;
}
+ tmio_mmc_set_clock(host, ios->clock);
/* power up SD bus */
if (host->set_pwr)
host->set_pwr(host->pdev, 1);
- } else if (ios->power_mode = MMC_POWER_OFF || !ios->clock) {
- /* power down SD bus */
- if (ios->power_mode = MMC_POWER_OFF) {
- if (host->set_pwr)
- host->set_pwr(host->pdev, 0);
- if ((pdata->flags & TMIO_MMC_HAS_COLD_CD) &&
- pdata->power) {
- pdata->power = false;
- pm_runtime_put(&host->pdev->dev);
- }
- }
- tmio_mmc_clk_stop(host);
- } else {
/* start bus clock */
tmio_mmc_clk_start(host);
+ } else if (ios->power_mode != MMC_POWER_UP) {
+ if (host->set_pwr)
+ host->set_pwr(host->pdev, 0);
+ if ((pdata->flags & TMIO_MMC_HAS_COLD_CD) &&
+ pdata->power) {
+ pdata->power = false;
+ pm_runtime_put(&host->pdev->dev);
+ }
+ tmio_mmc_clk_stop(host);
}
switch (ios->bus_width) {
@@ -897,8 +897,10 @@ int __devinit tmio_mmc_host_probe(struct tmio_mmc_host **host,
tmio_mmc_request_dma(_host, pdata);
/* We have to keep the device powered for its card detection to work */
- if (!(pdata->flags & TMIO_MMC_HAS_COLD_CD))
+ if (!(pdata->flags & TMIO_MMC_HAS_COLD_CD)) {
+ pdata->power = true;
pm_runtime_get_noresume(&pdev->dev);
+ }
mmc_add_host(mmc);
@@ -975,11 +977,16 @@ int tmio_mmc_host_resume(struct device *dev)
/* The MMC core will perform the complete set up */
host->pdata->power = false;
+ host->pm_global = true;
if (!host->pm_error)
pm_runtime_get_sync(dev);
- tmio_mmc_reset(mmc_priv(mmc));
- tmio_mmc_request_dma(host, host->pdata);
+ if (host->pm_global) {
+ /* Runtime PM resume callback didn't run */
+ tmio_mmc_reset(host);
+ tmio_mmc_request_dma(host, host->pdata);
+ host->pm_global = false;
+ }
return mmc_resume_host(mmc);
}
@@ -1000,12 +1007,15 @@ int tmio_mmc_host_runtime_resume(struct device *dev)
struct tmio_mmc_data *pdata = host->pdata;
tmio_mmc_reset(host);
+ tmio_mmc_request_dma(host, host->pdata);
if (pdata->power) {
/* Only entered after a card-insert interrupt */
- tmio_mmc_set_ios(mmc, &mmc->ios);
+ if (!mmc->card)
+ tmio_mmc_set_ios(mmc, &mmc->ios);
mmc_detect_change(mmc, msecs_to_jiffies(100));
}
+ host->pm_global = false;
return 0;
}
--
1.7.2.5
^ permalink raw reply related
* [PATCH v3] mmc: tmio: fix recursive spinlock, don't schedule with
From: Guennadi Liakhovetski @ 2011-07-14 10:12 UTC (permalink / raw)
To: linux-mmc
Cc: linux-sh, Ian Molton, Kuninori Morimoto, Chris Ball, Magnus Damm
In-Reply-To: <Pine.LNX.4.64.1106201803520.11365@axis700.grange>
Calling mmc_request_done() under a spinlock with interrupts disabled
leads to a recursive spin-lock on request retry path and to
scheduling in atomic context. This patch fixes both these problems
by moving mmc_request_done() to the scheduler workqueue.
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
This is a bug-fix: without it the system Oopses with LOCKDEP enabled, so,
it should really go in 3.0. OTOH it is pretty intrusine and non-trivial,
so, reviews and tests are highly appreciated! Also, unfortunately, I
wasn't able to test it well enough with SDIO, because the driver for the
only SDIO card, that I have, reproducibly crashes the kernel:
http://www.spinics.net/lists/kernel/msg1203334.html
So, mainly tested with SD-cards and only lightly with SDIO.
v3: no functional changes, only updated on top of current mmc-next
v2:
1. added a mutex to properly complete each .set_ios() call instead of
returning an error, when racing with another one.
2. oritect data inside tmio_mmc_finish_request()
3. don't reschedule card detection, if one is already pending
drivers/mmc/host/tmio_mmc.h | 6 +++++-
drivers/mmc/host/tmio_mmc_pio.c | 35 +++++++++++++++++++++++++++++------
2 files changed, 34 insertions(+), 7 deletions(-)
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 0c22df0..a2d6f9f 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -18,6 +18,7 @@
#include <linux/highmem.h>
#include <linux/mmc/tmio.h>
+#include <linux/mutex.h>
#include <linux/pagemap.h>
#include <linux/spinlock.h>
@@ -73,8 +74,11 @@ struct tmio_mmc_host {
/* Track lost interrupts */
struct delayed_work delayed_reset_work;
- spinlock_t lock;
+ struct work_struct done;
+
+ spinlock_t lock; /* protect host private data */
unsigned long last_req_ts;
+ struct mutex ios_lock; /* protect set_ios() context */
};
int tmio_mmc_host_probe(struct tmio_mmc_host **host,
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index f7dd3b1..a2f76ad 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -250,10 +250,16 @@ static void tmio_mmc_reset_work(struct work_struct *work)
/* called with host->lock held, interrupts disabled */
static void tmio_mmc_finish_request(struct tmio_mmc_host *host)
{
- struct mmc_request *mrq = host->mrq;
+ struct mmc_request *mrq;
+ unsigned long flags;
- if (!mrq)
+ spin_lock_irqsave(&host->lock, flags);
+
+ mrq = host->mrq;
+ if (IS_ERR_OR_NULL(mrq)) {
+ spin_unlock_irqrestore(&host->lock, flags);
return;
+ }
host->cmd = NULL;
host->data = NULL;
@@ -262,11 +268,18 @@ static void tmio_mmc_finish_request(struct tmio_mmc_host *host)
cancel_delayed_work(&host->delayed_reset_work);
host->mrq = NULL;
+ spin_unlock_irqrestore(&host->lock, flags);
- /* FIXME: mmc_request_done() can schedule! */
mmc_request_done(host->mmc, mrq);
}
+static void tmio_mmc_done_work(struct work_struct *work)
+{
+ struct tmio_mmc_host *host = container_of(work, struct tmio_mmc_host,
+ done);
+ tmio_mmc_finish_request(host);
+}
+
/* These are the bitmasks the tmio chip requires to implement the MMC response
* types. Note that R1 and R6 are the same in this scheme. */
#define APP_CMD 0x0040
@@ -433,7 +446,7 @@ void tmio_mmc_do_data_irq(struct tmio_mmc_host *host)
BUG();
}
- tmio_mmc_finish_request(host);
+ schedule_work(&host->done);
}
static void tmio_mmc_data_irq(struct tmio_mmc_host *host)
@@ -523,7 +536,7 @@ static void tmio_mmc_cmd_irq(struct tmio_mmc_host *host,
tasklet_schedule(&host->dma_issue);
}
} else {
- tmio_mmc_finish_request(host);
+ schedule_work(&host->done);
}
out:
@@ -573,7 +586,8 @@ irqreturn_t tmio_mmc_irq(int irq, void *devid)
if (ireg & (TMIO_STAT_CARD_INSERT | TMIO_STAT_CARD_REMOVE)) {
tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_CARD_INSERT |
TMIO_STAT_CARD_REMOVE);
- mmc_detect_change(host->mmc, msecs_to_jiffies(100));
+ if (!work_pending(&host->mmc->detect.work))
+ mmc_detect_change(host->mmc, msecs_to_jiffies(100));
goto out;
}
@@ -703,6 +717,8 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
struct tmio_mmc_data *pdata = host->pdata;
unsigned long flags;
+ mutex_lock(&host->ios_lock);
+
spin_lock_irqsave(&host->lock, flags);
if (host->mrq) {
if (IS_ERR(host->mrq)) {
@@ -718,6 +734,8 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
host->mrq->cmd->opcode, host->last_req_ts, jiffies);
}
spin_unlock_irqrestore(&host->lock, flags);
+
+ mutex_unlock(&host->ios_lock);
return;
}
@@ -771,6 +789,8 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
current->comm, task_pid_nr(current),
ios->clock, ios->power_mode);
host->mrq = NULL;
+
+ mutex_unlock(&host->ios_lock);
}
static int tmio_mmc_get_ro(struct mmc_host *mmc)
@@ -867,9 +887,11 @@ int __devinit tmio_mmc_host_probe(struct tmio_mmc_host **host,
tmio_mmc_enable_sdio_irq(mmc, 0);
spin_lock_init(&_host->lock);
+ mutex_init(&_host->ios_lock);
/* Init delayed work for request timeouts */
INIT_DELAYED_WORK(&_host->delayed_reset_work, tmio_mmc_reset_work);
+ INIT_WORK(&_host->done, tmio_mmc_done_work);
/* See if we also get DMA */
tmio_mmc_request_dma(_host, pdata);
@@ -917,6 +939,7 @@ void tmio_mmc_host_remove(struct tmio_mmc_host *host)
pm_runtime_get_sync(&pdev->dev);
mmc_remove_host(host->mmc);
+ cancel_work_sync(&host->done);
cancel_delayed_work_sync(&host->delayed_reset_work);
tmio_mmc_release_dma(host);
--
1.7.2.5
^ permalink raw reply related
* Re: [PATCH] ARM: mach-shmobile: use GPIO interrupt also for card
From: Guennadi Liakhovetski @ 2011-07-14 8:04 UTC (permalink / raw)
To: Magnus Damm; +Cc: linux-mmc, linux-sh, Magnus Damm
In-Reply-To: <CANqRtoQ+pi-2ek+z2NXhPiVj2prCs8wZ6NYiOX44HQ1NZrei7w@mail.gmail.com>
Hi Magnus
On Thu, 14 Jul 2011, Magnus Damm wrote:
> On Thu, Jul 14, 2011 at 8:25 AM, Guennadi Liakhovetski
> <g.liakhovetski@gmx.de> wrote:
> > When we switch to transaction-based runtime PM on SDHI / TMIO MMC,
> > also card eject events will have to be detected by the platform.
> > This patch prepares mackerel to this switch.
> >
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > ---
> > arch/arm/mach-shmobile/board-mackerel.c | 68 +++++++++++++++++++++++++++----
> > 1 files changed, 60 insertions(+), 8 deletions(-)
>
> Thanks for your work on this.
>
> What is the reason why we can't use GPIO for all cases of insert/eject
> event detection? Switching between GPIO and function mode for the pin
> seems overly complex IMO - that unless there is a real win behind it.
> Please explain.
Because the way, board-supported (cold) card-detection is currently
implemented on mackerel is, that on a card slot event a pm_runtime_get()
is issued, which then calls the .runtime_resume() hook in the tmio driver.
However, if the runtime PM is inactive, if the hook doesn't get called,
e.g., bacause the power domain, the respective SDHI controller is in, is
held on by other devices, the event would be missed. That's why the normal
pin state is the SDHI card-detection function, which then works performs
the detection in such cases.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply
* Re: [PATCH 2/2] i2c: i2c-riic: add dmaengine supporting
From: Yoshihiro Shimoda @ 2011-07-14 2:17 UTC (permalink / raw)
To: Ben Dooks
Cc: ben-linux-elnMNo+KYs3YtjvyW6yDsg,
linux-i2c-u79uwXL29TY76Z2rM5mHXA, SH-Linux
In-Reply-To: <20110713201018.GD3369-RazCHl0VsYgkUSuvROHNpA@public.gmane.org>
Hi Ben,
2011/07/14 5:10, Ben Dooks wrote:
> On Fri, Jul 01, 2011 at 10:00:50AM +0900, Yoshihiro Shimoda wrote:
>> diff --git a/drivers/i2c/busses/i2c-riic.c b/drivers/i2c/busses/i2c-riic.c
>> index dcc183b..22dd779 100644
>> --- a/drivers/i2c/busses/i2c-riic.c
>> +++ b/drivers/i2c/busses/i2c-riic.c
>> @@ -30,6 +30,7 @@
>> #include <linux/timer.h>
>> #include <linux/delay.h>
>> #include <linux/slab.h>
>> +#include <linux/dmaengine.h>
>>
>> #define RIIC_ICCR1 0x00
>> #define RIIC_ICCR2 0x01
>> @@ -164,6 +165,14 @@ struct riic_data {
>> void __iomem *reg;
>> struct i2c_adapter adap;
>> struct i2c_msg *msg;
>> + int index;
>> + unsigned char icsr2;
>> +
>> + /* for DMAENGINE */
>> + struct dma_chan *chan_tx;
>> + struct dma_chan *chan_rx;
>> + int dma_callbacked;
>> + wait_queue_head_t wait;
>> };
>
> What happens if the driver is compiled into a kernel
> without dma engine support?
I could compile the code without enabling "CONFIG_DMA_ENGINE".
This is because The "struct dma_chan" is always defined in
the "include/linux/dmaengine.h", I think.
>> +static int riic_master_transmit_dma(struct riic_data *pd)
>> +{
>> + struct scatterlist sg;
>> + unsigned char *buf = pd->msg->buf;
>> + struct dma_async_tx_descriptor *desc;
>> + int ret;
>> +
>> + sg_init_table(&sg, 1);
>> + sg_set_buf(&sg, buf, pd->msg->len);
>> + sg_dma_len(&sg) = pd->msg->len;
>> + dma_map_sg(pd->chan_tx->device->dev, &sg, 1, DMA_TO_DEVICE);
>> +
>> + desc = pd->chan_tx->device->device_prep_slave_sg(pd->chan_tx,
>> + &sg, 1, DMA_TO_DEVICE,
>> + DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
>
> surely there's a better way of calling this?
Umm, I checked other drivers in the "drivers/", but I didn't find
a better way. Also other drivers was similar code...
>> + dmaengine_submit(desc);
>> + dma_async_issue_pending(pd->chan_rx);
>> +
>> + ret = wait_event_timeout(pd->wait, pd->dma_callbacked, HZ);
>> + if (!pd->dma_callbacked && !ret) {
>
> hmm, if you did not time out, then you should be able to
> infer the pd->dma_callbacked?
If this did not time out, the pd->dma_callbacked is always set.
So, I will fix the code to "if (!ret && !pd->dma_callbacked) {".
>> +static void riic_request_dma(struct riic_data *pd,
>> + struct riic_platform_data *riic_data)
>> +{
>> + dma_cap_mask_t mask;
>> +
>> + if (riic_data->dma_tx.slave_id) {
>> + dma_cap_zero(mask);
>> + dma_cap_set(DMA_SLAVE, mask);
>> + pd->chan_tx = dma_request_channel(mask, riic_filter,
>> + &riic_data->dma_tx);
>> + }
>> + if (riic_data->dma_rx.slave_id) {
>> + dma_cap_zero(mask);
>> + dma_cap_set(DMA_SLAVE, mask);
>> + pd->chan_rx = dma_request_channel(mask, riic_filter,
>> + &riic_data->dma_rx);
>> + }
>> +}
>
> should there be a warning if the slave_id is valid and
> the dma channel cannot be requested?
I will add a warning message.
However, even if the driver doesn't use dma, it can use PIO mode.
Best regards,
Yoshihiro Shimoda
^ 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