Messages in this thread | | | Date | Tue, 23 Aug 2022 12:31:32 +0100 | From | Mark Brown <> | Subject | Re: [PATCH v2 3/4] ASoC: apple: mca: Start new platform driver |
| |
On Tue, Aug 23, 2022 at 09:33:36AM +0200, Martin Povišer wrote: > > On 22. 8. 2022, at 19:39, Mark Brown <broonie@kernel.org> wrote: > > On Fri, Aug 19, 2022 at 02:54:29PM +0200, Martin Povišer wrote:
> >> +// SPDX-License-Identifier: GPL-2.0-only > >> +/* > >> + * Apple SoCs MCA driver > >> + * > >> + * Copyright (C) The Asahi Linux Contributors > >> + * > >> + * The MCA peripheral is made up of a number of identical units called clusters.
> > Please make the entire comment block a C++ one so things look more > > intentional.
> Is this so that it does not look like the SPDX header was added > mechanically? I will do it, just curious what the reasoning is.
Yes, broadly.
> >> + /* > >> + * We can't power up the device earlier than this because > >> + * the power state driver would error out on seeing the device > >> + * as clock-gated. > >> + */ > >> + cl->pd_link = device_link_add(mca->dev, cl->pd_dev, > >> + DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME | > >> + DL_FLAG_RPM_ACTIVE);
> > I'm not clear on this dynamically adding and removing device links stuff > > - it looks like the main (only?) purpose is to take a runtime PM > > reference to the target device which is fine but it's not clear why > > device links are involved given that the links are created and destroyed > > every time the DAI is used, AFAICT always in the same fixed > > relationship. It's not a problem, it's just unclear.
> Indeed the only purpose is powering up the cluster’s power domain (there’s > one domain for each cluster). Adding the links is the only way I know to > do it. They need to be added dynamically (as opposed to statically linking, > say, the DAI’s ->dev to the cluster’s ->pd_dev, which I guess may do > something similar), because we need to sequence the power-up/power-down > with the enablement of the clocks.
You could also just do the underlying runtime power management operations directly couldn't you? It's not clear what the device link stuff is adding. [unhandled content-type:application/pgp-signature] | |