Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(13)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1324223002: move stmf7 sdram code to /platform (Closed)

Created:
5 years, 3 months ago by cpu_(ooo_6.6-7.5)
Modified:
5 years, 3 months ago
Reviewers:
travisg
Base URL:
https://github.com/travisg/lk.git@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

move stmf7 sdram code to /platform BUG=none R=travisg@google.com Committed: 0b0b79be57935ceadd7a0b4d886a955152d59a9f

Patch Set 1 #

Total comments: 8

Patch Set 2 : final version #

Total comments: 6

Patch Set 3 : review fixes #

Unified diffs Side-by-side diffs Delta from patch set Stats (+252 lines, -474 lines) Patch
A + platform/stm32f7xx/include/platform/sdram.h View 1 2 2 chunks +25 lines, -22 lines 0 comments Download
M platform/stm32f7xx/rules.mk View 1 chunk +1 line, -0 lines 0 comments Download
A + platform/stm32f7xx/sdram.c View 1 2 9 chunks +98 lines, -128 lines 0 comments Download
M target/stm32746g-eval2/init.c View 1 2 4 chunks +63 lines, -2 lines 0 comments Download
M target/stm32746g-eval2/rules.mk View 1 chunk +0 lines, -1 line 0 comments Download
D target/stm32746g-eval2/sdram.c View 1 chunk +0 lines, -321 lines 0 comments Download
M target/stm32f746g-disco/init.c View 1 2 3 chunks +65 lines, -0 lines 0 comments Download

Messages

Total messages: 8 (1 generated)
travisg
this is a test comment https://codereview.chromium.org/1324223002/diff/1/platform/stm32f7xx/include/platform/sdram.h File platform/stm32f7xx/include/platform/sdram.h (left): https://codereview.chromium.org/1324223002/diff/1/platform/stm32f7xx/include/platform/sdram.h#oldcode26 platform/stm32f7xx/include/platform/sdram.h:26: #include <sys/types.h> this is ...
5 years, 3 months ago (2015-09-03 00:45:20 UTC) #2
cpu_(ooo_6.6-7.5)
https://codereview.chromium.org/1324223002/diff/1/platform/stm32f7xx/include/platform/sdram.h File platform/stm32f7xx/include/platform/sdram.h (left): https://codereview.chromium.org/1324223002/diff/1/platform/stm32f7xx/include/platform/sdram.h#oldcode31 platform/stm32f7xx/include/platform/sdram.h:31: // TODO: fpu context On 2015/09/03 00:45:19, travisg wrote: ...
5 years, 3 months ago (2015-09-03 00:50:07 UTC) #3
travisg
lgtm https://codereview.chromium.org/1324223002/diff/1/platform/stm32f7xx/include/platform/sdram.h File platform/stm32f7xx/include/platform/sdram.h (left): https://codereview.chromium.org/1324223002/diff/1/platform/stm32f7xx/include/platform/sdram.h#oldcode26 platform/stm32f7xx/include/platform/sdram.h:26: #include <sys/types.h> On 2015/09/03 00:45:19, travisg wrote: > ...
5 years, 3 months ago (2015-09-03 01:00:18 UTC) #4
travisg
lgtm
5 years, 3 months ago (2015-09-03 01:00:19 UTC) #5
travisg
mostly looks good, just a few nits. https://codereview.chromium.org/1324223002/diff/20001/platform/stm32f7xx/include/platform/sdram.h File platform/stm32f7xx/include/platform/sdram.h (left): https://codereview.chromium.org/1324223002/diff/20001/platform/stm32f7xx/include/platform/sdram.h#oldcode26 platform/stm32f7xx/include/platform/sdram.h:26: #include <compiler.h> ...
5 years, 3 months ago (2015-09-03 20:48:12 UTC) #6
cpu_(ooo_6.6-7.5)
Committed patchset #3 (id:40001) manually as 0b0b79be57935ceadd7a0b4d886a955152d59a9f (presubmit successful).
5 years, 3 months ago (2015-09-03 21:10:50 UTC) #7
cpu_(ooo_6.6-7.5)
5 years, 3 months ago (2015-09-03 21:13:14 UTC) #8
Message was sent while issue was closed. 
https://codereview.chromium.org/1324223002/diff/20001/platform/stm32f7xx/incl...
File platform/stm32f7xx/include/platform/sdram.h (left):

https://codereview.chromium.org/1324223002/diff/20001/platform/stm32f7xx/incl...
platform/stm32f7xx/include/platform/sdram.h:26: #include <compiler.h>
On 2015/09/03 20:48:11, travisg wrote:
> will need #include <stdint.h> for the uint8_t below.

Done.

https://codereview.chromium.org/1324223002/diff/20001/platform/stm32f7xx/incl...
File platform/stm32f7xx/include/platform/sdram.h (right):

https://codereview.chromium.org/1324223002/diff/20001/platform/stm32f7xx/incl...
platform/stm32f7xx/include/platform/sdram.h:24: #define
__PLATFORM_STM32F7_SDRAM_H
On 2015/09/03 20:48:11, travisg wrote:
> can just use #pragma once here if you'd like. Generally newer code is using it
> in lk.

Done.

https://codereview.chromium.org/1324223002/diff/20001/platform/stm32f7xx/incl...
platform/stm32f7xx/include/platform/sdram.h:54: uint8_t
BSP_SDRAM_Init(sdram_config_t* config);
On 2015/09/03 20:48:11, travisg wrote:
> lets go ahead and move away from the BSP naming convention and follow
something
> like stm32_sdram_init();

Done.

Powered by Google App Engine
This is Rietveld 408576698