[PATCH v2 1/4] usb: gadget: atmel: Code refactor part 1

[Marek Vasut]

On 7/22/24 10:23 PM, Zixun LI wrote:
> - Sort includes
> - Forward declare controller structures

Please pick some more descriptive subject for this patch than "refactor 
part 1". If this was only sorting includes as a singular patch doing one 
change, the subject could well be "usb: gadget: atmel: Sort the 
includes", but this patch mixes two types of changes, so that makes it 
harder. You can always split the patch in two ...

> Signed-off-by: Zixun LI <zli at ogga.fr>
> ---
>   drivers/usb/gadget/atmel_usba_udc.c | 59 +++++++++++++++--------------
>   1 file changed, 30 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/usb/gadget/atmel_usba_udc.c b/drivers/usb/gadget/atmel_usba_udc.c
> index f99553df8d..476e7ed619 100644
> --- a/drivers/usb/gadget/atmel_usba_udc.c
> +++ b/drivers/usb/gadget/atmel_usba_udc.c
> @@ -7,19 +7,22 @@
>    *			   Bo Shen <voice.shen at atmel.com>
>    */
>   
> -#include <linux/bitops.h>
> -#include <linux/errno.h>
> +#include <clk.h>
> +#include <log.h>

clk.h and log.h was added. It seems this patch does more than sort includes.

> +#include <malloc.h>
>   #include <asm/gpio.h>
>   #include <asm/hardware.h>
> +#include <linux/bitops.h>
> +#include <linux/errno.h>
>   #include <linux/list.h>
> -#include <linux/printk.h>

It seems printk.h is no longer included ?

>   #include <linux/usb/ch9.h>
>   #include <linux/usb/gadget.h>
>   #include <linux/usb/atmel_usba_udc.h>
> -#include <malloc.h>
>   
>   #include "atmel_usba_udc.h"
>   
> +static struct usba_udc *controller;
> +
>   static int vbus_is_present(struct usba_udc *udc)
>   {
>   	/* No Vbus detection: Assume always present */
> @@ -506,12 +509,6 @@ usba_udc_set_selfpowered(struct usb_gadget *gadget, int is_selfpowered)
>   	return 0;
>   }
>   
> -static const struct usb_gadget_ops usba_udc_ops = {
> -	.get_frame		= usba_udc_get_frame,
> -	.wakeup			= usba_udc_wakeup,
> -	.set_selfpowered	= usba_udc_set_selfpowered,
> -};
> -
>   static struct usb_endpoint_descriptor usba_ep0_desc = {
>   	.bLength = USB_DT_ENDPOINT_SIZE,
>   	.bDescriptorType = USB_DT_ENDPOINT,
> @@ -1179,28 +1176,16 @@ static int atmel_usba_stop(struct usba_udc *udc)
>   	return 0;
>   }
>   
> -static struct usba_udc controller = {
> -	.regs = (unsigned *)ATMEL_BASE_UDPHS,
> -	.fifo = (unsigned *)ATMEL_BASE_UDPHS_FIFO,
> -	.gadget = {
> -		.ops		= &usba_udc_ops,
> -		.ep_list	= LIST_HEAD_INIT(controller.gadget.ep_list),
> -		.speed		= USB_SPEED_HIGH,
> -		.is_dualspeed	= 1,
> -		.name		= "atmel_usba_udc",
> -	},
> -};
> -
>   int dm_usb_gadget_handle_interrupts(struct udevice *dev)
>   {
> -	struct usba_udc *udc = &controller;
> +	struct usba_udc *udc = controller;
>   
>   	return usba_udc_irq(udc);
>   }
>   
>   int usb_gadget_register_driver(struct usb_gadget_driver *driver)
>   {
> -	struct usba_udc *udc = &controller;
> +	struct usba_udc *udc = controller;
>   	int ret;
>   
>   	if (!driver || !driver->bind || !driver->setup) {
> @@ -1228,7 +1213,7 @@ int usb_gadget_register_driver(struct usb_gadget_driver *driver)
>   
>   int usb_gadget_unregister_driver(struct usb_gadget_driver *driver)
>   {
> -	struct usba_udc *udc = &controller;
> +	struct usba_udc *udc = controller;
>   
>   	if (!driver || !driver->unbind || !driver->disconnect) {
>   		pr_err("bad paramter\n");
> @@ -1244,6 +1229,24 @@ int usb_gadget_unregister_driver(struct usb_gadget_driver *driver)
>   	return 0;
>   }
>   
> +static const struct usb_gadget_ops usba_udc_ops = {
> +	.get_frame		= usba_udc_get_frame,
> +	.wakeup			= usba_udc_wakeup,
> +	.set_selfpowered	= usba_udc_set_selfpowered,
> +};
> +
> +static struct usba_udc udc_controller = {
> +	.regs = (unsigned int *)ATMEL_BASE_UDPHS,
> +	.fifo = (unsigned int *)ATMEL_BASE_UDPHS_FIFO,
> +	.gadget = {
> +		.ops		= &usba_udc_ops,
> +		.ep_list	= LIST_HEAD_INIT(udc_controller.gadget.ep_list),
> +		.speed		= USB_SPEED_HIGH,
> +		.is_dualspeed	= 1,
> +		.name		= "atmel_usba_udc",
> +	},
> +};
> +
>   static struct usba_ep *usba_udc_pdata(struct usba_platform_data *pdata,
>   				      struct usba_udc *udc)
>   {
> @@ -1286,11 +1289,9 @@ static struct usba_ep *usba_udc_pdata(struct usba_platform_data *pdata,
>   
>   int usba_udc_probe(struct usba_platform_data *pdata)
>   {
> -	struct usba_udc *udc;
> -
> -	udc = &controller;
> +	controller = &udc_controller;
>   
> -	udc->usba_ep = usba_udc_pdata(pdata, udc);
> +	controller->usba_ep = usba_udc_pdata(pdata, controller);
>   
>   	return 0;
>   }

Where is the forward declaration ?