On 04/27/2018 07:51 AM, Ang, Chee Hong wrote: [...] >>>>> + /* Check for any error */ >>>>> + ret = >>>>> reconfig_status_resp[RECONFIG_STATUS_STATE]; >>>>> + if (ret && ret != MBOX_CFGSTAT_STATE_CONFIG) >>>>> + return ret; >>>>> + >>>>> + /* Make sure nStatus is not 0 */ >>>>> + ret = >>>>> reconfig_status_resp[RECONFIG_STATUS_PIN_STATUS]; >>>>> + if (!(ret & RCF_PIN_STATUS_NSTATUS)) >>>>> + return >>>>> MBOX_CFGSTAT_STATE_ERROR_HARDWARE; >>>> wait_for_bit_le32() or somesuch ? >>> No, we don't read the bit status directly from register. We only >>> need >>> to test the bit of the pin status stored in memory. >> Who's populating the memory ? > ret = mbox_send_cmd(MBOX_ID_UBOOT, MBOX_RECONFIG_STATUS, > MBOX_CMD_DIRECT, 0, NULL, 0, &reconfig_status_resp_len, > reconfig_status_resp); > > We send mailbox command to the device and the device will respond by > filling the 'reconfig_status_resp' with the device status.
Ah, I see. Would it make sense to implement a generic "poll for bit" for this mailbox communication ? Also, we have drivers/mailbox/ , would it make sense to move the mailbox stuff there ? [...] >>>>> + if (*resp_count < buf_size) { >>>>> + u32 rcv_len_max = buf_size - *resp_count; >>>>> + >>>>> + if (rcv_len_max > MBOX_RESP_BUFFER_SIZE) >>>>> + rcv_len_max = MBOX_RESP_BUFFER_SIZE; >>>>> + resp_len = mbox_rcv_resp(buf, rcv_len_max); >>>>> + >>>>> + for (i = 0; i < resp_len; i++) { >>>>> + resp_buf[(*w_index)++] = buf[i]; >>>> Is this like a memcpy() ? >>> No. This is a circular buffer, index to the memory location may >>> wrap >>> around. >> Two memcpys then ? > Will refactor this part in v2. Does it even have to be a circular buffer in the first place ? [...] >>>>> + /* Make sure we don't send MBOX_RECONFIG_STATUS too >>>>> fast >>>>> */ >>>>> + udelay(RECONFIG_STATUS_INTERVAL_DELAY_US); >>>> Hum, this is iffy, is that a hardware bug ? >>> Yes. The firmware running in that device is not able to response >>> quickly. >> And you cannot poll it in some way ? > I know this is ugly and looks buggy. But there are no mechanism to poll > whether the device is ready to accept any mailbox command or not. So > for now, slowing down the mailbox exchange is the only way to go. If it works reliably, so be it. -- Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot