Hi Simon,
On Mon, Jun 11, 2018 at 01:38:42PM -0600, Simon Glass wrote:
Hi,
On 11 June 2018 at 09:48, Manivannan Sadhasivam manivannan.sadhasivam@linaro.org wrote:
This commit adds Actions Semi OWL family base clock and S900 SoC specific clock support. For S900 peripheral clock support, only UART clock has been added for now.
Signed-off-by: Manivannan Sadhasivam manivannan.sadhasivam@linaro.org
arch/arm/include/asm/arch-owl/clk_owl.h | 61 +++++++++++++ arch/arm/include/asm/arch-owl/regs_s900.h | 64 +++++++++++++ arch/arm/mach-owl/Kconfig | 2 +- drivers/clk/Kconfig | 1 + drivers/clk/Makefile | 1 + drivers/clk/owl/Kconfig | 12 +++ drivers/clk/owl/Makefile | 4 + drivers/clk/owl/clk_owl.c | 60 +++++++++++++ drivers/clk/owl/clk_s900.c | 104 ++++++++++++++++++++++ 9 files changed, 308 insertions(+), 1 deletion(-) create mode 100644 arch/arm/include/asm/arch-owl/clk_owl.h create mode 100644 arch/arm/include/asm/arch-owl/regs_s900.h create mode 100644 drivers/clk/owl/Kconfig create mode 100644 drivers/clk/owl/Makefile create mode 100644 drivers/clk/owl/clk_owl.c create mode 100644 drivers/clk/owl/clk_s900.c
This doesn't look quite right to me. It looks like you should put all the code in clk_s900.c and delete clk_owl.c.
The intention is to separate generic OWL family and SoC specific part. I know that there isn't much in the OWL part now but since this is just a basic clk support and I will extend this in upcoming days with further peripherals, this makes sense to me.
Also, there are plans to support other OWL family SoCs like S500 and S700. So, in those cases this partition will avoid much code duplication. Moreover, the pattern followed here matches the Linux kernel common clock framework by some means.
Thanks, Mani
Regards, Simon