* [PATCH v3 1/2] phy-sun4i-usb: Use of_match_node to get model specific config data
@ 2015-11-25 16:50 ` Hans de Goede
0 siblings, 0 replies; 22+ messages in thread
From: Hans de Goede @ 2015-11-25 16:50 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: Reinder E.N. de Haan, Maxime Ripard, Chen-Yu Tsai,
linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Hans de Goede
Use of_match_node instead of calling of_device_is_compatible a ton of
times to get model specific config data.
Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
Changes in v3:
-New patch in v3 of this patch-set
---
drivers/phy/phy-sun4i-usb.c | 130 +++++++++++++++++++++++++++++---------------
1 file changed, 85 insertions(+), 45 deletions(-)
diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index b12964b..1d8f85d 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -88,12 +88,22 @@
#define DEBOUNCE_TIME msecs_to_jiffies(50)
#define POLL_TIME msecs_to_jiffies(250)
+enum sun4i_usb_phy_type {
+ sun4i_a10_phy,
+ sun8i_a33_phy,
+};
+
+struct sun4i_usb_phy_cfg {
+ int num_phys;
+ u32 disc_thresh;
+ enum sun4i_usb_phy_type type;
+ bool dedicated_clocks;
+};
+
struct sun4i_usb_phy_data {
void __iomem *base;
+ const struct sun4i_usb_phy_cfg *cfg;
struct mutex mutex;
- int num_phys;
- u32 disc_thresh;
- bool has_a33_phyctl;
struct sun4i_usb_phy {
struct phy *phy;
void __iomem *pmu;
@@ -164,12 +174,15 @@ static void sun4i_usb_phy_write(struct sun4i_usb_phy *phy, u32 addr, u32 data,
mutex_lock(&phy_data->mutex);
- if (phy_data->has_a33_phyctl) {
+ switch (phy_data->cfg->type) {
+ case sun4i_a10_phy:
+ phyctl = phy_data->base + REG_PHYCTL_A10;
+ break;
+ case sun8i_a33_phy:
phyctl = phy_data->base + REG_PHYCTL_A33;
/* A33 needs us to set phyctl to 0 explicitly */
writel(0, phyctl);
- } else {
- phyctl = phy_data->base + REG_PHYCTL_A10;
+ break;
}
for (i = 0; i < len; i++) {
@@ -249,7 +262,8 @@ static int sun4i_usb_phy_init(struct phy *_phy)
sun4i_usb_phy_write(phy, PHY_TX_AMPLITUDE_TUNE, 0x14, 5);
/* Disconnect threshold adjustment */
- sun4i_usb_phy_write(phy, PHY_DISCON_TH_SEL, data->disc_thresh, 2);
+ sun4i_usb_phy_write(phy, PHY_DISCON_TH_SEL,
+ data->cfg->disc_thresh, 2);
sun4i_usb_phy_passby(phy, 1);
@@ -476,7 +490,7 @@ static struct phy *sun4i_usb_phy_xlate(struct device *dev,
{
struct sun4i_usb_phy_data *data = dev_get_drvdata(dev);
- if (args->args[0] >= data->num_phys)
+ if (args->args[0] >= data->cfg->num_phys)
return ERR_PTR(-ENODEV);
return data->phys[args->args[0]].phy;
@@ -499,6 +513,59 @@ static int sun4i_usb_phy_remove(struct platform_device *pdev)
return 0;
}
+static const struct sun4i_usb_phy_cfg sun4i_a10_cfg = {
+ .num_phys = 3,
+ .disc_thresh = 3,
+ .type = sun4i_a10_phy,
+ .dedicated_clocks = false,
+};
+
+static const struct sun4i_usb_phy_cfg sun5i_a13_cfg = {
+ .num_phys = 2,
+ .disc_thresh = 2,
+ .type = sun4i_a10_phy,
+ .dedicated_clocks = false,
+};
+
+static const struct sun4i_usb_phy_cfg sun6i_a31_cfg = {
+ .num_phys = 3,
+ .disc_thresh = 3,
+ .type = sun4i_a10_phy,
+ .dedicated_clocks = true,
+};
+
+static const struct sun4i_usb_phy_cfg sun7i_a20_cfg = {
+ .num_phys = 3,
+ .disc_thresh = 2,
+ .type = sun4i_a10_phy,
+ .dedicated_clocks = false,
+};
+
+static const struct sun4i_usb_phy_cfg sun8i_a23_cfg = {
+ .num_phys = 2,
+ .disc_thresh = 3,
+ .type = sun4i_a10_phy,
+ .dedicated_clocks = true,
+};
+
+static const struct sun4i_usb_phy_cfg sun8i_a33_cfg = {
+ .num_phys = 2,
+ .disc_thresh = 3,
+ .type = sun8i_a33_phy,
+ .dedicated_clocks = true,
+};
+
+static const struct of_device_id sun4i_usb_phy_of_match[] = {
+ { .compatible = "allwinner,sun4i-a10-usb-phy", .data = &sun4i_a10_cfg },
+ { .compatible = "allwinner,sun5i-a13-usb-phy", .data = &sun5i_a13_cfg },
+ { .compatible = "allwinner,sun6i-a31-usb-phy", .data = &sun6i_a31_cfg },
+ { .compatible = "allwinner,sun7i-a20-usb-phy", .data = &sun7i_a20_cfg },
+ { .compatible = "allwinner,sun8i-a23-usb-phy", .data = &sun8i_a23_cfg },
+ { .compatible = "allwinner,sun8i-a33-usb-phy", .data = &sun8i_a33_cfg },
+ { },
+};
+MODULE_DEVICE_TABLE(of, sun4i_usb_phy_of_match);
+
static const unsigned int sun4i_usb_phy0_cable[] = {
EXTCON_USB,
EXTCON_USB_HOST,
@@ -511,10 +578,16 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct device_node *np = dev->of_node;
struct phy_provider *phy_provider;
- bool dedicated_clocks;
+ const struct of_device_id *match;
struct resource *res;
int i, ret;
+ match = of_match_node(sun4i_usb_phy_of_match, dev->of_node);
+ if (!match) {
+ dev_err(dev, "of_match_node() failed\n");
+ return -EINVAL;
+ }
+
data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
if (!data)
return -ENOMEM;
@@ -522,29 +595,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
mutex_init(&data->mutex);
INIT_DELAYED_WORK(&data->detect, sun4i_usb_phy0_id_vbus_det_scan);
dev_set_drvdata(dev, data);
-
- if (of_device_is_compatible(np, "allwinner,sun5i-a13-usb-phy") ||
- of_device_is_compatible(np, "allwinner,sun8i-a23-usb-phy") ||
- of_device_is_compatible(np, "allwinner,sun8i-a33-usb-phy"))
- data->num_phys = 2;
- else
- data->num_phys = 3;
-
- if (of_device_is_compatible(np, "allwinner,sun5i-a13-usb-phy") ||
- of_device_is_compatible(np, "allwinner,sun7i-a20-usb-phy"))
- data->disc_thresh = 2;
- else
- data->disc_thresh = 3;
-
- if (of_device_is_compatible(np, "allwinner,sun6i-a31-usb-phy") ||
- of_device_is_compatible(np, "allwinner,sun8i-a23-usb-phy") ||
- of_device_is_compatible(np, "allwinner,sun8i-a33-usb-phy"))
- dedicated_clocks = true;
- else
- dedicated_clocks = false;
-
- if (of_device_is_compatible(np, "allwinner,sun8i-a33-usb-phy"))
- data->has_a33_phyctl = true;
+ data->cfg = match->data;
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "phy_ctrl");
data->base = devm_ioremap_resource(dev, res);
@@ -590,7 +641,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
}
}
- for (i = 0; i < data->num_phys; i++) {
+ for (i = 0; i < data->cfg->num_phys; i++) {
struct sun4i_usb_phy *phy = data->phys + i;
char name[16];
@@ -602,7 +653,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
phy->vbus = NULL;
}
- if (dedicated_clocks)
+ if (data->cfg->dedicated_clocks)
snprintf(name, sizeof(name), "usb%d_phy", i);
else
strlcpy(name, "usb_phy", sizeof(name));
@@ -689,17 +740,6 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
return 0;
}
-static const struct of_device_id sun4i_usb_phy_of_match[] = {
- { .compatible = "allwinner,sun4i-a10-usb-phy" },
- { .compatible = "allwinner,sun5i-a13-usb-phy" },
- { .compatible = "allwinner,sun6i-a31-usb-phy" },
- { .compatible = "allwinner,sun7i-a20-usb-phy" },
- { .compatible = "allwinner,sun8i-a23-usb-phy" },
- { .compatible = "allwinner,sun8i-a33-usb-phy" },
- { },
-};
-MODULE_DEVICE_TABLE(of, sun4i_usb_phy_of_match);
-
static struct platform_driver sun4i_usb_phy_driver = {
.probe = sun4i_usb_phy_probe,
.remove = sun4i_usb_phy_remove,
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v3 1/2] phy-sun4i-usb: Use of_match_node to get model specific config data
@ 2015-11-25 16:50 ` Hans de Goede
0 siblings, 0 replies; 22+ messages in thread
From: Hans de Goede @ 2015-11-25 16:50 UTC (permalink / raw)
To: linux-arm-kernel
Use of_match_node instead of calling of_device_is_compatible a ton of
times to get model specific config data.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v3:
-New patch in v3 of this patch-set
---
drivers/phy/phy-sun4i-usb.c | 130 +++++++++++++++++++++++++++++---------------
1 file changed, 85 insertions(+), 45 deletions(-)
diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index b12964b..1d8f85d 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -88,12 +88,22 @@
#define DEBOUNCE_TIME msecs_to_jiffies(50)
#define POLL_TIME msecs_to_jiffies(250)
+enum sun4i_usb_phy_type {
+ sun4i_a10_phy,
+ sun8i_a33_phy,
+};
+
+struct sun4i_usb_phy_cfg {
+ int num_phys;
+ u32 disc_thresh;
+ enum sun4i_usb_phy_type type;
+ bool dedicated_clocks;
+};
+
struct sun4i_usb_phy_data {
void __iomem *base;
+ const struct sun4i_usb_phy_cfg *cfg;
struct mutex mutex;
- int num_phys;
- u32 disc_thresh;
- bool has_a33_phyctl;
struct sun4i_usb_phy {
struct phy *phy;
void __iomem *pmu;
@@ -164,12 +174,15 @@ static void sun4i_usb_phy_write(struct sun4i_usb_phy *phy, u32 addr, u32 data,
mutex_lock(&phy_data->mutex);
- if (phy_data->has_a33_phyctl) {
+ switch (phy_data->cfg->type) {
+ case sun4i_a10_phy:
+ phyctl = phy_data->base + REG_PHYCTL_A10;
+ break;
+ case sun8i_a33_phy:
phyctl = phy_data->base + REG_PHYCTL_A33;
/* A33 needs us to set phyctl to 0 explicitly */
writel(0, phyctl);
- } else {
- phyctl = phy_data->base + REG_PHYCTL_A10;
+ break;
}
for (i = 0; i < len; i++) {
@@ -249,7 +262,8 @@ static int sun4i_usb_phy_init(struct phy *_phy)
sun4i_usb_phy_write(phy, PHY_TX_AMPLITUDE_TUNE, 0x14, 5);
/* Disconnect threshold adjustment */
- sun4i_usb_phy_write(phy, PHY_DISCON_TH_SEL, data->disc_thresh, 2);
+ sun4i_usb_phy_write(phy, PHY_DISCON_TH_SEL,
+ data->cfg->disc_thresh, 2);
sun4i_usb_phy_passby(phy, 1);
@@ -476,7 +490,7 @@ static struct phy *sun4i_usb_phy_xlate(struct device *dev,
{
struct sun4i_usb_phy_data *data = dev_get_drvdata(dev);
- if (args->args[0] >= data->num_phys)
+ if (args->args[0] >= data->cfg->num_phys)
return ERR_PTR(-ENODEV);
return data->phys[args->args[0]].phy;
@@ -499,6 +513,59 @@ static int sun4i_usb_phy_remove(struct platform_device *pdev)
return 0;
}
+static const struct sun4i_usb_phy_cfg sun4i_a10_cfg = {
+ .num_phys = 3,
+ .disc_thresh = 3,
+ .type = sun4i_a10_phy,
+ .dedicated_clocks = false,
+};
+
+static const struct sun4i_usb_phy_cfg sun5i_a13_cfg = {
+ .num_phys = 2,
+ .disc_thresh = 2,
+ .type = sun4i_a10_phy,
+ .dedicated_clocks = false,
+};
+
+static const struct sun4i_usb_phy_cfg sun6i_a31_cfg = {
+ .num_phys = 3,
+ .disc_thresh = 3,
+ .type = sun4i_a10_phy,
+ .dedicated_clocks = true,
+};
+
+static const struct sun4i_usb_phy_cfg sun7i_a20_cfg = {
+ .num_phys = 3,
+ .disc_thresh = 2,
+ .type = sun4i_a10_phy,
+ .dedicated_clocks = false,
+};
+
+static const struct sun4i_usb_phy_cfg sun8i_a23_cfg = {
+ .num_phys = 2,
+ .disc_thresh = 3,
+ .type = sun4i_a10_phy,
+ .dedicated_clocks = true,
+};
+
+static const struct sun4i_usb_phy_cfg sun8i_a33_cfg = {
+ .num_phys = 2,
+ .disc_thresh = 3,
+ .type = sun8i_a33_phy,
+ .dedicated_clocks = true,
+};
+
+static const struct of_device_id sun4i_usb_phy_of_match[] = {
+ { .compatible = "allwinner,sun4i-a10-usb-phy", .data = &sun4i_a10_cfg },
+ { .compatible = "allwinner,sun5i-a13-usb-phy", .data = &sun5i_a13_cfg },
+ { .compatible = "allwinner,sun6i-a31-usb-phy", .data = &sun6i_a31_cfg },
+ { .compatible = "allwinner,sun7i-a20-usb-phy", .data = &sun7i_a20_cfg },
+ { .compatible = "allwinner,sun8i-a23-usb-phy", .data = &sun8i_a23_cfg },
+ { .compatible = "allwinner,sun8i-a33-usb-phy", .data = &sun8i_a33_cfg },
+ { },
+};
+MODULE_DEVICE_TABLE(of, sun4i_usb_phy_of_match);
+
static const unsigned int sun4i_usb_phy0_cable[] = {
EXTCON_USB,
EXTCON_USB_HOST,
@@ -511,10 +578,16 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct device_node *np = dev->of_node;
struct phy_provider *phy_provider;
- bool dedicated_clocks;
+ const struct of_device_id *match;
struct resource *res;
int i, ret;
+ match = of_match_node(sun4i_usb_phy_of_match, dev->of_node);
+ if (!match) {
+ dev_err(dev, "of_match_node() failed\n");
+ return -EINVAL;
+ }
+
data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
if (!data)
return -ENOMEM;
@@ -522,29 +595,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
mutex_init(&data->mutex);
INIT_DELAYED_WORK(&data->detect, sun4i_usb_phy0_id_vbus_det_scan);
dev_set_drvdata(dev, data);
-
- if (of_device_is_compatible(np, "allwinner,sun5i-a13-usb-phy") ||
- of_device_is_compatible(np, "allwinner,sun8i-a23-usb-phy") ||
- of_device_is_compatible(np, "allwinner,sun8i-a33-usb-phy"))
- data->num_phys = 2;
- else
- data->num_phys = 3;
-
- if (of_device_is_compatible(np, "allwinner,sun5i-a13-usb-phy") ||
- of_device_is_compatible(np, "allwinner,sun7i-a20-usb-phy"))
- data->disc_thresh = 2;
- else
- data->disc_thresh = 3;
-
- if (of_device_is_compatible(np, "allwinner,sun6i-a31-usb-phy") ||
- of_device_is_compatible(np, "allwinner,sun8i-a23-usb-phy") ||
- of_device_is_compatible(np, "allwinner,sun8i-a33-usb-phy"))
- dedicated_clocks = true;
- else
- dedicated_clocks = false;
-
- if (of_device_is_compatible(np, "allwinner,sun8i-a33-usb-phy"))
- data->has_a33_phyctl = true;
+ data->cfg = match->data;
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "phy_ctrl");
data->base = devm_ioremap_resource(dev, res);
@@ -590,7 +641,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
}
}
- for (i = 0; i < data->num_phys; i++) {
+ for (i = 0; i < data->cfg->num_phys; i++) {
struct sun4i_usb_phy *phy = data->phys + i;
char name[16];
@@ -602,7 +653,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
phy->vbus = NULL;
}
- if (dedicated_clocks)
+ if (data->cfg->dedicated_clocks)
snprintf(name, sizeof(name), "usb%d_phy", i);
else
strlcpy(name, "usb_phy", sizeof(name));
@@ -689,17 +740,6 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
return 0;
}
-static const struct of_device_id sun4i_usb_phy_of_match[] = {
- { .compatible = "allwinner,sun4i-a10-usb-phy" },
- { .compatible = "allwinner,sun5i-a13-usb-phy" },
- { .compatible = "allwinner,sun6i-a31-usb-phy" },
- { .compatible = "allwinner,sun7i-a20-usb-phy" },
- { .compatible = "allwinner,sun8i-a23-usb-phy" },
- { .compatible = "allwinner,sun8i-a33-usb-phy" },
- { },
-};
-MODULE_DEVICE_TABLE(of, sun4i_usb_phy_of_match);
-
static struct platform_driver sun4i_usb_phy_driver = {
.probe = sun4i_usb_phy_probe,
.remove = sun4i_usb_phy_remove,
--
2.5.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v3 2/2] phy-sun4i-usb: Add support for the host usb-phys found on the H3 SoC
2015-11-25 16:50 ` Hans de Goede
@ 2015-11-25 16:50 ` Hans de Goede
-1 siblings, 0 replies; 22+ messages in thread
From: Hans de Goede @ 2015-11-25 16:50 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: Reinder E.N. de Haan, Maxime Ripard, Chen-Yu Tsai,
linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Reinder de Haan,
Hans de Goede
From: Reinder de Haan <patchesrdh-I1/eAgTnXDYAvxtiuMwx3w@public.gmane.org>
Note this commit only adds support for phys 1-3, phy 0, the otg phy, is
not yet (fully) supported after this commit.
Signed-off-by: Reinder de Haan <patchesrdh-I1/eAgTnXDYAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
Changes in v2:
-Change break; after dev_err() to return, as intended, fixing a compiler
warning (the dev_err case should never be reached)
Changes in v3:
-Use of_match_node to get model specific config data
---
.../devicetree/bindings/phy/sun4i-usb-phy.txt | 1 +
drivers/phy/phy-sun4i-usb.c | 46 +++++++++++++++++-----
2 files changed, 38 insertions(+), 9 deletions(-)
diff --git a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
index 0cebf74..95736d7 100644
--- a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
+++ b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
@@ -9,6 +9,7 @@ Required properties:
* allwinner,sun7i-a20-usb-phy
* allwinner,sun8i-a23-usb-phy
* allwinner,sun8i-a33-usb-phy
+ * allwinner,sun8i-h3-usb-phy
- reg : a list of offset + length pairs
- reg-names :
* "phy_ctrl"
diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index 1d8f85d..2aed482 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -46,6 +46,9 @@
#define REG_PHYBIST 0x08
#define REG_PHYTUNE 0x0c
#define REG_PHYCTL_A33 0x10
+#define REG_PHY_UNK_H3 0x20
+
+#define REG_PMU_UNK_H3 0x10
#define PHYCTL_DATA BIT(7)
@@ -79,7 +82,7 @@
#define PHY_DISCON_TH_SEL 0x2a
#define PHY_SQUELCH_DETECT 0x3c
-#define MAX_PHYS 3
+#define MAX_PHYS 4
/*
* Note do not raise the debounce time, we must report Vusb high within 100ms
@@ -91,6 +94,7 @@
enum sun4i_usb_phy_type {
sun4i_a10_phy,
sun8i_a33_phy,
+ sun8i_h3_phy,
};
struct sun4i_usb_phy_cfg {
@@ -101,6 +105,7 @@ struct sun4i_usb_phy_cfg {
};
struct sun4i_usb_phy_data {
+ struct device *dev;
void __iomem *base;
const struct sun4i_usb_phy_cfg *cfg;
struct mutex mutex;
@@ -183,6 +188,9 @@ static void sun4i_usb_phy_write(struct sun4i_usb_phy *phy, u32 addr, u32 data,
/* A33 needs us to set phyctl to 0 explicitly */
writel(0, phyctl);
break;
+ case sun8i_h3_phy:
+ dev_err(phy_data->dev, "H3 usb_phy_write is not supported\n");
+ return;
}
for (i = 0; i < len; i++) {
@@ -243,6 +251,7 @@ static int sun4i_usb_phy_init(struct phy *_phy)
struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
struct sun4i_usb_phy_data *data = to_sun4i_usb_phy_data(phy);
int ret;
+ u32 val;
ret = clk_prepare_enable(phy->clk);
if (ret)
@@ -254,16 +263,26 @@ static int sun4i_usb_phy_init(struct phy *_phy)
return ret;
}
- /* Enable USB 45 Ohm resistor calibration */
- if (phy->index == 0)
- sun4i_usb_phy_write(phy, PHY_RES45_CAL_EN, 0x01, 1);
+ if (data->cfg->type == sun8i_h3_phy) {
+ if (phy->index == 0) {
+ val = readl(data->base + REG_PHY_UNK_H3);
+ writel(val & ~1, data->base + REG_PHY_UNK_H3);
+ }
- /* Adjust PHY's magnitude and rate */
- sun4i_usb_phy_write(phy, PHY_TX_AMPLITUDE_TUNE, 0x14, 5);
+ val = readl(phy->pmu + REG_PMU_UNK_H3);
+ writel(val & ~2, phy->pmu + REG_PMU_UNK_H3);
+ } else {
+ /* Enable USB 45 Ohm resistor calibration */
+ if (phy->index == 0)
+ sun4i_usb_phy_write(phy, PHY_RES45_CAL_EN, 0x01, 1);
- /* Disconnect threshold adjustment */
- sun4i_usb_phy_write(phy, PHY_DISCON_TH_SEL,
- data->cfg->disc_thresh, 2);
+ /* Adjust PHY's magnitude and rate */
+ sun4i_usb_phy_write(phy, PHY_TX_AMPLITUDE_TUNE, 0x14, 5);
+
+ /* Disconnect threshold adjustment */
+ sun4i_usb_phy_write(phy, PHY_DISCON_TH_SEL,
+ data->cfg->disc_thresh, 2);
+ }
sun4i_usb_phy_passby(phy, 1);
@@ -555,6 +574,13 @@ static const struct sun4i_usb_phy_cfg sun8i_a33_cfg = {
.dedicated_clocks = true,
};
+static const struct sun4i_usb_phy_cfg sun8i_h3_cfg = {
+ .num_phys = 4,
+ .disc_thresh = 3,
+ .type = sun8i_h3_phy,
+ .dedicated_clocks = true,
+};
+
static const struct of_device_id sun4i_usb_phy_of_match[] = {
{ .compatible = "allwinner,sun4i-a10-usb-phy", .data = &sun4i_a10_cfg },
{ .compatible = "allwinner,sun5i-a13-usb-phy", .data = &sun5i_a13_cfg },
@@ -562,6 +588,7 @@ static const struct of_device_id sun4i_usb_phy_of_match[] = {
{ .compatible = "allwinner,sun7i-a20-usb-phy", .data = &sun7i_a20_cfg },
{ .compatible = "allwinner,sun8i-a23-usb-phy", .data = &sun8i_a23_cfg },
{ .compatible = "allwinner,sun8i-a33-usb-phy", .data = &sun8i_a33_cfg },
+ { .compatible = "allwinner,sun8i-h3-usb-phy", .data = &sun8i_h3_cfg },
{ },
};
MODULE_DEVICE_TABLE(of, sun4i_usb_phy_of_match);
@@ -595,6 +622,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
mutex_init(&data->mutex);
INIT_DELAYED_WORK(&data->detect, sun4i_usb_phy0_id_vbus_det_scan);
dev_set_drvdata(dev, data);
+ data->dev = dev;
data->cfg = match->data;
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "phy_ctrl");
--
2.5.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v3 2/2] phy-sun4i-usb: Add support for the host usb-phys found on the H3 SoC
@ 2015-11-25 16:50 ` Hans de Goede
0 siblings, 0 replies; 22+ messages in thread
From: Hans de Goede @ 2015-11-25 16:50 UTC (permalink / raw)
To: linux-arm-kernel
From: Reinder de Haan <patchesrdh@mveas.com>
Note this commit only adds support for phys 1-3, phy 0, the otg phy, is
not yet (fully) supported after this commit.
Signed-off-by: Reinder de Haan <patchesrdh@mveas.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Change break; after dev_err() to return, as intended, fixing a compiler
warning (the dev_err case should never be reached)
Changes in v3:
-Use of_match_node to get model specific config data
---
.../devicetree/bindings/phy/sun4i-usb-phy.txt | 1 +
drivers/phy/phy-sun4i-usb.c | 46 +++++++++++++++++-----
2 files changed, 38 insertions(+), 9 deletions(-)
diff --git a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
index 0cebf74..95736d7 100644
--- a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
+++ b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
@@ -9,6 +9,7 @@ Required properties:
* allwinner,sun7i-a20-usb-phy
* allwinner,sun8i-a23-usb-phy
* allwinner,sun8i-a33-usb-phy
+ * allwinner,sun8i-h3-usb-phy
- reg : a list of offset + length pairs
- reg-names :
* "phy_ctrl"
diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index 1d8f85d..2aed482 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -46,6 +46,9 @@
#define REG_PHYBIST 0x08
#define REG_PHYTUNE 0x0c
#define REG_PHYCTL_A33 0x10
+#define REG_PHY_UNK_H3 0x20
+
+#define REG_PMU_UNK_H3 0x10
#define PHYCTL_DATA BIT(7)
@@ -79,7 +82,7 @@
#define PHY_DISCON_TH_SEL 0x2a
#define PHY_SQUELCH_DETECT 0x3c
-#define MAX_PHYS 3
+#define MAX_PHYS 4
/*
* Note do not raise the debounce time, we must report Vusb high within 100ms
@@ -91,6 +94,7 @@
enum sun4i_usb_phy_type {
sun4i_a10_phy,
sun8i_a33_phy,
+ sun8i_h3_phy,
};
struct sun4i_usb_phy_cfg {
@@ -101,6 +105,7 @@ struct sun4i_usb_phy_cfg {
};
struct sun4i_usb_phy_data {
+ struct device *dev;
void __iomem *base;
const struct sun4i_usb_phy_cfg *cfg;
struct mutex mutex;
@@ -183,6 +188,9 @@ static void sun4i_usb_phy_write(struct sun4i_usb_phy *phy, u32 addr, u32 data,
/* A33 needs us to set phyctl to 0 explicitly */
writel(0, phyctl);
break;
+ case sun8i_h3_phy:
+ dev_err(phy_data->dev, "H3 usb_phy_write is not supported\n");
+ return;
}
for (i = 0; i < len; i++) {
@@ -243,6 +251,7 @@ static int sun4i_usb_phy_init(struct phy *_phy)
struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
struct sun4i_usb_phy_data *data = to_sun4i_usb_phy_data(phy);
int ret;
+ u32 val;
ret = clk_prepare_enable(phy->clk);
if (ret)
@@ -254,16 +263,26 @@ static int sun4i_usb_phy_init(struct phy *_phy)
return ret;
}
- /* Enable USB 45 Ohm resistor calibration */
- if (phy->index == 0)
- sun4i_usb_phy_write(phy, PHY_RES45_CAL_EN, 0x01, 1);
+ if (data->cfg->type == sun8i_h3_phy) {
+ if (phy->index == 0) {
+ val = readl(data->base + REG_PHY_UNK_H3);
+ writel(val & ~1, data->base + REG_PHY_UNK_H3);
+ }
- /* Adjust PHY's magnitude and rate */
- sun4i_usb_phy_write(phy, PHY_TX_AMPLITUDE_TUNE, 0x14, 5);
+ val = readl(phy->pmu + REG_PMU_UNK_H3);
+ writel(val & ~2, phy->pmu + REG_PMU_UNK_H3);
+ } else {
+ /* Enable USB 45 Ohm resistor calibration */
+ if (phy->index == 0)
+ sun4i_usb_phy_write(phy, PHY_RES45_CAL_EN, 0x01, 1);
- /* Disconnect threshold adjustment */
- sun4i_usb_phy_write(phy, PHY_DISCON_TH_SEL,
- data->cfg->disc_thresh, 2);
+ /* Adjust PHY's magnitude and rate */
+ sun4i_usb_phy_write(phy, PHY_TX_AMPLITUDE_TUNE, 0x14, 5);
+
+ /* Disconnect threshold adjustment */
+ sun4i_usb_phy_write(phy, PHY_DISCON_TH_SEL,
+ data->cfg->disc_thresh, 2);
+ }
sun4i_usb_phy_passby(phy, 1);
@@ -555,6 +574,13 @@ static const struct sun4i_usb_phy_cfg sun8i_a33_cfg = {
.dedicated_clocks = true,
};
+static const struct sun4i_usb_phy_cfg sun8i_h3_cfg = {
+ .num_phys = 4,
+ .disc_thresh = 3,
+ .type = sun8i_h3_phy,
+ .dedicated_clocks = true,
+};
+
static const struct of_device_id sun4i_usb_phy_of_match[] = {
{ .compatible = "allwinner,sun4i-a10-usb-phy", .data = &sun4i_a10_cfg },
{ .compatible = "allwinner,sun5i-a13-usb-phy", .data = &sun5i_a13_cfg },
@@ -562,6 +588,7 @@ static const struct of_device_id sun4i_usb_phy_of_match[] = {
{ .compatible = "allwinner,sun7i-a20-usb-phy", .data = &sun7i_a20_cfg },
{ .compatible = "allwinner,sun8i-a23-usb-phy", .data = &sun8i_a23_cfg },
{ .compatible = "allwinner,sun8i-a33-usb-phy", .data = &sun8i_a33_cfg },
+ { .compatible = "allwinner,sun8i-h3-usb-phy", .data = &sun8i_h3_cfg },
{ },
};
MODULE_DEVICE_TABLE(of, sun4i_usb_phy_of_match);
@@ -595,6 +622,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
mutex_init(&data->mutex);
INIT_DELAYED_WORK(&data->detect, sun4i_usb_phy0_id_vbus_det_scan);
dev_set_drvdata(dev, data);
+ data->dev = dev;
data->cfg = match->data;
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "phy_ctrl");
--
2.5.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v3 2/2] phy-sun4i-usb: Add support for the host usb-phys found on the H3 SoC
2015-11-25 16:50 ` Hans de Goede
@ 2015-11-25 22:11 ` Rob Herring
-1 siblings, 0 replies; 22+ messages in thread
From: Rob Herring @ 2015-11-25 22:11 UTC (permalink / raw)
To: Hans de Goede
Cc: Kishon Vijay Abraham I, Reinder E.N. de Haan, Maxime Ripard,
Chen-Yu Tsai, linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Reinder de Haan
On Wed, Nov 25, 2015 at 05:50:02PM +0100, Hans de Goede wrote:
> From: Reinder de Haan <patchesrdh-I1/eAgTnXDYAvxtiuMwx3w@public.gmane.org>
>
> Note this commit only adds support for phys 1-3, phy 0, the otg phy, is
> not yet (fully) supported after this commit.
Shouldn't the OTG phy have a different compatible string?
>
> Signed-off-by: Reinder de Haan <patchesrdh-I1/eAgTnXDYAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> Changes in v2:
> -Change break; after dev_err() to return, as intended, fixing a compiler
> warning (the dev_err case should never be reached)
> Changes in v3:
> -Use of_match_node to get model specific config data
> ---
> .../devicetree/bindings/phy/sun4i-usb-phy.txt | 1 +
> drivers/phy/phy-sun4i-usb.c | 46 +++++++++++++++++-----
> 2 files changed, 38 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
> index 0cebf74..95736d7 100644
> --- a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
> +++ b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
> @@ -9,6 +9,7 @@ Required properties:
> * allwinner,sun7i-a20-usb-phy
> * allwinner,sun8i-a23-usb-phy
> * allwinner,sun8i-a33-usb-phy
> + * allwinner,sun8i-h3-usb-phy
> - reg : a list of offset + length pairs
> - reg-names :
> * "phy_ctrl"
> diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
> index 1d8f85d..2aed482 100644
> --- a/drivers/phy/phy-sun4i-usb.c
> +++ b/drivers/phy/phy-sun4i-usb.c
> @@ -46,6 +46,9 @@
> #define REG_PHYBIST 0x08
> #define REG_PHYTUNE 0x0c
> #define REG_PHYCTL_A33 0x10
> +#define REG_PHY_UNK_H3 0x20
> +
> +#define REG_PMU_UNK_H3 0x10
>
> #define PHYCTL_DATA BIT(7)
>
> @@ -79,7 +82,7 @@
> #define PHY_DISCON_TH_SEL 0x2a
> #define PHY_SQUELCH_DETECT 0x3c
>
> -#define MAX_PHYS 3
> +#define MAX_PHYS 4
>
> /*
> * Note do not raise the debounce time, we must report Vusb high within 100ms
> @@ -91,6 +94,7 @@
> enum sun4i_usb_phy_type {
> sun4i_a10_phy,
> sun8i_a33_phy,
> + sun8i_h3_phy,
> };
>
> struct sun4i_usb_phy_cfg {
> @@ -101,6 +105,7 @@ struct sun4i_usb_phy_cfg {
> };
>
> struct sun4i_usb_phy_data {
> + struct device *dev;
> void __iomem *base;
> const struct sun4i_usb_phy_cfg *cfg;
> struct mutex mutex;
> @@ -183,6 +188,9 @@ static void sun4i_usb_phy_write(struct sun4i_usb_phy *phy, u32 addr, u32 data,
> /* A33 needs us to set phyctl to 0 explicitly */
> writel(0, phyctl);
> break;
> + case sun8i_h3_phy:
> + dev_err(phy_data->dev, "H3 usb_phy_write is not supported\n");
> + return;
> }
>
> for (i = 0; i < len; i++) {
> @@ -243,6 +251,7 @@ static int sun4i_usb_phy_init(struct phy *_phy)
> struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
> struct sun4i_usb_phy_data *data = to_sun4i_usb_phy_data(phy);
> int ret;
> + u32 val;
>
> ret = clk_prepare_enable(phy->clk);
> if (ret)
> @@ -254,16 +263,26 @@ static int sun4i_usb_phy_init(struct phy *_phy)
> return ret;
> }
>
> - /* Enable USB 45 Ohm resistor calibration */
> - if (phy->index == 0)
> - sun4i_usb_phy_write(phy, PHY_RES45_CAL_EN, 0x01, 1);
> + if (data->cfg->type == sun8i_h3_phy) {
> + if (phy->index == 0) {
> + val = readl(data->base + REG_PHY_UNK_H3);
> + writel(val & ~1, data->base + REG_PHY_UNK_H3);
> + }
>
> - /* Adjust PHY's magnitude and rate */
> - sun4i_usb_phy_write(phy, PHY_TX_AMPLITUDE_TUNE, 0x14, 5);
> + val = readl(phy->pmu + REG_PMU_UNK_H3);
> + writel(val & ~2, phy->pmu + REG_PMU_UNK_H3);
> + } else {
> + /* Enable USB 45 Ohm resistor calibration */
> + if (phy->index == 0)
> + sun4i_usb_phy_write(phy, PHY_RES45_CAL_EN, 0x01, 1);
>
> - /* Disconnect threshold adjustment */
> - sun4i_usb_phy_write(phy, PHY_DISCON_TH_SEL,
> - data->cfg->disc_thresh, 2);
> + /* Adjust PHY's magnitude and rate */
> + sun4i_usb_phy_write(phy, PHY_TX_AMPLITUDE_TUNE, 0x14, 5);
> +
> + /* Disconnect threshold adjustment */
> + sun4i_usb_phy_write(phy, PHY_DISCON_TH_SEL,
> + data->cfg->disc_thresh, 2);
> + }
>
> sun4i_usb_phy_passby(phy, 1);
>
> @@ -555,6 +574,13 @@ static const struct sun4i_usb_phy_cfg sun8i_a33_cfg = {
> .dedicated_clocks = true,
> };
>
> +static const struct sun4i_usb_phy_cfg sun8i_h3_cfg = {
> + .num_phys = 4,
> + .disc_thresh = 3,
> + .type = sun8i_h3_phy,
> + .dedicated_clocks = true,
> +};
> +
> static const struct of_device_id sun4i_usb_phy_of_match[] = {
> { .compatible = "allwinner,sun4i-a10-usb-phy", .data = &sun4i_a10_cfg },
> { .compatible = "allwinner,sun5i-a13-usb-phy", .data = &sun5i_a13_cfg },
> @@ -562,6 +588,7 @@ static const struct of_device_id sun4i_usb_phy_of_match[] = {
> { .compatible = "allwinner,sun7i-a20-usb-phy", .data = &sun7i_a20_cfg },
> { .compatible = "allwinner,sun8i-a23-usb-phy", .data = &sun8i_a23_cfg },
> { .compatible = "allwinner,sun8i-a33-usb-phy", .data = &sun8i_a33_cfg },
> + { .compatible = "allwinner,sun8i-h3-usb-phy", .data = &sun8i_h3_cfg },
> { },
> };
> MODULE_DEVICE_TABLE(of, sun4i_usb_phy_of_match);
> @@ -595,6 +622,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
> mutex_init(&data->mutex);
> INIT_DELAYED_WORK(&data->detect, sun4i_usb_phy0_id_vbus_det_scan);
> dev_set_drvdata(dev, data);
> + data->dev = dev;
> data->cfg = match->data;
>
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "phy_ctrl");
> --
> 2.5.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v3 2/2] phy-sun4i-usb: Add support for the host usb-phys found on the H3 SoC
@ 2015-11-25 22:11 ` Rob Herring
0 siblings, 0 replies; 22+ messages in thread
From: Rob Herring @ 2015-11-25 22:11 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Nov 25, 2015 at 05:50:02PM +0100, Hans de Goede wrote:
> From: Reinder de Haan <patchesrdh@mveas.com>
>
> Note this commit only adds support for phys 1-3, phy 0, the otg phy, is
> not yet (fully) supported after this commit.
Shouldn't the OTG phy have a different compatible string?
>
> Signed-off-by: Reinder de Haan <patchesrdh@mveas.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -Change break; after dev_err() to return, as intended, fixing a compiler
> warning (the dev_err case should never be reached)
> Changes in v3:
> -Use of_match_node to get model specific config data
> ---
> .../devicetree/bindings/phy/sun4i-usb-phy.txt | 1 +
> drivers/phy/phy-sun4i-usb.c | 46 +++++++++++++++++-----
> 2 files changed, 38 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
> index 0cebf74..95736d7 100644
> --- a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
> +++ b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
> @@ -9,6 +9,7 @@ Required properties:
> * allwinner,sun7i-a20-usb-phy
> * allwinner,sun8i-a23-usb-phy
> * allwinner,sun8i-a33-usb-phy
> + * allwinner,sun8i-h3-usb-phy
> - reg : a list of offset + length pairs
> - reg-names :
> * "phy_ctrl"
> diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
> index 1d8f85d..2aed482 100644
> --- a/drivers/phy/phy-sun4i-usb.c
> +++ b/drivers/phy/phy-sun4i-usb.c
> @@ -46,6 +46,9 @@
> #define REG_PHYBIST 0x08
> #define REG_PHYTUNE 0x0c
> #define REG_PHYCTL_A33 0x10
> +#define REG_PHY_UNK_H3 0x20
> +
> +#define REG_PMU_UNK_H3 0x10
>
> #define PHYCTL_DATA BIT(7)
>
> @@ -79,7 +82,7 @@
> #define PHY_DISCON_TH_SEL 0x2a
> #define PHY_SQUELCH_DETECT 0x3c
>
> -#define MAX_PHYS 3
> +#define MAX_PHYS 4
>
> /*
> * Note do not raise the debounce time, we must report Vusb high within 100ms
> @@ -91,6 +94,7 @@
> enum sun4i_usb_phy_type {
> sun4i_a10_phy,
> sun8i_a33_phy,
> + sun8i_h3_phy,
> };
>
> struct sun4i_usb_phy_cfg {
> @@ -101,6 +105,7 @@ struct sun4i_usb_phy_cfg {
> };
>
> struct sun4i_usb_phy_data {
> + struct device *dev;
> void __iomem *base;
> const struct sun4i_usb_phy_cfg *cfg;
> struct mutex mutex;
> @@ -183,6 +188,9 @@ static void sun4i_usb_phy_write(struct sun4i_usb_phy *phy, u32 addr, u32 data,
> /* A33 needs us to set phyctl to 0 explicitly */
> writel(0, phyctl);
> break;
> + case sun8i_h3_phy:
> + dev_err(phy_data->dev, "H3 usb_phy_write is not supported\n");
> + return;
> }
>
> for (i = 0; i < len; i++) {
> @@ -243,6 +251,7 @@ static int sun4i_usb_phy_init(struct phy *_phy)
> struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
> struct sun4i_usb_phy_data *data = to_sun4i_usb_phy_data(phy);
> int ret;
> + u32 val;
>
> ret = clk_prepare_enable(phy->clk);
> if (ret)
> @@ -254,16 +263,26 @@ static int sun4i_usb_phy_init(struct phy *_phy)
> return ret;
> }
>
> - /* Enable USB 45 Ohm resistor calibration */
> - if (phy->index == 0)
> - sun4i_usb_phy_write(phy, PHY_RES45_CAL_EN, 0x01, 1);
> + if (data->cfg->type == sun8i_h3_phy) {
> + if (phy->index == 0) {
> + val = readl(data->base + REG_PHY_UNK_H3);
> + writel(val & ~1, data->base + REG_PHY_UNK_H3);
> + }
>
> - /* Adjust PHY's magnitude and rate */
> - sun4i_usb_phy_write(phy, PHY_TX_AMPLITUDE_TUNE, 0x14, 5);
> + val = readl(phy->pmu + REG_PMU_UNK_H3);
> + writel(val & ~2, phy->pmu + REG_PMU_UNK_H3);
> + } else {
> + /* Enable USB 45 Ohm resistor calibration */
> + if (phy->index == 0)
> + sun4i_usb_phy_write(phy, PHY_RES45_CAL_EN, 0x01, 1);
>
> - /* Disconnect threshold adjustment */
> - sun4i_usb_phy_write(phy, PHY_DISCON_TH_SEL,
> - data->cfg->disc_thresh, 2);
> + /* Adjust PHY's magnitude and rate */
> + sun4i_usb_phy_write(phy, PHY_TX_AMPLITUDE_TUNE, 0x14, 5);
> +
> + /* Disconnect threshold adjustment */
> + sun4i_usb_phy_write(phy, PHY_DISCON_TH_SEL,
> + data->cfg->disc_thresh, 2);
> + }
>
> sun4i_usb_phy_passby(phy, 1);
>
> @@ -555,6 +574,13 @@ static const struct sun4i_usb_phy_cfg sun8i_a33_cfg = {
> .dedicated_clocks = true,
> };
>
> +static const struct sun4i_usb_phy_cfg sun8i_h3_cfg = {
> + .num_phys = 4,
> + .disc_thresh = 3,
> + .type = sun8i_h3_phy,
> + .dedicated_clocks = true,
> +};
> +
> static const struct of_device_id sun4i_usb_phy_of_match[] = {
> { .compatible = "allwinner,sun4i-a10-usb-phy", .data = &sun4i_a10_cfg },
> { .compatible = "allwinner,sun5i-a13-usb-phy", .data = &sun5i_a13_cfg },
> @@ -562,6 +588,7 @@ static const struct of_device_id sun4i_usb_phy_of_match[] = {
> { .compatible = "allwinner,sun7i-a20-usb-phy", .data = &sun7i_a20_cfg },
> { .compatible = "allwinner,sun8i-a23-usb-phy", .data = &sun8i_a23_cfg },
> { .compatible = "allwinner,sun8i-a33-usb-phy", .data = &sun8i_a33_cfg },
> + { .compatible = "allwinner,sun8i-h3-usb-phy", .data = &sun8i_h3_cfg },
> { },
> };
> MODULE_DEVICE_TABLE(of, sun4i_usb_phy_of_match);
> @@ -595,6 +622,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
> mutex_init(&data->mutex);
> INIT_DELAYED_WORK(&data->detect, sun4i_usb_phy0_id_vbus_det_scan);
> dev_set_drvdata(dev, data);
> + data->dev = dev;
> data->cfg = match->data;
>
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "phy_ctrl");
> --
> 2.5.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 1/2] phy-sun4i-usb: Use of_match_node to get model specific config data
2015-11-25 16:50 ` Hans de Goede
@ 2015-11-26 4:22 ` Chen-Yu Tsai
-1 siblings, 0 replies; 22+ messages in thread
From: Chen-Yu Tsai @ 2015-11-26 4:22 UTC (permalink / raw)
To: Hans de Goede
Cc: Kishon Vijay Abraham I, Reinder E.N. de Haan, Maxime Ripard,
Chen-Yu Tsai, linux-usb, linux-arm-kernel, devicetree,
linux-sunxi
On Thu, Nov 26, 2015 at 12:50 AM, Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> Use of_match_node instead of calling of_device_is_compatible a ton of
> times to get model specific config data.
>
> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> Changes in v3:
> -New patch in v3 of this patch-set
> ---
> drivers/phy/phy-sun4i-usb.c | 130 +++++++++++++++++++++++++++++---------------
> 1 file changed, 85 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
> index b12964b..1d8f85d 100644
> --- a/drivers/phy/phy-sun4i-usb.c
> +++ b/drivers/phy/phy-sun4i-usb.c
> @@ -88,12 +88,22 @@
> #define DEBOUNCE_TIME msecs_to_jiffies(50)
> #define POLL_TIME msecs_to_jiffies(250)
>
> +enum sun4i_usb_phy_type {
> + sun4i_a10_phy,
> + sun8i_a33_phy,
> +};
> +
> +struct sun4i_usb_phy_cfg {
> + int num_phys;
> + u32 disc_thresh;
> + enum sun4i_usb_phy_type type;
> + bool dedicated_clocks;
> +};
> +
> struct sun4i_usb_phy_data {
> void __iomem *base;
> + const struct sun4i_usb_phy_cfg *cfg;
> struct mutex mutex;
> - int num_phys;
> - u32 disc_thresh;
> - bool has_a33_phyctl;
> struct sun4i_usb_phy {
> struct phy *phy;
> void __iomem *pmu;
> @@ -164,12 +174,15 @@ static void sun4i_usb_phy_write(struct sun4i_usb_phy *phy, u32 addr, u32 data,
>
> mutex_lock(&phy_data->mutex);
>
> - if (phy_data->has_a33_phyctl) {
> + switch (phy_data->cfg->type) {
> + case sun4i_a10_phy:
> + phyctl = phy_data->base + REG_PHYCTL_A10;
Any reason why this offset isn't incorporated into phy_data?
> + break;
> + case sun8i_a33_phy:
> phyctl = phy_data->base + REG_PHYCTL_A33;
> /* A33 needs us to set phyctl to 0 explicitly */
> writel(0, phyctl);
> - } else {
> - phyctl = phy_data->base + REG_PHYCTL_A10;
> + break;
> }
>
> for (i = 0; i < len; i++) {
> @@ -249,7 +262,8 @@ static int sun4i_usb_phy_init(struct phy *_phy)
> sun4i_usb_phy_write(phy, PHY_TX_AMPLITUDE_TUNE, 0x14, 5);
>
> /* Disconnect threshold adjustment */
> - sun4i_usb_phy_write(phy, PHY_DISCON_TH_SEL, data->disc_thresh, 2);
> + sun4i_usb_phy_write(phy, PHY_DISCON_TH_SEL,
> + data->cfg->disc_thresh, 2);
>
> sun4i_usb_phy_passby(phy, 1);
>
> @@ -476,7 +490,7 @@ static struct phy *sun4i_usb_phy_xlate(struct device *dev,
> {
> struct sun4i_usb_phy_data *data = dev_get_drvdata(dev);
>
> - if (args->args[0] >= data->num_phys)
> + if (args->args[0] >= data->cfg->num_phys)
> return ERR_PTR(-ENODEV);
>
> return data->phys[args->args[0]].phy;
> @@ -499,6 +513,59 @@ static int sun4i_usb_phy_remove(struct platform_device *pdev)
> return 0;
> }
>
> +static const struct sun4i_usb_phy_cfg sun4i_a10_cfg = {
> + .num_phys = 3,
> + .disc_thresh = 3,
> + .type = sun4i_a10_phy,
> + .dedicated_clocks = false,
> +};
> +
> +static const struct sun4i_usb_phy_cfg sun5i_a13_cfg = {
> + .num_phys = 2,
> + .disc_thresh = 2,
> + .type = sun4i_a10_phy,
> + .dedicated_clocks = false,
> +};
> +
> +static const struct sun4i_usb_phy_cfg sun6i_a31_cfg = {
> + .num_phys = 3,
> + .disc_thresh = 3,
> + .type = sun4i_a10_phy,
> + .dedicated_clocks = true,
> +};
> +
> +static const struct sun4i_usb_phy_cfg sun7i_a20_cfg = {
> + .num_phys = 3,
> + .disc_thresh = 2,
> + .type = sun4i_a10_phy,
> + .dedicated_clocks = false,
> +};
> +
> +static const struct sun4i_usb_phy_cfg sun8i_a23_cfg = {
> + .num_phys = 2,
> + .disc_thresh = 3,
> + .type = sun4i_a10_phy,
> + .dedicated_clocks = true,
> +};
> +
> +static const struct sun4i_usb_phy_cfg sun8i_a33_cfg = {
> + .num_phys = 2,
> + .disc_thresh = 3,
> + .type = sun8i_a33_phy,
> + .dedicated_clocks = true,
> +};
> +
> +static const struct of_device_id sun4i_usb_phy_of_match[] = {
> + { .compatible = "allwinner,sun4i-a10-usb-phy", .data = &sun4i_a10_cfg },
> + { .compatible = "allwinner,sun5i-a13-usb-phy", .data = &sun5i_a13_cfg },
> + { .compatible = "allwinner,sun6i-a31-usb-phy", .data = &sun6i_a31_cfg },
> + { .compatible = "allwinner,sun7i-a20-usb-phy", .data = &sun7i_a20_cfg },
> + { .compatible = "allwinner,sun8i-a23-usb-phy", .data = &sun8i_a23_cfg },
> + { .compatible = "allwinner,sun8i-a33-usb-phy", .data = &sun8i_a33_cfg },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, sun4i_usb_phy_of_match);
> +
> static const unsigned int sun4i_usb_phy0_cable[] = {
> EXTCON_USB,
> EXTCON_USB_HOST,
> @@ -511,10 +578,16 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
> struct device *dev = &pdev->dev;
> struct device_node *np = dev->of_node;
> struct phy_provider *phy_provider;
> - bool dedicated_clocks;
> + const struct of_device_id *match;
> struct resource *res;
> int i, ret;
>
> + match = of_match_node(sun4i_usb_phy_of_match, dev->of_node);
You can use of_device_get_match_data() for slightly less code. This
will also let you keep the of_device_id table where it was, at the
bottom.
> + if (!match) {
I'm working on something similar in the axp20x driver. Is there any
case of_match_node or of_device_get_match_data can fail?
Regards
ChenYu
> + dev_err(dev, "of_match_node() failed\n");
> + return -EINVAL;
> + }
> +
> data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> if (!data)
> return -ENOMEM;
> @@ -522,29 +595,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
> mutex_init(&data->mutex);
> INIT_DELAYED_WORK(&data->detect, sun4i_usb_phy0_id_vbus_det_scan);
> dev_set_drvdata(dev, data);
> -
> - if (of_device_is_compatible(np, "allwinner,sun5i-a13-usb-phy") ||
> - of_device_is_compatible(np, "allwinner,sun8i-a23-usb-phy") ||
> - of_device_is_compatible(np, "allwinner,sun8i-a33-usb-phy"))
> - data->num_phys = 2;
> - else
> - data->num_phys = 3;
> -
> - if (of_device_is_compatible(np, "allwinner,sun5i-a13-usb-phy") ||
> - of_device_is_compatible(np, "allwinner,sun7i-a20-usb-phy"))
> - data->disc_thresh = 2;
> - else
> - data->disc_thresh = 3;
> -
> - if (of_device_is_compatible(np, "allwinner,sun6i-a31-usb-phy") ||
> - of_device_is_compatible(np, "allwinner,sun8i-a23-usb-phy") ||
> - of_device_is_compatible(np, "allwinner,sun8i-a33-usb-phy"))
> - dedicated_clocks = true;
> - else
> - dedicated_clocks = false;
> -
> - if (of_device_is_compatible(np, "allwinner,sun8i-a33-usb-phy"))
> - data->has_a33_phyctl = true;
> + data->cfg = match->data;
>
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "phy_ctrl");
> data->base = devm_ioremap_resource(dev, res);
> @@ -590,7 +641,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
> }
> }
>
> - for (i = 0; i < data->num_phys; i++) {
> + for (i = 0; i < data->cfg->num_phys; i++) {
> struct sun4i_usb_phy *phy = data->phys + i;
> char name[16];
>
> @@ -602,7 +653,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
> phy->vbus = NULL;
> }
>
> - if (dedicated_clocks)
> + if (data->cfg->dedicated_clocks)
> snprintf(name, sizeof(name), "usb%d_phy", i);
> else
> strlcpy(name, "usb_phy", sizeof(name));
> @@ -689,17 +740,6 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
> return 0;
> }
>
> -static const struct of_device_id sun4i_usb_phy_of_match[] = {
> - { .compatible = "allwinner,sun4i-a10-usb-phy" },
> - { .compatible = "allwinner,sun5i-a13-usb-phy" },
> - { .compatible = "allwinner,sun6i-a31-usb-phy" },
> - { .compatible = "allwinner,sun7i-a20-usb-phy" },
> - { .compatible = "allwinner,sun8i-a23-usb-phy" },
> - { .compatible = "allwinner,sun8i-a33-usb-phy" },
> - { },
> -};
> -MODULE_DEVICE_TABLE(of, sun4i_usb_phy_of_match);
> -
> static struct platform_driver sun4i_usb_phy_driver = {
> .probe = sun4i_usb_phy_probe,
> .remove = sun4i_usb_phy_remove,
> --
> 2.5.0
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v3 1/2] phy-sun4i-usb: Use of_match_node to get model specific config data
@ 2015-11-26 4:22 ` Chen-Yu Tsai
0 siblings, 0 replies; 22+ messages in thread
From: Chen-Yu Tsai @ 2015-11-26 4:22 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Nov 26, 2015 at 12:50 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> Use of_match_node instead of calling of_device_is_compatible a ton of
> times to get model specific config data.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v3:
> -New patch in v3 of this patch-set
> ---
> drivers/phy/phy-sun4i-usb.c | 130 +++++++++++++++++++++++++++++---------------
> 1 file changed, 85 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
> index b12964b..1d8f85d 100644
> --- a/drivers/phy/phy-sun4i-usb.c
> +++ b/drivers/phy/phy-sun4i-usb.c
> @@ -88,12 +88,22 @@
> #define DEBOUNCE_TIME msecs_to_jiffies(50)
> #define POLL_TIME msecs_to_jiffies(250)
>
> +enum sun4i_usb_phy_type {
> + sun4i_a10_phy,
> + sun8i_a33_phy,
> +};
> +
> +struct sun4i_usb_phy_cfg {
> + int num_phys;
> + u32 disc_thresh;
> + enum sun4i_usb_phy_type type;
> + bool dedicated_clocks;
> +};
> +
> struct sun4i_usb_phy_data {
> void __iomem *base;
> + const struct sun4i_usb_phy_cfg *cfg;
> struct mutex mutex;
> - int num_phys;
> - u32 disc_thresh;
> - bool has_a33_phyctl;
> struct sun4i_usb_phy {
> struct phy *phy;
> void __iomem *pmu;
> @@ -164,12 +174,15 @@ static void sun4i_usb_phy_write(struct sun4i_usb_phy *phy, u32 addr, u32 data,
>
> mutex_lock(&phy_data->mutex);
>
> - if (phy_data->has_a33_phyctl) {
> + switch (phy_data->cfg->type) {
> + case sun4i_a10_phy:
> + phyctl = phy_data->base + REG_PHYCTL_A10;
Any reason why this offset isn't incorporated into phy_data?
> + break;
> + case sun8i_a33_phy:
> phyctl = phy_data->base + REG_PHYCTL_A33;
> /* A33 needs us to set phyctl to 0 explicitly */
> writel(0, phyctl);
> - } else {
> - phyctl = phy_data->base + REG_PHYCTL_A10;
> + break;
> }
>
> for (i = 0; i < len; i++) {
> @@ -249,7 +262,8 @@ static int sun4i_usb_phy_init(struct phy *_phy)
> sun4i_usb_phy_write(phy, PHY_TX_AMPLITUDE_TUNE, 0x14, 5);
>
> /* Disconnect threshold adjustment */
> - sun4i_usb_phy_write(phy, PHY_DISCON_TH_SEL, data->disc_thresh, 2);
> + sun4i_usb_phy_write(phy, PHY_DISCON_TH_SEL,
> + data->cfg->disc_thresh, 2);
>
> sun4i_usb_phy_passby(phy, 1);
>
> @@ -476,7 +490,7 @@ static struct phy *sun4i_usb_phy_xlate(struct device *dev,
> {
> struct sun4i_usb_phy_data *data = dev_get_drvdata(dev);
>
> - if (args->args[0] >= data->num_phys)
> + if (args->args[0] >= data->cfg->num_phys)
> return ERR_PTR(-ENODEV);
>
> return data->phys[args->args[0]].phy;
> @@ -499,6 +513,59 @@ static int sun4i_usb_phy_remove(struct platform_device *pdev)
> return 0;
> }
>
> +static const struct sun4i_usb_phy_cfg sun4i_a10_cfg = {
> + .num_phys = 3,
> + .disc_thresh = 3,
> + .type = sun4i_a10_phy,
> + .dedicated_clocks = false,
> +};
> +
> +static const struct sun4i_usb_phy_cfg sun5i_a13_cfg = {
> + .num_phys = 2,
> + .disc_thresh = 2,
> + .type = sun4i_a10_phy,
> + .dedicated_clocks = false,
> +};
> +
> +static const struct sun4i_usb_phy_cfg sun6i_a31_cfg = {
> + .num_phys = 3,
> + .disc_thresh = 3,
> + .type = sun4i_a10_phy,
> + .dedicated_clocks = true,
> +};
> +
> +static const struct sun4i_usb_phy_cfg sun7i_a20_cfg = {
> + .num_phys = 3,
> + .disc_thresh = 2,
> + .type = sun4i_a10_phy,
> + .dedicated_clocks = false,
> +};
> +
> +static const struct sun4i_usb_phy_cfg sun8i_a23_cfg = {
> + .num_phys = 2,
> + .disc_thresh = 3,
> + .type = sun4i_a10_phy,
> + .dedicated_clocks = true,
> +};
> +
> +static const struct sun4i_usb_phy_cfg sun8i_a33_cfg = {
> + .num_phys = 2,
> + .disc_thresh = 3,
> + .type = sun8i_a33_phy,
> + .dedicated_clocks = true,
> +};
> +
> +static const struct of_device_id sun4i_usb_phy_of_match[] = {
> + { .compatible = "allwinner,sun4i-a10-usb-phy", .data = &sun4i_a10_cfg },
> + { .compatible = "allwinner,sun5i-a13-usb-phy", .data = &sun5i_a13_cfg },
> + { .compatible = "allwinner,sun6i-a31-usb-phy", .data = &sun6i_a31_cfg },
> + { .compatible = "allwinner,sun7i-a20-usb-phy", .data = &sun7i_a20_cfg },
> + { .compatible = "allwinner,sun8i-a23-usb-phy", .data = &sun8i_a23_cfg },
> + { .compatible = "allwinner,sun8i-a33-usb-phy", .data = &sun8i_a33_cfg },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, sun4i_usb_phy_of_match);
> +
> static const unsigned int sun4i_usb_phy0_cable[] = {
> EXTCON_USB,
> EXTCON_USB_HOST,
> @@ -511,10 +578,16 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
> struct device *dev = &pdev->dev;
> struct device_node *np = dev->of_node;
> struct phy_provider *phy_provider;
> - bool dedicated_clocks;
> + const struct of_device_id *match;
> struct resource *res;
> int i, ret;
>
> + match = of_match_node(sun4i_usb_phy_of_match, dev->of_node);
You can use of_device_get_match_data() for slightly less code. This
will also let you keep the of_device_id table where it was, at the
bottom.
> + if (!match) {
I'm working on something similar in the axp20x driver. Is there any
case of_match_node or of_device_get_match_data can fail?
Regards
ChenYu
> + dev_err(dev, "of_match_node() failed\n");
> + return -EINVAL;
> + }
> +
> data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> if (!data)
> return -ENOMEM;
> @@ -522,29 +595,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
> mutex_init(&data->mutex);
> INIT_DELAYED_WORK(&data->detect, sun4i_usb_phy0_id_vbus_det_scan);
> dev_set_drvdata(dev, data);
> -
> - if (of_device_is_compatible(np, "allwinner,sun5i-a13-usb-phy") ||
> - of_device_is_compatible(np, "allwinner,sun8i-a23-usb-phy") ||
> - of_device_is_compatible(np, "allwinner,sun8i-a33-usb-phy"))
> - data->num_phys = 2;
> - else
> - data->num_phys = 3;
> -
> - if (of_device_is_compatible(np, "allwinner,sun5i-a13-usb-phy") ||
> - of_device_is_compatible(np, "allwinner,sun7i-a20-usb-phy"))
> - data->disc_thresh = 2;
> - else
> - data->disc_thresh = 3;
> -
> - if (of_device_is_compatible(np, "allwinner,sun6i-a31-usb-phy") ||
> - of_device_is_compatible(np, "allwinner,sun8i-a23-usb-phy") ||
> - of_device_is_compatible(np, "allwinner,sun8i-a33-usb-phy"))
> - dedicated_clocks = true;
> - else
> - dedicated_clocks = false;
> -
> - if (of_device_is_compatible(np, "allwinner,sun8i-a33-usb-phy"))
> - data->has_a33_phyctl = true;
> + data->cfg = match->data;
>
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "phy_ctrl");
> data->base = devm_ioremap_resource(dev, res);
> @@ -590,7 +641,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
> }
> }
>
> - for (i = 0; i < data->num_phys; i++) {
> + for (i = 0; i < data->cfg->num_phys; i++) {
> struct sun4i_usb_phy *phy = data->phys + i;
> char name[16];
>
> @@ -602,7 +653,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
> phy->vbus = NULL;
> }
>
> - if (dedicated_clocks)
> + if (data->cfg->dedicated_clocks)
> snprintf(name, sizeof(name), "usb%d_phy", i);
> else
> strlcpy(name, "usb_phy", sizeof(name));
> @@ -689,17 +740,6 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
> return 0;
> }
>
> -static const struct of_device_id sun4i_usb_phy_of_match[] = {
> - { .compatible = "allwinner,sun4i-a10-usb-phy" },
> - { .compatible = "allwinner,sun5i-a13-usb-phy" },
> - { .compatible = "allwinner,sun6i-a31-usb-phy" },
> - { .compatible = "allwinner,sun7i-a20-usb-phy" },
> - { .compatible = "allwinner,sun8i-a23-usb-phy" },
> - { .compatible = "allwinner,sun8i-a33-usb-phy" },
> - { },
> -};
> -MODULE_DEVICE_TABLE(of, sun4i_usb_phy_of_match);
> -
> static struct platform_driver sun4i_usb_phy_driver = {
> .probe = sun4i_usb_phy_probe,
> .remove = sun4i_usb_phy_remove,
> --
> 2.5.0
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: Re: [PATCH v3 1/2] phy-sun4i-usb: Use of_match_node to get model specific config data
2015-11-26 4:22 ` Chen-Yu Tsai
@ 2015-11-26 7:14 ` LABBE Corentin
-1 siblings, 0 replies; 22+ messages in thread
From: LABBE Corentin @ 2015-11-26 7:14 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Hans de Goede, Kishon Vijay Abraham I, Reinder E.N. de Haan,
Maxime Ripard, linux-usb, linux-arm-kernel, devicetree,
linux-sunxi
On Thu, Nov 26, 2015 at 12:22:59PM +0800, Chen-Yu Tsai wrote:
> On Thu, Nov 26, 2015 at 12:50 AM, Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > +
> > static const unsigned int sun4i_usb_phy0_cable[] = {
> > EXTCON_USB,
> > EXTCON_USB_HOST,
> > @@ -511,10 +578,16 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
> > struct device *dev = &pdev->dev;
> > struct device_node *np = dev->of_node;
> > struct phy_provider *phy_provider;
> > - bool dedicated_clocks;
> > + const struct of_device_id *match;
> > struct resource *res;
> > int i, ret;
> >
> > + match = of_match_node(sun4i_usb_phy_of_match, dev->of_node);
>
> You can use of_device_get_match_data() for slightly less code. This
> will also let you keep the of_device_id table where it was, at the
> bottom.
>
> > + if (!match) {
>
> I'm working on something similar in the axp20x driver. Is there any
> case of_match_node or of_device_get_match_data can fail?
>
>
Hello
I am working on some patch for that case and the conclusion was that case is possible.
See https://lkml.org/lkml/2015/11/12/97
So it is better to check it.
Regards
^ permalink raw reply [flat|nested] 22+ messages in thread
* [linux-sunxi] Re: [PATCH v3 1/2] phy-sun4i-usb: Use of_match_node to get model specific config data
@ 2015-11-26 7:14 ` LABBE Corentin
0 siblings, 0 replies; 22+ messages in thread
From: LABBE Corentin @ 2015-11-26 7:14 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Nov 26, 2015 at 12:22:59PM +0800, Chen-Yu Tsai wrote:
> On Thu, Nov 26, 2015 at 12:50 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> > +
> > static const unsigned int sun4i_usb_phy0_cable[] = {
> > EXTCON_USB,
> > EXTCON_USB_HOST,
> > @@ -511,10 +578,16 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
> > struct device *dev = &pdev->dev;
> > struct device_node *np = dev->of_node;
> > struct phy_provider *phy_provider;
> > - bool dedicated_clocks;
> > + const struct of_device_id *match;
> > struct resource *res;
> > int i, ret;
> >
> > + match = of_match_node(sun4i_usb_phy_of_match, dev->of_node);
>
> You can use of_device_get_match_data() for slightly less code. This
> will also let you keep the of_device_id table where it was, at the
> bottom.
>
> > + if (!match) {
>
> I'm working on something similar in the axp20x driver. Is there any
> case of_match_node or of_device_get_match_data can fail?
>
>
Hello
I am working on some patch for that case and the conclusion was that case is possible.
See https://lkml.org/lkml/2015/11/12/97
So it is better to check it.
Regards
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: Re: [PATCH v3 2/2] phy-sun4i-usb: Add support for the host usb-phys found on the H3 SoC
2015-11-25 22:11 ` Rob Herring
@ 2015-11-26 12:03 ` Hans de Goede
-1 siblings, 0 replies; 22+ messages in thread
From: Hans de Goede @ 2015-11-26 12:03 UTC (permalink / raw)
To: Rob Herring
Cc: Kishon Vijay Abraham I, Reinder E.N. de Haan, Maxime Ripard,
Chen-Yu Tsai, linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Reinder de Haan
Hi,
On 25-11-15 23:11, Rob Herring wrote:
> On Wed, Nov 25, 2015 at 05:50:02PM +0100, Hans de Goede wrote:
>> From: Reinder de Haan <patchesrdh-I1/eAgTnXDYAvxtiuMwx3w@public.gmane.org>
>>
>> Note this commit only adds support for phys 1-3, phy 0, the otg phy, is
>> not yet (fully) supported after this commit.
>
> Shouldn't the OTG phy have a different compatible string?
All 4 phy's are part of one hardware block and they
share some registers, so for sunxi we've always had
one devicetree node describing all usb-phys which
then instantiates multiple phys. E.G. for the A10 we
use:
usbphy: phy@01c13400 {
#phy-cells = <1>;
compatible = "allwinner,sun4i-a10-usb-phy";
...
};
usb_otg: usb@01c13000 {
...
phys = <&usbphy 0>;
...
};
ehci0: usb@01c14000 {
...
phys = <&usbphy 1>;
...
};
...
On the H3 some aspects of the phys have changed, but they still part
of a single hardware block and sharing registers, so we are modelling
them the same way in devicetree and thus they share a single compatible.
Regards,
Hans
>
>>
>> Signed-off-by: Reinder de Haan <patchesrdh-I1/eAgTnXDYAvxtiuMwx3w@public.gmane.org>
>> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> ---
>> Changes in v2:
>> -Change break; after dev_err() to return, as intended, fixing a compiler
>> warning (the dev_err case should never be reached)
>> Changes in v3:
>> -Use of_match_node to get model specific config data
>> ---
>> .../devicetree/bindings/phy/sun4i-usb-phy.txt | 1 +
>> drivers/phy/phy-sun4i-usb.c | 46 +++++++++++++++++-----
>> 2 files changed, 38 insertions(+), 9 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
>> index 0cebf74..95736d7 100644
>> --- a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
>> +++ b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
>> @@ -9,6 +9,7 @@ Required properties:
>> * allwinner,sun7i-a20-usb-phy
>> * allwinner,sun8i-a23-usb-phy
>> * allwinner,sun8i-a33-usb-phy
>> + * allwinner,sun8i-h3-usb-phy
>> - reg : a list of offset + length pairs
>> - reg-names :
>> * "phy_ctrl"
>> diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
>> index 1d8f85d..2aed482 100644
>> --- a/drivers/phy/phy-sun4i-usb.c
>> +++ b/drivers/phy/phy-sun4i-usb.c
>> @@ -46,6 +46,9 @@
>> #define REG_PHYBIST 0x08
>> #define REG_PHYTUNE 0x0c
>> #define REG_PHYCTL_A33 0x10
>> +#define REG_PHY_UNK_H3 0x20
>> +
>> +#define REG_PMU_UNK_H3 0x10
>>
>> #define PHYCTL_DATA BIT(7)
>>
>> @@ -79,7 +82,7 @@
>> #define PHY_DISCON_TH_SEL 0x2a
>> #define PHY_SQUELCH_DETECT 0x3c
>>
>> -#define MAX_PHYS 3
>> +#define MAX_PHYS 4
>>
>> /*
>> * Note do not raise the debounce time, we must report Vusb high within 100ms
>> @@ -91,6 +94,7 @@
>> enum sun4i_usb_phy_type {
>> sun4i_a10_phy,
>> sun8i_a33_phy,
>> + sun8i_h3_phy,
>> };
>>
>> struct sun4i_usb_phy_cfg {
>> @@ -101,6 +105,7 @@ struct sun4i_usb_phy_cfg {
>> };
>>
>> struct sun4i_usb_phy_data {
>> + struct device *dev;
>> void __iomem *base;
>> const struct sun4i_usb_phy_cfg *cfg;
>> struct mutex mutex;
>> @@ -183,6 +188,9 @@ static void sun4i_usb_phy_write(struct sun4i_usb_phy *phy, u32 addr, u32 data,
>> /* A33 needs us to set phyctl to 0 explicitly */
>> writel(0, phyctl);
>> break;
>> + case sun8i_h3_phy:
>> + dev_err(phy_data->dev, "H3 usb_phy_write is not supported\n");
>> + return;
>> }
>>
>> for (i = 0; i < len; i++) {
>> @@ -243,6 +251,7 @@ static int sun4i_usb_phy_init(struct phy *_phy)
>> struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
>> struct sun4i_usb_phy_data *data = to_sun4i_usb_phy_data(phy);
>> int ret;
>> + u32 val;
>>
>> ret = clk_prepare_enable(phy->clk);
>> if (ret)
>> @@ -254,16 +263,26 @@ static int sun4i_usb_phy_init(struct phy *_phy)
>> return ret;
>> }
>>
>> - /* Enable USB 45 Ohm resistor calibration */
>> - if (phy->index == 0)
>> - sun4i_usb_phy_write(phy, PHY_RES45_CAL_EN, 0x01, 1);
>> + if (data->cfg->type == sun8i_h3_phy) {
>> + if (phy->index == 0) {
>> + val = readl(data->base + REG_PHY_UNK_H3);
>> + writel(val & ~1, data->base + REG_PHY_UNK_H3);
>> + }
>>
>> - /* Adjust PHY's magnitude and rate */
>> - sun4i_usb_phy_write(phy, PHY_TX_AMPLITUDE_TUNE, 0x14, 5);
>> + val = readl(phy->pmu + REG_PMU_UNK_H3);
>> + writel(val & ~2, phy->pmu + REG_PMU_UNK_H3);
>> + } else {
>> + /* Enable USB 45 Ohm resistor calibration */
>> + if (phy->index == 0)
>> + sun4i_usb_phy_write(phy, PHY_RES45_CAL_EN, 0x01, 1);
>>
>> - /* Disconnect threshold adjustment */
>> - sun4i_usb_phy_write(phy, PHY_DISCON_TH_SEL,
>> - data->cfg->disc_thresh, 2);
>> + /* Adjust PHY's magnitude and rate */
>> + sun4i_usb_phy_write(phy, PHY_TX_AMPLITUDE_TUNE, 0x14, 5);
>> +
>> + /* Disconnect threshold adjustment */
>> + sun4i_usb_phy_write(phy, PHY_DISCON_TH_SEL,
>> + data->cfg->disc_thresh, 2);
>> + }
>>
>> sun4i_usb_phy_passby(phy, 1);
>>
>> @@ -555,6 +574,13 @@ static const struct sun4i_usb_phy_cfg sun8i_a33_cfg = {
>> .dedicated_clocks = true,
>> };
>>
>> +static const struct sun4i_usb_phy_cfg sun8i_h3_cfg = {
>> + .num_phys = 4,
>> + .disc_thresh = 3,
>> + .type = sun8i_h3_phy,
>> + .dedicated_clocks = true,
>> +};
>> +
>> static const struct of_device_id sun4i_usb_phy_of_match[] = {
>> { .compatible = "allwinner,sun4i-a10-usb-phy", .data = &sun4i_a10_cfg },
>> { .compatible = "allwinner,sun5i-a13-usb-phy", .data = &sun5i_a13_cfg },
>> @@ -562,6 +588,7 @@ static const struct of_device_id sun4i_usb_phy_of_match[] = {
>> { .compatible = "allwinner,sun7i-a20-usb-phy", .data = &sun7i_a20_cfg },
>> { .compatible = "allwinner,sun8i-a23-usb-phy", .data = &sun8i_a23_cfg },
>> { .compatible = "allwinner,sun8i-a33-usb-phy", .data = &sun8i_a33_cfg },
>> + { .compatible = "allwinner,sun8i-h3-usb-phy", .data = &sun8i_h3_cfg },
>> { },
>> };
>> MODULE_DEVICE_TABLE(of, sun4i_usb_phy_of_match);
>> @@ -595,6 +622,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
>> mutex_init(&data->mutex);
>> INIT_DELAYED_WORK(&data->detect, sun4i_usb_phy0_id_vbus_det_scan);
>> dev_set_drvdata(dev, data);
>> + data->dev = dev;
>> data->cfg = match->data;
>>
>> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "phy_ctrl");
>> --
>> 2.5.0
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 22+ messages in thread
* [linux-sunxi] Re: [PATCH v3 2/2] phy-sun4i-usb: Add support for the host usb-phys found on the H3 SoC
@ 2015-11-26 12:03 ` Hans de Goede
0 siblings, 0 replies; 22+ messages in thread
From: Hans de Goede @ 2015-11-26 12:03 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On 25-11-15 23:11, Rob Herring wrote:
> On Wed, Nov 25, 2015 at 05:50:02PM +0100, Hans de Goede wrote:
>> From: Reinder de Haan <patchesrdh@mveas.com>
>>
>> Note this commit only adds support for phys 1-3, phy 0, the otg phy, is
>> not yet (fully) supported after this commit.
>
> Shouldn't the OTG phy have a different compatible string?
All 4 phy's are part of one hardware block and they
share some registers, so for sunxi we've always had
one devicetree node describing all usb-phys which
then instantiates multiple phys. E.G. for the A10 we
use:
usbphy: phy at 01c13400 {
#phy-cells = <1>;
compatible = "allwinner,sun4i-a10-usb-phy";
...
};
usb_otg: usb at 01c13000 {
...
phys = <&usbphy 0>;
...
};
ehci0: usb at 01c14000 {
...
phys = <&usbphy 1>;
...
};
...
On the H3 some aspects of the phys have changed, but they still part
of a single hardware block and sharing registers, so we are modelling
them the same way in devicetree and thus they share a single compatible.
Regards,
Hans
>
>>
>> Signed-off-by: Reinder de Haan <patchesrdh@mveas.com>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Changes in v2:
>> -Change break; after dev_err() to return, as intended, fixing a compiler
>> warning (the dev_err case should never be reached)
>> Changes in v3:
>> -Use of_match_node to get model specific config data
>> ---
>> .../devicetree/bindings/phy/sun4i-usb-phy.txt | 1 +
>> drivers/phy/phy-sun4i-usb.c | 46 +++++++++++++++++-----
>> 2 files changed, 38 insertions(+), 9 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
>> index 0cebf74..95736d7 100644
>> --- a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
>> +++ b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
>> @@ -9,6 +9,7 @@ Required properties:
>> * allwinner,sun7i-a20-usb-phy
>> * allwinner,sun8i-a23-usb-phy
>> * allwinner,sun8i-a33-usb-phy
>> + * allwinner,sun8i-h3-usb-phy
>> - reg : a list of offset + length pairs
>> - reg-names :
>> * "phy_ctrl"
>> diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
>> index 1d8f85d..2aed482 100644
>> --- a/drivers/phy/phy-sun4i-usb.c
>> +++ b/drivers/phy/phy-sun4i-usb.c
>> @@ -46,6 +46,9 @@
>> #define REG_PHYBIST 0x08
>> #define REG_PHYTUNE 0x0c
>> #define REG_PHYCTL_A33 0x10
>> +#define REG_PHY_UNK_H3 0x20
>> +
>> +#define REG_PMU_UNK_H3 0x10
>>
>> #define PHYCTL_DATA BIT(7)
>>
>> @@ -79,7 +82,7 @@
>> #define PHY_DISCON_TH_SEL 0x2a
>> #define PHY_SQUELCH_DETECT 0x3c
>>
>> -#define MAX_PHYS 3
>> +#define MAX_PHYS 4
>>
>> /*
>> * Note do not raise the debounce time, we must report Vusb high within 100ms
>> @@ -91,6 +94,7 @@
>> enum sun4i_usb_phy_type {
>> sun4i_a10_phy,
>> sun8i_a33_phy,
>> + sun8i_h3_phy,
>> };
>>
>> struct sun4i_usb_phy_cfg {
>> @@ -101,6 +105,7 @@ struct sun4i_usb_phy_cfg {
>> };
>>
>> struct sun4i_usb_phy_data {
>> + struct device *dev;
>> void __iomem *base;
>> const struct sun4i_usb_phy_cfg *cfg;
>> struct mutex mutex;
>> @@ -183,6 +188,9 @@ static void sun4i_usb_phy_write(struct sun4i_usb_phy *phy, u32 addr, u32 data,
>> /* A33 needs us to set phyctl to 0 explicitly */
>> writel(0, phyctl);
>> break;
>> + case sun8i_h3_phy:
>> + dev_err(phy_data->dev, "H3 usb_phy_write is not supported\n");
>> + return;
>> }
>>
>> for (i = 0; i < len; i++) {
>> @@ -243,6 +251,7 @@ static int sun4i_usb_phy_init(struct phy *_phy)
>> struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
>> struct sun4i_usb_phy_data *data = to_sun4i_usb_phy_data(phy);
>> int ret;
>> + u32 val;
>>
>> ret = clk_prepare_enable(phy->clk);
>> if (ret)
>> @@ -254,16 +263,26 @@ static int sun4i_usb_phy_init(struct phy *_phy)
>> return ret;
>> }
>>
>> - /* Enable USB 45 Ohm resistor calibration */
>> - if (phy->index == 0)
>> - sun4i_usb_phy_write(phy, PHY_RES45_CAL_EN, 0x01, 1);
>> + if (data->cfg->type == sun8i_h3_phy) {
>> + if (phy->index == 0) {
>> + val = readl(data->base + REG_PHY_UNK_H3);
>> + writel(val & ~1, data->base + REG_PHY_UNK_H3);
>> + }
>>
>> - /* Adjust PHY's magnitude and rate */
>> - sun4i_usb_phy_write(phy, PHY_TX_AMPLITUDE_TUNE, 0x14, 5);
>> + val = readl(phy->pmu + REG_PMU_UNK_H3);
>> + writel(val & ~2, phy->pmu + REG_PMU_UNK_H3);
>> + } else {
>> + /* Enable USB 45 Ohm resistor calibration */
>> + if (phy->index == 0)
>> + sun4i_usb_phy_write(phy, PHY_RES45_CAL_EN, 0x01, 1);
>>
>> - /* Disconnect threshold adjustment */
>> - sun4i_usb_phy_write(phy, PHY_DISCON_TH_SEL,
>> - data->cfg->disc_thresh, 2);
>> + /* Adjust PHY's magnitude and rate */
>> + sun4i_usb_phy_write(phy, PHY_TX_AMPLITUDE_TUNE, 0x14, 5);
>> +
>> + /* Disconnect threshold adjustment */
>> + sun4i_usb_phy_write(phy, PHY_DISCON_TH_SEL,
>> + data->cfg->disc_thresh, 2);
>> + }
>>
>> sun4i_usb_phy_passby(phy, 1);
>>
>> @@ -555,6 +574,13 @@ static const struct sun4i_usb_phy_cfg sun8i_a33_cfg = {
>> .dedicated_clocks = true,
>> };
>>
>> +static const struct sun4i_usb_phy_cfg sun8i_h3_cfg = {
>> + .num_phys = 4,
>> + .disc_thresh = 3,
>> + .type = sun8i_h3_phy,
>> + .dedicated_clocks = true,
>> +};
>> +
>> static const struct of_device_id sun4i_usb_phy_of_match[] = {
>> { .compatible = "allwinner,sun4i-a10-usb-phy", .data = &sun4i_a10_cfg },
>> { .compatible = "allwinner,sun5i-a13-usb-phy", .data = &sun5i_a13_cfg },
>> @@ -562,6 +588,7 @@ static const struct of_device_id sun4i_usb_phy_of_match[] = {
>> { .compatible = "allwinner,sun7i-a20-usb-phy", .data = &sun7i_a20_cfg },
>> { .compatible = "allwinner,sun8i-a23-usb-phy", .data = &sun8i_a23_cfg },
>> { .compatible = "allwinner,sun8i-a33-usb-phy", .data = &sun8i_a33_cfg },
>> + { .compatible = "allwinner,sun8i-h3-usb-phy", .data = &sun8i_h3_cfg },
>> { },
>> };
>> MODULE_DEVICE_TABLE(of, sun4i_usb_phy_of_match);
>> @@ -595,6 +622,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
>> mutex_init(&data->mutex);
>> INIT_DELAYED_WORK(&data->detect, sun4i_usb_phy0_id_vbus_det_scan);
>> dev_set_drvdata(dev, data);
>> + data->dev = dev;
>> data->cfg = match->data;
>>
>> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "phy_ctrl");
>> --
>> 2.5.0
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [linux-sunxi] Re: [PATCH v3 1/2] phy-sun4i-usb: Use of_match_node to get model specific config data
2015-11-26 4:22 ` Chen-Yu Tsai
@ 2015-11-26 12:11 ` Hans de Goede
-1 siblings, 0 replies; 22+ messages in thread
From: Hans de Goede @ 2015-11-26 12:11 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Kishon Vijay Abraham I, Reinder E.N. de Haan, Maxime Ripard,
linux-usb, linux-arm-kernel, devicetree, linux-sunxi
Hi,
On 26-11-15 05:22, Chen-Yu Tsai wrote:
> On Thu, Nov 26, 2015 at 12:50 AM, Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> Use of_match_node instead of calling of_device_is_compatible a ton of
>> times to get model specific config data.
>>
>> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> ---
>> Changes in v3:
>> -New patch in v3 of this patch-set
>> ---
>> drivers/phy/phy-sun4i-usb.c | 130 +++++++++++++++++++++++++++++---------------
>> 1 file changed, 85 insertions(+), 45 deletions(-)
>>
>> diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
>> index b12964b..1d8f85d 100644
>> --- a/drivers/phy/phy-sun4i-usb.c
>> +++ b/drivers/phy/phy-sun4i-usb.c
>> @@ -88,12 +88,22 @@
>> #define DEBOUNCE_TIME msecs_to_jiffies(50)
>> #define POLL_TIME msecs_to_jiffies(250)
>>
>> +enum sun4i_usb_phy_type {
>> + sun4i_a10_phy,
>> + sun8i_a33_phy,
>> +};
>> +
>> +struct sun4i_usb_phy_cfg {
>> + int num_phys;
>> + u32 disc_thresh;
>> + enum sun4i_usb_phy_type type;
>> + bool dedicated_clocks;
>> +};
>> +
>> struct sun4i_usb_phy_data {
>> void __iomem *base;
>> + const struct sun4i_usb_phy_cfg *cfg;
>> struct mutex mutex;
>> - int num_phys;
>> - u32 disc_thresh;
>> - bool has_a33_phyctl;
>> struct sun4i_usb_phy {
>> struct phy *phy;
>> void __iomem *pmu;
>> @@ -164,12 +174,15 @@ static void sun4i_usb_phy_write(struct sun4i_usb_phy *phy, u32 addr, u32 data,
>>
>> mutex_lock(&phy_data->mutex);
>>
>> - if (phy_data->has_a33_phyctl) {
>> + switch (phy_data->cfg->type) {
>> + case sun4i_a10_phy:
>> + phyctl = phy_data->base + REG_PHYCTL_A10;
>
> Any reason why this offset isn't incorporated into phy_data?
You mean in phy_data->cfg I assume, the difference needed for
the "sun4i_usb_phy_write" functionality are not just the phyctl
register offset...
>
>> + break;
>> + case sun8i_a33_phy:
>> phyctl = phy_data->base + REG_PHYCTL_A33;
>> /* A33 needs us to set phyctl to 0 explicitly */
>> writel(0, phyctl);
e.g. the A33 needs this extra write, and on the H3 we need to do
similar bitbanging, but slightly different, see:
https://github.com/allwinner-zh/linux-3.4-sunxi/blob/master/drivers/usb/host/sunxi_hci.c#L899
Notice how it uses different addr and write register addresses
their through the usb_phy_csr_add and usb_phy_csr_write helper
functions as well as directly poking offset 0x20.
My current H3 usb-phy support patches do not deal with this,
as the h3 sdk code posted by the orange-pi people never calls
this function, so we do not need to worry about implementing
sun4i_usb_phy_write for H3, but if we ever want to tweak
lowlevel phy settings, and/or for the A83 we likely need to
implement a whole different sun4i_usb_phy_write code-path for
the H3/A83.
So it seems better to just have phy_data->cfg->type and deal
with any SoC specific differences on that, as there are more
differences to deal with then just the phyctl register offset.
Regards,
Hans
>> - } else {
>> - phyctl = phy_data->base + REG_PHYCTL_A10;
>> + break;
>> }
>>
>> for (i = 0; i < len; i++) {
>> @@ -249,7 +262,8 @@ static int sun4i_usb_phy_init(struct phy *_phy)
>> sun4i_usb_phy_write(phy, PHY_TX_AMPLITUDE_TUNE, 0x14, 5);
>>
>> /* Disconnect threshold adjustment */
>> - sun4i_usb_phy_write(phy, PHY_DISCON_TH_SEL, data->disc_thresh, 2);
>> + sun4i_usb_phy_write(phy, PHY_DISCON_TH_SEL,
>> + data->cfg->disc_thresh, 2);
>>
>> sun4i_usb_phy_passby(phy, 1);
>>
>> @@ -476,7 +490,7 @@ static struct phy *sun4i_usb_phy_xlate(struct device *dev,
>> {
>> struct sun4i_usb_phy_data *data = dev_get_drvdata(dev);
>>
>> - if (args->args[0] >= data->num_phys)
>> + if (args->args[0] >= data->cfg->num_phys)
>> return ERR_PTR(-ENODEV);
>>
>> return data->phys[args->args[0]].phy;
>> @@ -499,6 +513,59 @@ static int sun4i_usb_phy_remove(struct platform_device *pdev)
>> return 0;
>> }
>>
>> +static const struct sun4i_usb_phy_cfg sun4i_a10_cfg = {
>> + .num_phys = 3,
>> + .disc_thresh = 3,
>> + .type = sun4i_a10_phy,
>> + .dedicated_clocks = false,
>> +};
>> +
>> +static const struct sun4i_usb_phy_cfg sun5i_a13_cfg = {
>> + .num_phys = 2,
>> + .disc_thresh = 2,
>> + .type = sun4i_a10_phy,
>> + .dedicated_clocks = false,
>> +};
>> +
>> +static const struct sun4i_usb_phy_cfg sun6i_a31_cfg = {
>> + .num_phys = 3,
>> + .disc_thresh = 3,
>> + .type = sun4i_a10_phy,
>> + .dedicated_clocks = true,
>> +};
>> +
>> +static const struct sun4i_usb_phy_cfg sun7i_a20_cfg = {
>> + .num_phys = 3,
>> + .disc_thresh = 2,
>> + .type = sun4i_a10_phy,
>> + .dedicated_clocks = false,
>> +};
>> +
>> +static const struct sun4i_usb_phy_cfg sun8i_a23_cfg = {
>> + .num_phys = 2,
>> + .disc_thresh = 3,
>> + .type = sun4i_a10_phy,
>> + .dedicated_clocks = true,
>> +};
>> +
>> +static const struct sun4i_usb_phy_cfg sun8i_a33_cfg = {
>> + .num_phys = 2,
>> + .disc_thresh = 3,
>> + .type = sun8i_a33_phy,
>> + .dedicated_clocks = true,
>> +};
>> +
>> +static const struct of_device_id sun4i_usb_phy_of_match[] = {
>> + { .compatible = "allwinner,sun4i-a10-usb-phy", .data = &sun4i_a10_cfg },
>> + { .compatible = "allwinner,sun5i-a13-usb-phy", .data = &sun5i_a13_cfg },
>> + { .compatible = "allwinner,sun6i-a31-usb-phy", .data = &sun6i_a31_cfg },
>> + { .compatible = "allwinner,sun7i-a20-usb-phy", .data = &sun7i_a20_cfg },
>> + { .compatible = "allwinner,sun8i-a23-usb-phy", .data = &sun8i_a23_cfg },
>> + { .compatible = "allwinner,sun8i-a33-usb-phy", .data = &sun8i_a33_cfg },
>> + { },
>> +};
>> +MODULE_DEVICE_TABLE(of, sun4i_usb_phy_of_match);
>> +
>> static const unsigned int sun4i_usb_phy0_cable[] = {
>> EXTCON_USB,
>> EXTCON_USB_HOST,
>> @@ -511,10 +578,16 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
>> struct device *dev = &pdev->dev;
>> struct device_node *np = dev->of_node;
>> struct phy_provider *phy_provider;
>> - bool dedicated_clocks;
>> + const struct of_device_id *match;
>> struct resource *res;
>> int i, ret;
>>
>> + match = of_match_node(sun4i_usb_phy_of_match, dev->of_node);
>
> You can use of_device_get_match_data() for slightly less code. This
> will also let you keep the of_device_id table where it was, at the
> bottom.
>
>> + if (!match) {
>
> I'm working on something similar in the axp20x driver. Is there any
> case of_match_node or of_device_get_match_data can fail?
>
>
> Regards
> ChenYu
>
>> + dev_err(dev, "of_match_node() failed\n");
>> + return -EINVAL;
>> + }
>> +
>> data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>> if (!data)
>> return -ENOMEM;
>> @@ -522,29 +595,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
>> mutex_init(&data->mutex);
>> INIT_DELAYED_WORK(&data->detect, sun4i_usb_phy0_id_vbus_det_scan);
>> dev_set_drvdata(dev, data);
>> -
>> - if (of_device_is_compatible(np, "allwinner,sun5i-a13-usb-phy") ||
>> - of_device_is_compatible(np, "allwinner,sun8i-a23-usb-phy") ||
>> - of_device_is_compatible(np, "allwinner,sun8i-a33-usb-phy"))
>> - data->num_phys = 2;
>> - else
>> - data->num_phys = 3;
>> -
>> - if (of_device_is_compatible(np, "allwinner,sun5i-a13-usb-phy") ||
>> - of_device_is_compatible(np, "allwinner,sun7i-a20-usb-phy"))
>> - data->disc_thresh = 2;
>> - else
>> - data->disc_thresh = 3;
>> -
>> - if (of_device_is_compatible(np, "allwinner,sun6i-a31-usb-phy") ||
>> - of_device_is_compatible(np, "allwinner,sun8i-a23-usb-phy") ||
>> - of_device_is_compatible(np, "allwinner,sun8i-a33-usb-phy"))
>> - dedicated_clocks = true;
>> - else
>> - dedicated_clocks = false;
>> -
>> - if (of_device_is_compatible(np, "allwinner,sun8i-a33-usb-phy"))
>> - data->has_a33_phyctl = true;
>> + data->cfg = match->data;
>>
>> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "phy_ctrl");
>> data->base = devm_ioremap_resource(dev, res);
>> @@ -590,7 +641,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
>> }
>> }
>>
>> - for (i = 0; i < data->num_phys; i++) {
>> + for (i = 0; i < data->cfg->num_phys; i++) {
>> struct sun4i_usb_phy *phy = data->phys + i;
>> char name[16];
>>
>> @@ -602,7 +653,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
>> phy->vbus = NULL;
>> }
>>
>> - if (dedicated_clocks)
>> + if (data->cfg->dedicated_clocks)
>> snprintf(name, sizeof(name), "usb%d_phy", i);
>> else
>> strlcpy(name, "usb_phy", sizeof(name));
>> @@ -689,17 +740,6 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
>> return 0;
>> }
>>
>> -static const struct of_device_id sun4i_usb_phy_of_match[] = {
>> - { .compatible = "allwinner,sun4i-a10-usb-phy" },
>> - { .compatible = "allwinner,sun5i-a13-usb-phy" },
>> - { .compatible = "allwinner,sun6i-a31-usb-phy" },
>> - { .compatible = "allwinner,sun7i-a20-usb-phy" },
>> - { .compatible = "allwinner,sun8i-a23-usb-phy" },
>> - { .compatible = "allwinner,sun8i-a33-usb-phy" },
>> - { },
>> -};
>> -MODULE_DEVICE_TABLE(of, sun4i_usb_phy_of_match);
>> -
>> static struct platform_driver sun4i_usb_phy_driver = {
>> .probe = sun4i_usb_phy_probe,
>> .remove = sun4i_usb_phy_remove,
>> --
>> 2.5.0
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 22+ messages in thread
* [linux-sunxi] Re: [PATCH v3 1/2] phy-sun4i-usb: Use of_match_node to get model specific config data
@ 2015-11-26 12:11 ` Hans de Goede
0 siblings, 0 replies; 22+ messages in thread
From: Hans de Goede @ 2015-11-26 12:11 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On 26-11-15 05:22, Chen-Yu Tsai wrote:
> On Thu, Nov 26, 2015 at 12:50 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Use of_match_node instead of calling of_device_is_compatible a ton of
>> times to get model specific config data.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Changes in v3:
>> -New patch in v3 of this patch-set
>> ---
>> drivers/phy/phy-sun4i-usb.c | 130 +++++++++++++++++++++++++++++---------------
>> 1 file changed, 85 insertions(+), 45 deletions(-)
>>
>> diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
>> index b12964b..1d8f85d 100644
>> --- a/drivers/phy/phy-sun4i-usb.c
>> +++ b/drivers/phy/phy-sun4i-usb.c
>> @@ -88,12 +88,22 @@
>> #define DEBOUNCE_TIME msecs_to_jiffies(50)
>> #define POLL_TIME msecs_to_jiffies(250)
>>
>> +enum sun4i_usb_phy_type {
>> + sun4i_a10_phy,
>> + sun8i_a33_phy,
>> +};
>> +
>> +struct sun4i_usb_phy_cfg {
>> + int num_phys;
>> + u32 disc_thresh;
>> + enum sun4i_usb_phy_type type;
>> + bool dedicated_clocks;
>> +};
>> +
>> struct sun4i_usb_phy_data {
>> void __iomem *base;
>> + const struct sun4i_usb_phy_cfg *cfg;
>> struct mutex mutex;
>> - int num_phys;
>> - u32 disc_thresh;
>> - bool has_a33_phyctl;
>> struct sun4i_usb_phy {
>> struct phy *phy;
>> void __iomem *pmu;
>> @@ -164,12 +174,15 @@ static void sun4i_usb_phy_write(struct sun4i_usb_phy *phy, u32 addr, u32 data,
>>
>> mutex_lock(&phy_data->mutex);
>>
>> - if (phy_data->has_a33_phyctl) {
>> + switch (phy_data->cfg->type) {
>> + case sun4i_a10_phy:
>> + phyctl = phy_data->base + REG_PHYCTL_A10;
>
> Any reason why this offset isn't incorporated into phy_data?
You mean in phy_data->cfg I assume, the difference needed for
the "sun4i_usb_phy_write" functionality are not just the phyctl
register offset...
>
>> + break;
>> + case sun8i_a33_phy:
>> phyctl = phy_data->base + REG_PHYCTL_A33;
>> /* A33 needs us to set phyctl to 0 explicitly */
>> writel(0, phyctl);
e.g. the A33 needs this extra write, and on the H3 we need to do
similar bitbanging, but slightly different, see:
https://github.com/allwinner-zh/linux-3.4-sunxi/blob/master/drivers/usb/host/sunxi_hci.c#L899
Notice how it uses different addr and write register addresses
their through the usb_phy_csr_add and usb_phy_csr_write helper
functions as well as directly poking offset 0x20.
My current H3 usb-phy support patches do not deal with this,
as the h3 sdk code posted by the orange-pi people never calls
this function, so we do not need to worry about implementing
sun4i_usb_phy_write for H3, but if we ever want to tweak
lowlevel phy settings, and/or for the A83 we likely need to
implement a whole different sun4i_usb_phy_write code-path for
the H3/A83.
So it seems better to just have phy_data->cfg->type and deal
with any SoC specific differences on that, as there are more
differences to deal with then just the phyctl register offset.
Regards,
Hans
>> - } else {
>> - phyctl = phy_data->base + REG_PHYCTL_A10;
>> + break;
>> }
>>
>> for (i = 0; i < len; i++) {
>> @@ -249,7 +262,8 @@ static int sun4i_usb_phy_init(struct phy *_phy)
>> sun4i_usb_phy_write(phy, PHY_TX_AMPLITUDE_TUNE, 0x14, 5);
>>
>> /* Disconnect threshold adjustment */
>> - sun4i_usb_phy_write(phy, PHY_DISCON_TH_SEL, data->disc_thresh, 2);
>> + sun4i_usb_phy_write(phy, PHY_DISCON_TH_SEL,
>> + data->cfg->disc_thresh, 2);
>>
>> sun4i_usb_phy_passby(phy, 1);
>>
>> @@ -476,7 +490,7 @@ static struct phy *sun4i_usb_phy_xlate(struct device *dev,
>> {
>> struct sun4i_usb_phy_data *data = dev_get_drvdata(dev);
>>
>> - if (args->args[0] >= data->num_phys)
>> + if (args->args[0] >= data->cfg->num_phys)
>> return ERR_PTR(-ENODEV);
>>
>> return data->phys[args->args[0]].phy;
>> @@ -499,6 +513,59 @@ static int sun4i_usb_phy_remove(struct platform_device *pdev)
>> return 0;
>> }
>>
>> +static const struct sun4i_usb_phy_cfg sun4i_a10_cfg = {
>> + .num_phys = 3,
>> + .disc_thresh = 3,
>> + .type = sun4i_a10_phy,
>> + .dedicated_clocks = false,
>> +};
>> +
>> +static const struct sun4i_usb_phy_cfg sun5i_a13_cfg = {
>> + .num_phys = 2,
>> + .disc_thresh = 2,
>> + .type = sun4i_a10_phy,
>> + .dedicated_clocks = false,
>> +};
>> +
>> +static const struct sun4i_usb_phy_cfg sun6i_a31_cfg = {
>> + .num_phys = 3,
>> + .disc_thresh = 3,
>> + .type = sun4i_a10_phy,
>> + .dedicated_clocks = true,
>> +};
>> +
>> +static const struct sun4i_usb_phy_cfg sun7i_a20_cfg = {
>> + .num_phys = 3,
>> + .disc_thresh = 2,
>> + .type = sun4i_a10_phy,
>> + .dedicated_clocks = false,
>> +};
>> +
>> +static const struct sun4i_usb_phy_cfg sun8i_a23_cfg = {
>> + .num_phys = 2,
>> + .disc_thresh = 3,
>> + .type = sun4i_a10_phy,
>> + .dedicated_clocks = true,
>> +};
>> +
>> +static const struct sun4i_usb_phy_cfg sun8i_a33_cfg = {
>> + .num_phys = 2,
>> + .disc_thresh = 3,
>> + .type = sun8i_a33_phy,
>> + .dedicated_clocks = true,
>> +};
>> +
>> +static const struct of_device_id sun4i_usb_phy_of_match[] = {
>> + { .compatible = "allwinner,sun4i-a10-usb-phy", .data = &sun4i_a10_cfg },
>> + { .compatible = "allwinner,sun5i-a13-usb-phy", .data = &sun5i_a13_cfg },
>> + { .compatible = "allwinner,sun6i-a31-usb-phy", .data = &sun6i_a31_cfg },
>> + { .compatible = "allwinner,sun7i-a20-usb-phy", .data = &sun7i_a20_cfg },
>> + { .compatible = "allwinner,sun8i-a23-usb-phy", .data = &sun8i_a23_cfg },
>> + { .compatible = "allwinner,sun8i-a33-usb-phy", .data = &sun8i_a33_cfg },
>> + { },
>> +};
>> +MODULE_DEVICE_TABLE(of, sun4i_usb_phy_of_match);
>> +
>> static const unsigned int sun4i_usb_phy0_cable[] = {
>> EXTCON_USB,
>> EXTCON_USB_HOST,
>> @@ -511,10 +578,16 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
>> struct device *dev = &pdev->dev;
>> struct device_node *np = dev->of_node;
>> struct phy_provider *phy_provider;
>> - bool dedicated_clocks;
>> + const struct of_device_id *match;
>> struct resource *res;
>> int i, ret;
>>
>> + match = of_match_node(sun4i_usb_phy_of_match, dev->of_node);
>
> You can use of_device_get_match_data() for slightly less code. This
> will also let you keep the of_device_id table where it was, at the
> bottom.
>
>> + if (!match) {
>
> I'm working on something similar in the axp20x driver. Is there any
> case of_match_node or of_device_get_match_data can fail?
>
>
> Regards
> ChenYu
>
>> + dev_err(dev, "of_match_node() failed\n");
>> + return -EINVAL;
>> + }
>> +
>> data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>> if (!data)
>> return -ENOMEM;
>> @@ -522,29 +595,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
>> mutex_init(&data->mutex);
>> INIT_DELAYED_WORK(&data->detect, sun4i_usb_phy0_id_vbus_det_scan);
>> dev_set_drvdata(dev, data);
>> -
>> - if (of_device_is_compatible(np, "allwinner,sun5i-a13-usb-phy") ||
>> - of_device_is_compatible(np, "allwinner,sun8i-a23-usb-phy") ||
>> - of_device_is_compatible(np, "allwinner,sun8i-a33-usb-phy"))
>> - data->num_phys = 2;
>> - else
>> - data->num_phys = 3;
>> -
>> - if (of_device_is_compatible(np, "allwinner,sun5i-a13-usb-phy") ||
>> - of_device_is_compatible(np, "allwinner,sun7i-a20-usb-phy"))
>> - data->disc_thresh = 2;
>> - else
>> - data->disc_thresh = 3;
>> -
>> - if (of_device_is_compatible(np, "allwinner,sun6i-a31-usb-phy") ||
>> - of_device_is_compatible(np, "allwinner,sun8i-a23-usb-phy") ||
>> - of_device_is_compatible(np, "allwinner,sun8i-a33-usb-phy"))
>> - dedicated_clocks = true;
>> - else
>> - dedicated_clocks = false;
>> -
>> - if (of_device_is_compatible(np, "allwinner,sun8i-a33-usb-phy"))
>> - data->has_a33_phyctl = true;
>> + data->cfg = match->data;
>>
>> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "phy_ctrl");
>> data->base = devm_ioremap_resource(dev, res);
>> @@ -590,7 +641,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
>> }
>> }
>>
>> - for (i = 0; i < data->num_phys; i++) {
>> + for (i = 0; i < data->cfg->num_phys; i++) {
>> struct sun4i_usb_phy *phy = data->phys + i;
>> char name[16];
>>
>> @@ -602,7 +653,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
>> phy->vbus = NULL;
>> }
>>
>> - if (dedicated_clocks)
>> + if (data->cfg->dedicated_clocks)
>> snprintf(name, sizeof(name), "usb%d_phy", i);
>> else
>> strlcpy(name, "usb_phy", sizeof(name));
>> @@ -689,17 +740,6 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
>> return 0;
>> }
>>
>> -static const struct of_device_id sun4i_usb_phy_of_match[] = {
>> - { .compatible = "allwinner,sun4i-a10-usb-phy" },
>> - { .compatible = "allwinner,sun5i-a13-usb-phy" },
>> - { .compatible = "allwinner,sun6i-a31-usb-phy" },
>> - { .compatible = "allwinner,sun7i-a20-usb-phy" },
>> - { .compatible = "allwinner,sun8i-a23-usb-phy" },
>> - { .compatible = "allwinner,sun8i-a33-usb-phy" },
>> - { },
>> -};
>> -MODULE_DEVICE_TABLE(of, sun4i_usb_phy_of_match);
>> -
>> static struct platform_driver sun4i_usb_phy_driver = {
>> .probe = sun4i_usb_phy_probe,
>> .remove = sun4i_usb_phy_remove,
>> --
>> 2.5.0
>>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [linux-sunxi] Re: [PATCH v3 1/2] phy-sun4i-usb: Use of_match_node to get model specific config data
2015-11-26 12:11 ` Hans de Goede
@ 2015-11-27 8:53 ` Maxime Ripard
-1 siblings, 0 replies; 22+ messages in thread
From: Maxime Ripard @ 2015-11-27 8:53 UTC (permalink / raw)
To: Hans de Goede
Cc: Chen-Yu Tsai, Kishon Vijay Abraham I, Reinder E.N. de Haan,
linux-usb, linux-arm-kernel, devicetree, linux-sunxi
[-- Attachment #1: Type: text/plain, Size: 2376 bytes --]
On Thu, Nov 26, 2015 at 01:11:32PM +0100, Hans de Goede wrote:
> >>+enum sun4i_usb_phy_type {
> >>+ sun4i_a10_phy,
> >>+ sun8i_a33_phy,
> >>+};
> >>+
> >>+struct sun4i_usb_phy_cfg {
> >>+ int num_phys;
> >>+ u32 disc_thresh;
> >>+ enum sun4i_usb_phy_type type;
> >>+ bool dedicated_clocks;
> >>+};
> >>+
> >> struct sun4i_usb_phy_data {
> >> void __iomem *base;
> >>+ const struct sun4i_usb_phy_cfg *cfg;
> >> struct mutex mutex;
> >>- int num_phys;
> >>- u32 disc_thresh;
> >>- bool has_a33_phyctl;
> >> struct sun4i_usb_phy {
> >> struct phy *phy;
> >> void __iomem *pmu;
> >>@@ -164,12 +174,15 @@ static void sun4i_usb_phy_write(struct sun4i_usb_phy *phy, u32 addr, u32 data,
> >>
> >> mutex_lock(&phy_data->mutex);
> >>
> >>- if (phy_data->has_a33_phyctl) {
> >>+ switch (phy_data->cfg->type) {
> >>+ case sun4i_a10_phy:
> >>+ phyctl = phy_data->base + REG_PHYCTL_A10;
> >
> >Any reason why this offset isn't incorporated into phy_data?
>
> You mean in phy_data->cfg I assume, the difference needed for
> the "sun4i_usb_phy_write" functionality are not just the phyctl
> register offset...
>
> >
> >>+ break;
> >>+ case sun8i_a33_phy:
> >> phyctl = phy_data->base + REG_PHYCTL_A33;
> >> /* A33 needs us to set phyctl to 0 explicitly */
> >> writel(0, phyctl);
>
> e.g. the A33 needs this extra write, and on the H3 we need to do
> similar bitbanging, but slightly different, see:
>
> https://github.com/allwinner-zh/linux-3.4-sunxi/blob/master/drivers/usb/host/sunxi_hci.c#L899
>
> Notice how it uses different addr and write register addresses
> their through the usb_phy_csr_add and usb_phy_csr_write helper
> functions as well as directly poking offset 0x20.
Then it easy to support: one u8 for each register that changes, one
bool to tell if you need to clear the phyctl register or not, And you
don't have to duplicate the switch everywhere, and basically just
reimplement of_device_is_compatible without an actual compatible to
workaround the review ;)
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* [linux-sunxi] Re: [PATCH v3 1/2] phy-sun4i-usb: Use of_match_node to get model specific config data
@ 2015-11-27 8:53 ` Maxime Ripard
0 siblings, 0 replies; 22+ messages in thread
From: Maxime Ripard @ 2015-11-27 8:53 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Nov 26, 2015 at 01:11:32PM +0100, Hans de Goede wrote:
> >>+enum sun4i_usb_phy_type {
> >>+ sun4i_a10_phy,
> >>+ sun8i_a33_phy,
> >>+};
> >>+
> >>+struct sun4i_usb_phy_cfg {
> >>+ int num_phys;
> >>+ u32 disc_thresh;
> >>+ enum sun4i_usb_phy_type type;
> >>+ bool dedicated_clocks;
> >>+};
> >>+
> >> struct sun4i_usb_phy_data {
> >> void __iomem *base;
> >>+ const struct sun4i_usb_phy_cfg *cfg;
> >> struct mutex mutex;
> >>- int num_phys;
> >>- u32 disc_thresh;
> >>- bool has_a33_phyctl;
> >> struct sun4i_usb_phy {
> >> struct phy *phy;
> >> void __iomem *pmu;
> >>@@ -164,12 +174,15 @@ static void sun4i_usb_phy_write(struct sun4i_usb_phy *phy, u32 addr, u32 data,
> >>
> >> mutex_lock(&phy_data->mutex);
> >>
> >>- if (phy_data->has_a33_phyctl) {
> >>+ switch (phy_data->cfg->type) {
> >>+ case sun4i_a10_phy:
> >>+ phyctl = phy_data->base + REG_PHYCTL_A10;
> >
> >Any reason why this offset isn't incorporated into phy_data?
>
> You mean in phy_data->cfg I assume, the difference needed for
> the "sun4i_usb_phy_write" functionality are not just the phyctl
> register offset...
>
> >
> >>+ break;
> >>+ case sun8i_a33_phy:
> >> phyctl = phy_data->base + REG_PHYCTL_A33;
> >> /* A33 needs us to set phyctl to 0 explicitly */
> >> writel(0, phyctl);
>
> e.g. the A33 needs this extra write, and on the H3 we need to do
> similar bitbanging, but slightly different, see:
>
> https://github.com/allwinner-zh/linux-3.4-sunxi/blob/master/drivers/usb/host/sunxi_hci.c#L899
>
> Notice how it uses different addr and write register addresses
> their through the usb_phy_csr_add and usb_phy_csr_write helper
> functions as well as directly poking offset 0x20.
Then it easy to support: one u8 for each register that changes, one
bool to tell if you need to clear the phyctl register or not, And you
don't have to duplicate the switch everywhere, and basically just
reimplement of_device_is_compatible without an actual compatible to
workaround the review ;)
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20151127/a54dd928/attachment.sig>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [linux-sunxi] Re: [PATCH v3 1/2] phy-sun4i-usb: Use of_match_node to get model specific config data
2015-11-27 8:53 ` Maxime Ripard
@ 2015-11-27 10:37 ` Hans de Goede
-1 siblings, 0 replies; 22+ messages in thread
From: Hans de Goede @ 2015-11-27 10:37 UTC (permalink / raw)
To: Maxime Ripard
Cc: Chen-Yu Tsai, Kishon Vijay Abraham I, Reinder E.N. de Haan,
linux-usb, linux-arm-kernel, devicetree, linux-sunxi
Hi,
On 27-11-15 09:53, Maxime Ripard wrote:
> On Thu, Nov 26, 2015 at 01:11:32PM +0100, Hans de Goede wrote:
>>>> +enum sun4i_usb_phy_type {
>>>> + sun4i_a10_phy,
>>>> + sun8i_a33_phy,
>>>> +};
>>>> +
>>>> +struct sun4i_usb_phy_cfg {
>>>> + int num_phys;
>>>> + u32 disc_thresh;
>>>> + enum sun4i_usb_phy_type type;
>>>> + bool dedicated_clocks;
>>>> +};
>>>> +
>>>> struct sun4i_usb_phy_data {
>>>> void __iomem *base;
>>>> + const struct sun4i_usb_phy_cfg *cfg;
>>>> struct mutex mutex;
>>>> - int num_phys;
>>>> - u32 disc_thresh;
>>>> - bool has_a33_phyctl;
>>>> struct sun4i_usb_phy {
>>>> struct phy *phy;
>>>> void __iomem *pmu;
>>>> @@ -164,12 +174,15 @@ static void sun4i_usb_phy_write(struct sun4i_usb_phy *phy, u32 addr, u32 data,
>>>>
>>>> mutex_lock(&phy_data->mutex);
>>>>
>>>> - if (phy_data->has_a33_phyctl) {
>>>> + switch (phy_data->cfg->type) {
>>>> + case sun4i_a10_phy:
>>>> + phyctl = phy_data->base + REG_PHYCTL_A10;
>>>
>>> Any reason why this offset isn't incorporated into phy_data?
>>
>> You mean in phy_data->cfg I assume, the difference needed for
>> the "sun4i_usb_phy_write" functionality are not just the phyctl
>> register offset...
>>
>>>
>>>> + break;
>>>> + case sun8i_a33_phy:
>>>> phyctl = phy_data->base + REG_PHYCTL_A33;
>>>> /* A33 needs us to set phyctl to 0 explicitly */
>>>> writel(0, phyctl);
>>
>> e.g. the A33 needs this extra write, and on the H3 we need to do
>> similar bitbanging, but slightly different, see:
>>
>> https://github.com/allwinner-zh/linux-3.4-sunxi/blob/master/drivers/usb/host/sunxi_hci.c#L899
>>
>> Notice how it uses different addr and write register addresses
>> their through the usb_phy_csr_add and usb_phy_csr_write helper
>> functions as well as directly poking offset 0x20.
>
> Then it easy to support: one u8 for each register that changes, one
> bool to tell if you need to clear the phyctl register or not, And you
> don't have to duplicate the switch everywhere, and basically just
> reimplement of_device_is_compatible without an actual compatible to
> workaround the review ;)
You clearly have not looked at the actual code I've linked to, the
entire "algorithm" for sun4i_usb_phy_write is different on the H3.
Moreover, this has nothing to do with this patch, the code coding the
difference behavior for the a10 style phy and the a33 style phy is already
there before this patch-set, and this is not something Kishon asked
me to change.
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 22+ messages in thread
* [linux-sunxi] Re: [PATCH v3 1/2] phy-sun4i-usb: Use of_match_node to get model specific config data
@ 2015-11-27 10:37 ` Hans de Goede
0 siblings, 0 replies; 22+ messages in thread
From: Hans de Goede @ 2015-11-27 10:37 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On 27-11-15 09:53, Maxime Ripard wrote:
> On Thu, Nov 26, 2015 at 01:11:32PM +0100, Hans de Goede wrote:
>>>> +enum sun4i_usb_phy_type {
>>>> + sun4i_a10_phy,
>>>> + sun8i_a33_phy,
>>>> +};
>>>> +
>>>> +struct sun4i_usb_phy_cfg {
>>>> + int num_phys;
>>>> + u32 disc_thresh;
>>>> + enum sun4i_usb_phy_type type;
>>>> + bool dedicated_clocks;
>>>> +};
>>>> +
>>>> struct sun4i_usb_phy_data {
>>>> void __iomem *base;
>>>> + const struct sun4i_usb_phy_cfg *cfg;
>>>> struct mutex mutex;
>>>> - int num_phys;
>>>> - u32 disc_thresh;
>>>> - bool has_a33_phyctl;
>>>> struct sun4i_usb_phy {
>>>> struct phy *phy;
>>>> void __iomem *pmu;
>>>> @@ -164,12 +174,15 @@ static void sun4i_usb_phy_write(struct sun4i_usb_phy *phy, u32 addr, u32 data,
>>>>
>>>> mutex_lock(&phy_data->mutex);
>>>>
>>>> - if (phy_data->has_a33_phyctl) {
>>>> + switch (phy_data->cfg->type) {
>>>> + case sun4i_a10_phy:
>>>> + phyctl = phy_data->base + REG_PHYCTL_A10;
>>>
>>> Any reason why this offset isn't incorporated into phy_data?
>>
>> You mean in phy_data->cfg I assume, the difference needed for
>> the "sun4i_usb_phy_write" functionality are not just the phyctl
>> register offset...
>>
>>>
>>>> + break;
>>>> + case sun8i_a33_phy:
>>>> phyctl = phy_data->base + REG_PHYCTL_A33;
>>>> /* A33 needs us to set phyctl to 0 explicitly */
>>>> writel(0, phyctl);
>>
>> e.g. the A33 needs this extra write, and on the H3 we need to do
>> similar bitbanging, but slightly different, see:
>>
>> https://github.com/allwinner-zh/linux-3.4-sunxi/blob/master/drivers/usb/host/sunxi_hci.c#L899
>>
>> Notice how it uses different addr and write register addresses
>> their through the usb_phy_csr_add and usb_phy_csr_write helper
>> functions as well as directly poking offset 0x20.
>
> Then it easy to support: one u8 for each register that changes, one
> bool to tell if you need to clear the phyctl register or not, And you
> don't have to duplicate the switch everywhere, and basically just
> reimplement of_device_is_compatible without an actual compatible to
> workaround the review ;)
You clearly have not looked at the actual code I've linked to, the
entire "algorithm" for sun4i_usb_phy_write is different on the H3.
Moreover, this has nothing to do with this patch, the code coding the
difference behavior for the a10 style phy and the a33 style phy is already
there before this patch-set, and this is not something Kishon asked
me to change.
Regards,
Hans
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: Re: [PATCH v3 1/2] phy-sun4i-usb: Use of_match_node to get model specific config data
2015-11-27 10:37 ` Hans de Goede
@ 2015-11-27 10:41 ` Hans de Goede
-1 siblings, 0 replies; 22+ messages in thread
From: Hans de Goede @ 2015-11-27 10:41 UTC (permalink / raw)
To: Maxime Ripard
Cc: Chen-Yu Tsai, Kishon Vijay Abraham I, Reinder E.N. de Haan,
linux-usb, linux-arm-kernel, devicetree, linux-sunxi
Hi,
On 27-11-15 11:37, Hans de Goede wrote:
> Hi,
>
> On 27-11-15 09:53, Maxime Ripard wrote:
>> On Thu, Nov 26, 2015 at 01:11:32PM +0100, Hans de Goede wrote:
>>>>> +enum sun4i_usb_phy_type {
>>>>> + sun4i_a10_phy,
>>>>> + sun8i_a33_phy,
>>>>> +};
>>>>> +
>>>>> +struct sun4i_usb_phy_cfg {
>>>>> + int num_phys;
>>>>> + u32 disc_thresh;
>>>>> + enum sun4i_usb_phy_type type;
>>>>> + bool dedicated_clocks;
>>>>> +};
>>>>> +
>>>>> struct sun4i_usb_phy_data {
>>>>> void __iomem *base;
>>>>> + const struct sun4i_usb_phy_cfg *cfg;
>>>>> struct mutex mutex;
>>>>> - int num_phys;
>>>>> - u32 disc_thresh;
>>>>> - bool has_a33_phyctl;
>>>>> struct sun4i_usb_phy {
>>>>> struct phy *phy;
>>>>> void __iomem *pmu;
>>>>> @@ -164,12 +174,15 @@ static void sun4i_usb_phy_write(struct sun4i_usb_phy *phy, u32 addr, u32 data,
>>>>>
>>>>> mutex_lock(&phy_data->mutex);
>>>>>
>>>>> - if (phy_data->has_a33_phyctl) {
>>>>> + switch (phy_data->cfg->type) {
>>>>> + case sun4i_a10_phy:
>>>>> + phyctl = phy_data->base + REG_PHYCTL_A10;
>>>>
>>>> Any reason why this offset isn't incorporated into phy_data?
>>>
>>> You mean in phy_data->cfg I assume, the difference needed for
>>> the "sun4i_usb_phy_write" functionality are not just the phyctl
>>> register offset...
>>>
>>>>
>>>>> + break;
>>>>> + case sun8i_a33_phy:
>>>>> phyctl = phy_data->base + REG_PHYCTL_A33;
>>>>> /* A33 needs us to set phyctl to 0 explicitly */
>>>>> writel(0, phyctl);
>>>
>>> e.g. the A33 needs this extra write, and on the H3 we need to do
>>> similar bitbanging, but slightly different, see:
>>>
>>> https://github.com/allwinner-zh/linux-3.4-sunxi/blob/master/drivers/usb/host/sunxi_hci.c#L899
>>>
>>> Notice how it uses different addr and write register addresses
>>> their through the usb_phy_csr_add and usb_phy_csr_write helper
>>> functions as well as directly poking offset 0x20.
>>
>> Then it easy to support: one u8 for each register that changes, one
>> bool to tell if you need to clear the phyctl register or not, And you
>> don't have to duplicate the switch everywhere, and basically just
>> reimplement of_device_is_compatible without an actual compatible to
>> workaround the review ;)
>
> You clearly have not looked at the actual code I've linked to, the
> entire "algorithm" for sun4i_usb_phy_write is different on the H3.
>
> Moreover, this has nothing to do with this patch, the code coding the
> difference behavior for the a10 style phy and the a33 style phy is already
> there before this patch-set, and this is not something Kishon asked
> me to change.
And on top of that phy_data->cfg->type is also used in different places,
where the behavior for h3 is again completely different then for the other
phy-s. There are just some differences which cannot be encoded in the form
of register offsets or bools between the old a10 usb phys and the newer ones.
And yes I've considered to just do a whole new phy driver, but there is enough
overlap to make doing this in one driver more sensible. This may change when
we add otg support for the H3. But we can always do a new driver then, and
remove the limited (see the 2nd patch in this series) amount of changes needed
for the H3 from phy-sun4i-usb.c again.
Regards,
Hans
^ permalink raw reply [flat|nested] 22+ messages in thread
* [linux-sunxi] Re: [PATCH v3 1/2] phy-sun4i-usb: Use of_match_node to get model specific config data
@ 2015-11-27 10:41 ` Hans de Goede
0 siblings, 0 replies; 22+ messages in thread
From: Hans de Goede @ 2015-11-27 10:41 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On 27-11-15 11:37, Hans de Goede wrote:
> Hi,
>
> On 27-11-15 09:53, Maxime Ripard wrote:
>> On Thu, Nov 26, 2015 at 01:11:32PM +0100, Hans de Goede wrote:
>>>>> +enum sun4i_usb_phy_type {
>>>>> + sun4i_a10_phy,
>>>>> + sun8i_a33_phy,
>>>>> +};
>>>>> +
>>>>> +struct sun4i_usb_phy_cfg {
>>>>> + int num_phys;
>>>>> + u32 disc_thresh;
>>>>> + enum sun4i_usb_phy_type type;
>>>>> + bool dedicated_clocks;
>>>>> +};
>>>>> +
>>>>> struct sun4i_usb_phy_data {
>>>>> void __iomem *base;
>>>>> + const struct sun4i_usb_phy_cfg *cfg;
>>>>> struct mutex mutex;
>>>>> - int num_phys;
>>>>> - u32 disc_thresh;
>>>>> - bool has_a33_phyctl;
>>>>> struct sun4i_usb_phy {
>>>>> struct phy *phy;
>>>>> void __iomem *pmu;
>>>>> @@ -164,12 +174,15 @@ static void sun4i_usb_phy_write(struct sun4i_usb_phy *phy, u32 addr, u32 data,
>>>>>
>>>>> mutex_lock(&phy_data->mutex);
>>>>>
>>>>> - if (phy_data->has_a33_phyctl) {
>>>>> + switch (phy_data->cfg->type) {
>>>>> + case sun4i_a10_phy:
>>>>> + phyctl = phy_data->base + REG_PHYCTL_A10;
>>>>
>>>> Any reason why this offset isn't incorporated into phy_data?
>>>
>>> You mean in phy_data->cfg I assume, the difference needed for
>>> the "sun4i_usb_phy_write" functionality are not just the phyctl
>>> register offset...
>>>
>>>>
>>>>> + break;
>>>>> + case sun8i_a33_phy:
>>>>> phyctl = phy_data->base + REG_PHYCTL_A33;
>>>>> /* A33 needs us to set phyctl to 0 explicitly */
>>>>> writel(0, phyctl);
>>>
>>> e.g. the A33 needs this extra write, and on the H3 we need to do
>>> similar bitbanging, but slightly different, see:
>>>
>>> https://github.com/allwinner-zh/linux-3.4-sunxi/blob/master/drivers/usb/host/sunxi_hci.c#L899
>>>
>>> Notice how it uses different addr and write register addresses
>>> their through the usb_phy_csr_add and usb_phy_csr_write helper
>>> functions as well as directly poking offset 0x20.
>>
>> Then it easy to support: one u8 for each register that changes, one
>> bool to tell if you need to clear the phyctl register or not, And you
>> don't have to duplicate the switch everywhere, and basically just
>> reimplement of_device_is_compatible without an actual compatible to
>> workaround the review ;)
>
> You clearly have not looked at the actual code I've linked to, the
> entire "algorithm" for sun4i_usb_phy_write is different on the H3.
>
> Moreover, this has nothing to do with this patch, the code coding the
> difference behavior for the a10 style phy and the a33 style phy is already
> there before this patch-set, and this is not something Kishon asked
> me to change.
And on top of that phy_data->cfg->type is also used in different places,
where the behavior for h3 is again completely different then for the other
phy-s. There are just some differences which cannot be encoded in the form
of register offsets or bools between the old a10 usb phys and the newer ones.
And yes I've considered to just do a whole new phy driver, but there is enough
overlap to make doing this in one driver more sensible. This may change when
we add otg support for the H3. But we can always do a new driver then, and
remove the limited (see the 2nd patch in this series) amount of changes needed
for the H3 from phy-sun4i-usb.c again.
Regards,
Hans
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 2/2] phy-sun4i-usb: Add support for the host usb-phys found on the H3 SoC
2015-11-25 16:50 ` Hans de Goede
@ 2015-12-01 15:56 ` Priit Laes
-1 siblings, 0 replies; 22+ messages in thread
From: Priit Laes @ 2015-12-01 15:56 UTC (permalink / raw)
To: hdegoede-H+wXaHxf7aLQT0dZR+AlfA, Kishon Vijay Abraham I
Cc: Reinder E.N. de Haan, Maxime Ripard, Chen-Yu Tsai,
linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Reinder de Haan
On Wed, 2015-11-25 at 17:50 +0100, Hans de Goede wrote:
> From: Reinder de Haan <patchesrdh-I1/eAgTnXDYAvxtiuMwx3w@public.gmane.org>
>
> Note this commit only adds support for phys 1-3, phy 0, the otg phy,
> is
> not yet (fully) supported after this commit.
>
> Signed-off-by: Reinder de Haan <patchesrdh-I1/eAgTnXDYAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> Changes in v2:
> -Change break; after dev_err() to return, as intended, fixing a
> compiler
> warning (the dev_err case should never be reached)
> Changes in v3:
> -Use of_match_node to get model specific config data
> ---
> .../devicetree/bindings/phy/sun4i-usb-phy.txt | 1 +
> drivers/phy/phy-sun4i-usb.c | 46
> +++++++++++++++++-----
> 2 files changed, 38 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
> b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
> index 0cebf74..95736d7 100644
> --- a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
> +++ b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
> @@ -9,6 +9,7 @@ Required properties:
> * allwinner,sun7i-a20-usb-phy
> * allwinner,sun8i-a23-usb-phy
> * allwinner,sun8i-a33-usb-phy
> + * allwinner,sun8i-h3-usb-phy
> - reg : a list of offset + length pairs
> - reg-names :
> * "phy_ctrl"
> diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-
> usb.c
> index 1d8f85d..2aed482 100644
> --- a/drivers/phy/phy-sun4i-usb.c
> +++ b/drivers/phy/phy-sun4i-usb.c
> @@ -46,6 +46,9 @@
> #define REG_PHYBIST 0x08
> #define REG_PHYTUNE 0x0c
> #define REG_PHYCTL_A33 0x10
> +#define REG_PHY_UNK_H3 0x20
> +
> +#define REG_PMU_UNK_H3 0x10
>
> #define PHYCTL_DATA BIT(7)
>
> @@ -79,7 +82,7 @@
> #define PHY_DISCON_TH_SEL 0x2a
> #define PHY_SQUELCH_DETECT 0x3c
>
> -#define MAX_PHYS 3
> +#define MAX_PHYS 4
>
> /*
> * Note do not raise the debounce time, we must report Vusb high
> within 100ms
> @@ -91,6 +94,7 @@
> enum sun4i_usb_phy_type {
> sun4i_a10_phy,
> sun8i_a33_phy,
> + sun8i_h3_phy,
> };
>
> struct sun4i_usb_phy_cfg {
> @@ -101,6 +105,7 @@ struct sun4i_usb_phy_cfg {
> };
>
> struct sun4i_usb_phy_data {
> + struct device *dev;
> void __iomem *base;
> const struct sun4i_usb_phy_cfg *cfg;
> struct mutex mutex;
> @@ -183,6 +188,9 @@ static void sun4i_usb_phy_write(struct
> sun4i_usb_phy *phy, u32 addr, u32 data,
> /* A33 needs us to set phyctl to 0 explicitly */
> writel(0, phyctl);
> break;
> + case sun8i_h3_phy:
> + dev_err(phy_data->dev, "H3 usb_phy_write is not
> supported\n");
You should unlock the mutex here.
> + return;
Also, it might make sense to add default case what WARN's on invalid
enum.
> }
>
> for (i = 0; i < len; i++) {
> @@ -243,6 +251,7 @@ static int sun4i_usb_phy_init(struct phy *_phy)
> struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
> struct sun4i_usb_phy_data *data =
> to_sun4i_usb_phy_data(phy);
> int ret;
> + u32 val;
>
> ret = clk_prepare_enable(phy->clk);
> if (ret)
> @@ -254,16 +263,26 @@ static int sun4i_usb_phy_init(struct phy *_phy)
> return ret;
> }
>
> - /* Enable USB 45 Ohm resistor calibration */
> - if (phy->index == 0)
> - sun4i_usb_phy_write(phy, PHY_RES45_CAL_EN, 0x01, 1);
> + if (data->cfg->type == sun8i_h3_phy) {
> + if (phy->index == 0) {
> + val = readl(data->base + REG_PHY_UNK_H3);
> + writel(val & ~1, data->base +
> REG_PHY_UNK_H3);
> + }
>
> - /* Adjust PHY's magnitude and rate */
> - sun4i_usb_phy_write(phy, PHY_TX_AMPLITUDE_TUNE, 0x14, 5);
> + val = readl(phy->pmu + REG_PMU_UNK_H3);
> + writel(val & ~2, phy->pmu + REG_PMU_UNK_H3);
> + } else {
> + /* Enable USB 45 Ohm resistor calibration */
> + if (phy->index == 0)
> + sun4i_usb_phy_write(phy, PHY_RES45_CAL_EN,
> 0x01, 1);
>
> - /* Disconnect threshold adjustment */
> - sun4i_usb_phy_write(phy, PHY_DISCON_TH_SEL,
> - data->cfg->disc_thresh, 2);
> + /* Adjust PHY's magnitude and rate */
> + sun4i_usb_phy_write(phy, PHY_TX_AMPLITUDE_TUNE,
> 0x14, 5);
> +
> + /* Disconnect threshold adjustment */
> + sun4i_usb_phy_write(phy, PHY_DISCON_TH_SEL,
> + data->cfg->disc_thresh, 2);
> + }
>
> sun4i_usb_phy_passby(phy, 1);
>
> @@ -555,6 +574,13 @@ static const struct sun4i_usb_phy_cfg
> sun8i_a33_cfg = {
> .dedicated_clocks = true,
> };
>
> +static const struct sun4i_usb_phy_cfg sun8i_h3_cfg = {
> + .num_phys = 4,
> + .disc_thresh = 3,
> + .type = sun8i_h3_phy,
> + .dedicated_clocks = true,
> +};
> +
> static const struct of_device_id sun4i_usb_phy_of_match[] = {
> { .compatible = "allwinner,sun4i-a10-usb-phy", .data =
> &sun4i_a10_cfg },
> { .compatible = "allwinner,sun5i-a13-usb-phy", .data =
> &sun5i_a13_cfg },
> @@ -562,6 +588,7 @@ static const struct of_device_id
> sun4i_usb_phy_of_match[] = {
> { .compatible = "allwinner,sun7i-a20-usb-phy", .data =
> &sun7i_a20_cfg },
> { .compatible = "allwinner,sun8i-a23-usb-phy", .data =
> &sun8i_a23_cfg },
> { .compatible = "allwinner,sun8i-a33-usb-phy", .data =
> &sun8i_a33_cfg },
> + { .compatible = "allwinner,sun8i-h3-usb-phy", .data =
> &sun8i_h3_cfg },
> { },
> };
> MODULE_DEVICE_TABLE(of, sun4i_usb_phy_of_match);
> @@ -595,6 +622,7 @@ static int sun4i_usb_phy_probe(struct
> platform_device *pdev)
> mutex_init(&data->mutex);
> INIT_DELAYED_WORK(&data->detect,
> sun4i_usb_phy0_id_vbus_det_scan);
> dev_set_drvdata(dev, data);
> + data->dev = dev;
> data->cfg = match->data;
>
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> "phy_ctrl");
> --
> 2.5.0
>
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [linux-sunxi] [PATCH v3 2/2] phy-sun4i-usb: Add support for the host usb-phys found on the H3 SoC
@ 2015-12-01 15:56 ` Priit Laes
0 siblings, 0 replies; 22+ messages in thread
From: Priit Laes @ 2015-12-01 15:56 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, 2015-11-25 at 17:50 +0100, Hans de Goede wrote:
> From: Reinder de Haan <patchesrdh@mveas.com>
>
> Note this commit only adds support for phys 1-3, phy 0, the otg phy,
> is
> not yet (fully) supported after this commit.
>
> Signed-off-by: Reinder de Haan <patchesrdh@mveas.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -Change break; after dev_err() to return, as intended, fixing a
> compiler
> ?warning (the dev_err case should never be reached)
> Changes in v3:
> -Use of_match_node to get model specific config data
> ---
> ?.../devicetree/bindings/phy/sun4i-usb-phy.txt??????|??1 +
> ?drivers/phy/phy-sun4i-usb.c????????????????????????| 46
> +++++++++++++++++-----
> ?2 files changed, 38 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
> b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
> index 0cebf74..95736d7 100644
> --- a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
> +++ b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
> @@ -9,6 +9,7 @@ Required properties:
> ???* allwinner,sun7i-a20-usb-phy
> ???* allwinner,sun8i-a23-usb-phy
> ???* allwinner,sun8i-a33-usb-phy
> +??* allwinner,sun8i-h3-usb-phy
> ?- reg : a list of offset + length pairs
> ?- reg-names :
> ???* "phy_ctrl"
> diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-
> usb.c
> index 1d8f85d..2aed482 100644
> --- a/drivers/phy/phy-sun4i-usb.c
> +++ b/drivers/phy/phy-sun4i-usb.c
> @@ -46,6 +46,9 @@
> ?#define REG_PHYBIST 0x08
> ?#define REG_PHYTUNE 0x0c
> ?#define REG_PHYCTL_A33 0x10
> +#define REG_PHY_UNK_H3 0x20
> +
> +#define REG_PMU_UNK_H3 0x10
> ?
> ?#define PHYCTL_DATA BIT(7)
> ?
> @@ -79,7 +82,7 @@
> ?#define PHY_DISCON_TH_SEL 0x2a
> ?#define PHY_SQUELCH_DETECT 0x3c
> ?
> -#define MAX_PHYS 3
> +#define MAX_PHYS 4
> ?
> ?/*
> ? * Note do not raise the debounce time, we must report Vusb high
> within 100ms
> @@ -91,6 +94,7 @@
> ?enum sun4i_usb_phy_type {
> ? sun4i_a10_phy,
> ? sun8i_a33_phy,
> + sun8i_h3_phy,
> ?};
> ?
> ?struct sun4i_usb_phy_cfg {
> @@ -101,6 +105,7 @@ struct sun4i_usb_phy_cfg {
> ?};
> ?
> ?struct sun4i_usb_phy_data {
> + struct device *dev;
> ? void __iomem *base;
> ? const struct sun4i_usb_phy_cfg *cfg;
> ? struct mutex mutex;
> @@ -183,6 +188,9 @@ static void sun4i_usb_phy_write(struct
> sun4i_usb_phy *phy, u32 addr, u32 data,
> ? /* A33 needs us to set phyctl to 0 explicitly */
> ? writel(0, phyctl);
> ? break;
> + case sun8i_h3_phy:
> + dev_err(phy_data->dev, "H3 usb_phy_write is not
> supported\n");
You should unlock the mutex here.
> + return;
Also, it might make sense to add default case what WARN's on invalid
enum.
> ? }
> ?
> ? for (i = 0; i < len; i++) {
> @@ -243,6 +251,7 @@ static int sun4i_usb_phy_init(struct phy *_phy)
> ? struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
> ? struct sun4i_usb_phy_data *data =
> to_sun4i_usb_phy_data(phy);
> ? int ret;
> + u32 val;
> ?
> ? ret = clk_prepare_enable(phy->clk);
> ? if (ret)
> @@ -254,16 +263,26 @@ static int sun4i_usb_phy_init(struct phy *_phy)
> ? return ret;
> ? }
> ?
> - /* Enable USB 45 Ohm resistor calibration */
> - if (phy->index == 0)
> - sun4i_usb_phy_write(phy, PHY_RES45_CAL_EN, 0x01, 1);
> + if (data->cfg->type == sun8i_h3_phy) {
> + if (phy->index == 0) {
> + val = readl(data->base + REG_PHY_UNK_H3);
> + writel(val & ~1, data->base +
> REG_PHY_UNK_H3);
> + }
> ?
> - /* Adjust PHY's magnitude and rate */
> - sun4i_usb_phy_write(phy, PHY_TX_AMPLITUDE_TUNE, 0x14, 5);
> + val = readl(phy->pmu + REG_PMU_UNK_H3);
> + writel(val & ~2, phy->pmu + REG_PMU_UNK_H3);
> + } else {
> + /* Enable USB 45 Ohm resistor calibration */
> + if (phy->index == 0)
> + sun4i_usb_phy_write(phy, PHY_RES45_CAL_EN,
> 0x01, 1);
> ?
> - /* Disconnect threshold adjustment */
> - sun4i_usb_phy_write(phy, PHY_DISCON_TH_SEL,
> - ????data->cfg->disc_thresh, 2);
> + /* Adjust PHY's magnitude and rate */
> + sun4i_usb_phy_write(phy, PHY_TX_AMPLITUDE_TUNE,
> 0x14, 5);
> +
> + /* Disconnect threshold adjustment */
> + sun4i_usb_phy_write(phy, PHY_DISCON_TH_SEL,
> + ????data->cfg->disc_thresh, 2);
> + }
> ?
> ? sun4i_usb_phy_passby(phy, 1);
> ?
> @@ -555,6 +574,13 @@ static const struct sun4i_usb_phy_cfg
> sun8i_a33_cfg = {
> ? .dedicated_clocks = true,
> ?};
> ?
> +static const struct sun4i_usb_phy_cfg sun8i_h3_cfg = {
> + .num_phys = 4,
> + .disc_thresh = 3,
> + .type = sun8i_h3_phy,
> + .dedicated_clocks = true,
> +};
> +
> ?static const struct of_device_id sun4i_usb_phy_of_match[] = {
> ? { .compatible = "allwinner,sun4i-a10-usb-phy", .data =
> &sun4i_a10_cfg },
> ? { .compatible = "allwinner,sun5i-a13-usb-phy", .data =
> &sun5i_a13_cfg },
> @@ -562,6 +588,7 @@ static const struct of_device_id
> sun4i_usb_phy_of_match[] = {
> ? { .compatible = "allwinner,sun7i-a20-usb-phy", .data =
> &sun7i_a20_cfg },
> ? { .compatible = "allwinner,sun8i-a23-usb-phy", .data =
> &sun8i_a23_cfg },
> ? { .compatible = "allwinner,sun8i-a33-usb-phy", .data =
> &sun8i_a33_cfg },
> + { .compatible = "allwinner,sun8i-h3-usb-phy", .data =
> &sun8i_h3_cfg },
> ? { },
> ?};
> ?MODULE_DEVICE_TABLE(of, sun4i_usb_phy_of_match);
> @@ -595,6 +622,7 @@ static int sun4i_usb_phy_probe(struct
> platform_device *pdev)
> ? mutex_init(&data->mutex);
> ? INIT_DELAYED_WORK(&data->detect,
> sun4i_usb_phy0_id_vbus_det_scan);
> ? dev_set_drvdata(dev, data);
> + data->dev = dev;
> ? data->cfg = match->data;
> ?
> ? res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> "phy_ctrl");
> --
> 2.5.0
>
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2015-12-01 15:56 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-25 16:50 [PATCH v3 1/2] phy-sun4i-usb: Use of_match_node to get model specific config data Hans de Goede
2015-11-25 16:50 ` Hans de Goede
[not found] ` <1448470202-3628-1-git-send-email-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2015-11-25 16:50 ` [PATCH v3 2/2] phy-sun4i-usb: Add support for the host usb-phys found on the H3 SoC Hans de Goede
2015-11-25 16:50 ` Hans de Goede
[not found] ` <1448470202-3628-2-git-send-email-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2015-11-25 22:11 ` Rob Herring
2015-11-25 22:11 ` Rob Herring
2015-11-26 12:03 ` Hans de Goede
2015-11-26 12:03 ` [linux-sunxi] " Hans de Goede
2015-12-01 15:56 ` Priit Laes
2015-12-01 15:56 ` [linux-sunxi] " Priit Laes
2015-11-26 4:22 ` [PATCH v3 1/2] phy-sun4i-usb: Use of_match_node to get model specific config data Chen-Yu Tsai
2015-11-26 4:22 ` Chen-Yu Tsai
[not found] ` <CAGb2v64XL=bptxoCwiVkf8u-EfckpuASS3Foo=Q3i4a_6CaJYA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-11-26 7:14 ` LABBE Corentin
2015-11-26 7:14 ` [linux-sunxi] " LABBE Corentin
2015-11-26 12:11 ` Hans de Goede
2015-11-26 12:11 ` Hans de Goede
[not found] ` <5656F6F4.1040801-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2015-11-27 8:53 ` Maxime Ripard
2015-11-27 8:53 ` Maxime Ripard
2015-11-27 10:37 ` Hans de Goede
2015-11-27 10:37 ` Hans de Goede
[not found] ` <56583260.1080106-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2015-11-27 10:41 ` Hans de Goede
2015-11-27 10:41 ` [linux-sunxi] " Hans de Goede
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.