Hello Pascal,
The patch looks mostly good, just a few comments below.
On Wed, Jul 31, 2019 at 05:29:18PM +0200, Pascal van Leeuwen wrote:
> @@ -381,10 +383,11 @@ static int safexcel_hw_init(struct safexcel_crypto_priv *priv)
> EIP197_HIA_DxE_CFG_MAX_DATA_SIZE(8);
> val |= EIP197_HIA_DxE_CFG_DATA_CACHE_CTRL(WR_CACHE_3BITS);
> val |= EIP197_HIA_DSE_CFG_ALWAYS_BUFFERABLE;
> - /* FIXME: instability issues can occur for EIP97 but disabling it impact
> - * performances.
> + /*
> + * FIXME: instability issues can occur for EIP97 but disabling
> + * it impacts performance.
> */
No need to change the comment style here.
> @@ -514,7 +517,8 @@ void safexcel_dequeue(struct safexcel_crypto_priv *priv, int ring)
> struct safexcel_context *ctx;
> int ret, nreq = 0, cdesc = 0, rdesc = 0, commands, results;
>
> - /* If a request wasn't properly dequeued because of a lack of resources,
> + /*
> + * If a request wasn't properly dequeued because of a lack of resources,
> * proceeded it first,
> */
Ditto.
> @@ -543,7 +547,8 @@ void safexcel_dequeue(struct safexcel_crypto_priv *priv, int ring)
>
> - /* In case the send() helper did not issue any command to push
> + /*
> + * In case the send() helper did not issue any command to push
Ditto.
> - /* Not enough resources to handle all the requests. Bail out and save
> + /*
> + * Not enough resources to handle all the requests. Bail out and save
Ditto.
> @@ -731,7 +738,8 @@ static inline void safexcel_handle_result_descriptor(struct safexcel_crypto_priv
> EIP197_xDR_PROC_xD_COUNT(tot_descs * priv->config.rd_offset),
> EIP197_HIA_RDR(priv, ring) + EIP197_HIA_xDR_PROC_COUNT);
>
> - /* If the number of requests overflowed the counter, try to proceed more
> + /*
> + * If the number of requests overflowed the counter, try to proceed more
Ditto.
> +#if IS_ENABLED(CONFIG_OF)
> +/*
> + * for Device Tree platform driver
> + */
Single line comment should be:
/* comment */
> +#if IS_ENABLED(CONFIG_PCI)
> +/*
> + * PCIE devices - i.e. Inside Secure development boards
> + */
Here as well.
> +static int crypto_is_pci_probe(struct pci_dev *pdev,
> + const struct pci_device_id *ent)
The whole driver uses the "safexcel_" prefix for functions. You should
use it here as well for consistency.
> +void crypto_is_pci_remove(struct pci_dev *pdev)
Here as well.
> +static const struct pci_device_id crypto_is_pci_ids[] = {
Here as well.
> +static struct pci_driver crypto_is_pci_driver = {
Here as well.
> +static int __init crypto_is_init(void)
Here as well.
> +static void __exit crypto_is_exit(void)
Here as well.
Thanks,
Antoine
--
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
