staging: media: tegra-video: move tegra210_csi_soc declaration to csi.h
Subsystems (4)
PATCH 1/1 staging: media: tegra-video: move tegra210_csi_soc declaration to csi.h
Sparse warns that tegra210_csi_soc is not declared in tegra210.c. The symbol is referenced from csi.c, so it must remain global. Move the declaration to csi.h so users see it via the header and avoid extern declarations in .c files.
No functional change intended.
--- drivers/staging/media/tegra-video/csi.c | 4 ---- drivers/staging/media/tegra-video/csi.h | 4 ++++ 2 files changed, 4 insertions(+), 4 deletions(-)
drivers/staging/media/tegra-video/csi.c +0 -4
@@ -835,10 +835,6 @@ static void tegra_csi_remove(struct platform_device *pdev)pm_runtime_disable (&pdev->dev); } -#if defined(CONFIG_ARCH_TEGRA_210_SOC ) -extern const struct tegra_csi_soc tegra210_csi_soc; -#endif -static const struct of_device_id tegra_csi_of_id_table[] = { #if defined(CONFIG_ARCH_TEGRA_210_SOC) { .compatible = "nvidia,tegra210-csi", .data = &tegra210_csi_soc },
drivers/staging/media/tegra-video/csi.h +4 -0
@@ -130,6 +130,10 @@ struct tegra_csi_soc {unsigned int tpg_frmrate_table_size; }; +#if defined(CONFIG_ARCH_TEGRA_210_SOC ) +extern const struct tegra_csi_soc tegra210_csi_soc; +#endif +/** * struct tegra_csi - NVIDIA Tegra CSI device structure *
Signature · 1 line
2.43.0
Luca Ceresoli
maintainerHello Sun,
Quoted 4 lines · Sun Jian
Sparse warns that
tegra210_csi_socis not declared integra210.c. The symbol is referenced fromcsi.c, so it must remain global. Move the declaration tocsi.hso users see it via the header and avoid extern declarations in .c files.
Can you please add the exact sparse message to your commit log?
Quoted 24 lines
drivers/staging/media/tegra-video/csi.c | 4 ---- drivers/staging/media/tegra-video/csi.h | 4 ++++ 2 files changed, 4 insertions(+), 4 deletions(-)drivers/staging/media/tegra-video/csi.c +0
-4@@ -835,10 +835,6 @@ static void tegra_csi_remove(struct platform_device *pdev)pm_runtime_disable (&pdev->dev); } -#if defined(CONFIG_ARCH_TEGRA_210_SOC ) -extern const struct tegra_csi_soc tegra210_csi_soc; -#endif -static const struct of_device_id tegra_csi_of_id_table[] = { #if defined(CONFIG_ARCH_TEGRA_210_SOC) { .compatible = "nvidia,tegra210-csi", .data = &tegra210_csi_soc },drivers/staging/media/tegra-video/csi.h +0
-0
drivers/staging/media/tegra-video/csi.h struct tegra_csi_soc { · line 135
Quoted 6 lines
drivers/staging/media/tegra-video/csi.h +3
-0@@ -130,6 +130,10 @@ struct tegra_csi_soc {unsigned int tpg_frmrate_table_size; }; +#if defined(CONFIG_ARCH_TEGRA_210_SOC ) +extern const struct tegra_csi_soc tegra210_csi_soc; +#endif
Looks like a good idea indeed.
I checked how other structs shared across .c files are implemented by using 'git grep -w extern -- drivers/staging/media/tegra-video/' and I discovered:
* tegra20_vip_soc uses the exact same pattern you are fixing (extern declaration in .c file)
* tegra20_vi_soc and tegra210_vi_soc have the extern declaration in an include file already, as you are proposing
So it would be nice to convert tegra20_vip_soc as well, in the same way as tegra210_csi_soc. It can be a second patch in your v2 series.
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
v2 cover staging: media: tegra-video: clean up shared SoC declarations
In reply to Sun JianThis series cleans up tegra-video SoC data declarations shared across .c files by moving them to the corresponding headers.
No functional change intended.
---
Changes in v2:
- Add exact sparse warning line to patch 1 commit message.
- Add patch 2 moving tegra20_vip_soc declaration to vip.h for consistency.
Changes in v1:
- Initial version.
Sun Jian (2):
staging: media: tegra-video: move tegra210_csi_soc declaration to
csi.h
staging: media: tegra-video: move tegra20_vip_soc declaration to vip.h
drivers/staging/media/tegra-video/csi.c | 4 ---- drivers/staging/media/tegra-video/csi.h | 4 ++++ drivers/staging/media/tegra-video/vip.c | 4 ---- drivers/staging/media/tegra-video/vip.h | 4 ++++ 4 files changed, 8 insertions(+), 8 deletions(-)
Signature · 1 line
2.43.0
PATCH v2 1/2 staging: media: tegra-video: move tegra210_csi_soc declaration to csi.h
Sparse warns that tegra210_csi_soc is not declared in tegra210.c. The symbol is referenced from csi.c, so it must remain global. Move the declaration to csi.h so users see it via the header and avoid extern declarations in .c files.
SPARSE: drivers/staging/media/tegra-video/tegra210.c:1214:28: warning: symbol 'tegra210_csi_soc' was not declared. Should it be static?
No functional change intended.
--- drivers/staging/media/tegra-video/csi.c | 4 ---- drivers/staging/media/tegra-video/csi.h | 4 ++++ 2 files changed, 4 insertions(+), 4 deletions(-)
drivers/staging/media/tegra-video/csi.c +0 -4
@@ -835,10 +835,6 @@ static void tegra_csi_remove(struct platform_device *pdev)pm_runtime_disable (&pdev->dev); } -#if defined(CONFIG_ARCH_TEGRA_210_SOC ) -extern const struct tegra_csi_soc tegra210_csi_soc; -#endif -static const struct of_device_id tegra_csi_of_id_table[] = { #if defined(CONFIG_ARCH_TEGRA_210_SOC) { .compatible = "nvidia,tegra210-csi", .data = &tegra210_csi_soc },
drivers/staging/media/tegra-video/csi.h +4 -0
@@ -130,6 +130,10 @@ struct tegra_csi_soc {unsigned int tpg_frmrate_table_size; }; +#if defined(CONFIG_ARCH_TEGRA_210_SOC ) +extern const struct tegra_csi_soc tegra210_csi_soc; +#endif +/** * struct tegra_csi - NVIDIA Tegra CSI device structure *
Signature · 1 line
2.43.0
Quoted 5 lines · Sun Jian
Sparse warns that
tegra210_csi_socis not declared integra210.c. The symbol is referenced fromcsi.c, so it must remain global. Move the declaration tocsi.hso users see it via the header and avoid extern declarations in .c files.
Quoted 7 lines
SPARSE:
drivers/staging/media/tegra-video/tegra210.c:1214:28: warning: symbol 'tegra210_csi_soc' was not declared. Should it be static?No functional change intended.
Suggested-by Luca Ceresoli <[email protected]>Signed-off-by Sun Jian <[email protected]>
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
PATCH v2 2/2 staging: media: tegra-video: move tegra20_vip_soc declaration to vip.h
In reply to Sun Jiantegra20_vip_soc is shared across translation units but is currently declared via an extern in vip.c. Move the declaration to vip.h so users get it via the header and we avoid extern declarations in .c files, matching the pattern already used by tegra20_vi_soc and tegra210_vi_soc.
--- drivers/staging/media/tegra-video/vip.c | 4 ---- drivers/staging/media/tegra-video/vip.h | 4 ++++ 2 files changed, 4 insertions(+), 4 deletions(-)
drivers/staging/media/tegra-video/vip.c +0 -4
@@ -263,10 +263,6 @@ static void tegra_vip_remove(struct platform_device *pdev)pm_runtime_disable (&pdev->dev); } -#if defined(CONFIG_ARCH_TEGRA_2x_SOC) -extern const struct tegra_vip_soc tegra20_vip_soc; -#endif -static const struct of_device_id tegra_vip_of_id_table[] = { #if defined(CONFIG_ARCH_TEGRA_2x_SOC) { .compatible = "nvidia,tegra20-vip", .data = &tegra20_vip_soc },
drivers/staging/media/tegra-video/vip.h +4 -0
@@ -50,6 +50,10 @@ struct tegra_vip_soc {const struct tegra_vip_ops *ops; }; +#if defined(CONFIG_ARCH_TEGRA_2x_SOC) +extern const struct tegra_vip_soc tegra20_vip_soc; +#endif +/** * struct tegra_vip - NVIDIA Tegra VIP device structure *
Signature · 1 line
2.43.0
Re: [PATCH v2 2/2] staging: media: tegra-video: move tegra20_vip_soc declaration to vip.h
Quoted 8 lines · Sun Jian
tegra20_vip_socis shared across translation units but is currently declared via an extern invip.c. Move the declaration tovip.hso users get it via the header and we avoid extern declarations in .c files, matching the pattern already used bytegra20_vi_socandtegra210_vi_soc.Suggested-by Luca Ceresoli <[email protected]>Signed-off-by Sun Jian <[email protected]>
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
v3 cover staging: media: tegra-video: move shared SoC declarations to headers
In reply to Sun JianThis series cleans up tegra-video SoC data declarations shared across translation units by moving them to the corresponding headers, avoiding local extern declarations in .c files.
No functional change intended.
---
Changes in v3:
- Wrap the SPARSE warning in patch 1 commit message to <= 75 columns.
Changes in v2:
- Add the exact sparse warning line to patch 1 commit message.
- Add patch 2 moving tegra20_vip_soc declaration to vip.h for consistency.
Changes in v1:
- Initial version.
Sun Jian (2):
staging: media: tegra-video: move tegra210_csi_soc declaration to
csi.h
staging: media: tegra-video: move tegra20_vip_soc declaration to vip.h
drivers/staging/media/tegra-video/csi.c | 4 ---- drivers/staging/media/tegra-video/csi.h | 4 ++++ drivers/staging/media/tegra-video/vip.c | 4 ---- drivers/staging/media/tegra-video/vip.h | 4 ++++ 4 files changed, 8 insertions(+), 8 deletions(-)
Signature · 1 line
2.43.0
PATCH v3 1/2 staging: media: tegra-video: move tegra210_csi_soc declaration to csi.h
Sparse warns that tegra210_csi_soc is not declared in tegra210.c. The symbol is referenced from csi.c, so it must remain global. Move the declaration to csi.h so users see it via the header and avoid extern declarations in .c files.
SPARSE: drivers/staging/media/tegra-video/tegra210.c:1214:28: warning:
symbol 'tegra210_csi_soc' was not declared. Should it be static?
No functional change intended.
--- drivers/staging/media/tegra-video/csi.c | 4 ---- drivers/staging/media/tegra-video/csi.h | 4 ++++ 2 files changed, 4 insertions(+), 4 deletions(-)
drivers/staging/media/tegra-video/csi.c +0 -4
@@ -835,10 +835,6 @@ static void tegra_csi_remove(struct platform_device *pdev)pm_runtime_disable (&pdev->dev); } -#if defined(CONFIG_ARCH_TEGRA_210_SOC ) -extern const struct tegra_csi_soc tegra210_csi_soc; -#endif -static const struct of_device_id tegra_csi_of_id_table[] = { #if defined(CONFIG_ARCH_TEGRA_210_SOC) { .compatible = "nvidia,tegra210-csi", .data = &tegra210_csi_soc },
drivers/staging/media/tegra-video/csi.h +4 -0
@@ -130,6 +130,10 @@ struct tegra_csi_soc {unsigned int tpg_frmrate_table_size; }; +#if defined(CONFIG_ARCH_TEGRA_210_SOC ) +extern const struct tegra_csi_soc tegra210_csi_soc; +#endif +/** * struct tegra_csi - NVIDIA Tegra CSI device structure *
Signature · 1 line
2.43.0
PATCH v3 2/2 staging: media: tegra-video: move tegra20_vip_soc declaration to vip.h
In reply to Sun Jiantegra20_vip_soc is shared across translation units but is currently declared via an extern in vip.c. Move the declaration to vip.h so users get it via the header and we avoid extern declarations in .c files, matching the pattern already used by tegra20_vi_soc and tegra210_vi_soc.
--- drivers/staging/media/tegra-video/vip.c | 4 ---- drivers/staging/media/tegra-video/vip.h | 4 ++++ 2 files changed, 4 insertions(+), 4 deletions(-)
drivers/staging/media/tegra-video/vip.c +0 -4
@@ -263,10 +263,6 @@ static void tegra_vip_remove(struct platform_device *pdev)pm_runtime_disable (&pdev->dev); } -#if defined(CONFIG_ARCH_TEGRA_2x_SOC) -extern const struct tegra_vip_soc tegra20_vip_soc; -#endif -static const struct of_device_id tegra_vip_of_id_table[] = { #if defined(CONFIG_ARCH_TEGRA_2x_SOC) { .compatible = "nvidia,tegra20-vip", .data = &tegra20_vip_soc },
drivers/staging/media/tegra-video/vip.h +4 -0
@@ -50,6 +50,10 @@ struct tegra_vip_soc {const struct tegra_vip_ops *ops; }; +#if defined(CONFIG_ARCH_TEGRA_2x_SOC) +extern const struct tegra_vip_soc tegra20_vip_soc; +#endif +/** * struct tegra_vip - NVIDIA Tegra VIP device structure *
Signature · 1 line
2.43.0
Luca Ceresoli
maintainerRe: [PATCH v3 0/2] staging: media: tegra-video: move shared SoC declarations to headers
In reply to Sun JianQuoted 9 lines · Sun Jian
This series cleans up tegra-video SoC data declarations shared across translation units by moving them to the corresponding headers, avoiding local extern declarations in .c files.
No functional change intended.
---
Changes in v3:
- Wrap the SPARSE warning in patch 1 commit message to <= 75 columns.
My understanding and my knowledge of the common practice is that you shouldn't wrap lines if they are quoting a single path, log message, error message and the like.
So I think v2 should be applied, not v3. But I'll leave the decision to maintainers applying the patches.
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [PATCH v3 0/2] staging: media: tegra-video: move shared SoC declarations to headers
Hi all,
Sorry — I mistakenly replied only to Luca off-thread earlier. Re-sending the same note here via reply-to-all so everyone in the thread has the context.
v3 was sent only to silence the Media CI checkpatch warning about >75 columns in the commit message. I agree that for an exact sparse error/warning line, keeping it unwrapped makes more sense.
So: v2 keeps the original single-line SPARSE message, while v3 only wraps that line. There are no code changes between v2 and v3.
I’ll defer to the maintainer who picks up the series — if you prefer one over the other (v2 vs v3) I can follow that convention for the next revision.
Luca, sorry for the extra mail — posting this publicly to keep the discussion in one place.
Thanks,
Sun Jian
Quoted 25 lines · Luca Ceresoli
Quoted 9 lines · Sun Jian
This series cleans up tegra-video SoC data declarations shared across translation units by moving them to the corresponding headers, avoiding local extern declarations in .c files.
No functional change intended.
---
Changes in v3:
- Wrap the SPARSE warning in patch 1 commit message to <= 75 columns.My understanding and my knowledge of the common practice is that you shouldn't wrap lines if they are quoting a single path, log message, error message and the like.
So I think v2 should be applied, not v3. But I'll leave the decision to maintainers applying the patches.
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com