Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH net-next v3 2/4] net: phy: Add a helper to return the index for of the internal delay
From: Dan Murphy @ 2020-05-27 12:13 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: f.fainelli, hkallweit1, davem, robh, netdev, linux-kernel,
	devicetree
In-Reply-To: <20200527004220.GE782807@lunn.ch>

Andrew

On 5/26/20 7:42 PM, Andrew Lunn wrote:
>> +/**
>> + * phy_get_delay_index - returns the index of the internal delay
>> + * @phydev: phy_device struct
>> + * @delay_values: array of delays the PHY supports
>> + * @size: the size of the delay array
>> + * @int_delay: the internal delay to be looked up
>> + * @descending: if the delay array is in descending order
>> + *
>> + * Returns the index within the array of internal delay passed in.
>> + * Return errno if the delay is invalid or cannot be found.
>> + */
>> +s32 phy_get_delay_index(struct phy_device *phydev, int *delay_values, int size,
>> +			int int_delay, bool descending)
>> +{
>> +	if (int_delay < 0)
>> +		return -EINVAL;
>> +
>> +	if (size <= 0)
>> +		return -EINVAL;
>> +
>> +	if (descending)
>> +		return phy_find_descending_delay(phydev, delay_values, size,
>> +						 int_delay);
>> +
>> +	return phy_find_ascending_delay(phydev, delay_values, size, int_delay);
>> +}
>> +EXPORT_SYMBOL(phy_get_delay_index);
> Do we really need this ascending vs descending? This array is not
> coming from device tree of anything, it is a static list in the PHY
> driver. I would just define it needs to be ascending and be done.

I was thinking about the constraints of having just an ascending array 
helper.

If there is a PHY out there that has a descending delay array then this 
function is not a helper.

Then the PHY driver now has to implement a descending search or extend 
out this helper to do the same.

I can just keep it ascending for now but this helper may need to be 
updated in the future to accommodate any PHYs with descending delay arrays.

Dan


> 	Andrew

^ permalink raw reply

* [PATCH v4 02/11] dt-bindings: i2c: Discard i2c-slave flag from the DW I2C example
From: Serge Semin @ 2020-05-27 12:01 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: <20200527120111.5781-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 v4 00/11] i2c: designeware: Add Baikal-T1 System I2C support
From: Serge Semin @ 2020-05-27 12:01 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 mainline Linux kernel 5.6-rc4:
base-commit: 0e698dfa2822 ("Linux 5.7-rc4")
tag: v5.7-rc4

Note new vendor prefix for Baikal-T1 System I2C device will be 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.

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 v4 03/11] dt-bindings: i2c: dw: Add Baikal-T1 SoC I2C controller
From: Serge Semin @ 2020-05-27 12:01 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: <20200527120111.5781-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 v4 04/11] i2c: designware: Use `-y` to build multi-object modules
From: Serge Semin @ 2020-05-27 12:01 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: <20200527120111.5781-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

* [PATCH v4 05/11] i2c: designware: slave: Set DW I2C core module dependency
From: Serge Semin @ 2020-05-27 12:01 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: <20200527120111.5781-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 v4 01/11] dt-bindings: i2c: Convert DW I2C binding to DT schema
From: Serge Semin @ 2020-05-27 12:01 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: <20200527120111.5781-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 v4 06/11] i2c: designware: Add Baytrail sem config DW I2C platform dependency
From: Serge Semin @ 2020-05-27 12:01 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: <20200527120111.5781-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 v4 07/11] i2c: designware: Discard Cherry Trail model flag
From: Serge Semin @ 2020-05-27 12:01 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: <20200527120111.5781-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.

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 v4 09/11] i2c: designware: Retrieve quirk flags as early as possible
From: Serge Semin @ 2020-05-27 12:01 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: <20200527120111.5781-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.

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.
---
 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..ad292de2d260 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 v4 10/11] i2c: designware: Move reg-space remapping into a dedicated function
From: Serge Semin @ 2020-05-27 12:01 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: <20200527120111.5781-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>
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);
-
-	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 v4 08/11] i2c: designware: Convert driver to using regmap API
From: Serge Semin @ 2020-05-27 12:01 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: <20200527120111.5781-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
---
 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..04c4cffb1987 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -17,6 +17,7 @@
 #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>
@@ -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((u16)val, dev->base + reg);
+	writew_relaxed((u16)(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..18e94fdc43f6 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -15,6 +15,7 @@
 #include <linux/gpio/consumer.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>
@@ -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..465d0f1bb751 100644
--- a/drivers/i2c/busses/i2c-designware-slave.c
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -11,6 +11,7 @@
 #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>
@@ -20,12 +21,12 @@
 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 v4 11/11] i2c: designware: Add Baikal-T1 System I2C support
From: Serge Semin @ 2020-05-27 12:01 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: <20200527120111.5781-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>
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


^ permalink raw reply related

* Re: [PATCH v3 03/12] dt-bindings: i2c: Discard i2c-slave flag from the DW I2C example
From: Serge Semin @ 2020-05-27 12:07 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Serge Semin, Jarkko Nikula, Wolfram Sang, Rob Herring,
	Alexey Malahov, Thomas Bogendoerfer, Andy Shevchenko,
	Mika Westerberg, linux-mips, linux-i2c, devicetree,
	Linux Kernel Mailing List
In-Reply-To: <CAHp75VdHC+fZNgc5oFbgYaNDebudDROcM_peRzOZpTVy55+tJg@mail.gmail.com>

On Wed, May 27, 2020 at 12:30:04PM +0300, Andy Shevchenko wrote:
> On Wed, May 27, 2020 at 1:00 AM Serge Semin
> <Sergey.Semin@baikalelectronics.ru> wrote:
> >
> > 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:
> 
> Shouldn't be dtc whatever tools fixed?

See the first patch in the series.

> 
> > 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.
> 
> Doesn't sound like a good idea. If user happens in between of these
> ping-pong change, how they will know this subtle issue?

As I see it, there are three ways we can follow.
1) Apply the patch and revert when dtc is fixed.
2) Apply the patch, but add a comment above the property, that we need
   to get the 0x40000064 address back when dtc is dixed.
3) Leave this ugly warning be until dtc is fixed.

In a comment to v2 Rob mentioned a solution like 1). Personally I am ok with
either, though I'd like to see a Rob's final comment about this.

-Sergey 

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

^ permalink raw reply

* RE: [PATCH v13 2/2] media: v4l: xilinx: Add Xilinx MIPI CSI-2 Rx Subsystem driver
From: Vishal Sagar @ 2020-05-27 12:03 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Hyun Kwon, mchehab@kernel.org, robh+dt@kernel.org,
	mark.rutland@arm.com, Michal Simek, linux-media@vger.kernel.org,
	devicetree@vger.kernel.org, hans.verkuil@cisco.com,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, Dinesh Kumar, Sandip Kothari,
	Luca Ceresoli, Jacopo Mondi, Hyun Kwon
In-Reply-To: <20200524022722.GC6026@pendragon.ideasonboard.com>

Hi Laurent,

Thanks for reviewing this patch.

> -----Original Message-----
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: Sunday, May 24, 2020 7:57 AM
> To: Vishal Sagar <vsagar@xilinx.com>
> Cc: Hyun Kwon <hyunk@xilinx.com>; mchehab@kernel.org;
> robh+dt@kernel.org; mark.rutland@arm.com; Michal Simek
> <michals@xilinx.com>; linux-media@vger.kernel.org;
> devicetree@vger.kernel.org; hans.verkuil@cisco.com; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Dinesh Kumar
> <dineshk@xilinx.com>; Sandip Kothari <sandipk@xilinx.com>; Luca Ceresoli
> <luca@lucaceresoli.net>; Jacopo Mondi <jacopo@jmondi.org>; Hyun Kwon
> <hyunk@xilinx.com>
> Subject: Re: [PATCH v13 2/2] media: v4l: xilinx: Add Xilinx MIPI CSI-2 Rx
> Subsystem driver
> 
> Hi Vishal,
> 
> Thank you for the patch.
> 
> On Tue, May 12, 2020 at 08:49:47PM +0530, Vishal Sagar wrote:
> > The Xilinx MIPI CSI-2 Rx Subsystem soft IP is used to capture images
> > from MIPI CSI-2 camera sensors and output AXI4-Stream video data ready
> > for image processing. Please refer to PG232 for details.
> >
> > The CSI2 Rx controller filters out all packets except for the packets
> > with data type fixed in hardware. RAW8 packets are always allowed to
> > pass through.
> >
> > It is also used to setup and handle interrupts and enable the core. It
> > logs all the events in respective counters between streaming on and off.
> >
> > The driver supports only the video format bridge enabled configuration.
> > Some data types like YUV 422 10bpc, RAW16, RAW20 are supported when
> > the CSI v2.0 feature is enabled in design. When the VCX feature is
> > enabled, the maximum number of virtual channels becomes 16 from 4.
> >
> > Signed-off-by: Vishal Sagar <vishal.sagar@xilinx.com>
> > Reviewed-by: Hyun Kwon <hyun.kwon@xilinx.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > v13
> > - Based on Laurent's suggestions
> > - Removed unnecessary debug statement for vep
> > - Added TODO for clock to enable disable at streamon/off
> > - Fix for index to start from 0 for get_nth_mbus_format
> > - Removed macro XCSI_TIMEOUT_VAL
> > - Remove ndelay from hard reset
> > - Remove hard reset from irq handler
> > - Fix short packet fifo clear
> > - Add TODO for v4l2_subdev_notify for SLBF error
> > - Fix the enable condition in s_stream
> > - Fix condition in xcsi2rxss_set_format
> > - Fix enum_mbus_code for double enumeration of RAW8 Data type
> > - Removed core struct
> > - Added reviewed by Laurent
> >
> > v12
> > - Changes done as suggested by Laurent Pinchart and Luca Ceresoli
> > - Removed unused macros
> > - No local storage of supported formats
> > - Dropped init mbus fmts and removed xcsi2rxss_init_mbus_fmts()
> > - XCSI_GET_BITSET_STR removed
> > - Add data type and mbus LUT
> >   - Added xcsi2rxss_get_nth_mbus() and xcsi2rxss_get_dt()
> > - Replaced all core->dev with dev in dev_dbg() and related debug
> > prints
> > - Replaced xcsi2rxss_log_ipconfig() with single line
> > - Removed small functions to enable/disable interrupts and core
> > - Now save remote subdev in state struct before streaming on
> > - Made xcsi2rxss_reset as soft_reset()
> > - Added hard reset using video-reset gpio
> >   - 2 modes one with delay and another sleep
> > - Instead of reset-gpios it is not video-reset-gpios
> > - In irq handler
> >   - Moved clearing of ISR up
> >   - Dump / empty short packet fifo
> >   - Irq handler is now threaded
> > - Added init_cfg pad ops and removed open()
> > - Updated xcsi2rxss_set_format(), xcsi2rxss_enum_mbus_code() to use
> > the dt mbus lut
> > - xcsi2rxss_set_default_format() updated
> > - Moved mutex_init()
> > - Updated graph handling
> > - Removed unnecessary prints
> > - Use devm_platform_ioremap_resource() and platform_get_irq()
> > - Update KConfig description
> >
> > v11
> > - Fixed changes as suggested by Sakari Ailus
> > - Removed VIDEO_XILINX from KConfig
> > - Minor formatting
> > - Start / Stop upstream sub-device in xcsi2rxss_start_stream()
> >   and xcsi2rxss_stop_stream()
> > - Added v4l2_subdev_link_validate_default() in v4l2_subdev_pad_ops()
> > - Use fwnode_graph_get_endpoint_by_id() instead of parsing by self
> > - Set bus type as V4L2_MBUS_CSI2_DPHY in struct v4l2_fwnode_endpoint
> > - Remove num_clks from core. Instead use ARRAY_SIZE()
> > - Fixed SPDX header to GPL-2.0
> > - Update copyright year to 2020
> >
> > v10
> > - Removed all V4L2 controls and events based on Sakari's comments.
> > - Now stop_stream() before toggling rst_gpio
> > - Updated init_mbus() to throw error on array out of bound access
> > - Make events and vcx_events as counters instead of structures
> > - Minor fixes in set_format() enum_mbus_code() as suggested by Sakari
> >
> > v9
> > - Moved all controls and events to xilinx-csi2rxss.h
> > - Updated name and description of controls and events
> > - Get control base address from v4l2-controls.h (0x10c0)
> > - Fix KConfig for dependency on VIDEO_XILINX
> > - Added enum_mbus_code() support
> > - Added default format to be returned on open()
> > - Mark variables are const
> > - Remove references to short packet in comments
> > - Add check for streaming before setting active lanes control
> > - strlcpy -> strscpy
> > - Fix xcsi2rxss_set_format()
> >
> > v8
> > - Use clk_bulk* APIs
> > - Add gpio reset for asserting video_aresetn when stream line buffer
> > occurs
> > - Removed short packet related events and irq handling
> >   - V4L2_EVENT_XLNXCSIRX_SPKT and V4L2_EVENT_XLNXCSIRX_SPKT_OVF
> > removed
> > - Removed frame counter control
> V4L2_CID_XILINX_MIPICSISS_FRAME_COUNTER
> >   and xcsi2rxss_g_volatile_ctrl()
> > - Minor formatting fixes
> >
> > v7
> > - No change
> >
> > v6
> > - No change
> >
> > v5
> > - Removed bayer and updated related parts like set default format based
> >   on Luca Cersoli's comments.
> > - Added correct YUV422 10bpc media bus format
> >
> > v4
> > - Removed irq member from core structure
> > - Consolidated IP config prints in xcsi2rxss_log_ipconfig()
> > - Return -EINVAL in case of invalid ioctl
> > - Code formatting
> > - Added reviewed by Hyun Kwon
> >
> > v3
> > - Fixed comments given by Hyun.
> > - Removed DPHY 200 MHz clock. This will be controlled by DPHY driver
> > - Minor code formatting
> > - en_csi_v20 and vfb members removed from struct and made local to dt
> > parsing
> > - lock description updated
> > - changed to ratelimited type for all dev prints in irq handler
> > - Removed YUV 422 10bpc media format
> >
> > v2
> > - Fixed comments given by Hyun and Sakari.
> > - Made all bitmask using BIT() and GENMASK()
> > - Removed unused definitions
> > - Removed DPHY access. This will be done by separate DPHY PHY driver.
> > - Added support for CSI v2.0 for YUV 422 10bpc, RAW16, RAW20 and extra
> >   virtual channels
> > - Fixed the ports as sink and source
> > - Now use the v4l2fwnode API to get number of data-lanes
> > - Added clock framework support
> > - Removed the close() function
> > - updated the set format function
> > - support only VFB enabled configuration
> >
> >  drivers/media/platform/xilinx/Kconfig         |    7 +
> >  drivers/media/platform/xilinx/Makefile        |    1 +
> >  .../media/platform/xilinx/xilinx-csi2rxss.c   | 1114 +++++++++++++++++
> >  3 files changed, 1122 insertions(+)
> >  create mode 100644 drivers/media/platform/xilinx/xilinx-csi2rxss.c
> >
> > diff --git a/drivers/media/platform/xilinx/Kconfig
> > b/drivers/media/platform/xilinx/Kconfig
> > index 01c96fb66414..44587dccacf1 100644
> > --- a/drivers/media/platform/xilinx/Kconfig
> > +++ b/drivers/media/platform/xilinx/Kconfig
> > @@ -12,6 +12,13 @@ config VIDEO_XILINX
> >
> >  if VIDEO_XILINX
> >
> > +config VIDEO_XILINX_CSI2RXSS
> > +	tristate "Xilinx CSI-2 Rx Subsystem"
> > +	help
> > +	  Driver for Xilinx MIPI CSI-2 Rx Subsystem. This is a V4L sub-device
> > +	  based driver that takes input from CSI-2 Tx source and converts
> > +	  it into an AXI4-Stream.
> > +
> >  config VIDEO_XILINX_TPG
> >  	tristate "Xilinx Video Test Pattern Generator"
> >  	depends on VIDEO_XILINX
> > diff --git a/drivers/media/platform/xilinx/Makefile
> > b/drivers/media/platform/xilinx/Makefile
> > index 4cdc0b1ec7a5..6119a34f3043 100644
> > --- a/drivers/media/platform/xilinx/Makefile
> > +++ b/drivers/media/platform/xilinx/Makefile
> > @@ -3,5 +3,6 @@
> >  xilinx-video-objs += xilinx-dma.o xilinx-vip.o xilinx-vipp.o
> >
> >  obj-$(CONFIG_VIDEO_XILINX) += xilinx-video.o
> > +obj-$(CONFIG_VIDEO_XILINX_CSI2RXSS) += xilinx-csi2rxss.o
> >  obj-$(CONFIG_VIDEO_XILINX_TPG) += xilinx-tpg.o
> >  obj-$(CONFIG_VIDEO_XILINX_VTC) += xilinx-vtc.o diff --git
> > a/drivers/media/platform/xilinx/xilinx-csi2rxss.c
> > b/drivers/media/platform/xilinx/xilinx-csi2rxss.c
> > new file mode 100644
> > index 000000000000..cac08149ae7a
> > --- /dev/null
> > +++ b/drivers/media/platform/xilinx/xilinx-csi2rxss.c
> > @@ -0,0 +1,1114 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Driver for Xilinx MIPI CSI-2 Rx Subsystem
> > + *
> > + * Copyright (C) 2016 - 2020 Xilinx, Inc.
> > + *
> > + * Contacts: Vishal Sagar <vishal.sagar@xilinx.com>
> > + *
> > + */
> > +#include <linux/clk.h>
> > +#include <linux/delay.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/module.h>
> > +#include <linux/mutex.h>
> > +#include <linux/of.h>
> > +#include <linux/of_irq.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/v4l2-subdev.h>
> > +#include <media/media-entity.h>
> > +#include <media/v4l2-common.h>
> > +#include <media/v4l2-ctrls.h>
> > +#include <media/v4l2-fwnode.h>
> > +#include <media/v4l2-subdev.h>
> > +#include "xilinx-vip.h"
> > +
> > +/* Register register map */
> > +#define XCSI_CCR_OFFSET		0x00
> > +#define XCSI_CCR_SOFTRESET	BIT(1)
> > +#define XCSI_CCR_ENABLE		BIT(0)
> > +
> > +#define XCSI_PCR_OFFSET		0x04
> > +#define XCSI_PCR_MAXLANES_MASK	GENMASK(4, 3)
> > +#define XCSI_PCR_ACTLANES_MASK	GENMASK(1, 0)
> > +
> > +#define XCSI_CSR_OFFSET		0x10
> > +#define XCSI_CSR_PKTCNT		GENMASK(31, 16)
> > +#define XCSI_CSR_SPFIFOFULL	BIT(3)
> > +#define XCSI_CSR_SPFIFONE	BIT(2)
> > +#define XCSI_CSR_SLBF		BIT(1)
> > +#define XCSI_CSR_RIPCD		BIT(0)
> > +
> > +#define XCSI_GIER_OFFSET	0x20
> > +#define XCSI_GIER_GIE		BIT(0)
> > +
> > +#define XCSI_ISR_OFFSET		0x24
> > +#define XCSI_IER_OFFSET		0x28
> > +
> > +#define XCSI_ISR_FR		BIT(31)
> > +#define XCSI_ISR_VCXFE		BIT(30)
> > +#define XCSI_ISR_WCC		BIT(22)
> > +#define XCSI_ISR_ILC		BIT(21)
> > +#define XCSI_ISR_SPFIFOF	BIT(20)
> > +#define XCSI_ISR_SPFIFONE	BIT(19)
> > +#define XCSI_ISR_SLBF		BIT(18)
> > +#define XCSI_ISR_STOP		BIT(17)
> > +#define XCSI_ISR_SOTERR		BIT(13)
> > +#define XCSI_ISR_SOTSYNCERR	BIT(12)
> > +#define XCSI_ISR_ECC2BERR	BIT(11)
> > +#define XCSI_ISR_ECC1BERR	BIT(10)
> > +#define XCSI_ISR_CRCERR		BIT(9)
> > +#define XCSI_ISR_DATAIDERR	BIT(8)
> > +#define XCSI_ISR_VC3FSYNCERR	BIT(7)
> > +#define XCSI_ISR_VC3FLVLERR	BIT(6)
> > +#define XCSI_ISR_VC2FSYNCERR	BIT(5)
> > +#define XCSI_ISR_VC2FLVLERR	BIT(4)
> > +#define XCSI_ISR_VC1FSYNCERR	BIT(3)
> > +#define XCSI_ISR_VC1FLVLERR	BIT(2)
> > +#define XCSI_ISR_VC0FSYNCERR	BIT(1)
> > +#define XCSI_ISR_VC0FLVLERR	BIT(0)
> > +
> > +#define XCSI_ISR_ALLINTR_MASK	(0xc07e3fff)
> > +
> > +/*
> > + * Removed VCXFE mask as it doesn't exist in IER
> > + * Removed STOP state irq as this will keep driver in irq handler
> > +only  */
> > +#define XCSI_IER_INTR_MASK	(XCSI_ISR_ALLINTR_MASK &\
> > +				 ~(XCSI_ISR_STOP | XCSI_ISR_VCXFE))
> > +
> > +#define XCSI_SPKTR_OFFSET	0x30
> > +#define XCSI_SPKTR_DATA		GENMASK(23, 8)
> > +#define XCSI_SPKTR_VC		GENMASK(7, 6)
> > +#define XCSI_SPKTR_DT		GENMASK(5, 0)
> > +#define XCSI_SPKT_FIFO_DEPTH	31
> > +
> > +#define XCSI_VCXR_OFFSET	0x34
> > +#define XCSI_VCXR_VCERR		GENMASK(23, 0)
> > +#define XCSI_VCXR_FSYNCERR	BIT(1)
> > +#define XCSI_VCXR_FLVLERR	BIT(0)
> > +
> > +#define XCSI_CLKINFR_OFFSET	0x3C
> > +#define XCSI_CLKINFR_STOP	BIT(1)
> > +
> > +#define XCSI_DLXINFR_OFFSET	0x40
> > +#define XCSI_DLXINFR_STOP	BIT(5)
> > +#define XCSI_DLXINFR_SOTERR	BIT(1)
> > +#define XCSI_DLXINFR_SOTSYNCERR	BIT(0)
> > +#define XCSI_MAXDL_COUNT	0x4
> > +
> > +#define XCSI_VCXINF1R_OFFSET		0x60
> > +#define XCSI_VCXINF1R_LINECOUNT		GENMASK(31, 16)
> > +#define XCSI_VCXINF1R_LINECOUNT_SHIFT	16
> > +#define XCSI_VCXINF1R_BYTECOUNT		GENMASK(15, 0)
> > +
> > +#define XCSI_VCXINF2R_OFFSET	0x64
> > +#define XCSI_VCXINF2R_DT	GENMASK(5, 0)
> > +#define XCSI_MAXVCX_COUNT	16
> > +
> > +/*
> > + * Sink pad connected to sensor source pad.
> > + * Source pad connected to next module like demosaic.
> > + */
> > +#define XCSI_MEDIA_PADS		2
> > +#define XCSI_DEFAULT_WIDTH	1920
> > +#define XCSI_DEFAULT_HEIGHT	1080
> > +
> > +/* MIPI CSI-2 Data Types from spec */
> > +#define XCSI_DT_YUV4228B	0x1e
> > +#define XCSI_DT_YUV42210B	0x1f
> > +#define XCSI_DT_RGB444		0x20
> > +#define XCSI_DT_RGB555		0x21
> > +#define XCSI_DT_RGB565		0x22
> > +#define XCSI_DT_RGB666		0x23
> > +#define XCSI_DT_RGB888		0x24
> > +#define XCSI_DT_RAW6		0x28
> > +#define XCSI_DT_RAW7		0x29
> > +#define XCSI_DT_RAW8		0x2a
> > +#define XCSI_DT_RAW10		0x2b
> > +#define XCSI_DT_RAW12		0x2c
> > +#define XCSI_DT_RAW14		0x2d
> > +#define XCSI_DT_RAW16		0x2e
> > +#define XCSI_DT_RAW20		0x2f
> > +
> > +#define XCSI_VCX_START		4
> > +#define XCSI_MAX_VC		4
> > +#define XCSI_MAX_VCX		16
> > +
> > +#define XCSI_NEXTREG_OFFSET	4
> > +
> > +/* There are 2 events frame sync and frame level error per VC */
> > +#define XCSI_VCX_NUM_EVENTS	((XCSI_MAX_VCX - XCSI_MAX_VC) * 2)
> > +
> > +/**
> > + * struct xcsi2rxss_event - Event log structure
> > + * @mask: Event mask
> > + * @name: Name of the event
> > + */
> > +struct xcsi2rxss_event {
> > +	u32 mask;
> > +	const char *name;
> > +};
> > +
> > +static const struct xcsi2rxss_event xcsi2rxss_events[] = {
> > +	{ XCSI_ISR_FR, "Frame Received" },
> > +	{ XCSI_ISR_VCXFE, "VCX Frame Errors" },
> > +	{ XCSI_ISR_WCC, "Word Count Errors" },
> > +	{ XCSI_ISR_ILC, "Invalid Lane Count Error" },
> > +	{ XCSI_ISR_SPFIFOF, "Short Packet FIFO OverFlow Error" },
> > +	{ XCSI_ISR_SPFIFONE, "Short Packet FIFO Not Empty" },
> > +	{ XCSI_ISR_SLBF, "Streamline Buffer Full Error" },
> > +	{ XCSI_ISR_STOP, "Lane Stop State" },
> > +	{ XCSI_ISR_SOTERR, "SOT Error" },
> > +	{ XCSI_ISR_SOTSYNCERR, "SOT Sync Error" },
> > +	{ XCSI_ISR_ECC2BERR, "2 Bit ECC Unrecoverable Error" },
> > +	{ XCSI_ISR_ECC1BERR, "1 Bit ECC Recoverable Error" },
> > +	{ XCSI_ISR_CRCERR, "CRC Error" },
> > +	{ XCSI_ISR_DATAIDERR, "Data Id Error" },
> > +	{ XCSI_ISR_VC3FSYNCERR, "Virtual Channel 3 Frame Sync Error" },
> > +	{ XCSI_ISR_VC3FLVLERR, "Virtual Channel 3 Frame Level Error" },
> > +	{ XCSI_ISR_VC2FSYNCERR, "Virtual Channel 2 Frame Sync Error" },
> > +	{ XCSI_ISR_VC2FLVLERR, "Virtual Channel 2 Frame Level Error" },
> > +	{ XCSI_ISR_VC1FSYNCERR, "Virtual Channel 1 Frame Sync Error" },
> > +	{ XCSI_ISR_VC1FLVLERR, "Virtual Channel 1 Frame Level Error" },
> > +	{ XCSI_ISR_VC0FSYNCERR, "Virtual Channel 0 Frame Sync Error" },
> > +	{ XCSI_ISR_VC0FLVLERR, "Virtual Channel 0 Frame Level Error" } };
> > +
> > +#define XCSI_NUM_EVENTS		ARRAY_SIZE(xcsi2rxss_events)
> > +
> > +/*
> > + * This table provides a mapping between CSI-2 Data type
> > + * and media bus formats
> > + */
> > +static const u32 xcsi2dt_mbus_lut[][2] = {
> > +	{ XCSI_DT_YUV4228B, MEDIA_BUS_FMT_UYVY8_1X16 },
> > +	{ XCSI_DT_YUV42210B, MEDIA_BUS_FMT_UYVY10_1X20 },
> > +	{ XCSI_DT_RGB444, 0 },
> > +	{ XCSI_DT_RGB555, 0 },
> > +	{ XCSI_DT_RGB565, 0 },
> > +	{ XCSI_DT_RGB666, 0 },
> > +	{ XCSI_DT_RGB888, MEDIA_BUS_FMT_RBG888_1X24 },
> > +	{ XCSI_DT_RAW6, 0 },
> > +	{ XCSI_DT_RAW7, 0 },
> > +	{ XCSI_DT_RAW8, MEDIA_BUS_FMT_SRGGB8_1X8 },
> > +	{ XCSI_DT_RAW8, MEDIA_BUS_FMT_SBGGR8_1X8 },
> > +	{ XCSI_DT_RAW8, MEDIA_BUS_FMT_SGBRG8_1X8 },
> > +	{ XCSI_DT_RAW8, MEDIA_BUS_FMT_SGRBG8_1X8 },
> > +	{ XCSI_DT_RAW10, MEDIA_BUS_FMT_SRGGB10_1X10 },
> > +	{ XCSI_DT_RAW10, MEDIA_BUS_FMT_SBGGR10_1X10 },
> > +	{ XCSI_DT_RAW10, MEDIA_BUS_FMT_SGBRG10_1X10 },
> > +	{ XCSI_DT_RAW10, MEDIA_BUS_FMT_SGRBG10_1X10 },
> > +	{ XCSI_DT_RAW12, MEDIA_BUS_FMT_SRGGB12_1X12 },
> > +	{ XCSI_DT_RAW12, MEDIA_BUS_FMT_SBGGR12_1X12 },
> > +	{ XCSI_DT_RAW12, MEDIA_BUS_FMT_SGBRG12_1X12 },
> > +	{ XCSI_DT_RAW12, MEDIA_BUS_FMT_SGRBG12_1X12 },
> > +	{ XCSI_DT_RAW16, MEDIA_BUS_FMT_SRGGB16_1X16 },
> > +	{ XCSI_DT_RAW16, MEDIA_BUS_FMT_SBGGR16_1X16 },
> > +	{ XCSI_DT_RAW16, MEDIA_BUS_FMT_SGBRG16_1X16 },
> > +	{ XCSI_DT_RAW16, MEDIA_BUS_FMT_SGRBG16_1X16 },
> > +	{ XCSI_DT_RAW20, 0 },
> > +};
> > +
> > +/**
> > + * struct xcsi2rxss_state - CSI-2 Rx Subsystem device structure
> > + * @subdev: The v4l2 subdev structure
> > + * @format: Active V4L2 formats on each pad
> > + * @default_format: Default V4L2 format
> > + * @events: counter for events
> > + * @vcx_events: counter for vcx_events
> > + * @dev: Platform structure
> > + * @rsubdev: Remote subdev connected to sink pad
> > + * @rst_gpio: reset to video_aresetn
> > + * @clks: array of clocks
> > + * @iomem: Base address of subsystem
> > + * @max_num_lanes: Maximum number of lanes present
> > + * @datatype: Data type filter
> > + * @lock: mutex for accessing this structure
> > + * @pads: media pads
> > + * @streaming: Flag for storing streaming state
> > + * @enable_active_lanes: If number of active lanes can be modified
> > + * @en_vcx: If more than 4 VC are enabled
> > + *
> > + * This structure contains the device driver related parameters  */
> > +struct xcsi2rxss_state {
> > +	struct v4l2_subdev subdev;
> > +	struct v4l2_mbus_framefmt format;
> > +	struct v4l2_mbus_framefmt default_format;
> > +	u32 events[XCSI_NUM_EVENTS];
> > +	u32 vcx_events[XCSI_VCX_NUM_EVENTS];
> > +	struct device *dev;
> > +	struct v4l2_subdev *rsubdev;
> > +	struct gpio_desc *rst_gpio;
> > +	struct clk_bulk_data *clks;
> > +	void __iomem *iomem;
> > +	u32 max_num_lanes;
> > +	u32 datatype;
> > +	/* used to protect access to this struct */
> > +	struct mutex lock;
> > +	struct media_pad pads[XCSI_MEDIA_PADS];
> > +	bool streaming;
> > +	bool enable_active_lanes;
> > +	bool en_vcx;
> > +};
> > +
> > +static const struct clk_bulk_data xcsi2rxss_clks[] = {
> > +	{ .id = "lite_aclk" },
> > +	{ .id = "video_aclk" },
> > +};
> > +
> > +static inline struct xcsi2rxss_state * to_xcsi2rxssstate(struct
> > +v4l2_subdev *subdev) {
> > +	return container_of(subdev, struct xcsi2rxss_state, subdev); }
> > +
> > +/*
> > + * Register related operations
> > + */
> > +static inline u32 xcsi2rxss_read(struct xcsi2rxss_state *xcsi2rxss,
> > +u32 addr) {
> > +	return ioread32(xcsi2rxss->iomem + addr); }
> > +
> > +static inline void xcsi2rxss_write(struct xcsi2rxss_state *xcsi2rxss, u32 addr,
> > +				   u32 value)
> > +{
> > +	iowrite32(value, xcsi2rxss->iomem + addr); }
> > +
> > +static inline void xcsi2rxss_clr(struct xcsi2rxss_state *xcsi2rxss, u32 addr,
> > +				 u32 clr)
> > +{
> > +	xcsi2rxss_write(xcsi2rxss, addr,
> > +			xcsi2rxss_read(xcsi2rxss, addr) & ~clr); }
> > +
> > +static inline void xcsi2rxss_set(struct xcsi2rxss_state *xcsi2rxss, u32 addr,
> > +				 u32 set)
> > +{
> > +	xcsi2rxss_write(xcsi2rxss, addr, xcsi2rxss_read(xcsi2rxss, addr) |
> > +set); }
> > +
> > +/*
> > + * This function returns the nth mbus for a data type.
> > + * In case of error, mbus code returned is 0.
> > + */
> > +static u32 xcsi2rxss_get_nth_mbus(u32 dt, u32 n) {
> > +	unsigned int i;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(xcsi2dt_mbus_lut); i++) {
> > +		if (xcsi2dt_mbus_lut[i][0] == dt) {
> > +			if (n-- == 0)
> > +				return xcsi2dt_mbus_lut[i][1];
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +/* This returns the data type for a media bus format else 0 */ static
> > +u32 xcsi2rxss_get_dt(u32 mbus) {
> > +	unsigned int i;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(xcsi2dt_mbus_lut); i++) {
> > +		if (xcsi2dt_mbus_lut[i][1] == mbus)
> > +			return xcsi2dt_mbus_lut[i][0];
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +/**
> > + * xcsi2rxss_soft_reset - Does a soft reset of the MIPI CSI-2 Rx
> > +Subsystem
> > + * @state: Xilinx CSI-2 Rx Subsystem structure pointer
> > + *
> > + * Core takes less than 100 video clock cycles to reset.
> > + * So a larger timeout value is chosen for margin.
> > + *
> > + * Return: 0 - on success OR -ETIME if reset times out  */ static int
> > +xcsi2rxss_soft_reset(struct xcsi2rxss_state *state) {
> > +	u32 timeout = 1000; /* us */
> > +
> > +	xcsi2rxss_set(state, XCSI_CCR_OFFSET, XCSI_CCR_SOFTRESET);
> > +
> > +	while (xcsi2rxss_read(state, XCSI_CSR_OFFSET) & XCSI_CSR_RIPCD) {
> > +		if (timeout == 0) {
> > +			dev_err(state->dev, "soft reset timed out!\n");
> > +			return -ETIME;
> > +		}
> > +
> > +		timeout--;
> > +		udelay(1);
> > +	}
> > +
> > +	xcsi2rxss_clr(state, XCSI_CCR_OFFSET, XCSI_CCR_SOFTRESET);
> > +	return 0;
> > +}
> > +
> > +static void xcsi2rxss_hard_reset(struct xcsi2rxss_state *state) {
> > +	if (!state->rst_gpio)
> > +		return;
> > +
> > +	/* minimum of 40 dphy_clk_200M cycles */
> > +	gpiod_set_value_cansleep(state->rst_gpio, 1);
> > +	usleep_range(1, 2);
> > +	gpiod_set_value_cansleep(state->rst_gpio, 0); }
> > +
> > +static void xcsi2rxss_reset_event_counters(struct xcsi2rxss_state
> > +*state) {
> > +	unsigned int i;
> > +
> > +	for (i = 0; i < XCSI_NUM_EVENTS; i++)
> > +		state->events[i] = 0;
> > +
> > +	for (i = 0; i < XCSI_VCX_NUM_EVENTS; i++)
> > +		state->vcx_events[i] = 0;
> > +}
> > +
> > +/* Print event counters */
> > +static void xcsi2rxss_log_counters(struct xcsi2rxss_state *state) {
> > +	struct device *dev = state->dev;
> > +	unsigned int i;
> > +
> > +	for (i = 0; i < XCSI_NUM_EVENTS; i++) {
> > +		if (state->events[i] > 0) {
> > +			dev_info(dev, "%s events: %d\n",
> > +				 xcsi2rxss_events[i].name,
> > +				 state->events[i]);
> > +		}
> > +	}
> > +
> > +	if (state->en_vcx) {
> > +		for (i = 0; i < XCSI_VCX_NUM_EVENTS; i++) {
> > +			if (state->vcx_events[i] > 0) {
> > +				dev_info(dev,
> > +					 "VC %d Frame %s err vcx events:
> %d\n",
> > +					 (i / 2) + XCSI_VCX_START,
> > +					 i & 1 ? "Sync" : "Level",
> > +					 state->vcx_events[i]);
> > +			}
> > +		}
> > +	}
> > +}
> > +
> > +/**
> > + * xcsi2rxss_log_status - Logs the status of the CSI-2 Receiver
> > + * @sd: Pointer to V4L2 subdevice structure
> > + *
> > + * This function prints the current status of Xilinx MIPI CSI-2
> > + *
> > + * Return: 0 on success
> > + */
> > +static int xcsi2rxss_log_status(struct v4l2_subdev *sd) {
> > +	struct xcsi2rxss_state *xcsi2rxss = to_xcsi2rxssstate(sd);
> > +	struct device *dev = xcsi2rxss->dev;
> > +	const char *tr = "true";
> > +	const char *fa = "false";
> 
> I didn't notice this in the previous version, but you could just write "true" and
> "false" below. The compiler should perform string de-duplication automatically.
> 

I had this as a macro XCSI_GET_BITSET_STR till v11 and removed in v12
/* Macro to return "true" or "false" string if bit is set */
#define XCSI_GET_BITSET_STR(val, mask)	(val) & (mask) ? "true" : "false"

I will update this to "true" and "false".

> > +	u32 reg, data;
> > +	unsigned int i, max_vc;
> > +
> > +	mutex_lock(&xcsi2rxss->lock);
> > +
> > +	xcsi2rxss_log_counters(xcsi2rxss);
> > +
> > +	dev_info(dev, "***** Core Status *****\n");
> > +	data = xcsi2rxss_read(xcsi2rxss, XCSI_CSR_OFFSET);
> > +	dev_info(dev, "Short Packet FIFO Full = %s\n",
> > +		 data & XCSI_CSR_SPFIFOFULL ? tr : fa);
> > +	dev_info(dev, "Short Packet FIFO Not Empty = %s\n",
> > +		 data & XCSI_CSR_SPFIFONE ? tr : fa);
> > +	dev_info(dev, "Stream line buffer full = %s\n",
> > +		 data & XCSI_CSR_SLBF ? tr : fa);
> > +	dev_info(dev, "Soft reset/Core disable in progress = %s\n",
> > +		 data & XCSI_CSR_RIPCD ? tr : fa);
> > +
> > +	/* Clk & Lane Info  */
> > +	dev_info(dev, "******** Clock Lane Info *********\n");
> > +	data = xcsi2rxss_read(xcsi2rxss, XCSI_CLKINFR_OFFSET);
> > +	dev_info(dev, "Clock Lane in Stop State = %s\n",
> > +		 data & XCSI_CLKINFR_STOP ? tr : fa);
> > +
> > +	dev_info(dev, "******** Data Lane Info *********\n");
> > +	dev_info(dev, "Lane\tSoT Error\tSoT Sync Error\tStop State\n");
> > +	reg = XCSI_DLXINFR_OFFSET;
> > +	for (i = 0; i < XCSI_MAXDL_COUNT; i++) {
> > +		data = xcsi2rxss_read(xcsi2rxss, reg);
> > +
> > +		dev_info(dev, "%d\t%s\t\t%s\t\t%s\n", i,
> > +			 data & XCSI_DLXINFR_SOTERR ? tr : fa,
> > +			 data & XCSI_DLXINFR_SOTSYNCERR ? tr : fa,
> > +			 data & XCSI_DLXINFR_STOP ? tr : fa);
> > +
> > +		reg += XCSI_NEXTREG_OFFSET;
> > +	}
> > +
> > +	/* Virtual Channel Image Information */
> > +	dev_info(dev, "********** Virtual Channel Info ************\n");
> > +	dev_info(dev, "VC\tLine Count\tByte Count\tData Type\n");
> > +	if (xcsi2rxss->en_vcx)
> > +		max_vc = XCSI_MAX_VCX;
> > +	else
> > +		max_vc = XCSI_MAX_VC;
> > +
> > +	reg = XCSI_VCXINF1R_OFFSET;
> > +	for (i = 0; i < max_vc; i++) {
> > +		u32 line_count, byte_count, data_type;
> > +
> > +		/* Get line and byte count from VCXINFR1 Register */
> > +		data = xcsi2rxss_read(xcsi2rxss, reg);
> > +		byte_count = data & XCSI_VCXINF1R_BYTECOUNT;
> > +		line_count = data & XCSI_VCXINF1R_LINECOUNT;
> > +		line_count >>= XCSI_VCXINF1R_LINECOUNT_SHIFT;
> > +
> > +		/* Get data type from VCXINFR2 Register */
> > +		reg += XCSI_NEXTREG_OFFSET;
> > +		data = xcsi2rxss_read(xcsi2rxss, reg);
> > +		data_type = data & XCSI_VCXINF2R_DT;
> > +
> > +		dev_info(dev, "%d\t%d\t\t%d\t\t0x%x\n", i, line_count,
> > +			 byte_count, data_type);
> > +
> > +		/* Move to next pair of VC Info registers */
> > +		reg += XCSI_NEXTREG_OFFSET;
> > +	}
> > +
> > +	mutex_unlock(&xcsi2rxss->lock);
> > +
> > +	return 0;
> > +}
> > +
> > +static struct v4l2_subdev *xcsi2rxss_get_remote_subdev(struct
> > +media_pad *local) {
> > +	struct media_pad *remote;
> > +
> > +	remote = media_entity_remote_pad(local);
> > +	if (!remote || !is_media_entity_v4l2_subdev(remote->entity))
> > +		return NULL;
> > +
> > +	return media_entity_to_v4l2_subdev(remote->entity);
> > +}
> > +
> > +static int xcsi2rxss_start_stream(struct xcsi2rxss_state *state) {
> > +	int ret = 0;
> > +
> > +	/* enable core */
> > +	xcsi2rxss_set(state, XCSI_CCR_OFFSET, XCSI_CCR_ENABLE);
> > +
> > +	ret = xcsi2rxss_soft_reset(state);
> > +	if (ret < 0) {
> > +		state->streaming = false;
> > +		return ret;
> > +	}
> > +
> > +	/* enable interrupts */
> > +	xcsi2rxss_clr(state, XCSI_GIER_OFFSET, XCSI_GIER_GIE);
> > +	xcsi2rxss_write(state, XCSI_IER_OFFSET, XCSI_IER_INTR_MASK);
> > +	xcsi2rxss_set(state, XCSI_GIER_OFFSET, XCSI_GIER_GIE);
> > +
> > +	state->streaming = true;
> > +
> > +	state->rsubdev =
> > +		xcsi2rxss_get_remote_subdev(&state->pads[XVIP_PAD_SINK]);
> > +
> > +	ret = v4l2_subdev_call(state->rsubdev, video, s_stream, 1);
> > +	if (ret) {
> > +		/* disable interrupts */
> > +		xcsi2rxss_clr(state, XCSI_IER_OFFSET, XCSI_IER_INTR_MASK);
> > +		xcsi2rxss_clr(state, XCSI_GIER_OFFSET, XCSI_GIER_GIE);
> > +
> > +		/* disable core */
> > +		xcsi2rxss_clr(state, XCSI_CCR_OFFSET, XCSI_CCR_ENABLE);
> > +		state->streaming = false;
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> > +static void xcsi2rxss_stop_stream(struct xcsi2rxss_state *state) {
> > +	v4l2_subdev_call(state->rsubdev, video, s_stream, 0);
> > +
> > +	/* disable interrupts */
> > +	xcsi2rxss_clr(state, XCSI_IER_OFFSET, XCSI_IER_INTR_MASK);
> > +	xcsi2rxss_clr(state, XCSI_GIER_OFFSET, XCSI_GIER_GIE);
> > +
> > +	/* disable core */
> > +	xcsi2rxss_clr(state, XCSI_CCR_OFFSET, XCSI_CCR_ENABLE);
> > +	state->streaming = false;
> > +}
> > +
> > +/**
> > + * xcsi2rxss_irq_handler - Interrupt handler for CSI-2
> > + * @irq: IRQ number
> > + * @data: Pointer to device state
> > + *
> > + * In the interrupt handler, a list of event counters are updated for
> > + * corresponding interrupts. This is useful to get status / debug.
> > + *
> > + * Return: IRQ_HANDLED after handling interrupts  */ static
> > +irqreturn_t xcsi2rxss_irq_handler(int irq, void *data) {
> > +	struct xcsi2rxss_state *state = (struct xcsi2rxss_state *)data;
> > +	struct device *dev = state->dev;
> > +	u32 status;
> > +
> > +	status = xcsi2rxss_read(state, XCSI_ISR_OFFSET) &
> XCSI_ISR_ALLINTR_MASK;
> > +	xcsi2rxss_write(state, XCSI_ISR_OFFSET, status);
> > +
> > +	/* Received a short packet */
> > +	if (status & XCSI_ISR_SPFIFONE) {
> > +		u32 count = 0;
> > +
> > +		/*
> > +		 * Drain generic short packet FIFO by reading max 31
> > +		 * (fifo depth) short packets from fifo or till fifo is empty.
> > +		 */
> > +		for (count = 0; count < XCSI_SPKT_FIFO_DEPTH; ++count) {
> > +			u32 spfifostat, spkt;
> > +
> > +			spkt = xcsi2rxss_read(state, XCSI_SPKTR_OFFSET);
> > +			dev_dbg(dev, "Short packet = 0x%08x\n", spkt);
> > +			spfifostat = xcsi2rxss_read(state, XCSI_ISR_OFFSET);
> > +			spfifostat &= XCSI_ISR_SPFIFONE;
> > +			if (!spfifostat)
> > +				break;
> > +			xcsi2rxss_write(state, XCSI_ISR_OFFSET, spfifostat);
> > +		}
> > +	}
> > +
> > +	/* Short packet FIFO overflow */
> > +	if (status & XCSI_ISR_SPFIFOF)
> > +		dev_dbg_ratelimited(dev, "Short packet FIFO overflowed\n");
> > +
> > +	/*
> > +	 * Stream line buffer full
> > +	 * This means there is a backpressure from downstream IP
> > +	 */
> > +	if (status & XCSI_ISR_SLBF) {
> > +		dev_alert_ratelimited(dev, "Stream Line Buffer Full!\n");
> > +
> > +		/* disable interrupts */
> > +		xcsi2rxss_clr(state, XCSI_IER_OFFSET, XCSI_IER_INTR_MASK);
> > +		xcsi2rxss_clr(state, XCSI_GIER_OFFSET, XCSI_GIER_GIE);
> > +
> > +		/* disable core */
> > +		xcsi2rxss_clr(state, XCSI_CCR_OFFSET, XCSI_CCR_ENABLE);
> > +		state->streaming = false;
> 
> If you set streaming to false, a later call to xcsi2rxss_s_stream(0) will consider
> that the stream is already stopped, and skip calling
> xcsi2rxss_stop_stream() and xcsi2rxss_hard_reset(). Now that you have
> dropped the hard reset from the interrupt handler, we need to rely on it being
> called at stream stop time. Should you just drop the line here ?
> 

Yes I can drop the line "state->streaming = false" here in next version.
The comment below should guide the user to stop streaming in stream line buffer full case.

> > +
> > +		/*
> > +		 * The IP needs to be hard reset before it can be used now.
> > +		 * This will be done in streamoff.
> > +		 */
> > +
> > +		/*
> > +		 * TODO: Notify the whole pipeline with v4l2_subdev_notify()
> to
> > +		 * inform userspace.
> > +		 */
> > +	}
> > +
> > +	/* Increment event counters */
> > +	if (status & XCSI_ISR_ALLINTR_MASK) {
> > +		unsigned int i;
> > +
> > +		for (i = 0; i < XCSI_NUM_EVENTS; i++) {
> > +			if (!(status & xcsi2rxss_events[i].mask))
> > +				continue;
> > +			state->events[i]++;
> > +			dev_dbg_ratelimited(dev, "%s: %u\n",
> > +					    xcsi2rxss_events[i].name,
> > +					    state->events[i]);
> > +		}
> > +
> > +		if (status & XCSI_ISR_VCXFE && state->en_vcx) {
> > +			u32 vcxstatus;
> > +
> > +			vcxstatus = xcsi2rxss_read(state, XCSI_VCXR_OFFSET);
> > +			vcxstatus &= XCSI_VCXR_VCERR;
> > +			for (i = 0; i < XCSI_VCX_NUM_EVENTS; i++) {
> > +				if (!(vcxstatus & (1 << i)))
> > +					continue;
> > +				state->vcx_events[i]++;
> > +			}
> > +			xcsi2rxss_write(state, XCSI_VCXR_OFFSET, vcxstatus);
> > +		}
> > +	}
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +/**
> > + * xcsi2rxss_s_stream - It is used to start/stop the streaming.
> > + * @sd: V4L2 Sub device
> > + * @enable: Flag (True / False)
> > + *
> > + * This function controls the start or stop of streaming for the
> > + * Xilinx MIPI CSI-2 Rx Subsystem.
> > + *
> > + * Return: 0 on success, errors otherwise  */ static int
> > +xcsi2rxss_s_stream(struct v4l2_subdev *sd, int enable) {
> > +	struct xcsi2rxss_state *xcsi2rxss = to_xcsi2rxssstate(sd);
> > +	int ret = 0;
> > +
> > +	mutex_lock(&xcsi2rxss->lock);
> > +
> > +	if (enable == xcsi2rxss->streaming)
> > +		goto stream_done;
> > +
> > +	if (enable) {
> > +		xcsi2rxss_reset_event_counters(xcsi2rxss);
> > +		ret = xcsi2rxss_start_stream(xcsi2rxss);
> > +	} else {
> > +		xcsi2rxss_stop_stream(xcsi2rxss);
> > +		xcsi2rxss_hard_reset(xcsi2rxss);
> > +	}
> > +
> > +stream_done:
> > +	mutex_unlock(&xcsi2rxss->lock);
> > +	return ret;
> > +}
> > +
> > +static struct v4l2_mbus_framefmt *
> > +__xcsi2rxss_get_pad_format(struct xcsi2rxss_state *xcsi2rxss,
> > +			   struct v4l2_subdev_pad_config *cfg,
> > +			   unsigned int pad, u32 which)
> > +{
> > +	switch (which) {
> > +	case V4L2_SUBDEV_FORMAT_TRY:
> > +		return v4l2_subdev_get_try_format(&xcsi2rxss->subdev, cfg,
> pad);
> > +	case V4L2_SUBDEV_FORMAT_ACTIVE:
> > +		return &xcsi2rxss->format;
> > +	default:
> > +		return NULL;
> > +	}
> > +}
> > +
> > +/**
> > + * xcsi2rxss_init_cfg - Initialise the pad format config to default
> > + * @sd: Pointer to V4L2 Sub device structure
> > + * @cfg: Pointer to sub device pad information structure
> > + *
> > + * This function is used to initialize the pad format with the
> > +default
> > + * values.
> > + *
> > + * Return: 0 on success
> > + */
> > +static int xcsi2rxss_init_cfg(struct v4l2_subdev *sd,
> > +			      struct v4l2_subdev_pad_config *cfg) {
> > +	struct xcsi2rxss_state *xcsi2rxss = to_xcsi2rxssstate(sd);
> > +	struct v4l2_mbus_framefmt *format;
> > +	unsigned int i;
> > +
> > +	mutex_lock(&xcsi2rxss->lock);
> > +	for (i = 0; i < XCSI_MEDIA_PADS; i++) {
> > +		format = v4l2_subdev_get_try_format(sd, cfg, i);
> > +		*format = xcsi2rxss->default_format;
> > +	}
> > +	mutex_unlock(&xcsi2rxss->lock);
> > +
> > +	return 0;
> > +}
> > +
> > +/**
> > + * xcsi2rxss_get_format - Get the pad format
> > + * @sd: Pointer to V4L2 Sub device structure
> > + * @cfg: Pointer to sub device pad information structure
> > + * @fmt: Pointer to pad level media bus format
> > + *
> > + * This function is used to get the pad format information.
> > + *
> > + * Return: 0 on success
> > + */
> > +static int xcsi2rxss_get_format(struct v4l2_subdev *sd,
> > +				struct v4l2_subdev_pad_config *cfg,
> > +				struct v4l2_subdev_format *fmt)
> > +{
> > +	struct xcsi2rxss_state *xcsi2rxss = to_xcsi2rxssstate(sd);
> > +
> > +	mutex_lock(&xcsi2rxss->lock);
> > +	fmt->format = *__xcsi2rxss_get_pad_format(xcsi2rxss, cfg, fmt->pad,
> > +						  fmt->which);
> > +	mutex_unlock(&xcsi2rxss->lock);
> > +
> > +	return 0;
> > +}
> > +
> > +/**
> > + * xcsi2rxss_set_format - This is used to set the pad format
> > + * @sd: Pointer to V4L2 Sub device structure
> > + * @cfg: Pointer to sub device pad information structure
> > + * @fmt: Pointer to pad level media bus format
> > + *
> > + * This function is used to set the pad format. Since the pad format
> > +is fixed
> > + * in hardware, it can't be modified on run time. So when a format
> > +set is
> > + * requested by application, all parameters except the format type is
> > +saved
> > + * for the pad and the original pad format is sent back to the application.
> > + *
> > + * Return: 0 on success
> > + */
> > +static int xcsi2rxss_set_format(struct v4l2_subdev *sd,
> > +				struct v4l2_subdev_pad_config *cfg,
> > +				struct v4l2_subdev_format *fmt)
> > +{
> > +	struct xcsi2rxss_state *xcsi2rxss = to_xcsi2rxssstate(sd);
> > +	struct v4l2_mbus_framefmt *__format;
> > +	u32 dt;
> > +
> > +	/* only sink pad format can be updated */
> > +	mutex_lock(&xcsi2rxss->lock);
> > +
> > +	/*
> > +	 * Only the format->code parameter matters for CSI as the
> > +	 * CSI format cannot be changed at runtime.
> > +	 * Ensure that format to set is copied to over to CSI pad format
> > +	 */
> > +	__format = __xcsi2rxss_get_pad_format(xcsi2rxss, cfg,
> > +					      fmt->pad, fmt->which);
> > +
> > +	if (fmt->pad == XVIP_PAD_SOURCE) {
> > +		fmt->format = *__format;
> > +		mutex_unlock(&xcsi2rxss->lock);
> > +		return 0;
> > +	}
> > +
> > +	/*
> > +	 * RAW8 is supported in all datatypes. So if requested media bus
> format
> > +	 * is of RAW8 type, then allow to be set. In case core is configured to
> > +	 * other RAW, YUV422 8/10 or RGB888, set appropriate media bus
> format.
> > +	 */
> > +	dt = xcsi2rxss_get_dt(fmt->format.code);
> > +	if (dt != xcsi2rxss->datatype && dt != XCSI_DT_RAW8) {
> > +		dev_dbg(xcsi2rxss->dev, "Unsupported media bus format");
> > +		/* set the default format for the data type */
> > +		fmt->format.code = xcsi2rxss_get_nth_mbus(xcsi2rxss-
> >datatype,
> > +							  0);
> > +	}
> > +
> > +	*__format = fmt->format;
> > +	mutex_unlock(&xcsi2rxss->lock);
> > +
> > +	return 0;
> > +}
> > +
> > +/*
> > + * xcsi2rxss_enum_mbus_code - Handle pixel format enumeration
> > + * @sd : pointer to v4l2 subdev structure
> > + * @cfg: V4L2 subdev pad configuration
> > + * @code : pointer to v4l2_subdev_mbus_code_enum structure
> > + *
> > + * Return: -EINVAL or zero on success  */ int
> > +xcsi2rxss_enum_mbus_code(struct v4l2_subdev *sd,
> > +			     struct v4l2_subdev_pad_config *cfg,
> > +			     struct v4l2_subdev_mbus_code_enum *code)
> 
> As commented by the kbuild bot, this function should be static.

Yes I will update this in next version.

> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Good to go :-)
> 

Great! 😊 

> > +{
> > +	struct xcsi2rxss_state *state = to_xcsi2rxssstate(sd);
> > +	u32 dt, n;
> > +	int ret = 0;
> > +
> > +	/* RAW8 dt packets are available in all DT configurations */
> > +	if (code->index < 4) {
> > +		n = code->index;
> > +		dt = XCSI_DT_RAW8;
> > +	} else if (state->datatype != XCSI_DT_RAW8) {
> > +		n = code->index - 4;
> > +		dt = state->datatype;
> > +	} else {
> > +		return -EINVAL;
> > +	}
> > +
> > +	code->code = xcsi2rxss_get_nth_mbus(dt, n);
> > +	if (!code->code)
> > +		ret = -EINVAL;
> > +
> > +	return ret;
> > +}
> > +
> > +/*
> > +---------------------------------------------------------------------
> > +--------
> > + * Media Operations
> > + */
> > +
> > +static const struct media_entity_operations xcsi2rxss_media_ops = {
> > +	.link_validate = v4l2_subdev_link_validate };
> > +
> > +static const struct v4l2_subdev_core_ops xcsi2rxss_core_ops = {
> > +	.log_status = xcsi2rxss_log_status,
> > +};
> > +
> > +static const struct v4l2_subdev_video_ops xcsi2rxss_video_ops = {
> > +	.s_stream = xcsi2rxss_s_stream
> > +};
> > +
> > +static const struct v4l2_subdev_pad_ops xcsi2rxss_pad_ops = {
> > +	.init_cfg = xcsi2rxss_init_cfg,
> > +	.get_fmt = xcsi2rxss_get_format,
> > +	.set_fmt = xcsi2rxss_set_format,
> > +	.enum_mbus_code = xcsi2rxss_enum_mbus_code,
> > +	.link_validate = v4l2_subdev_link_validate_default,
> > +};
> > +
> > +static const struct v4l2_subdev_ops xcsi2rxss_ops = {
> > +	.core = &xcsi2rxss_core_ops,
> > +	.video = &xcsi2rxss_video_ops,
> > +	.pad = &xcsi2rxss_pad_ops
> > +};
> > +
> > +static int xcsi2rxss_parse_of(struct xcsi2rxss_state *xcsi2rxss) {
> > +	struct device *dev = xcsi2rxss->dev;
> > +	struct device_node *node = dev->of_node;
> > +
> > +	struct fwnode_handle *ep;
> > +	struct v4l2_fwnode_endpoint vep = {
> > +		.bus_type = V4L2_MBUS_CSI2_DPHY
> > +	};
> > +	bool en_csi_v20, vfb;
> > +	int ret;
> > +
> > +	en_csi_v20 = of_property_read_bool(node, "xlnx,en-csi-v2-0");
> > +	if (en_csi_v20)
> > +		xcsi2rxss->en_vcx = of_property_read_bool(node, "xlnx,en-
> vcx");
> > +
> > +	xcsi2rxss->enable_active_lanes =
> > +		of_property_read_bool(node, "xlnx,en-active-lanes");
> > +
> > +	ret = of_property_read_u32(node, "xlnx,csi-pxl-format",
> > +				   &xcsi2rxss->datatype);
> > +	if (ret < 0) {
> > +		dev_err(dev, "missing xlnx,csi-pxl-format property\n");
> > +		return ret;
> > +	}
> > +
> > +	switch (xcsi2rxss->datatype) {
> > +	case XCSI_DT_YUV4228B:
> > +	case XCSI_DT_RGB444:
> > +	case XCSI_DT_RGB555:
> > +	case XCSI_DT_RGB565:
> > +	case XCSI_DT_RGB666:
> > +	case XCSI_DT_RGB888:
> > +	case XCSI_DT_RAW6:
> > +	case XCSI_DT_RAW7:
> > +	case XCSI_DT_RAW8:
> > +	case XCSI_DT_RAW10:
> > +	case XCSI_DT_RAW12:
> > +	case XCSI_DT_RAW14:
> > +		break;
> > +	case XCSI_DT_YUV42210B:
> > +	case XCSI_DT_RAW16:
> > +	case XCSI_DT_RAW20:
> > +		if (!en_csi_v20) {
> > +			ret = -EINVAL;
> > +			dev_dbg(dev, "enable csi v2 for this pixel format");
> > +		}
> > +		break;
> > +	default:
> > +		ret = -EINVAL;
> > +	}
> > +	if (ret < 0) {
> > +		dev_err(dev, "invalid csi-pxl-format property!\n");
> > +		return ret;
> > +	}
> > +
> > +	vfb = of_property_read_bool(node, "xlnx,vfb");
> > +	if (!vfb) {
> > +		dev_err(dev, "operation without VFB is not supported\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev),
> > +					     XVIP_PAD_SINK, 0,
> > +
> FWNODE_GRAPH_ENDPOINT_NEXT);
> > +	if (!ep) {
> > +		dev_err(dev, "no sink port found");
> > +		return -EINVAL;
> > +	}
> > +
> > +	ret = v4l2_fwnode_endpoint_parse(ep, &vep);
> > +	fwnode_handle_put(ep);
> > +	if (ret) {
> > +		dev_err(dev, "error parsing sink port");
> > +		return ret;
> > +	}
> > +
> > +	dev_dbg(dev, "mipi number lanes = %d\n",
> > +		vep.bus.mipi_csi2.num_data_lanes);
> > +
> > +	xcsi2rxss->max_num_lanes = vep.bus.mipi_csi2.num_data_lanes;
> > +
> > +	ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev),
> > +					     XVIP_PAD_SOURCE, 0,
> > +
> FWNODE_GRAPH_ENDPOINT_NEXT);
> > +	if (!ep) {
> > +		dev_err(dev, "no source port found");
> > +		return -EINVAL;
> > +	}
> > +
> > +	fwnode_handle_put(ep);
> > +
> > +	dev_dbg(dev, "vcx %s, %u data lanes (%s), data type 0x%02x\n",
> > +		xcsi2rxss->en_vcx ? "enabled" : "disabled",
> > +		xcsi2rxss->max_num_lanes,
> > +		xcsi2rxss->enable_active_lanes ? "dynamic" : "static",
> > +		xcsi2rxss->datatype);
> > +
> > +	return 0;
> > +}
> > +
> > +static int xcsi2rxss_probe(struct platform_device *pdev) {
> > +	struct v4l2_subdev *subdev;
> > +	struct xcsi2rxss_state *xcsi2rxss;
> > +	int num_clks = ARRAY_SIZE(xcsi2rxss_clks);
> > +	struct device *dev = &pdev->dev;
> > +	int irq, ret;
> > +
> > +	xcsi2rxss = devm_kzalloc(dev, sizeof(*xcsi2rxss), GFP_KERNEL);
> > +	if (!xcsi2rxss)
> > +		return -ENOMEM;
> > +
> > +	xcsi2rxss->dev = dev;
> > +
> > +	xcsi2rxss->clks = devm_kmemdup(dev, xcsi2rxss_clks,
> > +				       sizeof(xcsi2rxss_clks), GFP_KERNEL);
> > +	if (!xcsi2rxss->clks)
> > +		return -ENOMEM;
> > +
> > +	/* Reset GPIO */
> > +	xcsi2rxss->rst_gpio = devm_gpiod_get_optional(dev, "video-reset",
> > +						      GPIOD_OUT_HIGH);
> > +	if (IS_ERR(xcsi2rxss->rst_gpio)) {
> > +		if (PTR_ERR(xcsi2rxss->rst_gpio) != -EPROBE_DEFER)
> > +			dev_err(dev, "Video Reset GPIO not setup in DT");
> > +		return PTR_ERR(xcsi2rxss->rst_gpio);
> > +	}
> > +
> > +	ret = xcsi2rxss_parse_of(xcsi2rxss);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	xcsi2rxss->iomem = devm_platform_ioremap_resource(pdev, 0);
> > +	if (IS_ERR(xcsi2rxss->iomem))
> > +		return PTR_ERR(xcsi2rxss->iomem);
> > +
> > +	irq = platform_get_irq(pdev, 0);
> > +	if (irq < 0)
> > +		return irq;
> > +
> > +	ret = devm_request_threaded_irq(dev, irq, NULL,
> > +					xcsi2rxss_irq_handler,
> IRQF_ONESHOT,
> > +					dev_name(dev), xcsi2rxss);
> > +	if (ret) {
> > +		dev_err(dev, "Err = %d Interrupt handler reg failed!\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	ret = clk_bulk_get(dev, num_clks, xcsi2rxss->clks);
> > +	if (ret)
> > +		return ret;
> > +
> > +	/* TODO: Enable/disable clocks at stream on/off time. */
> > +	ret = clk_bulk_prepare_enable(num_clks, xcsi2rxss->clks);
> > +	if (ret)
> > +		goto err_clk_put;
> > +
> > +	mutex_init(&xcsi2rxss->lock);
> > +
> > +	xcsi2rxss_hard_reset(xcsi2rxss);
> > +	xcsi2rxss_soft_reset(xcsi2rxss);
> > +
> > +	/* Initialize V4L2 subdevice and media entity */
> > +	xcsi2rxss->pads[XVIP_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
> > +	xcsi2rxss->pads[XVIP_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE;
> > +
> > +	/* Initialize the default format */
> > +	xcsi2rxss->default_format.code =
> > +		xcsi2rxss_get_nth_mbus(xcsi2rxss->datatype, 0);
> > +	xcsi2rxss->default_format.field = V4L2_FIELD_NONE;
> > +	xcsi2rxss->default_format.colorspace = V4L2_COLORSPACE_SRGB;
> > +	xcsi2rxss->default_format.width = XCSI_DEFAULT_WIDTH;
> > +	xcsi2rxss->default_format.height = XCSI_DEFAULT_HEIGHT;
> > +	xcsi2rxss->format = xcsi2rxss->default_format;
> > +
> > +	/* Initialize V4L2 subdevice and media entity */
> > +	subdev = &xcsi2rxss->subdev;
> > +	v4l2_subdev_init(subdev, &xcsi2rxss_ops);
> > +	subdev->dev = dev;
> > +	strscpy(subdev->name, dev_name(dev), sizeof(subdev->name));
> > +	subdev->flags |= V4L2_SUBDEV_FL_HAS_EVENTS |
> V4L2_SUBDEV_FL_HAS_DEVNODE;
> > +	subdev->entity.ops = &xcsi2rxss_media_ops;
> > +	v4l2_set_subdevdata(subdev, xcsi2rxss);
> > +
> > +	ret = media_entity_pads_init(&subdev->entity, XCSI_MEDIA_PADS,
> > +				     xcsi2rxss->pads);
> > +	if (ret < 0)
> > +		goto error;
> > +
> > +	platform_set_drvdata(pdev, xcsi2rxss);
> > +
> > +	ret = v4l2_async_register_subdev(subdev);
> > +	if (ret < 0) {
> > +		dev_err(dev, "failed to register subdev\n");
> > +		goto error;
> > +	}
> > +
> > +	return 0;
> > +error:
> > +	media_entity_cleanup(&subdev->entity);
> > +	mutex_destroy(&xcsi2rxss->lock);
> > +	clk_bulk_disable_unprepare(num_clks, xcsi2rxss->clks);
> > +err_clk_put:
> > +	clk_bulk_put(num_clks, xcsi2rxss->clks);
> > +	return ret;
> > +}
> > +
> > +static int xcsi2rxss_remove(struct platform_device *pdev) {
> > +	struct xcsi2rxss_state *xcsi2rxss = platform_get_drvdata(pdev);
> > +	struct v4l2_subdev *subdev = &xcsi2rxss->subdev;
> > +	int num_clks = ARRAY_SIZE(xcsi2rxss_clks);
> > +
> > +	v4l2_async_unregister_subdev(subdev);
> > +	media_entity_cleanup(&subdev->entity);
> > +	mutex_destroy(&xcsi2rxss->lock);
> > +	clk_bulk_disable_unprepare(num_clks, xcsi2rxss->clks);
> > +	clk_bulk_put(num_clks, xcsi2rxss->clks);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct of_device_id xcsi2rxss_of_id_table[] = {
> > +	{ .compatible = "xlnx,mipi-csi2-rx-subsystem-5.0", },
> > +	{ }
> > +};
> > +MODULE_DEVICE_TABLE(of, xcsi2rxss_of_id_table);
> > +
> > +static struct platform_driver xcsi2rxss_driver = {
> > +	.driver = {
> > +		.name		= "xilinx-csi2rxss",
> > +		.of_match_table	= xcsi2rxss_of_id_table,
> > +	},
> > +	.probe			= xcsi2rxss_probe,
> > +	.remove			= xcsi2rxss_remove,
> > +};
> > +
> > +module_platform_driver(xcsi2rxss_driver);
> > +
> > +MODULE_AUTHOR("Vishal Sagar <vsagar@xilinx.com>");
> > +MODULE_DESCRIPTION("Xilinx MIPI CSI-2 Rx Subsystem Driver");
> > +MODULE_LICENSE("GPL v2");
> 
> --
> Regards,
> 
> Laurent Pinchart

Regards
Vishal Sagar



^ permalink raw reply

* Re: [PATCH 1/2] dt-bindings: ASoC: renesas,rsnd: Add r8a7742 support
From: Mark Brown @ 2020-05-27 11:54 UTC (permalink / raw)
  To: Lad, Prabhakar
  Cc: Lad Prabhakar, Geert Uytterhoeven, Magnus Damm, Rob Herring,
	Liam Girdwood, alsa-devel, Linux-Renesas,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML
In-Reply-To: <CA+V-a8v0i71MCTNTPVD3XHuyGZiVjzuCkCUnvoUczeMr416ouQ@mail.gmail.com>

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

On Wed, May 27, 2020 at 12:32:28PM +0100, Lad, Prabhakar wrote:
> On Wed, May 27, 2020 at 12:25 PM Mark Brown <broonie@kernel.org> wrote:

> > > +                                 - "renesas,rcar_sound-r8a7742" (RZ/G1H)

> > I'd expect a matching patch adding this compatible to the driver.

> The Renesas R-Car sound for RZ/G1H is identical to the R-Car Gen2
> family. So no driver change is needed and  the fallback compatible
> value "renesas,rcar_sound-gen2" will be used in the SOC DT.

Then the patch to add the compatible string will be trivial.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* RE: [PATCH v13 1/2] media: dt-bindings: media: xilinx: Add Xilinx MIPI CSI-2 Rx Subsystem
From: Vishal Sagar @ 2020-05-27 11:53 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Hyun Kwon, mchehab@kernel.org, robh+dt@kernel.org,
	mark.rutland@arm.com, Michal Simek, linux-media@vger.kernel.org,
	devicetree@vger.kernel.org, hans.verkuil@cisco.com,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, Dinesh Kumar, Sandip Kothari,
	Luca Ceresoli, Jacopo Mondi, Hyun Kwon, Rob Herring
In-Reply-To: <20200524020214.GB6026@pendragon.ideasonboard.com>

Hi Laurent,

Thanks for reviewing this series.

> -----Original Message-----
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: Sunday, May 24, 2020 7:32 AM
> To: Vishal Sagar <vsagar@xilinx.com>
> Cc: Hyun Kwon <hyunk@xilinx.com>; mchehab@kernel.org;
> robh+dt@kernel.org; mark.rutland@arm.com; Michal Simek
> <michals@xilinx.com>; linux-media@vger.kernel.org;
> devicetree@vger.kernel.org; hans.verkuil@cisco.com; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Dinesh Kumar
> <dineshk@xilinx.com>; Sandip Kothari <sandipk@xilinx.com>; Luca Ceresoli
> <luca@lucaceresoli.net>; Jacopo Mondi <jacopo@jmondi.org>; Hyun Kwon
> <hyunk@xilinx.com>; Rob Herring <robh@kernel.org>
> Subject: Re: [PATCH v13 1/2] media: dt-bindings: media: xilinx: Add Xilinx MIPI
> CSI-2 Rx Subsystem
> 
> Hi Vishal,
> 
> Thank you for the patch.
> 
> On Tue, May 12, 2020 at 08:49:46PM +0530, Vishal Sagar wrote:
> > Add bindings documentation for Xilinx MIPI CSI-2 Rx Subsystem.
> >
> > The Xilinx MIPI CSI-2 Rx Subsystem consists of a CSI-2 Rx controller,
> > a D-PHY in Rx mode and a Video Format Bridge.
> >
> > Signed-off-by: Vishal Sagar <vishal.sagar@xilinx.com>
> > Reviewed-by: Hyun Kwon <hyun.kwon@xilinx.com>
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > v13
> > - Based on Laurent's suggestions
> > - Fixed the datatypes values as minimum and maximum
> > - condition added for en-vcx property
> >
> > v12
> > - Moved to yaml format
> > - Update CSI-2 and D-PHY
> > - Mention that bindings for D-PHY not here
> > - reset -> video-reset
> >
> > v11
> > - Modify compatible string from 4.0 to 5.0
> >
> > v10
> > - No changes
> >
> > v9
> > - Fix xlnx,vfb description.
> > - s/Optional/Required endpoint property.
> > - Move data-lanes description from Ports to endpoint property section.
> >
> > v8
> > - Added reset-gpios optional property to assert video_aresetn
> >
> > v7
> > - Removed the control name from dt bindings
> > - Updated the example dt node name to csi2rx
> >
> > v6
> > - Added "control" after V4L2_CID_XILINX_MIPICSISS_ACT_LANES as
> > suggested by Luca
> > - Added reviewed by Rob Herring
> >
> > v5
> > - Incorporated comments by Luca Cersoli
> > - Removed DPHY clock from description and example
> > - Removed bayer pattern from device tree MIPI CSI IP
> >   doesn't deal with bayer pattern.
> >
> > v4
> > - Added reviewed by Hyun Kwon
> >
> > v3
> > - removed interrupt parent as suggested by Rob
> > - removed dphy clock
> > - moved vfb to optional properties
> > - Added required and optional port properties section
> > - Added endpoint property section
> >
> > v2
> > - updated the compatible string to latest version supported
> > - removed DPHY related parameters
> > - added CSI v2.0 related property (including VCX for supporting upto 16
> >   virtual channels).
> > - modified csi-pxl-format from string to unsigned int type where the value
> >   is as per the CSI specification
> > - Defined port 0 and port 1 as sink and source ports.
> > - Removed max-lanes property as suggested by Rob and Sakari
> > .../bindings/media/xilinx/xlnx,csi2rxss.yaml  | 226 ++++++++++++++++++
> >  1 file changed, 226 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/media/xilinx/xlnx,csi2rxss.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/media/xilinx/xlnx,csi2rxss.yaml
> > b/Documentation/devicetree/bindings/media/xilinx/xlnx,csi2rxss.yaml
> > new file mode 100644
> > index 000000000000..b0885f461785
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/xilinx/xlnx,csi2rxss.yam
> > +++ l
> > @@ -0,0 +1,226 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/xilinx/xlnx,csi2rxss.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Xilinx MIPI CSI-2 Receiver Subsystem
> > +
> > +maintainers:
> > +  - Vishal Sagar <vishal.sagar@xilinx.com>
> > +
> > +description: |
> > +  The Xilinx MIPI CSI-2 Receiver Subsystem is used to capture MIPI
> > +CSI-2
> > +  traffic from compliant camera sensors and send the output as AXI4
> > +Stream
> > +  video data for image processing.
> > +  The subsystem consists of a MIPI D-PHY in slave mode which captures
> > +the
> > +  data packets. This is passed along the MIPI CSI-2 Rx IP which
> > +extracts the
> > +  packet data. The optional Video Format Bridge (VFB) converts this
> > +data to
> > +  AXI4 Stream video data.
> > +  For more details, please refer to PG232 Xilinx MIPI CSI-2 Receiver
> Subsystem.
> > +  Please note that this bindings includes only the MIPI CSI-2 Rx
> > +controller
> > +  and Video Format Bridge and not D-PHY.
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - enum:
> > +        - xlnx,mipi-csi2-rx-subsystem-5.0
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    description: List of clock specifiers
> > +    items:
> > +      - description: AXI Lite clock
> > +      - description: Video clock
> > +
> > +  clock-names:
> > +    items:
> > +      - const: lite_aclk
> > +      - const: video_aclk
> > +
> > +  xlnx,csi-pxl-format:
> > +    description: |
> > +      This denotes the CSI Data type selected in hw design.
> > +      Packets other than this data type (except for RAW8 and
> > +      User defined data types) will be filtered out.
> > +      Possible values are as below -
> > +      0x1e - YUV4228B
> > +      0x1f - YUV42210B
> > +      0x20 - RGB444
> > +      0x21 - RGB555
> > +      0x22 - RGB565
> > +      0x23 - RGB666
> > +      0x24 - RGB888
> > +      0x28 - RAW6
> > +      0x29 - RAW7
> > +      0x2a - RAW8
> > +      0x2b - RAW10
> > +      0x2c - RAW12
> > +      0x2d - RAW14
> > +      0x2e - RAW16
> > +      0x2f - RAW20
> > +    allOf:
> > +      - $ref: /schemas/types.yaml#/definitions/uint32
> > +      - anyOf:
> > +        - minimum: 0x1e
> > +        - maximum: 0x24
> > +        - minimum: 0x28
> > +        - maximum: 0x2f
> > +
> > +  xlnx,vfb:
> > +    type: boolean
> > +    description: Present when Video Format Bridge is enabled in IP
> > + configuration
> > +
> > +  xlnx,en-csi-v2-0:
> > +    type: boolean
> > +    description: Present if CSI v2 is enabled in IP configuration.
> > +
> > +  xlnx,en-vcx:
> > +    type: boolean
> > +    description: |
> > +      When present, there are maximum 16 virtual channels, else only 4.
> > +
> > +  xlnx,en-active-lanes:
> > +    type: boolean
> > +    description: |
> > +      Present if the number of active lanes can be re-configured at
> > +      runtime in the Protocol Configuration Register. Otherwise all lanes,
> > +      as set in IP configuration, are always active.
> > +
> > +  video-reset-gpios:
> > +    description: Optional specifier for a GPIO that asserts video_aresetn.
> > +    maxItems: 1
> > +
> > +  ports:
> > +    type: object
> > +
> > +    properties:
> > +      port@0:
> > +        type: object
> > +        description: |
> > +          Input / sink port node, single endpoint describing the
> > +          CSI-2 transmitter.
> > +
> > +        properties:
> > +          reg:
> > +            const: 0
> > +
> > +          endpoint:
> > +            type: object
> > +
> > +            properties:
> > +
> > +              data-lanes:
> > +                description: |
> > +                  This is required only in the sink port 0 endpoint which
> > +                  connects to MIPI CSI-2 source like sensor.
> > +                  The possible values are -
> > +                  1       - For 1 lane enabled in IP.
> > +                  1 2     - For 2 lanes enabled in IP.
> > +                  1 2 3   - For 3 lanes enabled in IP.
> > +                  1 2 3 4 - For 4 lanes enabled in IP.
> > +                items:
> > +                  - const: 1
> > +                  - const: 2
> > +                  - const: 3
> > +                  - const: 4
> > +
> > +              remote-endpoint: true
> > +
> > +            required:
> > +              - data-lanes
> > +              - remote-endpoint
> > +
> > +            additionalProperties: false
> > +
> > +        additionalProperties: false
> > +
> > +      port@1:
> > +        type: object
> > +        description: |
> > +          Output / source port node, endpoint describing modules
> > +          connected the CSI-2 receiver.
> > +
> > +        properties:
> > +
> > +          reg:
> > +            const: 1
> > +
> > +          endpoint:
> > +            type: object
> > +
> > +            properties:
> > +
> > +              remote-endpoint: true
> > +
> > +            required:
> > +              - remote-endpoint
> > +
> > +            additionalProperties: false
> > +
> > +        additionalProperties: false
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - clocks
> > +  - clock-names
> > +  - xlnx,csi-pxl-format
> > +  - ports
> > +
> > +if:
> > +  not:
> > +    required:
> > +      - xlnx,en-csi-v2-0
> > +then:
> > +  properties:
> > +    xlnx,en-vcx: false
> 
> As I've just commented on v12, I think we should condition the xlnx,csi-pxl-
> format property to xlnx,vfb being set. xlnx,csi-pxl-format should be removed
> from the required properties above, and the following conditions added:
> 
> allOf:
>   - if:
>       required:
>         - xlnx,vfb
>     then:
>       required:
>         - xlnx,csi-pxl-format
>     else:
>       properties:
>         xlnx,csi-pxl-format: false
> 
>   - if:
>       not:
>         required:
>           - xlnx,en-csi-v2-0
>     then:
>       properties:
>         xlnx,en-vcx: false
> 
> The 'allOf' is needed as you can't have two 'if' constructs at the top level.
> 
Thanks for sharing the explanation for this.
Can you please share where I can get this info?

> Please however let me know if my understanding is wrong and xlnx,csi-pxl-
> format is needed even when xlnx,vfb is not set. In that case please ignore this
> change (but please add the ... below).
> 

Ok. I will add ... in the end.

> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/gpio/gpio.h>
> > +    xcsi2rxss_1: csi2rx@a0020000 {
> > +        compatible = "xlnx,mipi-csi2-rx-subsystem-5.0";
> > +        reg = <0x0 0xa0020000 0x0 0x10000>;
> > +        interrupt-parent = <&gic>;
> > +        interrupts = <0 95 4>;
> > +        xlnx,csi-pxl-format = <0x2a>;
> > +        xlnx,vfb;
> > +        xlnx,en-active-lanes;
> > +        xlnx,en-csi-v2-0;
> > +        xlnx,en-vcx;
> > +        clock-names = "lite_aclk", "video_aclk";
> > +        clocks = <&misc_clk_0>, <&misc_clk_1>;
> > +        video-reset-gpios = <&gpio 86 GPIO_ACTIVE_LOW>;
> > +
> > +        ports {
> > +            #address-cells = <1>;
> > +            #size-cells = <0>;
> > +
> > +            port@0 {
> > +                /* Sink port */
> > +                reg = <0>;
> > +                csiss_in: endpoint {
> > +                    data-lanes = <1 2 3 4>;
> > +                    /* MIPI CSI-2 Camera handle */
> > +                    remote-endpoint = <&camera_out>;
> > +                };
> > +            };
> > +            port@1 {
> > +                /* Source port */
> > +                reg = <1>;
> > +                csiss_out: endpoint {
> > +                    remote-endpoint = <&vproc_in>;
> > +                };
> > +            };
> > +        };
> > +    };
> 
> YAML files usually end with
> 
> ...
> 
> on the last line to mark the end of file.
> 

Ok I will add this to the end of the file.

> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> --
> Regards,
> 
> Laurent Pinchart

Regards
Vishal Sagar


^ permalink raw reply

* Re: [PATCH v5 06/11] net: ethernet: mtk-star-emac: new driver
From: Bartosz Golaszewski @ 2020-05-27 11:48 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Nathan Chancellor, Rob Herring, David S . Miller,
	Matthias Brugger, John Crispin, Sean Wang, Mark Lee,
	Jakub Kicinski, Fabien Parent, Heiner Kallweit, Edwin Peer,
	devicetree, Linux Kernel Mailing List, netdev, Linux ARM,
	moderated list:ARM/Mediatek SoC..., Stephane Le Provost,
	Pedro Tsai, Andrew Perepech, Bartosz Golaszewski,
	clang-built-linux
In-Reply-To: <CAK8P3a3WXGZpeX0E8Kyuo5Rkv5acdkZN6_HNS61Y1=Jh+G+pRQ@mail.gmail.com>

śr., 27 maj 2020 o 13:33 Arnd Bergmann <arnd@arndb.de> napisał(a):
>
> On Wed, May 27, 2020 at 10:46 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> > > I don't know if there should be a new label that excludes that
> > > assignment for those particular gotos or if new_dma_addr should
> > > be initialized to something at the top. Please take a look at
> > > addressing this when you get a chance.
> > >
> > > Cheers,
> > > Nathan
> >
> > Hi Nathan,
> >
> > Thanks for reporting this! I have a fix ready and will send it shortly.
>
> I already have a workaround for this bug as well as another one
> in my tree that I'll send later today after some more testing.
>
> Feel free to wait for that, or just ignore mine if you already have a fix.
>
>        Arnd

Hi Arnd!

I already posted a fix[1]. Sorry for omitting you, but somehow your
name didn't pop up in get_maintainers.pl.

Bartosz

[1] https://lkml.org/lkml/2020/5/27/378

^ permalink raw reply

* RE: [PATCH v12 1/2] media: dt-bindings: media: xilinx: Add Xilinx MIPI CSI-2 Rx Subsystem
From: Vishal Sagar @ 2020-05-27 11:40 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Hyun Kwon, mchehab@kernel.org, robh+dt@kernel.org,
	mark.rutland@arm.com, hans.verkuil@cisco.com, Luca Ceresoli,
	Jacopo Mondi, Michal Simek, linux-media@vger.kernel.org,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, Dinesh Kumar, Sandip Kothari,
	Rob Herring
In-Reply-To: <20200524014548.GA6026@pendragon.ideasonboard.com>

Hi Laurent,

> -----Original Message-----
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: Sunday, May 24, 2020 7:16 AM
> To: Vishal Sagar <vsagar@xilinx.com>
> Cc: Hyun Kwon <hyunk@xilinx.com>; mchehab@kernel.org;
> robh+dt@kernel.org; mark.rutland@arm.com; hans.verkuil@cisco.com; Luca
> Ceresoli <luca@lucaceresoli.net>; Jacopo Mondi <jacopo@jmondi.org>;
> Michal Simek <michals@xilinx.com>; linux-media@vger.kernel.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; Dinesh Kumar <dineshk@xilinx.com>; Sandip Kothari
> <sandipk@xilinx.com>; Rob Herring <robh@kernel.org>
> Subject: Re: [PATCH v12 1/2] media: dt-bindings: media: xilinx: Add Xilinx MIPI
> CSI-2 Rx Subsystem
> 
> Hi Vishal,
> 
> On Fri, May 08, 2020 at 01:52:36PM +0000, Vishal Sagar wrote:
> > On Tuesday, May 5, 2020 7:53 PM, Laurent Pinchart wrote:
> > > On Thu, Apr 23, 2020 at 09:00:37PM +0530, Vishal Sagar wrote:
> > > > Add bindings documentation for Xilinx MIPI CSI-2 Rx Subsystem.
> > > >
> > > > The Xilinx MIPI CSI-2 Rx Subsystem consists of a CSI-2 Rx
> > > > controller, a D-PHY in Rx mode and a Video Format Bridge.
> > > >
> > > > Signed-off-by: Vishal Sagar <vishal.sagar@xilinx.com>
> > > > Reviewed-by: Hyun Kwon <hyun.kwon@xilinx.com>
> > > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > > Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net>
> > > > ---
> > > > v12
> > > > - Moved to yaml format
> > > > - Update CSI-2 and D-PHY
> > > > - Mention that bindings for D-PHY not here
> > > > - reset -> video-reset
> > > >
> > > > v11
> > > > - Modify compatible string from 4.0 to 5.0
> > > >
> > > > v10
> > > > - No changes
> > > >
> > > > v9
> > > > - Fix xlnx,vfb description.
> > > > - s/Optional/Required endpoint property.
> > > > - Move data-lanes description from Ports to endpoint property section.
> > > >
> > > > v8
> > > > - Added reset-gpios optional property to assert video_aresetn
> > > >
> > > > v7
> > > > - Removed the control name from dt bindings
> > > > - Updated the example dt node name to csi2rx
> > > >
> > > > v6
> > > > - Added "control" after V4L2_CID_XILINX_MIPICSISS_ACT_LANES as
> > > > suggested by Luca
> > > > - Added reviewed by Rob Herring
> > > >
> > > > v5
> > > > - Incorporated comments by Luca Cersoli
> > > > - Removed DPHY clock from description and example
> > > > - Removed bayer pattern from device tree MIPI CSI IP
> > > >   doesn't deal with bayer pattern.
> > > >
> > > > v4
> > > > - Added reviewed by Hyun Kwon
> > > >
> > > > v3
> > > > - removed interrupt parent as suggested by Rob
> > > > - removed dphy clock
> > > > - moved vfb to optional properties
> > > > - Added required and optional port properties section
> > > > - Added endpoint property section
> > > >
> > > > v2
> > > > - updated the compatible string to latest version supported
> > > > - removed DPHY related parameters
> > > > - added CSI v2.0 related property (including VCX for supporting upto 16
> > > >   virtual channels).
> > > > - modified csi-pxl-format from string to unsigned int type where the value
> > > >   is as per the CSI specification
> > > > - Defined port 0 and port 1 as sink and source ports.
> > > > - Removed max-lanes property as suggested by Rob and Sakari
> > > > .../bindings/media/xilinx/xlnx,csi2rxss.yaml  | 215
> > > > ++++++++++++++++++
> > > >  1 file changed, 215 insertions(+)  create mode 100644
> > > > Documentation/devicetree/bindings/media/xilinx/xlnx,csi2rxss.yaml
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/media/xilinx/xlnx,csi2rxss.yam
> > > > l
> > > > b/Documentation/devicetree/bindings/media/xilinx/xlnx,csi2rxss.yam
> > > > l
> > > > new file mode 100644
> > > > index 000000000000..365084e27f7e
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/media/xilinx/xlnx,csi2rxss
> > > > +++ .yam
> > > > +++ l
> > > > @@ -0,0 +1,215 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML
> > > > +1.2
> > > > +---
> > > > +$id:
> > > > +http://devicetree.org/schemas/media/xilinx/xlnx,csi2rxss.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Xilinx MIPI CSI-2 Receiver Subsystem
> > > > +
> > > > +maintainers:
> > > > +  - Vishal Sagar <vishal.sagar@xilinx.com>
> > > > +
> > > > +description: |
> > > > +  The Xilinx MIPI CSI-2 Receiver Subsystem is used to capture
> > > > +MIPI CSI-2
> > > > +  traffic from compliant camera sensors and send the output as
> > > > +AXI4 Stream
> > > > +  video data for image processing.
> > > > +  The subsystem consists of a MIPI D-PHY in slave mode which
> > > > +captures the
> > > > +  data packets. This is passed along the MIPI CSI-2 Rx IP which
> > > > +extracts the
> > > > +  packet data. The optional Video Format Bridge (VFB) converts
> > > > +this data to
> > > > +  AXI4 Stream video data.
> > > > +  For more details, please refer to PG232 Xilinx MIPI CSI-2 Receiver
> Subsystem.
> > > > +  Please note that this bindings includes only the MIPI CSI-2 Rx
> > > > +controller
> > > > +  and Video Format Bridge and not D-PHY.
> > >
> > > How should the D-PHY be handled, when DPY_EN_REG_IF is set to true ?
> >
> > It was suggested in v3 to have a separate D-PHY phy driver which would be
> initialized / configured from MIPI CSI-2 Rx driver.
> > Currently with the D-PHY register interface enabled, we don't have to
> > really configure anything but the following parameters
> > 1 - init
> > 2 - hs_settle
> > 3 - hs_timeout (High Speed mode timeout)
> > 4 - esc_timeout  (Escape mode timeout)
> > 5 - idelay tap
> >
> > The D-PHY Rx can be enabled/disabled and has a soft reset bit.
> > The HS and Escape mode timeout registers can be enabled with a default
> value via the Vivado IP GUI.
> >
> > The above parameters would have to be passed to D-PHY driver as custom
> > control via MIPI CSI-2 Rx driver as these depend on the sensor connected.
> > Probably these can be a new common control in V4L2 framework for CSI Rx
> controllers.
> 
> It seems that in that case, regardless of whether we go for a separate PHY
> driver or not, we will be able to extend the DT bindings, either with a phys
> property, or with an additional reg entry. That should be backward-
> compatible, so I'm not concerned.
> 
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    items:
> > > > +      - enum:
> > > > +        - xlnx,mipi-csi2-rx-subsystem-5.0
> > > > +
> > > > +  reg:
> > > > +    maxItems: 1
> > > > +
> > > > +  interrupts:
> > > > +    maxItems: 1
> > > > +
> > > > +  clocks:
> > > > +    description: List of clock specifiers
> > > > +    items:
> > > > +      - description: AXI Lite clock
> > > > +      - description: Video clock
> > > > +
> > > > +  clock-names:
> > > > +    items:
> > > > +      - const: lite_aclk
> > > > +      - const: video_aclk
> > > > +
> > > > +  xlnx,csi-pxl-format:
> > > > +    description: |
> > > > +      This denotes the CSI Data type selected in hw design.
> > > > +      Packets other than this data type (except for RAW8 and
> > > > +      User defined data types) will be filtered out.
> > > > +      Possible values are as below -
> > > > +      0x1e - YUV4228B
> > > > +      0x1f - YUV42210B
> > > > +      0x20 - RGB444
> > > > +      0x21 - RGB555
> > > > +      0x22 - RGB565
> > > > +      0x23 - RGB666
> > > > +      0x24 - RGB888
> > > > +      0x28 - RAW6
> > > > +      0x29 - RAW7
> > > > +      0x2a - RAW8
> > > > +      0x2b - RAW10
> > > > +      0x2c - RAW12
> > > > +      0x2d - RAW14
> > > > +      0x2e - RAW16
> > > > +      0x2f - RAW20
> > > > +    allOf:
> > > > +      - $ref: /schemas/types.yaml#/definitions/uint32
> > > > +      - enum: [0x1e, 0x1f, 0x20, 0x21, 0x22, 0x23, 0x24, 0x28,
> > > > + 0x29, 0x2a, 0x2b, 0x2c, 0x2d, 0x2e, 0x2f]
> > >
> > > This could also be written
> > >
> > >     allOf:
> > >       - $ref: /schemas/types.yaml#/definitions/uint32
> > >       - anyOf:
> > >         - minimum: 0x1e
> > >           maximum: 0x24
> > >         - minimum: 0x28
> > >           maximum: 0x2f
> > >
> > > if you want to make it a bit more compact (in the number of values,
> > > not the number of lines obviously), up to you.
> >
> > Thanks for sharing this method. I will update this in next version.
> >
> > > I will also quote the question (and your answer) from the previous
> > > version:
> > >
> > > > > Isn't this property required only when the VFB is present ?
> > > >
> > > > This will be present irrespective of VFB being enabled.
> > > > With VFB, the data on the bus will be as per Xilinx UG934 which is similar
> to media bus formats.
> > > >
> > > > Without VFB, it will just be plain data as it comes in data packets.
> > > > Refer to the Xilinx PG 232 "Pixel Packing When Video Format Bridge is Not
> Present"
> > > >
> > > > So the driver is currently made to load only in case VFB is enabled.
> > >
> > > I understand that the driver doesn't support the case where the VFB
> > > is disabled, but the DT bindings shouldn't care about that. The
> > > document of
> > > v4.1 of the subsystem states that the width of the video_out port is
> > > then selected under "CSI-2 Options TDATA width" (page 11). I however
> > > don't such such an option described on pages 55 or 56, but there's
> > > an AXIS_TDATA_WIDTH parameter on page 61.
> > >
> > > Is the pixel format relevant when the VFB is disabled ?
> >
> > When the VFB is disabled, all supported data types will be allowed i.e. no
> filtering will occur.
> > But the data output will be in 32/64 bit TDATA width (no dependence on
> > pixels per clock) and will adhere to the "Recommended Memory Storage"
> section of the CSI spec.
> > In this case data being sent on the bus won't compare to what media bus
> format documentation describes.
> 
> My understanding is that, in the case xlnx,vfb would be absent (not supported
> in the driver yet), the xlnx,csi-pxl-format should not be set ? If that's correct, I'd
> like to capture that in the bindings already.

That is correct. xlnx,csi-pxl-format is absent when xlnx,vfb is absent.

> It could be expressed by dropping xlnx,csi-pxl-format from the required
> section, and adding
> 
> if:
>   required:
>     - xlnx,vfb
> then:
>   required:
>     - xlnx,csi-pxl-format
> else:
>   properties:
>     xlnx,csi-pxl-format: false
> 
> As you will have two conditions defined, they should be grouped with
> 
> allOf:
>   - if:
>       required:
>         - xlnx,vfb
>     then:
>       required:
>         - xlnx,csi-pxl-format
>     else:
>       properties:
>         xlnx,csi-pxl-format: false
> 
>   - if:
>       not:
>         required:
>           - xlnx,en-csi-v2-0
>     then:
>       properties:
>         xlnx,en-vcx: false
> 

Thanks for sharing on how to add this condition in YAML.
I will update this in the next version.

> > > > +
> > > > +  xlnx,vfb:
> > > > +    type: boolean
> > > > +    description: Present when Video Format Bridge is enabled in
> > > > + IP configuration
> > > > +
> > > > +  xlnx,en-csi-v2-0:
> > > > +    type: boolean
> > > > +    description: Present if CSI v2 is enabled in IP configuration.
> > > > +
> > > > +  xlnx,en-vcx:
> > > > +    type: boolean
> > > > +    description: |
> > > > +      When present, there are maximum 16 virtual channels, else
> > > > +      only 4. This is present only if xlnx,en-csi-v2-0 is present.
> > >
> > > The last sentence should be removed, and replaced with
> > >
> > > if:
> > >   not:
> > >     required:
> > >       - xlnx,en-csi-v2-0
> > > then:
> > >   properties:
> > >     xlnx,en-vcx: false
> > >
> > > (to be placed after required: and before additionalProperties:).
> >
> > Got it. Thanks for the tips on YAML.
> > I will update this in next version.
> >
> > > > +
> > > > +  xlnx,en-active-lanes:
> > > > +    type: boolean
> > > > +    description: |
> > > > +      Present if the number of active lanes can be re-configured at
> > > > +      runtime in the Protocol Configuration Register. Otherwise all lanes,
> > > > +      as set in IP configuration, are always active.
> > > > +
> > > > +  video-reset-gpios:
> > > > +    description: Optional specifier for a GPIO that asserts video_aresetn.
> > > > +    maxItems: 1
> > > > +
> > > > +  ports:
> > > > +    type: object
> > > > +
> > > > +    properties:
> > > > +      port@0:
> > > > +        type: object
> > > > +        description: |
> > > > +          Input / sink port node, single endpoint describing the
> > > > +          CSI-2 transmitter.
> > > > +
> > > > +        properties:
> > > > +          reg:
> > > > +            const: 0
> > > > +
> > > > +          endpoint:
> > > > +            type: object
> > > > +
> > > > +            properties:
> > > > +
> > > > +              data-lanes:
> > > > +                description: |
> > > > +                  This is required only in the sink port 0 endpoint which
> > > > +                  connects to MIPI CSI-2 source like sensor.
> > > > +                  The possible values are -
> > > > +                  1       - For 1 lane enabled in IP.
> > > > +                  1 2     - For 2 lanes enabled in IP.
> > > > +                  1 2 3   - For 3 lanes enabled in IP.
> > > > +                  1 2 3 4 - For 4 lanes enabled in IP.
> > > > +                items:
> > > > +                  - const: 1
> > > > +                  - const: 2
> > > > +                  - const: 3
> > > > +                  - const: 4
> > > > +
> > > > +              remote-endpoint: true
> > > > +
> > > > +            required:
> > > > +              - data-lanes
> > > > +              - remote-endpoint
> > > > +
> > > > +            additionalProperties: false
> > > > +
> > > > +        additionalProperties: false
> > > > +
> > > > +      port@1:
> > > > +        type: object
> > > > +        description: |
> > > > +          Output / source port node, endpoint describing modules
> > > > +          connected the CSI-2 receiver.
> > > > +
> > > > +        properties:
> > > > +
> > > > +          reg:
> > > > +            const: 1
> > > > +
> > > > +          endpoint:
> > > > +            type: object
> > > > +
> > > > +            properties:
> > > > +
> > > > +              remote-endpoint: true
> > > > +
> > > > +            required:
> > > > +              - remote-endpoint
> > > > +
> > > > +            additionalProperties: false
> > > > +
> > > > +        additionalProperties: false
> > > > +
> > > > +required:
> > > > +  - compatible
> > > > +  - reg
> > > > +  - interrupts
> > > > +  - clocks
> > > > +  - clock-names
> > > > +  - xlnx,csi-pxl-format
> > > > +  - ports
> > > > +
> > > > +additionalProperties: false
> > > > +
> > > > +examples:
> > > > +  - |
> > > > +    #include <dt-bindings/gpio/gpio.h>
> > > > +    xcsi2rxss_1: csi2rx@a0020000 {
> > > > +        compatible = "xlnx,mipi-csi2-rx-subsystem-5.0";
> > > > +        reg = <0x0 0xa0020000 0x0 0x10000>;
> > > > +        interrupt-parent = <&gic>;
> > > > +        interrupts = <0 95 4>;
> > > > +        xlnx,csi-pxl-format = <0x2a>;
> > > > +        xlnx,vfb;
> > > > +        xlnx,en-active-lanes;
> > > > +        xlnx,en-csi-v2-0;
> > > > +        xlnx,en-vcx;
> > > > +        clock-names = "lite_aclk", "video_aclk";
> > > > +        clocks = <&misc_clk_0>, <&misc_clk_1>;
> > > > +        video-reset-gpios = <&gpio 86 GPIO_ACTIVE_LOW>;
> > > > +
> > > > +        ports {
> > > > +            #address-cells = <1>;
> > > > +            #size-cells = <0>;
> > > > +
> > > > +            port@0 {
> > > > +                /* Sink port */
> > > > +                reg = <0>;
> > > > +                csiss_in: endpoint {
> > > > +                    data-lanes = <1 2 3 4>;
> > > > +                    /* MIPI CSI-2 Camera handle */
> > > > +                    remote-endpoint = <&camera_out>;
> > > > +                };
> > > > +            };
> > > > +            port@1 {
> > > > +                /* Source port */
> > > > +                reg = <1>;
> > > > +                csiss_out: endpoint {
> > > > +                    remote-endpoint = <&vproc_in>;
> > > > +                };
> > > > +            };
> > > > +        };
> > > > +    };
> 
> --
> Regards,
> 
> Laurent Pinchart

Regards
Vishal Sagar


^ permalink raw reply

* Re: [PATCH v11 0/5] Loongson Generic PCI v11
From: Thomas Bogendoerfer @ 2020-05-27 11:35 UTC (permalink / raw)
  To: Jiaxun Yang
  Cc: linux-pci, Bjorn Helgaas, Rob Herring, Huacai Chen,
	Lorenzo Pieralisi, Paul Burton, devicetree, linux-kernel,
	linux-mips
In-Reply-To: <20200526092130.145550-1-jiaxun.yang@flygoat.com>

On Tue, May 26, 2020 at 05:21:11PM +0800, Jiaxun Yang wrote:
> v11 fixes a minor style issue in patch 2.
> 
> Now it have got enough ack,
> Thomas, cloud you please apply it to mips-next?
> 
> Thanks.
> 
> Jiaxun Yang (5):
>   PCI: Don't disable decoding when mmio_always_on is set
>   PCI: Add Loongson PCI Controller support
>   dt-bindings: Document Loongson PCI Host Controller
>   MIPS: DTS: Loongson64: Add PCI Controller Node
>   MIPS: Loongson64: Switch to generic PCI driver
> 
>  .../devicetree/bindings/pci/loongson.yaml     |  62 +++++
>  arch/mips/Kconfig                             |   1 +
>  arch/mips/boot/dts/loongson/rs780e-pch.dtsi   |  12 +
>  arch/mips/loongson64/Makefile                 |   2 +-
>  arch/mips/loongson64/vbios_quirk.c            |  29 ++
>  arch/mips/pci/Makefile                        |   1 -
>  arch/mips/pci/fixup-loongson3.c               |  71 -----
>  arch/mips/pci/ops-loongson3.c                 | 116 --------
>  drivers/pci/controller/Kconfig                |  10 +
>  drivers/pci/controller/Makefile               |   1 +
>  drivers/pci/controller/pci-loongson.c         | 247 ++++++++++++++++++
>  drivers/pci/probe.c                           |   2 +-
>  12 files changed, 364 insertions(+), 190 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/pci/loongson.yaml
>  create mode 100644 arch/mips/loongson64/vbios_quirk.c
>  delete mode 100644 arch/mips/pci/fixup-loongson3.c
>  delete mode 100644 arch/mips/pci/ops-loongson3.c
>  create mode 100644 drivers/pci/controller/pci-loongson.c

applied to mips-next.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

^ permalink raw reply

* Re: [PATCH v10 5/5] MIPS: Loongson64: Switch to generic PCI driver
From: Thomas Bogendoerfer @ 2020-05-27 11:34 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: Jiaxun Yang, linux-pci, Bjorn Helgaas, Rob Herring, Huacai Chen,
	Paul Burton, devicetree, linux-kernel, linux-mips
In-Reply-To: <20200526091227.GB19547@e121166-lin.cambridge.arm.com>

On Tue, May 26, 2020 at 10:12:27AM +0100, Lorenzo Pieralisi wrote:
> On Sat, May 23, 2020 at 12:36:56AM +0200, Thomas Bogendoerfer wrote:
> > On Fri, May 22, 2020 at 04:22:11PM +0100, Lorenzo Pieralisi wrote:
> > > On Fri, May 22, 2020 at 04:25:50PM +0200, Thomas Bogendoerfer wrote:
> > > > On Thu, May 14, 2020 at 09:16:41PM +0800, Jiaxun Yang wrote:
> > > > > We can now enable generic PCI driver in Kconfig, and remove legacy
> > > > > PCI driver code.
> > > > > 
> > > > > Radeon vbios quirk is moved to the platform folder to fit the
> > > > > new structure.
> > > > > 
> > > > > Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> > > > > --
> > > > > v9: Fix licenses tag
> > > > > ---
> > > > >  arch/mips/Kconfig                  |   1 +
> > > > >  arch/mips/loongson64/Makefile      |   2 +-
> > > > >  arch/mips/loongson64/vbios_quirk.c |  29 ++++++++
> > > > >  arch/mips/pci/Makefile             |   1 -
> > > > >  arch/mips/pci/fixup-loongson3.c    |  71 ------------------
> > > > >  arch/mips/pci/ops-loongson3.c      | 116 -----------------------------
> > > > >  6 files changed, 31 insertions(+), 189 deletions(-)
> > > > >  create mode 100644 arch/mips/loongson64/vbios_quirk.c
> > > > >  delete mode 100644 arch/mips/pci/fixup-loongson3.c
> > > > >  delete mode 100644 arch/mips/pci/ops-loongson3.c
> > > > 
> > > > Acked-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> > > 
> > > This patch (so the series) does not apply to v5.7-rc1 which is our
> > > baseline. I reiterate the point, isn't it better to take the whole
> > > series through the MIPS tree ?
> > 
> > sounds better then
> > 
> > > Failing that, the series has to
> > > be rebased (or split differently so that it can be taken through
> > > different trees), just let me know.
> > 
> > so let's take via mips-next. So can I add your Acked-by to the
> > first three patches ?
> 
> I have acked patch 2, you have all required ACKs to get this series via
> the MIPS tree now, apologies for the delay.

perfect, thanks.

Thomas.


-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

^ permalink raw reply

* Re: [PATCH 1/3] MIPS: ingenic: DTS: Add memory info of GCW Zero
From: Thomas Bogendoerfer @ 2020-05-27 11:36 UTC (permalink / raw)
  To: Paul Cercueil; +Cc: od, devicetree, linux-mips, linux-kernel
In-Reply-To: <20200525174155.457491-1-paul@crapouillou.net>

On Mon, May 25, 2020 at 07:41:53PM +0200, Paul Cercueil wrote:
> Add memory info of the GCW Zero in its devicetree. The bootloader
> generally provides this information, but since it is fixed to 512 MiB,
> it doesn't hurt to have it in devicetree. It allows the kernel to boot
> without any parameter passed as argument.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>  arch/mips/boot/dts/ingenic/gcw0.dts | 6 ++++++
>  1 file changed, 6 insertions(+)

applied patch 1-3 to mips-next.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

^ permalink raw reply

* Re: [PATCH v5 06/11] net: ethernet: mtk-star-emac: new driver
From: Arnd Bergmann @ 2020-05-27 11:33 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Nathan Chancellor, Rob Herring, David S . Miller,
	Matthias Brugger, John Crispin, Sean Wang, Mark Lee,
	Jakub Kicinski, Fabien Parent, Heiner Kallweit, Edwin Peer,
	devicetree, Linux Kernel Mailing List, netdev, Linux ARM,
	moderated list:ARM/Mediatek SoC..., Stephane Le Provost,
	Pedro Tsai, Andrew Perepech, Bartosz Golaszewski,
	clang-built-linux
In-Reply-To: <CAMRc=MevVsYZFDQif+8Zyv41sSkbS8XqWbKGdCvHooneXz88hg@mail.gmail.com>

On Wed, May 27, 2020 at 10:46 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> > I don't know if there should be a new label that excludes that
> > assignment for those particular gotos or if new_dma_addr should
> > be initialized to something at the top. Please take a look at
> > addressing this when you get a chance.
> >
> > Cheers,
> > Nathan
>
> Hi Nathan,
>
> Thanks for reporting this! I have a fix ready and will send it shortly.

I already have a workaround for this bug as well as another one
in my tree that I'll send later today after some more testing.

Feel free to wait for that, or just ignore mine if you already have a fix.

       Arnd

^ permalink raw reply

* Re: [PATCH 1/2] dt-bindings: ASoC: renesas,rsnd: Add r8a7742 support
From: Lad, Prabhakar @ 2020-05-27 11:32 UTC (permalink / raw)
  To: Mark Brown
  Cc: Lad Prabhakar, Geert Uytterhoeven, Magnus Damm, Rob Herring,
	Liam Girdwood, alsa-devel, Linux-Renesas,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML
In-Reply-To: <20200527112548.GD5308@sirena.org.uk>

Hi Mark,

Thank you for the review.

On Wed, May 27, 2020 at 12:25 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Tue, May 26, 2020 at 10:01:43PM +0100, Lad Prabhakar wrote:
>
> >                                 Examples with soctypes are:
> > +                                 - "renesas,rcar_sound-r8a7742" (RZ/G1H)
> >                                   - "renesas,rcar_sound-r8a7743" (RZ/G1M)
> >                                   - "renesas,rcar_sound-r8a7744" (RZ/G1N)
> >                                   - "renesas,rcar_sound-r8a7745" (RZ/G1E)
>
> I'd expect a matching patch adding this compatible to the driver.

The Renesas R-Car sound for RZ/G1H is identical to the R-Car Gen2
family. So no driver change is needed and  the fallback compatible
value "renesas,rcar_sound-gen2" will be used in the SOC DT.

Cheers,
--Prabhakar Lad

^ permalink raw reply

* [PATCH] i2c: add 'single-master' property to generic bindings
From: Wolfram Sang @ 2020-05-27 11:30 UTC (permalink / raw)
  To: linux-i2c; +Cc: devicetree, Rob Herring, Wolfram Sang, Laine Jaakko EXT

It is useful to know if we are the only master on a given bus. Because
this is a HW description of the bus, add it to the generic bindings.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: Laine Jaakko EXT <ext-jaakko.laine@vaisala.com>
---

We added 'multi-master' back then because most busses are single-master
and 'multi-master' was the exception. In hindsight, however, this was a
bad choice because 'multi-master' should be the default, i.e. if you
know nothing, you should assume there could be another master.

So, we can't deduce that a missing 'multi-master' property automatically
means 'single-master'. That's why we need this new property.

I am a bit tempted to mark 'multi-master' as deprecated because the
default should be multi-master. However, it might also be a bit more
descriptive to let "no property" still mean "we don't know". I'd be
thankful for more opinions here.

Thanks and happy hacking,

   Wolfram

 Documentation/devicetree/bindings/i2c/i2c.txt | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
index 819436b48fae..438ae123107e 100644
--- a/Documentation/devicetree/bindings/i2c/i2c.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c.txt
@@ -70,7 +70,12 @@ wants to support one of the below features, it should adapt these bindings.
 - multi-master
 	states that there is another master active on this bus. The OS can use
 	this information to adapt power management to keep the arbitration awake
-	all the time, for example.
+	all the time, for example. Can not be combined with 'single-master'.
+
+- single-master
+	states that there is no other master active on this bus. The OS can use
+	this information to detect a stalled bus more reliably, for example.
+	Can not be combined with 'multi-master'.
 
 Required properties (per child device)
 --------------------------------------
-- 
2.20.1


^ permalink raw reply related


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