Re: ide: Remove ide_spin_wait_hwgroup() and use special requests instead

[Elias Oltmanns <eo@nebensachen.de>]

Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:
> On Wednesday 13 August 2008, Elias Oltmanns wrote:
>> Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:
>
>> > Hi,
>> >
>> > On Monday 11 August 2008, Elias Oltmanns wrote:
[...]
>> >> +	rq->cmd_len = DEVSET_CDB_LEN;
>> >> +	ds = (ide_devset_cdb_t *)rq->cmd;
>> >> +	ds->opcode = REQ_DEVSET_EXEC;
>> >> +	ds->arg = arg;
>> >
>> > How's about just doing:
>> >
>> > 	rq->cmd[0] = REQ_DEVSET_EXEC;
>> > 	(int *)rq->cmd[1] = arg;
>> 
>>   CC [M]  drivers/ide/ide-io.o
>> drivers/ide/ide-io.c: In function 'ide_devset_execute':
>> drivers/ide/ide-io.c:748: warning: cast to pointer from integer of different size
>> drivers/ide/ide-io.c:748: error: lvalue required as left operand of assignment
>
> Heh, I must have been falling asleep already when writing this,
> it should have been:
>
> 	*(int *)&rq->cmd[1] = arg;

Well, this I can make sense of.

>
>> Personally, I'd feel more comfortable with casting rq->cmd to a
>> dedicated struct, especially since I'm not exactly a wizard when it
>> comes to casting. Naturally, if you prefer something else (and I get it
>> to work), I'll happily accept that too.
>
> What I find ugly about this dedicated struct is that it overlaps with
> rq->cmd[0] and in all other places we just use rq->cmd[0] directly
> (+ it is very easy for people unfamiliar with the code to overlook it).

Yes, I see.

>
> Then if 'opcode' gets removed all that is left is 'arg' so the struct
> no loger makes much sense.
>
> I fixed it locally (interdiff attached), I hope you're fine with it.

Certainly.

Regards,

Elias