lkml.org 
[lkml]   [2015]   [May]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v5 WIP 3/5] i2c-parport: define ports to connect
    From
    Date
    Hi Sudip,

    Le Wednesday 06 May 2015 à 15:46 +0530, Sudip Mukherjee a écrit :
    > as of now i2c-parport was connecting to all the available parallel
    > ports. Lets limit that to maximum of 4 instances and at the same time
    > define which instance connects to which parallel port

    A leading capital and a trailing dot would look better.

    > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
    > ---
    > drivers/i2c/busses/i2c-parport.c | 21 +++++++++++++++++++++
    > 1 file changed, 21 insertions(+)
    >
    > diff --git a/drivers/i2c/busses/i2c-parport.c b/drivers/i2c/busses/i2c-parport.c
    > index a1fac5a..a9b25c3 100644
    > --- a/drivers/i2c/busses/i2c-parport.c
    > +++ b/drivers/i2c/busses/i2c-parport.c
    > @@ -46,6 +46,9 @@ struct i2c_par {
    >
    > static LIST_HEAD(adapter_list);
    > static DEFINE_MUTEX(adapter_list_lock);
    > +#define MAX_DEVICE 4
    > +static int parport[MAX_DEVICE] = {0, -1, -1, -1};
    > +
    >
    > /* ----- Low-level parallel port access ----------------------------------- */
    >
    > @@ -163,6 +166,18 @@ static void i2c_parport_irq(void *data)
    > static void i2c_parport_attach(struct parport *port)
    > {
    > struct i2c_par *adapter;
    > + int i;
    > +
    > + for (i = 0; i < MAX_DEVICE; i++) {
    > + if (parport[i] == -1)
    > + continue;
    > + if (port->number == parport[i])
    > + break;
    > + }
    > + if (i == MAX_DEVICE) {
    > + pr_err("port mentioned not found\n");

    This error message needs to be improved. Someone seeing this in his/her
    logs would have no idea where it comes from and what it means exactly.
    You want to add "i2c-parport: " at the beginning, and say which port
    number was specified.

    > + return;
    > + }
    >
    > adapter = kzalloc(sizeof(struct i2c_par), GFP_KERNEL);
    > if (adapter == NULL) {
    > @@ -298,5 +313,11 @@ MODULE_AUTHOR("Jean Delvare <jdelvare@suse.de>");
    > MODULE_DESCRIPTION("I2C bus over parallel port");
    > MODULE_LICENSE("GPL");
    >
    > +module_param_array(parport, int, NULL, 0);
    > +MODULE_PARM_DESC(parport, "Atmost 4 instances are allowed.\n"

    You should first say what the parameter does, before going into the
    details. Please use __stringify(MAX_DEVICE) instead of hard-coding 4, so
    that it doesn't need to be updated if MAX_DEVICE ever changes.

    > + "Mention portnumbers in array.\n"

    Missing space between "port" and "numbers". Don't mention that this is
    an array here, "modinfo" already prints the parameter type. For improved
    readability, please also add a leading space to the second and following
    lines (as done for parameter "type" in i2c-parport.h.)

    > + "If the port is not to be used mention -1.\n"
    > + "Default is one instance connected to parport0\n");

    Missing trailing dot.

    > +
    > module_init(i2c_parport_init);
    > module_exit(i2c_parport_exit);

    I tested this patch and it works fine.

    Tested-by: Jean Delvare <jdelvare@suse.de>

    --
    Jean Delvare
    SUSE L3 Support



    \
     
     \ /
      Last update: 2015-05-19 10:41    [W:2.566 / U:0.316 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site