Message ID | 1579954983-11329-10-git-send-email-amittomer25@gmail.com |
---|---|
State | Deferred |
Delegated to: | Tom Rini |
Headers | show |
Series | Actions S700 SoC support | expand |
On Sat, Jan 25, 2020 at 05:52:51PM +0530, Amit Singh Tomar wrote: > Right now, Clock bindings for ethernet uses different names(even in Linux) > CLK_ETH_MAC for S900 and CLK_ETHERNET for S700, It causes compilation problem > when using them for common clock driver. > > Let's use same name CLK_ETHERNET for both S700 and S900. > > Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> Thanks, Mani > --- > Changes since v2: > * Newly added patch, not there in v2/v1. > --- > include/dt-bindings/clock/actions,s900-cmu.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > 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 >
On Sun, 23 Feb 2020 23:08:25 +0530 Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> wrote: Hi Amit, > On Sat, Jan 25, 2020 at 05:52:51PM +0530, Amit Singh Tomar wrote: > > Right now, Clock bindings for ethernet uses different names(even in Linux) > > CLK_ETH_MAC for S900 and CLK_ETHERNET for S700, It causes compilation problem > > when using them for common clock driver. > > > > Let's use same name CLK_ETHERNET for both S700 and S900. So are you changing the include file that you just imported from Linux? I don't think that's a good idea, as you start to divert from the kernel in a subtle way. And especially the header files should stay unchanged. So either you send this patch to the kernel first, or, probably better, you drop this change here, and unify the name at the point where it's used (#ifndef CLK_ETHERNET ....) Cheers, Andre. > > > > Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com> > > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > Thanks, > Mani > > > --- > > Changes since v2: > > * Newly added patch, not there in v2/v1. > > --- > > include/dt-bindings/clock/actions,s900-cmu.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > 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 > >
On Mon, Feb 24, 2020 at 02:37:22PM +0000, Andre Przywara wrote: > On Sun, 23 Feb 2020 23:08:25 +0530 > Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> wrote: > > Hi Amit, > > > On Sat, Jan 25, 2020 at 05:52:51PM +0530, Amit Singh Tomar wrote: > > > Right now, Clock bindings for ethernet uses different names(even in Linux) > > > CLK_ETH_MAC for S900 and CLK_ETHERNET for S700, It causes compilation problem > > > when using them for common clock driver. > > > > > > Let's use same name CLK_ETHERNET for both S700 and S900. > > So are you changing the include file that you just imported from Linux? I don't think that's a good idea, as you start to divert from the kernel in a subtle way. And especially the header files should stay unchanged. > So either you send this patch to the kernel first, or, probably better, you drop this change here, and unify the name at the point where it's used (#ifndef CLK_ETHERNET ....) > Good point. I'm happy to accept this change in kernel but not sure what Andreas will say. Thanks, Mani > Cheers, > Andre. > > > > > > > Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com> > > > > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > > > Thanks, > > Mani > > > > > --- > > > Changes since v2: > > > * Newly added patch, not there in v2/v1. > > > --- > > > include/dt-bindings/clock/actions,s900-cmu.h | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > 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 > > > >
Hi,
> So either you send this patch to the kernel first, or, probably better, you drop this change here, and unify the name at the point where it's used (#ifndef CLK_ETHERNET ....)
But this is something mentioned in cover letter:
"Patch(9/21) uses same name for ethernet clock binding and if it's ok,
would like to send it to LKML
as well."
anyways as suggested by Mani to drop Ethernet related patches for now,
this patch is not necessary in that case.
Meanwhile, would send it to LKML first for a review.
Thanks
-Amit
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)
Right now, Clock bindings for ethernet uses different names(even in Linux) CLK_ETH_MAC for S900 and CLK_ETHERNET for S700, It causes compilation problem when using them for common clock driver. Let's use same name CLK_ETHERNET for both S700 and S900. Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com> --- Changes since v2: * Newly added patch, not there in v2/v1. --- include/dt-bindings/clock/actions,s900-cmu.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)