[U-Boot] [RESEND PATCH v2 1/4] clk: clk-uclass: Fix style violations

[U-Boot] [RESEND PATCH v2 1/4] clk: clk-uclass: Fix style violations

[Mario Six]

checkpatch.pl complains that the clk_ops structures used in clk-uclass.c
ought to be const, so we mark them as const.

Reviewed-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Mario Six <mario.six@gdsys.cc>

---

v1 -> v2:
* Changed commit message and text to used the correct word "const"
  instead of "static".

---
 drivers/clk/clk-uclass.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
index 83ba13374c..32be2e85c5 100644
--- a/drivers/clk/clk-uclass.c
+++ b/drivers/clk/clk-uclass.c
@@ -15,9 +15,9 @@

 DECLARE_GLOBAL_DATA_PTR;

-static inline struct clk_ops *clk_dev_ops(struct udevice *dev)
+static inline const struct clk_ops *clk_dev_ops(struct udevice *dev)
 {
-	return (struct clk_ops *)dev->driver->ops;
+	return (const struct clk_ops *)dev->driver->ops;
 }

 #if CONFIG_IS_ENABLED(OF_CONTROL)
@@ -60,7 +60,7 @@ int clk_get_by_index(struct udevice *dev, int index, struct clk *clk)
 	int ret;
 	struct ofnode_phandle_args args;
 	struct udevice *dev_clk;
-	struct clk_ops *ops;
+	const struct clk_ops *ops;

 	debug("%s(dev=%p, index=%d, clk=%p)\n", __func__, dev, index, clk);

@@ -68,7 +68,7 @@ int clk_get_by_index(struct udevice *dev, int index, struct clk *clk)
 	clk->dev = NULL;

 	ret = dev_read_phandle_with_args(dev, "clocks", "#clock-cells", 0,
-					  index, &args);
+					 index, &args);
 	if (ret) {
 		debug("%s: fdtdec_parse_phandle_with_args failed: err=%d\n",
 		      __func__, ret);
@@ -142,7 +142,7 @@ int clk_release_all(struct clk *clk, int count)

 int clk_request(struct udevice *dev, struct clk *clk)
 {
-	struct clk_ops *ops = clk_dev_ops(dev);
+	const struct clk_ops *ops = clk_dev_ops(dev);

 	debug("%s(dev=%p, clk=%p)\n", __func__, dev, clk);

@@ -156,7 +156,7 @@ int clk_request(struct udevice *dev, struct clk *clk)

 int clk_free(struct clk *clk)
 {
-	struct clk_ops *ops = clk_dev_ops(clk->dev);
+	const struct clk_ops *ops = clk_dev_ops(clk->dev);

 	debug("%s(clk=%p)\n", __func__, clk);

@@ -168,7 +168,7 @@ int clk_free(struct clk *clk)

 ulong clk_get_rate(struct clk *clk)
 {
-	struct clk_ops *ops = clk_dev_ops(clk->dev);
+	const struct clk_ops *ops = clk_dev_ops(clk->dev);

 	debug("%s(clk=%p)\n", __func__, clk);

@@ -180,7 +180,7 @@ ulong clk_get_rate(struct clk *clk)

 ulong clk_set_rate(struct clk *clk, ulong rate)
 {
-	struct clk_ops *ops = clk_dev_ops(clk->dev);
+	const struct clk_ops *ops = clk_dev_ops(clk->dev);

 	debug("%s(clk=%p, rate=%lu)\n", __func__, clk, rate);

@@ -192,7 +192,7 @@ ulong clk_set_rate(struct clk *clk, ulong rate)

 int clk_enable(struct clk *clk)
 {
-	struct clk_ops *ops = clk_dev_ops(clk->dev);
+	const struct clk_ops *ops = clk_dev_ops(clk->dev);

 	debug("%s(clk=%p)\n", __func__, clk);

@@ -204,7 +204,7 @@ int clk_enable(struct clk *clk)

 int clk_disable(struct clk *clk)
 {
-	struct clk_ops *ops = clk_dev_ops(clk->dev);
+	const struct clk_ops *ops = clk_dev_ops(clk->dev);

 	debug("%s(clk=%p)\n", __func__, clk);

--
2.11.0

[U-Boot] [RESEND PATCH v2 2/4] clk: clk_fixed_rate: Fix style violation

[Mario Six]

Fix a mis-indented function call in clk_fixed_rate.c

Reviewed-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Mario Six <mario.six@gdsys.cc>
---

v1 -> v2:
None

---
 drivers/clk/clk_fixed_rate.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/clk_fixed_rate.c b/drivers/clk/clk_fixed_rate.c
index 63565b6ed8..9dd6bc5726 100644
--- a/drivers/clk/clk_fixed_rate.c
+++ b/drivers/clk/clk_fixed_rate.c
@@ -31,8 +31,8 @@ const struct clk_ops clk_fixed_rate_ops = {
 static int clk_fixed_rate_ofdata_to_platdata(struct udevice *dev)
 {
 #if !CONFIG_IS_ENABLED(OF_PLATDATA)
-	to_clk_fixed_rate(dev)->fixed_rate = dev_read_u32_default(dev,
-							"clock-frequency", 0);
+	to_clk_fixed_rate(dev)->fixed_rate =
+		dev_read_u32_default(dev, "clock-frequency", 0);
 #endif

 	return 0;
--
2.11.0

[U-Boot] [RESEND PATCH v2 3/4] clk: Remove superfluous gd declarations

[Mario Six]

The clk uclass was converted to support a live device tree recently,
hence the global data pointer declarations are no longer needed.

Reviewed-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Mario Six <mario.six@gdsys.cc>
---

v1 -> v2:
None

---
 drivers/clk/clk-uclass.c     | 2 --
 drivers/clk/clk_fixed_rate.c | 2 --
 2 files changed, 4 deletions(-)

diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
index 32be2e85c5..fbea72091b 100644
--- a/drivers/clk/clk-uclass.c
+++ b/drivers/clk/clk-uclass.c
@@ -13,8 +13,6 @@
 #include <dt-structs.h>
 #include <errno.h>

-DECLARE_GLOBAL_DATA_PTR;
-
 static inline const struct clk_ops *clk_dev_ops(struct udevice *dev)
 {
 	return (const struct clk_ops *)dev->driver->ops;
diff --git a/drivers/clk/clk_fixed_rate.c b/drivers/clk/clk_fixed_rate.c
index 9dd6bc5726..c9a9f0a20b 100644
--- a/drivers/clk/clk_fixed_rate.c
+++ b/drivers/clk/clk_fixed_rate.c
@@ -8,8 +8,6 @@
 #include <clk-uclass.h>
 #include <dm.h>

-DECLARE_GLOBAL_DATA_PTR;
-
 struct clk_fixed_rate {
 	unsigned long fixed_rate;
 };
--
2.11.0

[U-Boot] [RESEND PATCH v2 4/4] clk: Makefile: Sort entries alphabetically

[Mario Six]

The Makefile entries in the clk driver directory were not alphabetically
sorted. Correct this.

Reviewed-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Mario Six <mario.six@gdsys.cc>
---

v1 -> v2:
None

---
 drivers/clk/Makefile | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 876c2b816f..dab106ab7f 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -6,21 +6,21 @@
 #

 obj-$(CONFIG_$(SPL_TPL_)CLK) += clk-uclass.o clk_fixed_rate.o
-obj-$(CONFIG_ARCH_ROCKCHIP) += rockchip/
-obj-$(CONFIG_SANDBOX) += clk_sandbox.o
-obj-$(CONFIG_SANDBOX) += clk_sandbox_test.o
-obj-$(CONFIG_MACH_PIC32) += clk_pic32.o
-obj-$(CONFIG_CLK_RENESAS) += renesas/
-obj-$(CONFIG_CLK_ZYNQ) += clk_zynq.o
-obj-$(CONFIG_CLK_ZYNQMP) += clk_zynqmp.o

 obj-y += tegra/
-obj-$(CONFIG_CLK_UNIPHIER) += uniphier/
-obj-$(CONFIG_CLK_EXYNOS) += exynos/
+obj-$(CONFIG_ARCH_ASPEED) += aspeed/
+obj-$(CONFIG_ARCH_ROCKCHIP) += rockchip/
 obj-$(CONFIG_CLK_AT91) += at91/
 obj-$(CONFIG_CLK_BCM6345) += clk_bcm6345.o
 obj-$(CONFIG_CLK_BOSTON) += clk_boston.o
+obj-$(CONFIG_CLK_EXYNOS) += exynos/
 obj-$(CONFIG_CLK_HSDK) += clk-hsdk-cgu.o
-obj-$(CONFIG_ARCH_ASPEED) += aspeed/
+obj-$(CONFIG_CLK_RENESAS) += renesas/
 obj-$(CONFIG_CLK_STM32F) += clk_stm32f.o
+obj-$(CONFIG_CLK_UNIPHIER) += uniphier/
+obj-$(CONFIG_CLK_ZYNQ) += clk_zynq.o
+obj-$(CONFIG_CLK_ZYNQMP) += clk_zynqmp.o
+obj-$(CONFIG_MACH_PIC32) += clk_pic32.o
+obj-$(CONFIG_SANDBOX) += clk_sandbox.o
+obj-$(CONFIG_SANDBOX) += clk_sandbox_test.o
 obj-$(CONFIG_STM32H7) += clk_stm32h7.o
--
2.11.0

[U-Boot] [RESEND PATCH v2 2/4] clk: clk_fixed_rate: Fix style violation

[Dr. Philipp Tomsich]

Tom,

> On 15 Jan 2018, at 11:06, Mario Six <mario.six@gdsys.cc> wrote:
> 
> Fix a mis-indented function call in clk_fixed_rate.c

A general question: do we want to have such gardening commits
create an additional indirection in our history for people using
git-blame frequently (e.g. I usually use git-blame to find the last
commit that touched a line and then read the log message to find
out why something was changed… now I’d have to restart this
search whenever I hit a pure formatting change)?

My gut feeling would be that we should try to change lines only
when there is an actual change to the code happening.

Regards,
Philipp.

> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Mario Six <mario.six@gdsys.cc>
> ---
> 
> v1 -> v2:
> None
> 
> ---
> drivers/clk/clk_fixed_rate.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/clk_fixed_rate.c b/drivers/clk/clk_fixed_rate.c
> index 63565b6ed8..9dd6bc5726 100644
> --- a/drivers/clk/clk_fixed_rate.c
> +++ b/drivers/clk/clk_fixed_rate.c
> @@ -31,8 +31,8 @@ const struct clk_ops clk_fixed_rate_ops = {
> static int clk_fixed_rate_ofdata_to_platdata(struct udevice *dev)
> {
> #if !CONFIG_IS_ENABLED(OF_PLATDATA)
> -	to_clk_fixed_rate(dev)->fixed_rate = dev_read_u32_default(dev,
> -							"clock-frequency", 0);
> +	to_clk_fixed_rate(dev)->fixed_rate =
> +		dev_read_u32_default(dev, "clock-frequency", 0);
> #endif
> 
> 	return 0;
> --
> 2.11.0
> 

[U-Boot] [RESEND PATCH v2 2/4] clk: clk_fixed_rate: Fix style violation

[Mario Six]

On Mon, Jan 15, 2018 at 11:19 AM, Dr. Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
> Tom,
>
>> On 15 Jan 2018, at 11:06, Mario Six <mario.six@gdsys.cc> wrote:
>>
>> Fix a mis-indented function call in clk_fixed_rate.c
>
> A general question: do we want to have such gardening commits
> create an additional indirection in our history for people using
> git-blame frequently (e.g. I usually use git-blame to find the last
> commit that touched a line and then read the log message to find
> out why something was changed… now I’d have to restart this
> search whenever I hit a pure formatting change)?
>
> My gut feeling would be that we should try to change lines only
> when there is an actual change to the code happening.
>

From https://www.denx.de/wiki/U-Boot/Patches:

"Non-functional changes, i.e. whitespace and reformatting changes, should be
done in separate patches marked as cosmetic. This separation of functional and
cosmetic changes greatly facilitates the review process."

(granted, I didn't explicitly mark the patches as cosmetic)

I read that as a general permission to post style-fix patches. If there's a
different consensus, I'd like the page modified accordingly.

> Regards,
> Philipp.
>
Best regards,
Mario

>>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>> Signed-off-by: Mario Six <mario.six@gdsys.cc>
>> ---
>>
>> v1 -> v2:
>> None
>>
>> ---
>> drivers/clk/clk_fixed_rate.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/clk/clk_fixed_rate.c b/drivers/clk/clk_fixed_rate.c
>> index 63565b6ed8..9dd6bc5726 100644
>> --- a/drivers/clk/clk_fixed_rate.c
>> +++ b/drivers/clk/clk_fixed_rate.c
>> @@ -31,8 +31,8 @@ const struct clk_ops clk_fixed_rate_ops = {
>> static int clk_fixed_rate_ofdata_to_platdata(struct udevice *dev)
>> {
>> #if !CONFIG_IS_ENABLED(OF_PLATDATA)
>> -     to_clk_fixed_rate(dev)->fixed_rate = dev_read_u32_default(dev,
>> -                                                     "clock-frequency", 0);
>> +     to_clk_fixed_rate(dev)->fixed_rate =
>> +             dev_read_u32_default(dev, "clock-frequency", 0);
>> #endif
>>
>>       return 0;
>> --
>> 2.11.0
>>
>

[U-Boot] [RESEND PATCH v2 2/4] clk: clk_fixed_rate: Fix style violation

[Tom Rini]

On Mon, Jan 15, 2018 at 11:44:46AM +0100, Mario Six wrote:
> On Mon, Jan 15, 2018 at 11:19 AM, Dr. Philipp Tomsich
> <philipp.tomsich@theobroma-systems.com> wrote:
> > Tom,
> >
> >> On 15 Jan 2018, at 11:06, Mario Six <mario.six@gdsys.cc> wrote:
> >>
> >> Fix a mis-indented function call in clk_fixed_rate.c
> >
> > A general question: do we want to have such gardening commits
> > create an additional indirection in our history for people using
> > git-blame frequently (e.g. I usually use git-blame to find the last
> > commit that touched a line and then read the log message to find
> > out why something was changed… now I’d have to restart this
> > search whenever I hit a pure formatting change)?

It does make archaeology harder at times, true.

> > My gut feeling would be that we should try to change lines only
> > when there is an actual change to the code happening.
> >
> 
> From https://www.denx.de/wiki/U-Boot/Patches:
> 
> "Non-functional changes, i.e. whitespace and reformatting changes, should be
> done in separate patches marked as cosmetic. This separation of functional and
> cosmetic changes greatly facilitates the review process."
> 
> (granted, I didn't explicitly mark the patches as cosmetic)
> 
> I read that as a general permission to post style-fix patches. If there's a
> different consensus, I'd like the page modified accordingly.

But this is also true (and yes, so long as the commit is otherwise clear
that it's coding style, etc, fixes, it's not also marked as cosmetic)
that we do in fact want these clean-ups.  The biggest problem I see is
that checkpatch.pl isn't as easily integrated into our workflow as other
CI tools.  So new problems get in.

Now, it's not as hard as it might have been ages ago, and I can drop
checkpatch.pl -q --git origin/master.. into my build scripts and get
something not too bad to review to try and catch pretty bad formatting
problems at least.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180115/032768b1/attachment.sig>

[U-Boot] [RESEND PATCH v2 1/4] clk: clk-uclass: Fix style violations

[Simon Glass]

On 15 January 2018 at 03:06, Mario Six <mario.six@gdsys.cc> wrote:
> checkpatch.pl complains that the clk_ops structures used in clk-uclass.c
> ought to be const, so we mark them as const.
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Mario Six <mario.six@gdsys.cc>
>
> ---
>
> v1 -> v2:
> * Changed commit message and text to used the correct word "const"
>   instead of "static".
>
> ---
>  drivers/clk/clk-uclass.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)

Applied to u-boot-dm, thanks!

[U-Boot] [RESEND PATCH v2 2/4] clk: clk_fixed_rate: Fix style violation

[Simon Glass]

On 15 January 2018 at 06:05, Tom Rini <trini@konsulko.com> wrote:
> On Mon, Jan 15, 2018 at 11:44:46AM +0100, Mario Six wrote:
>> On Mon, Jan 15, 2018 at 11:19 AM, Dr. Philipp Tomsich
>> <philipp.tomsich@theobroma-systems.com> wrote:
>> > Tom,
>> >
>> >> On 15 Jan 2018, at 11:06, Mario Six <mario.six@gdsys.cc> wrote:
>> >>
>> >> Fix a mis-indented function call in clk_fixed_rate.c
>> >
>> > A general question: do we want to have such gardening commits
>> > create an additional indirection in our history for people using
>> > git-blame frequently (e.g. I usually use git-blame to find the last
>> > commit that touched a line and then read the log message to find
>> > out why something was changed… now I’d have to restart this
>> > search whenever I hit a pure formatting change)?
>
> It does make archaeology harder at times, true.
>
>> > My gut feeling would be that we should try to change lines only
>> > when there is an actual change to the code happening.
>> >
>>
>> From https://www.denx.de/wiki/U-Boot/Patches:
>>
>> "Non-functional changes, i.e. whitespace and reformatting changes, should be
>> done in separate patches marked as cosmetic. This separation of functional and
>> cosmetic changes greatly facilitates the review process."
>>
>> (granted, I didn't explicitly mark the patches as cosmetic)
>>
>> I read that as a general permission to post style-fix patches. If there's a
>> different consensus, I'd like the page modified accordingly.
>
> But this is also true (and yes, so long as the commit is otherwise clear
> that it's coding style, etc, fixes, it's not also marked as cosmetic)
> that we do in fact want these clean-ups.  The biggest problem I see is
> that checkpatch.pl isn't as easily integrated into our workflow as other
> CI tools.  So new problems get in.
>
> Now, it's not as hard as it might have been ages ago, and I can drop
> checkpatch.pl -q --git origin/master.. into my build scripts and get
> something not too bad to review to try and catch pretty bad formatting
> problems at least.

I'm assuming this means that we do want this series...

Applied to u-boot-dm, thanks!

[U-Boot] [RESEND PATCH v2 3/4] clk: Remove superfluous gd declarations

[Simon Glass]

On 15 January 2018 at 03:06, Mario Six <mario.six@gdsys.cc> wrote:
> The clk uclass was converted to support a live device tree recently,
> hence the global data pointer declarations are no longer needed.
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Mario Six <mario.six@gdsys.cc>
> ---
>
> v1 -> v2:
> None
>
> ---
>  drivers/clk/clk-uclass.c     | 2 --
>  drivers/clk/clk_fixed_rate.c | 2 --
>  2 files changed, 4 deletions(-)

Applied to u-boot-dm, thanks!

[U-Boot] [RESEND PATCH v2 4/4] clk: Makefile: Sort entries alphabetically

[Simon Glass]

On 15 January 2018 at 03:06, Mario Six <mario.six@gdsys.cc> wrote:
> The Makefile entries in the clk driver directory were not alphabetically
> sorted. Correct this.
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Mario Six <mario.six@gdsys.cc>
> ---
>
> v1 -> v2:
> None
>
> ---
>  drivers/clk/Makefile | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)

Applied to u-boot-dm, thanks!