| Message ID | 20211004134721.GD11689@kili (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/msm/disp: fix endian bug in debugfs code | expand |
On 04/10/2021 16:47, Dan Carpenter wrote: > The "vbif->features" is type unsigned long but the debugfs file > is treating it as a u32 type. This will work in little endian > systems, but the correct thing is to change the debugfs to use > an unsigned long. > > Fixes: 25fdd5933e4c ("drm/msm: Add SDM845 DPU support") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > You might wonder why this code has so many casts. It's required because > this data is const. Which is fine because the file is read only. > > drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c > index 21d20373eb8b..e645a886e3c6 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c > @@ -305,8 +305,8 @@ void dpu_debugfs_vbif_init(struct dpu_kms *dpu_kms, struct dentry *debugfs_root) > > debugfs_vbif = debugfs_create_dir(vbif_name, entry); > > - debugfs_create_u32("features", 0600, debugfs_vbif, > - (u32 *)&vbif->features); > + debugfs_create_ulong("features", 0600, debugfs_vbif, > + (unsigned long *)&vbif->features); As you are converting this to the ulong file, could you please also remove the now-unnecessary type cast? > > debugfs_create_u32("xin_halt_timeout", 0400, debugfs_vbif, > (u32 *)&vbif->xin_halt_timeout); >
On 2021-10-04 06:47, Dan Carpenter wrote: > The "vbif->features" is type unsigned long but the debugfs file > is treating it as a u32 type. This will work in little endian > systems, but the correct thing is to change the debugfs to use > an unsigned long. > > Fixes: 25fdd5933e4c ("drm/msm: Add SDM845 DPU support") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> Reviewed-by: Abhinav Kumar <abhinavk@codeaurora.org> > --- > You might wonder why this code has so many casts. It's required > because > this data is const. Which is fine because the file is read only. > > drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c > index 21d20373eb8b..e645a886e3c6 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c > @@ -305,8 +305,8 @@ void dpu_debugfs_vbif_init(struct dpu_kms > *dpu_kms, struct dentry *debugfs_root) > > debugfs_vbif = debugfs_create_dir(vbif_name, entry); > > - debugfs_create_u32("features", 0600, debugfs_vbif, > - (u32 *)&vbif->features); > + debugfs_create_ulong("features", 0600, debugfs_vbif, > + (unsigned long *)&vbif->features); > > debugfs_create_u32("xin_halt_timeout", 0400, debugfs_vbif, > (u32 *)&vbif->xin_halt_timeout);
On Tue, Oct 05, 2021 at 02:31:12AM +0300, Dmitry Baryshkov wrote: > On 04/10/2021 16:47, Dan Carpenter wrote: > > The "vbif->features" is type unsigned long but the debugfs file > > is treating it as a u32 type. This will work in little endian > > systems, but the correct thing is to change the debugfs to use > > an unsigned long. > > > > Fixes: 25fdd5933e4c ("drm/msm: Add SDM845 DPU support") > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > --- > > You might wonder why this code has so many casts. It's required because > > this data is const. Which is fine because the file is read only. ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > > > drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c > > index 21d20373eb8b..e645a886e3c6 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c > > @@ -305,8 +305,8 @@ void dpu_debugfs_vbif_init(struct dpu_kms *dpu_kms, struct dentry *debugfs_root) > > debugfs_vbif = debugfs_create_dir(vbif_name, entry); > > - debugfs_create_u32("features", 0600, debugfs_vbif, > > - (u32 *)&vbif->features); > > + debugfs_create_ulong("features", 0600, debugfs_vbif, > > + (unsigned long *)&vbif->features); > > As you are converting this to the ulong file, could you please also remove > the now-unnecessary type cast? I wanted to remove all the casting but they are required because of the const. regards, dan carpenter
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c index 21d20373eb8b..e645a886e3c6 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c @@ -305,8 +305,8 @@ void dpu_debugfs_vbif_init(struct dpu_kms *dpu_kms, struct dentry *debugfs_root) debugfs_vbif = debugfs_create_dir(vbif_name, entry); - debugfs_create_u32("features", 0600, debugfs_vbif, - (u32 *)&vbif->features); + debugfs_create_ulong("features", 0600, debugfs_vbif, + (unsigned long *)&vbif->features); debugfs_create_u32("xin_halt_timeout", 0400, debugfs_vbif, (u32 *)&vbif->xin_halt_timeout);
The "vbif->features" is type unsigned long but the debugfs file is treating it as a u32 type. This will work in little endian systems, but the correct thing is to change the debugfs to use an unsigned long. Fixes: 25fdd5933e4c ("drm/msm: Add SDM845 DPU support") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- You might wonder why this code has so many casts. It's required because this data is const. Which is fine because the file is read only. drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)