Re: [PATCH 1/7] usb: typec: Copy everything from struct typec_capability during registration

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/2/19 12:16 PM, Heikki Krogerus wrote:
On Wed, Oct 02, 2019 at 07:06:55PM +0300, Heikki Krogerus wrote:
This is a bit off topic, but that attribute file is really horrible.
Right now there is no way to know the actual capability of the
port in user space. If something changes a DRP port into sink or
source, there is no way to know after that that the port is actually
DRP capable.

That statement is not correct. I'm sorry. I don't know why did I put
it that way.

I wanted to say that now the userpsace is forced to keep track of a
specific sysfs file in order to be sure of the currently supported
role, That alone is wrong, but there is no way to guarantee that
one day we would not need to support another capability in a similar
fashion, and that would mean another sysfs file, and we just forced
the userspace to be modified. The capabilities of the port really need
to be static.


I assume you refer to port_type. If I remember correctly, this is actually
used by Android userspace code to specifically select if a port can be
source, sink, or drp. I am not sure if it is a good idea to enforce
port removal and re-registration in conjunction with this - the port
can, after all, be connected to some storage device or to a monitor.
I am not sure how we could "sell" to users the idea that if they change
the port type, their screen will go dark for a few seconds.

Am I missing something ?

Thanks,
Guenter

We can handle the capability changes like I propose below without
affecting the userspace.

So that ABI is "buggy", but even without the problem, I still really
think that allowing the capabilities to be changed like that in
general is completely wrong. I don't have a problem with changing the
capabilities, but IMO it should be handled at one level higher, at the
controller device level. If the capabilities of a port need to be
changed, the old port should be removed, and a new with the new
capabilities registered. That is the only way to handle it without
making things unnecessarily difficult for the user space.

I'm pretty sure that that was my counter proposal already at the time
when the attribute file was introduced, but I don't remember why
wasn't it accepted at the time? My protest against adding that
attribute file was in any case ignored :-(. In any case, my plan was
to later propose a new sysfs group that we offer to the parent
controller devices instead assigning it to the port devices, and that
group is meant to allow port capability changes the way I explained
above. Unless of course you are against it?

thanks,

--
heikki





[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux