Hi David,
On Sat, Jan 24, 2015 at 03:58:11PM -0800, David Cohen wrote:
> > +static int tusb1210_power_on(struct phy *phy)
> > +{
> > + struct tusb1210 *tusb = phy_get_drvdata(phy);
> > +
> > + gpiod_set_value_cansleep(tusb->gpio_reset, 1);
> > + gpiod_set_value_cansleep(tusb->gpio_cs, 1);
> > +
> > + /* Restore eye diagram optimisation value */
> > + ulpi_write(tusb->ulpi, TUSB1210_VENDOR_SPECIFIC2,
> > + tusb->eye_diagram_tuning);
>
> After you power on phy, ulpi bus may not be available right away. In
> intel case, phy power on happens during dwc3 power on. ULPI bus is not
> available until OTG controller and phy are in sync.
>
> In resume, you can't restore eye diagram from here.
I'm sorry but I don't think I understand? Where do we power on the phy
before dwc3 is powered on? Or is this a Baytrail-CR specific problem?
I can't see any problems with the hardware I have.
In any case, this sounds like purely dwc3 issue and not tusb1210
issue.
> > +static int tusb1210_probe(struct ulpi *ulpi)
> > +{
> > + struct gpio_desc *gpio;
> > + struct tusb1210 *tusb;
> > + int ret;
> > +
> > + tusb = devm_kzalloc(&ulpi->dev, sizeof(*tusb), GFP_KERNEL);
> > + if (!tusb)
> > + return -ENOMEM;
> > +
> > + gpio = devm_gpiod_get(&ulpi->dev, "reset");
> > + if (!IS_ERR(gpio)) {
> > + ret = gpiod_direction_output(gpio, 0);
> > + if (ret)
> > + return ret;
> > + tusb->gpio_reset = gpio;
> > + }
>
> You cannot proceed with probe if gpio reset is not available. Different
> from CS, it's a mandatory pin to toggle in order to power on/off phy and
> get it in sync with OTG controller.
Well, let's check -ENOENT and -ENODEV return values separately. The
reset pin is not used on all platforms so getting this gpio is
optional. This is the case even with some Intel's platforms using
TUSB1210.
> > +
> > + gpio = devm_gpiod_get(&ulpi->dev, "cs");
> > + if (!IS_ERR(gpio)) {
> > + ret = gpiod_direction_output(gpio, 0);
> > + if (ret)
> > + return ret;
> > + tusb->gpio_cs = gpio;
> > + }
> > +
> > + /* Store initial eye diagram optimisation value */
> > + ret = ulpi_read(ulpi, TUSB1210_VENDOR_SPECIFIC2);
>
> It's unclear if ulpi is accessible at this point. You can't read it at
> this point.
We wouldn't have reached this point if ulpi wasn't accessible.
Registering the ulpi interface would have already failed so no driver
would have been probed.
Thanks!
--
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
