Question regarding clocks in the DW-HDMI DT bindings

[Andy Yan]

Hi:


On 2016年11月25日 07:26, Laurent Pinchart wrote:
> Hello Fabio and Vladimir,
>
> Thank you for your quick responses.
>
> On Friday 25 Nov 2016 00:16:00 Vladimir Zapolskiy wrote:
>> On 11/25/2016 12:07 AM, Fabio Estevam wrote:
>>> On Thu, Nov 24, 2016 at 7:16 PM, Laurent Pinchart wrote:
>>>> Hi Andy,
>>>>
>>>> As the author of the DW-HDMI DT bindings this question is addressed to
>>>> you, but information from anyone is more than welcome.
>>>>
>>>> The DT bindings specify two clocks named "iahb" and "isfr" but don't
>>>> describe them. While I assume that the "isfr" clock corresponds to the
>>>> "isfrclk" input signal of the DW HDMI, there is no "iahb" clock
>>>> described in the IP core datasheet.
>>> i.MX6Q has a DW-HDMI IP block.
>>>
>>> The names in the devicetree binding matches the ones listed at the
>>> i.MX6Q Reference Manual - Table 33-1. HDMI Clocks
>> correct, for your convenience the table is copied below:
>>
>> Clock name |     Clock Root     | Description
>> -----------+--------------------+---------------------------------------
>>     iahbclk  | ahb_clk_root       | Bus clock
>>     icecclk  | ckil_sync_clk_root | CEC low-frequency clock (32kHZ)
>>     ihclk    | ahb_clk_root       | Module clock
>>     isfrclk  | video_27m_clk_root | Internal SFR clock (video clock 27MHz)
>>
>> Here AHB stands for ARM Advanced High-performance Bus.
> That's what I suspected. I believe the "iahb" name is wrong, as the DW HDMI TX
> IP core clearly documents the bus clock as being called "iapbclk". We could
> rename that in the DT bindings (with compatibility code in the driver to keep
> supporting the old name) but it might not be worth it. The bindings should
> however document that the "iahb" clock is the IP core's "iapbclk" bus clock.


     I got the clock name from I.MX6Q TRM, I also checked the name again 
with Rockchip IC design team now, hope to get some new information soon.
>
> Another question I have about the bus clock (CC'ing the devicetree mailing
> list as well as the clock maintainers) is whether it should be made optional.
> The clock is obviously mandatory from a hardware point of view (given that APB
> is a synchronous bus and thus requires a clock), but in some SoCs
> (specifically for the Renesas SoCs) that clock is always on and can't be
> controlled. We already omit bus clocks in DT for most IP cores when the clock
> can never be controlled (and we also omit a bunch of other clocks that we
> don't even know exist), so it could make sense to make the clock optional.
> Otherwise there would be runtime overhead trying to handle a clock that can't
> be controlled.

     If this is the case on Renesas SOCs, we can consider make the clock 
as optional. Or move all the clock operations to platform specific 
code(dw_hdmi-rockchip.c/dw_hdmi-imx.c)?

>> By the way while we're discussing DW HDMI bindings specific to iMX,
>> I would recommend to remove utterly hackish and iMX only "gpr"
>> property from the example in bindings/display/bridge/dw_hdmi.txt