linux-next: manual merge of the arm-soc tree with the i2c-embedded tree

Hi all,

Today's linux-next merge of the arm-soc tree got a conflict in
drivers/i2c/busses/i2c-nomadik.c between commit af97bace2cca
("i2c-nomadik: move header to <linux/platform_data/i2c-nomadik.h>") from
the i2c-embedded tree and commits 32e42c687e0a ("ARM: ux500: Remove
unused i2c platform_data initialisation code") and 8214fd238a66 ("i2c:
Add Device Tree support to the Nomadik I2C driver") from the arm-soc tree.

I fixed it up (I think - see below) and can carry the fix as necessary.
Please check this and talk to each other ...

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc drivers/i2c/busses/i2c-nomadik.c
index e6b93f3,5471303..0000000

--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c

@@@ -24,8 -23,10 +24,9 @@@
  #include <linux/io.h>
  #include <linux/regulator/consumer.h>
  #include <linux/pm_runtime.h>
 +#include <linux/platform_data/i2c-nomadik.h>
+ #include <linux/of.h>
  
 -#include <plat/i2c.h>
 -
  #define DRIVER_NAME "nmk-i2c"
  
  /* I2C Controller register offsets */
@@@ -911,23 -896,93 +908,94 @@@ static const struct i2c_algorithm nmk_i
  	.functionality	= nmk_i2c_functionality
  };
  
+ static struct nmk_i2c_controller u8500_i2c = {
+ 	/*
+ 	 * Slave data setup time; 250ns, 100ns, and 10ns, which
+ 	 * is 14, 6 and 2 respectively for a 48Mhz i2c clock.
+ 	 */
+ 	.slsu           = 0xe,
+ 	.tft            = 1,      /* Tx FIFO threshold */
+ 	.rft            = 8,      /* Rx FIFO threshold */
+ 	.clk_freq       = 400000, /* std. mode operation */
+ 	.timeout        = 200,    /* Slave response timeout(ms) */
+ 	.sm             = I2C_FREQ_MODE_FAST,
+ };
+ 
+ static int __devinit
+ nmk_i2c_of_probe(struct device_node *np, struct nmk_i2c_controller *pdata)
+ {
+ 	of_property_read_u32(np, "clock-frequency", (u32*)&pdata->clk_freq);
+ 	if (!pdata->clk_freq) {
+ 		pr_warn("%s: Clock frequency not found\n", np->full_name);
+ 		return -EINVAL;
+ 	}
+ 
+ 	of_property_read_u32(np, "stericsson,slsu", (u32*)&pdata->slsu);
+ 	if (!pdata->slsu) {
+ 		pr_warn("%s: Data line delay not found\n", np->full_name);
+ 		return -EINVAL;
+ 	}
+ 
+ 	of_property_read_u32(np, "stericsson,tft", (u32*)&pdata->tft);
+ 	if (!pdata->tft) {
+ 		pr_warn("%s: Tx FIFO threshold not found\n", np->full_name);
+ 		return -EINVAL;
+ 	}
+ 
+ 	of_property_read_u32(np, "stericsson,rft", (u32*)&pdata->rft);
+ 	if (!pdata->rft) {
+ 		pr_warn("%s: Rx FIFO threshold not found\n", np->full_name);
+ 		return -EINVAL;
+ 	}
+ 
+ 	of_property_read_u32(np, "stericsson,timeout", (u32*)&pdata->timeout);
+ 	if (!pdata->timeout) {
+ 		pr_warn("%s: Timeout not found\n", np->full_name);
+ 		return -EINVAL;
+ 	}
+ 
+ 	/*
+ 	 * This driver only supports fast and standard frequency
+ 	 * modes. If anything else is requested fall-back to standard.
+ 	 */
+ 	if (of_get_property(np, "stericsson,i2c_freq_mode_fast", NULL))
+ 		pdata->sm = I2C_FREQ_MODE_FAST;
+ 	else
+ 		pdata->sm = I2C_FREQ_MODE_STANDARD;
+ 
+ 	return 0;
+ }
+ 
 -static int __devinit nmk_i2c_probe(struct platform_device *pdev)
 +static atomic_t adapter_id = ATOMIC_INIT(0);
 +
 +static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
  {
  	int ret = 0;
- 	struct nmk_i2c_controller *pdata =
- 			adev->dev.platform_data;
 -	struct resource *res;
 -	struct nmk_i2c_controller *pdata = pdev->dev.platform_data;
 -	struct device_node *np = pdev->dev.of_node;
++	struct nmk_i2c_controller *pdata = adev->dev.platform_data;
++	struct device_node *np = adev->dev.of_node;
  	struct nmk_i2c_dev	*dev;
  	struct i2c_adapter *adap;
  
- 	if (!pdata) {
- 		dev_warn(&adev->dev, "no platform data\n");
- 		return -ENODEV;
+ 	if (np) {
+ 		if (!pdata) {
 -			pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
++			pdata = devm_kzalloc(&adev->dev, sizeof(*pdata), GFP_KERNEL);
+ 			if (!pdata) {
+ 				ret = -ENOMEM;
+ 				goto err_no_mem;
+ 			}
+ 		}
+ 		ret = nmk_i2c_of_probe(np, pdata);
+ 		if (ret)
+ 			kfree(pdata);
  	}
+ 
+ 	if (!pdata)
+ 		/* No i2c configuration found, using the default. */
+ 		pdata = &u8500_i2c;
+ 
  	dev = kzalloc(sizeof(struct nmk_i2c_dev), GFP_KERNEL);
  	if (!dev) {
 -		dev_err(&pdev->dev, "cannot allocate memory\n");
 +		dev_err(&adev->dev, "cannot allocate memory\n");
  		ret = -ENOMEM;
  		goto err_no_mem;
  	}
@@@ -969,12 -1037,12 +1037,11 @@@
  	adap->owner	= THIS_MODULE;
  	adap->class	= I2C_CLASS_HWMON | I2C_CLASS_SPD;
  	adap->algo	= &nmk_i2c_algo;
- 	adap->timeout	= pdata->timeout ? msecs_to_jiffies(pdata->timeout) :
- 		msecs_to_jiffies(20000);
+ 	adap->timeout	= msecs_to_jiffies(pdata->timeout);
 +	adap->nr = atomic_read(&adapter_id);
  	snprintf(adap->name, sizeof(adap->name),
 -		 "Nomadik I2C%d at %lx", pdev->id, (unsigned long)res->start);
 -
 -	/* fetch the controller id */
 -	adap->nr	= pdev->id;
 +		 "Nomadik I2C%d at %pR", adap->nr, &adev->res);
 +	atomic_inc(&adapter_id);
  
  	/* fetch the controller configuration from machine */
  	dev->cfg.clk_freq = pdata->clk_freq;
@@@ -1040,29 -1110,20 +1107,35 @@@ static int nmk_i2c_remove(struct amba_d
  	return 0;
  }
  
 +static struct amba_id nmk_i2c_ids[] = {
 +	{
 +		.id	= 0x00180024,
 +		.mask	= 0x00ffffff,
 +	},
 +	{
 +		.id	= 0x00380024,
 +		.mask	= 0x00ffffff,
 +	},
 +	{},
 +};
 +
 +MODULE_DEVICE_TABLE(amba, nmk_i2c_ids);
 +
+ static const struct of_device_id nmk_gpio_match[] = {
+ 	{ .compatible = "st,nomadik-i2c", },
+ 	{},
+ };
+ 
 -static struct platform_driver nmk_i2c_driver = {
 -	.driver = {
 +static struct amba_driver nmk_i2c_driver = {
 +	.drv = {
  		.owner = THIS_MODULE,
  		.name = DRIVER_NAME,
  		.pm = &nmk_i2c_pm,
+ 		.of_match_table = nmk_gpio_match,
  	},
 +	.id_table = nmk_i2c_ids,
  	.probe = nmk_i2c_probe,
 -	.remove = __devexit_p(nmk_i2c_remove),
 +	.remove = nmk_i2c_remove,
  };
  
  static int __init nmk_i2c_init(void)

Hi all,

On Tue, 10 Jul 2012 16:41:30 +1000 Stephen Rothwell [off-list ref] wrote:

Hi all,

Today's linux-next merge of the arm-soc tree got a conflict in
drivers/i2c/busses/i2c-nomadik.c between commit af97bace2cca
("i2c-nomadik: move header to <linux/platform_data/i2c-nomadik.h>") from

Also commit 235602146ec9 ("i2c-nomadik: turn the platform driver to an
amba driver")

the i2c-embedded tree and commits 32e42c687e0a ("ARM: ux500: Remove
unused i2c platform_data initialisation code") and 8214fd238a66 ("i2c:
Add Device Tree support to the Nomadik I2C driver") from the arm-soc tree.

I fixed it up (I think - see below) and can carry the fix as necessary.
Please check this and talk to each other ...

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

On Tue, Jul 10, 2012 at 04:41:30PM +1000, Stephen Rothwell wrote:

Hi all,

Today's linux-next merge of the arm-soc tree got a conflict in
drivers/i2c/busses/i2c-nomadik.c between commit af97bace2cca
("i2c-nomadik: move header to <linux/platform_data/i2c-nomadik.h>") from
the i2c-embedded tree and commits 32e42c687e0a ("ARM: ux500: Remove
unused i2c platform_data initialisation code") and 8214fd238a66 ("i2c:
Add Device Tree support to the Nomadik I2C driver") from the arm-soc tree.

I fixed it up (I think - see below) and can carry the fix as necessary.
Please check this and talk to each other ...

Hmm, I can't recall any discussion that the DT support patch should go
in via arm-soc. Did I miss something? Right now, I'd think it should go
in via i2c. I am fine with the other patch going in via arm-soc, but can
carry it as well.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

I fixed it up (I think - see below) and can carry the fix as necessary.
Please check this and talk to each other ...

I also noticed the bindings today [1]. They came in via a seperate
patch (suboptimal) which has no ack by a devicetree maintainer which I'd
really like to see here, because the bindings look suspicious to me.
They look like they mostly export register settings which is usually
questionable since we might want to abstract bindings so they can be
useful for a number of drivers.

Arnd, since you committed the patches, can you please comment? I'd
prefer to drop this DT conversion for now, otherwise we might have to
support this possibly rushed bindings forever? LinusW, what do you
think?

Thanks,

   Wolfram

[1] http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=commitdiff;h=87a174f3d7cea1b9c7596911c39616768f441629

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

On Thursday 12 July 2012, Wolfram Sang wrote:

quoted

I fixed it up (I think - see below) and can carry the fix as necessary.
Please check this and talk to each other ...

I also noticed the bindings today [1]. They came in via a seperate
patch (suboptimal) which has no ack by a devicetree maintainer which I'd
really like to see here, because the bindings look suspicious to me.
They look like they mostly export register settings which is usually
questionable since we might want to abstract bindings so they can be
useful for a number of drivers.

Arnd, since you committed the patches, can you please comment? I'd
prefer to drop this DT conversion for now, otherwise we might have to
support this possibly rushed bindings forever? LinusW, what do you
think?

I've dropped the ux500/dt branch for now. and will wait for a new one
to get submitted as a replacement. There were other things wrong with
the submission that Lee already knows about, so I think it's better
to do this the proper way now.

	Arnd

quoted

They look like they mostly export register settings which is usually
questionable since we might want to abstract bindings so they can be
useful for a number of drivers.

I'm not exactly sure what you mean by this. These bindings are specific 
to the ST-Ericsson driver and are localised, hence the 'stericsson,' 
vendor prefix. If you think other drivers might be able to make use of 
them then we can make them official i2c bindings and drop the 'stericsson,'.

quoted

Arnd, since you committed the patches, can you please comment? I'd
prefer to drop this DT conversion for now, otherwise we might have to
support this possibly rushed bindings forever? LinusW, what do you
think?

We would never have to support them forever. Even if they were deemed to 
be incorrect, or some other vendors wanted to use the same bindings, we 
would just send a patch fixing them.

I've dropped the ux500/dt branch for now. and will wait for a new one
to get submitted as a replacement. There were other things wrong with
the submission that Lee already knows about, so I think it's better
to do this the proper way now.

Okay, I've pulled the i2c patches out. I'll re-send the pull-request 
(less the i2c stuff) when we have an Ack from Sam on the AB8500 
documentation patch.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
M: +44 77 88 633 515
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

On Thu, Jul 12, 2012 at 3:12 PM, Wolfram Sang [off-list ref] wrote:

Arnd, since you committed the patches, can you please comment? I'd
prefer to drop this DT conversion for now, otherwise we might have to
support this possibly rushed bindings forever? LinusW, what do you
think?

Well I think I ACKed that from the point of view that it will work as
expected with ux500 with these bindings. What is best from the I2C
subsystem point of view is another question ...

Overall I think we have this general problem with a lot of DT
conversion happening right now: the tempo is set very high and
all chip vendors want DT support RealQuickNowPreferrablyYesterday
and that makes it hard for subsystem maintainers to hold back,
and I also fear vendor-specific properties are overused for this
reason.

And about the perpetual nature of device tree bindings it
appears to me that the modus operandi right now is to not
regard any of these as written in stone until they are removed
from the kernel tree. We have plenty of drivers patching
trees and drivers in one for the moment.

Yours,
Linus Walleij

Well I think I ACKed that from the point of view that it will work as
expected with ux500 with these bindings. What is best from the I2C
subsystem point of view is another question ...

Okay, thanks for clarifying.

Overall I think we have this general problem with a lot of DT
conversion happening right now: the tempo is set very high and
all chip vendors want DT support RealQuickNowPreferrablyYesterday
and that makes it hard for subsystem maintainers to hold back,
and I also fear vendor-specific properties are overused for this
reason.

Word.

And about the perpetual nature of device tree bindings it
appears to me that the modus operandi right now is to not
regard any of these as written in stone until they are removed
from the kernel tree. We have plenty of drivers patching
trees and drivers in one for the moment.

I don't get this one. Yes, they are of perpetual nature, so how could we
remove them from the kernel tree?

What I am afraid of is: tentative solutions tend to stay, because the
need for a proper solution is reduced. Yet, finding proper generic
bindings might take some time which doesn't meet the high pressure
around DT at the moment.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

On 16/07/12 11:17, Wolfram Sang wrote:

quoted

Well I think I ACKed that from the point of view that it will work as
expected with ux500 with these bindings. What is best from the I2C
subsystem point of view is another question ...

Okay, thanks for clarifying.

quoted

Overall I think we have this general problem with a lot of DT
conversion happening right now: the tempo is set very high and
all chip vendors want DT support RealQuickNowPreferrablyYesterday
and that makes it hard for subsystem maintainers to hold back,
and I also fear vendor-specific properties are overused for this
reason.

Word.

quoted

And about the perpetual nature of device tree bindings it
appears to me that the modus operandi right now is to not
regard any of these as written in stone until they are removed
from the kernel tree. We have plenty of drivers patching
trees and drivers in one for the moment.

I don't get this one. Yes, they are of perpetual nature, so how could we
remove them from the kernel tree?

What I am afraid of is: tentative solutions tend to stay, because the
need for a proper solution is reduced. Yet, finding proper generic
bindings might take some time which doesn't meet the high pressure
around DT at the moment.

I agree with what you say to some extent, but I believe that it is more 
important to have a working solution now than to ensure that each 
bindings are as unique as possible. After any suggestion of 
consolidation, a move from vendor specific to generically defined Device 
Tree bindings is trivial. Especially in the current stage where 
adaptions and definitions are still fluid.

Obviously some care is taken to ensure the bindings are as generic as 
possible, but not to the extent that will put the project back some 
months. Over past few months I have enabled many sub-systems; however, I 
think it would have been a fraction of that if we'd gone through the 
laborious process of immediate forced consolidation. I think it's fine 
to have platform/vendor specific bindings that work, then come back to 
unify them once the dust settles.

If you know of any bindings which you know are generic, I'm happy to 
define and swap them out for the ones I've proposed, but due to a change 
of project focus I can't afford to spend hours studying all of the 
drivers to match-up possible unifications.

Kind regards,
Lee

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
M: +44 77 88 633 515
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

On Mon, Jul 16, 2012 at 12:17 PM, Wolfram Sang [off-list ref] wrote:

quoted

And about the perpetual nature of device tree bindings it
appears to me that the modus operandi right now is to not
regard any of these as written in stone until they are removed
from the kernel tree. We have plenty of drivers patching
trees and drivers in one for the moment.

I don't get this one. Yes, they are of perpetual nature, so how could we
remove them from the kernel tree?

What I meant was that at the point when arch/arm/boot/dts/*
is (if ever) removed from the kernel tree and into its own git, so that
the standardization of bindings is decoupled from the Linux kernel
tree, from that point it is no longer possible to alter the bindings
and the code in sync, so at that point the bindings need to be
frozen and all subsequent work will need to gear down and
work on bindings before deployment.

That said, this does not at all solve the problem of already-deployed
device trees using old bindings...

What I am afraid of is: tentative solutions tend to stay, because the
need for a proper solution is reduced. Yet, finding proper generic
bindings might take some time which doesn't meet the high pressure
around DT at the moment.

I'm +1 on this. But I have learned that I have had to strike a
compromise with people who want to forge ahead. They see things
in the other way: perpetual committee discussions and no code
nor device trees being deployed... :-)

Yours,
Linus Walleij

On Mon, Jul 16, 2012 at 01:37:13PM +0200, Linus Walleij wrote:

On Mon, Jul 16, 2012 at 12:17 PM, Wolfram Sang [off-list ref] wrote:

quoted

quoted

And about the perpetual nature of device tree bindings it
appears to me that the modus operandi right now is to not
regard any of these as written in stone until they are removed
from the kernel tree. We have plenty of drivers patching
trees and drivers in one for the moment.

I don't get this one. Yes, they are of perpetual nature, so how could we
remove them from the kernel tree?

What I meant was that at the point when arch/arm/boot/dts/*
is (if ever) removed from the kernel tree and into its own git, so that
the standardization of bindings is decoupled from the Linux kernel
tree, from that point it is no longer possible to alter the bindings
and the code in sync, so at that point the bindings need to be
frozen and all subsequent work will need to gear down and
work on bindings before deployment.

Uhm, I seem to have missed that bindings are deemed more "flexible" as
long as they are coupled to in-kernel dts files? Is that discussed
somewhere? I do wonder about it...

That said, this does not at all solve the problem of already-deployed
device trees using old bindings...

... exactly because of this. I don't think it is possible to really drop
a binding. Only to mark them deprecated. Which can really be messy in
drivers.

quoted

What I am afraid of is: tentative solutions tend to stay, because the
need for a proper solution is reduced. Yet, finding proper generic
bindings might take some time which doesn't meet the high pressure
around DT at the moment.

I'm +1 on this. But I have learned that I have had to strike a
compromise with people who want to forge ahead. They see things
in the other way: perpetual committee discussions and no code
nor device trees being deployed... :-)

I do know both sides. I easily agree that we have to find a balance
somewhere to meet both interests. Though, currently my impressions from
maintaining I2C are:

a) it is not balanced, but too far on the "let's deploy stuff" side
b) developers have a tendency to simply map platform_data to bindings
   and are surprised when told this is often not possible

which adds burden to the maintainers (who sometimes might not even be
familiar with devicetree because they are not focused on embedded). And
I worry about bindings of unmaintained subsystems.

So, I guess, what I am basically hoping for is less pace and a wider
acceptance that bindings can be really non-trivial.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

quoted

What I am afraid of is: tentative solutions tend to stay, because the
need for a proper solution is reduced. Yet, finding proper generic
bindings might take some time which doesn't meet the high pressure
around DT at the moment.

I agree with what you say to some extent, but I believe that it is
more important to have a working solution now than to ensure that
each bindings are as unique as possible. After any suggestion of
consolidation, a move from vendor specific to generically defined
Device Tree bindings is trivial. Especially in the current stage
where adaptions and definitions are still fluid.

See my response to Linus. I do understand your view and where it comes
from. As a maintainer, I have other priorities. No offence involved,
it needs some settlement.

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

On 16/07/12 14:00, Wolfram Sang wrote:

quoted

quoted

What I am afraid of is: tentative solutions tend to stay, because the
need for a proper solution is reduced. Yet, finding proper generic
bindings might take some time which doesn't meet the high pressure
around DT at the moment.

I agree with what you say to some extent, but I believe that it is
more important to have a working solution now than to ensure that
each bindings are as unique as possible. After any suggestion of
consolidation, a move from vendor specific to generically defined
Device Tree bindings is trivial. Especially in the current stage
where adaptions and definitions are still fluid.

See my response to Linus. I do understand your view and where it comes
from. As a maintainer, I have other priorities. No offence involved,
it needs some settlement.

I'm certainly not adverse to doing this, although I'd prefer it was 
completed in the short-term. So should we do it?

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
M: +44 77 88 633 515
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

On Mon, Jul 16, 2012 at 2:35 PM, Wolfram Sang [off-list ref] wrote:

Uhm, I seem to have missed that bindings are deemed more "flexible" as
long as they are coupled to in-kernel dts files? Is that discussed
somewhere? I do wonder about it...

Well patches like this are sent out but not commented on from
the perspective of binding stability. So of course, they get merged.

Have a look at this commit:

commit 7f217794ffa72f208a250b79ab0b7ea3de19677f
Author: Arnd Bergmann [off-list ref]
Date:   Sun May 13 00:14:24 2012 -0400

    mmc: dt: Consolidate DT bindings

This is deleting custom properties from DTS files without
adding any code to fallback-support them on old device trees.

(Actually I like this commit a lot, since it is removing custom MMC
props and replacing these with standard bindings common for all
MMC/SD controllers, but ...)

I think there are more examples if you git log on arch/arm/boot/dts...

Yours,
Linus Walleij

On Mon, Jul 16, 2012 at 12:31:08PM +0100, Lee Jones wrote:

I agree with what you say to some extent, but I believe that it is
more important to have a working solution now than to ensure that
each bindings are as unique as possible. After any suggestion of
consolidation, a move from vendor specific to generically defined
Device Tree bindings is trivial. Especially in the current stage
where adaptions and definitions are still fluid.

Obviously some care is taken to ensure the bindings are as generic
as possible, but not to the extent that will put the project back
some months. Over past few months I have enabled many sub-systems;

It's not just about having generic bindings, it's also about having
bindings which have some abstraction and hope of reusability. An awful
lot of bindings are just straight dumps of Linux data structures into
the device tree which don't make a terribly great deal of sense as
bindings.

however, I think it would have been a fraction of that if we'd gone
through the laborious process of immediate forced consolidation. I
think it's fine to have platform/vendor specific bindings that work,
then come back to unify them once the dust settles.

In many of these cases we'd be better off just not putting things into
the device tree in the first place, leaving things at the basic "is the
device there" stuff.

On Mon, Jul 16, 2012 at 02:35:50PM +0200, Wolfram Sang wrote:

I do know both sides. I easily agree that we have to find a balance
somewhere to meet both interests. Though, currently my impressions from
maintaining I2C are:

a) it is not balanced, but too far on the "let's deploy stuff" side
b) developers have a tendency to simply map platform_data to bindings
   and are surprised when told this is often not possible

which adds burden to the maintainers (who sometimes might not even be
familiar with devicetree because they are not focused on embedded). And
I worry about bindings of unmaintained subsystems.

A issue I'm seeing with some of the subsystems mantained by people who
don't work on ARM is that they've got the DT message so strongly that
they're pushing back on people trying to add platform data at all even
on off SoC things that need to work with non-ARM processors.

On 17/07/12 14:06, Mark Brown wrote:

On Mon, Jul 16, 2012 at 12:31:08PM +0100, Lee Jones wrote:

quoted

I agree with what you say to some extent, but I believe that it is
more important to have a working solution now than to ensure that
each bindings are as unique as possible. After any suggestion of
consolidation, a move from vendor specific to generically defined
Device Tree bindings is trivial. Especially in the current stage
where adaptions and definitions are still fluid.

quoted

Obviously some care is taken to ensure the bindings are as generic
as possible, but not to the extent that will put the project back
some months. Over past few months I have enabled many sub-systems;

It's not just about having generic bindings, it's also about having
bindings which have some abstraction and hope of reusability. An awful
lot of bindings are just straight dumps of Linux data structures into
the device tree which don't make a terribly great deal of sense as
bindings.

The Device Tree should supply any platform configuration which the 
driver needs in order to correctly setup for a particular machine. This 
is exactly what the platform_data structure did before, hence is is 
reasonable to assume that whatever information resides in that structure 
would be required in the Device Tree.

quoted

however, I think it would have been a fraction of that if we'd gone
through the laborious process of immediate forced consolidation. I
think it's fine to have platform/vendor specific bindings that work,
then come back to unify them once the dust settles.

In many of these cases we'd be better off just not putting things into
the device tree in the first place, leaving things at the basic "is the
device there" stuff.

Then what do you do with the platform data?

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
M: +44 77 88 633 515
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

On Tue, Jul 17, 2012 at 02:30:01PM +0100, Lee Jones wrote:

On 17/07/12 14:06, Mark Brown wrote:

quoted

It's not just about having generic bindings, it's also about having
bindings which have some abstraction and hope of reusability. An awful
lot of bindings are just straight dumps of Linux data structures into
the device tree which don't make a terribly great deal of sense as
bindings.

The Device Tree should supply any platform configuration which the
driver needs in order to correctly setup for a particular machine.
This is exactly what the platform_data structure did before, hence
is is reasonable to assume that whatever information resides in that
structure would be required in the Device Tree.

An *awful* lot of what people are trying to put into platform data is
nothing to do with that, it's just the generic data the driver needs to
be able to understand the hardware at all.  Things like the MFD
breakdown, random parameters of the hardware which you can infer from
the device name and so on.

On 17/07/12 14:35, Mark Brown wrote:

On Tue, Jul 17, 2012 at 02:30:01PM +0100, Lee Jones wrote:

quoted

On 17/07/12 14:06, Mark Brown wrote:

quoted

quoted

It's not just about having generic bindings, it's also about having
bindings which have some abstraction and hope of reusability. An awful
lot of bindings are just straight dumps of Linux data structures into
the device tree which don't make a terribly great deal of sense as
bindings.

quoted

The Device Tree should supply any platform configuration which the
driver needs in order to correctly setup for a particular machine.
This is exactly what the platform_data structure did before, hence
is is reasonable to assume that whatever information resides in that
structure would be required in the Device Tree.

An *awful* lot of what people are trying to put into platform data is
nothing to do with that, it's just the generic data the driver needs to
be able to understand the hardware at all.  Things like the MFD
breakdown, random parameters of the hardware which you can infer from
the device name and so on.

I can only speak from a personal perspective on that one. I do _try_ not 
to put anything in there (platform data or Device Tree), which does not 
belong. I have no idea how successful that notion was however.

I'm sure sure this is relevant in the current case though, as the i2c 
properties proposed here are platform specific. What we're discussing is 
some consolidation of property names, which I do support in theory. What 
I fear is that this driver will lack Device Tree functionality for yet 
another kernel version if it isn't resolved quickly.

Wolfram, are you theorising about these the multiple use of these 
properties, or do you have some examples? I think we would do well to 
work on these, or else they're just going to lie dormant and not get done.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
M: +44 77 88 633 515
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

On Tue, Jul 17, 2012 at 03:02:00PM +0100, Lee Jones wrote:

I'm sure sure this is relevant in the current case though, as the
i2c properties proposed here are platform specific. What we're

I've not seen the specific example (though fankly it seems quite
surprising that there's anything other than bus speed that the platform
might want to configure for I2C...).

discussing is some consolidation of property names, which I do
support in theory. What I fear is that this driver will lack Device
Tree functionality for yet another kernel version if it isn't
resolved quickly.

Well, if checking the DT checky box is the important thing then just
adding an of_match_table ought to be enough?  It's fairly common for
platform data to have lots of stuff that's not used by most systems
so you can often cover 90% of systems with a very small subset of the 
configurability.

On 17/07/12 15:22, Mark Brown wrote:

On Tue, Jul 17, 2012 at 03:02:00PM +0100, Lee Jones wrote:

quoted

I'm sure sure this is relevant in the current case though, as the
i2c properties proposed here are platform specific. What we're

I've not seen the specific example (though fankly it seems quite
surprising that there's anything other than bus speed that the platform
might want to configure for I2C...).

quoted

discussing is some consolidation of property names, which I do
support in theory. What I fear is that this driver will lack Device
Tree functionality for yet another kernel version if it isn't
resolved quickly.

Well, if checking the DT checky box is the important thing then just
adding an of_match_table ought to be enough?  It's fairly common for
platform data to have lots of stuff that's not used by most systems
so you can often cover 90% of systems with a very small subset of the
configurability.

We could do that, but I'd rather do it in full.

I think it would be okay to take the 3 patches due for the v3.6 merge 
window, which target the nmk-i2c driver. If any consolidation happens in 
the mean-time/future I will personally do the work to bring the nmk-i2c 
into line to use any generic bindings which result.

No one else is ever going to use these vendor specific bindings, so I'm 
sure no issues will arise. It also means that we have a fully enabled 
driver which is capable of receiving a new configuration via minor 
changes to the dts, which is important for the current (only) user of 
this driver for an upcoming project.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
M: +44 77 88 633 515
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

On Tue, Jul 17, 2012 at 03:52:10PM +0100, Lee Jones wrote:

I think it would be okay to take the 3 patches due for the v3.6
merge window, which target the nmk-i2c driver. If any consolidation
happens in the mean-time/future I will personally do the work to
bring the nmk-i2c into line to use any generic bindings which
result.

No one else is ever going to use these vendor specific bindings, so
I'm sure no issues will arise. It also means that we have a fully
enabled driver which is capable of receiving a new configuration via
minor changes to the dts, which is important for the current (only)
user of this driver for an upcoming project.

So, this is pretty much what vendors always say about this stuff...

Looking at what's specified in the platform data in the current kernel
it seems like there's a mixture of things in there that are board and
silicon specific.  We've got parameters like the the speed mode and
timeout which are reasonably board specific but we've also got things
like the FIFO sizes which shouldn't be at all board specific and slsu
which really looks like it's something that that the driver ought to be
able to figure out for itself.

On Tue, Jul 17, 2012 at 04:20:01PM +0100, Mark Brown wrote:

Looking at what's specified in the platform data in the current kernel
it seems like there's a mixture of things in there that are board and
silicon specific.  We've got parameters like the the speed mode and
timeout which are reasonably board specific but we've also got things
like the FIFO sizes which shouldn't be at all board specific and slsu
which really looks like it's something that that the driver ought to be
able to figure out for itself.

I have an IP block getting different FIFO size on different IMX SoCs.
We could introduce a new compatible string for driver to figure it out,
but I think the simpler way is just have the data encoded in device
tree.  In any case DT is not completely limited in encoding board
specific data.  Today, we have IO region and interrupt number of
hardware blocks encoded in DT, and to me, FIFO size could just be
another aspect of hardware block we could choose to encode in DT.

-- 
Regards,
Shawn

On 17/07/12 16:20, Mark Brown wrote:

On Tue, Jul 17, 2012 at 03:52:10PM +0100, Lee Jones wrote:

quoted

I think it would be okay to take the 3 patches due for the v3.6
merge window, which target the nmk-i2c driver. If any consolidation
happens in the mean-time/future I will personally do the work to
bring the nmk-i2c into line to use any generic bindings which
result.

quoted

No one else is ever going to use these vendor specific bindings, so
I'm sure no issues will arise. It also means that we have a fully
enabled driver which is capable of receiving a new configuration via
minor changes to the dts, which is important for the current (only)
user of this driver for an upcoming project.

So, this is pretty much what vendors always say about this stuff...

I agree, but in this instance it really does stand to reason.

1. No unified bindings currently exist.
2. I don't have time to create them.
3. It will probably take quite a bit of time for someone else to get round to creating them.
4. The bindings I'm proposing are siloed by vendor and driver, so will cause no harm.
5. I've already volunteered to move them over to the unified ones once created.
6. These allow support for the driver to work with DT, at the moment it does not.

Personally, I think there is more to be gained by applying the (working) vendor specific bindings to the vendor specific driver until some more consolidated ones appear.

Looking at what's specified in the platform data in the current kernel
it seems like there's a mixture of things in there that are board and
silicon specific.  We've got parameters like the the speed mode and
timeout which are reasonably board specific but we've also got things
like the FIFO sizes which shouldn't be at all board specific and slsu
which really looks like it's something that that the driver ought to be
able to figure out for itself.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
M: +44 77 88 633 515 
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

On Wed, Jul 18, 2012 at 01:33:42PM +0800, Shawn Guo wrote:

I have an IP block getting different FIFO size on different IMX SoCs.
We could introduce a new compatible string for driver to figure it out,
but I think the simpler way is just have the data encoded in device
tree.  In any case DT is not completely limited in encoding board
specific data.  Today, we have IO region and interrupt number of
hardware blocks encoded in DT, and to me, FIFO size could just be
another aspect of hardware block we could choose to encode in DT.

So, this is part of the problem with encoding the description of the SoC
into the DT - we're not doing a good job of splitting out the silicon
description from the board specific description which is not a triumph
for maintainability since it means that we end up needing to modify the
DT for every board using the silicon if we want to use the new feature
(assuming people maintain binary compatibility with old DTs, which we
don't do a good job at either even for established DT architectures).

It's not the using device tree bit that creates concern for me here,
it's the fact that the board and silicon aren't being separated.

On 18/07/12 10:59, Mark Brown wrote:

On Wed, Jul 18, 2012 at 01:33:42PM +0800, Shawn Guo wrote:

quoted

I have an IP block getting different FIFO size on different IMX SoCs.
We could introduce a new compatible string for driver to figure it out,
but I think the simpler way is just have the data encoded in device
tree.  In any case DT is not completely limited in encoding board
specific data.  Today, we have IO region and interrupt number of
hardware blocks encoded in DT, and to me, FIFO size could just be
another aspect of hardware block we could choose to encode in DT.

So, this is part of the problem with encoding the description of the SoC
into the DT - we're not doing a good job of splitting out the silicon
description from the board specific description which is not a triumph
for maintainability since it means that we end up needing to modify the
DT for every board using the silicon if we want to use the new feature
(assuming people maintain binary compatibility with old DTs, which we
don't do a good job at either even for established DT architectures).

It's not the using device tree bit that creates concern for me here,
it's the fact that the board and silicon aren't being separated.

What's the difference?

The Device Tree describes the hardware (hardware == silicon + 
peripherals). So different platforms are different pieces of hardware. 
Everything the driver can't figure itself out from the silicon itself 
needs to be described in the DT.

We can describe the relationship with a hierarchical layout of the dts 
and dtsi files. So for my recent venture we have:

+- db8500.dtsi       // silicon
\	
 +-- snowball.dts    // board

We also have plans to split it up further:

          db8xx0.dtsi               // shared silicon
      _________/\_________
     /                    \
db8500.dtsi          db8540.dtsi    // differing silicon
\                    \
 +-- snowball.dts     +-- new_board.dts // board

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
M: +44 77 88 633 515
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

On Wed, Jul 18, 2012 at 11:29:26AM +0100, Lee Jones wrote:

On 18/07/12 10:59, Mark Brown wrote:

quoted

It's not the using device tree bit that creates concern for me here,
it's the fact that the board and silicon aren't being separated.

What's the difference?

quoted

+- db8500.dtsi       // silicon
\	
+-- snowball.dts    // board

I have glanced at some of this stuff in the past, thanks.  Now think how
that's working for people when they put the configuration for the
silicon in the same DT node as the configuration for the board...

On 18/07/12 11:33, Mark Brown wrote:

On Wed, Jul 18, 2012 at 11:29:26AM +0100, Lee Jones wrote:

quoted

On 18/07/12 10:59, Mark Brown wrote:

quoted

quoted

It's not the using device tree bit that creates concern for me here,
it's the fact that the board and silicon aren't being separated.

quoted

What's the difference?

quoted

quoted

+- db8500.dtsi       // silicon
\	
+-- snowball.dts    // board

I have glanced at some of this stuff in the past, thanks.  Now think how
that's working for people when they put the configuration for the
silicon in the same DT node as the configuration for the board...

Granted, ideally the configuration and silicon description should be 
separated. I don't know that is the case for us yet however. That is 
something I'll enforce when we have a new dts file, which will be coming 
very soon.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
M: +44 77 88 633 515
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

On Wed, Jul 18, 2012 at 08:35:21AM +0100, Lee Jones wrote:

Fix your mailer to word wrap within paragraphs.  I've reformatted your
mail for legibility.

I agree, but in this instance it really does stand to reason.

1. No unified bindings currently exist.
2. I don't have time to create them.
3. It will probably take quite a bit of time for someone else to get
   round to creating them.
4. The bindings I'm proposing are siloed by vendor and driver, so will
   cause no harm.

Right, this is just a restatement of the standard vendor line.

If the issue is purely about having generic bindings quite frankly it's
very hard to see how it could take much time or effort to handle the
generic bits for I2C, it's basically just the maximum bus frequency and
possibly also the various fast modes (though to a good approximation it
seems reasonable to just infer them from the bus frequency and then see
if we need any more).  One thing I frequently find is that people say
any sort of generic work is hard without explaining why, if there are
complex issues that's one thing but that's often not the case.

BTW, looking at the platform data again it seems like i2c_freq_mode it
seems very odd that it's driver specific?

5. I've already volunteered to move them over to the unified ones once
   created.
6. These allow support for the driver to work with DT, at the moment
   it does not.

Personally, I think there is more to be gained by applying the
(working) vendor specific bindings to the vendor specific driver until
some more consolidated ones appear.

Again, vendors always make great promises about how they're going to
keep everything up to date...

On 18/07/12 12:12, Mark Brown wrote:

On Wed, Jul 18, 2012 at 08:35:21AM +0100, Lee Jones wrote:

Fix your mailer to word wrap within paragraphs.  I've reformatted your
mail for legibility.

Does it always do that, or was it just this time? It's setup to 
word-wrap, for instance this paragraph should. I have an add-on, which 
disables wrapping, but I only enable that when I send individual patches 
out. I know it used to work, but I have a feeling it's broken.

quoted

I agree, but in this instance it really does stand to reason.

quoted

1. No unified bindings currently exist.
2. I don't have time to create them.
3. It will probably take quite a bit of time for someone else to get
    round to creating them.
4. The bindings I'm proposing are siloed by vendor and driver, so will
    cause no harm.

Right, this is just a restatement of the standard vendor line.

If the issue is purely about having generic bindings quite frankly it's
very hard to see how it could take much time or effort to handle the
generic bits for I2C, it's basically just the maximum bus frequency and

The frequency is already a generic binding, it's the others which need 
alignment.

possibly also the various fast modes (though to a good approximation it
seems reasonable to just infer them from the bus frequency and then see
if we need any more).  One thing I frequently find is that people say
any sort of generic work is hard without explaining why, if there are
complex issues that's one thing but that's often not the case.

I didn't say it was hard, I was it was time consuming. It would require 
looking at all of the other drivers and picking out bits which are the 
same. An i2c guy would be better to do it. I didn't even know what the 
nmk-i2c ones were (slsu, tft, rft, sm) until I was told my the author. I 
fear the other drivers will be just as cryptic.

BTW, looking at the platform data again it seems like i2c_freq_mode it
seems very odd that it's driver specific?

I agree.

quoted

5. I've already volunteered to move them over to the unified ones once
    created.
6. These allow support for the driver to work with DT, at the moment
    it does not.

quoted

Personally, I think there is more to be gained by applying the
(working) vendor specific bindings to the vendor specific driver until
some more consolidated ones appear.

Again, vendors always make great promises about how they're going to
keep everything up to date...

I'm not a vendor. I also keep my promises. :)

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
M: +44 77 88 633 515
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog