| Message ID | 20221106154826.6687-1-andre.przywara@arm.com |
|---|---|
| Headers | show |
| Series | ARM: suniv: USB and PopStick board support | expand |
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>
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 = <®_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"; > +};
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 = <®_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"; > > +}; >
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 = <®_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
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
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
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.
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 >