From: Hector Martin <marcan@marcan.st> To: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>, linux-arm-kernel@lists.infradead.org Cc: Alyssa Rosenzweig <alyssa@rosenzweig.io>, Sven Peter <sven@svenpeter.dev>, Marc Zyngier <maz@kernel.org>, Mark Kettenis <mark.kettenis@xs4all.nl>, Michael Turquette <mturquette@baylibre.com>, Stephen Boyd <sboyd@kernel.org>, Rob Herring <robh+dt@kernel.org>, Viresh Kumar <vireshk@kernel.org>, Nishanth Menon <nm@ti.com>, Catalin Marinas <catalin.marinas@arm.com>, "Rafael J. Wysocki" <rafael@kernel.org>, Kevin Hilman <khilman@kernel.org>, Ulf Hansson <ulf.hansson@linaro.org>, linux-clk@vger.kernel.org, devicetree@vger.kernel.org, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [RFC PATCH 6/9] memory: apple: Add apple-mcc driver to manage MCC perf in Apple SoCs Date: Thu, 14 Oct 2021 15:59:24 +0900 [thread overview] Message-ID: <2a6f14e5-fbc9-4b9a-9378-a4b5200bc3fb@marcan.st> (raw) In-Reply-To: <a9f6898d-bd76-b94e-52fc-98e9da1a04bd@canonical.com> 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 <marcan@marcan.st>"); >> +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
WARNING: multiple messages have this Message-ID (diff)
From: Hector Martin <marcan@marcan.st> To: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>, linux-arm-kernel@lists.infradead.org Cc: Alyssa Rosenzweig <alyssa@rosenzweig.io>, Sven Peter <sven@svenpeter.dev>, Marc Zyngier <maz@kernel.org>, Mark Kettenis <mark.kettenis@xs4all.nl>, Michael Turquette <mturquette@baylibre.com>, Stephen Boyd <sboyd@kernel.org>, Rob Herring <robh+dt@kernel.org>, Viresh Kumar <vireshk@kernel.org>, Nishanth Menon <nm@ti.com>, Catalin Marinas <catalin.marinas@arm.com>, "Rafael J. Wysocki" <rafael@kernel.org>, Kevin Hilman <khilman@kernel.org>, Ulf Hansson <ulf.hansson@linaro.org>, linux-clk@vger.kernel.org, devicetree@vger.kernel.org, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [RFC PATCH 6/9] memory: apple: Add apple-mcc driver to manage MCC perf in Apple SoCs Date: Thu, 14 Oct 2021 15:59:24 +0900 [thread overview] Message-ID: <2a6f14e5-fbc9-4b9a-9378-a4b5200bc3fb@marcan.st> (raw) In-Reply-To: <a9f6898d-bd76-b94e-52fc-98e9da1a04bd@canonical.com> 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 <marcan@marcan.st>"); >> +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
next prev parent reply other threads:[~2021-10-14 6:59 UTC|newest] Thread overview: 84+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-10-11 16:56 [RFC PATCH 0/9] Apple SoC CPU P-state switching Hector Martin 2021-10-11 16:56 ` Hector Martin 2021-10-11 16:56 ` [RFC PATCH 1/9] MAINTAINERS: apple: Add apple-mcc and clk-apple-cluster paths Hector Martin 2021-10-11 16:56 ` Hector Martin 2021-10-11 16:57 ` [RFC PATCH 2/9] dt-bindings: memory-controller: Add apple,mcc binding Hector Martin 2021-10-11 16:57 ` [RFC PATCH 2/9] dt-bindings: memory-controller: Add apple, mcc binding Hector Martin 2021-10-12 8:48 ` [RFC PATCH 2/9] dt-bindings: memory-controller: Add apple,mcc binding Krzysztof Kozlowski 2021-10-12 8:48 ` Krzysztof Kozlowski 2021-10-19 22:43 ` Rob Herring 2021-10-19 22:43 ` Rob Herring 2021-10-11 16:57 ` [RFC PATCH 3/9] dt-bindings: clock: Add apple,cluster-clk binding Hector Martin 2021-10-11 16:57 ` Hector Martin 2021-10-12 8:51 ` Krzysztof Kozlowski 2021-10-12 8:51 ` [RFC PATCH 3/9] dt-bindings: clock: Add apple, cluster-clk binding Krzysztof Kozlowski 2021-10-12 9:35 ` [RFC PATCH 3/9] dt-bindings: clock: Add apple,cluster-clk binding Viresh Kumar 2021-10-12 9:35 ` [RFC PATCH 3/9] dt-bindings: clock: Add apple, cluster-clk binding Viresh Kumar [not found] ` <D0DE08FE-562E-4A48-BCA0-9094DAFCA564@marcan.st> [not found] ` <20211012094302.3cownyzr4phxwifs@vireshk-i7> [not found] ` <64584F8C-D49F-41B5-9658-CF8A25186E67@marcan.st> [not found] ` <20211012095735.mhh2lzu52ohtotl6@vireshk-i7> 2021-10-12 13:48 ` [RFC PATCH 3/9] dt-bindings: clock: Add apple,cluster-clk binding Hector Martin 2021-10-12 13:48 ` [RFC PATCH 3/9] dt-bindings: clock: Add apple, cluster-clk binding Hector Martin 2021-10-14 21:47 ` Stephen Boyd 2021-10-11 16:57 ` [RFC PATCH 4/9] opp: core: Don't warn if required OPP device does not exist Hector Martin 2021-10-11 16:57 ` Hector Martin 2021-10-12 3:21 ` Viresh Kumar 2021-10-12 3:21 ` Viresh Kumar 2021-10-12 5:34 ` Hector Martin 2021-10-12 5:34 ` Hector Martin 2021-10-12 5:51 ` Viresh Kumar 2021-10-12 5:51 ` Viresh Kumar 2021-10-12 5:57 ` Hector Martin 2021-10-12 5:57 ` Hector Martin 2021-10-12 9:26 ` Viresh Kumar 2021-10-12 9:26 ` Viresh Kumar 2021-10-12 9:31 ` Hector Martin "marcan" 2021-10-12 9:31 ` Hector Martin "marcan" 2021-10-12 9:32 ` Viresh Kumar 2021-10-12 9:32 ` Viresh Kumar 2021-10-14 6:52 ` Hector Martin 2021-10-14 6:52 ` Hector Martin 2021-10-14 6:56 ` Viresh Kumar 2021-10-14 6:56 ` Viresh Kumar 2021-10-14 7:03 ` Hector Martin 2021-10-14 7:03 ` Hector Martin 2021-10-14 7:22 ` Viresh Kumar 2021-10-14 7:22 ` Viresh Kumar 2021-10-14 7:23 ` Hector Martin 2021-10-14 7:23 ` Hector Martin 2021-10-14 11:08 ` Ulf Hansson 2021-10-14 11:08 ` Ulf Hansson 2021-10-14 9:55 ` Ulf Hansson 2021-10-14 9:55 ` Ulf Hansson 2021-10-14 11:43 ` Hector Martin 2021-10-14 11:43 ` Hector Martin 2021-10-14 12:55 ` Ulf Hansson 2021-10-14 12:55 ` Ulf Hansson 2021-10-14 17:02 ` Hector Martin 2021-10-14 17:02 ` Hector Martin 2021-10-15 11:26 ` Ulf Hansson 2021-10-15 11:26 ` Ulf Hansson 2021-10-11 16:57 ` [RFC PATCH 5/9] PM: domains: Add of_genpd_add_provider_simple_noclk() Hector Martin 2021-10-11 16:57 ` Hector Martin 2021-10-11 16:57 ` [RFC PATCH 6/9] memory: apple: Add apple-mcc driver to manage MCC perf in Apple SoCs Hector Martin 2021-10-11 16:57 ` Hector Martin 2021-10-12 7:24 ` kernel test robot 2021-10-12 9:19 ` Krzysztof Kozlowski 2021-10-12 9:19 ` Krzysztof Kozlowski 2021-10-14 6:59 ` Hector Martin [this message] 2021-10-14 6:59 ` Hector Martin 2021-10-14 7:36 ` Krzysztof Kozlowski 2021-10-14 7:36 ` Krzysztof Kozlowski 2021-10-14 7:52 ` Hector Martin 2021-10-14 7:52 ` Hector Martin 2021-10-14 8:04 ` Krzysztof Kozlowski 2021-10-14 8:04 ` Krzysztof Kozlowski 2021-10-14 8:31 ` Hector Martin 2021-10-14 8:31 ` Hector Martin 2021-10-11 16:57 ` [RFC PATCH 7/9] clk: apple: Add clk-apple-cluster driver to manage CPU p-states Hector Martin 2021-10-11 16:57 ` Hector Martin 2021-10-13 3:45 ` kernel test robot 2021-10-14 22:07 ` Stephen Boyd 2021-10-17 9:16 ` Hector Martin 2021-10-17 9:16 ` Hector Martin 2021-10-11 16:57 ` [RFC PATCH 8/9] arm64: apple: Select MEMORY and APPLE_MCC Hector Martin 2021-10-11 16:57 ` Hector Martin 2021-10-11 16:57 ` [RFC PATCH 9/9] arm64: apple: Add CPU frequency scaling support for t8103 Hector Martin 2021-10-11 16:57 ` Hector Martin
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=2a6f14e5-fbc9-4b9a-9378-a4b5200bc3fb@marcan.st \ --to=marcan@marcan.st \ --cc=alyssa@rosenzweig.io \ --cc=catalin.marinas@arm.com \ --cc=devicetree@vger.kernel.org \ --cc=khilman@kernel.org \ --cc=krzysztof.kozlowski@canonical.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-clk@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pm@vger.kernel.org \ --cc=mark.kettenis@xs4all.nl \ --cc=maz@kernel.org \ --cc=mturquette@baylibre.com \ --cc=nm@ti.com \ --cc=rafael@kernel.org \ --cc=robh+dt@kernel.org \ --cc=sboyd@kernel.org \ --cc=sven@svenpeter.dev \ --cc=ulf.hansson@linaro.org \ --cc=vireshk@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.