Messages in this thread | | | Date | Wed, 11 Oct 2023 11:06:11 +0800 | Subject | Re: [PATCH 1/2] drm/tegra: Return an error code if fails | From | Sui Jingfeng <> |
| |
Hi,
On 2023/10/10 23:31, Thierry Reding wrote: > On Tue, Oct 10, 2023 at 03:22:56PM +0200, Thierry Reding wrote: >> On Mon, Jun 26, 2023 at 10:33:30PM +0800, Sui Jingfeng wrote: >>> Return -ENOMEM if tegra_bo_mmap() fails. >>> >>> Signed-off-by: Sui Jingfeng <suijingfeng@loongson.cn> >>> --- >>> drivers/gpu/drm/tegra/gem.c | 2 ++ >>> 1 file changed, 2 insertions(+) >> Sorry, this fell through the cracks. I think it'd be better if >> tegra_bo_mmap() were to be improved to always return either an ERR_PTR() >> encoded error code or a valid pointer. Throwing NULL into the mix isn't >> useful because it typically means something like -ENOMEM anyway. Error >> codes are more explicit, so since we're already using them for some >> cases, might as well return them for all. >> >> Actually, looks like tegra_bo_mmap() never actually returns an ERR_PTR() >> encoded error code. It's either obj->vaddr, the return value of vmap() >> (which is either NULL or the address of the mapping), or the address >> obtained from dma_buf_vmap_unlocked() (i.e. map.vaddr) or NULL on >> failure. So I think it would equally make sense to keep your patch and >> to remove the IS_ERR() check below it. >> >> I would slightly prefer the first option, but either is fine. > How about the attached patch?
I also prefer the prefer the first option. The attached patch is more better, because it solve the problem at lower level.
Reviewed-by: Sui Jingfeng <suijingfeng@loongson.cn>
> Thierry
| |