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 }
regards, dan carpenter
Hey Dan,
Thanks for the report, will take care of it.
On 2021-10-01 05:31, 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 }regards, dan carpenter
Thanks, Jessica Zhang
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?
Thanks,
Jessica Zhang
regards, dan carpenter
On Thu, Oct 14, 2021 at 06:43:22PM -0700, Jessica Zhang 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?
Oh, sorry. I never published this Smatch check. It generates 236 warnings and I'm not sure the rules here about where clk has to be unsigned long so I can't publish it... I think someone told me that it has to be unsigned long?
regards, dan carpenter
On 10/15/2021 1:12 AM, Dan Carpenter wrote:
On Thu, Oct 14, 2021 at 06:43:22PM -0700, Jessica Zhang 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?
Oh, sorry. I never published this Smatch check. It generates 236 warnings and I'm not sure the rules here about where clk has to be unsigned long so I can't publish it... I think someone told me that it has to be unsigned long?
Can you share which Smatch script (+ any command line options) you used to generate this warning? Just want to make sure I'm able to properly recreate this warning with Smatch on my end.
Thanks,
Jessica Zhang
regards, dan carpenter
On Fri, 15 Oct 2021 at 04:43, Jessica Zhang jesszhan@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,
Jessica Zhang
regards, dan carpenter
Hey Dmitry,
On 10/15/2021 11:24 AM, Dmitry Baryshkov wrote:
On Fri, 15 Oct 2021 at 04:43, Jessica Zhang jesszhan@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.
Thanks,
Jessica Zhang
Thanks,
Jessica Zhang
regards, dan carpenter
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@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.
Btw, I will be offline for the next two weeks...
regards, dan carpenter
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@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
dri-devel@lists.freedesktop.org
-
Dan Carpenter -
Dmitry Baryshkov -
Jessica Zhang -
Jessica Zhang -
jesszhan＠codeaurora.org