[v2,03/12] staging: typec: tcpci: support port config passed via dt

[v2,03/12] staging: typec: tcpci: support port config passed via dt

[Jun Li]

User can define the typec port properties in tcpci node to setup
the port config.

Signed-off-by: Li Jun <jun.li@nxp.com>
---
Changes for v2:
- Use infra APIs to get sink and source config.
- Improve the error message.

 drivers/staging/typec/tcpci.c | 70 ++++++++++++++++++++++++++++++++++++++-----
 include/linux/usb/tcpm.h      |  6 ++--
 2 files changed, 66 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c
index b6abaf7..be6ed16 100644
--- a/drivers/staging/typec/tcpci.c
+++ b/drivers/staging/typec/tcpci.c
@@ -426,17 +426,73 @@ static const struct regmap_config tcpci_regmap_config = {
 	.max_register = 0x7F, /* 0x80 .. 0xFF are vendor defined */
 };
 
-static const struct tcpc_config tcpci_tcpc_config = {
-	.type = TYPEC_PORT_DFP,
-	.default_role = TYPEC_SINK,
-};
-
+/* Populate struct tcpc_config from device-tree */
 static int tcpci_parse_config(struct tcpci *tcpci)
 {
+	struct tcpc_config *tcfg;
+	struct device_node *child;
+	int ret = -EINVAL;
+
 	tcpci->controls_vbus = true; /* XXX */
 
-	/* TODO: Populate struct tcpc_config from ACPI/device-tree */
-	tcpci->tcpc.config = &tcpci_tcpc_config;
+	tcpci->tcpc.config = devm_kzalloc(tcpci->dev, sizeof(*tcfg),
+					  GFP_KERNEL);
+	if (!tcpci->tcpc.config)
+		return -ENOMEM;
+
+	tcfg = tcpci->tcpc.config;
+
+	child = of_get_child_by_name(tcpci->dev->of_node, "connector");
+	if (!child) {
+		dev_err(tcpci->dev, "failed to get connector node.\n");
+		return -EINVAL;
+	}
+
+	/* Get port-type */
+	ret = typec_of_get_port_type(child);
+	if (ret < 0) {
+		dev_err(tcpci->dev, "failed to get correct port type.\n");
+		return ret;
+	}
+	tcfg->type = ret;
+
+	if (tcfg->type == TYPEC_PORT_UFP)
+		goto sink;
+
+	tcfg->src_pdo = devm_kcalloc(tcpci->dev, PDO_MAX_OBJECTS,
+				     sizeof(*tcfg->src_pdo), GFP_KERNEL);
+	if (!tcfg->src_pdo)
+		return -ENOMEM;
+
+	/* Get source capability */
+	ret = tcpm_of_get_src_config(child, tcfg);
+	if (ret <= 0) {
+		dev_err(tcpci->dev, "failed to get source pdo %d\n", ret);
+		return -EINVAL;
+	}
+
+	if (tcfg->type == TYPEC_PORT_DFP)
+		return 0;
+
+	/* Get the preferred power role for drp */
+	ret = typec_of_get_preferred_role(child);
+	if (ret < 0) {
+		dev_err(tcpci->dev, "failed to get correct preferred role.\n");
+		return ret;
+	}
+	tcfg->default_role = ret;
+sink:
+	tcfg->snk_pdo = devm_kcalloc(tcpci->dev, PDO_MAX_OBJECTS,
+				     sizeof(*tcfg->snk_pdo), GFP_KERNEL);
+	if (!tcfg->snk_pdo)
+		return -ENOMEM;
+
+	/* Get power sink config */
+	ret = tcpm_of_get_snk_config(child, tcfg);
+	if (ret < 0) {
+		dev_err(tcpci->dev, "failed to get sink configs %d\n", ret);
+		return -EINVAL;
+	}
 
 	return 0;
 }
diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
index 962eff1..8e9edb4 100644
--- a/include/linux/usb/tcpm.h
+++ b/include/linux/usb/tcpm.h
@@ -76,10 +76,10 @@ enum tcpm_transmit_type {
  * @alt_modes:	List of supported alternate modes
  */
 struct tcpc_config {
-	const u32 *src_pdo;
+	u32 *src_pdo;
 	unsigned int nr_src_pdo;
 
-	const u32 *snk_pdo;
+	u32 *snk_pdo;
 	unsigned int nr_snk_pdo;
 
 	const u32 *snk_vdo;
@@ -154,7 +154,7 @@ struct tcpc_mux_dev {
  * @mux:	Pointer to multiplexer data
  */
 struct tcpc_dev {
-	const struct tcpc_config *config;
+	struct tcpc_config *config;
 
 	int (*init)(struct tcpc_dev *dev);
 	int (*get_vbus)(struct tcpc_dev *dev);

[v2,03/12] staging: typec: tcpci: support port config passed via dt

[Heikki Krogerus]

Hi,

On Mon, Feb 26, 2018 at 07:49:10PM +0800, Li Jun wrote:
> User can define the typec port properties in tcpci node to setup
> the port config.
> 
> Signed-off-by: Li Jun <jun.li@nxp.com>
> ---
> Changes for v2:
> - Use infra APIs to get sink and source config.
> - Improve the error message.
> 
>  drivers/staging/typec/tcpci.c | 70 ++++++++++++++++++++++++++++++++++++++-----
>  include/linux/usb/tcpm.h      |  6 ++--
>  2 files changed, 66 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c
> index b6abaf7..be6ed16 100644
> --- a/drivers/staging/typec/tcpci.c
> +++ b/drivers/staging/typec/tcpci.c
> @@ -426,17 +426,73 @@ static const struct regmap_config tcpci_regmap_config = {
>  	.max_register = 0x7F, /* 0x80 .. 0xFF are vendor defined */
>  };
>  
> -static const struct tcpc_config tcpci_tcpc_config = {
> -	.type = TYPEC_PORT_DFP,
> -	.default_role = TYPEC_SINK,
> -};
> -
> +/* Populate struct tcpc_config from device-tree */
>  static int tcpci_parse_config(struct tcpci *tcpci)
>  {
> +	struct tcpc_config *tcfg;
> +	struct device_node *child;
> +	int ret = -EINVAL;
> +
>  	tcpci->controls_vbus = true; /* XXX */
>  
> -	/* TODO: Populate struct tcpc_config from ACPI/device-tree */
> -	tcpci->tcpc.config = &tcpci_tcpc_config;
> +	tcpci->tcpc.config = devm_kzalloc(tcpci->dev, sizeof(*tcfg),
> +					  GFP_KERNEL);
> +	if (!tcpci->tcpc.config)
> +		return -ENOMEM;
> +
> +	tcfg = tcpci->tcpc.config;
> +
> +	child = of_get_child_by_name(tcpci->dev->of_node, "connector");
> +	if (!child) {
> +		dev_err(tcpci->dev, "failed to get connector node.\n");
> +		return -EINVAL;
> +	}

Why do you need separate child node for the connector? You will always
have only one connector per tcpc, i.e. the tcpci already represents the
connector and all its capabilities.


Thanks,

[v2,03/12] staging: typec: tcpci: support port config passed via dt

[Jun Li]

Hi
> -----Original Message-----
> From: linux-usb-owner@vger.kernel.org
> [mailto:linux-usb-owner@vger.kernel.org] On Behalf Of Heikki Krogerus
> Sent: 2018年2月26日 22:06
> To: Jun Li <jun.li@nxp.com>
> Cc: gregkh@linuxfoundation.org; robh+dt@kernel.org; linux@roeck-us.net;
> a.hajda@samsung.com; mark.rutland@arm.com; yueyao@google.com;
> Peter Chen <peter.chen@nxp.com>; garsilva@embeddedor.com;
> o_leveque@orange.fr; shufan_lee@richtek.com; linux-usb@vger.kernel.org;
> devicetree@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
> Subject: Re: [PATCH v2 03/12] staging: typec: tcpci: support port config
> passed via dt
> 
> Hi,
> 
> On Mon, Feb 26, 2018 at 07:49:10PM +0800, Li Jun wrote:
> > User can define the typec port properties in tcpci node to setup the
> > port config.
> >
> > Signed-off-by: Li Jun <jun.li@nxp.com>
> > ---
> > Changes for v2:
> > - Use infra APIs to get sink and source config.
> > - Improve the error message.
> >
> >  drivers/staging/typec/tcpci.c | 70
> ++++++++++++++++++++++++++++++++++++++-----
> >  include/linux/usb/tcpm.h      |  6 ++--
> >  2 files changed, 66 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/staging/typec/tcpci.c
> > b/drivers/staging/typec/tcpci.c index b6abaf7..be6ed16 100644
> > --- a/drivers/staging/typec/tcpci.c
> > +++ b/drivers/staging/typec/tcpci.c
> > @@ -426,17 +426,73 @@ static const struct regmap_config
> tcpci_regmap_config = {
> >  	.max_register = 0x7F, /* 0x80 .. 0xFF are vendor defined */  };
> >
> > -static const struct tcpc_config tcpci_tcpc_config = {
> > -	.type = TYPEC_PORT_DFP,
> > -	.default_role = TYPEC_SINK,
> > -};
> > -
> > +/* Populate struct tcpc_config from device-tree */
> >  static int tcpci_parse_config(struct tcpci *tcpci)  {
> > +	struct tcpc_config *tcfg;
> > +	struct device_node *child;
> > +	int ret = -EINVAL;
> > +
> >  	tcpci->controls_vbus = true; /* XXX */
> >
> > -	/* TODO: Populate struct tcpc_config from ACPI/device-tree */
> > -	tcpci->tcpc.config = &tcpci_tcpc_config;
> > +	tcpci->tcpc.config = devm_kzalloc(tcpci->dev, sizeof(*tcfg),
> > +					  GFP_KERNEL);
> > +	if (!tcpci->tcpc.config)
> > +		return -ENOMEM;
> > +
> > +	tcfg = tcpci->tcpc.config;
> > +
> > +	child = of_get_child_by_name(tcpci->dev->of_node, "connector");
> > +	if (!child) {
> > +		dev_err(tcpci->dev, "failed to get connector node.\n");
> > +		return -EINVAL;
> > +	}
> 
> Why do you need separate child node for the connector? You will always
> have only one connector per tcpc, i.e. the tcpci already represents the
> connector and all its capabilities.
> 
This is my original idea, my understanding is Rob expects those properties should
apply for a common usb connector node[1], that way I need add a child node for it,
sorry I didn't make the dt-binding patches come first in this series, please see
patch 10,11.

[1] https://patchwork.kernel.org/patch/10231447/

thanks
Li Jun
> 
> Thanks,
> 
> --
> heikki
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in the
> body of a message to majordomo@vger.kernel.org More majordomo info at
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fvger
> .kernel.org%2Fmajordomo-info.html&data=02%7C01%7Cjun.li%40nxp.com
> %7Ce39f8cdcff664858d9ea08d57d221b7a%7C686ea1d3bc2b4c6fa92cd99c5
> c301635%7C0%7C1%7C636552507819862496&sdata=SgStkXiqwV7RIfXcReB
> vIZpOyLEXKNhvH%2FB6p4KXxsA%3D&reserved=0

[v2,03/12] staging: typec: tcpci: support port config passed via dt

[Heikki Krogerus]

Hi,

On Mon, Feb 26, 2018 at 02:30:53PM +0000, Jun Li wrote:
> > > +	child = of_get_child_by_name(tcpci->dev->of_node, "connector");
> > > +	if (!child) {
> > > +		dev_err(tcpci->dev, "failed to get connector node.\n");
> > > +		return -EINVAL;
> > > +	}
> > 
> > Why do you need separate child node for the connector? You will always
> > have only one connector per tcpc, i.e. the tcpci already represents the
> > connector and all its capabilities.
> > 
> This is my original idea, my understanding is Rob expects those properties should
> apply for a common usb connector node[1], that way I need add a child node for it,
> sorry I didn't make the dt-binding patches come first in this series, please see
> patch 10,11.
> 
> [1] https://patchwork.kernel.org/patch/10231447/

But was the idea really to put properties like the TCPC capabilities
under the usb connector child node? That will force us to extract
the same properties in two different methods in every USB Type-C
driver. One extracting them from DT, and another from other FW
interfaces and build-in properties.

To avoid that, let's just expect to get these properties in the node
for tcpc, not the usb connector child.


Thanks,

[v2,03/12] staging: typec: tcpci: support port config passed via dt

[Jun Li]

Hi

> -----Original Message-----
> From: Heikki Krogerus [mailto:heikki.krogerus@linux.intel.com]
> Sent: 2018年2月27日 19:03
> To: Jun Li <jun.li@nxp.com>
> Cc: gregkh@linuxfoundation.org; robh+dt@kernel.org; linux@roeck-us.net;
> a.hajda@samsung.com; mark.rutland@arm.com; yueyao@google.com;
> Peter Chen <peter.chen@nxp.com>; garsilva@embeddedor.com;
> o_leveque@orange.fr; shufan_lee@richtek.com; linux-usb@vger.kernel.org;
> devicetree@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
> Subject: Re: [PATCH v2 03/12] staging: typec: tcpci: support port config
> passed via dt
> 
> Hi,
> 
> On Mon, Feb 26, 2018 at 02:30:53PM +0000, Jun Li wrote:
> > > > +	child = of_get_child_by_name(tcpci->dev->of_node, "connector");
> > > > +	if (!child) {
> > > > +		dev_err(tcpci->dev, "failed to get connector node.\n");
> > > > +		return -EINVAL;
> > > > +	}
> > >
> > > Why do you need separate child node for the connector? You will
> > > always have only one connector per tcpc, i.e. the tcpci already
> > > represents the connector and all its capabilities.
> > >
> > This is my original idea, my understanding is Rob expects those
> > properties should apply for a common usb connector node[1], that way I
> > need add a child node for it, sorry I didn't make the dt-binding
> > patches come first in this series, please see patch 10,11.
> >
> > [1]
> >
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpat
> >
> chwork.kernel.org%2Fpatch%2F10231447%2F&data=02%7C01%7Cjun.li%40
> nxp.co
> >
> m%7Ce37ed8b084374e241d2e08d57dd1b02a%7C686ea1d3bc2b4c6fa92cd9
> 9c5c30163
> >
> 5%7C0%7C0%7C636553261972212376&sdata=hSNiAfXoTTzK3TjjkjWo7OJL7
> %2B3gDHT
> > I8NO0FQviDd4%3D&reserved=0
> 
> But was the idea really to put properties like the TCPC capabilities under the
> usb connector child node? That will force us to extract the same properties
> in two different methods in every USB Type-C driver. One extracting them
> from DT, and another from other FW interfaces and build-in properties.
> 
> To avoid that, let's just expect to get these properties in the node for tcpc,
> not the usb connector child.

I misunderstood it's better to put typec props under connector node in all cases
so we can have a unified typec description. As Rob comments that's only required
for multiple connectors for one controller, not for simple connector like my case,
I will put those props under tcpc node.

Jun Li
> 
> 
> Thanks,
> 
> --
> heikki

[v2,03/12] staging: typec: tcpci: support port config passed via dt

[Heikki Krogerus]

On Mon, Mar 05, 2018 at 08:53:00AM +0000, Jun Li wrote:
> > On Mon, Feb 26, 2018 at 02:30:53PM +0000, Jun Li wrote:
> > > > > +	child = of_get_child_by_name(tcpci->dev->of_node, "connector");
> > > > > +	if (!child) {
> > > > > +		dev_err(tcpci->dev, "failed to get connector node.\n");
> > > > > +		return -EINVAL;
> > > > > +	}
> > > >
> > > > Why do you need separate child node for the connector? You will
> > > > always have only one connector per tcpc, i.e. the tcpci already
> > > > represents the connector and all its capabilities.
> > > >
> > > This is my original idea, my understanding is Rob expects those
> > > properties should apply for a common usb connector node[1], that way I
> > > need add a child node for it, sorry I didn't make the dt-binding
> > > patches come first in this series, please see patch 10,11.
> > >
> > > [1]
> > >
> > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpat
> > >
> > chwork.kernel.org%2Fpatch%2F10231447%2F&data=02%7C01%7Cjun.li%40
> > nxp.co
> > >
> > m%7Ce37ed8b084374e241d2e08d57dd1b02a%7C686ea1d3bc2b4c6fa92cd9
> > 9c5c30163
> > >
> > 5%7C0%7C0%7C636553261972212376&sdata=hSNiAfXoTTzK3TjjkjWo7OJL7
> > %2B3gDHT
> > > I8NO0FQviDd4%3D&reserved=0
> > 
> > But was the idea really to put properties like the TCPC capabilities under the
> > usb connector child node? That will force us to extract the same properties
> > in two different methods in every USB Type-C driver. One extracting them
> > from DT, and another from other FW interfaces and build-in properties.
> > 
> > To avoid that, let's just expect to get these properties in the node for tcpc,
> > not the usb connector child.
> 
> I misunderstood it's better to put typec props under connector node in all cases
> so we can have a unified typec description. As Rob comments that's only required
> for multiple connectors for one controller, not for simple connector like my case,
> I will put those props under tcpc node.

Hold on! I was not considering multi-port PD/Type-C controllers.

I'm wrong about the child node forcing us to have two methods for
extracting the properties in the drivers. It does mean we can not use
device_property* functions as the child node is not (yet) bind to any
struct device, but we can still use fwnode_property* functions, which
is fine.

So it actually does make sense to define those properties for the
"connector" node instead of TCPC parent. They are generic "Type-C"
properties (right?), so we may want to use them with multiport
devices as well.


Br,

[v2,03/12] staging: typec: tcpci: support port config passed via dt

[Jun Li]

> -----Original Message-----
> From: Heikki Krogerus [mailto:heikki.krogerus@linux.intel.com]
> Sent: 2018年3月5日 17:54
> To: Jun Li <jun.li@nxp.com>
> Cc: gregkh@linuxfoundation.org; robh+dt@kernel.org; linux@roeck-us.net;
> a.hajda@samsung.com; mark.rutland@arm.com; yueyao@google.com;
> Peter Chen <peter.chen@nxp.com>; garsilva@embeddedor.com;
> o_leveque@orange.fr; shufan_lee@richtek.com; linux-usb@vger.kernel.org;
> devicetree@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
> Subject: Re: [PATCH v2 03/12] staging: typec: tcpci: support port config
> passed via dt
> 
> On Mon, Mar 05, 2018 at 08:53:00AM +0000, Jun Li wrote:
> > > On Mon, Feb 26, 2018 at 02:30:53PM +0000, Jun Li wrote:
> > > > > > +	child = of_get_child_by_name(tcpci->dev->of_node,
> "connector");
> > > > > > +	if (!child) {
> > > > > > +		dev_err(tcpci->dev, "failed to get connector node.\n");
> > > > > > +		return -EINVAL;
> > > > > > +	}
> > > > >
> > > > > Why do you need separate child node for the connector? You will
> > > > > always have only one connector per tcpc, i.e. the tcpci already
> > > > > represents the connector and all its capabilities.
> > > > >
> > > > This is my original idea, my understanding is Rob expects those
> > > > properties should apply for a common usb connector node[1], that
> > > > way I need add a child node for it, sorry I didn't make the
> > > > dt-binding patches come first in this series, please see patch 10,11.
> > > >
> > > > [1]
> > > >
> > >
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fp
> > > at
> > > >
> > >
> chwork.kernel.org%2Fpatch%2F10231447%2F&data=02%7C01%7Cjun.li%40
> > > nxp.co
> > > >
> > >
> m%7Ce37ed8b084374e241d2e08d57dd1b02a%7C686ea1d3bc2b4c6fa92cd9
> > > 9c5c30163
> > > >
> > >
> 5%7C0%7C0%7C636553261972212376&sdata=hSNiAfXoTTzK3TjjkjWo7OJL7
> > > %2B3gDHT
> > > > I8NO0FQviDd4%3D&reserved=0
> > >
> > > But was the idea really to put properties like the TCPC capabilities
> > > under the usb connector child node? That will force us to extract
> > > the same properties in two different methods in every USB Type-C
> > > driver. One extracting them from DT, and another from other FW
> interfaces and build-in properties.
> > >
> > > To avoid that, let's just expect to get these properties in the node
> > > for tcpc, not the usb connector child.
> >
> > I misunderstood it's better to put typec props under connector node in
> > all cases so we can have a unified typec description. As Rob comments
> > that's only required for multiple connectors for one controller, not
> > for simple connector like my case, I will put those props under tcpc node.
> 
> Hold on! I was not considering multi-port PD/Type-C controllers.
> 
> I'm wrong about the child node forcing us to have two methods for
> extracting the properties in the drivers. It does mean we can not use
> device_property* functions as the child node is not (yet) bind to any struct
> device, but we can still use fwnode_property* functions, which is fine.
> 

Yes, fwnode_property* function can be used in this case.

> So it actually does make sense to define those properties for the
> "connector" node instead of TCPC parent. They are generic "Type-C"
> properties (right?), so we may want to use them with multiport devices as
> well.
> 

Yes, that's the idea of my v2, I will keep this but via fwnode_property*.

> 
> Br,
> 
> --
> heikki

[v2,03/12] staging: typec: tcpci: support port config passed via dt

[Heikki Krogerus]

Hi,

On Mon, Mar 05, 2018 at 10:35:07AM +0000, Jun Li wrote:
> > So it actually does make sense to define those properties for the
> > "connector" node instead of TCPC parent. They are generic "Type-C"
> > properties (right?), so we may want to use them with multiport devices as
> > well.
> > 
> 
> Yes, that's the idea of my v2, I will keep this but via fwnode_property*.

Cool. While at it, can you also add a patch to this series where the
fwnode is bind to the port? Something like this:



Thanks,

diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c
index 9bd4412356c9..ac4e7605f9d5 100644
--- a/drivers/staging/typec/tcpci.c
+++ b/drivers/staging/typec/tcpci.c
@@ -452,6 +452,8 @@ static int tcpci_probe(struct i2c_client *client,
        if (IS_ERR(tcpci->regmap))
                return PTR_ERR(tcpci->regmap);

+       tcpci->tcpc.fwnode = device_get_named_child_node(&client->dev, "connector");
+
        tcpci->tcpc.init = tcpci_init;
        tcpci->tcpc.get_vbus = tcpci_get_vbus;
        tcpci->tcpc.set_vbus = tcpci_set_vbus;
diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
index f4d563ee7690..68a0ead400c0 100644
--- a/drivers/usb/typec/tcpm.c
+++ b/drivers/usb/typec/tcpm.c
@@ -3729,6 +3729,7 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
        else
                port->try_role = TYPEC_NO_PREFERRED_ROLE;

+       port->typec_caps.fwnode = tcpc->fwnode;
        port->typec_caps.prefer_role = tcpc->config->default_role;
        port->typec_caps.type = tcpc->config->type;
        port->typec_caps.revision = 0x0120;     /* Type-C spec release 1.2 */
diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
index ca1c0b57f03f..a25ebfea054d 100644
--- a/include/linux/usb/tcpm.h
+++ b/include/linux/usb/tcpm.h
@@ -127,6 +127,7 @@ struct tcpc_mux_dev {
 /**
  * struct tcpc_dev - Port configuration and callback functions
  * @config:    Pointer to port configuration
+ * @fwnode:    Pointer to port fwnode
  * @get_vbus:  Called to read current VBUS state
  * @get_current_limit:
  *             Optional; called by the tcpm core when configured as a snk
@@ -155,6 +156,7 @@ struct tcpc_mux_dev {
  */
 struct tcpc_dev {
        const struct tcpc_config *config;
+       struct fwnode_handle *fwnode;

        int (*init)(struct tcpc_dev *dev);
        int (*get_vbus)(struct tcpc_dev *dev);

[v2,03/12] staging: typec: tcpci: support port config passed via dt

[Jun Li]

> -----Original Message-----
> From: Heikki Krogerus [mailto:heikki.krogerus@linux.intel.com]
> Sent: 2018年3月5日 19:30
> To: Jun Li <jun.li@nxp.com>
> Cc: gregkh@linuxfoundation.org; robh+dt@kernel.org; linux@roeck-us.net;
> a.hajda@samsung.com; mark.rutland@arm.com; yueyao@google.com;
> Peter Chen <peter.chen@nxp.com>; garsilva@embeddedor.com;
> o_leveque@orange.fr; shufan_lee@richtek.com; linux-usb@vger.kernel.org;
> devicetree@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
> Subject: Re: [PATCH v2 03/12] staging: typec: tcpci: support port config
> passed via dt
> 
> Hi,
> 
> On Mon, Mar 05, 2018 at 10:35:07AM +0000, Jun Li wrote:
> > > So it actually does make sense to define those properties for the
> > > "connector" node instead of TCPC parent. They are generic "Type-C"
> > > properties (right?), so we may want to use them with multiport
> > > devices as well.
> > >
> >
> > Yes, that's the idea of my v2, I will keep this but via fwnode_property*.
> 
> Cool. While at it, can you also add a patch to this series where the fwnode is
> bind to the port? Something like this:

OK, I will add a patch for this.

> 
> diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c
> index 9bd4412356c9..ac4e7605f9d5 100644
> --- a/drivers/staging/typec/tcpci.c
> +++ b/drivers/staging/typec/tcpci.c
> @@ -452,6 +452,8 @@ static int tcpci_probe(struct i2c_client *client,
>         if (IS_ERR(tcpci->regmap))
>                 return PTR_ERR(tcpci->regmap);
> 
> +       tcpci->tcpc.fwnode =
> device_get_named_child_node(&client->dev,
> + "connector");
> +
>         tcpci->tcpc.init = tcpci_init;
>         tcpci->tcpc.get_vbus = tcpci_get_vbus;
>         tcpci->tcpc.set_vbus = tcpci_set_vbus; diff --git
> a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c index
> f4d563ee7690..68a0ead400c0 100644
> --- a/drivers/usb/typec/tcpm.c
> +++ b/drivers/usb/typec/tcpm.c
> @@ -3729,6 +3729,7 @@ struct tcpm_port *tcpm_register_port(struct
> device *dev, struct tcpc_dev *tcpc)
>         else
>                 port->try_role = TYPEC_NO_PREFERRED_ROLE;
> 
> +       port->typec_caps.fwnode = tcpc->fwnode;
>         port->typec_caps.prefer_role = tcpc->config->default_role;
>         port->typec_caps.type = tcpc->config->type;
>         port->typec_caps.revision = 0x0120;     /* Type-C spec release
> 1.2 */
> diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h index
> ca1c0b57f03f..a25ebfea054d 100644
> --- a/include/linux/usb/tcpm.h
> +++ b/include/linux/usb/tcpm.h
> @@ -127,6 +127,7 @@ struct tcpc_mux_dev {
>  /**
>   * struct tcpc_dev - Port configuration and callback functions
>   * @config:    Pointer to port configuration
> + * @fwnode:    Pointer to port fwnode
>   * @get_vbus:  Called to read current VBUS state
>   * @get_current_limit:
>   *             Optional; called by the tcpm core when configured as a
> snk
> @@ -155,6 +156,7 @@ struct tcpc_mux_dev {
>   */
>  struct tcpc_dev {
>         const struct tcpc_config *config;
> +       struct fwnode_handle *fwnode;
> 
>         int (*init)(struct tcpc_dev *dev);
>         int (*get_vbus)(struct tcpc_dev *dev);
> 
> 
> Thanks,
> 
> --
> heikki