On Tue, Jun 17, 2008 at 05:39:52PM -0500, Nathan Lynch wrote:
> Hi, mainly a couple of coding style things, but one minor bug (I
> think).
>
> [EMAIL PROTECTED] wrote:
> > From: Sonny Rao <[EMAIL PROTECTED]>
> >
> > +static int bsr_mmap(struct file *filp, struct vm_area_struct *vma)
> > +{
> > + unsigned long size = vma->vm_end - vma->vm_start;
> > + struct bsr_dev *dev = filp->private_data;
> > +
> > + if (size > dev->bsr_len || (size & (PAGE_SIZE-1)))
> > + return -EINVAL;
> > +
> > + vma->vm_flags |= (VM_IO | VM_DONTEXPAND);
> > + vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> > +
> > + if (io_remap_pfn_range(vma, vma->vm_start, dev->bsr_addr >> PAGE_SHIFT,
> > + size, vma->vm_page_prot))
> > + return -EAGAIN;
>
> Indentation is wrong.
Yeah I noticed that too.
> > +static void bsr_cleanup_devs(void)
> > +{
> > + int i;
> > + for (i=0 ; i < num_bsr_devs; i++) {
>
> i = 0
>
> > + struct bsr_dev *cur = bsr_devs + i;
> > + if (cur->bsr_device) {
> > + cdev_del(&cur->bsr_cdev);
> > + device_del(cur->bsr_device);
> > + }
> > + }
> > +
> > + kfree(bsr_devs);
> > +}
> > +
> > +static int bsr_create_devs(struct device_node *bn)
> > +{
> > + int reg_len, bsr_stride_len, bsr_bytes_len;
> > + const u64 *reg;
> > + const u32 *bsr_stride;
> > + const u32 *bsr_bytes;
> > + unsigned i;
> > +
> > + reg = of_get_property(bn, "reg", ®_len);
> > + bsr_stride = of_get_property(bn, "ibm,lock-stride", &bsr_stride_len);
> > + bsr_bytes = of_get_property(bn, "ibm,#lock-bytes", &bsr_bytes_len);
> > +
> > + if (!reg || !bsr_stride || !bsr_bytes ||
> > + (bsr_stride_len != bsr_bytes_len) ||
> > + (bsr_stride_len/4 != reg_len/16)) {
>
> bsr_stride_len / 4 != reg_len / 16
>
>
> > + printk(KERN_ERR "bsr of-node has missing/incorrect property\n");
> > + return -ENODEV;
> > + }
>
> ...
>
> > +static int __init bsr_init(void)
> > +{
> > + struct device_node *np;
> > + dev_t bsr_dev = MKDEV(bsr_major, 0);
> > + int ret = -ENODEV;
> > + int result;
> > +
> > + np = of_find_compatible_node(NULL, "ibm,bsr", "ibm,bsr");
> > + if (!np)
> > + goto out_err;
> > +
> > + bsr_class = class_create(THIS_MODULE, "bsr");
> > + if (IS_ERR(bsr_class)) {
> > + printk(KERN_ERR "class_create() failed for bsr_class\n");
> > + goto out_err;
>
> At this point I think you can leak a reference to np.
Yeah, you're right.
>
> > + }
> > + bsr_class->dev_attrs = bsr_dev_attrs;
> > +
> > + result = alloc_chrdev_region(&bsr_dev, 0, BSR_MAX_DEVS, "bsr");
> > + bsr_major = MAJOR(bsr_dev);
> > + if (result < 0) {
> > + printk(KERN_ERR "alloc_chrdev_region() failed for bsr\n");
> > + goto out_err_1;
> > + }
> > +
> > + if ((ret = bsr_create_devs(np)) < 0)
> > + goto out_err_2;
> > +
> > + of_node_put(np);
> > +
> > + return 0;
> > +
> > + out_err_2:
> > + unregister_chrdev_region(bsr_dev, BSR_MAX_DEVS);
> > +
> > + out_err_1:
> > + class_destroy(bsr_class);
> > + of_node_put(np);
> > +
> > + out_err:
> > +
> > + return ret;
> > +}
Ok Will fix and send out again
--
Sonny Rao, LTC Ozlabs
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
