On Thu, Nov 10, 2022 at 09:21:59AM -0800, Dmitry Torokhov wrote: > On Thu, Nov 10, 2022 at 03:42:40PM +0200, Andy Shevchenko wrote: > > On Wed, Nov 09, 2022 at 11:00:29AM -0800, Dmitry Torokhov wrote: > > > On Wed, Nov 09, 2022 at 01:25:06PM +0200, Andy Shevchenko wrote: > > > > On Tue, Nov 08, 2022 at 04:26:50PM -0800, Dmitry Torokhov wrote: ... > > > > > + if (!IS_ERR_OR_NULL(fwnode)) > > > > > > > > I think this is superfluous check. > > > > > > > > Now in the form of this series, you have only a single dev_dbg() that tries to > > > > dereference it. Do we really need to have it there, since every branch has its > > > > own dev_dbg() anyway? > > > > > > As I mentioned, I like to keep this check to show the reader that we > > > should only descend into gpiod_find_by_fwnode() if we have a valid > > > fwnode. It is less about code generation and more about the intent. > > > > Yes, but if fwnode is not found, we have a next check for that. > > No, the check you are talking about is for the GPIO not being located. > It does not have anything to do with fwnode validity. You are relying on > intimate knowledge of gpiod_find_by_fwnode() implementation and the fact > that in the current form it will withstand ERR_PTR-encoded or NULL > fwnode. > > I want to have the source code so clear in its intent so that I can be > woken up in the middle of the night with a huge hangover and still be > able to tell how it is supposed to behave. As you said let's leave it to Bart and Linus. > > I really don't > > think we lose anything by dropping the check and gaining the code generation as > > a side effect. > > This is cold path, happening only on startup. I am not saying that we > want to make it slow unnecessarily, but a condition branch that might > even get optimized out is not something we should be concerned here. Agree, that's why I called it "side effect". -- With Best Regards, Andy Shevchenko
