Re: [PATCH] usb: rework usb_maxpacket() and remove its third argument

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.)

thanks,

greg k-h



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux