linux-kernel - Re: [PATCH] phy: bcm-ns-usb2: new driver for USB 2.0 PHY on Northstar

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

Date:	Sun, 10 Apr 2016 19:44:46 +0200
From:	Rafał Miłecki <zajec5@...il.com>
To:	Jon Mason <jon.mason@...adcom.com>
Cc:	Kishon Vijay Abraham I <kishon@...com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Hauke Mehrtens <hauke@...ke-m.de>,
	Felix Fietkau <nbd@...nwrt.org>,
	Florian Fainelli <f.fainelli@...il.com>,
	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
	">" <bcm-kernel-feedback-list@...adcom.com>
Subject: Re: [PATCH] phy: bcm-ns-usb2: new driver for USB 2.0 PHY on Northstar

On 5 April 2016 at 21:34, Jon Mason <jon.mason@...adcom.com> wrote:
> On Wed, Mar 30, 2016 at 5:32 PM, Jon Mason <jon.mason@...adcom.com> wrote:
>> On Tue, Mar 29, 2016 at 10:01 AM, Rafał Miłecki <zajec5@...il.com> wrote:
>>> Northstar is a family of SoCs used in home routers. They have USB 2.0
>>> and 3.0 controllers with PHYs that need to be properly initialized.
>>> This driver provides PHY init support in a generic way and can be bound
>>> with an EHCI controller driver.
>>
>> Like the USB3 patch you just submitted for NS, this is a common IP block
>> with NSP.  I believe with some minor changes it can support both.  Please
>> allow me 1-2 days to look at these in more detail and see if I can get these
>> patches working on NSP.
>>
>
> After some internal discussion, I don't think this is going to work.  This
> IP block is common for NS, NSP, and a few others.  So binding it to BMCA is
> going to prevent us from being able to use it on any other platforms.
> However, a non-BMCA driver would still be usable by NS.  So, I think that is
> a superior solution.
>
> We are currently in the process of getting a Phy driver out which would
> cover all the iProc SoCs.  I think it is 1-2 weeks away from being
> submitted.  So, I think to go forward we should use that one for NS.
> However, that does not bridge the gap until it is accepted.
>
> So, I think we have 2 options.
> 1.  Wait for BCM to submit the iProc phy driver
> 2.  Push this now, and remove it after the iProc phy driver is accepted.
>
> Thoughts?

As Hauke noticed, there isn't any real bcma dependency in the
submitted driver. I put (very few) register defines in
include/linux/bcma/ just to make them re-usable e.g. in PCIe
controller/PHY driver. I think at some point we may also need some CRU
regs in clock driver for BCM53573 chipset, so some common place is
needed to avoid code duplication.

That said, I'm sorry but I'm uncomfortable with your idea of this PHY
driver development. I'm open to comments & suggestions. You can freely
point parts of code you think are badly designed. I'll try to improve
them.
I don't like that idea of dropping my driver just to replace it with
the one developed at BCM doing the same.

Could you review it once again and see if you can change your mind, please?