On Fri, 25 Aug 2023 18:47:14 +0530
Nipun Gupta <[email protected]> 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.
>
> Signed-off-by: Nipun Gupta <[email protected]>
> Reviewed-by: Pieter Jansen van Vuuren <[email protected]>
> ---
>
> Changes v6->v7:
> - None
>
> Changes v5->v6:
> - change cdx_clear_master() to int return type
>
> Changes v4->v5:
> - No change in this patch, patch 2/3 and patch 3/3 are updated
>
> Changes v3->v4:
> - Added user of the Bus master enable and disable APIs in patch 2/2.
> There is no change in this patch.
>
> Changes v2->v3:
> - Changed return value from EOPNOTSUPP to -EOPNOTSUPP in
> cdx_set_master()
>
> Changes v1->v2:
> - Replace bme with bus_master_enable
> - Added check for dev_configure API callback
> - remove un-necessary error prints
> - changed conditional to if-else
> - updated commit message to use 72 columns
>
> drivers/cdx/cdx.c | 32 ++++++++++++++
> drivers/cdx/controller/cdx_controller.c | 4 ++
> drivers/cdx/controller/mcdi_functions.c | 58 +++++++++++++++++++++++++
> drivers/cdx/controller/mcdi_functions.h | 13 ++++++
> include/linux/cdx/cdx_bus.h | 18 ++++++++
> 5 files changed, 125 insertions(+)
>
> diff --git a/drivers/cdx/cdx.c b/drivers/cdx/cdx.c
> index d2cad4c670a0..9efb7584f952 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 = -EOPNOTSUPP;
> +
> + dev_config.type = CDX_DEV_BUS_MASTER_CONF;
> + dev_config.bus_master_enable = true;
> + if (cdx->ops->dev_configure)
> + ret = cdx->ops->dev_configure(cdx, cdx_dev->bus_num,
> + cdx_dev->dev_num, &dev_config);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(cdx_set_master);
> +
> +int cdx_clear_master(struct cdx_device *cdx_dev)
> +{
> + struct cdx_controller *cdx = cdx_dev->cdx;
> + struct cdx_device_config dev_config;
> + int ret = -EOPNOTSUPP;
> +
> + dev_config.type = CDX_DEV_BUS_MASTER_CONF;
> + dev_config.bus_master_enable = false;
> + if (cdx->ops->dev_configure)
> + ret = cdx->ops->dev_configure(cdx, cdx_dev->bus_num,
> + cdx_dev->dev_num, &dev_config);
> +
> + return ret;
> +}
> +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..39aa569d8e07 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->bus_master_enable);
> + break;
> default:
> ret = -EINVAL;
> }
> diff --git a/drivers/cdx/controller/mcdi_functions.c b/drivers/cdx/controller/mcdi_functions.c
> index 0158f26533dd..6acd8fea4586 100644
> --- a/drivers/cdx/controller/mcdi_functions.c
> +++ b/drivers/cdx/controller/mcdi_functions.c
> @@ -137,3 +137,61 @@ 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);
> + u32 flags;
> + int ret;
> +
> + /*
> + * Get flags and then set/reset BUS_MASTER_BIT according to
> + * the input params.
> + */
Nit, this function isn't specific to the BUS_MASTER_BIT, but this
comment is. s/BUS_MASTER_BIT/lbn bit/? Thanks,
Alex
> + ret = cdx_mcdi_ctrl_flag_get(cdx, bus_num, dev_num, &flags);
> + if (ret)
> + return ret;
> +
> + flags = flags & (u32)(~(BIT(lbn)));
> + if (enable)
> + flags |= (1 << 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..8320ec3b9e37 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 bus_master_enable;
> };
>
> typedef int (*cdx_scan_cb)(struct cdx_controller *cdx);
> @@ -170,4 +172,20 @@ 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
> + *
> + * Return: 0 for success, -errno on failure
> + */
> +int cdx_clear_master(struct cdx_device *cdx_dev);
> +
> #endif /* _CDX_BUS_H_ */