linux-kernel - Re: [PATCH 8/8] phy: add driver for TI TUSB1210 ULPI PHY

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] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 13 Feb 2015 14:35:16 +0200
From:	Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To:	David Cohen <david.a.cohen@...ux.intel.com>
Cc:	Felipe Balbi <balbi@...com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Baolu Lu <baolu.lu@...ux.intel.com>, linux-usb@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Kishon Vijay Abraham I <kishon@...com>
Subject: Re: [PATCH 8/8] phy: add driver for TI TUSB1210 ULPI PHY

Hi,

On Thu, Feb 12, 2015 at 05:52:42PM -0800, David Cohen wrote:
> Hi Heikki,
> 
> Sorry I am starting a new branch on this thread.
> I need to go back to another topic on this same patch.
> 
> On Fri, Jan 23, 2015 at 05:12:58PM +0200, Heikki Krogerus wrote:
> > TUSB1210 ULPI PHY has vendor specific register for eye
> > diagram tuning. On some platforms the system firmware has
> > set optimized value to it. In order to not loose the
> > optimized value, the driver stores it during probe and
> > restores it every time the PHY is powered back on.
> > 
> > Signed-off-by: Heikki Krogerus <heikki.krogerus@...ux.intel.com>
> > Cc: Kishon Vijay Abraham I <kishon@...com>
> > ---
> 
> [snip]
> 
> > +	/* Store initial eye diagram optimisation value */
> > +	ret = ulpi_read(ulpi, TUSB1210_VENDOR_SPECIFIC2);
> 
> We can't rely on eye diagram optimization value here. It's possible the
> phy went through reset (e.g. module unloading/loading).
> We need a more consistent method. Could we use _DSD instead? That would
> be more compatible with DT too.

I'm don't have any problem with getting the value from a property.
Sounds like the correct thing to do. Are you thinking about putting
that _DSD method to the ACPI device object for the dwc3? The correct
place would probable be separate device object for the PHY, but if we
do that we need to consider how that object is bound to the ulpi
device. There are few ways we can do that, so it requires some
thinking.

In any case this driver we can already implement so that it expects to
get the value from property. We just need the name for it.

The register has actually separate fields for High speed output
impedance configuration and High speed output driver strength
configuration for eye diagram tuning, plus control bit for DP/DM swap.

Maybe we should actually get them from three separate properties:

        device_property_read_u8(dev, "datapolarity", &val);
        ...
        device_property_read_u8(dev, "zhsdrv", &val);
        ...
        device_property_read_u8(dev, "ihstx", &val);
        ...

What do you think?


Cheers,

-- 
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/