linux-kernel - Re: [PATCH v3 4/6] ARM: rockchip: add suspend and resume for RK3288

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAD=FV=WLqbtRSVU+jqwn9qt33uNLbaWqL6NOCOMiutWn=8iVPw@mail.gmail.com>
Date:	Mon, 27 Oct 2014 10:42:14 -0700
From:	Doug Anderson <dianders@...omium.org>
To:	Kevin Hilman <khilman@...nel.org>
Cc:	Chris <zyw@...k-chips.com>, Heiko Stuebner <heiko@...ech.de>,
	Mike Turquette <mturquette@...aro.org>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Kumar Gala <galak@...eaurora.org>,
	Russell King <linux@....linux.org.uk>,
	linux-rockchip@...ts.infradead.org,
	Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	Linus Walleij <linus.walleij@...aro.org>,
	Tony Xie <xxx@...k-chips.com>
Subject: Re: [PATCH v3 4/6] ARM: rockchip: add suspend and resume for RK3288

Kevin,

On Fri, Oct 24, 2014 at 2:47 PM, Kevin Hilman <khilman@...nel.org> wrote:
>> +static void rk3288_fill_in_bootram(u32 level)
>> +{
>> +     rkpm_bootdata_cpusp = rk3288_bootram_phy + (SZ_4K - 8);
>> +     rkpm_bootdata_cpu_code = virt_to_phys(cpu_resume);
>> +
>> +     rkpm_bootdata_l2ctlr_f  = 1;
>> +     rkpm_bootdata_l2ctlr = rk3288_l2_config();
>
> You're storing this to an offset in your assembly code, which is then
> copied to SRAM.  Why not just read/store it from your assembly code?

I didn't understand this comment.  Chris is saving the pre-suspend
value into a location that will be accessible to the resume code.  It
needs to be PC-relative for the resume code to get to it since there's
no MMU yet.

He could copy it to SRAM first and then put it into SRAM, but I'm not
sure it makes a huge difference.


>> +static int rk3288_suspend_enter(suspend_state_t state)
>> +{
>> +     local_fiq_disable();
>> +
>> +     rk3288_save_settings(ROCKCHIP_ARM_OFF_LOGIC_NORMAL);
>
> You (re)write your assembly code into SRAM every time you suspend.  Does
> your SRAM lose context? ...
>
>> +     cpu_suspend(0, rockchip_lpmode_enter);
>> +
>> +     rk3288_restore_settings();
>
> ... or is it just because you overwrite it here?
>
> Why are you reading/writing 4K of SRAM every suspend/resume?  Is it
> because the SRAM is actually shared with some other devices/drivers?
>
> If so, maybe you should be using an SRAM allocator so the SRAM can be
> shared, and you don't have to read/write a full page of SRAM for every
> suspend/resume.

This 4K chunk of SRAM is pretty specifically for suspend/resume, at
least on rk3288.  Other than the fact that it's only 4K, it has a lot
of severe limitations that keep it from being generally useful (it
will crash the SoC if you write non word-sized data to it and also it
can't be cached).

I'm going to suggest that we just don't save/restore it.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/