From patchwork Tue May 8 00:52:16 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Marty E. Plummer" X-Patchwork-Id: 10385013 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id BACB460236 for ; Tue, 8 May 2018 00:53:05 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id AF0042843C for ; Tue, 8 May 2018 00:53:05 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id A01B728590; Tue, 8 May 2018 00:53:05 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.9 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,MAILING_LIST_MULTI autolearn=ham version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 8B0F22843C for ; Tue, 8 May 2018 00:53:04 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:Mime-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=mA6OJwmv0y8oFTzG5SlqZ/9TqU452L+ndt0vvKIwFbg=; b=m2/h2Qwk8x8jrz Z+1avqXJ+7FJxNqR7M02SDInZuh3Y12WaMLz4mFhzCH12iOJy+7+8Kyo4T9i5IfyiITE/B9DYnsox t81vTTKOjIzsvXky3RAgMzf5BXbhivXC08/xfbRxQAFW4C9tYCT6qIyxxyxePfhIKEP9iGfMhGDL6 Lgg5bQ7PdgChVs3dTZbmLh9OaLU4HNPaGwucds52++a0Q8Ra5P9zypOQPOQzDd17Q7qb2jE8m1F7j Momq2i93jBKs3iIJQ6t8hGOYMtBZw+e+IQ15P7RKdu2j/TxrPwfqpaP+UflWsDWK4u3te40mVhsRd BchcqE/vIJ3sooFfoZPA==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1fFqsF-0002VL-Sq; Tue, 08 May 2018 00:52:59 +0000 Received: from mx-out1.startmail.com ([145.131.90.139]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1fFqsB-0002Td-1W for linux-rockchip@lists.infradead.org; Tue, 08 May 2018 00:52:57 +0000 Date: Mon, 7 May 2018 19:52:16 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=startmail.com; s=2017-11; t=1525740756; bh=GoX4b3pJIqZBPOuR12GTsNHOFV+bxJYZuJhWIGhJPmI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=s/mjD1g4i3TtCtsj6oVegi/K2dbdgWYA3OhISbgi/7G3xWkKf1kCH54Q9TfPYmGnh bU3kXT2D5Cmb8gHBfacs6thiLD1SGQIZUbf/zHOHcV0FbZ4KswAYf7dDc3pi7sVbgD tJwcdWljRvtSVFKSzmpnUv3s9EysKBncrXhmrDT0b+Db28ac1UWqryUnDnQDxQwRRN Dp6ueVM6K5VonzkcJixVkBPg6dh42SLKd1jpN+JLZq6Z2zq3WnGbWQJLWw8ycgpyS8 Lxg45b25B6GXX3iMb2r8/LRd1rsRx8GnfU0w2TgGZRvWsbHe10IFEmUN1Cfx/oIMXh HTYOXPX7JlOYQ== From: "Marty E. Plummer" To: "Dr. Philipp Tomsich" Subject: Re: [U-Boot] [PATCH 3/3] rockchip: fix incorrect detection of ram size Message-ID: <20180508005215.s3uivl43jfaryv7j@proprietary-killer> References: <20180506142513.19911-1-hanetzer@startmail.com> <20180506142513.19911-4-hanetzer@startmail.com> <6a0116f8-fd8a-dd53-2759-0014c7ab2700@rock-chips.com> <20180507023413.mil4wj7xtjy6gnuw@proprietary-killer> <791299C0-EEC0-4DB1-97C4-A8DAD3B1A481@theobroma-systems.com> Mime-Version: 1.0 Content-Disposition: inline In-Reply-To: <791299C0-EEC0-4DB1-97C4-A8DAD3B1A481@theobroma-systems.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20180507_175255_378475_FFB7BBEC X-CRM114-Status: GOOD ( 30.55 ) X-BeenThere: linux-rockchip@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: Upstream kernel work for Rockchip platforms List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: vagrant@debian.org, u-boot@lists.denx.de, Kever Yang , linux-rockchip@lists.infradead.org Sender: "Linux-rockchip" Errors-To: linux-rockchip-bounces+patchwork-linux-rockchip=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP On Mon, May 07, 2018 at 11:16:28AM +0200, Dr. Philipp Tomsich wrote: > > > On 7 May 2018, at 04:34, Marty E. Plummer wrote: > > > > On Mon, May 07, 2018 at 10:20:55AM +0800, Kever Yang wrote: > >> Hi Marty, > >> > >> > >> On 05/06/2018 10:25 PM, Marty E. Plummer wrote: > >>> Taken from coreboot's src/soc/rockchip/rk3288/sdram.c > >>> > >>> Without this change, my u-boot build for the asus c201 chromebook (4GiB) > >>> is incorrectly detected as 0 Bytes of ram. > >> > >> I know the root cause for this issue, and I have a local patch for it. > >> The rk3288 is 32bit, and 4GB size is just out of range, so we need to before > >> the max size before return with '<<20'. Sorry for forgot to send it out. > >> > >>> > >>> Signed-off-by: Marty E. Plummer > >>> --- > >>> arch/arm/mach-rockchip/sdram_common.c | 62 ++++++++++++++++----------- > >>> 1 file changed, 37 insertions(+), 25 deletions(-) > >>> > >>> diff --git a/arch/arm/mach-rockchip/sdram_common.c b/arch/arm/mach-rockchip/sdram_common.c > >>> index 76dbdc8715..a9c9f970a4 100644 > >>> --- a/arch/arm/mach-rockchip/sdram_common.c > >>> +++ b/arch/arm/mach-rockchip/sdram_common.c > >>> @@ -10,6 +10,8 @@ > >>> #include > >>> #include > >>> #include > >>> +#include > >>> +#include > >>> > >>> DECLARE_GLOBAL_DATA_PTR; > >>> size_t rockchip_sdram_size(phys_addr_t reg) > >>> @@ -19,34 +21,44 @@ size_t rockchip_sdram_size(phys_addr_t reg) > >>> size_t size_mb = 0; > >>> u32 ch; > >>> > >>> - u32 sys_reg = readl(reg); > >>> - u32 ch_num = 1 + ((sys_reg >> SYS_REG_NUM_CH_SHIFT) > >>> - & SYS_REG_NUM_CH_MASK); > >>> + if (!size_mb) { > >> > >> I don't understand this and follow up changes, we don't really need it, > >> isn't it? > >> I think don't need the changes before here. > > Yeah, that was just another level of indentation for the if (!size_mb) > > guard, but I've reworked the patch to not do that as it was pointed out > > that since size_mb is initialized to 0 prior. > >>> + /* > >>> + * we use the 0x00000000~0xfeffffff space > >>> + * since 0xff000000~0xffffffff is soc register space > >>> + * so we reserve it > >>> + */ > >>> + size_mb = min(size_mb, 0xff000000/SZ_1M); > >> > >> This is what we really need, as Klaus point out, we need to use > >> SDRAM_MAX_SIZE > >> instead of hard code. > > Yeah, I've got a rework on that which uses SDRAM_MAX_SIZE as instructed, > > build and boot tested on my hardware. > > In that case you just masked the problem but didn???t solve it: assuming size_mb > is size_t (I???ll assume this is 64bit, but did not check), then your 4GB is 0x1_0000_0000 ) > which overflows to 0x0 when converted to a u32. > > In other words: we need to figure out where the truncation occurs (image what > happens if a new 32bit processor with LPAE comes out???). > A very valid point. With the following patch to sdram_common.c and sdram_rk3288.c applied I get the debug output that follows it: --- U-Boot SPL 2018.05-rc3-02370-g309384e84b-dirty (May 07 2018 - 19:42:15 -0500) Trying to boot from SPI U-Boot 2018.05-rc3-02370-g309384e84b-dirty (May 07 2018 - 19:42:15 -0500) Model: Google Speedy DRAM: rockchip_sdram_size ff73009c 3c50dc50 rockchip_sdram_size: 42: chipsize_mb 400 rockchip_sdram_size: 49: size_mb 800 rank 2 col 11 bk 3 cs0_row 14 bw 2 row_3_4 0 rockchip_sdram_size: 42: chipsize_mb 400 rockchip_sdram_size: 49: size_mb 1000 rank 2 col 11 bk 3 cs0_row 14 bw 2 row_3_4 0 rockchip_sdram_size: 54: size_mb 1000 SDRAM base=0, size=fe000000 4 GiB MMC: dwmmc@ff0c0000: 1, dwmmc@ff0d0000: 2, dwmmc@ff0f0000: 0 In: cros-ec-keyb Out: vidconsole Err: vidconsole Model: Google Speedy rockchip_dnl_key_pressed: adc_channel_single_shot fail! Net: Net Initialization Skipped No ethernet found. Hit any key to stop autoboot: 0 I guess we need to change the size_t to something larger; unless I'm mistaken, that's a 32 bit value, right? and 0x100000000 is at least 40 bits, unless I'm missing the issue here somewhere. However, that would take a change to include/ram.h, and would impact far more than just rk3288/rockchip devices across the board, so I'm unsure how to proceed. Use the min macro here for now, and begin work migrating the ram_info size member to a 64-bit container? > >> > >> Thanks, > >> - Kever > >>> } > >>> > >>> return (size_t)size_mb << 20; > >> > >> > > _______________________________________________ > > U-Boot mailing list > > U-Boot@lists.denx.de > > https://lists.denx.de/listinfo/u-boot diff --git a/arch/arm/mach-rockchip/sdram_common.c b/arch/arm/mach-rockchip/sdram_common.c index 232a7fa655..0fe69bf558 100644 --- a/arch/arm/mach-rockchip/sdram_common.c +++ b/arch/arm/mach-rockchip/sdram_common.c @@ -4,6 +4,7 @@ * SPDX-License-Identifier: GPL-2.0+ */ +#define DEBUG 1 #include #include #include @@ -39,16 +40,19 @@ size_t rockchip_sdram_size(phys_addr_t reg) SYS_REG_ROW_3_4_MASK; chipsize_mb = (1 << (cs0_row + col + bk + bw - 20)); + debug("%s: %d: chipsize_mb %x\n", __func__, __LINE__, chipsize_mb); if (rank > 1) chipsize_mb += chipsize_mb >> (cs0_row - cs1_row); if (row_3_4) chipsize_mb = chipsize_mb * 3 / 4; size_mb += chipsize_mb; + debug("%s: %d: size_mb %x\n", __func__, __LINE__, size_mb); debug("rank %d col %d bk %d cs0_row %d bw %d row_3_4 %d\n", rank, col, bk, cs0_row, bw, row_3_4); } + debug("%s: %d: size_mb %x\n", __func__, __LINE__, size_mb); size_mb = min(size_mb, SDRAM_MAX_SIZE/SZ_1M); return (size_t)size_mb << 20; diff --git a/drivers/ram/rockchip/sdram_rk3288.c b/drivers/ram/rockchip/sdram_rk3288.c index d99bf12476..9738eb088f 100644 --- a/drivers/ram/rockchip/sdram_rk3288.c +++ b/drivers/ram/rockchip/sdram_rk3288.c @@ -7,6 +7,7 @@ * Adapted from coreboot. */ +#define DEBUG 1 #include #include #include