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: 9200733 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 9FCEB60757 for ; Mon, 27 Jun 2016 14:36:42 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 8FEE92859C for ; Mon, 27 Jun 2016 14:36:42 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 84CF2285A0; Mon, 27 Jun 2016 14:36:42 +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=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=unavailable version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 3BADD2859C for ; Mon, 27 Jun 2016 14:36:42 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.85_2 #1 (Red Hat Linux)) id 1bHXd3-0007ID-PG; Mon, 27 Jun 2016 14:35:13 +0000 Received: from mga09.intel.com ([134.134.136.24]) by bombadil.infradead.org with esmtp (Exim 4.85_2 #1 (Red Hat Linux)) id 1bHXd1-0005zm-7y for linux-arm-kernel@lists.infradead.org; Mon, 27 Jun 2016 14:35:11 +0000 Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga102.jf.intel.com with ESMTP; 27 Jun 2016 07:34:27 -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 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) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20160627_073511_361358_5D3D27D5 X-CRM114-Status: GOOD ( 16.30 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Felipe Balbi , Arnd Bergmann , Neil Armstrong , linux-arm-msm@vger.kernel.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, Bjorn Andersson , devicetree@vger.kernel.org, Rob Herring , Greg Kroah-Hartman , Andy Gross , linux-arm-kernel@lists.infradead.org Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.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;