Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 28 Mar 2022 11:15:05 +0300
From:   Heikki Krogerus <heikki.krogerus@linux.intel.com>
To:     Lukas Wunner <lukas@wunner.de>
Cc:     "Rafael J. Wysocki" <rjw@rjwysocki.net>,
        Felipe Balbi <balbi@kernel.org>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        Sakari Ailus <sakari.ailus@linux.intel.com>,
        linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
        linux-acpi@vger.kernel.org
Subject: Re: [PATCHv2 1/3] software node: Power management operations for
 software nodes
Message-ID: <YkFuiarj9z9YUgOs@kuha.fi.intel.com>
References: <20201029105941.63410-1-heikki.krogerus@linux.intel.com>
 <20201029105941.63410-2-heikki.krogerus@linux.intel.com>
 <20220325164255.GA12710@wunner.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220325164255.GA12710@wunner.de>
Precedence: bulk

Hi Lukas,

On Fri, Mar 25, 2022 at 05:42:55PM +0100, Lukas Wunner wrote:
> Hi Heikki,
> 
> saw this linked in your WSR and felt compelled to reply... ;)
> 
> On Thu, Oct 29, 2020 at 01:59:39PM +0300, Heikki Krogerus wrote:
> > +static int software_node_runtime_suspend(struct device *dev)
> > +{
> > +	struct swnode_pm_domain *domain = to_swnode_pm_domain(dev->pm_domain);
> > +	struct swnode *swnode = dev_to_swnode(dev);
> > +	int ret;
> > +
> > +	if (domain->primary && domain->primary->ops.runtime_suspend)
> > +		ret = domain->primary->ops.runtime_suspend(dev);
> > +	else if (dev->type && dev->type->pm && dev->type->pm->runtime_suspend)
> > +		ret = dev->type->pm->runtime_suspend(dev);
> > +	else if (dev->class && dev->class->pm && dev->class->pm->runtime_suspend)
> > +		ret = dev->class->pm->runtime_suspend(dev);
> > +	else if (dev->bus && dev->bus->pm && dev->bus->pm->runtime_suspend)
> > +		ret = dev->bus->pm->runtime_suspend(dev);
> > +	else
> > +		ret = pm_generic_runtime_suspend(dev);
> 
> This if/else ladder seems to be duplicated for every single PM callback
> in this patch.
> 
> Code size can be reduced significantly if you use offsetof() to determine
> the offset of the given callback in struct pm_ops, then pass that offset
> to a helper which contains the above-quoted if/else ladder and retrieves
> the callback.  Finally invoke the callback you've just retrieved.

I think Sakari already suggested that. I'll improve this part in the
next version.

thanks,

-- 
heikki