Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 6 Jun 2022 21:17:39 +0100
From:   Mark Brown <broonie@kernel.org>
To:     Martin =?utf-8?Q?Povi=C5=A1er?= <povik+lin@cutebit.org>
Cc:     Liam Girdwood <lgirdwood@gmail.com>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzk+dt@kernel.org>,
        Jaroslav Kysela <perex@perex.cz>,
        Takashi Iwai <tiwai@suse.com>, alsa-devel@alsa-project.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        Mark Kettenis <kettenis@openbsd.org>,
        Hector Martin <marcan@marcan.st>,
        Sven Peter <sven@svenpeter.dev>, asahi@lists.linux.dev
Subject: Re: [RFC PATCH v2 3/5] ASoC: apple: Add MCA platform driver for
 Apple SoCs
Message-ID: <Yp5g43IxFQsUoS/y@sirena.org.uk>
References: <20220606191910.16580-1-povik+lin@cutebit.org>
 <20220606191910.16580-4-povik+lin@cutebit.org>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="elKz+L5SmKmXJeQI"
Content-Disposition: inline
In-Reply-To: <20220606191910.16580-4-povik+lin@cutebit.org>
Precedence: bulk


--elKz+L5SmKmXJeQI
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Mon, Jun 06, 2022 at 09:19:08PM +0200, Martin Povi=C5=A1er wrote:

> +++ b/sound/soc/apple/mca.c
> @@ -0,0 +1,1122 @@
> +/*
> + * Apple SoCs MCA driver

Please add SPDX headers to all your files.

> +		mca_modify(cl, serdes_conf,
> +			SERDES_CONF_SOME_RST, SERDES_CONF_SOME_RST);
> +		(void) readl_relaxed(cl->base + serdes_conf);

Please drop the cast, casts to/from void are generally a warning sign as
they're unneeded in C.  If you want to document the barrier use a
comment or wrapper function.

> +	/*
> +	 * Codecs require clocks at time of umute with the 'mute_stream' op.
> +	 * We need to enable them here at the latest (frontend prepare would
> +	 * be too late).
> +	 */
> +	if (!mca_fe_clocks_in_use(fe_cl)) {
> +		ret =3D mca_fe_enable_clocks(fe_cl);
> +		if (ret < 0)
> +			return ret;
> +	}

This requirement is CODEC specific.  It's fine to bodge around to
satisfy it though, especially given the restricted set of platforms this
can be used with.

> +	fe_cl =3D &mca->clusters[cl->port_driver];
> +	if (!mca_fe_clocks_in_use(fe_cl))
> +		return 0; /* Nothing to do */
> +
> +	cl->clocks_in_use[substream->stream] =3D false;
> +
> +	if (!mca_fe_clocks_in_use(fe_cl))
> +		mca_fe_disable_clocks(fe_cl);

Are you sure this doesn't need locking?

--elKz+L5SmKmXJeQI
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAmKeYOIACgkQJNaLcl1U
h9BVlQf9Hhza64pdhAryt0leuNmoItrjQI4ru3oVzIPzjS1FTHojgISfLilo8RjN
xsVACb5GG0HKM5P/KUmFuH3+niL6W1zxmfO67gzGz0gr5zGpQLvy7XaNc52rVqM5
u5A9NH0Mqv4/OBbjCi8UzbhG8wZfkAIr2B6FENy3KsSbzdUq2crOrOhiS+TwEK8P
gQ0yfnnAAm7oueBiZ0ByCfMHOeBtG6dR3o5Yz+UhnZDmg783E0xvz4okjXcbuo2m
eHMDeulABPHv86DHGZ3KlvUHIK3n8398P1iZxs/0NuJqvNkT4JFcL06Y1TFYeywz
/0ogXX5hoScevm8+OW4gzuc99bGDBA==
=ItfL
-----END PGP SIGNATURE-----

--elKz+L5SmKmXJeQI--