linux-kernel - Re: [PATCH 2/4] phy: rockchip-usb: use rockchip_usb_phy

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2546474.rLYzuqPxCa@diego>
Date:   Mon, 22 Aug 2016 15:00:42 +0200
From:   Heiko Stübner <heiko@...ech.de>
To:     Kishon Vijay Abraham I <kishon@...com>
Cc:     Randy Li <ayaka@...lik.info>, linux-usb@...r.kernel.org,
        johnyoun@...opsys.com, gregkh@...uxfoundation.org,
        robh+dt@...nel.org, mark.rutland@....com,
        devicetree@...r.kernel.org, eddie.cai@...k-chips.com,
        randy.li@...k-chips.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/4] phy: rockchip-usb: use rockchip_usb_phy_reset to reset phy during wakeup

Am Montag, 22. August 2016, 17:17:41 schrieb Kishon Vijay Abraham I:
> Hi,
> 
> On Sunday 21 August 2016 02:02 AM, Randy Li wrote:
> > It is a hardware bug in RK3288, the only way to solve it is to
> > reset the phy.
> > 
> > Signed-off-by: Randy Li <ayaka@...lik.info>
> > ---
> > 
> >  drivers/phy/phy-rockchip-usb.c | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> > 
> > diff --git a/drivers/phy/phy-rockchip-usb.c
> > b/drivers/phy/phy-rockchip-usb.c index 2a7381f..734987f 100644
> > --- a/drivers/phy/phy-rockchip-usb.c
> > +++ b/drivers/phy/phy-rockchip-usb.c
> > @@ -29,6 +29,7 @@
> > 
> >  #include <linux/reset.h>
> >  #include <linux/regmap.h>
> >  #include <linux/mfd/syscon.h>
> > 
> > +#include <linux/delay.h>
> > 
> >  static int enable_usb_uart;
> > 
> > @@ -64,6 +65,7 @@ struct rockchip_usb_phy {
> > 
> >  	struct clk_hw	clk480m_hw;
> >  	struct phy	*phy;
> >  	bool		uart_enabled;
> > 
> > +	struct reset_control *reset;
> > 
> >  };
> >  
> >  static int rockchip_usb_phy_power(struct rockchip_usb_phy *phy,
> > 
> > @@ -144,9 +146,23 @@ static int rockchip_usb_phy_power_on(struct phy
> > *_phy)
> > 
> >  	return clk_prepare_enable(phy->clk480m);
> >  
> >  }
> > 
> > +static int rockchip_usb_phy_reset(struct phy *_phy)
> > +{
> > +	struct rockchip_usb_phy *phy = phy_get_drvdata(_phy);
> > +
> > +	if (phy->reset) {
> > +		reset_control_assert(phy->reset);
> > +		udelay(10);
> > +		reset_control_deassert(phy->reset);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > 
> >  static const struct phy_ops ops = {
> >  
> >  	.power_on	= rockchip_usb_phy_power_on,
> >  	.power_off	= rockchip_usb_phy_power_off,
> > 
> > +	.reset		= rockchip_usb_phy_reset,
> 
> why not just reuse the .init ops? reset can be done during initialization
> right?

The naming of power_on + power_off and init + exit probably suggests that they 
are supposed to be used in pairs. (aka module_init + module_exit and probably 
more)

But in fact I've seen different combinations so far (phy_init + phy_power_on 
... phy_power_off + phy_exit but also phy_power_on + phy_init ... phy_exit + 
phy_power_off), so I guess the semantics are not that strictly defined.