Hallo, Greg.

>> pp-by: Oleg Verych

> What does "pp-by:" mean?  Please use the proper "Signed-off-by:" line as
> per the Documenataion/SubmittingPatches,

Yes, of course.

> otherwise we can not use this patch.

Fine ! This is my measure to make sure patch will not go further without
testing and acknowledgement by people. As you know sign-off may be added
to this _Patch Proposition_ in any time later. And, as you know, Andrew
Morton wants patches with more quality.

> > +typedef union {
> > +	__le32 a; /* all */
> > +	struct {
> > +		__le32 sz : 16, cs : 8;
> > +	} d;
> > +} ti_firmware_header_t;
> 
> No, do not add typedefs to the kernel, it is not needed.  What was wrong
> with the original structure name?
> 
> Also, please provide better variable names, like the original structure
> had.  "d", "a", "cs", and "sz" are not sufficient.

While i saw some opinions around typedefs, i don't understand why. I've
choose this type of data structure, because of type of manipulation with
it: i use "int" variable to send whole header, thus i need parts of it to
be accessible without bit magic. Name of the structure is descriptive
enough, and i wanted less noise, while doing trivial stuff with its
members.

>> Subject: Re: [patch 03/05] ti_usb, device setup: without any artificial errors, use configuration changing
[]
> Please provide a better description of what the patch does.  The
>  Subject: isn't very descriptive.

As you probably know about my personal disagreement with current
"interface" drivers configuration scheme, this patch is bogus anyway.
Because Al used negative return values to indicate something, what is
*not* error at all!

> > -		status = -ENODEV;
> > +		status = 0x01E; /* positive status -- device to be reconfigured */
> >  		goto free_tdev;
> >  	}
> >  
> > -	/* the second configuration must be set (in sysfs by hotplug script) */
> >  	if (dev->actconfig->desc.bConfigurationValue == TI_BOOT_CONFIG) {
> > -		status = -ENODEV;
> > +		(void) usb_driver_set_configuration(dev, TI_ACTIVE_CONFIG);
> 
> Don't cast function return values.  If the function requires you to
> check the return value, then CHECK IT!!!

And if you will read description of that function, you will know, that it
guarantees nothing. Thus, i do not care what it tries to return in
exchange. Yes, probably better to add more ugly checking to this crutch,
but i don't think it will be helpful %)

> thanks,
> 
> greg k-h

Thank you for taking a look at it, but hardware testing will be more
useful, instead of this bureaucracy (:.

____

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
linux-usb-devel <at> lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel