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

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

[Prashant Malani]

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

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

[Prashant Malani]

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 <pmalani@chromium.org>
---
 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

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

[Prashant Malani]

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 <pmalani@chromium.org>
---
 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

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

[Prashant Malani]

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 <pmalani@chromium.org>
---
 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

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

[Prashant Malani]

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 <pmalani@chromium.org>
---
 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

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

[Tzung-Bi Shih]

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 <pmalani@chromium.org>

I have no idea but use your judgment:
Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>

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

[Tzung-Bi Shih]

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 <pmalani@chromium.org>

I have no idea but use your judgment:
Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>

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

[Tzung-Bi Shih]

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.

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

[Tzung-Bi Shih]

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 <pmalani@chromium.org>
> ---
>  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"?

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

[Heikki Krogerus]

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 <pmalani@chromium.org>

Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
>  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

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

[Heikki Krogerus]

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 <pmalani@chromium.org>

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

Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
>  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

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

[Heikki Krogerus]

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

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

[Prashant Malani]

Hi Heikki,

Thanks for the review.

On Tue, Aug 23, 2022 at 1:14 AM Heikki Krogerus
<heikki.krogerus@linux.intel.com> 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 <pmalani@chromium.org>
>
> 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 <heikki.krogerus@linux.intel.com>
>

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

[Prashant Malani]

Hi Tzung-Bi,

Thanks for reviewing the series.

On Mon, Aug 22, 2022 at 9:43 PM Tzung-Bi Shih <tzungbi@kernel.org> 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

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

[Prashant Malani]

Thanks for the reviews folks.

On Tue, Aug 23, 2022 at 1:17 AM Heikki Krogerus
<heikki.krogerus@linux.intel.com> 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

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

[patchwork-bot+chrome-platform]

Hello:

This series was applied to chrome-platform/linux.git (for-kernelci)
by Prashant Malani <pmalani@chromium.org>:

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

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

[Prashant Malani]

On Fri, Aug 26, 2022 at 3:40 PM
<patchwork-bot+chrome-platform@kernel.org> wrote:
>
> Hello:
>
> This series was applied to chrome-platform/linux.git (for-kernelci)
> by Prashant Malani <pmalani@chromium.org>:
>
> 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).

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

[patchwork-bot+chrome-platform]

Hello:

This series was applied to chrome-platform/linux.git (for-next)
by Prashant Malani <pmalani@chromium.org>:

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