spi: spi-ep93xx: Change dir type in ep93xx_spi_dma_{finish,prepare}

Clang warns when one enumerated type is implicitly converted to another.

drivers/spi/spi-ep93xx.c:342:62: warning: implicit conversion from enumeration type 'enum dma_transfer_direction' to different enumeration type 'enum dma_data_direction' [-Wenum-conversion]

        nents = dma_map_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:428:58: note: expanded from macro
'dma_map_sg'
#define dma_map_sg(d, s, n, r) dma_map_sg_attrs(d, s, n, r, 0)
                               ~~~~~~~~~~~~~~~~          ^
drivers/spi/spi-ep93xx.c:348:57: warning: implicit conversion from
enumeration type 'enum dma_transfer_direction' to different enumeration
type 'enum dma_data_direction' [-Wenum-conversion]
                dma_unmap_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:429:62: note: expanded from macro
'dma_unmap_sg'
#define dma_unmap_sg(d, s, n, r) dma_unmap_sg_attrs(d, s, n, r, 0)
                                 ~~~~~~~~~~~~~~~~~~          ^
drivers/spi/spi-ep93xx.c:377:56: warning: implicit conversion from
enumeration type 'enum dma_transfer_direction' to different enumeration
type 'enum dma_data_direction' [-Wenum-conversion]
        dma_unmap_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:429:62: note: expanded from macro
'dma_unmap_sg'
#define dma_unmap_sg(d, s, n, r) dma_unmap_sg_attrs(d, s, n, r, 0)
                                 ~~~~~~~~~~~~~~~~~~          ^

3 warnings generated.

dma_{,un}map_sg expects an enum of type dma_data_direction but this driver uses dma_transfer_direction for everything. Converting to dma_data_direction would be desirable but there are a few shared structures that expect dma_transfer_direction so it is just simpler to change the parameter here. dma_transfer_direction and dma_data_direction are different sizes but this driver only uses the 1 and 2 values which mean the same thing so this change is safe.

Signed-off-by Nathan Chancellor <[email protected]>

---

v1 -> v2:

* Fix escaped hash symbols for '#define' lines

 drivers/spi/spi-ep93xx.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

drivers/spi/spi-ep93xx.c +2 -5

index f1526757aaf6..189fc2225b69 100644
@@ -256,8 +256,7 @@ static int ep93xx_spi_read_write(struct spi_master *master)
  * in case of failure.
  */
 static struct dma_async_tx_descriptor *
-ep93xx_spi_dma_prepare(struct spi_master *master,
-                      enum dma_transfer_direction dir)
+ep93xx_spi_dma_prepare(struct spi_master *master, int dir)
 {
        struct ep93xx_spi *espi = spi_master_get_devdata(master);
        struct spi_transfer *xfer = master->cur_msg->state;

@@ -359,8 +358,7 @@ ep93xx_spi_dma_prepare(struct spi_master *master,
  * Function finishes with the DMA transfer. After this, the DMA buffer is
  * unmapped.
  */
-static void ep93xx_spi_dma_finish(struct spi_master *master,
-                                 enum dma_transfer_direction dir)
+static void ep93xx_spi_dma_finish(struct spi_master *master, int dir)
 {
        struct ep93xx_spi *espi = spi_master_get_devdata(master);
        struct dma_chan *chan;
--

2.19.0

Quoted 6 lines · Nathan Chancellor

Clang warns when one enumerated type is implicitly converted to another.

drivers/spi/spi-ep93xx.c:342:62: warning: implicit conversion from enumeration type 'enum dma_transfer_direction' to different enumeration type 'enum dma_data_direction' [-Wenum-conversion]

I'm just trying to think what this change would look like if this driver ONLY used enum dma_data_direction? There's an additional

warning in the header that a return value is messed up there, too:

In file included from drivers/spi/spi-ep93xx.c:34:
./include/linux/platform_data/dma-ep93xx.h:88:10: warning: implicit conversion from
enumeration type 'enum dma_data_direction' to different
enumeration type 'enum

      dma_transfer_direction' [-Wenum-conversion]
                return DMA_NONE;
                ~~~~~~ ^~~~~~~~

Quoted 71 lines

        nents = dma_map_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:428:58: note: expanded from macro
'dma_map_sg'
#define dma_map_sg(d, s, n, r) dma_map_sg_attrs(d, s, n, r, 0)
                               ~~~~~~~~~~~~~~~~          ^
drivers/spi/spi-ep93xx.c:348:57: warning: implicit conversion from
enumeration type 'enum dma_transfer_direction' to different enumeration
type 'enum dma_data_direction' [-Wenum-conversion]
                dma_unmap_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:429:62: note: expanded from macro
'dma_unmap_sg'
#define dma_unmap_sg(d, s, n, r) dma_unmap_sg_attrs(d, s, n, r, 0)
                                 ~~~~~~~~~~~~~~~~~~          ^
drivers/spi/spi-ep93xx.c:377:56: warning: implicit conversion from
enumeration type 'enum dma_transfer_direction' to different enumeration
type 'enum dma_data_direction' [-Wenum-conversion]
        dma_unmap_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:429:62: note: expanded from macro
'dma_unmap_sg'
#define dma_unmap_sg(d, s, n, r) dma_unmap_sg_attrs(d, s, n, r, 0)
                                 ~~~~~~~~~~~~~~~~~~          ^

3 warnings generated.

dma_{,un}map_sg expects an enum of type dma_data_direction but this driver uses dma_transfer_direction for everything. Converting to dma_data_direction would be desirable but there are a few shared structures that expect dma_transfer_direction so it is just simpler to change the parameter here. dma_transfer_direction and dma_data_direction are different sizes but this driver only uses the 1 and 2 values which mean the same thing so this change is safe.

Signed-off-by Nathan Chancellor <[email protected]>

---

v1 -> v2:

* Fix escaped hash symbols for '#define' lines

 drivers/spi/spi-ep93xx.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

drivers/spi/spi-ep93xx.c +2 -5

index f1526757aaf6..189fc2225b69 100644
@@ -256,8 +256,7 @@ static int ep93xx_spi_read_write(struct spi_master *master)
  * in case of failure.
  */
 static struct dma_async_tx_descriptor *
-ep93xx_spi_dma_prepare(struct spi_master *master,
-                      enum dma_transfer_direction dir)
+ep93xx_spi_dma_prepare(struct spi_master *master, int dir)
 {
        struct ep93xx_spi *espi = spi_master_get_devdata(master);
        struct spi_transfer *xfer = master->cur_msg->state;

@@ -359,8 +358,7 @@ ep93xx_spi_dma_prepare(struct spi_master *master,
  * Function finishes with the DMA transfer. After this, the DMA buffer is
  * unmapped.
  */
-static void ep93xx_spi_dma_finish(struct spi_master *master,
-                                 enum dma_transfer_direction dir)
+static void ep93xx_spi_dma_finish(struct spi_master *master, int dir)
 {
        struct ep93xx_spi *espi = spi_master_get_devdata(master);
        struct dma_chan *chan;
--

2.19.0

Signature · 2 lines

Thanks,
~Nick Desaulniers

Quoted 9 lines · Nick Desaulniers

Quoted 6 lines · Nathan Chancellor

Clang warns when one enumerated type is implicitly converted to another.

drivers/spi/spi-ep93xx.c:342:62: warning: implicit conversion from enumeration type 'enum dma_transfer_direction' to different enumeration type 'enum dma_data_direction' [-Wenum-conversion]

Quoted 2 lines

I'm just trying to think what this change would look like if this driver ONLY used enum dma_data_direction? There's an additional

I can work up a proof of concept patch but it was kind of convoluted when I tried before this patch because dmaengine_prep_slave_sg and the direction member in struct dma_slave_config expect an enum of type dma_transfer_direction so there are two explicit casts needed and I thought there were one or two more places that it complained.

This was the cleanest solution to me but I'm happy to rework it in any way that I need to.

Quoted 11 lines

warning in the header that a return value is messed up there, too:

In file included from drivers/spi/spi-ep93xx.c:34:
./include/linux/platform_data/dma-ep93xx.h:88:10: warning: implicit conversion from
enumeration type 'enum dma_data_direction' to different
enumeration type 'enum

      dma_transfer_direction' [-Wenum-conversion]
                return DMA_NONE;
                ~~~~~~ ^~~~~~~~

This is fixed in the dma tree and that function isn't used in this file:
https://git.kernel.org/pub/scm/linux/kernel/git/vkoul/slave-dma.git/commit/?id=9524d6b265f9b2b9a61fceb2ee2ce1c2a83e39ca

Quoted 77 lines

Quoted 70 lines

        nents = dma_map_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:428:58: note: expanded from macro
'dma_map_sg'
#define dma_map_sg(d, s, n, r) dma_map_sg_attrs(d, s, n, r, 0)
                               ~~~~~~~~~~~~~~~~          ^
drivers/spi/spi-ep93xx.c:348:57: warning: implicit conversion from
enumeration type 'enum dma_transfer_direction' to different enumeration
type 'enum dma_data_direction' [-Wenum-conversion]
                dma_unmap_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:429:62: note: expanded from macro
'dma_unmap_sg'
#define dma_unmap_sg(d, s, n, r) dma_unmap_sg_attrs(d, s, n, r, 0)
                                 ~~~~~~~~~~~~~~~~~~          ^
drivers/spi/spi-ep93xx.c:377:56: warning: implicit conversion from
enumeration type 'enum dma_transfer_direction' to different enumeration
type 'enum dma_data_direction' [-Wenum-conversion]
        dma_unmap_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:429:62: note: expanded from macro
'dma_unmap_sg'
#define dma_unmap_sg(d, s, n, r) dma_unmap_sg_attrs(d, s, n, r, 0)
                                 ~~~~~~~~~~~~~~~~~~          ^

3 warnings generated.

dma_{,un}map_sg expects an enum of type dma_data_direction but this driver uses dma_transfer_direction for everything. Converting to dma_data_direction would be desirable but there are a few shared structures that expect dma_transfer_direction so it is just simpler to change the parameter here. dma_transfer_direction and dma_data_direction are different sizes but this driver only uses the 1 and 2 values which mean the same thing so this change is safe.

Signed-off-by Nathan Chancellor <[email protected]>

---

v1 -> v2:

* Fix escaped hash symbols for '#define' lines

 drivers/spi/spi-ep93xx.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

drivers/spi/spi-ep93xx.c +2 -5

index f1526757aaf6..189fc2225b69 100644
@@ -256,8 +256,7 @@ static int ep93xx_spi_read_write(struct spi_master *master)
  * in case of failure.
  */
 static struct dma_async_tx_descriptor *
-ep93xx_spi_dma_prepare(struct spi_master *master,
-                      enum dma_transfer_direction dir)
+ep93xx_spi_dma_prepare(struct spi_master *master, int dir)
 {
        struct ep93xx_spi *espi = spi_master_get_devdata(master);
        struct spi_transfer *xfer = master->cur_msg->state;

@@ -359,8 +358,7 @@ ep93xx_spi_dma_prepare(struct spi_master *master,
  * Function finishes with the DMA transfer. After this, the DMA buffer is
  * unmapped.
  */
-static void ep93xx_spi_dma_finish(struct spi_master *master,
-                                 enum dma_transfer_direction dir)
+static void ep93xx_spi_dma_finish(struct spi_master *master, int dir)
 {
        struct ep93xx_spi *espi = spi_master_get_devdata(master);
        struct dma_chan *chan;
--

Quoted 2 lines

2.19.0

--
Thanks,
~Nick Desaulniers

Clang warns when one enumerated type is implicitly converted to another.

drivers/spi/spi-ep93xx.c:342:62: warning: implicit conversion from enumeration type 'enum dma_transfer_direction' to different enumeration type 'enum dma_data_direction' [-Wenum-conversion]

        nents = dma_map_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:428:58: note: expanded from macro
'dma_map_sg'
#define dma_map_sg(d, s, n, r) dma_map_sg_attrs(d, s, n, r, 0)
                               ~~~~~~~~~~~~~~~~          ^
drivers/spi/spi-ep93xx.c:348:57: warning: implicit conversion from
enumeration type 'enum dma_transfer_direction' to different enumeration
type 'enum dma_data_direction' [-Wenum-conversion]
                dma_unmap_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:429:62: note: expanded from macro
'dma_unmap_sg'
#define dma_unmap_sg(d, s, n, r) dma_unmap_sg_attrs(d, s, n, r, 0)
                                 ~~~~~~~~~~~~~~~~~~          ^
drivers/spi/spi-ep93xx.c:377:56: warning: implicit conversion from
enumeration type 'enum dma_transfer_direction' to different enumeration
type 'enum dma_data_direction' [-Wenum-conversion]
        dma_unmap_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:429:62: note: expanded from macro
'dma_unmap_sg'
#define dma_unmap_sg(d, s, n, r) dma_unmap_sg_attrs(d, s, n, r, 0)
                                 ~~~~~~~~~~~~~~~~~~          ^

3 warnings generated.

dma_{,un}map_sg expect an enum of type dma_data_direction but this driver uses dma_transfer_direction for everything. Convert the driver to use dma_data_direction for these two functions.

There are two places that strictly require an enum of type
dma_transfer_direction: the direction member in struct dma_slave_config and the direction parameter in dmaengine_prep_slave_sg. To avoid using an explicit cast, add a simple function, ep93xx_dma_data_to_trans_dir, to safely map between the two types because they are not 1 to 1 in meaning.

Signed-off-by Nathan Chancellor <[email protected]>

---

v1 -> v2:

* Fix escaped hash symbols for '#define' lines (\# -> #)

v2 -> v3:

* Instead of changing the dir parameter in the prepare and finish functions, convert the driver to use dma_data_direction for everywhere that makes sense. Add a helper function to convert between the two types as there is still a couple of locations that need a transfer enum (and no such function exists). Alternatively, the function could just convert from transfer -> data and be used in dma_{,un}map_sg and the rest of the driver be left alone. Hopefully I understood what Mika wanted!

 drivers/spi/spi-ep93xx.c | 37 ++++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)

drivers/spi/spi-ep93xx.c +16 -3

index f1526757aaf6..667da7964d63 100644
@@ -246,6 +246,19 @@ static int ep93xx_spi_read_write(struct spi_master *master)
        return -EINPROGRESS;
 }
 
+static enum dma_transfer_direction
+ep93xx_dma_data_to_trans_dir(enum dma_data_direction dir)
+{
+       switch (dir) {
+       case DMA_TO_DEVICE:
+               return DMA_MEM_TO_DEV;
+       case DMA_FROM_DEVICE:
+               return DMA_DEV_TO_MEM;
+       default:
+               return DMA_TRANS_NONE;
+       }
+}
+
 /**
  * ep93xx_spi_dma_prepare() - prepares a DMA transfer
  * @master: SPI master

@@ -257,7 +270,7 @@ static int ep93xx_spi_read_write(struct spi_master *master)
  */
 static struct dma_async_tx_descriptor *
 ep93xx_spi_dma_prepare(struct spi_master *master,
-                      enum dma_transfer_direction dir)
+                      enum dma_data_direction dir)
 {
        struct ep93xx_spi *espi = spi_master_get_devdata(master);
        struct spi_transfer *xfer = master->cur_msg->state;

@@ -277,9 +290,9 @@ ep93xx_spi_dma_prepare(struct spi_master *master,
                buswidth = DMA_SLAVE_BUSWIDTH_1_BYTE;
 
        memset(&conf, 0, sizeof(conf));
-       conf.direction = dir;
+       conf.direction = ep93xx_dma_data_to_trans_dir(dir);
 
-       if (dir == DMA_DEV_TO_MEM) {
+       if (dir == DMA_FROM_DEVICE) {
                chan = espi->dma_rx;
                buf = xfer->rx_buf;
                sgt = &espi->rx_sgt;

drivers/spi/spi-ep93xx.c +8 -8

+                                     DMA_CTRL_ACK);
        if (!txd) {
                dma_unmap_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
                return ERR_PTR(-ENOMEM);

@@ -360,13 +375,13 @@ ep93xx_spi_dma_prepare(struct spi_master *master,
  * unmapped.
  */
 static void ep93xx_spi_dma_finish(struct spi_master *master,
-                                 enum dma_transfer_direction dir)
+                                 enum dma_data_direction dir)
 {
        struct ep93xx_spi *espi = spi_master_get_devdata(master);
        struct dma_chan *chan;
        struct sg_table *sgt;
 
-       if (dir == DMA_DEV_TO_MEM) {
+       if (dir == DMA_FROM_DEVICE) {
                chan = espi->dma_rx;
                sgt = &espi->rx_sgt;
        } else {

@@ -381,8 +396,8 @@ static void ep93xx_spi_dma_callback(void *callback_param)
 {
        struct spi_master *master = callback_param;
 
-       ep93xx_spi_dma_finish(master, DMA_MEM_TO_DEV);
-       ep93xx_spi_dma_finish(master, DMA_DEV_TO_MEM);
+       ep93xx_spi_dma_finish(master, DMA_TO_DEVICE);
+       ep93xx_spi_dma_finish(master, DMA_FROM_DEVICE);
 
        spi_finalize_current_transfer(master);
 }

@@ -392,15 +407,15 @@ static int ep93xx_spi_dma_transfer(struct spi_master *master)
        struct ep93xx_spi *espi = spi_master_get_devdata(master);
        struct dma_async_tx_descriptor *rxd, *txd;
 
-       rxd = ep93xx_spi_dma_prepare(master, DMA_DEV_TO_MEM);
+       rxd = ep93xx_spi_dma_prepare(master, DMA_FROM_DEVICE);
        if (IS_ERR(rxd)) {
                dev_err(&master->dev, "DMA RX failed: %ld\n", PTR_ERR(rxd));
                return PTR_ERR(rxd);
        }
 
-       txd = ep93xx_spi_dma_prepare(master, DMA_MEM_TO_DEV);
+       txd = ep93xx_spi_dma_prepare(master, DMA_TO_DEVICE);
        if (IS_ERR(txd)) {
-               ep93xx_spi_dma_finish(master, DMA_DEV_TO_MEM);
+               ep93xx_spi_dma_finish(master, DMA_FROM_DEVICE);
                dev_err(&master->dev, "DMA TX failed: %ld\n", PTR_ERR(txd));
                return PTR_ERR(txd);
        }
--

2.19.0

Nathan,
Thanks for your patience and continued support working towards an optimal solution to fix this warning. Some thoughts below.

Quoted 113 lines · Nathan Chancellor

Clang warns when one enumerated type is implicitly converted to another.

drivers/spi/spi-ep93xx.c:342:62: warning: implicit conversion from enumeration type 'enum dma_transfer_direction' to different enumeration type 'enum dma_data_direction' [-Wenum-conversion]

        nents = dma_map_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:428:58: note: expanded from macro
'dma_map_sg'
#define dma_map_sg(d, s, n, r) dma_map_sg_attrs(d, s, n, r, 0)
                               ~~~~~~~~~~~~~~~~          ^
drivers/spi/spi-ep93xx.c:348:57: warning: implicit conversion from
enumeration type 'enum dma_transfer_direction' to different enumeration
type 'enum dma_data_direction' [-Wenum-conversion]
                dma_unmap_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:429:62: note: expanded from macro
'dma_unmap_sg'
#define dma_unmap_sg(d, s, n, r) dma_unmap_sg_attrs(d, s, n, r, 0)
                                 ~~~~~~~~~~~~~~~~~~          ^
drivers/spi/spi-ep93xx.c:377:56: warning: implicit conversion from
enumeration type 'enum dma_transfer_direction' to different enumeration
type 'enum dma_data_direction' [-Wenum-conversion]
        dma_unmap_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:429:62: note: expanded from macro
'dma_unmap_sg'
#define dma_unmap_sg(d, s, n, r) dma_unmap_sg_attrs(d, s, n, r, 0)
                                 ~~~~~~~~~~~~~~~~~~          ^

3 warnings generated.

dma_{,un}map_sg expect an enum of type dma_data_direction but this driver uses dma_transfer_direction for everything. Convert the driver to use dma_data_direction for these two functions.

There are two places that strictly require an enum of type
dma_transfer_direction: the direction member in struct dma_slave_config and the direction parameter in dmaengine_prep_slave_sg. To avoid using an explicit cast, add a simple function, ep93xx_dma_data_to_trans_dir, to safely map between the two types because they are not 1 to 1 in meaning.

Signed-off-by Nathan Chancellor <[email protected]>

---

v1 -> v2:

* Fix escaped hash symbols for '#define' lines (\# -> #)

v2 -> v3:

* Instead of changing the dir parameter in the prepare and finish functions, convert the driver to use dma_data_direction for everywhere that makes sense. Add a helper function to convert between the two types as there is still a couple of locations that need a transfer enum (and no such function exists). Alternatively, the function could just convert from transfer -> data and be used in dma_{,un}map_sg and the rest of the driver be left alone. Hopefully I understood what Mika wanted!

 drivers/spi/spi-ep93xx.c | 37 ++++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)

drivers/spi/spi-ep93xx.c +16 -3

index f1526757aaf6..667da7964d63 100644
@@ -246,6 +246,19 @@ static int ep93xx_spi_read_write(struct spi_master *master)
        return -EINPROGRESS;
 }
 
+static enum dma_transfer_direction
+ep93xx_dma_data_to_trans_dir(enum dma_data_direction dir)
+{
+       switch (dir) {
+       case DMA_TO_DEVICE:
+               return DMA_MEM_TO_DEV;
+       case DMA_FROM_DEVICE:
+               return DMA_DEV_TO_MEM;
+       default:
+               return DMA_TRANS_NONE;
+       }
+}
+
 /**
  * ep93xx_spi_dma_prepare() - prepares a DMA transfer
  * @master: SPI master

@@ -257,7 +270,7 @@ static int ep93xx_spi_read_write(struct spi_master *master)
  */
 static struct dma_async_tx_descriptor *
 ep93xx_spi_dma_prepare(struct spi_master *master,
-                      enum dma_transfer_direction dir)
+                      enum dma_data_direction dir)
 {
        struct ep93xx_spi *espi = spi_master_get_devdata(master);
        struct spi_transfer *xfer = master->cur_msg->state;

@@ -277,9 +290,9 @@ ep93xx_spi_dma_prepare(struct spi_master *master,
                buswidth = DMA_SLAVE_BUSWIDTH_1_BYTE;
 
        memset(&conf, 0, sizeof(conf));
-       conf.direction = dir;
+       conf.direction = ep93xx_dma_data_to_trans_dir(dir);
 
-       if (dir == DMA_DEV_TO_MEM) {
+       if (dir == DMA_FROM_DEVICE) {
                chan = espi->dma_rx;
                buf = xfer->rx_buf;
                sgt = &espi->rx_sgt;

@@ -343,7 +356,9 @@ ep93xx_spi_dma_prepare(struct spi_master *master,
        if (!nents)
                return ERR_PTR(-ENOMEM);
 

drivers/spi/spi-ep93xx.c ep93xx_spi_dma_prepare(struct spi_master *master, · line 360

Quoted 3 lines

drivers/spi/spi-ep93xx.c +2 -1

-       txd = dmaengine_prep_slave_sg(chan, sgt->sgl, nents, dir, DMA_CTRL_ACK);
+       txd = dmaengine_prep_slave_sg(chan, sgt->sgl, nents,
+                                     ep93xx_dma_data_to_trans_dir(dir),

So we've already calculated this once already in this function, and the value of dir hasn't changed since. While the compiler might be able to optimize out this common sub expression (CSE or GVN I think are the compiler passes that could optimize this out), I think it would be better to guarantee that this precalculated value is always used by replacing the rematerialization of the value (ep93xx_dma_data_to_trans_dir(dir)) with conf.direction.

Quoted 53 lines

drivers/spi/spi-ep93xx.c +8 -8

+                                     DMA_CTRL_ACK);
        if (!txd) {
                dma_unmap_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
                return ERR_PTR(-ENOMEM);

@@ -360,13 +375,13 @@ ep93xx_spi_dma_prepare(struct spi_master *master,
  * unmapped.
  */
 static void ep93xx_spi_dma_finish(struct spi_master *master,
-                                 enum dma_transfer_direction dir)
+                                 enum dma_data_direction dir)
 {
        struct ep93xx_spi *espi = spi_master_get_devdata(master);
        struct dma_chan *chan;
        struct sg_table *sgt;
 
-       if (dir == DMA_DEV_TO_MEM) {
+       if (dir == DMA_FROM_DEVICE) {
                chan = espi->dma_rx;
                sgt = &espi->rx_sgt;
        } else {

@@ -381,8 +396,8 @@ static void ep93xx_spi_dma_callback(void *callback_param)
 {
        struct spi_master *master = callback_param;
 
-       ep93xx_spi_dma_finish(master, DMA_MEM_TO_DEV);
-       ep93xx_spi_dma_finish(master, DMA_DEV_TO_MEM);
+       ep93xx_spi_dma_finish(master, DMA_TO_DEVICE);
+       ep93xx_spi_dma_finish(master, DMA_FROM_DEVICE);
 
        spi_finalize_current_transfer(master);
 }

@@ -392,15 +407,15 @@ static int ep93xx_spi_dma_transfer(struct spi_master *master)
        struct ep93xx_spi *espi = spi_master_get_devdata(master);
        struct dma_async_tx_descriptor *rxd, *txd;
 
-       rxd = ep93xx_spi_dma_prepare(master, DMA_DEV_TO_MEM);
+       rxd = ep93xx_spi_dma_prepare(master, DMA_FROM_DEVICE);
        if (IS_ERR(rxd)) {
                dev_err(&master->dev, "DMA RX failed: %ld\n", PTR_ERR(rxd));
                return PTR_ERR(rxd);
        }
 
-       txd = ep93xx_spi_dma_prepare(master, DMA_MEM_TO_DEV);
+       txd = ep93xx_spi_dma_prepare(master, DMA_TO_DEVICE);
        if (IS_ERR(txd)) {
-               ep93xx_spi_dma_finish(master, DMA_DEV_TO_MEM);
+               ep93xx_spi_dma_finish(master, DMA_FROM_DEVICE);
                dev_err(&master->dev, "DMA TX failed: %ld\n", PTR_ERR(txd));
                return PTR_ERR(txd);
        }
--

2.19.0

Signature · 2 lines

Thanks,
~Nick Desaulniers

Clang warns when one enumerated type is implicitly converted to another.

drivers/spi/spi-ep93xx.c:342:62: warning: implicit conversion from enumeration type 'enum dma_transfer_direction' to different enumeration type 'enum dma_data_direction' [-Wenum-conversion]

        nents = dma_map_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:428:58: note: expanded from macro
'dma_map_sg'
#define dma_map_sg(d, s, n, r) dma_map_sg_attrs(d, s, n, r, 0)
                               ~~~~~~~~~~~~~~~~          ^
drivers/spi/spi-ep93xx.c:348:57: warning: implicit conversion from
enumeration type 'enum dma_transfer_direction' to different enumeration
type 'enum dma_data_direction' [-Wenum-conversion]
                dma_unmap_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:429:62: note: expanded from macro
'dma_unmap_sg'
#define dma_unmap_sg(d, s, n, r) dma_unmap_sg_attrs(d, s, n, r, 0)
                                 ~~~~~~~~~~~~~~~~~~          ^
drivers/spi/spi-ep93xx.c:377:56: warning: implicit conversion from
enumeration type 'enum dma_transfer_direction' to different enumeration
type 'enum dma_data_direction' [-Wenum-conversion]
        dma_unmap_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:429:62: note: expanded from macro
'dma_unmap_sg'
#define dma_unmap_sg(d, s, n, r) dma_unmap_sg_attrs(d, s, n, r, 0)
                                 ~~~~~~~~~~~~~~~~~~          ^

3 warnings generated.

dma_{,un}map_sg expect an enum of type dma_data_direction but this driver uses dma_transfer_direction for everything. Convert the driver to use dma_data_direction for these two functions.

There are two places that strictly require an enum of type
dma_transfer_direction: the direction member in struct dma_slave_config and the direction parameter in dmaengine_prep_slave_sg. To avoid using an explicit cast, add a simple function, ep93xx_dma_data_to_trans_dir, to safely map between the two types because they are not 1 to 1 in meaning.

Signed-off-by Nathan Chancellor <[email protected]>

Clang warns when one enumerated type is implicitly converted to another.

drivers/spi/spi-ep93xx.c:342:62: warning: implicit conversion from enumeration type 'enum dma_transfer_direction' to different enumeration type 'enum dma_data_direction' [-Wenum-conversion]

        nents = dma_map_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:428:58: note: expanded from macro
'dma_map_sg'
#define dma_map_sg(d, s, n, r) dma_map_sg_attrs(d, s, n, r, 0)
                               ~~~~~~~~~~~~~~~~          ^
drivers/spi/spi-ep93xx.c:348:57: warning: implicit conversion from
enumeration type 'enum dma_transfer_direction' to different enumeration
type 'enum dma_data_direction' [-Wenum-conversion]
                dma_unmap_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:429:62: note: expanded from macro
'dma_unmap_sg'
#define dma_unmap_sg(d, s, n, r) dma_unmap_sg_attrs(d, s, n, r, 0)
                                 ~~~~~~~~~~~~~~~~~~          ^
drivers/spi/spi-ep93xx.c:377:56: warning: implicit conversion from
enumeration type 'enum dma_transfer_direction' to different enumeration
type 'enum dma_data_direction' [-Wenum-conversion]
        dma_unmap_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:429:62: note: expanded from macro
'dma_unmap_sg'
#define dma_unmap_sg(d, s, n, r) dma_unmap_sg_attrs(d, s, n, r, 0)
                                 ~~~~~~~~~~~~~~~~~~          ^

3 warnings generated.

dma_{,un}map_sg expect an enum of type dma_data_direction but this driver uses dma_transfer_direction for everything. Convert the driver to use dma_data_direction for these two functions.

There are two places that strictly require an enum of type
dma_transfer_direction: the direction member in struct dma_slave_config and the direction parameter in dmaengine_prep_slave_sg. To avoid using an explicit cast, add a simple function, ep93xx_dma_data_to_trans_dir, to safely map between the two types because they are not 1 to 1 in meaning.

Signed-off-by Nathan Chancellor <[email protected]>

---

v1 -> v2:

* Fix escaped hash symbols for '#define' lines (\# -> #).

v2 -> v3:

* Instead of changing the dir parameter in the prepare and finish functions, convert the driver to use dma_data_direction for everywhere that makes sense, thanks to review from Mika. Add a helper function to convert between the two types as there is still a couple of locations that need a transfer enum (and no such function exists).

v3 -> v4:

* Use 'conf.direction' in dmaengine_prep_slave_sg instead of the new function since the value was already set earlier in the function and never changed, thanks to review from Nick.

 drivers/spi/spi-ep93xx.c | 36 +++++++++++++++++++++++++-----------
 1 file changed, 25 insertions(+), 11 deletions(-)

drivers/spi/spi-ep93xx.c +25 -12

index f1526757aaf6..79fc3940245a 100644
@@ -246,6 +246,19 @@ static int ep93xx_spi_read_write(struct spi_master *master)
        return -EINPROGRESS;
 }
 
+static enum dma_transfer_direction
+ep93xx_dma_data_to_trans_dir(enum dma_data_direction dir)
+{
+       switch (dir) {
+       case DMA_TO_DEVICE:
+               return DMA_MEM_TO_DEV;
+       case DMA_FROM_DEVICE:
+               return DMA_DEV_TO_MEM;
+       default:
+               return DMA_TRANS_NONE;
+       }
+}
+
 /**
  * ep93xx_spi_dma_prepare() - prepares a DMA transfer
  * @master: SPI master

@@ -257,7 +270,7 @@ static int ep93xx_spi_read_write(struct spi_master *master)
  */
 static struct dma_async_tx_descriptor *
 ep93xx_spi_dma_prepare(struct spi_master *master,
-                      enum dma_transfer_direction dir)
+                      enum dma_data_direction dir)
 {
        struct ep93xx_spi *espi = spi_master_get_devdata(master);
        struct spi_transfer *xfer = master->cur_msg->state;

@@ -277,9 +290,9 @@ ep93xx_spi_dma_prepare(struct spi_master *master,
                buswidth = DMA_SLAVE_BUSWIDTH_1_BYTE;
 
        memset(&conf, 0, sizeof(conf));
-       conf.direction = dir;
+       conf.direction = ep93xx_dma_data_to_trans_dir(dir);
 
-       if (dir == DMA_DEV_TO_MEM) {
+       if (dir == DMA_FROM_DEVICE) {
                chan = espi->dma_rx;
                buf = xfer->rx_buf;
                sgt = &espi->rx_sgt;

@@ -343,7 +356,8 @@ ep93xx_spi_dma_prepare(struct spi_master *master,
        if (!nents)
                return ERR_PTR(-ENOMEM);
 
-       txd = dmaengine_prep_slave_sg(chan, sgt->sgl, nents, dir, DMA_CTRL_ACK);
+       txd = dmaengine_prep_slave_sg(chan, sgt->sgl, nents, conf.direction,
+                                     DMA_CTRL_ACK);
        if (!txd) {
                dma_unmap_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
                return ERR_PTR(-ENOMEM);

@@ -360,13 +374,13 @@ ep93xx_spi_dma_prepare(struct spi_master *master,
  * unmapped.
  */
 static void ep93xx_spi_dma_finish(struct spi_master *master,
-                                 enum dma_transfer_direction dir)
+                                 enum dma_data_direction dir)
 {
        struct ep93xx_spi *espi = spi_master_get_devdata(master);
        struct dma_chan *chan;
        struct sg_table *sgt;
 
-       if (dir == DMA_DEV_TO_MEM) {
+       if (dir == DMA_FROM_DEVICE) {
                chan = espi->dma_rx;
                sgt = &espi->rx_sgt;
        } else {

@@ -381,8 +395,8 @@ static void ep93xx_spi_dma_callback(void *callback_param)
 {
        struct spi_master *master = callback_param;
 
-       ep93xx_spi_dma_finish(master, DMA_MEM_TO_DEV);
-       ep93xx_spi_dma_finish(master, DMA_DEV_TO_MEM);
+       ep93xx_spi_dma_finish(master, DMA_TO_DEVICE);
+       ep93xx_spi_dma_finish(master, DMA_FROM_DEVICE);
 
        spi_finalize_current_transfer(master);
 }

@@ -392,15 +406,15 @@ static int ep93xx_spi_dma_transfer(struct spi_master *master)
        struct ep93xx_spi *espi = spi_master_get_devdata(master);
        struct dma_async_tx_descriptor *rxd, *txd;
 
-       rxd = ep93xx_spi_dma_prepare(master, DMA_DEV_TO_MEM);
+       rxd = ep93xx_spi_dma_prepare(master, DMA_FROM_DEVICE);
        if (IS_ERR(rxd)) {
                dev_err(&master->dev, "DMA RX failed: %ld\n", PTR_ERR(rxd));
                return PTR_ERR(rxd);
        }
 
-       txd = ep93xx_spi_dma_prepare(master, DMA_MEM_TO_DEV);
+       txd = ep93xx_spi_dma_prepare(master, DMA_TO_DEVICE);
        if (IS_ERR(txd)) {
-               ep93xx_spi_dma_finish(master, DMA_DEV_TO_MEM);
+               ep93xx_spi_dma_finish(master, DMA_FROM_DEVICE);
                dev_err(&master->dev, "DMA TX failed: %ld\n", PTR_ERR(txd));
                return PTR_ERR(txd);
        }
--

2.19.0

Clang warns when one enumerated type is implicitly converted to another.

drivers/spi/spi-ep93xx.c:342:62: warning: implicit conversion from enumeration type 'enum dma_transfer_direction' to different enumeration type 'enum dma_data_direction' [-Wenum-conversion]

        nents = dma_map_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:428:58: note: expanded from macro
'dma_map_sg'
#define dma_map_sg(d, s, n, r) dma_map_sg_attrs(d, s, n, r, 0)
                               ~~~~~~~~~~~~~~~~          ^
drivers/spi/spi-ep93xx.c:348:57: warning: implicit conversion from
enumeration type 'enum dma_transfer_direction' to different enumeration
type 'enum dma_data_direction' [-Wenum-conversion]
                dma_unmap_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:429:62: note: expanded from macro
'dma_unmap_sg'
#define dma_unmap_sg(d, s, n, r) dma_unmap_sg_attrs(d, s, n, r, 0)
                                 ~~~~~~~~~~~~~~~~~~          ^
drivers/spi/spi-ep93xx.c:377:56: warning: implicit conversion from
enumeration type 'enum dma_transfer_direction' to different enumeration
type 'enum dma_data_direction' [-Wenum-conversion]
        dma_unmap_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:429:62: note: expanded from macro
'dma_unmap_sg'
#define dma_unmap_sg(d, s, n, r) dma_unmap_sg_attrs(d, s, n, r, 0)
                                 ~~~~~~~~~~~~~~~~~~          ^

3 warnings generated.

dma_{,un}map_sg expect an enum of type dma_data_direction but this driver uses dma_transfer_direction for everything. Convert the driver to use dma_data_direction for these two functions.

There are two places that strictly require an enum of type
dma_transfer_direction: the direction member in struct dma_slave_config and the direction parameter in dmaengine_prep_slave_sg. To avoid using an explicit cast, add a simple function, ep93xx_dma_data_to_trans_dir, to safely map between the two types because they are not 1 to 1 in meaning.

Signed-off-by Nathan Chancellor <[email protected]>

From a1108c7b2efb892350ba6a0e932dfd45622f4e2b Mon Sep 17 00:00:00 2001

Clang warns when one enumerated type is implicitly converted to another.

drivers/spi/spi-ep93xx.c:342:62: warning: implicit conversion from enumeration type 'enum dma_transfer_direction' to different enumeration

type 'enum dma_data_direction' [-Wenum-conversion]

        nents = dma_map_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:428:58: note: expanded from macro
'dma_map_sg'
\#define dma_map_sg(d, s, n, r) dma_map_sg_attrs(d, s, n, r, 0)
                               ~~~~~~~~~~~~~~~~          ^
drivers/spi/spi-ep93xx.c:348:57: warning: implicit conversion from
enumeration type 'enum dma_transfer_direction' to different enumeration
type 'enum dma_data_direction' [-Wenum-conversion]
                dma_unmap_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:429:62: note: expanded from macro
'dma_unmap_sg'
\#define dma_unmap_sg(d, s, n, r) dma_unmap_sg_attrs(d, s, n, r, 0)
                                 ~~~~~~~~~~~~~~~~~~          ^
drivers/spi/spi-ep93xx.c:377:56: warning: implicit conversion from
enumeration type 'enum dma_transfer_direction' to different enumeration
type 'enum dma_data_direction' [-Wenum-conversion]
        dma_unmap_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:429:62: note: expanded from macro
'dma_unmap_sg'
\#define dma_unmap_sg(d, s, n, r) dma_unmap_sg_attrs(d, s, n, r, 0)
                                 ~~~~~~~~~~~~~~~~~~          ^

3 warnings generated.

dma_{,un}map_sg expects an enum of type dma_data_direction but this driver uses dma_transfer_direction for everything. Converting to dma_data_direction would be desirable but there are a few shared structures that expect dma_transfer_direction so it is just simpler to change the parameter here. dma_transfer_direction and dma_data_direction are different sizes but this driver only uses the 1 and 2 values which mean the same thing so this change is safe.

Signed-off-by Nathan Chancellor <[email protected]>

---
 drivers/spi/spi-ep93xx.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

drivers/spi/spi-ep93xx.c +2 -4

index f1526757aaf6..189fc2225b69 100644
@@ -256,8 +256,7 @@ static int ep93xx_spi_read_write(struct spi_master *master)
  * in case of failure.
  */
 static struct dma_async_tx_descriptor *
-ep93xx_spi_dma_prepare(struct spi_master *master,
-		       enum dma_transfer_direction dir)
+ep93xx_spi_dma_prepare(struct spi_master *master, int dir)
 {
 	struct ep93xx_spi *espi = spi_master_get_devdata(master);
 	struct spi_transfer *xfer = master->cur_msg->state;

@@ -359,8 +358,7 @@ ep93xx_spi_dma_prepare(struct spi_master *master,
  * Function finishes with the DMA transfer. After this, the DMA buffer is
  * unmapped.
  */
-static void ep93xx_spi_dma_finish(struct spi_master *master,
-				  enum dma_transfer_direction dir)
+static void ep93xx_spi_dma_finish(struct spi_master *master, int dir)
 {
 	struct ep93xx_spi *espi = spi_master_get_devdata(master);
 	struct dma_chan *chan;

Signature · 1 line

2.19.0

Quoted 4 lines · Nathan Chancellor

Clang warns when one enumerated type is implicitly converted to another.

drivers/spi/spi-ep93xx.c:342:62: warning: implicit conversion from enumeration type 'enum dma_transfer_direction' to different enumeration

Please remember to CC driver maintainers and authors on patch submissions so they can review things, copying in Hartley and Mika.

Quoted 67 lines

type 'enum dma_data_direction' [-Wenum-conversion]

        nents = dma_map_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:428:58: note: expanded from macro
'dma_map_sg'
\#define dma_map_sg(d, s, n, r) dma_map_sg_attrs(d, s, n, r, 0)
                               ~~~~~~~~~~~~~~~~          ^
drivers/spi/spi-ep93xx.c:348:57: warning: implicit conversion from
enumeration type 'enum dma_transfer_direction' to different enumeration
type 'enum dma_data_direction' [-Wenum-conversion]
                dma_unmap_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:429:62: note: expanded from macro
'dma_unmap_sg'
\#define dma_unmap_sg(d, s, n, r) dma_unmap_sg_attrs(d, s, n, r, 0)
                                 ~~~~~~~~~~~~~~~~~~          ^
drivers/spi/spi-ep93xx.c:377:56: warning: implicit conversion from
enumeration type 'enum dma_transfer_direction' to different enumeration
type 'enum dma_data_direction' [-Wenum-conversion]
        dma_unmap_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:429:62: note: expanded from macro
'dma_unmap_sg'
\#define dma_unmap_sg(d, s, n, r) dma_unmap_sg_attrs(d, s, n, r, 0)
                                 ~~~~~~~~~~~~~~~~~~          ^

3 warnings generated.

dma_{,un}map_sg expects an enum of type dma_data_direction but this driver uses dma_transfer_direction for everything. Converting to dma_data_direction would be desirable but there are a few shared structures that expect dma_transfer_direction so it is just simpler to change the parameter here. dma_transfer_direction and dma_data_direction are different sizes but this driver only uses the 1 and 2 values which mean the same thing so this change is safe.

Signed-off-by Nathan Chancellor <[email protected]>

---
 drivers/spi/spi-ep93xx.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

drivers/spi/spi-ep93xx.c +2 -4

index f1526757aaf6..189fc2225b69 100644
@@ -256,8 +256,7 @@ static int ep93xx_spi_read_write(struct spi_master *master)
  * in case of failure.
  */
 static struct dma_async_tx_descriptor *
-ep93xx_spi_dma_prepare(struct spi_master *master,
-		       enum dma_transfer_direction dir)
+ep93xx_spi_dma_prepare(struct spi_master *master, int dir)
 {
 	struct ep93xx_spi *espi = spi_master_get_devdata(master);
 	struct spi_transfer *xfer = master->cur_msg->state;

@@ -359,8 +358,7 @@ ep93xx_spi_dma_prepare(struct spi_master *master,
  * Function finishes with the DMA transfer. After this, the DMA buffer is
  * unmapped.
  */
-static void ep93xx_spi_dma_finish(struct spi_master *master,
-				  enum dma_transfer_direction dir)
+static void ep93xx_spi_dma_finish(struct spi_master *master, int dir)
 {
 	struct ep93xx_spi *espi = spi_master_get_devdata(master);
 	struct dma_chan *chan;

Signature · 1 line

2.19.0

Quoted 6 lines · Mark Brown

Quoted 4 lines · Nathan Chancellor

Clang warns when one enumerated type is implicitly converted to another.

drivers/spi/spi-ep93xx.c:342:62: warning: implicit conversion from enumeration type 'enum dma_transfer_direction' to different enumeration

Quoted 3 lines

Please remember to CC driver maintainers and authors on patch submissions so they can review things, copying in Hartley and Mika.

Hi Mark,

Thank you for that, I usually just rely on get_maintainer.pl but I'll be

better about looking at git history for major patch authors as well.

$ ./scripts/get_maintainer.pl drivers/spi/spi-ep93xx.c
Mark Brown <[email protected]> (maintainer:SPI SUBSYSTEM)
[email protected] (open list:SPI SUBSYSTEM)
[email protected] (open list)

Nathan

Quoted 67 lines

Quoted 67 lines

type 'enum dma_data_direction' [-Wenum-conversion]

        nents = dma_map_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:428:58: note: expanded from macro
'dma_map_sg'
\#define dma_map_sg(d, s, n, r) dma_map_sg_attrs(d, s, n, r, 0)
                               ~~~~~~~~~~~~~~~~          ^
drivers/spi/spi-ep93xx.c:348:57: warning: implicit conversion from
enumeration type 'enum dma_transfer_direction' to different enumeration
type 'enum dma_data_direction' [-Wenum-conversion]
                dma_unmap_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:429:62: note: expanded from macro
'dma_unmap_sg'
\#define dma_unmap_sg(d, s, n, r) dma_unmap_sg_attrs(d, s, n, r, 0)
                                 ~~~~~~~~~~~~~~~~~~          ^
drivers/spi/spi-ep93xx.c:377:56: warning: implicit conversion from
enumeration type 'enum dma_transfer_direction' to different enumeration
type 'enum dma_data_direction' [-Wenum-conversion]
        dma_unmap_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:429:62: note: expanded from macro
'dma_unmap_sg'
\#define dma_unmap_sg(d, s, n, r) dma_unmap_sg_attrs(d, s, n, r, 0)
                                 ~~~~~~~~~~~~~~~~~~          ^

3 warnings generated.

dma_{,un}map_sg expects an enum of type dma_data_direction but this driver uses dma_transfer_direction for everything. Converting to dma_data_direction would be desirable but there are a few shared structures that expect dma_transfer_direction so it is just simpler to change the parameter here. dma_transfer_direction and dma_data_direction are different sizes but this driver only uses the 1 and 2 values which mean the same thing so this change is safe.

Signed-off-by Nathan Chancellor <[email protected]>

---
 drivers/spi/spi-ep93xx.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

drivers/spi/spi-ep93xx.c +2 -5

index f1526757aaf6..189fc2225b69 100644
@@ -256,8 +256,7 @@ static int ep93xx_spi_read_write(struct spi_master *master)
  * in case of failure.
  */
 static struct dma_async_tx_descriptor *
-ep93xx_spi_dma_prepare(struct spi_master *master,
-                  enum dma_transfer_direction dir)
+ep93xx_spi_dma_prepare(struct spi_master *master, int dir)
 {
    struct ep93xx_spi *espi = spi_master_get_devdata(master);
    struct spi_transfer *xfer = master->cur_msg->state;

@@ -359,8 +358,7 @@ ep93xx_spi_dma_prepare(struct spi_master *master,
  * Function finishes with the DMA transfer. After this, the DMA buffer is
  * unmapped.
  */
-static void ep93xx_spi_dma_finish(struct spi_master *master,
-                             enum dma_transfer_direction dir)
+static void ep93xx_spi_dma_finish(struct spi_master *master, int dir)
 {
    struct ep93xx_spi *espi = spi_master_get_devdata(master);
    struct dma_chan *chan;
--

2.19.0

On Thu, Oct 4, 2018 at 9:04 AM Nathan Chancellor

Quoted 6 lines · Mark Brown

Quoted 3 lines

Please remember to CC driver maintainers and authors on patch submissions so they can review things, copying in Hartley and Mika.

Thank you for that, I usually just rely on get_maintainer.pl but I'll be

If get_maintainer.pl doesn't cc the right person, does that signal that the MAINTAINERS file has some omissions?

Quoted 4 lines · Nathan Chancellor

Clang warns when one enumerated type is implicitly converted to another.

drivers/spi/spi-ep93xx.c:342:62: warning: implicit conversion from enumeration type 'enum dma_transfer_direction' to different enumeration

Please remember to CC driver maintainers and authors on patch submissions so they can review things, copying in Hartley and Mika.

Quoted 27 lines

type 'enum dma_data_direction' [-Wenum-conversion]

        nents = dma_map_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:428:58: note: expanded from macro
'dma_map_sg'
\#define dma_map_sg(d, s, n, r) dma_map_sg_attrs(d, s, n, r, 0)
                               ~~~~~~~~~~~~~~~~          ^
drivers/spi/spi-ep93xx.c:348:57: warning: implicit conversion from
enumeration type 'enum dma_transfer_direction' to different enumeration
type 'enum dma_data_direction' [-Wenum-conversion]
                dma_unmap_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:429:62: note: expanded from macro
'dma_unmap_sg'
\#define dma_unmap_sg(d, s, n, r) dma_unmap_sg_attrs(d, s, n, r, 0)
                                 ~~~~~~~~~~~~~~~~~~          ^
drivers/spi/spi-ep93xx.c:377:56: warning: implicit conversion from
enumeration type 'enum dma_transfer_direction' to different enumeration
type 'enum dma_data_direction' [-Wenum-conversion]
        dma_unmap_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:429:62: note: expanded from macro
'dma_unmap_sg'
\#define dma_unmap_sg(d, s, n, r) dma_unmap_sg_attrs(d, s, n, r, 0)
                                 ~~~~~~~~~~~~~~~~~~          ^

3 warnings generated.

Quoted 7 lines

dma_{,un}map_sg expects an enum of type dma_data_direction but this driver uses dma_transfer_direction for everything. Converting to dma_data_direction would be desirable but there are a few shared structures that expect dma_transfer_direction so it is just simpler to change the parameter here. dma_transfer_direction and dma_data_direction are different sizes but this driver only uses the 1 and 2 values which mean the same thing so this change is safe.

Hi,

Quoted 8 lines · Mark Brown

Quoted 4 lines · Nathan Chancellor

Clang warns when one enumerated type is implicitly converted to another.

drivers/spi/spi-ep93xx.c:342:62: warning: implicit conversion from enumeration type 'enum dma_transfer_direction' to different enumeration

Please remember to CC driver maintainers and authors on patch submissions so they can review things, copying in Hartley and Mika.

Thanks for copying me.

Quoted 27 lines

Quoted 27 lines

type 'enum dma_data_direction' [-Wenum-conversion]

        nents = dma_map_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:428:58: note: expanded from macro
'dma_map_sg'
\#define dma_map_sg(d, s, n, r) dma_map_sg_attrs(d, s, n, r, 0)
                               ~~~~~~~~~~~~~~~~          ^
drivers/spi/spi-ep93xx.c:348:57: warning: implicit conversion from
enumeration type 'enum dma_transfer_direction' to different enumeration
type 'enum dma_data_direction' [-Wenum-conversion]
                dma_unmap_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:429:62: note: expanded from macro
'dma_unmap_sg'
\#define dma_unmap_sg(d, s, n, r) dma_unmap_sg_attrs(d, s, n, r, 0)
                                 ~~~~~~~~~~~~~~~~~~          ^
drivers/spi/spi-ep93xx.c:377:56: warning: implicit conversion from
enumeration type 'enum dma_transfer_direction' to different enumeration
type 'enum dma_data_direction' [-Wenum-conversion]
        dma_unmap_sg(chan->device->dev, sgt->sgl, sgt->nents, dir);
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./include/linux/dma-mapping.h:429:62: note: expanded from macro
'dma_unmap_sg'
\#define dma_unmap_sg(d, s, n, r) dma_unmap_sg_attrs(d, s, n, r, 0)
                                 ~~~~~~~~~~~~~~~~~~          ^

3 warnings generated.

Quoted 7 lines

Quoted 7 lines

dma_{,un}map_sg expects an enum of type dma_data_direction but this driver uses dma_transfer_direction for everything. Converting to dma_data_direction would be desirable but there are a few shared structures that expect dma_transfer_direction so it is just simpler to change the parameter here. dma_transfer_direction and dma_data_direction are different sizes but this driver only uses the 1 and 2 values which mean the same thing so this change is safe.

I would rather do the conversion than passing "int" to the function even if both enums happen to have same value now. I would be surprised if there is no helper function already for the conversion :)