From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kishon Vijay Abraham I Subject: Re: [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB Date: Mon, 19 Sep 2016 10:29:27 +0530 Message-ID: <57DF70AF.4050002@ti.com> References: <20160904213152.25837-1-martin.blumenstingl@googlemail.com> <20160904213152.25837-5-martin.blumenstingl@googlemail.com> <57DBAB2F.3040905@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Sender: linux-usb-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Martin Blumenstingl , arnd-r2nGTMty4D4@public.gmane.org Cc: Kevin Hilman , Ben Dooks , mark.rutland-5wv7dgnIgG8@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org, johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org, will.deacon-5wv7dgnIgG8@public.gmane.org, mturquette-rdvid1DuHRBWk0Htik3J/w@public.gmane.org, linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org, robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, catalin.marinas-5wv7dgnIgG8@public.gmane.org, carlo-KA+7E9HrN00dnm+yROfE0A@public.gmane.org, linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, linux-clk-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, jbrunet-rdvid1DuHRBWk0Htik3J/w@public.gmane.org List-Id: devicetree@vger.kernel.org Hi, On Monday 19 September 2016 01:26 AM, Martin Blumenstingl wrote: > Hi Kishon, > > On Fri, Sep 16, 2016 at 10:19 AM, Kishon Vijay Abraham I wrote: >> Hi, >> >> On Friday 09 September 2016 09:44 PM, Martin Blumenstingl wrote: >>> On Fri, Sep 9, 2016 at 5:33 PM, Kevin Hilman wrote: >>>> However, the problem with all of the solutions proposed (runtime PM ones >>>> included) is that we're forcing a board-specific design issue (2 devices >>>> sharing a reset line) into a driver that should not have any >>>> board-specific assumptions in it. >>>> >>>> For example, if this driver is used on another platform where different >>>> PHYs have different reset lines, then one of them (the unlucky one who >>>> is not probed first) will never get reset. So any form of per-device >>>> ref-counting is not a portable solution. >>> maybe we should also consider Ben's solution: he played with the USB >>> PHY on his Meson8b board. His approach was to have only one USB PHY >>> driver instance which exposes two PHYs. >>> The downside of this: the driver would have to know the offset of the >>> PHYs (0x0 for the first PHY, 0x20 for the second), but we could handle >>> the reset using runtime PM without any hacks. >> >> I think the offset information can come from the devicetree too. The phy can be >> modeled something like below. >> >> usb-phys@c0000000 { >> compatible = "amlogic,meson-gxbb-usb2-phy"; >> reg = <0x0 0xc0000000 0x0 0x40>; >> #address-cells = <2>; >> #size-cells = <2>; >> ranges = <0x0 0x0 0x0 0xc0000000 0x0 0x40>; >> resets = <&reset 34>; >> >> usb0_phy: usb_phy@0 { >> #phy-cells = <0>; >> reg = <0x0 0x0 0x0 0x20>; >> clocks = <&clkc CLKID_USB &clkc CLKID_USB0>; >> clock-names = "usb_general", "usb"; >> status = "disabled"; >> }; >> >> usb1_phy: usb_phy@20 { >> #phy-cells = <0>; >> reg = <0x0 0x20 0x0 0x20>; >> clocks = <&clkc CLKID_USB &clkc CLKID_USB1>; >> clock-names = "usb_general", "usb"; >> status = "disabled"; >> }; >> }; >> >> This way the driver will be probed only once (the reset can be done during >> probe). The phy driver should scan the dt node and for every sub-node it >> invokes phy_create? > I'll recap what we have discussed so far (so you don't have to re-read > the whole thread): > The reference driver treats both USB PHYs as separate devices (the > datasheet has no information about the PHYs though). The only > "special" thing is the shared reset line -> together with Philipp > Zabel (the reset framework maintainer) we decided to make > reset_control_reset work for shared reset lines. > > That means we can keep the two PHYs as separate devices inside the > .dts, while keeping everything else separate (just like the reference > driver) > Is this fine for you and Arnd? yeah.. I'm fine with that. Thanks Kishon > > > Regards, > Martin > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Subject: Re: [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB To: Martin Blumenstingl , References: <20160904213152.25837-1-martin.blumenstingl@googlemail.com> <20160904213152.25837-5-martin.blumenstingl@googlemail.com> <57DBAB2F.3040905@ti.com> CC: Kevin Hilman , Ben Dooks , , , , , , , , , , , , , , , From: Kishon Vijay Abraham I Message-ID: <57DF70AF.4050002@ti.com> Date: Mon, 19 Sep 2016 10:29:27 +0530 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" List-ID: Hi, On Monday 19 September 2016 01:26 AM, Martin Blumenstingl wrote: > Hi Kishon, > > On Fri, Sep 16, 2016 at 10:19 AM, Kishon Vijay Abraham I wrote: >> Hi, >> >> On Friday 09 September 2016 09:44 PM, Martin Blumenstingl wrote: >>> On Fri, Sep 9, 2016 at 5:33 PM, Kevin Hilman wrote: >>>> However, the problem with all of the solutions proposed (runtime PM ones >>>> included) is that we're forcing a board-specific design issue (2 devices >>>> sharing a reset line) into a driver that should not have any >>>> board-specific assumptions in it. >>>> >>>> For example, if this driver is used on another platform where different >>>> PHYs have different reset lines, then one of them (the unlucky one who >>>> is not probed first) will never get reset. So any form of per-device >>>> ref-counting is not a portable solution. >>> maybe we should also consider Ben's solution: he played with the USB >>> PHY on his Meson8b board. His approach was to have only one USB PHY >>> driver instance which exposes two PHYs. >>> The downside of this: the driver would have to know the offset of the >>> PHYs (0x0 for the first PHY, 0x20 for the second), but we could handle >>> the reset using runtime PM without any hacks. >> >> I think the offset information can come from the devicetree too. The phy can be >> modeled something like below. >> >> usb-phys@c0000000 { >> compatible = "amlogic,meson-gxbb-usb2-phy"; >> reg = <0x0 0xc0000000 0x0 0x40>; >> #address-cells = <2>; >> #size-cells = <2>; >> ranges = <0x0 0x0 0x0 0xc0000000 0x0 0x40>; >> resets = <&reset 34>; >> >> usb0_phy: usb_phy@0 { >> #phy-cells = <0>; >> reg = <0x0 0x0 0x0 0x20>; >> clocks = <&clkc CLKID_USB &clkc CLKID_USB0>; >> clock-names = "usb_general", "usb"; >> status = "disabled"; >> }; >> >> usb1_phy: usb_phy@20 { >> #phy-cells = <0>; >> reg = <0x0 0x20 0x0 0x20>; >> clocks = <&clkc CLKID_USB &clkc CLKID_USB1>; >> clock-names = "usb_general", "usb"; >> status = "disabled"; >> }; >> }; >> >> This way the driver will be probed only once (the reset can be done during >> probe). The phy driver should scan the dt node and for every sub-node it >> invokes phy_create? > I'll recap what we have discussed so far (so you don't have to re-read > the whole thread): > The reference driver treats both USB PHYs as separate devices (the > datasheet has no information about the PHYs though). The only > "special" thing is the shared reset line -> together with Philipp > Zabel (the reset framework maintainer) we decided to make > reset_control_reset work for shared reset lines. > > That means we can keep the two PHYs as separate devices inside the > .dts, while keeping everything else separate (just like the reference > driver) > Is this fine for you and Arnd? yeah.. I'm fine with that. Thanks Kishon > > > Regards, > Martin > From mboxrd@z Thu Jan 1 00:00:00 1970 From: kishon@ti.com (Kishon Vijay Abraham I) Date: Mon, 19 Sep 2016 10:29:27 +0530 Subject: [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB In-Reply-To: References: <20160904213152.25837-1-martin.blumenstingl@googlemail.com> <20160904213152.25837-5-martin.blumenstingl@googlemail.com> <57DBAB2F.3040905@ti.com> Message-ID: <57DF70AF.4050002@ti.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi, On Monday 19 September 2016 01:26 AM, Martin Blumenstingl wrote: > Hi Kishon, > > On Fri, Sep 16, 2016 at 10:19 AM, Kishon Vijay Abraham I wrote: >> Hi, >> >> On Friday 09 September 2016 09:44 PM, Martin Blumenstingl wrote: >>> On Fri, Sep 9, 2016 at 5:33 PM, Kevin Hilman wrote: >>>> However, the problem with all of the solutions proposed (runtime PM ones >>>> included) is that we're forcing a board-specific design issue (2 devices >>>> sharing a reset line) into a driver that should not have any >>>> board-specific assumptions in it. >>>> >>>> For example, if this driver is used on another platform where different >>>> PHYs have different reset lines, then one of them (the unlucky one who >>>> is not probed first) will never get reset. So any form of per-device >>>> ref-counting is not a portable solution. >>> maybe we should also consider Ben's solution: he played with the USB >>> PHY on his Meson8b board. His approach was to have only one USB PHY >>> driver instance which exposes two PHYs. >>> The downside of this: the driver would have to know the offset of the >>> PHYs (0x0 for the first PHY, 0x20 for the second), but we could handle >>> the reset using runtime PM without any hacks. >> >> I think the offset information can come from the devicetree too. The phy can be >> modeled something like below. >> >> usb-phys at c0000000 { >> compatible = "amlogic,meson-gxbb-usb2-phy"; >> reg = <0x0 0xc0000000 0x0 0x40>; >> #address-cells = <2>; >> #size-cells = <2>; >> ranges = <0x0 0x0 0x0 0xc0000000 0x0 0x40>; >> resets = <&reset 34>; >> >> usb0_phy: usb_phy at 0 { >> #phy-cells = <0>; >> reg = <0x0 0x0 0x0 0x20>; >> clocks = <&clkc CLKID_USB &clkc CLKID_USB0>; >> clock-names = "usb_general", "usb"; >> status = "disabled"; >> }; >> >> usb1_phy: usb_phy at 20 { >> #phy-cells = <0>; >> reg = <0x0 0x20 0x0 0x20>; >> clocks = <&clkc CLKID_USB &clkc CLKID_USB1>; >> clock-names = "usb_general", "usb"; >> status = "disabled"; >> }; >> }; >> >> This way the driver will be probed only once (the reset can be done during >> probe). The phy driver should scan the dt node and for every sub-node it >> invokes phy_create? > I'll recap what we have discussed so far (so you don't have to re-read > the whole thread): > The reference driver treats both USB PHYs as separate devices (the > datasheet has no information about the PHYs though). The only > "special" thing is the shared reset line -> together with Philipp > Zabel (the reset framework maintainer) we decided to make > reset_control_reset work for shared reset lines. > > That means we can keep the two PHYs as separate devices inside the > .dts, while keeping everything else separate (just like the reference > driver) > Is this fine for you and Arnd? yeah.. I'm fine with that. Thanks Kishon > > > Regards, > Martin > From mboxrd@z Thu Jan 1 00:00:00 1970 From: kishon@ti.com (Kishon Vijay Abraham I) Date: Mon, 19 Sep 2016 10:29:27 +0530 Subject: [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB In-Reply-To: References: <20160904213152.25837-1-martin.blumenstingl@googlemail.com> <20160904213152.25837-5-martin.blumenstingl@googlemail.com> <57DBAB2F.3040905@ti.com> Message-ID: <57DF70AF.4050002@ti.com> To: linus-amlogic@lists.infradead.org List-Id: linus-amlogic.lists.infradead.org Hi, On Monday 19 September 2016 01:26 AM, Martin Blumenstingl wrote: > Hi Kishon, > > On Fri, Sep 16, 2016 at 10:19 AM, Kishon Vijay Abraham I wrote: >> Hi, >> >> On Friday 09 September 2016 09:44 PM, Martin Blumenstingl wrote: >>> On Fri, Sep 9, 2016 at 5:33 PM, Kevin Hilman wrote: >>>> However, the problem with all of the solutions proposed (runtime PM ones >>>> included) is that we're forcing a board-specific design issue (2 devices >>>> sharing a reset line) into a driver that should not have any >>>> board-specific assumptions in it. >>>> >>>> For example, if this driver is used on another platform where different >>>> PHYs have different reset lines, then one of them (the unlucky one who >>>> is not probed first) will never get reset. So any form of per-device >>>> ref-counting is not a portable solution. >>> maybe we should also consider Ben's solution: he played with the USB >>> PHY on his Meson8b board. His approach was to have only one USB PHY >>> driver instance which exposes two PHYs. >>> The downside of this: the driver would have to know the offset of the >>> PHYs (0x0 for the first PHY, 0x20 for the second), but we could handle >>> the reset using runtime PM without any hacks. >> >> I think the offset information can come from the devicetree too. The phy can be >> modeled something like below. >> >> usb-phys at c0000000 { >> compatible = "amlogic,meson-gxbb-usb2-phy"; >> reg = <0x0 0xc0000000 0x0 0x40>; >> #address-cells = <2>; >> #size-cells = <2>; >> ranges = <0x0 0x0 0x0 0xc0000000 0x0 0x40>; >> resets = <&reset 34>; >> >> usb0_phy: usb_phy at 0 { >> #phy-cells = <0>; >> reg = <0x0 0x0 0x0 0x20>; >> clocks = <&clkc CLKID_USB &clkc CLKID_USB0>; >> clock-names = "usb_general", "usb"; >> status = "disabled"; >> }; >> >> usb1_phy: usb_phy at 20 { >> #phy-cells = <0>; >> reg = <0x0 0x20 0x0 0x20>; >> clocks = <&clkc CLKID_USB &clkc CLKID_USB1>; >> clock-names = "usb_general", "usb"; >> status = "disabled"; >> }; >> }; >> >> This way the driver will be probed only once (the reset can be done during >> probe). The phy driver should scan the dt node and for every sub-node it >> invokes phy_create? > I'll recap what we have discussed so far (so you don't have to re-read > the whole thread): > The reference driver treats both USB PHYs as separate devices (the > datasheet has no information about the PHYs though). The only > "special" thing is the shared reset line -> together with Philipp > Zabel (the reset framework maintainer) we decided to make > reset_control_reset work for shared reset lines. > > That means we can keep the two PHYs as separate devices inside the > .dts, while keeping everything else separate (just like the reference > driver) > Is this fine for you and Arnd? yeah.. I'm fine with that. Thanks Kishon > > > Regards, > Martin >