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

[Rafał Miłecki]

On 9 March 2017 at 09:11, Kishon Vijay Abraham I <kishon at ti.com> wrote:
>
>
> On Wednesday 08 March 2017 05:56 PM, Kishon Vijay Abraham I wrote:
>> Hi Rafal,
>>
>> On Wednesday 08 March 2017 05:43 PM, Rafał Miłecki wrote:
>>> On 10 February 2017 at 01:27, Jon Mason <jon.mason at broadcom.com> wrote:
>>>> 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.
>>>>
>>>> Per the discussion with Rafal, this is acceptable
>>>>
>>>> Acked-by: Jon Mason <jon.mason at broadcom.com>
>>>
>>> Hi Kishon, what's the status of this?
>>
>> Will be merging this in a day or so.
>
> merged, thanks.

Thank you!

-- 
Rafał