On Tue, 2018-07-17 at 13:48 +0200, Alexandre Belloni wrote:

The Microsemi Ocelot I2C controller is a designware IP. It also has a
second set of registers to allow tweaking SDA hold time and spike
filtering.

Can you elaborate a bit?

Are they platform specific? Are they shadow registers? Are they
something else? Datasheet link / excerpt would be also good to read.
 

 Optional properties :
+ - reg : for "mscc,ocelot-i2c", a second register set to configure
the SDA hold
+   time, named ICPU_CFG:TWI_DELAY in the datasheet.
+

Hmm... Is this registers unique to the SoC in question? Is address of
them fixed or may be configured on RTL level?

If former is right, why do we need a separate property?

 
+#define MSCC_ICPU_CFG_TWI_DELAY		0x0
+#define MSCC_ICPU_CFG_TWI_DELAY_ENABLE	BIT(0)
+#define MSCC_ICPU_CFG_TWI_SPIKE_FILTER	0x4
+
+static int mscc_twi_set_sda_hold_time(struct dw_i2c_dev *dev)
+{
+	writel((dev->sda_hold_time << 1) |
MSCC_ICPU_CFG_TWI_DELAY_ENABLE,
+	       dev->base_ext + MSCC_ICPU_CFG_TWI_DELAY);
+
+	return 0;
+}

Hmm... And does how this make native DesignWare IP's registers obsolete?

+	if (of_device_is_compatible(pdev->dev.of_node, "mscc,ocelot-
i2c"))

Can't you just ask for this unconditionally? Why not?
(It seems I might have known why not, but can we use named resource
instead in case this is not so SoC specific)


-- 
Andy Shevchenko [off-list ref]
Intel Finland Oy