Re: [PATCH v3 3/3] Input: snvs_pwrkey - only IRQ_HANDLED for our own events
Hi Marco, On Wed, Jul 29, 2020 at 09:55:13AM +0200, Marco Felsch wrote: > Hi, > > On 20-07-23 10:43, Horia Geantă wrote: > > From: André Draszik > > > > The snvs_pwrkey shares the SNVS LPSR status register with the snvs_rtc. > > > > This driver here should only return IRQ_HANDLED if the status register > > indicates that the event we're handling in the irq handler was genuinely > > intended for this driver. Otheriwse the interrupt subsystem will > > assume the interrupt was handled successfully even though it wasn't > > at all. > > After checking the RM and the imx6qdl.dtsi I'm not very sure that this > is right since the snvs-powerkey has a seperate irq-line. So we can be > sure that this irq is for us. If this is the case we don't need to check > the SNVS_LPSR_REG instead we only need to clear it. Wouldn't we want to know if for some reason we get spurious interrupts? Thanks. -- Dmitry
Re: [PATCH v3 2/3] Input: snvs_pwrkey - enable snvs clock as needed
On Wed, Jul 29, 2020 at 09:33:23AM +0200, Marco Felsch wrote:
> Hi,
>
> On 20-07-23 10:43, Horia Geantă wrote:
> > From: André Draszik
> >
> > At the moment, enabling this driver without the SNVS RTC driver
> > being active will hang the kernel as soon as the power button
> > is pressed.
> >
> > The reason is that in that case the SNVS isn't enabled, and
> > any attempt to read the SNVS registers will simply hang forever.
> >
> > Ensure the clock is enabled (during the interrupt handler) to
> > make this driver work.
> >
> > Also see commit 7f8993995410 ("drivers/rtc/rtc-snvs: add clock support")
> > and commit edb190cb1734
> > ("rtc: snvs: make sure clock is enabled for interrupt handle")
> > for similar updates to the snvs rtc driver.
> >
> > Signed-off-by: André Draszik
> > Reviewed-by: Horia Geantă
> > Signed-off-by: Horia Geantă
> > ---
> > drivers/input/keyboard/snvs_pwrkey.c | 28 +++-
> > 1 file changed, 27 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/input/keyboard/snvs_pwrkey.c
> > b/drivers/input/keyboard/snvs_pwrkey.c
> > index 2f5e3ab5ed63..382d2ae82c9b 100644
> > --- a/drivers/input/keyboard/snvs_pwrkey.c
> > +++ b/drivers/input/keyboard/snvs_pwrkey.c
> > @@ -16,6 +16,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> >
> > @@ -38,6 +39,7 @@ struct pwrkey_drv_data {
> > int wakeup;
> > struct timer_list check_timer;
> > struct input_dev *input;
> > + struct clk *clk;
> > u8 minor_rev;
> > };
> >
> > @@ -47,7 +49,10 @@ static void imx_imx_snvs_check_for_events(struct
> > timer_list *t)
> > struct input_dev *input = pdata->input;
> > u32 state;
> >
> > + clk_enable(pdata->clk);
> > regmap_read(pdata->snvs, SNVS_HPSR_REG, &state);
> > + clk_disable(pdata->clk);
> > +
> > state = state & SNVS_HPSR_BTN ? 1 : 0;
> >
> > /* only report new event if status changed */
> > @@ -74,11 +79,13 @@ static irqreturn_t imx_snvs_pwrkey_interrupt(int irq,
> > void *dev_id)
> >
> > pm_wakeup_event(input->dev.parent, 0);
> >
> > + clk_enable(pdata->clk);
> > +
> > regmap_read(pdata->snvs, SNVS_LPSR_REG, &lp_status);
> > if (lp_status & SNVS_LPSR_SPO) {
> > if (pdata->minor_rev == 0) {
> > /*
> > -* The first generation i.MX6 SoCs only sends an
> > +* The first generation i.MX[6|7] SoCs only send an
> > * interrupt on button release. To mimic power-key
> > * usage, we'll prepend a press event.
> > */
> > @@ -96,6 +103,8 @@ static irqreturn_t imx_snvs_pwrkey_interrupt(int irq,
> > void *dev_id)
> > /* clear SPO status */
> > regmap_write(pdata->snvs, SNVS_LPSR_REG, SNVS_LPSR_SPO);
> >
> > + clk_disable(pdata->clk);
>
> I'm not so happy about clk_enable/disable() during the interrupt
> routine since those routines should be handled fast. Since we assume
> that the clock is always oni
We do? I thought that the fact that clock might be off is the root of
the problem.
> I would rather call clk_prepare_enable()
> during probe() and keep the clock on.
clk_enable() must not sleep and is supposed to be lightweight. This
interrupt I believe is supposed to be infrequent, so having it here
should be fine and might save us some power.
>
> > +
> > return IRQ_HANDLED;
> > }
> >
> > @@ -140,6 +149,23 @@ static int imx_snvs_pwrkey_probe(struct
> > platform_device *pdev)
> > if (pdata->irq < 0)
> > return -EINVAL;
> >
> > + pdata->clk = devm_clk_get_optional(&pdev->dev, "snvs-pwrkey");
> > + if (IS_ERR(pdata->clk))
> > + return PTR_ERR(pdata->clk);
> > +
> > + error = clk_prepare(pdata->clk);
> > + if (error) {
> > + dev_err(&pdev->dev, "failed to prepare the snvs clock\n");
> > + return error;
> > + }
> > + error = devm_add_action_or_reset(&pdev->dev,
> > + (void(*)(void *))clk_unprepare,
> > + pdata->clk);
>
> I'm not a fan about those casts. However, the intentation should be
> fixed.
Yes, can we please create a proper function for this (or maybe we will
finally get devm clk API?).
Thanks.
--
Dmitry
Re: [tpmdd-devel] [PATCH] tpm: remove chip_num parameter from in-kernel API
On Tue, Oct 24, 2017 at 11:37:57AM -0600, Jason Gunthorpe wrote: > On Tue, Oct 24, 2017 at 10:02:00AM -0700, Dmitry Torokhov wrote: > > tpm-rng is abomination that should be kicked out as soon as possible. > > It wrecks havoc with the power management (TPM chip drivers may go > > into suspend state, but tpm_rng does not do any power management and > > happily forwards requests to suspended hardware) and may be available > > when there is no TPM at all yet (the drivers have not been probed yet, > > or have gotten a deferral, etc). > > Makes sense > > > TPM core should register HWRNGs when chips are ready. > > The main thing I've wanted from the TPM RNG is > 'add_early_randomness'.. I see... However you can't add any randomness if hardware is not quite there yet. > > We can certainly provide a TPM interface to hwrng, it seems > reasonable. > > Excep that we already have a user api in /dev/tpm to access the > tpm RNG, is the duplication a problem? If we already have userspace API we have to maintain this, even if it is duplicate, there is no way around it. I'd expect most of the users will use unified random API, while some TPM-oriented users may still go via /dev/tpm to use the same API as all other their requests. Thanks. -- Dmitry
Re: [tpmdd-devel] [PATCH] tpm: remove chip_num parameter from in-kernel API
On Tue, Oct 24, 2017 at 9:11 AM, Jason Gunthorpe wrote: > On Tue, Oct 24, 2017 at 09:37:33PM +0530, PrasannaKumar Muralidharan wrote: >> Hi Jason, >> >> On 24 October 2017 at 21:25, Jason Gunthorpe >> wrote: >> > On Tue, Oct 24, 2017 at 09:21:15PM +0530, PrasannaKumar Muralidharan wrote: >> > >> >> Please check the RFC [1]. It does use chip id. The rfc has issues and >> >> has to be fixed but still there could be users of the API. >> >> >> >> 1. https://www.spinics.net/lists/linux-crypto/msg28282.html >> > >> > That patch isn't safe at all. You need to store a kref to th chip in >> > the hwrng, not parse a string. >> >> The drivers/char/hw_random/tpm-rng.c module does not store the chip >> reference so I guess the usage is safe. > > It is using the default TPM, it is always safe to use the default tpm. tpm-rng is abomination that should be kicked out as soon as possible. It wrecks havoc with the power management (TPM chip drivers may go into suspend state, but tpm_rng does not do any power management and happily forwards requests to suspended hardware) and may be available when there is no TPM at all yet (the drivers have not been probed yet, or have gotten a deferral, etc). TPM core should register HWRNGs when chips are ready. Thanks. -- Dmitry
Re: [PATCH v5 REPOST 1/6] hw_random: place mutex around read functions and buffers.
On Tue, Sep 26, 2017 at 02:36:57AM -0400, Pankaj Gupta wrote: > > > > > A bit late to a party, but: > > > > On Mon, Dec 8, 2014 at 12:50 AM, Amos Kong wrote: > > > From: Rusty Russell > > > > > > There's currently a big lock around everything, and it means that we > > > can't query sysfs (eg /sys/devices/virtual/misc/hw_random/rng_current) > > > while the rng is reading. This is a real problem when the rng is slow, > > > or blocked (eg. virtio_rng with qemu's default /dev/random backend) > > > > > > This doesn't help (it leaves the current lock untouched), just adds a > > > lock to protect the read function and the static buffers, in preparation > > > for transition. > > > > > > Signed-off-by: Rusty Russell > > > --- > > ... > > > > > > @@ -160,13 +166,14 @@ static ssize_t rng_dev_read(struct file *filp, char > > > __user *buf, > > > goto out_unlock; > > > } > > > > > > + mutex_lock(&reading_mutex); > > > > I think this breaks O_NONBLOCK: we have hwrng core thread that is > > constantly pumps underlying rng for data; the thread takes the mutex > > and calls rng_get_data() that blocks until RNG responds. This means > > that even user specified O_NONBLOCK here we'll be waiting until > > [hwrng] thread releases reading_mutex before we can continue. > > I think for 'virtio_rng' for 'O_NON_BLOCK' 'rng_get_data' returns > without waiting for data which can let mutex to be used by other > threads waiting if any? > > rng_dev_read > rng_get_data > virtio_read As I said in the paragraph above the code that potentially holds the mutex for long time is the thread in hwrng core: hwrng_fillfn(). As it calls rng_get_data() with "wait" argument == 1 it may block while holding reading_mutex, which, in turn, will block rng_dev_read(), even if it was called with O_NONBLOCK. Thanks. -- Dmitry
Re: [PATCH v5 REPOST 1/6] hw_random: place mutex around read functions and buffers.
A bit late to a party, but:
On Mon, Dec 8, 2014 at 12:50 AM, Amos Kong wrote:
> From: Rusty Russell
>
> There's currently a big lock around everything, and it means that we
> can't query sysfs (eg /sys/devices/virtual/misc/hw_random/rng_current)
> while the rng is reading. This is a real problem when the rng is slow,
> or blocked (eg. virtio_rng with qemu's default /dev/random backend)
>
> This doesn't help (it leaves the current lock untouched), just adds a
> lock to protect the read function and the static buffers, in preparation
> for transition.
>
> Signed-off-by: Rusty Russell
> ---
...
>
> @@ -160,13 +166,14 @@ static ssize_t rng_dev_read(struct file *filp, char
> __user *buf,
> goto out_unlock;
> }
>
> + mutex_lock(&reading_mutex);
I think this breaks O_NONBLOCK: we have hwrng core thread that is
constantly pumps underlying rng for data; the thread takes the mutex
and calls rng_get_data() that blocks until RNG responds. This means
that even user specified O_NONBLOCK here we'll be waiting until
[hwrng] thread releases reading_mutex before we can continue.
> if (!data_avail) {
> bytes_read = rng_get_data(current_rng, rng_buffer,
> rng_buffer_size(),
> !(filp->f_flags & O_NONBLOCK));
> if (bytes_read < 0) {
> err = bytes_read;
> - goto out_unlock;
> + goto out_unlock_reading;
> }
> data_avail = bytes_read;
> }
Thanks.
--
Dmitry
Re: USB HID devices not linked to RNG
On Mon, Sep 7, 2015 at 2:09 PM, Stephan Mueller wrote: > Am Montag, 7. September 2015, 15:07:47 schrieb Mike Mestnik: > > Hi Mike, > >>This is just an off the wall guess. Wouldn't such entropy already be >>collected at a lower level, like raw usb traffic? > > The RNG collects data from interrupts (that also covers normal PS/2 mice and > keyboards), block devices and HID (it hooks itself into the input layer). > > So, raw USB traffic is not a source of entropy other than for the interrupts > the base PCI traffic would generate. > > What I am wondering: isn't the USB mouse/keyboard support hooking into the > kernel's input layer? All input devices add randomness, however I think for mouse add_input_randomness() will drop majority of events since they are likely have the same value (well, depends on which direction you are moving the mouse)... -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/6] hwrng: iproc-rng200 - do not use static structure
Instead of using static hwrng structure that is reused between
binds/unbinds of the device let's embed it into driver's private
structure that we allocate. This way we are guaranteed not to stumble
onto something left from previous bind attempt.
Signed-off-by: Dmitry Torokhov
---
drivers/char/hw_random/iproc-rng200.c | 44 +--
1 file changed, 21 insertions(+), 23 deletions(-)
diff --git a/drivers/char/hw_random/iproc-rng200.c
b/drivers/char/hw_random/iproc-rng200.c
index 276cb8a..2dbaf5c 100644
--- a/drivers/char/hw_random/iproc-rng200.c
+++ b/drivers/char/hw_random/iproc-rng200.c
@@ -48,9 +48,12 @@
#define RNG_FIFO_COUNT_RNG_FIFO_COUNT_MASK 0x00FF
struct iproc_rng200_dev {
- void __iomem*base;
+ struct hwrng rng;
+ void __iomem *base;
};
+#define to_rng_priv(rng) container_of(rng, struct iproc_rng200_dev, rng)
+
static void iproc_rng200_restart(void __iomem *rng_base)
{
uint32_t val;
@@ -89,11 +92,11 @@ static void iproc_rng200_restart(void __iomem *rng_base)
}
static int iproc_rng200_read(struct hwrng *rng, void *buf, size_t max,
- bool wait)
+bool wait)
{
- uint32_t status;
+ struct iproc_rng200_dev *priv = to_rng_priv(rng);
uint32_t num_remaining = max;
- struct iproc_rng200_dev *priv = (struct iproc_rng200_dev *)rng->priv;
+ uint32_t status;
#define MAX_RESETS_PER_READ 1
uint32_t num_resets = 0;
@@ -151,10 +154,8 @@ static int iproc_rng200_read(struct hwrng *rng, void *buf,
size_t max,
static int iproc_rng200_init(struct hwrng *rng)
{
+ struct iproc_rng200_dev *priv = to_rng_priv(rng);
uint32_t val;
- struct iproc_rng200_dev *priv;
-
- priv = (struct iproc_rng200_dev *)rng->priv;
/* Setup RNG. */
val = ioread32(priv->base + RNG_CTRL_OFFSET);
@@ -167,10 +168,8 @@ static int iproc_rng200_init(struct hwrng *rng)
static void iproc_rng200_cleanup(struct hwrng *rng)
{
+ struct iproc_rng200_dev *priv = to_rng_priv(rng);
uint32_t val;
- struct iproc_rng200_dev *priv;
-
- priv = (struct iproc_rng200_dev *)rng->priv;
/* Disable RNG hardware */
val = ioread32(priv->base + RNG_CTRL_OFFSET);
@@ -179,13 +178,6 @@ static void iproc_rng200_cleanup(struct hwrng *rng)
iowrite32(val, priv->base + RNG_CTRL_OFFSET);
}
-static struct hwrng iproc_rng200_ops = {
- .name = "iproc-rng200",
- .read = iproc_rng200_read,
- .init = iproc_rng200_init,
- .cleanup= iproc_rng200_cleanup,
-};
-
static int iproc_rng200_probe(struct platform_device *pdev)
{
struct iproc_rng200_dev *priv;
@@ -193,13 +185,10 @@ static int iproc_rng200_probe(struct platform_device
*pdev)
struct device *dev = &pdev->dev;
int ret;
- priv = devm_kzalloc(dev, sizeof(struct iproc_rng200_dev), GFP_KERNEL);
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
return -ENOMEM;
- iproc_rng200_ops.priv = (unsigned long)priv;
- platform_set_drvdata(pdev, priv);
-
/* Map peripheral */
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res) {
@@ -213,13 +202,20 @@ static int iproc_rng200_probe(struct platform_device
*pdev)
return PTR_ERR(priv->base);
}
+ priv->rng.name = "iproc-rng200",
+ priv->rng.read = iproc_rng200_read,
+ priv->rng.init = iproc_rng200_init,
+ priv->rng.cleanup = iproc_rng200_cleanup,
+
/* Register driver */
- ret = hwrng_register(&iproc_rng200_ops);
+ ret = hwrng_register(&priv->rng);
if (ret) {
dev_err(dev, "hwrng registration failed\n");
return ret;
}
+ platform_set_drvdata(pdev, priv);
+
dev_info(dev, "hwrng registered\n");
return 0;
@@ -227,8 +223,10 @@ static int iproc_rng200_probe(struct platform_device *pdev)
static int iproc_rng200_remove(struct platform_device *pdev)
{
+ struct iproc_rng200_dev *priv = platform_get_drvdata(pdev);
+
/* Unregister driver */
- hwrng_unregister(&iproc_rng200_ops);
+ hwrng_unregister(&priv->rng);
return 0;
}
--
2.2.0.rc0.207.ga3a616c
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/6] Introduce devm_hwrng_register and convert a few dirvers
A few drivers can benefit from devm-style interface for hwrng since it is quite often the last thing that isn't automatically managed. Using devm_hwrng_register() in such drivers allows get rid of manual error unwinding and drivers' remove() methods. I tested changes to iproc-rng200, the rest are compiled only. Thanks. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/6] hwrng: iproc-rng200 - make use of devm_hwrng_register
This allows us to get rid of driver's remove() method.
Signed-off-by: Dmitry Torokhov
---
drivers/char/hw_random/iproc-rng200.c | 15 +--
1 file changed, 1 insertion(+), 14 deletions(-)
diff --git a/drivers/char/hw_random/iproc-rng200.c
b/drivers/char/hw_random/iproc-rng200.c
index 2dbaf5c..3eaf7cb 100644
--- a/drivers/char/hw_random/iproc-rng200.c
+++ b/drivers/char/hw_random/iproc-rng200.c
@@ -208,29 +208,17 @@ static int iproc_rng200_probe(struct platform_device
*pdev)
priv->rng.cleanup = iproc_rng200_cleanup,
/* Register driver */
- ret = hwrng_register(&priv->rng);
+ ret = devm_hwrng_register(dev, &priv->rng);
if (ret) {
dev_err(dev, "hwrng registration failed\n");
return ret;
}
- platform_set_drvdata(pdev, priv);
-
dev_info(dev, "hwrng registered\n");
return 0;
}
-static int iproc_rng200_remove(struct platform_device *pdev)
-{
- struct iproc_rng200_dev *priv = platform_get_drvdata(pdev);
-
- /* Unregister driver */
- hwrng_unregister(&priv->rng);
-
- return 0;
-}
-
static const struct of_device_id iproc_rng200_of_match[] = {
{ .compatible = "brcm,iproc-rng200", },
{},
@@ -243,7 +231,6 @@ static struct platform_driver iproc_rng200_driver = {
.of_match_table = iproc_rng200_of_match,
},
.probe = iproc_rng200_probe,
- .remove = iproc_rng200_remove,
};
module_platform_driver(iproc_rng200_driver);
--
2.2.0.rc0.207.ga3a616c
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/6] hwrng: msm - make use of devm_hwrng_register
This allows us to get rid of remove() method.
Signed-off-by: Dmitry Torokhov
---
drivers/char/hw_random/msm-rng.c | 11 +--
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/drivers/char/hw_random/msm-rng.c b/drivers/char/hw_random/msm-rng.c
index cea1c70..96fb986 100644
--- a/drivers/char/hw_random/msm-rng.c
+++ b/drivers/char/hw_random/msm-rng.c
@@ -157,7 +157,7 @@ static int msm_rng_probe(struct platform_device *pdev)
rng->hwrng.cleanup = msm_rng_cleanup,
rng->hwrng.read = msm_rng_read,
- ret = hwrng_register(&rng->hwrng);
+ ret = devm_hwrng_register(&pdev->dev, &rng->hwrng);
if (ret) {
dev_err(&pdev->dev, "failed to register hwrng\n");
return ret;
@@ -166,14 +166,6 @@ static int msm_rng_probe(struct platform_device *pdev)
return 0;
}
-static int msm_rng_remove(struct platform_device *pdev)
-{
- struct msm_rng *rng = platform_get_drvdata(pdev);
-
- hwrng_unregister(&rng->hwrng);
- return 0;
-}
-
static const struct of_device_id msm_rng_of_match[] = {
{ .compatible = "qcom,prng", },
{}
@@ -182,7 +174,6 @@ MODULE_DEVICE_TABLE(of, msm_rng_of_match);
static struct platform_driver msm_rng_driver = {
.probe = msm_rng_probe,
- .remove = msm_rng_remove,
.driver = {
.name = KBUILD_MODNAME,
.of_match_table = of_match_ptr(msm_rng_of_match),
--
2.2.0.rc0.207.ga3a616c
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/6] hwrng: add devm_* interfaces
This change adds devm_hwrng_register and devm_hwrng_unregister which
use can simplify error unwinding and unbinding code paths in device
drivers.
Signed-off-by: Dmitry Torokhov
---
drivers/char/hw_random/core.c | 42 ++
include/linux/hw_random.h | 4
2 files changed, 46 insertions(+)
diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
index 32a8a86..83161dd 100644
--- a/drivers/char/hw_random/core.c
+++ b/drivers/char/hw_random/core.c
@@ -536,6 +536,48 @@ void hwrng_unregister(struct hwrng *rng)
}
EXPORT_SYMBOL_GPL(hwrng_unregister);
+static void devm_hwrng_release(struct device *dev, void *res)
+{
+ hwrng_unregister(*(struct hwrng **)res);
+}
+
+static int devm_hwrng_match(struct device *dev, void *res, void *data)
+{
+ struct hwrng **r = res;
+
+ if (WARN_ON(!r || !*r))
+ return 0;
+
+ return *r == data;
+}
+
+int devm_hwrng_register(struct device *dev, struct hwrng *rng)
+{
+ struct hwrng **ptr;
+ int error;
+
+ ptr = devres_alloc(devm_hwrng_release, sizeof(*ptr), GFP_KERNEL);
+ if (!ptr)
+ return -ENOMEM;
+
+ error = hwrng_register(rng);
+ if (error) {
+ devres_free(ptr);
+ return error;
+ }
+
+ *ptr = rng;
+ devres_add(dev, ptr);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(devm_hwrng_register);
+
+void devm_hwrng_unregister(struct device *dev, struct hwrng *rng)
+{
+ devres_release(dev, devm_hwrng_release, devm_hwrng_match, rng);
+}
+EXPORT_SYMBOL_GPL(devm_hwrng_unregister);
+
static int __init hwrng_modinit(void)
{
return register_miscdev();
diff --git a/include/linux/hw_random.h b/include/linux/hw_random.h
index eb7b414..4f7d8f4 100644
--- a/include/linux/hw_random.h
+++ b/include/linux/hw_random.h
@@ -50,10 +50,14 @@ struct hwrng {
struct completion cleanup_done;
};
+struct device;
+
/** Register a new Hardware Random Number Generator driver. */
extern int hwrng_register(struct hwrng *rng);
+extern int devm_hwrng_register(struct device *dev, struct hwrng *rng);
/** Unregister a Hardware Random Number Generator driver. */
extern void hwrng_unregister(struct hwrng *rng);
+extern void devm_hwrng_unregister(struct device *dve, struct hwrng *rng);
/** Feed random bits into the pool. */
extern void add_hwgenerator_randomness(const char *buffer, size_t count,
size_t entropy);
--
2.2.0.rc0.207.ga3a616c
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/6] hwrng: bcm63xx - make use of devm_hwrng_register
This change converts bcm63xx-rng to use devm* API for managing all
resources, which allows us to dispense with the rest of error handling
path and remove() function. Also we combine hwern and driver-private
data into a single allocation, use clk_prepare_enable() instead
of "naked" clk_enable() and move clock enabling/disabling into hwrnd
inti(0 and cleanup() methods so the clock stays off until rng is
used.
Signed-off-by: Dmitry Torokhov
---
drivers/char/hw_random/bcm63xx-rng.c | 87 +---
1 file changed, 31 insertions(+), 56 deletions(-)
diff --git a/drivers/char/hw_random/bcm63xx-rng.c
b/drivers/char/hw_random/bcm63xx-rng.c
index 27da00f..d1494ec 100644
--- a/drivers/char/hw_random/bcm63xx-rng.c
+++ b/drivers/char/hw_random/bcm63xx-rng.c
@@ -24,16 +24,22 @@
#define RNG_MASK 0x10
struct bcm63xx_rng_priv {
+ struct hwrng rng;
struct clk *clk;
void __iomem *regs;
};
-#define to_rng_priv(rng) ((struct bcm63xx_rng_priv *)rng->priv)
+#define to_rng_priv(rng) container_of(rng, struct bcm63xx_rng_priv, rng)
static int bcm63xx_rng_init(struct hwrng *rng)
{
struct bcm63xx_rng_priv *priv = to_rng_priv(rng);
u32 val;
+ int error;
+
+ error = clk_prepare_enable(priv->clk);
+ if (error)
+ return error;
val = __raw_readl(priv->regs + RNG_CTRL);
val |= RNG_EN;
@@ -50,6 +56,8 @@ static void bcm63xx_rng_cleanup(struct hwrng *rng)
val = __raw_readl(priv->regs + RNG_CTRL);
val &= ~RNG_EN;
__raw_writel(val, priv->regs + RNG_CTRL);
+
+ clk_didsable_unprepare(prov->clk);
}
static int bcm63xx_rng_data_present(struct hwrng *rng, int wait)
@@ -79,86 +87,53 @@ static int bcm63xx_rng_probe(struct platform_device *pdev)
r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!r) {
dev_err(&pdev->dev, "no iomem resource\n");
- ret = -ENXIO;
- goto out;
+ return -ENXIO;
}
priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
- if (!priv) {
- ret = -ENOMEM;
- goto out;
- }
-
- rng = devm_kzalloc(&pdev->dev, sizeof(*rng), GFP_KERNEL);
- if (!rng) {
- ret = -ENOMEM;
- goto out;
+ if (!priv)
+ return -ENOMEM;
+
+ priv->rng.name = pdev->name;
+ priv->rng.init = bcm63xx_rng_init;
+ priv->rng.cleanup = bcm63xx_rng_cleanup;
+ prov->rng.data_present = bcm63xx_rng_data_present;
+ priv->rng.data_read = bcm63xx_rng_data_read;
+
+ priv->clk = devm_clk_get(&pdev->dev, "ipsec");
+ if (IS_ERR(priv->clk)) {
+ error = PTR_ERR(priv->clk);
+ dev_err(&pdev->dev, "no clock for device: %d\n", error);
+ return error;
}
- platform_set_drvdata(pdev, rng);
- rng->priv = (unsigned long)priv;
- rng->name = pdev->name;
- rng->init = bcm63xx_rng_init;
- rng->cleanup = bcm63xx_rng_cleanup;
- rng->data_present = bcm63xx_rng_data_present;
- rng->data_read = bcm63xx_rng_data_read;
-
- clk = clk_get(&pdev->dev, "ipsec");
- if (IS_ERR(clk)) {
- dev_err(&pdev->dev, "no clock for device\n");
- ret = PTR_ERR(clk);
- goto out;
- }
-
- priv->clk = clk;
-
if (!devm_request_mem_region(&pdev->dev, r->start,
resource_size(r), pdev->name)) {
dev_err(&pdev->dev, "request mem failed");
- ret = -ENOMEM;
- goto out;
+ return -EBUSY;
}
priv->regs = devm_ioremap_nocache(&pdev->dev, r->start,
resource_size(r));
if (!priv->regs) {
dev_err(&pdev->dev, "ioremap failed");
- ret = -ENOMEM;
- goto out;
+ return -ENOMEM;
}
- clk_enable(clk);
-
- ret = hwrng_register(rng);
- if (ret) {
- dev_err(&pdev->dev, "failed to register rng device\n");
- goto out_clk_disable;
+ error = devm_hwrng_register(&pdev->dev, &priv->rng);
+ if (error) {
+ dev_err(&pdev->dev, "failed to register rng device: %d\n",
+ error);
+ return error;
}
dev_info(&pdev->dev, "registered RNG driver\n");
return 0;
-
-out_clk_disable:
- clk_disable(clk);
-out:
- return ret;
-}
-
-static int bcm63xx_rng_remove(struct platform_device *pdev)
-{
-
[PATCH 3/6] hwrng: exynos - make use of devm_hwrng_register
This allows us to get rid of remove() method.
Signed-off-by: Dmitry Torokhov
---
drivers/char/hw_random/exynos-rng.c | 12 +---
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/char/hw_random/exynos-rng.c
b/drivers/char/hw_random/exynos-rng.c
index fed0830..dc4701f 100644
--- a/drivers/char/hw_random/exynos-rng.c
+++ b/drivers/char/hw_random/exynos-rng.c
@@ -131,16 +131,7 @@ static int exynos_rng_probe(struct platform_device *pdev)
pm_runtime_use_autosuspend(&pdev->dev);
pm_runtime_enable(&pdev->dev);
- return hwrng_register(&exynos_rng->rng);
-}
-
-static int exynos_rng_remove(struct platform_device *pdev)
-{
- struct exynos_rng *exynos_rng = platform_get_drvdata(pdev);
-
- hwrng_unregister(&exynos_rng->rng);
-
- return 0;
+ return devm_hwrng_register(&pdev->dev, &exynos_rng->rng);
}
#ifdef CONFIG_PM
@@ -172,7 +163,6 @@ static struct platform_driver exynos_rng_driver = {
.pm = &exynos_rng_pm_ops,
},
.probe = exynos_rng_probe,
- .remove = exynos_rng_remove,
};
module_platform_driver(exynos_rng_driver);
--
2.2.0.rc0.207.ga3a616c
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] hwrng: omap - remove #ifdefery around PM methods
On Thu, Mar 12, 2015 at 08:00:35AM +1100, Herbert Xu wrote: > On Wed, Mar 11, 2015 at 08:44:07AM -0700, Dmitry Torokhov wrote: > > > > SIMPLE_DEV_PM_OPS() produces an empty omap_rng_pm structure in case of > > !CONFIG_PM_SLEEP so neither omap_rng_suspend nor omap_rng_resume will > > end up being referenced. > > OK, could you please resend this patch? Done. Thanks! -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RESEND PATCH] hwrng: omap - remove #ifdefery around PM methods
Instead of using #ifdefs let's mark suspend and resume methods as
__maybe_unused which will suppress compiler warnings about them being
unused and provide better compile coverage.
Because SIMPLE_DEV_PM_OPS() produces an empty omap_rng_pm structure in
case of !CONFIG_PM_SLEEP neither omap_rng_suspend nor omap_rng_resume
will end up being referenced and the change will not result in
increasing image size.
Signed-off-by: Dmitry Torokhov
---
drivers/char/hw_random/omap-rng.c | 15 +++
1 file changed, 3 insertions(+), 12 deletions(-)
diff --git a/drivers/char/hw_random/omap-rng.c
b/drivers/char/hw_random/omap-rng.c
index 7f3597d..5c171b1 100644
--- a/drivers/char/hw_random/omap-rng.c
+++ b/drivers/char/hw_random/omap-rng.c
@@ -422,9 +422,7 @@ static int omap_rng_remove(struct platform_device *pdev)
return 0;
}
-#ifdef CONFIG_PM_SLEEP
-
-static int omap_rng_suspend(struct device *dev)
+static int __maybe_unused omap_rng_suspend(struct device *dev)
{
struct omap_rng_dev *priv = dev_get_drvdata(dev);
@@ -434,7 +432,7 @@ static int omap_rng_suspend(struct device *dev)
return 0;
}
-static int omap_rng_resume(struct device *dev)
+static int __maybe_unused omap_rng_resume(struct device *dev)
{
struct omap_rng_dev *priv = dev_get_drvdata(dev);
@@ -445,18 +443,11 @@ static int omap_rng_resume(struct device *dev)
}
static SIMPLE_DEV_PM_OPS(omap_rng_pm, omap_rng_suspend, omap_rng_resume);
-#defineOMAP_RNG_PM (&omap_rng_pm)
-
-#else
-
-#defineOMAP_RNG_PM NULL
-
-#endif
static struct platform_driver omap_rng_driver = {
.driver = {
.name = "omap_rng",
- .pm = OMAP_RNG_PM,
+ .pm = &omap_rng_pm,
.of_match_table = of_match_ptr(omap_rng_of_match),
},
.probe = omap_rng_probe,
--
2.2.0.rc0.207.ga3a616c
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] hwrng: omap - remove #ifdefery around PM methods
Hi Herbert,
On Wed, Mar 11, 2015 at 09:59:57PM +1100, Herbert Xu wrote:
> On Mon, Mar 09, 2015 at 10:36:36AM -0700, Dmitry Torokhov wrote:
> > Instead of using #ifdefs let's mark suspend and resume methods as
> > __maybe_unused which will suppress compiler warnings about them being
> > unused and provide better compile coverage. This will not increase image
> > size.
> >
> > Signed-off-by: Dmitry Torokhov
> > ---
> > drivers/char/hw_random/omap-rng.c | 15 +++
> > 1 file changed, 3 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/char/hw_random/omap-rng.c
> > b/drivers/char/hw_random/omap-rng.c
> > index 7f3597d..5c171b1 100644
> > --- a/drivers/char/hw_random/omap-rng.c
> > +++ b/drivers/char/hw_random/omap-rng.c
> > @@ -422,9 +422,7 @@ static int omap_rng_remove(struct platform_device *pdev)
> > return 0;
> > }
> >
> > -#ifdef CONFIG_PM_SLEEP
> > -
> > -static int omap_rng_suspend(struct device *dev)
> > +static int __maybe_unused omap_rng_suspend(struct device *dev)
> > {
> > struct omap_rng_dev *priv = dev_get_drvdata(dev);
> >
> > @@ -434,7 +432,7 @@ static int omap_rng_suspend(struct device *dev)
> > return 0;
> > }
> >
> > -static int omap_rng_resume(struct device *dev)
> > +static int __maybe_unused omap_rng_resume(struct device *dev)
> > {
> > struct omap_rng_dev *priv = dev_get_drvdata(dev);
> >
> > @@ -445,18 +443,11 @@ static int omap_rng_resume(struct device *dev)
> > }
> >
> > static SIMPLE_DEV_PM_OPS(omap_rng_pm, omap_rng_suspend, omap_rng_resume);
> > -#defineOMAP_RNG_PM (&omap_rng_pm)
> > -
> > -#else
> > -
> > -#defineOMAP_RNG_PM NULL
> > -
> > -#endif
> >
> > static struct platform_driver omap_rng_driver = {
> > .driver = {
> > .name = "omap_rng",
> > - .pm = OMAP_RNG_PM,
> > + .pm = &omap_rng_pm,
>
> This will cause omap_rng_suspend/omap_rng_resume to be referenced
> always, thus rendering the __maybe_unused moot, no?
SIMPLE_DEV_PM_OPS() produces an empty omap_rng_pm structure in case of
!CONFIG_PM_SLEEP so neither omap_rng_suspend nor omap_rng_resume will
end up being referenced.
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] crypto: amcc: remove incorrect __init/__exit markups
Even if bus is not hot-pluggable, the devices can be bound and unbound
from the driver via sysfs, so we should not be using __init/__exit
annotations on probe() and remove() methods. The only exception is
drivers registered with platform_driver_probe() which specifically
disables sysfs bind/unbind attributes.
Signed-off-by: Dmitry Torokhov
---
drivers/crypto/amcc/crypto4xx_core.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/crypto/amcc/crypto4xx_core.c
b/drivers/crypto/amcc/crypto4xx_core.c
index d02b771..3b28e8c 100644
--- a/drivers/crypto/amcc/crypto4xx_core.c
+++ b/drivers/crypto/amcc/crypto4xx_core.c
@@ -1155,7 +1155,7 @@ struct crypto4xx_alg_common crypto4xx_alg[] = {
/**
* Module Initialization Routine
*/
-static int __init crypto4xx_probe(struct platform_device *ofdev)
+static int crypto4xx_probe(struct platform_device *ofdev)
{
int rc;
struct resource res;
@@ -1263,7 +1263,7 @@ err_alloc_dev:
return rc;
}
-static int __exit crypto4xx_remove(struct platform_device *ofdev)
+static int crypto4xx_remove(struct platform_device *ofdev)
{
struct device *dev = &ofdev->dev;
struct crypto4xx_core_device *core_dev = dev_get_drvdata(dev);
@@ -1291,7 +1291,7 @@ static struct platform_driver crypto4xx_driver = {
.of_match_table = crypto4xx_match,
},
.probe = crypto4xx_probe,
- .remove = __exit_p(crypto4xx_remove),
+ .remove = crypto4xx_remove,
};
module_platform_driver(crypto4xx_driver);
--
2.2.0.rc0.207.ga3a616c
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] crypto: qat - remove incorrect __exit markup
PCI bus is hot-pluggable, and even if it wasn't one can still unbind the
device from driver via sysfs, so we should not make driver's remove
method as __exit.
Signed-off-by: Dmitry Torokhov
---
drivers/crypto/qat/qat_dh895xcc/adf_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/crypto/qat/qat_dh895xcc/adf_drv.c
b/drivers/crypto/qat/qat_dh895xcc/adf_drv.c
index 8ffdb95..e7af9d5 100644
--- a/drivers/crypto/qat/qat_dh895xcc/adf_drv.c
+++ b/drivers/crypto/qat/qat_dh895xcc/adf_drv.c
@@ -379,7 +379,7 @@ out_err:
return ret;
}
-static void __exit adf_remove(struct pci_dev *pdev)
+static void adf_remove(struct pci_dev *pdev)
{
struct adf_accel_dev *accel_dev = adf_devmgr_pci_to_accel_dev(pdev);
--
2.2.0.rc0.207.ga3a616c
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] hwrng: omap - remove #ifdefery around PM methods
Instead of using #ifdefs let's mark suspend and resume methods as
__maybe_unused which will suppress compiler warnings about them being
unused and provide better compile coverage. This will not increase image
size.
Signed-off-by: Dmitry Torokhov
---
drivers/char/hw_random/omap-rng.c | 15 +++
1 file changed, 3 insertions(+), 12 deletions(-)
diff --git a/drivers/char/hw_random/omap-rng.c
b/drivers/char/hw_random/omap-rng.c
index 7f3597d..5c171b1 100644
--- a/drivers/char/hw_random/omap-rng.c
+++ b/drivers/char/hw_random/omap-rng.c
@@ -422,9 +422,7 @@ static int omap_rng_remove(struct platform_device *pdev)
return 0;
}
-#ifdef CONFIG_PM_SLEEP
-
-static int omap_rng_suspend(struct device *dev)
+static int __maybe_unused omap_rng_suspend(struct device *dev)
{
struct omap_rng_dev *priv = dev_get_drvdata(dev);
@@ -434,7 +432,7 @@ static int omap_rng_suspend(struct device *dev)
return 0;
}
-static int omap_rng_resume(struct device *dev)
+static int __maybe_unused omap_rng_resume(struct device *dev)
{
struct omap_rng_dev *priv = dev_get_drvdata(dev);
@@ -445,18 +443,11 @@ static int omap_rng_resume(struct device *dev)
}
static SIMPLE_DEV_PM_OPS(omap_rng_pm, omap_rng_suspend, omap_rng_resume);
-#defineOMAP_RNG_PM (&omap_rng_pm)
-
-#else
-
-#defineOMAP_RNG_PM NULL
-
-#endif
static struct platform_driver omap_rng_driver = {
.driver = {
.name = "omap_rng",
- .pm = OMAP_RNG_PM,
+ .pm = &omap_rng_pm,
.of_match_table = of_match_ptr(omap_rng_of_match),
},
.probe = omap_rng_probe,
--
2.2.0.rc0.207.ga3a616c
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] hwrng: octeon - remove incorrect __exit markups
Even if bus is not hot-pluggable, the devices can be unbound from the
driver via sysfs, so we should not be using __exit annotations on
remove() methods. The only exception is drivers registered with
platform_driver_probe() which specifically disables sysfs bind/unbind
attributes
Signed-off-by: Dmitry Torokhov
---
drivers/char/hw_random/octeon-rng.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/char/hw_random/octeon-rng.c
b/drivers/char/hw_random/octeon-rng.c
index be1c3f6..6234a4a 100644
--- a/drivers/char/hw_random/octeon-rng.c
+++ b/drivers/char/hw_random/octeon-rng.c
@@ -105,7 +105,7 @@ static int octeon_rng_probe(struct platform_device *pdev)
return 0;
}
-static int __exit octeon_rng_remove(struct platform_device *pdev)
+static int octeon_rng_remove(struct platform_device *pdev)
{
struct hwrng *rng = platform_get_drvdata(pdev);
@@ -119,7 +119,7 @@ static struct platform_driver octeon_rng_driver = {
.name = "octeon_rng",
},
.probe = octeon_rng_probe,
- .remove = __exit_p(octeon_rng_remove),
+ .remove = octeon_rng_remove,
};
module_platform_driver(octeon_rng_driver);
--
2.2.0.rc0.207.ga3a616c
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] hwrng: pseries-rng - remove incorrect __init/__exit markups
Even if bus is not hot-pluggable, the devices can be unbound from the
driver via sysfs, so we should not be using __exit annotations on
remove() methods. The only exception is drivers registered with
platform_driver_probe() which specifically disables sysfs bind/unbind
attributes.
Similarly probe() methods should not be marked __init unless
platform_driver_probe() is used.
Signed-off-by: Dmitry Torokhov
---
drivers/char/hw_random/pseries-rng.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/char/hw_random/pseries-rng.c
b/drivers/char/hw_random/pseries-rng.c
index bcf86f9..63ce51d 100644
--- a/drivers/char/hw_random/pseries-rng.c
+++ b/drivers/char/hw_random/pseries-rng.c
@@ -61,13 +61,13 @@ static struct hwrng pseries_rng = {
.read = pseries_rng_read,
};
-static int __init pseries_rng_probe(struct vio_dev *dev,
+static int pseries_rng_probe(struct vio_dev *dev,
const struct vio_device_id *id)
{
return hwrng_register(&pseries_rng);
}
-static int __exit pseries_rng_remove(struct vio_dev *dev)
+static int pseries_rng_remove(struct vio_dev *dev)
{
hwrng_unregister(&pseries_rng);
return 0;
--
2.2.0.rc0.207.ga3a616c
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] hwrng: omap - remove incorrect __exit markups
Even if bus is not hot-pluggable, the devices can be unbound from the
driver via sysfs, so we should not be using __exit annotations on
remove() methods. The only exception is drivers registered with
platform_driver_probe() which specifically disables sysfs bind/unbind
attributes.
Signed-off-by: Dmitry Torokhov
---
drivers/char/hw_random/omap-rng.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/char/hw_random/omap-rng.c
b/drivers/char/hw_random/omap-rng.c
index d14dcf7..7f3597d 100644
--- a/drivers/char/hw_random/omap-rng.c
+++ b/drivers/char/hw_random/omap-rng.c
@@ -408,7 +408,7 @@ err_ioremap:
return ret;
}
-static int __exit omap_rng_remove(struct platform_device *pdev)
+static int omap_rng_remove(struct platform_device *pdev)
{
struct omap_rng_dev *priv = platform_get_drvdata(pdev);
@@ -460,7 +460,7 @@ static struct platform_driver omap_rng_driver = {
.of_match_table = of_match_ptr(omap_rng_of_match),
},
.probe = omap_rng_probe,
- .remove = __exit_p(omap_rng_remove),
+ .remove = omap_rng_remove,
};
module_platform_driver(omap_rng_driver);
--
2.2.0.rc0.207.ga3a616c
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
