From: laurentiu.tudor@nxp.com (Laurentiu Tudor)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 6/8] staging: fsl-mc: don't use raw device io functions
Date: Tue, 18 Jul 2017 14:26:00 +0000 [thread overview]
Message-ID: <596E1A77.4030204@nxp.com> (raw)
In-Reply-To: <CAK8P3a0zH1nVoppNkT=uVgFp-NUkmenZ0AVc9vOwHEe_VEt_cQ@mail.gmail.com>
Hi Arnd,
On 07/18/2017 05:18 PM, Arnd Bergmann wrote:
> On Tue, Jul 18, 2017 at 3:37 PM, <laurentiu.tudor@nxp.com> wrote:
>> From: Laurentiu Tudor <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 <laurentiu.tudor@nxp.com>
>> ---
>> 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
WARNING: multiple messages have this Message-ID (diff)
From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
To: Arnd Bergmann <arnd@arndb.de>
Cc: gregkh <gregkh@linuxfoundation.org>,
Stuart Yoder <stuyoder@gmail.com>,
"devel@driverdev.osuosl.org" <devel@driverdev.osuosl.org>,
"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
Marc Zyngier <marc.zyngier@arm.com>,
Alexander Graf <agraf@suse.de>,
Robin Murphy <robin.murphy@arm.com>,
Ioana Ciornei <ioana.ciornei@nxp.com>,
"Ruxandra Ioana Radulescu" <ruxandra.radulescu@nxp.com>,
Bharat Bhushan <bharat.bhushan@nxp.com>,
Catalin Horghidan <catalin.horghidan@nxp.com>,
"Leo Li" <leoyang.li@nxp.com>, Roy Pledge <roy.pledge@nxp.com>,
Linux ARM <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v2 6/8] staging: fsl-mc: don't use raw device io functions
Date: Tue, 18 Jul 2017 14:26:00 +0000 [thread overview]
Message-ID: <596E1A77.4030204@nxp.com> (raw)
In-Reply-To: <CAK8P3a0zH1nVoppNkT=uVgFp-NUkmenZ0AVc9vOwHEe_VEt_cQ@mail.gmail.com>
Hi Arnd,
On 07/18/2017 05:18 PM, Arnd Bergmann wrote:
> On Tue, Jul 18, 2017 at 3:37 PM, <laurentiu.tudor@nxp.com> wrote:
>> From: Laurentiu Tudor <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 <laurentiu.tudor@nxp.com>
>> ---
>> 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
next prev parent reply other threads:[~2017-07-18 14:26 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-18 13:37 [PATCH v2 0/7] staging: fsl-mc: make the driver compile on other architectures laurentiu.tudor at nxp.com
2017-07-18 13:37 ` laurentiu.tudor
2017-07-18 13:37 ` [PATCH v2 1/8] staging: fsl-mc: add missing fsl_mc comment in struct msi_desc laurentiu.tudor at nxp.com
2017-07-18 13:37 ` laurentiu.tudor
2017-07-18 13:37 ` [PATCH v2 2/8] staging: fsl-mc: use generic memory barriers laurentiu.tudor at nxp.com
2017-07-18 13:37 ` laurentiu.tudor
2017-07-18 13:37 ` [PATCH v2 3/8] staging: fsl-mc: drop useless gic v3 related #include laurentiu.tudor at nxp.com
2017-07-18 13:37 ` laurentiu.tudor
2017-07-18 13:37 ` [PATCH v2 4/8] staging: fsl-mc: fix compilation with non-generic msi domain ops laurentiu.tudor at nxp.com
2017-07-18 13:37 ` laurentiu.tudor
2017-07-18 13:37 ` [PATCH v2 5/8] staging: fsl-mc: fix formating of phys_addr_t on 32 bits laurentiu.tudor at nxp.com
2017-07-18 13:37 ` laurentiu.tudor
2017-07-18 13:37 ` [PATCH v2 6/8] staging: fsl-mc: don't use raw device io functions laurentiu.tudor at nxp.com
2017-07-18 13:37 ` laurentiu.tudor
2017-07-18 14:18 ` Arnd Bergmann
2017-07-18 14:18 ` Arnd Bergmann
2017-07-18 14:26 ` Laurentiu Tudor [this message]
2017-07-18 14:26 ` Laurentiu Tudor
2017-07-18 14:28 ` Arnd Bergmann
2017-07-18 14:28 ` Arnd Bergmann
2017-07-18 13:37 ` [PATCH v2 7/8] staging: fsl-mc: make the driver compile on 32-bit laurentiu.tudor at nxp.com
2017-07-18 13:37 ` laurentiu.tudor
2017-07-18 13:37 ` [PATCH v2 8/8] staging: fsl-mc: allow the driver compile multi-arch laurentiu.tudor at nxp.com
2017-07-18 13:37 ` laurentiu.tudor
2017-07-18 14:25 ` Arnd Bergmann
2017-07-18 14:25 ` Arnd Bergmann
2017-07-18 14:36 ` Laurentiu Tudor
2017-07-18 14:36 ` Laurentiu Tudor
2017-07-18 14:39 ` Arnd Bergmann
2017-07-18 14:39 ` Arnd Bergmann
2017-07-18 14:26 ` [PATCH v2 0/7] staging: fsl-mc: make the driver compile on other architectures Arnd Bergmann
2017-07-18 14:26 ` Arnd Bergmann
2017-07-18 14:43 ` Laurentiu Tudor
2017-07-18 14:43 ` Laurentiu Tudor
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=596E1A77.4030204@nxp.com \
--to=laurentiu.tudor@nxp.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.