Re: [PATCH v2 02/17] ARM: at91: pm: move SAM9X60's PM under its own SoC config flag

[Alexandre Belloni]

On 27/11/2019 08:06:47+0000, Claudiu.Beznea@xxxxxxxxxxxxx wrote:
> 
> 
> On 26.11.2019 23:28, Alexandre Belloni wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > 
> > On 26/11/2019 15:12:06+0200, Claudiu Beznea wrote:
> >> Move SAM9X60's PM part under SoC config flag. This allows the building
> >> of SAM9X60 platform withouth depending on CONFIG_SOC_AT91SAM9 flag,
> >> allowing us to select only necessary config flags for SAM9X60.
> >>
> > 
> > I'm really wondering, how much space does that really save?
> > 
> > The net benefit seems to be very small...
> 
> Not that much, indeed. We want to be independent of SOC_AT91SAM9.
> 

The question is why? I don't see the technical benefit but I
definitively see the maintenance burden of having two separate configs
doing almost the same thing.

> > 
> >> Signed-off-by: Claudiu Beznea <claudiu.beznea@xxxxxxxxxxxxx>
> >> ---
> >>  arch/arm/mach-at91/Makefile   |  1 +
> >>  arch/arm/mach-at91/at91sam9.c | 18 ------------------
> >>  arch/arm/mach-at91/pm.c       |  2 +-
> >>  arch/arm/mach-at91/sam9x60.c  | 34 ++++++++++++++++++++++++++++++++++
> >>  4 files changed, 36 insertions(+), 19 deletions(-)
> >>  create mode 100644 arch/arm/mach-at91/sam9x60.c
> >>
> >> diff --git a/arch/arm/mach-at91/Makefile b/arch/arm/mach-at91/Makefile
> >> index de64301dcff2..f565490f1b70 100644
> >> --- a/arch/arm/mach-at91/Makefile
> >> +++ b/arch/arm/mach-at91/Makefile
> >> @@ -6,6 +6,7 @@
> >>  # CPU-specific support
> >>  obj-$(CONFIG_SOC_AT91RM9200) += at91rm9200.o
> >>  obj-$(CONFIG_SOC_AT91SAM9)   += at91sam9.o
> >> +obj-$(CONFIG_SOC_SAM9X60)    += sam9x60.o
> >>  obj-$(CONFIG_SOC_SAMA5)              += sama5.o
> >>  obj-$(CONFIG_SOC_SAMV7)              += samv7.o
> >>
> >> diff --git a/arch/arm/mach-at91/at91sam9.c b/arch/arm/mach-at91/at91sam9.c
> >> index bf629c90c758..7e572189a5eb 100644
> >> --- a/arch/arm/mach-at91/at91sam9.c
> >> +++ b/arch/arm/mach-at91/at91sam9.c
> >> @@ -31,21 +31,3 @@ DT_MACHINE_START(at91sam_dt, "Atmel AT91SAM9")
> >>       .init_machine   = at91sam9_init,
> >>       .dt_compat      = at91_dt_board_compat,
> >>  MACHINE_END
> >> -
> >> -static void __init sam9x60_init(void)
> >> -{
> >> -     of_platform_default_populate(NULL, NULL, NULL);
> >> -
> >> -     sam9x60_pm_init();
> >> -}
> >> -
> >> -static const char *const sam9x60_dt_board_compat[] __initconst = {
> >> -     "microchip,sam9x60",
> >> -     NULL
> >> -};
> >> -
> >> -DT_MACHINE_START(sam9x60_dt, "Microchip SAM9X60")
> >> -     /* Maintainer: Microchip */
> >> -     .init_machine   = sam9x60_init,
> >> -     .dt_compat      = sam9x60_dt_board_compat,
> >> -MACHINE_END
> >> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
> >> index d5af6aedc02c..56a6a49b19e2 100644
> >> --- a/arch/arm/mach-at91/pm.c
> >> +++ b/arch/arm/mach-at91/pm.c
> >> @@ -805,7 +805,7 @@ void __init at91rm9200_pm_init(void)
> >>
> >>  void __init sam9x60_pm_init(void)
> >>  {
> >> -     if (!IS_ENABLED(CONFIG_SOC_AT91SAM9))
> >> +     if (!IS_ENABLED(CONFIG_SOC_SAM9X60))
> >>               return;
> >>
> >>       at91_pm_modes_init();
> >> diff --git a/arch/arm/mach-at91/sam9x60.c b/arch/arm/mach-at91/sam9x60.c
> >> new file mode 100644
> >> index 000000000000..d8c739d25458
> >> --- /dev/null
> >> +++ b/arch/arm/mach-at91/sam9x60.c
> >> @@ -0,0 +1,34 @@
> >> +// SPDX-License-Identifier: GPL-2.0+
> >> +/*
> >> + * Setup code for SAM9X60.
> >> + *
> >> + * Copyright (C) 2019 Microchip Technology Inc. and its subsidiaries
> >> + *
> >> + * Author: Claudiu Beznea <claudiu.beznea@xxxxxxxxxxxxx>
> >> + */
> >> +
> >> +#include <linux/of.h>
> >> +#include <linux/of_platform.h>
> >> +
> >> +#include <asm/mach/arch.h>
> >> +#include <asm/system_misc.h>
> >> +
> >> +#include "generic.h"
> >> +
> >> +static void __init sam9x60_init(void)
> >> +{
> >> +     of_platform_default_populate(NULL, NULL, NULL);
> >> +
> >> +     sam9x60_pm_init();
> >> +}
> >> +
> >> +static const char *const sam9x60_dt_board_compat[] __initconst = {
> >> +     "microchip,sam9x60",
> >> +     NULL
> >> +};
> >> +
> >> +DT_MACHINE_START(sam9x60_dt, "Microchip SAM9X60")
> >> +     /* Maintainer: Microchip */
> >> +     .init_machine   = sam9x60_init,
> >> +     .dt_compat      = sam9x60_dt_board_compat,
> >> +MACHINE_END
> >> --
> >> 2.7.4
> >>
> > 
> > --
> > Alexandre Belloni, Bootlin
> > Embedded Linux and Kernel engineering
> > https://bootlin.com
> > 

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com