diff mbox

[3/4] usb: phy: twl4030: add support for reading restore on ID pin.

Message ID 20150224034037.31400.52748.stgit@notabene.brown (mailing list archive)
State New, archived
Headers show

Commit Message

NeilBrown Feb. 24, 2015, 3:40 a.m. UTC
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.

Signed-off-by: NeilBrown <neilb@suse.de>
---
 drivers/phy/phy-twl4030-usb.c |   63 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)



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

Comments

Pavel Machek March 2, 2015, 9:04 p.m. UTC | #1
Hi!

> 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.
> 
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
>  drivers/phy/phy-twl4030-usb.c |   63 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 63 insertions(+)
> 
> diff --git a/drivers/phy/phy-twl4030-usb.c b/drivers/phy/phy-twl4030-usb.c
> index 023fe150c7a1..759950898df9 100644
> --- a/drivers/phy/phy-twl4030-usb.c
> +++ b/drivers/phy/phy-twl4030-usb.c
> @@ -374,6 +374,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",

New /sys files should be documented somewhere...?

Does it make sense to change "440k" -> "440KOhm"?

Plus I guess you need to update Documentation/

Acked-by: Pavel Machek <pavel@ucw.cz>
NeilBrown March 4, 2015, 6:35 a.m. UTC | #2
On Mon, 2 Mar 2015 22:04:31 +0100 Pavel Machek <pavel@ucw.cz> wrote:

> Hi!
> 
> > 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.
> > 
> > Signed-off-by: NeilBrown <neilb@suse.de>
> > ---
> >  drivers/phy/phy-twl4030-usb.c |   63 +++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 63 insertions(+)
> > 
> > diff --git a/drivers/phy/phy-twl4030-usb.c b/drivers/phy/phy-twl4030-usb.c
> > index 023fe150c7a1..759950898df9 100644
> > --- a/drivers/phy/phy-twl4030-usb.c
> > +++ b/drivers/phy/phy-twl4030-usb.c
> > @@ -374,6 +374,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",
> 
> New /sys files should be documented somewhere...?

Preferably with the code...

> 
> Does it make sense to change "440k" -> "440KOhm"?

Interesting question.  I prefer to avoid including units in files - bare
numbers is better.  But there is no number to match "floating" unless I spell
it out as "infinity", and wouldn't be helpful.

Certainly "K" would be preferred over "k", and given that I have "ground"
and  "floating", it is more consistent to include the "Ohm"....

These are really names, not measures of resistance.  The data sheet calls
them:
 ID_RES_FLOAT  (or sometimes ID_FLOAT)
 ID_RES_440K
 ID_RES_200K
 ID_RES_102K
 ID_GND        (or sometimes ID_RES_GND)

So using those names is defensible.

I think I'll change them all to upper case, but leave out the "Ohm".
My justification is consistency with the data sheet.




> 
> Plus I guess you need to update Documentation/

I guess I'll need to give in to this eventually :-)

> 
> Acked-by: Pavel Machek <pavel@ucw.cz>
> 

Thanks,
NeilBrown
H. Nikolaus Schaller March 4, 2015, 6:54 a.m. UTC | #3
Am 04.03.2015 um 07:35 schrieb NeilBrown <neilb@suse.de>:

> On Mon, 2 Mar 2015 22:04:31 +0100 Pavel Machek <pavel@ucw.cz> wrote:
> 
>> Hi!
>> 
>>> 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.
>>> 
>>> Signed-off-by: NeilBrown <neilb@suse.de>
>>> ---
>>> drivers/phy/phy-twl4030-usb.c |   63 +++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 63 insertions(+)
>>> 
>>> diff --git a/drivers/phy/phy-twl4030-usb.c b/drivers/phy/phy-twl4030-usb.c
>>> index 023fe150c7a1..759950898df9 100644
>>> --- a/drivers/phy/phy-twl4030-usb.c
>>> +++ b/drivers/phy/phy-twl4030-usb.c
>>> @@ -374,6 +374,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",
>> 
>> New /sys files should be documented somewhere...?
> 
> Preferably with the code...
> 
>> 
>> Does it make sense to change "440k" -> "440KOhm"?
> 
> Interesting question.  I prefer to avoid including units in files - bare
> numbers is better.  But there is no number to match "floating" unless I spell
> it out as "infinity", and wouldn't be helpful.
> 
> Certainly "K" would be preferred over "k“,

The international standard for kilo = 1000 is a lower case k.
While it is (non-standard) to use K for 1024:

http://en.wikipedia.org/wiki/Kilobyte

So I would keep the string constants lower case to avoid this potential confusion.

> and given that I have "ground"
> and  "floating", it is more consistent to include the "Ohm"....
> 
> These are really names, not measures of resistance.  The data sheet calls
> them:
> ID_RES_FLOAT  (or sometimes ID_FLOAT)
> ID_RES_440K
> ID_RES_200K
> ID_RES_102K
> ID_GND        (or sometimes ID_RES_GND)
> 
> So using those names is defensible.
> 
> I think I'll change them all to upper case, but leave out the "Ohm".
> My justification is consistency with the data sheet.
> 
> 
> 
> 
>> 
>> Plus I guess you need to update Documentation/
> 
> I guess I'll need to give in to this eventually :-)
> 
>> 
>> Acked-by: Pavel Machek <pavel@ucw.cz>
>> 
> 
> Thanks,
> NeilBrown
> _______________________________________________
> Gta04-owner mailing list
> Gta04-owner@goldelico.com
> http://lists.goldelico.com/mailman/listinfo.cgi/gta04-owner

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pavel Machek March 4, 2015, 10:17 a.m. UTC | #4
Hi!

> > New /sys files should be documented somewhere...?
> 
> Preferably with the code...
> 
> > Does it make sense to change "440k" -> "440KOhm"?
> 
> Interesting question.  I prefer to avoid including units in files - bare
> numbers is better.  But there is no number to match "floating" unless I spell
> it out as "infinity", and wouldn't be helpful.
> 
> Certainly "K" would be preferred over "k", and given that I have "ground"
> and  "floating", it is more consistent to include the "Ohm"....
> 
> These are really names, not measures of resistance.  The data sheet calls
> them:
>  ID_RES_FLOAT  (or sometimes ID_FLOAT)
>  ID_RES_440K
>  ID_RES_200K
>  ID_RES_102K
>  ID_GND        (or sometimes ID_RES_GND)
> 
> So using those names is defensible.
> 
> I think I'll change them all to upper case, but leave out the "Ohm".
> My justification is consistency with the data sheet.

Does it make sense to use "_ohm" in the attribute name, then? (And
yes, I was wrong with the "K", "k" is actually right.)

> > Plus I guess you need to update Documentation/
> 
> I guess I'll need to give in to this eventually :-)

Yes please. It was useful in past.
									Pavel
NeilBrown March 22, 2015, 6:05 a.m. UTC | #5
On Wed, 4 Mar 2015 07:54:41 +0100 "Dr. H. Nikolaus Schaller"
<hns@goldelico.com> wrote:

> 
> Am 04.03.2015 um 07:35 schrieb NeilBrown <neilb@suse.de>:
> 
> > On Mon, 2 Mar 2015 22:04:31 +0100 Pavel Machek <pavel@ucw.cz> wrote:
> > 
> >> Hi!
> >> 
> >>> 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.
> >>> 
> >>> Signed-off-by: NeilBrown <neilb@suse.de>
> >>> ---
> >>> drivers/phy/phy-twl4030-usb.c |   63 +++++++++++++++++++++++++++++++++++++++++
> >>> 1 file changed, 63 insertions(+)
> >>> 
> >>> diff --git a/drivers/phy/phy-twl4030-usb.c b/drivers/phy/phy-twl4030-usb.c
> >>> index 023fe150c7a1..759950898df9 100644
> >>> --- a/drivers/phy/phy-twl4030-usb.c
> >>> +++ b/drivers/phy/phy-twl4030-usb.c
> >>> @@ -374,6 +374,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",
> >> 
> >> New /sys files should be documented somewhere...?
> > 
> > Preferably with the code...
> > 
> >> 
> >> Does it make sense to change "440k" -> "440KOhm"?
> > 
> > Interesting question.  I prefer to avoid including units in files - bare
> > numbers is better.  But there is no number to match "floating" unless I spell
> > it out as "infinity", and wouldn't be helpful.
> > 
> > Certainly "K" would be preferred over "k“,
> 
> The international standard for kilo = 1000 is a lower case k.
> While it is (non-standard) to use K for 1024:
> 
> http://en.wikipedia.org/wiki/Kilobyte
> 
> So I would keep the string constants lower case to avoid this potential confusion.

Yes, I got that backwards, didn't I.

I think I'll leave it as lower-case.
I won't include the word "ohm" - it is very uncommon to have units explicit
in sysfs attribute values.

Thanks,
NeilBrown
diff mbox

Patch

diff --git a/drivers/phy/phy-twl4030-usb.c b/drivers/phy/phy-twl4030-usb.c
index 023fe150c7a1..759950898df9 100644
--- a/drivers/phy/phy-twl4030-usb.c
+++ b/drivers/phy/phy-twl4030-usb.c
@@ -374,6 +374,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);
@@ -531,6 +581,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;
@@ -723,6 +783,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);
 
@@ -768,6 +830,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);