Re: [U-Boot] [PATCH v4 07/13] dm: clk: Define clk_get_parent() for clk operations
On Thu, 16 May 2019 at 16:11, Lukasz Majewski wrote: > > This commit adds the clk_get_parent() function, which is responsible > for getting the parent's struct clock pointer. > > U-boot's DM support for getting parent is different (the parent > relationship is in udevice) than the one in common clock framework (CCF) > in Linux. To obtain the pointer to struct clk of parent the > pdev->driver_data field is read. > > Signed-off-by: Lukasz Majewski > > --- > > Changes in v4: None > Changes in v3: > - New patch > > drivers/clk/clk-uclass.c | 15 +++ > include/clk.h| 9 + > 2 files changed, 24 insertions(+) Please can you add a test for this? Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v4 07/13] dm: clk: Define clk_get_parent() for clk operations
On Sat, 18 May 2019 10:08:36 -0600 Simon Glass wrote: > On Thu, 16 May 2019 at 16:11, Lukasz Majewski wrote: > > > > This commit adds the clk_get_parent() function, which is responsible > > for getting the parent's struct clock pointer. > > > > U-boot's DM support for getting parent is different (the parent > > relationship is in udevice) than the one in common clock framework > > (CCF) in Linux. To obtain the pointer to struct clk of parent the > > pdev->driver_data field is read. > > > > Signed-off-by: Lukasz Majewski > > > > --- > > > > Changes in v4: None > > Changes in v3: > > - New patch > > > > drivers/clk/clk-uclass.c | 15 +++ > > include/clk.h| 9 + > > 2 files changed, 24 insertions(+) > > Please can you add a test for this? It is implicitly covered here: http://patchwork.ozlabs.org/patch/1100767/ but maybe shall I add a separate test case? However, considering the other reply to the driver_data field usage, those tests would probably need redesign anyway. > > 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 pgpC_bHcLA7zI.pgp Description: OpenPGP digital signature ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v4 07/13] dm: clk: Define clk_get_parent() for clk operations
Hi Lukasz, On Sat, 18 May 2019 at 14:52, Lukasz Majewski wrote: > > On Sat, 18 May 2019 10:08:36 -0600 > Simon Glass wrote: > > > On Thu, 16 May 2019 at 16:11, Lukasz Majewski wrote: > > > > > > This commit adds the clk_get_parent() function, which is responsible > > > for getting the parent's struct clock pointer. > > > > > > U-boot's DM support for getting parent is different (the parent > > > relationship is in udevice) than the one in common clock framework > > > (CCF) in Linux. To obtain the pointer to struct clk of parent the > > > pdev->driver_data field is read. > > > > > > Signed-off-by: Lukasz Majewski > > > > > > --- > > > > > > Changes in v4: None > > > Changes in v3: > > > - New patch > > > > > > drivers/clk/clk-uclass.c | 15 +++ > > > include/clk.h| 9 + > > > 2 files changed, 24 insertions(+) > > > > Please can you add a test for this? > > It is implicitly covered here: > http://patchwork.ozlabs.org/patch/1100767/ > > but maybe shall I add a separate test case? That's fine I think. We just want to make sure that if someone breaks the code we will notice. > > However, considering the other reply to the driver_data field usage, > those tests would probably need redesign anyway. Yes I really am not keen on that. Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v4 07/13] dm: clk: Define clk_get_parent() for clk operations
On 17/05/19 00:10, Lukasz Majewski wrote:
> This commit adds the clk_get_parent() function, which is responsible
> for getting the parent's struct clock pointer.
>
> U-boot's DM support for getting parent is different (the parent
> relationship is in udevice) than the one in common clock framework (CCF)
> in Linux. To obtain the pointer to struct clk of parent the
> pdev->driver_data field is read.
>
> Signed-off-by: Lukasz Majewski
>
> ---
>
> Changes in v4: None
> Changes in v3:
> - New patch
>
> drivers/clk/clk-uclass.c | 15 +++
> include/clk.h| 9 +
> 2 files changed, 24 insertions(+)
>
> diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
> index 79b3b0494c..1a726dafaa 100644
> --- a/drivers/clk/clk-uclass.c
> +++ b/drivers/clk/clk-uclass.c
> @@ -379,6 +379,21 @@ ulong clk_get_rate(struct clk *clk)
> return ops->get_rate(clk);
> }
>
> +struct clk *clk_get_parent(struct clk *clk)
> +{
> + struct udevice *pdev;
> + struct clk *pclk;
> +
> + debug("%s(clk=%p)\n", __func__, clk);
> +
> + pdev = dev_get_parent(clk->dev);
> + pclk = (struct clk *)dev_get_driver_data(pdev);
> + if (!pclk)
> + return ERR_PTR(-ENODEV);
> +
> + return pclk;
> +}
> +
> ulong clk_set_rate(struct clk *clk, ulong rate)
> {
> const struct clk_ops *ops = clk_dev_ops(clk->dev);
> diff --git a/include/clk.h b/include/clk.h
> index 89dc64bfaf..0873b1e507 100644
> --- a/include/clk.h
> +++ b/include/clk.h
> @@ -259,6 +259,15 @@ int clk_free(struct clk *clk);
> ulong clk_get_rate(struct clk *clk);
>
> /**
> + * clk_get_parent() - Get current clock's parent.
> + *
> + * @clk: A clock struct that was previously successfully requested by
> + * clk_request/get_by_*().
> + * @return pointer to parent's struct clk, or error code passed as pointer
> + */
> +struct clk *clk_get_parent(struct clk *clk);
> +
> +/**
> * clk_set_rate() - Set current clock rate.
> *
> * @clk: A clock struct that was previously successfully requested by
>
Reviewed-by: Stefano Babic
Best regards,
Stefano Babic
--
=
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v4 07/13] dm: clk: Define clk_get_parent() for clk operations
> -Original Message-
> From: Lukasz Majewski [mailto:lu...@denx.de]
> Sent: 2019年5月17日 6:11
> To: Stefano Babic ; Fabio Estevam ;
> Marek Vasut ; Simon Glass ; Tom Rini
> ; u-boot@lists.denx.de; Jagan Teki
> ; Peng Fan ; Marcel
> Ziswiler ; Adam Ford
> Cc: Lukasz Majewski ; Neil Armstrong
> ; Philipp Tomsich
> ; Andreas Dannenberg
> ; Fabrice Gasnier ; Liviu
> Dudau
> Subject: [PATCH v4 07/13] dm: clk: Define clk_get_parent() for clk operations
>
> This commit adds the clk_get_parent() function, which is responsible for
> getting the parent's struct clock pointer.
>
> U-boot's DM support for getting parent is different (the parent relationship
> is
> in udevice) than the one in common clock framework (CCF) in Linux. To obtain
> the pointer to struct clk of parent the
> pdev->driver_data field is read.
>
> Signed-off-by: Lukasz Majewski
>
> ---
>
> Changes in v4: None
> Changes in v3:
> - New patch
>
> drivers/clk/clk-uclass.c | 15 +++
> include/clk.h| 9 +
> 2 files changed, 24 insertions(+)
>
> diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c index
> 79b3b0494c..1a726dafaa 100644
> --- a/drivers/clk/clk-uclass.c
> +++ b/drivers/clk/clk-uclass.c
> @@ -379,6 +379,21 @@ ulong clk_get_rate(struct clk *clk)
> return ops->get_rate(clk);
> }
>
> +struct clk *clk_get_parent(struct clk *clk) {
> + struct udevice *pdev;
> + struct clk *pclk;
> +
> + debug("%s(clk=%p)\n", __func__, clk);
> +
> + pdev = dev_get_parent(clk->dev);
> + pclk = (struct clk *)dev_get_driver_data(pdev);
This has a trick is that force driver_data to struct clk *,
and this requires all clk wrappers needs take clk
as the first element in the wrapper structure.
So better add a comment here. Then it is fine to me, and
Reviewed-by: Peng Fan
> + if (!pclk)
> + return ERR_PTR(-ENODEV);
> +
> + return pclk;
> +}
> +
> ulong clk_set_rate(struct clk *clk, ulong rate) {
> const struct clk_ops *ops = clk_dev_ops(clk->dev); diff --git
> a/include/clk.h b/include/clk.h index 89dc64bfaf..0873b1e507 100644
> --- a/include/clk.h
> +++ b/include/clk.h
> @@ -259,6 +259,15 @@ int clk_free(struct clk *clk); ulong
> clk_get_rate(struct clk *clk);
>
> /**
> + * clk_get_parent() - Get current clock's parent.
> + *
> + * @clk: A clock struct that was previously successfully requested by
> + * clk_request/get_by_*().
> + * @return pointer to parent's struct clk, or error code passed as
> +pointer */ struct clk *clk_get_parent(struct clk *clk);
> +
> +/**
> * clk_set_rate() - Set current clock rate.
> *
> * @clk: A clock struct that was previously successfully requested by
> --
> 2.11.0
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v4 07/13] dm: clk: Define clk_get_parent() for clk operations
> Subject: Re: [U-Boot] [PATCH v4 07/13] dm: clk: Define clk_get_parent() for
> clk operations
>
>
>
> > -Original Message-
> > From: Lukasz Majewski [mailto:lu...@denx.de]
> > Sent: 2019年5月17日 6:11
> > To: Stefano Babic ; Fabio Estevam
> > ; Marek Vasut ; Simon Glass
> > ; Tom Rini ;
> > u-boot@lists.denx.de; Jagan Teki ; Peng
> > Fan ; Marcel Ziswiler
> ;
> > Adam Ford
> > Cc: Lukasz Majewski ; Neil Armstrong
> > ; Philipp Tomsich
> > ; Andreas Dannenberg
> > ; Fabrice Gasnier ; Liviu
> > Dudau
> > Subject: [PATCH v4 07/13] dm: clk: Define clk_get_parent() for clk
> > operations
> >
> > This commit adds the clk_get_parent() function, which is responsible
> > for getting the parent's struct clock pointer.
> >
> > U-boot's DM support for getting parent is different (the parent
> > relationship is in udevice) than the one in common clock framework
> > (CCF) in Linux. To obtain the pointer to struct clk of parent the
> > pdev->driver_data field is read.
> >
> > Signed-off-by: Lukasz Majewski
> >
> > ---
> >
> > Changes in v4: None
> > Changes in v3:
> > - New patch
> >
> > drivers/clk/clk-uclass.c | 15 +++
> > include/clk.h| 9 +
> > 2 files changed, 24 insertions(+)
> >
> > diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c index
> > 79b3b0494c..1a726dafaa 100644
> > --- a/drivers/clk/clk-uclass.c
> > +++ b/drivers/clk/clk-uclass.c
> > @@ -379,6 +379,21 @@ ulong clk_get_rate(struct clk *clk)
> > return ops->get_rate(clk);
> > }
> >
> > +struct clk *clk_get_parent(struct clk *clk) {
> > + struct udevice *pdev;
> > + struct clk *pclk;
> > +
> > + debug("%s(clk=%p)\n", __func__, clk);
> > +
> > + pdev = dev_get_parent(clk->dev);
> > + pclk = (struct clk *)dev_get_driver_data(pdev);
>
> This has a trick is that force driver_data to struct clk *, and this requires
> all clk
> wrappers needs take clk as the first element in the wrapper structure.
> So better add a comment here. Then it is fine to me, and
Sorry, drop my upper comments. The code is correct.
So Reviewed-by: Peng Fan
>
> Reviewed-by: Peng Fan
>
> > + if (!pclk)
> > + return ERR_PTR(-ENODEV);
> > +
> > + return pclk;
> > +}
> > +
> > ulong clk_set_rate(struct clk *clk, ulong rate) {
> > const struct clk_ops *ops = clk_dev_ops(clk->dev); diff --git
> > a/include/clk.h b/include/clk.h index 89dc64bfaf..0873b1e507 100644
> > --- a/include/clk.h
> > +++ b/include/clk.h
> > @@ -259,6 +259,15 @@ int clk_free(struct clk *clk); ulong
> > clk_get_rate(struct clk *clk);
> >
> > /**
> > + * clk_get_parent() - Get current clock's parent.
> > + *
> > + * @clk: A clock struct that was previously successfully requested by
> > + * clk_request/get_by_*().
> > + * @return pointer to parent's struct clk, or error code passed as
> > +pointer */ struct clk *clk_get_parent(struct clk *clk);
> > +
> > +/**
> > * clk_set_rate() - Set current clock rate.
> > *
> > * @clk: A clock struct that was previously successfully requested by
> > --
> > 2.11.0
>
> ___
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.d
> enx.de%2Flistinfo%2Fu-boot&data=02%7C01%7CPeng.Fan%40nxp.com
> %7C9d8b9fb6d116473a8af508d6da8b1d64%7C686ea1d3bc2b4c6fa92cd99c5
> c301635%7C0%7C0%7C636936688386713361&sdata=68OR%2Fa6jjixt2
> h5bs0darbR4%2B5Yz88JD51RfjSCfK3Q%3D&reserved=0
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v4 07/13] dm: clk: Define clk_get_parent() for clk operations
> Subject: [PATCH v4 07/13] dm: clk: Define clk_get_parent() for clk operations
>
> This commit adds the clk_get_parent() function, which is responsible for
> getting the parent's struct clock pointer.
>
> U-boot's DM support for getting parent is different (the parent relationship
> is
> in udevice) than the one in common clock framework (CCF) in Linux. To obtain
> the pointer to struct clk of parent the
> pdev->driver_data field is read.
>
> Signed-off-by: Lukasz Majewski
>
> ---
>
> Changes in v4: None
> Changes in v3:
> - New patch
>
> drivers/clk/clk-uclass.c | 15 +++
> include/clk.h| 9 +
> 2 files changed, 24 insertions(+)
>
> diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c index
> 79b3b0494c..1a726dafaa 100644
> --- a/drivers/clk/clk-uclass.c
> +++ b/drivers/clk/clk-uclass.c
> @@ -379,6 +379,21 @@ ulong clk_get_rate(struct clk *clk)
> return ops->get_rate(clk);
> }
>
> +struct clk *clk_get_parent(struct clk *clk) {
> + struct udevice *pdev;
> + struct clk *pclk;
> +
> + debug("%s(clk=%p)\n", __func__, clk);
> +
> + pdev = dev_get_parent(clk->dev);
> + pclk = (struct clk *)dev_get_driver_data(pdev);
> + if (!pclk)
> + return ERR_PTR(-ENODEV);
> +
> + return pclk;
> +}
> +
> ulong clk_set_rate(struct clk *clk, ulong rate) {
> const struct clk_ops *ops = clk_dev_ops(clk->dev); diff --git
> a/include/clk.h b/include/clk.h index 89dc64bfaf..0873b1e507 100644
> --- a/include/clk.h
> +++ b/include/clk.h
> @@ -259,6 +259,15 @@ int clk_free(struct clk *clk); ulong
> clk_get_rate(struct clk *clk);
>
> /**
> + * clk_get_parent() - Get current clock's parent.
> + *
> + * @clk: A clock struct that was previously successfully requested by
> + * clk_request/get_by_*().
> + * @return pointer to parent's struct clk, or error code passed as
> +pointer */ struct clk *clk_get_parent(struct clk *clk);
> +
> +/**
> * clk_set_rate() - Set current clock rate.
> *
> * @clk: A clock struct that was previously successfully requested by
> --
Reviewed-by: Peng Fan
> 2.11.0
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
