linux-kernel - Re: [RFC PATCH v2 08/15] usb:cdns3: Implements device operations part of the API

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]

Date:   Fri, 14 Dec 2018 12:47:50 +0200
From:   Felipe Balbi <balbi@...nel.org>
To:     Sekhar Nori <nsekhar@...com>, Peter Chen <hzpeterchen@...il.com>,
        pawell@...ence.com
Cc:     rogerq@...com, devicetree@...r.kernel.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-usb@...r.kernel.org, lkml <linux-kernel@...r.kernel.org>,
        adouglas@...ence.com, jbergsagel@...com, nm@...com,
        sureshp@...ence.com, peter.chen@....com, pjez@...ence.com,
        kurahul@...ence.com
Subject: Re: [RFC PATCH v2 08/15] usb:cdns3: Implements device operations part of the API


Hi,

Sekhar Nori <nsekhar@...com> writes:

<snip>

>>>> All this should be part of comments in code along with information about
>>>> controller versions which suffer from the errata.
>>>>
>>>> Is there a version of controller available which does not have the
>>>> defect? Is there a future plan to fix this?
>>>>
>>>> If any of that is yes, you probably want to handle this with runtime
>>>> detection of version (like done with DWC3_REVISION_XXX macros).
>>>> Sometimes the hardware-read versions themselves are incorrect, so its
>>>> better to introduce a version specific compatible too like
>>>> "cdns,usb-1.0.0" (as hinted to by Rob Herring as well).
>>>>
>>>
>>> custom match_ep is used and works with all versions of the gen1
>>> controller. Future (gen2) releases of the controller won’t have such
>>> limitation but there is no plan to change current (gen1) functionality
>>> of the controller.
>>>
>>> I will add comment before cdns3_gadget_match_ep function.
>>> Also I will change cdns,usb3 to cdns,usb3-1.0.0 and add additional
>>> cdns,usb3-1.0.1 compatible.
>>>
>>> cdns,usb3-1.0.1 will be for current version of controller which I use.
>>> cdns,usb3-1.0.0 will be for older version - Peter Chan platform.
>>> I now that I have some changes in controller, and one of them require
>>> some changes in DRD driver. It will be safer to add two separate
>>> version in compatibles.
>>>
>> 
>> Pawel, could we have correct register to show controller version? It is
>> better we could version judgement at runtime instead of static compatible.
>
> Agree with detecting IP version at runtime.
>
> But please have some indication of version in compatible string too,

why? Runtime detection by revision register should be the way to go if
the HW provides it. Why duplicate the information in compatible string?

> especially since you already know there is going to be another revision
> of hardware. It has the advantage that one can easily grep to see which
> hardware is running current version of controller without having access
> to the hardware itself. Becomes useful later on when its time to
> clean-up unused code when boards become obsolete or for requesting
> testing help.

This doesn't sound like a very strong argument, actually. Specially when
you consider that, since driver will do revision checking based on
revision register, you already have strings to grep. Moreover, we don't
usually drop support just like that.

Personally, I wouldn't bother. Just like dwc3 never bothered and nothing
bad came about because of it. Well, there are quirks which are
undetectable and for those we have quirk flags. I much prefer that
arrangement.

-- 
balbi

Download attachment "signature.asc" of type "application/pgp-signature" (833 bytes)