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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 48554C636CC for ; Mon, 13 Feb 2023 17:51:16 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc: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=/C06qbmfmeSvqv5XiAJTzuSrRgbebQIEPN8KKdcYVwc=; b=oauGj3F/QDMPDZ tOlNPXPXr8NQH4zmXw9k92/kyJXIQu/ehJ2kgdeXNBcu7FaMnouiEheB9r5y07VDCqmjr/nQ7f3w0 ZHKwwyi/gvL78UWefMKGsI9Wtprt7Q92vEFGtjBTJ/VR90dc30/RqktN5WiRjOpTlv0LfCamqxGXV PJiVQ79kh3dCTWjURLqsbEY3i9gas1AOKhviHU+/jctSTojVWEYnTpo1JFEcLnWcDvurNQXfk58la Nz4A491W3bBntAM7NI7sRICCe/aDNnv1DE7vX54Hyqr2SVVPhhInUdAqvRpVdlf3N5z6ZeKddc9Fu 4uSHQ+JuExQ8By+hj7bw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pRcxy-00Fjhs-SV; Mon, 13 Feb 2023 17:50:15 +0000 Received: from mail-pj1-x102e.google.com ([2607:f8b0:4864:20::102e]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1pRcxu-00Fjgz-Ht for linux-arm-kernel@lists.infradead.org; Mon, 13 Feb 2023 17:50:12 +0000 Received: by mail-pj1-x102e.google.com with SMTP id z14-20020a17090abd8e00b00233bb9d6bdcso7794390pjr.4 for ; Mon, 13 Feb 2023 09:50:09 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=M8NVw51hJujJ9w6ww4tXYpEn/8Vq7ox+Z1FASgY/bLU=; b=Bd5fqu4Y/EyTummo2fVWma8W7zSH94KPXwjC+My/qv4w4lRqc2bTM0aeCnwo6RxXqx Qr+kuCxm+ap5+tWeT8OpIKQQTipytFb7/VmEUWg7rfnNZKpqDqQEHV5nGn8XO1FGTIk+ Nv4k3G3cJKiR3Q7ObE4YCnakTxt7A81IvBvGWk1/KSvFfHtxEp6CPMyvhBkOKyOuYFvK lTno7aCndNECZC+kSDNG8MR1VstW/BurZNGlpBM46+040Vmc0bbLtTufBBF0wZGWZc+x zD2cd1bgxeINi8EXEJNHjiAb5HnZPX/jb5phCXk8ACCb2hEiYZweiC4Kbi2OEhcQmVTc i1ag== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=M8NVw51hJujJ9w6ww4tXYpEn/8Vq7ox+Z1FASgY/bLU=; b=LFoTBmNnfFPGIJCG7iYgbtPTRGuTvORHspRjtmyrI+LsxYLZOy58SMieDRaF0j4fsv 6RtQP0/LxTeVhgwSZVTeD6lLMJnfbTyCVXhGiFYzgO32XLA2q3/qTh+O8QYKa2kKf/oQ ICplizdRgV5vOZZ92NMJxFjsglh+G4nDU5VlE8aA7Tp/A5BuFQYRZWhtEVVX0xFBRKM9 tAhWx4zfYIQB5RfTPI7Hv6SRBi7zPVb65Z61ND1p1i7ZjShKFRNKTS+di8XS8tX+iuWc undmh3TsQVINpWWVpRKLVAQ/Tu8o0nKLYGSehBB4n6S0n5KUPhbp2OokphVQ39cgxdwr O8eA== X-Gm-Message-State: AO0yUKVqCHtsdYzGHge68Dj+gH4gQfc7aQnp4Zkp1vScNFDi7oOynv7q Ehlm/G/aHWCv9DaVZBQ5cy/vWg== X-Google-Smtp-Source: AK7set9lsbgjl5TgXfCPbU1vvfIDzkRjBJr9zNN0yZX/rOiaR87z3+Pi1tvaUwDAPm+VBgOq2lq3rA== X-Received: by 2002:a17:90b:4a0b:b0:233:9fff:888e with SMTP id kk11-20020a17090b4a0b00b002339fff888emr13858074pjb.39.1676310609341; Mon, 13 Feb 2023 09:50:09 -0800 (PST) Received: from p14s ([2604:3d09:148c:c800:8b5:7925:cf2a:8bac]) by smtp.gmail.com with ESMTPSA id i61-20020a17090a3dc300b00231224439c1sm4290656pjc.27.2023.02.13.09.50.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 13 Feb 2023 09:50:08 -0800 (PST) Date: Mon, 13 Feb 2023 10:50:06 -0700 From: Mathieu Poirier To: Iuliana Prodan Cc: Peng Fan , "Peng Fan (OSS)" , "andersson@kernel.org" , "shawnguo@kernel.org" , "s.hauer@pengutronix.de" , "arnaud.pouliquen@foss.st.com" , Daniel Baluta , "kernel@pengutronix.de" , "festevam@gmail.com" , dl-linux-imx , "linux-remoteproc@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH V3 0/6] remoteproc: imx_rproc: support firmware in DDR Message-ID: <20230213175006.GA310433@p14s> References: <20230209063816.2782206-1-peng.fan@oss.nxp.com> <2c4997fa-973c-dee4-9b26-6b38a1ca4540@nxp.com> <73d34c86-7c31-6530-0915-aa470af5d9ca@nxp.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <73d34c86-7c31-6530-0915-aa470af5d9ca@nxp.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230213_095010_622390_DA804B22 X-CRM114-Status: GOOD ( 52.84 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Feb 13, 2023 at 12:15:59PM +0200, Iuliana Prodan wrote: > On 2/12/2023 9:43 AM, Peng Fan wrote: > > Hi Iuliana, > > > > > Subject: Re: [PATCH V3 0/6] remoteproc: imx_rproc: support firmware in > > > DDR > > > > > > > > > On 2/9/2023 8:38 AM, Peng Fan (OSS) wrote: > > > > From: Peng Fan > > > > > > > > V3: > > > > > > > > Daniel, Iuliana > > > > > > > > Please help review this patchset per Mathieu's comments. > > > > > > > > Thanks, > > > > Peng. > > > > > > > > Move patch 3 in v2 to 1st patch in v3 and add Fixes tag Per Daniel > > > > IMX_RPROC_ANY in patch 3 Per Mathieu > > > > Update comment and commit log in patch 5, 6. > > > > > > > > NXP SDK provides ".interrupts" section, but I am not sure how others > > > > build the firmware. So I still keep patch 6 as v2, return bootaddr > > > > if there is no ".interrupts" section. > > > > > > > > V2: > > > > patch 4 is introduced for sparse check warning fix > > > > > > > > This pachset is to support i.MX8M and i.MX93 Cortex-M core firmware > > > > could be in DDR, not just the default TCM. > > > > > > > > i.MX8M needs stack/pc value be stored in TCML entry address[0,4], the > > > > initial value could be got from firmware first section ".interrupts". > > > > i.MX93 is a bit different, it just needs the address of .interrupts > > > > section. NXP SDK always has .interrupts section. > > > > > > > > So first we need find the .interrupts section from firmware, so patch > > > > 1 is to reuse the code of find_table to introduce a new API > > > > rproc_elf_find_shdr to find shdr, the it could reused by i.MX driver. > > > > > > > > Patch 2 is introduce devtype for i.MX8M/93 > > > > > > > > Although patch 3 is correct the mapping, but this area was never used > > > > by NXP SW team, we directly use the DDR region, not the alias region. > > > > Since this patchset is first to support firmware in DDR, mark this > > > > patch as a fix does not make much sense. > > > > > > > > patch 4 and 5 is support i.MX8M/93 firmware in DDR with parsing > > > > .interrupts section. Detailed information in each patch commit message. > > > > > > > > Patches were tested on i.MX8MQ-EVK i.MX8MP-EVK i.MX93-11x11-EVK > > > If one can build their firmware as they want, then the .interrupt section can > > > also be called differently. > > > I don't think is a good idea to base all your implementation on this > > > assumption. > > > > > > It's clear there's a limitation when linking firmware in DDR, so this should be > > > well documented so one can compile their firmware and put the needed > > > section (interrupt as we call it in NXP SDK) always in TCML - independently > > > where the other section go. > > Ok, so .interrupt section should be a must in elf file if I understand correctly. > > > > I could add a check in V4 that if .interrupt section is not there, driver will report > > failure. > > > > How do you think? > > Peng, I stand by my opinion that the limitation of linking firmware in DDR > should be documented in an Application Note, or maybe there are other > documents where how to use imx_rproc is explained. > > The implementation based on the .interrupt section is not robust. > Maybe a user linked his firmware correctly in TCML, but the section is not > called .interrupt so the firmware loading will work. > > So, instead of using the section name, you should use the address. Can you be more specific on the above? > > First, check whether there is a section linked to TCML. > If there is none, check for section name - as you did. > If there is no section called .interrupt, give an error message. We have two ways of booting, one that puts the firmware image in the TCML and another in RAM. Based on the processor type, the first 8 bytes of the TCML need to include the address for the stack and PC value. I think the first thing to do is have two different firmware images, one for i.MX8M and another one for i.MX93. That should greatly simplify things. Second, there should always be a segment that adds the right information to the TMCL. That segment doesn't need a name, it simply have to be part of the segments that are copied to memory (any kind of memory) so that function rproc_elf_load_segments() can do its job. That pushes the complexity to the tool that generates the firmware image, exactly where it should be. This is how I think we should solve this problem based on the very limited information provided with this patchset. Please let me know if I missed something and we'll go from there. > > For all the above options please add comments in code, explaining each step. > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel