Hi, Greg KH:
I review all driver again.
I think only startup and shutdown not good.
I will modify like below.
If you are ok, I will submit next patch.
static int sunplus_startup(struct uart_port *port)
{
unsigned long flags;
unsigned int isc;
int ret;
ret = request_irq(port->irq, sunplus_uart_irq, 0, "sunplus_uart", port);
if (ret)
return ret;
spin_lock_irqsave(&port->lock, flags);
isc = readl(port->membase + SUP_UART_ISC); //add this line
isc |= SUP_UART_ISC_RXM;
writel(isc, port->membase + SUP_UART_ISC);
spin_unlock_irqrestore(&port->lock, flags);
return 0;
}
static void sunplus_shutdown(struct uart_port *port)
{
unsigned long flags;
unsigned int isc;
spin_lock_irqsave(&port->lock, flags);
isc = readl(port->membase + SUP_UART_ISC); //add this line
isc &= ~(SUP_UART_ISC_RXM | SUP_UART_ISC_TXM); //add this line
writel(isc, port->membase + SUP_UART_ISC); //modify this line
spin_unlock_irqrestore(&port->lock, flags);
free_irq(port->irq, port);
}
Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> 於 2022年1月26日 週三 下午9:47寫道：
>
> On Fri, Jan 14, 2022 at 10:22:56AM +0800, hammer hsieh wrote:
> > Jiri Slaby <jirislaby@xxxxxxxxxx> 於 2022年1月13日 週四 下午7:12寫道：
> > >
> > > On 13. 01. 22, 11:56, hammer hsieh wrote:
> > > >> Could you explain me what posted write is and how does it not matter in
> > > >> this case?
> > > >>
> > > >
> > > > Each UART ISC register contains
> > >
> > > No, you still don't follow what I write. Use your favorite web search
> > > for "posted write" and/or consult with your HW team.
> > >
> >
> > Maybe this time, we are on the same page.
> > Our SP7021 chipset is designed on ARM Cortex-A7 Quad core.
> > Register Access through AMBA(AXI bus), and it is non-cached.
> >
> > Did you mean
> > case1 have concern about "posted write", and you want to know why it not matter?
> > case2 will be safer?
> >
> > Case1 :
> > spin_lock_irq_save()
> > writel(0, target register)
> > spin_unlock_irqrestore()
>
> A lock does not mean that your write made it to the device. Please talk
> to the hardware designers to properly determine how to correctly write
> to the hardware and "know" that the write succeeded or not. This driver
> does not seem to take that into consideration at all.
>
> thanks,
>
> greg k-h
