Hi Steve,
On Wed, Nov 18, 2015 at 4:42 AM, Steve deRosier <derosier@xxxxxxxxx> wrote:
> Hi Julian,
>
> Thanks for looking at this.
>
> In short - I agree with your review and will do most of them. As a
> well as a few minor changes suggested by the kbuild test robot. Expect
> a new version shortly.
I'm only really reviewing for style, you might want to wait for Kalle
or someone else to do a technical review.
> On Mon, Nov 16, 2015 at 10:25 PM, Julian Calaby <julian.calaby@xxxxxxxxx> wrote:
>>>
>>> static void __exit ath6kl_sdio_exit(void)
>>> {
>>> sdio_unregister_driver(&ath6kl_sdio_driver);
>>> +
>>> +#ifdef CONFIG_GPIOLIB
>>> + /* Delay to avoid pulling the plug on the chip when an irq is pending
>>> + * and then getting a spurious message:
>>> + * "ath6kl: failed to get pending recv messages: -125"
>>> + */
>>> + msleep(300);
>>
>> Is there some actual synchronisation you can do here against the IRQ?
>> 300msec isn't a long time to wait, but it seems wrong here.
>>
>
> Considering this is only called on exit, I wasn't too worried about
> this, but then again, on reboot as well as a few types of
> reconfiguring the interface speed is an important consideration for
> our system.
>
> I'm open to suggestions. I had looked at this and didn't see an
> obvious way to try to sync with the IRQ as it goes through several
> subsystems.
Maybe having this in ath6kl is having it in the wrong layer?
I'm guessing this is on some board that already has a DTS. Do the mmc
pwrseq drivers do what you're after?
https://www.kernel.org/doc/Documentation/devicetree/bindings/mmc/mmc-pwrseq-emmc.txt
or https://www.kernel.org/doc/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
> But looking at it again in a different light...
>
> 125 is ECANCELED. Which is exactly right and appropriate, and
> ath6kl_htc_rxmsg_pending_handler() where the message comes from does a
> proper cleanup, or so it looks like. I wonder if maybe we can consider
> ECANCELED not an error that needs an error message as it might be a
> reasonable case?
>
> So instead of synchronization, just consider that particular status an
> expected and properly handled exception condition and not print an
> error?
Printing it as a debug instead of an error is probably a better option
here if you want to "silence" that error.
Thanks,
--
Julian Calaby
Email: julian.calaby@xxxxxxxxx
Profile: http://www.google.com/profiles/julian.calaby/
_______________________________________________
ath6kl mailing list
ath6kl@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/ath6kl
