On Sat, Aug 09, 2014 at 08:16:37PM +0200, Konrad Zapalowicz wrote:
> This commit binds global variables of dgnc driver in a structure so
> that it is logically consistent. The structure is accessed via getter
> function and as a result the externing of globals is removed. The names
> of the variables are also changed to be more eye friendly.
>
> Signed-off-by: Konrad Zapalowicz <bergo.torino@xxxxxxxxx>
> ---
>
> This patch applies on top of my two previous patch series. In case it
> is an issue I can resubmit the whole series for dgnc driver as one
> patch set - just let me know.
>
> drivers/staging/dgnc/dgnc_driver.c | 82 +++++++++++++++++++-------------------
> drivers/staging/dgnc/dgnc_driver.h | 20 ++++++----
> drivers/staging/dgnc/dgnc_mgmt.c | 48 ++++++++++++----------
> drivers/staging/dgnc/dgnc_sysfs.c | 31 ++++++++++----
> 4 files changed, 102 insertions(+), 79 deletions(-)
>
> diff --git a/drivers/staging/dgnc/dgnc_driver.c b/drivers/staging/dgnc/dgnc_driver.c
> index 724e4ab..9a23e9a 100644
> --- a/drivers/staging/dgnc/dgnc_driver.c
> +++ b/drivers/staging/dgnc/dgnc_driver.c
> @@ -83,17 +83,11 @@ static const struct file_operations dgnc_BoardFops = {
> .release = dgnc_mgmt_close
> };
>
> -
> -/*
> - * Globals
> - */
> -uint dgnc_NumBoards;
> -struct dgnc_board *dgnc_Board[MAXBOARDS];
> -DEFINE_SPINLOCK(dgnc_global_lock);
> -int dgnc_driver_state = DRIVER_INITIALIZED;
> -ulong dgnc_poll_counter;
> -uint dgnc_Major;
> -int dgnc_poll_tick = 20; /* Poll interval - 20 ms */
> +static struct dgnc_driver driver = {
> + .lock = __SPIN_LOCK_UNLOCKED(driver.lock),
> + .state = DRIVER_INITIALIZED,
> + .poll_tick = 20,
> +};
While it is great to get rid of a bunch of globals, you now just
replaced it with a different one :(
This all should be dynamic, no need for a static array of devices at
all.
There shouldn't be a need for the driver_state at all, and the poll_tick
should be device specific, not driver-wide. The major is fine, but it
can just be a single static variable, right?
So, I'd like to see the array of devices go away, can you do that
instead of just moving it elsewhere?
thanks,
greg k-h
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
