| 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
| ||
|
Date: Wed, 5 Feb 2014 09:30:35 -0800
From: Olof Johansson <olof@...om.net>
To: Kamil Debski <k.debski@...sung.com>
Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-samsung-soc@...r.kernel.org"
<linux-samsung-soc@...r.kernel.org>,
"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
Kyungmin Park <kyungmin.park@...sung.com>,
Kishon Vijay Abraham I <kishon@...com>,
Tomasz Figa <t.figa@...sung.com>,
Sylwester Nawrocki <s.nawrocki@...sung.com>,
Marek Szyprowski <m.szyprowski@...sung.com>,
Vivek Gautam <gautam.vivek@...sung.com>,
Mateusz Krawczuk <mat.krawczuk@...il.com>,
yulgon.kim@...sung.com, Praveen Paneri <p.paneri@...sung.com>,
av.tikhomirov@...sung.com, Jingoo Han <jg1.han@...sung.com>,
Kumar Gala <galak@...eaurora.org>,
Matt Porter <matt.porter@...aro.org>,
tjakobi@...h.uni-bielefeld.de,
Alan Stern <stern@...land.harvard.edu>
Subject: Re: [PATCH v6 8/8] usb: ehci-exynos: Change to use phy provided by
the generic phy framework
On Wed, Feb 5, 2014 at 7:57 AM, Kamil Debski <k.debski@...sung.com> wrote:
> Hi Olof,
>
> Thank you for your review.
>
>> From: Olof Johansson [mailto:olof@...om.net]
>> Sent: Wednesday, January 29, 2014 9:55 PM
>>
>> Hi,
>>
>> On Wed, Jan 29, 2014 at 9:29 AM, Kamil Debski <k.debski@...sung.com>
>> wrote:
>> > Change the phy provider used from the old one using the USB phy
>> > framework to a new one using the Generic phy framework.
>> >
>> > Signed-off-by: Kamil Debski <k.debski@...sung.com>
>> > ---
>> > .../devicetree/bindings/usb/exynos-usb.txt | 13 +++
>> > drivers/usb/host/ehci-exynos.c | 97
>> +++++++++++++-------
>> > 2 files changed, 76 insertions(+), 34 deletions(-)
>> >
>> > diff --git a/Documentation/devicetree/bindings/usb/exynos-usb.txt
>> > b/Documentation/devicetree/bindings/usb/exynos-usb.txt
>> > index d967ba1..25e199a 100644
>> > --- a/Documentation/devicetree/bindings/usb/exynos-usb.txt
>> > +++ b/Documentation/devicetree/bindings/usb/exynos-usb.txt
>> > @@ -12,6 +12,10 @@ Required properties:
>> > - interrupts: interrupt number to the cpu.
>> > - clocks: from common clock binding: handle to usb clock.
>> > - clock-names: from common clock binding: Shall be "usbhost".
>> > + - port: if in the SoC there are EHCI phys, they should be listed
>> here.
>> > +One phy per port. Each port should have its reg entry with a
>> > +consecutive number. Also it should contain phys and phy-names
>> entries
>> > +specifying the phy used by the port.
>> >
>> > Optional properties:
>> > - samsung,vbus-gpio: if present, specifies the GPIO that @@ -27,6
>> > +31,15 @@ Example:
>> >
>> > clocks = <&clock 285>;
>> > clock-names = "usbhost";
>> > +
>> > + #address-cells = <1>;
>> > + #size-cells = <0>;
>> > + port@0 {
>> > + reg = <0>;
>> > + phys = <&usb2phy 1>;
>> > + phy-names = "host";
>> > + status = "disabled";
>> > + };
>> > };
>> >
>> > OHCI
>>
>> [...]
>>
>> > @@ -102,14 +132,26 @@ static int exynos_ehci_probe(struct
>> platform_device *pdev)
>> > "samsung,exynos5440-ehci"))
>> > goto skip_phy;
>> >
>> > - phy = devm_usb_get_phy(&pdev->dev, USB_PHY_TYPE_USB2);
>> > - if (IS_ERR(phy)) {
>> > - usb_put_hcd(hcd);
>> > - dev_warn(&pdev->dev, "no platform data or transceiver
>> defined\n");
>> > - return -EPROBE_DEFER;
>> > - } else {
>> > - exynos_ehci->phy = phy;
>> > - exynos_ehci->otg = phy->otg;
>> > + for_each_available_child_of_node(pdev->dev.of_node, child) {
>> > + err = of_property_read_u32(child, "reg",
>> &phy_number);
>> > + if (err) {
>> > + dev_err(&pdev->dev, "Failed to parse device
>> tree\n");
>> > + of_node_put(child);
>> > + return err;
>> > + }
>> > + if (phy_number >= PHY_NUMBER) {
>> > + dev_err(&pdev->dev, "Failed to parse device
>> tree - number out of range\n");
>> > + of_node_put(child);
>> > + return -EINVAL;
>> > + }
>> > + phy = devm_of_phy_get(&pdev->dev, child, 0);
>> > + of_node_put(child);
>> > + if (IS_ERR(phy)) {
>> > + dev_err(&pdev->dev, "Failed to get phy number
>> %d",
>> > +
>> phy_number);
>> > + return PTR_ERR(phy);
>> > + }
>> > + exynos_ehci->phy[phy_number] = phy;
>>
>> this looks like it is now breaking older device trees, where ports
>> might not be described. Since device tree interfaces need to be
>> backwards compatible, you still need to handle the old case of not
>> having ports described.
>>
>> There are two ways of doing this:
>>
>> 1. Fall back to the old behavior if there are no ports 2. Use a new
>> compatible value for the new model with port subnodes, and if the old
>> compatible value is used, then fall back to the old behavior.
>>
>> I'm guessing (1) might be easiest since you can check for the presence
>> of #address-cells to tell if this is just an old style node, or if it's
>> a new-style node without any ports below it.
>
> The ultimate goal is to remove the old phy driver. Unfortunately
> this has to be synced with the new USB3 phy driver by Vivek Gautam. I think
> he
> is also close to completion. What about this case? In the end the old driver
> will be removed and no longer be supported. Having backward compatibility in
> mind, it is possible to have the old and the new phy driver together in one
> kernel release. But do we want to have two drivers doing the same thing at
> the same time?
It is mostly irrelevant if there is a new driver or not -- the old
device tree has to keep working. In this case it would mean that the
new driver needs to work with older device trees as well, or people
will see functionality regressing.
The device tree is a description of the hardware, not an extension of
the driver.
-Olof
--
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/
Powered by blists - more mailing lists