| Message ID | cover.1546981251.git.robin.murphy@arm.com |
|---|---|
| Headers | show |
| Series | NanoPC-T4 (and a bit of NanoPi M4) support | expand |
Am Dienstag, 8. Januar 2019, 22:57:27 CET schrieb Robin Murphy: > For whatever reason, the sdmmc_dectn function isn't working properly > as-is, and microSD insertion and removal goes unnoticed. Flipping the > pin into GPIO mode, however, does do the job, so let's just handle it > that way for now until someone feels inclined to figure out what GRF > voodoo or otherwise is needed for correct 'native' operation. > > Signed-off-by: Robin Murphy <robin.murphy@arm.com> > --- > arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi > index 9c723038d8f8..2a183a6af150 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi > +++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi > @@ -505,6 +505,10 @@ > sdmmc0_pwr_h: sdmmc0-pwr-h { > rockchip,pins = <0 RK_PA1 RK_FUNC_GPIO &pcfg_pull_none>; > }; > + > + sdmmc0_det_l: sdmmc0-det-l { alphabetically by node-name please, aka sdmmc0-det-* should be above sdmmc0-pwr-* If you're respinning the whole series this should be fixed, otherwise I can also do that when applying. Heiko > + rockchip,pins = <0 RK_PA7 RK_FUNC_GPIO &pcfg_pull_up>; > + }; > }; > > sdio-pwrseq { > @@ -563,9 +567,10 @@ > bus-width = <4>; > cap-sd-highspeed; > cap-mmc-highspeed; > + cd-gpios = <&gpio0 RK_PA7 GPIO_ACTIVE_LOW>; > disable-wp; > pinctrl-names = "default"; > - pinctrl-0 = <&sdmmc_bus4 &sdmmc_cd &sdmmc_clk &sdmmc_cmd>; > + pinctrl-0 = <&sdmmc_bus4 &sdmmc0_det_l &sdmmc_clk &sdmmc_cmd>; > sd-uhs-sdr104; > vmmc-supply = <&vcc3v0_sd>; > vqmmc-supply = <&vcc_sdio>; >
On 2019-01-08 10:42 pm, Heiko Stuebner wrote: > Am Dienstag, 8. Januar 2019, 22:57:27 CET schrieb Robin Murphy: >> For whatever reason, the sdmmc_dectn function isn't working properly >> as-is, and microSD insertion and removal goes unnoticed. Flipping the >> pin into GPIO mode, however, does do the job, so let's just handle it >> that way for now until someone feels inclined to figure out what GRF >> voodoo or otherwise is needed for correct 'native' operation. >> >> Signed-off-by: Robin Murphy <robin.murphy@arm.com> >> --- >> arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi >> index 9c723038d8f8..2a183a6af150 100644 >> --- a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi >> +++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi >> @@ -505,6 +505,10 @@ >> sdmmc0_pwr_h: sdmmc0-pwr-h { >> rockchip,pins = <0 RK_PA1 RK_FUNC_GPIO &pcfg_pull_none>; >> }; >> + >> + sdmmc0_det_l: sdmmc0-det-l { > > alphabetically by node-name please, > aka sdmmc0-det-* should be above sdmmc0-pwr-* Right you are, not sure how that one slipped through. > If you're respinning the whole series this should be fixed, > otherwise I can also do that when applying. I've fixed it up locally, although it might be worth holding off on this particular patch for the moment now that I've taken another look through the TRM and noticed those smoking-gun-looking bits in GRF_SIG_DETECT_CON{0,1} - I'll investigate... [ side note - do you reckon there'd be any value in bolting a debugfs interface onto the GRF driver, or is the realistic answer to just use /dev/mem like everyone else and stop having silly ideas? ] Thanks, Robin. > > > Heiko > >> + rockchip,pins = <0 RK_PA7 RK_FUNC_GPIO &pcfg_pull_up>; >> + }; >> }; >> >> sdio-pwrseq { >> @@ -563,9 +567,10 @@ >> bus-width = <4>; >> cap-sd-highspeed; >> cap-mmc-highspeed; >> + cd-gpios = <&gpio0 RK_PA7 GPIO_ACTIVE_LOW>; >> disable-wp; >> pinctrl-names = "default"; >> - pinctrl-0 = <&sdmmc_bus4 &sdmmc_cd &sdmmc_clk &sdmmc_cmd>; >> + pinctrl-0 = <&sdmmc_bus4 &sdmmc0_det_l &sdmmc_clk &sdmmc_cmd>; >> sd-uhs-sdr104; >> vmmc-supply = <&vcc3v0_sd>; >> vqmmc-supply = <&vcc_sdio>; >> > > > >
Am Mittwoch, 9. Januar 2019, 00:22:01 CET schrieb Robin Murphy: > On 2019-01-08 10:42 pm, Heiko Stuebner wrote: > > Am Dienstag, 8. Januar 2019, 22:57:27 CET schrieb Robin Murphy: > >> For whatever reason, the sdmmc_dectn function isn't working properly > >> as-is, and microSD insertion and removal goes unnoticed. Flipping the > >> pin into GPIO mode, however, does do the job, so let's just handle it > >> that way for now until someone feels inclined to figure out what GRF > >> voodoo or otherwise is needed for correct 'native' operation. > >> > >> Signed-off-by: Robin Murphy <robin.murphy@arm.com> > >> --- > >> arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi | 7 ++++++- > >> 1 file changed, 6 insertions(+), 1 deletion(-) > >> > >> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi > >> index 9c723038d8f8..2a183a6af150 100644 > >> --- a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi > >> +++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi > >> @@ -505,6 +505,10 @@ > >> sdmmc0_pwr_h: sdmmc0-pwr-h { > >> rockchip,pins = <0 RK_PA1 RK_FUNC_GPIO &pcfg_pull_none>; > >> }; > >> + > >> + sdmmc0_det_l: sdmmc0-det-l { > > > > alphabetically by node-name please, > > aka sdmmc0-det-* should be above sdmmc0-pwr-* > > Right you are, not sure how that one slipped through. > > > If you're respinning the whole series this should be fixed, > > otherwise I can also do that when applying. > > I've fixed it up locally, although it might be worth holding off on this > particular patch for the moment now that I've taken another look through > the TRM and noticed those smoking-gun-looking bits in > GRF_SIG_DETECT_CON{0,1} - I'll investigate... Personally I'm very much fine with using a cd-gpio instead of trying to bring order to the GRF "chaos" ;-) as I'm also not really sure if we gain anything with the IP-internal detect mechanism. But then do as you like :-D > [ side note - do you reckon there'd be any value in bolting a debugfs > interface onto the GRF driver, or is the realistic answer to just use > /dev/mem like everyone else and stop having silly ideas? ] I don't think it would help much ... especially when it really only outputs the register contents in the same way as mem or so does. Having to fiddle with GRF bits really should only happen in rare cases, so I guess mem should do the trick for that. And the GRF driver also was a bit controversial anyway, with the title of "dumping ground" looming over it ... so I'd like to keep it small and simple ;-) . Heiko
Am Dienstag, 8. Januar 2019, 22:57:25 CET schrieb Robin Murphy: > The nanopi4 boards differ primarily in their power trees, with the main > 5V and 3.3V rails having very different topologies on the smaller USB-C > powered boards vs. the 12V-powered T4, as well as minor variation in > other regulators related to various external connectors. > > Additionally, the recovery key is only present on the T4 - ADC_IN1 is > simply pulled high and not exposed on the other boards - and the lowest > common denominator for MMC speed is actually HS200 according to the > vendor DTs. > > Signed-off-by: Robin Murphy <robin.murphy@arm.com> this looks good to me, so only need patch2 to adapt and get reviewed. Heiko
Am Dienstag, 8. Januar 2019, 22:57:26 CET schrieb Robin Murphy: > In common with most Rockchip reference designs, NanoPC-T4 has a passive > IR receiver connected to PWM3. In lieu of a specialised driver for > PWM-based IR pulse measurement, running the pin as a GPIO with the basic > driver works perfectly well. > > Signed-off-by: Robin Murphy <robin.murphy@arm.com> easy change and of course looks good. As it doesn't apply due to coming after other changes, I'll have to wait for v2 though. Heiko
On 10/01/2019 13:26, Heiko Stuebner wrote: > Am Mittwoch, 9. Januar 2019, 00:22:01 CET schrieb Robin Murphy: >> On 2019-01-08 10:42 pm, Heiko Stuebner wrote: >>> Am Dienstag, 8. Januar 2019, 22:57:27 CET schrieb Robin Murphy: >>>> For whatever reason, the sdmmc_dectn function isn't working properly >>>> as-is, and microSD insertion and removal goes unnoticed. Flipping the >>>> pin into GPIO mode, however, does do the job, so let's just handle it >>>> that way for now until someone feels inclined to figure out what GRF >>>> voodoo or otherwise is needed for correct 'native' operation. >>>> >>>> Signed-off-by: Robin Murphy <robin.murphy@arm.com> >>>> --- >>>> arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi | 7 ++++++- >>>> 1 file changed, 6 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi >>>> index 9c723038d8f8..2a183a6af150 100644 >>>> --- a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi >>>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi >>>> @@ -505,6 +505,10 @@ >>>> sdmmc0_pwr_h: sdmmc0-pwr-h { >>>> rockchip,pins = <0 RK_PA1 RK_FUNC_GPIO &pcfg_pull_none>; >>>> }; >>>> + >>>> + sdmmc0_det_l: sdmmc0-det-l { >>> >>> alphabetically by node-name please, >>> aka sdmmc0-det-* should be above sdmmc0-pwr-* >> >> Right you are, not sure how that one slipped through. >> >>> If you're respinning the whole series this should be fixed, >>> otherwise I can also do that when applying. >> >> I've fixed it up locally, although it might be worth holding off on this >> particular patch for the moment now that I've taken another look through >> the TRM and noticed those smoking-gun-looking bits in >> GRF_SIG_DETECT_CON{0,1} - I'll investigate... > > Personally I'm very much fine with using a cd-gpio instead of trying > to bring order to the GRF "chaos" ;-) as I'm also not really sure > if we gain anything with the IP-internal detect mechanism. > > But then do as you like :-D I'm assuming from the wiring (and from how I recall my 3288/3328 boxes behaving) that the dedicated function implements its own debouncing - seems the switch in this particular socket is noisy enough that the GPIO interrupt is firing well over 100 times per physical event. Not a big deal really, I just need to scratch the itch of making things work 'properly' ;) >> [ side note - do you reckon there'd be any value in bolting a debugfs >> interface onto the GRF driver, or is the realistic answer to just use >> /dev/mem like everyone else and stop having silly ideas? ] > > I don't think it would help much ... especially when it really only outputs > the register contents in the same way as mem or so does. > > Having to fiddle with GRF bits really should only happen in rare cases, > so I guess mem should do the trick for that. > > And the GRF driver also was a bit controversial anyway, with the title > of "dumping ground" looming over it ... so I'd like to keep it small > and simple ;-) . Yeah, it would end up being one of those things that's really handy for about 5 minutes every couple of years, and a massive pain to maintain the rest of the time. Plus I realise that I'd want to compare the behaviour of stock kernels anyway, and having to patch and rebuild those rather defeats the point. Thanks, Robin.