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. > > Signed-off-by: Sean Anderson <sean...@gmail.com>
Acked-by: Lukasz Majewski <lu...@denx.de>
(Please commit clk related patches with this patch set)
> ---
> Changes for v3:
> - Don't return an error code where a no-op would be fine
>
> 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..5425f921ff 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 -ENOSYS;
> }
>
> 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 clk_get_rate(clk);
> }
>
> 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 0;
> }
>
> 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 0;
> }
>
> -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: lu...@denx.de
pgpM2kK6eIBs7.pgp
Description: OpenPGP digital signature
