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 X-Spam-Level: X-Spam-Status: No, score=-14.4 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 150D6C4338F for ; Mon, 2 Aug 2021 18:47:01 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id CA00E60F6E for ; Mon, 2 Aug 2021 18:47:00 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org CA00E60F6E Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org 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=Q6uCBzRGv4P/87MqM5imPZzGODWpZJXCxQiNNJT/qLw=; b=Sd3DdZMgmxH5ZT 2inmNRy4PM2OH/U27zuU1h2xe1ayiZ+dTc+8ULe6eUHzx2W71wCoJdKC5E9/7hw/fy8zmkZB/C9jU ktGoo+ABEBPWkJmZweOu0mUbCq5n5cKPp19oxoXBK61hMfa1ovMSbmn83kkA+Z1IokphBoCpiL/3E o2MN8fiIoRBvUkW6tA5BhrIssyoNY8DemAp9sYUl+crv7KEMZImgWZv7jwhiEMJWq50eJ+a1UsEtO EjOIe3PH5hDoqGE6hdcKEhfh7QlfBf75JVhAS8mb6IV+PnQfBICqs52iUCfMOATlcnSt993fadgSX 4aei218TSaLYoG1+NfFw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mAcvX-00HYEL-F8; Mon, 02 Aug 2021 18:44:39 +0000 Received: from mail-pl1-x630.google.com ([2607:f8b0:4864:20::630]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mAcvT-00HYDj-2g for linux-arm-kernel@lists.infradead.org; Mon, 02 Aug 2021 18:44:36 +0000 Received: by mail-pl1-x630.google.com with SMTP id q2so20678234plr.11 for ; Mon, 02 Aug 2021 11:44:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=xw9+sFIfMAH/HmCEcLTBDtyZL+QXMp45a8jz7EGFlQs=; b=lNUAgm4cfsGZpZpOBIfUNmXtN6bpNAsKBZqaVqUSw76PKDLn42SaFj8qzAJtj2dzvE X67IJRtds0aeKGgwCMiEAGAZ1aAX39E+ydYZIM1LgOvTKRVeLOvahqyDUKBIl2E0l6Bh /n1F/7zdCGy7oZW7a6+q2RdIDS8pC4A/kvjJPLsre65I03Hg0sqnJvovMmg9jtrmOWNd P6ZRhHSTDpirTHe3z2O8cYy2lfv0S9UZ57R7zL5eAfduBATsD7JiAmcZiIN9lYZhAn6b jRH+52ZfFZzX0FucOrM8BEPJCNxNfJwNQYGeZ5c1rTIO82QtkN0Z/4l74GEGJIbOFZnq z9FA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=xw9+sFIfMAH/HmCEcLTBDtyZL+QXMp45a8jz7EGFlQs=; b=WGOrpW6gKszIjvrlF/snO5XzRUe9qAnUgWRnMRInvLMAdZZ+pu0WQbhjN/k1NrjYrY PhBSqdoMMM7rHrFdQTZCLq6oTbW+MkD1qm3odAi3Gca4/QmfLUe3cjsOVcz9N2XvRdAW sf7jL/0cCU0twYeJLyg4+NmSEKEqYuiMvC37aZ7gWeoCl3d5KszCysEVA4LK8VUDar7K 3wuqLYB04dRzTRve7i22jhWHk7svgCQDx/Xl0R3/lI7DK03wAnz45sfs3xMmgCg0+FPd oo7PjKaMqeZwkPAnMh/Khwjdzvkph9GlAvlbSoQLl2ZugncwgOc2NuAHEuoSARBn08G5 Nphg== X-Gm-Message-State: AOAM533IDtJmZvEYM9TOxV+Lv5QGWT8Og1iVl9IZR61CImeoYQaSCqF0 Mq82pMrg+ZpukcwRArlQY3hVYQ== X-Google-Smtp-Source: ABdhPJzPw/PEJ0SUwgLcAc/Q/l57IlRuaGBJVb9uZRO2pi9zMLrb7kvAqwkZ0Qx/l0JuxwvxTl36FA== X-Received: by 2002:a17:90a:1196:: with SMTP id e22mr271872pja.168.1627929874131; Mon, 02 Aug 2021 11:44:34 -0700 (PDT) Received: from p14s (S0106889e681aac74.cg.shawcable.net. [68.147.0.187]) by smtp.gmail.com with ESMTPSA id r13sm14163627pgi.78.2021.08.02.11.44.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 02 Aug 2021 11:44:33 -0700 (PDT) Date: Mon, 2 Aug 2021 12:44:31 -0600 From: Mathieu Poirier To: Suman Anna Cc: Bjorn Andersson , Lokesh Vutla , Praneeth Bajjuri , Hari Nagalla , linux-remoteproc@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 1/5] remoteproc: Add support for detach-only during shutdown Message-ID: <20210802184431.GC3051951@p14s> References: <20210723220248.6554-1-s-anna@ti.com> <20210723220248.6554-2-s-anna@ti.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210723220248.6554-2-s-anna@ti.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210802_114435_199962_10C007AB X-CRM114-Status: GOOD ( 32.23 ) 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 Fri, Jul 23, 2021 at 05:02:44PM -0500, Suman Anna wrote: > The remoteproc core has support for both stopping and detaching a > remote processor that was attached to previously, through both the > remoteproc sysfs and cdev interfaces. The rproc_shutdown() though > unconditionally only uses the stop functionality at present. This > may not be the default desired functionality for all the remoteproc > platform drivers. > > Enhance the remoteproc core logic to key off the presence of the > .stop() ops and allow the individual remoteproc drivers to continue > to use the standard rproc_add() and rproc_del() API. This allows > the remoteproc drivers to only do detach if supported when the driver > is uninstalled, and the remote processor continues to run undisturbed > even after the driver removal. > > Signed-off-by: Suman Anna > --- > v2: Addressed various review comments from v1 > - Reworked the logic to not use remoteproc detach_on_shutdown and > rely only on rproc callback ops > - Updated the last para of the patch description > v1: https://patchwork.kernel.org/project/linux-remoteproc/patch/20210522000309.26134-3-s-anna@ti.com/ > > drivers/remoteproc/remoteproc_cdev.c | 7 +++++++ > drivers/remoteproc/remoteproc_core.c | 5 ++++- > drivers/remoteproc/remoteproc_sysfs.c | 6 ++++++ > 3 files changed, 17 insertions(+), 1 deletion(-) > > diff --git a/drivers/remoteproc/remoteproc_cdev.c b/drivers/remoteproc/remoteproc_cdev.c > index 4ad98b0b8caa..16c932beed88 100644 > --- a/drivers/remoteproc/remoteproc_cdev.c > +++ b/drivers/remoteproc/remoteproc_cdev.c > @@ -42,6 +42,13 @@ static ssize_t rproc_cdev_write(struct file *filp, const char __user *buf, size_ > rproc->state != RPROC_ATTACHED) > return -EINVAL; > > + if (rproc->state == RPROC_ATTACHED && This is already checked just above. > + !rproc->ops->stop) { This is checked in rproc_stop() where -EINVAL is returned if ops::stop has not been provided. > + dev_err(&rproc->dev, > + "stop not supported for this rproc, use detach\n"); The standard error message from the shell should be enough here, the same way it is enough when the "start" and "stop" scenarios fail. > + return -EINVAL; > + } > + > rproc_shutdown(rproc); > } else if (!strncmp(cmd, "detach", len)) { > if (rproc->state != RPROC_ATTACHED) > diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c > index 7de5905d276a..ab9e52180b04 100644 > --- a/drivers/remoteproc/remoteproc_core.c > +++ b/drivers/remoteproc/remoteproc_core.c > @@ -2075,7 +2075,10 @@ void rproc_shutdown(struct rproc *rproc) > if (!atomic_dec_and_test(&rproc->power)) > goto out; > > - ret = rproc_stop(rproc, false); > + if (rproc->state == RPROC_ATTACHED && !rproc->ops->stop) > + ret = __rproc_detach(rproc); > + else > + ret = rproc_stop(rproc, false); As I indicated in my last review I think rproc_shutdown() and rproc_del() should be decoupled and the right call made in the platform drivers based on the state of the remote processor. Conditions such as the above make the core code brittle, difficult to understand and tedious to maintain. Thanks, Mathieu > if (ret) { > atomic_inc(&rproc->power); > goto out; > diff --git a/drivers/remoteproc/remoteproc_sysfs.c b/drivers/remoteproc/remoteproc_sysfs.c > index ea8b89f97d7b..133e766f38d4 100644 > --- a/drivers/remoteproc/remoteproc_sysfs.c > +++ b/drivers/remoteproc/remoteproc_sysfs.c > @@ -206,6 +206,12 @@ static ssize_t state_store(struct device *dev, > rproc->state != RPROC_ATTACHED) > return -EINVAL; > > + if (rproc->state == RPROC_ATTACHED && > + !rproc->ops->stop) { > + dev_err(&rproc->dev, "stop not supported for this rproc, use detach\n"); > + return -EINVAL; > + } > + > rproc_shutdown(rproc); > } else if (sysfs_streq(buf, "detach")) { > if (rproc->state != RPROC_ATTACHED) > -- > 2.32.0 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel