[bug report] drm/msm: dsi: Handle dual-channel for 6G as well

[Jessica Zhang]

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 <jesszhan at 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
>
>