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 <jesszhan@xxxxxxxxxxxxxx> 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.
Btw, I will be offline for the next two weeks...
regards,
dan carpenter
