On Mon, Mar 9, 2015 at 8:01 PM, Thor Thayer
<[email protected]> wrote:
>
>
> On 03/07/2015 01:52 PM, Andy Shevchenko wrote:
>>
>> On Sat, Mar 7, 2015 at 1:46 AM, <[email protected]> wrote:
>>>
>>> From: Thor Thayer <[email protected]>
>>>
>>> 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"
>>> + 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 = …
>>> --- 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.
>>> };
>>>
>>> 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);
>>> +}
>>> +
>>
>> 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?
--
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
