Re: [PATCH] ASoC: fsl: imx-pcm-dma: Don't request dma channel in probe
Am Mittwoch, den 20.05.2020, 16:20 +0800 schrieb Shengjiu Wang: > Hi > > On Tue, May 19, 2020 at 6:04 PM Lucas Stach wrote: > > Am Dienstag, den 19.05.2020, 17:41 +0800 schrieb Shengjiu Wang: > > > There are two requirements that we need to move the request > > > of dma channel from probe to open. > > > > How do you handle -EPROBE_DEFER return code from the channel request if > > you don't do it in probe? > > I use the dma_request_slave_channel or dma_request_channel instead > of dmaengine_pcm_request_chan_of. so there should be not -EPROBE_DEFER > return code. This is a pretty weak argument. The dmaengine device might probe after you try to get the channel. Using a function to request the channel that doesn't allow you to handle probe deferral is IMHO a bug and should be fixed, instead of building even more assumptions on top of it. > > > - When dma device binds with power-domains, the power will > > > be enabled when we request dma channel. If the request of dma > > > channel happen on probe, then the power-domains will be always > > > enabled after kernel boot up, which is not good for power > > > saving, so we need to move the request of dma channel to .open(); > > > > This is certainly something which could be fixed in the dmaengine > > driver. > > Dma driver always call the pm_runtime_get_sync in > device_alloc_chan_resources, the device_alloc_chan_resources is > called when channel is requested. so power is enabled on channel > request. So why can't you fix the dmaengine driver to do that RPM call at a later time when the channel is actually going to be used? This will allow further power savings with other slave devices than the audio PCM. Regards, Lucas > > > - With FE-BE case, if the dma channel is requested in probe, > > > then there will be below issue, which is caused by that the > > > dma channel will be requested duplicately > > > > Why is this requested a second time? Is this just some missing cleanup > > on a deferred probe path? > > Not relate with deferred probe. With DMA1->ASRC->DMA2->ESAI case, > the DMA1->ASRC->DMA2 is in FE, ESAI is in BE. When ESAI drvier > probe, DMA3 channel is created with ESAI's "dma:tx" (DMA3 channel > is not used in this FE-BE case).When FE-BE startup, DMA2 > channel is created, it needs the ESAI's "dma:tx", so below warning > comes out. > > > Regards, > > Lucas > > > > > [ 638.906268] sysfs: cannot create duplicate filename > > > '/devices/soc0/soc/200.bus/200.spba-bus/2024000.esai/dma:tx' > > > [ 638.919061] CPU: 1 PID: 673 Comm: aplay Not tainted > > > 5.7.0-rc1-12956-gfc64b2585593 #287 > > > [ 638.927113] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree) > > > [ 638.933690] [] (unwind_backtrace) from [] > > > (show_stack+0x10/0x14) > > > [ 638.941464] [] (show_stack) from [] > > > (dump_stack+0xe4/0x118) > > > [ 638.948808] [] (dump_stack) from [] > > > (sysfs_warn_dup+0x50/0x64) > > > [ 638.956406] [] (sysfs_warn_dup) from [] > > > (sysfs_do_create_link_sd+0xc8/0xd4) > > > [ 638.965134] [] (sysfs_do_create_link_sd) from [] > > > (dma_request_chan+0xb0/0x210) > > > [ 638.974120] [] (dma_request_chan) from [] > > > (dma_request_slave_channel+0x8/0x14) > > > [ 638.983111] [] (dma_request_slave_channel) from [] > > > (fsl_asrc_dma_hw_params+0x1e0/0x438) > > > [ 638.992881] [] (fsl_asrc_dma_hw_params) from [] > > > (soc_pcm_hw_params+0x4a0/0x6a8) > > > [ 639.001952] [] (soc_pcm_hw_params) from [] > > > (dpcm_fe_dai_hw_params+0x70/0xe4) > > > [ 639.010765] [] (dpcm_fe_dai_hw_params) from [] > > > (snd_pcm_hw_params+0x158/0x418) > > > [ 639.019750] [] (snd_pcm_hw_params) from [] > > > (snd_pcm_ioctl+0x734/0x183c) > > > [ 639.028129] [] (snd_pcm_ioctl) from [] > > > (ksys_ioctl+0x2ac/0xb98) > > > [ 639.035812] [] (ksys_ioctl) from [] > > > (ret_fast_syscall+0x0/0x28) > > > [ 639.043490] Exception stack(0xec529fa8 to 0xec529ff0) > > > [ 639.048565] 9fa0: bee84650 01321870 0004 > > > c25c4111 bee84650 0002000f > > > [ 639.056766] 9fc0: bee84650 01321870 01321820 0036 1f40 > > > 0002c2f8 0003 > > > [ 639.064964] 9fe0: b6f483fc bee8451c b6ee2655 b6e1dcf8 > > > [ 639.070339] fsl-esai-dai 2024000.esai: Cannot create DMA dma:tx symlink > > > > > > Signed-off-by: Shengjiu Wang > > > --- > > > sound/soc/fs
Re: [PATCH] ASoC: fsl: imx-pcm-dma: Don't request dma channel in probe
Am Dienstag, den 19.05.2020, 17:41 +0800 schrieb Shengjiu Wang:
> There are two requirements that we need to move the request
> of dma channel from probe to open.
How do you handle -EPROBE_DEFER return code from the channel request if
you don't do it in probe?
> - When dma device binds with power-domains, the power will
> be enabled when we request dma channel. If the request of dma
> channel happen on probe, then the power-domains will be always
> enabled after kernel boot up, which is not good for power
> saving, so we need to move the request of dma channel to .open();
This is certainly something which could be fixed in the dmaengine
driver.
> - With FE-BE case, if the dma channel is requested in probe,
> then there will be below issue, which is caused by that the
> dma channel will be requested duplicately
Why is this requested a second time? Is this just some missing cleanup
on a deferred probe path?
Regards,
Lucas
> [ 638.906268] sysfs: cannot create duplicate filename
> '/devices/soc0/soc/200.bus/200.spba-bus/2024000.esai/dma:tx'
> [ 638.919061] CPU: 1 PID: 673 Comm: aplay Not tainted
> 5.7.0-rc1-12956-gfc64b2585593 #287
> [ 638.927113] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
> [ 638.933690] [] (unwind_backtrace) from []
> (show_stack+0x10/0x14)
> [ 638.941464] [] (show_stack) from []
> (dump_stack+0xe4/0x118)
> [ 638.948808] [] (dump_stack) from []
> (sysfs_warn_dup+0x50/0x64)
> [ 638.956406] [] (sysfs_warn_dup) from []
> (sysfs_do_create_link_sd+0xc8/0xd4)
> [ 638.965134] [] (sysfs_do_create_link_sd) from []
> (dma_request_chan+0xb0/0x210)
> [ 638.974120] [] (dma_request_chan) from []
> (dma_request_slave_channel+0x8/0x14)
> [ 638.983111] [] (dma_request_slave_channel) from []
> (fsl_asrc_dma_hw_params+0x1e0/0x438)
> [ 638.992881] [] (fsl_asrc_dma_hw_params) from []
> (soc_pcm_hw_params+0x4a0/0x6a8)
> [ 639.001952] [] (soc_pcm_hw_params) from []
> (dpcm_fe_dai_hw_params+0x70/0xe4)
> [ 639.010765] [] (dpcm_fe_dai_hw_params) from []
> (snd_pcm_hw_params+0x158/0x418)
> [ 639.019750] [] (snd_pcm_hw_params) from []
> (snd_pcm_ioctl+0x734/0x183c)
> [ 639.028129] [] (snd_pcm_ioctl) from []
> (ksys_ioctl+0x2ac/0xb98)
> [ 639.035812] [] (ksys_ioctl) from []
> (ret_fast_syscall+0x0/0x28)
> [ 639.043490] Exception stack(0xec529fa8 to 0xec529ff0)
> [ 639.048565] 9fa0: bee84650 01321870 0004 c25c4111
> bee84650 0002000f
> [ 639.056766] 9fc0: bee84650 01321870 01321820 0036 1f40
> 0002c2f8 0003
> [ 639.064964] 9fe0: b6f483fc bee8451c b6ee2655 b6e1dcf8
> [ 639.070339] fsl-esai-dai 2024000.esai: Cannot create DMA dma:tx symlink
>
> Signed-off-by: Shengjiu Wang
> ---
> sound/soc/fsl/imx-pcm-dma.c | 173 +---
> 1 file changed, 159 insertions(+), 14 deletions(-)
>
> diff --git a/sound/soc/fsl/imx-pcm-dma.c b/sound/soc/fsl/imx-pcm-dma.c
> index 04a9bc749016..dae53b384df4 100644
> --- a/sound/soc/fsl/imx-pcm-dma.c
> +++ b/sound/soc/fsl/imx-pcm-dma.c
> @@ -11,6 +11,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -29,24 +30,168 @@ static bool filter(struct dma_chan *chan, void *param)
> return true;
> }
>
> -static const struct snd_dmaengine_pcm_config imx_dmaengine_pcm_config = {
> - .prepare_slave_config = snd_dmaengine_pcm_prepare_slave_config,
> - .compat_filter_fn = filter,
> -};
> +static int imx_pcm_hw_params(struct snd_soc_component *component,
> + struct snd_pcm_substream *substream,
> + struct snd_pcm_hw_params *params)
> +{
> + struct snd_pcm_runtime *runtime = substream->runtime;
> + struct snd_soc_pcm_runtime *rtd = substream->private_data;
> + struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
> + struct snd_dmaengine_dai_dma_data *dma_data;
> + struct dma_slave_config config;
> + struct dma_chan *chan;
> + int ret = 0;
>
> -int imx_pcm_dma_init(struct platform_device *pdev, size_t size)
> + snd_pcm_set_runtime_buffer(substream, >dma_buffer);
> + runtime->dma_bytes = params_buffer_bytes(params);
> +
> + chan = snd_dmaengine_pcm_get_chan(substream);
> + if (!chan)
> + return -EINVAL;
> +
> + ret = snd_hwparams_to_dma_slave_config(substream, params, );
> + if (ret)
> + return ret;
> +
> + dma_data = snd_soc_dai_get_dma_data(cpu_dai, substream);
> + if (!dma_data)
> + return -EINVAL;
> +
> + snd_dmaengine_pcm_set_config_from_dai_data(substream,
> +dma_data,
> +);
> + return dmaengine_slave_config(chan, );
> +}
> +
> +static int imx_pcm_hw_free(struct snd_soc_component *component,
> +struct snd_pcm_substream *substream)
> {
> - struct snd_dmaengine_pcm_config *config;
> + snd_pcm_set_runtime_buffer(substream, NULL);
Re: [PATCH 06/10] ASoC: dt-bindings: Document dl_mask property
Am Montag, den 22.07.2019, 15:48 +0300 schrieb Daniel Baluta: > SAI supports up to 8 data lines. This property let the user > configure how many data lines should be used per transfer > direction (Tx/Rx). > > > Signed-off-by: Daniel Baluta > --- > Documentation/devicetree/bindings/sound/fsl-sai.txt | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/Documentation/devicetree/bindings/sound/fsl-sai.txt > b/Documentation/devicetree/bindings/sound/fsl-sai.txt > index 2e726b983845..59f4d965a5fb 100644 > --- a/Documentation/devicetree/bindings/sound/fsl-sai.txt > +++ b/Documentation/devicetree/bindings/sound/fsl-sai.txt > @@ -49,6 +49,11 @@ Optional properties: > > > - big-endian : Boolean property, required if all the SAI > > registers are big-endian rather than little-endian. > > + - fsl,dl_mask: list of two integers (bitmask, first for RX, > > second > > + for TX) representing enabled datalines. Bit 0 > > + represents first data line, bit 1 represents second > > + data line and so on. Data line is enabled if > > + corresponding bit is set to 1. No underscores in property names, please. Also this should document the default value used by the driver when the property is absent. Regards, Lucas
Re: [PATCH 05/10] ASoC: fsl_sai: Add support to enable multiple data lines
Am Montag, den 22.07.2019, 15:48 +0300 schrieb Daniel Baluta:
> SAI supports up to 8 Rx/Tx data lines which can be enabled
> using TCE/RCE bits of TCR3/RCR3 registers.
>
> Data lines to be enabled are read from DT fsl,dl_mask property.
> By default (if no DT entry is provided) only data line 0 is enabled.
>
> Note:
> We can only enable consecutive data lines starting with data line #0.
Why is the property a bitmask then? To me this sounds like we want to
have the number of lanes in the DT and convert to the bitmask inside
the driver.
> > Signed-off-by: Daniel Baluta
> ---
> sound/soc/fsl/fsl_sai.c | 10 +-
> sound/soc/fsl/fsl_sai.h | 6 --
> 2 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
> index 768341608695..d0fa02188b7c 100644
> --- a/sound/soc/fsl/fsl_sai.c
> +++ b/sound/soc/fsl/fsl_sai.c
> @@ -601,7 +601,7 @@ static int fsl_sai_startup(struct snd_pcm_substream
> *substream,
>
> > regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx),
> > FSL_SAI_CR3_TRCE_MASK,
> > - FSL_SAI_CR3_TRCE);
> > + FSL_SAI_CR3_TRCE(sai->soc_data->dl_mask[tx]);
>
> > ret = snd_pcm_hw_constraint_list(substream->runtime, 0,
> > SNDRV_PCM_HW_PARAM_RATE, _sai_rate_constraints);
> @@ -887,6 +887,14 @@ static int fsl_sai_probe(struct platform_device *pdev)
> > }
> > }
>
> > + /* active data lines mask for TX/RX, defaults to 1 (only the first
> > + * data line is enabled
> + */
Comment style not in line with Linux coding style.
Regards,
Lucas
> + sai->dl_mask[RX] = 1;
> > + sai->dl_mask[TX] = 1;
> > + of_property_read_u32_index(np, "fsl,dl_mask", RX, >dl_mask[RX]);
> > + of_property_read_u32_index(np, "fsl,dl_mask", TX, >dl_mask[TX]);
> +
> > irq = platform_get_irq(pdev, 0);
> > if (irq < 0) {
> > dev_err(>dev, "no irq for node %s\n", pdev->name);
> diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h
> index b1abeed2f78e..6d32f0950ec5 100644
> --- a/sound/soc/fsl/fsl_sai.h
> +++ b/sound/soc/fsl/fsl_sai.h
> @@ -109,8 +109,8 @@
> > #define FSL_SAI_CR2_DIV_MASK 0xff
>
> /* SAI Transmit and Receive Configuration 3 Register */
> > -#define FSL_SAI_CR3_TRCE BIT(16)
> > -#define FSL_SAI_CR3_TRCE_MASK GENMASK(16, 23)
> > +#define FSL_SAI_CR3_TRCE(x)((x) << 16)
> > +#define FSL_SAI_CR3_TRCE_MASK GENMASK(23, 16)
> > #define FSL_SAI_CR3_WDFL(x)(x)
> > #define FSL_SAI_CR3_WDFL_MASK 0x1f
>
> @@ -176,6 +176,8 @@ struct fsl_sai {
> > unsigned int slots;
> > unsigned int slot_width;
>
> > + unsigned int dl_mask[2];
> +
> > const struct fsl_sai_soc_data *soc_data;
> > struct snd_dmaengine_dai_dma_data dma_params_rx;
> > struct snd_dmaengine_dai_dma_data dma_params_tx;
Re: [PATCH 10/10] ASoC: fsl_sai: Add support for imx7ulp/imx8mq
Am Montag, den 22.07.2019, 15:48 +0300 schrieb Daniel Baluta:
> SAI module on imx7ulp/imx8m features 2 new registers (VERID and PARAM)
> at the beginning of register address space.
>
> On imx7ulp FIFOs can held up to 16 x 32 bit samples.
> On imx8mq FIFOs can held up to 128 x 32 bit samples.
>
> > Signed-off-by: Daniel Baluta
> ---
> sound/soc/fsl/fsl_sai.c | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
> index f2441b84877e..b05837465b5a 100644
> --- a/sound/soc/fsl/fsl_sai.c
> +++ b/sound/soc/fsl/fsl_sai.c
> @@ -1065,10 +1065,24 @@ static const struct fsl_sai_soc_data
> fsl_sai_imx6sx_data = {
> > .reg_offset = 0,
> };
>
> +static const struct fsl_sai_soc_data fsl_sai_imx7ulp_data = {
> > + .use_imx_pcm = true,
> > + .fifo_depth = 16,
> > + .reg_offset = 8,
> +};
> +
> +static const struct fsl_sai_soc_data fsl_sai_imx8mq_data = {
> > + .use_imx_pcm = true,
> > + .fifo_depth = 128,
> > + .reg_offset = 8,
> +};
> +
> static const struct of_device_id fsl_sai_ids[] = {
> > { .compatible = "fsl,vf610-sai", .data = _sai_vf610_data },
> > { .compatible = "fsl,imx6sx-sai", .data = _sai_imx6sx_data },
> > { .compatible = "fsl,imx6ul-sai", .data = _sai_imx6sx_data },
> > + { .compatible = "fsl,imx7ulp-sai", .data = _sai_imx7ulp_data },
> > + { .compatible = "fsl,imx8mq-sai", .data = _sai_imx8mq_data },
> >
Those two new compatibles need to be documented in the DT bindings.
Regards,
Lucas
Re: [PATCH 07/10] ASoC: fsl_sai: Add support for FIFO combine mode
Am Montag, den 22.07.2019, 15:48 +0300 schrieb Daniel Baluta:
> FIFO combining mode allows the separate FIFOs for multiple data
> channels
> to be used as a single FIFO for either software accesses or a single
> data
> channel or both.
>
> FIFO combined mode is described in chapter 13.10.3.5.4 from i.MX8MQ
> reference manual [1].
>
> For each direction (RX/TX) fifo combine mode is read from fsl,fcomb-
> mode
> DT property. By default, if no property is specified fifo combine
> mode
> is disabled.
>
> [1]https://cache.nxp.com/secured/assets/documents/en/reference-manual
> /IMX8MDQLQRM.pdf?__gda__=1563728701_38bea7f0f726472cc675cb141b91bec7&
> fileExt=.pdf
>
> Signed-off-by: Daniel Baluta
> ---
> sound/soc/fsl/fsl_sai.c | 37 +
> sound/soc/fsl/fsl_sai.h | 9 +
> 2 files changed, 46 insertions(+)
>
> diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
> index d0fa02188b7c..140014901fce 100644
> --- a/sound/soc/fsl/fsl_sai.c
> +++ b/sound/soc/fsl/fsl_sai.c
> @@ -475,6 +475,35 @@ static int fsl_sai_hw_params(struct
> snd_pcm_substream *substream,
> }
> }
>
> + switch (sai->soc_data->fcomb_mode[tx]) {
> + case FSL_SAI_FCOMB_NONE:
> + regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx),
> + FSL_SAI_CR4_FCOMB_SOFT |
> + FSL_SAI_CR4_FCOMB_SHIFT, 0);
> + break;
> + case FSL_SAI_FCOMB_SHIFT:
> + regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx),
> + FSL_SAI_CR4_FCOMB_SOFT |
> + FSL_SAI_CR4_FCOMB_SHIFT,
> + FSL_SAI_CR4_FCOMB_SHIFT);
> + break;
> + case FSL_SAI_FCOMB_SOFT:
> + regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx),
> + FSL_SAI_CR4_FCOMB_SOFT |
> + FSL_SAI_CR4_FCOMB_SHIFT,
> + FSL_SAI_CR4_FCOMB_SOFT);
> + break;
> + case FSL_SAI_FCOMB_BOTH:
> + regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx),
> + FSL_SAI_CR4_FCOMB_SOFT |
> + FSL_SAI_CR4_FCOMB_SHIFT,
> + FSL_SAI_CR4_FCOMB_SOFT |
> + FSL_SAI_CR4_FCOMB_SHIFT);
> + break;
> + default:
> + break;
> + }
This would probably look less redundant if you only select the bits to
set in the switch statement and move the regmap_update_bits after the
switch.
Regards,
Lucas
> regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx),
> FSL_SAI_CR4_SYWD_MASK |
> FSL_SAI_CR4_FRSZ_MASK,
> val_cr4);
> @@ -887,6 +916,14 @@ static int fsl_sai_probe(struct platform_device
> *pdev)
> }
> }
>
> + /* FIFO combine mode for TX/RX, defaults to disabled */
> + sai->fcomb_mode[RX] = FSL_SAI_FCOMB_NONE;
> + sai->fcomb_mode[TX] = FSL_SAI_FCOMB_NONE;
> + of_property_read_u32_index(np, "fsl,fcomb-mode", RX,
> + >fcomb_mode[RX]);
> + of_property_read_u32_index(np, "fsl,fcomb-mode", TX,
> + >fcomb_mode[TX]);
> +
> /* active data lines mask for TX/RX, defaults to 1 (only the
> first
> * data line is enabled
> */
> diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h
> index 6d32f0950ec5..abf140951187 100644
> --- a/sound/soc/fsl/fsl_sai.h
> +++ b/sound/soc/fsl/fsl_sai.h
> @@ -115,6 +115,8 @@
> #define FSL_SAI_CR3_WDFL_MASK0x1f
>
> /* SAI Transmit and Receive Configuration 4 Register */
> +#define FSL_SAI_CR4_FCOMB_SHIFT BIT(26)
> +#define FSL_SAI_CR4_FCOMB_SOFT BIT(27)
> #define FSL_SAI_CR4_FRSZ(x) (((x) - 1) << 16)
> #define FSL_SAI_CR4_FRSZ_MASK(0x1f << 16)
> #define FSL_SAI_CR4_SYWD(x) (((x) - 1) << 8)
> @@ -155,6 +157,12 @@
> #define FSL_SAI_MAXBURST_TX 6
> #define FSL_SAI_MAXBURST_RX 6
>
> +/* FIFO combine modes */
> +#define FSL_SAI_FCOMB_NONE 0
> +#define FSL_SAI_FCOMB_SHIFT1
> +#define FSL_SAI_FCOMB_SOFT 2
> +#define FSL_SAI_FCOMB_BOTH 3
> +
> struct fsl_sai_soc_data {
> bool use_imx_pcm;
> unsigned int fifo_depth;
> @@ -177,6 +185,7 @@ struct fsl_sai {
> unsigned int slot_width;
>
> unsigned int dl_mask[2];
> + unsigned int fcomb_mode[2];
>
> const struct fsl_sai_soc_data *soc_data;
> struct snd_dmaengine_dai_dma_data dma_params_rx;
Re: drivers binding to device node with multiple compatible strings
Hi, Am Freitag, den 28.09.2018, 12:43 -0700 schrieb Frank Rowand: > + Frank > > On 09/27/18 15:25, Li Yang wrote: > > Hi Rob and Grant, > > > > Various device tree specs are recommending to include all the > > potential compatible strings in the device node, with the order from > > most specific to most general. But it looks like Linux kernel doesn't > > provide a way to bind the device to the most specific driver, however, > > the first registered compatible driver will be bound. > > > > As more and more generic drivers are added to the Linux kernel, they > > are competing with the more specific vendor drivers and causes problem > > when both are built into the kernel. I'm wondering if there is a > > generic solution (or in plan) to make the most specific driver bound > > to the device. Or we have to disable the more general driver or > > remove the more general compatible string from the device tree? Not really contributing to the solution, but the hard question to answer is when do you know what the most specific driver is? The most specific driver might well be a module that can be loaded at any time, while there might already be other less specific drivers around. In general I would say that if your device is specific enough to warrant a whole new driver, it should not declare compatibility with the generic thing in the compatible, but then this is kind of exporting an Linux implementation detail to DT. Regards, Lucas
Re: [PATCH v2 00/22] Use MSI chip framework to configure MSI/MSI-X in all platforms
Am Sonntag, den 28.09.2014, 14:11 +0800 schrieb Yijing Wang: On 2014/9/28 10:32, Yijing Wang wrote: On 2014/9/26 17:05, Thierry Reding wrote: On Fri, Sep 26, 2014 at 10:54:32AM +0200, Thierry Reding wrote: [...] At least for Tegra it's trivial to just hook it up in tegra_pcie_scan_bus() directly (patch attached). Really attached this time. Thierry It looks good to me, so I will update the arm pci hostbridge driver to assign pci root bus the msi chip instead of current pcibios_add_bus(). But for other platforms which only have a one msi chip, I will kept the arch_find_msi_chip() temporarily for more comments, especially from Bjorn. Oh, sorry, I found designware and rcar use pci_scan_root_bus(), so we can not simply assign msi chip to root bus in all host drivers's scan functions. Designware will switch away from pci_scan_root_bus() in the 3.18 cycle and I would think it would be no problem to to the same with rcar. Regards, Lucas -- Pengutronix e.K. | Lucas Stach | Industrial Linux Solutions | http://www.pengutronix.de/ | ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v1 03/21] MSI: Remove the redundant irq_set_chip_data()
Am Dienstag, den 16.09.2014, 09:30 +0800 schrieb Yijing Wang: On 2014/9/15 22:00, Lucas Stach wrote: Am Freitag, den 05.09.2014, 18:09 +0800 schrieb Yijing Wang: Currently, pcie-designware, pcie-rcar, pci-tegra drivers use irq chip_data to save the msi_chip pointer. They already call irq_set_chip_data() in their own MSI irq map functions. So irq_set_chip_data() in arch_setup_msi_irq() is useless. Signed-off-by: Yijing Wang wangyij...@huawei.com --- drivers/pci/msi.c |2 -- 1 files changed, 0 insertions(+), 2 deletions(-) diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c index f6cb317..d547f7f 100644 --- a/drivers/pci/msi.c +++ b/drivers/pci/msi.c @@ -41,8 +41,6 @@ int __weak arch_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc) if (err 0) return err; - irq_set_chip_data(desc-irq, chip); - return 0; } arch_teardown_msi_irq() expects to find the msi_chip in the irq chip_data field. As this means drivers don't have any reasonable other possibility to stuff things into this field, I think it would make sense to do the cleanup the other way around: keep the irq_set_chip_data arch_setup_msi_irq() and rip it out of the individual drivers. Hi Lucas, thanks for your review and comments! irq_set_chip_data() should not be placed in MSI core functions, because other arch like x86, use irq_data-chip_data to stores irq_cfg. So how to set the chip_data is arch dependent. And this series is mainly to use MSI chip framework in all platforms. Currently, only ARM platform MSI drivers use the chip_data to store msi_chip, and the drivers call irq_set_chip_data() in their driver already. So I thought we should clean up it in MSI core code. Okay I see your point, so the cleanup done this way is okay. But then this still introduces a problem: arch_teardown_msi_irq() expects to find the msi_chip in the chip_data field, which is okay for all ARM PCI host drivers, but not for other arch MSI chips. You fix this by completely removing arch_teardown_msi_irq() at the end of the series, but this still has the potential to introduce issues for other arches than ARM within the series. So this patch should include a change to replace the line struct msi_chip *chip = irq_get_chip_data(irq); with something that doesn't rely on the msi_chip being in the irq chip_data field. Regards, Lucas -- Pengutronix e.K. | Lucas Stach | Industrial Linux Solutions | http://www.pengutronix.de/ | ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v1 03/21] MSI: Remove the redundant irq_set_chip_data()
Am Freitag, den 05.09.2014, 18:09 +0800 schrieb Yijing Wang: Currently, pcie-designware, pcie-rcar, pci-tegra drivers use irq chip_data to save the msi_chip pointer. They already call irq_set_chip_data() in their own MSI irq map functions. So irq_set_chip_data() in arch_setup_msi_irq() is useless. Signed-off-by: Yijing Wang wangyij...@huawei.com --- drivers/pci/msi.c |2 -- 1 files changed, 0 insertions(+), 2 deletions(-) diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c index f6cb317..d547f7f 100644 --- a/drivers/pci/msi.c +++ b/drivers/pci/msi.c @@ -41,8 +41,6 @@ int __weak arch_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc) if (err 0) return err; - irq_set_chip_data(desc-irq, chip); - return 0; } arch_teardown_msi_irq() expects to find the msi_chip in the irq chip_data field. As this means drivers don't have any reasonable other possibility to stuff things into this field, I think it would make sense to do the cleanup the other way around: keep the irq_set_chip_data arch_setup_msi_irq() and rip it out of the individual drivers. Regards, Lucas -- Pengutronix e.K. | Lucas Stach | Industrial Linux Solutions | http://www.pengutronix.de/ | ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v1 05/21] PCI/MSI: Introduce weak arch_find_msi_chip() to find MSI chip
Am Freitag, den 05.09.2014, 18:09 +0800 schrieb Yijing Wang:
Introduce weak arch_find_msi_chip() to find the match msi_chip.
Currently, MSI chip associates pci bus to msi_chip. Because in
ARM platform, there may be more than one MSI controller in system.
Associate pci bus to msi_chip help pci device to find the match
msi_chip and setup MSI/MSI-X irq correctly. But in other platform,
like in x86. we only need one MSI chip, because all device use
the same MSI address/data and irq etc. So it's no need to associate
pci bus to MSI chip, just use a arch function, arch_find_msi_chip()
to return the MSI chip for simplicity. The default weak
arch_find_msi_chip() used in ARM platform, find the MSI chip
by pci bus.
Hm, while one weak function sounds much better than the plethora we have
now, I wonder how much work it would be to associate the msi_chip with
the pci bus on other arches the same way as done on ARM. This way we
could kill this calling into arch specific functions which would make
things a bit clearer to follow I think.
Regards,
Lucas
Signed-off-by: Yijing Wang wangyij...@huawei.com
---
drivers/pci/msi.c |7 ++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index a77e7f7..539c11d 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -29,9 +29,14 @@ static int pci_msi_enable = 1;
/* Arch hooks */
+struct msi_chip * __weak arch_find_msi_chip(struct pci_dev *dev)
+{
+ return dev-bus-msi;
+}
+
int __weak arch_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc)
{
- struct msi_chip *chip = dev-bus-msi;
+ struct msi_chip *chip = arch_find_msi_chip(dev);
int err;
if (!chip || !chip-setup_irq)
--
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/ |
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v1 06/21] PCI/MSI: Refactor struct msi_chip to make it become more common
Am Freitag, den 05.09.2014, 18:09 +0800 schrieb Yijing Wang:
Now there are a lot of __weak arch functions in MSI code.
These functions make MSI driver complex. Thierry Reding Introduced
a new MSI chip framework to configure MSI/MSI-X irq in ARM. Use
the new MSI chip framework to refactor all other platform MSI
arch code to eliminate weak arch MSI functions. This patch add
.restore_irq() and .setup_irqs() to make it become more common.
Signed-off-by: Yijing Wang wangyij...@huawei.com
This change looks good to me:
Reviewed-by: Lucas Stach l.st...@pengutronix.de
---
drivers/pci/msi.c | 15 +++
include/linux/msi.h |3 +++
2 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 539c11d..d78d637 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -63,6 +63,11 @@ int __weak arch_setup_msi_irqs(struct pci_dev *dev, int
nvec, int type)
{
struct msi_desc *entry;
int ret;
+ struct msi_chip *chip;
+
+ chip = arch_find_msi_chip(dev);
+ if (chip chip-setup_irqs)
+ return chip-setup_irqs(dev, nvec, type);
/*
* If an architecture wants to support multiple MSI, it needs to
@@ -105,6 +110,11 @@ void default_teardown_msi_irqs(struct pci_dev *dev)
void __weak arch_teardown_msi_irqs(struct pci_dev *dev)
{
+ struct msi_chip *chip = arch_find_msi_chip(dev);
+
+ if (chip chip-teardown_irqs)
+ return chip-teardown_irqs(dev);
+
return default_teardown_msi_irqs(dev);
}
@@ -128,6 +138,11 @@ static void default_restore_msi_irq(struct pci_dev *dev,
int irq)
void __weak arch_restore_msi_irqs(struct pci_dev *dev)
{
+ struct msi_chip *chip = arch_find_msi_chip(dev);
+
+ if (chip chip-restore_irqs)
+ return chip-restore_irqs(dev);
+
return default_restore_msi_irqs(dev);
}
diff --git a/include/linux/msi.h b/include/linux/msi.h
index 5650848..92a51e7 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -72,7 +72,10 @@ struct msi_chip {
struct list_head list;
int (*setup_irq)(struct pci_dev *dev, struct msi_desc *desc);
+ int (*setup_irqs)(struct pci_dev *dev, int nvec, int type);
void (*teardown_irq)(unsigned int irq);
+ void (*teardown_irqs)(struct pci_dev *dev);
+ void (*restore_irqs)(struct pci_dev *dev);
};
#endif /* LINUX_MSI_H */
--
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/ |
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v1 21/21] PCI/MSI: Clean up unused MSI arch functions
Am Freitag, den 05.09.2014, 18:10 +0800 schrieb Yijing Wang:
Now we use struct msi_chip in all platforms to configure
MSI/MSI-X. We can clean up the unused arch functions.
Signed-off-by: Yijing Wang wangyij...@huawei.com
Reviewed-by: Lucas Stach l.st...@pengutronix.de
---
drivers/iommu/irq_remapping.c |2 +-
drivers/pci/msi.c | 99
-
include/linux/msi.h | 14 --
3 files changed, 39 insertions(+), 76 deletions(-)
diff --git a/drivers/iommu/irq_remapping.c b/drivers/iommu/irq_remapping.c
index 99b1c0f..6e645f0 100644
--- a/drivers/iommu/irq_remapping.c
+++ b/drivers/iommu/irq_remapping.c
@@ -92,7 +92,7 @@ error:
/*
* Restore altered MSI descriptor fields and prevent just destroyed
- * IRQs from tearing down again in default_teardown_msi_irqs()
+ * IRQs from tearing down again in teardown_msi_irqs()
*/
msidesc-irq = 0;
msidesc-nvec_used = 0;
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index d78d637..e3e7f4f 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -34,50 +34,31 @@ struct msi_chip * __weak arch_find_msi_chip(struct
pci_dev *dev)
return dev-bus-msi;
}
-int __weak arch_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc)
-{
- struct msi_chip *chip = arch_find_msi_chip(dev);
- int err;
-
- if (!chip || !chip-setup_irq)
- return -EINVAL;
-
- err = chip-setup_irq(dev, desc);
- if (err 0)
- return err;
-
- return 0;
-}
-
-void __weak arch_teardown_msi_irq(unsigned int irq)
-{
- struct msi_chip *chip = irq_get_chip_data(irq);
-
- if (!chip || !chip-teardown_irq)
- return;
-
- chip-teardown_irq(irq);
-}
-
-int __weak arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
+int setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
{
struct msi_desc *entry;
int ret;
struct msi_chip *chip;
chip = arch_find_msi_chip(dev);
- if (chip chip-setup_irqs)
+ if (!chip)
+ return -EINVAL;
+
+ if (chip-setup_irqs)
return chip-setup_irqs(dev, nvec, type);
/*
* If an architecture wants to support multiple MSI, it needs to
- * override arch_setup_msi_irqs()
+ * implement chip-setup_irqs().
*/
if (type == PCI_CAP_ID_MSI nvec 1)
return 1;
+ if (!chip-setup_irq)
+ return -EINVAL;
+
list_for_each_entry(entry, dev-msi_list, list) {
- ret = arch_setup_msi_irq(dev, entry);
+ ret = chip-setup_irq(dev, entry);
if (ret 0)
return ret;
if (ret 0)
@@ -87,13 +68,20 @@ int __weak arch_setup_msi_irqs(struct pci_dev *dev, int
nvec, int type)
return 0;
}
-/*
- * We have a default implementation available as a separate non-weak
- * function, as it is used by the Xen x86 PCI code
- */
-void default_teardown_msi_irqs(struct pci_dev *dev)
+static void teardown_msi_irqs(struct pci_dev *dev)
{
struct msi_desc *entry;
+ struct msi_chip *chip;
+
+ chip = arch_find_msi_chip(dev);
+ if (!chip)
+ return;
+
+ if (chip-teardown_irqs)
+ return chip-teardown_irqs(dev);
+
+ if (!chip-teardown_irq)
+ return;
list_for_each_entry(entry, dev-msi_list, list) {
int i, nvec;
@@ -104,20 +92,10 @@ void default_teardown_msi_irqs(struct pci_dev *dev)
else
nvec = 1 entry-msi_attrib.multiple;
for (i = 0; i nvec; i++)
- arch_teardown_msi_irq(entry-irq + i);
+ chip-teardown_irq(entry-irq + i);
}
}
-void __weak arch_teardown_msi_irqs(struct pci_dev *dev)
-{
- struct msi_chip *chip = arch_find_msi_chip(dev);
-
- if (chip chip-teardown_irqs)
- return chip-teardown_irqs(dev);
-
- return default_teardown_msi_irqs(dev);
-}
-
static void default_restore_msi_irq(struct pci_dev *dev, int irq)
{
struct msi_desc *entry;
@@ -136,10 +114,18 @@ static void default_restore_msi_irq(struct pci_dev
*dev, int irq)
write_msi_msg(irq, entry-msg);
}
-void __weak arch_restore_msi_irqs(struct pci_dev *dev)
+static void default_restore_msi_irqs(struct pci_dev *dev)
{
- struct msi_chip *chip = arch_find_msi_chip(dev);
+ struct msi_desc *entry = NULL;
+
+ list_for_each_entry(entry, dev-msi_list, list) {
+ default_restore_msi_irq(dev, entry-irq);
+ }
+}
+static void restore_msi_irqs(struct pci_dev *dev)
+{
+ struct msi_chip *chip = arch_find_msi_chip(dev);
if (chip chip-restore_irqs)
return chip-restore_irqs(dev);
@@ -248,15 +234,6 @@ void unmask_msi_irq(struct irq_data *data
Re: [PATCHv1 8/8] Documentation: Add device tree bindings for Freescale VF610 sound.
Am Donnerstag, den 17.10.2013, 17:01 +0800 schrieb Xiubo Li:
This adds the Document for Freescale VF610 sound driver under
Documentation/devicetree/bindings/sound/.
Signed-off-by: Xiubo Li li.xi...@freescale.com
---
.../devicetree/bindings/sound/fsl-sgtl5000.txt | 52
++
1 file changed, 52 insertions(+)
create mode 100644 Documentation/devicetree/bindings/sound/fsl-sgtl5000.txt
diff --git a/Documentation/devicetree/bindings/sound/fsl-sgtl5000.txt
b/Documentation/devicetree/bindings/sound/fsl-sgtl5000.txt
new file mode 100644
index 000..43e350f
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/fsl-sgtl5000.txt
This document name is overly generic, there are more than one FSL
platforms with SGTL5000 codecs. Please include the vf610 here.
@@ -0,0 +1,52 @@
+Freescale VF610 audio complex with SGTL5000 codec
+
+Required properties:
+- compatible: fsl,vf610-sgtl5000
+- model: The user-visible name of this sound complex.
+- saif-controllers: The phandle list of the SAI controller.
+- audio-codec: The phandle of the SGTL5000 audio codec.
+- audio-routing : A list of the connections between audio components.
+ Each entry is a pair of strings, the first being the connection's sink,
+ the second being the connection's source. Valid names could be power
+ supplies, SGTL5000 pins, and the jacks on the board:
+
+ -- Power supplies:
+ * Mic Bias
+
+ -- SGTL5000 pins:
+ * MIC_IN
+ * LINE_IN
+ * HP_OUT
+ * LINE_OUT
+
+ -- Board connectors:
+ * Mic Jack
+ * Line In Jack
+ * Headphone Jack
+ * Line Out Jack
+ * Ext Spk
+
+Example:
+
+sound {
+ compatible = fsl,vf610-sgtl5000;
+ model = vf610-sgtl5000;
+ saif-controller = sai2;
+ audio-codec = codec;
+ audio-routing =
+ MIC_IN, Mic Jack,
+ Mic Jack, Mic Bias,
+ LINE_IN, Line In Jack,
+ Headphone Jack, HP_OUT,
+ Ext Spk, LINE_OUT;
+};
+
+i2c0 {
+ ...
+
+ codec: sgtl5000@0a {
+compatible = fsl,sgtl5000;
+reg = 0x0a;
+clocks = clks VF610_CLK_SAI2;
+ };
+};
--
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
