Messages in this thread | | | From | Michael Buesch <> | Subject | Re: [patch 3/6] New Generic HW RNG (#2) | Date | Sun, 7 May 2006 17:39:44 +0200 |
| |
On Sunday 07 May 2006 17:22, you wrote: > On Sun, May 07, 2006 at 04:38:09PM +0200, Michael Buesch wrote: > > Add a driver for the x86 RNG. > > This driver is ported from the old hw_random.c > > > [skip] > > +static int __init intel_init(struct hwrng *rng) > > Cannot be __init anymore - now rng->init could be called at any time.
Sure, will fix this.
> Also, there is another problem with putting this function into > rng->init - if another RNG has been registered when this module is > loaded, ->init will not be called during hwrng_register(), so the > module load will succeed even if the chipset does not have RNG > hardware.
Ok, I see. The question is, are we going to hwrng_register() the intel, althought there is no device? We check for the PCI IDs. Anyway. This should be fixed. And I think this is also a good time to split the x86 driver, so all this init-stuff is much cleaner.
> > +{ > > + void __iomem *rng_mem; > > + int rc; > > + u8 hw_status; > > + > > + DPRINTK ("ENTER\n"); > > + > > + rng_mem = ioremap (INTEL_RNG_ADDR, INTEL_RNG_ADDR_LEN); > > + if (rng_mem == NULL) { > > + printk (KERN_ERR PFX "cannot ioremap RNG Memory\n"); > > + rc = -EBUSY; > > + goto err_out; > > + } > > + rng->priv = (unsigned long)rng_mem; > > + > > + /* Check for Intel 82802 */ > > + hw_status = intel_hwstatus (rng_mem); > > + if ((hw_status & INTEL_RNG_PRESENT) == 0) { > > + printk (KERN_ERR PFX "RNG not detected\n"); > > + rc = -ENODEV; > > + goto err_out_free_map; > > + } > > + > > + /* turn RNG h/w on, if it's off */ > > + if ((hw_status & INTEL_RNG_ENABLED) == 0) > > + hw_status = intel_hwstatus_set (rng_mem, hw_status | INTEL_RNG_ENABLED); > > + if ((hw_status & INTEL_RNG_ENABLED) == 0) { > > + printk (KERN_ERR PFX "cannot enable RNG, aborting\n"); > > + rc = -EIO; > > + goto err_out_free_map; > > + } > > + > > + DPRINTK ("EXIT, returning 0\n"); > > + return 0; > > + > > +err_out_free_map: > > + iounmap (rng_mem); > > +err_out: > > + DPRINTK ("EXIT, returning %d\n", rc); > > + return rc; > > +} > > + > [skip] > > +static int __init amd_init(struct hwrng *rng) > > Again, __init is wrong. > > [skip] > > +static int __init via_init(struct hwrng *rng) > > This __init is wrong too. > > [skip] >
Ah, and I found another bug in hwrng_unregister: current_rng = list_entry(rng_list.prev, struct hwrng, list); current_rng->init() should be called here (if nonNULL). If that fails current_rng = NULL;
Will fix that, too.
-- Greetings Michael. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |