From patchwork Fri Feb 16 17:35:53 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Lezcano X-Patchwork-Id: 10225223 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id F2347602CB for ; Fri, 16 Feb 2018 17:36:20 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E154129629 for ; Fri, 16 Feb 2018 17:36:20 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id D59332962C; Fri, 16 Feb 2018 17:36:20 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID autolearn=ham version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 25A8129629 for ; Fri, 16 Feb 2018 17:36:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date: Message-ID:From:References:To:Subject:Reply-To:Content-ID:Content-Description :Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=kh4erujE7E9tZJ7J01qMTV+nt6CGvKoFHI0DU8kJGWA=; b=COqYIHN0xeQX3d i1T4HabUtQBUtTq9WMQbbgNAV5sOamiIkd2qaNeh1OpJ8US4vW3fhk+l0xtLyUaS9w0NsM2PJLpM9 mhJSvfvDbHXyASexzO+WMr03kWBnZBEx5pCEoD/gTECGIr+1NOYGrdZcPvMUKk926DQxNZiZ82iX6 SIGUJL5iZqxgsPJ7bzzPQJzehvo6JxaN3Qo25wiIjHXwYD95HcDgR2CI4jsMXLB7uVal685GXDwVS lOnuQMmFJK1RF6/y7BgvY2ZCSs0n5DlMsNOO+H+wuEqOt1walkto1WwiS7NYnbR5if1SAuBIZ2ogU I03qCssO5VsGAFzF4Ovg==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.89 #1 (Red Hat Linux)) id 1emjvi-0007Ap-GZ; Fri, 16 Feb 2018 17:36:14 +0000 Received: from mail-wr0-x241.google.com ([2a00:1450:400c:c0c::241]) by bombadil.infradead.org with esmtps (Exim 4.89 #1 (Red Hat Linux)) id 1emjvb-000766-Pr for linux-arm-kernel@lists.infradead.org; Fri, 16 Feb 2018 17:36:10 +0000 Received: by mail-wr0-x241.google.com with SMTP id n7so3669637wrn.5 for ; Fri, 16 Feb 2018 09:35:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=TIUORmjybSg8lgJF/L3sVZSZfXwIh5D8/tlTJvtxnJU=; b=Tufeo2JCkIKgp2BruLlW9Hm0JKgkieaxuYKLcBJ9V4c7sht9Zb71Or3Cj2fIiZlfgI afokq6xY3tnOetK5MGNMnBu1oQ4jjZ5BNws+ZHG8Oz0gUonlt3IMxHBjF+TJ5wAI59mT Wi+qe7pYGRLfAT4RZ0W7y/E+dkedCZ/G0OWaQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=TIUORmjybSg8lgJF/L3sVZSZfXwIh5D8/tlTJvtxnJU=; b=bkhSOP3IhswUZ4N/ujKzj5/g1fnOPTXl7PV7hfgZk5g9JsIwR+LIofKjsTPQUJ6YHV 2Z07bLLDSPD3qQcgD5PWYcvEV4b+ouQ0/06qFgM5Wuzt/VvX8RykeOIvyCnkKrt2reAc Z2GQ3ed8YXdZymX+pVvXV9GyAKboCduxUSfi3hfFCwXnnWMxWrBQNNWXds5AIydtZ9cX H21Qnqe4aiLhYr0/2PH+9lYy8JPIlwOpXc/w0XubR7ojJ0DpCZMcDqs5puWytoKZjrf7 aLaosNPEcVw+PR2lqSaIBO+7+siSDZJqUMWOGuJJH5Hi0MQ7GGPlyHZQB+gy6irKu4R4 YiXQ== X-Gm-Message-State: APf1xPAEnZDz7TRfL3vmhyy4Wdpi+NKpOCW1PxKUbnLOP5uLPJyeh4BZ ynlBzCQKmNHAYU8Ccd2Keo27lQ== X-Google-Smtp-Source: AH8x2270M++c+NVmDZug+JRw8xDg2+0MhG3cbcejGJsvT9nUVSy00E8JfWe+15Vy0oS7ftUM5Ihr9Q== X-Received: by 10.223.167.72 with SMTP id e8mr6900092wrd.138.1518802555249; Fri, 16 Feb 2018 09:35:55 -0800 (PST) Received: from ?IPv6:2a01:e35:879a:6cd0:3e97:eff:fe5b:1402? ([2a01:e35:879a:6cd0:3e97:eff:fe5b:1402]) by smtp.googlemail.com with ESMTPSA id m6sm3323930wrb.78.2018.02.16.09.35.53 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 16 Feb 2018 09:35:54 -0800 (PST) Subject: Re: [PATCH] ARM64: Kconfig: Fix the missing hi655x common clk To: Arnd Bergmann References: <1496686434-13181-1-git-send-email-daniel.lezcano@linaro.org> <20170609154652.GG2244@mai> <20170612093853.GB2261@mai> From: Daniel Lezcano Message-ID: Date: Fri, 16 Feb 2018 18:35:53 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20180216_093607_884764_834CAA19 X-CRM114-Status: GOOD ( 30.03 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Ulf Hansson , Catalin Marinas , Will Deacon , open list , Wei Xu , John Stultz , Olof Johansson , "moderated list:ARM64 PORT \(AARCH64 ARCHITECTURE\)" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP On 12/06/2017 23:12, Arnd Bergmann wrote: > On Mon, Jun 12, 2017 at 11:38 AM, Daniel Lezcano > wrote: >> On Fri, Jun 09, 2017 at 10:48:13PM +0200, Arnd Bergmann wrote: >>> On Fri, Jun 9, 2017 at 10:15 PM, John Stultz wrote: >>>> On Fri, Jun 9, 2017 at 1:06 PM, Arnd Bergmann wrote: >>>>> On Fri, Jun 9, 2017 at 5:46 PM, Daniel Lezcano >>>>> wrote: >>>>> >>>>> Yes, but I'm not sure this is the right patch either. We tend to not >>>>> use 'select' for user-visible drivers, and most hisilicon platforms >>>>> won't need this driver. >>>>> >>>>> I think it would be more consistent to add this to the defconfig >>>>> and regard it as a user error when the driver is disabled on a >>>>> machine that needs it. >>>> >>>> Maybe the select is not exactly in the right place, but I don't really >>>> feel like a pmic on an SoC is a "user-visible driver". I deal with the >>>> board often and when the new dependency was made on the clk, I would >>>> have never have found it on my own w/o Ulf and Daniel pointing out >>>> what I needed to enable. >>> >>> What I meant is that the Kconfig option is user-visible. On a very high >>> level, this is a result of arch/arm64/Kconfig.platforms listing only >>> very broad categories of SoCs, in many cases only the manufacturers >>> of very different chip families, which then control the visibility of the >>> individual Kconfig items for things like pinctrl or clk. >>> >>> I now see that MFD_HI655X_PMIC is the top-level driver that you >>> have to select before enabling COMMON_CLK_HI655X, so the >>> patch is actually broken unless it actually selects both. >>> >>> How about simply adding a 'default MFD_HI655X_PMIC' to >>> COMMON_CLK_HI655X to enable it unless it is explicitly >>> turned off? >> >> Actually, I share John's opinion. >> >> Ideally when we choose a platform, all the relevants devices configuration >> options should be selected automatically from a single topmost node of a tree >> (platform selection) to all the nodes corresponding to the devices, leaving the >> user to select one simple option without knowledge of the SoC hardware >> internals. >> >> If the user is expert in the platform and knows exactly what he does, then he >> can select an _EXPERT_ like option and be able to disable some drivers. >> >> It is how I tend to write the Kconfig options, so the 'default MFD_HI655X_PMIC' >> is confusing for me. Wouldn't make sense to select COMMON_CLK_HI655X when >> MFD_HI655X_PMIC is enabled? > > I don't think it's that easy. When you do that, MFD_HI655X_PMIC gains > a dependency on COMMON_CLK and will again cause a warning on > machines that disable that during compile testing. > > Using 'select' for user-selectable options generally leads to problems, > and you are better off avoiding it. If you want to make the symbol impossible > to turn off for non-EXPERT configurations, you can write it like > > config COMMON_CLK_HI655X > tristate "Clock driver for Hi655x" if EXPERT > depends on (MFD_HI655X_PMIC || COMPILE_TEST) > depends on REGMAP > default MFD_HI655X_PMIC > > That way the option is completely hidden for non-EXPERT, > but still has the right default otherwise, and the dependencies > are tracked right for compile-testing. What about the options: CONFIG_HI3660_MBOX CONFIG_HI6220_MBOX CONFIG_STUB_CLK_HI6220 CONFIG_STUB_CLK_HI3660 ? Would make sense to do something like: diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig index b9546ab..3a07dfe 100644 --- a/arch/arm64/configs/defconfig +++ b/arch/arm64/configs/defconfig @@ -517,7 +517,6 @@ CONFIG_COMMON_CLK_CS2000_CP=y CONFIG_COMMON_CLK_S2MPS11=y CONFIG_CLK_QORIQ=y CONFIG_COMMON_CLK_PWM=y -CONFIG_STUB_CLK_HI3660=y CONFIG_COMMON_CLK_QCOM=y CONFIG_QCOM_CLK_SMD_RPM=y CONFIG_IPQ_GCC_8074=y @@ -529,8 +528,6 @@ CONFIG_HWSPINLOCK_QCOM=y CONFIG_ARM_MHU=y CONFIG_PLATFORM_MHU=y CONFIG_BCM2835_MBOX=y -CONFIG_HI3660_MBOX=y -CONFIG_HI6220_MBOX=y CONFIG_ROCKCHIP_IOMMU=y CONFIG_ARM_SMMU=y CONFIG_ARM_SMMU_V3=y diff --git a/drivers/clk/hisilicon/Kconfig b/drivers/clk/hisilicon/Kconfig index 1bd4355..becdb1d 100644 --- a/drivers/clk/hisilicon/Kconfig +++ b/drivers/clk/hisilicon/Kconfig @@ -44,14 +44,17 @@ config RESET_HISI Build reset controller driver for HiSilicon device chipsets. config STUB_CLK_HI6220 - bool "Hi6220 Stub Clock Driver" - depends on COMMON_CLK_HI6220 && MAILBOX - default ARCH_HISI + bool "Hi6220 Stub Clock Driver" if EXPERT + depends on (COMMON_CLK_HI6220 || COMPILE_TEST) + depends on MAILBOX + default COMMON_CLK_HI6220 help Build the Hisilicon Hi6220 stub clock driver. config STUB_CLK_HI3660 - bool "Hi3660 Stub Clock Driver" - depends on COMMON_CLK_HI3660 && MAILBOX + bool "Hi3660 Stub Clock Driver" if EXPERT + depends on (COMMON_CLK_HI3660 || COMPILE_TEST) + depends on MAILBOX + default COMMON_CLK_HI3660 help Build the Hisilicon Hi3660 stub clock driver. diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig index de8390d4..8d1726c 100644 --- a/drivers/mailbox/Kconfig +++ b/drivers/mailbox/Kconfig @@ -109,16 +109,19 @@ config TI_MESSAGE_MANAGER platform has support for the hardware block. config HI3660_MBOX - tristate "Hi3660 Mailbox" - depends on ARCH_HISI && OF + tristate "Hi3660 Mailbox" if EXPERT + depends on (ARCH_HISI || COMPILE_TEST) + depends on OF + default ARCH_HISI help An implementation of the hi3660 mailbox. It is used to send message between application processors and other processors/MCU/DSP. Select Y here if you want to use Hi3660 mailbox controller. config HI6220_MBOX - tristate "Hi6220 Mailbox" - depends on ARCH_HISI + tristate "Hi6220 Mailbox" if EXPERT + depends on (ARCH_HISI || COMPILE_TEST) + default ARCH_HISI help An implementation of the hi6220 mailbox. It is used to send message between application processors and MCU. Say Y here if you want to