| Message ID | 20240503003920.1482447-2-jthies@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: typec: ucsi: Update UCSI alternate mode | expand |
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 >
> … 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
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
> 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
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
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
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
Thank you for the feedback, I'll post a v4 series fixing these issues. Thanks, Jameson
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