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
