| Message ID | 20200731144146.6678-1-cezary.rojewski@intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | ASoC: core: Two step component registration | expand |
On 7/31/20 9:41 AM, Cezary Rojewski wrote: > Provide a mechanism for true two-step component registration. This > mimics device registration flow where initialization is the first step > while addition goes as second in line. Drivers may choose to modify > component's fields before registering component to ASoC subsystem via > snd_soc_add_component. I must admit I don't see where this might be used for Intel platforms, we've been happily using snd_soc_register_component() without hitting limitations. Also the only two uses of snd_soc_add_component() seem mainly driven by memory allocation - and avoiding a devm_kzalloc in snd_soc_register_component(). Out of curiosity, can you provide an example where this two-step would be required or beneficial? Thanks! > Patchset achieves status quo - behavior of snd_soc_register_component > remains unchanged. > > Cezary Rojewski (3): > ASoC: core: Relocate and expose snd_soc_component_initialize > ASoC: core: Simplify snd_soc_component_initialize declaration > ASoC: core: Two step component registration > > include/sound/soc-component.h | 3 -- > include/sound/soc.h | 11 +++--- > sound/soc/soc-component.c | 16 --------- > sound/soc/soc-core.c | 52 +++++++++++++++++---------- > sound/soc/soc-generic-dmaengine-pcm.c | 14 +++++--- > sound/soc/stm/stm32_adfsdm.c | 9 +++-- > 6 files changed, 55 insertions(+), 50 deletions(-) >
On 2020-07-31 5:07 PM, Pierre-Louis Bossart wrote: > On 7/31/20 9:41 AM, Cezary Rojewski wrote: >> Provide a mechanism for true two-step component registration. This >> mimics device registration flow where initialization is the first step >> while addition goes as second in line. Drivers may choose to modify >> component's fields before registering component to ASoC subsystem via >> snd_soc_add_component. > > I must admit I don't see where this might be used for Intel platforms, > we've been happily using snd_soc_register_component() without hitting > limitations. Patchset targets entire ASoC framework, not Intel catalog. As _initialize and _add are already in place, having a two-step registration is cohesive with other "frameworks" e.g. device one. New to ASoC? Trying to learn soc-components? Guess what, creation/registration procedure is exactly the same as one you're used to already! > Also the only two uses of snd_soc_add_component() seem mainly driven by > memory allocation - and avoiding a devm_kzalloc in > snd_soc_register_component(). In general, code quality and improvements to its flow should not require ton of usages. But hey, you got two already. > Out of curiosity, can you provide an example where this two-step would > be required or beneficial? Thanks! Overridding component->name which is currently always tied to fmt_single_name so you may choose a different name than the ->dev one.
On 7/31/20 5:47 PM, Cezary Rojewski wrote: > On 2020-07-31 5:07 PM, Pierre-Louis Bossart wrote: >> On 7/31/20 9:41 AM, Cezary Rojewski wrote: >>> Provide a mechanism for true two-step component registration. This >>> mimics device registration flow where initialization is the first step >>> while addition goes as second in line. Drivers may choose to modify >>> component's fields before registering component to ASoC subsystem via >>> snd_soc_add_component. >> >> I must admit I don't see where this might be used for Intel >> platforms, we've been happily using snd_soc_register_component() >> without hitting limitations. > > Patchset targets entire ASoC framework, not Intel catalog. As > _initialize and _add are already in place, having a two-step > registration is cohesive with other "frameworks" e.g. device one. > > New to ASoC? Trying to learn soc-components? Guess what, > creation/registration procedure is exactly the same as one you're used > to already! > >> Also the only two uses of snd_soc_add_component() seem mainly driven >> by memory allocation - and avoiding a devm_kzalloc in >> snd_soc_register_component(). > > In general, code quality and improvements to its flow should not > require ton of usages. But hey, you got two already. > >> Out of curiosity, can you provide an example where this two-step >> would be required or beneficial? Thanks! > > Overridding component->name which is currently always tied to > fmt_single_name so you may choose a different name than the ->dev one. For what it is worth, I think this is a sensible change for the outlined reasons. It's something I've always had in the back of my mind, but there was never enough of a need to actually do it. One change I'd like to see is the addition of snd_soc_component_alloc(), which combines the step of kzalloc() and snd_soc_component_init() as these will be done pretty much always in lockstep. And it also matches the APIs that other frameworks offer. - Lars
On 2020-07-31 5:58 PM, Lars-Peter Clausen wrote: > On 7/31/20 5:47 PM, Cezary Rojewski wrote: >> On 2020-07-31 5:07 PM, Pierre-Louis Bossart wrote: >>> On 7/31/20 9:41 AM, Cezary Rojewski wrote: >>>> Provide a mechanism for true two-step component registration. This >>>> mimics device registration flow where initialization is the first step >>>> while addition goes as second in line. Drivers may choose to modify >>>> component's fields before registering component to ASoC subsystem via >>>> snd_soc_add_component. >>> >>> I must admit I don't see where this might be used for Intel >>> platforms, we've been happily using snd_soc_register_component() >>> without hitting limitations. >> >> Patchset targets entire ASoC framework, not Intel catalog. As >> _initialize and _add are already in place, having a two-step >> registration is cohesive with other "frameworks" e.g. device one. >> >> New to ASoC? Trying to learn soc-components? Guess what, >> creation/registration procedure is exactly the same as one you're used >> to already! >> >>> Also the only two uses of snd_soc_add_component() seem mainly driven >>> by memory allocation - and avoiding a devm_kzalloc in >>> snd_soc_register_component(). >> >> In general, code quality and improvements to its flow should not >> require ton of usages. But hey, you got two already. >> >>> Out of curiosity, can you provide an example where this two-step >>> would be required or beneficial? Thanks! >> >> Overridding component->name which is currently always tied to >> fmt_single_name so you may choose a different name than the ->dev one. > > For what it is worth, I think this is a sensible change for the outlined > reasons. It's something I've always had in the back of my mind, but > there was never enough of a need to actually do it. > > One change I'd like to see is the addition of snd_soc_component_alloc(), > which combines the step of kzalloc() and snd_soc_component_init() as > these will be done pretty much always in lockstep. And it also matches > the APIs that other frameworks offer. > > - Lars > Nice, so it's not just me imagining things : D In general granular registration is robust and scales well into the future. Components functionality will only grow in time so I bet usecases don't end on my example. I'd suggest transition to _alloc/_init/other being separated from this patchset - let it serve as a middle step. Czarek
On Fri, Jul 31, 2020 at 05:58:00PM +0200, Lars-Peter Clausen wrote: > On 7/31/20 5:47 PM, Cezary Rojewski wrote: > > Patchset targets entire ASoC framework, not Intel catalog. As > > _initialize and _add are already in place, having a two-step > > registration is cohesive with other "frameworks" e.g. device one. > For what it is worth, I think this is a sensible change for the outlined > reasons. It's something I've always had in the back of my mind, but there > was never enough of a need to actually do it. Yeah, it's a common pattern in the kernel and someone might want it so there's no great reason not to do it. > One change I'd like to see is the addition of snd_soc_component_alloc(), > which combines the step of kzalloc() and snd_soc_component_init() as these > will be done pretty much always in lockstep. And it also matches the APIs > that other frameworks offer. That'd be good.
On Fri, 31 Jul 2020 16:41:43 +0200, Cezary Rojewski wrote: > Provide a mechanism for true two-step component registration. This > mimics device registration flow where initialization is the first step > while addition goes as second in line. Drivers may choose to modify > component's fields before registering component to ASoC subsystem via > snd_soc_add_component. > > Patchset achieves status quo - behavior of snd_soc_register_component > remains unchanged. > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [1/3] ASoC: core: Relocate and expose snd_soc_component_initialize commit: 08ff7209faf21daa01bf66c91c321ce52d4b4bdb [2/3] ASoC: core: Simplify snd_soc_component_initialize declaration commit: 7274d4cd8506bbff9bf2d7c2f73b2febff99abef [3/3] ASoC: core: Two step component registration commit: ea029dd8d0124fcd5db1c7003e87a7bd4ddb3bad All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark