From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932189AbbLNJtL (ORCPT ); Mon, 14 Dec 2015 04:49:11 -0500 Received: from mail-qk0-f174.google.com ([209.85.220.174]:36452 "EHLO mail-qk0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752660AbbLNJtI (ORCPT ); Mon, 14 Dec 2015 04:49:08 -0500 MIME-Version: 1.0 In-Reply-To: <20151209104752.GB1036@omega> References: <1449251148-19344-1-git-send-email-eric@anholt.net> <1449251148-19344-2-git-send-email-eric@anholt.net> <5665599A.4060509@nvidia.com> <7h8u54lqbv.fsf@deeprootsystems.com> <20151209104752.GB1036@omega> Date: Mon, 14 Dec 2015 10:49:07 +0100 Message-ID: Subject: Re: [PATCH v2 1/5] power: domain: add pm_genpd_exit From: Ulf Hansson To: Alexander Aring , Eric Anholt , Jon Hunter Cc: Kevin Hilman , "Rafael J. Wysocki" , Mark Rutland , "devicetree@vger.kernel.org" , Florian Fainelli , Pawel Moll , Stephen Warren , Greg Kroah-Hartman , "linux-pm@vger.kernel.org" , Lee Jones , "linux-kernel@vger.kernel.org" , Rob Herring , linux-rpi-kernel@lists.infradead.org, Pavel Machek , Len Brown , Ian Campbell , "linux-arm-kernel@lists.infradead.org" , Russell King - ARM Linux Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org +Russell [...] >> >> +void pm_genpd_exit(struct generic_pm_domain *genpd) >> >> +{ >> >> + if (IS_ERR_OR_NULL(genpd)) >> >> + return; >> >> + >> >> + /* check if domain is still in registered inside the pm subsystem */ >> >> + WARN_ON_ONCE(!list_empty(&genpd->master_links) || >> >> + !list_empty(&genpd->slave_links) || >> >> + !list_empty(&genpd->dev_list)); >> >> + >> > >> > Why not return an error here? Seems bad to remove it, if it could still >> > be referenced by other domains. >> >> I had pointed this out as well in an earlier review. >> > > I talked with Ulf Hansson about such handling and there exists currently > no handling to remove the pm_genpd on error handling (what our use case > is for RPi domains). > > The real solution would be a "pm_genpd_exit_recursive" functionality to > remove subdomains, etc -> simple everything. Anway I am not a expert into > power domain functionality and this simple approach was enough to him to > add "something" which we have actually a lack of support. > > [...] Just to be clear on my view. I think Russell made a good summary of how we should implement this API [1]. We should return an error, instead of WARN_ON_ONCE and continue. Perhaps you can do both a WARN *and* return an error. Regarding the similar patch from Jon Hunter, I would suggest we focus on Alexander's $subject patch and try to it queued for 4.5. Please send a new version. Also, as other SoC genpd driver isn't using a "pm_genpd_exit()" API, it shouldn't prevent neither Alexander/Eric or Jon from proceeding with the RPI/Tegra genpd drivers. Once the new API is in place you can update these drivers to properly deal with error handling and undo things from pm_genpd_init(). Kind regards Uffe [1] https://lkml.org/lkml/2015/12/9/308 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ulf Hansson Subject: Re: [PATCH v2 1/5] power: domain: add pm_genpd_exit Date: Mon, 14 Dec 2015 10:49:07 +0100 Message-ID: References: <1449251148-19344-1-git-send-email-eric@anholt.net> <1449251148-19344-2-git-send-email-eric@anholt.net> <5665599A.4060509@nvidia.com> <7h8u54lqbv.fsf@deeprootsystems.com> <20151209104752.GB1036@omega> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Return-path: In-Reply-To: <20151209104752.GB1036@omega> Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Alexander Aring , Eric Anholt , Jon Hunter Cc: Kevin Hilman , "Rafael J. Wysocki" , Mark Rutland , "devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , Florian Fainelli , Pawel Moll , Stephen Warren , Greg Kroah-Hartman , "linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , Lee Jones , "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , Rob Herring , linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, Pavel Machek , Len Brown , Ian Campbell , "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" , Russell King - ARM Linux List-Id: devicetree@vger.kernel.org +Russell [...] >> >> +void pm_genpd_exit(struct generic_pm_domain *genpd) >> >> +{ >> >> + if (IS_ERR_OR_NULL(genpd)) >> >> + return; >> >> + >> >> + /* check if domain is still in registered inside the pm subsystem */ >> >> + WARN_ON_ONCE(!list_empty(&genpd->master_links) || >> >> + !list_empty(&genpd->slave_links) || >> >> + !list_empty(&genpd->dev_list)); >> >> + >> > >> > Why not return an error here? Seems bad to remove it, if it could still >> > be referenced by other domains. >> >> I had pointed this out as well in an earlier review. >> > > I talked with Ulf Hansson about such handling and there exists currently > no handling to remove the pm_genpd on error handling (what our use case > is for RPi domains). > > The real solution would be a "pm_genpd_exit_recursive" functionality to > remove subdomains, etc -> simple everything. Anway I am not a expert into > power domain functionality and this simple approach was enough to him to > add "something" which we have actually a lack of support. > > [...] Just to be clear on my view. I think Russell made a good summary of how we should implement this API [1]. We should return an error, instead of WARN_ON_ONCE and continue. Perhaps you can do both a WARN *and* return an error. Regarding the similar patch from Jon Hunter, I would suggest we focus on Alexander's $subject patch and try to it queued for 4.5. Please send a new version. Also, as other SoC genpd driver isn't using a "pm_genpd_exit()" API, it shouldn't prevent neither Alexander/Eric or Jon from proceeding with the RPI/Tegra genpd drivers. Once the new API is in place you can update these drivers to properly deal with error handling and undo things from pm_genpd_init(). Kind regards Uffe [1] https://lkml.org/lkml/2015/12/9/308 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 From: ulf.hansson@linaro.org (Ulf Hansson) Date: Mon, 14 Dec 2015 10:49:07 +0100 Subject: [PATCH v2 1/5] power: domain: add pm_genpd_exit In-Reply-To: <20151209104752.GB1036@omega> References: <1449251148-19344-1-git-send-email-eric@anholt.net> <1449251148-19344-2-git-send-email-eric@anholt.net> <5665599A.4060509@nvidia.com> <7h8u54lqbv.fsf@deeprootsystems.com> <20151209104752.GB1036@omega> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org +Russell [...] >> >> +void pm_genpd_exit(struct generic_pm_domain *genpd) >> >> +{ >> >> + if (IS_ERR_OR_NULL(genpd)) >> >> + return; >> >> + >> >> + /* check if domain is still in registered inside the pm subsystem */ >> >> + WARN_ON_ONCE(!list_empty(&genpd->master_links) || >> >> + !list_empty(&genpd->slave_links) || >> >> + !list_empty(&genpd->dev_list)); >> >> + >> > >> > Why not return an error here? Seems bad to remove it, if it could still >> > be referenced by other domains. >> >> I had pointed this out as well in an earlier review. >> > > I talked with Ulf Hansson about such handling and there exists currently > no handling to remove the pm_genpd on error handling (what our use case > is for RPi domains). > > The real solution would be a "pm_genpd_exit_recursive" functionality to > remove subdomains, etc -> simple everything. Anway I am not a expert into > power domain functionality and this simple approach was enough to him to > add "something" which we have actually a lack of support. > > [...] Just to be clear on my view. I think Russell made a good summary of how we should implement this API [1]. We should return an error, instead of WARN_ON_ONCE and continue. Perhaps you can do both a WARN *and* return an error. Regarding the similar patch from Jon Hunter, I would suggest we focus on Alexander's $subject patch and try to it queued for 4.5. Please send a new version. Also, as other SoC genpd driver isn't using a "pm_genpd_exit()" API, it shouldn't prevent neither Alexander/Eric or Jon from proceeding with the RPI/Tegra genpd drivers. Once the new API is in place you can update these drivers to properly deal with error handling and undo things from pm_genpd_init(). Kind regards Uffe [1] https://lkml.org/lkml/2015/12/9/308