clk: clk-uclass: Fix style violations
PATCH RESEND 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.
---
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(-)
drivers/clk/clk-uclass.c +10 -10
@@ -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
PATCH RESEND v2 2/4 clk: clk_fixed_rate: Fix style violation
Fix a mis-indented function call in clk_fixed_rate.c
---
v1 -> v2:
None
--- drivers/clk/clk_fixed_rate.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/clk/clk_fixed_rate.c +2 -2
@@ -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
PATCH RESEND v2 2/4 clk: clk_fixed_rate: Fix style violation
Tom,
Quoted 3 lines · Mario Six
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.
Quoted 30 lines
Reviewed-by Simon Glass <[email protected]>Signed-off-by Mario Six <[email protected]>---
v1 -> v2:
None---
drivers/clk/clk_fixed_rate.c| 4 ++--1 file changed, 2 insertions(+), 2 deletions(-)drivers/clk/clk_fixed_rate.c +2
-3@@ -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
PATCH RESEND v2 2/4 clk: clk_fixed_rate: Fix style violation
Quoted 13 lines · Dr. Philipp Tomsich
Tom,
Quoted 3 lines · Mario Six
Fix a mis-indented function call in
clk_fixed_rate.cA 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)?
Quoted 3 lines
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.
Quoted 3 lines
Regards,
Philipp.
Best regards,
Mario
Quoted 31 lines
Quoted 30 lines
Reviewed-by Simon Glass <[email protected]>Signed-off-by Mario Six <[email protected]>---
v1 -> v2:
None---
drivers/clk/clk_fixed_rate.c| 4 ++--1 file changed, 2 insertions(+), 2 deletions(-)drivers/clk/clk_fixed_rate.c +2
-3@@ -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
PATCH RESEND v2 2/4 clk: clk_fixed_rate: Fix style violation
Quoted 6 lines · Mario Six
Quoted 4 lines · Dr. Philipp Tomsich
Tom,
Quoted 2 lines · Mario Six
Quoted 8 lines
Quoted 8 lines
Quoted 1 line
Fix a mis-indented function call in
clk_fixed_rate.cA 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.
Quoted 6 lines
Quoted 3 lines
My gut feeling would be that we should try to change lines only when there is an actual change to the code happening.
Quoted 8 lines
"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.
Signature · 8 lines
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>
PATCH RESEND v2 2/4 clk: clk_fixed_rate: Fix style violation
Quoted 39 lines · Tom Rini
Quoted 6 lines · Mario Six
Quoted 4 lines · Dr. Philipp Tomsich
Tom,
Quoted 2 lines · Mario Six
Quoted 8 lines
Quoted 8 lines
Quoted 1 line
Fix a mis-indented function call in
clk_fixed_rate.cA 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.
Quoted 6 lines
Quoted 3 lines
My gut feeling would be that we should try to change lines only when there is an actual change to the code happening.
Quoted 8 lines
"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.
Quoted 4 lines
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!
PATCH RESEND v2 3/4 clk: Remove superfluous gd declarations
In reply to Mario SixThe clk uclass was converted to support a live device tree recently, hence the global data pointer declarations are no longer needed.
---
v1 -> v2:
None
--- drivers/clk/clk-uclass.c | 2 -- drivers/clk/clk_fixed_rate.c | 2 -- 2 files changed, 4 deletions(-)
drivers/clk/clk-uclass.c +0 -2
@@ -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;
drivers/clk/clk_fixed_rate.c +0 -2
@@ -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
PATCH RESEND v2 3/4 clk: Remove superfluous gd declarations
Quoted 7 lines · 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 <[email protected]>Signed-off-by Mario Six <[email protected]>---
Quoted 7 lines
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!
PATCH RESEND v2 4/4 clk: Makefile: Sort entries alphabetically
In reply to Mario SixThe Makefile entries in the clk driver directory were not alphabetically sorted. Correct this.
---
v1 -> v2:
None
--- drivers/clk/Makefile | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)
drivers/clk/Makefile +10 -10
@@ -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
PATCH RESEND v2 4/4 clk: Makefile: Sort entries alphabetically
Quoted 5 lines · Mario Six
The
Makefileentries in the clk driver directory were not alphabetically sorted. Correct this.Reviewed-by Simon Glass <[email protected]>Signed-off-by Mario Six <[email protected]>
Quoted 8 lines
---
v1 -> v2:
None--- drivers/clk/Makefile | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)
Applied to u-boot-dm, thanks!
PATCH RESEND v2 1/4 clk: clk-uclass: Fix style violations
In reply to Mario SixQuoted 8 lines · Mario Six
checkpatch.pl complains that the
clk_opsstructures used inclk-uclass.cought to be const, so we mark them as const.Reviewed-by Simon Glass <[email protected]>Signed-off-by Mario Six <[email protected]>---
Quoted 7 lines
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!