LKML Archive mirror
 help / color / mirror / Atom feed
From: Stephen Boyd <sboyd@codeaurora.org>
To: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: Mike Turquette <mturquette@baylibre.com>,
	David Airlie <airlied@linux.ie>,
	Thierry Reding <thierry.reding@gmail.com>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org,
	dri-devel@lists.freedesktop.org, linux-sunxi@googlegroups.com,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Chen-Yu Tsai <wens@csie.org>, Hans de Goede <hdegoede@redhat.com>,
	Alexander Kaplan <alex@nextthing.co>,
	Boris Brezillon <boris.brezillon@free-electrons.com>,
	Wynter Woods <wynter@nextthing.co>,
	Thomas Petazzoni <thomas.petazzoni@free-electrons.com>,
	Rob Clark <robdclark@gmail.com>, Daniel Vetter <daniel@ffwll.ch>,
	Matthias Brugger <matthias.bgg@gmail.com>
Subject: Re: [PATCH v2 03/26] clk: Add regmap support
Date: Wed, 27 Jan 2016 23:56:21 -0800	[thread overview]
Message-ID: <20160128075621.GF12841@codeaurora.org> (raw)
In-Reply-To: <1452785109-6172-4-git-send-email-maxime.ripard@free-electrons.com>

On 01/14, Maxime Ripard wrote:
> From: Matthias Brugger <matthias.bgg@gmail.com>
> 
> Some devices like SoCs from Mediatek need to use the clock
> through a regmap interface.
> This patch adds regmap support for the simple multiplexer clock,
> the divider clock and the clock gate code.
> 
> Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Nak.

The whole anonymous union thing and clk-io.c file is not
appealing at all. I'd prefer we remove clk_readl/writel, not add
to it. Plus we're bloating the basic clock types and adding a
bunch of parallel registration APIs to assign a regmap pointer.
Really, let's stop adding stuff to the basic clock types, it's
getting out of hand.

To move forward on making regmap clks generic for everyone, let's
make sure that regmap clk code is a library that any driver can
use. No OF_CLK_DECLARE should exist because it doesn't make any
sense. No clk_register_regmap_{div,mux,gate}() functions, just a
clk_register_regmap() function to assign the regmap pointer from
the device. Drivers that want to use the code should need to
select a Kconfig symbol, so that we don't compile in support for
regmap clocks unless we really need them.

If we had a clk_regmap structure with a regmap pointer and a
clk_hw inside it I think that's all we would need. Once we had
that, we could let driver writers wrap that in their own
structures for dividers, muxes, gates, etc. and then have them
call library functions from their clk_ops that takes a regmap (or
clk_regmap struct), and offset to do the get_div/set_div,
get_parent/set_parent, enable/disable stuff.

The policy part can be shared with the basic clock types, because
we've already started libifying them. For example, we can ask the
divider code what the hardware value should be for a particular
divider type and it will tell us without writing any registers.
I had a patch for libifying muxes, not sure where it went. The
point being to leave the I/O part to the regmap code without
putting it behind another layer of indirection buried inside the
basic types. Make things flat and easy to follow.

I haven't thought through making new structs to hold the data for
offsets, masks, etc. but I guess we would want those so that we
could assign functions directly to clk_ops and not require any
boiler plate clk_ops implementations in drivers. There are a few
approaches here: different regmap structs for different basic
types, one mega struct that combines all the needs of the basic
types, or some private void pointer inside struct clk_regmap that
points to different basic type structs. Let's see how that goes.

Maybe we can lift drivers/clk/qcom/clk-regmap.c up into the
drivers/clk/ directory too. In the qcom design I put the
enable/disable bits (gate functionality) directly into the
clk_regmap structure. That may need some more thought if it was
the right idea to force enable/disable on every regmap clock
though. If we remove that and introduce a clk_regmap_gate things
should turn out alright.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

  reply	other threads:[~2016-01-28  7:56 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-14 15:24 [PATCH v2 00/26] drm: Add Allwinner A10 display engine support Maxime Ripard
2016-01-14 15:24 ` [PATCH v2 01/26] reset: Move DT cell size check to the core Maxime Ripard
2016-01-15 15:50   ` Philipp Zabel
2016-01-14 15:24 ` [PATCH v2 02/26] reset: Make reset_control_ops const Maxime Ripard
2016-01-15 15:50   ` Philipp Zabel
2016-01-14 15:24 ` [PATCH v2 03/26] clk: Add regmap support Maxime Ripard
2016-01-28  7:56   ` Stephen Boyd [this message]
2016-01-14 15:24 ` [PATCH v2 04/26] clk: composite: Add unregister function Maxime Ripard
2016-01-14 15:24 ` [PATCH v2 05/26] clk: sunxi: Add display and TCON0 clocks driver Maxime Ripard
2016-01-15  3:01   ` Rob Herring
2016-01-16 14:08   ` [linux-sunxi] " Priit Laes
2016-01-16 15:29   ` Chen-Yu Tsai
2016-02-03 20:18     ` Maxime Ripard
2016-01-14 15:24 ` [PATCH v2 06/26] clk: sunxi: Add PLL3 clock Maxime Ripard
2016-01-15  3:02   ` Rob Herring
2016-01-16 16:05   ` Chen-Yu Tsai
2016-02-03 20:27     ` Maxime Ripard
2016-01-14 15:24 ` [PATCH v2 07/26] clk: sunxi: Add TCON channel1 clock Maxime Ripard
2016-01-15  3:03   ` Rob Herring
2016-01-16 16:36   ` Chen-Yu Tsai
2016-02-03 20:29     ` Maxime Ripard
2016-01-14 15:24 ` [PATCH v2 08/26] clk: sun5i: add DRAM gates Maxime Ripard
2016-01-15  3:04   ` Rob Herring
2016-01-14 15:24 ` [PATCH v2 09/26] ARM: sun5i: dt: Add pll3 and pll7 clocks Maxime Ripard
2016-01-14 15:24 ` [PATCH v2 10/26] ARM: sun5i: a13: Add display and TCON clocks Maxime Ripard
2016-01-16 17:06   ` Chen-Yu Tsai
2016-02-03 20:31     ` Maxime Ripard
2016-02-05  9:49       ` Chen-Yu Tsai
2016-01-14 15:24 ` [PATCH v2 11/26] ARM: sun5i: Add DRAM gates Maxime Ripard
2016-01-16 17:10   ` Chen-Yu Tsai
2016-02-03 20:36     ` Maxime Ripard
2016-01-14 15:24 ` [PATCH v2 12/26] ARM: sun5i: Add TV encoder gate to the DTSI Maxime Ripard
2016-01-16 17:12   ` Chen-Yu Tsai
2016-01-14 15:24 ` [PATCH v2 13/26] drm/fb_cma_helper: Remove implicit call to disable_unused_functions Maxime Ripard
2016-01-14 23:13   ` Laurent Pinchart
2016-01-15 10:17     ` Daniel Vetter
2016-01-24 22:19       ` Laurent Pinchart
2016-01-25  7:29         ` Daniel Vetter
2016-01-25 19:02           ` Laurent Pinchart
2016-01-14 15:24 ` [PATCH v2 14/26] drm/modes: Rewrite the command line parser Maxime Ripard
2016-01-14 15:24 ` [PATCH v2 15/26] drm/modes: Support modes names on the command line Maxime Ripard
2016-01-14 15:24 ` [PATCH v2 16/26] drm: Add Allwinner A10 Display Engine support Maxime Ripard
2016-01-16 15:11   ` [linux-sunxi] " Priit Laes
2016-01-17 12:58     ` Priit Laes
2016-01-19 15:38     ` Maxime Ripard
2016-01-14 15:25 ` [PATCH v2 17/26] drm: sun4i: Add DT bindings documentation Maxime Ripard
2016-01-15  3:15   ` Rob Herring
2016-02-03 19:59     ` Maxime Ripard
2016-02-03 20:19       ` Rob Herring
2016-02-03 20:47         ` Maxime Ripard
2016-01-14 15:25 ` [PATCH v2 18/26] drm: sun4i: Add RGB output Maxime Ripard
2016-01-14 15:25 ` [PATCH v2 19/26] drm: sun4i: Add composite output Maxime Ripard
2016-01-14 15:25 ` [PATCH v2 20/26] drm: sun4i: tv: Add PAL output standard Maxime Ripard
2016-01-14 15:25 ` [PATCH v2 21/26] drm: sun4i: tv: Add NTSC " Maxime Ripard
2016-01-14 15:25 ` [PATCH v2 22/26] ARM: sun5i: r8: Add display blocks to the DTSI Maxime Ripard
2016-01-14 15:25 ` [PATCH v2 23/26] ARM: sun5i: chip: Enable the TV Encoder Maxime Ripard
2016-01-14 15:25 ` [PATCH v2 24/26] devicetree: Add olimex vendor prefix Maxime Ripard
2016-01-15  3:15   ` Rob Herring
2016-01-15  6:41     ` Stefan Wahren
2016-01-15  8:05       ` Maxime Ripard
2016-01-14 15:25 ` [PATCH v2 25/26] drm/panel: simple: Add timings for the Olimex LCD-OLinuXino-4.3TS Maxime Ripard
2016-01-15  3:17   ` Rob Herring
2016-01-14 15:25 ` [PATCH v2 26/26] DO NOT MERGE: ARM: sun5i: chip: Enable the LCD panel Maxime Ripard
2016-02-20 13:45 ` [linux-sunxi] [PATCH v2 00/26] drm: Add Allwinner A10 display engine support Priit Laes

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160128075621.GF12841@codeaurora.org \
    --to=sboyd@codeaurora.org \
    --cc=airlied@linux.ie \
    --cc=alex@nextthing.co \
    --cc=boris.brezillon@free-electrons.com \
    --cc=daniel@ffwll.ch \
    --cc=devicetree@vger.kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=hdegoede@redhat.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sunxi@googlegroups.com \
    --cc=matthias.bgg@gmail.com \
    --cc=maxime.ripard@free-electrons.com \
    --cc=mturquette@baylibre.com \
    --cc=p.zabel@pengutronix.de \
    --cc=robdclark@gmail.com \
    --cc=thierry.reding@gmail.com \
    --cc=thomas.petazzoni@free-electrons.com \
    --cc=wens@csie.org \
    --cc=wynter@nextthing.co \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).