Re: [Gta04-owner] [PATCH 5/6] phy: twl4030-usb: add support for reading resistor on ID pin.

2015-06-23 Thread Dr. H. Nikolaus Schaller
Hi Neil,

Am 01.06.2015 um 23:37 schrieb NeilBrown ne...@suse.de:

 On Mon, 1 Jun 2015 19:06:52 +0530 Kishon Vijay Abraham I kis...@ti.com
 wrote:
 
 Hi,
 
 On Thursday 16 April 2015 01:33 PM, NeilBrown wrote:
 From: NeilBrown ne...@suse.de
 
 The twl4030 phy can measure, with low precision, the
 resistance-to-ground of the ID pin.
 
 Add a function to read the value, and export the result
 via sysfs.
 
 Little sceptical about adding new sysfs entries. Do you have a good reason 
 to 
 add this?
 
 The hardware can report the value, so why not present it to user-space?

I just had another idea how to present the value to user space.

The TWL6030 has connected the USB ID pin to one of the GPADC channels:


http://lxr.free-electrons.com/source/drivers/iio/adc/twl6030-gpadc.c#L235

And therefore automatically presents the ID pin voltage through iio.

Would it be possible and useful to provide an iio interface for the 
resistance-to-ground
of the tw4030 ID pin as well?

This would resemble a 6 or 7 level ADC with non-linear scale, but better than 
nothing.

And to avoid the “floating” issue, it could also present some voltage value 
(assuming
a defined current).

So that “floating” is reported as some maximum voltage (e.g. 3.3V) and “ground” 
as 0V.

What do you think?

BR,
Nikolaus

 
 I originally used this with a udev rule which would configure the maximum
 current based on the resistance measure - to work with the particular charger
 hardware I have.
 
 More recent patches try to do all of the max-current configuration in the
 kernel, so I could live without exporting the value via sysfs if that is a
 show-stopper.
 
 I can't see where the scepticism comes from though.  It is a well defined
 and cleary documented feature of the hardware.  Why not expose it?
 
 Thanks,
 NeilBrown
 
 
 
 Thanks
 Kishon
 
 If the read fails, which it does sometimes, try again in 50msec.
 
 Acked-by: Pavel Machek pa...@ucw.cz
 Signed-off-by: NeilBrown ne...@suse.de
 ---
  .../ABI/testing/sysfs-platform-twl4030-usb |   22 +++
  drivers/phy/phy-twl4030-usb.c  |   63 
 
  2 files changed, 85 insertions(+)
 
 diff --git a/Documentation/ABI/testing/sysfs-platform-twl4030-usb 
 b/Documentation/ABI/testing/sysfs-platform-twl4030-usb
 index 512c51be64ae..425d23676f8a 100644
 --- a/Documentation/ABI/testing/sysfs-platform-twl4030-usb
 +++ b/Documentation/ABI/testing/sysfs-platform-twl4030-usb
 @@ -6,3 +6,25 @@ Description:
 Possible values: on, off.
 
 Changes are notified via select/poll.
 +
 +What: /sys/bus/platform/devices/*twl4030-usb/id
 +Description:
 +   Read-only report on measurement of USB-OTG ID pin.
 +
 +   The ID pin may be floating, grounded, or pulled to
 +   ground by a resistor.
 +
 +   A very course grained reading of the resistance is
 +   available.  The numbers given in kilo-ohms are roughly
 +   the center-point of the detected range.
 +
 +   Possible values are:
 +   ground
 +   102k
 +   200k
 +   440k
 +   floating
 +   unknown
 +
 +   unknown indicates a problem with trying to detect
 +   the resistance.
 diff --git a/drivers/phy/phy-twl4030-usb.c b/drivers/phy/phy-twl4030-usb.c
 index 3a707dd14238..1d6f3e70193e 100644
 --- a/drivers/phy/phy-twl4030-usb.c
 +++ b/drivers/phy/phy-twl4030-usb.c
 @@ -379,6 +379,56 @@ static void twl4030_i2c_access(struct twl4030_usb 
 *twl, int on)
 }
  }
 
 +enum twl4030_id_status {
 +   TWL4030_GROUND,
 +   TWL4030_102K,
 +   TWL4030_200K,
 +   TWL4030_440K,
 +   TWL4030_FLOATING,
 +   TWL4030_ID_UNKNOWN,
 +};
 +static char *twl4030_id_names[] = {
 +   ground,
 +   102k,
 +   200k,
 +   440k,
 +   floating,
 +   unknown
 +};
 +
 +enum twl4030_id_status twl4030_get_id(struct twl4030_usb *twl)
 +{
 +   int ret;
 +
 +   pm_runtime_get_sync(twl-dev);
 +   if (twl-usb_mode == T2_USB_MODE_ULPI)
 +   twl4030_i2c_access(twl, 1);
 +   ret = twl4030_usb_read(twl, ULPI_OTG_CTRL);
 +   if (ret  0 || !(ret  ULPI_OTG_ID_PULLUP)) {
 +   /* Need pull-up to read ID */
 +   twl4030_usb_set_bits(twl, ULPI_OTG_CTRL,
 +ULPI_OTG_ID_PULLUP);
 +   mdelay(50);
 +   }
 +   ret = twl4030_usb_read(twl, ID_STATUS);
 +   if (ret  0 || (ret  0x1f) == 0) {
 +   mdelay(50);
 +   ret = twl4030_usb_read(twl, ID_STATUS);
 +   }
 +
 +   if (twl-usb_mode == T2_USB_MODE_ULPI)
 +   twl4030_i2c_access(twl, 0);
 +   pm_runtime_put_autosuspend(twl-dev);
 +
 +   if (ret  0)
 +   return TWL4030_ID_UNKNOWN;
 +   ret = ffs(ret) - 1;
 +   if (ret  TWL4030_GROUND || ret  TWL4030_FLOATING)
 +   return TWL4030_ID_UNKNOWN;
 +
 +   return ret;
 +}
 +
  static void __twl4030_phy_power(struct twl4030_usb *twl, int on)
  {
 u8 pwr = twl4030_usb_read(twl, PHY_PWR_CTRL);
 @@ -532,6 +582,16 @@ static ssize_t twl4030_usb_vbus_show(struct device 
 *dev,
  }
  static DEVICE_ATTR(vbus, 0444, twl4030_usb_vbus_show, 

Re: [PATCH 5/6] phy: twl4030-usb: add support for reading resistor on ID pin.

2015-06-07 Thread Felipe Balbi
On Sat, Jun 06, 2015 at 03:10:09PM +0200, Pavel Machek wrote:
 On Tue 2015-06-02 07:37:31, NeilBrown wrote:
  On Mon, 1 Jun 2015 19:06:52 +0530 Kishon Vijay Abraham I kis...@ti.com
  wrote:
  
   Hi,
   
   On Thursday 16 April 2015 01:33 PM, NeilBrown wrote:
From: NeilBrown ne...@suse.de
   
The twl4030 phy can measure, with low precision, the
resistance-to-ground of the ID pin.
   
Add a function to read the value, and export the result
via sysfs.
   
   Little sceptical about adding new sysfs entries. Do you have a good 
   reason to 
   add this?
  
  The hardware can report the value, so why not present it to user-space?
  
  I originally used this with a udev rule which would configure the maximum
  current based on the resistance measure - to work with the particular 
  charger
  hardware I have.
  
  More recent patches try to do all of the max-current configuration in the
  kernel, so I could live without exporting the value via sysfs if that is a
  show-stopper.
  
  I can't see where the scepticism comes from though.  It is a well defined
  and cleary documented feature of the hardware.  Why not expose it?
 
 sysfs interface is supposed to work for different chargers, without userspace
 noticing.
 
 Are these values the ones that are likely to be useful there?

These values come from Battery Charger specification 1.1+, IIRC, so no
other values should show up unless documented in future BC revisions.

+   Possible values are:
+   ground
+   102k
+   200k
+   440k
+   floating
+   unknown
 
 ...or would it make more sense to export for example numerical ohms, as that's
 what are other chargers likely to provide?

How do you expose floating in Ohms ? UINT_MAX might be one way, but
that would have to be documented.

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH 5/6] phy: twl4030-usb: add support for reading resistor on ID pin.

2015-06-06 Thread Pavel Machek
On Tue 2015-06-02 07:37:31, NeilBrown wrote:
 On Mon, 1 Jun 2015 19:06:52 +0530 Kishon Vijay Abraham I kis...@ti.com
 wrote:
 
  Hi,
  
  On Thursday 16 April 2015 01:33 PM, NeilBrown wrote:
   From: NeilBrown ne...@suse.de
  
   The twl4030 phy can measure, with low precision, the
   resistance-to-ground of the ID pin.
  
   Add a function to read the value, and export the result
   via sysfs.
  
  Little sceptical about adding new sysfs entries. Do you have a good reason 
  to 
  add this?
 
 The hardware can report the value, so why not present it to user-space?
 
 I originally used this with a udev rule which would configure the maximum
 current based on the resistance measure - to work with the particular charger
 hardware I have.
 
 More recent patches try to do all of the max-current configuration in the
 kernel, so I could live without exporting the value via sysfs if that is a
 show-stopper.
 
 I can't see where the scepticism comes from though.  It is a well defined
 and cleary documented feature of the hardware.  Why not expose it?

sysfs interface is supposed to work for different chargers, without userspace
noticing.

Are these values the ones that are likely to be useful there?

   + Possible values are:
   + ground
   + 102k
   + 200k
   + 440k
   + floating
   + unknown

...or would it make more sense to export for example numerical ohms, as that's
what are other chargers likely to provide?

Thanks,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Gta04-owner] [PATCH 5/6] phy: twl4030-usb: add support for reading resistor on ID pin.

2015-06-02 Thread Pavel Machek
On Tue 2015-06-02 16:06:47, Dr. H. Nikolaus Schaller wrote:
 Hi,
 
 Am 02.06.2015 um 15:49 schrieb Kishon Vijay Abraham I kis...@ti.com:
 
  Hi,
  
  On Tuesday 02 June 2015 03:07 AM, NeilBrown wrote:
  On Mon, 1 Jun 2015 19:06:52 +0530 Kishon Vijay Abraham I kis...@ti.com
  wrote:
  
  Hi,
  
  On Thursday 16 April 2015 01:33 PM, NeilBrown wrote:
  From: NeilBrown ne...@suse.de
  
  The twl4030 phy can measure, with low precision, the
  resistance-to-ground of the ID pin.
  
  Add a function to read the value, and export the result
  via sysfs.
  
  Little sceptical about adding new sysfs entries. Do you have a good 
  reason to
  add this?
  
  The hardware can report the value, so why not present it to user-space?
  
  I originally used this with a udev rule which would configure the maximum
  current based on the resistance measure - to work with the particular 
  charger
  hardware I have.
  
  More recent patches try to do all of the max-current configuration in the
  kernel, so I could live without exporting the value via sysfs if that is a
  show-stopper.
  
  I can't see where the scepticism comes from though.  It is a well defined
  and cleary documented feature of the hardware.  Why not expose it?

Is it well defined enough that it will work on other chargers, too?

  ABI can never be removed or modified later. So should be really careful 
  before adding it.
 
 Is /sys considered ABI?

Yes.

 User space developers are always reminded not to rely on /sys nodes.

No.
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Gta04-owner] [PATCH 5/6] phy: twl4030-usb: add support for reading resistor on ID pin.

2015-06-02 Thread Dr. H. Nikolaus Schaller
Hi,

Am 02.06.2015 um 22:11 schrieb Pavel Machek pa...@ucw.cz:

 On Tue 2015-06-02 16:06:47, Dr. H. Nikolaus Schaller wrote:
 Hi,
 
 Am 02.06.2015 um 15:49 schrieb Kishon Vijay Abraham I kis...@ti.com:
 
 Hi,
 
 On Tuesday 02 June 2015 03:07 AM, NeilBrown wrote:
 On Mon, 1 Jun 2015 19:06:52 +0530 Kishon Vijay Abraham I kis...@ti.com
 wrote:
 
 Hi,
 
 On Thursday 16 April 2015 01:33 PM, NeilBrown wrote:
 From: NeilBrown ne...@suse.de
 
 The twl4030 phy can measure, with low precision, the
 resistance-to-ground of the ID pin.
 
 Add a function to read the value, and export the result
 via sysfs.
 
 Little sceptical about adding new sysfs entries. Do you have a good 
 reason to
 add this?
 
 The hardware can report the value, so why not present it to user-space?
 
 I originally used this with a udev rule which would configure the maximum
 current based on the resistance measure - to work with the particular 
 charger
 hardware I have.
 
 More recent patches try to do all of the max-current configuration in the
 kernel, so I could live without exporting the value via sysfs if that is a
 show-stopper.
 
 I can't see where the scepticism comes from though.  It is a well defined
 and cleary documented feature of the hardware.  Why not expose it?
 
 Is it well defined enough that it will work on other chargers, too?

It reports the resistance of the charger’s ID pin. So that works for all 
chargers connected
to a twl4030. As long as the ID pin goes to a 5 pin USB socket.

Other charger drivers do not need to expose a similar attribute since each 
twl4030 has its
unique path within the /sys tree.

 
 ABI can never be removed or modified later. So should be really careful 
 before adding it.
 
 Is /sys considered ABI?
 
 Yes.

You are right: https://lwn.net/Articles/172986/

But I am as well with my doubts: https://lwn.net/Articles/173093/

 
 User space developers are always reminded not to rely on /sys nodes.
 
 No.

Then please explain why I have the impression that it is quite unstable.

BR,
Nikolaus

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Gta04-owner] [PATCH 5/6] phy: twl4030-usb: add support for reading resistor on ID pin.

2015-06-02 Thread Dr. H. Nikolaus Schaller
Hi,

Am 02.06.2015 um 15:49 schrieb Kishon Vijay Abraham I kis...@ti.com:

 Hi,
 
 On Tuesday 02 June 2015 03:07 AM, NeilBrown wrote:
 On Mon, 1 Jun 2015 19:06:52 +0530 Kishon Vijay Abraham I kis...@ti.com
 wrote:
 
 Hi,
 
 On Thursday 16 April 2015 01:33 PM, NeilBrown wrote:
 From: NeilBrown ne...@suse.de
 
 The twl4030 phy can measure, with low precision, the
 resistance-to-ground of the ID pin.
 
 Add a function to read the value, and export the result
 via sysfs.
 
 Little sceptical about adding new sysfs entries. Do you have a good reason 
 to
 add this?
 
 The hardware can report the value, so why not present it to user-space?
 
 I originally used this with a udev rule which would configure the maximum
 current based on the resistance measure - to work with the particular charger
 hardware I have.
 
 More recent patches try to do all of the max-current configuration in the
 kernel, so I could live without exporting the value via sysfs if that is a
 show-stopper.
 
 I can't see where the scepticism comes from though.  It is a well defined
 and cleary documented feature of the hardware.  Why not expose it?
 
 ABI can never be removed or modified later. So should be really careful 
 before adding it.

Is /sys considered ABI? It is permanently changing. At least in what I see.

User space developers are always reminded not to rely on /sys nodes.
Or if they do they have to follow kernel changes at their own risk.

And if something is useful (and has a use case as Neil mentioned), why shouldn’t
it be provided.

There are use cases where user space needs to know the value. Udev rule being
an example. E.g. to make LEDs show the state.

Or see it as a debugging tool. Just cat /sys/…path…/id to check if your 3 types
of charger are recognised properly.

Or write a tool that displays the charger type.

So isn’t that a little too narrow view applied here?

Just my opinion.

BR,
Nikolaus

 
 Thanks
 Kishon
 
 
 Thanks,
 NeilBrown
 
 
 
 Thanks
 Kishon
 
 If the read fails, which it does sometimes, try again in 50msec.
 
 Acked-by: Pavel Machek pa...@ucw.cz
 Signed-off-by: NeilBrown ne...@suse.de
 ---
   .../ABI/testing/sysfs-platform-twl4030-usb |   22 +++
   drivers/phy/phy-twl4030-usb.c  |   63 
 
   2 files changed, 85 insertions(+)
 
 diff --git a/Documentation/ABI/testing/sysfs-platform-twl4030-usb 
 b/Documentation/ABI/testing/sysfs-platform-twl4030-usb
 index 512c51be64ae..425d23676f8a 100644
 --- a/Documentation/ABI/testing/sysfs-platform-twl4030-usb
 +++ b/Documentation/ABI/testing/sysfs-platform-twl4030-usb
 @@ -6,3 +6,25 @@ Description:
Possible values: on, off.
 
Changes are notified via select/poll.
 +
 +What: /sys/bus/platform/devices/*twl4030-usb/id
 +Description:
 +  Read-only report on measurement of USB-OTG ID pin.
 +
 +  The ID pin may be floating, grounded, or pulled to
 +  ground by a resistor.
 +
 +  A very course grained reading of the resistance is
 +  available.  The numbers given in kilo-ohms are roughly
 +  the center-point of the detected range.
 +
 +  Possible values are:
 +  ground
 +  102k
 +  200k
 +  440k
 +  floating
 +  unknown
 +
 +  unknown indicates a problem with trying to detect
 +  the resistance.
 diff --git a/drivers/phy/phy-twl4030-usb.c b/drivers/phy/phy-twl4030-usb.c
 index 3a707dd14238..1d6f3e70193e 100644
 --- a/drivers/phy/phy-twl4030-usb.c
 +++ b/drivers/phy/phy-twl4030-usb.c
 @@ -379,6 +379,56 @@ static void twl4030_i2c_access(struct twl4030_usb 
 *twl, int on)
}
   }
 
 +enum twl4030_id_status {
 +  TWL4030_GROUND,
 +  TWL4030_102K,
 +  TWL4030_200K,
 +  TWL4030_440K,
 +  TWL4030_FLOATING,
 +  TWL4030_ID_UNKNOWN,
 +};
 +static char *twl4030_id_names[] = {
 +  ground,
 +  102k,
 +  200k,
 +  440k,
 +  floating,
 +  unknown
 +};
 +
 +enum twl4030_id_status twl4030_get_id(struct twl4030_usb *twl)
 +{
 +  int ret;
 +
 +  pm_runtime_get_sync(twl-dev);
 +  if (twl-usb_mode == T2_USB_MODE_ULPI)
 +  twl4030_i2c_access(twl, 1);
 +  ret = twl4030_usb_read(twl, ULPI_OTG_CTRL);
 +  if (ret  0 || !(ret  ULPI_OTG_ID_PULLUP)) {
 +  /* Need pull-up to read ID */
 +  twl4030_usb_set_bits(twl, ULPI_OTG_CTRL,
 +   ULPI_OTG_ID_PULLUP);
 +  mdelay(50);
 +  }
 +  ret = twl4030_usb_read(twl, ID_STATUS);
 +  if (ret  0 || (ret  0x1f) == 0) {
 +  mdelay(50);
 +  ret = twl4030_usb_read(twl, ID_STATUS);
 +  }
 +
 +  if (twl-usb_mode == T2_USB_MODE_ULPI)
 +  twl4030_i2c_access(twl, 0);
 +  pm_runtime_put_autosuspend(twl-dev);
 +
 +  if (ret  0)
 +  return TWL4030_ID_UNKNOWN;
 +  ret = ffs(ret) - 1;
 +  if (ret  TWL4030_GROUND || ret  TWL4030_FLOATING)
 +  return TWL4030_ID_UNKNOWN;
 +
 +  return ret;
 +}
 +
   static void __twl4030_phy_power(struct twl4030_usb *twl, int on)
   {
u8 pwr = twl4030_usb_read(twl, PHY_PWR_CTRL);
 @@ -532,6 +582,16 @@ static ssize_t 

Re: [PATCH 5/6] phy: twl4030-usb: add support for reading resistor on ID pin.

2015-06-02 Thread Kishon Vijay Abraham I

Hi,

On Tuesday 02 June 2015 03:07 AM, NeilBrown wrote:

On Mon, 1 Jun 2015 19:06:52 +0530 Kishon Vijay Abraham I kis...@ti.com
wrote:


Hi,

On Thursday 16 April 2015 01:33 PM, NeilBrown wrote:

From: NeilBrown ne...@suse.de

The twl4030 phy can measure, with low precision, the
resistance-to-ground of the ID pin.

Add a function to read the value, and export the result
via sysfs.


Little sceptical about adding new sysfs entries. Do you have a good reason to
add this?


The hardware can report the value, so why not present it to user-space?

I originally used this with a udev rule which would configure the maximum
current based on the resistance measure - to work with the particular charger
hardware I have.

More recent patches try to do all of the max-current configuration in the
kernel, so I could live without exporting the value via sysfs if that is a
show-stopper.

I can't see where the scepticism comes from though.  It is a well defined
and cleary documented feature of the hardware.  Why not expose it?


ABI can never be removed or modified later. So should be really careful before 
adding it.


Thanks
Kishon



Thanks,
NeilBrown




Thanks
Kishon


If the read fails, which it does sometimes, try again in 50msec.

Acked-by: Pavel Machek pa...@ucw.cz
Signed-off-by: NeilBrown ne...@suse.de
---
   .../ABI/testing/sysfs-platform-twl4030-usb |   22 +++
   drivers/phy/phy-twl4030-usb.c  |   63 

   2 files changed, 85 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-platform-twl4030-usb 
b/Documentation/ABI/testing/sysfs-platform-twl4030-usb
index 512c51be64ae..425d23676f8a 100644
--- a/Documentation/ABI/testing/sysfs-platform-twl4030-usb
+++ b/Documentation/ABI/testing/sysfs-platform-twl4030-usb
@@ -6,3 +6,25 @@ Description:
Possible values: on, off.

Changes are notified via select/poll.
+
+What: /sys/bus/platform/devices/*twl4030-usb/id
+Description:
+   Read-only report on measurement of USB-OTG ID pin.
+
+   The ID pin may be floating, grounded, or pulled to
+   ground by a resistor.
+
+   A very course grained reading of the resistance is
+   available.  The numbers given in kilo-ohms are roughly
+   the center-point of the detected range.
+
+   Possible values are:
+   ground
+   102k
+   200k
+   440k
+   floating
+   unknown
+
+   unknown indicates a problem with trying to detect
+   the resistance.
diff --git a/drivers/phy/phy-twl4030-usb.c b/drivers/phy/phy-twl4030-usb.c
index 3a707dd14238..1d6f3e70193e 100644
--- a/drivers/phy/phy-twl4030-usb.c
+++ b/drivers/phy/phy-twl4030-usb.c
@@ -379,6 +379,56 @@ static void twl4030_i2c_access(struct twl4030_usb *twl, 
int on)
}
   }

+enum twl4030_id_status {
+   TWL4030_GROUND,
+   TWL4030_102K,
+   TWL4030_200K,
+   TWL4030_440K,
+   TWL4030_FLOATING,
+   TWL4030_ID_UNKNOWN,
+};
+static char *twl4030_id_names[] = {
+   ground,
+   102k,
+   200k,
+   440k,
+   floating,
+   unknown
+};
+
+enum twl4030_id_status twl4030_get_id(struct twl4030_usb *twl)
+{
+   int ret;
+
+   pm_runtime_get_sync(twl-dev);
+   if (twl-usb_mode == T2_USB_MODE_ULPI)
+   twl4030_i2c_access(twl, 1);
+   ret = twl4030_usb_read(twl, ULPI_OTG_CTRL);
+   if (ret  0 || !(ret  ULPI_OTG_ID_PULLUP)) {
+   /* Need pull-up to read ID */
+   twl4030_usb_set_bits(twl, ULPI_OTG_CTRL,
+ULPI_OTG_ID_PULLUP);
+   mdelay(50);
+   }
+   ret = twl4030_usb_read(twl, ID_STATUS);
+   if (ret  0 || (ret  0x1f) == 0) {
+   mdelay(50);
+   ret = twl4030_usb_read(twl, ID_STATUS);
+   }
+
+   if (twl-usb_mode == T2_USB_MODE_ULPI)
+   twl4030_i2c_access(twl, 0);
+   pm_runtime_put_autosuspend(twl-dev);
+
+   if (ret  0)
+   return TWL4030_ID_UNKNOWN;
+   ret = ffs(ret) - 1;
+   if (ret  TWL4030_GROUND || ret  TWL4030_FLOATING)
+   return TWL4030_ID_UNKNOWN;
+
+   return ret;
+}
+
   static void __twl4030_phy_power(struct twl4030_usb *twl, int on)
   {
u8 pwr = twl4030_usb_read(twl, PHY_PWR_CTRL);
@@ -532,6 +582,16 @@ static ssize_t twl4030_usb_vbus_show(struct device *dev,
   }
   static DEVICE_ATTR(vbus, 0444, twl4030_usb_vbus_show, NULL);

+static ssize_t twl4030_usb_id_show(struct device *dev,
+  struct device_attribute *attr,
+  char *buf)
+{
+   struct twl4030_usb *twl = dev_get_drvdata(dev);
+   return scnprintf(buf, PAGE_SIZE, %s\n,
+twl4030_id_names[twl4030_get_id(twl)]);
+}
+static DEVICE_ATTR(id, 0444, twl4030_usb_id_show, NULL);
+
   static irqreturn_t twl4030_usb_irq(int irq, void *_twl)
   {
struct twl4030_usb *twl = _twl;
@@ -709,6 +769,8 @@ 

Re: [PATCH 5/6] phy: twl4030-usb: add support for reading resistor on ID pin.

2015-06-01 Thread Kishon Vijay Abraham I

Hi,

On Thursday 16 April 2015 01:33 PM, NeilBrown wrote:

From: NeilBrown ne...@suse.de

The twl4030 phy can measure, with low precision, the
resistance-to-ground of the ID pin.

Add a function to read the value, and export the result
via sysfs.


Little sceptical about adding new sysfs entries. Do you have a good reason to 
add this?


Thanks
Kishon


If the read fails, which it does sometimes, try again in 50msec.

Acked-by: Pavel Machek pa...@ucw.cz
Signed-off-by: NeilBrown ne...@suse.de
---
  .../ABI/testing/sysfs-platform-twl4030-usb |   22 +++
  drivers/phy/phy-twl4030-usb.c  |   63 
  2 files changed, 85 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-platform-twl4030-usb 
b/Documentation/ABI/testing/sysfs-platform-twl4030-usb
index 512c51be64ae..425d23676f8a 100644
--- a/Documentation/ABI/testing/sysfs-platform-twl4030-usb
+++ b/Documentation/ABI/testing/sysfs-platform-twl4030-usb
@@ -6,3 +6,25 @@ Description:
Possible values: on, off.

Changes are notified via select/poll.
+
+What: /sys/bus/platform/devices/*twl4030-usb/id
+Description:
+   Read-only report on measurement of USB-OTG ID pin.
+
+   The ID pin may be floating, grounded, or pulled to
+   ground by a resistor.
+
+   A very course grained reading of the resistance is
+   available.  The numbers given in kilo-ohms are roughly
+   the center-point of the detected range.
+
+   Possible values are:
+   ground
+   102k
+   200k
+   440k
+   floating
+   unknown
+
+   unknown indicates a problem with trying to detect
+   the resistance.
diff --git a/drivers/phy/phy-twl4030-usb.c b/drivers/phy/phy-twl4030-usb.c
index 3a707dd14238..1d6f3e70193e 100644
--- a/drivers/phy/phy-twl4030-usb.c
+++ b/drivers/phy/phy-twl4030-usb.c
@@ -379,6 +379,56 @@ static void twl4030_i2c_access(struct twl4030_usb *twl, 
int on)
}
  }

+enum twl4030_id_status {
+   TWL4030_GROUND,
+   TWL4030_102K,
+   TWL4030_200K,
+   TWL4030_440K,
+   TWL4030_FLOATING,
+   TWL4030_ID_UNKNOWN,
+};
+static char *twl4030_id_names[] = {
+   ground,
+   102k,
+   200k,
+   440k,
+   floating,
+   unknown
+};
+
+enum twl4030_id_status twl4030_get_id(struct twl4030_usb *twl)
+{
+   int ret;
+
+   pm_runtime_get_sync(twl-dev);
+   if (twl-usb_mode == T2_USB_MODE_ULPI)
+   twl4030_i2c_access(twl, 1);
+   ret = twl4030_usb_read(twl, ULPI_OTG_CTRL);
+   if (ret  0 || !(ret  ULPI_OTG_ID_PULLUP)) {
+   /* Need pull-up to read ID */
+   twl4030_usb_set_bits(twl, ULPI_OTG_CTRL,
+ULPI_OTG_ID_PULLUP);
+   mdelay(50);
+   }
+   ret = twl4030_usb_read(twl, ID_STATUS);
+   if (ret  0 || (ret  0x1f) == 0) {
+   mdelay(50);
+   ret = twl4030_usb_read(twl, ID_STATUS);
+   }
+
+   if (twl-usb_mode == T2_USB_MODE_ULPI)
+   twl4030_i2c_access(twl, 0);
+   pm_runtime_put_autosuspend(twl-dev);
+
+   if (ret  0)
+   return TWL4030_ID_UNKNOWN;
+   ret = ffs(ret) - 1;
+   if (ret  TWL4030_GROUND || ret  TWL4030_FLOATING)
+   return TWL4030_ID_UNKNOWN;
+
+   return ret;
+}
+
  static void __twl4030_phy_power(struct twl4030_usb *twl, int on)
  {
u8 pwr = twl4030_usb_read(twl, PHY_PWR_CTRL);
@@ -532,6 +582,16 @@ static ssize_t twl4030_usb_vbus_show(struct device *dev,
  }
  static DEVICE_ATTR(vbus, 0444, twl4030_usb_vbus_show, NULL);

+static ssize_t twl4030_usb_id_show(struct device *dev,
+  struct device_attribute *attr,
+  char *buf)
+{
+   struct twl4030_usb *twl = dev_get_drvdata(dev);
+   return scnprintf(buf, PAGE_SIZE, %s\n,
+twl4030_id_names[twl4030_get_id(twl)]);
+}
+static DEVICE_ATTR(id, 0444, twl4030_usb_id_show, NULL);
+
  static irqreturn_t twl4030_usb_irq(int irq, void *_twl)
  {
struct twl4030_usb *twl = _twl;
@@ -709,6 +769,8 @@ static int twl4030_usb_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, twl);
if (device_create_file(pdev-dev, dev_attr_vbus))
dev_warn(pdev-dev, could not create sysfs file\n);
+   if (device_create_file(pdev-dev, dev_attr_id))
+   dev_warn(pdev-dev, could not create sysfs file\n);

ATOMIC_INIT_NOTIFIER_HEAD(twl-phy.notifier);

@@ -753,6 +815,7 @@ static int twl4030_usb_remove(struct platform_device *pdev)
pm_runtime_get_sync(twl-dev);
cancel_delayed_work(twl-id_workaround_work);
device_remove_file(twl-dev, dev_attr_vbus);
+   device_remove_file(twl-dev, dev_attr_id);

/* set transceiver mode to power on defaults */
twl4030_usb_set_mode(twl, -1);



--
To unsubscribe from this list: send the line 

Re: [PATCH 5/6] phy: twl4030-usb: add support for reading resistor on ID pin.

2015-06-01 Thread NeilBrown
On Mon, 1 Jun 2015 19:06:52 +0530 Kishon Vijay Abraham I kis...@ti.com
wrote:

 Hi,
 
 On Thursday 16 April 2015 01:33 PM, NeilBrown wrote:
  From: NeilBrown ne...@suse.de
 
  The twl4030 phy can measure, with low precision, the
  resistance-to-ground of the ID pin.
 
  Add a function to read the value, and export the result
  via sysfs.
 
 Little sceptical about adding new sysfs entries. Do you have a good reason to 
 add this?

The hardware can report the value, so why not present it to user-space?

I originally used this with a udev rule which would configure the maximum
current based on the resistance measure - to work with the particular charger
hardware I have.

More recent patches try to do all of the max-current configuration in the
kernel, so I could live without exporting the value via sysfs if that is a
show-stopper.

I can't see where the scepticism comes from though.  It is a well defined
and cleary documented feature of the hardware.  Why not expose it?

Thanks,
NeilBrown


 
 Thanks
 Kishon
 
  If the read fails, which it does sometimes, try again in 50msec.
 
  Acked-by: Pavel Machek pa...@ucw.cz
  Signed-off-by: NeilBrown ne...@suse.de
  ---
.../ABI/testing/sysfs-platform-twl4030-usb |   22 +++
drivers/phy/phy-twl4030-usb.c  |   63 
  
2 files changed, 85 insertions(+)
 
  diff --git a/Documentation/ABI/testing/sysfs-platform-twl4030-usb 
  b/Documentation/ABI/testing/sysfs-platform-twl4030-usb
  index 512c51be64ae..425d23676f8a 100644
  --- a/Documentation/ABI/testing/sysfs-platform-twl4030-usb
  +++ b/Documentation/ABI/testing/sysfs-platform-twl4030-usb
  @@ -6,3 +6,25 @@ Description:
  Possible values: on, off.
 
  Changes are notified via select/poll.
  +
  +What: /sys/bus/platform/devices/*twl4030-usb/id
  +Description:
  +   Read-only report on measurement of USB-OTG ID pin.
  +
  +   The ID pin may be floating, grounded, or pulled to
  +   ground by a resistor.
  +
  +   A very course grained reading of the resistance is
  +   available.  The numbers given in kilo-ohms are roughly
  +   the center-point of the detected range.
  +
  +   Possible values are:
  +   ground
  +   102k
  +   200k
  +   440k
  +   floating
  +   unknown
  +
  +   unknown indicates a problem with trying to detect
  +   the resistance.
  diff --git a/drivers/phy/phy-twl4030-usb.c b/drivers/phy/phy-twl4030-usb.c
  index 3a707dd14238..1d6f3e70193e 100644
  --- a/drivers/phy/phy-twl4030-usb.c
  +++ b/drivers/phy/phy-twl4030-usb.c
  @@ -379,6 +379,56 @@ static void twl4030_i2c_access(struct twl4030_usb 
  *twl, int on)
  }
}
 
  +enum twl4030_id_status {
  +   TWL4030_GROUND,
  +   TWL4030_102K,
  +   TWL4030_200K,
  +   TWL4030_440K,
  +   TWL4030_FLOATING,
  +   TWL4030_ID_UNKNOWN,
  +};
  +static char *twl4030_id_names[] = {
  +   ground,
  +   102k,
  +   200k,
  +   440k,
  +   floating,
  +   unknown
  +};
  +
  +enum twl4030_id_status twl4030_get_id(struct twl4030_usb *twl)
  +{
  +   int ret;
  +
  +   pm_runtime_get_sync(twl-dev);
  +   if (twl-usb_mode == T2_USB_MODE_ULPI)
  +   twl4030_i2c_access(twl, 1);
  +   ret = twl4030_usb_read(twl, ULPI_OTG_CTRL);
  +   if (ret  0 || !(ret  ULPI_OTG_ID_PULLUP)) {
  +   /* Need pull-up to read ID */
  +   twl4030_usb_set_bits(twl, ULPI_OTG_CTRL,
  +ULPI_OTG_ID_PULLUP);
  +   mdelay(50);
  +   }
  +   ret = twl4030_usb_read(twl, ID_STATUS);
  +   if (ret  0 || (ret  0x1f) == 0) {
  +   mdelay(50);
  +   ret = twl4030_usb_read(twl, ID_STATUS);
  +   }
  +
  +   if (twl-usb_mode == T2_USB_MODE_ULPI)
  +   twl4030_i2c_access(twl, 0);
  +   pm_runtime_put_autosuspend(twl-dev);
  +
  +   if (ret  0)
  +   return TWL4030_ID_UNKNOWN;
  +   ret = ffs(ret) - 1;
  +   if (ret  TWL4030_GROUND || ret  TWL4030_FLOATING)
  +   return TWL4030_ID_UNKNOWN;
  +
  +   return ret;
  +}
  +
static void __twl4030_phy_power(struct twl4030_usb *twl, int on)
{
  u8 pwr = twl4030_usb_read(twl, PHY_PWR_CTRL);
  @@ -532,6 +582,16 @@ static ssize_t twl4030_usb_vbus_show(struct device 
  *dev,
}
static DEVICE_ATTR(vbus, 0444, twl4030_usb_vbus_show, NULL);
 
  +static ssize_t twl4030_usb_id_show(struct device *dev,
  +  struct device_attribute *attr,
  +  char *buf)
  +{
  +   struct twl4030_usb *twl = dev_get_drvdata(dev);
  +   return scnprintf(buf, PAGE_SIZE, %s\n,
  +twl4030_id_names[twl4030_get_id(twl)]);
  +}
  +static DEVICE_ATTR(id, 0444, twl4030_usb_id_show, NULL);
  +
static irqreturn_t twl4030_usb_irq(int irq, void *_twl)
{
  struct twl4030_usb *twl = _twl;
  @@ -709,6 +769,8 @@ static int twl4030_usb_probe(struct platform_device 
  *pdev)
  platform_set_drvdata(pdev, twl);
  if (device_create_file(pdev-dev, dev_attr_vbus))
  

[PATCH 5/6] phy: twl4030-usb: add support for reading resistor on ID pin.

2015-04-16 Thread NeilBrown
From: NeilBrown ne...@suse.de

The twl4030 phy can measure, with low precision, the
resistance-to-ground of the ID pin.

Add a function to read the value, and export the result
via sysfs.

If the read fails, which it does sometimes, try again in 50msec.

Acked-by: Pavel Machek pa...@ucw.cz
Signed-off-by: NeilBrown ne...@suse.de
---
 .../ABI/testing/sysfs-platform-twl4030-usb |   22 +++
 drivers/phy/phy-twl4030-usb.c  |   63 
 2 files changed, 85 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-platform-twl4030-usb 
b/Documentation/ABI/testing/sysfs-platform-twl4030-usb
index 512c51be64ae..425d23676f8a 100644
--- a/Documentation/ABI/testing/sysfs-platform-twl4030-usb
+++ b/Documentation/ABI/testing/sysfs-platform-twl4030-usb
@@ -6,3 +6,25 @@ Description:
Possible values: on, off.
 
Changes are notified via select/poll.
+
+What: /sys/bus/platform/devices/*twl4030-usb/id
+Description:
+   Read-only report on measurement of USB-OTG ID pin.
+
+   The ID pin may be floating, grounded, or pulled to
+   ground by a resistor.
+
+   A very course grained reading of the resistance is
+   available.  The numbers given in kilo-ohms are roughly
+   the center-point of the detected range.
+
+   Possible values are:
+   ground
+   102k
+   200k
+   440k
+   floating
+   unknown
+
+   unknown indicates a problem with trying to detect
+   the resistance.
diff --git a/drivers/phy/phy-twl4030-usb.c b/drivers/phy/phy-twl4030-usb.c
index 3a707dd14238..1d6f3e70193e 100644
--- a/drivers/phy/phy-twl4030-usb.c
+++ b/drivers/phy/phy-twl4030-usb.c
@@ -379,6 +379,56 @@ static void twl4030_i2c_access(struct twl4030_usb *twl, 
int on)
}
 }
 
+enum twl4030_id_status {
+   TWL4030_GROUND,
+   TWL4030_102K,
+   TWL4030_200K,
+   TWL4030_440K,
+   TWL4030_FLOATING,
+   TWL4030_ID_UNKNOWN,
+};
+static char *twl4030_id_names[] = {
+   ground,
+   102k,
+   200k,
+   440k,
+   floating,
+   unknown
+};
+
+enum twl4030_id_status twl4030_get_id(struct twl4030_usb *twl)
+{
+   int ret;
+
+   pm_runtime_get_sync(twl-dev);
+   if (twl-usb_mode == T2_USB_MODE_ULPI)
+   twl4030_i2c_access(twl, 1);
+   ret = twl4030_usb_read(twl, ULPI_OTG_CTRL);
+   if (ret  0 || !(ret  ULPI_OTG_ID_PULLUP)) {
+   /* Need pull-up to read ID */
+   twl4030_usb_set_bits(twl, ULPI_OTG_CTRL,
+ULPI_OTG_ID_PULLUP);
+   mdelay(50);
+   }
+   ret = twl4030_usb_read(twl, ID_STATUS);
+   if (ret  0 || (ret  0x1f) == 0) {
+   mdelay(50);
+   ret = twl4030_usb_read(twl, ID_STATUS);
+   }
+
+   if (twl-usb_mode == T2_USB_MODE_ULPI)
+   twl4030_i2c_access(twl, 0);
+   pm_runtime_put_autosuspend(twl-dev);
+
+   if (ret  0)
+   return TWL4030_ID_UNKNOWN;
+   ret = ffs(ret) - 1;
+   if (ret  TWL4030_GROUND || ret  TWL4030_FLOATING)
+   return TWL4030_ID_UNKNOWN;
+
+   return ret;
+}
+
 static void __twl4030_phy_power(struct twl4030_usb *twl, int on)
 {
u8 pwr = twl4030_usb_read(twl, PHY_PWR_CTRL);
@@ -532,6 +582,16 @@ static ssize_t twl4030_usb_vbus_show(struct device *dev,
 }
 static DEVICE_ATTR(vbus, 0444, twl4030_usb_vbus_show, NULL);
 
+static ssize_t twl4030_usb_id_show(struct device *dev,
+  struct device_attribute *attr,
+  char *buf)
+{
+   struct twl4030_usb *twl = dev_get_drvdata(dev);
+   return scnprintf(buf, PAGE_SIZE, %s\n,
+twl4030_id_names[twl4030_get_id(twl)]);
+}
+static DEVICE_ATTR(id, 0444, twl4030_usb_id_show, NULL);
+
 static irqreturn_t twl4030_usb_irq(int irq, void *_twl)
 {
struct twl4030_usb *twl = _twl;
@@ -709,6 +769,8 @@ static int twl4030_usb_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, twl);
if (device_create_file(pdev-dev, dev_attr_vbus))
dev_warn(pdev-dev, could not create sysfs file\n);
+   if (device_create_file(pdev-dev, dev_attr_id))
+   dev_warn(pdev-dev, could not create sysfs file\n);
 
ATOMIC_INIT_NOTIFIER_HEAD(twl-phy.notifier);
 
@@ -753,6 +815,7 @@ static int twl4030_usb_remove(struct platform_device *pdev)
pm_runtime_get_sync(twl-dev);
cancel_delayed_work(twl-id_workaround_work);
device_remove_file(twl-dev, dev_attr_vbus);
+   device_remove_file(twl-dev, dev_attr_id);
 
/* set transceiver mode to power on defaults */
twl4030_usb_set_mode(twl, -1);


--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html