From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mike Turquette Subject: Re: [PATCH 0/7] clk: sun6i: Unify AHB1 clock and fix rate calculation Date: Fri, 26 Sep 2014 11:53:34 -0700 Message-ID: <20140926185334.19023.12876@quantum> References: <1410000448-9999-1-git-send-email-wens@csie.org> <20140911203623.GJ31276@lukather> <20140926002544.19023.39287@quantum> Reply-To: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Return-path: In-Reply-To: List-Post: , List-Help: , List-Archive: , List-Unsubscribe: , To: Chen-Yu Tsai Cc: Maxime Ripard , Emilio Lopez , Vinod Koul , Dan Williams , Grant Likely , Rob Herring , linux-arm-kernel , linux-sunxi , dmaengine-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree List-Id: devicetree@vger.kernel.org Quoting Chen-Yu Tsai (2014-09-25 17:55:27) > On Fri, Sep 26, 2014 at 8:25 AM, Mike Turquette wrote: > > Quoting Maxime Ripard (2014-09-11 13:36:23) > >> Hi Chen-Yu, > >> > >> On Sat, Sep 06, 2014 at 06:47:21PM +0800, Chen-Yu Tsai wrote: > >> > Hi everyone, > >> > > >> > This series unifies the mux and divider parts of the AHB1 clock found > >> > on sun6i and sun8i, while also adding support for the pre-divider on > >> > the PLL6 input. > >> > > >> > The rate calculation logic must factor in which parent it is using to > >> > calculate the rate, to decide whether to use the pre-divider or not. > >> > This is beyond the original factors clk design in sunxi. To avoid > >> > feature bloat, this is implemented as a seperate composite clk. > >> > > >> > The new clock driver is registered with a separate OF_CLK_DECLARE. > >> > This is done so that assigned-clocks* properties on the clk provider > >> > node can actually work. The clock framework arranges the clock setup > >> > order by checking whether all clock parents are available, by checking > >> > the node matching OF_CLK_DECLARE. > >> > > >> > However, the sunxi clk driver is based on the root node compatible, > >> > has no defined dependencies (parents), and is setup before the fixed-rate > >> > clocks. Thus when the ahb1 clock is added, all parents have rate = 0. > >> > There is no way to calculate the required clock factors to set a default > >> > clock rate under these circumstances. This happens when we set the > >> > defaults in the clock node (provider), rather than a clock consumer. > >> > > >> > I can think of 2 ways to solve the dependency issue, but neither is > >> > pretty. One would be to move the root fixed-rate clocks into the sunxi > >> > clk driver. The other would be separating all the clocks into individual > >> > OF_CLK_DECLARE statements, which adds a lot of boilerplate code. > >> > >> I don't know what Mike thinks of this, but I'd prefer the second. > > > > I do not fully understand the problem. Ideally the clock driver should > > have some way to fail with EPROBE_DEFER until the fixed-rate clocks are > > registered. Those fixed-rate parents are enumerated in your dts, right? > > Why isn't this enough? > > This is due to the way the sunxi clock driver is setup. The clock driver's > OF_CLK_DECLARE matches against the "soc" node, not the individual clock > nodes. When the setup function is called, it just registers all the > supported clocks. There are no dependencies listed. > > Unfortunately, the fixed-factor clock is in the middle of the whole clock > tree. The sunxi clock driver provides its parents _and_ its children. > Naturally the clock framework then probes the fixed-factor clock after > the sunxi ones. Any attempts to change the state of clocks under the > unavailable fixed-factor clock, such as done by of_clk_set_defaults(), > would get an incomplete clock, likely with no parents and parent_rate = 0. > That is until of_clk_init() finishes and all clocks are properly hooked > up. > > Anyway, this problem only occurred when I added clk-assigned-* defaults > to the clock provider node, which is not the case anymore. Makes sense. I guess you could ignore the problem until you need to use the assigned defaults. > > The second method i proposed is to have OF_CLK_DECLAREs for each individual > clock. An example can be found here: > > https://github.com/wens/linux/commit/1276898da02a93da4af163ed5bdc88cdead565dc > > This does add a lot of boilerplate code. Not really happy about it. But > it seems the proper way to split up the driver. Yeah, this is OK. Ugly, but OK. Regards, Mike > > > Cheers > ChenYu