[<prev] [next>] [day] [month] [year] [list]
Message-ID: <51d6a646-3125-5f6a-e5e4-34b74a886319@redhat.com>
Date: Wed, 1 Mar 2017 00:46:24 +0100
From: Hans de Goede <hdegoede@...hat.com>
To: Icenowy Zheng <icenowy@...c.xyz>,
Maxime Ripard <maxime.ripard@...e-electrons.com>,
Chen-Yu Tsai <wens@...e.org>,
Kishon Vijay Abraham I <kishon@...com>
Cc: devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, linux-sunxi@...glegroups.com
Subject: Re: [PATCH 1/3] phy: sun4i-usb: support automatically switch PHY0
route to MUSB/HCI
Hi,
On 28-02-17 16:27, Icenowy Zheng wrote:
> On newer Allwinner SoCs (H3 and after), the PHY0 node is routed to both
> MUSB controller for peripheral and host support (the host support is
> slightly broken), and a pair of EHCI/OHCI controllers, which provide a
> better support for host mode.
>
> Add support for automatically switch the route of PHY0 according to the
> status of dr_mode and id det pin.
>
> Only H3 have this function enabled in this patch, as further SoCs will
> be tested later and then have it enabled.
>
> Signed-off-by: Icenowy Zheng <icenowy@...c.xyz>
> ---
> drivers/phy/phy-sun4i-usb.c | 55 +++++++++++++++++++++++++++++++--------------
> 1 file changed, 38 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
> index a21b5f24a340..8b91afe8f42d 100644
> --- a/drivers/phy/phy-sun4i-usb.c
> +++ b/drivers/phy/phy-sun4i-usb.c
> @@ -49,12 +49,14 @@
> #define REG_PHYBIST 0x08
> #define REG_PHYTUNE 0x0c
> #define REG_PHYCTL_A33 0x10
> -#define REG_PHY_UNK_H3 0x20
> +#define REG_PHY_OTGCTL 0x20
>
> #define REG_PMU_UNK1 0x10
>
> #define PHYCTL_DATA BIT(7)
>
> +#define OTGCTL_ROUTE_MUSB BIT(0)
> +
> #define SUNXI_AHB_ICHR8_EN BIT(10)
> #define SUNXI_AHB_INCR4_BURST_EN BIT(9)
> #define SUNXI_AHB_INCRX_ALIGN_EN BIT(8)
> @@ -110,6 +112,7 @@ struct sun4i_usb_phy_cfg {
> u8 phyctl_offset;
> bool dedicated_clocks;
> bool enable_pmu_unk1;
> + bool phy0_dual_route;
> };
>
> struct sun4i_usb_phy_data {
> @@ -271,23 +274,16 @@ static int sun4i_usb_phy_init(struct phy *_phy)
> writel(val & ~2, phy->pmu + REG_PMU_UNK1);
> }
>
> - 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);
> - }
> - } else {
> - /* Enable USB 45 Ohm resistor calibration */
> - if (phy->index == 0)
> - sun4i_usb_phy_write(phy, PHY_RES45_CAL_EN, 0x01, 1);
> + /* Enable USB 45 Ohm resistor calibration */
> + if (phy->index == 0)
> + sun4i_usb_phy_write(phy, PHY_RES45_CAL_EN, 0x01, 1);
>
> - /* Adjust PHY's magnitude and rate */
> - sun4i_usb_phy_write(phy, PHY_TX_AMPLITUDE_TUNE, 0x14, 5);
> + /* 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);
> - }
> + /* Disconnect threshold adjustment */
> + sun4i_usb_phy_write(phy, PHY_DISCON_TH_SEL,
> + data->cfg->disc_thresh, 2);
>
> sun4i_usb_phy_passby(phy, 1);
>
> @@ -486,6 +482,26 @@ static const struct phy_ops sun4i_usb_phy_ops = {
> .owner = THIS_MODULE,
> };
>
> +static void sun4i_usb_phy0_reroute(struct sun4i_usb_phy_data *data, int id_det)
> +{
> + u32 regval;
> +
> + if (data->dr_mode == USB_DR_MODE_HOST)
> + id_det = 0; /* Force host */
> + if (data->dr_mode == USB_DR_MODE_PERIPHERAL)
> + id_det = 1; /* Force peripheral*/
The passed in id_det comes from sun4i_usb_phy0_get_id_det() which already
does:
switch (data->dr_mode) {
case USB_DR_MODE_OTG:
if (data->id_det_gpio)
return gpiod_get_value_cansleep(data->id_det_gpio);
else
return 1; /* Fallback to peripheral mode */
case USB_DR_MODE_HOST:
return 0;
case USB_DR_MODE_PERIPHERAL:
default:
return 1;
}
So the above 4 lines are unnecessary and should be removed.
Otherwise looks good, thank you for doing this.
Regards,
Hans
> +
> + regval = readl(data->base + REG_PHY_OTGCTL);
> + if (id_det == 0) {
> + /* Host mode. Route phy0 to EHCI/OHCI */
> + regval &= ~OTGCTL_ROUTE_MUSB;
> + } else {
> + /* Peripheral mode. Route phy0 to MUSB */
> + regval |= OTGCTL_ROUTE_MUSB;
> + }
> + writel(regval, data->base + REG_PHY_OTGCTL);
> +}
> +
> static void sun4i_usb_phy0_id_vbus_det_scan(struct work_struct *work)
> {
> struct sun4i_usb_phy_data *data =
> @@ -511,6 +527,10 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct work_struct *work)
> data->force_session_end = false;
>
> if (id_det != data->id_det) {
> + /* Re-route PHY0 if necessary */
> + if (data->cfg->phy0_dual_route)
> + sun4i_usb_phy0_reroute(data, id_det);
> +
> /* id-change, force session end if we've no vbus detection */
> if (data->dr_mode == USB_DR_MODE_OTG &&
> !sun4i_usb_phy0_have_vbus_det(data))
> @@ -700,7 +720,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
> return PTR_ERR(phy->reset);
> }
>
> - if (i) { /* No pmu for usbc0 */
> + if (i || data->cfg->phy0_dual_route) { /* No pmu for musb */
> snprintf(name, sizeof(name), "pmu%d", i);
> res = platform_get_resource_byname(pdev,
> IORESOURCE_MEM, name);
> @@ -825,6 +845,7 @@ static const struct sun4i_usb_phy_cfg sun8i_h3_cfg = {
> .disc_thresh = 3,
> .dedicated_clocks = true,
> .enable_pmu_unk1 = true,
> + .phy0_dual_route = true,
> };
>
> static const struct sun4i_usb_phy_cfg sun8i_v3s_cfg = {
>
Powered by blists - more mailing lists