Message ID | 20101111120326.22020026@doriath |
---|---|
State | New |
Headers | show |
On 11/11/2010 08:03 AM, Luiz Capitulino wrote: > Functions register_ioport_read() and register_ioport_write() are almost > identical, the only difference is that they write to different arrays. > > Introduce register_ioport_rw() to handle this difference and change both > functions to use it instead of duplicating code. > > Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com> > While it make take some scripting, let's do a global query/replace. Having two interfaces where one is only scarely used hurts code readability. We need to do the janitorial work when changing interfaces like this. Regards, Anthony Liguori > --- > > v2: Fix error messages and make register_ioport_rw() register both handlers > at the same call > > ioport.c | 37 ++++++++++++++++++------------------- > 1 files changed, 18 insertions(+), 19 deletions(-) > > diff --git a/ioport.c b/ioport.c > index ec3dc65..4560973 100644 > --- a/ioport.c > +++ b/ioport.c > @@ -137,41 +137,40 @@ static int ioport_bsize(int size, int *bsize) > } > > /* size is the word size in byte */ > -int register_ioport_read(pio_addr_t start, int length, int size, > - IOPortReadFunc *func, void *opaque) > +static int register_ioport_rw(pio_addr_t start, int length, int size, > + IOPortReadFunc *read_func, > + IOPortWriteFunc *write_func, void *opaque) > { > int i, bsize; > > if (ioport_bsize(size,&bsize)) { > - hw_error("register_ioport_read: invalid size"); > + hw_error("register_ioport_rw: invalid size"); > return -1; > } > for(i = start; i< start + length; i += size) { > - ioport_read_table[bsize][i] = func; > + if (read_func) { > + ioport_read_table[bsize][i] = read_func; > + } > + if (write_func) { > + ioport_write_table[bsize][i] = write_func; > + } > if (ioport_opaque[i] != NULL&& ioport_opaque[i] != opaque) > - hw_error("register_ioport_read: invalid opaque"); > + hw_error("register_ioport_rw: invalid opaque"); > ioport_opaque[i] = opaque; > } > return 0; > } > > -/* size is the word size in byte */ > +int register_ioport_read(pio_addr_t start, int length, int size, > + IOPortReadFunc *func, void *opaque) > +{ > + return register_ioport_rw(start, length, size, func, NULL, opaque); > +} > + > int register_ioport_write(pio_addr_t start, int length, int size, > IOPortWriteFunc *func, void *opaque) > { > - int i, bsize; > - > - if (ioport_bsize(size,&bsize)) { > - hw_error("register_ioport_write: invalid size"); > - return -1; > - } > - for(i = start; i< start + length; i += size) { > - ioport_write_table[bsize][i] = func; > - if (ioport_opaque[i] != NULL&& ioport_opaque[i] != opaque) > - hw_error("register_ioport_write: invalid opaque"); > - ioport_opaque[i] = opaque; > - } > - return 0; > + return register_ioport_rw(start, length, size, NULL, func, opaque); > } > > void isa_unassign_ioport(pio_addr_t start, int length) >
On Fri, 19 Nov 2010 19:50:10 -0600 Anthony Liguori <anthony@codemonkey.ws> wrote: > On 11/11/2010 08:03 AM, Luiz Capitulino wrote: > > Functions register_ioport_read() and register_ioport_write() are almost > > identical, the only difference is that they write to different arrays. > > > > Introduce register_ioport_rw() to handle this difference and change both > > functions to use it instead of duplicating code. > > > > Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com> > > > > While it make take some scripting, let's do a global query/replace. > > Having two interfaces where one is only scarely used hurts code > readability. We need to do the janitorial work when changing interfaces > like this. The goal of this patch was just to make register_ioport_read() and register_ioport_write() use common code, what janitorial work should we do? Export register_ioport_rw() and change drivers to use it, instead of calling register_ioport_read() and register_ioport_write()? > > Regards, > > Anthony Liguori > > > --- > > > > v2: Fix error messages and make register_ioport_rw() register both handlers > > at the same call > > > > ioport.c | 37 ++++++++++++++++++------------------- > > 1 files changed, 18 insertions(+), 19 deletions(-) > > > > diff --git a/ioport.c b/ioport.c > > index ec3dc65..4560973 100644 > > --- a/ioport.c > > +++ b/ioport.c > > @@ -137,41 +137,40 @@ static int ioport_bsize(int size, int *bsize) > > } > > > > /* size is the word size in byte */ > > -int register_ioport_read(pio_addr_t start, int length, int size, > > - IOPortReadFunc *func, void *opaque) > > +static int register_ioport_rw(pio_addr_t start, int length, int size, > > + IOPortReadFunc *read_func, > > + IOPortWriteFunc *write_func, void *opaque) > > { > > int i, bsize; > > > > if (ioport_bsize(size,&bsize)) { > > - hw_error("register_ioport_read: invalid size"); > > + hw_error("register_ioport_rw: invalid size"); > > return -1; > > } > > for(i = start; i< start + length; i += size) { > > - ioport_read_table[bsize][i] = func; > > + if (read_func) { > > + ioport_read_table[bsize][i] = read_func; > > + } > > + if (write_func) { > > + ioport_write_table[bsize][i] = write_func; > > + } > > if (ioport_opaque[i] != NULL&& ioport_opaque[i] != opaque) > > - hw_error("register_ioport_read: invalid opaque"); > > + hw_error("register_ioport_rw: invalid opaque"); > > ioport_opaque[i] = opaque; > > } > > return 0; > > } > > > > -/* size is the word size in byte */ > > +int register_ioport_read(pio_addr_t start, int length, int size, > > + IOPortReadFunc *func, void *opaque) > > +{ > > + return register_ioport_rw(start, length, size, func, NULL, opaque); > > +} > > + > > int register_ioport_write(pio_addr_t start, int length, int size, > > IOPortWriteFunc *func, void *opaque) > > { > > - int i, bsize; > > - > > - if (ioport_bsize(size,&bsize)) { > > - hw_error("register_ioport_write: invalid size"); > > - return -1; > > - } > > - for(i = start; i< start + length; i += size) { > > - ioport_write_table[bsize][i] = func; > > - if (ioport_opaque[i] != NULL&& ioport_opaque[i] != opaque) > > - hw_error("register_ioport_write: invalid opaque"); > > - ioport_opaque[i] = opaque; > > - } > > - return 0; > > + return register_ioport_rw(start, length, size, NULL, func, opaque); > > } > > > > void isa_unassign_ioport(pio_addr_t start, int length) > > >
diff --git a/ioport.c b/ioport.c index ec3dc65..4560973 100644 --- a/ioport.c +++ b/ioport.c @@ -137,41 +137,40 @@ static int ioport_bsize(int size, int *bsize) } /* size is the word size in byte */ -int register_ioport_read(pio_addr_t start, int length, int size, - IOPortReadFunc *func, void *opaque) +static int register_ioport_rw(pio_addr_t start, int length, int size, + IOPortReadFunc *read_func, + IOPortWriteFunc *write_func, void *opaque) { int i, bsize; if (ioport_bsize(size, &bsize)) { - hw_error("register_ioport_read: invalid size"); + hw_error("register_ioport_rw: invalid size"); return -1; } for(i = start; i < start + length; i += size) { - ioport_read_table[bsize][i] = func; + if (read_func) { + ioport_read_table[bsize][i] = read_func; + } + if (write_func) { + ioport_write_table[bsize][i] = write_func; + } if (ioport_opaque[i] != NULL && ioport_opaque[i] != opaque) - hw_error("register_ioport_read: invalid opaque"); + hw_error("register_ioport_rw: invalid opaque"); ioport_opaque[i] = opaque; } return 0; } -/* size is the word size in byte */ +int register_ioport_read(pio_addr_t start, int length, int size, + IOPortReadFunc *func, void *opaque) +{ + return register_ioport_rw(start, length, size, func, NULL, opaque); +} + int register_ioport_write(pio_addr_t start, int length, int size, IOPortWriteFunc *func, void *opaque) { - int i, bsize; - - if (ioport_bsize(size, &bsize)) { - hw_error("register_ioport_write: invalid size"); - return -1; - } - for(i = start; i < start + length; i += size) { - ioport_write_table[bsize][i] = func; - if (ioport_opaque[i] != NULL && ioport_opaque[i] != opaque) - hw_error("register_ioport_write: invalid opaque"); - ioport_opaque[i] = opaque; - } - return 0; + return register_ioport_rw(start, length, size, NULL, func, opaque); } void isa_unassign_ioport(pio_addr_t start, int length)
Functions register_ioport_read() and register_ioport_write() are almost identical, the only difference is that they write to different arrays. Introduce register_ioport_rw() to handle this difference and change both functions to use it instead of duplicating code. Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> --- v2: Fix error messages and make register_ioport_rw() register both handlers at the same call ioport.c | 37 ++++++++++++++++++------------------- 1 files changed, 18 insertions(+), 19 deletions(-)