All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andre Przywara <andre.przywara@arm.com>
To: Samuel Holland <samuel@sholland.org>
Cc: Icenowy Zheng <uwu@icenowy.me>, Chen-Yu Tsai <wens@csie.org>,
	Jernej Skrabec <jernej.skrabec@gmail.com>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	soc@kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-sunxi@lists.linux.dev, linux-phy@lists.infradead.org,
	linux-usb@vger.kernel.org, Kishon Vijay Abraham I <kishon@ti.com>,
	Vinod Koul <vkoul@kernel.org>
Subject: Re: [PATCH v3 10/11] phy: sun4i-usb: Replace types with explicit quirk flags
Date: Mon, 14 Nov 2022 00:20:25 +0000	[thread overview]
Message-ID: <20221114002025.351d3084@slackpad.lan> (raw)
In-Reply-To: <7b2606df-3ae5-2699-66bd-12815ffc785b@sholland.org>

On Sun, 13 Nov 2022 17:52:33 -0600
Samuel Holland <samuel@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@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@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
> 
> 


WARNING: multiple messages have this Message-ID (diff)
From: Andre Przywara <andre.przywara@arm.com>
To: Samuel Holland <samuel@sholland.org>
Cc: Icenowy Zheng <uwu@icenowy.me>, Chen-Yu Tsai <wens@csie.org>,
	Jernej Skrabec <jernej.skrabec@gmail.com>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	soc@kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-sunxi@lists.linux.dev, linux-phy@lists.infradead.org,
	linux-usb@vger.kernel.org, Kishon Vijay Abraham I <kishon@ti.com>,
	Vinod Koul <vkoul@kernel.org>
Subject: Re: [PATCH v3 10/11] phy: sun4i-usb: Replace types with explicit quirk flags
Date: Mon, 14 Nov 2022 00:20:25 +0000	[thread overview]
Message-ID: <20221114002025.351d3084@slackpad.lan> (raw)
In-Reply-To: <7b2606df-3ae5-2699-66bd-12815ffc785b@sholland.org>

On Sun, 13 Nov 2022 17:52:33 -0600
Samuel Holland <samuel@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@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@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
> 
> 


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

WARNING: multiple messages have this Message-ID (diff)
From: Andre Przywara <andre.przywara@arm.com>
To: Samuel Holland <samuel@sholland.org>
Cc: Icenowy Zheng <uwu@icenowy.me>, Chen-Yu Tsai <wens@csie.org>,
	Jernej Skrabec <jernej.skrabec@gmail.com>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	soc@kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-sunxi@lists.linux.dev, linux-phy@lists.infradead.org,
	linux-usb@vger.kernel.org, Kishon Vijay Abraham I <kishon@ti.com>,
	Vinod Koul <vkoul@kernel.org>
Subject: Re: [PATCH v3 10/11] phy: sun4i-usb: Replace types with explicit quirk flags
Date: Mon, 14 Nov 2022 00:20:25 +0000	[thread overview]
Message-ID: <20221114002025.351d3084@slackpad.lan> (raw)
In-Reply-To: <7b2606df-3ae5-2699-66bd-12815ffc785b@sholland.org>

On Sun, 13 Nov 2022 17:52:33 -0600
Samuel Holland <samuel@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@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@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
> 
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2022-11-14  0:21 UTC|newest]

Thread overview: 120+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-06 15:48 [PATCH v3 00/11] ARM: suniv: USB and PopStick board support Andre Przywara
2022-11-06 15:48 ` Andre Przywara
2022-11-06 15:48 ` Andre Przywara
2022-11-06 15:48 ` [PATCH v3 01/11] dt-bindings: phy: add binding document for Allwinner F1C100s USB PHY Andre Przywara
2022-11-06 15:48   ` Andre Przywara
2022-11-06 15:48   ` Andre Przywara
2022-11-13 22:32   ` Samuel Holland
2022-11-13 22:32     ` Samuel Holland
2022-11-13 22:32     ` Samuel Holland
2022-11-06 15:48 ` [PATCH v3 02/11] dt-bindings: usb: sunxi-musb: add F1C100s MUSB compatible string Andre Przywara
2022-11-06 15:48   ` Andre Przywara
2022-11-06 15:48   ` Andre Przywara
2022-11-13 22:34   ` Samuel Holland
2022-11-13 22:34     ` Samuel Holland
2022-11-13 22:34     ` Samuel Holland
2022-11-06 15:48 ` [PATCH v3 03/11] phy: sun4i-usb: add support for the USB PHY on F1C100s SoC Andre Przywara
2022-11-06 15:48   ` Andre Przywara
2022-11-06 15:48   ` Andre Przywara
2022-11-07 17:18   ` Jernej Škrabec
2022-11-07 17:18     ` Jernej Škrabec
2022-11-07 17:18     ` Jernej Škrabec
2022-11-10  7:35   ` Vinod Koul
2022-11-10  7:35     ` Vinod Koul
2022-11-10  7:35     ` Vinod Koul
2022-11-15  6:01     ` Jernej Škrabec
2022-11-15  6:01       ` Jernej Škrabec
2022-11-15  6:01       ` Jernej Škrabec
2022-11-15 10:03       ` Krzysztof Kozlowski
2022-11-15 10:03         ` Krzysztof Kozlowski
2022-11-15 10:03         ` Krzysztof Kozlowski
2022-11-15 10:44         ` Andre Przywara
2022-11-15 10:44           ` Andre Przywara
2022-11-15 10:44           ` Andre Przywara
2022-11-15 15:00           ` Krzysztof Kozlowski
2022-11-15 15:00             ` Krzysztof Kozlowski
2022-11-15 15:00             ` Krzysztof Kozlowski
2022-11-15 16:19             ` Andre Przywara
2022-11-15 16:19               ` Andre Przywara
2022-11-15 16:19               ` Andre Przywara
2022-11-15 16:29               ` Krzysztof Kozlowski
2022-11-15 16:29                 ` Krzysztof Kozlowski
2022-11-15 16:29                 ` Krzysztof Kozlowski
2022-11-15 17:57                 ` Andre Przywara
2022-11-15 17:57                   ` Andre Przywara
2022-11-15 17:57                   ` Andre Przywara
2022-11-24 17:49                   ` Vinod Koul
2022-11-24 17:49                     ` Vinod Koul
2022-11-24 17:49                     ` Vinod Koul
2022-11-24 22:13                     ` Andre Przywara
2022-11-24 22:13                       ` Andre Przywara
2022-11-24 22:13                       ` Andre Przywara
2022-11-06 15:48 ` [PATCH v3 04/11] musb: sunxi: add support for the F1C100s MUSB controller Andre Przywara
2022-11-06 15:48   ` Andre Przywara
2022-11-06 15:48   ` Andre Przywara
2022-11-06 15:48 ` [PATCH v3 05/11] ARM: dts: suniv: add USB-related device nodes Andre Przywara
2022-11-06 15:48   ` Andre Przywara
2022-11-06 15:48   ` Andre Przywara
2022-11-07 17:19   ` Jernej Škrabec
2022-11-07 17:19     ` Jernej Škrabec
2022-11-07 17:19     ` Jernej Škrabec
2022-11-06 15:48 ` [PATCH v3 06/11] ARM: dts: suniv: licheepi-nano: enable USB Andre Przywara
2022-11-06 15:48   ` Andre Przywara
2022-11-06 15:48   ` Andre Przywara
2022-11-07 17:19   ` Jernej Škrabec
2022-11-07 17:19     ` Jernej Škrabec
2022-11-07 17:19     ` Jernej Škrabec
2022-11-06 15:48 ` [PATCH v3 07/11] dt-bindings: vendor-prefixes: add Source Parts Andre Przywara
2022-11-06 15:48   ` Andre Przywara
2022-11-06 15:48   ` Andre Przywara
2022-11-06 15:48 ` [PATCH v3 08/11] dt-binding: arm: sunxi: add compatible strings for PopStick v1.1 Andre Przywara
2022-11-06 15:48   ` Andre Przywara
2022-11-06 15:48   ` Andre Przywara
2022-11-06 15:48 ` [PATCH v3 09/11] ARM: dts: suniv: add device tree " Andre Przywara
2022-11-06 15:48   ` Andre Przywara
2022-11-06 15:48   ` Andre Przywara
2022-11-07 17:35   ` Jernej Škrabec
2022-11-07 17:35     ` Jernej Škrabec
2022-11-07 17:35     ` Jernej Škrabec
2022-11-15 16:47     ` Andre Przywara
2022-11-15 16:47       ` Andre Przywara
2022-11-15 16:47       ` Andre Przywara
2022-11-13 22:41   ` Samuel Holland
2022-11-13 22:41     ` Samuel Holland
2022-11-13 22:41     ` Samuel Holland
2022-11-14  0:17     ` Andre Przywara
2022-11-14  0:17       ` Andre Przywara
2022-11-14  0:17       ` Andre Przywara
2022-11-14  0:41       ` Samuel Holland
2022-11-14  0:41         ` Samuel Holland
2022-11-14  0:41         ` Samuel Holland
2022-11-06 15:48 ` [PATCH v3 10/11] phy: sun4i-usb: Replace types with explicit quirk flags Andre Przywara
2022-11-06 15:48   ` Andre Przywara
2022-11-06 15:48   ` Andre Przywara
2022-11-06 15:54   ` Icenowy Zheng
2022-11-06 15:54     ` Icenowy Zheng
2022-11-06 15:54     ` Icenowy Zheng
2022-11-10  7:34     ` Vinod Koul
2022-11-10  7:34       ` Vinod Koul
2022-11-10  7:34       ` Vinod Koul
2022-11-10 11:40       ` Icenowy Zheng
2022-11-10 11:40         ` Icenowy Zheng
2022-11-10 11:40         ` Icenowy Zheng
2022-11-10 12:07         ` Andre Przywara
2022-11-10 12:07           ` Andre Przywara
2022-11-10 12:07           ` Andre Przywara
2022-11-13 23:52     ` Samuel Holland
2022-11-13 23:52       ` Samuel Holland
2022-11-13 23:52       ` Samuel Holland
2022-11-14  0:20       ` Andre Przywara [this message]
2022-11-14  0:20         ` Andre Przywara
2022-11-14  0:20         ` Andre Przywara
2022-11-07 17:44   ` Jernej Škrabec
2022-11-07 17:44     ` Jernej Škrabec
2022-11-07 17:44     ` Jernej Škrabec
2022-11-06 15:48 ` [PATCH v3 11/11] usb: musb: sunxi: Introduce config struct Andre Przywara
2022-11-06 15:48   ` Andre Przywara
2022-11-06 15:48   ` Andre Przywara
2022-11-07 17:56   ` Jernej Škrabec
2022-11-07 17:56     ` Jernej Škrabec
2022-11-07 17:56     ` Jernej Škrabec

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221114002025.351d3084@slackpad.lan \
    --to=andre.przywara@arm.com \
    --cc=devicetree@vger.kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jernej.skrabec@gmail.com \
    --cc=kishon@ti.com \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-phy@lists.infradead.org \
    --cc=linux-sunxi@lists.linux.dev \
    --cc=linux-usb@vger.kernel.org \
    --cc=robh+dt@kernel.org \
    --cc=samuel@sholland.org \
    --cc=soc@kernel.org \
    --cc=uwu@icenowy.me \
    --cc=vkoul@kernel.org \
    --cc=wens@csie.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.