From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754145AbbFRJm0 (ORCPT ); Thu, 18 Jun 2015 05:42:26 -0400 Received: from mailout2.w1.samsung.com ([210.118.77.12]:34214 "EHLO mailout2.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752890AbbFRJmQ (ORCPT ); Thu, 18 Jun 2015 05:42:16 -0400 X-AuditID: cbfec7f4-f79c56d0000012ee-ea-55829275455f Message-id: <55829269.2090309@samsung.com> Date: Thu, 18 Jun 2015 11:42:01 +0200 From: Andrzej Hajda User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-version: 1.0 To: Tomeu Vizoso , linux-arm-kernel@lists.infradead.org Cc: Alexander Holler , Alexandre Courbot , Arnd Bergmann , Dmitry Torokhov , Grant Likely , Greg Kroah-Hartman , Ian Campbell , Javier Martinez Canillas , Krzysztof Kozlowski , Kumar Gala , Len Brown , Linus Walleij , linux-kernel@vger.kernel.org, Lv Zheng , Mark Brown , Mark Rutland , Pawel Moll , "Rafael J. Wysocki" , Robert Moore , Rob Herring , Russell King , Stephen Warren , =?windows-1252?Q?Terje_Bergstr?= =?windows-1252?Q?=F6m?= , Thierry Reding Subject: Re: [PATCH 00/13] Discover and probe dependencies References: <1434548543-22949-1-git-send-email-tomeu.vizoso@collabora.com> In-reply-to: <1434548543-22949-1-git-send-email-tomeu.vizoso@collabora.com> Content-type: text/plain; charset=windows-1252 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA02Sa0hTYRjHe885OzuOVqep+aKINMxqpSX14cUiqg/x2tUkIYUuJz2o5XRs TrILmbdQ89JmBis1b8hMUTTygmJpWalEztRyulITTVNJpWXe2pTIb7/n//x4/l8ehpSMU45M aHgkrwznwqS0iGpbetPlrtbE+u0xm8VoUdMiRA8GvtGoOX8UoPQfeQL0fmyQQi8WagCKKyin 0VJKHWHJ9AC9nleg8VFPVDswQaPMhWICVQ51C1Bn3WMaGTvF6GdRhQAV9XQQKONhGYXa2wwC pJ/OpFBCwyshGnuZSKHWRGc0V5dDobQyA33IET9KiqFxaU4pwPFx92g8/0cDcGdaKoEXR3oo XNNfCHCtrl+I9cWzNC6o/07gypIkGvd119O4MbtUiKsKb+PUuEkaP+u6S/nYBogOBPFhoVG8 cvfBS6IQY/okUGjdrn18bqBjwJBzMrBhILsPFlgqVnkz/GAqp5OBiJGwRQDWtC0Tq8MIgBrj ssBqiVkZNJebVphit8LcaT1lZZrdARerPtNWtmfPwYnOd8Sqvwn+1posDsPYsX5wYtjZepNk 54XQcP8rYc1tWS+YlrLNqkvYY3A4f460sg17HMbFa1cUkvWAXzpk1phkXWBV6QSZAVjdmgLd f0u3xnoCyBJgz6sDFarLwXJPDxUnV6nDgz0CI+SVYPUFZmtAQYtXE2AZIF0vXieK9ZMIuChV tLwJQIaU2okDXC2ROIiLvs4rIy4q1WG8qgk4MZTUQZxdN3VWwgZzkfxVnlfwyn9bgrFxjAGn CQ6cvKDdHmganFW7Ltzatbzh/BbeJPZ/66Tn3Lz9maydnnneAafgmUbfX0dT5m4YD8/dLGkf C2240pG4d6nfHFRdOdUkq9W39TUPK118KtLrN0bYV8+4F93xdfBmCqdxWd54buuR3hlDQlXW if0lI0/Nn4QCg6lYHQ+o3nYppQrhPGWkUsX9BRRWHs7+AgAA Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Tomeu, I have few comments about the design. On 06/17/2015 03:42 PM, Tomeu Vizoso wrote: > Hi, > > this is another attempt at preventing deferred probe from obscuring why your > devices aren't probing and from delaying to the end of the boot process the > probe of the device you care the most. > > The major differences with my previous approach [0] are: > > * Dependencies are probed before the target is probed, so we don't have nested > probe() calls, avoiding a series of deadlock situations. > > * Dependencies could be stored and reused for other purposes such as for > passing resources to drivers ala devm_probe, or for warning when a device is > going to be unbound and has dependencies active, etc. With this approach we should assume many things, for example: 1. Dependencies are explicitly described in firmware (dts/dtb). It will not work for example with lookup tables present in gpios/clocks/regulators. 2. Provider create/register their resources only during probe. It is not always the case - for example componentized drivers in probe often calls only component_add, the real initialization is performed in bind callback. 3. Dependencies are mandatory, ie without it driver will not be able to successfully finish the probe. It should be not true. Sometimes device will require given resource only in specific scenario, or it can still probe successfully and ask for the resource later. I can also imagine that firmware can describe more information than given driver require, some resources even if they are present in the dts, will be not requested by the driver, it can be the case of drivers providing limited functionality, or just obsolete bindings. I have also more general design objection, which should not be necessarily true: Device node describes piece of the hardware which should be mainly interpreted by the driver. Parsing it in external code [1] violates this idea. Additionally we will have the same information parsed and interpreted in two different places (discovery framework and the driver), it does not look good to me. But as I said earlier it is just my opinion, not a solid evidence :) [1]: I know that for example clk_get is also located in external framework but currently the driver decides that it should call clk_get, my_private_clk_get, other_framework_clk_get or do not call it at all, this framework assumes that it will be always clk_get, or at least something compatible with it at binding level. Regards Andrzej > > * I have tried to keep it firmware-agnostic. The previous approach (on-demand > probing) could be done like this as well, but would require adding fwnode > APIs to all affected subsystems first. > > I have only implemented the class.get_dependencies() callback for the GPIO > subsystem and for the host1x bus because that's all that was needed on my Tegra > Chromebook to avoid deferred probes, but if this approach is deemed worthwhile > I will add more implementations so that deferred probes are avoided on the > other boards I have access to. > > [0] http://thread.gmane.org/gmane.linux.kernel.gpio/8465 > > Thanks, > > Tomeu > > Tomeu Vizoso (13): > gpiolib: Fix docs for gpiochip_add_pingroup_range > driver-core: defer all probes until late_initcall > ARM: tegra: Add gpio-ranges property > pinctrl: tegra: Only set the gpio range if needed > driver core: fix docbook for device_private.device > of/platform: Set fwnode field for new devices > driver-core: Add class.get_dependencies() callback > gpio: sysfs: implement class.get_dependencies() > gpu: host1x: implement class.get_dependencies() > driver-core: add for_each_class() > device property: add fwnode_get_parent() > device property: add fwnode_get_name() > driver-core: probe dependencies before probing > > arch/arm/boot/dts/tegra114.dtsi | 1 + > arch/arm/boot/dts/tegra124.dtsi | 1 + > arch/arm/boot/dts/tegra20.dtsi | 1 + > arch/arm/boot/dts/tegra30.dtsi | 1 + > drivers/base/base.h | 4 +- > drivers/base/class.c | 16 +++++ > drivers/base/dd.c | 128 +++++++++++++++++++++++++++++++++++++++- > drivers/base/property.c | 38 ++++++++++++ > drivers/gpio/gpiolib-sysfs.c | 81 +++++++++++++++++++++++++ > drivers/gpio/gpiolib.c | 2 +- > drivers/gpu/host1x/dev.c | 47 +++++++++++++++ > drivers/of/platform.c | 1 + > drivers/pinctrl/pinctrl-tegra.c | 19 +++++- > include/acpi/acpi_bus.h | 5 ++ > include/linux/acpi.h | 5 ++ > include/linux/device.h | 6 ++ > include/linux/fwnode.h | 5 ++ > include/linux/property.h | 4 ++ > 18 files changed, 361 insertions(+), 4 deletions(-) > From mboxrd@z Thu Jan 1 00:00:00 1970 From: a.hajda@samsung.com (Andrzej Hajda) Date: Thu, 18 Jun 2015 11:42:01 +0200 Subject: [PATCH 00/13] Discover and probe dependencies In-Reply-To: <1434548543-22949-1-git-send-email-tomeu.vizoso@collabora.com> References: <1434548543-22949-1-git-send-email-tomeu.vizoso@collabora.com> Message-ID: <55829269.2090309@samsung.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Tomeu, I have few comments about the design. On 06/17/2015 03:42 PM, Tomeu Vizoso wrote: > Hi, > > this is another attempt at preventing deferred probe from obscuring why your > devices aren't probing and from delaying to the end of the boot process the > probe of the device you care the most. > > The major differences with my previous approach [0] are: > > * Dependencies are probed before the target is probed, so we don't have nested > probe() calls, avoiding a series of deadlock situations. > > * Dependencies could be stored and reused for other purposes such as for > passing resources to drivers ala devm_probe, or for warning when a device is > going to be unbound and has dependencies active, etc. With this approach we should assume many things, for example: 1. Dependencies are explicitly described in firmware (dts/dtb). It will not work for example with lookup tables present in gpios/clocks/regulators. 2. Provider create/register their resources only during probe. It is not always the case - for example componentized drivers in probe often calls only component_add, the real initialization is performed in bind callback. 3. Dependencies are mandatory, ie without it driver will not be able to successfully finish the probe. It should be not true. Sometimes device will require given resource only in specific scenario, or it can still probe successfully and ask for the resource later. I can also imagine that firmware can describe more information than given driver require, some resources even if they are present in the dts, will be not requested by the driver, it can be the case of drivers providing limited functionality, or just obsolete bindings. I have also more general design objection, which should not be necessarily true: Device node describes piece of the hardware which should be mainly interpreted by the driver. Parsing it in external code [1] violates this idea. Additionally we will have the same information parsed and interpreted in two different places (discovery framework and the driver), it does not look good to me. But as I said earlier it is just my opinion, not a solid evidence :) [1]: I know that for example clk_get is also located in external framework but currently the driver decides that it should call clk_get, my_private_clk_get, other_framework_clk_get or do not call it at all, this framework assumes that it will be always clk_get, or at least something compatible with it at binding level. Regards Andrzej > > * I have tried to keep it firmware-agnostic. The previous approach (on-demand > probing) could be done like this as well, but would require adding fwnode > APIs to all affected subsystems first. > > I have only implemented the class.get_dependencies() callback for the GPIO > subsystem and for the host1x bus because that's all that was needed on my Tegra > Chromebook to avoid deferred probes, but if this approach is deemed worthwhile > I will add more implementations so that deferred probes are avoided on the > other boards I have access to. > > [0] http://thread.gmane.org/gmane.linux.kernel.gpio/8465 > > Thanks, > > Tomeu > > Tomeu Vizoso (13): > gpiolib: Fix docs for gpiochip_add_pingroup_range > driver-core: defer all probes until late_initcall > ARM: tegra: Add gpio-ranges property > pinctrl: tegra: Only set the gpio range if needed > driver core: fix docbook for device_private.device > of/platform: Set fwnode field for new devices > driver-core: Add class.get_dependencies() callback > gpio: sysfs: implement class.get_dependencies() > gpu: host1x: implement class.get_dependencies() > driver-core: add for_each_class() > device property: add fwnode_get_parent() > device property: add fwnode_get_name() > driver-core: probe dependencies before probing > > arch/arm/boot/dts/tegra114.dtsi | 1 + > arch/arm/boot/dts/tegra124.dtsi | 1 + > arch/arm/boot/dts/tegra20.dtsi | 1 + > arch/arm/boot/dts/tegra30.dtsi | 1 + > drivers/base/base.h | 4 +- > drivers/base/class.c | 16 +++++ > drivers/base/dd.c | 128 +++++++++++++++++++++++++++++++++++++++- > drivers/base/property.c | 38 ++++++++++++ > drivers/gpio/gpiolib-sysfs.c | 81 +++++++++++++++++++++++++ > drivers/gpio/gpiolib.c | 2 +- > drivers/gpu/host1x/dev.c | 47 +++++++++++++++ > drivers/of/platform.c | 1 + > drivers/pinctrl/pinctrl-tegra.c | 19 +++++- > include/acpi/acpi_bus.h | 5 ++ > include/linux/acpi.h | 5 ++ > include/linux/device.h | 6 ++ > include/linux/fwnode.h | 5 ++ > include/linux/property.h | 4 ++ > 18 files changed, 361 insertions(+), 4 deletions(-) >