Re: [PATCH] EXTCON: Get and set cable properties
On Mon, Dec 03, 2012 at 02:09:02AM +, Tc, Jenny wrote: > > > Could you please review this. This is a follow up patch for "PATCH] > > > extcon : callback function to read cable property" > > > > While I see nothing wrong with the patch itself, I beg you to send some > > users > > for the new calls. Don't be obsessed with the extcon internals too much, > > think more about how things will interact (i.e. I really really want to see > > how > > you use these calls from the power supply drivers). > > The usage of extcon cable property is captured in patch > https://lkml.org/lkml/2012/10/18/219 > This patch uses a extcon_dev callback function get_cable_properties() to get > the > cable properties. As discussed in the previous mail thread, it may not be > good to have a extcon call > back function since the extcon provider may not be aware of the cable > properties. This patch replaces > the callback function with an API, so that whoever knows the cable property, > can set the property > using the extcon API extcon_cable_set_data(). > > The usage flow would be > 1)Consumer gets a notification from the extcon > 2)consumer reads the property using the API extcon_cable_get_data > > This way it doesn't mandatory for the extcon provider to give the cable > property. > Anyone who is aware of the cable property can set the cable property using > the API. > It makes the consumer and provider implementations very simple. > > With this new API, the callback function in patch > https://lkml.org/lkml/2012/10/18/219 can be > replaced by the API extcon_cable_set_data(). Looking at this, the whole idea of hiding power source behind the "extcon" seems dubious. Why don't you use USB device to get the current? "extcon" subsystem, as I see it, should only be used to get notified about external connectors events. And that's all. And chargers probably should not even care about extcon (well, with the exception of the direct AC/gpio power source). For USB, it would make more sense if for you'd get plug/unplug notifications *and* properties from the USB device (or OTG transceiver) directly, not from the extcon. And I guess we have this mechanism already, see drivers/power/pda_power.c. Thanks, Anton -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] EXTCON: Get and set cable properties
On Mon, Dec 03, 2012 at 02:09:02AM +, Tc, Jenny wrote: Could you please review this. This is a follow up patch for PATCH] extcon : callback function to read cable property While I see nothing wrong with the patch itself, I beg you to send some users for the new calls. Don't be obsessed with the extcon internals too much, think more about how things will interact (i.e. I really really want to see how you use these calls from the power supply drivers). The usage of extcon cable property is captured in patch https://lkml.org/lkml/2012/10/18/219 This patch uses a extcon_dev callback function get_cable_properties() to get the cable properties. As discussed in the previous mail thread, it may not be good to have a extcon call back function since the extcon provider may not be aware of the cable properties. This patch replaces the callback function with an API, so that whoever knows the cable property, can set the property using the extcon API extcon_cable_set_data(). The usage flow would be 1)Consumer gets a notification from the extcon 2)consumer reads the property using the API extcon_cable_get_data This way it doesn't mandatory for the extcon provider to give the cable property. Anyone who is aware of the cable property can set the cable property using the API. It makes the consumer and provider implementations very simple. With this new API, the callback function in patch https://lkml.org/lkml/2012/10/18/219 can be replaced by the API extcon_cable_set_data(). Looking at this, the whole idea of hiding power source behind the extcon seems dubious. Why don't you use USB device to get the current? extcon subsystem, as I see it, should only be used to get notified about external connectors events. And that's all. And chargers probably should not even care about extcon (well, with the exception of the direct AC/gpio power source). For USB, it would make more sense if for you'd get plug/unplug notifications *and* properties from the USB device (or OTG transceiver) directly, not from the extcon. And I guess we have this mechanism already, see drivers/power/pda_power.c. Thanks, Anton -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] EXTCON: Get and set cable properties
On 12/15/2012 09:16 AM, Tc, Jenny wrote: > >>> I replied on the thread and pointed out issues that I see with this >>> solution. IMHO It's not fair to register a cable with >>> power_supply/regulator/charger manager just to expose the cable >> properties. >> Don't we do that with already in allmost all drivers?Like if I have a >> keyboard >> driver and then I register with input framework and if the keyboard driver >> needs clk services then I register with clk framework as well and same with >> other services needed by keyboard driver.I think it makes sense for a cable >> to register with different framework if it supports that functionality as >> those >> services also would know that we have a cable with so and so property. > > IMHO it's not a good choice to register a cable itself with any of the three > subsystem > (power_supply/regulator/charger manager) > > 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. > > In previous discussion Anton also agreed to this. > > Anton, > Could you please confirm? > > 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. No, charger-manager include information for battery/charger/charger cable dependency on target H/W. Each target has different h/w constraint for charging, so charger-manager can determine whether specific charger cable is used on target or not. > > As I said, extcon provider driver didn't provide directly charging current(int mA) and some state(unsigned long state) because the extcon provider driver(Micro USB interface controller device or other device related to external connector) haven't mechanism which detect dynamically charging current immediately. If you wish to provide charging current data to extcon consumer driver or framework, you should use regulator/power_supply framework or other method in extcon provider >> driver. > > This information need not to come from the extcon provider. It can come from > any driver > Who knows the state of a cable. It may be a platform driver or may be a > driver belongs to any > of the three subsystem we discussed above > (power_supply/regulator/charger_manager). > Just by having an API like this (extcon_cable_set_data), it's not mandatory > to register the cable > with any of the subsystem. > Also if you want to define 'struct extcon_chrgr_cable_props', you should certainly show how detect dynamically charging current or state. > > For USB cables, it's determined by USB enumeration. For USB 3.0 it would be > 900mA and > USB 2.0 it's 500mA. Also in un configured state this would be 150 and 100mA > respectively I think only USB enumeration determine standard charging current according to version of USB. But, Extcon subsystem always need detection mechanism. (e.g, Jack/Microphone detection device of Wolfson sound codec and MUIC(Micro USB Interface Controller) device of MAXIM) You didn't show any detection mechanism which detect changed state of host system. What can some device for detecting of following host state except of CONNECT/DISCONNECT state? I don't know. Please let me know it. - SUSPEND(Host suspend for SDP cable), - RESUME(Host wakeup) - UPDATE (to increase the charge current after USB enumaeration). - CONNECT - DISCOONECT -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] EXTCON: Get and set cable properties
On 12/15/2012 09:16 AM, Tc, Jenny wrote: I replied on the thread and pointed out issues that I see with this solution. IMHO It's not fair to register a cable with power_supply/regulator/charger manager just to expose the cable properties. Don't we do that with already in allmost all drivers?Like if I have a keyboard driver and then I register with input framework and if the keyboard driver needs clk services then I register with clk framework as well and same with other services needed by keyboard driver.I think it makes sense for a cable to register with different framework if it supports that functionality as those services also would know that we have a cable with so and so property. IMHO it's not a good choice to register a cable itself with any of the three subsystem (power_supply/regulator/charger manager) 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. In previous discussion Anton also agreed to this. Anton, Could you please confirm? 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. No, charger-manager include information for battery/charger/charger cable dependency on target H/W. Each target has different h/w constraint for charging, so charger-manager can determine whether specific charger cable is used on target or not. As I said, extcon provider driver didn't provide directly charging current(int mA) and some state(unsigned long state) because the extcon provider driver(Micro USB interface controller device or other device related to external connector) haven't mechanism which detect dynamically charging current immediately. If you wish to provide charging current data to extcon consumer driver or framework, you should use regulator/power_supply framework or other method in extcon provider driver. This information need not to come from the extcon provider. It can come from any driver Who knows the state of a cable. It may be a platform driver or may be a driver belongs to any of the three subsystem we discussed above (power_supply/regulator/charger_manager). Just by having an API like this (extcon_cable_set_data), it's not mandatory to register the cable with any of the subsystem. Also if you want to define 'struct extcon_chrgr_cable_props', you should certainly show how detect dynamically charging current or state. For USB cables, it's determined by USB enumeration. For USB 3.0 it would be 900mA and USB 2.0 it's 500mA. Also in un configured state this would be 150 and 100mA respectively I think only USB enumeration determine standard charging current according to version of USB. But, Extcon subsystem always need detection mechanism. (e.g, Jack/Microphone detection device of Wolfson sound codec and MUIC(Micro USB Interface Controller) device of MAXIM) You didn't show any detection mechanism which detect changed state of host system. What can some device for detecting of following host state except of CONNECT/DISCONNECT state? I don't know. Please let me know it. - SUSPEND(Host suspend for SDP cable), - RESUME(Host wakeup) - UPDATE (to increase the charge current after USB enumaeration). - CONNECT - DISCOONECT -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] EXTCON: Get and set cable properties
Anton,
Could you please have a look at my comments below?
-jtc
> > While I see nothing wrong with the patch itself, I beg you to send
> > some users for the new calls. Don't be obsessed with the extcon
> > internals too much, think more about how things will interact (i.e. I
> > really really want to see how you use these calls from the power supply
> drivers).
>
> The usage of extcon cable property is captured in patch
> https://lkml.org/lkml/2012/10/18/219
> This patch uses a extcon_dev callback function get_cable_properties() to get
> the cable properties. As discussed in the previous mail thread, it may not be
> good to have a extcon call back function since the extcon provider may not
> be aware of the cable properties. This patch replaces the callback function
> with an API, so that whoever knows the cable property, can set the property
> using the extcon API extcon_cable_set_data().
>
> The usage flow would be
> 1)Consumer gets a notification from the extcon 2)consumer reads the
> property using the API extcon_cable_get_data
>
> This way it doesn't mandatory for the extcon provider to give the cable
> property.
> Anyone who is aware of the cable property can set the cable property using
> the API.
> It makes the consumer and provider implementations very simple.
>
> With this new API, the callback function in patch
> https://lkml.org/lkml/2012/10/18/219 can be replaced by the API
> extcon_cable_set_data().
N�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z� �w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i
RE: [PATCH] EXTCON: Get and set cable properties
> > I replied on the thread and pointed out issues that I see with this > > solution. IMHO It's not fair to register a cable with > > power_supply/regulator/charger manager just to expose the cable > properties. > Don't we do that with already in allmost all drivers?Like if I have a keyboard > driver and then I register with input framework and if the keyboard driver > needs clk services then I register with clk framework as well and same with > other services needed by keyboard driver.I think it makes sense for a cable > to register with different framework if it supports that functionality as > those > services also would know that we have a cable with so and so property. IMHO it's not a good choice to register a cable itself with any of the three subsystem (power_supply/regulator/charger manager) 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. In previous discussion Anton also agreed to this. Anton, Could you please confirm? 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. > > > As I said, extcon provider driver didn't provide directly charging > > > current(int > > > mA) and some state(unsigned long state) because the extcon provider > > > driver(Micro USB interface controller device or other device related > > > to external connector) haven't mechanism which detect dynamically > > > charging current immediately. If you wish to provide charging > > > current data to extcon consumer driver or framework, you should use > > > regulator/power_supply framework or other method in extcon provider > driver. This information need not to come from the extcon provider. It can come from any driver Who knows the state of a cable. It may be a platform driver or may be a driver belongs to any of the three subsystem we discussed above (power_supply/regulator/charger_manager). Just by having an API like this (extcon_cable_set_data), it's not mandatory to register the cable with any of the subsystem. > > > Also if you want to define 'struct extcon_chrgr_cable_props', you > > > should certainly show how detect dynamically charging current or state. For USB cables, it's determined by USB enumeration. For USB 3.0 it would be 900mA and USB 2.0 it's 500mA. Also in un configured state this would be 150 and 100mA respectively
RE: [PATCH] EXTCON: Get and set cable properties
I replied on the thread and pointed out issues that I see with this solution. IMHO It's not fair to register a cable with power_supply/regulator/charger manager just to expose the cable properties. Don't we do that with already in allmost all drivers?Like if I have a keyboard driver and then I register with input framework and if the keyboard driver needs clk services then I register with clk framework as well and same with other services needed by keyboard driver.I think it makes sense for a cable to register with different framework if it supports that functionality as those services also would know that we have a cable with so and so property. IMHO it's not a good choice to register a cable itself with any of the three subsystem (power_supply/regulator/charger manager) 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. In previous discussion Anton also agreed to this. Anton, Could you please confirm? 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. As I said, extcon provider driver didn't provide directly charging current(int mA) and some state(unsigned long state) because the extcon provider driver(Micro USB interface controller device or other device related to external connector) haven't mechanism which detect dynamically charging current immediately. If you wish to provide charging current data to extcon consumer driver or framework, you should use regulator/power_supply framework or other method in extcon provider driver. This information need not to come from the extcon provider. It can come from any driver Who knows the state of a cable. It may be a platform driver or may be a driver belongs to any of the three subsystem we discussed above (power_supply/regulator/charger_manager). Just by having an API like this (extcon_cable_set_data), it's not mandatory to register the cable with any of the subsystem. Also if you want to define 'struct extcon_chrgr_cable_props', you should certainly show how detect dynamically charging current or state. For USB cables, it's determined by USB enumeration. For USB 3.0 it would be 900mA and USB 2.0 it's 500mA. Also in un configured state this would be 150 and 100mA respectively
RE: [PATCH] EXTCON: Get and set cable properties
Anton,
Could you please have a look at my comments below?
-jtc
While I see nothing wrong with the patch itself, I beg you to send
some users for the new calls. Don't be obsessed with the extcon
internals too much, think more about how things will interact (i.e. I
really really want to see how you use these calls from the power supply
drivers).
The usage of extcon cable property is captured in patch
https://lkml.org/lkml/2012/10/18/219
This patch uses a extcon_dev callback function get_cable_properties() to get
the cable properties. As discussed in the previous mail thread, it may not be
good to have a extcon call back function since the extcon provider may not
be aware of the cable properties. This patch replaces the callback function
with an API, so that whoever knows the cable property, can set the property
using the extcon API extcon_cable_set_data().
The usage flow would be
1)Consumer gets a notification from the extcon 2)consumer reads the
property using the API extcon_cable_get_data
This way it doesn't mandatory for the extcon provider to give the cable
property.
Anyone who is aware of the cable property can set the cable property using
the API.
It makes the consumer and provider implementations very simple.
With this new API, the callback function in patch
https://lkml.org/lkml/2012/10/18/219 can be replaced by the API
extcon_cable_set_data().
N�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z� �w���?��)ߢf��^jǫy�m��@A�a���
0��h���i
RE: [PATCH] EXTCON: Get and set cable properties
On Mon, 2012-12-03 at 01:53 +, Tc, Jenny wrote:
> > We discussed about this patch and then suggest some method to resolve this
> > issue by Myungjoo Ham. Why don't you write additional feature or your
> > opinion based on following patch by Myungjoo Ham?
> > -
> > http://git.kernel.org/?p=linux/kernel/git/mzx/extcon.git;a=commitdiff;h=73
> > 12b79d69a2b9f06af4f1254bc4644751e3e3ea
> >
>
> I replied on the thread and pointed out issues that I see with this solution.
> IMHO
> It's not fair to register a cable with power_supply/regulator/charger manager
> just to
> expose the cable properties.
Don't we do that with already in allmost all drivers?Like if I have a
keyboard driver and then I register with input framework and if the
keyboard driver needs clk services then I register with clk framework as
well and same with other services needed by keyboard driver.I think it
makes sense for a cable to register with different framework if it
supports that functionality as those services also would know that we
have a cable with so and so property.
>
> >
> > On 11/28/2012 06:49 AM, Jenny TC wrote:
> > > Existing EXTCON implementation doesn't give a mechanim to read the
> > > cable properties and extra states a cable needs to support. There are
> > > scenarios where a cable can have more than two
> > > states(CONNECT/DISCONNECT/SUSPEND/RESUME etc) and can have some
> > > properties associated with cables(mA)
> > >
> > > This patch introduces interface to get and set cable properties from
> > > EXTCON framework. The cable property can be set either by the extcon
> > > cable provider or by other subsystems who know the cable properties
> > > using eth API extcon_cable_set_data()
> > >
> > > When the consumer gets a notification from the extcon, it can use the
> > > extcon_cable_get_data() to get the cable properties irrespective of
> > > who provides the cable data.
> > >
> > > This gives a single interface for setting and getting the cable
> > > properties.
> > >
> > > Signed-off-by: Jenny TC
> > > ---
> > > drivers/extcon/extcon-class.c | 30
> > ++
> > > include/linux/extcon.h| 39
> > +++
> > > 2 files changed, 69 insertions(+)
> > >
> > > diff --git a/drivers/extcon/extcon-class.c
> > > b/drivers/extcon/extcon-class.c index d398821..304f343 100644
> > > --- a/drivers/extcon/extcon-class.c
> > > +++ b/drivers/extcon/extcon-class.c
> > > @@ -545,6 +545,36 @@ int extcon_unregister_notifier(struct extcon_dev
> > > *edev, } EXPORT_SYMBOL_GPL(extcon_unregister_notifier);
> > >
> > > +/**
> > > + * extcon_cable_set_data() - Set the data structure for a cable
> > > + * @edev:the extcon device
> > > + * @cable_index: the cable index of the correspondant
> > > + * @type:type of the data structure
> > > + * @data:
> > > + */
> > > +void extcon_cable_set_data(struct extcon_dev *edev, int cable_index,
> > > +enum extcon_cable_name type,
> > > +union extcon_cable_data data)
> > > +{
> > > + edev->cables[cable_index].type = type;
> > > + edev->cables[cable_index].data = data; }
> > > +
> > > +/**
> > > + * extcon_cable_get_data() - Get the data structure for a cable
> > > + * @edev:the extcon device
> > > + * @cable_index: the cable index of the correspondant
> > > + * @type:type of the data structure
> > > + * @data:the corresponding data structure (e.g., regulator)
> > > + */
> > > +void extcon_cable_get_data(struct extcon_dev *edev, int cable_index,
> > > +enum extcon_cable_name *type,
> > > +union extcon_cable_data *data)
> > > +{
> > > + *type = edev->cables[cable_index].type;
> > > + *data = edev->cables[cable_index].data; }
> > > +
> > > static struct device_attribute extcon_attrs[] = {
> > > __ATTR(state, S_IRUGO | S_IWUSR, state_show, state_store),
> > > __ATTR_RO(name),
> > > diff --git a/include/linux/extcon.h b/include/linux/extcon.h index
> > > 2c26c14..4556cc5 100644
> > > --- a/include/linux/extcon.h
> > > +++ b/include/linux/extcon.h
> > > @@ -135,6 +135,19 @@ struct extcon_dev {
> > > struct device_attribute *d_attrs_muex; };
> > >
> > > +/* FIXME: Is this the right place for this structure definition?
> > > + * Do we need to move it to power_supply.h?
> > > + */
> > > +struct extcon_chrgr_cable_props {
> > > + unsigned long state;
> > > + int mA;
> > > +};
> >
> > As I said, extcon provider driver didn't provide directly charging
> > current(int
> > mA) and some state(unsigned long state) because the extcon provider
> > driver(Micro USB interface controller device or other device related to
> > external connector) haven't mechanism which detect dynamically charging
> > current immediately. If you wish to provide charging current data to extcon
> > consumer driver or framework, you should use regulator/power_supply
> > framework or other method in extcon provider driver.
> >
> > The patch
RE: [PATCH] EXTCON: Get and set cable properties
> > Could you please review this. This is a follow up patch for "PATCH] > > extcon : callback function to read cable property" > > While I see nothing wrong with the patch itself, I beg you to send some users > for the new calls. Don't be obsessed with the extcon internals too much, > think more about how things will interact (i.e. I really really want to see > how > you use these calls from the power supply drivers). The usage of extcon cable property is captured in patch https://lkml.org/lkml/2012/10/18/219 This patch uses a extcon_dev callback function get_cable_properties() to get the cable properties. As discussed in the previous mail thread, it may not be good to have a extcon call back function since the extcon provider may not be aware of the cable properties. This patch replaces the callback function with an API, so that whoever knows the cable property, can set the property using the extcon API extcon_cable_set_data(). The usage flow would be 1)Consumer gets a notification from the extcon 2)consumer reads the property using the API extcon_cable_get_data This way it doesn't mandatory for the extcon provider to give the cable property. Anyone who is aware of the cable property can set the cable property using the API. It makes the consumer and provider implementations very simple. With this new API, the callback function in patch https://lkml.org/lkml/2012/10/18/219 can be replaced by the API extcon_cable_set_data().
RE: [PATCH] EXTCON: Get and set cable properties
> We discussed about this patch and then suggest some method to resolve this
> issue by Myungjoo Ham. Why don't you write additional feature or your
> opinion based on following patch by Myungjoo Ham?
> -
> http://git.kernel.org/?p=linux/kernel/git/mzx/extcon.git;a=commitdiff;h=73
> 12b79d69a2b9f06af4f1254bc4644751e3e3ea
>
I replied on the thread and pointed out issues that I see with this solution.
IMHO
It's not fair to register a cable with power_supply/regulator/charger manager
just to
expose the cable properties.
>
> On 11/28/2012 06:49 AM, Jenny TC wrote:
> > Existing EXTCON implementation doesn't give a mechanim to read the
> > cable properties and extra states a cable needs to support. There are
> > scenarios where a cable can have more than two
> > states(CONNECT/DISCONNECT/SUSPEND/RESUME etc) and can have some
> > properties associated with cables(mA)
> >
> > This patch introduces interface to get and set cable properties from
> > EXTCON framework. The cable property can be set either by the extcon
> > cable provider or by other subsystems who know the cable properties
> > using eth API extcon_cable_set_data()
> >
> > When the consumer gets a notification from the extcon, it can use the
> > extcon_cable_get_data() to get the cable properties irrespective of
> > who provides the cable data.
> >
> > This gives a single interface for setting and getting the cable properties.
> >
> > Signed-off-by: Jenny TC
> > ---
> > drivers/extcon/extcon-class.c | 30
> ++
> > include/linux/extcon.h| 39
> +++
> > 2 files changed, 69 insertions(+)
> >
> > diff --git a/drivers/extcon/extcon-class.c
> > b/drivers/extcon/extcon-class.c index d398821..304f343 100644
> > --- a/drivers/extcon/extcon-class.c
> > +++ b/drivers/extcon/extcon-class.c
> > @@ -545,6 +545,36 @@ int extcon_unregister_notifier(struct extcon_dev
> > *edev, } EXPORT_SYMBOL_GPL(extcon_unregister_notifier);
> >
> > +/**
> > + * extcon_cable_set_data() - Set the data structure for a cable
> > + * @edev: the extcon device
> > + * @cable_index: the cable index of the correspondant
> > + * @type: type of the data structure
> > + * @data:
> > + */
> > +void extcon_cable_set_data(struct extcon_dev *edev, int cable_index,
> > + enum extcon_cable_name type,
> > + union extcon_cable_data data)
> > +{
> > + edev->cables[cable_index].type = type;
> > + edev->cables[cable_index].data = data; }
> > +
> > +/**
> > + * extcon_cable_get_data() - Get the data structure for a cable
> > + * @edev: the extcon device
> > + * @cable_index: the cable index of the correspondant
> > + * @type: type of the data structure
> > + * @data: the corresponding data structure (e.g., regulator)
> > + */
> > +void extcon_cable_get_data(struct extcon_dev *edev, int cable_index,
> > + enum extcon_cable_name *type,
> > + union extcon_cable_data *data)
> > +{
> > + *type = edev->cables[cable_index].type;
> > + *data = edev->cables[cable_index].data; }
> > +
> > static struct device_attribute extcon_attrs[] = {
> > __ATTR(state, S_IRUGO | S_IWUSR, state_show, state_store),
> > __ATTR_RO(name),
> > diff --git a/include/linux/extcon.h b/include/linux/extcon.h index
> > 2c26c14..4556cc5 100644
> > --- a/include/linux/extcon.h
> > +++ b/include/linux/extcon.h
> > @@ -135,6 +135,19 @@ struct extcon_dev {
> > struct device_attribute *d_attrs_muex; };
> >
> > +/* FIXME: Is this the right place for this structure definition?
> > + * Do we need to move it to power_supply.h?
> > + */
> > +struct extcon_chrgr_cable_props {
> > + unsigned long state;
> > + int mA;
> > +};
>
> As I said, extcon provider driver didn't provide directly charging current(int
> mA) and some state(unsigned long state) because the extcon provider
> driver(Micro USB interface controller device or other device related to
> external connector) haven't mechanism which detect dynamically charging
> current immediately. If you wish to provide charging current data to extcon
> consumer driver or framework, you should use regulator/power_supply
> framework or other method in extcon provider driver.
>
> The patch suggested by Myungjoo Ham define following struct:
> union extcon_cable_data {
> struct regualtor *reg; /* EXTCON_CT_REGULATOR */
> struct power_supply *psy; /* EXTCON_CT_PSY */
> struct charger_cable *charger; /*
> EXTCON_CT_CHARGER_MANAGER */
> /* Please add accordingly with enum extcon_cable_type */
> };
>
> Also if you want to define 'struct extcon_chrgr_cable_props', you should
> certainly show how detect dynamically charging current or state.
>
> > +
> > +union extcon_cable_data {
> > + struct extcon_chrgr_cable_props chrgr_cbl_props;
> > + /* Please add accordingly*/
> > +};
> > +
> > /**
> > * struct extcon_cable - An internal data
Re: [PATCH] EXTCON: Get and set cable properties
On Sun, Dec 02, 2012 at 06:53:17AM +, Tc, Jenny wrote: > Could you please review this. This is a follow up patch for "PATCH] > extcon : callback function to read cable property" While I see nothing wrong with the patch itself, I beg you to send some users for the new calls. Don't be obsessed with the extcon internals too much, think more about how things will interact (i.e. I really really want to see how you use these calls from the power supply drivers). Thanks, Anton. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] EXTCON: Get and set cable properties
We discussed about this patch and then suggest some method to resolve
this issue by Myungjoo Ham. Why don't you write additional feature or
your opinion based on following patch by Myungjoo Ham?
-
http://git.kernel.org/?p=linux/kernel/git/mzx/extcon.git;a=commitdiff;h=7312b79d69a2b9f06af4f1254bc4644751e3e3ea
On 11/28/2012 06:49 AM, Jenny TC wrote:
> Existing EXTCON implementation doesn't give a mechanim to read the cable
> properties and extra states a cable needs to support. There are scenarios
> where a cable can have more than two states(CONNECT/DISCONNECT/SUSPEND/RESUME
> etc)
> and can have some properties associated with cables(mA)
>
> This patch introduces interface to get and set cable properties
> from EXTCON framework. The cable property can be set either by
> the extcon cable provider or by other subsystems who know the cable
> properties using eth API extcon_cable_set_data()
>
> When the consumer gets a notification from the extcon, it can use the
> extcon_cable_get_data() to get the cable properties irrespective of who
> provides the cable data.
>
> This gives a single interface for setting and getting the cable properties.
>
> Signed-off-by: Jenny TC
> ---
> drivers/extcon/extcon-class.c | 30 ++
> include/linux/extcon.h| 39 +++
> 2 files changed, 69 insertions(+)
>
> diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
> index d398821..304f343 100644
> --- a/drivers/extcon/extcon-class.c
> +++ b/drivers/extcon/extcon-class.c
> @@ -545,6 +545,36 @@ int extcon_unregister_notifier(struct extcon_dev *edev,
> }
> EXPORT_SYMBOL_GPL(extcon_unregister_notifier);
>
> +/**
> + * extcon_cable_set_data() - Set the data structure for a cable
> + * @edev:the extcon device
> + * @cable_index: the cable index of the correspondant
> + * @type:type of the data structure
> + * @data:
> + */
> +void extcon_cable_set_data(struct extcon_dev *edev, int cable_index,
> +enum extcon_cable_name type,
> +union extcon_cable_data data)
> +{
> + edev->cables[cable_index].type = type;
> + edev->cables[cable_index].data = data;
> +}
> +
> +/**
> + * extcon_cable_get_data() - Get the data structure for a cable
> + * @edev:the extcon device
> + * @cable_index: the cable index of the correspondant
> + * @type:type of the data structure
> + * @data:the corresponding data structure (e.g., regulator)
> + */
> +void extcon_cable_get_data(struct extcon_dev *edev, int cable_index,
> +enum extcon_cable_name *type,
> +union extcon_cable_data *data)
> +{
> + *type = edev->cables[cable_index].type;
> + *data = edev->cables[cable_index].data;
> +}
> +
> static struct device_attribute extcon_attrs[] = {
> __ATTR(state, S_IRUGO | S_IWUSR, state_show, state_store),
> __ATTR_RO(name),
> diff --git a/include/linux/extcon.h b/include/linux/extcon.h
> index 2c26c14..4556cc5 100644
> --- a/include/linux/extcon.h
> +++ b/include/linux/extcon.h
> @@ -135,6 +135,19 @@ struct extcon_dev {
> struct device_attribute *d_attrs_muex;
> };
>
> +/* FIXME: Is this the right place for this structure definition?
> + * Do we need to move it to power_supply.h?
> + */
> +struct extcon_chrgr_cable_props {
> + unsigned long state;
> + int mA;
> +};
As I said, extcon provider driver didn't provide directly charging current(int
mA)
and some state(unsigned long state) because the extcon provider driver(Micro
USB interface controller device or other device related to external connector)
haven't
mechanism which detect dynamically charging current immediately. If you wish to
provide charging current data to extcon consumer driver or framework, you should
use regulator/power_supply framework or other method in extcon provider driver.
The patch suggested by Myungjoo Ham define following struct:
union extcon_cable_data {
struct regualtor *reg; /* EXTCON_CT_REGULATOR */
struct power_supply *psy; /* EXTCON_CT_PSY */
struct charger_cable *charger; /* EXTCON_CT_CHARGER_MANAGER */
/* Please add accordingly with enum extcon_cable_type */
};
Also if you want to define 'struct extcon_chrgr_cable_props', you should
certainly show how
detect dynamically charging current or state.
> +
> +union extcon_cable_data {
> + struct extcon_chrgr_cable_props chrgr_cbl_props;
> + /* Please add accordingly*/
> +};
> +
> /**
> * struct extcon_cable - An internal data for each cable of extcon
> device.
> * @edev The extcon device
> @@ -143,6 +156,8 @@ struct extcon_dev {
> * @attr_name"name" sysfs entry
> * @attr_state "state" sysfs entry
> * @attrsArray pointing to attr_name and attr_state for attr_g
> + * @type:The type of @data.
> + * @data:The data structure representing
Re: [PATCH] EXTCON: Get and set cable properties
We discussed about this patch and then suggest some method to resolve
this issue by Myungjoo Ham. Why don't you write additional feature or
your opinion based on following patch by Myungjoo Ham?
-
http://git.kernel.org/?p=linux/kernel/git/mzx/extcon.git;a=commitdiff;h=7312b79d69a2b9f06af4f1254bc4644751e3e3ea
On 11/28/2012 06:49 AM, Jenny TC wrote:
Existing EXTCON implementation doesn't give a mechanim to read the cable
properties and extra states a cable needs to support. There are scenarios
where a cable can have more than two states(CONNECT/DISCONNECT/SUSPEND/RESUME
etc)
and can have some properties associated with cables(mA)
This patch introduces interface to get and set cable properties
from EXTCON framework. The cable property can be set either by
the extcon cable provider or by other subsystems who know the cable
properties using eth API extcon_cable_set_data()
When the consumer gets a notification from the extcon, it can use the
extcon_cable_get_data() to get the cable properties irrespective of who
provides the cable data.
This gives a single interface for setting and getting the cable properties.
Signed-off-by: Jenny TC jenny...@intel.com
---
drivers/extcon/extcon-class.c | 30 ++
include/linux/extcon.h| 39 +++
2 files changed, 69 insertions(+)
diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
index d398821..304f343 100644
--- a/drivers/extcon/extcon-class.c
+++ b/drivers/extcon/extcon-class.c
@@ -545,6 +545,36 @@ int extcon_unregister_notifier(struct extcon_dev *edev,
}
EXPORT_SYMBOL_GPL(extcon_unregister_notifier);
+/**
+ * extcon_cable_set_data() - Set the data structure for a cable
+ * @edev:the extcon device
+ * @cable_index: the cable index of the correspondant
+ * @type:type of the data structure
+ * @data:
+ */
+void extcon_cable_set_data(struct extcon_dev *edev, int cable_index,
+enum extcon_cable_name type,
+union extcon_cable_data data)
+{
+ edev-cables[cable_index].type = type;
+ edev-cables[cable_index].data = data;
+}
+
+/**
+ * extcon_cable_get_data() - Get the data structure for a cable
+ * @edev:the extcon device
+ * @cable_index: the cable index of the correspondant
+ * @type:type of the data structure
+ * @data:the corresponding data structure (e.g., regulator)
+ */
+void extcon_cable_get_data(struct extcon_dev *edev, int cable_index,
+enum extcon_cable_name *type,
+union extcon_cable_data *data)
+{
+ *type = edev-cables[cable_index].type;
+ *data = edev-cables[cable_index].data;
+}
+
static struct device_attribute extcon_attrs[] = {
__ATTR(state, S_IRUGO | S_IWUSR, state_show, state_store),
__ATTR_RO(name),
diff --git a/include/linux/extcon.h b/include/linux/extcon.h
index 2c26c14..4556cc5 100644
--- a/include/linux/extcon.h
+++ b/include/linux/extcon.h
@@ -135,6 +135,19 @@ struct extcon_dev {
struct device_attribute *d_attrs_muex;
};
+/* FIXME: Is this the right place for this structure definition?
+ * Do we need to move it to power_supply.h?
+ */
+struct extcon_chrgr_cable_props {
+ unsigned long state;
+ int mA;
+};
As I said, extcon provider driver didn't provide directly charging current(int
mA)
and some state(unsigned long state) because the extcon provider driver(Micro
USB interface controller device or other device related to external connector)
haven't
mechanism which detect dynamically charging current immediately. If you wish to
provide charging current data to extcon consumer driver or framework, you should
use regulator/power_supply framework or other method in extcon provider driver.
The patch suggested by Myungjoo Ham define following struct:
union extcon_cable_data {
struct regualtor *reg; /* EXTCON_CT_REGULATOR */
struct power_supply *psy; /* EXTCON_CT_PSY */
struct charger_cable *charger; /* EXTCON_CT_CHARGER_MANAGER */
/* Please add accordingly with enum extcon_cable_type */
};
Also if you want to define 'struct extcon_chrgr_cable_props', you should
certainly show how
detect dynamically charging current or state.
+
+union extcon_cable_data {
+ struct extcon_chrgr_cable_props chrgr_cbl_props;
+ /* Please add accordingly*/
+};
+
/**
* struct extcon_cable - An internal data for each cable of extcon
device.
* @edev The extcon device
@@ -143,6 +156,8 @@ struct extcon_dev {
* @attr_namename sysfs entry
* @attr_state state sysfs entry
* @attrsArray pointing to attr_name and attr_state for attr_g
+ * @type:The type of @data.
+ * @data:The data structure representing the status and states of this
cable.
*/
struct extcon_cable {
struct
Re: [PATCH] EXTCON: Get and set cable properties
On Sun, Dec 02, 2012 at 06:53:17AM +, Tc, Jenny wrote: Could you please review this. This is a follow up patch for PATCH] extcon : callback function to read cable property While I see nothing wrong with the patch itself, I beg you to send some users for the new calls. Don't be obsessed with the extcon internals too much, think more about how things will interact (i.e. I really really want to see how you use these calls from the power supply drivers). Thanks, Anton. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] EXTCON: Get and set cable properties
We discussed about this patch and then suggest some method to resolve this
issue by Myungjoo Ham. Why don't you write additional feature or your
opinion based on following patch by Myungjoo Ham?
-
http://git.kernel.org/?p=linux/kernel/git/mzx/extcon.git;a=commitdiff;h=73
12b79d69a2b9f06af4f1254bc4644751e3e3ea
I replied on the thread and pointed out issues that I see with this solution.
IMHO
It's not fair to register a cable with power_supply/regulator/charger manager
just to
expose the cable properties.
On 11/28/2012 06:49 AM, Jenny TC wrote:
Existing EXTCON implementation doesn't give a mechanim to read the
cable properties and extra states a cable needs to support. There are
scenarios where a cable can have more than two
states(CONNECT/DISCONNECT/SUSPEND/RESUME etc) and can have some
properties associated with cables(mA)
This patch introduces interface to get and set cable properties from
EXTCON framework. The cable property can be set either by the extcon
cable provider or by other subsystems who know the cable properties
using eth API extcon_cable_set_data()
When the consumer gets a notification from the extcon, it can use the
extcon_cable_get_data() to get the cable properties irrespective of
who provides the cable data.
This gives a single interface for setting and getting the cable properties.
Signed-off-by: Jenny TC jenny...@intel.com
---
drivers/extcon/extcon-class.c | 30
++
include/linux/extcon.h| 39
+++
2 files changed, 69 insertions(+)
diff --git a/drivers/extcon/extcon-class.c
b/drivers/extcon/extcon-class.c index d398821..304f343 100644
--- a/drivers/extcon/extcon-class.c
+++ b/drivers/extcon/extcon-class.c
@@ -545,6 +545,36 @@ int extcon_unregister_notifier(struct extcon_dev
*edev, } EXPORT_SYMBOL_GPL(extcon_unregister_notifier);
+/**
+ * extcon_cable_set_data() - Set the data structure for a cable
+ * @edev: the extcon device
+ * @cable_index: the cable index of the correspondant
+ * @type: type of the data structure
+ * @data:
+ */
+void extcon_cable_set_data(struct extcon_dev *edev, int cable_index,
+ enum extcon_cable_name type,
+ union extcon_cable_data data)
+{
+ edev-cables[cable_index].type = type;
+ edev-cables[cable_index].data = data; }
+
+/**
+ * extcon_cable_get_data() - Get the data structure for a cable
+ * @edev: the extcon device
+ * @cable_index: the cable index of the correspondant
+ * @type: type of the data structure
+ * @data: the corresponding data structure (e.g., regulator)
+ */
+void extcon_cable_get_data(struct extcon_dev *edev, int cable_index,
+ enum extcon_cable_name *type,
+ union extcon_cable_data *data)
+{
+ *type = edev-cables[cable_index].type;
+ *data = edev-cables[cable_index].data; }
+
static struct device_attribute extcon_attrs[] = {
__ATTR(state, S_IRUGO | S_IWUSR, state_show, state_store),
__ATTR_RO(name),
diff --git a/include/linux/extcon.h b/include/linux/extcon.h index
2c26c14..4556cc5 100644
--- a/include/linux/extcon.h
+++ b/include/linux/extcon.h
@@ -135,6 +135,19 @@ struct extcon_dev {
struct device_attribute *d_attrs_muex; };
+/* FIXME: Is this the right place for this structure definition?
+ * Do we need to move it to power_supply.h?
+ */
+struct extcon_chrgr_cable_props {
+ unsigned long state;
+ int mA;
+};
As I said, extcon provider driver didn't provide directly charging current(int
mA) and some state(unsigned long state) because the extcon provider
driver(Micro USB interface controller device or other device related to
external connector) haven't mechanism which detect dynamically charging
current immediately. If you wish to provide charging current data to extcon
consumer driver or framework, you should use regulator/power_supply
framework or other method in extcon provider driver.
The patch suggested by Myungjoo Ham define following struct:
union extcon_cable_data {
struct regualtor *reg; /* EXTCON_CT_REGULATOR */
struct power_supply *psy; /* EXTCON_CT_PSY */
struct charger_cable *charger; /*
EXTCON_CT_CHARGER_MANAGER */
/* Please add accordingly with enum extcon_cable_type */
};
Also if you want to define 'struct extcon_chrgr_cable_props', you should
certainly show how detect dynamically charging current or state.
+
+union extcon_cable_data {
+ struct extcon_chrgr_cable_props chrgr_cbl_props;
+ /* Please add accordingly*/
+};
+
/**
* struct extcon_cable - An internal data for each cable of extcon
device.
* @edev The extcon device
@@ -143,6 +156,8 @@ struct extcon_dev {
* @attr_name name sysfs entry
* @attr_state state sysfs
RE: [PATCH] EXTCON: Get and set cable properties
Could you please review this. This is a follow up patch for PATCH] extcon : callback function to read cable property While I see nothing wrong with the patch itself, I beg you to send some users for the new calls. Don't be obsessed with the extcon internals too much, think more about how things will interact (i.e. I really really want to see how you use these calls from the power supply drivers). The usage of extcon cable property is captured in patch https://lkml.org/lkml/2012/10/18/219 This patch uses a extcon_dev callback function get_cable_properties() to get the cable properties. As discussed in the previous mail thread, it may not be good to have a extcon call back function since the extcon provider may not be aware of the cable properties. This patch replaces the callback function with an API, so that whoever knows the cable property, can set the property using the extcon API extcon_cable_set_data(). The usage flow would be 1)Consumer gets a notification from the extcon 2)consumer reads the property using the API extcon_cable_get_data This way it doesn't mandatory for the extcon provider to give the cable property. Anyone who is aware of the cable property can set the cable property using the API. It makes the consumer and provider implementations very simple. With this new API, the callback function in patch https://lkml.org/lkml/2012/10/18/219 can be replaced by the API extcon_cable_set_data().
RE: [PATCH] EXTCON: Get and set cable properties
On Mon, 2012-12-03 at 01:53 +, Tc, Jenny wrote:
We discussed about this patch and then suggest some method to resolve this
issue by Myungjoo Ham. Why don't you write additional feature or your
opinion based on following patch by Myungjoo Ham?
-
http://git.kernel.org/?p=linux/kernel/git/mzx/extcon.git;a=commitdiff;h=73
12b79d69a2b9f06af4f1254bc4644751e3e3ea
I replied on the thread and pointed out issues that I see with this solution.
IMHO
It's not fair to register a cable with power_supply/regulator/charger manager
just to
expose the cable properties.
Don't we do that with already in allmost all drivers?Like if I have a
keyboard driver and then I register with input framework and if the
keyboard driver needs clk services then I register with clk framework as
well and same with other services needed by keyboard driver.I think it
makes sense for a cable to register with different framework if it
supports that functionality as those services also would know that we
have a cable with so and so property.
On 11/28/2012 06:49 AM, Jenny TC wrote:
Existing EXTCON implementation doesn't give a mechanim to read the
cable properties and extra states a cable needs to support. There are
scenarios where a cable can have more than two
states(CONNECT/DISCONNECT/SUSPEND/RESUME etc) and can have some
properties associated with cables(mA)
This patch introduces interface to get and set cable properties from
EXTCON framework. The cable property can be set either by the extcon
cable provider or by other subsystems who know the cable properties
using eth API extcon_cable_set_data()
When the consumer gets a notification from the extcon, it can use the
extcon_cable_get_data() to get the cable properties irrespective of
who provides the cable data.
This gives a single interface for setting and getting the cable
properties.
Signed-off-by: Jenny TC jenny...@intel.com
---
drivers/extcon/extcon-class.c | 30
++
include/linux/extcon.h| 39
+++
2 files changed, 69 insertions(+)
diff --git a/drivers/extcon/extcon-class.c
b/drivers/extcon/extcon-class.c index d398821..304f343 100644
--- a/drivers/extcon/extcon-class.c
+++ b/drivers/extcon/extcon-class.c
@@ -545,6 +545,36 @@ int extcon_unregister_notifier(struct extcon_dev
*edev, } EXPORT_SYMBOL_GPL(extcon_unregister_notifier);
+/**
+ * extcon_cable_set_data() - Set the data structure for a cable
+ * @edev:the extcon device
+ * @cable_index: the cable index of the correspondant
+ * @type:type of the data structure
+ * @data:
+ */
+void extcon_cable_set_data(struct extcon_dev *edev, int cable_index,
+enum extcon_cable_name type,
+union extcon_cable_data data)
+{
+ edev-cables[cable_index].type = type;
+ edev-cables[cable_index].data = data; }
+
+/**
+ * extcon_cable_get_data() - Get the data structure for a cable
+ * @edev:the extcon device
+ * @cable_index: the cable index of the correspondant
+ * @type:type of the data structure
+ * @data:the corresponding data structure (e.g., regulator)
+ */
+void extcon_cable_get_data(struct extcon_dev *edev, int cable_index,
+enum extcon_cable_name *type,
+union extcon_cable_data *data)
+{
+ *type = edev-cables[cable_index].type;
+ *data = edev-cables[cable_index].data; }
+
static struct device_attribute extcon_attrs[] = {
__ATTR(state, S_IRUGO | S_IWUSR, state_show, state_store),
__ATTR_RO(name),
diff --git a/include/linux/extcon.h b/include/linux/extcon.h index
2c26c14..4556cc5 100644
--- a/include/linux/extcon.h
+++ b/include/linux/extcon.h
@@ -135,6 +135,19 @@ struct extcon_dev {
struct device_attribute *d_attrs_muex; };
+/* FIXME: Is this the right place for this structure definition?
+ * Do we need to move it to power_supply.h?
+ */
+struct extcon_chrgr_cable_props {
+ unsigned long state;
+ int mA;
+};
As I said, extcon provider driver didn't provide directly charging
current(int
mA) and some state(unsigned long state) because the extcon provider
driver(Micro USB interface controller device or other device related to
external connector) haven't mechanism which detect dynamically charging
current immediately. If you wish to provide charging current data to extcon
consumer driver or framework, you should use regulator/power_supply
framework or other method in extcon provider driver.
The patch suggested by Myungjoo Ham define following struct:
union extcon_cable_data {
struct regualtor *reg; /* EXTCON_CT_REGULATOR */
struct power_supply *psy; /* EXTCON_CT_PSY */
struct charger_cable *charger; /*
RE: [PATCH] EXTCON: Get and set cable properties
Hi Myungjoo/Chanwoo/Aneesh/Anton,
Could you please review this. This is a follow up patch for "PATCH] extcon :
callback function to read cable property"
-jtc
> Subject: [PATCH] EXTCON: Get and set cable properties
>
> Existing EXTCON implementation doesn't give a mechanim to read the cable
> properties and extra states a cable needs to support. There are scenarios
> where a cable can have more than two
> states(CONNECT/DISCONNECT/SUSPEND/RESUME etc) and can have some
> properties associated with cables(mA)
>
> This patch introduces interface to get and set cable properties from EXTCON
> framework. The cable property can be set either by the extcon cable
> provider or by other subsystems who know the cable properties using eth
> API extcon_cable_set_data()
>
> When the consumer gets a notification from the extcon, it can use the
> extcon_cable_get_data() to get the cable properties irrespective of who
> provides the cable data.
>
> This gives a single interface for setting and getting the cable properties.
>
> Signed-off-by: Jenny TC
> ---
> drivers/extcon/extcon-class.c | 30 ++
> include/linux/extcon.h| 39
> +++
> 2 files changed, 69 insertions(+)
>
> diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
> index d398821..304f343 100644
> --- a/drivers/extcon/extcon-class.c
> +++ b/drivers/extcon/extcon-class.c
> @@ -545,6 +545,36 @@ int extcon_unregister_notifier(struct extcon_dev
> *edev, } EXPORT_SYMBOL_GPL(extcon_unregister_notifier);
>
> +/**
> + * extcon_cable_set_data() - Set the data structure for a cable
> + * @edev:the extcon device
> + * @cable_index: the cable index of the correspondant
> + * @type:type of the data structure
> + * @data:
> + */
> +void extcon_cable_set_data(struct extcon_dev *edev, int cable_index,
> +enum extcon_cable_name type,
> +union extcon_cable_data data)
> +{
> + edev->cables[cable_index].type = type;
> + edev->cables[cable_index].data = data; }
> +
> +/**
> + * extcon_cable_get_data() - Get the data structure for a cable
> + * @edev:the extcon device
> + * @cable_index: the cable index of the correspondant
> + * @type:type of the data structure
> + * @data:the corresponding data structure (e.g., regulator)
> + */
> +void extcon_cable_get_data(struct extcon_dev *edev, int cable_index,
> +enum extcon_cable_name *type,
> +union extcon_cable_data *data)
> +{
> + *type = edev->cables[cable_index].type;
> + *data = edev->cables[cable_index].data; }
> +
> static struct device_attribute extcon_attrs[] = {
> __ATTR(state, S_IRUGO | S_IWUSR, state_show, state_store),
> __ATTR_RO(name),
> diff --git a/include/linux/extcon.h b/include/linux/extcon.h index
> 2c26c14..4556cc5 100644
> --- a/include/linux/extcon.h
> +++ b/include/linux/extcon.h
> @@ -135,6 +135,19 @@ struct extcon_dev {
> struct device_attribute *d_attrs_muex; };
>
> +/* FIXME: Is this the right place for this structure definition?
> + * Do we need to move it to power_supply.h?
> + */
> +struct extcon_chrgr_cable_props {
> + unsigned long state;
> + int mA;
> +};
> +
> +union extcon_cable_data {
> + struct extcon_chrgr_cable_props chrgr_cbl_props;
> + /* Please add accordingly*/
> +};
> +
> /**
> * struct extcon_cable - An internal data for each cable of extcon
> device.
> * @edev The extcon device
> @@ -143,6 +156,8 @@ struct extcon_dev {
> * @attr_name"name" sysfs entry
> * @attr_state "state" sysfs entry
> * @attrsArray pointing to attr_name and attr_state for attr_g
> + * @type:The type of @data.
> + * @data:The data structure representing the status and states of this
> cable.
> */
> struct extcon_cable {
> struct extcon_dev *edev;
> @@ -153,6 +168,11 @@ struct extcon_cable {
> struct device_attribute attr_state;
>
> struct attribute *attrs[3]; /* to be fed to attr_g.attrs */
> +
> + union extcon_cable_data data;
> +
> + /* extcon cable type */
> + enum extcon_cable_name type;
> };
>
> /**
> @@ -183,6 +203,17 @@ extern void extcon_dev_unregister(struct
> extcon_dev *edev); extern struct extcon_dev
> *extcon_get_extcon_dev(const char *extcon_name);
>
> /*
> + * Following APIs are for managing the status and states of each cable.
> + * For example, if a cable is represented as a regulator, then the
> +cable
> + * may have struct regulato
RE: [PATCH] EXTCON: Get and set cable properties
Hi Myungjoo/Chanwoo/Aneesh/Anton,
Could you please review this. This is a follow up patch for PATCH] extcon :
callback function to read cable property
-jtc
Subject: [PATCH] EXTCON: Get and set cable properties
Existing EXTCON implementation doesn't give a mechanim to read the cable
properties and extra states a cable needs to support. There are scenarios
where a cable can have more than two
states(CONNECT/DISCONNECT/SUSPEND/RESUME etc) and can have some
properties associated with cables(mA)
This patch introduces interface to get and set cable properties from EXTCON
framework. The cable property can be set either by the extcon cable
provider or by other subsystems who know the cable properties using eth
API extcon_cable_set_data()
When the consumer gets a notification from the extcon, it can use the
extcon_cable_get_data() to get the cable properties irrespective of who
provides the cable data.
This gives a single interface for setting and getting the cable properties.
Signed-off-by: Jenny TC jenny...@intel.com
---
drivers/extcon/extcon-class.c | 30 ++
include/linux/extcon.h| 39
+++
2 files changed, 69 insertions(+)
diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
index d398821..304f343 100644
--- a/drivers/extcon/extcon-class.c
+++ b/drivers/extcon/extcon-class.c
@@ -545,6 +545,36 @@ int extcon_unregister_notifier(struct extcon_dev
*edev, } EXPORT_SYMBOL_GPL(extcon_unregister_notifier);
+/**
+ * extcon_cable_set_data() - Set the data structure for a cable
+ * @edev:the extcon device
+ * @cable_index: the cable index of the correspondant
+ * @type:type of the data structure
+ * @data:
+ */
+void extcon_cable_set_data(struct extcon_dev *edev, int cable_index,
+enum extcon_cable_name type,
+union extcon_cable_data data)
+{
+ edev-cables[cable_index].type = type;
+ edev-cables[cable_index].data = data; }
+
+/**
+ * extcon_cable_get_data() - Get the data structure for a cable
+ * @edev:the extcon device
+ * @cable_index: the cable index of the correspondant
+ * @type:type of the data structure
+ * @data:the corresponding data structure (e.g., regulator)
+ */
+void extcon_cable_get_data(struct extcon_dev *edev, int cable_index,
+enum extcon_cable_name *type,
+union extcon_cable_data *data)
+{
+ *type = edev-cables[cable_index].type;
+ *data = edev-cables[cable_index].data; }
+
static struct device_attribute extcon_attrs[] = {
__ATTR(state, S_IRUGO | S_IWUSR, state_show, state_store),
__ATTR_RO(name),
diff --git a/include/linux/extcon.h b/include/linux/extcon.h index
2c26c14..4556cc5 100644
--- a/include/linux/extcon.h
+++ b/include/linux/extcon.h
@@ -135,6 +135,19 @@ struct extcon_dev {
struct device_attribute *d_attrs_muex; };
+/* FIXME: Is this the right place for this structure definition?
+ * Do we need to move it to power_supply.h?
+ */
+struct extcon_chrgr_cable_props {
+ unsigned long state;
+ int mA;
+};
+
+union extcon_cable_data {
+ struct extcon_chrgr_cable_props chrgr_cbl_props;
+ /* Please add accordingly*/
+};
+
/**
* struct extcon_cable - An internal data for each cable of extcon
device.
* @edev The extcon device
@@ -143,6 +156,8 @@ struct extcon_dev {
* @attr_namename sysfs entry
* @attr_state state sysfs entry
* @attrsArray pointing to attr_name and attr_state for attr_g
+ * @type:The type of @data.
+ * @data:The data structure representing the status and states of this
cable.
*/
struct extcon_cable {
struct extcon_dev *edev;
@@ -153,6 +168,11 @@ struct extcon_cable {
struct device_attribute attr_state;
struct attribute *attrs[3]; /* to be fed to attr_g.attrs */
+
+ union extcon_cable_data data;
+
+ /* extcon cable type */
+ enum extcon_cable_name type;
};
/**
@@ -183,6 +203,17 @@ extern void extcon_dev_unregister(struct
extcon_dev *edev); extern struct extcon_dev
*extcon_get_extcon_dev(const char *extcon_name);
/*
+ * Following APIs are for managing the status and states of each cable.
+ * For example, if a cable is represented as a regulator, then the
+cable
+ * may have struct regulator as its data.
+ */
+extern void extcon_cable_set_data(struct extcon_dev *edev, int
cable_index,
+ enum extcon_cable_name type,
+ union extcon_cable_data data);
+extern void extcon_cable_get_data(struct extcon_dev *edev, int
cable_index,
+ enum extcon_cable_name *type,
+ union extcon_cable_data *data);
+/*
* get/set/update_state access the 32b encoded state value, which
represents
* states of all possible cables
[PATCH] EXTCON: Get and set cable properties
Existing EXTCON implementation doesn't give a mechanim to read the cable
properties and extra states a cable needs to support. There are scenarios
where a cable can have more than two states(CONNECT/DISCONNECT/SUSPEND/RESUME
etc)
and can have some properties associated with cables(mA)
This patch introduces interface to get and set cable properties
from EXTCON framework. The cable property can be set either by
the extcon cable provider or by other subsystems who know the cable
properties using eth API extcon_cable_set_data()
When the consumer gets a notification from the extcon, it can use the
extcon_cable_get_data() to get the cable properties irrespective of who
provides the cable data.
This gives a single interface for setting and getting the cable properties.
Signed-off-by: Jenny TC
---
drivers/extcon/extcon-class.c | 30 ++
include/linux/extcon.h| 39 +++
2 files changed, 69 insertions(+)
diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
index d398821..304f343 100644
--- a/drivers/extcon/extcon-class.c
+++ b/drivers/extcon/extcon-class.c
@@ -545,6 +545,36 @@ int extcon_unregister_notifier(struct extcon_dev *edev,
}
EXPORT_SYMBOL_GPL(extcon_unregister_notifier);
+/**
+ * extcon_cable_set_data() - Set the data structure for a cable
+ * @edev: the extcon device
+ * @cable_index: the cable index of the correspondant
+ * @type: type of the data structure
+ * @data:
+ */
+void extcon_cable_set_data(struct extcon_dev *edev, int cable_index,
+ enum extcon_cable_name type,
+ union extcon_cable_data data)
+{
+ edev->cables[cable_index].type = type;
+ edev->cables[cable_index].data = data;
+}
+
+/**
+ * extcon_cable_get_data() - Get the data structure for a cable
+ * @edev: the extcon device
+ * @cable_index: the cable index of the correspondant
+ * @type: type of the data structure
+ * @data: the corresponding data structure (e.g., regulator)
+ */
+void extcon_cable_get_data(struct extcon_dev *edev, int cable_index,
+ enum extcon_cable_name *type,
+ union extcon_cable_data *data)
+{
+ *type = edev->cables[cable_index].type;
+ *data = edev->cables[cable_index].data;
+}
+
static struct device_attribute extcon_attrs[] = {
__ATTR(state, S_IRUGO | S_IWUSR, state_show, state_store),
__ATTR_RO(name),
diff --git a/include/linux/extcon.h b/include/linux/extcon.h
index 2c26c14..4556cc5 100644
--- a/include/linux/extcon.h
+++ b/include/linux/extcon.h
@@ -135,6 +135,19 @@ struct extcon_dev {
struct device_attribute *d_attrs_muex;
};
+/* FIXME: Is this the right place for this structure definition?
+ * Do we need to move it to power_supply.h?
+ */
+struct extcon_chrgr_cable_props {
+ unsigned long state;
+ int mA;
+};
+
+union extcon_cable_data {
+ struct extcon_chrgr_cable_props chrgr_cbl_props;
+ /* Please add accordingly*/
+};
+
/**
* struct extcon_cable - An internal data for each cable of extcon device.
* @edev The extcon device
@@ -143,6 +156,8 @@ struct extcon_dev {
* @attr_name "name" sysfs entry
* @attr_state "state" sysfs entry
* @attrs Array pointing to attr_name and attr_state for attr_g
+ * @type: The type of @data.
+ * @data: The data structure representing the status and states of this
cable.
*/
struct extcon_cable {
struct extcon_dev *edev;
@@ -153,6 +168,11 @@ struct extcon_cable {
struct device_attribute attr_state;
struct attribute *attrs[3]; /* to be fed to attr_g.attrs */
+
+ union extcon_cable_data data;
+
+ /* extcon cable type */
+ enum extcon_cable_name type;
};
/**
@@ -183,6 +203,17 @@ extern void extcon_dev_unregister(struct extcon_dev *edev);
extern struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name);
/*
+ * Following APIs are for managing the status and states of each cable.
+ * For example, if a cable is represented as a regulator, then the cable
+ * may have struct regulator as its data.
+ */
+extern void extcon_cable_set_data(struct extcon_dev *edev, int cable_index,
+ enum extcon_cable_name type,
+ union extcon_cable_data data);
+extern void extcon_cable_get_data(struct extcon_dev *edev, int cable_index,
+ enum extcon_cable_name *type,
+ union extcon_cable_data *data);
+/*
* get/set/update_state access the 32b encoded state value, which represents
* states of all possible cables of the multistate port. For example, if one
* calls extcon_set_state(edev, 0x7), it may mean that all the three cables
@@ -244,6 +275,14 @@ static inline int extcon_dev_register(struct extcon_dev
*edev,
static inline void extcon_dev_unregister(struct extcon_dev *edev) { }
+static void
[PATCH] EXTCON: Get and set cable properties
Existing EXTCON implementation doesn't give a mechanim to read the cable
properties and extra states a cable needs to support. There are scenarios
where a cable can have more than two states(CONNECT/DISCONNECT/SUSPEND/RESUME
etc)
and can have some properties associated with cables(mA)
This patch introduces interface to get and set cable properties
from EXTCON framework. The cable property can be set either by
the extcon cable provider or by other subsystems who know the cable
properties using eth API extcon_cable_set_data()
When the consumer gets a notification from the extcon, it can use the
extcon_cable_get_data() to get the cable properties irrespective of who
provides the cable data.
This gives a single interface for setting and getting the cable properties.
Signed-off-by: Jenny TC jenny...@intel.com
---
drivers/extcon/extcon-class.c | 30 ++
include/linux/extcon.h| 39 +++
2 files changed, 69 insertions(+)
diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
index d398821..304f343 100644
--- a/drivers/extcon/extcon-class.c
+++ b/drivers/extcon/extcon-class.c
@@ -545,6 +545,36 @@ int extcon_unregister_notifier(struct extcon_dev *edev,
}
EXPORT_SYMBOL_GPL(extcon_unregister_notifier);
+/**
+ * extcon_cable_set_data() - Set the data structure for a cable
+ * @edev: the extcon device
+ * @cable_index: the cable index of the correspondant
+ * @type: type of the data structure
+ * @data:
+ */
+void extcon_cable_set_data(struct extcon_dev *edev, int cable_index,
+ enum extcon_cable_name type,
+ union extcon_cable_data data)
+{
+ edev-cables[cable_index].type = type;
+ edev-cables[cable_index].data = data;
+}
+
+/**
+ * extcon_cable_get_data() - Get the data structure for a cable
+ * @edev: the extcon device
+ * @cable_index: the cable index of the correspondant
+ * @type: type of the data structure
+ * @data: the corresponding data structure (e.g., regulator)
+ */
+void extcon_cable_get_data(struct extcon_dev *edev, int cable_index,
+ enum extcon_cable_name *type,
+ union extcon_cable_data *data)
+{
+ *type = edev-cables[cable_index].type;
+ *data = edev-cables[cable_index].data;
+}
+
static struct device_attribute extcon_attrs[] = {
__ATTR(state, S_IRUGO | S_IWUSR, state_show, state_store),
__ATTR_RO(name),
diff --git a/include/linux/extcon.h b/include/linux/extcon.h
index 2c26c14..4556cc5 100644
--- a/include/linux/extcon.h
+++ b/include/linux/extcon.h
@@ -135,6 +135,19 @@ struct extcon_dev {
struct device_attribute *d_attrs_muex;
};
+/* FIXME: Is this the right place for this structure definition?
+ * Do we need to move it to power_supply.h?
+ */
+struct extcon_chrgr_cable_props {
+ unsigned long state;
+ int mA;
+};
+
+union extcon_cable_data {
+ struct extcon_chrgr_cable_props chrgr_cbl_props;
+ /* Please add accordingly*/
+};
+
/**
* struct extcon_cable - An internal data for each cable of extcon device.
* @edev The extcon device
@@ -143,6 +156,8 @@ struct extcon_dev {
* @attr_name name sysfs entry
* @attr_state state sysfs entry
* @attrs Array pointing to attr_name and attr_state for attr_g
+ * @type: The type of @data.
+ * @data: The data structure representing the status and states of this
cable.
*/
struct extcon_cable {
struct extcon_dev *edev;
@@ -153,6 +168,11 @@ struct extcon_cable {
struct device_attribute attr_state;
struct attribute *attrs[3]; /* to be fed to attr_g.attrs */
+
+ union extcon_cable_data data;
+
+ /* extcon cable type */
+ enum extcon_cable_name type;
};
/**
@@ -183,6 +203,17 @@ extern void extcon_dev_unregister(struct extcon_dev *edev);
extern struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name);
/*
+ * Following APIs are for managing the status and states of each cable.
+ * For example, if a cable is represented as a regulator, then the cable
+ * may have struct regulator as its data.
+ */
+extern void extcon_cable_set_data(struct extcon_dev *edev, int cable_index,
+ enum extcon_cable_name type,
+ union extcon_cable_data data);
+extern void extcon_cable_get_data(struct extcon_dev *edev, int cable_index,
+ enum extcon_cable_name *type,
+ union extcon_cable_data *data);
+/*
* get/set/update_state access the 32b encoded state value, which represents
* states of all possible cables of the multistate port. For example, if one
* calls extcon_set_state(edev, 0x7), it may mean that all the three cables
@@ -244,6 +275,14 @@ static inline int extcon_dev_register(struct extcon_dev
*edev,
static inline void extcon_dev_unregister(struct extcon_dev *edev) { }
+static void
