Changes since v2:
- split the latest patch in 4
Changes since v1:
- Keep the hwrng name as "amd"
LABBE Corentin (8):
hwrng: amd: Fix style problem with blank line
hwrng: amd: use the BIT macro
hwrng: amd: Be consitent with the driver name
hwrng: amd: Remove asm/io.h
hwrng: amd: release_region must be called after hwrng_unregister
hwrng: amd: Replace global variable with private struct
hwrng: amd: Access hardware via ioread32/iowrite32
hwrng: amd: Convert to new hwrng read() API
drivers/char/hw_random/amd-rng.c | 150 +++++++++++++++++++++++++--------------
1 file changed, 96 insertions(+), 54 deletions(-)
--
2.7.3
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 <[email protected]>
---
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;
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;
- *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;
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;
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;
+ 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;
hwrng_unregister(&amd_rng);
- release_region(pmbase + 0xF0, 8);
+ release_region(priv->pmbase + 0xF0, 8);
+
+ kfree(priv);
}
module_init(mod_init);
--
2.7.3
This patch add usage of the BIT() macro
Signed-off-by: LABBE Corentin <[email protected]>
---
drivers/char/hw_random/amd-rng.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/char/hw_random/amd-rng.c b/drivers/char/hw_random/amd-rng.c
index 45b7965..d0042f5 100644
--- a/drivers/char/hw_random/amd-rng.c
+++ b/drivers/char/hw_random/amd-rng.c
@@ -78,11 +78,11 @@ static int amd_rng_init(struct hwrng *rng)
u8 rnen;
pci_read_config_byte(amd_pdev, 0x40, &rnen);
- rnen |= (1 << 7); /* RNG on */
+ rnen |= BIT(7); /* RNG on */
pci_write_config_byte(amd_pdev, 0x40, rnen);
pci_read_config_byte(amd_pdev, 0x41, &rnen);
- rnen |= (1 << 7); /* PMIO enable */
+ rnen |= BIT(7); /* PMIO enable */
pci_write_config_byte(amd_pdev, 0x41, rnen);
return 0;
@@ -93,7 +93,7 @@ static void amd_rng_cleanup(struct hwrng *rng)
u8 rnen;
pci_read_config_byte(amd_pdev, 0x40, &rnen);
- rnen &= ~(1 << 7); /* RNG off */
+ rnen &= ~BIT(7); /* RNG off */
pci_write_config_byte(amd_pdev, 0x40, rnen);
}
--
2.7.3
checkpatch complains about <asm/io.h> used instead of linux/io.h.
In fact it is not needed.
This patch remove it, and in the process, alphabetize the other headers.
Signed-off-by: LABBE Corentin <[email protected]>
---
drivers/char/hw_random/amd-rng.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/char/hw_random/amd-rng.c b/drivers/char/hw_random/amd-rng.c
index 93157af..de82fe3 100644
--- a/drivers/char/hw_random/amd-rng.c
+++ b/drivers/char/hw_random/amd-rng.c
@@ -24,12 +24,11 @@
* warranty of any kind, whether express or implied.
*/
-#include <linux/module.h>
+#include <linux/delay.h>
+#include <linux/hw_random.h>
#include <linux/kernel.h>
+#include <linux/module.h>
#include <linux/pci.h>
-#include <linux/hw_random.h>
-#include <linux/delay.h>
-#include <asm/io.h>
#define DRV_NAME "AMD768-HWRNG"
--
2.7.3
The driver release the memory region before being sure that nobody use
it.
This patch made hwrng_unregister ran before any release was done.
Signed-off-by: LABBE Corentin <[email protected]>
---
drivers/char/hw_random/amd-rng.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/char/hw_random/amd-rng.c b/drivers/char/hw_random/amd-rng.c
index de82fe3..383e197 100644
--- a/drivers/char/hw_random/amd-rng.c
+++ b/drivers/char/hw_random/amd-rng.c
@@ -151,8 +151,9 @@ static void __exit mod_exit(void)
{
u32 pmbase = (unsigned long)amd_rng.priv;
- release_region(pmbase + 0xF0, 8);
hwrng_unregister(&amd_rng);
+
+ release_region(pmbase + 0xF0, 8);
}
module_init(mod_init);
--
2.7.3
This patch convert the hwrng interface used by amd768-rng to its new API
by replacing data_read()/data_present() by read().
Signed-off-by: LABBE Corentin <[email protected]>
---
drivers/char/hw_random/amd-rng.c | 47 ++++++++++++++++++++++++----------------
1 file changed, 28 insertions(+), 19 deletions(-)
diff --git a/drivers/char/hw_random/amd-rng.c b/drivers/char/hw_random/amd-rng.c
index bfa14b9..9959c76 100644
--- a/drivers/char/hw_random/amd-rng.c
+++ b/drivers/char/hw_random/amd-rng.c
@@ -58,27 +58,37 @@ struct amd768_priv {
u32 pmbase;
};
-static int amd_rng_data_present(struct hwrng *rng, int wait)
+static int amd_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
{
+ u32 *data = buf;
struct amd768_priv *priv = (struct amd768_priv *)rng->priv;
- int data, i;
-
- for (i = 0; i < 20; i++) {
- data = !!(ioread32(priv->iobase + RNGDONE) & 1);
- if (data || !wait)
- break;
- udelay(10);
+ size_t read = 0;
+ /* We will wait at maximum one time per read */
+ int timeout = max / 4 + 1;
+
+ /*
+ * RNG data is available when RNGDONE is set to 1
+ * New random numbers are generated approximately 128 microseconds
+ * after RNGDATA is read
+ */
+ while (read < max) {
+ if (ioread32(priv->iobase + RNGDONE) == 0) {
+ if (wait) {
+ /* Delay given by datasheet */
+ usleep_range(128, 196);
+ if (timeout-- == 0)
+ return read;
+ } else {
+ return 0;
+ }
+ } else {
+ *data = ioread32(priv->iobase + RNGDATA);
+ data++;
+ read += 4;
+ }
}
- return data;
-}
-
-static int amd_rng_data_read(struct hwrng *rng, u32 *data)
-{
- struct amd768_priv *priv = (struct amd768_priv *)rng->priv;
-
- *data = ioread32(priv->iobase + RNGDATA);
- return 4;
+ return read;
}
static int amd_rng_init(struct hwrng *rng)
@@ -111,8 +121,7 @@ static struct hwrng amd_rng = {
.name = "amd",
.init = amd_rng_init,
.cleanup = amd_rng_cleanup,
- .data_present = amd_rng_data_present,
- .data_read = amd_rng_data_read,
+ .read = amd_rng_read,
};
static int __init mod_init(void)
--
2.7.3
Instead of accessing hw directly via pmbase, it's better to
access after ioport_map() via ioread32/iowrite32.
Signed-off-by: LABBE Corentin <[email protected]>
---
drivers/char/hw_random/amd-rng.c | 31 +++++++++++++++++++++++++------
1 file changed, 25 insertions(+), 6 deletions(-)
diff --git a/drivers/char/hw_random/amd-rng.c b/drivers/char/hw_random/amd-rng.c
index 4ef94e9..bfa14b9 100644
--- a/drivers/char/hw_random/amd-rng.c
+++ b/drivers/char/hw_random/amd-rng.c
@@ -32,6 +32,11 @@
#define DRV_NAME "AMD768-HWRNG"
+#define RNGDATA 0x00
+#define RNGDONE 0x04
+#define PMBASE_OFFSET 0xF0
+#define PMBASE_SIZE 8
+
/*
* Data for PCI driver interface
*
@@ -48,6 +53,7 @@ static const struct pci_device_id pci_tbl[] = {
MODULE_DEVICE_TABLE(pci, pci_tbl);
struct amd768_priv {
+ void __iomem *iobase;
struct pci_dev *pcidev;
u32 pmbase;
};
@@ -58,7 +64,7 @@ static int amd_rng_data_present(struct hwrng *rng, int wait)
int data, i;
for (i = 0; i < 20; i++) {
- data = !!(inl(priv->pmbase + 0xF4) & 1);
+ data = !!(ioread32(priv->iobase + RNGDONE) & 1);
if (data || !wait)
break;
udelay(10);
@@ -70,7 +76,7 @@ static int amd_rng_data_read(struct hwrng *rng, u32 *data)
{
struct amd768_priv *priv = (struct amd768_priv *)rng->priv;
- *data = inl(priv->pmbase + 0xF0);
+ *data = ioread32(priv->iobase + RNGDATA);
return 4;
}
@@ -138,12 +144,20 @@ found:
if (!priv)
return -ENOMEM;
- if (!request_region(pmbase + 0xF0, 8, DRV_NAME)) {
+ if (!request_region(pmbase + PMBASE_OFFSET, PMBASE_SIZE, DRV_NAME)) {
dev_err(&pdev->dev, DRV_NAME " region 0x%x already in use!\n",
pmbase + 0xF0);
err = -EBUSY;
goto out;
}
+
+ priv->iobase = ioport_map(pmbase + PMBASE_OFFSET, PMBASE_SIZE);
+ if (!priv->iobase) {
+ pr_err(DRV_NAME "Cannot map ioport\n");
+ err = -EINVAL;
+ goto err_iomap;
+ }
+
amd_rng.priv = (unsigned long)priv;
priv->pmbase = pmbase;
priv->pcidev = pdev;
@@ -152,11 +166,14 @@ found:
err = hwrng_register(&amd_rng);
if (err) {
pr_err(DRV_NAME " registering failed (%d)\n", err);
- release_region(pmbase + 0xF0, 8);
- goto out;
+ goto err_hwrng;
}
return 0;
+err_hwrng:
+ ioport_unmap(priv->iobase);
+err_iomap:
+ release_region(pmbase + PMBASE_OFFSET, PMBASE_SIZE);
out:
kfree(priv);
return err;
@@ -170,7 +187,9 @@ static void __exit mod_exit(void)
hwrng_unregister(&amd_rng);
- release_region(priv->pmbase + 0xF0, 8);
+ ioport_unmap(priv->iobase);
+
+ release_region(priv->pmbase + PMBASE_OFFSET, PMBASE_SIZE);
kfree(priv);
}
--
2.7.3
The driver name is displayed each time differently.
This patch make use of the same name everywhere.
Signed-off-by: LABBE Corentin <[email protected]>
---
drivers/char/hw_random/amd-rng.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/char/hw_random/amd-rng.c b/drivers/char/hw_random/amd-rng.c
index d0042f5..93157af 100644
--- a/drivers/char/hw_random/amd-rng.c
+++ b/drivers/char/hw_random/amd-rng.c
@@ -31,7 +31,7 @@
#include <linux/delay.h>
#include <asm/io.h>
-#define PFX KBUILD_MODNAME ": "
+#define DRV_NAME "AMD768-HWRNG"
/*
* Data for PCI driver interface
@@ -128,8 +128,8 @@ found:
pmbase &= 0x0000FF00;
if (pmbase == 0)
goto out;
- if (!request_region(pmbase + 0xF0, 8, "AMD HWRNG")) {
- dev_err(&pdev->dev, "AMD HWRNG region 0x%x already in use!\n",
+ 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;
@@ -137,11 +137,10 @@ found:
amd_rng.priv = (unsigned long)pmbase;
amd_pdev = pdev;
- pr_info("AMD768 RNG detected\n");
+ pr_info(DRV_NAME " detected\n");
err = hwrng_register(&amd_rng);
if (err) {
- pr_err(PFX "RNG registering failed (%d)\n",
- err);
+ pr_err(DRV_NAME " registering failed (%d)\n", err);
release_region(pmbase + 0xF0, 8);
goto out;
}
--
2.7.3
Some blank line are unncessary, and one is missing after declaration.
This patch fix thoses style problems.
Signed-off-by: LABBE Corentin <[email protected]>
---
drivers/char/hw_random/amd-rng.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/char/hw_random/amd-rng.c b/drivers/char/hw_random/amd-rng.c
index 48f6a83..45b7965 100644
--- a/drivers/char/hw_random/amd-rng.c
+++ b/drivers/char/hw_random/amd-rng.c
@@ -31,10 +31,8 @@
#include <linux/delay.h>
#include <asm/io.h>
-
#define PFX KBUILD_MODNAME ": "
-
/*
* Data for PCI driver interface
*
@@ -52,7 +50,6 @@ MODULE_DEVICE_TABLE(pci, pci_tbl);
static struct pci_dev *amd_pdev;
-
static int amd_rng_data_present(struct hwrng *rng, int wait)
{
u32 pmbase = (u32)rng->priv;
@@ -100,7 +97,6 @@ static void amd_rng_cleanup(struct hwrng *rng)
pci_write_config_byte(amd_pdev, 0x40, rnen);
}
-
static struct hwrng amd_rng = {
.name = "amd",
.init = amd_rng_init,
@@ -109,7 +105,6 @@ static struct hwrng amd_rng = {
.data_read = amd_rng_data_read,
};
-
static int __init mod_init(void)
{
int err = -ENODEV;
@@ -157,6 +152,7 @@ out:
static void __exit mod_exit(void)
{
u32 pmbase = (unsigned long)amd_rng.priv;
+
release_region(pmbase + 0xF0, 8);
hwrng_unregister(&amd_rng);
}
--
2.7.3
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 <[email protected]>
> ---
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Corentin,
On Fri, Aug 26, 2016 at 01:11:28PM +0200, LABBE Corentin wrote:
> Changes since v2:
> - split the latest patch in 4
> Changes since v1:
> - Keep the hwrng name as "amd"
>
> LABBE Corentin (8):
> hwrng: amd: Fix style problem with blank line
> hwrng: amd: use the BIT macro
> hwrng: amd: Be consitent with the driver name
> hwrng: amd: Remove asm/io.h
> hwrng: amd: release_region must be called after hwrng_unregister
> hwrng: amd: Replace global variable with private struct
> hwrng: amd: Access hardware via ioread32/iowrite32
> hwrng: amd: Convert to new hwrng read() API
>
> drivers/char/hw_random/amd-rng.c | 150 +++++++++++++++++++++++++--------------
> 1 file changed, 96 insertions(+), 54 deletions(-)
Once you've fixed up the casting in #6, you can add my
Reviewed-by: Jason Cooper <[email protected]>
to the series.
thx,
Jason.
On Sat, Aug 27, 2016 at 02:43:31PM +0000, Jason Cooper wrote:
> 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 <[email protected]>
> > ---
> > 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...
Hmm, I was assuming that, like other places in the tree, that priv was
declared void*. However, it's unsigned long in hw_random.h.
And, it looks like all users cast it. Either to a struct, or to a void
__iomem *.
So ignore what I said in my previous email. You can add my reviewed-by
without change.
It does look like /priv/ s/unsigned long/void */ would be a great
cleanup.
thx,
Jason.
On Fri, Aug 26, 2016 at 01:11:28PM +0200, LABBE Corentin wrote:
> Changes since v2:
> - split the latest patch in 4
> Changes since v1:
> - Keep the hwrng name as "amd"
All applied. Thanks.
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt