Hi, On 3 June 2018 at 23:14, Chee, Tien Fong <tien.fong.c...@intel.com> wrote: > On Fri, 2018-06-01 at 08:25 -0600, Simon Glass wrote: >> Hi, >> >> On 31 May 2018 at 02:08, <tien.fong.c...@intel.com> wrote: >> > >> > From: Tien Fong Chee <tien.fong.c...@intel.com> >> > >> > Update the dma_memcpy description on return argument for DMA330 >> > driver. >> > >> > Signed-off-by: Tien Fong Chee <tien.fong.c...@intel.com> >> > --- >> > include/dma.h | 4 ++-- >> > 1 file changed, 2 insertions(+), 2 deletions(-) >> > >> > diff --git a/include/dma.h b/include/dma.h >> > index 0a0c9dd..b825c1e 100644 >> > --- a/include/dma.h >> > +++ b/include/dma.h >> > @@ -79,8 +79,8 @@ int dma_get_device(u32 transfer_type, struct >> > udevice **devp); >> > * @dst - destination pointer >> > * @src - souce pointer >> > * @len - data length to be copied >> > - * @return - on successful transfer returns no of bytes >> > - transferred and on failure return error code. >> > + * @return - on successful transfer returns no of bytes or >> > zero(for DMA330) >> > + * transferred and on failure return error code. >> This is a public API so you cannot change it just for one device. You >> can change the API for everyone if you like. >> >> But why would it want to return 0? >> > Because we only able to check the DMA tranferring status, full transfer > or failed. I can return the len argument user set if full tranfer is > finished.
OK. My concern is that you have a comment saying that the function does something different for one device versus others. This is not the place for such a comment. Here you can just document that it can return two possible meanings. You should add comments explaining what 0 means too (e.g. completed, but length unknown?). For something in progress, you should use -EINPROGRESS / -EAGAIN Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
