|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 2/3] drivers: serial: add Qualcomm GENI-based serial driver
Hi, On 02/04/2024 22:19, Volodymyr Babchuk wrote: If you don't provide the early printk for arm32, then you need to add DEPENDS ON otherwise randconfig will fail on arm32.Hi Michal, Michal Orzel <michal.orzel@xxxxxxx> writes:Hello, On 29/03/2024 01:08, Volodymyr Babchuk wrote:Generic Interface (GENI) is a newer interface for low speed interfaces like UART, I2C or SPI. This patch adds the simple driver for the UART instance of GENI. Code is based on similar drivers in U-Boot and Linux kernel.Do you have a link to a manual?I wish I had. Qualcomm provides HW manuals only under very strict NDAs. At the time of writing I don't have access to the manual at all. Those patches are based solely on similar drivers in U-Boot and Linux kernel.This driver implements only simple synchronous mode, because although GENI supports FIFO mode, it needs to know number of characters **before** starting TX transaction. This is a stark contrast when compared to other UART peripherals, which allow adding characters to a FIFO while TX operation is running. The patch adds both normal UART driver and earlyprintk version. Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx> --- xen/arch/arm/Kconfig.debug | 19 +- xen/arch/arm/arm64/debug-qcom.inc | 76 +++++++Shouldn't all the files (+ other places) have geni in their names? Could qcom refer to other type of serial device?AFAIK, there is an older type of serial device. Both Linux and U-Boot use "msm" instead of "qcom" in drivers names for those devices. But I can add "geni" to the names, no big deal. [...] You see, this code is working inside-out. early printk code in Xen is written with assumption that early_uart_transmit don't need a scratch register. But this is not true for GENI... To send one character we must write two registers beforehand: TX_TRANS_LEN and CMD0. Only after that we can write something to FIFO. But early_uart_transmit has no scratch register to prepare values for TX_TRANS_LEN and CMD0. So we write what we do here 1. Reset the state machine with ABORT command 2. Write 1 to TX_TRANS_LEN 3. Write START_TX to CMD0 Now early_uart_transmit can write "wt" to the FIFO and after that it can use "wt" as a scratch register to repeat steps 2 and 3. AFAIU, this means we would potentially do one too many iteration. Does this have any implication to the runtime UART driver? But why do we need to repeat step 2/3? Is this because both registers will be reset after the character is written? Probably I need this as a comment to into the .inc file...+ mov w\c, #M_GENI_CMD_ABORT + str w\c, [\xb, #SE_GENI_M_CMD_CTRL_REG] +1: + ldr w\c, [\xb, #SE_GENI_M_IRQ_STATUS] /* Load IRQ status */ + tst w\c, #M_CMD_ABORT_EN /* Check TX_FIFI_WATERMARK_EN bit */The comment does not correspond to the code. Shouldn't this be the same as in early_uart_ready?We need to reset the state machine with ABORT command just in case. Would you mind to explain what you mean with "just in case"? A pointer to the corresponding Linux/U-boot code would be helpful. [...]
Is it because there is no 32-bit HW from qualcomm that supports virtualization? [...] +DT_DEVICE_START(qcom_uart, "QCOM UART", DEVICE_SERIAL) + .dt_match = qcom_uart_dt_match, + .init = qcom_uart_init, +DT_DEVICE_END + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * indent-tabs-mode: nil + * End: + */ -- 2.43.0What about vUART? You don't seem to set vuart information that is used by vuart.c and vpl011.cI am not sure that it is possible to emulate GENI UART with simple vuart API. vuart makes assumption that there is one simple status register and FIFO register operates on per-character basis, while even earlycon part of the driver in Linux tries to pack 4 characters to one FIFO write. So I agree that enabling vUART is probably going to be difficult. But you should be able to implement the vpl011 as it only require a base and an IRQ. That said, from the driver PoV, there is no differentiation today. We could add a extra parameter, but I don't think this needs to be handled in this series. Cheers, -- Julien Grall
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |