diff mbox

[4/9] drm/vmwgfx: Revert "drm/vmwgfx: Replace numeric parameter like 0444 with macro"

Message ID 1490653079-45042-5-git-send-email-syeh@vmware.com (mailing list archive)
State New, archived
Headers show

Commit Message

Sinclair Yeh March 27, 2017, 10:17 p.m. UTC 
From: Øyvind A. Holm <sunny@sunbase.org>

This reverts commit 2d8e60e8b074 ("drm/vmwgfx: Replace numeric
parameter like 0444 with macro")

The commit belongs to the series of 1285 patches sent to LKML on
2016-08-02, it changes the representation of file permissions from the
octal value "0600" to "S_IRUSR | S_IWUSR".

The general consensus was that the changes does not increase
readability, quite the opposite; 0600 is easier to parse mentally than
S_IRUSR | S_IWUSR.

It also causes argument inconsistency, due to commit 04319d89fbec
("drm/vmwgfx: Add an option to change assumed FB bpp") that added
another call to module_param_named() where the permissions are written
as 0600.

Signed-off-by: Øyvind A. Holm <sunny@sunbase.org>
Reviewed-by: Sinclair Yeh <syeh@vmware.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Øyvind A. Holm April 2, 2017, 6:54 a.m. UTC | #1 
On 2017-03-27 15:17:54, Sinclair Yeh wrote:
> From: Øyvind A. Holm <sunny@sunbase.org>
>
> This reverts commit 2d8e60e8b074 ("drm/vmwgfx: Replace numeric
> parameter like 0444 with macro")
> [...]
> index 45d711e..d31dc34 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -241,13 +241,13 @@ static int vmwgfx_pm_notifier(struct notifier_block *nb, unsigned long val,
>  			      void *ptr);
>
>  MODULE_PARM_DESC(enable_fbdev, "Enable vmwgfx fbdev");
> -module_param_named(enable_fbdev, enable_fbdev, int, S_IRUSR | S_IWUSR);
> +module_param_named(enable_fbdev, enable_fbdev, int, 0600);
>  MODULE_PARM_DESC(force_dma_api, "Force using the DMA API for TTM pages");
> -module_param_named(force_dma_api, vmw_force_iommu, int, S_IRUSR | S_IWUSR);
> +module_param_named(force_dma_api, vmw_force_iommu, int, 0600);
>  MODULE_PARM_DESC(restrict_iommu, "Try to limit IOMMU usage for TTM pages");
> -module_param_named(restrict_iommu, vmw_restrict_iommu, int, S_IRUSR | S_IWUSR);
> +module_param_named(restrict_iommu, vmw_restrict_iommu, int, 0600);
>  MODULE_PARM_DESC(force_coherent, "Force coherent TTM pages");
> -module_param_named(force_coherent, vmw_force_coherent, int, S_IRUSR | S_IWUSR);
> +module_param_named(force_coherent, vmw_force_coherent, int, 0600);
>  MODULE_PARM_DESC(restrict_dma_mask, "Restrict DMA mask to 44 bits with IOMMU");
>  module_param_named(restrict_dma_mask, vmw_restrict_dma_mask, int, S_IRUSR | S_IWUSR);
>  MODULE_PARM_DESC(assume_16bpp, "Assume 16-bpp when filtering modes");
> --
> 2.7.4

It seems as something has happened to the patch when applied, the last 
change is gone, it was in my original patch sent to the list. This 
change is missing:

  -module_param_named(restrict_dma_mask, vmw_restrict_dma_mask, int, S_IRUSR | S_IWUSR);
  +module_param_named(restrict_dma_mask, vmw_restrict_dma_mask, int, 0600);

I suppose it's to late to fix this exact commit (50f837371dd9) now 
because it's part of the request-pull mail sent 2017-03-31 16:32:55 
-0700. I can send a new patch created on top of commit 28c954299cd2 
("drm/vmwgfx: Properly check display/scanout surface size") that 
completes this patch, if it's ok.

Regards,
Øyvind

+-| Øyvind A. Holm <sunny@sunbase.org> - N 60.37604° E 5.33339° |-+
| OpenPGP: 0xFB0CBEE894A506E5 - http://www.sunbase.org/pubkey.asc |
| Fingerprint: A006 05D6 E676 B319 55E2  E77E FB0C BEE8 94A5 06E5 |
+------------| 32db7b80-176e-11e7-8de9-db5caa6d21d3 |-------------+
Sinclair Yeh April 3, 2017, 4:39 p.m. UTC | #2 
On Sun, Apr 02, 2017 at 08:54:14AM +0200, Øyvind A. Holm wrote:
> On 2017-03-27 15:17:54, Sinclair Yeh wrote:
> > From: Øyvind A. Holm <sunny@sunbase.org>
> >
> > This reverts commit 2d8e60e8b074 ("drm/vmwgfx: Replace numeric
> > parameter like 0444 with macro")
> > [...]
> > index 45d711e..d31dc34 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> > @@ -241,13 +241,13 @@ static int vmwgfx_pm_notifier(struct notifier_block *nb, unsigned long val,
> >  			      void *ptr);
> >
> >  MODULE_PARM_DESC(enable_fbdev, "Enable vmwgfx fbdev");
> > -module_param_named(enable_fbdev, enable_fbdev, int, S_IRUSR | S_IWUSR);
> > +module_param_named(enable_fbdev, enable_fbdev, int, 0600);
> >  MODULE_PARM_DESC(force_dma_api, "Force using the DMA API for TTM pages");
> > -module_param_named(force_dma_api, vmw_force_iommu, int, S_IRUSR | S_IWUSR);
> > +module_param_named(force_dma_api, vmw_force_iommu, int, 0600);
> >  MODULE_PARM_DESC(restrict_iommu, "Try to limit IOMMU usage for TTM pages");
> > -module_param_named(restrict_iommu, vmw_restrict_iommu, int, S_IRUSR | S_IWUSR);
> > +module_param_named(restrict_iommu, vmw_restrict_iommu, int, 0600);
> >  MODULE_PARM_DESC(force_coherent, "Force coherent TTM pages");
> > -module_param_named(force_coherent, vmw_force_coherent, int, S_IRUSR | S_IWUSR);
> > +module_param_named(force_coherent, vmw_force_coherent, int, 0600);
> >  MODULE_PARM_DESC(restrict_dma_mask, "Restrict DMA mask to 44 bits with IOMMU");
> >  module_param_named(restrict_dma_mask, vmw_restrict_dma_mask, int, S_IRUSR | S_IWUSR);
> >  MODULE_PARM_DESC(assume_16bpp, "Assume 16-bpp when filtering modes");
> > --
> > 2.7.4
> 
> It seems as something has happened to the patch when applied, the last 
> change is gone, it was in my original patch sent to the list. This 
> change is missing:
> 
>   -module_param_named(restrict_dma_mask, vmw_restrict_dma_mask, int, S_IRUSR | S_IWUSR);
>   +module_param_named(restrict_dma_mask, vmw_restrict_dma_mask, int, 0600);

Hmm... I'll put this as a new patch then.

thanks for letting me know!

Sinclair

> 
> I suppose it's to late to fix this exact commit (50f837371dd9) now 
> because it's part of the request-pull mail sent 2017-03-31 16:32:55 
> -0700. I can send a new patch created on top of commit 28c954299cd2 
> ("drm/vmwgfx: Properly check display/scanout surface size") that 
> completes this patch, if it's ok.
> 
> Regards,
> Øyvind
> 
> +-| Øyvind A. Holm <sunny@sunbase.org> - N 60.37604° E 5.33339° |-+
> | OpenPGP: 0xFB0CBEE894A506E5 - http://www.sunbase.org/pubkey.asc |
> | Fingerprint: A006 05D6 E676 B319 55E2  E77E FB0C BEE8 94A5 06E5 |
> +------------| 32db7b80-176e-11e7-8de9-db5caa6d21d3 |-------------+
diff mbox

Patch

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 45d711e..d31dc34 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -241,13 +241,13 @@  static int vmwgfx_pm_notifier(struct notifier_block *nb, unsigned long val,
 			      void *ptr);
 
 MODULE_PARM_DESC(enable_fbdev, "Enable vmwgfx fbdev");
-module_param_named(enable_fbdev, enable_fbdev, int, S_IRUSR | S_IWUSR);
+module_param_named(enable_fbdev, enable_fbdev, int, 0600);
 MODULE_PARM_DESC(force_dma_api, "Force using the DMA API for TTM pages");
-module_param_named(force_dma_api, vmw_force_iommu, int, S_IRUSR | S_IWUSR);
+module_param_named(force_dma_api, vmw_force_iommu, int, 0600);
 MODULE_PARM_DESC(restrict_iommu, "Try to limit IOMMU usage for TTM pages");
-module_param_named(restrict_iommu, vmw_restrict_iommu, int, S_IRUSR | S_IWUSR);
+module_param_named(restrict_iommu, vmw_restrict_iommu, int, 0600);
 MODULE_PARM_DESC(force_coherent, "Force coherent TTM pages");
-module_param_named(force_coherent, vmw_force_coherent, int, S_IRUSR | S_IWUSR);
+module_param_named(force_coherent, vmw_force_coherent, int, 0600);
 MODULE_PARM_DESC(restrict_dma_mask, "Restrict DMA mask to 44 bits with IOMMU");
 module_param_named(restrict_dma_mask, vmw_restrict_dma_mask, int, S_IRUSR | S_IWUSR);
 MODULE_PARM_DESC(assume_16bpp, "Assume 16-bpp when filtering modes");