[PATCH] drm/msm/disp: fix endian bug in debugfs code

[PATCH] drm/msm/disp: fix endian bug in debugfs code

[Dan Carpenter]

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);
 
 		debugfs_create_u32("xin_halt_timeout", 0400, debugfs_vbif,
 			(u32 *)&vbif->xin_halt_timeout);
-- 
2.20.1

Re: [PATCH] drm/msm/disp: fix endian bug in debugfs code

[Dmitry Baryshkov]

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);
> 


-- 
With best wishes
Dmitry

Re: [PATCH] drm/msm/disp: fix endian bug in debugfs code

[Dan Carpenter]

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

Re: [PATCH] drm/msm/disp: fix endian bug in debugfs code

[abhinavk]

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);