cdx: add support for bus mastering

Message ID 20230718100651.18317-1-nipun.gupta@amd.com
State New
Headers
Series cdx: add support for bus mastering |

Commit Message

Gupta, Nipun July 18, 2023, 10:06 a.m. UTC
  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.

Signed-off-by: Nipun Gupta <nipun.gupta@amd.com>
Reviewed-by: Pieter Jansen van Vuuren <pieter.jansen-van-vuuren@amd.com>
---
 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(+)
  

Comments

Greg KH July 18, 2023, 1:46 p.m. UTC | #1
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?

> Signed-off-by: Nipun Gupta <nipun.gupta@amd.com>
> Reviewed-by: Pieter Jansen van Vuuren <pieter.jansen-van-vuuren@amd.com>
> ---
>  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"?

And are you sure that the config can be on the stack?

> +	ret = cdx->ops->dev_configure(cdx, cdx_dev->bus_num,
> +				      cdx_dev->dev_num, &dev_config);

Will dev_configure always be there?

> +	if (ret)
> +		dev_err(&cdx_dev->dev, "device master enable failed\n");

Why error out here, shouldn't the call have printed something if it
wanted to?

> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(cdx_set_master);
> +
> +void cdx_clear_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 = false;
> +	ret = cdx->ops->dev_configure(cdx, cdx_dev->bus_num,
> +				      cdx_dev->dev_num, &dev_config);
> +	if (ret)
> +		dev_err(&cdx_dev->dev, "device master disable failed\n");

Same question as above.

> +}
> +EXPORT_SYMBOL_GPL(cdx_clear_master);
> +
>  /**
>   * cdx_bus_match - device to driver matching callback
>   * @dev: the cdx device to match against
> diff --git a/drivers/cdx/controller/cdx_controller.c b/drivers/cdx/controller/cdx_controller.c
> index dc52f95f8978..ba670f0427d3 100644
> --- a/drivers/cdx/controller/cdx_controller.c
> +++ b/drivers/cdx/controller/cdx_controller.c
> @@ -55,6 +55,10 @@ static int cdx_configure_device(struct cdx_controller *cdx,
>  	case CDX_DEV_RESET_CONF:
>  		ret = cdx_mcdi_reset_device(cdx->priv, bus_num, dev_num);
>  		break;
> +	case CDX_DEV_BUS_MASTER_CONF:
> +		ret = cdx_mcdi_bus_master_enable(cdx->priv, bus_num, dev_num,
> +						 dev_config->bme);
> +		break;
>  	default:
>  		ret = -EINVAL;
>  	}
> diff --git a/drivers/cdx/controller/mcdi_functions.c b/drivers/cdx/controller/mcdi_functions.c
> index 0158f26533dd..c4c00f376006 100644
> --- a/drivers/cdx/controller/mcdi_functions.c
> +++ b/drivers/cdx/controller/mcdi_functions.c
> @@ -137,3 +137,60 @@ int cdx_mcdi_reset_device(struct cdx_mcdi *cdx, u8 bus_num, u8 dev_num)
>  
>  	return ret;
>  }
> +
> +static int cdx_mcdi_ctrl_flag_get(struct cdx_mcdi *cdx, u8 bus_num,
> +				  u8 dev_num, u32 *flags)
> +{
> +	MCDI_DECLARE_BUF(inbuf, MC_CMD_CDX_DEVICE_CONTROL_GET_IN_LEN);
> +	MCDI_DECLARE_BUF(outbuf, MC_CMD_CDX_DEVICE_CONTROL_GET_OUT_LEN);

How big are these buffers?

> +	size_t outlen;
> +	int ret;
> +
> +	MCDI_SET_DWORD(inbuf, CDX_DEVICE_CONTROL_GET_IN_BUS, bus_num);
> +	MCDI_SET_DWORD(inbuf, CDX_DEVICE_CONTROL_GET_IN_DEVICE, dev_num);
> +	ret = cdx_mcdi_rpc(cdx, MC_CMD_CDX_DEVICE_CONTROL_GET, inbuf,
> +			   sizeof(inbuf), outbuf, sizeof(outbuf), &outlen);
> +	if (ret)
> +		return ret;
> +
> +	if (outlen != MC_CMD_CDX_DEVICE_CONTROL_GET_OUT_LEN)
> +		return -EIO;
> +
> +	*flags = MCDI_DWORD(outbuf, CDX_DEVICE_CONTROL_GET_OUT_FLAGS);
> +
> +	return 0;
> +}
> +
> +static int cdx_mcdi_ctrl_flag_set(struct cdx_mcdi *cdx, u8 bus_num,
> +				  u8 dev_num, bool enable, int lbn)
> +{
> +	MCDI_DECLARE_BUF(inbuf, MC_CMD_CDX_DEVICE_CONTROL_SET_IN_LEN);
> +	int ret, en;
> +	u32 flags;
> +
> +	/*
> +	 * Get flags and then set/reset BUS_MASTER_BIT according to
> +	 * the input params.
> +	 */
> +	ret = cdx_mcdi_ctrl_flag_get(cdx, bus_num, dev_num, &flags);
> +	if (ret)
> +		return ret;
> +
> +	en = enable ? 1 : 0;

if/then/else?


> +	flags = (flags & (u32)(~(BIT(lbn)))) | (en << lbn);
> +
> +	MCDI_SET_DWORD(inbuf, CDX_DEVICE_CONTROL_SET_IN_BUS, bus_num);
> +	MCDI_SET_DWORD(inbuf, CDX_DEVICE_CONTROL_SET_IN_DEVICE, dev_num);
> +	MCDI_SET_DWORD(inbuf, CDX_DEVICE_CONTROL_SET_IN_FLAGS, flags);
> +	ret = cdx_mcdi_rpc(cdx, MC_CMD_CDX_DEVICE_CONTROL_SET, inbuf,
> +			   sizeof(inbuf), NULL, 0, NULL);
> +
> +	return ret;
> +}
> +
> +int cdx_mcdi_bus_master_enable(struct cdx_mcdi *cdx, u8 bus_num,
> +			       u8 dev_num, bool enable)
> +{
> +	return cdx_mcdi_ctrl_flag_set(cdx, bus_num, dev_num, enable,
> +			MC_CMD_CDX_DEVICE_CONTROL_SET_IN_BUS_MASTER_ENABLE_LBN);
> +}
> diff --git a/drivers/cdx/controller/mcdi_functions.h b/drivers/cdx/controller/mcdi_functions.h
> index 7440ace5539a..a448d6581eb4 100644
> --- a/drivers/cdx/controller/mcdi_functions.h
> +++ b/drivers/cdx/controller/mcdi_functions.h
> @@ -58,4 +58,17 @@ int cdx_mcdi_get_dev_config(struct cdx_mcdi *cdx,
>  int cdx_mcdi_reset_device(struct cdx_mcdi *cdx,
>  			  u8 bus_num, u8 dev_num);
>  
> +/**
> + * cdx_mcdi_bus_master_enable - Set/Reset bus mastering for cdx device
> + *				represented by bus_num:dev_num
> + * @cdx: pointer to MCDI interface.
> + * @bus_num: Bus number.
> + * @dev_num: Device number.
> + * @enable: Enable bus mastering if set, disable otherwise.
> + *
> + * Return: 0 on success, <0 on failure
> + */
> +int cdx_mcdi_bus_master_enable(struct cdx_mcdi *cdx, u8 bus_num,
> +			       u8 dev_num, bool enable);
> +
>  #endif /* CDX_MCDI_FUNCTIONS_H */
> diff --git a/include/linux/cdx/cdx_bus.h b/include/linux/cdx/cdx_bus.h
> index bead71b7bc73..acb4e40bd20e 100644
> --- a/include/linux/cdx/cdx_bus.h
> +++ b/include/linux/cdx/cdx_bus.h
> @@ -21,11 +21,13 @@
>  struct cdx_controller;
>  
>  enum {
> +	CDX_DEV_BUS_MASTER_CONF,
>  	CDX_DEV_RESET_CONF,
>  };
>  
>  struct cdx_device_config {
>  	u8 type;
> +	bool bme;

What does "bme" mean?

thanks,

greg k-h
  
Gupta, Nipun July 19, 2023, 12:36 p.m. UTC | #2
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@amd.com>
>> Reviewed-by: Pieter Jansen van Vuuren <pieter.jansen-van-vuuren@amd.com>
>> ---
>>   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.

> 
> And are you sure that the config can be on the stack?

Yes, as this information is used by the controller and a new created 
command is sent to the Firmware/Hardware.

> 
>> +	ret = cdx->ops->dev_configure(cdx, cdx_dev->bus_num,
>> +				      cdx_dev->dev_num, &dev_config);
> 
> Will dev_configure always be there?

This is expected to be there, but will add a check for further assurance.

> 
>> +	if (ret)
>> +		dev_err(&cdx_dev->dev, "device master enable failed\n");
> 
> Why error out here, shouldn't the call have printed something if it
> wanted to?

Agree, will remove

> 
>> +
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(cdx_set_master);
>> +
>> +void cdx_clear_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 = false;
>> +	ret = cdx->ops->dev_configure(cdx, cdx_dev->bus_num,
>> +				      cdx_dev->dev_num, &dev_config);
>> +	if (ret)
>> +		dev_err(&cdx_dev->dev, "device master disable failed\n");
> 
> Same question as above.

sure will remove

> 
>> +}
>> +EXPORT_SYMBOL_GPL(cdx_clear_master);
>> +
>>   /**
>>    * cdx_bus_match - device to driver matching callback
>>    * @dev: the cdx device to match against
>> diff --git a/drivers/cdx/controller/cdx_controller.c b/drivers/cdx/controller/cdx_controller.c
>> index dc52f95f8978..ba670f0427d3 100644
>> --- a/drivers/cdx/controller/cdx_controller.c
>> +++ b/drivers/cdx/controller/cdx_controller.c
>> @@ -55,6 +55,10 @@ static int cdx_configure_device(struct cdx_controller *cdx,
>>   	case CDX_DEV_RESET_CONF:
>>   		ret = cdx_mcdi_reset_device(cdx->priv, bus_num, dev_num);
>>   		break;
>> +	case CDX_DEV_BUS_MASTER_CONF:
>> +		ret = cdx_mcdi_bus_master_enable(cdx->priv, bus_num, dev_num,
>> +						 dev_config->bme);
>> +		break;
>>   	default:
>>   		ret = -EINVAL;
>>   	}
>> diff --git a/drivers/cdx/controller/mcdi_functions.c b/drivers/cdx/controller/mcdi_functions.c
>> index 0158f26533dd..c4c00f376006 100644
>> --- a/drivers/cdx/controller/mcdi_functions.c
>> +++ b/drivers/cdx/controller/mcdi_functions.c
>> @@ -137,3 +137,60 @@ int cdx_mcdi_reset_device(struct cdx_mcdi *cdx, u8 bus_num, u8 dev_num)
>>   
>>   	return ret;
>>   }
>> +
>> +static int cdx_mcdi_ctrl_flag_get(struct cdx_mcdi *cdx, u8 bus_num,
>> +				  u8 dev_num, u32 *flags)
>> +{
>> +	MCDI_DECLARE_BUF(inbuf, MC_CMD_CDX_DEVICE_CONTROL_GET_IN_LEN);
>> +	MCDI_DECLARE_BUF(outbuf, MC_CMD_CDX_DEVICE_CONTROL_GET_OUT_LEN);
> 
> How big are these buffers?

The defines are as follows in mc_cdx_pcol.h:
#define MC_CMD_CDX_DEVICE_CONTROL_GET_IN_LEN                   8
#define MC_CMD_CDX_DEVICE_CONTROL_GET_OUT_LEN                  4

where MCDI_DECLARE_BUF leads to creation of a variable of data structure:
/* A doubleword (i.e. 4 byte) datatype - little-endian in HW */
struct cdx_dword {
         __le32 cdx_u32;
};

so inbuf is of 32 bytes (8 * 4) and outbuf is 16 bytes (4 * 4).

> 
>> +	size_t outlen;
>> +	int ret;
>> +
>> +	MCDI_SET_DWORD(inbuf, CDX_DEVICE_CONTROL_GET_IN_BUS, bus_num);
>> +	MCDI_SET_DWORD(inbuf, CDX_DEVICE_CONTROL_GET_IN_DEVICE, dev_num);
>> +	ret = cdx_mcdi_rpc(cdx, MC_CMD_CDX_DEVICE_CONTROL_GET, inbuf,
>> +			   sizeof(inbuf), outbuf, sizeof(outbuf), &outlen);
>> +	if (ret)
>> +		return ret;
>> +
>> +	if (outlen != MC_CMD_CDX_DEVICE_CONTROL_GET_OUT_LEN)
>> +		return -EIO;
>> +
>> +	*flags = MCDI_DWORD(outbuf, CDX_DEVICE_CONTROL_GET_OUT_FLAGS);
>> +
>> +	return 0;
>> +}
>> +
>> +static int cdx_mcdi_ctrl_flag_set(struct cdx_mcdi *cdx, u8 bus_num,
>> +				  u8 dev_num, bool enable, int lbn)
>> +{
>> +	MCDI_DECLARE_BUF(inbuf, MC_CMD_CDX_DEVICE_CONTROL_SET_IN_LEN);
>> +	int ret, en;
>> +	u32 flags;
>> +
>> +	/*
>> +	 * Get flags and then set/reset BUS_MASTER_BIT according to
>> +	 * the input params.
>> +	 */
>> +	ret = cdx_mcdi_ctrl_flag_get(cdx, bus_num, dev_num, &flags);
>> +	if (ret)
>> +		return ret;
>> +
>> +	en = enable ? 1 : 0;
> 
> if/then/else?

okay. will update

> 
> 
>> +	flags = (flags & (u32)(~(BIT(lbn)))) | (en << lbn);
>> +
>> +	MCDI_SET_DWORD(inbuf, CDX_DEVICE_CONTROL_SET_IN_BUS, bus_num);
>> +	MCDI_SET_DWORD(inbuf, CDX_DEVICE_CONTROL_SET_IN_DEVICE, dev_num);
>> +	MCDI_SET_DWORD(inbuf, CDX_DEVICE_CONTROL_SET_IN_FLAGS, flags);
>> +	ret = cdx_mcdi_rpc(cdx, MC_CMD_CDX_DEVICE_CONTROL_SET, inbuf,
>> +			   sizeof(inbuf), NULL, 0, NULL);
>> +
>> +	return ret;
>> +}
>> +
>> +int cdx_mcdi_bus_master_enable(struct cdx_mcdi *cdx, u8 bus_num,
>> +			       u8 dev_num, bool enable)
>> +{
>> +	return cdx_mcdi_ctrl_flag_set(cdx, bus_num, dev_num, enable,
>> +			MC_CMD_CDX_DEVICE_CONTROL_SET_IN_BUS_MASTER_ENABLE_LBN);
>> +}
>> diff --git a/drivers/cdx/controller/mcdi_functions.h b/drivers/cdx/controller/mcdi_functions.h
>> index 7440ace5539a..a448d6581eb4 100644
>> --- a/drivers/cdx/controller/mcdi_functions.h
>> +++ b/drivers/cdx/controller/mcdi_functions.h
>> @@ -58,4 +58,17 @@ int cdx_mcdi_get_dev_config(struct cdx_mcdi *cdx,
>>   int cdx_mcdi_reset_device(struct cdx_mcdi *cdx,
>>   			  u8 bus_num, u8 dev_num);
>>   
>> +/**
>> + * cdx_mcdi_bus_master_enable - Set/Reset bus mastering for cdx device
>> + *				represented by bus_num:dev_num
>> + * @cdx: pointer to MCDI interface.
>> + * @bus_num: Bus number.
>> + * @dev_num: Device number.
>> + * @enable: Enable bus mastering if set, disable otherwise.
>> + *
>> + * Return: 0 on success, <0 on failure
>> + */
>> +int cdx_mcdi_bus_master_enable(struct cdx_mcdi *cdx, u8 bus_num,
>> +			       u8 dev_num, bool enable);
>> +
>>   #endif /* CDX_MCDI_FUNCTIONS_H */
>> diff --git a/include/linux/cdx/cdx_bus.h b/include/linux/cdx/cdx_bus.h
>> index bead71b7bc73..acb4e40bd20e 100644
>> --- a/include/linux/cdx/cdx_bus.h
>> +++ b/include/linux/cdx/cdx_bus.h
>> @@ -21,11 +21,13 @@
>>   struct cdx_controller;
>>   
>>   enum {
>> +	CDX_DEV_BUS_MASTER_CONF,
>>   	CDX_DEV_RESET_CONF,
>>   };
>>   
>>   struct cdx_device_config {
>>   	u8 type;
>> +	bool bme;
> 
> What does "bme" mean?

This is bus master enable bit. Will add comment.

Thanks,
Nipun

> 
> thanks,
> 
> greg k-h
  
Greg KH July 19, 2023, 1:36 p.m. UTC | #3
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@amd.com>
> > > Reviewed-by: Pieter Jansen van Vuuren <pieter.jansen-van-vuuren@amd.com>
> > > ---
> > >   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
  

Patch

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;
+	ret = cdx->ops->dev_configure(cdx, cdx_dev->bus_num,
+				      cdx_dev->dev_num, &dev_config);
+	if (ret)
+		dev_err(&cdx_dev->dev, "device master enable failed\n");
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(cdx_set_master);
+
+void cdx_clear_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 = false;
+	ret = cdx->ops->dev_configure(cdx, cdx_dev->bus_num,
+				      cdx_dev->dev_num, &dev_config);
+	if (ret)
+		dev_err(&cdx_dev->dev, "device master disable failed\n");
+}
+EXPORT_SYMBOL_GPL(cdx_clear_master);
+
 /**
  * cdx_bus_match - device to driver matching callback
  * @dev: the cdx device to match against
diff --git a/drivers/cdx/controller/cdx_controller.c b/drivers/cdx/controller/cdx_controller.c
index dc52f95f8978..ba670f0427d3 100644
--- a/drivers/cdx/controller/cdx_controller.c
+++ b/drivers/cdx/controller/cdx_controller.c
@@ -55,6 +55,10 @@  static int cdx_configure_device(struct cdx_controller *cdx,
 	case CDX_DEV_RESET_CONF:
 		ret = cdx_mcdi_reset_device(cdx->priv, bus_num, dev_num);
 		break;
+	case CDX_DEV_BUS_MASTER_CONF:
+		ret = cdx_mcdi_bus_master_enable(cdx->priv, bus_num, dev_num,
+						 dev_config->bme);
+		break;
 	default:
 		ret = -EINVAL;
 	}
diff --git a/drivers/cdx/controller/mcdi_functions.c b/drivers/cdx/controller/mcdi_functions.c
index 0158f26533dd..c4c00f376006 100644
--- a/drivers/cdx/controller/mcdi_functions.c
+++ b/drivers/cdx/controller/mcdi_functions.c
@@ -137,3 +137,60 @@  int cdx_mcdi_reset_device(struct cdx_mcdi *cdx, u8 bus_num, u8 dev_num)
 
 	return ret;
 }
+
+static int cdx_mcdi_ctrl_flag_get(struct cdx_mcdi *cdx, u8 bus_num,
+				  u8 dev_num, u32 *flags)
+{
+	MCDI_DECLARE_BUF(inbuf, MC_CMD_CDX_DEVICE_CONTROL_GET_IN_LEN);
+	MCDI_DECLARE_BUF(outbuf, MC_CMD_CDX_DEVICE_CONTROL_GET_OUT_LEN);
+	size_t outlen;
+	int ret;
+
+	MCDI_SET_DWORD(inbuf, CDX_DEVICE_CONTROL_GET_IN_BUS, bus_num);
+	MCDI_SET_DWORD(inbuf, CDX_DEVICE_CONTROL_GET_IN_DEVICE, dev_num);
+	ret = cdx_mcdi_rpc(cdx, MC_CMD_CDX_DEVICE_CONTROL_GET, inbuf,
+			   sizeof(inbuf), outbuf, sizeof(outbuf), &outlen);
+	if (ret)
+		return ret;
+
+	if (outlen != MC_CMD_CDX_DEVICE_CONTROL_GET_OUT_LEN)
+		return -EIO;
+
+	*flags = MCDI_DWORD(outbuf, CDX_DEVICE_CONTROL_GET_OUT_FLAGS);
+
+	return 0;
+}
+
+static int cdx_mcdi_ctrl_flag_set(struct cdx_mcdi *cdx, u8 bus_num,
+				  u8 dev_num, bool enable, int lbn)
+{
+	MCDI_DECLARE_BUF(inbuf, MC_CMD_CDX_DEVICE_CONTROL_SET_IN_LEN);
+	int ret, en;
+	u32 flags;
+
+	/*
+	 * Get flags and then set/reset BUS_MASTER_BIT according to
+	 * the input params.
+	 */
+	ret = cdx_mcdi_ctrl_flag_get(cdx, bus_num, dev_num, &flags);
+	if (ret)
+		return ret;
+
+	en = enable ? 1 : 0;
+	flags = (flags & (u32)(~(BIT(lbn)))) | (en << lbn);
+
+	MCDI_SET_DWORD(inbuf, CDX_DEVICE_CONTROL_SET_IN_BUS, bus_num);
+	MCDI_SET_DWORD(inbuf, CDX_DEVICE_CONTROL_SET_IN_DEVICE, dev_num);
+	MCDI_SET_DWORD(inbuf, CDX_DEVICE_CONTROL_SET_IN_FLAGS, flags);
+	ret = cdx_mcdi_rpc(cdx, MC_CMD_CDX_DEVICE_CONTROL_SET, inbuf,
+			   sizeof(inbuf), NULL, 0, NULL);
+
+	return ret;
+}
+
+int cdx_mcdi_bus_master_enable(struct cdx_mcdi *cdx, u8 bus_num,
+			       u8 dev_num, bool enable)
+{
+	return cdx_mcdi_ctrl_flag_set(cdx, bus_num, dev_num, enable,
+			MC_CMD_CDX_DEVICE_CONTROL_SET_IN_BUS_MASTER_ENABLE_LBN);
+}
diff --git a/drivers/cdx/controller/mcdi_functions.h b/drivers/cdx/controller/mcdi_functions.h
index 7440ace5539a..a448d6581eb4 100644
--- a/drivers/cdx/controller/mcdi_functions.h
+++ b/drivers/cdx/controller/mcdi_functions.h
@@ -58,4 +58,17 @@  int cdx_mcdi_get_dev_config(struct cdx_mcdi *cdx,
 int cdx_mcdi_reset_device(struct cdx_mcdi *cdx,
 			  u8 bus_num, u8 dev_num);
 
+/**
+ * cdx_mcdi_bus_master_enable - Set/Reset bus mastering for cdx device
+ *				represented by bus_num:dev_num
+ * @cdx: pointer to MCDI interface.
+ * @bus_num: Bus number.
+ * @dev_num: Device number.
+ * @enable: Enable bus mastering if set, disable otherwise.
+ *
+ * Return: 0 on success, <0 on failure
+ */
+int cdx_mcdi_bus_master_enable(struct cdx_mcdi *cdx, u8 bus_num,
+			       u8 dev_num, bool enable);
+
 #endif /* CDX_MCDI_FUNCTIONS_H */
diff --git a/include/linux/cdx/cdx_bus.h b/include/linux/cdx/cdx_bus.h
index bead71b7bc73..acb4e40bd20e 100644
--- a/include/linux/cdx/cdx_bus.h
+++ b/include/linux/cdx/cdx_bus.h
@@ -21,11 +21,13 @@ 
 struct cdx_controller;
 
 enum {
+	CDX_DEV_BUS_MASTER_CONF,
 	CDX_DEV_RESET_CONF,
 };
 
 struct cdx_device_config {
 	u8 type;
+	bool bme;
 };
 
 typedef int (*cdx_scan_cb)(struct cdx_controller *cdx);
@@ -170,4 +172,18 @@  extern struct bus_type cdx_bus_type;
  */
 int cdx_dev_reset(struct device *dev);
 
+/**
+ * cdx_set_master - enables bus-mastering for CDX device
+ * @cdx_dev: the CDX device to enable
+ *
+ * Return: 0 for success, -errno on failure
+ */
+int cdx_set_master(struct cdx_device *cdx_dev);
+
+/**
+ * cdx_clear_master - disables bus-mastering for CDX device
+ * @cdx_dev: the CDX device to disable
+ */
+void cdx_clear_master(struct cdx_device *cdx_dev);
+
 #endif /* _CDX_BUS_H_ */