Hi Antony,
On Tue, Oct 10, 2017 at 03:26:28PM +0300, Antony Pavlov wrote:
> Signed-off-by: Antony Pavlov <antonynpavlov@xxxxxxxxx>
> ---
> arch/sandbox/Kconfig | 1 +
> arch/sandbox/Makefile | 6 +-
> arch/sandbox/mach-sandbox/include/mach/linux.h | 11 ++
> arch/sandbox/os/Makefile | 3 +
> arch/sandbox/os/ftdi.c | 173 +++++++++++++++++++++++++
> drivers/gpio/Kconfig | 4 +
> drivers/gpio/Makefile | 1 +
> drivers/gpio/gpio-libftdi1.c | 125 ++++++++++++++++++
> 8 files changed, 323 insertions(+), 1 deletion(-)
>
...
> +static struct ft2232_bitbang ftbb;
> +
...
> +
> +int barebox_libftdi1_init(void)
> +{
> + struct ftdi_context *ftdi;
> + int ret;
> + /* default FT2232 values */
> + uint16_t vendor_id = FTDI_VID;
> + uint16_t device_id = FTDI_8U2232C_PID;
> +
> + ftdi = ftdi_new();
> + if (!ftdi) {
> + fprintf(stderr, "ftdi_new failed\n");
> + goto error;
> + }
> +
> + ret = ftdi_usb_open(ftdi, vendor_id, device_id);
> + if (ret < 0 && ret != -5) {
> + fprintf(stderr, "unable to open ftdi device: %d (%s)\n",
> + ret, ftdi_get_error_string(ftdi));
> + goto error;
> + }
What does a return value of -5 mean? Isn't that an error?
> +
> + ftdi_set_interface(ftdi, INTERFACE_A);
> + ftdi_set_bitmode(ftdi, 0x00, BITMODE_MPSSE);
> +
> + ftbb.ftdi = ftdi;
> +
> + /* reset pins to default neutral state */
> + ftbb.dir = 0;
> + ftbb.odata = 0;
> + ftdi_set_high_byte_data_dir(&ftbb);
> +
> + return 0;
> +
> +error:
> + return -1;
> +}
> +
> +struct ft2232_bitbang *barebox_libftdi1_open(void)
> +{
> + if (barebox_libftdi1_init() < 0) {
> + printf("Could not initialize ftdi\n");
> + return NULL;
> + }
> +
> + return &ftbb;
> +}
Somethings fishy here. Do you want to create a new struct ft2232_bitbang
instance for each caller or do you want to return the same instance for
every call to barebox_libftdi1_open()? If you want to do the former you
shouldn't create a static struct ft2232_bitbang, but instead allocate it
dynamically. If you want to do the latter then you should do a "if
(initialized) return existing_instance".
> + gpio->chip.dev = dev;
> + gpio->chip.ops = &libftdi1_gpio_ops;
> + gpio->chip.base = 0;
> + gpio->chip.ngpio = 8;
> +
> + ret = gpiochip_add(&gpio->chip);
> +
> + dev_dbg(dev, "%d: probed gpio%d with base %d\n",
> + ret, dev->id, gpio->chip.base);
> +
> + return 0;
Would be good to actually check 'ret' for errors.
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@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
