Devicetree
 help / color / mirror / Atom feed
* [PATCH v5 03/11] dt-bindings: i2c: dw: Add Baikal-T1 SoC I2C controller
From: Serge Semin @ 2020-05-27 15:30 UTC (permalink / raw)
  To: Jarkko Nikula, Wolfram Sang, Rob Herring
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Thomas Bogendoerfer,
	Andy Shevchenko, Mika Westerberg, linux-mips, linux-i2c,
	devicetree, linux-kernel
In-Reply-To: <20200527153046.6172-1-Sergey.Semin@baikalelectronics.ru>

Add the "baikal,bt1-sys-i2c" compatible string to the DW I2C binding. Even
though the corresponding node is supposed to be a child of the Baikal-T1
System Controller, its reg property is left required for compatibility.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: linux-mips@vger.kernel.org

---

Changelog v2:
- Make the reg property being optional if it's Baikal-T1 System I2C DT
  node.

Changelog v3:
- Get back the reg property being mandatory even if it's Baikal-T1 System
  I2C DT node. Rob says it has to be in the DT node if there is a
  dedicated registers range in the System Controller registers space.
---
 Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
index 101d78e8f19d..8c9b3db1b1b8 100644
--- a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
+++ b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
@@ -31,6 +31,8 @@ properties:
         items:
           - const: mscc,ocelot-i2c
           - const: snps,designware-i2c
+      - description: Baikal-T1 SoC System I2C controller
+        const: baikal,bt1-sys-i2c
 
   reg:
     minItems: 1
-- 
2.26.2


^ permalink raw reply related

* [PATCH v5 02/11] dt-bindings: i2c: Discard i2c-slave flag from the DW I2C example
From: Serge Semin @ 2020-05-27 15:30 UTC (permalink / raw)
  To: Jarkko Nikula, Wolfram Sang, Rob Herring
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Thomas Bogendoerfer,
	Andy Shevchenko, Mika Westerberg, linux-mips, linux-i2c,
	devicetree, linux-kernel
In-Reply-To: <20200527153046.6172-1-Sergey.Semin@baikalelectronics.ru>

dtc currently doesn't support I2C_OWN_SLAVE_ADDRESS flag set in the
i2c "reg" property. If it is the compiler will print a warning:

Warning (i2c_bus_reg): /example-2/i2c@1120000/eeprom@64: I2C bus unit address format error, expected "40000064"
Warning (i2c_bus_reg): /example-2/i2c@1120000/eeprom@64:reg: I2C address must be less than 10-bits, got "0x40000064"

In order to silence dtc up let's discard the flag from the DW I2C DT
binding example for now. Just revert this commit when dtc is fixed.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: linux-mips@vger.kernel.org

---

Changelog v3:
- This is a new patch created as a result of the Rob request to remove
  the EEPROM-slave bit setting in the DT binndings example until the dtc
  is fixed.
---
 Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
index 4bd430b2b41d..101d78e8f19d 100644
--- a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
+++ b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
@@ -137,7 +137,7 @@ examples:
 
       eeprom@64 {
         compatible = "linux,slave-24c02";
-        reg = <0x40000064>;
+        reg = <0x64>;
       };
     };
   - |
-- 
2.26.2


^ permalink raw reply related

* [PATCH v5 05/11] i2c: designware: slave: Set DW I2C core module dependency
From: Serge Semin @ 2020-05-27 15:30 UTC (permalink / raw)
  To: Jarkko Nikula, Wolfram Sang
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Thomas Bogendoerfer,
	Andy Shevchenko, Mika Westerberg, Rob Herring, linux-mips,
	devicetree, linux-i2c, linux-kernel
In-Reply-To: <20200527153046.6172-1-Sergey.Semin@baikalelectronics.ru>

DW APB I2C slave code in fact depends on the DW I2C driver core, but not
on the platform code as it used to be before commit 90bc1ee6de9f ("i2c:
designware: Allow slave mode for PCI enumerated devices"). Yes, the I2C
slave interface is currently supported by both the platform and PCI
versions of the IP core, but it still depends on the DW I2C core
functionality and must be available only if the last one is enabled.
So make sure the DW APB I2C slave config is only available if the
I2C_DESIGNWARE_CORE config is enabled.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
 drivers/i2c/busses/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 1d940810e47e..7f92f6a96042 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -529,6 +529,7 @@ config I2C_DESIGNWARE_CORE
 
 config I2C_DESIGNWARE_SLAVE
 	bool "Synopsys DesignWare Slave"
+	depends on I2C_DESIGNWARE_CORE
 	select I2C_SLAVE
 	help
 	  If you say yes to this option, support will be included for the
-- 
2.26.2


^ permalink raw reply related

* [PATCH v5 00/11] i2c: designeware: Add Baikal-T1 System I2C support
From: Serge Semin @ 2020-05-27 15:30 UTC (permalink / raw)
  To: Jarkko Nikula, Wolfram Sang
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Maxim Kaurkin,
	Pavel Parkhomenko, Ramil Zaripov, Ekaterina Skachko, Vadim Vlasov,
	Alexey Kolotnikov, Thomas Bogendoerfer, Andy Shevchenko,
	Mika Westerberg, Rob Herring, linux-mips, linux-i2c, devicetree,
	linux-kernel

Jarkko, Wolfram, the merge window is upon us, please review/merge in/whatever
the patchset.

Initially this has been a small patchset which embedded the Baikal-T1
System I2C support into the DW APB I2C driver as is by using a simplest
way. After a short discussion with Andy we decided to implement what he
suggested (introduce regmap-based accessors and create a glue driver) and
even more than that to provide some cleanups of the code. So here is what
this patchset consists of.

First of all we've found out that current implementation of scripts/dtc
didn't support i2c dt nodes with 10bit and slave flags set in the
reg property. You'll see an error if you try to dt_binding_check it.
So the very first patch fixes the problem by adding these flags support
into the check_i2c_bus_reg() method.

Traditionally we converted the plain text-based DT binding to the DT schema
and added Baikal-T1 System I2C device support there. This required to mark
the reg property redundant for Baikal-T1 I2C since its reg-space is
indirectly accessed by means of the System Controller cmd/read/write
registers.

Then as Andy suggested we replaced the Synopsys DW APB I2C common driver
registers IO accessors into the regmap API methods. This doesn't change
the code logic much, though in two places we managed to replace some bulky
peaces of code with a ready-to-use regmap methods.

Additionally before adding the glue layer API we initiated a set of cleanups:
- Define components of the multi-object drivers (like i2c-designware-core.o
  and i2c-designware-paltform.o) with using `-y` suffixed makefile
  variables instead of `-objs` suffixed one. This is encouraged by
  Documentation/kbuild/makefiles.rst text since `-objs` is supposed to be used
  to build host programs.
- Make DW I2C slave driver depended on the DW I2C core code instead of the
  platform one, which it really is.
- Move Intel Baytrail semaphore feature to the platform if-clause of the
  kernel config.

After this we finally can introduce the glue layer API for the DW APB I2C
platform driver. So there are three methods exported from the driver:
i2c_dw_plat_setup(), i2c_dw_plat_clear(), &i2c_dw_plat_dev_pm_ops to
setup, cleanup and add PM operations to the glue driven I2C device. Before
setting the platform DW I2C device up the glue probe code is supposed to
create an instance of DW I2C device generic object and pre-initialize
its `struct device` pointer together with optional platform-specific
flags. In addition to that we converted the MSCC Ocelot SoC I2C specific
code into the glue layer seeing it's really too specific and, which is more
important, isn't that complicated so we could unpin it without much of
worrying to break something.

Meanwhile we discovered that MODEL_CHERRYTRAIL and MODEL_MASK actually
were no longer used in the code. MODEL_MSCC flag has been discarded since
the MSCC Ocelot I2C code conversion to the glue driver. So now we can get
rid of all the MODEL-specific flags.

Finally we introduced a glue driver with Baikal-T1 System I2C device
support. The driver probe tries to find a syscon regmap, creates the DW
APB I2C regmap based on it and passes it further to the DW I2C device
descriptor. Then it does normal DW APB I2C platform setup by calling a
generic setup method. Cleanup is straightforward. It's just calling a
generic DW APB I2C clean method.

This patchset is rebased and tested on the i2c/for-next (5.7-rc7):
base-commit: 228f95c14949 ("Merge remote-tracking branch 'spi/for-5.8' into spi-next")

Note new vendor prefix for Baikal-T1 System I2C device is added in the
framework of the next patchset:
https://lkml.org/lkml/2020/5/6/1047

Changelog v2:
- Fix the SoB tags.
- Use a shorter summary describing the bindings convertion patch.
- Patch "i2c: designware: Detect the FIFO size in the common code" has
  been acked by Jarkko and applied by Wolfram to for-next so drop it from
  the set.
- Patch "i2c: designware: Discard i2c_dw_read_comp_param() function" has
  been acked by Jarkko and applied by Wolfram to for-next so drop it from
  the set.
- Make sure that "mscc,ocelot-i2c" compatible node may have up to two
  registers space defined in the DT node, while normal DW I2C controller
  will have only one registers space.
- Add "mscc,ocelot-i2c" DT schema example to test the previous fix.
- Declare "unevaluatedProperties" property instead of
  "additionalProperties" one in the DT schema.
- Due to the previous fix we can now discard the dummy boolean properties
  declaration, since the proper type evaluation will be performed by the
  generic i2c-controller.yaml schema.
- Refactor the DW I2C APB driver related series to address the Andies
  notes.
- Convert DW APB I2C driver to using regmap instead of handwritten
  accessors.
- Use `-y` to build multi-object DW APB drivers.
- Fix DW APB I2C slave code dependency. It should depend on
  I2C_DESIGNWARE_CORE instead I2C_DESIGNWARE_PLATFORM.
- Move Baytrail semaphore config to the platform if-clause.
- Introduce a glue-layer platform driver API.
- Unpin Microsemi Ocelot I2C code into a glue driver.
- Remove MODEL_CHERRYTRAIL and MODEL_MASK as no longer needed.
- Add support for custom regmap passed from glue driver.
- Add Baikal-T1 System I2C support in a dedicated glue layer driver.

Link: https://lore.kernel.org/linux-i2c/20200510095019.20981-1-Sergey.Semin@baikalelectronics.ru/
Changelog v3:
- Move fixes and less invasive patches to the head of the series.
- Add patch "dt-bindings: i2c: Discard i2c-slave flag from the DW I2C
  example" since Rob says the flag can be discarded until dtc is fixed.
- Add patch "i2c: designware: Retrieve quirk flags as early as possible"
  as a first preparation before adding Baikal-T1 System I2C support.
- Add patch "i2c: designware: Move reg-space remapping into a dedicated
  function" as a second preparation before adding Baikal-T1 System I2C
  support.
- Add patch "i2c: designware: Add Baikal-T1 System I2C support", which
  integrates the Baikal-T1 I2C support into the DW I2C platform driver.
- Get back the reg property being mandatory even if it's Baikal-T1 System
  I2C DT node. Rob says it has to be in the DT node if there is a
  dedicated registers range in the System Controller registers space.
- Replace if-endif clause around the I2C_DESIGNWARE_BAYTRAIL config
  with "depends on" operator.

Link: https://lore.kernel.org/linux-i2c/20200526215528.16417-1-Sergey.Semin@baikalelectronics.ru/
Changelog v4:
- Rebase on top of the i2c/for-next branch.
- Use PTR_ERR_OR_ZERO() helper in the bt1_i2c_request_regs() and
  in the dw_i2c_plat_request_regs() methods.
- Discard devm_platform_get_and_ioremap_resource() utilization.
- Discard patch "scripts/dtc: check: Add 10bit/slave i2c reg flags
  support" since it must be merged in to the dtc upstream repository.

Link: https://lore.kernel.org/linux-i2c/20200527120111.5781-1-Sergey.Semin@baikalelectronics.ru
Changelog v5:
- Replace or-assignment with just assignment operator when getting
  the quirk flags.
- Keep alphabetical order of the include statements.
- Discard explicit u16-type cast in the dw_reg_write_word() method.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Maxim Kaurkin <Maxim.Kaurkin@baikalelectronics.ru>
Cc: Pavel Parkhomenko <Pavel.Parkhomenko@baikalelectronics.ru>
Cc: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
Cc: Ekaterina Skachko <Ekaterina.Skachko@baikalelectronics.ru>
Cc: Vadim Vlasov <V.Vlasov@baikalelectronics.ru>
Cc: Alexey Kolotnikov <Alexey.Kolotnikov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: linux-i2c@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

Serge Semin (11):
  dt-bindings: i2c: Convert DW I2C binding to DT schema
  dt-bindings: i2c: Discard i2c-slave flag from the DW I2C example
  dt-bindings: i2c: dw: Add Baikal-T1 SoC I2C controller
  i2c: designware: Use `-y` to build multi-object modules
  i2c: designware: slave: Set DW I2C core module dependency
  i2c: designware: Add Baytrail sem config DW I2C platform dependency
  i2c: designware: Discard Cherry Trail model flag
  i2c: designware: Convert driver to using regmap API
  i2c: designware: Retrieve quirk flags as early as possible
  i2c: designware: Move reg-space remapping into a dedicated function
  i2c: designware: Add Baikal-T1 System I2C support

 .../bindings/i2c/i2c-designware.txt           |  73 -------
 .../bindings/i2c/snps,designware-i2c.yaml     | 156 +++++++++++++++
 drivers/i2c/busses/Kconfig                    |  28 +--
 drivers/i2c/busses/Makefile                   |  18 +-
 drivers/i2c/busses/i2c-designware-common.c    | 178 +++++++++++++-----
 drivers/i2c/busses/i2c-designware-core.h      |  28 +--
 drivers/i2c/busses/i2c-designware-master.c    | 125 ++++++------
 drivers/i2c/busses/i2c-designware-pcidrv.c    |   1 -
 drivers/i2c/busses/i2c-designware-platdrv.c   |  96 +++++++++-
 drivers/i2c/busses/i2c-designware-slave.c     |  77 ++++----
 10 files changed, 520 insertions(+), 260 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-designware.txt
 create mode 100644 Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml

-- 
2.26.2


^ permalink raw reply

* [PATCH v5 07/11] i2c: designware: Discard Cherry Trail model flag
From: Serge Semin @ 2020-05-27 15:30 UTC (permalink / raw)
  To: Jarkko Nikula, Wolfram Sang, Andy Shevchenko, Mika Westerberg
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Thomas Bogendoerfer,
	Rob Herring, linux-mips, devicetree, linux-i2c, linux-kernel
In-Reply-To: <20200527153046.6172-1-Sergey.Semin@baikalelectronics.ru>

A PM workaround activated by the flag MODEL_CHERRYTRAIL has been removed
since commit 9cbeeca05049 ("i2c: designware: Remove Cherry Trail PMIC I2C
bus pm_disabled workaround"), but the flag most likely by mistake has been
left in the Dw I2C drivers. Let's remove it. Since MODEL_MSCC_OCELOT is
the only model-flag left, redefine it to be 0x100 so setting a very first
bit in the MODEL_MASK bits range.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org

---

Changelog v3:
- Since MSCC and Baikal-T1 will be a part of the platform driver code, we
  have to preserve the MODEL_MASK macro to use it to filter the model
  flags during the IP-specific quirks activation.
---
 drivers/i2c/busses/i2c-designware-core.h    | 3 +--
 drivers/i2c/busses/i2c-designware-pcidrv.c  | 1 -
 drivers/i2c/busses/i2c-designware-platdrv.c | 2 +-
 3 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 150de5e5c31b..b9ef9b0deef0 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -289,8 +289,7 @@ struct dw_i2c_dev {
 #define ACCESS_INTR_MASK	0x00000004
 #define ACCESS_NO_IRQ_SUSPEND	0x00000008
 
-#define MODEL_CHERRYTRAIL	0x00000100
-#define MODEL_MSCC_OCELOT	0x00000200
+#define MODEL_MSCC_OCELOT	0x00000100
 #define MODEL_MASK		0x00000f00
 
 u32 dw_readl(struct dw_i2c_dev *dev, int offset);
diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index 11a5e4751eab..947c096f86e3 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -149,7 +149,6 @@ static struct dw_pci_controller dw_pci_controllers[] = {
 	},
 	[cherrytrail] = {
 		.bus_num = -1,
-		.flags = MODEL_CHERRYTRAIL,
 		.scl_sda_cfg = &byt_config,
 	},
 	[elkhartlake] = {
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index f6d2c96e35ce..ca057aa9eac4 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -44,7 +44,7 @@ static const struct acpi_device_id dw_i2c_acpi_match[] = {
 	{ "INT3432", 0 },
 	{ "INT3433", 0 },
 	{ "80860F41", ACCESS_NO_IRQ_SUSPEND },
-	{ "808622C1", ACCESS_NO_IRQ_SUSPEND | MODEL_CHERRYTRAIL },
+	{ "808622C1", ACCESS_NO_IRQ_SUSPEND },
 	{ "AMD0010", ACCESS_INTR_MASK },
 	{ "AMDI0010", ACCESS_INTR_MASK },
 	{ "AMDI0510", 0 },
-- 
2.26.2


^ permalink raw reply related

* [PATCH v5 06/11] i2c: designware: Add Baytrail sem config DW I2C platform dependency
From: Serge Semin @ 2020-05-27 15:30 UTC (permalink / raw)
  To: Jarkko Nikula, Wolfram Sang
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Thomas Bogendoerfer,
	Andy Shevchenko, Mika Westerberg, Rob Herring, linux-mips,
	devicetree, linux-i2c, linux-kernel
In-Reply-To: <20200527153046.6172-1-Sergey.Semin@baikalelectronics.ru>

Currently Intel Baytrail I2C semaphore is a feature of the DW APB I2C
platform driver. It's a bit confusing to see it's config in the menu at
some separated place with no reference to the platform code. Let's move the
config definition to be below the I2C_DESIGNWARE_PLATFORM config and mark
it with "depends on I2C_DESIGNWARE_PLATFORM" statement. By doing so the
config menu will display the feature right below the DW I2C platform
driver item and will indent it to the right so signifying its belonging.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org

---

Changelog v3:
- Replace if-endif clause around the I2C_DESIGNWARE_BAYTRAIL config
  with "depends on" operator.
---
 drivers/i2c/busses/Kconfig | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 7f92f6a96042..7cd279c36898 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -549,20 +549,10 @@ config I2C_DESIGNWARE_PLATFORM
 	  This driver can also be built as a module.  If so, the module
 	  will be called i2c-designware-platform.
 
-config I2C_DESIGNWARE_PCI
-	tristate "Synopsys DesignWare PCI"
-	depends on PCI
-	select I2C_DESIGNWARE_CORE
-	help
-	  If you say yes to this option, support will be included for the
-	  Synopsys DesignWare I2C adapter. Only master mode is supported.
-
-	  This driver can also be built as a module.  If so, the module
-	  will be called i2c-designware-pci.
-
 config I2C_DESIGNWARE_BAYTRAIL
 	bool "Intel Baytrail I2C semaphore support"
 	depends on ACPI
+	depends on I2C_DESIGNWARE_PLATFORM
 	depends on (I2C_DESIGNWARE_PLATFORM=m && IOSF_MBI) || \
 		   (I2C_DESIGNWARE_PLATFORM=y && IOSF_MBI=y)
 	help
@@ -572,6 +562,17 @@ config I2C_DESIGNWARE_BAYTRAIL
 	  the platform firmware controlling it. You should say Y if running on
 	  a BayTrail system using the AXP288.
 
+config I2C_DESIGNWARE_PCI
+	tristate "Synopsys DesignWare PCI"
+	depends on PCI
+	select I2C_DESIGNWARE_CORE
+	help
+	  If you say yes to this option, support will be included for the
+	  Synopsys DesignWare I2C adapter. Only master mode is supported.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called i2c-designware-pci.
+
 config I2C_DIGICOLOR
 	tristate "Conexant Digicolor I2C driver"
 	depends on ARCH_DIGICOLOR || COMPILE_TEST
-- 
2.26.2


^ permalink raw reply related

* [PATCH v5 10/11] i2c: designware: Move reg-space remapping into a dedicated function
From: Serge Semin @ 2020-05-27 15:30 UTC (permalink / raw)
  To: Jarkko Nikula, Wolfram Sang, Andy Shevchenko, Mika Westerberg
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Thomas Bogendoerfer,
	Rob Herring, linux-mips, devicetree, linux-i2c, linux-kernel
In-Reply-To: <20200527153046.6172-1-Sergey.Semin@baikalelectronics.ru>

This is a preparation patch before adding a quirk with custom registers
map creation required for the Baikal-T1 System I2C support.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org

---

Changelog v3:
- This is a new patch, which has been created due to declining the
  glue-layer approach.

Changelog v4:
- Use PTR_ERR_OR_ZERO() helper in the bt1_i2c_request_regs() method.
- Discard devm_platform_get_and_ioremap_resource() utilization.
---
 drivers/i2c/busses/i2c-designware-platdrv.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 38657d821c72..9d467fa0e163 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -108,6 +108,15 @@ static void dw_i2c_plat_pm_cleanup(struct dw_i2c_dev *dev)
 		pm_runtime_put_noidle(dev->dev);
 }
 
+static int dw_i2c_plat_request_regs(struct dw_i2c_dev *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev->dev);
+
+	dev->base = devm_platform_ioremap_resource(pdev, 0);
+
+	return PTR_ERR_OR_ZERO(dev->base);
+}
+
 static int dw_i2c_plat_probe(struct platform_device *pdev)
 {
 	struct dw_i2c_platform_data *pdata = dev_get_platdata(&pdev->dev);
@@ -125,15 +134,14 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	dev->flags = (uintptr_t)device_get_match_data(&pdev->dev);
-
-	dev->base = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(dev->base))
-		return PTR_ERR(dev->base);
-
 	dev->dev = &pdev->dev;
 	dev->irq = irq;
 	platform_set_drvdata(pdev, dev);
 
+	ret = dw_i2c_plat_request_regs(dev);
+	if (ret)
+		return ret;
+
 	dev->rst = devm_reset_control_get_optional_exclusive(&pdev->dev, NULL);
 	if (IS_ERR(dev->rst))
 		return PTR_ERR(dev->rst);
-- 
2.26.2


^ permalink raw reply related

* [PATCH v5 11/11] i2c: designware: Add Baikal-T1 System I2C support
From: Serge Semin @ 2020-05-27 15:30 UTC (permalink / raw)
  To: Jarkko Nikula, Wolfram Sang, Andy Shevchenko, Mika Westerberg
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Thomas Bogendoerfer,
	Rob Herring, linux-mips, devicetree, linux-i2c, linux-kernel
In-Reply-To: <20200527153046.6172-1-Sergey.Semin@baikalelectronics.ru>

Baikal-T1 System Controller is equipped with a dedicated I2C Controller
which functionality is based on the DW APB I2C IP-core, the only
difference in a way it' registers are accessed. There are three access
register provided in the System Controller registers map, which indirectly
address the normal DW APB I2C registers space. So in order to have the
Baikal-T1 System I2C Controller supported by the common DW APB I2C driver
we created a dedicated Dw I2C controller model quirk, which retrieves the
syscon regmap from the parental dt node and creates a new regmap based on
it.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org

---

Changelog v3:
- This is a new patch, which has been created due to declining the
  glue-layer approach.

Changelog v4:
- Use PTR_ERR_OR_ZERO() helper in the bt1_i2c_request_regs() method.
---
 drivers/i2c/busses/Kconfig                  |  3 +-
 drivers/i2c/busses/i2c-designware-core.h    |  3 +
 drivers/i2c/busses/i2c-designware-platdrv.c | 78 ++++++++++++++++++++-
 3 files changed, 81 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 259e2325712a..0cf7aea30138 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -541,8 +541,9 @@ config I2C_DESIGNWARE_SLAVE
 
 config I2C_DESIGNWARE_PLATFORM
 	tristate "Synopsys DesignWare Platform"
-	select I2C_DESIGNWARE_CORE
 	depends on (ACPI && COMMON_CLK) || !ACPI
+	select I2C_DESIGNWARE_CORE
+	select MFD_SYSCON if MIPS_BAIKAL_T1
 	help
 	  If you say yes to this option, support will be included for the
 	  Synopsys DesignWare I2C adapter.
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index f5bbe3d6bcf8..556673a1f61b 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -183,6 +183,7 @@ struct reset_control;
  * struct dw_i2c_dev - private i2c-designware data
  * @dev: driver model device node
  * @map: IO registers map
+ * @sysmap: System controller registers map
  * @base: IO registers pointer
  * @ext: Extended IO registers pointer
  * @cmd_complete: tx completion indicator
@@ -235,6 +236,7 @@ struct reset_control;
 struct dw_i2c_dev {
 	struct device		*dev;
 	struct regmap		*map;
+	struct regmap		*sysmap;
 	void __iomem		*base;
 	void __iomem		*ext;
 	struct completion	cmd_complete;
@@ -290,6 +292,7 @@ struct dw_i2c_dev {
 #define ACCESS_NO_IRQ_SUSPEND	0x00000002
 
 #define MODEL_MSCC_OCELOT	0x00000100
+#define MODEL_BAIKAL_BT1	0x00000200
 #define MODEL_MASK		0x00000f00
 
 int i2c_dw_init_regmap(struct dw_i2c_dev *dev);
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 9d467fa0e163..240348ea9ff9 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -18,6 +18,7 @@
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/platform_data/i2c-designware.h>
@@ -25,6 +26,7 @@
 #include <linux/pm.h>
 #include <linux/pm_runtime.h>
 #include <linux/property.h>
+#include <linux/regmap.h>
 #include <linux/reset.h>
 #include <linux/sched.h>
 #include <linux/slab.h>
@@ -58,6 +60,63 @@ MODULE_DEVICE_TABLE(acpi, dw_i2c_acpi_match);
 #endif
 
 #ifdef CONFIG_OF
+#define BT1_I2C_CTL			0x100
+#define BT1_I2C_CTL_ADDR_MASK		GENMASK(7, 0)
+#define BT1_I2C_CTL_WR			BIT(8)
+#define BT1_I2C_CTL_GO			BIT(31)
+#define BT1_I2C_DI			0x104
+#define BT1_I2C_DO			0x108
+
+static int bt1_i2c_read(void *context, unsigned int reg, unsigned int *val)
+{
+	struct dw_i2c_dev *dev = context;
+	int ret;
+
+	/*
+	 * Note these methods shouldn't ever fail because the system controller
+	 * registers are memory mapped. We check the return value just in case.
+	 */
+	ret = regmap_write(dev->sysmap, BT1_I2C_CTL,
+			   BT1_I2C_CTL_GO | (reg & BT1_I2C_CTL_ADDR_MASK));
+	if (ret)
+		return ret;
+
+	return regmap_read(dev->sysmap, BT1_I2C_DO, val);
+}
+
+static int bt1_i2c_write(void *context, unsigned int reg, unsigned int val)
+{
+	struct dw_i2c_dev *dev = context;
+	int ret;
+
+	ret = regmap_write(dev->sysmap, BT1_I2C_DI, val);
+	if (ret)
+		return ret;
+
+	return regmap_write(dev->sysmap, BT1_I2C_CTL,
+		BT1_I2C_CTL_GO | BT1_I2C_CTL_WR | (reg & BT1_I2C_CTL_ADDR_MASK));
+}
+
+static struct regmap_config bt1_i2c_cfg = {
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+	.fast_io = true,
+	.reg_read = bt1_i2c_read,
+	.reg_write = bt1_i2c_write,
+	.max_register = DW_IC_COMP_TYPE
+};
+
+static int bt1_i2c_request_regs(struct dw_i2c_dev *dev)
+{
+	dev->sysmap = syscon_node_to_regmap(dev->dev->of_node->parent);
+	if (IS_ERR(dev->sysmap))
+		return PTR_ERR(dev->sysmap);
+
+	dev->map = devm_regmap_init(dev->dev, NULL, dev, &bt1_i2c_cfg);
+	return PTR_ERR_OR_ZERO(dev->map);
+}
+
 #define MSCC_ICPU_CFG_TWI_DELAY		0x0
 #define MSCC_ICPU_CFG_TWI_DELAY_ENABLE	BIT(0)
 #define MSCC_ICPU_CFG_TWI_SPIKE_FILTER	0x4
@@ -90,10 +149,16 @@ static int dw_i2c_of_configure(struct platform_device *pdev)
 static const struct of_device_id dw_i2c_of_match[] = {
 	{ .compatible = "snps,designware-i2c", },
 	{ .compatible = "mscc,ocelot-i2c", .data = (void *)MODEL_MSCC_OCELOT },
+	{ .compatible = "baikal,bt1-sys-i2c", .data = (void *)MODEL_BAIKAL_BT1 },
 	{},
 };
 MODULE_DEVICE_TABLE(of, dw_i2c_of_match);
 #else
+static int bt1_i2c_request_regs(struct dw_i2c_dev *dev)
+{
+	return -ENODEV;
+}
+
 static inline int dw_i2c_of_configure(struct platform_device *pdev)
 {
 	return -ENODEV;
@@ -111,10 +176,19 @@ static void dw_i2c_plat_pm_cleanup(struct dw_i2c_dev *dev)
 static int dw_i2c_plat_request_regs(struct dw_i2c_dev *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev->dev);
+	int ret;
 
-	dev->base = devm_platform_ioremap_resource(pdev, 0);
+	switch (dev->flags & MODEL_MASK) {
+	case MODEL_BAIKAL_BT1:
+		ret = bt1_i2c_request_regs(dev);
+		break;
+	default:
+		dev->base = devm_platform_ioremap_resource(pdev, 0);
+		ret = PTR_ERR_OR_ZERO(dev->base);
+		break;
+	}
 
-	return PTR_ERR_OR_ZERO(dev->base);
+	return ret;
 }
 
 static int dw_i2c_plat_probe(struct platform_device *pdev)
-- 
2.26.2


^ permalink raw reply related

* [PATCH v5 08/11] i2c: designware: Convert driver to using regmap API
From: Serge Semin @ 2020-05-27 15:30 UTC (permalink / raw)
  To: Jarkko Nikula, Wolfram Sang, Andy Shevchenko, Mika Westerberg
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Thomas Bogendoerfer,
	Rob Herring, devicetree, linux-mips, linux-i2c, linux-kernel
In-Reply-To: <20200527153046.6172-1-Sergey.Semin@baikalelectronics.ru>

Seeing the DW I2C driver is using flags-based accessors with two
conditional clauses it would be better to replace them with the regmap
API IO methods and to initialize the regmap object with read/write
callbacks specific to the controller registers map implementation. This
will be also handy for the drivers with non-standard registers mapping
(like an embedded into the Baikal-T1 System Controller DW I2C block, which
glue-driver is a part of this series).

As before the driver tries to detect the mapping setup at probe stage and
creates a regmap object accordingly, which will be used by the rest of the
code to correctly access the controller registers. In two places it was
appropriate to convert the hand-written read-modify-write and
read-poll-loop design patterns to the corresponding regmap API
ready-to-use methods.

Note the regmap IO methods return value is checked only at the probe
stage. The rest of the code won't do this because basically we have
MMIO-based regmap so non of the read/write methods can fail (this also
won't be needed for the Baikal-T1-specific I2C controller).

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org
Cc: linux-mips@vger.kernel.org

---

Changelog v5:
- Keep alphabetical order of the include statements.
- Discard explicit u16-type cast in the dw_reg_write_word() method.
---
 drivers/i2c/busses/Kconfig                 |   1 +
 drivers/i2c/busses/i2c-designware-common.c | 178 +++++++++++++++------
 drivers/i2c/busses/i2c-designware-core.h   |  22 ++-
 drivers/i2c/busses/i2c-designware-master.c | 125 ++++++++-------
 drivers/i2c/busses/i2c-designware-slave.c  |  77 ++++-----
 5 files changed, 248 insertions(+), 155 deletions(-)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 7cd279c36898..259e2325712a 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -526,6 +526,7 @@ config I2C_DAVINCI
 
 config I2C_DESIGNWARE_CORE
 	tristate
+	select REGMAP
 
 config I2C_DESIGNWARE_SLAVE
 	bool "Synopsys DesignWare Slave"
diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index ed302342f8db..cff4f69dadda 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -21,6 +21,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/pm_runtime.h>
+#include <linux/regmap.h>
 #include <linux/swab.h>
 #include <linux/types.h>
 
@@ -57,66 +58,123 @@ static char *abort_sources[] = {
 		"incorrect slave-transmitter mode configuration",
 };
 
-u32 dw_readl(struct dw_i2c_dev *dev, int offset)
+static int dw_reg_read(void *context, unsigned int reg, unsigned int *val)
 {
-	u32 value;
+	struct dw_i2c_dev *dev = context;
 
-	if (dev->flags & ACCESS_16BIT)
-		value = readw_relaxed(dev->base + offset) |
-			(readw_relaxed(dev->base + offset + 2) << 16);
-	else
-		value = readl_relaxed(dev->base + offset);
+	*val = readl_relaxed(dev->base + reg);
 
-	if (dev->flags & ACCESS_SWAP)
-		return swab32(value);
-	else
-		return value;
+	return 0;
 }
 
-void dw_writel(struct dw_i2c_dev *dev, u32 b, int offset)
+static int dw_reg_write(void *context, unsigned int reg, unsigned int val)
 {
-	if (dev->flags & ACCESS_SWAP)
-		b = swab32(b);
-
-	if (dev->flags & ACCESS_16BIT) {
-		writew_relaxed((u16)b, dev->base + offset);
-		writew_relaxed((u16)(b >> 16), dev->base + offset + 2);
-	} else {
-		writel_relaxed(b, dev->base + offset);
-	}
+	struct dw_i2c_dev *dev = context;
+
+	writel_relaxed(val, dev->base + reg);
+
+	return 0;
+}
+
+static int dw_reg_read_swab(void *context, unsigned int reg, unsigned int *val)
+{
+	struct dw_i2c_dev *dev = context;
+
+	*val = swab32(readl_relaxed(dev->base + reg));
+
+	return 0;
+}
+
+static int dw_reg_write_swab(void *context, unsigned int reg, unsigned int val)
+{
+	struct dw_i2c_dev *dev = context;
+
+	writel_relaxed(swab32(val), dev->base + reg);
+
+	return 0;
+}
+
+static int dw_reg_read_word(void *context, unsigned int reg, unsigned int *val)
+{
+	struct dw_i2c_dev *dev = context;
+
+	*val = readw_relaxed(dev->base + reg) |
+		(readw_relaxed(dev->base + reg + 2) << 16);
+
+	return 0;
+}
+
+static int dw_reg_write_word(void *context, unsigned int reg, unsigned int val)
+{
+	struct dw_i2c_dev *dev = context;
+
+	writew_relaxed(val, dev->base + reg);
+	writew_relaxed(val >> 16, dev->base + reg + 2);
+
+	return 0;
 }
 
 /**
- * i2c_dw_set_reg_access() - Set register access flags
+ * i2c_dw_init_regmap() - Initialize registers map
  * @dev: device private data
+ * @base: Registers map base address
  *
- * Autodetects needed register access mode and sets access flags accordingly.
- * This must be called before doing any other register access.
+ * Autodetects needed register access mode and creates the regmap with
+ * corresponding read/write callbacks. This must be called before doing any
+ * other register access.
  */
-int i2c_dw_set_reg_access(struct dw_i2c_dev *dev)
+int i2c_dw_init_regmap(struct dw_i2c_dev *dev)
 {
+	struct regmap_config map_cfg = {
+		.reg_bits = 32,
+		.val_bits = 32,
+		.reg_stride = 4,
+		.disable_locking = true,
+		.reg_read = dw_reg_read,
+		.reg_write = dw_reg_write,
+		.max_register = DW_IC_COMP_TYPE
+	};
 	u32 reg;
 	int ret;
 
+	/*
+	 * Skip detecting the registers map configuration if the regmap has
+	 * already been provided by a higher code.
+	 */
+	if (dev->map)
+		return 0;
+
 	ret = i2c_dw_acquire_lock(dev);
 	if (ret)
 		return ret;
 
-	reg = dw_readl(dev, DW_IC_COMP_TYPE);
+	reg = readl(dev->base + DW_IC_COMP_TYPE);
 	i2c_dw_release_lock(dev);
 
 	if (reg == swab32(DW_IC_COMP_TYPE_VALUE)) {
-		/* Configure register endianness access */
-		dev->flags |= ACCESS_SWAP;
+		map_cfg.reg_read = dw_reg_read_swab;
+		map_cfg.reg_write = dw_reg_write_swab;
 	} else if (reg == (DW_IC_COMP_TYPE_VALUE & 0x0000ffff)) {
-		/* Configure register access mode 16bit */
-		dev->flags |= ACCESS_16BIT;
+		map_cfg.reg_read = dw_reg_read_word;
+		map_cfg.reg_write = dw_reg_write_word;
 	} else if (reg != DW_IC_COMP_TYPE_VALUE) {
 		dev_err(dev->dev,
 			"Unknown Synopsys component type: 0x%08x\n", reg);
 		return -ENODEV;
 	}
 
+	/*
+	 * Note we'll check the return value of the regmap IO accessors only
+	 * at the probe stage. The rest of the code won't do this because
+	 * basically we have MMIO-based regmap so non of the read/write methods
+	 * can fail.
+	 */
+	dev->map = devm_regmap_init(dev->dev, NULL, dev, &map_cfg);
+	if (IS_ERR(dev->map)) {
+		dev_err(dev->dev, "Failed to init the registers map\n");
+		return PTR_ERR(dev->map);
+	}
+
 	return 0;
 }
 
@@ -327,11 +385,17 @@ int i2c_dw_set_sda_hold(struct dw_i2c_dev *dev)
 		return ret;
 
 	/* Configure SDA Hold Time if required */
-	reg = dw_readl(dev, DW_IC_COMP_VERSION);
+	ret = regmap_read(dev->map, DW_IC_COMP_VERSION, &reg);
+	if (ret)
+		goto err_release_lock;
+
 	if (reg >= DW_IC_SDA_HOLD_MIN_VERS) {
 		if (!dev->sda_hold_time) {
 			/* Keep previous hold time setting if no one set it */
-			dev->sda_hold_time = dw_readl(dev, DW_IC_SDA_HOLD);
+			ret = regmap_read(dev->map, DW_IC_SDA_HOLD,
+					  &dev->sda_hold_time);
+			if (ret)
+				goto err_release_lock;
 		}
 
 		/*
@@ -355,14 +419,16 @@ int i2c_dw_set_sda_hold(struct dw_i2c_dev *dev)
 		dev->sda_hold_time = 0;
 	}
 
+err_release_lock:
 	i2c_dw_release_lock(dev);
 
-	return 0;
+	return ret;
 }
 
 void __i2c_dw_disable(struct dw_i2c_dev *dev)
 {
 	int timeout = 100;
+	u32 status;
 
 	do {
 		__i2c_dw_disable_nowait(dev);
@@ -370,7 +436,8 @@ void __i2c_dw_disable(struct dw_i2c_dev *dev)
 		 * The enable status register may be unimplemented, but
 		 * in that case this test reads zero and exits the loop.
 		 */
-		if ((dw_readl(dev, DW_IC_ENABLE_STATUS) & 1) == 0)
+		regmap_read(dev->map, DW_IC_ENABLE_STATUS, &status);
+		if ((status & 1) == 0)
 			return;
 
 		/*
@@ -449,22 +516,23 @@ void i2c_dw_release_lock(struct dw_i2c_dev *dev)
  */
 int i2c_dw_wait_bus_not_busy(struct dw_i2c_dev *dev)
 {
-	int timeout = TIMEOUT;
+	u32 status;
+	int ret;
 
-	while (dw_readl(dev, DW_IC_STATUS) & DW_IC_STATUS_ACTIVITY) {
-		if (timeout <= 0) {
-			dev_warn(dev->dev, "timeout waiting for bus ready\n");
-			i2c_recover_bus(&dev->adapter);
+	ret = regmap_read_poll_timeout(dev->map, DW_IC_STATUS, status,
+				       !(status & DW_IC_STATUS_ACTIVITY),
+				       1100, 20000);
+	if (ret) {
+		dev_warn(dev->dev, "timeout waiting for bus ready\n");
 
-			if (dw_readl(dev, DW_IC_STATUS) & DW_IC_STATUS_ACTIVITY)
-				return -ETIMEDOUT;
-			return 0;
-		}
-		timeout--;
-		usleep_range(1000, 1100);
+		i2c_recover_bus(&dev->adapter);
+
+		regmap_read(dev->map, DW_IC_STATUS, &status);
+		if (!(status & DW_IC_STATUS_ACTIVITY))
+			ret = 0;
 	}
 
-	return 0;
+	return ret;
 }
 
 int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev)
@@ -490,15 +558,19 @@ int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev)
 		return -EIO;
 }
 
-void i2c_dw_set_fifo_size(struct dw_i2c_dev *dev)
+int i2c_dw_set_fifo_size(struct dw_i2c_dev *dev)
 {
 	u32 param, tx_fifo_depth, rx_fifo_depth;
+	int ret;
 
 	/*
 	 * Try to detect the FIFO depth if not set by interface driver,
 	 * the depth could be from 2 to 256 from HW spec.
 	 */
-	param = dw_readl(dev, DW_IC_COMP_PARAM_1);
+	ret = regmap_read(dev->map, DW_IC_COMP_PARAM_1, &param);
+	if (ret)
+		return ret;
+
 	tx_fifo_depth = ((param >> 16) & 0xff) + 1;
 	rx_fifo_depth = ((param >> 8)  & 0xff) + 1;
 	if (!dev->tx_fifo_depth) {
@@ -510,6 +582,8 @@ void i2c_dw_set_fifo_size(struct dw_i2c_dev *dev)
 		dev->rx_fifo_depth = min_t(u32, dev->rx_fifo_depth,
 				rx_fifo_depth);
 	}
+
+	return 0;
 }
 
 u32 i2c_dw_func(struct i2c_adapter *adap)
@@ -521,17 +595,19 @@ u32 i2c_dw_func(struct i2c_adapter *adap)
 
 void i2c_dw_disable(struct dw_i2c_dev *dev)
 {
+	u32 dummy;
+
 	/* Disable controller */
 	__i2c_dw_disable(dev);
 
 	/* Disable all interrupts */
-	dw_writel(dev, 0, DW_IC_INTR_MASK);
-	dw_readl(dev, DW_IC_CLR_INTR);
+	regmap_write(dev->map, DW_IC_INTR_MASK, 0);
+	regmap_read(dev->map, DW_IC_CLR_INTR, &dummy);
 }
 
 void i2c_dw_disable_int(struct dw_i2c_dev *dev)
 {
-	dw_writel(dev, 0, DW_IC_INTR_MASK);
+	regmap_write(dev->map, DW_IC_INTR_MASK, 0);
 }
 
 MODULE_DESCRIPTION("Synopsys DesignWare I2C bus adapter core");
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index b9ef9b0deef0..f5bbe3d6bcf8 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -15,6 +15,7 @@
 #include <linux/dev_printk.h>
 #include <linux/errno.h>
 #include <linux/i2c.h>
+#include <linux/regmap.h>
 #include <linux/types.h>
 
 #define DW_IC_DEFAULT_FUNCTIONALITY (I2C_FUNC_I2C |			\
@@ -126,8 +127,6 @@
 #define STATUS_WRITE_IN_PROGRESS	0x1
 #define STATUS_READ_IN_PROGRESS		0x2
 
-#define TIMEOUT			20 /* ms */
-
 /*
  * operation modes
  */
@@ -183,7 +182,9 @@ struct reset_control;
 /**
  * struct dw_i2c_dev - private i2c-designware data
  * @dev: driver model device node
+ * @map: IO registers map
  * @base: IO registers pointer
+ * @ext: Extended IO registers pointer
  * @cmd_complete: tx completion indicator
  * @clk: input reference clock
  * @pclk: clock required to access the registers
@@ -233,6 +234,7 @@ struct reset_control;
  */
 struct dw_i2c_dev {
 	struct device		*dev;
+	struct regmap		*map;
 	void __iomem		*base;
 	void __iomem		*ext;
 	struct completion	cmd_complete;
@@ -284,17 +286,13 @@ struct dw_i2c_dev {
 	bool			suspended;
 };
 
-#define ACCESS_SWAP		0x00000001
-#define ACCESS_16BIT		0x00000002
-#define ACCESS_INTR_MASK	0x00000004
-#define ACCESS_NO_IRQ_SUSPEND	0x00000008
+#define ACCESS_INTR_MASK	0x00000001
+#define ACCESS_NO_IRQ_SUSPEND	0x00000002
 
 #define MODEL_MSCC_OCELOT	0x00000100
 #define MODEL_MASK		0x00000f00
 
-u32 dw_readl(struct dw_i2c_dev *dev, int offset);
-void dw_writel(struct dw_i2c_dev *dev, u32 b, int offset);
-int i2c_dw_set_reg_access(struct dw_i2c_dev *dev);
+int i2c_dw_init_regmap(struct dw_i2c_dev *dev);
 u32 i2c_dw_scl_hcnt(u32 ic_clk, u32 tSYMBOL, u32 tf, int cond, int offset);
 u32 i2c_dw_scl_lcnt(u32 ic_clk, u32 tLOW, u32 tf, int offset);
 int i2c_dw_set_sda_hold(struct dw_i2c_dev *dev);
@@ -304,19 +302,19 @@ int i2c_dw_acquire_lock(struct dw_i2c_dev *dev);
 void i2c_dw_release_lock(struct dw_i2c_dev *dev);
 int i2c_dw_wait_bus_not_busy(struct dw_i2c_dev *dev);
 int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev);
-void i2c_dw_set_fifo_size(struct dw_i2c_dev *dev);
+int i2c_dw_set_fifo_size(struct dw_i2c_dev *dev);
 u32 i2c_dw_func(struct i2c_adapter *adap);
 void i2c_dw_disable(struct dw_i2c_dev *dev);
 void i2c_dw_disable_int(struct dw_i2c_dev *dev);
 
 static inline void __i2c_dw_enable(struct dw_i2c_dev *dev)
 {
-	dw_writel(dev, 1, DW_IC_ENABLE);
+	regmap_write(dev->map, DW_IC_ENABLE, 1);
 }
 
 static inline void __i2c_dw_disable_nowait(struct dw_i2c_dev *dev)
 {
-	dw_writel(dev, 0, DW_IC_ENABLE);
+	regmap_write(dev->map, DW_IC_ENABLE, 0);
 }
 
 void __i2c_dw_disable(struct dw_i2c_dev *dev);
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index 95eeec53c744..2cba21b945d8 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -18,6 +18,7 @@
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/pm_runtime.h>
+#include <linux/regmap.h>
 #include <linux/reset.h>
 
 #include "i2c-designware-core.h"
@@ -25,11 +26,11 @@
 static void i2c_dw_configure_fifo_master(struct dw_i2c_dev *dev)
 {
 	/* Configure Tx/Rx FIFO threshold levels */
-	dw_writel(dev, dev->tx_fifo_depth / 2, DW_IC_TX_TL);
-	dw_writel(dev, 0, DW_IC_RX_TL);
+	regmap_write(dev->map, DW_IC_TX_TL, dev->tx_fifo_depth / 2);
+	regmap_write(dev->map, DW_IC_RX_TL, 0);
 
 	/* Configure the I2C master */
-	dw_writel(dev, dev->master_cfg, DW_IC_CON);
+	regmap_write(dev->map, DW_IC_CON, dev->master_cfg);
 }
 
 static int i2c_dw_set_timings_master(struct dw_i2c_dev *dev)
@@ -44,8 +45,11 @@ static int i2c_dw_set_timings_master(struct dw_i2c_dev *dev)
 	ret = i2c_dw_acquire_lock(dev);
 	if (ret)
 		return ret;
-	comp_param1 = dw_readl(dev, DW_IC_COMP_PARAM_1);
+
+	ret = regmap_read(dev->map, DW_IC_COMP_PARAM_1, &comp_param1);
 	i2c_dw_release_lock(dev);
+	if (ret)
+		return ret;
 
 	/* Set standard and fast speed dividers for high/low periods */
 	sda_falling_time = t->sda_fall_ns ?: 300; /* ns */
@@ -187,22 +191,22 @@ static int i2c_dw_init_master(struct dw_i2c_dev *dev)
 	__i2c_dw_disable(dev);
 
 	/* Write standard speed timing parameters */
-	dw_writel(dev, dev->ss_hcnt, DW_IC_SS_SCL_HCNT);
-	dw_writel(dev, dev->ss_lcnt, DW_IC_SS_SCL_LCNT);
+	regmap_write(dev->map, DW_IC_SS_SCL_HCNT, dev->ss_hcnt);
+	regmap_write(dev->map, DW_IC_SS_SCL_LCNT, dev->ss_lcnt);
 
 	/* Write fast mode/fast mode plus timing parameters */
-	dw_writel(dev, dev->fs_hcnt, DW_IC_FS_SCL_HCNT);
-	dw_writel(dev, dev->fs_lcnt, DW_IC_FS_SCL_LCNT);
+	regmap_write(dev->map, DW_IC_FS_SCL_HCNT, dev->fs_hcnt);
+	regmap_write(dev->map, DW_IC_FS_SCL_LCNT, dev->fs_lcnt);
 
 	/* Write high speed timing parameters if supported */
 	if (dev->hs_hcnt && dev->hs_lcnt) {
-		dw_writel(dev, dev->hs_hcnt, DW_IC_HS_SCL_HCNT);
-		dw_writel(dev, dev->hs_lcnt, DW_IC_HS_SCL_LCNT);
+		regmap_write(dev->map, DW_IC_HS_SCL_HCNT, dev->hs_hcnt);
+		regmap_write(dev->map, DW_IC_HS_SCL_LCNT, dev->hs_lcnt);
 	}
 
 	/* Write SDA hold time if supported */
 	if (dev->sda_hold_time)
-		dw_writel(dev, dev->sda_hold_time, DW_IC_SDA_HOLD);
+		regmap_write(dev->map, DW_IC_SDA_HOLD, dev->sda_hold_time);
 
 	i2c_dw_configure_fifo_master(dev);
 	i2c_dw_release_lock(dev);
@@ -213,15 +217,15 @@ static int i2c_dw_init_master(struct dw_i2c_dev *dev)
 static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
 {
 	struct i2c_msg *msgs = dev->msgs;
-	u32 ic_con, ic_tar = 0;
+	u32 ic_con = 0, ic_tar = 0;
+	u32 dummy;
 
 	/* Disable the adapter */
 	__i2c_dw_disable(dev);
 
 	/* If the slave address is ten bit address, enable 10BITADDR */
-	ic_con = dw_readl(dev, DW_IC_CON);
 	if (msgs[dev->msg_write_idx].flags & I2C_M_TEN) {
-		ic_con |= DW_IC_CON_10BITADDR_MASTER;
+		ic_con = DW_IC_CON_10BITADDR_MASTER;
 		/*
 		 * If I2C_DYNAMIC_TAR_UPDATE is set, the 10-bit addressing
 		 * mode has to be enabled via bit 12 of IC_TAR register.
@@ -229,17 +233,17 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
 		 * detected from registers.
 		 */
 		ic_tar = DW_IC_TAR_10BITADDR_MASTER;
-	} else {
-		ic_con &= ~DW_IC_CON_10BITADDR_MASTER;
 	}
 
-	dw_writel(dev, ic_con, DW_IC_CON);
+	regmap_update_bits(dev->map, DW_IC_CON, DW_IC_CON_10BITADDR_MASTER,
+			   ic_con);
 
 	/*
 	 * Set the slave (target) address and enable 10-bit addressing mode
 	 * if applicable.
 	 */
-	dw_writel(dev, msgs[dev->msg_write_idx].addr | ic_tar, DW_IC_TAR);
+	regmap_write(dev->map, DW_IC_TAR,
+		     msgs[dev->msg_write_idx].addr | ic_tar);
 
 	/* Enforce disabled interrupts (due to HW issues) */
 	i2c_dw_disable_int(dev);
@@ -248,11 +252,11 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
 	__i2c_dw_enable(dev);
 
 	/* Dummy read to avoid the register getting stuck on Bay Trail */
-	dw_readl(dev, DW_IC_ENABLE_STATUS);
+	regmap_read(dev->map, DW_IC_ENABLE_STATUS, &dummy);
 
 	/* Clear and enable interrupts */
-	dw_readl(dev, DW_IC_CLR_INTR);
-	dw_writel(dev, DW_IC_INTR_MASTER_MASK, DW_IC_INTR_MASK);
+	regmap_read(dev->map, DW_IC_CLR_INTR, &dummy);
+	regmap_write(dev->map, DW_IC_INTR_MASK, DW_IC_INTR_MASTER_MASK);
 }
 
 /*
@@ -271,6 +275,7 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 	u32 buf_len = dev->tx_buf_len;
 	u8 *buf = dev->tx_buf;
 	bool need_restart = false;
+	unsigned int flr;
 
 	intr_mask = DW_IC_INTR_MASTER_MASK;
 
@@ -303,8 +308,11 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 				need_restart = true;
 		}
 
-		tx_limit = dev->tx_fifo_depth - dw_readl(dev, DW_IC_TXFLR);
-		rx_limit = dev->rx_fifo_depth - dw_readl(dev, DW_IC_RXFLR);
+		regmap_read(dev->map, DW_IC_TXFLR, &flr);
+		tx_limit = dev->tx_fifo_depth - flr;
+
+		regmap_read(dev->map, DW_IC_RXFLR, &flr);
+		rx_limit = dev->rx_fifo_depth - flr;
 
 		while (buf_len > 0 && tx_limit > 0 && rx_limit > 0) {
 			u32 cmd = 0;
@@ -337,11 +345,14 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 				if (dev->rx_outstanding >= dev->rx_fifo_depth)
 					break;
 
-				dw_writel(dev, cmd | 0x100, DW_IC_DATA_CMD);
+				regmap_write(dev->map, DW_IC_DATA_CMD,
+					     cmd | 0x100);
 				rx_limit--;
 				dev->rx_outstanding++;
-			} else
-				dw_writel(dev, cmd | *buf++, DW_IC_DATA_CMD);
+			} else {
+				regmap_write(dev->map, DW_IC_DATA_CMD,
+					     cmd | *buf++);
+			}
 			tx_limit--; buf_len--;
 		}
 
@@ -371,7 +382,7 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 	if (dev->msg_err)
 		intr_mask = 0;
 
-	dw_writel(dev, intr_mask,  DW_IC_INTR_MASK);
+	regmap_write(dev->map,  DW_IC_INTR_MASK, intr_mask);
 }
 
 static u8
@@ -399,7 +410,7 @@ i2c_dw_read(struct dw_i2c_dev *dev)
 	int rx_valid;
 
 	for (; dev->msg_read_idx < dev->msgs_num; dev->msg_read_idx++) {
-		u32 len;
+		u32 len, tmp;
 		u8 *buf;
 
 		if (!(msgs[dev->msg_read_idx].flags & I2C_M_RD))
@@ -413,18 +424,18 @@ i2c_dw_read(struct dw_i2c_dev *dev)
 			buf = dev->rx_buf;
 		}
 
-		rx_valid = dw_readl(dev, DW_IC_RXFLR);
+		regmap_read(dev->map, DW_IC_RXFLR, &rx_valid);
 
 		for (; len > 0 && rx_valid > 0; len--, rx_valid--) {
 			u32 flags = msgs[dev->msg_read_idx].flags;
 
-			*buf = dw_readl(dev, DW_IC_DATA_CMD);
+			regmap_read(dev->map, DW_IC_DATA_CMD, &tmp);
 			/* Ensure length byte is a valid value */
 			if (flags & I2C_M_RECV_LEN &&
-				*buf <= I2C_SMBUS_BLOCK_MAX && *buf > 0) {
-				len = i2c_dw_recv_len(dev, *buf);
+			    tmp <= I2C_SMBUS_BLOCK_MAX && tmp > 0) {
+				len = i2c_dw_recv_len(dev, tmp);
 			}
-			buf++;
+			*buf++ = tmp;
 			dev->rx_outstanding--;
 		}
 
@@ -542,7 +553,7 @@ static const struct i2c_adapter_quirks i2c_dw_quirks = {
 
 static u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev)
 {
-	u32 stat;
+	u32 stat, dummy;
 
 	/*
 	 * The IC_INTR_STAT register just indicates "enabled" interrupts.
@@ -550,47 +561,47 @@ static u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev)
 	 * in the IC_RAW_INTR_STAT register.
 	 *
 	 * That is,
-	 *   stat = dw_readl(IC_INTR_STAT);
+	 *   stat = readl(IC_INTR_STAT);
 	 * equals to,
-	 *   stat = dw_readl(IC_RAW_INTR_STAT) & dw_readl(IC_INTR_MASK);
+	 *   stat = readl(IC_RAW_INTR_STAT) & readl(IC_INTR_MASK);
 	 *
 	 * The raw version might be useful for debugging purposes.
 	 */
-	stat = dw_readl(dev, DW_IC_INTR_STAT);
+	regmap_read(dev->map, DW_IC_INTR_STAT, &stat);
 
 	/*
 	 * Do not use the IC_CLR_INTR register to clear interrupts, or
 	 * you'll miss some interrupts, triggered during the period from
-	 * dw_readl(IC_INTR_STAT) to dw_readl(IC_CLR_INTR).
+	 * readl(IC_INTR_STAT) to readl(IC_CLR_INTR).
 	 *
 	 * Instead, use the separately-prepared IC_CLR_* registers.
 	 */
 	if (stat & DW_IC_INTR_RX_UNDER)
-		dw_readl(dev, DW_IC_CLR_RX_UNDER);
+		regmap_read(dev->map, DW_IC_CLR_RX_UNDER, &dummy);
 	if (stat & DW_IC_INTR_RX_OVER)
-		dw_readl(dev, DW_IC_CLR_RX_OVER);
+		regmap_read(dev->map, DW_IC_CLR_RX_OVER, &dummy);
 	if (stat & DW_IC_INTR_TX_OVER)
-		dw_readl(dev, DW_IC_CLR_TX_OVER);
+		regmap_read(dev->map, DW_IC_CLR_TX_OVER, &dummy);
 	if (stat & DW_IC_INTR_RD_REQ)
-		dw_readl(dev, DW_IC_CLR_RD_REQ);
+		regmap_read(dev->map, DW_IC_CLR_RD_REQ, &dummy);
 	if (stat & DW_IC_INTR_TX_ABRT) {
 		/*
 		 * The IC_TX_ABRT_SOURCE register is cleared whenever
 		 * the IC_CLR_TX_ABRT is read.  Preserve it beforehand.
 		 */
-		dev->abort_source = dw_readl(dev, DW_IC_TX_ABRT_SOURCE);
-		dw_readl(dev, DW_IC_CLR_TX_ABRT);
+		regmap_read(dev->map, DW_IC_TX_ABRT_SOURCE, &dev->abort_source);
+		regmap_read(dev->map, DW_IC_CLR_TX_ABRT, &dummy);
 	}
 	if (stat & DW_IC_INTR_RX_DONE)
-		dw_readl(dev, DW_IC_CLR_RX_DONE);
+		regmap_read(dev->map, DW_IC_CLR_RX_DONE, &dummy);
 	if (stat & DW_IC_INTR_ACTIVITY)
-		dw_readl(dev, DW_IC_CLR_ACTIVITY);
+		regmap_read(dev->map, DW_IC_CLR_ACTIVITY, &dummy);
 	if (stat & DW_IC_INTR_STOP_DET)
-		dw_readl(dev, DW_IC_CLR_STOP_DET);
+		regmap_read(dev->map, DW_IC_CLR_STOP_DET, &dummy);
 	if (stat & DW_IC_INTR_START_DET)
-		dw_readl(dev, DW_IC_CLR_START_DET);
+		regmap_read(dev->map, DW_IC_CLR_START_DET, &dummy);
 	if (stat & DW_IC_INTR_GEN_CALL)
-		dw_readl(dev, DW_IC_CLR_GEN_CALL);
+		regmap_read(dev->map, DW_IC_CLR_GEN_CALL, &dummy);
 
 	return stat;
 }
@@ -612,7 +623,7 @@ static int i2c_dw_irq_handler_master(struct dw_i2c_dev *dev)
 		 * Anytime TX_ABRT is set, the contents of the tx/rx
 		 * buffers are flushed. Make sure to skip them.
 		 */
-		dw_writel(dev, 0, DW_IC_INTR_MASK);
+		regmap_write(dev->map, DW_IC_INTR_MASK, 0);
 		goto tx_aborted;
 	}
 
@@ -633,9 +644,9 @@ static int i2c_dw_irq_handler_master(struct dw_i2c_dev *dev)
 		complete(&dev->cmd_complete);
 	else if (unlikely(dev->flags & ACCESS_INTR_MASK)) {
 		/* Workaround to trigger pending interrupt */
-		stat = dw_readl(dev, DW_IC_INTR_MASK);
+		regmap_read(dev->map, DW_IC_INTR_MASK, &stat);
 		i2c_dw_disable_int(dev);
-		dw_writel(dev, stat, DW_IC_INTR_MASK);
+		regmap_write(dev->map, DW_IC_INTR_MASK, stat);
 	}
 
 	return 0;
@@ -646,8 +657,8 @@ static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
 	struct dw_i2c_dev *dev = dev_id;
 	u32 stat, enabled;
 
-	enabled = dw_readl(dev, DW_IC_ENABLE);
-	stat = dw_readl(dev, DW_IC_RAW_INTR_STAT);
+	regmap_read(dev->map, DW_IC_ENABLE, &enabled);
+	regmap_read(dev->map, DW_IC_RAW_INTR_STAT, &stat);
 	dev_dbg(dev->dev, "enabled=%#x stat=%#x\n", enabled, stat);
 	if (!enabled || !(stat & ~DW_IC_INTR_ACTIVITY))
 		return IRQ_NONE;
@@ -739,7 +750,7 @@ int i2c_dw_probe_master(struct dw_i2c_dev *dev)
 	dev->disable = i2c_dw_disable;
 	dev->disable_int = i2c_dw_disable_int;
 
-	ret = i2c_dw_set_reg_access(dev);
+	ret = i2c_dw_init_regmap(dev);
 	if (ret)
 		return ret;
 
@@ -747,7 +758,9 @@ int i2c_dw_probe_master(struct dw_i2c_dev *dev)
 	if (ret)
 		return ret;
 
-	i2c_dw_set_fifo_size(dev);
+	ret = i2c_dw_set_fifo_size(dev);
+	if (ret)
+		return ret;
 
 	ret = dev->init(dev);
 	if (ret)
diff --git a/drivers/i2c/busses/i2c-designware-slave.c b/drivers/i2c/busses/i2c-designware-slave.c
index 576e7af4e94b..44974b53a626 100644
--- a/drivers/i2c/busses/i2c-designware-slave.c
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -14,18 +14,19 @@
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/pm_runtime.h>
+#include <linux/regmap.h>
 
 #include "i2c-designware-core.h"
 
 static void i2c_dw_configure_fifo_slave(struct dw_i2c_dev *dev)
 {
 	/* Configure Tx/Rx FIFO threshold levels. */
-	dw_writel(dev, 0, DW_IC_TX_TL);
-	dw_writel(dev, 0, DW_IC_RX_TL);
+	regmap_write(dev->map, DW_IC_TX_TL, 0);
+	regmap_write(dev->map, DW_IC_RX_TL, 0);
 
 	/* Configure the I2C slave. */
-	dw_writel(dev, dev->slave_cfg, DW_IC_CON);
-	dw_writel(dev, DW_IC_INTR_SLAVE_MASK, DW_IC_INTR_MASK);
+	regmap_write(dev->map, DW_IC_CON, dev->slave_cfg);
+	regmap_write(dev->map, DW_IC_INTR_MASK, DW_IC_INTR_SLAVE_MASK);
 }
 
 /**
@@ -49,7 +50,7 @@ static int i2c_dw_init_slave(struct dw_i2c_dev *dev)
 
 	/* Write SDA hold time if supported */
 	if (dev->sda_hold_time)
-		dw_writel(dev, dev->sda_hold_time, DW_IC_SDA_HOLD);
+		regmap_write(dev->map, DW_IC_SDA_HOLD, dev->sda_hold_time);
 
 	i2c_dw_configure_fifo_slave(dev);
 	i2c_dw_release_lock(dev);
@@ -72,7 +73,7 @@ static int i2c_dw_reg_slave(struct i2c_client *slave)
 	 * the address to which the DW_apb_i2c responds.
 	 */
 	__i2c_dw_disable_nowait(dev);
-	dw_writel(dev, slave->addr, DW_IC_SAR);
+	regmap_write(dev->map, DW_IC_SAR, slave->addr);
 	dev->slave = slave;
 
 	__i2c_dw_enable(dev);
@@ -103,7 +104,7 @@ static int i2c_dw_unreg_slave(struct i2c_client *slave)
 
 static u32 i2c_dw_read_clear_intrbits_slave(struct dw_i2c_dev *dev)
 {
-	u32 stat;
+	u32 stat, dummy;
 
 	/*
 	 * The IC_INTR_STAT register just indicates "enabled" interrupts.
@@ -111,39 +112,39 @@ static u32 i2c_dw_read_clear_intrbits_slave(struct dw_i2c_dev *dev)
 	 * in the IC_RAW_INTR_STAT register.
 	 *
 	 * That is,
-	 *   stat = dw_readl(IC_INTR_STAT);
+	 *   stat = readl(IC_INTR_STAT);
 	 * equals to,
-	 *   stat = dw_readl(IC_RAW_INTR_STAT) & dw_readl(IC_INTR_MASK);
+	 *   stat = readl(IC_RAW_INTR_STAT) & readl(IC_INTR_MASK);
 	 *
 	 * The raw version might be useful for debugging purposes.
 	 */
-	stat = dw_readl(dev, DW_IC_INTR_STAT);
+	regmap_read(dev->map, DW_IC_INTR_STAT, &stat);
 
 	/*
 	 * Do not use the IC_CLR_INTR register to clear interrupts, or
 	 * you'll miss some interrupts, triggered during the period from
-	 * dw_readl(IC_INTR_STAT) to dw_readl(IC_CLR_INTR).
+	 * readl(IC_INTR_STAT) to readl(IC_CLR_INTR).
 	 *
 	 * Instead, use the separately-prepared IC_CLR_* registers.
 	 */
 	if (stat & DW_IC_INTR_TX_ABRT)
-		dw_readl(dev, DW_IC_CLR_TX_ABRT);
+		regmap_read(dev->map, DW_IC_CLR_TX_ABRT, &dummy);
 	if (stat & DW_IC_INTR_RX_UNDER)
-		dw_readl(dev, DW_IC_CLR_RX_UNDER);
+		regmap_read(dev->map, DW_IC_CLR_RX_UNDER, &dummy);
 	if (stat & DW_IC_INTR_RX_OVER)
-		dw_readl(dev, DW_IC_CLR_RX_OVER);
+		regmap_read(dev->map, DW_IC_CLR_RX_OVER, &dummy);
 	if (stat & DW_IC_INTR_TX_OVER)
-		dw_readl(dev, DW_IC_CLR_TX_OVER);
+		regmap_read(dev->map, DW_IC_CLR_TX_OVER, &dummy);
 	if (stat & DW_IC_INTR_RX_DONE)
-		dw_readl(dev, DW_IC_CLR_RX_DONE);
+		regmap_read(dev->map, DW_IC_CLR_RX_DONE, &dummy);
 	if (stat & DW_IC_INTR_ACTIVITY)
-		dw_readl(dev, DW_IC_CLR_ACTIVITY);
+		regmap_read(dev->map, DW_IC_CLR_ACTIVITY, &dummy);
 	if (stat & DW_IC_INTR_STOP_DET)
-		dw_readl(dev, DW_IC_CLR_STOP_DET);
+		regmap_read(dev->map, DW_IC_CLR_STOP_DET, &dummy);
 	if (stat & DW_IC_INTR_START_DET)
-		dw_readl(dev, DW_IC_CLR_START_DET);
+		regmap_read(dev->map, DW_IC_CLR_START_DET, &dummy);
 	if (stat & DW_IC_INTR_GEN_CALL)
-		dw_readl(dev, DW_IC_CLR_GEN_CALL);
+		regmap_read(dev->map, DW_IC_CLR_GEN_CALL, &dummy);
 
 	return stat;
 }
@@ -155,14 +156,14 @@ static u32 i2c_dw_read_clear_intrbits_slave(struct dw_i2c_dev *dev)
 
 static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
 {
-	u32 raw_stat, stat, enabled;
-	u8 val, slave_activity;
+	u32 raw_stat, stat, enabled, tmp;
+	u8 val = 0, slave_activity;
 
-	stat = dw_readl(dev, DW_IC_INTR_STAT);
-	enabled = dw_readl(dev, DW_IC_ENABLE);
-	raw_stat = dw_readl(dev, DW_IC_RAW_INTR_STAT);
-	slave_activity = ((dw_readl(dev, DW_IC_STATUS) &
-		DW_IC_STATUS_SLAVE_ACTIVITY) >> 6);
+	regmap_read(dev->map, DW_IC_INTR_STAT, &stat);
+	regmap_read(dev->map, DW_IC_ENABLE, &enabled);
+	regmap_read(dev->map, DW_IC_RAW_INTR_STAT, &raw_stat);
+	regmap_read(dev->map, DW_IC_STATUS, &tmp);
+	slave_activity = ((tmp & DW_IC_STATUS_SLAVE_ACTIVITY) >> 6);
 
 	if (!enabled || !(raw_stat & ~DW_IC_INTR_ACTIVITY) || !dev->slave)
 		return 0;
@@ -177,7 +178,8 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
 	if (stat & DW_IC_INTR_RD_REQ) {
 		if (slave_activity) {
 			if (stat & DW_IC_INTR_RX_FULL) {
-				val = dw_readl(dev, DW_IC_DATA_CMD);
+				regmap_read(dev->map, DW_IC_DATA_CMD, &tmp);
+				val = tmp;
 
 				if (!i2c_slave_event(dev->slave,
 						     I2C_SLAVE_WRITE_RECEIVED,
@@ -185,24 +187,24 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
 					dev_vdbg(dev->dev, "Byte %X acked!",
 						 val);
 				}
-				dw_readl(dev, DW_IC_CLR_RD_REQ);
+				regmap_read(dev->map, DW_IC_CLR_RD_REQ, &tmp);
 				stat = i2c_dw_read_clear_intrbits_slave(dev);
 			} else {
-				dw_readl(dev, DW_IC_CLR_RD_REQ);
-				dw_readl(dev, DW_IC_CLR_RX_UNDER);
+				regmap_read(dev->map, DW_IC_CLR_RD_REQ, &tmp);
+				regmap_read(dev->map, DW_IC_CLR_RX_UNDER, &tmp);
 				stat = i2c_dw_read_clear_intrbits_slave(dev);
 			}
 			if (!i2c_slave_event(dev->slave,
 					     I2C_SLAVE_READ_REQUESTED,
 					     &val))
-				dw_writel(dev, val, DW_IC_DATA_CMD);
+				regmap_write(dev->map, DW_IC_DATA_CMD, val);
 		}
 	}
 
 	if (stat & DW_IC_INTR_RX_DONE) {
 		if (!i2c_slave_event(dev->slave, I2C_SLAVE_READ_PROCESSED,
 				     &val))
-			dw_readl(dev, DW_IC_CLR_RX_DONE);
+			regmap_read(dev->map, DW_IC_CLR_RX_DONE, &tmp);
 
 		i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
 		stat = i2c_dw_read_clear_intrbits_slave(dev);
@@ -210,7 +212,8 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
 	}
 
 	if (stat & DW_IC_INTR_RX_FULL) {
-		val = dw_readl(dev, DW_IC_DATA_CMD);
+		regmap_read(dev->map, DW_IC_DATA_CMD, &tmp);
+		val = tmp;
 		if (!i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_RECEIVED,
 				     &val))
 			dev_vdbg(dev->dev, "Byte %X acked!", val);
@@ -263,7 +266,7 @@ int i2c_dw_probe_slave(struct dw_i2c_dev *dev)
 	dev->disable = i2c_dw_disable;
 	dev->disable_int = i2c_dw_disable_int;
 
-	ret = i2c_dw_set_reg_access(dev);
+	ret = i2c_dw_init_regmap(dev);
 	if (ret)
 		return ret;
 
@@ -271,7 +274,9 @@ int i2c_dw_probe_slave(struct dw_i2c_dev *dev)
 	if (ret)
 		return ret;
 
-	i2c_dw_set_fifo_size(dev);
+	ret = i2c_dw_set_fifo_size(dev);
+	if (ret)
+		return ret;
 
 	ret = dev->init(dev);
 	if (ret)
-- 
2.26.2


^ permalink raw reply related

* [PATCH v5 09/11] i2c: designware: Retrieve quirk flags as early as possible
From: Serge Semin @ 2020-05-27 15:30 UTC (permalink / raw)
  To: Jarkko Nikula, Wolfram Sang, Andy Shevchenko, Mika Westerberg
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Thomas Bogendoerfer,
	Rob Herring, linux-mips, devicetree, linux-i2c, linux-kernel
In-Reply-To: <20200527153046.6172-1-Sergey.Semin@baikalelectronics.ru>

Some platforms might need to activate the driver quirks at a very early
probe stage. For instance, Baikal-T1 System I2C doesn't need to map the
registers space as ones belong to the system controller. Instead it will
request the syscon regmap from the parental DT node. In order to be able
to do so let's retrieve the model flags right after the DW I2C private
data is created. While at it replace the or-assignment with just
assignment operator since or-ing is redundant at this stage.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org

---

Changelog v3:
- This is a new patch, which has been created due to declining the
  glue-layer approach.

Changelog v5:
- Replace or-assignment with just assignment operator.
---
 drivers/i2c/busses/i2c-designware-platdrv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index ca057aa9eac4..38657d821c72 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -124,6 +124,8 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 	if (!dev)
 		return -ENOMEM;
 
+	dev->flags = (uintptr_t)device_get_match_data(&pdev->dev);
+
 	dev->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(dev->base))
 		return PTR_ERR(dev->base);
@@ -146,8 +148,6 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 
 	i2c_dw_acpi_adjust_bus_speed(&pdev->dev);
 
-	dev->flags |= (uintptr_t)device_get_match_data(&pdev->dev);
-
 	if (pdev->dev.of_node)
 		dw_i2c_of_configure(pdev);
 
-- 
2.26.2


^ permalink raw reply related

* [PATCH v5 01/11] dt-bindings: i2c: Convert DW I2C binding to DT schema
From: Serge Semin @ 2020-05-27 15:30 UTC (permalink / raw)
  To: Jarkko Nikula, Wolfram Sang, Rob Herring
  Cc: Serge Semin, Serge Semin, Rob Herring, Alexey Malahov,
	Thomas Bogendoerfer, Andy Shevchenko, Mika Westerberg, linux-mips,
	linux-i2c, devicetree, linux-kernel
In-Reply-To: <20200527153046.6172-1-Sergey.Semin@baikalelectronics.ru>

Modern device tree bindings are supposed to be created as YAML-files
in accordance with dt-schema. This commit replaces Synopsys DW I2C
legacy bare text bindings with YAML file. As before the bindings file
states that the corresponding dts node is supposed to be compatible
either with generic DW I2C controller or with Microsemi Ocelot SoC I2C
one, to have registers, interrupts and clocks properties. In addition
the node may have clock-frequency, i2c-sda-hold-time-ns,
i2c-scl-falling-time-ns and i2c-sda-falling-time-ns optional properties.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Reviewed-by: Rob Herring <robh@kernel.org>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: linux-mips@vger.kernel.org

---

Changelog v2:
- Make sure that "mscc,ocelot-i2c" compatible node may have up to two
  registers space defined, while normal DW I2C controller will have only
  one registers space.
- Add "mscc,ocelot-i2c" example to test the previous fix.
- Declare "unevaluatedProperties" property instead of
  "additionalProperties" one.
- Due to the previous fix we can now discard the dummy boolean properties
  definitions, since the proper type evaluation will be performed by the
  generic i2c-controller.yaml schema.

Changelog v3:
- Discard $ref from the "-ns" suffixed properties since they've got the
  uint32-array type by default applied in the common schema. Set "maxItems: 1"
  there instead to make sure the property will have a single value specified.
---
 .../bindings/i2c/i2c-designware.txt           |  73 ---------
 .../bindings/i2c/snps,designware-i2c.yaml     | 154 ++++++++++++++++++
 2 files changed, 154 insertions(+), 73 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-designware.txt
 create mode 100644 Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml

diff --git a/Documentation/devicetree/bindings/i2c/i2c-designware.txt b/Documentation/devicetree/bindings/i2c/i2c-designware.txt
deleted file mode 100644
index 08be4d3846e5..000000000000
--- a/Documentation/devicetree/bindings/i2c/i2c-designware.txt
+++ /dev/null
@@ -1,73 +0,0 @@
-* Synopsys DesignWare I2C
-
-Required properties :
-
- - compatible : should be "snps,designware-i2c"
-                or "mscc,ocelot-i2c" with "snps,designware-i2c" for fallback
- - reg : Offset and length of the register set for the device
- - interrupts : <IRQ> where IRQ is the interrupt number.
- - clocks : phandles for the clocks, see the description of clock-names below.
-   The phandle for the "ic_clk" clock is required. The phandle for the "pclk"
-   clock is optional. If a single clock is specified but no clock-name, it is
-   the "ic_clk" clock. If both clocks are listed, the "ic_clk" must be first.
-
-Recommended properties :
-
- - clock-frequency : desired I2C bus clock frequency in Hz.
-
-Optional properties :
-
- - clock-names : Contains the names of the clocks:
-    "ic_clk", for the core clock used to generate the external I2C clock.
-    "pclk", the interface clock, required for register access.
-
- - reg : for "mscc,ocelot-i2c", a second register set to configure the SDA hold
-   time, named ICPU_CFG:TWI_DELAY in the datasheet.
-
- - i2c-sda-hold-time-ns : should contain the SDA hold time in nanoseconds.
-   This option is only supported in hardware blocks version 1.11a or newer and
-   on Microsemi SoCs ("mscc,ocelot-i2c" compatible).
-
- - i2c-scl-falling-time-ns : should contain the SCL falling time in nanoseconds.
-   This value which is by default 300ns is used to compute the tLOW period.
-
- - i2c-sda-falling-time-ns : should contain the SDA falling time in nanoseconds.
-   This value which is by default 300ns is used to compute the tHIGH period.
-
-Examples :
-
-	i2c@f0000 {
-		#address-cells = <1>;
-		#size-cells = <0>;
-		compatible = "snps,designware-i2c";
-		reg = <0xf0000 0x1000>;
-		interrupts = <11>;
-		clock-frequency = <400000>;
-	};
-
-	i2c@1120000 {
-		#address-cells = <1>;
-		#size-cells = <0>;
-		compatible = "snps,designware-i2c";
-		reg = <0x1120000 0x1000>;
-		interrupt-parent = <&ictl>;
-		interrupts = <12 1>;
-		clock-frequency = <400000>;
-		i2c-sda-hold-time-ns = <300>;
-		i2c-sda-falling-time-ns = <300>;
-		i2c-scl-falling-time-ns = <300>;
-	};
-
-	i2c@1120000 {
-		#address-cells = <1>;
-		#size-cells = <0>;
-		reg = <0x2000 0x100>;
-		clock-frequency = <400000>;
-		clocks = <&i2cclk>;
-		interrupts = <0>;
-
-		eeprom@64 {
-			compatible = "linux,slave-24c02";
-			reg = <0x40000064>;
-		};
-	};
diff --git a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
new file mode 100644
index 000000000000..4bd430b2b41d
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
@@ -0,0 +1,154 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i2c/snps,designware-i2c.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Synopsys DesignWare APB I2C Controller
+
+maintainers:
+  - Jarkko Nikula <jarkko.nikula@linux.intel.com>
+
+allOf:
+  - $ref: /schemas/i2c/i2c-controller.yaml#
+  - if:
+      properties:
+        compatible:
+          not:
+            contains:
+              const: mscc,ocelot-i2c
+    then:
+      properties:
+        reg:
+          maxItems: 1
+
+properties:
+  compatible:
+    oneOf:
+      - description: Generic Synopsys DesignWare I2C controller
+        const: snps,designware-i2c
+      - description: Microsemi Ocelot SoCs I2C controller
+        items:
+          - const: mscc,ocelot-i2c
+          - const: snps,designware-i2c
+
+  reg:
+    minItems: 1
+    items:
+      - description: DW APB I2C controller memory mapped registers
+      - description: |
+          ICPU_CFG:TWI_DELAY registers to setup the SDA hold time.
+          This registers are specific to the Ocelot I2C-controller.
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    minItems: 1
+    items:
+      - description: I2C controller reference clock source
+      - description: APB interface clock source
+
+  clock-names:
+    minItems: 1
+    items:
+      - const: ref
+      - const: pclk
+
+  resets:
+    maxItems: 1
+
+  clock-frequency:
+    description: Desired I2C bus clock frequency in Hz
+    enum: [100000, 400000, 1000000, 3400000]
+    default: 400000
+
+  i2c-sda-hold-time-ns:
+    maxItems: 1
+    description: |
+      The property should contain the SDA hold time in nanoseconds. This option
+      is only supported in hardware blocks version 1.11a or newer or on
+      Microsemi SoCs.
+
+  i2c-scl-falling-time-ns:
+    maxItems: 1
+    description: |
+      The property should contain the SCL falling time in nanoseconds.
+      This value is used to compute the tLOW period.
+    default: 300
+
+  i2c-sda-falling-time-ns:
+    maxItems: 1
+    description: |
+      The property should contain the SDA falling time in nanoseconds.
+      This value is used to compute the tHIGH period.
+    default: 300
+
+  dmas:
+    items:
+      - description: TX DMA Channel
+      - description: RX DMA Channel
+
+  dma-names:
+    items:
+      - const: tx
+      - const: rx
+
+unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+  - "#address-cells"
+  - "#size-cells"
+  - interrupts
+
+examples:
+  - |
+    i2c@f0000 {
+      compatible = "snps,designware-i2c";
+      reg = <0xf0000 0x1000>;
+      #address-cells = <1>;
+      #size-cells = <0>;
+      interrupts = <11>;
+      clock-frequency = <400000>;
+    };
+  - |
+    i2c@1120000 {
+      compatible = "snps,designware-i2c";
+      reg = <0x1120000 0x1000>;
+      #address-cells = <1>;
+      #size-cells = <0>;
+      interrupts = <12 1>;
+      clock-frequency = <400000>;
+      i2c-sda-hold-time-ns = <300>;
+      i2c-sda-falling-time-ns = <300>;
+      i2c-scl-falling-time-ns = <300>;
+    };
+  - |
+    i2c@2000 {
+      compatible = "snps,designware-i2c";
+      reg = <0x2000 0x100>;
+      #address-cells = <1>;
+      #size-cells = <0>;
+      clock-frequency = <400000>;
+      clocks = <&i2cclk>;
+      interrupts = <0>;
+
+      eeprom@64 {
+        compatible = "linux,slave-24c02";
+        reg = <0x40000064>;
+      };
+    };
+  - |
+    i2c@100400 {
+      compatible = "mscc,ocelot-i2c", "snps,designware-i2c";
+      reg = <0x100400 0x100>, <0x198 0x8>;
+      pinctrl-0 = <&i2c_pins>;
+      pinctrl-names = "default";
+      #address-cells = <1>;
+      #size-cells = <0>;
+      interrupts = <8>;
+      clocks = <&ahb_clk>;
+    };
+...
-- 
2.26.2


^ permalink raw reply related

* [PATCH v5 04/11] i2c: designware: Use `-y` to build multi-object modules
From: Serge Semin @ 2020-05-27 15:30 UTC (permalink / raw)
  To: Jarkko Nikula, Wolfram Sang
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Thomas Bogendoerfer,
	Andy Shevchenko, Mika Westerberg, Rob Herring, linux-mips,
	devicetree, linux-i2c, linux-kernel
In-Reply-To: <20200527153046.6172-1-Sergey.Semin@baikalelectronics.ru>

Since commit 4f8272802739 ("Documentation: update kbuild loadable modules
goals & examples") `-objs` is fitted for building host programs, lets
change DW I2C core, platform and PCI driver kbuild directives to using
`-y`, which more straightforward for device drivers. By doing so we can
discard the ifeq construction in favor to the more natural and less bulky
`<module>-$(CONFIG_X) += x.o`

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
 drivers/i2c/busses/Makefile | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index a33aa107a05d..c6813d7b2780 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -48,17 +48,15 @@ obj-$(CONFIG_I2C_CADENCE)	+= i2c-cadence.o
 obj-$(CONFIG_I2C_CBUS_GPIO)	+= i2c-cbus-gpio.o
 obj-$(CONFIG_I2C_CPM)		+= i2c-cpm.o
 obj-$(CONFIG_I2C_DAVINCI)	+= i2c-davinci.o
-obj-$(CONFIG_I2C_DESIGNWARE_CORE)	+= i2c-designware-core.o
-i2c-designware-core-objs := i2c-designware-common.o
-i2c-designware-core-objs += i2c-designware-master.o
-ifeq ($(CONFIG_I2C_DESIGNWARE_SLAVE),y)
-i2c-designware-core-objs += i2c-designware-slave.o
-endif
-obj-$(CONFIG_I2C_DESIGNWARE_PLATFORM)	+= i2c-designware-platform.o
-i2c-designware-platform-objs := i2c-designware-platdrv.o
+obj-$(CONFIG_I2C_DESIGNWARE_CORE)			+= i2c-designware-core.o
+i2c-designware-core-y					:= i2c-designware-common.o
+i2c-designware-core-y					+= i2c-designware-master.o
+i2c-designware-core-$(CONFIG_I2C_DESIGNWARE_SLAVE) 	+= i2c-designware-slave.o
+obj-$(CONFIG_I2C_DESIGNWARE_PLATFORM)			+= i2c-designware-platform.o
+i2c-designware-platform-y 				:= i2c-designware-platdrv.o
 i2c-designware-platform-$(CONFIG_I2C_DESIGNWARE_BAYTRAIL) += i2c-designware-baytrail.o
-obj-$(CONFIG_I2C_DESIGNWARE_PCI)	+= i2c-designware-pci.o
-i2c-designware-pci-objs := i2c-designware-pcidrv.o
+obj-$(CONFIG_I2C_DESIGNWARE_PCI)			+= i2c-designware-pci.o
+i2c-designware-pci-y					:= i2c-designware-pcidrv.o
 obj-$(CONFIG_I2C_DIGICOLOR)	+= i2c-digicolor.o
 obj-$(CONFIG_I2C_EFM32)		+= i2c-efm32.o
 obj-$(CONFIG_I2C_EG20T)		+= i2c-eg20t.o
-- 
2.26.2


^ permalink raw reply related

* Re: [PATCH v6 2/5] dt-bindings: iio: magnetometer: ak8975: convert format to yaml, add maintainer
From: Jonathan Albrieux @ 2020-05-27 15:29 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-kernel, ~postmarketos/upstreaming, Andy Shevchenko,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Hartmut Knaack, Jonathan Cameron, Lars-Peter Clausen,
	Linus Walleij, open list:IIO SUBSYSTEM AND DRIVERS,
	Peter Meerwald-Stadler, Jonathan Cameron
In-Reply-To: <20200526222847.GA492828@bogus>

On Tue, May 26, 2020 at 04:28:47PM -0600, Rob Herring wrote:
> On Mon, May 25, 2020 at 05:10:36PM +0200, Jonathan Albrieux wrote:
> > Converts documentation from txt format to yaml.
> 
> I would have converted to yaml and do any re-formatting/wording, then 
> added 'interrupts', but this is fine.
>

Thank you, I'll keep that in mind for future works like this one
 
> 
> > Signed-off-by: Jonathan Albrieux <jonathan.albrieux@gmail.com>
> > ---
> >  .../bindings/iio/magnetometer/ak8975.txt      | 37 ---------
> >  .../iio/magnetometer/asahi-kasei,ak8975.yaml  | 78 +++++++++++++++++++
> >  2 files changed, 78 insertions(+), 37 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/iio/magnetometer/ak8975.txt
> >  create mode 100644 Documentation/devicetree/bindings/iio/magnetometer/asahi-kasei,ak8975.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/magnetometer/ak8975.txt b/Documentation/devicetree/bindings/iio/magnetometer/ak8975.txt
> > deleted file mode 100644
> > index 0576b9df0bf2..000000000000
> > --- a/Documentation/devicetree/bindings/iio/magnetometer/ak8975.txt
> > +++ /dev/null
> > @@ -1,37 +0,0 @@
> > -* AsahiKASEI AK8975 magnetometer sensor
> > -
> > -Required properties:
> > -
> > -  - compatible : should be "asahi-kasei,ak8975".
> > -  - reg : the I2C address of the magnetometer.
> > -
> > -Optional properties:
> > -
> > -  - gpios : AK8975 has a "Data ready" pin (DRDY) which informs that data
> > -      is ready to be read and is possible to listen on it. If used,
> > -      this should be active high. Prefer interrupt over this.
> > -
> > -  - interrupts : interrupt for DRDY pin. Triggered on rising edge.
> > -
> > -  - vdd-supply: an optional regulator that needs to be on to provide VDD.
> > -
> > -  - mount-matrix: an optional 3x3 mounting rotation matrix.
> > -
> > -Example:
> > -
> > -ak8975@c {
> > -        compatible = "asahi-kasei,ak8975";
> > -        reg = <0x0c>;
> > -        interrupt-parent = <&gpio6>;
> > -        interrupts = <15 IRQ_TYPE_EDGE_RISING>;
> > -        vdd-supply = <&ldo_3v3_gnss>;
> > -        mount-matrix = "-0.984807753012208",  /* x0 */
> > -                       "0",                   /* y0 */
> > -                       "-0.173648177666930",  /* z0 */
> > -                       "0",                   /* x1 */
> > -                       "-1",                  /* y1 */
> > -                       "0",                   /* z1 */
> > -                       "-0.173648177666930",  /* x2 */
> > -                       "0",                   /* y2 */
> > -                       "0.984807753012208";   /* z2 */
> > -};
> > diff --git a/Documentation/devicetree/bindings/iio/magnetometer/asahi-kasei,ak8975.yaml b/Documentation/devicetree/bindings/iio/magnetometer/asahi-kasei,ak8975.yaml
> > new file mode 100644
> > index 000000000000..a603659d5fa5
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/magnetometer/asahi-kasei,ak8975.yaml
> > @@ -0,0 +1,78 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/magnetometer/asahi-kasei,ak8975.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: AsahiKASEI AK8975 magnetometer sensor
> > +
> > +maintainers:
> > +  - Jonathan Albrieux <jonathan.albrieux@gmail.com>
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - const: asahi-kasei,ak8975
> > +      - const: asahi-kasei,ak8963
> > +      - const: asahi-kasei,ak09911
> > +      - const: asahi-kasei,ak09912
> 
> These 4 can be an enum.
> 
> > +      - const: ak8975
> > +        deprecated: true
> > +      - const: ak8963
> > +        deprecated: true
> > +      - const: ak09911
> > +        deprecated: true
> > +      - const: ak09912
> > +        deprecated: true
> 
> And these 4 can be an enum+deprecated.
> 
> > +
> > +  reg:
> > +    maxItems: 1
> > +    description: the I2C address of the magnetometer.
> 
> Don't need a description.
> 
> > +
> > +  gpios:
> > +    description: |
> > +      AK8975 has a "Data ready" pin (DRDY) which informs that data
> > +      is ready to be read and is possible to listen on it. If used,
> > +      this should be active high. Prefer interrupt over this.
> 
> Need to define how many GPIOs (maxItems: 1).
> 

Ok, I'll edit those fields as soon as possible, thank you,

Best regards,
Jonathan Albrieux

> > +
> > +  interrupts:
> > +    maxItems: 1
> > +    description: interrupt for DRDY pin. Triggered on rising edge.
> > +
> > +  vdd-supply:
> > +    maxItems: 1
> > +    description: |
> > +      an optional regulator that needs to be on to provide VDD power to
> > +      the sensor.
> > +
> > +  mount-matrix:
> > +    description: an optional 3x3 mounting rotation matrix.
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        magnetometer@c {
> > +            compatible = "asahi-kasei,ak8975";
> > +            reg = <0x0c>;
> > +            interrupt-parent = <&gpio6>;
> > +            interrupts = <15 IRQ_TYPE_EDGE_RISING>;
> > +            vdd-supply = <&ldo_3v3_gnss>;
> > +            mount-matrix = "-0.984807753012208",  /* x0 */
> > +                           "0",                   /* y0 */
> > +                           "-0.173648177666930",  /* z0 */
> > +                           "0",                   /* x1 */
> > +                           "-1",                  /* y1 */
> > +                           "0",                   /* z1 */
> > +                           "-0.173648177666930",  /* x2 */
> > +                           "0",                   /* y2 */
> > +                           "0.984807753012208";   /* z2 */
> > +        };
> > +    };
> > -- 
> > 2.17.1
> > 

^ permalink raw reply

* Re: [V9, 1/2] media: dt-bindings: media: i2c: Document OV02A10 bindings
From: Rob Herring @ 2020-05-27 15:27 UTC (permalink / raw)
  To: Dongchun Zhu
  Cc: Linus Walleij, Bartosz Golaszewski, Mauro Carvalho Chehab,
	Andy Shevchenko, Mark Rutland, Sakari Ailus, Nicolas Boichat,
	Tomasz Figa, Matthias Brugger, Cao Bing Bu, srv_heupstream,
	moderated list:ARM/Mediatek SoC support,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, Sj Huang,
	Linux Media Mailing List, devicetree, Louis Kuo,
	Shengnan Wang (王圣男)
In-Reply-To: <1590569355.8804.448.camel@mhfsdcap03>

On Wed, May 27, 2020 at 2:51 AM Dongchun Zhu <dongchun.zhu@mediatek.com> wrote:
>
> Hi Rob,
>
> Thanks for the review. Please see my replies below.
>
> On Tue, 2020-05-26 at 12:28 -0600, Rob Herring wrote:
> > On Sat, May 23, 2020 at 04:41:02PM +0800, Dongchun Zhu wrote:
> > > Add DT bindings documentation for Omnivision OV02A10 image sensor.
> > >
> > > Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> > > ---
> > >  .../bindings/media/i2c/ovti,ov02a10.yaml           | 172 +++++++++++++++++++++
> > >  MAINTAINERS                                        |   7 +
> > >  2 files changed, 179 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov02a10.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov02a10.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov02a10.yaml
> > > new file mode 100644
> > > index 0000000..56f31b5
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov02a10.yaml
> > > @@ -0,0 +1,172 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > +# Copyright (c) 2020 MediaTek Inc.
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/media/i2c/ovti,ov02a10.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Omnivision OV02A10 CMOS Sensor Device Tree Bindings
> > > +
> > > +maintainers:
> > > +  - Dongchun Zhu <dongchun.zhu@mediatek.com>
> > > +
> > > +description: |-
> > > +  The Omnivision OV02A10 is a low-cost, high performance, 1/5-inch, 2 megapixel
> > > +  image sensor, which is the latest production derived from Omnivision's CMOS
> > > +  image sensor technology. Ihis chip supports high frame rate speeds up to 30fps
> > > +  @ 1600x1200 (UXGA) resolution transferred over a 1-lane MIPI interface. The
> > > +  sensor output is available via CSI-2 serial data output.
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: ovti,ov02a10
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  clocks:
> > > +    items:
> > > +      - description: top mux camtg clock
> > > +      - description: divider clock
> > > +
> > > +  clock-names:
> > > +    items:
> > > +      - const: eclk
> > > +      - const: freq_mux
> > > +
> > > +  clock-frequency:
> > > +    description:
> > > +      Frequency of the eclk clock in Hertz.
> > > +
>
> Rob, shall we use 'maxItems: 1' to constrain property: clock-frequency?

No, because it is a scalar, not an array.

> Or could we adopt 'clock-frequency: true' directly here?

As-is is fine.

> > > +  dovdd-supply:
> > > +    description:
> > > +      Definition of the regulator used as Digital I/O voltage supply.
> > > +
>
> Shall we add 'maxItems: 1' here?

No, supplies are always singular.


>
> > > +  avdd-supply:
> > > +    description:
> > > +      Definition of the regulator used as Analog voltage supply.
> > > +
>
> Ditto.
>
> > > +  dvdd-supply:
> > > +    description:
> > > +      Definition of the regulator used as Digital core voltage supply.
> > > +
>
> Ditto.
>
> > > +  powerdown-gpios:
> > > +    description:
> > > +      Must be the device tree identifier of the GPIO connected to the
> > > +      PD_PAD pin. This pin is used to place the OV02A10 into Standby mode
> > > +      or Shutdown mode. As the line is active low, it should be
> > > +      marked GPIO_ACTIVE_LOW.
> >
> > Need to define how many GPIOs ('maxItems: 1')
> >
>
> It would be fixed like this in next release.
> powerdown-gpios:
>   maxItems: 1
>   description:
>     Must be the device tree identifier of the GPIO connected to the
>     PD_PAD pin. This pin is used to place the OV02A10 into Standby mode
>     or Shutdown mode. As the line is active low, it should be
>     marked GPIO_ACTIVE_LOW.
>
> > > +
> > > +  reset-gpios:
> > > +    description:
> > > +      Must be the device tree identifier of the GPIO connected to the
> > > +      RST_PD pin. If specified, it will be asserted during driver probe.
> > > +      As the line is active high, it should be marked GPIO_ACTIVE_HIGH.
> >
> > Here too.
> >
>
> Similar as 'powerdown-gpios'.
> Fixed in next release.
>
> > > +
> > > +  rotation:
> > > +    description:
> > > +      Definition of the sensor's placement.
> > > +    allOf:
> > > +      - $ref: "/schemas/types.yaml#/definitions/uint32"
> > > +      - enum:
> > > +          - 0    # Sensor Mounted Upright
> > > +          - 180  # Sensor Mounted Upside Down
> > > +        default: 0
> > > +
> > > +  ovti,mipi-tx-speed:
> > > +    description:
> > > +      Indication of MIPI transmission speed select, which is to control D-PHY
> > > +      timing setting by adjusting MIPI clock voltage to improve the clock
> > > +      driver capability.
> > > +    allOf:
> > > +      - $ref: "/schemas/types.yaml#/definitions/uint32"
> > > +      - enum:
> > > +          - 0    #  20MHz -  30MHz
> > > +          - 1    #  30MHz -  50MHz
> > > +          - 2    #  50MHz -  75MHz
> > > +          - 3    #  75MHz - 100MHz
> > > +          - 4    # 100MHz - 130MHz
> > > +        default: 3
> > > +
> > > +  # See ../video-interfaces.txt for details
> > > +  port:
> > > +    type: object
> > > +    additionalProperties: false
> >
> > Should have a description of what data the port has.
> >
>
> It would be updated as below in next release.
> port:
>   type: object
>   additionalProperties: false
>   description:
>     Input port node, single endpoint describing the CSI-2 transmitter.

Output?

>
> > > +
> > > +    properties:
> > > +      endpoint:
> > > +        type: object
> > > +        additionalProperties: false
> > > +
> > > +        properties:
>
> Actually I wonder whether we need to declare 'clock-lanes' here?

Yes, if you are using it.

Rob

^ permalink raw reply

* Re: [PATCH v25 03/16] dt: bindings: lp50xx: Introduce the lp50xx family of RGB drivers
From: Dan Murphy @ 2020-05-27 15:05 UTC (permalink / raw)
  To: Pavel Machek; +Cc: jacek.anaszewski, robh, devicetree, linux-leds, linux-kernel
In-Reply-To: <20200527135848.GB5011@amd>

Pavel

On 5/27/20 8:58 AM, Pavel Machek wrote:
> Hi!
>
>
>> +          There can only be one instance of the ti,led-bank
>> +          property for each device node.  This is a required node is the LED
>> +          modules are to be backed.
> I don't understand the second sentence. Pretty sure it is not valid
> english.


If I make these changes is this still viable for 5.8 or would you then 
go into 5.9?

I mean v25 patchset was around for about 3+ weeks and now I have more 
changes.

Dan

>
> Best regards,
> 								Pavel
> 								

^ permalink raw reply

* Re: [PATCH v4 11/11] i2c: designware: Add Baikal-T1 System I2C support
From: Serge Semin @ 2020-05-27 15:04 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Serge Semin, Jarkko Nikula, Wolfram Sang, Mika Westerberg,
	Alexey Malahov, Thomas Bogendoerfer, Rob Herring, linux-mips,
	devicetree, linux-i2c, linux-kernel
In-Reply-To: <20200527140303.GC1634618@smile.fi.intel.com>

On Wed, May 27, 2020 at 05:03:03PM +0300, Andy Shevchenko wrote:
> On Wed, May 27, 2020 at 03:01:11PM +0300, Serge Semin wrote:
> > Baikal-T1 System Controller is equipped with a dedicated I2C Controller
> > which functionality is based on the DW APB I2C IP-core, the only
> > difference in a way it' registers are accessed. There are three access
> > register provided in the System Controller registers map, which indirectly
> > address the normal DW APB I2C registers space. So in order to have the
> > Baikal-T1 System I2C Controller supported by the common DW APB I2C driver
> > we created a dedicated Dw I2C controller model quirk, which retrieves the
> > syscon regmap from the parental dt node and creates a new regmap based on
> > it.
> 
> Yes, you see how cool it is now! FWIW,

Well, solution with glue-layers I liked more than this one. If your were
talking regarding the regmap conversion itself. I admit, it's neater.)

-Sergey

> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: linux-mips@vger.kernel.org
> > Cc: devicetree@vger.kernel.org
> > 
> > ---
> > 
> > Changelog v3:
> > - This is a new patch, which has been created due to declining the
> >   glue-layer approach.
> > 
> > Changelog v4:
> > - Use PTR_ERR_OR_ZERO() helper in the bt1_i2c_request_regs() method.
> > ---
> >  drivers/i2c/busses/Kconfig                  |  3 +-
> >  drivers/i2c/busses/i2c-designware-core.h    |  3 +
> >  drivers/i2c/busses/i2c-designware-platdrv.c | 78 ++++++++++++++++++++-
> >  3 files changed, 81 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> > index 259e2325712a..0cf7aea30138 100644
> > --- a/drivers/i2c/busses/Kconfig
> > +++ b/drivers/i2c/busses/Kconfig
> > @@ -541,8 +541,9 @@ config I2C_DESIGNWARE_SLAVE
> >  
> >  config I2C_DESIGNWARE_PLATFORM
> >  	tristate "Synopsys DesignWare Platform"
> > -	select I2C_DESIGNWARE_CORE
> >  	depends on (ACPI && COMMON_CLK) || !ACPI
> > +	select I2C_DESIGNWARE_CORE
> > +	select MFD_SYSCON if MIPS_BAIKAL_T1
> >  	help
> >  	  If you say yes to this option, support will be included for the
> >  	  Synopsys DesignWare I2C adapter.
> > diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
> > index f5bbe3d6bcf8..556673a1f61b 100644
> > --- a/drivers/i2c/busses/i2c-designware-core.h
> > +++ b/drivers/i2c/busses/i2c-designware-core.h
> > @@ -183,6 +183,7 @@ struct reset_control;
> >   * struct dw_i2c_dev - private i2c-designware data
> >   * @dev: driver model device node
> >   * @map: IO registers map
> > + * @sysmap: System controller registers map
> >   * @base: IO registers pointer
> >   * @ext: Extended IO registers pointer
> >   * @cmd_complete: tx completion indicator
> > @@ -235,6 +236,7 @@ struct reset_control;
> >  struct dw_i2c_dev {
> >  	struct device		*dev;
> >  	struct regmap		*map;
> > +	struct regmap		*sysmap;
> >  	void __iomem		*base;
> >  	void __iomem		*ext;
> >  	struct completion	cmd_complete;
> > @@ -290,6 +292,7 @@ struct dw_i2c_dev {
> >  #define ACCESS_NO_IRQ_SUSPEND	0x00000002
> >  
> >  #define MODEL_MSCC_OCELOT	0x00000100
> > +#define MODEL_BAIKAL_BT1	0x00000200
> >  #define MODEL_MASK		0x00000f00
> >  
> >  int i2c_dw_init_regmap(struct dw_i2c_dev *dev);
> > diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> > index 061c8d506c7c..d9c5337bb22b 100644
> > --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> > +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> > @@ -18,6 +18,7 @@
> >  #include <linux/interrupt.h>
> >  #include <linux/io.h>
> >  #include <linux/kernel.h>
> > +#include <linux/mfd/syscon.h>
> >  #include <linux/module.h>
> >  #include <linux/of.h>
> >  #include <linux/platform_data/i2c-designware.h>
> > @@ -25,6 +26,7 @@
> >  #include <linux/pm.h>
> >  #include <linux/pm_runtime.h>
> >  #include <linux/property.h>
> > +#include <linux/regmap.h>
> >  #include <linux/reset.h>
> >  #include <linux/sched.h>
> >  #include <linux/slab.h>
> > @@ -58,6 +60,63 @@ MODULE_DEVICE_TABLE(acpi, dw_i2c_acpi_match);
> >  #endif
> >  
> >  #ifdef CONFIG_OF
> > +#define BT1_I2C_CTL			0x100
> > +#define BT1_I2C_CTL_ADDR_MASK		GENMASK(7, 0)
> > +#define BT1_I2C_CTL_WR			BIT(8)
> > +#define BT1_I2C_CTL_GO			BIT(31)
> > +#define BT1_I2C_DI			0x104
> > +#define BT1_I2C_DO			0x108
> > +
> > +static int bt1_i2c_read(void *context, unsigned int reg, unsigned int *val)
> > +{
> > +	struct dw_i2c_dev *dev = context;
> > +	int ret;
> > +
> > +	/*
> > +	 * Note these methods shouldn't ever fail because the system controller
> > +	 * registers are memory mapped. We check the return value just in case.
> > +	 */
> > +	ret = regmap_write(dev->sysmap, BT1_I2C_CTL,
> > +			   BT1_I2C_CTL_GO | (reg & BT1_I2C_CTL_ADDR_MASK));
> > +	if (ret)
> > +		return ret;
> > +
> > +	return regmap_read(dev->sysmap, BT1_I2C_DO, val);
> > +}
> > +
> > +static int bt1_i2c_write(void *context, unsigned int reg, unsigned int val)
> > +{
> > +	struct dw_i2c_dev *dev = context;
> > +	int ret;
> > +
> > +	ret = regmap_write(dev->sysmap, BT1_I2C_DI, val);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return regmap_write(dev->sysmap, BT1_I2C_CTL,
> > +		BT1_I2C_CTL_GO | BT1_I2C_CTL_WR | (reg & BT1_I2C_CTL_ADDR_MASK));
> > +}
> > +
> > +static struct regmap_config bt1_i2c_cfg = {
> > +	.reg_bits = 32,
> > +	.val_bits = 32,
> > +	.reg_stride = 4,
> > +	.fast_io = true,
> > +	.reg_read = bt1_i2c_read,
> > +	.reg_write = bt1_i2c_write,
> > +	.max_register = DW_IC_COMP_TYPE
> > +};
> > +
> > +static int bt1_i2c_request_regs(struct dw_i2c_dev *dev)
> > +{
> > +	dev->sysmap = syscon_node_to_regmap(dev->dev->of_node->parent);
> > +	if (IS_ERR(dev->sysmap))
> > +		return PTR_ERR(dev->sysmap);
> > +
> > +	dev->map = devm_regmap_init(dev->dev, NULL, dev, &bt1_i2c_cfg);
> > +	return PTR_ERR_OR_ZERO(dev->map);
> > +}
> > +
> >  #define MSCC_ICPU_CFG_TWI_DELAY		0x0
> >  #define MSCC_ICPU_CFG_TWI_DELAY_ENABLE	BIT(0)
> >  #define MSCC_ICPU_CFG_TWI_SPIKE_FILTER	0x4
> > @@ -90,10 +149,16 @@ static int dw_i2c_of_configure(struct platform_device *pdev)
> >  static const struct of_device_id dw_i2c_of_match[] = {
> >  	{ .compatible = "snps,designware-i2c", },
> >  	{ .compatible = "mscc,ocelot-i2c", .data = (void *)MODEL_MSCC_OCELOT },
> > +	{ .compatible = "baikal,bt1-sys-i2c", .data = (void *)MODEL_BAIKAL_BT1 },
> >  	{},
> >  };
> >  MODULE_DEVICE_TABLE(of, dw_i2c_of_match);
> >  #else
> > +static int bt1_i2c_request_regs(struct dw_i2c_dev *dev)
> > +{
> > +	return -ENODEV;
> > +}
> > +
> >  static inline int dw_i2c_of_configure(struct platform_device *pdev)
> >  {
> >  	return -ENODEV;
> > @@ -111,10 +176,19 @@ static void dw_i2c_plat_pm_cleanup(struct dw_i2c_dev *dev)
> >  static int dw_i2c_plat_request_regs(struct dw_i2c_dev *dev)
> >  {
> >  	struct platform_device *pdev = to_platform_device(dev->dev);
> > +	int ret;
> >  
> > -	dev->base = devm_platform_ioremap_resource(pdev, 0);
> > +	switch (dev->flags & MODEL_MASK) {
> > +	case MODEL_BAIKAL_BT1:
> > +		ret = bt1_i2c_request_regs(dev);
> > +		break;
> > +	default:
> > +		dev->base = devm_platform_ioremap_resource(pdev, 0);
> > +		ret = PTR_ERR_OR_ZERO(dev->base);
> > +		break;
> > +	}
> >  
> > -	return PTR_ERR_OR_ZERO(dev->base);
> > +	return ret;
> >  }
> >  
> >  static int dw_i2c_plat_probe(struct platform_device *pdev)
> > -- 
> > 2.26.2
> > 
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

^ permalink raw reply

* Re: [PATCH v2 09/14] device core: Add ability to handle multiple dma offsets
From: Nicolas Saenz Julienne @ 2020-05-27 15:00 UTC (permalink / raw)
  To: Jim Quinlan, linux-pci, Christoph Hellwig,
	bcm-kernel-feedback-list
  Cc: Rob Herring, Frank Rowand, Greg Kroah-Hartman, Marek Szyprowski,
	Robin Murphy, Alan Stern, Oliver Neukum, Rafael J. Wysocki,
	Andy Shevchenko, Wolfram Sang, Corey Minyard, Srinivas Kandagatla,
	Suzuki K Poulose, Saravana Kannan, Heikki Krogerus, Dan Williams,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE, open list,
	open list:USB SUBSYSTEM, open list:DMA MAPPING HELPERS
In-Reply-To: <20200526191303.1492-10-james.quinlan@broadcom.com>

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

Hi Jim,
one thing comes to mind, there is a small test suite in drivers/of/unittest.c
(specifically of_unittest_pci_dma_ranges()) you could extend it to include your
use cases.

On Tue, 2020-05-26 at 15:12 -0400, Jim Quinlan wrote:
> The new field in struct device 'dma_pfn_offset_map' is used to facilitate
> the use of multiple pfn offsets between cpu addrs and dma addrs.  It is
> similar to 'dma_pfn_offset' except that the offset chosen depends on the
> cpu or dma address involved.
> 
> Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
> ---
>  drivers/of/address.c        | 65 +++++++++++++++++++++++++++++++++++--
>  drivers/usb/core/message.c  |  3 ++
>  drivers/usb/core/usb.c      |  3 ++
>  include/linux/device.h      | 10 +++++-
>  include/linux/dma-direct.h  | 10 ++++--
>  include/linux/dma-mapping.h | 46 ++++++++++++++++++++++++++
>  kernel/dma/Kconfig          | 13 ++++++++
>  7 files changed, 144 insertions(+), 6 deletions(-)
> 

[...]

> @@ -977,10 +1020,19 @@ int of_dma_get_range(struct device *dev, struct
> device_node *np, u64 *dma_addr,
>  		pr_debug("dma_addr(%llx) cpu_addr(%llx) size(%llx)\n",
>  			 range.bus_addr, range.cpu_addr, range.size);
>  
> +		num_ranges++;
>  		if (dma_offset && range.cpu_addr - range.bus_addr != dma_offset)
> {
> -			pr_warn("Can't handle multiple dma-ranges with different
> offsets on node(%pOF)\n", node);
> -			/* Don't error out as we'd break some existing DTs */
> -			continue;
> +			if (!IS_ENABLED(CONFIG_DMA_PFN_OFFSET_MAP)) {
> +				pr_warn("Can't handle multiple dma-ranges with
> different offsets on node(%pOF)\n", node);
> +				pr_warn("Perhaps set DMA_PFN_OFFSET_MAP=y?\n");
> +				/*
> +				 * Don't error out as we'd break some existing
> +				 * DTs that are using configs w/o
> +				 * CONFIG_DMA_PFN_OFFSET_MAP set.
> +				 */
> +				continue;

dev->bus_dma_limit is set in of_dma_configure(), this function's caller, based
on dma_start's value (set after this continue). So you'd be effectively setting
the dev->bus_dma_limit to whatever we get from the first dma-range.

This can be troublesome depending on how the dma-ranges are setup, for example
if the first dma-range doesn't include the CMA area, in arm64 generally set as
high as possible in ZONE_DMA32, that would render it useless for
dma/{direct/swiotlb}. Again depending on the bus_dma_limit value, if smaller
than ZONE_DMA you'd be unable to allocate any DMA memory.

IMO, a solution to this calls for a revamp of dma-direct's dma_capable(): match
the target DMA memory area with each dma-range we have to see if it fits.

> +			}
> +			dma_multi_pfn_offset = true;
>  		}
>  		dma_offset = range.cpu_addr - range.bus_addr;
>  
> @@ -991,6 +1043,13 @@ int of_dma_get_range(struct device *dev, struct
> device_node *np, u64 *dma_addr,
>  			dma_end = range.bus_addr + range.size;
>  	}
>  
> +	if (dma_multi_pfn_offset) {
> +		dma_offset = 0;
> +		ret = attach_dma_pfn_offset_map(dev, node, num_ranges);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	if (dma_start >= dma_end) {
>  		ret = -EINVAL;
>  		pr_debug("Invalid DMA ranges configuration on node(%pOF)\n",
> diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
> index 6197938dcc2d..aaa3e58f5eb4 100644
> --- a/drivers/usb/core/message.c
> +++ b/drivers/usb/core/message.c
> @@ -1960,6 +1960,9 @@ int usb_set_configuration(struct usb_device *dev, int
> configuration)
>  		 */
>  		intf->dev.dma_mask = dev->dev.dma_mask;
>  		intf->dev.dma_pfn_offset = dev->dev.dma_pfn_offset;
> +#ifdef CONFIG_DMA_PFN_OFFSET_MAP
> +		intf->dev.dma_pfn_offset_map = dev->dev.dma_pfn_offset_map;
> +#endif

Thanks for looking at this, that said, I see more instances of drivers changing
dma_pfn_offset outside of the core code. Why not doing this there too?

Also, are we 100% sure that dev->dev.dma_pfn_offset isn't going to be freed
before we're done using intf->dev? Maybe it's safer to copy the ranges?

>  		INIT_WORK(&intf->reset_ws, __usb_queue_reset_device);
>  		intf->minor = -1;
>  		device_initialize(&intf->dev);
> diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> index f16c26dc079d..d2ed4d90e56e 100644
> --- a/drivers/usb/core/usb.c
> +++ b/drivers/usb/core/usb.c
> @@ -612,6 +612,9 @@ struct usb_device *usb_alloc_dev(struct usb_device
> *parent,
>  	 */
>  	dev->dev.dma_mask = bus->sysdev->dma_mask;
>  	dev->dev.dma_pfn_offset = bus->sysdev->dma_pfn_offset;
> +#ifdef CONFIG_DMA_PFN_OFFSET_MAP
> +	dev->dev.dma_pfn_offset_map = bus->sysdev->dma_pfn_offset_map;
> +#endif
>  	set_dev_node(&dev->dev, dev_to_node(bus->sysdev));
>  	dev->state = USB_STATE_ATTACHED;
>  	dev->lpm_disable_count = 1;
> diff --git a/include/linux/device.h b/include/linux/device.h
> index ac8e37cd716a..67a240ad4fc5 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -493,6 +493,8 @@ struct dev_links_info {
>   * @bus_dma_limit: Limit of an upstream bridge or bus which imposes a smaller
>   *		DMA limit than the device itself supports.
>   * @dma_pfn_offset: offset of DMA memory range relatively of RAM
> + * @dma_pfn_offset_map:	Like dma_pfn_offset but used when there are
> multiple
> + *		pfn offsets for multiple dma-ranges.
>   * @dma_parms:	A low level driver may set these to teach IOMMU code
> about
>   * 		segment limitations.
>   * @dma_pools:	Dma pools (if dma'ble device).
> @@ -578,7 +580,13 @@ struct device {
>  					     allocations such descriptors. */
>  	u64		bus_dma_limit;	/* upstream dma constraint */
>  	unsigned long	dma_pfn_offset;
> -
> +#ifdef CONFIG_DMA_PFN_OFFSET_MAP
> +	const struct dma_pfn_offset_region *dma_pfn_offset_map;
> +					/* Like dma_pfn_offset, but for
> +					 * the unlikely case of multiple
> +					 * offsets. If non-null, dma_pfn_offset
> +					 * will be set to 0. */
> +#endif

I'm still sad this doesn't fully replace dma_pfn_offset & bus_dma_limit. I feel
the extra logic involved in incorporating this as default isn't going to be
noticeable as far as performance is concerned to single dma-range users, and
it'd make for a nicer DMA code. Also you'd force everyone to test their changes
on the multi dma-ranges code path, as opposed to having this disabled 99.9% of
the time (hence broken every so often).

Note that I sympathize with the amount of work involved on improving that, so
better wait to hear what more knowledgeable people have to say about this :)

Regards,
Nicolas


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATCH 1/2] dt-bindings: sound: tlv320adcx140: Add GPI config property
From: Mark Brown @ 2020-05-27 14:58 UTC (permalink / raw)
  To: lgirdwood, tiwai, Dan Murphy, perex; +Cc: linux-kernel, alsa-devel, devicetree
In-Reply-To: <20200526200917.10385-1-dmurphy@ti.com>

On Tue, 26 May 2020 15:09:16 -0500, Dan Murphy wrote:
> Add an array property that configures the General Purpose Input (GPI)
> register.  The device has 4 GPI pins and each pin can be configured in 1
> of 7 different ways.

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/2] dt-bindings: sound: tlv320adcx140: Add GPI config property
      commit: 2465d32bea35d1d56c6cfb08a96ebea3b475d8ec
[2/2] ASoC: tlv320adcx140: Add support for configuring GPI pins
      commit: 3c35e79cead31c3bd79875ae90f9655dc77ad13c

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

^ permalink raw reply

* Re: [PATCH] ASoC: tlv320adcx140: Fix warnings when using W=1
From: Mark Brown @ 2020-05-27 14:58 UTC (permalink / raw)
  To: lgirdwood, tiwai, Dan Murphy, perex
  Cc: linux-kernel, kbuild test robot, alsa-devel, devicetree
In-Reply-To: <20200526175247.15309-1-dmurphy@ti.com>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1323 bytes --]

On Tue, 26 May 2020 12:52:47 -0500, Dan Murphy wrote:
> Fix the warnings when using the W=1 compiler flag.
> 
> sound/soc/codecs/tlv320adcx140.c: In function ‘adcx140_reset’:
> sound/soc/codecs/tlv320adcx140.c:570:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable]
>   570 |  int ret = 0;
>       |      ^~~
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: tlv320adcx140: Fix warnings when using W=1
      commit: 850ba84b5c6d4ad4d1259584ebc0338eb769f2ef

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

^ permalink raw reply

* Re: [PATCH v4 10/11] i2c: designware: Move reg-space remapping into a dedicated function
From: Serge Semin @ 2020-05-27 14:54 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Serge Semin, Jarkko Nikula, Wolfram Sang, Mika Westerberg,
	Alexey Malahov, Thomas Bogendoerfer, Rob Herring, linux-mips,
	devicetree, linux-i2c, linux-kernel
In-Reply-To: <20200527135801.GB1634618@smile.fi.intel.com>

On Wed, May 27, 2020 at 04:58:01PM +0300, Andy Shevchenko wrote:
> On Wed, May 27, 2020 at 03:01:10PM +0300, Serge Semin wrote:
> > This is a preparation patch before adding a quirk with custom registers
> > map creation required for the Baikal-T1 System I2C support.
> 
> Looks good. Though one nit below.
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: linux-mips@vger.kernel.org
> > Cc: devicetree@vger.kernel.org
> > 
> > ---
> > 
> > Changelog v3:
> > - This is a new patch, which has been created due to declining the
> >   glue-layer approach.
> > 
> > Changelog v4:
> > - Use PTR_ERR_OR_ZERO() helper in the bt1_i2c_request_regs() method.
> > - Discard devm_platform_get_and_ioremap_resource() utilization.
> > ---
> >  drivers/i2c/busses/i2c-designware-platdrv.c | 18 +++++++++++++-----
> >  1 file changed, 13 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> > index ad292de2d260..061c8d506c7c 100644
> > --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> > +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> > @@ -108,6 +108,15 @@ static void dw_i2c_plat_pm_cleanup(struct dw_i2c_dev *dev)
> >  		pm_runtime_put_noidle(dev->dev);
> >  }
> >  
> > +static int dw_i2c_plat_request_regs(struct dw_i2c_dev *dev)
> > +{
> > +	struct platform_device *pdev = to_platform_device(dev->dev);
> > +
> > +	dev->base = devm_platform_ioremap_resource(pdev, 0);
> > +
> > +	return PTR_ERR_OR_ZERO(dev->base);
> > +}
> > +
> >  static int dw_i2c_plat_probe(struct platform_device *pdev)
> >  {
> >  	struct dw_i2c_platform_data *pdata = dev_get_platdata(&pdev->dev);
> > @@ -125,15 +134,14 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
> >  		return -ENOMEM;
> >  
> 
> >  	dev->flags |= (uintptr_t)device_get_match_data(&pdev->dev);
> 
> It's related to previous patch, but I just realized that '|' is not needed
> here. Care to amend in the previous patch?

Agreed. Thanks.

-Sergey

> 
> > -
> > -	dev->base = devm_platform_ioremap_resource(pdev, 0);
> > -	if (IS_ERR(dev->base))
> > -		return PTR_ERR(dev->base);
> > -
> >  	dev->dev = &pdev->dev;
> >  	dev->irq = irq;
> >  	platform_set_drvdata(pdev, dev);
> >  
> > +	ret = dw_i2c_plat_request_regs(dev);
> > +	if (ret)
> > +		return ret;
> > +
> >  	dev->rst = devm_reset_control_get_optional_exclusive(&pdev->dev, NULL);
> >  	if (IS_ERR(dev->rst))
> >  		return PTR_ERR(dev->rst);
> > -- 
> > 2.26.2
> > 
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

^ permalink raw reply

* Re: [PATCH net-next v3 4/4] net: dp83869: Add RGMII internal delay configuration
From: Dan Murphy @ 2020-05-27 14:51 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: f.fainelli, hkallweit1, davem, robh, netdev, linux-kernel,
	devicetree
In-Reply-To: <20200527131204.GB793752@lunn.ch>

Andrew

On 5/27/20 8:12 AM, Andrew Lunn wrote:
>> If the dt defines rgmii-rx/tx-id then these values are required not
>> optional.  That was the discussion on the binding.
> How many times do i need to say it. They are optional. If not
> specified, default to 2ns.

OK.  I guess then the DP83867 driver is wrong because it specifically 
states in bold

     /* RX delay *must* be specified if internal delay of RX is used. */

It was signed off in commit fafc5db28a2ff


>>>> +	ret = of_property_read_u32(of_node, "tx-internal-delay-ps",
>>>> +				   &dp83869->tx_id_delay);
>>>> +	if (ret) {
>>>> +		dp83869->tx_id_delay = ret;
>>>> +		ret = 0;
>>>> +	}
>>>> +
>>>>    	return ret;
>>>>    }
>>>>    #else
>>>> @@ -367,10 +388,45 @@ static int dp83869_configure_mode(struct phy_device *phydev,
>>>>    	return ret;
>>>>    }
>>>> +static int dp83869_get_delay(struct phy_device *phydev)
>>>> +{
>>>> +	struct dp83869_private *dp83869 = phydev->priv;
>>>> +	int delay_size = ARRAY_SIZE(dp83869_internal_delay);
>>>> +	int tx_delay = 0;
>>>> +	int rx_delay = 0;
>>>> +
>>>> +	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
>>>> +	    phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
>>>> +		tx_delay = phy_get_delay_index(phydev,
>>>> +					       &dp83869_internal_delay[0],
>>>> +					       delay_size, dp83869->tx_id_delay,
>>>> +					       false);
>>>> +		if (tx_delay < 0) {
>>>> +			phydev_err(phydev, "Tx internal delay is invalid\n");
>>>> +			return tx_delay;
>>>> +		}
>>>> +	}
>>>> +
>>>> +	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
>>>> +	    phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
>>>> +		rx_delay = phy_get_delay_index(phydev,
>>>> +					       &dp83869_internal_delay[0],
>>>> +					       delay_size, dp83869->rx_id_delay,
>>>> +					       false);
>>>> +		if (rx_delay < 0) {
>>>> +			phydev_err(phydev, "Rx internal delay is invalid\n");
>>>> +			return rx_delay;
>>>> +		}
>>>> +	}
>>> So any PHY using these properties is going to pretty much reproduce
>>> this code. Meaning is should all be in a helper.
>> The issue here is that the phy_mode may only be rgmii-txid so you only want
>> to find the tx_delay and return.
>>
>> Same with the RXID.  How is the helper supposed to know what delay to return
>> and look for?
> How does this code do it? It looks at the value of interface.

Actually I will be removing this check with setting the delays to default.

Dan


>      Andrew

^ permalink raw reply

* Re: [PATCH v8 06/14] media: platform: Improve the implementation of the system PM ops
From: Tomasz Figa @ 2020-05-27 14:46 UTC (permalink / raw)
  To: Xia Jiang
  Cc: Hans Verkuil, Mauro Carvalho Chehab, Rob Herring,
	Matthias Brugger, Rick Chang, Linux Media Mailing List,
	linux-devicetree, Linux Kernel Mailing List,
	list@263.net:IOMMU DRIVERS <iommu@lists.linux-foundation.org>, Joerg Roedel <joro@8bytes.org>,,
	moderated list:ARM/Mediatek SoC support, Marek Szyprowski,
	srv_heupstream, Sergey Senozhatsky, Hsu Wei-Cheng,
	Nicolas Boichat, maoguang.meng, Sj Huang
In-Reply-To: <1590544320.12671.10.camel@mhfsdcap03>

On Wed, May 27, 2020 at 3:58 AM Xia Jiang <xia.jiang@mediatek.com> wrote:
>
> On Thu, 2020-05-21 at 15:32 +0000, Tomasz Figa wrote:
> > Hi Xia,
> >
> > On Fri, Apr 03, 2020 at 05:40:25PM +0800, Xia Jiang wrote:
> > > Cancel reset hw operation in suspend and resume function because this
> > > will be done in device_run().
> >
> > This and...
> >
> > > Add spin_lock and unlock operation in irq and resume function to make
> > > sure that the current frame is processed completely before suspend.
> >
> > ...this are two separate changes. Please split.
> >
> > >
> > > Signed-off-by: Xia Jiang <xia.jiang@mediatek.com>
> > > ---
> > >  drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c | 11 +++++++++--
> > >  1 file changed, 9 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > > index dd5cadd101ef..2fa3711fdc9b 100644
> > > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > > @@ -911,6 +911,8 @@ static irqreturn_t mtk_jpeg_dec_irq(int irq, void *priv)
> > >     u32 dec_ret;
> > >     int i;
> > >
> > > +   spin_lock(&jpeg->hw_lock);
> > > +
> >
> > nit: For consistency, it is recommended to always use the same, i.e. the
> > strongest, spin_(un)lock_ primitives when operating on the same spinlock.
> > In this case it would be the irqsave(restore) variants.
> >
> > >     dec_ret = mtk_jpeg_dec_get_int_status(jpeg->dec_reg_base);
> > >     dec_irq_ret = mtk_jpeg_dec_enum_result(dec_ret);
> > >     ctx = v4l2_m2m_get_curr_priv(jpeg->m2m_dev);
> > > @@ -941,6 +943,7 @@ static irqreturn_t mtk_jpeg_dec_irq(int irq, void *priv)
> > >     v4l2_m2m_buf_done(src_buf, buf_state);
> > >     v4l2_m2m_buf_done(dst_buf, buf_state);
> > >     v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
> > > +   spin_unlock(&jpeg->hw_lock);
> > >     pm_runtime_put_sync(ctx->jpeg->dev);
> > >     return IRQ_HANDLED;
> > >  }
> > > @@ -1191,7 +1194,6 @@ static __maybe_unused int mtk_jpeg_pm_suspend(struct device *dev)
> > >  {
> > >     struct mtk_jpeg_dev *jpeg = dev_get_drvdata(dev);
> > >
> > > -   mtk_jpeg_dec_reset(jpeg->dec_reg_base);
> > >     mtk_jpeg_clk_off(jpeg);
> > >
> > >     return 0;
> > > @@ -1202,19 +1204,24 @@ static __maybe_unused int mtk_jpeg_pm_resume(struct device *dev)
> > >     struct mtk_jpeg_dev *jpeg = dev_get_drvdata(dev);
> > >
> > >     mtk_jpeg_clk_on(jpeg);
> > > -   mtk_jpeg_dec_reset(jpeg->dec_reg_base);
> > >
> > >     return 0;
> > >  }
> > >
> > >  static __maybe_unused int mtk_jpeg_suspend(struct device *dev)
> > >  {
> > > +   struct mtk_jpeg_dev *jpeg = dev_get_drvdata(dev);
> > > +   unsigned long flags;
> > >     int ret;
> > >
> > >     if (pm_runtime_suspended(dev))
> > >             return 0;
> > >
> > > +   spin_lock_irqsave(&jpeg->hw_lock, flags);
> >
> > What does this spinlock protect us from? I can see that it would prevent
> > the interrupt handler from being called, but is it okay to suspend the
> > system without handling the interrupt?
> Dear Tomasz,
> I mean that if current image is processed in irq handler,suspend
> function can not get the lock(it was locked in irq handler).Should I
> move the spin_lock_irqsave(&jpeg->hw_lock, flags) to the start location
> of suspend function or

Do we have any guarantee that the interrupt handler would be executed
and acquire the spinlock before mtk_jpeg_suspend() is called?

> use wait_event_timeout() to handle the interrupt
> before suspend?

Yes, that would indeed work better. :)

However, please refer to the v4l2_m2m suspend/resume helpers [1] and
the MTK FD driver [2] for how to implement this nicely.

[1] https://patchwork.kernel.org/patch/11272917/
[2] https://patchwork.kernel.org/patch/11272903/

Best regards,
Tomasz

^ permalink raw reply

* Re: [PATCH v3 05/10] media: i2c: imx290: Add configurable link frequency and pixel rate
From: kbuild test robot @ 2020-05-27 14:43 UTC (permalink / raw)
  To: Andrey Konovalov, mchehab, sakari.ailus, manivannan.sadhasivam
  Cc: kbuild-all, linux-media, linux-kernel, devicetree,
	linux-arm-kernel, c.barrett, a.brela, peter.griffin,
	Andrey Konovalov
In-Reply-To: <20200524192505.20682-6-andrey.konovalov@linaro.org>

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

Hi Andrey,

I love your patch! Yet something to improve:

[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v5.7-rc7 next-20200526]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Andrey-Konovalov/Improvements-to-IMX290-CMOS-driver/20200525-032909
base:   git://linuxtv.org/media_tree.git master
config: microblaze-randconfig-c023-20200527 (attached as .config)
compiler: microblaze-linux-gcc (GCC) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>, old ones prefixed by <<):

microblaze-linux-ld: drivers/media/i2c/imx290.o: in function `imx290_calc_pixel_rate':
>> drivers/media/i2c/imx290.c:465: undefined reference to `__divdi3'

vim +465 drivers/media/i2c/imx290.c

   458	
   459	static u64 imx290_calc_pixel_rate(struct imx290 *imx290)
   460	{
   461		s64 link_freq = imx290_get_link_freq(imx290);
   462		u8 nlanes = imx290->nlanes;
   463	
   464		/* pixel rate = link_freq * 2 * nr_of_lanes / bits_per_sample */
 > 465		return (link_freq * 2 * nlanes / 10);
   466	}
   467	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 32872 bytes --]

^ permalink raw reply

* Re: [PATCH v4 08/11] i2c: designware: Convert driver to using regmap API
From: Serge Semin @ 2020-05-27 14:39 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Serge Semin, Jarkko Nikula, Wolfram Sang, Mika Westerberg,
	Alexey Malahov, Thomas Bogendoerfer, Rob Herring, devicetree,
	linux-mips, linux-i2c, linux-kernel
In-Reply-To: <20200527135023.GZ1634618@smile.fi.intel.com>

On Wed, May 27, 2020 at 04:50:23PM +0300, Andy Shevchenko wrote:
> On Wed, May 27, 2020 at 03:01:08PM +0300, Serge Semin wrote:
> > Seeing the DW I2C driver is using flags-based accessors with two
> > conditional clauses it would be better to replace them with the regmap
> > API IO methods and to initialize the regmap object with read/write
> > callbacks specific to the controller registers map implementation. This
> > will be also handy for the drivers with non-standard registers mapping
> > (like an embedded into the Baikal-T1 System Controller DW I2C block, which
> > glue-driver is a part of this series).
> > 
> > As before the driver tries to detect the mapping setup at probe stage and
> > creates a regmap object accordingly, which will be used by the rest of the
> > code to correctly access the controller registers. In two places it was
> > appropriate to convert the hand-written read-modify-write and
> > read-poll-loop design patterns to the corresponding regmap API
> > ready-to-use methods.
> > 
> > Note the regmap IO methods return value is checked only at the probe
> > stage. The rest of the code won't do this because basically we have
> > MMIO-based regmap so non of the read/write methods can fail (this also
> > won't be needed for the Baikal-T1-specific I2C controller).
> 
> Thanks! My comments below.
> 
> ...
> 
> >  #include <linux/export.h>
> >  #include <linux/i2c.h>
> >  #include <linux/interrupt.h>
> > +#include <linux/regmap.h>
> >  #include <linux/io.h>
> >  #include <linux/kernel.h>
> >  #include <linux/module.h>
> 

> Please, keep ordered.

Thanks. I'll fix the order. It must have been shifted after rebase.
I made sure the order was ok before that.

> 
> ...
> 
> > +static int dw_reg_write_word(void *context, unsigned int reg, unsigned int val)
> > +{
> > +	struct dw_i2c_dev *dev = context;
> > +
> 
> > +	writew_relaxed((u16)val, dev->base + reg);
> > +	writew_relaxed((u16)(val >> 16), dev->base + reg + 2);
> 
> What does explicit casting here help to?
> I think you may drop it.

Good question. It has been there originally. I'll remove it in the next patchset
version.

> 
> > +	return 0;
> >  }
> 
> ...
> 
> >  #include <linux/errno.h>
> >  #include <linux/i2c.h>
> >  #include <linux/interrupt.h>
> > +#include <linux/regmap.h>
> >  #include <linux/io.h>
> >  #include <linux/module.h>
> >  #include <linux/pm_runtime.h>
> 
> Order?

Ok.

-Sergey

> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

^ permalink raw reply

* Re: [PATCH 16/17] dt-bindings: watchdog: renesas,wdt: Document r8a7742 support
From: Rob Herring @ 2020-05-27 14:38 UTC (permalink / raw)
  To: Lad, Prabhakar
  Cc: Lad Prabhakar, Geert Uytterhoeven, Jens Axboe, Wolfram Sang,
	Ulf Hansson, Sergei Shtylyov, David S. Miller, Wim Van Sebroeck,
	Guenter Roeck,
	open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
	Linux I2C, Linux MMC List, netdev, Linux-Renesas,
	Linux Watchdog Mailing List
In-Reply-To: <CA+V-a8t6mXkTUac69V=T8_27r_sdN+=MktDTM1mmtbXRn8SSQQ@mail.gmail.com>

On Wed, May 27, 2020 at 5:23 AM Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:
>
> Hi Rob,
>
> On Wed, May 27, 2020 at 2:31 AM Rob Herring <robh@kernel.org> wrote:
> >
> > On Fri, May 15, 2020 at 04:08:56PM +0100, Lad Prabhakar wrote:
> > > RZ/G1H (R8A7742) watchdog implementation is compatible with R-Car Gen2,
> > > therefore add relevant documentation.
> > >
> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
> > > ---
> > >  Documentation/devicetree/bindings/watchdog/renesas,wdt.txt | 1 +
> > >  1 file changed, 1 insertion(+)
> >
> > Meanwhile in the DT tree, converting this schema landed. Can you prepare
> > a version based on the schema.
> >
> This was kindly taken care by Stephen during merge in linux-next [1].

Yes, I'm aware of that. I was hoping for a better commit message which
stands on its own (essentially the one here).

Rob

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox