[PATCH v2 2/8] usb: phy: omap: Add new device types and remove omap_control_usb3_phy_power()
Benoit Cousson
bcousson at baylibre.com
Thu Aug 15 09:58:57 EDT 2013
Hi Roger,
On 15/08/2013 15:15, Roger Quadros wrote:
> Add support for new device types and in the process rid of "ti,type"
> device tree property. The correct type of device will be determined
> from the compatible string instead.
>
> Introduce a compatible string for each device type. At the moment
> we support 4 types Mailbox, USB2, USB3 and DRA7.
>
> Update DT binding information to reflect these changes.
>
> Also get rid of omap_control_usb3_phy_power(). Just one function
> i.e. omap_control_usb_phy_power() will now take care of all PHY types.
>
> Signed-off-by: Roger Quadros <rogerq at ti.com>
> ---
> Documentation/devicetree/bindings/usb/omap-usb.txt | 29 ++--
> drivers/usb/phy/phy-omap-control.c | 148 ++++++++++++--------
> drivers/usb/phy/phy-omap-usb2.c | 4 +
> drivers/usb/phy/phy-omap-usb3.c | 6 +-
> include/linux/usb/omap_control_usb.h | 24 ++--
> 5 files changed, 122 insertions(+), 89 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/usb/omap-usb.txt b/Documentation/devicetree/bindings/usb/omap-usb.txt
> index 57e71f6..1c6b54a 100644
> --- a/Documentation/devicetree/bindings/usb/omap-usb.txt
> +++ b/Documentation/devicetree/bindings/usb/omap-usb.txt
> @@ -73,22 +73,23 @@ omap_dwc3 {
> OMAP CONTROL USB
>
> Required properties:
> - - compatible: Should be "ti,omap-control-usb"
> + - compatible: Should be one of
> + "ti,omap-control-usb" - if it has otghs_control mailbox register
> + e.g. on OMAP4.
How generic is that one? I mean if this is applicable for OMAP4 only,
you'd better name it omap4-control-usb.
> + "ti,usb2-control-usb" - if it has Power down bit in control_dev_conf register
> + e.g. USB2_PHY on OMAP5.
> + "ti,usb3-control-usb" - if it has DPLL and individual Rx & Tx power control
> + e.g. USB3 PHY and SATA PHY on OMAP5.
> + "ti,dra7-control-usb" - if it has both power down and power aux registers
> + e.g. USB2 PHY on DRA7 platform.
> +
> - reg : Address and length of the register set for the device. It contains
> - the address of "control_dev_conf" and "otghs_control" or "phy_power_usb"
> - depending upon omap4 or omap5.
> - - reg-names: The names of the register addresses corresponding to the registers
> - filled in "reg".
> - - ti,type: This is used to differentiate whether the control module has
> - usb mailbox or usb3 phy power. omap4 has usb mailbox in control module to
> - notify events to the musb core and omap5 has usb3 phy power register to
> - power on usb3 phy. Should be "1" if it has mailbox and "2" if it has usb3
> - phy power.
> + the address of "otghs_control" for type 1 or "power" register for other types.
> + For type 4, it must also contain "power_aux" register.
> + - reg-names: should be otghs_control for type 1 and "power" for other types.
type1 and 4 are less obvious now that you have name, you should be more
explicit and name them directly.
>
> omap_control_usb: omap-control-usb at 4a002300 {
> compatible = "ti,omap-control-usb";
> - reg = <0x4a002300 0x4>,
> - <0x4a00233c 0x4>;
> - reg-names = "control_dev_conf", "otghs_control";
> - ti,type = <1>;
> + reg = <0x4a00233c 0x4>;
> + reg-names = "otghs_control";
> };
> diff --git a/drivers/usb/phy/phy-omap-control.c b/drivers/usb/phy/phy-omap-control.c
> index 3b9ee83..078c46f 100644
> --- a/drivers/usb/phy/phy-omap-control.c
> +++ b/drivers/usb/phy/phy-omap-control.c
> @@ -46,61 +46,76 @@ struct device *omap_get_control_dev(void)
> EXPORT_SYMBOL_GPL(omap_get_control_dev);
>
> /**
> - * omap_control_usb3_phy_power - power on/off the serializer using control
> - * module
> + * omap_control_usb_phy_power - power on/off the phy using control module reg
> * @dev: the control module device
> - * @on: 0 to off and 1 to on based on powering on or off the PHY
> - *
> - * usb3 PHY driver should call this API to power on or off the PHY.
> + * @on: 0 or 1, based on powering on or off the PHY
> */
> -void omap_control_usb3_phy_power(struct device *dev, bool on)
> +void omap_control_usb_phy_power(struct device *dev, int on)
> {
> - u32 val;
> + u32 val, val_aux;
> unsigned long rate;
> - struct omap_control_usb *control_usb = dev_get_drvdata(dev);
> + struct omap_control_usb *control_usb;
>
> - if (control_usb->type != OMAP_CTRL_DEV_TYPE2)
> + if (IS_ERR(dev) || !dev) {
> + pr_err("%s: invalid device\n", __func__);
> return;
> + }
>
> - rate = clk_get_rate(control_usb->sys_clk);
> - rate = rate/1000000;
> + control_usb = dev_get_drvdata(dev);
> + if (!control_usb) {
> + dev_err(dev, "%s: invalid control usb device\n", __func__);
> + return;
> + }
>
> - val = readl(control_usb->phy_power);
> + if (control_usb->type == OMAP_CTRL_TYPE_OMAP)
> + return;
>
> - if (on) {
> - val &= ~(OMAP_CTRL_USB_PWRCTL_CLK_CMD_MASK |
> - OMAP_CTRL_USB_PWRCTL_CLK_FREQ_MASK);
> - val |= OMAP_CTRL_USB3_PHY_TX_RX_POWERON <<
> - OMAP_CTRL_USB_PWRCTL_CLK_CMD_SHIFT;
> - val |= rate << OMAP_CTRL_USB_PWRCTL_CLK_FREQ_SHIFT;
> - } else {
> - val &= ~OMAP_CTRL_USB_PWRCTL_CLK_CMD_MASK;
> - val |= OMAP_CTRL_USB3_PHY_TX_RX_POWEROFF <<
> - OMAP_CTRL_USB_PWRCTL_CLK_CMD_SHIFT;
> - }
> + val = readl(control_usb->power);
>
> - writel(val, control_usb->phy_power);
> -}
> -EXPORT_SYMBOL_GPL(omap_control_usb3_phy_power);
> + switch (control_usb->type) {
> + case OMAP_CTRL_TYPE_USB2:
> + if (on)
> + val &= ~OMAP_CTRL_DEV_PHY_PD;
> + else
> + val |= OMAP_CTRL_DEV_PHY_PD;
> + break;
>
> -/**
> - * 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);
> + case OMAP_CTRL_TYPE_USB3:
> + rate = clk_get_rate(control_usb->sys_clk);
> + rate = rate/1000000;
> +
> + if (on) {
> + val &= ~(OMAP_CTRL_USB_PWRCTL_CLK_CMD_MASK |
> + OMAP_CTRL_USB_PWRCTL_CLK_FREQ_MASK);
> + val |= OMAP_CTRL_USB3_PHY_TX_RX_POWERON <<
> + OMAP_CTRL_USB_PWRCTL_CLK_CMD_SHIFT;
> + val |= rate << OMAP_CTRL_USB_PWRCTL_CLK_FREQ_SHIFT;
> + } else {
> + val &= ~OMAP_CTRL_USB_PWRCTL_CLK_CMD_MASK;
> + val |= OMAP_CTRL_USB3_PHY_TX_RX_POWEROFF <<
> + OMAP_CTRL_USB_PWRCTL_CLK_CMD_SHIFT;
> + }
> + break;
>
> - val = readl(control_usb->dev_conf);
> + case OMAP_CTRL_TYPE_DRA7:
> + val_aux = readl(control_usb->power_aux);
> + if (on) {
> + val &= ~OMAP_CTRL_USB2_PHY_PD;
> + val_aux |= 0x100;
> + } else {
> + val |= OMAP_CTRL_USB2_PHY_PD;
> + val_aux &= ~0x100;
> + }
>
> - if (on)
> - val &= ~OMAP_CTRL_DEV_PHY_PD;
> - else
> - val |= OMAP_CTRL_DEV_PHY_PD;
> + writel(val_aux, control_usb->power_aux);
> + break;
> + default:
> + dev_err(dev, "%s: type %d not recognized\n",
> + __func__, control_usb->type);
> + break;
> + }
>
> - writel(val, control_usb->dev_conf);
> + writel(val, control_usb->power);
> }
> EXPORT_SYMBOL_GPL(omap_control_usb_phy_power);
>
> @@ -172,7 +187,7 @@ void omap_control_usb_set_mode(struct device *dev,
> {
> struct omap_control_usb *ctrl_usb;
>
> - if (IS_ERR(dev) || control_usb->type != OMAP_CTRL_DEV_TYPE1)
> + if (IS_ERR(dev) || control_usb->type != OMAP_CTRL_TYPE_OMAP)
> return;
>
> ctrl_usb = dev_get_drvdata(dev);
> @@ -198,9 +213,7 @@ static int omap_control_usb_probe(struct platform_device *pdev)
> struct resource *res;
> struct device_node *np = pdev->dev.of_node;
>
> - if (np) {
> - of_property_read_u32(np, "ti,type", &control_usb->type);
> - } else {
> + if (!np) {
> /* We only support DT boot */
> return -ENODEV;
> }
> @@ -212,31 +225,34 @@ static int omap_control_usb_probe(struct platform_device *pdev)
> return -ENOMEM;
> }
>
> - control_usb->dev = &pdev->dev;
> + control_usb->dev = &pdev->dev;
> + control_usb->type = OMAP_CTRL_TYPE_OMAP;
>
> - res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> - "control_dev_conf");
> - control_usb->dev_conf = devm_ioremap_resource(&pdev->dev, res);
> - if (IS_ERR(control_usb->dev_conf))
> - return PTR_ERR(control_usb->dev_conf);
> + if (of_device_is_compatible(np, "ti,usb2-control-usb"))
> + control_usb->type = OMAP_CTRL_TYPE_USB2;
> + else if (of_device_is_compatible(np, "ti,usb3-control-usb"))
> + control_usb->type = OMAP_CTRL_TYPE_USB3;
> + else if (of_device_is_compatible(np, "ti,dra7-control-usb"))
> + control_usb->type = OMAP_CTRL_TYPE_DRA7;
You can avoid that by adding the type directly in the data field of the
omap_control_usb_id_table.
It will be more readable and scalable.
This how it was done on gpio-omap for example.
>
> - if (control_usb->type == OMAP_CTRL_DEV_TYPE1) {
> + if (control_usb->type == OMAP_CTRL_TYPE_OMAP) {
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> "otghs_control");
> control_usb->otghs_control = devm_ioremap_resource(
> &pdev->dev, res);
> if (IS_ERR(control_usb->otghs_control))
> return PTR_ERR(control_usb->otghs_control);
> - }
> -
> - if (control_usb->type == OMAP_CTRL_DEV_TYPE2) {
> + } else {
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> - "phy_power_usb");
> - control_usb->phy_power = devm_ioremap_resource(
> - &pdev->dev, res);
> - if (IS_ERR(control_usb->phy_power))
> - return PTR_ERR(control_usb->phy_power);
> + "power");
> + control_usb->power = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(control_usb->power)) {
> + dev_err(&pdev->dev, "Couldn't get power register\n");
> + return PTR_ERR(control_usb->power);
> + }
> + }
>
> + if (control_usb->type == OMAP_CTRL_TYPE_USB3) {
> control_usb->sys_clk = devm_clk_get(control_usb->dev,
> "sys_clkin");
> if (IS_ERR(control_usb->sys_clk)) {
> @@ -245,6 +261,15 @@ static int omap_control_usb_probe(struct platform_device *pdev)
> }
> }
>
> + if (control_usb->type == OMAP_CTRL_TYPE_DRA7) {
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> + "power_aux");
> + control_usb->power_aux = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(control_usb->power_aux)) {
> + dev_err(&pdev->dev, "Couldn't get power_aux register\n");
> + return PTR_ERR(control_usb->power_aux);
> + }
> + }
>
> dev_set_drvdata(control_usb->dev, control_usb);
>
> @@ -254,6 +279,9 @@ static int omap_control_usb_probe(struct platform_device *pdev)
> #ifdef CONFIG_OF
> static const struct of_device_id omap_control_usb_id_table[] = {
> { .compatible = "ti,omap-control-usb" },
> + { .compatible = "ti,usb2-control-usb" },
> + { .compatible = "ti,usb3-control-usb" },
> + { .compatible = "ti,dra7-control-usb" },
> {}
In that table you can add a data field with the proper type enum.
Regards,
Benoit
> };
> MODULE_DEVICE_TABLE(of, omap_control_usb_id_table);
> diff --git a/drivers/usb/phy/phy-omap-usb2.c b/drivers/usb/phy/phy-omap-usb2.c
> index 844ab68..376b367 100644
> --- a/drivers/usb/phy/phy-omap-usb2.c
> +++ b/drivers/usb/phy/phy-omap-usb2.c
> @@ -123,6 +123,10 @@ static int omap_usb2_probe(struct platform_device *pdev)
> {
> struct omap_usb *phy;
> struct usb_otg *otg;
> + struct device_node *node = pdev->dev.of_node;
> +
> + if (!node)
> + return -ENODEV;
>
> phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
> if (!phy) {
> diff --git a/drivers/usb/phy/phy-omap-usb3.c b/drivers/usb/phy/phy-omap-usb3.c
> index fc15694..4a7f27c 100644
> --- a/drivers/usb/phy/phy-omap-usb3.c
> +++ b/drivers/usb/phy/phy-omap-usb3.c
> @@ -100,7 +100,7 @@ static int omap_usb3_suspend(struct usb_phy *x, int suspend)
> udelay(1);
> } while (--timeout);
>
> - omap_control_usb3_phy_power(phy->control_dev, 0);
> + omap_control_usb_phy_power(phy->control_dev, 0);
>
> phy->is_suspended = 1;
> } else if (!suspend && phy->is_suspended) {
> @@ -189,7 +189,7 @@ static int omap_usb3_init(struct usb_phy *x)
> if (ret)
> return ret;
>
> - omap_control_usb3_phy_power(phy->control_dev, 1);
> + omap_control_usb_phy_power(phy->control_dev, 1);
>
> return 0;
> }
> @@ -245,7 +245,7 @@ static int omap_usb3_probe(struct platform_device *pdev)
> return -ENODEV;
> }
>
> - omap_control_usb3_phy_power(phy->control_dev, 0);
> + omap_control_usb_phy_power(phy->control_dev, 0);
> usb_add_phy_dev(&phy->phy);
>
> platform_set_drvdata(pdev, phy);
> diff --git a/include/linux/usb/omap_control_usb.h b/include/linux/usb/omap_control_usb.h
> index e2416b4..9eb6f9ca 100644
> --- a/include/linux/usb/omap_control_usb.h
> +++ b/include/linux/usb/omap_control_usb.h
> @@ -19,16 +19,23 @@
> #ifndef __OMAP_CONTROL_USB_H__
> #define __OMAP_CONTROL_USB_H__
>
> +enum omap_control_usb_type {
> + OMAP_CTRL_TYPE_OMAP = 1, /* Mailbox OTGHS_CONTROL */
> + OMAP_CTRL_TYPE_USB2, /* USB2_PHY, power down in CONTROL_DEV_CONF */
> + OMAP_CTRL_TYPE_USB3, /* USB3_PHY, DPLL & seperate Rx/Tx power */
> + OMAP_CTRL_TYPE_DRA7, /* USB2 PHY, power and power_aux e.g. DRA7 */
> +};
> +
> struct omap_control_usb {
> struct device *dev;
>
> - u32 __iomem *dev_conf;
> u32 __iomem *otghs_control;
> - u32 __iomem *phy_power;
> + u32 __iomem *power;
> + u32 __iomem *power_aux;
>
> struct clk *sys_clk;
>
> - u32 type;
> + enum omap_control_usb_type type;
> };
>
> enum omap_control_usb_mode {
> @@ -38,10 +45,6 @@ enum omap_control_usb_mode {
> USB_MODE_DISCONNECT,
> };
>
> -/* To differentiate ctrl module IP having either mailbox or USB3 PHY power */
> -#define OMAP_CTRL_DEV_TYPE1 0x1
> -#define OMAP_CTRL_DEV_TYPE2 0x2
> -
> #define OMAP_CTRL_DEV_PHY_PD BIT(0)
>
> #define OMAP_CTRL_DEV_AVALID BIT(0)
> @@ -59,10 +62,11 @@ enum omap_control_usb_mode {
> #define OMAP_CTRL_USB3_PHY_TX_RX_POWERON 0x3
> #define OMAP_CTRL_USB3_PHY_TX_RX_POWEROFF 0x0
>
> +#define OMAP_CTRL_USB2_PHY_PD BIT(28)
> +
> #if IS_ENABLED(CONFIG_OMAP_CONTROL_USB)
> extern struct device *omap_get_control_dev(void);
> extern void omap_control_usb_phy_power(struct device *dev, int on);
> -extern void omap_control_usb3_phy_power(struct device *dev, bool on);
> extern void omap_control_usb_set_mode(struct device *dev,
> enum omap_control_usb_mode mode);
> #else
> @@ -75,10 +79,6 @@ static inline void omap_control_usb_phy_power(struct device *dev, int on)
> {
> }
>
> -static inline void omap_control_usb3_phy_power(struct device *dev, int on)
> -{
> -}
> -
> static inline void omap_control_usb_set_mode(struct device *dev,
> enum omap_control_usb_mode mode)
> {
>
More information about the linux-arm-kernel
mailing list