* Ben Dooks | 2009-05-07 22:39:22 [+0100]: Sorry for the late reply. >> diff --git a/drivers/crypto/mv_crypto.c b/drivers/crypto/mv_crypto.c >> new file mode 100644 >> index 0000000..40eb083 >> --- /dev/null >> +++ b/drivers/crypto/mv_crypto.c >> +struct req_progress { >> + struct sg_mapping_iter src_sg_it; >> + struct sg_mapping_iter dst_sg_it; >> + >> + /* src mostly */ >> + int this_sg_b_left; >> + int src_start; >> + int crypt_len; >> + /* dst mostly */ >> + int this_dst_sg_b_left; >> + int dst_start; >> + int total_req_bytes; >> +}; > >kerneldoc style documentation wouldn't go amiss here. added >> + >> +static void reg_write(void __iomem *mem, u32 val) >> +{ >> + __raw_writel(val, mem); >> +} >> + >> +static u32 reg_read(void __iomem *mem) >> +{ >> + return __raw_readl(mem); >> +} > >do you really need to wrapper these? Not really. Initially I planned to pass the device handle instead of the memory pointer. Also using (addr, val) looks better than the other way around. >it is also readl/writel for pointers obtained from ioremap() correct. So I get rid of my wrapper and switch to readl/writel >> + >> +#if 0 >> +static void hex_dump(unsigned char *info, unsigned char *buf, unsigned int len) >> +{ >> + printk(KERN_ERR "%s\n", info); >> + print_hex_dump(KERN_ERR, "", DUMP_PREFIX_OFFSET, >> + 16, 1, >> + buf, len, false); >> + printk(KERN_CONT "\n"); >> +} >> +#endif > >#if 0 considered bad. I needed this a few times. Now I don't and its gone. >> + >> +static int m_probe(struct platform_device *pdev) >> +{ >> + struct crypto_priv *cp; >> + struct resource *res; >> + int irq; >> + int ret; >> + >> + if (cpg) { >> + printk(KERN_ERR "Second crypto dev?\n"); >> + return -EBUSY; >> + } >> + >> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs"); >> + if (!res) >> + return -ENODEV; > >Returning -ENODEV is considered harmful, it will not trigger any warning >from the driver core. I switched to ENXIO because that fits better (No such device or address) I thing. However this also doesn't trigger any warning from the core. What do you suggest? > >-- >Ben Thanks for the review Ben. Sebastian -- 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