linux-kernel - Re: [PATCH V9 1/1] usb:serial: Add Fintek F81532/534 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]

Message-ID: <3e42b288-2b98-f2aa-2758-321fee4cb65d@gmail.com>
Date:   Wed, 24 Aug 2016 14:36:51 +0800
From:   "Ji-Ze Hong (Peter Hong)" <hpeter@...il.com>
To:     Johan Hovold <johan@...nel.org>
Cc:     gregkh@...uxfoundation.org,
        One Thousand Gnomes <gnomes@...rguk.ukuu.org.uk>,
        tom_tsai@...tek.com.tw, peter_hong@...tek.com.tw,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
        "Ji-Ze Hong (Peter Hong)" <hpeter+linux_kernel@...il.com>
Subject: Re: [PATCH V9 1/1] usb:serial: Add Fintek F81532/534 driver

Hi Johan,

Johan Hovold 於 2016/8/23 下午 05:50 寫道:
> On Tue, Aug 23, 2016 at 04:23:44PM +0800, Ji-Ze Hong (Peter Hong) wrote:
>> Hi Johan,
>>
>> Johan Hovold 於 2016/8/22 下午 09:14 寫道:
>
> I'd say it's not worth trying to avoid that extra allocation, and there
> will be several further allocations done in the usb_control_msg path
> anyway. What you have today (i.e. in v9) is fine.

Ok, I'll keep set/get register the same with V9.


>>>> +		tty_port_num = f81534_phy_to_logic_port(serial, phy_port_num);
>>>> +		port = serial->port[tty_port_num];
>>>> +
>>>> +		/*
>>>> +		 * The device will send back all information when we submitted
>>>> +		 * a read URB (MSR/DATA/TX_EMPTY). But it maybe get callback
>>>> +		 * before port_probe() or after port_remove().
>>>> +		 *
>>>> +		 * So we'll verify the pointer. If the pointer is NULL, it's
>>>> +		 * mean the port not init complete and the block will skip.
>>>> +		 */
>>>> +		port_priv = usb_get_serial_port_data(port);
>>>
>>> Check if the port has been opened here instead, no need to store MSR for
>>> an unused port above.
>>
>> It's useless for MSR & Receive data when port is closed, but we need
>> the URB to receive TX empty flag. We may not received TX empty flag
>> if we don't process when port is closed. It'll make the port not
>> workable.
>
> But you explicitly clear the xmit fifo on open it seems?
>

The F81532/534 contains 2 blocks of H/W designs. One is a 16550A
compatible UART with 128 bytes FIFO, and another is a USB bridge with
DMA to access UART TX/RX FIFO and handle USB protocols.

The clear FIFO in f81534_open() is just clean UART TX/RX FIFO, not USB
bridge's RAM. So we must keep a read URB for get newest information via
USB bridge likes TX empty.

I'll try again to re-write the section as you mention, submit on first
open(), kill on last close() and test for some times. If had no other
issues, I'll apply to next patch, otherwise I'll preserve old method.

Thanks for your help.
-- 
With Best Regards,
Peter Hong