Date: Mon, 10 Jul 2017 00:37:47 +0200 From: Marius Strobl <marius@freebsd.org> To: Warner Losh <imp@bsdimp.com> Cc: "svn-src-head@freebsd.org" <svn-src-head@freebsd.org>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, src-committers <src-committers@freebsd.org>, Warner Losh <imp@freebsd.org> Subject: Re: svn commit: r320844 - in head: etc/mtree include lib/libcam sys/amd64/conf sys/arm/broadcom/bcm2835 sys/arm/conf sys/arm/ti sys/cam sys/cam/mmc sys/cam/scsi sys/conf sys/dev/mmc sys/dev/sdhci sys/m... Message-ID: <20170709223747.GC38352@alchemy.franken.de> In-Reply-To: <CANCZdfqim1ksF6%2BJ4j-nnEH5GKSHtAoGJOfJceOd6OjYtjys8g@mail.gmail.com> References: <201707091657.v69GvOar096942@repo.freebsd.org> <20170709184221.GB38352@alchemy.franken.de> <CANCZdfrQeqUC7_ppkhk2poSwr3Kgm7VGqBcOg5-AWzuPqJ0GCw@mail.gmail.com> <CANCZdfqim1ksF6%2BJ4j-nnEH5GKSHtAoGJOfJceOd6OjYtjys8g@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--cWoXeonUoKmBZSoM Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Sun, Jul 09, 2017 at 02:49:56PM -0600, Warner Losh wrote: > On Sun, Jul 9, 2017 at 2:40 PM, Warner Losh <imp@bsdimp.com> wrote: > > > > > > > On Sun, Jul 9, 2017 at 12:42 PM, Marius Strobl <marius@freebsd.org> wrote: > > > >> On Sun, Jul 09, 2017 at 04:57:24PM +0000, Warner Losh wrote: > >> > Author: imp > >> > Date: Sun Jul 9 16:57:24 2017 > >> > New Revision: 320844 > >> > URL: https://svnweb.freebsd.org/changeset/base/320844 > >> > > >> > Log: > >> > An MMC/SD/SDIO stack using CAM > >> > > >> > Implement the MMC/SD/SDIO protocol within a CAM framework. CAM's > >> > flexible queueing will make it easier to write non-storage drivers > >> > than the legacy stack. SDIO drivers from both the kernel and as > >> > userland daemons are possible, though much of that functionality will > >> > come later. > >> > >> At least with a non-MMCCAM kernel, with this revision in place I get > >> an endless storm of "unexpected" SDHCI_INT_CARD_INT interrupts during > >> boot. Apparently this is due to the fact that sdhci(4) now enables > >> these interrupts, but sdhci_generic_intr() neither actually handles > >> them nor clears them from intmask. > >> > > > > OK. I'll look into it. Since I don't have an SDHCI card in the system I > > tested it in, I never saw these... > > > > Looking at the code, the problem is obvious. It looks like it came in on a > late commit to the mmccam integration branch. I'll revert which should > solve your problem. Thanks, I can confirm that r320850 allows booting again. Compared to pre-r320844, sdhci(4) now still is incredibly noisy in the non-MMCCAM path, though. Can we get this removed again or at least put under sdhci_debug, along with removing the memory overhead for the non-MMCCAM case and some of the style(9) violations/inconsistencies (re-)introduced, using something like the attached patch? > Looking at my notes and test systems, I did test this > on a NUC, but it was an older version w/o the patch I just reverted. Sorry > for the hassle. r320850 Well, I might have given a different impression, but I don't actually care about NUCs as such; they are just a relatively cheap way of obtaining various combinations of Intel MMC/SDXC controllers and eMMC chips and are an acceptable loss should I ever manage to kill an eMMC chip during development. Marius --cWoXeonUoKmBZSoM Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="sdhci.diff" Index: sdhci.c =================================================================== --- sdhci.c (revision 320850) +++ sdhci.c (working copy) @@ -91,7 +91,7 @@ static void sdhci_start_data(struct sdhci_slot *sl static void sdhci_card_poll(void *); static void sdhci_card_task(void *, int); -/* CAM-related */ +#ifdef MMCCAM int sdhci_cam_get_possible_host_clock(struct sdhci_slot *slot, int proposed_clock); static int sdhci_cam_update_ios(struct sdhci_slot *slot); static int sdhci_cam_request(struct sdhci_slot *slot, union ccb *ccb); @@ -98,6 +98,7 @@ static int sdhci_cam_request(struct sdhci_slot *sl static void sdhci_cam_action(struct cam_sim *sim, union ccb *ccb); static void sdhci_cam_poll(struct cam_sim *sim); static int sdhci_cam_settran_settings(struct sdhci_slot *slot, union ccb *ccb); +#endif /* helper routines */ static void sdhci_dumpregs(struct sdhci_slot *slot); @@ -1021,10 +1022,6 @@ sdhci_generic_update_ios(device_t brdev, device_t struct sdhci_slot *slot = device_get_ivars(reqdev); struct mmc_ios *ios = &slot->host.ios; - device_printf(brdev, "This is a bridge device\n"); - device_printf(reqdev, "This is a request device\n"); - - slot_printf(slot, " <--- The locking slot is this\n"); SDHCI_LOCK(slot); /* Do full reset on bus power down to clear from any state. */ if (ios->power_mode == power_off) { @@ -1121,8 +1118,9 @@ static void sdhci_req_done(struct sdhci_slot *slot) { union ccb *ccb; + if (sdhci_debug > 1) - slot_printf(slot, "sdhci_req_done()\n"); + slot_printf(slot, "%s\n", __func_); if (slot->ccb != NULL && slot->curcmd != NULL) { callout_stop(&slot->timeout_callout); ccb = slot->ccb; @@ -1139,7 +1137,7 @@ sdhci_req_done(struct sdhci_slot *slot) } } #else -static void +static void sdhci_req_done(struct sdhci_slot *slot) { struct mmc_request *req; @@ -1320,7 +1318,7 @@ sdhci_finish_command(struct sdhci_slot *slot) if (sdhci_debug > 1) slot_printf(slot, "%s: called, err %d flags %d\n", - __func__, slot->curcmd->error, slot->curcmd->flags); + __func__, slot->curcmd->error, slot->curcmd->flags); slot->cmd_done = 1; /* * Interrupt aggregation: Restore command interrupt. @@ -1356,8 +1354,8 @@ sdhci_finish_command(struct sdhci_slot *slot) } if (sdhci_debug > 1) printf("Resp: %02x %02x %02x %02x\n", - slot->curcmd->resp[0], slot->curcmd->resp[1], - slot->curcmd->resp[2], slot->curcmd->resp[3]); + slot->curcmd->resp[0], slot->curcmd->resp[1], + slot->curcmd->resp[2], slot->curcmd->resp[3]); /* If data ready - finish. */ if (slot->data_done) @@ -1441,9 +1439,10 @@ sdhci_start_data(struct sdhci_slot *slot, struct m WR2(slot, SDHCI_BLOCK_COUNT, (data->len + 511) / 512); if (sdhci_debug > 1) - slot_printf(slot, "Block size: %02x, count %lu\n", (unsigned int) - SDHCI_MAKE_BLKSZ(DMA_BOUNDARY, (data->len < 512)?data->len:512), - (unsigned long)(data->len + 511) / 512); + slot_printf(slot, "Block size: %02x, count %lu\n", + (unsigned int)SDHCI_MAKE_BLKSZ(DMA_BOUNDARY, (data->len < + 512) ? data->len : 512), (unsigned long)(data->len + 511) / + 512); } void @@ -1657,6 +1656,7 @@ static void sdhci_data_irq(struct sdhci_slot *slot, uint32_t intmask) { struct mmc_data *data; + size_t left; if (!slot->curcmd) { slot_printf(slot, "Got data interrupt 0x%08x, but " @@ -1702,7 +1702,6 @@ sdhci_data_irq(struct sdhci_slot *slot, uint32_t i /* Handle DMA border. */ if (intmask & SDHCI_INT_DMA_END) { data = slot->curcmd->data; - size_t left; /* Unload DMA buffer ... */ left = data->len - slot->offset; @@ -1910,7 +1909,8 @@ sdhci_generic_write_ivar(device_t bus, device_t ch uint32_t clock, max_clock; int i; - slot_printf(slot, "sdhci_generic_write_ivar, var=%d\n", which); + if (sdhci_debug > 1) + slot_printf(slot, "%s: var=%d\n", __func__, which); switch (which) { default: return (EINVAL); @@ -1976,6 +1976,7 @@ sdhci_generic_write_ivar(device_t bus, device_t ch return (0); } +#ifdef MMCCAM /* CAM-related functions */ #include <cam/cam.h> #include <cam/cam_ccb.h> @@ -2294,5 +2295,6 @@ sdhci_cam_request(struct sdhci_slot *slot, union c } return (0); } +#endif /* MMCCAM */ MODULE_VERSION(sdhci, 1); Index: sdhci.h =================================================================== --- sdhci.h (revision 320850) +++ sdhci.h (working copy) @@ -28,6 +28,8 @@ #ifndef __SDHCI_H__ #define __SDHCI_H__ +#include "opt_mmccam.h" + #define DMA_BLOCK_SIZE 4096 #define DMA_BOUNDARY 0 /* DMA reload every 4K */ @@ -368,12 +370,13 @@ struct sdhci_slot { #define PLATFORM_DATA_STARTED 8 /* Data xfer is handled by platform */ struct mtx mtx; /* Slot mutex */ - /* CAM stuff */ +#ifdef MMCCAM union ccb *ccb; struct cam_devq *devq; struct cam_sim *sim; struct mtx sim_mtx; u_char card_present; /* XXX Maybe derive this from elsewhere? */ +#endif }; int sdhci_generic_read_ivar(device_t bus, device_t child, int which, @@ -400,6 +403,8 @@ bool sdhci_generic_get_card_present(device_t brdev void sdhci_generic_set_uhs_timing(device_t brdev, struct sdhci_slot *slot); void sdhci_handle_card_present(struct sdhci_slot *slot, bool is_present); -/* CAM-related */ +#ifdef MMCCAM void sdhci_cam_start_slot(struct sdhci_slot *slot); +#endif + #endif /* __SDHCI_H__ */ --cWoXeonUoKmBZSoM--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20170709223747.GC38352>