* [PATCH] stm: the number of masters should be (sw_end - sw_start + 1) @ 2015-12-11 3:26 Chunyan Zhang 2015-12-11 7:31 ` Alexander Shishkin 0 siblings, 1 reply; 7+ messages in thread From: Chunyan Zhang @ 2015-12-11 3:26 UTC (permalink / raw) To: alexander.shishkin; +Cc: linux-kernel, mathieu.poirier sw_end represents the last software master, sw_start is index of the first master, so the number of software masters should be sw_end - sw_start + 1. Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org> --- drivers/hwtracing/intel_th/sth.c | 2 +- drivers/hwtracing/stm/core.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/hwtracing/intel_th/sth.c b/drivers/hwtracing/intel_th/sth.c index 56101c3..28917d7 100644 --- a/drivers/hwtracing/intel_th/sth.c +++ b/drivers/hwtracing/intel_th/sth.c @@ -173,7 +173,7 @@ static int intel_th_sw_init(struct sth_device *sth) sth->stm.sw_start = reg & 0xffff; sth->stm.sw_end = reg >> 16; - sth->sw_nmasters = sth->stm.sw_end - sth->stm.sw_start; + sth->sw_nmasters = sth->stm.sw_end - sth->stm.sw_start + 1; dev_dbg(sth->dev, "sw_start: %x sw_end: %x masters: %x nchannels: %x\n", sth->stm.sw_start, sth->stm.sw_end, sth->sw_nmasters, sth->stm.sw_nchannels); diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c index 7f7bdb3..cb676f2 100644 --- a/drivers/hwtracing/stm/core.c +++ b/drivers/hwtracing/stm/core.c @@ -632,7 +632,7 @@ int stm_register_device(struct device *parent, struct stm_data *stm_data, if (!stm_data->packet || !stm_data->sw_nchannels) return -EINVAL; - nmasters = stm_data->sw_end - stm_data->sw_start; + nmasters = stm_data->sw_end - stm_data->sw_start + 1; stm = kzalloc(sizeof(*stm) + nmasters * sizeof(void *), GFP_KERNEL); if (!stm) return -ENOMEM; -- 1.9.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] stm: the number of masters should be (sw_end - sw_start + 1) 2015-12-11 3:26 [PATCH] stm: the number of masters should be (sw_end - sw_start + 1) Chunyan Zhang @ 2015-12-11 7:31 ` Alexander Shishkin 2015-12-11 7:59 ` Chunyan Zhang 2015-12-11 8:38 ` Chunyan Zhang 0 siblings, 2 replies; 7+ messages in thread From: Alexander Shishkin @ 2015-12-11 7:31 UTC (permalink / raw) To: Chunyan Zhang; +Cc: linux-kernel, mathieu.poirier Chunyan Zhang <zhang.chunyan@linaro.org> writes: > sw_end represents the last software master, sw_start is index of the > first master, so the number of software masters should be > sw_end - sw_start + 1. Looks about right, but it needs to be in two separate patches. > Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org> > --- > drivers/hwtracing/intel_th/sth.c | 2 +- > drivers/hwtracing/stm/core.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/hwtracing/intel_th/sth.c b/drivers/hwtracing/intel_th/sth.c > index 56101c3..28917d7 100644 > --- a/drivers/hwtracing/intel_th/sth.c > +++ b/drivers/hwtracing/intel_th/sth.c > @@ -173,7 +173,7 @@ static int intel_th_sw_init(struct sth_device *sth) > sth->stm.sw_start = reg & 0xffff; > sth->stm.sw_end = reg >> 16; > > - sth->sw_nmasters = sth->stm.sw_end - sth->stm.sw_start; > + sth->sw_nmasters = sth->stm.sw_end - sth->stm.sw_start + 1; > dev_dbg(sth->dev, "sw_start: %x sw_end: %x masters: %x nchannels: %x\n", > sth->stm.sw_start, sth->stm.sw_end, sth->sw_nmasters, > sth->stm.sw_nchannels); > diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c > index 7f7bdb3..cb676f2 100644 > --- a/drivers/hwtracing/stm/core.c > +++ b/drivers/hwtracing/stm/core.c > @@ -632,7 +632,7 @@ int stm_register_device(struct device *parent, struct stm_data *stm_data, > if (!stm_data->packet || !stm_data->sw_nchannels) > return -EINVAL; > > - nmasters = stm_data->sw_end - stm_data->sw_start; > + nmasters = stm_data->sw_end - stm_data->sw_start + 1; > stm = kzalloc(sizeof(*stm) + nmasters * sizeof(void *), GFP_KERNEL); Or even offsetof(struct stm_device, masters[stm_data->sw_end]). > if (!stm) > return -ENOMEM; > -- > 1.9.1 This is a very old version on git, btw. :) Thanks, -- Alex ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] stm: the number of masters should be (sw_end - sw_start + 1) 2015-12-11 7:31 ` Alexander Shishkin @ 2015-12-11 7:59 ` Chunyan Zhang 2015-12-11 8:38 ` Chunyan Zhang 1 sibling, 0 replies; 7+ messages in thread From: Chunyan Zhang @ 2015-12-11 7:59 UTC (permalink / raw) To: Alexander Shishkin; +Cc: linux-kernel@vger.kernel.org, Mathieu Poirier On Fri, Dec 11, 2015 at 3:31 PM, Alexander Shishkin <alexander.shishkin@linux.intel.com> wrote: > Chunyan Zhang <zhang.chunyan@linaro.org> writes: > >> sw_end represents the last software master, sw_start is index of the >> first master, so the number of software masters should be >> sw_end - sw_start + 1. > > Looks about right, but it needs to be in two separate patches. Will split this patch later. > >> Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org> >> --- >> drivers/hwtracing/intel_th/sth.c | 2 +- >> drivers/hwtracing/stm/core.c | 2 +- >> 2 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/hwtracing/intel_th/sth.c b/drivers/hwtracing/intel_th/sth.c >> index 56101c3..28917d7 100644 >> --- a/drivers/hwtracing/intel_th/sth.c >> +++ b/drivers/hwtracing/intel_th/sth.c >> @@ -173,7 +173,7 @@ static int intel_th_sw_init(struct sth_device *sth) >> sth->stm.sw_start = reg & 0xffff; >> sth->stm.sw_end = reg >> 16; >> >> - sth->sw_nmasters = sth->stm.sw_end - sth->stm.sw_start; >> + sth->sw_nmasters = sth->stm.sw_end - sth->stm.sw_start + 1; >> dev_dbg(sth->dev, "sw_start: %x sw_end: %x masters: %x nchannels: %x\n", >> sth->stm.sw_start, sth->stm.sw_end, sth->sw_nmasters, >> sth->stm.sw_nchannels); >> diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c >> index 7f7bdb3..cb676f2 100644 >> --- a/drivers/hwtracing/stm/core.c >> +++ b/drivers/hwtracing/stm/core.c >> @@ -632,7 +632,7 @@ int stm_register_device(struct device *parent, struct stm_data *stm_data, >> if (!stm_data->packet || !stm_data->sw_nchannels) >> return -EINVAL; >> >> - nmasters = stm_data->sw_end - stm_data->sw_start; >> + nmasters = stm_data->sw_end - stm_data->sw_start + 1; >> stm = kzalloc(sizeof(*stm) + nmasters * sizeof(void *), GFP_KERNEL); > > Or even offsetof(struct stm_device, masters[stm_data->sw_end]). Ok, I can add this modification in next version, no need to make this being a separated patch, right? > >> if (!stm) >> return -ENOMEM; >> -- >> 1.9.1 > > This is a very old version on git, btw. :) Didn't notice this before :), I have been using the one that Ubuntu 14.04 provided. Thanks, Chunyan > > Thanks, > -- > Alex ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] stm: the number of masters should be (sw_end - sw_start + 1) 2015-12-11 7:31 ` Alexander Shishkin 2015-12-11 7:59 ` Chunyan Zhang @ 2015-12-11 8:38 ` Chunyan Zhang 2015-12-11 8:51 ` Alexander Shishkin 1 sibling, 1 reply; 7+ messages in thread From: Chunyan Zhang @ 2015-12-11 8:38 UTC (permalink / raw) To: Alexander Shishkin; +Cc: linux-kernel@vger.kernel.org, Mathieu Poirier On Fri, Dec 11, 2015 at 3:31 PM, Alexander Shishkin <alexander.shishkin@linux.intel.com> wrote: > Chunyan Zhang <zhang.chunyan@linaro.org> writes: > >> sw_end represents the last software master, sw_start is index of the >> first master, so the number of software masters should be >> sw_end - sw_start + 1. > > Looks about right, but it needs to be in two separate patches. > >> Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org> >> --- >> drivers/hwtracing/intel_th/sth.c | 2 +- >> drivers/hwtracing/stm/core.c | 2 +- >> 2 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/hwtracing/intel_th/sth.c b/drivers/hwtracing/intel_th/sth.c >> index 56101c3..28917d7 100644 >> --- a/drivers/hwtracing/intel_th/sth.c >> +++ b/drivers/hwtracing/intel_th/sth.c >> @@ -173,7 +173,7 @@ static int intel_th_sw_init(struct sth_device *sth) >> sth->stm.sw_start = reg & 0xffff; >> sth->stm.sw_end = reg >> 16; >> >> - sth->sw_nmasters = sth->stm.sw_end - sth->stm.sw_start; >> + sth->sw_nmasters = sth->stm.sw_end - sth->stm.sw_start + 1; >> dev_dbg(sth->dev, "sw_start: %x sw_end: %x masters: %x nchannels: %x\n", >> sth->stm.sw_start, sth->stm.sw_end, sth->sw_nmasters, >> sth->stm.sw_nchannels); >> diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c >> index 7f7bdb3..cb676f2 100644 >> --- a/drivers/hwtracing/stm/core.c >> +++ b/drivers/hwtracing/stm/core.c >> @@ -632,7 +632,7 @@ int stm_register_device(struct device *parent, struct stm_data *stm_data, >> if (!stm_data->packet || !stm_data->sw_nchannels) >> return -EINVAL; >> >> - nmasters = stm_data->sw_end - stm_data->sw_start; >> + nmasters = stm_data->sw_end - stm_data->sw_start + 1; >> stm = kzalloc(sizeof(*stm) + nmasters * sizeof(void *), GFP_KERNEL); > > Or even offsetof(struct stm_device, masters[stm_data->sw_end]). > This should use 'offsetofend()'. Thanks, Chunyan >> if (!stm) >> return -ENOMEM; >> -- >> 1.9.1 > > This is a very old version on git, btw. :) > > Thanks, > -- > Alex ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] stm: the number of masters should be (sw_end - sw_start + 1) 2015-12-11 8:38 ` Chunyan Zhang @ 2015-12-11 8:51 ` Alexander Shishkin 2015-12-11 9:10 ` Chunyan Zhang 0 siblings, 1 reply; 7+ messages in thread From: Alexander Shishkin @ 2015-12-11 8:51 UTC (permalink / raw) To: Chunyan Zhang; +Cc: linux-kernel@vger.kernel.org, Mathieu Poirier Chunyan Zhang <zhang.chunyan@linaro.org> writes: > On Fri, Dec 11, 2015 at 3:31 PM, Alexander Shishkin > <alexander.shishkin@linux.intel.com> wrote: >> Chunyan Zhang <zhang.chunyan@linaro.org> writes: >> >>> sw_end represents the last software master, sw_start is index of the >>> first master, so the number of software masters should be >>> sw_end - sw_start + 1. >> >> Looks about right, but it needs to be in two separate patches. >> >>> Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org> >>> --- >>> drivers/hwtracing/intel_th/sth.c | 2 +- >>> drivers/hwtracing/stm/core.c | 2 +- >>> 2 files changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/hwtracing/intel_th/sth.c b/drivers/hwtracing/intel_th/sth.c >>> index 56101c3..28917d7 100644 >>> --- a/drivers/hwtracing/intel_th/sth.c >>> +++ b/drivers/hwtracing/intel_th/sth.c >>> @@ -173,7 +173,7 @@ static int intel_th_sw_init(struct sth_device *sth) >>> sth->stm.sw_start = reg & 0xffff; >>> sth->stm.sw_end = reg >> 16; >>> >>> - sth->sw_nmasters = sth->stm.sw_end - sth->stm.sw_start; >>> + sth->sw_nmasters = sth->stm.sw_end - sth->stm.sw_start + 1; >>> dev_dbg(sth->dev, "sw_start: %x sw_end: %x masters: %x nchannels: %x\n", >>> sth->stm.sw_start, sth->stm.sw_end, sth->sw_nmasters, >>> sth->stm.sw_nchannels); >>> diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c >>> index 7f7bdb3..cb676f2 100644 >>> --- a/drivers/hwtracing/stm/core.c >>> +++ b/drivers/hwtracing/stm/core.c >>> @@ -632,7 +632,7 @@ int stm_register_device(struct device *parent, struct stm_data *stm_data, >>> if (!stm_data->packet || !stm_data->sw_nchannels) >>> return -EINVAL; >>> >>> - nmasters = stm_data->sw_end - stm_data->sw_start; >>> + nmasters = stm_data->sw_end - stm_data->sw_start + 1; >>> stm = kzalloc(sizeof(*stm) + nmasters * sizeof(void *), GFP_KERNEL); >> >> Or even offsetof(struct stm_device, masters[stm_data->sw_end]). >> > > This should use 'offsetofend()'. No, actually, just scratch my previous comment as it was completely wrong, just fix the off-by-one. If we were to use offsetof(), it should rather be of masters[nmasters], but all we need is to fix the off-by-one right now. Regards, -- Alex ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] stm: the number of masters should be (sw_end - sw_start + 1) 2015-12-11 8:51 ` Alexander Shishkin @ 2015-12-11 9:10 ` Chunyan Zhang 2015-12-11 9:34 ` Alexander Shishkin 0 siblings, 1 reply; 7+ messages in thread From: Chunyan Zhang @ 2015-12-11 9:10 UTC (permalink / raw) To: Alexander Shishkin; +Cc: linux-kernel@vger.kernel.org, Mathieu Poirier On Fri, Dec 11, 2015 at 4:51 PM, Alexander Shishkin <alexander.shishkin@linux.intel.com> wrote: > Chunyan Zhang <zhang.chunyan@linaro.org> writes: > >> On Fri, Dec 11, 2015 at 3:31 PM, Alexander Shishkin >> <alexander.shishkin@linux.intel.com> wrote: >>> Chunyan Zhang <zhang.chunyan@linaro.org> writes: >>> >>>> sw_end represents the last software master, sw_start is index of the >>>> first master, so the number of software masters should be >>>> sw_end - sw_start + 1. >>> >>> Looks about right, but it needs to be in two separate patches. >>> >>>> Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org> >>>> --- >>>> drivers/hwtracing/intel_th/sth.c | 2 +- >>>> drivers/hwtracing/stm/core.c | 2 +- >>>> 2 files changed, 2 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/hwtracing/intel_th/sth.c b/drivers/hwtracing/intel_th/sth.c >>>> index 56101c3..28917d7 100644 >>>> --- a/drivers/hwtracing/intel_th/sth.c >>>> +++ b/drivers/hwtracing/intel_th/sth.c >>>> @@ -173,7 +173,7 @@ static int intel_th_sw_init(struct sth_device *sth) >>>> sth->stm.sw_start = reg & 0xffff; >>>> sth->stm.sw_end = reg >> 16; >>>> >>>> - sth->sw_nmasters = sth->stm.sw_end - sth->stm.sw_start; >>>> + sth->sw_nmasters = sth->stm.sw_end - sth->stm.sw_start + 1; >>>> dev_dbg(sth->dev, "sw_start: %x sw_end: %x masters: %x nchannels: %x\n", >>>> sth->stm.sw_start, sth->stm.sw_end, sth->sw_nmasters, >>>> sth->stm.sw_nchannels); >>>> diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c >>>> index 7f7bdb3..cb676f2 100644 >>>> --- a/drivers/hwtracing/stm/core.c >>>> +++ b/drivers/hwtracing/stm/core.c >>>> @@ -632,7 +632,7 @@ int stm_register_device(struct device *parent, struct stm_data *stm_data, >>>> if (!stm_data->packet || !stm_data->sw_nchannels) >>>> return -EINVAL; >>>> >>>> - nmasters = stm_data->sw_end - stm_data->sw_start; >>>> + nmasters = stm_data->sw_end - stm_data->sw_start + 1; >>>> stm = kzalloc(sizeof(*stm) + nmasters * sizeof(void *), GFP_KERNEL); >>> >>> Or even offsetof(struct stm_device, masters[stm_data->sw_end]). >>> >> >> This should use 'offsetofend()'. > > No, actually, just scratch my previous comment as it was completely > wrong, just fix the off-by-one. If we were to use offsetof(), it should > rather be of masters[nmasters], but all we need is to fix the off-by-one > right now. > Sorry, you may lose me here, what's 'off-by-one' ? Thanks, Chunyan > Regards, > -- > Alex ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] stm: the number of masters should be (sw_end - sw_start + 1) 2015-12-11 9:10 ` Chunyan Zhang @ 2015-12-11 9:34 ` Alexander Shishkin 0 siblings, 0 replies; 7+ messages in thread From: Alexander Shishkin @ 2015-12-11 9:34 UTC (permalink / raw) To: Chunyan Zhang; +Cc: linux-kernel@vger.kernel.org, Mathieu Poirier Chunyan Zhang <zhang.chunyan@linaro.org> writes: > On Fri, Dec 11, 2015 at 4:51 PM, Alexander Shishkin > <alexander.shishkin@linux.intel.com> wrote: >> Chunyan Zhang <zhang.chunyan@linaro.org> writes: >> >>> On Fri, Dec 11, 2015 at 3:31 PM, Alexander Shishkin >>> <alexander.shishkin@linux.intel.com> wrote: >>>> Chunyan Zhang <zhang.chunyan@linaro.org> writes: >>>> >>>>> sw_end represents the last software master, sw_start is index of the >>>>> first master, so the number of software masters should be >>>>> sw_end - sw_start + 1. >>>> >>>> Looks about right, but it needs to be in two separate patches. >>>> >>>>> Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org> >>>>> --- >>>>> drivers/hwtracing/intel_th/sth.c | 2 +- >>>>> drivers/hwtracing/stm/core.c | 2 +- >>>>> 2 files changed, 2 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/drivers/hwtracing/intel_th/sth.c b/drivers/hwtracing/intel_th/sth.c >>>>> index 56101c3..28917d7 100644 >>>>> --- a/drivers/hwtracing/intel_th/sth.c >>>>> +++ b/drivers/hwtracing/intel_th/sth.c >>>>> @@ -173,7 +173,7 @@ static int intel_th_sw_init(struct sth_device *sth) >>>>> sth->stm.sw_start = reg & 0xffff; >>>>> sth->stm.sw_end = reg >> 16; >>>>> >>>>> - sth->sw_nmasters = sth->stm.sw_end - sth->stm.sw_start; >>>>> + sth->sw_nmasters = sth->stm.sw_end - sth->stm.sw_start + 1; >>>>> dev_dbg(sth->dev, "sw_start: %x sw_end: %x masters: %x nchannels: %x\n", >>>>> sth->stm.sw_start, sth->stm.sw_end, sth->sw_nmasters, >>>>> sth->stm.sw_nchannels); >>>>> diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c >>>>> index 7f7bdb3..cb676f2 100644 >>>>> --- a/drivers/hwtracing/stm/core.c >>>>> +++ b/drivers/hwtracing/stm/core.c >>>>> @@ -632,7 +632,7 @@ int stm_register_device(struct device *parent, struct stm_data *stm_data, >>>>> if (!stm_data->packet || !stm_data->sw_nchannels) >>>>> return -EINVAL; >>>>> >>>>> - nmasters = stm_data->sw_end - stm_data->sw_start; >>>>> + nmasters = stm_data->sw_end - stm_data->sw_start + 1; >>>>> stm = kzalloc(sizeof(*stm) + nmasters * sizeof(void *), GFP_KERNEL); >>>> >>>> Or even offsetof(struct stm_device, masters[stm_data->sw_end]). >>>> >>> >>> This should use 'offsetofend()'. >> >> No, actually, just scratch my previous comment as it was completely >> wrong, just fix the off-by-one. If we were to use offsetof(), it should >> rather be of masters[nmasters], but all we need is to fix the off-by-one >> right now. >> > > Sorry, you may lose me here, what's 'off-by-one' ? It's an error when the result of your calculation is off by 1 (one too few or one too many). https://www.google.com/search?q=off-by-one Regards, -- Alex ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-12-11 9:36 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-12-11 3:26 [PATCH] stm: the number of masters should be (sw_end - sw_start + 1) Chunyan Zhang 2015-12-11 7:31 ` Alexander Shishkin 2015-12-11 7:59 ` Chunyan Zhang 2015-12-11 8:38 ` Chunyan Zhang 2015-12-11 8:51 ` Alexander Shishkin 2015-12-11 9:10 ` Chunyan Zhang 2015-12-11 9:34 ` Alexander Shishkin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox