linux-kernel - Re: [PATCH] west bridge, cyasgadget fixes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4C86A1D3.9060703@oracle.com>
Date:	Tue, 07 Sep 2010 13:34:27 -0700
From:	Randy Dunlap <randy.dunlap@...cle.com>
To:	david.cross@...ress.com
CC:	greg@...ah.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] west bridge, cyasgadget fixes

On 09/07/10 13:28, David Cross wrote:
> On Tue, 2010-09-07 at 12:48 -0700, Randy Dunlap wrote:
>> On Tue, 07 Sep 2010 12:44:07 -0700 David Cross wrote:
>>
>>> This patch contains update to the gadget driver, some of which are based
>>> on feedback from the Linux community concerning the usage of fat_get_block.
>>> This function call has since been removed along with all file system 
>>> specific calls.
>>> It also contains some fixes for locking issues.
>>> Please let me know if there are issues or concerns with this patch.
>>> Thanks,
>>> David
>>>
>>> Signed-off-by: David Cross <david.cross@...ress.com>
>>
>>
>> Is there a patch or patch series description where the west bridge is described?
> The initial patch had a description of the device itself. I am repeating
> it here for convenience:


{thanks}


> 
>> I'm asking mostly because some of the code (in the block driver patch)
>> looks like it should be in userspace, so I'm wondering why it's in the kernel tree?
> 
> Can you please be more specific concerning what you think should be in
> userspace?

I'll try to find time to review the patches, but reviewing such large patches is
going to be difficult for most people, I think.

>>
>>> diff -uprN -X linux-next-vanilla/Documentation/dontdiff linux-next-vanilla/drivers/staging/westbridge/astoria/gadget/cyasgadget.c linux-next-incl-sdk/drivers/staging/westbridge/astoria/gadget/cyasgadget.c
>>> --- linux-next-vanilla/drivers/staging/westbridge/astoria/gadget/cyasgadget.c	2010-08-31 19:32:51.000000000 -0700
>>> +++ linux-next-incl-sdk/drivers/staging/westbridge/astoria/gadget/cyasgadget.c	2010-09-07 11:12:44.000000000 -0700
>>> @@ -154,7 +154,7 @@ static void cy_as_gadget_usb_event_callb
>>>  		#ifndef WESTBRIDGE_NDEBUG
>>>  		ctrlreq = (struct usb_ctrlrequest *)evdata;
>>>  
>>> -		cy_as_hal_print_message("<1>_cy_as_event_usb_setup_packet "
>>> +		printk(KERN_INFO "<1>_cy_as_event_usb_setup_packet "
>>
>> Drop the "<1>" ??
>>
>> But lots of these are purely debug messages anyway, and shouldn't be printed
>> with printk(KERN_INFO ...)
>>
> You are probably right on both counts, I can change this for the next patch.
> The current version in linux-next has both of these issues already
> though, so I am not sure if it makes sense to redo this patch just for
> these items.

That depends on whether you ever want to get the driver out of the staging/ area....
Anyway, there are plenty of other reasons to send more patches, if we can get more
reviews of the current patches done.

>>>  							"received"
>>>  							"bRequestType=0x%x,"
>>>  							"bRequest=0x%x,"
>>


-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/