Re: [PATCH 1/2] clk/zynq/clkc: Add 'fclk-enable' feature

[SÃren Brinkmann]

On Thu, Oct 10, 2013 at 07:21:40AM +0200, Michal Simek wrote:
> On 10/09/2013 05:25 PM, SÃren Brinkmann wrote:
> > On Tue, Oct 08, 2013 at 04:38:17PM +0100, Mark Rutland wrote:
> >> On Tue, Oct 08, 2013 at 03:36:11PM +0100, Soren Brinkmann wrote:
> >>> In some use cases Zynq's FPGA clocks are used as static clock
> >>> generators for IP in the FPGA part of the SOC for which no Linux driver
> >>> exists and would control those clocks. To avoid automatic
> >>> gating of these clocks in such cases a new property - fclk-enable - is
> >>> added to the clock controller's DT description to accomodate such use
> >>> cases. It's value is a bitmask, where a set bit results in enabling
> >>> the corresponding FCLK through the clkc.
> >>>
> >>> FPGA clocks are handled following the rules below:
> >>>
> >>> If an FCLK is not enabled by bootloaders, that FCLK will be disabled in
> >>> Linux. Drivers can enable and control it through the CCF as usual.
> >>>
> >>> If an FCLK is enabled by bootloaders AND the corresponding bit in the
> >>> 'fclk-enable' DT property is set, that FCLK will be enabled by the clkc,
> >>> resulting in an off by one reference count for that clock. Ensuring it
> >>> will always be running.
> >>>
> >>> The default value for 'fclk-enable' is '0xf' (all FCLK's enabled by the
> >>> bootloader are enabled through the clkc.
> >>
> >> Why? Juding by the diff that's not what the code currently does, so why
> >> not leave it as 0, and only set it where required as a work-around?
> > The default is set as described here:
> > [...]
> >>> +	ret = of_property_read_u32(np, "fclk-enable", &fclk_enable);
> >>> +	if (ret)
> >>> +		fclk_enable = 0xf;
> >>> +
> > 
> > The default has been chosen this way, because it avoids trouble with
> > customers. Customers expect the FPGA clocks to be running, once they
> > configured them in the FPGA tools. If Linux gates them off for some
> > reason, it creates confusion and often ends up in customer support.
> > But in general I agree. It is kind of a work around and having zero as
> > default would be desirable. I'd leave this to Michal.
> 
> No problem to have default value setup to 0. And use this workaround
> when it is set in DTS.
All right, I'll prepare v2 with that default changed to 0.

	SÃren


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/