[OpenBIOS] [PATCH v2] Add USB OHCI + HID driver

[Programmingkid]

On Jun 6, 2014, at 3:34 PM, BALATON Zoltan wrote:

> On Fri, 6 Jun 2014, Programmingkid wrote:
>> On Jun 1, 2014, at 5:55 PM, BALATON Zoltan wrote:
>>> /* Normalize bInterval to log2 of microframes */
>>> +static int
>>> +usb_decode_interval(const int speed, const endpoint_type type, const unsigned char bInterval)
>>> +{
>>> +#define LOG2(a) ((sizeof(unsigned) << 3) - __builtin_clz(a) - 1)
>>> +	switch (speed) {
>>> +	case LOW_SPEED:
>>> +		switch (type) {
>>> +		case ISOCHRONOUS: case INTERRUPT:
>> 
>> Could you place each case on its own separate line please.
>> 
>> 
>>> +		switch (type) {
>>> +		case ISOCHRONOUS: case INTERRUPT:
>>> 
>> 
>>> +	case SUPER_SPEED:
>>> +		switch (type) {
>>> +		case ISOCHRONOUS: case INTERRUPT:
>> 
>> These cases should be on their own separate lines also.
> 
> These are in original code from coreboot. I did not change anything in that apart from those lines that needed porting and fixing for endianness so that it can be diffed to the original and any fixes can be easily taken from coreboot in the future. Reformatting code to match style could be done but it would make it more difficult to keep the two versions in sync. Please decide if you really want that.
> 
> Regards,
> BALATON Zoltan


How it is now does look kind of confusing. In every book of C I have read, case statements that run the same code always look like this:

case 1:
case 2:
	<code>
break;

It is very easy to read and understand. I think we should make our code look better than what Coreboot does and use the standard one case above the other method. Any 'diff' that is done will just show a slightly different case setup. Nothing we can't handle. 

Thank you very much for your improvements to OpenBIOS and QEMU :)