From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
To: Peter Chen <hzpeterchen@gmail.com>
CC: "kishon@ti.com" <kishon@ti.com>,
        "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
        "balbi@kernel.org" <balbi@kernel.org>,
        "Peter.Chen@nxp.com" <Peter.Chen@nxp.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
        "linux-renesas-soc@vger.kernel.org" 
        <linux-renesas-soc@vger.kernel.org>
Subject: RE: [PATCH v3] phy: rcar-gen3-usb2: add sysfs for usb role swap
Thread-Topic: [PATCH v3] phy: rcar-gen3-usb2: add sysfs for usb role swap
Thread-Index: AQHSNNUfgYkBz5mVJkWGG82HsIxsqaDO2l6AgAAK/3A=
Date: Tue, 8 Nov 2016 10:15:25 +0000
Message-ID: <HK2PR06MB0548DB0A4DFDAFEF0A27D2C2D8A60@HK2PR06MB0548.apcprd06.prod.outlook.com>
References: <1478069232-20570-1-git-send-email-yoshihiro.shimoda.uh@renesas.com>
 <20161108092516.GB12276@b29397-desktop>
In-Reply-To: <20161108092516.GB12276@b29397-desktop>
Accept-Language: ja-JP, en-US
Content-Language: ja-JP
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-originalarrivaltime: 08 Nov 2016 10:15:25.2446
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 53d82571-da19-47e4-9cb4-625a166a4a2a
X-MS-Exchange-Transport-CrossTenantHeadersStamped: HK2PR06MB0547
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Transfer-Encoding: 8bit
X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id uA8AFvsd006305
Content-Length: 1624
Lines: 62

Hi,

> From: Peter Chen
> Sent: Tuesday, November 08, 2016 6:25 PM
> 
> On Wed, Nov 02, 2016 at 03:47:12PM +0900, Yoshihiro Shimoda wrote:
< snip >
> > +	/* If current and new mode is the same, this returns the error */
> > +	if (is_host == new_mode_is_host)
> > +		return -EINVAL;
> > +
> > +	if (new_mode_is_host) {
> > +		if (!is_b_device && !is_host)	/* A-Peripheral */
> > +			rcar_gen3_init_from_a_peri_to_a_host(ch);
> > +		if (is_b_device && !is_host)	/* B-Peripheral */
> > +			rcar_gen3_init_for_b_host(ch);
> 
> is_host must be false here

Thank you for the review!
I will clean up the code and submit v4 patch soon.

> > +	} else {
> > +		if (!is_b_device && is_host)	/* A-Host */
> > +			rcar_gen3_init_for_a_peri(ch);
> > +		if (is_b_device && is_host)	/* B-Host */
> > +			rcar_gen3_init_for_peri(ch);
> 
> is_host must be true here

Same the above.

> > +	}
> > +
> > +	return count;
> > +}
> > +
> 
> How can you handle the case which ID pin is incorrect, eg, ID is 0, but
> it is peripheral mode at the initialization?

This driver always checks the ID pin and sets the suitable mode in initialization timing.
So, we can ignore such a situation.

< In detail >
 phy_init()			@ drivers/phy/phy-core.c
 --> rcar_gen3_phy_usb2_init()
  --> rcar_gen3_phy_init_otg()
   --> rcar_gen3_device_recognition()
    --> if (!rcar_gen3_check_id(ch))
          rcar_gen3_init_for_host(ch);   <-- Sets the mode as host
        else
          rcar_gen3_init_for_peri(ch);   <-- Sets the mode as peripheral
++phy->init_count;		@ drivers/phy/phy-core.c

Best regards,
Yoshihiro Shimoda

> --
> 
> Best Regards,
> Peter Chen