On 07/08/17 18:51, Ian Molton wrote:
> On 07/08/17 12:25, Arend van Spriel wrote:
>>> Handling of -ENOMEDIUM is altered, but as that's pretty much broken
>>> anyway
>>> we can ignore that.
>>
>> Please explain why you think it is broken.
>
> Not got the code to hand right now, but from memory, theres a trapdoor
> case where the state can wind up set to something that prevents it ever
> being changed again. I'll dig it up when I get back from holiday (this
> next few days).
Hi,
Here is the function I had in mind:
void brcmf_sdiod_change_state(struct brcmf_sdio_dev *sdiodev,
enum brcmf_sdiod_state state)
{
if (sdiodev->state == BRCMF_SDIOD_NOMEDIUM ||
state == sdiodev->state)
return;
brcmf_dbg(TRACE, "%d -> %d\n", sdiodev->state, state);
switch (sdiodev->state) {
case BRCMF_SDIOD_DATA:
/* any other state means bus interface is down */
brcmf_bus_change_state(sdiodev->bus_if, BRCMF_BUS_DOWN);
break;
case BRCMF_SDIOD_DOWN:
/* transition from DOWN to DATA means bus interface is up */
if (state == BRCMF_SDIOD_DATA)
brcmf_bus_change_state(sdiodev->bus_if,
BRCMF_BUS_UP);
break;
default:
break;
}
sdiodev->state = state;
}
If it's *ever* called with state = BRCMF_SDIOD_NOMEDIUM it will
eventually (last line) set sdiodev->state to the same value.
If its ever called again, the first if() statement will make it return
before ever changing sdiodev->state again, no matter what value is
passed for state.
This has to be a bug, surely?
-Ian
