Changeset View
Standalone View
sys/cam/mmc/mmc_sim.c
- This file was added.
/*- | |||||
* Copyright (c) 2020 Emmanuel Vadot <manu@FreeBSD.org> | |||||
* | |||||
* Redistribution and use in source and binary forms, with or without | |||||
* modification, are permitted provided that the following conditions | |||||
* are met: | |||||
* 1. Redistributions of source code must retain the above copyright | |||||
* notice, this list of conditions and the following disclaimer. | |||||
* 2. Redistributions in binary form must reproduce the above copyright | |||||
* notice, this list of conditions and the following disclaimer in the | |||||
* documentation and/or other materials provided with the distribution. | |||||
* | |||||
* THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR | |||||
* IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES | |||||
* OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. | |||||
* IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, | |||||
* INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, | |||||
* BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; | |||||
* LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED | |||||
* AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, | |||||
* OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY | |||||
* OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF | |||||
* SUCH DAMAGE. | |||||
* | |||||
* $FreeBSD$ | |||||
*/ | |||||
#include <sys/cdefs.h> | |||||
__FBSDID("$FreeBSD$"); | |||||
#include <sys/param.h> | |||||
#include <sys/kernel.h> | |||||
#include <sys/systm.h> | |||||
#include <sys/malloc.h> | |||||
#include <sys/mutex.h> | |||||
#include <cam/cam.h> | |||||
#include <cam/cam_ccb.h> | |||||
#include <cam/cam_debug.h> | |||||
#include <cam/cam_sim.h> | |||||
#include <cam/cam_xpt_sim.h> | |||||
#include <cam/mmc/mmc_sim.h> | |||||
#include "mmc_sim_if.h" | |||||
static void | |||||
mmc_cam_default_poll(struct cam_sim *sim) | |||||
{ | |||||
return; | |||||
} | |||||
static void | |||||
mmc_cam_sim_default_action(struct cam_sim *sim, union ccb *ccb) | |||||
{ | |||||
struct mmc_sim *mmc_sim; | |||||
struct ccb_trans_settings_mmc mmc; | |||||
int rv; | |||||
mmc_sim = cam_sim_softc(sim); | |||||
if (mmc_sim == NULL) { | |||||
ccb->ccb_h.status = CAM_SEL_TIMEOUT; | |||||
xpt_done(ccb); | |||||
return; | |||||
} | |||||
mtx_assert(&mmc_sim->mtx, MA_OWNED); | |||||
switch (ccb->ccb_h.func_code) { | |||||
case XPT_PATH_INQ: | |||||
rv = MMC_SIM_GET_TRAN_SETTINGS(mmc_sim->dev, &mmc); | |||||
imp: Is this synchronous w/o sleeping? Ditto below for the GET_TRAN_SETTINGS?
| |||||
manuAuthorUnsubmitted Done Inline ActionsThis one is okay, same for GET_TRAN_SETTINGS, problem is that we need SET_TRAN_SETTINGS to be async as we deal with clock and regulators and this make aquiring the global sx lock for clock/regulator. manu: This one is okay, same for GET_TRAN_SETTINGS, problem is that we need SET_TRAN_SETTINGS to be… | |||||
impUnsubmitted Not Done Inline ActionsIn SCSI land, where CAM was born, the SIMs seem to just set internal tables/state rather than do anything more complicated than a few outl() or memory write to the controller... I'm not sure how this mismatch will play out... There's also a convention where if both the scsi settings and the spi settings are 0, it's just a probe to see if something it there... Not sure where that convention is implemented, or if we have to worry about it for MMC.... I do understand the quandary, though, because I know it can be hard to change actual modes in MMC/SD land. We may need a different CCB to actually do the change... I need to study more. imp: In SCSI land, where CAM was born, the SIMs seem to just set internal tables/state rather than… | |||||
scottlUnsubmitted Not Done Inline ActionsThis description isn't really true, unfortunately. Changing the negotiation parameters in parallel SCSI is an asynchronous operation too. Yes, the SIM is responsible for delivering the change request to the controller, but you typically don't know the results of the request until a new CDB is issued and completed. Instead of having the driver invent a synchronous mechanism to set the property and then check the results, CAM was designed around SET_TRANS_SETTINGS being a fire-and-forget mechanism. The action doesn't need to be completed, or even successful, for the CCB to complete (and in parallel, SCSI, it often is not complete and not successful). Instead, the results can be determined later. In fact, the XPT doesn't care much about the results; transport speed is just a user-visible parameter, nothing more. The driver is responsible for returning values in GET_TRANS_SETTINGS, and it's perfectly fine for it to return old value while the new settings are still being processed. Please don't use an sx lock for this. I'll veto the review if you do. There's no reason that I can think of to use a sleepable lock in any I/O driver, and SX locks in FreeBSD are becoming anachronistic and should be avoided anyways. If you think that you need to use an sx lock, please contact me directly and we can review the architectural decisions that are driving that request. scottl: This description isn't really true, unfortunately. Changing the negotiation parameters in… | |||||
manuAuthorUnsubmitted Done Inline ActionsSX lock aren't used in mmc drivers but in the clock and regulator framework. How does one wait for SET_TRANS_SETTINGS to be done ? For GET_TRANS_SETTINGS it's ok right now from what I could see. manu: SX lock aren't used in mmc drivers but in the clock and regulator framework.
How does one wait… | |||||
scottlUnsubmitted Not Done Inline ActionsThe key is to not wait for SET_TRANS_SETTINGS to be done. One idea would be to schedule the work into a taskqueue, and return immediately. You return an error in the CCB if the parameters are incorrect or if it's not possible to schedule the taskq, otherwise just return CAM_REQ_CMP. When your taskqueue runs and completes, you can mark some internal state so that your driver knows, but that's not important to the higher level of CAM. Also, don't synchronizing GET_TRANS_SETTINGS on this taskqueue (or anything else), it's fine to return stale information in it. scottl: The key is to not wait for SET_TRANS_SETTINGS to be done. One idea would be to schedule the… | |||||
manuAuthorUnsubmitted Done Inline ActionsThe problem isn't the driver side of thing, the problem is that a lot of stuff in mmc_xpt.c is done synchronously as it's needed by the mmc protocol (discovery phase for example). manu: The problem isn't the driver side of thing, the problem is that a lot of stuff in mmc_xpt.c is… | |||||
if (rv != 0) { | |||||
ccb->ccb_h.status = CAM_REQ_INVALID; | |||||
} else { | |||||
mmc_path_inq(&ccb->cpi, "Deglitch Networks", | |||||
sim, mmc.host_max_data); | |||||
} | |||||
break; | |||||
case XPT_GET_TRAN_SETTINGS: | |||||
{ | |||||
struct ccb_trans_settings *cts = &ccb->cts; | |||||
rv = MMC_SIM_GET_TRAN_SETTINGS(mmc_sim->dev, &cts->proto_specific.mmc); | |||||
if (rv != 0) | |||||
ccb->ccb_h.status = CAM_REQ_INVALID; | |||||
else { | |||||
cts->protocol = PROTO_MMCSD; | |||||
cts->protocol_version = 1; | |||||
cts->transport = XPORT_MMCSD; | |||||
cts->transport_version = 1; | |||||
cts->xport_specific.valid = 0; | |||||
ccb->ccb_h.status = CAM_REQ_CMP; | |||||
} | |||||
break; | |||||
} | |||||
case XPT_SET_TRAN_SETTINGS: | |||||
{ | |||||
struct ccb_trans_settings *cts = &ccb->cts; | |||||
rv = MMC_SIM_SET_TRAN_SETTINGS(mmc_sim->dev, &cts->proto_specific.mmc); | |||||
if (rv != 0) | |||||
ccb->ccb_h.status = CAM_REQ_INVALID; | |||||
else | |||||
ccb->ccb_h.status = CAM_REQ_CMP; | |||||
break; | |||||
} | |||||
case XPT_RESET_BUS: | |||||
ccb->ccb_h.status = CAM_REQ_CMP; | |||||
break; | |||||
case XPT_MMC_IO: | |||||
{ | |||||
rv = MMC_SIM_CAM_REQUEST(mmc_sim->dev, ccb); | |||||
if (rv != 0) | |||||
ccb->ccb_h.status = CAM_REQ_INPROG; | |||||
else | |||||
ccb->ccb_h.status = CAM_REQ_INVALID; | |||||
return; | |||||
/* NOTREACHED */ | |||||
break; | |||||
impUnsubmitted Not Done Inline ActionsThese two lines aren't needed. imp: These two lines aren't needed. | |||||
} | |||||
default: | |||||
ccb->ccb_h.status = CAM_REQ_INVALID; | |||||
break; | |||||
} | |||||
xpt_done(ccb); | |||||
return; | |||||
} | |||||
int | |||||
mmc_cam_sim_alloc(device_t dev, const char *name, struct mmc_sim *mmc_sim) | |||||
{ | |||||
char sim_name[64], mtx_name[64]; | |||||
mmc_sim->dev = dev; | |||||
if ((mmc_sim->devq = cam_simq_alloc(1)) == NULL) { | |||||
goto fail; | |||||
} | |||||
snprintf(sim_name, sizeof(sim_name), "%s_sim", name); | |||||
snprintf(mtx_name, sizeof(mtx_name), "%s_mtx", name); | |||||
mtx_init(&mmc_sim->mtx, sim_name, NULL, MTX_DEF); | |||||
mmc_sim->sim = cam_sim_alloc_dev(mmc_cam_sim_default_action, | |||||
mmc_cam_default_poll, | |||||
name, mmc_sim, dev, | |||||
&mmc_sim->mtx, 1, 1, mmc_sim->devq); | |||||
if (mmc_sim->sim == NULL) { | |||||
cam_simq_free(mmc_sim->devq); | |||||
device_printf(dev, "cannot allocate CAM SIM\n"); | |||||
goto fail; | |||||
} | |||||
mtx_lock(&mmc_sim->mtx); | |||||
if (xpt_bus_register(mmc_sim->sim, dev, 0) != 0) { | |||||
device_printf(dev, "cannot register SCSI pass-through bus\n"); | |||||
cam_sim_free(mmc_sim->sim, FALSE); | |||||
cam_simq_free(mmc_sim->devq); | |||||
mtx_unlock(&mmc_sim->mtx); | |||||
goto fail; | |||||
} | |||||
mtx_unlock(&mmc_sim->mtx); | |||||
return (0); | |||||
fail: | |||||
mmc_cam_sim_free(mmc_sim); | |||||
return (1); | |||||
} | |||||
void | |||||
mmc_cam_sim_free(struct mmc_sim *mmc_sim) | |||||
{ | |||||
if (mmc_sim->sim != NULL) { | |||||
mtx_lock(&mmc_sim->mtx); | |||||
xpt_bus_deregister(cam_sim_path(mmc_sim->sim)); | |||||
cam_sim_free(mmc_sim->sim, FALSE); | |||||
mtx_unlock(&mmc_sim->mtx); | |||||
} | |||||
if (mmc_sim->devq != NULL) | |||||
cam_simq_free(mmc_sim->devq); | |||||
} | |||||
void | |||||
mmc_cam_sim_discover(struct mmc_sim *mmc_sim) | |||||
{ | |||||
mmccam_start_discovery(mmc_sim->sim); | |||||
} |
Is this synchronous w/o sleeping? Ditto below for the GET_TRAN_SETTINGS?