Hello Bartlomiej, On 08/18/2015 03:36 PM, Bartlomiej Zolnierkiewicz wrote: > > 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). >
Yes, which is true for the drivers under CHROME_PLATFORMS (i.e: CROS_EC_LPC has X86 || COMPILE_TEST) but my point was that CHROME_PLATFORMS is not a driver but a submenu that has (for now) drivers for ARM and X86. But that doesn't mean that a Chromebook for another arch could be produced and then you will need to add a X86 || ARM || FOO || BAR || COMPILE_TEST for CHROME_PLATFORMS which seems to me kind of silly. That's why patch removed the dependency for CHROME_PLATFORMS but no of course for the drivers for each arch. >>>> 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. ;) > Got it :) >> But I don't really have a strong opinion on either approach tbh. > > Please rework your patches to use "select". > Ok, will do. > Best regards, > -- > Bartlomiej Zolnierkiewicz > Samsung R&D Institute Poland > Samsung Electronics > Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
