extcon : callback function to read cable property

PATCH MyungJoo Ham 1 participant 1 message Nov 20, 2012

Share & tools

MyungJoo Ham

author

2012-11-20 03:45 UTC #0 lore raw

Quoted 101 lines

Quoted 90 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
I think that the role of extcon subsystem notify changed state(attached/detached) of cable to notifiee, but if you want to add property feature of cable, you should solve ambiguous issues.
First,
This patch only support the properties of charger cable but, never support property of other cable. The following structure depend on only charger cable. We can check it the following
structure:
Quoted 51 lines
Quoted 51 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
  struct extcon_chrg_cbl_props {
          enum extcon_chrgr_cbl_stat cable_state;
          unsigned long mA;
  };
I think that the patch have to support all of cables for property
feature.
Quoted 17 lines
Quoted 17 lines
Quoted 6 lines
My suggestion is to have a structure like this
struct  extcon_cablel_props {
    unsigned int cable_state;
            unsigned int data;
Why can't it be float/long/double??
Quoted 10 lines
}
Not all cables will have more than two states. If any cable has more than two states, the above structure makes it flexible to represent additional state and the data associated
Quoted 5 lines
Second,
Certainly, you should define common properties of all cables and specific properties of each cable. The properties of charger cable should never be defined only.
Quoted 6 lines
Quoted 6 lines
IMHO the extcon doesn't know anything about the cable except the state which is currently it is in and which also is set by the provider.I am unable to understand why should extcon provide more than what it knows?It should just limit itself to broadcasting the cable state and exploiting it for any other information would only lead to
more un-necessary code.
Quoted 9 lines
Quoted 9 lines
It should be same as IIO subsystem where the consumer and provider knows before hand what information they are going to share and with what precision and IIO core is just a way to do that.It doesn't know anything beyond what is given by the provider.
Same is the case with driver core where individual driver sets it's own private data and the driver core just gives that private data back to the driver as and when it needs but parsing the private data in the right way is up to the individual driver.
Quoted 4 lines
Quoted 1 line
I fail to understand why is not the case here.
The requirement is different from the IIO case. I am trying to extend the Power Supply class to support charging in a generic way
(https://lkml.org/lkml/2012/10/18/219).
Quoted 2 lines
The extcon consumer in this case is not a device driver. It's part of power
supply class driver itself.
Quoted 3 lines
I am open to any solution to get the cable properties dynamically. Do you find a better but generic mechanism for this?
Quoted 7 lines
Quoted 7 lines
Quoted 5 lines
I am trying to extend extcon just because I couldn’t find any other subsystem which gives cable notifications. IMHO, it's good if we can avoid consumer and provider driver level dependencies just by extending extcon. This will ensure that the same driver will work on any
platform as long as it's having the dependency only on extcon.
Quoted 1 line
We shouldn't put any driver level dependency between extcon
consumer
Quoted 10 lines
Quoted 10 lines
and provider.
Quoted 3 lines
When we do like that, the extcon consumer is expecting the similar implementation for the provider on all platforms. This may not be possible
Is there anything wrong in assuming similar implementation for all the providers?I think the providers know what it can provide and this may vary quite a lot.Or can we make a generic provider driver which will encompass all the randomness in the various provider drivers?This generic driver will get all the properties and since it will be generice anyone can use it to know what properties to expect.This
will keep the extcon design intact.
Quoted 2 lines
Maintainer??
I agreed about opinion of anish singh. The extcon provider driver provide generic
----
struct  extcon_cablel_props {
     unsigned int cable_state;
     unsigned int data;
}
Quoted 10 lines
----
You suggested upper structure and said it is only flexible to represent additional state, But, it is non-standard. What store real data on "unsigned int data"? It isn't determined and flexible. That is extcon consumer driver should already know type of real data or value of real data. The extcon consumer driver has strong dependency on extcon provider driver. In this case, if extcon provider driver can change data value of "unsigned int data", extcon consumer provider have to be modified according to extcon provider driver. I think it isn't correct apporach. So, I proposed that we should define properties for all cables.

Quoted 6 lines

I couldn’t find properties common for all type of cables. Alternate method I can think of is a new driver "extcon-charger.c". This driver will register with extcon subsystem.
The consumer and provider can use the APIs and properties exposed by this driver. This way we can ensure that extcon design is intact and the additional charger cable state and properties can be handled by this driver. Does that make sense?

Adding another API at a device driver is something to be avoided unless it's inevitable.

The state/status information required by a charger should be embedded in the data structure of the charger (e.g., regulator, power-supply-class, or charger-manager). However, we may consider bridging them via extcon anyway.

We may have:

	enum extcon_cable_type {
		EXTCON_CT_REGULATOR,
		EXTCON_CT_PSY,
		EXTCON_CT_CHARGER_CB,
		...
	};
and have the following included at struct extcon_cable:
	union {
		struct regulator *reg;
		struct power_supply *psy;
		struct charger_cable *charger_cb;
		...
	} cable data;
	enum extcon_cable_type cable_type;

Is there any problems with this approach?

If you need to embeded "current-limit" information, the regulator should be able to provide it (current-limit regulator).
If you need to embed "suspend/resume" status information in general for a specific cable type, you need to define corresponding data structure related to the cable type (class) and make it connected via extcon.

Quoted 78 lines

Quoted 78 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
and does not seems to be a scalable solution. IMHO, the extcon should provide a mechanism to retrieve the cable properties. Consumer drivers can use this API to get the cable properties without knowing the provider driver implementation. This will make the extcon drivers more
Quoted 4 lines
Quoted 4 lines
scalable and reusable across multiple platforms.
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Hope above structure would be enough to represent the common
cable
Quoted 12 lines
Quoted 12 lines
Quoted 12 lines
Quoted 12 lines
Quoted 6 lines
state and it's data. If a cable has more than two states, then extcon_update_state can be used to notify the consumer 1)Provider can just toggle the "state" argument to notify the consumer that cable state is changed OR
2) Add one more argument like extcon_update_state(struct extcon_dev *edev, u32 mask, u32 state1, u32 sate2)
This will cause other drivers to change such as arizona.
Quoted 5 lines
If the state2 is set, then consumers can use get_cable_properties() to query the cable properties. State2 need to be used only if the cable need to represent more than two state
Quoted 1 line
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 3 lines
Quoted 3 lines
Third,
If we finish to decide the properties for all cables, I'd like to see a example
Why do we think that state and property is the only thing which the consumer want to know?I am sure there would be some more
properties
Quoted 41 lines
Quoted 41 lines
Quoted 41 lines
Quoted 41 lines
which would be of some interest to consumers and there is quite a possibility that in future we might get a patch for that also.So instead of that limiting it to just state is a good idea.
Quoted 6 lines
Quoted 1 line
code.
Agreed. If we agree on the above structure, I can modify the charging subsystem patch to use the same structure.
(https://lkml.org/lkml/2012/10/18/219). This would give a reference for the
new feature.
Quoted 8 lines
Quoted 8 lines
You explained following changed state of USB according to Host state on other patch.
---------------------------
For USB2.0
1) CONNECT (100mA)->UPDATE(500mA)->DISCONNECT(0mA)
2) CONNECT (100mA)->UPDATE(500mA)->HOST
SUSPEND(2.5mA/500mA)-
Quoted 2 lines
Quoted 2 lines
Quoted 1 line
DISCONNECT(0mA)
3) CONNECT (100mA)->UPDATE(500mA)->HOST
SUSPEND(2.5mA/500mA)-
Quoted 5 lines
Quoted 5 lines
Quoted 1 line
HOST RESUME(500mA)->DISCONNECT(0mA)
For USB 3.0
4) CONNECT (150mA)->UPDATE(900mA)->DISCONNECT(0mA)
5) CONNECT (150mA)->UPDATE(900mA)-> HOST
SUSPEND(2.5mA/900mA)-
Quoted 2 lines
Quoted 2 lines
Quoted 1 line
DISCONNECT(0mA)
6) CONNECT (100mA)->UPDATE(900mA)->HOST
SUSPEND(2.5mA/900mA)-
Quoted 10 lines
Quoted 9 lines
Quoted 1 line
HOST RESUME(900mA)->DISCONNECT(0mA)
---------------------------
I have a question. Can the provider device driver(e.g., extcon-max77693.c/
extcon-max8997.c) detect the changed state of host? I'd like to see the example device driver that the provider device driver detect changed state of host.
Could you provide example device driver?
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 2 lines
Good question. The OTG drivers are capable of identifying the SUSPEND
event.
Quoted 2 lines
System cannot setup SDP (USB host) charging with maximum charging current - 500mA (USB2.0/ 900mA(USB 3)) without enumerating the
USB.

Quoted 16 lines

Quoted 8 lines
Quoted 6 lines
Quoted 6 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
The USB enumeration can be done only with a USB/OTG driver. IMHO the above extcon drivers (extcon-max77693.c/ extcon-max8997.c) are not capable of doing the USB enumeration and identifying the charge current. They can just identify the charger type -
SDP/DCP/CDP/ACA/AC.
Quoted 1 line
Quoted 1 line
Quoted 1 line
The intelligence for USB enumeration should be inside USB/OTG
driver.

�淸�{.n�+돴윯돪�+%듚�깁負쪐w��{.n�+돴쪐{깸G쳺��{ay��,j�f＂톒쉹��묎�z_溫(�쉸듶줷"얎�m��G쳺�◀?솳鈺�&ｘ�~뤳턨O뺞촾퇉v�^�m��띠�◀?뻀�