| Message ID | 1459931902-77324-1-git-send-email-sugar.zhang@rock-chips.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Apr 06, 2016 at 04:38:22PM +0800, Sugar Zhang wrote: > There are 3 i2s sdio pins, which iomux mode is as follows: s/i2s sdio/I2S\/SDIO muxed/ > > - sdi3_sdo1 > - sdi2_sdo2 > - sdi1_sdo3 > > we need to configure these pins' iomux mode via the GRF register > when use multi channel playback/capture. Why not a pinctrl binding here? If you want SDIO mode, then you need similar code in the SDIO driver (or just rely on default mode). > > Signed-off-by: Sugar Zhang <sugar.zhang@rock-chips.com> > --- > > .../devicetree/bindings/sound/rockchip-i2s.txt | 5 +++ > sound/soc/rockchip/rockchip_i2s.c | 39 +++++++++++++++++++++- > sound/soc/rockchip/rockchip_i2s.h | 8 +++++ > 3 files changed, 51 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/sound/rockchip-i2s.txt b/Documentation/devicetree/bindings/sound/rockchip-i2s.txt > index 6e86d8a..ad72a7d 100644 > --- a/Documentation/devicetree/bindings/sound/rockchip-i2s.txt > +++ b/Documentation/devicetree/bindings/sound/rockchip-i2s.txt > @@ -23,6 +23,11 @@ Required properties: > - rockchip,playback-channels: max playback channels, if not set, 8 channels default. > - rockchip,capture-channels: max capture channels, if not set, 2 channels default. > > +Required properties for controller which support multi channels playback/capture: > + > +- rockchip,grf: Should be phandle/offset pair. the phandle of the syscon node for GRF register, Wrap your lines at less than 80 chars. > + and the offset of the GRF for control register. > + > Example for rk3288 I2S controller: > > i2s@ff890000 {
Hi Rob, On 4/8/2016 01:58, Rob Herring Wrote: > On Wed, Apr 06, 2016 at 04:38:22PM +0800, Sugar Zhang wrote: >> There are 3 i2s sdio pins, which iomux mode is as follows: > > s/i2s sdio/I2S\/SDIO muxed/ > >> >> - sdi3_sdo1 >> - sdi2_sdo2 >> - sdi1_sdo3 >> >> we need to configure these pins' iomux mode via the GRF register >> when use multi channel playback/capture. > > Why not a pinctrl binding here? If you want SDIO mode, then you need > similar code in the SDIO driver (or just rely on default mode). > here, i2s_sdio means i2s_sdi/i2s_sdo, not means SDIO bus interface. for example: gpio3d[6] iomux select: 2'b 00: gpio 2'b 01: i2s0_sdi3sdo1 2'b 10: xxx 2'b 11: xxx we configure the i2s_sdi3sdo1 mode via the pinctrl, and then need another grf register to configure it is used for i2s0_sdi3 or i2s0_sdo1. >> >> Signed-off-by: Sugar Zhang <sugar.zhang@rock-chips.com> >> --- >> >> .../devicetree/bindings/sound/rockchip-i2s.txt | 5 +++ >> sound/soc/rockchip/rockchip_i2s.c | 39 +++++++++++++++++++++- >> sound/soc/rockchip/rockchip_i2s.h | 8 +++++ >> 3 files changed, 51 insertions(+), 1 deletion(-) >> >> diff --git a/Documentation/devicetree/bindings/sound/rockchip-i2s.txt b/Documentation/devicetree/bindings/sound/rockchip-i2s.txt >> index 6e86d8a..ad72a7d 100644 >> --- a/Documentation/devicetree/bindings/sound/rockchip-i2s.txt >> +++ b/Documentation/devicetree/bindings/sound/rockchip-i2s.txt >> @@ -23,6 +23,11 @@ Required properties: >> - rockchip,playback-channels: max playback channels, if not set, 8 channels default. >> - rockchip,capture-channels: max capture channels, if not set, 2 channels default. >> >> +Required properties for controller which support multi channels playback/capture: >> + >> +- rockchip,grf: Should be phandle/offset pair. the phandle of the syscon node for GRF register, > > Wrap your lines at less than 80 chars. Ok, will fix. > >> + and the offset of the GRF for control register. >> + >> Example for rk3288 I2S controller: >> >> i2s@ff890000 { > > _______________________________________________ > Linux-rockchip mailing list > Linux-rockchip@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-rockchip > > >
Hi, Am Mittwoch, 6. April 2016, 16:38:22 schrieb Sugar Zhang: > There are 3 i2s sdio pins, which iomux mode is as follows: > > - sdi3_sdo1 > - sdi2_sdo2 > - sdi1_sdo3 > > we need to configure these pins' iomux mode via the GRF register > when use multi channel playback/capture. > > Signed-off-by: Sugar Zhang <sugar.zhang@rock-chips.com> > --- > > .../devicetree/bindings/sound/rockchip-i2s.txt | 5 +++ > sound/soc/rockchip/rockchip_i2s.c | 39 > +++++++++++++++++++++- sound/soc/rockchip/rockchip_i2s.h > | 8 +++++ > 3 files changed, 51 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/sound/rockchip-i2s.txt > b/Documentation/devicetree/bindings/sound/rockchip-i2s.txt index > 6e86d8a..ad72a7d 100644 > --- a/Documentation/devicetree/bindings/sound/rockchip-i2s.txt > +++ b/Documentation/devicetree/bindings/sound/rockchip-i2s.txt > @@ -23,6 +23,11 @@ Required properties: > - rockchip,playback-channels: max playback channels, if not set, 8 > channels default. - rockchip,capture-channels: max capture channels, if > not set, 2 channels default. > > +Required properties for controller which support multi channels > playback/capture: + > +- rockchip,grf: Should be phandle/offset pair. the phandle of the syscon > node for GRF register, + and the offset of the GRF for control register. I think I'd like it more to use the generic grf-binding we always use everwhere else (just the phandle without any offset) and keep the actual offset in the driver on a per-soc basis. That way rockchip,grf stays consistent over all users. We already have the per-soc compatible values, so it should a easy to add a .data element with the necessary offset-information. > + > Example for rk3288 I2S controller: > > i2s@ff890000 { > diff --git a/sound/soc/rockchip/rockchip_i2s.c > b/sound/soc/rockchip/rockchip_i2s.c index 2f8e204..bc72780 100644 > --- a/sound/soc/rockchip/rockchip_i2s.c > +++ b/sound/soc/rockchip/rockchip_i2s.c [...] > @@ -478,6 +504,18 @@ static int rockchip_i2s_probe(struct platform_device > *pdev) return -ENOMEM; > } > > + i2s->dev = &pdev->dev; > + > + i2s->grf = syscon_regmap_lookup_by_phandle(node, "rockchip,grf"); > + if (!IS_ERR(i2s->grf)) { > + ret = of_property_read_u32_index(node, "rockchip,grf", > + 1, &i2s->iocfg_reg); > + if (ret) { > + dev_err(&pdev->dev, "Can't get iocfg_reg offset\n"); > + return ret; > + } > + } > + as said in the binding part, please use the generic grf handling and get the io-offset from per-soc devicetree data in the driver itself. > /* try to prepare related clocks */ > i2s->hclk = devm_clk_get(&pdev->dev, "i2s_hclk"); > if (IS_ERR(i2s->hclk)) { > @@ -517,7 +555,6 @@ static int rockchip_i2s_probe(struct platform_device > *pdev) i2s->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES; > i2s->capture_dma_data.maxburst = 4; > > - i2s->dev = &pdev->dev; > dev_set_drvdata(&pdev->dev, i2s); > > pm_runtime_enable(&pdev->dev); > diff --git a/sound/soc/rockchip/rockchip_i2s.h > b/sound/soc/rockchip/rockchip_i2s.h index dc6e2c7..9a6aabf 100644 > --- a/sound/soc/rockchip/rockchip_i2s.h > +++ b/sound/soc/rockchip/rockchip_i2s.h > @@ -236,4 +236,12 @@ enum { > #define I2S_TXDR (0x0024) > #define I2S_RXDR (0x0028) > > +/* io direction cfg register */ > +#define I2S_IO_DIRECTION_SHIFT 11 this setting is sitting in GRF_SOC_CON8 on the rk3399. That is part of the very volatile register set where settings move around a lot for each soc. So if we're having the io-offset in per-soc data, we can easily put the shift into it as well. > +#define I2S_IO_DIRECTION_MASK (7 << I2S_IO_DIRECTION_SHIFT) > +#define I2S_IO_8CH_OUT_2CH_IN (0 << I2S_IO_DIRECTION_SHIFT) > +#define I2S_IO_6CH_OUT_4CH_IN (1 << I2S_IO_DIRECTION_SHIFT) > +#define I2S_IO_4CH_OUT_6CH_IN (3 << I2S_IO_DIRECTION_SHIFT) > +#define I2S_IO_2CH_OUT_8CH_IN (7 << I2S_IO_DIRECTION_SHIFT) Keep the settings without the shift here and do the shift dynamically with the per-soc setting when changing the setting. Thanks Heiko
Hi Heiko, Got it, thanks for your advice, these will be done in next version. On 4/9/2016 01:14, Heiko Stuebner Wrote: > Hi, > > Am Mittwoch, 6. April 2016, 16:38:22 schrieb Sugar Zhang: >> There are 3 i2s sdio pins, which iomux mode is as follows: >> >> - sdi3_sdo1 >> - sdi2_sdo2 >> - sdi1_sdo3 >> >> we need to configure these pins' iomux mode via the GRF register >> when use multi channel playback/capture. >> >> Signed-off-by: Sugar Zhang <sugar.zhang@rock-chips.com> >> --- >> >> .../devicetree/bindings/sound/rockchip-i2s.txt | 5 +++ >> sound/soc/rockchip/rockchip_i2s.c | 39 >> +++++++++++++++++++++- sound/soc/rockchip/rockchip_i2s.h >> | 8 +++++ >> 3 files changed, 51 insertions(+), 1 deletion(-) >> >> diff --git a/Documentation/devicetree/bindings/sound/rockchip-i2s.txt >> b/Documentation/devicetree/bindings/sound/rockchip-i2s.txt index >> 6e86d8a..ad72a7d 100644 >> --- a/Documentation/devicetree/bindings/sound/rockchip-i2s.txt >> +++ b/Documentation/devicetree/bindings/sound/rockchip-i2s.txt >> @@ -23,6 +23,11 @@ Required properties: >> - rockchip,playback-channels: max playback channels, if not set, 8 >> channels default. - rockchip,capture-channels: max capture channels, if >> not set, 2 channels default. >> >> +Required properties for controller which support multi channels >> playback/capture: + >> +- rockchip,grf: Should be phandle/offset pair. the phandle of the syscon >> node for GRF register, + and the offset of the GRF for control register. > > I think I'd like it more to use the generic grf-binding we always use > everwhere else (just the phandle without any offset) and keep the actual > offset in the driver on a per-soc basis. > > That way rockchip,grf stays consistent over all users. > > We already have the per-soc compatible values, so it should a easy to add a > .data element with the necessary offset-information. > >> + >> Example for rk3288 I2S controller: >> >> i2s@ff890000 { >> diff --git a/sound/soc/rockchip/rockchip_i2s.c >> b/sound/soc/rockchip/rockchip_i2s.c index 2f8e204..bc72780 100644 >> --- a/sound/soc/rockchip/rockchip_i2s.c >> +++ b/sound/soc/rockchip/rockchip_i2s.c > > [...] > >> @@ -478,6 +504,18 @@ static int rockchip_i2s_probe(struct platform_device >> *pdev) return -ENOMEM; >> } >> >> + i2s->dev = &pdev->dev; >> + >> + i2s->grf = syscon_regmap_lookup_by_phandle(node, "rockchip,grf"); >> + if (!IS_ERR(i2s->grf)) { >> + ret = of_property_read_u32_index(node, "rockchip,grf", >> + 1, &i2s->iocfg_reg); >> + if (ret) { >> + dev_err(&pdev->dev, "Can't get iocfg_reg offset\n"); >> + return ret; >> + } >> + } >> + > > as said in the binding part, please use the generic grf handling and get the > io-offset from per-soc devicetree data in the driver itself. > > >> /* try to prepare related clocks */ >> i2s->hclk = devm_clk_get(&pdev->dev, "i2s_hclk"); >> if (IS_ERR(i2s->hclk)) { >> @@ -517,7 +555,6 @@ static int rockchip_i2s_probe(struct platform_device >> *pdev) i2s->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES; >> i2s->capture_dma_data.maxburst = 4; >> >> - i2s->dev = &pdev->dev; >> dev_set_drvdata(&pdev->dev, i2s); >> >> pm_runtime_enable(&pdev->dev); >> diff --git a/sound/soc/rockchip/rockchip_i2s.h >> b/sound/soc/rockchip/rockchip_i2s.h index dc6e2c7..9a6aabf 100644 >> --- a/sound/soc/rockchip/rockchip_i2s.h >> +++ b/sound/soc/rockchip/rockchip_i2s.h >> @@ -236,4 +236,12 @@ enum { >> #define I2S_TXDR (0x0024) >> #define I2S_RXDR (0x0028) >> >> +/* io direction cfg register */ >> +#define I2S_IO_DIRECTION_SHIFT 11 > > this setting is sitting in GRF_SOC_CON8 on the rk3399. That is part of the > very volatile register set where settings move around a lot for each soc. > > So if we're having the io-offset in per-soc data, we can easily put the > shift into it as well. > >> +#define I2S_IO_DIRECTION_MASK (7 << I2S_IO_DIRECTION_SHIFT) >> +#define I2S_IO_8CH_OUT_2CH_IN (0 << I2S_IO_DIRECTION_SHIFT) >> +#define I2S_IO_6CH_OUT_4CH_IN (1 << I2S_IO_DIRECTION_SHIFT) >> +#define I2S_IO_4CH_OUT_6CH_IN (3 << I2S_IO_DIRECTION_SHIFT) >> +#define I2S_IO_2CH_OUT_8CH_IN (7 << I2S_IO_DIRECTION_SHIFT) > > Keep the settings without the shift here and do the shift dynamically with > the per-soc setting when changing the setting. > > > Thanks > Heiko > > _______________________________________________ > Linux-rockchip mailing list > Linux-rockchip@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-rockchip > > >
diff --git a/Documentation/devicetree/bindings/sound/rockchip-i2s.txt b/Documentation/devicetree/bindings/sound/rockchip-i2s.txt index 6e86d8a..ad72a7d 100644 --- a/Documentation/devicetree/bindings/sound/rockchip-i2s.txt +++ b/Documentation/devicetree/bindings/sound/rockchip-i2s.txt @@ -23,6 +23,11 @@ Required properties: - rockchip,playback-channels: max playback channels, if not set, 8 channels default. - rockchip,capture-channels: max capture channels, if not set, 2 channels default. +Required properties for controller which support multi channels playback/capture: + +- rockchip,grf: Should be phandle/offset pair. the phandle of the syscon node for GRF register, + and the offset of the GRF for control register. + Example for rk3288 I2S controller: i2s@ff890000 { diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c index 2f8e204..bc72780 100644 --- a/sound/soc/rockchip/rockchip_i2s.c +++ b/sound/soc/rockchip/rockchip_i2s.c @@ -11,6 +11,7 @@ */ #include <linux/module.h> +#include <linux/mfd/syscon.h> #include <linux/delay.h> #include <linux/of_gpio.h> #include <linux/clk.h> @@ -33,6 +34,8 @@ struct rk_i2s_dev { struct snd_dmaengine_dai_dma_data playback_dma_data; struct regmap *regmap; + struct regmap *grf; + u32 iocfg_reg; bool is_master_mode; }; @@ -277,6 +280,29 @@ static int rockchip_i2s_hw_params(struct snd_pcm_substream *substream, I2S_TXCR_VDW_MASK | I2S_TXCR_CSR_MASK, val); + if (!IS_ERR(i2s->grf)) { + regmap_read(i2s->regmap, I2S_TXCR, &val); + val &= I2S_TXCR_CSR_MASK; + + switch (val) { + case I2S_CHN_4: + val = I2S_IO_4CH_OUT_6CH_IN; + break; + case I2S_CHN_6: + val = I2S_IO_6CH_OUT_4CH_IN; + break; + case I2S_CHN_8: + val = I2S_IO_8CH_OUT_2CH_IN; + break; + default: + val = I2S_IO_2CH_OUT_8CH_IN; + break; + } + + regmap_write(i2s->grf, i2s->iocfg_reg, + I2S_IO_DIRECTION_MASK << 16 | val); + } + regmap_update_bits(i2s->regmap, I2S_DMACR, I2S_DMACR_TDL_MASK, I2S_DMACR_TDL(16)); regmap_update_bits(i2s->regmap, I2S_DMACR, I2S_DMACR_RDL_MASK, @@ -478,6 +504,18 @@ static int rockchip_i2s_probe(struct platform_device *pdev) return -ENOMEM; } + i2s->dev = &pdev->dev; + + i2s->grf = syscon_regmap_lookup_by_phandle(node, "rockchip,grf"); + if (!IS_ERR(i2s->grf)) { + ret = of_property_read_u32_index(node, "rockchip,grf", + 1, &i2s->iocfg_reg); + if (ret) { + dev_err(&pdev->dev, "Can't get iocfg_reg offset\n"); + return ret; + } + } + /* try to prepare related clocks */ i2s->hclk = devm_clk_get(&pdev->dev, "i2s_hclk"); if (IS_ERR(i2s->hclk)) { @@ -517,7 +555,6 @@ static int rockchip_i2s_probe(struct platform_device *pdev) i2s->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES; i2s->capture_dma_data.maxburst = 4; - i2s->dev = &pdev->dev; dev_set_drvdata(&pdev->dev, i2s); pm_runtime_enable(&pdev->dev); diff --git a/sound/soc/rockchip/rockchip_i2s.h b/sound/soc/rockchip/rockchip_i2s.h index dc6e2c7..9a6aabf 100644 --- a/sound/soc/rockchip/rockchip_i2s.h +++ b/sound/soc/rockchip/rockchip_i2s.h @@ -236,4 +236,12 @@ enum { #define I2S_TXDR (0x0024) #define I2S_RXDR (0x0028) +/* io direction cfg register */ +#define I2S_IO_DIRECTION_SHIFT 11 +#define I2S_IO_DIRECTION_MASK (7 << I2S_IO_DIRECTION_SHIFT) +#define I2S_IO_8CH_OUT_2CH_IN (0 << I2S_IO_DIRECTION_SHIFT) +#define I2S_IO_6CH_OUT_4CH_IN (1 << I2S_IO_DIRECTION_SHIFT) +#define I2S_IO_4CH_OUT_6CH_IN (3 << I2S_IO_DIRECTION_SHIFT) +#define I2S_IO_2CH_OUT_8CH_IN (7 << I2S_IO_DIRECTION_SHIFT) + #endif /* _ROCKCHIP_IIS_H */
There are 3 i2s sdio pins, which iomux mode is as follows: - sdi3_sdo1 - sdi2_sdo2 - sdi1_sdo3 we need to configure these pins' iomux mode via the GRF register when use multi channel playback/capture. Signed-off-by: Sugar Zhang <sugar.zhang@rock-chips.com> --- .../devicetree/bindings/sound/rockchip-i2s.txt | 5 +++ sound/soc/rockchip/rockchip_i2s.c | 39 +++++++++++++++++++++- sound/soc/rockchip/rockchip_i2s.h | 8 +++++ 3 files changed, 51 insertions(+), 1 deletion(-)