Skip to contentSkip to conversation

spi: rockchip: check requesting dma channel with EPROBE_DEFER

DISCUSSION Dan Carpenter 2 participants 3 messages Mar 14, 2016 1 message not in the archive

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

Hello Shawn Lin,

The patch 61cadcf46cfd: "spi: rockchip: check requesting dma channel with EPROBE_DEFER" from Mar 9, 2016, leads to the following static checker warning:

drivers/spi/spi-rockchip.c:742 rockchip_spi_probe()
warn: passing zero to 'PTR_ERR'

drivers/spi/spi-rockchip.c
732
733 rs->dma_tx.ch = dma_request_slave_channel(rs->dev, "tx");
734 if (IS_ERR_OR_NULL(rs->dma_tx.ch)) {
735 /* Check tx to see if we need defer probing driver */
736 if (PTR_ERR(rs->dma_tx.ch) == -EPROBE_DEFER) {
737 ret = -EPROBE_DEFER;
738 goto err_get_fifo_len;

What's going on here? Are we planning to change dma_request_slave_channel() to return error pointers? Also what about other error pointers besides EPROBE_DEFER it seems dangerous to leave rs->dma_tx.ch as an error pointer. We probably eventually try to free it if it's non-NULL.

739 }
740 dev_warn(rs->dev, "Failed to request TX DMA channel\n");
741 }
742

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#1 lore raw
Quoted 18 lines · Dan Carpenter

Hello Shawn Lin,

The patch 61cadcf46cfd: "spi: rockchip: check requesting dma channel with EPROBE_DEFER" from Mar 9, 2016, leads to the following static checker warning:

drivers/spi/spi-rockchip.c:742 rockchip_spi_probe()
warn: passing zero to 'PTR_ERR'

drivers/spi/spi-rockchip.c

    732
    733          rs->dma_tx.ch = dma_request_slave_channel(rs->dev, "tx");
    734          if (IS_ERR_OR_NULL(rs->dma_tx.ch)) {
    735                  /* Check tx to see if we need defer probing driver */
    736                  if (PTR_ERR(rs->dma_tx.ch) == -EPROBE_DEFER) {
    737                          ret = -EPROBE_DEFER;
    738                          goto err_get_fifo_len;
Quoted 4 lines

What's going on here? Are we planning to change dma_request_slave_channel() to return error pointers? Also what about other error pointers besides EPROBE_DEFER it seems dangerous to leave rs->dma_tx.ch as an error pointer. We probably eventually try to free it if it's non-NULL.

yes, we are plannig to return EPROBE_DEFER/NULL for dma_request_slave_channel to make sure we don't decide the dma cap based on driver probe sequence.

No any other error pointer will be returned to the caller

Quoted 11 lines
    739                  }
    740                  dev_warn(rs->dev, "Failed to request TX DMA channel\n");
    741          }
    742

regards,
dan carpenter

Signature · 7 lines
Best Regards
Shawn Lin

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): [email protected]

#3 lore raw
Quoted 24 lines · Shawn Lin
Quoted 18 lines · Dan Carpenter

Hello Shawn Lin,

The patch 61cadcf46cfd: "spi: rockchip: check requesting dma channel with EPROBE_DEFER" from Mar 9, 2016, leads to the following static checker warning:

drivers/spi/spi-rockchip.c:742 rockchip_spi_probe()
warn: passing zero to 'PTR_ERR'

drivers/spi/spi-rockchip.c
732

   733          rs->dma_tx.ch = dma_request_slave_channel(rs->dev, "tx");
   734          if (IS_ERR_OR_NULL(rs->dma_tx.ch)) {
   735                  /* Check tx to see if we need defer probing driver */
   736                  if (PTR_ERR(rs->dma_tx.ch) == -EPROBE_DEFER) {
   737                          ret = -EPROBE_DEFER;
   738                          goto err_get_fifo_len;
Quoted 4 lines

What's going on here? Are we planning to change dma_request_slave_channel() to return error pointers? Also what about other error pointers besides EPROBE_DEFER it seems dangerous to leave rs->dma_tx.ch as an error pointer. We probably eventually try to free it if it's non-NULL.

Quoted 5 lines

yes, we are plannig to return EPROBE_DEFER/NULL for dma_request_slave_channel to make sure we don't decide the dma cap based on driver probe sequence.

No any other error pointer will be returned to the caller

It would not be terribly shocking if a couple years from now someone else adds a new error return without auditing all the caller functions.

regards,
dan carpenter