mbox series

[0/2] Add code to manage DSP related clocks

Message ID 20210902123216.787025-1-daniel.baluta@oss.nxp.com
Headers show
Series Add code to manage DSP related clocks | expand

Message

Daniel Baluta Sept. 2, 2021, 12:32 p.m. UTC
From: Daniel Baluta <daniel.baluta@nxp.com>

DSP node on the Linux kernel side must also take care of enabling
DAI/DMA related clocks.

By design we choose to manage DAI/DMA clocks from the kernel side
because of the architecture of some i.MX8 boards.

Clocks are handled by a special M4 core which runs a special
firmware called SCFW (System Controler firmware).

This communicates with A cores running Linux via a special Messaging
Unit and implements a custom API which is already implemented by the
Linux kernel i.MX clocks implementation.

Note that these clocks are optional. We can use the DSP without
them.

First patch was already sent to review on SOF Github:
https://github.com/thesofproject/linux/pull/3131

Daniel Baluta (2):
  ASoC: SOF: imx: Add code to manage DSP related clocks
  dt-bindings: dsp: fsl: Add DSP optional clocks documentation

 .../devicetree/bindings/dsp/fsl,dsp.yaml      | 33 ++++++++
 sound/soc/sof/imx/imx-common.c                | 77 +++++++++++++++++++
 sound/soc/sof/imx/imx-common.h                | 16 ++++
 sound/soc/sof/imx/imx8.c                      | 32 ++++++++
 sound/soc/sof/imx/imx8m.c                     | 33 ++++++++
 5 files changed, 191 insertions(+)

Comments

Daniel Baluta Sept. 3, 2021, 2:49 p.m. UTC | #1
On Fri, Sep 3, 2021 at 2:27 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Thu, Sep 02, 2021 at 03:32:15PM +0300, Daniel Baluta wrote:
>
> > +     for (i = 0; i < clks->num_dsp_clks; i++) {
> > +             clks->dsp_clks[i] = devm_clk_get(sdev->dev, clks->dsp_clks_names[i]);
>
> Looks like this could benefit from the use of the clk_bulk_ APIs?

Indeed! I open coded exactly what clk_bulk API is doing! Will use it in v2.
Daniel Baluta Sept. 3, 2021, 3 p.m. UTC | #2
On Thu, Sep 2, 2021 at 11:02 PM Péter Ujfalusi
<peter.ujfalusi@linux.intel.com> wrote:
>
> Hi Daniel,
>
> On 02/09/2021 15:32, Daniel Baluta wrote:
> > From: Daniel Baluta <daniel.baluta@nxp.com>
> >
> > There are two types of clocks:
> >       * DSP IP clocks
> >       * DAI clocks
> >
> > This clocks are necessary in order to power up DSP and DAIs.
> >
> > We choose to enable DAI clocks here because of the way i.MX8/i.MX8X
> > design handles resources (including clocks).
> >
> > All clocks are managed by a separate core (named SCU) which communicates
> > with Linux managed ARM core via a well known API.
> >
> > We parse and enable the clocks in probe function and disable them in
> > remove function.
> >
> > Future patches will introduce Power Management support so that we
> > disable clocks while DSP is not used or system enters power save.
>
> Unfortunately this patch does not apply to next.

Yes, because my patch is based on SOF topic/sof-dev branch and this small patch

https://github.com/thesofproject/linux/commit/b56c58b5938a626fb08fcf1d5e38d687b520ab89

is not in linux-next.

I plan to stay on SOF branch and get the review tags so we can merge
it in SOF tree.
>
> I might be a bit too cautius, but I would also add "&& COMMON_CLK" for
> the COMPILE_TEST in Kconfig or select it from where it is appropriate?

Maybe add a depends on COMMON_CLK for IMX hardware support? Altough,
if CLK support
is not selected clk API transforms itself into dummy wrappers.
>
> > Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
> > ---
> >   sound/soc/sof/imx/imx-common.c | 77 ++++++++++++++++++++++++++++++++++
> >   sound/soc/sof/imx/imx-common.h | 16 +++++++
> >   sound/soc/sof/imx/imx8.c       | 32 ++++++++++++++
> >   sound/soc/sof/imx/imx8m.c      | 33 +++++++++++++++
> >   4 files changed, 158 insertions(+)
> >
> > diff --git a/sound/soc/sof/imx/imx8m.c b/sound/soc/sof/imx/imx8m.c
> > index 30624fafc632..482c25ab15ce 100644
> > --- a/sound/soc/sof/imx/imx8m.c
> > +++ b/sound/soc/sof/imx/imx8m.c
> > @@ -23,6 +23,20 @@
> >   #define MBOX_OFFSET 0x800000
> >   #define MBOX_SIZE   0x1000
> >
> > +#define IMX8M_DSP_CLK_NUM    3
> > +static const char *imx8m_dsp_clks_names[IMX8M_DSP_CLK_NUM] = {
>
> static const char *imx8m_dsp_clks_names[]
>
> + ARRAY_SIZE(imx8m_dsp_clks_names) instead IMX8M_DSP_CLK_NUM ?

Yes, this is a good idea. Already fixed in v2 I sent eariler.