Re: [PATCH] cdx: add support for bus mastering

[Greg KH]

On Wed, Jul 19, 2023 at 06:06:40PM +0530, Gupta, Nipun wrote:
> 
> 
> On 7/18/2023 7:16 PM, Greg KH wrote:
> > On Tue, Jul 18, 2023 at 03:36:51PM +0530, Nipun Gupta wrote:
> > > Introduce cdx_set_master() and cdx_clear_master() APIs
> > > to support enable and disable of bus mastering. Drivers
> > > need to use these APIs to enable/disable DMAs from the
> > > CDX devices.
> > 
> > You do have a full 72 columns, why not use that?
> 
> sure, will update accordingly.
> 
> > 
> > > Signed-off-by: Nipun Gupta <nipun.gupta@xxxxxxx>
> > > Reviewed-by: Pieter Jansen van Vuuren <pieter.jansen-van-vuuren@xxxxxxx>
> > > ---
> > >   drivers/cdx/cdx.c                       | 32 ++++++++++++++
> > >   drivers/cdx/controller/cdx_controller.c |  4 ++
> > >   drivers/cdx/controller/mcdi_functions.c | 57 +++++++++++++++++++++++++
> > >   drivers/cdx/controller/mcdi_functions.h | 13 ++++++
> > >   include/linux/cdx/cdx_bus.h             | 16 +++++++
> > >   5 files changed, 122 insertions(+)
> > > 
> > > diff --git a/drivers/cdx/cdx.c b/drivers/cdx/cdx.c
> > > index d2cad4c670a0..efb24672b7d9 100644
> > > --- a/drivers/cdx/cdx.c
> > > +++ b/drivers/cdx/cdx.c
> > > @@ -182,6 +182,38 @@ cdx_match_id(const struct cdx_device_id *ids, struct cdx_device *dev)
> > >   	return NULL;
> > >   }
> > > +int cdx_set_master(struct cdx_device *cdx_dev)
> > > +{
> > > +	struct cdx_controller *cdx = cdx_dev->cdx;
> > > +	struct cdx_device_config dev_config;
> > > +	int ret;
> > > +
> > > +	dev_config.type = CDX_DEV_BUS_MASTER_CONF;
> > > +	dev_config.bme = true;
> > 
> > What is "bme"?
> 
> This is bus master enable. I will add a comment on the structure definition.

Better yet, spell it out "bus_master_enable" so no one has to look up
the comment, no need to try to make cryptic variable names :)

thanks,

greg k-h