[U-Boot] [RESEND PATCH v2 1/4] clk: clk-uclass: Fix style violations
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
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
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
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.
"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
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.
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!
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
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!
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
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!
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!
participants (4)
-
Dr. Philipp Tomsich -
Mario Six -
Simon Glass -
Tom Rini