Re: [PATCH v2 05/10] media: adv748x: add support for HDMI audio

[Alex Riesen]

Hi Geert,

Geert Uytterhoeven, Fri, Mar 20, 2020 12:05:20 +0100:
> On Fri, Mar 20, 2020 at 11:58 AM Alex Riesen <alexander.riesen@xxxxxxxxxxx> wrote:
> > Geert Uytterhoeven, Fri, Mar 20, 2020 09:43:29 +0100:
> > > On Thu, Mar 19, 2020 at 6:42 PM Alex Riesen <alexander.riesen@xxxxxxxxxxx> wrote:
> > > > This adds an implemention of SoC DAI driver which provides access to the
> > > > I2S port of the device.
> >
> > I just noticed I don't do clk_prepare_enable anywhere.
> > Shouldn't the clock master enable its clocks somewhere?
> 
> Usually the consumer is responsible for doing that.
> Does the rcar-sound driver do that?

No, it does not (verified by /sys/kernel/debug/clk/clk_summary during
transfer).

> But in this case, perhaps the clock should be enabled implicitly in response
> to a request from the audio subsystem, like you do below.

Ok...

> Note that you register a fixed-rate clock, which is assumed to be always
> enabled. Perhaps a gateable clock type is more appropriate?

The gated clock implementation requires use of an I/O register, which I don't
have in this case (an I2C connected device).

I considered implementing full clk_hw set of operations, but decided against
it: it's a lot for this simple configuration. Few other drivers do that, too.

Regards,
Alex