Discussion:
[PATCH v2 0/7] staging: fsl-mc: make the driver compile on other architectures
l***@nxp.com
2017-07-18 13:37:15 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 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 (7):
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: rewrite mc command send/receive to work on 32-bits
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 | 45 +++++++++-------------
include/linux/msi.h | 1 +
7 files changed, 32 insertions(+), 36 deletions(-)
--
2.9.4
l***@nxp.com
2017-07-18 13:37:16 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>
---
Notes:
-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-18 13:37:17 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>
---
Notes:
-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-18 13:37:19 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>
---
Notes:
-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-18 13:37:18 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>
---
Notes:
-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
l***@nxp.com
2017-07-18 13:37:21 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>
---
Notes:
-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
Arnd Bergmann
2017-07-18 14:18:21 UTC
Permalink
Post by l***@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.
---
-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);
}
Looks good, but just to be sure this is what you intended:

On 32-bit systems, this will now write val>>32 to cmd->header+4,
followed by writing val&0xffffffff to cmd->header.

You said earlier that the command is triggered when the final
four bytes are written, but it looks like the order is wrong now.

Should you use io-64-nonatomic-lo-hi.h instead of
io-64-nonatomic-hi-lo.h then?

Arnd
Laurentiu Tudor
2017-07-18 14:26:00 UTC
Permalink
Hi Arnd,
Post by Arnd Bergmann
Post by l***@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.
---
-v2
-new patch replacing https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.org%2Flkml%2F2017%2F7%2F17%2F419&data=01%7C01%7Claurentiu.tudor%40nxp.com%7C77381272b4914c9ac64708d4cde7d94e%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0&sdata=FTVLKox6T4i9OFmb%2B5BkSEDQrDrafXznY6nsJ0dgFSk%3D&reserved=0
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);
}
On 32-bit systems, this will now write val>>32 to cmd->header+4,
followed by writing val&0xffffffff to cmd->header.
Right. That's how it should happen.
Post by Arnd Bergmann
You said earlier that the command is triggered when the final
four bytes are written, but it looks like the order is wrong now.
Should you use io-64-nonatomic-lo-hi.h instead of
io-64-nonatomic-hi-lo.h then?
Maybe i made an error in my previous emails, but the hi-lo variant is
the correct one. The command execution is triggered when the _first_
32-bit half of the header (header&0xffffffff) is written, so that's why
it must be written last.

---
Thanks & Best Regards, Laurentiu
Arnd Bergmann
2017-07-18 14:28:10 UTC
Permalink
On Tue, Jul 18, 2017 at 4:26 PM, Laurentiu Tudor
Post by Laurentiu Tudor
Maybe i made an error in my previous emails, but the hi-lo variant is
the correct one. The command execution is triggered when the _first_
32-bit half of the header (header&0xffffffff) is written, so that's why
it must be written last.
I'm pretty sure I just misremembered it then. Thanks for the clarification.

Arnd
l***@nxp.com
2017-07-18 13:37:20 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>
---
Notes:
-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-18 13:37:23 UTC
Permalink
From: Laurentiu Tudor <***@nxp.com>

Drop dependency on ARCH_LAYERSCAPE (which in turn depends on ARM64),
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>
---
Notes:
-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..c0acc97 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
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
Arnd Bergmann
2017-07-18 14:25:36 UTC
Permalink
Post by l***@nxp.com
--- 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.
I would probably leave the dependency in there conditionally, like

depends on ARCH_LAYERSCAPE || COMPILE_TEST

That way, we can build the driver on all architectures with "make allmodconfig"
or "make randconfig", but regular users that disable COMPILE_TEST
won't be bothered by the extra config options unless they have the
right hardware.

Arnd
Laurentiu Tudor
2017-07-18 14:36:07 UTC
Permalink
Hi Arnd,
Post by Arnd Bergmann
Post by l***@nxp.com
--- 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.
I would probably leave the dependency in there conditionally, like
depends on ARCH_LAYERSCAPE || COMPILE_TEST
That way, we can build the driver on all architectures with "make allmodconfig"
or "make randconfig", but regular users that disable COMPILE_TEST
won't be bothered by the extra config options unless they have the
right hardware.
Good point, I'll take care of it. But don't you mean COMPILE_TEST be
added on the actual MC_BUS config, like so:

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
?

The other drivers that depend on the MC_BUS won't compile on other
architectures.

---
Thanks & Best Regards, Laurentiu
Arnd Bergmann
2017-07-18 14:39:03 UTC
Permalink
On Tue, Jul 18, 2017 at 4:36 PM, Laurentiu Tudor
Post by Laurentiu Tudor
Good point, I'll take care of it. But don't you mean COMPILE_TEST be
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
?
The other drivers that depend on the MC_BUS won't compile on other
architectures.
I was thinking of adding it to all three, but you are right we only needed
it for the bus.

Arnd
l***@nxp.com
2017-07-18 13:37:22 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>
---
Notes:
-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
Arnd Bergmann
2017-07-18 14:26:15 UTC
Permalink
Post by l***@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].
- 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/
Looks good overall. With the two minor questions addressed

Acked-by: Arnd Bergmann <***@arndb.de>
Laurentiu Tudor
2017-07-18 14:43:45 UTC
Permalink
Hi Arnd,
Post by Arnd Bergmann
Post by l***@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].
- 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://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.spinics.net%2Flists%2Flinux-driver-devel%2Fmsg100585.html&data=01%7C01%7Claurentiu.tudor%40nxp.com%7C2564747d26964ed5b3f408d4cde8f442%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0&sdata=Dz8vvLzQESel2UTXrhQ3JZyNhx5VhUdj6%2BE6TDLnXmc%3D&reserved=0
[2] https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.ozlabs.org%2Fpatch%2F789474%2F&data=01%7C01%7Claurentiu.tudor%40nxp.com%7C2564747d26964ed5b3f408d4cde8f442%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0&sdata=%2FiGSaa4j60INeTWEbT926iEIAtya6tqIiIoQN1yFbmA%3D&reserved=0
Looks good overall. With the two minor questions addressed
Thanks for the ack. I'll mention it the next version.

---
Thanks & Best Regards, Laurentiu

Loading...