Re: [PATCH 3/3] DMA: PL330: Balance module remove function with probe

[Inderpal Singh]

On 27 September 2012 21:36, Jassi Brar <jassisinghbrar@xxxxxxxxx> wrote:
> On Thu, Sep 27, 2012 at 9:11 PM, Inderpal Singh
> <inderpal.singh@xxxxxxxxxx> wrote:
>> On 27 September 2012 15:18, Vinod Koul <vinod.koul@xxxxxxxxxxxxxxx> wrote:
>>> On Wed, 2012-09-26 at 12:11 +0530, Inderpal Singh wrote:
>>>> If we fail pl330_remove while some client is queued, the force unload
>>>> will fail and the
>>>> force unload will lose its purpose.
>>>> How about conditionally DMA_TERMINATE_ALL and free resources like
>>>> below ?
>>> Why would you want to remove the driver when it is doing something
>>> useful? You have to ensure driver is not doing anything.
>>>
>>> What is point here?
>>>
>> As mentioned by jassi,  if the pl330 module is forced unloaded while
>> some client is queued, we have to manually do DMA_TERMINATE_ALL.
>>
> I meant that in the current situation. Not ideally.
>
>> If failing remove is a better option in case some client is queued, we
>> can do away with DMA_TERMINATE_ALL and free_chan_resources and simply
>> return a suitable error code from remove.
>>
> That was exactly what I suggested as an alternative.

Yes, but our discussion went about continue doing DMA_TERMINATE_ALL and freeing.

Now, if we have to check if any client is using the channel and then
decide. We will have to traverse the channel list twice once to check
the usage and second time to delete the nodes from the list if we go
ahead with remove.
The remove will look like below:

@@ -3008,18 +3008,19 @@ static int __devexit pl330_remove(struct
amba_device *adev)
        if (!pdmac)
                return 0;

+       /* check if any client is using any channel */
+       list_for_each_entry_safe(pch, _p, &pdmac->ddma.channels,
+                       chan.device_node) {
+
+               if (pch->chan.client_count)
+                       return -EBUSY;
+       }
+
        amba_set_drvdata(adev, NULL);

-       /* Idle the DMAC */
        list_for_each_entry_safe(pch, _p, &pdmac->ddma.channels,
                        chan.device_node) {

                /* Remove the channel */
                list_del(&pch->chan.device_node);
-
-               /* Flush the channel */
-               pl330_control(&pch->chan, DMA_TERMINATE_ALL, 0);
-               pl330_free_chan_resources(&pch->chan);
        }

Please suggest if there is any better way to do it.

Thanks,
Inder
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/