support i2c-designware-hs
Subsystems (4)
cover support i2c-designware-hs
Add support i2c-designware-hs.c, based on designware i2c ip
Zhangfei Gao (2):
i2c-designware: support no DW_IC_COMP_TYPE platform
i2c: designware: Add i2c-designware-hs
.../devicetree/bindings/i2c/i2c-designware-hs.txt | 30 +++ drivers/i2c/busses/Kconfig | 10 + drivers/i2c/busses/Makefile | 1 + drivers/i2c/busses/i2c-designware-core.c | 24 +-- drivers/i2c/busses/i2c-designware-core.h | 1 + drivers/i2c/busses/i2c-designware-hs.c | 194 ++++++++++++++++++++ 6 files changed, 249 insertions(+), 11 deletions(-) create mode 100644 Documentation/devicetree/bindings/i2c/i2c-designware-hs.txt create mode 100644 drivers/i2c/busses/i2c-designware-hs.c
Signature · 1 line
1.7.9.5
PATCH 1/2 i2c-designware: support no DW_IC_COMP_TYPE platform
Check accessor_flags before reading DW_IC_COMP_TYPE Give chance to platform no such register
--- drivers/i2c/busses/i2c-designware-core.c | 24 +++++++++++++----------- drivers/i2c/busses/i2c-designware-core.h | 1 + 2 files changed, 14 insertions(+), 11 deletions(-)
drivers/i2c/busses/i2c-designware-core.c +13 -11
@@ -286,17 +286,19 @@ int i2c_dw_init(struct dw_i2c_dev *dev) input_clock_khz = dev->get_clk_rate_khz (dev); - reg =dw_readl (dev,DW_IC_COMP_TYPE ); -if (reg ==___constant_swab32 (DW_IC_COMP_TYPE_VALUE )) { -/* Configure register endianess access */ - dev->accessor_flags |=ACCESS_SWAP ; - }else if (reg == (DW_IC_COMP_TYPE_VALUE &0x0000ffff )) { -/* Configure register access mode 16bit */ - dev->accessor_flags |=ACCESS_16BIT ; - }else if (reg !=DW_IC_COMP_TYPE_VALUE ) { -dev_err (dev->dev,"Unknown Synopsys component type: " -"0x%08x\n" , reg); -return -ENODEV ; +if (!dev->accessor_flags) { + reg =dw_readl (dev,DW_IC_COMP_TYPE ); +if (reg ==___constant_swab32 (DW_IC_COMP_TYPE_VALUE )) { +/* Configure register endianess access */ + dev->accessor_flags |=ACCESS_SWAP ; + }else if (reg == (DW_IC_COMP_TYPE_VALUE &0x0000ffff )) { +/* Configure register access mode 16bit */ + dev->accessor_flags |=ACCESS_16BIT ; + }else if (reg !=DW_IC_COMP_TYPE_VALUE ) { +dev_err (dev->dev,"Unknown Synopsys component type: " +"0x%08x\n" , reg); +return -ENODEV ; + } }/* Disable the adapter */
drivers/i2c/busses/i2c-designware-core.h +1 -0
@@ -94,6 +94,7 @@ struct dw_i2c_dev {#define ACCESS_SWAP 0x00000001#define ACCESS_16BIT 0x00000002 +#define ACCESS_32BIT 0x00000004extern u32 dw_readl (struct dw_i2c_dev *dev,int offset);extern void dw_writel (struct dw_i2c_dev *dev,u32 b,int offset);
Signature · 1 line
1.7.9.5
PATCH 1/2 i2c-designware: support no DW_IC_COMP_TYPE platform
Hi Zhangfei Gao,
Quoted 8 lines · Zhangfei Gao
Check
accessor_flagsbefore readingDW_IC_COMP_TYPEGive chance to platform no such registerSigned-off-by Zhangfei Gao <[email protected]>--- drivers/i2c/busses/i2c-designware-core.c | 24 +++++++++++++----------- drivers/i2c/busses/i2c-designware-core.h | 1 + 2 files changed, 14 insertions(+), 11 deletions(-)
[...]
drivers/i2c/busses/i2c-designware-core.h struct dw_i2c_dev { · line 97
Quoted 10 lines
drivers/i2c/busses/i2c-designware-core.h +1
-0@@ -94,6 +94,7 @@ struct dw_i2c_dev {#define ACCESS_SWAP 0x00000001#define ACCESS_16BIT 0x00000002 +#define ACCESS_32BIT 0x00000004
This belongs to the next patch, not to this one.
baruch
Signature · 3 lines
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -PATCH 1/2 i2c-designware: support no DW_IC_COMP_TYPE platform
Quoted 21 lines · Baruch Siach
Hi Zhangfei Gao,
Quoted 8 lines · Zhangfei Gao
Check
accessor_flagsbefore readingDW_IC_COMP_TYPEGive chance to platform no such registerSigned-off-by Zhangfei Gao <[email protected]>--- drivers/i2c/busses/i2c-designware-core.c | 24 +++++++++++++----------- drivers/i2c/busses/i2c-designware-core.h | 1 + 2 files changed, 14 insertions(+), 11 deletions(-)[...]
Quoted 7 lines
drivers/i2c/busses/i2c-designware-core.h +0
-0@@ -94,6 +94,7 @@ struct dw_i2c_dev {
Quoted 8 lines
drivers/i2c/busses/i2c-designware-core.h struct dw_i2c_dev { · line 97
Quoted 3 lines
drivers/i2c/busses/i2c-designware-core.h +1
-0#define ACCESS_SWAP 0x00000001#define ACCESS_16BIT 0x00000002 +#define ACCESS_32BIT 0x00000004This belongs to the next patch, not to this one.
baruch
Thanks baruch, make sense.
PATCH 2/2 i2c: designware: Add i2c-designware-hs
In reply to Zhangfei GaoAdd support hisilicon i2c driver, which reuse designware i2c ip
--- .../devicetree/bindings/i2c/i2c-designware-hs.txt | 30 +++ drivers/i2c/busses/Kconfig | 10 + drivers/i2c/busses/Makefile | 1 + drivers/i2c/busses/i2c-designware-hs.c | 194 ++++++++++++++++++++ 4 files changed, 235 insertions(+) create mode 100644 Documentation/devicetree/bindings/i2c/i2c-designware-hs.txt create mode 100644 drivers/i2c/busses/i2c-designware-hs.c
Documentation/devicetree/bindings/i2c/i2c-designware-hs.txt new file +30 -0
@@ -0,0 +1,30 @@ +* Hisilicon I2C Controller + +Required properties : + + - compatible : should be "hisilicon,designware-i2c" + - reg : Offset and length of the register set for the device + - interrupts : <IRQ> where IRQ is the interrupt number. + +Example : + + i2c0: i2c at fcb08000 { + compatible = "hs,designware-i2c"; + #address-cells = <1>; + #size-cells = <0>; + reg = <0xfcb08000 0x1000>; + interrupts = <0 28 4>; + clocks = <&pclk>; + status = "disabled"; + }; + + Client in i2c0 bus with add 0x58 could be added as example + i2c0: i2c at fcb08000 { + status = "ok"; + pinctrl-names = "default"; + pinctrl-0 = <&i2c0_pmx_func &i2c0_cfg_func>; + i2c_client1: i2c_client at 58 { + compatible = "hisilicon,i2c_client_tpa2028"; + reg = <0x58>; + }; + };
drivers/i2c/busses/Kconfig +10 -0
@@ -419,6 +419,16 @@ config I2C_DESIGNWARE_PCI This driver can also be built as a module. If so, the module will be called i2c-designware-pci. +config I2C_DESIGNWARE_HS + tristate "Synopsys DesignWare Hisilicon" + depends on ARCH_HI3xxx + depends on HAVE_CLK + select I2C_DESIGNWARE_CORE + help + If you say yes to this option, support will be included for the + Synopsys DesignWare I2C adapter on Hisilicon. + Only master mode is supported. + config I2C_EG20T tristate "Intel EG20T PCH/LAPIS Semicon IOH(ML7213/ML7223/ML7831) I2C" depends on PCI
drivers/i2c/busses/Makefile +1 -0
@@ -41,6 +41,7 @@ obj-$(CONFIG_I2C_DESIGNWARE_PLATFORM) += i2c-designware-platform.o i2c-designware-platform-objs := i2c-designware-platdrv.o obj-$(CONFIG_I2C_DESIGNWARE_PCI) += i2c-designware-pci.o i2c-designware-pci-objs := i2c-designware-pcidrv.o +obj-$(CONFIG_I2C_DESIGNWARE_HS) += i2c-designware-hs.o obj-$(CONFIG_I2C_EG20T) += i2c-eg20t.o obj-$(CONFIG_I2C_GPIO) += i2c-gpio.o obj-$(CONFIG_I2C_HIGHLANDER) += i2c-highlander.o
drivers/i2c/busses/i2c-designware-hs.c new file +194 -0
194 lines not on this page · raw message
Signature · 1 line
1.7.9.5
PATCH 2/2 i2c: designware: Add i2c-designware-hs
Hi Zhangfei Gao,
Quoted 20 lines · Zhangfei Gao
Add support hisilicon i2c driver, which reuse designware i2c ip
Signed-off-by Zhangfei Gao <[email protected]>--- .../devicetree/bindings/i2c/i2c-designware-hs.txt | 30 +++ drivers/i2c/busses/Kconfig | 10 + drivers/i2c/busses/Makefile | 1 + drivers/i2c/busses/i2c-designware-hs.c | 194 ++++++++++++++++++++ 4 files changed, 235 insertions(+) create mode 100644 Documentation/devicetree/bindings/i2c/i2c-designware-hs.txt create mode 100644 drivers/i2c/busses/i2c-designware-hs.cDocumentation/devicetree/bindings/i2c/i2c-designware-hs.txt new file +2
-0@@ -0,0 +1,30 @@ +* Hisilicon I2C Controller +
Documentation/devicetree/bindings/i2c/i2c-designware-hs.txt line 12
Quoted 10 lines
Documentation/devicetree/bindings/i2c/i2c-designware-hs.txt new file +10
-0+Required properties : + + - compatible : should be "hisilicon,designware-i2c" + - reg : Offset and length of the register set for the device + - interrupts : <IRQ> where IRQ is the interrupt number. + +Example : + + i2c0: i2c at fcb08000 { + compatible = "hs,designware-i2c";
A few comments on this one:
1. You should Cc devicetree-discuss at lists.ozlabs.org on patches touching ftd bindings (added to Cc)
2. The convention is to use the IC block designer in the "compatible" property prefix, in this case Symopsys (snps)
3. This does not match the compatible property in hs_dw_i2c_of_match[] below where you use "hisilicon,designware-i2c"
4. Please update Documentation/devicetree/bindings/vendor-prefixes.txt when adding new vendor prefixes
Documentation/devicetree/bindings/i2c/i2c-designware-hs.txt line 23
Quoted 11 lines
Documentation/devicetree/bindings/i2c/i2c-designware-hs.txt new file +11
-0+ #address-cells = <1>; + #size-cells = <0>; + reg = <0xfcb08000 0x1000>; + interrupts = <0 28 4>; + clocks = <&pclk>; + status = "disabled"; + }; + + Client in i2c0 bus with add 0x58 could be added as example + i2c0: i2c at fcb08000 { + status = "ok";
The convention is to use "okay".
Documentation/devicetree/bindings/i2c/i2c-designware-hs.txt line 30
Quoted 7 lines
Documentation/devicetree/bindings/i2c/i2c-designware-hs.txt new file +7
-0+ pinctrl-names = "default"; + pinctrl-0 = <&i2c0_pmx_func &i2c0_cfg_func>; + i2c_client1: i2c_client at 58 { + compatible = "hisilicon,i2c_client_tpa2028"; + reg = <0x58>; + }; + };
[...]
The code below looks like a direct copy of i2c-designware-platdrv.c. Is there any reason you can't use that code instead?
Quoted 25 lines
Documentation/devicetree/bindings/i2c/i2c-designware-hs.txt new file +25
-0+static struct i2c_algorithm hs_i2c_dw_algo = { + .master_xfer = i2c_dw_xfer, + .functionality = i2c_dw_func, +}; +static u32 hs_i2c_dw_get_clk_rate_khz(struct dw_i2c_dev *dev) +{ + return clk_get_rate(dev->clk)/1000; +} + +static int hs_dw_i2c_probe(struct platform_device *pdev) +{ + struct dw_i2c_dev *d; + struct i2c_adapter *adap; + struct resource *iores; + struct pinctrl *pinctrl; + int r; + + d = devm_kzalloc(&pdev->dev, sizeof(struct dw_i2c_dev), GFP_KERNEL); + if (!d) + return -ENOMEM; + + /* NOTE: driver uses the static register mapping */ + iores = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!iores) + return -EINVAL;
Documentation/devicetree/bindings/i2c/i2c-designware-hs.txt
Quoted 10 lines
Documentation/devicetree/bindings/i2c/i2c-designware-hs.txt new file +10
-0+ + d->base = devm_request_and_ioremap(&pdev->dev, iores); + if (!d->base) + return -EADDRNOTAVAIL; + + d->irq = platform_get_irq(pdev, 0); + if (d->irq < 0) { + dev_err(&pdev->dev, "no irq resource?\n"); + return d->irq; /* -ENXIO */ + }
[...]
baruch
Signature · 3 lines
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -PATCH 2/2 i2c: designware: Add i2c-designware-hs
Quoted 20 lines
Quoted 4 lines
+++
b/Documentation/devicetree/bindings/i2c/i2c-designware-hs.txtDocumentation/devicetree/bindings/i2c/i2c-designware-hs.txt new file +2
-0@@ -0,0 +1,30 @@ +* Hisilicon I2C Controller +Documentation/devicetree/bindings/i2c/i2c-designware-hs.txt line 12
Quoted 10 lines
Documentation/devicetree/bindings/i2c/i2c-designware-hs.txt new file +10
-0+Required properties : + + - compatible : should be "hisilicon,designware-i2c" + - reg : Offset and length of the register set for the device + - interrupts : <IRQ> where IRQ is the interrupt number. + +Example : + + i2c0: i2c at fcb08000 { + compatible = "hs,designware-i2c";A few comments on this one:
1. You should Cc devicetree-discuss at lists.ozlabs.org on patches touching ftd bindings (added to Cc)
Quoted 8 lines
2. The convention is to use the IC block designer in the "compatible" property prefix, in this case Symopsys (snps)
3. This does not match the compatible property in hs_dw_i2c_of_match[] below where you use "hisilicon,designware-i2c"
4. Please update
Documentation/devicetree/bindings/vendor-prefixes.txtwhen adding new vendor prefixes
Thanks Baruch for the kind education, really useful. How about using .compatible = "snps,hisilicon-i2c"
Quoted 5 lines
Documentation/devicetree/bindings/i2c/i2c-designware-hs.txt line 23
Quoted 3 lines
Documentation/devicetree/bindings/i2c/i2c-designware-hs.txt new file +3
-0+ Client in i2c0 bus with add 0x58 could be added as example + i2c0: i2c at fcb08000 { + status = "ok";The convention is to use "okay".
got it.
Quoted 9 lines
Documentation/devicetree/bindings/i2c/i2c-designware-hs.txt line 30
Quoted 8 lines
Documentation/devicetree/bindings/i2c/i2c-designware-hs.txt new file +7
-0+ pinctrl-names = "default"; + pinctrl-0 = <&i2c0_pmx_func &i2c0_cfg_func>; + i2c_client1: i2c_client at 58 { + compatible = "hisilicon,i2c_client_tpa2028"; + reg = <0x58>; + }; + };
Quoted 4 lines
[...]
The code below looks like a direct copy of
i2c-designware-platdrv.c. Is there any reason you can't use that code instead?
Not understood i2c-designware-platdrv.c can be directly touched. Usually, there is register function, or external function call.
It would be great if we could directly add hisilicon support in i2c-designware-platdrv.c.
How about adding these code to distinguish.
The concern is will platdrv.c become bigger and bigger? What in case private register have to be accessed?
struct dw_i2c_data {
u32 accessor_flags;
unsigned int tx_fifo_depth;
unsigned int rx_fifo_depth;
};static struct dw_i2c_data hisilicon_data = {
.accessor_flags = ACCESS_32BIT,
.tx_fifo_depth = 16,
.rx_fifo_depth = 16,
};{ .compatible = "snps,hisilicon-i2c", .data = &hisilicon_data},
PATCH 2/2 i2c: designware: Add i2c-designware-hs
Hi zhangfei gao,
Quoted 23 lines · zhangfei gao
Quoted 23 lines
Quoted 4 lines
+++
b/Documentation/devicetree/bindings/i2c/i2c-designware-hs.txtDocumentation/devicetree/bindings/i2c/i2c-designware-hs.txt new file +2
-0@@ -0,0 +1,30 @@ +* Hisilicon I2C Controller +Documentation/devicetree/bindings/i2c/i2c-designware-hs.txt line 12
Quoted 10 lines
Documentation/devicetree/bindings/i2c/i2c-designware-hs.txt new file +10
-0+Required properties : + + - compatible : should be "hisilicon,designware-i2c" + - reg : Offset and length of the register set for the device + - interrupts : <IRQ> where IRQ is the interrupt number. + +Example : + + i2c0: i2c at fcb08000 { + compatible = "hs,designware-i2c";A few comments on this one:
1. You should Cc devicetree-discuss at lists.ozlabs.org on patches touching ftd bindings (added to Cc)
2. The convention is to use the IC block designer in the "compatible" property prefix, in this case Symopsys (snps)
Quoted 8 lines
Quoted 5 lines
3. This does not match the compatible property in hs_dw_i2c_of_match[] below where you use "hisilicon,designware-i2c"
4. Please update
Documentation/devicetree/bindings/vendor-prefixes.txtwhen adding new vendor prefixesThanks Baruch for the kind education, really useful. How about using .compatible = "snps,hisilicon-i2c"
I don't think this is needed. See below.
Quoted 21 lines
Quoted 5 lines
Documentation/devicetree/bindings/i2c/i2c-designware-hs.txt line 23
Quoted 3 lines
Documentation/devicetree/bindings/i2c/i2c-designware-hs.txt new file +3
-0+ Client in i2c0 bus with add 0x58 could be added as example + i2c0: i2c at fcb08000 { + status = "ok";The convention is to use "okay".
got it.
Quoted 9 lines
Documentation/devicetree/bindings/i2c/i2c-designware-hs.txt line 30
Quoted 8 lines
Documentation/devicetree/bindings/i2c/i2c-designware-hs.txt new file +7
-0+ pinctrl-names = "default"; + pinctrl-0 = <&i2c0_pmx_func &i2c0_cfg_func>; + i2c_client1: i2c_client at 58 { + compatible = "hisilicon,i2c_client_tpa2028"; + reg = <0x58>; + }; + };Quoted 4 lines
[...]
The code below looks like a direct copy of
i2c-designware-platdrv.c. Is there any reason you can't use that code instead?
Quoted 8 lines
Not understood
i2c-designware-platdrv.ccan be directly touched. Usually, there is register function, or external function call.It would be great if we could directly add hisilicon support in
i2c-designware-platdrv.c.
How about adding these code to distinguish.The concern is will
platdrv.cbecome bigger and bigger?
The overall code size becomes much bigger when duplicating the code. It makes code maintenance harder.
Quoted 1 line
What in case private register have to be accessed?
Good question. I don't know what is the common convention in this case. Do you have such a need here?
Quoted 8 lines
struct dw_i2c_data { u32 accessor_flags; unsigned int tx_fifo_depth; unsigned int rx_fifo_depth; };static struct
dw_i2c_datahisilicon_data= {
.accessor_flags =ACCESS_32BIT,
This should be detected automatically in i2c_dw_init(). When ACCESS_16BIT is not set, access is 32bit wide. Doesn't it work for you?
Quoted 2 lines
.tx_fifo_depth = 16,
.rx_fifo_depth = 16,These should be encoded in new device-tree properties named "tx-fifo-size", and "rx-fifo-size". For example, see Documentation/devicetree/bindings/powerpc/4xx/emac.txt.
baruch
Quoted 2 lines
};
{ .compatible = "snps,hisilicon-i2c", .data =&hisilicon_data},
Signature · 3 lines
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -PATCH 2/2 i2c: designware: Add i2c-designware-hs
Quoted 6 lines
Quoted 3 lines
What in case private register have to be accessed?
Good question. I don't know what is the common convention in this case. Do you have such a need here?
Originally, there is private register for tuning timing of clk and data. Suppose there may some issue of catching data in clk trigger stage. However, in the test, still not need to set timing yet.
Quoted 7 lines
Quoted 7 lines
struct dw_i2c_data { u32 accessor_flags; unsigned int tx_fifo_depth; unsigned int rx_fifo_depth; };
Quoted 5 lines
Quoted 2 lines
static struct
dw_i2c_datahisilicon_data= {
.accessor_flags =ACCESS_32BIT,This should be detected automatically in
i2c_dw_init(). WhenACCESS_16BITis not set, access is 32bit wide. Doesn't it work for you?
Cool, this works, then the first patch can be ignored at all. My bad, not check when no value of DW_IC_COMP_TYPE.
Quoted 7 lines
Quoted 3 lines
.tx_fifo_depth = 16, .rx_fifo_depth = 16,These should be encoded in new device-tree properties named "tx-fifo-size", and "rx-fifo-size". For example, see
Documentation/devicetree/bindings/powerpc/4xx/emac.txt.
OK, will add these two property to Documentation/devicetree/bindings/i2c/i2c-designware.txt
Besides, would you mind change subsys_initcall(dw_i2c_init_driver);
tomodule_platform_driver(dw_i2c_driver);
Otherwise default pinctrl can not be auto-set, since subsys_initcall is too early.
Thanks