| Message ID | 4F589474.8050609@samsung.com |
|---|---|
| State | New |
| Headers | show |
Hi, On Thu, Mar 8, 2012 at 3:13 AM, Kukjin Kim <kgene.kim@samsung.com> wrote: > Kukjin Kim (2): > ARM: EXYNOS: cleanup clock part for new EXYNOS SoCs This is one large commit that does many things in one change. As the patch message says: ARM: EXYNOS: cleanup clock part for new EXYNOS SoCs - the prefix of clk register replace S5P_ with EXYNOS4_ - move mach-exynos/clock.c to mach-exynos/clock-exynos4.c - according to moving clock-exynos4.c, move <mach/exynos4-clock.h> to "clock-exynos4.h" - add prefix exynos4_ on clk declaration It makes it hard to review, especially when you combine a move with other changes. It's better to do the move in a separate commit that doesn't change code (or only changes very very little) and then do the other changes in separate commits later. -Olof
On 03/08/12 07:16, Olof Johansson wrote: > Hi, > > On Thu, Mar 8, 2012 at 3:13 AM, Kukjin Kim<kgene.kim@samsung.com> wrote: > >> Kukjin Kim (2): >> ARM: EXYNOS: cleanup clock part for new EXYNOS SoCs > > This is one large commit that does many things in one change. As the > patch message says: > > ARM: EXYNOS: cleanup clock part for new EXYNOS SoCs > > - the prefix of clk register replace S5P_ with EXYNOS4_ > - move mach-exynos/clock.c to mach-exynos/clock-exynos4.c > - according to moving clock-exynos4.c, > move<mach/exynos4-clock.h> to "clock-exynos4.h" > - add prefix exynos4_ on clk declaration > > It makes it hard to review, especially when you combine a move with > other changes. It's better to do the move in a separate commit that > doesn't change code (or only changes very very little) and then do the > other changes in separate commits later. > Hmm...OK, let me do as per your suggestion and will re-send this soon. Thanks. Best regards, Kgene. -- Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.
On Sat, Mar 10, 2012 at 07:35:29AM -0800, Kukjin Kim wrote: > On 03/09/12 07:33, Kukjin Kim wrote: > >On 03/08/12 07:16, Olof Johansson wrote: > >>Hi, > >> > >>On Thu, Mar 8, 2012 at 3:13 AM, Kukjin Kim<kgene.kim@samsung.com> wrote: > >> > >>>Kukjin Kim (2): > >>>ARM: EXYNOS: cleanup clock part for new EXYNOS SoCs > >> > >>This is one large commit that does many things in one change. As the > >>patch message says: > >> > >>ARM: EXYNOS: cleanup clock part for new EXYNOS SoCs > >> > >>- the prefix of clk register replace S5P_ with EXYNOS4_ > >>- move mach-exynos/clock.c to mach-exynos/clock-exynos4.c > >>- according to moving clock-exynos4.c, > >>move<mach/exynos4-clock.h> to "clock-exynos4.h" > >>- add prefix exynos4_ on clk declaration > >> > >>It makes it hard to review, especially when you combine a move with > >>other changes. It's better to do the move in a separate commit that > >>doesn't change code (or only changes very very little) and then do the > >>other changes in separate commits later. > >> > >Hmm...OK, let me do as per your suggestion and will re-send this soon. > > > Hi Olof, > > I addressed comments from you and I think it should be ok to you. Yes, much easier to review. > Please pull from: > git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git > next/cleanup-exynos-clock > > If any problems, please kindly let me know. > > Thanks. > > Best regards, > Kgene. > -- > Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer, > SW Solution Development Team, Samsung Electronics Co., Ltd. > > > The following changes since commit 192cfd58774b4d17b2fe8bdc77d89c2ef4e0591d: > > Linux 3.3-rc6 (2012-03-03 17:08:09 -0800) > > are available in the git repository at: > git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git > next/cleanup-exynos-clock The contents of the branch itself looks good, but now you have introduced a modified/delete conflict with yourself. You modify arch/arm/mach-exynos/clock.c in your cleanup-use-static branch, and then you move it here. That means that when I merge in this branch, git throws a conflict and I have to do manual edits to make the contents match. It's better if you base this branch on your cleanup-use-static branch, so that the move includes those edits. That way there is no conflict resolution to do at our end either. Can you please do that rebase and send a fresh pull request? I'll continue looking at your other requests meanwhile. Thanks! -Olof
On 03/10/12 09:35, Olof Johansson wrote: [...] > > The contents of the branch itself looks good, but now you have introduced > a modified/delete conflict with yourself. > > You modify arch/arm/mach-exynos/clock.c in your cleanup-use-static branch, and > then you move it here. That means that when I merge in this branch, git throws > a conflict and I have to do manual edits to make the contents match. > > It's better if you base this branch on your cleanup-use-static branch, so that > the move includes those edits. That way there is no conflict resolution to do > at our end either. > > Can you please do that rebase and send a fresh pull request? I'll continue > looking at your other requests meanwhile. > Hi Olof, Sorry for late response. OK I see and your suggestion sounds good to me. Let me send a new pull request after that. Thanks. Best regards, Kgene. -- Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.
Hi Arnd, Olof, Here is cleanup clock part for EXYNOS SoCs from: git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git next/cleanup-exynos-clock Please pull and if any problems, please kindly let me know. Thanks. Best regards, Kgene. -- Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd. The following changes since commit b01543dfe67bb1d191998e90d20534dc354de059: Linux 3.3-rc4 (2012-02-18 15:53:33 -0800) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git next/cleanup-exynos-clock Kukjin Kim (2): ARM: EXYNOS: cleanup clock part for new EXYNOS SoCs PM / devfreq: update the name of EXYNOS clock register arch/arm/mach-exynos/Makefile | 3 +- arch/arm/mach-exynos/clock-exynos4.c | 1564 +++++++++++++++++++++ arch/arm/mach-exynos/clock-exynos4.h | 32 + arch/arm/mach-exynos/clock-exynos4210.c | 46 +- arch/arm/mach-exynos/clock-exynos4212.c | 30 +- arch/arm/mach-exynos/clock.c | 1564 --------------------- arch/arm/mach-exynos/common.h | 9 + arch/arm/mach-exynos/include/mach/exynos4-clock.h | 43 - arch/arm/mach-exynos/include/mach/regs-clock.h | 364 +++--- arch/arm/mach-exynos/pm.c | 40 +- drivers/devfreq/exynos4_bus.c | 224 ++-- 11 files changed, 1959 insertions(+), 1960 deletions(-) create mode 100644 arch/arm/mach-exynos/clock-exynos4.c create mode 100644 arch/arm/mach-exynos/clock-exynos4.h delete mode 100644 arch/arm/mach-exynos/clock.c delete mode 100644 arch/arm/mach-exynos/include/mach/exynos4-clock.h