i2c: npcm7xx: add NPCM i2c controller driver
Tali Perry
maintainerv10 cover i2c: npcm7xx: add NPCM i2c controller driver
This patch set adds i2c controller support
for the Nuvoton NPCM Baseboard Management Controller (BMC).
NPCM7xx includes 16 I2C controllers. This driver operates the controller. This module also includes a slave mode.
---
v10 -> v9:
- Fix according to maintainer comments.
- binding file changed to yaml format.
- Shorten recovery flow.
- Add support for health monitoring counters.
v9 -> v8:
- Fix according to maintainer comments.
- Split lines of iowrite..(ioread..) to separate lines.
- Use readx_poll_timeout_atomic
- resolve various style issues.
v8 -> v7:
- Split to two commits, one for master, one for slave.
- Rename smb to i2c.
- Remove global vars.
v7 -> v6:
- Rebased on Linux 5.4-rc8 (was Linux 5.4-rc7).
- Fix issue found by kbuild test robot (redundant include).
- Note: left a warning related to fall through. This fall through is intentional.
v6 -> v5:
- Update documentation
v5 -> v4:
- support recovery
- master-slave switch support needed for IPMB
v4 -> v3:
- typo on cover letter.
v3 -> v2:
- fix dt binding: compatible name: omit "bus"
v2 -> v1:
- run check patch in strict mode.
- use linux crc.
- define regs in constant offset without base.
- remove debug prints.
- no declarations for local functions.
v1: initial version
---
Tali Perry (3):
dt-bindings: i2c: npcm7xx: add NPCM I2C controller documentation
i2c: npcm7xx: Add Nuvoton NPCM I2C controller driver
i2c: npcm7xx: Add support for slave mode for Nuvoton NPCM BMC I2C
controller driver.
.../bindings/i2c/nuvoton,npcm7xx-i2c.yaml | 62 + drivers/i2c/busses/Kconfig | 9 + drivers/i2c/busses/Makefile | 1 + drivers/i2c/busses/i2c-npcm7xx.c | 2467 +++++++++++++++++ 4 files changed, 2539 insertions(+) create mode 100644 Documentation/devicetree/bindings/i2c/nuvoton,npcm7xx-i2c.yaml create mode 100644 drivers/i2c/busses/i2c-npcm7xx.c
base-commit: 262f7a6b8317a06e7d51befb690f0bca06a473ea
Signature · 7 lines
2.22.0 _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Tali Perry
maintainerPATCH v10 1/3 dt-bindings: i2c: npcm7xx: add NPCM I2C controller
Added device tree binding documentation for Nuvoton BMC NPCM I2C controller.
--- .../bindings/i2c/nuvoton,npcm7xx-i2c.yaml | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 Documentation/devicetree/bindings/i2c/nuvoton,npcm7xx-i2c.yaml
Documentation/devicetree/bindings/i2c/nuvoton,npcm7xx-i2c.yaml new file +62 -0
@@ -0,0 +1,62 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/i2c/nuvoton,npcm7xx-i2c.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: nuvoton NPCM7XX I2C Controller Device Tree Bindings + +description: | + The NPCM750x includes sixteen I2C bus controllers. All Controllers support + both master and slave mode. Each controller can switch between master and slave + at run time (i.e. IPMB mode). Each controller has two 16 byte HW FIFO for TX and + RX. + +maintainers: + - Tali Perry <[email protected]> + +properties: + compatible: + - const: nuvoton,npcm7xx-i2c + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + clocks: + maxItems: 1 + items: + - description: Reference clock for the I2C bus + + clock-frequency: + maxItems: 1 + default: 100000 + enum: [ 100000, 400000, 1000000 ] + description: + SCL frequency to use (in Hz). If omitted, 100kHz is used. + +required: + - compatible + - reg + - interrupts + +allOf: + - $ref: /schemas/i2c/i2c-controller.yaml# + +unevaluatedProperties: false + +examples: + - | + i2c0: i2c@80000 { + compatible = "nuvoton,npcm750-i2c"; + reg = <0x80000 0x1000>; + clocks = <&clk NPCM7XX_CLK_APB2>; + bus-frequency = <100000>; + interrupts = <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>; + pinctrl-names = "default"; + pinctrl-0 = <&smb0_pins>; + }; + +...
Signature · 1 line
2.22.0
Rob Herring
maintainerRe: [PATCH v10 1/3] dt-bindings: i2c: npcm7xx: add NPCM I2C controller
Quoted 9 lines · Tali Perry
Added device tree binding documentation for Nuvoton BMC NPCM I2C controller.
Signed-off-by Tali Perry <[email protected]>--- .../bindings/i2c/nuvoton,npcm7xx-i2c.yaml | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 Documentation/devicetree/bindings/i2c/nuvoton,npcm7xx-i2c.yaml
My bot found errors running 'make dt_binding_check' on your patch:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/i2c/nuvoton,npcm7xx-i2c.yaml: properties:compatible: [{'const': 'nuvoton,npcm7xx-i2c'}] is not of type 'object', 'boolean' /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/i2c/nuvoton,npcm7xx-i2c.yaml: properties:clock-frequency:enum: False schema does not allow [100000, 400000, 1000000] /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/i2c/nuvoton,npcm7xx-i2c.yaml: properties:clocks: {'maxItems': 1, 'items': [{'description': 'Reference clock for the I2C bus'}]} is not valid under any of the given schemas (Possible causes of the failure):
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/i2c/nuvoton,npcm7xx-i2c.yaml: properties:clocks: 'maxItems' is not one of ['type', 'description', 'dependencies', 'properties', 'patternProperties', 'additionalProperties', 'unevaluatedProperties', 'deprecated', 'required', 'allOf', 'anyOf', 'oneOf', '$ref']
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/i2c/nuvoton,npcm7xx-i2c.yaml: properties:clock-frequency: 'maxItems' is not one of ['description', 'deprecated', 'const', 'enum', 'minimum', 'maximum', 'default', '$ref'] Documentation/devicetree/bindings/Makefile:12: recipe for target 'Documentation/devicetree/bindings/i2c/nuvoton,npcm7xx-i2c.example.dts' failed
make[1]: *** [Documentation/devicetree/bindings/i2c/nuvoton,npcm7xx-i2c.example.dts] Error 1
make[1]: *** Waiting for unfinished jobs....
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/i2c/nuvoton,npcm7xx-i2c.yaml: ignoring, error in schema: properties: compatible
warning: no schema found in file: ./Documentation/devicetree/bindings/i2c/nuvoton,npcm7xx-i2c.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/i2c/nuvoton,npcm7xx-i2c.yaml: ignoring, error in schema: properties: compatible
warning: no schema found in file: ./Documentation/devicetree/bindings/i2c/nuvoton,npcm7xx-i2c.yaml
Makefile:1300: recipe for target 'dt_binding_check' failed
make: *** [dt_binding_check] Error 2
See https://patchwork.ozlabs.org/patch/1287083
If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure dt-schema is up to date:
pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade
Please check and re-submit.
Re: [PATCH v10 1/3] dt-bindings: i2c: npcm7xx: add NPCM I2C controller
In reply to Tali PerryThe example uses "bus-frequency" while description says "clock-frequency" for bus speed.
Quoted 87 lines · On Sun, May 10, 2020, 12:27 Tali Perry <[email protected]> wrote:
Added device tree binding documentation for Nuvoton BMC NPCM I2C controller.
Signed-off-by Tali Perry <[email protected]>--- .../bindings/i2c/nuvoton,npcm7xx-i2c.yaml | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644
Documentation/devicetree/bindings/i2c/nuvoton,npcm7xx-i2c.yamldiff --git
a/Documentation/devicetree/bindings/i2c/nuvoton,npcm7xx-i2c.yamlb/Documentation/devicetree/bindings/i2c/nuvoton,npcm7xx-i2c.yamlnew file mode 100644
index 000000000000..d6f553154388Documentation/devicetree/bindings/i2c/nuvoton,npcm7xx-i2c.yaml new file +10
-0@@ -0,0 +1,62 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/i2c/nuvoton,npcm7xx-i2c.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: nuvoton NPCM7XX I2C Controller Device Tree Bindings + +description: | + The NPCM750x includes sixteen I2C bus controllers. All Controllerssupport
+ both master and slave mode. Each controller can switch between master and slave
+ at run time (i.e. IPMB mode). Each controller has two 16 byte HW FIFO for TX andDocumentation/devicetree/bindings/i2c/nuvoton,npcm7xx-i2c.yaml new file +50
-1+ RX. + +maintainers: + - Tali Perry <[email protected]> + +properties: + compatible: + - const: nuvoton,npcm7xx-i2c + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + clocks: + maxItems: 1 + items: + - description: Reference clock for the I2C bus + + clock-frequency: + maxItems: 1 + default: 100000 + enum: [ 100000, 400000, 1000000 ] + description: + SCL frequency to use (in Hz). If omitted, 100kHz is used. + +required: + - compatible + - reg + - interrupts + +allOf: + - $ref: /schemas/i2c/i2c-controller.yaml# + +unevaluatedProperties: false + +examples: + - | + i2c0: i2c@80000 { + compatible = "nuvoton,npcm750-i2c"; + reg = <0x80000 0x1000>; + clocks = <&clk NPCM7XX_CLK_APB2>; + bus-frequency = <100000>; + interrupts = <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>; + pinctrl-names = "default"; + pinctrl-0 = <&smb0_pins>; + }; + +... --2.22.0
Tali Perry
maintainerPATCH v10 2/3 i2c: npcm7xx: Add Nuvoton NPCM I2C controller driver
In reply to Tali PerryAdd Nuvoton NPCM BMC I2C controller driver.
--- drivers/i2c/busses/Kconfig | 9 + drivers/i2c/busses/Makefile | 1 + drivers/i2c/busses/i2c-npcm7xx.c | 1853 ++++++++++++++++++++++++++++++ 3 files changed, 1863 insertions(+) create mode 100644 drivers/i2c/busses/i2c-npcm7xx.c
drivers/i2c/busses/Kconfig +9 -0
@@ -791,6 +791,15 @@ config I2C_NOMADIK I2C interface from ST-Ericsson's Nomadik and Ux500 architectures, as well as the STA2X11 PCIe I/O HUB. +config I2C_NPCM7XX + tristate "Nuvoton I2C Controller" + depends on ARCH_NPCM7XX || COMPILE_TEST + help + If you say yes to this option, support will be included for the + Nuvoton I2C controller, which is available on the NPCM7xx BMC + controller. + Driver can also support slave mode (select I2C_SLAVE). + config I2C_OCORES tristate "OpenCores I2C Controller" help
drivers/i2c/busses/Makefile +1 -0
@@ -81,6 +81,7 @@ obj-$(CONFIG_I2C_MT7621) += i2c-mt7621.o obj-$(CONFIG_I2C_MV64XXX) += i2c-mv64xxx.o obj-$(CONFIG_I2C_MXS) += i2c-mxs.o obj-$(CONFIG_I2C_NOMADIK) += i2c-nomadik.o +obj-$(CONFIG_I2C_NPCM7XX) += i2c-npcm7xx.o obj-$(CONFIG_I2C_OCORES) += i2c-ocores.o obj-$(CONFIG_I2C_OMAP) += i2c-omap.o obj-$(CONFIG_I2C_OWL) += i2c-owl.o
drivers/i2c/busses/i2c-npcm7xx.c new file +1853 -0
1,853 lines not on this page · raw message
Signature · 7 lines
2.22.0 _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH v10 2/3] i2c: npcm7xx: Add Nuvoton NPCM I2C controller driver
Quoted 1 line · Tali Perry
Add Nuvoton NPCM BMC I2C controller driver.
Some cosmetic changes needs to be done.
...
Quoted 5 lines
+/*
+ * Nuvoton NPCM7xx I2C Controller driver
+ *
+ * Copyright (C) 2020 Nuvoton Technologies [email protected]
+ */
So, entire file has C99 comment style, but this and few other places. Any reason of inconsistency?
...
Quoted 1 line
+#if
IS_ENABLED(CONFIG_DEBUG_FS)
Why?
Quoted 2 lines
drivers/i2c/busses/i2c-npcm7xx.c new file +2
-0+#include <linux/debugfs.h> +#endif
...
Quoted 1 line
+//#define _I2C_DEBUG_
Leftover, remove.
...
drivers/i2c/busses/i2c-npcm7xx.c line 84
Quoted 6 lines
drivers/i2c/busses/i2c-npcm7xx.c new file +5
-0+// Common regs +#define NPCM_I2CSDA 0x00 +#define NPCM_I2CST 0x02 +#define NPCM_I2CCST 0x04 +#define NPCM_I2CCTL1 0x06
Indentation issue. And it's better to use TABs over spaces here and in the similar places above and below.
Quoted 6 lines
drivers/i2c/busses/i2c-npcm7xx.c new file +6
-0+#define NPCM_I2CADDR1 0x08 +#define NPCM_I2CCTL2 0x0A +#define NPCM_I2CADDR2 0x0C +#define NPCM_I2CCTL3 0x0E +#define NPCM_I2CCST2 0x18 +#define NPCM_I2CCST3 0x19
...
drivers/i2c/busses/i2c-npcm7xx.c line 234
Quoted 3 lines
drivers/i2c/busses/i2c-npcm7xx.c new file +3
-0+// supported clk settings. values in KHz. +#define I2C_FREQ_MIN 10 +#define I2C_FREQ_MAX 1000
_KHZ to both.
...
Quoted 1 line
+#define
I2C_NUM_OF_ADDR10
Is it 10-bit address support or what?
...
drivers/i2c/busses/i2c-npcm7xx.c line 293
Quoted 8 lines
drivers/i2c/busses/i2c-npcm7xx.c new file +8
-0+#if IS_ENABLED (CONFIG_DEBUG_FS ) +static struct dentry *npcm_i2c_debugfs_dir;/* i2c debugfs directory */ +static const char *ber_cnt_name ="ber_count" ; +static const char *rec_succ_cnt_name ="rec_succ_count" ; +static const char *rec_fail_cnt_name ="rec_fail_count" ; +static const char *nack_cnt_name ="nack_count" ; +static const char *timeout_cnt_name ="timeout_count" ; +#endif
Why are these global?
...
drivers/i2c/busses/i2c-npcm7xx.c line 641
Quoted 7 lines
drivers/i2c/busses/i2c-npcm7xx.c new file +6
-0+static void npcm_i2c_write_to_fifo_master (struct npcm_i2c *bus, +u16 max_bytes_to_send) +{ +// Fill the FIFO, while the FIFO is not full and there are more bytes to +// write + while ((max_bytes_to_send--) &&
Inner parentheses are redundant.
Quoted 7 lines
drivers/i2c/busses/i2c-npcm7xx.c new file +7
-0+ (I2C_HW_FIFO_SIZE - npcm_i2c_fifo_usage(bus))) { +if (bus->wr_ind < bus->wr_size) +npcm_i2c_wr_byte (bus, bus->wr_buf[bus->wr_ind++]); +else +npcm_i2c_wr_byte (bus,0xFF ); + } +}
...
drivers/i2c/busses/i2c-npcm7xx.c line 998
Quoted 4 lines
drivers/i2c/busses/i2c-npcm7xx.c new file +4
-0+// Clear stall only after setting STOP +iowrite8 (NPCM_I2CST_STASTR , bus->reg +NPCM_I2CST ); + + ret =IRQ_HANDLED ;
Indentation issue.
...
drivers/i2c/busses/i2c-npcm7xx.c line 1059
Quoted 6 lines
drivers/i2c/busses/i2c-npcm7xx.c new file +6
-0+if (bus->wr_size) +npcm_i2c_set_fifo (bus,-1 , + bus->wr_size); +else +npcm_i2c_set_fifo (bus, bus->rd_size, +-1 );
These two looks much better on one line.
...
drivers/i2c/busses/i2c-npcm7xx.c line 1067
Quoted 5 lines
drivers/i2c/busses/i2c-npcm7xx.c new file +5
-0+if (npcm_i2c_is_quick (bus) || bus->wr_size) +npcm_i2c_wr_byte (bus, bus->dest_addr); +else +npcm_i2c_wr_byte (bus, bus->dest_addr | +0x01 );
0x01 has its definition, hasn't it?
...
drivers/i2c/busses/i2c-npcm7xx.c line 1136
Quoted 11 lines
drivers/i2c/busses/i2c-npcm7xx.c new file +11
-0+// Repeat the following sequence until SDA is released +do { +// Issue a single SCL toggle +iowrite8 (NPCM_I2CCST_TGSCL , bus->reg +NPCM_I2CCST ); +udelay (20 ); +// If SDA line is inactive (high), stop +if (npcm_i2c_get_SDA (_adap)) { + done = true; + status =0 ; + } + }while (!done && iter--);
readx_poll_timeout() ?
...
drivers/i2c/busses/i2c-npcm7xx.c line 1162
Quoted 2 lines
drivers/i2c/busses/i2c-npcm7xx.c new file +2
-0+#if IS_ENABLED (CONFIG_DEBUG_FS ) +if (!status) {
Why not positive condition?
if (status) {
...
} else {
...
}Quoted 4 lines
drivers/i2c/busses/i2c-npcm7xx.c new file +3
-0+ } else { + } +#endif
...
drivers/i2c/busses/i2c-npcm7xx.c line 1226
Quoted 10 lines
drivers/i2c/busses/i2c-npcm7xx.c new file +9
-0+static int npcm_i2c_init_clk (struct npcm_i2c *bus,u32 bus_freq) +{ +u32 k1 =0 ; +u32 k2 =0 ; +u8 dbnct =0 ; +u32 sclfrq =0 ; +u8 hldt =7 ; +bool fast_mode = false; +u32 src_clk_freq;// in KHz
src_clk_khz ?
drivers/i2c/busses/i2c-npcm7xx.c line 1232
Quoted 7 lines
drivers/i2c/busses/i2c-npcm7xx.c new file +6
-0+ + src_clk_freq = bus->apb_clk /1000 ; + bus->bus_freq = bus_freq; + +// 100KHz and below: +if (bus_freq <= (I2C_MAX_STANDARD_MODE_FREQ /1000 )) {
Instead of all these / 1000 can't you use bus frequency in Hz and do division when it's really needed?
Quoted 24 lines
drivers/i2c/busses/i2c-npcm7xx.c new file +24
-0+ sclfrq = src_clk_freq / (bus_freq *4 ); + +if (sclfrq <SCLFRQ_MIN || sclfrq >SCLFRQ_MAX ) +return -EDOM ; + +if (src_clk_freq >=40000 ) + hldt =17 ; +else if (src_clk_freq >=12500 ) + hldt =15 ; +else + hldt =7 ; + } + +// 400KHz: +else if (bus_freq <= (I2C_MAX_FAST_MODE_FREQ /1000 )) { + sclfrq =0 ; + fast_mode = true; + +if (src_clk_freq <7500 ) +// 400KHZ cannot be supported for core clock < 7.5 MHZ +return -EDOM ; + +else if (src_clk_freq >=50000 ) { + k1 =80 ;
drivers/i2c/busses/i2c-npcm7xx.c line 1265
Quoted 10 lines
drivers/i2c/busses/i2c-npcm7xx.c new file +9
-0+ k2 =48 ; + hldt =12 ; + dbnct =7 ; + } + +// Master or Slave with frequency > 25 MHZ +else if (src_clk_freq >25000 ) { + hldt = (DIV_ROUND_UP (src_clk_freq *300 , +1000000 ) +7 ) &0xFF ;
How ' & 0xFF' is not no-op here and in the similar cases?
drivers/i2c/busses/i2c-npcm7xx.c line 1270
Quoted 5 lines
drivers/i2c/busses/i2c-npcm7xx.c new file +5
-0+ + k1 =DIV_ROUND_UP (src_clk_freq *1600 , +1000000 ); + k2 =DIV_ROUND_UP (src_clk_freq *900 , +1000000 );
Perhaps,
#define clk_coef(freq, mul) DIV_ROUND_UP((freq) * (mul), 1000000)
?
Quoted 44 lines
drivers/i2c/busses/i2c-npcm7xx.c new file +43
-0+ k1 =round_up (k1,2 ); + k2 =round_up (k2 +1 ,2 ); +if (k1 <SCLFRQ_MIN || k1 >SCLFRQ_MAX || + k2 <SCLFRQ_MIN || k2 >SCLFRQ_MAX ) +return -EDOM ; + } + } + +else if (bus_freq <= (I2C_MAX_FAST_MODE_PLUS_FREQ /1000 )) { + sclfrq =0 ; + fast_mode = true; + +if (src_clk_freq <24000 ) +// 1MHZ cannot be supported for master core clock < 15 MHZ +// or slave core clock < 24 MHZ +return -EDOM ; + + k1 =round_up ((DIV_ROUND_UP (src_clk_freq *620 , +1000000 )),2 ); + k2 =round_up ((DIV_ROUND_UP (src_clk_freq *380 , +1000000 ) +1 ),2 ); +if (k1 <SCLFRQ_MIN || k1 >SCLFRQ_MAX || + k2 <SCLFRQ_MIN || k2 >SCLFRQ_MAX ) +return -EDOM ; + +// Core clk > 40 MHZ +if (src_clk_freq >40000 ) { +// Set HLDT: +// SDA hold time: (HLDT-7) * T(CLK) >= 120 +// HLDT = 120/T(CLK) + 7 = 120 * FREQ(CLK) + 7 + hldt = (DIV_ROUND_UP (src_clk_freq *120 , +1000000 ) +7 ) &0xFF ; + }else { + hldt =7 ; + dbnct =2 ; + } + } + +// Frequency larger than 1 MHZ is not supported +else +return false; +return true; +}
...
drivers/i2c/busses/i2c-npcm7xx.c line 1401
Quoted 5 lines
drivers/i2c/busses/i2c-npcm7xx.c new file +4
-0+ ret =device_property_read_u32 (&pdev->dev,"bus-frequency" , &clk_freq); +if (ret <0 ) { +dev_info (&pdev->dev,"Could not read bus-frequency property\n" ); + clk_freq =100000 ;
We have define for this, don't we?
Review
Quoted 1 line
(quoted diff) +1
-0+ }
Wolfram, we discussed this simplified timings property parser, any news about it?
...
Quoted 6 lines
drivers/i2c/busses/i2c-npcm7xx.c new file +6
-0+static irqreturn_t npcm_i2c_bus_irq (int irq,void *dev_id) +{ +irqreturn_t ret; +struct npcm_i2c *bus = dev_id; + + bus->int_cnt++;
drivers/i2c/busses/i2c-npcm7xx.c line 1427
Quoted 10 lines
drivers/i2c/busses/i2c-npcm7xx.c new file +9
-0+ +if (npcm_i2c_is_master (bus)) + bus->master_or_slave =I2C_MASTER ; + +if (bus->master_or_slave ==I2C_MASTER ) { + bus->int_time_stamp = jiffies; + ret =npcm_i2c_int_master_handler (bus); +if (ret ==IRQ_HANDLED ) +return ret;
What's the point?
Quoted 3 lines
drivers/i2c/busses/i2c-npcm7xx.c new file +3
-0+ } +return IRQ_HANDLED ; +}
...
drivers/i2c/busses/i2c-npcm7xx.c line 1442
Quoted 1 line
drivers/i2c/busses/i2c-npcm7xx.c new file +1
-0+ bus->dest_addr = (slave_addr <<1 ) &0xFE ;
How ' & 0xFE' part is not a no-op?
...
drivers/i2c/busses/i2c-npcm7xx.c line 1563
Quoted 2 lines
drivers/i2c/busses/i2c-npcm7xx.c new file +2
-0+ time_left = jiffies + +msecs_to_jiffies (DEFAULT_STALL_COUNT ) +1 ;
It's perfectly one line. Fix here and in any other place with similar issue.
...
Quoted 1 line
+static int
i2c_init_debugfs(struct platform_device *pdev, struct npcm_i2c *bus)
Should be void.
...
drivers/i2c/busses/i2c-npcm7xx.c line 1749
Quoted 4 lines
drivers/i2c/busses/i2c-npcm7xx.c new file +3
-0+ bus->irq =platform_get_irq (pdev,0 ); +if (bus->irq <0 ) { +dev_err (&pdev->dev,"failed to get IRQ number\n" );
Duplicate message.
Quoted 2 lines
drivers/i2c/busses/i2c-npcm7xx.c new file +2
-0+return bus->irq; + }
...
Quoted 1 line
+#if
IS_ENABLED(CONFIG_DEBUG_FS)
Why? Okay, why here instead of making a stub?
drivers/i2c/busses/i2c-npcm7xx.c line 1781
Quoted 4 lines
drivers/i2c/busses/i2c-npcm7xx.c new file +3
-0+ ret =i2c_init_debugfs (pdev, bus); +if (ret <0 ) +return ret;
Wrong. Should not fail the probe.
Quoted 1 line
+#endif
...
Quoted 1 line
+#if
IS_ENABLED(CONFIG_DEBUG_FS)
Why? Just make it always present in the structure.
drivers/i2c/busses/i2c-npcm7xx.c line 1797
Quoted 1 line
drivers/i2c/busses/i2c-npcm7xx.c new file +1
-0+if (!!bus->debugfs) {
!! ???
Quoted 4 lines
drivers/i2c/busses/i2c-npcm7xx.c new file +4
-0+debugfs_remove_recursive (bus->debugfs); + bus->debugfs = NULL; + } +#endif
...
drivers/i2c/busses/i2c-npcm7xx.c line 1830
Quoted 7 lines
drivers/i2c/busses/i2c-npcm7xx.c new file +6
-0+ npcm_i2c_debugfs_dir =debugfs_create_dir ("i2c" ,NULL ); +if (IS_ERR_OR_NULL (npcm_i2c_debugfs_dir)) { +pr_warn ("i2c init of debugfs failed\n" ); + npcm_i2c_debugfs_dir =NULL ; +return -ENOMEM ; + }
Shouldn't prevent driver to work.
...
drivers/i2c/busses/i2c-npcm7xx.c line 1837
Quoted 1 line
drivers/i2c/busses/i2c-npcm7xx.c new file +1
-0+if (!!npcm_i2c_debugfs_dir) {
!! ???
drivers/i2c/busses/i2c-npcm7xx.c line 1839
Quoted 3 lines
drivers/i2c/busses/i2c-npcm7xx.c new file +2
-0+debugfs_remove_recursive (npcm_i2c_debugfs_dir); + npcm_i2c_debugfs_dir =NULL ;
What's the point?
Quoted 1 line
(quoted diff) +1
-0+ }
Signature · 2 lines
With Best Regards, Andy Shevchenko
Tali Perry
maintainerRe: [PATCH v10 2/3] i2c: npcm7xx: Add Nuvoton NPCM I2C controller driver
Quoted 6 lines · Andy Shevchenko
Quoted 1 line · Tali Perry
Add Nuvoton NPCM BMC I2C controller driver.
Some cosmetic changes needs to be done.
Thanks for the review and the comments.
Will fix all, have a few questions (below)
Quoted 8 lines
...
Quoted 5 lines
+/*
+ * Nuvoton NPCM7xx I2C Controller driver
+ *
+ * Copyright (C) 2020 Nuvoton Technologies [email protected]
+ */
Quoted 8 lines
So, entire file has C99 comment style, but this and few other places. Any reason of inconsistency?
...
Quoted 1 line
+#if
IS_ENABLED(CONFIG_DEBUG_FS)Why?
We wanted to add an optional feature to track i2c slave status.
the NPCM has 16 channels handling multiple devices each. Some of the devices are polled periodically, and might power down.
The user wanted to implement a health monitoring option
to occasionally check the status of the buses (how many timeouts, recovery etc.) This feature is optional and depends on CONFIG_DEBUG_FS The counters are exposed to user through the file system.
....
Quoted 6 lines
...
Quoted 1 line
+#define
I2C_NUM_OF_ADDR10Is it 10-bit address support or what?
No, the NPCM has an option to respond to multiple slave addresses (10 own slave addresses)
...
Quoted 12 lines
drivers/i2c/busses/i2c-npcm7xx.c line 1136
Quoted 11 lines
drivers/i2c/busses/i2c-npcm7xx.c new file +11
-0+// Repeat the following sequence until SDA is released +do { +// Issue a single SCL toggle +iowrite8 (NPCM_I2CCST_TGSCL , bus->reg +NPCM_I2CCST ); +udelay (20 ); +// If SDA line is inactive (high), stop +if (npcm_i2c_get_SDA (_adap)) { + done = true; + status =0 ; + } + }while (!done && iter--);
Quoted 1 line
readx_poll_timeout()?
Not exactly, readx_poll_timeout includes only a read operation, here there is a write in the middle. (iowrite8)
Quoted 3 lines
...
...
Thanks!
Tali Perry
Nuvoton Technologies
Re: [PATCH v10 2/3] i2c: npcm7xx: Add Nuvoton NPCM I2C controller driver
Quoted 3 lines · Tali Perry
Quoted 1 line · Andy Shevchenko
On Sun, May 10, 2020 at 01:23:29PM +0300, Tali Perry wrote:
...
Quoted 11 lines
Quoted 3 lines
Quoted 1 line
+#if
IS_ENABLED(CONFIG_DEBUG_FS)Why?
We wanted to add an optional feature to track i2c slave status.
the NPCM has 16 channels handling multiple devices each. Some of the devices are polled periodically, and might power down.
The user wanted to implement a health monitoring option
to occasionally check the status of the buses (how many timeouts, recovery etc.) This feature is optional and depends onCONFIG_DEBUG_FSThe counters are exposed to user through the file system.
What I meant is why do you need an #ifdef?
...
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
+#define
I2C_NUM_OF_ADDR10Is it 10-bit address support or what?
No, the NPCM has an option to respond to multiple slave addresses (10 own slave addresses)
Perhaps more descriptive name then?
...
Quoted 12 lines
Quoted 12 lines
Quoted 11 lines
drivers/i2c/busses/i2c-npcm7xx.c new file +11
-0+// Repeat the following sequence until SDA is released +do { +// Issue a single SCL toggle +iowrite8 (NPCM_I2CCST_TGSCL , bus->reg +NPCM_I2CCST ); +udelay (20 ); +// If SDA line is inactive (high), stop +if (npcm_i2c_get_SDA (_adap)) { + done = true; + status =0 ; + } + }while (!done && iter--);
Quoted 4 lines
Quoted 1 line
readx_poll_timeout()?Not exactly,
readx_poll_timeoutincludes only a read operation, here there is a write in the middle. (iowrite8)
Ah, indeed. Perhaps time to add writex_poll_timeout() ?
Signature · 2 lines
With Best Regards, Andy Shevchenko
Tali Perry
maintainerPATCH v10 3/3 i2c: npcm7xx: Add support for slave mode for Nuvoton
In reply to Tali PerryAdd support for slave mode for Nuvoton
NPCM BMC I2C controller driver.
--- drivers/i2c/busses/i2c-npcm7xx.c | 614 +++++++++++++++++++++++++++++++ 1 file changed, 614 insertions(+)
drivers/i2c/busses/i2c-npcm7xx.c +614 -0
@@ -73,6 +73,24 @@ enum i2c_state { I2C_STOP_PENDING, }; +#if IS_ENABLED (CONFIG_I2C_SLAVE ) +// Module supports setting multiple own slave addresses +enum i2c_addr { +I2C_SLAVE_ADDR1 =0 , +I2C_SLAVE_ADDR2 , +I2C_SLAVE_ADDR3 , +I2C_SLAVE_ADDR4 , +I2C_SLAVE_ADDR5 , +I2C_SLAVE_ADDR6 , +I2C_SLAVE_ADDR7 , +I2C_SLAVE_ADDR8 , +I2C_SLAVE_ADDR9 , +I2C_SLAVE_ADDR10 , +I2C_GC_ADDR , +I2C_ARP_ADDR , +}; +#endif +// init register and default value required to enable module #define NPCM_I2CSEGCTL 0xE4#define NPCM_I2CSEGCTL_INIT_VAL 0x0333F000
@@ -100,6 +118,14 @@ enum i2c_state {#define NPCM_I2CADDR6 0x16#define NPCM_I2CADDR10 0x17 +#if IS_ENABLED (CONFIG_I2C_SLAVE ) +// I2CADDR array: because the addr regs are sprinkled all over the address space +const int npcm_i2caddr[10 ] = {NPCM_I2CADDR1 ,NPCM_I2CADDR2 ,NPCM_I2CADDR3 , +NPCM_I2CADDR4 ,NPCM_I2CADDR5 ,NPCM_I2CADDR6 , +NPCM_I2CADDR7 ,NPCM_I2CADDR8 ,NPCM_I2CADDR9 , +NPCM_I2CADDR10 }; +#endif +#define NPCM_I2CCTL4 0x1A#define NPCM_I2CCTL5 0x1B#define NPCM_I2CSCLLT 0x1C // SCL Low Time
@@ -273,6 +299,16 @@ struct npcm_i2c {unsigned long int_time_stamp;unsigned long bus_freq;// in kHz u32 xmits; +#if IS_ENABLED (CONFIG_I2C_SLAVE ) +u8 own_slave_addr; +struct i2c_client *slave; +int slv_rd_size; +int slv_rd_ind; +int slv_wr_size; +int slv_wr_ind; +u8 slv_rd_buf[I2C_HW_FIFO_SIZE ]; +u8 slv_wr_buf[I2C_HW_FIFO_SIZE ]; +#endif #if IS_ENABLED (CONFIG_DEBUG_FS )struct dentry *debugfs;/* debugfs device directory */ uint64_t ber_count;
@@ -316,6 +352,10 @@ static void npcm_i2c_init_params(struct npcm_i2c *bus) bus->int_time_stamp =0 ; bus->PEC_use = false; bus->PEC_mask =0 ; +#if IS_ENABLED (CONFIG_I2C_SLAVE ) +if (bus->slave) + bus->master_or_slave =I2C_SLAVE ; +#endif }static inline void npcm_i2c_wr_byte (struct npcm_i2c *bus,u8 data)
@@ -363,6 +403,18 @@ static void npcm_i2c_disable(struct npcm_i2c *bus) {u8 i2cctl2; +#if IS_ENABLED (CONFIG_I2C_SLAVE ) +int i; + +// select bank 0 for I2C addresses +npcm_i2c_select_bank (bus,I2C_BANK_0 ); + +// Slave addresses removal +for (i =I2C_SLAVE_ADDR1 ; i <I2C_NUM_OF_ADDR ; i++) +iowrite8 (0 , bus->reg + npcm_i2caddr[i]); + +npcm_i2c_select_bank (bus,I2C_BANK_1 ); +#endif // Disable module. i2cctl2 =ioread8 (bus->reg +NPCM_I2CCTL2 ); i2cctl2 = i2cctl2 & ~I2CCTL2_ENABLE ;
@@ -529,6 +581,63 @@ static inline void npcm_i2c_nack(struct npcm_i2c *bus)iowrite8 (val, bus->reg +NPCM_I2CCTL1 ); } +#if IS_ENABLED (CONFIG_I2C_SLAVE ) +static void npcm_i2c_slave_int_enable (struct npcm_i2c *bus,bool enable) +{ +u8 i2cctl1; + +// enable interrupt on slave match: + i2cctl1 =ioread8 (bus->reg +NPCM_I2CCTL1 ); + i2cctl1 &= ~NPCM_I2CCTL1_RWS ; +if (enable) + i2cctl1 |=NPCM_I2CCTL1_NMINTE ; +else + i2cctl1 &= ~NPCM_I2CCTL1_NMINTE ; +iowrite8 (i2cctl1, bus->reg +NPCM_I2CCTL1 ); +} + +static int npcm_i2c_slave_enable_l (struct npcm_i2c *bus, +enum i2c_addr addr_type,u8 addr, +bool enable) +{ +u8 i2cctl1; +u8 i2cctl3; +u8 sa_reg; + +// npcm_i2c_slave_int_enable(bus, false); + sa_reg = (addr &0x7F ) |FIELD_PREP (NPCM_I2CADDR_SAEN , enable); +if (addr_type ==I2C_GC_ADDR ) { + i2cctl1 =ioread8 (bus->reg +NPCM_I2CCTL1 ); +if (enable) + i2cctl1 |=NPCM_I2CCTL1_GCMEN ; +else + i2cctl1 &= ~NPCM_I2CCTL1_GCMEN ; +iowrite8 (i2cctl1, bus->reg +NPCM_I2CCTL1 ); +return 0 ; + } +if (addr_type ==I2C_ARP_ADDR ) { + i2cctl3 =ioread8 (bus->reg +NPCM_I2CCTL3 ); +if (enable) + i2cctl3 |=I2CCTL3_ARPMEN ; +else + i2cctl3 &= ~I2CCTL3_ARPMEN ; +iowrite8 (i2cctl3, bus->reg +NPCM_I2CCTL3 ); +return 0 ; + } +if (addr_type >=I2C_ARP_ADDR ) +return -EFAULT ; +// select bank 0 for address 3 to 10 +if (addr_type >I2C_SLAVE_ADDR2 ) +npcm_i2c_select_bank (bus,I2C_BANK_0 ); +// Set and enable the address +iowrite8 (sa_reg, bus->reg + npcm_i2caddr[(int )addr_type]); +npcm_i2c_slave_int_enable (bus, enable); +if (addr_type >I2C_SLAVE_ADDR2 ) +npcm_i2c_select_bank (bus,I2C_BANK_1 ); +return 0 ; +} +#endif +static void npcm_i2c_reset (struct npcm_i2c *bus) {// Save I2CCTL1 relevant bits. It is being cleared when the module
@@ -556,6 +665,12 @@ static void npcm_i2c_reset(struct npcm_i2c *bus)// Clear all fifo bits: iowrite8 (NPCM_I2CFIF_CTS_CLR_FIFO , bus->reg +NPCM_I2CFIF_CTS ); +#if IS_ENABLED (CONFIG_I2C_SLAVE ) +if (bus->slave) +npcm_i2c_slave_enable_l (bus,I2C_SLAVE_ADDR1 , bus->slave->addr, + true); +#endif + bus->state =I2C_IDLE ; }
@@ -620,6 +735,10 @@ static void npcm_i2c_callback(struct npcm_i2c *bus, } bus->operation =I2C_NO_OPER ; +#if IS_ENABLED (CONFIG_I2C_SLAVE ) +if (bus->slave) + bus->master_or_slave =I2C_SLAVE ; +#endif }static u32 npcm_i2c_fifo_usage (struct npcm_i2c *bus)
@@ -647,6 +766,33 @@ static void npcm_i2c_write_to_fifo_master(struct npcm_i2c *bus, } } +#if IS_ENABLED (CONFIG_I2C_SLAVE ) +static void npcm_i2c_write_to_fifo_slave (struct npcm_i2c *bus, +u16 max_bytes_to_send) +{ +// Fill the FIFO, while the FIFO is not full and there are more bytes to +// write +npcm_i2c_clear_fifo_int (bus); +npcm_i2c_clear_tx_fifo (bus); +iowrite8 (0 , bus->reg +NPCM_I2CTXF_CTL ); + +while ((max_bytes_to_send--) && + (I2C_HW_FIFO_SIZE -npcm_i2c_fifo_usage (bus))) { + +if (bus->slv_wr_size >0 ) { + bus->slv_wr_ind = bus->slv_wr_ind %I2C_HW_FIFO_SIZE ; +npcm_i2c_wr_byte (bus, + bus->slv_wr_buf[bus->slv_wr_ind]); + bus->slv_wr_ind++; + bus->slv_wr_ind = bus->slv_wr_ind %I2C_HW_FIFO_SIZE ; + bus->slv_wr_size--;// size indicates the # of bytes in +// the SW buffer, not HW fifo. + }else { +break ; + } + } +} +#endif /* * configure the FIFO before using it. If nread is -1 RX FIFO will not be * configured. same for nwrite
@@ -705,6 +851,21 @@ static void npcm_i2c_read_from_fifo(struct npcm_i2c *bus, u8 bytes_in_fifo)if (bus->rd_ind < bus->rd_size) bus->rd_buf[bus->rd_ind++] = data; } +#if IS_ENABLED (CONFIG_I2C_SLAVE ) + else { +if (bus->slave) { + bus->slv_rd_ind = bus->slv_rd_ind % +I2C_HW_FIFO_SIZE ; + bus->slv_rd_buf[bus->slv_rd_ind] = data; + bus->slv_rd_ind++; +if (bus->slv_rd_ind ==1 && bus->read_block_use) +// 1st byte is length in block protocol + bus->slv_rd_size = data + + (u8 )bus->PEC_use + + (u8 )bus->read_block_use; + } + } +#endif } }
@@ -727,6 +888,433 @@ static void npcm_i2c_master_abort(struct npcm_i2c *bus) } } +#if IS_ENABLED (CONFIG_I2C_SLAVE ) +static u8 npcm_i2c_get_slave_addr (struct npcm_i2c *bus, +enum i2c_addr addr_type) +{ +u8 slave_add; + +// select bank 0 for address 3 to 10 +if (addr_type >I2C_SLAVE_ADDR2 ) +npcm_i2c_select_bank (bus,I2C_BANK_0 ); + + slave_add =ioread8 (bus->reg + npcm_i2caddr[(int )addr_type]); + +if (addr_type >I2C_SLAVE_ADDR2 ) +npcm_i2c_select_bank (bus,I2C_BANK_1 ); + +return slave_add; +} + +static int npcm_i2c_remove_slave_addr (struct npcm_i2c *bus,u8 slave_add) +{ +int i; + +// Set the enable bit + slave_add |=0x80 ; +npcm_i2c_select_bank (bus,I2C_BANK_0 ); +for (i =I2C_SLAVE_ADDR1 ; i <I2C_NUM_OF_ADDR ; i++) { +if (ioread8 (bus->reg + npcm_i2caddr[i]) == slave_add) +iowrite8 (0 , bus->reg + npcm_i2caddr[i]); + } +npcm_i2c_select_bank (bus,I2C_BANK_1 ); +return 0 ; +} + +static int npcm_i2c_slave_get_wr_buf (struct npcm_i2c *bus) +{ +int i; +u8 value =0 ; +int ret = bus->slv_wr_ind; + +// fill a cyclic buffer +for (i =0 ; i <I2C_HW_FIFO_SIZE ; i++) { +if (bus->slv_wr_size >=I2C_HW_FIFO_SIZE ) +break ; +i2c_slave_event (bus->slave,I2C_SLAVE_READ_REQUESTED , &value); + bus->slv_wr_buf[(bus->slv_wr_ind + bus->slv_wr_size) % +I2C_HW_FIFO_SIZE ] = value; + bus->slv_wr_size++; +i2c_slave_event (bus->slave,I2C_SLAVE_READ_PROCESSED , &value); + } +return I2C_HW_FIFO_SIZE - ret; +} + +static void npcm_i2c_slave_send_rd_buf (struct npcm_i2c *bus) +{ +int i; + +for (i =0 ; i < bus->slv_rd_ind; i++) +i2c_slave_event (bus->slave,I2C_SLAVE_WRITE_RECEIVED , + &bus->slv_rd_buf[i]); + +// once we send bytes up, need to reset the counter of the wr buf +// got data from master (new offset in device), ignore wr fifo: +if (bus->slv_rd_ind) { + bus->slv_wr_size =0 ; + bus->slv_wr_ind =0 ; + } + + bus->slv_rd_ind =0 ; + bus->slv_rd_size = bus->adap.quirks->max_read_len; + +npcm_i2c_clear_fifo_int (bus); +npcm_i2c_clear_rx_fifo (bus); +} + +static bool npcm_i2c_slave_receive (struct npcm_i2c *bus,u16 nread, +u8 *read_data) +{ + bus->state =I2C_OPER_STARTED ; + bus->operation =I2C_READ_OPER ; + bus->slv_rd_size = nread; + bus->slv_rd_ind =0 ; + +iowrite8 (0 , bus->reg +NPCM_I2CTXF_CTL ); +iowrite8 (I2C_HW_FIFO_SIZE , bus->reg +NPCM_I2CRXF_CTL ); + +npcm_i2c_clear_tx_fifo (bus); +npcm_i2c_clear_rx_fifo (bus); + +return true; +} + +static bool npcm_i2c_slave_xmit (struct npcm_i2c *bus,u16 nwrite, +u8 *write_data) +{ +if (nwrite ==0 ) +return false; + + bus->state =I2C_OPER_STARTED ; + bus->operation =I2C_WRITE_OPER ; + +// get the next buffer +npcm_i2c_slave_get_wr_buf (bus); + +if (nwrite >0 ) +npcm_i2c_write_to_fifo_slave (bus, nwrite); + +return true; +} + +/* +* currently slave IF only supports single byte operations. +* in order to utilyze the npcm HW FIFO, the driver will ask for 16 bytes +* at a time, pack them in buffer, and then transmit them all together +* to the FIFO and onward to the bus. +* NACK on read will be once reached to bus->adap->quirks->max_read_len. +* sending a NACK wherever the backend requests for it is not supported. +* the next two functions allow reading to local buffer before writing it all +* to the HW FIFO. +* ret val: number of bytes read from the IF: +*/ +static int npcm_i2c_slave_wr_buf_sync (struct npcm_i2c *bus) +{ +int left_in_fifo; + + left_in_fifo =FIELD_GET (NPCM_I2CTXF_STS_TX_BYTES , +ioread8 (bus->reg +NPCM_I2CTXF_STS )); +if (left_in_fifo >=I2C_HW_FIFO_SIZE ) +return left_in_fifo; + +// fifo already full: +if (bus->slv_wr_size >=I2C_HW_FIFO_SIZE ) +return left_in_fifo; + +// update the wr fifo ind, back to the untransmitted bytes: + bus->slv_wr_ind = bus->slv_wr_ind - left_in_fifo; + bus->slv_wr_size = bus->slv_wr_size + left_in_fifo; + +if (bus->slv_wr_ind <0 ) + bus->slv_wr_ind +=I2C_HW_FIFO_SIZE ; + +return left_in_fifo; +} + +static void npcm_i2c_slave_rd_wr (struct npcm_i2c *bus) +{ +if (FIELD_GET (NPCM_I2CST_XMIT ,ioread8 (bus->reg +NPCM_I2CST ))) { +/* +* Slave got an address match with direction bit 1 so +* it should transmit data +* Write till the master will NACK +*/ + bus->operation =I2C_WRITE_OPER ; +npcm_i2c_slave_xmit (bus, + bus->adap.quirks->max_write_len, + bus->slv_wr_buf); + }else { +/* +* Slave got an address match with direction bit 0 +* so it should receive data. +* this module does not support saying no to bytes. +* it will always ACK. +*/ + bus->operation =I2C_READ_OPER ; +npcm_i2c_read_from_fifo (bus,npcm_i2c_fifo_usage (bus)); + bus->stop_ind =I2C_SLAVE_RCV_IND ; +npcm_i2c_slave_send_rd_buf (bus); +npcm_i2c_slave_receive (bus, + bus->adap.quirks->max_read_len, + bus->slv_rd_buf); + } +} + +static irqreturn_t npcm_i2c_int_slave_handler (struct npcm_i2c *bus) +{ +u8 val; +irqreturn_t ret =IRQ_NONE ; +u8 i2cst =ioread8 (bus->reg +NPCM_I2CST ); + +// Slave: A NACK has occurred +if (FIELD_GET (NPCM_I2CST_NEGACK , i2cst)) { + bus->stop_ind =I2C_NACK_IND ; +npcm_i2c_slave_wr_buf_sync (bus); +if (bus->fifo_use) +// clear the FIFO +iowrite8 (NPCM_I2CFIF_CTS_CLR_FIFO , + bus->reg +NPCM_I2CFIF_CTS ); + +// In slave write, NACK is OK, otherwise it is a problem + bus->stop_ind =I2C_NO_STATUS_IND ; + bus->operation =I2C_NO_OPER ; + bus->own_slave_addr =0xFF ; + +/* +* Slave has to wait for I2C_STOP to decide this is the end +* of the transaction. +* Therefore transaction is not yet considered as done +*/ +iowrite8 (NPCM_I2CST_NEGACK , bus->reg +NPCM_I2CST ); + + ret =IRQ_HANDLED ; + } + +// Slave mode: a Bus Error (BER) has been identified +if (FIELD_GET (NPCM_I2CST_BER , i2cst)) { +// Check whether bus arbitration or Start or Stop during data +// xfer bus arbitration problem should not result in recovery + bus->stop_ind =I2C_BUS_ERR_IND ; + +// wait for bus busy before clear fifo +iowrite8 (NPCM_I2CFIF_CTS_CLR_FIFO , bus->reg +NPCM_I2CFIF_CTS ); + + bus->state =I2C_IDLE ; + +// in BER case we might get 2 interrupts: one for slave one for +// master ( for a channel which is master\slave switching) +if (completion_done (&bus->cmd_complete) == false) { + bus->cmd_err = -EIO ; +complete (&bus->cmd_complete); + } + bus->own_slave_addr =0xFF ; +iowrite8 (NPCM_I2CST_BER , bus->reg +NPCM_I2CST ); + ret =IRQ_HANDLED ; + } + +// A Slave Stop Condition has been identified +if (FIELD_GET (NPCM_I2CST_SLVSTP , i2cst)) { +int bytes_in_fifo =npcm_i2c_fifo_usage (bus); + + bus->stop_ind =I2C_SLAVE_DONE_IND ; + +if (bus->operation ==I2C_READ_OPER ) { +npcm_i2c_read_from_fifo (bus, bytes_in_fifo); + +// Slave done transmitting or receiving +// if the buffer is empty nothing will be sent + } + +// Slave done transmitting or receiving +// if the buffer is empty nothing will be sent +npcm_i2c_slave_send_rd_buf (bus); + + bus->stop_ind =I2C_NO_STATUS_IND ; + +/* +* Note, just because we got here, it doesn't mean we through +* away the wr buffer. +* we keep it until the next received offset. +*/ + bus->operation =I2C_NO_OPER ; + bus->int_cnt =0 ; + bus->own_slave_addr =0xFF ; +i2c_slave_event (bus->slave,I2C_SLAVE_STOP ,0 ); +iowrite8 (NPCM_I2CST_SLVSTP , bus->reg +NPCM_I2CST ); +if (bus->fifo_use) { +npcm_i2c_clear_fifo_int (bus); +npcm_i2c_clear_rx_fifo (bus); +npcm_i2c_clear_tx_fifo (bus); + +iowrite8 (NPCM_I2CFIF_CTS_CLR_FIFO , + bus->reg +NPCM_I2CFIF_CTS ); + } + bus->state =I2C_IDLE ; + ret =IRQ_HANDLED ; + } + +// restart condition occurred and Rx-FIFO was not empty +if (bus->fifo_use &&FIELD_GET (NPCM_I2CFIF_CTS_SLVRSTR , +ioread8 (bus->reg +NPCM_I2CFIF_CTS ))) { + bus->stop_ind =I2C_SLAVE_RESTART_IND ; + bus->master_or_slave =I2C_SLAVE ; +if (bus->operation ==I2C_READ_OPER ) +npcm_i2c_read_from_fifo (bus,npcm_i2c_fifo_usage (bus)); + bus->operation =I2C_WRITE_OPER ; +iowrite8 (0 , bus->reg +NPCM_I2CRXF_CTL ); + val =NPCM_I2CFIF_CTS_CLR_FIFO |NPCM_I2CFIF_CTS_SLVRSTR | +NPCM_I2CFIF_CTS_RXF_TXE ; +iowrite8 (val, bus->reg +NPCM_I2CFIF_CTS ); +npcm_i2c_slave_rd_wr (bus); + ret =IRQ_HANDLED ; + } + +// A Slave Address Match has been identified +if (FIELD_GET (NPCM_I2CST_NMATCH , i2cst)) { +u8 info =0 ; + +// Address match automatically implies slave mode + bus->master_or_slave =I2C_SLAVE ; +npcm_i2c_clear_fifo_int (bus); +npcm_i2c_clear_rx_fifo (bus); +npcm_i2c_clear_tx_fifo (bus); +iowrite8 (0 , bus->reg +NPCM_I2CTXF_CTL ); +iowrite8 (I2C_HW_FIFO_SIZE , bus->reg +NPCM_I2CRXF_CTL ); +if (FIELD_GET (NPCM_I2CST_XMIT , i2cst)) { + bus->operation =I2C_WRITE_OPER ; + }else { +i2c_slave_event (bus->slave,I2C_SLAVE_WRITE_REQUESTED , + &info); + bus->operation =I2C_READ_OPER ; + } +if (bus->own_slave_addr ==0xFF ) {// unknown address +// Check which type of address match + val =ioread8 (bus->reg +NPCM_I2CCST ); +if (!!(NPCM_I2CCST_MATCH & val)) { +u16 addr; +enum i2c_addr eaddr; +u8 i2ccst2; +u8 i2ccst3; + + i2ccst3 =ioread8 (bus->reg +NPCM_I2CCST3 ); + i2ccst2 =ioread8 (bus->reg +NPCM_I2CCST2 ); + +// the i2c module can response to 10 own SA. +// check which one was addressed by the master. +// repond to the first one. + addr = ((i2ccst3 &0x07 ) <<7 ) | + (i2ccst2 &0x7F ); + info =ffs (addr); + eaddr = (enum i2c_addr )info; + addr =npcm_i2c_get_slave_addr (bus, eaddr); + addr &=0x7F ; + bus->own_slave_addr = addr; +if (bus->PEC_mask &BIT (info)) + bus->PEC_use = true; +else + bus->PEC_use = false; + }else { + val =ioread8 (bus->reg +NPCM_I2CCST ); +if (!!(NPCM_I2CCST_GCMATCH & val)) + bus->own_slave_addr =0 ; +if (!!(NPCM_I2CCST_ARPMATCH & val)) + bus->own_slave_addr =0x61 ; + } + }else { +/* +* Slave match can happen in two options: +* 1. Start, SA, read (slave read without further ado) +* 2. Start, SA, read, data, restart, SA, read, ... +* (slave read in fragmented mode) +* 3. Start, SA, write, data, restart, SA, read, .. +* (regular write-read mode) +*/ +if ((bus->state ==I2C_OPER_STARTED && + bus->operation ==I2C_READ_OPER && + bus->stop_ind ==I2C_SLAVE_XMIT_IND ) || + bus->stop_ind ==I2C_SLAVE_RCV_IND ) { +// slave transmit after slave receive w/o Slave +// Stop implies repeated start + bus->stop_ind =I2C_SLAVE_RESTART_IND ; + } + } + +if (FIELD_GET (NPCM_I2CST_XMIT , i2cst)) + bus->stop_ind =I2C_SLAVE_XMIT_IND ; +else + bus->stop_ind =I2C_SLAVE_RCV_IND ; + bus->state =I2C_SLAVE_MATCH ; +npcm_i2c_slave_rd_wr (bus); +iowrite8 (NPCM_I2CST_NMATCH , bus->reg +NPCM_I2CST ); + ret =IRQ_HANDLED ; + } + +// Slave SDA status is set - transmit or receive, slave +if (FIELD_GET (NPCM_I2CST_SDAST , i2cst) || + (bus->fifo_use && + (npcm_i2c_tx_fifo_empty (bus) ||npcm_i2c_rx_fifo_full (bus)))) { +npcm_i2c_slave_rd_wr (bus); +iowrite8 (NPCM_I2CST_SDAST , bus->reg +NPCM_I2CST ); + ret =IRQ_HANDLED ; + }//SDAST + +return ret; +} + +static int npcm_i2c_reg_slave (struct i2c_client *client) +{ +unsigned long lock_flags; +struct npcm_i2c *bus =i2c_get_adapdata (client->adapter); + + bus->slave = client; + +if (!bus->slave) +return -EINVAL ; + +if (client->flags &I2C_CLIENT_TEN ) +return -EAFNOSUPPORT ; + +spin_lock_irqsave (&bus->lock, lock_flags); + +npcm_i2c_init_params (bus); + bus->slv_rd_size =0 ; + bus->slv_wr_size =0 ; + bus->slv_rd_ind =0 ; + bus->slv_wr_ind =0 ; +if (client->flags &I2C_CLIENT_PEC ) + bus->PEC_use = true; + +dev_info (bus->dev,"i2c%d register slave SA=0x%x, PEC=%d\n" , bus->num, + client->addr, bus->PEC_use); + +npcm_i2c_slave_enable_l (bus,I2C_SLAVE_ADDR1 , client->addr, true); +npcm_i2c_clear_fifo_int (bus); +npcm_i2c_clear_rx_fifo (bus); +npcm_i2c_clear_tx_fifo (bus); +npcm_i2c_slave_int_enable (bus, true); + +spin_unlock_irqrestore (&bus->lock, lock_flags); +return 0 ; +} + +static int npcm_i2c_unreg_slave (struct i2c_client *client) +{ +struct npcm_i2c *bus = client->adapter->algo_data; +unsigned long lock_flags; + +spin_lock_irqsave (&bus->lock, lock_flags); +if (!bus->slave) { +spin_unlock_irqrestore (&bus->lock, lock_flags); +return -EINVAL ; + } +npcm_i2c_slave_int_enable (bus, false); +npcm_i2c_remove_slave_addr (bus, client->addr); + bus->slave =NULL ; +spin_unlock_irqrestore (&bus->lock, lock_flags); +return 0 ; +} +#endif // CONFIG_I2C_SLAVE +static void npcm_i2c_master_fifo_read (struct npcm_i2c *bus) {int rcount;
@@ -1393,6 +1981,9 @@ static int __npcm_i2c_init(struct npcm_i2c *bus, struct platform_device *pdev) bus->state =I2C_DISABLE ; bus->master_or_slave =I2C_SLAVE ; bus->int_time_stamp =0 ; +#if IS_ENABLED (CONFIG_I2C_SLAVE ) + bus->slave =NULL ; +#endif bus->xmits =0 ; ret =device_property_read_u32 (&pdev->dev,"bus-frequency" , &clk_freq);
@@ -1426,6 +2017,14 @@ static irqreturn_t npcm_i2c_bus_irq(int irq, void *dev_id)if (ret ==IRQ_HANDLED )return ret; } +#if IS_ENABLED (CONFIG_I2C_SLAVE ) +if (bus->slave) { + bus->master_or_slave =I2C_SLAVE ; + ret =npcm_i2c_int_slave_handler (bus); +if (ret ==IRQ_HANDLED ) +return ret; + } +#endif return IRQ_HANDLED ; }
@@ -1568,6 +2167,11 @@ static int npcm_i2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,*/ spin_lock_irqsave (&bus->lock, flags); bus_busy =ioread8 (bus->reg +NPCM_I2CCST ) &NPCM_I2CCST_BB ; +#if IS_ENABLED (CONFIG_I2C_SLAVE ) +if (!bus_busy && bus->slave) +iowrite8 ((bus->slave->addr &0x7F ), + bus->reg +NPCM_I2CADDR1 ); +#endif spin_unlock_irqrestore (&bus->lock, flags); }while (time_is_after_jiffies (time_left) && bus_busy);
@@ -1620,6 +2224,12 @@ static int npcm_i2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,if (bus->cmd_err == -EAGAIN ) ret =i2c_recover_bus (adap); +#if IS_ENABLED (CONFIG_I2C_SLAVE ) +// reenable slave if it was enabled +if (bus->slave) +iowrite8 ((bus->slave->addr &0x7F ) |NPCM_I2CADDR_SAEN , + bus->reg +NPCM_I2CADDR1 ); +#endif return bus->cmd_err; }
@@ -1642,6 +2252,10 @@ static const struct i2c_adapter_quirks npcm_i2c_quirks = {static const struct i2c_algorithm npcm_i2c_algo = { .master_xfer = npcm_i2c_master_xfer, .functionality = npcm_i2c_functionality, +#if IS_ENABLED(CONFIG_I2C_SLAVE) + .reg_slave = npcm_i2c_reg_slave, + .unreg_slave = npcm_i2c_unreg_slave, +#endif };#if IS_ENABLED (CONFIG_DEBUG_FS )
Signature · 7 lines
2.22.0 _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel