Am 04.04.2018 um 15:35 schrieb Tom St Denis: > Fold the read/write methods of the various register access > debugfs entries into op functions. > > Signed-off-by: Tom St Denis <tom.stdenis at amd.com> I would rather split out a dword helper. E.g. something like: amdgpu_debugfs_regs_dword_op(struct file *f, char __user *buf, size_t size, loff_t *pos, Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â int (*op)(struct amdgpu_device *adev, char __user *buf)) { ... } And then you have op callbacks for PCIE, didt, SMC each in a read/write variant. Would save quite a bunch more of loc if I'm not completely mistaken. Christian. > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 140 +++++++++++----------------- > 1 file changed, 57 insertions(+), 83 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c > index c98e59721444..b80e18469d25 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c > @@ -177,8 +177,8 @@ static ssize_t amdgpu_debugfs_regs_write(struct file *f, const char __user *buf, > return amdgpu_debugfs_process_reg_op(false, f, (char __user *)buf, size, pos); > } > > -static ssize_t amdgpu_debugfs_regs_pcie_read(struct file *f, char __user *buf, > - size_t size, loff_t *pos) > +static ssize_t amdgpu_debugfs_regs_pcie_op(bool read, struct file *f, > + char __user *buf, size_t size, loff_t *pos) > { > struct amdgpu_device *adev = file_inode(f)->i_private; > ssize_t result = 0; > @@ -190,11 +190,18 @@ static ssize_t amdgpu_debugfs_regs_pcie_read(struct file *f, char __user *buf, > while (size) { > uint32_t value; > > - value = RREG32_PCIE(*pos >> 2); > - r = put_user(value, (uint32_t *)buf); > - if (r) > - return r; > + if (read) { > + value = RREG32_PCIE(*pos >> 2); > + r = put_user(value, (uint32_t *)buf); > + if (r) > + return r; > + } else { > + r = get_user(value, (uint32_t *)buf); > + if (r) > + return r; > > + WREG32_PCIE(*pos >> 2, value); > + } > result += 4; > buf += 4; > *pos += 4; > @@ -204,8 +211,20 @@ static ssize_t amdgpu_debugfs_regs_pcie_read(struct file *f, char __user *buf, > return result; > } > > +static ssize_t amdgpu_debugfs_regs_pcie_read(struct file *f, char __user *buf, > + size_t size, loff_t *pos) > +{ > + return amdgpu_debugfs_regs_pcie_op(true, f, buf, size, pos); > +} > + > static ssize_t amdgpu_debugfs_regs_pcie_write(struct file *f, const char __user *buf, > size_t size, loff_t *pos) > +{ > + return amdgpu_debugfs_regs_pcie_op(false, f, (char __user *)buf, size, pos); > +} > + > +static ssize_t amdgpu_debugfs_regs_didt_op(bool read, struct file *f, > + char __user *buf, size_t size, loff_t *pos) > { > struct amdgpu_device *adev = file_inode(f)->i_private; > ssize_t result = 0; > @@ -217,12 +236,18 @@ static ssize_t amdgpu_debugfs_regs_pcie_write(struct file *f, const char __user > while (size) { > uint32_t value; > > - r = get_user(value, (uint32_t *)buf); > - if (r) > - return r; > - > - WREG32_PCIE(*pos >> 2, value); > + if (read) { > + value = RREG32_DIDT(*pos >> 2); > + r = put_user(value, (uint32_t *)buf); > + if (r) > + return r; > + } else { > + r = get_user(value, (uint32_t *)buf); > + if (r) > + return r; > > + WREG32_DIDT(*pos >> 2, value); > + } > result += 4; > buf += 4; > *pos += 4; > @@ -235,32 +260,18 @@ static ssize_t amdgpu_debugfs_regs_pcie_write(struct file *f, const char __user > static ssize_t amdgpu_debugfs_regs_didt_read(struct file *f, char __user *buf, > size_t size, loff_t *pos) > { > - struct amdgpu_device *adev = file_inode(f)->i_private; > - ssize_t result = 0; > - int r; > - > - if (size & 0x3 || *pos & 0x3) > - return -EINVAL; > - > - while (size) { > - uint32_t value; > - > - value = RREG32_DIDT(*pos >> 2); > - r = put_user(value, (uint32_t *)buf); > - if (r) > - return r; > - > - result += 4; > - buf += 4; > - *pos += 4; > - size -= 4; > - } > - > - return result; > + return amdgpu_debugfs_regs_didt_op(true, f, buf, size, pos); > } > > static ssize_t amdgpu_debugfs_regs_didt_write(struct file *f, const char __user *buf, > size_t size, loff_t *pos) > +{ > + return amdgpu_debugfs_regs_didt_op(false, f, (char __user *)buf, size, pos); > +} > + > +static ssize_t amdgpu_debugfs_regs_smc_op(bool read, > + struct file *f, char __user *buf, > + size_t size, loff_t *pos) > { > struct amdgpu_device *adev = file_inode(f)->i_private; > ssize_t result = 0; > @@ -272,11 +283,17 @@ static ssize_t amdgpu_debugfs_regs_didt_write(struct file *f, const char __user > while (size) { > uint32_t value; > > - r = get_user(value, (uint32_t *)buf); > - if (r) > - return r; > - > - WREG32_DIDT(*pos >> 2, value); > + if (read) { > + value = RREG32_SMC(*pos); > + r = put_user(value, (uint32_t *)buf); > + if (r) > + return r; > + } else { > + r = get_user(value, (uint32_t *)buf); > + if (r) > + return r; > + WREG32_SMC(*pos, value); > + } > > result += 4; > buf += 4; > @@ -290,56 +307,13 @@ static ssize_t amdgpu_debugfs_regs_didt_write(struct file *f, const char __user > static ssize_t amdgpu_debugfs_regs_smc_read(struct file *f, char __user *buf, > size_t size, loff_t *pos) > { > - struct amdgpu_device *adev = file_inode(f)->i_private; > - ssize_t result = 0; > - int r; > - > - if (size & 0x3 || *pos & 0x3) > - return -EINVAL; > - > - while (size) { > - uint32_t value; > - > - value = RREG32_SMC(*pos); > - r = put_user(value, (uint32_t *)buf); > - if (r) > - return r; > - > - result += 4; > - buf += 4; > - *pos += 4; > - size -= 4; > - } > - > - return result; > + return amdgpu_debugfs_regs_smc_op(true, f, buf, size, pos); > } > > static ssize_t amdgpu_debugfs_regs_smc_write(struct file *f, const char __user *buf, > size_t size, loff_t *pos) > { > - struct amdgpu_device *adev = file_inode(f)->i_private; > - ssize_t result = 0; > - int r; > - > - if (size & 0x3 || *pos & 0x3) > - return -EINVAL; > - > - while (size) { > - uint32_t value; > - > - r = get_user(value, (uint32_t *)buf); > - if (r) > - return r; > - > - WREG32_SMC(*pos, value); > - > - result += 4; > - buf += 4; > - *pos += 4; > - size -= 4; > - } > - > - return result; > + return amdgpu_debugfs_regs_smc_op(false, f, (char __user *)buf, size, pos); > } > > static ssize_t amdgpu_debugfs_gca_config_read(struct file *f, char __user *buf,