LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Greg KH <greg@kroah.com>
To: Dirk Eibach <eibach@gdsys.de>
Cc: akpm@linux-foundation.org, linux-kernel@vger.kernel.org,
	Andy Whitcroft <apw@shadowen.org>,
	linux-usb@vger.kernel.org
Subject: Re: [PATCH] usb: add sysfs configuration interface for CP2101
Date: Fri, 29 Feb 2008 16:33:42 -0800	[thread overview]
Message-ID: <20080301003342.GB17761@kroah.com> (raw)
In-Reply-To: <47C7FB0F.1060902@gdsys.de>

On Fri, Feb 29, 2008 at 01:31:11PM +0100, Dirk Eibach wrote:
> So here is a new try.

Please provide a full changelog entry, and signed-off-by: if you wish to
have it applied to the tree.

As you are adding new sysfs entries, I need some documentation to be
added in Documentation/ABI/ describing these new files and how they are
to be used.  See the instructions there for the format needed.


> 
> ---
> diff -purN linux-2.6.24/drivers/usb/serial/cp2101.c linux-2.6.24-diff/drivers/usb/serial/cp2101.c
> --- linux-2.6.24/drivers/usb/serial/cp2101.c	2008-01-24 23:58:37.000000000 +0100
> +++ linux-2.6.24-diff/drivers/usb/serial/cp2101.c	2008-02-29 13:15:58.168233146 +0100
> @@ -18,6 +18,8 @@
>   */
>  
>  #include <linux/kernel.h>
> +#include <linux/fs.h>
> +#include <linux/platform_device.h>
>  #include <linux/errno.h>
>  #include <linux/slab.h>
>  #include <linux/tty.h>
> @@ -52,6 +54,8 @@ static void cp2101_shutdown(struct usb_s
>  
>  static int debug;
>  
> +static int enable_config = false;
> +
>  static struct usb_device_id id_table [] = {
>  	{ USB_DEVICE(0x08e6, 0x5501) }, /* Gemalto Prox-PU/CU contactless smartcard reader */
>  	{ USB_DEVICE(0x0FCF, 0x1003) }, /* Dynastream ANT development board */
> @@ -125,6 +129,7 @@ static struct usb_serial_driver cp2101_d
>  #define CP2101_CONTROL		0x07	/* Flow control line states */
>  #define CP2101_MODEMCTL		0x13	/* Modem controls */
>  #define CP2101_CONFIG_6		0x19	/* 6 bytes of config data ??? */
> +#define CP2101_EEPROM		0xFF	/* write configuration eeprom */
>  
>  /* CP2101_UART */
>  #define UART_ENABLE		0x0001
> @@ -167,6 +172,20 @@ static struct usb_serial_driver cp2101_d
>  #define CONTROL_WRITE_DTR	0x0100
>  #define CONTROL_WRITE_RTS	0x0200
>  
> +/* CP2101 CONFIGURATION EEPROM */
> +#define EEPROM_RELOAD		0x0008
> +#define EEPROM_VENDOR_ID	0x3701
> +#define EEPROM_PRODUCT_ID	0x3702
> +#define EEPROM_PRODUCTSTRING	0x3703
> +#define EEPROM_SERIALNUMBER	0x3704
> +#define EEPROM_POWER_ATTRIB	0x3705
> +#define EEPROM_MAX_POWER	0x3706
> +#define EEPROM_RELEASE_VERSION	0x3707
> +#define EEPROM_LOCK		0x370a
> +
> +#define SERIALNUMBER_MAX_CHARS	63
> +#define PRODUCTSTRING_MAX_CHARS	126
> +
>  /*
>   * cp2101_get_config
>   * Reads from the CP2101 configuration registers
> @@ -704,11 +723,286 @@ static void cp2101_break_ctl (struct usb
>  	cp2101_set_config(port, CP2101_BREAK, &state, 2);
>  }
>  
> +/*
> + * When loaded with module parameter "enable_config=1" the driver offers the
> + * following sysfs attributes to customize USB configuration data:
> + * reload: write "1" to reboot CP210x and activate the new configuration
> + * vendor_id: write 16 bit value to set
> + * product_id: write 16 bit value to set


This seems odd, you know we already allow any usb vendor and product id
to be automatically added to a driver after it has been loaded through
sysfs, right?  Wouldn't that take care of the majority of the issues you
have here?

Oh, wait, this is setting the device itself, nevermind.

You should use configfs for this instead, that's what it is for moreso
than sysfs, right?

thanks,

greg k-h

  parent reply	other threads:[~2008-03-01  1:19 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-29  9:02 Dirk Eibach
2008-02-29  9:40 ` Dirk Eibach
2008-02-29 10:02 ` Andrew Morton
2008-02-29 12:31   ` Dirk Eibach
2008-02-29 20:35     ` Andrew Morton
2008-03-03  9:27       ` Dirk Eibach
2008-03-03 15:07         ` Alan Stern
2008-03-03 16:28           ` Greg KH
2008-03-04  7:03             ` Dirk Eibach
2008-03-04 17:52               ` Alan Stern
2008-03-04 19:51               ` Greg KH
2008-04-09 12:16       ` Andy Whitcroft
2008-03-01  0:33     ` Greg KH [this message]
2008-02-29 14:44   ` Andy Whitcroft
2008-02-29 20:46     ` Andrew Morton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080301003342.GB17761@kroah.com \
    --to=greg@kroah.com \
    --cc=akpm@linux-foundation.org \
    --cc=apw@shadowen.org \
    --cc=eibach@gdsys.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --subject='Re: [PATCH] usb: add sysfs configuration interface for CP2101' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).