[PATCH v6 01/10] ASoC: phycore-ac97: Add DT support

Markus Pargmann mpa at pengutronix.de
Thu May 30 19:10:54 EST 2013


On Wed, May 29, 2013 at 09:53:45PM +0800, Shawn Guo wrote:
> On Tue, May 28, 2013 at 04:47:49PM +0200, Markus Pargmann wrote:
> > Add devicetree support for this audio soc fabric driver.
> > 
> > platform_of_node and cpu_of_node are set according to the fsl,audmux
> > phandle.
> > 
> > This patch adds handling of ac97 reset functions according to fsl ac97
> > support. They are setup from here to avoid board specific code in the
> > generic fsl-ssi driver.
> > 
> > This patch changes the handling of pca100 boards from non-DT to DT only.
> > pcm043 is still handled without DT.
> > 
> > Signed-off-by: Markus Pargmann <mpa at pengutronix.de>
> > ---
> > 
> > Notes:
> >     Changes in v6:
> >      - phycore-ac97 now manages the ac97 reset functions of the boards using
> >        this combination of ssi-codec.
> >      - Removed preprocessor ifs for DT, non-DT distinction. pcm043 is now
> >        non-DT only and pca100 DT only.
> >     
> >     Changes in v4:
> >      - New property phytec,audmux to check which audmux setup should be
> >        executed. Checking for fsl,imx21-audmux and fsl,imx31-audmux.
> >     
> >     Changes in v3:
> >      - Add some more information in the commit message.
> >     
> >     Changes in v2:
> >      - Simplify the driver, by combining audmux port configurations. The
> >        audmux driver actually knows on which platform he is running and
> >        will return the appropriate error code if we use functions for
> >        another platform. So we don't need to have the knowledge about it
> >        in phycore-ac97 and can try both functions. This removes the need
> >        of different compatibilities and renames it to phycore-ac97.
> >      - Use a phandle for the cpu_dai link.
> >      - Add devicetree binding documentation.
> >      - Rename binding to phytec,phycore-ac97 and fsl,ssi to phytec,ssi
> >     
> >     Changes in v2:
> >      - Simplify the driver, by combining audmux port configurations. The
> >        audmux driver actually knows on which platform he is running and
> >        will return the appropriate error code if we use functions for
> >        another platform. So we don't need to have the knowledge about it
> >        in phycore-ac97 and can try both functions. This removes the need
> >        of different compatibilities and renames it to imx27-ac97.
> >      - Use a phandle for the cpu_dai link.
> >      - Add devicetree binding documentation.
> >      - Rename binding to phytec,phycore-ac97 and fsl,ssi to phytec,ssi
> > 
> >  .../bindings/sound/phytec,phycore-ac97.txt         |  14 ++
> >  sound/soc/fsl/phycore-ac97.c                       | 241 ++++++++++++++++++---
> >  2 files changed, 229 insertions(+), 26 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/sound/phytec,phycore-ac97.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/sound/phytec,phycore-ac97.txt b/Documentation/devicetree/bindings/sound/phytec,phycore-ac97.txt
> > new file mode 100644
> > index 0000000..41201ff
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/sound/phytec,phycore-ac97.txt
> > @@ -0,0 +1,14 @@
> > +Phytec phycore AC97
> > +
> > +Required properties:
> > +- compatible: "phytec,phycore-ac97"
> > +- phytec,ssi: A phandle to the ssi device that is connected to ac97.
> > +- phytec,audmux: A phandle to the audmux device.
> > +
> > +Example:
> > +
> > +sound {
> > +	compatible = "phytec,phycore-ac97";
> > +	phytec,ssi = <&ssi1>;
> > +	phytec,audmux = <&audmux>;
> > +};
> > diff --git a/sound/soc/fsl/phycore-ac97.c b/sound/soc/fsl/phycore-ac97.c
> > index ae403c2..bf2c600 100644
> > --- a/sound/soc/fsl/phycore-ac97.c
> > +++ b/sound/soc/fsl/phycore-ac97.c
> > @@ -20,8 +20,14 @@
> >  #include <asm/mach-types.h>
> >  
> >  #include "imx-audmux.h"
> > +#include "fsl_ssi.h"
> > +
> > +#define DRV_NAME "phycore-audio-fabric"
> >  
> >  static struct snd_soc_card imx_phycore;
> > +static struct device_node *phycore_dai_cpu_node;
> > +static void (*phycore_ac97_reset) (struct snd_ac97 *ac97);
> > +static void (*phycore_ac97_warm_reset)(struct snd_ac97 *ac97);
> >  
> >  static struct snd_soc_ops imx_phycore_hifi_ops = {
> >  };
> > @@ -32,12 +38,12 @@ static struct snd_soc_dai_link imx_phycore_dai_ac97[] = {
> >  		.stream_name	= "HiFi",
> >  		.codec_dai_name		= "wm9712-hifi",
> >  		.codec_name	= "wm9712-codec",
> > -		.cpu_dai_name	= "imx-ssi.0",
> > -		.platform_name	= "imx-ssi.0",
> 
> I do not think these and phycore_ac97_plat_name changes are necessary,
> since of_node will always take precedence over name in match.

They are necessary, snd_soc_register_card returns with -EINVAL if both,
name and of_node, are set.

> 
> >  		.ops		= &imx_phycore_hifi_ops,
> >  	},
> >  };
> >  
> > +static const char phycore_ac97_plat_name[] = "imx-ssi.0";
> > +
> >  static struct snd_soc_card imx_phycore = {
> >  	.name		= "PhyCORE-ac97-audio",
> >  	.owner		= THIS_MODULE,
> > @@ -45,40 +51,52 @@ static struct snd_soc_card imx_phycore = {
> >  	.num_links	= ARRAY_SIZE(imx_phycore_dai_ac97),
> >  };
> >  
> > -static struct platform_device *imx_phycore_snd_ac97_device;
> > +static void phycore_ac97_imx21_audmux(void)
> > +{
> > +	imx_audmux_v1_configure_port(MX27_AUDMUX_HPCR1_SSI0,
> > +		IMX_AUDMUX_V1_PCR_SYN | /* 4wire mode */
> > +		IMX_AUDMUX_V1_PCR_TFCSEL(3) |
> > +		IMX_AUDMUX_V1_PCR_TCLKDIR | /* clock is output */
> > +		IMX_AUDMUX_V1_PCR_RXDSEL(3));
> > +	imx_audmux_v1_configure_port(3,
> > +		IMX_AUDMUX_V1_PCR_SYN | /* 4wire mode */
> > +		IMX_AUDMUX_V1_PCR_TFCSEL(0) |
> > +		IMX_AUDMUX_V1_PCR_TFSDIR |
> > +		IMX_AUDMUX_V1_PCR_RXDSEL(0));
> > +}
> > +
> > +static void phycore_ac97_imx31_audmux(void)
> > +{
> > +	imx_audmux_v2_configure_port(3,
> > +		IMX_AUDMUX_V2_PTCR_SYN | /* 4wire mode */
> > +		IMX_AUDMUX_V2_PTCR_TFSEL(0) |
> > +		IMX_AUDMUX_V2_PTCR_TFSDIR,
> > +		IMX_AUDMUX_V2_PDCR_RXDSEL(0));
> > +	imx_audmux_v2_configure_port(0,
> > +		IMX_AUDMUX_V2_PTCR_SYN | /* 4wire mode */
> > +		IMX_AUDMUX_V2_PTCR_TCSEL(3) |
> > +		IMX_AUDMUX_V2_PTCR_TCLKDIR, /* clock is output */
> > +		IMX_AUDMUX_V2_PDCR_RXDSEL(3));
> > +}
> > +
> >  static struct platform_device *imx_phycore_snd_device;
> >  
> > +static struct platform_device *imx_phycore_snd_ac97_device;
> > +
> >  static int __init imx_phycore_init(void)
> >  {
> >  	int ret;
> >  
> > -	if (machine_is_pca100()) {
> > -		imx_audmux_v1_configure_port(MX27_AUDMUX_HPCR1_SSI0,
> > -			IMX_AUDMUX_V1_PCR_SYN | /* 4wire mode */
> > -			IMX_AUDMUX_V1_PCR_TFCSEL(3) |
> > -			IMX_AUDMUX_V1_PCR_TCLKDIR | /* clock is output */
> > -			IMX_AUDMUX_V1_PCR_RXDSEL(3));
> > -		imx_audmux_v1_configure_port(3,
> > -			IMX_AUDMUX_V1_PCR_SYN | /* 4wire mode */
> > -			IMX_AUDMUX_V1_PCR_TFCSEL(0) |
> > -			IMX_AUDMUX_V1_PCR_TFSDIR |
> > -			IMX_AUDMUX_V1_PCR_RXDSEL(0));
> > -	} else if (machine_is_pcm043()) {
> > -		imx_audmux_v2_configure_port(3,
> > -			IMX_AUDMUX_V2_PTCR_SYN | /* 4wire mode */
> > -			IMX_AUDMUX_V2_PTCR_TFSEL(0) |
> > -			IMX_AUDMUX_V2_PTCR_TFSDIR,
> > -			IMX_AUDMUX_V2_PDCR_RXDSEL(0));
> > -		imx_audmux_v2_configure_port(0,
> > -			IMX_AUDMUX_V2_PTCR_SYN | /* 4wire mode */
> > -			IMX_AUDMUX_V2_PTCR_TCSEL(3) |
> > -			IMX_AUDMUX_V2_PTCR_TCLKDIR, /* clock is output */
> > -			IMX_AUDMUX_V2_PDCR_RXDSEL(3));
> > +	if (machine_is_pcm043()) {
> > +		phycore_ac97_imx31_audmux();
> >  	} else {
> >  		/* return happy. We might run on a totally different machine */
> >  		return 0;
> >  	}
> >  
> > +	imx_phycore_dai_ac97[0].cpu_dai_name = phycore_ac97_plat_name;
> > +	imx_phycore_dai_ac97[0].platform_name = phycore_ac97_plat_name;
> > +
> >  	imx_phycore_snd_ac97_device = platform_device_alloc("soc-audio", -1);
> >  	if (!imx_phycore_snd_ac97_device)
> >  		return -ENOMEM;
> > @@ -108,18 +126,189 @@ fail2:
> >  	platform_device_del(imx_phycore_snd_ac97_device);
> >  fail1:
> >  	platform_device_put(imx_phycore_snd_ac97_device);
> > +	imx_phycore_dai_ac97[0].cpu_dai_name = NULL;
> > +	imx_phycore_dai_ac97[0].platform_name = NULL;
> >  	return ret;
> >  }
> >  
> >  static void __exit imx_phycore_exit(void)
> >  {
> > +	if (!machine_is_pcm043())
> > +		return;
> > +
> >  	platform_device_unregister(imx_phycore_snd_device);
> >  	platform_device_unregister(imx_phycore_snd_ac97_device);
> > +
> > +	imx_phycore_dai_ac97[0].cpu_dai_name = NULL;
> > +	imx_phycore_dai_ac97[0].platform_name = NULL;
> >  }
> >  
> >  late_initcall(imx_phycore_init);
> >  module_exit(imx_phycore_exit);
> >  
> > +
> > +static const struct of_device_id imx_phycore_ac97_of_dev_id[] = {
> > +	{
> > +		.compatible = "phytec,phycore-ac97",
> > +	}, {
> > +		/* sentinel */
> > +	}
> > +};
> > +MODULE_DEVICE_TABLE(of, imx_phycore_ac97_of_dev_id);
> > +
> > +
> > +/*
> > + * Pointer to AC97 reset functions for specific boards
> > + */
> > +#if IS_ENABLED(CONFIG_MACH_PCA100)
> > +extern void pca100_ac97_cold_reset(struct snd_ac97 *ac97);
> > +extern void pca100_ac97_warm_reset(struct snd_ac97 *ac97);
> > +#else
> > +void pca100_ac97_cold_reset(struct snd_ac97 *ac97) { }
> > +void pca100_ac97_warm_reset(struct snd_ac97 *ac97) { }
> 
> static?

Thanks, added.

> 
> > +#endif
> > +
> > +#if IS_ENABLED(CONFIG_MACH_PCM043)
> > +extern void pcm043_ac97_cold_reset(struct snd_ac97 *ac97);
> > +extern void pcm043_ac97_warm_reset(struct snd_ac97 *ac97);
> > +#else
> > +void pcm043_ac97_cold_reset(struct snd_ac97 *ac97) { }
> > +void pcm043_ac97_warm_reset(struct snd_ac97 *ac97) { }
> > +#endif
> > +
> > +static void phycore_soc_ac97_reset(struct snd_ac97 *ac97)
> > +{
> > +	if (phycore_ac97_reset)
> > +		phycore_ac97_reset(ac97);
> > +	/* First read sometimes fails, do a dummy read */
> > +	fsl_ssi_ac97_read(ac97, 0);
> 
> This function is unavailable until patch #7, right?

Yes, sorry, I missed to update the order of the patches. I will fix it.

> 
> > +}
> > +
> > +static void phycore_soc_ac97_warm_reset(struct snd_ac97 *ac97)
> > +{
> > +	if (phycore_ac97_warm_reset)
> > +		phycore_ac97_warm_reset(ac97);
> > +
> > +	/* First read sometimes fails, do a dummy read */
> > +	fsl_ssi_ac97_read(ac97, 0);
> > +}
> > +
> > +static int phycore_ac97_setup_devices(struct platform_device *pdev)
> > +{
> > +	int ret;
> > +
> > +	imx_phycore_snd_device = platform_device_alloc("wm9712-codec", -1);
> > +	if (!imx_phycore_snd_device)
> > +		return -ENOMEM;
> > +
> > +	ret = platform_device_add(imx_phycore_snd_device);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "ASoC: Platform device allocation failed\n");
> > +		goto fail1;
> > +	}
> > +
> > +	ret = snd_soc_register_card(&imx_phycore);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "ASoC: soc card registration failed\n");
> > +		goto fail2;
> > +	}
> > +	return 0;
> > +
> > +fail2:
> > +	platform_device_del(imx_phycore_snd_device);
> > +fail1:
> > +	platform_device_put(imx_phycore_snd_device);
> > +	return ret;
> > +}
> > +
> > +static int imx_phycore_ac97_probe(struct platform_device *pdev)
> > +{
> > +	int ret;
> > +	struct device_node *ssi_np;
> > +	struct device_node *audmux_np;
> > +	const char *sprop;
> > +	const char *p;
> > +
> > +	imx_phycore.dev = &pdev->dev;
> > +
> > +	audmux_np = of_parse_phandle(pdev->dev.of_node, "phytec,audmux", 0);
> > +	if (!audmux_np) {
> > +		dev_err(&pdev->dev, "Failed to parse phytec,audmux phandle\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (of_device_is_compatible(audmux_np, "fsl,imx21-audmux")) {
> > +		phycore_ac97_imx21_audmux();
> > +	} else if (of_device_is_compatible(audmux_np, "fsl,imx31-audmux")) {
> > +		phycore_ac97_imx31_audmux();
> > +	} else {
> > +		dev_err(&pdev->dev, "Unknown audmux, failed to setup audmux\n");
> > +		of_node_put(audmux_np);
> > +		return -EINVAL;
> > +	}
> > +	of_node_put(audmux_np);
> > +
> > +	ssi_np = of_parse_phandle(pdev->dev.of_node, "phytec,ssi", 0);
> > +	if (!ssi_np) {
> > +		dev_err(&pdev->dev, "No valid ssi phandle found\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	imx_phycore_dai_ac97[0].cpu_of_node = ssi_np;
> > +	imx_phycore_dai_ac97[0].platform_of_node = ssi_np;
> > +	phycore_dai_cpu_node = ssi_np;
> > +
> > +	sprop = of_get_property(of_find_node_by_path("/"), "compatible", NULL);
> > +	p = strrchr(sprop, ',');
> > +	if (p)
> > +		sprop = p + 1;
> > +
> > +	if (!strcmp(sprop, "imx27-pca100")) {
> 
> Since this is a phycore/phytec specific ASoC-machine driver, what's the
> problem with matching the machine compatible string by simply calling
> of_machine_is_compatible()?

Changed.

> 
> > +		phycore_ac97_reset = pca100_ac97_cold_reset;
> > +		phycore_ac97_warm_reset = pca100_ac97_warm_reset;
> > +	} else if (!strcmp(sprop, "imx35-pcm043")) {
> > +		phycore_ac97_reset = pcm043_ac97_cold_reset;
> > +		phycore_ac97_warm_reset = pcm043_ac97_warm_reset;
> > +	} else {
> > +		dev_err(&pdev->dev, "Failed to set AC97 reset functions, unknown board.\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	fsl_ssi_ac97_set_reset(phycore_soc_ac97_reset,
> > +			phycore_soc_ac97_warm_reset);
> > +
> > +	ret = phycore_ac97_setup_devices(pdev);
> > +	if (ret)
> > +		of_node_put(phycore_dai_cpu_node);
> > +
> > +	return ret;
> > +}
> > +
> > +static int imx_phycore_ac97_remove(struct platform_device *pdev)
> > +{
> > +	of_node_put(phycore_dai_cpu_node);
> > +	phycore_dai_cpu_node = NULL;
> > +
> > +	platform_device_unregister(imx_phycore_snd_device);
> 
> snd_soc_unregister_card() call is missing?

Yes.

Thanks,

Markus

> 
> Shawn
> 
> > +
> > +	phycore_ac97_reset = NULL;
> > +	phycore_ac97_warm_reset = NULL;
> > +
> > +	return 0;
> > +}
> > +
> > +static struct platform_driver imx_phycore_ac97_driver = {
> > +	.probe		= imx_phycore_ac97_probe,
> > +	.remove		= imx_phycore_ac97_remove,
> > +	.driver		= {
> > +		.name	= DRV_NAME,
> > +		.owner	= THIS_MODULE,
> > +		.of_match_table = imx_phycore_ac97_of_dev_id,
> > +	},
> > +};
> > +
> > +module_platform_driver(imx_phycore_ac97_driver);
> > +
> >  MODULE_AUTHOR("Sascha Hauer <s.hauer at pengutronix.de>");
> > -MODULE_DESCRIPTION("PhyCORE ALSA SoC driver");
> > +MODULE_DESCRIPTION(DRV_NAME ": PhyCORE ALSA SoC fabric driver");
> >  MODULE_LICENSE("GPL");
> > -- 
> > 1.8.2.1
> > 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |


More information about the devicetree-discuss mailing list