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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id CE43CC5479D for ; Thu, 12 Jan 2023 02:10:46 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id B666E850D6; Thu, 12 Jan 2023 03:10:43 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=kernel.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.b="D29ctitS"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id A367085131; Thu, 12 Jan 2023 03:10:38 +0100 (CET) Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id A783F8500A for ; Thu, 12 Jan 2023 03:10:34 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=kernel.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=robh@kernel.org Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 7D15E612A3 for ; Thu, 12 Jan 2023 02:10:31 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id DF8D2C433EF for ; Thu, 12 Jan 2023 02:10:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1673489430; bh=TTYiYtABtK9WnhoeUHOs57sDfEEh2RrP1YiNx2pR2AU=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=D29ctitSuvZYpgQ+/n8XYIw0RhwXqZsMa+mSpTDIMR54YWVu/1k4ZfI9nFRrm85Q0 DL2c0CkAh66PvD/nWJSYBETLhSYJax6OUe4Q3kgh//ba7h9PwZJut328LDnnFz5EQQ kfJYygWQXAlTCoL98NLY/QdmGK1aPsQJBtWP0Ms5QlIwIsCq4UAoH9rbPO5mWSFm52 vM1gkw3zb+RX9OhXkXspFx7a0pbgGkx6h3eTzUBKm8EHrujoFaZU+o7TmoynqbpUD2 Hh1ETz/w9Q5AUMMOiBTLHAQxCWjULg/oxciyUQvK3wafm9sDgudWEBM5GjA59JatxN w7YT5+2nDgvtg== Received: by mail-vk1-f179.google.com with SMTP id t2so8144557vkk.9 for ; Wed, 11 Jan 2023 18:10:30 -0800 (PST) X-Gm-Message-State: AFqh2krNkfYRs47MW/oe4P/6L4C139MHYSqSrfjW2QjVNYqW1B36VrnC KyIEaKEZvH5ze3aQ2M2O8nvrEY139IqqlWqr0w== X-Google-Smtp-Source: AMrXdXso6UEEB28yUHN42Mww47kIUnVfpsSkpV+jMgwljBWuDcsDMnSQDYUQs30JoMRqW0IX8gQwb25INDqLP0GhoeY= X-Received: by 2002:a05:6122:221e:b0:3da:f920:c0ef with SMTP id bb30-20020a056122221e00b003daf920c0efmr515848vkb.26.1673489429809; Wed, 11 Jan 2023 18:10:29 -0800 (PST) MIME-Version: 1.0 References: <20221107192055.21669-1-abdellatif.elkhlifi@arm.com> <20221122131751.22747-1-abdellatif.elkhlifi@arm.com> <20221122131751.22747-4-abdellatif.elkhlifi@arm.com> <20221124132115.GA393@e121910.cambridge.arm.com> <20221219111251.GA22370@e121910.cambridge.arm.com> In-Reply-To: From: Rob Herring Date: Wed, 11 Jan 2023 20:10:18 -0600 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver To: Simon Glass Cc: Abdellatif El Khlifi , ilias.apalodimas@linaro.org, jens.wiklander@linaro.org, achin.gupta@arm.com, trini@konsulko.com, xueliang.zhong@arm.com, nd@arm.com, u-boot@lists.denx.de Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 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.6 at phobos.denx.de X-Virus-Status: Clean On Mon, Dec 19, 2022 at 1:21 PM Simon Glass wrote: > > Hi Abdellatif, > > On Mon, 19 Dec 2022 at 04:12, Abdellatif El Khlifi > wrote: > > > > On Mon, Dec 05, 2022 at 09:49:30AM -0600, Rob Herring wrote: > > > On Sun, Dec 4, 2022 at 1:22 PM Simon Glass wrote: > > > > > > > > Hi Rob, > > > > > > > > On Tue, 29 Nov 2022 at 05:22, Rob Herring wrote: > > > > > > > > > > On Fri, Nov 25, 2022 at 3:18 PM Simon Glass wr= ote: > > > > > > > > > > > > Hi Abdellatif, > > > > > > > > > > > > On Thu, 24 Nov 2022 at 06:21, Abdellatif El Khlifi wrote: > > > > > > > > > > > > > > On Tue, Nov 22, 2022 at 07:09:16PM -0700, Simon Glass wrote: > > > > > > > > should be called 'priov' and should beHi Abdellatif, > > > > > > > > > > > > > > > > > > > > [..] > > > > > > > > > > > > > > > +/** > > > > > > > > > + * ffa_device_get - create, bind and probe the arm_ffa d= evice > > > > > > > > > + * @pdev: the address of a device pointer (to be filled = when the arm_ffa bus device is created > > > > > > > > > + * successfully) > > > > > > > > > + * > > > > > > > > > + * This function makes sure the arm_ffa device is > > > > > > > > > + * created, bound to this driver, probed and ready to us= e. > > > > > > > > > + * Arm FF-A transport is implemented through a single U-= Boot > > > > > > > > > + * device managing the FF-A bus (arm_ffa). > > > > > > > > > + * > > > > > > > > > + * Return: > > > > > > > > > + * > > > > > > > > > + * 0 on success. Otherwise, failure > > > > > > > > > + */ > > > > > > > > > +int ffa_device_get(struct udevice **pdev) > > > > > > > > > +{ > > > > > > > > > + int ret; > > > > > > > > > + struct udevice *dev =3D NULL; > > > > > > > > > + > > > > > > > > > + ret =3D device_bind(dm_root(), DM_DRIVER_GET(arm_= ffa), FFA_DRV_NAME, NULL, ofnode_null(), > > > > > > > > > + &dev); > > > > > > > > > > > > > > > > Please add a DT binding. Even if only temporary, we need so= mething for this. > > > > > > > > > > > > > > Thanks for the feedback. I'm happy to address all the comment= s. > > > > > > > > > > > > > > Regarding DT binding and FF-A discovery. We agreed with Linar= o and Rob Herring > > > > > > > about the following: > > > > > > > > > > > > > > - DT is only for what we failed to make discoverable. For har= dware, we're stuck > > > > > > > with it. We shouldn't repeat that for software interfaces. = This approach is > > > > > > > already applied in the FF-A kernel driver which comes with = no DT support and > > > > > > > discovers the bus with bus_register() API [1]. > > > > > > > > > > > > This may be the UEFI view, but it is not how U-Boot works. This= is not something we are 'stuck' with. It is how we define what is present = on a device. This is how the PCI bus works in U-Boot. It is best practice i= n U-Boot to use the device tree to make this things visible and configurabl= e. Unlike with Linux there is no other way to provide configuration needed = by these devices. > > > > > > > > > > Where do you get UEFI out of this? > > > > > > > > I assume it was UEFI as there was no discussion about this in U-Boo= t. > > > > Which firmware project was consulted about this? > > > > > > > > > > > > > > It is the discoverability of hardware that is fixed (and we are s= tuck > > > > > with). We can't change hardware. The disoverability may be PCI > > > > > VID/PID, USB device descriptors, or nothing. We only use DT when = those > > > > > are not sufficient. For a software interface, there is no reason = to > > > > > make them non-discoverable as the interface can be fixed (at leas= t for > > > > > new things like FF-A). > > > > > > > > Here I am talking about the controller itself, the top-level node i= n > > > > the device tree. For PCI this is a device tree node and it should b= e > > > > the same here. So I am not saying that the devices on the bus need = to > > > > be in the device tree (that can be optional, but may be useful in s= ome > > > > situations where it is status and known). > > > > > > Sure, the PCI host bridges are not discoverable, have a bunch of > > > resources, and do need to be in DT. The downstream devices only do if > > > they have extra resources such as when a device is soldered down on a > > > board rather than a standard slot. > > > > > > > We need something like: > > > > > > > > ff-a { > > > > compatible =3D "something"; > > > > }; > > > > > > > > I don't know what mechanism is actually used to communicate with it= , > > > > but that will be enough to get the top-level driver started. > > > > > > There's discovery of FF-A itself and then discovery of FF-A features > > > (e.g. partitions). Both of those are discoverable without DT. The > > > first is done by checking the SMCCC version, then checking for FF-A > > > presence and features. Putting this into DT is redundant. Worse, what > > > if they disagree? > > > > Hi Simon, > > > > Do you agree with Rob, Ilias and myself that it makes more sense > > FF-A bus is discovered without a DT node and following the same approac= h as > > Linux ? (FF-A bus doesn't have a HW controller and is a purely SW bus, > > no configuration/description needed at DT level). > > > > Your suggestions are always welcome. > > I'm sorry I don't agree with that. It does need a compatible string, > like PCI has. You can just add it in U-Boot if Linux won't accept the > binding. It's not like PCI as the host side of PCI has non-discoverable resources. This all could have been designed better, but hindsight is 20/20 and things evolved step by step. There are a bunch of firmware services that are all behind SMCCC. The first (upstream) was PSCI. IIRC, SMCCC was invented a bit after that, but generalized PSCI for other services. Since then more have been added. More services get added one by one and yes we added bindings for them. Because what's one more... But that really needs to stop. We're stuck with h/w that's not discoverable, there's zero reason to do that with s/w interfaces. If we could redo everything, we'd have a node for SMCCC and that's it unless there's h/w resources provided to the rest of DT. But we can't, so SMCCC is discovered by the presence of PSCI. Rob