| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <54BCD238.3010904@xs4all.nl>
Date: Mon, 19 Jan 2015 10:45:28 +0100
From: Hans Verkuil <hverkuil@...all.nl>
To: Hao Liang <hliang1025@...il.com>
CC: scott.jiang.linux@...il.com, mchehab@....samsung.com,
linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
Hao.Liang@...log.com, adi-buildroot-devel@...ts.sourceforge.net,
Steven Miao <realmz6@...il.com>
Subject: Re: [PATCH] BLACKFIN MEDIA DRIVER: rewrite the blackfin style of
read/write into common style
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@...all.nl <mailto:hverkuil@...all.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@...il.com <mailto:hliang1025@...il.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;
> > }
> >
> >
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists