Re: [PATCH v3 10/11] phy: sun4i-usb: Replace types with explicit quirk flags

Icenowy Zheng uwu at icenowy.me
Sun Nov 6 07:54:46 PST 2022



于 2022年11月6日 GMT+08:00 下午11:48:25, Andre Przywara <andre.przywara at arm.com> 写到:
>So far we were assigning some crude "type" (SoC name, really) to each
>Allwinner USB PHY model, then guarding certain quirks based on this.
>This does not only look weird, but gets more or more cumbersome to
>maintain.
>
>Remove the bogus type names altogether, instead introduce flags for each
>quirk, and explicitly check for them.
>This improves readability, and simplifies future extensions.
>
>Signed-off-by: Andre Przywara <andre.przywara at arm.com>
>---
> drivers/phy/allwinner/phy-sun4i-usb.c | 50 ++++++++-------------------
> 1 file changed, 15 insertions(+), 35 deletions(-)
>
>diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c
>index 51fb24c6dcb3..422129c66282 100644
>--- a/drivers/phy/allwinner/phy-sun4i-usb.c
>+++ b/drivers/phy/allwinner/phy-sun4i-usb.c
>@@ -99,27 +99,17 @@
> #define DEBOUNCE_TIME			msecs_to_jiffies(50)
> #define POLL_TIME			msecs_to_jiffies(250)
> 
>-enum sun4i_usb_phy_type {
>-	sun4i_a10_phy,
>-	sun6i_a31_phy,
>-	sun8i_a33_phy,
>-	sun8i_a83t_phy,
>-	sun8i_h3_phy,
>-	sun8i_r40_phy,
>-	sun8i_v3s_phy,
>-	sun50i_a64_phy,
>-	sun50i_h6_phy,
>-};
>-
> struct sun4i_usb_phy_cfg {
> 	int num_phys;
> 	int hsic_index;
>-	enum sun4i_usb_phy_type type;
> 	u32 disc_thresh;
> 	u32 hci_phy_ctl_clear;
> 	u8 phyctl_offset;
> 	bool dedicated_clocks;
> 	bool phy0_dual_route;
>+	bool phy2_is_hsic;

Maybe use a `int hsic_phy` instead? But the problem is this practice is
assuming USB0 could not be HSIC -- although USB0 is usually OTG.

>+	bool siddq_in_base;
>+	bool poll_vbusen;
> 	int missing_phys;
> };
> 
>@@ -251,7 +241,7 @@ static void sun4i_usb_phy_passby(struct sun4i_usb_phy *phy, int enable)
> 		SUNXI_AHB_INCRX_ALIGN_EN | SUNXI_ULPI_BYPASS_EN;
> 
> 	/* A83T USB2 is HSIC */
>-	if (phy_data->cfg->type == sun8i_a83t_phy && phy->index == 2)
>+	if (phy_data->cfg->phy2_is_hsic && phy->index == 2)
> 		bits |= SUNXI_EHCI_HS_FORCE | SUNXI_HSIC_CONNECT_INT |
> 			SUNXI_HSIC;
> 
>@@ -295,8 +285,7 @@ static int sun4i_usb_phy_init(struct phy *_phy)
> 		writel(val, phy->pmu + REG_HCI_PHY_CTL);
> 	}
> 
>-	if (data->cfg->type == sun8i_a83t_phy ||
>-	    data->cfg->type == sun50i_h6_phy) {
>+	if (data->cfg->siddq_in_base) {
> 		if (phy->index == 0) {
> 			val = readl(data->base + data->cfg->phyctl_offset);
> 			val |= PHY_CTL_VBUSVLDEXT;
>@@ -340,8 +329,7 @@ static int sun4i_usb_phy_exit(struct phy *_phy)
> 	struct sun4i_usb_phy_data *data = to_sun4i_usb_phy_data(phy);
> 
> 	if (phy->index == 0) {
>-		if (data->cfg->type == sun8i_a83t_phy ||
>-		    data->cfg->type == sun50i_h6_phy) {
>+		if (data->cfg->siddq_in_base) {
> 			void __iomem *phyctl = data->base +
> 				data->cfg->phyctl_offset;
> 
>@@ -414,9 +402,8 @@ static bool sun4i_usb_phy0_poll(struct sun4i_usb_phy_data *data)
> 	 * vbus using the N_VBUSEN pin on the pmic, so we must poll
> 	 * when using the pmic for vbus-det _and_ we're driving vbus.
> 	 */
>-	if ((data->cfg->type == sun6i_a31_phy ||
>-	     data->cfg->type == sun8i_a33_phy) &&
>-	    data->vbus_power_supply && data->phys[0].regulator_on)
>+	if (data->cfg->poll_vbusen && data->vbus_power_supply &&
>+	    data->phys[0].regulator_on)
> 		return true;
> 
> 	return false;
>@@ -861,7 +848,6 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
> 
> static const struct sun4i_usb_phy_cfg suniv_f1c100s_cfg = {
> 	.num_phys = 1,
>-	.type = sun4i_a10_phy,
> 	.disc_thresh = 3,
> 	.phyctl_offset = REG_PHYCTL_A10,
> 	.dedicated_clocks = true,
>@@ -869,7 +855,6 @@ static const struct sun4i_usb_phy_cfg suniv_f1c100s_cfg = {
> 
> static const struct sun4i_usb_phy_cfg sun4i_a10_cfg = {
> 	.num_phys = 3,
>-	.type = sun4i_a10_phy,
> 	.disc_thresh = 3,
> 	.phyctl_offset = REG_PHYCTL_A10,
> 	.dedicated_clocks = false,
>@@ -877,7 +862,6 @@ static const struct sun4i_usb_phy_cfg sun4i_a10_cfg = {
> 
> static const struct sun4i_usb_phy_cfg sun5i_a13_cfg = {
> 	.num_phys = 2,
>-	.type = sun4i_a10_phy,
> 	.disc_thresh = 2,
> 	.phyctl_offset = REG_PHYCTL_A10,
> 	.dedicated_clocks = false,
>@@ -885,15 +869,14 @@ static const struct sun4i_usb_phy_cfg sun5i_a13_cfg = {
> 
> static const struct sun4i_usb_phy_cfg sun6i_a31_cfg = {
> 	.num_phys = 3,
>-	.type = sun6i_a31_phy,
> 	.disc_thresh = 3,
> 	.phyctl_offset = REG_PHYCTL_A10,
> 	.dedicated_clocks = true,
>+	.poll_vbusen = true,
> };
> 
> static const struct sun4i_usb_phy_cfg sun7i_a20_cfg = {
> 	.num_phys = 3,
>-	.type = sun4i_a10_phy,
> 	.disc_thresh = 2,
> 	.phyctl_offset = REG_PHYCTL_A10,
> 	.dedicated_clocks = false,
>@@ -901,31 +884,31 @@ static const struct sun4i_usb_phy_cfg sun7i_a20_cfg = {
> 
> static const struct sun4i_usb_phy_cfg sun8i_a23_cfg = {
> 	.num_phys = 2,
>-	.type = sun6i_a31_phy,
> 	.disc_thresh = 3,
> 	.phyctl_offset = REG_PHYCTL_A10,
> 	.dedicated_clocks = true,
>+	.poll_vbusen = true,
> };
> 
> static const struct sun4i_usb_phy_cfg sun8i_a33_cfg = {
> 	.num_phys = 2,
>-	.type = sun8i_a33_phy,
> 	.disc_thresh = 3,
> 	.phyctl_offset = REG_PHYCTL_A33,
> 	.dedicated_clocks = true,
>+	.poll_vbusen = true,
> };
> 
> static const struct sun4i_usb_phy_cfg sun8i_a83t_cfg = {
> 	.num_phys = 3,
> 	.hsic_index = 2,
>-	.type = sun8i_a83t_phy,
> 	.phyctl_offset = REG_PHYCTL_A33,
> 	.dedicated_clocks = true,
>+	.siddq_in_base = true,
>+	.phy2_is_hsic = true,
> };
> 
> static const struct sun4i_usb_phy_cfg sun8i_h3_cfg = {
> 	.num_phys = 4,
>-	.type = sun8i_h3_phy,
> 	.disc_thresh = 3,
> 	.phyctl_offset = REG_PHYCTL_A33,
> 	.dedicated_clocks = true,
>@@ -935,7 +918,6 @@ static const struct sun4i_usb_phy_cfg sun8i_h3_cfg = {
> 
> static const struct sun4i_usb_phy_cfg sun8i_r40_cfg = {
> 	.num_phys = 3,
>-	.type = sun8i_r40_phy,
> 	.disc_thresh = 3,
> 	.phyctl_offset = REG_PHYCTL_A33,
> 	.dedicated_clocks = true,
>@@ -945,7 +927,6 @@ static const struct sun4i_usb_phy_cfg sun8i_r40_cfg = {
> 
> static const struct sun4i_usb_phy_cfg sun8i_v3s_cfg = {
> 	.num_phys = 1,
>-	.type = sun8i_v3s_phy,
> 	.disc_thresh = 3,
> 	.phyctl_offset = REG_PHYCTL_A33,
> 	.dedicated_clocks = true,
>@@ -955,16 +936,15 @@ static const struct sun4i_usb_phy_cfg sun8i_v3s_cfg = {
> 
> static const struct sun4i_usb_phy_cfg sun20i_d1_cfg = {
> 	.num_phys = 2,
>-	.type = sun50i_h6_phy,
> 	.phyctl_offset = REG_PHYCTL_A33,
> 	.dedicated_clocks = true,
> 	.hci_phy_ctl_clear = PHY_CTL_SIDDQ,
> 	.phy0_dual_route = true,
>+	.siddq_in_base = true,
> };
> 
> static const struct sun4i_usb_phy_cfg sun50i_a64_cfg = {
> 	.num_phys = 2,
>-	.type = sun50i_a64_phy,
> 	.disc_thresh = 3,
> 	.phyctl_offset = REG_PHYCTL_A33,
> 	.dedicated_clocks = true,
>@@ -974,11 +954,11 @@ static const struct sun4i_usb_phy_cfg sun50i_a64_cfg = {
> 
> static const struct sun4i_usb_phy_cfg sun50i_h6_cfg = {
> 	.num_phys = 4,
>-	.type = sun50i_h6_phy,
> 	.phyctl_offset = REG_PHYCTL_A33,
> 	.dedicated_clocks = true,
> 	.phy0_dual_route = true,
> 	.missing_phys = BIT(1) | BIT(2),
>+	.siddq_in_base = true,
> };
> 
> static const struct of_device_id sun4i_usb_phy_of_match[] = {



More information about the linux-arm-kernel mailing list