Received: by 2002:a05:6a10:f347:0:0:0:0 with SMTP id d7csp337375pxu; Sun, 22 Nov 2020 09:46:20 -0800 (PST) X-Google-Smtp-Source: ABdhPJwctmTIBd3kJVcv2OyL5hmapnJhtZtsqsXfl5yc3n0t+mfr4S2uu7m/eno9bc86qEtMY0gv X-Received: by 2002:a05:6402:370:: with SMTP id s16mr42250000edw.50.1606067179817; Sun, 22 Nov 2020 09:46:19 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1606067179; cv=none; d=google.com; s=arc-20160816; b=X+cPsF+ePQ6wSMZUWTMBQ9IQTyTjPXLtsSLiuv3koHDgD2lSHijUpMyfSamEtWUOdr LSnuM1SAMhNwOy+C4us8hZz27SV+UP58xnDM1Lq9pqdMmD15GU3C3LRz6SLHMgvaYGZ+ 3sdLrKZafOi4AFpyzb0QsXEPEKgfPlUF8gIFTLIGFcShwnYURdAg09Wad2Po+mA8+a+j vkseRQVLY8Jh3VG1e5VvIhEwl6Ex8djAgUgB1SHV0tP3ouRFRIxmz+JB141VQKaym/Fg YeW7RxE6biNcBl9rvT/Eoh4+Ui13eCR1D3e1yGsZ/PCRAtdGB6bZ54aVs1fzwIhuAhif 4vTw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:content-language :in-reply-to:mime-version:user-agent:date:message-id:from:references :cc:to:subject:dkim-signature; bh=Vs7K0liplB0YMIyj5LAc8+c1AJgx/p9mg5YBTryXfy4=; b=KdRAHjxxE1muJJtgzZO7txTwUIegULqTjMjwmeTcwmc13ZgIyNCPWuwAvvZCkY1gnP IYf4d58XH/25wYbZbCHSXRIi0s7vFM13vbyRyF6BU3qXEQuDQ101Ip/7iv1d1aFoOatg WReSQ41P4uYsPcuqcSrMnEKqropvAVdmfONnBPNnzUPhitIiqkrPuACbyIJHEGb4nqdP /B3lgRtNO4m1BnjkTR0RfSfmTpGxw+sUoFyJOJf8F+yq9OIbi+OBusH4zzJYYbwJvZo4 LFf5udnxR5dBFDOYDUCev+FjbxcfxhyXGDX6k4rKc8jx/0aZFk/Xhf0I3idem3lFr4Yi 5vgw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@ti.com header.s=ti-com-17Q1 header.b="W//HRR50"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=NONE dis=NONE) header.from=ti.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id ec12si5426683ejb.727.2020.11.22.09.45.57; Sun, 22 Nov 2020 09:46:19 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@ti.com header.s=ti-com-17Q1 header.b="W//HRR50"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=NONE dis=NONE) header.from=ti.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728069AbgKVRm6 (ORCPT + 99 others); Sun, 22 Nov 2020 12:42:58 -0500 Received: from fllv0016.ext.ti.com ([198.47.19.142]:41810 "EHLO fllv0016.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727567AbgKVRm6 (ORCPT ); Sun, 22 Nov 2020 12:42:58 -0500 Received: from lelv0265.itg.ti.com ([10.180.67.224]) by fllv0016.ext.ti.com (8.15.2/8.15.2) with ESMTP id 0AMHgqJl071373; Sun, 22 Nov 2020 11:42:52 -0600 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ti.com; s=ti-com-17Q1; t=1606066972; bh=Vs7K0liplB0YMIyj5LAc8+c1AJgx/p9mg5YBTryXfy4=; h=Subject:To:CC:References:From:Date:In-Reply-To; b=W//HRR50R4MINWIoySDeSKkxvPj9Hf5Hq+YnObKREpQNhYi+HEAUmr2Nzt6bHAJtx M2nhc4ULs6nv66B6VJFKdSnrG/DJ6B+AR/FZgML1lW7WzdxkPQZDoSew1PLjuy5TGp oTQKu5jC89o7DTlOqq0JSGGWeiHecgbMJJeWXb4I= Received: from DLEE114.ent.ti.com (dlee114.ent.ti.com [157.170.170.25]) by lelv0265.itg.ti.com (8.15.2/8.15.2) with ESMTPS id 0AMHgqog048381 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=FAIL); Sun, 22 Nov 2020 11:42:52 -0600 Received: from DLEE101.ent.ti.com (157.170.170.31) by DLEE114.ent.ti.com (157.170.170.25) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1979.3; Sun, 22 Nov 2020 11:42:52 -0600 Received: from lelv0326.itg.ti.com (10.180.67.84) by DLEE101.ent.ti.com (157.170.170.31) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1979.3 via Frontend Transport; Sun, 22 Nov 2020 11:42:52 -0600 Received: from [10.250.68.46] (ileax41-snat.itg.ti.com [10.172.224.153]) by lelv0326.itg.ti.com (8.15.2/8.15.2) with ESMTP id 0AMHgpwF090283; Sun, 22 Nov 2020 11:42:52 -0600 Subject: Re: [PATCH] remoteproc: Add module parameter 'auto_boot' To: Paul Cercueil CC: Mathieu Poirier , Ohad Ben-Cohen , Bjorn Andersson , , , References: <20201115115056.83225-1-paul@crapouillou.net> <20201120223701.GF4137289@xps15> <65e4ed08-9709-533f-57bb-cb570165a461@ti.com> From: Suman Anna Message-ID: Date: Sun, 22 Nov 2020 11:42:51 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 8bit X-EXCLAIMER-MD-CONFIG: e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Paul, On 11/21/20 12:47 PM, Paul Cercueil wrote: > Hi Suman, > > Le ven. 20 nov. 2020 à 17:06, Suman Anna a écrit : >> Hi Paul, >> >> On 11/20/20 4:37 PM, Mathieu Poirier wrote: >>>  Hi Paul, >>> >>>  On Sun, Nov 15, 2020 at 11:50:56AM +0000, Paul Cercueil wrote: >>>>  Until now the remoteproc core would always default to trying to boot the >>>>  remote processor at startup. The various remoteproc drivers could >>>>  however override that setting. >>>> >>>>  Whether or not we want the remote processor to boot, really depends on >>>>  the nature of the processor itself - a processor built into a WiFi chip >>>>  will need to be booted for the WiFi hardware to be usable, for instance, >>>>  but a general-purpose co-processor does not have any predeterminated >>>>  function, and as such we cannot assume that the OS will want the >>>>  processor to be booted - yet alone that we have a single do-it-all >>>>  firmware to load. >>>> >>> >>>  If I understand correctly you have various remote processors that use the >>> same firmware >>>  but are serving different purposes - is this correct? >>> >>>>  Add a 'auto_boot' module parameter that instructs the remoteproc whether >>>>  or not it should auto-boot the remote processor, which will default to >>>>  "true" to respect the previous behaviour. >>>> >>> >>>  Given that the core can't be a module I wonder if this isn't something that >>>  would be better off in the specific platform driver or the device tree...  >>> Other >>>  people might have an opinion as well. >> >> I agree. Even it is a module, all it is setting up is default behavior, and >> doesn't buy you much. If you have one or more remoteproc drivers supporting >> different instances, and each one wants different behavior, you would have to >> customize it in the drivers anyway. ST drivers are customizing this using a DT >> flag. > > Devicetree is supposed to describe the hardware, not how you're supposed to use > the hardware... I agree, but it sneaked in somehow. Anyway, I am not proposing that you repeat the same approach. > >> Given that the individual platform drivers have to be modules, is there any >> issue in customizing this in your platform driver? > > No, I can patch the platform driver instead, but to me it clearly is a core issue. So, remoteproc core is just setting up a default, and it is upto the individual drivers to override it. I actually have two different rproc drivers on the same SoC wanting different behavior for example. I do like the runtime config in general compared to a build-time Kconfig option, but let's say we do add this option, then you would also need every rproc driver to actually set this field specifically to ensure their expected behavior doesn't change if it is installed as false. regards Suman > > Cheers, > -Paul > >> regards >> Suman >> >>> >>>  Thanks, >>>  Mathieu >>> >>>>  Signed-off-by: Paul Cercueil >>>>  --- >>>>   drivers/remoteproc/remoteproc_core.c | 7 ++++++- >>>>   1 file changed, 6 insertions(+), 1 deletion(-) >>>> >>>>  diff --git a/drivers/remoteproc/remoteproc_core.c >>>> b/drivers/remoteproc/remoteproc_core.c >>>>  index dab2c0f5caf0..687b1bfd49db 100644 >>>>  --- a/drivers/remoteproc/remoteproc_core.c >>>>  +++ b/drivers/remoteproc/remoteproc_core.c >>>>  @@ -44,6 +44,11 @@ >>>> >>>>   #define HIGH_BITS_MASK 0xFFFFFFFF00000000ULL >>>> >>>>  +static bool auto_boot = true; >>>>  +module_param(auto_boot, bool, 0400); >>>>  +MODULE_PARM_DESC(auto_boot, >>>>  +         "Auto-boot the remote processor [default=true]"); >>>>  + >>>>   static DEFINE_MUTEX(rproc_list_mutex); >>>>   static LIST_HEAD(rproc_list); >>>>   static struct notifier_block rproc_panic_nb; >>>>  @@ -2176,7 +2181,7 @@ struct rproc *rproc_alloc(struct device *dev, const >>>> char *name, >>>>           return NULL; >>>> >>>>       rproc->priv = &rproc[1]; >>>>  -    rproc->auto_boot = true; >>>>  +    rproc->auto_boot = auto_boot; >>>>       rproc->elf_class = ELFCLASSNONE; >>>>       rproc->elf_machine = EM_NONE; >>>> >>>>  -- >>>>  2.29.2 >>>> >> > >