All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] soc: fsl: various fixes
@ 2021-10-18 15:10 Ioana Ciornei
  2021-10-18 15:10 ` [PATCH 1/5] soc: fsl: dpio: use an explicit NULL instead of 0 Ioana Ciornei
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Ioana Ciornei @ 2021-10-18 15:10 UTC (permalink / raw)
  To: leoyang.li; +Cc: youri.querry_1, linux-kernel, Ioana Ciornei

This patch set has various unrelated fixes in the dpio driver and the
dpaa2-console one.
Patches were applied on latest linux-next and formatted from there.

Diana Craciun (1):
  soc: fsl: dpio: fix qbman alignment error in the virtualization
    context

Ioana Ciornei (2):
  soc: fsl: dpio: use an explicit NULL instead of 0
  soc: fsl: dpio: fix kernel-doc warnings

Robert-Ionut Alexa (1):
  soc: fsl: dpaa2-console: free buffer before returning from
    dpaa2_console_read

Youri Querry (1):
  soc: fsl: dpio: rename the enqueue descriptor variable

 drivers/soc/fsl/dpaa2-console.c     |  1 +
 drivers/soc/fsl/dpio/dpio-service.c | 42 ++++++++--------
 drivers/soc/fsl/dpio/qbman-portal.c | 76 ++++++++++++++---------------
 drivers/soc/fsl/dpio/qbman-portal.h | 39 +++++++++------
 4 files changed, 85 insertions(+), 73 deletions(-)

-- 
2.33.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/5] soc: fsl: dpio: use an explicit NULL instead of 0
  2021-10-18 15:10 [PATCH 0/5] soc: fsl: various fixes Ioana Ciornei
@ 2021-10-18 15:10 ` Ioana Ciornei
  2021-10-18 15:10 ` [PATCH 2/5] soc: fsl: dpio: rename the enqueue descriptor variable Ioana Ciornei
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Ioana Ciornei @ 2021-10-18 15:10 UTC (permalink / raw)
  To: leoyang.li; +Cc: youri.querry_1, linux-kernel, Ioana Ciornei

Use an explicit NULL pointer when calling qbman_swp_enqueue_multiple()
instead of a plain integer. Without this fix, we get the following
compile time error.

drivers/soc/fsl/dpio/dpio-service.c:466:60: warning: Using plain integer as NULL pointer

Fixes: 9d98809711ae ("soc: fsl: dpio: Adding QMAN multiple enqueue interface")
Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 drivers/soc/fsl/dpio/dpio-service.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/soc/fsl/dpio/dpio-service.c b/drivers/soc/fsl/dpio/dpio-service.c
index 3fd0d0840287..db0d3910fee4 100644
--- a/drivers/soc/fsl/dpio/dpio-service.c
+++ b/drivers/soc/fsl/dpio/dpio-service.c
@@ -500,7 +500,7 @@ int dpaa2_io_service_enqueue_multiple_fq(struct dpaa2_io *d,
 	qbman_eq_desc_set_no_orp(&ed, 0);
 	qbman_eq_desc_set_fq(&ed, fqid);
 
-	return qbman_swp_enqueue_multiple(d->swp, &ed, fd, 0, nb);
+	return qbman_swp_enqueue_multiple(d->swp, &ed, fd, NULL, nb);
 }
 EXPORT_SYMBOL(dpaa2_io_service_enqueue_multiple_fq);
 
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/5] soc: fsl: dpio: rename the enqueue descriptor variable
  2021-10-18 15:10 [PATCH 0/5] soc: fsl: various fixes Ioana Ciornei
  2021-10-18 15:10 ` [PATCH 1/5] soc: fsl: dpio: use an explicit NULL instead of 0 Ioana Ciornei
@ 2021-10-18 15:10 ` Ioana Ciornei
  2021-10-18 15:10 ` [PATCH 3/5] soc: fsl: dpio: fix kernel-doc warnings Ioana Ciornei
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Ioana Ciornei @ 2021-10-18 15:10 UTC (permalink / raw)
  To: leoyang.li; +Cc: youri.querry_1, linux-kernel, Ioana Ciornei

From: Youri Querry <youri.querry_1@nxp.com>

The struct qbman_eq_desc 'd' variable declaration is covering one of the
function parameters. This has no functional impact since this function
parameter was not used after the new declaration.
Even so, rename the variable so that we make the code more readable.

Fixes: 3b2abda7d28c ("soc: fsl: dpio: Replace QMAN array mode with ring mode enqueue")
Signed-off-by: Youri Querry <youri.querry_1@nxp.com>
Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 drivers/soc/fsl/dpio/qbman-portal.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/soc/fsl/dpio/qbman-portal.c b/drivers/soc/fsl/dpio/qbman-portal.c
index d3c58df6240d..eeefd1f19f0c 100644
--- a/drivers/soc/fsl/dpio/qbman-portal.c
+++ b/drivers/soc/fsl/dpio/qbman-portal.c
@@ -693,9 +693,9 @@ int qbman_swp_enqueue_multiple_direct(struct qbman_swp *s,
 		p = (s->addr_cena + QBMAN_CENA_SWP_EQCR(eqcr_pi & half_mask));
 		p[0] = cl[0] | s->eqcr.pi_vb;
 		if (flags && (flags[i] & QBMAN_ENQUEUE_FLAG_DCA)) {
-			struct qbman_eq_desc *d = (struct qbman_eq_desc *)p;
+			struct qbman_eq_desc *eq_desc = (struct qbman_eq_desc *)p;
 
-			d->dca = (1 << QB_ENQUEUE_CMD_DCA_EN_SHIFT) |
+			eq_desc->dca = (1 << QB_ENQUEUE_CMD_DCA_EN_SHIFT) |
 				((flags[i]) & QBMAN_EQCR_DCA_IDXMASK);
 		}
 		eqcr_pi++;
@@ -775,9 +775,9 @@ int qbman_swp_enqueue_multiple_mem_back(struct qbman_swp *s,
 		p = (s->addr_cena + QBMAN_CENA_SWP_EQCR(eqcr_pi & half_mask));
 		p[0] = cl[0] | s->eqcr.pi_vb;
 		if (flags && (flags[i] & QBMAN_ENQUEUE_FLAG_DCA)) {
-			struct qbman_eq_desc *d = (struct qbman_eq_desc *)p;
+			struct qbman_eq_desc *eq_desc = (struct qbman_eq_desc *)p;
 
-			d->dca = (1 << QB_ENQUEUE_CMD_DCA_EN_SHIFT) |
+			eq_desc->dca = (1 << QB_ENQUEUE_CMD_DCA_EN_SHIFT) |
 				((flags[i]) & QBMAN_EQCR_DCA_IDXMASK);
 		}
 		eqcr_pi++;
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/5] soc: fsl: dpio: fix kernel-doc warnings
  2021-10-18 15:10 [PATCH 0/5] soc: fsl: various fixes Ioana Ciornei
  2021-10-18 15:10 ` [PATCH 1/5] soc: fsl: dpio: use an explicit NULL instead of 0 Ioana Ciornei
  2021-10-18 15:10 ` [PATCH 2/5] soc: fsl: dpio: rename the enqueue descriptor variable Ioana Ciornei
@ 2021-10-18 15:10 ` Ioana Ciornei
  2021-10-18 15:10 ` [PATCH 4/5] soc: fsl: dpio: fix qbman alignment error in the virtualization context Ioana Ciornei
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Ioana Ciornei @ 2021-10-18 15:10 UTC (permalink / raw)
  To: leoyang.li; +Cc: youri.querry_1, linux-kernel, Ioana Ciornei

Fix all the kernel-doc warnings in the dpio driver. These are not major
problems, but it's easier to spot problems in new code when we start
with a clean kernel-doc.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 drivers/soc/fsl/dpio/dpio-service.c | 40 +++++++++++++-------------
 drivers/soc/fsl/dpio/qbman-portal.c | 44 ++++++++++++++---------------
 drivers/soc/fsl/dpio/qbman-portal.h | 39 ++++++++++++++++---------
 3 files changed, 67 insertions(+), 56 deletions(-)

diff --git a/drivers/soc/fsl/dpio/dpio-service.c b/drivers/soc/fsl/dpio/dpio-service.c
index db0d3910fee4..b166beb78ea5 100644
--- a/drivers/soc/fsl/dpio/dpio-service.c
+++ b/drivers/soc/fsl/dpio/dpio-service.c
@@ -96,7 +96,7 @@ static inline struct dpaa2_io *service_select(struct dpaa2_io *d)
  * dpaa2_io_service_select() - return a dpaa2_io service affined to this cpu
  * @cpu: the cpu id
  *
- * Return the affine dpaa2_io service, or NULL if there is no service affined
+ * Return: the affine dpaa2_io service, or NULL if there is no service affined
  * to the specified cpu. If DPAA2_IO_ANY_CPU is used, return the next available
  * service.
  */
@@ -128,7 +128,7 @@ static void dpaa2_io_dim_work(struct work_struct *w)
  * Activates a "struct dpaa2_io" corresponding to the given config of an actual
  * DPIO object.
  *
- * Return a valid dpaa2_io object for success, or NULL for failure.
+ * Return: a valid dpaa2_io object for success, or NULL for failure.
  */
 struct dpaa2_io *dpaa2_io_create(const struct dpaa2_io_desc *desc,
 				 struct device *dev)
@@ -220,7 +220,7 @@ void dpaa2_io_down(struct dpaa2_io *d)
  *
  * @obj: the given DPIO object.
  *
- * Return IRQ_HANDLED for success or IRQ_NONE if there
+ * Return: IRQ_HANDLED for success or IRQ_NONE if there
  * were no pending interrupts.
  */
 irqreturn_t dpaa2_io_irq(struct dpaa2_io *obj)
@@ -266,7 +266,7 @@ irqreturn_t dpaa2_io_irq(struct dpaa2_io *obj)
  *
  * @d: the given DPIO object.
  *
- * Return the cpu associated with the DPIO object
+ * Return: the cpu associated with the DPIO object
  */
 int dpaa2_io_get_cpu(struct dpaa2_io *d)
 {
@@ -291,7 +291,7 @@ EXPORT_SYMBOL(dpaa2_io_get_cpu);
  *        in order for the object to be configured to produce the right
  *        notification fields to the DPIO service.
  *
- * Return 0 for success, or -ENODEV for failure.
+ * Return: 0 for success, or -ENODEV for failure.
  */
 int dpaa2_io_service_register(struct dpaa2_io *d,
 			      struct dpaa2_io_notification_ctx *ctx,
@@ -361,7 +361,7 @@ EXPORT_SYMBOL_GPL(dpaa2_io_service_deregister);
  * that source to allow it to produce another FQDAN/CDAN, that's what this
  * function achieves.
  *
- * Return 0 for success.
+ * Return: 0 for success.
  */
 int dpaa2_io_service_rearm(struct dpaa2_io *d,
 			   struct dpaa2_io_notification_ctx *ctx)
@@ -390,7 +390,7 @@ EXPORT_SYMBOL_GPL(dpaa2_io_service_rearm);
  * @fqid: the given frame queue id.
  * @s: the dpaa2_io_store object for the result.
  *
- * Return 0 for success, or error code for failure.
+ * Return: 0 for success, or error code for failure.
  */
 int dpaa2_io_service_pull_fq(struct dpaa2_io *d, u32 fqid,
 			     struct dpaa2_io_store *s)
@@ -421,7 +421,7 @@ EXPORT_SYMBOL(dpaa2_io_service_pull_fq);
  * @channelid: the given channel id.
  * @s: the dpaa2_io_store object for the result.
  *
- * Return 0 for success, or error code for failure.
+ * Return: 0 for success, or error code for failure.
  */
 int dpaa2_io_service_pull_channel(struct dpaa2_io *d, u32 channelid,
 				  struct dpaa2_io_store *s)
@@ -453,7 +453,7 @@ EXPORT_SYMBOL_GPL(dpaa2_io_service_pull_channel);
  * @fqid: the given frame queue id.
  * @fd: the frame descriptor which is enqueued.
  *
- * Return 0 for successful enqueue, -EBUSY if the enqueue ring is not ready,
+ * Return: 0 for successful enqueue, -EBUSY if the enqueue ring is not ready,
  * or -ENODEV if there is no dpio service.
  */
 int dpaa2_io_service_enqueue_fq(struct dpaa2_io *d,
@@ -482,7 +482,7 @@ EXPORT_SYMBOL(dpaa2_io_service_enqueue_fq);
  * @fd: the frame descriptor which is enqueued.
  * @nb: number of frames to be enqueud
  *
- * Return 0 for successful enqueue, -EBUSY if the enqueue ring is not ready,
+ * Return: 0 for successful enqueue, -EBUSY if the enqueue ring is not ready,
  * or -ENODEV if there is no dpio service.
  */
 int dpaa2_io_service_enqueue_multiple_fq(struct dpaa2_io *d,
@@ -512,7 +512,7 @@ EXPORT_SYMBOL(dpaa2_io_service_enqueue_multiple_fq);
  * @fd: the frame descriptor which is enqueued.
  * @nb: number of frames to be enqueud
  *
- * Return 0 for successful enqueue, -EBUSY if the enqueue ring is not ready,
+ * Return: 0 for successful enqueue, -EBUSY if the enqueue ring is not ready,
  * or -ENODEV if there is no dpio service.
  */
 int dpaa2_io_service_enqueue_multiple_desc_fq(struct dpaa2_io *d,
@@ -554,7 +554,7 @@ EXPORT_SYMBOL(dpaa2_io_service_enqueue_multiple_desc_fq);
  * @qdbin: the given queuing destination bin.
  * @fd: the frame descriptor which is enqueued.
  *
- * Return 0 for successful enqueue, or -EBUSY if the enqueue ring is not ready,
+ * Return: 0 for successful enqueue, or -EBUSY if the enqueue ring is not ready,
  * or -ENODEV if there is no dpio service.
  */
 int dpaa2_io_service_enqueue_qd(struct dpaa2_io *d,
@@ -582,7 +582,7 @@ EXPORT_SYMBOL_GPL(dpaa2_io_service_enqueue_qd);
  * @buffers: the buffers to be released.
  * @num_buffers: the number of the buffers to be released.
  *
- * Return 0 for success, and negative error code for failure.
+ * Return: 0 for success, and negative error code for failure.
  */
 int dpaa2_io_service_release(struct dpaa2_io *d,
 			     u16 bpid,
@@ -609,7 +609,7 @@ EXPORT_SYMBOL_GPL(dpaa2_io_service_release);
  * @buffers: the buffer addresses for acquired buffers.
  * @num_buffers: the expected number of the buffers to acquire.
  *
- * Return a negative error code if the command failed, otherwise it returns
+ * Return: a negative error code if the command failed, otherwise it returns
  * the number of buffers acquired, which may be less than the number requested.
  * Eg. if the buffer pool is empty, this will return zero.
  */
@@ -646,7 +646,7 @@ EXPORT_SYMBOL_GPL(dpaa2_io_service_acquire);
  * The size of the storage is "max_frames*sizeof(struct dpaa2_dq)".
  * The 'dpaa2_io_store' returned is a DPIO service managed object.
  *
- * Return pointer to dpaa2_io_store struct for successfully created storage
+ * Return: pointer to dpaa2_io_store struct for successfully created storage
  * memory, or NULL on error.
  */
 struct dpaa2_io_store *dpaa2_io_store_create(unsigned int max_frames,
@@ -716,7 +716,7 @@ EXPORT_SYMBOL_GPL(dpaa2_io_store_destroy);
  * the caller to always check for this. As such, "is_last" can be used to
  * differentiate between "end-of-empty-dequeue" and "still-waiting".
  *
- * Return dequeue result for a valid dequeue result, or NULL for empty dequeue.
+ * Return: dequeue result for a valid dequeue result, or NULL for empty dequeue.
  */
 struct dpaa2_dq *dpaa2_io_store_next(struct dpaa2_io_store *s, int *is_last)
 {
@@ -760,7 +760,7 @@ EXPORT_SYMBOL_GPL(dpaa2_io_store_next);
  * Knowing the FQ count at run-time can be useful in debugging situations.
  * The instantaneous frame- and byte-count are hereby returned.
  *
- * Return 0 for a successful query, and negative error code if query fails.
+ * Return: 0 for a successful query, and negative error code if query fails.
  */
 int dpaa2_io_query_fq_count(struct dpaa2_io *d, u32 fqid,
 			    u32 *fcnt, u32 *bcnt)
@@ -794,7 +794,7 @@ EXPORT_SYMBOL_GPL(dpaa2_io_query_fq_count);
  * @bpid: the index of buffer pool to be queried.
  * @num: the queried number of buffers in the buffer pool.
  *
- * Return 0 for a successful query, and negative error code if query fails.
+ * Return: 0 for a successful query, and negative error code if query fails.
  */
 int dpaa2_io_query_bp_count(struct dpaa2_io *d, u16 bpid, u32 *num)
 {
@@ -823,7 +823,7 @@ EXPORT_SYMBOL_GPL(dpaa2_io_query_bp_count);
  * @d: the given DPIO object
  * @irq_holdoff: interrupt holdoff (timeout) period in us
  *
- * Return 0 for success, or negative error code on error.
+ * Return: 0 for success, or negative error code on error.
  */
 int dpaa2_io_set_irq_coalescing(struct dpaa2_io *d, u32 irq_holdoff)
 {
@@ -863,7 +863,7 @@ EXPORT_SYMBOL(dpaa2_io_set_adaptive_coalescing);
  * dpaa2_io_get_adaptive_coalescing() - Query adaptive coalescing state
  * @d: the given DPIO object
  *
- * Return 1 when adaptive coalescing is enabled on the DPIO object and 0
+ * Return: 1 when adaptive coalescing is enabled on the DPIO object and 0
  * otherwise.
  */
 int dpaa2_io_get_adaptive_coalescing(struct dpaa2_io *d)
diff --git a/drivers/soc/fsl/dpio/qbman-portal.c b/drivers/soc/fsl/dpio/qbman-portal.c
index eeefd1f19f0c..3fd54611ed98 100644
--- a/drivers/soc/fsl/dpio/qbman-portal.c
+++ b/drivers/soc/fsl/dpio/qbman-portal.c
@@ -241,7 +241,7 @@ static inline u8 qm_cyc_diff(u8 ringsize, u8 first, u8 last)
  *                    QBMan portal descriptor.
  * @d: the given qbman swp descriptor
  *
- * Return qbman_swp portal for success, NULL if the object cannot
+ * Return: qbman_swp portal for success, NULL if the object cannot
  * be created.
  */
 struct qbman_swp *qbman_swp_init(const struct qbman_swp_desc *d)
@@ -374,10 +374,10 @@ void qbman_swp_finish(struct qbman_swp *p)
 }
 
 /**
- * qbman_swp_interrupt_read_status()
+ * qbman_swp_interrupt_read_status() - Read interrupt status for swp
  * @p: the given software portal
  *
- * Return the value in the SWP_ISR register.
+ * Return: the value in the SWP_ISR register.
  */
 u32 qbman_swp_interrupt_read_status(struct qbman_swp *p)
 {
@@ -385,7 +385,7 @@ u32 qbman_swp_interrupt_read_status(struct qbman_swp *p)
 }
 
 /**
- * qbman_swp_interrupt_clear_status()
+ * qbman_swp_interrupt_clear_status() - Clear interrupt status for swp
  * @p: the given software portal
  * @mask: The mask to clear in SWP_ISR register
  */
@@ -398,7 +398,7 @@ void qbman_swp_interrupt_clear_status(struct qbman_swp *p, u32 mask)
  * qbman_swp_interrupt_get_trigger() - read interrupt enable register
  * @p: the given software portal
  *
- * Return the value in the SWP_IER register.
+ * Return: the value in the SWP_IER register.
  */
 u32 qbman_swp_interrupt_get_trigger(struct qbman_swp *p)
 {
@@ -419,7 +419,7 @@ void qbman_swp_interrupt_set_trigger(struct qbman_swp *p, u32 mask)
  * qbman_swp_interrupt_get_inhibit() - read interrupt mask register
  * @p: the given software portal object
  *
- * Return the value in the SWP_IIR register.
+ * Return: the value in the SWP_IIR register.
  */
 int qbman_swp_interrupt_get_inhibit(struct qbman_swp *p)
 {
@@ -442,7 +442,7 @@ void qbman_swp_interrupt_set_inhibit(struct qbman_swp *p, int inhibit)
  */
 
 /*
- * Returns a pointer to where the caller should fill in their management command
+ * Return: a pointer to where the caller should fill in their management command
  * (caller should ignore the verb byte)
  */
 void *qbman_swp_mc_start(struct qbman_swp *p)
@@ -587,7 +587,7 @@ void qbman_eq_desc_set_qd(struct qbman_eq_desc *d, u32 qdid,
  * Please note that 'fd' should only be NULL if the "action" of the
  * descriptor is "orp_hole" or "orp_nesn".
  *
- * Return 0 for successful enqueue, -EBUSY if the EQCR is not ready.
+ * Return: 0 for successful enqueue, -EBUSY if the EQCR is not ready.
  */
 static
 int qbman_swp_enqueue_direct(struct qbman_swp *s,
@@ -613,7 +613,7 @@ int qbman_swp_enqueue_direct(struct qbman_swp *s,
  * Please note that 'fd' should only be NULL if the "action" of the
  * descriptor is "orp_hole" or "orp_nesn".
  *
- * Return 0 for successful enqueue, -EBUSY if the EQCR is not ready.
+ * Return: 0 for successful enqueue, -EBUSY if the EQCR is not ready.
  */
 static
 int qbman_swp_enqueue_mem_back(struct qbman_swp *s,
@@ -639,7 +639,7 @@ int qbman_swp_enqueue_mem_back(struct qbman_swp *s,
  * @flags: table pointer of QBMAN_ENQUEUE_FLAG_DCA flags, not used if NULL
  * @num_frames: number of fd to be enqueued
  *
- * Return the number of fd enqueued, or a negative error number.
+ * Return: the number of fd enqueued, or a negative error number.
  */
 static
 int qbman_swp_enqueue_multiple_direct(struct qbman_swp *s,
@@ -722,7 +722,7 @@ int qbman_swp_enqueue_multiple_direct(struct qbman_swp *s,
  * @flags: table pointer of QBMAN_ENQUEUE_FLAG_DCA flags, not used if NULL
  * @num_frames: number of fd to be enqueued
  *
- * Return the number of fd enqueued, or a negative error number.
+ * Return: the number of fd enqueued, or a negative error number.
  */
 static
 int qbman_swp_enqueue_multiple_mem_back(struct qbman_swp *s,
@@ -803,7 +803,7 @@ int qbman_swp_enqueue_multiple_mem_back(struct qbman_swp *s,
  * @fd: table pointer of frame descriptor table to be enqueued
  * @num_frames: number of fd to be enqueued
  *
- * Return the number of fd enqueued, or a negative error number.
+ * Return: the number of fd enqueued, or a negative error number.
  */
 static
 int qbman_swp_enqueue_multiple_desc_direct(struct qbman_swp *s,
@@ -873,7 +873,7 @@ int qbman_swp_enqueue_multiple_desc_direct(struct qbman_swp *s,
  * @fd: table pointer of frame descriptor table to be enqueued
  * @num_frames: number of fd to be enqueued
  *
- * Return the number of fd enqueued, or a negative error number.
+ * Return: the number of fd enqueued, or a negative error number.
  */
 static
 int qbman_swp_enqueue_multiple_desc_mem_back(struct qbman_swp *s,
@@ -1096,7 +1096,7 @@ void qbman_pull_desc_set_channel(struct qbman_pull_desc *d, u32 chid,
  * @d: the software portal descriptor which has been configured with
  *     the set of qbman_pull_desc_set_*() calls
  *
- * Return 0 for success, and -EBUSY if the software portal is not ready
+ * Return: 0 for success, and -EBUSY if the software portal is not ready
  * to do pull dequeue.
  */
 static
@@ -1132,7 +1132,7 @@ int qbman_swp_pull_direct(struct qbman_swp *s, struct qbman_pull_desc *d)
  * @d: the software portal descriptor which has been configured with
  *     the set of qbman_pull_desc_set_*() calls
  *
- * Return 0 for success, and -EBUSY if the software portal is not ready
+ * Return: 0 for success, and -EBUSY if the software portal is not ready
  * to do pull dequeue.
  */
 static
@@ -1170,7 +1170,7 @@ int qbman_swp_pull_mem_back(struct qbman_swp *s, struct qbman_pull_desc *d)
  * qbman_swp_dqrr_next_direct() - Get an valid DQRR entry
  * @s: the software portal object
  *
- * Return NULL if there are no unconsumed DQRR entries. Return a DQRR entry
+ * Return: NULL if there are no unconsumed DQRR entries. Return a DQRR entry
  * only once, so repeated calls can return a sequence of DQRR entries, without
  * requiring they be consumed immediately or in any particular order.
  */
@@ -1262,7 +1262,7 @@ const struct dpaa2_dq *qbman_swp_dqrr_next_direct(struct qbman_swp *s)
  * qbman_swp_dqrr_next_mem_back() - Get an valid DQRR entry
  * @s: the software portal object
  *
- * Return NULL if there are no unconsumed DQRR entries. Return a DQRR entry
+ * Return: NULL if there are no unconsumed DQRR entries. Return a DQRR entry
  * only once, so repeated calls can return a sequence of DQRR entries, without
  * requiring they be consumed immediately or in any particular order.
  */
@@ -1367,7 +1367,7 @@ void qbman_swp_dqrr_consume(struct qbman_swp *s, const struct dpaa2_dq *dq)
  * @s: the software portal object
  * @dq: the dequeue result read from the memory
  *
- * Return 1 for getting a valid dequeue result, or 0 for not getting a valid
+ * Return: 1 for getting a valid dequeue result, or 0 for not getting a valid
  * dequeue result.
  *
  * Only used for user-provided storage of dequeue results, not DQRR. For
@@ -1449,7 +1449,7 @@ void qbman_release_desc_set_rcdi(struct qbman_release_desc *d, int enable)
  * @buffers:     a pointer pointing to the buffer address to be released
  * @num_buffers: number of buffers to be released,  must be less than 8
  *
- * Return 0 for success, -EBUSY if the release command ring is not ready.
+ * Return: 0 for success, -EBUSY if the release command ring is not ready.
  */
 int qbman_swp_release_direct(struct qbman_swp *s,
 			     const struct qbman_release_desc *d,
@@ -1491,7 +1491,7 @@ int qbman_swp_release_direct(struct qbman_swp *s,
  * @buffers:     a pointer pointing to the buffer address to be released
  * @num_buffers: number of buffers to be released,  must be less than 8
  *
- * Return 0 for success, -EBUSY if the release command ring is not ready.
+ * Return: 0 for success, -EBUSY if the release command ring is not ready.
  */
 int qbman_swp_release_mem_back(struct qbman_swp *s,
 			       const struct qbman_release_desc *d,
@@ -1548,7 +1548,7 @@ struct qbman_acquire_rslt {
  * @buffers:     a pointer pointing to the acquired buffer addresses
  * @num_buffers: number of buffers to be acquired, must be less than 8
  *
- * Return 0 for success, or negative error code if the acquire command
+ * Return: 0 for success, or negative error code if the acquire command
  * fails.
  */
 int qbman_swp_acquire(struct qbman_swp *s, u16 bpid, u64 *buffers,
@@ -1808,7 +1808,7 @@ u32 qbman_bp_info_num_free_bufs(struct qbman_bp_query_rslt *a)
  * @irq_threshold: interrupt threshold
  * @irq_holdoff: interrupt holdoff (timeout) period in us
  *
- * Return 0 for success, or negative error code on error.
+ * Return: 0 for success, or negative error code on error.
  */
 int qbman_swp_set_irq_coalescing(struct qbman_swp *p, u32 irq_threshold,
 				 u32 irq_holdoff)
diff --git a/drivers/soc/fsl/dpio/qbman-portal.h b/drivers/soc/fsl/dpio/qbman-portal.h
index b23883dd2725..58aac46cc735 100644
--- a/drivers/soc/fsl/dpio/qbman-portal.h
+++ b/drivers/soc/fsl/dpio/qbman-portal.h
@@ -250,7 +250,7 @@ void *qbman_swp_mc_result(struct qbman_swp *p);
  * @d:  the enqueue descriptor
  * @fd: the frame descriptor to be enqueued
  *
- * Return 0 for successful enqueue, -EBUSY if the EQCR is not ready.
+ * Return: 0 for successful enqueue, -EBUSY if the EQCR is not ready.
  */
 static inline int
 qbman_swp_enqueue(struct qbman_swp *s, const struct qbman_eq_desc *d,
@@ -268,7 +268,7 @@ qbman_swp_enqueue(struct qbman_swp *s, const struct qbman_eq_desc *d,
  * @flags: table pointer of QBMAN_ENQUEUE_FLAG_DCA flags, not used if NULL
  * @num_frames: number of fd to be enqueued
  *
- * Return the number of fd enqueued, or a negative error number.
+ * Return: the number of fd enqueued, or a negative error number.
  */
 static inline int
 qbman_swp_enqueue_multiple(struct qbman_swp *s,
@@ -288,7 +288,7 @@ qbman_swp_enqueue_multiple(struct qbman_swp *s,
  * @fd: table pointer of frame descriptor table to be enqueued
  * @num_frames: number of fd to be enqueued
  *
- * Return the number of fd enqueued, or a negative error number.
+ * Return: the number of fd enqueued, or a negative error number.
  */
 static inline int
 qbman_swp_enqueue_multiple_desc(struct qbman_swp *s,
@@ -304,6 +304,7 @@ qbman_swp_enqueue_multiple_desc(struct qbman_swp *s,
  * @dq: the dequeue result to be checked
  *
  * DQRR entries may contain non-dequeue results, ie. notifications
+ * Return: returns 1 if result is DQ, 0 otherwise
  */
 static inline int qbman_result_is_DQ(const struct dpaa2_dq *dq)
 {
@@ -314,6 +315,7 @@ static inline int qbman_result_is_DQ(const struct dpaa2_dq *dq)
  * qbman_result_is_SCN() - Check the dequeue result is notification or not
  * @dq: the dequeue result to be checked
  *
+ * Return: returns 1 if result is DQ is notification, 0 otherwise
  */
 static inline int qbman_result_is_SCN(const struct dpaa2_dq *dq)
 {
@@ -370,6 +372,9 @@ static inline int qbman_result_is_FQPN(const struct dpaa2_dq *dq)
 
 /**
  * qbman_result_SCN_state() - Get the state field in State-change notification
+ * @scn: state change notification
+ *
+ * Return: the state field from scn
  */
 static inline u8 qbman_result_SCN_state(const struct dpaa2_dq *scn)
 {
@@ -380,6 +385,9 @@ static inline u8 qbman_result_SCN_state(const struct dpaa2_dq *scn)
 
 /**
  * qbman_result_SCN_rid() - Get the resource id in State-change notification
+ * @scn: state change notification
+ *
+ * Return: the resource id from scn
  */
 static inline u32 qbman_result_SCN_rid(const struct dpaa2_dq *scn)
 {
@@ -388,6 +396,9 @@ static inline u32 qbman_result_SCN_rid(const struct dpaa2_dq *scn)
 
 /**
  * qbman_result_SCN_ctx() - Get the context data in State-change notification
+ * @scn: state change notification
+ *
+ * Return: the context data field from scn
  */
 static inline u64 qbman_result_SCN_ctx(const struct dpaa2_dq *scn)
 {
@@ -402,7 +413,7 @@ static inline u64 qbman_result_SCN_ctx(const struct dpaa2_dq *scn)
  * There are a couple of different ways that a FQ can end up parked state,
  * This schedules it.
  *
- * Return 0 for success, or negative error code for failure.
+ * Return: 0 for success, or negative error code for failure.
  */
 static inline int qbman_swp_fq_schedule(struct qbman_swp *s, u32 fqid)
 {
@@ -420,7 +431,7 @@ static inline int qbman_swp_fq_schedule(struct qbman_swp *s, u32 fqid)
  * empty at the time this happens, the resulting dq_entry will have no FD.
  * (qbman_result_DQ_fd() will return NULL.)
  *
- * Return 0 for success, or negative error code for failure.
+ * Return: 0 for success, or negative error code for failure.
  */
 static inline int qbman_swp_fq_force(struct qbman_swp *s, u32 fqid)
 {
@@ -434,7 +445,7 @@ static inline int qbman_swp_fq_force(struct qbman_swp *s, u32 fqid)
  *
  * This setting doesn't affect enqueues to the FQ, just dequeues.
  *
- * Return 0 for success, or negative error code for failure.
+ * Return: 0 for success, or negative error code for failure.
  */
 static inline int qbman_swp_fq_xon(struct qbman_swp *s, u32 fqid)
 {
@@ -454,7 +465,7 @@ static inline int qbman_swp_fq_xon(struct qbman_swp *s, u32 fqid)
  * that FQ for dequeuing, then the resulting dq_entry will have no FD.
  * (qbman_result_DQ_fd() will return NULL.)
  *
- * Return 0 for success, or negative error code for failure.
+ * Return: 0 for success, or negative error code for failure.
  */
 static inline int qbman_swp_fq_xoff(struct qbman_swp *s, u32 fqid)
 {
@@ -480,7 +491,7 @@ static inline int qbman_swp_fq_xoff(struct qbman_swp *s, u32 fqid)
  * @channelid: the channel index
  * @ctx:       the context to be set in CDAN
  *
- * Return 0 for success, or negative error code for failure.
+ * Return: 0 for success, or negative error code for failure.
  */
 static inline int qbman_swp_CDAN_set_context(struct qbman_swp *s, u16 channelid,
 					     u64 ctx)
@@ -495,7 +506,7 @@ static inline int qbman_swp_CDAN_set_context(struct qbman_swp *s, u16 channelid,
  * @s:         the software portal object
  * @channelid: the index of the channel to generate CDAN
  *
- * Return 0 for success, or negative error code for failure.
+ * Return: 0 for success, or negative error code for failure.
  */
 static inline int qbman_swp_CDAN_enable(struct qbman_swp *s, u16 channelid)
 {
@@ -509,7 +520,7 @@ static inline int qbman_swp_CDAN_enable(struct qbman_swp *s, u16 channelid)
  * @s:         the software portal object
  * @channelid: the index of the channel to generate CDAN
  *
- * Return 0 for success, or negative error code for failure.
+ * Return: 0 for success, or negative error code for failure.
  */
 static inline int qbman_swp_CDAN_disable(struct qbman_swp *s, u16 channelid)
 {
@@ -524,7 +535,7 @@ static inline int qbman_swp_CDAN_disable(struct qbman_swp *s, u16 channelid)
  * @channelid: the index of the channel to generate CDAN
  * @ctx:i      the context set in CDAN
  *
- * Return 0 for success, or negative error code for failure.
+ * Return: 0 for success, or negative error code for failure.
  */
 static inline int qbman_swp_CDAN_set_context_enable(struct qbman_swp *s,
 						    u16 channelid,
@@ -617,7 +628,7 @@ u32 qbman_bp_info_num_free_bufs(struct qbman_bp_query_rslt *a);
  * @buffers:     a pointer pointing to the buffer address to be released
  * @num_buffers: number of buffers to be released,  must be less than 8
  *
- * Return 0 for success, -EBUSY if the release command ring is not ready.
+ * Return: 0 for success, -EBUSY if the release command ring is not ready.
  */
 static inline int qbman_swp_release(struct qbman_swp *s,
 				    const struct qbman_release_desc *d,
@@ -633,7 +644,7 @@ static inline int qbman_swp_release(struct qbman_swp *s,
  * @d: the software portal descriptor which has been configured with
  *     the set of qbman_pull_desc_set_*() calls
  *
- * Return 0 for success, and -EBUSY if the software portal is not ready
+ * Return: 0 for success, and -EBUSY if the software portal is not ready
  * to do pull dequeue.
  */
 static inline int qbman_swp_pull(struct qbman_swp *s,
@@ -646,7 +657,7 @@ static inline int qbman_swp_pull(struct qbman_swp *s,
  * qbman_swp_dqrr_next() - Get an valid DQRR entry
  * @s: the software portal object
  *
- * Return NULL if there are no unconsumed DQRR entries. Return a DQRR entry
+ * Return: NULL if there are no unconsumed DQRR entries. Return a DQRR entry
  * only once, so repeated calls can return a sequence of DQRR entries, without
  * requiring they be consumed immediately or in any particular order.
  */
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/5] soc: fsl: dpio: fix qbman alignment error in the virtualization context
  2021-10-18 15:10 [PATCH 0/5] soc: fsl: various fixes Ioana Ciornei
                   ` (2 preceding siblings ...)
  2021-10-18 15:10 ` [PATCH 3/5] soc: fsl: dpio: fix kernel-doc warnings Ioana Ciornei
@ 2021-10-18 15:10 ` Ioana Ciornei
  2021-10-26 23:47   ` Leo Li
  2021-10-18 15:10 ` [PATCH 5/5] soc: fsl: dpaa2-console: free buffer before returning from dpaa2_console_read Ioana Ciornei
  2021-10-21 23:30 ` [PATCH 0/5] soc: fsl: various fixes Leo Li
  5 siblings, 1 reply; 9+ messages in thread
From: Ioana Ciornei @ 2021-10-18 15:10 UTC (permalink / raw)
  To: leoyang.li; +Cc: youri.querry_1, linux-kernel, Diana Craciun, Ioana Ciornei

From: Diana Craciun <diana.craciun@nxp.com>

When running as a guest, under KVM, the CENA region is mapped as device
memory, so uncacheable. When the memory is mapped as device memory, the
unaligned accesses are not allowed.  Memcpy is optimized to transfer 8
bytes at a time regardless of the start address and might cause
alignment issues.

Signed-off-by: Diana Craciun <diana.craciun@nxp.com>
Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 drivers/soc/fsl/dpio/qbman-portal.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/soc/fsl/dpio/qbman-portal.c b/drivers/soc/fsl/dpio/qbman-portal.c
index 3fd54611ed98..ef9cafd12534 100644
--- a/drivers/soc/fsl/dpio/qbman-portal.c
+++ b/drivers/soc/fsl/dpio/qbman-portal.c
@@ -679,9 +679,9 @@ int qbman_swp_enqueue_multiple_direct(struct qbman_swp *s,
 	for (i = 0; i < num_enqueued; i++) {
 		p = (s->addr_cena + QBMAN_CENA_SWP_EQCR(eqcr_pi & half_mask));
 		/* Skip copying the verb */
-		memcpy(&p[1], &cl[1], EQ_DESC_SIZE_WITHOUT_FD - 1);
-		memcpy(&p[EQ_DESC_SIZE_FD_START/sizeof(uint32_t)],
-		       &fd[i], sizeof(*fd));
+		memcpy_toio((__iomem void *)&p[1], &cl[1], EQ_DESC_SIZE_WITHOUT_FD - 1);
+		memcpy_toio((__iomem void *)&p[EQ_DESC_SIZE_FD_START / sizeof(uint32_t)],
+			    &fd[i], sizeof(*fd));
 		eqcr_pi++;
 	}
 
@@ -763,9 +763,9 @@ int qbman_swp_enqueue_multiple_mem_back(struct qbman_swp *s,
 	for (i = 0; i < num_enqueued; i++) {
 		p = (s->addr_cena + QBMAN_CENA_SWP_EQCR(eqcr_pi & half_mask));
 		/* Skip copying the verb */
-		memcpy(&p[1], &cl[1], EQ_DESC_SIZE_WITHOUT_FD - 1);
-		memcpy(&p[EQ_DESC_SIZE_FD_START/sizeof(uint32_t)],
-		       &fd[i], sizeof(*fd));
+		memcpy_toio((__iomem void *)&p[1], &cl[1], EQ_DESC_SIZE_WITHOUT_FD - 1);
+		memcpy_toio((__iomem void *)&p[EQ_DESC_SIZE_FD_START / sizeof(uint32_t)],
+			    &fd[i], sizeof(*fd));
 		eqcr_pi++;
 	}
 
@@ -837,9 +837,9 @@ int qbman_swp_enqueue_multiple_desc_direct(struct qbman_swp *s,
 		p = (s->addr_cena + QBMAN_CENA_SWP_EQCR(eqcr_pi & half_mask));
 		cl = (uint32_t *)(&d[i]);
 		/* Skip copying the verb */
-		memcpy(&p[1], &cl[1], EQ_DESC_SIZE_WITHOUT_FD - 1);
-		memcpy(&p[EQ_DESC_SIZE_FD_START/sizeof(uint32_t)],
-		       &fd[i], sizeof(*fd));
+		memcpy_toio((__iomem void *)&p[1], &cl[1], EQ_DESC_SIZE_WITHOUT_FD - 1);
+		memcpy_toio((__iomem void *)&p[EQ_DESC_SIZE_FD_START / sizeof(uint32_t)],
+			    &fd[i], sizeof(*fd));
 		eqcr_pi++;
 	}
 
@@ -907,9 +907,9 @@ int qbman_swp_enqueue_multiple_desc_mem_back(struct qbman_swp *s,
 		p = (s->addr_cena + QBMAN_CENA_SWP_EQCR(eqcr_pi & half_mask));
 		cl = (uint32_t *)(&d[i]);
 		/* Skip copying the verb */
-		memcpy(&p[1], &cl[1], EQ_DESC_SIZE_WITHOUT_FD - 1);
-		memcpy(&p[EQ_DESC_SIZE_FD_START/sizeof(uint32_t)],
-		       &fd[i], sizeof(*fd));
+		memcpy_toio((__iomem void *)&p[1], &cl[1], EQ_DESC_SIZE_WITHOUT_FD - 1);
+		memcpy_toio((__iomem void *)&p[EQ_DESC_SIZE_FD_START / sizeof(uint32_t)],
+			    &fd[i], sizeof(*fd));
 		eqcr_pi++;
 	}
 
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 5/5] soc: fsl: dpaa2-console: free buffer before returning from dpaa2_console_read
  2021-10-18 15:10 [PATCH 0/5] soc: fsl: various fixes Ioana Ciornei
                   ` (3 preceding siblings ...)
  2021-10-18 15:10 ` [PATCH 4/5] soc: fsl: dpio: fix qbman alignment error in the virtualization context Ioana Ciornei
@ 2021-10-18 15:10 ` Ioana Ciornei
  2021-10-21 23:30 ` [PATCH 0/5] soc: fsl: various fixes Leo Li
  5 siblings, 0 replies; 9+ messages in thread
From: Ioana Ciornei @ 2021-10-18 15:10 UTC (permalink / raw)
  To: leoyang.li
  Cc: youri.querry_1, linux-kernel, Robert-Ionut Alexa, Ioana Ciornei

From: Robert-Ionut Alexa <robert-ionut.alexa@nxp.com>

Free the kbuf buffer before returning from the dpaa2_console_read()
function. The variable no longer goes out of scope, leaking the storage
it points to.

Fixes: c93349d8c170 ("soc: fsl: add DPAA2 console support")
Signed-off-by: Robert-Ionut Alexa <robert-ionut.alexa@nxp.com>
Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 drivers/soc/fsl/dpaa2-console.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/soc/fsl/dpaa2-console.c b/drivers/soc/fsl/dpaa2-console.c
index 27243f706f37..53917410f2bd 100644
--- a/drivers/soc/fsl/dpaa2-console.c
+++ b/drivers/soc/fsl/dpaa2-console.c
@@ -231,6 +231,7 @@ static ssize_t dpaa2_console_read(struct file *fp, char __user *buf,
 	cd->cur_ptr += bytes;
 	written += bytes;
 
+	kfree(kbuf);
 	return written;
 
 err_free_buf:
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* RE: [PATCH 0/5] soc: fsl: various fixes
  2021-10-18 15:10 [PATCH 0/5] soc: fsl: various fixes Ioana Ciornei
                   ` (4 preceding siblings ...)
  2021-10-18 15:10 ` [PATCH 5/5] soc: fsl: dpaa2-console: free buffer before returning from dpaa2_console_read Ioana Ciornei
@ 2021-10-21 23:30 ` Leo Li
  5 siblings, 0 replies; 9+ messages in thread
From: Leo Li @ 2021-10-21 23:30 UTC (permalink / raw)
  To: Ioana Ciornei; +Cc: Youri Querry, linux-kernel



> -----Original Message-----
> From: Ioana Ciornei <ioana.ciornei@nxp.com>
> Sent: Monday, October 18, 2021 10:10 AM
> To: Leo Li <leoyang.li@nxp.com>
> Cc: Youri Querry <youri.querry_1@nxp.com>; linux-kernel@vger.kernel.org;
> Ioana Ciornei <ioana.ciornei@nxp.com>
> Subject: [PATCH 0/5] soc: fsl: various fixes
> 
> This patch set has various unrelated fixes in the dpio driver and the
> dpaa2-console one.
> Patches were applied on latest linux-next and formatted from there.
> 
> Diana Craciun (1):
>   soc: fsl: dpio: fix qbman alignment error in the virtualization
>     context
> 
> Ioana Ciornei (2):
>   soc: fsl: dpio: use an explicit NULL instead of 0
>   soc: fsl: dpio: fix kernel-doc warnings
> 
> Robert-Ionut Alexa (1):
>   soc: fsl: dpaa2-console: free buffer before returning from
>     dpaa2_console_read
> 
> Youri Querry (1):
>   soc: fsl: dpio: rename the enqueue descriptor variable

Patch 4-5 are applied for fix.

Patch 1-2 are applied for next.  Patch 3 cannot be applied cleanly, we probably can apply it after the changes in Linux-next get into the mainline.

Thanks.
> 
>  drivers/soc/fsl/dpaa2-console.c     |  1 +
>  drivers/soc/fsl/dpio/dpio-service.c | 42 ++++++++--------
>  drivers/soc/fsl/dpio/qbman-portal.c | 76 ++++++++++++++---------------
>  drivers/soc/fsl/dpio/qbman-portal.h | 39 +++++++++------
>  4 files changed, 85 insertions(+), 73 deletions(-)
> 
> --
> 2.33.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH 4/5] soc: fsl: dpio: fix qbman alignment error in the virtualization context
  2021-10-18 15:10 ` [PATCH 4/5] soc: fsl: dpio: fix qbman alignment error in the virtualization context Ioana Ciornei
@ 2021-10-26 23:47   ` Leo Li
  2021-10-28 11:32     ` Diana Madalina Craciun
  0 siblings, 1 reply; 9+ messages in thread
From: Leo Li @ 2021-10-26 23:47 UTC (permalink / raw)
  To: Ioana Ciornei, Diana Madalina Craciun
  Cc: Youri Querry, linux-kernel, Arnd Bergmann



> -----Original Message-----
> From: Ioana Ciornei <ioana.ciornei@nxp.com>
> Sent: Monday, October 18, 2021 10:11 AM
> To: Leo Li <leoyang.li@nxp.com>
> Cc: Youri Querry <youri.querry_1@nxp.com>; linux-kernel@vger.kernel.org;
> Diana Madalina Craciun <diana.craciun@nxp.com>; Ioana Ciornei
> <ioana.ciornei@nxp.com>
> Subject: [PATCH 4/5] soc: fsl: dpio: fix qbman alignment error in the
> virtualization context
> 
> From: Diana Craciun <diana.craciun@nxp.com>
> 
> When running as a guest, under KVM, the CENA region is mapped as device
> memory, so uncacheable. When the memory is mapped as device memory,
> the unaligned accesses are not allowed.  Memcpy is optimized to transfer 8
> bytes at a time regardless of the start address and might cause alignment
> issues.
> 
> Signed-off-by: Diana Craciun <diana.craciun@nxp.com>
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>

We get the following feedback from Arnd about this patch.  Could you respond to the comments?

"This patch looks very suspicious to me, I don't think it's generally safe to
use memcpy_toio() on a normal pointer, as the __iomem tokens may
be in a separate address range, even though this currently works
on arm64. Adding the  (__iomem void *) cast without a comment that
explains why it's added seems similarly wrong, and finally the
changeset text does not seem to match what the code does:

According to the text, the pointer is to a virtual address mapped as
"device memory" (i.e. PROT_DEVICE_nGnRE or PROT_DEVICE_nGnRnE),
but the code suggests it's actually write-combining normal
(PROT_NORMAL_NC)."

> ---
>  drivers/soc/fsl/dpio/qbman-portal.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/soc/fsl/dpio/qbman-portal.c
> b/drivers/soc/fsl/dpio/qbman-portal.c
> index 3fd54611ed98..ef9cafd12534 100644
> --- a/drivers/soc/fsl/dpio/qbman-portal.c
> +++ b/drivers/soc/fsl/dpio/qbman-portal.c
> @@ -679,9 +679,9 @@ int qbman_swp_enqueue_multiple_direct(struct
> qbman_swp *s,
>  	for (i = 0; i < num_enqueued; i++) {
>  		p = (s->addr_cena + QBMAN_CENA_SWP_EQCR(eqcr_pi &
> half_mask));
>  		/* Skip copying the verb */
> -		memcpy(&p[1], &cl[1], EQ_DESC_SIZE_WITHOUT_FD - 1);
> -		memcpy(&p[EQ_DESC_SIZE_FD_START/sizeof(uint32_t)],
> -		       &fd[i], sizeof(*fd));
> +		memcpy_toio((__iomem void *)&p[1], &cl[1],
> EQ_DESC_SIZE_WITHOUT_FD - 1);
> +		memcpy_toio((__iomem void
> *)&p[EQ_DESC_SIZE_FD_START / sizeof(uint32_t)],
> +			    &fd[i], sizeof(*fd));
>  		eqcr_pi++;
>  	}
> 
> @@ -763,9 +763,9 @@ int
> qbman_swp_enqueue_multiple_mem_back(struct qbman_swp *s,
>  	for (i = 0; i < num_enqueued; i++) {
>  		p = (s->addr_cena + QBMAN_CENA_SWP_EQCR(eqcr_pi &
> half_mask));
>  		/* Skip copying the verb */
> -		memcpy(&p[1], &cl[1], EQ_DESC_SIZE_WITHOUT_FD - 1);
> -		memcpy(&p[EQ_DESC_SIZE_FD_START/sizeof(uint32_t)],
> -		       &fd[i], sizeof(*fd));
> +		memcpy_toio((__iomem void *)&p[1], &cl[1],
> EQ_DESC_SIZE_WITHOUT_FD - 1);
> +		memcpy_toio((__iomem void
> *)&p[EQ_DESC_SIZE_FD_START / sizeof(uint32_t)],
> +			    &fd[i], sizeof(*fd));
>  		eqcr_pi++;
>  	}
> 
> @@ -837,9 +837,9 @@ int
> qbman_swp_enqueue_multiple_desc_direct(struct qbman_swp *s,
>  		p = (s->addr_cena + QBMAN_CENA_SWP_EQCR(eqcr_pi &
> half_mask));
>  		cl = (uint32_t *)(&d[i]);
>  		/* Skip copying the verb */
> -		memcpy(&p[1], &cl[1], EQ_DESC_SIZE_WITHOUT_FD - 1);
> -		memcpy(&p[EQ_DESC_SIZE_FD_START/sizeof(uint32_t)],
> -		       &fd[i], sizeof(*fd));
> +		memcpy_toio((__iomem void *)&p[1], &cl[1],
> EQ_DESC_SIZE_WITHOUT_FD - 1);
> +		memcpy_toio((__iomem void
> *)&p[EQ_DESC_SIZE_FD_START / sizeof(uint32_t)],
> +			    &fd[i], sizeof(*fd));
>  		eqcr_pi++;
>  	}
> 
> @@ -907,9 +907,9 @@ int
> qbman_swp_enqueue_multiple_desc_mem_back(struct qbman_swp *s,
>  		p = (s->addr_cena + QBMAN_CENA_SWP_EQCR(eqcr_pi &
> half_mask));
>  		cl = (uint32_t *)(&d[i]);
>  		/* Skip copying the verb */
> -		memcpy(&p[1], &cl[1], EQ_DESC_SIZE_WITHOUT_FD - 1);
> -		memcpy(&p[EQ_DESC_SIZE_FD_START/sizeof(uint32_t)],
> -		       &fd[i], sizeof(*fd));
> +		memcpy_toio((__iomem void *)&p[1], &cl[1],
> EQ_DESC_SIZE_WITHOUT_FD - 1);
> +		memcpy_toio((__iomem void
> *)&p[EQ_DESC_SIZE_FD_START / sizeof(uint32_t)],
> +			    &fd[i], sizeof(*fd));
>  		eqcr_pi++;
>  	}
> 
> --
> 2.33.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 4/5] soc: fsl: dpio: fix qbman alignment error in the virtualization context
  2021-10-26 23:47   ` Leo Li
@ 2021-10-28 11:32     ` Diana Madalina Craciun
  0 siblings, 0 replies; 9+ messages in thread
From: Diana Madalina Craciun @ 2021-10-28 11:32 UTC (permalink / raw)
  To: Leo Li, Ioana Ciornei; +Cc: Youri Querry, linux-kernel, Arnd Bergmann

Hi Leo,

Just drop this patch. I realize that the implementation is not correct. 
The problem should be resolved (if possible) elsewhere.

I will open another thread on the relevant lists regarding this problem.

Diana

On 10/27/2021 2:47 AM, Leo Li wrote:
>
>> -----Original Message-----
>> From: Ioana Ciornei <ioana.ciornei@nxp.com>
>> Sent: Monday, October 18, 2021 10:11 AM
>> To: Leo Li <leoyang.li@nxp.com>
>> Cc: Youri Querry <youri.querry_1@nxp.com>; linux-kernel@vger.kernel.org;
>> Diana Madalina Craciun <diana.craciun@nxp.com>; Ioana Ciornei
>> <ioana.ciornei@nxp.com>
>> Subject: [PATCH 4/5] soc: fsl: dpio: fix qbman alignment error in the
>> virtualization context
>>
>> From: Diana Craciun <diana.craciun@nxp.com>
>>
>> When running as a guest, under KVM, the CENA region is mapped as device
>> memory, so uncacheable. When the memory is mapped as device memory,
>> the unaligned accesses are not allowed.  Memcpy is optimized to transfer 8
>> bytes at a time regardless of the start address and might cause alignment
>> issues.
>>
>> Signed-off-by: Diana Craciun <diana.craciun@nxp.com>
>> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> We get the following feedback from Arnd about this patch.  Could you respond to the comments?
>
> "This patch looks very suspicious to me, I don't think it's generally safe to
> use memcpy_toio() on a normal pointer, as the __iomem tokens may
> be in a separate address range, even though this currently works
> on arm64. Adding the  (__iomem void *) cast without a comment that
> explains why it's added seems similarly wrong, and finally the
> changeset text does not seem to match what the code does:
>
> According to the text, the pointer is to a virtual address mapped as
> "device memory" (i.e. PROT_DEVICE_nGnRE or PROT_DEVICE_nGnRnE),
> but the code suggests it's actually write-combining normal
> (PROT_NORMAL_NC)."
>
>> ---
>>   drivers/soc/fsl/dpio/qbman-portal.c | 24 ++++++++++++------------
>>   1 file changed, 12 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/soc/fsl/dpio/qbman-portal.c
>> b/drivers/soc/fsl/dpio/qbman-portal.c
>> index 3fd54611ed98..ef9cafd12534 100644
>> --- a/drivers/soc/fsl/dpio/qbman-portal.c
>> +++ b/drivers/soc/fsl/dpio/qbman-portal.c
>> @@ -679,9 +679,9 @@ int qbman_swp_enqueue_multiple_direct(struct
>> qbman_swp *s,
>>   	for (i = 0; i < num_enqueued; i++) {
>>   		p = (s->addr_cena + QBMAN_CENA_SWP_EQCR(eqcr_pi &
>> half_mask));
>>   		/* Skip copying the verb */
>> -		memcpy(&p[1], &cl[1], EQ_DESC_SIZE_WITHOUT_FD - 1);
>> -		memcpy(&p[EQ_DESC_SIZE_FD_START/sizeof(uint32_t)],
>> -		       &fd[i], sizeof(*fd));
>> +		memcpy_toio((__iomem void *)&p[1], &cl[1],
>> EQ_DESC_SIZE_WITHOUT_FD - 1);
>> +		memcpy_toio((__iomem void
>> *)&p[EQ_DESC_SIZE_FD_START / sizeof(uint32_t)],
>> +			    &fd[i], sizeof(*fd));
>>   		eqcr_pi++;
>>   	}
>>
>> @@ -763,9 +763,9 @@ int
>> qbman_swp_enqueue_multiple_mem_back(struct qbman_swp *s,
>>   	for (i = 0; i < num_enqueued; i++) {
>>   		p = (s->addr_cena + QBMAN_CENA_SWP_EQCR(eqcr_pi &
>> half_mask));
>>   		/* Skip copying the verb */
>> -		memcpy(&p[1], &cl[1], EQ_DESC_SIZE_WITHOUT_FD - 1);
>> -		memcpy(&p[EQ_DESC_SIZE_FD_START/sizeof(uint32_t)],
>> -		       &fd[i], sizeof(*fd));
>> +		memcpy_toio((__iomem void *)&p[1], &cl[1],
>> EQ_DESC_SIZE_WITHOUT_FD - 1);
>> +		memcpy_toio((__iomem void
>> *)&p[EQ_DESC_SIZE_FD_START / sizeof(uint32_t)],
>> +			    &fd[i], sizeof(*fd));
>>   		eqcr_pi++;
>>   	}
>>
>> @@ -837,9 +837,9 @@ int
>> qbman_swp_enqueue_multiple_desc_direct(struct qbman_swp *s,
>>   		p = (s->addr_cena + QBMAN_CENA_SWP_EQCR(eqcr_pi &
>> half_mask));
>>   		cl = (uint32_t *)(&d[i]);
>>   		/* Skip copying the verb */
>> -		memcpy(&p[1], &cl[1], EQ_DESC_SIZE_WITHOUT_FD - 1);
>> -		memcpy(&p[EQ_DESC_SIZE_FD_START/sizeof(uint32_t)],
>> -		       &fd[i], sizeof(*fd));
>> +		memcpy_toio((__iomem void *)&p[1], &cl[1],
>> EQ_DESC_SIZE_WITHOUT_FD - 1);
>> +		memcpy_toio((__iomem void
>> *)&p[EQ_DESC_SIZE_FD_START / sizeof(uint32_t)],
>> +			    &fd[i], sizeof(*fd));
>>   		eqcr_pi++;
>>   	}
>>
>> @@ -907,9 +907,9 @@ int
>> qbman_swp_enqueue_multiple_desc_mem_back(struct qbman_swp *s,
>>   		p = (s->addr_cena + QBMAN_CENA_SWP_EQCR(eqcr_pi &
>> half_mask));
>>   		cl = (uint32_t *)(&d[i]);
>>   		/* Skip copying the verb */
>> -		memcpy(&p[1], &cl[1], EQ_DESC_SIZE_WITHOUT_FD - 1);
>> -		memcpy(&p[EQ_DESC_SIZE_FD_START/sizeof(uint32_t)],
>> -		       &fd[i], sizeof(*fd));
>> +		memcpy_toio((__iomem void *)&p[1], &cl[1],
>> EQ_DESC_SIZE_WITHOUT_FD - 1);
>> +		memcpy_toio((__iomem void
>> *)&p[EQ_DESC_SIZE_FD_START / sizeof(uint32_t)],
>> +			    &fd[i], sizeof(*fd));
>>   		eqcr_pi++;
>>   	}
>>
>> --
>> 2.33.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-10-28 11:32 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-18 15:10 [PATCH 0/5] soc: fsl: various fixes Ioana Ciornei
2021-10-18 15:10 ` [PATCH 1/5] soc: fsl: dpio: use an explicit NULL instead of 0 Ioana Ciornei
2021-10-18 15:10 ` [PATCH 2/5] soc: fsl: dpio: rename the enqueue descriptor variable Ioana Ciornei
2021-10-18 15:10 ` [PATCH 3/5] soc: fsl: dpio: fix kernel-doc warnings Ioana Ciornei
2021-10-18 15:10 ` [PATCH 4/5] soc: fsl: dpio: fix qbman alignment error in the virtualization context Ioana Ciornei
2021-10-26 23:47   ` Leo Li
2021-10-28 11:32     ` Diana Madalina Craciun
2021-10-18 15:10 ` [PATCH 5/5] soc: fsl: dpaa2-console: free buffer before returning from dpaa2_console_read Ioana Ciornei
2021-10-21 23:30 ` [PATCH 0/5] soc: fsl: various fixes Leo Li

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.