| Message ID | 20180129031640.17939-1-mlyle@lyle.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Mon, Jan 29, 2018 at 4:16 AM, Michael Lyle <mlyle@lyle.org> wrote: > On the Artik520 module, there's a bcm4354 attached to mshc_1. Enable > it, and turn on the regulator used for it, so that both wifi & bluetooth > work. > > Verified to work on the Artik 520 evaluation board. > > Signed-off-by: Michael Lyle <mlyle@lyle.org> Hi, Thanks for the patch. Few notes below. > --- > arch/arm/boot/dts/exynos3250-artik5.dtsi | 36 ++++++++++++++++++++++++++++++++ > 1 file changed, 36 insertions(+) > > diff --git a/arch/arm/boot/dts/exynos3250-artik5.dtsi b/arch/arm/boot/dts/exynos3250-artik5.dtsi > index 0aa577fe9f95..b2d441b1a7e3 100644 > --- a/arch/arm/boot/dts/exynos3250-artik5.dtsi > +++ b/arch/arm/boot/dts/exynos3250-artik5.dtsi > @@ -245,6 +245,7 @@ > regulator-name = "VLDO23_1.8V"; > regulator-min-microvolt = <1800000>; > regulator-max-microvolt = <1800000>; > + regulator-always-on; > }; > > ldo24_reg: LDO24 { > @@ -316,6 +317,41 @@ > status = "okay"; > }; > > +&pinctrl_1 { Please order the nodes alphabetically. > + wlanen: wlanen { > + samsung,pins = "gpx2-3"; > + samsung,pin-function = <EXYNOS_PIN_FUNC_OUTPUT>; > + samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>; > + samsung,pin-drv = <EXYNOS4_PIN_DRV_LV3>; > + samsung,pin-val = <1>; > + }; Are you sure you do not need a power sequence? It works fine during warm resets or after u-boot initialization? > +}; > + > +&mshc_1 { > + cap-sd-highspeed; > + cap-sdio-irq; > + disable-wp; > + broken-cd; Is this really broken-cd or non-removable card? > + bypass-smu; > + keep-power-in-suspend; > + fifo-depth = <0x40>; > + vqmmc-supply = <&ldo11_reg>; > + /* Voltage negotiation is broken for the SDIO periph so we > + * can't actually set the voltage here. > + * vmmc-supply = <&ldo23_reg>; > + */ Did you try using properties for respective voltage? For example mmc-hs200-1_8v? > + card-detect-delay = <500>; > + clock-frequency = <100000000>; > + max-frequency = <100000000>; > + samsung,dw-mshc-ciu-div = <3>; > + samsung,dw-mshc-sdr-timing = <0 1>; > + samsung,dw-mshc-ddr-timing = <1 2>; > + pinctrl-names = "default"; > + pinctrl-0 = <&sd1_cmd &sd1_clk &sd1_bus1 &sd1_bus4 &wlanen>; > + bus-width = <4>; > + status = "okay"; There is no brcm/wifi node here? Best regards, Krzysztof > +}; > + > &rtc { > clocks = <&cmu CLK_RTC>, <&s2mps14_osc S2MPS11_CLK_AP>; > clock-names = "rtc", "rtc_src"; > -- > 2.14.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[note: earlier on this thread I assumed linux-arm-kernel was on vger.kernel.org, so the patch was initially sent to the wrong address. When I resend, I will be sure to use the correct list address] Krzystztof-- Thank you for your review. I appreciate your patience as I've not worked on devicetree files before. On Mon, Jan 29, 2018 at 7:16 AM, Krzysztof Kozlowski <krzk@kernel.org> wrote: > Hi, > > Thanks for the patch. Few notes below. > >> --- >> arch/arm/boot/dts/exynos3250-artik5.dtsi | 36 ++++++++++++++++++++++++++++++++ >> 1 file changed, 36 insertions(+) >> >> diff --git a/arch/arm/boot/dts/exynos3250-artik5.dtsi b/arch/arm/boot/dts/exynos3250-artik5.dtsi >> index 0aa577fe9f95..b2d441b1a7e3 100644 >> --- a/arch/arm/boot/dts/exynos3250-artik5.dtsi >> +++ b/arch/arm/boot/dts/exynos3250-artik5.dtsi >> @@ -245,6 +245,7 @@ >> regulator-name = "VLDO23_1.8V"; >> regulator-min-microvolt = <1800000>; >> regulator-max-microvolt = <1800000>; >> + regulator-always-on; >> }; >> >> ldo24_reg: LDO24 { >> @@ -316,6 +317,41 @@ >> status = "okay"; >> }; >> >> +&pinctrl_1 { > > Please order the nodes alphabetically. OK, I will in an V2. >> + wlanen: wlanen { >> + samsung,pins = "gpx2-3"; >> + samsung,pin-function = <EXYNOS_PIN_FUNC_OUTPUT>; >> + samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>; >> + samsung,pin-drv = <EXYNOS4_PIN_DRV_LV3>; >> + samsung,pin-val = <1>; >> + }; > > Are you sure you do not need a power sequence? It works fine during > warm resets or after u-boot initialization? Yes, it seems to (I have about 20 or so cycles on it in informal test). I have no idea what the actual hardware is-- this pin is not mentioned in the module datasheet and there is no schematic. I found it through exploring an upstream vendor kernel. It's altogether possible this is "naughty" but it seems to work OK. >> +}; >> + >> +&mshc_1 { >> + cap-sd-highspeed; >> + cap-sdio-irq; >> + disable-wp; >> + broken-cd; > > Is this really broken-cd or non-removable card? I'll check if non-removable works. I remember it not working once, but I had other problems in my testing. >> + bypass-smu; >> + keep-power-in-suspend; >> + fifo-depth = <0x40>; >> + vqmmc-supply = <&ldo11_reg>; >> + /* Voltage negotiation is broken for the SDIO periph so we >> + * can't actually set the voltage here. >> + * vmmc-supply = <&ldo23_reg>; >> + */ > > Did you try using properties for respective voltage? For example mmc-hs200-1_8v? This is the vmmc supply, not vqmmc-- that would be for vqmmc, right (and signalling speed). The card says it wants a range of vmmc voltages, but the actual regulator is forced to 1.8V, and 1.8V is not one of those voltages. So absent a new quirk, I think I need to nail the regulator on. > >> + card-detect-delay = <500>; >> + clock-frequency = <100000000>; >> + max-frequency = <100000000>; >> + samsung,dw-mshc-ciu-div = <3>; >> + samsung,dw-mshc-sdr-timing = <0 1>; >> + samsung,dw-mshc-ddr-timing = <1 2>; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&sd1_cmd &sd1_clk &sd1_bus1 &sd1_bus4 &wlanen>; >> + bus-width = <4>; >> + status = "okay"; > > There is no brcm/wifi node here? It enumerates the card currently and then uses the card product information to load brcmfmac. Is there a reason to put a description of the actual card in? > > Best regards, > Krzysztof > Thanks, Mike -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 29, 2018 at 7:54 PM, Michael Lyle <mlyle@lyle.org> wrote: > [note: earlier on this thread I assumed linux-arm-kernel was on > vger.kernel.org, so the patch was initially sent to the wrong address. > When I resend, I will be sure to use the correct list address] > > Krzystztof-- > > Thank you for your review. I appreciate your patience as I've not > worked on devicetree files before. > > On Mon, Jan 29, 2018 at 7:16 AM, Krzysztof Kozlowski <krzk@kernel.org> wrote: >> Hi, >> >> Thanks for the patch. Few notes below. >> >>> --- >>> arch/arm/boot/dts/exynos3250-artik5.dtsi | 36 ++++++++++++++++++++++++++++++++ >>> 1 file changed, 36 insertions(+) >>> >>> diff --git a/arch/arm/boot/dts/exynos3250-artik5.dtsi b/arch/arm/boot/dts/exynos3250-artik5.dtsi >>> index 0aa577fe9f95..b2d441b1a7e3 100644 >>> --- a/arch/arm/boot/dts/exynos3250-artik5.dtsi >>> +++ b/arch/arm/boot/dts/exynos3250-artik5.dtsi >>> @@ -245,6 +245,7 @@ >>> regulator-name = "VLDO23_1.8V"; >>> regulator-min-microvolt = <1800000>; >>> regulator-max-microvolt = <1800000>; >>> + regulator-always-on; >>> }; >>> >>> ldo24_reg: LDO24 { >>> @@ -316,6 +317,41 @@ >>> status = "okay"; >>> }; >>> >>> +&pinctrl_1 { >> >> Please order the nodes alphabetically. > > OK, I will in an V2. > >>> + wlanen: wlanen { >>> + samsung,pins = "gpx2-3"; >>> + samsung,pin-function = <EXYNOS_PIN_FUNC_OUTPUT>; >>> + samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>; >>> + samsung,pin-drv = <EXYNOS4_PIN_DRV_LV3>; >>> + samsung,pin-val = <1>; >>> + }; >> >> Are you sure you do not need a power sequence? It works fine during >> warm resets or after u-boot initialization? > > Yes, it seems to (I have about 20 or so cycles on it in informal test). > > I have no idea what the actual hardware is-- this pin is not mentioned > in the module datasheet and there is no schematic. I found it through > exploring an upstream vendor kernel. It's altogether possible this is > "naughty" but it seems to work OK. The pin is WL_REG_ON so your configuration is correct. Although other DTSes add usually a power sequence which forces a hard-reset before initialization of device. If it works fine like this, then I am okay with it. > >>> +}; >>> + >>> +&mshc_1 { >>> + cap-sd-highspeed; >>> + cap-sdio-irq; >>> + disable-wp; >>> + broken-cd; >> >> Is this really broken-cd or non-removable card? > > I'll check if non-removable works. I remember it not working once, > but I had other problems in my testing. > >>> + bypass-smu; >>> + keep-power-in-suspend; >>> + fifo-depth = <0x40>; >>> + vqmmc-supply = <&ldo11_reg>; >>> + /* Voltage negotiation is broken for the SDIO periph so we >>> + * can't actually set the voltage here. >>> + * vmmc-supply = <&ldo23_reg>; >>> + */ >> >> Did you try using properties for respective voltage? For example mmc-hs200-1_8v? > > This is the vmmc supply, not vqmmc-- that would be for vqmmc, right > (and signalling speed). The card says it wants a range of vmmc > voltages, but the actual regulator is forced to 1.8V, and 1.8V is not > one of those voltages. So absent a new quirk, I think I need to nail > the regulator on. Having vmmc and vqmmc at 1.8V should be possible, for example all other Exynos54xx Odroids use such configuration (although for eMMC). However you mentioned that the card asks for different voltage? > >> >>> + card-detect-delay = <500>; >>> + clock-frequency = <100000000>; >>> + max-frequency = <100000000>; >>> + samsung,dw-mshc-ciu-div = <3>; >>> + samsung,dw-mshc-sdr-timing = <0 1>; >>> + samsung,dw-mshc-ddr-timing = <1 2>; >>> + pinctrl-names = "default"; >>> + pinctrl-0 = <&sd1_cmd &sd1_clk &sd1_bus1 &sd1_bus4 &wlanen>; >>> + bus-width = <4>; >>> + status = "okay"; >> >> There is no brcm/wifi node here? > > It enumerates the card currently and then uses the card product > information to load brcmfmac. Is there a reason to put a description > of the actual card in? This is probably non-pluggable card so adding simple brcm node would be good for full description of device... although it is not necessary, I think. The node might be needed to specify waking up interrupts (WL_DEV_WAKE which probably is GPX3-2. See the Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt and example in: Documentation/devicetree/bindings/mmc/mmc.txt Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/boot/dts/exynos3250-artik5.dtsi b/arch/arm/boot/dts/exynos3250-artik5.dtsi index 0aa577fe9f95..b2d441b1a7e3 100644 --- a/arch/arm/boot/dts/exynos3250-artik5.dtsi +++ b/arch/arm/boot/dts/exynos3250-artik5.dtsi @@ -245,6 +245,7 @@ regulator-name = "VLDO23_1.8V"; regulator-min-microvolt = <1800000>; regulator-max-microvolt = <1800000>; + regulator-always-on; }; ldo24_reg: LDO24 { @@ -316,6 +317,41 @@ status = "okay"; }; +&pinctrl_1 { + wlanen: wlanen { + samsung,pins = "gpx2-3"; + samsung,pin-function = <EXYNOS_PIN_FUNC_OUTPUT>; + samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>; + samsung,pin-drv = <EXYNOS4_PIN_DRV_LV3>; + samsung,pin-val = <1>; + }; +}; + +&mshc_1 { + cap-sd-highspeed; + cap-sdio-irq; + disable-wp; + broken-cd; + bypass-smu; + keep-power-in-suspend; + fifo-depth = <0x40>; + vqmmc-supply = <&ldo11_reg>; + /* Voltage negotiation is broken for the SDIO periph so we + * can't actually set the voltage here. + * vmmc-supply = <&ldo23_reg>; + */ + card-detect-delay = <500>; + clock-frequency = <100000000>; + max-frequency = <100000000>; + samsung,dw-mshc-ciu-div = <3>; + samsung,dw-mshc-sdr-timing = <0 1>; + samsung,dw-mshc-ddr-timing = <1 2>; + pinctrl-names = "default"; + pinctrl-0 = <&sd1_cmd &sd1_clk &sd1_bus1 &sd1_bus4 &wlanen>; + bus-width = <4>; + status = "okay"; +}; + &rtc { clocks = <&cmu CLK_RTC>, <&s2mps14_osc S2MPS11_CLK_AP>; clock-names = "rtc", "rtc_src";
On the Artik520 module, there's a bcm4354 attached to mshc_1. Enable it, and turn on the regulator used for it, so that both wifi & bluetooth work. Verified to work on the Artik 520 evaluation board. Signed-off-by: Michael Lyle <mlyle@lyle.org> --- arch/arm/boot/dts/exynos3250-artik5.dtsi | 36 ++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+)