| Message ID | 20160626072838.28082-11-stephen.boyd@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, Jun 26, 2016 at 12:28:27AM -0700, Stephen Boyd wrote: > The core framework already handles setting this parameter with a > platform quirk. Add the appropriate flag so that we always set > AHBBURST to 0. Technically DT should be doing this, but we always > do it for msm chipidea devices so setting the flag in the driver > works just as well. You still need to set AHB burst value at dts, this flag is just for override, see below: ahb-burst-config = <0x0>; > > Cc: Peter Chen <peter.chen@nxp.com> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org> > --- > drivers/usb/chipidea/ci_hdrc_msm.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c b/drivers/usb/chipidea/ci_hdrc_msm.c > index 3889809fd0c4..37591a4b1346 100644 > --- a/drivers/usb/chipidea/ci_hdrc_msm.c > +++ b/drivers/usb/chipidea/ci_hdrc_msm.c > @@ -24,7 +24,6 @@ static void ci_hdrc_msm_notify_event(struct ci_hdrc *ci, unsigned event) > switch (event) { > case CI_HDRC_CONTROLLER_RESET_EVENT: > dev_dbg(dev, "CI_HDRC_CONTROLLER_RESET_EVENT received\n"); > - writel(0, USB_AHBBURST); > /* use AHB transactor, allow posted data writes */ > writel(0x8, USB_AHBMODE); > usb_phy_init(ci->usb_phy); > @@ -47,7 +46,8 @@ static struct ci_hdrc_platform_data ci_hdrc_msm_platdata = { > .name = "ci_hdrc_msm", > .capoffset = DEF_CAPOFFSET, > .flags = CI_HDRC_REGS_SHARED | > - CI_HDRC_DISABLE_STREAMING, > + CI_HDRC_DISABLE_STREAMING | > + CI_HDRC_OVERRIDE_AHB_BURST, > > .notify_event = ci_hdrc_msm_notify_event, > }; > -- > 2.9.0.rc2.8.ga28705d > > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoting Peter Chen (2016-06-28 23:32:11) > On Sun, Jun 26, 2016 at 12:28:27AM -0700, Stephen Boyd wrote: > > The core framework already handles setting this parameter with a > > platform quirk. Add the appropriate flag so that we always set > > AHBBURST to 0. Technically DT should be doing this, but we always > > do it for msm chipidea devices so setting the flag in the driver > > works just as well. > > You still need to set AHB burst value at dts, this flag is just for > override, see below: > > ahb-burst-config = <0x0>; Right, I have added that to dts now, but the CI_HDRC_OVERRIDE_AHB_BURST flag allows us to specify it from the platdata structure in the ci_hdrc_msm.c file. As the value is zero for msm type controllers, I left it out of the static definition of platdata because all the non-initialized members of that structure are going to be zero anyway. I can explicitly set it to zero to make it more clear if you like.
On Wed, Jun 29, 2016 at 11:59:21AM -0700, Stephen Boyd wrote: > Quoting Peter Chen (2016-06-28 23:32:11) > > On Sun, Jun 26, 2016 at 12:28:27AM -0700, Stephen Boyd wrote: > > > The core framework already handles setting this parameter with a > > > platform quirk. Add the appropriate flag so that we always set > > > AHBBURST to 0. Technically DT should be doing this, but we always > > > do it for msm chipidea devices so setting the flag in the driver > > > works just as well. > > > > You still need to set AHB burst value at dts, this flag is just for > > override, see below: > > > > ahb-burst-config = <0x0>; > > Right, I have added that to dts now, but the CI_HDRC_OVERRIDE_AHB_BURST > flag allows us to specify it from the platdata structure in the > ci_hdrc_msm.c file. As the value is zero for msm type controllers, I > left it out of the static definition of platdata because all the > non-initialized members of that structure are going to be zero anyway. I > can explicitly set it to zero to make it more clear if you like. I suggest setting it explicitly at dts, at current code, it is set as zero explicitly too:)
Quoting Peter Chen (2016-06-29 18:18:27) > On Wed, Jun 29, 2016 at 11:59:21AM -0700, Stephen Boyd wrote: > > Quoting Peter Chen (2016-06-28 23:32:11) > > > On Sun, Jun 26, 2016 at 12:28:27AM -0700, Stephen Boyd wrote: > > > > The core framework already handles setting this parameter with a > > > > platform quirk. Add the appropriate flag so that we always set > > > > AHBBURST to 0. Technically DT should be doing this, but we always > > > > do it for msm chipidea devices so setting the flag in the driver > > > > works just as well. > > > > > > You still need to set AHB burst value at dts, this flag is just for > > > override, see below: > > > > > > ahb-burst-config = <0x0>; > > > > Right, I have added that to dts now, but the CI_HDRC_OVERRIDE_AHB_BURST > > flag allows us to specify it from the platdata structure in the > > ci_hdrc_msm.c file. As the value is zero for msm type controllers, I > > left it out of the static definition of platdata because all the > > non-initialized members of that structure are going to be zero anyway. I > > can explicitly set it to zero to make it more clear if you like. > > I suggest setting it explicitly at dts, at current code, it is set > as zero explicitly too:) > Yes I'm making sure to always set it in dts as well. Should we drop this patch and require dts to always have it set if needed? I was setting it in the driver in case we had some older dts lying around, but I suppose this is changing significantly enough where the qcom,ci_hdrc_msm binding needs to be redone anyway. I'd still like to remove writing the value in the reset callback though.
diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c b/drivers/usb/chipidea/ci_hdrc_msm.c index 3889809fd0c4..37591a4b1346 100644 --- a/drivers/usb/chipidea/ci_hdrc_msm.c +++ b/drivers/usb/chipidea/ci_hdrc_msm.c @@ -24,7 +24,6 @@ static void ci_hdrc_msm_notify_event(struct ci_hdrc *ci, unsigned event) switch (event) { case CI_HDRC_CONTROLLER_RESET_EVENT: dev_dbg(dev, "CI_HDRC_CONTROLLER_RESET_EVENT received\n"); - writel(0, USB_AHBBURST); /* use AHB transactor, allow posted data writes */ writel(0x8, USB_AHBMODE); usb_phy_init(ci->usb_phy); @@ -47,7 +46,8 @@ static struct ci_hdrc_platform_data ci_hdrc_msm_platdata = { .name = "ci_hdrc_msm", .capoffset = DEF_CAPOFFSET, .flags = CI_HDRC_REGS_SHARED | - CI_HDRC_DISABLE_STREAMING, + CI_HDRC_DISABLE_STREAMING | + CI_HDRC_OVERRIDE_AHB_BURST, .notify_event = ci_hdrc_msm_notify_event, };
The core framework already handles setting this parameter with a platform quirk. Add the appropriate flag so that we always set AHBBURST to 0. Technically DT should be doing this, but we always do it for msm chipidea devices so setting the flag in the driver works just as well. Cc: Peter Chen <peter.chen@nxp.com> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org> --- drivers/usb/chipidea/ci_hdrc_msm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)