Re: [PATCH v2 6/6] ASoC: dt-bindings: renesas,fsi: add support for multiple clocks

[Bui Duc Phuc]

Hi Geert,

Thanks for your feedback.

> Where does this match the driver?
> Usually the functional clock is called "fck".

Regarding the clock name "own", I used it because of the following
implementation in the current driver:

   clock->own = devm_clk_get(dev, NULL);
   if (IS_ERR(clock->own))
      return -EINVAL;

The driver currently fetches the first clock in the list (index 0) and
stores it in a variable named own.
That is why I named it "own" in the DT bindings to match.

However, I have noticed that other DTS files commonly use "fck" for
the functional clock.
Are you suggesting that I should also rename the variable from "own"
to "fck" in the driver code ??
For example:

    clocks = <&mstp2_clks R8A7740_CLK_SCIFA1>;
    clock-names = "fck";

But from the hardware manual, MSTP refers to a "Module Stop Clock",
not a "functional clock".
So I'm not sure if using "fck" here is appropriate. Could you explain
the reasoning behind calling this clock "fck"?

Regarding the FSI clocks, they can be categorized into two types:
audio clocks and module clocks (which may include bus/bridge clocks).
The driver itself does not explicitly handle the enabling/disabling of
the module clock; it only manages the audio clocks.
>From my code tracing:
    At boot: The kernel automatically attaches the PM domain and
prepares the clocks during device initialization.
    During playback (aplay): The FSI driver doesn't enable the module
clock directly. Instead, it is handled via:
    genpd_runtime_resume -> pm_clk_resume -> clk_core_enable ->
cpg_mstp_clock_endisable.
Since this module clock is essential for register access, it must
always be the first entry in the clocks property (index 0) so
devm_clk_get(dev, NULL)
can fetch it correctly, right?
Please let me know if my understanding is incorrect.

Best regards,
Phuc