Received: by 2002:a05:6a10:16a7:0:0:0:0 with SMTP id gp39csp145124pxb; Tue, 10 Nov 2020 23:11:58 -0800 (PST) X-Google-Smtp-Source: ABdhPJxoxlmn2hFFuTY21aKy+oX2KuEHw6ztX529oEnpSnR8+4QuvDXJGmShKAdMS4tdhm//gKn5 X-Received: by 2002:a05:6402:181a:: with SMTP id g26mr3667550edy.8.1605078718431; Tue, 10 Nov 2020 23:11:58 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1605078718; cv=none; d=google.com; s=arc-20160816; b=zHV0gZATAqcjSY6dDy1tHuvpw7AnGDEqifgtYIqWnhRlVeKt08LFY0KTte/aTV5cka tWQEyvYynM2/72TT99O8FxWmNq2SKSuF37fL8r/VZibbLmgo4rP6Cat4fwxaTUJYdalu 90fY2MOijq42OYbsJVgZyt0PTO4RGzE5Wp77+Qmr1aaVPW2FZvDcFri+MuNj10cEGFAs +cmqrChizPJaPE3lydl32XGzUUnc0vyCvKcZhLFEPq7qQBIZ69iVHpFXfovuQkRTLL7F ao592+yRcGUKhQr2NEtryuCu0oQX6Z17rBs6t3DuDgtgwUxPi5lCS/eqk1HFfKyuGp1E K7Mw== 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:dkim-signature; bh=qY6GsokNhT+EXzs5nQrwOVhDRZ4DEpog47vVj/s5f74=; b=bbWUrT2UkYat6hXR+QOPCb0WWJnFQ91vG8/5VbihuP+48l22ZYBawnoMzAuYR0NmDK 7iNF/dMamhtNrZkYmAE29KUv0NC8+PQnBFGYsFawmj4pPNpBhZZ4pAYpR6fogWb1LThX 5NSXdBTRAnlQxh9OxiPOvYIjbtjqd6yqkdWgbOtN/+J7tu9geMdDKA563j9BfAIiwjjy kBtbGiXYmjAS5cn2s15g1by+PcoduQCuybaduGOqrnF71nA1uyl151Pb9F/cbjczboyR nDub8qZZMVnxxE0iJuK0ciCOQm14yuIM+f4cVYRby88v905rYOUyW/fhW1wfjrravsT5 +A3A== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@infradead.org header.s=casper.20170209 header.b=ekNfnfmY; 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 Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id x91si860064edc.41.2020.11.10.23.11.34; Tue, 10 Nov 2020 23:11:58 -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=@infradead.org header.s=casper.20170209 header.b=ekNfnfmY; 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 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725916AbgKKHKJ (ORCPT + 99 others); Wed, 11 Nov 2020 02:10:09 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48450 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725828AbgKKHKJ (ORCPT ); Wed, 11 Nov 2020 02:10:09 -0500 Received: from casper.infradead.org (casper.infradead.org [IPv6:2001:8b0:10b:1236::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B8CC8C0613D1; Tue, 10 Nov 2020 23:10:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=qY6GsokNhT+EXzs5nQrwOVhDRZ4DEpog47vVj/s5f74=; b=ekNfnfmYL+you/M/HXdp4fwN1D E/jGSjNgqZgHOEhdrU6IBbZLY/KVbFadnyRB8Vs8JX9Hki9X87O418pPj4oSlVxedqp6Yv0IA2eyJ aDyrDVbzHRE6w1cI9Uydxiy74ax+uznsyudXBZXe9omCYy3yW/tnL6kKl5VzJZNy99+0PROUzaZym eNiO5Ufo58CFh2SZ/nXhnAPIZq3nYvKm/Owbt1RofmB8oQ45odlLITxJBMHoxUsUtsqLXpI9GxsgH Vkzb3EiJZx0VxUcHNkF5o9r9uFLDbKUK9uscus+dMNxeJ3yQ4OuAIdrejzHo3Qw6YYLxYFqMDR5/5 vWYOgksQ==; Received: from hch by casper.infradead.org with local (Exim 4.92.3 #3 (Red Hat Linux)) id 1kckGc-0002iV-7K; Wed, 11 Nov 2020 07:10:06 +0000 Date: Wed, 11 Nov 2020 07:10:06 +0000 From: Christoph Hellwig To: Ben Widawsky Cc: linux-cxl@vger.kernel.org, linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org, linux-acpi@vger.kernel.org, Dan Williams , Ira Weiny , Vishal Verma , "Kelley, Sean V" , Bjorn Helgaas , "Rafael J . Wysocki" Subject: Re: [RFC PATCH 1/9] cxl/acpi: Add an acpi_cxl module for the CXL interconnect Message-ID: <20201111071006.GB7829@infradead.org> References: <20201111054356.793390-1-ben.widawsky@intel.com> <20201111054356.793390-2-ben.widawsky@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20201111054356.793390-2-ben.widawsky@intel.com> X-SRS-Rewrite: SMTP reverse-path rewritten from by casper.infradead.org. See http://www.infradead.org/rpr.html Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Nov 10, 2020 at 09:43:48PM -0800, Ben Widawsky wrote: > +menuconfig CXL_BUS > + tristate "CXL (Compute Express Link) Devices Support" > + help > + CXL is a bus that is electrically compatible with PCI-E, but layers > + three protocols on that signalling (CXL.io, CXL.cache, and CXL.mem). The > + CXL.cache protocol allows devices to hold cachelines locally, the > + CXL.mem protocol allows devices to be fully coherent memory targets, the > + CXL.io protocol is equivalent to PCI-E. Say 'y' to enable support for > + the configuration and management of devices supporting these protocols. > + Please fix the overly long lines. > +static void acpi_cxl_desc_init(struct acpi_cxl_desc *acpi_desc, struct device *dev) Another overly long line. > +{ > + dev_set_drvdata(dev, acpi_desc); > + acpi_desc->dev = dev; > +} But this helper seems pretty pointless to start with. > +static int acpi_cxl_remove(struct acpi_device *adev) > +{ > + return 0; > +} The emptry remove callback is not needed. > +/* > + * If/when CXL support is defined by other platform firmware the kernel > + * will need a mechanism to select between the platform specific version > + * of this routine, until then, hard-code ACPI assumptions > + */ > +int cxl_bus_prepared(struct pci_dev *pdev) > +{ > + struct acpi_device *adev; > + struct pci_dev *root_port; > + struct device *root; > + > + root_port = pcie_find_root_port(pdev); > + if (!root_port) > + return -ENXIO; > + > + root = root_port->dev.parent; > + if (!root) > + return -ENXIO; > + > + adev = ACPI_COMPANION(root); > + if (!adev) > + return -ENXIO; > + > + /* TODO: OSC enabling */ > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(cxl_bus_prepared); What is the point of this function? I doesn't realy do anything, not even a CXL specific check. > > +/******************************************************************************* > + * > + * CEDT - CXL Early Discovery Table (ACPI 6.4) > + * Version 1 > + * > + ******************************************************************************/ > + Pleae use the normal Linux comment style. > +#define ACPI_CEDT_CHBS_VERSION_CXL11 (0) > +#define ACPI_CEDT_CHBS_VERSION_CXL20 (1) > + > +/* Values for length field above */ > + > +#define ACPI_CEDT_CHBS_LENGTH_CXL11 (0x2000) > +#define ACPI_CEDT_CHBS_LENGTH_CXL20 (0x10000) No need for the braces.