[U-Boot] [PATCH v4 09/34] phy: Add Allwinner A64 USB PHY driver

[Marek Vasut]

On 02/06/2018 03:25 PM, Jagan Teki wrote:
> USB PHY implementation for Allwinner SOC's can be handling
> in to single driver with different phy configs.
> 
> This driver handle all Allwinner USB PHY's start from 4I to
> 50I(except 9I). Currently added A64 compatibility more will
> add in next coming patches.
> 
> Current implementation is unable to get pinctrl, clock and reset
> details from DT since the dm code on these will add it future.
> 
> Driver named as phy-sun4i-usb.c since the same PHY logic
> work for all Allwinner SOC's start from 4I to A64 except 9I
> with different phy configurations.
> 
> Signed-off-by: Jagan Teki <jagan at amarulasolutions.com>

[...]

> +#define MAX_PHYS			4

There should be one PHY per controller , so why do you need this ?

> +enum sun4i_usb_phy_type {
> +	sun50i_a64_phy,
> +};
> +
> +struct sun4i_usb_phy_cfg {
> +	int num_phys;
> +	enum sun4i_usb_phy_type type;
> +	u32 disc_thresh;
> +	u8 phyctl_offset;
> +	bool enable_pmu_unk1;
> +	bool phy0_dual_route;
> +};
> +
> +struct sun4i_usb_phy_info {
> +	const char *gpio_vbus;
> +	const char *gpio_vbus_det;
> +	const char *gpio_id_det;
> +	int rst_mask;
> +} phy_info[] = {
> +	{
> +		.gpio_vbus = CONFIG_USB0_VBUS_PIN,
> +		.gpio_vbus_det = CONFIG_USB0_VBUS_DET,
> +		.gpio_id_det = CONFIG_USB0_ID_DET,
> +		.rst_mask = (CCM_USB_CTRL_PHY0_RST | CCM_USB_CTRL_PHY0_CLK),
> +	},
> +	{
> +		.gpio_vbus = CONFIG_USB1_VBUS_PIN,
> +		.gpio_vbus_det = NULL,
> +		.gpio_id_det = NULL,
> +		.rst_mask = (CCM_USB_CTRL_PHY1_RST | CCM_USB_CTRL_PHY1_CLK),
> +	},
> +	{
> +		.gpio_vbus = CONFIG_USB2_VBUS_PIN,
> +		.gpio_vbus_det = NULL,
> +		.gpio_id_det = NULL,
> +		.rst_mask = (CCM_USB_CTRL_PHY2_RST | CCM_USB_CTRL_PHY2_CLK),
> +	},
> +	{
> +		.gpio_vbus = CONFIG_USB3_VBUS_PIN,
> +		.gpio_vbus_det = NULL,
> +		.gpio_id_det = NULL,
> +		.rst_mask = (CCM_USB_CTRL_PHY3_RST | CCM_USB_CTRL_PHY3_CLK),
> +	},
> +};
> +
> +struct sun4i_usb_phy_plat {
> +	void __iomem *pmu;
> +	int power_on_count;
> +	int gpio_vbus;
> +	int gpio_vbus_det;
> +	int gpio_id_det;
> +	int rst_mask;
> +	int id;
> +};
> +
> +struct sun4i_usb_phy_data {
> +	void __iomem *base;
> +	struct sunxi_ccm_reg *ccm;
> +	const struct sun4i_usb_phy_cfg *cfg;
> +	struct sun4i_usb_phy_plat *usb_phy;
> +};
> +
> +static int initial_usb_scan_delay = CONFIG_INITIAL_USB_SCAN_DELAY;

Get rid of any static variables in this driver. And the delay should
come from DT .

> +static void sun4i_usb_phy_write(struct phy *phy, u32 addr, u32 data, int len)
> +{
> +	struct sun4i_usb_phy_data *phy_data = dev_get_priv(phy->dev);
> +	struct sun4i_usb_phy_plat *usb_phy = &phy_data->usb_phy[phy->id];
> +	u32 temp, usbc_bit = BIT(usb_phy->id * 2);
> +	void __iomem *phyctl = phy_data->base + phy_data->cfg->phyctl_offset;
> +	int i;
> +
> +	if (phy_data->cfg->phyctl_offset == REG_PHYCTL_A33) {

You should probably match on some type of the block here, not register
offset, to identify the SoC .

> +		/* SoCs newer than A33 need us to set phyctl to 0 explicitly */
> +		writel(0, phyctl);
> +	}
> +
> +	for (i = 0; i < len; i++) {
> +		temp = readl(phyctl);
> +
> +		/* clear the address portion */
> +		temp &= ~(0xff << 8);
> +
> +		/* set the address */
> +		temp |= ((addr + i) << 8);
> +		writel(temp, phyctl);

clrsetbits(), fix globally

> +		/* set the data bit and clear usbc bit*/
> +		temp = readb(phyctl);
> +		if (data & 0x1)
> +			temp |= PHYCTL_DATA;
> +		else
> +			temp &= ~PHYCTL_DATA;
> +		temp &= ~usbc_bit;
> +		writeb(temp, phyctl);
> +
> +		/* pulse usbc_bit */
> +		temp = readb(phyctl);
> +		temp |= usbc_bit;
> +		writeb(temp, phyctl);
> +
> +		temp = readb(phyctl);
> +		temp &= ~usbc_bit;
> +		writeb(temp, phyctl);
> +
> +		data >>= 1;
> +	}
> +}

[...]
-- 
Best regards,
Marek Vasut