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