| Message ID | 20240905223852.188355-1-minhuadotchen@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/rockchip: include rockchip_drm_drv.h | expand |
Hi Min Hua， Thanks for your work At 2024-09-06 06:38:50, "Min-Hua Chen" <minhuadotchen@gmail.com> wrote: >Include rockchip_drm_drv.h to fix the follow sparse warning: > >drivers/gpu/drm/rockchip/rockchip_vop2_reg.c:502:24: sparse: >warning: symbol 'vop2_platform_driver' was not declared. >Should it be static? > >No functional change intended. > >Signed-off-by: Min-Hua Chen <minhuadotchen@gmail.com> >--- > drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 1 + > 1 file changed, 1 insertion(+) > >diff --git a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c >index 18efb3fe1c00..c678d1b0fd7c 100644 >--- a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c >+++ b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c >@@ -14,6 +14,7 @@ > #include <drm/drm_print.h> > > #include "rockchip_drm_vop2.h" >+#include "rockchip_drm_drv.h" > We already have a patch[0] include rockchip_drm_drv.h in rockchip_drm_vop2.h [0]https://patchwork.kernel.org/project/linux-rockchip/patch/20240904120238.3856782-3-andyshrk@163.com/ > static const uint32_t formats_cluster[] = { > DRM_FORMAT_XRGB2101010, >-- >2.43.0 > > >_______________________________________________ >Linux-rockchip mailing list >Linux-rockchip@lists.infradead.org >http://lists.infradead.org/mailman/listinfo/linux-rockchip
Hi, >Hi Min Hua， > Thanks for your work > >At 2024-09-06 06:38:50, "Min-Hua Chen" <minhuadotchen@gmail.com> wrote: >>Include rockchip_drm_drv.h to fix the follow sparse warning: >> >>drivers/gpu/drm/rockchip/rockchip_vop2_reg.c:502:24: sparse: >>warning: symbol 'vop2_platform_driver' was not declared. >>Should it be static? >> >>No functional change intended. >> >>Signed-off-by: Min-Hua Chen <minhuadotchen@gmail.com> >>--- >> drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 1 + >> 1 file changed, 1 insertion(+) >> >>diff --git a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c >>index 18efb3fe1c00..c678d1b0fd7c 100644 >>--- a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c >>+++ b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c >>@@ -14,6 +14,7 @@ >> #include <drm/drm_print.h> >> >> #include "rockchip_drm_vop2.h" >>+#include "rockchip_drm_drv.h" >> > >We already have a patch[0] include rockchip_drm_drv.h in rockchip_drm_vop2.h > >[0]https://patchwork.kernel.org/project/linux-rockchip/patch/20240904120238.3856782-3-andyshrk@163.com/ that's great, thanks for letting me know. Min-Hua
On Fri Sep 6, 2024 at 2:42 AM CEST, Andy Yan wrote: > At 2024-09-06 06:38:50, "Min-Hua Chen" <minhuadotchen@gmail.com> wrote: > >Include rockchip_drm_drv.h to fix the follow sparse warning: > > > >drivers/gpu/drm/rockchip/rockchip_vop2_reg.c:502:24: sparse: > >warning: symbol 'vop2_platform_driver' was not declared. > >Should it be static? > > > >No functional change intended. > > > >Signed-off-by: Min-Hua Chen <minhuadotchen@gmail.com> > >--- > > drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 1 + > > 1 file changed, 1 insertion(+) > > > >diff --git a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c > >index 18efb3fe1c00..c678d1b0fd7c 100644 > >--- a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c > >+++ b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c > >@@ -14,6 +14,7 @@ > > #include <drm/drm_print.h> > > > > #include "rockchip_drm_vop2.h" > >+#include "rockchip_drm_drv.h" > > > > We already have a patch[0] include rockchip_drm_drv.h in rockchip_drm_vop2.h > > [0]https://patchwork.kernel.org/project/linux-rockchip/patch/20240904120238.3856782-3-andyshrk@163.com/ Maybe I'm missing something, but this patch seems to fix an already existing bug (which should have a Fixes tag?), which Andy also fixed while implementing a different (and unrelated) feature? Cheers, Diederik
Hi， At 2024-09-06 17:28:33, "Diederik de Haas" <didi.debian@cknow.org> wrote: >On Fri Sep 6, 2024 at 2:42 AM CEST, Andy Yan wrote: >> At 2024-09-06 06:38:50, "Min-Hua Chen" <minhuadotchen@gmail.com> wrote: >> >Include rockchip_drm_drv.h to fix the follow sparse warning: >> > >> >drivers/gpu/drm/rockchip/rockchip_vop2_reg.c:502:24: sparse: >> >warning: symbol 'vop2_platform_driver' was not declared. >> >Should it be static? >> > >> >No functional change intended. >> > >> >Signed-off-by: Min-Hua Chen <minhuadotchen@gmail.com> >> >--- >> > drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 1 + >> > 1 file changed, 1 insertion(+) >> > >> >diff --git a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c >> >index 18efb3fe1c00..c678d1b0fd7c 100644 >> >--- a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c >> >+++ b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c >> >@@ -14,6 +14,7 @@ >> > #include <drm/drm_print.h> >> > >> > #include "rockchip_drm_vop2.h" >> >+#include "rockchip_drm_drv.h" >> > >> >> We already have a patch[0] include rockchip_drm_drv.h in rockchip_drm_vop2.h >> >> [0]https://patchwork.kernel.org/project/linux-rockchip/patch/20240904120238.3856782-3-andyshrk@163.com/ > >Maybe I'm missing something, but this patch seems to fix an already >existing bug (which should have a Fixes tag?), which Andy also fixed >while implementing a different (and unrelated) feature? In fact, I don't know how to reproduce this compilation issue. While implementing my feature, I happened to find that I need to include rockchip_drm_drv.h in rockchip_drm_vop2.h > >Cheers, > Diederik
Hi, On Fri Sep 6, 2024 at 11:50 AM CEST, Andy Yan wrote: > At 2024-09-06 17:28:33, "Diederik de Haas" <didi.debian@cknow.org> wrote: > >On Fri Sep 6, 2024 at 2:42 AM CEST, Andy Yan wrote: > >> At 2024-09-06 06:38:50, "Min-Hua Chen" <minhuadotchen@gmail.com> wrote: > >> >Include rockchip_drm_drv.h to fix the follow sparse warning: > >> > > >> >drivers/gpu/drm/rockchip/rockchip_vop2_reg.c:502:24: sparse: > >> >warning: symbol 'vop2_platform_driver' was not declared. > >> >Should it be static? > >> > > >> >No functional change intended. > >> > > >> >Signed-off-by: Min-Hua Chen <minhuadotchen@gmail.com> > >> >--- > >> > drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 1 + > >> > 1 file changed, 1 insertion(+) > >> > > >> >diff --git a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c > >> >index 18efb3fe1c00..c678d1b0fd7c 100644 > >> >--- a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c > >> >+++ b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c > >> >@@ -14,6 +14,7 @@ > >> > #include <drm/drm_print.h> > >> > > >> > #include "rockchip_drm_vop2.h" > >> >+#include "rockchip_drm_drv.h" > >> > > >> > >> We already have a patch[0] include rockchip_drm_drv.h in rockchip_drm_vop2.h > >> > >> [0]https://patchwork.kernel.org/project/linux-rockchip/patch/20240904120238.3856782-3-andyshrk@163.com/ > > > >Maybe I'm missing something, but this patch seems to fix an already > >existing bug (which should have a Fixes tag?), which Andy also fixed > >while implementing a different (and unrelated) feature? > > In fact, I don't know how to reproduce this compilation issue. FWIW: I didn't see it either, but I assumed I was missing the right context (i.e. patches) needed to trigger that warning. > While implementing my feature, I happened to find that I need to > include rockchip_drm_drv.h in rockchip_drm_vop2.h Makes perfect sense :) But if the warning is indeed valid, it should be fixed on its own (IMO). Cheers, Diederik
>FWIW: I didn't see it either, but I assumed I was missing the right >context (i.e. patches) needed to trigger that warning. > FYI I triggered the warning by the following step: install 'sparse' first git clone https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git cd linux ARCH=arm64 LLVM=1 make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' mrproper defconfig all -j8 cheers, Min-Hua > >> While implementing my feature, I happened to find that I need to >> include rockchip_drm_drv.h in rockchip_drm_vop2.h > >Makes perfect sense :) >But if the warning is indeed valid, it should be fixed on its own (IMO). > >Cheers, > Diederik
On Sat Sep 7, 2024 at 5:02 AM CEST, Min-Hua Chen wrote: > >FWIW: I didn't see it either, but I assumed I was missing the right > >context (i.e. patches) needed to trigger that warning. > > I triggered the warning by the following step: > > install 'sparse' first > > ARCH=arm64 LLVM=1 make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' mrproper defconfig all -j8 This, especially the 'LLVM' part, is important context information and should be part of the commit message. I had only just started when I saw a number of sparse warnings: DTC arch/arm64/boot/dts/rockchip/rk3588-edgeble-neu6a-io.dtb OVL arch/arm64/boot/dts/ti/k3-j721e-evm.dtb OVL arch/arm64/boot/dts/ti/k3-j721s2-evm.dtb OVL arch/arm64/boot/dts/ti/k3-am654-idk.dtb DTC arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3-kb.dtb DTC arch/arm64/boot/dts/rockchip/rk3588-edgeble-neu6a-wifi.dtbo DTC arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3-lte.dtb DTC arch/arm64/boot/dts/rockchip/rk3588-edgeble-neu6b-io.dtb DTC arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dtb DTC arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9.dtb ../init/main.c:192:12: sparse: warning: symbol 'envp_init' was not declared. Should it be static? ../init/main.c:290:16: sparse: warning: cast to restricted __le32 ../init/main.c:291:16: sparse: warning: cast to restricted __le32 CHECK ../init/do_mounts.c And several followed, including in c-code files. So I stopped the build and assume you've identified a or several actual issues. I've seen several commits where changes were made because LLVM flagged potentially problematic code, where GCC did not, so it's quite possible you're on to something here. But it would be helpful if the commit message said what code was potentially problematic and why. And then the proper fix for that could indeed be to include `rockchip_drm_drv.h`. Cheers, Diederik
Hi ， At 2024-09-07 16:47:18, "Diederik de Haas" <didi.debian@cknow.org> wrote: >On Sat Sep 7, 2024 at 5:02 AM CEST, Min-Hua Chen wrote: >> >FWIW: I didn't see it either, but I assumed I was missing the right >> >context (i.e. patches) needed to trigger that warning. >> >> I triggered the warning by the following step: >> >> install 'sparse' first >> >> ARCH=arm64 LLVM=1 make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' mrproper defconfig all -j8 > >This, especially the 'LLVM' part, is important context information >and should be part of the commit message. > >I had only just started when I saw a number of sparse warnings: > > DTC arch/arm64/boot/dts/rockchip/rk3588-edgeble-neu6a-io.dtb > OVL arch/arm64/boot/dts/ti/k3-j721e-evm.dtb > OVL arch/arm64/boot/dts/ti/k3-j721s2-evm.dtb > OVL arch/arm64/boot/dts/ti/k3-am654-idk.dtb > DTC arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3-kb.dtb > DTC arch/arm64/boot/dts/rockchip/rk3588-edgeble-neu6a-wifi.dtbo > DTC arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3-lte.dtb > DTC arch/arm64/boot/dts/rockchip/rk3588-edgeble-neu6b-io.dtb > DTC arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dtb > DTC arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9.dtb >../init/main.c:192:12: sparse: warning: symbol 'envp_init' was not declared. Should it be static? >../init/main.c:290:16: sparse: warning: cast to restricted __le32 >../init/main.c:291:16: sparse: warning: cast to restricted __le32 > CHECK ../init/do_mounts.c > I can see same warnings， a lots of。 And also see the warning in vop2： drivers/gpu/drm/rockchip/rockchip_vop2_reg.c:502:24: sparse: warning: symbol 'vop2_platform_driver' was not declared. Should it be static? Min Hua，If you are agree，I will split it from my patch， and add a Fix tag ，and also add a SoB of you， Then resend in My V3 series，this will make my patch series easier。 >And several followed, including in c-code files. So I stopped the build >and assume you've identified a or several actual issues. > >I've seen several commits where changes were made because LLVM flagged >potentially problematic code, where GCC did not, so it's quite possible >you're on to something here. > >But it would be helpful if the commit message said what code was >potentially problematic and why. And then the proper fix for that could >indeed be to include `rockchip_drm_drv.h`. > >Cheers, > Diederik
>> DTC arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3-kb.dtb >> DTC arch/arm64/boot/dts/rockchip/rk3588-edgeble-neu6a-wifi.dtbo >> DTC arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3-lte.dtb >> DTC arch/arm64/boot/dts/rockchip/rk3588-edgeble-neu6b-io.dtb >> DTC arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dtb >> DTC arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9.dtb >>../init/main.c:192:12: sparse: warning: symbol 'envp_init' was not declared. Should it be static? >>../init/main.c:290:16: sparse: warning: cast to restricted __le32 >>../init/main.c:291:16: sparse: warning: cast to restricted __le32 >> CHECK ../init/do_mounts.c > >> > > >I can see same warnings， a lots of。 >And also see the warning in vop2： >drivers/gpu/drm/rockchip/rockchip_vop2_reg.c:502:24: sparse: warning: symbol 'vop2_platform_driver' was not declared. Should it be static? > > >Min Hua，If you are agree，I will split it from my patch， and add a Fix tag ，and also add a SoB of you， Then resend in My V3 series，this >will make my patch series easier。 It looks good to me. Thank you and Diederik for commenting. cheers, Min-Hua > >And several followed, including in c-code files. So I stopped the build >>and assume you've identified a or several actual issues. >> >>I've seen several commits where changes were made because LLVM flagged >>potentially problematic code, where GCC did not, so it's quite possible >>you're on to something here. >> >>But it would be helpful if the commit message said what code was >>potentially problematic and why. And then the proper fix for that could >>indeed be to include `rockchip_drm_drv.h`. >> >>Cheers, >> Diederik
diff --git a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c index 18efb3fe1c00..c678d1b0fd7c 100644 --- a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c +++ b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c @@ -14,6 +14,7 @@ #include <drm/drm_print.h> #include "rockchip_drm_vop2.h" +#include "rockchip_drm_drv.h" static const uint32_t formats_cluster[] = { DRM_FORMAT_XRGB2101010,
Include rockchip_drm_drv.h to fix the follow sparse warning: drivers/gpu/drm/rockchip/rockchip_vop2_reg.c:502:24: sparse: warning: symbol 'vop2_platform_driver' was not declared. Should it be static? No functional change intended. Signed-off-by: Min-Hua Chen <minhuadotchen@gmail.com> --- drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 1 + 1 file changed, 1 insertion(+)