[PATCH 1/4] mach-k3: common.c: Add dma device remove in spl exit

[Kumar, Udit]

Hi Prasanth,

On 10/6/2024 2:26 PM, Prasanth Mantena wrote:
> On 20:09-20241004, Kumar, Udit wrote:
> Hi Udit,
>> Hi Prasant,
>>
>> Thanks for series,
>>
>> Could we update the subject of patch something like
>>
>> Remove dma device in spl exit, Sorry but
>>
>> Add dma device remove in spl exit looks little confusing , are we adding or
>> removing ?
> Understood. Should've been more simpler like "Remove dma device in spl
> exit". Will update in next version.
>>
>> On 10/4/2024 6:50 PM, Prasanth Babu Mantena wrote:
>>> [..]
>>> Are you thinking to have DMA device optional ?
> Not really. Sorry, but didn't understand the question completely here.

+	rc = uclass_find_device(UCLASS_DMA, 0, &dev);
+	if (!rc && dev) {


I meant , here if there is some return code or device is not found then
at user level there is no notification.
May be adding some error print in case either some return code or device is NULL
will help to user know, that DMA was not cleaned properly.

>> Second question, if we enabled multiple DMAs in u-boot, then would you like
>> to remove all ?
> Considering this function is specific to k3, here we have channel
> allocation done in probe and there is a need to clean that channel
> resources which has been done in remove function in this series, call
> that remove function here. If there are such allocations done for other
> dma devices as well and have their remove functions implemented, then
> they should consider adding those as well here. I don't think there is
> any such case with other dmas.

Understood this is k3 specific , But here i meant multiple dma devices 
like udma, pdma etc.


>
>>
>>> +	if (!rc && dev) {
>>> +		rc = device_remove(dev, DM_REMOVE_NORMAL);
>>> +		if (rc)
>>> +			pr_warn("Cannot remove dma device '%s' (err=%d)\n",
>>> +				dev->name, rc);
>>> +	}
>>> +}
>>> +
>>>    void spl_board_prepare_for_boot(void)
>>>    {
>>> +#if !(defined(CONFIG_SYS_ICACHE_OFF) && defined(CONFIG_SYS_DCACHE_OFF))
>>>    	dcache_disable();
>>> +#endif
>> Please suggest , why we are protecting dcache_disable with flags ?
> This is carry forward which was already present. This patch removes
> the protection from whole spl_board_prepare_for_boot() function to
> dcache_disable() only. This alone looks like it has been added in this
> patch, but if we see the whole snippet, it can be understood well.


Thanks

>
> Thanks,
> Prasanth
>>
>>> +#if IS_ENABLED(CONFIG_SPL_DMA) && IS_ENABLED(CONFIG_SPL_DM_DEVICE_REMOVE)
>>> +	k3_dma_remove();
>>> +#endif
>>>    }
>>> +#if !(defined(CONFIG_SYS_ICACHE_OFF) && defined(CONFIG_SYS_DCACHE_OFF))
>>>    void spl_board_prepare_for_linux(void)
>>>    {
>>>    	dcache_disable();