From: hammer hsieh <hammerh0314@gmail.com>
To: Jiri Slaby <jirislaby@kernel.org>
Cc: Greg KH <gregkh@linuxfoundation.org>,
robh+dt@kernel.org, linux-serial@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
p.zabel@pengutronix.de, wells.lu@sunplus.com,
"hammer.hsieh" <hammer.hsieh@sunplus.com>
Subject: Re: [PATCH v6 2/2] serial:sunplus-uart:Add Sunplus SoC UART Driver
Date: Thu, 13 Jan 2022 16:54:17 +0800 [thread overview]
Message-ID: <CAOX-t54oA9V94d3901w2xKSagSzmXc9r=TDTtbgaSLfL1DxNbw@mail.gmail.com> (raw)
In-Reply-To: <fcd43c65-6201-9e44-061c-f04e39cef726@kernel.org>
Jiri Slaby <jirislaby@kernel.org> 於 2022年1月13日 週四 下午3:06寫道:
>
> Hi,
>
> On 12. 01. 22, 10:24, Hammer Hsieh wrote:
> > Add Sunplus SoC UART Driver
> ...
> > --- /dev/null
> > +++ b/drivers/tty/serial/sunplus-uart.c
> > @@ -0,0 +1,756 @@
> ...
> > +/* Register offsets */
> > +#define SUP_UART_DATA 0x00
> > +#define SUP_UART_LSR 0x04
> > +#define SUP_UART_MSR 0x08
> > +#define SUP_UART_LCR 0x0C
> > +#define SUP_UART_MCR 0x10
> > +#define SUP_UART_DIV_L 0x14
> > +#define SUP_UART_DIV_H 0x18
> > +#define SUP_UART_ISC 0x1C
> > +#define SUP_UART_TX_RESIDUE 0x20
> > +#define SUP_UART_RX_RESIDUE 0x24
> > +
> > +/* Line Status Register bits */
> > +#define SUP_UART_LSR_BC BIT(5) /* break condition status */
> > +#define SUP_UART_LSR_FE BIT(4) /* frame error status */
> > +#define SUP_UART_LSR_OE BIT(3) /* overrun error status */
> > +#define SUP_UART_LSR_PE BIT(2) /* parity error status */
>
> I just wonder why do the HW creators feel so creative to redefine the
> world...
>
Our IC designer create it, I just try to make it work.
Indeed, before Greg KH mentioned 8250-like, I didn't realize our uart
so much like 8250.
Thanks for your review.
> > +static void sunplus_shutdown(struct uart_port *port)
> > +{
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&port->lock, flags);
> > + writel(0, port->membase + SUP_UART_ISC);
> > + spin_unlock_irqrestore(&port->lock, flags);
>
> I asked last time:
> * What bus is this -- posting?
>
> You replied:
> * Here just clear interrupt.
> * Not really understand your comment?
>
> So I am asking again:
> What bus is this? Isn't a posted write a problem here? I mean, shouldn't
> you read from the register so that the write hits the device? That
> depends on the bus this sits on, so just asking.
>
Each UART has its own ISC register.
Ex.
dev/ttySUP0 base_adr = 0x9C00-0000 , isc_addr = 0x9C00-001C
dev/ttySUP1 base_adr = 0x9C00-0080 , isc_addr = 0x9C00-009C
dev/ttySUP2 base_adr = 0x9C00-0100 , isc_addr = 0x9C00-011C
dev/ttySUP3 base_adr = 0x9C00-0180 , isc_addr = 0x9C00-019C
dev/ttySUP4 base_adr = 0x9C00-0200 , isc_addr = 0x9C00-021C
So sunplus_shutdown() just simply turn off its own device isc only.
That's why I didn't read register value, just write 0 for it.
> Other than that the driver looks much better now, i.e. LGTM.
>
> thanks,
> --
> js
> suse labs
next prev parent reply other threads:[~2022-01-13 8:54 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-12 9:24 [PATCH v6 0/2] Add UART driver for Suplus SP7021 SoC Hammer Hsieh
2022-01-12 9:24 ` [PATCH v6 1/2] dt-bindings:serial:Add bindings doc for Sunplus SoC UART Driver Hammer Hsieh
2022-01-12 9:24 ` [PATCH v6 2/2] serial:sunplus-uart:Add " Hammer Hsieh
2022-01-13 7:06 ` Jiri Slaby
2022-01-13 8:54 ` hammer hsieh [this message]
2022-01-13 9:08 ` Jiri Slaby
2022-01-13 10:56 ` hammer hsieh
2022-01-13 11:12 ` Jiri Slaby
2022-01-14 2:22 ` hammer hsieh
2022-01-26 13:47 ` Greg KH
2022-01-28 3:36 ` hammer hsieh
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='CAOX-t54oA9V94d3901w2xKSagSzmXc9r=TDTtbgaSLfL1DxNbw@mail.gmail.com' \
--to=hammerh0314@gmail.com \
--cc=devicetree@vger.kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=hammer.hsieh@sunplus.com \
--cc=jirislaby@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-serial@vger.kernel.org \
--cc=p.zabel@pengutronix.de \
--cc=robh+dt@kernel.org \
--cc=wells.lu@sunplus.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).