2014-06-24 10:35:28

by Antoine Tenart

[permalink] [raw]
Subject: [PATCH v2 00/12] ARM: berlin: USB support

This series adds the support for generic ChipIdea USB controllers,
the USB PHYs of the Marvell Berlin SoCs and also adds a reset
controller for these SoCs.

The reset controller is used by the USB PHY driver and shares the
existing chip controller node with the clocks and one pin controller.

The Marvell Berlin USB controllers are host only on the BG2Q and are
compatible with USB ChipIdea. We here add a glue to use the available
common functions for this kind of controllers, and add a generic
driver. A USB PHY driver is also added to control the PHY.

Changes since v1:
- made the Berlin CI USB driver a generic one
- added support to custom offset for the reset register
- added fixed regulators to support supply the VBUS
- modified the PHY driver to support the one one the BG2CD as
well
- documented the reset properties
- added bindings for the BG2CD
- cosmetic fixes

Antoine Ténart (10):
reset: add the Berlin reset controller driver
Documentation: bindings: add reset bindings docs for Marvell Berlin
SoCs
ARM: Berlin: select the reset controller
ARM: dts: berlin: add a required reset property in the chip controller
node
usb: phy: add the Berlin USB PHY driver
Documentation: bindings: add doc for the Berlin USB PHY
usb: chipidea: add a generic driver
Documentation: bindings: add doc for the generic ChipIdea USB driver
ARM: dts: berlin: add BG2Q nodes for USB support
ARM: dts: Berlin: enable USB on the BG2Q DMP

Sebastian Hesselbarth (2):
ARM: dts: berlin: add BG2CD nodes for USB support
ARM: dts: berlin: enable USB on the Google Chromecast

.../devicetree/bindings/arm/marvell,berlin.txt | 10 +
.../devicetree/bindings/usb/berlin-usb-phy.txt | 16 ++
.../devicetree/bindings/usb/ci-hdrc-generic.txt | 22 +++
arch/arm/boot/dts/berlin2.dtsi | 1 +
arch/arm/boot/dts/berlin2cd-google-chromecast.dts | 4 +
arch/arm/boot/dts/berlin2cd.dtsi | 35 ++++
arch/arm/boot/dts/berlin2q-marvell-dmp.dts | 53 ++++++
arch/arm/boot/dts/berlin2q.dtsi | 53 ++++++
arch/arm/mach-berlin/Kconfig | 2 +
drivers/reset/Makefile | 1 +
drivers/reset/reset-berlin.c | 131 +++++++++++++
drivers/usb/chipidea/Makefile | 1 +
drivers/usb/chipidea/ci_hdrc_generic.c | 108 +++++++++++
drivers/usb/phy/Kconfig | 9 +
drivers/usb/phy/Makefile | 1 +
drivers/usb/phy/phy-berlin-usb.c | 211 +++++++++++++++++++++
16 files changed, 658 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/berlin-usb-phy.txt
create mode 100644 Documentation/devicetree/bindings/usb/ci-hdrc-generic.txt
create mode 100644 drivers/reset/reset-berlin.c
create mode 100644 drivers/usb/chipidea/ci_hdrc_generic.c
create mode 100644 drivers/usb/phy/phy-berlin-usb.c

--
1.9.1


2014-06-24 10:35:33

by Antoine Tenart

[permalink] [raw]
Subject: [PATCH v2 08/12] Documentation: bindings: add doc for the generic ChipIdea USB driver

Document the generic ChipIdea USB driver bindings.

Signed-off-by: Antoine Ténart <[email protected]>
---
.../devicetree/bindings/usb/ci-hdrc-generic.txt | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/ci-hdrc-generic.txt

diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-generic.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-generic.txt
new file mode 100644
index 000000000000..4c85f9944077
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/ci-hdrc-generic.txt
@@ -0,0 +1,22 @@
+* Generic ChipIdea USB controller
+
+Required properties:
+- compatible: should be "chipidea-usb"
+- reg: base address and length of the registers
+- interrupts: interrupt for the USB controller
+
+Optional properties:
+- clocks: reference to the USB clock
+- usb-phy: reference to the USB PHY
+- vbus-supply: reference to the VBUS regulator
+
+Example:
+
+ usb@f7ed0000 {
+ compatible = "chipidea-usb";
+ reg = <0xf7ed0000 0x10000>;
+ interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&chip CLKID_USB0>;
+ usb-phy = <&usb_phy0>;
+ vbus-supply = <&reg_usb0_vbus>;
+ };
--
1.9.1

2014-06-24 10:35:32

by Antoine Tenart

[permalink] [raw]
Subject: [PATCH v2 03/12] ARM: Berlin: select the reset controller

The Marvell Berlin SoCs now has a reset controller. Add the needed
configuration.

Signed-off-by: Antoine Ténart <[email protected]>
---
arch/arm/mach-berlin/Kconfig | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/arm/mach-berlin/Kconfig b/arch/arm/mach-berlin/Kconfig
index 2631cfc5ab0d..3a4fd1e41801 100644
--- a/arch/arm/mach-berlin/Kconfig
+++ b/arch/arm/mach-berlin/Kconfig
@@ -1,11 +1,13 @@
menuconfig ARCH_BERLIN
bool "Marvell Berlin SoCs" if ARCH_MULTI_V7
+ select ARCH_HAS_RESET_CONTROLLER
select ARCH_REQUIRE_GPIOLIB
select ARM_GIC
select GENERIC_IRQ_CHIP
select DW_APB_ICTL
select DW_APB_TIMER_OF
select PINCTRL
+ select RESET_CONTROLLER

if ARCH_BERLIN

--
1.9.1

2014-06-24 10:35:49

by Antoine Tenart

[permalink] [raw]
Subject: [PATCH v2 11/12] ARM: dts: berlin: add BG2CD nodes for USB support

From: Sebastian Hesselbarth <[email protected]>

Adds nodes describing the Marvell Berlin BG2CD USB PHY and USB. The BG2CD
SoC has 2 USB ChipIdea controllers, with usb0 host-only and usb1 dual-role
capable.

Signed-off-by: Sebastian Hesselbarth <[email protected]>
---
arch/arm/boot/dts/berlin2cd.dtsi | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)

diff --git a/arch/arm/boot/dts/berlin2cd.dtsi b/arch/arm/boot/dts/berlin2cd.dtsi
index 68f7032b4686..d35c9fbaa397 100644
--- a/arch/arm/boot/dts/berlin2cd.dtsi
+++ b/arch/arm/boot/dts/berlin2cd.dtsi
@@ -66,6 +66,22 @@
clocks = <&chip CLKID_TWD>;
};

+ usb_phy0: usb-phy@b74000 {
+ compatible = "marvell,berlin2cd-usb-phy";
+ reg = <0xb74000 0x128>;
+ #phy-cells = <0>;
+ resets = <&chip 0x178 23>;
+ status = "disabled";
+ };
+
+ usb_phy1: usb-phy@b78000 {
+ compatible = "marvell,berlin2cd-usb-phy";
+ reg = <0xb78000 0x128>;
+ #phy-cells = <0>;
+ resets = <&chip 0x178 24>;
+ status = "disabled";
+ };
+
apb@e80000 {
compatible = "simple-bus";
#address-cells = <1>;
@@ -242,6 +258,24 @@
};
};

+ usb0: usb@ed0000 {
+ compatible = "chipidea-usb";
+ reg = <0xed0000 0x200>;
+ interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&chip CLKID_USB0>;
+ usb-phy = <&usb_phy0>;
+ status = "disabled";
+ };
+
+ usb1: usb@ee0000 {
+ compatible = "chipidea-usb";
+ reg = <0xee0000 0x200>;
+ interrupts = <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&chip CLKID_USB1>;
+ usb-phy = <&usb_phy1>;
+ status = "disabled";
+ };
+
apb@fc0000 {
compatible = "simple-bus";
#address-cells = <1>;
--
1.9.1

2014-06-24 10:35:48

by Antoine Tenart

[permalink] [raw]
Subject: [PATCH v2 12/12] ARM: dts: berlin: enable USB on the Google Chromecast

From: Sebastian Hesselbarth <[email protected]>

Enable usb1 on Google Chromecast which is connected to micro-USB
plug used for external power supply, too.

Signed-off-by: Sebastian Hesselbarth <[email protected]>
---
arch/arm/boot/dts/berlin2cd-google-chromecast.dts | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/berlin2cd-google-chromecast.dts b/arch/arm/boot/dts/berlin2cd-google-chromecast.dts
index bcd81ffc495d..5c42c3bfb613 100644
--- a/arch/arm/boot/dts/berlin2cd-google-chromecast.dts
+++ b/arch/arm/boot/dts/berlin2cd-google-chromecast.dts
@@ -27,3 +27,7 @@
};

&uart0 { status = "okay"; };
+
+&usb_phy1 { status = "okay"; };
+
+&usb1 { status = "okay"; };
--
1.9.1

2014-06-24 10:36:31

by Antoine Tenart

[permalink] [raw]
Subject: [PATCH v2 10/12] ARM: dts: Berlin: enable USB on the BG2Q DMP

Enable the 2 available USB PHY and USB nodes on the Marvell Berlin BG2Q
DMP.

Signed-off-by: Antoine Ténart <[email protected]>
---
arch/arm/boot/dts/berlin2q-marvell-dmp.dts | 53 ++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)

diff --git a/arch/arm/boot/dts/berlin2q-marvell-dmp.dts b/arch/arm/boot/dts/berlin2q-marvell-dmp.dts
index 995150f93795..0d515046f5f5 100644
--- a/arch/arm/boot/dts/berlin2q-marvell-dmp.dts
+++ b/arch/arm/boot/dts/berlin2q-marvell-dmp.dts
@@ -7,6 +7,8 @@
*/

/dts-v1/;
+
+#include <dt-bindings/gpio/gpio.h>
#include "berlin2q.dtsi"

/ {
@@ -21,6 +23,39 @@
choosen {
bootargs = "console=ttyS0,115200 earlyprintk";
};
+
+ regulators {
+ compatible = "simple-bus";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ reg_usb0_vbus: regulator@0 {
+ compatible = "regulator-fixed";
+ regulator-name = "usb0_vbus";
+ regulator-min-microvolt = <5000000>;
+ regulator-max-microvolt = <5000000>;
+ gpio = <&portb 8 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ };
+
+ reg_usb1_vbus: regulator@1 {
+ compatible = "regulator-fixed";
+ regulator-name = "usb1_vbus";
+ regulator-min-microvolt = <5000000>;
+ regulator-max-microvolt = <5000000>;
+ gpio = <&portb 10 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ };
+
+ reg_usb2_vbus: regulator@2 {
+ compatible = "regulator-fixed";
+ regulator-name = "usb2_vbus";
+ regulator-min-microvolt = <5000000>;
+ regulator-max-microvolt = <5000000>;
+ gpio = <&portb 12 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ };
+ };
};

&sdhci1 {
@@ -37,3 +72,21 @@
&uart0 {
status = "okay";
};
+
+&usb_phy0 {
+ status = "okay";
+};
+
+&usb_phy2 {
+ status = "okay";
+};
+
+&usb0 {
+ vbus-supply = <&reg_usb0_vbus>;
+ status = "okay";
+};
+
+&usb2 {
+ vbus-supply = <&reg_usb2_vbus>;
+ status = "okay";
+};
--
1.9.1

2014-06-24 10:36:51

by Antoine Tenart

[permalink] [raw]
Subject: [PATCH v2 09/12] ARM: dts: berlin: add BG2Q nodes for USB support

Adds nodes describing the Marvell Berlin BG2Q USB PHY and USB. The BG2Q
SoC has 3 USB host controller, compatible with ChipIdea.

Signed-off-by: Antoine Ténart <[email protected]>
---
arch/arm/boot/dts/berlin2q.dtsi | 52 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 52 insertions(+)

diff --git a/arch/arm/boot/dts/berlin2q.dtsi b/arch/arm/boot/dts/berlin2q.dtsi
index a6f7ec325200..0726ab891e0a 100644
--- a/arch/arm/boot/dts/berlin2q.dtsi
+++ b/arch/arm/boot/dts/berlin2q.dtsi
@@ -111,6 +111,39 @@
#interrupt-cells = <3>;
};

+ usb_phy2: usb-phy@a2f400 {
+ compatible = "marvell,berlin2-usb-phy";
+ reg = <0xa2f400 0x128>;
+ #phy-cells = <0>;
+ resets = <&chip 0x104 14>;
+ status = "disabled";
+ };
+
+ usb2: usb@a30000 {
+ compatible = "chipidea-usb";
+ reg = <0xa30000 0x10000>;
+ interrupts = <GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&chip CLKID_USB2>;
+ usb-phy = <&usb_phy2>;
+ status = "disabled";
+ };
+
+ usb_phy0: usb-phy@b74000 {
+ compatible = "marvell,berlin2-usb-phy";
+ reg = <0xb74000 0x128>;
+ #phy-cells = <0>;
+ resets = <&chip 0x104 12>;
+ status = "disabled";
+ };
+
+ usb_phy1: usb-phy@b78000 {
+ compatible = "marvell,berlin2-usb-phy";
+ reg = <0xb78000 0x128>;
+ #phy-cells = <0>;
+ resets = <&chip 0x104 13>;
+ status = "disabled";
+ };
+
apb@e80000 {
compatible = "simple-bus";
#address-cells = <1>;
@@ -304,6 +337,24 @@
clock-names = "refclk";
};

+ usb0: usb@ed0000 {
+ compatible = "chipidea-usb";
+ reg = <0xed0000 0x10000>;
+ interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&chip CLKID_USB0>;
+ usb-phy = <&usb_phy0>;
+ status = "disabled";
+ };
+
+ usb1: usb@ee0000 {
+ compatible = "chipidea-usb";
+ reg = <0xee0000 0x10000>;
+ interrupts = <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&chip CLKID_USB1>;
+ usb-phy = <&usb_phy1>;
+ status = "disabled";
+ };
+
apb@fc0000 {
compatible = "simple-bus";
#address-cells = <1>;
@@ -360,5 +411,6 @@
interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
};
};
+
};
};
--
1.9.1

2014-06-24 10:35:30

by Antoine Tenart

[permalink] [raw]
Subject: [PATCH v2 02/12] Documentation: bindings: add reset bindings docs for Marvell Berlin SoCs

Add the reset binding documentation to the SoC binding documentation as
the reset driver in Marvell Berlin SoC is part of the chip/system
control registers. This patch adds the required properties to configure
the reset controller.

Signed-off-by: Antoine Ténart <[email protected]>
---
Documentation/devicetree/bindings/arm/marvell,berlin.txt | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/marvell,berlin.txt b/Documentation/devicetree/bindings/arm/marvell,berlin.txt
index 94013a9a8769..32a1a8a652ad 100644
--- a/Documentation/devicetree/bindings/arm/marvell,berlin.txt
+++ b/Documentation/devicetree/bindings/arm/marvell,berlin.txt
@@ -90,11 +90,21 @@ Required subnode-properties:
- groups: a list of strings describing the group names.
- function: a string describing the function used to mux the groups.

+* Reset controller binding
+
+A reset controller is part of the chip control registers set. The chip control
+node also provides the reset. The register set is not at the same offset between
+Berlin SoCs.
+
+Required property:
+- #reset-cells: must be set to 2
+
Example:

chip: chip-control@ea0000 {
compatible = "marvell,berlin2-chip-ctrl";
#clock-cells = <1>;
+ #reset-cells = <2>;
reg = <0xea0000 0x400>;
clocks = <&refclk>, <&externaldev 0>;
clock-names = "refclk", "video_ext0";
--
1.9.1

2014-06-24 10:37:19

by Antoine Tenart

[permalink] [raw]
Subject: [PATCH v2 07/12] usb: chipidea: add a generic driver

Add a generic ChipIdea driver, with optional PHY and clock, to support
ChipIdea controllers that doesn't need specific functions.

Needed for the Marvell Berlin SoCs SUB controllers.

Signed-off-by: Antoine Ténart <[email protected]>
---
drivers/usb/chipidea/Makefile | 1 +
drivers/usb/chipidea/ci_hdrc_generic.c | 108 +++++++++++++++++++++++++++++++++
2 files changed, 109 insertions(+)
create mode 100644 drivers/usb/chipidea/ci_hdrc_generic.c

diff --git a/drivers/usb/chipidea/Makefile b/drivers/usb/chipidea/Makefile
index 2f099c7df7b5..c705f0fe7a00 100644
--- a/drivers/usb/chipidea/Makefile
+++ b/drivers/usb/chipidea/Makefile
@@ -20,4 +20,5 @@ endif

ifneq ($(CONFIG_OF),)
obj-$(CONFIG_USB_CHIPIDEA) += usbmisc_imx.o ci_hdrc_imx.o
+ obj-$(CONFIG_USB_CHIPIDEA) += ci_hdrc_generic.o
endif
diff --git a/drivers/usb/chipidea/ci_hdrc_generic.c b/drivers/usb/chipidea/ci_hdrc_generic.c
new file mode 100644
index 000000000000..27520710a1f7
--- /dev/null
+++ b/drivers/usb/chipidea/ci_hdrc_generic.c
@@ -0,0 +1,108 @@
+/*
+ * Copyright (C) 2014 Marvell Technology Group Ltd.
+ *
+ * Antoine Ténart <[email protected]>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/clk.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/usb/chipidea.h>
+#include <linux/usb/hcd.h>
+#include <linux/usb/ulpi.h>
+
+#include "ci.h"
+
+struct ci_hdrc_generic_priv {
+ struct platform_device *ci_pdev;
+ struct clk *clk;
+};
+
+static int ci_hdrc_generic_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct ci_hdrc_generic_priv *priv;
+ struct ci_hdrc_platform_data ci_pdata = {
+ .name = "ci_hdrc",
+ .capoffset = DEF_CAPOFFSET,
+ .flags = CI_HDRC_REQUIRE_TRANSCEIVER |
+ CI_HDRC_DISABLE_STREAMING,
+ };
+ int ret;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->clk = devm_clk_get(dev, NULL);
+ if (!IS_ERR(priv->clk)) {
+ ret = clk_prepare_enable(priv->clk);
+ if (ret) {
+ dev_err(dev, "failed to enable the clock: %d\n", ret);
+ return ret;
+ }
+ }
+
+ ci_pdata.phy = devm_usb_get_phy_by_phandle(dev, "usb-phy", 0);
+ if (IS_ERR(ci_pdata.phy))
+ /* PHY is optional */
+ ci_pdata.phy = NULL;
+
+ priv->ci_pdev = ci_hdrc_add_device(dev, pdev->resource,
+ pdev->num_resources, &ci_pdata);
+ if (IS_ERR(priv->ci_pdev)) {
+ ret = PTR_ERR(priv->ci_pdev);
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev,
+ "failed to register ci_hdrc platform device: %d\n",
+ ret);
+ goto clk_err;
+ }
+
+ platform_set_drvdata(pdev, priv);
+
+ pm_runtime_no_callbacks(dev);
+ pm_runtime_enable(dev);
+
+ return 0;
+
+clk_err:
+ clk_disable_unprepare(priv->clk);
+ return ret;
+}
+
+static int ci_hdrc_generic_remove(struct platform_device *pdev)
+{
+ struct ci_hdrc_generic_priv *priv = platform_get_drvdata(pdev);
+
+ pm_runtime_disable(&pdev->dev);
+ ci_hdrc_remove_device(priv->ci_pdev);
+ clk_disable_unprepare(priv->clk);
+
+ return 0;
+}
+
+static const struct of_device_id ci_hdrc_generic_of_match[] = {
+ { .compatible = "chipidea-usb" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, ci_hdrc_generic_of_match);
+
+static struct platform_driver ci_hdrc_generic_driver = {
+ .probe = ci_hdrc_generic_probe,
+ .remove = ci_hdrc_generic_remove,
+ .driver = {
+ .name = "chipidea-usb",
+ .owner = THIS_MODULE,
+ .of_match_table = ci_hdrc_generic_of_match,
+ },
+};
+module_platform_driver(ci_hdrc_generic_driver);
+
+MODULE_DESCRIPTION("Generic ChipIdea HDRC USB binding");
+MODULE_AUTHOR("Antoine Ténart <[email protected]>");
+MODULE_LICENSE("GPL");
--
1.9.1

2014-06-24 10:37:50

by Antoine Tenart

[permalink] [raw]
Subject: [PATCH v2 05/12] usb: phy: add the Berlin USB PHY driver

Add the driver driving the Marvell Berlin USB PHY. This allows to
initialize the PHY and to use it from the USB driver later.

Signed-off-by: Antoine Ténart <[email protected]>
---
drivers/usb/phy/Kconfig | 9 ++
drivers/usb/phy/Makefile | 1 +
drivers/usb/phy/phy-berlin-usb.c | 211 +++++++++++++++++++++++++++++++++++++++
3 files changed, 221 insertions(+)
create mode 100644 drivers/usb/phy/phy-berlin-usb.c

diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index e253fa05be68..9a47cc1c73fe 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -18,6 +18,15 @@ config AB8500_USB
This transceiver supports high and full speed devices plus,
in host mode, low speed.

+config BERLIN_USBPHY
+ tristate "Marvell Berlin USB Transceiver Driver"
+ depends on ARCH_BERLIN
+ select USB_PHY
+ help
+ Enable this to support the USB tranceiver on Marvell Berlin
+ SoCs.
+
+
config FSL_USB2_OTG
bool "Freescale USB OTG Transceiver Driver"
depends on USB_EHCI_FSL && USB_FSL_USB2 && USB_OTG_FSM && PM_RUNTIME
diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
index 24a91332d4ad..450ac91c5e20 100644
--- a/drivers/usb/phy/Makefile
+++ b/drivers/usb/phy/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_OF) += of.o
# transceiver drivers, keep the list sorted

obj-$(CONFIG_AB8500_USB) += phy-ab8500-usb.o
+obj-$(CONFIG_BERLIN_USBPHY) += phy-berlin-usb.o
obj-$(CONFIG_FSL_USB2_OTG) += phy-fsl-usb.o
obj-$(CONFIG_ISP1301_OMAP) += phy-isp1301-omap.o
obj-$(CONFIG_NOP_USB_XCEIV) += phy-generic.o
diff --git a/drivers/usb/phy/phy-berlin-usb.c b/drivers/usb/phy/phy-berlin-usb.c
new file mode 100644
index 000000000000..276d28f5cb83
--- /dev/null
+++ b/drivers/usb/phy/phy-berlin-usb.c
@@ -0,0 +1,211 @@
+/*
+ * Copyright (C) 2014 Marvell Technology Group Ltd.
+ *
+ * Antoine Ténart <[email protected]>
+ * Jisheng Zhang <[email protected]>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/gpio.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+#include <linux/usb/phy.h>
+
+#define USB_PHY_PLL 0x04
+#define USB_PHY_PLL_CONTROL 0x08
+#define USB_PHY_TX_CTRL0 0x10
+#define USB_PHY_TX_CTRL1 0x14
+#define USB_PHY_TX_CTRL2 0x18
+#define USB_PHY_RX_CTRL 0x20
+#define USB_PHY_ANALOG 0x34
+
+/* USB_PHY_PLL */
+#define CLK_REF_DIV(x) ((x) << 4)
+#define FEEDBACK_CLK_DIV(x) ((x) << 8)
+
+/* USB_PHY_PLL_CONTROL */
+#define CLK_STABLE BIT(0)
+#define PLL_CTRL_PIN BIT(1)
+#define PLL_CTRL_REG BIT(2)
+#define PLL_ON BIT(3)
+#define PHASE_OFF_TOL_125 (0x0 << 5)
+#define PHASE_OFF_TOL_250 BIT(5)
+#define KVC0_CALIB (0x0 << 9)
+#define KVC0_REG_CTRL BIT(9)
+#define KVC0_HIGH (0x0 << 10)
+#define KVC0_LOW (0x3 << 10)
+#define CLK_BLK_EN BIT(13)
+
+/* USB_PHY_TX_CTRL0 */
+#define EXT_HS_RCAL_EN BIT(3)
+#define EXT_FS_RCAL_EN BIT(4)
+#define IMPCAL_VTH_DIV(x) ((x) << 5)
+#define EXT_RS_RCAL_DIV(x) ((x) << 8)
+#define EXT_FS_RCAL_DIV(x) ((x) << 12)
+
+/* USB_PHY_TX_CTRL1 */
+#define TX_VDD15_14 (0x0 << 4)
+#define TX_VDD15_15 BIT(4)
+#define TX_VDD15_16 (0x2 << 4)
+#define TX_VDD15_17 (0x3 << 4)
+#define TX_VDD12_VDD (0x0 << 6)
+#define TX_VDD12_11 BIT(6)
+#define TX_VDD12_12 (0x2 << 6)
+#define TX_VDD12_13 (0x3 << 6)
+#define LOW_VDD_EN BIT(8)
+#define TX_OUT_AMP(x) ((x) << 9)
+
+/* USB_PHY_TX_CTRL2 */
+#define TX_CHAN_CTRL_REG(x) ((x) << 0)
+#define DRV_SLEWRATE(x) ((x) << 4)
+#define IMP_CAL_FS_HS_DLY_0 (0x0 << 6)
+#define IMP_CAL_FS_HS_DLY_1 BIT(6)
+#define IMP_CAL_FS_HS_DLY_2 (0x2 << 6)
+#define IMP_CAL_FS_HS_DLY_3 (0x3 << 6)
+#define FS_DRV_EN_MASK(x) ((x) << 8)
+#define HS_DRV_EN_MASK(x) ((x) << 12)
+
+/* USB_PHY_RX_CTRL */
+#define PHASE_FREEZE_DLY_2_CL (0x0 << 0)
+#define PHASE_FREEZE_DLY_4_CL BIT(0)
+#define ACK_LENGTH_8_CL (0x0 << 2)
+#define ACK_LENGTH_12_CL BIT(2)
+#define ACK_LENGTH_16_CL (0x2 << 2)
+#define ACK_LENGTH_20_CL (0x3 << 2)
+#define SQ_LENGTH_3 (0x0 << 4)
+#define SQ_LENGTH_6 BIT(4)
+#define SQ_LENGTH_9 (0x2 << 4)
+#define SQ_LENGTH_12 (0x3 << 4)
+#define DISCON_THRESHOLD_260 (0x0 << 6)
+#define DISCON_THRESHOLD_270 BIT(6)
+#define DISCON_THRESHOLD_280 (0x2 << 6)
+#define DISCON_THRESHOLD_290 (0x3 << 6)
+#define SQ_THRESHOLD(x) ((x) << 8)
+#define LPF_COEF(x) ((x) << 12)
+#define INTPL_CUR_10 (0x0 << 14)
+#define INTPL_CUR_20 BIT(14)
+#define INTPL_CUR_30 (0x2 << 14)
+#define INTPL_CUR_40 (0x3 << 14)
+
+/* USB_PHY_ANALOG */
+#define ANA_PWR_UP BIT(1)
+#define ANA_PWR_DOWN BIT(2)
+#define V2I_VCO_RATIO(x) ((x) << 7)
+#define R_ROTATE_90 (0x0 << 10)
+#define R_ROTATE_0 BIT(10)
+#define MODE_TEST_EN BIT(11)
+#define ANA_TEST_DC_CTRL(x) ((x) << 12)
+
+#define to_berlin_phy_priv(p) container_of((p), struct berlin_phy_priv, phy)
+
+static const u32 phy_berlin_pll_dividers[] = {
+ /* Berlin 2 */
+ CLK_REF_DIV(0xc) | FEEDBACK_CLK_DIV(0x54),
+ /* Berlin 2CD */
+ CLK_REF_DIV(0x6) | FEEDBACK_CLK_DIV(0x55),
+};
+
+struct berlin_phy_priv {
+ void __iomem *base;
+ struct usb_phy phy;
+ struct reset_control *rst_ctrl;
+ u32 pll_divider;
+};
+
+static int berlin_phy_init(struct usb_phy *phy)
+{
+ struct berlin_phy_priv *priv = to_berlin_phy_priv(phy);
+
+ reset_control_reset(priv->rst_ctrl);
+
+ writel(priv->pll_divider,
+ priv->base + USB_PHY_PLL);
+ writel(CLK_STABLE | PLL_CTRL_REG | PHASE_OFF_TOL_250 | KVC0_REG_CTRL |
+ CLK_BLK_EN, priv->base + USB_PHY_PLL_CONTROL);
+ writel(V2I_VCO_RATIO(0x5) | R_ROTATE_0 | ANA_TEST_DC_CTRL(0x5),
+ priv->base + USB_PHY_ANALOG);
+ writel(PHASE_FREEZE_DLY_4_CL | ACK_LENGTH_16_CL | SQ_LENGTH_12 |
+ DISCON_THRESHOLD_260 | SQ_THRESHOLD(0xa) | LPF_COEF(0x2) |
+ INTPL_CUR_30, priv->base + USB_PHY_RX_CTRL);
+
+ writel(TX_VDD12_13 | TX_OUT_AMP(0x3), priv->base + USB_PHY_TX_CTRL1);
+ writel(EXT_HS_RCAL_EN | IMPCAL_VTH_DIV(0x3) | EXT_RS_RCAL_DIV(0x4),
+ priv->base + USB_PHY_TX_CTRL0);
+
+ writel(EXT_HS_RCAL_EN | IMPCAL_VTH_DIV(0x3) | EXT_RS_RCAL_DIV(0x4) |
+ EXT_FS_RCAL_DIV(0x2), priv->base + USB_PHY_TX_CTRL0);
+
+ writel(EXT_HS_RCAL_EN | IMPCAL_VTH_DIV(0x3) | EXT_RS_RCAL_DIV(0x4),
+ priv->base + USB_PHY_TX_CTRL0);
+ writel(TX_CHAN_CTRL_REG(0xf) | DRV_SLEWRATE(0x3) | IMP_CAL_FS_HS_DLY_3 |
+ FS_DRV_EN_MASK(0xd), priv->base + USB_PHY_TX_CTRL2);
+
+ return 0;
+}
+
+static const struct of_device_id phy_berlin_sata_of_match[] = {
+ {
+ .compatible = "marvell,berlin2-usb-phy",
+ .data = &phy_berlin_pll_dividers[0],
+ },
+ {
+ .compatible = "marvell,berlin2cd-usb-phy",
+ .data = &phy_berlin_pll_dividers[1],
+ },
+ { },
+};
+MODULE_DEVICE_TABLE(of, phy_berlin_sata_of_match);
+
+static int berlin_phy_probe(struct platform_device *pdev)
+{
+ const struct of_device_id *match =
+ of_match_device(phy_berlin_sata_of_match, &pdev->dev);
+ struct berlin_phy_priv *priv;
+ struct resource *res;
+
+ priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ priv->base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(priv->base))
+ return PTR_ERR(priv->base);
+
+ priv->rst_ctrl = devm_reset_control_get(&pdev->dev, NULL);
+ if (IS_ERR(priv->rst_ctrl))
+ return PTR_ERR(priv->rst_ctrl);
+
+ priv->pll_divider = *((u32 *)match->data);
+
+ priv->phy.io_priv = priv->base;
+ priv->phy.dev = &pdev->dev;
+ priv->phy.label = "phy-berlin-usb";
+ priv->phy.init = berlin_phy_init;
+ priv->phy.type = USB_PHY_TYPE_USB2;
+
+ platform_set_drvdata(pdev, priv);
+
+ return usb_add_phy_dev(&priv->phy);
+}
+
+static struct platform_driver phy_berlin_usb_driver = {
+ .probe = berlin_phy_probe,
+ .driver = {
+ .name = "phy-berlin-usb",
+ .owner = THIS_MODULE,
+ .of_match_table = phy_berlin_sata_of_match,
+ },
+};
+module_platform_driver(phy_berlin_usb_driver);
+
+MODULE_AUTHOR("Antoine Ténart <[email protected]>");
+MODULE_DESCRIPTION("Marvell Berlin USB PHY driver");
+MODULE_LICENSE("GPL");
--
1.9.1

2014-06-24 10:37:48

by Antoine Tenart

[permalink] [raw]
Subject: [PATCH v2 06/12] Documentation: bindings: add doc for the Berlin USB PHY

Document the bindings of the Marvell Berlin USB PHY driver.

Signed-off-by: Antoine Ténart <[email protected]>
---
Documentation/devicetree/bindings/usb/berlin-usb-phy.txt | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/berlin-usb-phy.txt

diff --git a/Documentation/devicetree/bindings/usb/berlin-usb-phy.txt b/Documentation/devicetree/bindings/usb/berlin-usb-phy.txt
new file mode 100644
index 000000000000..3a2a5cc42d45
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/berlin-usb-phy.txt
@@ -0,0 +1,16 @@
+* Marvell Berlin USB PHY
+
+Required properties:
+- compatible: "marvell,berlin2-usb-phy" or "marvell,berlin2cd-usb-phy"
+- reg: base address and length of the registers
+- #phys-cells: should be 0
+- resets: reference to the reset controller
+
+Example:
+
+ usb-phy@f774000 {
+ compatible = "marvell,berlin2-usb-phy";
+ reg = <0xf774000 0x128>;
+ #phy-cells = <0>;
+ resets = <&chip 0x104 14>;
+ };
--
1.9.1

2014-06-24 10:38:58

by Antoine Tenart

[permalink] [raw]
Subject: [PATCH v2 04/12] ARM: dts: berlin: add a required reset property in the chip controller node

The chip controller node now also describes the Marvell Berlin reset
controller. Add the required 'reset-cells' property.

Signed-off-by: Antoine Ténart <[email protected]>
---
arch/arm/boot/dts/berlin2.dtsi | 1 +
arch/arm/boot/dts/berlin2cd.dtsi | 1 +
arch/arm/boot/dts/berlin2q.dtsi | 1 +
3 files changed, 3 insertions(+)

diff --git a/arch/arm/boot/dts/berlin2.dtsi b/arch/arm/boot/dts/berlin2.dtsi
index 2477dac4d643..97b12492675e 100644
--- a/arch/arm/boot/dts/berlin2.dtsi
+++ b/arch/arm/boot/dts/berlin2.dtsi
@@ -243,6 +243,7 @@
chip: chip-control@ea0000 {
compatible = "marvell,berlin2-chip-ctrl";
#clock-cells = <1>;
+ #reset-cells = <2>;
reg = <0xea0000 0x400>;
clocks = <&refclk>;
clock-names = "refclk";
diff --git a/arch/arm/boot/dts/berlin2cd.dtsi b/arch/arm/boot/dts/berlin2cd.dtsi
index cc1df65da504..68f7032b4686 100644
--- a/arch/arm/boot/dts/berlin2cd.dtsi
+++ b/arch/arm/boot/dts/berlin2cd.dtsi
@@ -231,6 +231,7 @@
chip: chip-control@ea0000 {
compatible = "marvell,berlin2cd-chip-ctrl";
#clock-cells = <1>;
+ #reset-cells = <2>;
reg = <0xea0000 0x400>;
clocks = <&refclk>;
clock-names = "refclk";
diff --git a/arch/arm/boot/dts/berlin2q.dtsi b/arch/arm/boot/dts/berlin2q.dtsi
index 635a16a64cb4..a6f7ec325200 100644
--- a/arch/arm/boot/dts/berlin2q.dtsi
+++ b/arch/arm/boot/dts/berlin2q.dtsi
@@ -298,6 +298,7 @@
chip: chip-control@ea0000 {
compatible = "marvell,berlin2q-chip-ctrl";
#clock-cells = <1>;
+ #reset-cells = <2>;
reg = <0xea0000 0x400>, <0xdd0170 0x10>;
clocks = <&refclk>;
clock-names = "refclk";
--
1.9.1

2014-06-24 10:35:27

by Antoine Tenart

[permalink] [raw]
Subject: [PATCH v2 01/12] reset: add the Berlin reset controller driver

Add a reset controller for Marvell Berlin SoCs which is used by the
USB PHYs drivers (for now).

Signed-off-by: Antoine Ténart <[email protected]>
Signed-off-by: Sebastian Hesselbarth <[email protected]>
---
drivers/reset/Makefile | 1 +
drivers/reset/reset-berlin.c | 131 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 132 insertions(+)
create mode 100644 drivers/reset/reset-berlin.c

diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 60fed3d7820b..157d421f755b 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -1,4 +1,5 @@
obj-$(CONFIG_RESET_CONTROLLER) += core.o
obj-$(CONFIG_ARCH_SOCFPGA) += reset-socfpga.o
+obj-$(CONFIG_ARCH_BERLIN) += reset-berlin.o
obj-$(CONFIG_ARCH_SUNXI) += reset-sunxi.o
obj-$(CONFIG_ARCH_STI) += sti/
diff --git a/drivers/reset/reset-berlin.c b/drivers/reset/reset-berlin.c
new file mode 100644
index 000000000000..c988c08825b4
--- /dev/null
+++ b/drivers/reset/reset-berlin.c
@@ -0,0 +1,131 @@
+/*
+ * Copyright (C) 2014 Marvell Technology Group Ltd.
+ *
+ * Antoine Ténart <[email protected]>
+ * Sebastian Hesselbarth <[email protected]>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/reset-controller.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+#define to_berlin_reset_priv(p) \
+ container_of((p), struct berlin_reset_priv, rcdev)
+
+struct berlin_reset_priv {
+ spinlock_t lock;
+ void __iomem *base;
+ unsigned int size;
+ struct reset_controller_dev rcdev;
+};
+
+static int berlin_reset_reset(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ struct berlin_reset_priv *priv = to_berlin_reset_priv(rcdev);
+ int offset = id >> 8;
+ int mask = BIT(id & 0xff);
+
+ writel(mask, priv->base + offset);
+
+ /* let the reset be effective */
+ udelay(10);
+
+ return 0;
+}
+
+static struct reset_control_ops berlin_reset_ops = {
+ .reset = berlin_reset_reset,
+};
+
+static int berlin_reset_xlate(struct reset_controller_dev *rcdev,
+ const struct of_phandle_args *reset_spec)
+{
+ struct berlin_reset_priv *priv = to_berlin_reset_priv(rcdev);
+ unsigned offset, bit;
+
+ if (WARN_ON(reset_spec->args_count != rcdev->of_reset_n_cells))
+ return -EINVAL;
+
+ offset = reset_spec->args[0];
+ bit = reset_spec->args[1];
+
+ if (offset >= priv->size)
+ return -EINVAL;
+
+ if (bit >= rcdev->nr_resets)
+ return -EINVAL;
+
+ return (offset << 8) | bit;
+}
+
+static int __berlin_reset_init(struct device_node *np)
+{
+ struct berlin_reset_priv *priv;
+ struct resource res;
+ resource_size_t size;
+ int ret;
+
+ priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ ret = of_address_to_resource(np, 0, &res);
+ if (ret)
+ goto err;
+
+ size = resource_size(&res);
+ priv->base = ioremap(res.start, size);
+ if (!priv->base) {
+ ret = -ENOMEM;
+ goto err;
+ }
+ priv->size = size;
+
+ priv->rcdev.owner = THIS_MODULE;
+ priv->rcdev.nr_resets = 32;
+ priv->rcdev.ops = &berlin_reset_ops;
+ priv->rcdev.of_node = np;
+ priv->rcdev.of_reset_n_cells = 2;
+ priv->rcdev.of_xlate = berlin_reset_xlate;
+
+ reset_controller_register(&priv->rcdev);
+
+ return 0;
+
+err:
+ kfree(priv);
+ return ret;
+}
+
+static const struct of_device_id berlin_reset_of_match[] __initconst = {
+ { .compatible = "marvell,berlin2-chip-ctrl" },
+ { .compatible = "marvell,berlin2cd-chip-ctrl" },
+ { .compatible = "marvell,berlin2q-chip-ctrl" },
+ { },
+};
+
+static int __init berlin_reset_init(void)
+{
+ struct device_node *np;
+ int ret;
+
+ for_each_matching_node(np, berlin_reset_of_match) {
+ ret = __berlin_reset_init(np);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+arch_initcall(berlin_reset_init);
--
1.9.1

2014-06-24 10:51:06

by Jingoo Han

[permalink] [raw]
Subject: Re: [PATCH v2 07/12] usb: chipidea: add a generic driver

On Tuesday, June 24, 2014 7:35 PM, Antoine Tenart wrote:
>
> Add a generic ChipIdea driver, with optional PHY and clock, to support
> ChipIdea controllers that doesn't need specific functions.

s/doesn't/don't

>
> Needed for the Marvell Berlin SoCs SUB controllers.

s/SUB/USB

> Signed-off-by: Antoine Ténart <[email protected]>
> ---
> drivers/usb/chipidea/Makefile | 1 +
> drivers/usb/chipidea/ci_hdrc_generic.c | 108 +++++++++++++++++++++++++++++++++
> 2 files changed, 109 insertions(+)
> create mode 100644 drivers/usb/chipidea/ci_hdrc_generic.c
>
> diff --git a/drivers/usb/chipidea/Makefile b/drivers/usb/chipidea/Makefile
> index 2f099c7df7b5..c705f0fe7a00 100644
> --- a/drivers/usb/chipidea/Makefile
> +++ b/drivers/usb/chipidea/Makefile
> @@ -20,4 +20,5 @@ endif
>
> ifneq ($(CONFIG_OF),)
> obj-$(CONFIG_USB_CHIPIDEA) += usbmisc_imx.o ci_hdrc_imx.o
> + obj-$(CONFIG_USB_CHIPIDEA) += ci_hdrc_generic.o
> endif
> diff --git a/drivers/usb/chipidea/ci_hdrc_generic.c b/drivers/usb/chipidea/ci_hdrc_generic.c
> new file mode 100644
> index 000000000000..27520710a1f7
> --- /dev/null
> +++ b/drivers/usb/chipidea/ci_hdrc_generic.c

[...]

> +
> +MODULE_DESCRIPTION("Generic ChipIdea HDRC USB binding");
> +MODULE_AUTHOR("Antoine Ténart <[email protected]>");
> +MODULE_LICENSE("GPL");

How about "GPL v2"?

Best regards,
Jingoo Han

> --
> 1.9.1

2014-06-24 11:13:46

by Philipp Zabel

[permalink] [raw]
Subject: Re: [PATCH v2 01/12] reset: add the Berlin reset controller driver

Hi Antoine,

Am Dienstag, den 24.06.2014, 12:35 +0200 schrieb Antoine Ténart:
[...]
> +++ b/drivers/reset/reset-berlin.c
> @@ -0,0 +1,131 @@
> +/*
> + * Copyright (C) 2014 Marvell Technology Group Ltd.
> + *
> + * Antoine Ténart <[email protected]>
> + * Sebastian Hesselbarth <[email protected]>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset-controller.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +
> +#define to_berlin_reset_priv(p) \
> + container_of((p), struct berlin_reset_priv, rcdev)
> +
> +struct berlin_reset_priv {
> + spinlock_t lock;

lock is not used anymore.

[...]
> +static int berlin_reset_xlate(struct reset_controller_dev *rcdev,
> + const struct of_phandle_args *reset_spec)
> +{
> + struct berlin_reset_priv *priv = to_berlin_reset_priv(rcdev);
> + unsigned offset, bit;
> +
> + if (WARN_ON(reset_spec->args_count != rcdev->of_reset_n_cells))
> + return -EINVAL;
> +
> + offset = reset_spec->args[0];
> + bit = reset_spec->args[1];
> +
> + if (offset >= priv->size)
> + return -EINVAL;
> +
> + if (bit >= rcdev->nr_resets)
> + return -EINVAL;

This could be considered a misuse of nr_resets, even if the core only
ever uses nr_resets in the _xlate function. Maybe it would be more clear
to just leave nr_resets empty if you don't know the actual number and
hardcode 32 here.

[...]
> +static int __berlin_reset_init(struct device_node *np)
> +{
> + struct berlin_reset_priv *priv;
> + struct resource res;
> + resource_size_t size;
> + int ret;
> +
> + priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + ret = of_address_to_resource(np, 0, &res);
> + if (ret)
> + goto err;
> +
> + size = resource_size(&res);
> + priv->base = ioremap(res.start, size);
> + if (!priv->base) {
> + ret = -ENOMEM;
> + goto err;
> + }
> + priv->size = size;
> +
> + priv->rcdev.owner = THIS_MODULE;
> + priv->rcdev.nr_resets = 32;

Leave nr_resets empty, use the correct value, or at least add a comment
that this is not the number of resets in the rcdev as documented in the
structure documentation.

> + priv->rcdev.ops = &berlin_reset_ops;
> + priv->rcdev.of_node = np;
> + priv->rcdev.of_reset_n_cells = 2;
> + priv->rcdev.of_xlate = berlin_reset_xlate;
> +
> + reset_controller_register(&priv->rcdev);
> +
> + return 0;
> +
> +err:
> + kfree(priv);
> + return ret;
> +}
> +
> +static const struct of_device_id berlin_reset_of_match[] __initconst = {
> + { .compatible = "marvell,berlin2-chip-ctrl" },
> + { .compatible = "marvell,berlin2cd-chip-ctrl" },
> + { .compatible = "marvell,berlin2q-chip-ctrl" },
> + { },
> +};
> +
> +static int __init berlin_reset_init(void)
> +{
> + struct device_node *np;
> + int ret;
> +
> + for_each_matching_node(np, berlin_reset_of_match) {
> + ret = __berlin_reset_init(np);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +arch_initcall(berlin_reset_init);

Other than the above, and with the understanding that this is going to
be turned into a platform driver at some point in the future,

Acked-by: Philipp Zabel <[email protected]>

regards
Philipp

2014-06-24 11:14:17

by Philipp Zabel

[permalink] [raw]
Subject: Re: [PATCH v2 02/12] Documentation: bindings: add reset bindings docs for Marvell Berlin SoCs

Am Dienstag, den 24.06.2014, 12:35 +0200 schrieb Antoine Ténart:
> Add the reset binding documentation to the SoC binding documentation as
> the reset driver in Marvell Berlin SoC is part of the chip/system
> control registers. This patch adds the required properties to configure
> the reset controller.
>
> Signed-off-by: Antoine Ténart <[email protected]>

Acked-by: Philipp Zabel <[email protected]>

2014-06-24 11:14:51

by Philipp Zabel

[permalink] [raw]
Subject: Re: [PATCH v2 04/12] ARM: dts: berlin: add a required reset property in the chip controller node

Am Dienstag, den 24.06.2014, 12:35 +0200 schrieb Antoine Ténart:
> The chip controller node now also describes the Marvell Berlin reset
> controller. Add the required 'reset-cells' property.
>
> Signed-off-by: Antoine Ténart <[email protected]>

Acked-by: Philipp Zabel <[email protected]>

2014-06-24 12:05:27

by Antoine Tenart

[permalink] [raw]
Subject: Re: [PATCH v2 01/12] reset: add the Berlin reset controller driver

Hi Philipp,

On Tue, Jun 24, 2014 at 01:13:36PM +0200, Philipp Zabel wrote:
> Am Dienstag, den 24.06.2014, 12:35 +0200 schrieb Antoine T?nart:
> > +
> > +struct berlin_reset_priv {
> > + spinlock_t lock;
>
> lock is not used anymore.

Oops. I'll remove it.

>
> [...]
> > +static int berlin_reset_xlate(struct reset_controller_dev *rcdev,
> > + const struct of_phandle_args *reset_spec)
> > +{
> > + struct berlin_reset_priv *priv = to_berlin_reset_priv(rcdev);
> > + unsigned offset, bit;
> > +
> > + if (WARN_ON(reset_spec->args_count != rcdev->of_reset_n_cells))
> > + return -EINVAL;
> > +
> > + offset = reset_spec->args[0];
> > + bit = reset_spec->args[1];
> > +
> > + if (offset >= priv->size)
> > + return -EINVAL;
> > +
> > + if (bit >= rcdev->nr_resets)
> > + return -EINVAL;
>
> This could be considered a misuse of nr_resets, even if the core only
> ever uses nr_resets in the _xlate function. Maybe it would be more clear
> to just leave nr_resets empty if you don't know the actual number and
> hardcode 32 here.
>
> [...]
> > +static int __berlin_reset_init(struct device_node *np)
> > +{
> > + struct berlin_reset_priv *priv;
> > + struct resource res;
> > + resource_size_t size;
> > + int ret;
> > +
> > + priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> > + if (!priv)
> > + return -ENOMEM;
> > +
> > + ret = of_address_to_resource(np, 0, &res);
> > + if (ret)
> > + goto err;
> > +
> > + size = resource_size(&res);
> > + priv->base = ioremap(res.start, size);
> > + if (!priv->base) {
> > + ret = -ENOMEM;
> > + goto err;
> > + }
> > + priv->size = size;
> > +
> > + priv->rcdev.owner = THIS_MODULE;
> > + priv->rcdev.nr_resets = 32;
>
> Leave nr_resets empty, use the correct value, or at least add a comment
> that this is not the number of resets in the rcdev as documented in the
> structure documentation.

Ok. I'll hardcode the value in the driver then.

>
> Other than the above, and with the understanding that this is going to
> be turned into a platform driver at some point in the future,
>
> Acked-by: Philipp Zabel <[email protected]>

Sure, that's the plan.

Antoine

--
Antoine T?nart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

2014-06-27 04:56:00

by Peter Chen

[permalink] [raw]
Subject: Re: [PATCH v2 07/12] usb: chipidea: add a generic driver

On Tue, Jun 24, 2014 at 12:35:16PM +0200, Antoine T?nart wrote:
> Add a generic ChipIdea driver, with optional PHY and clock, to support
> ChipIdea controllers that doesn't need specific functions.
>
> Needed for the Marvell Berlin SoCs SUB controllers.
>
> Signed-off-by: Antoine T?nart <[email protected]>
> ---
> drivers/usb/chipidea/Makefile | 1 +
> drivers/usb/chipidea/ci_hdrc_generic.c | 108 +++++++++++++++++++++++++++++++++
> 2 files changed, 109 insertions(+)
> create mode 100644 drivers/usb/chipidea/ci_hdrc_generic.c
>
> diff --git a/drivers/usb/chipidea/Makefile b/drivers/usb/chipidea/Makefile
> index 2f099c7df7b5..c705f0fe7a00 100644
> --- a/drivers/usb/chipidea/Makefile
> +++ b/drivers/usb/chipidea/Makefile
> @@ -20,4 +20,5 @@ endif
>
> ifneq ($(CONFIG_OF),)
> obj-$(CONFIG_USB_CHIPIDEA) += usbmisc_imx.o ci_hdrc_imx.o
> + obj-$(CONFIG_USB_CHIPIDEA) += ci_hdrc_generic.o
> endif

As a generic driver, you may need to support both dt and non-dt
solution.

> diff --git a/drivers/usb/chipidea/ci_hdrc_generic.c b/drivers/usb/chipidea/ci_hdrc_generic.c
> new file mode 100644
> index 000000000000..27520710a1f7
> --- /dev/null
> +++ b/drivers/usb/chipidea/ci_hdrc_generic.c
> @@ -0,0 +1,108 @@
> +/*
> + * Copyright (C) 2014 Marvell Technology Group Ltd.
> + *
> + * Antoine T?nart <[email protected]>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/usb/chipidea.h>
> +#include <linux/usb/hcd.h>
> +#include <linux/usb/ulpi.h>
> +
> +#include "ci.h"
> +
> +struct ci_hdrc_generic_priv {
> + struct platform_device *ci_pdev;
> + struct clk *clk;
> +};
> +
> +static int ci_hdrc_generic_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct ci_hdrc_generic_priv *priv;
> + struct ci_hdrc_platform_data ci_pdata = {
> + .name = "ci_hdrc",

How about this using dev_name(&pdev->dev) for name?

> + .capoffset = DEF_CAPOFFSET,
> + .flags = CI_HDRC_REQUIRE_TRANSCEIVER |
> + CI_HDRC_DISABLE_STREAMING,
> + };
> + int ret;
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + priv->clk = devm_clk_get(dev, NULL);
> + if (!IS_ERR(priv->clk)) {
> + ret = clk_prepare_enable(priv->clk);
> + if (ret) {
> + dev_err(dev, "failed to enable the clock: %d\n", ret);
> + return ret;
> + }
> + }
> +
> + ci_pdata.phy = devm_usb_get_phy_by_phandle(dev, "usb-phy", 0);
> + if (IS_ERR(ci_pdata.phy))
> + /* PHY is optional */
> + ci_pdata.phy = NULL;
> +
> + priv->ci_pdev = ci_hdrc_add_device(dev, pdev->resource,
> + pdev->num_resources, &ci_pdata);
> + if (IS_ERR(priv->ci_pdev)) {
> + ret = PTR_ERR(priv->ci_pdev);
> + if (ret != -EPROBE_DEFER)
> + dev_err(dev,
> + "failed to register ci_hdrc platform device: %d\n",
> + ret);
> + goto clk_err;
> + }
> +
> + platform_set_drvdata(pdev, priv);
> +
> + pm_runtime_no_callbacks(dev);
> + pm_runtime_enable(dev);
> +
> + return 0;
> +
> +clk_err:
> + clk_disable_unprepare(priv->clk);

You may need to add "if (!IS_ERR(priv->clk))"

> + return ret;
> +}
> +
> +static int ci_hdrc_generic_remove(struct platform_device *pdev)
> +{
> + struct ci_hdrc_generic_priv *priv = platform_get_drvdata(pdev);
> +
> + pm_runtime_disable(&pdev->dev);
> + ci_hdrc_remove_device(priv->ci_pdev);
> + clk_disable_unprepare(priv->clk);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id ci_hdrc_generic_of_match[] = {
> + { .compatible = "chipidea-usb" },
> + { }
> +};

Even as a generic driver, you can also use your own compatible string.

> +MODULE_DEVICE_TABLE(of, ci_hdrc_generic_of_match);
> +
> +static struct platform_driver ci_hdrc_generic_driver = {
> + .probe = ci_hdrc_generic_probe,
> + .remove = ci_hdrc_generic_remove,
> + .driver = {
> + .name = "chipidea-usb",
> + .owner = THIS_MODULE,
> + .of_match_table = ci_hdrc_generic_of_match,
> + },
> +};
> +module_platform_driver(ci_hdrc_generic_driver);
> +
> +MODULE_DESCRIPTION("Generic ChipIdea HDRC USB binding");
> +MODULE_AUTHOR("Antoine T?nart <[email protected]>");
> +MODULE_LICENSE("GPL");
> --
> 1.9.1
>

--

Best Regards,
Peter Chen

2014-06-27 05:12:40

by Peter Chen

[permalink] [raw]
Subject: Re: [PATCH v2 07/12] usb: chipidea: add a generic driver

On Fri, Jun 27, 2014 at 11:25:07AM +0800, Peter Chen wrote:
> On Tue, Jun 24, 2014 at 12:35:16PM +0200, Antoine T?nart wrote:
> > Add a generic ChipIdea driver, with optional PHY and clock, to support
> > ChipIdea controllers that doesn't need specific functions.
> >
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id ci_hdrc_generic_of_match[] = {
> > + { .compatible = "chipidea-usb" },
> > + { }
> > +};
>
> Even as a generic driver, you can also use your own compatible string.
>
> > +MODULE_DEVICE_TABLE(of, ci_hdrc_generic_of_match);
> > +
> > +static struct platform_driver ci_hdrc_generic_driver = {
> > + .probe = ci_hdrc_generic_probe,
> > + .remove = ci_hdrc_generic_remove,
> > + .driver = {
> > + .name = "chipidea-usb",
> > + .owner = THIS_MODULE,
> > + .of_match_table = ci_hdrc_generic_of_match,
> > + },
> > +};
> > +module_platform_driver(ci_hdrc_generic_driver);
> > +
> > +MODULE_DESCRIPTION("Generic ChipIdea HDRC USB binding");
> > +MODULE_AUTHOR("Antoine T?nart <[email protected]>");
> > +MODULE_LICENSE("GPL");
> > --
> > 1.9.1
> >
>
> --
>

Besides, I haven't seen dma_coerce_mask_and_coherent API calling,
where you set your dma mask?

--

Best Regards,
Peter Chen

2014-06-27 15:56:56

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH v2 05/12] usb: phy: add the Berlin USB PHY driver

On Tue, Jun 24, 2014 at 12:35:14PM +0200, Antoine T?nart wrote:
> Add the driver driving the Marvell Berlin USB PHY. This allows to
> initialize the PHY and to use it from the USB driver later.
>
> Signed-off-by: Antoine T?nart <[email protected]>

since this is a brand new driver, it should go to drivers/phy instead.

> ---
> drivers/usb/phy/Kconfig | 9 ++
> drivers/usb/phy/Makefile | 1 +
> drivers/usb/phy/phy-berlin-usb.c | 211 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 221 insertions(+)
> create mode 100644 drivers/usb/phy/phy-berlin-usb.c
>
> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
> index e253fa05be68..9a47cc1c73fe 100644
> --- a/drivers/usb/phy/Kconfig
> +++ b/drivers/usb/phy/Kconfig
> @@ -18,6 +18,15 @@ config AB8500_USB
> This transceiver supports high and full speed devices plus,
> in host mode, low speed.
>
> +config BERLIN_USBPHY
> + tristate "Marvell Berlin USB Transceiver Driver"
> + depends on ARCH_BERLIN
> + select USB_PHY
> + help
> + Enable this to support the USB tranceiver on Marvell Berlin
> + SoCs.
> +
> +
> config FSL_USB2_OTG
> bool "Freescale USB OTG Transceiver Driver"
> depends on USB_EHCI_FSL && USB_FSL_USB2 && USB_OTG_FSM && PM_RUNTIME
> diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
> index 24a91332d4ad..450ac91c5e20 100644
> --- a/drivers/usb/phy/Makefile
> +++ b/drivers/usb/phy/Makefile
> @@ -7,6 +7,7 @@ obj-$(CONFIG_OF) += of.o
> # transceiver drivers, keep the list sorted
>
> obj-$(CONFIG_AB8500_USB) += phy-ab8500-usb.o
> +obj-$(CONFIG_BERLIN_USBPHY) += phy-berlin-usb.o
> obj-$(CONFIG_FSL_USB2_OTG) += phy-fsl-usb.o
> obj-$(CONFIG_ISP1301_OMAP) += phy-isp1301-omap.o
> obj-$(CONFIG_NOP_USB_XCEIV) += phy-generic.o
> diff --git a/drivers/usb/phy/phy-berlin-usb.c b/drivers/usb/phy/phy-berlin-usb.c
> new file mode 100644
> index 000000000000..276d28f5cb83
> --- /dev/null
> +++ b/drivers/usb/phy/phy-berlin-usb.c
> @@ -0,0 +1,211 @@
> +/*
> + * Copyright (C) 2014 Marvell Technology Group Ltd.
> + *
> + * Antoine T?nart <[email protected]>
> + * Jisheng Zhang <[email protected]>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include <linux/gpio.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/of_gpio.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset.h>
> +#include <linux/usb/phy.h>
> +
> +#define USB_PHY_PLL 0x04
> +#define USB_PHY_PLL_CONTROL 0x08
> +#define USB_PHY_TX_CTRL0 0x10
> +#define USB_PHY_TX_CTRL1 0x14
> +#define USB_PHY_TX_CTRL2 0x18
> +#define USB_PHY_RX_CTRL 0x20
> +#define USB_PHY_ANALOG 0x34
> +
> +/* USB_PHY_PLL */
> +#define CLK_REF_DIV(x) ((x) << 4)
> +#define FEEDBACK_CLK_DIV(x) ((x) << 8)
> +
> +/* USB_PHY_PLL_CONTROL */
> +#define CLK_STABLE BIT(0)
> +#define PLL_CTRL_PIN BIT(1)
> +#define PLL_CTRL_REG BIT(2)
> +#define PLL_ON BIT(3)
> +#define PHASE_OFF_TOL_125 (0x0 << 5)
> +#define PHASE_OFF_TOL_250 BIT(5)
> +#define KVC0_CALIB (0x0 << 9)
> +#define KVC0_REG_CTRL BIT(9)
> +#define KVC0_HIGH (0x0 << 10)
> +#define KVC0_LOW (0x3 << 10)
> +#define CLK_BLK_EN BIT(13)
> +
> +/* USB_PHY_TX_CTRL0 */
> +#define EXT_HS_RCAL_EN BIT(3)
> +#define EXT_FS_RCAL_EN BIT(4)
> +#define IMPCAL_VTH_DIV(x) ((x) << 5)
> +#define EXT_RS_RCAL_DIV(x) ((x) << 8)
> +#define EXT_FS_RCAL_DIV(x) ((x) << 12)
> +
> +/* USB_PHY_TX_CTRL1 */
> +#define TX_VDD15_14 (0x0 << 4)
> +#define TX_VDD15_15 BIT(4)
> +#define TX_VDD15_16 (0x2 << 4)
> +#define TX_VDD15_17 (0x3 << 4)
> +#define TX_VDD12_VDD (0x0 << 6)
> +#define TX_VDD12_11 BIT(6)
> +#define TX_VDD12_12 (0x2 << 6)
> +#define TX_VDD12_13 (0x3 << 6)
> +#define LOW_VDD_EN BIT(8)
> +#define TX_OUT_AMP(x) ((x) << 9)
> +
> +/* USB_PHY_TX_CTRL2 */
> +#define TX_CHAN_CTRL_REG(x) ((x) << 0)
> +#define DRV_SLEWRATE(x) ((x) << 4)
> +#define IMP_CAL_FS_HS_DLY_0 (0x0 << 6)
> +#define IMP_CAL_FS_HS_DLY_1 BIT(6)
> +#define IMP_CAL_FS_HS_DLY_2 (0x2 << 6)
> +#define IMP_CAL_FS_HS_DLY_3 (0x3 << 6)
> +#define FS_DRV_EN_MASK(x) ((x) << 8)
> +#define HS_DRV_EN_MASK(x) ((x) << 12)
> +
> +/* USB_PHY_RX_CTRL */
> +#define PHASE_FREEZE_DLY_2_CL (0x0 << 0)
> +#define PHASE_FREEZE_DLY_4_CL BIT(0)
> +#define ACK_LENGTH_8_CL (0x0 << 2)
> +#define ACK_LENGTH_12_CL BIT(2)
> +#define ACK_LENGTH_16_CL (0x2 << 2)
> +#define ACK_LENGTH_20_CL (0x3 << 2)
> +#define SQ_LENGTH_3 (0x0 << 4)
> +#define SQ_LENGTH_6 BIT(4)
> +#define SQ_LENGTH_9 (0x2 << 4)
> +#define SQ_LENGTH_12 (0x3 << 4)
> +#define DISCON_THRESHOLD_260 (0x0 << 6)
> +#define DISCON_THRESHOLD_270 BIT(6)
> +#define DISCON_THRESHOLD_280 (0x2 << 6)
> +#define DISCON_THRESHOLD_290 (0x3 << 6)
> +#define SQ_THRESHOLD(x) ((x) << 8)
> +#define LPF_COEF(x) ((x) << 12)
> +#define INTPL_CUR_10 (0x0 << 14)
> +#define INTPL_CUR_20 BIT(14)
> +#define INTPL_CUR_30 (0x2 << 14)
> +#define INTPL_CUR_40 (0x3 << 14)
> +
> +/* USB_PHY_ANALOG */
> +#define ANA_PWR_UP BIT(1)
> +#define ANA_PWR_DOWN BIT(2)
> +#define V2I_VCO_RATIO(x) ((x) << 7)
> +#define R_ROTATE_90 (0x0 << 10)
> +#define R_ROTATE_0 BIT(10)
> +#define MODE_TEST_EN BIT(11)
> +#define ANA_TEST_DC_CTRL(x) ((x) << 12)
> +
> +#define to_berlin_phy_priv(p) container_of((p), struct berlin_phy_priv, phy)
> +
> +static const u32 phy_berlin_pll_dividers[] = {
> + /* Berlin 2 */
> + CLK_REF_DIV(0xc) | FEEDBACK_CLK_DIV(0x54),
> + /* Berlin 2CD */
> + CLK_REF_DIV(0x6) | FEEDBACK_CLK_DIV(0x55),
> +};
> +
> +struct berlin_phy_priv {
> + void __iomem *base;
> + struct usb_phy phy;
> + struct reset_control *rst_ctrl;
> + u32 pll_divider;
> +};
> +
> +static int berlin_phy_init(struct usb_phy *phy)
> +{
> + struct berlin_phy_priv *priv = to_berlin_phy_priv(phy);
> +
> + reset_control_reset(priv->rst_ctrl);
> +
> + writel(priv->pll_divider,
> + priv->base + USB_PHY_PLL);
> + writel(CLK_STABLE | PLL_CTRL_REG | PHASE_OFF_TOL_250 | KVC0_REG_CTRL |
> + CLK_BLK_EN, priv->base + USB_PHY_PLL_CONTROL);
> + writel(V2I_VCO_RATIO(0x5) | R_ROTATE_0 | ANA_TEST_DC_CTRL(0x5),
> + priv->base + USB_PHY_ANALOG);
> + writel(PHASE_FREEZE_DLY_4_CL | ACK_LENGTH_16_CL | SQ_LENGTH_12 |
> + DISCON_THRESHOLD_260 | SQ_THRESHOLD(0xa) | LPF_COEF(0x2) |
> + INTPL_CUR_30, priv->base + USB_PHY_RX_CTRL);
> +
> + writel(TX_VDD12_13 | TX_OUT_AMP(0x3), priv->base + USB_PHY_TX_CTRL1);
> + writel(EXT_HS_RCAL_EN | IMPCAL_VTH_DIV(0x3) | EXT_RS_RCAL_DIV(0x4),
> + priv->base + USB_PHY_TX_CTRL0);
> +
> + writel(EXT_HS_RCAL_EN | IMPCAL_VTH_DIV(0x3) | EXT_RS_RCAL_DIV(0x4) |
> + EXT_FS_RCAL_DIV(0x2), priv->base + USB_PHY_TX_CTRL0);
> +
> + writel(EXT_HS_RCAL_EN | IMPCAL_VTH_DIV(0x3) | EXT_RS_RCAL_DIV(0x4),
> + priv->base + USB_PHY_TX_CTRL0);
> + writel(TX_CHAN_CTRL_REG(0xf) | DRV_SLEWRATE(0x3) | IMP_CAL_FS_HS_DLY_3 |
> + FS_DRV_EN_MASK(0xd), priv->base + USB_PHY_TX_CTRL2);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id phy_berlin_sata_of_match[] = {
> + {
> + .compatible = "marvell,berlin2-usb-phy",
> + .data = &phy_berlin_pll_dividers[0],
> + },
> + {
> + .compatible = "marvell,berlin2cd-usb-phy",
> + .data = &phy_berlin_pll_dividers[1],
> + },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, phy_berlin_sata_of_match);
> +
> +static int berlin_phy_probe(struct platform_device *pdev)
> +{
> + const struct of_device_id *match =
> + of_match_device(phy_berlin_sata_of_match, &pdev->dev);
> + struct berlin_phy_priv *priv;
> + struct resource *res;
> +
> + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + priv->base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(priv->base))
> + return PTR_ERR(priv->base);
> +
> + priv->rst_ctrl = devm_reset_control_get(&pdev->dev, NULL);
> + if (IS_ERR(priv->rst_ctrl))
> + return PTR_ERR(priv->rst_ctrl);
> +
> + priv->pll_divider = *((u32 *)match->data);
> +
> + priv->phy.io_priv = priv->base;
> + priv->phy.dev = &pdev->dev;
> + priv->phy.label = "phy-berlin-usb";
> + priv->phy.init = berlin_phy_init;
> + priv->phy.type = USB_PHY_TYPE_USB2;
> +
> + platform_set_drvdata(pdev, priv);
> +
> + return usb_add_phy_dev(&priv->phy);
> +}
> +
> +static struct platform_driver phy_berlin_usb_driver = {
> + .probe = berlin_phy_probe,
> + .driver = {
> + .name = "phy-berlin-usb",
> + .owner = THIS_MODULE,
> + .of_match_table = phy_berlin_sata_of_match,
> + },
> +};
> +module_platform_driver(phy_berlin_usb_driver);
> +
> +MODULE_AUTHOR("Antoine T?nart <[email protected]>");
> +MODULE_DESCRIPTION("Marvell Berlin USB PHY driver");
> +MODULE_LICENSE("GPL");
> --
> 1.9.1
>

--
balbi


Attachments:
(No filename) (8.69 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments

2014-06-27 16:06:03

by Antoine Tenart

[permalink] [raw]
Subject: Re: [PATCH v2 05/12] usb: phy: add the Berlin USB PHY driver

Hi Felipe,

On Fri, Jun 27, 2014 at 10:56:22AM -0500, Felipe Balbi wrote:
> On Tue, Jun 24, 2014 at 12:35:14PM +0200, Antoine T?nart wrote:
> > Add the driver driving the Marvell Berlin USB PHY. This allows to
> > initialize the PHY and to use it from the USB driver later.
> >
> > Signed-off-by: Antoine T?nart <[email protected]>
>
> since this is a brand new driver, it should go to drivers/phy instead.

This PHY is used by a ChipIdea USB driver, which uses the provided
common function for ChipIdea. These functions use the usb_phy framework.
That's why this PHY driver is there.

Antoine

--
Antoine T?nart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

2014-06-27 23:05:09

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH v2 05/12] usb: phy: add the Berlin USB PHY driver

On Fri, Jun 27, 2014 at 06:05:57PM +0200, Antoine T?nart wrote:
> Hi Felipe,
>
> On Fri, Jun 27, 2014 at 10:56:22AM -0500, Felipe Balbi wrote:
> > On Tue, Jun 24, 2014 at 12:35:14PM +0200, Antoine T?nart wrote:
> > > Add the driver driving the Marvell Berlin USB PHY. This allows to
> > > initialize the PHY and to use it from the USB driver later.
> > >
> > > Signed-off-by: Antoine T?nart <[email protected]>
> >
> > since this is a brand new driver, it should go to drivers/phy instead.
>
> This PHY is used by a ChipIdea USB driver, which uses the provided
> common function for ChipIdea. These functions use the usb_phy framework.
> That's why this PHY driver is there.

right, but you can add support for the new PHY layer which would help
other users convert their PHY drivers to the new PHY framework.

--
balbi


Attachments:
(No filename) (841.00 B)
signature.asc (819.00 B)
Digital signature
Download all attachments

2014-06-30 13:45:52

by Antoine Tenart

[permalink] [raw]
Subject: Re: [PATCH v2 07/12] usb: chipidea: add a generic driver

Peter,

On Fri, Jun 27, 2014 at 11:25:07AM +0800, Peter Chen wrote:
> On Tue, Jun 24, 2014 at 12:35:16PM +0200, Antoine T?nart wrote:
> >
> > ifneq ($(CONFIG_OF),)
> > obj-$(CONFIG_USB_CHIPIDEA) += usbmisc_imx.o ci_hdrc_imx.o
> > + obj-$(CONFIG_USB_CHIPIDEA) += ci_hdrc_generic.o
> > endif
>
> As a generic driver, you may need to support both dt and non-dt
> solution.

Since the dt is now the best practice and since there is no need (yet)
for a non-dt usage of this driver shouldn't we let anyone needing it
implement it when the time comes?

> > +static int ci_hdrc_generic_probe(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct ci_hdrc_generic_priv *priv;
> > + struct ci_hdrc_platform_data ci_pdata = {
> > + .name = "ci_hdrc",
>
> How about this using dev_name(&pdev->dev) for name?

Yes, why not. I don't have a strong preference.

> > +
> > +clk_err:
> > + clk_disable_unprepare(priv->clk);
>
> You may need to add "if (!IS_ERR(priv->clk))"

Right! I'll update this.

> > +
> > +static const struct of_device_id ci_hdrc_generic_of_match[] = {
> > + { .compatible = "chipidea-usb" },
> > + { }
> > +};
>
> Even as a generic driver, you can also use your own compatible string.

Well, there is nothing specific about the Berlin CI. Some subsystems
use the 'generic' keyword in these cases. Do you see a particular reason
I should use some Berlin related compatible here?


Thanks for the review!

Antoine

--
Antoine T?nart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

2014-06-30 13:55:29

by Antoine Tenart

[permalink] [raw]
Subject: Re: [PATCH v2 07/12] usb: chipidea: add a generic driver

Hello,

On Tue, Jun 24, 2014 at 07:51:01PM +0900, Jingoo Han wrote:
> On Tuesday, June 24, 2014 7:35 PM, Antoine Tenart wrote:
> >
> > Add a generic ChipIdea driver, with optional PHY and clock, to support
> > ChipIdea controllers that doesn't need specific functions.
>
> s/doesn't/don't
>
> >
> > Needed for the Marvell Berlin SoCs SUB controllers.
>
> s/SUB/USB

Right, I'll fix these.

>
> > +
> > +MODULE_DESCRIPTION("Generic ChipIdea HDRC USB binding");
> > +MODULE_AUTHOR("Antoine T?nart <[email protected]>");
> > +MODULE_LICENSE("GPL");
>
> How about "GPL v2"?

Well, "GPL" stands for "GNU Public License v2 or later" as documented
in:
http://lxr.free-electrons.com/source/include/linux/module.h#L100

Is there a reason I should use "GPLv2"? Or is this a best practice?

Thanks!

Antoine

--
Antoine T?nart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

2014-06-30 14:52:17

by Antoine Tenart

[permalink] [raw]
Subject: Re: [PATCH v2 05/12] usb: phy: add the Berlin USB PHY driver

Felipe,

On Fri, Jun 27, 2014 at 06:04:33PM -0500, Felipe Balbi wrote:
> On Fri, Jun 27, 2014 at 06:05:57PM +0200, Antoine T?nart wrote:
> > Hi Felipe,
> >
> > On Fri, Jun 27, 2014 at 10:56:22AM -0500, Felipe Balbi wrote:
> > > On Tue, Jun 24, 2014 at 12:35:14PM +0200, Antoine T?nart wrote:
> > > > Add the driver driving the Marvell Berlin USB PHY. This allows to
> > > > initialize the PHY and to use it from the USB driver later.
> > > >
> > > > Signed-off-by: Antoine T?nart <[email protected]>
> > >
> > > since this is a brand new driver, it should go to drivers/phy instead.
> >
> > This PHY is used by a ChipIdea USB driver, which uses the provided
> > common function for ChipIdea. These functions use the usb_phy framework.
> > That's why this PHY driver is there.
>
> right, but you can add support for the new PHY layer which would help
> other users convert their PHY drivers to the new PHY framework.

Adding the support for the new PHY layer in the common CI code is not
complicated, but these functions also use some parts from usb hcd or usb
otg which are only usb_phy compatible.

Shouldn't these parts add the new PHY support first, to avoid ending up
with a fairly big series, quite long to do and complicated to review?

Antoine

--
Antoine T?nart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

2014-06-30 18:34:42

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH v2 05/12] usb: phy: add the Berlin USB PHY driver

On Mon, Jun 30, 2014 at 04:52:11PM +0200, Antoine T?nart wrote:
> Felipe,
>
> On Fri, Jun 27, 2014 at 06:04:33PM -0500, Felipe Balbi wrote:
> > On Fri, Jun 27, 2014 at 06:05:57PM +0200, Antoine T?nart wrote:
> > > Hi Felipe,
> > >
> > > On Fri, Jun 27, 2014 at 10:56:22AM -0500, Felipe Balbi wrote:
> > > > On Tue, Jun 24, 2014 at 12:35:14PM +0200, Antoine T?nart wrote:
> > > > > Add the driver driving the Marvell Berlin USB PHY. This allows to
> > > > > initialize the PHY and to use it from the USB driver later.
> > > > >
> > > > > Signed-off-by: Antoine T?nart <[email protected]>
> > > >
> > > > since this is a brand new driver, it should go to drivers/phy instead.
> > >
> > > This PHY is used by a ChipIdea USB driver, which uses the provided
> > > common function for ChipIdea. These functions use the usb_phy framework.
> > > That's why this PHY driver is there.
> >
> > right, but you can add support for the new PHY layer which would help
> > other users convert their PHY drivers to the new PHY framework.
>
> Adding the support for the new PHY layer in the common CI code is not
> complicated, but these functions also use some parts from usb hcd or usb
> otg which are only usb_phy compatible.
>
> Shouldn't these parts add the new PHY support first, to avoid ending up
> with a fairly big series, quite long to do and complicated to review?

sure, patches are welcome. :-)

--
balbi


Attachments:
(No filename) (1.39 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments

2014-07-01 01:53:16

by Peter Chen

[permalink] [raw]
Subject: Re: [PATCH v2 07/12] usb: chipidea: add a generic driver

On Mon, Jun 30, 2014 at 03:33:13PM +0200, Antoine T?nart wrote:
> Peter,
>
> On Fri, Jun 27, 2014 at 11:25:07AM +0800, Peter Chen wrote:
> > On Tue, Jun 24, 2014 at 12:35:16PM +0200, Antoine T?nart wrote:
> > >
> > > ifneq ($(CONFIG_OF),)
> > > obj-$(CONFIG_USB_CHIPIDEA) += usbmisc_imx.o ci_hdrc_imx.o
> > > + obj-$(CONFIG_USB_CHIPIDEA) += ci_hdrc_generic.o
> > > endif
> >
> > As a generic driver, you may need to support both dt and non-dt
> > solution.
>
> Since the dt is now the best practice and since there is no need (yet)
> for a non-dt usage of this driver shouldn't we let anyone needing it
> implement it when the time comes?
>

No, at least your code structure should support both dt and non-dt,
and let the compile pass for non-dt platform if you don't have one.
Then, someone with non-dt platform can change few to support it.
A good example is: drivers/usb/host/ehci-platform.c

> > > +static int ci_hdrc_generic_probe(struct platform_device *pdev)
> > > +{
> > > + struct device *dev = &pdev->dev;
> > > + struct ci_hdrc_generic_priv *priv;
> > > + struct ci_hdrc_platform_data ci_pdata = {
> > > + .name = "ci_hdrc",
> >
> > How about this using dev_name(&pdev->dev) for name?
>
> Yes, why not. I don't have a strong preference.
>
> > > +
> > > +clk_err:
> > > + clk_disable_unprepare(priv->clk);
> >
> > You may need to add "if (!IS_ERR(priv->clk))"
>
> Right! I'll update this.
>
> > > +
> > > +static const struct of_device_id ci_hdrc_generic_of_match[] = {
> > > + { .compatible = "chipidea-usb" },
> > > + { }
> > > +};
> >
> > Even as a generic driver, you can also use your own compatible string.
>
> Well, there is nothing specific about the Berlin CI. Some subsystems
> use the 'generic' keyword in these cases. Do you see a particular reason
> I should use some Berlin related compatible here?
>

Not must, one suggestion is: can you change the compatible string
to "chipidea-usb-generic"?

--

Best Regards,
Peter Chen

2014-07-01 07:24:32

by Antoine Tenart

[permalink] [raw]
Subject: Re: [PATCH v2 07/12] usb: chipidea: add a generic driver

Peter,

On Tue, Jul 01, 2014 at 08:21:14AM +0800, Peter Chen wrote:
> On Mon, Jun 30, 2014 at 03:33:13PM +0200, Antoine T?nart wrote:
> > On Fri, Jun 27, 2014 at 11:25:07AM +0800, Peter Chen wrote:
> > > On Tue, Jun 24, 2014 at 12:35:16PM +0200, Antoine T?nart wrote:
> > > >
> > > > ifneq ($(CONFIG_OF),)
> > > > obj-$(CONFIG_USB_CHIPIDEA) += usbmisc_imx.o ci_hdrc_imx.o
> > > > + obj-$(CONFIG_USB_CHIPIDEA) += ci_hdrc_generic.o
> > > > endif
> > >
> > > As a generic driver, you may need to support both dt and non-dt
> > > solution.
> >
> > Since the dt is now the best practice and since there is no need (yet)
> > for a non-dt usage of this driver shouldn't we let anyone needing it
> > implement it when the time comes?
> >
>
> No, at least your code structure should support both dt and non-dt,
> and let the compile pass for non-dt platform if you don't have one.
> Then, someone with non-dt platform can change few to support it.
> A good example is: drivers/usb/host/ehci-platform.c

Ok. I'll isolate dt-specific code in the probe, and remove the CONFIG_OF
dependency.

>
> > > > +static int ci_hdrc_generic_probe(struct platform_device *pdev)
> > > > +{
> > > > + struct device *dev = &pdev->dev;
> > > > + struct ci_hdrc_generic_priv *priv;
> > > > + struct ci_hdrc_platform_data ci_pdata = {
> > > > + .name = "ci_hdrc",
> > >
> > > How about this using dev_name(&pdev->dev) for name?
> >
> > Yes, why not. I don't have a strong preference.
> >
> > > > +
> > > > +clk_err:
> > > > + clk_disable_unprepare(priv->clk);
> > >
> > > You may need to add "if (!IS_ERR(priv->clk))"
> >
> > Right! I'll update this.
> >
> > > > +
> > > > +static const struct of_device_id ci_hdrc_generic_of_match[] = {
> > > > + { .compatible = "chipidea-usb" },
> > > > + { }
> > > > +};
> > >
> > > Even as a generic driver, you can also use your own compatible string.
> >
> > Well, there is nothing specific about the Berlin CI. Some subsystems
> > use the 'generic' keyword in these cases. Do you see a particular reason
> > I should use some Berlin related compatible here?
> >
>
> Not must, one suggestion is: can you change the compatible string
> to "chipidea-usb-generic"?

Sounds good, I'll update.


Antoine

--
Antoine T?nart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

2014-07-01 08:55:44

by Sebastian Hesselbarth

[permalink] [raw]
Subject: Re: [PATCH v2 07/12] usb: chipidea: add a generic driver

On 07/01/2014 02:21 AM, Peter Chen wrote:
> On Mon, Jun 30, 2014 at 03:33:13PM +0200, Antoine T?nart wrote:
>> On Fri, Jun 27, 2014 at 11:25:07AM +0800, Peter Chen wrote:
>>> On Tue, Jun 24, 2014 at 12:35:16PM +0200, Antoine T?nart wrote:
>>>> +
>>>> +static const struct of_device_id ci_hdrc_generic_of_match[] = {
>>>> + { .compatible = "chipidea-usb" },
>>>> + { }
>>>> +};
>>>
>>> Even as a generic driver, you can also use your own compatible string.
>>
>> Well, there is nothing specific about the Berlin CI. Some subsystems
>> use the 'generic' keyword in these cases. Do you see a particular reason
>> I should use some Berlin related compatible here?
>
> Not must, one suggestion is: can you change the compatible string
> to "chipidea-usb-generic"?

I don't know about ChipIdea/ARC/DW's product portfolio but I guess
the compatible should also carry '2.0' or 'usb2' in it. Or we just
use some version number like 'chipidea,ci13000' or 'chipidea,ci13xxx'.

Sebastian

2014-07-01 10:02:19

by Peter Chen

[permalink] [raw]
Subject: Re: [PATCH v2 07/12] usb: chipidea: add a generic driver

On Tue, Jul 01, 2014 at 10:55:37AM +0200, Sebastian Hesselbarth wrote:
> On 07/01/2014 02:21 AM, Peter Chen wrote:
> >On Mon, Jun 30, 2014 at 03:33:13PM +0200, Antoine T?nart wrote:
> >>On Fri, Jun 27, 2014 at 11:25:07AM +0800, Peter Chen wrote:
> >>>On Tue, Jun 24, 2014 at 12:35:16PM +0200, Antoine T?nart wrote:
> >>>>+
> >>>>+static const struct of_device_id ci_hdrc_generic_of_match[] = {
> >>>>+ { .compatible = "chipidea-usb" },
> >>>>+ { }
> >>>>+};
> >>>
> >>>Even as a generic driver, you can also use your own compatible string.
> >>
> >>Well, there is nothing specific about the Berlin CI. Some subsystems
> >>use the 'generic' keyword in these cases. Do you see a particular reason
> >>I should use some Berlin related compatible here?
> >
> >Not must, one suggestion is: can you change the compatible string
> >to "chipidea-usb-generic"?
>
> I don't know about ChipIdea/ARC/DW's product portfolio but I guess
> the compatible should also carry '2.0' or 'usb2' in it. Or we just
> use some version number like 'chipidea,ci13000' or 'chipidea,ci13xxx'.
>
> Sebastian
>

The recommended format for compatible string is: "manufacturer,model",
I agree with "chipidea,ci13xxx", thanks.

--

Best Regards,
Peter Chen

2014-07-01 10:42:59

by Alexandre Belloni

[permalink] [raw]
Subject: Re: [PATCH v2 07/12] usb: chipidea: add a generic driver

On 01/07/2014 at 16:30:08 +0800, Peter Chen wrote :
> > >>Well, there is nothing specific about the Berlin CI. Some subsystems
> > >>use the 'generic' keyword in these cases. Do you see a particular reason
> > >>I should use some Berlin related compatible here?
> > >
> > >Not must, one suggestion is: can you change the compatible string
> > >to "chipidea-usb-generic"?
> >
> > I don't know about ChipIdea/ARC/DW's product portfolio but I guess
> > the compatible should also carry '2.0' or 'usb2' in it. Or we just
> > use some version number like 'chipidea,ci13000' or 'chipidea,ci13xxx'.
> >
>
> The recommended format for compatible string is: "manufacturer,model",
> I agree with "chipidea,ci13xxx", thanks.
>

I think we should probably avoid using wildcards in the compatible
string.

--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

2014-07-01 15:18:46

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH v2 07/12] usb: chipidea: add a generic driver

Hello.

On 07/01/2014 02:42 PM, Alexandre Belloni wrote:

>>>>> Well, there is nothing specific about the Berlin CI. Some subsystems
>>>>> use the 'generic' keyword in these cases. Do you see a particular reason
>>>>> I should use some Berlin related compatible here?

>>>> Not must, one suggestion is: can you change the compatible string
>>>> to "chipidea-usb-generic"?

>>> I don't know about ChipIdea/ARC/DW's product portfolio but I guess
>>> the compatible should also carry '2.0' or 'usb2' in it. Or we just
>>> use some version number like 'chipidea,ci13000' or 'chipidea,ci13xxx'.

>> The recommended format for compatible string is: "manufacturer,model",
>> I agree with "chipidea,ci13xxx", thanks.

> I think we should probably avoid using wildcards in the compatible
> string.

I'm sure wildcards shouldn't be allowed there. :-)

WBR, Sergei

2014-07-02 01:10:06

by Peter Chen

[permalink] [raw]
Subject: RE: [PATCH v2 07/12] usb: chipidea: add a generic driver


>
> Hello.
>
> On 07/01/2014 02:42 PM, Alexandre Belloni wrote:
>
> >>>>> Well, there is nothing specific about the Berlin CI. Some
> >>>>> subsystems use the 'generic' keyword in these cases. Do you see a
> >>>>> particular reason I should use some Berlin related compatible here?
>
> >>>> Not must, one suggestion is: can you change the compatible string
> >>>> to "chipidea-usb-generic"?
>
> >>> I don't know about ChipIdea/ARC/DW's product portfolio but I guess
> >>> the compatible should also carry '2.0' or 'usb2' in it. Or we just
> >>> use some version number like 'chipidea,ci13000' or 'chipidea,ci13xxx'.
>
> >> The recommended format for compatible string is:
> >> "manufacturer,model", I agree with "chipidea,ci13xxx", thanks.
>
> > I think we should probably avoid using wildcards in the compatible
> > string.
>
> I'm sure wildcards shouldn't be allowed there. :-)

Then, what's your guys recommend, how about "chipidea,usb2-generic"?

Peter

Subject: Re: [PATCH v2 07/12] usb: chipidea: add a generic driver

Since its a generic driver, support for configuring the dma_mask using
dma_coerce_mask_and_coherent would be good.

Regards,
Punnaiah

On Tue, Jun 24, 2014 at 4:05 PM, Antoine Ténart
<[email protected]> wrote:
> Add a generic ChipIdea driver, with optional PHY and clock, to support
> ChipIdea controllers that doesn't need specific functions.
>
> Needed for the Marvell Berlin SoCs SUB controllers.
>
> Signed-off-by: Antoine Ténart <[email protected]>
> ---
> drivers/usb/chipidea/Makefile | 1 +
> drivers/usb/chipidea/ci_hdrc_generic.c | 108 +++++++++++++++++++++++++++++++++
> 2 files changed, 109 insertions(+)
> create mode 100644 drivers/usb/chipidea/ci_hdrc_generic.c
>
> diff --git a/drivers/usb/chipidea/Makefile b/drivers/usb/chipidea/Makefile
> index 2f099c7df7b5..c705f0fe7a00 100644
> --- a/drivers/usb/chipidea/Makefile
> +++ b/drivers/usb/chipidea/Makefile
> @@ -20,4 +20,5 @@ endif
>
> ifneq ($(CONFIG_OF),)
> obj-$(CONFIG_USB_CHIPIDEA) += usbmisc_imx.o ci_hdrc_imx.o
> + obj-$(CONFIG_USB_CHIPIDEA) += ci_hdrc_generic.o
> endif
> diff --git a/drivers/usb/chipidea/ci_hdrc_generic.c b/drivers/usb/chipidea/ci_hdrc_generic.c
> new file mode 100644
> index 000000000000..27520710a1f7
> --- /dev/null
> +++ b/drivers/usb/chipidea/ci_hdrc_generic.c
> @@ -0,0 +1,108 @@
> +/*
> + * Copyright (C) 2014 Marvell Technology Group Ltd.
> + *
> + * Antoine Ténart <[email protected]>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/usb/chipidea.h>
> +#include <linux/usb/hcd.h>
> +#include <linux/usb/ulpi.h>
> +
> +#include "ci.h"
> +
> +struct ci_hdrc_generic_priv {
> + struct platform_device *ci_pdev;
> + struct clk *clk;
> +};
> +
> +static int ci_hdrc_generic_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct ci_hdrc_generic_priv *priv;
> + struct ci_hdrc_platform_data ci_pdata = {
> + .name = "ci_hdrc",
> + .capoffset = DEF_CAPOFFSET,
> + .flags = CI_HDRC_REQUIRE_TRANSCEIVER |
> + CI_HDRC_DISABLE_STREAMING,
> + };
> + int ret;
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + priv->clk = devm_clk_get(dev, NULL);
> + if (!IS_ERR(priv->clk)) {
> + ret = clk_prepare_enable(priv->clk);
> + if (ret) {
> + dev_err(dev, "failed to enable the clock: %d\n", ret);
> + return ret;
> + }
> + }
> +
> + ci_pdata.phy = devm_usb_get_phy_by_phandle(dev, "usb-phy", 0);
> + if (IS_ERR(ci_pdata.phy))
> + /* PHY is optional */
> + ci_pdata.phy = NULL;
> +
> + priv->ci_pdev = ci_hdrc_add_device(dev, pdev->resource,
> + pdev->num_resources, &ci_pdata);
> + if (IS_ERR(priv->ci_pdev)) {
> + ret = PTR_ERR(priv->ci_pdev);
> + if (ret != -EPROBE_DEFER)
> + dev_err(dev,
> + "failed to register ci_hdrc platform device: %d\n",
> + ret);
> + goto clk_err;
> + }
> +
> + platform_set_drvdata(pdev, priv);
> +
> + pm_runtime_no_callbacks(dev);
> + pm_runtime_enable(dev);
> +
> + return 0;
> +
> +clk_err:
> + clk_disable_unprepare(priv->clk);
> + return ret;
> +}
> +
> +static int ci_hdrc_generic_remove(struct platform_device *pdev)
> +{
> + struct ci_hdrc_generic_priv *priv = platform_get_drvdata(pdev);
> +
> + pm_runtime_disable(&pdev->dev);
> + ci_hdrc_remove_device(priv->ci_pdev);
> + clk_disable_unprepare(priv->clk);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id ci_hdrc_generic_of_match[] = {
> + { .compatible = "chipidea-usb" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, ci_hdrc_generic_of_match);
> +
> +static struct platform_driver ci_hdrc_generic_driver = {
> + .probe = ci_hdrc_generic_probe,
> + .remove = ci_hdrc_generic_remove,
> + .driver = {
> + .name = "chipidea-usb",
> + .owner = THIS_MODULE,
> + .of_match_table = ci_hdrc_generic_of_match,
> + },
> +};
> +module_platform_driver(ci_hdrc_generic_driver);
> +
> +MODULE_DESCRIPTION("Generic ChipIdea HDRC USB binding");
> +MODULE_AUTHOR("Antoine Ténart <[email protected]>");
> +MODULE_LICENSE("GPL");
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2014-07-15 15:22:43

by Antoine Tenart

[permalink] [raw]
Subject: Re: [PATCH v2 07/12] usb: chipidea: add a generic driver

Hi guys,

On Wed, Jul 02, 2014 at 01:10:00AM +0000, Peter Chen wrote:
> > On 07/01/2014 02:42 PM, Alexandre Belloni wrote:
> >
> > >>>>> Well, there is nothing specific about the Berlin CI. Some
> > >>>>> subsystems use the 'generic' keyword in these cases. Do you see a
> > >>>>> particular reason I should use some Berlin related compatible here?
> >
> > >>>> Not must, one suggestion is: can you change the compatible string
> > >>>> to "chipidea-usb-generic"?
> >
> > >>> I don't know about ChipIdea/ARC/DW's product portfolio but I guess
> > >>> the compatible should also carry '2.0' or 'usb2' in it. Or we just
> > >>> use some version number like 'chipidea,ci13000' or 'chipidea,ci13xxx'.
> >
> > >> The recommended format for compatible string is:
> > >> "manufacturer,model", I agree with "chipidea,ci13xxx", thanks.
> >
> > > I think we should probably avoid using wildcards in the compatible
> > > string.
> >
> > I'm sure wildcards shouldn't be allowed there. :-)
>
> Then, what's your guys recommend, how about "chipidea,usb2-generic"?

So what do you think? "chipidea,ci13", "chipidea,usb2-generic" or
something else?

I tend to prefer something without the 'generic' keyword.

After updating the compatible I'll send a v3 based on USB changes
introducing the generic PHY support[1].

[1] https://lkml.org/lkml/2014/7/15/330


Antoine

--
Antoine T?nart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

2014-07-15 15:26:23

by Antoine Tenart

[permalink] [raw]
Subject: Re: [PATCH v2 07/12] usb: chipidea: add a generic driver

Hi,

On Thu, Jul 03, 2014 at 08:17:34AM +0530, punnaiah choudary kalluri wrote:
> Since its a generic driver, support for configuring the dma_mask using
> dma_coerce_mask_and_coherent would be good.
>

Should I add a DMA mask dt property? Or just not add this and wait for
someone needing it in his case?

Antoine

--
Antoine T?nart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

2014-07-16 00:37:59

by Peter Chen

[permalink] [raw]
Subject: Re: [PATCH v2 07/12] usb: chipidea: add a generic driver

On Tue, Jul 15, 2014 at 05:22:30PM +0200, Antoine T?nart wrote:
> Hi guys,
>
> On Wed, Jul 02, 2014 at 01:10:00AM +0000, Peter Chen wrote:
> > > On 07/01/2014 02:42 PM, Alexandre Belloni wrote:
> > >
> > > >>>>> Well, there is nothing specific about the Berlin CI. Some
> > > >>>>> subsystems use the 'generic' keyword in these cases. Do you see a
> > > >>>>> particular reason I should use some Berlin related compatible here?
> > >
> > > >>>> Not must, one suggestion is: can you change the compatible string
> > > >>>> to "chipidea-usb-generic"?
> > >
> > > >>> I don't know about ChipIdea/ARC/DW's product portfolio but I guess
> > > >>> the compatible should also carry '2.0' or 'usb2' in it. Or we just
> > > >>> use some version number like 'chipidea,ci13000' or 'chipidea,ci13xxx'.
> > >
> > > >> The recommended format for compatible string is:
> > > >> "manufacturer,model", I agree with "chipidea,ci13xxx", thanks.
> > >
> > > > I think we should probably avoid using wildcards in the compatible
> > > > string.
> > >
> > > I'm sure wildcards shouldn't be allowed there. :-)
> >
> > Then, what's your guys recommend, how about "chipidea,usb2-generic"?
>
> So what do you think? "chipidea,ci13", "chipidea,usb2-generic" or
> something else?
>

Do you agree to use "chipidea,usb2", And add ci13xxx at
MODULE_DESCRIPTION?

--

Best Regards,
Peter Chen

2014-07-16 00:50:02

by Peter Chen

[permalink] [raw]
Subject: Re: [PATCH v2 07/12] usb: chipidea: add a generic driver

On Tue, Jul 15, 2014 at 05:24:37PM +0200, Antoine T?nart wrote:
> Hi,
>
> On Thu, Jul 03, 2014 at 08:17:34AM +0530, punnaiah choudary kalluri wrote:
> > Since its a generic driver, support for configuring the dma_mask using
> > dma_coerce_mask_and_coherent would be good.
> >
>
> Should I add a DMA mask dt property? Or just not add this and wait for
> someone needing it in his case?
>

As a dt property, since most of platforms will use it. For PIO mode, the
property can be NULL.

--

Best Regards,
Peter Chen