| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <5d8690e4-a4ec-4ee9-4bed-bc6bf32ae80b@collabora.com>
Date: Tue, 26 Jul 2022 10:11:43 +0200
From: AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>
To: Chen-Yu Tsai <wenst@...omium.org>
Cc: matthias.bgg@...il.com, robh+dt@...nel.org,
krzysztof.kozlowski+dt@...aro.org,
linux-arm-kernel@...ts.infradead.org,
linux-mediatek@...ts.infradead.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 4/8] arm64: dts: mediatek: cherry: Enable secondary
SD/MMC controller
Il 26/07/22 06:04, Chen-Yu Tsai ha scritto:
> On Mon, Jul 25, 2022 at 6:20 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@...labora.com> wrote:
>>
>> Il 25/07/22 10:54, Chen-Yu Tsai ha scritto:
>>> On Thu, Jul 21, 2022 at 10:51 PM AngeloGioacchino Del Regno
>>> <angelogioacchino.delregno@...labora.com> wrote:
>>>>
>>>> As of now, all of the boards based on the cherry platform have a
>>>> usable secondary SD/MMC controller, usually for SD cards: enable
>>>> it to allow both booting from it and generally accessing external
>>>> storage.
>>>>
>>>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
>>>> ---
>>>> .../boot/dts/mediatek/mt8195-cherry.dtsi | 62 +++++++++++++++++++
>>>> 1 file changed, 62 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8195-cherry.dtsi b/arch/arm64/boot/dts/mediatek/mt8195-cherry.dtsi
>>>> index 2853f7f76c90..8859957c7b27 100644
>>>> --- a/arch/arm64/boot/dts/mediatek/mt8195-cherry.dtsi
>>>> +++ b/arch/arm64/boot/dts/mediatek/mt8195-cherry.dtsi
>>>> @@ -17,6 +17,7 @@ aliases {
>>>> i2c5 = &i2c5;
>>>> i2c7 = &i2c7;
>>>> mmc0 = &mmc0;
>>>> + mmc1 = &mmc1;
>>>> serial0 = &uart0;
>>>> };
>>>>
>>>> @@ -227,6 +228,24 @@ &mmc0 {
>>>> vqmmc-supply = <&mt6359_vufs_ldo_reg>;
>>>> };
>>>>
>>>> +&mmc1 {
>>>> + status = "okay";
>>>> +
>>>> + bus-width = <4>;
>>>> + cap-sd-highspeed;
>>>> + cd-gpios = <&pio 54 GPIO_ACTIVE_LOW>;
>>>> + max-frequency = <200000000>;
>>>> + no-mmc;
>>>> + no-sdio;
>>>> + pinctrl-names = "default", "state_uhs";
>>>> + pinctrl-0 = <&mmc1_pins_default>;
>>>> + pinctrl-1 = <&mmc1_pins_uhs>;
>>>> + sd-uhs-sdr50;
>>>> + sd-uhs-sdr104;
>>>> + vmmc-supply = <&mt_pmic_vmch_ldo_reg>;
>>>> + vqmmc-supply = <&mt_pmic_vmc_ldo_reg>;
>>>> +};
>>>> +
>>>> /* for CPU-L */
>>>> &mt6359_vcore_buck_reg {
>>>> regulator-always-on;
>>>> @@ -575,6 +594,49 @@ pins-rst {
>>>> };
>>>> };
>>>>
>>>> + mmc1_pins_default: mmc1-default-pins {
>>>> + pins-cmd-dat {
>>>> + pinmux = <PINMUX_GPIO110__FUNC_MSDC1_CMD>,
>>>> + <PINMUX_GPIO112__FUNC_MSDC1_DAT0>,
>>>> + <PINMUX_GPIO113__FUNC_MSDC1_DAT1>,
>>>> + <PINMUX_GPIO114__FUNC_MSDC1_DAT2>,
>>>> + <PINMUX_GPIO115__FUNC_MSDC1_DAT3>;
>>>> + input-enable;
>>>> + drive-strength = <8>;
>>>> + bias-pull-up = <MTK_PUPD_SET_R1R0_01>;
>>>> + };
>>>> +
>>>> + pins-clk {
>>>> + pinmux = <PINMUX_GPIO111__FUNC_MSDC1_CLK>;
>>>> + drive-strength = <8>;
>>>> + bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
>>>> + };
>>>> +
>>>> + pins-insert {
>>>> + pinmux = <PINMUX_GPIO54__FUNC_GPIO54>;
>>>> + bias-pull-up;
>>>> + };
>>>> + };
>>>> +
>>>> + mmc1_pins_uhs: mmc1-uhs-pins {
>>>> + pins-cmd-dat {
>>>> + pinmux = <PINMUX_GPIO110__FUNC_MSDC1_CMD>,
>>>> + <PINMUX_GPIO112__FUNC_MSDC1_DAT0>,
>>>> + <PINMUX_GPIO113__FUNC_MSDC1_DAT1>,
>>>> + <PINMUX_GPIO114__FUNC_MSDC1_DAT2>,
>>>> + <PINMUX_GPIO115__FUNC_MSDC1_DAT3>;
>>>> + input-enable;
>>>> + drive-strength = <8>;
>>>> + bias-pull-up = <MTK_PUPD_SET_R1R0_01>;
>>>> + };
>>>> +
>>>> + pins-clk {
>>>> + pinmux = <PINMUX_GPIO111__FUNC_MSDC1_CLK>;
>>>> + drive-strength = <8>;
>>>> + bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
>>>> + };
>>>
>>> I wonder if pins-insert should be duplicated here. And there's no
>>> difference between the standard and UHS pinconfigs. One would expect
>>> higher drive strength on the UHS set, if two sets were required.
>>> So maybe we should just have one set, and use that one for both
>>> the default and uhs states.
>>>
>>
>> I don't think that it would really make a lot of sense to duplicate the
>> insertion pin setup in the UHS-specific pinctrl set...
>>
>> Whenever you remove the uSD card, the controller goes back to default,
>> as the first steps in card initialization are always happening at low
>> speed and only after that we can switch to UHS speeds... so we do expect
>> that the first-ever state is always `default` (by spec!), which means
>> that we are also ensuring that the insertion pin setup is always done.
>
> Right. What I wanted to say was that, besides the insertion pin, there's
> no difference between the default and uhs states here. So why have two
> copies instead of one that is referenced twice? (the uhs state is required
> by the binding).
>
OH!!!! Right. Yes I definitely agree with you on that one.
Let's split the pins-insert in `mmc1_pins_det: mmc1-detect-pins` and add
a reference to it in the default pinctrl, that's a sensible option.
I'll do that for v3!
> ChenYu
>
>> Cheers,
>> Angelo
>>
>>> Otherwise,
>>>
>>> Reviewed-by: Chen-Yu Tsai <wenst@...omium.org>
>>> Tested-by: Chen-Yu Tsai <wenst@...omium.org>
>>>
>>>> + };
>>>> +
>>>> nor_pins_default: nor-default-pins {
>>>> pins-ck-io {
>>>> pinmux = <PINMUX_GPIO142__FUNC_SPINOR_IO0>,
>>>> --
>>>> 2.35.1
>>>>
>>>>
>>
--
AngeloGioacchino Del Regno
Software Engineer
Collabora Ltd.
Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK
Registered in England & Wales, no. 5513718
Powered by blists - more mailing lists