Re: [PATCH 5/5] ASoC: mc13783: Add devicetree support

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1.1: Type: text/plain, Size: 1438 bytes --]

On Sun, Jan 05, 2014 at 11:38:35AM +0400, Alexander Shiyan wrote:
> This patch adds devicetree support for mc13783-codec.
> Since we have not compatible string for this codec, just override
> of_node for allow using phandle of node in the DT.

All DT patches should be sent to the DT maintainers for review.

>  Documentation/devicetree/bindings/mfd/mc13xxx.txt |  4 +++-
>  drivers/mfd/mc13xxx-core.c                        | 12 +++++++-----
> 
Since this has MFD updates you also need to send it to the MFD
maintainers.

>  Optional properties:
>  - fsl,mc13xxx-uses-adc : Indicate the ADC is being used
> -- fsl,mc13xxx-uses-codec : Indicate the Audio Codec is being used

This is an incompatible binding change; deprecating is fine but it
should still be supported.  Is there not a default set of port
assignments that can be used?

> +	pdev->dev.of_node = of_find_node_by_name(pdev->dev.parent->of_node,
> +						 "codec");
> +

You shouldn't be messing with the of_node in the struct device, this is
managed by the driver model and setting one that wasn't added in sync
with everything else can introduce breakage.

> +	if (priv->adc_ssi_port != MC13783_SSI1_PORT &&
> +	    priv->adc_ssi_port != MC13783_SSI2_PORT)
> +		return -EINVAL;
> +	if (priv->dac_ssi_port != MC13783_SSI1_PORT &&
> +	    priv->dac_ssi_port != MC13783_SSI2_PORT)
> +		return -EINVAL;

What about DAC or ADC only systems?

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]