Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v3 3/3] ASoC: ti: Add custom machine driver for j721e EVM
 (CPB and IVI)
From:   Peter Ujfalusi <peter.ujfalusi@ti.com>
To:     <broonie@kernel.org>, <lgirdwood@gmail.com>, <robh+dt@kernel.org>
CC:     <alsa-devel@alsa-project.org>, <devicetree@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>
References: <20200612085909.15018-1-peter.ujfalusi@ti.com>
 <20200612085909.15018-4-peter.ujfalusi@ti.com>
Message-ID: <7f2c4297-3ad9-5b8f-c9a5-5120078120c6@ti.com>
Date:   Fri, 12 Jun 2020 12:27:17 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.9.0
MIME-Version: 1.0
In-Reply-To: <20200612085909.15018-4-peter.ujfalusi@ti.com>
Content-Type: text/plain; charset="utf-8"
Content-Language: en-US
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


On 12/06/2020 11.59, Peter Ujfalusi wrote:
> The audio support on the board is using pcm3168a codec connected to McA=
SP10
> serializers in parallel setup.
> The pcm3168a SCKI clock is coming via the j721e AUDIO_REFCLK2 pin.
> In order to support 48KHz and 44.1KHz family of sampling rates the pare=
nt clock
> for AUDIO_REFCLK2 needs to be changed between PLL4 (for 48KHz) and PLL1=
5 (for
> 44.1KHz). The same PLLs are used for McASP10's AUXCLK clock via differe=
nt
> HSDIVIDER.
>=20
> Generic card can not be used for the board as we need to switch between=

> clock paths for different sampling rate families and also need to chang=
e
> the slot_width between 16 and 24 bit audio.
>=20
> The audio support on the Infotainment Expansion Board consists of McASP=
0
> connected to two pcm3168a codecs with dedicated set of serializers to e=
ach.
> The SCKI for pcm3168a is sourced from j721e AUDIO_REFCLK0 pin.
> It is extending the audio support on the CPB.
>=20
> Due to the fact that the same PLL4/15 is used by both domains (CPB/IVI)=

> there are cross restriction on sampling rates.
>=20
> The IVI side is represented as multicodec setup.
>=20
> PCMs available on a plain CPB (no IVI addon):
> hw:0,0 - cpb playback (8 channels)
> hw:0,1 - cpb capture (6 channels)
>=20
> When the IVI addon is present, additional two PCMs will be present:
> hw:0,2 - ivi multicodec playback (16 channels)
> hw:0,3 - ivi multicodec capture (12 channels)
>=20
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> ---
>  sound/soc/ti/Kconfig     |   8 +
>  sound/soc/ti/Makefile    |   2 +
>  sound/soc/ti/j721e-evm.c | 887 +++++++++++++++++++++++++++++++++++++++=

>  3 files changed, 897 insertions(+)
>  create mode 100644 sound/soc/ti/j721e-evm.c

> diff --git a/sound/soc/ti/j721e-evm.c b/sound/soc/ti/j721e-evm.c
> new file mode 100644
> index 000000000000..096f045a1120
> --- /dev/null
> +++ b/sound/soc/ti/j721e-evm.c

=2E..

> +static int j721e_get_clocks(struct device *dev,
> +			    struct j721e_audio_clocks *clocks, char *prefix)
> +{
> +	struct clk *parent;
> +	char *clk_name;
> +	int ret;
> +
> +	clocks->target =3D devm_clk_get(dev, prefix);
> +	if (IS_ERR(clocks->target)) {
> +		ret =3D PTR_ERR(clocks->target);
> +		if (ret !=3D -EPROBE_DEFER)
> +			dev_err(dev, "failed to acquire %s': %d\n",

Looks like I have extra "'" in the prints...

> +				prefix, ret);
> +		return ret;
> +	}
> +
> +	clk_name =3D kasprintf(GFP_KERNEL, "%s-48000", prefix);
> +	if (clk_name) {
> +		parent =3D devm_clk_get(dev, clk_name);
> +		kfree(clk_name);
> +		if (IS_ERR(parent)) {
> +			ret =3D PTR_ERR(parent);
> +			if (ret !=3D -EPROBE_DEFER)
> +				dev_err(dev, "failed to acquire %s': %d\n",
> +					prefix, ret);
> +			return ret;

It should be like this to really not fail with single PLL (from DT), but
it is not documented and supported officially.

if (ret =3D=3D -EPROBE_DEFER)
	return ret;

dev_dbg(dev, "no 48KHz parent for %s: %d\n", prefix, ret);
parent =3D NULL;

> +		}
> +		clocks->parent[J721E_CLK_PARENT_48000] =3D parent;
> +	} else {
> +		return -ENOMEM;
> +	}
> +
> +	clk_name =3D kasprintf(GFP_KERNEL, "%s-44100", prefix);
> +	if (clk_name) {
> +		parent =3D devm_clk_get(dev, clk_name);
> +		kfree(clk_name);
> +		if (IS_ERR(parent)) {
> +			ret =3D PTR_ERR(parent);
> +			if (ret !=3D -EPROBE_DEFER)
> +				dev_err(dev, "failed to acquire %s': %d\n",
> +					prefix, ret);
> +			return ret;

and here

> +		}
> +		clocks->parent[J721E_CLK_PARENT_44100] =3D parent;
> +	} else {
> +		return -ENOMEM;
> +	}

then:

if (!clocks->parent[J721E_CLK_PARENT_44100] &&
    !clocks->parent[J721E_CLK_PARENT_48000]) {
	dev_err(dev, "At least on parent clock is needed for %s\n",
		prefix);
	return -EINVAL;
}

> +
> +	return 0;
> +}

- P=C3=A9ter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki