All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jiri Slaby <jslaby@suse.com>,
	linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 0/2] serial: 8250_dw: add fractional divisor support
Date: Tue, 3 Jul 2018 10:22:57 +0800	[thread overview]
Message-ID: <20180703102257.678db6a8@xhacker.debian> (raw)
In-Reply-To: <59b14f97234271a7d859fa07f27bd66ec252ccc9.camel@linux.intel.com>

Hi,

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

> On Mon, 2018-07-02 at 13:18 +0300, Andy Shevchenko wrote:
> > On Mon, 2018-07-02 at 18:04 +0800, Jisheng Zhang wrote:  
> > > 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?

Thanks,
Jisheng

WARNING: multiple messages have this Message-ID (diff)
From: Jisheng.Zhang@synaptics.com (Jisheng Zhang)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 0/2] serial: 8250_dw: add fractional divisor support
Date: Tue, 3 Jul 2018 10:22:57 +0800	[thread overview]
Message-ID: <20180703102257.678db6a8@xhacker.debian> (raw)
In-Reply-To: <59b14f97234271a7d859fa07f27bd66ec252ccc9.camel@linux.intel.com>

Hi,

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

> On Mon, 2018-07-02 at 13:18 +0300, Andy Shevchenko wrote:
> > On Mon, 2018-07-02 at 18:04 +0800, Jisheng Zhang wrote:  
> > > 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?

Thanks,
Jisheng

WARNING: multiple messages have this Message-ID (diff)
From: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jiri Slaby <jslaby@suse.com>, <linux-serial@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 0/2] serial: 8250_dw: add fractional divisor support
Date: Tue, 3 Jul 2018 10:22:57 +0800	[thread overview]
Message-ID: <20180703102257.678db6a8@xhacker.debian> (raw)
In-Reply-To: <59b14f97234271a7d859fa07f27bd66ec252ccc9.camel@linux.intel.com>

Hi,

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

> On Mon, 2018-07-02 at 13:18 +0300, Andy Shevchenko wrote:
> > On Mon, 2018-07-02 at 18:04 +0800, Jisheng Zhang wrote:  
> > > 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?

Thanks,
Jisheng

  reply	other threads:[~2018-07-03  2:22 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-02 10:04 [PATCH 0/2] serial: 8250_dw: add fractional divisor support Jisheng Zhang
2018-07-02 10:04 ` Jisheng Zhang
2018-07-02 10:05 ` [PATCH 1/2] serial: 8250: introduce get_divisor() and set_divisor() hook Jisheng Zhang
2018-07-02 10:05   ` Jisheng Zhang
2018-07-02 10:06 ` [PATCH 2/2] serial: 8250_dw: add fractional divisor support Jisheng Zhang
2018-07-02 10:06   ` Jisheng Zhang
2018-07-02 10:18 ` [PATCH 0/2] " Andy Shevchenko
2018-07-02 10:18   ` Andy Shevchenko
2018-07-02 11:51   ` Andy Shevchenko
2018-07-02 11:51     ` Andy Shevchenko
2018-07-03  2:22     ` Jisheng Zhang [this message]
2018-07-03  2:22       ` Jisheng Zhang
2018-07-03  2:22       ` Jisheng Zhang
2018-07-03  2:48       ` Jisheng Zhang
2018-07-03  2:48         ` Jisheng Zhang
2018-07-03  2:48         ` Jisheng Zhang
2018-07-03 13:03         ` Andy Shevchenko
2018-07-03 13:03           ` Andy Shevchenko
2018-07-03 13:02       ` Andy Shevchenko
2018-07-03 13:02         ` Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180703102257.678db6a8@xhacker.debian \
    --to=jisheng.zhang@synaptics.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jslaby@suse.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.