Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Fri, 25 Mar 2022 17:42:55 +0100
From:   Lukas Wunner <lukas@wunner.de>
To:     Heikki Krogerus <heikki.krogerus@linux.intel.com>
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: <20220325164255.GA12710@wunner.de>
References: <20201029105941.63410-1-heikki.krogerus@linux.intel.com>
 <20201029105941.63410-2-heikki.krogerus@linux.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20201029105941.63410-2-heikki.krogerus@linux.intel.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

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.

That way you only need the if/else ladder once in your patch.

For an example, see pcie_port_device_iter() and its callers
pcie_port_device_suspend() and so on:

https://elixir.bootlin.com/linux/latest/source/drivers/pci/pcie/portdrv_core.c#L372

Thanks,

Lukas