Re: [PATCH v4 2/3] ARM: mvebu: Add quirk for i2c

[Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 1692 bytes --]

On Wed, Jan 08, 2014 at 11:15:05AM +0100, Gregory CLEMENT wrote:
> Hi Wolfram,
> 
> On 08/01/2014 00:06, Wolfram Sang wrote:
> > On Tue, Jan 07, 2014 at 11:38:53AM -0700, Jason Gunthorpe wrote:
> >> On Tue, Jan 07, 2014 at 05:35:03PM +0100, Gregory CLEMENT wrote:
> >>> +static struct property i2c_offload_broken = {
> >>> +	.name = "offload-broken",
> >>> +};
> >>> +
> >>> +static void __init i2c_quirk(void)
> >>> +{
> >>> +	struct device_node *np;
> >>> +	u32 dev, rev;
> >>> +
> >>> +	/*
> >>> +	 * Only revisons more recent than A0 support the offload
> >>> +	 * mechanism. We can exit only if we are sure that we can
> >>> +	 * get the SoC revision and it is more recent than A0.
> >>> +	 */
> >>> +	if (mvebu_get_soc_id(&rev, &dev) == 0 && dev > MV78XX0_A0_REV)
> >>> +		return;
> >>> +
> >>> +	for_each_compatible_node(np, NULL, "marvell,mv78230-i2c")
> >>> +		of_add_property(np, &i2c_offload_broken);
> >>
> >> I like this approach.
> > 
> > Sorry, but I don't.
> > 
> >> However, when I first read this I thought it should be a -a0 specific
> >> compatible string, not a 'offload-broken' property - any idea what the
> >> DT consensus is here? I've seen both approach in use ..
> > 
> > I prefer the replacement of the compatible string. If it should really
> > be a seperate property, then it should be a vendor specific property. It
> > is not generic, at all.
> 
> Something like "marvell,offload-broken" would be acceptable?

A tad more, yes. Still, since this is a feature/quirk of the IP core
revision, it should be deduced from the compatible property IMO. It
cannot be configured anywhere, so it doesn't change on board level.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]