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 4FF3CC388F9 for ; Wed, 11 Nov 2020 22:43:51 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id F3CE22100A for ; Wed, 11 Nov 2020 22:43:49 +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="rTT49Ank" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727069AbgKKWns (ORCPT ); Wed, 11 Nov 2020 17:43:48 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53990 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726731AbgKKWln (ORCPT ); Wed, 11 Nov 2020 17:41:43 -0500 Received: from mail-ej1-x644.google.com (mail-ej1-x644.google.com [IPv6:2a00:1450:4864:20::644]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2D40EC0401C6 for ; Wed, 11 Nov 2020 14:41:04 -0800 (PST) Received: by mail-ej1-x644.google.com with SMTP id f23so4997444ejk.2 for ; Wed, 11 Nov 2020 14:41:04 -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=ALVcAvrDAuBZ3IPCrkjPwHAX8+DNa5pqM0afUjn5X6s=; b=rTT49AnkhNr487K9n5NKwYO9WMUE3MNY/UrxCB1TueO1d+BsQebe1bVAjto6CnRmk/ RDpD3FTxsnpbumIEQX0xW8WRxu+N8dbL7+UlwJuliNYW4tOrjIHEsijjBDMt5SiFkT7W qIC8M6dcNADqftbsDDTwdltE4fVgS48XJPqta990E5jyRjX44U9fYqfo3bpsnhM2g0J7 eynDKGo1p6EV/6yF+gGpA1ZpwHDZOUAKO9aCvylNPSLobnovjeE+6vOb7pc4GrNqOJdL VpWgI5+VW1m4VWGcBZwG83xjGz4jrrCYOWJBRZrRhJgpclGLGZV4RXEKWrc44rurwU4I eK6A== 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=ALVcAvrDAuBZ3IPCrkjPwHAX8+DNa5pqM0afUjn5X6s=; b=KfSse7r3ScilV1P4F8mtx5NgjRry4+9xuelCVsmdtgi9JjsHAX+rPCK8SkiHtlyBFj jIJ6Q1PTztYHZ52zmdjxqHaE4W+PagioUf0oJ4YkUVWiBNlKED0CcdoczAGM2X5QLuQo Mb+ycIpVlNEYIDKHY1MVbsmj92JFhQwJGwnb/71IjtUnQJ0hwfe+PlQLEa93EohvqmlW Gtt37NWAVoxZYiiXMsY0nJ8/je7HLCMrQjSolTgl9X/nwmOnas7JPeTtn2c0YsSiZve1 e5S5WA+1UgkKsd/lNf9ze51UX5re7VRTo8AUf5j2rRBBjAMeWBV/Dr3v4hy/xcqyRZyn 8U+A== X-Gm-Message-State: AOAM532XCJw4pGOBiw7NnY9r9QClgNN8Lp5leTjC7UC3on7kZWT9ROv6 PzTAyAqoFIxtNoCQV2IDDjjDkiZnUW9n24P3AkxomQ== X-Google-Smtp-Source: ABdhPJxZkk2CPz2+JgnqesQh/6Q2toSIuomPC2gF0BhYxxkrw/FgNoCUwNXohjnwegJoJqNJZnPv//TQH98s+vGiwVw= X-Received: by 2002:a17:906:ad8e:: with SMTP id la14mr25980011ejb.264.1605134462866; Wed, 11 Nov 2020 14:41:02 -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> <4a8b5a64-7ba0-a275-744f-6642f98e2213@infradead.org> In-Reply-To: <4a8b5a64-7ba0-a275-744f-6642f98e2213@infradead.org> From: Dan Williams Date: Wed, 11 Nov 2020 14:40:52 -0800 Message-ID: Subject: Re: [RFC PATCH 3/9] cxl/mem: Add a driver for the type-3 mailbox To: Randy Dunlap Cc: Christoph Hellwig , 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 Wed, Nov 11, 2020 at 1:42 PM Randy Dunlap wrote: > > On 11/11/20 9:17 AM, Dan Williams wrote: > > 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. > > What we as developers put into mainline kernel Kconfig files has nothing > to do with what distros use in their distro config files. > Or at least it shouldn't. Maybe your experience has been different. I agree with that sentiment, but walk it back through the requirement I mentioned above... *if* we want a top-level CXL option (default n) that goes and enables many CXL sub-options the default for those sub-options is something that needs to be listed in the Kconfig. 'm' is more flexible than 'y', so if a user wants CXL at all, and doesn't care about how, I'd prefer it's 'm' rather than 'y'. I have had to go submit distro config fixes when Kconfig defaulted to 'y' when 'm' was available, and the reasoning for why it was 'y' was "oh, that was the Kconfig default when I flipped this other option". > >>> +// 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. > > > > Hm, it's not in coding-style AFAICT but Linus has OK-ed C99 style comments: > http://lkml.iu.edu/hypermail/linux/kernel/1607.1/00627.html > > > >>> +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. > > Sure, MAINTAINERS is fine, but the MODULE_AUTHOR() above provides > no useful information. > Even saying (made up) linux-devel@linux.intel.com would be slightly better, > but some kind of contact info would be great. Otherwise just delete that line. True, if the goal is to allow random end users to email support questions about this module I'd rather not put my email there. Instead, if it's someone that has kernel development questions then they should be able to use MAINTAINERS for that contact.