[PATCH 0/4] phy: rockchip-inno-usb2: document improvements and allow to force B-device valid session bit.

[PATCH 0/4] phy: rockchip-inno-usb2: document improvements and allow to force B-device valid session bit.

[Enric Balletbo i Serra]

Hi all,

The main purpose of this patchset is have the Type-C port on the Samsung
Chromebook Plus work as a device or in OTG mode. While doing it I spent
some time to fix some documentation issues. So, the first and the second
patch are not really related to the topic and can be picked
independently of the future discussion generated by the third and the
fourth patch (these two patches are the ones that makes the type-c port
work as device)

The patchset was tested on a Samsung Chromebook Plus by configuring one
port to work as device, configure a cdc ethernet gadget and communicate
via ethernet gadget my workstation with the chromebook through a usb-a
to type-c cable.

Best regards,
 Enric


Enric Balletbo i Serra (4):
  phy: rockchip-inno-usb2: fix misspelling and kernel-doc documentation.
  dt-bindings: phy-rockchip-inno-usb2: add documentation for extcon and
    utmi-avalid properties.
  phy: rockchip-inno-usb2: allow to force the B-Device Session Valid
    bit.
  dt-bindings: phy-rockchip-inno-usb2: add new rockchip,force-bvalid
    property.

 .../bindings/phy/phy-rockchip-inno-usb2.txt   |  8 +++
 drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 70 ++++++++++++++-----
 2 files changed, 62 insertions(+), 16 deletions(-)

-- 
2.18.0

[PATCH 1/4] phy: rockchip-inno-usb2: fix misspelling and kernel-doc documentation.

[Enric Balletbo i Serra]

Fix the typo flase -> false and clean up the kernel-doc documentation in
phy-rockchip-inno.usb2.c and fix the following warnings when documentation
is built.

  :58: warning: missing initial short description
  :69: warning: cannot understand function prototype: 'enum usb_chg_state '
  :97: warning: missing initial short description
  :136: warning: cannot understand function prototype: 'struct rockchip_usb2phy_port_cfg '
  :157: warning: cannot understand function prototype: 'struct rockchip_usb2phy_cfg '
  :163: warning: Function parameter or member 'port_cfgs' not described in 'rockchip_usb2phy_cfg'
  :187: warning: cannot understand function prototype: 'struct rockchip_usb2phy_port '
  :204: warning: Function parameter or member 'port_cfg' not described in 'rockchip_usb2phy_port'
  :207: warning: missing initial short description
  :234: warning: Function parameter or member 'dev' not described in 'rockchip_usb2phy'
  :234: warning: Function parameter or member 'clk480m_hw' not described in 'rockchip_usb2phy'

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

 drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 35 ++++++++++---------
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
index 5049dac79bd0..e1ef3e34163c 100644
--- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
+++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
@@ -55,16 +55,16 @@ enum rockchip_usb2phy_host_state {
 };
 
 /**
- * Different states involved in USB charger detection.
- * USB_CHG_STATE_UNDEFINED	USB charger is not connected or detection
+ * enum usb_chg_state - Different states involved in USB charger detection.
+ * @USB_CHG_STATE_UNDEFINED:	USB charger is not connected or detection
  *				process is not yet started.
- * USB_CHG_STATE_WAIT_FOR_DCD	Waiting for Data pins contact.
- * USB_CHG_STATE_DCD_DONE	Data pin contact is detected.
- * USB_CHG_STATE_PRIMARY_DONE	Primary detection is completed (Detects
+ * @USB_CHG_STATE_WAIT_FOR_DCD:	Waiting for Data pins contact.
+ * @USB_CHG_STATE_DCD_DONE:	Data pin contact is detected.
+ * @USB_CHG_STATE_PRIMARY_DONE:	Primary detection is completed (Detects
  *				between SDP and DCP/CDP).
- * USB_CHG_STATE_SECONDARY_DONE	Secondary detection is completed (Detects
- *				between DCP and CDP).
- * USB_CHG_STATE_DETECTED	USB charger type is determined.
+ * @USB_CHG_STATE_SECONDARY_DONE: Secondary detection is completed (Detects
+ *				  between DCP and CDP).
+ * @USB_CHG_STATE_DETECTED:	USB charger type is determined.
  */
 enum usb_chg_state {
 	USB_CHG_STATE_UNDEFINED = 0,
@@ -94,7 +94,7 @@ struct usb2phy_reg {
 };
 
 /**
- * struct rockchip_chg_det_reg: usb charger detect registers
+ * struct rockchip_chg_det_reg - usb charger detect registers
  * @cp_det: charging port detected successfully.
  * @dcp_det: dedicated charging port detected successfully.
  * @dp_det: assert data pin connect successfully.
@@ -120,7 +120,7 @@ struct rockchip_chg_det_reg {
 };
 
 /**
- * struct rockchip_usb2phy_port_cfg: usb-phy port configuration.
+ * struct rockchip_usb2phy_port_cfg - usb-phy port configuration.
  * @phy_sus: phy suspend register.
  * @bvalid_det_en: vbus valid rise detection enable register.
  * @bvalid_det_st: vbus valid rise detection status register.
@@ -148,10 +148,11 @@ struct rockchip_usb2phy_port_cfg {
 };
 
 /**
- * struct rockchip_usb2phy_cfg: usb-phy configuration.
+ * struct rockchip_usb2phy_cfg - usb-phy configuration.
  * @reg: the address offset of grf for usb-phy config.
  * @num_ports: specify how many ports that the phy has.
  * @clkout_ctl: keep on/turn off output clk of phy.
+ * @port_cfgs: usb-phy port configurations.
  * @chg_det: charger detection registers.
  */
 struct rockchip_usb2phy_cfg {
@@ -163,12 +164,13 @@ struct rockchip_usb2phy_cfg {
 };
 
 /**
- * struct rockchip_usb2phy_port: usb-phy port data.
+ * struct rockchip_usb2phy_port - usb-phy port data.
+ * @phy: generic phy.
  * @port_id: flag for otg port or host port.
  * @suspended: phy suspended flag.
  * @utmi_avalid: utmi avalid status usage flag.
  *	true	- use avalid to get vbus status
- *	flase	- use bvalid to get vbus status
+ *	false	- use bvalid to get vbus status
  * @vbus_attached: otg device vbus status.
  * @bvalid_irq: IRQ number assigned for vbus valid rise detection.
  * @ls_irq: IRQ number assigned for linestate detection.
@@ -178,7 +180,7 @@ struct rockchip_usb2phy_cfg {
  * @chg_work: charge detect work.
  * @otg_sm_work: OTG state machine work.
  * @sm_work: HOST state machine work.
- * @phy_cfg: port register configuration, assigned by driver data.
+ * @port_cfg: port register configuration, assigned by driver data.
  * @event_nb: hold event notification callback.
  * @state: define OTG enumeration states before device reset.
  * @mode: the dr_mode of the controller.
@@ -203,12 +205,13 @@ struct rockchip_usb2phy_port {
 };
 
 /**
- * struct rockchip_usb2phy: usb2.0 phy driver data.
+ * struct rockchip_usb2phy - usb2.0 phy driver data.
+ * @dev: pointer to device.
  * @grf: General Register Files regmap.
  * @usbgrf: USB General Register Files regmap.
  * @clk: clock struct of phy input clk.
  * @clk480m: clock struct of phy output clk.
- * @clk_hw: clock struct of phy output clk management.
+ * @clk480m_hw: clock struct of phy output clk management.
  * @chg_state: states involved in USB charger detection.
  * @chg_type: USB charger types.
  * @dcd_retries: The retry count used to track Data contact
-- 
2.18.0

[PATCH 2/4] dt-bindings: phy-rockchip-inno-usb2: add documentation for extcon and utmi-avalid properties.

[Enric Balletbo i Serra]

Commit 98898f3bc83c8 ("phy: rockchip-inno-usb2: support otg-port for
rk3399") introduces two new properties. The extcon property is used to
detect the cable-state, and the rockchip,utmi-avalid is used to indicate
which register should be used to detect the vbus state.

Document these properties in the documentation binding.

Fixes: 98898f3bc83c8 ("phy: rockchip-inno-usb2: support otg-port for rk3399")
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

 .../devicetree/bindings/phy/phy-rockchip-inno-usb2.txt         | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
index 074a7b3b0425..2d4808d3920b 100644
--- a/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
+++ b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
@@ -23,6 +23,7 @@ Optional properties:
 		 register files". When set driver will request its
 		 phandle as one companion-grf for some special SoCs
 		 (e.g RV1108).
+ - extcon : phandle to the extcon device for the otg phy.
 
 Required nodes : a sub-node is required for each port the phy provides.
 		 The sub-node name is used to identify host or otg port,
@@ -45,6 +46,8 @@ Required properties (port (child) node):
 Optional properties:
  - phy-supply : phandle to a regulator that provides power to VBUS.
 		See ./phy-bindings.txt for details.
+ - rockchip,utmi-avalid : boolean, use the avalid register to get vbus status.
+			  Otherwise, use the bvalid register.
 
 Example:
 
-- 
2.18.0

[PATCH 3/4] phy: rockchip-inno-usb2: allow to force the B-Device Session Valid bit.

[Enric Balletbo i Serra]

The OTG disconnection event is generated after the presence/abscense of
an ID connection, but some platforms doesn't have the ID pin connected, so
the event is not generated. In such case, for detecting the disconnection
event, we can get the cable state from an extcon driver. We need, though,
to force to set the B-Device Session Valid bit on the PHY to have the
device respond to setup address. Otherwise, the following error is
shown:

    usb 2-2: Device not responding to setup address.
    usb 2-2: device not accepting address 14, error -71
    usb usb2-port2: unable to enumerate USB device

The patch allows to tell the PHY to force the B-Device Session Valid bit
when the OTG role is device and clear that bit if the OTG role is host.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

 drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 35 +++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
index e1ef3e34163c..e7337c60ff9d 100644
--- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
+++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
@@ -125,6 +125,7 @@ struct rockchip_chg_det_reg {
  * @bvalid_det_en: vbus valid rise detection enable register.
  * @bvalid_det_st: vbus valid rise detection status register.
  * @bvalid_det_clr: vbus valid rise detection clear register.
+ * @bvalid_session: force B-device session valid register.
  * @ls_det_en: linestate detection enable register.
  * @ls_det_st: linestate detection state register.
  * @ls_det_clr: linestate detection clear register.
@@ -138,6 +139,7 @@ struct rockchip_usb2phy_port_cfg {
 	struct usb2phy_reg	bvalid_det_en;
 	struct usb2phy_reg	bvalid_det_st;
 	struct usb2phy_reg	bvalid_det_clr;
+	struct usb2phy_reg	bvalid_session;
 	struct usb2phy_reg	ls_det_en;
 	struct usb2phy_reg	ls_det_st;
 	struct usb2phy_reg	ls_det_clr;
@@ -172,6 +174,7 @@ struct rockchip_usb2phy_cfg {
  *	true	- use avalid to get vbus status
  *	false	- use bvalid to get vbus status
  * @vbus_attached: otg device vbus status.
+ * @force_bvalid: force the control of the B-device session valid bit.
  * @bvalid_irq: IRQ number assigned for vbus valid rise detection.
  * @ls_irq: IRQ number assigned for linestate detection.
  * @otg_mux_irq: IRQ number which multiplex otg-id/otg-bvalid/linestate
@@ -191,6 +194,7 @@ struct rockchip_usb2phy_port {
 	bool		suspended;
 	bool		utmi_avalid;
 	bool		vbus_attached;
+	bool		force_bvalid;
 	int		bvalid_irq;
 	int		ls_irq;
 	int		otg_mux_irq;
@@ -561,6 +565,13 @@ static void rockchip_usb2phy_otg_sm_work(struct work_struct *work)
 	switch (rport->state) {
 	case OTG_STATE_UNDEFINED:
 		rport->state = OTG_STATE_B_IDLE;
+		if (rport->force_bvalid) {
+			property_enable(rphy->grf,
+					&rport->port_cfg->bvalid_session,
+					true);
+			dev_dbg(&rport->phy->dev,
+				"set the B-Device Session Valid\n");
+		}
 		if (!vbus_attach)
 			rockchip_usb2phy_power_off(rport->phy);
 		/* fall through */
@@ -568,6 +579,14 @@ static void rockchip_usb2phy_otg_sm_work(struct work_struct *work)
 		if (extcon_get_state(rphy->edev, EXTCON_USB_HOST) > 0) {
 			dev_dbg(&rport->phy->dev, "usb otg host connect\n");
 			rport->state = OTG_STATE_A_HOST;
+			/* When leaving device mode force end the session */
+			if (rport->force_bvalid) {
+				property_enable(rphy->grf,
+					&rport->port_cfg->bvalid_session,
+					false);
+				dev_dbg(&rport->phy->dev,
+					"clear the B-Device Session Valid\n");
+			}
 			rockchip_usb2phy_power_on(rport->phy);
 			return;
 		} else if (vbus_attach) {
@@ -642,6 +661,14 @@ static void rockchip_usb2phy_otg_sm_work(struct work_struct *work)
 		if (extcon_get_state(rphy->edev, EXTCON_USB_HOST) == 0) {
 			dev_dbg(&rport->phy->dev, "usb otg host disconnect\n");
 			rport->state = OTG_STATE_B_IDLE;
+			/* When leaving host mode force start the session */
+			if (rport->force_bvalid) {
+				property_enable(rphy->grf,
+					&rport->port_cfg->bvalid_session,
+					true);
+				dev_dbg(&rport->phy->dev,
+					"set the B-Device Session Valid\n");
+			}
 			rockchip_usb2phy_power_off(rport->phy);
 		}
 		break;
@@ -1024,6 +1051,12 @@ static int rockchip_usb2phy_otg_port_init(struct rockchip_usb2phy *rphy,
 	INIT_DELAYED_WORK(&rport->chg_work, rockchip_chg_detect_work);
 	INIT_DELAYED_WORK(&rport->otg_sm_work, rockchip_usb2phy_otg_sm_work);
 
+	rport->force_bvalid = false;
+	if (of_device_is_compatible(rphy->dev->of_node,
+				    "rockchip,rk3399-usb2phy"))
+		rport->force_bvalid = of_property_read_bool(child_np,
+						"rockchip,force-bvalid");
+
 	rport->utmi_avalid =
 		of_property_read_bool(child_np, "rockchip,utmi-avalid");
 
@@ -1349,6 +1382,7 @@ static const struct rockchip_usb2phy_cfg rk3399_phy_cfgs[] = {
 				.bvalid_det_en	= { 0xe3c0, 3, 3, 0, 1 },
 				.bvalid_det_st	= { 0xe3e0, 3, 3, 0, 1 },
 				.bvalid_det_clr	= { 0xe3d0, 3, 3, 0, 1 },
+				.bvalid_session = { 0x4498, 4, 4, 0, 1 },
 				.utmi_avalid	= { 0xe2ac, 7, 7, 0, 1 },
 				.utmi_bvalid	= { 0xe2ac, 12, 12, 0, 1 },
 			},
@@ -1384,6 +1418,7 @@ static const struct rockchip_usb2phy_cfg rk3399_phy_cfgs[] = {
 				.bvalid_det_en  = { 0xe3c0, 8, 8, 0, 1 },
 				.bvalid_det_st  = { 0xe3e0, 8, 8, 0, 1 },
 				.bvalid_det_clr = { 0xe3d0, 8, 8, 0, 1 },
+				.bvalid_session = { 0x4518, 4, 4, 0, 1 },
 				.utmi_avalid	= { 0xe2ac, 10, 10, 0, 1 },
 				.utmi_bvalid    = { 0xe2ac, 16, 16, 0, 1 },
 			},
-- 
2.18.0

[PATCH 4/4] dt-bindings: phy-rockchip-inno-usb2: add new rockchip,force-bvalid property.

[Enric Balletbo i Serra]

This property is used when the otg-id pin is not connected. When this
property is set it forces to set the B-Device Session Valid bit when the
port works as device and clears that bit when the port works as host.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

 .../devicetree/bindings/phy/phy-rockchip-inno-usb2.txt       | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
index 2d4808d3920b..55761f466c41 100644
--- a/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
+++ b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
@@ -48,6 +48,11 @@ Optional properties:
 		See ./phy-bindings.txt for details.
  - rockchip,utmi-avalid : boolean, use the avalid register to get vbus status.
 			  Otherwise, use the bvalid register.
+ - rockchip,force-bvalid : boolean, set this to force the B-Device Session
+			  Valid bit when the usb port is in device mode. This
+			  is used when the otg-id pin is not connected.
+			  Only supported in case of compatible being:
+			  * "rockchip,rk3399-usb2phy"
 
 Example:
 
-- 
2.18.0

Re: [PATCH 3/4] phy: rockchip-inno-usb2: allow to force the B-Device Session Valid bit.

[Heiko Stuebner]

Hi Enric,

Am Mittwoch, 15. August 2018, 11:59:33 CEST schrieb Enric Balletbo i Serra:
> The OTG disconnection event is generated after the presence/abscense of
> an ID connection, but some platforms doesn't have the ID pin connected, so
> the event is not generated. In such case, for detecting the disconnection
> event, we can get the cable state from an extcon driver. We need, though,
> to force to set the B-Device Session Valid bit on the PHY to have the
> device respond to setup address. Otherwise, the following error is
> shown:
> 
>     usb 2-2: Device not responding to setup address.
>     usb 2-2: device not accepting address 14, error -71
>     usb usb2-port2: unable to enumerate USB device
> 
> The patch allows to tell the PHY to force the B-Device Session Valid bit
> when the OTG role is device and clear that bit if the OTG role is host.
> 
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

> @@ -1024,6 +1051,12 @@ static int rockchip_usb2phy_otg_port_init(struct rockchip_usb2phy *rphy,
>  	INIT_DELAYED_WORK(&rport->chg_work, rockchip_chg_detect_work);
>  	INIT_DELAYED_WORK(&rport->otg_sm_work, rockchip_usb2phy_otg_sm_work);
>  
> +	rport->force_bvalid = false;
> +	if (of_device_is_compatible(rphy->dev->of_node,
> +				    "rockchip,rk3399-usb2phy"))
> +		rport->force_bvalid = of_property_read_bool(child_np,
> +						"rockchip,force-bvalid");

That feels a bit clumsy, especially as the rk3399 seems to have the id
connection in general ... maybe you could just make that conditional
on the presence of the extcon?

Or alternatively if needed put that in the soc-specific data struct we
have already instead open-coding soc compatible checks.

Heiko

Re: [PATCH 2/4] dt-bindings: phy-rockchip-inno-usb2: add documentation for extcon and utmi-avalid properties.

[Heiko Stuebner]

Hi Enric,

Am Mittwoch, 15. August 2018, 11:59:32 CEST schrieb Enric Balletbo i Serra:
> Commit 98898f3bc83c8 ("phy: rockchip-inno-usb2: support otg-port for
> rk3399") introduces two new properties. The extcon property is used to
> detect the cable-state, and the rockchip,utmi-avalid is used to indicate
> which register should be used to detect the vbus state.
> 
> Document these properties in the documentation binding.
> 
> Fixes: 98898f3bc83c8 ("phy: rockchip-inno-usb2: support otg-port for rk3399")
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
> 
>  .../devicetree/bindings/phy/phy-rockchip-inno-usb2.txt         | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
> index 074a7b3b0425..2d4808d3920b 100644
> --- a/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
> +++ b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
> @@ -23,6 +23,7 @@ Optional properties:
>  		 register files". When set driver will request its
>  		 phandle as one companion-grf for some special SoCs
>  		 (e.g RV1108).
> + - extcon : phandle to the extcon device for the otg phy.

That should probably mention that this is the extcon providing the cable-state?


>  Required nodes : a sub-node is required for each port the phy provides.
>  		 The sub-node name is used to identify host or otg port,
> @@ -45,6 +46,8 @@ Required properties (port (child) node):
>  Optional properties:
>   - phy-supply : phandle to a regulator that provides power to VBUS.
>  		See ./phy-bindings.txt for details.
> + - rockchip,utmi-avalid : boolean, use the avalid register to get vbus status.
> +			  Otherwise, use the bvalid register.

Not having looked to deeply into the usb2 phy, this might raise questions
on why this is a hardware-description? Is this needed when something is not
connected on the board?


Heiko

Re: [PATCH 1/4] phy: rockchip-inno-usb2: fix misspelling and kernel-doc documentation.

[Heiko Stuebner]

Am Mittwoch, 15. August 2018, 11:59:31 CEST schrieb Enric Balletbo i Serra:
> Fix the typo flase -> false and clean up the kernel-doc documentation in
> phy-rockchip-inno.usb2.c and fix the following warnings when documentation
> is built.
> 
>   :58: warning: missing initial short description
>   :69: warning: cannot understand function prototype: 'enum usb_chg_state '
>   :97: warning: missing initial short description
>   :136: warning: cannot understand function prototype: 'struct rockchip_usb2phy_port_cfg '
>   :157: warning: cannot understand function prototype: 'struct rockchip_usb2phy_cfg '
>   :163: warning: Function parameter or member 'port_cfgs' not described in 'rockchip_usb2phy_cfg'
>   :187: warning: cannot understand function prototype: 'struct rockchip_usb2phy_port '
>   :204: warning: Function parameter or member 'port_cfg' not described in 'rockchip_usb2phy_port'
>   :207: warning: missing initial short description
>   :234: warning: Function parameter or member 'dev' not described in 'rockchip_usb2phy'
>   :234: warning: Function parameter or member 'clk480m_hw' not described in 'rockchip_usb2phy'
> 
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

Reviewed-by: Heiko Stuebner <heiko@sntech.de>

Re: [PATCH 4/4] dt-bindings: phy-rockchip-inno-usb2: add new rockchip,force-bvalid property.

[Heiko Stuebner]

Am Mittwoch, 15. August 2018, 11:59:34 CEST schrieb Enric Balletbo i Serra:
> This property is used when the otg-id pin is not connected. When this
> property is set it forces to set the B-Device Session Valid bit when the
> port works as device and clears that bit when the port works as host.
> 
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
> 
>  .../devicetree/bindings/phy/phy-rockchip-inno-usb2.txt       | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
> index 2d4808d3920b..55761f466c41 100644
> --- a/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
> +++ b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
> @@ -48,6 +48,11 @@ Optional properties:
>  		See ./phy-bindings.txt for details.
>   - rockchip,utmi-avalid : boolean, use the avalid register to get vbus status.
>  			  Otherwise, use the bvalid register.
> + - rockchip,force-bvalid : boolean, set this to force the B-Device Session
> +			  Valid bit when the usb port is in device mode. This
> +			  is used when the otg-id pin is not connected.
> +			  Only supported in case of compatible being:
> +			  * "rockchip,rk3399-usb2phy"

I guess you could drop that rk3399 mention instead make the
driver complain? Trying to keep that list up to date will get hard
and I guess the other socs may very well hide that setting somewhere
in their undocumented phy registers as well.

I guess a bit of alphabetical ordering might also be nice,
rockchip,force-bvalid above rockchip,utmi-avalid
Makes it easier to read ;-)

Heiko

Re: [PATCH 3/4] phy: rockchip-inno-usb2: allow to force the B-Device Session Valid bit.

[Enric Balletbo i Serra]

Hi Heiko,

On 15/08/18 12:18, Heiko Stuebner wrote:
> Hi Enric,
> 
> Am Mittwoch, 15. August 2018, 11:59:33 CEST schrieb Enric Balletbo i Serra:
>> The OTG disconnection event is generated after the presence/abscense of
>> an ID connection, but some platforms doesn't have the ID pin connected, so
>> the event is not generated. In such case, for detecting the disconnection
>> event, we can get the cable state from an extcon driver. We need, though,
>> to force to set the B-Device Session Valid bit on the PHY to have the
>> device respond to setup address. Otherwise, the following error is
>> shown:
>>
>>     usb 2-2: Device not responding to setup address.
>>     usb 2-2: device not accepting address 14, error -71
>>     usb usb2-port2: unable to enumerate USB device
>>
>> The patch allows to tell the PHY to force the B-Device Session Valid bit
>> when the OTG role is device and clear that bit if the OTG role is host.
>>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> 
>> @@ -1024,6 +1051,12 @@ static int rockchip_usb2phy_otg_port_init(struct rockchip_usb2phy *rphy,
>>  	INIT_DELAYED_WORK(&rport->chg_work, rockchip_chg_detect_work);
>>  	INIT_DELAYED_WORK(&rport->otg_sm_work, rockchip_usb2phy_otg_sm_work);
>>  
>> +	rport->force_bvalid = false;
>> +	if (of_device_is_compatible(rphy->dev->of_node,
>> +				    "rockchip,rk3399-usb2phy"))
>> +		rport->force_bvalid = of_property_read_bool(child_np,
>> +						"rockchip,force-bvalid");
> 
> That feels a bit clumsy, especially as the rk3399 seems to have the id

Agree, It is, let me explain :)

Ideally we shouldn't have this check, to get rid of this check I only need the
offsets for bvalid_session register for all the compatibles and fill in phy
configuration data (rk3228_phy_cfgs, &rk3328_phy_cfgs, rk3366_phy_cfgs,
rv1108_phy_cfgs)

To be honest I didn't look if all the datasheets are public available, let me do
some research. Or, if anyone has the datasheet and can tell where the
bvalid_session bit is I can fill all the data.

Best regards,
 Enric

> connection in general ... maybe you could just make that conditional
> on the presence of the extcon?
> 
> Or alternatively if needed put that in the soc-specific data struct we
> have already instead open-coding soc compatible checks.
> 
> Heiko
> 
> 
> 

Re: [PATCH 3/4] phy: rockchip-inno-usb2: allow to force the B-Device Session Valid bit.

[Heiko Stuebner]

Am Mittwoch, 15. August 2018, 12:34:42 CEST schrieb Enric Balletbo i Serra:
> Hi Heiko,
> 
> On 15/08/18 12:18, Heiko Stuebner wrote:
> > Hi Enric,
> > 
> > Am Mittwoch, 15. August 2018, 11:59:33 CEST schrieb Enric Balletbo i Serra:
> >> The OTG disconnection event is generated after the presence/abscense of
> >> an ID connection, but some platforms doesn't have the ID pin connected, so
> >> the event is not generated. In such case, for detecting the disconnection
> >> event, we can get the cable state from an extcon driver. We need, though,
> >> to force to set the B-Device Session Valid bit on the PHY to have the
> >> device respond to setup address. Otherwise, the following error is
> >> shown:
> >>
> >>     usb 2-2: Device not responding to setup address.
> >>     usb 2-2: device not accepting address 14, error -71
> >>     usb usb2-port2: unable to enumerate USB device
> >>
> >> The patch allows to tell the PHY to force the B-Device Session Valid bit
> >> when the OTG role is device and clear that bit if the OTG role is host.
> >>
> >> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> > 
> >> @@ -1024,6 +1051,12 @@ static int rockchip_usb2phy_otg_port_init(struct rockchip_usb2phy *rphy,
> >>  	INIT_DELAYED_WORK(&rport->chg_work, rockchip_chg_detect_work);
> >>  	INIT_DELAYED_WORK(&rport->otg_sm_work, rockchip_usb2phy_otg_sm_work);
> >>  
> >> +	rport->force_bvalid = false;
> >> +	if (of_device_is_compatible(rphy->dev->of_node,
> >> +				    "rockchip,rk3399-usb2phy"))
> >> +		rport->force_bvalid = of_property_read_bool(child_np,
> >> +						"rockchip,force-bvalid");
> > 
> > That feels a bit clumsy, especially as the rk3399 seems to have the id
> 
> Agree, It is, let me explain :)
> 
> Ideally we shouldn't have this check, to get rid of this check I only need the
> offsets for bvalid_session register for all the compatibles and fill in phy
> configuration data (rk3228_phy_cfgs, &rk3328_phy_cfgs, rk3366_phy_cfgs,
> rv1108_phy_cfgs)
> 
> To be honest I didn't look if all the datasheets are public available, let me do
> some research. Or, if anyone has the datasheet and can tell where the
> bvalid_session bit is I can fill all the data.

Or just always check for the presence of the property and just make the
driver warn if that bvalid_session setting is not available for that soc yet.

From the TRMs I have your rk3399 bvalid_session settings seems to live
in the depths of the undocumented inno-phy regs, so that will probably
be the same for other socs using that phy.


Heiko

Re: [PATCH 2/4] dt-bindings: phy-rockchip-inno-usb2: add documentation for extcon and utmi-avalid properties.

[Enric Balletbo i Serra]

Hi Heiko,

On 15/08/18 12:29, Heiko Stuebner wrote:
> Hi Enric,
> 
> Am Mittwoch, 15. August 2018, 11:59:32 CEST schrieb Enric Balletbo i Serra:
>> Commit 98898f3bc83c8 ("phy: rockchip-inno-usb2: support otg-port for
>> rk3399") introduces two new properties. The extcon property is used to
>> detect the cable-state, and the rockchip,utmi-avalid is used to indicate
>> which register should be used to detect the vbus state.
>>
>> Document these properties in the documentation binding.
>>
>> Fixes: 98898f3bc83c8 ("phy: rockchip-inno-usb2: support otg-port for rk3399")
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>> ---
>>
>>  .../devicetree/bindings/phy/phy-rockchip-inno-usb2.txt         | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
>> index 074a7b3b0425..2d4808d3920b 100644
>> --- a/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
>> +++ b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
>> @@ -23,6 +23,7 @@ Optional properties:
>>  		 register files". When set driver will request its
>>  		 phandle as one companion-grf for some special SoCs
>>  		 (e.g RV1108).
>> + - extcon : phandle to the extcon device for the otg phy.
> 
> That should probably mention that this is the extcon providing the cable-state?

ack.

> 
> 
>>  Required nodes : a sub-node is required for each port the phy provides.
>>  		 The sub-node name is used to identify host or otg port,
>> @@ -45,6 +46,8 @@ Required properties (port (child) node):
>>  Optional properties:
>>   - phy-supply : phandle to a regulator that provides power to VBUS.
>>  		See ./phy-bindings.txt for details.
>> + - rockchip,utmi-avalid : boolean, use the avalid register to get vbus status.
>> +			  Otherwise, use the bvalid register.
> 
> Not having looked to deeply into the usb2 phy, this might raise questions
> on why this is a hardware-description? Is this needed when something is not
> connected on the board?

I asked myself the same question and even I thought in just remove that code.

After some investigation, though, I saw that the UTMI+ specification [1] has two
signals similar to ID signal (page 11), the AValid signal is used to indicate if
the session for an A-peripheral is valid and the BValid signal that is used to
indicate if the session for a B-peripheral is valid. I suppose that use of one
or the other matters in some cases, but AFAICT this is not used and I didn't see
any binding using it.

Maybe someone else can give us more clues on the importance or not of this property?

[1] https://www.nxp.com/docs/en/brochure/UTMI-PLUS-SPECIFICATION.pdf

Enric

> 
> 
> Heiko
> 
> 

Re: [PATCH 2/4] dt-bindings: phy-rockchip-inno-usb2: add documentation for extcon and utmi-avalid properties.

[Heiko Stuebner]

Hi Enric,

Am Mittwoch, 15. August 2018, 13:08:00 CEST schrieb Enric Balletbo i Serra:
> On 15/08/18 12:29, Heiko Stuebner wrote:
> > Am Mittwoch, 15. August 2018, 11:59:32 CEST schrieb Enric Balletbo i Serra:
> >> Commit 98898f3bc83c8 ("phy: rockchip-inno-usb2: support otg-port for
> >> rk3399") introduces two new properties. The extcon property is used to
> >> detect the cable-state, and the rockchip,utmi-avalid is used to indicate
> >> which register should be used to detect the vbus state.
> >>
> >> Document these properties in the documentation binding.
> >>
> >> Fixes: 98898f3bc83c8 ("phy: rockchip-inno-usb2: support otg-port for rk3399")
> >> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

[...]

> >> @@ -45,6 +46,8 @@ Required properties (port (child) node):
> >>  Optional properties:
> >>   - phy-supply : phandle to a regulator that provides power to VBUS.
> >>  		See ./phy-bindings.txt for details.
> >> + - rockchip,utmi-avalid : boolean, use the avalid register to get vbus status.
> >> +			  Otherwise, use the bvalid register.
> > 
> > Not having looked to deeply into the usb2 phy, this might raise questions
> > on why this is a hardware-description? Is this needed when something is not
> > connected on the board?
> 
> I asked myself the same question and even I thought in just remove that code.
> 
> After some investigation, though, I saw that the UTMI+ specification [1] has two
> signals similar to ID signal (page 11), the AValid signal is used to indicate if
> the session for an A-peripheral is valid and the BValid signal that is used to
> indicate if the session for a B-peripheral is valid. I suppose that use of one
> or the other matters in some cases, but AFAICT this is not used and I didn't see
> any binding using it.
> 
> Maybe someone else can give us more clues on the importance or not of this property?

so I've looked in mainline, chromeos-4.4 and the Rockchip vendor-kernel
and the only board using that property at all is the rk3399-evb-rev1
and -rev2 in the vendor kernel.

The existence of a further -rev3 (which also looks way better cared for
compared rev1+2) indicates that the older ones are probably some sort
of preproduction models, where some wiring (on the soc or board) may
have gone wrong.

So while I would keep all the avalid settings in the driver, we could just
drop reading that property quietly - as Rob wrote some days ago
"it's only an incompatible change if someone notices" [0] and from the
above it doesn't look like it ;-) .

Heiko

[0] https://www.spinics.net/lists/devicetree/msg243978.html

Re: [PATCH 2/4] dt-bindings: phy-rockchip-inno-usb2: add documentation for extcon and utmi-avalid properties.

[Rob Herring]

On Wed, Aug 15, 2018 at 11:59:32AM +0200, Enric Balletbo i Serra wrote:
> Commit 98898f3bc83c8 ("phy: rockchip-inno-usb2: support otg-port for
> rk3399") introduces two new properties. The extcon property is used to
> detect the cable-state, and the rockchip,utmi-avalid is used to indicate
> which register should be used to detect the vbus state.
> 
> Document these properties in the documentation binding.
> 
> Fixes: 98898f3bc83c8 ("phy: rockchip-inno-usb2: support otg-port for rk3399")
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
> 
>  .../devicetree/bindings/phy/phy-rockchip-inno-usb2.txt         | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
> index 074a7b3b0425..2d4808d3920b 100644
> --- a/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
> +++ b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
> @@ -23,6 +23,7 @@ Optional properties:
>  		 register files". When set driver will request its
>  		 phandle as one companion-grf for some special SoCs
>  		 (e.g RV1108).
> + - extcon : phandle to the extcon device for the otg phy.

extcon is not a good binding. The usb connector binding should be used 
instead for new users.

Rob

Re: [PATCH 4/4] dt-bindings: phy-rockchip-inno-usb2: add new rockchip,force-bvalid property.

[Rob Herring]

On Wed, Aug 15, 2018 at 11:59:34AM +0200, Enric Balletbo i Serra wrote:
> This property is used when the otg-id pin is not connected. When this
> property is set it forces to set the B-Device Session Valid bit when the
> port works as device and clears that bit when the port works as host.
> 
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
> 
>  .../devicetree/bindings/phy/phy-rockchip-inno-usb2.txt       | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
> index 2d4808d3920b..55761f466c41 100644
> --- a/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
> +++ b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
> @@ -48,6 +48,11 @@ Optional properties:
>  		See ./phy-bindings.txt for details.
>   - rockchip,utmi-avalid : boolean, use the avalid register to get vbus status.
>  			  Otherwise, use the bvalid register.
> + - rockchip,force-bvalid : boolean, set this to force the B-Device Session
> +			  Valid bit when the usb port is in device mode. This
> +			  is used when the otg-id pin is not connected.
> +			  Only supported in case of compatible being:
> +			  * "rockchip,rk3399-usb2phy"

Shouldn't you describe the property of the h/w that ID pin is not 
connected, rather than what you do with that information.

Is the pin not connected because it's a connector without ID pin? If so, 
that should be a property of the connector (or inferred from the 
connector compatible string).

Rob

Re: [PATCH 2/4] dt-bindings: phy-rockchip-inno-usb2: add documentation for extcon and utmi-avalid properties.

[Enric Balletbo i Serra]

Hi Rob,

On 16/08/18 00:21, Rob Herring wrote:
> On Wed, Aug 15, 2018 at 11:59:32AM +0200, Enric Balletbo i Serra wrote:
>> Commit 98898f3bc83c8 ("phy: rockchip-inno-usb2: support otg-port for
>> rk3399") introduces two new properties. The extcon property is used to
>> detect the cable-state, and the rockchip,utmi-avalid is used to indicate
>> which register should be used to detect the vbus state.
>>
>> Document these properties in the documentation binding.
>>
>> Fixes: 98898f3bc83c8 ("phy: rockchip-inno-usb2: support otg-port for rk3399")
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>> ---
>>
>>  .../devicetree/bindings/phy/phy-rockchip-inno-usb2.txt         | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
>> index 074a7b3b0425..2d4808d3920b 100644
>> --- a/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
>> +++ b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
>> @@ -23,6 +23,7 @@ Optional properties:
>>  		 register files". When set driver will request its
>>  		 phandle as one companion-grf for some special SoCs
>>  		 (e.g RV1108).
>> + - extcon : phandle to the extcon device for the otg phy.
> 
> extcon is not a good binding. The usb connector binding should be used 
> instead for new users.
> 

Ok, although the extcon thing is implemented in the driver, so I was trying to
just describe how is implemented now, looks like nobody is using it apart from
me (patches not upstreamed yet). I suppose switch to the usb connector binding
will imply some driver modifications. I will take a look.

Thanks,
 Enric

> Rob
> 

Re: [PATCH 1/4] phy: rockchip-inno-usb2: fix misspelling and kernel-doc documentation.

[Enric Balletbo Serra]

Hi Heiko and all,

Missatge de Heiko Stuebner <heiko@sntech.de> del dia dc., 15 d’ag.
2018 a les 12:31:
>
> Am Mittwoch, 15. August 2018, 11:59:31 CEST schrieb Enric Balletbo i Serra:
> > Fix the typo flase -> false and clean up the kernel-doc documentation in
> > phy-rockchip-inno.usb2.c and fix the following warnings when documentation
> > is built.
> >
> >   :58: warning: missing initial short description
> >   :69: warning: cannot understand function prototype: 'enum usb_chg_state '
> >   :97: warning: missing initial short description
> >   :136: warning: cannot understand function prototype: 'struct rockchip_usb2phy_port_cfg '
> >   :157: warning: cannot understand function prototype: 'struct rockchip_usb2phy_cfg '
> >   :163: warning: Function parameter or member 'port_cfgs' not described in 'rockchip_usb2phy_cfg'
> >   :187: warning: cannot understand function prototype: 'struct rockchip_usb2phy_port '
> >   :204: warning: Function parameter or member 'port_cfg' not described in 'rockchip_usb2phy_port'
> >   :207: warning: missing initial short description
> >   :234: warning: Function parameter or member 'dev' not described in 'rockchip_usb2phy'
> >   :234: warning: Function parameter or member 'clk480m_hw' not described in 'rockchip_usb2phy'
> >
> > Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
>

I included this patch in these series because I found the errors when
I was working on this. But this patch doesn't really depend on the
other patches and probably I should have had to send independently. I
am planning to send a new version of these series excluding this
patch. I think that the patch can be already picked (still applies
cleanly and is fine to pick?) or do you prefer I resend?

Thanks,
 Enric

>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 1/4] phy: rockchip-inno-usb2: fix misspelling and kernel-doc documentation.

[Heiko Stübner]

Hi Enric,

Am Dienstag, 8. Januar 2019, 18:35:38 CET schrieb Enric Balletbo Serra:
> Hi Heiko and all,
> 
> Missatge de Heiko Stuebner <heiko@sntech.de> del dia dc., 15 d’ag.
> 
> 2018 a les 12:31:
> > Am Mittwoch, 15. August 2018, 11:59:31 CEST schrieb Enric Balletbo i 
Serra:
> > > Fix the typo flase -> false and clean up the kernel-doc documentation in
> > > phy-rockchip-inno.usb2.c and fix the following warnings when
> > > documentation
> > > is built.
> > > 
> > >   :58: warning: missing initial short description
> > >   :69: warning: cannot understand function prototype: 'enum
> > >   :usb_chg_state '
> > >   :97: warning: missing initial short description
> > >   :136: warning: cannot understand function prototype: 'struct
> > >   :rockchip_usb2phy_port_cfg ' 157: warning: cannot understand function
> > >   :prototype: 'struct rockchip_usb2phy_cfg ' 163: warning: Function
> > >   :parameter or member 'port_cfgs' not described in
> > >   :'rockchip_usb2phy_cfg' 187: warning: cannot understand function
> > >   :prototype: 'struct rockchip_usb2phy_port ' 204: warning: Function
> > >   :parameter or member 'port_cfg' not described in
> > >   :'rockchip_usb2phy_port' 207: warning: missing initial short
> > >   :description
> > >   :234: warning: Function parameter or member 'dev' not described in
> > >   :'rockchip_usb2phy' 234: warning: Function parameter or member
> > >   :'clk480m_hw' not described in 'rockchip_usb2phy'> > 
> > > Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> > 
> > Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> 
> I included this patch in these series because I found the errors when
> I was working on this. But this patch doesn't really depend on the
> other patches and probably I should have had to send independently. I
> am planning to send a new version of these series excluding this
> patch. I think that the patch can be already picked (still applies
> cleanly and is fine to pick?) or do you prefer I resend?

I would resend it.

The patch is from august 2018, so I'd guess Kishon won't have it in his
inbox anymore, so a resend may be easier for everyone.


Heiko

Re: [PATCH 4/4] dt-bindings: phy-rockchip-inno-usb2: add new rockchip,force-bvalid property.

[Enric Balletbo Serra]

Hi Rob,

Missatge de Rob Herring <robh@kernel.org> del dia dj., 16 d’ag. 2018 a les 0:26:
>
> On Wed, Aug 15, 2018 at 11:59:34AM +0200, Enric Balletbo i Serra wrote:
> > This property is used when the otg-id pin is not connected. When this
> > property is set it forces to set the B-Device Session Valid bit when the
> > port works as device and clears that bit when the port works as host.
> >
> > Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> > ---
> >
> >  .../devicetree/bindings/phy/phy-rockchip-inno-usb2.txt       | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
> > index 2d4808d3920b..55761f466c41 100644
> > --- a/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
> > +++ b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
> > @@ -48,6 +48,11 @@ Optional properties:
> >               See ./phy-bindings.txt for details.
> >   - rockchip,utmi-avalid : boolean, use the avalid register to get vbus status.
> >                         Otherwise, use the bvalid register.
> > + - rockchip,force-bvalid : boolean, set this to force the B-Device Session
> > +                       Valid bit when the usb port is in device mode. This
> > +                       is used when the otg-id pin is not connected.
> > +                       Only supported in case of compatible being:
> > +                       * "rockchip,rk3399-usb2phy"
>
> Shouldn't you describe the property of the h/w that ID pin is not
> connected, rather than what you do with that information.
>

What about "rockchip, otg-id-not-connected"?

> Is the pin not connected because it's a connector without ID pin? If so,
> that should be a property of the connector (or inferred from the
> connector compatible string).
>

No, it's not the connector. it is not wired on the phy and the cable
detection is done via an extcon.

Thanks,
 Enric

> Rob
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 4/4] dt-bindings: phy-rockchip-inno-usb2: add new rockchip,force-bvalid property.

[Heiko Stuebner]

Hi Enric,
Am Donnerstag, 10. Januar 2019, 10:06:38 CET schrieb Enric Balletbo Serra:
> Missatge de Rob Herring <robh@kernel.org> del dia dj., 16 d’ag. 2018 a les 0:26:
> >
> > On Wed, Aug 15, 2018 at 11:59:34AM +0200, Enric Balletbo i Serra wrote:
> > > This property is used when the otg-id pin is not connected. When this
> > > property is set it forces to set the B-Device Session Valid bit when the
> > > port works as device and clears that bit when the port works as host.
> > >
> > > Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> > > ---
> > >
> > >  .../devicetree/bindings/phy/phy-rockchip-inno-usb2.txt       | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
> > > index 2d4808d3920b..55761f466c41 100644
> > > --- a/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
> > > +++ b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
> > > @@ -48,6 +48,11 @@ Optional properties:
> > >               See ./phy-bindings.txt for details.
> > >   - rockchip,utmi-avalid : boolean, use the avalid register to get vbus status.
> > >                         Otherwise, use the bvalid register.
> > > + - rockchip,force-bvalid : boolean, set this to force the B-Device Session
> > > +                       Valid bit when the usb port is in device mode. This
> > > +                       is used when the otg-id pin is not connected.
> > > +                       Only supported in case of compatible being:
> > > +                       * "rockchip,rk3399-usb2phy"
> >
> > Shouldn't you describe the property of the h/w that ID pin is not
> > connected, rather than what you do with that information.
> >
> 
> What about "rockchip, otg-id-not-connected"?

just pointing back to our discussion in patch3 about simply assuming
id-not-connected in the case of the extcon missing

I still think that might be the cleaner variant? But I guess you're
probably already looking into doing that as so far you only resend the
cleanup patches :-) .


Heiko