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 aws-us-west-2-korg-lkml-1.web.codeaurora.org (localhost.localdomain [127.0.0.1]) by smtp.lore.kernel.org (Postfix) with ESMTP id 150A6C433FE for ; Tue, 11 Oct 2022 01:35:47 +0000 (UTC) Received: from mailout4.zoneedit.com (mailout4.zoneedit.com [64.68.198.64]) by mx.groups.io with SMTP id smtpd.web10.2517.1665452139884942918 for ; Mon, 10 Oct 2022 18:35:40 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=none, err=permanent DNS error (domain: denix.org, ip: 64.68.198.64, mailfrom: denis@denix.org) Received: from localhost (localhost [127.0.0.1]) by mailout4.zoneedit.com (Postfix) with ESMTP id 3E76F40D37; Tue, 11 Oct 2022 01:35:39 +0000 (UTC) Received: from mailout4.zoneedit.com ([127.0.0.1]) by localhost (zmo14-pco.easydns.vpn [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id cF9N9BPW_oHo; Tue, 11 Oct 2022 01:35:39 +0000 (UTC) Received: from mail.denix.org (pool-100-15-80-88.washdc.fios.verizon.net [100.15.80.88]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mailout4.zoneedit.com (Postfix) with ESMTPSA id 0A0E740D2F; Tue, 11 Oct 2022 01:35:35 +0000 (UTC) Received: by mail.denix.org (Postfix, from userid 1000) id F1163174A18; Mon, 10 Oct 2022 21:35:33 -0400 (EDT) Date: Mon, 10 Oct 2022 21:35:33 -0400 From: Denys Dmytriyenko To: Andrew Davis Cc: Denys Dmytriyenko , Ryan Eatmon , meta-arago@lists.yoctoproject.org Subject: Re: [meta-arago][master/kirkstone][PATCH 7/7] meta-arago: Use new J7 SoC names over specific board names Message-ID: <20221011013533.GE30861@denix.org> References: <20221006150628.17044-1-afd@ti.com> <20221006150628.17044-7-afd@ti.com> <20221007201014.GA30861@denix.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.20 (2009-06-14) List-Id: X-Webhook-Received: from li982-79.members.linode.com [45.33.32.79] by aws-us-west-2-korg-lkml-1.web.codeaurora.org with HTTPS for ; Tue, 11 Oct 2022 01:35:47 -0000 X-Groupsio-URL: https://lists.yoctoproject.org/g/meta-arago/message/14065 On Mon, Oct 10, 2022 at 08:26:26AM -0500, Andrew Davis wrote: > On 10/7/22 3:10 PM, Denys Dmytriyenko wrote: > >On Thu, Oct 06, 2022 at 10:06:28AM -0500, Andrew Davis via lists.yoctoproject.org wrote: > >>Now that we have SoC names, we can avoid adding features based on the > >>board name. We expect folks to create their own boards based on these > >>SoCs, and so using the TI made EVM board name everywhere adds extra churn > >>when adding a new board. Plus it is more correct for most of these > >>features as they depend on the SoC, not on the EVM board. > >> > >>One other thing we do here is to not use the generic "j7" name, > >>the current and future J7 devices are far to feature diverse > >>to group at this level. Grouping like that will lead to the wrong > >>things getting enabled as new J7 SoCs are added. > > > >Hmm, this second part is rather backwards (IMO) in some places, see below. > > > > > >>diff --git a/meta-arago-distro/recipes-core/images/tisdk-core-bundle.inc b/meta-arago-distro/recipes-core/images/tisdk-core-bundle.inc > >>index 3c31ba18..296eef7a 100644 > >>--- a/meta-arago-distro/recipes-core/images/tisdk-core-bundle.inc > >>+++ b/meta-arago-distro/recipes-core/images/tisdk-core-bundle.inc > >>@@ -24,7 +24,9 @@ DTB_FILTER:am57xx-hs-evm = "${DTB_FILTER:am57xx-evm}" > >> DTB_FILTER:ti43x = "am43" > >> DTB_FILTER:omapl138 = "da850" > >> DTB_FILTER:am65xx = "am65" > >>-DTB_FILTER:j7 = "j721e" > >>+DTB_FILTER:j721e = "j721e" > >>+DTB_FILTER:j7200 = "j7200" > >>+DTB_FILTER:j721s2 = "j721s2" > > > >Yes, here it does make perfect sense. > > > > > >>diff --git a/meta-arago-distro/recipes-core/packagegroups/packagegroup-arago-tisdk-addons.bb b/meta-arago-distro/recipes-core/packagegroups/packagegroup-arago-tisdk-addons.bb > >>index f4e72a89..c01e9497 100644 > >>--- a/meta-arago-distro/recipes-core/packagegroups/packagegroup-arago-tisdk-addons.bb > >>+++ b/meta-arago-distro/recipes-core/packagegroups/packagegroup-arago-tisdk-addons.bb > >>@@ -61,9 +61,9 @@ UTILS:append:am64xx = " ti-rtos-firmware pru-icss" > >> UTILS:append:am62xx = " ti-rtos-firmware" > >> #UTILS:append:am65xx = " ti-rtos-firmware pru-icss pru-pwm-fw" > >> UTILS:append:am65xx = " ti-rtos-firmware pru-icss" > >>-UTILS:append:j7 = " ti-rtos-firmware" > >>-UTILS:append:j721e-evm = " pru-icss" > >>-UTILS:append:j721e-hs-evm = " pru-icss" > >>+UTILS:append:j721e = " ti-rtos-firmware pru-icss" > >>+UTILS:append:j7200 = " ti-rtos-firmware" > >>+UTILS:append:j721s2 = " ti-rtos-firmware" > > > >Here - not so much. ti-rtos-firmware is applicable to all j7. So, I'd leave > >that line alone and only replace adding pru-icss to specific EVMs to adding it > >to j721e SoC family. > > > >Or go even further - ti-rtos-firmware is common to all k3 platforms, so all > >the individual am65xx, am64xx and am62xx, along with j7, could be replaced > >with one line: > > > >UTILS:append:k3 = " ti-rtos-firmware" > > > >And then add pri-icss or other extra FW to specific SoC families only. > > > > If you want I can make this re-arrangement in a follow up patch. > > > > >>diff --git a/meta-arago-distro/recipes-core/packagegroups/ti-analytics.bb b/meta-arago-distro/recipes-core/packagegroups/ti-analytics.bb > >>index e16e4d51..e6e0b915 100644 > >>--- a/meta-arago-distro/recipes-core/packagegroups/ti-analytics.bb > >>+++ b/meta-arago-distro/recipes-core/packagegroups/ti-analytics.bb > >>@@ -19,7 +19,9 @@ ANALYTICS = "" > >> # ${@['','qt-opencv-opencl-opengl-multithreaded'][oe.utils.all_distro_features(d, 'opencv opencl opengl', True, False) and bb.utils.contains('MACHINE_FEATURES', 'gpu dsp', True, False, d)]} \ > >> # ${@['','barcode-roi'][oe.utils.all_distro_features(d, 'opencv', True, False) and bb.utils.contains('MACHINE_FEATURES', 'dsp', True, False, d)]} \ > >> #" > >>-ANALYTICS:j7 = "" > >>+ANALYTICS:j721e = "" > >>+ANALYTICS:j7200 = "" > >>+ANALYTICS:j721s2 = "" > > > >Well, this is probably completely wrong by now, anyway. > > > > Yes it probably is, but then my point in this patch was to > not make any changes to functionality (or as few as possible). > So if it is broken now, it was broken just the same before. > > > > >> ANALYTICS:omapl138 = "" > >> RDEPENDS:${PN} = "\ > >>diff --git a/meta-arago-distro/recipes-core/packagegroups/ti-test.bb b/meta-arago-distro/recipes-core/packagegroups/ti-test.bb > >>index 99a6cc82..5f56f8be 100644 > >>--- a/meta-arago-distro/recipes-core/packagegroups/ti-test.bb > >>+++ b/meta-arago-distro/recipes-core/packagegroups/ti-test.bb > >>@@ -78,7 +78,15 @@ ARAGO_TI_TEST:append:k3 = " \ > >> k3conf \ > >> " > >>-ARAGO_TI_TEST:append:j7 = " \ > >>+ARAGO_TI_TEST:append:j721e = " \ > >>+ ufs-utils \ > >>+" > >>+ > >>+ARAGO_TI_TEST:append:j7200 = " \ > >>+ ufs-utils \ > >>+" > >>+ > >>+ARAGO_TI_TEST:append:j721s2 = " \ > >> ufs-utils \ > >> " > > > >ufs-utils is quite generic and should probably be added to k3 or maybe even > >all platforms... > > > > Same as above, would you like this done in a follow up patch? This patch only > fans out the name keeping the same functionality as before. If that exposes > existing issues or opportunities for cleanup, then I can make those next. It would have been fine to do further cleanup/optimization/collapsing of the code in a followup patch, if you weren't first expanding it here... In other words, it seems replacing "j7" override with more specific "j721e/j7200/j721s2" overrides only makes sense for DTB_FILTER case and should be left alone in other 3 places. -- Denys