All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/4] usb: typec: ucsi: Update UCSI alternate mode
@ 2024-05-03  0:39 Jameson Thies
  2024-05-03  0:39 ` [PATCH v3 1/4] usb: typec: ucsi: Fix null pointer dereference in trace Jameson Thies
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Jameson Thies @ 2024-05-03  0:39 UTC (permalink / raw
  To: heikki.krogerus, linux-usb
  Cc: jthies, pmalani, bleung, abhishekpandit, andersson,
	dmitry.baryshkov, fabrice.gasnier, gregkh, hdegoede,
	neil.armstrong, rajaram.regupathy, saranya.gopal, linux-kernel

Hi Heikki,

This series appliess some changes to the UCSI driver to help support AP
driven alternate mode entry. This includes...

1. An update to the altmode sysfs group after registration to make
"active" writable.
2. A change to the ucsi_partner_task delay when queuing
ucsi_check_altmodes to prevent it from running before other discovery
functions.
3. An update to always define a number of alternate modes for partners
and plugs.

Not related to AP driven altmode entry, there is an additional fix for a
null derefrence in this series.

I tested the series on a ChromeOS v6.8 kernel merged with usb-testing.
That build had some additinal patches to enable a PPM in ChromeOS. Let
me know if you have any questions.

Thanks,
Jameson

Changes in V3:
- Returns typec_port_register_altmode call from
ucsi_register_displayport when CONFIG_TYPEC_DP_ALTMODE is not enabled.

Changes in V2:
- Checks for error response from ucsi_register_displayport when
registering DisplayPort alternate mode.

Abhishek Pandit-Subedi (2):
  usb: typec: ucsi: Fix null pointer dereference in trace
  usb: typec: Update sysfs when setting ops

Jameson Thies (2):
  usb: typec: ucsi: Delay alternate mode discovery
  usb: typec: ucsi: Always set number of alternate modes

 drivers/usb/typec/altmodes/displayport.c |  2 +-
 drivers/usb/typec/class.c                | 18 +++++++++++++++++-
 drivers/usb/typec/ucsi/displayport.c     |  2 +-
 drivers/usb/typec/ucsi/ucsi.c            | 18 +++++++++++++-----
 drivers/usb/typec/ucsi/ucsi.h            |  2 +-
 include/linux/usb/typec.h                |  3 +++
 6 files changed, 36 insertions(+), 9 deletions(-)


base-commit: 080e73c9411b9ebc4c22e8ee8a12a9f109b85819
-- 
2.45.0.rc1.225.g2a3ae87e7f-goog


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v3 1/4] usb: typec: ucsi: Fix null pointer dereference in trace
  2024-05-03  0:39 [PATCH v3 0/4] usb: typec: ucsi: Update UCSI alternate mode Jameson Thies
@ 2024-05-03  0:39 ` Jameson Thies
  2024-05-03  1:18   ` Benson Leung
                     ` (2 more replies)
  2024-05-03  0:39 ` [PATCH v3 2/4] usb: typec: Update sysfs when setting ops Jameson Thies
                   ` (2 subsequent siblings)
  3 siblings, 3 replies; 13+ messages in thread
From: Jameson Thies @ 2024-05-03  0:39 UTC (permalink / raw
  To: heikki.krogerus, linux-usb
  Cc: jthies, pmalani, bleung, abhishekpandit, andersson,
	dmitry.baryshkov, fabrice.gasnier, gregkh, hdegoede,
	neil.armstrong, rajaram.regupathy, saranya.gopal, linux-kernel

From: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>

ucsi_register_altmode checks IS_ERR on returned pointer and treats
NULL as valid. When CONFIG_TYPEC_DP_ALTMODE is not enabled
ucsi_register_displayport returns NULL which causese a NULL pointer
dereference in trace. Rather than return NULL, call
typec_port_register_altmode to register DisplayPort alternate mode
as a non-controllable mode when CONFIG_TYPEC_DP_ALTMODE is not enabled.

Reviewed-by: Jameson Thies <jthies@google.com>
Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
---
Changes in V3:
- Returns typec_port_register_altmode call from
ucsi_register_displayport when CONFIG_TYPEC_DP_ALTMODE is not enabled.

Changes in V2:
- Checks for error response from ucsi_register_displayport when
registering DisplayPort alternate mode.

 drivers/usb/typec/ucsi/ucsi.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/typec/ucsi/ucsi.h b/drivers/usb/typec/ucsi/ucsi.h
index c4d103db9d0f8..f66224a270bc6 100644
--- a/drivers/usb/typec/ucsi/ucsi.h
+++ b/drivers/usb/typec/ucsi/ucsi.h
@@ -496,7 +496,7 @@ ucsi_register_displayport(struct ucsi_connector *con,
 			  bool override, int offset,
 			  struct typec_altmode_desc *desc)
 {
-	return NULL;
+	return typec_port_register_altmode(con->port, desc);
 }
 
 static inline void
-- 
2.45.0.rc1.225.g2a3ae87e7f-goog


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 2/4] usb: typec: Update sysfs when setting ops
  2024-05-03  0:39 [PATCH v3 0/4] usb: typec: ucsi: Update UCSI alternate mode Jameson Thies
  2024-05-03  0:39 ` [PATCH v3 1/4] usb: typec: ucsi: Fix null pointer dereference in trace Jameson Thies
@ 2024-05-03  0:39 ` Jameson Thies
  2024-05-03  0:39 ` [PATCH v3 3/4] usb: typec: ucsi: Delay alternate mode discovery Jameson Thies
  2024-05-03  0:39 ` [PATCH v3 4/4] usb: typec: ucsi: Always set number of alternate modes Jameson Thies
  3 siblings, 0 replies; 13+ messages in thread
From: Jameson Thies @ 2024-05-03  0:39 UTC (permalink / raw
  To: heikki.krogerus, linux-usb
  Cc: jthies, pmalani, bleung, abhishekpandit, andersson,
	dmitry.baryshkov, fabrice.gasnier, gregkh, hdegoede,
	neil.armstrong, rajaram.regupathy, saranya.gopal, linux-kernel,
	Benson Leung

From: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>

When adding altmode ops, update the sysfs group so that visibility is
also recalculated.

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Reviewed-by: Benson Leung <bleung@chromium.org>
Reviewed-by: Jameson Thies <jthies@google.com>
Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
---
Changes in V3:
- None.

Changes in V2:
- None.

 drivers/usb/typec/altmodes/displayport.c |  2 +-
 drivers/usb/typec/class.c                | 18 +++++++++++++++++-
 drivers/usb/typec/ucsi/displayport.c     |  2 +-
 include/linux/usb/typec.h                |  3 +++
 4 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
index 596cd4806018b..92cc1b1361208 100644
--- a/drivers/usb/typec/altmodes/displayport.c
+++ b/drivers/usb/typec/altmodes/displayport.c
@@ -746,7 +746,7 @@ int dp_altmode_probe(struct typec_altmode *alt)
 	dp->alt = alt;
 
 	alt->desc = "DisplayPort";
-	alt->ops = &dp_altmode_ops;
+	typec_altmode_set_ops(alt, &dp_altmode_ops);
 
 	if (plug) {
 		plug->desc = "Displayport";
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 9610e647a8d48..9262fcd4144f8 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -467,6 +467,22 @@ static const struct attribute_group *typec_altmode_groups[] = {
 	NULL
 };
 
+/**
+ * typec_altmode_set_ops - Set ops for altmode
+ * @adev: Handle to the alternate mode
+ * @ops: Ops for the alternate mode
+ *
+ * After setting ops, attribute visiblity needs to be refreshed if the alternate
+ * mode can be activated.
+ */
+void typec_altmode_set_ops(struct typec_altmode *adev,
+			   const struct typec_altmode_ops *ops)
+{
+	adev->ops = ops;
+	sysfs_update_group(&adev->dev.kobj, &typec_altmode_group);
+}
+EXPORT_SYMBOL_GPL(typec_altmode_set_ops);
+
 static int altmode_id_get(struct device *dev)
 {
 	struct ida *ids;
@@ -2317,7 +2333,7 @@ void typec_port_register_altmodes(struct typec_port *port,
 			continue;
 		}
 
-		alt->ops = ops;
+		typec_altmode_set_ops(alt, ops);
 		typec_altmode_set_drvdata(alt, drvdata);
 		altmodes[index] = alt;
 		index++;
diff --git a/drivers/usb/typec/ucsi/displayport.c b/drivers/usb/typec/ucsi/displayport.c
index d9d3c91125ca8..eb7b8d6e47d00 100644
--- a/drivers/usb/typec/ucsi/displayport.c
+++ b/drivers/usb/typec/ucsi/displayport.c
@@ -337,7 +337,7 @@ struct typec_altmode *ucsi_register_displayport(struct ucsi_connector *con,
 	dp->con = con;
 	dp->alt = alt;
 
-	alt->ops = &ucsi_displayport_ops;
+	typec_altmode_set_ops(alt, &ucsi_displayport_ops);
 	typec_altmode_set_drvdata(alt, dp);
 
 	return alt;
diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
index b35b427561ab5..549275f8ac1b3 100644
--- a/include/linux/usb/typec.h
+++ b/include/linux/usb/typec.h
@@ -167,6 +167,9 @@ struct typec_port *typec_altmode2port(struct typec_altmode *alt);
 
 void typec_altmode_update_active(struct typec_altmode *alt, bool active);
 
+void typec_altmode_set_ops(struct typec_altmode *alt,
+			   const struct typec_altmode_ops *ops);
+
 enum typec_plug_index {
 	TYPEC_PLUG_SOP_P,
 	TYPEC_PLUG_SOP_PP,
-- 
2.45.0.rc1.225.g2a3ae87e7f-goog


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 3/4] usb: typec: ucsi: Delay alternate mode discovery
  2024-05-03  0:39 [PATCH v3 0/4] usb: typec: ucsi: Update UCSI alternate mode Jameson Thies
  2024-05-03  0:39 ` [PATCH v3 1/4] usb: typec: ucsi: Fix null pointer dereference in trace Jameson Thies
  2024-05-03  0:39 ` [PATCH v3 2/4] usb: typec: Update sysfs when setting ops Jameson Thies
@ 2024-05-03  0:39 ` Jameson Thies
  2024-05-03  0:39 ` [PATCH v3 4/4] usb: typec: ucsi: Always set number of alternate modes Jameson Thies
  3 siblings, 0 replies; 13+ messages in thread
From: Jameson Thies @ 2024-05-03  0:39 UTC (permalink / raw
  To: heikki.krogerus, linux-usb
  Cc: jthies, pmalani, bleung, abhishekpandit, andersson,
	dmitry.baryshkov, fabrice.gasnier, gregkh, hdegoede,
	neil.armstrong, rajaram.regupathy, saranya.gopal, linux-kernel,
	Benson Leung

Delay the ucsi_check_altmodes task to be inline with surrounding partner
tasks. This allows partner, cable and identity discovery to complete
before alternate mode registration. With that order, alternate mode
discovery can be used to indicate the ucsi driver has completed
discovery.

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Reviewed-by: Benson Leung <bleung@chromium.org>
Signed-off-by: Jameson Thies <jthies@google.com>
---
Changes in V3:
- None.

Changes in V2:
- None.

 drivers/usb/typec/ucsi/ucsi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index cb52e7b0a2c5c..bb6e57064513d 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -963,7 +963,7 @@ static void ucsi_pwr_opmode_change(struct ucsi_connector *con)
 		con->rdo = con->status.request_data_obj;
 		typec_set_pwr_opmode(con->port, TYPEC_PWR_MODE_PD);
 		ucsi_partner_task(con, ucsi_get_src_pdos, 30, 0);
-		ucsi_partner_task(con, ucsi_check_altmodes, 30, 0);
+		ucsi_partner_task(con, ucsi_check_altmodes, 30, HZ);
 		ucsi_partner_task(con, ucsi_register_partner_pdos, 1, HZ);
 		break;
 	case UCSI_CONSTAT_PWR_OPMODE_TYPEC1_5:
@@ -1247,7 +1247,7 @@ static void ucsi_handle_connector_change(struct work_struct *work)
 	}
 
 	if (con->status.change & UCSI_CONSTAT_CAM_CHANGE)
-		ucsi_partner_task(con, ucsi_check_altmodes, 1, 0);
+		ucsi_partner_task(con, ucsi_check_altmodes, 1, HZ);
 
 out_unlock:
 	mutex_unlock(&con->lock);
-- 
2.45.0.rc1.225.g2a3ae87e7f-goog


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 4/4] usb: typec: ucsi: Always set number of alternate modes
  2024-05-03  0:39 [PATCH v3 0/4] usb: typec: ucsi: Update UCSI alternate mode Jameson Thies
                   ` (2 preceding siblings ...)
  2024-05-03  0:39 ` [PATCH v3 3/4] usb: typec: ucsi: Delay alternate mode discovery Jameson Thies
@ 2024-05-03  0:39 ` Jameson Thies
  3 siblings, 0 replies; 13+ messages in thread
From: Jameson Thies @ 2024-05-03  0:39 UTC (permalink / raw
  To: heikki.krogerus, linux-usb
  Cc: jthies, pmalani, bleung, abhishekpandit, andersson,
	dmitry.baryshkov, fabrice.gasnier, gregkh, hdegoede,
	neil.armstrong, rajaram.regupathy, saranya.gopal, linux-kernel,
	Benson Leung

Providing the number of known alternate modes allows user space to
determine when device registration has completed. Always register a
number of known alternate modes for the partner and cable plug, even
when the number of supported alternate modes is 0.

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Reviewed-by: Benson Leung <bleung@chromium.org>
Signed-off-by: Jameson Thies <jthies@google.com>
---
Changes in V3:
- None.

Changes in V2:
- None.

 drivers/usb/typec/ucsi/ucsi.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index bb6e57064513d..52a14bfe4107e 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -812,10 +812,11 @@ static int ucsi_check_altmodes(struct ucsi_connector *con)
 	/* Ignoring the errors in this case. */
 	if (con->partner_altmode[0]) {
 		num_partner_am = ucsi_get_num_altmode(con->partner_altmode);
-		if (num_partner_am > 0)
-			typec_partner_set_num_altmodes(con->partner, num_partner_am);
+		typec_partner_set_num_altmodes(con->partner, num_partner_am);
 		ucsi_altmode_update_active(con);
 		return 0;
+	} else {
+		typec_partner_set_num_altmodes(con->partner, 0);
 	}
 
 	return ret;
@@ -1138,7 +1139,7 @@ static int ucsi_check_connection(struct ucsi_connector *con)
 static int ucsi_check_cable(struct ucsi_connector *con)
 {
 	u64 command;
-	int ret;
+	int ret, num_plug_am;
 
 	if (con->cable)
 		return 0;
@@ -1172,6 +1173,13 @@ static int ucsi_check_cable(struct ucsi_connector *con)
 			return ret;
 	}
 
+	if (con->plug_altmode[0]) {
+		num_plug_am = ucsi_get_num_altmode(con->plug_altmode);
+		typec_plug_set_num_altmodes(con->plug, num_plug_am);
+	} else {
+		typec_plug_set_num_altmodes(con->plug, 0);
+	}
+
 	return 0;
 }
 
-- 
2.45.0.rc1.225.g2a3ae87e7f-goog


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 1/4] usb: typec: ucsi: Fix null pointer dereference in trace
  2024-05-03  0:39 ` [PATCH v3 1/4] usb: typec: ucsi: Fix null pointer dereference in trace Jameson Thies
@ 2024-05-03  1:18   ` Benson Leung
  2024-05-03 15:35   ` Markus Elfring
  2024-05-06 10:03   ` [PATCH v3 " Heikki Krogerus
  2 siblings, 0 replies; 13+ messages in thread
From: Benson Leung @ 2024-05-03  1:18 UTC (permalink / raw
  To: Jameson Thies
  Cc: heikki.krogerus, linux-usb, pmalani, abhishekpandit, andersson,
	dmitry.baryshkov, fabrice.gasnier, gregkh, hdegoede,
	neil.armstrong, rajaram.regupathy, saranya.gopal, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1625 bytes --]

Hi Jameson,

On Fri, May 03, 2024 at 12:39:17AM +0000, Jameson Thies wrote:
> From: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> 
> ucsi_register_altmode checks IS_ERR on returned pointer and treats
> NULL as valid. When CONFIG_TYPEC_DP_ALTMODE is not enabled
> ucsi_register_displayport returns NULL which causese a NULL pointer
> dereference in trace. Rather than return NULL, call
> typec_port_register_altmode to register DisplayPort alternate mode
> as a non-controllable mode when CONFIG_TYPEC_DP_ALTMODE is not enabled.
> 
> Reviewed-by: Jameson Thies <jthies@google.com>
> Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>

Reviewed-by: Benson Leung <bleung@chromium.org>

> ---
> Changes in V3:
> - Returns typec_port_register_altmode call from
> ucsi_register_displayport when CONFIG_TYPEC_DP_ALTMODE is not enabled.
> 
> Changes in V2:
> - Checks for error response from ucsi_register_displayport when
> registering DisplayPort alternate mode.
> 
>  drivers/usb/typec/ucsi/ucsi.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/typec/ucsi/ucsi.h b/drivers/usb/typec/ucsi/ucsi.h
> index c4d103db9d0f8..f66224a270bc6 100644
> --- a/drivers/usb/typec/ucsi/ucsi.h
> +++ b/drivers/usb/typec/ucsi/ucsi.h
> @@ -496,7 +496,7 @@ ucsi_register_displayport(struct ucsi_connector *con,
>  			  bool override, int offset,
>  			  struct typec_altmode_desc *desc)
>  {
> -	return NULL;
> +	return typec_port_register_altmode(con->port, desc);
>  }
>  
>  static inline void
> -- 
> 2.45.0.rc1.225.g2a3ae87e7f-goog
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 1/4] usb: typec: ucsi: Fix null pointer dereference in trace
  2024-05-03  0:39 ` [PATCH v3 1/4] usb: typec: ucsi: Fix null pointer dereference in trace Jameson Thies
  2024-05-03  1:18   ` Benson Leung
@ 2024-05-03 15:35   ` Markus Elfring
  2024-05-03 19:05     ` Jameson Thies
  2024-05-06 10:03   ` [PATCH v3 " Heikki Krogerus
  2 siblings, 1 reply; 13+ messages in thread
From: Markus Elfring @ 2024-05-03 15:35 UTC (permalink / raw
  To: Abhishek Pandit-Subedi, linux-usb, kernel-janitors, Benson Leung,
	Heikki Krogerus, Jameson Thies
  Cc: LKML, Bjorn Andersson, Dmitry Baryshkov, Fabrice Gasnier,
	Greg Kroah-Hartman, Hans de Goede, Neil Armstrong,
	Prashant Malani, Rajaram Regupathy, Saranya Gopal,
	Uwe Kleine-König

>                                      … which causese a NULL pointer
…

I hope that a typo will be avoided in the change description for the final commit.


…
> ---
> Changes in V3:
…

How do you think about to mention also adjustments for the commit message here?

Regards,
Markus

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 1/4] usb: typec: ucsi: Fix null pointer dereference in trace
  2024-05-03 15:35   ` Markus Elfring
@ 2024-05-03 19:05     ` Jameson Thies
  2024-05-04  5:12       ` [v3 " Markus Elfring
  0 siblings, 1 reply; 13+ messages in thread
From: Jameson Thies @ 2024-05-03 19:05 UTC (permalink / raw
  To: Markus Elfring
  Cc: Abhishek Pandit-Subedi, linux-usb, kernel-janitors, Benson Leung,
	Heikki Krogerus, LKML, Bjorn Andersson, Dmitry Baryshkov,
	Fabrice Gasnier, Greg Kroah-Hartman, Hans de Goede,
	Neil Armstrong, Prashant Malani, Rajaram Regupathy, Saranya Gopal,
	Uwe Kleine-König

Hi Markus,
thank you for catching the typo. If I upload a v4 patch, I will fix it
there. I don't think it is necessary to mention changes to the commit
message in the section below the commit message.

Thanks,
Jameson

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [v3 1/4] usb: typec: ucsi: Fix null pointer dereference in trace
  2024-05-03 19:05     ` Jameson Thies
@ 2024-05-04  5:12       ` Markus Elfring
  2024-05-04  7:24         ` Greg Kroah-Hartman
  0 siblings, 1 reply; 13+ messages in thread
From: Markus Elfring @ 2024-05-04  5:12 UTC (permalink / raw
  To: Jameson Thies, Abhishek Pandit-Subedi, linux-usb, kernel-janitors
  Cc: Benson Leung, Heikki Krogerus, LKML, Bjorn Andersson,
	Dmitry Baryshkov, Fabrice Gasnier, Greg Kroah-Hartman,
	Hans de Goede, Neil Armstrong, Prashant Malani, Rajaram Regupathy,
	Saranya Gopal, Uwe Kleine-König

> I don't think it is necessary to mention changes to the commit message
> in the section below the commit message.

Did you notice that other contributors occasionally share hints about
adjustments for parts of commit messages?
Will further information presentation become better supported?

Regards,
Markus

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [v3 1/4] usb: typec: ucsi: Fix null pointer dereference in trace
  2024-05-04  5:12       ` [v3 " Markus Elfring
@ 2024-05-04  7:24         ` Greg Kroah-Hartman
  0 siblings, 0 replies; 13+ messages in thread
From: Greg Kroah-Hartman @ 2024-05-04  7:24 UTC (permalink / raw
  To: Markus Elfring
  Cc: Jameson Thies, Abhishek Pandit-Subedi, linux-usb, kernel-janitors,
	Benson Leung, Heikki Krogerus, LKML, Bjorn Andersson,
	Dmitry Baryshkov, Fabrice Gasnier, Hans de Goede, Neil Armstrong,
	Prashant Malani, Rajaram Regupathy, Saranya Gopal,
	Uwe Kleine-König

On Sat, May 04, 2024 at 07:12:49AM +0200, Markus Elfring wrote:
> > I don't think it is necessary to mention changes to the commit message
> > in the section below the commit message.
> 
> Did you notice that other contributors occasionally share hints about
> adjustments for parts of commit messages?
> Will further information presentation become better supported?

Hi,

This is the semi-friendly patch-bot of Greg Kroah-Hartman.

Markus, you seem to have sent a nonsensical or otherwise pointless
review comment to a patch submission on a Linux kernel developer mailing
list.  I strongly suggest that you not do this anymore.  Please do not
bother developers who are actively working to produce patches and
features with comments that, in the end, are a waste of time.

Patch submitter, please ignore Markus's suggestion; you do not need to
follow it at all.  The person/bot/AI that sent it is being ignored by
almost all Linux kernel maintainers for having a persistent pattern of
behavior of producing distracting and pointless commentary, and
inability to adapt to feedback.  Please feel free to also ignore emails
from them.

thanks,

greg k-h's patch email bot

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 1/4] usb: typec: ucsi: Fix null pointer dereference in trace
  2024-05-03  0:39 ` [PATCH v3 1/4] usb: typec: ucsi: Fix null pointer dereference in trace Jameson Thies
  2024-05-03  1:18   ` Benson Leung
  2024-05-03 15:35   ` Markus Elfring
@ 2024-05-06 10:03   ` Heikki Krogerus
  2024-05-10  9:35     ` Greg KH
  2 siblings, 1 reply; 13+ messages in thread
From: Heikki Krogerus @ 2024-05-06 10:03 UTC (permalink / raw
  To: Jameson Thies
  Cc: linux-usb, pmalani, bleung, abhishekpandit, andersson,
	dmitry.baryshkov, fabrice.gasnier, gregkh, hdegoede,
	neil.armstrong, rajaram.regupathy, saranya.gopal, linux-kernel

On Fri, May 03, 2024 at 12:39:17AM +0000, Jameson Thies wrote:
> From: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> 
> ucsi_register_altmode checks IS_ERR on returned pointer and treats
> NULL as valid. When CONFIG_TYPEC_DP_ALTMODE is not enabled
> ucsi_register_displayport returns NULL which causese a NULL pointer
> dereference in trace. Rather than return NULL, call
> typec_port_register_altmode to register DisplayPort alternate mode
> as a non-controllable mode when CONFIG_TYPEC_DP_ALTMODE is not enabled.
> 
> Reviewed-by: Jameson Thies <jthies@google.com>
> Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>

You delivered the patch, so you should have used SoB instead of
Reviewed-by tag:
https://docs.kernel.org/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by

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

> ---
> Changes in V3:
> - Returns typec_port_register_altmode call from
> ucsi_register_displayport when CONFIG_TYPEC_DP_ALTMODE is not enabled.
> 
> Changes in V2:
> - Checks for error response from ucsi_register_displayport when
> registering DisplayPort alternate mode.
> 
>  drivers/usb/typec/ucsi/ucsi.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/typec/ucsi/ucsi.h b/drivers/usb/typec/ucsi/ucsi.h
> index c4d103db9d0f8..f66224a270bc6 100644
> --- a/drivers/usb/typec/ucsi/ucsi.h
> +++ b/drivers/usb/typec/ucsi/ucsi.h
> @@ -496,7 +496,7 @@ ucsi_register_displayport(struct ucsi_connector *con,
>  			  bool override, int offset,
>  			  struct typec_altmode_desc *desc)
>  {
> -	return NULL;
> +	return typec_port_register_altmode(con->port, desc);
>  }
>  
>  static inline void
> -- 
> 2.45.0.rc1.225.g2a3ae87e7f-goog

-- 
heikki

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 1/4] usb: typec: ucsi: Fix null pointer dereference in trace
  2024-05-06 10:03   ` [PATCH v3 " Heikki Krogerus
@ 2024-05-10  9:35     ` Greg KH
  2024-05-10 17:47       ` Jameson Thies
  0 siblings, 1 reply; 13+ messages in thread
From: Greg KH @ 2024-05-10  9:35 UTC (permalink / raw
  To: Heikki Krogerus
  Cc: Jameson Thies, linux-usb, pmalani, bleung, abhishekpandit,
	andersson, dmitry.baryshkov, fabrice.gasnier, hdegoede,
	neil.armstrong, rajaram.regupathy, saranya.gopal, linux-kernel

On Mon, May 06, 2024 at 01:03:12PM +0300, Heikki Krogerus wrote:
> On Fri, May 03, 2024 at 12:39:17AM +0000, Jameson Thies wrote:
> > From: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> > 
> > ucsi_register_altmode checks IS_ERR on returned pointer and treats
> > NULL as valid. When CONFIG_TYPEC_DP_ALTMODE is not enabled
> > ucsi_register_displayport returns NULL which causese a NULL pointer
> > dereference in trace. Rather than return NULL, call
> > typec_port_register_altmode to register DisplayPort alternate mode
> > as a non-controllable mode when CONFIG_TYPEC_DP_ALTMODE is not enabled.
> > 
> > Reviewed-by: Jameson Thies <jthies@google.com>
> > Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> 
> You delivered the patch, so you should have used SoB instead of
> Reviewed-by tag:
> https://docs.kernel.org/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by

Not "should", that would be "must". I can't take it like this, sorry.

greg k-h

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 1/4] usb: typec: ucsi: Fix null pointer dereference in trace
  2024-05-10  9:35     ` Greg KH
@ 2024-05-10 17:47       ` Jameson Thies
  0 siblings, 0 replies; 13+ messages in thread
From: Jameson Thies @ 2024-05-10 17:47 UTC (permalink / raw
  To: Greg KH
  Cc: Heikki Krogerus, linux-usb, pmalani, bleung, abhishekpandit,
	andersson, dmitry.baryshkov, fabrice.gasnier, hdegoede,
	neil.armstrong, rajaram.regupathy, saranya.gopal, linux-kernel

Thank you for the feedback, I'll post a v4 series fixing these issues.

Thanks,
Jameson

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2024-05-10 17:47 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-03  0:39 [PATCH v3 0/4] usb: typec: ucsi: Update UCSI alternate mode Jameson Thies
2024-05-03  0:39 ` [PATCH v3 1/4] usb: typec: ucsi: Fix null pointer dereference in trace Jameson Thies
2024-05-03  1:18   ` Benson Leung
2024-05-03 15:35   ` Markus Elfring
2024-05-03 19:05     ` Jameson Thies
2024-05-04  5:12       ` [v3 " Markus Elfring
2024-05-04  7:24         ` Greg Kroah-Hartman
2024-05-06 10:03   ` [PATCH v3 " Heikki Krogerus
2024-05-10  9:35     ` Greg KH
2024-05-10 17:47       ` Jameson Thies
2024-05-03  0:39 ` [PATCH v3 2/4] usb: typec: Update sysfs when setting ops Jameson Thies
2024-05-03  0:39 ` [PATCH v3 3/4] usb: typec: ucsi: Delay alternate mode discovery Jameson Thies
2024-05-03  0:39 ` [PATCH v3 4/4] usb: typec: ucsi: Always set number of alternate modes Jameson Thies

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.