Re: [dpdk-dev] [PATCH v2 1/8] crypto/bcmfs: add BCMFS driver

[Akhil Goyal <akhil.goyal@nxp.com>]

Hi Vikas,

> +BCMFS crypto PMD depend upon the devices present in the path
> +/sys/bus/platform/devices/fs<version>/<dev_name> on the platform.
> +Each cryptodev PMD instance can be attached to the nodes present
> +in the mentioned path.

It would be good, if you can mention the details about the SDKs which need
To be installed, any kernel dependencies if any.
The device path mentioned is from which rootfs? This looks incomplete documentation.

> diff --git a/doc/guides/cryptodevs/index.rst b/doc/guides/cryptodevs/index.rst
> index a67ed5a28..5d7e028bd 100644
> --- a/doc/guides/cryptodevs/index.rst
> +++ b/doc/guides/cryptodevs/index.rst
> @@ -29,3 +29,4 @@ Crypto Device Drivers
>      qat
>      virtio
>      zuc
> +    bcmfs

It is better to maintain an alphabetical order.

> diff --git a/drivers/crypto/bcmfs/bcmfs_device.c
> b/drivers/crypto/bcmfs/bcmfs_device.c
> new file mode 100644
> index 000000000..47c776de6
> --- /dev/null
> +++ b/drivers/crypto/bcmfs/bcmfs_device.c
> @@ -0,0 +1,256 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(C) 2020 Broadcom.
> + * All rights reserved.
> + */
> +
> +#include <dirent.h>
> +#include <stdbool.h>
> +#include <sys/queue.h>
> +
> +#include <rte_string_fns.h>
> +
> +#include "bcmfs_device.h"
> +#include "bcmfs_logs.h"
> +
> +struct bcmfs_device_attr {
> +	const char name[BCMFS_MAX_PATH_LEN];
> +	const char suffix[BCMFS_DEV_NAME_LEN];
> +	const enum bcmfs_device_type type;
> +	const uint32_t offset;
> +	const uint32_t version;
> +};
> +
> +/* BCMFS supported devices */
> +static struct bcmfs_device_attr dev_table[] = {
> +	{
> +		.name = "fs4",
> +		.suffix = "crypto_mbox",
> +		.type = BCMFS_SYM_FS4,
> +		.offset = 0,
> +		.version = 0x76303031
> +	},
> +	{
> +		.name = "fs5",
> +		.suffix = "mbox",
> +		.type = BCMFS_SYM_FS5,
> +		.offset = 0,
> +		.version = 0x76303032
> +	},
> +	{
> +		/* sentinel */
> +	}
> +};
> +
> +TAILQ_HEAD(fsdev_list, bcmfs_device);
> +static struct fsdev_list fsdev_list = TAILQ_HEAD_INITIALIZER(fsdev_list);
> +
> +static struct bcmfs_device *
> +fsdev_allocate_one_dev(struct rte_vdev_device *vdev,
> +		       char *dirpath,
> +		       char *devname,
> +		       enum bcmfs_device_type dev_type __rte_unused)
> +{
> +	struct bcmfs_device *fsdev;
> +
> +	fsdev = calloc(1, sizeof(*fsdev));

Can we use rte_calloc

> +	if (!fsdev)
> +		return NULL;
> +
> +	if (strlen(dirpath) > sizeof(fsdev->dirname)) {
> +		BCMFS_LOG(ERR, "dir path name is too long");
> +		goto cleanup;
> +	}
> +
> +	if (strlen(devname) > sizeof(fsdev->name)) {
> +		BCMFS_LOG(ERR, "devname is too long");
> +		goto cleanup;
> +	}
> +
> +	strcpy(fsdev->dirname, dirpath);
> +	strcpy(fsdev->name, devname);
> +
> +	fsdev->vdev = vdev;
> +
> +	TAILQ_INSERT_TAIL(&fsdev_list, fsdev, next);
> +
> +	return fsdev;
> +
> +cleanup:
> +	free(fsdev);
> +
> +	return NULL;
> +}
> +

<snip>

> diff --git a/drivers/crypto/meson.build b/drivers/crypto/meson.build
> index a2423507a..8e06d0533 100644
> --- a/drivers/crypto/meson.build
> +++ b/drivers/crypto/meson.build
> @@ -23,7 +23,8 @@ drivers = ['aesni_gcm',
>  	   'scheduler',
>  	   'snow3g',
>  	   'virtio',
> -	   'zuc']
> +	   'zuc',
> +	   'bcmfs']

Please maintain an alphabetical order.