From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752456AbcGLIdG (ORCPT ); Tue, 12 Jul 2016 04:33:06 -0400 Received: from mail-eopbgr20040.outbound.protection.outlook.com ([40.107.2.40]:11136 "EHLO EUR02-VE1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752147AbcGLIdA convert rfc822-to-8bit (ORCPT ); Tue, 12 Jul 2016 04:33:00 -0400 X-Greylist: delayed 16410 seconds by postgrey-1.27 at vger.kernel.org; Tue, 12 Jul 2016 04:32:59 EDT From: Rajesh Bhagat To: Peter Chen CC: "linux-usb@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "devicetree@vger.kernel.org" , Peter Chen , "gregkh@linuxfoundation.org" , "kishon@ti.com" , "robh+dt@kernel.org" , "shawnguo@kernel.org" , "linux-arm-kernel@lists.infradead.org" Subject: RE: [PATCH v2 1/5] drivers: usb: chipidea: Add qoriq platform driver Thread-Topic: [PATCH v2 1/5] drivers: usb: chipidea: Add qoriq platform driver Thread-Index: AQHR2Zq1MDrZgQu1fEO30WQbkQFdrqASy/4AgAFX6EA= Date: Tue, 12 Jul 2016 03:59:07 +0000 Message-ID: References: <1468038656-10345-1-git-send-email-rajesh.bhagat@nxp.com> <1468038656-10345-2-git-send-email-rajesh.bhagat@nxp.com> <20160711064358.GG31647@shlinux2> In-Reply-To: <20160711064358.GG31647@shlinux2> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=rajesh.bhagat@nxp.com; x-originating-ip: [192.88.169.1] x-ms-office365-filtering-correlation-id: 6491ed42-b9e4-493c-f7dd-08d3aa08dfb9 x-microsoft-exchange-diagnostics: 1;HE1PR0401MB2330;6:R1/+2nb7biO8tdeBbj20u9TnaXVcg/IIxLhYjQhUjSBtcSb3AQyEjBQZhaQeJ5R2xP/XSqmZzBtt6oPJ2RFtEUkrbnjLKbigOHgZdk4aYIz7JTszy/CxqPdvGD4rvzS1lSbU5F06WxV+DZFp5k8cM79sCxbQNVYS/iJuX8s9ZOqgaRbr9UAm+mT7iAdvMSH6h9Gw81/AeT+8siqxxgKCK57SkjhScPe4NIAbLiuk39iXFYUXjtWh8f96XlUvQ/QgXF4dK1XqaWF08XTIkrBkEbR5LrKKHfBJYqrh6+pkfE29LlbkNpwzDA0sLp/yJjtP0y3EkTTugQJdscESqePk5g==;5:7spXNPFqp/fhX5o6GO6tmfSY3+DTcpXwP+TYvSDfcaZcN0GS83Ws/w8jMb1eOzmucK1k5Sv9O3+CwjUVa/M1vKDtsgSG/p8Rply7kx575fIgNz33O3xkbxdNnU+b5FsRIZwBpSzKZ/+eizs4zQYypA==;24:6a+oHlYL2hphGvQi/HG+QJuD+Ll88Zpa7Mc2rpB4ddw1Iam6vIu2xsMUOzzClIS4ESh7riy9HiP79cxIUextovXpiEAQOiJ+MdAfnpdNBG4=;7:tlpdn7tZC/MqLyE4WRtNeZYnXroNvlsz82SuTYgsuTBRWR2NyV8+OxS3qG7adhKhKoWQbw4nCOmi4DbEJtlyAOlUzjIv7AWKiM91Aun1ZpfhVoF8lnSmI8Voxu5K+y/j+wnkoRWWYa/rj5+1oIroeAYJaaML3mZHgVXDXKHjv2uDACH55ZYO5bUvnuaZNh9YjmhhUFhlXsMCYPTfuumS6nnM8v5WkaPoqHz14stZ6UBzbbDx1ePfu2z4/uMI2R+L x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:HE1PR0401MB2330; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(31051911155226)(9452136761055)(185117386973197)(258649278758335); x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(601004)(2401047)(5005006)(8121501046)(3002001)(10201501046)(6055026);SRVR:HE1PR0401MB2330;BCL:0;PCL:0;RULEID:;SRVR:HE1PR0401MB2330; x-forefront-prvs: 0001227049 x-forefront-antispam-report: SFV:NSPM;SFS:(10009020)(6009001)(7916002)(13464003)(24454002)(189002)(199003)(377454003)(43544003)(86362001)(68736007)(77096005)(3280700002)(2906002)(97736004)(76576001)(105586002)(575784001)(106356001)(2900100001)(3846002)(110136002)(33656002)(106116001)(4326007)(2950100001)(92566002)(7696003)(7846002)(19580405001)(305945005)(5003600100003)(5002640100001)(10400500002)(9686002)(102836003)(7736002)(6116002)(74316002)(3660700001)(8936002)(189998001)(586003)(122556002)(19580395003)(1411001)(87936001)(50986999)(76176999)(54356999)(66066001)(101416001)(8676002)(81166006)(81156014)(2004002);DIR:OUT;SFP:1101;SCL:1;SRVR:HE1PR0401MB2330;H:HE1PR0401MB2331.eurprd04.prod.outlook.com;FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;LANG:en; spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 X-OriginatorOrg: nxp.com X-MS-Exchange-CrossTenant-originalarrivaltime: 12 Jul 2016 03:59:07.7677 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635 X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1PR0401MB2330 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > -----Original Message----- > From: Peter Chen [mailto:hzpeterchen@gmail.com] > Sent: Monday, July 11, 2016 12:14 PM > To: Rajesh Bhagat > Cc: linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org; > devicetree@vger.kernel.org; Peter Chen ; > gregkh@linuxfoundation.org; kishon@ti.com; robh+dt@kernel.org; > shawnguo@kernel.org; linux-arm-kernel@lists.infradead.org > Subject: Re: [PATCH v2 1/5] drivers: usb: chipidea: Add qoriq platform driver > > On Sat, Jul 09, 2016 at 10:00:52AM +0530, Rajesh Bhagat wrote: > > Adds qoriq platform driver for chipidea controller, verfied on LS1021A > > and LS1012A platforms. > > > > Signed-off-by: Rajesh Bhagat > > --- > > Changes in v2: > > - Replaced Freescale with QorIQ in comments section > > - Added macros to remove hardcoding while programming registers > > - Changed the compatible string to fsl,ci-qoriq-usb2 and added > > version > > - Removed calls to devm free/release calls > > > > drivers/usb/chipidea/Makefile | 2 + > > drivers/usb/chipidea/ci_hdrc_qoriq.c | 237 > > +++++++++++++++++++++++++++++++++++ > > drivers/usb/chipidea/ci_hdrc_qoriq.h | 65 ++++++++++ > > 3 files changed, 304 insertions(+) > > create mode 100644 drivers/usb/chipidea/ci_hdrc_qoriq.c > > create mode 100644 drivers/usb/chipidea/ci_hdrc_qoriq.h > > > > diff --git a/drivers/usb/chipidea/Makefile > > b/drivers/usb/chipidea/Makefile index 518e445..3122b86b 100644 > > --- a/drivers/usb/chipidea/Makefile > > +++ b/drivers/usb/chipidea/Makefile > > @@ -14,3 +14,5 @@ obj-$(CONFIG_USB_CHIPIDEA) += ci_hdrc_zevio.o > > obj-$(CONFIG_USB_CHIPIDEA_PCI) += ci_hdrc_pci.o > > > > obj-$(CONFIG_USB_CHIPIDEA_OF) += usbmisc_imx.o ci_hdrc_imx.o > > + > > +obj-$(CONFIG_USB_CHIPIDEA) += ci_hdrc_qoriq.o > > diff --git a/drivers/usb/chipidea/ci_hdrc_qoriq.c > > b/drivers/usb/chipidea/ci_hdrc_qoriq.c > > new file mode 100644 > > index 0000000..3f478c6 > > --- /dev/null > > +++ b/drivers/usb/chipidea/ci_hdrc_qoriq.c > > @@ -0,0 +1,237 @@ > > +/* > > + * QorIQ SoC USB 2.0 Controller driver > > + * > > + * Copyright 2016 Freescale Semiconductor, Inc. > > + * Author: Rajesh Bhagat > > + * > > + * This program is free software; you can redistribute it and/or > > +modify > > + * it under the terms of the GNU General Public License version 2 as > > + * published by the Free Software Foundation. > > + * > > + */ > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#include "ci.h" > > +#include "ci_hdrc_qoriq.h" > > + > > +struct ci_hdrc_qoriq_data { > > + struct phy *phy; > > + struct clk *clk; > > + void __iomem *qoriq_regs; > > + struct platform_device *ci_pdev; > > + enum usb_phy_interface phy_mode; > > +}; > > + > > +/* > > + * clock helper functions > > + */ > > +static int ci_hdrc_qoriq_get_clks(struct platform_device *pdev) { > > + int ret; > > + struct device *dev = &pdev->dev; > > + struct ci_hdrc_qoriq_data *data = platform_get_drvdata(pdev); > > + > > + data->clk = devm_clk_get(dev, "usb2-clock"); > > + if (IS_ERR(data->clk)) { > > + dev_err(dev, "failed to get clk, err=%ld\n", > > + PTR_ERR(data->clk)); > > + return ret; Hello Peter, > > return PTR_ERR(data->clk), and delete ret. > Will take care in v3. > > + } > > + return 0; > > +} > > + > > +static int ci_hdrc_qoriq_prepare_enable_clks(struct platform_device > > +*pdev) { > > + int ret; > > + struct device *dev = &pdev->dev; > > + struct ci_hdrc_qoriq_data *data = platform_get_drvdata(pdev); > > + > > + ret = clk_prepare_enable(data->clk); > > + if (ret) { > > + dev_err(dev, "failed to prepare/enable clk, err=%d\n", ret); > > + return ret; > > + } > > + return 0; > > +} > > + > > +static void ci_hdrc_qoriq_disable_unprepare_clks(struct > > +platform_device *pdev) { > > + struct ci_hdrc_qoriq_data *data = platform_get_drvdata(pdev); > > + > > + clk_disable_unprepare(data->clk); > > +} > > + > > +static int ci_hdrc_qoriq_usb_setup(struct platform_device *pdev) { > > + u32 reg; > > + struct resource *res; > > + struct device *dev = &pdev->dev; > > + struct ci_hdrc_qoriq_data *data = platform_get_drvdata(pdev); > > + > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + if (!res) { > > + dev_err(dev, "failed to get I/O memory\n"); > > + return -ENOENT; > > + } > > + > > + dev_dbg(dev, "res->start %llx, resource_size(res) %llx\n", res->start, > > + resource_size(res)); > > Delete above debug message please. > Will take care in v3. > > + data->qoriq_regs = devm_ioremap(dev, res->start, resource_size(res)); > > + if (IS_ERR(data->qoriq_regs)) { > > + dev_err(dev, "failed to remap I/O memory\n"); > > + return -ENOMEM; > > + } > > + > > This mapping will be freed soon, using ioremap instead. > Correct. It is not required afterwards. Will use ioremap instead of devm_ioremap in v3. > > + data->phy_mode = of_usb_get_phy_mode(pdev->dev.of_node); > > + dev_dbg(dev, "phy_mode %d\n", data->phy_mode); > > Delete above debug message please. > Will take care in v3. > > + > > + reg = ioread32be(data->qoriq_regs + QORIQ_SOC_USB_CTRL); > > + switch (data->phy_mode) { > > + case USBPHY_INTERFACE_MODE_ULPI: > > + iowrite32be(reg | ~UTMI_PHY_EN, > > + data->qoriq_regs + QORIQ_SOC_USB_CTRL); > > + reg = ioread32be(data->qoriq_regs + QORIQ_SOC_USB_CTRL); > > + iowrite32be(reg | USB_CTRL_USB_EN, > > + data->qoriq_regs + QORIQ_SOC_USB_CTRL); > > + break; > > + default: > > + dev_err(dev, "unsupported phy_mode %d\n", data->phy_mode); > > + return -EINVAL; > > + } > > + > > + /* Setup Snooping for all the 4GB space */ > > + /* SNOOP1 starts from 0x0, size 2G */ > > + iowrite32be(SNOOP_SIZE_2GB, data->qoriq_regs + > QORIQ_SOC_USB_SNOOP1); > > + /* SNOOP2 starts from 0x80000000, size 2G */ > > + iowrite32be(SNOOP_SIZE_2GB | 0x80000000, > > + data->qoriq_regs + QORIQ_SOC_USB_SNOOP2); > > What does this snoop mean? > Snoop here refers to the cache coherency capability of Soc. > > + > > + iowrite32be(PRICTRL_PRI_LVL, data->qoriq_regs + > QORIQ_SOC_USB_PRICTRL); > > + iowrite32be(AGECNTTHRSH_THRESHOLD, data->qoriq_regs + > > + QORIQ_SOC_USB_AGECNTTHRSH); > > + iowrite32be(SICTRL_RD_PREFETCH_32_BYTE, data->qoriq_regs + > > + QORIQ_SOC_USB_SICTRL); > > + > > + devm_iounmap(dev, data->qoriq_regs); > > iounmap. Will take care in v3. > > > + return 0; > > +} > > + > > +static int ci_hdrc_qoriq_probe(struct platform_device *pdev) { > > + int ret; > > + struct device *dev = &pdev->dev; > > + struct ci_hdrc_qoriq_data *data; > > + struct ci_hdrc_platform_data pdata = { > > + .name = dev_name(dev), > > + .capoffset = DEF_CAPOFFSET, > > + .flags = CI_HDRC_DISABLE_STREAMING, > > + }; > > + > > + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); > > + if (!data) > > + return -ENOMEM; > > + > > + platform_set_drvdata(pdev, data); > > + > > + ret = ci_hdrc_qoriq_get_clks(pdev); > > + if (ret) > > + goto err_out; > > + > > + ret = ci_hdrc_qoriq_prepare_enable_clks(pdev); > > + if (ret) > > + goto err_out; > > Why not return directly? > Okay. Will return directly from here in v3. > > + > > + ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32)); > > + if (ret) { > > + dev_err(dev, "failed to set coherent dma mask, err=%d\n", ret); > > + goto err_clks; > > + } > > This dma setting does not be needed, the device tree will set it when the device is > created. > Okay, will drop dma_coerce_mask_and_coherent usage in v3. > > + > > + ret = ci_hdrc_qoriq_usb_setup(pdev); > > + if (ret) { > > + dev_err(dev, "failed to perform qoriq_usb2 setup, err=%d\n", > > + ret); > > + goto err_clks; > > + } > > + > > + data->phy = devm_phy_get(dev, "usb2-phy"); > > + if (IS_ERR(data->phy)) { > > + ret = PTR_ERR(data->phy); > > + /* Return -EINVAL if no usbphy is available */ > > + if (ret == -ENODEV) > > + ret = -EINVAL; > > + dev_err(dev, "failed get phy device, err=%d\n", ret); > > + goto err_clks; > > + } > > + pdata.phy = data->phy; > > The chipidea core will do that. > Okay, I checked now drivers/usb/chipidea/core.c is calling devm_phy_get with "usb-phy", will remove above code and rename from "usb2-phy" to "usb-phy" in dts in v3. > > + > > + data->ci_pdev = ci_hdrc_add_device(dev, > > + pdev->resource, pdev->num_resources, > > + &pdata); > > + if (IS_ERR(data->ci_pdev)) { > > + ret = PTR_ERR(data->ci_pdev); > > + dev_err(dev, > > + "failed to register ci_hdrc platform device, err=%d\n", > > + ret); > > + goto err_clks; > > + } > > + > > + pm_runtime_no_callbacks(dev); > > + pm_runtime_enable(dev); > > + > > + dev_dbg(dev, "initialized\n"); > > + return 0; > > + > > +err_clks: > > + ci_hdrc_qoriq_disable_unprepare_clks(pdev); > > If you have only one clock, it is unnecessary to use dedicated APIs for clock operation. > We do have multiple clocks, but currently one is integrated in code. Hence created the APIs for future use. > > +err_out: > > + return ret; > > +} > > + > > +static int ci_hdrc_qoriq_remove(struct platform_device *pdev) { > > + struct device *dev = &pdev->dev; > > + struct ci_hdrc_qoriq_data *data = platform_get_drvdata(pdev); > > + > > + pm_runtime_disable(dev); > > + ci_hdrc_remove_device(data->ci_pdev); > > + ci_hdrc_qoriq_disable_unprepare_clks(pdev); > > + dev_dbg(dev, "de-initialized\n"); > > + return 0; > > +} > > + > > +static void ci_hdrc_qoriq_shutdown(struct platform_device *pdev) { > > + ci_hdrc_qoriq_remove(pdev); > > +} > > + > > +static const struct of_device_id ci_hdrc_qoriq_dt_ids[] = { > > + { .compatible = "fsl,ci-qoriq-usb2"}, > > + { /* sentinel */ } > > +}; > > +MODULE_DEVICE_TABLE(of, ci_hdrc_qoriq_dt_ids); > > + > > +static struct platform_driver ci_hdrc_qoriq_driver = { > > + .probe = ci_hdrc_qoriq_probe, > > + .remove = ci_hdrc_qoriq_remove, > > + .shutdown = ci_hdrc_qoriq_shutdown, > > + .driver = { > > + .name = "ci_qoriq_usb2", > > + .of_match_table = ci_hdrc_qoriq_dt_ids, > > + }, > > +}; > > + > > +module_platform_driver(ci_hdrc_qoriq_driver); > > + > > +MODULE_ALIAS("platform:ci-qoriq-usb2"); > > +MODULE_LICENSE("GPL v2"); > > +MODULE_DESCRIPTION("CI HDRC QORIQ USB binding"); > > +MODULE_AUTHOR("Rajesh Bhagat "); > > diff --git a/drivers/usb/chipidea/ci_hdrc_qoriq.h > > b/drivers/usb/chipidea/ci_hdrc_qoriq.h > > new file mode 100644 > > index 0000000..afd29442 > > --- /dev/null > > +++ b/drivers/usb/chipidea/ci_hdrc_qoriq.h > > @@ -0,0 +1,65 @@ > > +/* > > + * QorIQ SoC USB 2.0 Controller driver > > + * > > + * Copyright 2016 Freescale Semiconductor, Inc. > > + * Author: Rajesh Bhagat > > + * > > + * This program is free software; you can redistribute it and/or > > +modify > > + * it under the terms of the GNU General Public License version 2 as > > + * published by the Free Software Foundation. > > + * > > + */ > > +#ifndef __DRIVER_USB_CHIPIDEA_CI_HDRC_QORIQ_H > > +#define __DRIVER_USB_CHIPIDEA_CI_HDRC_QORIQ_H > > + > > +/* offsets for the non-ehci registers in the QORIQ SOC USB controller */ > > +#define QORIQ_SOC_USB_SBUSCFG 0x90 > > +#define SBUSCFG_INCR8 0x02 /* INCR8, specified */ > > +#define QORIQ_SOC_USB_ULPIVP 0x170 > > +#define QORIQ_SOC_USB_PORTSC1 0x184 > > +#define PORT_PTS_MSK (3<<30) > > +#define PORT_PTS_UTMI (0<<30) > > +#define PORT_PTS_ULPI (2<<30) > > +#define PORT_PTS_SERIAL (3<<30) > > +#define PORT_PTS_PTW (1<<28) > > +#define QORIQ_SOC_USB_PORTSC2 0x188 > > +#define QORIQ_SOC_USB_USBMODE 0x1a8 > > +#define USBMODE_CM_MASK (3 << 0) /* controller mode mask > */ > > +#define USBMODE_CM_HOST (3 << 0) /* controller mode: host */ > > +#define USBMODE_ES (1 << 2) /* (Big) Endian Select */ > > + > > +#define QORIQ_SOC_USB_USBGENCTRL 0x200 > > +#define USBGENCTRL_PPP (1 << 3) > > +#define USBGENCTRL_PFP (1 << 2) > > +#define QORIQ_SOC_USB_ISIPHYCTRL 0x204 > > +#define ISIPHYCTRL_PXE (1) > > +#define ISIPHYCTRL_PHYE (1 << 4) > > + > > +#define QORIQ_SOC_USB_SNOOP1 0x400 /* NOTE: big-endian */ > > +#define QORIQ_SOC_USB_SNOOP2 0x404 /* NOTE: big-endian */ > > +#define QORIQ_SOC_USB_AGECNTTHRSH 0x408 /* NOTE: big-endian */ > > +#define AGECNTTHRSH_THRESHOLD 0x40 > > +#define QORIQ_SOC_USB_PRICTRL 0x40c /* NOTE: big-endian */ > > +#define PRICTRL_PRI_LVL 0xc > > +#define QORIQ_SOC_USB_SICTRL 0x410 /* NOTE: big-endian */ > > +#define SICTRL_RD_PREFETCH_32_BYTE (0x1) > > +#define SICTRL_RD_PREFETCH_64_BYTE (0x0) > > +#define QORIQ_SOC_USB_CTRL 0x500 /* NOTE: big-endian */ > > +#define CTRL_UTMI_PHY_EN (1 << 9) > > +#define CTRL_PHY_CLK_VALID (1 << 17) > > +#define SNOOP_SIZE_2GB 0x1e > > + > > +/* control Register Bit Masks */ > > +#define CONTROL_REGISTER_W1C_MASK 0x00020000 /* W1C: > PHY_CLK_VALID */ > > +#define ULPI_INT_EN (1<<0) > > +#define WU_INT_EN (1<<1) > > +#define USB_CTRL_USB_EN (1<<2) > > +#define LINE_STATE_FILTER__EN (1<<3) > > +#define KEEP_OTG_ON (1<<4) > > +#define OTG_PORT (1<<5) > > +#define PLL_RESET (1<<8) > > +#define UTMI_PHY_EN (1<<9) > > +#define ULPI_PHY_CLK_SEL (1<<10) > > +#define PHY_CLK_VALID (1<<17) > > + > > Move necessary definitions to source file, and delete this header file please, this > header file is only used by ci_hdrc_qoriq.c. > Okay, will take care in v3. Best Regards, Rajesh Bhagat > -- > > Best Regards, > Peter Chen From mboxrd@z Thu Jan 1 00:00:00 1970 From: Rajesh Bhagat Subject: RE: [PATCH v2 1/5] drivers: usb: chipidea: Add qoriq platform driver Date: Tue, 12 Jul 2016 03:59:07 +0000 Message-ID: References: <1468038656-10345-1-git-send-email-rajesh.bhagat@nxp.com> <1468038656-10345-2-git-send-email-rajesh.bhagat@nxp.com> <20160711064358.GG31647@shlinux2> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20160711064358.GG31647@shlinux2> Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Peter Chen Cc: "devicetree@vger.kernel.org" , Peter Chen , "gregkh@linuxfoundation.org" , "linux-usb@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "kishon@ti.com" , "robh+dt@kernel.org" , "shawnguo@kernel.org" , "linux-arm-kernel@lists.infradead.org" List-Id: devicetree@vger.kernel.org > -----Original Message----- > From: Peter Chen [mailto:hzpeterchen@gmail.com] > Sent: Monday, July 11, 2016 12:14 PM > To: Rajesh Bhagat > Cc: linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org; > devicetree@vger.kernel.org; Peter Chen ; > gregkh@linuxfoundation.org; kishon@ti.com; robh+dt@kernel.org; > shawnguo@kernel.org; linux-arm-kernel@lists.infradead.org > Subject: Re: [PATCH v2 1/5] drivers: usb: chipidea: Add qoriq platform driver > > On Sat, Jul 09, 2016 at 10:00:52AM +0530, Rajesh Bhagat wrote: > > Adds qoriq platform driver for chipidea controller, verfied on LS1021A > > and LS1012A platforms. > > > > Signed-off-by: Rajesh Bhagat > > --- > > Changes in v2: > > - Replaced Freescale with QorIQ in comments section > > - Added macros to remove hardcoding while programming registers > > - Changed the compatible string to fsl,ci-qoriq-usb2 and added > > version > > - Removed calls to devm free/release calls > > > > drivers/usb/chipidea/Makefile | 2 + > > drivers/usb/chipidea/ci_hdrc_qoriq.c | 237 > > +++++++++++++++++++++++++++++++++++ > > drivers/usb/chipidea/ci_hdrc_qoriq.h | 65 ++++++++++ > > 3 files changed, 304 insertions(+) > > create mode 100644 drivers/usb/chipidea/ci_hdrc_qoriq.c > > create mode 100644 drivers/usb/chipidea/ci_hdrc_qoriq.h > > > > diff --git a/drivers/usb/chipidea/Makefile > > b/drivers/usb/chipidea/Makefile index 518e445..3122b86b 100644 > > --- a/drivers/usb/chipidea/Makefile > > +++ b/drivers/usb/chipidea/Makefile > > @@ -14,3 +14,5 @@ obj-$(CONFIG_USB_CHIPIDEA) += ci_hdrc_zevio.o > > obj-$(CONFIG_USB_CHIPIDEA_PCI) += ci_hdrc_pci.o > > > > obj-$(CONFIG_USB_CHIPIDEA_OF) += usbmisc_imx.o ci_hdrc_imx.o > > + > > +obj-$(CONFIG_USB_CHIPIDEA) += ci_hdrc_qoriq.o > > diff --git a/drivers/usb/chipidea/ci_hdrc_qoriq.c > > b/drivers/usb/chipidea/ci_hdrc_qoriq.c > > new file mode 100644 > > index 0000000..3f478c6 > > --- /dev/null > > +++ b/drivers/usb/chipidea/ci_hdrc_qoriq.c > > @@ -0,0 +1,237 @@ > > +/* > > + * QorIQ SoC USB 2.0 Controller driver > > + * > > + * Copyright 2016 Freescale Semiconductor, Inc. > > + * Author: Rajesh Bhagat > > + * > > + * This program is free software; you can redistribute it and/or > > +modify > > + * it under the terms of the GNU General Public License version 2 as > > + * published by the Free Software Foundation. > > + * > > + */ > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#include "ci.h" > > +#include "ci_hdrc_qoriq.h" > > + > > +struct ci_hdrc_qoriq_data { > > + struct phy *phy; > > + struct clk *clk; > > + void __iomem *qoriq_regs; > > + struct platform_device *ci_pdev; > > + enum usb_phy_interface phy_mode; > > +}; > > + > > +/* > > + * clock helper functions > > + */ > > +static int ci_hdrc_qoriq_get_clks(struct platform_device *pdev) { > > + int ret; > > + struct device *dev = &pdev->dev; > > + struct ci_hdrc_qoriq_data *data = platform_get_drvdata(pdev); > > + > > + data->clk = devm_clk_get(dev, "usb2-clock"); > > + if (IS_ERR(data->clk)) { > > + dev_err(dev, "failed to get clk, err=%ld\n", > > + PTR_ERR(data->clk)); > > + return ret; Hello Peter, > > return PTR_ERR(data->clk), and delete ret. > Will take care in v3. > > + } > > + return 0; > > +} > > + > > +static int ci_hdrc_qoriq_prepare_enable_clks(struct platform_device > > +*pdev) { > > + int ret; > > + struct device *dev = &pdev->dev; > > + struct ci_hdrc_qoriq_data *data = platform_get_drvdata(pdev); > > + > > + ret = clk_prepare_enable(data->clk); > > + if (ret) { > > + dev_err(dev, "failed to prepare/enable clk, err=%d\n", ret); > > + return ret; > > + } > > + return 0; > > +} > > + > > +static void ci_hdrc_qoriq_disable_unprepare_clks(struct > > +platform_device *pdev) { > > + struct ci_hdrc_qoriq_data *data = platform_get_drvdata(pdev); > > + > > + clk_disable_unprepare(data->clk); > > +} > > + > > +static int ci_hdrc_qoriq_usb_setup(struct platform_device *pdev) { > > + u32 reg; > > + struct resource *res; > > + struct device *dev = &pdev->dev; > > + struct ci_hdrc_qoriq_data *data = platform_get_drvdata(pdev); > > + > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + if (!res) { > > + dev_err(dev, "failed to get I/O memory\n"); > > + return -ENOENT; > > + } > > + > > + dev_dbg(dev, "res->start %llx, resource_size(res) %llx\n", res->start, > > + resource_size(res)); > > Delete above debug message please. > Will take care in v3. > > + data->qoriq_regs = devm_ioremap(dev, res->start, resource_size(res)); > > + if (IS_ERR(data->qoriq_regs)) { > > + dev_err(dev, "failed to remap I/O memory\n"); > > + return -ENOMEM; > > + } > > + > > This mapping will be freed soon, using ioremap instead. > Correct. It is not required afterwards. Will use ioremap instead of devm_ioremap in v3. > > + data->phy_mode = of_usb_get_phy_mode(pdev->dev.of_node); > > + dev_dbg(dev, "phy_mode %d\n", data->phy_mode); > > Delete above debug message please. > Will take care in v3. > > + > > + reg = ioread32be(data->qoriq_regs + QORIQ_SOC_USB_CTRL); > > + switch (data->phy_mode) { > > + case USBPHY_INTERFACE_MODE_ULPI: > > + iowrite32be(reg | ~UTMI_PHY_EN, > > + data->qoriq_regs + QORIQ_SOC_USB_CTRL); > > + reg = ioread32be(data->qoriq_regs + QORIQ_SOC_USB_CTRL); > > + iowrite32be(reg | USB_CTRL_USB_EN, > > + data->qoriq_regs + QORIQ_SOC_USB_CTRL); > > + break; > > + default: > > + dev_err(dev, "unsupported phy_mode %d\n", data->phy_mode); > > + return -EINVAL; > > + } > > + > > + /* Setup Snooping for all the 4GB space */ > > + /* SNOOP1 starts from 0x0, size 2G */ > > + iowrite32be(SNOOP_SIZE_2GB, data->qoriq_regs + > QORIQ_SOC_USB_SNOOP1); > > + /* SNOOP2 starts from 0x80000000, size 2G */ > > + iowrite32be(SNOOP_SIZE_2GB | 0x80000000, > > + data->qoriq_regs + QORIQ_SOC_USB_SNOOP2); > > What does this snoop mean? > Snoop here refers to the cache coherency capability of Soc. > > + > > + iowrite32be(PRICTRL_PRI_LVL, data->qoriq_regs + > QORIQ_SOC_USB_PRICTRL); > > + iowrite32be(AGECNTTHRSH_THRESHOLD, data->qoriq_regs + > > + QORIQ_SOC_USB_AGECNTTHRSH); > > + iowrite32be(SICTRL_RD_PREFETCH_32_BYTE, data->qoriq_regs + > > + QORIQ_SOC_USB_SICTRL); > > + > > + devm_iounmap(dev, data->qoriq_regs); > > iounmap. Will take care in v3. > > > + return 0; > > +} > > + > > +static int ci_hdrc_qoriq_probe(struct platform_device *pdev) { > > + int ret; > > + struct device *dev = &pdev->dev; > > + struct ci_hdrc_qoriq_data *data; > > + struct ci_hdrc_platform_data pdata = { > > + .name = dev_name(dev), > > + .capoffset = DEF_CAPOFFSET, > > + .flags = CI_HDRC_DISABLE_STREAMING, > > + }; > > + > > + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); > > + if (!data) > > + return -ENOMEM; > > + > > + platform_set_drvdata(pdev, data); > > + > > + ret = ci_hdrc_qoriq_get_clks(pdev); > > + if (ret) > > + goto err_out; > > + > > + ret = ci_hdrc_qoriq_prepare_enable_clks(pdev); > > + if (ret) > > + goto err_out; > > Why not return directly? > Okay. Will return directly from here in v3. > > + > > + ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32)); > > + if (ret) { > > + dev_err(dev, "failed to set coherent dma mask, err=%d\n", ret); > > + goto err_clks; > > + } > > This dma setting does not be needed, the device tree will set it when the device is > created. > Okay, will drop dma_coerce_mask_and_coherent usage in v3. > > + > > + ret = ci_hdrc_qoriq_usb_setup(pdev); > > + if (ret) { > > + dev_err(dev, "failed to perform qoriq_usb2 setup, err=%d\n", > > + ret); > > + goto err_clks; > > + } > > + > > + data->phy = devm_phy_get(dev, "usb2-phy"); > > + if (IS_ERR(data->phy)) { > > + ret = PTR_ERR(data->phy); > > + /* Return -EINVAL if no usbphy is available */ > > + if (ret == -ENODEV) > > + ret = -EINVAL; > > + dev_err(dev, "failed get phy device, err=%d\n", ret); > > + goto err_clks; > > + } > > + pdata.phy = data->phy; > > The chipidea core will do that. > Okay, I checked now drivers/usb/chipidea/core.c is calling devm_phy_get with "usb-phy", will remove above code and rename from "usb2-phy" to "usb-phy" in dts in v3. > > + > > + data->ci_pdev = ci_hdrc_add_device(dev, > > + pdev->resource, pdev->num_resources, > > + &pdata); > > + if (IS_ERR(data->ci_pdev)) { > > + ret = PTR_ERR(data->ci_pdev); > > + dev_err(dev, > > + "failed to register ci_hdrc platform device, err=%d\n", > > + ret); > > + goto err_clks; > > + } > > + > > + pm_runtime_no_callbacks(dev); > > + pm_runtime_enable(dev); > > + > > + dev_dbg(dev, "initialized\n"); > > + return 0; > > + > > +err_clks: > > + ci_hdrc_qoriq_disable_unprepare_clks(pdev); > > If you have only one clock, it is unnecessary to use dedicated APIs for clock operation. > We do have multiple clocks, but currently one is integrated in code. Hence created the APIs for future use. > > +err_out: > > + return ret; > > +} > > + > > +static int ci_hdrc_qoriq_remove(struct platform_device *pdev) { > > + struct device *dev = &pdev->dev; > > + struct ci_hdrc_qoriq_data *data = platform_get_drvdata(pdev); > > + > > + pm_runtime_disable(dev); > > + ci_hdrc_remove_device(data->ci_pdev); > > + ci_hdrc_qoriq_disable_unprepare_clks(pdev); > > + dev_dbg(dev, "de-initialized\n"); > > + return 0; > > +} > > + > > +static void ci_hdrc_qoriq_shutdown(struct platform_device *pdev) { > > + ci_hdrc_qoriq_remove(pdev); > > +} > > + > > +static const struct of_device_id ci_hdrc_qoriq_dt_ids[] = { > > + { .compatible = "fsl,ci-qoriq-usb2"}, > > + { /* sentinel */ } > > +}; > > +MODULE_DEVICE_TABLE(of, ci_hdrc_qoriq_dt_ids); > > + > > +static struct platform_driver ci_hdrc_qoriq_driver = { > > + .probe = ci_hdrc_qoriq_probe, > > + .remove = ci_hdrc_qoriq_remove, > > + .shutdown = ci_hdrc_qoriq_shutdown, > > + .driver = { > > + .name = "ci_qoriq_usb2", > > + .of_match_table = ci_hdrc_qoriq_dt_ids, > > + }, > > +}; > > + > > +module_platform_driver(ci_hdrc_qoriq_driver); > > + > > +MODULE_ALIAS("platform:ci-qoriq-usb2"); > > +MODULE_LICENSE("GPL v2"); > > +MODULE_DESCRIPTION("CI HDRC QORIQ USB binding"); > > +MODULE_AUTHOR("Rajesh Bhagat "); > > diff --git a/drivers/usb/chipidea/ci_hdrc_qoriq.h > > b/drivers/usb/chipidea/ci_hdrc_qoriq.h > > new file mode 100644 > > index 0000000..afd29442 > > --- /dev/null > > +++ b/drivers/usb/chipidea/ci_hdrc_qoriq.h > > @@ -0,0 +1,65 @@ > > +/* > > + * QorIQ SoC USB 2.0 Controller driver > > + * > > + * Copyright 2016 Freescale Semiconductor, Inc. > > + * Author: Rajesh Bhagat > > + * > > + * This program is free software; you can redistribute it and/or > > +modify > > + * it under the terms of the GNU General Public License version 2 as > > + * published by the Free Software Foundation. > > + * > > + */ > > +#ifndef __DRIVER_USB_CHIPIDEA_CI_HDRC_QORIQ_H > > +#define __DRIVER_USB_CHIPIDEA_CI_HDRC_QORIQ_H > > + > > +/* offsets for the non-ehci registers in the QORIQ SOC USB controller */ > > +#define QORIQ_SOC_USB_SBUSCFG 0x90 > > +#define SBUSCFG_INCR8 0x02 /* INCR8, specified */ > > +#define QORIQ_SOC_USB_ULPIVP 0x170 > > +#define QORIQ_SOC_USB_PORTSC1 0x184 > > +#define PORT_PTS_MSK (3<<30) > > +#define PORT_PTS_UTMI (0<<30) > > +#define PORT_PTS_ULPI (2<<30) > > +#define PORT_PTS_SERIAL (3<<30) > > +#define PORT_PTS_PTW (1<<28) > > +#define QORIQ_SOC_USB_PORTSC2 0x188 > > +#define QORIQ_SOC_USB_USBMODE 0x1a8 > > +#define USBMODE_CM_MASK (3 << 0) /* controller mode mask > */ > > +#define USBMODE_CM_HOST (3 << 0) /* controller mode: host */ > > +#define USBMODE_ES (1 << 2) /* (Big) Endian Select */ > > + > > +#define QORIQ_SOC_USB_USBGENCTRL 0x200 > > +#define USBGENCTRL_PPP (1 << 3) > > +#define USBGENCTRL_PFP (1 << 2) > > +#define QORIQ_SOC_USB_ISIPHYCTRL 0x204 > > +#define ISIPHYCTRL_PXE (1) > > +#define ISIPHYCTRL_PHYE (1 << 4) > > + > > +#define QORIQ_SOC_USB_SNOOP1 0x400 /* NOTE: big-endian */ > > +#define QORIQ_SOC_USB_SNOOP2 0x404 /* NOTE: big-endian */ > > +#define QORIQ_SOC_USB_AGECNTTHRSH 0x408 /* NOTE: big-endian */ > > +#define AGECNTTHRSH_THRESHOLD 0x40 > > +#define QORIQ_SOC_USB_PRICTRL 0x40c /* NOTE: big-endian */ > > +#define PRICTRL_PRI_LVL 0xc > > +#define QORIQ_SOC_USB_SICTRL 0x410 /* NOTE: big-endian */ > > +#define SICTRL_RD_PREFETCH_32_BYTE (0x1) > > +#define SICTRL_RD_PREFETCH_64_BYTE (0x0) > > +#define QORIQ_SOC_USB_CTRL 0x500 /* NOTE: big-endian */ > > +#define CTRL_UTMI_PHY_EN (1 << 9) > > +#define CTRL_PHY_CLK_VALID (1 << 17) > > +#define SNOOP_SIZE_2GB 0x1e > > + > > +/* control Register Bit Masks */ > > +#define CONTROL_REGISTER_W1C_MASK 0x00020000 /* W1C: > PHY_CLK_VALID */ > > +#define ULPI_INT_EN (1<<0) > > +#define WU_INT_EN (1<<1) > > +#define USB_CTRL_USB_EN (1<<2) > > +#define LINE_STATE_FILTER__EN (1<<3) > > +#define KEEP_OTG_ON (1<<4) > > +#define OTG_PORT (1<<5) > > +#define PLL_RESET (1<<8) > > +#define UTMI_PHY_EN (1<<9) > > +#define ULPI_PHY_CLK_SEL (1<<10) > > +#define PHY_CLK_VALID (1<<17) > > + > > Move necessary definitions to source file, and delete this header file please, this > header file is only used by ci_hdrc_qoriq.c. > Okay, will take care in v3. Best Regards, Rajesh Bhagat > -- > > Best Regards, > Peter Chen From mboxrd@z Thu Jan 1 00:00:00 1970 From: rajesh.bhagat@nxp.com (Rajesh Bhagat) Date: Tue, 12 Jul 2016 03:59:07 +0000 Subject: [PATCH v2 1/5] drivers: usb: chipidea: Add qoriq platform driver In-Reply-To: <20160711064358.GG31647@shlinux2> References: <1468038656-10345-1-git-send-email-rajesh.bhagat@nxp.com> <1468038656-10345-2-git-send-email-rajesh.bhagat@nxp.com> <20160711064358.GG31647@shlinux2> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org > -----Original Message----- > From: Peter Chen [mailto:hzpeterchen at gmail.com] > Sent: Monday, July 11, 2016 12:14 PM > To: Rajesh Bhagat > Cc: linux-usb at vger.kernel.org; linux-kernel at vger.kernel.org; > devicetree at vger.kernel.org; Peter Chen ; > gregkh at linuxfoundation.org; kishon at ti.com; robh+dt at kernel.org; > shawnguo at kernel.org; linux-arm-kernel at lists.infradead.org > Subject: Re: [PATCH v2 1/5] drivers: usb: chipidea: Add qoriq platform driver > > On Sat, Jul 09, 2016 at 10:00:52AM +0530, Rajesh Bhagat wrote: > > Adds qoriq platform driver for chipidea controller, verfied on LS1021A > > and LS1012A platforms. > > > > Signed-off-by: Rajesh Bhagat > > --- > > Changes in v2: > > - Replaced Freescale with QorIQ in comments section > > - Added macros to remove hardcoding while programming registers > > - Changed the compatible string to fsl,ci-qoriq-usb2 and added > > version > > - Removed calls to devm free/release calls > > > > drivers/usb/chipidea/Makefile | 2 + > > drivers/usb/chipidea/ci_hdrc_qoriq.c | 237 > > +++++++++++++++++++++++++++++++++++ > > drivers/usb/chipidea/ci_hdrc_qoriq.h | 65 ++++++++++ > > 3 files changed, 304 insertions(+) > > create mode 100644 drivers/usb/chipidea/ci_hdrc_qoriq.c > > create mode 100644 drivers/usb/chipidea/ci_hdrc_qoriq.h > > > > diff --git a/drivers/usb/chipidea/Makefile > > b/drivers/usb/chipidea/Makefile index 518e445..3122b86b 100644 > > --- a/drivers/usb/chipidea/Makefile > > +++ b/drivers/usb/chipidea/Makefile > > @@ -14,3 +14,5 @@ obj-$(CONFIG_USB_CHIPIDEA) += ci_hdrc_zevio.o > > obj-$(CONFIG_USB_CHIPIDEA_PCI) += ci_hdrc_pci.o > > > > obj-$(CONFIG_USB_CHIPIDEA_OF) += usbmisc_imx.o ci_hdrc_imx.o > > + > > +obj-$(CONFIG_USB_CHIPIDEA) += ci_hdrc_qoriq.o > > diff --git a/drivers/usb/chipidea/ci_hdrc_qoriq.c > > b/drivers/usb/chipidea/ci_hdrc_qoriq.c > > new file mode 100644 > > index 0000000..3f478c6 > > --- /dev/null > > +++ b/drivers/usb/chipidea/ci_hdrc_qoriq.c > > @@ -0,0 +1,237 @@ > > +/* > > + * QorIQ SoC USB 2.0 Controller driver > > + * > > + * Copyright 2016 Freescale Semiconductor, Inc. > > + * Author: Rajesh Bhagat > > + * > > + * This program is free software; you can redistribute it and/or > > +modify > > + * it under the terms of the GNU General Public License version 2 as > > + * published by the Free Software Foundation. > > + * > > + */ > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#include "ci.h" > > +#include "ci_hdrc_qoriq.h" > > + > > +struct ci_hdrc_qoriq_data { > > + struct phy *phy; > > + struct clk *clk; > > + void __iomem *qoriq_regs; > > + struct platform_device *ci_pdev; > > + enum usb_phy_interface phy_mode; > > +}; > > + > > +/* > > + * clock helper functions > > + */ > > +static int ci_hdrc_qoriq_get_clks(struct platform_device *pdev) { > > + int ret; > > + struct device *dev = &pdev->dev; > > + struct ci_hdrc_qoriq_data *data = platform_get_drvdata(pdev); > > + > > + data->clk = devm_clk_get(dev, "usb2-clock"); > > + if (IS_ERR(data->clk)) { > > + dev_err(dev, "failed to get clk, err=%ld\n", > > + PTR_ERR(data->clk)); > > + return ret; Hello Peter, > > return PTR_ERR(data->clk), and delete ret. > Will take care in v3. > > + } > > + return 0; > > +} > > + > > +static int ci_hdrc_qoriq_prepare_enable_clks(struct platform_device > > +*pdev) { > > + int ret; > > + struct device *dev = &pdev->dev; > > + struct ci_hdrc_qoriq_data *data = platform_get_drvdata(pdev); > > + > > + ret = clk_prepare_enable(data->clk); > > + if (ret) { > > + dev_err(dev, "failed to prepare/enable clk, err=%d\n", ret); > > + return ret; > > + } > > + return 0; > > +} > > + > > +static void ci_hdrc_qoriq_disable_unprepare_clks(struct > > +platform_device *pdev) { > > + struct ci_hdrc_qoriq_data *data = platform_get_drvdata(pdev); > > + > > + clk_disable_unprepare(data->clk); > > +} > > + > > +static int ci_hdrc_qoriq_usb_setup(struct platform_device *pdev) { > > + u32 reg; > > + struct resource *res; > > + struct device *dev = &pdev->dev; > > + struct ci_hdrc_qoriq_data *data = platform_get_drvdata(pdev); > > + > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + if (!res) { > > + dev_err(dev, "failed to get I/O memory\n"); > > + return -ENOENT; > > + } > > + > > + dev_dbg(dev, "res->start %llx, resource_size(res) %llx\n", res->start, > > + resource_size(res)); > > Delete above debug message please. > Will take care in v3. > > + data->qoriq_regs = devm_ioremap(dev, res->start, resource_size(res)); > > + if (IS_ERR(data->qoriq_regs)) { > > + dev_err(dev, "failed to remap I/O memory\n"); > > + return -ENOMEM; > > + } > > + > > This mapping will be freed soon, using ioremap instead. > Correct. It is not required afterwards. Will use ioremap instead of devm_ioremap in v3. > > + data->phy_mode = of_usb_get_phy_mode(pdev->dev.of_node); > > + dev_dbg(dev, "phy_mode %d\n", data->phy_mode); > > Delete above debug message please. > Will take care in v3. > > + > > + reg = ioread32be(data->qoriq_regs + QORIQ_SOC_USB_CTRL); > > + switch (data->phy_mode) { > > + case USBPHY_INTERFACE_MODE_ULPI: > > + iowrite32be(reg | ~UTMI_PHY_EN, > > + data->qoriq_regs + QORIQ_SOC_USB_CTRL); > > + reg = ioread32be(data->qoriq_regs + QORIQ_SOC_USB_CTRL); > > + iowrite32be(reg | USB_CTRL_USB_EN, > > + data->qoriq_regs + QORIQ_SOC_USB_CTRL); > > + break; > > + default: > > + dev_err(dev, "unsupported phy_mode %d\n", data->phy_mode); > > + return -EINVAL; > > + } > > + > > + /* Setup Snooping for all the 4GB space */ > > + /* SNOOP1 starts from 0x0, size 2G */ > > + iowrite32be(SNOOP_SIZE_2GB, data->qoriq_regs + > QORIQ_SOC_USB_SNOOP1); > > + /* SNOOP2 starts from 0x80000000, size 2G */ > > + iowrite32be(SNOOP_SIZE_2GB | 0x80000000, > > + data->qoriq_regs + QORIQ_SOC_USB_SNOOP2); > > What does this snoop mean? > Snoop here refers to the cache coherency capability of Soc. > > + > > + iowrite32be(PRICTRL_PRI_LVL, data->qoriq_regs + > QORIQ_SOC_USB_PRICTRL); > > + iowrite32be(AGECNTTHRSH_THRESHOLD, data->qoriq_regs + > > + QORIQ_SOC_USB_AGECNTTHRSH); > > + iowrite32be(SICTRL_RD_PREFETCH_32_BYTE, data->qoriq_regs + > > + QORIQ_SOC_USB_SICTRL); > > + > > + devm_iounmap(dev, data->qoriq_regs); > > iounmap. Will take care in v3. > > > + return 0; > > +} > > + > > +static int ci_hdrc_qoriq_probe(struct platform_device *pdev) { > > + int ret; > > + struct device *dev = &pdev->dev; > > + struct ci_hdrc_qoriq_data *data; > > + struct ci_hdrc_platform_data pdata = { > > + .name = dev_name(dev), > > + .capoffset = DEF_CAPOFFSET, > > + .flags = CI_HDRC_DISABLE_STREAMING, > > + }; > > + > > + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); > > + if (!data) > > + return -ENOMEM; > > + > > + platform_set_drvdata(pdev, data); > > + > > + ret = ci_hdrc_qoriq_get_clks(pdev); > > + if (ret) > > + goto err_out; > > + > > + ret = ci_hdrc_qoriq_prepare_enable_clks(pdev); > > + if (ret) > > + goto err_out; > > Why not return directly? > Okay. Will return directly from here in v3. > > + > > + ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32)); > > + if (ret) { > > + dev_err(dev, "failed to set coherent dma mask, err=%d\n", ret); > > + goto err_clks; > > + } > > This dma setting does not be needed, the device tree will set it when the device is > created. > Okay, will drop dma_coerce_mask_and_coherent usage in v3. > > + > > + ret = ci_hdrc_qoriq_usb_setup(pdev); > > + if (ret) { > > + dev_err(dev, "failed to perform qoriq_usb2 setup, err=%d\n", > > + ret); > > + goto err_clks; > > + } > > + > > + data->phy = devm_phy_get(dev, "usb2-phy"); > > + if (IS_ERR(data->phy)) { > > + ret = PTR_ERR(data->phy); > > + /* Return -EINVAL if no usbphy is available */ > > + if (ret == -ENODEV) > > + ret = -EINVAL; > > + dev_err(dev, "failed get phy device, err=%d\n", ret); > > + goto err_clks; > > + } > > + pdata.phy = data->phy; > > The chipidea core will do that. > Okay, I checked now drivers/usb/chipidea/core.c is calling devm_phy_get with "usb-phy", will remove above code and rename from "usb2-phy" to "usb-phy" in dts in v3. > > + > > + data->ci_pdev = ci_hdrc_add_device(dev, > > + pdev->resource, pdev->num_resources, > > + &pdata); > > + if (IS_ERR(data->ci_pdev)) { > > + ret = PTR_ERR(data->ci_pdev); > > + dev_err(dev, > > + "failed to register ci_hdrc platform device, err=%d\n", > > + ret); > > + goto err_clks; > > + } > > + > > + pm_runtime_no_callbacks(dev); > > + pm_runtime_enable(dev); > > + > > + dev_dbg(dev, "initialized\n"); > > + return 0; > > + > > +err_clks: > > + ci_hdrc_qoriq_disable_unprepare_clks(pdev); > > If you have only one clock, it is unnecessary to use dedicated APIs for clock operation. > We do have multiple clocks, but currently one is integrated in code. Hence created the APIs for future use. > > +err_out: > > + return ret; > > +} > > + > > +static int ci_hdrc_qoriq_remove(struct platform_device *pdev) { > > + struct device *dev = &pdev->dev; > > + struct ci_hdrc_qoriq_data *data = platform_get_drvdata(pdev); > > + > > + pm_runtime_disable(dev); > > + ci_hdrc_remove_device(data->ci_pdev); > > + ci_hdrc_qoriq_disable_unprepare_clks(pdev); > > + dev_dbg(dev, "de-initialized\n"); > > + return 0; > > +} > > + > > +static void ci_hdrc_qoriq_shutdown(struct platform_device *pdev) { > > + ci_hdrc_qoriq_remove(pdev); > > +} > > + > > +static const struct of_device_id ci_hdrc_qoriq_dt_ids[] = { > > + { .compatible = "fsl,ci-qoriq-usb2"}, > > + { /* sentinel */ } > > +}; > > +MODULE_DEVICE_TABLE(of, ci_hdrc_qoriq_dt_ids); > > + > > +static struct platform_driver ci_hdrc_qoriq_driver = { > > + .probe = ci_hdrc_qoriq_probe, > > + .remove = ci_hdrc_qoriq_remove, > > + .shutdown = ci_hdrc_qoriq_shutdown, > > + .driver = { > > + .name = "ci_qoriq_usb2", > > + .of_match_table = ci_hdrc_qoriq_dt_ids, > > + }, > > +}; > > + > > +module_platform_driver(ci_hdrc_qoriq_driver); > > + > > +MODULE_ALIAS("platform:ci-qoriq-usb2"); > > +MODULE_LICENSE("GPL v2"); > > +MODULE_DESCRIPTION("CI HDRC QORIQ USB binding"); > > +MODULE_AUTHOR("Rajesh Bhagat "); > > diff --git a/drivers/usb/chipidea/ci_hdrc_qoriq.h > > b/drivers/usb/chipidea/ci_hdrc_qoriq.h > > new file mode 100644 > > index 0000000..afd29442 > > --- /dev/null > > +++ b/drivers/usb/chipidea/ci_hdrc_qoriq.h > > @@ -0,0 +1,65 @@ > > +/* > > + * QorIQ SoC USB 2.0 Controller driver > > + * > > + * Copyright 2016 Freescale Semiconductor, Inc. > > + * Author: Rajesh Bhagat > > + * > > + * This program is free software; you can redistribute it and/or > > +modify > > + * it under the terms of the GNU General Public License version 2 as > > + * published by the Free Software Foundation. > > + * > > + */ > > +#ifndef __DRIVER_USB_CHIPIDEA_CI_HDRC_QORIQ_H > > +#define __DRIVER_USB_CHIPIDEA_CI_HDRC_QORIQ_H > > + > > +/* offsets for the non-ehci registers in the QORIQ SOC USB controller */ > > +#define QORIQ_SOC_USB_SBUSCFG 0x90 > > +#define SBUSCFG_INCR8 0x02 /* INCR8, specified */ > > +#define QORIQ_SOC_USB_ULPIVP 0x170 > > +#define QORIQ_SOC_USB_PORTSC1 0x184 > > +#define PORT_PTS_MSK (3<<30) > > +#define PORT_PTS_UTMI (0<<30) > > +#define PORT_PTS_ULPI (2<<30) > > +#define PORT_PTS_SERIAL (3<<30) > > +#define PORT_PTS_PTW (1<<28) > > +#define QORIQ_SOC_USB_PORTSC2 0x188 > > +#define QORIQ_SOC_USB_USBMODE 0x1a8 > > +#define USBMODE_CM_MASK (3 << 0) /* controller mode mask > */ > > +#define USBMODE_CM_HOST (3 << 0) /* controller mode: host */ > > +#define USBMODE_ES (1 << 2) /* (Big) Endian Select */ > > + > > +#define QORIQ_SOC_USB_USBGENCTRL 0x200 > > +#define USBGENCTRL_PPP (1 << 3) > > +#define USBGENCTRL_PFP (1 << 2) > > +#define QORIQ_SOC_USB_ISIPHYCTRL 0x204 > > +#define ISIPHYCTRL_PXE (1) > > +#define ISIPHYCTRL_PHYE (1 << 4) > > + > > +#define QORIQ_SOC_USB_SNOOP1 0x400 /* NOTE: big-endian */ > > +#define QORIQ_SOC_USB_SNOOP2 0x404 /* NOTE: big-endian */ > > +#define QORIQ_SOC_USB_AGECNTTHRSH 0x408 /* NOTE: big-endian */ > > +#define AGECNTTHRSH_THRESHOLD 0x40 > > +#define QORIQ_SOC_USB_PRICTRL 0x40c /* NOTE: big-endian */ > > +#define PRICTRL_PRI_LVL 0xc > > +#define QORIQ_SOC_USB_SICTRL 0x410 /* NOTE: big-endian */ > > +#define SICTRL_RD_PREFETCH_32_BYTE (0x1) > > +#define SICTRL_RD_PREFETCH_64_BYTE (0x0) > > +#define QORIQ_SOC_USB_CTRL 0x500 /* NOTE: big-endian */ > > +#define CTRL_UTMI_PHY_EN (1 << 9) > > +#define CTRL_PHY_CLK_VALID (1 << 17) > > +#define SNOOP_SIZE_2GB 0x1e > > + > > +/* control Register Bit Masks */ > > +#define CONTROL_REGISTER_W1C_MASK 0x00020000 /* W1C: > PHY_CLK_VALID */ > > +#define ULPI_INT_EN (1<<0) > > +#define WU_INT_EN (1<<1) > > +#define USB_CTRL_USB_EN (1<<2) > > +#define LINE_STATE_FILTER__EN (1<<3) > > +#define KEEP_OTG_ON (1<<4) > > +#define OTG_PORT (1<<5) > > +#define PLL_RESET (1<<8) > > +#define UTMI_PHY_EN (1<<9) > > +#define ULPI_PHY_CLK_SEL (1<<10) > > +#define PHY_CLK_VALID (1<<17) > > + > > Move necessary definitions to source file, and delete this header file please, this > header file is only used by ci_hdrc_qoriq.c. > Okay, will take care in v3. Best Regards, Rajesh Bhagat > -- > > Best Regards, > Peter Chen