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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.