[lkml]   [2011]   [Mar]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH 1/7] char/mei: PCI device and char driver support.
    On Tuesday 22 March 2011, Oren Weil wrote:
    > +static ssize_t mei_read(struct file *file, char __user *ubuf,
    > + size_t length, loff_t *offset)
    > +{
    > + int i;
    > + int rets;
    > + int err;
    > + int if_num = iminor(file->f_dentry->d_inode);
    > + struct mei_file_private *file_ext = file->private_data;
    > + struct mei_cb_private *priv_cb_pos = NULL;
    > + struct mei_cb_private *priv_cb = NULL;
    > + struct mei_device *dev;
    > +
    > + if (!mei_device)
    > + return -ENODEV;
    > +
    > + dev = pci_get_drvdata(mei_device);
    > + if (if_num != MEI_MINOR_NUMBER || !dev || !file_ext)
    > + return -ENODEV;
    > +

    I'm very confused by this code and how you support multiple
    instances of PCI devices, cb_list entries and concurrent connections:

    * You look up a global pointer to mei_device, when there could
    be multiple PCI devices.

    * You use the minor number of the chardev as an index throughout
    the code, but bail out if it does not match a constant.

    * You walk a list of mei_cb_private entries when you should have
    just a single one for this file. In other places, you walk a list
    of mei_file_private entries.

    I think you should try to document your data structures and how
    they relate to each other, and remove all that turn out not to
    be needed in the process.

    From the brief look I had at this rather complex code, I would
    assume that it could look something like this:

    * In the PCI probe function, create a misc character device for
    every instance you find behind each device. Remove the global
    variables and constants.

    * Unify mei_file_private and mei_cb_private, allocate at open
    time but fill when the ioctl gets called.

    Would that work, or am I missing something major?


     \ /
      Last update: 2011-03-22 18:25    [W:0.022 / U:20.120 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site