Hi Corentin,
On Fri, Aug 26, 2016 at 01:11:34PM +0200, LABBE Corentin wrote:
> Instead of having two global variable, it's better to use a
> private struct. This will permit to remove amd_pdev variable
>
> Signed-off-by: LABBE Corentin <clabbe.montjoie@xxxxxxxxx>
> ---
> drivers/char/hw_random/amd-rng.c | 57 ++++++++++++++++++++++++++--------------
> 1 file changed, 38 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/char/hw_random/amd-rng.c b/drivers/char/hw_random/amd-rng.c
> index 383e197..4ef94e9 100644
> --- a/drivers/char/hw_random/amd-rng.c
> +++ b/drivers/char/hw_random/amd-rng.c
> @@ -47,15 +47,18 @@ static const struct pci_device_id pci_tbl[] = {
> };
> MODULE_DEVICE_TABLE(pci, pci_tbl);
>
> -static struct pci_dev *amd_pdev;
> +struct amd768_priv {
> + struct pci_dev *pcidev;
> + u32 pmbase;
> +};
>
> static int amd_rng_data_present(struct hwrng *rng, int wait)
> {
> - u32 pmbase = (u32)rng->priv;
> + struct amd768_priv *priv = (struct amd768_priv *)rng->priv;
Please remove unnecessary casts...
> int data, i;
>
> for (i = 0; i < 20; i++) {
> - data = !!(inl(pmbase + 0xF4) & 1);
> + data = !!(inl(priv->pmbase + 0xF4) & 1);
> if (data || !wait)
> break;
> udelay(10);
> @@ -65,35 +68,37 @@ static int amd_rng_data_present(struct hwrng *rng, int wait)
>
> static int amd_rng_data_read(struct hwrng *rng, u32 *data)
> {
> - u32 pmbase = (u32)rng->priv;
> + struct amd768_priv *priv = (struct amd768_priv *)rng->priv;
here,
>
> - *data = inl(pmbase + 0xF0);
> + *data = inl(priv->pmbase + 0xF0);
>
> return 4;
> }
>
> static int amd_rng_init(struct hwrng *rng)
> {
> + struct amd768_priv *priv = (struct amd768_priv *)rng->priv;
here,
> u8 rnen;
>
> - pci_read_config_byte(amd_pdev, 0x40, &rnen);
> + pci_read_config_byte(priv->pcidev, 0x40, &rnen);
> rnen |= BIT(7); /* RNG on */
> - pci_write_config_byte(amd_pdev, 0x40, rnen);
> + pci_write_config_byte(priv->pcidev, 0x40, rnen);
>
> - pci_read_config_byte(amd_pdev, 0x41, &rnen);
> + pci_read_config_byte(priv->pcidev, 0x41, &rnen);
> rnen |= BIT(7); /* PMIO enable */
> - pci_write_config_byte(amd_pdev, 0x41, rnen);
> + pci_write_config_byte(priv->pcidev, 0x41, rnen);
>
> return 0;
> }
>
> static void amd_rng_cleanup(struct hwrng *rng)
> {
> + struct amd768_priv *priv = (struct amd768_priv *)rng->priv;
here,
> u8 rnen;
>
> - pci_read_config_byte(amd_pdev, 0x40, &rnen);
> + pci_read_config_byte(priv->pcidev, 0x40, &rnen);
> rnen &= ~BIT(7); /* RNG off */
> - pci_write_config_byte(amd_pdev, 0x40, rnen);
> + pci_write_config_byte(priv->pcidev, 0x40, rnen);
> }
>
> static struct hwrng amd_rng = {
> @@ -110,6 +115,7 @@ static int __init mod_init(void)
> struct pci_dev *pdev = NULL;
> const struct pci_device_id *ent;
> u32 pmbase;
> + struct amd768_priv *priv;
>
> for_each_pci_dev(pdev) {
> ent = pci_match_id(pci_tbl, pdev);
> @@ -117,24 +123,30 @@ static int __init mod_init(void)
> goto found;
> }
> /* Device not found. */
> - goto out;
> + return -ENODEV;
>
> found:
> err = pci_read_config_dword(pdev, 0x58, &pmbase);
> if (err)
> - goto out;
> - err = -EIO;
> + return err;
> +
> pmbase &= 0x0000FF00;
> if (pmbase == 0)
> - goto out;
> + return -EIO;
> +
> + priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> if (!request_region(pmbase + 0xF0, 8, DRV_NAME)) {
> dev_err(&pdev->dev, DRV_NAME " region 0x%x already in use!\n",
> pmbase + 0xF0);
> err = -EBUSY;
> goto out;
> }
> - amd_rng.priv = (unsigned long)pmbase;
> - amd_pdev = pdev;
> + amd_rng.priv = (unsigned long)priv;
here,
> + priv->pmbase = pmbase;
> + priv->pcidev = pdev;
>
> pr_info(DRV_NAME " detected\n");
> err = hwrng_register(&amd_rng);
> @@ -143,17 +155,24 @@ found:
> release_region(pmbase + 0xF0, 8);
> goto out;
> }
> + return 0;
> +
> out:
> + kfree(priv);
> return err;
> }
>
> static void __exit mod_exit(void)
> {
> - u32 pmbase = (unsigned long)amd_rng.priv;
> + struct amd768_priv *priv;
> +
> + priv = (struct amd768_priv *)amd_rng.priv;
and here.
thx,
Jason.
>
> hwrng_unregister(&amd_rng);
>
> - release_region(pmbase + 0xF0, 8);
> + release_region(priv->pmbase + 0xF0, 8);
> +
> + kfree(priv);
> }
>
> module_init(mod_init);
> --
> 2.7.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
