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:   Fri, 23 Sep 2022 11:25:19 +0000
From:   Sakari Ailus <sakari.ailus@linux.intel.com>
To:     Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc:     linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org,
        Daniel Scally <djrscally@gmail.com>,
        Heikki Krogerus <heikki.krogerus@linux.intel.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        kernel test robot <lkp@intel.com>
Subject: Re: [PATCH v1 1/1] device property: Add const qualifier to
 device_get_match_data() parameter
Message-ID: <Yy2Xn+w1ajJ4At+o@paasikivi.fi.intel.com>
References: <20220922135410.49694-1-andriy.shevchenko@linux.intel.com>
 <YyzEHk2TTcsIO0ha@paasikivi.fi.intel.com>
 <Yy2Jph0AAugY/tn/@smile.fi.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Yy2Jph0AAugY/tn/@smile.fi.intel.com>
Precedence: bulk

Hi Andy,

On Fri, Sep 23, 2022 at 01:25:42PM +0300, Andy Shevchenko wrote:
> On Thu, Sep 22, 2022 at 08:22:54PM +0000, Sakari Ailus wrote:
> > On Thu, Sep 22, 2022 at 04:54:10PM +0300, Andy Shevchenko wrote:
> > > Add const qualifier to the device_get_match_data() parameter.
> > > Some of the future users may utilize this function without
> > > forcing the type.
> > 
> > From const to non-const? This is what this patch does, right?
> 
> Right.
> 
> > > All the same, dev_fwnode() may be used with a const qualifier.
> > > 
> > > Reported-by: kernel test robot <lkp@intel.com>
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> > > -struct fwnode_handle *dev_fwnode(struct device *dev)
> > > +struct fwnode_handle *dev_fwnode(const struct device *dev)
> > 
> > If you have const struct device pointer, then the embedded fwnode handle in
> > that object sure is const, too. Isn't it?
> > 
> > If you really have const struct device pointer (where do you?), then I'd
> 
> device_get_match_data() expects the const parameter, but due to dev_fwnode()
> it can't be satisfied. This has been reported by LKP when I tried to submit
> a wrapper:
> https://lore.kernel.org/linux-spi/20220921204520.23984-1-andriy.shevchenko@linux.intel.com/
> 
> Yes, I probably can drop const there, but it will be again awkward to see
> almost all APIs beneath using const and upper one without it for unclear
> (to the reader) reasons.

dev could indeed be const there otherwise, I understand, but it's certainly
not better to force it non-const elsewhere for that.

> 
> > suggest to add another function, dev_fwnode_const() that is otherwise the
> > same except the argument as well as the return value are const.
> 
> Perhaps this is the case, but does it mean we need to duplicate APIs when
> we know we don't modify data? Seems road to bloating the code for peanuts.
> 
> > Or alternatively define it as a macro and use _Generic()?
> 
> Yeah, I have the mixed feelings about _Generic for generic APIs because
> it requires to convert some stuff to macros when type checking of the
> parameters can be missed (if a target takes two or more of them).

It's not uncommon to use wrapper functions in addition to get type checking
done properly.

> 
> It might work here (we have a single parameter), but in general...

If it works here, why not to do it then? :-)

-- 
Sakari Ailus