[PATCH v3 00/11] ARM: suniv: USB and PopStick board support

[PATCH v3 00/11] ARM: suniv: USB and PopStick board support

[Andre Przywara]

This is an update to Icenowy's v2 of the series, fixing the smaller
issues mentioned during the review. Also it adds two patches that
improve the quirk handling for the sunxi MUSB and USB PHY driver, as it
was hinted in some replies. I put those patches at the end, to not
jeopardy the actual USB functionality of the Allwinner F1C100s series.
For a changelog see below.
================
This patchset introduces support for F1C100s' USB and SourceParts
PopStick board.

The DT binding and driver support for SUNIV USB PHY/MUSB are added, in
addition to DT changes to the DTSI and Lichee Nano DT. A new DT is added
for SourceParts PopStick v1.1 board.

Changelog v1 ... v2:
- USB PHY binding: clarify the relation with other phy-sun4i-usb bindings
- Add Popstick binding and .dts patches

Changelog v2 ... v3:
- remove redundant "Device Tree Bindings" suffix in DT binding doc title
- add BSD license to binding doc file (as per checkpatch)
- use existing PHY sun4i_a10_phy type instead of inventing new one
- fix some commit message title prefixes
- use proper plural spelling for usb0_id_det-gpios
- popstick.dts: Reorder otg_sram node reference alphabetically
- popstick.dts: Add regulator- prefix to 3.3V regulator node name
- popstick.dts: Fix status, compatible and reg property order
- popstick.dts: Drop unneeded mmc0 and spi0 aliases
- add two patches to clean up sunxi MUSB and USB PHY driver
- add Acks and Reviewed-by's


Andre Przywara (2):
  phy: sun4i-usb: Replace types with explicit quirk flags
  usb: musb: sunxi: Introduce config struct

Icenowy Zheng (9):
  dt-bindings: phy: add binding document for Allwinner F1C100s USB PHY
  dt-bindings: usb: sunxi-musb: add F1C100s MUSB compatible string
  phy: sun4i-usb: add support for the USB PHY on F1C100s SoC
  musb: sunxi: add support for the F1C100s MUSB controller
  ARM: dts: suniv: add USB-related device nodes
  ARM: dts: suniv: licheepi-nano: enable USB
  dt-bindings: vendor-prefixes: add Source Parts
  dt-binding: arm: sunxi: add compatible strings for PopStick v1.1
  ARM: dts: suniv: add device tree for PopStick v1.1

 .../devicetree/bindings/arm/sunxi.yaml        |  7 ++
 .../phy/allwinner,suniv-f1c100s-usb-phy.yaml  | 83 ++++++++++++++++
 .../usb/allwinner,sun4i-a10-musb.yaml         |  1 +
 .../devicetree/bindings/vendor-prefixes.yaml  |  2 +
 arch/arm/boot/dts/Makefile                    |  3 +-
 .../boot/dts/suniv-f1c100s-licheepi-nano.dts  | 16 +++
 arch/arm/boot/dts/suniv-f1c100s.dtsi          | 26 +++++
 .../boot/dts/suniv-f1c200s-popstick-v1.1.dts  | 99 +++++++++++++++++++
 drivers/phy/allwinner/phy-sun4i-usb.c         | 58 +++++------
 drivers/usb/musb/sunxi.c                      | 97 +++++++++++++-----
 10 files changed, 335 insertions(+), 57 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/phy/allwinner,suniv-f1c100s-usb-phy.yaml
 create mode 100644 arch/arm/boot/dts/suniv-f1c200s-popstick-v1.1.dts

-- 
2.35.5

[PATCH v3 01/11] dt-bindings: phy: add binding document for Allwinner F1C100s USB PHY

[Andre Przywara]

From: Icenowy Zheng <uwu@icenowy.me>

Allwinner F1C100s has the most simple USB PHY among all Allwinner SoCs,
because it has only one OTG USB controller, no host-only OHCI/EHCI
controllers.

Add a binding document for it. Following the current situation of one
YAML file per SoC, this one is based on
allwinner,sun8i-v3s-usb-phy.yaml, but with OHCI/EHCI-related bits
removed. (The same driver in Linux, phy-sun4i-usb, covers all these
binding files now.)

Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 .../phy/allwinner,suniv-f1c100s-usb-phy.yaml  | 83 +++++++++++++++++++
 1 file changed, 83 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/allwinner,suniv-f1c100s-usb-phy.yaml

diff --git a/Documentation/devicetree/bindings/phy/allwinner,suniv-f1c100s-usb-phy.yaml b/Documentation/devicetree/bindings/phy/allwinner,suniv-f1c100s-usb-phy.yaml
new file mode 100644
index 000000000000..948839499235
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/allwinner,suniv-f1c100s-usb-phy.yaml
@@ -0,0 +1,83 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/allwinner,suniv-f1c100s-usb-phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Allwinner F1C100s USB PHY
+
+maintainers:
+  - Chen-Yu Tsai <wens@csie.org>
+  - Maxime Ripard <mripard@kernel.org>
+
+properties:
+  "#phy-cells":
+    const: 1
+
+  compatible:
+    const: allwinner,suniv-f1c100s-usb-phy
+
+  reg:
+    maxItems: 1
+    description: PHY Control registers
+
+  reg-names:
+    const: phy_ctrl
+
+  clocks:
+    maxItems: 1
+    description: USB OTG PHY bus clock
+
+  clock-names:
+    const: usb0_phy
+
+  resets:
+    maxItems: 1
+    description: USB OTG reset
+
+  reset-names:
+    const: usb0_reset
+
+  usb0_id_det-gpios:
+    maxItems: 1
+    description: GPIO to the USB OTG ID pin
+
+  usb0_vbus_det-gpios:
+    maxItems: 1
+    description: GPIO to the USB OTG VBUS detect pin
+
+  usb0_vbus_power-supply:
+    description: Power supply to detect the USB OTG VBUS
+
+  usb0_vbus-supply:
+    description: Regulator controlling USB OTG VBUS
+
+required:
+  - "#phy-cells"
+  - compatible
+  - clocks
+  - clock-names
+  - reg
+  - reg-names
+  - resets
+  - reset-names
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/clock/suniv-ccu-f1c100s.h>
+    #include <dt-bindings/reset/suniv-ccu-f1c100s.h>
+
+    phy@1c13400 {
+        compatible = "allwinner,suniv-f1c100s-usb-phy";
+        reg = <0x01c13400 0x10>;
+        reg-names = "phy_ctrl";
+        clocks = <&ccu CLK_USB_PHY0>;
+        clock-names = "usb0_phy";
+        resets = <&ccu RST_USB_PHY0>;
+        reset-names = "usb0_reset";
+        #phy-cells = <1>;
+        usb0_id_det-gpios = <&pio 4 2 GPIO_ACTIVE_HIGH>;
+    };
-- 
2.35.5

Re: [PATCH v3 01/11] dt-bindings: phy: add binding document for Allwinner F1C100s USB PHY

[Samuel Holland]

On 11/6/22 09:48, Andre Przywara wrote:
> From: Icenowy Zheng <uwu@icenowy.me>
> 
> Allwinner F1C100s has the most simple USB PHY among all Allwinner SoCs,
> because it has only one OTG USB controller, no host-only OHCI/EHCI
> controllers.
> 
> Add a binding document for it. Following the current situation of one
> YAML file per SoC, this one is based on
> allwinner,sun8i-v3s-usb-phy.yaml, but with OHCI/EHCI-related bits
> removed. (The same driver in Linux, phy-sun4i-usb, covers all these
> binding files now.)
> 
> Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Reviewed-by: Andre Przywara <andre.przywara@arm.com>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  .../phy/allwinner,suniv-f1c100s-usb-phy.yaml  | 83 +++++++++++++++++++
>  1 file changed, 83 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/allwinner,suniv-f1c100s-usb-phy.yaml

Reviewed-by: Samuel Holland <samuel@sholland.org>

[PATCH v3 02/11] dt-bindings: usb: sunxi-musb: add F1C100s MUSB compatible string

[Andre Przywara]

From: Icenowy Zheng <uwu@icenowy.me>

Allwinner F1C100s has a hybrid MUSB controller between the A10 one and
the A33 one.

Add a compatible string for it.

Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 .../devicetree/bindings/usb/allwinner,sun4i-a10-musb.yaml        | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/usb/allwinner,sun4i-a10-musb.yaml b/Documentation/devicetree/bindings/usb/allwinner,sun4i-a10-musb.yaml
index 8992eff6ce38..9ae634280bf4 100644
--- a/Documentation/devicetree/bindings/usb/allwinner,sun4i-a10-musb.yaml
+++ b/Documentation/devicetree/bindings/usb/allwinner,sun4i-a10-musb.yaml
@@ -17,6 +17,7 @@ properties:
       - const: allwinner,sun6i-a31-musb
       - const: allwinner,sun8i-a33-musb
       - const: allwinner,sun8i-h3-musb
+      - const: allwinner,suniv-f1c100s-musb
       - items:
           - enum:
               - allwinner,sun8i-a83t-musb
-- 
2.35.5

Re: [PATCH v3 02/11] dt-bindings: usb: sunxi-musb: add F1C100s MUSB compatible string

[Samuel Holland]

On 11/6/22 09:48, Andre Przywara wrote:
> From: Icenowy Zheng <uwu@icenowy.me>
> 
> Allwinner F1C100s has a hybrid MUSB controller between the A10 one and
> the A33 one.
> 
> Add a compatible string for it.
> 
> Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  .../devicetree/bindings/usb/allwinner,sun4i-a10-musb.yaml        | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/allwinner,sun4i-a10-musb.yaml b/Documentation/devicetree/bindings/usb/allwinner,sun4i-a10-musb.yaml
> index 8992eff6ce38..9ae634280bf4 100644
> --- a/Documentation/devicetree/bindings/usb/allwinner,sun4i-a10-musb.yaml
> +++ b/Documentation/devicetree/bindings/usb/allwinner,sun4i-a10-musb.yaml
> @@ -17,6 +17,7 @@ properties:
>        - const: allwinner,sun6i-a31-musb
>        - const: allwinner,sun8i-a33-musb
>        - const: allwinner,sun8i-h3-musb
> +      - const: allwinner,suniv-f1c100s-musb

This would be a good place to use an enum. Either way:

Reviewed-by: Samuel Holland <samuel@sholland.org>

>        - items:
>            - enum:
>                - allwinner,sun8i-a83t-musb

[PATCH v3 03/11] phy: sun4i-usb: add support for the USB PHY on F1C100s SoC

[Andre Przywara]

From: Icenowy Zheng <uwu@icenowy.me>

The F1C100s SoC has one USB OTG port connected to a MUSB controller.

Add support for its USB PHY.

Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/phy/allwinner/phy-sun4i-usb.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c
index 3a3831f6059a..51fb24c6dcb3 100644
--- a/drivers/phy/allwinner/phy-sun4i-usb.c
+++ b/drivers/phy/allwinner/phy-sun4i-usb.c
@@ -859,6 +859,14 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
 	return 0;
 }
 
+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,
+};
+
 static const struct sun4i_usb_phy_cfg sun4i_a10_cfg = {
 	.num_phys = 3,
 	.type = sun4i_a10_phy,
@@ -988,6 +996,8 @@ static const struct of_device_id sun4i_usb_phy_of_match[] = {
 	{ .compatible = "allwinner,sun50i-a64-usb-phy",
 	  .data = &sun50i_a64_cfg},
 	{ .compatible = "allwinner,sun50i-h6-usb-phy", .data = &sun50i_h6_cfg },
+	{ .compatible = "allwinner,suniv-f1c100s-usb-phy",
+	  .data = &suniv_f1c100s_cfg },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, sun4i_usb_phy_of_match);
-- 
2.35.5

Re: [PATCH v3 03/11] phy: sun4i-usb: add support for the USB PHY on F1C100s SoC

[Jernej Škrabec]

Dne nedelja, 06. november 2022 ob 16:48:18 CET je Andre Przywara napisal(a):
> From: Icenowy Zheng <uwu@icenowy.me>
> 
> The F1C100s SoC has one USB OTG port connected to a MUSB controller.
> 
> Add support for its USB PHY.
> 
> Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej

Re: [PATCH v3 03/11] phy: sun4i-usb: add support for the USB PHY on F1C100s SoC

[Jernej Škrabec]

Dne četrtek, 10. november 2022 ob 08:35:39 CET je Vinod Koul napisal(a):
> On 06-11-22, 15:48, Andre Przywara wrote:
> > From: Icenowy Zheng <uwu@icenowy.me>
> > 
> > The F1C100s SoC has one USB OTG port connected to a MUSB controller.
> > 
> > Add support for its USB PHY.
> 
> This does not apply for me, please rebase and resend
> 
> Also, consider splitting phy patches from this. I dont think there is
> any dependency

DT patches in this series depend on functionality added here.

Best regards,
Jernej

Re: [PATCH v3 03/11] phy: sun4i-usb: add support for the USB PHY on F1C100s SoC

[Krzysztof Kozlowski]

On 15/11/2022 07:01, Jernej Škrabec wrote:
> Dne četrtek, 10. november 2022 ob 08:35:39 CET je Vinod Koul napisal(a):
>> On 06-11-22, 15:48, Andre Przywara wrote:
>>> From: Icenowy Zheng <uwu@icenowy.me>
>>>
>>> The F1C100s SoC has one USB OTG port connected to a MUSB controller.
>>>
>>> Add support for its USB PHY.
>>
>> This does not apply for me, please rebase and resend
>>
>> Also, consider splitting phy patches from this. I dont think there is
>> any dependency
> 
> DT patches in this series depend on functionality added here.
> 

DTS always goes separately from driver changes because it is a hardware
description. Depending on driver means you have potential ABI break, so
it is already a warning sign.

Best regards,
Krzysztof

Re: [PATCH v3 03/11] phy: sun4i-usb: add support for the USB PHY on F1C100s SoC

[Andre Przywara]

On Tue, 15 Nov 2022 11:03:24 +0100
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

Hi,

> On 15/11/2022 07:01, Jernej Škrabec wrote:
> > Dne četrtek, 10. november 2022 ob 08:35:39 CET je Vinod Koul napisal(a):  
> >> On 06-11-22, 15:48, Andre Przywara wrote:  
> >>> From: Icenowy Zheng <uwu@icenowy.me>
> >>>
> >>> The F1C100s SoC has one USB OTG port connected to a MUSB controller.
> >>>
> >>> Add support for its USB PHY.  
> >>
> >> This does not apply for me, please rebase and resend
> >>
> >> Also, consider splitting phy patches from this. I dont think there is
> >> any dependency  
> > 
> > DT patches in this series depend on functionality added here.
> >   
> 
> DTS always goes separately from driver changes because it is a hardware
> description. Depending on driver means you have potential ABI break, so
> it is already a warning sign.

We understand that ;-)
What Jernej meant was that the DTS patches at the end depend on patch
01/10, which adds to the PHY binding doc. I am not sure if Vinod's
suggestion was about splitting off 01/10, 03/10, and 10/10, or just the
two latter which touch the driver.

I can split off 03/10 and 10/10, rebased on top of linux-phy.git/next, and
send that to Vinod.
Then I would keep 01/10 in a respin of this series here, to satisfy the
dependency of the later DTS patches, and Vinod can pick that one patch from
there?

Cheers,
Andre

Re: [PATCH v3 03/11] phy: sun4i-usb: add support for the USB PHY on F1C100s SoC

[Krzysztof Kozlowski]

On 15/11/2022 11:44, Andre Przywara wrote:
> On Tue, 15 Nov 2022 11:03:24 +0100
> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> 
> Hi,
> 
>> On 15/11/2022 07:01, Jernej Škrabec wrote:
>>> Dne četrtek, 10. november 2022 ob 08:35:39 CET je Vinod Koul napisal(a):  
>>>> On 06-11-22, 15:48, Andre Przywara wrote:  
>>>>> From: Icenowy Zheng <uwu@icenowy.me>
>>>>>
>>>>> The F1C100s SoC has one USB OTG port connected to a MUSB controller.
>>>>>
>>>>> Add support for its USB PHY.  
>>>>
>>>> This does not apply for me, please rebase and resend
>>>>
>>>> Also, consider splitting phy patches from this. I dont think there is
>>>> any dependency  
>>>
>>> DT patches in this series depend on functionality added here.
>>>   
>>
>> DTS always goes separately from driver changes because it is a hardware
>> description. Depending on driver means you have potential ABI break, so
>> it is already a warning sign.
> 
> We understand that ;-)
> What Jernej meant was that the DTS patches at the end depend on patch
> 01/10, which adds to the PHY binding doc. I am not sure if Vinod's
> suggestion was about splitting off 01/10, 03/10, and 10/10, or just the
> two latter which touch the driver.
> 
> I can split off 03/10 and 10/10, rebased on top of linux-phy.git/next, and
> send that to Vinod.
> Then I would keep 01/10 in a respin of this series here, to satisfy the
> dependency of the later DTS patches, and Vinod can pick that one patch from
> there?

There is no hard dependency of DTS on bindings. You can split these (and
some maintainers prefer that way) and in DTS patches just provide the
link to the bindings, saying it is in progress.

The bindings should be however kept with driver changes as it goes the
same way.

Best regards,
Krzysztof

Re: [PATCH v3 03/11] phy: sun4i-usb: add support for the USB PHY on F1C100s SoC

[Andre Przywara]

On Tue, 15 Nov 2022 16:00:54 +0100
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

Hi,

> On 15/11/2022 11:44, Andre Przywara wrote:
> > On Tue, 15 Nov 2022 11:03:24 +0100
> > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> > 
> > Hi,
> >   
> >> On 15/11/2022 07:01, Jernej Škrabec wrote:  
> >>> Dne četrtek, 10. november 2022 ob 08:35:39 CET je Vinod Koul napisal(a):    
> >>>> On 06-11-22, 15:48, Andre Przywara wrote:    
> >>>>> From: Icenowy Zheng <uwu@icenowy.me>
> >>>>>
> >>>>> The F1C100s SoC has one USB OTG port connected to a MUSB controller.
> >>>>>
> >>>>> Add support for its USB PHY.    
> >>>>
> >>>> This does not apply for me, please rebase and resend
> >>>>
> >>>> Also, consider splitting phy patches from this. I dont think there is
> >>>> any dependency    
> >>>
> >>> DT patches in this series depend on functionality added here.
> >>>     
> >>
> >> DTS always goes separately from driver changes because it is a hardware
> >> description. Depending on driver means you have potential ABI break, so
> >> it is already a warning sign.  
> > 
> > We understand that ;-)
> > What Jernej meant was that the DTS patches at the end depend on patch
> > 01/10, which adds to the PHY binding doc. I am not sure if Vinod's
> > suggestion was about splitting off 01/10, 03/10, and 10/10, or just the
> > two latter which touch the driver.
> > 
> > I can split off 03/10 and 10/10, rebased on top of linux-phy.git/next, and
> > send that to Vinod.
> > Then I would keep 01/10 in a respin of this series here, to satisfy the
> > dependency of the later DTS patches, and Vinod can pick that one patch from
> > there?  
> 
> There is no hard dependency of DTS on bindings. You can split these (and
> some maintainers prefer that way) and in DTS patches just provide the
> link to the bindings, saying it is in progress.

But that breaks "make dtbs_check", doesn't it?

I would think that the DT bits - bindings first, then DTS files using it -
should be bundled. This is how I imagine the future(TM), where DTs and
bindings live outside the kernel repo.

> The bindings should be however kept with driver changes as it goes the
> same way.

I understand that the bindings describe the contract the driver acts on,
but going forward I think driver changes would need to come later, then
(since they will live in a separate repo at some day)?
Maybe pointing to the binding changes in progress?

So with a separate repo we would actually need to upstream just the
bindings first, then could push driver changes and .dts files
independently?

And for now it looks like we are stuck with putting everything in one
series, to make both checkpatch and dtbs_check happy.

Cheers,
Andre

Re: [PATCH v3 03/11] phy: sun4i-usb: add support for the USB PHY on F1C100s SoC

[Krzysztof Kozlowski]

On 15/11/2022 17:19, Andre Przywara wrote:
> On Tue, 15 Nov 2022 16:00:54 +0100
> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> 
> Hi,
> 
>> On 15/11/2022 11:44, Andre Przywara wrote:
>>> On Tue, 15 Nov 2022 11:03:24 +0100
>>> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
>>>
>>> Hi,
>>>   
>>>> On 15/11/2022 07:01, Jernej Škrabec wrote:  
>>>>> Dne četrtek, 10. november 2022 ob 08:35:39 CET je Vinod Koul napisal(a):    
>>>>>> On 06-11-22, 15:48, Andre Przywara wrote:    
>>>>>>> From: Icenowy Zheng <uwu@icenowy.me>
>>>>>>>
>>>>>>> The F1C100s SoC has one USB OTG port connected to a MUSB controller.
>>>>>>>
>>>>>>> Add support for its USB PHY.    
>>>>>>
>>>>>> This does not apply for me, please rebase and resend
>>>>>>
>>>>>> Also, consider splitting phy patches from this. I dont think there is
>>>>>> any dependency    
>>>>>
>>>>> DT patches in this series depend on functionality added here.
>>>>>     
>>>>
>>>> DTS always goes separately from driver changes because it is a hardware
>>>> description. Depending on driver means you have potential ABI break, so
>>>> it is already a warning sign.  
>>>
>>> We understand that ;-)
>>> What Jernej meant was that the DTS patches at the end depend on patch
>>> 01/10, which adds to the PHY binding doc. I am not sure if Vinod's
>>> suggestion was about splitting off 01/10, 03/10, and 10/10, or just the
>>> two latter which touch the driver.
>>>
>>> I can split off 03/10 and 10/10, rebased on top of linux-phy.git/next, and
>>> send that to Vinod.
>>> Then I would keep 01/10 in a respin of this series here, to satisfy the
>>> dependency of the later DTS patches, and Vinod can pick that one patch from
>>> there?  
>>
>> There is no hard dependency of DTS on bindings. You can split these (and
>> some maintainers prefer that way) and in DTS patches just provide the
>> link to the bindings, saying it is in progress.
> 
> But that breaks "make dtbs_check", doesn't it?

The check will be broken anyway because binding goes via driver
subsystem and DTS goes via arm-soc.

If both make to the linux-next and next release, then it's not a problem.

> 
> I would think that the DT bits - bindings first, then DTS files using it -
> should be bundled. This is how I imagine the future(TM), where DTs and
> bindings live outside the kernel repo.

Yes, that's preferred. Therefore in DTS patch you say the binding is not
merged and it is here - lore link.

> 
>> The bindings should be however kept with driver changes as it goes the
>> same way.
> 
> I understand that the bindings describe the contract the driver acts on,
> but going forward I think driver changes would need to come later, then
> (since they will live in a separate repo at some day)?
> Maybe pointing to the binding changes in progress?

Later as one commit later - yes. Later as other option - not really, why?

> So with a separate repo we would actually need to upstream just the
> bindings first, then could push driver changes and .dts files
> independently?

There is no separate repo, so we talk about Linux case now.

> And for now it looks like we are stuck with putting everything in one
> series, to make both checkpatch and dtbs_check happy.

You should rather make maintainers happy :) and here one asked to split.

Best regards,
Krzysztof

Re: [PATCH v3 03/11] phy: sun4i-usb: add support for the USB PHY on F1C100s SoC

[Andre Przywara]

On Tue, 15 Nov 2022 17:29:09 +0100
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

Hi,

> On 15/11/2022 17:19, Andre Przywara wrote:
> > On Tue, 15 Nov 2022 16:00:54 +0100
> > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> > 
> > Hi,
> >   
> >> On 15/11/2022 11:44, Andre Przywara wrote:  
> >>> On Tue, 15 Nov 2022 11:03:24 +0100
> >>> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> >>>
> >>> Hi,
> >>>     
> >>>> On 15/11/2022 07:01, Jernej Škrabec wrote:    
> >>>>> Dne četrtek, 10. november 2022 ob 08:35:39 CET je Vinod Koul napisal(a):      
> >>>>>> On 06-11-22, 15:48, Andre Przywara wrote:      
> >>>>>>> From: Icenowy Zheng <uwu@icenowy.me>
> >>>>>>>
> >>>>>>> The F1C100s SoC has one USB OTG port connected to a MUSB controller.
> >>>>>>>
> >>>>>>> Add support for its USB PHY.      
> >>>>>>
> >>>>>> This does not apply for me, please rebase and resend
> >>>>>>
> >>>>>> Also, consider splitting phy patches from this. I dont think there is
> >>>>>> any dependency      
> >>>>>
> >>>>> DT patches in this series depend on functionality added here.
> >>>>>       
> >>>>
> >>>> DTS always goes separately from driver changes because it is a hardware
> >>>> description. Depending on driver means you have potential ABI break, so
> >>>> it is already a warning sign.    
> >>>
> >>> We understand that ;-)
> >>> What Jernej meant was that the DTS patches at the end depend on patch
> >>> 01/10, which adds to the PHY binding doc. I am not sure if Vinod's
> >>> suggestion was about splitting off 01/10, 03/10, and 10/10, or just the
> >>> two latter which touch the driver.
> >>>
> >>> I can split off 03/10 and 10/10, rebased on top of linux-phy.git/next, and
> >>> send that to Vinod.
> >>> Then I would keep 01/10 in a respin of this series here, to satisfy the
> >>> dependency of the later DTS patches, and Vinod can pick that one patch from
> >>> there?    
> >>
> >> There is no hard dependency of DTS on bindings. You can split these (and
> >> some maintainers prefer that way) and in DTS patches just provide the
> >> link to the bindings, saying it is in progress.  
> > 
> > But that breaks "make dtbs_check", doesn't it?  
> 
> The check will be broken anyway because binding goes via driver
> subsystem and DTS goes via arm-soc.
> 
> If both make to the linux-next and next release, then it's not a problem.
> 
> > 
> > I would think that the DT bits - bindings first, then DTS files using it -
> > should be bundled. This is how I imagine the future(TM), where DTs and
> > bindings live outside the kernel repo.  
> 
> Yes, that's preferred. Therefore in DTS patch you say the binding is not
> merged and it is here - lore link.
> 
> >   
> >> The bindings should be however kept with driver changes as it goes the
> >> same way.  
> > 
> > I understand that the bindings describe the contract the driver acts on,
> > but going forward I think driver changes would need to come later, then
> > (since they will live in a separate repo at some day)?
> > Maybe pointing to the binding changes in progress?  
> 
> Later as one commit later - yes. Later as other option - not really, why?
> 
> > So with a separate repo we would actually need to upstream just the
> > bindings first, then could push driver changes and .dts files
> > independently?  
> 
> There is no separate repo, so we talk about Linux case now.
> 
> > And for now it looks like we are stuck with putting everything in one
> > series, to make both checkpatch and dtbs_check happy.  
> 
> You should rather make maintainers happy :) and here one asked to split.

Well, he asked to split off the USB PHY patches from the rest of the
series, since there is some conflict with the recently merged H616 USB PHY
patches. It is still unclear to me whether this split includes the binding
patch, or just the two patches touching the actual code.

Cheers,
Andre.

Re: [PATCH v3 03/11] phy: sun4i-usb: add support for the USB PHY on F1C100s SoC

[Andre Przywara]

On Thu, 24 Nov 2022 23:19:30 +0530
Vinod Koul <vkoul@kernel.org> wrote:

> On 15-11-22, 17:57, Andre Przywara wrote:
> > On Tue, 15 Nov 2022 17:29:09 +0100
> > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> > 
> > Hi,
> >   
> > > On 15/11/2022 17:19, Andre Przywara wrote:  
> > > > On Tue, 15 Nov 2022 16:00:54 +0100
> > > > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> > > > 
> > > > Hi,
> > > >     
> > > >> On 15/11/2022 11:44, Andre Przywara wrote:    
> > > >>> On Tue, 15 Nov 2022 11:03:24 +0100
> > > >>> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> > > >>>
> > > >>> Hi,
> > > >>>       
> > > >>>> On 15/11/2022 07:01, Jernej Škrabec wrote:      
> > > >>>>> Dne četrtek, 10. november 2022 ob 08:35:39 CET je Vinod Koul napisal(a):        
> > > >>>>>> On 06-11-22, 15:48, Andre Przywara wrote:        
> > > >>>>>>> From: Icenowy Zheng <uwu@icenowy.me>
> > > >>>>>>>
> > > >>>>>>> The F1C100s SoC has one USB OTG port connected to a MUSB controller.
> > > >>>>>>>
> > > >>>>>>> Add support for its USB PHY.        
> > > >>>>>>
> > > >>>>>> This does not apply for me, please rebase and resend
> > > >>>>>>
> > > >>>>>> Also, consider splitting phy patches from this. I dont think there is
> > > >>>>>> any dependency        
> > > >>>>>
> > > >>>>> DT patches in this series depend on functionality added here.
> > > >>>>>         
> > > >>>>
> > > >>>> DTS always goes separately from driver changes because it is a hardware
> > > >>>> description. Depending on driver means you have potential ABI break, so
> > > >>>> it is already a warning sign.      
> > > >>>
> > > >>> We understand that ;-)
> > > >>> What Jernej meant was that the DTS patches at the end depend on patch
> > > >>> 01/10, which adds to the PHY binding doc. I am not sure if Vinod's
> > > >>> suggestion was about splitting off 01/10, 03/10, and 10/10, or just the
> > > >>> two latter which touch the driver.
> > > >>>
> > > >>> I can split off 03/10 and 10/10, rebased on top of linux-phy.git/next, and
> > > >>> send that to Vinod.
> > > >>> Then I would keep 01/10 in a respin of this series here, to satisfy the
> > > >>> dependency of the later DTS patches, and Vinod can pick that one patch from
> > > >>> there?      
> > > >>
> > > >> There is no hard dependency of DTS on bindings. You can split these (and
> > > >> some maintainers prefer that way) and in DTS patches just provide the
> > > >> link to the bindings, saying it is in progress.    
> > > > 
> > > > But that breaks "make dtbs_check", doesn't it?    
> > > 
> > > The check will be broken anyway because binding goes via driver
> > > subsystem and DTS goes via arm-soc.
> > > 
> > > If both make to the linux-next and next release, then it's not a problem.
> > >   
> > > > 
> > > > I would think that the DT bits - bindings first, then DTS files using it -
> > > > should be bundled. This is how I imagine the future(TM), where DTs and
> > > > bindings live outside the kernel repo.    
> > > 
> > > Yes, that's preferred. Therefore in DTS patch you say the binding is not
> > > merged and it is here - lore link.
> > >   
> > > >     
> > > >> The bindings should be however kept with driver changes as it goes the
> > > >> same way.    
> > > > 
> > > > I understand that the bindings describe the contract the driver acts on,
> > > > but going forward I think driver changes would need to come later, then
> > > > (since they will live in a separate repo at some day)?
> > > > Maybe pointing to the binding changes in progress?    
> > > 
> > > Later as one commit later - yes. Later as other option - not really, why?
> > >   
> > > > So with a separate repo we would actually need to upstream just the
> > > > bindings first, then could push driver changes and .dts files
> > > > independently?    
> > > 
> > > There is no separate repo, so we talk about Linux case now.
> > >   
> > > > And for now it looks like we are stuck with putting everything in one
> > > > series, to make both checkpatch and dtbs_check happy.    
> > > 
> > > You should rather make maintainers happy :) and here one asked to split.  
> > 
> > Well, he asked to split off the USB PHY patches from the rest of the
> > series, since there is some conflict with the recently merged H616 USB PHY
> > patches. It is still unclear to me whether this split includes the binding
> > patch, or just the two patches touching the actual code.  
> 
> That mean split off USB phy and binding patches from rest and send for
> review

Thanks, I figured, and that's what I did:

https://lore.kernel.org/linux-arm-kernel/20221116151603.819533-1-andre.przywara@arm.com/

Hope that fits!

Cheers,
Andre

> 
> DTS or anything else should not be part of that
> 

[PATCH v3 04/11] musb: sunxi: add support for the F1C100s MUSB controller

[Andre Przywara]

From: Icenowy Zheng <uwu@icenowy.me>

The suniv SoC has a MUSB controller like the one in A33, but with a SRAM
region to be claimed.

Add support for it.

Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/usb/musb/sunxi.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
index 7f9a999cd5ff..4b368d16a73a 100644
--- a/drivers/usb/musb/sunxi.c
+++ b/drivers/usb/musb/sunxi.c
@@ -722,14 +722,17 @@ static int sunxi_musb_probe(struct platform_device *pdev)
 	INIT_WORK(&glue->work, sunxi_musb_work);
 	glue->host_nb.notifier_call = sunxi_musb_host_notifier;
 
-	if (of_device_is_compatible(np, "allwinner,sun4i-a10-musb"))
+	if (of_device_is_compatible(np, "allwinner,sun4i-a10-musb") ||
+	    of_device_is_compatible(np, "allwinner,suniv-f1c100s-musb")) {
 		set_bit(SUNXI_MUSB_FL_HAS_SRAM, &glue->flags);
+	}
 
 	if (of_device_is_compatible(np, "allwinner,sun6i-a31-musb"))
 		set_bit(SUNXI_MUSB_FL_HAS_RESET, &glue->flags);
 
 	if (of_device_is_compatible(np, "allwinner,sun8i-a33-musb") ||
-	    of_device_is_compatible(np, "allwinner,sun8i-h3-musb")) {
+	    of_device_is_compatible(np, "allwinner,sun8i-h3-musb") ||
+	    of_device_is_compatible(np, "allwinner,suniv-f1c100s-musb")) {
 		set_bit(SUNXI_MUSB_FL_HAS_RESET, &glue->flags);
 		set_bit(SUNXI_MUSB_FL_NO_CONFIGDATA, &glue->flags);
 	}
@@ -815,6 +818,7 @@ static const struct of_device_id sunxi_musb_match[] = {
 	{ .compatible = "allwinner,sun6i-a31-musb", },
 	{ .compatible = "allwinner,sun8i-a33-musb", },
 	{ .compatible = "allwinner,sun8i-h3-musb", },
+	{ .compatible = "allwinner,suniv-f1c100s-musb", },
 	{}
 };
 MODULE_DEVICE_TABLE(of, sunxi_musb_match);
-- 
2.35.5

[PATCH v3 05/11] ARM: dts: suniv: add USB-related device nodes

[Andre Przywara]

From: Icenowy Zheng <uwu@icenowy.me>

The suniv SoC has a USB OTG controller and a USB PHY like other
Allwinner SoCs.

Add their device tree node.

Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm/boot/dts/suniv-f1c100s.dtsi | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/arch/arm/boot/dts/suniv-f1c100s.dtsi b/arch/arm/boot/dts/suniv-f1c100s.dtsi
index 0edc1724407b..a01541ba42c5 100644
--- a/arch/arm/boot/dts/suniv-f1c100s.dtsi
+++ b/arch/arm/boot/dts/suniv-f1c100s.dtsi
@@ -133,6 +133,32 @@ mmc1: mmc@1c10000 {
 			#size-cells = <0>;
 		};
 
+		usb_otg: usb@1c13000 {
+			compatible = "allwinner,suniv-f1c100s-musb";
+			reg = <0x01c13000 0x0400>;
+			clocks = <&ccu CLK_BUS_OTG>;
+			resets = <&ccu RST_BUS_OTG>;
+			interrupts = <26>;
+			interrupt-names = "mc";
+			phys = <&usbphy 0>;
+			phy-names = "usb";
+			extcon = <&usbphy 0>;
+			allwinner,sram = <&otg_sram 1>;
+			status = "disabled";
+		};
+
+		usbphy: phy@1c13400 {
+			compatible = "allwinner,suniv-f1c100s-usb-phy";
+			reg = <0x01c13400 0x10>;
+			reg-names = "phy_ctrl";
+			clocks = <&ccu CLK_USB_PHY0>;
+			clock-names = "usb0_phy";
+			resets = <&ccu RST_USB_PHY0>;
+			reset-names = "usb0_reset";
+			#phy-cells = <1>;
+			status = "disabled";
+		};
+
 		ccu: clock@1c20000 {
 			compatible = "allwinner,suniv-f1c100s-ccu";
 			reg = <0x01c20000 0x400>;
-- 
2.35.5

Re: [PATCH v3 05/11] ARM: dts: suniv: add USB-related device nodes

[Jernej Škrabec]

Dne nedelja, 06. november 2022 ob 16:48:20 CET je Andre Przywara napisal(a):
> From: Icenowy Zheng <uwu@icenowy.me>
> 
> The suniv SoC has a USB OTG controller and a USB PHY like other
> Allwinner SoCs.
> 
> Add their device tree node.
> 
> Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
> Reviewed-by: Andre Przywara <andre.przywara@arm.com>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej

[PATCH v3 06/11] ARM: dts: suniv: licheepi-nano: enable USB

[Andre Przywara]

From: Icenowy Zheng <uwu@icenowy.me>

Lichee Pi Nano has a Micro-USB connector, with its D+, D- pins connected
to the USB pins of the SoC and ID pin connected to PE2 GPIO.

Enable the USB functionality.

Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 .../arm/boot/dts/suniv-f1c100s-licheepi-nano.dts | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm/boot/dts/suniv-f1c100s-licheepi-nano.dts b/arch/arm/boot/dts/suniv-f1c100s-licheepi-nano.dts
index 04e59b8381cb..43896723a994 100644
--- a/arch/arm/boot/dts/suniv-f1c100s-licheepi-nano.dts
+++ b/arch/arm/boot/dts/suniv-f1c100s-licheepi-nano.dts
@@ -6,6 +6,8 @@
 /dts-v1/;
 #include "suniv-f1c100s.dtsi"
 
+#include <dt-bindings/gpio/gpio.h>
+
 / {
 	model = "Lichee Pi Nano";
 	compatible = "licheepi,licheepi-nano", "allwinner,suniv-f1c100s";
@@ -50,8 +52,22 @@ flash@0 {
 	};
 };
 
+&otg_sram {
+	status = "okay";
+};
+
 &uart0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart0_pe_pins>;
 	status = "okay";
 };
+
+&usb_otg {
+	dr_mode = "otg";
+	status = "okay";
+};
+
+&usbphy {
+	usb0_id_det-gpios = <&pio 4 2 GPIO_ACTIVE_HIGH>; /* PE2 */
+	status = "okay";
+};
-- 
2.35.5

Re: [PATCH v3 06/11] ARM: dts: suniv: licheepi-nano: enable USB

[Jernej Škrabec]

Dne nedelja, 06. november 2022 ob 16:48:21 CET je Andre Przywara napisal(a):
> From: Icenowy Zheng <uwu@icenowy.me>
> 
> Lichee Pi Nano has a Micro-USB connector, with its D+, D- pins connected
> to the USB pins of the SoC and ID pin connected to PE2 GPIO.
> 
> Enable the USB functionality.
> 
> Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej

[PATCH v3 07/11] dt-bindings: vendor-prefixes: add Source Parts

[Andre Przywara]

From: Icenowy Zheng <uwu@icenowy.me>

Source Parts Inc. [1] is a company that makes a series of SBCs, SoMs,
etc under a brand called Popcorn Computer [2].

Add it to the vendor prefixes list.

[1] https://source.parts/
[2] https://popcorncomputer.com/

Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 6e323a380294..035ef859fbc5 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1203,6 +1203,8 @@ patternProperties:
     description: Solomon Systech Limited
   "^sony,.*":
     description: Sony Corporation
+  "^sourceparts,.*":
+    description: Source Parts Inc.
   "^spansion,.*":
     description: Spansion Inc.
   "^sparkfun,.*":
-- 
2.35.5

[PATCH v3 08/11] dt-binding: arm: sunxi: add compatible strings for PopStick v1.1

[Andre Przywara]

From: Icenowy Zheng <uwu@icenowy.me>

SourceParts PopStick is a F1C200s-based stick-shaped SBC.

Add a compatible string list for its v1.1 version (the first public
one).

Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 Documentation/devicetree/bindings/arm/sunxi.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/sunxi.yaml b/Documentation/devicetree/bindings/arm/sunxi.yaml
index 3ad1cd50e3fe..c6e0ad7f461d 100644
--- a/Documentation/devicetree/bindings/arm/sunxi.yaml
+++ b/Documentation/devicetree/bindings/arm/sunxi.yaml
@@ -807,6 +807,13 @@ properties:
           - const: sinlinx,sina33
           - const: allwinner,sun8i-a33
 
+      - description: SourceParts PopStick v1.1
+        items:
+          - const: sourceparts,popstick-v1.1
+          - const: sourceparts,popstick
+          - const: allwinner,suniv-f1c200s
+          - const: allwinner,suniv-f1c100s
+
       - description: SL631 Action Camera with IMX179
         items:
           - const: allwinner,sl631-imx179
-- 
2.35.5

[PATCH v3 09/11] ARM: dts: suniv: add device tree for PopStick v1.1

[Andre Przywara]

From: Icenowy Zheng <uwu@icenowy.me>

PopStick is a minimal Allwinner F1C200s dongle, with its USB controller
wired to a USB Type-A port, a SD slot and a SPI NAND flash on board, and
an on-board CH340 USB-UART converted connected to F1C200s's UART0.

Add a device tree for it. As F1C200s is just F1C100s with a different
DRAM chip co-packaged, directly use F1C100s DTSI here.

This commit covers the v1.1 version of this board, which is now shipped.
v1.0 is some internal sample that have not been shipped at all.

Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm/boot/dts/Makefile                    |  3 +-
 .../boot/dts/suniv-f1c200s-popstick-v1.1.dts  | 99 +++++++++++++++++++
 2 files changed, 101 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/boot/dts/suniv-f1c200s-popstick-v1.1.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 6aa7dc4db2fc..0249c07bd8a6 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -1391,7 +1391,8 @@ dtb-$(CONFIG_MACH_SUN9I) += \
 	sun9i-a80-optimus.dtb \
 	sun9i-a80-cubieboard4.dtb
 dtb-$(CONFIG_MACH_SUNIV) += \
-	suniv-f1c100s-licheepi-nano.dtb
+	suniv-f1c100s-licheepi-nano.dtb \
+	suniv-f1c200s-popstick-v1.1.dtb
 dtb-$(CONFIG_ARCH_TEGRA_2x_SOC) += \
 	tegra20-acer-a500-picasso.dtb \
 	tegra20-asus-tf101.dtb \
diff --git a/arch/arm/boot/dts/suniv-f1c200s-popstick-v1.1.dts b/arch/arm/boot/dts/suniv-f1c200s-popstick-v1.1.dts
new file mode 100644
index 000000000000..7d69b5fcb905
--- /dev/null
+++ b/arch/arm/boot/dts/suniv-f1c200s-popstick-v1.1.dts
@@ -0,0 +1,99 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright 2022 Icenowy Zheng <uwu@icenowy.me>
+ */
+
+/dts-v1/;
+#include "suniv-f1c100s.dtsi"
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/leds/common.h>
+
+/ {
+	model = "Popcorn Computer PopStick v1.1";
+	compatible = "sourceparts,popstick-v1.1", "sourceparts,popstick",
+		     "allwinner,suniv-f1c200s", "allwinner,suniv-f1c100s";
+
+	aliases {
+		serial0 = &uart0;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+
+	leds {
+		compatible = "gpio-leds";
+
+		led {
+			function = LED_FUNCTION_STATUS;
+			color = <LED_COLOR_ID_GREEN>;
+			gpios = <&pio 4 6 GPIO_ACTIVE_HIGH>; /* PE6 */
+			linux,default-trigger = "heartbeat";
+		};
+	};
+
+	reg_vcc3v3: regulator-3v3 {
+		compatible = "regulator-fixed";
+		regulator-name = "vcc3v3";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+	};
+};
+
+&mmc0 {
+	cd-gpios = <&pio 4 3 GPIO_ACTIVE_LOW>; /* PE3 */
+	bus-width = <4>;
+	disable-wp;
+	vmmc-supply = <&reg_vcc3v3>;
+	status = "okay";
+};
+
+&otg_sram {
+	status = "okay";
+};
+
+&spi0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&spi0_pc_pins>;
+	status = "okay";
+
+	flash@0 {
+		compatible = "spi-nand";
+		reg = <0>;
+		spi-max-frequency = <40000000>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		partitions {
+			compatible = "fixed-partitions";
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			partition@0 {
+				label = "u-boot-with-spl";
+				reg = <0x0 0x100000>;
+			};
+
+			ubi@100000 {
+				label = "ubi";
+				reg = <0x100000 0x7f00000>;
+			};
+		};
+	};
+};
+
+&uart0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart0_pe_pins>;
+	status = "okay";
+};
+
+&usb_otg {
+	dr_mode = "peripheral";
+	status = "okay";
+};
+
+&usbphy {
+	status = "okay";
+};
-- 
2.35.5

Re: [PATCH v3 09/11] ARM: dts: suniv: add device tree for PopStick v1.1

[Jernej Škrabec]

Dne nedelja, 06. november 2022 ob 16:48:24 CET je Andre Przywara napisal(a):
> From: Icenowy Zheng <uwu@icenowy.me>
> 
> PopStick is a minimal Allwinner F1C200s dongle, with its USB controller
> wired to a USB Type-A port, a SD slot and a SPI NAND flash on board, and
> an on-board CH340 USB-UART converted connected to F1C200s's UART0.
> 
> Add a device tree for it. As F1C200s is just F1C100s with a different
> DRAM chip co-packaged, directly use F1C100s DTSI here.
> 
> This commit covers the v1.1 version of this board, which is now shipped.
> v1.0 is some internal sample that have not been shipped at all.
> 
> Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  arch/arm/boot/dts/Makefile                    |  3 +-
>  .../boot/dts/suniv-f1c200s-popstick-v1.1.dts  | 99 +++++++++++++++++++
>  2 files changed, 101 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm/boot/dts/suniv-f1c200s-popstick-v1.1.dts
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 6aa7dc4db2fc..0249c07bd8a6 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -1391,7 +1391,8 @@ dtb-$(CONFIG_MACH_SUN9I) += \
>  	sun9i-a80-optimus.dtb \
>  	sun9i-a80-cubieboard4.dtb
>  dtb-$(CONFIG_MACH_SUNIV) += \
> -	suniv-f1c100s-licheepi-nano.dtb
> +	suniv-f1c100s-licheepi-nano.dtb \
> +	suniv-f1c200s-popstick-v1.1.dtb
>  dtb-$(CONFIG_ARCH_TEGRA_2x_SOC) += \
>  	tegra20-acer-a500-picasso.dtb \
>  	tegra20-asus-tf101.dtb \
> diff --git a/arch/arm/boot/dts/suniv-f1c200s-popstick-v1.1.dts
> b/arch/arm/boot/dts/suniv-f1c200s-popstick-v1.1.dts new file mode 100644
> index 000000000000..7d69b5fcb905
> --- /dev/null
> +++ b/arch/arm/boot/dts/suniv-f1c200s-popstick-v1.1.dts
> @@ -0,0 +1,99 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright 2022 Icenowy Zheng <uwu@icenowy.me>
> + */
> +
> +/dts-v1/;
> +#include "suniv-f1c100s.dtsi"
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/leds/common.h>
> +
> +/ {
> +	model = "Popcorn Computer PopStick v1.1";
> +	compatible = "sourceparts,popstick-v1.1", "sourceparts,popstick",
> +		     "allwinner,suniv-f1c200s", "allwinner,suniv-
f1c100s";
> +
> +	aliases {
> +		serial0 = &uart0;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial0:115200n8";
> +	};
> +
> +	leds {
> +		compatible = "gpio-leds";
> +
> +		led {
> +			function = LED_FUNCTION_STATUS;
> +			color = <LED_COLOR_ID_GREEN>;
> +			gpios = <&pio 4 6 GPIO_ACTIVE_HIGH>; /* PE6 
*/
> +			linux,default-trigger = "heartbeat";
> +		};
> +	};
> +
> +	reg_vcc3v3: regulator-3v3 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vcc3v3";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +	};
> +};
> +
> +&mmc0 {
> +	cd-gpios = <&pio 4 3 GPIO_ACTIVE_LOW>; /* PE3 */
> +	bus-width = <4>;
> +	disable-wp;
> +	vmmc-supply = <&reg_vcc3v3>;
> +	status = "okay";
> +};
> +
> +&otg_sram {
> +	status = "okay";
> +};
> +
> +&spi0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&spi0_pc_pins>;
> +	status = "okay";
> +
> +	flash@0 {
> +		compatible = "spi-nand";
> +		reg = <0>;
> +		spi-max-frequency = <40000000>;
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +
> +		partitions {

"make dtbs_check" is not happy with cells and partitions. Do we really need 
them? If yes, then binding needs to be updated beforehand.

Best regards,
Jernej

> +			compatible = "fixed-partitions";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +
> +			partition@0 {
> +				label = "u-boot-with-spl";
> +				reg = <0x0 0x100000>;
> +			};
> +
> +			ubi@100000 {
> +				label = "ubi";
> +				reg = <0x100000 0x7f00000>;
> +			};
> +		};
> +	};
> +};
> +
> +&uart0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&uart0_pe_pins>;
> +	status = "okay";
> +};
> +
> +&usb_otg {
> +	dr_mode = "peripheral";
> +	status = "okay";
> +};
> +
> +&usbphy {
> +	status = "okay";
> +};
> --
> 2.35.5

Re: [PATCH v3 09/11] ARM: dts: suniv: add device tree for PopStick v1.1

[Andre Przywara]

On Mon, 07 Nov 2022 18:35:09 +0100
Jernej Škrabec <jernej.skrabec@gmail.com> wrote:

Hi Jernej,

> Dne nedelja, 06. november 2022 ob 16:48:24 CET je Andre Przywara napisal(a):
> > From: Icenowy Zheng <uwu@icenowy.me>
> > 
> > PopStick is a minimal Allwinner F1C200s dongle, with its USB controller
> > wired to a USB Type-A port, a SD slot and a SPI NAND flash on board, and
> > an on-board CH340 USB-UART converted connected to F1C200s's UART0.
> > 
> > Add a device tree for it. As F1C200s is just F1C100s with a different
> > DRAM chip co-packaged, directly use F1C100s DTSI here.
> > 
> > This commit covers the v1.1 version of this board, which is now shipped.
> > v1.0 is some internal sample that have not been shipped at all.
> > 
> > Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > ---
> >  arch/arm/boot/dts/Makefile                    |  3 +-
> >  .../boot/dts/suniv-f1c200s-popstick-v1.1.dts  | 99 +++++++++++++++++++
> >  2 files changed, 101 insertions(+), 1 deletion(-)
> >  create mode 100644 arch/arm/boot/dts/suniv-f1c200s-popstick-v1.1.dts
> > 
> > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> > index 6aa7dc4db2fc..0249c07bd8a6 100644
> > --- a/arch/arm/boot/dts/Makefile
> > +++ b/arch/arm/boot/dts/Makefile
> > @@ -1391,7 +1391,8 @@ dtb-$(CONFIG_MACH_SUN9I) += \
> >  	sun9i-a80-optimus.dtb \
> >  	sun9i-a80-cubieboard4.dtb
> >  dtb-$(CONFIG_MACH_SUNIV) += \
> > -	suniv-f1c100s-licheepi-nano.dtb
> > +	suniv-f1c100s-licheepi-nano.dtb \
> > +	suniv-f1c200s-popstick-v1.1.dtb
> >  dtb-$(CONFIG_ARCH_TEGRA_2x_SOC) += \
> >  	tegra20-acer-a500-picasso.dtb \
> >  	tegra20-asus-tf101.dtb \
> > diff --git a/arch/arm/boot/dts/suniv-f1c200s-popstick-v1.1.dts
> > b/arch/arm/boot/dts/suniv-f1c200s-popstick-v1.1.dts new file mode 100644
> > index 000000000000..7d69b5fcb905
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/suniv-f1c200s-popstick-v1.1.dts
> > @@ -0,0 +1,99 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +/*
> > + * Copyright 2022 Icenowy Zheng <uwu@icenowy.me>
> > + */
> > +
> > +/dts-v1/;
> > +#include "suniv-f1c100s.dtsi"
> > +
> > +#include <dt-bindings/gpio/gpio.h>
> > +#include <dt-bindings/leds/common.h>
> > +
> > +/ {
> > +	model = "Popcorn Computer PopStick v1.1";
> > +	compatible = "sourceparts,popstick-v1.1", "sourceparts,popstick",
> > +		     "allwinner,suniv-f1c200s", "allwinner,suniv-  
> f1c100s";
> > +
> > +	aliases {
> > +		serial0 = &uart0;
> > +	};
> > +
> > +	chosen {
> > +		stdout-path = "serial0:115200n8";
> > +	};
> > +
> > +	leds {
> > +		compatible = "gpio-leds";
> > +
> > +		led {
> > +			function = LED_FUNCTION_STATUS;
> > +			color = <LED_COLOR_ID_GREEN>;
> > +			gpios = <&pio 4 6 GPIO_ACTIVE_HIGH>; /* PE6   
> */
> > +			linux,default-trigger = "heartbeat";
> > +		};
> > +	};
> > +
> > +	reg_vcc3v3: regulator-3v3 {
> > +		compatible = "regulator-fixed";
> > +		regulator-name = "vcc3v3";
> > +		regulator-min-microvolt = <3300000>;
> > +		regulator-max-microvolt = <3300000>;
> > +	};
> > +};
> > +
> > +&mmc0 {
> > +	cd-gpios = <&pio 4 3 GPIO_ACTIVE_LOW>; /* PE3 */
> > +	bus-width = <4>;
> > +	disable-wp;
> > +	vmmc-supply = <&reg_vcc3v3>;
> > +	status = "okay";
> > +};
> > +
> > +&otg_sram {
> > +	status = "okay";
> > +};
> > +
> > +&spi0 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&spi0_pc_pins>;
> > +	status = "okay";
> > +
> > +	flash@0 {
> > +		compatible = "spi-nand";
> > +		reg = <0>;
> > +		spi-max-frequency = <40000000>;
> > +		#address-cells = <1>;
> > +		#size-cells = <1>;
> > +
> > +		partitions {  
> 
> "make dtbs_check" is not happy with cells and partitions. Do we really need 
> them? If yes, then binding needs to be updated beforehand.

Right, my dt-validate run didn't initially complain, but does now, after
updating dt-schema.git.
So yes, I will just drop the partitions, for now. The partition scheme
seems to be legit, as described in mtd/fixed-partitions.yaml, but it
apparently just needs to be somehow included in spi-nand.yaml. Will do
that later, then update this .dts.

Cheers,
Andre

> 
> > +			compatible = "fixed-partitions";
> > +			#address-cells = <1>;
> > +			#size-cells = <1>;
> > +
> > +			partition@0 {
> > +				label = "u-boot-with-spl";
> > +				reg = <0x0 0x100000>;
> > +			};
> > +
> > +			ubi@100000 {
> > +				label = "ubi";
> > +				reg = <0x100000 0x7f00000>;
> > +			};
> > +		};
> > +	};
> > +};
> > +
> > +&uart0 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&uart0_pe_pins>;
> > +	status = "okay";
> > +};
> > +
> > +&usb_otg {
> > +	dr_mode = "peripheral";
> > +	status = "okay";
> > +};
> > +
> > +&usbphy {
> > +	status = "okay";
> > +};
> > --
> > 2.35.5  
> 
> 

Re: [PATCH v3 09/11] ARM: dts: suniv: add device tree for PopStick v1.1

[Samuel Holland]

On 11/6/22 09:48, Andre Przywara wrote:
> From: Icenowy Zheng <uwu@icenowy.me>
> 
> PopStick is a minimal Allwinner F1C200s dongle, with its USB controller
> wired to a USB Type-A port, a SD slot and a SPI NAND flash on board, and
> an on-board CH340 USB-UART converted connected to F1C200s's UART0.
> 
> Add a device tree for it. As F1C200s is just F1C100s with a different
> DRAM chip co-packaged, directly use F1C100s DTSI here.
> 
> This commit covers the v1.1 version of this board, which is now shipped.
> v1.0 is some internal sample that have not been shipped at all.
> 
> Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  arch/arm/boot/dts/Makefile                    |  3 +-
>  .../boot/dts/suniv-f1c200s-popstick-v1.1.dts  | 99 +++++++++++++++++++
>  2 files changed, 101 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm/boot/dts/suniv-f1c200s-popstick-v1.1.dts
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 6aa7dc4db2fc..0249c07bd8a6 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -1391,7 +1391,8 @@ dtb-$(CONFIG_MACH_SUN9I) += \
>  	sun9i-a80-optimus.dtb \
>  	sun9i-a80-cubieboard4.dtb
>  dtb-$(CONFIG_MACH_SUNIV) += \
> -	suniv-f1c100s-licheepi-nano.dtb
> +	suniv-f1c100s-licheepi-nano.dtb \
> +	suniv-f1c200s-popstick-v1.1.dtb
>  dtb-$(CONFIG_ARCH_TEGRA_2x_SOC) += \
>  	tegra20-acer-a500-picasso.dtb \
>  	tegra20-asus-tf101.dtb \
> diff --git a/arch/arm/boot/dts/suniv-f1c200s-popstick-v1.1.dts b/arch/arm/boot/dts/suniv-f1c200s-popstick-v1.1.dts
> new file mode 100644
> index 000000000000..7d69b5fcb905
> --- /dev/null
> +++ b/arch/arm/boot/dts/suniv-f1c200s-popstick-v1.1.dts
> @@ -0,0 +1,99 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright 2022 Icenowy Zheng <uwu@icenowy.me>
> + */
> +
> +/dts-v1/;
> +#include "suniv-f1c100s.dtsi"
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/leds/common.h>
> +
> +/ {
> +	model = "Popcorn Computer PopStick v1.1";
> +	compatible = "sourceparts,popstick-v1.1", "sourceparts,popstick",
> +		     "allwinner,suniv-f1c200s", "allwinner,suniv-f1c100s";
> +
> +	aliases {
> +		serial0 = &uart0;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial0:115200n8";
> +	};
> +
> +	leds {
> +		compatible = "gpio-leds";
> +
> +		led {
> +			function = LED_FUNCTION_STATUS;
> +			color = <LED_COLOR_ID_GREEN>;
> +			gpios = <&pio 4 6 GPIO_ACTIVE_HIGH>; /* PE6 */
> +			linux,default-trigger = "heartbeat";
> +		};
> +	};
> +
> +	reg_vcc3v3: regulator-3v3 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vcc3v3";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +	};
> +};
> +
> +&mmc0 {
> +	cd-gpios = <&pio 4 3 GPIO_ACTIVE_LOW>; /* PE3 */
> +	bus-width = <4>;
> +	disable-wp;
> +	vmmc-supply = <&reg_vcc3v3>;
> +	status = "okay";
> +};
> +
> +&otg_sram {
> +	status = "okay";
> +};
> +
> +&spi0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&spi0_pc_pins>;
> +	status = "okay";
> +
> +	flash@0 {
> +		compatible = "spi-nand";
> +		reg = <0>;
> +		spi-max-frequency = <40000000>;
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +
> +		partitions {
> +			compatible = "fixed-partitions";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +
> +			partition@0 {
> +				label = "u-boot-with-spl";
> +				reg = <0x0 0x100000>;
> +			};
> +
> +			ubi@100000 {
> +				label = "ubi";
> +				reg = <0x100000 0x7f00000>;
> +			};
> +		};
> +	};
> +};
> +
> +&uart0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&uart0_pe_pins>;
> +	status = "okay";
> +};
> +
> +&usb_otg {
> +	dr_mode = "peripheral";

The patch description says the board has a USB Type-A port. Why is the
USB controller set to peripheral mode?

Regards,
Samuel

> +	status = "okay";
> +};
> +
> +&usbphy {
> +	status = "okay";
> +};

Re: [PATCH v3 09/11] ARM: dts: suniv: add device tree for PopStick v1.1

[Andre Przywara]

On Sun, 13 Nov 2022 16:41:04 -0600
Samuel Holland <samuel@sholland.org> wrote:

> On 11/6/22 09:48, Andre Przywara wrote:
> > From: Icenowy Zheng <uwu@icenowy.me>
> > 
> > PopStick is a minimal Allwinner F1C200s dongle, with its USB controller
> > wired to a USB Type-A port, a SD slot and a SPI NAND flash on board, and
> > an on-board CH340 USB-UART converted connected to F1C200s's UART0.
> > 
> > Add a device tree for it. As F1C200s is just F1C100s with a different
> > DRAM chip co-packaged, directly use F1C100s DTSI here.
> > 
> > This commit covers the v1.1 version of this board, which is now shipped.
> > v1.0 is some internal sample that have not been shipped at all.
> > 
> > Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > ---
> >  arch/arm/boot/dts/Makefile                    |  3 +-
> >  .../boot/dts/suniv-f1c200s-popstick-v1.1.dts  | 99 +++++++++++++++++++
> >  2 files changed, 101 insertions(+), 1 deletion(-)
> >  create mode 100644 arch/arm/boot/dts/suniv-f1c200s-popstick-v1.1.dts
> > 
> > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> > index 6aa7dc4db2fc..0249c07bd8a6 100644
> > --- a/arch/arm/boot/dts/Makefile
> > +++ b/arch/arm/boot/dts/Makefile
> > @@ -1391,7 +1391,8 @@ dtb-$(CONFIG_MACH_SUN9I) += \
> >  	sun9i-a80-optimus.dtb \
> >  	sun9i-a80-cubieboard4.dtb
> >  dtb-$(CONFIG_MACH_SUNIV) += \
> > -	suniv-f1c100s-licheepi-nano.dtb
> > +	suniv-f1c100s-licheepi-nano.dtb \
> > +	suniv-f1c200s-popstick-v1.1.dtb
> >  dtb-$(CONFIG_ARCH_TEGRA_2x_SOC) += \
> >  	tegra20-acer-a500-picasso.dtb \
> >  	tegra20-asus-tf101.dtb \
> > diff --git a/arch/arm/boot/dts/suniv-f1c200s-popstick-v1.1.dts b/arch/arm/boot/dts/suniv-f1c200s-popstick-v1.1.dts
> > new file mode 100644
> > index 000000000000..7d69b5fcb905
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/suniv-f1c200s-popstick-v1.1.dts
> > @@ -0,0 +1,99 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +/*
> > + * Copyright 2022 Icenowy Zheng <uwu@icenowy.me>
> > + */
> > +
> > +/dts-v1/;
> > +#include "suniv-f1c100s.dtsi"
> > +
> > +#include <dt-bindings/gpio/gpio.h>
> > +#include <dt-bindings/leds/common.h>
> > +
> > +/ {
> > +	model = "Popcorn Computer PopStick v1.1";
> > +	compatible = "sourceparts,popstick-v1.1", "sourceparts,popstick",
> > +		     "allwinner,suniv-f1c200s", "allwinner,suniv-f1c100s";
> > +
> > +	aliases {
> > +		serial0 = &uart0;
> > +	};
> > +
> > +	chosen {
> > +		stdout-path = "serial0:115200n8";
> > +	};
> > +
> > +	leds {
> > +		compatible = "gpio-leds";
> > +
> > +		led {
> > +			function = LED_FUNCTION_STATUS;
> > +			color = <LED_COLOR_ID_GREEN>;
> > +			gpios = <&pio 4 6 GPIO_ACTIVE_HIGH>; /* PE6 */
> > +			linux,default-trigger = "heartbeat";
> > +		};
> > +	};
> > +
> > +	reg_vcc3v3: regulator-3v3 {
> > +		compatible = "regulator-fixed";
> > +		regulator-name = "vcc3v3";
> > +		regulator-min-microvolt = <3300000>;
> > +		regulator-max-microvolt = <3300000>;
> > +	};
> > +};
> > +
> > +&mmc0 {
> > +	cd-gpios = <&pio 4 3 GPIO_ACTIVE_LOW>; /* PE3 */
> > +	bus-width = <4>;
> > +	disable-wp;
> > +	vmmc-supply = <&reg_vcc3v3>;
> > +	status = "okay";
> > +};
> > +
> > +&otg_sram {
> > +	status = "okay";
> > +};
> > +
> > +&spi0 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&spi0_pc_pins>;
> > +	status = "okay";
> > +
> > +	flash@0 {
> > +		compatible = "spi-nand";
> > +		reg = <0>;
> > +		spi-max-frequency = <40000000>;
> > +		#address-cells = <1>;
> > +		#size-cells = <1>;
> > +
> > +		partitions {
> > +			compatible = "fixed-partitions";
> > +			#address-cells = <1>;
> > +			#size-cells = <1>;
> > +
> > +			partition@0 {
> > +				label = "u-boot-with-spl";
> > +				reg = <0x0 0x100000>;
> > +			};
> > +
> > +			ubi@100000 {
> > +				label = "ubi";
> > +				reg = <0x100000 0x7f00000>;
> > +			};
> > +		};
> > +	};
> > +};
> > +
> > +&uart0 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&uart0_pe_pins>;
> > +	status = "okay";
> > +};
> > +
> > +&usb_otg {
> > +	dr_mode = "peripheral";  
> 
> The patch description says the board has a USB Type-A port. Why is the
> USB controller set to peripheral mode?

It's a USB type-A *plug*, not a socket, because it's some TV stick
style of device, just with a USB instead of an HDMI plug.

Cheers,
Andre

> 
> > +	status = "okay";
> > +};
> > +
> > +&usbphy {
> > +	status = "okay";
> > +};  
> 

Re: [PATCH v3 09/11] ARM: dts: suniv: add device tree for PopStick v1.1

[Samuel Holland]

On 11/13/22 18:17, Andre Przywara wrote:
> On Sun, 13 Nov 2022 16:41:04 -0600
> Samuel Holland <samuel@sholland.org> wrote:
> 
>> On 11/6/22 09:48, Andre Przywara wrote:
>>> From: Icenowy Zheng <uwu@icenowy.me>
>>>
>>> PopStick is a minimal Allwinner F1C200s dongle, with its USB controller
>>> wired to a USB Type-A port, a SD slot and a SPI NAND flash on board, and
>>> an on-board CH340 USB-UART converted connected to F1C200s's UART0.
>>>
>>> Add a device tree for it. As F1C200s is just F1C100s with a different
>>> DRAM chip co-packaged, directly use F1C100s DTSI here.
>>>
>>> This commit covers the v1.1 version of this board, which is now shipped.
>>> v1.0 is some internal sample that have not been shipped at all.
>>>
>>> Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>> ---
>>>  arch/arm/boot/dts/Makefile                    |  3 +-
>>>  .../boot/dts/suniv-f1c200s-popstick-v1.1.dts  | 99 +++++++++++++++++++
>>>  2 files changed, 101 insertions(+), 1 deletion(-)
>>>  create mode 100644 arch/arm/boot/dts/suniv-f1c200s-popstick-v1.1.dts
>>>
>>> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
>>> index 6aa7dc4db2fc..0249c07bd8a6 100644
>>> --- a/arch/arm/boot/dts/Makefile
>>> +++ b/arch/arm/boot/dts/Makefile
>>> @@ -1391,7 +1391,8 @@ dtb-$(CONFIG_MACH_SUN9I) += \
>>>  	sun9i-a80-optimus.dtb \
>>>  	sun9i-a80-cubieboard4.dtb
>>>  dtb-$(CONFIG_MACH_SUNIV) += \
>>> -	suniv-f1c100s-licheepi-nano.dtb
>>> +	suniv-f1c100s-licheepi-nano.dtb \
>>> +	suniv-f1c200s-popstick-v1.1.dtb
>>>  dtb-$(CONFIG_ARCH_TEGRA_2x_SOC) += \
>>>  	tegra20-acer-a500-picasso.dtb \
>>>  	tegra20-asus-tf101.dtb \
>>> diff --git a/arch/arm/boot/dts/suniv-f1c200s-popstick-v1.1.dts b/arch/arm/boot/dts/suniv-f1c200s-popstick-v1.1.dts
>>> new file mode 100644
>>> index 000000000000..7d69b5fcb905
>>> --- /dev/null
>>> +++ b/arch/arm/boot/dts/suniv-f1c200s-popstick-v1.1.dts
>>> @@ -0,0 +1,99 @@
>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>> +/*
>>> + * Copyright 2022 Icenowy Zheng <uwu@icenowy.me>
>>> + */
>>> +
>>> +/dts-v1/;
>>> +#include "suniv-f1c100s.dtsi"
>>> +
>>> +#include <dt-bindings/gpio/gpio.h>
>>> +#include <dt-bindings/leds/common.h>
>>> +
>>> +/ {
>>> +	model = "Popcorn Computer PopStick v1.1";
>>> +	compatible = "sourceparts,popstick-v1.1", "sourceparts,popstick",
>>> +		     "allwinner,suniv-f1c200s", "allwinner,suniv-f1c100s";
>>> +
>>> +	aliases {
>>> +		serial0 = &uart0;
>>> +	};
>>> +
>>> +	chosen {
>>> +		stdout-path = "serial0:115200n8";
>>> +	};
>>> +
>>> +	leds {
>>> +		compatible = "gpio-leds";
>>> +
>>> +		led {
>>> +			function = LED_FUNCTION_STATUS;
>>> +			color = <LED_COLOR_ID_GREEN>;
>>> +			gpios = <&pio 4 6 GPIO_ACTIVE_HIGH>; /* PE6 */
>>> +			linux,default-trigger = "heartbeat";
>>> +		};
>>> +	};
>>> +
>>> +	reg_vcc3v3: regulator-3v3 {
>>> +		compatible = "regulator-fixed";
>>> +		regulator-name = "vcc3v3";
>>> +		regulator-min-microvolt = <3300000>;
>>> +		regulator-max-microvolt = <3300000>;
>>> +	};
>>> +};
>>> +
>>> +&mmc0 {
>>> +	cd-gpios = <&pio 4 3 GPIO_ACTIVE_LOW>; /* PE3 */
>>> +	bus-width = <4>;
>>> +	disable-wp;
>>> +	vmmc-supply = <&reg_vcc3v3>;
>>> +	status = "okay";
>>> +};
>>> +
>>> +&otg_sram {
>>> +	status = "okay";
>>> +};
>>> +
>>> +&spi0 {
>>> +	pinctrl-names = "default";
>>> +	pinctrl-0 = <&spi0_pc_pins>;
>>> +	status = "okay";
>>> +
>>> +	flash@0 {
>>> +		compatible = "spi-nand";
>>> +		reg = <0>;
>>> +		spi-max-frequency = <40000000>;
>>> +		#address-cells = <1>;
>>> +		#size-cells = <1>;
>>> +
>>> +		partitions {
>>> +			compatible = "fixed-partitions";
>>> +			#address-cells = <1>;
>>> +			#size-cells = <1>;
>>> +
>>> +			partition@0 {
>>> +				label = "u-boot-with-spl";
>>> +				reg = <0x0 0x100000>;
>>> +			};
>>> +
>>> +			ubi@100000 {
>>> +				label = "ubi";
>>> +				reg = <0x100000 0x7f00000>;
>>> +			};
>>> +		};
>>> +	};
>>> +};
>>> +
>>> +&uart0 {
>>> +	pinctrl-names = "default";
>>> +	pinctrl-0 = <&uart0_pe_pins>;
>>> +	status = "okay";
>>> +};
>>> +
>>> +&usb_otg {
>>> +	dr_mode = "peripheral";  
>>
>> The patch description says the board has a USB Type-A port. Why is the
>> USB controller set to peripheral mode?
> 
> It's a USB type-A *plug*, not a socket, because it's some TV stick
> style of device, just with a USB instead of an HDMI plug.

Ah, that makes sense. Please clarify this in the commit message.

Regards,
Samuel

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

[Andre Przywara]

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;
+	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[] = {
-- 
2.35.5

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

[Icenowy Zheng]

于 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.

>+	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[] = {

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

[Icenowy Zheng]

在 2022-11-10星期四的 13:04 +0530，Vinod Koul写道：
> On 06-11-22, 23: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.
> 
> why should it be int.. dont think hsic_phy is improvement over
> phy2_is_hsic?

Yes because it may express phy1_is_hsic, etc (although this kind of
thing hadn't happened yet).

> 
> > 
> > > +       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[] = {
> 

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

[Andre Przywara]

On Thu, 10 Nov 2022 19:40:58 +0800
Icenowy Zheng <uwu@icenowy.me> wrote:

Hi,

> 在 2022-11-10星期四的 13:04 +0530，Vinod Koul写道：
> > On 06-11-22, 23: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.  
> > 
> > why should it be int.. dont think hsic_phy is improvement over
> > phy2_is_hsic?  
> 
> Yes because it may express phy1_is_hsic, etc (although this kind of
> thing hadn't happened yet).

Yeah, I tried to not interpret too much into this, instead just named it
as it is used today. I don't have any insight into the A83T PHY or in
Allwinner's plans regarding this. So far this seems like a one-off hack
that is needed for this particular PHY on this particular SoC.
It it code-internal anyway, so we can change it at any time later should
the need arise.

If people like another name better, I am of course happy to use that.

Cheers,
Andre

> 
> >   
> > >   
> > > > +       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[] = {  
> >   
> 

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

[Samuel Holland]

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.

Regards,
Samuel

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

[Andre Przywara]

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
> 
> 

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

[Jernej Škrabec]

Dne nedelja, 06. november 2022 ob 16:48:25 CET je Andre Przywara napisal(a):
> 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>

Thanks for working on that, nice cleanup!

Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej

[PATCH v3 11/11] usb: musb: sunxi: Introduce config struct

[Andre Przywara]

Currently the probe routine explicitly compares the compatible string of
the device node to figure out which features and quirks a certain
Allwinner MUSB model requires. This gets harder to maintain for new
SoCs.

Add a struct sunxi_musb_cfg that names the features and quirks
explicitly, and create instances of this struct for every type of MUSB
device we support. Then bind this to the compatible strings via the OF
data feature.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/usb/musb/sunxi.c | 101 +++++++++++++++++++++++++++++----------
 1 file changed, 75 insertions(+), 26 deletions(-)

diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
index 4b368d16a73a..266f8baf5af0 100644
--- a/drivers/usb/musb/sunxi.c
+++ b/drivers/usb/musb/sunxi.c
@@ -15,6 +15,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/phy/phy-sun4i-usb.h>
 #include <linux/platform_device.h>
 #include <linux/reset.h>
@@ -67,6 +68,13 @@
 #define SUNXI_MUSB_FL_NO_CONFIGDATA		7
 #define SUNXI_MUSB_FL_PHY_MODE_PEND		8
 
+struct sunxi_musb_cfg {
+	int nr_endpoints;
+	bool has_sram;
+	bool has_reset;
+	bool no_configdata;
+};
+
 /* Our read/write methods need access and do not get passed in a musb ref :| */
 static struct musb *sunxi_musb;
 
@@ -625,7 +633,7 @@ static const struct musb_platform_ops sunxi_musb_ops = {
 #define SUNXI_MUSB_MAX_EP_NUM	6
 #define SUNXI_MUSB_RAM_BITS	11
 
-static struct musb_fifo_cfg sunxi_musb_mode_cfg[] = {
+static struct musb_fifo_cfg sunxi_musb_mode_cfg_5eps[] = {
 	MUSB_EP_FIFO_SINGLE(1, FIFO_TX, 512),
 	MUSB_EP_FIFO_SINGLE(1, FIFO_RX, 512),
 	MUSB_EP_FIFO_SINGLE(2, FIFO_TX, 512),
@@ -641,7 +649,7 @@ static struct musb_fifo_cfg sunxi_musb_mode_cfg[] = {
 /* H3/V3s OTG supports only 4 endpoints */
 #define SUNXI_MUSB_MAX_EP_NUM_H3	5
 
-static struct musb_fifo_cfg sunxi_musb_mode_cfg_h3[] = {
+static struct musb_fifo_cfg sunxi_musb_mode_cfg_4eps[] = {
 	MUSB_EP_FIFO_SINGLE(1, FIFO_TX, 512),
 	MUSB_EP_FIFO_SINGLE(1, FIFO_RX, 512),
 	MUSB_EP_FIFO_SINGLE(2, FIFO_TX, 512),
@@ -652,18 +660,18 @@ static struct musb_fifo_cfg sunxi_musb_mode_cfg_h3[] = {
 	MUSB_EP_FIFO_SINGLE(4, FIFO_RX, 512),
 };
 
-static const struct musb_hdrc_config sunxi_musb_hdrc_config = {
-	.fifo_cfg       = sunxi_musb_mode_cfg,
-	.fifo_cfg_size  = ARRAY_SIZE(sunxi_musb_mode_cfg),
+static const struct musb_hdrc_config sunxi_musb_hdrc_config_5eps = {
+	.fifo_cfg       = sunxi_musb_mode_cfg_5eps,
+	.fifo_cfg_size  = ARRAY_SIZE(sunxi_musb_mode_cfg_5eps),
 	.multipoint	= true,
 	.dyn_fifo	= true,
 	.num_eps	= SUNXI_MUSB_MAX_EP_NUM,
 	.ram_bits	= SUNXI_MUSB_RAM_BITS,
 };
 
-static struct musb_hdrc_config sunxi_musb_hdrc_config_h3 = {
-	.fifo_cfg       = sunxi_musb_mode_cfg_h3,
-	.fifo_cfg_size  = ARRAY_SIZE(sunxi_musb_mode_cfg_h3),
+static struct musb_hdrc_config sunxi_musb_hdrc_config_4eps = {
+	.fifo_cfg       = sunxi_musb_mode_cfg_4eps,
+	.fifo_cfg_size  = ARRAY_SIZE(sunxi_musb_mode_cfg_4eps),
 	.multipoint	= true,
 	.dyn_fifo	= true,
 	.num_eps	= SUNXI_MUSB_MAX_EP_NUM_H3,
@@ -677,6 +685,7 @@ static int sunxi_musb_probe(struct platform_device *pdev)
 	struct platform_device_info	pinfo;
 	struct sunxi_glue		*glue;
 	struct device_node		*np = pdev->dev.of_node;
+	const struct sunxi_musb_cfg	*cfg;
 	int ret;
 
 	if (!np) {
@@ -713,29 +722,35 @@ static int sunxi_musb_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 	pdata.platform_ops	= &sunxi_musb_ops;
-	if (!of_device_is_compatible(np, "allwinner,sun8i-h3-musb"))
-		pdata.config = &sunxi_musb_hdrc_config;
-	else
-		pdata.config = &sunxi_musb_hdrc_config_h3;
+
+	cfg = of_device_get_match_data(&pdev->dev);
+	if (!cfg)
+		return -EINVAL;
+
+	switch (cfg->nr_endpoints) {
+	case 4:
+		pdata.config = &sunxi_musb_hdrc_config_4eps;
+		break;
+	case 5:
+		pdata.config = &sunxi_musb_hdrc_config_5eps;
+		break;
+	default:
+		dev_err(&pdev->dev, "Only 4 or 5 endpoints supported\n");
+		return -EINVAL;
+	}
 
 	glue->dev = &pdev->dev;
 	INIT_WORK(&glue->work, sunxi_musb_work);
 	glue->host_nb.notifier_call = sunxi_musb_host_notifier;
 
-	if (of_device_is_compatible(np, "allwinner,sun4i-a10-musb") ||
-	    of_device_is_compatible(np, "allwinner,suniv-f1c100s-musb")) {
+	if (cfg->has_sram)
 		set_bit(SUNXI_MUSB_FL_HAS_SRAM, &glue->flags);
-	}
 
-	if (of_device_is_compatible(np, "allwinner,sun6i-a31-musb"))
+	if (cfg->has_reset)
 		set_bit(SUNXI_MUSB_FL_HAS_RESET, &glue->flags);
 
-	if (of_device_is_compatible(np, "allwinner,sun8i-a33-musb") ||
-	    of_device_is_compatible(np, "allwinner,sun8i-h3-musb") ||
-	    of_device_is_compatible(np, "allwinner,suniv-f1c100s-musb")) {
-		set_bit(SUNXI_MUSB_FL_HAS_RESET, &glue->flags);
+	if (cfg->no_configdata)
 		set_bit(SUNXI_MUSB_FL_NO_CONFIGDATA, &glue->flags);
-	}
 
 	glue->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(glue->clk)) {
@@ -813,12 +828,46 @@ static int sunxi_musb_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct sunxi_musb_cfg sun4i_a10_musb_cfg = {
+	.nr_endpoints = 5,
+	.has_sram = true,
+};
+
+static const struct sunxi_musb_cfg sun6i_a31_musb_cfg = {
+	.nr_endpoints = 5,
+	.has_reset = true,
+};
+
+static const struct sunxi_musb_cfg sun8i_a33_musb_cfg = {
+	.nr_endpoints = 5,
+	.has_reset = true,
+	.no_configdata = true,
+};
+
+static const struct sunxi_musb_cfg sun8i_h3_musb_cfg = {
+	.nr_endpoints = 4,
+	.has_reset = true,
+	.no_configdata = true,
+};
+
+static const struct sunxi_musb_cfg suniv_f1c100s_musb_cfg = {
+	.nr_endpoints = 5,
+	.has_sram = true,
+	.has_reset = true,
+	.no_configdata = true,
+};
+
 static const struct of_device_id sunxi_musb_match[] = {
-	{ .compatible = "allwinner,sun4i-a10-musb", },
-	{ .compatible = "allwinner,sun6i-a31-musb", },
-	{ .compatible = "allwinner,sun8i-a33-musb", },
-	{ .compatible = "allwinner,sun8i-h3-musb", },
-	{ .compatible = "allwinner,suniv-f1c100s-musb", },
+	{ .compatible = "allwinner,sun4i-a10-musb",
+	  .data = &sun4i_a10_musb_cfg, },
+	{ .compatible = "allwinner,sun6i-a31-musb",
+	  .data = &sun6i_a31_musb_cfg, },
+	{ .compatible = "allwinner,sun8i-a33-musb",
+	  .data = &sun8i_a33_musb_cfg, },
+	{ .compatible = "allwinner,sun8i-h3-musb",
+	  .data = &sun8i_h3_musb_cfg, },
+	{ .compatible = "allwinner,suniv-f1c100s-musb",
+	  .data = &suniv_f1c100s_musb_cfg, },
 	{}
 };
 MODULE_DEVICE_TABLE(of, sunxi_musb_match);
-- 
2.35.5

Re: [PATCH v3 11/11] usb: musb: sunxi: Introduce config struct

[Jernej Škrabec]

Hi Andre!

Dne nedelja, 06. november 2022 ob 16:48:26 CET je Andre Przywara napisal(a):
> Currently the probe routine explicitly compares the compatible string of
> the device node to figure out which features and quirks a certain
> Allwinner MUSB model requires. This gets harder to maintain for new
> SoCs.
> 
> Add a struct sunxi_musb_cfg that names the features and quirks
> explicitly, and create instances of this struct for every type of MUSB
> device we support. Then bind this to the compatible strings via the OF
> data feature.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  drivers/usb/musb/sunxi.c | 101 +++++++++++++++++++++++++++++----------
>  1 file changed, 75 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
> index 4b368d16a73a..266f8baf5af0 100644
> --- a/drivers/usb/musb/sunxi.c
> +++ b/drivers/usb/musb/sunxi.c
> @@ -15,6 +15,7 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> +#include <linux/of_device.h>
>  #include <linux/phy/phy-sun4i-usb.h>
>  #include <linux/platform_device.h>
>  #include <linux/reset.h>
> @@ -67,6 +68,13 @@
>  #define SUNXI_MUSB_FL_NO_CONFIGDATA		7
>  #define SUNXI_MUSB_FL_PHY_MODE_PEND		8
> 
> +struct sunxi_musb_cfg {
> +	int nr_endpoints;
> +	bool has_sram;
> +	bool has_reset;
> +	bool no_configdata;
> +};
> +
>  /* Our read/write methods need access and do not get passed in a musb ref
> :| */ static struct musb *sunxi_musb;
> 
> @@ -625,7 +633,7 @@ static const struct musb_platform_ops sunxi_musb_ops = {
> #define SUNXI_MUSB_MAX_EP_NUM	6
>  #define SUNXI_MUSB_RAM_BITS	11
> 
> -static struct musb_fifo_cfg sunxi_musb_mode_cfg[] = {
> +static struct musb_fifo_cfg sunxi_musb_mode_cfg_5eps[] = {
>  	MUSB_EP_FIFO_SINGLE(1, FIFO_TX, 512),
>  	MUSB_EP_FIFO_SINGLE(1, FIFO_RX, 512),
>  	MUSB_EP_FIFO_SINGLE(2, FIFO_TX, 512),
> @@ -641,7 +649,7 @@ static struct musb_fifo_cfg sunxi_musb_mode_cfg[] = {
>  /* H3/V3s OTG supports only 4 endpoints */
>  #define SUNXI_MUSB_MAX_EP_NUM_H3	5
> 
> -static struct musb_fifo_cfg sunxi_musb_mode_cfg_h3[] = {
> +static struct musb_fifo_cfg sunxi_musb_mode_cfg_4eps[] = {
>  	MUSB_EP_FIFO_SINGLE(1, FIFO_TX, 512),
>  	MUSB_EP_FIFO_SINGLE(1, FIFO_RX, 512),
>  	MUSB_EP_FIFO_SINGLE(2, FIFO_TX, 512),
> @@ -652,18 +660,18 @@ static struct musb_fifo_cfg sunxi_musb_mode_cfg_h3[] =
> { MUSB_EP_FIFO_SINGLE(4, FIFO_RX, 512),
>  };
> 
> -static const struct musb_hdrc_config sunxi_musb_hdrc_config = {
> -	.fifo_cfg       = sunxi_musb_mode_cfg,
> -	.fifo_cfg_size  = ARRAY_SIZE(sunxi_musb_mode_cfg),
> +static const struct musb_hdrc_config sunxi_musb_hdrc_config_5eps = {
> +	.fifo_cfg       = sunxi_musb_mode_cfg_5eps,
> +	.fifo_cfg_size  = ARRAY_SIZE(sunxi_musb_mode_cfg_5eps),
>  	.multipoint	= true,
>  	.dyn_fifo	= true,
>  	.num_eps	= SUNXI_MUSB_MAX_EP_NUM,
>  	.ram_bits	= SUNXI_MUSB_RAM_BITS,
>  };
> 
> -static struct musb_hdrc_config sunxi_musb_hdrc_config_h3 = {
> -	.fifo_cfg       = sunxi_musb_mode_cfg_h3,
> -	.fifo_cfg_size  = ARRAY_SIZE(sunxi_musb_mode_cfg_h3),
> +static struct musb_hdrc_config sunxi_musb_hdrc_config_4eps = {

While at it, you can mark above struct as const. 5eps struct is already marked 
as const.

> +	.fifo_cfg       = sunxi_musb_mode_cfg_4eps,
> +	.fifo_cfg_size  = ARRAY_SIZE(sunxi_musb_mode_cfg_4eps),
>  	.multipoint	= true,
>  	.dyn_fifo	= true,
>  	.num_eps	= SUNXI_MUSB_MAX_EP_NUM_H3,
> @@ -677,6 +685,7 @@ static int sunxi_musb_probe(struct platform_device
> *pdev) struct platform_device_info	pinfo;
>  	struct sunxi_glue		*glue;
>  	struct device_node		*np = pdev->dev.of_node;
> +	const struct sunxi_musb_cfg	*cfg;
>  	int ret;
> 
>  	if (!np) {
> @@ -713,29 +722,35 @@ static int sunxi_musb_probe(struct platform_device
> *pdev) return -EINVAL;
>  	}
>  	pdata.platform_ops	= &sunxi_musb_ops;
> -	if (!of_device_is_compatible(np, "allwinner,sun8i-h3-musb"))
> -		pdata.config = &sunxi_musb_hdrc_config;
> -	else
> -		pdata.config = &sunxi_musb_hdrc_config_h3;
> +
> +	cfg = of_device_get_match_data(&pdev->dev);
> +	if (!cfg)
> +		return -EINVAL;
> +
> +	switch (cfg->nr_endpoints) {
> +	case 4:
> +		pdata.config = &sunxi_musb_hdrc_config_4eps;
> +		break;
> +	case 5:
> +		pdata.config = &sunxi_musb_hdrc_config_5eps;
> +		break;
> +	default:
> +		dev_err(&pdev->dev, "Only 4 or 5 endpoints 
supported\n");
> +		return -EINVAL;
> +	}

Overall nice cleanup! Only thing I would rather see different is to use pointer 
to struct musb_fifo_cfg directly in config struct. That way you avoid above 
switch case.

Best regards,
Jernej

> 
>  	glue->dev = &pdev->dev;
>  	INIT_WORK(&glue->work, sunxi_musb_work);
>  	glue->host_nb.notifier_call = sunxi_musb_host_notifier;
> 
> -	if (of_device_is_compatible(np, "allwinner,sun4i-a10-musb") ||
> -	    of_device_is_compatible(np, "allwinner,suniv-f1c100s-musb")) {
> +	if (cfg->has_sram)
>  		set_bit(SUNXI_MUSB_FL_HAS_SRAM, &glue->flags);
> -	}
> 
> -	if (of_device_is_compatible(np, "allwinner,sun6i-a31-musb"))
> +	if (cfg->has_reset)
>  		set_bit(SUNXI_MUSB_FL_HAS_RESET, &glue->flags);
> 
> -	if (of_device_is_compatible(np, "allwinner,sun8i-a33-musb") ||
> -	    of_device_is_compatible(np, "allwinner,sun8i-h3-musb") ||
> -	    of_device_is_compatible(np, "allwinner,suniv-f1c100s-musb")) {
> -		set_bit(SUNXI_MUSB_FL_HAS_RESET, &glue->flags);
> +	if (cfg->no_configdata)
>  		set_bit(SUNXI_MUSB_FL_NO_CONFIGDATA, &glue->flags);
> -	}
> 
>  	glue->clk = devm_clk_get(&pdev->dev, NULL);
>  	if (IS_ERR(glue->clk)) {
> @@ -813,12 +828,46 @@ static int sunxi_musb_remove(struct platform_device
> *pdev) return 0;
>  }
> 
> +static const struct sunxi_musb_cfg sun4i_a10_musb_cfg = {
> +	.nr_endpoints = 5,
> +	.has_sram = true,
> +};
> +
> +static const struct sunxi_musb_cfg sun6i_a31_musb_cfg = {
> +	.nr_endpoints = 5,
> +	.has_reset = true,
> +};
> +
> +static const struct sunxi_musb_cfg sun8i_a33_musb_cfg = {
> +	.nr_endpoints = 5,
> +	.has_reset = true,
> +	.no_configdata = true,
> +};
> +
> +static const struct sunxi_musb_cfg sun8i_h3_musb_cfg = {
> +	.nr_endpoints = 4,
> +	.has_reset = true,
> +	.no_configdata = true,
> +};
> +
> +static const struct sunxi_musb_cfg suniv_f1c100s_musb_cfg = {
> +	.nr_endpoints = 5,
> +	.has_sram = true,
> +	.has_reset = true,
> +	.no_configdata = true,
> +};
> +
>  static const struct of_device_id sunxi_musb_match[] = {
> -	{ .compatible = "allwinner,sun4i-a10-musb", },
> -	{ .compatible = "allwinner,sun6i-a31-musb", },
> -	{ .compatible = "allwinner,sun8i-a33-musb", },
> -	{ .compatible = "allwinner,sun8i-h3-musb", },
> -	{ .compatible = "allwinner,suniv-f1c100s-musb", },
> +	{ .compatible = "allwinner,sun4i-a10-musb",
> +	  .data = &sun4i_a10_musb_cfg, },
> +	{ .compatible = "allwinner,sun6i-a31-musb",
> +	  .data = &sun6i_a31_musb_cfg, },
> +	{ .compatible = "allwinner,sun8i-a33-musb",
> +	  .data = &sun8i_a33_musb_cfg, },
> +	{ .compatible = "allwinner,sun8i-h3-musb",
> +	  .data = &sun8i_h3_musb_cfg, },
> +	{ .compatible = "allwinner,suniv-f1c100s-musb",
> +	  .data = &suniv_f1c100s_musb_cfg, },
>  	{}
>  };
>  MODULE_DEVICE_TABLE(of, sunxi_musb_match);
> --
> 2.35.5