Re: sdhc(4) quirks

2017-05-05 Thread Mark Kettenis
> Date: Fri, 5 May 2017 16:52:17 +0200
> From: Patrick Wildt 
> 
> On Fri, May 05, 2017 at 02:26:51PM +0200, Mark Kettenis wrote:
> > I'm working on support for the SDHC controller on the Rockchip RK3399
> > such that I can use the onboard eMMC on the Firefly-RK3399.  This
> > controller is based on the Arasan eMMC 5.1 "IP", which has a standard
> > SDHC 3.0 interface.  However there are some minor quirks.
> > 
> > Setting the signalling voltage to 1.8V (which is the only setting the
> > device supports) doesn't work.  For this reason, I add a hook to
> > override the sdhc_signal_voltage() function just like we did for the
> > sdhc_card_detect() function a while ago.
> > 
> > I also can't get the double-data rate mode to work.  So I added a flag
> > that disables the DDR52 mode when set.  My intention is to remove that
> > flag again if I get DDR52 to work on my board.
> > 
> > ok?
> 
> I thought you didn't want too many quirks in there. :)

Heh.

I'd like to avoid too much obfuscation/conditionals in the core
command code.  That's why I added the sdhc_voltage_override() function
instead of another flag.

NetBSD tried to support all kinds of controllers that deviate
considerably from the SDHC spec (such as our imxesdhc(4) and it makes
the core code simply too messay.  But it is far from clear where we
should draw the line...

> > Index: dev/sdmmc/sdhc.c
> > ===
> > RCS file: /cvs/src/sys/dev/sdmmc/sdhc.c,v
> > retrieving revision 1.54
> > diff -u -p -r1.54 sdhc.c
> > --- dev/sdmmc/sdhc.c6 Apr 2017 03:15:29 -   1.54
> > +++ dev/sdmmc/sdhc.c5 May 2017 12:17:48 -
> > @@ -25,6 +25,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  
> >  #include 
> > @@ -317,6 +318,9 @@ sdhc_host_found(struct sdhc_softc *sc, b
> > saa.caps |= SMC_CAPS_MMC_DDR52;
> > }
> >  
> > +   if (ISSET(sc->sc_flags, SDHC_F_NODDR50))
> > +   saa.caps &= ~SMC_CAPS_MMC_DDR52;
> 
> Replace those 4 spaces with a tab.
> 
> > +
> > hp->sdmmc = config_found(&sc->sc_dev, &saa, NULL);
> > if (hp->sdmmc == NULL) {
> > error = 0;
> > @@ -683,6 +687,9 @@ int
> >  sdhc_signal_voltage(sdmmc_chipset_handle_t sch, int signal_voltage)
> >  {
> > struct sdhc_host *hp = sch;
> > +
> > +   if (hp->sc->sc_signal_voltage)
> > +   return hp->sc->sc_signal_voltage(hp->sc, signal_voltage);
> >  
> > if (SDHC_SPEC_VERSION(hp->version) < SDHC_SPEC_V3)
> > return EINVAL;
> > Index: dev/sdmmc/sdhcvar.h
> > ===
> > RCS file: /cvs/src/sys/dev/sdmmc/sdhcvar.h,v
> > retrieving revision 1.9
> > diff -u -p -r1.9 sdhcvar.h
> > --- dev/sdmmc/sdhcvar.h 30 Apr 2016 11:32:23 -  1.9
> > +++ dev/sdmmc/sdhcvar.h 5 May 2017 12:17:48 -
> > @@ -32,6 +32,7 @@ struct sdhc_softc {
> > bus_dma_tag_t sc_dmat;
> >  
> > int (*sc_card_detect)(struct sdhc_softc *);
> > +   int (*sc_signal_voltage)(struct sdhc_softc *, int);
> >  };
> >  
> >  /* Host controller functions called by the attachment driver. */
> > @@ -45,5 +46,6 @@ void  sdhc_needs_discover(struct sdhc_sof
> >  
> >  /* flag values */
> >  #define SDHC_F_NOPWR0  (1 << 0)
> > +#define SDHC_F_NODDR50 (1 << 1)
> >  
> >  #endif
> > 
> 



Re: sdhc(4) quirks

2017-05-05 Thread Patrick Wildt
On Fri, May 05, 2017 at 02:26:51PM +0200, Mark Kettenis wrote:
> I'm working on support for the SDHC controller on the Rockchip RK3399
> such that I can use the onboard eMMC on the Firefly-RK3399.  This
> controller is based on the Arasan eMMC 5.1 "IP", which has a standard
> SDHC 3.0 interface.  However there are some minor quirks.
> 
> Setting the signalling voltage to 1.8V (which is the only setting the
> device supports) doesn't work.  For this reason, I add a hook to
> override the sdhc_signal_voltage() function just like we did for the
> sdhc_card_detect() function a while ago.
> 
> I also can't get the double-data rate mode to work.  So I added a flag
> that disables the DDR52 mode when set.  My intention is to remove that
> flag again if I get DDR52 to work on my board.
> 
> ok?

I thought you didn't want too many quirks in there. :)  No objections
though, ok by me apart from a whitespace thing.

> 
> 
> Index: dev/sdmmc/sdhc.c
> ===
> RCS file: /cvs/src/sys/dev/sdmmc/sdhc.c,v
> retrieving revision 1.54
> diff -u -p -r1.54 sdhc.c
> --- dev/sdmmc/sdhc.c  6 Apr 2017 03:15:29 -   1.54
> +++ dev/sdmmc/sdhc.c  5 May 2017 12:17:48 -
> @@ -25,6 +25,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #include 
> @@ -317,6 +318,9 @@ sdhc_host_found(struct sdhc_softc *sc, b
>   saa.caps |= SMC_CAPS_MMC_DDR52;
>   }
>  
> + if (ISSET(sc->sc_flags, SDHC_F_NODDR50))
> + saa.caps &= ~SMC_CAPS_MMC_DDR52;

Replace those 4 spaces with a tab.

> +
>   hp->sdmmc = config_found(&sc->sc_dev, &saa, NULL);
>   if (hp->sdmmc == NULL) {
>   error = 0;
> @@ -683,6 +687,9 @@ int
>  sdhc_signal_voltage(sdmmc_chipset_handle_t sch, int signal_voltage)
>  {
>   struct sdhc_host *hp = sch;
> +
> + if (hp->sc->sc_signal_voltage)
> + return hp->sc->sc_signal_voltage(hp->sc, signal_voltage);
>  
>   if (SDHC_SPEC_VERSION(hp->version) < SDHC_SPEC_V3)
>   return EINVAL;
> Index: dev/sdmmc/sdhcvar.h
> ===
> RCS file: /cvs/src/sys/dev/sdmmc/sdhcvar.h,v
> retrieving revision 1.9
> diff -u -p -r1.9 sdhcvar.h
> --- dev/sdmmc/sdhcvar.h   30 Apr 2016 11:32:23 -  1.9
> +++ dev/sdmmc/sdhcvar.h   5 May 2017 12:17:48 -
> @@ -32,6 +32,7 @@ struct sdhc_softc {
>   bus_dma_tag_t sc_dmat;
>  
>   int (*sc_card_detect)(struct sdhc_softc *);
> + int (*sc_signal_voltage)(struct sdhc_softc *, int);
>  };
>  
>  /* Host controller functions called by the attachment driver. */
> @@ -45,5 +46,6 @@ voidsdhc_needs_discover(struct sdhc_sof
>  
>  /* flag values */
>  #define SDHC_F_NOPWR0(1 << 0)
> +#define SDHC_F_NODDR50   (1 << 1)
>  
>  #endif
> 



sdhc(4) quirks

2017-05-05 Thread Mark Kettenis
I'm working on support for the SDHC controller on the Rockchip RK3399
such that I can use the onboard eMMC on the Firefly-RK3399.  This
controller is based on the Arasan eMMC 5.1 "IP", which has a standard
SDHC 3.0 interface.  However there are some minor quirks.

Setting the signalling voltage to 1.8V (which is the only setting the
device supports) doesn't work.  For this reason, I add a hook to
override the sdhc_signal_voltage() function just like we did for the
sdhc_card_detect() function a while ago.

I also can't get the double-data rate mode to work.  So I added a flag
that disables the DDR52 mode when set.  My intention is to remove that
flag again if I get DDR52 to work on my board.

ok?


Index: dev/sdmmc/sdhc.c
===
RCS file: /cvs/src/sys/dev/sdmmc/sdhc.c,v
retrieving revision 1.54
diff -u -p -r1.54 sdhc.c
--- dev/sdmmc/sdhc.c6 Apr 2017 03:15:29 -   1.54
+++ dev/sdmmc/sdhc.c5 May 2017 12:17:48 -
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -317,6 +318,9 @@ sdhc_host_found(struct sdhc_softc *sc, b
saa.caps |= SMC_CAPS_MMC_DDR52;
}
 
+   if (ISSET(sc->sc_flags, SDHC_F_NODDR50))
+   saa.caps &= ~SMC_CAPS_MMC_DDR52;
+
hp->sdmmc = config_found(&sc->sc_dev, &saa, NULL);
if (hp->sdmmc == NULL) {
error = 0;
@@ -683,6 +687,9 @@ int
 sdhc_signal_voltage(sdmmc_chipset_handle_t sch, int signal_voltage)
 {
struct sdhc_host *hp = sch;
+
+   if (hp->sc->sc_signal_voltage)
+   return hp->sc->sc_signal_voltage(hp->sc, signal_voltage);
 
if (SDHC_SPEC_VERSION(hp->version) < SDHC_SPEC_V3)
return EINVAL;
Index: dev/sdmmc/sdhcvar.h
===
RCS file: /cvs/src/sys/dev/sdmmc/sdhcvar.h,v
retrieving revision 1.9
diff -u -p -r1.9 sdhcvar.h
--- dev/sdmmc/sdhcvar.h 30 Apr 2016 11:32:23 -  1.9
+++ dev/sdmmc/sdhcvar.h 5 May 2017 12:17:48 -
@@ -32,6 +32,7 @@ struct sdhc_softc {
bus_dma_tag_t sc_dmat;
 
int (*sc_card_detect)(struct sdhc_softc *);
+   int (*sc_signal_voltage)(struct sdhc_softc *, int);
 };
 
 /* Host controller functions called by the attachment driver. */
@@ -45,5 +46,6 @@ void  sdhc_needs_discover(struct sdhc_sof
 
 /* flag values */
 #define SDHC_F_NOPWR0  (1 << 0)
+#define SDHC_F_NODDR50 (1 << 1)
 
 #endif