Received: by 2002:a05:7412:31a9:b0:e2:908c:2ebd with SMTP id et41csp2849651rdb; Tue, 12 Sep 2023 14:14:17 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHt6qWfFzv2DmaIQ8+xyZT7F7WZUMNn3AbDSeQfuQWSjmNYQ08ZOqVyfxseJ4CfRfXXvobB X-Received: by 2002:a05:6a00:981:b0:68e:2d2d:56c1 with SMTP id u1-20020a056a00098100b0068e2d2d56c1mr1113468pfg.9.1694553256756; Tue, 12 Sep 2023 14:14:16 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1694553256; cv=none; d=google.com; s=arc-20160816; b=IKbIvIXFAFLbU9MXO9jWsLTVr9idr2mwL+yCtSAVNwjDZw9g9H2FHAKUyYTJXhtWr4 Qc+G3HUdIRjDILahTHHzm31Fgcza9dUliR5dB+ZnuLLlZAiaf3otLu+f1v0NpmKpXgey nJCaUdIO2AzXSbO8c4iq5h3Gxh1ie7L4UC5aOhpIUXJgu7Z0G5NCZabkNeg1d+DIm0b1 n/oAyrENit+DWd+HRIm8VT82jlKffG8dv6335IfvNoXoId4fScxg7163mYvFDTXD5Nj8 6ZLRko9iSCEkqA6EtF5E43dTwgnorYiJ/Vt2sebIzLWYmrDv/P1U1jfF1J7UNoWMFNvO y0fQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-disposition:mime-version :references:message-id:subject:cc:to:from:date; bh=sLH9+Nv2n0h0ruF37ZxOtSh8qdtjACK4vwo7xGuSYpk=; fh=TbgDsgopWf/1rB5DGbkQEQUTBm+vUq6cJULN3A5sHLo=; b=Of2mdgeAI/ObKo3LOSc4Sj6Gbenc+CyA26trugkwqeqTBQgd/Z3giBSQ0slcjUjwjT Oq1SWubMKHhGIECMiLirEkWy+Uut+v4nUPjq3Euf49yuwnO+ybeFbbK/UqVEI+0GmaBN E9wtG4bOxAgiWksDZ0mXD+/ny6+S7kV8KkYMUZEsB9cccSHnbVpB0zl/fLk9bFPNoB+L fny/QpLgIaQcgmTmjmEQwSDVz36Yg1y4YjxQG3+35hEK/EJIwtRHohIr0sYnt0dlJ+4Q hrZ5SrCspRImXfE+RpffYOCrt5jOz7BMPdgI1QwANqpffNV06YPxAcSjGR5S3QuGwvbJ 2mMg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from agentk.vger.email (agentk.vger.email. [23.128.96.32]) by mx.google.com with ESMTPS id b19-20020a63cf53000000b00573fbbf187dsi8425360pgj.216.2023.09.12.14.14.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 12 Sep 2023 14:14:16 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by agentk.vger.email (Postfix) with ESMTP id EA68C8098401; Tue, 12 Sep 2023 14:14:08 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at agentk.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234637AbjILVN5 (ORCPT + 99 others); Tue, 12 Sep 2023 17:13:57 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37074 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230453AbjILVN4 (ORCPT ); Tue, 12 Sep 2023 17:13:56 -0400 Received: from pidgin.makrotopia.org (pidgin.makrotopia.org [185.142.180.65]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5C3FA10CC; Tue, 12 Sep 2023 14:13:52 -0700 (PDT) Received: from local by pidgin.makrotopia.org with esmtpsa (TLS1.3:TLS_AES_256_GCM_SHA384:256) (Exim 4.96) (envelope-from ) id 1qgAhe-0007po-1g; Tue, 12 Sep 2023 21:13:46 +0000 Date: Tue, 12 Sep 2023 22:13:35 +0100 From: Daniel Golle To: Jeremy Kerr Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Lee Jones , Rob Herring , Krzysztof Kozlowski , Arnd Bergmann , Philipp Zabel , Mark Brown Subject: Re: [PATCH v4 2/2] mfd: syscon: allow reset control for syscon devices Message-ID: References: <20230105005010.124948-1-jk@codeconstruct.com.au> <20230105005010.124948-3-jk@codeconstruct.com.au> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230105005010.124948-3-jk@codeconstruct.com.au> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (agentk.vger.email [0.0.0.0]); Tue, 12 Sep 2023 14:14:09 -0700 (PDT) X-Spam-Status: No, score=-0.8 required=5.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on agentk.vger.email Hi Jeremy, On Thu, Jan 05, 2023 at 08:50:10AM +0800, Jeremy Kerr wrote: > Simple syscon devices may require deassertion of a reset signal in order > to access their register set. Rather than requiring a custom driver to > implement this, we can use the generic "resets" specifiers to link a > reset line to the syscon. > > This change adds an optional reset line to the syscon device > description, and deasserts the reset if detected. > > Signed-off-by: Jeremy Kerr > Reviewed-by: Arnd Bergmann > > --- > v2: > * do reset control in the early of_syscon_register() path, rather than > the platform device init, which isn't used. > v3: > * use a direct reset_control_deassert rather than handling in the > regmap > v4: > * collapse unnecessary `else` block > --- > drivers/mfd/syscon.c | 27 +++++++++++++++++++++------ > 1 file changed, 21 insertions(+), 6 deletions(-) > > diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c > index bdb2ce7ff03b..57b29c325131 100644 > --- a/drivers/mfd/syscon.c > +++ b/drivers/mfd/syscon.c > @@ -20,6 +20,7 @@ > #include > #include > #include > +#include > #include > #include > > @@ -31,6 +32,7 @@ static LIST_HEAD(syscon_list); > struct syscon { > struct device_node *np; > struct regmap *regmap; > + struct reset_control *reset; You are reserving a field 'reset' in this struct but then never use it. I stumbled upon this because I was wondering if it'd make sense to have something like int syscon_reset(struct syscon *syscon) { if (!syscon->reset) return -EOPNOTSUPP; return reset_control_reset(syscon->reset); } But then, of course, why not have syscon_reset_assert, *_deassert, ... ? Wrapping the reset control ops, of course, would have the advantage the syscon driver would be able to track whether reset is asserted. However, making the reset shared also allows the syscon user to use the reset as well and is mich easier to implement: diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c index 57b29c3251312..55f3f855f0305 100644 --- a/drivers/mfd/syscon.c +++ b/drivers/mfd/syscon.c @@ -32,7 +32,6 @@ static LIST_HEAD(syscon_list); struct syscon { struct device_node *np; struct regmap *regmap; - struct reset_control *reset; struct list_head list; }; @@ -130,15 +129,16 @@ static struct syscon *of_syscon_register(struct device_node *np, bool check_res) goto err_attach_clk; } - reset = of_reset_control_get_optional_exclusive(np, NULL); + reset = of_reset_control_get_shared(np, NULL); if (IS_ERR(reset)) { ret = PTR_ERR(reset); - goto err_attach_clk; + if (ret != -ENOENT) + goto err_attach_clk; + } else { + ret = reset_control_deassert(reset); + if (ret) + goto err_reset; } - - ret = reset_control_deassert(reset); - if (ret) - goto err_reset; } syscon->regmap = regmap; --- Let me know what you think. > struct list_head list; > }; > > @@ -40,7 +42,7 @@ static const struct regmap_config syscon_regmap_config = { > .reg_stride = 4, > }; > > -static struct syscon *of_syscon_register(struct device_node *np, bool check_clk) > +static struct syscon *of_syscon_register(struct device_node *np, bool check_res) > { > struct clk *clk; > struct syscon *syscon; > @@ -50,6 +52,7 @@ static struct syscon *of_syscon_register(struct device_node *np, bool check_clk) > int ret; > struct regmap_config syscon_config = syscon_regmap_config; > struct resource res; > + struct reset_control *reset; > > syscon = kzalloc(sizeof(*syscon), GFP_KERNEL); > if (!syscon) > @@ -114,7 +117,7 @@ static struct syscon *of_syscon_register(struct device_node *np, bool check_clk) > goto err_regmap; > } > > - if (check_clk) { > + if (check_res) { > clk = of_clk_get(np, 0); > if (IS_ERR(clk)) { > ret = PTR_ERR(clk); > @@ -124,8 +127,18 @@ static struct syscon *of_syscon_register(struct device_node *np, bool check_clk) > } else { > ret = regmap_mmio_attach_clk(regmap, clk); > if (ret) > - goto err_attach; > + goto err_attach_clk; > } > + > + reset = of_reset_control_get_optional_exclusive(np, NULL); > + if (IS_ERR(reset)) { > + ret = PTR_ERR(reset); > + goto err_attach_clk; > + } > + > + ret = reset_control_deassert(reset); > + if (ret) > + goto err_reset; > } > > syscon->regmap = regmap; > @@ -137,7 +150,9 @@ static struct syscon *of_syscon_register(struct device_node *np, bool check_clk) > > return syscon; > > -err_attach: > +err_reset: > + reset_control_put(reset); > +err_attach_clk: > if (!IS_ERR(clk)) > clk_put(clk); > err_clk: > @@ -150,7 +165,7 @@ static struct syscon *of_syscon_register(struct device_node *np, bool check_clk) > } > > static struct regmap *device_node_get_regmap(struct device_node *np, > - bool check_clk) > + bool check_res) > { > struct syscon *entry, *syscon = NULL; > > @@ -165,7 +180,7 @@ static struct regmap *device_node_get_regmap(struct device_node *np, > spin_unlock(&syscon_list_slock); > > if (!syscon) > - syscon = of_syscon_register(np, check_clk); > + syscon = of_syscon_register(np, check_res); > > if (IS_ERR(syscon)) > return ERR_CAST(syscon); > -- > 2.38.1 >