Enhancements to twl4030 phy to support better charging.

From: NeilBrown <[email protected]>

This driver device one local attribute: vbus.
Describe that in Documentation/ABI/testing/sysfs-platform/twl4030-usb.

Signed-off-by NeilBrown <[email protected]>

---
 .../ABI/testing/sysfs-platform-twl4030-usb         |    8 ++++++++
 1 file changed, 8 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-platform-twl4030-usb

Documentation/ABI/testing/sysfs-platform-twl4030-usb new file +0 -0

new file mode 100644
index 000000000000..512c51be64ae

Quoted 11 lines · NeilBrown

From: NeilBrown <[email protected]>

This driver device one local attribute: vbus.
Describe that in Documentation/ABI/testing/sysfs-platform/twl4030-usb.

Signed-off-by NeilBrown <[email protected]>

---
 .../ABI/testing/sysfs-platform-twl4030-usb         |    8 ++++++++
 1 file changed, 8 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-platform-twl4030-usb

Documentation/ABI/testing/sysfs-platform-twl4030-usb line 6

Quoted 12 lines

Documentation/ABI/testing/sysfs-platform-twl4030-usb new file +6 -0

new file mode 100644
index 000000000000..512c51be64ae
@@ -0,0 +1,8 @@
+What: /sys/bus/platform/devices/*twl4030-usb/vbus
+Description:
+	Read-only status reporting if VBUS (approx 5V)
+	is being supplied by the USB bus.
+
+	Possible values: "on", "off".

Would bit be better to have values "0" and "1"? Kernel usually does that for booleans...

From: NeilBrown <[email protected]>

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 <[email protected]>

Signed-off-by NeilBrown <[email protected]>

---
  .../ABI/testing/sysfs-platform-twl4030-usb         |   22 +++++++
  drivers/phy/phy-twl4030-usb.c                      |   63 ++++++++++++++++++++
  2 files changed, 85 insertions(+)

Documentation/ABI/testing/sysfs-platform-twl4030-usb +22 -0

index 512c51be64ae..425d23676f8a 100644
@@ -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.

drivers/phy/phy-twl4030-usb.c +63 -0

index 3a707dd14238..1d6f3e70193e 100644
@@ -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);

Hi,

Quoted 2 lines · NeilBrown

From: NeilBrown <[email protected]>

Quoted 5 lines

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

Quoted 137 lines

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

Acked-by Pavel Machek <[email protected]>

Signed-off-by NeilBrown <[email protected]>

---
  .../ABI/testing/sysfs-platform-twl4030-usb         |   22 +++++++
  drivers/phy/phy-twl4030-usb.c                      |   63 ++++++++++++++++++++
  2 files changed, 85 insertions(+)

Documentation/ABI/testing/sysfs-platform-twl4030-usb +22 -0

index 512c51be64ae..425d23676f8a 100644
@@ -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.

drivers/phy/phy-twl4030-usb.c +63 -0

index 3a707dd14238..1d6f3e70193e 100644
@@ -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);

Quoted 5 lines · On Mon, 1 Jun 2015 19:06:52 +0530 Kishon Vijay Abraham I <[email protected]> wrote:

Hi,

Quoted 2 lines · NeilBrown

From: NeilBrown <[email protected]>

Quoted 8 lines

Quoted 5 lines

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

Quoted 140 lines

Thanks
Kishon

Quoted 137 lines

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

Acked-by Pavel Machek <[email protected]>

Signed-off-by NeilBrown <[email protected]>

---
   .../ABI/testing/sysfs-platform-twl4030-usb         |   22 +++++++
   drivers/phy/phy-twl4030-usb.c                      |   63 ++++++++++++++++++++
   2 files changed, 85 insertions(+)

Documentation/ABI/testing/sysfs-platform-twl4030-usb +22 -0

index 512c51be64ae..425d23676f8a 100644
@@ -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.

drivers/phy/phy-twl4030-usb.c +63 -0

index 3a707dd14238..1d6f3e70193e 100644
@@ -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);

Hi,

Quoted 23 lines · NeilBrown

Quoted 5 lines · On Mon, 1 Jun 2015 19:06:52 +0530 Kishon Vijay Abraham I <[email protected]> wrote:

Hi,

Quoted 2 lines · NeilBrown

From: NeilBrown <[email protected]>

Quoted 8 lines

Quoted 5 lines

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.

Quoted 6 lines

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

Quoted 141 lines

Thanks,
NeilBrown

Quoted 136 lines

Thanks
Kishon

Quoted 133 lines

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

Acked-by Pavel Machek <[email protected]>

Signed-off-by NeilBrown <[email protected]>

---
  .../ABI/testing/sysfs-platform-twl4030-usb         |   22 +++++++
  drivers/phy/phy-twl4030-usb.c                      |   63 ++++++++++++++++++++
  2 files changed, 85 insertions(+)

Documentation/ABI/testing/sysfs-platform-twl4030-usb +22 -0

index 512c51be64ae..425d23676f8a 100644
@@ -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.

drivers/phy/phy-twl4030-usb.c +63 -0

index 3a707dd14238..1d6f3e70193e 100644
@@ -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);
 

Quoted 5 lines

Quoted 4 lines

drivers/phy/phy-twl4030-usb.c static int twl4030_usb_remove(struct platform_device *pdev) · line 821

Quoted 4 lines

drivers/phy/phy-twl4030-usb.c +0 -0

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

_______________________________________________
Gta04-owner mailing list
[email protected]
http://lists.goldelico.com/mailman/listinfo.cgi/gta04-owner

Hi,

Am 02.06.2015 um 15:49 schrieb Kishon Vijay Abraham I <[email protected]>:

Quoted 32 lines

Hi,

Quoted 29 lines · NeilBrown

Quoted 5 lines · On Mon, 1 Jun 2015 19:06:52 +0530 Kishon Vijay Abraham I <[email protected]> wrote:

Hi,

Quoted 2 lines · NeilBrown

From: NeilBrown <[email protected]>

Quoted 8 lines

Quoted 5 lines

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?

Quoted 1 line

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

Is /sys considered ABI?

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

Quoted 30 lines

Hi,

Am 02.06.2015 um 15:49 schrieb Kishon Vijay Abraham I <[email protected]>:

Quoted 26 lines

Hi,

Quoted 23 lines · NeilBrown

Quoted 5 lines · On Mon, 1 Jun 2015 19:06:52 +0530 Kishon Vijay Abraham I <[email protected]> wrote:

Hi,

Quoted 2 lines · NeilBrown

From: NeilBrown <[email protected]>

Quoted 8 lines

Quoted 5 lines

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.

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

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?

Quoted 4 lines

Quoted 2 lines

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

Is /sys considered ABI?

Yes.

Quoted 2 lines

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

No.

Quoted 5 lines · On Mon, 1 Jun 2015 19:06:52 +0530 Kishon Vijay Abraham I <[email protected]> wrote:

Hi,

Quoted 2 lines · NeilBrown

From: NeilBrown <[email protected]>

Quoted 8 lines

Quoted 5 lines

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?

Quoted 7 lines

Documentation/ABI/testing/sysfs-platform-twl4030-usb Description: · line 27

Quoted 7 lines

Documentation/ABI/testing/sysfs-platform-twl4030-usb +7 -0

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

Quoted 27 lines · NeilBrown

Quoted 5 lines · On Mon, 1 Jun 2015 19:06:52 +0530 Kishon Vijay Abraham I <[email protected]> wrote:

Hi,

Quoted 2 lines · NeilBrown

From: NeilBrown <[email protected]>

Quoted 8 lines

Quoted 5 lines

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.

Quoted 2 lines

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?

Quoted 7 lines

Quoted 7 lines

Documentation/ABI/testing/sysfs-platform-twl4030-usb Description: · line 27

Quoted 7 lines

Documentation/ABI/testing/sysfs-platform-twl4030-usb +7 -0

+	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?

Quoted 8 lines · On Mon, 1 Jun 2015 19:06:52 +0530 Kishon Vijay Abraham I <[email protected]> wrote:

Hi,

Quoted 5 lines · NeilBrown

From: NeilBrown <[email protected]>

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

Quoted 5 lines

Quoted 2 lines

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

Quoted 136 lines

Thanks
Kishon

Quoted 133 lines

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

Acked-by Pavel Machek <[email protected]>

Signed-off-by NeilBrown <[email protected]>

---
 .../ABI/testing/sysfs-platform-twl4030-usb         |   22 +++++++
 drivers/phy/phy-twl4030-usb.c                      |   63 ++++++++++++++++++++
 2 files changed, 85 insertions(+)

Documentation/ABI/testing/sysfs-platform-twl4030-usb +22 -0

index 512c51be64ae..425d23676f8a 100644
@@ -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.

drivers/phy/phy-twl4030-usb.c +63 -0

index 3a707dd14238..1d6f3e70193e 100644
@@ -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);
 

Quoted 4 lines

drivers/phy/phy-twl4030-usb.c static int twl4030_usb_remove(struct platform_device *pdev) · line 821

Quoted 4 lines

drivers/phy/phy-twl4030-usb.c +0 -0

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

_______________________________________________
Gta04-owner mailing list
[email protected]
http://lists.goldelico.com/mailman/listinfo.cgi/gta04-owner

From: NeilBrown <[email protected]>

Signed-off-by NeilBrown <[email protected]>

---
  drivers/phy/phy-twl4030-usb.c |   67 +++++++++++++++++++++++++++++++++++++++++
  1 file changed, 67 insertions(+)

drivers/phy/phy-twl4030-usb.c +53 -0

index 1d6f3e70193e..c42153d43ec2 100644
@@ -40,6 +40,7 @@
  #include <linux/regulator/consumer.h>
  #include <linux/err.h>
  #include <linux/slab.h>
+#include <linux/extcon.h>
 
  /* Register defines */
 

@@ -173,6 +174,9 @@ struct twl4030_usb {
  	enum omap_musb_vbus_id_status linkstat;
  	bool			vbus_supplied;
 
+	/* cable connection */
+	struct extcon_dev	edev;
+
  	struct delayed_work	id_workaround_work;
  };
 

@@ -592,6 +596,54 @@ static ssize_t twl4030_usb_id_show(struct device *dev,
  }
  static DEVICE_ATTR(id, 0444, twl4030_usb_id_show, NULL);
 
+static const char *usb_cables[] = {
+	"USB", /* id is floating */
+	"Charger-downstream", /* id is floating and D+ shorted to D- */
+	"USB-Host", /* id is ground */
+	"USB-ACA", /* "Accessory Charger Adapter" id is something else */
+	NULL
+};
+enum {
+	TWL_CABLE_USB,
+	TWL_CABLE_CHARGER,	/* Not used - twl4030 can detect charger,
+				 * but driver cannot yet */
+	TWL_CABLE_OTG,
+	TWL_CABLE_ACA,
+};
+static u32 all_exclusive[] =  {0xFFFFFFFF, 0};
+
+static void twl4030_usb_report_cable(struct twl4030_usb *twl)
+{
+	enum twl4030_id_status sts;
+
+	if (!cable_present(twl->linkstat)) {
+		extcon_set_state(&twl->edev, 0);
+		return;
+	}
+
+	sts = twl4030_get_id(twl);
+
+	switch (sts) {
+	case TWL4030_FLOATING: /* USB downstream */
+		extcon_update_state(&twl->edev,
+				    1<<TWL_CABLE_USB,
+				    1<<TWL_CABLE_USB);
+		break;
+	case TWL4030_GROUND: /* USB host */
+		extcon_update_state(&twl->edev,
+				    1<<TWL_CABLE_OTG,
+				    1<<TWL_CABLE_OTG);
+		break;
+	default: /* Some resistor */
+		extcon_update_state(&twl->edev,
+				    1<<TWL_CABLE_ACA,
+				    1<<TWL_CABLE_ACA);
+		/* An ACA should set port to 'host' mode */
+		twl->linkstat = OMAP_MUSB_ID_GROUND;
+		break;
+	}
+}
+
  static irqreturn_t twl4030_usb_irq(int irq, void *_twl)
  {
  	struct twl4030_usb *twl = _twl;

@@ -628,6 +680,7 @@ static irqreturn_t twl4030_usb_irq(int irq, void *_twl)
  			pm_runtime_put_autosuspend(twl->dev);
  		}
  		omap_musb_mailbox(status);
+		twl4030_usb_report_cable(twl);
  	}
 
  	/* don't schedule during sleep - irq works right then */

+extcon MAINTAINERS

Hi,

Quoted 2 lines · NeilBrown

From: NeilBrown <[email protected]>

commit msg pls.

Quoted 34 lines

Signed-off-by NeilBrown <[email protected]>

---
  drivers/phy/phy-twl4030-usb.c |   67
+++++++++++++++++++++++++++++++++++++++++
  1 file changed, 67 insertions(+)

drivers/phy/phy-twl4030-usb.c +4 -0

index 1d6f3e70193e..c42153d43ec2 100644
@@ -40,6 +40,7 @@
  #include <linux/regulator/consumer.h>
  #include <linux/err.h>
  #include <linux/slab.h>
+#include <linux/extcon.h>
 
  /* Register defines */
 

@@ -173,6 +174,9 @@ struct twl4030_usb {
        enum omap_musb_vbus_id_status linkstat;
        bool                    vbus_supplied;
 
+       /* cable connection */
+       struct extcon_dev       edev;
+
        struct delayed_work     id_workaround_work;
  };
 

@@ -592,6 +596,54 @@ static ssize_t twl4030_usb_id_show(struct device
 *dev,
  }
  static DEVICE_ATTR(id, 0444, twl4030_usb_id_show, NULL);
 

drivers/phy/phy-twl4030-usb.c static ssize_t twl4030_usb_id_show(struct device *dev, · line 604

Quoted 6 lines

drivers/phy/phy-twl4030-usb.c +6 -0

+static const char *usb_cables[] = {
+       "USB", /* id is floating */
+       "Charger-downstream", /* id is floating and D+ shorted to D- */
+       "USB-Host", /* id is ground */
+       "USB-ACA", /* "Accessory Charger Adapter" id is something else */
+       NULL

Quoted 32 lines

drivers/phy/phy-twl4030-usb.c +32 -0

+};
+enum {
+       TWL_CABLE_USB,
+       TWL_CABLE_CHARGER,      /* Not used - twl4030 can detect charger,
+                                * but driver cannot yet */
+       TWL_CABLE_OTG,
+       TWL_CABLE_ACA,
+};
+static u32 all_exclusive[] =  {0xFFFFFFFF, 0};
+
+static void twl4030_usb_report_cable(struct twl4030_usb *twl)
+{
+       enum twl4030_id_status sts;
+
+       if (!cable_present(twl->linkstat)) {
+               extcon_set_state(&twl->edev, 0);
+               return;
+       }
+
+       sts = twl4030_get_id(twl);
+
+       switch (sts) {
+       case TWL4030_FLOATING: /* USB downstream */
+               extcon_update_state(&twl->edev,
+                                   1<<TWL_CABLE_USB,
+                                   1<<TWL_CABLE_USB);
+               break;
+       case TWL4030_GROUND: /* USB host */
+               extcon_update_state(&twl->edev,
+                                   1<<TWL_CABLE_OTG,
+                                   1<<TWL_CABLE_OTG);
+               break;

drivers/phy/phy-twl4030-usb.c static ssize_t twl4030_usb_id_show(struct device *dev, · line 644

Quoted 8 lines

drivers/phy/phy-twl4030-usb.c +8 -0

+       default: /* Some resistor */
+               extcon_update_state(&twl->edev,
+                                   1<<TWL_CABLE_ACA,
+                                   1<<TWL_CABLE_ACA);
+               /* An ACA should set port to 'host' mode */
+               twl->linkstat = OMAP_MUSB_ID_GROUND;
+               break;
+       }

Quoted 13 lines

drivers/phy/phy-twl4030-usb.c +2 -0

+}
+
  static irqreturn_t twl4030_usb_irq(int irq, void *_twl)
  {

        struct twl4030_usb *twl = _twl;

drivers/phy/phy-twl4030-usb.c +1 -0

@@ -628,6 +680,7 @@ static irqreturn_t twl4030_usb_irq(int irq, void
 *_twl)
                        pm_runtime_put_autosuspend(twl->dev);
                }
                omap_musb_mailbox(status);
+               twl4030_usb_report_cable(twl);
        }
 

drivers/phy/phy-twl4030-usb.c static int twl4030_usb_probe(struct platform_device · line 824

Quoted 8 lines

        /* don't schedule during sleep - irq works right then */

drivers/phy/phy-twl4030-usb.c +2 -0

@@ -766,6 +819,20 @@ static int twl4030_usb_probe(struct platform_device
 *pdev)
        }
        usb_add_phy_dev(&twl->phy);
 
+       twl->edev.name = devm_kasprintf(twl->dev, GFP_KERNEL, "%s-usb",
+                                       dev_name(twl->dev->parent));

drivers/phy/phy-twl4030-usb.c static int twl4030_usb_probe(struct platform_device *pdev) · line 827

Quoted 4 lines

drivers/phy/phy-twl4030-usb.c +4 -0

+       twl->edev.supported_cable = usb_cables;
+       twl->edev.mutually_exclusive = all_exclusive;
+       twl->edev.print_name = NULL; /* why would you change this? */
+       twl->edev.print_state = NULL; /* probably want to change this */

Not sure about those two callbacks. Chanwoo?

Thanks
Kishon