| Message ID | 20210912202907.28471-5-digetx@gmail.com |
|---|---|
| State | Rejected |
| Headers | show |
| Series | Tegra cpuidle driver fixes and improvements for 5.16 | expand |
On Sun, Sep 12, 2021 at 11:29:05PM +0300, Dmitry Osipenko wrote: > Add stubs needed for compile-testing of tegra-cpuidle driver. > > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- > include/linux/clk/tegra.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/include/linux/clk/tegra.h b/include/linux/clk/tegra.h > index d128ad1570aa..9bd06d8a5436 100644 > --- a/include/linux/clk/tegra.h > +++ b/include/linux/clk/tegra.h > @@ -42,7 +42,11 @@ struct tegra_cpu_car_ops { > #endif > }; > > +#ifdef CONFIG_ARCH_TEGRA > extern struct tegra_cpu_car_ops *tegra_cpu_car_ops; > +#else > +static struct tegra_cpu_car_ops *tegra_cpu_car_ops __maybe_unused; > +#endif Ugh... this one seems a bit over the top, to be honest. The only place where this seems to be used is in arch/arm/mach-tegra/pm.c, but that already uses one of the stubs from include/linux/clk/tegra.h, so I'm wondering if we can't define that latter stub in a way to make it unnecessary to declare this bogus pointer. I'll play around with this a little bit. Thierry
On Mon, Oct 04, 2021 at 09:33:57PM +0200, Thierry Reding wrote: > On Sun, Sep 12, 2021 at 11:29:05PM +0300, Dmitry Osipenko wrote: > > Add stubs needed for compile-testing of tegra-cpuidle driver. > > > > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > > --- > > include/linux/clk/tegra.h | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/include/linux/clk/tegra.h b/include/linux/clk/tegra.h > > index d128ad1570aa..9bd06d8a5436 100644 > > --- a/include/linux/clk/tegra.h > > +++ b/include/linux/clk/tegra.h > > @@ -42,7 +42,11 @@ struct tegra_cpu_car_ops { > > #endif > > }; > > > > +#ifdef CONFIG_ARCH_TEGRA > > extern struct tegra_cpu_car_ops *tegra_cpu_car_ops; > > +#else > > +static struct tegra_cpu_car_ops *tegra_cpu_car_ops __maybe_unused; > > +#endif > > Ugh... this one seems a bit over the top, to be honest. The only place > where this seems to be used is in arch/arm/mach-tegra/pm.c, but that > already uses one of the stubs from include/linux/clk/tegra.h, so I'm > wondering if we can't define that latter stub in a way to make it > unnecessary to declare this bogus pointer. > > I'll play around with this a little bit. The below does the trick for me as well: --- >8 --- diff --git a/include/linux/clk/tegra.h b/include/linux/clk/tegra.h index d128ad1570aa..d261db7e6060 100644 --- a/include/linux/clk/tegra.h +++ b/include/linux/clk/tegra.h @@ -42,6 +42,7 @@ struct tegra_cpu_car_ops { #endif }; +#ifdef CONFIG_ARCH_TEGRA extern struct tegra_cpu_car_ops *tegra_cpu_car_ops; static inline void tegra_wait_cpu_in_reset(u32 cpu) @@ -83,6 +84,27 @@ static inline void tegra_disable_cpu_clock(u32 cpu) tegra_cpu_car_ops->disable_clock(cpu); } +#else +static inline void tegra_wait_cpu_in_reset(u32 cpu) +{ +} + +static inline void tegra_put_cpu_in_reset(u32 cpu) +{ +} + +static inline void tegra_cpu_out_of_reset(u32 cpu) +{ +} + +static inline void tegra_enable_cpu_clock(u32 cpu) +{ +} + +static inline void tegra_disable_cpu_clock(u32 cpu) +{ +} +#endif #ifdef CONFIG_PM_SLEEP static inline bool tegra_cpu_rail_off_ready(void) --- >8 --- Do you mind if I replace your version with that? I think that's a little bit cleaner because it should be easier for the compiler to completely compile it out. Thierry
04.10.2021 23:02, Thierry Reding пишет: > On Mon, Oct 04, 2021 at 09:33:57PM +0200, Thierry Reding wrote: >> On Sun, Sep 12, 2021 at 11:29:05PM +0300, Dmitry Osipenko wrote: >>> Add stubs needed for compile-testing of tegra-cpuidle driver. >>> >>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >>> --- >>> include/linux/clk/tegra.h | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/include/linux/clk/tegra.h b/include/linux/clk/tegra.h >>> index d128ad1570aa..9bd06d8a5436 100644 >>> --- a/include/linux/clk/tegra.h >>> +++ b/include/linux/clk/tegra.h >>> @@ -42,7 +42,11 @@ struct tegra_cpu_car_ops { >>> #endif >>> }; >>> >>> +#ifdef CONFIG_ARCH_TEGRA >>> extern struct tegra_cpu_car_ops *tegra_cpu_car_ops; >>> +#else >>> +static struct tegra_cpu_car_ops *tegra_cpu_car_ops __maybe_unused; >>> +#endif >> >> Ugh... this one seems a bit over the top, to be honest. The only place >> where this seems to be used is in arch/arm/mach-tegra/pm.c, but that >> already uses one of the stubs from include/linux/clk/tegra.h, so I'm >> wondering if we can't define that latter stub in a way to make it >> unnecessary to declare this bogus pointer. >> >> I'll play around with this a little bit. > > The below does the trick for me as well: > > --- >8 --- > diff --git a/include/linux/clk/tegra.h b/include/linux/clk/tegra.h > index d128ad1570aa..d261db7e6060 100644 > --- a/include/linux/clk/tegra.h > +++ b/include/linux/clk/tegra.h > @@ -42,6 +42,7 @@ struct tegra_cpu_car_ops { > #endif > }; > > +#ifdef CONFIG_ARCH_TEGRA > extern struct tegra_cpu_car_ops *tegra_cpu_car_ops; > > static inline void tegra_wait_cpu_in_reset(u32 cpu) > @@ -83,6 +84,27 @@ static inline void tegra_disable_cpu_clock(u32 cpu) > > tegra_cpu_car_ops->disable_clock(cpu); > } > +#else > +static inline void tegra_wait_cpu_in_reset(u32 cpu) > +{ > +} > + > +static inline void tegra_put_cpu_in_reset(u32 cpu) > +{ > +} > + > +static inline void tegra_cpu_out_of_reset(u32 cpu) > +{ > +} > + > +static inline void tegra_enable_cpu_clock(u32 cpu) > +{ > +} > + > +static inline void tegra_disable_cpu_clock(u32 cpu) > +{ > +} > +#endif > > #ifdef CONFIG_PM_SLEEP > static inline bool tegra_cpu_rail_off_ready(void) > --- >8 --- > > Do you mind if I replace your version with that? I think that's a little > bit cleaner because it should be easier for the compiler to completely > compile it out. I don't mind, please choose what you prefer more. This was one of the first variants of this patch, it adds more lines and it won't be compiled with ARCH_TEGRA=n and COMPILE_TEST=n anyways.
diff --git a/include/linux/clk/tegra.h b/include/linux/clk/tegra.h index d128ad1570aa..9bd06d8a5436 100644 --- a/include/linux/clk/tegra.h +++ b/include/linux/clk/tegra.h @@ -42,7 +42,11 @@ struct tegra_cpu_car_ops { #endif }; +#ifdef CONFIG_ARCH_TEGRA extern struct tegra_cpu_car_ops *tegra_cpu_car_ops; +#else +static struct tegra_cpu_car_ops *tegra_cpu_car_ops __maybe_unused; +#endif static inline void tegra_wait_cpu_in_reset(u32 cpu) {
Add stubs needed for compile-testing of tegra-cpuidle driver. Signed-off-by: Dmitry Osipenko <digetx@gmail.com> --- include/linux/clk/tegra.h | 4 ++++ 1 file changed, 4 insertions(+)