linux-kernel - Re: [PATCH v7 2/2] serial:sunplus-uart:Add Sunplus SoC UART Driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 9 Feb 2022 18:53:03 +0800
From:   hammer hsieh <hammerh0314@...il.com>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     Jiri Slaby <jirislaby@...nel.org>, robh+dt@...nel.org,
        linux-serial@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, p.zabel@...gutronix.de,
        wells.lu@...plus.com, "hammer.hsieh" <hammer.hsieh@...plus.com>
Subject: Re: [PATCH v7 2/2] serial:sunplus-uart:Add Sunplus SoC UART Driver

Ok, thanks for your explaination, I got it now.
I will document "posted write" info in the top of the file or top of
startup/shutdown function.

And kernel test robot report me build error and warning with gcc
11.2.0 ia64 / powerpc.
I will fix it and send next patch.

Greg KH <gregkh@...uxfoundation.org> 於 2022年2月8日 週二 下午7:31寫道：
>
> On Tue, Feb 08, 2022 at 07:16:52PM +0800, hammer hsieh wrote:
> > Jiri Slaby <jirislaby@...nel.org> 於 2022年2月8日 週二 下午2:27寫道：
> > >
> > > Hi,
> > >
> > > On 07. 02. 22, 6:58, Hammer Hsieh wrote:
> > > > +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);
> > > > +     isc &= ~(SUP_UART_ISC_RXM | SUP_UART_ISC_TXM);
> > >
> > > Is this correct? I mean: will the SUP_UART_ISC read contain the control
> > > bits, not only status bits?
> > >
> >
> > I assume reviewers don't like writel(0,xxx).
> > So I use definition to let the code easy to read.
> > The purpose is to clear all interrupt.
> > Bit[3:0] status bit only for read, write 1 or 0 no effect.
> >
> > > > +     writel(isc, port->membase + SUP_UART_ISC);
> > > > +
> > > > +     spin_unlock_irqrestore(&port->lock, flags);
> > > > +
> > > > +     free_irq(port->irq, port);
> > >
> > > I am still waiting for explanation why this is safe with respect to
> > > posted writes.
> > >
> >
> > Actually I'm not IC designer, not expert for bus design.
> > About data incoherence issue between memory bus and peripheral bus.
> > In case of AXI bus, use non-posted write can avoid data incoherence issue.
> > What if in case of posted write:
> > Send a specific command after last write command.
> > SDCTRL identify specific command, means previous write command done.
> > Then send interrupt signal to interrupt controller.
> > And then interrupt controller send done signal to Master.
> > Master receive done signal, means write command done.
> > Then issue a interrupt or proceed next write command.
>
> But how does the kernel know when the write is completed?  The kernel
> seems to ignore that here entirely, so the write could actually complete
> seconds later, which would not be a good thing, right?
>
> Traditionally, we want to ensure that a write() completes, so on some
> busses, we have to do a read to ensure that the write made it to the
> hardware before we can continue on.  That is not happening here which is
> why Jiri keeps bringing it up.  It looks broken to us, and you need to
> document it somewhere (in the changelog?  In the top of the file?) as to
> why this is not needed.
>
> thanks,
>
> greg k-h