From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751995AbdBOJuD (ORCPT ); Wed, 15 Feb 2017 04:50:03 -0500 Received: from mail.free-electrons.com ([62.4.15.54]:38512 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751544AbdBOJt7 (ORCPT ); Wed, 15 Feb 2017 04:49:59 -0500 Date: Wed, 15 Feb 2017 10:49:54 +0100 From: Maxime Ripard To: Chen-Yu Tsai Cc: Michael Turquette , Stephen Boyd , linux-clk , linux-arm-kernel , linux-kernel Subject: Re: [PATCH 4/5] clk: sunxi-ng: Add driver for A83T CCU Message-ID: <20170215094954.h3wyaxlqkeb342yu@lukather> References: <20170214033526.16977-1-wens@csie.org> <20170214033526.16977-5-wens@csie.org> <20170214095819.utsftcvti5zdmlmi@lukather> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="veriv3zmxufd2jsd" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.6.2-neo (2016-08-21) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --veriv3zmxufd2jsd Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Feb 14, 2017 at 06:26:39PM +0800, Chen-Yu Tsai wrote: > On Tue, Feb 14, 2017 at 5:58 PM, Maxime Ripard > wrote: > > On Tue, Feb 14, 2017 at 11:35:25AM +0800, Chen-Yu Tsai wrote: > >> +/* Some PLLs are input * N / div1 / P. Model them as NKMP with no K */ > > > > Is that even working? >=20 > Looking at the nkmp clock code, only .recalc_rate will work properly thou= gh. > Maybe I could fix up the code so it handles zero width factors. >=20 > > I'm not quite sure we want to do that. We might model it as a NP clock > > with a variable prediv? >=20 > There's no NP clock type yet. And a problem with a variable prediv is that > it doesn't participate in factor calculation. It's effectively fixed. >=20 > I did this for the A80 as well though. Fixing up the NKMP clock might be > easier. Then maybe we just need a NMP clock type then. What I'm really afraid of is that we'll just end up in a clk-factors situation that was simply impossible to maintain without breaking anything, hence why we had different clock types then. > > > >> +/* Use a separate clock for the pre-divider on the AHB1 PLL-PERIPH in= put */ > >> +static SUNXI_CCU_M(pll_periph_ahb1_clk, "pll-periph-ahb1", "pll-perip= h", > >> + 0x054, 6, 2, 0); > >> + > >> +static const char * const ahb1_parents[] =3D { "osc16M-d512", "osc24M= ", > >> + "pll-periph-ahb1", > >> + "pll-periph-ahb1" }; > >> +static struct ccu_div ahb1_clk =3D { > >> + .div =3D _SUNXI_CCU_DIV_FLAGS(4, 2, CLK_DIVIDER_POWER= _OF_TWO), > >> + .mux =3D _SUNXI_CCU_MUX(12, 2), > >> + .common =3D { > >> + .reg =3D 0x054, > >> + .hw.init =3D CLK_HW_INIT_PARENTS("ahb1", > >> + ahb1_parents, > >> + &ccu_div_ops, > >> + 0), > >> + }, > >> +}; > > > > What's different from a pre divider only for a given index here? >=20 > The variable pre-divider is shared for both pll-periph mux inputs. > This is one way to handle it. The other would be to extend ccu_mux > to handle multiple variable pre-dividers. I don't really want to do > that if this is the only instance that needs it though. Every addition we made was only needed by one instance at first :) We are working that way for fixed pre-dividers already, I don't see why we can't have it for variable ones too. > >> +/* > >> + * MMC2 supports what's called the "new timing mode". The CCU and the= MMC > >> + * controller must be in sync about which mode is used. The new mode = moves > >> + * the clock delay controls (and possibly the delay lines) into the M= MC > >> + * block. Also, the output of the clock is divided by 2. The output a= nd > >> + * sample phase clocks are unused under this mode. > >> + * > >> + * This new mode seems to be preferred. Hence we force this clock to = the > >> + * new mode. And we don't add the phase clocks. > >> + */ > > > > I'm sorry, but I said this several times, this isn't working. We > > should model it properly, and not hack this around in the clock > > driver. > > > > As you say in your comment, the MMC driver needs to be aware about > > which mode is used, in order to also set a bit in one of its registers > > accordingly, and modify its sampling behaviour. > > > > The new timing is preferred, but our previous clock implementations > > didn't hardcode it, so we can't even rely on that behaviour to always > > write it in our driver. >=20 > Correct. With the A83T there has never been a merged clock driver though. > I realize this is a one off thing. >=20 > > This is not something specific to the A83T, but is found in all the > > SoCs since the A23, so we need to come up with a good solution to > > address that. > > > > I'm not sure what a good solution would be though. One would be to > > just have a private function of our own to switch in the new mode (if > > relevant, because only the MMC2 controllers have it), but that would > > lead to troubles with !sunxi-ng. Not something we can't deal with, but > > some extra precautions should be taken (make sure to protect the call > > through an ifdef / IS_DEFINED, check that the sunxi-ng driver has been > > probed, etc.) >=20 > If the custom function route is acceptable, I'll come up with something. I think it would be a great start yes. I'll try to discuss it with Mike and Stephen at ELC and see what they think about that. Maxime --=20 Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com --veriv3zmxufd2jsd Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIcBAEBCAAGBQJYpCQ+AAoJEBx+YmzsjxAg23UQAIkStfh/zN6YEABKSD8lhDVV vpyCEN/UU7DAQyL/l+eNS7NdvypbYgFAxcOgOTX6uOhMJOywmlQeVhGsQHmR4bSk V8IfcgbKG0g4TNPKgsKDBxxcNigP0mognpqO98pz1BTfjdheX9AHvpzXsr/uZGhO l5tBAdX1IdkomFiyoAJq8kMXB0Uhwy/iWOLYbXfc9w++tmGFQidyAofuH9L8ttlj 6KRPNaFVuHGbI4V0Et5+C8lcowtePYetr4GxoXVZCGEJbqhW8bmtfTXRSUiyHU9x FPiBOxYpNI5t9AgaCHQMGy/zgYOKiZo4IXl8ii5nCrYcdCyO3eVfay4CjNYw3i5s pVwiPHgofMwRphCm9Te0ENTJsgKbs1yCe/N0GQgxzvuXjuKymkth18fvUzk76f0s IhBA5udss8ZKG6fJ94Tu60lAsQtHLqHOm20yUxUuEDPDRXwm6qEkGKlpjyI1zvme WLDnWTGg8ahp1NmxqQRpC/l+xWgMEvKry7n/sk1vsYQg+aw2ro0oZO/bwl3YqoEO 8ENub7mrtLQ9eXS/KFKDqqm54YxcsfeXXlzhWMBzYLLAhsDk6z2oMidkR0i3gBCb HSmYJOWHovHQuDyXUHFvoFfS+T/crWVsxQsJngBqra0+k79EtBtfXbO+Nj83Fh2o k6qBUy3PrFYSVXdieU5J =eQJI -----END PGP SIGNATURE----- --veriv3zmxufd2jsd--