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

[Prasanth Mantena]

On 09:59, Kumar, Udit wrote:
> 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.

Will check and add an error notification here.

> 
> > > 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.

In general there will only one dma either udma(flash boot) or pdma(ethboot)
being used in spl at boot time to load the next stage images. There is
no case where multiple dmas work at same time atleast for now. If such
case comes in future and need a remove, it should be taken care.

Thanks,
Prasanth

> 
> 
> > 
> > > 
> > > > +	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();