2022-08-19 19:17:50

by Prashant Malani

[permalink] [raw]
Subject: [PATCH 0/4] platform/chrome: cros_ec_typec: Altmode fixes

This is a short series of minor fixes and changes to prepare the
ChromeOS Type-C driver to better support alternate mode drivers.

Prashant Malani (4):
platform/chrome: cros_ec_typec: Add bit offset for DP VDO
platform/chrome: cros_ec_typec: Correct alt mode index
platform/chrome: cros_ec_typec: Stash port driver info
platform/chrome: cros_ec_typec: Use Type-C driver data

drivers/platform/chrome/cros_ec_typec.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)

--
2.37.1.595.g718a3a8f04-goog


2022-08-19 19:19:01

by Prashant Malani

[permalink] [raw]
Subject: [PATCH 1/4] platform/chrome: cros_ec_typec: Add bit offset for DP VDO

Use the right macro while constructing the DP_PORT_VDO to ensure the Pin
Assignment offsets are correct.

Fixes: 1ff5d97f070c ("platform/chrome: cros_ec_typec: Register port altmodes")
Signed-off-by: Prashant Malani <[email protected]>
---
drivers/platform/chrome/cros_ec_typec.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index de6ee0f926a6..4d81d8d45b73 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -25,7 +25,8 @@

#define DRV_NAME "cros-ec-typec"

-#define DP_PORT_VDO (BIT(DP_PIN_ASSIGN_C) | BIT(DP_PIN_ASSIGN_D) | DP_CAP_DFP_D)
+#define DP_PORT_VDO (DP_CONF_SET_PIN_ASSIGN(BIT(DP_PIN_ASSIGN_C) | BIT(DP_PIN_ASSIGN_D)) | \
+ DP_CAP_DFP_D)

/* Supported alt modes. */
enum {
--
2.37.1.595.g718a3a8f04-goog

2022-08-19 19:19:32

by Prashant Malani

[permalink] [raw]
Subject: [PATCH 2/4] platform/chrome: cros_ec_typec: Correct alt mode index

Alt mode indices used by USB PD (Power Delivery) start with 1, not 0.

Update the alt mdoe registration code to factor this in to the alt mode
descriptor.

Fixes: de0f49487db3 ("platform/chrome: cros_ec_typec: Register partner altmodes")
Signed-off-by: Prashant Malani <[email protected]>
---
drivers/platform/chrome/cros_ec_typec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index 4d81d8d45b73..dc5722db2066 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -698,7 +698,7 @@ static int cros_typec_register_altmodes(struct cros_typec_data *typec, int port_
for (j = 0; j < sop_disc->svids[i].mode_count; j++) {
memset(&desc, 0, sizeof(desc));
desc.svid = sop_disc->svids[i].svid;
- desc.mode = j;
+ desc.mode = j + 1;
desc.vdo = sop_disc->svids[i].mode_vdo[j];

if (is_partner)
--
2.37.1.595.g718a3a8f04-goog

2022-08-19 19:19:41

by Prashant Malani

[permalink] [raw]
Subject: [PATCH 3/4] platform/chrome: cros_ec_typec: Stash port driver info

Stash port number and a pointer to the driver-specific struct in the
local Type-C port struct.

These can be useful to the port driver to figure out how to communicate
with the ChromeOS EC when an altmode driver related callback is invoked
from the Type-C class code.

Signed-off-by: Prashant Malani <[email protected]>
---
drivers/platform/chrome/cros_ec_typec.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index dc5722db2066..7daf4207c11e 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -44,6 +44,7 @@ struct cros_typec_altmode_node {
/* Per port data. */
struct cros_typec_port {
struct typec_port *port;
+ int port_num;
/* Initial capabilities for the port. */
struct typec_capability caps;
struct typec_partner *partner;
@@ -71,6 +72,8 @@ struct cros_typec_port {
struct ec_response_typec_discovery *disc_data;
struct list_head partner_mode_list;
struct list_head plug_mode_list;
+
+ struct cros_typec_data *typec_data;
};

/* Platform-specific data for the Chrome OS EC Type C controller. */
@@ -368,6 +371,8 @@ static int cros_typec_init_ports(struct cros_typec_data *typec)
goto unregister_ports;
}

+ cros_port->port_num = port_num;
+ cros_port->typec_data = typec;
typec->ports[port_num] = cros_port;
cap = &cros_port->caps;

--
2.37.1.595.g718a3a8f04-goog

2022-08-19 19:28:31

by Prashant Malani

[permalink] [raw]
Subject: [PATCH 4/4] platform/chrome: cros_ec_typec: Use Type-C driver data

Altmode driver callbacks need EC-specific port information to
communicate with the ChromeOS EC. To accomplish this, save a
pointer to the driver-specific port struct in the Type-C port's
driver data field.

Signed-off-by: Prashant Malani <[email protected]>
---
drivers/platform/chrome/cros_ec_typec.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index 7daf4207c11e..e3f75440030d 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -379,6 +379,7 @@ static int cros_typec_init_ports(struct cros_typec_data *typec)
ret = cros_typec_parse_port_props(cap, fwnode, dev);
if (ret < 0)
goto unregister_ports;
+ cap->driver_data = cros_port;

cros_port->port = typec_register_port(dev, cap);
if (IS_ERR(cros_port->port)) {
--
2.37.1.595.g718a3a8f04-goog

2022-08-23 04:46:19

by Tzung-Bi Shih

[permalink] [raw]
Subject: Re: [PATCH 1/4] platform/chrome: cros_ec_typec: Add bit offset for DP VDO

On Fri, Aug 19, 2022 at 07:08:02PM +0000, Prashant Malani wrote:
> Use the right macro while constructing the DP_PORT_VDO to ensure the Pin
> Assignment offsets are correct.
>
> Fixes: 1ff5d97f070c ("platform/chrome: cros_ec_typec: Register port altmodes")
> Signed-off-by: Prashant Malani <[email protected]>

I have no idea but use your judgment:
Reviewed-by: Tzung-Bi Shih <[email protected]>

2022-08-23 04:46:56

by Tzung-Bi Shih

[permalink] [raw]
Subject: Re: [PATCH 2/4] platform/chrome: cros_ec_typec: Correct alt mode index

On Fri, Aug 19, 2022 at 07:08:03PM +0000, Prashant Malani wrote:
> Alt mode indices used by USB PD (Power Delivery) start with 1, not 0.
>
> Update the alt mdoe registration code to factor this in to the alt mode
> descriptor.
>
> Fixes: de0f49487db3 ("platform/chrome: cros_ec_typec: Register partner altmodes")
> Signed-off-by: Prashant Malani <[email protected]>

I have no idea but use your judgment:
Reviewed-by: Tzung-Bi Shih <[email protected]>

2022-08-23 04:58:27

by Tzung-Bi Shih

[permalink] [raw]
Subject: Re: [PATCH 3/4] platform/chrome: cros_ec_typec: Stash port driver info

On Fri, Aug 19, 2022 at 07:08:04PM +0000, Prashant Malani wrote:
> Stash port number and a pointer to the driver-specific struct in the
> local Type-C port struct.
>
> These can be useful to the port driver to figure out how to communicate
> with the ChromeOS EC when an altmode driver related callback is invoked
> from the Type-C class code.

The patch looks good to me. But I would suggest to send it in later series
that uses the driver-specific struct (e.g. in altmode driver related callbacks)
to make the usage clear.

2022-08-23 05:35:18

by Tzung-Bi Shih

[permalink] [raw]
Subject: Re: [PATCH 4/4] platform/chrome: cros_ec_typec: Use Type-C driver data

On Fri, Aug 19, 2022 at 07:08:05PM +0000, Prashant Malani wrote:
> Altmode driver callbacks need EC-specific port information to
> communicate with the ChromeOS EC. To accomplish this, save a
> pointer to the driver-specific port struct in the Type-C port's
> driver data field.
>
> Signed-off-by: Prashant Malani <[email protected]>
> ---
> drivers/platform/chrome/cros_ec_typec.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
> index 7daf4207c11e..e3f75440030d 100644
> --- a/drivers/platform/chrome/cros_ec_typec.c
> +++ b/drivers/platform/chrome/cros_ec_typec.c
> @@ -379,6 +379,7 @@ static int cros_typec_init_ports(struct cros_typec_data *typec)
> ret = cros_typec_parse_port_props(cap, fwnode, dev);
> if (ret < 0)
> goto unregister_ports;
> + cap->driver_data = cros_port;

Same as previous patch. I would suggest to send it in later series. For
example, I have no knowledge to judge if `cap` is a correct place to save
the driver data.

For example, I'm wondering: is the `cap` "the Type-C port's driver"?

2022-08-23 08:59:22

by Heikki Krogerus

[permalink] [raw]
Subject: Re: [PATCH 3/4] platform/chrome: cros_ec_typec: Stash port driver info

On Tue, Aug 23, 2022 at 04:43:41AM +0000, Tzung-Bi Shih wrote:
> On Fri, Aug 19, 2022 at 07:08:04PM +0000, Prashant Malani wrote:
> > Stash port number and a pointer to the driver-specific struct in the
> > local Type-C port struct.
> >
> > These can be useful to the port driver to figure out how to communicate
> > with the ChromeOS EC when an altmode driver related callback is invoked
> > from the Type-C class code.
>
> The patch looks good to me. But I would suggest to send it in later series
> that uses the driver-specific struct (e.g. in altmode driver related callbacks)
> to make the usage clear.

I agree.

thanks,

--
heikki

2022-08-23 09:04:31

by Heikki Krogerus

[permalink] [raw]
Subject: Re: [PATCH 2/4] platform/chrome: cros_ec_typec: Correct alt mode index

Hi,

On Fri, Aug 19, 2022 at 07:08:03PM +0000, Prashant Malani wrote:
> Alt mode indices used by USB PD (Power Delivery) start with 1, not 0.
>
> Update the alt mdoe registration code to factor this in to the alt mode
> descriptor.
>
> Fixes: de0f49487db3 ("platform/chrome: cros_ec_typec: Register partner altmodes")
> Signed-off-by: Prashant Malani <[email protected]>

Shouldn't this be applied also to the stable kernels?

Acked-by: Heikki Krogerus <[email protected]>

> ---
> drivers/platform/chrome/cros_ec_typec.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
> index 4d81d8d45b73..dc5722db2066 100644
> --- a/drivers/platform/chrome/cros_ec_typec.c
> +++ b/drivers/platform/chrome/cros_ec_typec.c
> @@ -698,7 +698,7 @@ static int cros_typec_register_altmodes(struct cros_typec_data *typec, int port_
> for (j = 0; j < sop_disc->svids[i].mode_count; j++) {
> memset(&desc, 0, sizeof(desc));
> desc.svid = sop_disc->svids[i].svid;
> - desc.mode = j;
> + desc.mode = j + 1;
> desc.vdo = sop_disc->svids[i].mode_vdo[j];
>
> if (is_partner)
> --
> 2.37.1.595.g718a3a8f04-goog

--
heikki

2022-08-23 09:55:52

by Heikki Krogerus

[permalink] [raw]
Subject: Re: [PATCH 1/4] platform/chrome: cros_ec_typec: Add bit offset for DP VDO

On Fri, Aug 19, 2022 at 07:08:02PM +0000, Prashant Malani wrote:
> Use the right macro while constructing the DP_PORT_VDO to ensure the Pin
> Assignment offsets are correct.
>
> Fixes: 1ff5d97f070c ("platform/chrome: cros_ec_typec: Register port altmodes")
> Signed-off-by: Prashant Malani <[email protected]>

Acked-by: Heikki Krogerus <[email protected]>

> ---
> drivers/platform/chrome/cros_ec_typec.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
> index de6ee0f926a6..4d81d8d45b73 100644
> --- a/drivers/platform/chrome/cros_ec_typec.c
> +++ b/drivers/platform/chrome/cros_ec_typec.c
> @@ -25,7 +25,8 @@
>
> #define DRV_NAME "cros-ec-typec"
>
> -#define DP_PORT_VDO (BIT(DP_PIN_ASSIGN_C) | BIT(DP_PIN_ASSIGN_D) | DP_CAP_DFP_D)
> +#define DP_PORT_VDO (DP_CONF_SET_PIN_ASSIGN(BIT(DP_PIN_ASSIGN_C) | BIT(DP_PIN_ASSIGN_D)) | \
> + DP_CAP_DFP_D)
>
> /* Supported alt modes. */
> enum {
> --
> 2.37.1.595.g718a3a8f04-goog

--
heikki

2022-08-23 19:10:06

by Prashant Malani

[permalink] [raw]
Subject: Re: [PATCH 2/4] platform/chrome: cros_ec_typec: Correct alt mode index

Hi Heikki,

Thanks for the review.

On Tue, Aug 23, 2022 at 1:14 AM Heikki Krogerus
<[email protected]> wrote:
>
> Hi,
>
> On Fri, Aug 19, 2022 at 07:08:03PM +0000, Prashant Malani wrote:
> > Alt mode indices used by USB PD (Power Delivery) start with 1, not 0.
> >
> > Update the alt mdoe registration code to factor this in to the alt mode
> > descriptor.
> >
> > Fixes: de0f49487db3 ("platform/chrome: cros_ec_typec: Register partner altmodes")
> > Signed-off-by: Prashant Malani <[email protected]>
>
> Shouldn't this be applied also to the stable kernels?

We're not planning on backporting alt mode driver stuff to older
kernels, so probably not necessary
(aside from where it gets auto-magically picked).
Userspace currently doesn't use the mode index for anything.

Please LMK if you still feel it should go to stable.

>
> Acked-by: Heikki Krogerus <[email protected]>
>

2022-08-23 19:29:55

by Prashant Malani

[permalink] [raw]
Subject: Re: [PATCH 4/4] platform/chrome: cros_ec_typec: Use Type-C driver data

Hi Tzung-Bi,

Thanks for reviewing the series.

On Mon, Aug 22, 2022 at 9:43 PM Tzung-Bi Shih <[email protected]> wrote:
> > diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
> > index 7daf4207c11e..e3f75440030d 100644
> > --- a/drivers/platform/chrome/cros_ec_typec.c
> > +++ b/drivers/platform/chrome/cros_ec_typec.c
> > @@ -379,6 +379,7 @@ static int cros_typec_init_ports(struct cros_typec_data *typec)
> > ret = cros_typec_parse_port_props(cap, fwnode, dev);
> > if (ret < 0)
> > goto unregister_ports;
> > + cap->driver_data = cros_port;
>
> Same as previous patch. I would suggest to send it in later series. For
> example, I have no knowledge to judge if `cap` is a correct place to save
> the driver data.
>
> For example, I'm wondering: is the `cap` "the Type-C port's driver"?

The Type-C framework uses [1] the cap->driver_data while creating the
port device.

That said, sure, I can resend patch 3 and 4 when I upload the alt mode series.

[1] https://elixir.bootlin.com/linux/latest/source/drivers/usb/typec/class.c#L2098

2022-08-23 19:47:42

by Prashant Malani

[permalink] [raw]
Subject: Re: [PATCH 3/4] platform/chrome: cros_ec_typec: Stash port driver info

Thanks for the reviews folks.

On Tue, Aug 23, 2022 at 1:17 AM Heikki Krogerus
<[email protected]> wrote:
>
> On Tue, Aug 23, 2022 at 04:43:41AM +0000, Tzung-Bi Shih wrote:
> > On Fri, Aug 19, 2022 at 07:08:04PM +0000, Prashant Malani wrote:
> > > Stash port number and a pointer to the driver-specific struct in the
> > > local Type-C port struct.
> > >
> > > These can be useful to the port driver to figure out how to communicate
> > > with the ChromeOS EC when an altmode driver related callback is invoked
> > > from the Type-C class code.
> >
> > The patch looks good to me. But I would suggest to send it in later series
> > that uses the driver-specific struct (e.g. in altmode driver related callbacks)
> > to make the usage clear.
>
> I agree.

Ok. I will resend this along with the alt mode series.

BR,

-Prashant

Subject: Re: [PATCH 0/4] platform/chrome: cros_ec_typec: Altmode fixes

Hello:

This series was applied to chrome-platform/linux.git (for-kernelci)
by Prashant Malani <[email protected]>:

On Fri, 19 Aug 2022 19:08:01 +0000 you wrote:
> This is a short series of minor fixes and changes to prepare the
> ChromeOS Type-C driver to better support alternate mode drivers.
>
> Prashant Malani (4):
> platform/chrome: cros_ec_typec: Add bit offset for DP VDO
> platform/chrome: cros_ec_typec: Correct alt mode index
> platform/chrome: cros_ec_typec: Stash port driver info
> platform/chrome: cros_ec_typec: Use Type-C driver data
>
> [...]

Here is the summary with links:
- [1/4] platform/chrome: cros_ec_typec: Add bit offset for DP VDO
https://git.kernel.org/chrome-platform/c/1903adae0464
- [2/4] platform/chrome: cros_ec_typec: Correct alt mode index
https://git.kernel.org/chrome-platform/c/4e477663e396
- [3/4] platform/chrome: cros_ec_typec: Stash port driver info
(no matching commit)
- [4/4] platform/chrome: cros_ec_typec: Use Type-C driver data
(no matching commit)

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html


2022-08-26 23:08:51

by Prashant Malani

[permalink] [raw]
Subject: Re: [PATCH 0/4] platform/chrome: cros_ec_typec: Altmode fixes

On Fri, Aug 26, 2022 at 3:40 PM
<[email protected]> wrote:
>
> Hello:
>
> This series was applied to chrome-platform/linux.git (for-kernelci)
> by Prashant Malani <[email protected]>:
>
> On Fri, 19 Aug 2022 19:08:01 +0000 you wrote:
> > This is a short series of minor fixes and changes to prepare the
> > ChromeOS Type-C driver to better support alternate mode drivers.
> >
> > Prashant Malani (4):
> > platform/chrome: cros_ec_typec: Add bit offset for DP VDO
> > platform/chrome: cros_ec_typec: Correct alt mode index
> > platform/chrome: cros_ec_typec: Stash port driver info
> > platform/chrome: cros_ec_typec: Use Type-C driver data
> >
> > [...]
>
> Here is the summary with links:
> - [1/4] platform/chrome: cros_ec_typec: Add bit offset for DP VDO
> https://git.kernel.org/chrome-platform/c/1903adae0464
> - [2/4] platform/chrome: cros_ec_typec: Correct alt mode index
> https://git.kernel.org/chrome-platform/c/4e477663e396
> - [3/4] platform/chrome: cros_ec_typec: Stash port driver info
> (no matching commit)
> - [4/4] platform/chrome: cros_ec_typec: Use Type-C driver data
> (no matching commit)

This message isn't completely right; I've only applied patches 1 and 2
( 3 and 4 will be resubmitted as part of the altmode support series
later).

Subject: Re: [PATCH 0/4] platform/chrome: cros_ec_typec: Altmode fixes

Hello:

This series was applied to chrome-platform/linux.git (for-next)
by Prashant Malani <[email protected]>:

On Fri, 19 Aug 2022 19:08:01 +0000 you wrote:
> This is a short series of minor fixes and changes to prepare the
> ChromeOS Type-C driver to better support alternate mode drivers.
>
> Prashant Malani (4):
> platform/chrome: cros_ec_typec: Add bit offset for DP VDO
> platform/chrome: cros_ec_typec: Correct alt mode index
> platform/chrome: cros_ec_typec: Stash port driver info
> platform/chrome: cros_ec_typec: Use Type-C driver data
>
> [...]

Here is the summary with links:
- [1/4] platform/chrome: cros_ec_typec: Add bit offset for DP VDO
https://git.kernel.org/chrome-platform/c/1903adae0464
- [2/4] platform/chrome: cros_ec_typec: Correct alt mode index
https://git.kernel.org/chrome-platform/c/4e477663e396
- [3/4] platform/chrome: cros_ec_typec: Stash port driver info
(no matching commit)
- [4/4] platform/chrome: cros_ec_typec: Use Type-C driver data
(no matching commit)

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html