| Message ID | 20241107112955.v3.3.I439cffc7bf76d94f5850eb85980f1197c4f9154c@changeid (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Thunderbolt and DP altmode support for cros-ec-typec | expand |
On Thu, Nov 07, 2024 at 11:29:56AM -0800, Abhishek Pandit-Subedi wrote: > Enforce the same requirement as when we attempt to activate altmode via > sysfs (do not enter partner mode if port mode is not active). In order > to set a default value for this field, also introduce the inactive field > in struct typec_altmode_desc. > > Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org> Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > --- > > Changes in v3: > - Use port.active instead of introducing auto-enter field > - Introduce inactive field to typec_altmode_desc to set default value > for active. > - Always make port 'active' field writable > > drivers/usb/typec/altmodes/displayport.c | 7 +++++-- > drivers/usb/typec/altmodes/thunderbolt.c | 6 +++++- > drivers/usb/typec/class.c | 5 +++-- > include/linux/usb/typec.h | 2 ++ > 4 files changed, 15 insertions(+), 5 deletions(-) > > diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c > index 3245e03d59e6..f4116e96f6a1 100644 > --- a/drivers/usb/typec/altmodes/displayport.c > +++ b/drivers/usb/typec/altmodes/displayport.c > @@ -767,8 +767,11 @@ int dp_altmode_probe(struct typec_altmode *alt) > if (plug) > typec_altmode_set_drvdata(plug, dp); > > - dp->state = plug ? DP_STATE_ENTER_PRIME : DP_STATE_ENTER; > - schedule_work(&dp->work); > + /* Only attempt to enter altmode if port is active. */ > + if (port->active) { > + dp->state = plug ? DP_STATE_ENTER_PRIME : DP_STATE_ENTER; > + schedule_work(&dp->work); > + } > > return 0; > } > diff --git a/drivers/usb/typec/altmodes/thunderbolt.c b/drivers/usb/typec/altmodes/thunderbolt.c > index a945b9d35a1d..45567abc3bb8 100644 > --- a/drivers/usb/typec/altmodes/thunderbolt.c > +++ b/drivers/usb/typec/altmodes/thunderbolt.c > @@ -212,6 +212,7 @@ static const struct typec_altmode_ops tbt_altmode_ops = { > > static int tbt_altmode_probe(struct typec_altmode *alt) > { > + const struct typec_altmode *port = typec_altmode_get_partner(alt); > struct tbt_altmode *tbt; > > tbt = devm_kzalloc(&alt->dev, sizeof(*tbt), GFP_KERNEL); > @@ -226,7 +227,10 @@ static int tbt_altmode_probe(struct typec_altmode *alt) > typec_altmode_set_drvdata(alt, tbt); > typec_altmode_set_ops(alt, &tbt_altmode_ops); > > - if (tbt_ready(alt)) { > + /* Only attempt to enter altmode if port is active and cable/plug > + * information is ready. > + */ > + if (port->active && tbt_ready(alt)) { > if (tbt->plug[TYPEC_PLUG_SOP_PP]) > tbt->state = TBT_STATE_SOP_PP_ENTER; > else if (tbt->plug[TYPEC_PLUG_SOP_P]) > diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c > index febe453b96be..b5e67a57762c 100644 > --- a/drivers/usb/typec/class.c > +++ b/drivers/usb/typec/class.c > @@ -458,7 +458,8 @@ static umode_t typec_altmode_attr_is_visible(struct kobject *kobj, > struct typec_altmode *adev = to_typec_altmode(kobj_to_dev(kobj)); > > if (attr == &dev_attr_active.attr) > - if (!adev->ops || !adev->ops->activate) > + if (!is_typec_port(adev->dev.parent) && > + (!adev->ops || !adev->ops->activate)) > return 0444; > > return attr->mode; > @@ -563,7 +564,7 @@ typec_register_altmode(struct device *parent, > > if (is_port) { > alt->attrs[3] = &dev_attr_supported_roles.attr; > - alt->adev.active = true; /* Enabled by default */ > + alt->adev.active = !desc->inactive; /* Enabled by default */ > } > > sprintf(alt->group_name, "mode%d", desc->mode); > diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h > index d616b8807000..56c01771c190 100644 > --- a/include/linux/usb/typec.h > +++ b/include/linux/usb/typec.h > @@ -140,6 +140,7 @@ int typec_cable_set_identity(struct typec_cable *cable); > * @mode: Index of the Mode > * @vdo: VDO returned by Discover Modes USB PD command > * @roles: Only for ports. DRP if the mode is available in both roles > + * @inactive: Only for ports. Make this port inactive (default is active). > * > * Description of an Alternate Mode which a connector, cable plug or partner > * supports. > @@ -150,6 +151,7 @@ struct typec_altmode_desc { > u32 vdo; > /* Only used with ports */ > enum typec_port_data roles; > + bool inactive : 1; > }; > > void typec_partner_set_pd_revision(struct typec_partner *partner, u16 pd_revision); > -- > 2.47.0.277.g8800431eea-goog
On Thu, Nov 07, 2024 at 11:29:56AM -0800, Abhishek Pandit-Subedi wrote: > Enforce the same requirement as when we attempt to activate altmode via > sysfs (do not enter partner mode if port mode is not active). In order > to set a default value for this field, also introduce the inactive field > in struct typec_altmode_desc. > > Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org> > --- > > Changes in v3: > - Use port.active instead of introducing auto-enter field > - Introduce inactive field to typec_altmode_desc to set default value > for active. > - Always make port 'active' field writable > > drivers/usb/typec/altmodes/displayport.c | 7 +++++-- > drivers/usb/typec/altmodes/thunderbolt.c | 6 +++++- > drivers/usb/typec/class.c | 5 +++-- > include/linux/usb/typec.h | 2 ++ > 4 files changed, 15 insertions(+), 5 deletions(-) > > diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c > index 3245e03d59e6..f4116e96f6a1 100644 > --- a/drivers/usb/typec/altmodes/displayport.c > +++ b/drivers/usb/typec/altmodes/displayport.c > @@ -767,8 +767,11 @@ int dp_altmode_probe(struct typec_altmode *alt) > if (plug) > typec_altmode_set_drvdata(plug, dp); > > - dp->state = plug ? DP_STATE_ENTER_PRIME : DP_STATE_ENTER; > - schedule_work(&dp->work); > + /* Only attempt to enter altmode if port is active. */ > + if (port->active) { > + dp->state = plug ? DP_STATE_ENTER_PRIME : DP_STATE_ENTER; > + schedule_work(&dp->work); > + } What prevents active from changing right after you checked it? > @@ -150,6 +151,7 @@ struct typec_altmode_desc { > u32 vdo; > /* Only used with ports */ > enum typec_port_data roles; > + bool inactive : 1; A boolean bitfield? That's not needed, please just do this properly. thanks, greg k-h
diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c index 3245e03d59e6..f4116e96f6a1 100644 --- a/drivers/usb/typec/altmodes/displayport.c +++ b/drivers/usb/typec/altmodes/displayport.c @@ -767,8 +767,11 @@ int dp_altmode_probe(struct typec_altmode *alt) if (plug) typec_altmode_set_drvdata(plug, dp); - dp->state = plug ? DP_STATE_ENTER_PRIME : DP_STATE_ENTER; - schedule_work(&dp->work); + /* Only attempt to enter altmode if port is active. */ + if (port->active) { + dp->state = plug ? DP_STATE_ENTER_PRIME : DP_STATE_ENTER; + schedule_work(&dp->work); + } return 0; } diff --git a/drivers/usb/typec/altmodes/thunderbolt.c b/drivers/usb/typec/altmodes/thunderbolt.c index a945b9d35a1d..45567abc3bb8 100644 --- a/drivers/usb/typec/altmodes/thunderbolt.c +++ b/drivers/usb/typec/altmodes/thunderbolt.c @@ -212,6 +212,7 @@ static const struct typec_altmode_ops tbt_altmode_ops = { static int tbt_altmode_probe(struct typec_altmode *alt) { + const struct typec_altmode *port = typec_altmode_get_partner(alt); struct tbt_altmode *tbt; tbt = devm_kzalloc(&alt->dev, sizeof(*tbt), GFP_KERNEL); @@ -226,7 +227,10 @@ static int tbt_altmode_probe(struct typec_altmode *alt) typec_altmode_set_drvdata(alt, tbt); typec_altmode_set_ops(alt, &tbt_altmode_ops); - if (tbt_ready(alt)) { + /* Only attempt to enter altmode if port is active and cable/plug + * information is ready. + */ + if (port->active && tbt_ready(alt)) { if (tbt->plug[TYPEC_PLUG_SOP_PP]) tbt->state = TBT_STATE_SOP_PP_ENTER; else if (tbt->plug[TYPEC_PLUG_SOP_P]) diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c index febe453b96be..b5e67a57762c 100644 --- a/drivers/usb/typec/class.c +++ b/drivers/usb/typec/class.c @@ -458,7 +458,8 @@ static umode_t typec_altmode_attr_is_visible(struct kobject *kobj, struct typec_altmode *adev = to_typec_altmode(kobj_to_dev(kobj)); if (attr == &dev_attr_active.attr) - if (!adev->ops || !adev->ops->activate) + if (!is_typec_port(adev->dev.parent) && + (!adev->ops || !adev->ops->activate)) return 0444; return attr->mode; @@ -563,7 +564,7 @@ typec_register_altmode(struct device *parent, if (is_port) { alt->attrs[3] = &dev_attr_supported_roles.attr; - alt->adev.active = true; /* Enabled by default */ + alt->adev.active = !desc->inactive; /* Enabled by default */ } sprintf(alt->group_name, "mode%d", desc->mode); diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h index d616b8807000..56c01771c190 100644 --- a/include/linux/usb/typec.h +++ b/include/linux/usb/typec.h @@ -140,6 +140,7 @@ int typec_cable_set_identity(struct typec_cable *cable); * @mode: Index of the Mode * @vdo: VDO returned by Discover Modes USB PD command * @roles: Only for ports. DRP if the mode is available in both roles + * @inactive: Only for ports. Make this port inactive (default is active). * * Description of an Alternate Mode which a connector, cable plug or partner * supports. @@ -150,6 +151,7 @@ struct typec_altmode_desc { u32 vdo; /* Only used with ports */ enum typec_port_data roles; + bool inactive : 1; }; void typec_partner_set_pd_revision(struct typec_partner *partner, u16 pd_revision);
Enforce the same requirement as when we attempt to activate altmode via sysfs (do not enter partner mode if port mode is not active). In order to set a default value for this field, also introduce the inactive field in struct typec_altmode_desc. Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org> --- Changes in v3: - Use port.active instead of introducing auto-enter field - Introduce inactive field to typec_altmode_desc to set default value for active. - Always make port 'active' field writable drivers/usb/typec/altmodes/displayport.c | 7 +++++-- drivers/usb/typec/altmodes/thunderbolt.c | 6 +++++- drivers/usb/typec/class.c | 5 +++-- include/linux/usb/typec.h | 2 ++ 4 files changed, 15 insertions(+), 5 deletions(-)