From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kevin Hilman Subject: Re: [PATCH 02/24] drivercore: Bind/unbind power domain on probe/remove Date: Wed, 11 Jun 2014 11:16:34 -0700 Message-ID: <7h38fbia65.fsf@paris.lan> References: <1402397497-26737-1-git-send-email-ulf.hansson@linaro.org> <1894835.cKK4602bPf@vostro.rjw.lan> <20140610212731.GA11191@kroah.com> <53977BC1.5000301@gmail.com> Mime-Version: 1.0 Content-Type: text/plain Return-path: Received: from mail-pa0-f42.google.com ([209.85.220.42]:34887 "EHLO mail-pa0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752640AbaFKSQh (ORCPT ); Wed, 11 Jun 2014 14:16:37 -0400 Received: by mail-pa0-f42.google.com with SMTP id lj1so79276pab.15 for ; Wed, 11 Jun 2014 11:16:37 -0700 (PDT) In-Reply-To: <53977BC1.5000301@gmail.com> (Tomasz Figa's message of "Tue, 10 Jun 2014 23:42:25 +0200") Sender: linux-pm-owner@vger.kernel.org List-Id: linux-pm@vger.kernel.org To: Tomasz Figa Cc: Greg Kroah-Hartman , "Rafael J. Wysocki" , Ulf Hansson , "Rafael J. Wysocki" , Len Brown , Pavel Machek , "linux-pm@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , Alan Stern , Daniel Lezcano , "devicetree@vger.kernel.org" , Linus Walleij , Simon Horman , Magnus Damm , Ben Dooks , Kukjin Kim , Stephen Boyd , Philipp Zabel , Mark Brown , Tomasz Figa Tomasz Figa writes: > On 10.06.2014 23:27, Greg Kroah-Hartman wrote: >> On Tue, Jun 10, 2014 at 11:27:45PM +0200, Rafael J. Wysocki wrote: >>> On Tuesday, June 10, 2014 02:53:26 PM Ulf Hansson wrote: >>>> On 10 June 2014 14:11, Rafael J. Wysocki wrote: >>>>> On Tue, Jun 10, 2014 at 12:51 PM, Ulf Hansson wrote: >>>>>> From: Tomasz Figa >>>>>> >>>>>> On a number of platforms, devices are part of controllable power >>>>>> domains, which need to be enabled before such devices can be accessed >>>>>> and may be powered down when the device is idle to save some power. >>>>>> This means that on systems that support power domain control using >>>>>> generic power domains subsystem, it is necessary to add device to its >>>>>> power domain before binding a driver to it and remove it from its power >>>>>> domain after its driver is unbound to make sure that an unused device >>>>>> does not affect power domain state. >>>>>> >>>>>> Since this is not limited to particular busses and specific >>>>>> archs/platforms, >>>>> >>>>> Actually, this isn't correrct. It is limited to the platforms that >>>>> use Device Trees now. >>>> >>>> Correct, we should update the commit message/docs. >>>> >>>>> >>>>> Moreover, it is not consistent with the way we add devices to the ACPI PM >>>>> domain, which is the ACPI counterpart of this. >>>> >>>> I am not sure why you think consistency for ACPI is important here. >>>> ACPI PM will still be able to handle it's domain/device registering as >>>> before. There are even other pm_domains that don't use genpd which >>>> need to handle this themselves. >>> >>> My point is that doing things like that in different places for different >>> firmware interfaces is confusing and likely to lead to coding mistakes in >>> the future. >>> >>>> Or are you saying that you prefer bus notifiers in favour of making >>>> use of the driver core for this matter? >>> >>> Well, please grep for acpi_dev_pm_attach() and see where it is done. >>> Surely not in drivers/base/dd.c. Also I'm not sure why you're talking >>> about bus notifiers in this context. >>> >>>> Shouldn't the driver core handle most of the common things for a device >>>> driver? >>> >>> Common, yes. Platform-specific, no. >>> >>>> Let's compare how the pinctrls are being managed in the driver core, for >>>> example. >>> >>> pinctrl has Device Trees support only at the moment (as far as firmware >>> interfaces go) and quite frankly I'm not sure if/how we'll need to change >>> it to cover ACPI as well. >>> >>> But for power domains, please keep that stuff away from dd.c. That is, >>> unless Greg specifically disagrees with me and decides to apply this >>> patch regardless. :-) >> >> Nope, no disagreement from me toward you at all here, keep up the good >> work :) > > OK, so proposed solution is to put this in: > > - platform_drv_probe(), > - spi_drv_probe(), > - i2c_device_probe(), > - amba_probe(), > > ... > > - and any other bus type, which can have devices instantiated from DT. Since this is a DT feature, what about calling __pm_genpd_of_add_device() when the power-domain nodes are discovered? Kevin From mboxrd@z Thu Jan 1 00:00:00 1970 From: khilman@linaro.org (Kevin Hilman) Date: Wed, 11 Jun 2014 11:16:34 -0700 Subject: [PATCH 02/24] drivercore: Bind/unbind power domain on probe/remove In-Reply-To: <53977BC1.5000301@gmail.com> (Tomasz Figa's message of "Tue, 10 Jun 2014 23:42:25 +0200") References: <1402397497-26737-1-git-send-email-ulf.hansson@linaro.org> <1894835.cKK4602bPf@vostro.rjw.lan> <20140610212731.GA11191@kroah.com> <53977BC1.5000301@gmail.com> Message-ID: <7h38fbia65.fsf@paris.lan> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Tomasz Figa writes: > On 10.06.2014 23:27, Greg Kroah-Hartman wrote: >> On Tue, Jun 10, 2014 at 11:27:45PM +0200, Rafael J. Wysocki wrote: >>> On Tuesday, June 10, 2014 02:53:26 PM Ulf Hansson wrote: >>>> On 10 June 2014 14:11, Rafael J. Wysocki wrote: >>>>> On Tue, Jun 10, 2014 at 12:51 PM, Ulf Hansson wrote: >>>>>> From: Tomasz Figa >>>>>> >>>>>> On a number of platforms, devices are part of controllable power >>>>>> domains, which need to be enabled before such devices can be accessed >>>>>> and may be powered down when the device is idle to save some power. >>>>>> This means that on systems that support power domain control using >>>>>> generic power domains subsystem, it is necessary to add device to its >>>>>> power domain before binding a driver to it and remove it from its power >>>>>> domain after its driver is unbound to make sure that an unused device >>>>>> does not affect power domain state. >>>>>> >>>>>> Since this is not limited to particular busses and specific >>>>>> archs/platforms, >>>>> >>>>> Actually, this isn't correrct. It is limited to the platforms that >>>>> use Device Trees now. >>>> >>>> Correct, we should update the commit message/docs. >>>> >>>>> >>>>> Moreover, it is not consistent with the way we add devices to the ACPI PM >>>>> domain, which is the ACPI counterpart of this. >>>> >>>> I am not sure why you think consistency for ACPI is important here. >>>> ACPI PM will still be able to handle it's domain/device registering as >>>> before. There are even other pm_domains that don't use genpd which >>>> need to handle this themselves. >>> >>> My point is that doing things like that in different places for different >>> firmware interfaces is confusing and likely to lead to coding mistakes in >>> the future. >>> >>>> Or are you saying that you prefer bus notifiers in favour of making >>>> use of the driver core for this matter? >>> >>> Well, please grep for acpi_dev_pm_attach() and see where it is done. >>> Surely not in drivers/base/dd.c. Also I'm not sure why you're talking >>> about bus notifiers in this context. >>> >>>> Shouldn't the driver core handle most of the common things for a device >>>> driver? >>> >>> Common, yes. Platform-specific, no. >>> >>>> Let's compare how the pinctrls are being managed in the driver core, for >>>> example. >>> >>> pinctrl has Device Trees support only at the moment (as far as firmware >>> interfaces go) and quite frankly I'm not sure if/how we'll need to change >>> it to cover ACPI as well. >>> >>> But for power domains, please keep that stuff away from dd.c. That is, >>> unless Greg specifically disagrees with me and decides to apply this >>> patch regardless. :-) >> >> Nope, no disagreement from me toward you at all here, keep up the good >> work :) > > OK, so proposed solution is to put this in: > > - platform_drv_probe(), > - spi_drv_probe(), > - i2c_device_probe(), > - amba_probe(), > > ... > > - and any other bus type, which can have devices instantiated from DT. Since this is a DT feature, what about calling __pm_genpd_of_add_device() when the power-domain nodes are discovered? Kevin