Re: [PATCH v4 3/3] ARM: dts: add I2C device nodes for Broadcom Cygnus

["Uwe Kleine-König" <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>]

Hello,

On Fri, Jan 16, 2015 at 11:24:09AM -0800, Ray Jui wrote:
> >> +	i2c0: i2c@18008000 {
> >> +		compatible = "brcm,iproc-i2c";
> > in patch 2 you wrote the driver is for a family of SoCs, right? Then I'd
> > make this:
> > 
> > 	compatible = "brcm,$mysoc-iproc-i2c", "brcm,iproc-i2c";
> > 
> Sorry could you please help to explain the intention here? Note the
> iProc I2C IP can be found in various iProc family of SoCs, but to my
> best knowledge, there hasn't been any changes of the IP in any of those
> SoCs.
This is just for making the device tree stable in the future. Consider
your gentle hardware engineers "fix" a small issue for the next
generation iproc SoC "pony" that needs an incompatible software change.

Then you can fix the driver without updating the device trees by
switching to the SoC specific compatible string for "pony". And in case
the hardware engineers didn't tell you that there is a change and the
need for the software change is only detected when the machines are
already shipped, you're happy if you can fix your kernel without needing
to change the bootloader that provides the dtb.
So start already today to add the (for now unused) compatible string.

> Is the compatible ID "brcm,$mysoc-iproc-i2c" only to clarify that it's
> for a specific SoC? If so, what should the compatible ID array look
> like? Should it be changed to the following?
> 
> static const struct of_device_id bcm_iproc_i2c_of_match[] = {
> 	{ .compatible = "brcm,iproc-i2c" },
> 	{ .compatible = "brcm,$mysoc-iproc-i2c" },
> 	{},
> };
No, there is no need, see above.

If something is still unclear, don't hesitate to ask.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |