Messages in this thread | | | Subject | Re: [PATCH 04/12] usb: common: Small class for USB role switches | From | Hans de Goede <> | Date | Sun, 25 Feb 2018 13:16:52 +0100 |
| |
Hi,
On 16-02-18 15:07, Andy Shevchenko wrote: > On Fri, Feb 16, 2018 at 12:47 PM, Hans de Goede <hdegoede@redhat.com> wrote: > >> USB role switch is a device that can be used to choose the >> data role for USB connector. With dual-role capable USB >> controllers, the controller itself will be the switch, but >> on some platforms the USB host and device controllers are >> separate IPs and there is a mux between them and the >> connector. On those platforms the mux driver will need to >> register the switch. >> >> With USB Type-C connectors, the host-to-device relationship >> is negotiated over the Configuration Channel (CC). That >> means the USB Type-C drivers need to be in control of the >> role switch. The class provides a simple API for the USB >> Type-C drivers for the control. >> >> For other types of USB connectors (mainly microAB) the class >> provides user space control via sysfs attribute file that >> can be used to request role swapping from the switch. > >> +static int __switch_match(struct device *dev, const void *name) > > bool? > >> +{ >> + return !strcmp((const char *)name, dev_name(dev)); >> +} > > >> +static ssize_t role_store(struct device *dev, struct device_attribute *attr, >> + const char *buf, size_t size) >> +{ >> + struct usb_role_switch *sw = to_role_switch(dev); >> + int ret; >> + >> + ret = sysfs_match_string(usb_roles, buf); >> + if (ret < 0) { >> + bool res; >> + >> + /* Extra check if the user wants to disable the switch */ >> + ret = kstrtobool(buf, &res); >> + if (ret || res) >> + return -EINVAL; >> + } >> + > >> + ret = usb_role_switch_set_role(sw, ret); >> + if (!ret) >> + ret = size; >> + >> + return ret; > > Perhaps > > ret = ... > if (ret) > return ret; > > return size;
Ack, that is better, fixed for v2.
Regards,
Hans
> >> +} > >> +struct usb_role_switch * >> +usb_role_switch_register(struct device *parent, >> + const struct usb_role_switch_desc *desc) >> +{ >> + struct usb_role_switch *sw; >> + int ret; >> + >> + if (!desc || !desc->set) >> + return ERR_PTR(-EINVAL); >> + >> + sw = kzalloc(sizeof(*sw), GFP_KERNEL); >> + if (!sw) >> + return ERR_PTR(-ENOMEM); > >> + ret = device_register(&sw->dev); >> + if (ret) { >> + put_device(&sw->dev); > > Memory leak? > >> + return ERR_PTR(ret); >> + } >> + >> + /* TODO: Symlinks for the host port and the device controller. */ >> + >> + return sw; >> +} > >> +void usb_role_switch_unregister(struct usb_role_switch *sw) >> +{ >> + if (sw && !IS_ERR(sw)) > > !IS_ERR_OR_NULL() ? > >> + device_unregister(&sw->dev); >> +} >
| |