| Message ID | 20221114205055.1547497-1-robdclark@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] drm/msm/a6xx: Fix speed-bin detection vs probe-defer | expand |
Hi, On Mon, Nov 14, 2022 at 12:50 PM Rob Clark <robdclark@gmail.com> wrote: > > From: Rob Clark <robdclark@chromium.org> > > If we get an error (other than -ENOENT) we need to propagate that up the > stack. Otherwise if the nvmem driver hasn't probed yet, we'll end up > end up claiming that we support all the OPPs which is not likely to be > true (and on some generations impossible to be true, ie. if there are > conflicting OPPs). > > v2: Update commit msg, gc unused label, etc > > Fixed: fe7952c629da ("drm/msm: Add speed-bin support to a618 gpu") > Signed-off-by: Rob Clark <robdclark@chromium.org> > --- > drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 10 ++++------ > 1 file changed, 4 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > index 7fe60c65a1eb..6ae77e88060f 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > @@ -1941,7 +1941,7 @@ static u32 fuse_to_supp_hw(struct device *dev, struct adreno_rev rev, u32 fuse) > > static int a6xx_set_supported_hw(struct device *dev, struct adreno_rev rev) > { > - u32 supp_hw = UINT_MAX; > + u32 supp_hw; > u32 speedbin; > int ret; > > @@ -1953,15 +1953,13 @@ static int a6xx_set_supported_hw(struct device *dev, struct adreno_rev rev) > if (ret == -ENOENT) { > return 0; > } else if (ret) { > - DRM_DEV_ERROR(dev, > - "failed to read speed-bin (%d). Some OPPs may not be supported by hardware", > - ret); > - goto done; > + dev_err_probe(dev, ret, > + "failed to read speed-bin. Some OPPs may not be supported by hardware"); > + return ret; Both before and after this change, I think you're missing a "\n" at the end of your error string? If you want to get even fancier, dev_err_probe is designed to run "braceless" and returns "ret" as its return value. This you could do: if (ret == -ENOENT) return ret; else if (ret) return dev_err_probe(dev, ret, ...) After adding the "\n" then either with the extra fanciness or as you have it: Reviewed-by: Douglas Anderson <dianders@chromium.org> -Doug
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c index 7fe60c65a1eb..6ae77e88060f 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c @@ -1941,7 +1941,7 @@ static u32 fuse_to_supp_hw(struct device *dev, struct adreno_rev rev, u32 fuse) static int a6xx_set_supported_hw(struct device *dev, struct adreno_rev rev) { - u32 supp_hw = UINT_MAX; + u32 supp_hw; u32 speedbin; int ret; @@ -1953,15 +1953,13 @@ static int a6xx_set_supported_hw(struct device *dev, struct adreno_rev rev) if (ret == -ENOENT) { return 0; } else if (ret) { - DRM_DEV_ERROR(dev, - "failed to read speed-bin (%d). Some OPPs may not be supported by hardware", - ret); - goto done; + dev_err_probe(dev, ret, + "failed to read speed-bin. Some OPPs may not be supported by hardware"); + return ret; } supp_hw = fuse_to_supp_hw(dev, rev, speedbin); -done: ret = devm_pm_opp_set_supported_hw(dev, &supp_hw, 1); if (ret) return ret;