Re: drm/amd/powerplay: add dpm force multiple levels on cz/tonga/fiji/polaris (v2)

From: Eric Huang <jinhuieric.huang@amd.com>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: dri-devel@lists.freedesktop.org
Subject: Re: drm/amd/powerplay: add dpm force multiple levels on cz/tonga/fiji/polaris (v2)
Date: Tue, 10 May 2016 13:40:17 -0400	[thread overview]
Message-ID: <57321D01.7040304@amd.com> (raw)
In-Reply-To: <20160510105121.GA11874@mwanda>

Hi Dan,

It is for avoiding the last char of newline. I will change it.

Thanks,
Eric

On 16-05-10 06:51 AM, Dan Carpenter wrote:
> Hello Eric Huang,
>
> The patch 5632708f4452: "drm/amd/powerplay: add dpm force multiple
> levels on cz/tonga/fiji/polaris (v2)" from Apr 12, 2016, leads to the
> following static checker warning:
>
> 	drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c:368 amdgpu_set_pp_dpm_sclk()
> 	warn: why is the last element skipped?
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
>     356  static ssize_t amdgpu_set_pp_dpm_sclk(struct device *dev,
>     357                  struct device_attribute *attr,
>     358                  const char *buf,
>     359                  size_t count)
>     360  {
>     361          struct drm_device *ddev = dev_get_drvdata(dev);
>     362          struct amdgpu_device *adev = ddev->dev_private;
>     363          int ret;
>     364          long level;
>     365          uint32_t i, mask = 0;
>     366          char sub_str[2];
>     367
>     368          for (i = 0; i < strlen(buf) - 1; i++) {
>
> strlen() returns the number of characters not counting the NUL
> terminator.  I guess this assumes that the last character is a newline?
> Could we check for that instead?
>
>     369                  sub_str[0] = *(buf + i);
>     370                  sub_str[1] = '\0';
>     371                  ret = kstrtol(sub_str, 0, &level);
>     372
>     373                  if (ret) {
>     374                          count = -EINVAL;
>     375                          goto fail;
>     376                  }
>     377                  mask |= 1 << level;
>     378          }
>     379
>     380          if (adev->pp_enabled)
>     381                  amdgpu_dpm_force_clock_level(adev, PP_SCLK, mask);
>     382  fail:
>     383          return count;
>     384  }
>
> There are a couple other:
> drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c:412 amdgpu_set_pp_dpm_mclk() warn: why is the last element skipped?
> drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c:456 amdgpu_set_pp_dpm_pcie() warn: why is the last element skipped?
>
> regards,
> dan carpenter

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel