Skip to contentSkip to conversation

usb: typec: altmodes: select DRM_AUX_HPD_BRIDGE for TYPEC_DP_ALTMODE

PATCH Superseded Chaoyi Chen 5 participants 8 messages Jul 5 – Jul 9

Subsystems (3)
USB USB Type-C class USB Type-C bus for alternate modes

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Chaoyi Chen

author
#0 lore raw

PATCH v2 1/1 usb: typec: altmodes: select DRM_AUX_HPD_BRIDGE for TYPEC_DP_ALTMODE

From: Chaoyi Chen <[email protected]>

When TYPEC_DP_ALTMODE is enabled, DRM_AUX_HPD_BRIDGE is most likely also needed for embedded platforms. Select it when available.

Suggested-by Xu Yang <[email protected]>
Signed-off-by Chaoyi Chen <[email protected]>
---
 drivers/usb/typec/altmodes/Kconfig | 1 +
 1 file changed, 1 insertion(+)
drivers/usb/typec/altmodes/Kconfig +1 -0
index 7867fa7c405d..1f35b253d6b8 100644
@@ -5,6 +5,7 @@ menu "USB Type-C Alternate Mode drivers"
 config TYPEC_DP_ALTMODE
 	tristate "DisplayPort Alternate Mode driver"
 	depends on DRM
+	imply DRM_AUX_HPD_BRIDGE if DRM_BRIDGE && OF
 	help
 	  DisplayPort USB Type-C Alternate Mode allows DisplayPort
 	  displays and adapters to be attached to the USB Type-C
Signature · 1 line
2.53.0
#1 lore raw

Hi all,

drivers/usb/typec/altmodes/Kconfig menu "USB Type-C Alternate Mode drivers" · line 8

Quoted 1 line · Chaoyi Chen
drivers/usb/typec/altmodes/Kconfig +1 -0
+	imply DRM_AUX_HPD_BRIDGE if DRM_BRIDGE && OF

When I suggested that 'imply' may be more appropriate here on v1, I was hoping to have more of a conversation first.

'Imply' is safer to use than 'select', and intended for use when the option is highly preferred (but not actually required).

Perhaps Xu could provide an opinion?

- Julian Braha

#2 lore raw

Hi Julian,

Quoted 8 lines · Julian Braha

Hi all,

drivers/usb/typec/altmodes/Kconfig menu "USB Type-C Alternate Mode drivers" · line 8

Quoted 1 line · Chaoyi Chen
drivers/usb/typec/altmodes/Kconfig +1 -0
+	imply DRM_AUX_HPD_BRIDGE if DRM_BRIDGE && OF

When I suggested that 'imply' may be more appropriate here on v1, I was hoping to have more of a conversation first.

Quoted 5 lines

'Imply' is safer to use than 'select', and intended for use when the option is highly preferred (but not actually required).

Perhaps Xu could provide an opinion?

I think this is reasonable, as x86 works fine without DRM_AUX_HPD_BRIDGE.

Signature · 2 lines
Best, 
Chaoyi
#3 lore raw

Hi Chaoyi,

Quoted 1 line · Chaoyi Chen

I think this is reasonable, as x86 works fine without DRM_AUX_HPD_BRIDGE.

Understood, thank you for clarifying.

I think this patch needs a v3 as the commit message and title no longer match what the patch does and would make the commit history confusing. Also, in the next version, could you add the changelog and links to each previous revision below the '---'?

Finally, a link to the original conversation that led to this patch would be good to add to the body:
https://lore.kernel.org/all/[email protected]/

Then I will give you my reviewed-by :)

- Julian Braha

#4 lore raw
In reply to Julian Braha
Quoted 5 lines · Julian Braha

Hi all,

Quoted 1 line · Chaoyi Chen

+ imply DRM_AUX_HPD_BRIDGE if DRM_BRIDGE && OF

Quoted 7 lines

When I suggested that 'imply' may be more appropriate here on v1, I was hoping to have more of a conversation first.

'Imply' is safer to use than 'select', and intended for use when the option is highly preferred (but not actually required).

Perhaps Xu could provide an opinion?

Config DRM_AUX_HPD_BRIDGE is not user visiable, so I think "select" and "imply" have the same effects here. And there is no force dependency bewteen DRM_AUX_HPD_BRIDGE and TYPEC_DP_ALTMODE, it's ok for me to use imply here.

Thanks,
Xu Yang

maintainer
#5 lore raw
In reply to Chaoyi Chen
Quoted 16 lines · Chaoyi Chen

From: Chaoyi Chen <[email protected]>

When TYPEC_DP_ALTMODE is enabled, DRM_AUX_HPD_BRIDGE is most likely also needed for embedded platforms. Select it when available.

Suggested-by Xu Yang <[email protected]>
Signed-off-by Chaoyi Chen <[email protected]>
---
 drivers/usb/typec/altmodes/Kconfig | 1 +
 1 file changed, 1 insertion(+)
drivers/usb/typec/altmodes/Kconfig +0 -0
index 7867fa7c405d..1f35b253d6b8 100644

drivers/usb/typec/altmodes/Kconfig menu "USB Type-C Alternate Mode drivers" · line 8

Quoted 4 lines
drivers/usb/typec/altmodes/Kconfig +1 -0
@@ -5,6 +5,7 @@ menu "USB Type-C Alternate Mode drivers"
 config TYPEC_DP_ALTMODE
 	tristate "DisplayPort Alternate Mode driver"
 	depends on DRM
+	imply DRM_AUX_HPD_BRIDGE if DRM_BRIDGE && OF

I hate "imply", as this just makes things more complex. Either this is a dependency or not, right? Why not fix that?

thanks,

greg k-h

#6 lore raw

Hello Greg,

Quoted 12 lines · Greg Kroah-Hartman
Quoted 11 lines · Chaoyi Chen

From: Chaoyi Chen <[email protected]>

When TYPEC_DP_ALTMODE is enabled, DRM_AUX_HPD_BRIDGE is most likely also needed for embedded platforms. Select it when available.

Suggested-by Xu Yang <[email protected]>
Signed-off-by Chaoyi Chen <[email protected]>
---
 drivers/usb/typec/altmodes/Kconfig | 1 +
 1 file changed, 1 insertion(+)
Quoted 13 lines

drivers/usb/typec/altmodes/Kconfig menu "USB Type-C Alternate Mode drivers" · line 8

Quoted 9 lines
drivers/usb/typec/altmodes/Kconfig +1 -0
index 7867fa7c405d..1f35b253d6b8 100644
@@ -5,6 +5,7 @@ menu "USB Type-C Alternate Mode drivers"
 config TYPEC_DP_ALTMODE
 	tristate "DisplayPort Alternate Mode driver"
 	depends on DRM
+	imply DRM_AUX_HPD_BRIDGE if DRM_BRIDGE && OF

I hate "imply", as this just makes things more complex. Either this is a dependency or not, right? Why not fix that?

Perhaps we can just use the v1 content directly?
Hopefully that's already simple enough.

https://lore.kernel.org/all/[email protected]/

Quoted 5 lines

thanks,

greg k-h

Signature · 2 lines
Best, 
Chaoyi
maintainer
#7 lore raw
Quoted 29 lines · Chaoyi Chen

Hello Greg,

Quoted 12 lines · Greg Kroah-Hartman
Quoted 11 lines · Chaoyi Chen

From: Chaoyi Chen <[email protected]>

When TYPEC_DP_ALTMODE is enabled, DRM_AUX_HPD_BRIDGE is most likely also needed for embedded platforms. Select it when available.

Suggested-by Xu Yang <[email protected]>
Signed-off-by Chaoyi Chen <[email protected]>
---
 drivers/usb/typec/altmodes/Kconfig | 1 +
 1 file changed, 1 insertion(+)
Quoted 13 lines

drivers/usb/typec/altmodes/Kconfig menu "USB Type-C Alternate Mode drivers" · line 8

Quoted 9 lines
drivers/usb/typec/altmodes/Kconfig +1 -0
index 7867fa7c405d..1f35b253d6b8 100644
@@ -5,6 +5,7 @@ menu "USB Type-C Alternate Mode drivers"
 config TYPEC_DP_ALTMODE
 	tristate "DisplayPort Alternate Mode driver"
 	depends on DRM
+	imply DRM_AUX_HPD_BRIDGE if DRM_BRIDGE && OF

I hate "imply", as this just makes things more complex. Either this is a dependency or not, right? Why not fix that?

Quoted 4 lines

Perhaps we can just use the v1 content directly?
Hopefully that's already simple enough.

https://lore.kernel.org/all/[email protected]/

Maybe, send it as a v3 and see if the maintainers of those drivers agree?

thanks,

greg k-h