Re: [PATCH] Fix broken bus voltage setting in sdhc

2018-11-11 Thread Ben Pye
On Sat, Nov 10, 2018 at 03:45:41PM +, Ben Pye wrote:
> On Sat, Nov 10, 2018 at 06:30:37AM +, Ben Pye wrote:
> > On Wed, Nov 07, 2018 at 05:33:13PM +0100, Mark Kettenis wrote:
> > > > From: Ben Pye 
> > > > Content-Type: text/plain; charset="utf-8"
> > > > 
> > > > I have been attempting to run OpenBSD on my HP Chromebook 13, it's a
> > > > Skylake device with eMMC storage. Previously sdhc attempted to set the
> > > > same bus voltage multiple times, and after the first, successful,
> > > > attempt it would break resulting in all later commands timing out. This
> > > > patch changes sdhc such that it only sets the voltage if the request is
> > > > for a different level, this is the behaviour FreeBSD has.
> > > 
> > > That makes sense.  We'll need to test this on more hardware.  And
> > > maybe we need to reset hp->vdd in some places (suspend/resume, resets).
> > 
> > As you suspected suspend/resume isn't working with this current patch. At
> > least for my hardware I need to restore the bus_power setting on resume
> > in addition to the other registers. This seems to get the eMMC device
> > back up.
> 
> I have included the new patch to get the device attached after suspend
> below. I believe if you are not using softraid over this device then
> this should be sufficient to get working suspend/resume. Unfortunately
> softraid does not handle the detach and reattach of sd0, though I am now
> convinced that it's really a seperate issue.
> 
> Ben.
> 

As per my recent email to this list, I think that setting the bus power
back after resume is probably the wrong approach to take. For this
reason I'd suggest we don't keep the save_vdd value and call to
sdhc_bus_power after resume. This will have to be handled differently, I
have edited the patch below to reflect this.

Ben

> Index: dev/sdmmc/sdhc.c
> ===
> RCS file: /cvs/src/sys/dev/sdmmc/sdhc.c,v
> retrieving revision 1.61
> diff -u -p -r1.61 sdhc.c
> --- dev/sdmmc/sdhc.c  6 Sep 2018 10:15:17 -   1.61
> +++ dev/sdmmc/sdhc.c  10 Nov 2018 15:40:40 -
> @@ -53,6 +53,7 @@ struct sdhc_host {
>   u_int8_t regs[14];  /* host controller state */
>   u_int16_t intr_status;  /* soft interrupt status */
>   u_int16_t intr_error_status;/* soft error status */
> + u_int8_t vdd;   /* current vdd */
>  
>   bus_dmamap_t adma_map;
>   bus_dma_segment_t adma_segs[1];
> @@ -420,6 +421,8 @@ sdhc_host_reset(sdmmc_chipset_handle_t s
>  
>   s = splsdmmc();
>  
> + hp->vdd = 0;
> +
>   /* Disable all interrupts. */
>   HWRITE2(hp, SDHC_NINTR_SIGNAL_EN, 0);
>  
> @@ -491,6 +494,16 @@ sdhc_bus_power(sdmmc_chipset_handle_t sc
>   struct sdhc_host *hp = sch;
>   u_int8_t vdd;
>   int s;
> +
> + /*
> +  * If the requested vdd is the same as current vdd return.
> +  */
> + if (hp->vdd == ocr)
> + return 0;
> +
> + hp->vdd = ocr;
>  
>   s = splsdmmc();
> 



Re: [PATCH] Fix broken bus voltage setting in sdhc

2018-11-10 Thread Jonathan Gray
On Sat, Nov 10, 2018 at 01:22:47PM -0500, Brandon Mercer wrote:
> On Sat, Nov 10, 2018 at 03:45:41PM +, Ben Pye wrote:
> > On Sat, Nov 10, 2018 at 06:30:37AM +, Ben Pye wrote:
> > > On Wed, Nov 07, 2018 at 05:33:13PM +0100, Mark Kettenis wrote:
> > > > > From: Ben Pye 
> > > > > Content-Type: text/plain; charset="utf-8"
> > > > > 
> > > > > I have been attempting to run OpenBSD on my HP Chromebook 13, it's a
> > > > > Skylake device with eMMC storage. Previously sdhc attempted to set the
> > > > > same bus voltage multiple times, and after the first, successful,
> > > > > attempt it would break resulting in all later commands timing out. 
> > > > > This
> > > > > patch changes sdhc such that it only sets the voltage if the request 
> > > > > is
> > > > > for a different level, this is the behaviour FreeBSD has.
> > > > 
> > > > That makes sense.  We'll need to test this on more hardware.  And
> > > > maybe we need to reset hp->vdd in some places (suspend/resume, resets).
> > > 
> > > As you suspected suspend/resume isn't working with this current patch. At
> > > least for my hardware I need to restore the bus_power setting on resume
> > > in addition to the other registers. This seems to get the eMMC device
> > > back up.
> > 
> > I have included the new patch to get the device attached after suspend
> > below. I believe if you are not using softraid over this device then
> > this should be sufficient to get working suspend/resume. Unfortunately
> > softraid does not handle the detach and reattach of sd0, though I am now
> > convinced that it's really a seperate issue.
> > 
> 
> Thanks for this, Ben. I am running this on my chromebook 13. I've added
> the device ids for the mmc to pcidevs as well. 
> 
> OK? 
> 
> Index: sys/dev/pci/pcidevs
> ===
> RCS file: /cvs/src/sys/dev/pci/pcidevs,v
> retrieving revision 1.1866
> diff -u -p -u -r1.1866 pcidevs
> --- sys/dev/pci/pcidevs   8 Nov 2018 06:54:13 -   1.1866
> +++ sys/dev/pci/pcidevs   10 Nov 2018 17:10:16 -
> @@ -5108,6 +5108,7 @@ product INTEL 100SERIES_LP_PCIE_12  0x9d1
>  product INTEL 100SERIES_LP_PMC   0x9d21  100 Series PMC
>  product INTEL 100SERIES_LP_SMB   0x9d23  100 Series SMBus
>  product INTEL 100SERIES_LP_SPI   0x9d24  100 Series SPI
> +product INTEL 100SERIES_LP_EMMC 0x9d2b   100 Series eMMC

This is missing a tab before 0x and should be put after the UART_2
entry to be sorted by device id.

>  product INTEL 100SERIES_LP_UART_10x9d27  100 Series UART
>  product INTEL 100SERIES_LP_UART_20x9d28  100 Series UART
>  product INTEL 100SERIES_LP_XHCI  0x9d2f  100 Series xHCI
> Index: sys/dev/pci/pcidevs.h
> ===
> RCS file: /cvs/src/sys/dev/pci/pcidevs.h,v
> retrieving revision 1.1859
> diff -u -p -u -r1.1859 pcidevs.h
> --- sys/dev/pci/pcidevs.h 8 Nov 2018 06:55:06 -   1.1859
> +++ sys/dev/pci/pcidevs.h 10 Nov 2018 17:10:18 -
> @@ -5113,6 +5113,7 @@
>  #define  PCI_PRODUCT_INTEL_100SERIES_LP_PMC  0x9d21  /* 100 
> Series PMC */
>  #define  PCI_PRODUCT_INTEL_100SERIES_LP_SMB  0x9d23  /* 100 
> Series SMBus */
>  #define  PCI_PRODUCT_INTEL_100SERIES_LP_SPI  0x9d24  /* 100 
> Series SPI */
> +#define  PCI_PRODUCT_INTEL_100SERIES_LP_EMMC 0x9d2b  /* 100 
> Series eMMC */
>  #define  PCI_PRODUCT_INTEL_100SERIES_LP_UART_1   0x9d27  /* 100 
> Series UART */
>  #define  PCI_PRODUCT_INTEL_100SERIES_LP_UART_2   0x9d28  /* 100 
> Series UART */
>  #define  PCI_PRODUCT_INTEL_100SERIES_LP_XHCI 0x9d2f  /* 100 
> Series xHCI */
> Index: sys/dev/pci/pcidevs_data.h
> ===
> RCS file: /cvs/src/sys/dev/pci/pcidevs_data.h,v
> retrieving revision 1.1854
> diff -u -p -u -r1.1854 pcidevs_data.h
> --- sys/dev/pci/pcidevs_data.h8 Nov 2018 06:55:06 -   1.1854
> +++ sys/dev/pci/pcidevs_data.h10 Nov 2018 17:10:19 -
> @@ -17936,6 +17936,10 @@ static const struct pci_known_product pc
>   "100 Series SPI",
>   },
>   {
> + PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_100SERIES_LP_EMMC,
> + "100 Series eMMC",
> + },
> + {
>   PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_100SERIES_LP_UART_1,
>   "100 Series UART",
>   },
> Index: sys/dev/sdmmc/sdhc.c
> ===
> RCS file: /cvs/src/sys/dev/sdmmc/sdhc.c,v
> retrieving revision 1.61
> diff -u -p -u -r1.61 sdhc.c
> --- sys/dev/sdmmc/sdhc.c  6 Sep 2018 10:15:17 -   1.61
> +++ sys/dev/sdmmc/sdhc.c  10 Nov 2018 17:10:19 -
> @@ -53,6 +53,8 @@ struct sdhc_host {
>   u_int8_t regs[14];  /* host controller state */
>   u_int16_t intr_status;  /* soft interrupt status */
>   u_int16_t 

Re: [PATCH] Fix broken bus voltage setting in sdhc

2018-11-10 Thread Brandon Mercer
On Sat, Nov 10, 2018 at 03:45:41PM +, Ben Pye wrote:
> On Sat, Nov 10, 2018 at 06:30:37AM +, Ben Pye wrote:
> > On Wed, Nov 07, 2018 at 05:33:13PM +0100, Mark Kettenis wrote:
> > > > From: Ben Pye 
> > > > Content-Type: text/plain; charset="utf-8"
> > > > 
> > > > I have been attempting to run OpenBSD on my HP Chromebook 13, it's a
> > > > Skylake device with eMMC storage. Previously sdhc attempted to set the
> > > > same bus voltage multiple times, and after the first, successful,
> > > > attempt it would break resulting in all later commands timing out. This
> > > > patch changes sdhc such that it only sets the voltage if the request is
> > > > for a different level, this is the behaviour FreeBSD has.
> > > 
> > > That makes sense.  We'll need to test this on more hardware.  And
> > > maybe we need to reset hp->vdd in some places (suspend/resume, resets).
> > 
> > As you suspected suspend/resume isn't working with this current patch. At
> > least for my hardware I need to restore the bus_power setting on resume
> > in addition to the other registers. This seems to get the eMMC device
> > back up.
> 
> I have included the new patch to get the device attached after suspend
> below. I believe if you are not using softraid over this device then
> this should be sufficient to get working suspend/resume. Unfortunately
> softraid does not handle the detach and reattach of sd0, though I am now
> convinced that it's really a seperate issue.
> 

Thanks for this, Ben. I am running this on my chromebook 13. I've added
the device ids for the mmc to pcidevs as well. 

OK? 

Index: sys/dev/pci/pcidevs
===
RCS file: /cvs/src/sys/dev/pci/pcidevs,v
retrieving revision 1.1866
diff -u -p -u -r1.1866 pcidevs
--- sys/dev/pci/pcidevs 8 Nov 2018 06:54:13 -   1.1866
+++ sys/dev/pci/pcidevs 10 Nov 2018 17:10:16 -
@@ -5108,6 +5108,7 @@ product INTEL 100SERIES_LP_PCIE_120x9d1
 product INTEL 100SERIES_LP_PMC 0x9d21  100 Series PMC
 product INTEL 100SERIES_LP_SMB 0x9d23  100 Series SMBus
 product INTEL 100SERIES_LP_SPI 0x9d24  100 Series SPI
+product INTEL 100SERIES_LP_EMMC 0x9d2b 100 Series eMMC
 product INTEL 100SERIES_LP_UART_1  0x9d27  100 Series UART
 product INTEL 100SERIES_LP_UART_2  0x9d28  100 Series UART
 product INTEL 100SERIES_LP_XHCI0x9d2f  100 Series xHCI
Index: sys/dev/pci/pcidevs.h
===
RCS file: /cvs/src/sys/dev/pci/pcidevs.h,v
retrieving revision 1.1859
diff -u -p -u -r1.1859 pcidevs.h
--- sys/dev/pci/pcidevs.h   8 Nov 2018 06:55:06 -   1.1859
+++ sys/dev/pci/pcidevs.h   10 Nov 2018 17:10:18 -
@@ -5113,6 +5113,7 @@
 #definePCI_PRODUCT_INTEL_100SERIES_LP_PMC  0x9d21  /* 100 
Series PMC */
 #definePCI_PRODUCT_INTEL_100SERIES_LP_SMB  0x9d23  /* 100 
Series SMBus */
 #definePCI_PRODUCT_INTEL_100SERIES_LP_SPI  0x9d24  /* 100 
Series SPI */
+#definePCI_PRODUCT_INTEL_100SERIES_LP_EMMC 0x9d2b  /* 100 
Series eMMC */
 #definePCI_PRODUCT_INTEL_100SERIES_LP_UART_1   0x9d27  /* 100 
Series UART */
 #definePCI_PRODUCT_INTEL_100SERIES_LP_UART_2   0x9d28  /* 100 
Series UART */
 #definePCI_PRODUCT_INTEL_100SERIES_LP_XHCI 0x9d2f  /* 100 
Series xHCI */
Index: sys/dev/pci/pcidevs_data.h
===
RCS file: /cvs/src/sys/dev/pci/pcidevs_data.h,v
retrieving revision 1.1854
diff -u -p -u -r1.1854 pcidevs_data.h
--- sys/dev/pci/pcidevs_data.h  8 Nov 2018 06:55:06 -   1.1854
+++ sys/dev/pci/pcidevs_data.h  10 Nov 2018 17:10:19 -
@@ -17936,6 +17936,10 @@ static const struct pci_known_product pc
"100 Series SPI",
},
{
+   PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_100SERIES_LP_EMMC,
+   "100 Series eMMC",
+   },
+   {
PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_100SERIES_LP_UART_1,
"100 Series UART",
},
Index: sys/dev/sdmmc/sdhc.c
===
RCS file: /cvs/src/sys/dev/sdmmc/sdhc.c,v
retrieving revision 1.61
diff -u -p -u -r1.61 sdhc.c
--- sys/dev/sdmmc/sdhc.c6 Sep 2018 10:15:17 -   1.61
+++ sys/dev/sdmmc/sdhc.c10 Nov 2018 17:10:19 -
@@ -53,6 +53,8 @@ struct sdhc_host {
u_int8_t regs[14];  /* host controller state */
u_int16_t intr_status;  /* soft interrupt status */
u_int16_t intr_error_status;/* soft error status */
+   u_int8_t vdd;   /* current vdd */
+   u_int8_t save_vdd;  /* vdd before suspend */
 
bus_dmamap_t adma_map;
bus_dma_segment_t adma_segs[1];
@@ -364,6 +366,7 @@ sdhc_activate(struct device *self, int a
/* Save the host controller state. */
for (n = 0; n < 

Re: [PATCH] Fix broken bus voltage setting in sdhc

2018-11-10 Thread Ben Pye
On Sat, Nov 10, 2018 at 06:30:37AM +, Ben Pye wrote:
> On Wed, Nov 07, 2018 at 05:33:13PM +0100, Mark Kettenis wrote:
> > > From: Ben Pye 
> > > Content-Type: text/plain; charset="utf-8"
> > > 
> > > I have been attempting to run OpenBSD on my HP Chromebook 13, it's a
> > > Skylake device with eMMC storage. Previously sdhc attempted to set the
> > > same bus voltage multiple times, and after the first, successful,
> > > attempt it would break resulting in all later commands timing out. This
> > > patch changes sdhc such that it only sets the voltage if the request is
> > > for a different level, this is the behaviour FreeBSD has.
> > 
> > That makes sense.  We'll need to test this on more hardware.  And
> > maybe we need to reset hp->vdd in some places (suspend/resume, resets).
> 
> As you suspected suspend/resume isn't working with this current patch. At
> least for my hardware I need to restore the bus_power setting on resume
> in addition to the other registers. This seems to get the eMMC device
> back up.

I have included the new patch to get the device attached after suspend
below. I believe if you are not using softraid over this device then
this should be sufficient to get working suspend/resume. Unfortunately
softraid does not handle the detach and reattach of sd0, though I am now
convinced that it's really a seperate issue.

Ben.

Index: dev/sdmmc/sdhc.c
===
RCS file: /cvs/src/sys/dev/sdmmc/sdhc.c,v
retrieving revision 1.61
diff -u -p -r1.61 sdhc.c
--- dev/sdmmc/sdhc.c6 Sep 2018 10:15:17 -   1.61
+++ dev/sdmmc/sdhc.c10 Nov 2018 15:40:40 -
@@ -53,6 +53,8 @@ struct sdhc_host {
u_int8_t regs[14];  /* host controller state */
u_int16_t intr_status;  /* soft interrupt status */
u_int16_t intr_error_status;/* soft error status */
+   u_int8_t vdd;   /* current vdd */
+   u_int8_t save_vdd;  /* vdd before suspend */
 
bus_dmamap_t adma_map;
bus_dma_segment_t adma_segs[1];
@@ -364,6 +366,7 @@ sdhc_activate(struct device *self, int a
/* Save the host controller state. */
for (n = 0; n < sc->sc_nhosts; n++) {
hp = sc->sc_host[n];
+   hp->save_vdd = hp->vdd;
for (i = 0; i < sizeof hp->regs; i++)
hp->regs[i] = HREAD1(hp, i);
}
@@ -373,6 +376,7 @@ sdhc_activate(struct device *self, int a
for (n = 0; n < sc->sc_nhosts; n++) {
hp = sc->sc_host[n];
(void)sdhc_host_reset(hp);
+   sdhc_bus_power(hp, hp->save_vdd);
for (i = 0; i < sizeof hp->regs; i++)
HWRITE1(hp, i, hp->regs[i]);
}
@@ -420,6 +424,8 @@ sdhc_host_reset(sdmmc_chipset_handle_t s
 
s = splsdmmc();
 
+   hp->vdd = 0;
+
/* Disable all interrupts. */
HWRITE2(hp, SDHC_NINTR_SIGNAL_EN, 0);
 
@@ -489,6 +495,14 @@ sdhc_bus_power(sdmmc_chipset_handle_t sc
struct sdhc_host *hp = sch;
u_int8_t vdd;
int s;
+
+   /*
+* If the requested vdd is the same as current vdd return.
+*/
+   if (hp->vdd == ocr)
+   return 0;
+
+   hp->vdd = ocr;
 
s = splsdmmc();



Re: [PATCH] Fix broken bus voltage setting in sdhc

2018-11-09 Thread Ben Pye
On Wed, Nov 07, 2018 at 05:33:13PM +0100, Mark Kettenis wrote:
> > From: Ben Pye 
> > Content-Type: text/plain; charset="utf-8"
> > 
> > I have been attempting to run OpenBSD on my HP Chromebook 13, it's a
> > Skylake device with eMMC storage. Previously sdhc attempted to set the
> > same bus voltage multiple times, and after the first, successful,
> > attempt it would break resulting in all later commands timing out. This
> > patch changes sdhc such that it only sets the voltage if the request is
> > for a different level, this is the behaviour FreeBSD has.
> 
> That makes sense.  We'll need to test this on more hardware.  And
> maybe we need to reset hp->vdd in some places (suspend/resume, resets).

As you suspected suspend/resume isn't working with this current patch. At
least for my hardware I need to restore the bus_power setting on resume
in addition to the other registers. This seems to get the eMMC device
back up.

I am however hitting another (potentially unrelated?) issue having done
this. I am using softraid for FDE and so my physical device is sd0 and
my softraid device is sd1. After resume sd0 is detached and reattached
and appears to still be working, the device name is printed to the dmesg
output. sd1 however stays in a broken state and so eventually the system
crashes as it's root filesystem just disappeared. I have included a
(partial - I have ended up adding a lot of logging to get it coming up
after suspend) dmesg below. Any ideas?

Thanks
Ben

sdmmc0: CID: mid=0x90 oid=0x014a pnm="HBG4e\^E" rev=0x00 psn=0x507b6955 mdt=000
sdmmc0: read_bl_len=512 sector_size=512
scsibus1 at sdmmc0: 2 targets, initiator 0
sd0 at scsibus1 targ 1 lun 0:  SCSI2 0/direct 
removable
sd0: 29824MB, 512 bytes/sector, 61079552 sectors
ugen0 at uhub0 port 3 "Intel Bluetooth" rev 2.00/0.03 addr 2
uvideo0 at uhub0 port 7 configuration 1 interface 0 "Generic HP Truevision HD" 
rev 2.00/0.06 a
video0 at uvideo0
vscsi0 at root
scsibus2 at vscsi0: 256 targets
softraid0 at root
scsibus3 at softraid0: 256 targets
softraid0: sd1 was not shutdown properly
sd1 at scsibus3 targ 1 lun 0:  SCSI2 0/direct fixed
sd1: 29823MB, 512 bytes/sector, 61077937 sectors
sdhc0: timeout waiting for 0 
(state=1fef0106)
softraid0: I/O error 5 on dev 0x400 at block 16
softraid0: could not write metadata to sd0a
root on sd1a (2d11d44f123d9141.a) swap on sd1b dump on sd1b
... (iwm, usb, video, etc messages omitted)
sd0 detached
scsibus1 detached
softraid0: sd1: i/o error 0 @ CRYPTO block 11831120
softraid0: sd1: i/o error 0 @ CRYPTO block 11820400
softraid0: sd1: i/o error 0 @ CRYPTO block 11831120
softraid0: sd1: i/o error 0 @ CRYPTO block 9755216
softraid0: sd1: i/o error 0 @ CRYPTO block 9748640
... (more i/o errors omitted)
sdmmc0: CID: mid=0x90 oid=0x014a pnm="HBG4e\^E" rev=0x00 psn=0x507b6955 mdt=000
sdmmc0: read_bl_len=512 sector_size=512
scsibus1 at sdmmc0: 2 targets, initiator 0
sd0 at scsibus1 targ 1 lun 0:  SCSI2 0/direct 
removable
sd0: 29824MB, 512 bytes/sector, 61079552 sectors
softraid0: sd1: i/o error 0 @ CRYPTO block 15148176
softraid0: sd1: i/o error 0 @ CRYPTO block 15141600
softraid0: sd1: i/o error 0 @ CRYPTO block 9755216
softraid0: sd1: i/o error 0 @ CRYPTO block 9748640
softraid0: sd1: i/o error 0 @ CRYPTO block 8144
softraid0: sd1: i/o error 0 @ CRYPTO block 1568
softraid0: sd1: i/o error 0 @ CRYPTO block 6291568
... (further i/o errors until panic)

> 
> Cheers,
> 
> Mark
> 
> > Index: sys/dev/sdmmc/sdhc.c
> > ===
> > RCS file: /cvs/src/sys/dev/sdmmc/sdhc.c,v
> > retrieving revision 1.61
> > diff -u -p -u -p -r1.61 sdhc.c
> > --- sys/dev/sdmmc/sdhc.c6 Sep 2018 10:15:17 -   1.61
> > +++ sys/dev/sdmmc/sdhc.c7 Nov 2018 15:36:10 -
> > @@ -53,6 +53,7 @@ struct sdhc_host {
> > u_int8_t regs[14];  /* host controller state */
> > u_int16_t intr_status;  /* soft interrupt status */
> > u_int16_t intr_error_status;/* soft error status */
> > +   u_int8_t vdd;   /* current vdd */
> >  
> > bus_dmamap_t adma_map;
> > bus_dma_segment_t adma_segs[1];
> > @@ -420,6 +421,8 @@ sdhc_host_reset(sdmmc_chipset_handle_t s
> >  
> > s = splsdmmc();
> >  
> > +   hp->vdd = 0;
> > +
> > /* Disable all interrupts. */
> > HWRITE2(hp, SDHC_NINTR_SIGNAL_EN, 0);
> >  
> > @@ -491,6 +494,16 @@ sdhc_bus_power(sdmmc_chipset_handle_t sc
> > int s;
> >  
> > s = splsdmmc();
> > +
> > +   /* 
> > +* If the requested vdd is the same as current vdd return.
> > +*/
> > +   if (hp->vdd == ocr) {
> > +   splx(s);
> > +   return 0;
> > +   }
> > +
> > +   hp->vdd = ocr;
> >  
> > /*
> >  * Disable bus power before voltage change.
> > 
> > 
> 



Re: [PATCH] Fix broken bus voltage setting in sdhc

2018-11-07 Thread Mark Kettenis
> From: Ben Pye 
> Content-Type: text/plain; charset="utf-8"
> 
> I have been attempting to run OpenBSD on my HP Chromebook 13, it's a
> Skylake device with eMMC storage. Previously sdhc attempted to set the
> same bus voltage multiple times, and after the first, successful,
> attempt it would break resulting in all later commands timing out. This
> patch changes sdhc such that it only sets the voltage if the request is
> for a different level, this is the behaviour FreeBSD has.

That makes sense.  We'll need to test this on more hardware.  And
maybe we need to reset hp->vdd in some places (suspend/resume, resets).

Cheers,

Mark

> Index: sys/dev/sdmmc/sdhc.c
> ===
> RCS file: /cvs/src/sys/dev/sdmmc/sdhc.c,v
> retrieving revision 1.61
> diff -u -p -u -p -r1.61 sdhc.c
> --- sys/dev/sdmmc/sdhc.c  6 Sep 2018 10:15:17 -   1.61
> +++ sys/dev/sdmmc/sdhc.c  7 Nov 2018 15:36:10 -
> @@ -53,6 +53,7 @@ struct sdhc_host {
>   u_int8_t regs[14];  /* host controller state */
>   u_int16_t intr_status;  /* soft interrupt status */
>   u_int16_t intr_error_status;/* soft error status */
> + u_int8_t vdd;   /* current vdd */
>  
>   bus_dmamap_t adma_map;
>   bus_dma_segment_t adma_segs[1];
> @@ -420,6 +421,8 @@ sdhc_host_reset(sdmmc_chipset_handle_t s
>  
>   s = splsdmmc();
>  
> + hp->vdd = 0;
> +
>   /* Disable all interrupts. */
>   HWRITE2(hp, SDHC_NINTR_SIGNAL_EN, 0);
>  
> @@ -491,6 +494,16 @@ sdhc_bus_power(sdmmc_chipset_handle_t sc
>   int s;
>  
>   s = splsdmmc();
> +
> + /* 
> +  * If the requested vdd is the same as current vdd return.
> +  */
> + if (hp->vdd == ocr) {
> + splx(s);
> + return 0;
> + }
> +
> + hp->vdd = ocr;
>  
>   /*
>* Disable bus power before voltage change.
> 
> 



[PATCH] Fix broken bus voltage setting in sdhc

2018-11-07 Thread Ben Pye
I have been attempting to run OpenBSD on my HP Chromebook 13, it's a
Skylake device with eMMC storage. Previously sdhc attempted to set the
same bus voltage multiple times, and after the first, successful,
attempt it would break resulting in all later commands timing out. This
patch changes sdhc such that it only sets the voltage if the request is
for a different level, this is the behaviour FreeBSD has.

Ben.

Index: sys/dev/sdmmc/sdhc.c
===
RCS file: /cvs/src/sys/dev/sdmmc/sdhc.c,v
retrieving revision 1.61
diff -u -p -u -p -r1.61 sdhc.c
--- sys/dev/sdmmc/sdhc.c6 Sep 2018 10:15:17 -   1.61
+++ sys/dev/sdmmc/sdhc.c7 Nov 2018 15:36:10 -
@@ -53,6 +53,7 @@ struct sdhc_host {
u_int8_t regs[14];  /* host controller state */
u_int16_t intr_status;  /* soft interrupt status */
u_int16_t intr_error_status;/* soft error status */
+   u_int8_t vdd;   /* current vdd */
 
bus_dmamap_t adma_map;
bus_dma_segment_t adma_segs[1];
@@ -420,6 +421,8 @@ sdhc_host_reset(sdmmc_chipset_handle_t s
 
s = splsdmmc();
 
+   hp->vdd = 0;
+
/* Disable all interrupts. */
HWRITE2(hp, SDHC_NINTR_SIGNAL_EN, 0);
 
@@ -491,6 +494,16 @@ sdhc_bus_power(sdmmc_chipset_handle_t sc
int s;
 
s = splsdmmc();
+
+   /* 
+* If the requested vdd is the same as current vdd return.
+*/
+   if (hp->vdd == ocr) {
+   splx(s);
+   return 0;
+   }
+
+   hp->vdd = ocr;
 
/*
 * Disable bus power before voltage change.