Message ID | 20181016030239.15449-1-anarsoul@gmail.com |
---|---|
Headers | show |
Series | Add support for audiocodec in Allwinner A64 | expand |
Hi, On Mon, Oct 15, 2018 at 08:02:30PM -0700, Vasily Khoruzhick wrote: > This series adds Allwinner A64 audiocodec support into sun4i-i2s, > sun8i-codec drivers, introduces new sun50i-codec-analog driver and enables > sound on Pine64, SoPine boards and Pinebook. > > I2S for audiocodec in A64 is different from other 3 I2S modules but > similar to one in A10, digital part of codec is compatible with A33 and > analog controls part is completely different from other SoCs - it shares > only few bits in few registers, so adding support for it into existing > sun8i-codec-analog would mean duplicating all the widgets, controls and > some routes and making it hard to read. Therefore it makes sense to > introduce new driver. > > v2: - Use simple-amplifier for speaker amp on Pinebook > - Rename sun50i-a64-i2s to sun50i-a64-codec-i2s to preserve compatible > string for other 3 I2S modules in A64 in case if there's any > incompatibility with H3 > v3: - renamed sunxi-adda-pr-regmap to sun8i-adda-pr-regmap > - use ilog2() to calculate reg value for LRCK div instead of using a > table > v4: - dts: don't use 'Mic' and 'Headset Mic' widgets from sun8i-codec, > define our board-level widgets instead. You also need to collect the tags that are given by the various people involved in that review :) Maxime
On Tue, Oct 16, 2018 at 12:13:26AM -0700, Vasily Khoruzhick wrote: > On Tue, Oct 16, 2018, 00:09 Maxime Ripard <[1]maxime.ripard@bootlin.com> wrote: > > Hi, > > On Mon, Oct 15, 2018 at 08:02:30PM -0700, Vasily Khoruzhick wrote: > > This series adds Allwinner A64 audiocodec support into sun4i-i2s, > > sun8i-codec drivers, introduces new sun50i-codec-analog driver and > enables > > sound on Pine64, SoPine boards and Pinebook. > > > > I2S for audiocodec in A64 is different from other 3 I2S modules but > > similar to one in A10, digital part of codec is compatible with A33 and > > analog controls part is completely different from other SoCs - it shares > > only few bits in few registers, so adding support for it into existing > > sun8i-codec-analog would mean duplicating all the widgets, controls and > > some routes and making it hard to read. Therefore it makes sense to > > introduce new driver. > > > > v2: - Use simple-amplifier for speaker amp on Pinebook > > - Rename sun50i-a64-i2s to sun50i-a64-codec-i2s to preserve > compatible > > string for other 3 I2S modules in A64 in case if there's any > > incompatibility with H3 > > v3: - renamed sunxi-adda-pr-regmap to sun8i-adda-pr-regmap > > - use ilog2() to calculate reg value for LRCK div instead of using a > > table > > v4: - dts: don't use 'Mic' and 'Headset Mic' widgets from sun8i-codec, > > define our board-level widgets instead. > > You also need to collect the tags that are given by the various people > involved in that review :) > > > My bad. Do you want me to send v5 with all the tags? Yep, please, thankS! Maxime
Hi! On Mon, Oct 15, 2018 at 08:02:32PM -0700, Vasily Khoruzhick wrote: > BCLK / LRCK ratio should be sample size * channels, but it was > hardcoded to 32 (0x1 is 32 as per A33 and A64 datasheets). > > Calculate it basing on sample size and number of channels. > > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com> > --- > sound/soc/sunxi/sun8i-codec.c | 26 +++++++++++++++++++++----- > 1 file changed, 21 insertions(+), 5 deletions(-) > > diff --git a/sound/soc/sunxi/sun8i-codec.c b/sound/soc/sunxi/sun8i-codec.c > index fb37dd927e33..2467fab94a19 100644 > --- a/sound/soc/sunxi/sun8i-codec.c > +++ b/sound/soc/sunxi/sun8i-codec.c > @@ -24,6 +24,7 @@ > #include <linux/io.h> > #include <linux/pm_runtime.h> > #include <linux/regmap.h> > +#include <linux/log2.h> > > #include <sound/pcm_params.h> > #include <sound/soc.h> > @@ -52,7 +53,6 @@ > #define SUN8I_AIF1CLK_CTRL_AIF1_LRCK_INV 13 > #define SUN8I_AIF1CLK_CTRL_AIF1_BCLK_DIV 9 > #define SUN8I_AIF1CLK_CTRL_AIF1_LRCK_DIV 6 > -#define SUN8I_AIF1CLK_CTRL_AIF1_LRCK_DIV_16 (1 << 6) > #define SUN8I_AIF1CLK_CTRL_AIF1_WORD_SIZ 4 > #define SUN8I_AIF1CLK_CTRL_AIF1_WORD_SIZ_16 (1 << 4) > #define SUN8I_AIF1CLK_CTRL_AIF1_DATA_FMT 2 > @@ -257,8 +257,8 @@ static int sun8i_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) > } > > struct sun8i_codec_clk_div { > - u8 div; > - u8 val; > + unsigned int div; > + u8 val; > }; Do we still need that type change? It looks like you're not using it anywhere in that patch. Thanks! Maxime
On Wednesday, October 17, 2018 12:12:48 AM PDT Maxime Ripard wrote: > Hi! > > On Mon, Oct 15, 2018 at 08:02:32PM -0700, Vasily Khoruzhick wrote: > > BCLK / LRCK ratio should be sample size * channels, but it was > > hardcoded to 32 (0x1 is 32 as per A33 and A64 datasheets). > > > > Calculate it basing on sample size and number of channels. > > > > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com> > > --- > > > > sound/soc/sunxi/sun8i-codec.c | 26 +++++++++++++++++++++----- > > 1 file changed, 21 insertions(+), 5 deletions(-) > > > > diff --git a/sound/soc/sunxi/sun8i-codec.c b/sound/soc/sunxi/sun8i-codec.c > > index fb37dd927e33..2467fab94a19 100644 > > --- a/sound/soc/sunxi/sun8i-codec.c > > +++ b/sound/soc/sunxi/sun8i-codec.c > > @@ -24,6 +24,7 @@ > > > > #include <linux/io.h> > > #include <linux/pm_runtime.h> > > #include <linux/regmap.h> > > > > +#include <linux/log2.h> > > > > #include <sound/pcm_params.h> > > #include <sound/soc.h> > > > > @@ -52,7 +53,6 @@ > > > > #define SUN8I_AIF1CLK_CTRL_AIF1_LRCK_INV 13 > > #define SUN8I_AIF1CLK_CTRL_AIF1_BCLK_DIV 9 > > #define SUN8I_AIF1CLK_CTRL_AIF1_LRCK_DIV 6 > > > > -#define SUN8I_AIF1CLK_CTRL_AIF1_LRCK_DIV_16 (1 << 6) > > > > #define SUN8I_AIF1CLK_CTRL_AIF1_WORD_SIZ 4 > > #define SUN8I_AIF1CLK_CTRL_AIF1_WORD_SIZ_16 (1 << 4) > > #define SUN8I_AIF1CLK_CTRL_AIF1_DATA_FMT 2 > > > > @@ -257,8 +257,8 @@ static int sun8i_set_fmt(struct snd_soc_dai *dai, > > unsigned int fmt)> > > } > > > > struct sun8i_codec_clk_div { > > > > - u8 div; > > - u8 val; > > + unsigned int div; > > + u8 val; > > > > }; > > Do we still need that type change? It looks like you're not using it > anywhere in that patch. We don't need it, I'll drop it for v5. > > Thanks! > Maxime