Re: [PATCH v2 3/3] ASoC: fsl_easrc: Add EASRC ASoC CPU DAI and platform drivers
Hi
Signed-off-by: Shengjiu Wang shengjiu.wang@nxp.com
sound/soc/fsl/Kconfig | 10 + sound/soc/fsl/Makefile | 2 + sound/soc/fsl/fsl_asrc_common.h | 1 + sound/soc/fsl/fsl_easrc.c | 2265 +++++++++++++++++++++++++++++++ sound/soc/fsl/fsl_easrc.h | 668 +++++++++ sound/soc/fsl/fsl_easrc_dma.c | 440 ++++++
I see a 90% similarity between fsl_asrc_dma and fsl_easrc_dma files. Would it be possible reuse the existing code? Could share structures from my point of view, just like it reuses "enum asrc_pair_index", I know differentiating "pair" and "context" is a big point here though.
A possible quick solution for that, off the top of my head, could be:
in fsl_asrc_common.h
struct fsl_asrc { .... }; struct fsl_asrc_pair { .... };in fsl_easrc.h
/* Renaming shared structures */ #define fsl_easrc fsl_asrc #define fsl_easrc_context fsl_asrc_pairMay be a good idea to see if others have some opinion too.
We need to modify the fsl_asrc and fsl_asrc_pair, let them To be used by both driver, also we need to put the specific Definition for each module to same struct, right?
+static const struct regmap_config fsl_easrc_regmap_config = {
.readable_reg = fsl_easrc_readable_reg,.volatile_reg = fsl_easrc_volatile_reg,.writeable_reg = fsl_easrc_writeable_reg,Can we use regmap_range and regmap_access_table?
Can the regmap_range support discontinuous registers? The reg_stride = 4.
Best regards Wang shengjiu
On Mon, Feb 24, 2020 at 08:53:25AM +0000, S.j. Wang wrote:
Hi
Signed-off-by: Shengjiu Wang shengjiu.wang@nxp.com
sound/soc/fsl/Kconfig | 10 + sound/soc/fsl/Makefile | 2 + sound/soc/fsl/fsl_asrc_common.h | 1 + sound/soc/fsl/fsl_easrc.c | 2265 +++++++++++++++++++++++++++++++ sound/soc/fsl/fsl_easrc.h | 668 +++++++++ sound/soc/fsl/fsl_easrc_dma.c | 440 ++++++
I see a 90% similarity between fsl_asrc_dma and fsl_easrc_dma files. Would it be possible reuse the existing code? Could share structures from my point of view, just like it reuses "enum asrc_pair_index", I know differentiating "pair" and "context" is a big point here though.
A possible quick solution for that, off the top of my head, could be:
in fsl_asrc_common.h
struct fsl_asrc { .... }; struct fsl_asrc_pair { .... };in fsl_easrc.h
/* Renaming shared structures */ #define fsl_easrc fsl_asrc #define fsl_easrc_context fsl_asrc_pairMay be a good idea to see if others have some opinion too.
We need to modify the fsl_asrc and fsl_asrc_pair, let them To be used by both driver, also we need to put the specific Definition for each module to same struct, right?
Yea. A merged structure if that doesn't look that bad. I see most of the fields in struct fsl_asrc are being reused by in fsl_easrc.
+static const struct regmap_config fsl_easrc_regmap_config = {
.readable_reg = fsl_easrc_readable_reg,.volatile_reg = fsl_easrc_volatile_reg,.writeable_reg = fsl_easrc_writeable_reg,Can we use regmap_range and regmap_access_table?
Can the regmap_range support discontinuous registers? The reg_stride = 4.
I think it does. Giving an example here: https://github.com/torvalds/linux/blob/master/drivers/mfd/da9063-i2c.c
On Tue, Feb 25, 2020 at 4:05 PM Nicolin Chen nicoleotsuka@gmail.com wrote:
On Mon, Feb 24, 2020 at 08:53:25AM +0000, S.j. Wang wrote:
Hi
Signed-off-by: Shengjiu Wang shengjiu.wang@nxp.com
sound/soc/fsl/Kconfig | 10 + sound/soc/fsl/Makefile | 2 + sound/soc/fsl/fsl_asrc_common.h | 1 + sound/soc/fsl/fsl_easrc.c | 2265 +++++++++++++++++++++++++++++++ sound/soc/fsl/fsl_easrc.h | 668 +++++++++ sound/soc/fsl/fsl_easrc_dma.c | 440 ++++++
I see a 90% similarity between fsl_asrc_dma and fsl_easrc_dma files. Would it be possible reuse the existing code? Could share structures from my point of view, just like it reuses "enum asrc_pair_index", I know differentiating "pair" and "context" is a big point here though.
A possible quick solution for that, off the top of my head, could be:
in fsl_asrc_common.h
struct fsl_asrc { .... }; struct fsl_asrc_pair { .... };in fsl_easrc.h
/* Renaming shared structures */ #define fsl_easrc fsl_asrc #define fsl_easrc_context fsl_asrc_pairMay be a good idea to see if others have some opinion too.
We need to modify the fsl_asrc and fsl_asrc_pair, let them To be used by both driver, also we need to put the specific Definition for each module to same struct, right?
Yea. A merged structure if that doesn't look that bad. I see most of the fields in struct fsl_asrc are being reused by in fsl_easrc.
+static const struct regmap_config fsl_easrc_regmap_config = {
.readable_reg = fsl_easrc_readable_reg,.volatile_reg = fsl_easrc_volatile_reg,.writeable_reg = fsl_easrc_writeable_reg,Can we use regmap_range and regmap_access_table?
Can the regmap_range support discontinuous registers? The reg_stride = 4.
I think it does. Giving an example here: https://github.com/torvalds/linux/blob/master/drivers/mfd/da9063-i2c.c
The register in this i2c driver are continuous, from 0x00, 0x01, 0x02...
But our case is 0x00, 0x04, 0x08, does it work?
best regards wang shengjiu
On Wed, Feb 26, 2020 at 09:51:39AM +0800, Shengjiu Wang wrote:
+static const struct regmap_config fsl_easrc_regmap_config = {
.readable_reg = fsl_easrc_readable_reg,.volatile_reg = fsl_easrc_volatile_reg,.writeable_reg = fsl_easrc_writeable_reg,Can we use regmap_range and regmap_access_table?
Can the regmap_range support discontinuous registers? The reg_stride = 4.
I think it does. Giving an example here: https://github.com/torvalds/linux/blob/master/drivers/mfd/da9063-i2c.c
The register in this i2c driver are continuous, from 0x00, 0x01, 0x02...
But our case is 0x00, 0x04, 0x08, does it work?
Ah...I see your point now. I am not very sure -- have only used in I2C drivers. You can ignore if it doesn't likely work for us.
participants (3)
-
Nicolin Chen -
S.j. Wang -
Shengjiu Wang