| Message ID | 1436348468-4126-6-git-send-email-chunfeng.yun@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Chunfeng, On Wed, Jul 8, 2015 at 5:41 PM, Chunfeng Yun <chunfeng.yun@mediatek.com> wrote: > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> > --- > arch/arm64/boot/dts/mediatek/mt8173-evb.dts | 15 +++++++++++++++ > arch/arm64/boot/dts/mediatek/mt8173.dtsi | 27 +++++++++++++++++++++++++++ > 2 files changed, 42 insertions(+) > > diff --git a/arch/arm64/boot/dts/mediatek/mt8173-evb.dts b/arch/arm64/boot/dts/mediatek/mt8173-evb.dts > index f433c21..cb63dc3 100644 > --- a/arch/arm64/boot/dts/mediatek/mt8173-evb.dts > +++ b/arch/arm64/boot/dts/mediatek/mt8173-evb.dts > @@ -13,6 +13,7 @@ > */ > > /dts-v1/; > +#include <dt-bindings/gpio/gpio.h> > #include "mt8173.dtsi" > > / { > @@ -32,6 +33,15 @@ > }; > > chosen { }; > + > + usb_p1_vbus: fixedregulator@0 { Why @0 ? > + compatible = "regulator-fixed"; > + regulator-name = "usb_vbus"; > + regulator-min-microvolt = <5000000>; > + regulator-max-microvolt = <5000000>; > + gpio = <&pio 130 GPIO_ACTIVE_HIGH>; > + enable-active-high; > + }; > }; > > &pwrap { > @@ -211,3 +221,8 @@ > &uart0 { > status = "okay"; > }; > + > +&usb { > + reg-vusb33-supply = <&mt6397_vusb_reg>; > + reg-vbus-supply = <&usb_p1_vbus>; > +}; > diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi > index 0696f8f..452bd0a 100644 > --- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi > +++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi > @@ -15,6 +15,7 @@ > #include <dt-bindings/interrupt-controller/irq.h> > #include <dt-bindings/interrupt-controller/arm-gic.h> > #include <dt-bindings/reset-controller/mt8173-resets.h> > +#include <dt-bindings/power/mt8173-power.h> Sort alphabetically, so: power before reset > #include "mt8173-pinfunc.h" > > / { > @@ -393,6 +394,32 @@ > #size-cells = <0>; > status = "disabled"; > }; > + > + usb: usb30@11270000 { > + compatible = "mediatek,mt8173-xhci"; > + reg = <0 0x11270000 0 0x1000>; > + interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_LOW>; > + usb-phy = <&u3phy>; > + usb3-lpm-capable; > + clocks = <&topckgen CLK_TOP_USB30_SEL>; > + clock-names = "sys_mac"; > + }; > + > + u3phy: usb-phy@11271000 { > + compatible = "mediatek,mt8173-u3phy"; > + reg = <0 0x11271000 0 0x3000>, > + <0 0x11280000 0 0x20000>; > + power-domains = <&scpsys MT8173_POWER_DOMAIN_USB>; > + usb-wakeup-ctrl = <&pericfg>; > + wakeup-src = <1>; > + u2port-num = <2>; Mediatek specific properties should be prefixed with "mediatek,". I also find it nicer if you put the standard properties first: (compatible, reg, clocks, clock-names, power-domains) then device specific properties: (usb-wakeup-ctrl, wakeup-src, u2port-num) and then the final: status="disabled"; This last is currently missing, so please add it, too. Also, is there some pinctrl settings that must be set for USB? Lastly, I could not quite figure out which patch this set was based on. Patch [0] mentions v4.2-rc1, however, the .dts changes in this patch did not apply cleanly. Perhaps it was based on a particular patch of Matthias tree? Thanks! -Dan > + clocks = <&pericfg CLK_PERI_USB0>, > + <&pericfg CLK_PERI_USB1>, > + <&apmixedsys CLK_APMIXED_REF2USB_TX>; > + clock-names = "wakeup_deb_p0", > + "wakeup_deb_p1", > + "u3phya_ref"; > + }; > }; > }; > > -- > 1.8.1.1.dirty > > > _______________________________________________ > Linux-mediatek mailing list > Linux-mediatek@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek
Hi, On Tue, 2015-07-14 at 18:12 +0800, Daniel Kurtz wrote: > Hi Chunfeng, > > On Wed, Jul 8, 2015 at 5:41 PM, Chunfeng Yun <chunfeng.yun@mediatek.com> wrote: > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> > > --- > > arch/arm64/boot/dts/mediatek/mt8173-evb.dts | 15 +++++++++++++++ > > arch/arm64/boot/dts/mediatek/mt8173.dtsi | 27 +++++++++++++++++++++++++++ > > 2 files changed, 42 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/mediatek/mt8173-evb.dts b/arch/arm64/boot/dts/mediatek/mt8173-evb.dts > > index f433c21..cb63dc3 100644 > > --- a/arch/arm64/boot/dts/mediatek/mt8173-evb.dts > > +++ b/arch/arm64/boot/dts/mediatek/mt8173-evb.dts > > @@ -13,6 +13,7 @@ > > */ > > > > /dts-v1/; > > +#include <dt-bindings/gpio/gpio.h> > > #include "mt8173.dtsi" > > > > / { > > @@ -32,6 +33,15 @@ > > }; > > > > chosen { }; > > + > > + usb_p1_vbus: fixedregulator@0 { > > Why @0 ? It is the first fixed regulator, so set it to 0 as a index. I will remove it later > > > + compatible = "regulator-fixed"; > > + regulator-name = "usb_vbus"; > > + regulator-min-microvolt = <5000000>; > > + regulator-max-microvolt = <5000000>; > > + gpio = <&pio 130 GPIO_ACTIVE_HIGH>; > > + enable-active-high; > > + }; > > }; > > > > &pwrap { > > @@ -211,3 +221,8 @@ > > &uart0 { > > status = "okay"; > > }; > > + > > +&usb { > > + reg-vusb33-supply = <&mt6397_vusb_reg>; > > + reg-vbus-supply = <&usb_p1_vbus>; > > +}; > > diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi > > index 0696f8f..452bd0a 100644 > > --- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi > > +++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi > > @@ -15,6 +15,7 @@ > > #include <dt-bindings/interrupt-controller/irq.h> > > #include <dt-bindings/interrupt-controller/arm-gic.h> > > #include <dt-bindings/reset-controller/mt8173-resets.h> > > +#include <dt-bindings/power/mt8173-power.h> > > Sort alphabetically, so: > power before reset > Ok > > #include "mt8173-pinfunc.h" > > > > / { > > @@ -393,6 +394,32 @@ > > #size-cells = <0>; > > status = "disabled"; > > }; > > + > > + usb: usb30@11270000 { > > + compatible = "mediatek,mt8173-xhci"; > > + reg = <0 0x11270000 0 0x1000>; > > + interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_LOW>; > > + usb-phy = <&u3phy>; > > + usb3-lpm-capable; > > + clocks = <&topckgen CLK_TOP_USB30_SEL>; > > + clock-names = "sys_mac"; > > + }; > > + > > + u3phy: usb-phy@11271000 { > > + compatible = "mediatek,mt8173-u3phy"; > > + reg = <0 0x11271000 0 0x3000>, > > + <0 0x11280000 0 0x20000>; > > + power-domains = <&scpsys MT8173_POWER_DOMAIN_USB>; > > + usb-wakeup-ctrl = <&pericfg>; > > + wakeup-src = <1>; > > + u2port-num = <2>; > > Mediatek specific properties should be prefixed with "mediatek,". > I also find it nicer if you put the standard properties first: > (compatible, reg, clocks, clock-names, power-domains) > then device specific properties: > (usb-wakeup-ctrl, wakeup-src, u2port-num) > and then the final: > status="disabled"; > > This last is currently missing, so please add it, too. > I will revise it. Thanks a lot. > Also, is there some pinctrl settings that must be set for USB? > There is no special pinctrl setting for USB on EVB board on which driver is tested. > Lastly, I could not quite figure out which patch this set was based on. > Patch [0] mentions v4.2-rc1, however, the .dts changes in this patch > did not apply cleanly. > Perhaps it was based on a particular patch of Matthias tree? > Yes, it is. > Thanks! > -Dan > > > + clocks = <&pericfg CLK_PERI_USB0>, > > + <&pericfg CLK_PERI_USB1>, > > + <&apmixedsys CLK_APMIXED_REF2USB_TX>; > > + clock-names = "wakeup_deb_p0", > > + "wakeup_deb_p1", > > + "u3phya_ref"; > > + }; > > }; > > }; > > > > -- > > 1.8.1.1.dirty > > > > > > _______________________________________________ > > Linux-mediatek mailing list > > Linux-mediatek@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-mediatek
Hi Daniel, Chunfeng, On Mon, 2015-07-20 at 22:39 +0800, chunfeng yun wrote: > Hi, > > On Tue, 2015-07-14 at 18:12 +0800, Daniel Kurtz wrote: <...> > > > + > > > + usb_p1_vbus: fixedregulator@0 { > > > > Why @0 ? > It is the first fixed regulator, so set it to 0 as a index. > I will remove it later Since this name should be unique, I think we should use gpio number as address for fixedregulator in case we need other fixedregulators. So this could be: usb_p1_vbus: fixedregulator@130 { Also, we should sort according to the address(gpio number) for these nodes. Does this make sense? Joe.C > > > > > + compatible = "regulator-fixed"; > > > + regulator-name = "usb_vbus"; > > > + regulator-min-microvolt = <5000000>; > > > + regulator-max-microvolt = <5000000>; > > > + gpio = <&pio 130 GPIO_ACTIVE_HIGH>; > > > + enable-active-high; > > > + };
On Mon, Jul 20, 2015 at 10:51 PM, Yingjoe Chen <yingjoe.chen@mediatek.com> wrote: > > Hi Daniel, Chunfeng, > > > On Mon, 2015-07-20 at 22:39 +0800, chunfeng yun wrote: >> Hi, >> >> On Tue, 2015-07-14 at 18:12 +0800, Daniel Kurtz wrote: > <...> >> > > + >> > > + usb_p1_vbus: fixedregulator@0 { >> > >> > Why @0 ? >> It is the first fixed regulator, so set it to 0 as a index. >> I will remove it later > > Since this name should be unique, I think we should use gpio number as > address for fixedregulator in case we need other fixedregulators. So > this could be: > > usb_p1_vbus: fixedregulator@130 { > > Also, we should sort according to the address(gpio number) for these > nodes. Does this make sense? Yes - that makes some sense. But, a quick check of other "fixedregulator@" nodes in arch/arm/boot/dts/ shows that mostly they use @0 , @1. So, I think the original patch is probably fine. Sorry for the noise, -Dan > > Joe.C > >> > >> > > + compatible = "regulator-fixed"; >> > > + regulator-name = "usb_vbus"; >> > > + regulator-min-microvolt = <5000000>; >> > > + regulator-max-microvolt = <5000000>; >> > > + gpio = <&pio 130 GPIO_ACTIVE_HIGH>; >> > > + enable-active-high; >> > > + }; > >
diff --git a/arch/arm64/boot/dts/mediatek/mt8173-evb.dts b/arch/arm64/boot/dts/mediatek/mt8173-evb.dts index f433c21..cb63dc3 100644 --- a/arch/arm64/boot/dts/mediatek/mt8173-evb.dts +++ b/arch/arm64/boot/dts/mediatek/mt8173-evb.dts @@ -13,6 +13,7 @@ */ /dts-v1/; +#include <dt-bindings/gpio/gpio.h> #include "mt8173.dtsi" / { @@ -32,6 +33,15 @@ }; chosen { }; + + usb_p1_vbus: fixedregulator@0 { + compatible = "regulator-fixed"; + regulator-name = "usb_vbus"; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + gpio = <&pio 130 GPIO_ACTIVE_HIGH>; + enable-active-high; + }; }; &pwrap { @@ -211,3 +221,8 @@ &uart0 { status = "okay"; }; + +&usb { + reg-vusb33-supply = <&mt6397_vusb_reg>; + reg-vbus-supply = <&usb_p1_vbus>; +}; diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi index 0696f8f..452bd0a 100644 --- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi +++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi @@ -15,6 +15,7 @@ #include <dt-bindings/interrupt-controller/irq.h> #include <dt-bindings/interrupt-controller/arm-gic.h> #include <dt-bindings/reset-controller/mt8173-resets.h> +#include <dt-bindings/power/mt8173-power.h> #include "mt8173-pinfunc.h" / { @@ -393,6 +394,32 @@ #size-cells = <0>; status = "disabled"; }; + + usb: usb30@11270000 { + compatible = "mediatek,mt8173-xhci"; + reg = <0 0x11270000 0 0x1000>; + interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_LOW>; + usb-phy = <&u3phy>; + usb3-lpm-capable; + clocks = <&topckgen CLK_TOP_USB30_SEL>; + clock-names = "sys_mac"; + }; + + u3phy: usb-phy@11271000 { + compatible = "mediatek,mt8173-u3phy"; + reg = <0 0x11271000 0 0x3000>, + <0 0x11280000 0 0x20000>; + power-domains = <&scpsys MT8173_POWER_DOMAIN_USB>; + usb-wakeup-ctrl = <&pericfg>; + wakeup-src = <1>; + u2port-num = <2>; + clocks = <&pericfg CLK_PERI_USB0>, + <&pericfg CLK_PERI_USB1>, + <&apmixedsys CLK_APMIXED_REF2USB_TX>; + clock-names = "wakeup_deb_p0", + "wakeup_deb_p1", + "u3phya_ref"; + }; }; };
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> --- arch/arm64/boot/dts/mediatek/mt8173-evb.dts | 15 +++++++++++++++ arch/arm64/boot/dts/mediatek/mt8173.dtsi | 27 +++++++++++++++++++++++++++ 2 files changed, 42 insertions(+)