On 2/16/22 20:38, John Garry wrote: > On 16/02/2022 11:36, Damien Le Moal wrote: >> On 2/15/22 19:55, John Garry wrote: >>> On 14/02/2022 02:17, Damien Le Moal wrote: >>>> Avoid repeatedly declaring "struct task_status_struct *ts" to handle >>>> error cases by declaring this variable for the entire function scope. >>>> This allows simplifying the error cases, and together with the addition >>>> of blank lines make the code more readable. >>>> >>>> Reviewed-by: John Garry<john.garry@xxxxxxxxxx> >>>> Signed-off-by: Damien Le Moal<damien.lemoal@xxxxxxxxxxxxxxxxxx> >>>> --- >>> >>> I assume that this can now just be merged with 30/31 >> > > Hi Damien, > >> patch 30 cleans up pm8001_task_exec(). This patch is for >> pm8001_queue_command(). I preferred to separate to facilitate review. >> But if you insist, I can merge these into a much bigger "code cleanup" >> patch... >> > I don't mind really. > > BTW, on a separate topic, IIRC you said that rmmod hangs for this driver > - if so, did you investigate why? The problem is gone with the fixes. I suspect it was due to the buggy non-data command handling (likely, the flush issued when stopping the device on rmmod). I have not tackled/tried again the QD change failure though. Preparing v4 now. Will check the QD change. > > Thanks, > John -- Damien Le Moal Western Digital Research