Re: [PATCH 1/2] i2c-designware: make *CNT values configurable
On Mon, Aug 05, 2013 at 12:02:26PM +0200, Wolfram Sang wrote: Would it make sense to add generic I2C device tree properties for those parameters? These parameters are independent of the actual bus driver, rather a PCB property... And as such the correct place would be device tree or ACPI or similar. If there are other bus drivers that make use of tr/tf transition times, I think it makes sense. Wolfram, what's your opinion on this? If it is a PCB property, it makes sense to have generic bindings for it. Can they have very-safe defaults and thus be optional? We can definitely have safe defaults that work for a given driver/hardware. I don't think the same defaults would be safe for all drivers/hardware: The timing strategies of different I2C hardware seems to vary widely (which edges of the clock are sampled, how does different hardware deal with hold times etc) and depending on which parameters are available to the driver to control these timings, the safe values would either have to be the minimum or the maximum in the range allowed by the I2C specification. Every driver would thus have to implement its own defaults in case the properties are not defined. Christian pgpIYNPelPudx.pgp Description: PGP signature
Re: [PATCH 1/2] i2c-designware: make *CNT values configurable
Every driver would thus have to implement its own defaults in case the properties are not defined. Placing the defaults at driver level sounds fine to me. Thanks! -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 答复: [PATCH v3 2/2] i2c: imx: Add Vybrid VF610 I2C controller support
[Adding other devicetree maintainers to Cc]
On Mon, Aug 12, 2013 at 01:56:07PM +0100, Lu Jingchang-B35083 wrote:
发件人: Mark Rutland [mark.rutl...@arm.com]
发送时间: 2013年8月10日 22:08
收件人: Lu Jingchang-B35083
抄送: w...@the-dreams.de; Estevam Fabio-R49496; Li Xiaochun-B41219;
s.ha...@pengutronix.de; linux-i2c@vger.kernel.org; Jin Zhengxiong-R64188;
shawn@linaro.org; linux-arm-ker...@lists.infradead.org
主题: Re: [PATCH v3 2/2] i2c: imx: Add Vybrid VF610 I2C controller support
On Fri, Aug 02, 2013 at 05:44:08AM +0100, Jingchang Lu wrote:
Add Freescale Vybrid VF610 I2C controller support to
imx I2C driver framework.
Some operation is different from imx I2C controller.
The register offset, the i2c clock divider value table,
the module enabling(I2CR_IEN) which is just invert with imx,
and the interrupt flag(I2SR) clearing opcode is w1c on VF610
but w0c on imx.
Signed-off-by: Jason Jin jason@freescale.com
Signed-off-by: Xiaochun Li b41...@freescale.com
Signed-off-by: Jingchang Lu b35...@freescale.com
---
changes in v3:
Using struct naming the i2c clock {div, regval} pair.
Using address shift handling registers address difference.
changes in v2:
Fix building section mismatch(es) warning.
drivers/i2c/busses/i2c-imx.c | 146
---
1 file changed, 122 insertions(+), 24 deletions(-)
[...]
@@ -145,6 +233,7 @@ MODULE_DEVICE_TABLE(platform, imx_i2c_devtype);
static const struct of_device_id i2c_imx_dt_ids[] = {
{ .compatible = fsl,imx1-i2c, .data =
imx_i2c_devtype[IMX1_I2C], },
{ .compatible = fsl,imx21-i2c, .data =
imx_i2c_devtype[IMX21_I2C], },
+ { .compatible = fsl,vf610-i2c, .data =
imx_i2c_devtype[VF610_I2C], },
{ /* sentinel */ }
};
That string doesn't seem to be documented anywhere (from a quick grep of
Documentation/devicetree), and there's no binding update included
here. It would be nice for that to be fixed :)
[Lu Jingchang]
The binding string for i2c-imx driver in
Documentation/devicetree/bindings/i2c/i2c-imx.txt use a wildcard format
of - compatible : Should be fsl,chip-i2c for device using this driver.
Neither fsl,imx1-i2c nor fsl,imx21-i2c
is described in the binding document. So I just leave the vf610 i2c
compatible with this.
I'm not a big fan on wildcards in bindings, as it leaves people free to
put anything in and claim it's a documented binding, and makes it far
harder for an os to actually implement drivers for said binding, as
there's no canonical reference for the set of valid variations.
Obviously there is some precedent, but I'm not sure it's something we
want to stick with, and we can prevent it my updating the documentation
now.
Does anyone else have an opinion?
Mark.
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 答复: [PATCH v3 2/2] i2c: imx: Add Vybrid VF610 I2C controller support
On Monday 12 of August 2013 17:43:54 Mark Rutland wrote:
[Adding other devicetree maintainers to Cc]
On Mon, Aug 12, 2013 at 01:56:07PM +0100, Lu Jingchang-B35083 wrote:
发件人: Mark Rutland [mark.rutl...@arm.com]
发送时间: 2013年8月10日 22:08
收件人: Lu Jingchang-B35083
抄送: w...@the-dreams.de; Estevam Fabio-R49496; Li Xiaochun-B41219;
s.ha...@pengutronix.de; linux-i2c@vger.kernel.org; Jin
Zhengxiong-R64188; shawn@linaro.org;
linux-arm-ker...@lists.infradead.org 主题: Re: [PATCH v3 2/2] i2c:
imx: Add Vybrid VF610 I2C controller support
On Fri, Aug 02, 2013 at 05:44:08AM +0100, Jingchang Lu wrote:
Add Freescale Vybrid VF610 I2C controller support to
imx I2C driver framework.
Some operation is different from imx I2C controller.
The register offset, the i2c clock divider value table,
the module enabling(I2CR_IEN) which is just invert with imx,
and the interrupt flag(I2SR) clearing opcode is w1c on VF610
but w0c on imx.
Signed-off-by: Jason Jin jason@freescale.com
Signed-off-by: Xiaochun Li b41...@freescale.com
Signed-off-by: Jingchang Lu b35...@freescale.com
---
changes in v3:
Using struct naming the i2c clock {div, regval} pair.
Using address shift handling registers address difference.
changes in v2:
Fix building section mismatch(es) warning.
drivers/i2c/busses/i2c-imx.c | 146
--- 1 file changed, 122
insertions(+), 24 deletions(-)
[...]
@@ -145,6 +233,7 @@ MODULE_DEVICE_TABLE(platform, imx_i2c_devtype);
static const struct of_device_id i2c_imx_dt_ids[] = {
{ .compatible = fsl,imx1-i2c, .data =
imx_i2c_devtype[IMX1_I2C], },
{ .compatible = fsl,imx21-i2c, .data =
imx_i2c_devtype[IMX21_I2C], },
+ { .compatible = fsl,vf610-i2c, .data =
imx_i2c_devtype[VF610_I2C], },
{ /* sentinel */ }
};
That string doesn't seem to be documented anywhere (from a quick grep
of Documentation/devicetree), and there's no binding update included
here. It would be nice for that to be fixed :)
[Lu Jingchang]
The binding string for i2c-imx driver in
Documentation/devicetree/bindings/i2c/i2c-imx.txt use a wildcard
format of - compatible : Should be fsl,chip-i2c for device
using this driver. Neither fsl,imx1-i2c nor fsl,imx21-i2c is
described in the binding document. So I just leave the vf610 i2c
compatible with this.
I'm not a big fan on wildcards in bindings, as it leaves people free to
put anything in and claim it's a documented binding, and makes it far
harder for an os to actually implement drivers for said binding, as
there's no canonical reference for the set of valid variations.
Obviously there is some precedent, but I'm not sure it's something we
want to stick with, and we can prevent it my updating the documentation
now.
Does anyone else have an opinion?
In case of Samsung platforms we decided to always use the name of first
SoC in which given IP appeared and list all compatible SoCs in binding
documentation. This is IMHO the most consistent way, as there is no
confusion about IP versions (not always listed in documentation, not even
saying about unavailable documentation) or potential problems with new
SoCs matching the wildcard, but having different IP.
In this particular case, the chip wildcard can be easily transformed
into a non-wildcard binding, by listing all supported chip values, i.e.
adding following text to binding documentation:
8--
The chip can be one of:
- imx1 - for i.MX1 and compatible SoCs,
- imx21 - for i.MX21 and compatible SoCs,
- vf610 - for Vybrid VF610 and compatible SoCs.
--8
Best regards,
Tomasz
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/2] i2c: imx: Add Vybrid VF610 I2C controller support
On Aug 12, 2013, at 11:59 AM, Tomasz Figa wrote:
On Monday 12 of August 2013 17:43:54 Mark Rutland wrote:
[Adding other devicetree maintainers to Cc]
On Mon, Aug 12, 2013 at 01:56:07PM +0100, Lu Jingchang-B35083 wrote:
发件人: Mark Rutland [mark.rutl...@arm.com]
发送时间: 2013年8月10日 22:08
收件人: Lu Jingchang-B35083
抄送: w...@the-dreams.de; Estevam Fabio-R49496; Li Xiaochun-B41219;
s.ha...@pengutronix.de; linux-i2c@vger.kernel.org; Jin
Zhengxiong-R64188; shawn@linaro.org;
linux-arm-ker...@lists.infradead.org 主题: Re: [PATCH v3 2/2] i2c:
imx: Add Vybrid VF610 I2C controller support
On Fri, Aug 02, 2013 at 05:44:08AM +0100, Jingchang Lu wrote:
Add Freescale Vybrid VF610 I2C controller support to
imx I2C driver framework.
Some operation is different from imx I2C controller.
The register offset, the i2c clock divider value table,
the module enabling(I2CR_IEN) which is just invert with imx,
and the interrupt flag(I2SR) clearing opcode is w1c on VF610
but w0c on imx.
Signed-off-by: Jason Jin jason@freescale.com
Signed-off-by: Xiaochun Li b41...@freescale.com
Signed-off-by: Jingchang Lu b35...@freescale.com
---
changes in v3:
Using struct naming the i2c clock {div, regval} pair.
Using address shift handling registers address difference.
changes in v2:
Fix building section mismatch(es) warning.
drivers/i2c/busses/i2c-imx.c | 146
--- 1 file changed, 122
insertions(+), 24 deletions(-)
[...]
@@ -145,6 +233,7 @@ MODULE_DEVICE_TABLE(platform, imx_i2c_devtype);
static const struct of_device_id i2c_imx_dt_ids[] = {
{ .compatible = fsl,imx1-i2c, .data =
imx_i2c_devtype[IMX1_I2C], },
{ .compatible = fsl,imx21-i2c, .data =
imx_i2c_devtype[IMX21_I2C], },
+ { .compatible = fsl,vf610-i2c, .data =
imx_i2c_devtype[VF610_I2C], },
{ /* sentinel */ }
};
That string doesn't seem to be documented anywhere (from a quick grep
of Documentation/devicetree), and there's no binding update included
here. It would be nice for that to be fixed :)
[Lu Jingchang]
The binding string for i2c-imx driver in
Documentation/devicetree/bindings/i2c/i2c-imx.txt use a wildcard
format of - compatible : Should be fsl,chip-i2c for device
using this driver. Neither fsl,imx1-i2c nor fsl,imx21-i2c is
described in the binding document. So I just leave the vf610 i2c
compatible with this.
I'm not a big fan on wildcards in bindings, as it leaves people free to
put anything in and claim it's a documented binding, and makes it far
harder for an os to actually implement drivers for said binding, as
there's no canonical reference for the set of valid variations.
Obviously there is some precedent, but I'm not sure it's something we
want to stick with, and we can prevent it my updating the documentation
now.
Does anyone else have an opinion?
In case of Samsung platforms we decided to always use the name of first
SoC in which given IP appeared and list all compatible SoCs in binding
documentation. This is IMHO the most consistent way, as there is no
confusion about IP versions (not always listed in documentation, not even
saying about unavailable documentation) or potential problems with new
SoCs matching the wildcard, but having different IP.
In this particular case, the chip wildcard can be easily transformed
into a non-wildcard binding, by listing all supported chip values, i.e.
adding following text to binding documentation:
8--
The chip can be one of:
- imx1 - for i.MX1 and compatible SoCs,
- imx21 - for i.MX21 and compatible SoCs,
- vf610 - for Vybrid VF610 and compatible SoCs.
--8
Best regards,
Tomasz
One way to handle this is to introduce a SoC Family chip name doc with the
possible list of names and than reference that doc in the binding. Kinda like
how we have vendor-prefixes.txt.
And maybe instead of doing chip its imx-chip or some reference that isn't
too generic such that in the future a .dts could be validated.
- k
--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by
The Linux Foundation
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 答复: [PATCH v3 2/2] i2c: imx: Add Vybrid VF610 I2C controller support
On 08/12/2013 10:43 AM, Mark Rutland wrote:
[Adding other devicetree maintainers to Cc]
On Mon, Aug 12, 2013 at 01:56:07PM +0100, Lu Jingchang-B35083 wrote:
发件人: Mark Rutland [mark.rutl...@arm.com]
发送时间: 2013年8月10日 22:08
收件人: Lu Jingchang-B35083
抄送: w...@the-dreams.de; Estevam Fabio-R49496; Li Xiaochun-B41219;
s.ha...@pengutronix.de; linux-i2c@vger.kernel.org; Jin Zhengxiong-R64188;
shawn@linaro.org; linux-arm-ker...@lists.infradead.org
主题: Re: [PATCH v3 2/2] i2c: imx: Add Vybrid VF610 I2C controller support
On Fri, Aug 02, 2013 at 05:44:08AM +0100, Jingchang Lu wrote:
Add Freescale Vybrid VF610 I2C controller support to
imx I2C driver framework.
Some operation is different from imx I2C controller.
The register offset, the i2c clock divider value table,
the module enabling(I2CR_IEN) which is just invert with imx,
and the interrupt flag(I2SR) clearing opcode is w1c on VF610
but w0c on imx.
static const struct of_device_id i2c_imx_dt_ids[] = {
{ .compatible = fsl,imx1-i2c, .data =
imx_i2c_devtype[IMX1_I2C], },
{ .compatible = fsl,imx21-i2c, .data =
imx_i2c_devtype[IMX21_I2C], },
+ { .compatible = fsl,vf610-i2c, .data =
imx_i2c_devtype[VF610_I2C], },
That string doesn't seem to be documented anywhere (from a quick grep of
Documentation/devicetree), and there's no binding update included
here. It would be nice for that to be fixed :)
[Lu Jingchang]
The binding string for i2c-imx driver in
Documentation/devicetree/bindings/i2c/i2c-imx.txt use a wildcard format
of - compatible : Should be fsl,chip-i2c for device using this
driver. Neither fsl,imx1-i2c nor fsl,imx21-i2c
is described in the binding document. So I just leave the vf610 i2c
compatible with this.
I'm not a big fan on wildcards in bindings, as it leaves people free to
put anything in and claim it's a documented binding, and makes it far
harder for an os to actually implement drivers for said binding, as
there's no canonical reference for the set of valid variations.
Obviously there is some precedent, but I'm not sure it's something we
want to stick with, and we can prevent it my updating the documentation
now.
Does anyone else have an opinion?
I suppose technically we should list out every exact string in the
binding, but it's a little annoying to have to update the binding doc
every time a new chip comes out (and I expect that'll happen more and
more!) just to add a new compatible value since all the differences are
known internally to the driver and don't impact the binding...
Kumar's idea could be a reasonable compromise, although I guess it
doesn't technically cover the case of there being 10 chips, each of
which has *some* new/different IP block, yet some IP blocks not changing
in every chip, and hence drivers might only directly binding to 5 of the
10 possible compatible values in some cases...
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
