Skip to content

Commit

Permalink
Merge branch 'net-ipa-add-a-driver-shutdown-callback'
Browse files Browse the repository at this point in the history 
Alex Elder says:

====================
net: ipa: add a driver shutdown callback

The final patch in this series adds a driver shutdown callback for
the IPA driver.  The patches leading up to that address some issues
encountered while ensuring that callback worked as expected:
  - The first just reports a little more information when channels
    or event rings are in unexpected states
  - The second patch recognizes a condition where an as-yet-unused
    channel does not require a reset during teardown
  - The third patch explicitly ignores a certain error condition,
    because it can't be avoided, and is harmless if it occurs
  - The fourth properly handles requests to retry a channel HALT
    request
  - The fifth makes a second attempt to stop modem activity during
    shutdown if it's busy

The shutdown callback is implemented by calling the existing remove
callback function (reporting if that returns an error).
====================

Link: https://lore.kernel.org/r/20201119224929.23819-1-elder@linaro.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
  • Loading branch information
Jakub Kicinski committed Nov 21, 2020
2 parents 0ee6de2 + ae1d72f commit c900378
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 27 deletions.
101 changes: 76 additions & 25 deletions drivers/net/ipa/gsi.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@
#define GSI_CMD_TIMEOUT 5 /* seconds */

#define GSI_CHANNEL_STOP_RX_RETRIES 10
#define GSI_CHANNEL_MODEM_HALT_RETRIES 10

#define GSI_MHI_EVENT_ID_START 10 /* 1st reserved event id */
#define GSI_MHI_EVENT_ID_END 16 /* Last reserved event id */
Expand Down Expand Up @@ -365,15 +366,15 @@ static int gsi_evt_ring_alloc_command(struct gsi *gsi, u32 evt_ring_id)
/* Get initial event ring state */
evt_ring->state = gsi_evt_ring_state(gsi, evt_ring_id);
if (evt_ring->state != GSI_EVT_RING_STATE_NOT_ALLOCATED) {
dev_err(gsi->dev, "bad event ring state %u before alloc\n",
evt_ring->state);
dev_err(gsi->dev, "event ring %u bad state %u before alloc\n",
evt_ring_id, evt_ring->state);
return -EINVAL;
}

ret = evt_ring_command(gsi, evt_ring_id, GSI_EVT_ALLOCATE);
if (!ret && evt_ring->state != GSI_EVT_RING_STATE_ALLOCATED) {
dev_err(gsi->dev, "bad event ring state %u after alloc\n",
evt_ring->state);
dev_err(gsi->dev, "event ring %u bad state %u after alloc\n",
evt_ring_id, evt_ring->state);
ret = -EIO;
}

Expand All @@ -389,15 +390,15 @@ static void gsi_evt_ring_reset_command(struct gsi *gsi, u32 evt_ring_id)

if (state != GSI_EVT_RING_STATE_ALLOCATED &&
state != GSI_EVT_RING_STATE_ERROR) {
dev_err(gsi->dev, "bad event ring state %u before reset\n",
evt_ring->state);
dev_err(gsi->dev, "event ring %u bad state %u before reset\n",
evt_ring_id, evt_ring->state);
return;
}

ret = evt_ring_command(gsi, evt_ring_id, GSI_EVT_RESET);
if (!ret && evt_ring->state != GSI_EVT_RING_STATE_ALLOCATED)
dev_err(gsi->dev, "bad event ring state %u after reset\n",
evt_ring->state);
dev_err(gsi->dev, "event ring %u bad state %u after reset\n",
evt_ring_id, evt_ring->state);
}

/* Issue a hardware de-allocation request for an allocated event ring */
Expand All @@ -407,15 +408,15 @@ static void gsi_evt_ring_de_alloc_command(struct gsi *gsi, u32 evt_ring_id)
int ret;

if (evt_ring->state != GSI_EVT_RING_STATE_ALLOCATED) {
dev_err(gsi->dev, "bad event ring state %u before dealloc\n",
evt_ring->state);
dev_err(gsi->dev, "event ring %u state %u before dealloc\n",
evt_ring_id, evt_ring->state);
return;
}

ret = evt_ring_command(gsi, evt_ring_id, GSI_EVT_DE_ALLOC);
if (!ret && evt_ring->state != GSI_EVT_RING_STATE_NOT_ALLOCATED)
dev_err(gsi->dev, "bad event ring state %u after dealloc\n",
evt_ring->state);
dev_err(gsi->dev, "event ring %u bad state %u after dealloc\n",
evt_ring_id, evt_ring->state);
}

/* Fetch the current state of a channel from hardware */
Expand Down Expand Up @@ -479,7 +480,8 @@ static int gsi_channel_alloc_command(struct gsi *gsi, u32 channel_id)
/* Get initial channel state */
state = gsi_channel_state(channel);
if (state != GSI_CHANNEL_STATE_NOT_ALLOCATED) {
dev_err(dev, "bad channel state %u before alloc\n", state);
dev_err(dev, "channel %u bad state %u before alloc\n",
channel_id, state);
return -EINVAL;
}

Expand All @@ -488,7 +490,8 @@ static int gsi_channel_alloc_command(struct gsi *gsi, u32 channel_id)
/* Channel state will normally have been updated */
state = gsi_channel_state(channel);
if (!ret && state != GSI_CHANNEL_STATE_ALLOCATED) {
dev_err(dev, "bad channel state %u after alloc\n", state);
dev_err(dev, "channel %u bad state %u after alloc\n",
channel_id, state);
ret = -EIO;
}

Expand All @@ -505,7 +508,8 @@ static int gsi_channel_start_command(struct gsi_channel *channel)
state = gsi_channel_state(channel);
if (state != GSI_CHANNEL_STATE_ALLOCATED &&
state != GSI_CHANNEL_STATE_STOPPED) {
dev_err(dev, "bad channel state %u before start\n", state);
dev_err(dev, "channel %u bad state %u before start\n",
gsi_channel_id(channel), state);
return -EINVAL;
}

Expand All @@ -514,7 +518,8 @@ static int gsi_channel_start_command(struct gsi_channel *channel)
/* Channel state will normally have been updated */
state = gsi_channel_state(channel);
if (!ret && state != GSI_CHANNEL_STATE_STARTED) {
dev_err(dev, "bad channel state %u after start\n", state);
dev_err(dev, "channel %u bad state %u after start\n",
gsi_channel_id(channel), state);
ret = -EIO;
}

Expand All @@ -538,7 +543,8 @@ static int gsi_channel_stop_command(struct gsi_channel *channel)

if (state != GSI_CHANNEL_STATE_STARTED &&
state != GSI_CHANNEL_STATE_STOP_IN_PROC) {
dev_err(dev, "bad channel state %u before stop\n", state);
dev_err(dev, "channel %u bad state %u before stop\n",
gsi_channel_id(channel), state);
return -EINVAL;
}

Expand All @@ -553,7 +559,8 @@ static int gsi_channel_stop_command(struct gsi_channel *channel)
if (state == GSI_CHANNEL_STATE_STOP_IN_PROC)
return -EAGAIN;

dev_err(dev, "bad channel state %u after stop\n", state);
dev_err(dev, "channel %u bad state %u after stop\n",
gsi_channel_id(channel), state);

return -EIO;
}
Expand All @@ -570,7 +577,10 @@ static void gsi_channel_reset_command(struct gsi_channel *channel)
state = gsi_channel_state(channel);
if (state != GSI_CHANNEL_STATE_STOPPED &&
state != GSI_CHANNEL_STATE_ERROR) {
dev_err(dev, "bad channel state %u before reset\n", state);
/* No need to reset a channel already in ALLOCATED state */
if (state != GSI_CHANNEL_STATE_ALLOCATED)
dev_err(dev, "channel %u bad state %u before reset\n",
gsi_channel_id(channel), state);
return;
}

Expand All @@ -579,7 +589,8 @@ static void gsi_channel_reset_command(struct gsi_channel *channel)
/* Channel state will normally have been updated */
state = gsi_channel_state(channel);
if (!ret && state != GSI_CHANNEL_STATE_ALLOCATED)
dev_err(dev, "bad channel state %u after reset\n", state);
dev_err(dev, "channel %u bad state %u after reset\n",
gsi_channel_id(channel), state);
}

/* Deallocate an ALLOCATED GSI channel */
Expand All @@ -592,7 +603,8 @@ static void gsi_channel_de_alloc_command(struct gsi *gsi, u32 channel_id)

state = gsi_channel_state(channel);
if (state != GSI_CHANNEL_STATE_ALLOCATED) {
dev_err(dev, "bad channel state %u before dealloc\n", state);
dev_err(dev, "channel %u bad state %u before dealloc\n",
channel_id, state);
return;
}

Expand All @@ -601,7 +613,8 @@ static void gsi_channel_de_alloc_command(struct gsi *gsi, u32 channel_id)
/* Channel state will normally have been updated */
state = gsi_channel_state(channel);
if (!ret && state != GSI_CHANNEL_STATE_NOT_ALLOCATED)
dev_err(dev, "bad channel state %u after dealloc\n", state);
dev_err(dev, "channel %u bad state %u after dealloc\n",
channel_id, state);
}

/* Ring an event ring doorbell, reporting the last entry processed by the AP.
Expand Down Expand Up @@ -1075,10 +1088,38 @@ static void gsi_isr_gp_int1(struct gsi *gsi)
u32 result;
u32 val;

/* This interrupt is used to handle completions of the two GENERIC
* GSI commands. We use these to allocate and halt channels on
* the modem's behalf due to a hardware quirk on IPA v4.2. Once
* allocated, the modem "owns" these channels, and as a result we
* have no way of knowing the channel's state at any given time.
*
* It is recommended that we halt the modem channels we allocated
* when shutting down, but it's possible the channel isn't running
* at the time we issue the HALT command. We'll get an error in
* that case, but it's harmless (the channel is already halted).
*
* For this reason, we silently ignore a CHANNEL_NOT_RUNNING error
* if we receive it.
*/
val = ioread32(gsi->virt + GSI_CNTXT_SCRATCH_0_OFFSET);
result = u32_get_bits(val, GENERIC_EE_RESULT_FMASK);
if (result != GENERIC_EE_SUCCESS)

switch (result) {
case GENERIC_EE_SUCCESS:
case GENERIC_EE_CHANNEL_NOT_RUNNING:
gsi->result = 0;
break;

case GENERIC_EE_RETRY:
gsi->result = -EAGAIN;
break;

default:
dev_err(gsi->dev, "global INT1 generic result %u\n", result);
gsi->result = -EIO;
break;
}

complete(&gsi->completion);
}
Expand Down Expand Up @@ -1590,7 +1631,7 @@ static int gsi_generic_command(struct gsi *gsi, u32 channel_id,
iowrite32(BIT(ERROR_INT), gsi->virt + GSI_CNTXT_GLOB_IRQ_EN_OFFSET);

if (success)
return 0;
return gsi->result;

dev_err(gsi->dev, "GSI generic command %u to channel %u timed out\n",
opcode, channel_id);
Expand All @@ -1606,7 +1647,17 @@ static int gsi_modem_channel_alloc(struct gsi *gsi, u32 channel_id)

static void gsi_modem_channel_halt(struct gsi *gsi, u32 channel_id)
{
(void)gsi_generic_command(gsi, channel_id, GSI_GENERIC_HALT_CHANNEL);
u32 retries = GSI_CHANNEL_MODEM_HALT_RETRIES;
int ret;

do
ret = gsi_generic_command(gsi, channel_id,
GSI_GENERIC_HALT_CHANNEL);
while (ret == -EAGAIN && retries--);

if (ret)
dev_err(gsi->dev, "error %d halting modem channel %u\n",
ret, channel_id);
}

/* Setup function for channels */
Expand Down
1 change: 1 addition & 0 deletions drivers/net/ipa/gsi.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ struct gsi {
u32 type_enabled_bitmap; /* GSI IRQ types enabled */
u32 ieob_enabled_bitmap; /* IEOB IRQ enabled (event rings) */
struct completion completion; /* for global EE commands */
int result; /* Negative errno (generic commands) */
struct mutex mutex; /* protects commands, programming */
};

Expand Down
19 changes: 17 additions & 2 deletions drivers/net/ipa/ipa_main.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -863,6 +863,11 @@ static int ipa_remove(struct platform_device *pdev)

if (ipa->setup_complete) {
ret = ipa_modem_stop(ipa);
/* If starting or stopping is in progress, try once more */
if (ret == -EBUSY) {
usleep_range(USEC_PER_MSEC, 2 * USEC_PER_MSEC);
ret = ipa_modem_stop(ipa);
}
if (ret)
return ret;

Expand All @@ -883,6 +888,15 @@ static int ipa_remove(struct platform_device *pdev)
return 0;
}

static void ipa_shutdown(struct platform_device *pdev)
{
int ret;

ret = ipa_remove(pdev);
if (ret)
dev_err(&pdev->dev, "shutdown: remove returned %d\n", ret);
}

/**
* ipa_suspend() - Power management system suspend callback
* @dev: IPA device structure
Expand Down Expand Up @@ -940,8 +954,9 @@ static const struct dev_pm_ops ipa_pm_ops = {
};

static struct platform_driver ipa_driver = {
.probe = ipa_probe,
.remove = ipa_remove,
.probe = ipa_probe,
.remove = ipa_remove,
.shutdown = ipa_shutdown,
.driver = {
.name = "ipa",
.pm = &ipa_pm_ops,
Expand Down

0 comments on commit c900378

Please sign in to comment.