Re: [PATCH v3 3/7] usb: mux: add common code for Intel dual role port mux

[Felipe Balbi]

Hi,

Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> writes:
>> On 03/11/2016 07:57 AM, Greg Kroah-Hartman wrote:
>> > On Thu, Mar 10, 2016 at 01:39:43PM +0100, Oliver Neukum wrote:
>> >> On Tue, 2016-03-08 at 15:53 +0800, Lu Baolu wrote:
>> >>
>> >>> diff --git a/Documentation/ABI/testing/sysfs-bus-platform b/Documentation/ABI/testing/sysfs-bus-platform
>> >>> index 5172a61..a2261cb 100644
>> >>> --- a/Documentation/ABI/testing/sysfs-bus-platform
>> >>> +++ b/Documentation/ABI/testing/sysfs-bus-platform
>> >>> @@ -18,3 +18,18 @@ Description:
>> >>>  		devices to opt-out of driver binding using a driver_override
>> >>>  		name such as "none".  Only a single driver may be specified in
>> >>>  		the override, there is no support for parsing delimiters.
>> >>> +
>> >>> +What:		/sys/bus/platform/devices/.../intel_mux
>> >> Hi,
>> >>
>> >> is there any reason to call this "intel_mux"? We want a common interface
>> >> for such things. So how about "role_mux" or "master_slave_mux"?
>> > I agree, don't make this intel specific, as it shouldn't be.

right, we can easily change the name. What happens when
e.g. AMD/NVidia/TI/MediaTek/Allwinner/etc come up with a similar setup
which can't be supported by Intel's mux driver ? (see more below,
agreeing on a name here doesn't seem to be enough)

>> By the way, do you expect a class for port mux in sysfs?
>
> Why would you create a class?  What is that going to do here?

well, you said you didn't want this to be Intel-specific. We could just
change the name and remove "intel_" from it, but then we would fall into
this situation where possibly different mux drivers might be
needed. That almost requires some small abstraction between sysfs and
the actual implementation, does it not ?

A bus_type doesn't fit here and a simple 'agreement' that mux drivers
will create a sysfs file named 'mux' is too fragile. Thus...

> What happened to getting internal help in designing this api?  This
> shouldn't be my job :)

I looked at this Baolu but, at least to me, it's unclear what you're
hinting here. Sure, the API is a bit odd in that we register a mux and
unregister a device (that has been fixed), but is there anything else ?

We're not asking that you design the API for us, rather just make your
comments a little clearer.

-- 
balbi

Attachment: signature.asc
Description: PGP signature