Re: [PATCH 1/4] drivers: usb: phy: add a new driver for usb part of control module

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Fri, Jan 18, 2013 at 03:10:42PM +0530, Kishon Vijay Abraham I wrote:
> Added a new driver for the usb part of control module. This has an API
> to power on the USB2 phy and an API to write to the mailbox depending on
> whether MUSB has to act in host mode or in device mode.
> 
> Writing to control module registers for doing the above task which was
> previously done in omap glue and in omap-usb2 phy will be removed.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@xxxxxx>
> ---
>  Documentation/devicetree/bindings/usb/omap-usb.txt |   26 ++-
>  Documentation/devicetree/bindings/usb/usb-phy.txt  |    5 +
>  drivers/usb/phy/Kconfig                            |    9 +
>  drivers/usb/phy/Makefile                           |    1 +
>  drivers/usb/phy/omap-control-usb.c                 |  204 ++++++++++++++++++++
>  include/linux/usb/omap_control_usb.h               |   72 +++++++
>  6 files changed, 316 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/usb/phy/omap-control-usb.c
>  create mode 100644 include/linux/usb/omap_control_usb.h
> 
> diff --git a/Documentation/devicetree/bindings/usb/omap-usb.txt b/Documentation/devicetree/bindings/usb/omap-usb.txt
> index 29a043e..ead6ba9 100644
> --- a/Documentation/devicetree/bindings/usb/omap-usb.txt
> +++ b/Documentation/devicetree/bindings/usb/omap-usb.txt
> @@ -1,4 +1,4 @@
> -OMAP GLUE
> +OMAP GLUE AND OTHER OMAP SPECIFIC COMPONENTS
>  
>  OMAP MUSB GLUE
>   - compatible : Should be "ti,omap4-musb" or "ti,omap3-musb"
> @@ -16,6 +16,10 @@ OMAP MUSB GLUE
>   - power : Should be "50". This signifies the controller can supply upto
>     100mA when operating in host mode.
>  
> +Optional properties:
> + - ctrl_module : phandle of the control module this glue uses to write to
> +   mailbox
> +
>  SOC specific device node entry
>  usb_otg_hs: usb_otg_hs@4a0ab000 {
>  	compatible = "ti,omap4-musb";
> @@ -23,6 +27,7 @@ usb_otg_hs: usb_otg_hs@4a0ab000 {
>  	multipoint = <1>;
>  	num_eps = <16>;
>  	ram_bits = <12>;
> +	ctrl_module = <&omap_control_usb>;
>  };
>  
>  Board specific device node entry
> @@ -31,3 +36,22 @@ Board specific device node entry
>  	mode = <3>;
>  	power = <50>;
>  };
> +
> +OMAP CONTROL USB
> +
> +Required properties:
> + - compatible: Should be "ti,omap-control-usb"
> + - reg : Address and length of the register set for the device. It contains
> +   the address of "control_dev_conf" and "otghs_control".
> + - reg-names: The names of the register addresses corresponding to the registers
> +   filled in "reg".
> + - ti,has_mailbox: This is used to specify if the platform has mailbox in
> +   control module.
> +
> +omap_control_usb: omap-control-usb@4a002300 {
> +	compatible = "ti,omap-control-usb";
> +	reg = <0x4a002300 0x4>,
> +	      <0x4a00233c 0x4>;
> +	reg-names = "control_dev_conf", "otghs_control";
> +	ti,has_mailbox;
> +};
> diff --git a/Documentation/devicetree/bindings/usb/usb-phy.txt b/Documentation/devicetree/bindings/usb/usb-phy.txt
> index 80d4148..2466b6f 100644
> --- a/Documentation/devicetree/bindings/usb/usb-phy.txt
> +++ b/Documentation/devicetree/bindings/usb/usb-phy.txt
> @@ -8,10 +8,15 @@ Required properties:
>  add the address of control module dev conf register until a driver for
>  control module is added
>  
> +Optional properties:
> + - ctrl_module : phandle of the control module used by PHY driver to power on
> +   the PHY.
> +
>  This is usually a subnode of ocp2scp to which it is connected.
>  
>  usb2phy@4a0ad080 {
>  	compatible = "ti,omap-usb2";
>  	reg = <0x4a0ad080 0x58>,
>  	      <0x4a002300 0x4>;
> +	ctrl_module = <&omap_control_usb>;
>  };
> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
> index 5de6e7f..a7277ee 100644
> --- a/drivers/usb/phy/Kconfig
> +++ b/drivers/usb/phy/Kconfig
> @@ -14,6 +14,15 @@ config OMAP_USB2
>  	  The USB OTG controller communicates with the comparator using this
>  	  driver.
>  
> +config OMAP_CONTROL_USB
> +	tristate "OMAP CONTROL USB Driver"
> +	depends on ARCH_OMAP2PLUS
> +	help
> +	  Enable this to add support for the USB part present in the control
> +	  module. This driver has API to power on the PHY and to write to the
> +	  mailbox. The mailbox is present only in omap4 and the register to
> +	  power on the PHY is present in omap4 and omap5.
> +
>  config USB_ISP1301
>  	tristate "NXP ISP1301 USB transceiver support"
>  	depends on USB || USB_GADGET
> diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
> index 1a579a8..0dea4d2 100644
> --- a/drivers/usb/phy/Makefile
> +++ b/drivers/usb/phy/Makefile
> @@ -5,6 +5,7 @@
>  ccflags-$(CONFIG_USB_DEBUG) := -DDEBUG
>  
>  obj-$(CONFIG_OMAP_USB2)			+= omap-usb2.o
> +obj-$(CONFIG_OMAP_CONTROL_USB)		+= omap-control-usb.o
>  obj-$(CONFIG_USB_ISP1301)		+= isp1301.o
>  obj-$(CONFIG_MV_U3D_PHY)		+= mv_u3d_phy.o
>  obj-$(CONFIG_USB_EHCI_TEGRA)	+= tegra_usb_phy.o
> diff --git a/drivers/usb/phy/omap-control-usb.c b/drivers/usb/phy/omap-control-usb.c
> new file mode 100644
> index 0000000..7edeb41
> --- /dev/null
> +++ b/drivers/usb/phy/omap-control-usb.c
> @@ -0,0 +1,204 @@
> +/*
> + * omap-control-usb.c - The USB part of control module.
> + *
> + * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * Author: Kishon Vijay Abraham I <kishon@xxxxxx>
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/of.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/usb/omap_control_usb.h>
> +
> +static struct omap_control_usb *control_usb;
> +
> +/**
> + * get_omap_control_dev - returns the device pointer for this control device
> + *
> + * This API should be called to get the device pointer for this control
> + * module device. This device pointer should be passed to all other API's
> + * in this driver.
> + *
> + * To be used by PHY driver and glue driver
> + */
> +struct device *get_omap_control_dev(void)

let's do a small rename here and call it:

omap_get_control_dev(), just to keep consistency with rest of the file.

> +{
> +	if (!control_usb)
> +		return ERR_PTR(-ENODEV);
> +
> +	return control_usb->dev;
> +}
> +EXPORT_SYMBOL_GPL(get_omap_control_dev);
> +
> +/**
> + * omap_control_usb_phy_power - power on/off the phy using control module reg
> + * @dev: the control module device
> + * @on: 0 or 1, based on powering on or off the PHY
> + */
> +void omap_control_usb_phy_power(struct device *dev, int on)
> +{
> +	u32 val;
> +	struct omap_control_usb	*control_usb = dev_get_drvdata(dev);
> +
> +	if (on) {
> +		val = readl(control_usb->dev_conf);
> +		if (val & PHY_PD)
> +			writel(~PHY_PD, control_usb->dev_conf);
> +	} else {
> +		writel(PHY_PD, control_usb->dev_conf);
> +	}

this can be better. I would rather have this:

struct omap_control_usb *ctrl = dev_get_drvdata(dev);
u32 val;

val = readl(ctrl->dev_conf);

if (on)
	val &= ~OMAP_CTRL_DEV_PHY_PD;
else
	val |= OMAP_CTRL_DEV_PHY_PD;

writel(val, ctrl->dev_conf);



> +}
> +EXPORT_SYMBOL_GPL(omap_control_usb_phy_power);
> +
> +/**
> + * omap_control_usb_host_mode - set AVALID, VBUSVALID and ID pin in grounded
> + * @dev: struct device *
> + *
> + * This is an API to write to the mailbox register to notify the usb core that
> + * a usb device has been connected.
> + */
> +void omap_control_usb_host_mode(struct device *dev)
> +{
> +	u32 val;
> +	struct omap_control_usb	*control_usb = dev_get_drvdata(dev);
> +
> +	val = AVALID | VBUSVALID;
> +
> +	writel(val, control_usb->otghs_control);

I would like to make this future proof too:

val = readl(ctrl->otghs_control);
val |= OMAP_CTRL_DEV_AVALID | OMAP_CTRL_DEV_VBUSVALID;
writel(val, ctrl->otghs_control);

> +void omap_control_usb_device_mode(struct device *dev)
> +{
> +	u32 val;
> +	struct omap_control_usb	*control_usb = dev_get_drvdata(dev);
> +
> +	val = IDDIG | AVALID | VBUSVALID;

ditto. Read it before changing it. (the idea is that in future the other
bits might be used for something else, so you risk breaking future
functionality).

> +
> +	writel(val, control_usb->otghs_control);
> +}
> +EXPORT_SYMBOL_GPL(omap_control_usb_device_mode);
> +
> +/**
> + * omap_control_usb_set_sessionend - Enable SESSIONEND and IDIG to high
> + * impedance
> + * @dev: struct device *
> + *
> + * This is an API to write to the mailbox register to notify the usb core
> + * it's now in disconnected state.
> + */
> +void omap_control_usb_set_sessionend(struct device *dev)
> +{
> +	u32 val;
> +	struct omap_control_usb	*control_usb = dev_get_drvdata(dev);
> +
> +	val = SESSEND | IDDIG;

ditto

> +
> +	writel(val, control_usb->otghs_control);
> +}
> +EXPORT_SYMBOL_GPL(omap_control_usb_set_sessionend);
> +
> +static int omap_control_usb_probe(struct platform_device *pdev)
> +{
> +	struct resource	*res;
> +	struct device_node *np = pdev->dev.of_node;
> +	struct omap_control_usb_platform_data *pdata = pdev->dev.platform_data;
> +
> +	control_usb = devm_kzalloc(&pdev->dev, sizeof(*control_usb),
> +	    GFP_KERNEL);

you're using space indentation here.

> +	if (!control_usb) {
> +		dev_err(&pdev->dev, "unable to alloc memory for control usb\n");
> +		return -ENOMEM;
> +	}
> +
> +	if (np) {
> +		control_usb->has_mailbox = of_property_read_bool(np,
> +		    "ti,has_mailbox");
> +	} else if (pdata) {
> +		control_usb->has_mailbox = pdata->has_mailbox;
> +	} else {
> +		dev_err(&pdev->dev, "no pdata present\n");
> +		return -EINVAL;

does it make sense to default to has_mailbox as true ? I mean, seems
like this is how it's going to be in future devices looking at the
current trend.

Another idea is to try to get the extra resource below, if it fails you
try to continue without it ;-)

> +	}
> +
> +	control_usb->dev	= &pdev->dev;
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> +	    "control_dev_conf");

using space indentation here again.

> +	control_usb->dev_conf = devm_request_and_ioremap(&pdev->dev, res);
> +	if (control_usb->dev_conf == NULL) {
> +		dev_err(&pdev->dev, "Failed to obtain io memory\n");
> +		return -EADDRNOTAVAIL;
> +	}
> +
> +	if (control_usb->has_mailbox) {
> +		res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> +		    "otghs_control");
> +		control_usb->otghs_control = devm_request_and_ioremap(
> +		    &pdev->dev, res);
> +		if (control_usb->otghs_control == NULL) {
> +			dev_err(&pdev->dev, "Failed to obtain io memory\n");
> +			return -EADDRNOTAVAIL;
> +		}
> +	}
> +
> +	dev_set_drvdata(control_usb->dev, control_usb);
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id omap_control_usb_id_table[] = {
> +	{ .compatible = "ti,omap-control-usb" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, omap_control_usb_id_table);
> +#endif
> +
> +static struct platform_driver omap_control_usb_driver = {
> +	.probe		= omap_control_usb_probe,
> +	.driver		= {
> +		.name	= "omap-control-usb",
> +		.owner	= THIS_MODULE,

no need to set owner here. When you register the platform_driver, it
will be set for you.

> +		.of_match_table = of_match_ptr(omap_control_usb_id_table),
> +	},
> +};
> +
> +static int __init omap_control_usb_init(void)
> +{
> +	return platform_driver_register(&omap_control_usb_driver);
> +}
> +subsys_initcall(omap_control_usb_init);
> +
> +static void __exit omap_control_usb_exit(void)
> +{
> +	platform_driver_unregister(&omap_control_usb_driver);
> +}
> +module_exit(omap_control_usb_exit);
> +
> +MODULE_ALIAS("platform: omap_control_usb");
> +MODULE_AUTHOR("Texas Instruments Inc.");
> +MODULE_DESCRIPTION("OMAP CONTROL USB DRIVER");

don't scream to your users like that ;-)

MODULE_DESCRIPTION("OMAP Control Module USB Driver");

> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/usb/omap_control_usb.h b/include/linux/usb/omap_control_usb.h
> new file mode 100644
> index 0000000..a16f993
> --- /dev/null
> +++ b/include/linux/usb/omap_control_usb.h
> @@ -0,0 +1,72 @@
> +/*
> + * omap_control_usb.h - Header file for the USB part of control module.
> + *
> + * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * Author: Kishon Vijay Abraham I <kishon@xxxxxx>
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#ifndef __OMAP_CONTROL_USB_H__
> +#define __OMAP_CONTROL_USB_H__
> +
> +struct omap_control_usb {
> +	struct device *dev;
> +
> +	u32 __iomem *dev_conf;
> +	u32 __iomem *otghs_control;
> +
> +	u8 has_mailbox:1;
> +};
> +
> +struct omap_control_usb_platform_data {
> +	u8 has_mailbox:1;
> +};
> +
> +#define	PHY_PD		BIT(0)
> +
> +#define	AVALID		BIT(0)
> +#define	BVALID		BIT(1)
> +#define	VBUSVALID	BIT(2)
> +#define	SESSEND		BIT(3)
> +#define	IDDIG		BIT(4)

please prepend with OMAP_CTRL_ or something similar.

-- 
balbi

Attachment: signature.asc
Description: Digital signature


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux