* 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
