Re: [PATCH v6] USB: serial: ftdi_sio: Add MTP NVM support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Loic.
Thanks for fixing comments. Looks good to me now.

On 6/26/18, Loic Poulain <loic.poulain@xxxxxxxxxx> wrote:
> Most of FTDI's devices have an EEPROM which records FTDI devices
> configuration setting (e.g. the VID, PID, I/O config...) and user
> data. For example, FT232R and FTX chips have 128-byte and 2048-byte
> internal EEPROM respectively.
>
> This patch adds support for FTDI EEPROM read/write via USB control
> transfers and register a new nvm device to the nvmem core.
>
> This permits to expose the EEPROM as a sysfs file, allowing userspace
> to read/modify FTDI configuration and its user data without having to
> rely on a specific userspace USB driver.
>
> Moreover, any upcoming new tentative to add CBUS GPIO support could
> integrate CBUS EEPROM configuration reading in order to determine
> which of the CBUS pins are available as GPIO.
>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>
> Signed-off-by: Loic Poulain <loic.poulain@xxxxxxxxxx>

Reviewed-by: Ajay Gupta <ajaykuee@xxxxxxxxx>
> ---
>  v2: Use ifdef instead of IS_ENABLED
>      error message in case of nvmem registering failure
>      Fix space/tab in Kconfig
>  v3: Make nvmem a child of the usb dev instead of the serial port
>      Add macros defining eeprom sizes
>      Check read/write size is a nultiple of the eeprom word-size
>      Remove useless change in Kconfig
>  v4: Reword commit message
>      Remove default-yes from Kconfig
>      Change includes ordering
>      Use default linux size defines
>      Use get_unaligned_le16 helper
>      Prepend EEPROM functions with ftdi_
>      Error message in ftdi_eeprom_register()
>  v5: Fix missing linux/sizes header
>  v6: Ordering new headers insertion
>      Remove unecessary additional buf pointer from read/write_eeprom
>
>  drivers/usb/serial/Kconfig    |  10 ++++
>  drivers/usb/serial/ftdi_sio.c | 118
> ++++++++++++++++++++++++++++++++++++++++++
>  drivers/usb/serial/ftdi_sio.h |  28 ++++++++++
>  3 files changed, 156 insertions(+)
>
> diff --git a/drivers/usb/serial/Kconfig b/drivers/usb/serial/Kconfig
> index 533f127..5747562 100644
> --- a/drivers/usb/serial/Kconfig
> +++ b/drivers/usb/serial/Kconfig
> @@ -181,6 +181,16 @@ config USB_SERIAL_FTDI_SIO
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called ftdi_sio.
>
> +config USB_SERIAL_FTDI_SIO_NVMEM
> +	bool "FTDI MTP non-volatile memory support"
> +	depends on USB_SERIAL_FTDI_SIO
> +	select NVMEM
> +	help
> +	  Say yes here to add support for the MTP non-volatile memory
> +	  present on FTDI. Most of FTDI's devices have an EEPROM which
> +	  records FTDI device's configuration setting as well as user
> +	  data.
> +
>  config USB_SERIAL_VISOR
>  	tristate "USB Handspring Visor / Palm m50x / Sony Clie Driver"
>  	help
> diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
> index 7ea221d..34daa8c 100644
> --- a/drivers/usb/serial/ftdi_sio.c
> +++ b/drivers/usb/serial/ftdi_sio.c
> @@ -34,12 +34,17 @@
>  #include <linux/tty_driver.h>
>  #include <linux/tty_flip.h>
>  #include <linux/module.h>
> +#include <linux/nvmem-provider.h>
> +#include <linux/sizes.h>
>  #include <linux/spinlock.h>
>  #include <linux/mutex.h>
>  #include <linux/uaccess.h>
>  #include <linux/usb.h>
>  #include <linux/serial.h>
>  #include <linux/usb/serial.h>
> +
> +#include <asm/unaligned.h>
> +
>  #include "ftdi_sio.h"
>  #include "ftdi_sio_ids.h"
>
> @@ -73,6 +78,8 @@ struct ftdi_private {
>  	unsigned int latency;		/* latency setting in use */
>  	unsigned short max_packet_size;
>  	struct mutex cfg_lock; /* Avoid mess by parallel calls of config ioctl()
> and change_speed() */
> +
> +	struct nvmem_device *eeprom;
>  };
>
>  /* struct ftdi_sio_quirk is used by devices requiring special attention.
> */
> @@ -1529,6 +1536,110 @@ static int get_lsr_info(struct usb_serial_port
> *port,
>  	return 0;
>  }
>
> +#ifdef CONFIG_USB_SERIAL_FTDI_SIO_NVMEM
> +
> +static int ftdi_write_eeprom(void *priv, unsigned int off, void *val,
> +			     size_t bytes)
> +{
> +	struct usb_serial_port *port = priv;
> +	struct usb_device *udev = port->serial->dev;
> +
> +	if (bytes % 2) /* 16-bit eeprom */
> +		return -EINVAL;
> +
> +	while (bytes) {
> +		int rv;
> +
> +		rv = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
> +				     FTDI_SIO_WRITE_EEPROM_REQUEST,
> +				     FTDI_SIO_WRITE_EEPROM_REQUEST_TYPE,
> +				     get_unaligned_le16(val), off / 2, NULL,
> +				     0, WDR_TIMEOUT);
> +		if (rv < 0)
> +			return rv;
> +
> +		off += 2;
> +		val += 2;
> +		bytes -= 2;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ftdi_read_eeprom(void *priv, unsigned int off, void *val,
> +			    size_t bytes)
> +{
> +	struct usb_serial_port *port = priv;
> +	struct usb_device *udev = port->serial->dev;
> +
> +	if (bytes % 2) /* 16-bit eeprom */
> +		return -EINVAL;
> +
> +	while (bytes) {
> +		int rv;
> +
> +		rv = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
> +				     FTDI_SIO_READ_EEPROM_REQUEST,
> +				     FTDI_SIO_READ_EEPROM_REQUEST_TYPE,
> +				     0, off / 2, val, 2, WDR_TIMEOUT);
> +		if (rv < 0)
> +			return rv;
> +
> +		off += 2;
> +		val += 2;
> +		bytes -= 2;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ftdi_register_eeprom(struct usb_serial_port *port)
> +{
> +	struct ftdi_private *priv = usb_get_serial_port_data(port);
> +	struct usb_device *udev = port->serial->dev;
> +	struct nvmem_config nvmconf = {};
> +
> +	switch (priv->chip_type) {
> +	case FTX:
> +		nvmconf.size = SZ_2K;
> +		break;
> +	case FT232RL:
> +		nvmconf.size = SZ_128;
> +		break;
> +	default:
> +		return 0;
> +	}
> +
> +	nvmconf.word_size = 2;
> +	nvmconf.stride = 2;
> +	nvmconf.read_only = false;
> +	nvmconf.priv = port;
> +	nvmconf.dev = &udev->dev;
> +	nvmconf.reg_read = ftdi_read_eeprom;
> +	nvmconf.reg_write = ftdi_write_eeprom;
> +	nvmconf.owner = THIS_MODULE;
> +
> +	priv->eeprom = nvmem_register(&nvmconf);
> +	if (IS_ERR(priv->eeprom)) {
> +		dev_err(&udev->dev, "Unable to register FTDI EEPROM\n");
> +		priv->eeprom = NULL;
> +		return -ENOMEM;
> +	}
> +
> +	dev_info(&udev->dev, "Registered %d-byte FTDI EEPROM\n", nvmconf.size);
> +
> +	return 0;
> +}
> +
> +static void ftdi_unregister_eeprom(struct usb_serial_port *port)
> +{
> +	struct ftdi_private *priv = usb_get_serial_port_data(port);
> +
> +	if (priv->eeprom)
> +		nvmem_unregister(priv->eeprom);
> +}
> +
> +#endif /* CONFIG_USB_SERIAL_FTDI_SIO_NVMEM */
>
>  /* Determine type of FTDI chip based on USB config and descriptor. */
>  static void ftdi_determine_type(struct usb_serial_port *port)
> @@ -1814,6 +1925,10 @@ static int ftdi_sio_port_probe(struct usb_serial_port
> *port)
>  		priv->latency = 16;
>  	write_latency_timer(port);
>  	create_sysfs_attrs(port);
> +#ifdef CONFIG_USB_SERIAL_FTDI_SIO_NVMEM
> +	ftdi_register_eeprom(port);
> +#endif
> +
>  	return 0;
>  }
>
> @@ -1931,6 +2046,9 @@ static int ftdi_sio_port_remove(struct usb_serial_port
> *port)
>  {
>  	struct ftdi_private *priv = usb_get_serial_port_data(port);
>
> +#ifdef CONFIG_USB_SERIAL_FTDI_SIO_NVMEM
> +	ftdi_unregister_eeprom(port);
> +#endif
>  	remove_sysfs_attrs(port);
>
>  	kfree(priv);
> diff --git a/drivers/usb/serial/ftdi_sio.h b/drivers/usb/serial/ftdi_sio.h
> index dcd0b6e..9eab961 100644
> --- a/drivers/usb/serial/ftdi_sio.h
> +++ b/drivers/usb/serial/ftdi_sio.h
> @@ -36,6 +36,8 @@
>  #define FTDI_SIO_SET_ERROR_CHAR		7 /* Set the error character */
>  #define FTDI_SIO_SET_LATENCY_TIMER	9 /* Set the latency timer */
>  #define FTDI_SIO_GET_LATENCY_TIMER	10 /* Get the latency timer */
> +#define FTDI_SIO_READ_EEPROM		0x90 /* Read eeprom */
> +#define FTDI_SIO_WRITE_EEPROM		0x91 /* Write eeprom */
>
>  /* Interface indices for FT2232, FT2232H and FT4232H devices */
>  #define INTERFACE_A		1
> @@ -400,6 +402,32 @@ enum ftdi_sio_baudrate {
>   *
>   */
>
> + /* FTDI_SIO_READ_EEPROM */
> +#define FTDI_SIO_READ_EEPROM_REQUEST_TYPE 0xc0
> +#define FTDI_SIO_READ_EEPROM_REQUEST FTDI_SIO_READ_EEPROM
> +/*
> + *  BmRequestType:   1100 0000b
> + *  bRequest:        FTDI_SIO_READ_EEPROM
> + *  wValue:          0
> + *  wIndex:          Word Index
> + *  wLength:         2
> + *  Data:            return data (a word)
> + *
> + */
> +
> +/* FTDI_SIO_WRITE_EEPROM */
> +#define FTDI_SIO_WRITE_EEPROM_REQUEST_TYPE 0x40
> +#define FTDI_SIO_WRITE_EEPROM_REQUEST FTDI_SIO_WRITE_EEPROM
> +/*
> + *  BmRequestType:   0100 0000b
> + *  bRequest:        FTDI_SIO_WRITE_EEPROM
> + *  wValue:          Data (word)
> + *  wIndex:          Word Index
> + *  wLength:         0
> + *  Data:            None
> + *
> + */
> +
>  /* FTDI_SIO_GET_MODEM_STATUS */
>  /* Retrieve the current value of the modem status register */
>
> --
> 2.7.4
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux