Re: [PATCH 2/2] usb: dwc2: gadget reuse ahbcfg assigned from platform

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

 



On 6 February 2015 at 16:07, Kaukab, Yousaf <yousaf.kaukab@xxxxxxxxx> wrote:
>> >> GAHBCFG_HBSTLEN_INCR4 << diff --git a/drivers/usb/dwc2/gadget.c
>> >> b/drivers/usb/dwc2/gadget.c index 15aa578..20085de 100644
>> >> --- a/drivers/usb/dwc2/gadget.c
>> >> +++ b/drivers/usb/dwc2/gadget.c
>> >> @@ -2314,9 +2314,13 @@ void s3c_hsotg_core_init_disconnected(struct
>> >> dwc2_hsotg *hsotg,
>> >>               GINTSTS_USBSUSP | GINTSTS_WKUPINT,
>> >>               hsotg->regs + GINTMSK);
>> >>
>> >> +     if ((hsotg->core_params) && (hsotg->core_params->ahbcfg != -
>> >> 1))
>> >> +             val = hsotg->core_params->ahbcfg &
>> >> ~GAHBCFG_CTRL_MASK;
>> >> +     else
>> >> +             val = GAHBCFG_HBSTLEN_INCR4 <<
>> >> GAHBCFG_HBSTLEN_SHIFT;
>> >> +
>> >>       if (using_dma(hsotg))
>> >> -             writel(GAHBCFG_GLBL_INTR_EN |
>> >> GAHBCFG_DMA_EN |
>> >> -                    (GAHBCFG_HBSTLEN_INCR4 <<
>> >> GAHBCFG_HBSTLEN_SHIFT),
>> >> +             writel(GAHBCFG_GLBL_INTR_EN |
>> >> GAHBCFG_DMA_EN | val,
>> >>                      hsotg->regs + GAHBCFG);
>> >>       else
>> >>               writel(((hsotg->dedicated_fifos) ?
>> >> (GAHBCFG_NP_TXF_EMP_LVL |
>> >
>> > There are other bits in GAHBCFG that can be set from platform. They will be
>> preserved by your patch, as they are not part of GAHBCFG_CTRL_MASK, but
>> only in case dma is enabled. Perhaps preserve them in non-dma case as well.
>>
>> Here may have issue if also set hsotg->core_params->ahbcfg for non-dma case,
>> since GAHBCFG[4:1] may be set.
>
> You can mask off HBstLen in that case. However, I don't think setting burst length will be an issue in non DMA case as DWC2 will not act as a bus master. John, can you please confirm if setting burst length will be an issue in non-dma case?

It would be great if John has some input.
I am not sure, just doubt ahbcfg is specifically used for dma mode.

static int dwc2_gahbcfg_init(struct dwc2_hsotg *hsotg)
{
        case GHWCFG2_INT_DMA_ARCH:
                dev_dbg(hsotg->dev, "Internal DMA Mode\n");
                if (hsotg->core_params->ahbcfg != -1) {
                        ahbcfg &= GAHBCFG_CTRL_MASK;
                        ahbcfg |= hsotg->core_params->ahbcfg &
                                  ~GAHBCFG_CTRL_MASK;
                }
                break;
}

Looks like only GHWCFG2_INT_DMA_ARCH case cares the value of ahbcfg.

>
>>
>> Though from drivers/usb/dwc2/core.h we can not see @ahbcfg is specifically
>> used for dma case, most case in drivers/usb/dwc2/platform.c use ahbcfg is set
>> hbstlen, GAHBCFG[4:1].
>> For example, our platform set GAHBCFG_HBSTLEN_INCR16.
>>
>> So I just assume @ahbcfg is used for dma case.
>> What do you think.
>
> While you are fixing it, why not fix it for other bits, for example AHBSingle, InvDescEndianness etc.,  which are part of the same register and will be overwritten at the same place.

Yes, understand.
Not sure other value need to be overwirtten, if only GAHBCFG[4:1],
burst len, maybe we can add another property?

Will update accordingly after John give some info.

Thanks Yousaf
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




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

  Powered by Linux