From: Laurentiu Tudor <[email protected]>
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
From: Laurentiu Tudor <[email protected]>
The mc-bus specific field, fsl_mc in struct msi_desc is missing its
comment so add it.
Signed-off-by: Laurentiu Tudor <[email protected]>
---
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
From: Laurentiu Tudor <[email protected]>
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 <[email protected]>
---
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
From: Laurentiu Tudor <[email protected]>
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 <[email protected]>
---
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
From: Laurentiu Tudor <[email protected]>
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 <[email protected]>
---
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
From: Laurentiu Tudor <[email protected]>
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 <[email protected]>
---
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
From: Laurentiu Tudor <[email protected]>
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 <[email protected]>
---
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
From: Laurentiu Tudor <[email protected]>
Nothing from linux/irqchip/arm-gic-v3.h is used, so the #include can be
safely dropped.
Signed-off-by: Laurentiu Tudor <[email protected]>
---
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
From: Laurentiu Tudor <[email protected]>
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 <[email protected]>
---
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
On Tue, Jul 18, 2017 at 3:37 PM, <[email protected]> wrote:
> From: Laurentiu Tudor <[email protected]>
>
> 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 <[email protected]>
> ---
> 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);
> }
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
On Tue, Jul 18, 2017 at 3:37 PM, <[email protected]> wrote:
> --- 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
Hi Arnd,
On 07/18/2017 05:18 PM, Arnd Bergmann wrote:
> On Tue, Jul 18, 2017 at 3:37 PM, <[email protected]> wrote:
>> From: Laurentiu Tudor <[email protected]>
>>
>> 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 <[email protected]>
>> ---
>> Notes:
>> -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);
>> }
>
> 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.
Right. That's how it should happen.
> 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
On Tue, Jul 18, 2017 at 3:37 PM, <[email protected]> wrote:
> From: Laurentiu Tudor <[email protected]>
>
> 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/
Looks good overall. With the two minor questions addressed
Acked-by: Arnd Bergmann <[email protected]>
On Tue, Jul 18, 2017 at 4:26 PM, Laurentiu Tudor
<[email protected]> wrote:
>
> 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
Hi Arnd,
On 07/18/2017 05:25 PM, Arnd Bergmann wrote:
> On Tue, Jul 18, 2017 at 3:37 PM, <[email protected]> wrote:
>
>> --- 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
On Tue, Jul 18, 2017 at 4:36 PM, Laurentiu Tudor
<[email protected]> wrote:
> 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.
I was thinking of adding it to all three, but you are right we only needed
it for the bus.
Arnd
Hi Arnd,
On 07/18/2017 05:26 PM, Arnd Bergmann wrote:
> On Tue, Jul 18, 2017 at 3:37 PM, <[email protected]> wrote:
>> From: Laurentiu Tudor <[email protected]>
>>
>> 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://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
>
> Acked-by: Arnd Bergmann <[email protected]>
>
Thanks for the ack. I'll mention it the next version.
---
Thanks & Best Regards, Laurentiu