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=-3.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=no 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 4515FC5517A for ; Wed, 11 Nov 2020 17:17:52 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C51CF206CA for ; Wed, 11 Nov 2020 17:17:51 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=intel-com.20150623.gappssmtp.com header.i=@intel-com.20150623.gappssmtp.com header.b="1DH8VH1J" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727045AbgKKRRv (ORCPT ); Wed, 11 Nov 2020 12:17:51 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60700 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726397AbgKKRRu (ORCPT ); Wed, 11 Nov 2020 12:17:50 -0500 Received: from mail-ed1-x542.google.com (mail-ed1-x542.google.com [IPv6:2a00:1450:4864:20::542]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 204B5C0617A6 for ; Wed, 11 Nov 2020 09:17:50 -0800 (PST) Received: by mail-ed1-x542.google.com with SMTP id e18so3112317edy.6 for ; Wed, 11 Nov 2020 09:17:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=eQoq2RQCuafra2/12uiljwu+rCNgXWp3T6t2+sz8FTY=; b=1DH8VH1JKBZbEDlGbtroiVXOVC0/URrnHQ4E8/tzT86j+/9Tva1H6wuleBdwke7NC0 OtOJ5SRmQEMcfqXHix2vJ5Kn8HKTL1/oLfya12FbOsO3FsuLYwnjILQiqYj0mmVqQmXe Cuzt9YWBUz/3UdIPnqCEFjRKthmWHf8atJVS3KK4AVkpnTPVBpBpbAZHnG8I43hcBvXW Dk0rimoptgAKqXt+ANHhaSihWslTP3l33/U4JZ+UD25Y7G9behZfPGt1U9buudqH9QV0 03q5re0g52eZpMu1pz1uDdM8rCL00SnesUh0Gsb1iRIs7nPiQag7jBP9pfO1pyFr2IEo Ll0A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=eQoq2RQCuafra2/12uiljwu+rCNgXWp3T6t2+sz8FTY=; b=S5tka1K1wMo2g2ePBnUxidlaZKhclsFg1M3SYJhO2kX0omUzS98AUu6rz4BQjPCh7L Z4COhqmSsBveCmCep8+UDGpahYEVwXkTROFyduBoT/Of+78Uxl3gKwtWDeb5CPZK6kxy 9LZtmlZA6QA9rg2331xUyxzVZB31dzqpnW6r5dvhlVgsVlKIeE/aEkNhvOzq5O0/1PGY uKBq7XN+hSmsh6yZ9J26VCnZCi9CtlCtQi+SZSv9RPiSEf/sb6nSI3brNqA6g4sY7ANI 4bDzBII+QRQfNf4/dauLcowUGf8ni8PbkU4nnibue/NWhpQ3DoLNAJrgjbVLStDH4HYD UNaA== X-Gm-Message-State: AOAM530OvlBg9jEnJJ0QRPWshxpdNhWPo/Dv9ecUusjwIQ9M1J+R4R05 lr4vTOfhJu7vF8Uur+c1EbFuz/PG/X08mg8bxEtrsQ== X-Google-Smtp-Source: ABdhPJwTwbtangRhABKsob3Xr0CrmoBXrd71z27l2ZDvApiVBFS1mPJU50IdPt5HUcdZ+QRQ7ssj77pEMJ8fTBox2mA= X-Received: by 2002:aa7:ca44:: with SMTP id j4mr511437edt.354.1605115068596; Wed, 11 Nov 2020 09:17:48 -0800 (PST) MIME-Version: 1.0 References: <20201111054356.793390-1-ben.widawsky@intel.com> <20201111054356.793390-4-ben.widawsky@intel.com> <20201111071231.GC7829@infradead.org> In-Reply-To: <20201111071231.GC7829@infradead.org> From: Dan Williams Date: Wed, 11 Nov 2020 09:17:37 -0800 Message-ID: Subject: Re: [RFC PATCH 3/9] cxl/mem: Add a driver for the type-3 mailbox To: Christoph Hellwig Cc: Ben Widawsky , linux-cxl@vger.kernel.org, Linux Kernel Mailing List , Linux PCI , Linux ACPI , Ira Weiny , Vishal Verma , "Kelley, Sean V" , Bjorn Helgaas , "Rafael J . Wysocki" Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Nov 10, 2020 at 11:12 PM Christoph Hellwig wrote: > > On Tue, Nov 10, 2020 at 09:43:50PM -0800, Ben Widawsky wrote: > > +config CXL_MEM > > + tristate "CXL.mem Device Support" > > + depends on PCI && CXL_BUS_PROVIDER != n > > depend on PCI && CXL_BUS_PROVIDER > > > + default m if CXL_BUS_PROVIDER > > Please don't set weird defaults for new code. Especially not default > to module crap like this. This goes back to what people like Dave C. asked for LIBNVDIMM / DAX, a way to blanket turn on a subsystem without needing to go hunt down individual configs. All of CXL is "default n", but if someone turns on a piece of it they get all of it by default. The user can then opt-out on pieces after that first opt-in. If there's a better way to turn on suggested configs I'm open to switch to that style. As for the "default m" I was worried that it would be "default y" without the specificity, but I did not test that... will check. There have been times when I wished that distros defaulted bleeding edge new enabling to 'm' and putting that default in the Kconfig maybe saves me from needing to file individual config changes to distros after the fact. > > > +// Copyright(c) 2020 Intel Corporation. All rights reserved. > > Please don't use '//' for anything but the SPDX header. Ok, I find // following by /* */ a bit ugly, but I don't care enough to fight. > > > + > > + pci_read_config_word(pdev, pos + PCI_DVSEC_VENDOR_OFFSET, &vendor); > > + pci_read_config_word(pdev, pos + PCI_DVSEC_ID_OFFSET, &id); > > + if (vendor == PCI_DVSEC_VENDOR_CXL && dvsec == id) > > + return pos; > > + > > + pos = pci_find_next_ext_capability(pdev, pos, PCI_EXT_CAP_ID_DVSEC); > > Overly long lines again. I thought 100 is the new 80 these days? > > +static void cxl_mem_remove(struct pci_dev *pdev) > > +{ > > +} > > No need for the empty remove callback. True, will fix. > > > +MODULE_AUTHOR("Intel Corporation"); > > A module author is not a company. At least I don't have a copyright assignment clause, I don't agree with the vanity of listing multiple people here especially when MAINTAINERS has the contact info, and I don't want to maintain a list as people do drive-by contributions and we need to figure out at what level of contribution mandates a new MODULE_AUTHOR line. Now, that said I would be ok to duplicate the MAINTAINERS as MODULE_AUTHOR lines, but I otherwise expect MAINTAINERS is the central source for module contact info.