From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.3 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4F03FC4338F for ; Mon, 9 Aug 2021 16:31:25 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 31A6F60F56 for ; Mon, 9 Aug 2021 16:31:23 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 31A6F60F56 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=konsulko.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 7B52F81D6C; Mon, 9 Aug 2021 18:31:21 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=konsulko.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=konsulko.com header.i=@konsulko.com header.b="EFZygJgg"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id DE3C082051; Mon, 9 Aug 2021 18:31:18 +0200 (CEST) Received: from mail-qv1-xf2d.google.com (mail-qv1-xf2d.google.com [IPv6:2607:f8b0:4864:20::f2d]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 4D60D807F3 for ; Mon, 9 Aug 2021 18:31:15 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=konsulko.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=trini@konsulko.com Received: by mail-qv1-xf2d.google.com with SMTP id db14so9250718qvb.10 for ; Mon, 09 Aug 2021 09:31:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=konsulko.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=szZT6mqtjxVR+9u2YiH7Yj7lgJ75CeP4Fh0QMeM54tM=; b=EFZygJggHwXRfKw/lJO5hSlMMRNN6Qb2+MCxLRQvs6sPb6Bf2ehQZlzbVwrf6M8ho4 e4sHRx+//16B89TrEs+XscYpa8wVxomQ6bIeBGdOTw/qjamRZmcsAL4IVXoKPwUo7g1s yXjrMNQeWOs649F3L0h+31k8XYsRJAEdcv7P8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=szZT6mqtjxVR+9u2YiH7Yj7lgJ75CeP4Fh0QMeM54tM=; b=fgSpmoi+reIObazOasOJaN4aG/eJNOm8WvkLhGcQ7XUE6QTZG/WSmF1H/x5sygTevB MdiaBeW/iP1Qy7/KjXAm077F9MfrMEFbA0ZL6i0ZZSHkDMhgR78C1god0C9LvgovAjGg flMzW8rKHeqR74rATE7dz9C2uDQiaEXbrayAvALsgnMoFsIly95t5+uAvQzURxW78rdd dIGku/Y/TCaeyj+qH/N5fSc6xhKsLfahC1A9CZ33zgPi/euF71OpEvEU0UToOCHq5YZd dbfXZ0nlJGTSAKxVd44x+V9t7ikzm5Y/7tZyvuAjemsRAoe5Sg541gj1oy+8UYEsCk0G 1etQ== X-Gm-Message-State: AOAM532nXPUnFchhjrrHPWyAIhXq9M6d4ZznevEUu87VfCciBBmrne9W bTDOhLyoITprJxbXF0CKX2shsA== X-Google-Smtp-Source: ABdhPJxEch+5YG0F8s42Jkfh0uGQcwGmJavAsruk6vycNsCsnbaurHr9JFvuFjSv7T7/2D0IoOLRZg== X-Received: by 2002:ad4:5fcd:: with SMTP id jq13mr24690227qvb.57.1628526673990; Mon, 09 Aug 2021 09:31:13 -0700 (PDT) Received: from bill-the-cat (2603-6081-7b01-cbda-b1cf-f982-fc10-d612.res6.spectrum.com. [2603:6081:7b01:cbda:b1cf:f982:fc10:d612]) by smtp.gmail.com with ESMTPSA id a8sm8521078qkn.63.2021.08.09.09.31.12 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Mon, 09 Aug 2021 09:31:12 -0700 (PDT) Date: Mon, 9 Aug 2021 12:31:10 -0400 From: Tom Rini To: Michal Simek Cc: u-boot@lists.denx.de, git@xilinx.com, Alexandre GRIVEAUX , Ashok Reddy Soma , Aswath Govindraju , Ilias Apalodimas , Lukasz Majewski , Michal Simek , Simon Glass , T Karthik Reddy Subject: Re: [PATCH] xilinx: Disable ARCH_FIXUP_FDT_MEMORY Message-ID: <20210809163110.GZ858@bill-the-cat> References: <1f2589b334942bc5adeedd5df58e6af32ec31ce2.1628252571.git.michal.simek@xilinx.com> <20210806184656.GD858@bill-the-cat> <5637d26e-aaf8-fc4f-b681-dd10b16a9359@xilinx.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="bLgs+aUf1UT65bBm" Content-Disposition: inline In-Reply-To: <5637d26e-aaf8-fc4f-b681-dd10b16a9359@xilinx.com> X-Clacks-Overhead: GNU Terry Pratchett User-Agent: Mutt/1.9.4 (2018-02-28) X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean --bLgs+aUf1UT65bBm Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Aug 09, 2021 at 08:24:48AM +0200, Michal Simek wrote: >=20 >=20 > On 8/6/21 8:46 PM, Tom Rini wrote: > > On Fri, Aug 06, 2021 at 02:22:56PM +0200, Michal Simek wrote: > >=20 > >> Based on DT spec you can have one memory node which multiple ranges or > >> multiple nodes. > >> fdt_fixup_memory_banks() is not implemented in a correct way when mult= iple > >> memory nodes are present because all ranges are put it to the first me= mory > >> node found. And next memory nodes are kept in DT which ends up in the = same > >> range specification in the same DT. > >> > >> Here is what it is happening. > >> Origin DT. > >> memory@0 { > >> device_type =3D "memory"; > >> reg =3D <0x0 0x0 0x0 0x80000000>; > >> }; > >> > >> memory@800000000 { > >> device_type =3D "memory"; > >> reg =3D <0x8 0x00000000 0x0 0x80000000>; > >> }; > >> > >> After fdt_fixup_memory_banks() > >> > >> memory@0 { > >> device_type =3D "memory"; > >> reg =3D <0x0 0x0 0x0 0x80000000>, <0x8 0x00000000 0x0 0x800000= 00>; > >> }; > >> > >> memory@800000000 { > >> device_type =3D "memory"; > >> reg =3D <0x8 0x00000000 0x0 0x80000000>; > >> }; > >> > >> As is visible memory@0 node got second range but there is still > >> memory@800000000 node present and 2G range is listed twice. > >> > >> The solution can't be that second node is removed because it can be > >> referenced already that's why it is better for us to disable this opti= on > >> for now. > >=20 > > I don't object to the patch at all. The main use of > > fdt_fixup_memory_banks() is for the (at least used to be most common) > > case of device trees that didn't fill in memory size, so that it could > > be done at run-time, depending on the amount found. I do wonder if > > CONFIG_ARCH_FIXUP_FDT_MEMORY being default makes the most sense these > > days. Or maybe we just need some comments on fdt_fixup_memory_banks > > making it clear which way we implement the spec as it does allow for as > > you note, either way. >=20 > The multi memory node configuration is not that common and the most of > SOC are using one node with multiple ranges. It means I would say a lot > of SOCs are not affected. OK. > Not sure how many SOCs are really using this feature that at run time > you detect amount of memory. I remember freescale powerquicks with > reading SPD eeprom on DIMMs and then one custom board with xilinx soc > which was produced with two memory sizes where detection was performed. I think there's still a large number of platforms that don't describe the amount of memory in the device tree and let it be figured out at "run time", even if there's not nearly the level of SPD EEPROM related smarts as there are in some environments. U-Boot's get_ram_size() is usually good enough for those cases. But it's been a long time since I checked a wide variety of dts files, and that it gets the more than 4GB case wrong too is why there are a number of platforms that opt-out. > But anyway if you want me to add comment to that function to describe > what it is implemented now I can do it. > And of course the best would be to update this function handle both ways > but for us is it better to disable this for now till we have real need > to use it. > We have also done internally one implementation but I don't think it > works in generic way. A comment for now would be a great start, thanks! --=20 Tom --bLgs+aUf1UT65bBm Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQGzBAABCgAdFiEEGjx/cOCPqxcHgJu/FHw5/5Y0tywFAmERWEQACgkQFHw5/5Y0 tyxVpwwAmXafBd85lzVsqO0ODnlwxwkHB9yB2h8dSFUbmZYhzNCPzsga4SMZNlOa mMpFtH4jh2EeeCeJOGmV9/tV1DSr5pk7RiWZ153lG38gpR504VGpnNG61BIi6kKZ 9kL+pqzK+YrvZqOQd6RhL3yZLNmtJTTxGp9na/FZPL0PvUYa5vM2MM7VV3ukSYHA u1p7Nv0yZzO5GI5EzVMKuAMZtUC+SxdXL4wfn6ExTlolflkE3DxjA1khTaCy+fu7 E+dfzsySpmv9ZrSo+AWaB1UZB1OeMGsLwTyFrAxENwiPOZIjbP9bkrzQ7SdJVcl3 ljRTtwYCvqM4dPaWX1u2aZzcpznj3CLxdccW66WRTNeqs7oqi5ksv8hha3VyMRxJ 74R5HM5VslwRvIqJcJl3Hby1IqsYM6vB2oUlvXgR6r+VvQ8S7deg7/LHr/ML802M +DoUr0fFj0wsS4e6zl1mmzQefyuSa2alMHVNE7J8FsbWsZ3ntlDYkMFtHZZ71LFB 7wo3eHwt =zfqP -----END PGP SIGNATURE----- --bLgs+aUf1UT65bBm--