On Tue, 3 Jul 2018 10:22:57 +0800 Jisheng Zhang wrote:

Hi,

On Mon, 2 Jul 2018 14:51:03 +0300 Andy Shevchenko wrote:

quoted

On Mon, 2018-07-02 at 13:18 +0300, Andy Shevchenko wrote:  

quoted

On Mon, 2018-07-02 at 18:04 +0800, Jisheng Zhang wrote:    

quoted

For Synopsys DesignWare 8250 uart which version >= 4.00a, there's a
valid divisor latch fraction register. The fractional divisor width
is
4bits ~ 6bits.
    

There are several serial IPs that have fractional divider built-in.
None
is using any specific hooks. Why do you need in your case, esp. taking
into consideration that we have a custom ->set_termios() callback?    

Okay, I see that in 8250 we have hooks embedded into 8250_port.c which
is not the best solution.

For example it prevents better splitting Exar code.
So, we would need these hooks, but better to integrate them in the same
way like it's done for the rest of 8250 ones, i.e.
- rename existing to have a "do" word
- create new functions which would be a replacement that choose between
"do" variant and custom one
- not sure if we need to export "do" variants (at least for now)  

So you mean add the support as following:

1.rename current serial8250_set_divisor as serial8250_do_set_divisor

2.add a new serial8250_set_divisor which will be as simple as

static void serial8250_set_divisor(struct uart_port *port, ...)
{
	struct uart_8250_port *up = up_to_u8250p(port);

	if (up->set_divisor)
		up->set_divisor(...);
	else
		serial8250_do_set_divisor(...);
}

could you please confirm?

Another issue is I'm not sure which struct to add the hook,
struct uart_port or struct uart_8250_port? Currently, it seems that only
uart_8250_port needs this hook, sure, adding the hook to struct uart_port
is fine either. Could you please kindly give some suggestions?

patching struct uart_port seems a bit overhead. After reading the code
again, I propose another solution, similar as what dl_write() is used in
8250 core:

1.introduce the hook to struct uart_8250_port as my previous patches do, 

2.rename current serial8250_set_divisor() as default_serial_get_divisor()
then introduce a new serial8250_set_divisor() as:
static inline void serial8250_set_divisor(struct uart_8250_port *up,....)
{
	up->set_divisor();
}

and point up->set-divisor to default_serial_get_divisor

what do you think about this solution?

Thanks

Thanks,
Jisheng