Re: [PATCH 1/2] stringlist-functions: add sorted insert

Re: [PATCH 1/2] stringlist-functions: add sorted insert

[Sascha Hauer]

On Fri, Dec 09, 2011 at 10:52:47AM +0100, Alexander Aring wrote:
> Add sorted insert in stringlist functions.
> Also added function to checked if string is already
> in string list.
> 
> Signed-off-by: Alexander Aring <a.aring@phytec.de>
> ---
>  include/stringlist.h |    2 ++
>  lib/stringlist.c     |   49 +++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 51 insertions(+), 0 deletions(-)
> 
> diff --git a/include/stringlist.h b/include/stringlist.h
> index 3453e9a..c923542 100644
> --- a/include/stringlist.h
> +++ b/include/stringlist.h
> @@ -9,6 +9,8 @@ struct string_list {
>  };
>  
>  int string_list_add(struct string_list *sl, char *str);
> +int string_list_add_sorted(struct string_list *sl, char *str);
> +int string_list_contains(struct string_list *sl, char *str);
>  void string_list_print_by_column(struct string_list *sl);
>  
>  static inline void string_list_init(struct string_list *sl)
> diff --git a/lib/stringlist.c b/lib/stringlist.c
> index 9ccf8fa..40a0fc9 100644
> --- a/lib/stringlist.c
> +++ b/lib/stringlist.c
> @@ -3,6 +3,29 @@
>  #include <malloc.h>
>  #include <stringlist.h>
>  
> +static int string_list_compare(struct list_head *a, struct list_head *b)
> +{
> +	int result = 0;
> +	char *achr, *bchr;
> +	achr = ((struct string_list *)(a))->str;
> +	bchr = ((struct string_list *)(b))->str;
> +
> +	while (*achr != '\0' || *bchr != '\0') {
> +		result = *achr - *bchr;
> +		if (result < 0)
> +			return -1;
> +		if (result > 0)
> +			return 0;
> +		achr++;
> +		bchr++;
> +	}

Can't you use strcmp here instead?

> +
> +	/* Avoid a endless loop because >=0 insert str in list
> +	 * Insert string if strings are equal or is equal,
> +	 * but lengths are not the same */
> +	return 0;
> +}
> +
>  int string_list_add(struct string_list *sl, char *str)
>  {
>  	struct string_list *new;
> @@ -16,6 +39,31 @@ int string_list_add(struct string_list *sl, char *str)
>  	return 0;
>  }
>  
> +int string_list_add_sorted(struct string_list *sl, char *str)
> +{
> +	struct string_list *new;
> +
> +	new = xmalloc(sizeof(struct string_list) + strlen(str) + 1);
> +
> +	strcpy(new->str, str);
> +
> +	list_add_sort(&new->list, &sl->list, string_list_compare);
> +
> +	return 0;
> +}
> +
> +int string_list_contains(struct string_list *sl, char *str)
> +{
> +	struct string_list *entry;
> +
> +	list_for_each_entry(entry, &sl->list, list) {
> +		if (!strcmp(str, entry->str))
> +			return 1;
> +	}
> +
> +	return 0;
> +}
> +
>  void string_list_print_by_column(struct string_list *sl)
>  {
>  	int len = 0, num, i;
> @@ -44,3 +92,4 @@ void string_list_print_by_column(struct string_list *sl)
>  	if (i % num)
>  		printf("\n");
>  }
> +

Please don't add trailing new lines.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] auto-completion: add auto-completion for path files

[Sascha Hauer]

On Fri, Dec 09, 2011 at 10:52:48AM +0100, Alexander Aring wrote:
> Add auto-completion for path files.
> 
> Signed-off-by: Alexander Aring <a.aring@phytec.de>
> ---
>  commands/ls.c     |    4 +-
>  common/complete.c |   68 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 68 insertions(+), 4 deletions(-)
> 
> diff --git a/commands/ls.c b/commands/ls.c
> index 278a8bc..070aa90 100644
> --- a/commands/ls.c
> +++ b/commands/ls.c
> @@ -68,7 +68,7 @@ int ls(const char *path, ulong flags)
>  		if (stat(tmp, &s))
>  			goto out;
>  		if (flags & LS_COLUMN)
> -			string_list_add(&sl, d->d_name);
> +			string_list_add_sorted(&sl, d->d_name);
>  		else
>  			ls_one(d->d_name, &s);
>  	}
> @@ -156,7 +156,7 @@ static int do_ls(struct command *cmdtp, int argc, char *argv[])
>  
>  		if (!(s.st_mode & S_IFDIR)) {
>  			if (flags & LS_COLUMN)
> -				string_list_add(&sl, argv[o]);
> +				string_list_add_sorted(&sl, argv[o]);
>  			else
>  				ls_one(argv[o], &s);
>  		}

Ok, but this seems unrelated to the subject of this patch. Please
separate.

> diff --git a/common/complete.c b/common/complete.c
> index 46ba871..0b79f92 100644
> --- a/common/complete.c
> +++ b/common/complete.c
> @@ -27,6 +27,7 @@
>  #include <libgen.h>
>  #include <command.h>
>  #include <stringlist.h>
> +#include <environment.h>
>  
>  static int file_complete(struct string_list *sl, char *instr)
>  {
> @@ -55,7 +56,7 @@ static int file_complete(struct string_list *sl, char *instr)
>  				strcat(tmp, "/");
>  			else
>  				strcat(tmp, " ");
> -			string_list_add(sl, tmp);
> +			string_list_add_sorted(sl, tmp);
>  		}
>  	}
>  
> @@ -67,6 +68,67 @@ out:
>  	return 0;
>  }
>  
> +static int path_command_complete(struct string_list *sl, char *instr)
> +{
> +	struct stat s;
> +	DIR *dir;
> +	struct dirent *d;
> +	char tmp[PATH_MAX];
> +	char *path, *p, *n;
> +
> +	p = path = strdup(getenv("PATH"));
> +
> +	if (!path)
> +		return -1;
> +
> +	while (p) {
> +		n = strchr(p, ':');
> +		if (n)
> +			*n++ = '\0';
> +		if (*p != '\0') {

You can save one indention level by doing

		if (*p == '\0' ) {
			p = n;
			continue;
		}


> +			dir = opendir(p);
> +
> +			/* We need to check all PATH dirs, so if one failed,
> +			 * try next */
> +			if (!dir) {
> +				p = n;
> +				continue;
> +			}
> +
> +			while ((d = readdir(dir))) {
> +				if (!strcmp(d->d_name, ".") ||
> +					!strcmp(d->d_name, ".."))
> +					continue;
> +
> +				if (!strncmp(instr, d->d_name, strlen(instr))) {
> +					strcpy(tmp, d->d_name);
> +					if (!stat(tmp, &s) &&
> +						S_ISDIR(s.st_mode))
> +						continue;
> +					else
> +						strcat(tmp, " ");
> +
> +					/* This function is called
> +					 * after command_complete,
> +					 * so we check if a double
> +					 * entry exist */
> +					if (string_list_contains
> +							(sl, tmp) == 0) {
> +						string_list_add_sorted(sl, tmp);
> +					}
> +				}
> +			}
> +
> +			closedir(dir);
> +		}
> +		p = n;
> +	}
> +
> +	free(path);
> +
> +	return 0;
> +}
> +
-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/5] twlcore: rename twl4030 to twlcore driver

[Sascha Hauer]

On Fri, Dec 09, 2011 at 10:52:49AM +0100, Alexander Aring wrote:
> Abstract a general twl device driver twlcore to call i2c send/write
> functions.
> Renamed mfd/twl4030.c to mfd/twlcore.c.
> Rewrote twl4030-otg to use twlcore driver instead of old twl4030
> driver.
> 
> Rename patch to use twlcore instead twl4030 in beagleboard.
> 
> Fixed some code-styling issues pointed out by checkpatch.
> 
> Signed-off-by: Alexander Aring <a.aring@phytec.de>
> ---
>  arch/arm/boards/omap/board-beagle.c |    2 +-
>  drivers/mfd/Kconfig                 |    4 +-
>  drivers/mfd/Makefile                |    2 +-
>  drivers/mfd/twl-core.c              |  190 ++++++++++++++
>  drivers/mfd/twl4030.c               |  186 --------------
>  drivers/usb/otg/twl4030.c           |   52 ++--
>  include/mfd/twl-core.h              |  461 +++++++++++++++++++++++++++++++++++
>  include/mfd/twl4030.h               |  461 -----------------------------------
>  8 files changed, 681 insertions(+), 677 deletions(-)
>  create mode 100644 drivers/mfd/twl-core.c
>  delete mode 100644 drivers/mfd/twl4030.c
>  create mode 100644 include/mfd/twl-core.h
>  delete mode 100644 include/mfd/twl4030.h
> 
> diff --git a/arch/arm/boards/omap/board-beagle.c b/arch/arm/boards/omap/board-beagle.c
> index bfb08f7..4dd782f 100644
> --- a/arch/arm/boards/omap/board-beagle.c
> +++ b/arch/arm/boards/omap/board-beagle.c
> @@ -275,7 +275,7 @@ static struct ehci_platform_data ehci_pdata = {
>  
>  static struct i2c_board_info i2c_devices[] = {
>  	{
> -		I2C_BOARD_INFO("twl4030", 0x48),
> +		I2C_BOARD_INFO("twlcore", 0x48),

The name should stay "twl4030" because that's what it is. You could
make the driver match for both twl4030 and twl6030 instead.

>  	},
>  };
>  
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 96440d8..1677e2f 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -16,9 +16,9 @@ config I2C_LP3972
>  	depends on I2C
>  	bool "LP3972 driver"
>  
> -config I2C_TWL4030
> +config I2C_TWLCORE
>  	depends on I2C
> -	bool "TWL4030 driver"
> +	bool "TWLCORE driver"

I think we should rather prompt the user for twl4030/twl6030 instead.

>  	select GPIO
>  
>  config DRIVER_SPI_MC13783
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index d411f23..c54a6a1 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -2,5 +2,5 @@ obj-$(CONFIG_I2C_MC13892) += mc13892.o
>  obj-$(CONFIG_I2C_MC34704) += mc34704.o
>  obj-$(CONFIG_I2C_MC9SDZ60) += mc9sdz60.o
>  obj-$(CONFIG_I2C_LP3972) += lp3972.o
> -obj-$(CONFIG_I2C_TWL4030) += twl4030.o
> +obj-$(CONFIG_I2C_TWLCORE) += twl-core.o
>  obj-$(CONFIG_DRIVER_SPI_MC13783) += mc13783.o
> diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c

Please generate patches with file renames with git format-patch -M

From looking at the register layout the twl4030 and the twl6030 do
not have much in common. What can be shared between both seems to
be the core driver. I suggest that you do something like this:

struct twl_core {
	struct cdev		cdev;
	struct i2c_client	*client;
};

struct twl4030 {
	struct twl_core core;
};

struct twl6030 {
	struct twl_core core;
};

Then you can rename the twl4030_reg_* functions to twlcore_reg_* and
create wrapper static inline functions with the twl4030_reg_* names
which call twlcore_reg_*

The rationale is that the twl4030 otg driver should still work with
struct twl4030 and the twl4030 register accessors as this won't work
with the twl6030.


Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox