From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751169AbcB2QnL (ORCPT ); Mon, 29 Feb 2016 11:43:11 -0500 Received: from mout.kundenserver.de ([212.227.126.133]:63546 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750775AbcB2QnI (ORCPT ); Mon, 29 Feb 2016 11:43:08 -0500 From: Arnd Bergmann To: linux-arm-kernel@lists.infradead.org Cc: Tomi Valkeinen , Paul Bolle , linux-samsung-soc@vger.kernel.org, Donghwa Lee , Krzysztof Kozlowski , linux-kernel@vger.kernel.org, Inki Dae , Kyungmin Park , Kukjin Kim , linux-fbdev@vger.kernel.org, Jean-Christophe Plagniol-Villard Subject: Re: [PATCH v2] video: exynos: fix modular build Date: Mon, 29 Feb 2016 17:39:59 +0100 Message-ID: <6291541.7IMAnfX0g2@wuerfel> User-Agent: KMail/4.11.5 (Linux/3.16.0-10-generic; KDE/4.11.5; x86_64; ; ) In-Reply-To: <56D46DFD.6090706@ti.com> References: <1456490307-823812-1-git-send-email-arnd@arndb.de> <56D46DFD.6090706@ti.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:jfQRdaOOGJ2ErS3VQyIu2bAw71zQgPHEa90AGyzmOoCuligaYj6 b+4W9RP5WPNApWEbanMQx+wdTyGWLnP5CLgrcPNrueM5dXGD6VpoKHMWVVPimQoly72DxWc BPlNNl81m/G2RPdJzdlmUmfc/rhrnKpdVl+WYakhg+gZUB+nVnOHTSlM9MRRn+T95bSzhrk 1tL0MX2rCj3vv76LnOklw== X-UI-Out-Filterresults: notjunk:1;V01:K0:1DfTAq4r8BM=:sqSso9cZN6OHK/aw8idtbX Gp5MC6fce580NeOp6ML++2JsqPxrrlylWcpD3u7alP4lWGt13CvKgr7skdhAO6k1c/JlBT6ox KNE/ycGWT1p1NcOBjJkLsmSXHz8xDph07Z4/SJN64D1tCFVf7vy0FinZX7XdPMhsBmywIgBQG dmKvQgebv+iSF9hnsqmxEYTHSn+nwJGwT0L/1jD4TyVP5Y1a2SzTfjrpJwuZWW/Ss52tccJWc W48EYLjqBXF3hf9FrDn4HklahjNicNQtPul0vmfsIT3uVafxaG7g2r81OLgiere8slQfZP3LC UXPBmgFCUZMxCVLRhKTDDY8BnvRFku7dvW9Pd/qx7RbY5Z5DHMD7RZ3SwMVHxyy/9PArZlh9m elcpiawVNKx7h/RXTA6H1bXbEh6w+rJa0mWqWk5eeCzE6eJon8wOu6JH3OQJl+kBfW6x8h7uK vY1fb3UIKsNBTVLfJd9lIKMpbwUExX883CRAnL982wFruisMwKdUFUy7EVigOv4baVvGZ3bsh GuzVuJG3hX9LmuEdSPmoG6KSBan7HqOhfsefIs2zNBh01zZDra0CxQYnOymSFX2O+n5WllpG0 /+gbo0qDpZ40JnQjdfYXIZnxCRwPHv5Wg+E8Jh0HAMC3Aan7pRtpjUticsTRTUMVR1KLzde76 6NdssXBAE0pxuTAGSTorY20NszRggcYgulYTjVJF8rAsbD61Kh7PZvK5F3z6Rre2QXKvryZ7l 5hh+VHMp3Sx7FL4o Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Monday 29 February 2016 18:12:45 Tomi Valkeinen wrote: > Hi, > > On 26/02/16 14:38, Arnd Bergmann wrote: > > The s6e8ax0 driver has a dependency on BACKLIGHT_CLASS_DEVICE, > > which can be configured as a loadable module, so we have to > > make the driver a tristate symbol as well, to avoid this error: > > > > drivers/built-in.o: In function `s6e8ax0_probe': > > :(.text+0x23a48): undefined reference to `devm_backlight_device_register' > > If a 'bool' Kconfig option depends on BACKLIGHT_CLASS_DEVICE, shouldn't > the Kconfig dependency take care of having BACKLIGHT_CLASS_DEVICE as > built-in? No, that's not how Kconfig interprets it. There are many bool option that depend on tristate options but can be enabled if the dependency is built-in. Take this one for example: config FIRMWARE_EDID bool "Enable firmware EDID" depends on FB We clearly want to be able to turn this on even for FB=m. > > This also means we get another error from a missing export, which > > this fixes as well: > > > > ERROR: "exynos_mipi_dsi_register_lcd_driver" [drivers/video/fbdev/exynos/s6e8ax0.ko] undefined! > > > > The drivers are all written to be loadable modules already, > > except the Kconfig options for that are missing, which makes > > the patch really easy. > > Looks and sound fine, except doesn't this tell that the drivers have > never been tested as modules? Did you or someone else actually test these? No, this is not runtime tested. Generally there is very little that can go wrong here though. An alternative would be to change the dependency to depends on BACKLIGHT_CLASS_DEVICE=y which doesn't allow the driver to be turned on for the =m case. However, no other framebuffer driver does this. > > diff --git a/drivers/video/fbdev/exynos/Makefile b/drivers/video/fbdev/exynos/Makefile > > index b5b1bd228abb..02d8dc522fea 100644 > > --- a/drivers/video/fbdev/exynos/Makefile > > +++ b/drivers/video/fbdev/exynos/Makefile > > @@ -2,6 +2,8 @@ > > # Makefile for the exynos video drivers. > > # > > > > -obj-$(CONFIG_EXYNOS_MIPI_DSI) += exynos_mipi_dsi.o exynos_mipi_dsi_common.o \ > > - exynos_mipi_dsi_lowlevel.o > > +obj-$(CONFIG_EXYNOS_MIPI_DSI) += exynos-mipi-dsi-mod.o > > + > > +exynos-mipi-dsi-mod-objs += exynos_mipi_dsi.o exynos_mipi_dsi_common.o \ > > + exynos_mipi_dsi_lowlevel.o > > Hmm, why is this makefile change needed? The original Makefile would link each file into a separate module, but that cannot work, because they reference symbols from each other that are not exported to other modules. With my change, all the files get linked into a single module. Arnd