On 10/16/2021 12:35 PM, Dan Carpenter wrote:
On Fri, Oct 15, 2021 at 12:34:20PM -0700, Jessica Zhang wrote:
Hey Dmitry,

On 10/15/2021 11:24 AM, Dmitry Baryshkov wrote:
On Fri, 15 Oct 2021 at 04:43, Jessica Zhang <jessz...@codeaurora.org> wrote:
Hey Dan,

On 10/1/2021 5:31 AM, Dan Carpenter wrote:
Hello Sean Paul,

The patch a6bcddbc2ee1: "drm/msm: dsi: Handle dual-channel for 6G as
well" from Jul 25, 2018, leads to the following
Smatch static checker warning:

        drivers/gpu/drm/msm/dsi/dsi_host.c:729 dsi_calc_clk_rate_6g()
        warn: wrong type for 'msm_host->esc_clk_rate' (should be 'ulong')

drivers/gpu/drm/msm/dsi/dsi_host.c
       721 int dsi_calc_clk_rate_6g(struct msm_dsi_host *msm_host, bool 
is_bonded_dsi)
       722 {
       723         if (!msm_host->mode) {
       724                 pr_err("%s: mode not set\n", __func__);
       725                 return -EINVAL;
       726         }
       727
       728         dsi_calc_pclk(msm_host, is_bonded_dsi);
--> 729         msm_host->esc_clk_rate = clk_get_rate(msm_host->esc_clk);
                   ^^^^^^^^^^^^^^^^^^^^^^
I don't know why Smatch is suddenly warning about ancient msm code, but
clock rates should be unsigned long.  (I don't remember why).

       730         return 0;
       731 }
I'm unable to recreate the warning with Smatch. After running
build_kernel_data.sh, I ran `<path to smatch>/smatch_scripts/kchecker
drivers/gpu/drm/msm/dsi/dsi_host.c` and got the following output:

CHECK scripts/mod/empty.c
CALL scripts/checksyscalls.sh
CALL scripts/atomic/check-atomics.sh
CHECK arch/arm64/kernel/vdso/vgettimeofday.c
CC drivers/gpu/drm/msm/dsi/dsi_host.o
CHECK drivers/gpu/drm/msm/dsi/dsi_host.c
drivers/gpu/drm/msm/dsi/dsi_host.c:2380 msm_dsi_host_power_on() warn:
missing error code 'ret'

Is there a specific .config you're using (that's not the default
mainline defconfig)? If so, can you please share it?
Are you running your checks with ARM32 or ARM64 in mind?
Note, esc_clk_rate is u32, while clk_get_rate()'s returns unsigned long.
It would make sense to change all three clocks rates in msm_dsi_host
struct (and several places where they are used) to unsigned long.
Thanks for the response. I'm aware of what's causing this issue and how to
fix it, but I want to also be able to recreate the warning locally with
Smatch.
No, sorry, I haven't published that check.  It's just something I have
locally.
Understood. It would be helpful for future warnings if the issue was reproducible using Smatch scripts available in the repo since we would be able to verify the fix locally.

Btw, I will be offline for the next two weeks...

Gotcha, thanks for the heads up! I will release a fix ASAP for you to ack.

Best,

Jessica Zhang

regards,
dan carpenter


Reply via email to