Re: [PATCH] uacce: add print information if not enable sva

[yekai(A)]

On 2021/6/4 15:51, Greg KH wrote:
> On Fri, Jun 04, 2021 at 03:46:09PM +0800, Kai Ye wrote:
> > Add print information necessary if user not enable sva.
> >
> > Signed-off-by: Kai Ye <yekai13@xxxxxxxxxx>
> > ---
> >  drivers/misc/uacce/uacce.c | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/misc/uacce/uacce.c b/drivers/misc/uacce/uacce.c
> > index bae18ef0..fe38af8 100644
> > --- a/drivers/misc/uacce/uacce.c
> > +++ b/drivers/misc/uacce/uacce.c
> > @@ -387,15 +387,22 @@ static void uacce_release(struct device *dev)
> >
> >  static unsigned int uacce_enable_sva(struct device *parent, unsigned int flags)
> >  {
> > +	int ret;
> > +
> >  	if (!(flags & UACCE_DEV_SVA))
> >  		return flags;
> >
> >  	flags &= ~UACCE_DEV_SVA;
> >
> > -	if (iommu_dev_enable_feature(parent, IOMMU_DEV_FEAT_IOPF))
> > +	ret = iommu_dev_enable_feature(parent, IOMMU_DEV_FEAT_IOPF);
> > +	if (ret) {
> > +		dev_err(parent, "failed to enable IOPF feature! ret = %d\n", ret);
>
> Why is this needed?  Has this ever happened in real life such that the
> log message is now required?
>
>
>
> >  		return flags;
> > +	}
> >
> > -	if (iommu_dev_enable_feature(parent, IOMMU_DEV_FEAT_SVA)) {
> > +	ret = iommu_dev_enable_feature(parent, IOMMU_DEV_FEAT_SVA);
> > +	if (ret) {
> > +		dev_err(parent, "failed to enable SVA feature! ret = %d\n", ret);
>
> Same here, does this happen in real systems?
>
> thanks,
>
> greg k-h
> .
In a other debug version, the SVA feature failed to be enabled, and no 
related information was printed. we don't know the cause of the problem. 
Finally, Although, it's not the problem here. but we find that the sva 
enable function doesn't have debug information. Therefore, it is 
inconvenient to locate the fault.
so i think the log message is required.

thanks

Kai