Hi Laurent, all, On Fri, Nov 25, 2016 at 7:22 AM, Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> wrote: > Hi Andy, > > On Friday 25 Nov 2016 10:56:53 Andy Yan wrote: >> On 2016年11月25日 07:26, Laurent Pinchart wrote: >> > 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. > > Thank you. While at it, could you ask them which version of the DW HDMI IP > used in the SoC ? > >> > 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)? > > I'd prefer keeping the code generic, otherwise we'd end up with platform- > specific code that would perform the same operations on most platforms. I'll > submit a patch soon to make the clock optional, we can discuss it then. Yes, let's keep the code generic. Absence of a "standard' clock is OK and we should accept the small overhead incurred in providing a solution that works for everyone. This prevents hardware-specific hacks in the driver. Related: we really should model bus clocks whenever possible. I've seen other attempts to merge functional/logic and bus clocks into a single entity (e.g. a single struct clk_hw/clk_core that turns both clocks on and off) and this defeats some fine-grained power management scenarios that the hardware designers had in mind when creating separate controls for the clocks. Regards, Mike > >> >> 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 > > -- > Regards, > > Laurent Pinchart > -- Michael Turquette CEO BayLibre - At the Heart of Embedded Linux http://baylibre.com/ -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
