helloSystem / ISO

helloSystem Live and installation ISO
https://github.com/helloSystem/
BSD 3-Clause "New" or "Revised" License
805 stars 59 forks source link

Cannot upload to esp8266 using CH341/CH340 serial adapter #479

Open probonopd opened 1 year ago

probonopd commented 1 year ago

Cannot seem to upload to esp8266 based devices that use a WinChipHead CH341/CH340 serial adapter using esptool.py. It works with other serial adapters. Bug in uchcom?

https://github.com/freebsd/freebsd-src/commits/main/sys/dev/usb/serial/uchcom.c

Reference:

hselasky commented 1 year ago

Hi,

Maybe a change like this needs to be ported to the uchcom.c driver:

commit 40e43b056df9aa2392f673fcacc72725c2201658 Author: Hans Petter Selasky hselasky@FreeBSD.org Date: Tue Aug 30 16:01:43 2022 +0200

umodem(4): Clear stall at every open.
probonopd commented 1 year ago

Hi @hselasky, thanks for chiming in. Appreciate your help. To clarify, do you think a change like

https://reviews.freebsd.org/rG40e43b056df9aa2392f673fcacc72725c2201658

would need to be made in the uchcom driver?

So to be concrete,

https://github.com/freebsd/freebsd-src/blob/f25a0a0f21183a52eeb4a860a88114aebec5f249/sys/dev/usb/serial/uchcom.c#L673-L681

The exsiting

uchcom_cfg_open(struct ucom_softc *ucom)
{
    struct uchcom_softc *sc = ucom->sc_parent;

    DPRINTF("\n");

    uchcom_update_version(sc);
    uchcom_update_status(sc);
}

would need to be changed like this? Then we probably also need something like

uchcom_cfg_open(struct ucom_softc *ucom)
{
    struct uchcom_softc *sc = ucom->sc_parent;

    /* clear stall */
    if ((sc->sc_super_ucom.sc_flag & UCOM_FLAG_DEVICE_MODE) == 0) {
        usbd_xfer_set_stall(sc->sc_xfer[UCHCOM_BULK_WR]);
        usbd_xfer_set_stall(sc->sc_xfer[UCHCOM_BULK_RD]);
    }

    DPRINTF("\n");

    uchcom_update_version(sc);
    uchcom_update_status(sc);
}

and

/*
 * As speeds for uchcom devices increase, these numbers will need to
 * be increased. This needs to be tested.
 *
 * TODO: The TTY buffers should be increased!
 */
#define UCHCOM_BUF_SIZE 1024

I am not sure about the buffer size above. Maybe we should increase it to be on the safe side?

static const struct usb_config uchcom_config_data[UCHCOM_N_TRANSFER] = {
    [UCHCOM_BULK_WR] = {
        .type = UE_BULK,
        .endpoint = UE_ADDR_ANY,
        .direction = UE_DIR_TX,
        .if_index = 0,
        .bufsize = UCHCOM_BUF_SIZE,
        .flags = {.pipe_bof = 1,.force_short_xfer = 1,},
        .callback = &uchcom_write_callback,
        .usb_mode = USB_MODE_DUAL,
    },

    [UCHCOM_BULK_RD] = {
        .type = UE_BULK,
        .endpoint = UE_ADDR_ANY,
        .direction = UE_DIR_RX,
        .if_index = 0,
        .bufsize = UCHCOM_BUF_SIZE,
        .flags = {.pipe_bof = 1,.short_xfer_ok = 1,},
        .callback = &uchcom_read_callback,
        .usb_mode = USB_MODE_DUAL,
    },

    [UCHCOM_BULK_DT_WR] = {
    ...

I really don't know what I am doing here, so I'd appreciate some help on this. Should I open a FreeBSD ticket on Bugzilla?

hselasky commented 1 year ago

The current buffer size is ok.

The clear stall change is OK.

Yes, make a PR, and add hselasky@freebsd.org , and I'll review your patch. You can also use: differential revision:

https://reviews.freebsd.org/

Upload the patch and I'll help you. Maybe you learn something :-)

--HPS

probonopd commented 1 year ago

Thanks @hselasky. Here: https://reviews.freebsd.org/D39770