| Message ID | 20240318022928.509130-2-wangyuquan1236@phytium.com.cn |
|---|---|
| State | Accepted |
| Headers | show |
| Series | cxl/mem: Fix for the index of Clear Event Record Handle | expand |
On Mon, 18 Mar 2024 10:29:28 +0800 Yuquan Wang <wangyuquan1236@phytium.com.cn> wrote: > The dev_dbg info for Clear Event Records mailbox command would report > the handle of the next record to clear not the current one. > > This was because the index 'i' had incremented before printing the > current handle value. > > Signed-off-by: Yuquan Wang <wangyuquan1236@phytium.com.cn> > --- > drivers/cxl/core/mbox.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c > index 9adda4795eb7..b810a6aa3010 100644 > --- a/drivers/cxl/core/mbox.c > +++ b/drivers/cxl/core/mbox.c > @@ -915,7 +915,7 @@ static int cxl_clear_event_record(struct cxl_memdev_state *mds, > > payload->handles[i++] = gen->hdr.handle; > dev_dbg(mds->cxlds.dev, "Event log '%d': Clearing %u\n", log, > - le16_to_cpu(payload->handles[i])); > + le16_to_cpu(payload->handles[i-1])); Trivial but needs spaces around the -. e.g. [i - 1] Maybe Dan can fix up whilst applying. Otherwise Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > if (i == max_handles) { > payload->nr_recs = i;
On Mon, Mar 18, 2024 at 10:29:28AM +0800, Yuquan Wang wrote: > The dev_dbg info for Clear Event Records mailbox command would report > the handle of the next record to clear not the current one. > > This was because the index 'i' had incremented before printing the > current handle value. > > Signed-off-by: Yuquan Wang <wangyuquan1236@phytium.com.cn> > --- > drivers/cxl/core/mbox.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c > index 9adda4795eb7..b810a6aa3010 100644 > --- a/drivers/cxl/core/mbox.c > +++ b/drivers/cxl/core/mbox.c > @@ -915,7 +915,7 @@ static int cxl_clear_event_record(struct cxl_memdev_state *mds, > > payload->handles[i++] = gen->hdr.handle; > dev_dbg(mds->cxlds.dev, "Event log '%d': Clearing %u\n", log, > - le16_to_cpu(payload->handles[i])); > + le16_to_cpu(payload->handles[i-1])); LGTM except for the space issue mentioned by Jonathan. After the fix, Reviewed-by: Fan Ni <fan.ni@samsung.com> Fan > > if (i == max_handles) { > payload->nr_recs = i; > -- > 2.34.1 >
Jonathan Cameron wrote: > On Mon, 18 Mar 2024 10:29:28 +0800 > Yuquan Wang <wangyuquan1236@phytium.com.cn> wrote: > > > The dev_dbg info for Clear Event Records mailbox command would report > > the handle of the next record to clear not the current one. > > > > This was because the index 'i' had incremented before printing the > > current handle value. > > > > Signed-off-by: Yuquan Wang <wangyuquan1236@phytium.com.cn> > > --- > > drivers/cxl/core/mbox.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c > > index 9adda4795eb7..b810a6aa3010 100644 > > --- a/drivers/cxl/core/mbox.c > > +++ b/drivers/cxl/core/mbox.c > > @@ -915,7 +915,7 @@ static int cxl_clear_event_record(struct cxl_memdev_state *mds, > > > > payload->handles[i++] = gen->hdr.handle; > > dev_dbg(mds->cxlds.dev, "Event log '%d': Clearing %u\n", log, > > - le16_to_cpu(payload->handles[i])); > > + le16_to_cpu(payload->handles[i-1])); > Trivial but needs spaces around the -. e.g. [i - 1] > > Maybe Dan can fix up whilst applying. > > Otherwise > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> I have enlisted Dave to start wrangling CXL kernel patches upstream, and I will fall back to just reviewing. Dave, you can add my: Reviewed-by: Dan Williams <dan.j.williams@intel.com> ...with the same caveat as above.
On 3/18/24 5:38 PM, Dan Williams wrote: > Jonathan Cameron wrote: >> On Mon, 18 Mar 2024 10:29:28 +0800 >> Yuquan Wang <wangyuquan1236@phytium.com.cn> wrote: >> >>> The dev_dbg info for Clear Event Records mailbox command would report >>> the handle of the next record to clear not the current one. >>> >>> This was because the index 'i' had incremented before printing the >>> current handle value. >>> >>> Signed-off-by: Yuquan Wang <wangyuquan1236@phytium.com.cn> >>> --- >>> drivers/cxl/core/mbox.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c >>> index 9adda4795eb7..b810a6aa3010 100644 >>> --- a/drivers/cxl/core/mbox.c >>> +++ b/drivers/cxl/core/mbox.c >>> @@ -915,7 +915,7 @@ static int cxl_clear_event_record(struct cxl_memdev_state *mds, >>> >>> payload->handles[i++] = gen->hdr.handle; >>> dev_dbg(mds->cxlds.dev, "Event log '%d': Clearing %u\n", log, >>> - le16_to_cpu(payload->handles[i])); >>> + le16_to_cpu(payload->handles[i-1])); >> Trivial but needs spaces around the -. e.g. [i - 1] >> >> Maybe Dan can fix up whilst applying. >> >> Otherwise >> >> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > I have enlisted Dave to start wrangling CXL kernel patches upstream, and > I will fall back to just reviewing. > > Dave, you can add my: > > Reviewed-by: Dan Williams <dan.j.williams@intel.com> > > ...with the same caveat as above. Applied, updated, and added Fixes: 6ebe28f9ec72 ("cxl/mem: Read, trace, and clear events on driver load") >
diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c index 9adda4795eb7..b810a6aa3010 100644 --- a/drivers/cxl/core/mbox.c +++ b/drivers/cxl/core/mbox.c @@ -915,7 +915,7 @@ static int cxl_clear_event_record(struct cxl_memdev_state *mds, payload->handles[i++] = gen->hdr.handle; dev_dbg(mds->cxlds.dev, "Event log '%d': Clearing %u\n", log, - le16_to_cpu(payload->handles[i])); + le16_to_cpu(payload->handles[i-1])); if (i == max_handles) { payload->nr_recs = i;
The dev_dbg info for Clear Event Records mailbox command would report the handle of the next record to clear not the current one. This was because the index 'i' had incremented before printing the current handle value. Signed-off-by: Yuquan Wang <wangyuquan1236@phytium.com.cn> --- drivers/cxl/core/mbox.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)