From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 100971A1E8B for ; Fri, 14 Oct 2016 06:32:51 -0700 (PDT) Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 6A369800B0; Fri, 14 Oct 2016 13:32:50 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-51.phx2.redhat.com [10.3.116.51]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u9EDWnu0000722; Fri, 14 Oct 2016 09:32:49 -0400 To: Bhupesh Sharma References: <1476437615-29534-1-git-send-email-bhupesh.sharma@nxp.com> <38e8d690-b981-4b37-1ad2-6c6975ad2a9a@redhat.com> Cc: "edk2-devel@ml01.01.org" From: Laszlo Ersek Message-ID: <53299f19-a5f8-99c8-78cd-219c4fb31699@redhat.com> Date: Fri, 14 Oct 2016 15:32:48 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Fri, 14 Oct 2016 13:32:50 +0000 (UTC) Subject: Re: [PATCH 1/1] MdePkg/IoLib: Add support for big-endian MMIO X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 14 Oct 2016 13:32:51 -0000 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit On 10/14/16 15:17, Bhupesh Sharma wrote: > Hi Laszlo, > >> From: Laszlo Ersek [mailto:lersek@redhat.com] >> Sent: Friday, October 14, 2016 5:37 PM >> >> On 10/14/16 11:33, Bhupesh Sharma wrote: >>> Various IPs on NXP/FSL SoCs having ARM64 cores have big-endian MMIO >>> interfaces. >>> >>> This implies that a byte-swap operation is needed to read/write such >>> BE MMIO registers from the LE ARM64 cores. >>> >>> This patch adds the support for the same. >>> >>> Contributed-under: TianoCore Contribution Agreement 1.0 >>> Signed-off-by: Meenakshi Aggarwal >>> Signed-off-by: Bhupesh Sharma >>> --- >>> MdePkg/Include/Library/IoLib.h | 364 >> ++++++++++++++++++++ >>> MdePkg/Library/BaseIoLibIntrinsic/IoLibArm.c | 479 >>> +++++++++++++++++++++++++++ >>> 2 files changed, 843 insertions(+) >> >> I think this is both overkill and incomplete, at the same time :) >> >> - Incomplete because only one IoLib instance gets the implementation. > > Agree, but do we have an example of similar BE MMIO IPs on other > Architectures/soc? I was only aware of NXP/FSL having such MMIO > interfaces as the IPs have been reused from PPC SoC, which used > to have a BE core and hence did not require a SwapByte. For example, the fw_cfg device of QEMU is big endian. OvmfPkg and ArmVirtPkg call SwapBytesXX as necessary, in combination with MmioWriteXX. In general, I think it's expected that a library declared under MdePkg will not cause build failures (unresolved symbols) in any platform code, once the library is resolved correctly in the platform DSC. > >> - Overkill because you can easily use the SwapBytes16, SwapBytes32, >> SwapBytes64 functions -- also from BaseLib --, for transforming >> MmioWrite arguments and MmioRead results. >> > > Yes, but that means at every IP driver needs to especially carry such > arguments and transform the results. Indeed. > That might be an overkill. I think it should be possible to factor out these functions to a separate library, or non-standard protocol, that is central enough for the platform or device in question, but not core enough for putting into MdePkg. > We already have similar implementations MMIO implementations in Linux for e.g. > http://lxr.free-electrons.com/source/include/asm-generic/io.h#L642 Hmmm. That does detract from the value of my "overkill" argument. So I guess I'll have to defer to the MdePkg maintainers on that. Regarding my "incomplete" argument, it still stands. I think it's logically impossible to introduce a library class that is simultaneously - central enough to merit a place under MdePkg, but - not central enough to receive implementations (library instances) for all the platforms supported by edk2 at the moment. Thanks Laszlo