Received-SPF: pass (google.com: domain of linux-kernel+bounces-104065-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Fri, 15 Mar 2024 10:48:28 +0530
From: Dhruva Gole <d-gole@ti.com>
To: "Rafael J. Wysocki" <rafael@kernel.org>
CC: Tony Lindgren <tony@atomide.com>, <theo.lebrun@bootlin.com>,
        Sibi Sankar
	<quic_sibis@quicinc.com>,
        Viresh Kumar <viresh.kumar@linaro.org>, <linux-pm@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, Ulf Hansson
	<ulf.hansson@linaro.org>
Subject: Re: [PATCH] PM: wakeup: Add a missing return case in init_wakeup
Message-ID: <20240315051828.su2qx57ecyhv3up3@dhruva>
References: <20240314075429.1164810-1-d-gole@ti.com>
 <CAJZ5v0iUt+tudMV-rJ80GR6iNQVv06+054h0UyNgpEOUF6QLRQ@mail.gmail.com>
 <20240314151846.u3r3eaklrl3hf7pi@dhruva>
 <CAJZ5v0gwkKa+AYgOwydzsKjo=_M56t88PwVo7R+fe-53abAdVw@mail.gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAJZ5v0gwkKa+AYgOwydzsKjo=_M56t88PwVo7R+fe-53abAdVw@mail.gmail.com>

On Mar 14, 2024 at 16:29:36 +0100, Rafael J. Wysocki wrote:
> On Thu, Mar 14, 2024 at 4:18 PM Dhruva Gole <d-gole@ti.com> wrote:
> >
> > Hi,
> >
> > On Mar 14, 2024 at 15:01:36 +0100, Rafael J. Wysocki wrote:
> > > On Thu, Mar 14, 2024 at 8:55 AM Dhruva Gole <d-gole@ti.com> wrote:
> > > >
> > > > The device_wakeup_disable call can return an error if no dev exists
> > > > however this was being ignored. Catch this return value and propagate it
> > > > onward in device_init_wakeup.
> > >
> > > Why does this matter to the callers of device_init_wakeup()?
> >
> > If atall !dev->power.can_wakeup then the caller should know something is
> > funny right?
> 
> What would the caller do with this information?
> 
> They attempted to disable wakeup on a device that doesn't exist or is
> not wake-capable, and so what?

Using drivers/char/hw_random/xgene-rng.c as an example, we can atleast
print a warning or something to make the user aware of an unclean state.

Is the argument here that if the caller can't do anything meaningful
then what's the point of returning any error?

If so, then my preference would be just to propagate as much information
upward from the stack whether the caller can make use of that error and
in what way is upto the caller, if nothing else then even a warn or
error print is still useful piece of information.

However if it's useless to return anything from device_wakeup_disable
then we might as well make that function a void or something and avoid
any confusion as to if there's any point in returning error at that
point.

-- 
Best regards,
Dhruva Gole <d-gole@ti.com>