public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Lukasz Majewski <lukma@denx.de>
To: u-boot@lists.denx.de
Subject: [PATCH v3 02/12] clk: Check that ops of composite clock components, exist before calling
Date: Thu, 6 Feb 2020 22:41:29 +0100	[thread overview]
Message-ID: <20200206224129.167a96c1@jawa> (raw)
In-Reply-To: <29ab2836-b5b7-d57f-84be-f9fe0593ace7@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.
> 
> Signed-off-by: Sean Anderson <seanga2@gmail.com>

Acked-by: Lukasz Majewski <lukma@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: 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/20200206/9c003965/attachment.sig>

  reply	other threads:[~2020-02-06 21:41 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-02 19:56 [PATCH v3 00/12] riscv: Add Sipeed Maix support Sean Anderson
2020-02-02 19:58 ` [PATCH v3 01/12] clk: Always use the supplied struct clk Sean Anderson
2020-02-06 21:21   ` Lukasz Majewski
     [not found]     ` <752D002CFF5D0F4FA35C0100F1D73F3FA46D30F5@ATCPCS16.andestech.com>
2020-02-12  1:23       ` Rick Chen
2020-02-02 19:58 ` [PATCH v3 02/12] clk: Check that ops of composite clock components, exist before calling Sean Anderson
2020-02-06 21:41   ` Lukasz Majewski [this message]
2020-02-02 19:59 ` [PATCH v3 03/12] clk: Unconditionally recursively en-/dis-able clocks Sean Anderson
2020-02-06 21:45   ` Lukasz Majewski
2020-02-02 20:01 ` [PATCH v3 04/12] reset: Add generic reset driver Sean Anderson
2020-02-03  0:04   ` Simon Glass
2020-02-03 23:14     ` Sean Anderson
2020-02-04 11:06   ` Bin Meng
2020-02-04 14:14     ` Sean Anderson
2020-02-02 20:02 ` [PATCH v3 05/12] dm: Add support for simple-pm-bus Sean Anderson
2020-02-03  0:04   ` Simon Glass
2020-02-03 23:15     ` Sean Anderson
2020-02-05  0:16       ` Simon Glass
2020-02-04 11:13   ` Bin Meng
2020-02-02 20:04 ` [PATCH v3 06/12] riscv: Add headers for asm/global_data.h Sean Anderson
2020-02-04 11:17   ` Bin Meng
2020-02-02 20:05 ` [PATCH v3 07/12] riscv: Add option to support RISC-V privileged spec 1.9.1 Sean Anderson
2020-02-04 11:21   ` Bin Meng
2020-02-04 14:19     ` Sean Anderson
2020-02-04 14:38       ` Bin Meng
2020-02-04 14:48         ` Sean Anderson
2020-02-04 16:04           ` Bin Meng
2020-02-04 16:07             ` Sean Anderson
2020-02-02 20:06 ` [PATCH v3 08/12] riscv: Allow use of reset drivers Sean Anderson
2020-02-04 11:22   ` Bin Meng
2020-02-02 20:07 ` [PATCH v3 09/12] riscv: Add K210 pll support Sean Anderson
2020-02-02 20:07 ` [PATCH v3 10/12] riscv: Add K210 clock support Sean Anderson
2020-02-06 21:51   ` Lukasz Majewski
2020-02-02 20:10 ` [PATCH v3 11/12] riscv: Add device tree for K210 Sean Anderson
2020-02-04 11:32   ` Bin Meng
2020-02-04 14:23     ` Sean Anderson
2020-02-04 14:40       ` Bin Meng
2020-02-02 20:10 ` [PATCH v3 12/12] riscv: Add initial Sipeed Maix support Sean Anderson
2020-02-04 11:38   ` Bin Meng
2020-02-04 14:26     ` Sean Anderson
2020-02-04 14:42       ` Bin Meng
2020-02-04 14:49         ` 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=20200206224129.167a96c1@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