Re: [PATCH v6 4/5] device property: Constify fwnode_handle_get()

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>,
	Rob Herring <robh@kernel.org>,
	ACPI Devel Maling List <linux-acpi@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
	<devicetree@vger.kernel.org>, Daniel Scally <djrscally@gmail.com>,
	Heikki Krogerus <heikki.krogerus@linux.intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Rob Herring <robh+dt@kernel.org>,
	Frank Rowand <frowand.list@gmail.com>,
	Len Brown <lenb@kernel.org>
Subject: Re: [PATCH v6 4/5] device property: Constify fwnode_handle_get()
Date: Thu, 14 Apr 2022 16:09:45 +0300	[thread overview]
Message-ID: <YlgdGb/vKfdCXXx1@smile.fi.intel.com> (raw)
In-Reply-To: <YldE6L7MbgS0NXQY@paasikivi.fi.intel.com>

On Thu, Apr 14, 2022 at 12:47:20AM +0300, Sakari Ailus wrote:
> On Wed, Apr 13, 2022 at 09:23:17PM +0300, Andy Shevchenko wrote:
> > On Wed, Apr 13, 2022 at 09:19:28PM +0300, Andy Shevchenko wrote:
> > > On Wed, Apr 13, 2022 at 08:10:22PM +0200, Rafael J. Wysocki wrote:
> > > > On Fri, Apr 8, 2022 at 8:49 PM Andy Shevchenko
> > > > <andriy.shevchenko@linux.intel.com> wrote:

...

> > > > > -struct fwnode_handle *fwnode_handle_get(struct fwnode_handle *fwnode)
> > > > > +struct fwnode_handle *fwnode_handle_get(const struct fwnode_handle *fwnode)
> > > > >  {
> > > 
> > > > >         if (!fwnode_has_op(fwnode, get))
> > > > >                 return fwnode;
> > > 
> > > ^^^^, so it needs a casting, but then we have to comment why is so.
> > 
> > Note, it means that the fwnode parameter either invalid or has no given option.
> > It's not a problem to drop casting in the first case, but the second one should
> > be justified and Sakari wants to be sure that the initial container is not
> > const, which seems can't be achieved even with the original code.
> 
> I wonder if I'm missing something. The fwnode argument originally was not
> const here.

Yes, and our discussion went to the direction of what const qualifier implies
here. I assume that the const means that we do not modify the fwnode object,
while its container is another story which we have no influence on. You, if
I read your messages correctly, insisting that const here implies that the
container object is const as well.

Reading current implementation I see now, that with children APIs we have
two pointers passed, while with parent APIs only a single one. In children
API due to above is easy to use const qualifier for the first argument.
Parent APIs missed that and hence have this problem that we can't constify
their parameters.

to_of_node() expects const parameter while returns non-const container.
Is it a subtle issue there? (I believe it should be consistent then)

This patch and the followed one can be moved without understanding why
we need the non-const parameter there.

-- 
With Best Regards,
Andy Shevchenko