Re: [PATCH 7/7] ASoC: ti: omap-mcbsp: Simplify lock and resource handling

[Bui Duc Phuc]

Hi Mark,

>> This function now mixes scoped and goto based cleanup which cleanup.h
>> warns us not to do - are you sure this is a good idea?

If you'd prefer to remove all goto jumps from the function,
I can prepare an additional patch for that as well.

-------------------------------------------------

--- a/sound/soc/ti/omap-mcbsp.c
+++ b/sound/soc/ti/omap-mcbsp.c
@@ -322,42 +322,40 @@ static int omap_mcbsp_request(struct omap_mcbsp *mcbsp)
                                  "McBSP", (void *)mcbsp);
                if (err != 0) {
                        dev_err(mcbsp->dev, "Unable to request IRQ\n");
-                       goto err_clk_disable;
                }
        } else {
                err = request_irq(mcbsp->tx_irq, omap_mcbsp_tx_irq_handler, 0,
                                  "McBSP TX", (void *)mcbsp);
                if (err != 0) {
                        dev_err(mcbsp->dev, "Unable to request TX IRQ\n");
-                       goto err_clk_disable;
-               }
-
-               err = request_irq(mcbsp->rx_irq, omap_mcbsp_rx_irq_handler, 0,
-                                 "McBSP RX", (void *)mcbsp);
-               if (err != 0) {
-                       dev_err(mcbsp->dev, "Unable to request RX IRQ\n");
-                       goto err_free_irq;
+               } else {
+                       err = request_irq(mcbsp->rx_irq,
omap_mcbsp_rx_irq_handler, 0,
+                                         "McBSP RX", (void *)mcbsp);
+                       if (err != 0) {
+                               dev_err(mcbsp->dev, "Unable to request
RX IRQ\n");
+                               free_irq(mcbsp->tx_irq, (void *)mcbsp);
+                       }
                }
        }

-       return 0;
-err_free_irq:
-       free_irq(mcbsp->tx_irq, (void *)mcbsp);
-err_clk_disable:
-       if(mcbsp->pdata->ops && mcbsp->pdata->ops->free)
-               mcbsp->pdata->ops->free(mcbsp->id - 1);
+       if (err != 0) {
+               if(mcbsp->pdata->ops && mcbsp->pdata->ops->free)
+                       mcbsp->pdata->ops->free(mcbsp->id - 1);

-       /* Disable wakeup behavior */
-       if (mcbsp->pdata->has_wakeup)
-               MCBSP_WRITE(mcbsp, WAKEUPEN, 0);
+               /* Disable wakeup behavior */
+               if (mcbsp->pdata->has_wakeup)
+                       MCBSP_WRITE(mcbsp, WAKEUPEN, 0);

-       scoped_guard(spinlock, &mcbsp->lock) {
-               reg_cache = mcbsp->reg_cache;
-               mcbsp->free = true;
-               mcbsp->reg_cache = NULL;
+               scoped_guard(spinlock, &mcbsp->lock) {
+                       reg_cache = mcbsp->reg_cache;
+                       mcbsp->free = true;
+                       mcbsp->reg_cache = NULL;
+               }
+
+               return err;
        }

-       return err;
+       return 0;
 }

------------------------------------------------

Please let me know if this looks okay to you, and I'll prepare the patch.

Best regards,
Phuc