From mboxrd@z Thu Jan 1 00:00:00 1970 From: Geert Uytterhoeven Subject: Re: [PATCH/RFC v3 20/22] clk: renesas: r8a7778: Remove obsolete r8a7778_clocks_init() Date: Thu, 2 Jun 2016 09:13:36 +0200 Message-ID: References: <1464808880-343-1-git-send-email-geert+renesas@glider.be> <1464808880-343-21-git-send-email-geert+renesas@glider.be> <9f19c6e5-54d1-abd8-b666-bdf1a502d9c4@de.bosch.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Return-path: In-Reply-To: <9f19c6e5-54d1-abd8-b666-bdf1a502d9c4@de.bosch.com> Sender: linux-renesas-soc-owner@vger.kernel.org To: Dirk Behme Cc: Geert Uytterhoeven , Simon Horman , Magnus Damm , Laurent Pinchart , Philipp Zabel , Michael Turquette , Stephen Boyd , linux-renesas-soc@vger.kernel.org, "linux-arm-kernel@lists.infradead.org" , "devicetree@vger.kernel.org" List-Id: devicetree@vger.kernel.org Hi Dirk, On Thu, Jun 2, 2016 at 7:54 AM, Dirk Behme wrote: > On 01.06.2016 21:21, Geert Uytterhoeven wrote: >> The R-Car M1A board code no longer calls r8a7778_clocks_init(). >> >> Signed-off-by: Geert Uytterhoeven >> --- >> v3: >> - New. >> --- >> drivers/clk/renesas/clk-r8a7778.c | 13 ------------- >> include/linux/clk/renesas.h | 1 - >> 2 files changed, 14 deletions(-) >> >> diff --git a/drivers/clk/renesas/clk-r8a7778.c >> b/drivers/clk/renesas/clk-r8a7778.c >> index 07ea411098a75ad1..886a8380e91247a1 100644 >> --- a/drivers/clk/renesas/clk-r8a7778.c >> +++ b/drivers/clk/renesas/clk-r8a7778.c >> @@ -143,16 +143,3 @@ static void __init r8a7778_cpg_clocks_init(struct >> device_node *np) >> >> CLK_OF_DECLARE(r8a7778_cpg_clks, "renesas,r8a7778-cpg-clocks", >> r8a7778_cpg_clocks_init); >> - >> -void __init r8a7778_clocks_init(u32 mode) >> -{ >> - BUG_ON(!(mode & BIT(19))); >> - >> - cpg_mode_rates = (!!(mode & BIT(18)) << 2) | >> - (!!(mode & BIT(12)) << 1) | >> - (!!(mode & BIT(11))); >> - cpg_mode_divs = (!!(mode & BIT(2)) << 1) | >> - (!!(mode & BIT(1))); >> - >> - of_clk_init(NULL); >> -} > > Just a question on how you structured the patches: Is there a special reason > why you do the adding of new code and the removal of dead code in two > patches? > > Having both in one patch normally makes it more obvious which old code is > replaced by which new code. > > An example: In patch > > [PATCH/RFC v3 11/22] clk: renesas: r8a7778: Obtain mode pin values using > R-Car RST driver > > I wondered where the section > > + BUG_ON(!(mode & BIT(19))); > + > + cpg_mode_rates = (!!(mode & BIT(18)) << 2) | > + (!!(mode & BIT(12)) << 1) | > + (!!(mode & BIT(11))); > + cpg_mode_divs = (!!(mode & BIT(2)) << 1) | > + (!!(mode & BIT(1))); > > comes from. This becomes obvious with this patch 20/22. But it's 9 patches > later ;) So why don't make the whole replacement visible in one patch? Because that would break bisection. Path 20 depends on patch 17, which depends on patch 11. These three can't be a single patch because they touch multiple subsystems. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds From mboxrd@z Thu Jan 1 00:00:00 1970 From: geert@linux-m68k.org (Geert Uytterhoeven) Date: Thu, 2 Jun 2016 09:13:36 +0200 Subject: [PATCH/RFC v3 20/22] clk: renesas: r8a7778: Remove obsolete r8a7778_clocks_init() In-Reply-To: <9f19c6e5-54d1-abd8-b666-bdf1a502d9c4@de.bosch.com> References: <1464808880-343-1-git-send-email-geert+renesas@glider.be> <1464808880-343-21-git-send-email-geert+renesas@glider.be> <9f19c6e5-54d1-abd8-b666-bdf1a502d9c4@de.bosch.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Dirk, On Thu, Jun 2, 2016 at 7:54 AM, Dirk Behme wrote: > On 01.06.2016 21:21, Geert Uytterhoeven wrote: >> The R-Car M1A board code no longer calls r8a7778_clocks_init(). >> >> Signed-off-by: Geert Uytterhoeven >> --- >> v3: >> - New. >> --- >> drivers/clk/renesas/clk-r8a7778.c | 13 ------------- >> include/linux/clk/renesas.h | 1 - >> 2 files changed, 14 deletions(-) >> >> diff --git a/drivers/clk/renesas/clk-r8a7778.c >> b/drivers/clk/renesas/clk-r8a7778.c >> index 07ea411098a75ad1..886a8380e91247a1 100644 >> --- a/drivers/clk/renesas/clk-r8a7778.c >> +++ b/drivers/clk/renesas/clk-r8a7778.c >> @@ -143,16 +143,3 @@ static void __init r8a7778_cpg_clocks_init(struct >> device_node *np) >> >> CLK_OF_DECLARE(r8a7778_cpg_clks, "renesas,r8a7778-cpg-clocks", >> r8a7778_cpg_clocks_init); >> - >> -void __init r8a7778_clocks_init(u32 mode) >> -{ >> - BUG_ON(!(mode & BIT(19))); >> - >> - cpg_mode_rates = (!!(mode & BIT(18)) << 2) | >> - (!!(mode & BIT(12)) << 1) | >> - (!!(mode & BIT(11))); >> - cpg_mode_divs = (!!(mode & BIT(2)) << 1) | >> - (!!(mode & BIT(1))); >> - >> - of_clk_init(NULL); >> -} > > Just a question on how you structured the patches: Is there a special reason > why you do the adding of new code and the removal of dead code in two > patches? > > Having both in one patch normally makes it more obvious which old code is > replaced by which new code. > > An example: In patch > > [PATCH/RFC v3 11/22] clk: renesas: r8a7778: Obtain mode pin values using > R-Car RST driver > > I wondered where the section > > + BUG_ON(!(mode & BIT(19))); > + > + cpg_mode_rates = (!!(mode & BIT(18)) << 2) | > + (!!(mode & BIT(12)) << 1) | > + (!!(mode & BIT(11))); > + cpg_mode_divs = (!!(mode & BIT(2)) << 1) | > + (!!(mode & BIT(1))); > > comes from. This becomes obvious with this patch 20/22. But it's 9 patches > later ;) So why don't make the whole replacement visible in one patch? Because that would break bisection. Path 20 depends on patch 17, which depends on patch 11. These three can't be a single patch because they touch multiple subsystems. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds