[PATCH for-4.11 1/2] Revert "phy: Add USB3 PHY support for Broadcom NSP SoC"

[Jon Mason]

On Thu, Feb 9, 2017 at 2:18 PM, Florian Fainelli <f.fainelli at gmail.com> wrote:
> On 02/08/2017 11:21 PM, Rafał Miłecki wrote:
>> On 2017-02-09 00:44, Florian Fainelli wrote:
>>> On 02/08/2017 03:39 PM, Rafał Miłecki wrote:
>>>> On 2017-02-09 00:32, Florian Fainelli wrote:
>>>>> On 02/08/2017 03:30 PM, Rafał Miłecki wrote:
>>>>>> From: Rafał Miłecki <rafal at milecki.pl>
>>>>>>
>>>>>> This reverts commit d7bc1a7d41bf ("phy: Add USB3 PHY support for
>>>>>> Broadcom NSP SoC") as we already have driver for this PHY (shared
>>>>>> by NS
>>>>>> and NSP). It was added in commit e5666281d9ea ("phy: bcm-ns-usb3: new
>>>>>> driver for USB 3.0 PHY on Northstar").
>>>>>>
>>>>>> Instead of adding separated driver & duplicating code we should
>>>>>> work on
>>>>>> improving existing (old) one. Thanks to work done by Broadcom we know
>>>>>> there is MDIO bus we weren't aware of & we know register names which
>>>>>> makes initialization more clear. This is very valuable info and we
>>>>>> should work on using it in existing driver afterwards.
>>>>>
>>>>> Should not we first extend the old driver to support NSP and then
>>>>> revert
>>>>> d7bc1a7d41bf ("phy: Add USB3 PHY support for Broadcom NSP SoC")?
>>>>
>>>> Sounds like a weird / dirty development method to me: adding duplicated
>>>> code
>>>> first then working on cleaning it. Unless you mean drivers/staging/.
>>>
>>> There was clearly a mistake in submitting this NSP USB PHY driver, and
>>> it should have been a patch against the existing NS USB PHY driver, but
>>> it was not, okay fair enough.
>>>
>>> It's one thing to address that in the future, and it's another thing to
>>> flat out revert the driver just because you don't like the duplication.
>>>
>>> I don't like that either, and we can discuss on how to improve things
>>> (like have the maintainer review that too), but duplication is a lesser
>>> evil than not having the hardware supported at all, and even more so,
>>> purposely reverting in the name of removing that duplication, that's
>>> intentionally breaking working hardware!
>>
>> Hardware support is not excuse and I don't think it ever was in the Linux.
>>
>> We don't accept badly designed drivers just because they provide new hw
>> support.
>> We have various standards (for quality, style, design, code) at kernel
>> and we
>> stick to them unless it's drivers/staging/. As you said this driver
>> shouldn't be
>> pushed in the first place.
>>
>> Dropping hardware support in kernel happens. Sometimes it's about ancient
>> devices, sometimes about code quality (some forgotten staging drivers
>> used to be
>> dropped AFAIK).
>>
>> Additionally you're talking about support that was *just* added and
>> isn't used
>> by anyone in the wild world yet.
>
> Except people working on it at Broadcom, but fair enough.
>
>>
>> This hardware was missing upstream support for 4 years so 2 extra months
>> won't
>> really hurt anyone.
>>
>> I really don't see excusee or need for keeping this driver.
>>
>> If you want to (and you feel it's well designed), we can keep
>> brcm,nsp-usb3-phy.txt
>
> No it's fine, let's drop it all and replace it with whatever you and Jon
> come up with next.
>
>>
>> I vote for focusing on existing driver improvements instead of looking for
>> excuses for keeping driver that shouldn't be added in the first place.
>> Jon seems to be already working on this, I'm willing to help him, I'm
>> sure we
>> can get you a proper support for the next merge window.
>
> Fair enough, I dropped Dhanajay's changes ("ARM: dts: NSP: Add USB nodes
> for Northstar Plus") from devicetree/next so you and Jon can figure out
> what is the best thing to move forward and we minimize the amount of
> incompatible DT stuff to be sorted out later on. So as far as I am
> concerned, there are no board/SoC DTS changes to be patched later on, we
> could re-apply this patch as-is, or we could have to define a new binding.
> --
> Florian

(previous email got caught in HTML filters)

Per the discussion with Rafal, this is acceptable

Acked-by: Jon Mason <jon.mason at broadcom.com>