| Message ID | 5dd9f8b0c1dc154c73fb883cb948768ae68d1ccb.camel@sealevel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [V1] serial: exar: Preserve FCTR[5] bit in pci_xr17v35x_setup() | expand |
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@sealevel.com> > --- > 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); > > Hi, 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. Thanks, Matthew Howell
On Mon, Apr 08, 2024 at 01:11:42PM +0000, 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@sealevel.com> > > --- > > 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); > > > > > > Hi, > > 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. There was review comments, did you not see them: https://lore.kernel.org/all/ZdaAI4uZ1Byx2cWs@surfacebook.localdomain/
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@sealevel.com> > > --- > > 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. pci_fastcom335_setup() seems to have the same problem? Path 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.
On Mon, 8 Apr 2024, Ilpo Järvinen wrote: > 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@sealevel.com> > > > --- > > > 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. > > pci_fastcom335_setup() seems to have the same problem? Path small part I meant "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. > >
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@linux.intel.com. 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@sealevel.com> > > > --- > > > 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. >
On Mon, 2024-04-08 at 16:56 +0200, gregkh@linuxfoundation.org wrote: > On Mon, Apr 08, 2024 at 01:11:42PM +0000, 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@sealevel.com> > > > --- > > > 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); > > > > > > > > > > Hi, > > > > 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. > > There was review comments, did you not see them: > https://lore.kernel.org/all/ZdaAI4uZ1Byx2cWs@surfacebook.localdomain/ No, I can't find that anywhere in my mailbox, junk folder or otherwise. I will address Andy's comments in his thread though now that I am aware of it. -- Matthew H.
On Thu, 2024-02-22 at 00:58 +0200, andy.shevchenko@gmail.com wrote: > Wed, Feb 21, 2024 at 09:16:46PM +0000, Matthew Howell kirjoitti: > > 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. > > First of all, please avoid In-Reply-to: for the new versions of the change, > i.o.w. new version — new email thread. > Apologies, I didn't mean to do so. I had created the response from a "reply-all" in my email client and thought manually removing the in- reply-to would be sufficient to create a new email instead, but it seems the email client tried to be "smart" and kept the in-reply-to anyway. > Second, the above commit message sounds like a fix. Does it deserve Fixes tag? > Yes, I believe so. I had considered adding that and don't recall why I decided to leave it off. > ... > > > unsigned int baud = 7812500; > > u8 __iomem *p; > > int ret; > > + u8 en485mask; > > Please, preserve reversed xmas tree order. > > unsigned int baud = 7812500; > u8 __iomem *p; > u8 en485mask; > int ret; > I will resubmit with these suggestions in mind. P.S: Sorry for the late response, for some reason this didn't make it to my mailbox and I was only just made aware of your response.
On Mon, 8 Apr 2024, Matthew Howell wrote: > 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@linux.intel.com. 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@sealevel.com> > > > > --- > > > > 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. Thought that and device_property_read_bool(dev, "linux,rs485-enabled-at-boot-time") > 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. ??? Are you saying RS485 is enabled without kernel knowing about it? I don't think that's the correct way to do things. > > 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. Okay, it seems fastcom part of the driver doesn't support rs485. There's stupid naming in this driver, "generic_rs485_config()" isn't really that generic it seems (and on top of that, the init flow is hard to follow). :-/
On Wed, 2024-04-10 at 16:49 +0300, Ilpo Järvinen wrote: > On Mon, 8 Apr 2024, Matthew Howell wrote: > > > 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@linux.intel.com. 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@sealevel.com> > > > > > --- > > > > > 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. > > Thought that and device_property_read_bool(dev, > "linux,rs485-enabled-at-boot-time") > I am not familiar with that property. Is that something that can be set in userspace or via a kernel parameter? Or is it 'hard-coded' into the device tree binding for a particular device? > > 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. > > ??? > > Are you saying RS485 is enabled without kernel knowing about it? I don't > think that's the correct way to do things. > That is correct. However, I wouldn't say it is an incorrect way of doing things. Some reasoning/justification below: * Kernel/Driver/Software independent RS485 Auto Enable is not new. I can't speak for other vendors, but Sealevel has been creating products with this functionality for several decades, dating back to at least 1994. * In those products, the kernel was *necessarily* entirely unaware of whether RS485 was in use because most OS's and applications did not support RS485 at the time and as such was implement to be entirely transparent to the OS and application. * Since then, many UARTs have integrated Auto RS485 Enable into a single package and we have moved towards utilizing this integrated functionality when it is available. * I say the above to point out that there is a long history of RS485 Auto Enable without kernel involvement, and therefore in my view it is just as valid to have RS485 Auto Enabled set via hardware without the kernel being aware as it is for the kernel to be aware. * Finally, to the day many applications still do not support RS485. Therefore, having the ability to set RS485 via hardware is still a valuable feature for users, and one that Exar/Maxlinear seems to have recognized, as I don't know why they would even have the EN485 pin if not for this sort of use-case. Perhaps the optimal solution would be if there were a method for the hardware to inform the kernel that it is configured for RS485 Auto Enable via hardware, but I'm not aware of a supported way of doing this. The only thing I can think of at the moment is just a check in the init/setup that sets SER_RS485_ENABLED if FCTR[5] is enabled. I don't know if this would be considered an improvement though. -- Matthew H. > > > 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. > > Okay, it seems fastcom part of the driver doesn't support rs485. > There's stupid naming in this driver, "generic_rs485_config()" isn't > really that generic it seems (and on top of that, the init flow is > hard to follow). :-/ > > > > -- > i.
On Wed, 10 Apr 2024, Matthew Howell wrote: > On Wed, 2024-04-10 at 16:49 +0300, Ilpo Järvinen wrote: > > On Mon, 8 Apr 2024, Matthew Howell wrote: > > > On Mon, 2024-04-08 at 19:48 +0300, Ilpo Järvinen wrote: > > > > 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@sealevel.com> > > > > > > --- > > > > > > 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. > > > > Thought that and device_property_read_bool(dev, > > "linux,rs485-enabled-at-boot-time") > > > > I am not familiar with that property. Is that something that can be set > in userspace or via a kernel parameter? Or is it 'hard-coded' into the > device tree binding for a particular device? Through DT, yes (or ACPI equivalent). > > > 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. > > > > ??? > > > > Are you saying RS485 is enabled without kernel knowing about it? I don't > > think that's the correct way to do things. > > That is correct. However, I wouldn't say it is an incorrect way of doing > things. Some reasoning/justification below: > > * Kernel/Driver/Software independent RS485 Auto Enable is not new. I > can't speak for other vendors, but Sealevel has been creating products > with this functionality for several decades, dating back to at least > 1994. > > * In those products, the kernel was *necessarily* entirely unaware of > whether RS485 was in use because most OS's and applications did not > support RS485 at the time and as such was implement to be entirely > transparent to the OS and application. > > * Since then, many UARTs have integrated Auto RS485 Enable into a single > package and we have moved towards utilizing this integrated > functionality when it is available. > > * I say the above to point out that there is a long history of RS485 > Auto Enable without kernel involvement, and therefore in my view it is > just as valid to have RS485 Auto Enabled set via hardware without the > kernel being aware as it is for the kernel to be aware. Is this "Auto" thing done with some dip switches/jumpers and is cast to stone after hw init? So driver's .probe() could just read that state and adjust kernel rs485 state based one it? > * Finally, to the day many applications still do not support RS485. > Therefore, having the ability to set RS485 via hardware is still a > valuable feature for users, and one that Exar/Maxlinear seems to have > recognized, as I don't know why they would even have the EN485 pin if > not for this sort of use-case. Once the serial line has been transitioned into RS485 mode, userspace does not need to be aware of it (which can happen through DT so no userspace involvement). So this shouldn't be seen as a problem. > Perhaps the optimal solution would be if there were a method for the > hardware to inform the kernel that it is configured for RS485 Auto > Enable via hardware, but I'm not aware of a supported way of doing this. > The only thing I can think of at the moment is just a check in the > init/setup that sets SER_RS485_ENABLED if FCTR[5] is enabled. I don't > know if this would be considered an improvement though. I don't think such mechanism exists beyond DT currently, but I don't see why it could be added.
On Thu, 2024-04-11 at 11:27 +0300, Ilpo Järvinen wrote: > On Wed, 10 Apr 2024, Matthew Howell wrote: > > On Wed, 2024-04-10 at 16:49 +0300, Ilpo Järvinen wrote: > > > On Mon, 8 Apr 2024, Matthew Howell wrote: > > > > On Mon, 2024-04-08 at 19:48 +0300, Ilpo Järvinen wrote: > > > > > 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@sealevel.com> > > > > > > > --- > > > > > > > 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. > > > > > > Thought that and device_property_read_bool(dev, > > > "linux,rs485-enabled-at-boot-time") > > > > > > > I am not familiar with that property. Is that something that can be set > > in userspace or via a kernel parameter? Or is it 'hard-coded' into the > > device tree binding for a particular device? > > Through DT, yes (or ACPI equivalent). > > > > > 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. > > > > > > ??? > > > > > > Are you saying RS485 is enabled without kernel knowing about it? I don't > > > think that's the correct way to do things. > > > > That is correct. However, I wouldn't say it is an incorrect way of doing > > things. Some reasoning/justification below: > > > > * Kernel/Driver/Software independent RS485 Auto Enable is not new. I > > can't speak for other vendors, but Sealevel has been creating products > > with this functionality for several decades, dating back to at least > > 1994. > > > > * In those products, the kernel was *necessarily* entirely unaware of > > whether RS485 was in use because most OS's and applications did not > > support RS485 at the time and as such was implement to be entirely > > transparent to the OS and application. > > > > * Since then, many UARTs have integrated Auto RS485 Enable into a single > > package and we have moved towards utilizing this integrated > > functionality when it is available. > > > > * I say the above to point out that there is a long history of RS485 > > Auto Enable without kernel involvement, and therefore in my view it is > > just as valid to have RS485 Auto Enabled set via hardware without the > > kernel being aware as it is for the kernel to be aware. > > Is this "Auto" thing done with some dip switches/jumpers and is cast to > stone after hw init? So driver's .probe() could just read that state > and adjust kernel rs485 state based one it? > The behaviour of the patch at the moment is that changes to SER_RS485_ENABLED can 'reset' the state of FCTR[5]. For example, if an application sets SER_RS485_ENABLED to true, then back to false, the card will lose the Auto-RS485 Enable setting. On one hand, this provides users a method of 'resetting' FCTR[5] on a channel that they do not want to be RS485 in order to work around that limitation of the EN485 pin, but on the other it a little bit weird to enabling and hen disabling SER_RS485_ENABLED does not leave you where you in the state started if EN485 was asserted at boot. > > * Finally, to the day many applications still do not support RS485. > > Therefore, having the ability to set RS485 via hardware is still a > > valuable feature for users, and one that Exar/Maxlinear seems to have > > recognized, as I don't know why they would even have the EN485 pin if > > not for this sort of use-case. > > Once the serial line has been transitioned into RS485 mode, userspace does > not need to be aware of it (which can happen through DT so no userspace > involvement). So this shouldn't be seen as a problem. > Ah, I had missed of_add_property() in the Device Tree kernel API. Would it be appropriate to find and modify the device tree node from exar_pci_probe()? I ask because it looks like all or most device tree operations in the 8250 driver are confined to 8250_of.c, but I believe the FCTR is specific to Exar so I think your suggestion of checking the register value during probe makes sense. > > Perhaps the optimal solution would be if there were a method for the > > hardware to inform the kernel that it is configured for RS485 Auto > > Enable via hardware, but I'm not aware of a supported way of doing this. > > The only thing I can think of at the moment is just a check in the > > init/setup that sets SER_RS485_ENABLED if FCTR[5] is enabled. I don't > > know if this would be considered an improvement though. > > I don't think such mechanism exists beyond DT currently, but I don't see > why it could be added. > -- > i. If DT already has a mechanism that can be used I would rather do that than add additional mechanisms for the same thing. -- Matthew H.
On Thu, 2024-04-11 at 13:01 -0400, Matthew Howell wrote: > On Thu, 2024-04-11 at 11:27 +0300, Ilpo Järvinen wrote: > > On Wed, 10 Apr 2024, Matthew Howell wrote: > > > On Wed, 2024-04-10 at 16:49 +0300, Ilpo Järvinen wrote: > > > > On Mon, 8 Apr 2024, Matthew Howell wrote: > > > > > On Mon, 2024-04-08 at 19:48 +0300, Ilpo Järvinen wrote: > > > > > > 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@sealevel.com> > > > > > > > > --- > > > > > > > > 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. > > > > > > > > Thought that and device_property_read_bool(dev, > > > > "linux,rs485-enabled-at-boot-time") > > > > > > > > > > I am not familiar with that property. Is that something that can be set > > > in userspace or via a kernel parameter? Or is it 'hard-coded' into the > > > device tree binding for a particular device? > > > > Through DT, yes (or ACPI equivalent). > > > > > > > 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. > > > > > > > > ??? > > > > > > > > Are you saying RS485 is enabled without kernel knowing about it? I don't > > > > think that's the correct way to do things. > > > > > > That is correct. However, I wouldn't say it is an incorrect way of doing > > > things. Some reasoning/justification below: > > > > > > * Kernel/Driver/Software independent RS485 Auto Enable is not new. I > > > can't speak for other vendors, but Sealevel has been creating products > > > with this functionality for several decades, dating back to at least > > > 1994. > > > > > > * In those products, the kernel was *necessarily* entirely unaware of > > > whether RS485 was in use because most OS's and applications did not > > > support RS485 at the time and as such was implement to be entirely > > > transparent to the OS and application. > > > > > > * Since then, many UARTs have integrated Auto RS485 Enable into a single > > > package and we have moved towards utilizing this integrated > > > functionality when it is available. > > > > > > * I say the above to point out that there is a long history of RS485 > > > Auto Enable without kernel involvement, and therefore in my view it is > > > just as valid to have RS485 Auto Enabled set via hardware without the > > > kernel being aware as it is for the kernel to be aware. > > > > Is this "Auto" thing done with some dip switches/jumpers and is cast to > > stone after hw init? So driver's .probe() could just read that state > > and adjust kernel rs485 state based one it? > > > > The behaviour of the patch at the moment is that changes to > SER_RS485_ENABLED can 'reset' the state of FCTR[5]. For example, if an > application sets SER_RS485_ENABLED to true, then back to false, the card > will lose the Auto-RS485 Enable setting. > > On one hand, this provides users a method of 'resetting' FCTR[5] on a > channel that they do not want to be RS485 in order to work around that > limitation of the EN485 pin, but on the other it a little bit weird to > enabling and hen disabling SER_RS485_ENABLED does not leave you where > you in the state started if EN485 was asserted at boot. > > > > * Finally, to the day many applications still do not support RS485. > > > Therefore, having the ability to set RS485 via hardware is still a > > > valuable feature for users, and one that Exar/Maxlinear seems to have > > > recognized, as I don't know why they would even have the EN485 pin if > > > not for this sort of use-case. > > > > Once the serial line has been transitioned into RS485 mode, userspace does > > not need to be aware of it (which can happen through DT so no userspace > > involvement). So this shouldn't be seen as a problem. > > > > Ah, I had missed of_add_property() in the Device Tree kernel API. > > Would it be appropriate to find and modify the device tree node from > exar_pci_probe()? I ask because it looks like all or most device tree > operations in the 8250 driver are confined to 8250_of.c, but I believe > the FCTR is specific to Exar so I think your suggestion of checking the > register value during probe makes sense. > > > > Perhaps the optimal solution would be if there were a method for the > > > hardware to inform the kernel that it is configured for RS485 Auto > > > Enable via hardware, but I'm not aware of a supported way of doing this. > > > The only thing I can think of at the moment is just a check in the > > > init/setup that sets SER_RS485_ENABLED if FCTR[5] is enabled. I don't > > > know if this would be considered an improvement though. > > > > I don't think such mechanism exists beyond DT currently, but I don't see > > why it could be added. > > > -- > > i. > > If DT already has a mechanism that can be used I would rather do that > than add additional mechanisms for the same thing. > After looking into this some more, I'm not sure if Device Tree will work. I was initially under the impression that DT was present for both embedded and ACPI capable systems due to /sys/devices/LNXSYSTEM being described as being a Device Tree representation of ACPI. However, after looking further I am not sure that this means that DT properties are respected on ACPI systems. Do you know whether this is the case? If not, do you know the ACPI equivalent of "rs485-enabled-at-boot-time"? I've not yet had much luck finding anything. -- Matthew H. > -- > Matthew H.
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);
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@sealevel.com> ---