2021-04-29 08:38:12

by Pali Rohár

[permalink] [raw]
Subject: [PATCH v2 mvebu + mvebu/dt64 1/6] firmware: turris-mox-rwtm: fix reply status decoding function

From: Marek Behún <[email protected]>

The status decoding function mox_get_status() currently contains a dead
code path: if the error status is not MBOX_STS_SUCCESS, it always
returns -EIO, so the comparison to MBOX_STS_FAIL is never executed and
we don't get the actual error code sent by the firmware.

Fix this.

Signed-off-by: Marek Behún <[email protected]>
Fixes: 389711b37493 ("firmware: Add Turris Mox rWTM firmware driver")
---
drivers/firmware/turris-mox-rwtm.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/turris-mox-rwtm.c b/drivers/firmware/turris-mox-rwtm.c
index 62f0d1a5dd32..f85acdb3130c 100644
--- a/drivers/firmware/turris-mox-rwtm.c
+++ b/drivers/firmware/turris-mox-rwtm.c
@@ -147,11 +147,14 @@ MOX_ATTR_RO(pubkey, "%s\n", pubkey);

static int mox_get_status(enum mbox_cmd cmd, u32 retval)
{
- if (MBOX_STS_CMD(retval) != cmd ||
- MBOX_STS_ERROR(retval) != MBOX_STS_SUCCESS)
+ if (MBOX_STS_CMD(retval) != cmd)
return -EIO;
else if (MBOX_STS_ERROR(retval) == MBOX_STS_FAIL)
return -(int)MBOX_STS_VALUE(retval);
+ else if (MBOX_STS_ERROR(retval) == MBOX_STS_BADCMD)
+ return -ENOSYS;
+ else if (MBOX_STS_ERROR(retval) != MBOX_STS_SUCCESS)
+ return -EIO;
else
return MBOX_STS_VALUE(retval);
}
--
2.20.1


2021-04-29 08:38:12

by Pali Rohár

[permalink] [raw]
Subject: [PATCH v2 mvebu + mvebu/dt64 2/6] firmware: turris-mox-rwtm: report failures better

From: Marek Behún <[email protected]>

Report a notice level message if a command is not supported by the rWTM
firmware.

This should not be an error, merely a notice, because the firmware can
be used on non-CZ.NIC boards that do not have manufacturing information
burned.

Signed-off-by: Marek Behún <[email protected]>
Fixes: 389711b37493 ("firmware: Add Turris Mox rWTM firmware driver")
---
drivers/firmware/turris-mox-rwtm.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/firmware/turris-mox-rwtm.c b/drivers/firmware/turris-mox-rwtm.c
index f85acdb3130c..d7e3489e4bf2 100644
--- a/drivers/firmware/turris-mox-rwtm.c
+++ b/drivers/firmware/turris-mox-rwtm.c
@@ -204,11 +204,14 @@ static int mox_get_board_info(struct mox_rwtm *rwtm)
return ret;

ret = mox_get_status(MBOX_CMD_BOARD_INFO, reply->retval);
- if (ret < 0 && ret != -ENODATA) {
- return ret;
- } else if (ret == -ENODATA) {
+ if (ret == -ENODATA) {
dev_warn(rwtm->dev,
"Board does not have manufacturing information burned!\n");
+ } else if (ret == -ENOSYS) {
+ dev_notice(rwtm->dev,
+ "Firmware does not support the BOARD_INFO command\n");
+ } else if (ret < 0) {
+ return ret;
} else {
rwtm->serial_number = reply->status[1];
rwtm->serial_number <<= 32;
@@ -237,10 +240,13 @@ static int mox_get_board_info(struct mox_rwtm *rwtm)
return ret;

ret = mox_get_status(MBOX_CMD_ECDSA_PUB_KEY, reply->retval);
- if (ret < 0 && ret != -ENODATA) {
- return ret;
- } else if (ret == -ENODATA) {
+ if (ret == -ENODATA) {
dev_warn(rwtm->dev, "Board has no public key burned!\n");
+ } else if (ret == -ENOSYS) {
+ dev_notice(rwtm->dev,
+ "Firmware does not support the ECDSA_PUB_KEY command\n");
+ } else if (ret < 0) {
+ return ret;
} else {
u32 *s = reply->status;

--
2.20.1

2021-04-29 08:38:29

by Pali Rohár

[permalink] [raw]
Subject: [PATCH v2 mvebu + mvebu/dt64 3/6] firmware: turris-mox-rwtm: fail probing when firmware does not support hwrng

When Marvell's rWTM firmware, which does not support the GET_RANDOM
command, is used, kernel prints an error message
hwrng: no data available
every 10 seconds.

Fail probing of this driver if the rWTM firmware does not support the
GET_RANDOM command.

This makes it possible to put this driver's compatible into generic
armada-37xx device tree, to be available for other Armada 3720 devices
besides Turris MOX. If they use the rWTM firmware from CZ.NIC, they will
have HWRNG available, and if not, the driver won't be complaining.

Signed-off-by: Pali Rohár <[email protected]>
Fixes: 389711b37493 ("firmware: Add Turris Mox rWTM firmware driver")
Signed-off-by: Marek Behún <[email protected]>
---
drivers/firmware/turris-mox-rwtm.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)

diff --git a/drivers/firmware/turris-mox-rwtm.c b/drivers/firmware/turris-mox-rwtm.c
index d7e3489e4bf2..3ef9687dddca 100644
--- a/drivers/firmware/turris-mox-rwtm.c
+++ b/drivers/firmware/turris-mox-rwtm.c
@@ -260,6 +260,27 @@ static int mox_get_board_info(struct mox_rwtm *rwtm)
return 0;
}

+static int check_get_random_support(struct mox_rwtm *rwtm)
+{
+ struct armada_37xx_rwtm_tx_msg msg;
+ int ret;
+
+ msg.command = MBOX_CMD_GET_RANDOM;
+ msg.args[0] = 1;
+ msg.args[1] = rwtm->buf_phys;
+ msg.args[2] = 4;
+
+ ret = mbox_send_message(rwtm->mbox, &msg);
+ if (ret < 0)
+ return ret;
+
+ ret = wait_for_completion_timeout(&rwtm->cmd_done, HZ / 2);
+ if (ret < 0)
+ return ret;
+
+ return mox_get_status(MBOX_CMD_GET_RANDOM, rwtm->reply.retval);
+}
+
static int mox_hwrng_read(struct hwrng *rng, void *data, size_t max, bool wait)
{
struct mox_rwtm *rwtm = (struct mox_rwtm *) rng->priv;
@@ -497,6 +518,13 @@ static int turris_mox_rwtm_probe(struct platform_device *pdev)
if (ret < 0)
dev_warn(dev, "Cannot read board information: %i\n", ret);

+ ret = check_get_random_support(rwtm);
+ if (ret < 0) {
+ dev_notice(dev,
+ "Firmware does not support the GET_RANDOM command\n");
+ goto free_channel;
+ }
+
rwtm->hwrng.name = DRIVER_NAME "_hwrng";
rwtm->hwrng.read = mox_hwrng_read;
rwtm->hwrng.priv = (unsigned long) rwtm;
--
2.20.1

2021-04-29 08:39:03

by Pali Rohár

[permalink] [raw]
Subject: [PATCH v2 mvebu + mvebu/dt64 5/6] firmware: turris-mox-rwtm: add marvell,armada-3700-rwtm-firmware compatible string

Add more generic compatible string 'marvell,armada-3700-rwtm-firmware' for
this driver, since it can also be used on other Armada 3720 devices.

Current compatible string 'cznic,turris-mox-rwtm' is kept for backward
compatibility.

Signed-off-by: Pali Rohár <[email protected]>
Fixes: 389711b37493 ("firmware: Add Turris Mox rWTM firmware driver")

---
We are also planning to work on extending this driver to support
accessing OTP, which will also work with Marvell's default WTMI
firmware.
---
drivers/firmware/turris-mox-rwtm.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/firmware/turris-mox-rwtm.c b/drivers/firmware/turris-mox-rwtm.c
index 1cf4f1087492..c2d34dc8ba46 100644
--- a/drivers/firmware/turris-mox-rwtm.c
+++ b/drivers/firmware/turris-mox-rwtm.c
@@ -569,6 +569,7 @@ static int turris_mox_rwtm_remove(struct platform_device *pdev)

static const struct of_device_id turris_mox_rwtm_match[] = {
{ .compatible = "cznic,turris-mox-rwtm", },
+ { .compatible = "marvell,armada-3700-rwtm-firmware", },
{ },
};

--
2.20.1

2021-04-29 08:39:29

by Pali Rohár

[permalink] [raw]
Subject: [PATCH v2 mvebu + mvebu/dt64 6/6] arm64: dts: marvell: armada-37xx: move firmware node to generic dtsi file

Move the turris-mox-rwtm firmware node from Turris MOX' device tree into
the generic armada-37xx.dtsi file and use the generic compatible string
'marvell,armada-3700-rwtm-firmware' instead of the current one.

The Turris MOX rWTM firmware can be used on any Armada 37xx device,
giving them access to the rWTM hardware random number generator, which
is otherwise unavailable.

This change allows Linux to load the turris-mox-rwtm.ko module on these
boards.

Tested on ESPRESSObin v5 with both default Marvell WTMI firmware and
CZ.NIC's firmware. With default WTMI firmware the turris-mox-rwtm fails
to probe, while with CZ.NIC's firmware it registers the HW random number
generator.

Signed-off-by: Pali Rohár <[email protected]>
Signed-off-by: Marek Behún <[email protected]>
Cc: <[email protected]> # 5.4+: 46d2f6d0c99f ("arm64: dts: armada-3720-turris-mox: add firmware node")
---
arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts | 8 --------
arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 8 ++++++++
2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
index 0753cc489638..ebb0ddf8d306 100644
--- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
+++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
@@ -107,14 +107,6 @@
/* enabled by U-Boot if SFP module is present */
status = "disabled";
};
-
- firmware {
- turris-mox-rwtm {
- compatible = "cznic,turris-mox-rwtm";
- mboxes = <&rwtm 0>;
- status = "okay";
- };
- };
};

&i2c0 {
diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
index 1b7f43e27589..847a2d12a4be 100644
--- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
@@ -505,4 +505,12 @@
};
};
};
+
+ firmware {
+ armada-3700-rwtm {
+ compatible = "marvell,armada-3700-rwtm-firmware";
+ mboxes = <&rwtm 0>;
+ status = "okay";
+ };
+ };
};
--
2.20.1

2021-04-29 08:40:15

by Pali Rohár

[permalink] [raw]
Subject: [PATCH v2 mvebu + mvebu/dt64 4/6] firmware: turris-mox-rwtm: show message about HWRNG registration

Currently it is hard to determinate if on Armada 3720 device is HWRNG
by running kernel accessible or not. So print information message into
dmesg when HWRNG is available and registration was successful.

Signed-off-by: Pali Rohár <[email protected]>
Fixes: 389711b37493 ("firmware: Add Turris Mox rWTM firmware driver")
---
drivers/firmware/turris-mox-rwtm.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/firmware/turris-mox-rwtm.c b/drivers/firmware/turris-mox-rwtm.c
index 3ef9687dddca..1cf4f1087492 100644
--- a/drivers/firmware/turris-mox-rwtm.c
+++ b/drivers/firmware/turris-mox-rwtm.c
@@ -542,6 +542,8 @@ static int turris_mox_rwtm_probe(struct platform_device *pdev)
goto free_channel;
}

+ dev_info(dev, "HWRNG successfully registered\n");
+
return 0;

free_channel:
--
2.20.1

2021-05-03 18:03:16

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH v2 mvebu + mvebu/dt64 1/6] firmware: turris-mox-rwtm: fix reply status decoding function

On Thu, Apr 29, 2021 at 10:36:31AM +0200, Pali Roh?r wrote:
> From: Marek Beh?n <[email protected]>
>
> The status decoding function mox_get_status() currently contains a dead
> code path: if the error status is not MBOX_STS_SUCCESS, it always
> returns -EIO, so the comparison to MBOX_STS_FAIL is never executed and
> we don't get the actual error code sent by the firmware.
>
> Fix this.
>
> Signed-off-by: Marek Beh?n <[email protected]>
> Fixes: 389711b37493 ("firmware: Add Turris Mox rWTM firmware driver")

You have put a fixes tag here, meaning you want it in stable? How does
dead code elimination fulfil the stable requirements?

Do any of these changes contain real fixes?

Andrew

2021-05-05 16:21:46

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH v2 mvebu + mvebu/dt64 1/6] firmware: turris-mox-rwtm: fix reply status decoding function

On Wed, May 05, 2021 at 06:04:33PM +0200, Marek Beh?n wrote:
> On Mon, 3 May 2021 14:22:49 +0200
> Andrew Lunn <[email protected]> wrote:
>
> > On Thu, Apr 29, 2021 at 10:36:31AM +0200, Pali Roh?r wrote:
> > > From: Marek Beh?n <[email protected]>
> > >
> > > The status decoding function mox_get_status() currently contains a dead
> > > code path: if the error status is not MBOX_STS_SUCCESS, it always
> > > returns -EIO, so the comparison to MBOX_STS_FAIL is never executed and
> > > we don't get the actual error code sent by the firmware.
> > >
> > > Fix this.
> > >
> > > Signed-off-by: Marek Beh?n <[email protected]>
> > > Fixes: 389711b37493 ("firmware: Add Turris Mox rWTM firmware driver")
> >
> > You have put a fixes tag here, meaning you want it in stable? How does
> > dead code elimination fulfil the stable requirements?
> >
> > Do any of these changes contain real fixes?
> >
> > Andrew
>
> Andrew, this is not dead code elimination.

Please word you commit message differently.

The status decoding function mox_get_status() currently contains an
incorrect check: ...

Andrew

2021-05-05 20:01:29

by Marek Behún

[permalink] [raw]
Subject: Re: [PATCH v2 mvebu + mvebu/dt64 1/6] firmware: turris-mox-rwtm: fix reply status decoding function

On Mon, 3 May 2021 14:22:49 +0200
Andrew Lunn <[email protected]> wrote:

> On Thu, Apr 29, 2021 at 10:36:31AM +0200, Pali Rohár wrote:
> > From: Marek Behún <[email protected]>
> >
> > The status decoding function mox_get_status() currently contains a dead
> > code path: if the error status is not MBOX_STS_SUCCESS, it always
> > returns -EIO, so the comparison to MBOX_STS_FAIL is never executed and
> > we don't get the actual error code sent by the firmware.
> >
> > Fix this.
> >
> > Signed-off-by: Marek Behún <[email protected]>
> > Fixes: 389711b37493 ("firmware: Add Turris Mox rWTM firmware driver")
>
> You have put a fixes tag here, meaning you want it in stable? How does
> dead code elimination fulfil the stable requirements?
>
> Do any of these changes contain real fixes?
>
> Andrew

Andrew, this is not dead code elimination. Rather it is that there is
dead code path due to an incorrect check. By correcting the check, the
dead code path becomes alive and starts reporting errors correctly.
This fix is nedeed in stable so that stable will report errors
correctly.

Marek

2021-05-11 21:49:05

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH v2 mvebu + mvebu/dt64 1/6] firmware: turris-mox-rwtm: fix reply status decoding function

On Wednesday 05 May 2021 18:20:46 Andrew Lunn wrote:
> On Wed, May 05, 2021 at 06:04:33PM +0200, Marek Behún wrote:
> > On Mon, 3 May 2021 14:22:49 +0200
> > Andrew Lunn <[email protected]> wrote:
> >
> > > On Thu, Apr 29, 2021 at 10:36:31AM +0200, Pali Rohár wrote:
> > > > From: Marek Behún <[email protected]>
> > > >
> > > > The status decoding function mox_get_status() currently contains a dead
> > > > code path: if the error status is not MBOX_STS_SUCCESS, it always
> > > > returns -EIO, so the comparison to MBOX_STS_FAIL is never executed and
> > > > we don't get the actual error code sent by the firmware.
> > > >
> > > > Fix this.
> > > >
> > > > Signed-off-by: Marek Behún <[email protected]>
> > > > Fixes: 389711b37493 ("firmware: Add Turris Mox rWTM firmware driver")
> > >
> > > You have put a fixes tag here, meaning you want it in stable? How does
> > > dead code elimination fulfil the stable requirements?
> > >
> > > Do any of these changes contain real fixes?
> > >
> > > Andrew
> >
> > Andrew, this is not dead code elimination.
>
> Please word you commit message differently.
>
> The status decoding function mox_get_status() currently contains an
> incorrect check: ...
>
> Andrew

Andrew, Marek has already updated commit message and I have sent a new
version v3 of this patch series with this update. It is OK now?