Message-ID: <20ADAB092842284E95860F279283C56444A584@BGSMSX101.gar.corp.intel.com>
Date:	Tue, 20 Nov 2012 01:39:51 +0000
From:	"Tc, Jenny" <jenny.tc@...el.com>
To:	anish singh <anish198519851985@...il.com>
CC:	Chanwoo Choi <cw00.choi@...sung.com>,
	"myungjoo.ham@...sung.com" <myungjoo.ham@...sung.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"myunjoo.ham@...il.com" <myunjoo.ham@...il.com>,
	"lockwood@...roid.com" <lockwood@...roid.com>,
	"peterhuewe@....de" <peterhuewe@....de>,
	"broonie@...nsource.wolfsonmicro.com" 
	<broonie@...nsource.wolfsonmicro.com>,
	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
	"lars@...afoo.de" <lars@...afoo.de>,
	"jic23@...nel.org" <jic23@...nel.org>,
	"Anton Vorontsov (cbouatmailru@...il.com)" <cbouatmailru@...il.com>,
	"Anton Vorontsov (anton.vorontsov@...aro.org)" 
	<anton.vorontsov@...aro.org>,
	"Pallala, Ramakrishna" <ramakrishna.pallala@...el.com>
Subject: RE: [PATCH] extcon : callback function to read cable property

> >> > > 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:
> >> > >   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.
> >> >
> >> > 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??
> >> > }
> >> > 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
> >> >
> >> > >
> >> > > 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.
> >> 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.
> >> 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.
> >>
> >> 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).
> > The extcon consumer in this case is not a device driver. It's part of power
> supply class driver itself.
> > I am open to any solution to get the cable properties dynamically. Do
> > you find a better but generic mechanism for this?
> >
> > 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.
> > We shouldn't put any driver level dependency between extcon consumer
> and provider.
> > 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.

Maintainer??

> 
> > 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
> scalable and reusable across multiple platforms.
> >
> >> >
> >> > Hope above structure would be enough to represent the common cable
> >> > 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.
> >> > 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
> >> >
> >> > >
> >> > > 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
> >> 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.
> >> > > 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.
> >> >
> >> > >
> >> > > 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)-
> >> > > >DISCONNECT(0mA)
> >> > > 3) CONNECT (100mA)->UPDATE(500mA)->HOST
> >> SUSPEND(2.5mA/500mA)-
> >> > > >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)-
> >> > > >DISCONNECT(0mA)
> >> > > 6) CONNECT (100mA)->UPDATE(900mA)->HOST
> >> SUSPEND(2.5mA/900mA)-
> >> > > >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?
> >> >
> >> > Good question. The OTG drivers are capable of identifying the
> >> > SUSPEND
> >> event.
> >> > System cannot setup SDP (USB host) charging with maximum charging
> >> > current - 500mA (USB2.0/ 900mA(USB 3)) without enumerating the USB.
> >> > 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.
> >> > The intelligence for USB enumeration should be inside USB/OTG driver.
> >> >
> >>
> >

Powered by blists - more mailing lists