| Message ID | 20241127-clk-audio-fix-rst-missing-v1-1-9f9d0ab98fce@baylibre.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Neil Armstrong |
| Headers | show |
| Series | clk: amlogic: axg-audio: select RESET_MESON_AUX | expand |
On Wed, Nov 27, 2024 at 07:47:46PM +0100, Jerome Brunet wrote: > Depending on RESET_MESON_AUX result in axg-audio support being turned > off by default for the users of arm64 defconfig, which is kind of a > regression for them. > Cc: Mark Brown <broonie@kernel.org> > Fixes: 681ed497d676 ("clk: amlogic: axg-audio: fix Kconfig dependency on RESET_MESON_AUX") > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> Reviewed-by: Mark Brown <broonie@kernel.org> Reported-by: Mark Brown <broonie@kernel.org> (I reported this to Jerome on IRC) > --- > Hello Stephen, > This fixes a problem introduced in this merge window. > Could you please take it directly ? It'd be great to get this into -rc1, I've got some of the affected systems in CI.
On Wed, Nov 27, 2024, at 19:47, Jerome Brunet wrote: > Depending on RESET_MESON_AUX result in axg-audio support being turned > off by default for the users of arm64 defconfig, which is kind of a > regression for them. > > RESET_MESON_AUX is not in directly the defconfig, so depending on it turn > COMMON_CLK_AXG_AUDIO off. The clock provided by this module are > necessary for every axg audio devices. Those are now deferring. > > Select RESET_MESON_AUX rather than just depending on it. > With this, the audio subsystem of the affected platform should probe > correctly again > > Cc: Mark Brown <broonie@kernel.org> > Fixes: 681ed497d676 ("clk: amlogic: axg-audio: fix Kconfig dependency > on RESET_MESON_AUX") > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> febb5d7348ff07c2da0cb5fd41d2ad2607e5bd5d..ea16bfde0df2d7bfebb041161f6b96bbb35003ed > 100644 > --- a/drivers/clk/meson/Kconfig > +++ b/drivers/clk/meson/Kconfig > @@ -106,7 +106,7 @@ config COMMON_CLK_AXG_AUDIO > select COMMON_CLK_MESON_SCLK_DIV > select COMMON_CLK_MESON_CLKC_UTILS > select REGMAP_MMIO > - depends on RESET_MESON_AUX > + select RESET_MESON_AUX > help > Support for the audio clock controller on AmLogic A113D devices, > aka axg, Say Y if you want audio subsystem to work. You should generally not 'select' a symbol from another subsystem, as this risks introducing dependency loops, and missing dependencies. It looks like RESET_MESON_AUX is a user-visible symbol, so you can simply ask users to turn it on, and add it to the defconfig. I also see some silliness going on in the include/soc/amlogic/reset-meson-aux.h, which has a non-working 'static inline' definition of the exported function. Before my fix, that would have caused the problem auf a non-working audio driver. Arnd
On Wed 27 Nov 2024 at 20:30, "Arnd Bergmann" <arnd@arndb.de> wrote: > On Wed, Nov 27, 2024, at 19:47, Jerome Brunet wrote: >> Depending on RESET_MESON_AUX result in axg-audio support being turned >> off by default for the users of arm64 defconfig, which is kind of a >> regression for them. >> >> RESET_MESON_AUX is not in directly the defconfig, so depending on it turn >> COMMON_CLK_AXG_AUDIO off. The clock provided by this module are >> necessary for every axg audio devices. Those are now deferring. >> >> Select RESET_MESON_AUX rather than just depending on it. >> With this, the audio subsystem of the affected platform should probe >> correctly again >> >> Cc: Mark Brown <broonie@kernel.org> >> Fixes: 681ed497d676 ("clk: amlogic: axg-audio: fix Kconfig dependency >> on RESET_MESON_AUX") >> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> > > > febb5d7348ff07c2da0cb5fd41d2ad2607e5bd5d..ea16bfde0df2d7bfebb041161f6b96bbb35003ed >> 100644 >> --- a/drivers/clk/meson/Kconfig >> +++ b/drivers/clk/meson/Kconfig >> @@ -106,7 +106,7 @@ config COMMON_CLK_AXG_AUDIO >> select COMMON_CLK_MESON_SCLK_DIV >> select COMMON_CLK_MESON_CLKC_UTILS >> select REGMAP_MMIO >> - depends on RESET_MESON_AUX >> + select RESET_MESON_AUX >> help >> Support for the audio clock controller on AmLogic A113D devices, >> aka axg, Say Y if you want audio subsystem to work. > > You should generally not 'select' a symbol from another > subsystem, as this risks introducing dependency loops, > and missing dependencies. I do understand that one needs to be careful with that sort of things but I don't think this is happening here. > > It looks like RESET_MESON_AUX is a user-visible symbol, > so you can simply ask users to turn it on, and add it to > the defconfig. That would work yes but It's really something a user should not be concerned with. I can follow-up with another change to remove the user visibilty of RESET_MESON_AUX. It is always going to be something requested by another driver. > > I also see some silliness going on in the > include/soc/amlogic/reset-meson-aux.h, which has a > non-working 'static inline' definition of the exported > function. Before my fix, that would have caused the > problem auf a non-working audio driver. If by 'silliness' you mean there is symbol definition for when RESET_MESON_AUX is disabled, indeed I guess that could go away. Thanks for pointing it out. > > Arnd
diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig index febb5d7348ff07c2da0cb5fd41d2ad2607e5bd5d..ea16bfde0df2d7bfebb041161f6b96bbb35003ed 100644 --- a/drivers/clk/meson/Kconfig +++ b/drivers/clk/meson/Kconfig @@ -106,7 +106,7 @@ config COMMON_CLK_AXG_AUDIO select COMMON_CLK_MESON_SCLK_DIV select COMMON_CLK_MESON_CLKC_UTILS select REGMAP_MMIO - depends on RESET_MESON_AUX + select RESET_MESON_AUX help Support for the audio clock controller on AmLogic A113D devices, aka axg, Say Y if you want audio subsystem to work.
Depending on RESET_MESON_AUX result in axg-audio support being turned off by default for the users of arm64 defconfig, which is kind of a regression for them. RESET_MESON_AUX is not in directly the defconfig, so depending on it turn COMMON_CLK_AXG_AUDIO off. The clock provided by this module are necessary for every axg audio devices. Those are now deferring. Select RESET_MESON_AUX rather than just depending on it. With this, the audio subsystem of the affected platform should probe correctly again Cc: Mark Brown <broonie@kernel.org> Fixes: 681ed497d676 ("clk: amlogic: axg-audio: fix Kconfig dependency on RESET_MESON_AUX") Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> --- Hello Stephen, This fixes a problem introduced in this merge window. Could you please take it directly ? Thanks --- drivers/clk/meson/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- base-commit: 6f3d2b5299b0a8bcb8a9405a8d3fceb24f79c4f0 change-id: 20241127-clk-audio-fix-rst-missing-0b80628d934b Best regards,