Re: [PATCH 0/4] driver_core: Auxiliary drvdata helper cleanup

From: Jason Gunthorpe <jgg@ziepe.ca>
To: "David E. Box" <david.e.box@linux.intel.com>
Cc: gregkh@linuxfoundation.org, mustafa.ismail@intel.com,
	shiraz.saleem@intel.com, dledford@redhat.com, leon@kernel.org,
	saeedm@nvidia.com, davem@davemloft.net, kuba@kernel.org,
	vkoul@kernel.org, yung-chuan.liao@linux.intel.com,
	pierre-louis.bossart@linux.intel.com, mst@redhat.com,
	jasowang@redhat.com, andriy.shevchenko@linux.intel.com,
	hdegoede@redhat.com, virtualization@lists.linux-foundation.org,
	alsa-devel@alsa-project.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-rdma@vger.kernel.org
Subject: Re: [PATCH 0/4] driver_core: Auxiliary drvdata helper cleanup
Date: Wed, 22 Dec 2021 13:18:51 -0400	[thread overview]
Message-ID: <20211222171851.GO6467@ziepe.ca> (raw)
In-Reply-To: <35bca887e697597f7b3e1944b3dd7347c6defca1.camel@linux.intel.com>

On Tue, Dec 21, 2021 at 04:48:17PM -0800, David E. Box wrote:
> On Tue, 2021-12-21 at 20:09 -0400, Jason Gunthorpe wrote:
> > On Tue, Dec 21, 2021 at 03:58:48PM -0800, David E. Box wrote:
> > > Depends on "driver core: auxiliary bus: Add driver data helpers" patch [1].
> > > Applies the helpers to all auxiliary device drivers using
> > > dev_(get/set)_drvdata. Drivers were found using the following search:
> > > 
> > >     grep -lr "struct auxiliary_device" $(grep -lr "drvdata" .)
> > > 
> > > Changes were build tested using the following configs:
> > > 
> > >     vdpa/mlx5:       CONFIG_MLX5_VDPA_NET
> > >     net/mlx53:       CONFIG_MLX5_CORE_EN
> > >     soundwire/intel: CONFIG_SOUNDWIRE_INTEL
> > >     RDAM/irdma:      CONFIG_INFINIBAND_IRDMA
> > >                      CONFIG_MLX5_INFINIBAND
> > > 
> > > [1] https://www.spinics.net/lists/platform-driver-x86/msg29940.html 
> > 
> > I have to say I don't really find this to be a big readability
> > improvement.
> 
> I should have referenced the thread [1] discussing the benefit of this change
> since the question was asked and answered already. The idea is that drivers
> shouldn't have to touch the device API directly if they are already using a
> higher level core API (auxiliary bus) that can do that on its behalf.

Driver writers should rarely use the auxilary device type directly, the
should always immediately container_of it to their proper derived
type.

> > Also, what use is 'to_auxiliary_dev()' ? I didn't see any users added..
>
> This was not added by that patch.

It was added by the referenced patch, and seems totally pointless cut
and paste, again because nothing should be using the auxiliary_device
type for anything more than container_of'ing to their own type.

We've been ripping out bus specific APIs in favour of generic ones
(see the work on the DMA API for instance) so this whole concept seems
regressive, particularly when applied to auxiliary bus which does not
have an API of its own.

Jason