All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Boyd <sboyd@kernel.org>
To: "A.s. Dong" <aisheng.dong@nxp.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"mturquette@baylibre.com" <mturquette@baylibre.com>,
	"shawnguo@kernel.org" <shawnguo@kernel.org>,
	dl-linux-imx <linux-imx@nxp.com>,
	"thor.thayer@linux.intel.com" <thor.thayer@linux.intel.com>,
	"linux-clk@vger.kernel.org" <linux-clk@vger.kernel.org>
Subject: RE: [PATCH V3 0/4] clk: new APIs to handle all available clocks
Date: Tue, 28 Aug 2018 20:08:38 -0700	[thread overview]
Message-ID: <153551211897.129321.14630649454821319736@swboyd.mtv.corp.google.com> (raw)
In-Reply-To: <AM0PR04MB4211368561A415B255782CBE803D0@AM0PR04MB4211.eurprd04.prod.outlook.com>

Quoting A.s. Dong (2018-08-16 19:33:52)
> Hi Stephen,
> 
> Do you want me to resend this series for review?
> It seems have been pending for quite a long time.
> 
> Thor just pinged me for its status as he wants to use it.
> 

I was waiting for someone to try them out or review them. Good that it
happened!

I've taken a look at the patches and I'm slightly annoyed with the API
that passes in a double pointer to clk_bulk_data and returns a count of
the number of clks found. I guess it's ok though. It's really just this
line:

	devres->clks = *clks;

which makes my brain all confused and go think about what's being
assigned and if it's a struct copy or not.

Maybe this on top would make it easier to take? I'll think about it
tonight.

---8<---
diff --git a/drivers/clk/clk-devres.c b/drivers/clk/clk-devres.c
index 6d3ca5ec5de8..12c87457eca1 100644
--- a/drivers/clk/clk-devres.c
+++ b/drivers/clk/clk-devres.c
@@ -81,9 +81,9 @@ int __must_check devm_clk_bulk_get_all(struct device *dev,
 	if (!devres)
 		return -ENOMEM;
 
-	ret = clk_bulk_get_all(dev, clks);
+	ret = clk_bulk_get_all(dev, &devres->clks);
 	if (ret > 0) {
-		devres->clks = *clks;
+		*clks = devres->clks;
 		devres->num_clks = ret;
 		devres_add(dev, devres);
 	} else {

WARNING: multiple messages have this Message-ID (diff)
From: Stephen Boyd <sboyd@kernel.org>
To: "A.s. Dong" <aisheng.dong@nxp.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"mturquette@baylibre.com" <mturquette@baylibre.com>,
	"shawnguo@kernel.org" <shawnguo@kernel.org>,
	dl-linux-imx <linux-imx@nxp.com>,
	"thor.thayer@linux.intel.com" <thor.thayer@linux.intel.com>,
	"linux-clk@vger.kernel.org" <linux-clk@vger.kernel.org>
Subject: RE: [PATCH V3 0/4] clk: new APIs to handle all available clocks
Date: Tue, 28 Aug 2018 20:08:38 -0700	[thread overview]
Message-ID: <153551211897.129321.14630649454821319736@swboyd.mtv.corp.google.com> (raw)
In-Reply-To: <AM0PR04MB4211368561A415B255782CBE803D0@AM0PR04MB4211.eurprd04.prod.outlook.com>

Quoting A.s. Dong (2018-08-16 19:33:52)
> Hi Stephen,
> =

> Do you want me to resend this series for review?
> It seems have been pending for quite a long time.
> =

> Thor just pinged me for its status as he wants to use it.
> =


I was waiting for someone to try them out or review them. Good that it
happened!

I've taken a look at the patches and I'm slightly annoyed with the API
that passes in a double pointer to clk_bulk_data and returns a count of
the number of clks found. I guess it's ok though. It's really just this
line:

	devres->clks =3D *clks;

which makes my brain all confused and go think about what's being
assigned and if it's a struct copy or not.

Maybe this on top would make it easier to take? I'll think about it
tonight.

---8<---
diff --git a/drivers/clk/clk-devres.c b/drivers/clk/clk-devres.c
index 6d3ca5ec5de8..12c87457eca1 100644
--- a/drivers/clk/clk-devres.c
+++ b/drivers/clk/clk-devres.c
@@ -81,9 +81,9 @@ int __must_check devm_clk_bulk_get_all(struct device *dev,
 	if (!devres)
 		return -ENOMEM;
 =

-	ret =3D clk_bulk_get_all(dev, clks);
+	ret =3D clk_bulk_get_all(dev, &devres->clks);
 	if (ret > 0) {
-		devres->clks =3D *clks;
+		*clks =3D devres->clks;
 		devres->num_clks =3D ret;
 		devres_add(dev, devres);
 	} else {

WARNING: multiple messages have this Message-ID (diff)
From: sboyd@kernel.org (Stephen Boyd)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH V3 0/4] clk: new APIs to handle all available clocks
Date: Tue, 28 Aug 2018 20:08:38 -0700	[thread overview]
Message-ID: <153551211897.129321.14630649454821319736@swboyd.mtv.corp.google.com> (raw)
In-Reply-To: <AM0PR04MB4211368561A415B255782CBE803D0@AM0PR04MB4211.eurprd04.prod.outlook.com>

Quoting A.s. Dong (2018-08-16 19:33:52)
> Hi Stephen,
> 
> Do you want me to resend this series for review?
> It seems have been pending for quite a long time.
> 
> Thor just pinged me for its status as he wants to use it.
> 

I was waiting for someone to try them out or review them. Good that it
happened!

I've taken a look at the patches and I'm slightly annoyed with the API
that passes in a double pointer to clk_bulk_data and returns a count of
the number of clks found. I guess it's ok though. It's really just this
line:

	devres->clks = *clks;

which makes my brain all confused and go think about what's being
assigned and if it's a struct copy or not.

Maybe this on top would make it easier to take? I'll think about it
tonight.

---8<---
diff --git a/drivers/clk/clk-devres.c b/drivers/clk/clk-devres.c
index 6d3ca5ec5de8..12c87457eca1 100644
--- a/drivers/clk/clk-devres.c
+++ b/drivers/clk/clk-devres.c
@@ -81,9 +81,9 @@ int __must_check devm_clk_bulk_get_all(struct device *dev,
 	if (!devres)
 		return -ENOMEM;
 
-	ret = clk_bulk_get_all(dev, clks);
+	ret = clk_bulk_get_all(dev, &devres->clks);
 	if (ret > 0) {
-		devres->clks = *clks;
+		*clks = devres->clks;
 		devres->num_clks = ret;
 		devres_add(dev, devres);
 	} else {

  parent reply	other threads:[~2018-08-29  3:08 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-25 10:37 [PATCH V3 0/4] clk: new APIs to handle all available clocks Dong Aisheng
2018-05-25 10:37 ` Dong Aisheng
2018-05-25 10:37 ` [PATCH V3 1/4] clk: bulk: add of_clk_bulk_get() Dong Aisheng
2018-05-25 10:37   ` Dong Aisheng
2018-05-25 10:37 ` [PATCH V3 2/4] clk: add new APIs to operate on all available clocks Dong Aisheng
2018-05-25 10:37   ` Dong Aisheng
2018-05-25 10:37 ` [PATCH V3 3/4] clk: add managed version of clk_bulk_get_all Dong Aisheng
2018-05-25 10:37   ` Dong Aisheng
2018-05-25 10:37 ` [PATCH V3 4/4] video: simplefb: switch to use clk_bulk API to simplify clock operations Dong Aisheng
2018-05-25 10:37   ` Dong Aisheng
2018-05-25 10:37   ` Dong Aisheng
2018-06-20  2:53 ` [PATCH V3 0/4] clk: new APIs to handle all available clocks A.s. Dong
2018-06-20  2:53   ` A.s. Dong
2018-06-20  2:53   ` A.s. Dong
2018-08-17  2:33   ` A.s. Dong
2018-08-17  2:33     ` A.s. Dong
2018-08-17  2:33     ` A.s. Dong
2018-08-20 16:32     ` Thor Thayer
2018-08-20 16:32       ` Thor Thayer
2018-08-20 16:32       ` Thor Thayer
2018-08-29  3:08     ` Stephen Boyd [this message]
2018-08-29  3:08       ` Stephen Boyd
2018-08-29  3:08       ` Stephen Boyd
2018-08-29 12:26       ` A.s. Dong
2018-08-29 12:26         ` A.s. Dong
2018-08-29 12:26         ` A.s. Dong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=153551211897.129321.14630649454821319736@swboyd.mtv.corp.google.com \
    --to=sboyd@kernel.org \
    --cc=aisheng.dong@nxp.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-imx@nxp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mturquette@baylibre.com \
    --cc=shawnguo@kernel.org \
    --cc=thor.thayer@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.