dw_dmac: backlink to dw_dma in dw_dma_chan is superfluous
PATCH 1/2 dw_dmac: backlink to dw_dma in dw_dma_chan is superfluous
The same information could be exctracted from the struct dma_chan.
The patch introduces two helpers dwc_get_src_data_width() and dwc_get_dst_data_width() as well.
Signed-off-by Andy Shevchenko <[email protected]>
--- drivers/dma/dw_dmac.c | 27 ++++++++++++++++++++------- drivers/dma/dw_dmac_regs.h | 3 --- 2 files changed, 20 insertions(+), 10 deletions(-)
drivers/dma/dw_dmac.c +20 -7
@@ -73,6 +73,22 @@ static inline unsigned int dwc_get_sms(struct dw_dma_slave *slave)*/ #define NR_DESCS_PER_CHANNEL 64 +static inline unsigned int dwc_get_src_data_width (struct dma_chan *chan) +{ +struct dw_dma *dw =to_dw_dma (chan->device); +struct dw_dma_slave *dws = chan->private; + +return dw->data_width[dwc_get_sms (dws)]; +} + +static inline unsigned int dwc_get_dst_data_width (struct dma_chan *chan) +{ +struct dw_dma *dw =to_dw_dma (chan->device); +struct dw_dma_slave *dws = chan->private; + +return dw->data_width[dwc_get_dms (dws)]; +} +/*----------------------------------------------------------------------*/ /*
@@ -703,7 +719,6 @@ dwc_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src, size_t len, unsigned long flags) {struct dw_dma_chan *dwc =to_dw_dma_chan (chan); -struct dw_dma_slave *dws = chan->private;struct dw_desc *desc;struct dw_desc *first;struct dw_desc *prev;
@@ -726,8 +741,8 @@ dwc_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src, dwc->direction =DMA_MEM_TO_MEM ; - data_width = min_t(unsigned int, dwc->dw->data_width[dwc_get_sms(dws)], - dwc->dw->data_width[dwc_get_dms (dws)]); + data_width =min (dwc_get_src_data_width (chan), +dwc_get_dst_data_width (chan)); src_width = dst_width = min_t(unsigned int, data_width,dwc_fast_fls (src | dest | len));
@@ -792,7 +807,6 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl, unsigned long flags, void *context) { struct dw_dma_chan *dwc = to_dw_dma_chan(chan); -struct dw_dma_slave *dws = chan->private;struct dma_slave_config *sconfig = &dwc->dma_sconfig;struct dw_desc *prev;struct dw_desc *first;
@@ -826,7 +840,7 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl, ctllo |= sconfig->device_fc ?DWC_CTLL_FC (DW_DMA_FC_P_M2P ) :DWC_CTLL_FC (DW_DMA_FC_D_M2P ); - data_width = dwc->dw->data_width[dwc_get_sms (dws)]; + data_width =dwc_get_src_data_width (chan);for_each_sg (sgl, sg, sg_len, i) {struct dw_desc *desc;
@@ -889,7 +903,7 @@ slave_sg_todev_fill_desc: ctllo |= sconfig->device_fc ?DWC_CTLL_FC (DW_DMA_FC_P_P2M ) :DWC_CTLL_FC (DW_DMA_FC_D_P2M ); - data_width = dwc->dw->data_width[dwc_get_dms (dws)]; + data_width =dwc_get_dst_data_width (chan);for_each_sg (sgl, sg, sg_len, i) {struct dw_desc *desc;
@@ -1720,7 +1734,6 @@ static int dw_probe(struct platform_device *pdev)channel_clear_bit (dw,CH_EN , dwc->mask); - dwc->dw = dw; dwc->direction =DMA_TRANS_NONE ;/* hardware configuration */
drivers/dma/dw_dmac_regs.h +0 -3
@@ -214,9 +214,6 @@ struct dw_dma_chan {/* configuration passed via DMA_SLAVE_CONFIG */ struct dma_slave_config dma_sconfig; - -/* backlink to dw_dma */ -struct dw_dma *dw; }; static inline struct dw_dma_chan_regs __iomem *
Signature · 1 line
1.7.10.4
PATCH 2/2 dw_dmac: check for mapping errors
Otherwise we get a warning in case of CONFIG_DMA_API_DEBUG=y
[ 45.775943] WARNING: at lib/dma-debug.c:933 check_unmap+0x5d6/0x6ac() [ 45.782369] dw_dmac dw_dmac.0: DMA-API: device driver failed to check map error[device address=0x00000000356efcc0] [size=28 bytes] [mapped as single]
Signed-off-by Andy Shevchenko <[email protected]>
--- drivers/dma/dw_dmac.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)
drivers/dma/dw_dmac.c +12 -2
@@ -1107,6 +1107,7 @@ static int dwc_alloc_chan_resources(struct dma_chan *chan)struct dw_desc *desc;int i;unsigned long flags; +int ret =0 ;dev_vdbg (chan2dev (chan),"%s\n" , __func__);
@@ -1131,8 +1132,7 @@ static int dwc_alloc_chan_resources(struct dma_chan *chan) desc =kzalloc (sizeof (struct dw_desc ),GFP_KERNEL );if (!desc) { -dev_info (chan2dev (chan), -"only allocated %d descriptors\n" , i); + ret = -ENOMEM ;spin_lock_irqsave (&dwc->lock, flags);break ; }
@@ -1143,6 +1143,13 @@ static int dwc_alloc_chan_resources(struct dma_chan *chan) desc->txd.flags =DMA_CTRL_ACK ; desc->txd.phys =dma_map_single (chan2parent (chan), &desc->lli,sizeof (desc->lli),DMA_TO_DEVICE ); + ret =dma_mapping_error (chan2parent (chan), desc->txd.phys); +if (ret) { +kfree (desc); +spin_lock_irqsave (&dwc->lock, flags); +break ; + } +dwc_desc_put (dwc, desc);spin_lock_irqsave (&dwc->lock, flags);
@@ -1151,6 +1158,9 @@ static int dwc_alloc_chan_resources(struct dma_chan *chan)spin_unlock_irqrestore (&dwc->lock, flags); +if (ret) +dev_info (chan2dev (chan),"only allocated %d descriptors\n" , i); +dev_dbg (chan2dev (chan),"%s: allocated %d descriptors\n" , __func__, i);return i;
Signature · 1 line
1.7.10.4
Re: [PATCH 2/2] dw_dmac: check for mapping errors
Quoted 47 lines · Andy Shevchenko
Otherwise we get a warning in case of
CONFIG_DMA_API_DEBUG=y[ 45.775943] WARNING: at lib/dma-debug.c:933 check_unmap+0x5d6/0x6ac() [ 45.782369] dw_dmac dw_dmac.0: DMA-API: device driver failed to check map error[device address=0x00000000356efcc0] [size=28 bytes] [mapped as single]Signed-off-by Andy Shevchenko <[email protected]>--- drivers/dma/dw_dmac.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)drivers/dma/dw_dmac.c +9
-2@@ -1107,6 +1107,7 @@ static int dwc_alloc_chan_resources(struct dma_chan *chan)struct dw_desc *desc;int i;unsigned long flags; +int ret =0 ;dev_vdbg (chan2dev (chan),"%s\n" , __func__);@@ -1131,8 +1132,7 @@ static int dwc_alloc_chan_resources(struct dma_chan *chan) desc =kzalloc (sizeof (struct dw_desc ),GFP_KERNEL );if (!desc) { -dev_info (chan2dev (chan), -"only allocated %d descriptors\n" , i); + ret = -ENOMEM ;spin_lock_irqsave (&dwc->lock, flags);break ; }@@ -1143,6 +1143,13 @@ static int dwc_alloc_chan_resources(struct dma_chan *chan) desc->txd.flags =DMA_CTRL_ACK ; desc->txd.phys =dma_map_single (chan2parent (chan), &desc->lli,sizeof (desc->lli),DMA_TO_DEVICE ); + ret =dma_mapping_error (chan2parent (chan), desc->txd.phys); +if (ret) { +kfree (desc); +spin_lock_irqsave (&dwc->lock, flags); +break ; + } +dwc_desc_put (dwc, desc);spin_lock_irqsave (&dwc->lock, flags);
drivers/dma/dw_dmac.c static int dwc_alloc_chan_resources(struct dma_chan *chan) · line 1166
Quoted 9 lines
drivers/dma/dw_dmac.c +3
-0@@ -1151,6 +1158,9 @@ static int dwc_alloc_chan_resources(struct dma_chan *chan)spin_unlock_irqrestore (&dwc->lock, flags); +if (ret) +dev_info (chan2dev (chan),"only allocated %d descriptors\n" , i); +dev_dbg (chan2dev (chan),"%s: allocated %d descriptors\n" , __func__, i);return i;
I would use goto with appropriate label: to avoid lock/unlock of spin_lock.
Quoted 16 lines · Andy Shevchenko
The same information could be exctracted from the struct
dma_chan.The patch introduces two helpers
dwc_get_src_data_width()anddwc_get_dst_data_width()as well.Signed-off-by Andy Shevchenko <[email protected]>--- drivers/dma/dw_dmac.c | 27 ++++++++++++++++++++------- drivers/dma/dw_dmac_regs.h | 3 --- 2 files changed, 20 insertions(+), 10 deletions(-)drivers/dma/dw_dmac.c +0
-0
drivers/dma/dw_dmac.c static inline unsigned int dwc_get_sms(struct dw_dma_slave *slave) · line 79
Quoted 7 lines
drivers/dma/dw_dmac.c +4
-0@@ -73,6 +73,22 @@ static inline unsigned int dwc_get_sms(struct dw_dma_slave *slave)*/ #define NR_DESCS_PER_CHANNEL 64 +static inline unsigned int dwc_get_src_data_width (struct dma_chan *chan) +{ +struct dw_dma *dw =to_dw_dma (chan->device); +struct dw_dma_slave *dws = chan->private;
I never liked the tabs used before the variable names in this driver. Though have kept them while fixing existing routines.
So, i would like you to use spaces for new functions.
drivers/dma/dw_dmac.c static inline unsigned int dwc_get_sms(struct dw_dma_slave *slave) · line 90
Quoted 11 lines
drivers/dma/dw_dmac.c +11
-0+ +return dw->data_width[dwc_get_sms (dws)]; +} + +static inline unsigned int dwc_get_dst_data_width (struct dma_chan *chan) +{ +struct dw_dma *dw =to_dw_dma (chan->device); +struct dw_dma_slave *dws = chan->private; + +return dw->data_width[dwc_get_dms (dws)]; +}
Can you try merging these routines into a single routine, by adding a single param?
Quoted 12 lines
drivers/dma/dw_dmac.c +0
-1/*----------------------------------------------------------------------*/ /* @@ -703,7 +719,6 @@ dwc_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src, size_t len, unsigned long flags) {struct dw_dma_chan *dwc =to_dw_dma_chan (chan); -struct dw_dma_slave *dws = chan->private;struct dw_desc *desc;struct dw_desc *first;struct dw_desc *prev;
drivers/dma/dw_dmac.c dwc_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src, · line 745
Quoted 7 lines
drivers/dma/dw_dmac.c +2
-2@@ -726,8 +741,8 @@ dwc_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src, dwc->direction =DMA_MEM_TO_MEM ; - data_width = min_t(unsigned int, dwc->dw->data_width[dwc_get_sms(dws)], - dwc->dw->data_width[dwc_get_dms (dws)]); + data_width =min (dwc_get_src_data_width (chan), +dwc_get_dst_data_width (chan));
min_t() is more efficient then min(), that's why it was used.
Quoted 53 lines
drivers/dma/dw_dmac.c +2
-4src_width = dst_width = min_t(unsigned int, data_width,dwc_fast_fls (src | dest | len));@@ -792,7 +807,6 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl, unsigned long flags, void *context) { struct dw_dma_chan *dwc = to_dw_dma_chan(chan); -struct dw_dma_slave *dws = chan->private;struct dma_slave_config *sconfig = &dwc->dma_sconfig;struct dw_desc *prev;struct dw_desc *first;@@ -826,7 +840,7 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl, ctllo |= sconfig->device_fc ?DWC_CTLL_FC (DW_DMA_FC_P_M2P ) :DWC_CTLL_FC (DW_DMA_FC_D_M2P ); - data_width = dwc->dw->data_width[dwc_get_sms (dws)]; + data_width =dwc_get_src_data_width (chan);for_each_sg (sgl, sg, sg_len, i) {struct dw_desc *desc;@@ -889,7 +903,7 @@ slave_sg_todev_fill_desc: ctllo |= sconfig->device_fc ?DWC_CTLL_FC (DW_DMA_FC_P_P2M ) :DWC_CTLL_FC (DW_DMA_FC_D_P2M ); - data_width = dwc->dw->data_width[dwc_get_dms (dws)]; + data_width =dwc_get_dst_data_width (chan);for_each_sg (sgl, sg, sg_len, i) {struct dw_desc *desc;@@ -1720,7 +1734,6 @@ static int dw_probe(struct platform_device *pdev)channel_clear_bit (dw,CH_EN , dwc->mask); - dwc->dw = dw; dwc->direction =DMA_TRANS_NONE ;/* hardware configuration */ drivers/dma/dw_dmac_regs.h +0
-4@@ -214,9 +214,6 @@ struct dw_dma_chan {/* configuration passed via DMA_SLAVE_CONFIG */ struct dma_slave_config dma_sconfig; - -/* backlink to dw_dma */ -struct dw_dma *dw; }; static inline struct dw_dma_chan_regs __iomem * --1.7.10.4