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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, 13 Nov 2022 17:52:33 -0600
Samuel Holland <samuel@xxxxxxxxxxxx> wrote:

> On 11/6/22 09:54, Icenowy Zheng wrote:
> > 
> > 
> > 于 2022年11月6日 GMT+08:00 下午11:48:25, Andre Przywara <andre.przywara@xxxxxxx> 写到:  
> >> 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@xxxxxxx>
> >> ---
> >> 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
> 
> 




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux