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

Andre Przywara andre.przywara at arm.com
Sun Nov 13 16:20:25 PST 2022


On Sun, 13 Nov 2022 17:52:33 -0600
Samuel Holland <samuel at sholland.org> wrote:

> On 11/6/22 09:54, Icenowy Zheng wrote:
> > 
> > 
> > 于 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.  
> 
> There is already a `hsic_index` variable in the struct we can use.

Ha, indeed, good find! And we are already checking for the same
condition (is this the HSIC port?) using this variable. Saves one more
member in the struct.

Thanks!
Andre


> 
> Regards,
> Samuel
> 
> 




More information about the linux-arm-kernel mailing list