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