[U-Boot,v4,07/13] dm: clk: Define clk_get_parent() for clkoperations

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 <lukma@denx.de>

---

Changes in v4: None
Changes in v3:
- New patch

 drivers/clk/clk-uclass.c | 15 +++++++++++++++
 include/clk.h            |  9 +++++++++
 2 files changed, 24 insertions(+)

> -----Original Message-----
> From: Lukasz Majewski [mailto:lukma@denx.de]
> Sent: 2019年5月17日 6:11
> To: Stefano Babic <sbabic@denx.de>; Fabio Estevam <festevam@gmail.com>;
> Marek Vasut <marex@denx.de>; Simon Glass <sjg@chromium.org>; Tom Rini
> <trini@konsulko.com>; u-boot@lists.denx.de; Jagan Teki
> <jagan@amarulasolutions.com>; Peng Fan <peng.fan@nxp.com>; Marcel
> Ziswiler <marcel.ziswiler@toradex.com>; Adam Ford <aford173@gmail.com>
> Cc: Lukasz Majewski <lukma@denx.de>; Neil Armstrong
> <narmstrong@baylibre.com>; Philipp Tomsich
> <philipp.tomsich@theobroma-systems.com>; Andreas Dannenberg
> <dannenberg@ti.com>; Fabrice Gasnier <fabrice.gasnier@st.com>; Liviu
> Dudau <Liviu.Dudau@foss.arm.com>
> 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 <lukma@denx.de>
> 
> ---
> 
> 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 <peng.fan@nxp.com>

> +	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

> Subject: Re: [U-Boot] [PATCH v4 07/13] dm: clk: Define clk_get_parent() for
> clk operations
> 
> 
> 
> > -----Original Message-----
> > From: Lukasz Majewski [mailto:lukma@denx.de]
> > Sent: 2019年5月17日 6:11
> > To: Stefano Babic <sbabic@denx.de>; Fabio Estevam
> > <festevam@gmail.com>; Marek Vasut <marex@denx.de>; Simon Glass
> > <sjg@chromium.org>; Tom Rini <trini@konsulko.com>;
> > u-boot@lists.denx.de; Jagan Teki <jagan@amarulasolutions.com>; Peng
> > Fan <peng.fan@nxp.com>; Marcel Ziswiler
> <marcel.ziswiler@toradex.com>;
> > Adam Ford <aford173@gmail.com>
> > Cc: Lukasz Majewski <lukma@denx.de>; Neil Armstrong
> > <narmstrong@baylibre.com>; Philipp Tomsich
> > <philipp.tomsich@theobroma-systems.com>; Andreas Dannenberg
> > <dannenberg@ti.com>; Fabrice Gasnier <fabrice.gasnier@st.com>; Liviu
> > Dudau <Liviu.Dudau@foss.arm.com>
> > 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 <lukma@denx.de>
> >
> > ---
> >
> > 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 <peng.fan@nxp.com>

> 
> Reviewed-by: Peng Fan <peng.fan@nxp.com>
> 
> > +	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&amp;data=02%7C01%7CPeng.Fan%40nxp.com
> %7C9d8b9fb6d116473a8af508d6da8b1d64%7C686ea1d3bc2b4c6fa92cd99c5
> c301635%7C0%7C0%7C636936688386713361&amp;sdata=68OR%2Fa6jjixt2
> h5bs0darbR4%2B5Yz88JD51RfjSCfK3Q%3D&amp;reserved=0

> 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 <lukma@denx.de>
> 
> ---
> 
> 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 <peng.fan@nxp.com>

> 2.11.0

On Thu, 16 May 2019 at 16:11, Lukasz Majewski <lukma@denx.de> 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 <lukma@denx.de>
>
> ---
>
> 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

On Sat, 18 May 2019 10:08:36 -0600
Simon Glass <sjg@chromium.org> wrote:

> On Thu, 16 May 2019 at 16:11, Lukasz Majewski <lukma@denx.de> 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 <lukma@denx.de>
> >
> > ---
> >
> > 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: lukma@denx.de

Hi Lukasz,

On Sat, 18 May 2019 at 14:52, Lukasz Majewski <lukma@denx.de> wrote:
>
> On Sat, 18 May 2019 10:08:36 -0600
> Simon Glass <sjg@chromium.org> wrote:
>
> > On Thu, 16 May 2019 at 16:11, Lukasz Majewski <lukma@denx.de> 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 <lukma@denx.de>
> > >
> > > ---
> > >
> > > 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

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 <lukma@denx.de>
> 
> ---
> 
> 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 <sbabic@denx.de>

Best regards,
Stefano Babic

> 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 <lukma@denx.de>
> Reviewed-by: Peng Fan <peng.fan@nxp.com>
> Reviewed-by: Peng Fan <peng.fan@nxp.com>
> Reviewed-by: Stefano Babic <sbabic@denx.de>

Applied to u-boot-imx, master, thanks !

Best regards,
Stefano Babic