From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
To: ShuFanLee <leechu729@gmail.com>
Cc: linux@roeck-us.net, linux-kernel@vger.kernel.org,
linux-usb@vger.kernel.org, cy_huang@richtek.com,
shufan_lee@richtek.com
Subject: staging: typec: handle vendor defined part and modify drp toggling flow
Date: Wed, 14 Feb 2018 12:56:41 +0200 [thread overview]
Message-ID: <20180214105641.GE1480@kuha.fi.intel.com> (raw)
On Wed, Feb 14, 2018 at 05:24:04PM +0800, ShuFanLee wrote:
> From: ShuFanLee <shufan_lee@richtek.com>
>
> Handle vendor defined behavior in tcpci_init and tcpci_irq.
> More operations can be extended in tcpci_vendor_data if needed.
> According to TCPCI specification, 4.4.5.2 ROLE_CONTROL,
> TCPC shall not start DRP toggling until subsequently the TCPM
> writes to the COMMAND register to start DRP toggling.
> DRP toggling flow is chagned as following:
> - Write DRP = 0 & Rd/Rd
> - Write DRP = 1
> - Set LOOK4CONNECTION command
>
> Signed-off-by: ShuFanLee <shufan_lee@richtek.com>
> ---
> drivers/staging/typec/tcpci.c | 98 ++++++++++++++++++++++++++++++++++++-------
> drivers/staging/typec/tcpci.h | 15 +++++++
> 2 files changed, 97 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c
> index 9bd4412..b3a97b3 100644
> --- a/drivers/staging/typec/tcpci.c
> +++ b/drivers/staging/typec/tcpci.c
> @@ -30,6 +30,7 @@ struct tcpci {
> bool controls_vbus;
>
> struct tcpc_dev tcpc;
> + struct tcpci_vendor_data *vdata;
> };
>
> static inline struct tcpci *tcpc_to_tcpci(struct tcpc_dev *tcpc)
> @@ -37,16 +38,29 @@ static inline struct tcpci *tcpc_to_tcpci(struct tcpc_dev *tcpc)
> return container_of(tcpc, struct tcpci, tcpc);
> }
>
> -static int tcpci_read16(struct tcpci *tcpci, unsigned int reg,
> - u16 *val)
> +int tcpci_read16(struct tcpci *tcpci, unsigned int reg, u16 *val)
> {
> return regmap_raw_read(tcpci->regmap, reg, val, sizeof(u16));
> }
> +EXPORT_SYMBOL_GPL(tcpci_read16);
>
> -static int tcpci_write16(struct tcpci *tcpci, unsigned int reg, u16 val)
> +int tcpci_write16(struct tcpci *tcpci, unsigned int reg, u16 val)
> {
> return regmap_raw_write(tcpci->regmap, reg, &val, sizeof(u16));
> }
> +EXPORT_SYMBOL_GPL(tcpci_write16);
> +
> +int tcpci_read8(struct tcpci *tcpci, unsigned int reg, u8 *val)
> +{
> + return regmap_raw_read(tcpci->regmap, reg, val, sizeof(u8));
> +}
> +EXPORT_SYMBOL_GPL(tcpci_read8);
> +
> +int tcpci_write8(struct tcpci *tcpci, unsigned int reg, u8 val)
> +{
> + return regmap_raw_write(tcpci->regmap, reg, &val, sizeof(u8));
> +}
> +EXPORT_SYMBOL_GPL(tcpci_write8);
I don't think there is any need to export those wrappers. You can
always expect the glue driver to supply the regmap as member of that
struct tcpci_vendor_data. See below..
> static int tcpci_set_cc(struct tcpc_dev *tcpc, enum typec_cc_status cc)
> {
> @@ -98,8 +112,10 @@ static int tcpci_set_cc(struct tcpc_dev *tcpc, enum typec_cc_status cc)
> static int tcpci_start_drp_toggling(struct tcpc_dev *tcpc,
> enum typec_cc_status cc)
> {
> + int ret;
> struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> - unsigned int reg = TCPC_ROLE_CTRL_DRP;
> + unsigned int reg = (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC1_SHIFT) |
> + (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC2_SHIFT);
>
> switch (cc) {
> default:
> @@ -116,8 +132,19 @@ static int tcpci_start_drp_toggling(struct tcpc_dev *tcpc,
> TCPC_ROLE_CTRL_RP_VAL_SHIFT);
> break;
> }
> -
> - return regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg);
> + ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg);
> + if (ret < 0)
> + return ret;
> + usleep_range(500, 1000);
> + reg |= TCPC_ROLE_CTRL_DRP;
> + ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg);
> + if (ret < 0)
> + return ret;
> + ret = regmap_write(tcpci->regmap, TCPC_COMMAND,
> + TCPC_CMD_LOOK4CONNECTION);
> + if (ret < 0)
> + return ret;
> + return 0;
> }
>
> static enum typec_cc_status tcpci_to_typec_cc(unsigned int cc, bool sink)
> @@ -323,6 +350,15 @@ static int tcpci_init(struct tcpc_dev *tcpc)
> if (time_after(jiffies, timeout))
> return -ETIMEDOUT;
>
> + /* Handle vendor init */
> + if (tcpci->vdata) {
> + if (tcpci->vdata->init) {
> + ret = (*tcpci->vdata->init)(tcpci, tcpci->vdata);
ret = tcpci->vdata->init(...
> + if (ret < 0)
> + return ret;
> + }
> + }
> +
> /* Clear all events */
> ret = tcpci_write16(tcpci, TCPC_ALERT, 0xffff);
> if (ret < 0)
> @@ -351,6 +387,13 @@ static irqreturn_t tcpci_irq(int irq, void *dev_id)
>
> tcpci_read16(tcpci, TCPC_ALERT, &status);
>
> + /* Handle vendor defined interrupt */
> + if (tcpci->vdata) {
> + if (tcpci->vdata->irq_handler)
> + (*tcpci->vdata->irq_handler)(tcpci, tcpci->vdata,
> + &status);
Ditto.
> + }
> +
> /*
> * Clear alert status for everything except RX_STATUS, which shouldn't
> * be cleared until we have successfully retrieved message.
> @@ -417,7 +460,7 @@ static irqreturn_t tcpci_irq(int irq, void *dev_id)
> .reg_bits = 8,
> .val_bits = 8,
>
> - .max_register = 0x7F, /* 0x80 .. 0xFF are vendor defined */
> + .max_register = 0xFF, /* 0x80 .. 0xFF are vendor defined */
> };
>
> static const struct tcpc_config tcpci_tcpc_config = {
> @@ -435,22 +478,22 @@ static int tcpci_parse_config(struct tcpci *tcpci)
> return 0;
> }
>
> -static int tcpci_probe(struct i2c_client *client,
> - const struct i2c_device_id *i2c_id)
> +struct tcpci *tcpci_register_port(struct i2c_client *client,
> + struct tcpci_vendor_data *vdata)
struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_...)
{
...
> {
> struct tcpci *tcpci;
> int err;
>
> tcpci = devm_kzalloc(&client->dev, sizeof(*tcpci), GFP_KERNEL);
> if (!tcpci)
> - return -ENOMEM;
> + return ERR_PTR(-ENOMEM);
>
> tcpci->client = client;
struct tcpci does not need that "client" member.
> tcpci->dev = &client->dev;
tcpci->dev = dev;
> - i2c_set_clientdata(client, tcpci);
> + tcpci->vdata = vdata;
> tcpci->regmap = devm_regmap_init_i2c(client, &tcpci_regmap_config);
You would now do that in your glue driver, and here just:
tcpci->regmap = data->regmap;
> tcpci->tcpc.init = tcpci_init;
> tcpci->tcpc.get_vbus = tcpci_get_vbus;
> @@ -467,7 +510,7 @@ static int tcpci_probe(struct i2c_client *client,
>
> err = tcpci_parse_config(tcpci);
> if (err < 0)
> - return err;
> + return ERR_PTR(err);
>
> /* Disable chip interrupts */
> tcpci_write16(tcpci, TCPC_ALERT_MASK, 0);
> @@ -477,17 +520,40 @@ static int tcpci_probe(struct i2c_client *client,
> IRQF_ONESHOT | IRQF_TRIGGER_LOW,
> dev_name(tcpci->dev), tcpci);
The interrupt could also be requested in the glue driver. Then from
the irq routine in your glue driver, you call tcpci_irq(). That would
mean you export tcpci_irq() of course.
> if (err < 0)
> - return err;
> + return ERR_PTR(err);
>
> tcpci->port = tcpm_register_port(tcpci->dev, &tcpci->tcpc);
> - return PTR_ERR_OR_ZERO(tcpci->port);
> + if (PTR_ERR_OR_ZERO(tcpci->port))
> + return ERR_CAST(tcpci->port);
> +
> + return tcpci;
> +}
> +EXPORT_SYMBOL_GPL(tcpci_register_port);
> +
> +void tcpci_unregister_port(struct tcpci *tcpci)
> +{
> + tcpm_unregister_port(tcpci->port);
> +}
> +EXPORT_SYMBOL_GPL(tcpci_unregister_port);
> +
> +static int tcpci_probe(struct i2c_client *client,
> + const struct i2c_device_id *i2c_id)
> +{
> + struct tcpci *tcpci;
> +
> + tcpci = tcpci_register_port(client, NULL);
> + if (PTR_ERR_OR_ZERO(tcpci))
> + return PTR_ERR(tcpci);
> +
> + i2c_set_clientdata(client, tcpci);
> + return 0;
> }
That would then look something like this:
static int tcpci_probe(struct i2c_client *client...
{
struct tcpci_data data; /* s/tcpci_vendor_data/tcpci_data/ */
struct tcpci *tcpci;
data.regmap = devm_regmap_init_i2c(client, &tcpci_regmap_config);
if (IS_ERR(tcpci->regmap))
return PTR_ERR(tcpci->regmap);
/*
* NOTE: everything in struct tcpci_data should be copied in
* tcpci_register_port().
*/
tcpci = tcpci_register_port(&client->dev, &data);
if (IS_ERR(tcpci))
return PTR_ERR(tcpci);
devm_request_threaded_irq(&client->dev, client->irq, NULL,
tcpci_irq,
...
i2c_set_clientdata(client, tcpci);
return 0;
}
Br,
WARNING: multiple messages have this Message-ID (diff)
From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
To: ShuFanLee <leechu729@gmail.com>
Cc: linux@roeck-us.net, linux-kernel@vger.kernel.org,
linux-usb@vger.kernel.org, cy_huang@richtek.com,
shufan_lee@richtek.com
Subject: Re: [PATCH] staging: typec: handle vendor defined part and modify drp toggling flow
Date: Wed, 14 Feb 2018 12:56:41 +0200 [thread overview]
Message-ID: <20180214105641.GE1480@kuha.fi.intel.com> (raw)
In-Reply-To: <1518600244-29949-1-git-send-email-leechu729@gmail.com>
On Wed, Feb 14, 2018 at 05:24:04PM +0800, ShuFanLee wrote:
> From: ShuFanLee <shufan_lee@richtek.com>
>
> Handle vendor defined behavior in tcpci_init and tcpci_irq.
> More operations can be extended in tcpci_vendor_data if needed.
> According to TCPCI specification, 4.4.5.2 ROLE_CONTROL,
> TCPC shall not start DRP toggling until subsequently the TCPM
> writes to the COMMAND register to start DRP toggling.
> DRP toggling flow is chagned as following:
> - Write DRP = 0 & Rd/Rd
> - Write DRP = 1
> - Set LOOK4CONNECTION command
>
> Signed-off-by: ShuFanLee <shufan_lee@richtek.com>
> ---
> drivers/staging/typec/tcpci.c | 98 ++++++++++++++++++++++++++++++++++++-------
> drivers/staging/typec/tcpci.h | 15 +++++++
> 2 files changed, 97 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c
> index 9bd4412..b3a97b3 100644
> --- a/drivers/staging/typec/tcpci.c
> +++ b/drivers/staging/typec/tcpci.c
> @@ -30,6 +30,7 @@ struct tcpci {
> bool controls_vbus;
>
> struct tcpc_dev tcpc;
> + struct tcpci_vendor_data *vdata;
> };
>
> static inline struct tcpci *tcpc_to_tcpci(struct tcpc_dev *tcpc)
> @@ -37,16 +38,29 @@ static inline struct tcpci *tcpc_to_tcpci(struct tcpc_dev *tcpc)
> return container_of(tcpc, struct tcpci, tcpc);
> }
>
> -static int tcpci_read16(struct tcpci *tcpci, unsigned int reg,
> - u16 *val)
> +int tcpci_read16(struct tcpci *tcpci, unsigned int reg, u16 *val)
> {
> return regmap_raw_read(tcpci->regmap, reg, val, sizeof(u16));
> }
> +EXPORT_SYMBOL_GPL(tcpci_read16);
>
> -static int tcpci_write16(struct tcpci *tcpci, unsigned int reg, u16 val)
> +int tcpci_write16(struct tcpci *tcpci, unsigned int reg, u16 val)
> {
> return regmap_raw_write(tcpci->regmap, reg, &val, sizeof(u16));
> }
> +EXPORT_SYMBOL_GPL(tcpci_write16);
> +
> +int tcpci_read8(struct tcpci *tcpci, unsigned int reg, u8 *val)
> +{
> + return regmap_raw_read(tcpci->regmap, reg, val, sizeof(u8));
> +}
> +EXPORT_SYMBOL_GPL(tcpci_read8);
> +
> +int tcpci_write8(struct tcpci *tcpci, unsigned int reg, u8 val)
> +{
> + return regmap_raw_write(tcpci->regmap, reg, &val, sizeof(u8));
> +}
> +EXPORT_SYMBOL_GPL(tcpci_write8);
I don't think there is any need to export those wrappers. You can
always expect the glue driver to supply the regmap as member of that
struct tcpci_vendor_data. See below..
> static int tcpci_set_cc(struct tcpc_dev *tcpc, enum typec_cc_status cc)
> {
> @@ -98,8 +112,10 @@ static int tcpci_set_cc(struct tcpc_dev *tcpc, enum typec_cc_status cc)
> static int tcpci_start_drp_toggling(struct tcpc_dev *tcpc,
> enum typec_cc_status cc)
> {
> + int ret;
> struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> - unsigned int reg = TCPC_ROLE_CTRL_DRP;
> + unsigned int reg = (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC1_SHIFT) |
> + (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC2_SHIFT);
>
> switch (cc) {
> default:
> @@ -116,8 +132,19 @@ static int tcpci_start_drp_toggling(struct tcpc_dev *tcpc,
> TCPC_ROLE_CTRL_RP_VAL_SHIFT);
> break;
> }
> -
> - return regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg);
> + ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg);
> + if (ret < 0)
> + return ret;
> + usleep_range(500, 1000);
> + reg |= TCPC_ROLE_CTRL_DRP;
> + ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg);
> + if (ret < 0)
> + return ret;
> + ret = regmap_write(tcpci->regmap, TCPC_COMMAND,
> + TCPC_CMD_LOOK4CONNECTION);
> + if (ret < 0)
> + return ret;
> + return 0;
> }
>
> static enum typec_cc_status tcpci_to_typec_cc(unsigned int cc, bool sink)
> @@ -323,6 +350,15 @@ static int tcpci_init(struct tcpc_dev *tcpc)
> if (time_after(jiffies, timeout))
> return -ETIMEDOUT;
>
> + /* Handle vendor init */
> + if (tcpci->vdata) {
> + if (tcpci->vdata->init) {
> + ret = (*tcpci->vdata->init)(tcpci, tcpci->vdata);
ret = tcpci->vdata->init(...
> + if (ret < 0)
> + return ret;
> + }
> + }
> +
> /* Clear all events */
> ret = tcpci_write16(tcpci, TCPC_ALERT, 0xffff);
> if (ret < 0)
> @@ -351,6 +387,13 @@ static irqreturn_t tcpci_irq(int irq, void *dev_id)
>
> tcpci_read16(tcpci, TCPC_ALERT, &status);
>
> + /* Handle vendor defined interrupt */
> + if (tcpci->vdata) {
> + if (tcpci->vdata->irq_handler)
> + (*tcpci->vdata->irq_handler)(tcpci, tcpci->vdata,
> + &status);
Ditto.
> + }
> +
> /*
> * Clear alert status for everything except RX_STATUS, which shouldn't
> * be cleared until we have successfully retrieved message.
> @@ -417,7 +460,7 @@ static irqreturn_t tcpci_irq(int irq, void *dev_id)
> .reg_bits = 8,
> .val_bits = 8,
>
> - .max_register = 0x7F, /* 0x80 .. 0xFF are vendor defined */
> + .max_register = 0xFF, /* 0x80 .. 0xFF are vendor defined */
> };
>
> static const struct tcpc_config tcpci_tcpc_config = {
> @@ -435,22 +478,22 @@ static int tcpci_parse_config(struct tcpci *tcpci)
> return 0;
> }
>
> -static int tcpci_probe(struct i2c_client *client,
> - const struct i2c_device_id *i2c_id)
> +struct tcpci *tcpci_register_port(struct i2c_client *client,
> + struct tcpci_vendor_data *vdata)
struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_...)
{
...
> {
> struct tcpci *tcpci;
> int err;
>
> tcpci = devm_kzalloc(&client->dev, sizeof(*tcpci), GFP_KERNEL);
> if (!tcpci)
> - return -ENOMEM;
> + return ERR_PTR(-ENOMEM);
>
> tcpci->client = client;
struct tcpci does not need that "client" member.
> tcpci->dev = &client->dev;
tcpci->dev = dev;
> - i2c_set_clientdata(client, tcpci);
> + tcpci->vdata = vdata;
> tcpci->regmap = devm_regmap_init_i2c(client, &tcpci_regmap_config);
You would now do that in your glue driver, and here just:
tcpci->regmap = data->regmap;
> tcpci->tcpc.init = tcpci_init;
> tcpci->tcpc.get_vbus = tcpci_get_vbus;
> @@ -467,7 +510,7 @@ static int tcpci_probe(struct i2c_client *client,
>
> err = tcpci_parse_config(tcpci);
> if (err < 0)
> - return err;
> + return ERR_PTR(err);
>
> /* Disable chip interrupts */
> tcpci_write16(tcpci, TCPC_ALERT_MASK, 0);
> @@ -477,17 +520,40 @@ static int tcpci_probe(struct i2c_client *client,
> IRQF_ONESHOT | IRQF_TRIGGER_LOW,
> dev_name(tcpci->dev), tcpci);
The interrupt could also be requested in the glue driver. Then from
the irq routine in your glue driver, you call tcpci_irq(). That would
mean you export tcpci_irq() of course.
> if (err < 0)
> - return err;
> + return ERR_PTR(err);
>
> tcpci->port = tcpm_register_port(tcpci->dev, &tcpci->tcpc);
> - return PTR_ERR_OR_ZERO(tcpci->port);
> + if (PTR_ERR_OR_ZERO(tcpci->port))
> + return ERR_CAST(tcpci->port);
> +
> + return tcpci;
> +}
> +EXPORT_SYMBOL_GPL(tcpci_register_port);
> +
> +void tcpci_unregister_port(struct tcpci *tcpci)
> +{
> + tcpm_unregister_port(tcpci->port);
> +}
> +EXPORT_SYMBOL_GPL(tcpci_unregister_port);
> +
> +static int tcpci_probe(struct i2c_client *client,
> + const struct i2c_device_id *i2c_id)
> +{
> + struct tcpci *tcpci;
> +
> + tcpci = tcpci_register_port(client, NULL);
> + if (PTR_ERR_OR_ZERO(tcpci))
> + return PTR_ERR(tcpci);
> +
> + i2c_set_clientdata(client, tcpci);
> + return 0;
> }
That would then look something like this:
static int tcpci_probe(struct i2c_client *client...
{
struct tcpci_data data; /* s/tcpci_vendor_data/tcpci_data/ */
struct tcpci *tcpci;
data.regmap = devm_regmap_init_i2c(client, &tcpci_regmap_config);
if (IS_ERR(tcpci->regmap))
return PTR_ERR(tcpci->regmap);
/*
* NOTE: everything in struct tcpci_data should be copied in
* tcpci_register_port().
*/
tcpci = tcpci_register_port(&client->dev, &data);
if (IS_ERR(tcpci))
return PTR_ERR(tcpci);
devm_request_threaded_irq(&client->dev, client->irq, NULL,
tcpci_irq,
...
i2c_set_clientdata(client, tcpci);
return 0;
}
Br,
--
heikki
next reply other threads:[~2018-02-14 10:56 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-14 10:56 Heikki Krogerus [this message]
2018-02-14 10:56 ` [PATCH] staging: typec: handle vendor defined part and modify drp toggling flow Heikki Krogerus
-- strict thread matches above, loose matches on Subject: below --
2018-03-02 18:43 ShuFanLee
2018-03-02 14:38 Jun Li
2018-03-01 11:53 shufan_lee(李書帆)
2018-03-01 10:06 Jun Li
2018-02-28 3:40 shufan_lee(李書帆)
2018-02-28 2:14 shufan_lee(李書帆)
2018-02-22 10:16 Jun Li
2018-02-21 22:15 Guenter Roeck
2018-02-21 15:02 ShuFanLee
2018-02-14 9:24 ShuFanLee
2018-02-14 9:24 ` [PATCH] " ShuFanLee
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180214105641.GE1480@kuha.fi.intel.com \
--to=heikki.krogerus@linux.intel.com \
--cc=cy_huang@richtek.com \
--cc=leechu729@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=shufan_lee@richtek.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.