Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH 1/3] ARM: dts: r8a7742-iwg21d-q7: Enable HSUSB, USB2.0 and XHCI
From: Sergei Shtylyov @ 2020-05-28  9:36 UTC (permalink / raw)
  To: Lad Prabhakar, Geert Uytterhoeven, Magnus Damm, Rob Herring
  Cc: linux-renesas-soc, devicetree, linux-kernel, Prabhakar
In-Reply-To: <1590611013-26029-2-git-send-email-prabhakar.mahadev-lad.rj@bp.renesas.com>

Hello!

On 27.05.2020 23:23, Lad Prabhakar wrote:

> Enable support for HSUB, USB2.0 and xhci on iWave RZ/G1H carrier board.

    HSUSB, xHCI.

> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
[...]

MBR, Sergei

^ permalink raw reply

* Re: [PATCH 2/6] dt-bindings: serdev: ngsm: Add binding for serdev-ngsm
From: Johan Hovold @ 2020-05-28  9:38 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Greg Kroah-Hartman, Johan Hovold, Rob Herring, Alan Cox,
	Lee Jones, Jiri Slaby, Merlijn Wajer, Pavel Machek, Peter Hurley,
	Sebastian Reichel, linux-serial, devicetree, linux-kernel,
	linux-omap
In-Reply-To: <20200512214713.40501-3-tony@atomide.com>

On Tue, May 12, 2020 at 02:47:09PM -0700, Tony Lindgren wrote:
> Add a binding document for a generic serdev-ngsm driver that can be
> used to bring up TS 27.010 line discipline with Linux n_gsm support
> on a serial port.
> 
> As the Motorola Mapphone modems require some custom handling, they
> are handled with a separate compatible.
> 
> Let's also add vendor string for ETSI as we're using a ETSI 3GPP
> TS 27.010 standard.
> 
> Reviewed-by: Pavel Machek <pavel@ucw.cz>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
>  .../bindings/serdev/serdev-ngsm.yaml          | 64 +++++++++++++++++++
>  .../devicetree/bindings/vendor-prefixes.yaml  |  2 +
>  2 files changed, 66 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/serdev/serdev-ngsm.yaml
> 
> diff --git a/Documentation/devicetree/bindings/serdev/serdev-ngsm.yaml b/Documentation/devicetree/bindings/serdev/serdev-ngsm.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serdev/serdev-ngsm.yaml
> @@ -0,0 +1,64 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/serdev/serdev-ngsm.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Generic serdev-ngsm TS 27.010 driver
> +
> +maintainers:
> +  - Tony Lindgren <tony@atomide.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - etsi,3gpp-ts27010-adaption1
> +      - motorola,mapphone-mdm6600-serial
> +
> +  ttymask:
> +    $ref: /schemas/types.yaml#/definitions/uint64
> +    description: Mask of the TS 27.010 channel TTY interfaces to start (64 bit)
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: motorola,mapphone-mdm6600-serial
> +    then:
> +      properties:
> +        phys:
> +          $ref: /schemas/types.yaml#/definitions/phandle-array
> +          description: USB PHY needed for shared GPIO PM wake-up pins
> +          maxItems: 1
> +
> +        phy-names:
> +          description: Name of the USB PHY
> +          const: usb

As I've mentioned before, I think this whole USB phy dependency is
misleading and doesn't accurately describe the hardware as devicetree
should.

It's the modem that needs to be woken up regardless of whether you use
its USB or serial interface.

> +
> +      required:
> +        - phys
> +        - phy-names
> +
> +required:
> +  - compatible
> +  - ttymask

This is a new property which it seems you forgot define. Currently it
looks like a linuxism ("tty") which doesn't belong in the devicetree.

Perhaps a rename is all that's needed (e.g. portmask or similar).

> +  - "#address-cells"
> +  - "#size-cells"
> +
> +examples:
> +  - |
> +    modem {
> +      compatible = "motorola,mapphone-mdm6600-serial";
> +      ttymask = <0 0x00001fee>;
> +      phys = <&fsusb1_phy>;
> +      phy-names = "usb";
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +    };
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> @@ -323,6 +323,8 @@ patternProperties:
>      description: Espressif Systems Co. Ltd.
>    "^est,.*":
>      description: ESTeem Wireless Modems
> +  "^etsi,.*":
> +    description: ETSI

Spell out the acronym?

>    "^ettus,.*":
>      description: NI Ettus Research
>    "^eukrea,.*":

Johan

^ permalink raw reply

* Re: [PATCH 3/6] dt-bindings: serdev: ngsm: Add binding for GNSS child node
From: Johan Hovold @ 2020-05-28  9:51 UTC (permalink / raw)
  To: Rob Herring
  Cc: Tony Lindgren, Greg Kroah-Hartman, Johan Hovold, Alan Cox,
	Lee Jones, Jiri Slaby, Merlijn Wajer, Pavel Machek, Peter Hurley,
	Sebastian Reichel, linux-serial, devicetree, linux-kernel,
	linux-omap
In-Reply-To: <20200527192817.GA2587830@bogus>

On Wed, May 27, 2020 at 01:28:17PM -0600, Rob Herring wrote:
> On Tue, May 12, 2020 at 02:47:10PM -0700, Tony Lindgren wrote:
> > For motorola modem case, we may have a GNSS device on channel 4.
> > Let's add that to the binding and example.
> > 
> > Signed-off-by: Tony Lindgren <tony@atomide.com>
> > ---
> >  .../devicetree/bindings/serdev/serdev-ngsm.yaml          | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/serdev/serdev-ngsm.yaml b/Documentation/devicetree/bindings/serdev/serdev-ngsm.yaml
> > --- a/Documentation/devicetree/bindings/serdev/serdev-ngsm.yaml
> > +++ b/Documentation/devicetree/bindings/serdev/serdev-ngsm.yaml
> > @@ -42,6 +42,10 @@ allOf:
> >            description: Name of the USB PHY
> >            const: usb
> >  
> > +        compatible:
> > +          description: GNSS receiver
> > +          const: motorola,mapphone-mdm6600-gnss
> 
> I'm not sure how this isn't failing on the example because it is wrong.
> 
> You're saying this compatible belongs at the same level as 
> phys/phy-names, but that would be the parent which already has a 
> compatible. You have to define a child node property (gnss@4) and have 
> 'compatible' under it. At that point, this schema becomes very much 
> Motorola specific.
> 
> > +
> >        required:
> >          - phys
> >          - phy-names
> > @@ -61,4 +65,9 @@ examples:
> >        phy-names = "usb";
> >        #address-cells = <1>;
> >        #size-cells = <0>;
> > +
> > +      gnss@4 {
> > +         compatible = "motorola,mapphone-mdm6600-gnss";
> > +         reg = <4>;
> > +      };
> >      };
> > -- 
> > 2.26.2

And since we're describing a mux, I think you need nodes for the virtual
ports rather than a reg property in what should be a serial client. That
is something like

	serial@nnn {
		modem {
			compatible = "etsi,ts27001-mux";

			serial@4 {
				compatible = "etsi,ts27001-serial";
				reg = <4>;

				gnss {
					compatible = "motorola,motmdm-gnss";
				};
			};
		};
	};

This way you can actually use serdev for the client drivers (e.g. for
gnss), and those drivers also be used for non-muxed ports if needed
(e.g. over USB).

Johan

^ permalink raw reply

* [PATCH v4] dmaengine: mediatek-cqdma: add dt-bindings and remove redundant queue
From: EastL @ 2020-05-28  9:57 UTC (permalink / raw)
  To: Sean Wang
  Cc: vkoul, robh+dt, mark.rutland, matthias.bgg, dmaengine,
	linux-kernel, linux-arm-kernel, linux-mediatek, devicetree,
	wsd_upstream

This patch set adds document the devicetree bindings for MediaTek Command-Queue DMA controller,
and remove redundant queue structure, add dma-channel-mask for DMA capability and fix compatible.

Changes since v3:
- fix dt_binding_check errors

Changes since v2:
- add devicetree bindings for MediaTek Command-Queue DMA controller

Changes since v1:
- remove redundant queue structure
- fix wrong description and tags in the earlier patch
- add dma-channel-mask for DMA capability
- fix compatible for common

^ permalink raw reply

* [PATCH v4 3/4] dmaengine: mediatek-cqdma: fix compatible
From: EastL @ 2020-05-28  9:57 UTC (permalink / raw)
  To: Sean Wang
  Cc: vkoul, robh+dt, mark.rutland, matthias.bgg, dmaengine,
	linux-kernel, linux-arm-kernel, linux-mediatek, devicetree,
	wsd_upstream, EastL
In-Reply-To: <1590659832-31476-1-git-send-email-EastL.Lee@mediatek.com>

This patch fixes mediatek-cqdma compatible to common.

Signed-off-by: EastL <EastL.Lee@mediatek.com>
---
 drivers/dma/mediatek/mtk-cqdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/mediatek/mtk-cqdma.c b/drivers/dma/mediatek/mtk-cqdma.c
index 905bbcb..bca7118 100644
--- a/drivers/dma/mediatek/mtk-cqdma.c
+++ b/drivers/dma/mediatek/mtk-cqdma.c
@@ -544,7 +544,7 @@ static void mtk_cqdma_hw_deinit(struct mtk_cqdma_device *cqdma)
 }
 
 static const struct of_device_id mtk_cqdma_match[] = {
-	{ .compatible = "mediatek,mt6765-cqdma" },
+	{ .compatible = "mediatek,cqdma" },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, mtk_cqdma_match);
-- 
1.9.1

^ permalink raw reply related

* [PATCH v4 1/4] dt-bindings: dmaengine: Add MediaTek Command-Queue DMA controller bindings
From: EastL @ 2020-05-28  9:57 UTC (permalink / raw)
  To: Sean Wang
  Cc: vkoul, robh+dt, mark.rutland, matthias.bgg, dmaengine,
	linux-kernel, linux-arm-kernel, linux-mediatek, devicetree,
	wsd_upstream, EastL
In-Reply-To: <1590659832-31476-1-git-send-email-EastL.Lee@mediatek.com>

Document the devicetree bindings for MediaTek Command-Queue DMA controller
which could be found on MT6779 SoC or other similar Mediatek SoCs.

Signed-off-by: EastL <EastL.Lee@mediatek.com>
---
 .../devicetree/bindings/dma/mtk-cqdma.yaml         | 100 +++++++++++++++++++++
 1 file changed, 100 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dma/mtk-cqdma.yaml

diff --git a/Documentation/devicetree/bindings/dma/mtk-cqdma.yaml b/Documentation/devicetree/bindings/dma/mtk-cqdma.yaml
new file mode 100644
index 0000000..045aa0c
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/mtk-cqdma.yaml
@@ -0,0 +1,100 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/dma/mtk-cqdma.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek Command-Queue DMA controller Device Tree Binding
+
+maintainers:
+  - EastL <EastL.Lee@mediatek.com>
+
+description:
+  MediaTek Command-Queue DMA controller (CQDMA) on Mediatek SoC
+  is dedicated to memory-to-memory transfer through queue based
+  descriptor management.
+
+properties:
+  "#dma-cells":
+    minimum: 1
+    # Should be enough
+    maximum: 255
+    description:
+      Used to provide DMA controller specific information.
+
+  compatible:
+    const: mediatek,cqdma
+
+  reg:
+    minItems: 1
+    maxItems: 255
+
+  interrupts:
+    minItems: 1
+    maxItems: 255
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    const: cqdma
+
+  dma-channel-mask:
+    description:
+      Bitmask of available DMA channels in ascending order that are
+      not reserved by firmware and are available to the
+      kernel. i.e. first channel corresponds to LSB.
+      The first item in the array is for channels 0-31, the second is for
+      channels 32-63, etc.
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32-array
+    items:
+      minItems: 1
+      # Should be enough
+      maxItems: 255
+
+  dma-channels:
+    $ref: /schemas/types.yaml#definitions/uint32
+    description:
+      Number of DMA channels supported by the controller.
+
+  dma-requests:
+    $ref: /schemas/types.yaml#definitions/uint32
+    description:
+      Number of DMA request signals supported by the controller.
+
+required:
+  - "#dma-cells"
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - dma-channel-mask
+  - dma-channels
+  - dma-requests
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/mt6779-clk.h>
+    cqdma: dma-controller@10212000 {
+        compatible = "mediatek,cqdma";
+        reg = <0 0x10212000 0 0x80>,
+            <0 0x10212080 0 0x80>,
+            <0 0x10212100 0 0x80>;
+        interrupts = <GIC_SPI 139 IRQ_TYPE_LEVEL_LOW>,
+            <GIC_SPI 140 IRQ_TYPE_LEVEL_LOW>,
+            <GIC_SPI 141 IRQ_TYPE_LEVEL_LOW>;
+        clocks = <&infracfg_ao CLK_INFRA_CQ_DMA>;
+        clock-names = "cqdma";
+        dma-channel-mask = <63>;
+        dma-channels = <3>;
+        dma-requests = <32>;
+        #dma-cells = <1>;
+    };
+
+...
-- 
1.9.1

^ permalink raw reply related

* [PATCH v4 4/4] dmaengine: mediatek-cqdma: add dma mask for capability
From: EastL @ 2020-05-28  9:57 UTC (permalink / raw)
  To: Sean Wang
  Cc: vkoul, robh+dt, mark.rutland, matthias.bgg, dmaengine,
	linux-kernel, linux-arm-kernel, linux-mediatek, devicetree,
	wsd_upstream, EastL
In-Reply-To: <1590659832-31476-1-git-send-email-EastL.Lee@mediatek.com>

This patch add dma mask for capability.

Change-Id: I31f4622f9541d769702029532e5f5f185815dda2
Signed-off-by: EastL <EastL.Lee@mediatek.com>
---
 drivers/dma/mediatek/mtk-cqdma.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/dma/mediatek/mtk-cqdma.c b/drivers/dma/mediatek/mtk-cqdma.c
index bca7118..1805a76 100644
--- a/drivers/dma/mediatek/mtk-cqdma.c
+++ b/drivers/dma/mediatek/mtk-cqdma.c
@@ -117,6 +117,7 @@ struct mtk_cqdma_vchan {
  * @clk:                    The clock that device internal is using
  * @dma_requests:           The number of VCs the device supports to
  * @dma_channels:           The number of PCs the device supports to
+ * @dma_mask:               A mask for DMA capability
  * @vc:                     The pointer to all available VCs
  * @pc:                     The pointer to all the underlying PCs
  */
@@ -126,6 +127,7 @@ struct mtk_cqdma_device {
 
 	u32 dma_requests;
 	u32 dma_channels;
+	u32 dma_mask;
 	struct mtk_cqdma_vchan *vc;
 	struct mtk_cqdma_pchan **pc;
 };
@@ -549,6 +551,7 @@ static void mtk_cqdma_hw_deinit(struct mtk_cqdma_device *cqdma)
 };
 MODULE_DEVICE_TABLE(of, mtk_cqdma_match);
 
+static u64 cqdma_dmamask;
 static int mtk_cqdma_probe(struct platform_device *pdev)
 {
 	struct mtk_cqdma_device *cqdma;
@@ -607,6 +610,16 @@ static int mtk_cqdma_probe(struct platform_device *pdev)
 		cqdma->dma_channels = MTK_CQDMA_NR_PCHANS;
 	}
 
+	if (pdev->dev.of_node && of_property_read_u32(pdev->dev.of_node,
+						      "dma-channel-mask",
+						      &cqdma->dma_mask)) {
+		dev_info(&pdev->dev,
+			 "Using 0 as missing dma-channel-mask property\n");
+	} else {
+		cqdma_dmamask = DMA_BIT_MASK(cqdma->dma_mask);
+		pdev->dev.dma_mask = &cqdma_dmamask;
+	}
+
 	cqdma->pc = devm_kcalloc(&pdev->dev, cqdma->dma_channels,
 				 sizeof(*cqdma->pc), GFP_KERNEL);
 	if (!cqdma->pc)
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH v4 02/11] dt-bindings: i2c: Discard i2c-slave flag from the DW I2C example
From: Andy Shevchenko @ 2020-05-28 10:00 UTC (permalink / raw)
  To: Serge Semin
  Cc: Serge Semin, Rob Herring, Jarkko Nikula, Wolfram Sang,
	Alexey Malahov, Thomas Bogendoerfer, Mika Westerberg, linux-mips,
	linux-i2c, devicetree, linux-kernel
In-Reply-To: <20200528083923.yjlm5ur7cslgxdau@mobilestation>

On Thu, May 28, 2020 at 11:39:23AM +0300, Serge Semin wrote:
> On Wed, May 27, 2020 at 08:56:24PM +0300, Andy Shevchenko wrote:
> > On Wed, May 27, 2020 at 08:18:41PM +0300, Serge Semin wrote:
> > > On Wed, May 27, 2020 at 11:12:04AM -0600, Rob Herring wrote:
> > > > On Wed, May 27, 2020 at 03:01:02PM +0300, Serge Semin 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:
> > > > > 
> > > > > 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.
> > 
> > > > >        eeprom@64 {
> > > > >          compatible = "linux,slave-24c02";
> > > > > -        reg = <0x40000064>;
> > > > > +        reg = <0x64>;
> > > > 
> > > > But the compatible is a slave, so you need an example with a different 
> > > > device.
> > > 
> > 
> > > Ok. I'll replace the sub-node with just "atmel,24c02" compatible string then.
> > 
> > But how it will be different to the another slave connected to the master?
> > 
> > This example is specifically to show that DesingWare I²C controller may be
> > switched to slave mode.
> 
> Well, dtc doesn't support it and prints warning that the address is invalid.
> Though I do understand you concern and is mostly agree with it. Let's do this in
> the next way. I'll resend the series with eeprom@64 sub-node replaced with just
> a normal eeprom-device. The message log will have an info why this has been
> done. In the non-mergeable section of the patch I'll suggest to Rob reconsider
> the patch acking, since we can leave the slave-marked sub-node and just live
> with the dtc warning until it's fixed in there.

Thanks!

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* Re: [RESEND][V5 PATCH 1/2] dt-bindings: Added device tree binding for max98390
From: Steve Lee @ 2020-05-28 10:02 UTC (permalink / raw)
  To: Rob Herring
  Cc: Steve Lee, Liam Girdwood, Mark Brown, ALSA development,
	devicetree, Linux Kernel Mailing List, ryan.lee.maxim, ryans.lee
In-Reply-To: <20200526223642.GA506893@bogus>

On Wed, May 27, 2020 at 7:36 AM Rob Herring <robh@kernel.org> wrote:
>
> On Mon, May 18, 2020 at 09:49:30AM +0900, Steve Lee wrote:
> > Add documentation for DT binding of max98390 amplifier driver.
> >
> > Signed-off-by: Steve Lee <steves.lee@maximintegrated.com>
> > ---
> >
> >
> > Changed since V4:
> >       * No changes.
> > Changed since V3:
> >       * No changes.
> > Changed since V2:
> >       * No changes.
> > Changed since V1:
> >       * Modified sample text in example
>
> You are obviously sending patches too quickly. Give folks a chance to
> review.

 Thanks for your feedback !.

>
> >
> >  .../devicetree/bindings/sound/max98390.txt    | 26 +++++++++++++++++++
>
> Bindings are now in DT schema format. Please convert this. See
> Documentation/devicetree/writing-schema.rst

 Thanks for review. I will change txt to yaml version.

>
> >  1 file changed, 26 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/sound/max98390.txt
> >
> > diff --git a/Documentation/devicetree/bindings/sound/max98390.txt b/Documentation/devicetree/bindings/sound/max98390.txt
> > new file mode 100644
> > index 000000000000..0ddd4c6ae55e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/sound/max98390.txt
> > @@ -0,0 +1,26 @@
> > +Maxim Integrated MAX98390 Speaker Amplifier
> > +
> > +This device supports I2C.
> > +
> > +Required properties:
> > +
> > + - compatible : "maxim,max98390"
> > +
> > + - reg : the I2C address of the device.
> > +
> > +Optional properties:
> > +
> > +- maxim,temperature_calib
> > +  u32. The calculated temperature data was measured while doing the calibration. Data : Temp / 100 * 2^12
> > +
> > +- maxim,r0_calib
> > +  u32. This is r0 calibration data which was measured in factory mode.
>
> Unless these are shared already with other Maxim chips, s/_/-/.
>
> > +
> > +Example:
> > +
> > +codec: max98390@38 {
>
> amplifier@38

 I will change example as you advise.

>
> > +     compatible = "maxim,max98390";
> > +     reg = <0x38>;
> > +     maxim,temperature_calib = <1024>;
> > +     maxim,r0_calib = <100232>;
> > +};
> > --
> > 2.17.1
> >

^ permalink raw reply

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

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

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Thanks!

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

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

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

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

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Conditionally, in case Wolfram and Jarkko are fine with shuffling model
defines, which I consider an unneeded churn.

> 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
> 

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* Re: [PATCH v2 2/3] mmc: sdhci-of-arasan: Add support for Intel Keem Bay
From: Ulf Hansson @ 2020-05-28 10:14 UTC (permalink / raw)
  To: Wan Ahmad Zainie
  Cc: Rob Herring, Adrian Hunter, Michal Simek,
	linux-mmc@vger.kernel.org, DTML
In-Reply-To: <20200526062758.17642-3-wan.ahmad.zainie.wan.mohamad@intel.com>

On Tue, 26 May 2020 at 08:29, Wan Ahmad Zainie
<wan.ahmad.zainie.wan.mohamad@intel.com> wrote:
>
> Intel Keem Bay SoC eMMC/SD/SDIO controller is based on
> Arasan SD 3.0 / eMMC 5.1 host controller IP.
>
> However, it does not support 64-bit access as its AXI interface
> has 32-bit address ports.
>
> Signed-off-by: Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com>
> Reviewed-by: Adrian Hunter <adrian.hunter@intel.com>

Applied for next, thanks!

Kind regards
Uffe


> ---
>  drivers/mmc/host/sdhci-of-arasan.c | 123 +++++++++++++++++++++++++++++
>  1 file changed, 123 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
> index 2fe2c4dcc280..db9b544465cd 100644
> --- a/drivers/mmc/host/sdhci-of-arasan.c
> +++ b/drivers/mmc/host/sdhci-of-arasan.c
> @@ -75,6 +75,7 @@ struct sdhci_arasan_soc_ctl_field {
>   *
>   * @baseclkfreq:       Where to find corecfg_baseclkfreq
>   * @clockmultiplier:   Where to find corecfg_clockmultiplier
> + * @support64b:                Where to find SUPPORT64B bit
>   * @hiword_update:     If true, use HIWORD_UPDATE to access the syscon
>   *
>   * It's up to the licensee of the Arsan IP block to make these available
> @@ -84,6 +85,7 @@ struct sdhci_arasan_soc_ctl_field {
>  struct sdhci_arasan_soc_ctl_map {
>         struct sdhci_arasan_soc_ctl_field       baseclkfreq;
>         struct sdhci_arasan_soc_ctl_field       clockmultiplier;
> +       struct sdhci_arasan_soc_ctl_field       support64b;
>         bool                                    hiword_update;
>  };
>
> @@ -180,6 +182,13 @@ static const struct sdhci_arasan_soc_ctl_map intel_lgm_sdxc_soc_ctl_map = {
>         .hiword_update = false,
>  };
>
> +static const struct sdhci_arasan_soc_ctl_map intel_keembay_soc_ctl_map = {
> +       .baseclkfreq = { .reg = 0x0, .width = 8, .shift = 14 },
> +       .clockmultiplier = { .reg = 0x4, .width = 8, .shift = 14 },
> +       .support64b = { .reg = 0x4, .width = 1, .shift = 24 },
> +       .hiword_update = false,
> +};
> +
>  /**
>   * sdhci_arasan_syscon_write - Write to a field in soc_ctl registers
>   *
> @@ -1095,6 +1104,50 @@ static struct sdhci_arasan_of_data sdhci_arasan_generic_data = {
>         .clk_ops = &arasan_clk_ops,
>  };
>
> +static const struct sdhci_pltfm_data sdhci_keembay_emmc_pdata = {
> +       .ops = &sdhci_arasan_cqe_ops,
> +       .quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
> +               SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC |
> +               SDHCI_QUIRK_NO_LED |
> +               SDHCI_QUIRK_32BIT_DMA_ADDR |
> +               SDHCI_QUIRK_32BIT_DMA_SIZE |
> +               SDHCI_QUIRK_32BIT_ADMA_SIZE,
> +       .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
> +               SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN |
> +               SDHCI_QUIRK2_CAPS_BIT63_FOR_HS400 |
> +               SDHCI_QUIRK2_STOP_WITH_TC |
> +               SDHCI_QUIRK2_BROKEN_64_BIT_DMA,
> +};
> +
> +static const struct sdhci_pltfm_data sdhci_keembay_sd_pdata = {
> +       .ops = &sdhci_arasan_ops,
> +       .quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
> +               SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC |
> +               SDHCI_QUIRK_NO_LED |
> +               SDHCI_QUIRK_32BIT_DMA_ADDR |
> +               SDHCI_QUIRK_32BIT_DMA_SIZE |
> +               SDHCI_QUIRK_32BIT_ADMA_SIZE,
> +       .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
> +               SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN |
> +               SDHCI_QUIRK2_CARD_ON_NEEDS_BUS_ON |
> +               SDHCI_QUIRK2_STOP_WITH_TC |
> +               SDHCI_QUIRK2_BROKEN_64_BIT_DMA,
> +};
> +
> +static const struct sdhci_pltfm_data sdhci_keembay_sdio_pdata = {
> +       .ops = &sdhci_arasan_ops,
> +       .quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
> +               SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC |
> +               SDHCI_QUIRK_NO_LED |
> +               SDHCI_QUIRK_32BIT_DMA_ADDR |
> +               SDHCI_QUIRK_32BIT_DMA_SIZE |
> +               SDHCI_QUIRK_32BIT_ADMA_SIZE,
> +       .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
> +               SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN |
> +               SDHCI_QUIRK2_HOST_OFF_CARD_ON |
> +               SDHCI_QUIRK2_BROKEN_64_BIT_DMA,
> +};
> +
>  static struct sdhci_arasan_of_data sdhci_arasan_rk3399_data = {
>         .soc_ctl_map = &rk3399_soc_ctl_map,
>         .pdata = &sdhci_arasan_cqe_pdata,
> @@ -1140,6 +1193,21 @@ static struct sdhci_arasan_of_data sdhci_arasan_versal_data = {
>         .clk_ops = &versal_clk_ops,
>  };
>
> +static struct sdhci_arasan_of_data intel_keembay_emmc_data = {
> +       .soc_ctl_map = &intel_keembay_soc_ctl_map,
> +       .pdata = &sdhci_keembay_emmc_pdata,
> +};
> +
> +static struct sdhci_arasan_of_data intel_keembay_sd_data = {
> +       .soc_ctl_map = &intel_keembay_soc_ctl_map,
> +       .pdata = &sdhci_keembay_sd_pdata,
> +};
> +
> +static struct sdhci_arasan_of_data intel_keembay_sdio_data = {
> +       .soc_ctl_map = &intel_keembay_soc_ctl_map,
> +       .pdata = &sdhci_keembay_sdio_pdata,
> +};
> +
>  static const struct of_device_id sdhci_arasan_of_match[] = {
>         /* SoC-specific compatible strings w/ soc_ctl_map */
>         {
> @@ -1154,6 +1222,18 @@ static const struct of_device_id sdhci_arasan_of_match[] = {
>                 .compatible = "intel,lgm-sdhci-5.1-sdxc",
>                 .data = &intel_lgm_sdxc_data,
>         },
> +       {
> +               .compatible = "intel,keembay-sdhci-5.1-emmc",
> +               .data = &intel_keembay_emmc_data,
> +       },
> +       {
> +               .compatible = "intel,keembay-sdhci-5.1-sd",
> +               .data = &intel_keembay_sd_data,
> +       },
> +       {
> +               .compatible = "intel,keembay-sdhci-5.1-sdio",
> +               .data = &intel_keembay_sdio_data,
> +       },
>         /* Generic compatible below here */
>         {
>                 .compatible = "arasan,sdhci-8.9a",
> @@ -1297,6 +1377,40 @@ static void sdhci_arasan_unregister_sdclk(struct device *dev)
>         of_clk_del_provider(dev->of_node);
>  }
>
> +/**
> + * sdhci_arasan_update_support64b - Set SUPPORT_64B (64-bit System Bus Support)
> + *
> + * This should be set based on the System Address Bus.
> + * 0: the Core supports only 32-bit System Address Bus.
> + * 1: the Core supports 64-bit System Address Bus.
> + *
> + * NOTES:
> + * - For Keem Bay, it is required to clear this bit. Its default value is 1'b1.
> + *   Keem Bay does not support 64-bit access.
> + *
> + * @host               The sdhci_host
> + */
> +static void sdhci_arasan_update_support64b(struct sdhci_host *host, u32 value)
> +{
> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +       struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host);
> +       const struct sdhci_arasan_soc_ctl_map *soc_ctl_map =
> +               sdhci_arasan->soc_ctl_map;
> +
> +       /* Having a map is optional */
> +       if (!soc_ctl_map)
> +               return;
> +
> +       /* If we have a map, we expect to have a syscon */
> +       if (!sdhci_arasan->soc_ctl_base) {
> +               pr_warn("%s: Have regmap, but no soc-ctl-syscon\n",
> +                       mmc_hostname(host->mmc));
> +               return;
> +       }
> +
> +       sdhci_arasan_syscon_write(host, &soc_ctl_map->support64b, value);
> +}
> +
>  /**
>   * sdhci_arasan_register_sdclk - Register the sdcardclk for a PHY to use
>   *
> @@ -1469,6 +1583,15 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
>                                     "rockchip,rk3399-sdhci-5.1"))
>                 sdhci_arasan_update_clockmultiplier(host, 0x0);
>
> +       if (of_device_is_compatible(np, "intel,keembay-sdhci-5.1-emmc") ||
> +           of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sd") ||
> +           of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sdio")) {
> +               sdhci_arasan_update_clockmultiplier(host, 0x0);
> +               sdhci_arasan_update_support64b(host, 0x0);
> +
> +               host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY;
> +       }
> +
>         sdhci_arasan_update_baseclkfreq(host);
>
>         ret = sdhci_arasan_register_sdclk(sdhci_arasan, clk_xin, &pdev->dev);
> --
> 2.17.1
>

^ permalink raw reply

* Re: [PATCH v2 1/3] dt-bindings: mmc: arasan: Add compatible strings for Intel Keem Bay
From: Ulf Hansson @ 2020-05-28 10:14 UTC (permalink / raw)
  To: Wan Ahmad Zainie
  Cc: Rob Herring, Adrian Hunter, Michal Simek,
	linux-mmc@vger.kernel.org, DTML
In-Reply-To: <20200526062758.17642-2-wan.ahmad.zainie.wan.mohamad@intel.com>

On Tue, 26 May 2020 at 08:29, Wan Ahmad Zainie
<wan.ahmad.zainie.wan.mohamad@intel.com> wrote:
>
> Add new compatible strings in sdhci-of-arasan.c to support Intel Keem Bay
> eMMC/SD/SDIO controller, based on Arasan SDHCI 5.1.
>
> Signed-off-by: Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com>

Applied for next, thanks!

Kind regards
Uffe


> ---
>  .../devicetree/bindings/mmc/arasan,sdhci.txt  | 42 +++++++++++++++++++
>  1 file changed, 42 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> index 630fe707f5c4..f29bf7dd2ece 100644
> --- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> @@ -27,6 +27,12 @@ Required Properties:
>        For this device it is strongly suggested to include arasan,soc-ctl-syscon.
>      - "intel,lgm-sdhci-5.1-sdxc", "arasan,sdhci-5.1": Intel LGM SDXC PHY
>        For this device it is strongly suggested to include arasan,soc-ctl-syscon.
> +    - "intel,keembay-sdhci-5.1-emmc", "arasan,sdhci-5.1": Intel Keem Bay eMMC
> +      For this device it is strongly suggested to include arasan,soc-ctl-syscon.
> +    - "intel,keembay-sdhci-5.1-sd": Intel Keem Bay SD controller
> +      For this device it is strongly suggested to include arasan,soc-ctl-syscon.
> +    - "intel,keembay-sdhci-5.1-sdio": Intel Keem Bay SDIO controller
> +      For this device it is strongly suggested to include arasan,soc-ctl-syscon.
>
>    [5] Documentation/devicetree/bindings/mmc/sdhci-am654.txt
>
> @@ -148,3 +154,39 @@ Example:
>                 phy-names = "phy_arasan";
>                 arasan,soc-ctl-syscon = <&sysconf>;
>         };
> +
> +       mmc: mmc@33000000 {
> +               compatible = "intel,keembay-sdhci-5.1-emmc", "arasan,sdhci-5.1";
> +               interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
> +               reg = <0x0 0x33000000 0x0 0x300>;
> +               clock-names = "clk_xin", "clk_ahb";
> +               clocks = <&scmi_clk KEEM_BAY_PSS_AUX_EMMC>,
> +                        <&scmi_clk KEEM_BAY_PSS_EMMC>;
> +               phys = <&emmc_phy>;
> +               phy-names = "phy_arasan";
> +               assigned-clocks = <&scmi_clk KEEM_BAY_PSS_AUX_EMMC>;
> +               assigned-clock-rates = <200000000>;
> +               clock-output-names = "emmc_cardclock";
> +               #clock-cells = <0>;
> +               arasan,soc-ctl-syscon = <&mmc_phy_syscon>;
> +       };
> +
> +       sd0: mmc@31000000 {
> +               compatible = "intel,keembay-sdhci-5.1-sd";
> +               interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>;
> +               reg = <0x0 0x31000000 0x0 0x300>;
> +               clock-names = "clk_xin", "clk_ahb";
> +               clocks = <&scmi_clk KEEM_BAY_PSS_AUX_SD0>,
> +                        <&scmi_clk KEEM_BAY_PSS_SD0>;
> +               arasan,soc-ctl-syscon = <&sd0_phy_syscon>;
> +       };
> +
> +       sd1: mmc@32000000 {
> +               compatible = "intel,keembay-sdhci-5.1-sdio";
> +               interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>;
> +               reg = <0x0 0x32000000 0x0 0x300>;
> +               clock-names = "clk_xin", "clk_ahb";
> +               clocks = <&scmi_clk KEEM_BAY_PSS_AUX_SD1>,
> +                        <&scmi_clk KEEM_BAY_PSS_SD1>;
> +               arasan,soc-ctl-syscon = <&sd1_phy_syscon>;
> +       };
> --
> 2.17.1
>

^ permalink raw reply

* Re: [PATCH 2/2] mmc: mmci_sdmmc: fix DMA API warning max segment size
From: Ulf Hansson @ 2020-05-28 10:14 UTC (permalink / raw)
  To: Ludovic Barre
  Cc: Rob Herring, Srini Kandagatla, Maxime Coquelin, Alexandre Torgue,
	Linux ARM, Linux Kernel Mailing List, DTML,
	linux-mmc@vger.kernel.org, linux-stm32
In-Reply-To: <20200526155103.12514-3-ludovic.barre@st.com>

On Tue, 26 May 2020 at 17:51, Ludovic Barre <ludovic.barre@st.com> wrote:
>
> Turning on CONFIG_DMA_API_DEBUG_SG results in the following warning:
> WARNING: CPU: 1 PID: 85 at kernel/dma/debug.c:1302 debug_dma_map_sg+0x2a0/0x3cc
> mmci-pl18x 58005000.sdmmc: DMA-API: mapping sg segment longer than device claims to support [len=126976] [max=65536]
>
> dma api debug checks and compares the segment size to
> dma_get_max_seg_size (dev->dma_parms->max_segment_size),
> the sdmmc variant has an internal DMA and should define
> its max_segment_size constraint to avoid this warning.
>
> This Patch defines the dev->dma_parms->max_segment_size
> with the constraint already set for mmc core
> (host->mmc->max_seg_size).
>
> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>

Applied for next, thanks!

Note, a manual backport is needed for stable, as
dma_set_max_seg_size() will fail for older kernels.
We needed to revert 9495b7e92f7 ("driver core: platform: Initialize
dma_parms for platform devices"), for stable kernels [1].

Kind regards
Uffe

[1]
https://lkml.org/lkml/2020/5/26/1216


> ---
>  drivers/mmc/host/mmci_stm32_sdmmc.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c
> index 2965b1c062e1..51db30acf4dc 100644
> --- a/drivers/mmc/host/mmci_stm32_sdmmc.c
> +++ b/drivers/mmc/host/mmci_stm32_sdmmc.c
> @@ -119,20 +119,19 @@ static void sdmmc_idma_unprep_data(struct mmci_host *host,
>  static int sdmmc_idma_setup(struct mmci_host *host)
>  {
>         struct sdmmc_idma *idma;
> +       struct device *dev = mmc_dev(host->mmc);
>
> -       idma = devm_kzalloc(mmc_dev(host->mmc), sizeof(*idma), GFP_KERNEL);
> +       idma = devm_kzalloc(dev, sizeof(*idma), GFP_KERNEL);
>         if (!idma)
>                 return -ENOMEM;
>
>         host->dma_priv = idma;
>
>         if (host->variant->dma_lli) {
> -               idma->sg_cpu = dmam_alloc_coherent(mmc_dev(host->mmc),
> -                                                  SDMMC_LLI_BUF_LEN,
> +               idma->sg_cpu = dmam_alloc_coherent(dev, SDMMC_LLI_BUF_LEN,
>                                                    &idma->sg_dma, GFP_KERNEL);
>                 if (!idma->sg_cpu) {
> -                       dev_err(mmc_dev(host->mmc),
> -                               "Failed to alloc IDMA descriptor\n");
> +                       dev_err(dev, "Failed to alloc IDMA descriptor\n");
>                         return -ENOMEM;
>                 }
>                 host->mmc->max_segs = SDMMC_LLI_BUF_LEN /
> @@ -143,7 +142,7 @@ static int sdmmc_idma_setup(struct mmci_host *host)
>                 host->mmc->max_seg_size = host->mmc->max_req_size;
>         }
>
> -       return 0;
> +       return dma_set_max_seg_size(dev, host->mmc->max_seg_size);
>  }
>
>  static int sdmmc_idma_start(struct mmci_host *host, unsigned int *datactrl)
> --
> 2.17.1
>

^ permalink raw reply

* Re: [PATCH 1/2] mmc: mmci_sdmmc: fix DMA API warning overlapping mappings
From: Ulf Hansson @ 2020-05-28 10:14 UTC (permalink / raw)
  To: Ludovic Barre
  Cc: Rob Herring, Srini Kandagatla, Maxime Coquelin, Alexandre Torgue,
	Linux ARM, Linux Kernel Mailing List, DTML,
	linux-mmc@vger.kernel.org, linux-stm32
In-Reply-To: <20200526155103.12514-2-ludovic.barre@st.com>

On Tue, 26 May 2020 at 17:51, Ludovic Barre <ludovic.barre@st.com> wrote:
>
> Turning on CONFIG_DMA_API_DEBUG_SG results in the following warning:
> WARNING: CPU: 1 PID: 20 at kernel/dma/debug.c:500 add_dma_entry+0x16c/0x17c
> DMA-API: exceeded 7 overlapping mappings of cacheline 0x031d2645
> Modules linked in:
> CPU: 1 PID: 20 Comm: kworker/1:1 Not tainted 5.5.0-rc2-00021-gdeda30999c2b-dirty #49
> Hardware name: STM32 (Device Tree Support)
> Workqueue: events_freezable mmc_rescan
> [<c03138c0>] (unwind_backtrace) from [<c030d760>] (show_stack+0x10/0x14)
> [<c030d760>] (show_stack) from [<c0f2eb28>] (dump_stack+0xc0/0xd4)
> [<c0f2eb28>] (dump_stack) from [<c034a14c>] (__warn+0xd0/0xf8)
> [<c034a14c>] (__warn) from [<c034a530>] (warn_slowpath_fmt+0x94/0xb8)
> [<c034a530>] (warn_slowpath_fmt) from [<c03bca0c>] (add_dma_entry+0x16c/0x17c)
> [<c03bca0c>] (add_dma_entry) from [<c03bdf54>] (debug_dma_map_sg+0xe4/0x3d4)
> [<c03bdf54>] (debug_dma_map_sg) from [<c0d09244>] (sdmmc_idma_prep_data+0x94/0xf8)
> [<c0d09244>] (sdmmc_idma_prep_data) from [<c0d05a2c>] (mmci_prep_data+0x2c/0xb0)
> [<c0d05a2c>] (mmci_prep_data) from [<c0d073ec>] (mmci_start_data+0x134/0x2f0)
> [<c0d073ec>] (mmci_start_data) from [<c0d078d0>] (mmci_request+0xe8/0x154)
> [<c0d078d0>] (mmci_request) from [<c0cecb44>] (mmc_start_request+0x94/0xbc)
>
> DMA api debug brings to light leaking dma-mappings, dma_map_sg and
> dma_unmap_sg are not correctly balanced.
>
> If a request is prepared, the dma_map/unmap are done in asynchronous
> call pre_req (prep_data) and post_req (unprep_data). In this case
> the dma-mapping is right balanced.
>
> But if the request was not prepared, the data->host_cookie is
> define to zero and the dma_map/unmap must be done in the request.
> The dma_map is called by mmci_dma_start (prep_data), but there is
> no dma_unmap in this case.
>
> This patch adds dma_unmap_sg when the dma is finalized and
> the data cookie is zero (request not prepared).
>
> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>

Applied for next by adding a fixes tag and a stable tag, thanks!

Fixes: 46b723dd867d ("mmc: mmci: add stm32 sdmmc variant")

Kind regards
Uffe


> ---
>  drivers/mmc/host/mmci_stm32_sdmmc.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c
> index 14f99d8aa3f0..2965b1c062e1 100644
> --- a/drivers/mmc/host/mmci_stm32_sdmmc.c
> +++ b/drivers/mmc/host/mmci_stm32_sdmmc.c
> @@ -188,6 +188,9 @@ static int sdmmc_idma_start(struct mmci_host *host, unsigned int *datactrl)
>  static void sdmmc_idma_finalize(struct mmci_host *host, struct mmc_data *data)
>  {
>         writel_relaxed(0, host->base + MMCI_STM32_IDMACTRLR);
> +
> +       if (!data->host_cookie)
> +               sdmmc_idma_unprep_data(host, data, 0);
>  }
>
>  static void mmci_sdmmc_set_clkreg(struct mmci_host *host, unsigned int desired)
> --
> 2.17.1
>

^ permalink raw reply

* Re: [PATCH v2 3/3] dt-bindings: mmc: convert arasan sdhci bindings to yaml
From: Ulf Hansson @ 2020-05-28 10:14 UTC (permalink / raw)
  To: Wan Ahmad Zainie, Rob Herring
  Cc: Adrian Hunter, Michal Simek, linux-mmc@vger.kernel.org, DTML
In-Reply-To: <20200526062758.17642-4-wan.ahmad.zainie.wan.mohamad@intel.com>

On Tue, 26 May 2020 at 08:29, Wan Ahmad Zainie
<wan.ahmad.zainie.wan.mohamad@intel.com> wrote:
>
> Convert arasan,sdhci.txt file to yaml. The new file arasan,sdhci.yaml
> will inherit properties from mmc-controller.yaml. 'sdhci' is no longer
> a valid name for node and should be changed to 'mmc'.
>
> Suggested-by: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com>

I didn't queue this one yet, as I am would appreciate some feedback
from Rob first.

Kind regards
Uffe


> ---
>  .../devicetree/bindings/mmc/arasan,sdhci.txt  | 192 ------------
>  .../devicetree/bindings/mmc/arasan,sdhci.yaml | 293 ++++++++++++++++++
>  2 files changed, 293 insertions(+), 192 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
>  create mode 100644 Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml
>
> diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> deleted file mode 100644
> index f29bf7dd2ece..000000000000
> --- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> +++ /dev/null
> @@ -1,192 +0,0 @@
> -Device Tree Bindings for the Arasan SDHCI Controller
> -
> -  The bindings follow the mmc[1], clock[2], interrupt[3] and phy[4] bindings.
> -  Only deviations are documented here.
> -
> -  [1] Documentation/devicetree/bindings/mmc/mmc.txt
> -  [2] Documentation/devicetree/bindings/clock/clock-bindings.txt
> -  [3] Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
> -  [4] Documentation/devicetree/bindings/phy/phy-bindings.txt
> -
> -Required Properties:
> -  - compatible: Compatibility string.  One of:
> -    - "arasan,sdhci-8.9a": generic Arasan SDHCI 8.9a PHY
> -    - "arasan,sdhci-4.9a": generic Arasan SDHCI 4.9a PHY
> -    - "arasan,sdhci-5.1": generic Arasan SDHCI 5.1 PHY
> -    - "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1": rk3399 eMMC PHY
> -      For this device it is strongly suggested to include arasan,soc-ctl-syscon.
> -    - "xlnx,zynqmp-8.9a": ZynqMP SDHCI 8.9a PHY
> -      For this device it is strongly suggested to include clock-output-names and
> -      #clock-cells.
> -    - "xlnx,versal-8.9a": Versal SDHCI 8.9a PHY
> -      For this device it is strongly suggested to include clock-output-names and
> -      #clock-cells.
> -    - "ti,am654-sdhci-5.1", "arasan,sdhci-5.1": TI AM654 MMC PHY
> -       Note: This binding has been deprecated and moved to [5].
> -    - "intel,lgm-sdhci-5.1-emmc", "arasan,sdhci-5.1": Intel LGM eMMC PHY
> -      For this device it is strongly suggested to include arasan,soc-ctl-syscon.
> -    - "intel,lgm-sdhci-5.1-sdxc", "arasan,sdhci-5.1": Intel LGM SDXC PHY
> -      For this device it is strongly suggested to include arasan,soc-ctl-syscon.
> -    - "intel,keembay-sdhci-5.1-emmc", "arasan,sdhci-5.1": Intel Keem Bay eMMC
> -      For this device it is strongly suggested to include arasan,soc-ctl-syscon.
> -    - "intel,keembay-sdhci-5.1-sd": Intel Keem Bay SD controller
> -      For this device it is strongly suggested to include arasan,soc-ctl-syscon.
> -    - "intel,keembay-sdhci-5.1-sdio": Intel Keem Bay SDIO controller
> -      For this device it is strongly suggested to include arasan,soc-ctl-syscon.
> -
> -  [5] Documentation/devicetree/bindings/mmc/sdhci-am654.txt
> -
> -  - reg: From mmc bindings: Register location and length.
> -  - clocks: From clock bindings: Handles to clock inputs.
> -  - clock-names: From clock bindings: Tuple including "clk_xin" and "clk_ahb"
> -  - interrupts: Interrupt specifier
> -
> -Required Properties for "arasan,sdhci-5.1":
> -  - phys: From PHY bindings: Phandle for the Generic PHY for arasan.
> -  - phy-names:  MUST be "phy_arasan".
> -
> -Optional Properties:
> -  - arasan,soc-ctl-syscon: A phandle to a syscon device (see ../mfd/syscon.txt)
> -    used to access core corecfg registers.  Offsets of registers in this
> -    syscon are determined based on the main compatible string for the device.
> -  - clock-output-names: If specified, this will be the name of the card clock
> -    which will be exposed by this device.  Required if #clock-cells is
> -    specified.
> -  - #clock-cells: If specified this should be the value <0> or <1>. With this
> -    property in place we will export one or two clocks representing the Card
> -    Clock. These clocks are expected to be consumed by our PHY.
> -  - xlnx,fails-without-test-cd: when present, the controller doesn't work when
> -    the CD line is not connected properly, and the line is not connected
> -    properly. Test mode can be used to force the controller to function.
> -  - xlnx,int-clock-stable-broken: when present, the controller always reports
> -    that the internal clock is stable even when it is not.
> -
> -  - xlnx,mio-bank: When specified, this will indicate the MIO bank number in
> -    which the command and data lines are configured. If not specified, driver
> -    will assume this as 0.
> -
> -Example:
> -       sdhci@e0100000 {
> -               compatible = "arasan,sdhci-8.9a";
> -               reg = <0xe0100000 0x1000>;
> -               clock-names = "clk_xin", "clk_ahb";
> -               clocks = <&clkc 21>, <&clkc 32>;
> -               interrupt-parent = <&gic>;
> -               interrupts = <0 24 4>;
> -       } ;
> -
> -       sdhci@e2800000 {
> -               compatible = "arasan,sdhci-5.1";
> -               reg = <0xe2800000 0x1000>;
> -               clock-names = "clk_xin", "clk_ahb";
> -               clocks = <&cru 8>, <&cru 18>;
> -               interrupt-parent = <&gic>;
> -               interrupts = <0 24 4>;
> -               phys = <&emmc_phy>;
> -               phy-names = "phy_arasan";
> -       } ;
> -
> -       sdhci: sdhci@fe330000 {
> -               compatible = "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1";
> -               reg = <0x0 0xfe330000 0x0 0x10000>;
> -               interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
> -               clocks = <&cru SCLK_EMMC>, <&cru ACLK_EMMC>;
> -               clock-names = "clk_xin", "clk_ahb";
> -               arasan,soc-ctl-syscon = <&grf>;
> -               assigned-clocks = <&cru SCLK_EMMC>;
> -               assigned-clock-rates = <200000000>;
> -               clock-output-names = "emmc_cardclock";
> -               phys = <&emmc_phy>;
> -               phy-names = "phy_arasan";
> -               #clock-cells = <0>;
> -       };
> -
> -       sdhci: mmc@ff160000 {
> -               compatible = "xlnx,zynqmp-8.9a", "arasan,sdhci-8.9a";
> -               interrupt-parent = <&gic>;
> -               interrupts = <0 48 4>;
> -               reg = <0x0 0xff160000 0x0 0x1000>;
> -               clocks = <&clk200>, <&clk200>;
> -               clock-names = "clk_xin", "clk_ahb";
> -               clock-output-names = "clk_out_sd0", "clk_in_sd0";
> -               #clock-cells = <1>;
> -               clk-phase-sd-hs = <63>, <72>;
> -       };
> -
> -       sdhci: mmc@f1040000 {
> -               compatible = "xlnx,versal-8.9a", "arasan,sdhci-8.9a";
> -               interrupt-parent = <&gic>;
> -               interrupts = <0 126 4>;
> -               reg = <0x0 0xf1040000 0x0 0x10000>;
> -               clocks = <&clk200>, <&clk200>;
> -               clock-names = "clk_xin", "clk_ahb";
> -               clock-output-names = "clk_out_sd0", "clk_in_sd0";
> -               #clock-cells = <1>;
> -               clk-phase-sd-hs = <132>, <60>;
> -       };
> -
> -       emmc: sdhci@ec700000 {
> -               compatible = "intel,lgm-sdhci-5.1-emmc", "arasan,sdhci-5.1";
> -               reg = <0xec700000 0x300>;
> -               interrupt-parent = <&ioapic1>;
> -               interrupts = <44 1>;
> -               clocks = <&cgu0 LGM_CLK_EMMC5>, <&cgu0 LGM_CLK_NGI>,
> -                        <&cgu0 LGM_GCLK_EMMC>;
> -               clock-names = "clk_xin", "clk_ahb", "gate";
> -               clock-output-names = "emmc_cardclock";
> -               #clock-cells = <0>;
> -               phys = <&emmc_phy>;
> -               phy-names = "phy_arasan";
> -               arasan,soc-ctl-syscon = <&sysconf>;
> -       };
> -
> -       sdxc: sdhci@ec600000 {
> -               compatible = "arasan,sdhci-5.1", "intel,lgm-sdhci-5.1-sdxc";
> -               reg = <0xec600000 0x300>;
> -               interrupt-parent = <&ioapic1>;
> -               interrupts = <43 1>;
> -               clocks = <&cgu0 LGM_CLK_SDIO>, <&cgu0 LGM_CLK_NGI>,
> -                        <&cgu0 LGM_GCLK_SDXC>;
> -               clock-names = "clk_xin", "clk_ahb", "gate";
> -               clock-output-names = "sdxc_cardclock";
> -               #clock-cells = <0>;
> -               phys = <&sdxc_phy>;
> -               phy-names = "phy_arasan";
> -               arasan,soc-ctl-syscon = <&sysconf>;
> -       };
> -
> -       mmc: mmc@33000000 {
> -               compatible = "intel,keembay-sdhci-5.1-emmc", "arasan,sdhci-5.1";
> -               interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
> -               reg = <0x0 0x33000000 0x0 0x300>;
> -               clock-names = "clk_xin", "clk_ahb";
> -               clocks = <&scmi_clk KEEM_BAY_PSS_AUX_EMMC>,
> -                        <&scmi_clk KEEM_BAY_PSS_EMMC>;
> -               phys = <&emmc_phy>;
> -               phy-names = "phy_arasan";
> -               assigned-clocks = <&scmi_clk KEEM_BAY_PSS_AUX_EMMC>;
> -               assigned-clock-rates = <200000000>;
> -               clock-output-names = "emmc_cardclock";
> -               #clock-cells = <0>;
> -               arasan,soc-ctl-syscon = <&mmc_phy_syscon>;
> -       };
> -
> -       sd0: mmc@31000000 {
> -               compatible = "intel,keembay-sdhci-5.1-sd";
> -               interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>;
> -               reg = <0x0 0x31000000 0x0 0x300>;
> -               clock-names = "clk_xin", "clk_ahb";
> -               clocks = <&scmi_clk KEEM_BAY_PSS_AUX_SD0>,
> -                        <&scmi_clk KEEM_BAY_PSS_SD0>;
> -               arasan,soc-ctl-syscon = <&sd0_phy_syscon>;
> -       };
> -
> -       sd1: mmc@32000000 {
> -               compatible = "intel,keembay-sdhci-5.1-sdio";
> -               interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>;
> -               reg = <0x0 0x32000000 0x0 0x300>;
> -               clock-names = "clk_xin", "clk_ahb";
> -               clocks = <&scmi_clk KEEM_BAY_PSS_AUX_SD1>,
> -                        <&scmi_clk KEEM_BAY_PSS_SD1>;
> -               arasan,soc-ctl-syscon = <&sd1_phy_syscon>;
> -       };
> diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml b/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml
> new file mode 100644
> index 000000000000..927e2f13958b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml
> @@ -0,0 +1,293 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/mmc/arasan,sdhci.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Device Tree Bindings for the Arasan SDHCI Controller
> +
> +allOf:
> +  - $ref: "mmc-controller.yaml#"
> +
> +maintainers:
> +  - Adrian Hunter <adrian.hunter@intel.com>
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - const: arasan,sdhci-8.9a                # generic Arasan SDHCI 8.9a PHY
> +      - const: arasan,sdhci-4.9a                # generic Arasan SDHCI 4.9a PHY
> +      - const: arasan,sdhci-5.1                 # generic Arasan SDHCI 5.1 PHY
> +      - items:
> +          - const: rockchip,rk3399-sdhci-5.1    # rk3399 eMMC PHY
> +          - const: arasan,sdhci-5.1
> +        description: |
> +          For this device it is strongly suggested to include
> +          arasan,soc-ctl-syscon.
> +      - items:
> +          - const: xlnx,zynqmp-8.9a             # ZynqMP SDHCI 8.9a PHY
> +          - const: arasan,sdhci-8.9a
> +        description: |
> +          For this device it is strongly suggested to include
> +          clock-output-names and '#clock-cells'.
> +      - items:
> +          - const: xlnx,versal-8.9a             # Versal SDHCI 8.9a PHY
> +          - const: arasan,sdhci-8.9a
> +        description: |
> +          For this device it is strongly suggested to include
> +          clock-output-names and '#clock-cells'.
> +      - items:
> +          - const: intel,lgm-sdhci-5.1-emmc     # Intel LGM eMMC PHY
> +          - const: arasan,sdhci-5.1
> +        description: |
> +          For this device it is strongly suggested to include
> +          arasan,soc-ctl-syscon.
> +      - items:
> +          - const: intel,lgm-sdhci-5.1-sdxc     # Intel LGM SDXC PHY
> +          - const: arasan,sdhci-5.1
> +        description: |
> +          For this device it is strongly suggested to include
> +          arasan,soc-ctl-syscon.
> +      - items:
> +          - const: intel,keembay-sdhci-5.1-emmc # Intel Keem Bay eMMC PHY
> +          - const: arasan,sdhci-5.1
> +        description: |
> +          For this device it is strongly suggested to include
> +          arasan,soc-ctl-syscon.
> +      - const: intel,keembay-sdhci-5.1-sd       # Intel Keem Bay SD controller
> +        description: |
> +          For this device it is strongly suggested to include
> +          arasan,soc-ctl-syscon.
> +      - const: intel,keembay-sdhci-5.1-sdio     # Intel Keem Bay SDIO controller
> +        description: |
> +          For this device it is strongly suggested to include
> +          arasan,soc-ctl-syscon.
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    minItems: 2
> +    maxItems: 3
> +
> +  clock-names:
> +    minItems: 2
> +    items:
> +      - const: clk_xin
> +      - const: clk_ahb
> +      - const: gate
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  phys:
> +    maxItems: 1
> +
> +  phy-names:
> +    const: phy_arasan
> +
> +  arasan,soc-ctl-syscon:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: |
> +      A phandle to a syscon device (see ../mfd/syscon.txt) used to access
> +      core corecfg registers. Offsets of registers in this syscon are
> +      determined based on the main compatible string for the device.
> +
> +  clock-output-names:
> +    description: |
> +      If specified, this will be the name of the card clock which will
> +      be exposed by this device. Required if '#clock-cells' is specified.
> +
> +  '#clock-cells':
> +    enum: [0, 1]
> +    description: |
> +      With this property in place we will export one or two clocks
> +      representing the Card Clock. These clocks are expected to be
> +      consumed by our PHY.
> +
> +  xlnx,fails-without-test-cd:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description: |
> +      When present, the controller doesn't work when the CD line is not
> +      connected properly, and the line is not connected properly.
> +      Test mode can be used to force the controller to function.
> +
> +  xlnx,int-clock-stable-broken:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description: |
> +      When present, the controller always reports that the internal clock
> +      is stable even when it is not.
> +
> +  xlnx,mio-bank:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      When specified, this will indicate the MIO bank number in which
> +      the command and data lines are configured. If not specified, driver
> +      will assume this as 0.
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +
> +if:
> +  properties:
> +    compatible:
> +      contains:
> +        const: arasan,sdhci-5.1
> +then:
> +  required:
> +    - phys
> +    - phy-names
> +
> +examples:
> +  - |
> +    mmc@e0100000 {
> +          compatible = "arasan,sdhci-8.9a";
> +          reg = <0xe0100000 0x1000>;
> +          clock-names = "clk_xin", "clk_ahb";
> +          clocks = <&clkc 21>, <&clkc 32>;
> +          interrupt-parent = <&gic>;
> +          interrupts = <0 24 4>;
> +    };
> +
> +  - |
> +    mmc@e2800000 {
> +          compatible = "arasan,sdhci-5.1";
> +          reg = <0xe2800000 0x1000>;
> +          clock-names = "clk_xin", "clk_ahb";
> +          clocks = <&cru 8>, <&cru 18>;
> +          interrupt-parent = <&gic>;
> +          interrupts = <0 24 4>;
> +          phys = <&emmc_phy>;
> +          phy-names = "phy_arasan";
> +    };
> +
> +  - |
> +    #include <dt-bindings/clock/rk3399-cru.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    mmc@fe330000 {
> +          compatible = "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1";
> +          reg = <0x0 0xfe330000 0x0 0x10000>;
> +          interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
> +          clocks = <&cru SCLK_EMMC>, <&cru ACLK_EMMC>;
> +          clock-names = "clk_xin", "clk_ahb";
> +          arasan,soc-ctl-syscon = <&grf>;
> +          assigned-clocks = <&cru SCLK_EMMC>;
> +          assigned-clock-rates = <200000000>;
> +          clock-output-names = "emmc_cardclock";
> +          phys = <&emmc_phy>;
> +          phy-names = "phy_arasan";
> +          #clock-cells = <0>;
> +    };
> +
> +  - |
> +    mmc@ff160000 {
> +          compatible = "xlnx,zynqmp-8.9a", "arasan,sdhci-8.9a";
> +          interrupt-parent = <&gic>;
> +          interrupts = <0 48 4>;
> +          reg = <0x0 0xff160000 0x0 0x1000>;
> +          clocks = <&clk200>, <&clk200>;
> +          clock-names = "clk_xin", "clk_ahb";
> +          clock-output-names = "clk_out_sd0", "clk_in_sd0";
> +          #clock-cells = <1>;
> +          clk-phase-sd-hs = <63 72>;
> +    };
> +
> +  - |
> +    mmc@f1040000 {
> +          compatible = "xlnx,versal-8.9a", "arasan,sdhci-8.9a";
> +          interrupt-parent = <&gic>;
> +          interrupts = <0 126 4>;
> +          reg = <0x0 0xf1040000 0x0 0x10000>;
> +          clocks = <&clk200>, <&clk200>;
> +          clock-names = "clk_xin", "clk_ahb";
> +          clock-output-names = "clk_out_sd0", "clk_in_sd0";
> +          #clock-cells = <1>;
> +          clk-phase-sd-hs = <132>, <60>;
> +    };
> +
> +  - |
> +    #define LGM_CLK_EMMC5
> +    #define LGM_CLK_NGI
> +    #define LGM_GCLK_EMMC
> +    mmc@ec700000 {
> +          compatible = "intel,lgm-sdhci-5.1-emmc", "arasan,sdhci-5.1";
> +          reg = <0xec700000 0x300>;
> +          interrupt-parent = <&ioapic1>;
> +          interrupts = <44 1>;
> +          clocks = <&cgu0 LGM_CLK_EMMC5>, <&cgu0 LGM_CLK_NGI>,
> +                   <&cgu0 LGM_GCLK_EMMC>;
> +          clock-names = "clk_xin", "clk_ahb", "gate";
> +          clock-output-names = "emmc_cardclock";
> +          #clock-cells = <0>;
> +          phys = <&emmc_phy>;
> +          phy-names = "phy_arasan";
> +          arasan,soc-ctl-syscon = <&sysconf>;
> +    };
> +
> +  - |
> +    #define LGM_CLK_SDIO
> +    #define LGM_GCLK_SDXC
> +    mmc@ec600000 {
> +          compatible = "intel,lgm-sdhci-5.1-sdxc", "arasan,sdhci-5.1";
> +          reg = <0xec600000 0x300>;
> +          interrupt-parent = <&ioapic1>;
> +          interrupts = <43 1>;
> +          clocks = <&cgu0 LGM_CLK_SDIO>, <&cgu0 LGM_CLK_NGI>,
> +                   <&cgu0 LGM_GCLK_SDXC>;
> +          clock-names = "clk_xin", "clk_ahb", "gate";
> +          clock-output-names = "sdxc_cardclock";
> +          #clock-cells = <0>;
> +          phys = <&sdxc_phy>;
> +          phy-names = "phy_arasan";
> +          arasan,soc-ctl-syscon = <&sysconf>;
> +    };
> +
> +  - |
> +    #define KEEM_BAY_PSS_AUX_EMMC
> +    #define KEEM_BAY_PSS_EMMC
> +    mmc@33000000 {
> +          compatible = "intel,keembay-sdhci-5.1-emmc", "arasan,sdhci-5.1";
> +          interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
> +          reg = <0x0 0x33000000 0x0 0x300>;
> +          clock-names = "clk_xin", "clk_ahb";
> +          clocks = <&scmi_clk KEEM_BAY_PSS_AUX_EMMC>,
> +                   <&scmi_clk KEEM_BAY_PSS_EMMC>;
> +          phys = <&emmc_phy>;
> +          phy-names = "phy_arasan";
> +          assigned-clocks = <&scmi_clk KEEM_BAY_PSS_AUX_EMMC>;
> +          assigned-clock-rates = <200000000>;
> +          clock-output-names = "emmc_cardclock";
> +          #clock-cells = <0>;
> +          arasan,soc-ctl-syscon = <&mmc_phy_syscon>;
> +    };
> +
> +  - |
> +    #define KEEM_BAY_PSS_AUX_SD0
> +    #define KEEM_BAY_PSS_SD0
> +    mmc@31000000 {
> +          compatible = "intel,keembay-sdhci-5.1-sd";
> +          interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>;
> +          reg = <0x0 0x31000000 0x0 0x300>;
> +          clock-names = "clk_xin", "clk_ahb";
> +          clocks = <&scmi_clk KEEM_BAY_PSS_AUX_SD0>,
> +                   <&scmi_clk KEEM_BAY_PSS_SD0>;
> +          arasan,soc-ctl-syscon = <&sd0_phy_syscon>;
> +    };
> +
> +  - |
> +    #define KEEM_BAY_PSS_AUX_SD1
> +    #define KEEM_BAY_PSS_SD1
> +    mmc@32000000 {
> +          compatible = "intel,keembay-sdhci-5.1-sdio";
> +          interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>;
> +          reg = <0x0 0x32000000 0x0 0x300>;
> +          clock-names = "clk_xin", "clk_ahb";
> +          clocks = <&scmi_clk KEEM_BAY_PSS_AUX_SD1>,
> +                   <&scmi_clk KEEM_BAY_PSS_SD1>;
> +          arasan,soc-ctl-syscon = <&sd1_phy_syscon>;
> +    };
> --
> 2.17.1
>

^ permalink raw reply

* RE: [PATCH 0/4] Change i.MX/MXS SoCs ocotp/iim node name to efuse
From: Andy Duan @ 2020-05-28 10:23 UTC (permalink / raw)
  To: Anson Huang, robh+dt@kernel.org, shawnguo@kernel.org,
	s.hauer@pengutronix.de, kernel@pengutronix.de, festevam@gmail.com,
	Daniel Baluta, Leonard Crestez, Peng Fan, aford173@gmail.com,
	Jun Li, S.j. Wang, Horia Geanta, Aisheng Dong, agx@sigxcpu.org,
	l.stach@pengutronix.de, andrew.smirnov@gmail.com,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
  Cc: dl-linux-imx
In-Reply-To: <1590635570-8541-1-git-send-email-Anson.Huang@nxp.com>

From: Anson Huang <Anson.Huang@nxp.com> Sent: Thursday, May 28, 2020 11:13 AM
> In the nvmem yaml schema, it requires the nodename to be one of
> "eeprom|efuse|nvram", so need to change all i.MX/MXS SoCs ocotp/iim node
> name to efuse:
> 
> MXS platforms: i.MX23/28;
> i.MX platforms with IIM: i.MX25/27/31/35/51/53.
> i.MX ARMv7 platforms with OCOTP: i.MX6QDL/6SL/6SX/6SLL/6UL/7S/7ULP.
> i.MX ARMv8 platforms with OCOTP: i.MX8MQ/8MM/8MN/8MP.

Reviewed-by: Fugang Duan <fugang.duan@nxp.com>
> 
> Anson Huang (4):
>   ARM: dts: imx: change ocotp node name on i.MX6/7 SoCs
>   arm64: dts: imx8m: change ocotp node name on i.MX8M SoCs
>   ARM: dts: imx: change ocotp node name on MXS SoCs
>   ARM: dts: imx: change iim node name on i.MX SoCs
> 
>  arch/arm/boot/dts/imx23.dtsi              | 2 +-
>  arch/arm/boot/dts/imx25.dtsi              | 2 +-
>  arch/arm/boot/dts/imx27.dtsi              | 2 +-
>  arch/arm/boot/dts/imx28.dtsi              | 2 +-
>  arch/arm/boot/dts/imx31.dtsi              | 2 +-
>  arch/arm/boot/dts/imx35.dtsi              | 2 +-
>  arch/arm/boot/dts/imx51.dtsi              | 2 +-
>  arch/arm/boot/dts/imx53.dtsi              | 2 +-
>  arch/arm/boot/dts/imx6qdl.dtsi            | 2 +-
>  arch/arm/boot/dts/imx6sl.dtsi             | 2 +-
>  arch/arm/boot/dts/imx6sll.dtsi            | 2 +-
>  arch/arm/boot/dts/imx6sx.dtsi             | 2 +-
>  arch/arm/boot/dts/imx6ul.dtsi             | 2 +-
>  arch/arm/boot/dts/imx7s.dtsi              | 2 +-
>  arch/arm/boot/dts/imx7ulp.dtsi            | 2 +-
>  arch/arm64/boot/dts/freescale/imx8mm.dtsi | 2 +-
> arch/arm64/boot/dts/freescale/imx8mn.dtsi | 2 +-
> arch/arm64/boot/dts/freescale/imx8mp.dtsi | 2 +-
> arch/arm64/boot/dts/freescale/imx8mq.dtsi | 2 +-
>  19 files changed, 19 insertions(+), 19 deletions(-)
> 
> --
> 2.7.4


^ permalink raw reply

* [V6 PATCH 1/2] dt-bindings: Added device tree binding for max98390
From: Steve Lee @ 2020-05-28 10:37 UTC (permalink / raw)
  To: lgirdwood, broonie, robh+dt, alsa-devel, devicetree, linux-kernel
  Cc: ryan.lee.maxim, ryans.lee, steves.lee.maxim, Steve Lee

Add DT binding of max98390 amplifier driver.

Signed-off-by: Steve Lee <steves.lee@maximintegrated.com>
---
Changed since V5:
	* Change txt to yaml and fix up the examples.
Changed since V4:
	* No changes.
Changed since V3:
	* No changes.
Changed since V2:
	* No changes.
Changed since V1:
	* Modified sample text in example

 .../bindings/sound/maxim,max98390.yaml        | 39 +++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/maxim,max98390.yaml

diff --git a/Documentation/devicetree/bindings/sound/maxim,max98390.yaml b/Documentation/devicetree/bindings/sound/maxim,max98390.yaml
new file mode 100644
index 000000000000..1ed4ab9e1c37
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/maxim,max98390.yaml
@@ -0,0 +1,39 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/maxim,max98390.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Maxim Integrated MAX98390 Speaker Amplifier with Integrated Dynamic Speaker Management
+
+maintainers:
+  - Steve Lee <steves.lee@maximintegrated.com>
+
+properties:
+  compatible:
+      const: maxim,max98390
+
+  reg:
+    maxItems: 1
+    description: I2C address of the device.
+
+  temperature_calib:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: The calculated temperature data was measured while doing the calibration. Data : Temp / 100 * 2^12
+
+  r0_calib:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: This is r0 calibration data which was measured in factory mode.
+
+required:
+  - compatible
+  - reg
+
+examples:
+  - |
+    max98390: amplifier@38 {
+            compatible = "maxim,max98390";
+            reg = <0x38>;
+            maxim,temperature_calib = <1024>;
+            maxim,r0_calib = <100232>;
+    };
-- 
2.17.1


^ permalink raw reply related

* Re: [Freedreno] [PATCH 5/6] drm: msm: a6xx: use dev_pm_opp_set_bw to set DDR bandwidth
From: Sharat Masetty @ 2020-05-28 11:02 UTC (permalink / raw)
  To: Rob Clark
  Cc: freedreno,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	dri-devel, linux-arm-msm, Linux Kernel Mailing List,
	Georgi Djakov, Matthias Kaehlcke, Viresh Kumar, saravanak,
	Sibi Sankar, Rajendra Nayak, Jordan Crouse
In-Reply-To: <CAF6AEGvOtgpHMuiw01QgRYGEBB2rp5QOdVMpkTMsi0c-QSSv1Q@mail.gmail.com>


On 5/27/2020 9:08 PM, Rob Clark wrote:
> On Wed, May 27, 2020 at 1:47 AM Sharat Masetty <smasetty@codeaurora.org> wrote:
>> + more folks
>>
>> On 5/18/2020 9:55 PM, Rob Clark wrote:
>>> On Mon, May 18, 2020 at 7:23 AM Jordan Crouse <jcrouse@codeaurora.org> wrote:
>>>> On Thu, May 14, 2020 at 04:24:18PM +0530, Sharat Masetty wrote:
>>>>> This patches replaces the previously used static DDR vote and uses
>>>>> dev_pm_opp_set_bw() to scale GPU->DDR bandwidth along with scaling
>>>>> GPU frequency.
>>>>>
>>>>> Signed-off-by: Sharat Masetty <smasetty@codeaurora.org>
>>>>> ---
>>>>>    drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 6 +-----
>>>>>    1 file changed, 1 insertion(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
>>>>> index 2d8124b..79433d3 100644
>>>>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
>>>>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
>>>>> @@ -141,11 +141,7 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct dev_pm_opp *opp)
>>>>>
>>>>>         gmu->freq = gmu->gpu_freqs[perf_index];
>>>>>
>>>>> -     /*
>>>>> -      * Eventually we will want to scale the path vote with the frequency but
>>>>> -      * for now leave it at max so that the performance is nominal.
>>>>> -      */
>>>>> -     icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216));
>>>>> +     dev_pm_opp_set_bw(&gpu->pdev->dev, opp);
>>>>>    }
>>>> This adds an implicit requirement that all targets need bandwidth settings
>>>> defined in the OPP or they won't get a bus vote at all. I would prefer that
>>>> there be an default escape valve but if not you'll need to add
>>>> bandwidth values for the sdm845 OPP that target doesn't regress.
>>>>
>>> it looks like we could maybe do something like:
>>>
>>>     ret = dev_pm_opp_set_bw(...);
>>>     if (ret) {
>>>         dev_warn_once(dev, "no bandwidth settings");
>>>         icc_set_bw(...);
>>>     }
>>>
>>> ?
>>>
>>> BR,
>>> -R
>> There is a bit of an issue here - Looks like its not possible to two icc
>> handles to the same path.  Its causing double enumeration of the paths
>> in the icc core and messing up path votes. With [1] Since opp/core
>> already gets a handle to the icc path as part of table add,  drm/msm
>> could do either
>>
>> a) Conditionally enumerate gpu->icc_path handle only when pm/opp core
>> has not got the icc path handle. I could use something like [2] to
>> determine if should initialize gpu->icc_path*
>>
>> b) Add peak-opp-configs in 845 dt and mandate all future versions to use
>> this bindings. With this, I can remove gpu->icc_path from msm/drm
>> completely and only rely on opp/core for bw voting.
> The main thing is that we want to make sure newer dtb always works on
> an older kernel without regression.. but, hmm..  I guess the
> interconnects/interconnects-names properties haven't landed yet in
> sdm845.dtsi?  Maybe that lets us go with the simpler approach (b).
> Looks like we haven't wired up interconnect for 8916 or 8996 either,
> so probably we can just mandate this for all of them?

I checked all three 845, 820 and 8916 and none of them have the 
interconnect configs for GPU. So, I think we are good here. I'll go with 
option (b) and re-spin v3. Adding interconnects and opp-peak-kBps 
configs for previous chips can be taken up as a separate activity.

Sharat

> If we have landed the interconnect dts hookup for gpu somewhere that
> I'm overlooking, I guess we would have to go with (a) and keep the
> existing interconnects/interconnects-names properties.
>
> BR,
> -R
>
>> [1] - https://lore.kernel.org/patchwork/cover/1240687/
>>
>> [2] - https://patchwork.kernel.org/patch/11527573/
>>
>> Let me know your thoughts
>>
>> Sharat
>>
>>>> Jordan
>>>>
>>>>>    unsigned long a6xx_gmu_get_freq(struct msm_gpu *gpu)
>>>>> --
>>>>> 2.7.4
>>>>>
>>>> --
>>>> The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>>>> a Linux Foundation Collaborative Project
>>>> _______________________________________________
>>>> Freedreno mailing list
>>>> Freedreno@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/freedreno

^ permalink raw reply

* Re: [PATCH] of/platform: Avoid compilation warning
From: Miquel Raynal @ 2020-05-28 11:03 UTC (permalink / raw)
  To: Rob Herring; +Cc: Mark Rutland, devicetree, Thomas Petazzoni, linux-kernel
In-Reply-To: <20200527183555.GA2512243@bogus>

Hi Rob,

Rob Herring <robh@kernel.org> wrote on Wed, 27 May 2020 12:35:55 -0600:

> On Thu, May 14, 2020 at 07:07:07PM +0200, Miquel Raynal wrote:
> > The of_find_device_by_node() helper has its dummy counterpart for when
> > CONFIG_OF is not enabled. However, it is clearly stated in the kernel
> > documentation that it "takes a reference to the embedded struct device
> > which needs to be dropped after use". Which means the of_dev_put()
> > helper might have to be called afterwards. Unfortunately, there is no
> > of_dev_put() dummy function if OF_CONFIG is not enabled which seems
> > odd in this case. The of_dev_put() helper is pretty simple, it just
> > checks the validity of the single argument and calls put_device() on
> > it. One can just call put_device() directly to avoid any build issue
> > but I find much more accurate in this case to create the dummy
> > helper.
> > 
> > With this helper, a file using of_find_device_by_node() can also call
> > of_dev_put() without triggering the following:  
> 
> IMO, you should use platform_device_put() instead. It has the NULL check 
> too.
> 
> I imagine of_dev_put() is left over from when OF devices were not 
> platform devices. 

Ok, makes sense. Perhaps we should entirely get rid of it, I don't see
a lot of users left. Or at least update the comment I was mentioning.

Cheers,
Miquèl

^ permalink raw reply

* Re: [PATCH] usb/phy-generic: Add support for OTG VBUS supply control
From: Peter Chen @ 2020-05-28 11:20 UTC (permalink / raw)
  To: Mike Looijmans
  Cc: linux-usb@vger.kernel.org, gregkh@linuxfoundation.org,
	robh+dt@kernel.org, balbi@kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20200526145051.31520-1-mike.looijmans@topic.nl>

On 20-05-26 16:50:51, Mike Looijmans wrote:
> This enables support for VBUS on boards where the power is supplied
> by a regulator. The regulator is enabled when the USB port enters
> HOST mode.
> 

Why you don't do this at your host controller driver?

Peter
> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
> ---
>  .../devicetree/bindings/usb/usb-nop-xceiv.txt |  3 ++
>  drivers/usb/phy/phy-generic.c                 | 44 ++++++++++++++++++-
>  drivers/usb/phy/phy-generic.h                 |  2 +
>  3 files changed, 48 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
> index 4dc6a8ee3071..775a19fdb613 100644
> --- a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
> +++ b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
> @@ -16,6 +16,9 @@ Optional properties:
>  
>  - vcc-supply: phandle to the regulator that provides power to the PHY.
>  
> +- vbus-supply: phandle to the regulator that provides the VBUS power for when
> +  the device is in HOST mode.
> +
>  - reset-gpios: Should specify the GPIO for reset.
>  
>  - vbus-detect-gpio: should specify the GPIO detecting a VBus insertion
> diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c
> index 661a229c105d..ebfb90764511 100644
> --- a/drivers/usb/phy/phy-generic.c
> +++ b/drivers/usb/phy/phy-generic.c
> @@ -203,13 +203,43 @@ static int nop_set_host(struct usb_otg *otg, struct usb_bus *host)
>  	return 0;
>  }
>  
> +static int nop_set_vbus(struct usb_otg *otg, bool enabled)
> +{
> +	struct usb_phy_generic *nop;
> +	int ret;
> +
> +	if (!otg)
> +		return -ENODEV;
> +
> +	nop = container_of(otg->usb_phy, struct usb_phy_generic, phy);
> +
> +	if (!nop->vbus_reg)
> +		return 0;
> +
> +	if (enabled) {
> +		if (nop->vbus_reg_enabled)
> +			return 0;
> +		ret = regulator_enable(nop->vbus_reg);
> +		if (ret < 0)
> +			return ret;
> +		nop->vbus_reg_enabled = true;
> +	} else {
> +		if (!nop->vbus_reg_enabled)
> +			return 0;
> +		ret = regulator_disable(nop->vbus_reg);
> +		if (ret < 0)
> +			return ret;
> +		nop->vbus_reg_enabled = false;
> +	}
> +}
> +
>  int usb_phy_gen_create_phy(struct device *dev, struct usb_phy_generic *nop)
>  {
>  	enum usb_phy_type type = USB_PHY_TYPE_USB2;
>  	int err = 0;
>  
>  	u32 clk_rate = 0;
> -	bool needs_vcc = false, needs_clk = false;
> +	bool needs_vcc = false, needs_clk = false, needs_vbus = false;
>  
>  	if (dev->of_node) {
>  		struct device_node *node = dev->of_node;
> @@ -219,6 +249,7 @@ int usb_phy_gen_create_phy(struct device *dev, struct usb_phy_generic *nop)
>  
>  		needs_vcc = of_property_read_bool(node, "vcc-supply");
>  		needs_clk = of_property_read_bool(node, "clocks");
> +		needs_vbus = of_property_read_bool(node, "vbus-supply");
>  	}
>  	nop->gpiod_reset = devm_gpiod_get_optional(dev, "reset",
>  						   GPIOD_ASIS);
> @@ -268,6 +299,16 @@ int usb_phy_gen_create_phy(struct device *dev, struct usb_phy_generic *nop)
>  			return -EPROBE_DEFER;
>  	}
>  
> +	nop->vbus_reg = devm_regulator_get(dev, "vbus");
> +	if (IS_ERR(nop->vbus_reg)) {
> +		dev_dbg(dev, "Error getting vbus regulator: %ld\n",
> +					PTR_ERR(nop->vbus_reg));
> +		if (needs_vbus)
> +			return -EPROBE_DEFER;
> +
> +		nop->vbus_reg = NULL;
> +	}
> +
>  	nop->dev		= dev;
>  	nop->phy.dev		= nop->dev;
>  	nop->phy.label		= "nop-xceiv";
> @@ -278,6 +319,7 @@ int usb_phy_gen_create_phy(struct device *dev, struct usb_phy_generic *nop)
>  	nop->phy.otg->usb_phy		= &nop->phy;
>  	nop->phy.otg->set_host		= nop_set_host;
>  	nop->phy.otg->set_peripheral	= nop_set_peripheral;
> +	nop->phy.otg->set_vbus		= nop_set_vbus;
>  
>  	return 0;
>  }
> diff --git a/drivers/usb/phy/phy-generic.h b/drivers/usb/phy/phy-generic.h
> index 7ee80211a688..a3663639ea1e 100644
> --- a/drivers/usb/phy/phy-generic.h
> +++ b/drivers/usb/phy/phy-generic.h
> @@ -14,7 +14,9 @@ struct usb_phy_generic {
>  	struct gpio_desc *gpiod_reset;
>  	struct gpio_desc *gpiod_vbus;
>  	struct regulator *vbus_draw;
> +	struct regulator *vbus_reg;
>  	bool vbus_draw_enabled;
> +	bool vbus_reg_enabled;
>  	unsigned long mA;
>  	unsigned int vbus;
>  };
> -- 
> 2.17.1
> 

-- 

Thanks,
Peter Chen

^ permalink raw reply

* [PATCH v6 02/18] mtd: rawnand: Create a new enumeration to describe OOB placement
From: Miquel Raynal @ 2020-05-28 11:30 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd,
	Rob Herring, Mark Rutland, devicetree
  Cc: Boris Brezillon, Thomas Petazzoni, Paul Cercueil, Chuanhong Guo,
	Weijie Gao, linux-arm-kernel, Mason Yang, Julien Su,
	Miquel Raynal
In-Reply-To: <20200528113113.9166-1-miquel.raynal@bootlin.com>

There is currently a confusion between the ECC type/mode/provider
(eg. on-host, software, on-die or none) and the ECC bytes placement.

Create a new enumeration to describe this placement.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/mtd/nand/raw/nand_base.c |  5 +++++
 include/linux/mtd/rawnand.h      | 14 ++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index bd3f5a875e39..4d2d444f9db9 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -5018,6 +5018,11 @@ static const char * const nand_ecc_modes[] = {
 	[NAND_ECC_ON_DIE]	= "on-die",
 };
 
+static const char * const nand_ecc_placement[] = {
+	[NAND_ECC_PLACEMENT_OOB] = "oob",
+	[NAND_ECC_PLACEMENT_INTERLEAVED] = "interleaved",
+};
+
 static int of_get_nand_ecc_mode(struct device_node *np)
 {
 	const char *pm;
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 65b1c1c18b41..5e014807e050 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -92,6 +92,20 @@ enum nand_ecc_mode {
 	NAND_ECC_ON_DIE,
 };
 
+/**
+ * enum nand_ecc_placement - NAND ECC bytes placement
+ * @NAND_ECC_PLACEMENT_UNKNOWN: The actual position of the ECC bytes is unknown
+ * @NAND_ECC_PLACEMENT_OOB: The ECC bytes are located in the OOB area
+ * @NAND_ECC_PLACEMENT_INTERLEAVED: Syndrome layout, there are ECC bytes
+ *                                  interleaved with regular data in the main
+ *                                  area
+ */
+enum nand_ecc_placement {
+	NAND_ECC_PLACEMENT_UNKNOWN,
+	NAND_ECC_PLACEMENT_OOB,
+	NAND_ECC_PLACEMENT_INTERLEAVED,
+};
+
 enum nand_ecc_algo {
 	NAND_ECC_UNKNOWN,
 	NAND_ECC_HAMMING,
-- 
2.20.1


^ permalink raw reply related

* [PATCH v6 03/18] mtd: rawnand: Separate the ECC engine type and the OOB placement
From: Miquel Raynal @ 2020-05-28 11:30 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd,
	Rob Herring, Mark Rutland, devicetree
  Cc: Boris Brezillon, Thomas Petazzoni, Paul Cercueil, Chuanhong Guo,
	Weijie Gao, linux-arm-kernel, Mason Yang, Julien Su,
	Miquel Raynal
In-Reply-To: <20200528113113.9166-1-miquel.raynal@bootlin.com>

The use of "syndrome" placement should not be encoded in the ECC
engine mode/type.

Create a "placement" field in NAND chip and change all occurrences of
the NAND_ECC_HW_SYNDROME enumeration to be just NAND_ECC_HW and
possibly a placement entry like NAND_ECC_PLACEMENT_INTERLEAVED.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 arch/arm/mach-davinci/board-dm355-leopard.c |   3 +-
 drivers/mtd/nand/raw/cafe_nand.c            |   3 +-
 drivers/mtd/nand/raw/davinci_nand.c         |   5 +-
 drivers/mtd/nand/raw/denali.c               |   3 +-
 drivers/mtd/nand/raw/diskonchip.c           |   3 +-
 drivers/mtd/nand/raw/lpc32xx_slc.c          |   3 +-
 drivers/mtd/nand/raw/nand_base.c            | 109 +++++++++++---------
 drivers/mtd/nand/raw/r852.c                 |   3 +-
 include/linux/mtd/rawnand.h                 |   6 +-
 include/linux/platform_data/mtd-davinci.h   |   1 +
 10 files changed, 81 insertions(+), 58 deletions(-)

diff --git a/arch/arm/mach-davinci/board-dm355-leopard.c b/arch/arm/mach-davinci/board-dm355-leopard.c
index b9e9950dd300..4c8a592754ac 100644
--- a/arch/arm/mach-davinci/board-dm355-leopard.c
+++ b/arch/arm/mach-davinci/board-dm355-leopard.c
@@ -76,7 +76,8 @@ static struct davinci_nand_pdata davinci_nand_data = {
 	.mask_chipsel		= BIT(14),
 	.parts			= davinci_nand_partitions,
 	.nr_parts		= ARRAY_SIZE(davinci_nand_partitions),
-	.ecc_mode		= NAND_ECC_HW_SYNDROME,
+	.ecc_mode		= NAND_HW_ECC_ENGINE,
+	.ecc_placement		= NAND_ECC_PLACEMENT_INTERLEAVED,
 	.ecc_bits		= 4,
 	.bbt_options		= NAND_BBT_USE_FLASH,
 };
diff --git a/drivers/mtd/nand/raw/cafe_nand.c b/drivers/mtd/nand/raw/cafe_nand.c
index 92173790f20b..2bf8ab542e38 100644
--- a/drivers/mtd/nand/raw/cafe_nand.c
+++ b/drivers/mtd/nand/raw/cafe_nand.c
@@ -629,7 +629,8 @@ static int cafe_nand_attach_chip(struct nand_chip *chip)
 		goto out_free_dma;
 	}
 
-	cafe->nand.ecc.mode = NAND_ECC_HW_SYNDROME;
+	cafe->nand.ecc.mode = NAND_ECC_HW;
+	cafe->nand.ecc.placement = NAND_ECC_PLACEMENT_INTERLEAVED;
 	cafe->nand.ecc.size = mtd->writesize;
 	cafe->nand.ecc.bytes = 14;
 	cafe->nand.ecc.strength = 4;
diff --git a/drivers/mtd/nand/raw/davinci_nand.c b/drivers/mtd/nand/raw/davinci_nand.c
index d975a62caaa5..2e5d6c113b56 100644
--- a/drivers/mtd/nand/raw/davinci_nand.c
+++ b/drivers/mtd/nand/raw/davinci_nand.c
@@ -168,7 +168,7 @@ static int nand_davinci_correct_1bit(struct nand_chip *chip, u_char *dat,
 /*
  * 4-bit hardware ECC ... context maintained over entire AEMIF
  *
- * This is a syndrome engine, but we avoid NAND_ECC_HW_SYNDROME
+ * This is a syndrome engine, but we avoid NAND_ECC_PLACEMENT_INTERLEAVED
  * since that forces use of a problematic "infix OOB" layout.
  * Among other things, it trashes manufacturer bad block markers.
  * Also, and specific to this hardware, it ECC-protects the "prepad"
@@ -851,6 +851,7 @@ static int nand_davinci_probe(struct platform_device *pdev)
 
 	/* Use board-specific ECC config */
 	info->chip.ecc.mode	= pdata->ecc_mode;
+	info->chip.ecc.placement = pdata->ecc_placement;
 
 	spin_lock_irq(&davinci_nand_lock);
 
@@ -897,7 +898,7 @@ static int nand_davinci_remove(struct platform_device *pdev)
 	int ret;
 
 	spin_lock_irq(&davinci_nand_lock);
-	if (info->chip.ecc.mode == NAND_ECC_HW_SYNDROME)
+	if (info->chip.ecc.placement == NAND_ECC_PLACEMENT_INTERLEAVED)
 		ecc4_busy = false;
 	spin_unlock_irq(&davinci_nand_lock);
 
diff --git a/drivers/mtd/nand/raw/denali.c b/drivers/mtd/nand/raw/denali.c
index 4e6e1578aa2d..514a97ea4450 100644
--- a/drivers/mtd/nand/raw/denali.c
+++ b/drivers/mtd/nand/raw/denali.c
@@ -1237,7 +1237,8 @@ int denali_chip_init(struct denali_controller *denali,
 	chip->bbt_options |= NAND_BBT_USE_FLASH;
 	chip->bbt_options |= NAND_BBT_NO_OOB;
 	chip->options |= NAND_NO_SUBPAGE_WRITE;
-	chip->ecc.mode = NAND_ECC_HW_SYNDROME;
+	chip->ecc.mode = NAND_ECC_HW;
+	chip->ecc.placement = NAND_ECC_PLACEMENT_INTERLEAVED;
 	chip->ecc.read_page = denali_read_page;
 	chip->ecc.write_page = denali_write_page;
 	chip->ecc.read_page_raw = denali_read_page_raw;
diff --git a/drivers/mtd/nand/raw/diskonchip.c b/drivers/mtd/nand/raw/diskonchip.c
index 43721863a0d8..40360352136b 100644
--- a/drivers/mtd/nand/raw/diskonchip.c
+++ b/drivers/mtd/nand/raw/diskonchip.c
@@ -1456,7 +1456,8 @@ static int __init doc_probe(unsigned long physadr)
 	nand->ecc.calculate	= doc200x_calculate_ecc;
 	nand->ecc.correct	= doc200x_correct_data;
 
-	nand->ecc.mode		= NAND_ECC_HW_SYNDROME;
+	nand->ecc.mode		= NAND_ECC_HW;
+	nand->ecc.placement	= NAND_ECC_PLACEMENT_INTERLEAVED;
 	nand->ecc.size		= 512;
 	nand->ecc.bytes		= 6;
 	nand->ecc.strength	= 2;
diff --git a/drivers/mtd/nand/raw/lpc32xx_slc.c b/drivers/mtd/nand/raw/lpc32xx_slc.c
index b151fd000815..ccb189c8e343 100644
--- a/drivers/mtd/nand/raw/lpc32xx_slc.c
+++ b/drivers/mtd/nand/raw/lpc32xx_slc.c
@@ -881,7 +881,8 @@ static int lpc32xx_nand_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, host);
 
 	/* NAND callbacks for LPC32xx SLC hardware */
-	chip->ecc.mode = NAND_ECC_HW_SYNDROME;
+	chip->ecc.mode = NAND_ECC_HW;
+	chip->ecc.placement = NAND_ECC_PLACEMENT_INTERLEAVED;
 	chip->legacy.read_byte = lpc32xx_nand_read_byte;
 	chip->legacy.read_buf = lpc32xx_nand_read_buf;
 	chip->legacy.write_buf = lpc32xx_nand_write_buf;
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 4d2d444f9db9..9fbd2a474b62 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -5772,61 +5772,74 @@ static int nand_scan_tail(struct nand_chip *chip)
 
 	switch (ecc->mode) {
 	case NAND_ECC_HW:
-		/* Use standard hwecc read page function? */
-		if (!ecc->read_page)
-			ecc->read_page = nand_read_page_hwecc;
-		if (!ecc->write_page)
-			ecc->write_page = nand_write_page_hwecc;
-		if (!ecc->read_page_raw)
-			ecc->read_page_raw = nand_read_page_raw;
-		if (!ecc->write_page_raw)
-			ecc->write_page_raw = nand_write_page_raw;
-		if (!ecc->read_oob)
-			ecc->read_oob = nand_read_oob_std;
-		if (!ecc->write_oob)
-			ecc->write_oob = nand_write_oob_std;
-		if (!ecc->read_subpage)
-			ecc->read_subpage = nand_read_subpage;
-		if (!ecc->write_subpage && ecc->hwctl && ecc->calculate)
-			ecc->write_subpage = nand_write_subpage_hwecc;
-		fallthrough;
-	case NAND_ECC_HW_SYNDROME:
-		if ((!ecc->calculate || !ecc->correct || !ecc->hwctl) &&
-		    (!ecc->read_page ||
-		     ecc->read_page == nand_read_page_hwecc ||
-		     !ecc->write_page ||
-		     ecc->write_page == nand_write_page_hwecc)) {
-			WARN(1, "No ECC functions supplied; hardware ECC not possible\n");
-			ret = -EINVAL;
-			goto err_nand_manuf_cleanup;
-		}
-		/* Use standard syndrome read/write page function? */
-		if (!ecc->read_page)
-			ecc->read_page = nand_read_page_syndrome;
-		if (!ecc->write_page)
-			ecc->write_page = nand_write_page_syndrome;
-		if (!ecc->read_page_raw)
-			ecc->read_page_raw = nand_read_page_raw_syndrome;
-		if (!ecc->write_page_raw)
-			ecc->write_page_raw = nand_write_page_raw_syndrome;
-		if (!ecc->read_oob)
-			ecc->read_oob = nand_read_oob_syndrome;
-		if (!ecc->write_oob)
-			ecc->write_oob = nand_write_oob_syndrome;
+		switch (ecc->placement) {
+		case NAND_ECC_PLACEMENT_UNKNOWN:
+		case NAND_ECC_PLACEMENT_OOB:
+			/* Use standard hwecc read page function? */
+			if (!ecc->read_page)
+				ecc->read_page = nand_read_page_hwecc;
+			if (!ecc->write_page)
+				ecc->write_page = nand_write_page_hwecc;
+			if (!ecc->read_page_raw)
+				ecc->read_page_raw = nand_read_page_raw;
+			if (!ecc->write_page_raw)
+				ecc->write_page_raw = nand_write_page_raw;
+			if (!ecc->read_oob)
+				ecc->read_oob = nand_read_oob_std;
+			if (!ecc->write_oob)
+				ecc->write_oob = nand_write_oob_std;
+			if (!ecc->read_subpage)
+				ecc->read_subpage = nand_read_subpage;
+			if (!ecc->write_subpage && ecc->hwctl && ecc->calculate)
+				ecc->write_subpage = nand_write_subpage_hwecc;
+			fallthrough;
 
-		if (mtd->writesize >= ecc->size) {
-			if (!ecc->strength) {
-				WARN(1, "Driver must set ecc.strength when using hardware ECC\n");
+		case NAND_ECC_PLACEMENT_INTERLEAVED:
+			if ((!ecc->calculate || !ecc->correct || !ecc->hwctl) &&
+			    (!ecc->read_page ||
+			     ecc->read_page == nand_read_page_hwecc ||
+			     !ecc->write_page ||
+			     ecc->write_page == nand_write_page_hwecc)) {
+				WARN(1, "No ECC functions supplied; hardware ECC not possible\n");
 				ret = -EINVAL;
 				goto err_nand_manuf_cleanup;
 			}
+			/* Use standard syndrome read/write page function? */
+			if (!ecc->read_page)
+				ecc->read_page = nand_read_page_syndrome;
+			if (!ecc->write_page)
+				ecc->write_page = nand_write_page_syndrome;
+			if (!ecc->read_page_raw)
+				ecc->read_page_raw = nand_read_page_raw_syndrome;
+			if (!ecc->write_page_raw)
+				ecc->write_page_raw = nand_write_page_raw_syndrome;
+			if (!ecc->read_oob)
+				ecc->read_oob = nand_read_oob_syndrome;
+			if (!ecc->write_oob)
+				ecc->write_oob = nand_write_oob_syndrome;
+
+			if (mtd->writesize >= ecc->size) {
+				if (!ecc->strength) {
+					WARN(1, "Driver must set ecc.strength when using hardware ECC\n");
+					ret = -EINVAL;
+					goto err_nand_manuf_cleanup;
+				}
+				break;
+			}
+			pr_warn("%d byte HW ECC not possible on %d byte page size, fallback to SW ECC\n",
+				ecc->size, mtd->writesize);
+			ecc->mode = NAND_ECC_SOFT;
+			ecc->algo = NAND_ECC_HAMMING;
 			break;
+
+		default:
+			pr_warn("Invalid NAND_ECC_PLACEMENT %d\n",
+				ecc->placement);
+			ret = -EINVAL;
+			goto err_nand_manuf_cleanup;
 		}
-		pr_warn("%d byte HW ECC not possible on %d byte page size, fallback to SW ECC\n",
-			ecc->size, mtd->writesize);
-		ecc->mode = NAND_ECC_SOFT;
-		ecc->algo = NAND_ECC_HAMMING;
 		fallthrough;
+
 	case NAND_ECC_SOFT:
 		ret = nand_set_ecc_soft_ops(chip);
 		if (ret) {
diff --git a/drivers/mtd/nand/raw/r852.c b/drivers/mtd/nand/raw/r852.c
index f865e3a47b01..f0988cda4479 100644
--- a/drivers/mtd/nand/raw/r852.c
+++ b/drivers/mtd/nand/raw/r852.c
@@ -859,7 +859,8 @@ static int  r852_probe(struct pci_dev *pci_dev, const struct pci_device_id *id)
 	chip->legacy.write_buf = r852_write_buf;
 
 	/* ecc */
-	chip->ecc.mode = NAND_ECC_HW_SYNDROME;
+	chip->ecc.mode = NAND_ECC_HW;
+	chip->ecc.placement = NAND_ECC_PLACEMENT_INTERLEAVED;
 	chip->ecc.size = R852_DMA_LEN;
 	chip->ecc.bytes = SM_OOB_SIZE;
 	chip->ecc.strength = 2;
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 5e014807e050..f6ffd174abb7 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -325,6 +325,7 @@ static const struct nand_ecc_caps __name = {			\
 /**
  * struct nand_ecc_ctrl - Control structure for ECC
  * @mode:	ECC mode
+ * @placement:	OOB bytes placement
  * @algo:	ECC algorithm
  * @steps:	number of ECC steps per page
  * @size:	data bytes per ECC step
@@ -352,7 +353,7 @@ static const struct nand_ecc_caps __name = {			\
  *			controller and always return contiguous in-band and
  *			out-of-band data even if they're not stored
  *			contiguously on the NAND chip (e.g.
- *			NAND_ECC_HW_SYNDROME interleaves in-band and
+ *			NAND_ECC_PLACEMENT_INTERLEAVED interleaves in-band and
  *			out-of-band data).
  * @write_page_raw:	function to write a raw page without ECC. This function
  *			should hide the specific layout used by the ECC
@@ -360,7 +361,7 @@ static const struct nand_ecc_caps __name = {			\
  *			in-band and out-of-band data. ECC controller is
  *			responsible for doing the appropriate transformations
  *			to adapt to its specific layout (e.g.
- *			NAND_ECC_HW_SYNDROME interleaves in-band and
+ *			NAND_ECC_PLACEMENT_INTERLEAVED interleaves in-band and
  *			out-of-band data).
  * @read_page:	function to read a page according to the ECC generator
  *		requirements; returns maximum number of bitflips corrected in
@@ -377,6 +378,7 @@ static const struct nand_ecc_caps __name = {			\
  */
 struct nand_ecc_ctrl {
 	enum nand_ecc_mode mode;
+	enum nand_ecc_placement placement;
 	enum nand_ecc_algo algo;
 	int steps;
 	int size;
diff --git a/include/linux/platform_data/mtd-davinci.h b/include/linux/platform_data/mtd-davinci.h
index 03e92c71b3fa..3383101c233b 100644
--- a/include/linux/platform_data/mtd-davinci.h
+++ b/include/linux/platform_data/mtd-davinci.h
@@ -69,6 +69,7 @@ struct davinci_nand_pdata {		/* platform_data */
 	 * using it with large page chips.
 	 */
 	enum nand_ecc_mode	ecc_mode;
+	enum nand_ecc_placement	ecc_placement;
 	u8			ecc_bits;
 
 	/* e.g. NAND_BUSWIDTH_16 */
-- 
2.20.1


^ permalink raw reply related

* [PATCH v6 01/18] dt-bindings: mtd: Document nand-ecc-placement
From: Miquel Raynal @ 2020-05-28 11:30 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd,
	Rob Herring, Mark Rutland, devicetree
  Cc: Boris Brezillon, Thomas Petazzoni, Paul Cercueil, Chuanhong Guo,
	Weijie Gao, linux-arm-kernel, Mason Yang, Julien Su,
	Miquel Raynal
In-Reply-To: <20200528113113.9166-1-miquel.raynal@bootlin.com>

This optional property defines where the ECC bytes are expected to be
stored. No value defaults to an unknown location, while these
locations can be explicitly set to OOB or interleaved depending if
the ECC bytes are entirely stored in the OOB area or mixed with
regular data in the main area (also sometimes referred as
"syndrome").

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 .../devicetree/bindings/mtd/nand-controller.yaml       | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/nand-controller.yaml b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
index d261b7096c69..4a0798247d2d 100644
--- a/Documentation/devicetree/bindings/mtd/nand-controller.yaml
+++ b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
@@ -56,6 +56,16 @@ patternProperties:
           (Linux will handle the calculations). soft_bch is deprecated
           and should be replaced by soft and nand-ecc-algo.
 
+      nand-ecc-placement:
+        allOf:
+          - $ref: /schemas/types.yaml#/definitions/string
+          - enum: [ oob, interleaved ]
+        description:
+          Location of the ECC bytes. This location is unknown by default
+          but can be explicitly set to "oob", if all ECC bytes are
+          known to be stored in the OOB area, or "interleaved" if ECC
+          bytes will be interleaved with regular data in the main area.
+
       nand-ecc-algo:
         allOf:
           - $ref: /schemas/types.yaml#/definitions/string
-- 
2.20.1


^ permalink raw reply related

* [PATCH v6 04/18] mtd: rawnand: Create a helper to retrieve the ECC placement
From: Miquel Raynal @ 2020-05-28 11:30 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd,
	Rob Herring, Mark Rutland, devicetree
  Cc: Boris Brezillon, Thomas Petazzoni, Paul Cercueil, Chuanhong Guo,
	Weijie Gao, linux-arm-kernel, Mason Yang, Julien Su,
	Miquel Raynal
In-Reply-To: <20200528113113.9166-1-miquel.raynal@bootlin.com>

Use it from nand_dt_init() to initialize the ECC structure.

This allows the deprecation of the hw_syndrome ECC mode.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/nand_base.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 9fbd2a474b62..fd0bfe9bf7ae 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -5047,6 +5047,34 @@ static int of_get_nand_ecc_mode(struct device_node *np)
 	return -ENODEV;
 }
 
+enum nand_ecc_placement of_get_nand_ecc_placement(struct device_node *np)
+{
+	enum nand_ecc_placement placement;
+	const char *pm;
+	int err;
+
+	err = of_property_read_string(np, "nand-ecc-placement", &pm);
+	if (!err) {
+		for (placement = NAND_ECC_PLACEMENT_INTERLEAVED;
+		     placement < ARRAY_SIZE(nand_ecc_placement); placement++) {
+			if (!strcasecmp(pm, nand_ecc_placement[placement]))
+				return placement;
+		}
+	}
+
+	/*
+	 * For backward compatibility we support few obsoleted values that don't
+	 * have their mappings into the nand_ecc_placement enum anymore.
+	 */
+	err = of_property_read_string(np, "nand-ecc-mode", &pm);
+	if (!err) {
+		if (!strcasecmp(pm, "hw_syndrome"))
+			return NAND_ECC_PLACEMENT_INTERLEAVED;
+	}
+
+	return NAND_ECC_PLACEMENT_UNKNOWN;
+}
+
 static const char * const nand_ecc_algos[] = {
 	[NAND_ECC_HAMMING]	= "hamming",
 	[NAND_ECC_BCH]		= "bch",
@@ -5143,6 +5171,7 @@ static int nand_dt_init(struct nand_chip *chip)
 
 	ecc_mode = of_get_nand_ecc_mode(dn);
 	ecc_algo = of_get_nand_ecc_algo(dn);
+	chip->ecc.placement = of_get_nand_ecc_placement(dn);
 	ecc_strength = of_get_nand_ecc_strength(dn);
 	ecc_step = of_get_nand_ecc_step_size(dn);
 
-- 
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