Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v2 4/4] docs: counter: Document character device interface
From: Pavel Machek @ 2020-05-29 13:26 UTC (permalink / raw)
  To: William Breathitt Gray
  Cc: kamel.bouhara, gwendal, david, linux-iio, patrick.havelange,
	alexandre.belloni, linux-kernel, linux-arm-kernel,
	mcoquelin.stm32, fabrice.gasnier, syednwaris, linux-stm32, jic23,
	alexandre.torgue
In-Reply-To: <db0a9206d31c82f8381316ef5ff9872bfb53665b.1589654470.git.vilhelm.gray@gmail.com>

On Sat 2020-05-16 15:20:02, William Breathitt Gray wrote:
> This patch adds high-level documentation about the Counter subsystem
> character device interface.
> 
> Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
> ---
>  Documentation/driver-api/generic-counter.rst | 112 +++++++++++++------
>  1 file changed, 76 insertions(+), 36 deletions(-)
> 
> diff --git a/Documentation/driver-api/generic-counter.rst b/Documentation/driver-api/generic-counter.rst
> index 8f85c30dea0b..58045b33b576 100644
> --- a/Documentation/driver-api/generic-counter.rst
> +++ b/Documentation/driver-api/generic-counter.rst

> +
> +Counter chrdev
> +--------------
> +Translates counter data to the standard Counter character device; data
> +is transferred via standard character device read/write calls.
> +
> +Sysfs Interface
> +===============
> +
> +Several sysfs attributes are generated by the Generic Counter interface,
> +and reside under the `/sys/bus/counter/devices/counterX` directory,
> +where `X` is to the respective counter device id. Please see
> +Documentation/ABI/testing/sysfs-bus-counter for detailed information on
> +each Generic Counter interface sysfs attribute.
> +
> +Through these sysfs attributes, programs and scripts may interact with
> +the Generic Counter paradigm Counts, Signals, and Synapses of respective
> +counter devices.
> +
> +Counter Character Device
> +========================
> +
> +Counter character device nodes are created under the `/dev` directory as
> +`counterX`, where `X` is the respective counter device id. Defines for
> +the standard Counter data types are exposed via the userspace
> +`include/uapi/linux/counter-types.h` file.
> +
> +The first 196095 bytes of the character device serve as a control
> +selection area where control exposure of desired Counter components and
> +extensions may be selected. Each byte serves as a boolean selection
> +indicator for a respective Counter component or extension. The format of
> +this area is as follows:
> +
> +* For each device extension, a byte is required.
> +* For each Signal, a byte is reserved for the Signal component, and a
> +  byte is reserved for each Signal extension.
> +* For each Count, a byte is reserved for the Count component, a byte is
> +  reserved for the count function, a byte is reserved for each Synapse
> +  action, and byte is reserved for each Count extension.
> +
> +The selected Counter components and extensions may then be interfaced
> +after the first 196095 bytes via standard character device read/write
> +operations. The number of bytes available for each component or
> +extension is dependent on their respective data type: u8 will have 1
> +byte available, u64 will have 8 bytes available, strings will have 64
> +bytes available, etc.

This looks like very, very strange interface, and not described in detail
required to understand it.

Could you take a look at input subsystem, /dev/input/event0? Perhaps it is 
directly usable, and if not something similar should probably be acceptable.

Best regards,
									Pavel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v3 1/9] dt-bindings: atmel-tcb: convert bindings to json-schema
From: Alexandre Belloni @ 2020-05-29 13:21 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: devicetree, kamel.bouhara, Daniel Lezcano, linux-kernel,
	Rob Herring, Thomas Gleixner, linux-arm-kernel
In-Reply-To: <20200529101314.2ueuhgnrqq3a764f@linutronix.de>

Hi,

On 29/05/2020 12:13:14+0200, Sebastian Andrzej Siewior wrote:
> Rob, could you please bless the DT parts of this series? Daniel Lezcano
> asked for the blessing in:
>   https://lkml.kernel.org/r/f0feb409-11fb-08de-cc06-216a16de994a@linaro.org
> 

There is actually one comment I need to address that Rob made on another
series that was also including this patch. I'll send a new version
today.

> On 2020-05-06 10:05:46 [+0200], Alexandre Belloni wrote:
> > Convert Atmel Timer Counter Blocks bindings to DT schema format using
> > json-schema.
> > 
> > Also move it out of mfd as it is not and has never been related to mfd.
> > 
> > Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> > ---
> > Cc: Rob Herring <robh+dt@kernel.org>
> > 
> > Changes in v3:
> >  - Moved the child node documentation to the parent documentation
> > 
> > Changes in v2:
> >  - Rebased on v5.7-rc1
> >  - Moved the binding documentation to its proper place
> >  - Added back the atmel,tcb-timer child node documentation
> > 
> > 
> >  .../devicetree/bindings/mfd/atmel-tcb.txt     |  56 --------
> >  .../soc/microchip/atmel,at91rm9200-tcb.yaml   | 126 ++++++++++++++++++
> 
> Sebastian

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr
From: Herbert Xu @ 2020-05-29 13:19 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Eric Biggers, Stephan Mueller, Linux Crypto Mailing List,
	Linux ARM
In-Reply-To: <CAMj1kXF2-jJ6yGh9m759V2858_c45txoUBgKirvR-4z4GOHUfQ@mail.gmail.com>

On Fri, May 29, 2020 at 03:10:43PM +0200, Ard Biesheuvel wrote:
>
> OK, so the undocumented assumption is that algif_skcipher requests are
> delineated by ALG_SET_IV commands, and that anything that gets sent to
> the socket in between should be treated as a single request, right? I

Correct.

> think that makes sense, but do note that this deviates from Stephan's
> use case, where the ciphertext stealing block swap was applied after
> every call into af_alg, with the IV being inherited from one request
> to the next. I think that case was invalid to begin with, I just hope
> no other use cases exist where this unspecified behavior is being
> relied upon.

That does indeed sound broken.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH v8 5/5] mtd: rawnand: Move the user input parsing bits to the ECC framework
From: Miquel Raynal @ 2020-05-29 13:16 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd,
	Rob Herring, Mark Rutland, devicetree
  Cc: Julien Su, Weijie Gao, Paul Cercueil, Boris Brezillon,
	Thomas Petazzoni, Miquel Raynal, Mason Yang, Chuanhong Guo,
	linux-arm-kernel
In-Reply-To: <20200529131602.21532-1-miquel.raynal@bootlin.com>

Many helpers are generic to all NAND chips, they should not be
restricted to be only used by raw NAND controller drivers. They might
later be used by generic ECC engines and SPI-NAND devices as well so
move them into a more generic place.

To avoid moving all the raw NAND core "history" into the generic NAND
layer, we already moved certain bits into legacy helpers in the raw
NAND core to ensure backward compatibility.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/ecc.c                       | 138 +++++++++++++++++
 drivers/mtd/nand/raw/atmel/nand-controller.c |   3 +-
 drivers/mtd/nand/raw/denali.c                |   3 +
 drivers/mtd/nand/raw/nand_base.c             | 150 ++-----------------
 drivers/mtd/nand/raw/sunxi_nand.c            |   3 +-
 drivers/mtd/nand/raw/tegra_nand.c            |   5 +-
 include/linux/mtd/nand.h                     |   7 +
 include/linux/mtd/rawnand.h                  |   1 -
 8 files changed, 166 insertions(+), 144 deletions(-)

diff --git a/drivers/mtd/nand/ecc.c b/drivers/mtd/nand/ecc.c
index ad08a047dfc5..1ac7aaa6c6c2 100644
--- a/drivers/mtd/nand/ecc.c
+++ b/drivers/mtd/nand/ecc.c
@@ -328,6 +328,144 @@ const struct mtd_ooblayout_ops *nand_get_large_page_hamming_ooblayout(void)
 }
 EXPORT_SYMBOL_GPL(nand_get_large_page_hamming_ooblayout);
 
+static enum nand_ecc_engine_type
+of_get_nand_ecc_engine_type(struct device_node *np)
+{
+	return NAND_ECC_ENGINE_TYPE_INVALID;
+}
+
+static const char * const nand_ecc_placement[] = {
+	[NAND_ECC_PLACEMENT_OOB] = "oob",
+	[NAND_ECC_PLACEMENT_INTERLEAVED] = "interleaved",
+};
+
+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_OOB;
+		     placement < ARRAY_SIZE(nand_ecc_placement); placement++) {
+			if (!strcasecmp(pm, nand_ecc_placement[placement]))
+				return placement;
+		}
+	}
+
+	return NAND_ECC_PLACEMENT_UNKNOWN;
+}
+
+static const char * const nand_ecc_algos[] = {
+	[NAND_ECC_ALGO_HAMMING]	= "hamming",
+	[NAND_ECC_ALGO_BCH]	= "bch",
+	[NAND_ECC_ALGO_RS]	= "rs",
+};
+
+static enum nand_ecc_algo of_get_nand_ecc_algo(struct device_node *np)
+{
+	enum nand_ecc_algo ecc_algo;
+	const char *pm;
+	int err;
+
+	err = of_property_read_string(np, "nand-ecc-algo", &pm);
+	if (!err) {
+		for (ecc_algo = NAND_ECC_ALGO_HAMMING;
+		     ecc_algo < ARRAY_SIZE(nand_ecc_algos);
+		     ecc_algo++) {
+			if (!strcasecmp(pm, nand_ecc_algos[ecc_algo]))
+				return ecc_algo;
+		}
+	}
+
+	return NAND_ECC_ALGO_UNKNOWN;
+}
+
+static int of_get_nand_ecc_step_size(struct device_node *np)
+{
+	int ret;
+	u32 val;
+
+	ret = of_property_read_u32(np, "nand-ecc-step-size", &val);
+	return ret ? ret : val;
+}
+
+static int of_get_nand_ecc_strength(struct device_node *np)
+{
+	int ret;
+	u32 val;
+
+	ret = of_property_read_u32(np, "nand-ecc-strength", &val);
+	return ret ? ret : val;
+}
+
+static inline bool of_get_nand_ecc_maximize(struct device_node *np)
+{
+	return of_property_read_bool(np, "nand-ecc-maximize");
+}
+
+void nand_ecc_read_user_conf(struct nand_device *nand)
+{
+	struct device_node *dn = nanddev_get_of_node(nand);
+	int strength, size;
+
+	nand->ecc.user_conf.engine_type = of_get_nand_ecc_engine_type(dn);
+	nand->ecc.user_conf.algo = of_get_nand_ecc_algo(dn);
+	nand->ecc.user_conf.placement = of_get_nand_ecc_placement(dn);
+
+	strength = of_get_nand_ecc_strength(dn);
+	if (strength >= 0)
+		nand->ecc.user_conf.strength = strength;
+
+	size = of_get_nand_ecc_step_size(dn);
+	if (size >= 0)
+		nand->ecc.user_conf.step_size = size;
+
+	if (of_get_nand_ecc_maximize(dn))
+		nand->ecc.user_conf.flags |= NAND_ECC_MAXIMIZE;
+}
+EXPORT_SYMBOL(nand_ecc_read_user_conf);
+
+/**
+ * nand_ecc_correction_is_enough - Check if the chip configuration meets the
+ *                                 datasheet requirements.
+ *
+ * @nand: Device to check
+ *
+ * If our configuration corrects A bits per B bytes and the minimum
+ * required correction level is X bits per Y bytes, then we must ensure
+ * both of the following are true:
+ *
+ * (1) A / B >= X / Y
+ * (2) A >= X
+ *
+ * Requirement (1) ensures we can correct for the required bitflip density.
+ * Requirement (2) ensures we can correct even when all bitflips are clumped
+ * in the same sector.
+ */
+bool nand_ecc_correction_is_enough(struct nand_device *nand)
+{
+	const struct nand_ecc_props *reqs = nanddev_get_ecc_requirements(nand);
+	const struct nand_ecc_props *conf = nanddev_get_ecc_conf(nand);
+	struct mtd_info *mtd = nanddev_to_mtd(nand);
+	int corr, ds_corr;
+
+	if (conf->step_size == 0 || reqs->step_size == 0)
+		/* Not enough information */
+		return true;
+
+	/*
+	 * We get the number of corrected bits per page to compare
+	 * the correction density.
+	 */
+	corr = (mtd->writesize * conf->strength) / conf->step_size;
+	ds_corr = (mtd->writesize * reqs->strength) / reqs->step_size;
+
+	return corr >= ds_corr && conf->strength >= reqs->strength;
+}
+EXPORT_SYMBOL(nand_ecc_correction_is_enough);
+
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Miquel Raynal <miquel.raynal@bootlin.com>");
 MODULE_DESCRIPTION("Generic ECC engine");
diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
index 08df7f23b859..39d8fe15b8ab 100644
--- a/drivers/mtd/nand/raw/atmel/nand-controller.c
+++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
@@ -1046,6 +1046,7 @@ static int atmel_nand_pmecc_init(struct nand_chip *chip)
 	const struct nand_ecc_props *requirements =
 		nanddev_get_ecc_requirements(&chip->base);
 	struct mtd_info *mtd = nand_to_mtd(chip);
+	struct nand_device *nanddev = mtd_to_nanddev(mtd);
 	struct atmel_nand *nand = to_atmel_nand(chip);
 	struct atmel_nand_controller *nc;
 	struct atmel_pmecc_user_req req;
@@ -1070,7 +1071,7 @@ static int atmel_nand_pmecc_init(struct nand_chip *chip)
 			chip->ecc.size = val;
 	}
 
-	if (chip->ecc.options & NAND_ECC_MAXIMIZE)
+	if (nanddev->ecc.user_conf.flags & NAND_ECC_MAXIMIZE)
 		req.ecc.strength = ATMEL_PMECC_MAXIMIZE_ECC_STRENGTH;
 	else if (chip->ecc.strength)
 		req.ecc.strength = chip->ecc.strength;
diff --git a/drivers/mtd/nand/raw/denali.c b/drivers/mtd/nand/raw/denali.c
index a6a6464974ec..51bc014ebc0a 100644
--- a/drivers/mtd/nand/raw/denali.c
+++ b/drivers/mtd/nand/raw/denali.c
@@ -1181,6 +1181,7 @@ int denali_chip_init(struct denali_controller *denali,
 {
 	struct nand_chip *chip = &dchip->chip;
 	struct mtd_info *mtd = nand_to_mtd(chip);
+	struct nand_device *nanddev = mtd_to_nanddev(mtd);
 	struct denali_chip *dchip2;
 	int i, j, ret;
 
@@ -1248,6 +1249,8 @@ int denali_chip_init(struct denali_controller *denali,
 
 	mtd_set_ooblayout(mtd, &denali_ooblayout_ops);
 
+	nanddev->ecc.user_conf.flags |= NAND_ECC_MAXIMIZE;
+
 	ret = nand_scan(chip, dchip->nsels);
 	if (ret)
 		return ret;
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index afc3506468ba..036e88cb52a1 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -4854,17 +4854,6 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
 	return ret;
 }
 
-static const char * const nand_ecc_placement[] = {
-	[NAND_ECC_PLACEMENT_OOB] = "oob",
-	[NAND_ECC_PLACEMENT_INTERLEAVED] = "interleaved",
-};
-
-static enum nand_ecc_engine_type
-of_get_nand_ecc_engine_type(struct device_node *np)
-{
-	return NAND_ECC_ENGINE_TYPE_INVALID;
-}
-
 static enum nand_ecc_engine_type
 of_get_rawnand_ecc_engine_type_legacy(struct device_node *np)
 {
@@ -4916,24 +4905,6 @@ of_get_rawnand_ecc_engine_type_legacy(struct device_node *np)
 	return NAND_ECC_ENGINE_TYPE_INVALID;
 }
 
-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_OOB;
-		     placement < ARRAY_SIZE(nand_ecc_placement); placement++) {
-			if (!strcasecmp(pm, nand_ecc_placement[placement]))
-				return placement;
-		}
-	}
-
-	return NAND_ECC_PLACEMENT_UNKNOWN;
-}
-
 enum nand_ecc_placement
 of_get_rawnand_ecc_placement_legacy(struct device_node *np)
 {
@@ -4949,31 +4920,6 @@ of_get_rawnand_ecc_placement_legacy(struct device_node *np)
 	return NAND_ECC_PLACEMENT_UNKNOWN;
 }
 
-static const char * const nand_ecc_algos[] = {
-	[NAND_ECC_ALGO_HAMMING]	= "hamming",
-	[NAND_ECC_ALGO_BCH]	= "bch",
-	[NAND_ECC_ALGO_RS]	= "rs",
-};
-
-static enum nand_ecc_algo of_get_nand_ecc_algo(struct device_node *np)
-{
-	enum nand_ecc_algo ecc_algo;
-	const char *pm;
-	int err;
-
-	err = of_property_read_string(np, "nand-ecc-algo", &pm);
-	if (!err) {
-		for (ecc_algo = NAND_ECC_ALGO_HAMMING;
-		     ecc_algo < ARRAY_SIZE(nand_ecc_algos);
-		     ecc_algo++) {
-			if (!strcasecmp(pm, nand_ecc_algos[ecc_algo]))
-				return ecc_algo;
-		}
-	}
-
-	return NAND_ECC_ALGO_UNKNOWN;
-}
-
 static enum nand_ecc_algo of_get_rawnand_ecc_algo_legacy(struct device_node *np)
 {
 	const char *pm;
@@ -4990,48 +4936,10 @@ static enum nand_ecc_algo of_get_rawnand_ecc_algo_legacy(struct device_node *np)
 	return NAND_ECC_ALGO_UNKNOWN;
 }
 
-static int of_get_nand_ecc_step_size(struct device_node *np)
-{
-	int ret;
-	u32 val;
-
-	ret = of_property_read_u32(np, "nand-ecc-step-size", &val);
-	return ret ? ret : val;
-}
-
-static int of_get_nand_ecc_strength(struct device_node *np)
-{
-	int ret;
-	u32 val;
-
-	ret = of_property_read_u32(np, "nand-ecc-strength", &val);
-	return ret ? ret : val;
-}
-
-static void nand_ecc_read_user_conf(struct nand_chip *chip)
-{
-	struct device_node *dn = nand_get_flash_node(chip);
-	struct nand_device *nand = &chip->base;
-	int strength, size;
-
-	nand->ecc.user_conf.engine_type = of_get_nand_ecc_engine_type(dn);
-	nand->ecc.user_conf.algo = of_get_nand_ecc_algo(dn);
-	nand->ecc.user_conf.placement = of_get_nand_ecc_placement(dn);
-
-	strength = of_get_nand_ecc_strength(dn);
-	if (strength >= 0)
-		nand->ecc.user_conf.strength = strength;
-
-	size = of_get_nand_ecc_step_size(dn);
-	if (size >= 0)
-		nand->ecc.user_conf.step_size = size;
-}
-
 static void rawnand_ecc_read_legacy_user_conf(struct nand_chip *chip)
 {
 	struct device_node *dn = nand_get_flash_node(chip);
-	struct nand_device *nand = &chip->base;
-	struct nand_ecc_props *user_conf = &nand->ecc.user_conf;
+	struct nand_ecc_props *user_conf = &chip->base.ecc.user_conf;
 
 	if (user_conf->engine_type != NAND_ECC_ENGINE_TYPE_INVALID)
 		user_conf->engine_type = of_get_rawnand_ecc_engine_type_legacy(dn);
@@ -5081,10 +4989,7 @@ static int rawnand_dt_init(struct nand_chip *chip)
 	if (of_get_nand_on_flash_bbt(dn))
 		chip->bbt_options |= NAND_BBT_USE_FLASH;
 
-	if (of_property_read_bool(dn, "nand-ecc-maximize"))
-		chip->ecc.options |= NAND_ECC_MAXIMIZE;
-
-	nand_ecc_read_user_conf(chip);
+	nand_ecc_read_user_conf(nand);
 	rawnand_ecc_read_legacy_user_conf(chip);
 
 	/*
@@ -5214,6 +5119,7 @@ static void nand_scan_ident_cleanup(struct nand_chip *chip)
 static int nand_set_ecc_soft_ops(struct nand_chip *chip)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
+	struct nand_device *nanddev = mtd_to_nanddev(mtd);
 	struct nand_ecc_ctrl *ecc = &chip->ecc;
 
 	if (WARN_ON(ecc->engine_type != NAND_ECC_ENGINE_TYPE_SOFT))
@@ -5289,7 +5195,7 @@ static int nand_set_ecc_soft_ops(struct nand_chip *chip)
 		 * used.
 		 */
 		if (mtd->ooblayout == nand_get_large_page_ooblayout() &&
-		    ecc->options & NAND_ECC_MAXIMIZE) {
+		    nanddev->ecc.user_conf.flags & NAND_ECC_MAXIMIZE) {
 			int steps, bytes;
 
 			/* Always prefer 1k blocks over 512bytes ones */
@@ -5529,11 +5435,12 @@ nand_maximize_ecc(struct nand_chip *chip,
  * @caps: ECC engine caps info structure
  * @oobavail: OOB size that the ECC engine can use
  *
- * Choose the ECC configuration according to following logic
+ * Choose the ECC configuration according to following logic.
  *
  * 1. If both ECC step size and ECC strength are already set (usually by DT)
  *    then check if it is supported by this controller.
- * 2. If NAND_ECC_MAXIMIZE is set, then select maximum ECC strength.
+ * 2. If the user provided the nand-ecc-maximize property, then select maximum
+ *    ECC strength.
  * 3. Otherwise, try to match the ECC step size and ECC strength closest
  *    to the chip's requirement. If available OOB size can't fit the chip
  *    requirement then fallback to the maximum ECC step size and ECC strength.
@@ -5544,6 +5451,7 @@ int nand_ecc_choose_conf(struct nand_chip *chip,
 			 const struct nand_ecc_caps *caps, int oobavail)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
+	struct nand_device *nanddev = mtd_to_nanddev(mtd);
 
 	if (WARN_ON(oobavail < 0 || oobavail > mtd->oobsize))
 		return -EINVAL;
@@ -5551,7 +5459,7 @@ int nand_ecc_choose_conf(struct nand_chip *chip,
 	if (chip->ecc.size && chip->ecc.strength)
 		return nand_check_ecc_caps(chip, caps, oobavail);
 
-	if (chip->ecc.options & NAND_ECC_MAXIMIZE)
+	if (nanddev->ecc.user_conf.flags & NAND_ECC_MAXIMIZE)
 		return nand_maximize_ecc(chip, caps, oobavail);
 
 	if (!nand_match_ecc_req(chip, caps, oobavail))
@@ -5561,43 +5469,6 @@ int nand_ecc_choose_conf(struct nand_chip *chip,
 }
 EXPORT_SYMBOL_GPL(nand_ecc_choose_conf);
 
-/*
- * Check if the chip configuration meet the datasheet requirements.
-
- * If our configuration corrects A bits per B bytes and the minimum
- * required correction level is X bits per Y bytes, then we must ensure
- * both of the following are true:
- *
- * (1) A / B >= X / Y
- * (2) A >= X
- *
- * Requirement (1) ensures we can correct for the required bitflip density.
- * Requirement (2) ensures we can correct even when all bitflips are clumped
- * in the same sector.
- */
-static bool nand_ecc_strength_good(struct nand_chip *chip)
-{
-	struct mtd_info *mtd = nand_to_mtd(chip);
-	struct nand_ecc_ctrl *ecc = &chip->ecc;
-	const struct nand_ecc_props *requirements =
-		nanddev_get_ecc_requirements(&chip->base);
-	int corr, ds_corr;
-
-	if (ecc->size == 0 || requirements->step_size == 0)
-		/* Not enough information */
-		return true;
-
-	/*
-	 * We get the number of corrected bits per page to compare
-	 * the correction density.
-	 */
-	corr = (mtd->writesize * ecc->strength) / ecc->size;
-	ds_corr = (mtd->writesize * requirements->strength) /
-		  requirements->step_size;
-
-	return corr >= ds_corr && ecc->strength >= requirements->strength;
-}
-
 static int rawnand_erase(struct nand_device *nand, const struct nand_pos *pos)
 {
 	struct nand_chip *chip = container_of(nand, struct nand_chip,
@@ -5653,6 +5524,7 @@ static const struct nand_ops rawnand_ops = {
 static int nand_scan_tail(struct nand_chip *chip)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
+	struct nand_device *nanddev = mtd_to_nanddev(mtd);
 	struct nand_ecc_ctrl *ecc = &chip->ecc;
 	int ret, i;
 
@@ -5880,7 +5752,7 @@ static int nand_scan_tail(struct nand_chip *chip)
 	mtd->oobavail = ret;
 
 	/* ECC sanity check: warn if it's too weak */
-	if (!nand_ecc_strength_good(chip))
+	if (!nand_ecc_correction_is_enough(nanddev))
 		pr_warn("WARNING: %s: the ECC used on your system (%db/%dB) is too weak compared to the one required by the NAND chip (%db/%dB)\n",
 			mtd->name,
 			nanddev_get_ecc_conf(&chip->base)->strength,
diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
index 490ba485e939..f863fabb8610 100644
--- a/drivers/mtd/nand/raw/sunxi_nand.c
+++ b/drivers/mtd/nand/raw/sunxi_nand.c
@@ -1609,12 +1609,13 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct nand_chip *nand,
 	static const u8 strengths[] = { 16, 24, 28, 32, 40, 48, 56, 60, 64 };
 	struct sunxi_nfc *nfc = to_sunxi_nfc(nand->controller);
 	struct mtd_info *mtd = nand_to_mtd(nand);
+	struct nand_device *nanddev = mtd_to_nanddev(mtd);
 	struct sunxi_nand_hw_ecc *data;
 	int nsectors;
 	int ret;
 	int i;
 
-	if (ecc->options & NAND_ECC_MAXIMIZE) {
+	if (nanddev->ecc.user_conf.flags & NAND_ECC_MAXIMIZE) {
 		int bytes;
 
 		ecc->size = 1024;
diff --git a/drivers/mtd/nand/raw/tegra_nand.c b/drivers/mtd/nand/raw/tegra_nand.c
index fecdb7e8f9e8..1267e7529ca2 100644
--- a/drivers/mtd/nand/raw/tegra_nand.c
+++ b/drivers/mtd/nand/raw/tegra_nand.c
@@ -840,9 +840,10 @@ static int tegra_nand_get_strength(struct nand_chip *chip, const int *strength,
 				   int strength_len, int bits_per_step,
 				   int oobsize)
 {
+	struct nand_device *base = mtd_to_nanddev(nand_to_mtd(chip));
 	const struct nand_ecc_props *requirements =
-		nanddev_get_ecc_requirements(&chip->base);
-	bool maximize = chip->ecc.options & NAND_ECC_MAXIMIZE;
+		nanddev_get_ecc_requirements(base);
+	bool maximize = base->ecc.user_conf.flags & NAND_ECC_MAXIMIZE;
 	int i;
 
 	/*
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 77757aabeba2..3484374fb0cd 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -182,6 +182,7 @@ enum nand_ecc_algo {
  * @algo: ECC algorithm (if relevant)
  * @strength: ECC strength
  * @step_size: Number of bytes per step
+ * @flags: Misc properties
  */
 struct nand_ecc_props {
 	enum nand_ecc_engine_type engine_type;
@@ -189,10 +190,14 @@ struct nand_ecc_props {
 	enum nand_ecc_algo algo;
 	unsigned int strength;
 	unsigned int step_size;
+	unsigned int flags;
 };
 
 #define NAND_ECCREQ(str, stp) { .strength = (str), .step_size = (stp) }
 
+/* NAND ECC misc flags */
+#define NAND_ECC_MAXIMIZE BIT(0)
+
 /**
  * struct nand_bbt - bad block table object
  * @cache: in memory BBT cache
@@ -264,12 +269,14 @@ struct nand_ecc_engine {
 	struct nand_ecc_engine_ops *ops;
 };
 
+void nand_ecc_read_user_conf(struct nand_device *nand);
 int nand_ecc_init_ctx(struct nand_device *nand);
 void nand_ecc_cleanup_ctx(struct nand_device *nand);
 int nand_ecc_prepare_io_req(struct nand_device *nand,
 			    struct nand_page_io_req *req);
 int nand_ecc_finish_io_req(struct nand_device *nand,
 			   struct nand_page_io_req *req);
+bool nand_ecc_correction_is_enough(struct nand_device *nand);
 
 /**
  * struct nand_ecc - Information relative to the ECC
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 66f69a1d27a5..9d69fa6608ae 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -98,7 +98,6 @@ struct nand_chip;
  * pages and you want to rely on the default implementation.
  */
 #define NAND_ECC_GENERIC_ERASED_CHECK	BIT(0)
-#define NAND_ECC_MAXIMIZE		BIT(1)
 
 /*
  * Option constants for bizarre disfunctionality and real
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v8 4/5] mtd: rawnand: Move generic OOB layouts to the ECC framework
From: Miquel Raynal @ 2020-05-29 13:16 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd,
	Rob Herring, Mark Rutland, devicetree
  Cc: Julien Su, Weijie Gao, Paul Cercueil, Boris Brezillon,
	Thomas Petazzoni, Miquel Raynal, Mason Yang, Chuanhong Guo,
	linux-arm-kernel
In-Reply-To: <20200529131602.21532-1-miquel.raynal@bootlin.com>

These layouts can be used by any driver, move them to the ECC core.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/ecc.c           | 176 ++++++++++++++++++++++++++++++
 drivers/mtd/nand/raw/Kconfig     |   1 +
 drivers/mtd/nand/raw/nand_base.c | 177 +------------------------------
 include/linux/mtd/nand.h         |   4 +
 include/linux/mtd/rawnand.h      |   5 +-
 5 files changed, 183 insertions(+), 180 deletions(-)

diff --git a/drivers/mtd/nand/ecc.c b/drivers/mtd/nand/ecc.c
index f7300ba37167..ad08a047dfc5 100644
--- a/drivers/mtd/nand/ecc.c
+++ b/drivers/mtd/nand/ecc.c
@@ -152,6 +152,182 @@ int nand_ecc_finish_io_req(struct nand_device *nand,
 }
 EXPORT_SYMBOL(nand_ecc_finish_io_req);
 
+/* Define default oob placement schemes for large and small page devices */
+static int nand_ooblayout_ecc_sp(struct mtd_info *mtd, int section,
+				 struct mtd_oob_region *oobregion)
+{
+	struct nand_device *nand = mtd_to_nanddev(mtd);
+	unsigned int total_ecc_bytes = nand->ecc.ctx.total;
+
+	if (section > 1)
+		return -ERANGE;
+
+	if (!section) {
+		oobregion->offset = 0;
+		if (mtd->oobsize == 16)
+			oobregion->length = 4;
+		else
+			oobregion->length = 3;
+	} else {
+		if (mtd->oobsize == 8)
+			return -ERANGE;
+
+		oobregion->offset = 6;
+		oobregion->length = total_ecc_bytes - 4;
+	}
+
+	return 0;
+}
+
+static int nand_ooblayout_free_sp(struct mtd_info *mtd, int section,
+				  struct mtd_oob_region *oobregion)
+{
+	if (section > 1)
+		return -ERANGE;
+
+	if (mtd->oobsize == 16) {
+		if (section)
+			return -ERANGE;
+
+		oobregion->length = 8;
+		oobregion->offset = 8;
+	} else {
+		oobregion->length = 2;
+		if (!section)
+			oobregion->offset = 3;
+		else
+			oobregion->offset = 6;
+	}
+
+	return 0;
+}
+
+static const struct mtd_ooblayout_ops nand_ooblayout_sp_ops = {
+	.ecc = nand_ooblayout_ecc_sp,
+	.free = nand_ooblayout_free_sp,
+};
+
+const struct mtd_ooblayout_ops *nand_get_small_page_ooblayout(void)
+{
+	return &nand_ooblayout_sp_ops;
+}
+EXPORT_SYMBOL_GPL(nand_get_small_page_ooblayout);
+
+static int nand_ooblayout_ecc_lp(struct mtd_info *mtd, int section,
+				 struct mtd_oob_region *oobregion)
+{
+	struct nand_device *nand = mtd_to_nanddev(mtd);
+	unsigned int total_ecc_bytes = nand->ecc.ctx.total;
+
+	if (section || !total_ecc_bytes)
+		return -ERANGE;
+
+	oobregion->length = total_ecc_bytes;
+	oobregion->offset = mtd->oobsize - oobregion->length;
+
+	return 0;
+}
+
+static int nand_ooblayout_free_lp(struct mtd_info *mtd, int section,
+				  struct mtd_oob_region *oobregion)
+{
+	struct nand_device *nand = mtd_to_nanddev(mtd);
+	unsigned int total_ecc_bytes = nand->ecc.ctx.total;
+
+	if (section)
+		return -ERANGE;
+
+	oobregion->length = mtd->oobsize - total_ecc_bytes - 2;
+	oobregion->offset = 2;
+
+	return 0;
+}
+
+static const struct mtd_ooblayout_ops nand_ooblayout_lp_ops = {
+	.ecc = nand_ooblayout_ecc_lp,
+	.free = nand_ooblayout_free_lp,
+};
+
+const struct mtd_ooblayout_ops *nand_get_large_page_ooblayout(void)
+{
+	return &nand_ooblayout_lp_ops;
+}
+EXPORT_SYMBOL_GPL(nand_get_large_page_ooblayout);
+
+/*
+ * Support the old "large page" layout used for 1-bit Hamming ECC where ECC
+ * are placed at a fixed offset.
+ */
+static int nand_ooblayout_ecc_lp_hamming(struct mtd_info *mtd, int section,
+					 struct mtd_oob_region *oobregion)
+{
+	struct nand_device *nand = mtd_to_nanddev(mtd);
+	unsigned int total_ecc_bytes = nand->ecc.ctx.total;
+
+	if (section)
+		return -ERANGE;
+
+	switch (mtd->oobsize) {
+	case 64:
+		oobregion->offset = 40;
+		break;
+	case 128:
+		oobregion->offset = 80;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	oobregion->length = total_ecc_bytes;
+	if (oobregion->offset + oobregion->length > mtd->oobsize)
+		return -ERANGE;
+
+	return 0;
+}
+
+static int nand_ooblayout_free_lp_hamming(struct mtd_info *mtd, int section,
+					  struct mtd_oob_region *oobregion)
+{
+	struct nand_device *nand = mtd_to_nanddev(mtd);
+	unsigned int total_ecc_bytes = nand->ecc.ctx.total;
+	int ecc_offset = 0;
+
+	if (section < 0 || section > 1)
+		return -ERANGE;
+
+	switch (mtd->oobsize) {
+	case 64:
+		ecc_offset = 40;
+		break;
+	case 128:
+		ecc_offset = 80;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (section == 0) {
+		oobregion->offset = 2;
+		oobregion->length = ecc_offset - 2;
+	} else {
+		oobregion->offset = ecc_offset + total_ecc_bytes;
+		oobregion->length = mtd->oobsize - oobregion->offset;
+	}
+
+	return 0;
+}
+
+static const struct mtd_ooblayout_ops nand_ooblayout_lp_hamming_ops = {
+	.ecc = nand_ooblayout_ecc_lp_hamming,
+	.free = nand_ooblayout_free_lp_hamming,
+};
+
+const struct mtd_ooblayout_ops *nand_get_large_page_hamming_ooblayout(void)
+{
+	return &nand_ooblayout_lp_hamming_ops;
+}
+EXPORT_SYMBOL_GPL(nand_get_large_page_hamming_ooblayout);
+
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Miquel Raynal <miquel.raynal@bootlin.com>");
 MODULE_DESCRIPTION("Generic ECC engine");
diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
index 85280e327bfe..6ab3184ca8eb 100644
--- a/drivers/mtd/nand/raw/Kconfig
+++ b/drivers/mtd/nand/raw/Kconfig
@@ -13,6 +13,7 @@ config MTD_NAND_ECC_SW_HAMMING_SMC
 menuconfig MTD_RAW_NAND
 	tristate "Raw/Parallel NAND Device Support"
 	select MTD_NAND_CORE
+	select MTD_NAND_ECC
 	select MTD_NAND_ECC_SW_HAMMING
 	help
 	  This enables support for accessing all type of raw/parallel
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 8dc230892b90..afc3506468ba 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -34,6 +34,7 @@
 #include <linux/mm.h>
 #include <linux/types.h>
 #include <linux/mtd/mtd.h>
+#include <linux/mtd/nand.h>
 #include <linux/mtd/nand_ecc.h>
 #include <linux/mtd/nand_bch.h>
 #include <linux/interrupt.h>
@@ -45,182 +46,6 @@
 
 #include "internals.h"
 
-/* Define default oob placement schemes for large and small page devices */
-static int nand_ooblayout_ecc_sp(struct mtd_info *mtd, int section,
-				 struct mtd_oob_region *oobregion)
-{
-	struct nand_chip *chip = mtd_to_nand(mtd);
-	struct nand_ecc_ctrl *ecc = &chip->ecc;
-
-	if (section > 1)
-		return -ERANGE;
-
-	if (!section) {
-		oobregion->offset = 0;
-		if (mtd->oobsize == 16)
-			oobregion->length = 4;
-		else
-			oobregion->length = 3;
-	} else {
-		if (mtd->oobsize == 8)
-			return -ERANGE;
-
-		oobregion->offset = 6;
-		oobregion->length = ecc->total - 4;
-	}
-
-	return 0;
-}
-
-static int nand_ooblayout_free_sp(struct mtd_info *mtd, int section,
-				  struct mtd_oob_region *oobregion)
-{
-	if (section > 1)
-		return -ERANGE;
-
-	if (mtd->oobsize == 16) {
-		if (section)
-			return -ERANGE;
-
-		oobregion->length = 8;
-		oobregion->offset = 8;
-	} else {
-		oobregion->length = 2;
-		if (!section)
-			oobregion->offset = 3;
-		else
-			oobregion->offset = 6;
-	}
-
-	return 0;
-}
-
-static const struct mtd_ooblayout_ops nand_ooblayout_sp_ops = {
-	.ecc = nand_ooblayout_ecc_sp,
-	.free = nand_ooblayout_free_sp,
-};
-
-const struct mtd_ooblayout_ops *nand_get_small_page_ooblayout(void)
-{
-	return &nand_ooblayout_sp_ops;
-}
-EXPORT_SYMBOL_GPL(nand_get_small_page_ooblayout);
-
-static int nand_ooblayout_ecc_lp(struct mtd_info *mtd, int section,
-				 struct mtd_oob_region *oobregion)
-{
-	struct nand_chip *chip = mtd_to_nand(mtd);
-	struct nand_ecc_ctrl *ecc = &chip->ecc;
-
-	if (section || !ecc->total)
-		return -ERANGE;
-
-	oobregion->length = ecc->total;
-	oobregion->offset = mtd->oobsize - oobregion->length;
-
-	return 0;
-}
-
-static int nand_ooblayout_free_lp(struct mtd_info *mtd, int section,
-				  struct mtd_oob_region *oobregion)
-{
-	struct nand_chip *chip = mtd_to_nand(mtd);
-	struct nand_ecc_ctrl *ecc = &chip->ecc;
-
-	if (section)
-		return -ERANGE;
-
-	oobregion->length = mtd->oobsize - ecc->total - 2;
-	oobregion->offset = 2;
-
-	return 0;
-}
-
-static const struct mtd_ooblayout_ops nand_ooblayout_lp_ops = {
-	.ecc = nand_ooblayout_ecc_lp,
-	.free = nand_ooblayout_free_lp,
-};
-
-const struct mtd_ooblayout_ops *nand_get_large_page_ooblayout(void)
-{
-	return &nand_ooblayout_lp_ops;
-}
-EXPORT_SYMBOL_GPL(nand_get_large_page_ooblayout);
-
-/*
- * Support the old "large page" layout used for 1-bit Hamming ECC where ECC
- * are placed at a fixed offset.
- */
-static int nand_ooblayout_ecc_lp_hamming(struct mtd_info *mtd, int section,
-					 struct mtd_oob_region *oobregion)
-{
-	struct nand_chip *chip = mtd_to_nand(mtd);
-	struct nand_ecc_ctrl *ecc = &chip->ecc;
-
-	if (section)
-		return -ERANGE;
-
-	switch (mtd->oobsize) {
-	case 64:
-		oobregion->offset = 40;
-		break;
-	case 128:
-		oobregion->offset = 80;
-		break;
-	default:
-		return -EINVAL;
-	}
-
-	oobregion->length = ecc->total;
-	if (oobregion->offset + oobregion->length > mtd->oobsize)
-		return -ERANGE;
-
-	return 0;
-}
-
-static int nand_ooblayout_free_lp_hamming(struct mtd_info *mtd, int section,
-					  struct mtd_oob_region *oobregion)
-{
-	struct nand_chip *chip = mtd_to_nand(mtd);
-	struct nand_ecc_ctrl *ecc = &chip->ecc;
-	int ecc_offset = 0;
-
-	if (section < 0 || section > 1)
-		return -ERANGE;
-
-	switch (mtd->oobsize) {
-	case 64:
-		ecc_offset = 40;
-		break;
-	case 128:
-		ecc_offset = 80;
-		break;
-	default:
-		return -EINVAL;
-	}
-
-	if (section == 0) {
-		oobregion->offset = 2;
-		oobregion->length = ecc_offset - 2;
-	} else {
-		oobregion->offset = ecc_offset + ecc->total;
-		oobregion->length = mtd->oobsize - oobregion->offset;
-	}
-
-	return 0;
-}
-
-static const struct mtd_ooblayout_ops nand_ooblayout_lp_hamming_ops = {
-	.ecc = nand_ooblayout_ecc_lp_hamming,
-	.free = nand_ooblayout_free_lp_hamming,
-};
-
-const struct mtd_ooblayout_ops *nand_get_large_page_hamming_ooblayout(void)
-{
-	return &nand_ooblayout_lp_hamming_ops;
-}
-EXPORT_SYMBOL_GPL(nand_get_large_page_hamming_ooblayout);
-
 static int nand_pairing_dist3_get_info(struct mtd_info *mtd, int page,
 				       struct mtd_pairing_info *info)
 {
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index f5cc0aee565c..77757aabeba2 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -127,6 +127,10 @@ struct nand_page_io_req {
 	int mode;
 };
 
+const struct mtd_ooblayout_ops *nand_get_small_page_ooblayout(void);
+const struct mtd_ooblayout_ops *nand_get_large_page_ooblayout(void);
+const struct mtd_ooblayout_ops *nand_get_large_page_hamming_ooblayout(void);
+
 /**
  * enum nand_ecc_engine_type - NAND ECC engine type
  * @NAND_ECC_ENGINE_TYPE_INVALID: Invalid value
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index b455cb22168f..66f69a1d27a5 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -14,6 +14,7 @@
 #define __LINUX_MTD_RAWNAND_H
 
 #include <linux/mtd/mtd.h>
+#include <linux/mtd/nand.h>
 #include <linux/mtd/flashchip.h>
 #include <linux/mtd/bbm.h>
 #include <linux/mtd/jedec.h>
@@ -1147,10 +1148,6 @@ struct nand_chip {
 	int (*unlock_area)(struct nand_chip *chip, loff_t ofs, uint64_t len);
 };
 
-const struct mtd_ooblayout_ops *nand_get_small_page_ooblayout(void);
-const struct mtd_ooblayout_ops *nand_get_large_page_ooblayout(void);
-const struct mtd_ooblayout_ops *nand_get_large_page_hamming_ooblayout(void);
-
 static inline struct nand_chip *mtd_to_nand(struct mtd_info *mtd)
 {
 	return container_of(mtd, struct nand_chip, base.mtd);
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v8 3/5] mtd: rawnand: Write a compatibility layer
From: Miquel Raynal @ 2020-05-29 13:16 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd,
	Rob Herring, Mark Rutland, devicetree
  Cc: Julien Su, Weijie Gao, Paul Cercueil, Boris Brezillon,
	Thomas Petazzoni, Miquel Raynal, Mason Yang, Chuanhong Guo,
	linux-arm-kernel
In-Reply-To: <20200529131602.21532-1-miquel.raynal@bootlin.com>

Before moving generic bits from the raw NAND core to the generic NAND
core, let's disociate clearly what is a rawnand legacy property, and
what should be made public to other NAND users.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/nand_base.c | 158 +++++++++++++++++++++----------
 include/linux/mtd/rawnand.h      |  12 ---
 2 files changed, 107 insertions(+), 63 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index f120b0b4f591..8dc230892b90 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -5029,14 +5029,6 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
 	return ret;
 }
 
-static const char * const nand_ecc_modes[] = {
-	[NAND_ECC_NONE]		= "none",
-	[NAND_ECC_SOFT]		= "soft",
-	[NAND_ECC_HW]		= "hw",
-	[NAND_ECC_HW_SYNDROME]	= "hw_syndrome",
-	[NAND_ECC_ON_DIE]	= "on-die",
-};
-
 static const char * const nand_ecc_placement[] = {
 	[NAND_ECC_PLACEMENT_OOB] = "oob",
 	[NAND_ECC_PLACEMENT_INTERLEAVED] = "interleaved",
@@ -5045,7 +5037,30 @@ static const char * const nand_ecc_placement[] = {
 static enum nand_ecc_engine_type
 of_get_nand_ecc_engine_type(struct device_node *np)
 {
-	enum nand_ecc_mode eng_type;
+	return NAND_ECC_ENGINE_TYPE_INVALID;
+}
+
+static enum nand_ecc_engine_type
+of_get_rawnand_ecc_engine_type_legacy(struct device_node *np)
+{
+	enum nand_ecc_legacy_mode {
+		NAND_ECC_INVALID,
+		NAND_ECC_NONE,
+		NAND_ECC_SOFT,
+		NAND_ECC_SOFT_BCH,
+		NAND_ECC_HW,
+		NAND_ECC_HW_SYNDROME,
+		NAND_ECC_ON_DIE,
+	};
+	const char * const nand_ecc_legacy_modes[] = {
+		[NAND_ECC_NONE]		= "none",
+		[NAND_ECC_SOFT]		= "soft",
+		[NAND_ECC_SOFT_BCH]	= "soft_bch",
+		[NAND_ECC_HW]		= "hw",
+		[NAND_ECC_HW_SYNDROME]	= "hw_syndrome",
+		[NAND_ECC_ON_DIE]	= "on-die",
+	};
+	enum nand_ecc_legacy_mode eng_type;
 	const char *pm;
 	int err;
 
@@ -5054,12 +5069,13 @@ of_get_nand_ecc_engine_type(struct device_node *np)
 		return NAND_ECC_ENGINE_TYPE_INVALID;
 
 	for (eng_type = NAND_ECC_NONE;
-	     eng_type < ARRAY_SIZE(nand_ecc_modes); eng_type++) {
-		if (!strcasecmp(pm, nand_ecc_modes[eng_type])) {
+	     eng_type < ARRAY_SIZE(nand_ecc_legacy_modes); eng_type++) {
+		if (!strcasecmp(pm, nand_ecc_legacy_modes[eng_type])) {
 			switch (eng_type) {
 			case NAND_ECC_NONE:
 				return NAND_ECC_ENGINE_TYPE_NONE;
 			case NAND_ECC_SOFT:
+			case NAND_ECC_SOFT_BCH:
 				return NAND_ECC_ENGINE_TYPE_SOFT;
 			case NAND_ECC_HW:
 			case NAND_ECC_HW_SYNDROME:
@@ -5072,14 +5088,6 @@ of_get_nand_ecc_engine_type(struct device_node *np)
 		}
 	}
 
-	/*
-	 * For backward compatibility we support few obsoleted values that don't
-	 * have their mappings into the nand_ecc_engine_providers enum anymore
-	 * (they were merged with other enums).
-	 */
-	if (!strcasecmp(pm, "soft_bch"))
-		return NAND_ECC_ENGINE_TYPE_SOFT;
-
 	return NAND_ECC_ENGINE_TYPE_INVALID;
 }
 
@@ -5091,17 +5099,22 @@ enum nand_ecc_placement of_get_nand_ecc_placement(struct device_node *np)
 
 	err = of_property_read_string(np, "nand-ecc-placement", &pm);
 	if (!err) {
-		for (placement = NAND_ECC_PLACEMENT_INTERLEAVED;
+		for (placement = NAND_ECC_PLACEMENT_OOB;
 		     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.
-	 */
+	return NAND_ECC_PLACEMENT_UNKNOWN;
+}
+
+enum nand_ecc_placement
+of_get_rawnand_ecc_placement_legacy(struct device_node *np)
+{
+	const char *pm;
+	int err;
+
 	err = of_property_read_string(np, "nand-ecc-mode", &pm);
 	if (!err) {
 		if (!strcasecmp(pm, "hw_syndrome"))
@@ -5133,10 +5146,14 @@ static enum nand_ecc_algo of_get_nand_ecc_algo(struct device_node *np)
 		}
 	}
 
-	/*
-	 * For backward compatibility we also read "nand-ecc-mode" checking
-	 * for some obsoleted values that were specifying ECC algorithm.
-	 */
+	return NAND_ECC_ALGO_UNKNOWN;
+}
+
+static enum nand_ecc_algo of_get_rawnand_ecc_algo_legacy(struct device_node *np)
+{
+	const char *pm;
+	int err;
+
 	err = of_property_read_string(np, "nand-ecc-mode", &pm);
 	if (!err) {
 		if (!strcasecmp(pm, "soft"))
@@ -5166,6 +5183,41 @@ static int of_get_nand_ecc_strength(struct device_node *np)
 	return ret ? ret : val;
 }
 
+static void nand_ecc_read_user_conf(struct nand_chip *chip)
+{
+	struct device_node *dn = nand_get_flash_node(chip);
+	struct nand_device *nand = &chip->base;
+	int strength, size;
+
+	nand->ecc.user_conf.engine_type = of_get_nand_ecc_engine_type(dn);
+	nand->ecc.user_conf.algo = of_get_nand_ecc_algo(dn);
+	nand->ecc.user_conf.placement = of_get_nand_ecc_placement(dn);
+
+	strength = of_get_nand_ecc_strength(dn);
+	if (strength >= 0)
+		nand->ecc.user_conf.strength = strength;
+
+	size = of_get_nand_ecc_step_size(dn);
+	if (size >= 0)
+		nand->ecc.user_conf.step_size = size;
+}
+
+static void rawnand_ecc_read_legacy_user_conf(struct nand_chip *chip)
+{
+	struct device_node *dn = nand_get_flash_node(chip);
+	struct nand_device *nand = &chip->base;
+	struct nand_ecc_props *user_conf = &nand->ecc.user_conf;
+
+	if (user_conf->engine_type != NAND_ECC_ENGINE_TYPE_INVALID)
+		user_conf->engine_type = of_get_rawnand_ecc_engine_type_legacy(dn);
+
+	if (user_conf->algo != NAND_ECC_ALGO_UNKNOWN)
+		user_conf->algo = of_get_rawnand_ecc_algo_legacy(dn);
+
+	if (user_conf->placement != NAND_ECC_PLACEMENT_UNKNOWN)
+		user_conf->placement = of_get_rawnand_ecc_placement_legacy(dn);
+}
+
 static int of_get_nand_bus_width(struct device_node *np)
 {
 	u32 val;
@@ -5187,12 +5239,10 @@ static bool of_get_nand_on_flash_bbt(struct device_node *np)
 	return of_property_read_bool(np, "nand-on-flash-bbt");
 }
 
-static int nand_dt_init(struct nand_chip *chip)
+static int rawnand_dt_init(struct nand_chip *chip)
 {
+	struct nand_device *nand = mtd_to_nanddev(nand_to_mtd(chip));
 	struct device_node *dn = nand_get_flash_node(chip);
-	enum nand_ecc_engine_type ecc_type;
-	enum nand_ecc_algo ecc_algo;
-	int ecc_strength, ecc_step;
 
 	if (!dn)
 		return 0;
@@ -5206,27 +5256,33 @@ static int nand_dt_init(struct nand_chip *chip)
 	if (of_get_nand_on_flash_bbt(dn))
 		chip->bbt_options |= NAND_BBT_USE_FLASH;
 
-	ecc_type = of_get_nand_ecc_engine_type(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);
-
-	if (ecc_type != NAND_ECC_ENGINE_TYPE_INVALID)
-		chip->ecc.engine_type = ecc_type;
-
-	if (ecc_algo != NAND_ECC_ALGO_UNKNOWN)
-		chip->ecc.algo = ecc_algo;
-
-	if (ecc_strength >= 0)
-		chip->ecc.strength = ecc_strength;
-
-	if (ecc_step > 0)
-		chip->ecc.size = ecc_step;
-
 	if (of_property_read_bool(dn, "nand-ecc-maximize"))
 		chip->ecc.options |= NAND_ECC_MAXIMIZE;
 
+	nand_ecc_read_user_conf(chip);
+	rawnand_ecc_read_legacy_user_conf(chip);
+
+	/*
+	 * If neither the user nor the NAND controller have requested a specific
+	 * ECC engine type, we will default to NAND_ECC_ENGINE_TYPE_ON_HOST.
+	 */
+	nand->ecc.defaults.engine_type = NAND_ECC_ENGINE_TYPE_ON_HOST;
+
+	/*
+	 * Use the user requested engine type, unless there is none, in this
+	 * case default to the NAND controller choice, otherwise fallback to
+	 * the raw NAND default one.
+	 */
+	if (nand->ecc.user_conf.engine_type != NAND_ECC_ENGINE_TYPE_INVALID)
+		chip->ecc.engine_type = nand->ecc.user_conf.engine_type;
+	if (chip->ecc.engine_type == NAND_ECC_ENGINE_TYPE_INVALID)
+		chip->ecc.engine_type = nand->ecc.defaults.engine_type;
+
+	chip->ecc.placement = nand->ecc.user_conf.placement;
+	chip->ecc.algo = nand->ecc.user_conf.algo;
+	chip->ecc.strength = nand->ecc.user_conf.strength;
+	chip->ecc.size = nand->ecc.user_conf.step_size;
+
 	return 0;
 }
 
@@ -5263,7 +5319,7 @@ static int nand_scan_ident(struct nand_chip *chip, unsigned int maxchips,
 	/* Enforce the right timings for reset/detection */
 	onfi_fill_data_interface(chip, NAND_SDR_IFACE, 0);
 
-	ret = nand_dt_init(chip);
+	ret = rawnand_dt_init(chip);
 	if (ret)
 		return ret;
 
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index f3eb47c09e57..b455cb22168f 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -80,18 +80,6 @@ struct nand_chip;
 
 #define NAND_DATA_IFACE_CHECK_ONLY	-1
 
-/*
- * Constants for ECC_MODES
- */
-enum nand_ecc_mode {
-	NAND_ECC_INVALID,
-	NAND_ECC_NONE,
-	NAND_ECC_SOFT,
-	NAND_ECC_HW,
-	NAND_ECC_HW_SYNDROME,
-	NAND_ECC_ON_DIE,
-};
-
 /*
  * Constants for Hardware ECC
  */
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v8 2/5] mtd: rawnand: Hide the generic OOB layout objects behind helpers
From: Miquel Raynal @ 2020-05-29 13:15 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd,
	Rob Herring, Mark Rutland, devicetree
  Cc: Julien Su, Weijie Gao, Paul Cercueil, Boris Brezillon,
	Thomas Petazzoni, Miquel Raynal, Mason Yang, Chuanhong Guo,
	linux-arm-kernel
In-Reply-To: <20200529131602.21532-1-miquel.raynal@bootlin.com>

Stop exposing these objects, create helpers to retrieve them instead.

Also export an helper for the Hamming large page ops for later use.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/arasan-nand-controller.c |  2 +-
 drivers/mtd/nand/raw/atmel/nand-controller.c  |  2 +-
 drivers/mtd/nand/raw/davinci_nand.c           |  3 +-
 .../mtd/nand/raw/ingenic/ingenic_nand_drv.c   |  6 ++--
 drivers/mtd/nand/raw/nand_base.c              | 35 ++++++++++++++-----
 drivers/mtd/nand/raw/nand_toshiba.c           |  2 +-
 drivers/mtd/nand/raw/vf610_nfc.c              |  2 +-
 include/linux/mtd/rawnand.h                   |  5 +--
 8 files changed, 38 insertions(+), 19 deletions(-)

diff --git a/drivers/mtd/nand/raw/arasan-nand-controller.c b/drivers/mtd/nand/raw/arasan-nand-controller.c
index a0b5c539ca73..6fe61393bd26 100644
--- a/drivers/mtd/nand/raw/arasan-nand-controller.c
+++ b/drivers/mtd/nand/raw/arasan-nand-controller.c
@@ -980,7 +980,7 @@ static int anfc_init_hw_ecc_controller(struct arasan_nfc *nfc,
 		return -EINVAL;
 	}
 
-	mtd_set_ooblayout(mtd, &nand_ooblayout_lp_ops);
+	mtd_set_ooblayout(mtd, nand_get_large_page_ooblayout());
 
 	ecc->steps = mtd->writesize / ecc->size;
 	ecc->algo = NAND_ECC_ALGO_BCH;
diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
index 3fba91d7991a..08df7f23b859 100644
--- a/drivers/mtd/nand/raw/atmel/nand-controller.c
+++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
@@ -1108,7 +1108,7 @@ static int atmel_nand_pmecc_init(struct nand_chip *chip)
 
 	chip->options |= NAND_NO_SUBPAGE_WRITE;
 
-	mtd_set_ooblayout(mtd, &nand_ooblayout_lp_ops);
+	mtd_set_ooblayout(mtd, nand_get_large_page_ooblayout());
 
 	return 0;
 }
diff --git a/drivers/mtd/nand/raw/davinci_nand.c b/drivers/mtd/nand/raw/davinci_nand.c
index 58966a9706b1..427f320fb79b 100644
--- a/drivers/mtd/nand/raw/davinci_nand.c
+++ b/drivers/mtd/nand/raw/davinci_nand.c
@@ -645,7 +645,8 @@ static int davinci_nand_attach_chip(struct nand_chip *chip)
 				mtd_set_ooblayout(mtd,
 						  &hwecc4_small_ooblayout_ops);
 			} else if (chunks == 4 || chunks == 8) {
-				mtd_set_ooblayout(mtd, &nand_ooblayout_lp_ops);
+				mtd_set_ooblayout(mtd,
+						  nand_get_large_page_ooblayout());
 				info->chip.ecc.read_page = nand_davinci_read_page_hwecc_oob_first;
 			} else {
 				return -EIO;
diff --git a/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c b/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c
index 70309f18124c..0e9d426fe4f2 100644
--- a/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c
+++ b/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c
@@ -243,8 +243,10 @@ static int ingenic_nand_attach_chip(struct nand_chip *chip)
 	/* For legacy reasons we use a different layout on the qi,lb60 board. */
 	if (of_machine_is_compatible("qi,lb60"))
 		mtd_set_ooblayout(mtd, &qi_lb60_ooblayout_ops);
-	else
+	else if (nfc->soc_info->oob_layout)
 		mtd_set_ooblayout(mtd, nfc->soc_info->oob_layout);
+	else
+		mtd_set_ooblayout(mtd, nand_get_large_page_ooblayout());
 
 	return 0;
 }
@@ -532,7 +534,6 @@ static const struct jz_soc_info jz4740_soc_info = {
 	.data_offset = 0x00000000,
 	.cmd_offset = 0x00008000,
 	.addr_offset = 0x00010000,
-	.oob_layout = &nand_ooblayout_lp_ops,
 };
 
 static const struct jz_soc_info jz4725b_soc_info = {
@@ -546,7 +547,6 @@ static const struct jz_soc_info jz4780_soc_info = {
 	.data_offset = 0x00000000,
 	.cmd_offset = 0x00400000,
 	.addr_offset = 0x00800000,
-	.oob_layout = &nand_ooblayout_lp_ops,
 };
 
 static const struct of_device_id ingenic_nand_dt_match[] = {
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index ed0f642be993..f120b0b4f591 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -95,11 +95,16 @@ static int nand_ooblayout_free_sp(struct mtd_info *mtd, int section,
 	return 0;
 }
 
-const struct mtd_ooblayout_ops nand_ooblayout_sp_ops = {
+static const struct mtd_ooblayout_ops nand_ooblayout_sp_ops = {
 	.ecc = nand_ooblayout_ecc_sp,
 	.free = nand_ooblayout_free_sp,
 };
-EXPORT_SYMBOL_GPL(nand_ooblayout_sp_ops);
+
+const struct mtd_ooblayout_ops *nand_get_small_page_ooblayout(void)
+{
+	return &nand_ooblayout_sp_ops;
+}
+EXPORT_SYMBOL_GPL(nand_get_small_page_ooblayout);
 
 static int nand_ooblayout_ecc_lp(struct mtd_info *mtd, int section,
 				 struct mtd_oob_region *oobregion)
@@ -131,11 +136,16 @@ static int nand_ooblayout_free_lp(struct mtd_info *mtd, int section,
 	return 0;
 }
 
-const struct mtd_ooblayout_ops nand_ooblayout_lp_ops = {
+static const struct mtd_ooblayout_ops nand_ooblayout_lp_ops = {
 	.ecc = nand_ooblayout_ecc_lp,
 	.free = nand_ooblayout_free_lp,
 };
-EXPORT_SYMBOL_GPL(nand_ooblayout_lp_ops);
+
+const struct mtd_ooblayout_ops *nand_get_large_page_ooblayout(void)
+{
+	return &nand_ooblayout_lp_ops;
+}
+EXPORT_SYMBOL_GPL(nand_get_large_page_ooblayout);
 
 /*
  * Support the old "large page" layout used for 1-bit Hamming ECC where ECC
@@ -205,6 +215,12 @@ static const struct mtd_ooblayout_ops nand_ooblayout_lp_hamming_ops = {
 	.free = nand_ooblayout_free_lp_hamming,
 };
 
+const struct mtd_ooblayout_ops *nand_get_large_page_hamming_ooblayout(void)
+{
+	return &nand_ooblayout_lp_hamming_ops;
+}
+EXPORT_SYMBOL_GPL(nand_get_large_page_hamming_ooblayout);
+
 static int nand_pairing_dist3_get_info(struct mtd_info *mtd, int page,
 				       struct mtd_pairing_info *info)
 {
@@ -5382,7 +5398,7 @@ static int nand_set_ecc_soft_ops(struct nand_chip *chip)
 				return -EINVAL;
 			}
 
-			mtd_set_ooblayout(mtd, &nand_ooblayout_lp_ops);
+			mtd_set_ooblayout(mtd, nand_get_large_page_ooblayout());
 
 		}
 
@@ -5391,7 +5407,7 @@ static int nand_set_ecc_soft_ops(struct nand_chip *chip)
 		 * used, otherwise we don't know how many bytes can really be
 		 * used.
 		 */
-		if (mtd->ooblayout == &nand_ooblayout_lp_ops &&
+		if (mtd->ooblayout == nand_get_large_page_ooblayout() &&
 		    ecc->options & NAND_ECC_MAXIMIZE) {
 			int steps, bytes;
 
@@ -5793,11 +5809,12 @@ static int nand_scan_tail(struct nand_chip *chip)
 		switch (mtd->oobsize) {
 		case 8:
 		case 16:
-			mtd_set_ooblayout(mtd, &nand_ooblayout_sp_ops);
+			mtd_set_ooblayout(mtd, nand_get_small_page_ooblayout());
 			break;
 		case 64:
 		case 128:
-			mtd_set_ooblayout(mtd, &nand_ooblayout_lp_hamming_ops);
+			mtd_set_ooblayout(mtd,
+					  nand_get_large_page_hamming_ooblayout());
 			break;
 		default:
 			/*
@@ -5809,7 +5826,7 @@ static int nand_scan_tail(struct nand_chip *chip)
 			 */
 			if (ecc->engine_type == NAND_ECC_ENGINE_TYPE_NONE) {
 				mtd_set_ooblayout(mtd,
-						&nand_ooblayout_lp_ops);
+						  nand_get_large_page_ooblayout());
 				break;
 			}
 
diff --git a/drivers/mtd/nand/raw/nand_toshiba.c b/drivers/mtd/nand/raw/nand_toshiba.c
index 8fcf40d0ba0a..3174914e33e0 100644
--- a/drivers/mtd/nand/raw/nand_toshiba.c
+++ b/drivers/mtd/nand/raw/nand_toshiba.c
@@ -140,7 +140,7 @@ static void toshiba_nand_benand_init(struct nand_chip *chip)
 
 	chip->options |= NAND_SUBPAGE_READ;
 
-	mtd_set_ooblayout(mtd, &nand_ooblayout_lp_ops);
+	mtd_set_ooblayout(mtd, nand_get_large_page_ooblayout());
 }
 
 static void toshiba_nand_decode_id(struct nand_chip *chip)
diff --git a/drivers/mtd/nand/raw/vf610_nfc.c b/drivers/mtd/nand/raw/vf610_nfc.c
index 8ee2c1f539c4..50dc0c93140c 100644
--- a/drivers/mtd/nand/raw/vf610_nfc.c
+++ b/drivers/mtd/nand/raw/vf610_nfc.c
@@ -779,7 +779,7 @@ static int vf610_nfc_attach_chip(struct nand_chip *chip)
 		mtd->oobsize = 64;
 
 	/* Use default large page ECC layout defined in NAND core */
-	mtd_set_ooblayout(mtd, &nand_ooblayout_lp_ops);
+	mtd_set_ooblayout(mtd, nand_get_large_page_ooblayout());
 	if (chip->ecc.strength == 32) {
 		nfc->ecc_mode = ECC_60_BYTE;
 		chip->ecc.bytes = 60;
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 8f7f1cce3b4b..f3eb47c09e57 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -1159,8 +1159,9 @@ struct nand_chip {
 	int (*unlock_area)(struct nand_chip *chip, loff_t ofs, uint64_t len);
 };
 
-extern const struct mtd_ooblayout_ops nand_ooblayout_sp_ops;
-extern const struct mtd_ooblayout_ops nand_ooblayout_lp_ops;
+const struct mtd_ooblayout_ops *nand_get_small_page_ooblayout(void);
+const struct mtd_ooblayout_ops *nand_get_large_page_ooblayout(void);
+const struct mtd_ooblayout_ops *nand_get_large_page_hamming_ooblayout(void);
 
 static inline struct nand_chip *mtd_to_nand(struct mtd_info *mtd)
 {
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v8 1/5] mtd: nand: Convert generic NAND bits to use the ECC framework
From: Miquel Raynal @ 2020-05-29 13:15 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd,
	Rob Herring, Mark Rutland, devicetree
  Cc: Julien Su, Weijie Gao, Paul Cercueil, Boris Brezillon,
	Thomas Petazzoni, Miquel Raynal, Mason Yang, Chuanhong Guo,
	linux-arm-kernel
In-Reply-To: <20200529131602.21532-1-miquel.raynal@bootlin.com>

Embed a generic NAND ECC high-level object in the nand_device
structure to carry all the ECC engine configuration/data.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/nand_base.c |  4 +++-
 include/linux/mtd/nand.h         | 12 ++++++------
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index e8e22d79f422..ed0f642be993 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -5984,7 +5984,9 @@ static int nand_scan_tail(struct nand_chip *chip)
 	/* ECC sanity check: warn if it's too weak */
 	if (!nand_ecc_strength_good(chip))
 		pr_warn("WARNING: %s: the ECC used on your system (%db/%dB) is too weak compared to the one required by the NAND chip (%db/%dB)\n",
-			mtd->name, chip->ecc.strength, chip->ecc.size,
+			mtd->name,
+			nanddev_get_ecc_conf(&chip->base)->strength,
+			nanddev_get_ecc_conf(&chip->base)->step_size,
 			nanddev_get_ecc_requirements(&chip->base)->strength,
 			nanddev_get_ecc_requirements(&chip->base)->step_size);
 
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 488d8b14b9ae..f5cc0aee565c 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -290,7 +290,7 @@ struct nand_ecc {
  * struct nand_device - NAND device
  * @mtd: MTD instance attached to the NAND device
  * @memorg: memory layout
- * @eccreq: ECC requirements
+ * @ecc: NAND ECC object attached to the NAND device
  * @rowconv: position to row address converter
  * @bbt: bad block table info
  * @ops: NAND operations attached to the NAND device
@@ -298,8 +298,8 @@ struct nand_ecc {
  * Generic NAND object. Specialized NAND layers (raw NAND, SPI NAND, OneNAND)
  * should declare their own NAND object embedding a nand_device struct (that's
  * how inheritance is done).
- * struct_nand_device->memorg and struct_nand_device->eccreq should be filled
- * at device detection time to reflect the NAND device
+ * struct_nand_device->memorg and struct_nand_device->ecc.requirements should
+ * be filled at device detection time to reflect the NAND device
  * capabilities/requirements. Once this is done nanddev_init() can be called.
  * It will take care of converting NAND information into MTD ones, which means
  * the specialized NAND layers should never manually tweak
@@ -308,7 +308,7 @@ struct nand_ecc {
 struct nand_device {
 	struct mtd_info mtd;
 	struct nand_memory_organization memorg;
-	struct nand_ecc_props eccreq;
+	struct nand_ecc ecc;
 	struct nand_row_converter rowconv;
 	struct nand_bbt bbt;
 	const struct nand_ops *ops;
@@ -519,7 +519,7 @@ nanddev_get_memorg(struct nand_device *nand)
 const struct nand_ecc_props *
 nanddev_get_ecc_conf(struct nand_device *nand)
 {
-	return &nand->eccreq;
+	return &nand->ecc.ctx.conf;
 }
 
 /**
@@ -530,7 +530,7 @@ nanddev_get_ecc_conf(struct nand_device *nand)
 const struct nand_ecc_props *
 nanddev_get_ecc_requirements(struct nand_device *nand)
 {
-	return &nand->eccreq;
+	return &nand->ecc.requirements;
 }
 
 int nanddev_init(struct nand_device *nand, const struct nand_ops *ops,
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v8 0/5] Preparation to the generic ECC engine abstraction
From: Miquel Raynal @ 2020-05-29 13:15 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd,
	Rob Herring, Mark Rutland, devicetree
  Cc: Julien Su, Weijie Gao, Paul Cercueil, Boris Brezillon,
	Thomas Petazzoni, Miquel Raynal, Mason Yang, Chuanhong Guo,
	linux-arm-kernel

This is a respin of the end of my previous series, just the patches which needed to be fixed.

Changes in v8:
* Split "Convert generic NAND bits to ECC framework" into several peaces:
  > added two helpers
  > converted SPI-NAND then raw-NAND.
* Fixed a comment.
* Used the _ooblayout suffix instead of _layout.

Miquel Raynal (5):
  mtd: nand: Convert generic NAND bits to use the ECC framework
  mtd: rawnand: Hide the generic OOB layout objects behind helpers
  mtd: rawnand: Write a compatibility layer
  mtd: rawnand: Move generic OOB layouts to the ECC framework
  mtd: rawnand: Move the user input parsing bits to the ECC framework

 drivers/mtd/nand/ecc.c                        | 314 +++++++++++++++
 drivers/mtd/nand/raw/Kconfig                  |   1 +
 drivers/mtd/nand/raw/arasan-nand-controller.c |   2 +-
 drivers/mtd/nand/raw/atmel/nand-controller.c  |   5 +-
 drivers/mtd/nand/raw/davinci_nand.c           |   3 +-
 drivers/mtd/nand/raw/denali.c                 |   3 +
 .../mtd/nand/raw/ingenic/ingenic_nand_drv.c   |   6 +-
 drivers/mtd/nand/raw/nand_base.c              | 380 ++++--------------
 drivers/mtd/nand/raw/nand_toshiba.c           |   2 +-
 drivers/mtd/nand/raw/sunxi_nand.c             |   3 +-
 drivers/mtd/nand/raw/tegra_nand.c             |   5 +-
 drivers/mtd/nand/raw/vf610_nfc.c              |   2 +-
 include/linux/mtd/nand.h                      |  23 +-
 include/linux/mtd/rawnand.h                   |  17 +-
 14 files changed, 428 insertions(+), 338 deletions(-)

-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 3/4] pinctrl: bcm2835: Match BCM7211 compatible string
From: Stefan Wahren @ 2020-05-29 13:11 UTC (permalink / raw)
  To: Florian Fainelli, linux-kernel
  Cc: moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Geert Uytterhoeven, Scott Branden, Ray Jui, Linus Walleij,
	Matti Vaittinen, open list:PIN CONTROL SUBSYSTEM, Rob Herring,
	maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE...,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	Nicolas Saenz Julienne
In-Reply-To: <20200528192112.26123-4-f.fainelli@gmail.com>

Hi Florian,

Am 28.05.20 um 21:21 schrieb Florian Fainelli:
> The BCM7211 SoC uses the same pinconf_ops as the ones defined for the
> BCM2711 SoC, match the compatible string and use the correct set of
> options.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  drivers/pinctrl/bcm/pinctrl-bcm2835.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
> index 06bd2b70af3c..e8ad1824c6b3 100644
> --- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c
> +++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
> @@ -1137,6 +1137,10 @@ static const struct of_device_id bcm2835_pinctrl_match[] = {
>  		.compatible = "brcm,bcm2711-gpio",
>  		.data = &bcm2711_plat_data,
>  	},
> +	{
> +		.compatible = "brcm,bcm7211-gpio",
> +		.data = &bcm2711_pinconf_ops,

this doesn't look safe. Maybe bcm2711_plat_data?

Looks like the original patch series based on a older version. Please
double check this still applies since the introduction of 58 GPIO
support for BCM2711.

Regards
Stefan

> +	},
>  	{}
>  };
>  


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr
From: Ard Biesheuvel @ 2020-05-29 13:10 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Eric Biggers, Stephan Mueller, Linux Crypto Mailing List,
	Linux ARM
In-Reply-To: <20200529120216.GA3752@gondor.apana.org.au>

On Fri, 29 May 2020 at 14:02, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> On Fri, May 29, 2020 at 02:00:14PM +0200, Ard Biesheuvel wrote:
> >
> > Even if this is the case, it requires that an skcipher implementation
> > stores an output IV in the buffer that skcipher request's IV field
> > points to. Currently, we only check whether this is the case for CBC
> > implementations, and so it is quite likely that lots of h/w
> > accelerators or arch code don't adhere to this today.
>
> They are and have always been broken because algif_skcipher has
> always relied on this.
>

OK, so the undocumented assumption is that algif_skcipher requests are
delineated by ALG_SET_IV commands, and that anything that gets sent to
the socket in between should be treated as a single request, right? I
think that makes sense, but do note that this deviates from Stephan's
use case, where the ciphertext stealing block swap was applied after
every call into af_alg, with the IV being inherited from one request
to the next. I think that case was invalid to begin with, I just hope
no other use cases exist where this unspecified behavior is being
relied upon.

> > This might be feasible for the generic CTS driver wrapping h/w
> > accelerated CBC. But how is this supposed to work, e.g., for the two
> > existing h/w implementations of cts(cbc(aes)) that currently ignore
> > this?
>
> They'll have to disable chaining.
>
> The way I'm doing this would allow some implementations to allow
> chaining while others of the same algorithm can disable chaining
> and require the whole request to be presented together.
>

Fair enough.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [RFC PATCH V4 4/4] platform: mtk-isp: Add Mediatek FD driver
From: Tomasz Figa @ 2020-05-29 12:59 UTC (permalink / raw)
  To: Jerry-ch Chen
  Cc: linux-devicetree, Sean Cheng (鄭昇弘),
	Laurent Pinchart, zwisler, srv_heupstream,
	Christie Yu (游雅惠), Hans Verkuil,
	Jungo Lin (林明俊), Sj Huang, yuzhao,
	Hans Verkuil, Pi-Hsun Shih,
	Frederic Chen (陳俊元), Matthias Brugger,
	moderated list:ARM/Mediatek SoC support, Mauro Carvalho Chehab,
	list@263.net:IOMMU DRIVERS <iommu@lists.linux-foundation.org>, Joerg Roedel <joro@8bytes.org>, ,
	Linux Media Mailing List
In-Reply-To: <1590755163.23156.24.camel@mtksdccf07>

On Fri, May 29, 2020 at 2:26 PM Jerry-ch Chen
<Jerry-ch.Chen@mediatek.com> wrote:
>
> Hi Tomasz,
>
> I Appreciate your review comments, here's the reply.
>
> On Mon, 2020-05-25 at 14:24 +0200, Tomasz Figa wrote:
> > r
> >
> > On Fri, May 22, 2020 at 4:11 PM Jerry-ch Chen
> > <Jerry-ch.Chen@mediatek.com> wrote:
> > >
> > > Hi Tomasz,
> > >
> > > On Thu, 2020-05-21 at 18:28 +0000, Tomasz Figa wrote:
> > > > Hi Jerry,
> > > >
> > > > On Wed, Dec 04, 2019 at 08:47:32PM +0800, Jerry-ch Chen wrote:
[snip]
> > Isn't still a need to clamp() width and height to min/max, though?
> Yes, I'll add them back.
>
> This function will be refined as :
>
> static void mtk_fd_fill_pixfmt_mp(struct v4l2_pix_format_mplane *dfmt,
>                                   u32 pixfmt)
> {
>         v4l2_fill_pixfmt_mp(dfmt, pixfmt, dfmt->width, dfmt->height);
>
>         dfmt->field = V4L2_FIELD_NONE;
>         dfmt->colorspace = V4L2_COLORSPACE_BT2020;
>         dfmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
>         dfmt->quantization = V4L2_QUANTIZATION_DEFAULT;
>         dfmt->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(dfmt->colorspace);
>
>         /* Keep user setting as possible */
>         dfmt->width = clamp(dfmt->width,
>                             MTK_FD_OUTPUT_MIN_WIDTH,
>                             MTK_FD_OUTPUT_MAX_WIDTH);
>         dfmt->height = clamp(dfmt->height,
>                              MTK_FD_OUTPUT_MIN_HEIGHT,
>                              MTK_FD_OUTPUT_MAX_HEIGHT);

Note that this would cause the other fields of dfmt to be inconsistent
with width and height. The correct way to do this would be to first
clamp and then call v4l2_fill_pixfmt_mp().

Best regards,
Tomasz

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] ASoC: mediatek: mt6358: support DMIC one-wire mode
From: Mark Brown @ 2020-05-29 13:05 UTC (permalink / raw)
  To: Tzung-Bi Shih
  Cc: Hariprasad Kelam, Linux Kernel Mailing List, howie.huang,
	Takashi Iwai, ALSA development, Jiaxin Yu, Liam Girdwood,
	linux-mediatek, Matthias Brugger, linux-arm-kernel
In-Reply-To: <CA+Px+wVSwJK-=75chKLjSEe3bPRtV2wD95W5D_pdR0Pw0G470A@mail.gmail.com>


[-- Attachment #1.1: Type: text/plain, Size: 548 bytes --]

On Fri, May 29, 2020 at 07:22:43PM +0800, Tzung-Bi Shih wrote:
> On Fri, May 29, 2020 at 7:09 PM Mark Brown <broonie@kernel.org> wrote:

> > What is DMIC one wire mode?  This doesn't sound like something I'd
> > expect to vary at runtime.

> It means: 1 PDM data wire carries 2 channel data (rising edge for left
> and falling edge for right).

I thought that was normal for DMICs - is this selecting between left and
right or something?

> The setting shouldn't and won't change at runtime.  Would you suggest
> putting it into DTS binding?

Yes.

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH v2 2/3] media: rockchip: Introduce driver for Rockhip's camera interface
From: Maxime Chevallier @ 2020-05-29 13:04 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Robin Murphy, Rob Herring, Mark Rutland,
	Heiko Stuebner, Hans Verkuil
  Cc: devicetree, linux-kernel, Maxime Chevallier, Paul Kocialkowski,
	linux-rockchip, Thomas Petazzoni, Miquel Raynal, linux-arm-kernel,
	linux-media
In-Reply-To: <20200529130405.929429-1-maxime.chevallier@bootlin.com>

Introduce a driver for the camera interface on some Rockchip platforms.

This controller supports CSI2, Parallel and BT656 interfaces, but for
now only the parallel interface could be tested, hence it's the only one
that's supported in the first version of this driver.

This controller can be fond on PX30, RK1808, RK3128, RK3288 and RK3288,
but for now it's only be tested on PX30.

Most of this driver was written follwing the BSP driver from rockchip,
removing the parts that either didn't fit correctly the guidelines, or
that couldn't be tested.

This basic version doesn't support cropping nor scaling, and is only
designed with one sensor being attached to it a any time.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---

Changes since V1 :

 - Convert to the bulk APIs for clocks and resets
 - remove useless references to priv data
 - Move around some init functions at probe time
 - Upate some helpers to more suitable ones

Here is the output from v4l2-compliance. There are no fails in the final
summary, but there is one in the output that I didn't catch previously.

Still, here's the V2 in the meantime, if you have any further reviews
ompliance SHA: not available, 64 bits

Compliance test for rkcif device /dev/video0:

Driver Info:
	Driver name      : rkcif
	Card type        : rkcif
	Bus info         : platform:ff490000.cif
	Driver version   : 5.7.0
	Capabilities     : 0x84201000
		Video Capture Multiplanar
		Streaming
		Extended Pix Format
		Device Capabilities
	Device Caps      : 0x04201000
		Video Capture Multiplanar
		Streaming
		Extended Pix Format
Media Driver Info:
	Driver name      : rkcif
	Model            : rkcif
	Serial           : 
	Bus info         : 
	Media version    : 5.7.0
	Hardware revision: 0x00000000 (0)
	Driver version   : 5.7.0
Interface Info:
	ID               : 0x03000002
	Type             : V4L Video
Entity Info:
	ID               : 0x00000001 (1)
	Name             : video_rkcif
	Function         : V4L2 I/O
	Pad 0x01000004   : 0: Sink
	  Link 0x02000007: from remote pad 0x1000006 of entity 'tw9900 2-0044': Data, Enabled

Required ioctls:
	test MC information (see 'Media Driver Info' above): OK
	test VIDIOC_QUERYCAP: OK

Allow for multiple opens:
	test second /dev/video0 open: OK
	test VIDIOC_QUERYCAP: OK
	test VIDIOC_G/S_PRIORITY: OK
	test for unlimited opens: OK

Debug ioctls:
	test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
	test VIDIOC_LOG_STATUS: OK (Not Supported)

Input ioctls:
	test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
	test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
	test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
	test VIDIOC_ENUMAUDIO: OK (Not Supported)
	test VIDIOC_G/S/ENUMINPUT: OK
	test VIDIOC_G/S_AUDIO: OK (Not Supported)
	Inputs: 1 Audio Inputs: 0 Tuners: 0

Output ioctls:
	test VIDIOC_G/S_MODULATOR: OK (Not Supported)
	test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
	test VIDIOC_ENUMAUDOUT: OK (Not Supported)
	test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
	test VIDIOC_G/S_AUDOUT: OK (Not Supported)
	Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
	test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
	test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
	test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
	test VIDIOC_G/S_EDID: OK (Not Supported)

Control ioctls (Input 0):
	test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
	test VIDIOC_QUERYCTRL: OK
	test VIDIOC_G/S_CTRL: OK
	test VIDIOC_G/S/TRY_EXT_CTRLS: OK
	test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK (Not Supported)
	test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
	Standard Controls: 0 Private Controls: 0

Format ioctls (Input 0):
	test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
	test VIDIOC_G/S_PARM: OK (Not Supported)
	test VIDIOC_G_FBUF: OK (Not Supported)
	test VIDIOC_G_FMT: OK
	test VIDIOC_TRY_FMT: OK
	test VIDIOC_S_FMT: OK
	test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
	test Cropping: OK (Not Supported)
	test Composing: OK (Not Supported)
		fail: v4l2-test-formats.cpp(1772): node->can_scale && node->frmsizes_count[v4l_format_g_pixelformat(&cur)]
	test Scaling: OK

Codec ioctls (Input 0):
	test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
	test VIDIOC_G_ENC_INDEX: OK (Not Supported)
	test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)

Buffer ioctls (Input 0):
	test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
	test VIDIOC_EXPBUF: OK
	test Requests: OK (Not Supported)

Total for rkcif device /dev/video0: 45, Succeeded: 45, Failed: 0, Warnings: 0

 drivers/media/platform/Kconfig                |   13 +
 drivers/media/platform/Makefile               |    1 +
 drivers/media/platform/rockchip/cif/Makefile  |    3 +
 drivers/media/platform/rockchip/cif/capture.c | 1170 +++++++++++++++++
 drivers/media/platform/rockchip/cif/dev.c     |  358 +++++
 drivers/media/platform/rockchip/cif/dev.h     |  213 +++
 drivers/media/platform/rockchip/cif/regs.h    |  256 ++++
 7 files changed, 2014 insertions(+)
 create mode 100644 drivers/media/platform/rockchip/cif/Makefile
 create mode 100644 drivers/media/platform/rockchip/cif/capture.c
 create mode 100644 drivers/media/platform/rockchip/cif/dev.c
 create mode 100644 drivers/media/platform/rockchip/cif/dev.h
 create mode 100644 drivers/media/platform/rockchip/cif/regs.h

diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index e01bbb9dd1c1..d4ec5e36bca7 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -460,6 +460,19 @@ config VIDEO_ROCKCHIP_RGA
 
 	  To compile this driver as a module choose m here.
 
+config VIDEO_ROCKCHIP_CIF
+	tristate "Rockchip Camera Interface"
+	depends on VIDEO_DEV && VIDEO_V4L2
+	depends on ARCH_ROCKCHIP || COMPILE_TEST
+	select VIDEOBUF2_DMA_SG
+	select VIDEOBUF2_DMA_CONTIG
+	select V4L2_FWNODE
+	select V4L2_MEM2MEM_DEV
+	help
+	  This is a v4l2 driver for Rockchip SOC Camera interface.
+
+	  To compile this driver as a module choose m here.
+
 config VIDEO_TI_VPE
 	tristate "TI VPE (Video Processing Engine) driver"
 	depends on VIDEO_DEV && VIDEO_V4L2
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index d13db96e3015..67e7ac034be1 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -68,6 +68,7 @@ obj-$(CONFIG_VIDEO_RENESAS_JPU)		+= rcar_jpu.o
 obj-$(CONFIG_VIDEO_RENESAS_VSP1)	+= vsp1/
 
 obj-$(CONFIG_VIDEO_ROCKCHIP_RGA)	+= rockchip/rga/
+obj-$(CONFIG_VIDEO_ROCKCHIP_CIF)	+= rockchip/cif/
 
 obj-y	+= omap/
 
diff --git a/drivers/media/platform/rockchip/cif/Makefile b/drivers/media/platform/rockchip/cif/Makefile
new file mode 100644
index 000000000000..727990824316
--- /dev/null
+++ b/drivers/media/platform/rockchip/cif/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_VIDEO_ROCKCHIP_CIF) += video_rkcif.o
+video_rkcif-objs += dev.o capture.o
diff --git a/drivers/media/platform/rockchip/cif/capture.c b/drivers/media/platform/rockchip/cif/capture.c
new file mode 100644
index 000000000000..adab6704129f
--- /dev/null
+++ b/drivers/media/platform/rockchip/cif/capture.c
@@ -0,0 +1,1170 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Rockchip CIF Driver
+ *
+ * Copyright (C) 2018 Rockchip Electronics Co., Ltd.
+ * Copyright (C) 2020 Maxime Chevallier <maxime.chevallier@bootlin.com>
+ */
+
+#include <linux/delay.h>
+#include <linux/pm_runtime.h>
+#include <linux/reset.h>
+#include <media/v4l2-common.h>
+#include <media/v4l2-event.h>
+#include <media/v4l2-fh.h>
+#include <media/v4l2-fwnode.h>
+#include <media/v4l2-ioctl.h>
+#include <media/v4l2-mc.h>
+#include <media/v4l2-subdev.h>
+#include <media/videobuf2-dma-contig.h>
+
+#include "dev.h"
+#include "regs.h"
+
+#define CIF_REQ_BUFS_MIN	1
+#define CIF_MIN_WIDTH		64
+#define CIF_MIN_HEIGHT		64
+#define CIF_MAX_WIDTH		8192
+#define CIF_MAX_HEIGHT		8192
+
+#define RKCIF_PLANE_Y			0
+#define RKCIF_PLANE_CBCR		1
+
+#define CIF_FETCH_Y_LAST_LINE(VAL) ((VAL) & 0x1fff)
+/* Check if swap y and c in bt1120 mode */
+#define CIF_FETCH_IS_Y_FIRST(VAL) ((VAL) & 0xf)
+
+/* Get xsubs and ysubs for fourcc formats
+ *
+ * @xsubs: horizontal color samples in a 4*4 matrix, for yuv
+ * @ysubs: vertical color samples in a 4*4 matrix, for yuv
+ */
+static int fcc_xysubs(u32 fcc, u32 *xsubs, u32 *ysubs)
+{
+	switch (fcc) {
+	case V4L2_PIX_FMT_NV16:
+	case V4L2_PIX_FMT_NV61:
+		*xsubs = 2;
+		*ysubs = 1;
+		break;
+	case V4L2_PIX_FMT_NV21:
+	case V4L2_PIX_FMT_NV12:
+		*xsubs = 2;
+		*ysubs = 2;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static const struct cif_output_fmt out_fmts[] = {
+	{
+		.fourcc = V4L2_PIX_FMT_NV16,
+		.cplanes = 2,
+		.mplanes = 1,
+		.fmt_val = YUV_OUTPUT_422 | UV_STORAGE_ORDER_UVUV,
+		.bpp = { 8, 16 },
+	}, {
+		.fourcc = V4L2_PIX_FMT_NV61,
+		.fmt_val = YUV_OUTPUT_422 | UV_STORAGE_ORDER_VUVU,
+		.cplanes = 2,
+		.mplanes = 1,
+		.bpp = { 8, 16 },
+	},
+	{
+		.fourcc = V4L2_PIX_FMT_NV12,
+		.fmt_val = YUV_OUTPUT_420 | UV_STORAGE_ORDER_UVUV,
+		.cplanes = 2,
+		.mplanes = 1,
+		.bpp = { 8, 16 },
+		.mbus = MEDIA_BUS_FMT_UYVY8_2X8,
+	},
+	{
+		.fourcc = V4L2_PIX_FMT_NV21,
+		.fmt_val = YUV_OUTPUT_420 | UV_STORAGE_ORDER_VUVU,
+		.cplanes = 2,
+		.mplanes = 1,
+		.bpp = { 8, 16 },
+	}, {
+		.fourcc = V4L2_PIX_FMT_RGB24,
+		.cplanes = 1,
+		.mplanes = 1,
+		.bpp = { 24 },
+	}, {
+		.fourcc = V4L2_PIX_FMT_RGB565,
+		.cplanes = 1,
+		.mplanes = 1,
+		.bpp = { 16 },
+	}, {
+		.fourcc = V4L2_PIX_FMT_BGR666,
+		.cplanes = 1,
+		.mplanes = 1,
+		.bpp = { 18 },
+	}, {
+		.fourcc = V4L2_PIX_FMT_SRGGB8,
+		.cplanes = 1,
+		.mplanes = 1,
+		.bpp = { 8 },
+	}, {
+		.fourcc = V4L2_PIX_FMT_SGRBG8,
+		.cplanes = 1,
+		.mplanes = 1,
+		.bpp = { 8 },
+	}, {
+		.fourcc = V4L2_PIX_FMT_SGBRG8,
+		.cplanes = 1,
+		.mplanes = 1,
+		.bpp = { 8 },
+	}, {
+		.fourcc = V4L2_PIX_FMT_SBGGR8,
+		.cplanes = 1,
+		.mplanes = 1,
+		.bpp = { 8 },
+	}, {
+		.fourcc = V4L2_PIX_FMT_SRGGB10,
+		.cplanes = 1,
+		.mplanes = 1,
+		.bpp = { 16 },
+	}, {
+		.fourcc = V4L2_PIX_FMT_SGRBG10,
+		.cplanes = 1,
+		.mplanes = 1,
+		.bpp = { 16 },
+	}, {
+		.fourcc = V4L2_PIX_FMT_SGBRG10,
+		.cplanes = 1,
+		.mplanes = 1,
+		.bpp = { 16 },
+	}, {
+		.fourcc = V4L2_PIX_FMT_SBGGR10,
+		.cplanes = 1,
+		.mplanes = 1,
+		.bpp = { 16 },
+	}, {
+		.fourcc = V4L2_PIX_FMT_SRGGB12,
+		.cplanes = 1,
+		.mplanes = 1,
+		.bpp = { 16 },
+	}, {
+		.fourcc = V4L2_PIX_FMT_SGRBG12,
+		.cplanes = 1,
+		.mplanes = 1,
+		.bpp = { 16 },
+	}, {
+		.fourcc = V4L2_PIX_FMT_SGBRG12,
+		.cplanes = 1,
+		.mplanes = 1,
+		.bpp = { 16 },
+	}, {
+		.fourcc = V4L2_PIX_FMT_SBGGR12,
+		.cplanes = 1,
+		.mplanes = 1,
+		.bpp = { 16 },
+	}, {
+		.fourcc = V4L2_PIX_FMT_SBGGR16,
+		.cplanes = 1,
+		.mplanes = 1,
+		.bpp = { 16 },
+	}, {
+		.fourcc = V4L2_PIX_FMT_Y16,
+		.cplanes = 1,
+		.mplanes = 1,
+		.bpp = { 16 },
+	}
+};
+
+static const struct cif_input_fmt in_fmts[] = {
+	{
+		.mbus_code	= MEDIA_BUS_FMT_YUYV8_2X8,
+		.dvp_fmt_val	= YUV_INPUT_422 | YUV_INPUT_ORDER_YUYV,
+		.csi_fmt_val	= CSI_WRDDR_TYPE_YUV422,
+		.fmt_type	= CIF_FMT_TYPE_YUV,
+		.field		= V4L2_FIELD_NONE,
+	}, {
+		.mbus_code	= MEDIA_BUS_FMT_YUYV8_2X8,
+		.dvp_fmt_val	= YUV_INPUT_422 | YUV_INPUT_ORDER_YUYV,
+		.csi_fmt_val	= CSI_WRDDR_TYPE_YUV422,
+		.fmt_type	= CIF_FMT_TYPE_YUV,
+		.field		= V4L2_FIELD_INTERLACED,
+	}, {
+		.mbus_code	= MEDIA_BUS_FMT_YVYU8_2X8,
+		.dvp_fmt_val	= YUV_INPUT_422 | YUV_INPUT_ORDER_YVYU,
+		.csi_fmt_val	= CSI_WRDDR_TYPE_YUV422,
+		.fmt_type	= CIF_FMT_TYPE_YUV,
+		.field		= V4L2_FIELD_NONE,
+	}, {
+		.mbus_code	= MEDIA_BUS_FMT_YVYU8_2X8,
+		.dvp_fmt_val	= YUV_INPUT_422 | YUV_INPUT_ORDER_YVYU,
+		.csi_fmt_val	= CSI_WRDDR_TYPE_YUV422,
+		.fmt_type	= CIF_FMT_TYPE_YUV,
+		.field		= V4L2_FIELD_INTERLACED,
+	}, {
+		.mbus_code	= MEDIA_BUS_FMT_UYVY8_2X8,
+		.dvp_fmt_val	= YUV_INPUT_422 | YUV_INPUT_ORDER_UYVY,
+		.csi_fmt_val	= CSI_WRDDR_TYPE_YUV422,
+		.fmt_type	= CIF_FMT_TYPE_YUV,
+		.field		= V4L2_FIELD_NONE,
+	}, {
+		.mbus_code	= MEDIA_BUS_FMT_UYVY8_2X8,
+		.dvp_fmt_val	= YUV_INPUT_422 | YUV_INPUT_ORDER_UYVY,
+		.csi_fmt_val	= CSI_WRDDR_TYPE_YUV422,
+		.fmt_type	= CIF_FMT_TYPE_YUV,
+		.field		= V4L2_FIELD_INTERLACED,
+	}, {
+		.mbus_code	= MEDIA_BUS_FMT_VYUY8_2X8,
+		.dvp_fmt_val	= YUV_INPUT_422 | YUV_INPUT_ORDER_VYUY,
+		.csi_fmt_val	= CSI_WRDDR_TYPE_YUV422,
+		.fmt_type	= CIF_FMT_TYPE_YUV,
+		.field		= V4L2_FIELD_NONE,
+	}, {
+		.mbus_code	= MEDIA_BUS_FMT_VYUY8_2X8,
+		.dvp_fmt_val	= YUV_INPUT_422 | YUV_INPUT_ORDER_VYUY,
+		.csi_fmt_val	= CSI_WRDDR_TYPE_YUV422,
+		.fmt_type	= CIF_FMT_TYPE_YUV,
+		.field		= V4L2_FIELD_INTERLACED,
+	}, {
+		.mbus_code	= MEDIA_BUS_FMT_SBGGR8_1X8,
+		.dvp_fmt_val	= INPUT_MODE_RAW | RAW_DATA_WIDTH_8,
+		.csi_fmt_val	= CSI_WRDDR_TYPE_RAW8,
+		.fmt_type	= CIF_FMT_TYPE_RAW,
+		.field		= V4L2_FIELD_NONE,
+	}, {
+		.mbus_code	= MEDIA_BUS_FMT_SGBRG8_1X8,
+		.dvp_fmt_val	= INPUT_MODE_RAW | RAW_DATA_WIDTH_8,
+		.csi_fmt_val	= CSI_WRDDR_TYPE_RAW8,
+		.fmt_type	= CIF_FMT_TYPE_RAW,
+		.field		= V4L2_FIELD_NONE,
+	}, {
+		.mbus_code	= MEDIA_BUS_FMT_SGRBG8_1X8,
+		.dvp_fmt_val	= INPUT_MODE_RAW | RAW_DATA_WIDTH_8,
+		.csi_fmt_val	= CSI_WRDDR_TYPE_RAW8,
+		.fmt_type	= CIF_FMT_TYPE_RAW,
+		.field		= V4L2_FIELD_NONE,
+	}, {
+		.mbus_code	= MEDIA_BUS_FMT_SRGGB8_1X8,
+		.dvp_fmt_val	= INPUT_MODE_RAW | RAW_DATA_WIDTH_8,
+		.csi_fmt_val	= CSI_WRDDR_TYPE_RAW8,
+		.fmt_type	= CIF_FMT_TYPE_RAW,
+		.field		= V4L2_FIELD_NONE,
+	}, {
+		.mbus_code	= MEDIA_BUS_FMT_SBGGR10_1X10,
+		.dvp_fmt_val	= INPUT_MODE_RAW | RAW_DATA_WIDTH_10,
+		.csi_fmt_val	= CSI_WRDDR_TYPE_RAW10,
+		.fmt_type	= CIF_FMT_TYPE_RAW,
+		.field		= V4L2_FIELD_NONE,
+	}, {
+		.mbus_code	= MEDIA_BUS_FMT_SGBRG10_1X10,
+		.dvp_fmt_val	= INPUT_MODE_RAW | RAW_DATA_WIDTH_10,
+		.csi_fmt_val	= CSI_WRDDR_TYPE_RAW10,
+		.fmt_type	= CIF_FMT_TYPE_RAW,
+		.field		= V4L2_FIELD_NONE,
+	}, {
+		.mbus_code	= MEDIA_BUS_FMT_SGRBG10_1X10,
+		.dvp_fmt_val	= INPUT_MODE_RAW | RAW_DATA_WIDTH_10,
+		.csi_fmt_val	= CSI_WRDDR_TYPE_RAW10,
+		.fmt_type	= CIF_FMT_TYPE_RAW,
+		.field		= V4L2_FIELD_NONE,
+	}, {
+		.mbus_code	= MEDIA_BUS_FMT_SRGGB10_1X10,
+		.dvp_fmt_val	= INPUT_MODE_RAW | RAW_DATA_WIDTH_10,
+		.csi_fmt_val	= CSI_WRDDR_TYPE_RAW10,
+		.fmt_type	= CIF_FMT_TYPE_RAW,
+		.field		= V4L2_FIELD_NONE,
+	}, {
+		.mbus_code	= MEDIA_BUS_FMT_SBGGR12_1X12,
+		.dvp_fmt_val	= INPUT_MODE_RAW | RAW_DATA_WIDTH_12,
+		.csi_fmt_val	= CSI_WRDDR_TYPE_RAW12,
+		.fmt_type	= CIF_FMT_TYPE_RAW,
+		.field		= V4L2_FIELD_NONE,
+	}, {
+		.mbus_code	= MEDIA_BUS_FMT_SGBRG12_1X12,
+		.dvp_fmt_val	= INPUT_MODE_RAW | RAW_DATA_WIDTH_12,
+		.csi_fmt_val	= CSI_WRDDR_TYPE_RAW12,
+		.fmt_type	= CIF_FMT_TYPE_RAW,
+		.field		= V4L2_FIELD_NONE,
+	}, {
+		.mbus_code	= MEDIA_BUS_FMT_SGRBG12_1X12,
+		.dvp_fmt_val	= INPUT_MODE_RAW | RAW_DATA_WIDTH_12,
+		.csi_fmt_val	= CSI_WRDDR_TYPE_RAW12,
+		.fmt_type	= CIF_FMT_TYPE_RAW,
+		.field		= V4L2_FIELD_NONE,
+	}, {
+		.mbus_code	= MEDIA_BUS_FMT_SRGGB12_1X12,
+		.dvp_fmt_val	= INPUT_MODE_RAW | RAW_DATA_WIDTH_12,
+		.csi_fmt_val	= CSI_WRDDR_TYPE_RAW12,
+		.fmt_type	= CIF_FMT_TYPE_RAW,
+		.field		= V4L2_FIELD_NONE,
+	}, {
+		.mbus_code	= MEDIA_BUS_FMT_RGB888_1X24,
+		.csi_fmt_val	= CSI_WRDDR_TYPE_RGB888,
+		.field		= V4L2_FIELD_NONE,
+	}, {
+		.mbus_code	= MEDIA_BUS_FMT_Y8_1X8,
+		.dvp_fmt_val	= INPUT_MODE_RAW | RAW_DATA_WIDTH_8,
+		.csi_fmt_val	= CSI_WRDDR_TYPE_RAW8,
+		.fmt_type	= CIF_FMT_TYPE_RAW,
+		.field		= V4L2_FIELD_NONE,
+	}, {
+		.mbus_code	= MEDIA_BUS_FMT_Y10_1X10,
+		.dvp_fmt_val	= INPUT_MODE_RAW | RAW_DATA_WIDTH_10,
+		.csi_fmt_val	= CSI_WRDDR_TYPE_RAW10,
+		.fmt_type	= CIF_FMT_TYPE_RAW,
+		.field		= V4L2_FIELD_NONE,
+	}, {
+		.mbus_code	= MEDIA_BUS_FMT_Y12_1X12,
+		.dvp_fmt_val	= INPUT_MODE_RAW | RAW_DATA_WIDTH_12,
+		.csi_fmt_val	= CSI_WRDDR_TYPE_RAW12,
+		.fmt_type	= CIF_FMT_TYPE_RAW,
+		.field		= V4L2_FIELD_NONE,
+	}
+};
+
+static const struct
+cif_input_fmt *get_input_fmt(struct v4l2_subdev *sd)
+{
+	struct v4l2_subdev_format fmt;
+	int ret;
+	u32 i;
+
+	fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
+	fmt.pad = 0;
+	ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &fmt);
+	if (ret < 0) {
+		v4l2_warn(sd->v4l2_dev,
+			  "sensor fmt invalid, set to default size\n");
+		goto set_default;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(in_fmts); i++)
+		if (fmt.format.code == in_fmts[i].mbus_code &&
+		    fmt.format.field == in_fmts[i].field)
+			return &in_fmts[i];
+
+	v4l2_err(sd->v4l2_dev, "remote sensor mbus code not supported\n");
+
+set_default:
+	return NULL;
+}
+
+	static const struct
+cif_output_fmt *find_output_fmt(struct rkcif_stream *stream, u32 pixelfmt)
+{
+	const struct cif_output_fmt *fmt;
+	u32 i;
+
+	for (i = 0; i < ARRAY_SIZE(out_fmts); i++) {
+		fmt = &out_fmts[i];
+		if (fmt->fourcc == pixelfmt)
+			return fmt;
+	}
+
+	return NULL;
+}
+
+/***************************** stream operations ******************************/
+static void rkcif_assign_new_buffer_oneframe(struct rkcif_stream *stream)
+{
+	struct rkcif_dummy_buffer *dummy_buf = &stream->dummy_buf;
+	struct rkcif_device *dev = stream->cifdev;
+	void __iomem *base = dev->base_addr;
+
+	/* Set up an empty buffer for the next frame */
+	spin_lock(&stream->vbq_lock);
+	if (!list_empty(&stream->buf_head)) {
+		stream->curr_buf = list_first_entry(&stream->buf_head,
+					struct rkcif_buffer, queue);
+		list_del(&stream->curr_buf->queue);
+	} else {
+		stream->curr_buf = NULL;
+	}
+	spin_unlock(&stream->vbq_lock);
+
+	if (stream->curr_buf) {
+		write_cif_reg(base, CIF_FRM0_ADDR_Y,
+			      stream->curr_buf->buff_addr[RKCIF_PLANE_Y]);
+		write_cif_reg(base, CIF_FRM0_ADDR_UV,
+			      stream->curr_buf->buff_addr[RKCIF_PLANE_CBCR]);
+		write_cif_reg(base, CIF_FRM1_ADDR_Y,
+			      stream->curr_buf->buff_addr[RKCIF_PLANE_Y]);
+		write_cif_reg(base, CIF_FRM1_ADDR_UV,
+			      stream->curr_buf->buff_addr[RKCIF_PLANE_CBCR]);
+	} else {
+		write_cif_reg(base, CIF_FRM0_ADDR_Y, dummy_buf->dma_addr);
+		write_cif_reg(base, CIF_FRM0_ADDR_UV, dummy_buf->dma_addr);
+		write_cif_reg(base, CIF_FRM1_ADDR_Y, dummy_buf->dma_addr);
+		write_cif_reg(base, CIF_FRM1_ADDR_UV, dummy_buf->dma_addr);
+	}
+}
+
+static void rkcif_stream_stop(struct rkcif_stream *stream)
+{
+	struct rkcif_device *cif_dev = stream->cifdev;
+	void __iomem *base = cif_dev->base_addr;
+	u32 val;
+
+	val = read_cif_reg(base, CIF_CTRL);
+	write_cif_reg(base, CIF_CTRL, val & (~ENABLE_CAPTURE));
+	write_cif_reg(base, CIF_INTEN, 0x0);
+	write_cif_reg(base, CIF_INTSTAT, 0x3ff);
+	write_cif_reg(base, CIF_FRAME_STATUS, 0x0);
+
+	stream->state = RKCIF_STATE_READY;
+}
+
+static int rkcif_queue_setup(struct vb2_queue *queue,
+			     unsigned int *num_buffers,
+			     unsigned int *num_planes,
+			     unsigned int sizes[],
+			     struct device *alloc_devs[])
+{
+	struct rkcif_stream *stream = queue->drv_priv;
+	const struct v4l2_pix_format_mplane *pixm;
+	const struct cif_output_fmt *cif_fmt;
+	u32 i;
+
+	pixm = &stream->pixm;
+	cif_fmt = stream->cif_fmt_out;
+
+	if (*num_planes) {
+		if (*num_planes != cif_fmt->mplanes)
+			return -EINVAL;
+
+		for (i = 0; i < cif_fmt->mplanes; i++)
+			if (sizes[i] < pixm->plane_fmt[i].sizeimage)
+				return -EINVAL;
+		return 0;
+	}
+
+	*num_planes = cif_fmt->mplanes;
+
+	for (i = 0; i < cif_fmt->mplanes; i++) {
+		const struct v4l2_plane_pix_format *plane_fmt;
+
+		plane_fmt = &pixm->plane_fmt[i];
+		sizes[i] = plane_fmt->sizeimage;
+	}
+
+	return 0;
+}
+
+/*
+ * The vb2_buffer are stored in rkcif_buffer, in order to unify
+ * mplane buffer and none-mplane buffer.
+ */
+static void rkcif_buf_queue(struct vb2_buffer *vb)
+{
+	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
+	struct rkcif_buffer *cifbuf = to_rkcif_buffer(vbuf);
+	struct vb2_queue *queue = vb->vb2_queue;
+	struct rkcif_stream *stream = queue->drv_priv;
+	struct v4l2_pix_format_mplane *pixm = &stream->pixm;
+	const struct cif_output_fmt *fmt = stream->cif_fmt_out;
+	unsigned long lock_flags = 0;
+	int i;
+
+	memset(cifbuf->buff_addr, 0, sizeof(cifbuf->buff_addr));
+	/* If mplanes > 1, every c-plane has its own m-plane,
+	 * otherwise, multiple c-planes are in the same m-plane
+	 */
+	for (i = 0; i < fmt->mplanes; i++)
+		cifbuf->buff_addr[i] = vb2_dma_contig_plane_dma_addr(vb, i);
+
+	if (fmt->mplanes == 1) {
+		for (i = 0; i < fmt->cplanes - 1; i++)
+			cifbuf->buff_addr[i + 1] = cifbuf->buff_addr[i] +
+				pixm->plane_fmt[i].bytesperline * pixm->height;
+	}
+
+	spin_lock_irqsave(&stream->vbq_lock, lock_flags);
+	list_add_tail(&cifbuf->queue, &stream->buf_head);
+	spin_unlock_irqrestore(&stream->vbq_lock, lock_flags);
+}
+
+static int rkcif_create_dummy_buf(struct rkcif_stream *stream)
+{
+	struct rkcif_dummy_buffer *dummy_buf = &stream->dummy_buf;
+	struct rkcif_device *dev = stream->cifdev;
+
+	/* get a maximum plane size */
+	dummy_buf->size = max3(stream->pixm.plane_fmt[0].bytesperline *
+		stream->pixm.height,
+		stream->pixm.plane_fmt[1].sizeimage,
+		stream->pixm.plane_fmt[2].sizeimage);
+
+	dummy_buf->vaddr = dma_alloc_coherent(dev->dev, dummy_buf->size,
+					      &dummy_buf->dma_addr,
+					      GFP_KERNEL);
+	if (!dummy_buf->vaddr) {
+		v4l2_err(&dev->v4l2_dev,
+			 "Failed to allocate the memory for dummy buffer\n");
+		return -ENOMEM;
+	}
+
+	v4l2_info(&dev->v4l2_dev, "Allocate dummy buffer, size: 0x%08x\n",
+		  dummy_buf->size);
+
+	return 0;
+}
+
+static void rkcif_destroy_dummy_buf(struct rkcif_stream *stream)
+{
+	struct rkcif_dummy_buffer *dummy_buf = &stream->dummy_buf;
+	struct rkcif_device *dev = stream->cifdev;
+
+	dma_free_coherent(dev->dev, dummy_buf->size,
+			  dummy_buf->vaddr, dummy_buf->dma_addr);
+}
+
+static void rkcif_stop_streaming(struct vb2_queue *queue)
+{
+	struct rkcif_stream *stream = queue->drv_priv;
+	struct rkcif_device *dev = stream->cifdev;
+	struct rkcif_buffer *buf;
+	struct v4l2_subdev *sd;
+	int ret;
+
+	stream->stopping = true;
+	ret = wait_event_timeout(stream->wq_stopped,
+				 stream->state != RKCIF_STATE_STREAMING,
+				 msecs_to_jiffies(1000));
+	if (!ret) {
+		rkcif_stream_stop(stream);
+		stream->stopping = false;
+	}
+	pm_runtime_put(dev->dev);
+
+	/* stop the sub device*/
+	sd = dev->sensor.sd;
+	v4l2_subdev_call(sd, video, s_stream, 0);
+	v4l2_subdev_call(sd, core, s_power, 0);
+
+	/* release buffers */
+	if (stream->curr_buf) {
+		list_add_tail(&stream->curr_buf->queue, &stream->buf_head);
+		stream->curr_buf = NULL;
+	}
+	if (stream->next_buf) {
+		list_add_tail(&stream->next_buf->queue, &stream->buf_head);
+		stream->next_buf = NULL;
+	}
+
+	while (!list_empty(&stream->buf_head)) {
+		buf = list_first_entry(&stream->buf_head,
+				       struct rkcif_buffer, queue);
+		list_del(&buf->queue);
+		vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
+	}
+
+	rkcif_destroy_dummy_buf(stream);
+}
+
+static u32 rkcif_determine_input_mode(struct rkcif_device *dev)
+{
+	struct rkcif_sensor_info *sensor_info = &dev->sensor;
+	struct rkcif_stream *stream = &dev->stream;
+	v4l2_std_id std;
+	u32 mode = INPUT_MODE_YUV;
+	int ret;
+
+	ret = v4l2_subdev_call(sensor_info->sd, video, querystd, &std);
+	if (ret == 0) {
+		/* retrieve std from sensor if exist */
+		switch (std) {
+		case V4L2_STD_NTSC:
+			mode = INPUT_MODE_NTSC;
+			break;
+		case V4L2_STD_PAL:
+			mode = INPUT_MODE_PAL;
+			break;
+		case V4L2_STD_ATSC:
+			mode = INPUT_MODE_BT1120;
+			break;
+		default:
+			v4l2_err(&dev->v4l2_dev,
+				 "std: %lld is not supported", std);
+		}
+	} else {
+		/* determine input mode by mbus_code (fmt_type) */
+		switch (stream->cif_fmt_in->fmt_type) {
+		case CIF_FMT_TYPE_YUV:
+			mode = INPUT_MODE_YUV;
+			break;
+		case CIF_FMT_TYPE_RAW:
+			mode = INPUT_MODE_RAW;
+			break;
+		}
+	}
+
+	return mode;
+}
+
+static inline u32 rkcif_scl_ctl(struct rkcif_stream *stream)
+{
+	u32 fmt_type = stream->cif_fmt_in->fmt_type;
+
+	return (fmt_type == CIF_FMT_TYPE_YUV) ?
+		ENABLE_YUV_16BIT_BYPASS : ENABLE_RAW_16BIT_BYPASS;
+}
+
+static int rkcif_stream_start(struct rkcif_stream *stream)
+{
+	u32 val, mbus_flags, href_pol, vsync_pol,
+	    xfer_mode = 0, yc_swap = 0, skip_top = 0;
+	struct rkcif_device *dev = stream->cifdev;
+	struct rkcif_sensor_info *sensor_info;
+	void __iomem *base = dev->base_addr;
+
+	sensor_info = &dev->sensor;
+	stream->frame_idx = 0;
+
+	mbus_flags = sensor_info->mbus.flags;
+	href_pol = (mbus_flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH) ?
+			HSY_HIGH_ACTIVE : HSY_LOW_ACTIVE;
+	vsync_pol = (mbus_flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH) ?
+			VSY_HIGH_ACTIVE : VSY_LOW_ACTIVE;
+
+	if (rkcif_determine_input_mode(dev) == INPUT_MODE_BT1120) {
+		if (stream->cif_fmt_in->field == V4L2_FIELD_NONE)
+			xfer_mode = BT1120_TRANSMIT_PROGRESS;
+		else
+			xfer_mode = BT1120_TRANSMIT_INTERFACE;
+		if (!CIF_FETCH_IS_Y_FIRST(stream->cif_fmt_in->dvp_fmt_val))
+			yc_swap = BT1120_YC_SWAP;
+	}
+
+	val = vsync_pol | href_pol | rkcif_determine_input_mode(dev) |
+	      stream->cif_fmt_out->fmt_val | stream->cif_fmt_in->dvp_fmt_val |
+	      xfer_mode | yc_swap;
+	write_cif_reg(base, CIF_FOR, val);
+	val = stream->pixm.width;
+	if (stream->cif_fmt_in->fmt_type == CIF_FMT_TYPE_RAW)
+		val = stream->pixm.width * 2;
+	write_cif_reg(base, CIF_VIR_LINE_WIDTH, val);
+	write_cif_reg(base, CIF_SET_SIZE,
+		      stream->pixm.width | (stream->pixm.height << 16));
+
+	v4l2_subdev_call(sensor_info->sd, sensor, g_skip_top_lines, &skip_top);
+
+	write_cif_reg(base, CIF_CROP, skip_top << CIF_CROP_Y_SHIFT);
+	write_cif_reg(base, CIF_FRAME_STATUS, FRAME_STAT_CLS);
+	write_cif_reg(base, CIF_INTSTAT, INTSTAT_CLS);
+	write_cif_reg(base, CIF_SCL_CTRL, rkcif_scl_ctl(stream));
+
+	rkcif_assign_new_buffer_oneframe(stream);
+
+	write_cif_reg(base, CIF_INTEN, FRAME_END_EN | LINE_ERR_EN |
+			    PST_INF_FRAME_END);
+
+	if (dev->data->chip_id == CHIP_RK1808_CIF &&
+	    rkcif_determine_input_mode(dev) == INPUT_MODE_BT1120)
+		write_cif_reg(base, CIF_CTRL,
+			      AXI_BURST_16 | MODE_PINGPONG | ENABLE_CAPTURE);
+	else
+		write_cif_reg(base, CIF_CTRL,
+			      AXI_BURST_16 | MODE_ONEFRAME | ENABLE_CAPTURE);
+
+	stream->state = RKCIF_STATE_STREAMING;
+
+	return 0;
+}
+
+static int rkcif_start_streaming(struct vb2_queue *queue, unsigned int count)
+{
+	struct rkcif_stream *stream = queue->drv_priv;
+	struct rkcif_device *dev = stream->cifdev;
+	struct v4l2_device *v4l2_dev = &dev->v4l2_dev;
+	struct v4l2_subdev *sd;
+	int ret;
+
+	if (WARN_ON(stream->state != RKCIF_STATE_READY)) {
+		ret = -EBUSY;
+		v4l2_err(v4l2_dev, "stream in busy state\n");
+		goto destroy_buf;
+	}
+
+	stream->cif_fmt_in = get_input_fmt(dev->sensor.sd);
+
+	ret = rkcif_create_dummy_buf(stream);
+	if (ret < 0) {
+		v4l2_err(v4l2_dev, "Failed to create dummy_buf, %d\n", ret);
+		goto destroy_buf;
+	}
+
+	ret = pm_runtime_get_sync(dev->dev);
+	if (ret < 0) {
+		v4l2_err(v4l2_dev, "Failed to get runtime pm, %d\n", ret);
+		goto destroy_dummy_buf;
+	}
+
+	/* start sub-devices */
+	sd = dev->sensor.sd;
+	ret = v4l2_subdev_call(sd, core, s_power, 1);
+	if (ret < 0 && ret != -ENOIOCTLCMD)
+		goto runtime_put;
+	ret = v4l2_subdev_call(sd, video, s_stream, 1);
+	if (ret < 0)
+		goto subdev_poweroff;
+
+	ret = rkcif_stream_start(stream);
+	if (ret < 0)
+		goto stop_stream;
+
+	return 0;
+
+stop_stream:
+	rkcif_stream_stop(stream);
+subdev_poweroff:
+	v4l2_subdev_call(sd, core, s_power, 0);
+runtime_put:
+	pm_runtime_put(dev->dev);
+destroy_dummy_buf:
+	rkcif_destroy_dummy_buf(stream);
+destroy_buf:
+	while (!list_empty(&stream->buf_head)) {
+		struct rkcif_buffer *buf;
+
+		buf = list_first_entry(&stream->buf_head,
+				       struct rkcif_buffer, queue);
+		list_del(&buf->queue);
+		vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_QUEUED);
+	}
+
+	return ret;
+}
+
+static struct vb2_ops rkcif_vb2_ops = {
+	.queue_setup = rkcif_queue_setup,
+	.buf_queue = rkcif_buf_queue,
+	.wait_prepare = vb2_ops_wait_prepare,
+	.wait_finish = vb2_ops_wait_finish,
+	.stop_streaming = rkcif_stop_streaming,
+	.start_streaming = rkcif_start_streaming,
+};
+
+static int rkcif_init_vb2_queue(struct vb2_queue *q,
+				struct rkcif_stream *stream,
+				enum v4l2_buf_type buf_type)
+{
+	q->type = buf_type;
+	q->io_modes = VB2_MMAP | VB2_DMABUF;
+	q->drv_priv = stream;
+	q->ops = &rkcif_vb2_ops;
+	q->mem_ops = &vb2_dma_contig_memops;
+	q->buf_struct_size = sizeof(struct rkcif_buffer);
+	q->min_buffers_needed = CIF_REQ_BUFS_MIN;
+	q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
+	q->lock = &stream->vlock;
+	q->dev = stream->cifdev->dev;
+
+	return vb2_queue_init(q);
+}
+
+static void rkcif_set_fmt(struct rkcif_stream *stream,
+			  struct v4l2_pix_format_mplane *pixm,
+			  bool try)
+{
+	const struct cif_output_fmt *fmt;
+	struct v4l2_rect input_rect;
+	unsigned int imagesize = 0, planes;
+	u32 xsubs = 1, ysubs = 1, i;
+
+	fmt = find_output_fmt(stream, pixm->pixelformat);
+	if (!fmt)
+		fmt = &out_fmts[0];
+
+	input_rect.width = CIF_MAX_WIDTH;
+	input_rect.height = CIF_MAX_HEIGHT;
+
+	pixm->width = clamp_t(u32, pixm->width,
+				CIF_MIN_WIDTH, input_rect.width);
+	pixm->height = clamp_t(u32, pixm->height,
+				CIF_MIN_HEIGHT, input_rect.height);
+
+	pixm->num_planes = fmt->mplanes;
+	pixm->quantization = V4L2_QUANTIZATION_DEFAULT;
+	pixm->colorspace = V4L2_COLORSPACE_SRGB;
+
+	pixm->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(pixm->colorspace);
+	pixm->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(pixm->colorspace);
+
+	pixm->pixelformat = fmt->fourcc;
+	pixm->field = V4L2_FIELD_NONE;
+
+	/* calculate plane size and image size */
+	fcc_xysubs(fmt->fourcc, &xsubs, &ysubs);
+
+	planes = fmt->cplanes ? fmt->cplanes : fmt->mplanes;
+
+	for (i = 0; i < planes; i++) {
+		struct v4l2_plane_pix_format *plane_fmt;
+		int width, height, bpl, size;
+
+		if (i == 0) {
+			width = pixm->width;
+			height = pixm->height;
+		} else {
+			width = pixm->width / xsubs;
+			height = pixm->height / ysubs;
+		}
+
+		bpl = width * fmt->bpp[i] / 8;
+		size = bpl * height;
+		imagesize += size;
+
+		if (fmt->mplanes > i) {
+			/* Set bpl and size for each mplane */
+			plane_fmt = pixm->plane_fmt + i;
+			plane_fmt->bytesperline = bpl;
+			plane_fmt->sizeimage = size;
+		}
+	}
+
+	/* convert to non-MPLANE format.
+	 * It's important since we want to unify non-MPLANE
+	 * and MPLANE.
+	 */
+	if (fmt->mplanes == 1)
+		pixm->plane_fmt[0].sizeimage = imagesize;
+
+	if (!try) {
+		stream->cif_fmt_out = fmt;
+		stream->pixm = *pixm;
+	}
+}
+
+void rkcif_stream_init(struct rkcif_device *dev)
+{
+	struct rkcif_stream *stream = &dev->stream;
+	struct v4l2_pix_format_mplane pixm;
+
+	memset(stream, 0, sizeof(*stream));
+	memset(&pixm, 0, sizeof(pixm));
+	stream->cifdev = dev;
+
+	INIT_LIST_HEAD(&stream->buf_head);
+	spin_lock_init(&stream->vbq_lock);
+	stream->state = RKCIF_STATE_READY;
+	init_waitqueue_head(&stream->wq_stopped);
+
+	/* Set default format */
+	pixm.pixelformat = V4L2_PIX_FMT_NV12;
+	pixm.width = RKCIF_DEFAULT_WIDTH;
+	pixm.height = RKCIF_DEFAULT_HEIGHT;
+	rkcif_set_fmt(stream, &pixm, false);
+
+	stream->crop.left = 0;
+	stream->crop.top = 0;
+	stream->crop.width = 10;
+	stream->crop.height = 10;
+}
+
+static int rkcif_fh_open(struct file *filp)
+{
+	struct video_device *vdev = video_devdata(filp);
+	struct rkcif_stream *stream = to_rkcif_stream(vdev);
+	struct rkcif_device *cifdev = stream->cifdev;
+
+	rkcif_soft_reset(cifdev);
+
+	return v4l2_fh_open(filp);
+}
+
+static const struct v4l2_file_operations rkcif_fops = {
+	.open = rkcif_fh_open,
+	.release = vb2_fop_release,
+	.unlocked_ioctl = video_ioctl2,
+	.poll = vb2_fop_poll,
+	.mmap = vb2_fop_mmap,
+};
+
+static int rkcif_enum_input(struct file *file, void *priv,
+			    struct v4l2_input *input)
+{
+	if (input->index > 0)
+		return -EINVAL;
+
+	input->type = V4L2_INPUT_TYPE_CAMERA;
+	strlcpy(input->name, "Camera", sizeof(input->name));
+
+	return 0;
+}
+
+static int rkcif_try_fmt_vid_cap_mplane(struct file *file, void *fh,
+					struct v4l2_format *f)
+{
+	struct rkcif_stream *stream = video_drvdata(file);
+
+	rkcif_set_fmt(stream, &f->fmt.pix_mp, true);
+
+	return 0;
+}
+
+static int rkcif_enum_fmt_vid_cap(struct file *file, void *priv,
+					 struct v4l2_fmtdesc *f)
+{
+	const struct cif_output_fmt *fmt = NULL;
+
+	if (f->index >= ARRAY_SIZE(out_fmts))
+		return -EINVAL;
+
+	fmt = &out_fmts[f->index];
+	f->pixelformat = fmt->fourcc;
+
+	return 0;
+}
+
+static int rkcif_s_fmt_vid_cap_mplane(struct file *file,
+				      void *priv, struct v4l2_format *f)
+{
+	struct rkcif_stream *stream = video_drvdata(file);
+
+	rkcif_set_fmt(stream, &f->fmt.pix_mp, false);
+
+	return 0;
+}
+
+static int rkcif_g_fmt_vid_cap_mplane(struct file *file, void *fh,
+				      struct v4l2_format *f)
+{
+	struct rkcif_stream *stream = video_drvdata(file);
+
+	f->fmt.pix_mp = stream->pixm;
+
+	return 0;
+}
+
+static int rkcif_querycap(struct file *file, void *priv,
+			  struct v4l2_capability *cap)
+{
+	struct rkcif_stream *stream = video_drvdata(file);
+	struct device *dev = stream->cifdev->dev;
+
+	strlcpy(cap->driver, dev->driver->name, sizeof(cap->driver));
+	strlcpy(cap->card, dev->driver->name, sizeof(cap->card));
+	snprintf(cap->bus_info, sizeof(cap->bus_info),
+		 "platform:%s", dev_name(dev));
+
+	return 0;
+}
+
+static int rkcif_enum_framesizes(struct file *file, void *fh,
+				 struct v4l2_frmsizeenum *fsize)
+{
+	struct rkcif_stream *stream = video_drvdata(file);
+	struct rkcif_device *dev = stream->cifdev;
+	struct v4l2_subdev_frame_size_enum fse = {
+		.index = fsize->index,
+		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
+	};
+	const struct cif_output_fmt *fmt;
+	int ret;
+
+	if (!dev->sensor.sd)
+		return -EINVAL;
+
+	fmt = find_output_fmt(stream, fsize->pixel_format);
+	if (!fmt)
+		return -EINVAL;
+
+	fse.code = fmt->mbus;
+
+	ret = v4l2_subdev_call(dev->sensor.sd, pad, enum_frame_size,
+			       NULL, &fse);
+	if (ret)
+		return ret;
+
+	fsize->type = V4L2_FRMSIZE_TYPE_DISCRETE;
+	fsize->discrete.width = fse.max_width;
+	fsize->discrete.height = fse.max_height;
+
+	return 0;
+}
+
+static int rkcif_g_input(struct file *file, void *fh, unsigned int *i)
+{
+	*i = 0;
+	return 0;
+}
+
+static int rkcif_s_input(struct file *file, void *fh, unsigned int i)
+{
+	if (i)
+		return -EINVAL;
+
+	return 0;
+}
+
+static const struct v4l2_ioctl_ops rkcif_v4l2_ioctl_ops = {
+	.vidioc_reqbufs = vb2_ioctl_reqbufs,
+	.vidioc_querybuf = vb2_ioctl_querybuf,
+	.vidioc_create_bufs = vb2_ioctl_create_bufs,
+	.vidioc_qbuf = vb2_ioctl_qbuf,
+	.vidioc_expbuf = vb2_ioctl_expbuf,
+	.vidioc_dqbuf = vb2_ioctl_dqbuf,
+	.vidioc_prepare_buf = vb2_ioctl_prepare_buf,
+	.vidioc_streamon = vb2_ioctl_streamon,
+	.vidioc_streamoff = vb2_ioctl_streamoff,
+
+	.vidioc_enum_fmt_vid_cap = rkcif_enum_fmt_vid_cap,
+	.vidioc_try_fmt_vid_cap_mplane = rkcif_try_fmt_vid_cap_mplane,
+	.vidioc_s_fmt_vid_cap_mplane = rkcif_s_fmt_vid_cap_mplane,
+	.vidioc_g_fmt_vid_cap_mplane = rkcif_g_fmt_vid_cap_mplane,
+	.vidioc_querycap = rkcif_querycap,
+	.vidioc_enum_framesizes = rkcif_enum_framesizes,
+
+	.vidioc_enum_input = rkcif_enum_input,
+	.vidioc_g_input = rkcif_g_input,
+	.vidioc_s_input = rkcif_s_input,
+};
+
+void rkcif_unregister_stream_vdev(struct rkcif_device *dev)
+{
+	struct rkcif_stream *stream = &dev->stream;
+
+	media_entity_cleanup(&stream->vdev.entity);
+	video_unregister_device(&stream->vdev);
+}
+
+int rkcif_register_stream_vdev(struct rkcif_device *dev)
+{
+	struct rkcif_stream *stream = &dev->stream;
+	struct v4l2_device *v4l2_dev = &dev->v4l2_dev;
+	struct video_device *vdev = &stream->vdev;
+	int ret;
+
+	strlcpy(vdev->name, CIF_VIDEODEVICE_NAME, sizeof(vdev->name));
+	mutex_init(&stream->vlock);
+
+	vdev->ioctl_ops = &rkcif_v4l2_ioctl_ops;
+	vdev->release = video_device_release_empty;
+	vdev->fops = &rkcif_fops;
+	vdev->minor = -1;
+	vdev->v4l2_dev = v4l2_dev;
+	vdev->lock = &stream->vlock;
+	vdev->device_caps = V4L2_CAP_VIDEO_CAPTURE_MPLANE |
+			    V4L2_CAP_STREAMING;
+	video_set_drvdata(vdev, stream);
+	vdev->vfl_dir = VFL_DIR_RX;
+	stream->pad.flags = MEDIA_PAD_FL_SINK;
+
+	rkcif_init_vb2_queue(&stream->buf_queue, stream,
+			V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE |
+			V4L2_BUF_TYPE_VIDEO_CAPTURE);
+	vdev->queue = &stream->buf_queue;
+	strscpy(vdev->name, KBUILD_MODNAME, sizeof(vdev->name));
+
+	ret = video_register_device(vdev, VFL_TYPE_VIDEO, -1);
+	if (ret < 0) {
+		v4l2_err(v4l2_dev,
+			 "video_register_device failed with error %d\n", ret);
+		return ret;
+	}
+
+	ret = media_entity_pads_init(&vdev->entity, 1, &stream->pad);
+	if (ret < 0)
+		goto unreg;
+
+	return 0;
+unreg:
+	video_unregister_device(vdev);
+	return ret;
+}
+
+static void rkcif_vb_done_oneframe(struct rkcif_stream *stream,
+				   struct vb2_v4l2_buffer *vb_done)
+{
+	const struct cif_output_fmt *fmt = stream->cif_fmt_out;
+	u32 i;
+
+	/* Dequeue a filled buffer */
+	for (i = 0; i < fmt->mplanes; i++) {
+		vb2_set_plane_payload(&vb_done->vb2_buf, i,
+			stream->pixm.plane_fmt[i].sizeimage);
+	}
+	vb_done->vb2_buf.timestamp = ktime_get_ns();
+	vb2_buffer_done(&vb_done->vb2_buf, VB2_BUF_STATE_DONE);
+}
+
+void rkcif_irq_oneframe(struct rkcif_device *cif_dev)
+{
+	struct rkcif_stream *stream = &cif_dev->stream;
+	u32 lastline, lastpix, ctl, cif_frmst, intstat;
+	void __iomem *base = cif_dev->base_addr;
+
+	intstat = read_cif_reg(base, CIF_INTSTAT);
+	cif_frmst = read_cif_reg(base, CIF_FRAME_STATUS);
+	lastline = CIF_FETCH_Y_LAST_LINE(read_cif_reg(base, CIF_LAST_LINE));
+	lastpix = read_cif_reg(base, CIF_LAST_PIX);
+	ctl = read_cif_reg(base, CIF_CTRL);
+
+	/* There are two irqs enabled:
+	 *  - PST_INF_FRAME_END: cif FIFO is ready, this is prior to FRAME_END
+	 *  -         FRAME_END: cif has saved frame to memory, a frame ready
+	 */
+
+	if ((intstat & PST_INF_FRAME_END)) {
+		write_cif_reg(base, CIF_INTSTAT, PST_INF_FRAME_END_CLR);
+
+		if (stream->stopping)
+			/* To stop CIF ASAP, before FRAME_END irq */
+			write_cif_reg(base, CIF_CTRL, ctl & (~ENABLE_CAPTURE));
+	}
+
+	if ((intstat & LINE_ERR)) {
+		write_cif_reg(base, CIF_INTSTAT, LINE_ERR_CLR);
+
+		if (stream->stopping) {
+			rkcif_stream_stop(stream);
+			stream->stopping = false;
+			wake_up(&stream->wq_stopped);
+			return;
+		}
+
+		v4l2_err(&cif_dev->v4l2_dev,
+			 "Bad frame, irq:0x%x frmst:0x%x size:%dx%d\n",
+			 intstat, cif_frmst, lastline, lastpix);
+		/* Clear status to receive into the same buffer */
+		write_cif_reg(base, CIF_FRAME_STATUS, FRM0_STAT_CLS);
+		return;
+	}
+
+
+	if ((intstat & FRAME_END)) {
+		struct vb2_v4l2_buffer *vb_done = NULL;
+
+		write_cif_reg(base, CIF_INTSTAT, FRAME_END_CLR);
+
+		if (stream->stopping) {
+			rkcif_stream_stop(stream);
+			stream->stopping = false;
+			wake_up(&stream->wq_stopped);
+			return;
+		}
+
+		if (lastline != stream->pixm.height ||
+		    !(cif_frmst & CIF_F0_READY)) {
+			v4l2_err(&cif_dev->v4l2_dev,
+				 "Bad frame, irq:0x%x frmst:0x%x size:%dx%d\n",
+				 intstat, cif_frmst, lastline, lastpix);
+			/* Clear status to receive into the same buffer */
+			write_cif_reg(base, CIF_FRAME_STATUS, FRM0_STAT_CLS);
+			return;
+		}
+
+		if (stream->curr_buf)
+			vb_done = &stream->curr_buf->vb;
+		rkcif_assign_new_buffer_oneframe(stream);
+
+		/* In one-frame mode, must clear status manually to enable
+		 * the next frame end irq
+		 */
+		write_cif_reg(base, CIF_FRAME_STATUS, FRM0_STAT_CLS);
+
+		if (vb_done)
+			rkcif_vb_done_oneframe(stream, vb_done);
+
+		stream->frame_idx++;
+	}
+}
diff --git a/drivers/media/platform/rockchip/cif/dev.c b/drivers/media/platform/rockchip/cif/dev.c
new file mode 100644
index 000000000000..dbd5fdbd1cef
--- /dev/null
+++ b/drivers/media/platform/rockchip/cif/dev.c
@@ -0,0 +1,358 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Rockchip CIF Driver
+ *
+ * Copyright (C) 2018 Rockchip Electronics Co., Ltd.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_graph.h>
+#include <linux/of_platform.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/reset.h>
+#include <linux/pm_runtime.h>
+#include <linux/pinctrl/consumer.h>
+#include <media/v4l2-fwnode.h>
+
+#include "dev.h"
+#include "regs.h"
+
+#define RKCIF_VERNO_LEN		10
+
+static int rkcif_create_links(struct rkcif_device *dev)
+{
+	struct v4l2_subdev *sd = dev->sensor.sd;
+	int ret;
+
+	ret = media_entity_get_fwnode_pad(&sd->entity, sd->fwnode,
+					  MEDIA_PAD_FL_SOURCE);
+	if (ret)
+		return ret;
+
+	ret = media_create_pad_link(&sd->entity, 0,
+				    &dev->stream.vdev.entity, 0,
+				    MEDIA_LNK_FL_ENABLED);
+	if (ret) {
+		dev_err(dev->dev, "failed to create link");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int subdev_notifier_complete(struct v4l2_async_notifier *notifier)
+{
+	struct rkcif_device *dev;
+	int ret;
+
+	dev = container_of(notifier, struct rkcif_device, notifier);
+
+	mutex_lock(&dev->media_dev.graph_mutex);
+
+	ret = rkcif_create_links(dev);
+	if (ret < 0)
+		goto unlock;
+
+	ret = v4l2_device_register_subdev_nodes(&dev->v4l2_dev);
+	if (ret < 0)
+		goto unlock;
+
+unlock:
+	mutex_unlock(&dev->media_dev.graph_mutex);
+	return ret;
+}
+
+static int subdev_notifier_bound(struct v4l2_async_notifier *notifier,
+				 struct v4l2_subdev *subdev,
+				 struct v4l2_async_subdev *asd)
+{
+	struct rkcif_device *cif_dev = container_of(notifier,
+					struct rkcif_device, notifier);
+
+	int pad;
+
+	cif_dev->sensor.sd = subdev;
+	pad = media_entity_get_fwnode_pad(&subdev->entity, subdev->fwnode,
+					  MEDIA_PAD_FL_SOURCE);
+	if (pad < 0)
+		return pad;
+
+	cif_dev->sensor.pad = pad;
+
+	return 0;
+}
+
+static const struct v4l2_async_notifier_operations subdev_notifier_ops = {
+	.bound = subdev_notifier_bound,
+	.complete = subdev_notifier_complete,
+};
+
+static int cif_subdev_notifier(struct rkcif_device *cif_dev)
+{
+	struct v4l2_async_notifier *ntf = &cif_dev->notifier;
+	struct device *dev = cif_dev->dev;
+	struct v4l2_fwnode_endpoint vep = {
+		.bus_type = V4L2_MBUS_PARALLEL,
+	};
+	struct fwnode_handle *ep;
+	int ret;
+
+	v4l2_async_notifier_init(ntf);
+
+	ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), 0, 0,
+			FWNODE_GRAPH_ENDPOINT_NEXT);
+	if (!ep)
+		return -EINVAL;
+
+	ret = v4l2_fwnode_endpoint_parse(ep, &vep);
+	if (ret)
+		return ret;
+
+	ret = v4l2_async_notifier_add_fwnode_remote_subdev(ntf, ep,
+							   &cif_dev->asd);
+	if (ret)
+		return ret;
+
+	ntf->ops = &subdev_notifier_ops;
+
+	fwnode_handle_put(ep);
+
+	ret = v4l2_async_notifier_register(&cif_dev->v4l2_dev, ntf);
+	return ret;
+}
+
+static int rkcif_register_platform_subdevs(struct rkcif_device *cif_dev)
+{
+	int ret;
+
+	ret = rkcif_register_stream_vdev(cif_dev);
+	if (ret < 0)
+		return ret;
+
+	ret = cif_subdev_notifier(cif_dev);
+	if (ret < 0) {
+		v4l2_err(&cif_dev->v4l2_dev,
+			 "Failed to register subdev notifier(%d)\n", ret);
+		rkcif_unregister_stream_vdev(cif_dev);
+	}
+
+	return 0;
+}
+
+static struct clk_bulk_data px30_cif_clks[] = {
+	{ .id = "aclk" },
+	{ .id = "hclk" },
+	{ .id = "pclkin" },
+};
+
+static const struct cif_match_data px30_cif_match_data = {
+	.chip_id = CHIP_PX30_CIF,
+	.clks = px30_cif_clks,
+	.clks_num = ARRAY_SIZE(px30_cif_clks),
+};
+
+static const struct of_device_id rkcif_plat_of_match[] = {
+	{
+		.compatible = "rockchip,px30-cif",
+		.data = &px30_cif_match_data,
+	},
+	{},
+};
+
+static irqreturn_t rkcif_irq_handler(int irq, void *ctx)
+{
+	struct device *dev = ctx;
+	struct rkcif_device *cif_dev = dev_get_drvdata(dev);
+
+	rkcif_irq_oneframe(cif_dev);
+
+	return IRQ_HANDLED;
+}
+
+static void rkcif_disable_sys_clk(struct rkcif_device *cif_dev)
+{
+	int i;
+
+	for (i = cif_dev->data->clks_num - 1; i >= 0; i--)
+		clk_disable_unprepare(cif_dev->clks[i]);
+}
+
+static int rkcif_enable_sys_clk(struct rkcif_device *cif_dev)
+{
+	int i, ret = -EINVAL;
+
+	for (i = 0; i < cif_dev->data->clks_num; i++) {
+		ret = clk_prepare_enable(cif_dev->clks[i]);
+
+		if (ret < 0)
+			goto err;
+	}
+
+	return 0;
+
+err:
+	for (--i; i >= 0; --i)
+		clk_disable_unprepare(cif_dev->clks[i]);
+
+	return ret;
+}
+
+void rkcif_soft_reset(struct rkcif_device *cif_dev)
+{
+	reset_control_assert(cif_dev->cif_rst);
+
+	udelay(5);
+
+	reset_control_deassert(cif_dev->cif_rst);
+}
+
+static int rkcif_plat_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct v4l2_device *v4l2_dev;
+	struct rkcif_device *cif_dev;
+	const struct cif_match_data *data;
+	int ret, irq;
+
+	cif_dev = devm_kzalloc(dev, sizeof(*cif_dev), GFP_KERNEL);
+	if (!cif_dev)
+		return -ENOMEM;
+
+	dev_set_drvdata(dev, cif_dev);
+	cif_dev->dev = dev;
+
+	data = of_device_get_match_data(&pdev->dev);
+	if (!data)
+		return -EINVAL;
+
+	cif_dev->data = data;
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
+		return irq;
+
+	ret = devm_request_irq(dev, irq, rkcif_irq_handler, IRQF_SHARED,
+			       dev_driver_string(dev), dev);
+	if (ret < 0) {
+		dev_err(dev, "request irq failed: %d\n", ret);
+		return ret;
+	}
+
+	cif_dev->irq = irq;
+
+	cif_dev->base_addr = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(cif_dev->base_addr))
+		return PTR_ERR(cif_dev->base_addr);
+
+	ret = of_reserved_mem_device_init(dev);
+	if (ret)
+		v4l2_info(v4l2_dev, "No reserved memory region assign to CIF\n");
+
+	ret = devm_clk_bulk_get(dev, data->clks_num, data->clks);
+	if (ret)
+		return ret;
+
+	cif_dev->cif_rst = devm_reset_control_array_get(dev, false, false);
+	if (IS_ERR(cif_dev->cif_rst))
+		return PTR_ERR(cif_dev->cif_rst);
+
+	/* Initialize the stream */
+	rkcif_stream_init(cif_dev);
+
+	strlcpy(cif_dev->media_dev.model, "rkcif",
+		sizeof(cif_dev->media_dev.model));
+	cif_dev->media_dev.dev = &pdev->dev;
+	v4l2_dev = &cif_dev->v4l2_dev;
+	v4l2_dev->mdev = &cif_dev->media_dev;
+	strlcpy(v4l2_dev->name, "rkcif", sizeof(v4l2_dev->name));
+	v4l2_ctrl_handler_init(&cif_dev->ctrl_handler, 8);
+	v4l2_dev->ctrl_handler = &cif_dev->ctrl_handler;
+
+	ret = v4l2_device_register(cif_dev->dev, &cif_dev->v4l2_dev);
+	if (ret < 0)
+		return ret;
+
+	media_device_init(&cif_dev->media_dev);
+
+	ret = media_device_register(&cif_dev->media_dev);
+	if (ret < 0) {
+		v4l2_err(v4l2_dev, "Failed to register media device: %d\n",
+			 ret);
+		goto err_unreg_v4l2_dev;
+	}
+
+	/* create & register platefom subdev (from of_node) */
+	ret = rkcif_register_platform_subdevs(cif_dev);
+	if (ret < 0)
+		goto err_unreg_media_dev;
+
+	pm_runtime_enable(&pdev->dev);
+
+	return 0;
+
+err_unreg_media_dev:
+	media_device_unregister(&cif_dev->media_dev);
+err_unreg_v4l2_dev:
+	v4l2_device_unregister(&cif_dev->v4l2_dev);
+	return ret;
+}
+
+static int rkcif_plat_remove(struct platform_device *pdev)
+{
+	struct rkcif_device *cif_dev = platform_get_drvdata(pdev);
+
+	pm_runtime_disable(&pdev->dev);
+
+	media_device_unregister(&cif_dev->media_dev);
+	v4l2_device_unregister(&cif_dev->v4l2_dev);
+	rkcif_unregister_stream_vdev(cif_dev);
+
+	return 0;
+}
+
+static int __maybe_unused rkcif_runtime_suspend(struct device *dev)
+{
+	struct rkcif_device *cif_dev = dev_get_drvdata(dev);
+
+	rkcif_disable_sys_clk(cif_dev);
+
+	return pinctrl_pm_select_sleep_state(dev);
+}
+
+static int __maybe_unused rkcif_runtime_resume(struct device *dev)
+{
+	struct rkcif_device *cif_dev = dev_get_drvdata(dev);
+	int ret;
+
+	ret = pinctrl_pm_select_default_state(dev);
+	if (ret < 0)
+		return ret;
+	rkcif_enable_sys_clk(cif_dev);
+
+	return 0;
+}
+
+static const struct dev_pm_ops rkcif_plat_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+				pm_runtime_force_resume)
+	SET_RUNTIME_PM_OPS(rkcif_runtime_suspend, rkcif_runtime_resume, NULL)
+};
+
+static struct platform_driver rkcif_plat_drv = {
+	.driver = {
+		   .name = CIF_DRIVER_NAME,
+		   .of_match_table = of_match_ptr(rkcif_plat_of_match),
+		   .pm = &rkcif_plat_pm_ops,
+	},
+	.probe = rkcif_plat_probe,
+	.remove = rkcif_plat_remove,
+};
+
+module_platform_driver(rkcif_plat_drv);
+MODULE_AUTHOR("Rockchip Camera/ISP team");
+MODULE_DESCRIPTION("Rockchip CIF platform driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/media/platform/rockchip/cif/dev.h b/drivers/media/platform/rockchip/cif/dev.h
new file mode 100644
index 000000000000..2126c2220ad2
--- /dev/null
+++ b/drivers/media/platform/rockchip/cif/dev.h
@@ -0,0 +1,213 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Rockchip CIF Driver
+ *
+ * Copyright (C) 2018 Rockchip Electronics Co., Ltd.
+ */
+
+#ifndef _RKCIF_DEV_H
+#define _RKCIF_DEV_H
+
+#include <linux/mutex.h>
+#include <media/media-device.h>
+#include <media/media-entity.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-device.h>
+#include <media/videobuf2-v4l2.h>
+
+#define CIF_DRIVER_NAME		"rkcif"
+#define CIF_VIDEODEVICE_NAME	"stream_cif"
+
+#define RKCIF_MAX_BUS_CLK	8
+#define RKCIF_MAX_SENSOR	2
+#define RKCIF_MAX_RESET		5
+#define RKCIF_MAX_CSI_CHANNEL	4
+
+#define RKCIF_DEFAULT_WIDTH	640
+#define RKCIF_DEFAULT_HEIGHT	480
+
+#define write_cif_reg(base, addr, val)  writel(val, (addr) + (base))
+#define read_cif_reg(base, addr) readl((addr) + (base))
+
+#define write_csihost_reg(base, addr, val)  writel(val, (addr) + (base))
+#define read_csihost_reg(base, addr) readl((addr) + (base))
+
+enum rkcif_state {
+	RKCIF_STATE_DISABLED,
+	RKCIF_STATE_READY,
+	RKCIF_STATE_STREAMING
+};
+
+enum rkcif_chip_id {
+	CHIP_PX30_CIF,
+	CHIP_RK1808_CIF,
+	CHIP_RK3128_CIF,
+	CHIP_RK3288_CIF
+};
+
+enum host_type_t {
+	RK_CSI_RXHOST,
+	RK_DSI_RXHOST
+};
+
+struct rkcif_buffer {
+	struct vb2_v4l2_buffer vb;
+	struct list_head queue;
+	union {
+		u32 buff_addr[VIDEO_MAX_PLANES];
+		void *vaddr[VIDEO_MAX_PLANES];
+	};
+};
+
+struct rkcif_dummy_buffer {
+	void *vaddr;
+	dma_addr_t dma_addr;
+	u32 size;
+};
+
+extern int rkcif_debug;
+
+static inline struct rkcif_buffer *to_rkcif_buffer(struct vb2_v4l2_buffer *vb)
+{
+	return container_of(vb, struct rkcif_buffer, vb);
+}
+
+/*
+ * struct rkcif_sensor_info - Sensor infomations
+ * @mbus: media bus configuration
+ */
+struct rkcif_sensor_info {
+	struct v4l2_subdev *sd;
+	int pad;
+	struct v4l2_mbus_config mbus;
+	int lanes;
+};
+
+/*
+ * struct cif_output_fmt - The output format
+ *
+ * @fourcc: pixel format in fourcc
+ * @cplanes: number of colour planes
+ * @fmt_val: the fmt val corresponding to CIF_FOR register
+ * @bpp: bits per pixel for each cplanes
+ */
+struct cif_output_fmt {
+	u32 fourcc;
+	u32 mbus;
+	u8 cplanes;
+	u8 mplanes;
+	u32 fmt_val;
+	u8 bpp[VIDEO_MAX_PLANES];
+};
+
+enum cif_fmt_type {
+	CIF_FMT_TYPE_YUV = 0,
+	CIF_FMT_TYPE_RAW,
+};
+
+/*
+ * struct cif_input_fmt - The input mbus format from sensor
+ *
+ * @mbus_code: mbus format
+ * @dvp_fmt_val: the fmt val corresponding to CIF_FOR register
+ * @csi_fmt_val: the fmt val corresponding to CIF_CSI_ID_CTRL
+ * @field: the field type of the input from sensor
+ */
+struct cif_input_fmt {
+	u32 mbus_code;
+	u32 dvp_fmt_val;
+	u32 csi_fmt_val;
+	enum cif_fmt_type fmt_type;
+	enum v4l2_field field;
+};
+
+/*
+ * struct rkcif_stream - Stream states TODO
+ *
+ * @vbq_lock: lock to protect buf_queue
+ * @buf_queue: queued buffer list
+ * @dummy_buf: dummy space to store dropped data
+ *
+ * rkcif use shadowsock registers, so it need two buffer at a time
+ * @curr_buf: the buffer used for current frame
+ * @next_buf: the buffer used for next frame
+ */
+struct rkcif_stream {
+	struct rkcif_device		*cifdev;
+	enum rkcif_state		state;
+	bool				stopping;
+	wait_queue_head_t		wq_stopped;
+	int				frame_idx;
+	int				frame_phase;
+
+	/* lock between irq and buf_queue */
+	spinlock_t			vbq_lock;
+	struct vb2_queue		buf_queue;
+	struct list_head		buf_head;
+	struct rkcif_dummy_buffer	dummy_buf;
+	struct rkcif_buffer		*curr_buf;
+	struct rkcif_buffer		*next_buf;
+
+	/* vfd lock */
+	struct mutex			vlock;
+	struct video_device		vdev;
+	/* TODO: pad for dvp and mipi separately? */
+	struct media_pad		pad;
+
+	const struct cif_output_fmt	*cif_fmt_out;
+	const struct cif_input_fmt	*cif_fmt_in;
+	struct v4l2_pix_format_mplane	pixm;
+	struct v4l2_rect		crop;
+	int				crop_enable;
+};
+
+static inline struct rkcif_stream *to_rkcif_stream(struct video_device *vdev)
+{
+	return container_of(vdev, struct rkcif_stream, vdev);
+}
+
+struct cif_match_data {
+	int chip_id;
+	struct clk_bulk_data *clks;
+	int clks_num;
+};
+
+/*
+ * struct rkcif_device - ISP platform device
+ * @base_addr: base register address
+ * @active_sensor: sensor in-use, set when streaming on
+ * @stream: capture video device
+ */
+struct rkcif_device {
+	struct list_head		list;
+	struct device			*dev;
+	int				irq;
+	void __iomem			*base_addr;
+	void __iomem			*csi_base;
+	struct clk			*clks[RKCIF_MAX_BUS_CLK];
+	int				clk_size;
+	struct vb2_alloc_ctx		*alloc_ctx;
+	bool				iommu_en;
+	struct iommu_domain		*domain;
+	struct reset_control		*cif_rst;
+
+	struct v4l2_device		v4l2_dev;
+	struct media_device		media_dev;
+	struct v4l2_ctrl_handler	ctrl_handler;
+	struct v4l2_async_notifier	notifier;
+	struct v4l2_async_subdev	asd;
+	struct rkcif_sensor_info	sensor;
+
+	struct rkcif_stream		stream;
+	const struct cif_match_data	*data;
+};
+
+void rkcif_unregister_stream_vdev(struct rkcif_device *dev);
+int rkcif_register_stream_vdev(struct rkcif_device *dev);
+void rkcif_stream_init(struct rkcif_device *dev);
+
+void rkcif_irq_oneframe(struct rkcif_device *cif_dev);
+void rkcif_irq_pingpong(struct rkcif_device *cif_dev);
+void rkcif_soft_reset(struct rkcif_device *cif_dev);
+
+#endif
diff --git a/drivers/media/platform/rockchip/cif/regs.h b/drivers/media/platform/rockchip/cif/regs.h
new file mode 100644
index 000000000000..5e0f926c70d3
--- /dev/null
+++ b/drivers/media/platform/rockchip/cif/regs.h
@@ -0,0 +1,256 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Rockchip CIF Driver
+ *
+ * Copyright (C) 2018 Rockchip Electronics Co., Ltd.
+ */
+
+#ifndef _RKCIF_REGS_H
+#define _RKCIF_REGS_H
+
+/* CIF Reg Offset */
+#define CIF_CTRL			0x00
+#define CIF_INTEN			0x04
+#define CIF_INTSTAT			0x08
+#define CIF_FOR				0x0c
+#define CIF_LINE_NUM_ADDR		0x10
+#define CIF_FRM0_ADDR_Y			0x14
+#define CIF_FRM0_ADDR_UV		0x18
+#define CIF_FRM1_ADDR_Y			0x1c
+#define CIF_FRM1_ADDR_UV		0x20
+#define CIF_VIR_LINE_WIDTH		0x24
+#define CIF_SET_SIZE			0x28
+#define CIF_SCM_ADDR_Y			0x2c
+#define CIF_SCM_ADDR_U			0x30
+#define CIF_SCM_ADDR_V			0x34
+#define CIF_WB_UP_FILTER		0x38
+#define CIF_WB_LOW_FILTER		0x3c
+#define CIF_WBC_CNT			0x40
+#define CIF_CROP			0x44
+#define CIF_SCL_CTRL			0x48
+#define CIF_SCL_DST			0x4c
+#define CIF_SCL_FCT			0x50
+#define CIF_SCL_VALID_NUM		0x54
+#define CIF_LINE_LOOP_CTR		0x58
+#define CIF_FRAME_STATUS		0x60
+#define CIF_CUR_DST			0x64
+#define CIF_LAST_LINE			0x68
+#define CIF_LAST_PIX			0x6c
+
+/* RK1808 CIF CSI Registers Offset */
+#define CIF_CSI_ID0_CTRL0		0x80
+#define CIF_CSI_ID0_CTRL1		0x84
+#define CIF_CSI_ID1_CTRL0		0x88
+#define CIF_CSI_ID1_CTRL1		0x8c
+#define CIF_CSI_ID2_CTRL0		0x90
+#define CIF_CSI_ID2_CTRL1		0x94
+#define CIF_CSI_ID3_CTRL0		0x98
+#define CIF_CSI_ID3_CTRL1		0x9c
+#define CIF_CSI_WATER_LINE		0xa0
+#define CIF_CSI_FRM0_ADDR_Y_ID0		0xa4
+#define CIF_CSI_FRM1_ADDR_Y_ID0		0xa8
+#define CIF_CSI_FRM0_ADDR_UV_ID0	0xac
+#define CIF_CSI_FRM1_ADDR_UV_ID0	0xb0
+#define CIF_CSI_FRM0_VLW_Y_ID0		0xb4
+#define CIF_CSI_FRM1_VLW_Y_ID0		0xb8
+#define CIF_CSI_FRM0_VLW_UV_ID0		0xbc
+#define CIF_CSI_FRM1_VLW_UV_ID0		0xc0
+#define CIF_CSI_FRM0_ADDR_Y_ID1		0xc4
+#define CIF_CSI_FRM1_ADDR_Y_ID1		0xc8
+#define CIF_CSI_FRM0_ADDR_UV_ID1	0xcc
+#define CIF_CSI_FRM1_ADDR_UV_ID1	0xd0
+#define CIF_CSI_FRM0_VLW_Y_ID1		0xd4
+#define CIF_CSI_FRM1_VLW_Y_ID1		0xd8
+#define CIF_CSI_FRM0_VLW_UV_ID1		0xdc
+#define CIF_CSI_FRM1_VLW_UV_ID1		0xe0
+#define CIF_CSI_FRM0_ADDR_Y_ID2		0xe4
+#define CIF_CSI_FRM1_ADDR_Y_ID2		0xe8
+#define CIF_CSI_FRM0_ADDR_UV_ID2	0xec
+#define CIF_CSI_FRM1_ADDR_UV_ID2	0xf0
+#define CIF_CSI_FRM0_VLW_Y_ID2		0xf4
+#define CIF_CSI_FRM1_VLW_Y_ID2		0xf8
+#define CIF_CSI_FRM0_VLW_UV_ID2		0xfc
+#define CIF_CSI_FRM1_VLW_UV_ID2		0x100
+#define CIF_CSI_FRM0_ADDR_Y_ID3		0x104
+#define CIF_CSI_FRM1_ADDR_Y_ID3		0x108
+#define CIF_CSI_FRM0_ADDR_UV_ID3	0x10c
+#define CIF_CSI_FRM1_ADDR_UV_ID3	0x110
+#define CIF_CSI_FRM0_VLW_Y_ID3		0x114
+#define CIF_CSI_FRM1_VLW_Y_ID3		0x118
+#define CIF_CSI_FRM0_VLW_UV_ID3		0x11c
+#define CIF_CSI_FRM1_VLW_UV_ID3		0x120
+#define CIF_CSI_INTEN			0x124
+#define CIF_CSI_INTSTAT			0x128
+#define CIF_CSI_LINE_INT_NUM_ID0_1	0x12c
+#define CIF_CSI_LINE_INT_NUM_ID2_3	0x130
+#define CIF_CSI_LINE_CNT_ID0_1		0x134
+#define CIF_CSI_LINE_CNT_ID2_3		0x138
+#define CIF_CSI_ID0_CROP_START		0x13c
+#define CIF_CSI_ID1_CROP_START		0x140
+#define CIF_CSI_ID2_CROP_START		0x144
+#define CIF_CSI_ID3_CROP_START		0x148
+
+/* The key register bit description */
+
+/* CIF_CTRL Reg */
+#define DISABLE_CAPTURE			(0x0 << 0)
+#define ENABLE_CAPTURE			(0x1 << 0)
+#define MODE_ONEFRAME			(0x0 << 1)
+#define MODE_PINGPONG			(0x1 << 1)
+#define MODE_LINELOOP			(0x2 << 1)
+#define AXI_BURST_16			(0xF << 12)
+
+/* CIF_INTEN */
+#define INTEN_DISABLE			(0x0 << 0)
+#define FRAME_END_EN			(0x1 << 0)
+#define LINE_ERR_EN			(0x1 << 2)
+#define BUS_ERR_EN			(0x1 << 6)
+#define SCL_ERR_EN			(0x1 << 7)
+#define PST_INF_FRAME_END_EN		(0x1 << 9)
+
+/* CIF INTSTAT */
+#define INTSTAT_CLS			(0x3FF)
+#define FRAME_END			(0x01 << 0)
+#define LINE_ERR			(0x01 << 2)
+#define PST_INF_FRAME_END		(0x01 << 9)
+#define FRAME_END_CLR			(0x01 << 0)
+#define LINE_ERR_CLR			(0x01 << 2)
+#define PST_INF_FRAME_END_CLR		(0x01 << 9)
+#define INTSTAT_ERR			(0xFC)
+
+/* FRAME STATUS */
+#define FRAME_STAT_CLS			0x00
+#define FRM0_STAT_CLS			0x20	/* write 0 to clear frame 0 */
+
+/* CIF FORMAT */
+#define VSY_HIGH_ACTIVE			(0x01 << 0)
+#define VSY_LOW_ACTIVE			(0x00 << 0)
+#define HSY_LOW_ACTIVE			(0x01 << 1)
+#define HSY_HIGH_ACTIVE			(0x00 << 1)
+#define INPUT_MODE_YUV			(0x00 << 2)
+#define INPUT_MODE_PAL			(0x02 << 2)
+#define INPUT_MODE_NTSC			(0x03 << 2)
+#define INPUT_MODE_BT1120		(0x07 << 2)
+#define INPUT_MODE_RAW			(0x04 << 2)
+#define INPUT_MODE_JPEG			(0x05 << 2)
+#define INPUT_MODE_MIPI			(0x06 << 2)
+#define YUV_INPUT_ORDER_UYVY		(0x00 << 5)
+#define YUV_INPUT_ORDER_YVYU		(0x01 << 5)
+#define YUV_INPUT_ORDER_VYUY		(0x10 << 5)
+#define YUV_INPUT_ORDER_YUYV		(0x03 << 5)
+#define YUV_INPUT_422			(0x00 << 7)
+#define YUV_INPUT_420			(0x01 << 7)
+#define INPUT_420_ORDER_EVEN		(0x00 << 8)
+#define INPUT_420_ORDER_ODD		(0x01 << 8)
+#define CCIR_INPUT_ORDER_ODD		(0x00 << 9)
+#define CCIR_INPUT_ORDER_EVEN		(0x01 << 9)
+#define RAW_DATA_WIDTH_8		(0x00 << 11)
+#define RAW_DATA_WIDTH_10		(0x01 << 11)
+#define RAW_DATA_WIDTH_12		(0x02 << 11)
+#define YUV_OUTPUT_422			(0x00 << 16)
+#define YUV_OUTPUT_420			(0x01 << 16)
+#define OUTPUT_420_ORDER_EVEN		(0x00 << 17)
+#define OUTPUT_420_ORDER_ODD		(0x01 << 17)
+#define RAWD_DATA_LITTLE_ENDIAN		(0x00 << 18)
+#define RAWD_DATA_BIG_ENDIAN		(0x01 << 18)
+#define UV_STORAGE_ORDER_UVUV		(0x00 << 19)
+#define UV_STORAGE_ORDER_VUVU		(0x01 << 19)
+#define BT1120_CLOCK_SINGLE_EDGES	(0x00 << 24)
+#define BT1120_CLOCK_DOUBLE_EDGES	(0x01 << 24)
+#define BT1120_TRANSMIT_INTERFACE	(0x00 << 25)
+#define BT1120_TRANSMIT_PROGRESS	(0x01 << 25)
+#define BT1120_YC_SWAP			(0x01 << 26)
+
+/* CIF_SCL_CTRL */
+#define ENABLE_SCL_DOWN			(0x01 << 0)
+#define DISABLE_SCL_DOWN		(0x00 << 0)
+#define ENABLE_SCL_UP			(0x01 << 1)
+#define DISABLE_SCL_UP			(0x00 << 1)
+#define ENABLE_YUV_16BIT_BYPASS		(0x01 << 4)
+#define DISABLE_YUV_16BIT_BYPASS	(0x00 << 4)
+#define ENABLE_RAW_16BIT_BYPASS		(0x01 << 5)
+#define DISABLE_RAW_16BIT_BYPASS	(0x00 << 5)
+#define ENABLE_32BIT_BYPASS		(0x01 << 6)
+#define DISABLE_32BIT_BYPASS		(0x00 << 6)
+
+/* CIF_INTSTAT */
+#define CIF_F0_READY			(0x01 << 0)
+#define CIF_F1_READY			(0x01 << 1)
+
+/* CIF CROP */
+#define CIF_CROP_Y_SHIFT		16
+#define CIF_CROP_X_SHIFT		0
+
+/* CIF_CSI_ID_CTRL0 */
+#define CSI_DISABLE_CAPTURE		(0x0 << 0)
+#define CSI_ENABLE_CAPTURE		(0x1 << 0)
+#define CSI_WRDDR_TYPE_RAW8		(0x0 << 1)
+#define CSI_WRDDR_TYPE_RAW10		(0x1 << 1)
+#define CSI_WRDDR_TYPE_RAW12		(0x2 << 1)
+#define CSI_WRDDR_TYPE_RGB888		(0x3 << 1)
+#define CSI_WRDDR_TYPE_YUV422		(0x4 << 1)
+#define CSI_DISABLE_COMMAND_MODE	(0x0 << 4)
+#define CSI_ENABLE_COMMAND_MODE		(0x1 << 4)
+#define CSI_DISABLE_CROP		(0x0 << 5)
+#define CSI_ENABLE_CROP			(0x1 << 5)
+
+/* CIF_CSI_INTEN */
+#define CSI_FRAME0_START_INTEN(id)	(0x1 << ((id) * 2))
+#define CSI_FRAME1_START_INTEN(id)	(0x1 << ((id) * 2 + 1))
+#define CSI_FRAME0_END_INTEN(id)	(0x1 << ((id) * 2 + 8))
+#define CSI_FRAME1_END_INTEN(id)	(0x1 << ((id) * 2 + 9))
+#define CSI_DMA_Y_FIFO_OVERFLOW_INTEN	(0x1 << 16)
+#define CSI_DMA_UV_FIFO_OVERFLOW_INTEN	(0x1 << 17)
+#define CSI_CONFIG_FIFO_OVERFLOW_INTEN	(0x1 << 18)
+#define CSI_BANDWIDTH_LACK_INTEN	(0x1 << 19)
+#define CSI_RX_FIFO_OVERFLOW_INTEN	(0x1 << 20)
+#define CSI_ALL_FRAME_START_INTEN	(0xff << 0)
+#define CSI_ALL_FRAME_END_INTEN		(0xff << 8)
+#define CSI_ALL_ERROR_INTEN		(0x1f << 16)
+
+/* CIF_CSI_INTSTAT */
+#define CSI_FRAME0_START_ID0		(0x1 << 0)
+#define CSI_FRAME1_START_ID0		(0x1 << 1)
+#define CSI_FRAME0_START_ID1		(0x1 << 2)
+#define CSI_FRAME1_START_ID1		(0x1 << 3)
+#define CSI_FRAME0_START_ID2		(0x1 << 4)
+#define CSI_FRAME1_START_ID2		(0x1 << 5)
+#define CSI_FRAME0_START_ID3		(0x1 << 6)
+#define CSI_FRAME1_START_ID3		(0x1 << 7)
+#define CSI_FRAME0_END_ID0		(0x1 << 8)
+#define CSI_FRAME1_END_ID0		(0x1 << 9)
+#define CSI_FRAME0_END_ID1		(0x1 << 10)
+#define CSI_FRAME1_END_ID1		(0x1 << 11)
+#define CSI_FRAME0_END_ID2		(0x1 << 12)
+#define CSI_FRAME1_END_ID2		(0x1 << 13)
+#define CSI_FRAME0_END_ID3		(0x1 << 14)
+#define CSI_FRAME1_END_ID3		(0x1 << 15)
+#define CSI_DMA_Y_FIFO_OVERFLOW		(0x1 << 16)
+#define CSI_DMA_UV_FIFO_OVERFLOW	(0x1 << 17)
+#define CSI_CONFIG_FIFO_OVERFLOW	(0x1 << 18)
+#define CSI_BANDWIDTH_LACK		(0x1 << 19)
+#define CSI_RX_FIFO_OVERFLOW		(0x1 << 20)
+
+#define CSI_FIFO_OVERFLOW	(CSI_DMA_Y_FIFO_OVERFLOW |	\
+				 CSI_DMA_UV_FIFO_OVERFLOW |	\
+				 CSI_CONFIG_FIFO_OVERFLOW |	\
+				 CSI_RX_FIFO_OVERFLOW)
+
+/* CSI Host Registers Define */
+#define CSIHOST_N_LANES		0x04
+#define CSIHOST_PHY_RSTZ	0x0c
+#define CSIHOST_RESETN		0x10
+#define CSIHOST_ERR1		0x20
+#define CSIHOST_ERR2		0x24
+#define CSIHOST_MSK1		0x28
+#define CSIHOST_MSK2		0x2c
+#define CSIHOST_CONTROL		0x40
+
+#define SW_CPHY_EN(x)		((x) << 0)
+#define SW_DSI_EN(x)		((x) << 4)
+#define SW_DATATYPE_FS(x)	((x) << 8)
+#define SW_DATATYPE_FE(x)	((x) << 14)
+#define SW_DATATYPE_LS(x)	((x) << 20)
+#define SW_DATATYPE_LE(x)	((x) << 26)
+
+#endif
-- 
2.25.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v2 1/3] media: dt-bindings: media: Document Rockchip CIF bindings
From: Maxime Chevallier @ 2020-05-29 13:04 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Robin Murphy, Rob Herring, Mark Rutland,
	Heiko Stuebner, Hans Verkuil
  Cc: devicetree, linux-kernel, Maxime Chevallier, Paul Kocialkowski,
	linux-rockchip, Thomas Petazzoni, Miquel Raynal, linux-arm-kernel,
	linux-media
In-Reply-To: <20200529130405.929429-1-maxime.chevallier@bootlin.com>

Add a documentation for the Rockchip Camera Interface controller
binding.

This controller can be found on platforms such as the PX30 or the
RK3288, the PX30 being the only platform supported so far.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---

Changes since V1

 - Updated the clock and reset names
 - Added missing includes in the example, so that the make dt_binding_check passes

 .../bindings/media/rockchip-cif.yaml          | 100 ++++++++++++++++++
 1 file changed, 100 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/rockchip-cif.yaml

diff --git a/Documentation/devicetree/bindings/media/rockchip-cif.yaml b/Documentation/devicetree/bindings/media/rockchip-cif.yaml
new file mode 100644
index 000000000000..f11a30ca9d42
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/rockchip-cif.yaml
@@ -0,0 +1,100 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/rockchip-cif.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Rockchip Camera Interface (CIF)
+
+maintainers:
+  - Maxime Chevallier <maxime.chevallier@bootlin.com>
+
+description: |-
+  Camera Interface for Rockcip platforms
+
+properties:
+  compatible:
+    const: rockchip,px30-cif
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: ACLK
+      - description: HCLK
+      - description: PCLK IN
+
+  clock-names:
+    items:
+      - const: aclk
+      - const: hclkf
+      - const: pclkin
+
+  resets:
+    items:
+      - description: AXI
+      - description: AHB
+      - description: PCLK IN
+
+  reset-names:
+    items:
+      - const: axi
+      - const: ahb
+      - const: pclkin
+
+  power-domains:
+    maxItems: 1
+    description: phandle to the associated power domain
+
+  # See ./video-interfaces.txt for details
+  port:
+    type: object
+    additionalProperties: false
+
+    properties:
+      endpoint:
+        type: object
+
+        properties:
+          remote-endpoint: true
+
+        required:
+          - remote-endpoint
+
+    required:
+      - endpoint
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/px30-cru.h>
+    #include <dt-bindings/power/px30-power.h>
+
+    cif: cif@ff490000 {
+    	compatible = "rockchip,px30-cif";
+    	reg = <0x0 0xff490000 0x0 0x200>;
+    	interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
+    	clocks = <&cru ACLK_CIF>, <&cru HCLK_CIF>, <&cru PCLK_CIF>, <&cru SCLK_CIF_OUT>;
+    	clock-names = "aclk_cif", "hclk_cif", "pclk_cif", "cif_out";
+    	resets = <&cru SRST_CIF_A>, <&cru SRST_CIF_H>, <&cru SRST_CIF_PCLKIN>;
+    	reset-names = "rst_cif_a", "rst_cif_h", "rst_cif_pclkin";
+    	power-domains = <&power PX30_PD_VI>;
+            port {
+                    cif_in: endpoint {
+                            remote-endpoint = <&tw9900_out>;
+                    };
+            };
+    };
+...
-- 
2.25.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v2 0/3] media: rockchip: Introduce driver for the camera interface on PX30
From: Maxime Chevallier @ 2020-05-29 13:04 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Robin Murphy, Rob Herring, Mark Rutland,
	Heiko Stuebner, Hans Verkuil
  Cc: devicetree, linux-kernel, Maxime Chevallier, Paul Kocialkowski,
	linux-rockchip, Thomas Petazzoni, Miquel Raynal, linux-arm-kernel,
	linux-media

Hello everyone,

Here's a V2 of the series adding very basic support for the camera interface on
the Rockchip PX30 SoC.

Thanks to everyone that commented on the first series, your reviews were
very helpful :)

This Camera Interface is also supported on other Rockchip SoC such as
the RK1808, RK3128, RK3288 and RK3288, but for now I've only been able to
test it on the PX30, using a PAL format.

This driver is mostly based on the driver found in Rockchip's BSP, that
has been trimmed down to support the set of features that I was able to test,
that is pretty much a very basic one-frame capture and video streaming
with GStreamer. 

This first draft only supports the Parallel interface, although the
controller has support for BT656 and CSI2.

Finally, this controller has an iommu that could be used in this driver,
but as of today I've not been able to get it to work.

Any review is welcome.

Thanks,

Maxime

--- Changes since V1 ---

 - Took reviews from Rob, Hans, Robin and Heiko into account :
  - Renamed the clocks in the binding
  - Fixed the DT schema compiling
  - Fixed a few typos
  - Used the clk bulk API
  - Used the reset array API
  - Changed a few helpers for more suitable ones
  - Rebased on 5.7-rc7



Maxime Chevallier (3):
  media: dt-bindings: media: Document Rockchip CIF bindings
  media: rockchip: Introduce driver for Rockhip's camera interface
  arm64: dts: rockchip: Add the camera interface description of the PX30

 .../bindings/media/rockchip-cif.yaml          |  100 ++
 arch/arm64/boot/dts/rockchip/px30.dtsi        |   12 +
 drivers/media/platform/Kconfig                |   13 +
 drivers/media/platform/Makefile               |    1 +
 drivers/media/platform/rockchip/cif/Makefile  |    3 +
 drivers/media/platform/rockchip/cif/capture.c | 1170 +++++++++++++++++
 drivers/media/platform/rockchip/cif/dev.c     |  358 +++++
 drivers/media/platform/rockchip/cif/dev.h     |  213 +++
 drivers/media/platform/rockchip/cif/regs.h    |  256 ++++
 9 files changed, 2126 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/rockchip-cif.yaml
 create mode 100644 drivers/media/platform/rockchip/cif/Makefile
 create mode 100644 drivers/media/platform/rockchip/cif/capture.c
 create mode 100644 drivers/media/platform/rockchip/cif/dev.c
 create mode 100644 drivers/media/platform/rockchip/cif/dev.h
 create mode 100644 drivers/media/platform/rockchip/cif/regs.h

-- 
2.25.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH v2 3/3] arm64: dts: rockchip: Add the camera interface description of the PX30
From: Maxime Chevallier @ 2020-05-29 13:04 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Robin Murphy, Rob Herring, Mark Rutland,
	Heiko Stuebner, Hans Verkuil
  Cc: devicetree, linux-kernel, Maxime Chevallier, Paul Kocialkowski,
	linux-rockchip, Thomas Petazzoni, Miquel Raynal, linux-arm-kernel,
	linux-media
In-Reply-To: <20200529130405.929429-1-maxime.chevallier@bootlin.com>

The PX30 has a camera interface, supporting CSI2, BT656 and Parallel
modes. Add a DT description for this interface.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---

Changes since V1:

 - Updated the clock and reset names
 - Reordered the properties to have clocks and resets bundled together

 arch/arm64/boot/dts/rockchip/px30.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
index 9f84aaaf3fba..d895541b7216 100644
--- a/arch/arm64/boot/dts/rockchip/px30.dtsi
+++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
@@ -1140,6 +1140,18 @@ vopl_mmu: iommu@ff470f00 {
 		status = "disabled";
 	};
 
+	cif: cif@ff490000 {
+		compatible = "rockchip,px30-cif";
+		reg = <0x0 0xff490000 0x0 0x200>;
+		interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&cru ACLK_CIF>, <&cru HCLK_CIF>, <&cru PCLK_CIF>, <&cru SCLK_CIF_OUT>;
+		clock-names = "aclk", "hclk", "pclkin";
+		power-domains = <&power PX30_PD_VI>;
+		resets = <&cru SRST_CIF_A>, <&cru SRST_CIF_H>, <&cru SRST_CIF_PCLKIN>;
+		reset-names = "axi", "ahb", "pclkin";
+		status = "disabled";
+	};
+
 	qos_gmac: qos@ff518000 {
 		compatible = "syscon";
 		reg = <0x0 0xff518000 0x0 0x20>;
-- 
2.25.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH 0/8] Convert the intel iommu driver to the dma-iommu api
From: Christoph Hellwig @ 2020-05-29 12:45 UTC (permalink / raw)
  To: Logan Gunthorpe
  Cc: kvm, David Airlie, Joonas Lahtinen, dri-devel, Bjorn Andersson,
	linux-tegra, Julien Grall, Thierry Reding, Will Deacon,
	Marek Szyprowski, Jean-Philippe Brucker, linux-samsung-soc,
	Marc Zyngier, Krzysztof Kozlowski, Jonathan Hunter,
	linux-rockchip, Andy Gross, linux-arm-kernel, linux-s390,
	linux-arm-msm, intel-gfx, Jani Nikula, Alex Williamson,
	linux-mediatek, Rodrigo Vivi, Matthias Brugger, Thomas Gleixner,
	virtualization, Gerald Schaefer, David Woodhouse, Cornelia Huck,
	linux-kernel, Tom Murphy, iommu, Kukjin Kim, Robin Murphy
In-Reply-To: <465815ae-9292-f37a-59b9-03949cb68460@deltatee.com>

On Thu, May 28, 2020 at 06:00:44PM -0600, Logan Gunthorpe wrote:
> > This issue is most likely in the i915 driver and is most likely caused by the driver not respecting the return value of the dma_map_ops::map_sg function. You can see the driver ignoring the return value here:
> > https://github.com/torvalds/linux/blob/7e0165b2f1a912a06e381e91f0f4e495f4ac3736/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c#L51
> > 
> > Previously this didn’t cause issues because the intel map_sg always returned the same number of elements as the input scatter gather list but with the change to this dma-iommu api this is no longer the case. I wasn’t able to track the bug down to a specific line of code unfortunately.  

Mark did a big audit into the map_sg API abuse and initially had
some i915 patches, but then gave up on them with this comment:

"The biggest TODO is DRM/i915 driver and I don't feel brave enough to fix
 it fully. The driver creatively uses sg_table->orig_nents to store the
 size of the allocate scatterlist and ignores the number of the entries
 returned by dma_map_sg function. In this patchset I only fixed the
 sg_table objects exported by dmabuf related functions. I hope that I
 didn't break anything there."

it would be really nice if the i915 maintainers could help with sorting
that API abuse out.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] irqchip/gic-v3-its: Don't try to move a disabled irq
From: Saidi, Ali @ 2020-05-29 12:36 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Herrenschmidt, Benjamin, Jason Cooper, Machulsky, Zorik,
	linux-kernel@vger.kernel.org, Zilberman, Zeev,
	linux-arm-kernel@lists.infradead.org, Thomas Gleixner,
	Woodhouse, David
In-Reply-To: <8c3be990888ecfb7cca9503853dc4aac@kernel.org>

Hi Marc,

> On May 29, 2020, at 3:33 AM, Marc Zyngier <maz@kernel.org> wrote:
> 
> Hi Ali,
> 
>> On 2020-05-29 02:55, Ali Saidi wrote:
>> If an interrupt is disabled the ITS driver has sent a discard removing
>> the DeviceID and EventID from the ITT. After this occurs it can't be
>> moved to another collection with a MOVI and a command error occurs if
>> attempted. Before issuing the MOVI command make sure that the IRQ isn't
>> disabled and change the activate code to try and use the previous
>> affinity.
>> 
>> Signed-off-by: Ali Saidi <alisaidi@amazon.com>
>> ---
>> drivers/irqchip/irq-gic-v3-its.c | 18 +++++++++++++++---
>> 1 file changed, 15 insertions(+), 3 deletions(-)
>> 
>> diff --git a/drivers/irqchip/irq-gic-v3-its.c
>> b/drivers/irqchip/irq-gic-v3-its.c
>> index 124251b0ccba..1235dd9a2fb2 100644
>> --- a/drivers/irqchip/irq-gic-v3-its.c
>> +++ b/drivers/irqchip/irq-gic-v3-its.c
>> @@ -1540,7 +1540,11 @@ static int its_set_affinity(struct irq_data *d,
>> const struct cpumask *mask_val,
>>      /* don't set the affinity when the target cpu is same as current one
>> */
>>      if (cpu != its_dev->event_map.col_map[id]) {
>>              target_col = &its_dev->its->collections[cpu];
>> -             its_send_movi(its_dev, target_col, id);
>> +
>> +             /* If the IRQ is disabled a discard was sent so don't move */
>> +             if (!irqd_irq_disabled(d))
>> +                     its_send_movi(its_dev, target_col, id);
>> +
> 
> This looks wrong. What you are testing here is whether the interrupt
> is masked, not that there isn't a valid translation.
I’m not exactly sure the correct condition, but what I’m looking for is interrupts which are deactivated and we have thus sent a discard. 

> 
> In the commit message, you're saying that we've issued a discard. This
> hints at doing a set_affinity on an interrupt that has been deactivated
> (mapping removed). Is that actually the case? If so, why was it
> deactivated
> the first place?
This is the case. If we down a NIC, that interface’s MSIs will be deactivated but remain allocated until the device is unbound from the driver or the NIC is brought up. 

While stressing down/up a device I’ve found that irqbalance can move interrupts and you end up with the situation described. The device is downed, the interrupts are deactivated but still present and then trying to move one results in sending a MOVI after the DISCARD which is an error per the GIC spec. 

> 
>>              its_dev->event_map.col_map[id] = cpu;
>>              irq_data_update_effective_affinity(d, cpumask_of(cpu));
>>      }
>> @@ -3439,8 +3443,16 @@ static int its_irq_domain_activate(struct
>> irq_domain *domain,
>>      if (its_dev->its->numa_node >= 0)
>>              cpu_mask = cpumask_of_node(its_dev->its->numa_node);
>> 
>> -     /* Bind the LPI to the first possible CPU */
>> -     cpu = cpumask_first_and(cpu_mask, cpu_online_mask);
>> +     /* If the cpu set to a different CPU that is still online use it */
>> +     cpu = its_dev->event_map.col_map[event];
>> +
>> +     cpumask_and(cpu_mask, cpu_mask, cpu_online_mask);
>> +
>> +     if (!cpumask_test_cpu(cpu, cpu_mask)) {
>> +             /* Bind the LPI to the first possible CPU */
>> +             cpu = cpumask_first(cpu_mask);
>> +     }
>> +
>>      if (cpu >= nr_cpu_ids) {
>>              if (its_dev->its->flags & ITS_FLAGS_WORKAROUND_CAVIUM_23144)
>>                      return -EINVAL;
> 
> So you deactivate an interrupt, do a set_affinity that doesn't issue
> a MOVI but preserves the affinity, then reactivate it and hope that
> the new mapping will target the "right" CPU.
> 
> That seems a bit mad, but I presume this isn't the whole story...
Doing some experiments it appears as though other interrupts controllers do preserve affinity across deactivate/activate, so this is my attempt at doing the same. 

Thanks,
Ali
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [RFC PATCH V4 4/4] platform: mtk-isp: Add Mediatek FD driver
From: Jerry-ch Chen @ 2020-05-29 12:26 UTC (permalink / raw)
  To: Tomasz Figa
  Cc: linux-devicetree, Sean Cheng (鄭昇弘),
	Laurent Pinchart, zwisler, srv_heupstream,
	Christie Yu (游雅惠), jerry-ch.chen,
	Hans Verkuil, Jungo Lin (林明俊), Sj Huang,
	yuzhao, Hans Verkuil, Pi-Hsun Shih,
	Frederic Chen (陳俊元), Matthias Brugger,
	moderated list:ARM/Mediatek SoC support, Mauro Carvalho Chehab,
	list@263.net:IOMMU DRIVERS <iommu@lists.linux-foundation.org>,  Joerg  Roedel <joro@8bytes.org>, ,
	Linux Media Mailing List
In-Reply-To: <CAAFQd5DodDfWsHkhQZP-M70k9_2sUwwb4zHtWfTx5EDyEKkwow@mail.gmail.com>

Hi Tomasz,

I Appreciate your review comments, here's the reply.

On Mon, 2020-05-25 at 14:24 +0200, Tomasz Figa wrote:
> r
> 
> On Fri, May 22, 2020 at 4:11 PM Jerry-ch Chen
> <Jerry-ch.Chen@mediatek.com> wrote:
> >
> > Hi Tomasz,
> >
> > On Thu, 2020-05-21 at 18:28 +0000, Tomasz Figa wrote:
> > > Hi Jerry,
> > >
> > > On Wed, Dec 04, 2019 at 08:47:32PM +0800, Jerry-ch Chen wrote:
> [snip]
> > > > +
> > > > +enum face_angle {
> > > > +   MTK_FD_FACE_FRONT,
> > > > +   MTK_FD_FACE_RIGHT_50,
> > > > +   MTK_FD_FACE_LEFT_50,
> > > > +   MTK_FD_FACE_RIGHT_90,
> > > > +   MTK_FD_FACE_LEFT_90,
> > > > +   MTK_FD_FACE_ANGLE_NUM,
> > > > +};
> > >
> > > This enum seems to define values for the V4L2_CID_MTK_FD_DETECT_POSE
> > > control. Considering that this is an enumeration and the values are
> > > actually integers (-90, -50, 0, 50, 90), perhaps this should be an
> > > INTEGER_MENU control instead?
> > >
> >
> > this ioctl let user select multiple face positions(combination of angles
> > and directions) to be detected. so I thought I am not able to use the
> > INTEGER_MENU for this purpose.
> 
> Ah, okay, I thought there is only one selection possible.
> 
> >
> > A bit-field as following should be used by user.
> > I consider adding it to uapi.
> >
> > struct face_direction_def {
> > __u16 MTK_FD_FACE_DIR_0 : 1,
> >         MTK_FD_FACE_DIR_30 : 1,
> >         MTK_FD_FACE_DIR_60 : 1,
> >         MTK_FD_FACE_DIR_90 : 1,
> >         MTK_FD_FACE_DIR_120 : 1,
> >         MTK_FD_FACE_DIR_150 : 1,
> >         MTK_FD_FACE_DIR_180 : 1,
> >         MTK_FD_FACE_DIR_210 : 1,
> >         MTK_FD_FACE_DIR_240 : 1,
> >         MTK_FD_FACE_DIR_270 : 1,
> >         MTK_FD_FACE_DIR_300 : 1,
> >         MTK_FD_FACE_DIR_330 : 1,
> >         : 4;
> > };
> 
> Note that bit fields are not recommended in UAPI because of not being
> well specified by the language. Instead bits should be defined using
> macros with explicit masks or sometimes enums.
> 
Ok, I'll define them in macro with masks.

> >
> > User can also select some face directions of each face angle in one
> > ioctl, for example:
> >
> > /*
> >  * u16 face_directions[MTK_FD_FACE_ANGLE_NUM] = {0};
> >  *
> >  *      face_directions[MTK_FD_FACE_FRONT] = 0x7; //angle:0, dir:0,30,60
> >  *      face_directions[MTK_FACE_RIGHT_50] = 0x2; //angle:50, dir:30
> >  *
> >  */
> 
> Makes sense, thanks.
> 
> >
> > > > +
> > > > +struct fd_buffer {
> > > > +   __u32 scp_addr; /* used by SCP */
> > > > +   __u32 dma_addr; /* used by DMA HW */
> > > > +} __packed;
> > fd buffer is used for scp ipi
> >
> > > > +
> > > > +struct fd_face_result {
> > > > +   char data[16];
> > > > +};
> > fd_face_result is used for user, so it should be moved to
> > include/uapi/linux.
> > In fact, it has bit-field definition for user, so I would like to define
> > it in include/uapi/linux as following:
> >
> > struct fd_face_result {
> >   __u64 face_idx : 12,
> >         type : 1,
> >         x0 : 10,
> >         y0 : 10,
> >         x1 : 10,
> >         y1 : 10,
> >         fcv1 : 11;
> >   __u64 fcv2 : 7,
> >         rip_dir : 4,
> >         rop_dir : 3,
> >         det_size : 5;
> > };
> >
> 
> Indeed this should be defined, but as per my comment above, please
> avoid using the bitfield construct and define shifts and masks
> instead.
> 
Ok, I'll fix it.

> >
> > > > +
> > > > +struct fd_user_output {
> > > > +   struct fd_face_result results[MTK_FD_MAX_RESULT_NUM];
> > > > +   __u16 number;
> > >
> > > Is this perhaps the number of results? If so, would num_results be a better
> > > name?
> > >
> > yes, fixed.
> > > > +};
> > >
> > > Since this struct is the meta buffer format, it is a part of the userspace
> > > interface and should be defined in a header under include/uapi/linux/.
> > >
> > Ok, I will create include/uapi/linux/mtk_fd_40.h
> > which suppose to include structures that userspace will use.
> > should the private IOCTLs be placed in it together?
> >
> 
> Sorry, what private IOCTLs are you referring to? If you mean private
> control IDs, then yes, they should go to that header.
yes, the IDs, sorry for the wrong expression.

> 
> [snip]
> > > > +static int mtk_fd_vb2_queue_setup(struct vb2_queue *vq,
> > > > +                             unsigned int *num_buffers,
> > > > +                             unsigned int *num_planes,
> > > > +                             unsigned int sizes[],
> > > > +                             struct device *alloc_devs[])
> > > > +{
> > > > +   struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> > > > +   unsigned int size[2];
> > > > +   unsigned int plane;
> > > > +
> > > > +   switch (vq->type) {
> > > > +   case V4L2_BUF_TYPE_META_CAPTURE:
> > > > +           size[0] = ctx->dst_fmt.buffersize;
> > > > +           break;
> > > > +   case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
> > > > +           size[0] = ctx->src_fmt.plane_fmt[0].sizeimage;
> > > > +           if (*num_planes == 2)
> > > > +                   size[1] = ctx->src_fmt.plane_fmt[1].sizeimage;
> > > > +           break;
> > > > +   }
> > >
> > > Is this code above needed? The code below sets sizes[] and it uses a for loop,
> > > without opencoded assignment for the second plane.
> > >
> >
> > Looks like not really useful here,
> > it should check sizes and num_planes if num_plane not zero,
> > and for V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, it will at most have 2
> > planes, maybe no need for loop as well.
> 
> Loops generally make the code cleaner and there might be some desire
> to add support for more formats in the future, e.g. in case a next
> generation of the hardware shows up.
> 
Ok, got it.

> > I will refine this function as following:
> > mtk_fd_vb2_queue_setup(...)
> > {
> >         struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> >
> >         if (*num_planes == 0) {
> >                 if (vq->type == V4L2_BUF_TYPE_META_CAPTURE) {
> >                         sizes[0] = ctx->dst_fmt.buffersize;
> >                         *num_planes = 1;
> >                         return 0;
> >                 } else if (vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
> >                         *num_planes = ctx->src_fmt.num_planes;
> >                         sizes[0] = ctx->src_fmt.plane_fmt[0].sizeimage;
> >                         if (*num_planes == 2)
> >                                 sizes[1] = ctx->src_fmt.plane_fmt[1].sizeimage;
> >                         return 0;
> >                 }
> >                 return -EINVAL;
> >         }
> >
> >         /* If num_plane not zero, check the num_plane and sizes*/
> >         if (vq->type == V4L2_BUF_TYPE_META_CAPTURE) {
> >                 if ((*num_planes == 1) &&
> >                     (sizes[0] <= ctx->dst_fmt.buffersize))
> >                         return 0;
> 
> nit: The typical convention is to check for problems and return the
> error code earlier, with the success handled at the end of the block.
> 
Got it.

> >                 else
> >                         return -EINVAL;
> >         }
> >         if (vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
> >                 if ((*num_planes == 1) &&
> >                     (sizes[0] <= ctx->src_fmt.plane_fmt[0].sizeimage))
> >                         return 0;
> >                 else if ((*num_planes == 2) &&
> >                          (sizes[0] <= ctx->src_fmt.plane_fmt[0].sizeimage) &&
> >                          (sizes[1] <= ctx->src_fmt.plane_fmt[1].sizeimage))
> >                         return 0;
> 
> Wouldn't a loop eliminate the need to if/else if through the various
> supported cases and duplicate the size checks?
> 
> >                 else
> >                         return -EINVAL;
> >
> >         }
> >         return 0;
> > }
> 
> How about the following?
> 
> mtk_fd_vb2_queue_setup(...)
> {
>         struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> 
>         if (vq->type == V4L2_BUF_TYPE_META_CAPTURE) {
>                 if (*num_planes == 0) {
>                         *num_planes = 1;
>                         sizes[0] = ctx->dst_fmt.buffersize;
>                         return 0;
>                 }
> 
>                 if (*num_planes != 1 || sizes[0] < ctx->dst_fmt.buffersize)
>                                 return -EINVAL;
> 
>                 return 0;
>         }
> 
>         /* V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE */
>         if (*num_planes == 0) {
>                         *num_planes = ctx->src_fmt.num_planes;
>                         for (i = 0; i < ctx->src_fmt.num_planes; ++i)
>                                 sizes[i] = ctx->src_fmt.plane_fmt[i].sizeimage;
>                         return 0;
>         }
> 
>         if (*num_planes < ctx->src_fmt.num_planes)
>                 return -EINVAL;
> 
>         for (i = 0; i < ctx->src_fmt.num_planes; ++i)
>                 if (sizes[i] < ctx->src_fmt.plane_fmt[i].sizeimage)
>                         return -EINVAL;
> 
>         return 0;
> }
> 
> Note that it fully separates the code dealing with each queue and thus
> improves the readability.
> 
> In this case, it could actually be beneficial to split the vb2_ops
> implementation into one that deals only with video_output_mplane and
> one only with meta_capture. This would allow eliminating the special
> casing based on vq->type and thus further simplify the code. Not sure
> if it applies to the other vb2 callbacks, though.
> 
Got it, thanks for the comments.

> [snip]
> > > > +static void mtk_fd_fill_pixfmt_mp(struct v4l2_pix_format_mplane *dfmt,
> > > > +                             const struct v4l2_pix_format_mplane *sfmt)
> > > > +{
> > > > +   dfmt->field = V4L2_FIELD_NONE;
> > > > +   dfmt->colorspace = V4L2_COLORSPACE_BT2020;
> > > > +   dfmt->num_planes = sfmt->num_planes;
> > > > +   dfmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> > > > +   dfmt->quantization = V4L2_QUANTIZATION_DEFAULT;
> > > > +   dfmt->xfer_func =
> > > > +           V4L2_MAP_XFER_FUNC_DEFAULT(dfmt->colorspace);
> > > > +
> > > > +   /* Keep user setting as possible */
> > > > +   dfmt->width = clamp(dfmt->width,
> > > > +                       MTK_FD_OUTPUT_MIN_WIDTH,
> > > > +                       MTK_FD_OUTPUT_MAX_WIDTH);
> > > > +   dfmt->height = clamp(dfmt->height,
> > > > +                        MTK_FD_OUTPUT_MIN_HEIGHT,
> > > > +                        MTK_FD_OUTPUT_MAX_HEIGHT);
> > > > +
> > > > +   if (sfmt->num_planes == 2) {
> > > > +           /* NV16M and NV61M has 1 byte per pixel */
> > > > +           dfmt->plane_fmt[0].bytesperline = dfmt->width;
> > > > +           dfmt->plane_fmt[1].bytesperline = dfmt->width;
> > > > +   } else {
> > > > +           /* 2 bytes per pixel */
> > > > +           dfmt->plane_fmt[0].bytesperline = dfmt->width * 2;
> > > > +   }
> > > > +
> > > > +   dfmt->plane_fmt[0].sizeimage =
> > > > +           dfmt->height * dfmt->plane_fmt[0].bytesperline;
> > >
> > > Could some of the code above be replaced with v4l2_fill_pixfmt_mp()?
> > >
> > I would like to refine as following
> >
> > mtk_fd_fill_pixfmt_mp(...){
> >         v4l2_fill_pixfmt_mp(dfmt, sfmt->pixelformat, dfmt->width,
> > dfmt->height);
> >
> >         dfmt->field = V4L2_FIELD_NONE;
> >         dfmt->colorspace = V4L2_COLORSPACE_BT2020;
> >         dfmt->num_planes = sfmt->num_planes;
> 
> num_planes should be already filled in by the helper. That actually
> raises a question on whether there is any need to have sfmt passed to
> this function at all. Perhaps all we need is the value of pixelformat?
> 
Yes, I'll replace sfmt with u32 pixfmt.

> >         dfmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> >         dfmt->quantization = V4L2_QUANTIZATION_DEFAULT;
> >         dfmt->xfer_func =
> >                 V4L2_MAP_XFER_FUNC_DEFAULT(dfmt->colorspace);
> > }
> >
> 
> Isn't still a need to clamp() width and height to min/max, though?
Yes, I'll add them back.

This function will be refined as :

static void mtk_fd_fill_pixfmt_mp(struct v4l2_pix_format_mplane *dfmt,
                                  u32 pixfmt)
{
        v4l2_fill_pixfmt_mp(dfmt, pixfmt, dfmt->width, dfmt->height);

        dfmt->field = V4L2_FIELD_NONE;
        dfmt->colorspace = V4L2_COLORSPACE_BT2020;
        dfmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
        dfmt->quantization = V4L2_QUANTIZATION_DEFAULT;
        dfmt->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(dfmt->colorspace);

        /* Keep user setting as possible */
        dfmt->width = clamp(dfmt->width,
                            MTK_FD_OUTPUT_MIN_WIDTH,
                            MTK_FD_OUTPUT_MAX_WIDTH);
        dfmt->height = clamp(dfmt->height,
                             MTK_FD_OUTPUT_MIN_HEIGHT,
                             MTK_FD_OUTPUT_MAX_HEIGHT);
}


> 
> [snip]
> > > > +   fd_param.user_param.src_img_fmt =
> > > > +           get_fd_img_fmt(ctx->src_fmt.pixelformat);
> > > > +   if (ctx->src_fmt.num_planes == 2)
> > > > +           fd_param.src_img[1].dma_addr =
> > > > +                   vb2_dma_contig_plane_dma_addr(&src_buf->vb2_buf, 1);
> > >
> > > nit: Could this be moved above, to be just below src_img[0] initialization,
> > > for readability reasons?
> > >
> > Ok, this function will be refined as
> >
> > static void mtk_fd_device_run(void *priv)
> > {
> >         struct mtk_fd_ctx *ctx = priv;
> >         struct mtk_fd_dev *fd = ctx->fd_dev;
> >         struct vb2_v4l2_buffer *src_buf, *dst_buf;
> >         struct fd_enq_param fd_param;
> >
> >         src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> >         dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
> >
> >         fd_param.src_img[0].dma_addr =
> >                 vb2_dma_contig_plane_dma_addr(&src_buf->vb2_buf, 0);
> >         if (ctx->src_fmt.num_planes == 2)
> >                 fd_param.src_img[1].dma_addr =
> >                         vb2_dma_contig_plane_dma_addr(&src_buf->vb2_buf, 1);
> 
> How about making this a loop in terms of ctx->src_fmt.num_planes?
> 
yes, it will be refined as following, I use the src_vb2_buf to reduce
the length for fitting 80 columns

src_vb2_buf = &src_buf->vb2_buf;
dst_vb2_buf = &dst_buf->vb2_buf;

for (i = 0; i < ctx->src_fmt.num_planes; i++)
	fd_param.src_img[i].dma_addr =
		vb2_dma_contig_plane_dma_addr(src_vb2_buf,i);

fd_param.user_result.dma_addr =
	vb2_dma_contig_plane_dma_addr(dst_vb2_buf, 0);

> >         fd_param.user_result.dma_addr =
> >                 vb2_dma_contig_plane_dma_addr(&dst_buf->vb2_buf, 0);
> >         fd_param.user_param.src_img_fmt =
> >                 get_fd_img_fmt(fd->dev, ctx->src_fmt.pixelformat);
> >
> >         mtk_fd_fill_user_param(&fd_param.user_param, &ctx->hdl);
> >
> >         /* Complete request controls if any */
> >         v4l2_ctrl_request_complete(src_buf->vb2_buf.req_obj.req, &ctx->hdl);
> >
> >         fd->output = vb2_plane_vaddr(&dst_buf->vb2_buf, 0);
> >         mtk_fd_hw_job_exec(fd, &fd_param);
> > }
> 
> Best regards,
> Tomasz

Thanks and Best regards,
Jerry
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 1/1] tty: serial: owl: Initialize lock before registering port
From: Andy Shevchenko @ 2020-05-29 12:03 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Greg Kroah-Hartman, Manivannan Sadhasivam, linux-actions,
	Linux Kernel Mailing List, Cristian Ciocaltea,
	open list:SERIAL DRIVERS, Jiri Slaby, Andy Shevchenko,
	linux-arm Mailing List
In-Reply-To: <4c40d6c1-47e8-b4ec-f2fd-e9767b03630c@suse.de>

On Fri, May 29, 2020 at 2:56 PM Andreas Färber <afaerber@suse.de> wrote:
>
> Am 29.05.20 um 13:34 schrieb Greg Kroah-Hartman:
> > On Fri, May 29, 2020 at 02:06:47PM +0300, Cristian Ciocaltea wrote:
> >> Running a lockdep-enabled kernel leads to the following splat when
> >> probing the owl-uart driver:

...

> >> Fixes: a3cb39d258ef ("serial: core: Allow detach and attach serial device for console")
> >> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com>

> >> +    spin_lock_init(&owl_port->port.lock);
> >> +
> >>      ret = uart_add_one_port(&owl_uart_driver, &owl_port->port);
> >>      if (ret)
> >>              owl_uart_ports[pdev->id] = NULL;
> >
> > Ugh, another one :(
> >
> > Thanks for this, will queue this up now.
>
> Thanks. If this is the expected pattern now, I'll also have to update
> in-flight patches, such as Sunplus.

Expected is to register properly console via register_console() call.
If you do so, you also need to initialize spin lock before coming to
uart_add_one_port().
It seems drivers (ab)used that feature.

$ git grep -n -w register_console -- drivers/tty/serial/ | cut -f1 -d:
| sort -u | wc -l
    37
$ git grep -n _CONSOLE -- drivers/tty/serial/ | cut -f1 -d:  | sort -u | wc -l
    77

But as a quick fix this initialization is okay.

-- 
With Best Regards,
Andy Shevchenko

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr
From: Herbert Xu @ 2020-05-29 12:02 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Eric Biggers, Stephan Mueller, Linux Crypto Mailing List,
	Linux ARM
In-Reply-To: <CAMj1kXFFPKWwwSpLnPmNa_Up1syMb7T5STG7Tw8mRuRqSzc9vw@mail.gmail.com>

On Fri, May 29, 2020 at 02:00:14PM +0200, Ard Biesheuvel wrote:
>
> Even if this is the case, it requires that an skcipher implementation
> stores an output IV in the buffer that skcipher request's IV field
> points to. Currently, we only check whether this is the case for CBC
> implementations, and so it is quite likely that lots of h/w
> accelerators or arch code don't adhere to this today.

They are and have always been broken because algif_skcipher has
always relied on this.

> This might be feasible for the generic CTS driver wrapping h/w
> accelerated CBC. But how is this supposed to work, e.g., for the two
> existing h/w implementations of cts(cbc(aes)) that currently ignore
> this?

They'll have to disable chaining.

The way I'm doing this would allow some implementations to allow
chaining while others of the same algorithm can disable chaining
and require the whole request to be presented together.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr
From: Ard Biesheuvel @ 2020-05-29 12:00 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Eric Biggers, Stephan Mueller, Linux Crypto Mailing List,
	Linux ARM
In-Reply-To: <20200529115126.GA3573@gondor.apana.org.au>

On Fri, 29 May 2020 at 13:51, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> On Fri, May 29, 2020 at 10:20:27AM +0200, Ard Biesheuvel wrote:
> >
> > But many implementation do not return an output IV at all. The only
> > mode that requires it (for the selftests to pass) is CBC.
>
> Most modes can be chained, e.g., CBC, PCBC, OFB, CFB and CTR.
> As it stands algif_skcipher requres all algorithms to support
> chaining.
>

Even if this is the case, it requires that an skcipher implementation
stores an output IV in the buffer that skcipher request's IV field
points to. Currently, we only check whether this is the case for CBC
implementations, and so it is quite likely that lots of h/w
accelerators or arch code don't adhere to this today.


> > For XTS, we would have to carry some metadata around that tells you
> > whether the initial encryption of the IV has occurred or not. In the
>
> You're right, XTS in its current form cannot be chained.  So we
> do need a way to mark that for algif_skcipher.
>
> > CTS case, you need two swap the last two blocks of ciphertext at the
> > very end.
>
> CTS can be easily chained.  You just need to always keep two blocks
> from being processed until you reach the end.
>

This might be feasible for the generic CTS driver wrapping h/w
accelerated CBC. But how is this supposed to work, e.g., for the two
existing h/w implementations of cts(cbc(aes)) that currently ignore
this?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 1/1] tty: serial: owl: Initialize lock before registering port
From: Andreas Färber @ 2020-05-29 11:53 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Cristian Ciocaltea
  Cc: Manivannan Sadhasivam, linux-actions, linux-kernel, linux-serial,
	Jiri Slaby, Andy Shevchenko, linux-arm-kernel
In-Reply-To: <20200529113419.GA1631227@kroah.com>

Am 29.05.20 um 13:34 schrieb Greg Kroah-Hartman:
> On Fri, May 29, 2020 at 02:06:47PM +0300, Cristian Ciocaltea wrote:
>> Running a lockdep-enabled kernel leads to the following splat when
>> probing the owl-uart driver:
>>
>> [    1.271784] b0124000.serial: ttyOWL2 at MMIO 0xb0124000 (irq = 22, base_baud = 1500000) is a owl-uart
>> [    1.281226] INFO: trying to register non-static key.
>> [    1.286179] the code is fine but needs lockdep annotation.
>> [    1.291648] turning off the locking correctness validator.
>> [    1.297125] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.7.0-rc7+ #70
>> [    1.303462] Hardware name: Generic DT based system
>> [    1.308268] [<80111d3c>] (unwind_backtrace) from [<8010c9b8>] (show_stack+0x10/0x14)
>> [    1.316003] [<8010c9b8>] (show_stack) from [<805016b4>] (dump_stack+0xb4/0xe0)
>> [    1.323218] [<805016b4>] (dump_stack) from [<80182dec>] (register_lock_class+0x25c/0x8f4)
>> [    1.331391] [<80182dec>] (register_lock_class) from [<8017ee34>] (__lock_acquire+0xb4/0x2ee4)
>> [    1.339901] [<8017ee34>] (__lock_acquire) from [<8018291c>] (lock_acquire+0x424/0x4c8)
>> [    1.347813] [<8018291c>] (lock_acquire) from [<808597b0>] (_raw_spin_lock_irqsave+0x54/0x68)
>> [    1.356242] [<808597b0>] (_raw_spin_lock_irqsave) from [<80582e94>] (uart_add_one_port+0x384/0x510)
>> [    1.365276] [<80582e94>] (uart_add_one_port) from [<8058b4d0>] (owl_uart_probe+0x1bc/0x248)
>> [    1.373621] [<8058b4d0>] (owl_uart_probe) from [<8059c0e4>] (platform_drv_probe+0x48/0x94)
>> [    1.381874] [<8059c0e4>] (platform_drv_probe) from [<805997c4>] (really_probe+0x200/0x470)
>> [    1.390123] [<805997c4>] (really_probe) from [<80599c80>] (driver_probe_device+0x16c/0x1bc)
>> [    1.398461] [<80599c80>] (driver_probe_device) from [<80599f18>] (device_driver_attach+0x44/0x60)
>> [    1.407317] [<80599f18>] (device_driver_attach) from [<8059a078>] (__driver_attach+0x144/0x14c)
>> [    1.416000] [<8059a078>] (__driver_attach) from [<805978ac>] (bus_for_each_dev+0x5c/0xb4)
>> [    1.424162] [<805978ac>] (bus_for_each_dev) from [<8059896c>] (bus_add_driver+0x118/0x204)
>> [    1.432410] [<8059896c>] (bus_add_driver) from [<8059ae6c>] (driver_register+0xbc/0xf8)
>> [    1.440405] [<8059ae6c>] (driver_register) from [<80c1fd24>] (owl_uart_init+0x20/0x40)
>> [    1.448313] [<80c1fd24>] (owl_uart_init) from [<801021d8>] (do_one_initcall+0x184/0x3a4)
>> [    1.456399] [<801021d8>] (do_one_initcall) from [<80c01100>] (kernel_init_freeable+0x264/0x2e4)
>> [    1.465085] [<80c01100>] (kernel_init_freeable) from [<80850a88>] (kernel_init+0x8/0x110)
>> [    1.473249] [<80850a88>] (kernel_init) from [<80100114>] (ret_from_fork+0x14/0x20)
>> [    1.480800] Exception stack(0xee8bdfb0 to 0xee8bdff8)
>> [    1.485841] dfa0:                                     00000000 00000000 00000000 00000000
>> [    1.494002] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>> [    1.502162] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000
>> [    1.508914] printk: console [ttyOWL2] enabled
>>
>> The locking issue occurs in uart_configure_port() when trying to
>> guard the call to set_mctrl().
>>
>> uart_add_one_port() should normally initialize the spinlock via
>> uart_port_spin_lock_init(), but it never happens because the port is
>> detected as a console and, as a consequence, the spinlock is expected
>> to be already initialized.
>>
>> The commit a3cb39d258ef
>> ("serial: core: Allow detach and attach serial device for console")
>> changed the lock initialization logic to assume the spinlock is
>> initialized even if the console is not enabled.
>>
>> Therefore, initialize the lock explicitly in owl_uart_probe(), before
>> attempting to invoke uart_add_one_port().
>>
>> Fixes: a3cb39d258ef ("serial: core: Allow detach and attach serial device for console")
>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com>
>> ---
>>   drivers/tty/serial/owl-uart.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/tty/serial/owl-uart.c b/drivers/tty/serial/owl-uart.c
>> index c149f8c30007..c2fa2f15d50a 100644
>> --- a/drivers/tty/serial/owl-uart.c
>> +++ b/drivers/tty/serial/owl-uart.c
>> @@ -705,6 +705,8 @@ static int owl_uart_probe(struct platform_device *pdev)
>>   	owl_uart_ports[pdev->id] = owl_port;
>>   	platform_set_drvdata(pdev, owl_port);
>>   
>> +	spin_lock_init(&owl_port->port.lock);
>> +
>>   	ret = uart_add_one_port(&owl_uart_driver, &owl_port->port);
>>   	if (ret)
>>   		owl_uart_ports[pdev->id] = NULL;
> 
> Ugh, another one :(
> 
> Thanks for this, will queue this up now.

Thanks. If this is the expected pattern now, I'll also have to update 
in-flight patches, such as Sunplus.

Regards,
Andreas

-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer
HRB 36809 (AG Nürnberg)

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr
From: Herbert Xu @ 2020-05-29 11:51 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Eric Biggers, Stephan Mueller, Linux Crypto Mailing List,
	Linux ARM
In-Reply-To: <CAMj1kXE43VvEXyYQF=s5HybhF6Wq23wDTrvTfNV9d4fUVZZ8aw@mail.gmail.com>

On Fri, May 29, 2020 at 10:20:27AM +0200, Ard Biesheuvel wrote:
>
> But many implementation do not return an output IV at all. The only
> mode that requires it (for the selftests to pass) is CBC.

Most modes can be chained, e.g., CBC, PCBC, OFB, CFB and CTR.
As it stands algif_skcipher requres all algorithms to support
chaining.
 
> For XTS, we would have to carry some metadata around that tells you
> whether the initial encryption of the IV has occurred or not. In the

You're right, XTS in its current form cannot be chained.  So we
do need a way to mark that for algif_skcipher.

> CTS case, you need two swap the last two blocks of ciphertext at the
> very end.

CTS can be easily chained.  You just need to always keep two blocks
from being processed until you reach the end.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply


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