Date: Tue, 20 Jan 2009 22:06:33 +0200 From: Alexander Motin <mav@FreeBSD.org> To: "M. Warner Losh" <imp@bsdimp.com> Cc: freebsd-arm@freebsd.org Subject: Re: Mount root from SD card? Message-ID: <49762EC9.1010006@FreeBSD.org> In-Reply-To: <49762CEF.1000405@FreeBSD.org> References: <20090120.114051.-854291995.imp@bsdimp.com> <4976215B.40302@FreeBSD.org> <20090120.122312.1543793985.imp@bsdimp.com> <20090120.123230.-272218744.imp@bsdimp.com> <49762CEF.1000405@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Alexander Motin wrote:
> M. Warner Losh wrote:
>> In message: <20090120.122312.1543793985.imp@bsdimp.com>
>> "M. Warner Losh" <imp@bsdimp.com> writes:
>> : : IMHO it is incorrect to disable 4bit mode on that stage, it is too
>> late : : there. It should be done at controller capabilities
>> announcement stage. : : If you are not objecting, I would remove that
>> wire4 variable.
>> : : I am objecting. The code is there so that the rest of the driver
>> does
>> : the right thing when doing 4-bit. It needs to be a capability too.
>> : : However, before we go monkeying with this, we need to find the
>> : underlying bug.
>>
>> I've got the following patch, untested, that I think does what I think
>> needs to be done. Not sure about the return value from update_ios...
>>
>> Index: at91_mci.c
>> ===================================================================
>> --- at91_mci.c (revision 187479)
>> +++ at91_mci.c (working copy)
>> @@ -201,7 +201,10 @@
>> sc->host.f_min = 375000;
>> sc->host.f_max = 30000000;
>> sc->host.host_ocr = MMC_OCR_320_330 | MMC_OCR_330_340;
>> - sc->host.caps = MMC_CAP_4_BIT_DATA;
>> + if (sc->wire4)
>> + sc->host.caps = MMC_CAP_4_BIT_DATA;
>> + else
>> + sc->host.caps = 0;
>> child = device_add_child(dev, "mmc", 0);
>> device_set_ivars(dev, &sc->host);
>> err = bus_generic_attach(dev);
>
> Ok. I agree with this part. If for some reason you really need this
> wire4 variable, this is the right way to use it.
>
>> @@ -295,6 +298,8 @@
>> clkdiv = (at91_master_clock / ios->clock) / 2;
>> }
>> if (ios->bus_width == bus_width_4 && sc->wire4)
>> + return EINVAL;
>> + if (ios->bus_width == bus_width_4)
>> WR4(sc, MCI_SDCR, RD4(sc, MCI_SDCR) | MCI_SDCR_SDCBUS);
>> else
>> WR4(sc, MCI_SDCR, RD4(sc, MCI_SDCR) & ~MCI_SDCR_SDCBUS);
>>
>
> This part is correct, but useless. If the first part have not set
> MMC_CAP_4_BIT_DATA, then mmc layer will never request bus_width_4.
> That's why I have proposed to remove " && sc->wire4" check from here.
No, stop, this part is incorrect. Correct but useless would be:
if (ios->bus_width == bus_width_4 && !sc->wire4)
return EINVAL;
--
Alexander Motin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?49762EC9.1010006>