| Message ID | 20170216120028.GA29698@mwanda (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Feb 16, 2017 at 7:00 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote: > We're off by one here. We free something that wasn't allocated and we > don't free msm_host->bus_clks[]. > > Fixes: 6e0eb52eba9e ("drm/msm/dsi: Parse bus clocks from a list") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> Thanks Reviewed-by: Rob Clark <robdclark@gmail.com> > > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c > index 1fc07ce24686..4fa32296162e 100644 > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c > @@ -437,7 +437,7 @@ static int dsi_bus_clk_enable(struct msm_dsi_host *msm_host) > > return 0; > err: > - for (; i > 0; i--) > + while (i--) > clk_disable_unprepare(msm_host->bus_clks[i]); > > return ret;
On Thu, 16 Feb 2017, Rob Clark <robdclark@gmail.com> wrote: > On Thu, Feb 16, 2017 at 7:00 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote: >> We're off by one here. We free something that wasn't allocated and we >> don't free msm_host->bus_clks[]. >> >> Fixes: 6e0eb52eba9e ("drm/msm/dsi: Parse bus clocks from a list") >> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > Thanks > > Reviewed-by: Rob Clark <robdclark@gmail.com> And for good measure, Reviewed-by: Jani Nikula <jani.nikula@intel.com> > >> >> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c >> index 1fc07ce24686..4fa32296162e 100644 >> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c >> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c >> @@ -437,7 +437,7 @@ static int dsi_bus_clk_enable(struct msm_dsi_host *msm_host) >> >> return 0; >> err: >> - for (; i > 0; i--) >> + while (i--) >> clk_disable_unprepare(msm_host->bus_clks[i]); >> >> return ret;
On Thu, Feb 16, 2017 at 02:27:08PM +0200, Jani Nikula wrote: > On Thu, 16 Feb 2017, Rob Clark <robdclark@gmail.com> wrote: > > On Thu, Feb 16, 2017 at 7:00 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote: > >> We're off by one here. We free something that wasn't allocated and we > >> don't free msm_host->bus_clks[]. > >> > >> Fixes: 6e0eb52eba9e ("drm/msm/dsi: Parse bus clocks from a list") > >> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > > > Thanks > > > > Reviewed-by: Rob Clark <robdclark@gmail.com> > > And for good measure, > > Reviewed-by: Jani Nikula <jani.nikula@intel.com> So 2 r-b from maintainers, no one said he'll apply. This isn't really great coordination :-) Note that drm-misc-next is always open, so you could always smash it in there, irrespective of merge window state. hint, hint ... -Daniel > > > > > >> > >> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c > >> index 1fc07ce24686..4fa32296162e 100644 > >> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c > >> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c > >> @@ -437,7 +437,7 @@ static int dsi_bus_clk_enable(struct msm_dsi_host *msm_host) > >> > >> return 0; > >> err: > >> - for (; i > 0; i--) > >> + while (i--) > >> clk_disable_unprepare(msm_host->bus_clks[i]); > >> > >> return ret; > > -- > Jani Nikula, Intel Open Source Technology Center > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Sun, 26 Feb 2017, Daniel Vetter <daniel@ffwll.ch> wrote: > On Thu, Feb 16, 2017 at 02:27:08PM +0200, Jani Nikula wrote: >> On Thu, 16 Feb 2017, Rob Clark <robdclark@gmail.com> wrote: >> > On Thu, Feb 16, 2017 at 7:00 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote: >> >> We're off by one here. We free something that wasn't allocated and we >> >> don't free msm_host->bus_clks[]. >> >> >> >> Fixes: 6e0eb52eba9e ("drm/msm/dsi: Parse bus clocks from a list") >> >> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> >> > >> > Thanks >> > >> > Reviewed-by: Rob Clark <robdclark@gmail.com> >> >> And for good measure, >> >> Reviewed-by: Jani Nikula <jani.nikula@intel.com> > > So 2 r-b from maintainers, no one said he'll apply. This isn't really > great coordination :-) Note that drm-misc-next is always open, so you > could always smash it in there, irrespective of merge window state. hint, > hint ... Admittedly I shied away from touching drm/msm. BR, Jani. > -Daniel > >> >> >> > >> >> >> >> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c >> >> index 1fc07ce24686..4fa32296162e 100644 >> >> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c >> >> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c >> >> @@ -437,7 +437,7 @@ static int dsi_bus_clk_enable(struct msm_dsi_host *msm_host) >> >> >> >> return 0; >> >> err: >> >> - for (; i > 0; i--) >> >> + while (i--) >> >> clk_disable_unprepare(msm_host->bus_clks[i]); >> >> >> >> return ret; >> >> -- >> Jani Nikula, Intel Open Source Technology Center >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Mon, Feb 27, 2017 at 11:18 AM, Jani Nikula <jani.nikula@linux.intel.com> wrote: > On Sun, 26 Feb 2017, Daniel Vetter <daniel@ffwll.ch> wrote: >> On Thu, Feb 16, 2017 at 02:27:08PM +0200, Jani Nikula wrote: >>> On Thu, 16 Feb 2017, Rob Clark <robdclark@gmail.com> wrote: >>> > On Thu, Feb 16, 2017 at 7:00 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote: >>> >> We're off by one here. We free something that wasn't allocated and we >>> >> don't free msm_host->bus_clks[]. >>> >> >>> >> Fixes: 6e0eb52eba9e ("drm/msm/dsi: Parse bus clocks from a list") >>> >> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> >>> > >>> > Thanks >>> > >>> > Reviewed-by: Rob Clark <robdclark@gmail.com> >>> >>> And for good measure, >>> >>> Reviewed-by: Jani Nikula <jani.nikula@intel.com> >> >> So 2 r-b from maintainers, no one said he'll apply. This isn't really >> great coordination :-) Note that drm-misc-next is always open, so you >> could always smash it in there, irrespective of merge window state. hint, >> hint ... > > Admittedly I shied away from touching drm/msm. Well that's kinda my point, we have a pile of maintainers who could push this, which means if no one says they do, the patch will likely get lost. Especially if the main maintainer (Rob here) smashes an r-b onto a patch it's super confusing, at least to me. I guess this is a downside to having lots of committers, and I started to stumble over this in a bunch of places. I think as a rule we should always state when we plan to or have merged a patch, and if it's just an r-b assume it's lost ... At least that's how I deal with core refactorings touching drivers, otherwise we'd probably never get them all landed. -Daniel
On Mon, Feb 27, 2017 at 5:25 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > On Mon, Feb 27, 2017 at 11:18 AM, Jani Nikula > <jani.nikula@linux.intel.com> wrote: >> On Sun, 26 Feb 2017, Daniel Vetter <daniel@ffwll.ch> wrote: >>> On Thu, Feb 16, 2017 at 02:27:08PM +0200, Jani Nikula wrote: >>>> On Thu, 16 Feb 2017, Rob Clark <robdclark@gmail.com> wrote: >>>> > On Thu, Feb 16, 2017 at 7:00 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote: >>>> >> We're off by one here. We free something that wasn't allocated and we >>>> >> don't free msm_host->bus_clks[]. >>>> >> >>>> >> Fixes: 6e0eb52eba9e ("drm/msm/dsi: Parse bus clocks from a list") >>>> >> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> >>>> > >>>> > Thanks >>>> > >>>> > Reviewed-by: Rob Clark <robdclark@gmail.com> >>>> >>>> And for good measure, >>>> >>>> Reviewed-by: Jani Nikula <jani.nikula@intel.com> >>> >>> So 2 r-b from maintainers, no one said he'll apply. This isn't really >>> great coordination :-) Note that drm-misc-next is always open, so you >>> could always smash it in there, irrespective of merge window state. hint, >>> hint ... >> >> Admittedly I shied away from touching drm/msm. > > Well that's kinda my point, we have a pile of maintainers who could > push this, which means if no one says they do, the patch will likely > get lost. Especially if the main maintainer (Rob here) smashes an r-b > onto a patch it's super confusing, at least to me. the r-b was in case you wanted to pick it up in drm-misc (with the usual backup plan of sending it in an msm-fixes pull req after the first rc or two). I don't mind pushing small fixes to drm-misc instead, since I generally don't have a lot of 'em BR, -R > I guess this is a downside to having lots of committers, and I started > to stumble over this in a bunch of places. I think as a rule we should > always state when we plan to or have merged a patch, and if it's just > an r-b assume it's lost ... At least that's how I deal with core > refactorings touching drivers, otherwise we'd probably never get them > all landed. > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 1fc07ce24686..4fa32296162e 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -437,7 +437,7 @@ static int dsi_bus_clk_enable(struct msm_dsi_host *msm_host) return 0; err: - for (; i > 0; i--) + while (i--) clk_disable_unprepare(msm_host->bus_clks[i]); return ret;
We're off by one here. We free something that wasn't allocated and we don't free msm_host->bus_clks[]. Fixes: 6e0eb52eba9e ("drm/msm/dsi: Parse bus clocks from a list") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>