On Wed, 31 Jul 2024 12:36:19 +0200, Jaroslav Kysela wrote: > > On 31. 07. 24 12:30, Takashi Iwai wrote: > > On Tue, 30 Jul 2024 16:55:19 +0200, > > Jaroslav Kysela wrote: > >> > >> On 30. 07. 24 16:37, Stefan Binding wrote: > >>> Add a kernel parameter to allow coefficients to be exposed as ALSA controls. > >>> > >>> When the CS35L41 loads its firmware, it has a number of controls to > >>> affect its behaviour. Currently, these controls are exposed as ALSA > >>> Controls by default. > >>> > >>> However, nothing in userspace currently uses them, and is unlikely to > >>> do so in the future, therefore we don't need to create ASLA controls > >>> for them. > >>> > >>> These controls can be useful for debug, so we can add a kernel > >>> parameter to re-enable them if necessary. > >>> > >>> Disabling these controls would prevent userspace from trying to read > >>> these controls when the CS35L41 is hibernating, which ordinarily > >>> would result in an error message. > >> > >> This is probably not a right argument to add this code. The codec > >> should be powered up when those controls are accessed or those > >> controls should be cached by the driver. > >> > >> Although the controls have not been used yet, exposing them in this > >> way is not ideal. > >> > >> Could you fix the driver (no I/O errors)? > > > > While we should fix the potential errors at hibernation, it's not bad > > to hide those controls, IMO. For the normal use cases, it's nothing > > but a cause of troubles, after all. > > I do not think that the situation is so obvious. Different > coefficients can be used in various UCM profiles for example. If that's the supposed use-case, yes. I doubt it, though, but this needs clarification from Cirrus people. > But for debugging we have debugfs when the developer thinks that the > feature is not useful for users. The module parameter solution is not > good in my eyes. Yeah, I believe we should disable it unconditionally, and provide a different way like debugfs in the firmware driver side, too -- again, if the exposure is only for debugging. Takashi
