| Message ID | 20230608195631.55364-3-CFSworks@gmail.com |
|---|---|
| State | Deferred |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | sunxi, usb: UDC/DM gadget model support | expand |
On Thu, 8 Jun 2023 13:56:31 -0600 Sam Edwards <cfsworks@gmail.com> wrote: Hi, John asked me have a look at this. > Since many sunxi boards do not implement a `board_usb_init`, it's I am confused, what has this have to do with gadget support? *No* sunxi board build provides board_usb_init(), but apparently this works fine for now. I am all for this converting to DM part, but the rationale seems a bit off. Also can you give some reason for this patch? What does this fix or improve? "it's better" is a bit thin, "complying with DM" would already be sufficient, but maybe there is more? > better if we just make the sunxi USB driver compatible with the > DM gadget model, as many other musb-new variants already are. > > This change has been verified working on a T113s. > > Signed-off-by: Sam Edwards <CFSworks@gmail.com> > --- > drivers/usb/musb-new/sunxi.c | 50 +++++++++++++++++++++++------------- > 1 file changed, 32 insertions(+), 18 deletions(-) > > diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c > index 510b254f7d..6658cd995d 100644 > --- a/drivers/usb/musb-new/sunxi.c > +++ b/drivers/usb/musb-new/sunxi.c > @@ -444,6 +444,16 @@ static struct musb_hdrc_config musb_config_h3 = { > .ram_bits = SUNXI_MUSB_RAM_BITS, > }; > > +#if CONFIG_IS_ENABLED(DM_USB_GADGET) Please no more #ifdef's. Is there any reason to *not* force DM_USB_GADGET now, for all sunxi boards, in Kconfig? Either by "select"ing it in the USB Kconfig, or in arch/arm/Kconfig, like other platforms do. Then you don't need to care about the !DM_USB_GADGET definition of this function and can drop the #ifdef. > +int dm_usb_gadget_handle_interrupts(struct udevice *dev) { coding style > + struct sunxi_glue *glue = dev_get_priv(dev); > + struct musb_host_data *host = &glue->mdata; > + > + host->host->isr(0, host->host); > + return 0; > +} > +#endif > + > static int musb_usb_probe(struct udevice *dev) > { > struct sunxi_glue *glue = dev_get_priv(dev); > @@ -452,10 +462,6 @@ static int musb_usb_probe(struct udevice *dev) > void *base = dev_read_addr_ptr(dev); > int ret; > > -#ifdef CONFIG_USB_MUSB_HOST > - struct usb_bus_priv *priv = dev_get_uclass_priv(dev); > -#endif > - > if (!base) > return -EINVAL; > > @@ -486,23 +492,31 @@ static int musb_usb_probe(struct udevice *dev) > pdata.platform_ops = &sunxi_musb_ops; > pdata.config = glue->cfg->config; > > -#ifdef CONFIG_USB_MUSB_HOST > - priv->desc_before_addr = true; > + if (IS_ENABLED(CONFIG_USB_MUSB_HOST)) { > + struct usb_bus_priv *priv = dev_get_uclass_priv(dev); > + priv->desc_before_addr = true; > > - pdata.mode = MUSB_HOST; > - host->host = musb_init_controller(&pdata, &glue->dev, base); > - if (!host->host) > - return -EIO; > + pdata.mode = MUSB_HOST; > + host->host = musb_init_controller(&pdata, &glue->dev, base); > + if (!host->host) > + return -EIO; > > - return musb_lowlevel_init(host); > -#else > - pdata.mode = MUSB_PERIPHERAL; > - host->host = musb_register(&pdata, &glue->dev, base); > - if (IS_ERR_OR_NULL(host->host)) > - return -EIO; > + return musb_lowlevel_init(host); > + } else if (CONFIG_IS_ENABLED(DM_USB_GADGET)) { > + pdata.mode = MUSB_PERIPHERAL; > + host->host = musb_init_controller(&pdata, &glue->dev, base); > + if (!host->host) > + return -EIO; > > - return 0; > -#endif > + return usb_add_gadget_udc(&glue->dev, &host->host->g); > + } else { > + pdata.mode = MUSB_PERIPHERAL; > + host->host = musb_register(&pdata, &glue->dev, base); > + if (IS_ERR_OR_NULL(host->host)) > + return -EIO; > + > + return 0; > + } That looks like a good cleanup! Just need to test it briefly, but it seems like the gist of this patch is fine. Cheers, Andre > } > > static int musb_usb_remove(struct udevice *dev)
On 6/27/24 09:06, Andre Przywara wrote: > On Thu, 8 Jun 2023 13:56:31 -0600 > Sam Edwards <cfsworks@gmail.com> wrote: > > Hi, > > John asked me have a look at this. Hi Andre, it's good to hear from you again, I'd first like to make sure you're aware that the date on this patch is June *2023,* not June 2024. It's possible things have changed substantially in the past year. I do not know if this patch is still a necessity; though if John is nudging about it, it probably is. > >> Since many sunxi boards do not implement a `board_usb_init`, it's > > I am confused, what has this have to do with gadget support? *No* sunxi > board build provides board_usb_init(), but apparently this works fine for > now. > I am all for this converting to DM part, but the rationale seems a bit > off. For context, board_usb_init() is (was?) the non-DM entry point for USB functionality; it is (was?) *the* implementation of usb_gadget_initialize() when !DM_USB_GADGET. > > Also can you give some reason for this patch? What does this fix or > improve? "it's better" is a bit thin, "complying with DM" would already be > sufficient, but maybe there is more? Eh, yeah, "better" is something of a question-begging word isn't it? :) The main point is to be compatible with DM's view of UDC, which as you said is a worthy goal in itself. It's "better" because this allows using DM's all-purpose implementation of usb_gadget_initialize(), which is (was?) necessary for those targets lacking board_usb_init(). > >> better if we just make the sunxi USB driver compatible with the >> DM gadget model, as many other musb-new variants already are. >> >> This change has been verified working on a T113s. >> >> Signed-off-by: Sam Edwards <CFSworks@gmail.com> >> --- >> drivers/usb/musb-new/sunxi.c | 50 +++++++++++++++++++++++------------- >> 1 file changed, 32 insertions(+), 18 deletions(-) >> >> diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c >> index 510b254f7d..6658cd995d 100644 >> --- a/drivers/usb/musb-new/sunxi.c >> +++ b/drivers/usb/musb-new/sunxi.c >> @@ -444,6 +444,16 @@ static struct musb_hdrc_config musb_config_h3 = { >> .ram_bits = SUNXI_MUSB_RAM_BITS, >> }; >> >> +#if CONFIG_IS_ENABLED(DM_USB_GADGET) > > Please no more #ifdef's. Is there any reason to *not* force > DM_USB_GADGET now, for all sunxi boards, in Kconfig? > Either by "select"ing it in the USB Kconfig, or in arch/arm/Kconfig, like > other platforms do. > Then you don't need to care about the !DM_USB_GADGET definition of this > function and can drop the #ifdef. I wouldn't be the one to ask. I can't think of any such reason myself. But to me it sounds like since *no sunxi board provides board_usb_init()* the only way USB gadgets *could* work is with DM_USB_GADGET? That'd be reason enough to force it. > >> +int dm_usb_gadget_handle_interrupts(struct udevice *dev) { > > coding style Sentence fragments are harder to understand. I am assuming you are saying, "Please put the opening '{' on its own line." > >> + struct sunxi_glue *glue = dev_get_priv(dev); >> + struct musb_host_data *host = &glue->mdata; >> + >> + host->host->isr(0, host->host); >> + return 0; >> +} >> +#endif >> + >> static int musb_usb_probe(struct udevice *dev) >> { >> struct sunxi_glue *glue = dev_get_priv(dev); >> @@ -452,10 +462,6 @@ static int musb_usb_probe(struct udevice *dev) >> void *base = dev_read_addr_ptr(dev); >> int ret; >> >> -#ifdef CONFIG_USB_MUSB_HOST >> - struct usb_bus_priv *priv = dev_get_uclass_priv(dev); >> -#endif >> - >> if (!base) >> return -EINVAL; >> >> @@ -486,23 +492,31 @@ static int musb_usb_probe(struct udevice *dev) >> pdata.platform_ops = &sunxi_musb_ops; >> pdata.config = glue->cfg->config; >> >> -#ifdef CONFIG_USB_MUSB_HOST >> - priv->desc_before_addr = true; >> + if (IS_ENABLED(CONFIG_USB_MUSB_HOST)) { >> + struct usb_bus_priv *priv = dev_get_uclass_priv(dev); >> + priv->desc_before_addr = true; >> >> - pdata.mode = MUSB_HOST; >> - host->host = musb_init_controller(&pdata, &glue->dev, base); >> - if (!host->host) >> - return -EIO; >> + pdata.mode = MUSB_HOST; >> + host->host = musb_init_controller(&pdata, &glue->dev, base); >> + if (!host->host) >> + return -EIO; >> >> - return musb_lowlevel_init(host); >> -#else >> - pdata.mode = MUSB_PERIPHERAL; >> - host->host = musb_register(&pdata, &glue->dev, base); >> - if (IS_ERR_OR_NULL(host->host)) >> - return -EIO; >> + return musb_lowlevel_init(host); >> + } else if (CONFIG_IS_ENABLED(DM_USB_GADGET)) { >> + pdata.mode = MUSB_PERIPHERAL; >> + host->host = musb_init_controller(&pdata, &glue->dev, base); >> + if (!host->host) >> + return -EIO; >> >> - return 0; >> -#endif >> + return usb_add_gadget_udc(&glue->dev, &host->host->g); >> + } else { >> + pdata.mode = MUSB_PERIPHERAL; >> + host->host = musb_register(&pdata, &glue->dev, base); >> + if (IS_ERR_OR_NULL(host->host)) >> + return -EIO; >> + >> + return 0; >> + } > > That looks like a good cleanup! Just need to test it briefly, but it seems > like the gist of this patch is fine. I think it would be wise to test it a little better than "briefly" given the age of the patch. I'm not well-equipped to do any testing myself right now or I'd volunteer. > > Cheers, > Andre Likewise, Sam > > >> } >> >> static int musb_usb_remove(struct udevice *dev) >
On Thu, 27 Jun 2024 17:25:57 -0600 Sam Edwards <cfsworks@gmail.com> wrote: Hi Sam, thanks for coming back so quickly! > On 6/27/24 09:06, Andre Przywara wrote: > > On Thu, 8 Jun 2023 13:56:31 -0600 > > Sam Edwards <cfsworks@gmail.com> wrote: > > > > Hi, > > > > John asked me have a look at this. > > Hi Andre, it's good to hear from you again, > > I'd first like to make sure you're aware that the date on this patch is > June *2023,* not June 2024. It's possible things have changed > substantially in the past year. Not in the Allwinner MUSB driver, but indeed the gadget support in U-Boot has changed since then. > I do not know if this patch is still a > necessity; though if John is nudging about it, it probably is. Yes apparently he needs it, though I am not entirely sure why. USB gadget has worked for ages in sunxi, without DM_USB_GADGET support, that's why I was a bit puzzled why this patch seems so important. And secondly I was put off by John's initial reply that it would trigger many USB errors for him. He later rectified it, but I must have missed that message. > >> Since many sunxi boards do not implement a `board_usb_init`, it's > > > > I am confused, what has this have to do with gadget support? *No* sunxi > > board build provides board_usb_init(), but apparently this works fine for > > now. > > I am all for this converting to DM part, but the rationale seems a bit > > off. > > For context, board_usb_init() is (was?) the non-DM entry point for USB > functionality; I think board_usb_init() comes from the old days, to provide hooks in case certain boards need something special to enable USB, like an extra regulator, power domain or clock to toggle. These days this should all be covered by DT properties, so we should not (and do not!) need it. And there is an empty fallback implementation in common/usb.c. > it is (was?) *the* implementation of > usb_gadget_initialize() when !DM_USB_GADGET. Mmh, was that so? None of them seemed to call usb_gadget_initialize(), though? I checked older releases, but usb_gadget_initialize() seemed to have always been called by the respective gadget code (fastboot, ums, ethernet)? > > Also can you give some reason for this patch? What does this fix or > > improve? "it's better" is a bit thin, "complying with DM" would already be > > sufficient, but maybe there is more? > > Eh, yeah, "better" is something of a question-begging word isn't it? :) > > The main point is to be compatible with DM's view of UDC, which as you > said is a worthy goal in itself. It's "better" because this allows using > DM's all-purpose implementation of usb_gadget_initialize(), which is > (was?) necessary for those targets lacking board_usb_init(). That's what triggered my confusion: there is no sunxi board as such that implements board_usb_init(), and with our USB PHY code being DT driven, we should not need it anyway. There are empty fallback implementations in common/usb.c and drivers/usb/gadget/g_dnl.c, and we pick up one of them, given the right Kconfig symbols defined. I just tried it: enabling the "ums" command, and it worked: $ make orangepi_zero3_defconfig $ scripts/config --enable CONFIG_CMD_USB_MASS_STORAGE $ make olddefconfig $ make then, on the U-Boot prompt: => ums 0 mmc 0 and the SD card block device popped up on my host. (That the ums command is useful enough to be enabled by default is a separate story). Gadget Ethernet and fastboot are even enabled by default, so whenever you have a defconfig with CONFIG_USB_MUSB_GADGET=y, you should get this functionality. So can anyone tell why this patch is needed so desperately? > >> better if we just make the sunxi USB driver compatible with the > >> DM gadget model, as many other musb-new variants already are. > >> > >> This change has been verified working on a T113s. > >> > >> Signed-off-by: Sam Edwards <CFSworks@gmail.com> > >> --- > >> drivers/usb/musb-new/sunxi.c | 50 +++++++++++++++++++++++------------- > >> 1 file changed, 32 insertions(+), 18 deletions(-) > >> > >> diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c > >> index 510b254f7d..6658cd995d 100644 > >> --- a/drivers/usb/musb-new/sunxi.c > >> +++ b/drivers/usb/musb-new/sunxi.c > >> @@ -444,6 +444,16 @@ static struct musb_hdrc_config musb_config_h3 = { > >> .ram_bits = SUNXI_MUSB_RAM_BITS, > >> }; > >> > >> +#if CONFIG_IS_ENABLED(DM_USB_GADGET) > > > > Please no more #ifdef's. Is there any reason to *not* force > > DM_USB_GADGET now, for all sunxi boards, in Kconfig? > > Either by "select"ing it in the USB Kconfig, or in arch/arm/Kconfig, like > > other platforms do. > > Then you don't need to care about the !DM_USB_GADGET definition of this > > function and can drop the #ifdef. > > I wouldn't be the one to ask. I can't think of any such reason myself. > But to me it sounds like since *no sunxi board provides > board_usb_init()* the only way USB gadgets *could* work is with > DM_USB_GADGET? That'd be reason enough to force it. See above. So I think the only reason to *not* have DM_USB_GADGET in general is because some driver is not ready for that yet, or there is some odd device that requires some extra hacks that the DM implementation does not cover. So with this patch we remove those reasons for sunxi, AFAICT. The Allwinner MUSB driver is now DM ready, so there is no reason to not set it. Think of it more as we are now declaring the sunxi MUSB driver as DM ready. That's why I want to see it set unconditionally, as it's not a user choice per se. Eventually this DM specific symbol will go away at some point, as it did for other subsystems like Ethernet already. > >> +int dm_usb_gadget_handle_interrupts(struct udevice *dev) { > > > > coding style > > Sentence fragments are harder to understand. I am assuming you are > saying, "Please put the opening '{' on its own line." Funny enough I had these extra words, but then deleted them in fear it would sound too patronising for this obvious typo ;-) > >> + struct sunxi_glue *glue = dev_get_priv(dev); > >> + struct musb_host_data *host = &glue->mdata; > >> + > >> + host->host->isr(0, host->host); > >> + return 0; > >> +} > >> +#endif > >> + > >> static int musb_usb_probe(struct udevice *dev) > >> { > >> struct sunxi_glue *glue = dev_get_priv(dev); > >> @@ -452,10 +462,6 @@ static int musb_usb_probe(struct udevice *dev) > >> void *base = dev_read_addr_ptr(dev); > >> int ret; > >> > >> -#ifdef CONFIG_USB_MUSB_HOST > >> - struct usb_bus_priv *priv = dev_get_uclass_priv(dev); > >> -#endif > >> - > >> if (!base) > >> return -EINVAL; > >> > >> @@ -486,23 +492,31 @@ static int musb_usb_probe(struct udevice *dev) > >> pdata.platform_ops = &sunxi_musb_ops; > >> pdata.config = glue->cfg->config; > >> > >> -#ifdef CONFIG_USB_MUSB_HOST > >> - priv->desc_before_addr = true; > >> + if (IS_ENABLED(CONFIG_USB_MUSB_HOST)) { > >> + struct usb_bus_priv *priv = dev_get_uclass_priv(dev); > >> + priv->desc_before_addr = true; > >> > >> - pdata.mode = MUSB_HOST; > >> - host->host = musb_init_controller(&pdata, &glue->dev, base); > >> - if (!host->host) > >> - return -EIO; > >> + pdata.mode = MUSB_HOST; > >> + host->host = musb_init_controller(&pdata, &glue->dev, base); > >> + if (!host->host) > >> + return -EIO; > >> > >> - return musb_lowlevel_init(host); > >> -#else > >> - pdata.mode = MUSB_PERIPHERAL; > >> - host->host = musb_register(&pdata, &glue->dev, base); > >> - if (IS_ERR_OR_NULL(host->host)) > >> - return -EIO; > >> + return musb_lowlevel_init(host); > >> + } else if (CONFIG_IS_ENABLED(DM_USB_GADGET)) { > >> + pdata.mode = MUSB_PERIPHERAL; > >> + host->host = musb_init_controller(&pdata, &glue->dev, base); > >> + if (!host->host) > >> + return -EIO; > >> > >> - return 0; > >> -#endif > >> + return usb_add_gadget_udc(&glue->dev, &host->host->g); > >> + } else { > >> + pdata.mode = MUSB_PERIPHERAL; > >> + host->host = musb_register(&pdata, &glue->dev, base); > >> + if (IS_ERR_OR_NULL(host->host)) > >> + return -EIO; > >> + > >> + return 0; > >> + } > > > > That looks like a good cleanup! Just need to test it briefly, but it seems > > like the gist of this patch is fine. > > I think it would be wise to test it a little better than "briefly" given > the age of the patch. I'm not well-equipped to do any testing myself > right now or I'd volunteer. So I gave it a spin on my OPi Zero 3. It seems to work(TM), although I need to stop the connection to the Ethernet gadget: => ums 0 mmc 0 UMS: LUN 0, dev mmc 0, hwpart 0, sector 0x0, count 0x3af000 All UDCs in use (1 available), use the unbind command g_dnl_register: failed!, error: -19 g_dnl_register failed => unbind ethernet 1 => ums 0 mmc 0 => ums 0 mmc 0 UMS: LUN 0, dev mmc 0, hwpart 0, sector 0x0, count 0x3af000 (works and waits at this point) This seems like a quite annoying limitation, if anyone has an idea what should be done here, I am all ears. Maybe dropping the default Ethernet connection? Thanks! Andre > > > > > >> } > >> > >> static int musb_usb_remove(struct udevice *dev) > >
Hi, On Fri, Jun 28, 2024 at 04:17:27PM +0100, Andre Przywara wrote: > > I do not know if this patch is still a > > necessity; though if John is nudging about it, it probably is. > > Yes apparently he needs it, though I am not entirely sure why. > USB gadget has worked for ages in sunxi, without DM_USB_GADGET support, > that's why I was a bit puzzled why this patch seems so important. > > And secondly I was put off by John's initial reply that it would trigger > many USB errors for him. He later rectified it, but I must have missed > that message. This patch is necessary for gadget to work on the T113 board I have. Without it I get this error during fastboot: Controller uninitialized g_dnl_register: failed!, error: -6 John.
On Sat, 29 Jun 2024 02:22:14 +1000 John Watts <contact@jookia.org> wrote: Hi, > On Fri, Jun 28, 2024 at 04:17:27PM +0100, Andre Przywara wrote: > > > I do not know if this patch is still a > > > necessity; though if John is nudging about it, it probably is. > > > > Yes apparently he needs it, though I am not entirely sure why. > > USB gadget has worked for ages in sunxi, without DM_USB_GADGET support, > > that's why I was a bit puzzled why this patch seems so important. > > > > And secondly I was put off by John's initial reply that it would trigger > > many USB errors for him. He later rectified it, but I must have missed > > that message. > > This patch is necessary for gadget to work on the T113 board I have. > Without it I get this error during fastboot: > > Controller uninitialized > g_dnl_register: failed!, error: -6 So I dug into this, and this is what I think is going on: If you add "CONFIG_USB_MUSB_GADGET=y" to your *defconfig*, it should work. If you just enable it via menuconfig or by editing .config, it will not. Adding CONFIG_USB_ETHER to the mix should fix this. This symbol is automatically selected in Kconfig when it sees USB_MUSB_GADGET, but apparently only during a defconfig run, it's not selected during the other methods. Not sure what causes this difference in behaviour, but I have seen this in U-Boot before. That being said: this is of course not a proper solution, the DM support proposed in that patch is the right way to go, but it should explain why it works for me, but not for you. As for this patch: the problem is where the Ethernet gadget code apparently grabs the UDC device, without actually using it already. This prevents other gadgets like ums to be used, without unbinding the Ethernet gadget with a non-obvious command first. So I think we need a solution for this first, otherwise the user experience would suffer. Cheers, Andre
diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c index 510b254f7d..6658cd995d 100644 --- a/drivers/usb/musb-new/sunxi.c +++ b/drivers/usb/musb-new/sunxi.c @@ -444,6 +444,16 @@ static struct musb_hdrc_config musb_config_h3 = { .ram_bits = SUNXI_MUSB_RAM_BITS, }; +#if CONFIG_IS_ENABLED(DM_USB_GADGET) +int dm_usb_gadget_handle_interrupts(struct udevice *dev) { + struct sunxi_glue *glue = dev_get_priv(dev); + struct musb_host_data *host = &glue->mdata; + + host->host->isr(0, host->host); + return 0; +} +#endif + static int musb_usb_probe(struct udevice *dev) { struct sunxi_glue *glue = dev_get_priv(dev); @@ -452,10 +462,6 @@ static int musb_usb_probe(struct udevice *dev) void *base = dev_read_addr_ptr(dev); int ret; -#ifdef CONFIG_USB_MUSB_HOST - struct usb_bus_priv *priv = dev_get_uclass_priv(dev); -#endif - if (!base) return -EINVAL; @@ -486,23 +492,31 @@ static int musb_usb_probe(struct udevice *dev) pdata.platform_ops = &sunxi_musb_ops; pdata.config = glue->cfg->config; -#ifdef CONFIG_USB_MUSB_HOST - priv->desc_before_addr = true; + if (IS_ENABLED(CONFIG_USB_MUSB_HOST)) { + struct usb_bus_priv *priv = dev_get_uclass_priv(dev); + priv->desc_before_addr = true; - pdata.mode = MUSB_HOST; - host->host = musb_init_controller(&pdata, &glue->dev, base); - if (!host->host) - return -EIO; + pdata.mode = MUSB_HOST; + host->host = musb_init_controller(&pdata, &glue->dev, base); + if (!host->host) + return -EIO; - return musb_lowlevel_init(host); -#else - pdata.mode = MUSB_PERIPHERAL; - host->host = musb_register(&pdata, &glue->dev, base); - if (IS_ERR_OR_NULL(host->host)) - return -EIO; + return musb_lowlevel_init(host); + } else if (CONFIG_IS_ENABLED(DM_USB_GADGET)) { + pdata.mode = MUSB_PERIPHERAL; + host->host = musb_init_controller(&pdata, &glue->dev, base); + if (!host->host) + return -EIO; - return 0; -#endif + return usb_add_gadget_udc(&glue->dev, &host->host->g); + } else { + pdata.mode = MUSB_PERIPHERAL; + host->host = musb_register(&pdata, &glue->dev, base); + if (IS_ERR_OR_NULL(host->host)) + return -EIO; + + return 0; + } } static int musb_usb_remove(struct udevice *dev)
Since many sunxi boards do not implement a `board_usb_init`, it's better if we just make the sunxi USB driver compatible with the DM gadget model, as many other musb-new variants already are. This change has been verified working on a T113s. Signed-off-by: Sam Edwards <CFSworks@gmail.com> --- drivers/usb/musb-new/sunxi.c | 50 +++++++++++++++++++++++------------- 1 file changed, 32 insertions(+), 18 deletions(-)