On Fri, Oct 11, 2024 at 03:07:21PM +0200, Greg Kroah-Hartman wrote:
> On Fri, Oct 11, 2024 at 03:43:59PM +0300, Heikki Krogerus wrote:
> > +What: /sys/class/typec/<port>/usb_capability
> > +Date: May 2024
>
> It's a bit past May 2024 :)
Yes. I'm sorry about that.
> > +static ssize_t
> > +usb_capability_show(struct device *dev, struct device_attribute *attr, char *buf)
> > +{
> > + enum usb_mode mode = to_typec_port(dev)->usb_mode;
> > + u8 cap = to_typec_port(dev)->cap->usb_capability;
> > + int len = 0;
> > + int i;
> > +
> > + for (i = USB_MODE_USB2; i < USB_MODE_USB4 + 1; i++) {
> > + if (!(BIT(i - 1) & cap))
> > + continue;
> > +
> > + if (i == mode)
> > + len += sysfs_emit_at(buf, len, "[%s] ", usb_modes[i]);
> > + else
> > + len += sysfs_emit_at(buf, len, "%s ", usb_modes[i]);
> > + }
> > +
> > + buf[len - 1] = '\n';
>
> Nit, shouldn't this be another call to sysfs_emit_at()?
Yes.
> > @@ -240,6 +251,7 @@ struct typec_partner_desc {
> > * @port_type_set: Set port type
> > * @pd_get: Get available USB Power Delivery Capabilities.
> > * @pd_set: Set USB Power Delivery Capabilities.
> > + * @default_usb_mode_set: USB Mode to be used by default with Enter_USB Message
> > */
> > struct typec_operations {
> > int (*try_role)(struct typec_port *port, int role);
> > @@ -250,6 +262,7 @@ struct typec_operations {
> > enum typec_port_type type);
> > struct usb_power_delivery **(*pd_get)(struct typec_port *port);
> > int (*pd_set)(struct typec_port *port, struct usb_power_delivery *pd);
> > + int (*default_usb_mode_set)(struct typec_port *port, enum usb_mode mode);
>
> Naming is hard, but usually it's "noun_verb" so wouldn't this be just
> "mode_set_default"? We know it's usb :)
>
> But why default, why not just "mode_set"? or "set_mode" given you have
> "try_role" here, but then you have "pd_set". Ick, I don't know, it's
> your code, so your call, nevermind...
I think I'll just change it this back to the way it was in the last
version, and drop "default" from the name.
thanks,
--
heikki
