From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753115AbbHRNhs (ORCPT ); Tue, 18 Aug 2015 09:37:48 -0400 Received: from mailout1.samsung.com ([203.254.224.24]:56044 "EHLO mailout1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753084AbbHRNhq (ORCPT ); Tue, 18 Aug 2015 09:37:46 -0400 X-AuditID: cbfee61b-f79706d000001b96-56-55d335283430 From: Bartlomiej Zolnierkiewicz To: Javier Martinez Canillas Cc: Olof Johansson , Geert Uytterhoeven , Doug Anderson , Gwendal Grignou , Sjoerd Simons , linux-kernel@vger.kernel.org Subject: Re: [PATCH] platform/chrome: Make CROS_EC_PROTO a user selectable option Date: Tue, 18 Aug 2015 15:36:13 +0200 Message-id: <4744942.ssWfRsvQQJ@amdc1976> User-Agent: KMail/4.13.3 (Linux/3.13.0-57-generic; KDE/4.13.3; x86_64; ; ) In-reply-to: <55D32CE2.3050605@osg.samsung.com> References: <1439882106-23406-1-git-send-email-javier@osg.samsung.com> <1525696.gQFOjPpKHL@amdc1976> <55D32CE2.3050605@osg.samsung.com> MIME-version: 1.0 Content-transfer-encoding: 7Bit Content-type: text/plain; charset=us-ascii X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrMLMWRmVeSWpSXmKPExsVy+t9jAV0N08uhBkvOGVmcXXaQzeLZrb1M FrMXH2WxePN2DZPF5V1z2CxOXf/MZjH9+FtWB3aP2Q0XWTz+Pr/O4nHocAejx5UTTaweW/rv snt83iQXwBbFZZOSmpNZllqkb5fAlbH9wFX2gnsiFbe7FrE3MK4Q6GLk5JAQMJFYNaudDcIW k7hwbz2YLSSwlFFi5pfYLkYuIPsro8TrpxdYQBJsAlYSE9tXMYLYIgKmEr/WvmAFKWIWeMco 0drTzgySEBYIlrg4+RxTFyMHB4uAqsSxgwYgYV4BTYmmufdZQWxRAS+J778awMo5BfQlLrRO YIdY3Mkocf6YGES9oMSPyffA9jILyEvs2z+VFcLWkli/8zjTBEaBWUjKZiEpm4WkbAEj8ypG idSC5ILipPRco7zUcr3ixNzi0rx0veT83E2M4KB/Jr2D8fAu90OMAhyMSjy8FwouhQqxJpYV V+YeYpTgYFYS4ZXkvRwqxJuSWFmVWpQfX1Sak1p8iFGag0VJnFffZFOokEB6YklqdmpqQWoR TJaJg1OqgVF/7/k/q3j2TIh8om7N+9nL9MPt7eaVCs5a5za1FcVeOfK4hisqJUQ85G+qgpSA RfqUR0vln+ct/dlk7Xli70YtYakUH+fnalL2fjv9tMuPXnz74a7C7B9ng2dP2+K8L1n409vq dgObq3P1Vr5bUj01KW7hywjpSR3W0zteJ316vLZ42ov+NbJKLMUZiYZazEXFiQC42T/MdgIA AA== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On Tuesday, August 18, 2015 03:02:26 PM Javier Martinez Canillas wrote: > Hello Bartlomiej, > > Thanks a lot for your feedback. > > On 08/18/2015 02:40 PM, Bartlomiej Zolnierkiewicz wrote: > > > > Hi, > > > > On Tuesday, August 18, 2015 09:15:06 AM Javier Martinez Canillas wrote: > >> The boolean CROS_EC_PROTO symbol is selected by MFD_CROS_EC but that can > >> cause Kconfig circular dependencies so is better to change the select to > > > > Could you please give a reference to the problem or error message > > that you're getting (I was not following the previous discussion). > > > > Yes, the original Kconfig warning message was: > > warning: (MFD_CROS_EC) selects CHROME_PLATFORMS which has unmet direct dependencies (X86 || ARM) > > Paul fixed on [0] by making MFD_CROS_EC depends on X86 || ARM but that > is not really true since the driver could be used in other platforms so > I tried to instead fix it by removing unneeded dependencies in [1] but > one of the patches was nacked by Geert [2] who mentioned the issue about > mixing select and "depends on". I think that Geert's comments are valid. Dependencies should limit the config option to the archs for which the driver is really needed (the rest can be handled with COMPILE_TEST dependency). > >> a depends on. But in order to be able to change that, the CROS_EC_PROTO > >> symbol has to be one that can be selected by the user. > > > > Looking at the code behind the config option it seems that it is just > > a helper library and should not be made user-visible. Why can't > > Yeah, I in fact used as a reference the DRM/KMS helpers (DRM_KMS_HELPER) > that is selected by drivers instead of depending on it. > > > the issue be fixed the other way (make other config options select > > CROS_EC_PROTO consistently)? > > > > That is another option indeed. I thought that the use of select was > discouraged and that it was preferable to use "depends on" even when > the Kconfig symbol is a boolean to enable support for some helpers. No, this is not true. It is just that "select" has to be used with care. ;) > But I don't really have a strong opinion on either approach tbh. Please rework your patches to use "select". Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics > > Best regards, > > -- > > Bartlomiej Zolnierkiewicz > > Samsung R&D Institute Poland > > Samsung Electronics > > > > [0]: https://lkml.org/lkml/2015/6/20/219 > [1]: https://lkml.org/lkml/2015/6/24/689 > [2]: https://lkml.org/lkml/2015/8/17/103 > > Best regards,