* [PATCH] uacce: add print information if not enable sva @ 2021-06-04 7:46 Kai Ye 2021-06-04 7:51 ` Greg KH 2021-06-04 7:57 ` Joe Perches 0 siblings, 2 replies; 5+ messages in thread From: Kai Ye @ 2021-06-04 7:46 UTC (permalink / raw) To: gregkh, linux-accelerators, linux-kernel, linuxarm, zhangfei.gao, wangzhou1, yekai13 Add print information necessary if user not enable sva. Signed-off-by: Kai Ye <yekai13@huawei.com> --- 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); 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); iommu_dev_disable_feature(parent, IOMMU_DEV_FEAT_IOPF); return flags; } -- 2.7.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] uacce: add print information if not enable sva 2021-06-04 7:46 [PATCH] uacce: add print information if not enable sva Kai Ye @ 2021-06-04 7:51 ` Greg KH 2021-06-08 1:39 ` yekai(A) 2021-06-04 7:57 ` Joe Perches 1 sibling, 1 reply; 5+ messages in thread From: Greg KH @ 2021-06-04 7:51 UTC (permalink / raw) To: Kai Ye Cc: linux-accelerators, linux-kernel, linuxarm, zhangfei.gao, wangzhou1 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@huawei.com> > --- > 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] uacce: add print information if not enable sva 2021-06-04 7:51 ` Greg KH @ 2021-06-08 1:39 ` yekai(A) 2021-06-09 9:52 ` Greg KH 0 siblings, 1 reply; 5+ messages in thread From: yekai(A) @ 2021-06-08 1:39 UTC (permalink / raw) To: Greg KH Cc: linux-accelerators, linux-kernel, linuxarm, zhangfei.gao, wangzhou1 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@huawei.com> >> --- >> 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] uacce: add print information if not enable sva 2021-06-08 1:39 ` yekai(A) @ 2021-06-09 9:52 ` Greg KH 0 siblings, 0 replies; 5+ messages in thread From: Greg KH @ 2021-06-09 9:52 UTC (permalink / raw) To: yekai(A) Cc: linux-accelerators, linux-kernel, linuxarm, zhangfei.gao, wangzhou1 On Tue, Jun 08, 2021 at 09:39:52AM +0800, yekai(A) wrote: > > > 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@huawei.com> > > > --- > > > 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. Ok, can you resend with the changes that Joe suggested to make this even easier to use? thanks, greg k-h ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] uacce: add print information if not enable sva 2021-06-04 7:46 [PATCH] uacce: add print information if not enable sva Kai Ye 2021-06-04 7:51 ` Greg KH @ 2021-06-04 7:57 ` Joe Perches 1 sibling, 0 replies; 5+ messages in thread From: Joe Perches @ 2021-06-04 7:57 UTC (permalink / raw) To: Kai Ye, gregkh, linux-accelerators, linux-kernel, linuxarm, zhangfei.gao, wangzhou1 On Fri, 2021-06-04 at 15:46 +0800, Kai Ye wrote: > Add print information necessary if user not enable sva. [] > diff --git 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; [] > + ret = iommu_dev_enable_feature(parent, IOMMU_DEV_FEAT_IOPF); > + if (ret) { > + dev_err(parent, "failed to enable IOPF feature! ret = %d\n", ret); Perhaps use %pe, ERR_PTR(ret) to get descriptive error output > + dev_err(parent, "failed to enable SVA feature! ret = %d\n", ret); here too ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-06-09 9:52 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-06-04 7:46 [PATCH] uacce: add print information if not enable sva Kai Ye 2021-06-04 7:51 ` Greg KH 2021-06-08 1:39 ` yekai(A) 2021-06-09 9:52 ` Greg KH 2021-06-04 7:57 ` Joe Perches
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).