Re: [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB

[Arnd Bergmann]

On Monday, September 19, 2016 10:29:27 AM CEST Kishon Vijay Abraham I wrote:
> On Monday 19 September 2016 01:26 AM, Martin Blumenstingl wrote:
> > On Fri, Sep 16, 2016 at 10:19 AM, Kishon Vijay Abraham I  
> > wrote:

> >> 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.
> 

Agreed, if we don't need to have a device modeled around both
instance, that's ideal.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB

[Kishon Vijay Abraham I]

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@c000 {
>> compatible = "amlogic,meson-gxbb-usb2-phy";
>> reg = <0x0 0xc000 0x0 0x40>;
>> #address-cells = <2>;
>> #size-cells = <2>;
>> ranges = <0x0 0x0 0x0 0xc000 0x0 0x40>;
>> resets = < 34>;
>>
>> usb0_phy: usb_phy@0 {
>> #phy-cells = <0>;
>> reg = <0x0 0x0 0x0 0x20>;
>> clocks = < CLKID_USB  CLKID_USB0>;
>> clock-names = "usb_general", "usb";
>> status = "disabled";
>> };
>>
>> usb1_phy: usb_phy@20 {
>> #phy-cells = <0>;
>> reg = <0x0 0x20 0x0 0x20>;
>> clocks = < CLKID_USB  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB

[Martin Blumenstingl]

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@c000 {
> compatible = "amlogic,meson-gxbb-usb2-phy";
> reg = <0x0 0xc000 0x0 0x40>;
> #address-cells = <2>;
> #size-cells = <2>;
> ranges = <0x0 0x0 0x0 0xc000 0x0 0x40>;
> resets = < 34>;
>
> usb0_phy: usb_phy@0 {
> #phy-cells = <0>;
> reg = <0x0 0x0 0x0 0x20>;
> clocks = < CLKID_USB  CLKID_USB0>;
> clock-names = "usb_general", "usb";
> status = "disabled";
> };
>
> usb1_phy: usb_phy@20 {
> #phy-cells = <0>;
> reg = <0x0 0x20 0x0 0x20>;
> clocks = < CLKID_USB  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?


Regards,
Martin
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB

[Arnd Bergmann]

On Friday, September 16, 2016 1:49:59 PM CEST Kishon Vijay Abraham I wrote:
> 
> I think the offset information can come from the devicetree too. The phy can 
> be
> modeled something like below.
> 
> usb-phys@c000 {
> compatible = "amlogic,meson-gxbb-usb2-phy";
> reg = <0x0 0xc000 0x0 0x40>;
> #address-cells = <2>;
> #size-cells = <2>;
> ranges = <0x0 0x0 0x0 0xc000 0x0 0x40>;
> resets = < 34>;
> 
> usb0_phy: usb_phy@0 {
> #phy-cells = <0>;
> reg = <0x0 0x0 0x0 0x20>;
> clocks = < CLKID_USB  CLKID_USB0>;
> clock-names = "usb_general", "usb";
> status = "disabled";
> };
> 
> usb1_phy: usb_phy@20 {
> #phy-cells = <0>;
> reg = <0x0 0x20 0x0 0x20>;
> clocks = < CLKID_USB  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?

Why not just use #phy-cells=<1> and pass the phy number as an argument
in the reference?

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB

[Kishon Vijay Abraham I]

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@c000 {
compatible = "amlogic,meson-gxbb-usb2-phy";
reg = <0x0 0xc000 0x0 0x40>;
#address-cells = <2>;
#size-cells = <2>;
ranges = <0x0 0x0 0x0 0xc000 0x0 0x40>;
resets = < 34>;

usb0_phy: usb_phy@0 {
#phy-cells = <0>;
reg = <0x0 0x0 0x0 0x20>;
clocks = < CLKID_USB  CLKID_USB0>;
clock-names = "usb_general", "usb";
status = "disabled";
};

usb1_phy: usb_phy@20 {
#phy-cells = <0>;
reg = <0x0 0x20 0x0 0x20>;
clocks = < CLKID_USB  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?

Thanks
Kishon
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB

[Philipp Zabel]

Hi Martin,

Am Mittwoch, den 14.09.2016, 23:23 +0200 schrieb Martin Blumenstingl:
[...]
> > We could add a triggered flag or a counter to struct reset_control, and
> > have reset_control_reset_once do nothing if it is already set /
> > incremented. Since the reset_control goes away with the last consumer,
> > the shared reset line would get triggered again after unbinding both PHY
> > devices.
> I guess that'd do the trick for us:
> - we could use devm_reset_control_get_optional_shared() during probe
> - power_on would then call reset_control_reset()
> - the code in reset_control_reset would be changed: the if
> (WARN_ON(rstc->shared)) would be removed. then we return 0 if
> (rstc->shared && atomic_read(>shared_triggered)). otherwise we
> proceed with the old logic, except that we use
> atomic_set(>shared_triggerred, 1) in case of success (if an
> error was returned we leave it as "not triggered").
> 
> Let me know if you want me to (at least try to) implement that and send an 
> RFC.

Yes, please give it a try. reset_control_reset should still WARN if the
deassert count is set, and reset_(de)assert should do so if triggered is
set. Mixing the two won't work. And it should be documented that shared
reset_control_reset may do nothing if the reset was already triggered by
another consumer.

regards
Philipp

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB

[Martin Blumenstingl]

On Wed, Sep 14, 2016 at 10:37 AM, Philipp Zabel  wrote:
> Am Dienstag, den 13.09.2016, 20:38 +0200 schrieb Martin Blumenstingl:
>> Hi Philipp,
>>
>> On Tue, Sep 13, 2016 at 5:28 PM, Philipp Zabel  
>> wrote:
>> > Hi Martin,
>> >
>> > Am Freitag, den 09.09.2016, 22:36 +0200 schrieb Martin Blumenstingl:
>> >> On Fri, Sep 9, 2016 at 5:33 PM, Kevin Hilman  wrote:
>> >> > Martin Blumenstingl  writes:
>> >> >
>> >> >> On Thu, Sep 8, 2016 at 10:53 PM, Ben Dooks  
>> >> >> wrote:
>> >> >>> On 08/09/16 21:42, Kevin Hilman wrote:
>> >> 
>> >>  Ben Dooks  writes:
>> >> 
>> >> > On 08/09/16 20:52, Martin Blumenstingl wrote:
>> >> >>
>> >> >> On Thu, Sep 8, 2016 at 9:35 PM, Kevin Hilman 
>> >> >> wrote:
>> >> 
>> >>  + phy = devm_phy_create(>dev, NULL, 
>> >>  _meson_usb2_ops);
>> >>  + if (IS_ERR(phy)) {
>> >>  + dev_err(>dev, "failed to create PHY\n");
>> >>  + return PTR_ERR(phy);
>> >>  + }
>> >>  +
>> >>  + if (usb_reset_refcnt++ == 0) {
>> >>  + ret = device_reset(>dev);
>> >>  + if (ret) {
>> >>  + dev_err(>dev, "Failed to reset USB 
>> >>  PHY\n");
>> >>  + return ret;
>> >>  + }
>> >>  + }
>> >> >>>
>> >> >>>
>> >> >>> The ref count + reset here looks like something that could/should 
>> >> >>> be
>> >> >>> handled in a runtime PM callback.
>> >> >>
>> >> >> Unfortunately that doesn't work (as Jerome found out) because both
>> >> >> PHYs are sharing the same reset line.
>> >> >> So if the second PHY would call device_reset then it would also 
>> >> >> reset
>> >> >> the first PHY!
>> >> >>
>> >> >> There's a comment above the declaration of usb_reset_refcnt which
>> >> >> tries to explain this:
>> >> >> "The PHYs are sharing a common reset line -> we are only allowed to
>> >> >> reset once for all PHYs."
>> >> >> Maybe I should move this comment to the "if (usb_reset_refcnt++ == 
>> >> >> 0)
>> >> >> {" line to make it easier to see?
>> >> >>
>> >> >
>> >> > pm-runtime has refcounting in it. When one of the nodes turns on,
>> >> > the pm-runtime will call your driver to say there is a user when
>> >> > this first use turns up.
>> >> >
>> >> > If all the sub-phys turn off and drop their refcount then the driver
>> >> > is called to say there are no more users and you can go to sleep.
>> >> 
>> >> 
>> >>  After a chat w/Martin on IRC, It turns out runtime PM wont help here.
>> >> 
>> >>  The reason is because there are physically two PHY devices[1].  
>> >>  Those 2
>> >>  devices will be treated independely by runtime PM, and have separate
>> >>  use-counting, which means doing what I proposed would cause a reset 
>> >>  to
>> >>  happen when either device was probed.
>> >> 
>> >>  So, I think it's OK as it is.
>> >> >>>
>> >> >>>
>> >> >>> Surely you can do pm_runtime_get/put on the phy's parent platform
>> >> >>> device and do it that way?
>> >> >> could you please be more specific with that (do you mean 
>> >> >> pdev->dev.parent)?
>> >> >> so we would use pm_runtime_{get_sync,put} with the parent, while we
>> >> >> would still define the runtime_resume in our driver.
>> >> >
>> >> > You'd also need to do get/put on the children, but yes, that's what Ben
>> >> > is suggesting.
>> >> >
>> >> > 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.
>> >> indeed, so in simple words we would need something like
>> >> reset_control_do_once(rstc, RESET/ASSERT/DEASSERT) which would
>> >> remember internally if any action has already been executed: if not it
>> >> does a _reset, _assert or _deassert and otherwise it does nothing.
>> >>
>> >> > I'm not sure yet how the reset framework is supposed to handle shared
>> >> > reset lines, but that needs some investigation.  I quick glance and it
>> >> > seems that reset controllers can have shared lines, so that should be
>> >> > investigated.
>> >> I added Philipp and Hans to this thread - maybe they can comment on this.
>> >> To sum it up, our problem is:
>> >> - there are 

Re: [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB

[Martin Blumenstingl]

On Wed, Sep 14, 2016 at 10:37 AM, Philipp Zabel  wrote:
> Am Dienstag, den 13.09.2016, 17:59 -0700 schrieb Kevin Hilman:
>> Martin Blumenstingl  writes:
>>
>> > On Tue, Sep 13, 2016 at 5:28 PM, Philipp Zabel  
>> > wrote:
>>
>> [...]
>>
>> >>> I added Philipp and Hans to this thread - maybe they can comment on this.
>> >>> To sum it up, our problem is:
>> >>> - there are two separate USB PHYs on Meson GXBB
>> >>> - both are sharing the same reset line (provided by the reset-meson 
>> >>> driver)
>> >>> - during initialization of the PHYs we must only call
>> >>> reset_control_reset(rstc) once (if we do it for the first *and* second
>> >>> PHY then the first PHY gets confused once the second PHY uses the
>> >>> reset because the first PHY's state is reset as well)
>> >>
>> >> If you have an initially asserted reset line and you can enable the
>> >> first module by deasserting the reset via reset_control_deassert (and
>> >> reset_control_assert to signal when the module may be disabled again
>> >> after use), shared resets are for you.
>> >>
>> >> If you need a reset pulse or have no direct control over the reset line,
>> >> (device_reset), the reset framework currently has no solution for this.
>> >> The ugly thing about reset_control_once would be that it can't re-reset
>> >> modules when unloading and reloading driver modules.
>> >
>> > The corresponding reset driver in question is reset-meson, which only
>> > implements reset (assert/deassert are not implemented). However, I
>> > don't know if this is due to hardware design.
>> > I think the hardware implements the latter, but maybe Neil can give
>> > more information here (I currently don't have access to my board so I
>> > cannot test how the hardware actually behaves).
>>
>> It's implemented that way because the hardware only supports a reset
>> pulse.
>
> Would it be possible to bring down both PHYs drivers, pull the reset
> line once, and then bring the drivers back up again?
I guess that this is the rmmod case: I haven't tested it yet but that
should work (even with the current code and .dts)
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB

[Philipp Zabel]

Am Dienstag, den 13.09.2016, 20:38 +0200 schrieb Martin Blumenstingl:
> Hi Philipp,
> 
> On Tue, Sep 13, 2016 at 5:28 PM, Philipp Zabel  wrote:
> > Hi Martin,
> >
> > Am Freitag, den 09.09.2016, 22:36 +0200 schrieb Martin Blumenstingl:
> >> On Fri, Sep 9, 2016 at 5:33 PM, Kevin Hilman  wrote:
> >> > Martin Blumenstingl  writes:
> >> >
> >> >> On Thu, Sep 8, 2016 at 10:53 PM, Ben Dooks  
> >> >> wrote:
> >> >>> On 08/09/16 21:42, Kevin Hilman wrote:
> >> 
> >>  Ben Dooks  writes:
> >> 
> >> > On 08/09/16 20:52, Martin Blumenstingl wrote:
> >> >>
> >> >> On Thu, Sep 8, 2016 at 9:35 PM, Kevin Hilman 
> >> >> wrote:
> >> 
> >>  + phy = devm_phy_create(>dev, NULL, 
> >>  _meson_usb2_ops);
> >>  + if (IS_ERR(phy)) {
> >>  + dev_err(>dev, "failed to create PHY\n");
> >>  + return PTR_ERR(phy);
> >>  + }
> >>  +
> >>  + if (usb_reset_refcnt++ == 0) {
> >>  + ret = device_reset(>dev);
> >>  + if (ret) {
> >>  + dev_err(>dev, "Failed to reset USB 
> >>  PHY\n");
> >>  + return ret;
> >>  + }
> >>  + }
> >> >>>
> >> >>>
> >> >>> The ref count + reset here looks like something that could/should 
> >> >>> be
> >> >>> handled in a runtime PM callback.
> >> >>
> >> >> Unfortunately that doesn't work (as Jerome found out) because both
> >> >> PHYs are sharing the same reset line.
> >> >> So if the second PHY would call device_reset then it would also 
> >> >> reset
> >> >> the first PHY!
> >> >>
> >> >> There's a comment above the declaration of usb_reset_refcnt which
> >> >> tries to explain this:
> >> >> "The PHYs are sharing a common reset line -> we are only allowed to
> >> >> reset once for all PHYs."
> >> >> Maybe I should move this comment to the "if (usb_reset_refcnt++ == 
> >> >> 0)
> >> >> {" line to make it easier to see?
> >> >>
> >> >
> >> > pm-runtime has refcounting in it. When one of the nodes turns on,
> >> > the pm-runtime will call your driver to say there is a user when
> >> > this first use turns up.
> >> >
> >> > If all the sub-phys turn off and drop their refcount then the driver
> >> > is called to say there are no more users and you can go to sleep.
> >> 
> >> 
> >>  After a chat w/Martin on IRC, It turns out runtime PM wont help here.
> >> 
> >>  The reason is because there are physically two PHY devices[1].  Those 
> >>  2
> >>  devices will be treated independely by runtime PM, and have separate
> >>  use-counting, which means doing what I proposed would cause a reset to
> >>  happen when either device was probed.
> >> 
> >>  So, I think it's OK as it is.
> >> >>>
> >> >>>
> >> >>> Surely you can do pm_runtime_get/put on the phy's parent platform
> >> >>> device and do it that way?
> >> >> could you please be more specific with that (do you mean 
> >> >> pdev->dev.parent)?
> >> >> so we would use pm_runtime_{get_sync,put} with the parent, while we
> >> >> would still define the runtime_resume in our driver.
> >> >
> >> > You'd also need to do get/put on the children, but yes, that's what Ben
> >> > is suggesting.
> >> >
> >> > 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.
> >> indeed, so in simple words we would need something like
> >> reset_control_do_once(rstc, RESET/ASSERT/DEASSERT) which would
> >> remember internally if any action has already been executed: if not it
> >> does a _reset, _assert or _deassert and otherwise it does nothing.
> >>
> >> > I'm not sure yet how the reset framework is supposed to handle shared
> >> > reset lines, but that needs some investigation.  I quick glance and it
> >> > seems that reset controllers can have shared lines, so that should be
> >> > investigated.
> >> I added Philipp and Hans to this thread - maybe they can comment on this.
> >> To sum it up, our problem is:
> >> - there are two separate USB PHYs on Meson GXBB
> >> - both are sharing the same reset line (provided by the reset-meson driver)
> >> - during initialization of the PHYs we must only call
> >> 

Re: [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB

[Philipp Zabel]

Am Dienstag, den 13.09.2016, 17:59 -0700 schrieb Kevin Hilman:
> Martin Blumenstingl  writes:
> 
> > On Tue, Sep 13, 2016 at 5:28 PM, Philipp Zabel  
> > wrote:
> 
> [...]
> 
> >>> I added Philipp and Hans to this thread - maybe they can comment on this.
> >>> To sum it up, our problem is:
> >>> - there are two separate USB PHYs on Meson GXBB
> >>> - both are sharing the same reset line (provided by the reset-meson 
> >>> driver)
> >>> - during initialization of the PHYs we must only call
> >>> reset_control_reset(rstc) once (if we do it for the first *and* second
> >>> PHY then the first PHY gets confused once the second PHY uses the
> >>> reset because the first PHY's state is reset as well)
> >>
> >> If you have an initially asserted reset line and you can enable the
> >> first module by deasserting the reset via reset_control_deassert (and
> >> reset_control_assert to signal when the module may be disabled again
> >> after use), shared resets are for you.
> >>
> >> If you need a reset pulse or have no direct control over the reset line,
> >> (device_reset), the reset framework currently has no solution for this.
> >> The ugly thing about reset_control_once would be that it can't re-reset
> >> modules when unloading and reloading driver modules.
> >
> > The corresponding reset driver in question is reset-meson, which only
> > implements reset (assert/deassert are not implemented). However, I
> > don't know if this is due to hardware design.
> > I think the hardware implements the latter, but maybe Neil can give
> > more information here (I currently don't have access to my board so I
> > cannot test how the hardware actually behaves).
> 
> It's implemented that way because the hardware only supports a reset
> pulse.

Would it be possible to bring down both PHYs drivers, pull the reset
line once, and then bring the drivers back up again?

regards
Philipp


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB

[Philipp Zabel]

Am Dienstag, den 13.09.2016, 17:59 -0700 schrieb Kevin Hilman:
> Martin Blumenstingl  writes:
> 
> > On Tue, Sep 13, 2016 at 5:28 PM, Philipp Zabel  
> > wrote:
> 
> [...]
> 
> >>> I added Philipp and Hans to this thread - maybe they can comment on this.
> >>> To sum it up, our problem is:
> >>> - there are two separate USB PHYs on Meson GXBB
> >>> - both are sharing the same reset line (provided by the reset-meson 
> >>> driver)
> >>> - during initialization of the PHYs we must only call
> >>> reset_control_reset(rstc) once (if we do it for the first *and* second
> >>> PHY then the first PHY gets confused once the second PHY uses the
> >>> reset because the first PHY's state is reset as well)
> >>
> >> If you have an initially asserted reset line and you can enable the
> >> first module by deasserting the reset via reset_control_deassert (and
> >> reset_control_assert to signal when the module may be disabled again
> >> after use), shared resets are for you.
> >>
> >> If you need a reset pulse or have no direct control over the reset line,
> >> (device_reset), the reset framework currently has no solution for this.
> >> The ugly thing about reset_control_once would be that it can't re-reset
> >> modules when unloading and reloading driver modules.
> >
> > The corresponding reset driver in question is reset-meson, which only
> > implements reset (assert/deassert are not implemented). However, I
> > don't know if this is due to hardware design.
> > I think the hardware implements the latter, but maybe Neil can give
> > more information here (I currently don't have access to my board so I
> > cannot test how the hardware actually behaves).
> 
> It's implemented that way because the hardware only supports a reset
> pulse.

Would it be possible to bring down both PHYs drivers, pull the reset
line once, and then bring the drivers back up again on this hardware?

regards
Philipp

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB

[Kevin Hilman]

Martin Blumenstingl  writes:

> On Tue, Sep 13, 2016 at 5:28 PM, Philipp Zabel  wrote:

[...]

>>> I added Philipp and Hans to this thread - maybe they can comment on this.
>>> To sum it up, our problem is:
>>> - there are two separate USB PHYs on Meson GXBB
>>> - both are sharing the same reset line (provided by the reset-meson driver)
>>> - during initialization of the PHYs we must only call
>>> reset_control_reset(rstc) once (if we do it for the first *and* second
>>> PHY then the first PHY gets confused once the second PHY uses the
>>> reset because the first PHY's state is reset as well)
>>
>> If you have an initially asserted reset line and you can enable the
>> first module by deasserting the reset via reset_control_deassert (and
>> reset_control_assert to signal when the module may be disabled again
>> after use), shared resets are for you.
>>
>> If you need a reset pulse or have no direct control over the reset line,
>> (device_reset), the reset framework currently has no solution for this.
>> The ugly thing about reset_control_once would be that it can't re-reset
>> modules when unloading and reloading driver modules.
>
> The corresponding reset driver in question is reset-meson, which only
> implements reset (assert/deassert are not implemented). However, I
> don't know if this is due to hardware design.
> I think the hardware implements the latter, but maybe Neil can give
> more information here (I currently don't have access to my board so I
> cannot test how the hardware actually behaves).

It's implemented that way because the hardware only supports a reset
pulse.

Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB

[Martin Blumenstingl]

Hi Philipp,

On Tue, Sep 13, 2016 at 5:28 PM, Philipp Zabel  wrote:
> Hi Martin,
>
> Am Freitag, den 09.09.2016, 22:36 +0200 schrieb Martin Blumenstingl:
>> On Fri, Sep 9, 2016 at 5:33 PM, Kevin Hilman  wrote:
>> > Martin Blumenstingl  writes:
>> >
>> >> On Thu, Sep 8, 2016 at 10:53 PM, Ben Dooks  
>> >> wrote:
>> >>> On 08/09/16 21:42, Kevin Hilman wrote:
>> 
>>  Ben Dooks  writes:
>> 
>> > On 08/09/16 20:52, Martin Blumenstingl wrote:
>> >>
>> >> On Thu, Sep 8, 2016 at 9:35 PM, Kevin Hilman 
>> >> wrote:
>> 
>>  + phy = devm_phy_create(>dev, NULL, _meson_usb2_ops);
>>  + if (IS_ERR(phy)) {
>>  + dev_err(>dev, "failed to create PHY\n");
>>  + return PTR_ERR(phy);
>>  + }
>>  +
>>  + if (usb_reset_refcnt++ == 0) {
>>  + ret = device_reset(>dev);
>>  + if (ret) {
>>  + dev_err(>dev, "Failed to reset USB 
>>  PHY\n");
>>  + return ret;
>>  + }
>>  + }
>> >>>
>> >>>
>> >>> The ref count + reset here looks like something that could/should be
>> >>> handled in a runtime PM callback.
>> >>
>> >> Unfortunately that doesn't work (as Jerome found out) because both
>> >> PHYs are sharing the same reset line.
>> >> So if the second PHY would call device_reset then it would also reset
>> >> the first PHY!
>> >>
>> >> There's a comment above the declaration of usb_reset_refcnt which
>> >> tries to explain this:
>> >> "The PHYs are sharing a common reset line -> we are only allowed to
>> >> reset once for all PHYs."
>> >> Maybe I should move this comment to the "if (usb_reset_refcnt++ == 0)
>> >> {" line to make it easier to see?
>> >>
>> >
>> > pm-runtime has refcounting in it. When one of the nodes turns on,
>> > the pm-runtime will call your driver to say there is a user when
>> > this first use turns up.
>> >
>> > If all the sub-phys turn off and drop their refcount then the driver
>> > is called to say there are no more users and you can go to sleep.
>> 
>> 
>>  After a chat w/Martin on IRC, It turns out runtime PM wont help here.
>> 
>>  The reason is because there are physically two PHY devices[1].  Those 2
>>  devices will be treated independely by runtime PM, and have separate
>>  use-counting, which means doing what I proposed would cause a reset to
>>  happen when either device was probed.
>> 
>>  So, I think it's OK as it is.
>> >>>
>> >>>
>> >>> Surely you can do pm_runtime_get/put on the phy's parent platform
>> >>> device and do it that way?
>> >> could you please be more specific with that (do you mean 
>> >> pdev->dev.parent)?
>> >> so we would use pm_runtime_{get_sync,put} with the parent, while we
>> >> would still define the runtime_resume in our driver.
>> >
>> > You'd also need to do get/put on the children, but yes, that's what Ben
>> > is suggesting.
>> >
>> > 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.
>> indeed, so in simple words we would need something like
>> reset_control_do_once(rstc, RESET/ASSERT/DEASSERT) which would
>> remember internally if any action has already been executed: if not it
>> does a _reset, _assert or _deassert and otherwise it does nothing.
>>
>> > I'm not sure yet how the reset framework is supposed to handle shared
>> > reset lines, but that needs some investigation.  I quick glance and it
>> > seems that reset controllers can have shared lines, so that should be
>> > investigated.
>> I added Philipp and Hans to this thread - maybe they can comment on this.
>> To sum it up, our problem is:
>> - there are two separate USB PHYs on Meson GXBB
>> - both are sharing the same reset line (provided by the reset-meson driver)
>> - during initialization of the PHYs we must only call
>> reset_control_reset(rstc) once (if we do it for the first *and* second
>> PHY then the first PHY gets confused once the second PHY uses the
>> reset because the first PHY's state is reset as well)
>
> If you have an initially asserted reset line and you can enable the
> first module by deasserting the reset via reset_control_deassert (and
> reset_control_assert 

Re: [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB

[Philipp Zabel]

Hi Martin,

Am Freitag, den 09.09.2016, 22:36 +0200 schrieb Martin Blumenstingl:
> On Fri, Sep 9, 2016 at 5:33 PM, Kevin Hilman  wrote:
> > Martin Blumenstingl  writes:
> >
> >> On Thu, Sep 8, 2016 at 10:53 PM, Ben Dooks  
> >> wrote:
> >>> On 08/09/16 21:42, Kevin Hilman wrote:
> 
>  Ben Dooks  writes:
> 
> > On 08/09/16 20:52, Martin Blumenstingl wrote:
> >>
> >> On Thu, Sep 8, 2016 at 9:35 PM, Kevin Hilman 
> >> wrote:
> 
>  + phy = devm_phy_create(>dev, NULL, _meson_usb2_ops);
>  + if (IS_ERR(phy)) {
>  + dev_err(>dev, "failed to create PHY\n");
>  + return PTR_ERR(phy);
>  + }
>  +
>  + if (usb_reset_refcnt++ == 0) {
>  + ret = device_reset(>dev);
>  + if (ret) {
>  + dev_err(>dev, "Failed to reset USB 
>  PHY\n");
>  + return ret;
>  + }
>  + }
> >>>
> >>>
> >>> The ref count + reset here looks like something that could/should be
> >>> handled in a runtime PM callback.
> >>
> >> Unfortunately that doesn't work (as Jerome found out) because both
> >> PHYs are sharing the same reset line.
> >> So if the second PHY would call device_reset then it would also reset
> >> the first PHY!
> >>
> >> There's a comment above the declaration of usb_reset_refcnt which
> >> tries to explain this:
> >> "The PHYs are sharing a common reset line -> we are only allowed to
> >> reset once for all PHYs."
> >> Maybe I should move this comment to the "if (usb_reset_refcnt++ == 0)
> >> {" line to make it easier to see?
> >>
> >
> > pm-runtime has refcounting in it. When one of the nodes turns on,
> > the pm-runtime will call your driver to say there is a user when
> > this first use turns up.
> >
> > If all the sub-phys turn off and drop their refcount then the driver
> > is called to say there are no more users and you can go to sleep.
> 
> 
>  After a chat w/Martin on IRC, It turns out runtime PM wont help here.
> 
>  The reason is because there are physically two PHY devices[1].  Those 2
>  devices will be treated independely by runtime PM, and have separate
>  use-counting, which means doing what I proposed would cause a reset to
>  happen when either device was probed.
> 
>  So, I think it's OK as it is.
> >>>
> >>>
> >>> Surely you can do pm_runtime_get/put on the phy's parent platform
> >>> device and do it that way?
> >> could you please be more specific with that (do you mean pdev->dev.parent)?
> >> so we would use pm_runtime_{get_sync,put} with the parent, while we
> >> would still define the runtime_resume in our driver.
> >
> > You'd also need to do get/put on the children, but yes, that's what Ben
> > is suggesting.
> >
> > 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.
> indeed, so in simple words we would need something like
> reset_control_do_once(rstc, RESET/ASSERT/DEASSERT) which would
> remember internally if any action has already been executed: if not it
> does a _reset, _assert or _deassert and otherwise it does nothing.
> 
> > I'm not sure yet how the reset framework is supposed to handle shared
> > reset lines, but that needs some investigation.  I quick glance and it
> > seems that reset controllers can have shared lines, so that should be
> > investigated.
> I added Philipp and Hans to this thread - maybe they can comment on this.
> To sum it up, our problem is:
> - there are two separate USB PHYs on Meson GXBB
> - both are sharing the same reset line (provided by the reset-meson driver)
> - during initialization of the PHYs we must only call
> reset_control_reset(rstc) once (if we do it for the first *and* second
> PHY then the first PHY gets confused once the second PHY uses the
> reset because the first PHY's state is reset as well)

If you have an initially asserted reset line and you can enable the
first module by deasserting the reset via reset_control_deassert (and
reset_control_assert to signal when the module may be disabled again
after use), shared resets are for you.

If you need a reset pulse or have no direct control over the reset line,
(device_reset), the reset framework currently has 

Re: [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB

[Kevin Hilman]

Martin Blumenstingl  writes:

> On Fri, Sep 9, 2016 at 10:36 PM, Martin Blumenstingl
>  wrote:
>> On Fri, Sep 9, 2016 at 5:33 PM, Kevin Hilman  wrote:
>>> Martin Blumenstingl  writes:
>>>
 On Thu, Sep 8, 2016 at 10:53 PM, Ben Dooks  
 wrote:
> On 08/09/16 21:42, Kevin Hilman wrote:
>>
>> Ben Dooks  writes:
>>
>>> On 08/09/16 20:52, Martin Blumenstingl wrote:

 On Thu, Sep 8, 2016 at 9:35 PM, Kevin Hilman 
 wrote:
>>
>> + phy = devm_phy_create(>dev, NULL, _meson_usb2_ops);
>> + if (IS_ERR(phy)) {
>> + dev_err(>dev, "failed to create PHY\n");
>> + return PTR_ERR(phy);
>> + }
>> +
>> + if (usb_reset_refcnt++ == 0) {
>> + ret = device_reset(>dev);
>> + if (ret) {
>> + dev_err(>dev, "Failed to reset USB 
>> PHY\n");
>> + return ret;
>> + }
>> + }
>
>
> The ref count + reset here looks like something that could/should be
> handled in a runtime PM callback.

 Unfortunately that doesn't work (as Jerome found out) because both
 PHYs are sharing the same reset line.
 So if the second PHY would call device_reset then it would also reset
 the first PHY!

 There's a comment above the declaration of usb_reset_refcnt which
 tries to explain this:
 "The PHYs are sharing a common reset line -> we are only allowed to
 reset once for all PHYs."
 Maybe I should move this comment to the "if (usb_reset_refcnt++ == 0)
 {" line to make it easier to see?

>>>
>>> pm-runtime has refcounting in it. When one of the nodes turns on,
>>> the pm-runtime will call your driver to say there is a user when
>>> this first use turns up.
>>>
>>> If all the sub-phys turn off and drop their refcount then the driver
>>> is called to say there are no more users and you can go to sleep.
>>
>>
>> After a chat w/Martin on IRC, It turns out runtime PM wont help here.
>>
>> The reason is because there are physically two PHY devices[1].  Those 2
>> devices will be treated independely by runtime PM, and have separate
>> use-counting, which means doing what I proposed would cause a reset to
>> happen when either device was probed.
>>
>> So, I think it's OK as it is.
>
>
> Surely you can do pm_runtime_get/put on the phy's parent platform
> device and do it that way?
 could you please be more specific with that (do you mean pdev->dev.parent)?
 so we would use pm_runtime_{get_sync,put} with the parent, while we
 would still define the runtime_resume in our driver.
>>>
>>> You'd also need to do get/put on the children, but yes, that's what Ben
>>> is suggesting.
>>>
>>> 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.
>> indeed, so in simple words we would need something like
>> reset_control_do_once(rstc, RESET/ASSERT/DEASSERT) which would
>> remember internally if any action has already been executed: if not it
>> does a _reset, _assert or _deassert and otherwise it does nothing.
> for now I've implemented something less hacky: I made the reset
> optional and only specified it for phy0.

That's slightly better, but could misbehave if devices are probed/loaded
in different order?  But, that shouldn't be a blocker for the driver.

> During Jerome's tests the reset was not needed, while on my board it's
> required to bring both PHYs up.
> Additionally the USB PHY reference driver does not have any reset
> logic for newer SoCs (GXL), so making the reset optional doesn't sound
> that bad to me.

Agreed.

Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB

[Martin Blumenstingl]

On Fri, Sep 9, 2016 at 5:33 PM, Kevin Hilman  wrote:
> Martin Blumenstingl  writes:
>
>> On Thu, Sep 8, 2016 at 10:53 PM, Ben Dooks  wrote:
>>> On 08/09/16 21:42, Kevin Hilman wrote:

 Ben Dooks  writes:

> On 08/09/16 20:52, Martin Blumenstingl wrote:
>>
>> On Thu, Sep 8, 2016 at 9:35 PM, Kevin Hilman 
>> wrote:

 + phy = devm_phy_create(>dev, NULL, _meson_usb2_ops);
 + if (IS_ERR(phy)) {
 + dev_err(>dev, "failed to create PHY\n");
 + return PTR_ERR(phy);
 + }
 +
 + if (usb_reset_refcnt++ == 0) {
 + ret = device_reset(>dev);
 + if (ret) {
 + dev_err(>dev, "Failed to reset USB PHY\n");
 + return ret;
 + }
 + }
>>>
>>>
>>> The ref count + reset here looks like something that could/should be
>>> handled in a runtime PM callback.
>>
>> Unfortunately that doesn't work (as Jerome found out) because both
>> PHYs are sharing the same reset line.
>> So if the second PHY would call device_reset then it would also reset
>> the first PHY!
>>
>> There's a comment above the declaration of usb_reset_refcnt which
>> tries to explain this:
>> "The PHYs are sharing a common reset line -> we are only allowed to
>> reset once for all PHYs."
>> Maybe I should move this comment to the "if (usb_reset_refcnt++ == 0)
>> {" line to make it easier to see?
>>
>
> pm-runtime has refcounting in it. When one of the nodes turns on,
> the pm-runtime will call your driver to say there is a user when
> this first use turns up.
>
> If all the sub-phys turn off and drop their refcount then the driver
> is called to say there are no more users and you can go to sleep.


 After a chat w/Martin on IRC, It turns out runtime PM wont help here.

 The reason is because there are physically two PHY devices[1].  Those 2
 devices will be treated independely by runtime PM, and have separate
 use-counting, which means doing what I proposed would cause a reset to
 happen when either device was probed.

 So, I think it's OK as it is.
>>>
>>>
>>> Surely you can do pm_runtime_get/put on the phy's parent platform
>>> device and do it that way?
>> could you please be more specific with that (do you mean pdev->dev.parent)?
>> so we would use pm_runtime_{get_sync,put} with the parent, while we
>> would still define the runtime_resume in our driver.
>
> You'd also need to do get/put on the children, but yes, that's what Ben
> is suggesting.
>
> 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.
indeed, so in simple words we would need something like
reset_control_do_once(rstc, RESET/ASSERT/DEASSERT) which would
remember internally if any action has already been executed: if not it
does a _reset, _assert or _deassert and otherwise it does nothing.

> I'm not sure yet how the reset framework is supposed to handle shared
> reset lines, but that needs some investigation.  I quick glance and it
> seems that reset controllers can have shared lines, so that should be
> investigated.
I added Philipp and Hans to this thread - maybe they can comment on this.
To sum it up, our problem is:
- there are two separate USB PHYs on Meson GXBB
- both are sharing the same reset line (provided by the reset-meson driver)
- during initialization of the PHYs we must only call
reset_control_reset(rstc) once (if we do it for the first *and* second
PHY then the first PHY gets confused once the second PHY uses the
reset because the first PHY's state is reset as well)


Regards,
Martin
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB

[Martin Blumenstingl]

On Fri, Sep 9, 2016 at 7:21 PM, Ben Dooks  wrote:
> On 09/09/16 17:14, 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 checked the USB PHY reference driver: it seems that there will be a
>> new USB PHY with the GXL/GXM SoCs.
>> So maybe we could live with the assumption that the PHYs are at
>> consecutive addresses.
>>
>>> I'm not sure yet how the reset framework is supposed to handle shared
>>> reset lines, but that needs some investigation.  I quick glance and it
>>> seems that reset controllers can have shared lines, so that should be
>>> investigated.
>>
>> unfortunately shared resets are not allowed to use reset_control_reset,
>> see [0]
>>
>>
>> [0] http://lxr.free-electrons.com/source/drivers/reset/core.c#L102
>
>
> If we didn't have the shared reset, we'd have one of node per phy
> and not have to have two sub-nodes... I don't think any other bits
> of the PHY framework are shared.
okay, sounds reasonable - so we should try to get this scenario
supported through by the reset framework -> see my other mail.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB

[Ben Dooks]

On 09/09/16 17:14, 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 checked the USB PHY reference driver: it seems that there will be a
new USB PHY with the GXL/GXM SoCs.
So maybe we could live with the assumption that the PHYs are at
consecutive addresses.


I'm not sure yet how the reset framework is supposed to handle shared
reset lines, but that needs some investigation.  I quick glance and it
seems that reset controllers can have shared lines, so that should be
investigated.

unfortunately shared resets are not allowed to use reset_control_reset, see [0]


[0] http://lxr.free-electrons.com/source/drivers/reset/core.c#L102


If we didn't have the shared reset, we'd have one of node per phy
and not have to have two sub-nodes... I don't think any other bits
of the PHY framework are shared.

--
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB

[Kevin Hilman]

Martin Blumenstingl  writes:

> 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 checked the USB PHY reference driver: it seems that there will be a
> new USB PHY with the GXL/GXM SoCs.
> So maybe we could live with the assumption that the PHYs are at
> consecutive addresses.

But isn't that also forcing us to make board-specific assumptions inside
the driver.

Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB

[Martin Blumenstingl]

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 checked the USB PHY reference driver: it seems that there will be a
new USB PHY with the GXL/GXM SoCs.
So maybe we could live with the assumption that the PHYs are at
consecutive addresses.

> I'm not sure yet how the reset framework is supposed to handle shared
> reset lines, but that needs some investigation.  I quick glance and it
> seems that reset controllers can have shared lines, so that should be
> investigated.
unfortunately shared resets are not allowed to use reset_control_reset, see [0]


[0] http://lxr.free-electrons.com/source/drivers/reset/core.c#L102
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB

[Kevin Hilman]

Martin Blumenstingl  writes:

> On Thu, Sep 8, 2016 at 10:53 PM, Ben Dooks  wrote:
>> On 08/09/16 21:42, Kevin Hilman wrote:
>>>
>>> Ben Dooks  writes:
>>>
 On 08/09/16 20:52, Martin Blumenstingl wrote:
>
> On Thu, Sep 8, 2016 at 9:35 PM, Kevin Hilman 
> wrote:
>>>
>>> + phy = devm_phy_create(>dev, NULL, _meson_usb2_ops);
>>> + if (IS_ERR(phy)) {
>>> + dev_err(>dev, "failed to create PHY\n");
>>> + return PTR_ERR(phy);
>>> + }
>>> +
>>> + if (usb_reset_refcnt++ == 0) {
>>> + ret = device_reset(>dev);
>>> + if (ret) {
>>> + dev_err(>dev, "Failed to reset USB PHY\n");
>>> + return ret;
>>> + }
>>> + }
>>
>>
>> The ref count + reset here looks like something that could/should be
>> handled in a runtime PM callback.
>
> Unfortunately that doesn't work (as Jerome found out) because both
> PHYs are sharing the same reset line.
> So if the second PHY would call device_reset then it would also reset
> the first PHY!
>
> There's a comment above the declaration of usb_reset_refcnt which
> tries to explain this:
> "The PHYs are sharing a common reset line -> we are only allowed to
> reset once for all PHYs."
> Maybe I should move this comment to the "if (usb_reset_refcnt++ == 0)
> {" line to make it easier to see?
>

 pm-runtime has refcounting in it. When one of the nodes turns on,
 the pm-runtime will call your driver to say there is a user when
 this first use turns up.

 If all the sub-phys turn off and drop their refcount then the driver
 is called to say there are no more users and you can go to sleep.
>>>
>>>
>>> After a chat w/Martin on IRC, It turns out runtime PM wont help here.
>>>
>>> The reason is because there are physically two PHY devices[1].  Those 2
>>> devices will be treated independely by runtime PM, and have separate
>>> use-counting, which means doing what I proposed would cause a reset to
>>> happen when either device was probed.
>>>
>>> So, I think it's OK as it is.
>>
>>
>> Surely you can do pm_runtime_get/put on the phy's parent platform
>> device and do it that way?
> could you please be more specific with that (do you mean pdev->dev.parent)?
> so we would use pm_runtime_{get_sync,put} with the parent, while we
> would still define the runtime_resume in our driver.

You'd also need to do get/put on the children, but yes, that's what Ben
is suggesting.

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.

I'm not sure yet how the reset framework is supposed to handle shared
reset lines, but that needs some investigation.  I quick glance and it
seems that reset controllers can have shared lines, so that should be
investigated.

Kevin





--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB

[Martin Blumenstingl]

On Thu, Sep 8, 2016 at 10:53 PM, Ben Dooks  wrote:
> On 08/09/16 21:42, Kevin Hilman wrote:
>>
>> Ben Dooks  writes:
>>
>>> On 08/09/16 20:52, Martin Blumenstingl wrote:

 On Thu, Sep 8, 2016 at 9:35 PM, Kevin Hilman 
 wrote:
>>
>> + phy = devm_phy_create(>dev, NULL, _meson_usb2_ops);
>> + if (IS_ERR(phy)) {
>> + dev_err(>dev, "failed to create PHY\n");
>> + return PTR_ERR(phy);
>> + }
>> +
>> + if (usb_reset_refcnt++ == 0) {
>> + ret = device_reset(>dev);
>> + if (ret) {
>> + dev_err(>dev, "Failed to reset USB PHY\n");
>> + return ret;
>> + }
>> + }
>
>
> The ref count + reset here looks like something that could/should be
> handled in a runtime PM callback.

 Unfortunately that doesn't work (as Jerome found out) because both
 PHYs are sharing the same reset line.
 So if the second PHY would call device_reset then it would also reset
 the first PHY!

 There's a comment above the declaration of usb_reset_refcnt which
 tries to explain this:
 "The PHYs are sharing a common reset line -> we are only allowed to
 reset once for all PHYs."
 Maybe I should move this comment to the "if (usb_reset_refcnt++ == 0)
 {" line to make it easier to see?

>>>
>>> pm-runtime has refcounting in it. When one of the nodes turns on,
>>> the pm-runtime will call your driver to say there is a user when
>>> this first use turns up.
>>>
>>> If all the sub-phys turn off and drop their refcount then the driver
>>> is called to say there are no more users and you can go to sleep.
>>
>>
>> After a chat w/Martin on IRC, It turns out runtime PM wont help here.
>>
>> The reason is because there are physically two PHY devices[1].  Those 2
>> devices will be treated independely by runtime PM, and have separate
>> use-counting, which means doing what I proposed would cause a reset to
>> happen when either device was probed.
>>
>> So, I think it's OK as it is.
>
>
> Surely you can do pm_runtime_get/put on the phy's parent platform
> device and do it that way?
could you please be more specific with that (do you mean pdev->dev.parent)?
so we would use pm_runtime_{get_sync,put} with the parent, while we
would still define the runtime_resume in our driver.

I'd be happy if that works and we can remove that refcounting hack
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB

[Ben Dooks]

On 08/09/16 21:42, Kevin Hilman wrote:

Ben Dooks  writes:


On 08/09/16 20:52, Martin Blumenstingl wrote:

On Thu, Sep 8, 2016 at 9:35 PM, Kevin Hilman  wrote:

+ phy = devm_phy_create(>dev, NULL, _meson_usb2_ops);
+ if (IS_ERR(phy)) {
+ dev_err(>dev, "failed to create PHY\n");
+ return PTR_ERR(phy);
+ }
+
+ if (usb_reset_refcnt++ == 0) {
+ ret = device_reset(>dev);
+ if (ret) {
+ dev_err(>dev, "Failed to reset USB PHY\n");
+ return ret;
+ }
+ }


The ref count + reset here looks like something that could/should be
handled in a runtime PM callback.

Unfortunately that doesn't work (as Jerome found out) because both
PHYs are sharing the same reset line.
So if the second PHY would call device_reset then it would also reset
the first PHY!

There's a comment above the declaration of usb_reset_refcnt which
tries to explain this:
"The PHYs are sharing a common reset line -> we are only allowed to
reset once for all PHYs."
Maybe I should move this comment to the "if (usb_reset_refcnt++ == 0)
{" line to make it easier to see?



pm-runtime has refcounting in it. When one of the nodes turns on,
the pm-runtime will call your driver to say there is a user when
this first use turns up.

If all the sub-phys turn off and drop their refcount then the driver
is called to say there are no more users and you can go to sleep.


After a chat w/Martin on IRC, It turns out runtime PM wont help here.

The reason is because there are physically two PHY devices[1].  Those 2
devices will be treated independely by runtime PM, and have separate
use-counting, which means doing what I proposed would cause a reset to
happen when either device was probed.

So, I think it's OK as it is.


Surely you can do pm_runtime_get/put on the phy's parent platform
device and do it that way?

--
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB

[Kevin Hilman]

Ben Dooks  writes:

> On 08/09/16 20:52, Martin Blumenstingl wrote:
>> On Thu, Sep 8, 2016 at 9:35 PM, Kevin Hilman  wrote:
 + phy = devm_phy_create(>dev, NULL, _meson_usb2_ops);
 + if (IS_ERR(phy)) {
 + dev_err(>dev, "failed to create PHY\n");
 + return PTR_ERR(phy);
 + }
 +
 + if (usb_reset_refcnt++ == 0) {
 + ret = device_reset(>dev);
 + if (ret) {
 + dev_err(>dev, "Failed to reset USB PHY\n");
 + return ret;
 + }
 + }
>>>
>>> The ref count + reset here looks like something that could/should be
>>> handled in a runtime PM callback.
>> Unfortunately that doesn't work (as Jerome found out) because both
>> PHYs are sharing the same reset line.
>> So if the second PHY would call device_reset then it would also reset
>> the first PHY!
>>
>> There's a comment above the declaration of usb_reset_refcnt which
>> tries to explain this:
>> "The PHYs are sharing a common reset line -> we are only allowed to
>> reset once for all PHYs."
>> Maybe I should move this comment to the "if (usb_reset_refcnt++ == 0)
>> {" line to make it easier to see?
>>
>
> pm-runtime has refcounting in it. When one of the nodes turns on,
> the pm-runtime will call your driver to say there is a user when
> this first use turns up.
>
> If all the sub-phys turn off and drop their refcount then the driver
> is called to say there are no more users and you can go to sleep.

After a chat w/Martin on IRC, It turns out runtime PM wont help here.

The reason is because there are physically two PHY devices[1].  Those 2
devices will be treated independely by runtime PM, and have separate
use-counting, which means doing what I proposed would cause a reset to
happen when either device was probed.

So, I think it's OK as it is.

Kevin

[1] from the DT patch:

diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi 
b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
index 2e8a3d9..02dfc54 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
@@ -151,6 +151,34 @@
#size-cells = <2>;
ranges;
 
+   usb-phys@c000 {
+   compatible = "simple-bus";
+   reg = <0x0 0xc000 0x0 0x40>;
+   #address-cells = <2>;
+   #size-cells = <2>;
+   ranges = <0x0 0x0 0x0 0xc000 0x0 0x40>;
+
+   usb0_phy: usb_phy@0 {
+   compatible = "amlogic,meson-gxbb-usb2-phy";
+   #phy-cells = <0>;
+   reg = <0x0 0x0 0x0 0x20>;
+   resets = < 34>;
+   clocks = < CLKID_USB  CLKID_USB0>;
+   clock-names = "usb_general", "usb";
+   status = "disabled";
+   };
+
+   usb1_phy: usb_phy@20 {
+   compatible = "amlogic,meson-gxbb-usb2-phy";
+   #phy-cells = <0>;
+   reg = <0x0 0x20 0x0 0x20>;
+   resets = < 34>;
+   clocks = < CLKID_USB  CLKID_USB1>;
+   clock-names = "usb_general", "usb";
+   status = "disabled";
+   };
+   };
+
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB

[Ben Dooks]

On 08/09/16 20:52, Martin Blumenstingl wrote:

On Thu, Sep 8, 2016 at 9:35 PM, Kevin Hilman  wrote:

+ phy = devm_phy_create(>dev, NULL, _meson_usb2_ops);
+ if (IS_ERR(phy)) {
+ dev_err(>dev, "failed to create PHY\n");
+ return PTR_ERR(phy);
+ }
+
+ if (usb_reset_refcnt++ == 0) {
+ ret = device_reset(>dev);
+ if (ret) {
+ dev_err(>dev, "Failed to reset USB PHY\n");
+ return ret;
+ }
+ }


The ref count + reset here looks like something that could/should be
handled in a runtime PM callback.

Unfortunately that doesn't work (as Jerome found out) because both
PHYs are sharing the same reset line.
So if the second PHY would call device_reset then it would also reset
the first PHY!

There's a comment above the declaration of usb_reset_refcnt which
tries to explain this:
"The PHYs are sharing a common reset line -> we are only allowed to
reset once for all PHYs."
Maybe I should move this comment to the "if (usb_reset_refcnt++ == 0)
{" line to make it easier to see?



pm-runtime has refcounting in it. When one of the nodes turns on,
the pm-runtime will call your driver to say there is a user when
this first use turns up.

If all the sub-phys turn off and drop their refcount then the driver
is called to say there are no more users and you can go to sleep.

So, in phy_meson_usb2_power_on() you could do:

pm_runtime_get_sync(pdev);

and in phy_meson_usb2_power_off

pm_runtime_put(pdev);

https://www.kernel.org/doc/Documentation/power/runtime_pm.txt

--
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB

[Martin Blumenstingl]

On Thu, Sep 8, 2016 at 9:35 PM, Kevin Hilman  wrote:
>> + phy = devm_phy_create(>dev, NULL, _meson_usb2_ops);
>> + if (IS_ERR(phy)) {
>> + dev_err(>dev, "failed to create PHY\n");
>> + return PTR_ERR(phy);
>> + }
>> +
>> + if (usb_reset_refcnt++ == 0) {
>> + ret = device_reset(>dev);
>> + if (ret) {
>> + dev_err(>dev, "Failed to reset USB PHY\n");
>> + return ret;
>> + }
>> + }
>
> The ref count + reset here looks like something that could/should be
> handled in a runtime PM callback.
Unfortunately that doesn't work (as Jerome found out) because both
PHYs are sharing the same reset line.
So if the second PHY would call device_reset then it would also reset
the first PHY!

There's a comment above the declaration of usb_reset_refcnt which
tries to explain this:
"The PHYs are sharing a common reset line -> we are only allowed to
reset once for all PHYs."
Maybe I should move this comment to the "if (usb_reset_refcnt++ == 0)
{" line to make it easier to see?


Regards,
Martin
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB

[Ben Dooks]

On 08/09/16 20:35, Kevin Hilman wrote:

Martin Blumenstingl  writes:


This is a new driver for the USB PHY found in Meson8b and GXBB SoCs.

Signed-off-by: Martin Blumenstingl 
Signed-off-by: Jerome Brunet 


I tested this on meson-gxbb-p200 with USB host and a mass storage
device.

Tested-by: Kevin Hilman 

A minor question/comment below, for you and for Kishon...

[...]


+static int phy_meson_usb2_probe(struct platform_device *pdev)
+{
+   struct phy_meson_usb2_priv *priv;
+   struct resource *res;
+   struct phy *phy;
+   struct phy_provider *phy_provider;
+   int ret;
+
+   priv = devm_kzalloc(>dev, sizeof(*priv), GFP_KERNEL);
+   if (!priv)
+   return -ENOMEM;
+
+   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+   priv->regs = devm_ioremap_resource(>dev, res);
+   if (IS_ERR(priv->regs))
+   return PTR_ERR(priv->regs);
+
+   priv->clk_usb_general = devm_clk_get(>dev, "usb_general");
+   if (IS_ERR(priv->clk_usb_general))
+   return PTR_ERR(priv->clk_usb_general);
+
+   priv->clk_usb = devm_clk_get(>dev, "usb");
+   if (IS_ERR(priv->clk_usb))
+   return PTR_ERR(priv->clk_usb);
+
+   priv->dr_mode = of_usb_get_dr_mode_by_phy(pdev->dev.of_node, -1);
+   if (priv->dr_mode == USB_DR_MODE_UNKNOWN) {
+   dev_err(>dev,
+   "missing dual role configuration of the controller\n");
+   return -EINVAL;
+   }
+
+   phy = devm_phy_create(>dev, NULL, _meson_usb2_ops);
+   if (IS_ERR(phy)) {
+   dev_err(>dev, "failed to create PHY\n");
+   return PTR_ERR(phy);
+   }
+
+   if (usb_reset_refcnt++ == 0) {
+   ret = device_reset(>dev);
+   if (ret) {
+   dev_err(>dev, "Failed to reset USB PHY\n");
+   return ret;
+   }
+   }


The ref count + reset here looks like something that could/should be
handled in a runtime PM callback.

IOW, there should be a pm_runtime_get_sync() here, and in the
->runtime_resume() callback, the device_reset() would be called.
Runtime PM does the refcounting, and only calls ->runtime_resume() on
the 0 -> 1 transition.

This isn't a big deal for now, so I'll let Kishon decide, but using
runtime PM from the start will help enabling other PM features later.


I agree, pm_runtime would probably be the best place to handle >1
device with shared items such as reset.

The version I wrote, I simply enabled the clocks and reset the device
when probed to work around the shared reset.


--
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB

[Martin Blumenstingl]

This is a new driver for the USB PHY found in Meson8b and GXBB SoCs.

Signed-off-by: Martin Blumenstingl 
Signed-off-by: Jerome Brunet 
---
 drivers/phy/Kconfig  |  11 ++
 drivers/phy/Makefile |   1 +
 drivers/phy/phy-meson-usb2.c | 299 +++
 3 files changed, 311 insertions(+)
 create mode 100644 drivers/phy/phy-meson-usb2.c

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 19bff3a..6ad87ec 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -453,4 +453,15 @@ config PHY_NS2_PCIE
help
  Enable this to support the Broadcom Northstar2 PCIe PHY.
  If unsure, say N.
+
+config PHY_MESON_USB2
+   tristate "Meson USB2 PHY driver"
+   default ARCH_MESON
+   depends on OF && (ARCH_MESON || COMPILE_TEST)
+   select GENERIC_PHY
+   help
+ Enable this to support the Meson USB2 PHYs found in Meson8b
+ and GXBB SoCs.
+ If unsure, say N.
+
 endmenu
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 90ae198..dd507ac 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -56,3 +56,4 @@ obj-$(CONFIG_PHY_PISTACHIO_USB)   += 
phy-pistachio-usb.o
 obj-$(CONFIG_PHY_CYGNUS_PCIE)  += phy-bcm-cygnus-pcie.o
 obj-$(CONFIG_ARCH_TEGRA) += tegra/
 obj-$(CONFIG_PHY_NS2_PCIE) += phy-bcm-ns2-pcie.o
+obj-$(CONFIG_PHY_MESON_USB2)   += phy-meson-usb2.o
diff --git a/drivers/phy/phy-meson-usb2.c b/drivers/phy/phy-meson-usb2.c
new file mode 100644
index 000..8cda138
--- /dev/null
+++ b/drivers/phy/phy-meson-usb2.c
@@ -0,0 +1,299 @@
+/*
+ * Meson USB2 PHY driver
+ *
+ * Copyright (C) 2016 Martin Blumenstingl 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see .
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define REG_CONFIG 0x00
+   #define REG_CONFIG_CLK_EN   BIT(0)
+   #define REG_CONFIG_CLK_SEL_MASK GENMASK(3, 1)
+   #define REG_CONFIG_CLK_DIV_MASK GENMASK(10, 4)
+   #define REG_CONFIG_CLK_32k_ALTSEL   BIT(15)
+   #define REG_CONFIG_TEST_TRIGBIT(31)
+
+#define REG_CTRL   0x04
+   #define REG_CTRL_SOFT_PRST  BIT(0)
+   #define REG_CTRL_SOFT_HRESETBIT(1)
+   #define REG_CTRL_SS_SCALEDOWN_MODE_MASK GENMASK(3, 2)
+   #define REG_CTRL_CLK_DET_RSTBIT(4)
+   #define REG_CTRL_INTR_SEL   BIT(5)
+   #define REG_CTRL_CLK_DETECTED   BIT(8)
+   #define REG_CTRL_SOF_SENT_RCVD_TGL  BIT(9)
+   #define REG_CTRL_SOF_TOGGLE_OUT BIT(10)
+   #define REG_CTRL_POWER_ON_RESET BIT(15)
+   #define REG_CTRL_SLEEPM BIT(16)
+   #define REG_CTRL_TX_BITSTUFF_ENN_H  BIT(17)
+   #define REG_CTRL_TX_BITSTUFF_ENNBIT(18)
+   #define REG_CTRL_COMMON_ON  BIT(19)
+   #define REG_CTRL_REF_CLK_SEL_MASK   GENMASK(21, 20)
+   #define REG_CTRL_REF_CLK_SEL_SHIFT  20
+   #define REG_CTRL_FSEL_MASK  GENMASK(24, 22)
+   #define REG_CTRL_FSEL_SHIFT 22
+   #define REG_CTRL_PORT_RESET BIT(25)
+   #define REG_CTRL_THREAD_ID_MASK GENMASK(31, 26)
+
+#define REG_ENDP_INTR  0x08
+
+/* bits [31:26], [24:21] and [15:3] seem to be read-only */
+#define REG_ADP_BC 0x0c
+   #define REG_ADP_BC_VBUS_VLD_EXT_SEL BIT(0)
+   #define REG_ADP_BC_VBUS_VLD_EXT BIT(1)
+   #define REG_ADP_BC_OTG_DISABLE  BIT(2)
+   #define REG_ADP_BC_ID_PULLUPBIT(3)
+   #define REG_ADP_BC_DRV_VBUS BIT(4)
+   #define REG_ADP_BC_ADP_PRB_EN   BIT(5)
+   #define REG_ADP_BC_ADP_DISCHARGEBIT(6)
+   #define REG_ADP_BC_ADP_CHARGE   BIT(7)
+   #define REG_ADP_BC_SESS_END BIT(8)
+   #define REG_ADP_BC_DEVICE_SESS_VLD  BIT(9)
+   #define REG_ADP_BC_B_VALID  BIT(10)
+   #define REG_ADP_BC_A_VALID  BIT(11)
+   #define REG_ADP_BC_ID_DIG   BIT(12)
+   #define REG_ADP_BC_VBUS_VALID