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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHp75VeCHMmYsgrb5GThCfsG_TekZjRfy+MsveAk01QDaUR8gQ@mail.gmail.com>
Date:	Tue, 26 May 2015 09:49:57 +0300
From:	Andy Shevchenko <andy.shevchenko@...il.com>
To:	Vinod Koul <vinod.koul@...el.com>
Cc:	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Lee Jones <lee.jones@...aro.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Mika Westerberg <mika.westerberg@...ux.intel.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	dmaengine <dmaengine@...r.kernel.org>,
	Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
	Jarkko Nikula <jarkko.nikula@...ux.intel.com>
Subject: Re: [PATCH v2 7/8] dmaengine: add a driver for Intel integrated DMA 64-bit

On Tue, May 26, 2015 at 7:06 AM, Vinod Koul <vinod.koul@...el.com> wrote:
> On Mon, May 25, 2015 at 07:09:31PM +0300, Andy Shevchenko wrote:
>> Intel integrated DMA (iDMA) 64-bit is a specific IP that is used as a part of
>> LPSS devices such as HSUART or SPI. The iDMA IP is attached for private
>> usage on each host controller independently.
>>
>> While it has similarities with Synopsys DesignWare DMA, the following
>> distinctions doesn't allow to use the existing driver:
>> - 64-bit mode with corresponding changes in Hardware Linked List data structure
>> - many slight differences in the channel registers
>>
>> Moreover this driver is based on the DMA virtual channels framework that helps
>> to make the driver cleaner and easy to understand.
>>
> Looking at code and iDMA controllers (if this is the same as I have used), we
> have register compatibility with DW controller, so why new driver and why not
> use and enhance dw driver ?

Take a look closer. There are many, like I mentioned, slight but not
least changes in the registers, besides *64-bit mode*:
- ctl_hi represents bytes, not items
- 2 bytes of burst is supported (dw has no gap there)
- shuffling bits between ctl_* and cfg_*
- new bits with different meaning in ctl_* and cfg_*.

Preliminary we did a patchset for dw_dmac, but above hw changes blows
up and messes the driver code. I really would prefer to have those two
separate.

However, the 32-bit iDMA which is used in Baytrail might be driven by dw_dmac.

>> @@ -0,0 +1,233 @@
>> +/*
>> + * Driver for the Intel integrated DMA 64-bit
>> + *
>> + * Copyright (C) 2015 Intel Corporation
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#ifndef __DMA_IDMA64_H__
>> +#define __DMA_IDMA64_H__
>> +
>> +#include <linux/device.h>
>> +#include <linux/io.h>
>> +#include <linux/spinlock.h>
>> +#include <linux/types.h>
>> +
>> +#include "virt-dma.h"
>> +
>> +/* Channel registers */
>> +
>> +#define IDMA64_CH_SAR                0x00    /* Source Address Register */
>> +#define IDMA64_CH_DAR                0x08    /* Destination Address Register */
>> +#define IDMA64_CH_LLP                0x10    /* Linked List Pointer */
>> +#define IDMA64_CH_CTL_LO     0x18    /* Control Register Low */
>> +#define IDMA64_CH_CTL_HI     0x1c    /* Control Register High */
>> +#define IDMA64_CH_SSTAT              0x20
>> +#define IDMA64_CH_DSTAT              0x28
>> +#define IDMA64_CH_SSTATAR    0x30
>> +#define IDMA64_CH_DSTATAR    0x38
>> +#define IDMA64_CH_CFG_LO     0x40    /* Configuration Register Low */
>> +#define IDMA64_CH_CFG_HI     0x44    /* Configuration Register High */
>> +#define IDMA64_CH_SGR                0x48
>> +#define IDMA64_CH_DSR                0x50
>> +
>> +#define IDMA64_CH_LENGTH     0x58
>> +
>> +/* Bitfields in CTL_LO */
>> +#define IDMA64C_CTLL_INT_EN          (1 << 0)        /* irqs enabled? */
>> +#define IDMA64C_CTLL_DST_WIDTH(x)    ((x) << 1)      /* bytes per element */
>> +#define IDMA64C_CTLL_SRC_WIDTH(x)    ((x) << 4)
>> +#define IDMA64C_CTLL_DST_INC         (0 << 8)        /* DAR update/not */
>> +#define IDMA64C_CTLL_DST_FIX         (1 << 8)
>> +#define IDMA64C_CTLL_SRC_INC         (0 << 10)       /* SAR update/not */
>> +#define IDMA64C_CTLL_SRC_FIX         (1 << 10)
>> +#define IDMA64C_CTLL_DST_MSIZE(x)    ((x) << 11)     /* burst, #elements */
>> +#define IDMA64C_CTLL_SRC_MSIZE(x)    ((x) << 14)
>> +#define IDMA64C_CTLL_FC_M2P          (1 << 20)       /* mem-to-periph */
>> +#define IDMA64C_CTLL_FC_P2M          (2 << 20)       /* periph-to-mem */
>> +#define IDMA64C_CTLL_LLP_D_EN                (1 << 27)       /* dest block chain */
>> +#define IDMA64C_CTLL_LLP_S_EN                (1 << 28)       /* src block chain */
>> +
>> +/* Bitfields in CTL_HI */
>> +#define IDMA64C_CTLH_BLOCK_TS(x)     ((x) & ((1 << 17) - 1))
>> +#define IDMA64C_CTLH_DONE            (1 << 17)
>> +
>> +/* Bitfields in CFG_LO */
>> +#define IDMA64C_CFGL_DST_BURST_ALIGN (1 << 0)        /* dst burst align */
>> +#define IDMA64C_CFGL_SRC_BURST_ALIGN (1 << 1)        /* src burst align */
>> +#define IDMA64C_CFGL_CH_SUSP         (1 << 8)
>> +#define IDMA64C_CFGL_FIFO_EMPTY              (1 << 9)
>> +#define IDMA64C_CFGL_CH_DRAIN                (1 << 10)       /* drain FIFO */
>> +#define IDMA64C_CFGL_DST_OPT_BL              (1 << 20)       /* optimize dst burst length */
>> +#define IDMA64C_CFGL_SRC_OPT_BL              (1 << 21)       /* optimize src burst length */
>> +
>> +/* Bitfields in CFG_HI */
>> +#define IDMA64C_CFGH_SRC_PER(x)              ((x) << 0)      /* src peripheral */
>> +#define IDMA64C_CFGH_DST_PER(x)              ((x) << 4)      /* dst peripheral */
>> +#define IDMA64C_CFGH_RD_ISSUE_THD(x) ((x) << 8)
>> +#define IDMA64C_CFGH_RW_ISSUE_THD(x) ((x) << 18)
>> +
>> +/* Interrupt registers */
>> +
>> +#define IDMA64_INT_XFER              0x00
>> +#define IDMA64_INT_BLOCK     0x08
>> +#define IDMA64_INT_SRC_TRAN  0x10
>> +#define IDMA64_INT_DST_TRAN  0x18
>> +#define IDMA64_INT_ERROR     0x20
>> +
>> +#define IDMA64_RAW(x)                (0x2c0 + IDMA64_INT_##x)        /* r */
>> +#define IDMA64_STATUS(x)     (0x2e8 + IDMA64_INT_##x)        /* r (raw & mask) */
>> +#define IDMA64_MASK(x)               (0x310 + IDMA64_INT_##x)        /* rw (set = irq enabled) */
>> +#define IDMA64_CLEAR(x)              (0x338 + IDMA64_INT_##x)        /* w (ack, affects "raw") */
>> +
>> +/* Common registers */
>> +
>> +#define IDMA64_STATUS_INT    0x360   /* r */
>> +#define IDMA64_CFG           0x398
>> +#define IDMA64_CH_EN         0x3a0
>> +
>> +/* Bitfields in CFG */
>> +#define IDMA64_CFG_DMA_EN            (1 << 0)
>> +
>> +/* Hardware descriptor for Linked LIst transfers */
>> +struct idma64_lli {
>> +     u64             sar;
>> +     u64             dar;
>> +     u64             llp;
>> +     u32             ctllo;
>> +     u32             ctlhi;
>> +     u32             sstat;
>> +     u32             dstat;
>> +};
>> +
>> +struct idma64_hw_desc {
>> +     struct idma64_lli *lli;
>> +     dma_addr_t llp;
>> +     dma_addr_t phys;
>> +     unsigned int len;
>> +};
>> +
>> +struct idma64_desc {
>> +     struct virt_dma_desc vdesc;
>> +     enum dma_transfer_direction direction;
>> +     struct idma64_hw_desc *hw;
>> +     unsigned int ndesc;
>> +     enum dma_status status;
>> +};
>> +
>> +static inline struct idma64_desc *to_idma64_desc(struct virt_dma_desc *vdesc)
>> +{
>> +     return container_of(vdesc, struct idma64_desc, vdesc);
>> +}
>> +
>> +struct idma64_chan {
>> +     struct virt_dma_chan vchan;
>> +
>> +     void __iomem *regs;
>> +     spinlock_t lock;
>> +
>> +     /* hardware configuration */
>> +     enum dma_transfer_direction direction;
>> +     unsigned int mask;
>> +     struct dma_slave_config config;
>> +
>> +     void *pool;
>> +     struct idma64_desc *desc;
>> +};
>> +
>> +static inline struct idma64_chan *to_idma64_chan(struct dma_chan *chan)
>> +{
>> +     return container_of(chan, struct idma64_chan, vchan.chan);
>> +}
>> +
>> +#define channel_set_bit(idma64, reg, mask)   \
>> +     dma_writel(idma64, reg, ((mask) << 8) | (mask))
>> +#define channel_clear_bit(idma64, reg, mask) \
>> +     dma_writel(idma64, reg, ((mask) << 8) | 0)
>> +
>> +static inline u32 idma64c_readl(struct idma64_chan *idma64c, int offset)
>> +{
>> +     return readl(idma64c->regs + offset);
>> +}
>> +
>> +static inline void idma64c_writel(struct idma64_chan *idma64c, int offset,
>> +                               u32 value)
>> +{
>> +     writel(value, idma64c->regs + offset);
>> +}
>> +
>> +#define channel_readl(idma64c, reg)          \
>> +     idma64c_readl(idma64c, IDMA64_CH_##reg)
>> +#define channel_writel(idma64c, reg, value)  \
>> +     idma64c_writel(idma64c, IDMA64_CH_##reg, (value))
>> +
>> +static inline u64 idma64c_readq(struct idma64_chan *idma64c, int offset)
>> +{
>> +     u64 l, h;
>> +
>> +     l = idma64c_readl(idma64c, offset);
>> +     h = idma64c_readl(idma64c, offset + 4);
>> +
>> +     return l | (h << 32);
>> +}
>> +
>> +static inline void idma64c_writeq(struct idma64_chan *idma64c, int offset,
>> +                               u64 value)
>> +{
>> +     idma64c_writel(idma64c, offset, value);
>> +     idma64c_writel(idma64c, offset + 4, value >> 32);
>> +}
>> +
>> +#define channel_readq(idma64c, reg)          \
>> +     idma64c_readq(idma64c, IDMA64_CH_##reg)
>> +#define channel_writeq(idma64c, reg, value)  \
>> +     idma64c_writeq(idma64c, IDMA64_CH_##reg, (value))
>> +
>> +struct idma64 {
>> +     struct dma_device dma;
>> +
>> +     void __iomem *regs;
>> +
>> +     /* channels */
>> +     unsigned short all_chan_mask;
>> +     struct idma64_chan *chan;
>> +};
>> +
>> +static inline struct idma64 *to_idma64(struct dma_device *ddev)
>> +{
>> +     return container_of(ddev, struct idma64, dma);
>> +}
>> +
>> +static inline u32 idma64_readl(struct idma64 *idma64, int offset)
>> +{
>> +     return readl(idma64->regs + offset);
>> +}
>> +
>> +static inline void idma64_writel(struct idma64 *idma64, int offset, u32 value)
>> +{
>> +     writel(value, idma64->regs + offset);
>> +}
>> +
>> +#define dma_readl(idma64, reg)                       \
>> +     idma64_readl(idma64, IDMA64_##reg)
>> +#define dma_writel(idma64, reg, value)               \
>> +     idma64_writel(idma64, IDMA64_##reg, (value))
>> +
>> +/**
>> + * struct idma64_chip - representation of DesignWare DMA controller hardware
>> + * @dev:             struct device of the DMA controller
>> + * @irq:             irq line
>> + * @regs:            memory mapped I/O space
>> + * @idma64:          struct idma64 that is filed by idma64_probe()
>> + */
>> +struct idma64_chip {
>> +     struct device   *dev;
>> +     int             irq;
>> +     void __iomem    *regs;
>> +     struct idma64   *idma64;
>> +};
>> +
>> +#endif /* __DMA_IDMA64_H__ */
>> --
>> 2.1.4
>>
>
> --
> --
> 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/



-- 
With Best Regards,
Andy Shevchenko
--
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

Powered by Openwall GNU/*/Linux Powered by OpenVZ