From patchwork Mon Oct 14 16:30:56 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Daniel Lezcano X-Patchwork-Id: 21006 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-pa0-f69.google.com (mail-pa0-f69.google.com [209.85.220.69]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 0E6E320DA3 for ; Mon, 14 Oct 2013 16:31:01 +0000 (UTC) Received: by mail-pa0-f69.google.com with SMTP id kq14sf14297404pab.0 for ; Mon, 14 Oct 2013 09:31:01 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:message-id:date:from:user-agent :mime-version:to:cc:subject:references:in-reply-to:x-original-sender :x-original-authentication-results:precedence:mailing-list:list-id :list-post:list-help:list-archive:list-unsubscribe:content-type :content-transfer-encoding; bh=fa7L4cTkXA8kbig2GYpK97rBSACHk1mW8K0e6EINPt0=; b=aqmhfxPo+6/nbUA4QJmJbSxsBPKuZCnlZ8YHI8RvDxurVWSqTtzuK58hRUZ9MfGnlB 361g+gwIhTGp68Srk8SHnxwjMK+DlvBIis3DEurxhNZymwnUuT8FKR/7X52dmDbbqhfr vVTp9Osu7Ea2rA/plkS6UXca/MEMFEhV2+URs52xhEY0fiGPmCpUdybYrWKeFIBe84t+ fq/tpKlykNZgFhGeau+CsNjp2DEUEUKrjSg/TgSvA+EAyXMm6p0uGwY/T8zNoS1DXZwv MzLUcvUv+AWues6zAp9BKH3MSiG13rL0UlEhf/E7HIOrLfxyUFosrDAg2H1Gv12znc71 ZWxQ== X-Received: by 10.66.148.8 with SMTP id to8mr12746455pab.0.1381768260984; Mon, 14 Oct 2013 09:31:00 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.49.117.69 with SMTP id kc5ls2303657qeb.42.gmail; Mon, 14 Oct 2013 09:31:00 -0700 (PDT) X-Received: by 10.220.142.140 with SMTP id q12mr1234809vcu.21.1381768260802; Mon, 14 Oct 2013 09:31:00 -0700 (PDT) Received: from mail-vc0-f174.google.com (mail-vc0-f174.google.com [209.85.220.174]) by mx.google.com with ESMTPS id sw5si1846406veb.99.1969.12.31.16.00.00 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Mon, 14 Oct 2013 09:31:00 -0700 (PDT) Received-SPF: neutral (google.com: 209.85.220.174 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) client-ip=209.85.220.174; Received: by mail-vc0-f174.google.com with SMTP id ie18so591170vcb.19 for ; Mon, 14 Oct 2013 09:31:00 -0700 (PDT) X-Gm-Message-State: ALoCoQl1Jz4b4qcropPA8ZP09plW1BiNDcWbt3ZhSUaIt3qQBeaMQbeuhDkYm4gWFUozp6MltFRE X-Received: by 10.52.116.74 with SMTP id ju10mr2011938vdb.20.1381768260677; Mon, 14 Oct 2013 09:31:00 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patches@linaro.org Received: by 10.220.174.196 with SMTP id u4csp235974vcz; Mon, 14 Oct 2013 09:30:59 -0700 (PDT) X-Received: by 10.180.206.180 with SMTP id lp20mr15281040wic.48.1381768258970; Mon, 14 Oct 2013 09:30:58 -0700 (PDT) Received: from mail-we0-f174.google.com (mail-we0-f174.google.com [74.125.82.174]) by mx.google.com with ESMTPS id o12si5783972wiv.18.2013.10.14.09.30.58 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Mon, 14 Oct 2013 09:30:58 -0700 (PDT) Received-SPF: neutral (google.com: 74.125.82.174 is neither permitted nor denied by best guess record for domain of daniel.lezcano@linaro.org) client-ip=74.125.82.174; Received: by mail-we0-f174.google.com with SMTP id u56so7201704wes.5 for ; Mon, 14 Oct 2013 09:30:58 -0700 (PDT) X-Received: by 10.194.23.73 with SMTP id k9mr31412466wjf.24.1381768258417; Mon, 14 Oct 2013 09:30:58 -0700 (PDT) Received: from [192.168.1.150] (AToulouse-654-1-457-225.w83-205.abo.wanadoo.fr. [83.205.200.225]) by mx.google.com with ESMTPSA id dq11sm35818832wid.3.2013.10.14.09.30.56 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Mon, 14 Oct 2013 09:30:57 -0700 (PDT) Message-ID: <525C1C40.7010508@linaro.org> Date: Mon, 14 Oct 2013 18:30:56 +0200 From: Daniel Lezcano User-Agent: Mozilla/5.0 (X11; Linux i686; rv:24.0) Gecko/20100101 Thunderbird/24.0 MIME-Version: 1.0 To: Jean-Christophe PLAGNIOL-VILLARD CC: linux@maxim.org.za, nicolas.ferre@atmel.com, linux-arm-kernel@lists.infradead.org, patches@linaro.org, linaro-kernel@lists.linaro.org, linux-pm@vger.kernel.org Subject: Re: [PATCH 1/2] ARM: at91: cpuidle: convert to platform driver References: <1381593099-1184-1-git-send-email-daniel.lezcano@linaro.org> <20131014140406.GH11420@ns203013.ovh.net> <525BFD31.3020406@linaro.org> <20131014142719.GI11420@ns203013.ovh.net> In-Reply-To: <20131014142719.GI11420@ns203013.ovh.net> X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: daniel.lezcano@linaro.org X-Original-Authentication-Results: mx.google.com; spf=neutral (google.com: 209.85.220.174 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Precedence: list Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org List-ID: X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , On 10/14/2013 04:27 PM, Jean-Christophe PLAGNIOL-VILLARD wrote: > On 16:18 Mon 14 Oct , Daniel Lezcano wrote: >> On 10/14/2013 04:04 PM, Jean-Christophe PLAGNIOL-VILLARD wrote: >>> On 17:51 Sat 12 Oct , Daniel Lezcano wrote: >>>> Use the platform driver model to separate the cpuidle specific code from >>>> the low level pm code. It allows to remove the dependency between >>>> these two components. >>>> >>>> Tested-on usb-a9263 (at91sam9263) >>>> >>>> Signed-off-by: Daniel Lezcano [ ... ] >>>> --- >>>> /* AT91RM9200 SDRAM low-power mode cannot be used with self-refresh. */ >>>> - if (cpu_is_at91rm9200()) >>>> + if (cpu_is_at91rm9200()) { >>>> + at91_cpuidle_device.dev.platform_data = at91rm9200_standby; >>>> at91_ramc_write(0, AT91RM9200_SDRAMC_LPR, 0); >>>> + } else if (cpu_is_at91sam9g45()) { >>>> + at91_cpuidle_device.dev.platform_data = at91sam9g45_standby; >>>> + } else if (cpu_is_at91sam9263()) { >>>> + at91_cpuidle_device.dev.platform_data = at91sam9263_standby; >>>> + } else { >>>> + at91_cpuidle_device.dev.platform_data = at91sam9_standby; >>> no this is too dangerous when adding new SoC >>> >>> you must list the supported SoC >>> >>> and I prefer to move this code to the SoC init structure >> >> Do you mean register the platform_device in eg. >> at91rm9200_initialize with the right platform data ? >> > yes as example > > Best Regards, Hi Jean-Christophe, I did the changes but there are different drawbacks with the approach. The first one is the AT91_SOC_START is too early to call the platform_device_register. The second one is we have multiple declaration of the platform_device which is not really nice. I found the following solution: The platform_device is static in pm.c, there is a at91_pm_set_standby function. All AT91_SOC_START call at91_pm_set_standby with the right standby function callback, which in turns set the platform data for the cpuidle platform device. When the init call for pm is made, it registers the platform device. That has the benefit to encapsulate the platform device and having it in a single place and also we have the information to remove the { if then else } statements we have in 'at91_pm_enter's case block for PM_SUSPEND_STANDBY. Does it sound ok for you ? Thanks -- Daniel >> Linaro.org │ Open source software for ARM SoCs >> >> Follow Linaro: Facebook | >> Twitter | >> Blog >> diff --git a/arch/arm/mach-at91/at91rm9200.c b/arch/arm/mach-at91/at91rm9200.c index 4aad93d..0d234f2 100644 --- a/arch/arm/mach-at91/at91rm9200.c +++ b/arch/arm/mach-at91/at91rm9200.c @@ -27,6 +27,7 @@ #include "generic.h" #include "clock.h" #include "sam9_smc.h" +#include "pm.h" /* -------------------------------------------------------------------- * Clocks @@ -337,6 +338,8 @@ static void __init at91rm9200_initialize(void) /* Initialize GPIO subsystem */ at91_gpio_init(at91rm9200_gpio, cpu_is_at91rm9200_bga() ? AT91RM9200_BGA : AT91RM9200_PQFP); + + at91_pm_set_standby(at91rm9200_standby); } diff --git a/arch/arm/mach-at91/at91sam9260.c b/arch/arm/mach-at91/at91sam9260.c index 5de6074..9fd7aa6 100644 --- a/arch/arm/mach-at91/at91sam9260.c +++ b/arch/arm/mach-at91/at91sam9260.c @@ -28,6 +28,7 @@ #include "generic.h" #include "clock.h" #include "sam9_smc.h" +#include "pm.h" /* -------------------------------------------------------------------- * Clocks @@ -307,7 +308,6 @@ static struct at91_gpio_bank at91sam9260_gpio[] __initdata = { /* -------------------------------------------------------------------- * AT91SAM9260 processor initialization * -------------------------------------------------------------------- */ - static void __init at91sam9xe_map_io(void) { unsigned long sram_size; @@ -351,6 +351,8 @@ static void __init at91sam9260_initialize(void) /* Register GPIO subsystem */ at91_gpio_init(at91sam9260_gpio, 3); + + at91_pm_set_standby(at91sam9_standby); } /* -------------------------------------------------------------------- diff --git a/arch/arm/mach-at91/at91sam9261.c b/arch/arm/mach-at91/at91sam9261.c index 0e07932..1edbb6f 100644 --- a/arch/arm/mach-at91/at91sam9261.c +++ b/arch/arm/mach-at91/at91sam9261.c @@ -27,6 +27,7 @@ #include "generic.h" #include "clock.h" #include "sam9_smc.h" +#include "pm.h" /* -------------------------------------------------------------------- * Clocks @@ -293,6 +294,8 @@ static void __init at91sam9261_initialize(void) /* Register GPIO subsystem */ at91_gpio_init(at91sam9261_gpio, 3); + + at91_pm_set_sandby(at91sam9_standby); } /* -------------------------------------------------------------------- diff --git a/arch/arm/mach-at91/at91sam9263.c b/arch/arm/mach-at91/at91sam9263.c index 6ce7d18..b222d59 100644 --- a/arch/arm/mach-at91/at91sam9263.c +++ b/arch/arm/mach-at91/at91sam9263.c @@ -26,6 +26,7 @@ #include "generic.h" #include "clock.h" #include "sam9_smc.h" +#include "pm.h" /* -------------------------------------------------------------------- * Clocks @@ -304,7 +305,6 @@ static struct at91_gpio_bank at91sam9263_gpio[] __initdata = { /* -------------------------------------------------------------------- * AT91SAM9263 processor initialization * -------------------------------------------------------------------- */ - static void __init at91sam9263_map_io(void) { at91_init_sram(0, AT91SAM9263_SRAM0_BASE, AT91SAM9263_SRAM0_SIZE); @@ -330,6 +330,8 @@ static void __init at91sam9263_initialize(void) /* Register GPIO subsystem */ at91_gpio_init(at91sam9263_gpio, 5); + + at91_pm_set_standby(at91sam9263_standby); } /* -------------------------------------------------------------------- diff --git a/arch/arm/mach-at91/at91sam9g45.c b/arch/arm/mach-at91/at91sam9g45.c index 474ee04..4186f37 100644 --- a/arch/arm/mach-at91/at91sam9g45.c +++ b/arch/arm/mach-at91/at91sam9g45.c @@ -26,6 +26,7 @@ #include "generic.h" #include "clock.h" #include "sam9_smc.h" +#include "pm.h" /* -------------------------------------------------------------------- * Clocks @@ -355,7 +356,6 @@ static struct at91_gpio_bank at91sam9g45_gpio[] __initdata = { /* -------------------------------------------------------------------- * AT91SAM9G45 processor initialization * -------------------------------------------------------------------- */ - static void __init at91sam9g45_map_io(void) { at91_init_sram(0, AT91SAM9G45_SRAM_BASE, AT91SAM9G45_SRAM_SIZE); @@ -379,6 +379,8 @@ static void __init at91sam9g45_initialize(void) /* Register GPIO subsystem */ at91_gpio_init(at91sam9g45_gpio, 5); + + at91_pm_set_standby(at91sam9g45_standby); } /* -------------------------------------------------------------------- diff --git a/arch/arm/mach-at91/at91sam9n12.c b/arch/arm/mach-at91/at91sam9n12.c index c7d670d..cc61e09 100644 --- a/arch/arm/mach-at91/at91sam9n12.c +++ b/arch/arm/mach-at91/at91sam9n12.c @@ -21,6 +21,7 @@ #include "generic.h" #include "clock.h" #include "sam9_smc.h" +#include "pm.h" /* -------------------------------------------------------------------- * Clocks @@ -217,13 +218,18 @@ static void __init at91sam9n12_register_clocks(void) /* -------------------------------------------------------------------- * AT91SAM9N12 processor initialization * -------------------------------------------------------------------- */ - static void __init at91sam9n12_map_io(void) { at91_init_sram(0, AT91SAM9N12_SRAM_BASE, AT91SAM9N12_SRAM_SIZE); } +static void __init at91sam9n12_initialize(void) +{ + at91_pm_set_standby(at91sam9_standby); +} + AT91_SOC_START(at91sam9n12) .map_io = at91sam9n12_map_io, .register_clocks = at91sam9n12_register_clocks, + .init = at91sam9n12_initialize, AT91_SOC_END diff --git a/arch/arm/mach-at91/at91sam9rl.c b/arch/arm/mach-at91/at91sam9rl.c index d4ec0d9..b0c8cb6 100644 --- a/arch/arm/mach-at91/at91sam9rl.c +++ b/arch/arm/mach-at91/at91sam9rl.c @@ -27,6 +27,7 @@ #include "generic.h" #include "clock.h" #include "sam9_smc.h" +#include "pm.h" /* -------------------------------------------------------------------- * Clocks @@ -261,7 +262,6 @@ static struct at91_gpio_bank at91sam9rl_gpio[] __initdata = { /* -------------------------------------------------------------------- * AT91SAM9RL processor initialization * -------------------------------------------------------------------- */ - static void __init at91sam9rl_map_io(void) { unsigned long sram_size; @@ -296,6 +296,8 @@ static void __init at91sam9rl_initialize(void) /* Register GPIO subsystem */ at91_gpio_init(at91sam9rl_gpio, 4); + + at91_pm_set_standby(at91sam9_standby); } /* -------------------------------------------------------------------- diff --git a/arch/arm/mach-at91/at91sam9x5.c b/arch/arm/mach-at91/at91sam9x5.c index 916e5a1..9ef7bc6 100644 --- a/arch/arm/mach-at91/at91sam9x5.c +++ b/arch/arm/mach-at91/at91sam9x5.c @@ -21,6 +21,7 @@ #include "generic.h" #include "clock.h" #include "sam9_smc.h" +#include "pm.h" /* -------------------------------------------------------------------- * Clocks @@ -316,12 +317,15 @@ static void __init at91sam9x5_register_clocks(void) /* -------------------------------------------------------------------- * AT91SAM9x5 processor initialization * -------------------------------------------------------------------- */ - static void __init at91sam9x5_map_io(void) { at91_init_sram(0, AT91SAM9X5_SRAM_BASE, AT91SAM9X5_SRAM_SIZE); } +static void __init at91sam9x5_initialize(void) +{ + at91_pm_set_standby(at91sam9_standby); +} /* -------------------------------------------------------------------- * Interrupt initialization * -------------------------------------------------------------------- */ @@ -329,4 +333,5 @@ static void __init at91sam9x5_map_io(void) AT91_SOC_START(at91sam9x5) .map_io = at91sam9x5_map_io, .register_clocks = at91sam9x5_register_clocks, + .init = at91sam9x5_initialize, AT91_SOC_END diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c index debdbf8..42b955c 100644 --- a/arch/arm/mach-at91/pm.c +++ b/arch/arm/mach-at91/pm.c @@ -318,6 +318,11 @@ static struct platform_device at91_cpuidle_device = { .name = "cpuidle-at91", }; +void at91_pm_set_standby(void (*at91_standby)(void)) +{ + at91_cpuidle_device.dev.platform_data = at91_standby; +} + static int __init at91_pm_init(void) { #ifdef CONFIG_AT91_SLOW_CLOCK @@ -327,16 +332,8 @@ 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_cpuidle_device.dev.platform_data = at91rm9200_standby; + if (cpu_is_at91rm9200()) at91_ramc_write(0, AT91RM9200_SDRAMC_LPR, 0); - } else if (cpu_is_at91sam9g45()) { - at91_cpuidle_device.dev.platform_data = at91sam9g45_standby; - } else if (cpu_is_at91sam9263()) { - at91_cpuidle_device.dev.platform_data = at91sam9263_standby; - } else { - at91_cpuidle_device.dev.platform_data = at91sam9_standby; - } platform_device_register(&at91_cpuidle_device); diff --git a/arch/arm/mach-at91/pm.h b/arch/arm/mach-at91/pm.h index 2f5908f..76dd1a7 100644 --- a/arch/arm/mach-at91/pm.h +++ b/arch/arm/mach-at91/pm.h @@ -11,9 +11,13 @@ #ifndef __ARCH_ARM_MACH_AT91_PM #define __ARCH_ARM_MACH_AT91_PM +#include + #include #include +extern void at91_pm_set_standby(void (*at91_standby)(void)); + /* * The AT91RM9200 goes into self-refresh mode with this command, and will * terminate self-refresh automatically on the next SDRAM access. diff --git a/arch/arm/mach-at91/sama5d3.c b/arch/arm/mach-at91/sama5d3.c index 4012797..9a53db6 100644 --- a/arch/arm/mach-at91/sama5d3.c +++ b/arch/arm/mach-at91/sama5d3.c @@ -21,6 +21,7 @@ #include "generic.h" #include "clock.h" #include "sam9_smc.h" +#include "pm.h" /* -------------------------------------------------------------------- * Clocks @@ -365,13 +366,18 @@ static void __init sama5d3_register_clocks(void) /* -------------------------------------------------------------------- * AT91SAM9x5 processor initialization * -------------------------------------------------------------------- */ - static void __init sama5d3_map_io(void) { at91_init_sram(0, SAMA5D3_SRAM_BASE, SAMA5D3_SRAM_SIZE); } +static void __init sama5d3_initialize(void) +{ + at91_set_standby(at91sam9_standby); +} + AT91_SOC_START(sama5d3) .map_io = sama5d3_map_io, .register_clocks = sama5d3_register_clocks, + .init = sama5d3_initialize, AT91_SOC_END