Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Date:   Fri, 15 Jul 2022 09:42:05 +0200
From:   Michael Walle <michael@walle.cc>
To:     Sherry Sun <sherry.sun@nxp.com>
Cc:     gregkh@linuxfoundation.org, jirislaby@kernel.org,
        linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org,
        dl-linux-imx <linux-imx@nxp.com>
Subject: Re: [PATCH 2/2] tty: serial: fsl_lpuart: writing a 1 and then a 0 to
 trigger a break character
In-Reply-To: <AS8PR04MB840448675E64E4FCDEEF91A1928B9@AS8PR04MB8404.eurprd04.prod.outlook.com>
References: <20220715025944.11076-1-sherry.sun@nxp.com>
 <20220715025944.11076-3-sherry.sun@nxp.com>
 <509669b26b5899088e9b77ed94d103ee@walle.cc>
 <AS8PR04MB840448675E64E4FCDEEF91A1928B9@AS8PR04MB8404.eurprd04.prod.outlook.com>
User-Agent: Roundcube Webmail/1.4.13
Message-ID: <e2560f01fd1731ea2422d82c97efcc6f@walle.cc>
Precedence: bulk

Hi,

Am 2022-07-15 09:20, schrieb Sherry Sun:
>> Subject: Re: [PATCH 2/2] tty: serial: fsl_lpuart: writing a 1 and then 
>> a 0 to
>> trigger a break character
>> 
>> Hi,
>> 
>> Am 2022-07-15 04:59, schrieb Sherry Sun:
>> > According to the lpuart reference manual, need to writing a 1 and then
>> > a
>> > 0 to the UARTCTRL_SBK field queues a break character in the transmit
>> > data stream. Only writing a 1 cannot trigger the break character, so
>> > fix it.
>> 
>> I don't think this is correct. The tty core will already call this:
>>    .break_ctl(port, 1)
>>    usleep()
>>    .break_ctl(port, 0)
>> 
>> So you'll have your 1->0 transition.
>> 
>> My RM from the LS1028A says the following:
>> 
>> | Writing a 1 and then a 0 to SBK queues a break character in the
>> | transmit data stream. Additional break characters of 10 to 13, or 13
>> | to 16 if LPUART_STATBRK13] is set, bit times of logic 0 are queued 
>> as
>> | long as SBK is set. Depending on the timing of the set and clear of
>> | SBK relative to the information currently being transmitted, a 
>> second
>> | break character may be queued before software clears SBK.
>> 
>> To me it seems that setting the SBK bit just pulls the TX line low and 
>> releasing
>> it will return to normal transmitter mode.
>> 
> 
> Hi Michael,
> 
> Actually set break_ctl(tty, -1) then break_ctl(tty, 0) is only done in
> the send_break() function.
> If we call TIOCSBRK from user space, it will only set break_ctl(tty,
> -1) without break_ctl(tty, 0).

That is expected. no? There is also the TIOCCBRK which will clear the
break. TIOCSBRK will just turn the break on.

I'm not sure if the break is already transmitted when the SBK bit
is set, though. Is that your problem here? I'd need to check that
on the real hardware.

> And from the definition of .break_ctl(port,ctl), the callback is used
> to Control the transmission of a break
> signal(Documentation/driver-api/serial/driver.rst), if ctl is nonzero,
> it should queues a break character. I don't think it is reasonable to
> call break_ctl() twice in order to send one break signal.

Maybe Gred can correct me, but to me it seems like the .break_ctl()
will set the *state* according to the argument, that is either
turning it on or turning it off (Except if TTY_DRIVER_HARDWARE_BREAK
is set, but that doesn't seem to be supported by the ioctl interface.)

> Also I have tried other uart IP, such as drivers/tty/serial/imx.c, it
> also queues a break character if we call break_ctl() once. So I
> believe the break_ctl() in lpuart driver should be fixed.

-michael