* [PATCH] BLACKFIN MEDIA DRIVER: rewrite the blackfin style of read/write into common style @ 2015-01-14 6:57 Hao Liang 2015-01-16 11:34 ` Hans Verkuil 0 siblings, 1 reply; 4+ messages in thread From: Hao Liang @ 2015-01-14 6:57 UTC (permalink / raw) To: scott.jiang.linux, mchehab Cc: linux-media, linux-kernel, Hao.Liang, adi-buildroot-devel, Hao Liang Signed-off-by: Hao Liang <hliang1025@gmail.com> --- drivers/media/platform/blackfin/ppi.c | 72 ++++++++++++++++----------------- 1 file changed, 35 insertions(+), 37 deletions(-) diff --git a/drivers/media/platform/blackfin/ppi.c b/drivers/media/platform/blackfin/ppi.c index cff63e5..de4b5c7 100644 --- a/drivers/media/platform/blackfin/ppi.c +++ b/drivers/media/platform/blackfin/ppi.c @@ -20,6 +20,7 @@ #include <linux/module.h> #include <linux/slab.h> #include <linux/platform_device.h> +#include <linux/io.h> #include <asm/bfin_ppi.h> #include <asm/blackfin.h> @@ -59,10 +60,10 @@ static irqreturn_t ppi_irq_err(int irq, void *dev_id) /* register on bf561 is cleared when read * others are W1C */ - status = bfin_read16(®->status); + status = readw(®->status); if (status & 0x3000) ppi->err = true; - bfin_write16(®->status, 0xff00); + writew(0xff00, ®->status); break; } case PPI_TYPE_EPPI: @@ -70,10 +71,10 @@ static irqreturn_t ppi_irq_err(int irq, void *dev_id) struct bfin_eppi_regs *reg = info->base; unsigned short status; - status = bfin_read16(®->status); + status = readw(®->status); if (status & 0x2) ppi->err = true; - bfin_write16(®->status, 0xffff); + writew(0xffff, ®->status); break; } case PPI_TYPE_EPPI3: @@ -81,10 +82,10 @@ static irqreturn_t ppi_irq_err(int irq, void *dev_id) struct bfin_eppi3_regs *reg = info->base; unsigned long stat; - stat = bfin_read32(®->stat); + stat = readl(®->stat); if (stat & 0x2) ppi->err = true; - bfin_write32(®->stat, 0xc0ff); + writel(0xc0ff, ®->stat); break; } default: @@ -139,26 +140,25 @@ static int ppi_start(struct ppi_if *ppi) case PPI_TYPE_PPI: { struct bfin_ppi_regs *reg = info->base; - bfin_write16(®->control, ppi->ppi_control); + writew(ppi->ppi_control, ®->control); break; } case PPI_TYPE_EPPI: { struct bfin_eppi_regs *reg = info->base; - bfin_write32(®->control, ppi->ppi_control); + writel(ppi->ppi_control, ®->control); break; } case PPI_TYPE_EPPI3: { struct bfin_eppi3_regs *reg = info->base; - bfin_write32(®->ctl, ppi->ppi_control); + writel(ppi->ppi_control, ®->ctl); break; } default: return -EINVAL; } - SSYNC(); return 0; } @@ -172,19 +172,19 @@ static int ppi_stop(struct ppi_if *ppi) case PPI_TYPE_PPI: { struct bfin_ppi_regs *reg = info->base; - bfin_write16(®->control, ppi->ppi_control); + writew(ppi->ppi_control, ®->control); break; } case PPI_TYPE_EPPI: { struct bfin_eppi_regs *reg = info->base; - bfin_write32(®->control, ppi->ppi_control); + writel(ppi->ppi_control, ®->control); break; } case PPI_TYPE_EPPI3: { struct bfin_eppi3_regs *reg = info->base; - bfin_write32(®->ctl, ppi->ppi_control); + writel(ppi->ppi_control, ®->ctl); break; } default: @@ -195,7 +195,6 @@ static int ppi_stop(struct ppi_if *ppi) clear_dma_irqstat(info->dma_ch); disable_dma(info->dma_ch); - SSYNC(); return 0; } @@ -242,9 +241,9 @@ static int ppi_set_params(struct ppi_if *ppi, struct ppi_params *params) if (params->ppi_control & DMA32) dma32 = 1; - bfin_write16(®->control, ppi->ppi_control); - bfin_write16(®->count, samples_per_line - 1); - bfin_write16(®->frame, params->frame); + writew(ppi->ppi_control, ®->control); + writew(samples_per_line - 1, ®->count); + writew(params->frame, ®->frame); break; } case PPI_TYPE_EPPI: @@ -255,13 +254,13 @@ static int ppi_set_params(struct ppi_if *ppi, struct ppi_params *params) || (params->ppi_control & 0x38000) > DLEN_16) dma32 = 1; - bfin_write32(®->control, ppi->ppi_control); - bfin_write16(®->line, samples_per_line); - bfin_write16(®->frame, params->frame); - bfin_write16(®->hdelay, hdelay); - bfin_write16(®->vdelay, params->vdelay); - bfin_write16(®->hcount, hcount); - bfin_write16(®->vcount, params->height); + writel(ppi->ppi_control, ®->control); + writew(samples_per_line, ®->line); + writew(params->frame, ®->frame); + writew(hdelay, ®->hdelay); + writew(params->vdelay, ®->vdelay); + writew(hcount, ®->hcount); + writew(params->height, ®->vcount); break; } case PPI_TYPE_EPPI3: @@ -272,15 +271,15 @@ static int ppi_set_params(struct ppi_if *ppi, struct ppi_params *params) || (params->ppi_control & 0x70000) > DLEN_16) dma32 = 1; - bfin_write32(®->ctl, ppi->ppi_control); - bfin_write32(®->line, samples_per_line); - bfin_write32(®->frame, params->frame); - bfin_write32(®->hdly, hdelay); - bfin_write32(®->vdly, params->vdelay); - bfin_write32(®->hcnt, hcount); - bfin_write32(®->vcnt, params->height); + writel(ppi->ppi_control, ®->ctl); + writel(samples_per_line, ®->line); + writel(params->frame, ®->frame); + writel(hdelay, ®->hdly); + writel(params->vdelay, ®->vdly); + writel(hcount, ®->hcnt); + writel(params->height, ®->vcnt); if (params->int_mask) - bfin_write32(®->imsk, params->int_mask & 0xFF); + writel(params->int_mask & 0xFF, ®->imsk); if (ppi->ppi_control & PORT_DIR) { u32 hsync_width, vsync_width, vsync_period; @@ -288,10 +287,10 @@ static int ppi_set_params(struct ppi_if *ppi, struct ppi_params *params) * params->bpp / params->dlen; vsync_width = params->vsync * samples_per_line; vsync_period = samples_per_line * params->frame; - bfin_write32(®->fs1_wlhb, hsync_width); - bfin_write32(®->fs1_paspl, samples_per_line); - bfin_write32(®->fs2_wlvb, vsync_width); - bfin_write32(®->fs2_palpf, vsync_period); + writel(hsync_width, ®->fs1_wlhb); + writel(samples_per_line, ®->fs1_paspl); + writel(vsync_width, ®->fs2_wlvb); + writel(vsync_period, ®->fs2_palpf); } break; } @@ -313,7 +312,6 @@ static int ppi_set_params(struct ppi_if *ppi, struct ppi_params *params) set_dma_y_count(info->dma_ch, params->height); set_dma_config(info->dma_ch, dma_config); - SSYNC(); return 0; } -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] BLACKFIN MEDIA DRIVER: rewrite the blackfin style of read/write into common style 2015-01-14 6:57 [PATCH] BLACKFIN MEDIA DRIVER: rewrite the blackfin style of read/write into common style Hao Liang @ 2015-01-16 11:34 ` Hans Verkuil [not found] ` <CAPig_tmCBeNQXqc+8-Sn+5h-Gq_XT8VBHmhtfy48eoAgm75bCw@mail.gmail.com> 0 siblings, 1 reply; 4+ messages in thread From: Hans Verkuil @ 2015-01-16 11:34 UTC (permalink / raw) To: Hao Liang, scott.jiang.linux, mchehab Cc: linux-media, linux-kernel, Hao.Liang, adi-buildroot-devel Hi Hao, Why would you do this? read/writew() is AFAICT not the same as bfin_read/write16 (defined in arch/blackfin/include/asm/def_LPBlackfin.h). And all other blackfin sources I've seen all use bfin_read/write. So unless there is a good reason for this change I am not going to accept this. Regards, Hans On 01/14/2015 07:57 AM, Hao Liang wrote: > Signed-off-by: Hao Liang <hliang1025@gmail.com> > --- > drivers/media/platform/blackfin/ppi.c | 72 ++++++++++++++++----------------- > 1 file changed, 35 insertions(+), 37 deletions(-) > > diff --git a/drivers/media/platform/blackfin/ppi.c b/drivers/media/platform/blackfin/ppi.c > index cff63e5..de4b5c7 100644 > --- a/drivers/media/platform/blackfin/ppi.c > +++ b/drivers/media/platform/blackfin/ppi.c > @@ -20,6 +20,7 @@ > #include <linux/module.h> > #include <linux/slab.h> > #include <linux/platform_device.h> > +#include <linux/io.h> > > #include <asm/bfin_ppi.h> > #include <asm/blackfin.h> > @@ -59,10 +60,10 @@ static irqreturn_t ppi_irq_err(int irq, void *dev_id) > /* register on bf561 is cleared when read > * others are W1C > */ > - status = bfin_read16(®->status); > + status = readw(®->status); > if (status & 0x3000) > ppi->err = true; > - bfin_write16(®->status, 0xff00); > + writew(0xff00, ®->status); > break; > } > case PPI_TYPE_EPPI: > @@ -70,10 +71,10 @@ static irqreturn_t ppi_irq_err(int irq, void *dev_id) > struct bfin_eppi_regs *reg = info->base; > unsigned short status; > > - status = bfin_read16(®->status); > + status = readw(®->status); > if (status & 0x2) > ppi->err = true; > - bfin_write16(®->status, 0xffff); > + writew(0xffff, ®->status); > break; > } > case PPI_TYPE_EPPI3: > @@ -81,10 +82,10 @@ static irqreturn_t ppi_irq_err(int irq, void *dev_id) > struct bfin_eppi3_regs *reg = info->base; > unsigned long stat; > > - stat = bfin_read32(®->stat); > + stat = readl(®->stat); > if (stat & 0x2) > ppi->err = true; > - bfin_write32(®->stat, 0xc0ff); > + writel(0xc0ff, ®->stat); > break; > } > default: > @@ -139,26 +140,25 @@ static int ppi_start(struct ppi_if *ppi) > case PPI_TYPE_PPI: > { > struct bfin_ppi_regs *reg = info->base; > - bfin_write16(®->control, ppi->ppi_control); > + writew(ppi->ppi_control, ®->control); > break; > } > case PPI_TYPE_EPPI: > { > struct bfin_eppi_regs *reg = info->base; > - bfin_write32(®->control, ppi->ppi_control); > + writel(ppi->ppi_control, ®->control); > break; > } > case PPI_TYPE_EPPI3: > { > struct bfin_eppi3_regs *reg = info->base; > - bfin_write32(®->ctl, ppi->ppi_control); > + writel(ppi->ppi_control, ®->ctl); > break; > } > default: > return -EINVAL; > } > > - SSYNC(); > return 0; > } > > @@ -172,19 +172,19 @@ static int ppi_stop(struct ppi_if *ppi) > case PPI_TYPE_PPI: > { > struct bfin_ppi_regs *reg = info->base; > - bfin_write16(®->control, ppi->ppi_control); > + writew(ppi->ppi_control, ®->control); > break; > } > case PPI_TYPE_EPPI: > { > struct bfin_eppi_regs *reg = info->base; > - bfin_write32(®->control, ppi->ppi_control); > + writel(ppi->ppi_control, ®->control); > break; > } > case PPI_TYPE_EPPI3: > { > struct bfin_eppi3_regs *reg = info->base; > - bfin_write32(®->ctl, ppi->ppi_control); > + writel(ppi->ppi_control, ®->ctl); > break; > } > default: > @@ -195,7 +195,6 @@ static int ppi_stop(struct ppi_if *ppi) > clear_dma_irqstat(info->dma_ch); > disable_dma(info->dma_ch); > > - SSYNC(); > return 0; > } > > @@ -242,9 +241,9 @@ static int ppi_set_params(struct ppi_if *ppi, struct ppi_params *params) > if (params->ppi_control & DMA32) > dma32 = 1; > > - bfin_write16(®->control, ppi->ppi_control); > - bfin_write16(®->count, samples_per_line - 1); > - bfin_write16(®->frame, params->frame); > + writew(ppi->ppi_control, ®->control); > + writew(samples_per_line - 1, ®->count); > + writew(params->frame, ®->frame); > break; > } > case PPI_TYPE_EPPI: > @@ -255,13 +254,13 @@ static int ppi_set_params(struct ppi_if *ppi, struct ppi_params *params) > || (params->ppi_control & 0x38000) > DLEN_16) > dma32 = 1; > > - bfin_write32(®->control, ppi->ppi_control); > - bfin_write16(®->line, samples_per_line); > - bfin_write16(®->frame, params->frame); > - bfin_write16(®->hdelay, hdelay); > - bfin_write16(®->vdelay, params->vdelay); > - bfin_write16(®->hcount, hcount); > - bfin_write16(®->vcount, params->height); > + writel(ppi->ppi_control, ®->control); > + writew(samples_per_line, ®->line); > + writew(params->frame, ®->frame); > + writew(hdelay, ®->hdelay); > + writew(params->vdelay, ®->vdelay); > + writew(hcount, ®->hcount); > + writew(params->height, ®->vcount); > break; > } > case PPI_TYPE_EPPI3: > @@ -272,15 +271,15 @@ static int ppi_set_params(struct ppi_if *ppi, struct ppi_params *params) > || (params->ppi_control & 0x70000) > DLEN_16) > dma32 = 1; > > - bfin_write32(®->ctl, ppi->ppi_control); > - bfin_write32(®->line, samples_per_line); > - bfin_write32(®->frame, params->frame); > - bfin_write32(®->hdly, hdelay); > - bfin_write32(®->vdly, params->vdelay); > - bfin_write32(®->hcnt, hcount); > - bfin_write32(®->vcnt, params->height); > + writel(ppi->ppi_control, ®->ctl); > + writel(samples_per_line, ®->line); > + writel(params->frame, ®->frame); > + writel(hdelay, ®->hdly); > + writel(params->vdelay, ®->vdly); > + writel(hcount, ®->hcnt); > + writel(params->height, ®->vcnt); > if (params->int_mask) > - bfin_write32(®->imsk, params->int_mask & 0xFF); > + writel(params->int_mask & 0xFF, ®->imsk); > if (ppi->ppi_control & PORT_DIR) { > u32 hsync_width, vsync_width, vsync_period; > > @@ -288,10 +287,10 @@ static int ppi_set_params(struct ppi_if *ppi, struct ppi_params *params) > * params->bpp / params->dlen; > vsync_width = params->vsync * samples_per_line; > vsync_period = samples_per_line * params->frame; > - bfin_write32(®->fs1_wlhb, hsync_width); > - bfin_write32(®->fs1_paspl, samples_per_line); > - bfin_write32(®->fs2_wlvb, vsync_width); > - bfin_write32(®->fs2_palpf, vsync_period); > + writel(hsync_width, ®->fs1_wlhb); > + writel(samples_per_line, ®->fs1_paspl); > + writel(vsync_width, ®->fs2_wlvb); > + writel(vsync_period, ®->fs2_palpf); > } > break; > } > @@ -313,7 +312,6 @@ static int ppi_set_params(struct ppi_if *ppi, struct ppi_params *params) > set_dma_y_count(info->dma_ch, params->height); > set_dma_config(info->dma_ch, dma_config); > > - SSYNC(); > return 0; > } > > ^ permalink raw reply [flat|nested] 4+ messages in thread
[parent not found: <CAPig_tmCBeNQXqc+8-Sn+5h-Gq_XT8VBHmhtfy48eoAgm75bCw@mail.gmail.com>]
* Re: [PATCH] BLACKFIN MEDIA DRIVER: rewrite the blackfin style of read/write into common style [not found] ` <CAPig_tmCBeNQXqc+8-Sn+5h-Gq_XT8VBHmhtfy48eoAgm75bCw@mail.gmail.com> @ 2015-01-19 9:45 ` Hans Verkuil 2015-01-20 9:42 ` Steven Miao 0 siblings, 1 reply; 4+ messages in thread From: Hans Verkuil @ 2015-01-19 9:45 UTC (permalink / raw) To: Hao Liang Cc: scott.jiang.linux, mchehab, linux-media, linux-kernel, Hao.Liang, adi-buildroot-devel, Steven Miao On 01/19/2015 04:13 AM, Hao Liang wrote: > Hi Hans, > > Thank you for your reply. > This change comes largely from a non-blackfin architecture dsp processor of ADI want to reuse this driver. > And i have tested common read/write function on blackfin board to ensure usability and stability. Well, looking at arch/blackfin/include/asm/def_LPBlackfin.h it seems that for certain blackfin variants the bfin_read/write functions insert an extra nop, so unless you test on one of those variants you wouldn't see any problems relating to this change (and possibly not even then if this is a rare race condition). You will have to check this with the blackfin architecture maintainer, Steven Miao (added to the CC list). Without the OK of someone who actually understands that architecture and the 'ANOMALY_05000198' issues I am not going to merge this. If the bfin_read/write functions are really needed for the blackfin architecture, then you can always make ppi_read/write functions that check the architecture and use bfin_read/write if it is a blackfin and readw/writew otherwise. That should be safe. Regards, Hans > > BR > Hao > > 2015-01-16 19:34 GMT+08:00 Hans Verkuil <hverkuil@xs4all.nl <mailto:hverkuil@xs4all.nl>>: > > Hi Hao, > > Why would you do this? read/writew() is AFAICT not the same as bfin_read/write16 > (defined in arch/blackfin/include/asm/def_LPBlackfin.h). And all other blackfin > sources I've seen all use bfin_read/write. > > So unless there is a good reason for this change I am not going to accept this. > > Regards, > > Hans > > On 01/14/2015 07:57 AM, Hao Liang wrote: > > Signed-off-by: Hao Liang <hliang1025@gmail.com <mailto:hliang1025@gmail.com>> > > --- > > drivers/media/platform/blackfin/ppi.c | 72 ++++++++++++++++----------------- > > 1 file changed, 35 insertions(+), 37 deletions(-) > > > > diff --git a/drivers/media/platform/blackfin/ppi.c b/drivers/media/platform/blackfin/ppi.c > > index cff63e5..de4b5c7 100644 > > --- a/drivers/media/platform/blackfin/ppi.c > > +++ b/drivers/media/platform/blackfin/ppi.c > > @@ -20,6 +20,7 @@ > > #include <linux/module.h> > > #include <linux/slab.h> > > #include <linux/platform_device.h> > > +#include <linux/io.h> > > > > #include <asm/bfin_ppi.h> > > #include <asm/blackfin.h> > > @@ -59,10 +60,10 @@ static irqreturn_t ppi_irq_err(int irq, void *dev_id) > > /* register on bf561 is cleared when read > > * others are W1C > > */ > > - status = bfin_read16(®->status); > > + status = readw(®->status); > > if (status & 0x3000) > > ppi->err = true; > > - bfin_write16(®->status, 0xff00); > > + writew(0xff00, ®->status); > > break; > > } > > case PPI_TYPE_EPPI: > > @@ -70,10 +71,10 @@ static irqreturn_t ppi_irq_err(int irq, void *dev_id) > > struct bfin_eppi_regs *reg = info->base; > > unsigned short status; > > > > - status = bfin_read16(®->status); > > + status = readw(®->status); > > if (status & 0x2) > > ppi->err = true; > > - bfin_write16(®->status, 0xffff); > > + writew(0xffff, ®->status); > > break; > > } > > case PPI_TYPE_EPPI3: > > @@ -81,10 +82,10 @@ static irqreturn_t ppi_irq_err(int irq, void *dev_id) > > struct bfin_eppi3_regs *reg = info->base; > > unsigned long stat; > > > > - stat = bfin_read32(®->stat); > > + stat = readl(®->stat); > > if (stat & 0x2) > > ppi->err = true; > > - bfin_write32(®->stat, 0xc0ff); > > + writel(0xc0ff, ®->stat); > > break; > > } > > default: > > @@ -139,26 +140,25 @@ static int ppi_start(struct ppi_if *ppi) > > case PPI_TYPE_PPI: > > { > > struct bfin_ppi_regs *reg = info->base; > > - bfin_write16(®->control, ppi->ppi_control); > > + writew(ppi->ppi_control, ®->control); > > break; > > } > > case PPI_TYPE_EPPI: > > { > > struct bfin_eppi_regs *reg = info->base; > > - bfin_write32(®->control, ppi->ppi_control); > > + writel(ppi->ppi_control, ®->control); > > break; > > } > > case PPI_TYPE_EPPI3: > > { > > struct bfin_eppi3_regs *reg = info->base; > > - bfin_write32(®->ctl, ppi->ppi_control); > > + writel(ppi->ppi_control, ®->ctl); > > break; > > } > > default: > > return -EINVAL; > > } > > > > - SSYNC(); > > return 0; > > } > > > > @@ -172,19 +172,19 @@ static int ppi_stop(struct ppi_if *ppi) > > case PPI_TYPE_PPI: > > { > > struct bfin_ppi_regs *reg = info->base; > > - bfin_write16(®->control, ppi->ppi_control); > > + writew(ppi->ppi_control, ®->control); > > break; > > } > > case PPI_TYPE_EPPI: > > { > > struct bfin_eppi_regs *reg = info->base; > > - bfin_write32(®->control, ppi->ppi_control); > > + writel(ppi->ppi_control, ®->control); > > break; > > } > > case PPI_TYPE_EPPI3: > > { > > struct bfin_eppi3_regs *reg = info->base; > > - bfin_write32(®->ctl, ppi->ppi_control); > > + writel(ppi->ppi_control, ®->ctl); > > break; > > } > > default: > > @@ -195,7 +195,6 @@ static int ppi_stop(struct ppi_if *ppi) > > clear_dma_irqstat(info->dma_ch); > > disable_dma(info->dma_ch); > > > > - SSYNC(); > > return 0; > > } > > > > @@ -242,9 +241,9 @@ static int ppi_set_params(struct ppi_if *ppi, struct ppi_params *params) > > if (params->ppi_control & DMA32) > > dma32 = 1; > > > > - bfin_write16(®->control, ppi->ppi_control); > > - bfin_write16(®->count, samples_per_line - 1); > > - bfin_write16(®->frame, params->frame); > > + writew(ppi->ppi_control, ®->control); > > + writew(samples_per_line - 1, ®->count); > > + writew(params->frame, ®->frame); > > break; > > } > > case PPI_TYPE_EPPI: > > @@ -255,13 +254,13 @@ static int ppi_set_params(struct ppi_if *ppi, struct ppi_params *params) > > || (params->ppi_control & 0x38000) > DLEN_16) > > dma32 = 1; > > > > - bfin_write32(®->control, ppi->ppi_control); > > - bfin_write16(®->line, samples_per_line); > > - bfin_write16(®->frame, params->frame); > > - bfin_write16(®->hdelay, hdelay); > > - bfin_write16(®->vdelay, params->vdelay); > > - bfin_write16(®->hcount, hcount); > > - bfin_write16(®->vcount, params->height); > > + writel(ppi->ppi_control, ®->control); > > + writew(samples_per_line, ®->line); > > + writew(params->frame, ®->frame); > > + writew(hdelay, ®->hdelay); > > + writew(params->vdelay, ®->vdelay); > > + writew(hcount, ®->hcount); > > + writew(params->height, ®->vcount); > > break; > > } > > case PPI_TYPE_EPPI3: > > @@ -272,15 +271,15 @@ static int ppi_set_params(struct ppi_if *ppi, struct ppi_params *params) > > || (params->ppi_control & 0x70000) > DLEN_16) > > dma32 = 1; > > > > - bfin_write32(®->ctl, ppi->ppi_control); > > - bfin_write32(®->line, samples_per_line); > > - bfin_write32(®->frame, params->frame); > > - bfin_write32(®->hdly, hdelay); > > - bfin_write32(®->vdly, params->vdelay); > > - bfin_write32(®->hcnt, hcount); > > - bfin_write32(®->vcnt, params->height); > > + writel(ppi->ppi_control, ®->ctl); > > + writel(samples_per_line, ®->line); > > + writel(params->frame, ®->frame); > > + writel(hdelay, ®->hdly); > > + writel(params->vdelay, ®->vdly); > > + writel(hcount, ®->hcnt); > > + writel(params->height, ®->vcnt); > > if (params->int_mask) > > - bfin_write32(®->imsk, params->int_mask & 0xFF); > > + writel(params->int_mask & 0xFF, ®->imsk); > > if (ppi->ppi_control & PORT_DIR) { > > u32 hsync_width, vsync_width, vsync_period; > > > > @@ -288,10 +287,10 @@ static int ppi_set_params(struct ppi_if *ppi, struct ppi_params *params) > > * params->bpp / params->dlen; > > vsync_width = params->vsync * samples_per_line; > > vsync_period = samples_per_line * params->frame; > > - bfin_write32(®->fs1_wlhb, hsync_width); > > - bfin_write32(®->fs1_paspl, samples_per_line); > > - bfin_write32(®->fs2_wlvb, vsync_width); > > - bfin_write32(®->fs2_palpf, vsync_period); > > + writel(hsync_width, ®->fs1_wlhb); > > + writel(samples_per_line, ®->fs1_paspl); > > + writel(vsync_width, ®->fs2_wlvb); > > + writel(vsync_period, ®->fs2_palpf); > > } > > break; > > } > > @@ -313,7 +312,6 @@ static int ppi_set_params(struct ppi_if *ppi, struct ppi_params *params) > > set_dma_y_count(info->dma_ch, params->height); > > set_dma_config(info->dma_ch, dma_config); > > > > - SSYNC(); > > return 0; > > } > > > > > > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] BLACKFIN MEDIA DRIVER: rewrite the blackfin style of read/write into common style 2015-01-19 9:45 ` Hans Verkuil @ 2015-01-20 9:42 ` Steven Miao 0 siblings, 0 replies; 4+ messages in thread From: Steven Miao @ 2015-01-20 9:42 UTC (permalink / raw) To: Hans Verkuil Cc: Hao Liang, scott.jiang.linux, mchehab, linux-media, open list:CAN NETWORK DRIVERS <linux-can@vger.kernel.org>, open list:NETWORKING DRIVERS <netdev@vger.kernel.org>, open list, Hao.Liang, bfin Hi Hans, On Mon, Jan 19, 2015 at 5:45 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote: > On 01/19/2015 04:13 AM, Hao Liang wrote: >> Hi Hans, >> >> Thank you for your reply. >> This change comes largely from a non-blackfin architecture dsp processor of ADI want to reuse this driver. >> And i have tested common read/write function on blackfin board to ensure usability and stability. > > Well, looking at arch/blackfin/include/asm/def_LPBlackfin.h it seems that for certain > blackfin variants the bfin_read/write functions insert an extra nop, so unless you > test on one of those variants you wouldn't see any problems relating to this change > (and possibly not even then if this is a rare race condition). > > You will have to check this with the blackfin architecture maintainer, Steven Miao > (added to the CC list). Without the OK of someone who actually understands that > architecture and the 'ANOMALY_05000198' issues I am not going to merge this. the ANOMALY_05000198 only exists in blackfin bf533 and bf561 old silicon revision, and this ppi driver mostly will be used on bf609 and later platform. > > If the bfin_read/write functions are really needed for the blackfin architecture, we will sync the __raw_read/write macros with bfin_read/write, and test it on most of the blackfin bf5xx and bf6xx platform. > then you can always make ppi_read/write functions that check the architecture and use > bfin_read/write if it is a blackfin and readw/writew otherwise. > > That should be safe. > > Regards, > > Hans > >> >> BR >> Hao >> >> 2015-01-16 19:34 GMT+08:00 Hans Verkuil <hverkuil@xs4all.nl <mailto:hverkuil@xs4all.nl>>: >> >> Hi Hao, >> >> Why would you do this? read/writew() is AFAICT not the same as bfin_read/write16 >> (defined in arch/blackfin/include/asm/def_LPBlackfin.h). And all other blackfin >> sources I've seen all use bfin_read/write. >> >> So unless there is a good reason for this change I am not going to accept this. >> >> Regards, >> >> Hans >> >> On 01/14/2015 07:57 AM, Hao Liang wrote: >> > Signed-off-by: Hao Liang <hliang1025@gmail.com <mailto:hliang1025@gmail.com>> >> > --- >> > drivers/media/platform/blackfin/ppi.c | 72 ++++++++++++++++----------------- >> > 1 file changed, 35 insertions(+), 37 deletions(-) >> > >> > diff --git a/drivers/media/platform/blackfin/ppi.c b/drivers/media/platform/blackfin/ppi.c >> > index cff63e5..de4b5c7 100644 >> > --- a/drivers/media/platform/blackfin/ppi.c >> > +++ b/drivers/media/platform/blackfin/ppi.c >> > @@ -20,6 +20,7 @@ >> > #include <linux/module.h> >> > #include <linux/slab.h> >> > #include <linux/platform_device.h> >> > +#include <linux/io.h> >> > >> > #include <asm/bfin_ppi.h> >> > #include <asm/blackfin.h> >> > @@ -59,10 +60,10 @@ static irqreturn_t ppi_irq_err(int irq, void *dev_id) >> > /* register on bf561 is cleared when read >> > * others are W1C >> > */ >> > - status = bfin_read16(®->status); >> > + status = readw(®->status); >> > if (status & 0x3000) >> > ppi->err = true; >> > - bfin_write16(®->status, 0xff00); >> > + writew(0xff00, ®->status); >> > break; >> > } >> > case PPI_TYPE_EPPI: >> > @@ -70,10 +71,10 @@ static irqreturn_t ppi_irq_err(int irq, void *dev_id) >> > struct bfin_eppi_regs *reg = info->base; >> > unsigned short status; >> > >> > - status = bfin_read16(®->status); >> > + status = readw(®->status); >> > if (status & 0x2) >> > ppi->err = true; >> > - bfin_write16(®->status, 0xffff); >> > + writew(0xffff, ®->status); >> > break; >> > } >> > case PPI_TYPE_EPPI3: >> > @@ -81,10 +82,10 @@ static irqreturn_t ppi_irq_err(int irq, void *dev_id) >> > struct bfin_eppi3_regs *reg = info->base; >> > unsigned long stat; >> > >> > - stat = bfin_read32(®->stat); >> > + stat = readl(®->stat); >> > if (stat & 0x2) >> > ppi->err = true; >> > - bfin_write32(®->stat, 0xc0ff); >> > + writel(0xc0ff, ®->stat); >> > break; >> > } >> > default: >> > @@ -139,26 +140,25 @@ static int ppi_start(struct ppi_if *ppi) >> > case PPI_TYPE_PPI: >> > { >> > struct bfin_ppi_regs *reg = info->base; >> > - bfin_write16(®->control, ppi->ppi_control); >> > + writew(ppi->ppi_control, ®->control); >> > break; >> > } >> > case PPI_TYPE_EPPI: >> > { >> > struct bfin_eppi_regs *reg = info->base; >> > - bfin_write32(®->control, ppi->ppi_control); >> > + writel(ppi->ppi_control, ®->control); >> > break; >> > } >> > case PPI_TYPE_EPPI3: >> > { >> > struct bfin_eppi3_regs *reg = info->base; >> > - bfin_write32(®->ctl, ppi->ppi_control); >> > + writel(ppi->ppi_control, ®->ctl); >> > break; >> > } >> > default: >> > return -EINVAL; >> > } >> > >> > - SSYNC(); >> > return 0; >> > } >> > >> > @@ -172,19 +172,19 @@ static int ppi_stop(struct ppi_if *ppi) >> > case PPI_TYPE_PPI: >> > { >> > struct bfin_ppi_regs *reg = info->base; >> > - bfin_write16(®->control, ppi->ppi_control); >> > + writew(ppi->ppi_control, ®->control); >> > break; >> > } >> > case PPI_TYPE_EPPI: >> > { >> > struct bfin_eppi_regs *reg = info->base; >> > - bfin_write32(®->control, ppi->ppi_control); >> > + writel(ppi->ppi_control, ®->control); >> > break; >> > } >> > case PPI_TYPE_EPPI3: >> > { >> > struct bfin_eppi3_regs *reg = info->base; >> > - bfin_write32(®->ctl, ppi->ppi_control); >> > + writel(ppi->ppi_control, ®->ctl); >> > break; >> > } >> > default: >> > @@ -195,7 +195,6 @@ static int ppi_stop(struct ppi_if *ppi) >> > clear_dma_irqstat(info->dma_ch); >> > disable_dma(info->dma_ch); >> > >> > - SSYNC(); >> > return 0; >> > } >> > >> > @@ -242,9 +241,9 @@ static int ppi_set_params(struct ppi_if *ppi, struct ppi_params *params) >> > if (params->ppi_control & DMA32) >> > dma32 = 1; >> > >> > - bfin_write16(®->control, ppi->ppi_control); >> > - bfin_write16(®->count, samples_per_line - 1); >> > - bfin_write16(®->frame, params->frame); >> > + writew(ppi->ppi_control, ®->control); >> > + writew(samples_per_line - 1, ®->count); >> > + writew(params->frame, ®->frame); >> > break; >> > } >> > case PPI_TYPE_EPPI: >> > @@ -255,13 +254,13 @@ static int ppi_set_params(struct ppi_if *ppi, struct ppi_params *params) >> > || (params->ppi_control & 0x38000) > DLEN_16) >> > dma32 = 1; >> > >> > - bfin_write32(®->control, ppi->ppi_control); >> > - bfin_write16(®->line, samples_per_line); >> > - bfin_write16(®->frame, params->frame); >> > - bfin_write16(®->hdelay, hdelay); >> > - bfin_write16(®->vdelay, params->vdelay); >> > - bfin_write16(®->hcount, hcount); >> > - bfin_write16(®->vcount, params->height); >> > + writel(ppi->ppi_control, ®->control); >> > + writew(samples_per_line, ®->line); >> > + writew(params->frame, ®->frame); >> > + writew(hdelay, ®->hdelay); >> > + writew(params->vdelay, ®->vdelay); >> > + writew(hcount, ®->hcount); >> > + writew(params->height, ®->vcount); >> > break; >> > } >> > case PPI_TYPE_EPPI3: >> > @@ -272,15 +271,15 @@ static int ppi_set_params(struct ppi_if *ppi, struct ppi_params *params) >> > || (params->ppi_control & 0x70000) > DLEN_16) >> > dma32 = 1; >> > >> > - bfin_write32(®->ctl, ppi->ppi_control); >> > - bfin_write32(®->line, samples_per_line); >> > - bfin_write32(®->frame, params->frame); >> > - bfin_write32(®->hdly, hdelay); >> > - bfin_write32(®->vdly, params->vdelay); >> > - bfin_write32(®->hcnt, hcount); >> > - bfin_write32(®->vcnt, params->height); >> > + writel(ppi->ppi_control, ®->ctl); >> > + writel(samples_per_line, ®->line); >> > + writel(params->frame, ®->frame); >> > + writel(hdelay, ®->hdly); >> > + writel(params->vdelay, ®->vdly); >> > + writel(hcount, ®->hcnt); >> > + writel(params->height, ®->vcnt); >> > if (params->int_mask) >> > - bfin_write32(®->imsk, params->int_mask & 0xFF); >> > + writel(params->int_mask & 0xFF, ®->imsk); >> > if (ppi->ppi_control & PORT_DIR) { >> > u32 hsync_width, vsync_width, vsync_period; >> > >> > @@ -288,10 +287,10 @@ static int ppi_set_params(struct ppi_if *ppi, struct ppi_params *params) >> > * params->bpp / params->dlen; >> > vsync_width = params->vsync * samples_per_line; >> > vsync_period = samples_per_line * params->frame; >> > - bfin_write32(®->fs1_wlhb, hsync_width); >> > - bfin_write32(®->fs1_paspl, samples_per_line); >> > - bfin_write32(®->fs2_wlvb, vsync_width); >> > - bfin_write32(®->fs2_palpf, vsync_period); >> > + writel(hsync_width, ®->fs1_wlhb); >> > + writel(samples_per_line, ®->fs1_paspl); >> > + writel(vsync_width, ®->fs2_wlvb); >> > + writel(vsync_period, ®->fs2_palpf); >> > } >> > break; >> > } >> > @@ -313,7 +312,6 @@ static int ppi_set_params(struct ppi_if *ppi, struct ppi_params *params) >> > set_dma_y_count(info->dma_ch, params->height); >> > set_dma_config(info->dma_ch, dma_config); >> > >> > - SSYNC(); >> > return 0; >> > } >> > >> > >> >> > -steven ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-01-20 9:42 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-01-14 6:57 [PATCH] BLACKFIN MEDIA DRIVER: rewrite the blackfin style of read/write into common style Hao Liang
2015-01-16 11:34 ` Hans Verkuil
[not found] ` <CAPig_tmCBeNQXqc+8-Sn+5h-Gq_XT8VBHmhtfy48eoAgm75bCw@mail.gmail.com>
2015-01-19 9:45 ` Hans Verkuil
2015-01-20 9:42 ` Steven Miao
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox