From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-qv1-f49.google.com (mail-qv1-f49.google.com [209.85.219.49]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 55C9B17B50A for ; Tue, 7 Jan 2025 16:18:43 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.219.49 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736266727; cv=none; b=bOx5hcSGzDgoGi/m1hbNRRhe2yF22Px/vW1LTGV0PwprmV+tbWw2+Uy9GlXM2qHG1PfgaRA4HHNV5mTb+ZdDJFKnE3HB13Tn3WHv67KZGQmc+gvYq421YnofKMP7DsONuSiXFlQuwuKg8rsged0/e6XeymUIo8MONCmJoYeNy6c= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736266727; c=relaxed/simple; bh=DsDpwZtFplSEkT6noLhJ6QOEOEjv2MxU5xuN7dFjQQ8=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=TvP3RAog6aVDVPYCfcFQSHwqKr7YwBHZYjY+YzD7EAN3xWBvXOmJ99TMkXzCLure/OcN6QDWG4g3DR/8mWOSqfooSTGlwp0PG934fzqOzG1Nyirb4pOz8h481SdKkUqfkaxr9a3vXBMagGupBpfWkWs4EDBgRmYzBSx5qXjoaDY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=gourry.net; spf=pass smtp.mailfrom=gourry.net; dkim=pass (2048-bit key) header.d=gourry.net header.i=@gourry.net header.b=Fynw53ny; arc=none smtp.client-ip=209.85.219.49 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=gourry.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gourry.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gourry.net header.i=@gourry.net header.b="Fynw53ny" Received: by mail-qv1-f49.google.com with SMTP id 6a1803df08f44-6d900c27af7so126076726d6.2 for ; Tue, 07 Jan 2025 08:18:43 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gourry.net; s=google; t=1736266722; x=1736871522; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=FaxcSnz2S5smrIZV+cxVoc71rFoVpYYB2um+pmHtbYg=; b=Fynw53nyINO9Ss+e3FtZumBKqeEtJ4RJ5OA6SZ4A40L8pgLn+c/IO5qNnnNNkFzBVJ szUB3UvuUH2VfTbZ9dUKSRRf/hRm4dYj8bx/WswfJD1Xa5sJ+vknFdfr2YM90gwZdsDU h2E9kHewyMiMq4SQykAMvSb/tvGg0tBtOOmY/UOtQOKBpWCNjpcWVzxRvesRdha2m0Zf vwPlXtqgVKwOxhigRWIbF1BU+rvg0QN5tQolBvdJWMBIQtISJrjkIvBZvPfSYhpdlKVm i/KGlob8/Xs1WJ8X+Rur4Ag7WGH9ToOnNvS5rqxWOPZmUZtsUqBJXqCn/06YRgplnhjX yGXA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736266722; x=1736871522; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=FaxcSnz2S5smrIZV+cxVoc71rFoVpYYB2um+pmHtbYg=; b=Kn9UsprV+kdjwHXfD0HIqxJPrCbxr62G/Z0mE7LQmSBqSIZR2OJuplRd9hOAngVTij vHYfVBwwG9OHxboKSs2PdOQuZ8JLl1+Ddyhc8o2xc8L89d5XIZvW6IisgiKFSRicgkK3 tgzhFQcgGwMCFD8+695OJrCPTFRghsC6uOgCjfRmyFagHd0N8sI8QmRZJS8QcI4H0Qmn TQ595JulNsLy01zfri4sv0LimDXu4P3La6S4ZKPOeWnIz31SmAGE6MZnTVaxqQel0NuL jvqwq7QmrbL9uk7QPIILrW+BfILzFty2y+fN+ngudZ4cWoArbVA3ZQbEgIvKAT2wvpx2 EHqA== X-Forwarded-Encrypted: i=1; AJvYcCVA7l1/8ULnxrkkD/a4Nkil2cBlQ8QOeQLU1kcD/3D3GN4hzySDZQECpHFW9fmOQ+yPPx/pLKRDhjmzjaU=@vger.kernel.org X-Gm-Message-State: AOJu0YwhyCe/hyje0ZlQWtgRKgDZhINfCfpsKwgXkMpEG6s0NErYP92u 2KFaTB+/EkVFzKRo4403spZTHcZrBQgL8HkngEQ1pRKhnZKEurHXpGiU16fFAW4= X-Gm-Gg: ASbGnctmnef0xUG/HfCHx89ukyUvL3uax6H6lB1I5Wvz8pXmCmi9fo6U6qMjHCTp4uA HISPYHLXnp9VU3e0tXvUONNZ9NBmZ6R2PL4X5L/B9rzvfKUk5aUSxGNasjx1RyEjqBdCij8DJ+I WJUL/bNycHTv3uz62rSGHsAioovtuYM3LjQa4b2qzeln00l7mkhkzeNCuV38cqO83i40WJTulTy NvT3McWBn0NPFsqrRulNToOej2zcfAgLHmQUFK1yIUHIGcEtbdbgT9wMu7rqNE5HkyPuCj+UChQ rAjFMmAxtgJB79IolbfLgL938cnNY22YA4p8430= X-Google-Smtp-Source: AGHT+IGem7wzFy4INJNzPyNqGkX4WOpJLzsebBNHiJG6ibLSwau55oro1R8Xpz7jo2OdpjPGBr1LlA== X-Received: by 2002:ad4:4ee7:0:b0:6cb:f40c:b868 with SMTP id 6a1803df08f44-6dd233b7ec7mr968281096d6.46.1736266722319; Tue, 07 Jan 2025 08:18:42 -0800 (PST) Received: from gourry-fedora-PF4VCD3F (pool-173-79-56-208.washdc.fios.verizon.net. [173.79.56.208]) by smtp.gmail.com with ESMTPSA id 6a1803df08f44-6dd181d53ecsm182205516d6.114.2025.01.07.08.18.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 07 Jan 2025 08:18:41 -0800 (PST) Date: Tue, 7 Jan 2025 11:18:39 -0500 From: Gregory Price To: Robert Richter Cc: Alison Schofield , Vishal Verma , Ira Weiny , Dan Williams , Jonathan Cameron , Dave Jiang , Davidlohr Bueso , linux-cxl@vger.kernel.org, linux-kernel@vger.kernel.org, "Fabio M. De Francesco" , Terry Bowman Subject: Re: [PATCH v1 02/29] cxl/pci: Moving code in cxl_hdm_decode_init() Message-ID: References: <20250107141015.3367194-1-rrichter@amd.com> <20250107141015.3367194-3-rrichter@amd.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20250107141015.3367194-3-rrichter@amd.com> On Tue, Jan 07, 2025 at 03:09:48PM +0100, Robert Richter wrote: > Commit 3f9e07531778 ("cxl/pci: simplify the check of mem_enabled in > cxl_hdm_decode_init()") changed the code flow in this function. The > root port is determined before a check to leave the function. Since > the root port is not used by the check it can be moved to run the > check first. This improves code readability and avoids unnesessary > code execution. > > Signed-off-by: Robert Richter > --- > drivers/cxl/core/pci.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c > index 3e8d20f8955c..d206378c4cbc 100644 > --- a/drivers/cxl/core/pci.c > +++ b/drivers/cxl/core/pci.c > @@ -419,14 +419,6 @@ int cxl_hdm_decode_init(struct cxl_dev_state *cxlds, struct cxl_hdm *cxlhdm, > if (!hdm) > return -ENODEV; > > - root = to_cxl_port(port->dev.parent); > - while (!is_cxl_root(root) && is_cxl_port(root->dev.parent)) > - root = to_cxl_port(root->dev.parent); > - if (!is_cxl_root(root)) { > - dev_err(dev, "Failed to acquire root port for HDM enable\n"); > - return -ENODEV; > - } > - Can't say definitively, but my reading of the original ordering suggests the intent was to bail out of enabling anything if the cxl root cannot be found (which suggests much larger issues). This code flow allows the device to have its bits twiddled when the root cannot be found - is that what we want? > if (!info->mem_enabled) { > rc = devm_cxl_enable_hdm(&port->dev, cxlhdm); > if (rc) > @@ -435,6 +427,14 @@ int cxl_hdm_decode_init(struct cxl_dev_state *cxlds, struct cxl_hdm *cxlhdm, > return devm_cxl_enable_mem(&port->dev, cxlds); > } > > + root = to_cxl_port(port->dev.parent); > + while (!is_cxl_root(root) && is_cxl_port(root->dev.parent)) > + root = to_cxl_port(root->dev.parent); > + if (!is_cxl_root(root)) { > + dev_err(dev, "Failed to acquire root port for HDM enable\n"); > + return -ENODEV; > + } > + > for (i = 0, allowed = 0; i < info->ranges; i++) { > struct device *cxld_dev; > > -- > 2.39.5 >