Zack's Kernel News

Struggling To Support USB Type-C

Heikki Krogerus posted a patch to add support for USB's Type-C connector class. As he put it, "The purpose of the USB Type-C connector class is to provide a unified interface for the user space to get the status and basic information about USB Type-C connectors on a system, take control over data role swapping, and, when the port supports USB Power Delivery, also control power role swapping and Alternate Modes."

Heikki's interface went into the sysfs directory and exported a lot of data about current operational role, current power role, the port VCONN Source, and many other data items. Ports would be named usbc0, usbc1, and so on.

Greg Kroah-Hartman offered some comments. After a cursory glance, he liked the patch, but there were a few things that needed to change. They went over some of those technical details together. Other people joined in, and Heikki spun out a few more versions of the patch. Among the issues under discussion were temporary problems like memory leaks. In terms of the overall feature, everyone seemed to be basically in support of it.

The thing about USB Type-C is that it's the new hotness in terms of data exchange and power delivery. Support for it is not optional if Linux wants to retain world domination status. For example, the MacBook Pro relies on USB-C. Among everything else, you can plug the connector in either right side up or upside down, a feature other USB connectors don't have.

At the same time, even though it's indispensable, the developers still have to figure out whether various behaviors should be set as module parameters, boot options, sysfs files, IOCTLs, system calls, or any number of other possibilities.

It's also necessary, as was pointed out in the discussion, to compensate for deficiencies in manufacturer's firmware. As Heikki put it at one point, "Unfortunately, we cannot assume the firmware to be always correct. Companies love to recycle the firmware. We are going to see products from a company X that should prefer source role, a desktop for example, but still give the OS a device property that says otherwise. The reason for that is most likely because the previous product from that company was some kind of mobile device."

While Greg at one point said, "firmware is 'hard', but it gets really really tiring constantly having to paper over firmware and hardware bugs in the kernel just because we seem to be the ones that are willing to actually fix problems that others cause."

Ultimately, Greg took a closer look at Heikki's patch and discovered some eyebrow-raising constructions, and the deeper he looked, the worse the code appeared to him. Eventually, he told Heikki that the patch needed a lot more work, and that "I'm now going to require that you get other internal Intel developers to sign off on this code before I review it again. You have resources at your disposal that others do not with your internal mailing lists containing senior kernel developers. Use it and don't waste the community's time to do basic code review that they should be doing instead."

So Greg chucked the code back to the mother ship, Intel, for a thorough going-over before it could even enter the sphere of kernel developers again.

Guenter Roeck, who had previously given his "tested-by" thumbs up for this patch, now said to Heikki, "This hurts both your and my reputation, and obviously will make me quite hesitant to add a 'Reviewed-by:' to the next version of the series." But Greg tried to tone things down, saying, "it doesn't bother me at all. I want and need your reviews of those portions that I don't know as well (i.e., the userspace api and functionality.) So don't take it personally; the driver model isn't that easy of a topic to mess with in places. Loads of people get it wrong." And he said he just really wanted those internal Intel reviews. But Heikki still apologized for the quality of the code.

Overall, a harsh debate. But not very far out of step with the sort of criticism that might normally be leveled against a patch submission. However, it's fairly rare for someone like Greg to decide to halt all reviews of a given patch until the corporation of origin does more work on it.

Blocking Hardware Input Events

Pali Rohár posted a patch to support disabling events from any input device connected to the system. Once disabled, a piece of hardware would no longer deliver any events to userspace. Some common hardware that might use this feature would be keyboards, touchpads, and touchscreens.

Nikita Yushchenko liked the patch, but Bastien Nocera thought it might need more thought. He had implemented something similar in userspace to disable touch devices when the screen has been suspended. Among other questions, he asked if Pali's patch might just be better in userspace and not be a kernel feature at all.

David Herrmann also pointed out that if you didn't want events from a device, you should simply not open that device.

Pali replied that doing this in userspace might require making changes to every piece of user software that took input, but Bastien replied that, "There's usually a display manager in between the application and the input device. Whether it's, or a Wayland compositor." And so the input could be trapped at that level, instead of in the kernel.

There was a bit more discussion before the conversation petered out. In general, though, to paraphrase Murphy's Law: Anything that can be done outside the kernel, will be done outside the kernel. There are very few exceptions, especially when there are already existing userspace implementations that are very similar to the proposed kernel feature.

Zack Brown

The Linux kernel mailing list comprises the core of Linux development activities. Traffic volumes are immense, often reaching 10,000 messages in a week, and keeping up to date with the entire scope of development is a virtually impossible task for one person. One of the few brave souls to take on this task is Zack Brown.

Buy this article as PDF

Express-Checkout as PDF
Price $2.95
(incl. VAT)

Buy Linux Magazine

Get it on Google Play

US / Canada

Get it on Google Play

UK / Australia

Related content

comments powered by Disqus

Direct Download

Read full article as PDF:

Price $2.95