Fix Xilinx SystemACE driver to handle empty CF slot

From: Grant Likely <grant.likely@secretlab.ca>

The SystemACE driver does not handle an empty CF slot gracefully.  An
empty CF slot ends up hanging the system.  This patch adds a check for
the CF state and stops trying to process requests if the slot is empty.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---

 drivers/block/xsysace.c |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)

Oops, sorry Jens.  I forgot to CC: you on this patch.

g.

On Sat, Feb 28, 2009 at 1:46 PM, Grant Likely <grant.likely@secretlab.ca> wrote:
> From: Grant Likely <grant.likely@secretlab.ca>
>
> The SystemACE driver does not handle an empty CF slot gracefully.  An
> empty CF slot ends up hanging the system.  This patch adds a check for
> the CF state and stops trying to process requests if the slot is empty.
>
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> ---
>
>  drivers/block/xsysace.c |   22 ++++++++++++++++++++++
>  1 files changed, 22 insertions(+), 0 deletions(-)
>
>
> diff --git a/drivers/block/xsysace.c b/drivers/block/xsysace.c
> index 381d686..ec5b8ca 100644
> --- a/drivers/block/xsysace.c
> +++ b/drivers/block/xsysace.c
> @@ -489,6 +489,28 @@ static void ace_fsm_dostate(struct ace_device *ace)
>                ace->fsm_state, ace->id_req_count);
>  #endif
>
> +       /* Verify that there is actually a CF in the slot.  If not, then
> +        * bail out back to the idle state and wake up all the waiters */
> +       status = ace_in32(ace, ACE_STATUS);
> +       if ((status & ACE_STATUS_CFDETECT) == 0) {
> +               ace->fsm_state = ACE_FSM_STATE_IDLE;
> +               ace->media_change = 1;
> +               set_capacity(ace->gd, 0);
> +               dev_info(ace->dev, "No CF in slot\n");
> +
> +               /* Drop all pending requests */
> +               while ((req = elv_next_request(ace->queue)) != NULL)
> +                       end_request(req, 0);
> +
> +               /* Drop back to IDLE state and notify waiters */
> +               ace->fsm_state = ACE_FSM_STATE_IDLE;
> +               ace->id_result = -EIO;
> +               while (ace->id_req_count) {
> +                       complete(&ace->id_completion);
> +                       ace->id_req_count--;
> +               }
> +       }
> +
>        switch (ace->fsm_state) {
>        case ACE_FSM_STATE_IDLE:
>                /* See if there is anything to do */
>
>

On Fri, Mar 06 2009, Grant Likely wrote:
> Oops, sorry Jens.  I forgot to CC: you on this patch.
> 
> g.
> 
> On Sat, Feb 28, 2009 at 1:46 PM, Grant Likely <grant.likely@secretlab.ca> wrote:
> > From: Grant Likely <grant.likely@secretlab.ca>
> >
> > The SystemACE driver does not handle an empty CF slot gracefully.  An
> > empty CF slot ends up hanging the system.  This patch adds a check for
> > the CF state and stops trying to process requests if the slot is empty.

So with patches like this, it's always nice to know what your target is.
Do you want this in .29, or just queued up for .30? It's not always easy
to judge the urgency of such patches :-)

Also, I note that you are using end_request() throughout the driver. We
really want to get away from that, you should be using blk_end_request()
as that will handle full requests and not just segment-by-segment. No
worries for this patch, but you may want to consider that for a future
patch.

On Fri, Mar 6, 2009 at 1:46 PM, Jens Axboe <jens.axboe@oracle.com> wrote:
> On Fri, Mar 06 2009, Grant Likely wrote:
>> > The SystemACE driver does not handle an empty CF slot gracefully.  An
>> > empty CF slot ends up hanging the system.  This patch adds a check for
>> > the CF state and stops trying to process requests if the slot is empty.
>
> So with patches like this, it's always nice to know what your target is.
> Do you want this in .29, or just queued up for .30? It's not always easy
> to judge the urgency of such patches :-)

The driver completely falls down and hangs the system if the CF slot
is empty, so I would like to get it into .29.  On the other hand, it
has been a long standing issue, so if merging it will raise any
eyebrows then I'm okay to wait for .30.

> Also, I note that you are using end_request() throughout the driver. We
> really want to get away from that, you should be using blk_end_request()
> as that will handle full requests and not just segment-by-segment. No
> worries for this patch, but you may want to consider that for a future
> patch.

Okay, will do.

Thanks,
g.