linux-kernel - Re: [PATCH 0/5] Add phy support for AM335X platform using Generic PHy framework

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:	Mon, 08 Jul 2013 21:44:33 +0200
From:	Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To:	George Cherian <george.cherian@...com>
CC:	linux-usb@...r.kernel.org, balbi@...com,
	linux-kernel@...r.kernel.org, gregkh@...uxfoundation.org,
	linux-omap@...r.kernel.org, kishon@...com
Subject: Re: [PATCH 0/5] Add phy support for AM335X platform using Generic
 PHy framework

On 07/08/2013 12:43 PM, George Cherian wrote:
> This patch series adds phy support for AM335X platform.
> This patch series is based on Generic PHY framework [1]. 
> 
> 
> This series has
> 	- adds dual musb instances support for am335x platform (just for testing)
> 	- adds phy-amxxxx-usb driver used in AMxxxx platforms
> 	- adds dt  bindings for the phys
> 	- removes usb-phy and replaced with generic phy apis in glue layer

No, I don't like this all. You did the one thing I tried to avoid while
posting my quick-and-dirty phy driver recently: You duplicated a lot of
code which can be served by the nop driver and added only power
on/power off callbacks.
In numbers:
>  7 files changed, 316 insertions(+), 70 deletions(-)
vs
   2 files changed, 117 insertions, 12 deletions

I assumed you had also the OTG callbacks (set host/device mode) and
wake up but I don't see it there.
Adding a power regulator would do the same job, wouldn't it? If the phy
driver remains just doing power on/off I suggest simply adding a power
regulator. If it will do more I would move the am35xx specific bits
into a separate file and glue it to the nop driver.

What else? The abstraction in device tree is wrong. It remains wrong if
add stuff on top to it.
We need two nodes each one with a glue layer and a musb child node. The
instances crap in kernel has to vanish. Also that means your phy nodes
are wrong. This is not musb with two ports but two musb instances each
with one port.

Sebastian
--
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/