Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <20190902222015.11360-1-martin.blumenstingl@googlemail.com>
References: <6a3c26bc6e25d883686287883528dbde30725922.1566975410.git.rahul.tanwar@linux.intel.com> <20190902222015.11360-1-martin.blumenstingl@googlemail.com>
Cc:     andriy.shevchenko@intel.com, cheol.yong.kim@intel.com,
        linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org,
        mark.rutland@arm.com, mturquette@baylibre.com,
        qi-ming.wu@intel.com, rahul.tanwar@intel.com, robh+dt@kernel.org,
        robhkernel.org@vger.kernel.org, yixin.zhu@linux.intel.com
Subject: RE: [PATCH v1 1/2] clk: intel: Add CGU clock driver for a new SoC
To:     Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
        rahul.tanwar@linux.intel.com
From:   Stephen Boyd <sboyd@kernel.org>
User-Agent: alot/0.8.1
Date:   Tue, 03 Sep 2019 16:54:07 -0700
Message-Id: <20190903235408.B546D208E3@mail.kernel.org>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Quoting Martin Blumenstingl (2019-09-02 15:20:15)
> +struct intel_clk_gate {
> +       struct clk_hw hw;
> +       struct device *dev;
> +       struct regmap *map;
> +       unsigned int reg;
> +       u8 shift;
> +       unsigned long flags;
> +};
> I know at least two existing regmap clock implementations:
> - drivers/clk/qcom/clk-regmap*
> - drivers/clk/meson/clk-regmap*
>=20
> it would be great if we could decide to re-use one of those for the
> "generic" clock types (mux, divider and gate).
> Stephen, do you have any preference here?
> personally I like the meson one, but I'm biased because I've used it
> a lot in the past and I haven't used the qcom one at all.

The topic comes up once in a while. Making a set of regmap clk_ops and
structures might work to consolidate the code across the different
drivers. I can't recall why we didn't combine the two implementations at
that point. I do remember that Mike was opposed to pushing it directly
into the core framework as part of struct clk_hw out of fear of bloating
the clk_hw structure.

I don't particularly like how the meson implementation makes every clk
the same type and has a 'data' member of clk_regmap to differentiate the
types (mux, gate, div, etc.) It avoids nesting structures but otherwise
serves the same purpose as having there be container structures for the
different types that all have a clk_regmap structure inside. qcom
implementation also takes a shortcut and adds enable/disable logic into
the clk_regmap structure. To consolidate the two I imagine we would need
to change both implementations to use a struct like:

	struct clk_regmap {
		struct clk_hw hw;
		struct regmap *map;
	};