| Message ID | 20190419191730.9437-3-peron.clem@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Allwinner H6 SPDIF support | expand |
On Fri, Apr 19, 2019 at 09:17:27PM +0200, Clément Péron wrote: > Allwinner H6 has a different mapping for the fifo register controller. > > Actually only the fifo tx flush bit is used. > > Add a new quirk to know the correct fifo tx flush bit. > > Signed-off-by: Clément Péron <peron.clem@gmail.com> > --- > sound/soc/sunxi/sun4i-spdif.c | 42 ++++++++++++++++++++++++++++++----- > 1 file changed, 36 insertions(+), 6 deletions(-) > > diff --git a/sound/soc/sunxi/sun4i-spdif.c b/sound/soc/sunxi/sun4i-spdif.c > index b4af4aabead1..19e4bf9caa24 100644 > --- a/sound/soc/sunxi/sun4i-spdif.c > +++ b/sound/soc/sunxi/sun4i-spdif.c > @@ -75,6 +75,18 @@ > #define SUN4I_SPDIF_FCTL_RXOM(v) ((v) << 0) > #define SUN4I_SPDIF_FCTL_RXOM_MASK GENMASK(1, 0) > > +#define SUN50I_H6_SPDIF_FCTL (0x14) > + #define SUN50I_H6_SPDIF_FCTL_HUB_EN BIT(31) > + #define SUN50I_H6_SPDIF_FCTL_FTX BIT(30) > + #define SUN50I_H6_SPDIF_FCTL_FRX BIT(29) > + #define SUN50I_H6_SPDIF_FCTL_TXTL(v) ((v) << 12) > + #define SUN50I_H6_SPDIF_FCTL_TXTL_MASK GENMASK(19, 12) > + #define SUN50I_H6_SPDIF_FCTL_RXTL(v) ((v) << 4) > + #define SUN50I_H6_SPDIF_FCTL_RXTL_MASK GENMASK(10, 4) > + #define SUN50I_H6_SPDIF_FCTL_TXIM BIT(2) > + #define SUN50I_H6_SPDIF_FCTL_RXOM(v) ((v) << 0) > + #define SUN50I_H6_SPDIF_FCTL_RXOM_MASK GENMASK(1, 0) > + > #define SUN4I_SPDIF_FSTA (0x18) > #define SUN4I_SPDIF_FSTA_TXE BIT(14) > #define SUN4I_SPDIF_FSTA_TXECNTSHT (8) > @@ -169,16 +181,25 @@ struct sun4i_spdif_dev { > struct snd_soc_dai_driver cpu_dai_drv; > struct regmap *regmap; > struct snd_dmaengine_dai_dma_data dma_params_tx; > + const struct sun4i_spdif_quirks *quirks; I guess this will generate a warning since the structure hasn't been defined yet? > +}; > + > +struct sun4i_spdif_quirks { > + unsigned int reg_dac_txdata; /* TX FIFO offset for DMA config */ > + unsigned int reg_fctl_ftx; /* TX FIFO flush bitmask */ > + bool has_reset; You don't really need to move it around, you can just add the structure prototype. If you do want to move it around, then please do so in a separate patch > }; > > static void sun4i_spdif_configure(struct sun4i_spdif_dev *host) > { > + const struct sun4i_spdif_quirks *quirks = host->quirks; > + > /* soft reset SPDIF */ > regmap_write(host->regmap, SUN4I_SPDIF_CTL, SUN4I_SPDIF_CTL_RESET); > > /* flush TX FIFO */ > regmap_update_bits(host->regmap, SUN4I_SPDIF_FCTL, > - SUN4I_SPDIF_FCTL_FTX, SUN4I_SPDIF_FCTL_FTX); > + quirks->reg_fctl_ftx, quirks->reg_fctl_ftx); > > /* clear TX counter */ > regmap_write(host->regmap, SUN4I_SPDIF_TXCNT, 0); > @@ -405,22 +426,26 @@ static struct snd_soc_dai_driver sun4i_spdif_dai = { > .name = "spdif", > }; > > -struct sun4i_spdif_quirks { > - unsigned int reg_dac_txdata; /* TX FIFO offset for DMA config */ > - bool has_reset; > -}; > - > static const struct sun4i_spdif_quirks sun4i_a10_spdif_quirks = { > .reg_dac_txdata = SUN4I_SPDIF_TXFIFO, > + .reg_fctl_ftx = SUN4I_SPDIF_FCTL_FTX, > }; > > static const struct sun4i_spdif_quirks sun6i_a31_spdif_quirks = { > .reg_dac_txdata = SUN4I_SPDIF_TXFIFO, > + .reg_fctl_ftx = SUN4I_SPDIF_FCTL_FTX, > .has_reset = true, > }; > > static const struct sun4i_spdif_quirks sun8i_h3_spdif_quirks = { > .reg_dac_txdata = SUN8I_SPDIF_TXFIFO, > + .reg_fctl_ftx = SUN4I_SPDIF_FCTL_FTX, > + .has_reset = true, > +}; > > +static const struct sun4i_spdif_quirks sun50i_h6_spdif_quirks = { > + .reg_dac_txdata = SUN8I_SPDIF_TXFIFO, > + .reg_fctl_ftx = SUN50I_H6_SPDIF_FCTL_FTX, > .has_reset = true, The reg_dac_txdata and reg_fctl_ftx changes here should also be part of a separate patch. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Hi Maxime, On Thu, 2 May 2019 at 10:25, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > On Fri, Apr 19, 2019 at 09:17:27PM +0200, Clément Péron wrote: > > Allwinner H6 has a different mapping for the fifo register controller. > > > > Actually only the fifo tx flush bit is used. > > > > Add a new quirk to know the correct fifo tx flush bit. > > > > Signed-off-by: Clément Péron <peron.clem@gmail.com> > > --- > > sound/soc/sunxi/sun4i-spdif.c | 42 ++++++++++++++++++++++++++++++----- > > 1 file changed, 36 insertions(+), 6 deletions(-) > > > > diff --git a/sound/soc/sunxi/sun4i-spdif.c b/sound/soc/sunxi/sun4i-spdif.c > > index b4af4aabead1..19e4bf9caa24 100644 > > --- a/sound/soc/sunxi/sun4i-spdif.c > > +++ b/sound/soc/sunxi/sun4i-spdif.c > > @@ -75,6 +75,18 @@ > > #define SUN4I_SPDIF_FCTL_RXOM(v) ((v) << 0) > > #define SUN4I_SPDIF_FCTL_RXOM_MASK GENMASK(1, 0) > > > > +#define SUN50I_H6_SPDIF_FCTL (0x14) > > + #define SUN50I_H6_SPDIF_FCTL_HUB_EN BIT(31) > > + #define SUN50I_H6_SPDIF_FCTL_FTX BIT(30) > > + #define SUN50I_H6_SPDIF_FCTL_FRX BIT(29) > > + #define SUN50I_H6_SPDIF_FCTL_TXTL(v) ((v) << 12) > > + #define SUN50I_H6_SPDIF_FCTL_TXTL_MASK GENMASK(19, 12) > > + #define SUN50I_H6_SPDIF_FCTL_RXTL(v) ((v) << 4) > > + #define SUN50I_H6_SPDIF_FCTL_RXTL_MASK GENMASK(10, 4) > > + #define SUN50I_H6_SPDIF_FCTL_TXIM BIT(2) > > + #define SUN50I_H6_SPDIF_FCTL_RXOM(v) ((v) << 0) > > + #define SUN50I_H6_SPDIF_FCTL_RXOM_MASK GENMASK(1, 0) > > + > > #define SUN4I_SPDIF_FSTA (0x18) > > #define SUN4I_SPDIF_FSTA_TXE BIT(14) > > #define SUN4I_SPDIF_FSTA_TXECNTSHT (8) > > @@ -169,16 +181,25 @@ struct sun4i_spdif_dev { > > struct snd_soc_dai_driver cpu_dai_drv; > > struct regmap *regmap; > > struct snd_dmaengine_dai_dma_data dma_params_tx; > > + const struct sun4i_spdif_quirks *quirks; > > I guess this will generate a warning since the structure hasn't been > defined yet? It's a pointer to a structure so no warning from the compiler. > > > +}; > > + > > +struct sun4i_spdif_quirks { > > + unsigned int reg_dac_txdata; /* TX FIFO offset for DMA config */ > > + unsigned int reg_fctl_ftx; /* TX FIFO flush bitmask */ > > + bool has_reset; > > You don't really need to move it around, you can just add the > structure prototype. > > If you do want to move it around, then please do so in a separate patch I have choose to move it to follow what is done in the sun4i-i2s. I will put it in a separate patch and make the comment a bit more proper. > > > }; > > > > static void sun4i_spdif_configure(struct sun4i_spdif_dev *host) > > { > > + const struct sun4i_spdif_quirks *quirks = host->quirks; > > + > > /* soft reset SPDIF */ > > regmap_write(host->regmap, SUN4I_SPDIF_CTL, SUN4I_SPDIF_CTL_RESET); > > > > /* flush TX FIFO */ > > regmap_update_bits(host->regmap, SUN4I_SPDIF_FCTL, > > - SUN4I_SPDIF_FCTL_FTX, SUN4I_SPDIF_FCTL_FTX); > > + quirks->reg_fctl_ftx, quirks->reg_fctl_ftx); > > > > /* clear TX counter */ > > regmap_write(host->regmap, SUN4I_SPDIF_TXCNT, 0); > > @@ -405,22 +426,26 @@ static struct snd_soc_dai_driver sun4i_spdif_dai = { > > .name = "spdif", > > }; > > > > -struct sun4i_spdif_quirks { > > - unsigned int reg_dac_txdata; /* TX FIFO offset for DMA config */ > > - bool has_reset; > > -}; > > - > > static const struct sun4i_spdif_quirks sun4i_a10_spdif_quirks = { > > .reg_dac_txdata = SUN4I_SPDIF_TXFIFO, > > + .reg_fctl_ftx = SUN4I_SPDIF_FCTL_FTX, > > }; > > > > static const struct sun4i_spdif_quirks sun6i_a31_spdif_quirks = { > > .reg_dac_txdata = SUN4I_SPDIF_TXFIFO, > > + .reg_fctl_ftx = SUN4I_SPDIF_FCTL_FTX, > > .has_reset = true, > > }; > > > > static const struct sun4i_spdif_quirks sun8i_h3_spdif_quirks = { > > .reg_dac_txdata = SUN8I_SPDIF_TXFIFO, > > + .reg_fctl_ftx = SUN4I_SPDIF_FCTL_FTX, > > + .has_reset = true, > > +}; > > > > +static const struct sun4i_spdif_quirks sun50i_h6_spdif_quirks = { > > + .reg_dac_txdata = SUN8I_SPDIF_TXFIFO, > > + .reg_fctl_ftx = SUN50I_H6_SPDIF_FCTL_FTX, > > .has_reset = true, > > The reg_dac_txdata and reg_fctl_ftx changes here should also be part > of a separate patch. You mean the reg_fctl_ftx quirk and the H6 introduction should be split ? Thanks for the review, Clement > > Maxime > > -- > Maxime Ripard, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com
On Thu, May 02, 2019 at 11:39:24AM +0200, Clément Péron wrote: > > > @@ -169,16 +181,25 @@ struct sun4i_spdif_dev { > > > struct snd_soc_dai_driver cpu_dai_drv; > > > struct regmap *regmap; > > > struct snd_dmaengine_dai_dma_data dma_params_tx; > > > + const struct sun4i_spdif_quirks *quirks; > > > > I guess this will generate a warning since the structure hasn't been > > defined yet? > > It's a pointer to a structure so no warning from the compiler. Damn, I was convinced just declaring a pointer to a structure would result to a gcc warning. Nevermind then. > > > @@ -405,22 +426,26 @@ static struct snd_soc_dai_driver sun4i_spdif_dai = { > > > .name = "spdif", > > > }; > > > > > > -struct sun4i_spdif_quirks { > > > - unsigned int reg_dac_txdata; /* TX FIFO offset for DMA config */ > > > - bool has_reset; > > > -}; > > > - > > > static const struct sun4i_spdif_quirks sun4i_a10_spdif_quirks = { > > > .reg_dac_txdata = SUN4I_SPDIF_TXFIFO, > > > + .reg_fctl_ftx = SUN4I_SPDIF_FCTL_FTX, > > > }; > > > > > > static const struct sun4i_spdif_quirks sun6i_a31_spdif_quirks = { > > > .reg_dac_txdata = SUN4I_SPDIF_TXFIFO, > > > + .reg_fctl_ftx = SUN4I_SPDIF_FCTL_FTX, > > > .has_reset = true, > > > }; > > > > > > static const struct sun4i_spdif_quirks sun8i_h3_spdif_quirks = { > > > .reg_dac_txdata = SUN8I_SPDIF_TXFIFO, > > > + .reg_fctl_ftx = SUN4I_SPDIF_FCTL_FTX, > > > + .has_reset = true, > > > +}; > > > > > > +static const struct sun4i_spdif_quirks sun50i_h6_spdif_quirks = { > > > + .reg_dac_txdata = SUN8I_SPDIF_TXFIFO, > > > + .reg_fctl_ftx = SUN50I_H6_SPDIF_FCTL_FTX, > > > .has_reset = true, > > > > The reg_dac_txdata and reg_fctl_ftx changes here should also be part > > of a separate patch. > > You mean the reg_fctl_ftx quirk and the H6 introduction should be split ? Yep Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
diff --git a/sound/soc/sunxi/sun4i-spdif.c b/sound/soc/sunxi/sun4i-spdif.c index b4af4aabead1..19e4bf9caa24 100644 --- a/sound/soc/sunxi/sun4i-spdif.c +++ b/sound/soc/sunxi/sun4i-spdif.c @@ -75,6 +75,18 @@ #define SUN4I_SPDIF_FCTL_RXOM(v) ((v) << 0) #define SUN4I_SPDIF_FCTL_RXOM_MASK GENMASK(1, 0) +#define SUN50I_H6_SPDIF_FCTL (0x14) + #define SUN50I_H6_SPDIF_FCTL_HUB_EN BIT(31) + #define SUN50I_H6_SPDIF_FCTL_FTX BIT(30) + #define SUN50I_H6_SPDIF_FCTL_FRX BIT(29) + #define SUN50I_H6_SPDIF_FCTL_TXTL(v) ((v) << 12) + #define SUN50I_H6_SPDIF_FCTL_TXTL_MASK GENMASK(19, 12) + #define SUN50I_H6_SPDIF_FCTL_RXTL(v) ((v) << 4) + #define SUN50I_H6_SPDIF_FCTL_RXTL_MASK GENMASK(10, 4) + #define SUN50I_H6_SPDIF_FCTL_TXIM BIT(2) + #define SUN50I_H6_SPDIF_FCTL_RXOM(v) ((v) << 0) + #define SUN50I_H6_SPDIF_FCTL_RXOM_MASK GENMASK(1, 0) + #define SUN4I_SPDIF_FSTA (0x18) #define SUN4I_SPDIF_FSTA_TXE BIT(14) #define SUN4I_SPDIF_FSTA_TXECNTSHT (8) @@ -169,16 +181,25 @@ struct sun4i_spdif_dev { struct snd_soc_dai_driver cpu_dai_drv; struct regmap *regmap; struct snd_dmaengine_dai_dma_data dma_params_tx; + const struct sun4i_spdif_quirks *quirks; +}; + +struct sun4i_spdif_quirks { + unsigned int reg_dac_txdata; /* TX FIFO offset for DMA config */ + unsigned int reg_fctl_ftx; /* TX FIFO flush bitmask */ + bool has_reset; }; static void sun4i_spdif_configure(struct sun4i_spdif_dev *host) { + const struct sun4i_spdif_quirks *quirks = host->quirks; + /* soft reset SPDIF */ regmap_write(host->regmap, SUN4I_SPDIF_CTL, SUN4I_SPDIF_CTL_RESET); /* flush TX FIFO */ regmap_update_bits(host->regmap, SUN4I_SPDIF_FCTL, - SUN4I_SPDIF_FCTL_FTX, SUN4I_SPDIF_FCTL_FTX); + quirks->reg_fctl_ftx, quirks->reg_fctl_ftx); /* clear TX counter */ regmap_write(host->regmap, SUN4I_SPDIF_TXCNT, 0); @@ -405,22 +426,26 @@ static struct snd_soc_dai_driver sun4i_spdif_dai = { .name = "spdif", }; -struct sun4i_spdif_quirks { - unsigned int reg_dac_txdata; /* TX FIFO offset for DMA config */ - bool has_reset; -}; - static const struct sun4i_spdif_quirks sun4i_a10_spdif_quirks = { .reg_dac_txdata = SUN4I_SPDIF_TXFIFO, + .reg_fctl_ftx = SUN4I_SPDIF_FCTL_FTX, }; static const struct sun4i_spdif_quirks sun6i_a31_spdif_quirks = { .reg_dac_txdata = SUN4I_SPDIF_TXFIFO, + .reg_fctl_ftx = SUN4I_SPDIF_FCTL_FTX, .has_reset = true, }; static const struct sun4i_spdif_quirks sun8i_h3_spdif_quirks = { .reg_dac_txdata = SUN8I_SPDIF_TXFIFO, + .reg_fctl_ftx = SUN4I_SPDIF_FCTL_FTX, + .has_reset = true, +}; + +static const struct sun4i_spdif_quirks sun50i_h6_spdif_quirks = { + .reg_dac_txdata = SUN8I_SPDIF_TXFIFO, + .reg_fctl_ftx = SUN50I_H6_SPDIF_FCTL_FTX, .has_reset = true, }; @@ -437,6 +462,10 @@ static const struct of_device_id sun4i_spdif_of_match[] = { .compatible = "allwinner,sun8i-h3-spdif", .data = &sun8i_h3_spdif_quirks, }, + { + .compatible = "allwinner,sun50i-h6-spdif", + .data = &sun50i_h6_spdif_quirks, + }, { /* sentinel */ } }; MODULE_DEVICE_TABLE(of, sun4i_spdif_of_match); @@ -501,6 +530,7 @@ static int sun4i_spdif_probe(struct platform_device *pdev) dev_err(&pdev->dev, "Failed to determine the quirks to use\n"); return -ENODEV; } + host->quirks = quirks; host->regmap = devm_regmap_init_mmio(&pdev->dev, base, &sun4i_spdif_regmap_config);
Allwinner H6 has a different mapping for the fifo register controller. Actually only the fifo tx flush bit is used. Add a new quirk to know the correct fifo tx flush bit. Signed-off-by: Clément Péron <peron.clem@gmail.com> --- sound/soc/sunxi/sun4i-spdif.c | 42 ++++++++++++++++++++++++++++++----- 1 file changed, 36 insertions(+), 6 deletions(-)