[U-Boot] [PATCH v5 1/5] dma: lpc32xx: add DMA driver
From: Sylvain Lemieux slemieux@tycoint.com
Incorporate DMA driver from legacy LPCLinux NXP BSP. The files taken from the legacy patch are: - lpc32xx DMA driver - lpc3250 header file DMA registers definition.
The legacy driver was updated and clean-up as part of the integration with the latest u-boot.
Signed-off-by: Sylvain Lemieux slemieux@tycoint.com --- Changes from v4 to v5: * Addressed Marek's comments on LPC32xx DMA driver: - Move macro use by the DMA client to write the next DMA linked list item address to NAND SLC driver. - updated "lpc32xx_dma_wait_status()" implementation to make if stetement easier to read. - updated multiline comments style.
Changes from v3 to v4: * Addressed Marek's comments on LPC32xx DMA driver: - updated multiline comments style. - use "u32" instead of "uint32_t". - fixed unbounded loop.
Changes from v2 to v3: * Addressed Marek's comments on LPC32xx DMA driver: - use defined in header file instead of local definition. - remove for loop and use "ffz()". * Provide define to assign next DMA linked list item address to removed typecast in DMA client (ex. NAND SLC).
Changes from v1 to v2: * Moved the DMA patch as the first patch of the series. * The NAND SLC patch (link below) is applied before this patch: https://patchwork.ozlabs.org/patch/497308/
Update to the legacy driver to integrate with the latest u-boot: 1) Fixed checkpatch script output in legacy code. 2) Use LPC32xx definition from "cpu.h" and "clk.h". 3) Incorporate DMA specific register definition from "lpc3250.h" header file from legacy BSP patch from LPCLinux. 4) Use u-boot API for register access to remove the volatile in register definition taken from "lpc3250.h" header file. 5) Add DMA interface to "dma.h". 6) Add dma clock control register bits (clk.h). 7) Add functions to initialize the DMA clock.
The legacy BSP patch (u-boot-2009.03_lpc32x0-v1.07.patch.tar.bz2) was downloaded from the LPCLinux Web site.
arch/arm/cpu/arm926ejs/lpc32xx/devices.c | 6 + arch/arm/include/asm/arch-lpc32xx/clk.h | 3 + arch/arm/include/asm/arch-lpc32xx/dma.h | 37 ++++++ arch/arm/include/asm/arch-lpc32xx/sys_proto.h | 1 + drivers/dma/Makefile | 1 + drivers/dma/lpc32xx_dma.c | 163 ++++++++++++++++++++++++++ 6 files changed, 211 insertions(+) create mode 100644 arch/arm/include/asm/arch-lpc32xx/dma.h create mode 100644 drivers/dma/lpc32xx_dma.c
diff --git a/arch/arm/cpu/arm926ejs/lpc32xx/devices.c b/arch/arm/cpu/arm926ejs/lpc32xx/devices.c index c0c9c6c..0d2ef7a 100644 --- a/arch/arm/cpu/arm926ejs/lpc32xx/devices.c +++ b/arch/arm/cpu/arm926ejs/lpc32xx/devices.c @@ -65,6 +65,12 @@ void lpc32xx_slc_nand_init(void) writel(CLK_NAND_SLC | CLK_NAND_SLC_SELECT, &clk->flashclk_ctrl); }
+void lpc32xx_dma_init(void) +{ + /* Enable DMA interface */ + setbits_le32(&clk->dmaclk_ctrl, DMA_CLK_ENABLE); +} + void lpc32xx_i2c_init(unsigned int devnum) { /* Enable I2C interface */ diff --git a/arch/arm/include/asm/arch-lpc32xx/clk.h b/arch/arm/include/asm/arch-lpc32xx/clk.h index 010211a..663f6bc 100644 --- a/arch/arm/include/asm/arch-lpc32xx/clk.h +++ b/arch/arm/include/asm/arch-lpc32xx/clk.h @@ -158,6 +158,9 @@ struct clk_pm_regs { #define CLK_NAND_SLC_SELECT (1 << 2) #define CLK_NAND_MLC_INT (1 << 5)
+/* DMA Clock Control Register bits */ +#define DMA_CLK_ENABLE (1 << 0) + /* SSP Clock Control Register bits */ #define CLK_SSP0_ENABLE_CLOCK (1 << 0)
diff --git a/arch/arm/include/asm/arch-lpc32xx/dma.h b/arch/arm/include/asm/arch-lpc32xx/dma.h new file mode 100644 index 0000000..15d829c --- /dev/null +++ b/arch/arm/include/asm/arch-lpc32xx/dma.h @@ -0,0 +1,37 @@ +/* + * LPC32xx DMA Controller Interface + * + * Copyright (C) 2008-2015 by NXP Semiconductors + * All rights reserved. + * + * @Author: Kevin Wells + * @Descr: Definitions for LPC3250 chip + * @References: NXP LPC3250 User's Guide + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#ifndef _LPC32XX_DMA_H +#define _LPC32XX_DMA_H + +#include <common.h> + +/* + * DMA linked list structure used with a channel's LLI register; + * refer to UM10326, "LPC32x0 and LPC32x0/01 User manual" - Rev. 3 + * tables 84, 85, 86 & 87 for details. + */ +struct lpc32xx_dmac_ll { + u32 dma_src; + u32 dma_dest; + u32 next_lli; + u32 next_ctrl; +}; + +int lpc32xx_dma_get_channel(void); +int lpc32xx_dma_start_xfer(int channel, const struct lpc32xx_dmac_ll *desc, + u32 config); +int lpc32xx_dma_wait_status(int channel); +void lpc32xx_dma_put_channel(int channel); + +#endif /* _LPC32XX_DMA_H */ diff --git a/arch/arm/include/asm/arch-lpc32xx/sys_proto.h b/arch/arm/include/asm/arch-lpc32xx/sys_proto.h index 0845f83..7f997d9 100644 --- a/arch/arm/include/asm/arch-lpc32xx/sys_proto.h +++ b/arch/arm/include/asm/arch-lpc32xx/sys_proto.h @@ -13,6 +13,7 @@ void lpc32xx_uart_init(unsigned int uart_id); void lpc32xx_mac_init(void); void lpc32xx_mlc_nand_init(void); void lpc32xx_slc_nand_init(void); +void lpc32xx_dma_init(void); void lpc32xx_i2c_init(unsigned int devnum); void lpc32xx_ssp_init(void); #if defined(CONFIG_SPL_BUILD) diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile index 4c8fcc2..f95fe70 100644 --- a/drivers/dma/Makefile +++ b/drivers/dma/Makefile @@ -10,3 +10,4 @@ obj-$(CONFIG_APBH_DMA) += apbh_dma.o obj-$(CONFIG_FSL_DMA) += fsl_dma.o obj-$(CONFIG_TI_KSNAV) += keystone_nav.o keystone_nav_cfg.o obj-$(CONFIG_TI_EDMA3) += ti-edma3.o +obj-$(CONFIG_DMA_LPC32XX) += lpc32xx_dma.o diff --git a/drivers/dma/lpc32xx_dma.c b/drivers/dma/lpc32xx_dma.c new file mode 100644 index 0000000..0452013 --- /dev/null +++ b/drivers/dma/lpc32xx_dma.c @@ -0,0 +1,163 @@ +/* + * Copyright (C) 2008-2015 by NXP Semiconductors + * All rights reserved. + * + * @Author: Kevin Wells + * @Descr: LPC3250 DMA controller interface support functions + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <asm/arch/dma.h> +#include <asm/arch/cpu.h> +#include <asm/arch/clk.h> +#include <asm/arch/sys_proto.h> +#include <asm/io.h> + +/* + * Notes: + * + * Only bitfield masks/values which are actually used by the driver + * are defined. + */ + +/* DMA controller channel register structure */ +struct dmac_chan_reg { + u32 src_addr; + u32 dest_addr; + u32 lli; + u32 control; + u32 config_ch; + u32 reserved[3]; +}; + +/* DMA controller register structures */ +struct dma_reg { + u32 int_stat; + u32 int_tc_stat; + u32 int_tc_clear; + u32 int_err_stat; + u32 int_err_clear; + u32 raw_tc_stat; + u32 raw_err_stat; + u32 chan_enable; + u32 sw_burst_req; + u32 sw_single_req; + u32 sw_last_burst_req; + u32 sw_last_single_req; + u32 config; + u32 sync; + u32 reserved[50]; + struct dmac_chan_reg dma_chan[8]; +}; + +/* Macro pointing to DMA registers */ +#define DMA_NO_OF_CHANNELS 8 + +/* config register definitions */ +#define DMAC_CTRL_ENABLE (1 << 0) /* For enabling the DMA controller */ + +static u32 alloc_ch; + +static struct dma_reg *dma = (struct dma_reg *)DMA_BASE; + +int lpc32xx_dma_get_channel(void) +{ + int i; + + if (!alloc_ch) { /* First time caller */ + /* + * DMA clock are enable by "lpc32xx_dma_init()" and should + * be call by board "board_early_init_f()" function. + */ + + /* + * Make sure DMA controller and all channels are disabled. + * Controller is in little-endian mode. Disable sync signals. + */ + writel(0, &dma->config); + writel(0, &dma->sync); + + /* Clear interrupt and error statuses */ + writel(0xFF, &dma->int_tc_clear); + writel(0xFF, &dma->raw_tc_stat); + writel(0xFF, &dma->int_err_clear); + writel(0xFF, &dma->raw_err_stat); + + /* Enable DMA controller */ + writel(DMAC_CTRL_ENABLE, &dma->config); + } + + i = ffz(alloc_ch); + + /* Check if all the available channels are busy */ + if (unlikely(i == DMA_NO_OF_CHANNELS)) + return -1; + alloc_ch |= BIT_MASK(i); + return i; +} + +int lpc32xx_dma_start_xfer(int channel, const struct lpc32xx_dmac_ll *desc, + u32 config) +{ + if (unlikely((BIT_MASK(channel) & alloc_ch) == 0)) { + printf("ERR: Request for xfer on unallocated channel %d\r\n", + channel); + BUG(); + } + writel(BIT_MASK(channel), &dma->int_tc_clear); + writel(BIT_MASK(channel), &dma->int_err_clear); + writel(desc->dma_src, &dma->dma_chan[channel].src_addr); + writel(desc->dma_dest, &dma->dma_chan[channel].dest_addr); + writel(desc->next_lli, &dma->dma_chan[channel].lli); + writel(desc->next_ctrl, &dma->dma_chan[channel].control); + writel(config, &dma->dma_chan[channel].config_ch); + + return 0; +} + +int lpc32xx_dma_wait_status(int channel) +{ + unsigned long start; + u32 reg; + + start = get_timer(0); + while (1) { + reg = readl(&dma->raw_tc_stat); + reg |= readl(dma->raw_err_stat); + if (reg & BIT_MASK(channel)) + break; + + if (get_timer(start) > 10000) + return -1; + + udelay(1); + } + + if (unlikely(readl(&dma->raw_err_stat) & BIT_MASK(channel))) { + setbits_le32(&dma->int_err_clear, BIT_MASK(channel)); + setbits_le32(&dma->raw_err_stat, BIT_MASK(channel)); + return -1; + } + setbits_le32(&dma->int_tc_clear, BIT_MASK(channel)); + setbits_le32(&dma->raw_tc_stat, BIT_MASK(channel)); + return 0; +} + +void lpc32xx_dma_put_channel(int channel) +{ + /* Check if given channel no is valid */ + if (channel >= DMA_NO_OF_CHANNELS || channel < 0) + return; + alloc_ch &= ~BIT_MASK(channel); + + /* Shut down channel */ + writel(0, &dma->dma_chan[channel].control); + writel(0, &dma->dma_chan[channel].config_ch); + clrbits_le32(&dma->sync, BIT_MASK(channel)); + + if (!alloc_ch) + /* Disable DMA controller */ + clrbits_le32(&dma->config, DMAC_CTRL_ENABLE); +}
On Wednesday, August 05, 2015 at 08:31:54 PM, slemieux.tyco@gmail.com wrote:
From: Sylvain Lemieux slemieux@tycoint.com
Incorporate DMA driver from legacy LPCLinux NXP BSP. The files taken from the legacy patch are:
- lpc32xx DMA driver
- lpc3250 header file DMA registers definition.
The legacy driver was updated and clean-up as part of the integration with the latest u-boot.
Signed-off-by: Sylvain Lemieux slemieux@tycoint.com
[...]
+int lpc32xx_dma_wait_status(int channel) +{
- unsigned long start;
- u32 reg;
- start = get_timer(0);
- while (1) {
reg = readl(&dma->raw_tc_stat);reg |= readl(dma->raw_err_stat);if (reg & BIT_MASK(channel))break;if (get_timer(start) > 10000)return -1;
return -ETIMEDOUT;
udelay(1);- }
- if (unlikely(readl(&dma->raw_err_stat) & BIT_MASK(channel))) {
setbits_le32(&dma->int_err_clear, BIT_MASK(channel));setbits_le32(&dma->raw_err_stat, BIT_MASK(channel));return -1;- }
- setbits_le32(&dma->int_tc_clear, BIT_MASK(channel));
- setbits_le32(&dma->raw_tc_stat, BIT_MASK(channel));
- return 0;
+}
Acked-by: Marek Vasut marex@denx.de
Best regards, Marek Vasut
Hi Sylvain,
On 05.08.2015 21:31, slemieux.tyco@gmail.com wrote:
From: Sylvain Lemieux slemieux@tycoint.com
Incorporate DMA driver from legacy LPCLinux NXP BSP. The files taken from the legacy patch are:
- lpc32xx DMA driver
- lpc3250 header file DMA registers definition.
The legacy driver was updated and clean-up as part of the integration with the latest u-boot.
Signed-off-by: Sylvain Lemieux slemieux@tycoint.com
Changes from v4 to v5:
- Addressed Marek's comments on LPC32xx DMA driver:
- Move macro use by the DMA client to write the next DMA linked list item address to NAND SLC driver.
- updated "lpc32xx_dma_wait_status()" implementation to make if stetement easier to read.
- updated multiline comments style.
Changes from v3 to v4:
- Addressed Marek's comments on LPC32xx DMA driver:
- updated multiline comments style.
- use "u32" instead of "uint32_t".
- fixed unbounded loop.
Changes from v2 to v3:
- Addressed Marek's comments on LPC32xx DMA driver:
- use defined in header file instead of local definition.
- remove for loop and use "ffz()".
- Provide define to assign next DMA linked list item address to removed typecast in DMA client (ex. NAND SLC).
Changes from v1 to v2:
- Moved the DMA patch as the first patch of the series.
- The NAND SLC patch (link below) is applied before this patch: https://patchwork.ozlabs.org/patch/497308/
Update to the legacy driver to integrate with the latest u-boot:
- Fixed checkpatch script output in legacy code.
- Use LPC32xx definition from "cpu.h" and "clk.h".
- Incorporate DMA specific register definition from "lpc3250.h" header file from legacy BSP patch from LPCLinux.
- Use u-boot API for register access to remove the volatile in register definition taken from "lpc3250.h" header file.
- Add DMA interface to "dma.h".
- Add dma clock control register bits (clk.h).
- Add functions to initialize the DMA clock.
The legacy BSP patch (u-boot-2009.03_lpc32x0-v1.07.patch.tar.bz2) was downloaded from the LPCLinux Web site.
arch/arm/cpu/arm926ejs/lpc32xx/devices.c | 6 + arch/arm/include/asm/arch-lpc32xx/clk.h | 3 + arch/arm/include/asm/arch-lpc32xx/dma.h | 37 ++++++ arch/arm/include/asm/arch-lpc32xx/sys_proto.h | 1 + drivers/dma/Makefile | 1 + drivers/dma/lpc32xx_dma.c | 163 ++++++++++++++++++++++++++ 6 files changed, 211 insertions(+) create mode 100644 arch/arm/include/asm/arch-lpc32xx/dma.h create mode 100644 drivers/dma/lpc32xx_dma.c
diff --git a/arch/arm/cpu/arm926ejs/lpc32xx/devices.c b/arch/arm/cpu/arm926ejs/lpc32xx/devices.c index c0c9c6c..0d2ef7a 100644 --- a/arch/arm/cpu/arm926ejs/lpc32xx/devices.c +++ b/arch/arm/cpu/arm926ejs/lpc32xx/devices.c @@ -65,6 +65,12 @@ void lpc32xx_slc_nand_init(void) writel(CLK_NAND_SLC | CLK_NAND_SLC_SELECT, &clk->flashclk_ctrl); }
+void lpc32xx_dma_init(void) +{
- /* Enable DMA interface */
- setbits_le32(&clk->dmaclk_ctrl, DMA_CLK_ENABLE);
it is better to have writel() here, since &clk->dmaclk_ctrl can be contaminated, e.g. in case of soft reset or if uboot is executed from another bootloader.
+}
void lpc32xx_i2c_init(unsigned int devnum) { /* Enable I2C interface */ diff --git a/arch/arm/include/asm/arch-lpc32xx/clk.h b/arch/arm/include/asm/arch-lpc32xx/clk.h index 010211a..663f6bc 100644 --- a/arch/arm/include/asm/arch-lpc32xx/clk.h +++ b/arch/arm/include/asm/arch-lpc32xx/clk.h @@ -158,6 +158,9 @@ struct clk_pm_regs { #define CLK_NAND_SLC_SELECT (1 << 2) #define CLK_NAND_MLC_INT (1 << 5)
+/* DMA Clock Control Register bits */ +#define DMA_CLK_ENABLE (1 << 0)
/* SSP Clock Control Register bits */ #define CLK_SSP0_ENABLE_CLOCK (1 << 0)
diff --git a/arch/arm/include/asm/arch-lpc32xx/dma.h b/arch/arm/include/asm/arch-lpc32xx/dma.h new file mode 100644 index 0000000..15d829c --- /dev/null +++ b/arch/arm/include/asm/arch-lpc32xx/dma.h @@ -0,0 +1,37 @@ +/*
- LPC32xx DMA Controller Interface
- Copyright (C) 2008-2015 by NXP Semiconductors
- All rights reserved.
- @Author: Kevin Wells
- @Descr: Definitions for LPC3250 chip
- @References: NXP LPC3250 User's Guide
- SPDX-License-Identifier: GPL-2.0+
- */
+#ifndef _LPC32XX_DMA_H +#define _LPC32XX_DMA_H
+#include <common.h>
+/*
- DMA linked list structure used with a channel's LLI register;
- refer to UM10326, "LPC32x0 and LPC32x0/01 User manual" - Rev. 3
- tables 84, 85, 86 & 87 for details.
- */
+struct lpc32xx_dmac_ll {
- u32 dma_src;
- u32 dma_dest;
- u32 next_lli;
- u32 next_ctrl;
+};
+int lpc32xx_dma_get_channel(void); +int lpc32xx_dma_start_xfer(int channel, const struct lpc32xx_dmac_ll *desc,
u32 config);+int lpc32xx_dma_wait_status(int channel); +void lpc32xx_dma_put_channel(int channel);
There is no users of lpc32xx_dma_put_channel() interface, do you have them in mind?
+#endif /* _LPC32XX_DMA_H */ diff --git a/arch/arm/include/asm/arch-lpc32xx/sys_proto.h b/arch/arm/include/asm/arch-lpc32xx/sys_proto.h index 0845f83..7f997d9 100644 --- a/arch/arm/include/asm/arch-lpc32xx/sys_proto.h +++ b/arch/arm/include/asm/arch-lpc32xx/sys_proto.h @@ -13,6 +13,7 @@ void lpc32xx_uart_init(unsigned int uart_id); void lpc32xx_mac_init(void); void lpc32xx_mlc_nand_init(void); void lpc32xx_slc_nand_init(void); +void lpc32xx_dma_init(void);
I would propose to try to follow alphabetic ordering here, yes, lpc32xx_i2c_init() violates it, but please move lpc32xx_dma_init() upper.
void lpc32xx_i2c_init(unsigned int devnum); void lpc32xx_ssp_init(void); #if defined(CONFIG_SPL_BUILD) diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile index 4c8fcc2..f95fe70 100644 --- a/drivers/dma/Makefile +++ b/drivers/dma/Makefile @@ -10,3 +10,4 @@ obj-$(CONFIG_APBH_DMA) += apbh_dma.o obj-$(CONFIG_FSL_DMA) += fsl_dma.o obj-$(CONFIG_TI_KSNAV) += keystone_nav.o keystone_nav_cfg.o obj-$(CONFIG_TI_EDMA3) += ti-edma3.o +obj-$(CONFIG_DMA_LPC32XX) += lpc32xx_dma.o diff --git a/drivers/dma/lpc32xx_dma.c b/drivers/dma/lpc32xx_dma.c new file mode 100644 index 0000000..0452013 --- /dev/null +++ b/drivers/dma/lpc32xx_dma.c @@ -0,0 +1,163 @@ +/*
- Copyright (C) 2008-2015 by NXP Semiconductors
- All rights reserved.
- @Author: Kevin Wells
- @Descr: LPC3250 DMA controller interface support functions
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <asm/arch/dma.h> +#include <asm/arch/cpu.h> +#include <asm/arch/clk.h> +#include <asm/arch/sys_proto.h> +#include <asm/io.h>
+/*
- Notes:
- Only bitfield masks/values which are actually used by the driver
- are defined.
- */
The note can be removed, it does not add any useful information.
+/* DMA controller channel register structure */ +struct dmac_chan_reg {
- u32 src_addr;
- u32 dest_addr;
- u32 lli;
- u32 control;
- u32 config_ch;
- u32 reserved[3];
+};
+/* DMA controller register structures */ +struct dma_reg {
- u32 int_stat;
- u32 int_tc_stat;
- u32 int_tc_clear;
- u32 int_err_stat;
- u32 int_err_clear;
- u32 raw_tc_stat;
- u32 raw_err_stat;
- u32 chan_enable;
- u32 sw_burst_req;
- u32 sw_single_req;
- u32 sw_last_burst_req;
- u32 sw_last_single_req;
- u32 config;
- u32 sync;
- u32 reserved[50];
- struct dmac_chan_reg dma_chan[8];
+};
+/* Macro pointing to DMA registers */ +#define DMA_NO_OF_CHANNELS 8
+/* config register definitions */ +#define DMAC_CTRL_ENABLE (1 << 0) /* For enabling the DMA controller */
+static u32 alloc_ch;
+static struct dma_reg *dma = (struct dma_reg *)DMA_BASE;
+int lpc32xx_dma_get_channel(void) +{
- int i;
- if (!alloc_ch) { /* First time caller */
/** DMA clock are enable by "lpc32xx_dma_init()" and should* be call by board "board_early_init_f()" function.*//** Make sure DMA controller and all channels are disabled.* Controller is in little-endian mode. Disable sync signals.*/writel(0, &dma->config);writel(0, &dma->sync);/* Clear interrupt and error statuses */writel(0xFF, &dma->int_tc_clear);writel(0xFF, &dma->raw_tc_stat);writel(0xFF, &dma->int_err_clear);writel(0xFF, &dma->raw_err_stat);/* Enable DMA controller */writel(DMAC_CTRL_ENABLE, &dma->config);- }
- i = ffz(alloc_ch);
- /* Check if all the available channels are busy */
- if (unlikely(i == DMA_NO_OF_CHANNELS))
return -1;- alloc_ch |= BIT_MASK(i);
- return i;
+}
+int lpc32xx_dma_start_xfer(int channel, const struct lpc32xx_dmac_ll *desc,
u32 config)+{
- if (unlikely((BIT_MASK(channel) & alloc_ch) == 0)) {
Would it be possible to change "int channel" to some unsigned type?
If the intention is to reserve (channel < 0) for potentially invalid/unallocated channel, as it is done in NAND SLC, please add a check for this case here.
printf("ERR: Request for xfer on unallocated channel %d\r\n",
I believe "\r\n" can be replaced with "\n", both drivers of LPC32xx UARTs handle this correctly.
channel);BUG();
The function returns int type, but in fact it returns only 0, either this function return type can be changed to void, or IMO better to return error here instead of fatal BUG() and add a check on client side.
- }
- writel(BIT_MASK(channel), &dma->int_tc_clear);
- writel(BIT_MASK(channel), &dma->int_err_clear);
- writel(desc->dma_src, &dma->dma_chan[channel].src_addr);
- writel(desc->dma_dest, &dma->dma_chan[channel].dest_addr);
- writel(desc->next_lli, &dma->dma_chan[channel].lli);
- writel(desc->next_ctrl, &dma->dma_chan[channel].control);
- writel(config, &dma->dma_chan[channel].config_ch);
- return 0;
+}
+int lpc32xx_dma_wait_status(int channel) +{
- unsigned long start;
- u32 reg;
- start = get_timer(0);
- while (1) {
reg = readl(&dma->raw_tc_stat);reg |= readl(dma->raw_err_stat);if (reg & BIT_MASK(channel))break;if (get_timer(start) > 10000)
Please define the delay value as a macro, macro name should contain _MS suffix.
Also I would propose to add an explanatory pr_err() here.
return -1;udelay(1);- }
- if (unlikely(readl(&dma->raw_err_stat) & BIT_MASK(channel))) {
setbits_le32(&dma->int_err_clear, BIT_MASK(channel));setbits_le32(&dma->raw_err_stat, BIT_MASK(channel));
I would propose to add explanatory pr_err() here to let user know what happens.
return -1;- }
- setbits_le32(&dma->int_tc_clear, BIT_MASK(channel));
- setbits_le32(&dma->raw_tc_stat, BIT_MASK(channel));
- return 0;
+}
+void lpc32xx_dma_put_channel(int channel) +{
- /* Check if given channel no is valid */
- if (channel >= DMA_NO_OF_CHANNELS || channel < 0)
return;- alloc_ch &= ~BIT_MASK(channel);
- /* Shut down channel */
- writel(0, &dma->dma_chan[channel].control);
- writel(0, &dma->dma_chan[channel].config_ch);
- clrbits_le32(&dma->sync, BIT_MASK(channel));
- if (!alloc_ch)
/* Disable DMA controller */clrbits_le32(&dma->config, DMAC_CTRL_ENABLE);+}
-- With best wishes, Vladimir
Hi Vladimir,
Thanks for the feedback;
Marek, Vladimir,
there is a question below; I will wait for feedback before sending an updated version of the patch.
Sylvain
-----Original Message----- From: Vladimir Zapolskiy [mailto:vz@mleia.com]
Hi Sylvain,
On 05.08.2015 21:31, slemieux.tyco@gmail.com wrote:
From: Sylvain Lemieux slemieux@tycoint.com
Incorporate DMA driver from legacy LPCLinux NXP BSP. The files taken from the legacy patch are:
- lpc32xx DMA driver
- lpc3250 header file DMA registers definition.
The legacy driver was updated and clean-up as part of the integration with the latest u-boot.
Signed-off-by: Sylvain Lemieux slemieux@tycoint.com
[...]
diff --git a/arch/arm/include/asm/arch-lpc32xx/dma.h b/arch/arm/include/asm/arch-lpc32xx/dma.h new file mode 100644 index 0000000..15d829c --- /dev/null +++ b/arch/arm/include/asm/arch-lpc32xx/dma.h @@ -0,0 +1,37 @@ +/*
- LPC32xx DMA Controller Interface
[...]
+int lpc32xx_dma_get_channel(void); +int lpc32xx_dma_start_xfer(int channel, const struct lpc32xx_dmac_ll *desc,
u32 config);+int lpc32xx_dma_wait_status(int channel); +void lpc32xx_dma_put_channel(int channel);
There is no users of lpc32xx_dma_put_channel() interface, do you have them in mind?
The legacy NXP BSP driver was providing the support the get and release a channel; I kept it there, knowing is currently not used.
Do we want to keep this feature or remove it and only allowed channel allocation only?
[...]
+++ b/drivers/dma/lpc32xx_dma.c @@ -0,0 +1,163 @@ +/*
[...]
+int lpc32xx_dma_start_xfer(int channel, const struct lpc32xx_dmac_ll *desc,
u32 config)+{
- if (unlikely((BIT_MASK(channel) & alloc_ch) == 0)) {
Would it be possible to change "int channel" to some unsigned type?
If the intention is to reserve (channel < 0) for potentially invalid/unallocated channel, as it is done in NAND SLC, please add a check for this case here.
This was the legacy NXP BSP original behavior.
I will change the channel to "unsigned int" for the transfer API; the reserve API will continue to be return the channel as "int", to allowed validation; the NAND SLC will keep the channel number as an "unsigned int".
printf("ERR: Request for xfer on unallocated channel %d\r\n",
[...]
channel);BUG();The function returns int type, but in fact it returns only 0, either this function return type can be changed to void, or IMO better to return error here instead of fatal BUG() and add a check on client side.
I will modify it to return -1 on error and check on client side; The client side will generate the fatal BUG().
[...]
-- With best wishes, Vladimir
________________________________
This e-mail contains privileged and confidential information intended for the use of the addressees named above. If you are not the intended recipient of this e-mail, you are hereby notified that you must not disseminate, copy or take any action in respect of any information contained in it. If you have received this e-mail in error, please notify the sender immediately by e-mail and immediately destroy this e-mail and its attachments.
Hi Sylvain,
On 06.08.2015 16:50, LEMIEUX, SYLVAIN wrote:
Hi Vladimir,
Thanks for the feedback;
Marek, Vladimir,
there is a question below; I will wait for feedback before sending an updated version of the patch.
Sylvain
-----Original Message----- From: Vladimir Zapolskiy [mailto:vz@mleia.com]
Hi Sylvain,
On 05.08.2015 21:31, slemieux.tyco@gmail.com wrote:
From: Sylvain Lemieux slemieux@tycoint.com
Incorporate DMA driver from legacy LPCLinux NXP BSP. The files taken from the legacy patch are:
- lpc32xx DMA driver
- lpc3250 header file DMA registers definition.
The legacy driver was updated and clean-up as part of the integration with the latest u-boot.
Signed-off-by: Sylvain Lemieux slemieux@tycoint.com
[...]
diff --git a/arch/arm/include/asm/arch-lpc32xx/dma.h b/arch/arm/include/asm/arch-lpc32xx/dma.h new file mode 100644 index 0000000..15d829c --- /dev/null +++ b/arch/arm/include/asm/arch-lpc32xx/dma.h @@ -0,0 +1,37 @@ +/*
- LPC32xx DMA Controller Interface
[...]
+int lpc32xx_dma_get_channel(void); +int lpc32xx_dma_start_xfer(int channel, const struct lpc32xx_dmac_ll *desc,
u32 config);+int lpc32xx_dma_wait_status(int channel); +void lpc32xx_dma_put_channel(int channel);
There is no users of lpc32xx_dma_put_channel() interface, do you have them in mind?
The legacy NXP BSP driver was providing the support the get and release a channel; I kept it there, knowing is currently not used.
Do we want to keep this feature or remove it and only allowed channel allocation only?
[...]
I have no definite answer, in my opinion dead code should be removed, if its use is not planned. I would recommend to get the answer from a maintainer, who accepts this code (Tom Rini ?).
+++ b/drivers/dma/lpc32xx_dma.c @@ -0,0 +1,163 @@ +/*
[...]
+int lpc32xx_dma_start_xfer(int channel, const struct lpc32xx_dmac_ll *desc,
u32 config)+{
- if (unlikely((BIT_MASK(channel) & alloc_ch) == 0)) {
Would it be possible to change "int channel" to some unsigned type?
If the intention is to reserve (channel < 0) for potentially invalid/unallocated channel, as it is done in NAND SLC, please add a check for this case here.
This was the legacy NXP BSP original behavior.
I will change the channel to "unsigned int" for the transfer API; the reserve API will continue to be return the channel as "int", to allowed validation;
Generally there is no need to preserve any correspondence between mainlined code and code found in legacy BSP.
the NAND SLC will keep the channel number as an "unsigned int".
Okay, in this case the correctness of the passed argument is offloaded to a client. Fortunately the sole client NAND SLC does not pass -1 here (the statement is based on code review).
printf("ERR: Request for xfer on unallocated channel %d\r\n",[...]
channel);BUG();The function returns int type, but in fact it returns only 0, either this function return type can be changed to void, or IMO better to return error here instead of fatal BUG() and add a check on client side.
I will modify it to return -1 on error and check on client side; The client side will generate the fatal BUG().
Sounds good, I can imagine a potential client, which may gracefully handle this error, but NAND drivers are not in the list. If problem is encountered here, it means that the problem is on the client side and it is fair enough to place BUG() right on that client side.
-- With best wishes, Vladimir
-----Original Message----- From: Vladimir Zapolskiy [mailto:vz@mleia.com] Sent: 6-Aug-15 2:43 PM
Hi Sylvain,
On 06.08.2015 16:50, LEMIEUX, SYLVAIN wrote:
Hi Vladimir,
Thanks for the feedback;
Marek, Vladimir,
there is a question below; I will wait for feedback before sending an updated version of the patch.
Sylvain
-----Original Message----- From: Vladimir Zapolskiy [mailto:vz@mleia.com]
Hi Sylvain,
On 05.08.2015 21:31, slemieux.tyco@gmail.com wrote:
From: Sylvain Lemieux slemieux@tycoint.com
Incorporate DMA driver from legacy LPCLinux NXP BSP. The files taken from the legacy patch are:
- lpc32xx DMA driver
- lpc3250 header file DMA registers definition.
The legacy driver was updated and clean-up as part of the integration with the latest u-boot.
Signed-off-by: Sylvain Lemieux slemieux@tycoint.com
[...]
diff --git a/arch/arm/include/asm/arch-lpc32xx/dma.h b/arch/arm/include/asm/arch-lpc32xx/dma.h new file mode 100644 index 0000000..15d829c --- /dev/null +++ b/arch/arm/include/asm/arch-lpc32xx/dma.h @@ -0,0 +1,37 @@ +/*
- LPC32xx DMA Controller Interface
[...]
+int lpc32xx_dma_get_channel(void); +int lpc32xx_dma_start_xfer(int channel, const struct lpc32xx_dmac_ll *desc,
u32 config);+int lpc32xx_dma_wait_status(int channel); +void lpc32xx_dma_put_channel(int channel);
There is no users of lpc32xx_dma_put_channel() interface, do you have them in mind?
The legacy NXP BSP driver was providing the support the get and release a channel; I kept it there, knowing is currently not used.
Do we want to keep this feature or remove it and only allowed channel allocation only?
[...]
I have no definite answer, in my opinion dead code should be removed, if its use is not planned. I would recommend to get the answer from a maintainer, who accepts this code (Tom Rini ?).
Agree with you regarding dead code;
This code will only be useful if we were managing DMA channel allocation and release. I will remove it, as there is currently no need to support this functionality. If it is require one day, it can be added later.
[...]
-- With best wishes, Vladimir
________________________________
This e-mail contains privileged and confidential information intended for the use of the addressees named above. If you are not the intended recipient of this e-mail, you are hereby notified that you must not disseminate, copy or take any action in respect of any information contained in it. If you have received this e-mail in error, please notify the sender immediately by e-mail and immediately destroy this e-mail and its attachments.
participants (4)
-
LEMIEUX, SYLVAIN -
Marek Vasut -
slemieux.tycoï¼ gmail.com -
Vladimir Zapolskiy