| Message ID | 20240125100619.154873-1-rengarajan.s@microchip.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v1,tty] 8250: microchip: Add 4 Mbps support in PCI1XXXX UART | expand |
On Thu, Jan 25, 2024 at 03:36:19PM +0530, Rengarajan S wrote: > The current clock input is set to 62.5 MHz for supporting fractional > divider, which enables generation of an acceptable baud rate from any > frequency. With the current clock input the baud rate range is limited > to 3.9 Mbps. Hence, the current range is extended to support 4 Mbps > with Burst mode operation. Divisor calculation for a given baud rate is > updated as the sampling rate is reduced from 16 to 8 for 4 Mbps. ... > +#define UART_BAUD_4MBPS 4000000 (4 * MEGA) ? (will need to include units.h, if not yet) ... > + frac_div = readl(port->membase + FRAC_DIV_CFG_REG); > + Unneeded blank line. > + if (frac_div == UART_BIT_DIVISOR_16) > + sample_cnt = UART_BIT_SAMPLE_CNT_16; > + else > + sample_cnt = UART_BIT_SAMPLE_CNT_8; ... > + /* > + * Microchip PCI1XXXX UART supports maximum baud rate up to 4 Mbps > + */ > + if (up->port.type == PORT_MCHP16550A) > + max = 4000000; No. Please refactor the way the 8250_port won't be modified. Also you have a define for this constant, use it.
On Sun, Jan 28, 2024 at 05:27:24PM +0200, Andy Shevchenko wrote: > On Thu, Jan 25, 2024 at 03:36:19PM +0530, Rengarajan S wrote: ... > > +#define UART_BAUD_4MBPS 4000000 > > (4 * MEGA) ? (will need to include units.h, if not yet) ...and use proper namespace for the definition. UART_ one is not owned by this driver.
Hi Andy Shevchenko, Thanks for reviewing the patch. Please find my comments inline. On Sun, 2024-01-28 at 17:27 +0200, Andy Shevchenko wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you > know the content is safe > > On Thu, Jan 25, 2024 at 03:36:19PM +0530, Rengarajan S wrote: > > The current clock input is set to 62.5 MHz for supporting > > fractional > > divider, which enables generation of an acceptable baud rate from > > any > > frequency. With the current clock input the baud rate range is > > limited > > to 3.9 Mbps. Hence, the current range is extended to support 4 Mbps > > with Burst mode operation. Divisor calculation for a given baud > > rate is > > updated as the sampling rate is reduced from 16 to 8 for 4 Mbps. > > ... > > > +#define UART_BAUD_4MBPS 4000000 > > (4 * MEGA) ? (will need to include units.h, if not yet) Thanks. Will address the change in the next patch revision. > > ... > > > + frac_div = readl(port->membase + FRAC_DIV_CFG_REG); > > > + > > Unneeded blank line. Will remove it in the next patch revision. > > > + if (frac_div == UART_BIT_DIVISOR_16) > > + sample_cnt = UART_BIT_SAMPLE_CNT_16; > > + else > > + sample_cnt = UART_BIT_SAMPLE_CNT_8; > > ... > > > + /* > > + * Microchip PCI1XXXX UART supports maximum baud rate up to 4 > > Mbps > > + */ > > + if (up->port.type == PORT_MCHP16550A) > > + max = 4000000; > > No. Please refactor the way the 8250_port won't be modified. > > Also you have a define for this constant, use it. The current UART clk in MCHP Ports in pci1xxxx.c is set to 62.5 MHz in order to support fractional baud rates which enables generation of acceptable baud rate and lower error percentage from any available frequency. With 62.5 MHz the maximum supported baud rate supported as per serial_8250_get_baud_rate is 3.9 Mbps. In order to extend the support to 4 Mbps we had hardcoded the max value to 4 Mbps. Since, baud rate is calculated here we needed to make these changes in 8250_port and could not find a way to handle as part 8250_pci1xxxx. Can you let us know any alternatives to address this upper(max) limit? > > -- > With Best Regards, > Andy Shevchenko > >
On Thu, 2024-02-01 at 13:52 +0200, Andy Shevchenko wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you > know the content is safe > > On Tue, Jan 30, 2024 at 10:52:41AM +0000, > Rengarajan.S@microchip.com wrote: > > On Sun, 2024-01-28 at 17:27 +0200, Andy Shevchenko wrote: > > > On Thu, Jan 25, 2024 at 03:36:19PM +0530, Rengarajan S wrote: > > ... > > > > > + /* > > > > + * Microchip PCI1XXXX UART supports maximum baud rate up > > > > to 4 > > > > Mbps > > > > + */ > > > > + if (up->port.type == PORT_MCHP16550A) > > > > + max = 4000000; > > > > > > No. Please refactor the way the 8250_port won't be modified. > > > > > > Also you have a define for this constant, use it. > > > > The current UART clk in MCHP Ports in pci1xxxx.c is set to 62.5 MHz > > in > > order to support fractional baud rates which enables generation of > > acceptable baud rate and lower error percentage from any available > > frequency. With 62.5 MHz the maximum supported baud rate supported > > as > > per serial_8250_get_baud_rate is 3.9 Mbps. In order to extend the > > support to 4 Mbps we had hardcoded the max value to 4 Mbps. Since, > > baud > > rate is calculated here we needed to make these changes in > > 8250_port > > and could not find a way to handle as part 8250_pci1xxxx. Can you > > let > > us know any alternatives to address this upper(max) limit? > > Update port->uartclk accordingly in your driver, see how other 8250_* > drivers > do that (e.g., 8250_mid). > > So, it will no go with hack in the 8250_port. Hi Andy Shevchenko, The UART clock is currently fixed to 62.5 MHz for supporting fractional baud rates which allows the user to set any given baud rate with lower errors. Changing the clock would no longer support the fractional baud rate support feature. However, will check for any alternate way to add the support in the 8250_pci1xxxx driver file and will update the patch accordingly in the next revision. > > -- > With Best Regards, > Andy Shevchenko > >
diff --git a/drivers/tty/serial/8250/8250_pci1xxxx.c b/drivers/tty/serial/8250/8250_pci1xxxx.c index d53605bf908d..6cfeba058dba 100644 --- a/drivers/tty/serial/8250/8250_pci1xxxx.c +++ b/drivers/tty/serial/8250/8250_pci1xxxx.c @@ -82,7 +82,8 @@ #define ADCL_CFG_PIN_SEL BIT(1) #define ADCL_CFG_EN BIT(0) -#define UART_BIT_SAMPLE_CNT 16 +#define UART_BIT_SAMPLE_CNT_8 8 +#define UART_BIT_SAMPLE_CNT_16 16 #define BAUD_CLOCK_DIV_INT_MSK GENMASK(31, 8) #define ADCL_CFG_RTS_DELAY_MASK GENMASK(11, 8) #define UART_CLOCK_DEFAULT (62500 * HZ_PER_KHZ) @@ -96,6 +97,7 @@ (UART_WAKE_N_PIN | UART_WAKE_NCTS | UART_WAKE_INT) #define UART_BAUD_CLK_DIVISOR_REG 0x54 +#define FRAC_DIV_CFG_REG 0x58 #define UART_RESET_REG 0x94 #define UART_RESET_D3_RESET_DISABLE BIT(16) @@ -104,6 +106,10 @@ #define UART_TX_BURST_FIFO 0xA0 #define UART_RX_BURST_FIFO 0xA4 +#define UART_BIT_DIVISOR_8 0x26731000 +#define UART_BIT_DIVISOR_16 0x6ef71000 +#define UART_BAUD_4MBPS 4000000 + #define MAX_PORTS 4 #define PORT_OFFSET 0x100 #define RX_BUF_SIZE 512 @@ -210,15 +216,24 @@ static int pci1xxxx_get_num_ports(struct pci_dev *dev) static unsigned int pci1xxxx_get_divisor(struct uart_port *port, unsigned int baud, unsigned int *frac) { + unsigned int uart_sample_cnt; unsigned int quot; + if (baud >= UART_BAUD_4MBPS) { + uart_sample_cnt = UART_BIT_SAMPLE_CNT_8; + writel(UART_BIT_DIVISOR_8, (port->membase + FRAC_DIV_CFG_REG)); + } else { + uart_sample_cnt = UART_BIT_SAMPLE_CNT_16; + writel(UART_BIT_DIVISOR_16, (port->membase + FRAC_DIV_CFG_REG)); + } + /* * Calculate baud rate sampling period in nanoseconds. * Fractional part x denotes x/255 parts of a nanosecond. */ - quot = NSEC_PER_SEC / (baud * UART_BIT_SAMPLE_CNT); - *frac = (NSEC_PER_SEC - quot * baud * UART_BIT_SAMPLE_CNT) * - 255 / UART_BIT_SAMPLE_CNT / baud; + quot = NSEC_PER_SEC / (baud * uart_sample_cnt); + *frac = (NSEC_PER_SEC - quot * baud * uart_sample_cnt) * + 255 / uart_sample_cnt / baud; return quot; } @@ -237,7 +252,16 @@ static int pci1xxxx_rs485_config(struct uart_port *port, u32 delay_in_baud_periods; u32 baud_period_in_ns; u32 mode_cfg = 0; + u32 sample_cnt; u32 clock_div; + u32 frac_div; + + frac_div = readl(port->membase + FRAC_DIV_CFG_REG); + + if (frac_div == UART_BIT_DIVISOR_16) + sample_cnt = UART_BIT_SAMPLE_CNT_16; + else + sample_cnt = UART_BIT_SAMPLE_CNT_8; /* * pci1xxxx's uart hardware supports only RTS delay after @@ -253,7 +277,7 @@ static int pci1xxxx_rs485_config(struct uart_port *port, clock_div = readl(port->membase + UART_BAUD_CLK_DIVISOR_REG); baud_period_in_ns = FIELD_GET(BAUD_CLOCK_DIV_INT_MSK, clock_div) * - UART_BIT_SAMPLE_CNT; + sample_cnt; delay_in_baud_periods = rs485->delay_rts_after_send * NSEC_PER_MSEC / baud_period_in_ns; diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c index 925ee1d61afb..2a85bc9475f9 100644 --- a/drivers/tty/serial/8250/8250_port.c +++ b/drivers/tty/serial/8250/8250_port.c @@ -2688,6 +2688,7 @@ static unsigned int serial8250_get_baud_rate(struct uart_port *port, struct ktermios *termios, const struct ktermios *old) { + struct uart_8250_port *up = up_to_u8250p(port); unsigned int tolerance = port->uartclk / 100; unsigned int min; unsigned int max; @@ -2705,6 +2706,12 @@ static unsigned int serial8250_get_baud_rate(struct uart_port *port, max = (port->uartclk + tolerance) / 16; } + /* + * Microchip PCI1XXXX UART supports maximum baud rate up to 4 Mbps + */ + if (up->port.type == PORT_MCHP16550A) + max = 4000000; + /* * Ask the core to calculate the divisor for us. * Allow 1% tolerance at the upper limit so uart clks marginally
The current clock input is set to 62.5 MHz for supporting fractional divider, which enables generation of an acceptable baud rate from any frequency. With the current clock input the baud rate range is limited to 3.9 Mbps. Hence, the current range is extended to support 4 Mbps with Burst mode operation. Divisor calculation for a given baud rate is updated as the sampling rate is reduced from 16 to 8 for 4 Mbps. Signed-off-by: Rengarajan S <rengarajan.s@microchip.com> --- drivers/tty/serial/8250/8250_pci1xxxx.c | 34 +++++++++++++++++++++---- drivers/tty/serial/8250/8250_port.c | 7 +++++ 2 files changed, 36 insertions(+), 5 deletions(-)