linux-kernel - Re: [PATCH 3/3] phy: ulpi: add driver for TI TUSB1210

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20150123161838.GE8585@saruman.tx.rr.com>
Date:	Fri, 23 Jan 2015 10:18:38 -0600
From:	Felipe Balbi <balbi@...com>
To:	Heikki Krogerus <heikki.krogerus@...ux.intel.com>
CC:	Felipe Balbi <balbi@...com>,
	Kishon Vijay Abraham I <kishon@...com>,
	Baolu Lu <baolu.lu@...ux.intel.com>,
	<linux-usb@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/3] phy: ulpi: add driver for TI TUSB1210

On Fri, Jan 23, 2015 at 10:12:41AM +0200, Heikki Krogerus wrote:
> Hi,
> 
> On Thu, Jan 22, 2015 at 02:49:25PM -0600, Felipe Balbi wrote:
> > On Wed, Jan 21, 2015 at 11:17:49AM +0200, Heikki Krogerus wrote:
> > > > > +	/* Store initial eye diagram optimisation value */
> > > > > +	ret = ulpi_read(ulpi, ULPI_EXT_VENDOR_SPECIFIC);
> > > > 
> > > > do they *all* use this register for eye diagram optimization or is this
> > > > something that Intel decided to do ?
> > > > 
> > > > (sorry, don't know much about tusb1210 other than it sucks like hell :-)
> > > 
> > > All I know that somebody needs to save the value. The ones using this
> > > PHY who don't need to save it can most likely live without the driver.
> > 
> > right, but what I mean is: is it mandatory that Eye diagram
> > configuration be stored in *this* register? Or is it more like a scratch
> > register which Intel just happens to be using for Eye diagram data ?
> 
> The eye diagram tuning is in that register. Here's the spec:
> http://www.ti.com/lit/ds/symlink/tusb1210.pdf
> 
> I'll add definition for the register (which is colourfully named
> "VENDOR_SPECIFIC2").

alright, thanks.

> > > > > +	if (ret < 0)
> > > > > +		return ret;
> > > > > +
> > > > > +	tusb->ctx[0] = ret;
> > > > > +
> > > > > +	tusb->phy = ulpi_phy_create(ulpi, &phy_ops);
> > > > > +	if (IS_ERR(tusb->phy))
> > > > > +		return PTR_ERR(tusb->phy);
> > > > > +
> > > > > +	tusb->ulpi = ulpi;
> > > > > +
> > > > > +	phy_set_drvdata(tusb->phy, tusb);
> > > > > +	dev_set_drvdata(&ulpi->dev, tusb);
> > > > > +	return 0;
> > > > > +}
> > > > > +
> > > > > +static void tusb1210_remove(struct ulpi *ulpi)
> > > > > +{
> > > > > +	struct tusb1210 *tusb = dev_get_drvdata(&ulpi->dev);
> > > > 
> > > > completely unrelated to $subject, but we might want to have a
> > > > ulpi_{set,get}_drvdata() at some point.
> > > 
> > > Makes sense.
> > > 
> > > > In fact, we might decide to add an entire ULPI bus, eventually, though
> > > > I'm still considering if there's any benefit to that.
> > > 
> > > I don't think I understand this comment? ULPI bus is what I'm
> > > introducing in this set (the first patch in it)?
> > 
> > I mean introducing a real struct bus ulpi_bus_type :-) With match,
> > probe, remove, etc.
> 
> I'm already doing that. Please check the first patch in this set:
> "phy: add bus for USB ULPI PHYs".

yeah, sorry about that.

-- 
balbi

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)