Re: [PATCH v4 1/4] mtd: spi-nor: add OTP support

From: Michael Walle
Date: Mon Mar 15 2021 - 06:02:45 EST


Am 2021-03-15 10:44, schrieb Tudor.Ambarus@xxxxxxxxxxxxx:
On 3/15/21 11:39 AM, Tudor Ambarus - M18064 wrote:
On 3/15/21 11:23 AM, Michael Walle wrote:

cut

diff --git a/drivers/mtd/spi-nor/otp.c b/drivers/mtd/spi-nor/otp.c
new file mode 100644
index 000000000000..4e301fd5156b
--- /dev/null
+++ b/drivers/mtd/spi-nor/otp.c
@@ -0,0 +1,218 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * OTP support for SPI NOR flashes
+ *
+ * Copyright (C) 2021 Michael Walle <michael@xxxxxxxx>> + */
+
+#include <linux/log2.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/spi-nor.h>
+
+#include "core.h"
+
+#define spi_nor_otp_ops(nor) ((nor)->params->otp.ops)
+#define spi_nor_otp_region_len(nor) ((nor)->params->otp.org->len)
+#define spi_nor_otp_n_regions(nor)
((nor)->params->otp.org->n_regions)

I don't like these wrappers because they gratuiously hide what's really
there.
I find the code more easier to read without these wrappers, because I
don't
have to memorize what these wrappers are doing, and I better see how
the code
is organized.

TBH I find it easier on the eye and I've never seen so much dereferences
as in mtd/spi-nor.

the dereferences will still be there, but will be just hidden by these wrappers,
don't they? Why would one prefer the wrappers?

That's why I wrote "easier on the eye", yes that is subjective. But
there are also technical aspects, for example, the helpers all
operate on "struct nor*", so you don't have to use the
dereference chain or build up local variables. For example, see
spi_nor_otp_read_write(), it doesn't have to know
anything about the "struct spi_nor_otp_organization".

And of course you could change the actual implementation
without touching the code everywhere,.

-michael