On 2016/4/7 22:55, Mathieu Poirier wrote: > On 7 April 2016 at 02:23, Kefeng Wang <wangkefeng.wang@xxxxxxxxxx> wrote: >> This adds the Hisilicon Random Number Generator(RNG) support, >> which is found in Hip04 and Hip05 soc. >> >> Signed-off-by: Kefeng Wang <wangkefeng.wang@xxxxxxxxxx> >> --- >> drivers/char/hw_random/Kconfig | 13 ++++ >> drivers/char/hw_random/Makefile | 1 + >> drivers/char/hw_random/hisi-rng.c | 121 ++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 135 insertions(+) >> create mode 100644 drivers/char/hw_random/hisi-rng.c [...] >> + >> +#define RNG_SEED 0x0 >> +#define RNG_CTRL 0x4 >> + #define RNG_SEED_SEL BIT(2) >> + #define RNG_RING_EN BIT(1) >> + #define RNG_EN BIT(0) >> +#define RNG_RAN_NUM 0x10 >> +#define RNG_PHY_SEED 0x14 >> +#define RNG_RELOAD_ERR 0x84 >> + >> +#define to_hisi_rng(p) container_of(p, struct hisi_rng, rng) >> + >> +static int seed_sel; >> +module_param(seed_sel, int, S_IRUGO); >> +MODULE_PARM_DESC(seed_sel, "Auto reload seed. 0, use LFSR(default); 1, use ring oscillator."); >> + >> +struct hisi_rng { >> + void __iomem *base; >> + struct hwrng rng; >> +}; >> + >> +static int hisi_rng_init(struct hwrng *rng) >> +{ >> + struct hisi_rng *hrng = to_hisi_rng(rng); >> + int val = RNG_EN; >> + u32 seed; >> + >> + /* get a random number as initial seed */ >> + get_random_bytes(&seed, sizeof(seed)); >> + >> + writel_relaxed(seed, hrng->base + RNG_SEED); >> + >> + if (seed_sel == 1) >> + val |= RNG_RING_EN | RNG_SEED_SEL; > > Hello Kefeng, > > Please explain what this mode does. Otherwise reviewers have to make > a guess based on the description of the module parameter. The seed is reload periodically, there are two choice of seeds, default using the val from LFSR, seed_sel = 1 will use the val generated by ring oscillator. > >> + >> + writel_relaxed(val, hrng->base + RNG_CTRL); >> + return 0; >> +} >> + >> +static void hisi_rng_cleanup(struct hwrng *rng) >> +{ >> + struct hisi_rng *hrng = to_hisi_rng(rng); >> + >> + writel_relaxed(0, hrng->base + RNG_CTRL); >> +} >> + >> +static int hisi_rng_read(struct hwrng *rng, void *buf, size_t max, >> + bool wait) > > What are variable 'max' and 'wait' used for? As far as I can tell > they can be removed. Ok, will use data_read callback in struct hwrng. > >> +{ >> + struct hisi_rng *hrng = to_hisi_rng(rng); >> + u32 *data = buf; >> + >> + *data = readl_relaxed(hrng->base + RNG_RAN_NUM); >> + return 4; > > Why not simply doing the readl_relaxed() in the the code rather than > introducing a new function? Not clear, your mean introduce the data variable? Will use following data_read, thanks. static int hisi_rng_data_read(struct hwrng *rng, u32 *data) { struct hisi_rng *hrng = to_hisi_rng(rng); *data = readl_relaxed(hrng->base + RNG_RAN_NUM); return 4; } Thanks, Kefeng -- 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