Re: [RFCv4 PATCH 01/13] usb: composite: add make_group and add_function operations
* Andrzej Pietrasiewicz | 2012-11-22 13:06:55 [+0100]:
>Using configfs to create/configure a usb gadget requires
>providing a method to create config group for a usb function
>and a wrapper for usb_add_function.
>
>Signed-off-by: Andrzej Pietrasiewicz
>Signed-off-by: Kyungmin Park
Just a side note: The signed-off here should show the way patch has been
passed. Based on this it looks like you stopped working on it and passed
it over to Kyungmin. That is okay but since you are posting it I guess
it is the other way around.
>diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
>index e84c754..5bac1f8 100644
>--- a/include/linux/usb/composite.h
>+++ b/include/linux/usb/composite.h
>@@ -39,6 +39,7 @@
> #include
> #include
> #include
>+#include
>
> /*
> * USB function drivers should return USB_GADGET_DELAYED_STATUS if they
>@@ -146,6 +147,11 @@ struct usb_function {
>* we can't restructure things to avoid mismatching.
>* Related: unbind() may kfree() but bind() won't...
>*/
>+
>+ struct config_group *(*make_group)(struct config_group *parent,
>+ const char *name);
>+ int (*add_function)(struct usb_configuration *c, struct usb_function *f,
>+ struct config_item *item, void *data);
a wrapper? I though you pass the configfs root node and the function
will add all files / subdirectories. Why do we have two callbacks?
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFCv4 PATCH 01/13] usb: composite: add make_group and add_function operations
On Thursday, November 22, 2012 3:55 PM Sebastian Andrzej Siewior wrote: > * Andrzej Pietrasiewicz | 2012-11-22 13:06:55 [+0100]: > > >Using configfs to create/configure a usb gadget requires > >providing a method to create config group for a usb function > >and a wrapper for usb_add_function. > > > >Signed-off-by: Andrzej Pietrasiewicz > >Signed-off-by: Kyungmin Park > > Just a side note: The signed-off here should show the way patch has been > passed. Based on this it looks like you stopped working on it and passed > it over to Kyungmin. That is okay but since you are posting it I guess > it is the other way around. > I guess it should be: Reviewed-by: Kyungmin Park -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFCv4 PATCH 01/13] usb: composite: add make_group and add_function operations
> * Andrzej Pietrasiewicz | 2012-11-22 13:06:55 [+0100]: >> Using configfs to create/configure a usb gadget requires >> providing a method to create config group for a usb function >> and a wrapper for usb_add_function. >> >> Signed-off-by: Andrzej Pietrasiewicz >> Signed-off-by: Kyungmin Park On Thursday, November 22, 2012 3:55 PM Sebastian Andrzej Siewior wrote: > Just a side note: The signed-off here should show the way patch has been > passed. Based on this it looks like you stopped working on it and passed > it over to Kyungmin. That is okay but since you are posting it I guess > it is the other way around. On Thu, Nov 22 2012, Andrzej Pietrasiewicz wrote: > I guess it should be: > > Reviewed-by: Kyungmin Park I think neither is correct. The reviewed-by tag implies that the person did a careful review of the code as per “Reviewer's statement of oversight” (see Documentation/SubmittingPatches). What actually happens is Kyungmin giving a green light to shipping the patch from copyright stand-point since Samsung is copyright holder and Andrzej has no power to say weather he can or cannot release the code. So logical path the code took was: Andrzej -> Kyungmin -> Andrzej -> linux-usb If you look at other patches coming from SPRC (including mine while I was working for Samsung) they all have the same Signed-off schema where the first line is of the author and second is of Kyungmin. -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz(o o) ooo +--ooO--(_)--Ooo-- pgpcsjcCN4M39.pgp Description: PGP signature
RE: [RFCv4 PATCH 01/13] usb: composite: add make_group and add_function operations
On Thursday, November 22, 2012 3:55 PM Sebastian Andrzej Siewior wrote:
> * Andrzej Pietrasiewicz | 2012-11-22 13:06:55 [+0100]:
>
> >diff --git a/include/linux/usb/composite.h
> b/include/linux/usb/composite.h
> >index e84c754..5bac1f8 100644
> >--- a/include/linux/usb/composite.h
> >+++ b/include/linux/usb/composite.h
> >@@ -39,6 +39,7 @@
> > #include
> > #include
> > #include
> >+#include
> >
> > /*
> > * USB function drivers should return USB_GADGET_DELAYED_STATUS if they
> >@@ -146,6 +147,11 @@ struct usb_function {
> > * we can't restructure things to avoid mismatching.
> > * Related: unbind() may kfree() but bind() won't...
> > */
> >+
> >+struct config_group *(*make_group)(struct config_group *parent,
> >+ const char *name);
> >+int (*add_function)(struct usb_configuration *c, struct usb_function
> *f,
> >+struct config_item *item, void *data);
>
> a wrapper? I though you pass the configfs root node and the function
> will add all files / subdirectories. Why do we have two callbacks?
>
make_group explanation:
In configfs the directory needs to know how to create its subdirectories.
In order to do it there are make_group and make_item methods in
struct configfs_group_operations (make_group and make_item are mutually
exclusive, i.e. only one of them can be specified). In the following
UFG implementation the function-dependent directories subtree is
rooted at some configfs directory which is of a generic type. So there
needs to be a way to pass this function-specific method to the generic
code. make_group here serves this purpose. It is used during gadget
setup, that is, when the configfs directories are created but prior
to actual binding of the gadget. Since creating the directories
does not equal gadget binding, it needs to be a separate function.
add_function explanation:
In order to use your framework for registering the usb functions,
the usb_get_function should be called from the generic code (otherwise
it leads to a situation where one needs to have a function's module
loaded in order to request this module... not good :O ). Once the
generic code has the function, it can call usb_add_funciton.
But some functions (as is the case with mass storage)
seem to need some additional function-specific activities together
with usb_add_function. So I decided to collect them in one
add_function operation with the semantics: do usb_add_function
plus any necessary function-specific things related to adding
this function.
Andrzej
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv4 PATCH 01/13] usb: composite: add make_group and add_function operations
On 11/22/2012 09:48 PM, Michal Nazarewicz wrote: I think neither is correct. The reviewed-by tag implies that the person did a careful review of the code as per “Reviewer's statement of oversight” (see Documentation/SubmittingPatches). What actually happens is Kyungmin giving a green light to shipping the patch from copyright stand-point since Samsung is copyright holder and Andrzej has no power to say weather he can or cannot release the code. So logical path the code took was: Andrzej -> Kyungmin -> Andrzej -> linux-usb Aha. So is Kyungmin a lawyer and not a hacker as I assumed in the first place. If you look at other patches coming from SPRC (including mine while I was working for Samsung) they all have the same Signed-off schema where the first line is of the author and second is of Kyungmin. This together with the statement above explains a lot to me. I always saw that and wondered how much code he can write. I assumed that Kyungmin was some kind of kick-ass hacker that knows all the chips very well and therefore writes all of the Samsung code ahead of HW and then is too busy with other stuff and so other people in his team push his patches mainline and deal with the review. I know that other companies work like that, where a small group of people does the bring-up and then others take their code and try to merge upstream. And this impressed me because Kyungmin is a person and not a small group. Anyway. Signed-off indicates that he was involved in code development but he was not. As it seems it me, his OKAY is very important why not add him as Acked-By: ... [copyright] I added the [copyright] as the subsystem since he did Ack only a part of the patch, not the functionality etc. I know that (now) but others might not. Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFCv4 PATCH 01/13] usb: composite: add make_group and add_function operations
Hi, > -Original Message- > From: Sebastian Andrzej Siewior [mailto:bige...@linutronix.de] > Sent: Friday, November 23, 2012 7:17 PM > To: Michal Nazarewicz > Cc: Andrzej Pietrasiewicz; linux-usb@vger.kernel.org; 'Kyungmin Park'; > 'Felipe Balbi'; 'Greg Kroah-Hartman'; 'Joel Becker'; Marek Szyprowski > Subject: Re: [RFCv4 PATCH 01/13] usb: composite: add make_group and > add_function operations > > On 11/22/2012 09:48 PM, Michal Nazarewicz wrote: > > I think neither is correct. The reviewed-by tag implies that the > > person did a careful review of the code as per “Reviewer's statement > > of oversight” (see Documentation/SubmittingPatches). > > > > What actually happens is Kyungmin giving a green light to shipping the > > patch from copyright stand-point since Samsung is copyright holder and > > Andrzej has no power to say weather he can or cannot release the code. > > > > So logical path the code took was: > > > > Andrzej -> Kyungmin -> Andrzej -> linux-usb > > Aha. So is Kyungmin a lawyer and not a hacker as I assumed in the first > place. > > > If you look at other patches coming from SPRC (including mine while I > > was working for Samsung) they all have the same Signed-off schema > > where the first line is of the author and second is of Kyungmin. > > This together with the statement above explains a lot to me. I always saw > that and wondered how much code he can write. I assumed that Kyungmin was > some kind of kick-ass hacker that knows all the chips very well and > therefore writes all of the Samsung code ahead of HW and then is too busy > with other stuff and so other people in his team push his patches mainline > and deal with the review. > I know that other companies work like that, where a small group of people > does the bring-up and then others take their code and try to merge > upstream. And this impressed me because Kyungmin is a person and not a > small group. > > Anyway. > Signed-off indicates that he was involved in code development but he was > not. As it seems it me, his OKAY is very important why not add him as > > Acked-By: ... [copyright] > > I added the [copyright] as the subsystem since he did Ack only a part of > the patch, not the functionality etc. I know that (now) but others might > not. Even though all codes are not tested at internal tree, but most codes are tested internal tree. And these internal tree is managed by me. That's reason to add Signed-off as internal tree maintainer. And most of codes from us, I checked it by internal approval process. If you don't feel it's not correct Signed-off scheme. No problem to replace it with Reviewed-by or Acked-by. Thank you, Kyungmin Park > > Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFCv4 PATCH 01/13] usb: composite: add make_group and add_function operations
One more, I checked it again "Documentation/SubmittingPatches" In the 12) Sign your work You can find the following paragraphs. "Some people also put extra tags at the end. They'll just be ignored for now, but you can do this to mark internal company procedures or just point out some special detail about the sign-off." And 13) When to use Acked-by: and Cc: "The Signed-off-by: tag indicates that the signer was involved in the development of the patch, or that he/she was in the patch's delivery path." I used Signed-off as our contribution to open source from our teams. Of course some codes are written by me with correct Signed-off Thank you, Kyungmin Park > -Original Message- > From: Kyungmin Park [mailto:kyungmin.p...@samsung.com] > Sent: Friday, November 23, 2012 8:05 PM > To: 'Sebastian Andrzej Siewior'; 'Michal Nazarewicz' > Cc: 'Andrzej Pietrasiewicz'; 'linux-usb@vger.kernel.org'; 'Felipe Balbi'; > 'Greg Kroah-Hartman'; 'Joel Becker'; 'Marek Szyprowski' > Subject: RE: [RFCv4 PATCH 01/13] usb: composite: add make_group and > add_function operations > > Hi, > > > -Original Message- > > From: Sebastian Andrzej Siewior [mailto:bige...@linutronix.de] > > Sent: Friday, November 23, 2012 7:17 PM > > To: Michal Nazarewicz > > Cc: Andrzej Pietrasiewicz; linux-usb@vger.kernel.org; 'Kyungmin Park'; > > 'Felipe Balbi'; 'Greg Kroah-Hartman'; 'Joel Becker'; Marek Szyprowski > > Subject: Re: [RFCv4 PATCH 01/13] usb: composite: add make_group and > > add_function operations > > > > On 11/22/2012 09:48 PM, Michal Nazarewicz wrote: > > > I think neither is correct. The reviewed-by tag implies that the > > > person did a careful review of the code as per “Reviewer's statement > > > of oversight” (see Documentation/SubmittingPatches). > > > > > > What actually happens is Kyungmin giving a green light to shipping the > > > patch from copyright stand-point since Samsung is copyright holder and > > > Andrzej has no power to say weather he can or cannot release the code. > > > > > > So logical path the code took was: > > > > > > Andrzej -> Kyungmin -> Andrzej -> linux-usb > > > > Aha. So is Kyungmin a lawyer and not a hacker as I assumed in the first > > place. > > > > > If you look at other patches coming from SPRC (including mine while I > > > was working for Samsung) they all have the same Signed-off schema > > > where the first line is of the author and second is of Kyungmin. > > > > This together with the statement above explains a lot to me. I always > saw > > that and wondered how much code he can write. I assumed that Kyungmin > was > > some kind of kick-ass hacker that knows all the chips very well and > > therefore writes all of the Samsung code ahead of HW and then is too > busy > > with other stuff and so other people in his team push his patches > mainline > > and deal with the review. > > I know that other companies work like that, where a small group of > people > > does the bring-up and then others take their code and try to merge > > upstream. And this impressed me because Kyungmin is a person and not a > > small group. > > > > Anyway. > > Signed-off indicates that he was involved in code development but he was > > not. As it seems it me, his OKAY is very important why not add him as > > > > Acked-By: ... [copyright] > > > > I added the [copyright] as the subsystem since he did Ack only a part of > > the patch, not the functionality etc. I know that (now) but others might > > not. > > Even though all codes are not tested at internal tree, but most codes are > tested internal tree. And these internal tree is managed by me. > That's reason to add Signed-off as internal tree maintainer. > And most of codes from us, I checked it by internal approval process. If > you don't feel it's not correct Signed-off scheme. > No problem to replace it with Reviewed-by or Acked-by. > > Thank you, > Kyungmin Park > > > > > Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv4 PATCH 01/13] usb: composite: add make_group and add_function operations
On Fri, Nov 23, 2012 at 08:05:29PM +0900, Kyungmin Park wrote: > Hi, > > > -Original Message- > > From: Sebastian Andrzej Siewior [mailto:bige...@linutronix.de] > > Sent: Friday, November 23, 2012 7:17 PM > > To: Michal Nazarewicz > > Cc: Andrzej Pietrasiewicz; linux-usb@vger.kernel.org; 'Kyungmin Park'; > > 'Felipe Balbi'; 'Greg Kroah-Hartman'; 'Joel Becker'; Marek Szyprowski > > Subject: Re: [RFCv4 PATCH 01/13] usb: composite: add make_group and > > add_function operations > > > > On 11/22/2012 09:48 PM, Michal Nazarewicz wrote: > > > I think neither is correct. The reviewed-by tag implies that the > > > person did a careful review of the code as per “Reviewer's statement > > > of oversight” (see Documentation/SubmittingPatches). > > > > > > What actually happens is Kyungmin giving a green light to shipping the > > > patch from copyright stand-point since Samsung is copyright holder and > > > Andrzej has no power to say weather he can or cannot release the code. > > > > > > So logical path the code took was: > > > > > > Andrzej -> Kyungmin -> Andrzej -> linux-usb > > > > Aha. So is Kyungmin a lawyer and not a hacker as I assumed in the first > > place. > > > > > If you look at other patches coming from SPRC (including mine while I > > > was working for Samsung) they all have the same Signed-off schema > > > where the first line is of the author and second is of Kyungmin. > > > > This together with the statement above explains a lot to me. I always saw > > that and wondered how much code he can write. I assumed that Kyungmin was > > some kind of kick-ass hacker that knows all the chips very well and > > therefore writes all of the Samsung code ahead of HW and then is too busy > > with other stuff and so other people in his team push his patches mainline > > and deal with the review. > > I know that other companies work like that, where a small group of people > > does the bring-up and then others take their code and try to merge > > upstream. And this impressed me because Kyungmin is a person and not a > > small group. > > > > Anyway. > > Signed-off indicates that he was involved in code development but he was > > not. As it seems it me, his OKAY is very important why not add him as > > > > Acked-By: ... [copyright] > > > > I added the [copyright] as the subsystem since he did Ack only a part of > > the patch, not the functionality etc. I know that (now) but others might > > not. > > Even though all codes are not tested at internal tree, but most codes are > tested internal tree. And these internal tree is managed by me. > That's reason to add Signed-off as internal tree maintainer. > And most of codes from us, I checked it by internal approval process. If you > don't feel it's not correct Signed-off scheme. > No problem to replace it with Reviewed-by or Acked-by. No, it is correct that you used Signed-off-by: here. I know lots of managers that have added Signed-off-by: lines to patches, it is not necessary to be a developer of the patch to do this, Signed-off-by: means what it says in the Documentation/SubmittingPatches file. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
