On 07/31/2017 02:33 PM, Marek Vasut wrote:
On 07/31/2017 07:17 PM, Alexandru Gagniuc wrote:
Hi Marek,
Thank you again for your feedback. I've applied a majority of your
suggestions, and I am very happy with the result. I should have v2
posted within a day or so.
[snip]
+/*
+ * This mask does not match reality. Get over it:
What is this about ?
Each stage of the QSPI chain has two registers. The second register has
a bitfield which takes in the length of the stage. For example, for
DATA2, we can set the length up to 0x4000, but for ADDR2, we can only
set a max of 4 bytes. I wrote this comment as a reminder to myself to be
careful about using this mask. I'll rephrase the comment for [v2]
Please do.
Staged for [PATCH v2]
+ * DATA2: 0x3fff
+ * CMD2: 0x0003
+ * ADDR2: 0x0007
+ * PERF2: 0x0000
+ * HI_Z: 0x003f
+ * BCNT: 0x0007
+ */
+#define CHAIN_LEN(x) ((x - 1) & ASPI_DATA_LEN_MASK)
+
+struct anarion_qspi {
+ struct spi_nor nor;
+ struct device *dev;
+ uintptr_t regbase;
Should be void __iomem * I guess ?
I chose uintptr_t as opposed to void *, because arithmetic on void * is
not valid in C. What is the right answer hen, without risking undefined
behavior?
What sort of arithmetic ? It's perfectly valid in general ...
ISO/IEC 9899:201x, Section 6.5.6, constraint(2) is not met when the one
of the operands to addition is a void pointer.
Section 6.2.5 (19) defines void to be an incomplete type.
[snip]
Is this stuff below something like ioread32_rep() ?
+ aspi_write_reg(aspi, ASPI_REG_BYTE_COUNT, sizeof(uint32_t));
+ while (len >= 4) {
+ data = aspi_read_reg(aspi, ASPI_REG_DATA1);
+ memcpy(buf, &data, sizeof(data));
+ buf += 4;
+ len -= 4;
+ }
That is very similar to ioread32_rep, yes. I kept this as for the
reasons outlined above, but changing this to _rep() seems innocent enough.
What reason ?
Being able to share the code between the different codebases where it is
used.
Alex
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
