linux-kernel - Re: [PATCH v1 1/1] driver core: Move fw

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Thu, 22 Feb 2024 16:54:39 -0800
From: Saravana Kannan <saravanak@...gle.com>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>, linux-kernel@...r.kernel.org, 
	linux-acpi@...r.kernel.org, "Rafael J. Wysocki" <rafael@...nel.org>, 
	Daniel Scally <djrscally@...il.com>, Heikki Krogerus <heikki.krogerus@...ux.intel.com>, 
	Sakari Ailus <sakari.ailus@...ux.intel.com>, Len Brown <lenb@...nel.org>
Subject: Re: [PATCH v1 1/1] driver core: Move fw_devlink stuff to where it belongs

On Wed, Feb 21, 2024 at 4:49 AM Andy Shevchenko
<andriy.shevchenko@...ux.intel.com> wrote:
>
> On Wed, Feb 21, 2024 at 02:48:52PM +0200, Andy Shevchenko wrote:
> > On Tue, Feb 20, 2024 at 06:08:56PM -0800, Saravana Kannan wrote:
> > > On Tue, Feb 20, 2024 at 8:10 AM Andy Shevchenko
> > > <andriy.shevchenko@...ux.intel.com> wrote:
> > > >
> > > > A few APIs that belong specifically to the fw_devlink APIs
> > > > - are exposed to others without need
> > > > - prevents device property code to be cleaned up in the future
> > > >
> > > > Resolve this mess by moving fw_devlink code to where it belongs
> > > > and hide from others.
>
> ...
>
> > > The rest of the functions here are related to parents and children of
> > > a fwnode. So, why is this function considered to be in the wrong
> > > place?
> >
> > When devlink was added it made a few fields in struct fwnode_handle.
> > These fields have no common grounds with device properties. In particular
> > struct device pointer is solely for devlinks and shouldn't be used with
> > them. Hence this patch. TL;DR: they semantically do _not_ belong to
> > the device property APIs.

But fwnode_is_ancestor_of() uses none of those new fields and seems
like a very reasonable API to provide. I understand if you want to
make the "device link only" argument for fwnode_get_next_parent_dev().

> On top of that for the 4+ years no new users appeared, so exporting them was
> a clear mistake. Hence Fixes tags.

We have plenty of APIs in .h files (not the same as export to modules)
-- that have only 1 user. And even some with no users. You don't move
a string function out of lib/string.c just because there's only one
user of that function. We put functions in C files that make sense.

I think Fixes is a bit of an overkill. It's not a bug. Fixes get
propagated to LTS. This is certainly not LTS worthy. I'm not going to
NACK or push back on this patch for these reasons, but just letting
you know that you come off as unreasonably grumpy when you do these
things even for fwnode_is_ancestor_of() :)

-Saravana