Re: [U-Boot] [PATCH v2 04/17] dm: clk: Define clk_get_by_id() for clk operations

[Simon Glass]

On Thu, 31 Jan 2019 at 02:04, Lukasz Majewski  wrote:
>
> This commit adds the clk_get_by_id() function, which is responsible
> for getting the udevice with matching clk->id. Such approach allows
> re-usage of inherit DM list relationship for the same class (UCLASS_CLK).
> As a result - we don't need any other external list - it is just enough
> to look for UCLASS_CLK related udevices.
>
> Signed-off-by: Lukasz Majewski 
> ---
>
> Changes in v2: None
>
>  drivers/clk/clk-uclass.c | 22 ++
>  include/clk.h| 11 +++
>  2 files changed, 33 insertions(+)
>

Please add a test that calls this.

> diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
> index f1640dda67..12ec0baa74 100644
> --- a/drivers/clk/clk-uclass.c
> +++ b/drivers/clk/clk-uclass.c
> @@ -455,6 +455,28 @@ int clk_disable_bulk(struct clk_bulk *bulk)
> return 0;
>  }
>
> +int clk_get_by_id(ulong id, struct clk **c)

Can you use clkp instead of c?

> +{
> +   struct udevice *dev;
> +   struct uclass *uc;
> +   int ret;
> +
> +   ret = uclass_get(UCLASS_CLK, &uc);
> +   if (ret)
> +   return ret;
> +
> +   uclass_foreach_dev(dev, uc) {
> +   struct clk *clk = (struct clk *)dev_get_driver_data(dev);
> +
> +   if (clk->id == id) {
> +   *c = clk;
> +   return 0;
> +   }
> +   }
> +
> +   return -ENODEV;

I wonder if -ENOENT would be better?

> +}
> +
>  UCLASS_DRIVER(clk) = {
> .id = UCLASS_CLK,
> .name   = "clk",
> diff --git a/include/clk.h b/include/clk.h
> index 8224295ec3..045e60357d 100644
> --- a/include/clk.h
> +++ b/include/clk.h
> @@ -315,4 +315,15 @@ static inline bool clk_valid(struct clk *clk)
>  {
> return !!clk->dev;
>  }
> +
> +/**
> + * clk_get_by_id() - Get the clock by knowing its ID
> + *
> + * @id:The clock ID to search for
> + *
> + * @c: A pointer to clock struct that has been found among added clocks
> + *  to UCLASS_CLK
> + * @return zero on success, or -ve error code.

-NOENT on error? I think you can be specific here

> + */
> +int clk_get_by_id(ulong id, struct clk **c);
>  #endif
> --
> 2.11.0
>

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v2 04/17] dm: clk: Define clk_get_by_id() for clk operations

[Lukasz Majewski]

Hi Simon,

> On Thu, 31 Jan 2019 at 02:04, Lukasz Majewski  wrote:
> >
> > This commit adds the clk_get_by_id() function, which is responsible
> > for getting the udevice with matching clk->id. Such approach allows
> > re-usage of inherit DM list relationship for the same class
> > (UCLASS_CLK). As a result - we don't need any other external list -
> > it is just enough to look for UCLASS_CLK related udevices.
> >
> > Signed-off-by: Lukasz Majewski 
> > ---
> >
> > Changes in v2: None
> >
> >  drivers/clk/clk-uclass.c | 22 ++
> >  include/clk.h| 11 +++
> >  2 files changed, 33 insertions(+)
> >  
> 
> Please add a test that calls this.

Ok.

> 
> > diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
> > index f1640dda67..12ec0baa74 100644
> > --- a/drivers/clk/clk-uclass.c
> > +++ b/drivers/clk/clk-uclass.c
> > @@ -455,6 +455,28 @@ int clk_disable_bulk(struct clk_bulk *bulk)
> > return 0;
> >  }
> >
> > +int clk_get_by_id(ulong id, struct clk **c)  
> 
> Can you use clkp instead of c?

Ok.

> 
> > +{
> > +   struct udevice *dev;
> > +   struct uclass *uc;
> > +   int ret;
> > +
> > +   ret = uclass_get(UCLASS_CLK, &uc);
> > +   if (ret)
> > +   return ret;
> > +
> > +   uclass_foreach_dev(dev, uc) {
> > +   struct clk *clk = (struct clk
> > *)dev_get_driver_data(dev); +
> > +   if (clk->id == id) {
> > +   *c = clk;
> > +   return 0;
> > +   }
> > +   }
> > +
> > +   return -ENODEV;  
> 
> I wonder if -ENOENT would be better?

Yes, it would better describe the situation.

> 
> > +}
> > +
> >  UCLASS_DRIVER(clk) = {
> > .id = UCLASS_CLK,
> > .name   = "clk",
> > diff --git a/include/clk.h b/include/clk.h
> > index 8224295ec3..045e60357d 100644
> > --- a/include/clk.h
> > +++ b/include/clk.h
> > @@ -315,4 +315,15 @@ static inline bool clk_valid(struct clk *clk)
> >  {
> > return !!clk->dev;
> >  }
> > +
> > +/**
> > + * clk_get_by_id() - Get the clock by knowing its ID
> > + *
> > + * @id:The clock ID to search for
> > + *
> > + * @c: A pointer to clock struct that has been found among added
> > clocks
> > + *  to UCLASS_CLK
> > + * @return zero on success, or -ve error code.  
> 
> -NOENT on error? I think you can be specific here

Ok.

> 
> > + */
> > +int clk_get_by_id(ulong id, struct clk **c);
> >  #endif
> > --
> > 2.11.0
> >  
> 
> Regards,
> Simon


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


pgpWrntwN68aJ.pgp
Description: OpenPGP digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot