On Mon, 2024-04-08 at 19:48 +0300, Ilpo Järvinen wrote: > [Some people who received this message don't often get email from ilpo.jarvinen@xxxxxxxxxxxxxxx. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] > > ⚠Caution: External email. Exercise extreme caution with links or attachments.⚠ > > > On Mon, 8 Apr 2024, Matthew Howell wrote: > > > On Wed, 2024-02-21 at 16:16 -0500, Matthew Howell wrote: > > > Allows the use of the EN485 hardware pin by preserving the value of > > > FCTR[5] in pci_xr17v35x_setup(). > > > > > > Per the XR17V35X datasheet, the EN485 hardware pin works by setting > > > FCTR[5] when the pin is active. pci_xr17v35x_setup() prevented the use > > > of EN485 because it overwrote the FCTR register. > > > > > > Signed-off-by: Matthew Howell <matthew.howell@xxxxxxxxxxxx> > > > --- > > > V1 -> V2 > > > Fixed wordwrap in diff > > > > > > diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c > > > index 23366f868..97711606f 100644 > > > --- a/drivers/tty/serial/8250/8250_exar.c > > > +++ b/drivers/tty/serial/8250/8250_exar.c > > > @@ -596,6 +596,7 @@ pci_xr17v35x_setup(struct exar8250 *priv, struct pci_dev *pcidev, > > > unsigned int baud = 7812500; > > > u8 __iomem *p; > > > int ret; > > > + u8 en485mask; > > > > > > port->port.uartclk = baud * 16; > > > port->port.rs485_config = platform->rs485_config; > > > @@ -618,7 +619,8 @@ pci_xr17v35x_setup(struct exar8250 *priv, struct pci_dev *pcidev, > > > p = port->port.membase; > > > > > > writeb(0x00, p + UART_EXAR_8XMODE); > > > - writeb(UART_FCTR_EXAR_TRGD, p + UART_EXAR_FCTR); > > > + en485mask = readb(p + UART_EXAR_FCTR) & UART_FCTR_EXAR_485; > > > + writeb(UART_FCTR_EXAR_TRGD | en485mask, p + UART_EXAR_FCTR); > > > writeb(128, p + UART_EXAR_TXTRG); > > > writeb(128, p + UART_EXAR_RXTRG); > > Why you need to read rs485 state from the register? It should be available > in ->rs485.flags & SER_RS485_ENABLED. > Please correct me if I am wrong, but my understanding is that SER_RS485_ENABLED is set from userspace using the TIOCRS485 IOCTL. However, this is not the only way that the FCTR register can be changed. In particular, per XR17V35X datasheet, the EN485 pin is sampled on power-on and transfers the logic state to FCTR[5]. Our card takes advantage of this to allow users to configure RS485 in scenarios where they cannot, or do not want to, modify their software to set SER_RS485_ENABLED. However, this functionality of the UART does not currently work with this driver because the entire FCTR register is being overwritten, thereby erasing whatever value was written to FCTR[5] on UART power-up. The driver cannot know whether FCTR[5] was set on power-up without reading the FCTR, therefore it must be read. > pci_fastcom335_setup() seems to have the same problem? That small part > seems to be common code anyway which should be moved into helper, only the > trigger threshold seems to differ which can be given in a parameter. > Technically, yes mit has the same problem, but I am not sure it is an actual issue and am hesitant make the suggested changes for the following reasons: 1) The fastcom card may depend on the behaviour working as-is. 2) I only have Sealevel & reference Exar hardware to test, not Fastcom, so I have no way to test if the changes negatively impact fastcom 3) Finally, while I am not familar with the fastcom 335, it doesn't have any dip-switches or jumpers, which leads me to believe it doesn't have a way for users to utilize the EN485 pin. If this is the case, then there is no end-user benefit to 'fixing' this in pci_fastcom335_setup(). If someone with a fastcom 335 card is willing to test then I can take a stab at the suggested change, but at the moment I see at least some 'risk' in making this change, but no 'reward' for doing so. -- Matthew H. > -- > i. > > > Just wanted to follow-up on this to see if anyone has had a time to > > review the above submission? Please let me know if there are any issues > > / anything I need to do. >
