From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jani Nikula Date: Mon, 20 Apr 2020 08:02:33 +0000 Subject: Re: [PATCH 7/8] fbdev: rework backlight dependencies Message-Id: <871roi37qe.fsf@intel.com> List-Id: References: <20200417155553.675905-1-arnd@arndb.de> <20200417155553.675905-8-arnd@arndb.de> <20200417170444.GB30483@ravnborg.org> In-Reply-To: <20200417170444.GB30483@ravnborg.org> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Sam Ravnborg , Arnd Bergmann Cc: marex@denx.de, Jason Gunthorpe , linux-fbdev@vger.kernel.org, dsd@laptop.org, Nicolas Pitre , airlied@linux.ie, masahiroy@kernel.org, jfrederich@gmail.com, Saeed Mahameed , thellstrom@vmware.com, dri-devel@lists.freedesktop.org, linux-renesas-soc@vger.kernel.org, Andrzej Hajda , kieran.bingham+renesas@ideasonboard.com, geert@linux-m68k.org, haojian.zhuang@gmail.com, linux-graphics-maintainer@vmware.com, robert.jarzmik@free.fr, daniel@zonque.org, Laurent.pinchart@ideasonboard.com On Fri, 17 Apr 2020, Sam Ravnborg wrote: > Hi Arnd. > > On Fri, Apr 17, 2020 at 05:55:52PM +0200, Arnd Bergmann wrote: >> Rather than having CONFIG_FB_BACKLIGHT select CONFIG_BACKLIGHT_CLASS_DEVICE, >> make any driver that needs it have a dependency on the class device >> being available, to prevent circular dependencies. >> >> This is the same way that the backlight is already treated for the DRM >> subsystem. > > I am not happy with the direction of this patch. > It is not easy to understand that one has to enable backlight to > be allowed to select a display or an fbdev driver. Arguably that is a problem in Kconfig, and that applies to *all* dependencies everywhere. It isn't something new to this patch. For example, in the context of this patch you have: config HT16K33 tristate "Holtek Ht16K33 LED controller with keyscan" depends on FB && OF && I2C && INPUT + depends on BACKLIGHT_CLASS_DEVICE The same thing could be said about FB and OF and IC2 and INPUT for HT16K33, right? Why would *backlight* be the tipping point that makes this difficult to understand? Yeah, I agree it's not straightforward, but I think depends is the right choice, not select. The effort for making this easier to understand should be directed at the various menuconfig tools to better highlight the dependencies that should be enabled to let you enable other options. > How about somthing like this: I think this is a hack in Kconfig files that should really be fixed in the Kconfig tooling instead. IMHO Kconfig should be as simple a description of the dependencies as possible, not so much a UI language. FWIW the patch is Acked-by: Jani Nikula BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center 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 X-Spam-Level: X-Spam-Status: No, score=-0.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id BFA7BC3815B for ; Mon, 20 Apr 2020 08:02:44 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A5062206D5 for ; Mon, 20 Apr 2020 08:02:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726048AbgDTICo (ORCPT ); Mon, 20 Apr 2020 04:02:44 -0400 Received: from mga12.intel.com ([192.55.52.136]:20584 "EHLO mga12.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726054AbgDTICo (ORCPT ); Mon, 20 Apr 2020 04:02:44 -0400 IronPort-SDR: ze4geTdS44okCkVgIqe855DrLf2g4FWNnOp/HXW5ilMw4RMJMEWmrOPTR1YJF0TszXZwk454bl TPN2MIBUtJJw== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Apr 2020 01:02:43 -0700 IronPort-SDR: FuTeOBDq3EWzERLG+wP/aJ56TVYKzRwUG7FrNE48/EOgCU8Fu6TO3rHp5dk/b68TrPcF0uOuYK rKUR5Cm2VpCQ== X-IronPort-AV: E=Sophos;i="5.72,406,1580803200"; d="scan'208";a="429035397" Received: from iastakh-mobl.ccr.corp.intel.com (HELO localhost) ([10.252.63.229]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Apr 2020 01:02:36 -0700 From: Jani Nikula To: Sam Ravnborg , Arnd Bergmann Cc: dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org, marex@denx.de, dsd@laptop.org, Andrzej Hajda , airlied@linux.ie, masahiroy@kernel.org, Nicolas Pitre , Saeed Mahameed , thellstrom@vmware.com, haojian.zhuang@gmail.com, geert@linux-m68k.org, linux-renesas-soc@vger.kernel.org, Jason Gunthorpe , kieran.bingham+renesas@ideasonboard.com, linux-graphics-maintainer@vmware.com, Laurent.pinchart@ideasonboard.com, jfrederich@gmail.com, robert.jarzmik@free.fr, daniel@zonque.org Subject: Re: [PATCH 7/8] fbdev: rework backlight dependencies In-Reply-To: <20200417170444.GB30483@ravnborg.org> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <20200417155553.675905-1-arnd@arndb.de> <20200417155553.675905-8-arnd@arndb.de> <20200417170444.GB30483@ravnborg.org> Date: Mon, 20 Apr 2020 11:02:33 +0300 Message-ID: <871roi37qe.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-renesas-soc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-renesas-soc@vger.kernel.org On Fri, 17 Apr 2020, Sam Ravnborg wrote: > Hi Arnd. > > On Fri, Apr 17, 2020 at 05:55:52PM +0200, Arnd Bergmann wrote: >> Rather than having CONFIG_FB_BACKLIGHT select CONFIG_BACKLIGHT_CLASS_DEVICE, >> make any driver that needs it have a dependency on the class device >> being available, to prevent circular dependencies. >> >> This is the same way that the backlight is already treated for the DRM >> subsystem. > > I am not happy with the direction of this patch. > It is not easy to understand that one has to enable backlight to > be allowed to select a display or an fbdev driver. Arguably that is a problem in Kconfig, and that applies to *all* dependencies everywhere. It isn't something new to this patch. For example, in the context of this patch you have: config HT16K33 tristate "Holtek Ht16K33 LED controller with keyscan" depends on FB && OF && I2C && INPUT + depends on BACKLIGHT_CLASS_DEVICE The same thing could be said about FB and OF and IC2 and INPUT for HT16K33, right? Why would *backlight* be the tipping point that makes this difficult to understand? Yeah, I agree it's not straightforward, but I think depends is the right choice, not select. The effort for making this easier to understand should be directed at the various menuconfig tools to better highlight the dependencies that should be enabled to let you enable other options. > How about somthing like this: I think this is a hack in Kconfig files that should really be fixed in the Kconfig tooling instead. IMHO Kconfig should be as simple a description of the dependencies as possible, not so much a UI language. FWIW the patch is Acked-by: Jani Nikula BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center 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 X-Spam-Level: X-Spam-Status: No, score=-0.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B5F67C54FCC for ; Mon, 20 Apr 2020 08:02:46 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (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 94CBE206D5 for ; Mon, 20 Apr 2020 08:02:46 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 94CBE206D5 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 06A1188647; Mon, 20 Apr 2020 08:02:46 +0000 (UTC) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by gabe.freedesktop.org (Postfix) with ESMTPS id 6C5DF88647 for ; Mon, 20 Apr 2020 08:02:44 +0000 (UTC) IronPort-SDR: XAI1l0nD/PLj2aKl5v2n3jNKbkcwKq1Y5vmiH4MT/eQQIgaOLCnYOTjiSw30+JkLmX4DaIGbPr 3IQHu5v3lP4w== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Apr 2020 01:02:43 -0700 IronPort-SDR: FuTeOBDq3EWzERLG+wP/aJ56TVYKzRwUG7FrNE48/EOgCU8Fu6TO3rHp5dk/b68TrPcF0uOuYK rKUR5Cm2VpCQ== X-IronPort-AV: E=Sophos;i="5.72,406,1580803200"; d="scan'208";a="429035397" Received: from iastakh-mobl.ccr.corp.intel.com (HELO localhost) ([10.252.63.229]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Apr 2020 01:02:36 -0700 From: Jani Nikula To: Sam Ravnborg , Arnd Bergmann Subject: Re: [PATCH 7/8] fbdev: rework backlight dependencies In-Reply-To: <20200417170444.GB30483@ravnborg.org> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <20200417155553.675905-1-arnd@arndb.de> <20200417155553.675905-8-arnd@arndb.de> <20200417170444.GB30483@ravnborg.org> Date: Mon, 20 Apr 2020 11:02:33 +0300 Message-ID: <871roi37qe.fsf@intel.com> MIME-Version: 1.0 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: marex@denx.de, Jason Gunthorpe , linux-fbdev@vger.kernel.org, dsd@laptop.org, Nicolas Pitre , airlied@linux.ie, masahiroy@kernel.org, jfrederich@gmail.com, Saeed Mahameed , thellstrom@vmware.com, dri-devel@lists.freedesktop.org, linux-renesas-soc@vger.kernel.org, Andrzej Hajda , kieran.bingham+renesas@ideasonboard.com, geert@linux-m68k.org, haojian.zhuang@gmail.com, linux-graphics-maintainer@vmware.com, robert.jarzmik@free.fr, daniel@zonque.org, Laurent.pinchart@ideasonboard.com Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On Fri, 17 Apr 2020, Sam Ravnborg wrote: > Hi Arnd. > > On Fri, Apr 17, 2020 at 05:55:52PM +0200, Arnd Bergmann wrote: >> Rather than having CONFIG_FB_BACKLIGHT select CONFIG_BACKLIGHT_CLASS_DEVICE, >> make any driver that needs it have a dependency on the class device >> being available, to prevent circular dependencies. >> >> This is the same way that the backlight is already treated for the DRM >> subsystem. > > I am not happy with the direction of this patch. > It is not easy to understand that one has to enable backlight to > be allowed to select a display or an fbdev driver. Arguably that is a problem in Kconfig, and that applies to *all* dependencies everywhere. It isn't something new to this patch. For example, in the context of this patch you have: config HT16K33 tristate "Holtek Ht16K33 LED controller with keyscan" depends on FB && OF && I2C && INPUT + depends on BACKLIGHT_CLASS_DEVICE The same thing could be said about FB and OF and IC2 and INPUT for HT16K33, right? Why would *backlight* be the tipping point that makes this difficult to understand? Yeah, I agree it's not straightforward, but I think depends is the right choice, not select. The effort for making this easier to understand should be directed at the various menuconfig tools to better highlight the dependencies that should be enabled to let you enable other options. > How about somthing like this: I think this is a hack in Kconfig files that should really be fixed in the Kconfig tooling instead. IMHO Kconfig should be as simple a description of the dependencies as possible, not so much a UI language. FWIW the patch is Acked-by: Jani Nikula BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel