Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   "Alim Akhtar" <alim.akhtar@samsung.com>
To:     "'Henrik Grimler'" <henrik@grimler.se>
Cc:     "'Krzysztof Kozlowski'" <krzysztof.kozlowski@canonical.com>,
        <semen.protsenko@linaro.org>, <virag.david003@gmail.com>,
        <martin.juecker@gmail.com>, <cw00.choi@samsung.com>,
        <m.szyprowski@samsung.com>, <robh+dt@kernel.org>,
        <devicetree@vger.kernel.org>, <linux-samsung-soc@vger.kernel.org>,
        <linux-arm-kernel@lists.infradead.org>,
        <linux-kernel@vger.kernel.org>,
        <~postmarketos/upstreaming@lists.sr.ht>
In-Reply-To: <YeWgNJaDHxKROpQs@grimlerstat>
Subject: RE: [PATCH v3 3/3] ARM: dts: Add support for Samsung Chagall WiFi
Date:   Tue, 18 Jan 2022 12:56:30 +0530
Message-ID: <01c501d80c3c$b820fb90$2862f2b0$@samsung.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Thread-Index: AQKTrlzLhJbN5e22s0Ks3rGJhMIi0gCNReXFAo1zs98Cp4BzmQGZbeD4AaJ5xbYBhx1hmaqc71SA
Content-Language: en-us
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 105P
DLP-Filter: Pass
References: <20220116165035.437274-1-henrik@grimler.se>
        <20220116165035.437274-4-henrik@grimler.se>
        <ca8c4613-a058-6cde-f9e6-8530f142a821@canonical.com>
        <CGME20220116205940epcas5p3dbab01fb6bb7a4af76169231e6d94469@epcas5p3.samsung.com>
        <YeSHNCywXhp8gHC7@L14.lan> <001301d80b66$190901a0$4b1b04e0$@samsung.com>
        <YeWgNJaDHxKROpQs@grimlerstat>
Precedence: bulk

Hi Henrik

>-----Original Message-----
>From: Henrik Grimler [mailto:henrik@grimler.se]
>Sent: Monday, January 17, 2022 10:28 PM
>To: Alim Akhtar <alim.akhtar@samsung.com>
>Cc: 'Krzysztof Kozlowski' <krzysztof.kozlowski@canonical.com>;
>semen.protsenko@linaro.org; virag.david003@gmail.com;
>martin.juecker@gmail.com; cw00.choi@samsung.com;
>m.szyprowski@samsung.com; robh+dt@kernel.org;
>devicetree@vger.kernel.org; linux-samsung-soc@vger.kernel.org; linux-arm-
>kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
>~postmarketos/upstreaming@lists.sr.ht
>Subject: Re: [PATCH v3 3/3] ARM: dts: Add support for Samsung Chagall WiFi
>
>Hi Alim,
>
>> >> > +/* External sdcard */
>> >> > +&mmc_2 {
>> >> > +	status = "okay";
>> >> > +	bus-width = <4>;
>> >> > +	cap-sd-highspeed;
>> >> > +	card-detect-delay = <200>;
>> >> > +	pinctrl-0 = <&sd2_clk &sd2_cmd &mmc2_cd &sd2_bus1
>&sd2_bus4>;
>> >> > +	pinctrl-names = "default";
>> >> > +	samsung,dw-mshc-ciu-div = <3>;
>> >> > +	samsung,dw-mshc-ddr-timing = <0 2>;
>> >> > +	samsung,dw-mshc-sdr-timing = <0 4>;
>> >> > +	sd-uhs-sdr50;
>> >> > +	vmmc-supply = <&ldo19_reg>;
>> >> > +	vqmmc-supply = <&ldo13_reg>;
>> >> > +};
>> >> > +
>> >> > +&pinctrl_0 {
>> >> > +	mmc2_cd: sd2-cd-pins {
>> >> > +		samsung,pins = "gpx2-4";
>> >>
>> >> Interesting... I looked at vendor sources to board-chagall and
>> >> standard pin gpc2-2 is mentioned as PULL down and not-connected
>> >comment.
>> >>
>> >> gpx2-4 seems not mentioned at all, unless other board files are
>> >> actually used.
>> >
>> >Gpio seems to be spread out. GPIO_T_FLASH_DETECT is defined as gpx2-4
>> >in board-universal5420-mmc.c, and then used for card detection.
>> >(Looking at it now again I see that write protection through sd2_wp
>> >should also be supported for mmc_2, can add that in next patch set.)
>> >
>> Card detect pin is generally a special function (dedicated pin) pin,
>> directly connected from SoC to card external slot.
>> And for exynos5420 it is gpc2-2 as pointed by Krzysztof.
>
>gpio/pinctrl is still not my area of expertise so your feedback is greatly
>appreciated!
>
>gpc2-2 does not seem to be the card-detect pin for these tablets.  If I add
>sd2_cd to pinctrl-0 above I am unable to use the sdcard at all, instead
dmesg is
>filled with repeating messages like:
>
>[  18.669050] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot req
>300000Hz, actual 297619HZ div = 84) [  18.703711] mmc_host mmc1: Bus speed
>(slot 0) = 50000000Hz (slot req 200000Hz, actual 200000HZ div = 125) [
>18.737167] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot req
>100000Hz, actual 100000HZ div = 250) [  19.058352] mmc_host mmc1: Bus
>speed (slot 0) = 50000000Hz (slot req 400000Hz, actual 396825HZ div = 63) [
>19.093037] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot req
>300000Hz, actual 297619HZ div = 84) [  19.127701] mmc_host mmc1: Bus speed
>(slot 0) = 50000000Hz (slot req 200000Hz, actual 200000HZ div = 125) [
>19.162380] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot req
>100000Hz, actual 100000HZ div = 250)
>
>gpx2-4 on the other hand switches state when an sdcard is inserted or
>removed.  After exporting relevant pins I have, with the card
>inserted:
>
># grep -A 2 gpx2 /sys/kernel/debug/gpio
>gpiochip3: GPIOs 24-31, parent: platform/13400000.pinctrl, gpx2:
> gpio-26  (                    |Power               ) in  hi IRQ ACTIVE LOW
> gpio-28  (                    |sysfs               ) in  lo
>
>and when removed gpx2-4 changes to high:
>
>gpiochip3: GPIOs 24-31, parent: platform/13400000.pinctrl, gpx2:
> gpio-26  (                    |Power               ) in  hi IRQ ACTIVE LOW
> gpio-28  (                    |sysfs               ) in  hi
>
>All the gpc2 pins are low no matter if sdcard is inserted or removed:
>
>gpiochip7: GPIOs 56-62, parent: platform/13410000.pinctrl, gpc2:
> gpio-56  (                    |sysfs               ) in  lo
> gpio-57  (                    |sysfs               ) in  lo
> gpio-58  (                    |sysfs               ) in  lo
> gpio-59  (                    |sysfs               ) in  lo
> gpio-60  (                    |sysfs               ) in  lo
> gpio-61  (                    |sysfs               ) in  lo
> gpio-62  (                    |sysfs               ) in  lo
>
>That being said, I am not sure if it is working properly:
>
>> Also PIN_FUNC_2 for gpx2-4 indicate that it is wakeup_int2 function.
>> Do you get a card detect interrupt when removing and inserting the
>> card to the card slot?
> 
This is the confirmation that this board does not uses gpc2-2 but gpx2-4. 
Thanks for detailed log.

>(What is the best way to check this?) I don't get any irq related messages
in

Probably if you are interested, you can enable few debug in dw-mmc driver to
check a message like "card is present" etc.

>dmesg, and I don't see any changes in /proc/interrupts if tracked with
>something like `watch -n1 "cat /proc/interrupts"` when card is removed or
>inserted, so seems likely that pin config is wrong.
>All I can find in vendor kernel for this device is that pull config is set
to
>S3C_GPIO_PULL_NONE [1].  For some of the sibling tablets the pin is
>configured as {S3C_GPIO_INPUT, GPIO_LV_N, S3C_GPIO_PULL_NONE} [2]
>though, which I think should correspond to having:
>
>mmc2_cd: mmc2-cd-pins {
>  samsung,pins = "gpx2-4";
>  samsung,pin-function = <EXYNOS_PIN_FUNC_INPUT>;
>  samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
>  samsung,pin-drv = <EXYNOS5420_PIN_DRV_LV3>; };
>
>Changing to that does not seem to change anything though, I don't get any
>interrupt, or any messages in dmesg, when inserting or removing the sdcard.
>
>> Also to avoid confusion, probably you can change the node name as
>"mmc2_cd:
>> mmc2-cd-pins"
>> (as sd2-cd-pins is gpc2-2 for this SoC)
>
>Thanks, will fix in a v4 (after discussions here are settled).
>
I am convinced that gpc2-2 is not used for card detect for this board.
So no other query on this patch.
Thanks

>[1] https://protect2.fireeye.com/v1/url?k=bba823ae-dad5cbe9-bba9a8e1-
>74fe485fff30-b4ccbbe9776f501d&q=1&e=9951c78c-16c8-449e-913b-
>d4d1f1f9bb6b&u=https%3A%2F%2Fgithub.com%2Fexynos5420%2Fandroid_k
>ernel_samsung_exynos5420%2Fblob%2Flineage-
>17.1%2Farch%2Farm%2Fmach-exynos%2Fboard-universal5420-
>mmc.c%23L388
>[2] https://protect2.fireeye.com/v1/url?k=97465c6e-f63bb429-9747d721-
>74fe485fff30-4111389a6d2b68b4&q=1&e=9951c78c-16c8-449e-913b-
>d4d1f1f9bb6b&u=https%3A%2F%2Fgithub.com%2Fexynos5420%2Fandroid_k
>ernel_samsung_exynos5420%2Fblob%2Flineage-
>17.1%2Farch%2Farm%2Fmach-exynos%2Fboard-n1-gpio.c%23L119
>
>Best regards,
>Henrik Grimler