From: Lukasz Majewski <lukma@denx.de>
To: u-boot@lists.denx.de
Subject: [PATCH v2 02/11] clk: Check that ops of composite clock components, exist before calling
Date: Sun, 26 Jan 2020 22:25:58 +0100 [thread overview]
Message-ID: <20200126222558.0e319389@jawa> (raw)
In-Reply-To: <d1c6c726-9cfb-dc2a-cd88-f02f07b5abd0@gmail.com>
Hi Sean,
> clk_composite_ops was shared between all devices in the composite
> clock driver. If one clock had a feature (such as supporting
> set_parent) which another clock did not, it could call a null pointer
> dereference.
>
> This patch does three things
> 1. It adds null-pointer checks to all composite clock functions.
> 2. It makes clk_composite_ops const and sets its functions at
> compile-time. 3. It adds some basic sanity checks to num_parents.
>
> The combined effect of these changes is that any of mux, rate, or
> gate can be NULL, and composite clocks will still function normally.
> Previously, at least mux had to exist, since clk_composite_get_parent
> was used to determine the parent for clk_register.
>
Thank you for those checks - up till now this code was tuned for i.MX
(and in-fact this SoC's clock tree).
However, I would like to first wait for the consensus regarding the
shape of composite clocks (and reply to this mail):
"clk: Always use the supplied struct clk"
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> ---
> drivers/clk/clk-composite.c | 57
> +++++++++++++++++++++++-------------- 1 file changed, 35
> insertions(+), 22 deletions(-)
>
> diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c
> index d0f273d47f..ea9982fd57 100644
> --- a/drivers/clk/clk-composite.c
> +++ b/drivers/clk/clk-composite.c
> @@ -22,7 +22,10 @@ static u8 clk_composite_get_parent(struct clk *clk)
> (struct clk *)dev_get_clk_ptr(clk->dev) : clk);
> struct clk *mux = composite->mux;
>
> - return clk_mux_get_parent(mux);
> + if (mux)
> + return clk_mux_get_parent(mux);
> + else
> + return 0;
> }
>
> static int clk_composite_set_parent(struct clk *clk, struct clk
> *parent) @@ -32,7 +35,10 @@ static int
> clk_composite_set_parent(struct clk *clk, struct clk *parent) const
> struct clk_ops *mux_ops = composite->mux_ops; struct clk *mux =
> composite->mux;
> - return mux_ops->set_parent(mux, parent);
> + if (mux && mux_ops)
> + return mux_ops->set_parent(mux, parent);
> + else
> + return -ENOTSUPP;
> }
>
> static unsigned long clk_composite_recalc_rate(struct clk *clk)
> @@ -42,7 +48,10 @@ static unsigned long
> clk_composite_recalc_rate(struct clk *clk) const struct clk_ops
> *rate_ops = composite->rate_ops; struct clk *rate = composite->rate;
>
> - return rate_ops->get_rate(rate);
> + if (rate && rate_ops)
> + return rate_ops->get_rate(rate);
> + else
> + return clk_get_parent_rate(clk);
> }
>
> static ulong clk_composite_set_rate(struct clk *clk, unsigned long
> rate) @@ -52,7 +61,10 @@ static ulong clk_composite_set_rate(struct
> clk *clk, unsigned long rate) const struct clk_ops *rate_ops =
> composite->rate_ops; struct clk *clk_rate = composite->rate;
>
> - return rate_ops->set_rate(clk_rate, rate);
> + if (rate && rate_ops)
> + return rate_ops->set_rate(clk_rate, rate);
> + else
> + return -ENOTSUPP;
> }
>
> static int clk_composite_enable(struct clk *clk)
> @@ -62,7 +74,10 @@ static int clk_composite_enable(struct clk *clk)
> const struct clk_ops *gate_ops = composite->gate_ops;
> struct clk *gate = composite->gate;
>
> - return gate_ops->enable(gate);
> + if (gate && gate_ops)
> + return gate_ops->enable(gate);
> + else
> + return -ENOTSUPP;
> }
>
> static int clk_composite_disable(struct clk *clk)
> @@ -72,15 +87,12 @@ static int clk_composite_disable(struct clk *clk)
> const struct clk_ops *gate_ops = composite->gate_ops;
> struct clk *gate = composite->gate;
>
> - gate_ops->disable(gate);
> -
> - return 0;
> + if (gate && gate_ops)
> + return gate_ops->disable(gate);
> + else
> + return -ENOTSUPP;
> }
>
> -struct clk_ops clk_composite_ops = {
> - /* This will be set according to clk_register_composite */
> -};
> -
> struct clk *clk_register_composite(struct device *dev, const char
> *name, const char * const *parent_names,
> int num_parents, struct clk *mux,
> @@ -94,7 +106,9 @@ struct clk *clk_register_composite(struct device
> *dev, const char *name, struct clk *clk;
> struct clk_composite *composite;
> int ret;
> - struct clk_ops *composite_ops = &clk_composite_ops;
> +
> + if (!num_parents || (num_parents != 1 && !mux))
> + return ERR_PTR(-EINVAL);
>
> composite = kzalloc(sizeof(*composite), GFP_KERNEL);
> if (!composite)
> @@ -103,8 +117,6 @@ struct clk *clk_register_composite(struct device
> *dev, const char *name, if (mux && mux_ops) {
> composite->mux = mux;
> composite->mux_ops = mux_ops;
> - if (mux_ops->set_parent)
> - composite_ops->set_parent =
> clk_composite_set_parent; mux->data = (ulong)composite;
> }
>
> @@ -113,11 +125,6 @@ struct clk *clk_register_composite(struct device
> *dev, const char *name, clk = ERR_PTR(-EINVAL);
> goto err;
> }
> - composite_ops->get_rate = clk_composite_recalc_rate;
> -
> - /* .set_rate requires either .round_rate or
> .determine_rate */
> - if (rate_ops->set_rate)
> - composite_ops->set_rate =
> clk_composite_set_rate;
> composite->rate = rate;
> composite->rate_ops = rate_ops;
> @@ -132,8 +139,6 @@ struct clk *clk_register_composite(struct device
> *dev, const char *name,
> composite->gate = gate;
> composite->gate_ops = gate_ops;
> - composite_ops->enable = clk_composite_enable;
> - composite_ops->disable = clk_composite_disable;
> gate->data = (ulong)composite;
> }
>
> @@ -160,6 +165,14 @@ err:
> return clk;
> }
>
> +static const struct clk_ops clk_composite_ops = {
> + .set_parent = clk_composite_set_parent,
> + .get_rate = clk_composite_recalc_rate,
> + .set_rate = clk_composite_set_rate,
> + .enable = clk_composite_enable,
> + .disable = clk_composite_disable,
> +};
> +
> U_BOOT_DRIVER(clk_composite) = {
> .name = UBOOT_DM_CLK_COMPOSITE,
> .id = UCLASS_CLK,
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200126/807181c9/attachment.sig>
next prev parent reply other threads:[~2020-01-26 21:25 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-15 22:45 [PATCH v2 00/11] riscv: Add Sipeed Maix support Sean Anderson
2020-01-15 22:47 ` [PATCH v2 01/11] clk: Always use the supplied struct clk Sean Anderson
[not found] ` <752D002CFF5D0F4FA35C0100F1D73F3FA46C88BE@ATCPCS16.andestech.com>
2020-01-21 1:54 ` Rick Chen
2020-01-21 2:02 ` Sean Anderson
2020-01-21 2:23 ` Rick Chen
2020-01-21 3:18 ` Sean Anderson
2020-01-23 5:53 ` Sean Anderson
2020-01-24 14:27 ` Lukasz Majewski
2020-01-24 23:22 ` Sean Anderson
2020-01-25 20:18 ` Lukasz Majewski
2020-01-26 21:20 ` Lukasz Majewski
2020-01-26 22:07 ` Sean Anderson
2020-01-27 23:40 ` Lukasz Majewski
2020-01-28 16:11 ` Sean Anderson
2020-01-30 0:29 ` Lukasz Majewski
2020-01-30 5:47 ` Sean Anderson
2020-01-31 9:18 ` Lukasz Majewski
2020-01-15 22:49 ` [PATCH v2 02/11] clk: Check that ops of composite clock components, exist before calling Sean Anderson
2020-01-26 21:25 ` Lukasz Majewski [this message]
2020-01-15 22:50 ` [PATCH v2 03/11] riscv: Add headers for asm/global_data.h Sean Anderson
[not found] ` <752D002CFF5D0F4FA35C0100F1D73F3FA46C88DF@ATCPCS16.andestech.com>
2020-01-21 2:07 ` Rick Chen
2020-01-21 2:17 ` Sean Anderson
2020-01-26 22:04 ` Lukas Auer
2020-01-26 22:12 ` Sean Anderson
2020-01-26 22:23 ` Lukas Auer
2020-01-15 22:51 ` [PATCH v2 04/11] riscv: Add an option to default to RV64I Sean Anderson
[not found] ` <752D002CFF5D0F4FA35C0100F1D73F3FA46C88FE@ATCPCS16.andestech.com>
2020-01-21 2:16 ` Rick Chen
2020-01-15 22:53 ` [PATCH v2 05/11] riscv: Add option to disable writes to mcounteren Sean Anderson
2020-01-26 22:09 ` Lukas Auer
2020-01-26 22:24 ` Sean Anderson
2020-01-30 22:13 ` Lukas Auer
2020-01-15 22:55 ` [PATCH v2 06/11] riscv: Fix incorrect cpu frequency on RV64 Sean Anderson
2020-01-26 22:04 ` Lukas Auer
2020-01-15 23:04 ` [PATCH v2 07/11] riscv: Add initial Sipeed Maix support Sean Anderson
2020-01-26 22:17 ` Lukas Auer
2020-01-27 1:09 ` Sean Anderson
2020-01-30 22:21 ` Lukas Auer
2020-02-02 6:06 ` Sean Anderson
2020-01-15 23:16 ` [PATCH v2 00/11] riscv: Add " Sean Anderson
2020-01-15 23:18 ` [PATCH v2 08/11] riscv: Add device tree for K210 Sean Anderson
[not found] ` <752D002CFF5D0F4FA35C0100F1D73F3FA46C8947@ATCPCS16.andestech.com>
2020-01-21 2:54 ` Rick Chen
2020-01-15 23:20 ` [PATCH v2 09/11] riscv: Add K210 sysctl support Sean Anderson
2020-01-15 23:24 ` [PATCH v2 10/11] riscv: Add K210 pll support Sean Anderson
2020-01-15 23:26 ` [PATCH v2 11/11] riscv: Add K210 clock support Sean Anderson
2020-01-21 3:46 ` [PATCH v2 08/11] riscv: Add device tree for K210 Sean Anderson
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=20200126222558.0e319389@jawa \
--to=lukma@denx.de \
--cc=u-boot@lists.denx.de \
/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