Ben Dooks [ben-i2c@xxxxxxxxx] wrote:
> On Tue, Jan 17, 2012 at 07:18:20PM +0530, Jayachandran C. wrote:
>> From: Ganesan Ramalingam <ganesanr@xxxxxxxxxxxxxxxxx>
>>
>> Add support for the intergrated I2C controller on Netlogic
>> XLR/XLS MIPS SoC.
>>
>> +/*
>> + * Need un-swapped IO for the SoC I2C registers, use __raw_ IO
>> + */
>> +static inline void xlr_i2c_wreg(u32 __iomem *base, unsigned int reg, u32 val)
>> +{
>> + __raw_writel(val, base + reg);
>> +}
>> +
>> +static inline u32 xlr_i2c_rdreg(u32 __iomem *base, unsigned int reg)
>> +{
>> + return __raw_readl(base + reg);
>> +}
>
> Here, there's the use of __raw accessors with memory returned from
> ioremap(). Please use readl/writel here, or provide a _good_ reason
> for not using them.
We had tried to explain it in the comment above the block. The I2C registers on
this SoC are in an MMIO area that is big-endian. The PCI memory and IO space of
this SoC is little-endian, and we have kept a byteswapping readl/writel so that the
generic PCI drivers will work.
>> +
>> + platform_set_drvdata(pdev, priv);
>> + ret = xlr_i2c_add_bus(priv);
>> +
>> + if (ret < 0) {
>> + dev_err(&priv->adap.dev, "Failed to add i2c bus.\n");
>> + ret = -ENXIO;
> Hmm, not sure if -ENXIO is the best return code form here. I would probably
> leave the 'ret' alone, and pass it to the upper layer.
This can be fixed up.
Thanks for the review, will post an updated version.
JC.
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
