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 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7E962C4332F for ; Thu, 14 Oct 2021 06:59:37 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 64908600CD for ; Thu, 14 Oct 2021 06:59:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229902AbhJNHBk (ORCPT ); Thu, 14 Oct 2021 03:01:40 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49096 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229502AbhJNHBj (ORCPT ); Thu, 14 Oct 2021 03:01:39 -0400 Received: from mail.marcansoft.com (marcansoft.com [IPv6:2a01:298:fe:f::2]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A95C6C061570; Wed, 13 Oct 2021 23:59:34 -0700 (PDT) Received: from [127.0.0.1] (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) (Authenticated sender: marcan@marcan.st) by mail.marcansoft.com (Postfix) with ESMTPSA id D151C41AC8; Thu, 14 Oct 2021 06:59:26 +0000 (UTC) To: Krzysztof Kozlowski , linux-arm-kernel@lists.infradead.org Cc: Alyssa Rosenzweig , Sven Peter , Marc Zyngier , Mark Kettenis , Michael Turquette , Stephen Boyd , Rob Herring , Viresh Kumar , Nishanth Menon , Catalin Marinas , "Rafael J. Wysocki" , Kevin Hilman , Ulf Hansson , linux-clk@vger.kernel.org, devicetree@vger.kernel.org, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org References: <20211011165707.138157-1-marcan@marcan.st> <20211011165707.138157-7-marcan@marcan.st> From: Hector Martin Subject: Re: [RFC PATCH 6/9] memory: apple: Add apple-mcc driver to manage MCC perf in Apple SoCs Message-ID: <2a6f14e5-fbc9-4b9a-9378-a4b5200bc3fb@marcan.st> Date: Thu, 14 Oct 2021 15:59:24 +0900 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: es-ES Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 12/10/2021 18.19, Krzysztof Kozlowski wrote: >> +// SPDX-License-Identifier: GPL-2.0-only OR MIT >> +/* >> + * Apple SoC MCC memory controller performance control driver >> + * >> + * Copyright The Asahi Linux Contributors > > Copyright date? We've gone over this one a few times already; most copyright dates quickly become outdated and meaningless :) See: https://www.linuxfoundation.org/blog/copyright-notices-in-open-source-software-projects/ >> +static int apple_mcc_probe(struct platform_device *pdev) >> +{ >> + struct device *dev = &pdev->dev; >> + struct device_node *node = dev->of_node; > > By convention mostly we call the variable "np". Ack, I'll change it for v2. >> + mcc->reg_base = devm_platform_ioremap_resource(pdev, 0); >> + if (IS_ERR(mcc->reg_base)) >> + return PTR_ERR(mcc->reg_base); >> + >> + if (of_property_read_u32(node, "apple,num-channels", &mcc->num_channels)) { > > Don't you have a limit of supported channels? It cannot be any uint32... Today, it's max 8. But if come Monday we find out Apple's new chips have 16 channels and otherwise the same register layout, I'd much rather not have to change the driver... >> + dev_err(dev, "missing apple,num-channels property\n"); > > Use almost everywhere dev_err_probe - less code and you get error msg > printed. Heh, I didn't know about that one. Thanks! >> + >> + dev_info(dev, "Apple MCC performance driver initialized\n"); > > Please skip it, or at least make it a dev_dbg, you don't print any > valuable information here. Ack, I'll remove this. >> +static struct platform_driver apple_mcc_driver = { >> + .probe = apple_mcc_probe, >> + .driver = { >> + .name = "apple-mcc", >> + .of_match_table = apple_mcc_of_match, >> + }, >> +}; > > module_platform_driver() goes here. Ack, will fix for v2. > >> + >> +MODULE_AUTHOR("Hector Martin "); >> +MODULE_DESCRIPTION("MCC memory controller performance tuning driver for Apple SoCs"); >> +MODULE_LICENSE("GPL v2"); > > I think this will be "Dual MIT/GPL", based on your SPDX. Ah, I didn't realize that was a valid option for MODULE_LICENSE. I guess anything containing "GPL" works with EXPORT_SYMBOL_GPL? Thanks for the review! -- Hector Martin (marcan@marcan.st) Public Key: https://mrcn.st/pub 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 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1C217C433EF for ; Thu, 14 Oct 2021 07:01:38 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id CCE8F600CD for ; Thu, 14 Oct 2021 07:01:37 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org CCE8F600CD Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=marcan.st Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Content-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:Subject: From:References:Cc:To:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=kB1KQh0yGkZSG4mF6QW++u/hIkLmPLL4epDNuH+0FLM=; b=fiLJXeVfTSqYfUC3HE4fXpf1uO +fC/6G7EWNMKdGIUPI2PqcA4AahrUsI1t654Nirp8F+DsFzKcYQzmbX0xHXa1ShYJNzSzw7S+4IwI X586CrYjAbep2klar06Sk9FIE4KBkX69j6MYQU9fkdINn6ErcmxDoaoMwnm03WWuVGd6TlawvBR7k vd0xLgGqmzPmR3XB9mNfkBuDhvTCI72OtoHtc+QktYJ17X1MId7l5Y/N2+bp4jwLMqxagffWdVHiu wh6fIZ+K5LQ9uiL5z7gvoXZFmFAKOZfV5sVHFFk0yk2yYzilmt4PvzNHSPWYG5pWjdDWZB09i3FjY WS2h70bw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mauiJ-001nPj-R7; Thu, 14 Oct 2021 06:59:39 +0000 Received: from marcansoft.com ([2a01:298:fe:f::2] helo=mail.marcansoft.com) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mauiF-001nO4-Gm for linux-arm-kernel@lists.infradead.org; Thu, 14 Oct 2021 06:59:37 +0000 Received: from [127.0.0.1] (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) (Authenticated sender: marcan@marcan.st) by mail.marcansoft.com (Postfix) with ESMTPSA id D151C41AC8; Thu, 14 Oct 2021 06:59:26 +0000 (UTC) To: Krzysztof Kozlowski , linux-arm-kernel@lists.infradead.org Cc: Alyssa Rosenzweig , Sven Peter , Marc Zyngier , Mark Kettenis , Michael Turquette , Stephen Boyd , Rob Herring , Viresh Kumar , Nishanth Menon , Catalin Marinas , "Rafael J. Wysocki" , Kevin Hilman , Ulf Hansson , linux-clk@vger.kernel.org, devicetree@vger.kernel.org, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org References: <20211011165707.138157-1-marcan@marcan.st> <20211011165707.138157-7-marcan@marcan.st> From: Hector Martin Subject: Re: [RFC PATCH 6/9] memory: apple: Add apple-mcc driver to manage MCC perf in Apple SoCs Message-ID: <2a6f14e5-fbc9-4b9a-9378-a4b5200bc3fb@marcan.st> Date: Thu, 14 Oct 2021 15:59:24 +0900 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: Content-Language: es-ES X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211013_235935_735081_60A74388 X-CRM114-Status: GOOD ( 20.97 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 12/10/2021 18.19, Krzysztof Kozlowski wrote: >> +// SPDX-License-Identifier: GPL-2.0-only OR MIT >> +/* >> + * Apple SoC MCC memory controller performance control driver >> + * >> + * Copyright The Asahi Linux Contributors > > Copyright date? We've gone over this one a few times already; most copyright dates quickly become outdated and meaningless :) See: https://www.linuxfoundation.org/blog/copyright-notices-in-open-source-software-projects/ >> +static int apple_mcc_probe(struct platform_device *pdev) >> +{ >> + struct device *dev = &pdev->dev; >> + struct device_node *node = dev->of_node; > > By convention mostly we call the variable "np". Ack, I'll change it for v2. >> + mcc->reg_base = devm_platform_ioremap_resource(pdev, 0); >> + if (IS_ERR(mcc->reg_base)) >> + return PTR_ERR(mcc->reg_base); >> + >> + if (of_property_read_u32(node, "apple,num-channels", &mcc->num_channels)) { > > Don't you have a limit of supported channels? It cannot be any uint32... Today, it's max 8. But if come Monday we find out Apple's new chips have 16 channels and otherwise the same register layout, I'd much rather not have to change the driver... >> + dev_err(dev, "missing apple,num-channels property\n"); > > Use almost everywhere dev_err_probe - less code and you get error msg > printed. Heh, I didn't know about that one. Thanks! >> + >> + dev_info(dev, "Apple MCC performance driver initialized\n"); > > Please skip it, or at least make it a dev_dbg, you don't print any > valuable information here. Ack, I'll remove this. >> +static struct platform_driver apple_mcc_driver = { >> + .probe = apple_mcc_probe, >> + .driver = { >> + .name = "apple-mcc", >> + .of_match_table = apple_mcc_of_match, >> + }, >> +}; > > module_platform_driver() goes here. Ack, will fix for v2. > >> + >> +MODULE_AUTHOR("Hector Martin "); >> +MODULE_DESCRIPTION("MCC memory controller performance tuning driver for Apple SoCs"); >> +MODULE_LICENSE("GPL v2"); > > I think this will be "Dual MIT/GPL", based on your SPDX. Ah, I didn't realize that was a valid option for MODULE_LICENSE. I guess anything containing "GPL" works with EXPORT_SYMBOL_GPL? Thanks for the review! -- Hector Martin (marcan@marcan.st) Public Key: https://mrcn.st/pub _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel