From patchwork Mon Jun 27 14:34:22 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Heikki Krogerus X-Patchwork-Id: 9200729 X-Patchwork-Delegate: agross@codeaurora.org Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 897E960757 for ; Mon, 27 Jun 2016 14:35:35 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 78BC328592 for ; Mon, 27 Jun 2016 14:35:35 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6BDFD28581; Mon, 27 Jun 2016 14:35:35 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0FF2E28581 for ; Mon, 27 Jun 2016 14:35:35 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751850AbcF0OfE (ORCPT ); Mon, 27 Jun 2016 10:35:04 -0400 Received: from mga02.intel.com ([134.134.136.20]:31396 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751655AbcF0OfD (ORCPT ); Mon, 27 Jun 2016 10:35:03 -0400 Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga101.jf.intel.com with ESMTP; 27 Jun 2016 07:34:28 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,537,1459839600"; d="scan'208";a="129258190" Received: from kuha.fi.intel.com ([10.237.72.189]) by fmsmga004.fm.intel.com with SMTP; 27 Jun 2016 07:34:23 -0700 Received: by kuha.fi.intel.com (sSMTP sendmail emulation); Mon, 27 Jun 2016 17:34:22 +0300 Date: Mon, 27 Jun 2016 17:34:22 +0300 From: Heikki Krogerus To: Stephen Boyd Cc: linux-usb@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org, Andy Gross , Bjorn Andersson , Neil Armstrong , Arnd Bergmann , Felipe Balbi , Greg Kroah-Hartman , devicetree@vger.kernel.org, Rob Herring Subject: Re: [PATCH 02/21] usb: ulpi: Support device discovery via DT Message-ID: <20160627143422.GB21398@kuha.fi.intel.com> References: <20160626072838.28082-1-stephen.boyd@linaro.org> <20160626072838.28082-3-stephen.boyd@linaro.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20160626072838.28082-3-stephen.boyd@linaro.org> User-Agent: Mutt/1.6.1 (2016-04-27) Sender: linux-arm-msm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi, I'm fine with most of the patch, except.. On Sun, Jun 26, 2016 at 12:28:19AM -0700, Stephen Boyd wrote: > @@ -39,7 +42,10 @@ static int ulpi_match(struct device *dev, struct device_driver *driver) > struct ulpi *ulpi = to_ulpi_dev(dev); > const struct ulpi_device_id *id; > > - for (id = drv->id_table; id->vendor; id++) > + if (of_driver_match_device(dev, driver)) > + return 1; I don't like this part. We should match separately like that only if the bus does not support native enumeration, and of course ULPI with its vendor and product IDs does. There really should always be IDs to match with here. So exceptions have to be solved before we attempt matching. Since we also have to support platforms where the PHY is initially powered off and reading the IDs from the registers is not possible because of that, I think we should consider getting the product and vendor IDs optionally from device properties. Something like this: That should cover both cases. You would just have to create the IDs yourself in this case. Thanks, diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c index 01c0c04..6228a85 100644 --- a/drivers/usb/common/ulpi.c +++ b/drivers/usb/common/ulpi.c @@ -152,7 +152,7 @@ EXPORT_SYMBOL_GPL(ulpi_unregister_driver); /* -------------------------------------------------------------------------- */ -static int ulpi_register(struct device *dev, struct ulpi *ulpi) +static int ulpi_read_id(struct ulpi *ulpi) { int ret; @@ -174,6 +174,21 @@ static int ulpi_register(struct device *dev, struct ulpi *ulpi) ulpi->id.product = ulpi_read(ulpi, ULPI_PRODUCT_ID_LOW); ulpi->id.product |= ulpi_read(ulpi, ULPI_PRODUCT_ID_HIGH) << 8; + return 0; +} + +static int ulpi_register(struct device *dev, struct ulpi *ulpi) +{ + int ret; + + ret = device_property_read_u16(dev, "ulpi-vendor", &ulpi->id.vendor); + ret |= device_property_read_u16(dev, "ulpi-product", &ulpi->id.product); + if (ret) { + ret = ulpi_read_id(ulpi); + if (ret) + return ret; + } + ulpi->dev.parent = dev; ulpi->dev.bus = &ulpi_bus; ulpi->dev.type = &ulpi_dev_type;