| Message ID | 5e22c587-5698-e132-4429-48674a6ddbb7@omp.ru (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mmc: renesas_sdhi_sys_dmac: use DMA_SLAVE_BUSWIDTH_UNDEFINED | expand |
+ Vinod On Fri, 21 Jan 2022 at 21:38, Sergey Shtylyov <s.shtylyov@omp.ru> wrote: > > The 'dma_slave_config::{src|dst}_addr_width' fields have the *enum* type > which isn't isomorphic with the *bool* type, however is used as a boolean > expression. Use the *enum* dma_slave_buswidth's value corresponding to 0 > instead. > > Found by Linux Verification Center (linuxtesting.org) with the SVACE static > analysis tool. I have no strong opinion, but just wanted to make sure this is inline with what Vinod wants. There are not so many users checking DMA_SLAVE_BUSWIDTH_UNDEFINED, perhaps we should simply drop it and instead leave the checks below as is? > > Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru> Kind regards Uffe > > --- > This patch is against the 'next' branch of Ulf Hansson's 'mmc.git' repo. > > drivers/mmc/host/renesas_sdhi_sys_dmac.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > Index: mmc/drivers/mmc/host/renesas_sdhi_sys_dmac.c > =================================================================== > --- mmc.orig/drivers/mmc/host/renesas_sdhi_sys_dmac.c > +++ mmc/drivers/mmc/host/renesas_sdhi_sys_dmac.c > @@ -370,7 +370,7 @@ static void renesas_sdhi_sys_dmac_reques > cfg.dst_addr = res->start + > (CTL_SD_DATA_PORT << host->bus_shift); > cfg.dst_addr_width = priv->dma_priv.dma_buswidth; > - if (!cfg.dst_addr_width) > + if (cfg.dst_addr_width == DMA_SLAVE_BUSWIDTH_UNDEFINED) > cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES; > cfg.src_addr = 0; > ret = dmaengine_slave_config(host->chan_tx, &cfg); > @@ -389,7 +389,7 @@ static void renesas_sdhi_sys_dmac_reques > cfg.direction = DMA_DEV_TO_MEM; > cfg.src_addr = cfg.dst_addr + host->pdata->dma_rx_offset; > cfg.src_addr_width = priv->dma_priv.dma_buswidth; > - if (!cfg.src_addr_width) > + if (cfg.src_addr_width == DMA_SLAVE_BUSWIDTH_UNDEFINED) > cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES; > cfg.dst_addr = 0; > ret = dmaengine_slave_config(host->chan_rx, &cfg);
On 31-01-22, 17:26, Ulf Hansson wrote: > + Vinod > > On Fri, 21 Jan 2022 at 21:38, Sergey Shtylyov <s.shtylyov@omp.ru> wrote: > > > > The 'dma_slave_config::{src|dst}_addr_width' fields have the *enum* type > > which isn't isomorphic with the *bool* type, however is used as a boolean > > expression. Use the *enum* dma_slave_buswidth's value corresponding to 0 > > instead. > > > > Found by Linux Verification Center (linuxtesting.org) with the SVACE static > > analysis tool. > > I have no strong opinion, but just wanted to make sure this is inline > with what Vinod wants. > > There are not so many users checking DMA_SLAVE_BUSWIDTH_UNDEFINED, > perhaps we should simply drop it and instead leave the checks below as > is? DMA_SLAVE_BUSWIDTH_UNDEFINED was defined as 0. I think it might be overkill and should be fine to keep the check as is.. > > > > > Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru> > > Kind regards > Uffe > > > > > --- > > This patch is against the 'next' branch of Ulf Hansson's 'mmc.git' repo. > > > > drivers/mmc/host/renesas_sdhi_sys_dmac.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > Index: mmc/drivers/mmc/host/renesas_sdhi_sys_dmac.c > > =================================================================== > > --- mmc.orig/drivers/mmc/host/renesas_sdhi_sys_dmac.c > > +++ mmc/drivers/mmc/host/renesas_sdhi_sys_dmac.c > > @@ -370,7 +370,7 @@ static void renesas_sdhi_sys_dmac_reques > > cfg.dst_addr = res->start + > > (CTL_SD_DATA_PORT << host->bus_shift); > > cfg.dst_addr_width = priv->dma_priv.dma_buswidth; > > - if (!cfg.dst_addr_width) > > + if (cfg.dst_addr_width == DMA_SLAVE_BUSWIDTH_UNDEFINED) > > cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES; > > cfg.src_addr = 0; > > ret = dmaengine_slave_config(host->chan_tx, &cfg); > > @@ -389,7 +389,7 @@ static void renesas_sdhi_sys_dmac_reques > > cfg.direction = DMA_DEV_TO_MEM; > > cfg.src_addr = cfg.dst_addr + host->pdata->dma_rx_offset; > > cfg.src_addr_width = priv->dma_priv.dma_buswidth; > > - if (!cfg.src_addr_width) > > + if (cfg.src_addr_width == DMA_SLAVE_BUSWIDTH_UNDEFINED) > > cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES; > > cfg.dst_addr = 0; > > ret = dmaengine_slave_config(host->chan_rx, &cfg);
Index: mmc/drivers/mmc/host/renesas_sdhi_sys_dmac.c =================================================================== --- mmc.orig/drivers/mmc/host/renesas_sdhi_sys_dmac.c +++ mmc/drivers/mmc/host/renesas_sdhi_sys_dmac.c @@ -370,7 +370,7 @@ static void renesas_sdhi_sys_dmac_reques cfg.dst_addr = res->start + (CTL_SD_DATA_PORT << host->bus_shift); cfg.dst_addr_width = priv->dma_priv.dma_buswidth; - if (!cfg.dst_addr_width) + if (cfg.dst_addr_width == DMA_SLAVE_BUSWIDTH_UNDEFINED) cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES; cfg.src_addr = 0; ret = dmaengine_slave_config(host->chan_tx, &cfg); @@ -389,7 +389,7 @@ static void renesas_sdhi_sys_dmac_reques cfg.direction = DMA_DEV_TO_MEM; cfg.src_addr = cfg.dst_addr + host->pdata->dma_rx_offset; cfg.src_addr_width = priv->dma_priv.dma_buswidth; - if (!cfg.src_addr_width) + if (cfg.src_addr_width == DMA_SLAVE_BUSWIDTH_UNDEFINED) cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES; cfg.dst_addr = 0; ret = dmaengine_slave_config(host->chan_rx, &cfg);
The 'dma_slave_config::{src|dst}_addr_width' fields have the *enum* type which isn't isomorphic with the *bool* type, however is used as a boolean expression. Use the *enum* dma_slave_buswidth's value corresponding to 0 instead. Found by Linux Verification Center (linuxtesting.org) with the SVACE static analysis tool. Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru> --- This patch is against the 'next' branch of Ulf Hansson's 'mmc.git' repo. drivers/mmc/host/renesas_sdhi_sys_dmac.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)