[PATCH 1/5] spi/mxs: Fix extra CS pulses and read mode in multi-transfer messages
Trent Piepho
tpiepho at gmail.com
Tue Apr 2 08:40:48 EDT 2013
On Tue, Apr 2, 2013 at 3:32 AM, Marek Vasut <marex at denx.de> wrote:
>>
>> Don't see anything in CodingStyle that one should be preferred over
>> the other.
>
> There ain't any I'm aware of, but to paraphrase you, let's keep the format
> that's already used in the driver ;-) :
>
> 114 if (dev->mode & ~(SPI_CPOL | SPI_CPHA))
Thanks for pointing that out, as that code can be deleted. Which
means I don't need to be consistent with it anymore and can create a
new standard, to be followed henceforth, now and forever.
But it looks like the or method is more common in the kernel code. So
I'll change it in V2.
>> > btw. are you using MX23 or MX28 to test this?
>>
>> IMX28.
>
> Is that any mainline board?
Nope, custom hardware.
>> >> Existing code that uses the first/last flags is wrong and needs to be
>> >> changed. Therefor code using 'first' and 'last' will be changed.
>> >>
>> >> Passing the flags as pointers is bad practice and makes no sense to
>> >> do. Does it make any sense to re-write code fix the way it uses first
>> >> and last, then re-write those same lines a second time to not use
>> >> pointers?
>> >
>> > You can always flip it the other way -- first rework the use of flags,
>> > then apply the fix. It'd make the patch much easier to understand, don't
>> > you think?
>>
>> So I should change 'first' to not be a pointer to an integer in one
>> patch, then delete the flag entirely in another patch?
>
> I'd say make a patch (0001) that implements the transition to using your newly
> defined flags and then make a patch (0002) that "Fix extra CS pulses and read
> mode in multi-transfer messages". One patch shall do one thing, that's the usual
> rule of the thumb. Obviously keep them in a series if these patches shall go in
> together. And why doesn't squashing them all together work you might ask --
> because reviewing the result is hard.
5 patches have now been split into 12.
More information about the linux-arm-kernel
mailing list