On Sat, Mar 05, 2022 at 02:23:46AM +0900, Vincent MAILHOL wrote:
> <Removed people in CC>
>
> On Fri. 4 Mar. 2022 at 23:20, Greg Kroah-Hartman
> <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> > On Fri, Mar 04, 2022 at 07:53:50PM +0900, Vincent Mailhol wrote:
> > > Change the prototype of usb_maxpacket() from:
> > > | static inline __u16
> > > | usb_maxpacket(struct usb_device *udev, int pipe, int is_out)
> > >
> > > into:
> > > | static inline u16 usb_maxpacket(struct usb_device *dev, int pipe)
> > >
> > > and rewrite the function.
> > >
> > > Rationale:
> > >
> > > * The third argument of usb_maxpacket(): is_out can be derived from
> > > its second one: pipe using usb_pipeout(pipe). Furthermore,
> > > usb_pipoout(pipe) is being called within usb_maxpacket()
> > > regardless to confirm the input.
> > >
> > > * This function is not exposed to the UAPI so return type should be
> > > u16, not __u16.
> > >
> > > * Let's not reinvent the wheel and rely on usb_endpoint_maxp() to
> > > make this a one liner function.
> > >
> > > All the users of usb_endpoint_maxp() are then updated.
> > >
> > > Two of the users: oxu210hp-hcd.c and isp1760-hcd.c rely on a local
> > > macro: max_packet() to mask the maximum size. Because this masking is
> > > already performed by usb_maxpacket(), this patch also removes these
> > > redundant sanitization and remove the local macro if not needed any
> > > more (keep it in oxu210hp-hcd.c which uses it elsewhere but remove it
> > > in isp1760-hcd.c).
> >
> > This type of "change all callers of this function" is brutal to do like
> > this, as is evident by the number of people you had to cc: here.
> >
> > How about doing it the normal way. Create a new function, with the
> > proper options you wish to see be used, and then move everyone over to
> > it, and when that is done, remove the old function. Bonus points for
> > doing this with some crazy macros to keep the old name in the end (it
> > can be done, but I don't recommend it for the faint-of-heart, so it's
> > not required.)
>
> Thank you for the explanation :)
>
> If I understand correctly, I should:
> * First change linux/usb.h
Yes.
> * Wait for other trees to pull the patch
Pull what?
> * Change the drivers
You can fix up many of these at the same time in a patch series. I can
take many of them in the USB tree as well if needed.
> * Clean up linux/usb.h
>
> I did not find the inspiration for a better name so I would like to
> keep the old one.
> Would the below code meet your expectation of "doing some crazy macros"?
>
> | static inline u16 __usb_maxpacket(struct usb_device *dev, int pipe)
> | {
> | return usb_endpoint_maxp(&usb_pipe_endpoint(dev, pipe)->desc);
> | }
> |
> | #define usb_maxpacket(dev, pipe, deprecated...) __usb_maxpacket(dev, pipe)
Yeah, as you are just dropping the last argument, that might be all that
is needed as this could be quite simpler than I was thinking.
Try it and see!
thanks,
greg k-h
