Discussion:
[PATCH v3 0/8] staging: fsl-mc: make the driver compile on other architectures
l***@nxp.com
2017-07-19 11:42:24 UTC
Permalink
From: Laurentiu Tudor <***@nxp.com>

Apart from a small change (first patch) which adds a missing comment,
this series make the bus driver compile on other architectures, as per
GregKH comment [1].
Compiled tested on:
- booke powerpc (corenet{32,64}_smp_defconfig) with this ppc patch [2]
- x86 (i386_defconfig, x86_64_defconfig, needs CONFIG_OF)
- arm64 (defconfig)

[1] https://www.spinics.net/lists/linux-driver-devel/msg100585.html
[2] https://patchwork.ozlabs.org/patch/789474/

version 3 changes
- add COMPILE_TEST instead of dropping ARCH_LAYERSCAPE so
that other arches menuconfig is not polluted with an
useless option (Arnd)

version 2 changes
- use writeq() / writeq_relaxed() / readq_relaxed() instead
of raw versions (Robin)
- use linux/io-64-nonatomic-hi-lo.h to make the driver compile
on 32-bit platforms (Robin)
- add extra LE <-> CPU so that standard device io may be used (Arnd)

Laurentiu Tudor (8):
staging: fsl-mc: add missing fsl_mc comment in struct msi_desc
staging: fsl-mc: use generic memory barriers
staging: fsl-mc: drop useless gic v3 related #include
staging: fsl-mc: fix compilation with non-generic msi domain ops
staging: fsl-mc: fix formating of phys_addr_t on 32 bits
staging: fsl-mc: don't use raw device io functions
staging: fsl-mc: make the driver compile on 32-bit
staging: fsl-mc: allow the driver compile multi-arch

drivers/staging/fsl-dpaa2/Kconfig | 2 +-
drivers/staging/fsl-mc/bus/Kconfig | 4 +--
drivers/staging/fsl-mc/bus/fsl-mc-msi.c | 5 ++-
.../staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c | 3 +-
drivers/staging/fsl-mc/bus/mc-io.c | 8 ++---
drivers/staging/fsl-mc/bus/mc-sys.c | 36 +++++++++++++---------
include/linux/msi.h | 1 +
7 files changed, 36 insertions(+), 23 deletions(-)
--
2.9.4
l***@nxp.com
2017-07-19 11:42:26 UTC
Permalink
From: Laurentiu Tudor <***@nxp.com>

No need to use arch-specific memory barriers; switch to using generic
ones. The rmb()s were useless so drop them.

Signed-off-by: Laurentiu Tudor <***@nxp.com>
Acked-by: Arnd Bergmann <***@arndb.de>
---
Notes:
-v3
-no changes
-v2
-no changes

drivers/staging/fsl-mc/bus/mc-sys.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/fsl-mc/bus/mc-sys.c b/drivers/staging/fsl-mc/bus/mc-sys.c
index a1704c3..012abd5 100644
--- a/drivers/staging/fsl-mc/bus/mc-sys.c
+++ b/drivers/staging/fsl-mc/bus/mc-sys.c
@@ -127,7 +127,8 @@ static inline void mc_write_command(struct mc_command __iomem *portal,
/* copy command parameters into the portal */
for (i = 0; i < MC_CMD_NUM_OF_PARAMS; i++)
__raw_writeq(cmd->params[i], &portal->params[i]);
- __iowmb();
+ /* ensure command params are committed before submitting it */
+ wmb();

/* submit the command by writing the header */
__raw_writeq(cmd->header, &portal->header);
@@ -150,9 +151,7 @@ static inline enum mc_cmd_status mc_read_response(struct mc_command __iomem *
enum mc_cmd_status status;

/* Copy command response header from MC portal: */
- __iormb();
resp->header = __raw_readq(&portal->header);
- __iormb();
status = mc_cmd_hdr_read_status(resp);
if (status != MC_CMD_STATUS_OK)
return status;
@@ -160,7 +159,6 @@ static inline enum mc_cmd_status mc_read_response(struct mc_command __iomem *
/* Copy command response data from MC portal: */
for (i = 0; i < MC_CMD_NUM_OF_PARAMS; i++)
resp->params[i] = __raw_readq(&portal->params[i]);
- __iormb();

return status;
}
--
2.9.4
l***@nxp.com
2017-07-19 11:42:28 UTC
Permalink
From: Laurentiu Tudor <***@nxp.com>

The bus driver relies on generic msi domain ops.
Fix compilation for architectures that don't provide it (e.g. x86_64).

Signed-off-by: Laurentiu Tudor <***@nxp.com>
Acked-by: Arnd Bergmann <***@arndb.de>
---
Notes:
-v3
-no changes
-v2
-no changes

drivers/staging/fsl-mc/bus/fsl-mc-msi.c | 4 ++++
drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c | 2 ++
2 files changed, 6 insertions(+)

diff --git a/drivers/staging/fsl-mc/bus/fsl-mc-msi.c b/drivers/staging/fsl-mc/bus/fsl-mc-msi.c
index ee6e3b7..18774ee 100644
--- a/drivers/staging/fsl-mc/bus/fsl-mc-msi.c
+++ b/drivers/staging/fsl-mc/bus/fsl-mc-msi.c
@@ -17,6 +17,7 @@
#include <linux/msi.h>
#include "fsl-mc-private.h"

+#ifdef GENERIC_MSI_DOMAIN_OPS
/*
* Generate a unique ID identifying the interrupt (only used within the MSI
* irqdomain. Combine the icid with the interrupt index.
@@ -38,6 +39,9 @@ static void fsl_mc_msi_set_desc(msi_alloc_info_t *arg,
arg->hwirq = fsl_mc_domain_calc_hwirq(to_fsl_mc_device(desc->dev),
desc);
}
+#else
+#define fsl_mc_msi_set_desc NULL
+#endif

static void fsl_mc_msi_update_dom_ops(struct msi_domain_info *info)
{
diff --git a/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c b/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
index e798ea4..cd73c58 100644
--- a/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
+++ b/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
@@ -45,7 +45,9 @@ static int its_fsl_mc_msi_prepare(struct irq_domain *msi_domain,
* NOTE: This device id corresponds to the IOMMU stream ID
* associated with the DPRC object (ICID).
*/
+#ifdef GENERIC_MSI_DOMAIN_OPS
info->scratchpad[0].ul = mc_bus_dev->icid;
+#endif
msi_info = msi_get_domain_info(msi_domain->parent);
return msi_info->ops->msi_prepare(msi_domain->parent, dev, nvec, info);
}
--
2.9.4
l***@nxp.com
2017-07-19 11:42:29 UTC
Permalink
From: Laurentiu Tudor <***@nxp.com>

Use correct format specifier for phys_addr_t variables (%pa) instead
of %llx. This fixes these warnings on 32 bit targets:
"format '%llx' expects argument of type 'long long unsigned int',
but argument 4 has type 'phys_addr_t' [-Wformat=]"

Signed-off-by: Laurentiu Tudor <***@nxp.com>
Acked-by: Arnd Bergmann <***@arndb.de>
---
Notes:
-v3
-no changes
-v2
-no changes

drivers/staging/fsl-mc/bus/mc-io.c | 8 ++++----
drivers/staging/fsl-mc/bus/mc-sys.c | 12 ++++++------
2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/fsl-mc/bus/mc-io.c b/drivers/staging/fsl-mc/bus/mc-io.c
index 35221a17..4e6f99a 100644
--- a/drivers/staging/fsl-mc/bus/mc-io.c
+++ b/drivers/staging/fsl-mc/bus/mc-io.c
@@ -129,8 +129,8 @@ int __must_check fsl_create_mc_io(struct device *dev,
"mc_portal");
if (!res) {
dev_err(dev,
- "devm_request_mem_region failed for MC portal %#llx\n",
- mc_portal_phys_addr);
+ "devm_request_mem_region failed for MC portal %pa\n",
+ &mc_portal_phys_addr);
return -EBUSY;
}

@@ -139,8 +139,8 @@ int __must_check fsl_create_mc_io(struct device *dev,
mc_portal_size);
if (!mc_portal_virt_addr) {
dev_err(dev,
- "devm_ioremap_nocache failed for MC portal %#llx\n",
- mc_portal_phys_addr);
+ "devm_ioremap_nocache failed for MC portal %pa\n",
+ &mc_portal_phys_addr);
return -ENXIO;
}

diff --git a/drivers/staging/fsl-mc/bus/mc-sys.c b/drivers/staging/fsl-mc/bus/mc-sys.c
index 012abd5..195d9f3 100644
--- a/drivers/staging/fsl-mc/bus/mc-sys.c
+++ b/drivers/staging/fsl-mc/bus/mc-sys.c
@@ -196,8 +196,8 @@ static int mc_polling_wait_preemptible(struct fsl_mc_io *mc_io,

if (time_after_eq(jiffies, jiffies_until_timeout)) {
dev_dbg(mc_io->dev,
- "MC command timed out (portal: %#llx, dprc handle: %#x, command: %#x)\n",
- mc_io->portal_phys_addr,
+ "MC command timed out (portal: %pa, dprc handle: %#x, command: %#x)\n",
+ &mc_io->portal_phys_addr,
(unsigned int)mc_cmd_hdr_read_token(cmd),
(unsigned int)mc_cmd_hdr_read_cmdid(cmd));

@@ -236,8 +236,8 @@ static int mc_polling_wait_atomic(struct fsl_mc_io *mc_io,
timeout_usecs -= MC_CMD_COMPLETION_POLLING_MAX_SLEEP_USECS;
if (timeout_usecs == 0) {
dev_dbg(mc_io->dev,
- "MC command timed out (portal: %#llx, dprc handle: %#x, command: %#x)\n",
- mc_io->portal_phys_addr,
+ "MC command timed out (portal: %pa, dprc handle: %#x, command: %#x)\n",
+ &mc_io->portal_phys_addr,
(unsigned int)mc_cmd_hdr_read_token(cmd),
(unsigned int)mc_cmd_hdr_read_cmdid(cmd));

@@ -290,8 +290,8 @@ int mc_send_command(struct fsl_mc_io *mc_io, struct mc_command *cmd)

if (status != MC_CMD_STATUS_OK) {
dev_dbg(mc_io->dev,
- "MC command failed: portal: %#llx, dprc handle: %#x, command: %#x, status: %s (%#x)\n",
- mc_io->portal_phys_addr,
+ "MC command failed: portal: %pa, dprc handle: %#x, command: %#x, status: %s (%#x)\n",
+ &mc_io->portal_phys_addr,
(unsigned int)mc_cmd_hdr_read_token(cmd),
(unsigned int)mc_cmd_hdr_read_cmdid(cmd),
mc_status_to_string(status),
--
2.9.4
l***@nxp.com
2017-07-19 11:42:32 UTC
Permalink
From: Laurentiu Tudor <***@nxp.com>

Add an alternate dependency on COMPILE_TEST, thus leaving
this driver compile on other architectures.
Also, other drivers depending on the bus are updated to depend
on ARCH_LAYERSCAPE until they'll also be made multi-arch.
This was compiled tested on:
- booke powerpc (corenet{32,64}_smp_defconfig)
- x86 (i386_defconfig, x86_64_defconfig, needs CONFIG_OF)
- arm64 (defconfig)

Signed-off-by: Laurentiu Tudor <***@nxp.com>
Acked-by: Arnd Bergmann <***@arndb.de>
---
Notes:
-v3
-add COMPILE_TEST instead of dropping ARCH_LAYERSCAPE
-v2
-no changes

drivers/staging/fsl-dpaa2/Kconfig | 2 +-
drivers/staging/fsl-mc/bus/Kconfig | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/fsl-dpaa2/Kconfig b/drivers/staging/fsl-dpaa2/Kconfig
index 730fd6d..dfff675 100644
--- a/drivers/staging/fsl-dpaa2/Kconfig
+++ b/drivers/staging/fsl-dpaa2/Kconfig
@@ -4,7 +4,7 @@

config FSL_DPAA2
bool "Freescale DPAA2 devices"
- depends on FSL_MC_BUS
+ depends on FSL_MC_BUS && ARCH_LAYERSCAPE
---help---
Build drivers for Freescale DataPath Acceleration
Architecture (DPAA2) family of SoCs.
diff --git a/drivers/staging/fsl-mc/bus/Kconfig b/drivers/staging/fsl-mc/bus/Kconfig
index a10aaf0..1af8d1d 100644
--- a/drivers/staging/fsl-mc/bus/Kconfig
+++ b/drivers/staging/fsl-mc/bus/Kconfig
@@ -8,7 +8,7 @@

config FSL_MC_BUS
bool "QorIQ DPAA2 fsl-mc bus driver"
- depends on OF && ARCH_LAYERSCAPE
+ depends on OF && (ARCH_LAYERSCAPE || COMPILE_TEST)
select GENERIC_MSI_IRQ_DOMAIN
help
Driver to enable the bus infrastructure for the QorIQ DPAA2
@@ -18,7 +18,7 @@ config FSL_MC_BUS

config FSL_MC_DPIO
tristate "QorIQ DPAA2 DPIO driver"
- depends on FSL_MC_BUS
+ depends on FSL_MC_BUS && ARCH_LAYERSCAPE
help
Driver for the DPAA2 DPIO object. A DPIO provides queue and
buffer management facilities for software to interact with
--
2.9.4
l***@nxp.com
2017-07-19 11:42:25 UTC
Permalink
From: Laurentiu Tudor <***@nxp.com>

The mc-bus specific field, fsl_mc in struct msi_desc is missing its
comment so add it.

Signed-off-by: Laurentiu Tudor <***@nxp.com>
Acked-by: Arnd Bergmann <***@arndb.de>
---
Notes:
-v3
-no changes
-v2
-no changes

include/linux/msi.h | 1 +
1 file changed, 1 insertion(+)

diff --git a/include/linux/msi.h b/include/linux/msi.h
index df6d592..80e3b56 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -66,6 +66,7 @@ struct fsl_mc_msi_desc {
* @mask_pos: [PCI MSI] Mask register position
* @mask_base: [PCI MSI-X] Mask register base address
* @platform: [platform] Platform device specific msi descriptor data
+ * @fsl_mc: [fsl-mc] FSL MC device specific msi descriptor data
*/
struct msi_desc {
/* Shared device/bus type independent data */
--
2.9.4
l***@nxp.com
2017-07-19 11:42:30 UTC
Permalink
From: Laurentiu Tudor <***@nxp.com>

As raw device io functions are not portable and don't handle byte-order
(triggering suspicion that endianness isn't handled well) switch to
using the standard api.
Since MC expects LE byte-order and the upper layers already take care
of that, we need to trick the device io api by doing a LE -> CPU
conversion just before calling it. This way, the CPU -> LE conversion
done in the api puts the data back in the right byte-order. Obviously,
for reads the extra step is mirrored: there's a CPU -> LE conversion
following the API call.

Signed-off-by: Laurentiu Tudor <***@nxp.com>
Acked-by: Arnd Bergmann <***@arndb.de>
---
Notes:
-v3
-no changes
-v2
-new patch replacing https://lkml.org/lkml/2017/7/17/419

drivers/staging/fsl-mc/bus/mc-sys.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/fsl-mc/bus/mc-sys.c b/drivers/staging/fsl-mc/bus/mc-sys.c
index 195d9f3..8a6dc47 100644
--- a/drivers/staging/fsl-mc/bus/mc-sys.c
+++ b/drivers/staging/fsl-mc/bus/mc-sys.c
@@ -126,12 +126,15 @@ static inline void mc_write_command(struct mc_command __iomem *portal,

/* copy command parameters into the portal */
for (i = 0; i < MC_CMD_NUM_OF_PARAMS; i++)
- __raw_writeq(cmd->params[i], &portal->params[i]);
- /* ensure command params are committed before submitting it */
- wmb();
+ /*
+ * Data is already in the expected LE byte-order. Do an
+ * extra LE -> CPU conversion so that the CPU -> LE done in
+ * the device io write api puts it back in the right order.
+ */
+ writeq_relaxed(le64_to_cpu(cmd->params[i]), &portal->params[i]);

/* submit the command by writing the header */
- __raw_writeq(cmd->header, &portal->header);
+ writeq(le64_to_cpu(cmd->header), &portal->header);
}

/**
@@ -151,14 +154,20 @@ static inline enum mc_cmd_status mc_read_response(struct mc_command __iomem *
enum mc_cmd_status status;

/* Copy command response header from MC portal: */
- resp->header = __raw_readq(&portal->header);
+ resp->header = cpu_to_le64(readq_relaxed(&portal->header));
status = mc_cmd_hdr_read_status(resp);
if (status != MC_CMD_STATUS_OK)
return status;

/* Copy command response data from MC portal: */
for (i = 0; i < MC_CMD_NUM_OF_PARAMS; i++)
- resp->params[i] = __raw_readq(&portal->params[i]);
+ /*
+ * Data is expected to be in LE byte-order. Do an
+ * extra CPU -> LE to revert the LE -> CPU done in
+ * the device io read api.
+ */
+ resp->params[i] =
+ cpu_to_le64(readq_relaxed(&portal->params[i]));

return status;
}
--
2.9.4
l***@nxp.com
2017-07-19 11:42:31 UTC
Permalink
From: Laurentiu Tudor <***@nxp.com>

Since there's no real constrain in MC to do only atomic 64-bit we can
enable this driver on 32-bit platforms too.
Include linux/io-64-nonatomic-hi-lo.h to make quad device io apis used
in the driver available on 32-bit platforms.

Signed-off-by: Laurentiu Tudor <***@nxp.com>
Acked-by: Arnd Bergmann <***@arndb.de>
---
Notes:
-v3
-no changes
-v2
-new patch replacing https://lkml.org/lkml/2017/7/17/419

drivers/staging/fsl-mc/bus/mc-sys.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/staging/fsl-mc/bus/mc-sys.c b/drivers/staging/fsl-mc/bus/mc-sys.c
index 8a6dc47..7ce105b 100644
--- a/drivers/staging/fsl-mc/bus/mc-sys.c
+++ b/drivers/staging/fsl-mc/bus/mc-sys.c
@@ -37,6 +37,7 @@
#include <linux/ioport.h>
#include <linux/device.h>
#include <linux/io.h>
+#include <linux/io-64-nonatomic-hi-lo.h>
#include "../include/mc.h"

#include "dpmcp.h"
--
2.9.4
l***@nxp.com
2017-07-19 11:42:27 UTC
Permalink
From: Laurentiu Tudor <***@nxp.com>

Nothing from linux/irqchip/arm-gic-v3.h is used, so the #include can be
safely dropped.

Signed-off-by: Laurentiu Tudor <***@nxp.com>
Acked-by: Arnd Bergmann <***@arndb.de>
---
Notes:
-v3
-no changes
-v2
-no changes

drivers/staging/fsl-mc/bus/fsl-mc-msi.c | 1 -
drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c | 1 -
2 files changed, 2 deletions(-)

diff --git a/drivers/staging/fsl-mc/bus/fsl-mc-msi.c b/drivers/staging/fsl-mc/bus/fsl-mc-msi.c
index c04a2f2..ee6e3b7 100644
--- a/drivers/staging/fsl-mc/bus/fsl-mc-msi.c
+++ b/drivers/staging/fsl-mc/bus/fsl-mc-msi.c
@@ -11,7 +11,6 @@

#include <linux/of_device.h>
#include <linux/of_address.h>
-#include <linux/irqchip/arm-gic-v3.h>
#include <linux/of_irq.h>
#include <linux/irq.h>
#include <linux/irqdomain.h>
diff --git a/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c b/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
index 865d385..e798ea4 100644
--- a/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
+++ b/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
@@ -11,7 +11,6 @@

#include <linux/of_device.h>
#include <linux/of_address.h>
-#include <linux/irqchip/arm-gic-v3.h>
#include <linux/irq.h>
#include <linux/msi.h>
#include <linux/of.h>
--
2.9.4
Loading...