linux-kernel - Re: [PATCH v3 1/3] phy: Add new Exynos USB PHY driver

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:	Wed, 6 Nov 2013 18:20:36 +0530
From:	Kishon Vijay Abraham I <kishon@...com>
To:	Tomasz Figa <t.figa@...sung.com>
CC:	Kamil Debski <k.debski@...sung.com>,
	<linux-kernel@...r.kernel.org>,
	<linux-samsung-soc@...r.kernel.org>, <linux-usb@...r.kernel.org>,
	<devicetree@...r.kernel.org>, <linux-arm@...r.kernel.org>,
	<kyungmin.park@...sung.com>, <s.nawrocki@...sung.com>,
	<m.szyprowski@...sung.com>, <gautam.vivek@...sung.com>,
	<mat.krawczuk@...il.com>, <yulgon.kim@...sung.com>,
	<p.paneri@...sung.com>, <av.tikhomirov@...sung.com>,
	<jg1.han@...sung.com>, <galak@...eaurora.org>
Subject: Re: [PATCH v3 1/3] phy: Add new Exynos USB PHY driver

Hi,

On Wednesday 06 November 2013 05:08 PM, Tomasz Figa wrote:
> Hi Kishon
>
> On Wednesday 06 of November 2013 13:48:13 Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Tuesday 05 November 2013 09:43 PM, Kamil Debski wrote:
>>> Add a new driver for the Exynos USB PHY. The new driver uses the generic
>>> PHY framework. The driver includes support for the Exynos 4x10 and 4x12
>>> SoC families.
>>>
>>> Signed-off-by: Kamil Debski <k.debski@...sung.com>
>>> Signed-off-by: Kyungmin Park <kyungmin.park@...sung.com>
>>> ---
>>>    .../devicetree/bindings/phy/samsung-usbphy.txt     |   52 ++++
>>>    drivers/phy/Kconfig                                |   23 +-
>>>    drivers/phy/Makefile                               |    4 +
>>>    drivers/phy/phy-exynos-usb2.c                      |  234 ++++++++++++++
>>>    drivers/phy/phy-exynos-usb2.h                      |   87 ++++++
>>>    drivers/phy/phy-exynos4210-usb2.c                  |  272 ++++++++++++++++
>>>    drivers/phy/phy-exynos4212-usb2.c                  |  324 ++++++++++++++++++++
>>>    7 files changed, 995 insertions(+), 1 deletion(-)
>>>    create mode 100644 Documentation/devicetree/bindings/phy/samsung-usbphy.txt
>>>    create mode 100644 drivers/phy/phy-exynos-usb2.c
>>>    create mode 100644 drivers/phy/phy-exynos-usb2.h
>>>    create mode 100644 drivers/phy/phy-exynos4210-usb2.c
>>>    create mode 100644 drivers/phy/phy-exynos4212-usb2.c
> [snip]
>>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>>> index a344f3d..bdf0fab 100644
>>> --- a/drivers/phy/Kconfig
>>> +++ b/drivers/phy/Kconfig
> [snip]
>>> @@ -51,4 +51,25 @@ config PHY_EXYNOS_DP_VIDEO
>>>    	help
>>>    	  Support for Display Port PHY found on Samsung EXYNOS SoCs.
>>>
>>> +config PHY_EXYNOS_USB2
>>> +	tristate "Samsung USB 2.0 PHY driver"
>>> +	help
>>> +	  Enable this to support Samsung USB phy helper driver for Samsung SoCs.
>>> +	  This driver provides common interface to interact, for Samsung
>>> +	  USB 2.0 PHY driver.
>>
>> I still think we can get rid of this helper driver and have a single
>> driver for both PHY_EXYNOS4210_USB2 and PHY_EXYNOS4212_USB2.
>
> This helper driver is a really nice way to avoid code duplication, while
> still leaving the code clean and readable.
>
> All the Samsung USB 2.0 PHYs require exactly the same semantics
> (isolation, reference rate configuration, power up, power on), but each
> one has completely different layout of registers and bits inside
> registers.

I just did a diff of registers in exynos 4210 and 4212 PHY drivers [1] 
and couldn't find that big a difference in register layout. Of course 
there are a few changes in HSIC bit fields and PHYFSEL but that's only 
minimal and could well be handled in a single driver.

[1] -> http://diffchecker.com/py3tp68m
>
> Making a big single driver would end up being identical to the old Exynos
> USB2PHY driver with a lot of if and switch statements inside most of
> functions, which is not only ugly but makes any further extension hard.

Probably we shouldn't try to over design and just convert the old exynos 
usb2 phy driver to the generic phy framework to begin with?

Thanks
Kishon
--
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/