Re: [PATCH v3 01/10] ASoC: dt-bindings: renesas,fsi: add support multiple clocks

[Krzysztof Kozlowski <krzk@kernel.org>]

On 15/05/2026 12:20, Bui Duc Phuc wrote:
> Hi Krzysztof,
> 
> On Fri, May 15, 2026 at 1:46 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>> The FSI on r8a7740 requires the SPU bus/bridge clock to be enabled before
>>> accessing its registers. Without this clock, any register access leads to
>>
>> But why are you adding all these clocks to sh73a0 as well?
>>
> 
> The FSI IP and its clock management seem to be architecturally identical between
> these two SoCs. For instance, both sh73a0 and r8a7740 use the exact same
> register address 0xe6150084 for the SPU DIV6 clock control. Therefore,
> it's highly
> likely they share the same bus dependency for register access.

This must be also mentioned in the commit msg.

> 
> If there are further doubts regarding the sh73a0 internal bus topology, perhaps
> @Geert Uytterhoeven could kindly double-check if this SPU bridge clock
> dependency also applies to sh73a0 as it does for r8a7740?
> 
> 
> 
>>>   - CPG DIV6 clocks (icka/b) as functional clock parents.
>>
>> You do not need to add parents of clocks.
>>
> 
> I see your point. I will update the description to list icka/b simply as
> 'functional clocks' instead of 'parents', as their hierarchy is already
> handled by the clock provider.
> 
> 
> 
>>>   - FSI internal dividers (diva/b) for audio clock generation.
>>
>> Internal dividers do not have representation. They are internal.
> 
> I see your point. What I intended to describe was the internal divider
> configuration for Port A/B within the FSIDIV block, not separate clock
> representations in CCF.
> I will rephrase this as:
> DIVA/DIVB divider settings used for audio clock generation.
> 
> In v1, I brought up this FSIDIV topic with Morimoto and Geert.
> 
>>> By the way, I’d like to discuss the fsidiv clock handling.
>>> In the legacy implementation, it was handled here:
>>> https://elixir.bootlin.com/linux/v7.0-rc7/source/drivers/sh/clk/cpg.c.
>>> Currently, this has not been ported to the Common Clock Framework (CCF) for
>>> R8A7740, and it resides in a different register range from the core CPG.
>>> For v2, would you prefer that I implement a small clock provider for
>>> fsidiv within
>>> the FSI driver, or should it be added under drivers/clk/renesas/?
> 
>> I think it should be under drivers/clk/renesas, but Geert ?
> 
> However, I haven't heard back from Geert yet.
> 
>> This cannot be flexible.
>>
>>> +        - fck  # Main FSI module clock
>>> +        - spu  # optional SPU bus/bridge clock
>>> +        - icka # optional CPG DIV6 functional clocks for FSI port A
>>> +        - ickb # optional CPG DIV6 functional clocks for FSI port B
>>> +        - diva # optional Internal FSI dividers for port A used for audio clock generation
>>> +        - divb # optional Internal FSI dividers for port B used for audio clock generation
>>> +        - xcka # optional External clock inputs for FSI port A provided by the board
>>> +        - xckb # optional External clock inputs for FSI port B provided by the board
> 
> There is also an ongoing discussion about how strict/flexible
> the DT clock constraints should be for FSI in this thread:
> https://lore.kernel.org/all/CAABR9nEhOTz1-0NmCMTbz=-+782Pto0yovSQhBXrXqhLwMg80Q@mail.gmail.com/
> 
> Geert and Rob have already shared some opinions there,
> so it may be useful to continue the discussion in that thread as well.

I missed that before sending my reply.

Best regards,
Krzysztof