linux-kernel - Re: [PATCH 1/2] USB: serial: cp210x: Implement 16-bit register access functions

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAL_jEz-RToesTdLX0XA1k7BirJtDDEYweycY3o8ZsgduTasUHQ@mail.gmail.com>
Date:	Tue, 20 Oct 2015 09:19:05 -0500
From:	Konstantin Shkolnyy <konstantin.shkolnyy@...il.com>
To:	Johan Hovold <johan@...nel.org>
Cc:	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] USB: serial: cp210x: Implement 16-bit register access functions

On Tue, Oct 20, 2015 at 8:02 AM, Johan Hovold <johan@...nel.org> wrote:
> On Tue, Oct 20, 2015 at 07:52:31AM -0500, Konstantin Shkolnyy wrote:
>> On Tue, Oct 20, 2015 at 2:45 AM, Johan Hovold <johan@...nel.org> wrote:
>> [...]
>> > Instead of adding the new helpers to read u16 as a prerequisite for
>> > fixing the broken cp2108 support, just reuse the current config register
>> > helpers for now (in order to keep the fixes minimal and potentially
>> > backportable). Once the fixes are in place, feel free to clean up the
>> > remaining register accesses.
>>
>> The current helpers take "port" as a parameter. You pointed out
>> previously that port shouldn't be used in probe(). That made me
>> implement new helpers cp210x_write_u16_reg and cp210x_read_u16_reg
>> that don't use port. Probe() now calls cp210x_activate_workarounds
>> which in turn calls these new helpers.
>
> Oh, that's right.
>
>> An alternative would be to call usb_control_msg from
>> cp210x_activate_workarounds, but I think it would make it look pretty
>> ugly.
>
> Or you move the quirk-detect (and private data allocation) to port_probe
> instead (and remove startup/release). These devices have exactly one
> port per interface, so you wouldn't introduce any redundancy.

OK, how about this:
patch #1
- attach/release replaced with port_probe/port_release;
- changes in the code using the "private" data since it has moved from
serial to port;
- cp210x_activate_workarounds in the port_probe, using the current helpers;
- a new helper cp210x_get_line_ctl that calls the current helper and
swaps the bytes.
patch #2
- sending "purge" from close, using the current helper.

However, the Tx queue bug fixed by the "purge" doesn't have a
detection method. We only know that it exists in the same cp2108 that
has the "byte swap" bug. So the "purge" is activated by the same "byte
swap" flag and  patch #2 depends on patch #1. Also, there is no reason
to only apply one of the patches. Do you think we can merge these
patches?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/