Re: [PATCH v2 1/8] ARM: at91: pm: rework cpu detection
Hi, On 16/01/2015 at 02:44:39 +, Yang, Wenyou wrote : > > - /* AT91RM9200 SDRAM low-power mode cannot be used with self-refresh. > > */ > > - if (cpu_is_at91rm9200()) > > + at91_pm_data.memctrl = AT91_MEMCTRL_SDRAMC; > > + > > + if (of_machine_is_compatible("atmel,at91rm9200")) { > > + /* > > +* AT91RM9200 SDRAM low-power mode cannot be used with > > +* self-refresh. > > +*/ > > at91_ramc_write(0, AT91RM9200_SDRAMC_LPR, 0); > > - > > + > > + at91_pm_data.uhp_udp_mask = AT91RM9200_PMC_UHP | > > + AT91RM9200_PMC_UDP; > > + at91_pm_data.memctrl = AT91_MEMCTRL_MC; > > + } else if (of_machine_is_compatible("atmel,at91sam9260") || > > + of_machine_is_compatible("atmel,at91sam9g20") || > > + of_machine_is_compatible("atmel,at91sam9261") || > > + of_machine_is_compatible("atmel,at91sam9g10") || > > + of_machine_is_compatible("atmel,at91sam9263")) { > > + at91_pm_data.uhp_udp_mask = AT91SAM926x_PMC_UHP | > > + AT91SAM926x_PMC_UDP; > > + } else if (of_machine_is_compatible("atmel,at91sam9g45")) { > > + at91_pm_data.memctrl = AT91_MEMCTRL_DDRSDR; > > + } > > + > About the memory controller type, I prefer to get it during the memory > controller initialization, from the sram_ids[].data which defined in the > setup.c, > As this, > static const struct at91_ramc_of_data at91rm9200_ramc_of_data = { > .ramc_type = AT91_MEMCTRL_MC, > }; > ... > static struct of_device_id ramc_ids[] = { > { .compatible = "atmel,at91rm9200-sdramc", .data = > _ramc_of_data}, > ... ... > { /*sentinel*/ } > }; > > What about you? Yes, we agreed that using of_machine_is_compatible is not nice and that is why I remove that usage in patch 4. We still have to fill the uhp_udp_mask and that would mean adding a match on the pmc compatible string. I would prefer not doing that. Or maybe we can just remove the check, I don't think it it necessary anymore. At some point in time, I would like to be able to get rid of the ramc_ids in mach-at91 but I'm not sure how yet. Maybe we can do what you suggest after http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/316771.html Because then, the ram detection is local to pm.c -- Alexandre Belloni, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/8] ARM: at91: pm: rework cpu detection
Hi, On 16/01/2015 at 02:44:39 +, Yang, Wenyou wrote : - /* AT91RM9200 SDRAM low-power mode cannot be used with self-refresh. */ - if (cpu_is_at91rm9200()) + at91_pm_data.memctrl = AT91_MEMCTRL_SDRAMC; + + if (of_machine_is_compatible(atmel,at91rm9200)) { + /* +* AT91RM9200 SDRAM low-power mode cannot be used with +* self-refresh. +*/ at91_ramc_write(0, AT91RM9200_SDRAMC_LPR, 0); - + + at91_pm_data.uhp_udp_mask = AT91RM9200_PMC_UHP | + AT91RM9200_PMC_UDP; + at91_pm_data.memctrl = AT91_MEMCTRL_MC; + } else if (of_machine_is_compatible(atmel,at91sam9260) || + of_machine_is_compatible(atmel,at91sam9g20) || + of_machine_is_compatible(atmel,at91sam9261) || + of_machine_is_compatible(atmel,at91sam9g10) || + of_machine_is_compatible(atmel,at91sam9263)) { + at91_pm_data.uhp_udp_mask = AT91SAM926x_PMC_UHP | + AT91SAM926x_PMC_UDP; + } else if (of_machine_is_compatible(atmel,at91sam9g45)) { + at91_pm_data.memctrl = AT91_MEMCTRL_DDRSDR; + } + About the memory controller type, I prefer to get it during the memory controller initialization, from the sram_ids[].data which defined in the setup.c, As this, static const struct at91_ramc_of_data at91rm9200_ramc_of_data = { .ramc_type = AT91_MEMCTRL_MC, }; ... static struct of_device_id ramc_ids[] = { { .compatible = atmel,at91rm9200-sdramc, .data = at91rm9200_ramc_of_data}, ... ... { /*sentinel*/ } }; What about you? Yes, we agreed that using of_machine_is_compatible is not nice and that is why I remove that usage in patch 4. We still have to fill the uhp_udp_mask and that would mean adding a match on the pmc compatible string. I would prefer not doing that. Or maybe we can just remove the check, I don't think it it necessary anymore. At some point in time, I would like to be able to get rid of the ramc_ids in mach-at91 but I'm not sure how yet. Maybe we can do what you suggest after http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/316771.html Because then, the ram detection is local to pm.c -- Alexandre Belloni, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v2 1/8] ARM: at91: pm: rework cpu detection
Hi Alexandre, > -Original Message- > From: linux-arm-kernel [mailto:linux-arm-kernel-boun...@lists.infradead.org] > On > Behalf Of Alexandre Belloni > Sent: Thursday, January 15, 2015 10:59 PM > To: Ferre, Nicolas > Cc: Boris Brezillon; Arnd Bergmann; linux-kernel@vger.kernel.org; Alexandre > Belloni; Jean-Christophe Plagniol-Villard; > linux-arm-ker...@lists.infradead.org > Subject: [PATCH v2 1/8] ARM: at91: pm: rework cpu detection > > Store SoC differences in a struct to remove cpu_is_* usage. > > Signed-off-by: Alexandre Belloni > --- > arch/arm/mach-at91/pm.c | 54 ++- > -- > 1 file changed, 33 insertions(+), 21 deletions(-) > > diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c index > 9b15169a1c62..79aa793d1f00 100644 > --- a/arch/arm/mach-at91/pm.c > +++ b/arch/arm/mach-at91/pm.c > @@ -17,6 +17,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -32,6 +33,11 @@ > #include "generic.h" > #include "pm.h" > > +static struct { > + unsigned long uhp_udp_mask; > + int memctrl; > +} at91_pm_data; > + > static void (*at91_pm_standby)(void); > > static int at91_pm_valid_state(suspend_state_t state) @@ -71,17 +77,9 @@ > static > int at91_pm_verify_clocks(void) > scsr = at91_pmc_read(AT91_PMC_SCSR); > > /* USB must not be using PLLB */ > - if (cpu_is_at91rm9200()) { > - if ((scsr & (AT91RM9200_PMC_UHP | > AT91RM9200_PMC_UDP)) != 0) { > - pr_err("AT91: PM - Suspend-to-RAM with USB still > active\n"); > - return 0; > - } > - } else if (cpu_is_at91sam9260() || cpu_is_at91sam9261() || > cpu_is_at91sam9263() > - || cpu_is_at91sam9g20() || cpu_is_at91sam9g10()) { > - if ((scsr & (AT91SAM926x_PMC_UHP | > AT91SAM926x_PMC_UDP)) != 0) { > - pr_err("AT91: PM - Suspend-to-RAM with USB still > active\n"); > - return 0; > - } > + if ((scsr & at91_pm_data.uhp_udp_mask) != 0) { > + pr_err("AT91: PM - Suspend-to-RAM with USB still active\n"); > + return 0; > } > > /* PCK0..PCK3 must be disabled, or configured to use clk32k */ @@ - > 149,18 +147,13 @@ static int at91_pm_enter(suspend_state_t state) >* turning off the main oscillator; reverse on wakeup. >*/ > if (slow_clock) { > - int memctrl = AT91_MEMCTRL_SDRAMC; > - > - if (cpu_is_at91rm9200()) > - memctrl = AT91_MEMCTRL_MC; > - else if (cpu_is_at91sam9g45()) > - memctrl = AT91_MEMCTRL_DDRSDR; > #ifdef CONFIG_AT91_SLOW_CLOCK > /* copy slow_clock handler to SRAM, and call it > */ > memcpy(slow_clock, at91_slow_clock, > at91_slow_clock_sz); #endif > slow_clock(at91_pmc_base, at91_ramc_base[0], > -at91_ramc_base[1], memctrl); > +at91_ramc_base[1], > +at91_pm_data.memctrl); > break; > } else { > pr_info("AT91: PM - no slow clock mode > enabled ...\n"); @@ -237,10 +230,29 @@ static int __init at91_pm_init(void) > > pr_info("AT91: Power Management%s\n", (slow_clock ? " (with slow clock > mode)" : "")); > > - /* AT91RM9200 SDRAM low-power mode cannot be used with self-refresh. > */ > - if (cpu_is_at91rm9200()) > + at91_pm_data.memctrl = AT91_MEMCTRL_SDRAMC; > + > + if (of_machine_is_compatible("atmel,at91rm9200")) { > + /* > + * AT91RM9200 SDRAM low-power mode cannot be used with > + * self-refresh. > + */ > at91_ramc_write(0, AT91RM9200_SDRAMC_LPR, 0); > - > + > + at91_pm_data.uhp_udp_mask = AT91RM9200_PMC_UHP | > + AT91RM9200_PMC_UDP; > + at91_pm_data.memctrl = AT91_MEMCTRL_MC; > + } else if (of_machine_is_compatible("atmel,at91sam9260") || > +of_machine_is_compatible("atmel,at91sam9g20") || > +of_machine_is_compatible("atmel,at91sam9261") || > +of_machine_is_compatible("atmel,at91sam9g10") || > +of_machine_is_compatible("atmel,at91sam9263")) { > + at91_pm_data.uhp_udp_mask = AT91SAM926x_PMC_UHP | > + AT91SAM926x_PMC_UDP; > + } else if (of_machine_is_compatible("atmel,at91sam9g45")) { > + at91_pm_data.memctrl = AT91_MEMCTRL_DDRSDR; > + } > + About the memory controller type, I prefer to get it during the memory controller initialization,
RE: [PATCH v2 1/8] ARM: at91: pm: rework cpu detection
Hi Alexandre, -Original Message- From: linux-arm-kernel [mailto:linux-arm-kernel-boun...@lists.infradead.org] On Behalf Of Alexandre Belloni Sent: Thursday, January 15, 2015 10:59 PM To: Ferre, Nicolas Cc: Boris Brezillon; Arnd Bergmann; linux-kernel@vger.kernel.org; Alexandre Belloni; Jean-Christophe Plagniol-Villard; linux-arm-ker...@lists.infradead.org Subject: [PATCH v2 1/8] ARM: at91: pm: rework cpu detection Store SoC differences in a struct to remove cpu_is_* usage. Signed-off-by: Alexandre Belloni alexandre.bell...@free-electrons.com --- arch/arm/mach-at91/pm.c | 54 ++- -- 1 file changed, 33 insertions(+), 21 deletions(-) diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c index 9b15169a1c62..79aa793d1f00 100644 --- a/arch/arm/mach-at91/pm.c +++ b/arch/arm/mach-at91/pm.c @@ -17,6 +17,7 @@ #include linux/interrupt.h #include linux/sysfs.h #include linux/module.h +#include linux/of.h #include linux/platform_device.h #include linux/io.h #include linux/clk/at91_pmc.h @@ -32,6 +33,11 @@ #include generic.h #include pm.h +static struct { + unsigned long uhp_udp_mask; + int memctrl; +} at91_pm_data; + static void (*at91_pm_standby)(void); static int at91_pm_valid_state(suspend_state_t state) @@ -71,17 +77,9 @@ static int at91_pm_verify_clocks(void) scsr = at91_pmc_read(AT91_PMC_SCSR); /* USB must not be using PLLB */ - if (cpu_is_at91rm9200()) { - if ((scsr (AT91RM9200_PMC_UHP | AT91RM9200_PMC_UDP)) != 0) { - pr_err(AT91: PM - Suspend-to-RAM with USB still active\n); - return 0; - } - } else if (cpu_is_at91sam9260() || cpu_is_at91sam9261() || cpu_is_at91sam9263() - || cpu_is_at91sam9g20() || cpu_is_at91sam9g10()) { - if ((scsr (AT91SAM926x_PMC_UHP | AT91SAM926x_PMC_UDP)) != 0) { - pr_err(AT91: PM - Suspend-to-RAM with USB still active\n); - return 0; - } + if ((scsr at91_pm_data.uhp_udp_mask) != 0) { + pr_err(AT91: PM - Suspend-to-RAM with USB still active\n); + return 0; } /* PCK0..PCK3 must be disabled, or configured to use clk32k */ @@ - 149,18 +147,13 @@ static int at91_pm_enter(suspend_state_t state) * turning off the main oscillator; reverse on wakeup. */ if (slow_clock) { - int memctrl = AT91_MEMCTRL_SDRAMC; - - if (cpu_is_at91rm9200()) - memctrl = AT91_MEMCTRL_MC; - else if (cpu_is_at91sam9g45()) - memctrl = AT91_MEMCTRL_DDRSDR; #ifdef CONFIG_AT91_SLOW_CLOCK /* copy slow_clock handler to SRAM, and call it */ memcpy(slow_clock, at91_slow_clock, at91_slow_clock_sz); #endif slow_clock(at91_pmc_base, at91_ramc_base[0], -at91_ramc_base[1], memctrl); +at91_ramc_base[1], +at91_pm_data.memctrl); break; } else { pr_info(AT91: PM - no slow clock mode enabled ...\n); @@ -237,10 +230,29 @@ static int __init at91_pm_init(void) pr_info(AT91: Power Management%s\n, (slow_clock ? (with slow clock mode) : )); - /* AT91RM9200 SDRAM low-power mode cannot be used with self-refresh. */ - if (cpu_is_at91rm9200()) + at91_pm_data.memctrl = AT91_MEMCTRL_SDRAMC; + + if (of_machine_is_compatible(atmel,at91rm9200)) { + /* + * AT91RM9200 SDRAM low-power mode cannot be used with + * self-refresh. + */ at91_ramc_write(0, AT91RM9200_SDRAMC_LPR, 0); - + + at91_pm_data.uhp_udp_mask = AT91RM9200_PMC_UHP | + AT91RM9200_PMC_UDP; + at91_pm_data.memctrl = AT91_MEMCTRL_MC; + } else if (of_machine_is_compatible(atmel,at91sam9260) || +of_machine_is_compatible(atmel,at91sam9g20) || +of_machine_is_compatible(atmel,at91sam9261) || +of_machine_is_compatible(atmel,at91sam9g10) || +of_machine_is_compatible(atmel,at91sam9263)) { + at91_pm_data.uhp_udp_mask = AT91SAM926x_PMC_UHP | + AT91SAM926x_PMC_UDP; + } else if (of_machine_is_compatible(atmel,at91sam9g45)) { + at91_pm_data.memctrl = AT91_MEMCTRL_DDRSDR; + } + About the memory controller type, I prefer to get it during the memory controller initialization, from the