Re: [PATCH v2 1/4] nvmem: nintendo-otp: Add new driver for the Wii and Wii U OTP

Hi,


On Wed, May 19, 2021 at 11:50:41AM +0200, Emmanuel Gil Peyrot wrote:

This OTP is read-only and contains various keys used by the console to
decrypt, encrypt or verify various pieces of storage.

Its size depends on the console, it is 128 bytes on the Wii and
1024 bytes on the Wii U (split into eight 128 bytes banks).

It can be used directly by writing into one register and reading from
the other one, without any additional synchronisation.

Signed-off-by: Emmanuel Gil Peyrot <redacted>
---

A link to the (third-party) documentation would be nice, either in the
commit message or in the code itself.

(https://wiiubrew.org/wiki/Hardware/OTP i guess)

[...]

+static int nintendo_otp_reg_read(void *context,
+				 unsigned int reg, void *_val, size_t bytes)
+{
+	struct nintendo_otp_priv *priv = context;
+	u32 *val = _val;
+	int words = bytes >> 2;
+	u32 bank, addr;
+
+	while (words--) {
+		bank = (reg << 1) & ~0xff;

This is a bit non-obvious, IMHO. As far as I understand it, the expanded
formula is:

	bank = (reg / 128) << 8;

I.e. first divide by bank size, then shift the parameter into the right
place.

+		addr = (reg >> 2) & 0x1f;

Here, I think it's about the word size (4 bytes); I think / 4 would be
clearer.

I *think* (but haven't checked) that gcc should generate efficent shifts
for the divisions above, so using the division operator shouldn't be
problem.

+		iowrite32be(OTP_READ | bank | addr, priv->regs + HW_OTPCMD);
+		*val++ = ioread32be(priv->regs + HW_OTPDATA);
+		reg += 4;
+	}
+
+	return 0;
+}
+

[...]

+	if (of_id->data) {
+		const struct nintendo_otp_devtype_data *data = of_id->data;
+		config.name = data->name;
+		config.size = data->num_banks * 128;

Given that 128 appears a few times, perhaps a #define would be good.

+	}
+
+	config.dev = dev;
+	config.priv = priv;
+
+	nvmem = devm_nvmem_register(dev, &config);
+
+	return PTR_ERR_OR_ZERO(nvmem);
+}
+
+static struct platform_driver nintendo_otp_driver = {
+	.probe = nintendo_otp_probe,
+	.driver = {
+		.name = "nintendo-otp",
+		.of_match_table = nintendo_otp_of_table,
+	},
+};
+module_platform_driver(nintendo_otp_driver);
+MODULE_AUTHOR("Emmanuel Gil Peyrot [off-list ref]");
+MODULE_DESCRIPTION("Nintendo Wii and Wii U OTP driver");
+MODULE_LICENSE("GPL v2");
-- 
2.31.1

Tested-by: Jonathan Neuschäfer <redacted>  # on Wii



Thanks,
Jonathan Neuschäfer