Skip to contentSkip to conversation

extcon : callback function to read cable property

PATCH MyungJoo Ham 3 participants 3 messages Nov 20, 2012 – Nov 22, 2012

lore
Share & tools
lore

MyungJoo Ham

author
#0 lore raw
Quoted 22 lines · Anton Vorontsov

The idea of using union seemed good to me, what happened to it?

I mean, MyungJoo Ham wrote:

| 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;

This sounds good to me...

Quoted 9 lines
Quoted 1 line

* extcon is not designed to support any cable state other than CONNECT/DISCONNECT

Dunno for this one. Can we get these additional states via "properties" as described above?

Thanks,
Anton.

The "RFC" patch for this feature is now shown at:
http://git.kernel.org/?p=linux/kernel/git/mzx/extcon.git;a=commit;h=5655aeef83addaec77a3b9387a3dd18b6c146dd5
(Note that "for-add-feature" branch is sort of "RFC" branch)

I'm now considering relaying notifications of data updates possible via the notifier block of register-interest. Any inputs are welcomed.

Anyway, for the USB issue of "suspend/resume at host-side and current-limit at device-side", we will need to
1. update regulator subsystem to have notification for current change (it does for enable/disable/voltage-changes)
2. let the charger use current-regulator
3. let the one who detects (usb driver?) changes at host-side change the current limit of that current-regulator
4. let the event from current-regulator relayed via extcon.
We may use power-supply class as well for this issue. (may need to update if power-supply class does not have notifications and suspend/resume states)

Cheers,
MyungJoo

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

#1 lore raw
Quoted 9 lines

The "RFC" patch for this feature is now shown at:
http://git.kernel.org/?p=linux/kernel/git/mzx/extcon.git;a=commit;h=5655a
eef83addaec77a3b9387a3dd18b6c146dd5
(Note that "for-add-feature" branch is sort of "RFC" branch)

I'm now considering relaying notifications of data updates possible via the notifier block of register-interest. Any inputs are welcomed.

Quoted 9 lines

Anyway, for the USB issue of "suspend/resume at host-side and current-limit at device-side", we will need to 1. update regulator subsystem to have notification for current change (it does for enable/disable/voltage-changes)
2. let the charger use current-regulator 3. let the one who detects (usb driver?) changes at host-side change the current limit of that current- regulator 4. let the event from current-regulator relayed via extcon. We may use power-supply class as well for this issue. (may need to update if power-supply class does not have notifications and suspend/resume states)

For this solution to work, the cable provider itself
need to register with any of these subsystems - power_supply/regulator/charger manager IMHO a cable provider shouldn't register itself with any of the subsystem.

For example it cannot register with power_supply subsystem
since it's not a power_supply. It's just source for a power_supply. We register either charger/battery with power supply. I couldn't find a way to register the cable with power supply subsystem.

Anton,
Do you have any suggestion here?

I think the same case with regulator framework also. A cable doesn’t belong
to a regulator framework. A cable doesn’t expose any control attribute (current control/voltage control). It just have properties (eg current) controlled by external agents (Host machine/wall charger)

And charger manager is a consumer and not a provider. It cannot decide the charger cable capabilities.

I have a modified version of the above patch which uses properties directly.
https://gitorious.org/linux-charging-framework/linux-charging-framework/commit/ff358373dcb32027ebe1a267fc8b8999a3bd37e4

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

#2 lore raw

On Thu, Nov 22, 2012 at 01:00:52PM +0000, Tc, Jenny wrote:
[...]

Quoted 8 lines

For this solution to work, the cable provider itself
need to register with any of these subsystems - power_supply/regulator/charger manager IMHO a cable provider shouldn't register itself with any of the subsystem.

For example it cannot register with power_supply subsystem
since it's not a power_supply. It's just source for a power_supply. We register either charger/battery with power supply. I couldn't find a way to register the cable with power supply subsystem.

Yes, I guess that doesn't make much sense.

Quoted 11 lines

Anton,
Do you have any suggestion here?

I think the same case with regulator framework also. A cable doesn��t belong
to a regulator framework. A cable doesn��t expose any control attribute (current control/voltage control). It just have properties (eg current) controlled by external agents (Host machine/wall charger)

And charger manager is a consumer and not a provider. It cannot decide the charger cable capabilities.

I have a modified version of the above patch which uses properties directly.
https://gitorious.org/linux-charging-framework/linux-charging-framework/commit/ff358373dcb32027ebe1a267fc8b8999a3bd37e4

(Please, always inline the patches.)

I spent some time trying to understand what exactly you're trying to accomplish and I failed, and that's why:

1. I see only some small snippets of the code, sometimes by means of URLs and references to another patches, which is hard to follow when you have like tens of emails in the thread;
2. I believe I still didn't see a user of this callback.

So, basically you're trying to force me to read your mind and guess your ideas, but as it appears, I'm not good at it. :)

So, please do the following:

- Prepare a complete, but minimal workable solution, a patchset that shows how exactly you use the new features that you introduce;

- The series must be an all-in-1 patchset, so people won't need to go back and forth trying to understand how things depend on each other;

- Briefly describe how things work, a typical use-case and how drivers interact with each other. E.g. which driver registers extcon_device, which driver reads mA field, which driver sets mA field (and based on what information), which driver listens for the extcon events, which driver produces the extcon events, etc. No need for lengthy explanations about why you made the decisions, just a brief explanation of how things currently work and interact.

Thanks,
Anton.