[PATCH 1/3] phy: rockchip-typec: enable usb3 host during usb3 phy power on

From: William wu <redacted>

We have forced usb3 to work in usb2 only mode in firmware by setting
usb3tousb2_en (bit3 of GRF_USB3PHY0/1_CON0) to 1, and setting
host_u3_port_disable (bit0 of GRF_USB3OTG0/1_CON1) to 1 and host_u3_port
(bit15~12 of GRF_USB3OTG0/1_CON1) to 0. So we need to re-enable usb3
host.

Note that the RK3399 TRM suggests that we should keep the whole usb3
controller in reset for the duration of the Type-C PHY initialization.
However, it's hard to assert the reset in the current framework of
reset. And according to the TRM, it doesn't require that we should
clear the usb3tousb2 bit before pipe ready. So let's enable the usb3
host after pipe ready to avoid the Type-C PHY initialization failure.

Signed-off-by: William wu <redacted>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
---
 arch/arm64/boot/dts/rockchip/rk3399.dtsi  |  4 ++++
 drivers/phy/rockchip/phy-rockchip-typec.c | 15 +++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 2f2dee0e2f3c..6c72f8aca74c 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi

@@ -1438,6 +1438,8 @@
 		rockchip,grf = <&grf>;
 		rockchip,typec-conn-dir = <0xe580 0 16>;
 		rockchip,usb3tousb2-en = <0xe580 3 19>;
+		rockchip,usb3-host-disable = <0x2434 0 16>;
+		rockchip,usb3-host-port = <0x2434 12 28>;
 		rockchip,external-psm = <0xe588 14 30>;
 		rockchip,pipe-status = <0xe5c0 0 0>;
 		rockchip,uphy-dp-sel = <0x6268 19 19>;

@@ -1468,6 +1470,8 @@
 		rockchip,grf = <&grf>;
 		rockchip,typec-conn-dir = <0xe58c 0 16>;
 		rockchip,usb3tousb2-en = <0xe58c 3 19>;
+		rockchip,usb3-host-disable = <0x2444 0 16>;
+		rockchip,usb3-host-port = <0x2444 12 28>;
 		rockchip,external-psm = <0xe594 14 30>;
 		rockchip,pipe-status = <0xe5c0 16 16>;
 		rockchip,uphy-dp-sel = <0x6268 3 19>;

diff --git a/drivers/phy/rockchip/phy-rockchip-typec.c b/drivers/phy/rockchip/phy-rockchip-typec.c
index 3ed44bee0fdc..cc07f528cc48 100644
--- a/drivers/phy/rockchip/phy-rockchip-typec.c
+++ b/drivers/phy/rockchip/phy-rockchip-typec.c

@@ -358,6 +358,8 @@ struct usb3phy_reg {
 struct rockchip_usb3phy_port_cfg {
 	struct usb3phy_reg typec_conn_dir;
 	struct usb3phy_reg usb3tousb2_en;
+	struct usb3phy_reg usb3_host_disable;
+	struct usb3phy_reg usb3_host_port;
 	struct usb3phy_reg external_psm;
 	struct usb3phy_reg pipe_status;
 	struct usb3phy_reg uphy_dp_sel;

@@ -853,6 +855,9 @@ static int rockchip_usb3_phy_power_on(struct phy *phy)
 		regmap_read(tcphy->grf_regs, reg->offset, &val);
 		if (!(val & BIT(reg->enable_bit))) {
 			tcphy->mode |= new_mode & (MODE_DFP_USB | MODE_UFP_USB);
+			/* enable usb3 host */
+			property_enable(tcphy, &cfg->usb3_host_disable, 0);
+			property_enable(tcphy, &cfg->usb3_host_port, 1);
 			goto unlock_ret;
 		}
 		usleep_range(10, 20);

@@ -1023,6 +1028,16 @@ static int tcphy_parse_dt(struct rockchip_typec_phy *tcphy,
 	if (ret)
 		return ret;
 
+	ret = tcphy_get_param(dev, &cfg->usb3_host_disable,
+			      "rockchip,usb3-host-disable");
+	if (ret)
+		return ret;
+
+	ret = tcphy_get_param(dev, &cfg->usb3_host_port,
+			      "rockchip,usb3-host-port");
+	if (ret)
+		return ret;
+
 	ret = tcphy_get_param(dev, &cfg->external_psm,
 			      "rockchip,external-psm");
 	if (ret)

-- 
2.15.1

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

From: Chris Zhong <redacted>

The usb3tousb2_en BIT will be clear to 0 in probe(), it make USB
controller work at USB3 mode, and if the USB phy is turned on with DP
only mode(4 lanes DP), the rockchip_usb3_phy_power_on() will return
directly, so usb3_host_disable and usb3_host_port these 2 BIT will keep
a same value as coreboot. In coreboot, these 3 BITs are set as USB2
mode, but now one of the bits is changed to USB3, it make USB controller
work at a unknown status.

These 3 BITs should be changed to USB2, if the Type-C works at 4 lanes
mode, and then switch it back to USB3 mode, when USB disconnect.

Signed-off-by: Chris Zhong <redacted>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
---
 drivers/phy/rockchip/phy-rockchip-typec.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-typec.c b/drivers/phy/rockchip/phy-rockchip-typec.c
index cc07f528cc48..087c754caf25 100644
--- a/drivers/phy/rockchip/phy-rockchip-typec.c
+++ b/drivers/phy/rockchip/phy-rockchip-typec.c

@@ -821,6 +821,18 @@ static int tcphy_get_mode(struct rockchip_typec_phy *tcphy)
 	return mode;
 }
 
+static int tcphy_cfg_usb3_to_usb2_only(struct rockchip_typec_phy *tcphy,
+				       bool value)
+{
+	struct rockchip_usb3phy_port_cfg *cfg = &tcphy->port_cfgs;
+
+	property_enable(tcphy, &cfg->usb3tousb2_en, value);
+	property_enable(tcphy, &cfg->usb3_host_disable, value);
+	property_enable(tcphy, &cfg->usb3_host_port, !value);
+
+	return 0;
+}
+
 static int rockchip_usb3_phy_power_on(struct phy *phy)
 {
 	struct rockchip_typec_phy *tcphy = phy_get_drvdata(phy);

@@ -838,8 +850,10 @@ static int rockchip_usb3_phy_power_on(struct phy *phy)
 	}
 
 	/* DP-only mode; fall back to USB2 */
-	if (!(new_mode & (MODE_DFP_USB | MODE_UFP_USB)))
+	if (!(new_mode & (MODE_DFP_USB | MODE_UFP_USB))) {
+		tcphy_cfg_usb3_to_usb2_only(tcphy, true);
 		goto unlock_ret;
+	}
 
 	if (tcphy->mode == new_mode)
 		goto unlock_ret;

@@ -855,9 +869,9 @@ static int rockchip_usb3_phy_power_on(struct phy *phy)
 		regmap_read(tcphy->grf_regs, reg->offset, &val);
 		if (!(val & BIT(reg->enable_bit))) {
 			tcphy->mode |= new_mode & (MODE_DFP_USB | MODE_UFP_USB);
+
 			/* enable usb3 host */
-			property_enable(tcphy, &cfg->usb3_host_disable, 0);
-			property_enable(tcphy, &cfg->usb3_host_port, 1);
+			tcphy_cfg_usb3_to_usb2_only(tcphy, false);
 			goto unlock_ret;
 		}
 		usleep_range(10, 20);

@@ -878,6 +892,7 @@ static int rockchip_usb3_phy_power_off(struct phy *phy)
 	struct rockchip_typec_phy *tcphy = phy_get_drvdata(phy);
 
 	mutex_lock(&tcphy->lock);
+	tcphy_cfg_usb3_to_usb2_only(tcphy, false);
 
 	if (tcphy->mode == MODE_DISCONNECT)
 		goto unlock;

-- 
2.15.1

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

From: William wu <redacted>

rockchip,usb3-host-disable is the register of type-c phy disable usb3 host
rockchip,usb3-host-port is the register of type-c phy usb3 port number

Signed-off-by: William wu <redacted>
Signed-off-by: Enric Balletbo i Serra <redacted>
---
 Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt b/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt
index c3be83be9615..9085d95d0079 100644
--- a/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt
+++ b/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt

@@ -36,6 +36,12 @@ offset, enable bit, write mask bit.
  - rockchip,uphy-dp-sel : the register of type-c phy enable DP function
    for type-c phy0, it must be <0x6268 19 19>;
    for type-c phy1, it must be <0x6268 3 19>;
+ - rockchip,usb3-host-disable : the register of type-c phy disable usb3 host
+   for type-c phy0, it must be <0x2434 0 16>;
+   for type-c phy1, it must be <0x2444 0 16>;
+ - rockchip,usb3-host-port : the register of type-c phy usb3 port number
+   for type-c phy0, it must be <0x2434 12 28>;
+   for type-c phy1, it must be <0x2444 12 28>;
 
 Required nodes : a sub-node is required for each port the phy provides.
 		 The sub-node name is used to identify dp or usb3 port,

-- 
2.15.1

On Thu, Feb 8, 2018 at 9:20 AM, Enric Balletbo i Serra
[off-list ref] wrote:

When does this list stop? Adding properties for various register
fields doesn't scale. This information should be in the driver and
based on the compatible string if necessary.

Rob

Hi Rob,

2018-02-08 18:52 GMT+01:00 Rob Herring [off-list ref]:

On Thu, Feb 8, 2018 at 9:20 AM, Enric Balletbo i Serra
[off-list ref] wrote:

quoted

From: William wu <redacted>

rockchip,usb3-host-disable is the register of type-c phy disable usb3 host
rockchip,usb3-host-port is the register of type-c phy usb3 port number

Signed-off-by: William wu <redacted>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
---
 Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt b/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt
index c3be83be9615..9085d95d0079 100644
--- a/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt
+++ b/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt

@@ -36,6 +36,12 @@ offset, enable bit, write mask bit.
  - rockchip,uphy-dp-sel : the register of type-c phy enable DP function
    for type-c phy0, it must be <0x6268 19 19>;
    for type-c phy1, it must be <0x6268 3 19>;
+ - rockchip,usb3-host-disable : the register of type-c phy disable usb3 host
+   for type-c phy0, it must be <0x2434 0 16>;
+   for type-c phy1, it must be <0x2444 0 16>;
+ - rockchip,usb3-host-port : the register of type-c phy usb3 port number
+   for type-c phy0, it must be <0x2434 12 28>;
+   for type-c phy1, it must be <0x2444 12 28>;

When does this list stop? Adding properties for various register
fields doesn't scale. This information should be in the driver and
based on the compatible string if necessary.

I see, seams reasonable to me, is this applicable to the new ones only
or I should get rid of all the proprieties like this from the DT
(including the old ones)?

Thanks,
  Enric

Rob

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

On Thu, Feb 8, 2018 at 3:23 PM, Enric Balletbo Serra
[off-list ref] wrote:

Hi Rob,

2018-02-08 18:52 GMT+01:00 Rob Herring [off-list ref]:

quoted

On Thu, Feb 8, 2018 at 9:20 AM, Enric Balletbo i Serra
[off-list ref] wrote:

quoted

From: William wu <redacted>

rockchip,usb3-host-disable is the register of type-c phy disable usb3 host
rockchip,usb3-host-port is the register of type-c phy usb3 port number

Signed-off-by: William wu <redacted>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
---
 Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt b/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt
index c3be83be9615..9085d95d0079 100644
--- a/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt
+++ b/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt

@@ -36,6 +36,12 @@ offset, enable bit, write mask bit.
  - rockchip,uphy-dp-sel : the register of type-c phy enable DP function
    for type-c phy0, it must be <0x6268 19 19>;
    for type-c phy1, it must be <0x6268 3 19>;
+ - rockchip,usb3-host-disable : the register of type-c phy disable usb3 host
+   for type-c phy0, it must be <0x2434 0 16>;
+   for type-c phy1, it must be <0x2444 0 16>;
+ - rockchip,usb3-host-port : the register of type-c phy usb3 port number
+   for type-c phy0, it must be <0x2434 12 28>;
+   for type-c phy1, it must be <0x2444 12 28>;

When does this list stop? Adding properties for various register
fields doesn't scale. This information should be in the driver and
based on the compatible string if necessary.

I see, seams reasonable to me, is this applicable to the new ones only
or I should get rid of all the proprieties like this from the DT
(including the old ones)?

We're already kind of stuck with the existing ones. So it depends if
people want to phase them out or not.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hi,

On Mon, Feb 12, 2018 at 10:43:41AM -0600, Rob Herring wrote:

On Thu, Feb 8, 2018 at 3:23 PM, Enric Balletbo Serra
[off-list ref] wrote:

quoted

2018-02-08 18:52 GMT+01:00 Rob Herring [off-list ref]:

quoted

On Thu, Feb 8, 2018 at 9:20 AM, Enric Balletbo i Serra
[off-list ref] wrote:

quoted

--- a/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt
+++ b/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt

@@ -36,6 +36,12 @@ offset, enable bit, write mask bit.
  - rockchip,uphy-dp-sel : the register of type-c phy enable DP function
    for type-c phy0, it must be <0x6268 19 19>;
    for type-c phy1, it must be <0x6268 3 19>;
+ - rockchip,usb3-host-disable : the register of type-c phy disable usb3 host
+   for type-c phy0, it must be <0x2434 0 16>;
+   for type-c phy1, it must be <0x2444 0 16>;
+ - rockchip,usb3-host-port : the register of type-c phy usb3 port number
+   for type-c phy0, it must be <0x2434 12 28>;
+   for type-c phy1, it must be <0x2444 12 28>;

When does this list stop? Adding properties for various register
fields doesn't scale. This information should be in the driver and
based on the compatible string if necessary.

I see, seams reasonable to me, is this applicable to the new ones only
or I should get rid of all the proprieties like this from the DT
(including the old ones)?

We're already kind of stuck with the existing ones. So it depends if
people want to phase them out or not.

FWIW, any Chrome{device} using these sort of bindings is perfectly
capable of handling changed bindings (we ship DTBs with the kernel). But
that's not typically how mainline covers binding deprecation.

If we're going to start recommending not putting these offsets in the
DT, I'd vote for deprecating them, for consistency. (Otherwise, we'll
keep running into this same question.) We only documented the RK3399
("rockchip,rk3399-typec-phy") binding, so all users should have the same
offsets. I dunno if/how we pick a time for eventually removing the
bindings entirely.

Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Mon, Feb 12, 2018 at 3:26 PM, Brian Norris [off-list ref] wrote:

Hi,

On Mon, Feb 12, 2018 at 10:43:41AM -0600, Rob Herring wrote:

quoted

On Thu, Feb 8, 2018 at 3:23 PM, Enric Balletbo Serra
[off-list ref] wrote:

quoted

2018-02-08 18:52 GMT+01:00 Rob Herring [off-list ref]:

quoted

On Thu, Feb 8, 2018 at 9:20 AM, Enric Balletbo i Serra
[off-list ref] wrote:

quoted

--- a/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt
+++ b/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt

@@ -36,6 +36,12 @@ offset, enable bit, write mask bit.
  - rockchip,uphy-dp-sel : the register of type-c phy enable DP function
    for type-c phy0, it must be <0x6268 19 19>;
    for type-c phy1, it must be <0x6268 3 19>;
+ - rockchip,usb3-host-disable : the register of type-c phy disable usb3 host
+   for type-c phy0, it must be <0x2434 0 16>;
+   for type-c phy1, it must be <0x2444 0 16>;
+ - rockchip,usb3-host-port : the register of type-c phy usb3 port number
+   for type-c phy0, it must be <0x2434 12 28>;
+   for type-c phy1, it must be <0x2444 12 28>;

When does this list stop? Adding properties for various register
fields doesn't scale. This information should be in the driver and
based on the compatible string if necessary.

I see, seams reasonable to me, is this applicable to the new ones only
or I should get rid of all the proprieties like this from the DT
(including the old ones)?

We're already kind of stuck with the existing ones. So it depends if
people want to phase them out or not.

FWIW, any Chrome{device} using these sort of bindings is perfectly
capable of handling changed bindings (we ship DTBs with the kernel). But
that's not typically how mainline covers binding deprecation.

If it's CrOS only that's using these, then it's really up to you all.
I guess it depends if many folks are trying to run mainline on CrOS
devices and don't necessarily keep things in sync.

If we're going to start recommending not putting these offsets in the
DT, I'd vote for deprecating them, for consistency. (Otherwise, we'll
keep running into this same question.) We only documented the RK3399
("rockchip,rk3399-typec-phy") binding, so all users should have the same
offsets. I dunno if/how we pick a time for eventually removing the
bindings entirely.

Yes, makes sense.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On 12 February 2018 at 23:29, Rob Herring [off-list ref] wrote:

On Mon, Feb 12, 2018 at 3:26 PM, Brian Norris [off-list ref] wrote:

quoted

Hi,

On Mon, Feb 12, 2018 at 10:43:41AM -0600, Rob Herring wrote:

quoted

On Thu, Feb 8, 2018 at 3:23 PM, Enric Balletbo Serra
[off-list ref] wrote:

quoted

2018-02-08 18:52 GMT+01:00 Rob Herring [off-list ref]:

quoted

On Thu, Feb 8, 2018 at 9:20 AM, Enric Balletbo i Serra
[off-list ref] wrote:

quoted

--- a/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt
+++ b/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt

@@ -36,6 +36,12 @@ offset, enable bit, write mask bit.
  - rockchip,uphy-dp-sel : the register of type-c phy enable DP function
    for type-c phy0, it must be <0x6268 19 19>;
    for type-c phy1, it must be <0x6268 3 19>;
+ - rockchip,usb3-host-disable : the register of type-c phy disable usb3 host
+   for type-c phy0, it must be <0x2434 0 16>;
+   for type-c phy1, it must be <0x2444 0 16>;
+ - rockchip,usb3-host-port : the register of type-c phy usb3 port number
+   for type-c phy0, it must be <0x2434 12 28>;
+   for type-c phy1, it must be <0x2444 12 28>;

When does this list stop? Adding properties for various register
fields doesn't scale. This information should be in the driver and
based on the compatible string if necessary.

I see, seams reasonable to me, is this applicable to the new ones only
or I should get rid of all the proprieties like this from the DT
(including the old ones)?

We're already kind of stuck with the existing ones. So it depends if
people want to phase them out or not.

FWIW, any Chrome{device} using these sort of bindings is perfectly
capable of handling changed bindings (we ship DTBs with the kernel). But
that's not typically how mainline covers binding deprecation.

If it's CrOS only that's using these, then it's really up to you all.
I guess it depends if many folks are trying to run mainline on CrOS
devices and don't necessarily keep things in sync.

For what it's worth I run mainline on my Chromebook Plus (rk3399-gru-kevin),
but in order to have a somewhat working setup you need to run
4.16-rc1 + various patches from the rockchip mailing list which means
you have to keep up with the latest mainline (both kernel and devicetree)
anyway. So I'm all in favour of cleaning up the devicetree.

quoted

If we're going to start recommending not putting these offsets in the
DT, I'd vote for deprecating them, for consistency. (Otherwise, we'll
keep running into this same question.) We only documented the RK3399
("rockchip,rk3399-typec-phy") binding, so all users should have the same
offsets. I dunno if/how we pick a time for eventually removing the
bindings entirely.

Yes, makes sense.

Rob

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Hi,

2018-02-13 10:18 GMT+01:00 Emil Renner Berthing
[off-list ref]:

On 12 February 2018 at 23:29, Rob Herring [off-list ref] wrote:

quoted

On Mon, Feb 12, 2018 at 3:26 PM, Brian Norris [off-list ref] wrote:

quoted

Hi,

On Mon, Feb 12, 2018 at 10:43:41AM -0600, Rob Herring wrote:

quoted

On Thu, Feb 8, 2018 at 3:23 PM, Enric Balletbo Serra
[off-list ref] wrote:

quoted

2018-02-08 18:52 GMT+01:00 Rob Herring [off-list ref]:

quoted

On Thu, Feb 8, 2018 at 9:20 AM, Enric Balletbo i Serra
[off-list ref] wrote:

quoted

--- a/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt
+++ b/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt

@@ -36,6 +36,12 @@ offset, enable bit, write mask bit.
  - rockchip,uphy-dp-sel : the register of type-c phy enable DP function
    for type-c phy0, it must be <0x6268 19 19>;
    for type-c phy1, it must be <0x6268 3 19>;
+ - rockchip,usb3-host-disable : the register of type-c phy disable usb3 host
+   for type-c phy0, it must be <0x2434 0 16>;
+   for type-c phy1, it must be <0x2444 0 16>;
+ - rockchip,usb3-host-port : the register of type-c phy usb3 port number
+   for type-c phy0, it must be <0x2434 12 28>;
+   for type-c phy1, it must be <0x2444 12 28>;

When does this list stop? Adding properties for various register
fields doesn't scale. This information should be in the driver and
based on the compatible string if necessary.

I see, seams reasonable to me, is this applicable to the new ones only
or I should get rid of all the proprieties like this from the DT
(including the old ones)?

We're already kind of stuck with the existing ones. So it depends if
people want to phase them out or not.

FWIW, any Chrome{device} using these sort of bindings is perfectly
capable of handling changed bindings (we ship DTBs with the kernel). But
that's not typically how mainline covers binding deprecation.

If it's CrOS only that's using these, then it's really up to you all.
I guess it depends if many folks are trying to run mainline on CrOS
devices and don't necessarily keep things in sync.

For what it's worth I run mainline on my Chromebook Plus (rk3399-gru-kevin),
but in order to have a somewhat working setup you need to run
4.16-rc1 + various patches from the rockchip mailing list which means
you have to keep up with the latest mainline (both kernel and devicetree)
anyway. So I'm all in favour of cleaning up the devicetree.

quoted

quoted

If we're going to start recommending not putting these offsets in the
DT, I'd vote for deprecating them, for consistency. (Otherwise, we'll
keep running into this same question.) We only documented the RK3399
("rockchip,rk3399-typec-phy") binding, so all users should have the same
offsets. I dunno if/how we pick a time for eventually removing the
bindings entirely.

Yes, makes sense.

One question, maybe silly question, that comes to my mind is, as the
offsets for same register are different between type-c phy0 and type-c
phy1 and there is two instances, the driver needs to know which type-c
phyter is and I'm not sure the proper way to do it. It is just check
the type-c phyter base address? So if base address is 0xff7c0000
(phy0) we know that we should apply the offsets for phy0 and if base
address is 0xff800000 we know that we should apply the offsets for
phy1?

Best regards,
 Enric

quoted

Rob

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

Hi Enric,

Am Dienstag, 13. Februar 2018, 23:08:26 CET schrieb Enric Balletbo Serra:

2018-02-13 10:18 GMT+01:00 Emil Renner Berthing

[off-list ref]:

quoted

On 12 February 2018 at 23:29, Rob Herring [off-list ref] wrote:

quoted

On Mon, Feb 12, 2018 at 3:26 PM, Brian Norris [off-list ref] 

wrote:

quoted

quoted

quoted

Hi,

On Mon, Feb 12, 2018 at 10:43:41AM -0600, Rob Herring wrote:

quoted

On Thu, Feb 8, 2018 at 3:23 PM, Enric Balletbo Serra

[off-list ref] wrote:

quoted

2018-02-08 18:52 GMT+01:00 Rob Herring [off-list ref]:

quoted

On Thu, Feb 8, 2018 at 9:20 AM, Enric Balletbo i Serra

[off-list ref] wrote:

quoted

--- a/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt
+++ b/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt

@@ -36,6 +36,12 @@ offset, enable bit, write mask bit.

  - rockchip,uphy-dp-sel : the register of type-c phy enable DP
  function
  
    for type-c phy0, it must be <0x6268 19 19>;
    for type-c phy1, it must be <0x6268 3 19>;

+ - rockchip,usb3-host-disable : the register of type-c phy disable

usb3 host +   for type-c phy0, it must be <0x2434 0 16>;
+   for type-c phy1, it must be <0x2444 0 16>;
+ - rockchip,usb3-host-port : the register of type-c phy usb3 port
number
+   for type-c phy0, it must be <0x2434 12 28>;
+   for type-c phy1, it must be <0x2444 12 28>;

When does this list stop? Adding properties for various register
fields doesn't scale. This information should be in the driver and
based on the compatible string if necessary.

I see, seams reasonable to me, is this applicable to the new ones
only
or I should get rid of all the proprieties like this from the DT
(including the old ones)?

We're already kind of stuck with the existing ones. So it depends if
people want to phase them out or not.

FWIW, any Chrome{device} using these sort of bindings is perfectly
capable of handling changed bindings (we ship DTBs with the kernel). But
that's not typically how mainline covers binding deprecation.

If it's CrOS only that's using these, then it's really up to you all.
I guess it depends if many folks are trying to run mainline on CrOS
devices and don't necessarily keep things in sync.

For what it's worth I run mainline on my Chromebook Plus
(rk3399-gru-kevin), but in order to have a somewhat working setup you
need to run
4.16-rc1 + various patches from the rockchip mailing list which means
you have to keep up with the latest mainline (both kernel and devicetree)
anyway. So I'm all in favour of cleaning up the devicetree.

quoted

quoted

If we're going to start recommending not putting these offsets in the
DT, I'd vote for deprecating them, for consistency. (Otherwise, we'll
keep running into this same question.) We only documented the RK3399
("rockchip,rk3399-typec-phy") binding, so all users should have the same
offsets. I dunno if/how we pick a time for eventually removing the
bindings entirely.

Yes, makes sense.

One question, maybe silly question, that comes to my mind is, as the
offsets for same register are different between type-c phy0 and type-c
phy1 and there is two instances, the driver needs to know which type-c
phyter is and I'm not sure the proper way to do it. It is just check
the type-c phyter base address? So if base address is 0xff7c0000
(phy0) we know that we should apply the offsets for phy0 and if base
address is 0xff800000 we know that we should apply the offsets for
phy1?

sounds reasonable and we already did something similar for example
for the inno-usb2 phys where you can find the struct rockchip_usb2phy_cfg
matching against a reg property. GRF reg offset in that case but
matching against the base address for the type-c phy should therefore
be fine as well.


Heiko
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html