Re: sdhc(4) base clock

[Visa Hankala]

On Sun, Mar 18, 2018 at 06:33:59PM +0100, Mark Kettenis wrote:
> > Date: Sun, 18 Mar 2018 12:17:09 +
> > From: Visa Hankala 
> > 
> > On Sat, Mar 17, 2018 at 07:41:39PM +0100, Mark Kettenis wrote:
> > > Index: dev/sdmmc/sdhc.c
> > > ===
> > > RCS file: /cvs/src/sys/dev/sdmmc/sdhc.c,v
> > > retrieving revision 1.56
> > > diff -u -p -r1.56 sdhc.c
> > > --- dev/sdmmc/sdhc.c  10 Feb 2018 05:21:13 -  1.56
> > > +++ dev/sdmmc/sdhc.c  17 Mar 2018 18:34:08 -
> > > @@ -203,6 +203,11 @@ sdhc_host_found(struct sdhc_softc *sc, b
> > >   hp->clkbase = SDHC_BASE_FREQ_KHZ(caps);
> > >   }
> > >   if (hp->clkbase == 0) {
> > > + /* Make sure we can clock down to 400 kHz. */
> > > + max_clock = 400 * 2046;
> > > + hp->clkbase = sc->sc_clkbase;
> > > + }
> > 
> > The above looks strange. Did you mean "up to 400 MHz"?
> > max_clock is in kHz.
> 
> Right.  The maximum clock divisor for SDHC 3.0 is 2046.  So if the
> base clock is 400 * 2046 kHz, using the maximum divisor yields 400
> kHz.

Ah, now I understand. Please use a macro for the value 2046,
say SDHC_CLK_DIV_MAX_V3, to help the next code reader.
With that tweak, OK visa@

Re: sdhc(4) base clock

[Mark Kettenis]

> Date: Sun, 18 Mar 2018 12:17:09 +
> From: Visa Hankala 
> 
> On Sat, Mar 17, 2018 at 07:41:39PM +0100, Mark Kettenis wrote:
> > Index: dev/sdmmc/sdhc.c
> > ===
> > RCS file: /cvs/src/sys/dev/sdmmc/sdhc.c,v
> > retrieving revision 1.56
> > diff -u -p -r1.56 sdhc.c
> > --- dev/sdmmc/sdhc.c10 Feb 2018 05:21:13 -  1.56
> > +++ dev/sdmmc/sdhc.c17 Mar 2018 18:34:08 -
> > @@ -203,6 +203,11 @@ sdhc_host_found(struct sdhc_softc *sc, b
> > hp->clkbase = SDHC_BASE_FREQ_KHZ(caps);
> > }
> > if (hp->clkbase == 0) {
> > +   /* Make sure we can clock down to 400 kHz. */
> > +   max_clock = 400 * 2046;
> > +   hp->clkbase = sc->sc_clkbase;
> > +   }
> 
> The above looks strange. Did you mean "up to 400 MHz"?
> max_clock is in kHz.

Right.  The maximum clock divisor for SDHC 3.0 is 2046.  So if the
base clock is 400 * 2046 kHz, using the maximum divisor yields 400
kHz.

Re: sdhc(4) base clock

[Visa Hankala]

On Sat, Mar 17, 2018 at 07:41:39PM +0100, Mark Kettenis wrote:
> Index: dev/sdmmc/sdhc.c
> ===
> RCS file: /cvs/src/sys/dev/sdmmc/sdhc.c,v
> retrieving revision 1.56
> diff -u -p -r1.56 sdhc.c
> --- dev/sdmmc/sdhc.c  10 Feb 2018 05:21:13 -  1.56
> +++ dev/sdmmc/sdhc.c  17 Mar 2018 18:34:08 -
> @@ -203,6 +203,11 @@ sdhc_host_found(struct sdhc_softc *sc, b
>   hp->clkbase = SDHC_BASE_FREQ_KHZ(caps);
>   }
>   if (hp->clkbase == 0) {
> + /* Make sure we can clock down to 400 kHz. */
> + max_clock = 400 * 2046;
> + hp->clkbase = sc->sc_clkbase;
> + }

The above looks strange. Did you mean "up to 400 MHz"?
max_clock is in kHz.

sdhc(4) base clock

[Mark Kettenis]

The SDHC controller found on the Marvell Armada 7K and 8K SoCs has all
the base clock frequency bits in the capabilities register set to 0.
The SDHC 3.0 spec says that this means that "the Host System has to
get the information via another method".  This diff provides that
method.  The sdhc(4) bus attachment glue can set sc_clkbase to the
appropriate frequency.  I have a followup diff that adds code to do
that for dev/fdt/sdhc_fdt.c.

ok?


Index: dev/sdmmc/sdhcvar.h
===
RCS file: /cvs/src/sys/dev/sdmmc/sdhcvar.h,v
retrieving revision 1.10
diff -u -p -r1.10 sdhcvar.h
--- dev/sdmmc/sdhcvar.h 5 May 2017 15:10:07 -   1.10
+++ dev/sdmmc/sdhcvar.h 17 Mar 2018 18:34:08 -
@@ -28,6 +28,7 @@ struct sdhc_softc {
struct sdhc_host **sc_host;
int sc_nhosts;
u_int sc_flags;
+   u_int sc_clkbase;
 
bus_dma_tag_t sc_dmat;
 
Index: dev/sdmmc/sdhc.c
===
RCS file: /cvs/src/sys/dev/sdmmc/sdhc.c,v
retrieving revision 1.56
diff -u -p -r1.56 sdhc.c
--- dev/sdmmc/sdhc.c10 Feb 2018 05:21:13 -  1.56
+++ dev/sdmmc/sdhc.c17 Mar 2018 18:34:08 -
@@ -203,6 +203,11 @@ sdhc_host_found(struct sdhc_softc *sc, b
hp->clkbase = SDHC_BASE_FREQ_KHZ(caps);
}
if (hp->clkbase == 0) {
+   /* Make sure we can clock down to 400 kHz. */
+   max_clock = 400 * 2046;
+   hp->clkbase = sc->sc_clkbase;
+   }
+   if (hp->clkbase == 0) {
/* The attachment driver must tell us. */
printf("%s: base clock frequency unknown\n",
sc->sc_dev.dv_xname);