linux-kernel - RE: [PATCH v1 1/2] clk: intel: Add CGU clock driver for a new SoC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20190903235408.B546D208E3@mail.kernel.org>
Date:   Tue, 03 Sep 2019 16:54:07 -0700
From:   Stephen Boyd <sboyd@...nel.org>
To:     Martin Blumenstingl <martin.blumenstingl@...glemail.com>,
        rahul.tanwar@...ux.intel.com
Cc:     andriy.shevchenko@...el.com, cheol.yong.kim@...el.com,
        linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org,
        mark.rutland@....com, mturquette@...libre.com,
        qi-ming.wu@...el.com, rahul.tanwar@...el.com, robh+dt@...nel.org,
        robhkernel.org@...r.kernel.org, yixin.zhu@...ux.intel.com
Subject: RE: [PATCH v1 1/2] clk: intel: Add CGU clock driver for a new SoC

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*
> 
> 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;
	};