[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@xxxxxxxxxx> 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 devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
