[alsa-devel] [PATCH] ASoC: rsnd: Protect register accesses with a spinlock instead of a mutex
The hardware registers are accessed from atomic contexts (the rsnd_soc_dai_trigger function, for instance, is called with the PCM substream spinlock held). They thus can't be protected by a mutex.
Protect regmap register accesses with a spinlock instead of a mutex by setting the fast_io flag.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com --- sound/soc/sh/rcar/gen.c | 1 + 1 file changed, 1 insertion(+)
An even better solution might be to use regmap-mmio instead of a custom bus. Morimoto-san, is there anything that would prevent the driver from switching to regmap-mmio ?
diff --git a/sound/soc/sh/rcar/gen.c b/sound/soc/sh/rcar/gen.c index 73ce4c9..ec9ac5e 100644 --- a/sound/soc/sh/rcar/gen.c +++ b/sound/soc/sh/rcar/gen.c @@ -66,6 +66,7 @@ static int rsnd_regmap_read32(void *context, }
static struct regmap_bus rsnd_regmap_bus = { + .fast_io = true, .write = rsnd_regmap_write32, .read = rsnd_regmap_read32, .reg_format_endian_default = REGMAP_ENDIAN_NATIVE,
Hi Laurent
The hardware registers are accessed from atomic contexts (the rsnd_soc_dai_trigger function, for instance, is called with the PCM substream spinlock held). They thus can't be protected by a mutex.
Protect regmap register accesses with a spinlock instead of a mutex by setting the fast_io flag.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com
sound/soc/sh/rcar/gen.c | 1 + 1 file changed, 1 insertion(+)
An even better solution might be to use regmap-mmio instead of a custom bus. Morimoto-san, is there anything that would prevent the driver from switching to regmap-mmio ?
I guess it is possilbe to use regmap-mmio. I check it.
diff --git a/sound/soc/sh/rcar/gen.c b/sound/soc/sh/rcar/gen.c index 73ce4c9..ec9ac5e 100644 --- a/sound/soc/sh/rcar/gen.c +++ b/sound/soc/sh/rcar/gen.c @@ -66,6 +66,7 @@ static int rsnd_regmap_read32(void *context, }
static struct regmap_bus rsnd_regmap_bus = {
- .fast_io = true, .write = rsnd_regmap_write32, .read = rsnd_regmap_read32, .reg_format_endian_default = REGMAP_ENDIAN_NATIVE,
-- Regards,
Laurent Pinchart
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Hi Laurent
The hardware registers are accessed from atomic contexts (the rsnd_soc_dai_trigger function, for instance, is called with the PCM substream spinlock held). They thus can't be protected by a mutex.
Protect regmap register accesses with a spinlock instead of a mutex by setting the fast_io flag.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com
sound/soc/sh/rcar/gen.c | 1 + 1 file changed, 1 insertion(+)
An even better solution might be to use regmap-mmio instead of a custom bus. Morimoto-san, is there anything that would prevent the driver from switching to regmap-mmio ?
I guess it is possilbe to use regmap-mmio. I check it.
Hmm... rsnd driver is using "regmap_field". regmap-mmio requests "offset" on "reg" regmap-filed requests "address" on "reg"
So, if rsnd driver uses regmap-mmio, then, it needs tricky initialize like...
regmap_init_mmio(dev, 0, config)
Best regards --- Kuninori Morimoto
On 07/24/2014 04:16 AM, Kuninori Morimoto wrote:
Hi Laurent
The hardware registers are accessed from atomic contexts (the rsnd_soc_dai_trigger function, for instance, is called with the PCM substream spinlock held). They thus can't be protected by a mutex.
Protect regmap register accesses with a spinlock instead of a mutex by setting the fast_io flag.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com
sound/soc/sh/rcar/gen.c | 1 + 1 file changed, 1 insertion(+)
An even better solution might be to use regmap-mmio instead of a custom bus. Morimoto-san, is there anything that would prevent the driver from switching to regmap-mmio ?
I guess it is possilbe to use regmap-mmio. I check it.
Hmm... rsnd driver is using "regmap_field". regmap-mmio requests "offset" on "reg" regmap-filed requests "address" on "reg"
So, if rsnd driver uses regmap-mmio, then, it needs tricky initialize like...
regmap_init_mmio(dev, 0, config)
Create one regmap instance per base address and in the regmap_fields use an relative offset to the base address rather than the absolute address. That's how the API is intended to be used, the current implementation is quite a hack.
- Lars
Hi Lars # added Mark
Hmm... rsnd driver is using "regmap_field". regmap-mmio requests "offset" on "reg" regmap-filed requests "address" on "reg"
So, if rsnd driver uses regmap-mmio, then, it needs tricky initialize like...
regmap_init_mmio(dev, 0, config)Create one regmap instance per base address and in the regmap_fields use an relative offset to the base address rather than the absolute address. That's how the API is intended to be used, the current implementation is quite a hack.
Hmm... But it (= base address mapping was not fixed between Gen1 and Gen2) was the reason why I was asked to use regmap_fields
On 07/28/2014 09:40 AM, Kuninori Morimoto wrote:
Hi Lars # added Mark
Hmm... rsnd driver is using "regmap_field". regmap-mmio requests "offset" on "reg" regmap-filed requests "address" on "reg"
So, if rsnd driver uses regmap-mmio, then, it needs tricky initialize like...
regmap_init_mmio(dev, 0, config)Create one regmap instance per base address and in the regmap_fields use an relative offset to the base address rather than the absolute address. That's how the API is intended to be used, the current implementation is quite a hack.
Hmm... But it (= base address mapping was not fixed between Gen1 and Gen2) was the reason why I was asked to use regmap_fields
I think that is fine. But you are only using a single regmap instance even though there are multiple unrelated register maps used and then you specify the offset in the regmap_fields as a absolute address. This is supposed to be a relative offset to the base address.
So basically use: ".reg = offset" instead of ".reg = (unsigned int)gen->base[reg_id] + offset" and when creating the field instead of passing a global regmap instance pass the regmap instance for the register map in who's range the register falls.
- Lars
Hi Lars
But it (= base address mapping was not fixed between Gen1 and Gen2) was the reason why I was asked to use regmap_fields
I think that is fine. But you are only using a single regmap instance even though there are multiple unrelated register maps used and then you specify the offset in the regmap_fields as a absolute address. This is supposed to be a relative offset to the base address.
So basically use: ".reg = offset" instead of ".reg = (unsigned int)gen->base[reg_id] + offset" and when creating the field instead of passing a global regmap instance pass the regmap instance for the register map in who's range the register falls.
Hmm... I re-checked it, and I could understand. And, indeed it was hackish. OK, I try to modify it, then, it seems can use regmap-mmio.
On 23/07/14 22:19, Laurent Pinchart wrote:
The hardware registers are accessed from atomic contexts (the rsnd_soc_dai_trigger function, for instance, is called with the PCM substream spinlock held). They thus can't be protected by a mutex.
Protect regmap register accesses with a spinlock instead of a mutex by setting the fast_io flag.
I reported this ages ago.
I ended up removing regmap entirely, it just locks the machine solid and provides no useful functionality for the driver.
participants (5)
-
Ben Dooks -
Kuninori Morimoto -
Kuninori Morimoto -
Lars-Peter Clausen -
Laurent Pinchart