[PATCH 1/3] crypto: inside-secure - Expand soc data structure

[PATCH 1/3] crypto: inside-secure - Expand soc data structure

[Peter Harliman Liem]

Currently platform data is assigned directly to
version string(instead of struct). To make it more
scalable, we move it to use data struct instead.
This allows customization for individual platforms other
than version string.

Signed-off-by: Peter Harliman Liem <pliem@maxlinear.com>
---
 drivers/crypto/inside-secure/safexcel.c | 49 +++++++++++++++++--------
 drivers/crypto/inside-secure/safexcel.h |  6 ++-
 2 files changed, 38 insertions(+), 17 deletions(-)

diff --git a/drivers/crypto/inside-secure/safexcel.c b/drivers/crypto/inside-secure/safexcel.c
index ad0d8c4a71ac..02c103da09a9 100644
--- a/drivers/crypto/inside-secure/safexcel.c
+++ b/drivers/crypto/inside-secure/safexcel.c
@@ -410,20 +410,25 @@ static int eip197_load_firmwares(struct safexcel_crypto_priv *priv)
 	int i, j, ret = 0, pe;
 	int ipuesz, ifppsz, minifw = 0;
 
-	if (priv->version == EIP197D_MRVL)
-		dir = "eip197d";
-	else if (priv->version == EIP197B_MRVL ||
-		 priv->version == EIP197_DEVBRD)
+	switch (priv->data->version) {
+	case EIP197_DEVBRD:
+	case EIP197B_MRVL:
 		dir = "eip197b";
-	else
-		return -ENODEV;
+		break;
+	case EIP197D_MRVL:
+		dir = "eip197d";
+		break;
+	default:
+		/* generic case */
+		dir = "";
+	}
 
 retry_fw:
 	for (i = 0; i < FW_NB; i++) {
 		snprintf(fw_path, 37, "inside-secure/%s/%s", dir, fw_name[i]);
 		ret = firmware_request_nowarn(&fw[i], fw_path, priv->dev);
 		if (ret) {
-			if (minifw || priv->version != EIP197B_MRVL)
+			if (minifw || priv->data->version != EIP197B_MRVL)
 				goto release_fw;
 
 			/* Fallback to the old firmware location for the
@@ -1597,7 +1602,7 @@ static int safexcel_probe_generic(void *pdev,
 
 	safexcel_configure(priv);
 
-	if (IS_ENABLED(CONFIG_PCI) && priv->version == EIP197_DEVBRD) {
+	if (IS_ENABLED(CONFIG_PCI) && priv->data->version == EIP197_DEVBRD) {
 		/*
 		 * Request MSI vectors for global + 1 per ring -
 		 * or just 1 for older dev images
@@ -1731,7 +1736,7 @@ static int safexcel_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	priv->dev = dev;
-	priv->version = (enum safexcel_eip_version)of_device_get_match_data(dev);
+	priv->data = (struct safexcel_of_data *)of_device_get_match_data(dev);
 
 	platform_set_drvdata(pdev, priv);
 
@@ -1806,27 +1811,39 @@ static int safexcel_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct safexcel_of_data eip97ies_mrvl_data = {
+	.version = EIP97IES_MRVL,
+};
+
+static const struct safexcel_of_data eip197b_mrvl_data = {
+	.version = EIP197B_MRVL,
+};
+
+static const struct safexcel_of_data eip197d_mrvl_data = {
+	.version = EIP197D_MRVL,
+};
+
 static const struct of_device_id safexcel_of_match_table[] = {
 	{
 		.compatible = "inside-secure,safexcel-eip97ies",
-		.data = (void *)EIP97IES_MRVL,
+		.data = &eip97ies_mrvl_data,
 	},
 	{
 		.compatible = "inside-secure,safexcel-eip197b",
-		.data = (void *)EIP197B_MRVL,
+		.data = &eip197b_mrvl_data,
 	},
 	{
 		.compatible = "inside-secure,safexcel-eip197d",
-		.data = (void *)EIP197D_MRVL,
+		.data = &eip197d_mrvl_data,
 	},
 	/* For backward compatibility and intended for generic use */
 	{
 		.compatible = "inside-secure,safexcel-eip97",
-		.data = (void *)EIP97IES_MRVL,
+		.data = &eip97ies_mrvl_data,
 	},
 	{
 		.compatible = "inside-secure,safexcel-eip197",
-		.data = (void *)EIP197B_MRVL,
+		.data = &eip197b_mrvl_data,
 	},
 	{},
 };
@@ -1862,7 +1879,7 @@ static int safexcel_pci_probe(struct pci_dev *pdev,
 		return -ENOMEM;
 
 	priv->dev = dev;
-	priv->version = (enum safexcel_eip_version)ent->driver_data;
+	priv->data = (struct safexcel_of_data *)ent->driver_data;
 
 	pci_set_drvdata(pdev, priv);
 
@@ -1881,7 +1898,7 @@ static int safexcel_pci_probe(struct pci_dev *pdev,
 	}
 	priv->base = pcim_iomap_table(pdev)[0];
 
-	if (priv->version == EIP197_DEVBRD) {
+	if (priv->data->version == EIP197_DEVBRD) {
 		dev_dbg(dev, "Device identified as FPGA based development board - applying HW reset\n");
 
 		rc = pcim_iomap_regions(pdev, 4, "crypto_safexcel");
diff --git a/drivers/crypto/inside-secure/safexcel.h b/drivers/crypto/inside-secure/safexcel.h
index 797ff91512e0..1b8ccb33202b 100644
--- a/drivers/crypto/inside-secure/safexcel.h
+++ b/drivers/crypto/inside-secure/safexcel.h
@@ -733,6 +733,10 @@ enum safexcel_eip_version {
 	EIP197_DEVBRD
 };
 
+struct safexcel_of_data {
+	enum safexcel_eip_version version;
+};
+
 /* Priority we use for advertising our algorithms */
 #define SAFEXCEL_CRA_PRIORITY		300
 
@@ -815,7 +819,7 @@ struct safexcel_crypto_priv {
 	struct clk *reg_clk;
 	struct safexcel_config config;
 
-	enum safexcel_eip_version version;
+	struct safexcel_of_data *data;
 	struct safexcel_register_offsets offsets;
 	struct safexcel_hwconfig hwconfig;
 	u32 flags;
-- 
2.17.1

[PATCH 2/3] crypto: inside-secure - Add fw_little_endian option

[Peter Harliman Liem]

This is to add fw_little_endian option, which can
be used for platform which firmware is using little-endian
(instead of big-endian).

Signed-off-by: Peter Harliman Liem <pliem@maxlinear.com>
---
 drivers/crypto/inside-secure/safexcel.c | 15 +++++++++++----
 drivers/crypto/inside-secure/safexcel.h |  1 +
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/inside-secure/safexcel.c b/drivers/crypto/inside-secure/safexcel.c
index 02c103da09a9..955170d2dd7c 100644
--- a/drivers/crypto/inside-secure/safexcel.c
+++ b/drivers/crypto/inside-secure/safexcel.c
@@ -316,14 +316,21 @@ static void eip197_init_firmware(struct safexcel_crypto_priv *priv)
 static int eip197_write_firmware(struct safexcel_crypto_priv *priv,
 				  const struct firmware *fw)
 {
-	const __be32 *data = (const __be32 *)fw->data;
+	const u32 *data = (const u32 *)fw->data;
+	u32 val;
 	int i;
 
 	/* Write the firmware */
-	for (i = 0; i < fw->size / sizeof(u32); i++)
-		writel(be32_to_cpu(data[i]),
+	for (i = 0; i < fw->size / sizeof(u32); i++) {
+		if (priv->data->fw_little_endian)
+			val = le32_to_cpu(data[i]);
+		else
+			val = be32_to_cpu(data[i]);
+
+		writel(val,
 		       priv->base + EIP197_CLASSIFICATION_RAMS +
-		       i * sizeof(__be32));
+		       i * sizeof(*data));
+	}
 
 	/* Exclude final 2 NOPs from size */
 	return i - EIP197_FW_TERMINAL_NOPS;
diff --git a/drivers/crypto/inside-secure/safexcel.h b/drivers/crypto/inside-secure/safexcel.h
index 1b8ccb33202b..a89dd8dc1ddd 100644
--- a/drivers/crypto/inside-secure/safexcel.h
+++ b/drivers/crypto/inside-secure/safexcel.h
@@ -735,6 +735,7 @@ enum safexcel_eip_version {
 
 struct safexcel_of_data {
 	enum safexcel_eip_version version;
+	bool fw_little_endian;
 };
 
 /* Priority we use for advertising our algorithms */
-- 
2.17.1

[PATCH 3/3] crypto: inside-secure - Add MaxLinear platform

[Peter Harliman Liem]

This is to add MaxLinear platform into compatible id.
Firmware endianness option is added since MaxLinear
firmware is in little endian format.

Signed-off-by: Peter Harliman Liem <pliem@maxlinear.com>
---
 drivers/crypto/inside-secure/safexcel.c | 9 +++++++++
 drivers/crypto/inside-secure/safexcel.h | 3 ++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/inside-secure/safexcel.c b/drivers/crypto/inside-secure/safexcel.c
index 955170d2dd7c..4b7c18db290c 100644
--- a/drivers/crypto/inside-secure/safexcel.c
+++ b/drivers/crypto/inside-secure/safexcel.c
@@ -1830,6 +1830,11 @@ static const struct safexcel_of_data eip197d_mrvl_data = {
 	.version = EIP197D_MRVL,
 };
 
+static const struct safexcel_of_data eip197_mxl_data = {
+	.version = EIP197_MXL,
+	.fw_little_endian = true,
+};
+
 static const struct of_device_id safexcel_of_match_table[] = {
 	{
 		.compatible = "inside-secure,safexcel-eip97ies",
@@ -1843,6 +1848,10 @@ static const struct of_device_id safexcel_of_match_table[] = {
 		.compatible = "inside-secure,safexcel-eip197d",
 		.data = &eip197d_mrvl_data,
 	},
+	{
+		.compatible = "inside-secure,safexcel-eip197-mxl",
+		.data = &eip197_mxl_data,
+	},
 	/* For backward compatibility and intended for generic use */
 	{
 		.compatible = "inside-secure,safexcel-eip97",
diff --git a/drivers/crypto/inside-secure/safexcel.h b/drivers/crypto/inside-secure/safexcel.h
index a89dd8dc1ddd..078dbfb2b094 100644
--- a/drivers/crypto/inside-secure/safexcel.h
+++ b/drivers/crypto/inside-secure/safexcel.h
@@ -730,7 +730,8 @@ enum safexcel_eip_version {
 	EIP97IES_MRVL,
 	EIP197B_MRVL,
 	EIP197D_MRVL,
-	EIP197_DEVBRD
+	EIP197_DEVBRD,
+	EIP197_MXL,
 };
 
 struct safexcel_of_data {
-- 
2.17.1

Re: [PATCH 2/3] crypto: inside-secure - Add fw_little_endian option

[kernel test robot]

Hi Peter,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on herbert-cryptodev-2.6/master]
[also build test WARNING on herbert-crypto-2.6/master linus/master v6.0-rc6 next-20220920]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Peter-Harliman-Liem/crypto-inside-secure-Expand-soc-data-structure/20220920-170235
base:   https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
config: parisc-randconfig-s051-20220921 (https://download.01.org/0day-ci/archive/20220921/202209211245.XnqzyM7N-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 12.1.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.4-39-gce1a6720-dirty
        # https://github.com/intel-lab-lkp/linux/commit/637d1b2810d1e9da47b6a637f9cea7c5bb4bf765
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Peter-Harliman-Liem/crypto-inside-secure-Expand-soc-data-structure/20220920-170235
        git checkout 637d1b2810d1e9da47b6a637f9cea7c5bb4bf765
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=parisc SHELL=/bin/bash drivers/crypto/inside-secure/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

sparse warnings: (new ones prefixed by >>)
>> drivers/crypto/inside-secure/safexcel.c:326:31: sparse: sparse: cast to restricted __le32
>> drivers/crypto/inside-secure/safexcel.c:326:31: sparse: sparse: cast to restricted __le32
>> drivers/crypto/inside-secure/safexcel.c:326:31: sparse: sparse: cast to restricted __le32
>> drivers/crypto/inside-secure/safexcel.c:326:31: sparse: sparse: cast to restricted __le32
>> drivers/crypto/inside-secure/safexcel.c:326:31: sparse: sparse: cast to restricted __le32
>> drivers/crypto/inside-secure/safexcel.c:326:31: sparse: sparse: cast to restricted __le32
   drivers/crypto/inside-secure/safexcel.c:328:31: sparse: sparse: cast to restricted __be32

vim +326 drivers/crypto/inside-secure/safexcel.c

   315	
   316	static int eip197_write_firmware(struct safexcel_crypto_priv *priv,
   317					  const struct firmware *fw)
   318	{
   319		const u32 *data = (const u32 *)fw->data;
   320		u32 val;
   321		int i;
   322	
   323		/* Write the firmware */
   324		for (i = 0; i < fw->size / sizeof(u32); i++) {
   325			if (priv->data->fw_little_endian)
 > 326				val = le32_to_cpu(data[i]);
   327			else
   328				val = be32_to_cpu(data[i]);
   329	
   330			writel(val,
   331			       priv->base + EIP197_CLASSIFICATION_RAMS +
   332			       i * sizeof(*data));
   333		}
   334	
   335		/* Exclude final 2 NOPs from size */
   336		return i - EIP197_FW_TERMINAL_NOPS;
   337	}
   338	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH 1/3] crypto: inside-secure - Expand soc data structure

[Antoine Tenart]

Quoting Peter Harliman Liem (2022-09-20 10:01:37)
> @@ -410,20 +410,25 @@ static int eip197_load_firmwares(struct safexcel_crypto_priv *priv)
>         int i, j, ret = 0, pe;
>         int ipuesz, ifppsz, minifw = 0;
>  
> -       if (priv->version == EIP197D_MRVL)
> -               dir = "eip197d";
> -       else if (priv->version == EIP197B_MRVL ||
> -                priv->version == EIP197_DEVBRD)
> +       switch (priv->data->version) {
> +       case EIP197_DEVBRD:
> +       case EIP197B_MRVL:
>                 dir = "eip197b";
> -       else
> -               return -ENODEV;
> +               break;
> +       case EIP197D_MRVL:
> +               dir = "eip197d";
> +               break;
> +       default:
> +               /* generic case */
> +               dir = "";
> +       }

Why "generic case"? We shouldn't end up in this case and the logic
changed after this patch: an error was returned before.

The if vs switch is mostly a question of taste here, I don't have
anything against it but it's also not necessary as we're not supporting
lots of versions. So you could keep it as-is and keep the patch
restricted to its topic.

> +static const struct safexcel_of_data eip97ies_mrvl_data = {
> +       .version = EIP97IES_MRVL,
> +};
> +
> +static const struct safexcel_of_data eip197b_mrvl_data = {
> +       .version = EIP197B_MRVL,
> +};
> +
> +static const struct safexcel_of_data eip197d_mrvl_data = {
> +       .version = EIP197D_MRVL,
> +};
> +
>  static const struct of_device_id safexcel_of_match_table[] = {
>         {
>                 .compatible = "inside-secure,safexcel-eip97ies",
> -               .data = (void *)EIP97IES_MRVL,
> +               .data = &eip97ies_mrvl_data,
>         },
>         {
>                 .compatible = "inside-secure,safexcel-eip197b",
> -               .data = (void *)EIP197B_MRVL,
> +               .data = &eip197b_mrvl_data,
>         },
>         {
>                 .compatible = "inside-secure,safexcel-eip197d",
> -               .data = (void *)EIP197D_MRVL,
> +               .data = &eip197d_mrvl_data,
>         },
>         /* For backward compatibility and intended for generic use */
>         {
>                 .compatible = "inside-secure,safexcel-eip97",
> -               .data = (void *)EIP97IES_MRVL,
> +               .data = &eip97ies_mrvl_data,
>         },
>         {
>                 .compatible = "inside-secure,safexcel-eip197",
> -               .data = (void *)EIP197B_MRVL,
> +               .data = &eip197b_mrvl_data,
>         },
>         {},
>  };

The pci_device_id counterpart update is missing.

> +++ b/drivers/crypto/inside-secure/safexcel.h
> @@ -733,6 +733,10 @@ enum safexcel_eip_version {
>         EIP197_DEVBRD
>  };
>  
> +struct safexcel_of_data {
> +       enum safexcel_eip_version version;
> +};

The driver supports both of and PCI ids, you can rename this to
safexcel_priv_data.

Thanks!
Antoine

Re: [PATCH 2/3] crypto: inside-secure - Add fw_little_endian option

[Antoine Tenart]

Quoting Peter Harliman Liem (2022-09-20 10:01:38)
> This is to add fw_little_endian option, which can
> be used for platform which firmware is using little-endian
> (instead of big-endian).

That's surprising, releasing fw in various endianness.

Cc Pascal who might know.

Re: [PATCH 1/3] crypto: inside-secure - Expand soc data structure

[Peter Harliman Liem]

On 23/9/2022 5:09 pm, Antoine Tenart wrote:
> Quoting Peter Harliman Liem (2022-09-20 10:01:37)
>> +       default:
>> +               /* generic case */
>> +               dir = "";
>> +       }
> 
> Why "generic case"? We shouldn't end up in this case and the logic
> changed after this patch: an error was returned before.
> 
> The if vs switch is mostly a question of taste here, I don't have
> anything against it but it's also not necessary as we're not supporting
> lots of versions. So you could keep it as-is and keep the patch
> restricted to its topic.

This change has been removed in v2.

>>         {
>>                 .compatible = "inside-secure,safexcel-eip197",
>> -               .data = (void *)EIP197B_MRVL,
>> +               .data = &eip197b_mrvl_data,
>>         },
>>         {},
>>  };
> 
> The pci_device_id counterpart update is missing.

I missed that. Thanks for catching.
Updated in v2.

>> +struct safexcel_of_data {
>> +       enum safexcel_eip_version version;
>> +};
> 
> The driver supports both of and PCI ids, you can rename this to
> safexcel_priv_data.

Updated in v2.

Thanks!

-Peter-

Re: [PATCH 2/3] crypto: inside-secure - Add fw_little_endian option

[Peter Harliman Liem]

On 23/9/2022 5:23 pm, Antoine Tenart wrote:
> Quoting Peter Harliman Liem (2022-09-20 10:01:38)
>> This is to add fw_little_endian option, which can
>> be used for platform which firmware is using little-endian
>> (instead of big-endian).
> 
> That's surprising, releasing fw in various endianness.
> 
> Cc Pascal who might know.
> 

Indeed. Would be great if someone could help confirm in case I'm wrong.

Thanks!

-Peter-