USB: serial: ftdi_sio: Add MTP NVM support

[Srinivas Kandagatla <srinivas.kandagatla@linaro.org>]

Overall nvmem side looks good!
Minor nits below.

On 14/06/18 21:08, Loic Poulain 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. FT230R chip integrates a 128-byte eeprom, FT230X a 2048-byte
> eeprom...
> 
> 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.
> 
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> ---
>   drivers/usb/serial/Kconfig    |  11 +++++
>   drivers/usb/serial/ftdi_sio.c | 108 ++++++++++++++++++++++++++++++++++++++++++
>   drivers/usb/serial/ftdi_sio.h |  28 +++++++++++
>   3 files changed, 147 insertions(+)
> 
> diff --git a/drivers/usb/serial/Kconfig b/drivers/usb/serial/Kconfig
> index 533f127..2dd2f5d 100644
> --- a/drivers/usb/serial/Kconfig
> +++ b/drivers/usb/serial/Kconfig
> @@ -181,6 +181,17 @@ 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
Looks inconsistent tab and spaces here.

> +        select NVMEM
> +        default y
??

Does that mean all the FTDIs have EEPROM?

> +        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..03c9c75 100644
> --- a/drivers/usb/serial/ftdi_sio.c
> +++ b/drivers/usb/serial/ftdi_sio.c
> @@ -40,6 +40,7 @@
>   #include <linux/usb.h>
>   #include <linux/serial.h>
>   #include <linux/usb/serial.h>
> +#include <linux/nvmem-provider.h>
>   #include "ftdi_sio.h"
>   #include "ftdi_sio_ids.h"
>   
> @@ -73,6 +74,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 +1532,104 @@ static int get_lsr_info(struct usb_serial_port *port,
>   	return 0;
>   }
>   
> +#if IS_ENABLED(CONFIG_USB_SERIAL_FTDI_SIO_NVMEM)
I think only ifdef should be Okay here as this Kconfig is just bool
> +

...

> +static int register_eeprom(struct usb_serial_port *port)
> +{
> +	struct ftdi_private *priv = usb_get_serial_port_data(port);
> +	struct nvmem_config nvmconf = {};
> +
> +	switch (priv->chip_type) {
> +	case FTX:
> +		nvmconf.size = 2048;
> +		break;
> +	case FT232RL:
> +		nvmconf.size = 128;
> +		break;
> +	default:
> +		return 0;
> +	}
> +
> +	nvmconf.word_size = 2;
> +	nvmconf.stride = 2;
> +	nvmconf.read_only = false;
> +	nvmconf.priv = port;
> +	nvmconf.dev = &port->dev;
> +	nvmconf.reg_read = read_eeprom;
> +	nvmconf.reg_write = write_eeprom;
> +	nvmconf.owner = THIS_MODULE;
> +
> +	priv->eeprom = nvmem_register(&nvmconf);
> +	if (IS_ERR(priv->eeprom)) {
> +		priv->eeprom = NULL;
> +		return -ENOMEM;
> +	}
> +
> +	return 0;
> +}
> +
> +static void 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 +1915,10 @@ static int ftdi_sio_port_probe(struct usb_serial_port *port)
>   		priv->latency = 16;
>   	write_latency_timer(port);
>   	create_sysfs_attrs(port);
> +#if IS_ENABLED(CONFIG_USB_SERIAL_FTDI_SIO_NVMEM)
> +	register_eeprom(port);

Users will be clueless if the register_eeprom fails here.



> +#endif
> +
>   	return 0;
>   }
>   
> @@ -1931,6 +2036,9 @@ static int ftdi_sio_port_remove(struct usb_serial_port *port)
>   {
>   	struct ftdi_private *priv = usb_get_serial_port_data(port);
>   
> +#if IS_ENABLED(CONFIG_USB_SERIAL_FTDI_SIO_NVMEM)
> +	unregister_eeprom(port);
> +#endif
>   	remove_sysfs_attrs(port);
>   
>   	kfree(priv);

thanks,
srini
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html