Re: [Intel-gfx] [PATCH] gpu: Consistently use octal not symbolic permissions

2018-05-25 Thread Jani Nikula
On Thu, 24 May 2018, Joe Perches  wrote:
> On Fri, 2018-05-25 at 09:41 +0300, Jani Nikula wrote:
>> On Thu, 24 May 2018, Joe Perches  wrote:
>> > There is currently a mixture of octal and symbolic permissions uses
>> > in files in drivers/gpu/drm and one file in drivers/gpu.
>> > 
>> > There are ~270 existing octal uses and ~115 S_ uses.
>> > 
>> > Convert all the S_ symbolic permissions to their octal equivalents
>> > as using octal and not symbolic permissions is preferred by many as more
>> > readable.
>> > 
>> > see: https://lkml.org/lkml/2016/8/2/1945
>> > 
>> > Done with automated conversion via:
>> > $ ./scripts/checkpatch.pl -f --types=SYMBOLIC_PERMS --fix-inplace 
>> > 
>> > 
>> > Miscellanea:
>> > 
>> > o Wrapped modified multi-line calls to a single line where appropriate
>> > o Realign modified multi-line calls to open parenthesis
>> > o drivers/gpu/drm/msm/adreno/a5xx_debugfs.c has a world-writeable
>> >   debug permission for "reset" - perhaps that should be modified
>> > Signed-off-by: Joe Perches 
>> > ---
>> >  drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c|  2 +-
>> >  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 98 
>> > +++---
>> >  drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c   |  3 +-
>> >  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c|  9 +-
>> >  drivers/gpu/drm/armada/armada_debugfs.c|  4 +-
>> >  drivers/gpu/drm/drm_debugfs.c  |  6 +-
>> >  drivers/gpu/drm/drm_debugfs_crc.c  |  4 +-
>> >  drivers/gpu/drm/drm_sysfs.c|  2 +-
>> >  drivers/gpu/drm/i915/gvt/firmware.c|  2 +-
>> >  drivers/gpu/drm/i915/i915_debugfs.c|  8 +-
>> >  drivers/gpu/drm/i915/i915_perf.c   |  2 +-
>> >  drivers/gpu/drm/i915/i915_sysfs.c  | 22 ++---
>> >  drivers/gpu/drm/i915/intel_pipe_crc.c  |  2 +-
>> 
>> Please send at least i915 changes separately. There's zero reason to
>> make our lives harder for this change.
>
> The idea is to avoid unnecessary multiple patches for
> individual trees.

You're changing like a dozen trees. And apparently maintainers of
different trees also have different opinions on whether this is a good
idea or not. Mass changes like this go nowhere, and if they do, will
cause unnecessary conflicts that could have been trivially avoided.

BR,
Jani.

> But you could do that via something like:
>
> $ git am --include='drivers/gpu/drm/i915/*' 
>
> cheers, Joe
>

-- 
Jani Nikula, Intel Open Source Graphics Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] gpu: Consistently use octal not symbolic permissions

2018-05-25 Thread Joe Perches
On Fri, 2018-05-25 at 09:41 +0300, Jani Nikula wrote:
> On Thu, 24 May 2018, Joe Perches  wrote:
> > There is currently a mixture of octal and symbolic permissions uses
> > in files in drivers/gpu/drm and one file in drivers/gpu.
> > 
> > There are ~270 existing octal uses and ~115 S_ uses.
> > 
> > Convert all the S_ symbolic permissions to their octal equivalents
> > as using octal and not symbolic permissions is preferred by many as more
> > readable.
> > 
> > see: https://lkml.org/lkml/2016/8/2/1945
> > 
> > Done with automated conversion via:
> > $ ./scripts/checkpatch.pl -f --types=SYMBOLIC_PERMS --fix-inplace 
> > 
> > Miscellanea:
> > 
> > o Wrapped modified multi-line calls to a single line where appropriate
> > o Realign modified multi-line calls to open parenthesis
> > o drivers/gpu/drm/msm/adreno/a5xx_debugfs.c has a world-writeable
> >   debug permission for "reset" - perhaps that should be modified
> > Signed-off-by: Joe Perches 
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c|  2 +-
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 98 
> > +++---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c   |  3 +-
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c|  9 +-
> >  drivers/gpu/drm/armada/armada_debugfs.c|  4 +-
> >  drivers/gpu/drm/drm_debugfs.c  |  6 +-
> >  drivers/gpu/drm/drm_debugfs_crc.c  |  4 +-
> >  drivers/gpu/drm/drm_sysfs.c|  2 +-
> >  drivers/gpu/drm/i915/gvt/firmware.c|  2 +-
> >  drivers/gpu/drm/i915/i915_debugfs.c|  8 +-
> >  drivers/gpu/drm/i915/i915_perf.c   |  2 +-
> >  drivers/gpu/drm/i915/i915_sysfs.c  | 22 ++---
> >  drivers/gpu/drm/i915/intel_pipe_crc.c  |  2 +-
> 
> Please send at least i915 changes separately. There's zero reason to
> make our lives harder for this change.

The idea is to avoid unnecessary multiple patches for
individual trees.

But you could do that via something like:

$ git am --include='drivers/gpu/drm/i915/*' 

cheers, Joe

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] gpu: Consistently use octal not symbolic permissions

2018-05-25 Thread Christian König

Well I think we rejected that multiple times now.

At least I find the symbolic permissions easier to read and I absolutely 
don't see any reason why we should only use one form.


Christian.

Am 24.05.2018 um 22:22 schrieb Joe Perches:

There is currently a mixture of octal and symbolic permissions uses
in files in drivers/gpu/drm and one file in drivers/gpu.

There are ~270 existing octal uses and ~115 S_ uses.

Convert all the S_ symbolic permissions to their octal equivalents
as using octal and not symbolic permissions is preferred by many as more
readable.

see: https://lkml.org/lkml/2016/8/2/1945

Done with automated conversion via:
$ ./scripts/checkpatch.pl -f --types=SYMBOLIC_PERMS --fix-inplace 

Miscellanea:

o Wrapped modified multi-line calls to a single line where appropriate
o Realign modified multi-line calls to open parenthesis
o drivers/gpu/drm/msm/adreno/a5xx_debugfs.c has a world-writeable
   debug permission for "reset" - perhaps that should be modified

Signed-off-by: Joe Perches 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c|  2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 98 +++---
  drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c   |  3 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c|  9 +-
  drivers/gpu/drm/armada/armada_debugfs.c|  4 +-
  drivers/gpu/drm/drm_debugfs.c  |  6 +-
  drivers/gpu/drm/drm_debugfs_crc.c  |  4 +-
  drivers/gpu/drm/drm_sysfs.c|  2 +-
  drivers/gpu/drm/i915/gvt/firmware.c|  2 +-
  drivers/gpu/drm/i915/i915_debugfs.c|  8 +-
  drivers/gpu/drm/i915/i915_perf.c   |  2 +-
  drivers/gpu/drm/i915/i915_sysfs.c  | 22 ++---
  drivers/gpu/drm/i915/intel_pipe_crc.c  |  2 +-
  drivers/gpu/drm/msm/adreno/a5xx_debugfs.c  |  5 +-
  drivers/gpu/drm/msm/msm_perf.c |  4 +-
  drivers/gpu/drm/msm/msm_rd.c   |  4 +-
  drivers/gpu/drm/nouveau/nouveau_debugfs.c  |  2 +-
  drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c| 11 ++-
  .../drm/omapdrm/displays/panel-sony-acx565akm.c|  6 +-
  .../drm/omapdrm/displays/panel-tpo-td043mtea1.c| 10 +--
  drivers/gpu/drm/radeon/radeon_pm.c | 26 +++---
  drivers/gpu/drm/radeon/radeon_ttm.c|  4 +-
  drivers/gpu/drm/sti/sti_drv.c  |  2 +-
  drivers/gpu/drm/tinydrm/mipi-dbi.c |  4 +-
  drivers/gpu/drm/ttm/ttm_bo.c   |  2 +-
  drivers/gpu/drm/ttm/ttm_memory.c   | 12 +--
  drivers/gpu/drm/ttm/ttm_page_alloc.c   |  6 +-
  drivers/gpu/drm/ttm/ttm_page_alloc_dma.c   |  6 +-
  drivers/gpu/drm/udl/udl_fb.c   |  4 +-
  drivers/gpu/host1x/debug.c | 12 +--
  30 files changed, 138 insertions(+), 146 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index f5fb93795a69..7b29febff511 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -830,7 +830,7 @@ int amdgpu_debugfs_regs_init(struct amdgpu_device *adev)
  
  	for (i = 0; i < ARRAY_SIZE(debugfs_regs); i++) {

ent = debugfs_create_file(debugfs_regs_names[i],
- S_IFREG | S_IRUGO, root,
+ S_IFREG | 0444, root,
  adev, debugfs_regs[i]);
if (IS_ERR(ent)) {
for (j = 0; j < i; j++) {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index b455da487782..fa55d7e9e784 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -905,39 +905,39 @@ static ssize_t amdgpu_set_pp_power_profile_mode(struct 
device *dev,
return -EINVAL;
  }
  
-static DEVICE_ATTR(power_dpm_state, S_IRUGO | S_IWUSR, amdgpu_get_dpm_state, amdgpu_set_dpm_state);

-static DEVICE_ATTR(power_dpm_force_performance_level, S_IRUGO | S_IWUSR,
+static DEVICE_ATTR(power_dpm_state, 0644, amdgpu_get_dpm_state, 
amdgpu_set_dpm_state);
+static DEVICE_ATTR(power_dpm_force_performance_level, 0644,
   amdgpu_get_dpm_forced_performance_level,
   amdgpu_set_dpm_forced_performance_level);
-static DEVICE_ATTR(pp_num_states, S_IRUGO, amdgpu_get_pp_num_states, NULL);
-static DEVICE_ATTR(pp_cur_state, S_IRUGO, amdgpu_get_pp_cur_state, NULL);
-static DEVICE_ATTR(pp_force_state, S_IRUGO | S_IWUSR,
-   amdgpu_get_pp_force_state,
-   amdgpu_set_pp_force_state);
-static DEVICE_ATTR(pp_table, S_IRUGO | S_IWUSR,
-   amdgpu_get_pp_table,
-   amdgpu_set_pp_table);
-static DEVICE_ATTR(pp_dpm_sclk, S_IRUGO | S_IWUSR,
-   amdgpu_get_pp_dpm_sclk,
- 

Re: [Intel-gfx] [PATCH] gpu: Consistently use octal not symbolic permissions

2018-05-25 Thread Jani Nikula
On Thu, 24 May 2018, Joe Perches  wrote:
> There is currently a mixture of octal and symbolic permissions uses
> in files in drivers/gpu/drm and one file in drivers/gpu.
>
> There are ~270 existing octal uses and ~115 S_ uses.
>
> Convert all the S_ symbolic permissions to their octal equivalents
> as using octal and not symbolic permissions is preferred by many as more
> readable.
>
> see: https://lkml.org/lkml/2016/8/2/1945
>
> Done with automated conversion via:
> $ ./scripts/checkpatch.pl -f --types=SYMBOLIC_PERMS --fix-inplace 
>
> Miscellanea:
>
> o Wrapped modified multi-line calls to a single line where appropriate
> o Realign modified multi-line calls to open parenthesis
> o drivers/gpu/drm/msm/adreno/a5xx_debugfs.c has a world-writeable
>   debug permission for "reset" - perhaps that should be modified
>
> Signed-off-by: Joe Perches 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c|  2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 98 
> +++---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c   |  3 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c|  9 +-
>  drivers/gpu/drm/armada/armada_debugfs.c|  4 +-
>  drivers/gpu/drm/drm_debugfs.c  |  6 +-
>  drivers/gpu/drm/drm_debugfs_crc.c  |  4 +-
>  drivers/gpu/drm/drm_sysfs.c|  2 +-
>  drivers/gpu/drm/i915/gvt/firmware.c|  2 +-
>  drivers/gpu/drm/i915/i915_debugfs.c|  8 +-
>  drivers/gpu/drm/i915/i915_perf.c   |  2 +-
>  drivers/gpu/drm/i915/i915_sysfs.c  | 22 ++---
>  drivers/gpu/drm/i915/intel_pipe_crc.c  |  2 +-

Please send at least i915 changes separately. There's zero reason to
make our lives harder for this change.

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Graphics Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] gpu: Consistently use octal not symbolic permissions

2018-05-24 Thread Joe Perches
There is currently a mixture of octal and symbolic permissions uses
in files in drivers/gpu/drm and one file in drivers/gpu.

There are ~270 existing octal uses and ~115 S_ uses.

Convert all the S_ symbolic permissions to their octal equivalents
as using octal and not symbolic permissions is preferred by many as more
readable.

see: https://lkml.org/lkml/2016/8/2/1945

Done with automated conversion via:
$ ./scripts/checkpatch.pl -f --types=SYMBOLIC_PERMS --fix-inplace 

Miscellanea:

o Wrapped modified multi-line calls to a single line where appropriate
o Realign modified multi-line calls to open parenthesis
o drivers/gpu/drm/msm/adreno/a5xx_debugfs.c has a world-writeable
  debug permission for "reset" - perhaps that should be modified

Signed-off-by: Joe Perches 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c|  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 98 +++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c   |  3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c|  9 +-
 drivers/gpu/drm/armada/armada_debugfs.c|  4 +-
 drivers/gpu/drm/drm_debugfs.c  |  6 +-
 drivers/gpu/drm/drm_debugfs_crc.c  |  4 +-
 drivers/gpu/drm/drm_sysfs.c|  2 +-
 drivers/gpu/drm/i915/gvt/firmware.c|  2 +-
 drivers/gpu/drm/i915/i915_debugfs.c|  8 +-
 drivers/gpu/drm/i915/i915_perf.c   |  2 +-
 drivers/gpu/drm/i915/i915_sysfs.c  | 22 ++---
 drivers/gpu/drm/i915/intel_pipe_crc.c  |  2 +-
 drivers/gpu/drm/msm/adreno/a5xx_debugfs.c  |  5 +-
 drivers/gpu/drm/msm/msm_perf.c |  4 +-
 drivers/gpu/drm/msm/msm_rd.c   |  4 +-
 drivers/gpu/drm/nouveau/nouveau_debugfs.c  |  2 +-
 drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c| 11 ++-
 .../drm/omapdrm/displays/panel-sony-acx565akm.c|  6 +-
 .../drm/omapdrm/displays/panel-tpo-td043mtea1.c| 10 +--
 drivers/gpu/drm/radeon/radeon_pm.c | 26 +++---
 drivers/gpu/drm/radeon/radeon_ttm.c|  4 +-
 drivers/gpu/drm/sti/sti_drv.c  |  2 +-
 drivers/gpu/drm/tinydrm/mipi-dbi.c |  4 +-
 drivers/gpu/drm/ttm/ttm_bo.c   |  2 +-
 drivers/gpu/drm/ttm/ttm_memory.c   | 12 +--
 drivers/gpu/drm/ttm/ttm_page_alloc.c   |  6 +-
 drivers/gpu/drm/ttm/ttm_page_alloc_dma.c   |  6 +-
 drivers/gpu/drm/udl/udl_fb.c   |  4 +-
 drivers/gpu/host1x/debug.c | 12 +--
 30 files changed, 138 insertions(+), 146 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index f5fb93795a69..7b29febff511 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -830,7 +830,7 @@ int amdgpu_debugfs_regs_init(struct amdgpu_device *adev)
 
for (i = 0; i < ARRAY_SIZE(debugfs_regs); i++) {
ent = debugfs_create_file(debugfs_regs_names[i],
- S_IFREG | S_IRUGO, root,
+ S_IFREG | 0444, root,
  adev, debugfs_regs[i]);
if (IS_ERR(ent)) {
for (j = 0; j < i; j++) {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index b455da487782..fa55d7e9e784 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -905,39 +905,39 @@ static ssize_t amdgpu_set_pp_power_profile_mode(struct 
device *dev,
return -EINVAL;
 }
 
-static DEVICE_ATTR(power_dpm_state, S_IRUGO | S_IWUSR, amdgpu_get_dpm_state, 
amdgpu_set_dpm_state);
-static DEVICE_ATTR(power_dpm_force_performance_level, S_IRUGO | S_IWUSR,
+static DEVICE_ATTR(power_dpm_state, 0644, amdgpu_get_dpm_state, 
amdgpu_set_dpm_state);
+static DEVICE_ATTR(power_dpm_force_performance_level, 0644,
   amdgpu_get_dpm_forced_performance_level,
   amdgpu_set_dpm_forced_performance_level);
-static DEVICE_ATTR(pp_num_states, S_IRUGO, amdgpu_get_pp_num_states, NULL);
-static DEVICE_ATTR(pp_cur_state, S_IRUGO, amdgpu_get_pp_cur_state, NULL);
-static DEVICE_ATTR(pp_force_state, S_IRUGO | S_IWUSR,
-   amdgpu_get_pp_force_state,
-   amdgpu_set_pp_force_state);
-static DEVICE_ATTR(pp_table, S_IRUGO | S_IWUSR,
-   amdgpu_get_pp_table,
-   amdgpu_set_pp_table);
-static DEVICE_ATTR(pp_dpm_sclk, S_IRUGO | S_IWUSR,
-   amdgpu_get_pp_dpm_sclk,
-   amdgpu_set_pp_dpm_sclk);
-static DEVICE_ATTR(pp_dpm_mclk, S_IRUGO | S_IWUSR,
-   amdgpu_get_pp_dpm_mclk,
-   amdgpu_set_pp_dpm_mclk);
-static DEVICE_ATTR(pp_dpm_pcie, S_IRUGO | S_IWUSR,
-   amdgpu_get_pp_dpm_pcie,
-