| 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: <20130729150552.GM29095@intel.com>
Date: Mon, 29 Jul 2013 20:35:52 +0530
From: Vinod Koul <vinod.koul@...el.com>
To: Sergei Shtylyov <sergei.shtylyov@...entembedded.com>
Cc: djbw@...com, linux-kernel@...r.kernel.org, linux-sh@...r.kernel.org
Subject: Re: [PATCH] dma: add driver for R-Car HPB-DMAC
On Sun, Jun 30, 2013 at 12:15:50AM +0400, Sergei Shtylyov wrote:
> +config RCAR_HPB_DMAE
> + tristate "Renesas R-Car HPB DMAC support"
> + depends on SH_DMAE_BASE
you should select DMA stuff here
> + help
> +#define DSAR0 0x00
> +#define DDAR0 0x04
> +#define DTCR0 0x08
> +#define DSAR1 0x0C
> +#define DDAR1 0x10
> +#define DTCR1 0x14
> +#define DSASR 0x18
> +#define DDASR 0x1C
> +#define DTCSR 0x20
> +#define DPTR 0x24
> +#define DCR 0x28
> +#define DCMDR 0x2C
> +#define DSTPR 0x30
> +#define DSTSR 0x34
> +#define DDBGR 0x38
> +#define DDBGR2 0x3C
> +#define HPB_CHAN(n) (0x40 * (n))
Pls namespace this aptly
> +
> +/* DMA command register (DCMDR) bits */
> +#define DCMDR_BDOUT BIT(7)
> +#define DCMDR_DQSPD BIT(6)
> +#define DCMDR_DQSPC BIT(5)
> +#define DCMDR_DMSPD BIT(4)
> +#define DCMDR_DMSPC BIT(3)
> +#define DCMDR_DQEND BIT(2)
> +#define DCMDR_DNXT BIT(1)
> +#define DCMDR_DMEN BIT(0)
ditto
> +
> +/* DMA forced stop register (DSTPR) bits */
> +#define DSTPR_DMSTP BIT(0)
> +
> +/* DMA status register (DSTSR) bits */
> +#define DSTSR_DMSTS BIT(0)
> +
> +/* DMA common registers */
> +#define DTIMR 0x00
> +#define DINTSR0 0x0C
> +#define DINTSR1 0x10
> +#define DINTCR0 0x14
> +#define DINTCR1 0x18
> +#define DINTMR0 0x1C
> +#define DINTMR1 0x20
> +#define DACTSR0 0x24
> +#define DACTSR1 0x28
> +#define HSRSTR(n) (0x40 + (n) * 4)
here too
> +static void ch_reg_write(struct hpb_dmae_chan *hpb_dc, u32 data, u32 reg)
> +{
> + __raw_writel(data, hpb_dc->base + reg);
> +}
> +
> +static u32 ch_reg_read(struct hpb_dmae_chan *hpb_dc, u32 reg)
> +{
> + return __raw_readl(hpb_dc->base + reg);
> +}
You dont need barrier?
> +static void dcmdr_write(struct hpb_dmae_device *hpbdev, u32 data)
> +{
> + __raw_writel(data, hpbdev->chan_reg + DCMDR);
> +}
> +
> +static void hsrstr_write(struct hpb_dmae_device *hpbdev, u32 ch)
> +{
> + __raw_writel(0x1, hpbdev->comm_reg + HSRSTR(ch));
> +}
why do you need reads for specfic registers? And why isn't this using above
helpers?
> +static u32 dintsr_read(struct hpb_dmae_device *hpbdev, u32 ch)
> +{
> + u32 v;
> +
> + if (ch < 32)
> + v = __raw_readl(hpbdev->comm_reg + DINTSR0) >> ch;
> + else
> + v = __raw_readl(hpbdev->comm_reg + DINTSR1) >> (ch - 32);
ditto
> + return v & 0x1;
> +}
> +
> +static void dintcr_write(struct hpb_dmae_device *hpbdev, u32 ch)
> +{
> + if (ch < 32)
> + __raw_writel((0x1 << ch), hpbdev->comm_reg + DINTCR0);
> + else
> + __raw_writel((0x1 << (ch - 32)), hpbdev->comm_reg + DINTCR1);
> +}
> +
> +static void asyncmdr_write(struct hpb_dmae_device *hpbdev, u32 data)
> +{
> + __raw_writel(data, hpbdev->mode_reg);
> +}
> +
> +static u32 asyncmdr_read(struct hpb_dmae_device *hpbdev)
> +{
> + return __raw_readl(hpbdev->mode_reg);
> +}
> +
> +static void hpb_dmae_enable_int(struct hpb_dmae_device *hpbdev, u32 ch)
> +{
> + u32 intreg;
> +
> + spin_lock_irq(&hpbdev->reg_lock);
> + if (ch < 32) {
> + intreg = __raw_readl(hpbdev->comm_reg + DINTMR0);
> + __raw_writel(BIT(ch) | intreg, hpbdev->comm_reg + DINTMR0);
> + } else {
> + intreg = __raw_readl(hpbdev->comm_reg + DINTMR1);
> + __raw_writel(BIT(ch - 32) | intreg, hpbdev->comm_reg + DINTMR1);
> + }
> + spin_unlock_irq(&hpbdev->reg_lock);
> +}
> +
> +static void hpb_dmae_async_reset(struct hpb_dmae_device *hpbdev, u32 data)
> +{
> + u32 rstr;
> + int timeout = 10000; /* 100 ms */
> +
> + spin_lock(&hpbdev->reg_lock);
> + rstr = __raw_readl(hpbdev->reset_reg);
> + rstr |= data;
> + __raw_writel(rstr, hpbdev->reset_reg);
> + do {
> + rstr = __raw_readl(hpbdev->reset_reg);
> + if ((rstr & data) == data)
> + break;
> + udelay(10);
> + } while (timeout--);
> +
> + if (timeout < 0)
<= perhaps
> + dev_err(hpbdev->shdma_dev.dma_dev.dev,
> + "%s timeout\n", __func__);
> +
> + rstr &= ~data;
> + __raw_writel(rstr, hpbdev->reset_reg);
no barriers?
> + spin_unlock(&hpbdev->reg_lock);
> +}
> +
> +static void hpb_dmae_set_async_mode(struct hpb_dmae_device *hpbdev,
> + u32 mask, u32 data)
> +{
> + u32 mode;
> +
> + spin_lock_irq(&hpbdev->reg_lock);
> + mode = asyncmdr_read(hpbdev);
> + mode &= ~mask;
> + mode |= data;
> + asyncmdr_write(hpbdev, mode);
> + spin_unlock_irq(&hpbdev->reg_lock);
> +}
> +
> +static void hpb_dmae_ctl_stop(struct hpb_dmae_device *hpbdev)
> +{
> + dcmdr_write(hpbdev, DCMDR_DQSPD);
> +}
> +
> +static void hpb_dmae_reset(struct hpb_dmae_device *hpbdev)
> +{
> + u32 ch;
> +
> + for (ch = 0; ch < hpbdev->pdata->num_hw_channels; ch++)
> + hsrstr_write(hpbdev, ch);
> +}
> +
> +static unsigned int calc_xmit_shift(struct hpb_dmae_chan *hpb_chan)
> +{
> + struct hpb_dmae_device *hpbdev = to_dev(hpb_chan);
> + struct hpb_dmae_pdata *pdata = hpbdev->pdata;
> + int width = ch_reg_read(hpb_chan, DCR);
> + int i;
> +
> + switch (width & (DCR_SPDS_MASK | DCR_DPDS_MASK)) {
> + case DCR_SPDS_8BIT | DCR_DPDS_8BIT:
> + default:
> + i = XMIT_SZ_8BIT;
> + break;
this is unconventional, typically default is supposed to be last and is more
readble IMO
> +static bool hpb_dmae_desc_completed(struct shdma_chan *schan,
> + struct shdma_desc *sdesc)
> +{
> + return true;
> +}
always?
> +/* DMA control register (DCR) bits */
> +#define DCR_DTAMD (1u << 26)
> +#define DCR_DTAC (1u << 25)
> +#define DCR_DTAU (1u << 24)
> +#define DCR_DTAU1 (1u << 23)
> +#define DCR_SWMD (1u << 22)
> +#define DCR_BTMD (1u << 21)
> +#define DCR_PKMD (1u << 20)
> +#define DCR_CT (1u << 18)
> +#define DCR_ACMD (1u << 17)
> +#define DCR_DIP (1u << 16)
> +#define DCR_SMDL (1u << 13)
> +#define DCR_SPDAM (1u << 12)
> +#define DCR_SDRMD_MASK (3u << 10)
> +#define DCR_SDRMD_MOD (0u << 10)
> +#define DCR_SDRMD_AUTO (1u << 10)
> +#define DCR_SDRMD_TIMER (2u << 10)
> +#define DCR_SPDS_MASK (3u << 8)
> +#define DCR_SPDS_8BIT (0u << 8)
> +#define DCR_SPDS_16BIT (1u << 8)
> +#define DCR_SPDS_32BIT (2u << 8)
> +#define DCR_DMDL (1u << 5)
> +#define DCR_DPDAM (1u << 4)
> +#define DCR_DDRMD_MASK (3u << 2)
> +#define DCR_DDRMD_MOD (0u << 2)
> +#define DCR_DDRMD_AUTO (1u << 2)
> +#define DCR_DDRMD_TIMER (2u << 2)
> +#define DCR_DPDS_MASK (3u << 0)
> +#define DCR_DPDS_8BIT (0u << 0)
> +#define DCR_DPDS_16BIT (1u << 0)
> +#define DCR_DPDS_32BIT (2u << 0)
again need namespace..
> +
> +/* Asynchronous reset register (ASYNCRSTR) bits */
> +#define ASYNCRSTR_ASRST41 BIT(10)
> +#define ASYNCRSTR_ASRST40 BIT(9)
> +#define ASYNCRSTR_ASRST39 BIT(8)
> +#define ASYNCRSTR_ASRST27 BIT(7)
> +#define ASYNCRSTR_ASRST26 BIT(6)
> +#define ASYNCRSTR_ASRST25 BIT(5)
> +#define ASYNCRSTR_ASRST24 BIT(4)
> +#define ASYNCRSTR_ASRST23 BIT(3)
> +#define ASYNCRSTR_ASRST22 BIT(2)
> +#define ASYNCRSTR_ASRST21 BIT(1)
> +#define ASYNCRSTR_ASRST20 BIT(0)
ditto...
~Vinod
--
--
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