linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/5] drivers: usb: chipidea: Add qoriq platform driver
       [not found] ` <1468038656-10345-2-git-send-email-rajesh.bhagat@nxp.com>
@ 2016-07-09  5:53   ` kbuild test robot
  2016-07-09  5:59   ` kbuild test robot
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 15+ messages in thread
From: kbuild test robot @ 2016-07-09  5:53 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

[auto build test WARNING on phy/next]
[also build test WARNING on v4.7-rc6 next-20160708]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Rajesh-Bhagat/drivers-usb-chipidea-Add-qoriq-platform-driver/20160709-130557
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git next
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.1.1-1) 6.1.1 20160430
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   In file included from include/linux/printk.h:289:0,
                    from include/linux/kernel.h:13,
                    from include/linux/list.h:8,
                    from include/linux/module.h:9,
                    from drivers/usb/chipidea/ci_hdrc_qoriq.c:12:
   drivers/usb/chipidea/ci_hdrc_qoriq.c: In function 'ci_hdrc_qoriq_usb_setup':
>> drivers/usb/chipidea/ci_hdrc_qoriq.c:85:15: warning: format '%llx' expects argument of type 'long long unsigned int', but argument 4 has type 'resource_size_t {aka unsigned int}' [-Wformat=]
     dev_dbg(dev, "res->start %llx, resource_size(res) %llx\n", res->start,
                  ^
   include/linux/dynamic_debug.h:86:39: note: in definition of macro 'dynamic_dev_dbg'
      __dynamic_dev_dbg(&descriptor, dev, fmt, \
                                          ^~~
>> drivers/usb/chipidea/ci_hdrc_qoriq.c:85:2: note: in expansion of macro 'dev_dbg'
     dev_dbg(dev, "res->start %llx, resource_size(res) %llx\n", res->start,
     ^~~~~~~
   drivers/usb/chipidea/ci_hdrc_qoriq.c:85:15: warning: format '%llx' expects argument of type 'long long unsigned int', but argument 5 has type 'resource_size_t {aka unsigned int}' [-Wformat=]
     dev_dbg(dev, "res->start %llx, resource_size(res) %llx\n", res->start,
                  ^
   include/linux/dynamic_debug.h:86:39: note: in definition of macro 'dynamic_dev_dbg'
      __dynamic_dev_dbg(&descriptor, dev, fmt, \
                                          ^~~
>> drivers/usb/chipidea/ci_hdrc_qoriq.c:85:2: note: in expansion of macro 'dev_dbg'
     dev_dbg(dev, "res->start %llx, resource_size(res) %llx\n", res->start,
     ^~~~~~~

vim +85 drivers/usb/chipidea/ci_hdrc_qoriq.c

     6	 *
     7	 * This program is free software; you can redistribute it and/or modify
     8	 * it under the terms of the GNU General Public License version 2 as
     9	 * published by the Free Software Foundation.
    10	 *
    11	 */
  > 12	#include <linux/module.h>
    13	#include <linux/of_platform.h>
    14	#include <linux/of_gpio.h>
    15	#include <linux/platform_device.h>
    16	#include <linux/pm_runtime.h>
    17	#include <linux/dma-mapping.h>
    18	#include <linux/usb/of.h>
    19	#include <linux/usb/chipidea.h>
    20	#include <linux/clk.h>
    21	
    22	#include "ci.h"
    23	#include "ci_hdrc_qoriq.h"
    24	
    25	struct ci_hdrc_qoriq_data {
    26		struct phy *phy;
    27		struct clk *clk;
    28		void __iomem *qoriq_regs;
    29		struct platform_device *ci_pdev;
    30		enum usb_phy_interface phy_mode;
    31	};
    32	
    33	/*
    34	 * clock helper functions
    35	 */
    36	static int ci_hdrc_qoriq_get_clks(struct platform_device *pdev)
    37	{
    38		int ret;
    39		struct device *dev = &pdev->dev;
    40		struct ci_hdrc_qoriq_data *data = platform_get_drvdata(pdev);
    41	
    42		data->clk = devm_clk_get(dev, "usb2-clock");
    43		if (IS_ERR(data->clk)) {
    44			dev_err(dev, "failed to get clk, err=%ld\n",
    45						PTR_ERR(data->clk));
    46				return ret;
    47		}
    48		return 0;
    49	}
    50	
    51	static int ci_hdrc_qoriq_prepare_enable_clks(struct platform_device *pdev)
    52	{
    53		int ret;
    54		struct device *dev = &pdev->dev;
    55		struct ci_hdrc_qoriq_data *data = platform_get_drvdata(pdev);
    56	
    57		ret = clk_prepare_enable(data->clk);
    58		if (ret) {
    59			dev_err(dev, "failed to prepare/enable clk, err=%d\n", ret);
    60			return ret;
    61		}
    62		return 0;
    63	}
    64	
    65	static void ci_hdrc_qoriq_disable_unprepare_clks(struct platform_device *pdev)
    66	{
    67		struct ci_hdrc_qoriq_data *data = platform_get_drvdata(pdev);
    68	
    69		clk_disable_unprepare(data->clk);
    70	}
    71	
    72	static int ci_hdrc_qoriq_usb_setup(struct platform_device *pdev)
    73	{
    74		u32 reg;
    75		struct resource *res;
    76		struct device *dev = &pdev->dev;
    77		struct ci_hdrc_qoriq_data *data = platform_get_drvdata(pdev);
    78	
    79		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
    80		if (!res) {
    81			dev_err(dev, "failed to get I/O memory\n");
    82			return -ENOENT;
    83		}
    84	
  > 85		dev_dbg(dev, "res->start %llx, resource_size(res) %llx\n", res->start,
    86			resource_size(res));
    87		data->qoriq_regs = devm_ioremap(dev, res->start, resource_size(res));
    88		if (IS_ERR(data->qoriq_regs)) {

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/octet-stream
Size: 55044 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160709/bca032e6/attachment-0001.obj>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 1/5] drivers: usb: chipidea: Add qoriq platform driver
       [not found] ` <1468038656-10345-2-git-send-email-rajesh.bhagat@nxp.com>
  2016-07-09  5:53   ` [PATCH v2 1/5] drivers: usb: chipidea: Add qoriq platform driver kbuild test robot
@ 2016-07-09  5:59   ` kbuild test robot
  2016-07-09  6:13   ` kbuild test robot
  2016-07-11  6:43   ` Peter Chen
  3 siblings, 0 replies; 15+ messages in thread
From: kbuild test robot @ 2016-07-09  5:59 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

[auto build test WARNING on phy/next]
[also build test WARNING on v4.7-rc6 next-20160708]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Rajesh-Bhagat/drivers-usb-chipidea-Add-qoriq-platform-driver/20160709-130557
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git next
config: blackfin-allmodconfig (attached as .config)
compiler: bfin-uclinux-gcc (GCC) 4.6.3
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=blackfin 

All warnings (new ones prefixed by >>):

   drivers/usb/chipidea/ci_hdrc_qoriq.c: In function 'ci_hdrc_qoriq_usb_setup':
>> drivers/usb/chipidea/ci_hdrc_qoriq.c:85:2: warning: format '%llx' expects argument of type 'long long unsigned int', but argument 4 has type 'resource_size_t' [-Wformat]
   drivers/usb/chipidea/ci_hdrc_qoriq.c:85:2: warning: format '%llx' expects argument of type 'long long unsigned int', but argument 5 has type 'resource_size_t' [-Wformat]

vim +85 drivers/usb/chipidea/ci_hdrc_qoriq.c

    69		clk_disable_unprepare(data->clk);
    70	}
    71	
    72	static int ci_hdrc_qoriq_usb_setup(struct platform_device *pdev)
    73	{
    74		u32 reg;
    75		struct resource *res;
    76		struct device *dev = &pdev->dev;
    77		struct ci_hdrc_qoriq_data *data = platform_get_drvdata(pdev);
    78	
    79		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
    80		if (!res) {
    81			dev_err(dev, "failed to get I/O memory\n");
    82			return -ENOENT;
    83		}
    84	
  > 85		dev_dbg(dev, "res->start %llx, resource_size(res) %llx\n", res->start,
    86			resource_size(res));
    87		data->qoriq_regs = devm_ioremap(dev, res->start, resource_size(res));
    88		if (IS_ERR(data->qoriq_regs)) {
    89			dev_err(dev, "failed to remap I/O memory\n");
    90			return -ENOMEM;
    91		}
    92	
    93		data->phy_mode = of_usb_get_phy_mode(pdev->dev.of_node);

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/octet-stream
Size: 40629 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160709/ca0e5ac7/attachment-0001.obj>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 1/5] drivers: usb: chipidea: Add qoriq platform driver
       [not found] ` <1468038656-10345-2-git-send-email-rajesh.bhagat@nxp.com>
  2016-07-09  5:53   ` [PATCH v2 1/5] drivers: usb: chipidea: Add qoriq platform driver kbuild test robot
  2016-07-09  5:59   ` kbuild test robot
@ 2016-07-09  6:13   ` kbuild test robot
  2016-07-11  6:43   ` Peter Chen
  3 siblings, 0 replies; 15+ messages in thread
From: kbuild test robot @ 2016-07-09  6:13 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

[auto build test WARNING on phy/next]
[also build test WARNING on v4.7-rc6 next-20160708]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Rajesh-Bhagat/drivers-usb-chipidea-Add-qoriq-platform-driver/20160709-130557
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git next
config: mn10300-allyesconfig (attached as .config)
compiler: am33_2.0-linux-gcc (GCC) 4.9.0
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=mn10300 

All warnings (new ones prefixed by >>):

   In file included from include/linux/printk.h:289:0,
                    from include/linux/kernel.h:13,
                    from include/linux/list.h:8,
                    from include/linux/module.h:9,
                    from drivers/usb/chipidea/ci_hdrc_qoriq.c:12:
   drivers/usb/chipidea/ci_hdrc_qoriq.c: In function 'ci_hdrc_qoriq_usb_setup':
>> include/linux/dynamic_debug.h:64:16: warning: format '%llx' expects argument of type 'long long unsigned int', but argument 4 has type 'resource_size_t' [-Wformat=]
     static struct _ddebug  __aligned(8)   \
                   ^
   include/linux/dynamic_debug.h:84:2: note: in expansion of macro 'DEFINE_DYNAMIC_DEBUG_METADATA'
     DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt);  \
     ^
   include/linux/device.h:1197:2: note: in expansion of macro 'dynamic_dev_dbg'
     dynamic_dev_dbg(dev, format, ##__VA_ARGS__); \
     ^
   drivers/usb/chipidea/ci_hdrc_qoriq.c:85:2: note: in expansion of macro 'dev_dbg'
     dev_dbg(dev, "res->start %llx, resource_size(res) %llx\n", res->start,
     ^
   include/linux/dynamic_debug.h:64:16: warning: format '%llx' expects argument of type 'long long unsigned int', but argument 5 has type 'resource_size_t' [-Wformat=]
     static struct _ddebug  __aligned(8)   \
                   ^
   include/linux/dynamic_debug.h:84:2: note: in expansion of macro 'DEFINE_DYNAMIC_DEBUG_METADATA'
     DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt);  \
     ^
   include/linux/device.h:1197:2: note: in expansion of macro 'dynamic_dev_dbg'
     dynamic_dev_dbg(dev, format, ##__VA_ARGS__); \
     ^
   drivers/usb/chipidea/ci_hdrc_qoriq.c:85:2: note: in expansion of macro 'dev_dbg'
     dev_dbg(dev, "res->start %llx, resource_size(res) %llx\n", res->start,
     ^

vim +64 include/linux/dynamic_debug.h

b48420c1 Jim Cromie  2012-04-27  48  					const char *modname);
b48420c1 Jim Cromie  2012-04-27  49  
cbc46635 Joe Perches 2011-08-11  50  struct device;
cbc46635 Joe Perches 2011-08-11  51  
b9075fa9 Joe Perches 2011-10-31  52  extern __printf(3, 4)
906d2015 Joe Perches 2014-09-24  53  void __dynamic_dev_dbg(struct _ddebug *descriptor, const struct device *dev,
b9075fa9 Joe Perches 2011-10-31  54  		       const char *fmt, ...);
cbc46635 Joe Perches 2011-08-11  55  
ffa10cb4 Jason Baron 2011-08-11  56  struct net_device;
ffa10cb4 Jason Baron 2011-08-11  57  
b9075fa9 Joe Perches 2011-10-31  58  extern __printf(3, 4)
906d2015 Joe Perches 2014-09-24  59  void __dynamic_netdev_dbg(struct _ddebug *descriptor,
ffa10cb4 Jason Baron 2011-08-11  60  			  const struct net_device *dev,
b9075fa9 Joe Perches 2011-10-31  61  			  const char *fmt, ...);
ffa10cb4 Jason Baron 2011-08-11  62  
07613b0b Jason Baron 2011-10-04  63  #define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt)		\
c0d2af63 Joe Perches 2012-10-18 @64  	static struct _ddebug  __aligned(8)			\
07613b0b Jason Baron 2011-10-04  65  	__attribute__((section("__verbose"))) name = {		\
07613b0b Jason Baron 2011-10-04  66  		.modname = KBUILD_MODNAME,			\
07613b0b Jason Baron 2011-10-04  67  		.function = __func__,				\
07613b0b Jason Baron 2011-10-04  68  		.filename = __FILE__,				\
07613b0b Jason Baron 2011-10-04  69  		.format = (fmt),				\
07613b0b Jason Baron 2011-10-04  70  		.lineno = __LINE__,				\
07613b0b Jason Baron 2011-10-04  71  		.flags =  _DPRINTK_FLAGS_DEFAULT,		\
07613b0b Jason Baron 2011-10-04  72  	}

:::::: The code at line 64 was first introduced by commit
:::::: c0d2af637863940b1a4fb208224ca7acb905c39f dynamic_debug: Remove unnecessary __used

:::::: TO: Joe Perches <joe@perches.com>
:::::: CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/octet-stream
Size: 39173 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160709/686a9682/attachment-0001.obj>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 1/5] drivers: usb: chipidea: Add qoriq platform driver
       [not found] ` <1468038656-10345-2-git-send-email-rajesh.bhagat@nxp.com>
                     ` (2 preceding siblings ...)
  2016-07-09  6:13   ` kbuild test robot
@ 2016-07-11  6:43   ` Peter Chen
  2016-07-12  3:59     ` Rajesh Bhagat
  3 siblings, 1 reply; 15+ messages in thread
From: Peter Chen @ 2016-07-11  6:43 UTC (permalink / raw)
  To: linux-arm-kernel

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 <rajesh.bhagat@nxp.com>
> ---
> 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 <rajesh.bhagat@nxp.com>
> + *
> + * 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 <linux/module.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_gpio.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/usb/of.h>
> +#include <linux/usb/chipidea.h>
> +#include <linux/clk.h>
> +
> +#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;

return PTR_ERR(data->clk), and delete ret.

> +	}
> +	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.

> +	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.

> +	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.

> +
> +	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?

> +
> +	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.

> +	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?

> +
> +	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.

> +
> +	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.

> +
> +	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.

> +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 <rajesh.bhagat@nxp.com>");
> 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 <rajesh.bhagat@nxp.com>
> + *
> + * 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. 

-- 

Best Regards,
Peter Chen

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 2/5] usb: DT binding documentation for qoriq usb 2.0 controller
       [not found] ` <1468038656-10345-3-git-send-email-rajesh.bhagat@nxp.com>
@ 2016-07-11  6:48   ` Peter Chen
  2016-07-12  3:59     ` Rajesh Bhagat
  2016-07-16  0:28   ` Rob Herring
  1 sibling, 1 reply; 15+ messages in thread
From: Peter Chen @ 2016-07-11  6:48 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, Jul 09, 2016 at 10:00:53AM +0530, Rajesh Bhagat wrote:
> Describes the qoriq usb 2.0 controller driver binding, currently used
> for LS1021A and LS1012A platform.
> 
> Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>
> ---
> Changes in v2: 
>  - Adds DT binding documentation for qoriq usb 2.0 controller
>  - Changed the compatible string to fsl,ci-qoriq-usb2
> 
>  .../devicetree/bindings/usb/ci-hdrc-qoriq.txt      | 34 ++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/ci-hdrc-qoriq.txt
> 
> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-qoriq.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-qoriq.txt
> new file mode 100644
> index 0000000..8ad7306
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-qoriq.txt
> @@ -0,0 +1,34 @@
> +* Freescale QorIQ SoC USB 2.0 Controllers
> +
> +Required properties:
> +- compatible: Should be "fsl,ci-qoriq-usb2"
> +  Wherever applicable, the IP version of the USB controller should
> +  also be mentioned (for eg. fsl,ci-qoriq-usb2-vX.Y).
> +  where, X.Y is IP version of USB controller.

Why you need to add IP version at compatible string?
Does it can't be read out from ID register of Identification Registers.

> +- reg: Should contain registers location and length
> +- interrupts: Should contain controller interrupt
> +- phy-names: from the *Generic PHY* bindings
> +- phys: from the *Generic PHY* bindings
> +- clocks: clock provider specifier
> +- clock-names: shall be "usb2-clock"
> +Refer to clk/clock-bindings.txt for generic clock consumer properties
> +
> +Recommended properties:
> +- dr_mode: One of "host" or "peripheral".

Do you support dual-role?

> +- phy_type: the type of the phy connected to the core. Should be one
> +  of "utmi", "utmi_wide", "ulpi", "serial" or "hsic". Without this
> +  property the PORTSC register won't be touched
> +
> +Examples:
> +usb at 8600000 {
> +	compatible =  "fsl,ci-qoriq-usb2",
> +		      "fsl,ci-qoriq-usb2-v2.5";
> +	reg = <0x0 0x8600000 0x0 0x1000>;
> +	interrupts = <0 139 0x4>;
> +	phy-names = "usb2-phy";
> +	phys = <&usbphy0>;
> +	clock-names = "usb2-clock";
> +	clocks = <&clockgen 4 3>;
> +	dr_mode = "host";
> +	phy_type = "ulpi";
> +};
> -- 
> 2.6.2.198.g614a2ac
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 

Best Regards,
Peter Chen

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 3/5] drivers: usb: phy: Add qoriq usb 2.0 phy driver support
       [not found] ` <1468038656-10345-4-git-send-email-rajesh.bhagat@nxp.com>
@ 2016-07-11  6:54   ` Peter Chen
  2016-07-12  3:59     ` Rajesh Bhagat
  0 siblings, 1 reply; 15+ messages in thread
From: Peter Chen @ 2016-07-11  6:54 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, Jul 09, 2016 at 10:00:54AM +0530, Rajesh Bhagat wrote:
> Adds qoriq usb 2.0 phy driver support for LS1021A and LS1012A
> platform.
> 
> Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>
> ---
> Changes in v2:
>  - Replaced Freescale with QorIQ in comments section
>  - Changed the compatible string to fsl,qoriq-usb2-phy and added version
>  - Added dependency on ARCH_MXC/ARCH_LAYERSCAPE and OF in Kconfig
>  - Dropped CONFIG_ULPI #ifdefs to make code generic
>  - Removed calls to devm free/release calls
> 
>  drivers/phy/Kconfig          |   8 ++
>  drivers/phy/Makefile         |   1 +
>  drivers/phy/phy-qoriq-usb2.c | 228 +++++++++++++++++++++++++++++++++++++++++++
>  drivers/phy/phy-qoriq-usb2.h |  50 ++++++++++
>  4 files changed, 287 insertions(+)
>  create mode 100644 drivers/phy/phy-qoriq-usb2.c
>  create mode 100644 drivers/phy/phy-qoriq-usb2.h
> 
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index b869b98..cc69299 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -434,4 +434,12 @@ config PHY_CYGNUS_PCIE
>  
>  source "drivers/phy/tegra/Kconfig"
>  
> +config PHY_QORIQ_USB2
> +	tristate "QorIQ USB 2.0 PHY driver"
> +	depends on ARCH_MXC || ARCH_LAYERSCAPE

It seems mxc platforms do not use this PHY.
Besides, if you are using ULPI phy, you need
to depend on ULPI bus.

Peter
> +	depends on OF
> +	select GENERIC_PHY
> +	help
> +	  Enable this to support the USB2.0 PHY on the QorIQ SoC.
> +
>  endmenu
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index 9c3e73c..044105a 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -53,5 +53,6 @@ obj-$(CONFIG_PHY_TUSB1210)		+= phy-tusb1210.o
>  obj-$(CONFIG_PHY_BRCM_SATA)		+= phy-brcm-sata.o
>  obj-$(CONFIG_PHY_PISTACHIO_USB)		+= phy-pistachio-usb.o
>  obj-$(CONFIG_PHY_CYGNUS_PCIE)		+= phy-bcm-cygnus-pcie.o
> +obj-$(CONFIG_PHY_QORIQ_USB2)            += phy-qoriq-usb2.o
>  
>  obj-$(CONFIG_ARCH_TEGRA) += tegra/
> diff --git a/drivers/phy/phy-qoriq-usb2.c b/drivers/phy/phy-qoriq-usb2.c
> new file mode 100644
> index 0000000..f74d255
> --- /dev/null
> +++ b/drivers/phy/phy-qoriq-usb2.c
> @@ -0,0 +1,228 @@
> +/*
> + * QorIQ SoC USB 2.0 PHY driver
> + *
> + * Copyright 2016 Freescale Semiconductor, Inc.
> + * Author: Rajesh Bhagat <rajesh.bhagat@nxp.com>
> + *
> + * 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 <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/usb/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_platform.h>
> +#include <linux/phy/phy.h>
> +#include <linux/usb/phy.h>
> +#include <linux/usb/ulpi.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +
> +#include "phy-qoriq-usb2.h"
> +
> +static int qoriq_usb2_phy_init(struct phy *_phy)
> +{
> +	struct qoriq_usb2_phy_ctx *ctx = phy_get_drvdata(_phy);
> +	struct device *dev = ctx->dev;
> +
> +	if (ctx->ulpi_phy) {
> +		if (usb_phy_init(ctx->ulpi_phy)) {
> +			dev_err(dev, "unable to init transceiver, probably missing\n");
> +			return -ENODEV;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int qoriq_usb2_phy_power_on(struct phy *_phy)
> +{
> +	struct qoriq_usb2_phy_ctx *ctx = phy_get_drvdata(_phy);
> +	u32 flags;
> +
> +	if (ctx->ulpi_phy) {
> +		flags = usb_phy_io_read(ctx->ulpi_phy, ULPI_OTG_CTRL);
> +		usb_phy_io_write(ctx->ulpi_phy, flags |
> +				 (ULPI_OTG_CTRL_DRVVBUS_EXT |
> +				 ULPI_OTG_CTRL_EXTVBUSIND), ULPI_OTG_CTRL);
> +		flags = usb_phy_io_read(ctx->ulpi_phy, ULPI_IFC_CTRL);
> +		usb_phy_io_write(ctx->ulpi_phy, flags |
> +				 (ULPI_IFC_CTRL_EXTERNAL_VBUS |
> +				 ULPI_IFC_CTRL_PASSTHRU), ULPI_IFC_CTRL);
> +	}
> +
> +	return 0;
> +}
> +
> +static int qoriq_usb2_phy_power_off(struct phy *_phy)
> +{
> +	/* TODO: Add logic to power off phy */
> +
> +	return 0;
> +}
> +
> +static int qoriq_usb2_phy_exit(struct phy *_phy)
> +{
> +	struct qoriq_usb2_phy_ctx *ctx = phy_get_drvdata(_phy);
> +
> +	if (ctx->ulpi_phy)
> +		usb_phy_shutdown(ctx->ulpi_phy);
> +
> +	return 0;
> +}
> +
> +static const struct phy_ops ops = {
> +	.init		= qoriq_usb2_phy_init,
> +	.power_on	= qoriq_usb2_phy_power_on,
> +	.power_off	= qoriq_usb2_phy_power_off,
> +	.exit		= qoriq_usb2_phy_exit,
> +	.owner		= THIS_MODULE,
> +};
> +
> +
> +static enum qoriq_usb2_phy_ver of_usb_get_phy_version(struct device_node *np)
> +{
> +	enum qoriq_usb2_phy_ver phy_version = QORIQ_PHY_UNKNOWN;
> +
> +	if (of_device_is_compatible(np, "fsl,qoriq-usb2-phy")) {
> +		if (of_device_is_compatible(np, "fsl,qoriq-usb2-phy-v1.0"))
> +			phy_version = QORIQ_PHY_LEGACY;
> +		else if (of_device_is_compatible(np, "fsl,qoriq-usb2-phy-v2.0"))
> +			phy_version = QORIQ_PHY_NXP_ISP1508;
> +	}
> +	return phy_version;
> +}
> +
> +static int qoriq_usb2_phy_probe(struct platform_device *pdev)
> +{
> +	int ret;
> +	struct resource *res;
> +	struct qoriq_usb2_phy_ctx *ctx;
> +	struct device *dev = &pdev->dev;
> +	const struct of_device_id *of_id;
> +	struct phy_provider *phy_provider;
> +	struct device_node *np = pdev->dev.of_node;
> +
> +	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
> +
> +	ctx->dev = dev;
> +
> +	of_id = of_match_device(dev->driver->of_match_table, dev);
> +	if (!of_id) {
> +		dev_err(dev, "failed to get device match\n");
> +		ret = -EINVAL;
> +		goto err_out;
> +	}
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res) {
> +		dev_err(dev, "failed to get I/O memory\n");
> +		ret = -ENOENT;
> +		goto err_out;
> +	}
> +
> +	ctx->regs = devm_ioremap(dev, res->start, resource_size(res));
> +	if (!ctx->regs) {
> +		dev_err(dev, "failed to remap I/O memory\n");
> +		ret = -ENOMEM;
> +		goto err_out;
> +	}
> +
> +	platform_set_drvdata(pdev, ctx);
> +
> +	ctx->phy = devm_phy_create(ctx->dev, NULL, &ops);
> +	if (IS_ERR(ctx->phy)) {
> +		dev_err(dev, "failed to create PHY\n");
> +		ret = PTR_ERR(ctx->phy);
> +		goto err_out;
> +	}
> +	phy_set_drvdata(ctx->phy, ctx);
> +
> +	ctx->phy_version = of_usb_get_phy_version(np);
> +	if (ctx->phy_version == QORIQ_PHY_UNKNOWN) {
> +		ret = -EINVAL;
> +		dev_err(dev, "failed to get PHY version\n");
> +		goto err_out;
> +	}
> +
> +	ctx->phy_type = of_usb_get_phy_mode(np);
> +	switch (ctx->phy_type) {
> +	case USBPHY_INTERFACE_MODE_ULPI:
> +		switch (ctx->phy_version) {
> +		case QORIQ_PHY_NXP_ISP1508:
> +			ctx->ulpi_phy = qoriq_otg_ulpi_create(0);
> +			if (!ctx->ulpi_phy) {
> +				dev_err(dev, "qoriq_otg_ulpi_create returned NULL\n");
> +				ret = -ENOMEM;
> +				goto err_out;
> +			}
> +			ctx->ulpi_phy->io_priv = ctx->regs + ULPI_VIEWPORT;
> +			break;
> +		default:
> +			ctx->ulpi_phy = NULL;
> +			break;
> +		}
> +		break;
> +	default:
> +		dev_err(&pdev->dev, "phy_type %d is invalid or unsupported\n",
> +			ctx->phy_type);
> +		ret = -EINVAL;
> +		goto err_out;
> +	}
> +
> +	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> +	if (IS_ERR(phy_provider)) {
> +		dev_err(dev, "failed to register phy_provider\n");
> +		ret = PTR_ERR_OR_ZERO(phy_provider);
> +		goto err_out;
> +	}
> +
> +	dev_dbg(dev, "initialized\n");
> +	return 0;
> +
> +err_out:
> +	return ret;
> +}
> +
> +static int qoriq_usb2_phy_remove(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct qoriq_usb2_phy_ctx *ctx = platform_get_drvdata(pdev);
> +
> +	devm_phy_destroy(ctx->dev, ctx->phy);
> +	devm_iounmap(dev, ctx->regs);
> +	dev_dbg(dev, "de-initialized\n");
> +	return 0;
> +}
> +
> +static const struct of_device_id qoriq_usb2_phy_dt_ids[] = {
> +	{ .compatible = "fsl,qoriq-usb2-phy"},
> +	{}
> +};
> +
> +MODULE_DEVICE_TABLE(of, qoriq_usb2_phy_dt_ids);
> +
> +static struct platform_driver qoriq_usb2_phy_driver = {
> +	.probe		= qoriq_usb2_phy_probe,
> +	.remove		= qoriq_usb2_phy_remove,
> +	.driver		= {
> +		.name	= "qoriq_usb2_phy",
> +		.owner		= THIS_MODULE,
> +		.of_match_table = of_match_ptr(qoriq_usb2_phy_dt_ids),
> +	},
> +};
> +
> +module_platform_driver(qoriq_usb2_phy_driver);
> +
> +MODULE_ALIAS("platform:qoriq-usb2-phy");
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("QorIQ SoC USB PHY driver");
> +MODULE_AUTHOR("Rajesh Bhagat <rajesh.bhagat@nxp.com>");
> diff --git a/drivers/phy/phy-qoriq-usb2.h b/drivers/phy/phy-qoriq-usb2.h
> new file mode 100644
> index 0000000..47c37a5
> --- /dev/null
> +++ b/drivers/phy/phy-qoriq-usb2.h
> @@ -0,0 +1,50 @@
> +/*
> + * Freescale SoC USB 2.0 PHY driver
> + *
> + * Copyright 2016 Freescale Semiconductor, Inc.
> + * Author: Rajesh Bhagat <rajesh.bhagat@nxp.com>
> + *
> + * 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 _PHY_QORIQ_USB2_H
> +#define _PHY_QORIQ_USB2_H
> +
> +#include <linux/clk.h>
> +#include <linux/phy/phy.h>
> +#include <linux/device.h>
> +#include <linux/regmap.h>
> +
> +#define ULPI_VIEWPORT           0x170
> +
> +enum qoriq_usb2_phy_ver {
> +	QORIQ_PHY_LEGACY,
> +	QORIQ_PHY_NXP_ISP1508,
> +	QORIQ_PHY_UNKNOWN,
> +};
> +
> +struct qoriq_usb2_phy_ctx {
> +	struct phy *phy;
> +	struct clk *clk;
> +	struct device *dev;
> +	void __iomem *regs;
> +	struct usb_phy *ulpi_phy;
> +	enum usb_phy_interface phy_type;
> +	enum qoriq_usb2_phy_ver phy_version;
> +};
> +
> +#ifdef CONFIG_USB_ULPI_VIEWPORT
> +static inline struct usb_phy *qoriq_otg_ulpi_create(unsigned int flags)
> +{
> +	return otg_ulpi_create(&ulpi_viewport_access_ops, flags);
> +}
> +#else
> +static inline struct usb_phy *qoriq_otg_ulpi_create(unsigned int flags)
> +{
> +	return NULL;
> +}
> +#endif
> +
> +#endif
> -- 
> 2.6.2.198.g614a2ac
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 

Best Regards,
Peter Chen

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 1/5] drivers: usb: chipidea: Add qoriq platform driver
  2016-07-11  6:43   ` Peter Chen
@ 2016-07-12  3:59     ` Rajesh Bhagat
  2016-07-15  7:13       ` Peter Chen
  0 siblings, 1 reply; 15+ messages in thread
From: Rajesh Bhagat @ 2016-07-12  3:59 UTC (permalink / raw)
  To: linux-arm-kernel



> -----Original Message-----
> From: Peter Chen [mailto:hzpeterchen at gmail.com]
> Sent: Monday, July 11, 2016 12:14 PM
> To: Rajesh Bhagat <rajesh.bhagat@nxp.com>
> Cc: linux-usb at vger.kernel.org; linux-kernel at vger.kernel.org;
> devicetree at vger.kernel.org; Peter Chen <peter.chen@nxp.com>;
> 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 <rajesh.bhagat@nxp.com>
> > ---
> > 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 <rajesh.bhagat@nxp.com>
> > + *
> > + * 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 <linux/module.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/of_gpio.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/dma-mapping.h>
> > +#include <linux/usb/of.h>
> > +#include <linux/usb/chipidea.h>
> > +#include <linux/clk.h>
> > +
> > +#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 <rajesh.bhagat@nxp.com>");
> > 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 <rajesh.bhagat@nxp.com>
> > + *
> > + * 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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 2/5] usb: DT binding documentation for qoriq usb 2.0 controller
  2016-07-11  6:48   ` [PATCH v2 2/5] usb: DT binding documentation for qoriq usb 2.0 controller Peter Chen
@ 2016-07-12  3:59     ` Rajesh Bhagat
  2016-07-15  7:15       ` Peter Chen
  0 siblings, 1 reply; 15+ messages in thread
From: Rajesh Bhagat @ 2016-07-12  3:59 UTC (permalink / raw)
  To: linux-arm-kernel



> -----Original Message-----
> From: Peter Chen [mailto:hzpeterchen at gmail.com]
> Sent: Monday, July 11, 2016 12:19 PM
> To: Rajesh Bhagat <rajesh.bhagat@nxp.com>
> Cc: linux-usb at vger.kernel.org; linux-kernel at vger.kernel.org;
> devicetree at vger.kernel.org; Peter Chen <peter.chen@nxp.com>;
> 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 2/5] usb: DT binding documentation for qoriq usb 2.0
> controller
> 
> On Sat, Jul 09, 2016 at 10:00:53AM +0530, Rajesh Bhagat wrote:
> > Describes the qoriq usb 2.0 controller driver binding, currently used
> > for LS1021A and LS1012A platform.
> >
> > Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>
> > ---
> > Changes in v2:
> >  - Adds DT binding documentation for qoriq usb 2.0 controller
> >  - Changed the compatible string to fsl,ci-qoriq-usb2
> >
> >  .../devicetree/bindings/usb/ci-hdrc-qoriq.txt      | 34
> ++++++++++++++++++++++
> >  1 file changed, 34 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/usb/ci-hdrc-qoriq.txt
> >
> > diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-qoriq.txt
> > b/Documentation/devicetree/bindings/usb/ci-hdrc-qoriq.txt
> > new file mode 100644
> > index 0000000..8ad7306
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-qoriq.txt
> > @@ -0,0 +1,34 @@
> > +* Freescale QorIQ SoC USB 2.0 Controllers
> > +
> > +Required properties:
> > +- compatible: Should be "fsl,ci-qoriq-usb2"
> > +  Wherever applicable, the IP version of the USB controller should
> > +  also be mentioned (for eg. fsl,ci-qoriq-usb2-vX.Y).
> > +  where, X.Y is IP version of USB controller.

Hello Peter, 

> 
> Why you need to add IP version at compatible string?
> Does it can't be read out from ID register of Identification Registers.
> 

I agree. Will drop this controller version thing in DTS in v3.

> > +- reg: Should contain registers location and length
> > +- interrupts: Should contain controller interrupt
> > +- phy-names: from the *Generic PHY* bindings
> > +- phys: from the *Generic PHY* bindings
> > +- clocks: clock provider specifier
> > +- clock-names: shall be "usb2-clock"
> > +Refer to clk/clock-bindings.txt for generic clock consumer properties
> > +
> > +Recommended properties:
> > +- dr_mode: One of "host" or "peripheral".
> 
> Do you support dual-role?
> 

Yes. We do support  both host/peripheral mode. 


Best Regards,
Rajesh Bhagat 

> > +- phy_type: the type of the phy connected to the core. Should be one
> > +  of "utmi", "utmi_wide", "ulpi", "serial" or "hsic". Without this
> > +  property the PORTSC register won't be touched
> > +
> > +Examples:
> > +usb at 8600000 {
> > +	compatible =  "fsl,ci-qoriq-usb2",
> > +		      "fsl,ci-qoriq-usb2-v2.5";
> > +	reg = <0x0 0x8600000 0x0 0x1000>;
> > +	interrupts = <0 139 0x4>;
> > +	phy-names = "usb2-phy";
> > +	phys = <&usbphy0>;
> > +	clock-names = "usb2-clock";
> > +	clocks = <&clockgen 4 3>;
> > +	dr_mode = "host";
> > +	phy_type = "ulpi";
> > +};
> > --
> > 2.6.2.198.g614a2ac
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-usb"
> > in the body of a message to majordomo at vger.kernel.org More majordomo
> > info at  http://vger.kernel.org/majordomo-info.html
> 
> --
> 
> Best Regards,
> Peter Chen

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 3/5] drivers: usb: phy: Add qoriq usb 2.0 phy driver support
  2016-07-11  6:54   ` [PATCH v2 3/5] drivers: usb: phy: Add qoriq usb 2.0 phy driver support Peter Chen
@ 2016-07-12  3:59     ` Rajesh Bhagat
  0 siblings, 0 replies; 15+ messages in thread
From: Rajesh Bhagat @ 2016-07-12  3:59 UTC (permalink / raw)
  To: linux-arm-kernel



> -----Original Message-----
> From: Peter Chen [mailto:hzpeterchen at gmail.com]
> Sent: Monday, July 11, 2016 12:24 PM
> To: Rajesh Bhagat <rajesh.bhagat@nxp.com>
> Cc: linux-usb at vger.kernel.org; linux-kernel at vger.kernel.org;
> devicetree at vger.kernel.org; Peter Chen <peter.chen@nxp.com>;
> 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 3/5] drivers: usb: phy: Add qoriq usb 2.0 phy driver support
> 
> On Sat, Jul 09, 2016 at 10:00:54AM +0530, Rajesh Bhagat wrote:
> > Adds qoriq usb 2.0 phy driver support for LS1021A and LS1012A
> > platform.
> >
> > Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>
> > ---
> > Changes in v2:
> >  - Replaced Freescale with QorIQ in comments section
> >  - Changed the compatible string to fsl,qoriq-usb2-phy and added
> > version
> >  - Added dependency on ARCH_MXC/ARCH_LAYERSCAPE and OF in Kconfig
> >  - Dropped CONFIG_ULPI #ifdefs to make code generic
> >  - Removed calls to devm free/release calls
> >
> >  drivers/phy/Kconfig          |   8 ++
> >  drivers/phy/Makefile         |   1 +
> >  drivers/phy/phy-qoriq-usb2.c | 228
> > +++++++++++++++++++++++++++++++++++++++++++
> >  drivers/phy/phy-qoriq-usb2.h |  50 ++++++++++
> >  4 files changed, 287 insertions(+)
> >  create mode 100644 drivers/phy/phy-qoriq-usb2.c  create mode 100644
> > drivers/phy/phy-qoriq-usb2.h
> >
> > diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index
> > b869b98..cc69299 100644
> > --- a/drivers/phy/Kconfig
> > +++ b/drivers/phy/Kconfig
> > @@ -434,4 +434,12 @@ config PHY_CYGNUS_PCIE
> >
> >  source "drivers/phy/tegra/Kconfig"
> >
> > +config PHY_QORIQ_USB2
> > +	tristate "QorIQ USB 2.0 PHY driver"
> > +	depends on ARCH_MXC || ARCH_LAYERSCAPE
> 

Hello Peter,

> It seems mxc platforms do not use this PHY.
> Besides, if you are using ULPI phy, you need to depend on ULPI bus.
> 

QorIQ platform are having both ULPI and non-ULPI PHY variants, Hence
this PHY driver is targeted for both. And driver takes decision on run time 
which PHY is there according to information passed in DTS files. 


Best Regards,
Rajesh Bhagat 

> Peter
> > +	depends on OF
> > +	select GENERIC_PHY
> > +	help
> > +	  Enable this to support the USB2.0 PHY on the QorIQ SoC.
> > +
> >  endmenu
> > diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile index
> > 9c3e73c..044105a 100644
> > --- a/drivers/phy/Makefile
> > +++ b/drivers/phy/Makefile
> > @@ -53,5 +53,6 @@ obj-$(CONFIG_PHY_TUSB1210)		+= phy-tusb1210.o
> >  obj-$(CONFIG_PHY_BRCM_SATA)		+= phy-brcm-sata.o
> >  obj-$(CONFIG_PHY_PISTACHIO_USB)		+= phy-pistachio-usb.o
> >  obj-$(CONFIG_PHY_CYGNUS_PCIE)		+= phy-bcm-cygnus-pcie.o
> > +obj-$(CONFIG_PHY_QORIQ_USB2)            += phy-qoriq-usb2.o
> >
> >  obj-$(CONFIG_ARCH_TEGRA) += tegra/
> > diff --git a/drivers/phy/phy-qoriq-usb2.c
> > b/drivers/phy/phy-qoriq-usb2.c new file mode 100644 index
> > 0000000..f74d255
> > --- /dev/null
> > +++ b/drivers/phy/phy-qoriq-usb2.c
> > @@ -0,0 +1,228 @@
> > +/*
> > + * QorIQ SoC USB 2.0 PHY driver
> > + *
> > + * Copyright 2016 Freescale Semiconductor, Inc.
> > + * Author: Rajesh Bhagat <rajesh.bhagat@nxp.com>
> > + *
> > + * 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 <linux/clk.h>
> > +#include <linux/clk-provider.h>
> > +#include <linux/io.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/usb/of.h>
> > +#include <linux/of_address.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/phy/phy.h>
> > +#include <linux/usb/phy.h>
> > +#include <linux/usb/ulpi.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/platform_device.h>
> > +
> > +#include "phy-qoriq-usb2.h"
> > +
> > +static int qoriq_usb2_phy_init(struct phy *_phy) {
> > +	struct qoriq_usb2_phy_ctx *ctx = phy_get_drvdata(_phy);
> > +	struct device *dev = ctx->dev;
> > +
> > +	if (ctx->ulpi_phy) {
> > +		if (usb_phy_init(ctx->ulpi_phy)) {
> > +			dev_err(dev, "unable to init transceiver, probably missing\n");
> > +			return -ENODEV;
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int qoriq_usb2_phy_power_on(struct phy *_phy) {
> > +	struct qoriq_usb2_phy_ctx *ctx = phy_get_drvdata(_phy);
> > +	u32 flags;
> > +
> > +	if (ctx->ulpi_phy) {
> > +		flags = usb_phy_io_read(ctx->ulpi_phy, ULPI_OTG_CTRL);
> > +		usb_phy_io_write(ctx->ulpi_phy, flags |
> > +				 (ULPI_OTG_CTRL_DRVVBUS_EXT |
> > +				 ULPI_OTG_CTRL_EXTVBUSIND), ULPI_OTG_CTRL);
> > +		flags = usb_phy_io_read(ctx->ulpi_phy, ULPI_IFC_CTRL);
> > +		usb_phy_io_write(ctx->ulpi_phy, flags |
> > +				 (ULPI_IFC_CTRL_EXTERNAL_VBUS |
> > +				 ULPI_IFC_CTRL_PASSTHRU), ULPI_IFC_CTRL);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int qoriq_usb2_phy_power_off(struct phy *_phy) {
> > +	/* TODO: Add logic to power off phy */
> > +
> > +	return 0;
> > +}
> > +
> > +static int qoriq_usb2_phy_exit(struct phy *_phy) {
> > +	struct qoriq_usb2_phy_ctx *ctx = phy_get_drvdata(_phy);
> > +
> > +	if (ctx->ulpi_phy)
> > +		usb_phy_shutdown(ctx->ulpi_phy);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct phy_ops ops = {
> > +	.init		= qoriq_usb2_phy_init,
> > +	.power_on	= qoriq_usb2_phy_power_on,
> > +	.power_off	= qoriq_usb2_phy_power_off,
> > +	.exit		= qoriq_usb2_phy_exit,
> > +	.owner		= THIS_MODULE,
> > +};
> > +
> > +
> > +static enum qoriq_usb2_phy_ver of_usb_get_phy_version(struct
> > +device_node *np) {
> > +	enum qoriq_usb2_phy_ver phy_version = QORIQ_PHY_UNKNOWN;
> > +
> > +	if (of_device_is_compatible(np, "fsl,qoriq-usb2-phy")) {
> > +		if (of_device_is_compatible(np, "fsl,qoriq-usb2-phy-v1.0"))
> > +			phy_version = QORIQ_PHY_LEGACY;
> > +		else if (of_device_is_compatible(np, "fsl,qoriq-usb2-phy-v2.0"))
> > +			phy_version = QORIQ_PHY_NXP_ISP1508;
> > +	}
> > +	return phy_version;
> > +}
> > +
> > +static int qoriq_usb2_phy_probe(struct platform_device *pdev) {
> > +	int ret;
> > +	struct resource *res;
> > +	struct qoriq_usb2_phy_ctx *ctx;
> > +	struct device *dev = &pdev->dev;
> > +	const struct of_device_id *of_id;
> > +	struct phy_provider *phy_provider;
> > +	struct device_node *np = pdev->dev.of_node;
> > +
> > +	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> > +	if (!ctx)
> > +		return -ENOMEM;
> > +
> > +	ctx->dev = dev;
> > +
> > +	of_id = of_match_device(dev->driver->of_match_table, dev);
> > +	if (!of_id) {
> > +		dev_err(dev, "failed to get device match\n");
> > +		ret = -EINVAL;
> > +		goto err_out;
> > +	}
> > +
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +	if (!res) {
> > +		dev_err(dev, "failed to get I/O memory\n");
> > +		ret = -ENOENT;
> > +		goto err_out;
> > +	}
> > +
> > +	ctx->regs = devm_ioremap(dev, res->start, resource_size(res));
> > +	if (!ctx->regs) {
> > +		dev_err(dev, "failed to remap I/O memory\n");
> > +		ret = -ENOMEM;
> > +		goto err_out;
> > +	}
> > +
> > +	platform_set_drvdata(pdev, ctx);
> > +
> > +	ctx->phy = devm_phy_create(ctx->dev, NULL, &ops);
> > +	if (IS_ERR(ctx->phy)) {
> > +		dev_err(dev, "failed to create PHY\n");
> > +		ret = PTR_ERR(ctx->phy);
> > +		goto err_out;
> > +	}
> > +	phy_set_drvdata(ctx->phy, ctx);
> > +
> > +	ctx->phy_version = of_usb_get_phy_version(np);
> > +	if (ctx->phy_version == QORIQ_PHY_UNKNOWN) {
> > +		ret = -EINVAL;
> > +		dev_err(dev, "failed to get PHY version\n");
> > +		goto err_out;
> > +	}
> > +
> > +	ctx->phy_type = of_usb_get_phy_mode(np);
> > +	switch (ctx->phy_type) {
> > +	case USBPHY_INTERFACE_MODE_ULPI:
> > +		switch (ctx->phy_version) {
> > +		case QORIQ_PHY_NXP_ISP1508:
> > +			ctx->ulpi_phy = qoriq_otg_ulpi_create(0);
> > +			if (!ctx->ulpi_phy) {
> > +				dev_err(dev, "qoriq_otg_ulpi_create returned NULL\n");
> > +				ret = -ENOMEM;
> > +				goto err_out;
> > +			}
> > +			ctx->ulpi_phy->io_priv = ctx->regs + ULPI_VIEWPORT;
> > +			break;
> > +		default:
> > +			ctx->ulpi_phy = NULL;
> > +			break;
> > +		}
> > +		break;
> > +	default:
> > +		dev_err(&pdev->dev, "phy_type %d is invalid or unsupported\n",
> > +			ctx->phy_type);
> > +		ret = -EINVAL;
> > +		goto err_out;
> > +	}
> > +
> > +	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> > +	if (IS_ERR(phy_provider)) {
> > +		dev_err(dev, "failed to register phy_provider\n");
> > +		ret = PTR_ERR_OR_ZERO(phy_provider);
> > +		goto err_out;
> > +	}
> > +
> > +	dev_dbg(dev, "initialized\n");
> > +	return 0;
> > +
> > +err_out:
> > +	return ret;
> > +}
> > +
> > +static int qoriq_usb2_phy_remove(struct platform_device *pdev) {
> > +	struct device *dev = &pdev->dev;
> > +	struct qoriq_usb2_phy_ctx *ctx = platform_get_drvdata(pdev);
> > +
> > +	devm_phy_destroy(ctx->dev, ctx->phy);
> > +	devm_iounmap(dev, ctx->regs);
> > +	dev_dbg(dev, "de-initialized\n");
> > +	return 0;
> > +}
> > +
> > +static const struct of_device_id qoriq_usb2_phy_dt_ids[] = {
> > +	{ .compatible = "fsl,qoriq-usb2-phy"},
> > +	{}
> > +};
> > +
> > +MODULE_DEVICE_TABLE(of, qoriq_usb2_phy_dt_ids);
> > +
> > +static struct platform_driver qoriq_usb2_phy_driver = {
> > +	.probe		= qoriq_usb2_phy_probe,
> > +	.remove		= qoriq_usb2_phy_remove,
> > +	.driver		= {
> > +		.name	= "qoriq_usb2_phy",
> > +		.owner		= THIS_MODULE,
> > +		.of_match_table = of_match_ptr(qoriq_usb2_phy_dt_ids),
> > +	},
> > +};
> > +
> > +module_platform_driver(qoriq_usb2_phy_driver);
> > +
> > +MODULE_ALIAS("platform:qoriq-usb2-phy");
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_DESCRIPTION("QorIQ SoC USB PHY driver"); MODULE_AUTHOR("Rajesh
> > +Bhagat <rajesh.bhagat@nxp.com>");
> > diff --git a/drivers/phy/phy-qoriq-usb2.h
> > b/drivers/phy/phy-qoriq-usb2.h new file mode 100644 index
> > 0000000..47c37a5
> > --- /dev/null
> > +++ b/drivers/phy/phy-qoriq-usb2.h
> > @@ -0,0 +1,50 @@
> > +/*
> > + * Freescale SoC USB 2.0 PHY driver
> > + *
> > + * Copyright 2016 Freescale Semiconductor, Inc.
> > + * Author: Rajesh Bhagat <rajesh.bhagat@nxp.com>
> > + *
> > + * 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 _PHY_QORIQ_USB2_H
> > +#define _PHY_QORIQ_USB2_H
> > +
> > +#include <linux/clk.h>
> > +#include <linux/phy/phy.h>
> > +#include <linux/device.h>
> > +#include <linux/regmap.h>
> > +
> > +#define ULPI_VIEWPORT           0x170
> > +
> > +enum qoriq_usb2_phy_ver {
> > +	QORIQ_PHY_LEGACY,
> > +	QORIQ_PHY_NXP_ISP1508,
> > +	QORIQ_PHY_UNKNOWN,
> > +};
> > +
> > +struct qoriq_usb2_phy_ctx {
> > +	struct phy *phy;
> > +	struct clk *clk;
> > +	struct device *dev;
> > +	void __iomem *regs;
> > +	struct usb_phy *ulpi_phy;
> > +	enum usb_phy_interface phy_type;
> > +	enum qoriq_usb2_phy_ver phy_version; };
> > +
> > +#ifdef CONFIG_USB_ULPI_VIEWPORT
> > +static inline struct usb_phy *qoriq_otg_ulpi_create(unsigned int
> > +flags) {
> > +	return otg_ulpi_create(&ulpi_viewport_access_ops, flags); } #else
> > +static inline struct usb_phy *qoriq_otg_ulpi_create(unsigned int
> > +flags) {
> > +	return NULL;
> > +}
> > +#endif
> > +
> > +#endif
> > --
> > 2.6.2.198.g614a2ac
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-usb"
> > in the body of a message to majordomo at vger.kernel.org More majordomo
> > info at  http://vger.kernel.org/majordomo-info.html
> 
> --
> 
> Best Regards,
> Peter Chen

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 1/5] drivers: usb: chipidea: Add qoriq platform driver
  2016-07-12  3:59     ` Rajesh Bhagat
@ 2016-07-15  7:13       ` Peter Chen
  2016-07-15  8:13         ` Rajesh Bhagat
  0 siblings, 1 reply; 15+ messages in thread
From: Peter Chen @ 2016-07-15  7:13 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jul 12, 2016 at 03:59:07AM +0000, Rajesh Bhagat wrote:
> > > +
> > > +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. 

If you could not integrate one more clocks, I suggest not creating dedicated
API until you need in future.

-- 

Best Regards,
Peter Chen

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 2/5] usb: DT binding documentation for qoriq usb 2.0 controller
  2016-07-12  3:59     ` Rajesh Bhagat
@ 2016-07-15  7:15       ` Peter Chen
  2016-07-15  8:13         ` Rajesh Bhagat
  0 siblings, 1 reply; 15+ messages in thread
From: Peter Chen @ 2016-07-15  7:15 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jul 12, 2016 at 03:59:14AM +0000, Rajesh Bhagat wrote:
> 
> 
> > -----Original Message-----
> > From: Peter Chen [mailto:hzpeterchen at gmail.com]
> > Sent: Monday, July 11, 2016 12:19 PM
> > To: Rajesh Bhagat <rajesh.bhagat@nxp.com>
> > Cc: linux-usb at vger.kernel.org; linux-kernel at vger.kernel.org;
> > devicetree at vger.kernel.org; Peter Chen <peter.chen@nxp.com>;
> > 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 2/5] usb: DT binding documentation for qoriq usb 2.0
> > controller
> > 
> > On Sat, Jul 09, 2016 at 10:00:53AM +0530, Rajesh Bhagat wrote:
> > > Describes the qoriq usb 2.0 controller driver binding, currently used
> > > for LS1021A and LS1012A platform.
> > >
> > > Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>
> > > ---
> > > Changes in v2:
> > >  - Adds DT binding documentation for qoriq usb 2.0 controller
> > >  - Changed the compatible string to fsl,ci-qoriq-usb2
> > >
> > >  .../devicetree/bindings/usb/ci-hdrc-qoriq.txt      | 34
> > ++++++++++++++++++++++
> > >  1 file changed, 34 insertions(+)
> > >  create mode 100644
> > > Documentation/devicetree/bindings/usb/ci-hdrc-qoriq.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-qoriq.txt
> > > b/Documentation/devicetree/bindings/usb/ci-hdrc-qoriq.txt
> > > new file mode 100644
> > > index 0000000..8ad7306
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-qoriq.txt
> > > @@ -0,0 +1,34 @@
> > > +* Freescale QorIQ SoC USB 2.0 Controllers
> > > +
> > > +Required properties:
> > > +- compatible: Should be "fsl,ci-qoriq-usb2"
> > > +  Wherever applicable, the IP version of the USB controller should
> > > +  also be mentioned (for eg. fsl,ci-qoriq-usb2-vX.Y).
> > > +  where, X.Y is IP version of USB controller.
> 
> Hello Peter, 
> 
> > 
> > Why you need to add IP version at compatible string?
> > Does it can't be read out from ID register of Identification Registers.
> > 
> 
> I agree. Will drop this controller version thing in DTS in v3.
> 
> > > +- reg: Should contain registers location and length
> > > +- interrupts: Should contain controller interrupt
> > > +- phy-names: from the *Generic PHY* bindings
> > > +- phys: from the *Generic PHY* bindings
> > > +- clocks: clock provider specifier
> > > +- clock-names: shall be "usb2-clock"
> > > +Refer to clk/clock-bindings.txt for generic clock consumer properties
> > > +
> > > +Recommended properties:
> > > +- dr_mode: One of "host" or "peripheral".
> > 
> > Do you support dual-role?
> > 
> 
> Yes. We do support  both host/peripheral mode. 
> 

I mean dual-role switch. If you support that, the dr_mode
should be "otg".

-- 

Best Regards,
Peter Chen

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 1/5] drivers: usb: chipidea: Add qoriq platform driver
  2016-07-15  7:13       ` Peter Chen
@ 2016-07-15  8:13         ` Rajesh Bhagat
  0 siblings, 0 replies; 15+ messages in thread
From: Rajesh Bhagat @ 2016-07-15  8:13 UTC (permalink / raw)
  To: linux-arm-kernel



> -----Original Message-----
> From: Peter Chen [mailto:hzpeterchen at gmail.com]
> Sent: Friday, July 15, 2016 12:43 PM
> To: Rajesh Bhagat <rajesh.bhagat@nxp.com>
> Cc: linux-usb at vger.kernel.org; linux-kernel at vger.kernel.org;
> devicetree at vger.kernel.org; Peter Chen <peter.chen@nxp.com>;
> 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 Tue, Jul 12, 2016 at 03:59:07AM +0000, Rajesh Bhagat wrote:
> > > > +
> > > > +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.

Hello Peter, 

> 
> If you could not integrate one more clocks, I suggest not creating dedicated API until
> you need in future.
> 

Okay, Will take care in v3. 

Best Regards,
Rajesh Bhagat 

> --
> 
> Best Regards,
> Peter Chen

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 2/5] usb: DT binding documentation for qoriq usb 2.0 controller
  2016-07-15  7:15       ` Peter Chen
@ 2016-07-15  8:13         ` Rajesh Bhagat
  0 siblings, 0 replies; 15+ messages in thread
From: Rajesh Bhagat @ 2016-07-15  8:13 UTC (permalink / raw)
  To: linux-arm-kernel



> -----Original Message-----
> From: Peter Chen [mailto:hzpeterchen at gmail.com]
> Sent: Friday, July 15, 2016 12:45 PM
> To: Rajesh Bhagat <rajesh.bhagat@nxp.com>
> Cc: linux-usb at vger.kernel.org; linux-kernel at vger.kernel.org;
> devicetree at vger.kernel.org; Peter Chen <peter.chen@nxp.com>;
> 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 2/5] usb: DT binding documentation for qoriq usb 2.0
> controller
> 
> On Tue, Jul 12, 2016 at 03:59:14AM +0000, Rajesh Bhagat wrote:
> >
> >
> > > -----Original Message-----
> > > From: Peter Chen [mailto:hzpeterchen at gmail.com]
> > > Sent: Monday, July 11, 2016 12:19 PM
> > > To: Rajesh Bhagat <rajesh.bhagat@nxp.com>
> > > Cc: linux-usb at vger.kernel.org; linux-kernel at vger.kernel.org;
> > > devicetree at vger.kernel.org; Peter Chen <peter.chen@nxp.com>;
> > > 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 2/5] usb: DT binding documentation for qoriq
> > > usb 2.0 controller
> > >
> > > On Sat, Jul 09, 2016 at 10:00:53AM +0530, Rajesh Bhagat wrote:
> > > > Describes the qoriq usb 2.0 controller driver binding, currently
> > > > used for LS1021A and LS1012A platform.
> > > >
> > > > Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>
> > > > ---
> > > > Changes in v2:
> > > >  - Adds DT binding documentation for qoriq usb 2.0 controller
> > > >  - Changed the compatible string to fsl,ci-qoriq-usb2
> > > >
> > > >  .../devicetree/bindings/usb/ci-hdrc-qoriq.txt      | 34
> > > ++++++++++++++++++++++
> > > >  1 file changed, 34 insertions(+)
> > > >  create mode 100644
> > > > Documentation/devicetree/bindings/usb/ci-hdrc-qoriq.txt
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/usb/ci-hdrc-qoriq.txt
> > > > b/Documentation/devicetree/bindings/usb/ci-hdrc-qoriq.txt
> > > > new file mode 100644
> > > > index 0000000..8ad7306
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-qoriq.txt
> > > > @@ -0,0 +1,34 @@
> > > > +* Freescale QorIQ SoC USB 2.0 Controllers
> > > > +
> > > > +Required properties:
> > > > +- compatible: Should be "fsl,ci-qoriq-usb2"
> > > > +  Wherever applicable, the IP version of the USB controller
> > > > +should
> > > > +  also be mentioned (for eg. fsl,ci-qoriq-usb2-vX.Y).
> > > > +  where, X.Y is IP version of USB controller.
> >
> > Hello Peter,
> >
> > >
> > > Why you need to add IP version at compatible string?
> > > Does it can't be read out from ID register of Identification Registers.
> > >
> >
> > I agree. Will drop this controller version thing in DTS in v3.
> >
> > > > +- reg: Should contain registers location and length
> > > > +- interrupts: Should contain controller interrupt
> > > > +- phy-names: from the *Generic PHY* bindings
> > > > +- phys: from the *Generic PHY* bindings
> > > > +- clocks: clock provider specifier
> > > > +- clock-names: shall be "usb2-clock"
> > > > +Refer to clk/clock-bindings.txt for generic clock consumer
> > > > +properties
> > > > +
> > > > +Recommended properties:
> > > > +- dr_mode: One of "host" or "peripheral".
> > >
> > > Do you support dual-role?
> > >
> >
> > Yes. We do support  both host/peripheral mode.
> >
> 

Hello Peter, 

> I mean dual-role switch. If you support that, the dr_mode should be "otg".
> 

For now, we don't support otg mode. 

Best Regards,
Rajesh Bhagat 

> --
> 
> Best Regards,
> Peter Chen

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 2/5] usb: DT binding documentation for qoriq usb 2.0 controller
       [not found] ` <1468038656-10345-3-git-send-email-rajesh.bhagat@nxp.com>
  2016-07-11  6:48   ` [PATCH v2 2/5] usb: DT binding documentation for qoriq usb 2.0 controller Peter Chen
@ 2016-07-16  0:28   ` Rob Herring
  1 sibling, 0 replies; 15+ messages in thread
From: Rob Herring @ 2016-07-16  0:28 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, Jul 09, 2016 at 10:00:53AM +0530, Rajesh Bhagat wrote:
> Describes the qoriq usb 2.0 controller driver binding, currently used
> for LS1021A and LS1012A platform.
> 
> Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>
> ---
> Changes in v2: 
>  - Adds DT binding documentation for qoriq usb 2.0 controller
>  - Changed the compatible string to fsl,ci-qoriq-usb2
> 
>  .../devicetree/bindings/usb/ci-hdrc-qoriq.txt      | 34 ++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/ci-hdrc-qoriq.txt
> 
> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-qoriq.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-qoriq.txt
> new file mode 100644
> index 0000000..8ad7306
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-qoriq.txt
> @@ -0,0 +1,34 @@
> +* Freescale QorIQ SoC USB 2.0 Controllers
> +
> +Required properties:
> +- compatible: Should be "fsl,ci-qoriq-usb2"
> +  Wherever applicable, the IP version of the USB controller should
> +  also be mentioned (for eg. fsl,ci-qoriq-usb2-vX.Y).
> +  where, X.Y is IP version of USB controller.

Please document known IP versions.

> +- reg: Should contain registers location and length
> +- interrupts: Should contain controller interrupt
> +- phy-names: from the *Generic PHY* bindings
> +- phys: from the *Generic PHY* bindings
> +- clocks: clock provider specifier
> +- clock-names: shall be "usb2-clock"

clock-names is kind of pointless for a single clock and '-clock' is 
redundant.

> +Refer to clk/clock-bindings.txt for generic clock consumer properties
> +
> +Recommended properties:
> +- dr_mode: One of "host" or "peripheral".
> +- phy_type: the type of the phy connected to the core. Should be one
> +  of "utmi", "utmi_wide", "ulpi", "serial" or "hsic". Without this
> +  property the PORTSC register won't be touched
> +
> +Examples:
> +usb at 8600000 {
> +	compatible =  "fsl,ci-qoriq-usb2",
> +		      "fsl,ci-qoriq-usb2-v2.5";

Order should be most specific to least specific.

> +	reg = <0x0 0x8600000 0x0 0x1000>;
> +	interrupts = <0 139 0x4>;
> +	phy-names = "usb2-phy";
> +	phys = <&usbphy0>;
> +	clock-names = "usb2-clock";
> +	clocks = <&clockgen 4 3>;
> +	dr_mode = "host";
> +	phy_type = "ulpi";
> +};
> -- 
> 2.6.2.198.g614a2ac
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 4/5] phy: DT binding documentation for qoriq usb 2.0 phy
       [not found] ` <1468038656-10345-5-git-send-email-rajesh.bhagat@nxp.com>
@ 2016-07-16  0:35   ` Rob Herring
  0 siblings, 0 replies; 15+ messages in thread
From: Rob Herring @ 2016-07-16  0:35 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, Jul 09, 2016 at 10:00:55AM +0530, Rajesh Bhagat wrote:
> Describes the qoriq usb 2.0 phy driver binding, currently used
> for LS1021A and LS1012A platform.
> 
> Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>
> ---
> Changes in v2:
>  - Adds DT binding documentation for qoriq usb 2.0 phy
>  - Changed the compatible string to fsl,qoriq-usb2-phy
> 
>  .../devicetree/bindings/phy/qoriq-usb2-phy.txt     | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/qoriq-usb2-phy.txt
> 
> diff --git a/Documentation/devicetree/bindings/phy/qoriq-usb2-phy.txt b/Documentation/devicetree/bindings/phy/qoriq-usb2-phy.txt
> new file mode 100644
> index 0000000..f043855
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qoriq-usb2-phy.txt
> @@ -0,0 +1,22 @@
> +QorIQ SoC USB 2.0 PHY
> +
> +Required properties:
> + - compatible: should be "fsl,qoriq-usb2-phy",
> +   Wherever applicable, the version of the USB PHY should
> +   also be mentioned (for eg. fsl,qoriq-usb2-phy-vX.Y).
> +   where, X = Phy vendor(Legacy = 1, NXP = 2) and Y = PHY version

What does Legacy mean? FSL?

Use SoC specific compatible strings.

> + - reg : Address and length of the usb phy control register set.
> + - phy_type : For multi port host USB controllers, should be one of
> +   "ulpi", or "serial". For dual role USB controllers, should be
> +   one of "ulpi", "utmi", "utmi_wide", or "serial".
> +
> +The main purpose of this PHY driver is to enable the USB PHY reference clock
> +gate on the QorIQ SOC for USB2 PHY OR implement errata workaround in
> +future. Otherwise it is just an NOP PHY driver.
> +
> +usbphy0: usbphy at 8600000 {
> +        compatible = "fsl,qoriq-usb2-phy" "fsl,qoriq-usb2-phy-vX.Y";

most specific first.

> +        reg = <0x0 0x8600000 0x0 0x1000>;
> +        #phy-cells = <0>;
> +        phy_type = "ulpi";
> +};

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2016-07-16  0:35 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1468038656-10345-1-git-send-email-rajesh.bhagat@nxp.com>
     [not found] ` <1468038656-10345-2-git-send-email-rajesh.bhagat@nxp.com>
2016-07-09  5:53   ` [PATCH v2 1/5] drivers: usb: chipidea: Add qoriq platform driver kbuild test robot
2016-07-09  5:59   ` kbuild test robot
2016-07-09  6:13   ` kbuild test robot
2016-07-11  6:43   ` Peter Chen
2016-07-12  3:59     ` Rajesh Bhagat
2016-07-15  7:13       ` Peter Chen
2016-07-15  8:13         ` Rajesh Bhagat
     [not found] ` <1468038656-10345-3-git-send-email-rajesh.bhagat@nxp.com>
2016-07-11  6:48   ` [PATCH v2 2/5] usb: DT binding documentation for qoriq usb 2.0 controller Peter Chen
2016-07-12  3:59     ` Rajesh Bhagat
2016-07-15  7:15       ` Peter Chen
2016-07-15  8:13         ` Rajesh Bhagat
2016-07-16  0:28   ` Rob Herring
     [not found] ` <1468038656-10345-4-git-send-email-rajesh.bhagat@nxp.com>
2016-07-11  6:54   ` [PATCH v2 3/5] drivers: usb: phy: Add qoriq usb 2.0 phy driver support Peter Chen
2016-07-12  3:59     ` Rajesh Bhagat
     [not found] ` <1468038656-10345-5-git-send-email-rajesh.bhagat@nxp.com>
2016-07-16  0:35   ` [PATCH v2 4/5] phy: DT binding documentation for qoriq usb 2.0 phy Rob Herring

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).