Fixes / cleanups in dw_dmac (affects on few subsystems)
Subsystems (8)
Andy Shevchenko
reviewerv5 cover Fixes / cleanups in dw_dmac (affects on few subsystems)
This patch series (v3: http://www.spinics.net/lists/kernel/msg2215303.html)
contains a number of mostly minor fixes and cleanups for the DW DMA driver. A couple of them affect the DT binding so these may need to be updated to maintain compatibility (old format is still supported though). The rest should be relatively straight-forward.
This version has been tested on the following bare metal platforms:
- ATNGW100 (avr32 based platform) with dmatest
- Sam460ex (powerpc 44x based platform) with SATA
- Intel Braswell with UART
- Intel Galileo (Intel Quark based platform) with UART
(SATA driver and Intel Galileo UART support are based on this series and not
available in upstream yet)
Vinod, there are few patch sets developed on top of this one, so, the idea is to keep this in an immuutable branch / tag.
Changes since v4:
- send proper set of patches
- add changelog
Changes since v3:
- add patch 1 to check value of dma-masters property
- drop the upstreamed patches
- update patch 2 to keep an array for data-width property as well
Changes since v2:
- add patch 1 to fix master selection which was broken for long time
- remove "use field-by-field initialization" patch since like Mans metioned in has mostly no value and even might increase error prone
- rebase on top of recent linux-next
- wide testing on several platforms
Changes since v1:
- zeroing struct dw_dma_slave before use
- fall back to old data_width property if data-width is not found
- append tags for few patches
- correct title of cover letter
- rebase on top of recent linux-next
Andy Shevchenko (4):
dmaengine: dw: platform: check nr_masters to be non-zero
dmaengine: dw: revisit data_width property
dmaengine: dw: keep entire platform data in struct dw_dma
dmaengine: dw: pass platform data via struct dw_dma_chip
Documentation/devicetree/bindings/dma/snps-dma.txt | 6 +- arch/arc/boot/dts/abilis_tb10x.dtsi | 2 +- arch/arm/boot/dts/spear13xx.dtsi | 4 +- drivers/ata/sata_dwc_460ex.c | 2 +- drivers/dma/dw/core.c | 75 ++++++++-------------- drivers/dma/dw/pci.c | 5 +- drivers/dma/dw/platform.c | 33 ++++++---- drivers/dma/dw/regs.h | 5 +- include/linux/dma/dw.h | 5 +- include/linux/platform_data/dma-dw.h | 4 +- sound/soc/intel/common/sst-firmware.c | 2 +- 11 files changed, 65 insertions(+), 78 deletions(-)
Signature · 1 line
2.8.0.rc3
Andy Shevchenko
reviewerPATCH v5 1/4 dmaengine: dw: platform: check nr_masters to be non-zero
The nr_masters value equal to 0 is invalid since this DMA controller has to have at least one master.
Check this before we proceed with the rest of properties.
--- drivers/dma/dw/platform.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-)
drivers/dma/dw/platform.c +11 -10
@@ -103,6 +103,7 @@ dw_dma_parse_dt(struct platform_device *pdev)struct device_node *np = pdev->dev.of_node;struct dw_dma_platform_data *pdata;u32 tmp, arr[DW_DMA_MAX_NR_MASTERS ]; +u32 nr_masters;u32 nr_channels;if (!np) {
@@ -110,6 +111,13 @@ dw_dma_parse_dt(struct platform_device *pdev)return NULL ; } +if (of_property_read_u32 (np,"dma-masters" , &nr_masters)) +return NULL ; +if (nr_masters <1 || nr_masters >DW_DMA_MAX_NR_MASTERS ) +return NULL ; + + pdata->nr_masters = nr_masters; +if (of_property_read_u32 (np,"dma-channels" , &nr_channels))return NULL ;
@@ -131,17 +139,10 @@ dw_dma_parse_dt(struct platform_device *pdev)if (!of_property_read_u32 (np,"block_size" , &tmp)) pdata->block_size = tmp; -if (!of_property_read_u32 (np,"dma-masters" , &tmp)) { -if (tmp >DW_DMA_MAX_NR_MASTERS ) -return NULL ; - - pdata->nr_masters = tmp; - } - -if (!of_property_read_u32_array (np,"data_width" , arr, - pdata->nr_masters)) -for (tmp =0 ; tmp < pdata->nr_masters; tmp++) +if (!of_property_read_u32_array (np,"data_width" , arr, nr_masters)) { +for (tmp =0 ; tmp < nr_masters; tmp++) pdata->data_width[tmp] = arr[tmp]; + }return pdata; }
Signature · 1 line
2.8.0.rc3
kbuild test robot
build botRe: [PATCH v5 1/4] dmaengine: dw: platform: check nr_masters to be non-zero
reproduce:
Hi,
[auto build test WARNING on next-20160421]
[also build test WARNING on v4.6-rc4]
[cannot apply to robh/for-next asoc/for-next v4.6-rc4 v4.6-rc3 v4.6-rc2]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
url: https://github.com/0day-ci/linux/commits/Andy-Shevchenko/Fixes-cleanups-in-dw_dmac-affects-on-few-subsystems/20160421-231541 config: i386-randconfig-x003-201616 (attached as .config) reproduce: # save the attached .config to linux build tree make ARCH=i386
Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings
All warnings (new ones prefixed by >>):
drivers/dma/dw/platform.c: In function 'dw_probe':
>> drivers/dma/dw/platform.c:119:20: warning: 'pdata' may be used uninitialized in this function [-Wmaybe-uninitialized]
pdata->nr_masters = nr_masters;
^
drivers/dma/dw/platform.c:104:31: note: 'pdata' was declared here
struct dw_dma_platform_data *pdata;
^vim +/pdata +119 drivers/dma/dw/platform.c
103 struct device_node *np = pdev->dev.of_node;
104 struct dw_dma_platform_data *pdata;
105 u32 tmp, arr[DW_DMA_MAX_NR_MASTERS];
106 u32 nr_masters;
107 u32 nr_channels;
108
109 if (!np) {
110 dev_err(&pdev->dev, "Missing DT data\n");
111 return NULL;
112 }
113
114 if (of_property_read_u32(np, "dma-masters", &nr_masters))
115 return NULL;
116 if (nr_masters < 1 || nr_masters > DW_DMA_MAX_NR_MASTERS)
117 return NULL;
118
> 119 pdata->nr_masters = nr_masters;
120
121 if (of_property_read_u32(np, "dma-channels", &nr_channels))
122 return NULL;
123
124 pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
125 if (!pdata)
126 return NULL;
127
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Andy Shevchenko
reviewerPATCH v5 2/4 dmaengine: dw: revisit data_width property
In reply to Andy ShevchenkoThere are several changes are done here:
- Convert the property to be in bytes
Besides this is common practice for such property the use of a value in bytes much more convenient than handling the encoded value.
- Rename data_width to data-width in the device tree bindings
- While here, replace dwc_fast_ffs() by __ffs()
The change leaves the support for old format as well just in case someone will use a newer kernel with an old device tree blob.
--- Documentation/devicetree/bindings/dma/snps-dma.txt | 6 ++-- arch/arc/boot/dts/abilis_tb10x.dtsi | 2 +- arch/arm/boot/dts/spear13xx.dtsi | 4 +-- drivers/dma/dw/core.c | 42 ++++++---------------- drivers/dma/dw/platform.c | 5 ++- include/linux/platform_data/dma-dw.h | 2 +- 6 files changed, 21 insertions(+), 40 deletions(-)
Documentation/devicetree/bindings/dma/snps-dma.txt +3 -3
@@ -13,8 +13,8 @@ Required properties: - chan_priority: priority of channels. 0 (default): increase from chan 0->n, 1: increase from chan n->0 - block_size: Maximum block size supported by the controller -- data_width: Maximum data width supported by hardware per AHB master - (0 - 8bits, 1 - 16bits, ..., 5 - 256bits) +- data-width: Maximum data width supported by hardware per AHB master + (in bytes, power of 2) Optional properties:
@@ -38,7 +38,7 @@ Example: chan_allocation_order = <1>; chan_priority = <1>; block_size = <0xfff>; - data_width = <3 3>; + data-width = <8 8>; }; DMA clients connected to the Designware DMA controller must use the format
arch/arc/boot/dts/abilis_tb10x.dtsi +1 -1
@@ -126,7 +126,7 @@ chan_allocation_order = <0>; chan_priority = <1>; block_size = <0x7ff>; - data_width = <2>; + data-width = <4>; clocks = <&ahb_clk>; clock-names = "hclk"; };
arch/arm/boot/dts/spear13xx.dtsi +2 -2
@@ -117,7 +117,7 @@ chan_priority = <1>; block_size = <0xfff>; dma-masters = <2>; - data_width = <3 3>; + data-width = <8 8>; }; dma@eb000000 {
@@ -133,7 +133,7 @@ chan_allocation_order = <1>; chan_priority = <1>; block_size = <0xfff>; - data_width = <3 3>; + data-width = <8 8>; }; fsmc: flash@b0000000 {
drivers/dma/dw/core.c +10 -32
102 lines not on this page · raw message
drivers/dma/dw/platform.c +4 -1
@@ -139,9 +139,12 @@ dw_dma_parse_dt(struct platform_device *pdev)if (!of_property_read_u32 (np,"block_size" , &tmp)) pdata->block_size = tmp; -if (!of_property_read_u32_array (np,"data_width" , arr, nr_masters)) { +if (!of_property_read_u32_array (np,"data-width" , arr, nr_masters)) {for (tmp =0 ; tmp < nr_masters; tmp++) pdata->data_width[tmp] = arr[tmp]; + }else if (!of_property_read_u32_array (np,"data_width" , arr, nr_masters)) { +for (tmp =0 ; tmp < nr_masters; tmp++) + pdata->data_width[tmp] =BIT (arr[tmp] &0x07 ); }return pdata;
include/linux/platform_data/dma-dw.h +1 -1
@@ -43,7 +43,7 @@ struct dw_dma_slave {* @block_size: Maximum block size supported by the controller * @nr_masters: Number of AHB masters supported by the controller * @data_width: Maximum data width supported by hardware per AHB master -* (0 - 8bits, 1 - 16bits, ..., 5 - 256bits) +* (in bytes, power of 2) */ struct dw_dma_platform_data {unsigned int nr_channels;
Signature · 1 line
2.8.0.rc3
Andy Shevchenko
reviewerPATCH v5 3/4 dmaengine: dw: keep entire platform data in struct dw_dma
In reply to Andy ShevchenkoKeep the entire platform data in the struct dw_dma. It makes the driver a bit cleaner.
--- drivers/dma/dw/core.c | 30 +++++++++++++++--------------- drivers/dma/dw/platform.c | 4 ++-- drivers/dma/dw/regs.h | 5 ++--- include/linux/platform_data/dma-dw.h | 2 +- 4 files changed, 20 insertions(+), 21 deletions(-)
drivers/dma/dw/core.c +15 -15
@@ -665,7 +665,7 @@ dwc_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,u8 m_master = dwc->m_master;unsigned int src_width;unsigned int dst_width; -unsigned int data_width = dw->data_width[m_master]; +unsigned int data_width = dw->pdata->data_width[m_master];u32 ctllo;u8 lms =DWC_LLP_LMS (m_master);
@@ -745,7 +745,7 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,dma_addr_t reg;unsigned int reg_width;unsigned int mem_width; -unsigned int data_width = dw->data_width[m_master]; +unsigned int data_width = dw->pdata->data_width[m_master];unsigned int i;struct scatterlist *sg;size_t total_len =0 ;
@@ -1444,7 +1444,6 @@ int dw_dma_probe(struct dw_dma_chip *chip, struct dw_dma_platform_data *pdata)struct dw_dma *dw;bool autocfg = false;unsigned int dw_params; -unsigned int max_blk_size =0 ;unsigned int i;int err;
@@ -1452,6 +1451,10 @@ int dw_dma_probe(struct dw_dma_chip *chip, struct dw_dma_platform_data *pdata)if (!dw)return -ENOMEM ; + dw->pdata =devm_kzalloc (chip->dev,sizeof (*dw->pdata),GFP_KERNEL ); +if (!dw->pdata) +return -ENOMEM ; + dw->regs = chip->regs; chip->dw = dw;
@@ -1467,11 +1470,8 @@ int dw_dma_probe(struct dw_dma_chip *chip, struct dw_dma_platform_data *pdata) goto err_pdata; } - pdata =devm_kzalloc (chip->dev,sizeof (*pdata),GFP_KERNEL ); -if (!pdata) { - err = -ENOMEM ; - goto err_pdata; - } +/* Reassign the platform data pointer */ + pdata = dw->pdata;/* Get hardware configuration parameters */ pdata->nr_channels = (dw_params >>DW_PARAMS_NR_CHAN &7 ) +1 ;
@@ -1480,7 +1480,7 @@ int dw_dma_probe(struct dw_dma_chip *chip, struct dw_dma_platform_data *pdata) pdata->data_width[i] =4 << (dw_params >>DW_PARAMS_DATA_WIDTH (i) &3 ); } - max_blk_size =dma_readl (dw,MAX_BLK_SIZE ); + pdata->block_size =dma_readl (dw,MAX_BLK_SIZE );/* Fill platform data with the default values */ pdata->is_private = true;
@@ -1490,6 +1490,11 @@ int dw_dma_probe(struct dw_dma_chip *chip, struct dw_dma_platform_data *pdata) }else if (pdata->nr_channels >DW_DMA_MAX_NR_CHANNELS ) { err = -EINVAL ; goto err_pdata; + }else { +memcpy (dw->pdata, pdata,sizeof (*dw->pdata)); + +/* Reassign the platform data pointer */ + pdata = dw->pdata; } dw->chan = devm_kcalloc(chip->dev, pdata->nr_channels, sizeof(*dw->chan),
@@ -1499,11 +1504,6 @@ int dw_dma_probe(struct dw_dma_chip *chip, struct dw_dma_platform_data *pdata) goto err_pdata; } -/* Get hardware configuration parameters */ - dw->nr_masters = pdata->nr_masters; -for (i =0 ; i < dw->nr_masters; i++) - dw->data_width[i] = pdata->data_width[i]; -/* Calculate all channel mask before DMA setup */ dw->all_chan_mask = (1 << pdata->nr_channels) -1 ;
@@ -1570,7 +1570,7 @@ int dw_dma_probe(struct dw_dma_chip *chip, struct dw_dma_platform_data *pdata)* up to 0x0a for 4095. */ dwc->block_size = - (4 << ((max_blk_size >>4 * i) &0xf )) -1 ; + (4 << ((pdata->block_size >>4 * i) &0xf )) -1 ; dwc->nollp = (dwc_params >>DWC_PARAMS_MBLK_EN &0x1 ) ==0 ; }else {
drivers/dma/dw/platform.c +2 -2
@@ -47,8 +47,8 @@ static struct dma_chan *dw_dma_of_xlate(struct of_phandle_args *dma_spec,if (WARN_ON (slave.src_id >=DW_DMA_MAX_NR_REQUESTS || slave.dst_id >=DW_DMA_MAX_NR_REQUESTS || - slave.m_master >= dw->nr_masters || - slave.p_master >= dw->nr_masters)) + slave.m_master >= dw->pdata->nr_masters || + slave.p_master >= dw->pdata->nr_masters))return NULL ;dma_cap_zero (cap);
drivers/dma/dw/regs.h +2 -3
@@ -281,9 +281,8 @@ struct dw_dma {u8 all_chan_mask;u8 in_use; -/* hardware configuration */ -unsigned char nr_masters; -unsigned char data_width[DW_DMA_MAX_NR_MASTERS ]; +/* platform data */ +struct dw_dma_platform_data *pdata; };static inline struct dw_dma_regs __iomem *__dw_regs (struct dw_dma *dw)
include/linux/platform_data/dma-dw.h +1 -1
@@ -55,7 +55,7 @@ struct dw_dma_platform_data {#define CHAN_PRIORITY_ASCENDING 0/* chan0 highest */ #define CHAN_PRIORITY_DESCENDING 1/* chan7 highest */ unsigned char chan_priority; -unsigned short block_size; +unsigned int block_size;unsigned char nr_masters;unsigned char data_width[DW_DMA_MAX_NR_MASTERS ]; };
Signature · 1 line
2.8.0.rc3
Andy Shevchenko
reviewerPATCH v5 4/4 dmaengine: dw: pass platform data via struct dw_dma_chip
In reply to Andy ShevchenkoWe pass struct dw_dma_chip to the dw_dma_probe() anyway, thus we may use it to pass platform data as well.
While here, constify the source of platform data.
--- drivers/ata/sata_dwc_460ex.c | 2 +- drivers/dma/dw/core.c | 9 +++++---- drivers/dma/dw/pci.c | 5 +++-- drivers/dma/dw/platform.c | 5 +++-- include/linux/dma/dw.h | 5 ++++- sound/soc/intel/common/sst-firmware.c | 2 +- 6 files changed, 17 insertions(+), 11 deletions(-)
drivers/ata/sata_dwc_460ex.c +1 -1
@@ -1248,7 +1248,7 @@ static int sata_dwc_probe(struct platform_device *ofdev) hsdev->dma->dev = &ofdev->dev;/* Initialize AHB DMAC */ - err =dw_dma_probe (hsdev->dma,NULL ); + err =dw_dma_probe (hsdev->dma);if (err) goto error_dma_iomap;
drivers/dma/dw/core.c +5 -4
@@ -1439,8 +1439,9 @@ EXPORT_SYMBOL(dw_dma_cyclic_free);/*----------------------------------------------------------------------*/ -int dw_dma_probe (struct dw_dma_chip *chip,struct dw_dma_platform_data *pdata) +int dw_dma_probe (struct dw_dma_chip *chip) { +struct dw_dma_platform_data *pdata;struct dw_dma *dw;bool autocfg = false;unsigned int dw_params;
@@ -1460,7 +1461,7 @@ int dw_dma_probe(struct dw_dma_chip *chip, struct dw_dma_platform_data *pdata)pm_runtime_get_sync (chip->dev); -if (!pdata) { +if (!chip->pdata) { dw_params =dma_readl (dw,DW_PARAMS );dev_dbg (chip->dev,"DW_PARAMS: 0x%08x\n" , dw_params);
@@ -1487,11 +1488,11 @@ int dw_dma_probe(struct dw_dma_chip *chip, struct dw_dma_platform_data *pdata) pdata->is_memcpy = true; pdata->chan_allocation_order =CHAN_ALLOCATION_ASCENDING ; pdata->chan_priority =CHAN_PRIORITY_ASCENDING ; - }else if (pdata->nr_channels >DW_DMA_MAX_NR_CHANNELS ) { + }else if (chip->pdata->nr_channels >DW_DMA_MAX_NR_CHANNELS ) { err = -EINVAL ; goto err_pdata; }else { -memcpy (dw->pdata, pdata,sizeof (*dw->pdata)); +memcpy (dw->pdata, chip->pdata,sizeof (*dw->pdata));/* Reassign the platform data pointer */ pdata = dw->pdata;
drivers/dma/dw/pci.c +3 -2
@@ -17,8 +17,8 @@static int dw_pci_probe (struct pci_dev *pdev,const struct pci_device_id *pid) { +const struct dw_dma_platform_data *pdata = (void *)pid->driver_data;struct dw_dma_chip *chip; -struct dw_dma_platform_data *pdata = (void *)pid->driver_data;int ret; ret =pcim_enable_device (pdev);
@@ -49,8 +49,9 @@ static int dw_pci_probe(struct pci_dev *pdev, const struct pci_device_id *pid) chip->dev = &pdev->dev; chip->regs =pcim_iomap_table (pdev)[0 ]; chip->irq = pdev->irq; + chip->pdata = pdata; - ret =dw_dma_probe (chip, pdata); + ret =dw_dma_probe (chip);if (ret)return ret;
drivers/dma/dw/platform.c +3 -2
@@ -162,7 +162,7 @@ static int dw_probe(struct platform_device *pdev)struct dw_dma_chip *chip;struct device *dev = &pdev->dev;struct resource *mem; -struct dw_dma_platform_data *pdata; +const struct dw_dma_platform_data *pdata;int err; chip =devm_kzalloc (dev,sizeof (*chip),GFP_KERNEL );
@@ -187,6 +187,7 @@ static int dw_probe(struct platform_device *pdev) pdata =dw_dma_parse_dt (pdev); chip->dev = dev; + chip->pdata = pdata; chip->clk =devm_clk_get (chip->dev,"hclk" );if (IS_ERR (chip->clk))
@@ -197,7 +198,7 @@ static int dw_probe(struct platform_device *pdev)pm_runtime_enable (&pdev->dev); - err =dw_dma_probe (chip, pdata); + err =dw_dma_probe (chip);if (err) goto err_dw_dma_probe;
include/linux/dma/dw.h +4 -1
@@ -27,6 +27,7 @@ struct dw_dma;* @regs: memory mapped I/O space * @clk: hclk clock * @dw: struct dw_dma that is filed by dw_dma_probe() +* @pdata: pointer to platform data */ struct dw_dma_chip {struct device *dev;
@@ -34,10 +35,12 @@ struct dw_dma_chip {void __iomem *regs;struct clk *clk;struct dw_dma *dw; + +const struct dw_dma_platform_data *pdata; };/* Export to the platform drivers */ -int dw_dma_probe (struct dw_dma_chip *chip,struct dw_dma_platform_data *pdata); +int dw_dma_probe (struct dw_dma_chip *chip);int dw_dma_remove (struct dw_dma_chip *chip);/* DMA API extensions */
sound/soc/intel/common/sst-firmware.c +1 -1
@@ -203,7 +203,7 @@ static struct dw_dma_chip *dw_probe(struct device *dev, struct resource *mem, chip->dev = dev; - err =dw_dma_probe (chip,NULL ); + err =dw_dma_probe (chip);if (err)return ERR_PTR (err);
Signature · 1 line
2.8.0.rc3