All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] dt-bindings: clock: Use "CLK_ETHERNET" for the Ethernet clock
@ 2020-03-22 12:11 Amit Singh Tomar
  2020-03-22 17:43 ` Andreas Färber
  0 siblings, 1 reply; 3+ messages in thread
From: Amit Singh Tomar @ 2020-03-22 12:11 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: robh, manivannan.sadhasivam, afaerber, Amit Singh Tomar, andre.przywara

Right now, dt clock binding for ethernet uses different names CLK_ETH_MAC for S900
and CLK_ETHERNET for S700, while dt clock binding for most of the other devices uses
same name(for instance, the UART clock binding that uses CLK_UARTx).

Let's use same name CLK_ETHERNET for both S700 and S900.

Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
---
Noticed it while working on U-BOOT Ethernet support for S700 where we have common clock driver used
by S700 and S900. Patch[1] was initially sent to U-BOOT list.

[1]: https://patchwork.ozlabs.org/patch/1229219/
---
 drivers/clk/actions/owl-s900.c               | 2 +-
 include/dt-bindings/clock/actions,s900-cmu.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/actions/owl-s900.c b/drivers/clk/actions/owl-s900.c
index 7908909..5293086 100644
--- a/drivers/clk/actions/owl-s900.c
+++ b/drivers/clk/actions/owl-s900.c
@@ -648,7 +648,7 @@ static struct clk_hw_onecell_data s900_hw_clks = {
 		[CLK_DE0]		= &de_clk.common.hw,
 		[CLK_DMM]		= &dmm_clk.common.hw,
 		[CLK_EDP]		= &edp_clk.common.hw,
-		[CLK_ETH_MAC]		= &eth_mac_clk.common.hw,
+		[CLK_ETHERNET]		= &eth_mac_clk.common.hw,
 		[CLK_GPU_CORE]		= &gpu_core_clk.common.hw,
 		[CLK_GPU_MEM]		= &gpu_mem_clk.common.hw,
 		[CLK_GPU_SYS]		= &gpu_sys_clk.common.hw,
diff --git a/include/dt-bindings/clock/actions,s900-cmu.h b/include/dt-bindings/clock/actions,s900-cmu.h
index 7c12515..2247f1c 100644
--- a/include/dt-bindings/clock/actions,s900-cmu.h
+++ b/include/dt-bindings/clock/actions,s900-cmu.h
@@ -121,7 +121,7 @@
 #define CLK_DDR1			97
 #define CLK_DMM				98
 
-#define CLK_ETH_MAC			99
+#define CLK_ETHERNET			99
 #define CLK_RMII_REF			100
 
 #define CLK_NR_CLKS			(CLK_RMII_REF + 1)
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] dt-bindings: clock: Use "CLK_ETHERNET" for the Ethernet clock
  2020-03-22 12:11 [PATCH] dt-bindings: clock: Use "CLK_ETHERNET" for the Ethernet clock Amit Singh Tomar
@ 2020-03-22 17:43 ` Andreas Färber
  2020-03-24 10:07   ` Amit Tomer
  0 siblings, 1 reply; 3+ messages in thread
From: Andreas Färber @ 2020-03-22 17:43 UTC (permalink / raw)
  To: Amit Singh Tomar
  Cc: robh, manivannan.sadhasivam, linux-arm-kernel, andre.przywara

Hi Amit,

Am 22.03.20 um 13:11 schrieb Amit Singh Tomar:
> Right now, dt clock binding for ethernet uses different names CLK_ETH_MAC for S900
> and CLK_ETHERNET for S700, while dt clock binding for most of the other devices uses
> same name(for instance, the UART clock binding that uses CLK_UARTx).
> 
> Let's use same name CLK_ETHERNET for both S700 and S900.
> 
> Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
> ---
> Noticed it while working on U-BOOT Ethernet support for S700 where we have common clock driver used
> by S700 and S900. Patch[1] was initially sent to U-BOOT list.
> 
> [1]: https://patchwork.ozlabs.org/patch/1229219/
> ---
>   drivers/clk/actions/owl-s900.c               | 2 +-
>   include/dt-bindings/clock/actions,s900-cmu.h | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)

Never mix changes to bindings and drivers. Separating them exposes the 
difficulty in the change your proposing:

> diff --git a/drivers/clk/actions/owl-s900.c b/drivers/clk/actions/owl-s900.c
> index 7908909..5293086 100644
> --- a/drivers/clk/actions/owl-s900.c
> +++ b/drivers/clk/actions/owl-s900.c
> @@ -648,7 +648,7 @@ static struct clk_hw_onecell_data s900_hw_clks = {
>   		[CLK_DE0]		= &de_clk.common.hw,
>   		[CLK_DMM]		= &dmm_clk.common.hw,
>   		[CLK_EDP]		= &edp_clk.common.hw,
> -		[CLK_ETH_MAC]		= &eth_mac_clk.common.hw,
> +		[CLK_ETHERNET]		= &eth_mac_clk.common.hw,
>   		[CLK_GPU_CORE]		= &gpu_core_clk.common.hw,
>   		[CLK_GPU_MEM]		= &gpu_mem_clk.common.hw,
>   		[CLK_GPU_SYS]		= &gpu_sys_clk.common.hw,
> diff --git a/include/dt-bindings/clock/actions,s900-cmu.h b/include/dt-bindings/clock/actions,s900-cmu.h
> index 7c12515..2247f1c 100644
> --- a/include/dt-bindings/clock/actions,s900-cmu.h
> +++ b/include/dt-bindings/clock/actions,s900-cmu.h
> @@ -121,7 +121,7 @@
>   #define CLK_DDR1			97
>   #define CLK_DMM				98
>   
> -#define CLK_ETH_MAC			99
> +#define CLK_ETHERNET			99

The bindings are not supposed to change in breaking ways. What you could 
consider instead is to define CLK_ETHERNET as alias, keeping CLK_ETH_MAC 
for backwards compatibility.

Regards,
Andreas

>   #define CLK_RMII_REF			100
>   
>   #define CLK_NR_CLKS			(CLK_RMII_REF + 1)

-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer
HRB 36809 (AG Nürnberg)

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] dt-bindings: clock: Use "CLK_ETHERNET" for the Ethernet clock
  2020-03-22 17:43 ` Andreas Färber
@ 2020-03-24 10:07   ` Amit Tomer
  0 siblings, 0 replies; 3+ messages in thread
From: Amit Tomer @ 2020-03-24 10:07 UTC (permalink / raw)
  To: Andreas Färber
  Cc: robh, Manivannan Sadhasivam, linux-arm-kernel, Andre Przywara

Hi Andreas,

> Never mix changes to bindings and drivers. Separating them exposes the
> difficulty in the change your proposing:

Thanks for letting me know, wasn't aware about this rule.

> > diff --git a/drivers/clk/actions/owl-s900.c b/drivers/clk/actions/owl-s900.c
> > index 7908909..5293086 100644
> > --- a/drivers/clk/actions/owl-s900.c
> > +++ b/drivers/clk/actions/owl-s900.c
> > @@ -648,7 +648,7 @@ static struct clk_hw_onecell_data s900_hw_clks = {
> >               [CLK_DE0]               = &de_clk.common.hw,
> >               [CLK_DMM]               = &dmm_clk.common.hw,
> >               [CLK_EDP]               = &edp_clk.common.hw,
> > -             [CLK_ETH_MAC]           = &eth_mac_clk.common.hw,
> > +             [CLK_ETHERNET]          = &eth_mac_clk.common.hw,
> >               [CLK_GPU_CORE]          = &gpu_core_clk.common.hw,
> >               [CLK_GPU_MEM]           = &gpu_mem_clk.common.hw,
> >               [CLK_GPU_SYS]           = &gpu_sys_clk.common.hw,
> > diff --git a/include/dt-bindings/clock/actions,s900-cmu.h b/include/dt-bindings/clock/actions,s900-cmu.h
> > index 7c12515..2247f1c 100644
> > --- a/include/dt-bindings/clock/actions,s900-cmu.h
> > +++ b/include/dt-bindings/clock/actions,s900-cmu.h
> > @@ -121,7 +121,7 @@
> >   #define CLK_DDR1                    97
> >   #define CLK_DMM                             98
> >
> > -#define CLK_ETH_MAC                  99
> > +#define CLK_ETHERNET                 99
>
> The bindings are not supposed to change in breaking ways. What you could
> consider instead is to define CLK_ETHERNET as alias, keeping CLK_ETH_MAC
> for backwards compatibility.

Sure, I would try using CLK_ETHERNET as alias in U-BOOT and see how it goes.

Thanks,
-Amit

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-03-24 10:08 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-22 12:11 [PATCH] dt-bindings: clock: Use "CLK_ETHERNET" for the Ethernet clock Amit Singh Tomar
2020-03-22 17:43 ` Andreas Färber
2020-03-24 10:07   ` Amit Tomer

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.