On 03/09/2015 01:54 PM, Andy Shevchenko wrote:
On Mon, Mar 9, 2015 at 8:01 PM, Thor Thayer
<tthayer@xxxxxxxxxxxxxxxxxxxxx> wrote:
On 03/07/2015 01:52 PM, Andy Shevchenko wrote:
On Sat, Mar 7, 2015 at 1:46 AM, <tthayer@xxxxxxxxxxxxxxxxxxxxx> wrote:
From: Thor Thayer <tthayer@xxxxxxxxxxxxxxxxxxxxx>
Altera's Arria10 SoC interconnect requires a 32 bit write for APB
peripherals. The current spi-dw driver uses 16bit accesses in
some locations. Use function pointers to support 32 bit accesses
but retain legacy 16 bit access.
--- a/drivers/spi/spi-dw-mmio.c
+++ b/drivers/spi/spi-dw-mmio.c
@@ -76,8 +76,13 @@ static int dw_spi_mmio_probe(struct platform_device
*pdev)
num_cs = 4;
- if (pdev->dev.of_node)
+ if (pdev->dev.of_node) {
of_property_read_u32(pdev->dev.of_node, "num-cs",
&num_cs);
+ if (of_property_read_bool(pdev->dev.of_node,
"32bit_access")) {
"32bit-access"
Thanks. I will make the change.
+ dws->read_w = dw_readw32;
+ dws->write_w = dw_writew32;
Can we use just readw/writew (w/o underscores) as names for the
accessors?
I tried this initially and got a namespace conflict with the readw & write2
macros.
macro writew passed 3 arguments, but takes just 2
macro readw passed 2 arguments, but takes just 1
Might be I wasn't clear enough. I meant
dws->readw = …
dws->writew = …
Yes, we agree. These are exactly what I was using when I saw the error.
--- a/drivers/spi/spi-dw.h
+++ b/drivers/spi/spi-dw.h
@@ -141,6 +141,8 @@ struct dw_spi {
#ifdef CONFIG_DEBUG_FS
struct dentry *debugfs;
#endif
+ u16 (*read_w)(struct dw_spi *dws, u32 offset);
+ void (*write_w)(struct dw_spi *dws, u32 offset, u16 val);
readw
writew
As I can see there are no such field names in struct dw_spi.
Yes. This seems safe but I still see the macro conflict which is strange
to me. Like you, I initially used the dws->readw and dws->writew because
I didn't see any conflicting field names.
Are readw & writew reserved variable names?
};
static inline u32 dw_readl(struct dw_spi *dws, u32 offset)
@@ -163,6 +165,16 @@ static inline void dw_writew(struct dw_spi *dws, u32
offset, u16 val)
__raw_writew(val, dws->regs + offset);
}
+static inline u16 dw_readw32(struct dw_spi *dws, u32 offset)
+{
+ return (u16)__raw_readl(dws->regs + offset)
Maybe return dw_readl(dws, offset);
+}
+
+static inline void dw_writew32(struct dw_spi *dws, u32 offset, u16 val)
+{
+ __raw_writel((u32)val, dws->regs + offset);
dw_writel(dws, offset, val);
Yes, I'll make these two changes - at least the write change. Based on
feedback from your suggestion below, the dw_readw32 may be removed.
+}
+
So, does simple
dws->readw = dw_readl;
dws->writew = dw_writel;
work for you?
Yes, but I get the macro conflict shown above and "assignment from
incompatible pointer type" warnings. If I use the dws->read_w and
dws->write_w names, I get the incompatible pointer type warnings but it
works.
Thanks for reviewing.
Okay, I guess there are two variants:
a) replace u16 by u32 in existing functions;
b) introduce new ones like you did.
If no other opinions I think we better go b), but see above comments.
And one more thing. If dw_readw() works for maybe we leave them as is for now?
Yes, I just need the 32 bit write. I was trying to remain consistent but
I agree that only changing only writes would minimize the changes.
Thanks!
--
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
