RE: [PATCH v1 1/2] fu540: prci: add request and free clock handlers

[Sagar Kadam]

Hi Sean,

> -Original Message-
> From: Sean Anderson 
> Sent: Friday, February 21, 2020 11:53 AM
> To: Sagar Kadam ; u-boot@lists.denx.de
> Cc: lu...@denx.de; bmeng...@gmail.com; anup.pa...@wdc.com; Paul
> Walmsley ( Sifive) ; Vincent Chen
> 
> Subject: Re: [PATCH v1 1/2] fu540: prci: add request and free clock handlers
> 
> On 2/18/20 11:13 AM, Sagar Shrikant Kadam wrote:
> > +static int sifive_fu540_prci_clk_free(struct clk *clk) {
> > +   debug("%s(clk=%p) (dev=%p, id=%lu)\n", __func__, clk, clk->dev,
> > + clk->id);
> > +
> > +   if (clk->id >= ARRAY_SIZE(__prci_init_clocks))
> > +   return -EINVAL;
> > +
> > +   return 0;
> > +}
> > +
> 
> I don't think this function is necessary, since no struct clk should be 
> passed to
> clk_free except one which was previously successfully requested.
> 
Thanks for suggestion.
I can drop this id check and keep the debug message as done in other similar 
drivers.

BR,
Sagar Kadam

> >  static int sifive_fu540_prci_probe(struct udevice *dev)  {
> > int i, err;
> > @@ -611,6 +633,8 @@ static int sifive_fu540_prci_probe(struct udevice
> > *dev)  static struct clk_ops sifive_fu540_prci_ops = {
> > .set_rate = sifive_fu540_prci_set_rate,
> > .get_rate = sifive_fu540_prci_get_rate,
> > +   .request  = sifive_fu540_prci_clk_request,
> > +   .rfree= sifive_fu540_prci_clk_free,
> >  };
> >
> >  static const struct udevice_id sifive_fu540_prci_ids[] = {
> >
> 
> --Sean

Re: [PATCH v1 1/2] fu540: prci: add request and free clock handlers

[Sean Anderson]

On 2/18/20 11:13 AM, Sagar Shrikant Kadam wrote:
> +static int sifive_fu540_prci_clk_free(struct clk *clk)
> +{
> + debug("%s(clk=%p) (dev=%p, id=%lu)\n", __func__, clk, clk->dev,
> +   clk->id);
> +
> + if (clk->id >= ARRAY_SIZE(__prci_init_clocks))
> + return -EINVAL;
> +
> + return 0;
> +}
> +

I don't think this function is necessary, since no struct clk should be
passed to clk_free except one which was previously successfully
requested.

>  static int sifive_fu540_prci_probe(struct udevice *dev)
>  {
>   int i, err;
> @@ -611,6 +633,8 @@ static int sifive_fu540_prci_probe(struct udevice *dev)
>  static struct clk_ops sifive_fu540_prci_ops = {
>   .set_rate = sifive_fu540_prci_set_rate,
>   .get_rate = sifive_fu540_prci_get_rate,
> + .request  = sifive_fu540_prci_clk_request,
> + .rfree= sifive_fu540_prci_clk_free,
>  };
>  
>  static const struct udevice_id sifive_fu540_prci_ids[] = {
>

--Sean

[PATCH v1 1/2] fu540: prci: add request and free clock handlers

[Sagar Shrikant Kadam]

Add handlers to check if a valid clock id is used to request clock
by any driver using clk_request/clk_free API calls.

Signed-off-by: Sagar Shrikant Kadam 
Tested-by: Vincent Chen 
---
 drivers/clk/sifive/fu540-prci.c | 24 
 1 file changed, 24 insertions(+)

diff --git a/drivers/clk/sifive/fu540-prci.c b/drivers/clk/sifive/fu540-prci.c
index 8847178..ecf29a0 100644
--- a/drivers/clk/sifive/fu540-prci.c
+++ b/drivers/clk/sifive/fu540-prci.c
@@ -580,6 +580,28 @@ static ulong sifive_fu540_prci_set_rate(struct clk *clk, 
ulong rate)
return rate;
 }
 
+static int sifive_fu540_prci_clk_request(struct clk *clk)
+{
+   debug("%s(clk=%p) (dev=%p, id=%lu)\n", __func__, clk, clk->dev,
+ clk->id);
+
+   if (clk->id >= ARRAY_SIZE(__prci_init_clocks))
+   return -EINVAL;
+
+   return 0;
+}
+
+static int sifive_fu540_prci_clk_free(struct clk *clk)
+{
+   debug("%s(clk=%p) (dev=%p, id=%lu)\n", __func__, clk, clk->dev,
+ clk->id);
+
+   if (clk->id >= ARRAY_SIZE(__prci_init_clocks))
+   return -EINVAL;
+
+   return 0;
+}
+
 static int sifive_fu540_prci_probe(struct udevice *dev)
 {
int i, err;
@@ -611,6 +633,8 @@ static int sifive_fu540_prci_probe(struct udevice *dev)
 static struct clk_ops sifive_fu540_prci_ops = {
.set_rate = sifive_fu540_prci_set_rate,
.get_rate = sifive_fu540_prci_get_rate,
+   .request  = sifive_fu540_prci_clk_request,
+   .rfree= sifive_fu540_prci_clk_free,
 };
 
 static const struct udevice_id sifive_fu540_prci_ids[] = {
-- 
2.7.4