On Tue, Feb 23, 2021 at 09:56:13PM +0530, Allen Pais wrote: > > > > > > > - /* > > > > > - * Ask OP-TEE to free all cached shared memory objects to decrease > > > > > - * reference counters and also avoid wild pointers in secure world > > > > > - * into the old shared memory range. > > > > > - */ > > > > > - optee_disable_shm_cache(optee); > > > > > + if (shutdown) { > > > > > + optee_disable_shm_cache(optee); > > > > > + } else { > > > > > + /* > > > > > + * Ask OP-TEE to free all cached shared memory > > > > > + * objects to decrease reference counters and > > > > > + * also avoid wild pointers in secure world > > > > > + * into the old shared memory range. > > > > > + */ > > > > > + optee_disable_shm_cache(optee); > > > > Calling optee_disable_shm_cache() in both if and else. It could be > > > > put in front of if(). > > > > > > > > > > Ideally, I could just use optee_remove for shutdown() too. > > > But it would not look good. Hence this approach. > > > > What is the problem with using optee_remove() for shutdown()? > > > > There is no problem, I just thought it would be more cleaner/readable > with this approach. If you'd like to keep it simple by just calling > optee_remove() for shutdown() too, I could quickly send out V2. In the patch you posted it looks like you'd like to call only optee_disable_shm_cache() in the case of shutdown. Like: static void optee_shutdown(struct platform_device *pdev) { optee_disable_shm_cache(platform_get_drvdata(pdev)); } and optee_remove() kept as it was before this patch. Cheers, Jens