* Re: [PATCH v5 3/7] mtd: spi-nor: add opcodes for octal Read/Write commands
From: Boris Brezillon @ 2018-12-10 11:27 UTC (permalink / raw)
To: Yogesh Narayan Gaur
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, vigneshr@ti.com,
robh@kernel.org, linux-kernel@vger.kernel.org,
linux-spi@vger.kernel.org, marek.vasut@gmail.com,
frieder.schrempf@exceet.de, broonie@kernel.org,
linux-mtd@lists.infradead.org, computersforpeace@gmail.com,
shawnguo@kernel.org, linux-arm-kernel@lists.infradead.org
In-Reply-To: <VI1PR04MB5726164D7E52B013FF2F135B99A50@VI1PR04MB5726.eurprd04.prod.outlook.com>
On Mon, 10 Dec 2018 11:17:20 +0000
Yogesh Narayan Gaur <yogeshnarayan.gaur@nxp.com> wrote:
> Hi Boris,
>
> > -----Original Message-----
> > From: Boris Brezillon [mailto:boris.brezillon@bootlin.com]
> > Sent: Monday, December 10, 2018 4:27 PM
> > To: Yogesh Narayan Gaur <yogeshnarayan.gaur@nxp.com>
> > Cc: linux-mtd@lists.infradead.org; broonie@kernel.org;
> > marek.vasut@gmail.com; vigneshr@ti.com; linux-spi@vger.kernel.org;
> > devicetree@vger.kernel.org; robh@kernel.org; mark.rutland@arm.com;
> > shawnguo@kernel.org; linux-arm-kernel@lists.infradead.org;
> > computersforpeace@gmail.com; frieder.schrempf@exceet.de; linux-
> > kernel@vger.kernel.org
> > Subject: Re: [PATCH v5 3/7] mtd: spi-nor: add opcodes for octal Read/Write
> > commands
> >
> > On Mon, 3 Dec 2018 08:39:18 +0000
> > Yogesh Narayan Gaur <yogeshnarayan.gaur@nxp.com> wrote:
> >
> > > - Add opcodes for octal I/O commands
> > > * Read : 1-1-8 and 1-8-8 protocol
> > > * Write : 1-1-8 and 1-8-8 protocol
> > > * opcodes for 4-byte address mode command
> > >
> > > - Entry of macros in _convert_3to4_xxx function
> > >
> > > - Add flag specifying flash support octal read commands.
> > >
> > > Signed-off-by: Vignesh R <vigneshr@ti.com>
> > > Signed-off-by: Yogesh Gaur <yogeshnarayan.gaur@nxp.com>
> >
> > Looks like the SoB and Author lines do not match
> >
> > Yogesh Narayan Gaur <yogeshnarayan.gaur@nxp.com> vs Yogesh Gaur
> > <yogeshnarayan.gaur@nxp.com>
> >
> > Can you find a way to make them match?
> I am sending the patches with my smtp server of OutlookOffice and in that my user name is "Yogesh Narayan Gaur" and in gitconfig of my Linux machine I set user name as "Yogesh Gaur".
> Is it mandatory to have same Author name in SoB and Author lines, if yes I would change the settings in gitconfig file.
We have scripts that check that the author/committer has its SoB,
depending on how strict the check is, it might complaint that SoB an
author/committer do not match, so yes, please change gitconfig to make
them match.
Thanks,
Boris
_______________________________________________
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 v5 1/5] spi: spi-mem: Add driver for NXP FlexSPI controller
From: Yogesh Narayan Gaur @ 2018-12-10 11:25 UTC (permalink / raw)
To: Boris Brezillon
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, robh@kernel.org,
linux-kernel@vger.kernel.org, Schrempf Frieder,
linux-spi@vger.kernel.org, marek.vasut@gmail.com,
broonie@kernel.org, linux-mtd@lists.infradead.org,
computersforpeace@gmail.com, shawnguo@kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20181210120925.3c6336f6@bbrezillon>
Hi Boris,
> -----Original Message-----
> From: Boris Brezillon [mailto:boris.brezillon@bootlin.com]
> Sent: Monday, December 10, 2018 4:39 PM
> To: Yogesh Narayan Gaur <yogeshnarayan.gaur@nxp.com>
> Cc: Schrempf Frieder <frieder.schrempf@kontron.de>; linux-
> mtd@lists.infradead.org; marek.vasut@gmail.com; broonie@kernel.org; linux-
> spi@vger.kernel.org; devicetree@vger.kernel.org; robh@kernel.org;
> mark.rutland@arm.com; shawnguo@kernel.org; linux-arm-
> kernel@lists.infradead.org; computersforpeace@gmail.com; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH v5 1/5] spi: spi-mem: Add driver for NXP FlexSPI controller
>
> On Mon, 10 Dec 2018 10:59:54 +0000
> Yogesh Narayan Gaur <yogeshnarayan.gaur@nxp.com> wrote:
>
> > Hi Boris,
> >
> > > -----Original Message-----
> > > From: Boris Brezillon [mailto:boris.brezillon@bootlin.com]
> > > Sent: Monday, December 10, 2018 4:20 PM
> > > To: Yogesh Narayan Gaur <yogeshnarayan.gaur@nxp.com>
> > > Cc: Schrempf Frieder <frieder.schrempf@kontron.de>; linux-
> > > mtd@lists.infradead.org; marek.vasut@gmail.com; broonie@kernel.org;
> > > linux- spi@vger.kernel.org; devicetree@vger.kernel.org;
> > > robh@kernel.org; mark.rutland@arm.com; shawnguo@kernel.org;
> > > linux-arm- kernel@lists.infradead.org; computersforpeace@gmail.com;
> > > linux- kernel@vger.kernel.org
> > > Subject: Re: [PATCH v5 1/5] spi: spi-mem: Add driver for NXP FlexSPI
> > > controller
> > >
> > > On Mon, 10 Dec 2018 10:43:56 +0000
> > > Yogesh Narayan Gaur <yogeshnarayan.gaur@nxp.com> wrote:
> > >
> > > > > > Thus, in LUT preparation we have assigned only the base address.
> > > > > > Now if I have assigned ahb_buf_size to FSPI_FLSHXXCR0 register
> > > > > > then for
> > > > > read/write data beyond limit of ahb_buf_size offset I get data corruption.
> > > > >
> > > > > Why would you do that? We have the ->adjust_op_size() exactly
> > > > > for this reason, so, if someone tries to do a spi_mem_op with
> > > > > data.nbytes > ahb_buf_size you should return an error.
> > > > >
> > > > Let me explain my implementation with example. If I have to write
> > > > data of size
> > > 0x100 bytes at offset 0x1200 for CS1, I would program as below:
> > > > In func nxp_fspi_select_mem(), would set value of controller
> > > > address space
> > > size, memmap_phy_size, to FSPI_FLSHA2CR0 and rest all FSPI_FLSHXXCR0 as
> 0.
> > > > Value of memmap_phy_size is 0x10000000 i.e. 256 MB for my
> > > > LX2160ARDB
> > > target.
> > > > Then in nxp_fspi_prepare_lut(), I would prepare LUT ADDR with
> > > > address length
> > > requirement 3/4 byte for NOR or 1/2/3/4 bytes for NAND flash.
> > > > Also for LUT_NXP_WRITE would program data bytes as 0.
> > > >
> > > > Then inside func nxp_fspi_do_op(), set register FSPI_IPCR0 as the
> > > > address offset i.e. 0x1200 and in register FSPI_IPCR1 program the
> > > > data size to write i.e. 0x100
> > > >
> > > > If, as suggested if I tries to mark value of register
> > > > FSPI_FLSHA2CR0 equal to
> > > ahb_buf_size (0x800), then access for address 0x1200 gives me wrong
> > > data. This is because as per the controller specification access to
> > > flash connected at CS1 can be performed under range of FSPI_ FLSHA1CR0
> and FSPI_ FLSHA2CR0.
> > >
> > > Don't you have a way to set an offset to apply to the address
> > > accessed through the AHB? And if you don't, how will it work if your
> > > mapping is smaller than the flash size?
> >
> > Write operations are triggered using IP commands instead of AHB command.
> > For Read AHB command is used and in this we are adding the offset when
> performing memcpy_fromIO operation
> > memcpy_fromio(op->data.buf.in, (f->ahb_addr + op->addr.val),
> > len);
> >
> > AHB/IP operations are independent of the way how CS got selected. CS
> selection depends, e.g. CS1 on the value of register FSPI_FLSHA1CR0 and
> FSPI_FLSHA2CR0.
> >
> > Mapping can never going to be smaller than the connected flash size as per
> discussion with the Board design team and if it's possible by user manually
> changes the non-soldered part then flash area beyond complete mapping is not
> accessible.
> > On LX2160ARDB, with mapping of 256MB, for now we are having 4 flash
> devices connected with size as 64 MB. If user wants he can have only one single
> flash with flash size of 256MB.
>
> Given that the dirmap interface has now been merged and the MTD side of
> things is soon to be merged, I'd recommend you to implement it in your
> v6 and only use non-AHB accesses for the ->exec_op() implementation.
This would going to be performance hit if I would use non-AHB accesses for ->exec_op().
In read in v5 I am using AHB mode for read if read data size is greater than rxfifo size and if its less than rxfifo then use IP mode for read.
--
Regards
Yogesh Gaur
_______________________________________________
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 v2 0/4] Add Actions Semi Owl family S700 I2C support
From: Parthiban Nallathambi @ 2018-12-10 11:25 UTC (permalink / raw)
To: afaerber, manivannan.sadhasivam, robh+dt, mark.rutland, wsa
Cc: devicetree, linux, linux-actions, linux-kernel, thomas.liau,
linux-i2c, pn, linux-arm-kernel
In-Reply-To: <20181126185821.2480449-1-pn@denx.de>
Ping on this patch series!
On 11/26/18 7:58 PM, Parthiban Nallathambi wrote:
> This patch series adds support for Actions Semi Owl SoC family S700
> I2C controller. S700 provides 4 I2C masters and with cubieboard7
> 2 (I2C0 and I2C1) are exposed.
>
> Added pinctrl definition for I2C controllers in cubieboard7. This patch
> depends on s700 pinctrl driver support (yet to be merged),
> https://lkml.org/lkml/2018/11/19/514
> https://lore.kernel.org/patchwork/patch/1012859/
>
> Changelog in v2:
> - Initial version https://lore.kernel.org/patchwork/patch/1011911/ only
> added the I2C nodes using s900 compatible property. Now, new s700
> compatiable string is added and used for S700
> - Device tree bindings added with s700 compatible string
> - pinctrl definition for cubieboard7
>
> Parthiban Nallathambi (4):
> dt-bindings: i2c: Add S700 support for Actions Semi Soc's
> i2c: Add Actions Semiconductor Owl family S700 I2C support
> arm64: dts: actions: s700: Add I2C controller nodes
> arm64: dts: actions: s700-cubieboard7: Enable I2C0 and I2C1
>
> .../devicetree/bindings/i2c/i2c-owl.txt | 2 +-
> .../boot/dts/actions/s700-cubieboard7.dts | 53 +++++++++++++++++++
> arch/arm64/boot/dts/actions/s700.dtsi | 40 ++++++++++++++
> drivers/i2c/busses/i2c-owl.c | 1 +
> 4 files changed, 95 insertions(+), 1 deletion(-)
>
--
Thanks,
Parthiban N
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-22 Fax: (+49)-8142-66989-80 Email: pn@denx.de
_______________________________________________
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 2/2] ARM: defconfig: Switch LPC18xx to use PL11x DRM driver
From: Vladimir Zapolskiy @ 2018-12-10 11:23 UTC (permalink / raw)
To: Linus Walleij; +Cc: linux-arm-kernel
In-Reply-To: <20181210101620.418-2-linus.walleij@linaro.org>
Hi Linus,
On 12/10/2018 12:16 PM, Linus Walleij wrote:
> None of the LPC18xx device trees contains any display settings,
> it just defines a device tree node for the CLCD (PL11x) set
> as "disabled" and no panels are attached on any device tree.
>
> This I conclude that the hardware is dormant on existing
> systems, so we can without any problems switch the defconfig
> over from the old ARMCLCD frame buffer driver to the new
> PL11x DRM driver.
>
Correct.
FWIW I've tested DRM PL111 driver on my board, if you are interested
the pending DT patch is https://patchwork.kernel.org/patch/10636591/
Thank you for the changes, I'll review and collect them for v4.22.
--
Best wishes,
Vladimir
_______________________________________________
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 v7 2/2] mtd: rawnand: meson: add support for Amlogic NAND flash controller
From: Liang Yang @ 2018-12-10 11:23 UTC (permalink / raw)
To: Miquel Raynal, Jianxin Pan
Cc: Rob Herring, Hanjie Lin, Victor Wan, Marek Vasut,
Martin Blumenstingl, Richard Weinberger, Neil Armstrong,
Yixun Lan, linux-kernel, Boris Brezillon, Jian Hu, linux-mtd,
Kevin Hilman, Carlo Caione, linux-amlogic, Brian Norris,
David Woodhouse, linux-arm-kernel, Jerome Brunet
In-Reply-To: <20181207102456.1dc67e07@xps13>
On 2018/12/7 17:24, Miquel Raynal wrote:
> Hi Jianxin,
>
> Looks good to me overall, a few comments inline.
>
> Jianxin Pan <jianxin.pan@amlogic.com> wrote on Sat, 17 Nov 2018
> 00:40:38 +0800:
>
>> From: Liang Yang <liang.yang@amlogic.com>
>>
>> Add initial support for the Amlogic NAND flash controller which found
>> in the Meson-GXBB/GXL/AXG SoCs.
>>
>> Signed-off-by: Liang Yang <liang.yang@amlogic.com>
>> Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
>> Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com>
>> ---
>> drivers/mtd/nand/raw/Kconfig | 10 +
>> drivers/mtd/nand/raw/Makefile | 1 +
>> drivers/mtd/nand/raw/meson_nand.c | 1417 +++++++++++++++++++++++++++++++++++++
>> 3 files changed, 1428 insertions(+)
>> create mode 100644 drivers/mtd/nand/raw/meson_nand.c
>>
>> diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
>> index c7efc31..223b041 100644
>> --- a/drivers/mtd/nand/raw/Kconfig
>> +++ b/drivers/mtd/nand/raw/Kconfig
>> @@ -541,4 +541,14 @@ config MTD_NAND_TEGRA
>> is supported. Extra OOB bytes when using HW ECC are currently
>> not supported.
>>
>> +config MTD_NAND_MESON
>> + tristate "Support for NAND controller on Amlogic's Meson SoCs"
>> + depends on ARCH_MESON || COMPILE_TEST
>> + depends on COMMON_CLK_AMLOGIC
>> + select COMMON_CLK_REGMAP_MESON
>> + select MFD_SYSCON
>> + help
>> + Enables support for NAND controller on Amlogic's Meson SoCs.
>> + This controller is found on Meson GXBB, GXL, AXG SoCs.
>> +
>> endif # MTD_NAND
>> diff --git a/drivers/mtd/nand/raw/Makefile b/drivers/mtd/nand/raw/Makefile
>> index 57159b3..a2cc2fe 100644
>> --- a/drivers/mtd/nand/raw/Makefile
>> +++ b/drivers/mtd/nand/raw/Makefile
>> @@ -56,6 +56,7 @@ obj-$(CONFIG_MTD_NAND_BRCMNAND) += brcmnand/
>> obj-$(CONFIG_MTD_NAND_QCOM) += qcom_nandc.o
>> obj-$(CONFIG_MTD_NAND_MTK) += mtk_ecc.o mtk_nand.o
>> obj-$(CONFIG_MTD_NAND_TEGRA) += tegra_nand.o
>> +obj-$(CONFIG_MTD_NAND_MESON) += meson_nand.o
>>
>> nand-objs := nand_base.o nand_legacy.o nand_bbt.o nand_timings.o nand_ids.o
>> nand-objs += nand_onfi.o
>> diff --git a/drivers/mtd/nand/raw/meson_nand.c b/drivers/mtd/nand/raw/meson_nand.c
>> new file mode 100644
>> index 0000000..c566636
>> --- /dev/null
>> +++ b/drivers/mtd/nand/raw/meson_nand.c
>> @@ -0,0 +1,1417 @@
>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> +/*
>> + * Amlogic Meson Nand Flash Controller Driver
>> + *
>> + * Copyright (c) 2018 Amlogic, inc.
>> + * Author: Liang Yang <liang.yang@amlogic.com>
>> + */
>> +
>> +#include <linux/platform_device.h>
>> +#include <linux/dma-mapping.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/clk.h>
>> +#include <linux/mtd/rawnand.h>
>> +#include <linux/mtd/mtd.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/regmap.h>
>> +#include <linux/slab.h>
>> +#include <linux/module.h>
>> +#include <linux/iopoll.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>> +
>> +#define NFC_REG_CMD 0x00
>> +#define NFC_CMD_DRD (0x8 << 14)
>> +#define NFC_CMD_IDLE (0xc << 14)
>> +#define NFC_CMD_DWR (0x4 << 14)
>> +#define NFC_CMD_CLE (0x5 << 14)
>> +#define NFC_CMD_ALE (0x6 << 14)
>> +#define NFC_CMD_ADL ((0 << 16) | (3 << 20))
>> +#define NFC_CMD_ADH ((1 << 16) | (3 << 20))
>> +#define NFC_CMD_AIL ((2 << 16) | (3 << 20))
>> +#define NFC_CMD_AIH ((3 << 16) | (3 << 20))
>> +#define NFC_CMD_SEED ((8 << 16) | (3 << 20))
>> +#define NFC_CMD_M2N ((0 << 17) | (2 << 20))
>> +#define NFC_CMD_N2M ((1 << 17) | (2 << 20))
>> +#define NFC_CMD_RB BIT(20)
>> +#define NFC_CMD_IO6 ((0xb << 10) | (1 << 18))
>> +#define NFC_CMD_SCRAMBLER_ENABLE BIT(19)
>> +#define NFC_CMD_RB_INT BIT(14)
>> +
>> +#define NFC_CMD_GET_SIZE(x) (((x) >> 22) & GENMASK(4, 0))
>> +
>> +#define NFC_REG_CFG 0x04
>> +#define NFC_REG_DADR 0x08
>> +#define NFC_REG_IADR 0x0c
>> +#define NFC_REG_BUF 0x10
>> +#define NFC_REG_INFO 0x14
>> +#define NFC_REG_DC 0x18
>> +#define NFC_REG_ADR 0x1c
>> +#define NFC_REG_DL 0x20
>> +#define NFC_REG_DH 0x24
>> +#define NFC_REG_CADR 0x28
>> +#define NFC_REG_SADR 0x2c
>> +#define NFC_REG_PINS 0x30
>> +#define NFC_REG_VER 0x38
>> +
>> +#define NFC_RB_IRQ_EN BIT(21)
>> +#define NFC_INT_MASK (3 << 20)
>> +
>> +#define CMDRWGEN(cmd_dir, ran, bch, short_mode, page_size, pages) \
>> + ( \
>> + (cmd_dir) | \
>> + ((ran) << 19) | \
>> + ((bch) << 14) | \
>> + ((short_mode) << 13) | \
>> + (((page_size) & 0x7f) << 6) | \
>> + ((pages) & 0x3f) \
>> + )
>> +
>> +#define GENCMDDADDRL(adl, addr) ((adl) | ((addr) & 0xffff))
>> +#define GENCMDDADDRH(adh, addr) ((adh) | (((addr) >> 16) & 0xffff))
>> +#define GENCMDIADDRL(ail, addr) ((ail) | ((addr) & 0xffff))
>> +#define GENCMDIADDRH(aih, addr) ((aih) | (((addr) >> 16) & 0xffff))
>> +
>> +#define RB_STA(x) (1 << (26 + (x)))
>> +#define DMA_DIR(dir) ((dir) ? NFC_CMD_N2M : NFC_CMD_M2N)
>> +
>> +#define ECC_CHECK_RETURN_FF (-1)
>> +
>> +#define NAND_CE0 (0xe << 10)
>> +#define NAND_CE1 (0xd << 10)
>> +
>> +#define DMA_BUSY_TIMEOUT 0x100000
>> +#define CMD_FIFO_EMPTY_TIMEOUT 1000
>> +
>> +#define MAX_CE_NUM 2
>> +
>> +/* eMMC clock register, misc control */
>> +#define SD_EMMC_CLOCK 0x00
>> +#define CLK_ALWAYS_ON BIT(28)
>> +#define CLK_SELECT_NAND BIT(31)
>> +#define CLK_DIV_MASK GENMASK(5, 0)
>> +
>> +#define NFC_CLK_CYCLE 6
>> +
>> +/* nand flash controller delay 3 ns */
>> +#define NFC_DEFAULT_DELAY 3000
>> +
>> +#define MAX_ECC_INDEX 10
>> +
>> +#define MUX_CLK_NUM_PARENTS 2
>> +
>> +#define ROW_ADDER(page, index) (((page) >> (8 * (index))) & 0xff)
>> +#define MAX_CYCLE_ADDRS 5
>> +#define DIRREAD 1
>> +#define DIRWRITE 0
>> +
>> +#define ECC_PARITY_BCH8_512B 14
>> +
>> +#define PER_INFO_BYTE 8
>> +
>> +#define ECC_COMPLETE BIT(31)
>> +#define ECC_ERR_CNT(x) (((x) >> 24) & GENMASK(5, 0))
>> +#define ECC_ZERO_CNT(x) (((x) >> 16) & GENMASK(5, 0))
>> +
>> +struct meson_nfc_nand_chip {
>> + struct list_head node;
>> + struct nand_chip nand;
>> + unsigned long clk_rate;
>> + unsigned long level1_divider;
>> + u32 bus_timing;
>> + u32 twb;
>> + u32 tadl;
>> + u32 tbers_max;
>> +
>> + u32 bch_mode;
>> + u8 *data_buf;
>> + __le64 *info_buf;
>> + u32 nsels;
>> + u8 sels[0];
>> +};
>> +
>> +struct meson_nand_ecc {
>> + u32 bch;
>> + u32 strength;
>> +};
>> +
>> +struct meson_nfc_data {
>> + const struct nand_ecc_caps *ecc_caps;
>> +};
>> +
>> +struct meson_nfc_param {
>> + u32 chip_select;
>> + u32 rb_select;
>> +};
>> +
>> +struct nand_rw_cmd {
>> + u32 cmd0;
>> + u32 addrs[MAX_CYCLE_ADDRS];
>> + u32 cmd1;
>> +};
>> +
>> +struct nand_timing {
>> + u32 twb;
>> + u32 tadl;
>> + u32 tbers_max;
>> +};
>> +
>> +struct meson_nfc {
>> + struct nand_controller controller;
>> + struct clk *core_clk;
>> + struct clk *device_clk;
>> + struct clk *phase_tx;
>> + struct clk *phase_rx;
>> +
>> + unsigned long clk_rate;
>> + u32 bus_timing;
>> +
>> + struct device *dev;
>> + void __iomem *reg_base;
>> + struct regmap *reg_clk;
>> + struct completion completion;
>> + struct list_head chips;
>> + const struct meson_nfc_data *data;
>> + struct meson_nfc_param param;
>> + struct nand_timing timing;
>> + union {
>> + int cmd[32];
>> + struct nand_rw_cmd rw;
>> + } cmdfifo;
>> +
>> + dma_addr_t daddr;
>> + dma_addr_t iaddr;
>> +
>> + unsigned long assigned_cs;
>> +};
>> +
>> +enum {
>> + NFC_ECC_BCH8_1K = 2,
>> + NFC_ECC_BCH24_1K,
>> + NFC_ECC_BCH30_1K,
>> + NFC_ECC_BCH40_1K,
>> + NFC_ECC_BCH50_1K,
>> + NFC_ECC_BCH60_1K,
>> +};
>> +
>> +#define MESON_ECC_DATA(b, s) { .bch = (b), .strength = (s)}
>> +
>> +static int meson_nand_calc_ecc_bytes(int step_size, int strength)
>> +{
>> + int ecc_bytes;
>> +
>> + if (step_size == 512 && strength == 8)
>> + return ECC_PARITY_BCH8_512B;
>> +
>> + ecc_bytes = DIV_ROUND_UP(strength * fls(step_size * 8), 8);
>> + ecc_bytes = ALIGN(ecc_bytes, 2);
>> +
>> + return ecc_bytes;
>> +}
>> +
>> +NAND_ECC_CAPS_SINGLE(meson_gxl_ecc_caps,
>> + meson_nand_calc_ecc_bytes, 1024, 8, 24, 30, 40, 50, 60);
>> +NAND_ECC_CAPS_SINGLE(meson_axg_ecc_caps,
>> + meson_nand_calc_ecc_bytes, 1024, 8);
>> +
>> +static struct meson_nfc_nand_chip *to_meson_nand(struct nand_chip *nand)
>> +{
>> + return container_of(nand, struct meson_nfc_nand_chip, nand);
>> +}
>> +
>> +static void meson_nfc_select_chip(struct nand_chip *nand, int chip)
>> +{
>> + struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
>> + struct meson_nfc *nfc = nand_get_controller_data(nand);
>> + int ret, value;
>> +
>> + if (chip < 0 || WARN_ON_ONCE(chip > MAX_CE_NUM))
>> + return;
>> +
>> + nfc->param.chip_select = meson_chip->sels[chip] ? NAND_CE1 : NAND_CE0;
>> + nfc->param.rb_select = nfc->param.chip_select;
>> + nfc->timing.twb = meson_chip->twb;
>> + nfc->timing.tadl = meson_chip->tadl;
>> + nfc->timing.tbers_max = meson_chip->tbers_max;
>> +
>> + if (chip >= 0) {
>> + if (nfc->clk_rate != meson_chip->clk_rate) {
>> + ret = clk_set_rate(nfc->device_clk,
>> + meson_chip->clk_rate);
>> + if (ret) {
>> + dev_err(nfc->dev, "failed to set clock rate\n");
>> + return;
>> + }
>> + nfc->clk_rate = meson_chip->clk_rate;
>> + }
>> + if (nfc->bus_timing != meson_chip->bus_timing) {
>> + value = (NFC_CLK_CYCLE - 1)
>> + | (meson_chip->bus_timing << 5);
>> + writel(value, nfc->reg_base + NFC_REG_CFG);
>> + writel((1 << 31), nfc->reg_base + NFC_REG_CMD);
>> + nfc->bus_timing = meson_chip->bus_timing;
>> + }
>> + }
>
> Don't you have timing registers to set?
>
there is no other timing setting except meson_chip->bus_timing.
>> +}
>> +
>> +static void meson_nfc_cmd_idle(struct meson_nfc *nfc, u32 time)
>> +{
>> + writel(nfc->param.chip_select | NFC_CMD_IDLE | (time & 0x3ff),
>> + nfc->reg_base + NFC_REG_CMD);
>> +}
>> +
>> +static void meson_nfc_cmd_seed(struct meson_nfc *nfc, u32 seed)
>> +{
>> + writel(NFC_CMD_SEED | (0xc2 + (seed & 0x7fff)),
>> + nfc->reg_base + NFC_REG_CMD);
>> +}
>> +
>> +static void meson_nfc_cmd_access(struct nand_chip *nand, int raw, bool dir)
>> +{
>> + struct mtd_info *mtd = nand_to_mtd(nand);
>> + struct meson_nfc *nfc = nand_get_controller_data(mtd_to_nand(mtd));
>> + struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
>> + u32 bch = meson_chip->bch_mode, cmd;
>> + int len = mtd->writesize, pagesize, pages;
>> + int scramble = (nand->options & NAND_NEED_SCRAMBLING) ? 1 : 0;
>
> There are quite a few places where you use hardcoded values, I would
> have preferred preprocessor defines for that. In this case, something
> link:
> >
> // naming is just as a reference, use whatever you want
> +#define CMD_SCRAMBLE BIT(19)
> [...]
> +int scramble = nand->options & NAND_NEED_SCRAMBLING) ? CMD_SCRAMBLE : 0;
>
> would be better (you can extend to other places as well).
>
ok, i will fix it.
>> +
>> + pagesize = nand->ecc.size;
>> +
>> + if (raw) {
>> + len = mtd->writesize + mtd->oobsize;
>> + cmd = (len & 0x3fff) | (scramble << 19) | DMA_DIR(dir);
>> + writel(cmd, nfc->reg_base + NFC_REG_CMD);
>> + return;
>> + }
>> +
>> + pages = len / nand->ecc.size;
>> +
>> + cmd = CMDRWGEN(DMA_DIR(dir), scramble, bch, 0, pagesize, pages);
>> +
>> + writel(cmd, nfc->reg_base + NFC_REG_CMD);
>> +}
>> +
>> +static void meson_nfc_drain_cmd(struct meson_nfc *nfc)
>> +{
>> + /*
>> + * Insert two commands to make sure all valid commands are finished.
>> + *
>> + * The Nand flash controller is designed as two stages pipleline -
>> + * a) fetch and b) excute.
>> + * There might be cases when the driver see command queue is empty,
>> + * but the Nand flash controller still has two commands buffered,
>> + * one is fetched into NFC request queue (ready to run), and another
>> + * is actively executing. So pushing 2 "IDLE" commands guarantees that
>> + * the pipeline is emptied.
>> + */
>> + meson_nfc_cmd_idle(nfc, 0);
>> + meson_nfc_cmd_idle(nfc, 0);
>> +}
>> +
>> +static int meson_nfc_wait_cmd_finish(struct meson_nfc *nfc,
>> + unsigned int timeout_ms)
>> +{
>> + u32 cmd_size = 0;
>> + int ret;
>> +
>> + /* wait cmd fifo is empty */
>> + ret = readl_relaxed_poll_timeout(nfc->reg_base + NFC_REG_CMD, cmd_size,
>> + !NFC_CMD_GET_SIZE(cmd_size),
>> + 10, timeout_ms * 1000);
>> + if (ret)
>> + dev_err(nfc->dev, "wait for empty cmd FIFO time out\n");
>> +
>> + return ret;
>> +}
>> +
>> +static int meson_nfc_wait_dma_finish(struct meson_nfc *nfc)
>> +{
>> + meson_nfc_drain_cmd(nfc);
>> +
>> + return meson_nfc_wait_cmd_finish(nfc, DMA_BUSY_TIMEOUT);
>> +}
>> +
>> +static u8 *meson_nfc_oob_ptr(struct nand_chip *nand, int i)
>> +{
>> + struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
>> + int len;
>> +
>> + len = nand->ecc.size * (i + 1) + (nand->ecc.bytes + 2) * i;
>> +
>> + return meson_chip->data_buf + len;
>> +}
>> +
>> +static u8 *meson_nfc_data_ptr(struct nand_chip *nand, int i)
>> +{
>> + struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
>> + int len, temp;
>> +
>> + temp = nand->ecc.size + nand->ecc.bytes;
>> + len = (temp + 2) * i;
>> +
>> + return meson_chip->data_buf + len;
>> +}
>> +
>> +static void meson_nfc_get_data_oob(struct nand_chip *nand,
>> + u8 *buf, u8 *oobbuf)
>> +{
>> + int i, oob_len = 0;
>> + u8 *dsrc, *osrc;
>> +
>> + oob_len = nand->ecc.bytes + 2;
>> + for (i = 0; i < nand->ecc.steps; i++) {
>> + if (buf) {
>> + dsrc = meson_nfc_data_ptr(nand, i);
>> + memcpy(buf, dsrc, nand->ecc.size);
>> + buf += nand->ecc.size;
>> + }
>> + osrc = meson_nfc_oob_ptr(nand, i);
>> + memcpy(oobbuf, osrc, oob_len);
>> + oobbuf += oob_len;
>> + }
>> +}
>> +
>> +static void meson_nfc_set_data_oob(struct nand_chip *nand,
>> + const u8 *buf, u8 *oobbuf)
>> +{
>> + int i, oob_len = 0;
>> + u8 *dsrc, *osrc;
>> +
>> + oob_len = nand->ecc.bytes + 2;
>> + for (i = 0; i < nand->ecc.steps; i++) {
>> + if (buf) {
>> + dsrc = meson_nfc_data_ptr(nand, i);
>> + memcpy(dsrc, buf, nand->ecc.size);
>> + buf += nand->ecc.size;
>> + }
>> + osrc = meson_nfc_oob_ptr(nand, i);
>> + memcpy(osrc, oobbuf, oob_len);
>> + oobbuf += oob_len;
>> + }
>> +}
>> +
>> +static int meson_nfc_queue_rb(struct meson_nfc *nfc, int timeout_ms)
>> +{
>> + u32 cmd, cfg;
>> + int ret = 0;
>> +
>> + meson_nfc_cmd_idle(nfc, nfc->timing.twb);
>> + meson_nfc_drain_cmd(nfc);
>> + meson_nfc_wait_cmd_finish(nfc, CMD_FIFO_EMPTY_TIMEOUT);
>> +
>> + cfg = readl(nfc->reg_base + NFC_REG_CFG);
>> + cfg |= (1 << 21);
>> + writel(cfg, nfc->reg_base + NFC_REG_CFG);
>> +
>> + init_completion(&nfc->completion);
>> +
>> + /* use the max erase time as the maximum clock for waiting R/B */
>> + cmd = NFC_CMD_RB | NFC_CMD_RB_INT
>> + | nfc->param.chip_select | nfc->timing.tbers_max;
>
> Nit: I think the '|' should be on the previous line.
>
ok
>> + writel(cmd, nfc->reg_base + NFC_REG_CMD);
>> +
>> + ret = wait_for_completion_timeout(&nfc->completion,
>> + msecs_to_jiffies(timeout_ms));
>> + if (ret == 0)
>> + ret = -1;
>> +
>> + return ret;
>> +}
>> +
>> +static void meson_nfc_set_user_byte(struct nand_chip *nand, u8 *oob_buf)
>> +{
>> + struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
>> + __le64 *info;
>> + int i, count;
>> +
>> + for (i = 0, count = 0; i < nand->ecc.steps; i++, count += 2) {
>> + info = &meson_chip->info_buf[i];
>> + *info |= oob_buf[count];
>> + *info |= oob_buf[count + 1] << 8;
>> + }
>> +}
>> +
>> +static void meson_nfc_get_user_byte(struct nand_chip *nand, u8 *oob_buf)
>> +{
>> + struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
>> + __le64 *info;
>> + int i, count;
>> +
>> + for (i = 0, count = 0; i < nand->ecc.steps; i++, count += 2) {
>> + info = &meson_chip->info_buf[i];
>> + oob_buf[count] = *info;
>> + oob_buf[count + 1] = *info >> 8;
>> + }
>> +}
>> +
>> +static int meson_nfc_ecc_correct(struct nand_chip *nand)
>> +{
>> + struct mtd_info *mtd = nand_to_mtd(nand);
>> + struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
>> + __le64 *info;
>> + u32 bitflips = 0, i;
>> + int scramble;
>> + u8 zero_cnt;
>> +
>> + scramble = (nand->options & NAND_NEED_SCRAMBLING) ? 1 : 0;
>> +
>> + for (i = 0; i < nand->ecc.steps; i++) {
>> + info = &meson_chip->info_buf[i];
>> + if (ECC_ERR_CNT(*info) == 0x3f) {
>> + zero_cnt = ECC_ZERO_CNT(*info);
>> + if (scramble && zero_cnt < nand->ecc.strength)
>> + return ECC_CHECK_RETURN_FF;
>
> This and what you do later with ECC_CHECK_RETURN_FF is pretty unclear
> to me.
>
it means a blank page here and later we will set data_buf and oob_buf
all 0xff befor return back.
>> + mtd->ecc_stats.failed++;
>> + continue;
>> + }
>> + mtd->ecc_stats.corrected += ECC_ERR_CNT(*info);
>> + bitflips = max_t(u32, bitflips, ECC_ERR_CNT(*info));
>> + }
>
> Are you sure you handle correctly empty pages with bf?
>
if scramble is enable, i would say yes here.
when scramble is disabled, i am considering how to use the helper
nand_check_erased_ecc_chunk, but it seems that i can't get the ecc
bytes which is caculated by ecc engine.by the way, nfc dma doesn't send
out the ecc parity bytes.
so i would suggest using scramble.
>> +
>> + return bitflips;
>> +}
>> +
>> +static int meson_nfc_dma_buffer_setup(struct nand_chip *nand, u8 *databuf,
>> + int datalen, u8 *infobuf, int infolen,
>> + enum dma_data_direction dir)
>> +{
>> + struct meson_nfc *nfc = nand_get_controller_data(nand);
>> + u32 cmd;
>> + int ret = 0;
>> +
>> + nfc->daddr = dma_map_single(nfc->dev, (void *)databuf, datalen, dir);
>> + ret = dma_mapping_error(nfc->dev, nfc->daddr);
>> + if (ret) {
>> + dev_err(nfc->dev, "dma mapping error\n");
>> + return ret;
>> + }
>> + cmd = GENCMDDADDRL(NFC_CMD_ADL, nfc->daddr);
>> + writel(cmd, nfc->reg_base + NFC_REG_CMD);
>> +
>> + cmd = GENCMDDADDRH(NFC_CMD_ADH, nfc->daddr);
>> + writel(cmd, nfc->reg_base + NFC_REG_CMD);
>> +
>> + if (infobuf) {
>> + nfc->iaddr = dma_map_single(nfc->dev, infobuf, infolen, dir);
>> + ret = dma_mapping_error(nfc->dev, nfc->iaddr);
>> + if (ret) {
>> + dev_err(nfc->dev, "dma mapping error\n");
>> + dma_unmap_single(nfc->dev,
>> + nfc->daddr, datalen, dir);
>> + return ret;
>> + }
>> + cmd = GENCMDIADDRL(NFC_CMD_AIL, nfc->iaddr);
>> + writel(cmd, nfc->reg_base + NFC_REG_CMD);
>> +
>> + cmd = GENCMDIADDRH(NFC_CMD_AIH, nfc->iaddr);
>> + writel(cmd, nfc->reg_base + NFC_REG_CMD);
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static void meson_nfc_dma_buffer_release(struct nand_chip *nand,
>> + int infolen, int datalen,
>> + enum dma_data_direction dir)
>> +{
>> + struct meson_nfc *nfc = nand_get_controller_data(nand);
>> +
>> + dma_unmap_single(nfc->dev, nfc->daddr, datalen, dir);
>> + if (infolen)
>> + dma_unmap_single(nfc->dev, nfc->iaddr, infolen, dir);
>> +}
>> +
>> +static int meson_nfc_read_buf(struct nand_chip *nand, u8 *buf, int len)
>> +{
>> + struct meson_nfc *nfc = nand_get_controller_data(nand);
>> + int ret = 0;
>> + u32 cmd;
>> + u8 *info;
>> +
>> + info = kzalloc(PER_INFO_BYTE, GFP_KERNEL);
>> + ret = meson_nfc_dma_buffer_setup(nand, buf, len, info,
>> + PER_INFO_BYTE, DMA_FROM_DEVICE);
>> + if (ret)
>> + return ret;
>> +
>> + cmd = NFC_CMD_N2M | (len & 0x3fff);
>> + writel(cmd, nfc->reg_base + NFC_REG_CMD);
>> +
>> + meson_nfc_drain_cmd(nfc);
>> + meson_nfc_wait_cmd_finish(nfc, 1000);
>> + meson_nfc_dma_buffer_release(nand, len, PER_INFO_BYTE, DMA_FROM_DEVICE);
>> + kfree(info);
>> +
>> + return ret;
>> +}
>> +
>> +static int meson_nfc_write_buf(struct nand_chip *nand, u8 *buf, int len)
>> +{
>> + struct meson_nfc *nfc = nand_get_controller_data(nand);
>> + int ret = 0;
>> + u32 cmd;
>> +
>> + ret = meson_nfc_dma_buffer_setup(nand, buf, len, NULL,
>> + 0, DMA_TO_DEVICE);
>> + if (ret)
>> + return ret;
>> +
>> + cmd = NFC_CMD_M2N | (len & 0x3fff);
>> + writel(cmd, nfc->reg_base + NFC_REG_CMD);
>> +
>> + meson_nfc_drain_cmd(nfc);
>> + meson_nfc_wait_cmd_finish(nfc, 1000);
>> + meson_nfc_dma_buffer_release(nand, len, 0, DMA_TO_DEVICE);
>> +
>> + return ret;
>> +}
>> +
>> +static int meson_nfc_rw_cmd_prepare_and_execute(struct nand_chip *nand,
>> + int page, bool in)
>> +{
>> + struct mtd_info *mtd = nand_to_mtd(nand);
>> + struct meson_nfc *nfc = nand_get_controller_data(nand);
>> + const struct nand_sdr_timings *sdr =
>> + nand_get_sdr_timings(&nand->data_interface);
>> + u32 *addrs = nfc->cmdfifo.rw.addrs;
>> + u32 cs = nfc->param.chip_select;
>> + u32 cmd0, cmd_num, row_start;
>> + int ret = 0, i;
>> +
>> + cmd_num = sizeof(struct nand_rw_cmd) / sizeof(int);
>> +
>> + cmd0 = in ? NAND_CMD_READ0 : NAND_CMD_SEQIN;
>> + nfc->cmdfifo.rw.cmd0 = cs | NFC_CMD_CLE | cmd0;
>> +
>> + addrs[0] = cs | NFC_CMD_ALE | 0;
>> + if (mtd->writesize <= 512) {
>> + cmd_num--;
>> + row_start = 1;
>> + } else {
>> + addrs[1] = cs | NFC_CMD_ALE | 0;
>> + row_start = 2;
>> + }
>> +
>> + addrs[row_start] = cs | NFC_CMD_ALE | ROW_ADDER(page, 0);
>> + addrs[row_start + 1] = cs | NFC_CMD_ALE | ROW_ADDER(page, 1);
>> +
>> + if (nand->options & NAND_ROW_ADDR_3)
>> + addrs[row_start + 2] =
>> + cs | NFC_CMD_ALE | ROW_ADDER(page, 2);
>> + else
>> + cmd_num--;
>> +
>> + /* subtract cmd1 */
>> + cmd_num--;
>> +
>> + for (i = 0; i < cmd_num; i++)
>> + writel_relaxed(nfc->cmdfifo.cmd[i],
>> + nfc->reg_base + NFC_REG_CMD);
>> +
>> + if (in) {
>> + nfc->cmdfifo.rw.cmd1 = cs | NFC_CMD_CLE | NAND_CMD_READSTART;
>> + writel(nfc->cmdfifo.rw.cmd1, nfc->reg_base + NFC_REG_CMD);
>> + meson_nfc_queue_rb(nfc, PSEC_TO_MSEC(sdr->tR_max));
>> + } else {
>> + meson_nfc_cmd_idle(nfc, nfc->timing.tadl);
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static int meson_nfc_write_page_sub(struct nand_chip *nand,
>> + int page, int raw)
>> +{
>> + struct mtd_info *mtd = nand_to_mtd(nand);
>> + const struct nand_sdr_timings *sdr =
>> + nand_get_sdr_timings(&nand->data_interface);
>> + struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
>> + struct meson_nfc *nfc = nand_get_controller_data(nand);
>> + int data_len, info_len;
>> + u32 cmd;
>> + int ret;
>> +
>> + data_len = mtd->writesize + mtd->oobsize;
>> + info_len = nand->ecc.steps * PER_INFO_BYTE;
>> +
>> + ret = meson_nfc_rw_cmd_prepare_and_execute(nand, page, DIRWRITE);
>> + if (ret)
>> + return ret;
>> +
>> + ret = meson_nfc_dma_buffer_setup(nand, meson_chip->data_buf,
>> + data_len, (u8 *)meson_chip->info_buf,
>> + info_len, DMA_TO_DEVICE);
>> + if (ret)
>> + return ret;
>> +
>> + meson_nfc_cmd_seed(nfc, page);
>> + meson_nfc_cmd_access(nand, raw, DIRWRITE);
>> + cmd = nfc->param.chip_select | NFC_CMD_CLE | NAND_CMD_PAGEPROG;
>> + writel(cmd, nfc->reg_base + NFC_REG_CMD);
>> + meson_nfc_queue_rb(nfc, PSEC_TO_MSEC(sdr->tPROG_max));
>> +
>> + meson_nfc_dma_buffer_release(nand, data_len, info_len, DMA_TO_DEVICE);
>> +
>> + return ret;
>> +}
>> +
>> +static int meson_nfc_write_page_raw(struct nand_chip *nand, const u8 *buf,
>> + int oob_required, int page)
>> +{
>> + u8 *oob_buf = nand->oob_poi;
>> +
>> + meson_nfc_set_data_oob(nand, buf, oob_buf);
>> +
>> + return meson_nfc_write_page_sub(nand, page, 1);
>> +}
>> +
>> +static int meson_nfc_write_page_hwecc(struct nand_chip *nand,
>> + const u8 *buf, int oob_required, int page)
>> +{
>> + struct mtd_info *mtd = nand_to_mtd(nand);
>> + struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
>> + u8 *oob_buf = nand->oob_poi;
>> +
>> + memcpy(meson_chip->data_buf, buf, mtd->writesize);
>> + memset(meson_chip->info_buf, 0, nand->ecc.steps * PER_INFO_BYTE);
>> + meson_nfc_set_user_byte(nand, oob_buf);
>> +
>> + return meson_nfc_write_page_sub(nand, page, 0);
>> +}
>> +
>> +static void meson_nfc_check_ecc_pages_valid(struct meson_nfc *nfc,
>> + struct nand_chip *nand, int raw)
>> +{
>> + struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
>> + __le64 *info;
>> + u32 neccpages;
>> + int ret;
>> +
>> + neccpages = raw ? 1 : nand->ecc.steps;
>> + info = &meson_chip->info_buf[neccpages - 1];
>> + do {
>> + usleep_range(10, 15);
>> + /* info is updated by nfc dma engine*/
>> + smp_rmb();
>> + ret = *info & ECC_COMPLETE;
>> + } while (!ret);
>> +}
>> +
>> +static int meson_nfc_read_page_sub(struct nand_chip *nand,
>> + int page, int raw)
>> +{
>> + struct mtd_info *mtd = nand_to_mtd(nand);
>> + struct meson_nfc *nfc = nand_get_controller_data(nand);
>> + struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
>> + int data_len, info_len;
>> + int ret;
>> +
>> + data_len = mtd->writesize + mtd->oobsize;
>> + info_len = nand->ecc.steps * PER_INFO_BYTE;
>> +
>> + ret = meson_nfc_rw_cmd_prepare_and_execute(nand, page, DIRREAD);
>> + if (ret)
>> + return ret;
>> +
>> + ret = meson_nfc_dma_buffer_setup(nand, meson_chip->data_buf,
>> + data_len, (u8 *)meson_chip->info_buf,
>> + info_len, DMA_FROM_DEVICE);
>> + if (ret)
>> + return ret;
>> +
>> + meson_nfc_cmd_seed(nfc, page);
>> + meson_nfc_cmd_access(nand, raw, DIRREAD);
>> + ret = meson_nfc_wait_dma_finish(nfc);
>> + meson_nfc_check_ecc_pages_valid(nfc, nand, raw);
>> +
>> + meson_nfc_dma_buffer_release(nand, data_len, info_len, DMA_FROM_DEVICE);
>> +
>> + return ret;
>> +}
>> +
>> +static int meson_nfc_read_page_raw(struct nand_chip *nand, u8 *buf,
>> + int oob_required, int page)
>> +{
>> + u8 *oob_buf = nand->oob_poi;
>> + int ret;
>> +
>> + ret = meson_nfc_read_page_sub(nand, page, 1);
>> + if (ret)
>> + return ret;
>> +
>> + meson_nfc_get_data_oob(nand, buf, oob_buf);
>> +
>> + return 0;
>> +}
>> +
>> +static int meson_nfc_read_page_hwecc(struct nand_chip *nand, u8 *buf,
>> + int oob_required, int page)
>> +{
>> + struct mtd_info *mtd = nand_to_mtd(nand);
>> + struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
>> + u8 *oob_buf = nand->oob_poi;
>> + int ret;
>> +
>> + ret = meson_nfc_read_page_sub(nand, page, 0);
>> + if (ret)
>> + return ret;
>> +
>> + meson_nfc_get_user_byte(nand, oob_buf);
>> +
>> + ret = meson_nfc_ecc_correct(nand);
>> + if (ret == ECC_CHECK_RETURN_FF) {
>> + if (buf)
>> + memset(buf, 0xff, mtd->writesize);
>> +
>> + memset(oob_buf, 0xff, mtd->oobsize);
>> + return 0;
>> + }
>> +
>> + if (buf && buf != meson_chip->data_buf)
>> + memcpy(buf, meson_chip->data_buf, mtd->writesize);
>> +
>> + return ret;
>> +}
>> +
>> +static int meson_nfc_read_oob_raw(struct nand_chip *nand, int page)
>> +{
>> + return meson_nfc_read_page_raw(nand, NULL, 1, page);
>> +}
>> +
>> +static int meson_nfc_read_oob(struct nand_chip *nand, int page)
>> +{
>> + return meson_nfc_read_page_hwecc(nand, NULL, 1, page);
>> +}
>> +
>> +void *
>> +meson_nand_op_get_dma_safe_input_buf(const struct nand_op_instr *instr)
>> +{
>> + if (WARN_ON(instr->type != NAND_OP_DATA_IN_INSTR))
>> + return NULL;
>> + if (virt_addr_valid(instr->ctx.data.buf.in) &&
>> + !object_is_on_stack(instr->ctx.data.buf.in))
>> + return instr->ctx.data.buf.in;
>> +
>> + return kzalloc(instr->ctx.data.len, GFP_KERNEL);
>
> I think allocating memory and using it without ever testing the
> allocation succeeded is wrong. You do that in many places. I would like
> to see allocations properly handled.
>
ok, i will fix it.
>> +}
>> +
>> +void
>> +meson_nand_op_put_dma_safe_input_buf(const struct nand_op_instr *instr,
>> + void *buf)
>> +{
>> + if (WARN_ON(instr->type != NAND_OP_DATA_IN_INSTR) ||
>> + WARN_ON(!buf))
>> + return;
>> + if (buf == instr->ctx.data.buf.in)
>> + return;
>> +
>> + memcpy(instr->ctx.data.buf.in, buf, instr->ctx.data.len);
>> + kfree(buf);
>> +}
>> +
>> +const void *
>> +meson_nand_op_get_dma_safe_output_buf(const struct nand_op_instr *instr)
>> +{
>> + if (WARN_ON(instr->type != NAND_OP_DATA_OUT_INSTR))
>> + return NULL;
>> +
>> + if (virt_addr_valid(instr->ctx.data.buf.out) &&
>> + !object_is_on_stack(instr->ctx.data.buf.out))
>
> Can you please create helpers for that? I guess it will help removing
> these checks once the core will have a DMA-safe approach.
>
I will use below definition:
#define BUFFER_IS_DMA_SAFE(x) \
(virt_addr_valid((x)) && (!object_is_on_stack((x))))
Is it ok?
>> + return instr->ctx.data.buf.out;
>> +
>> + return kmemdup(instr->ctx.data.buf.out,
>> + instr->ctx.data.len, GFP_KERNEL);
>> +}
>> +
>> +void
>> +meson_nand_op_put_dma_safe_output_buf(const struct nand_op_instr *instr,
>> + const void *buf)
>> +{
>> + if (WARN_ON(instr->type != NAND_OP_DATA_OUT_INSTR) ||
>> + WARN_ON(!buf))
>> + return;
>> +
>> + if (buf != instr->ctx.data.buf.out)
>> + kfree(buf);
>> +}
>> +
>> +static int meson_nfc_exec_op(struct nand_chip *nand,
>> + const struct nand_operation *op, bool check_only)
>> +{
>> + struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
>> + struct meson_nfc *nfc = nand_get_controller_data(nand);
>> + const struct nand_op_instr *instr = NULL;
>> + void *buf;
>> + u32 op_id, delay_idle, cmd;
>> + int i;
>> +
>> + for (op_id = 0; op_id < op->ninstrs; op_id++) {
>> + instr = &op->instrs[op_id];
>> + delay_idle = DIV_ROUND_UP(PSEC_TO_NSEC(instr->delay_ns),
>> + meson_chip->level1_divider *
>> + NFC_CLK_CYCLE);
>> + switch (instr->type) {
>> + case NAND_OP_CMD_INSTR:
>> + cmd = nfc->param.chip_select | NFC_CMD_CLE;
>> + cmd |= instr->ctx.cmd.opcode & 0xff;
>> + writel(cmd, nfc->reg_base + NFC_REG_CMD);
>> + meson_nfc_cmd_idle(nfc, delay_idle);
>> + break;
>> +
>> + case NAND_OP_ADDR_INSTR:
>> + for (i = 0; i < instr->ctx.addr.naddrs; i++) {
>> + cmd = nfc->param.chip_select | NFC_CMD_ALE;
>> + cmd |= instr->ctx.addr.addrs[i] & 0xff;
>> + writel(cmd, nfc->reg_base + NFC_REG_CMD);
>> + }
>> + meson_nfc_cmd_idle(nfc, delay_idle);
>> + break;
>> +
>> + case NAND_OP_DATA_IN_INSTR:
>> + buf = meson_nand_op_get_dma_safe_input_buf(instr);
>> + meson_nfc_read_buf(nand, buf,
>> + instr->ctx.data.len);
>> + meson_nand_op_put_dma_safe_input_buf(instr, buf);
>> + break;
>> +
>> + case NAND_OP_DATA_OUT_INSTR:
>> + buf =
>> + (void *)meson_nand_op_get_dma_safe_output_buf(instr);
>> + meson_nfc_write_buf(nand, buf,
>> + instr->ctx.data.len);
>> + meson_nand_op_put_dma_safe_output_buf(instr, buf);
>> + break;
>> +
>> + case NAND_OP_WAITRDY_INSTR:
>> + meson_nfc_queue_rb(nfc, instr->ctx.waitrdy.timeout_ms);
>> + if (instr->delay_ns)
>> + meson_nfc_cmd_idle(nfc, delay_idle);
>> + break;
>> + }
>> + }
>> + meson_nfc_wait_cmd_finish(nfc, 1000);
>> + return 0;
>> +}
>> +
>> +static int meson_ooblayout_ecc(struct mtd_info *mtd, int section,
>> + struct mtd_oob_region *oobregion)
>> +{
>> + struct nand_chip *nand = mtd_to_nand(mtd);
>> +
>> + if (section >= nand->ecc.steps)
>> + return -ERANGE;
>> +
>> + oobregion->offset = 2 + (section * (2 + nand->ecc.bytes));
>> + oobregion->length = nand->ecc.bytes;
>> +
>> + return 0;
>> +}
>> +
>> +static int meson_ooblayout_free(struct mtd_info *mtd, int section,
>> + struct mtd_oob_region *oobregion)
>> +{
>> + struct nand_chip *nand = mtd_to_nand(mtd);
>> +
>> + if (section >= nand->ecc.steps)
>> + return -ERANGE;
>> +
>> + oobregion->offset = section * (2 + nand->ecc.bytes);
>> + oobregion->length = 2;
>> +
>> + return 0;
>> +}
>> +
>> +static const struct mtd_ooblayout_ops meson_ooblayout_ops = {
>> + .ecc = meson_ooblayout_ecc,
>> + .free = meson_ooblayout_free,
>> +};
>> +
>> +static int meson_nfc_clk_init(struct meson_nfc *nfc)
>> +{
>> + int ret;
>> +
>> + /* request core clock */
>> + nfc->core_clk = devm_clk_get(nfc->dev, "core");
>> + if (IS_ERR(nfc->core_clk)) {
>> + dev_err(nfc->dev, "failed to get core clk\n");
>> + return PTR_ERR(nfc->core_clk);
>> + }
>> +
>> + nfc->device_clk = devm_clk_get(nfc->dev, "device");
>> + if (IS_ERR(nfc->device_clk)) {
>> + dev_err(nfc->dev, "failed to get device clk\n");
>> + return PTR_ERR(nfc->device_clk);
>> + }
>> +
>> + nfc->phase_tx = devm_clk_get(nfc->dev, "tx");
>> + if (IS_ERR(nfc->phase_tx)) {
>> + dev_err(nfc->dev, "failed to get tx clk\n");
>> + return PTR_ERR(nfc->phase_tx);
>> + }
>> +
>> + nfc->phase_rx = devm_clk_get(nfc->dev, "rx");
>> + if (IS_ERR(nfc->phase_rx)) {
>> + dev_err(nfc->dev, "failed to get rx clk\n");
>> + return PTR_ERR(nfc->phase_rx);
>> + }
>> +
>> + /* init SD_EMMC_CLOCK to sane defaults w/min clock rate */
>> + regmap_update_bits(nfc->reg_clk,
>> + 0, CLK_SELECT_NAND, CLK_SELECT_NAND);
>> +
>> + ret = clk_prepare_enable(nfc->core_clk);
>> + if (ret) {
>> + dev_err(nfc->dev, "failed to enable core clk\n");
>> + return ret;
>> + }
>> +
>> + ret = clk_prepare_enable(nfc->device_clk);
>> + if (ret) {
>> + dev_err(nfc->dev, "failed to enable device clk\n");
>> + clk_disable_unprepare(nfc->core_clk);
>> + return ret;
>> + }
>> +
>> + ret = clk_prepare_enable(nfc->phase_tx);
>> + if (ret) {
>> + dev_err(nfc->dev, "failed to enable tx clk\n");
>> + clk_disable_unprepare(nfc->core_clk);
>> + clk_disable_unprepare(nfc->device_clk);
>> + return ret;
>> + }
>> +
>> + ret = clk_prepare_enable(nfc->phase_rx);
>> + if (ret) {
>> + dev_err(nfc->dev, "failed to enable rx clk\n");
>> + clk_disable_unprepare(nfc->core_clk);
>> + clk_disable_unprepare(nfc->device_clk);
>> + clk_disable_unprepare(nfc->phase_tx);
>
> This error case is a good candidate to a goto statement.
>
ok
>> + return ret;
>> + }
>> +
>> + ret = clk_set_rate(nfc->device_clk, 24000000);
>> + if (ret)
>> + return ret;
>> +
>> + return 0;
>> +}
>> +
>> +static void meson_nfc_disable_clk(struct meson_nfc *nfc)
>> +{
>> + clk_disable_unprepare(nfc->phase_rx);
>> + clk_disable_unprepare(nfc->phase_tx);
>> + clk_disable_unprepare(nfc->device_clk);
>> + clk_disable_unprepare(nfc->core_clk);
>> +}
>> +
>> +static void meson_nfc_free_buffer(struct nand_chip *nand)
>> +{
>> + struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
>> +
>> + kfree(meson_chip->info_buf);
>> + kfree(meson_chip->data_buf);
>> +}
>> +
>> +static int meson_chip_buffer_init(struct nand_chip *nand)
>> +{
>> + struct mtd_info *mtd = nand_to_mtd(nand);
>> + struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
>> + u32 page_bytes, info_bytes, nsectors;
>> +
>> + nsectors = mtd->writesize / nand->ecc.size;
>> +
>> + page_bytes = mtd->writesize + mtd->oobsize;
>> + info_bytes = nsectors * PER_INFO_BYTE;
>> +
>> + meson_chip->data_buf = kmalloc(page_bytes, GFP_KERNEL);
>> + if (!meson_chip->data_buf)
>> + return -ENOMEM;
>> +
>> + meson_chip->info_buf = kmalloc(info_bytes, GFP_KERNEL);
>> + if (!meson_chip->info_buf) {
>> + kfree(meson_chip->data_buf);
>> + return -ENOMEM;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static
>> +int meson_nfc_setup_data_interface(struct nand_chip *nand, int csline,
>> + const struct nand_data_interface *conf)
>> +{
>> + struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
>> + const struct nand_sdr_timings *timings;
>> + u32 div, bt_min, bt_max, tbers_clocks;
>> +
>> + timings = nand_get_sdr_timings(conf);
>> + if (IS_ERR(timings))
>> + return -ENOTSUPP;
>> +
>> + if (csline == NAND_DATA_IFACE_CHECK_ONLY)
>> + return 0;
>> +
>> + div = DIV_ROUND_UP((timings->tRC_min / 1000), NFC_CLK_CYCLE);
>> + bt_min = (timings->tREA_max + NFC_DEFAULT_DELAY) / div;
>> + bt_max = (NFC_DEFAULT_DELAY + timings->tRHOH_min +
>> + timings->tRC_min / 2) / div;
>> +
>> + meson_chip->twb = DIV_ROUND_UP(PSEC_TO_NSEC(timings->tWB_max),
>> + div * NFC_CLK_CYCLE);
>> + meson_chip->tadl = DIV_ROUND_UP(PSEC_TO_NSEC(timings->tADL_min),
>> + div * NFC_CLK_CYCLE);
>> + tbers_clocks = DIV_ROUND_UP(PSEC_TO_NSEC(timings->tBERS_max),
>> + div * NFC_CLK_CYCLE);
>> + meson_chip->tbers_max = ilog2(tbers_clocks);
>> + if (!is_power_of_2(tbers_clocks))
>> + meson_chip->tbers_max++;
>> +
>> + bt_min = DIV_ROUND_UP(bt_min, 1000);
>> + bt_max = DIV_ROUND_UP(bt_max, 1000);
>> +
>> + if (bt_max < bt_min)
>> + return -EINVAL;
>> +
>> + meson_chip->level1_divider = div;
>> + meson_chip->clk_rate = 1000000000 / meson_chip->level1_divider;
>> + meson_chip->bus_timing = (bt_min + bt_max) / 2 + 1;
>> +
>> + return 0;
>> +}
>> +
>> +static int meson_nand_bch_mode(struct nand_chip *nand)
>> +{
>> + struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
>> + struct meson_nand_ecc meson_ecc[] = {
>> + MESON_ECC_DATA(NFC_ECC_BCH8_1K, 8),
>> + MESON_ECC_DATA(NFC_ECC_BCH24_1K, 24),
>> + MESON_ECC_DATA(NFC_ECC_BCH30_1K, 30),
>> + MESON_ECC_DATA(NFC_ECC_BCH40_1K, 40),
>> + MESON_ECC_DATA(NFC_ECC_BCH50_1K, 50),
>> + MESON_ECC_DATA(NFC_ECC_BCH60_1K, 60),
>> + };
>
> Maybe this array could be static?
>
ok
>> + int i;
>> +
>> + if (nand->ecc.strength > 60 || nand->ecc.strength < 8)
>> + return -EINVAL;
>> +
>> + for (i = 0; i < sizeof(meson_ecc); i++) {
>> + if (meson_ecc[i].strength == nand->ecc.strength) {
>> + meson_chip->bch_mode = meson_ecc[i].bch;
>> + return 0;
>> + }
>> + }
>> +
>> + return -EINVAL;
>> +}
>> +
>> +static int meson_nand_attach_chip(struct nand_chip *nand)
>> +{
>> + struct meson_nfc *nfc = nand_get_controller_data(nand);
>> + struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
>> + struct mtd_info *mtd = nand_to_mtd(nand);
>> + int nsectors = mtd->writesize / 1024;
>> + int ret;
>> +
>> + if (!mtd->name) {
>> + mtd->name = devm_kasprintf(nfc->dev, GFP_KERNEL,
>> + "%s:nand%d",
>> + dev_name(nfc->dev),
>> + meson_chip->sels[0]);
>> + if (!mtd->name)
>> + return -ENOMEM;
>> + }
>> +
>> + if (nand->bbt_options & NAND_BBT_USE_FLASH)
>> + nand->bbt_options |= NAND_BBT_NO_OOB;
>> +
>> + nand->options |= NAND_NO_SUBPAGE_WRITE;
>> +
>> + ret = nand_ecc_choose_conf(nand, nfc->data->ecc_caps,
>> + mtd->oobsize - 2 * nsectors);
>> + if (ret) {
>> + dev_err(nfc->dev, "failed to ecc init\n");
>> + return -EINVAL;
>> + }
>> +
>> + ret = meson_nand_bch_mode(nand);
>> + if (ret)
>> + return -EINVAL;
>> +
>> + nand->ecc.mode = NAND_ECC_HW;
>> + nand->ecc.write_page_raw = meson_nfc_write_page_raw;
>> + nand->ecc.write_page = meson_nfc_write_page_hwecc;
>> + nand->ecc.write_oob_raw = nand_write_oob_std;
>> + nand->ecc.write_oob = nand_write_oob_std;
>> +
>> + nand->ecc.read_page_raw = meson_nfc_read_page_raw;
>> + nand->ecc.read_page = meson_nfc_read_page_hwecc;
>> + nand->ecc.read_oob_raw = meson_nfc_read_oob_raw;
>> + nand->ecc.read_oob = meson_nfc_read_oob;
>> +
>> + if (nand->options & NAND_BUSWIDTH_16) {
>> + dev_err(nfc->dev, "16bits buswidth not supported");
>> + return -EINVAL;
>> + }
>> + meson_chip_buffer_init(nand);
>> + if (ret)
>> + return -ENOMEM;
>> +
>> + return ret;
>> +}
>> +
>> +static const struct nand_controller_ops meson_nand_controller_ops = {
>> + .attach_chip = meson_nand_attach_chip,
>
> Don't you need a ->detach_chip hook to free data_buf/info_buf
> buffers?
>
ok, i will add it.
>> +};
>> +
>> +static int
>> +meson_nfc_nand_chip_init(struct device *dev,
>> + struct meson_nfc *nfc, struct device_node *np)
>> +{
>> + struct meson_nfc_nand_chip *meson_chip;
>> + struct nand_chip *nand;
>> + struct mtd_info *mtd;
>> + int ret, i;
>> + u32 tmp, nsels;
>> +
>> + if (!of_get_property(np, "reg", &nsels))
>> + return -EINVAL;
>> +
>> + nsels /= sizeof(u32);
>> + if (!nsels || nsels > MAX_CE_NUM) {
>> + dev_err(dev, "invalid reg property size\n");
>> + return -EINVAL;
>> + }
>> +
>> + meson_chip = devm_kzalloc(dev,
>> + sizeof(*meson_chip) + (nsels * sizeof(u8)),
>> + GFP_KERNEL);
>> + if (!meson_chip)
>> + return -ENOMEM;
>> +
>> + meson_chip->nsels = nsels;
>> +
>> + for (i = 0; i < nsels; i++) {
>> + ret = of_property_read_u32_index(np, "reg", i, &tmp);
>> + if (ret) {
>> + dev_err(dev, "could not retrieve reg property: %d\n",
>> + ret);
>> + return ret;
>> + }
>> +
>> + if (test_and_set_bit(tmp, &nfc->assigned_cs)) {
>> + dev_err(dev, "CS %d already assigned\n", tmp);
>> + return -EINVAL;
>> + }
>> + }
>> +
>> + nand = &meson_chip->nand;
>> + nand->controller = &nfc->controller;
>> + nand->controller->ops = &meson_nand_controller_ops;
>> + nand_set_flash_node(nand, np);
>> + nand_set_controller_data(nand, nfc);
>> +
>> + nand->options |= NAND_USE_BOUNCE_BUFFER;
>> + nand->select_chip = meson_nfc_select_chip;
>> + nand->exec_op = meson_nfc_exec_op;
>> + nand->setup_data_interface = meson_nfc_setup_data_interface;
>> + mtd = nand_to_mtd(nand);
>> + mtd->owner = THIS_MODULE;
>> + mtd->dev.parent = dev;
>> +
>> + ret = nand_scan(nand, nsels);
>> + if (ret)
>> + return ret;
>> +
>> + ret = mtd_device_register(mtd, NULL, 0);
>> + if (ret) {
>> + dev_err(dev, "failed to register mtd device: %d\n", ret);
>> + nand_cleanup(nand);
>> + return ret;
>> + }
>> +
>> + list_add_tail(&meson_chip->node, &nfc->chips);
>> +
>> + return 0;
>> +}
>> +
>> +static int meson_nfc_nand_chip_cleanup(struct meson_nfc *nfc)
>> +{
>> + struct meson_nfc_nand_chip *meson_chip;
>> + struct mtd_info *mtd;
>> + int ret;
>> +
>> + while (!list_empty(&nfc->chips)) {
>> + meson_chip = list_first_entry(&nfc->chips,
>> + struct meson_nfc_nand_chip, node);
>> + mtd = nand_to_mtd(&meson_chip->nand);
>> + ret = mtd_device_unregister(mtd);
>> + if (ret)
>> + return ret;
>> +
>> + meson_nfc_free_buffer(&meson_chip->nand);
>> + nand_cleanup(&meson_chip->nand);
>> + list_del(&meson_chip->node);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int meson_nfc_nand_chips_init(struct device *dev,
>> + struct meson_nfc *nfc)
>> +{
>> + struct device_node *np = dev->of_node;
>> + struct device_node *nand_np;
>> + int ret;
>> +
>> + for_each_child_of_node(np, nand_np) {
>> + ret = meson_nfc_nand_chip_init(dev, nfc, nand_np);
>> + if (ret) {
>> + meson_nfc_nand_chip_cleanup(nfc);
>> + return ret;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static irqreturn_t meson_nfc_irq(int irq, void *id)
>> +{
>> + struct meson_nfc *nfc = id;
>> + u32 cfg;
>> +
>> + cfg = readl(nfc->reg_base + NFC_REG_CFG);
>> + if (!(cfg & NFC_RB_IRQ_EN))
>> + return IRQ_NONE;
>> +
>> + cfg &= ~(NFC_RB_IRQ_EN);
>> + writel(cfg, nfc->reg_base + NFC_REG_CFG);
>> +
>> + complete(&nfc->completion);
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static const struct meson_nfc_data meson_gxl_data = {
>> + .ecc_caps = &meson_gxl_ecc_caps,
>> +};
>> +
>> +static const struct meson_nfc_data meson_axg_data = {
>> + .ecc_caps = &meson_axg_ecc_caps,
>> +};
>> +
>> +static const struct of_device_id meson_nfc_id_table[] = {
>> + {
>> + .compatible = "amlogic,meson-gxl-nfc",
>> + .data = &meson_gxl_data,
>> + }, {
>> + .compatible = "amlogic,meson-axg-nfc",
>> + .data = &meson_axg_data,
>> + },
>> + {}
>> +};
>> +MODULE_DEVICE_TABLE(of, meson_nfc_id_table);
>> +
>> +static int meson_nfc_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct meson_nfc *nfc;
>> + struct resource *res;
>> + int ret, irq;
>> +
>> + nfc = devm_kzalloc(dev, sizeof(*nfc), GFP_KERNEL);
>> + if (!nfc)
>> + return -ENOMEM;
>> +
>> + nfc->data = of_device_get_match_data(&pdev->dev);
>> + if (!nfc->data)
>> + return -ENODEV;
>> +
>> + nand_controller_init(&nfc->controller);
>> + INIT_LIST_HEAD(&nfc->chips);
>> +
>> + nfc->dev = dev;
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + nfc->reg_base = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(nfc->reg_base))
>> + return PTR_ERR(nfc->reg_base);
>> +
>> + nfc->reg_clk =
>> + syscon_regmap_lookup_by_phandle(dev->of_node,
>> + "amlogic,mmc-syscon");
>> + if (IS_ERR(nfc->reg_clk)) {
>> + dev_err(dev, "Failed to lookup clock base\n");
>> + return PTR_ERR(nfc->reg_clk);
>> + }
>> +
>> + irq = platform_get_irq(pdev, 0);
>> + if (irq < 0) {
>> + dev_err(dev, "no nfi irq resource\n");
>> + return -EINVAL;
>> + }
>> +
>> + ret = meson_nfc_clk_init(nfc);
>> + if (ret) {
>> + dev_err(dev, "failed to initialize nand clk\n");
>> + goto err;
>
> Useless goto, a return would be enough.
>
ok
>> + }
>> +
>> + writel(0, nfc->reg_base + NFC_REG_CFG);
>> + ret = devm_request_irq(dev, irq, meson_nfc_irq, 0, dev_name(dev), nfc);
>> + if (ret) {
>> + dev_err(dev, "failed to request nfi irq\n");
>> + ret = -EINVAL;
>> + goto err_clk;
>> + }
>> +
>> + ret = dma_set_mask(dev, DMA_BIT_MASK(32));
>> + if (ret) {
>> + dev_err(dev, "failed to set dma mask\n");
>
> Nit: I prefer when acronyms are upper case in plain English (DMA, NAND,
> IRQ, etc).
>
ok, i will fix it.
>> + goto err_clk;
>> + }
>> +
>> + platform_set_drvdata(pdev, nfc);
>> +
>> + ret = meson_nfc_nand_chips_init(dev, nfc);
>> + if (ret) {
>> + dev_err(dev, "failed to init nand chips\n");
>> + goto err_clk;
>> + }
>> +
>> + return 0;
>> +
>> +err_clk:
>> + meson_nfc_disable_clk(nfc);
>> +err:
>
> This goto can be removed.
>
ok
>> + return ret;
>> +}
>> +
>> +static int meson_nfc_remove(struct platform_device *pdev)
>> +{
>> + struct meson_nfc *nfc = platform_get_drvdata(pdev);
>> + int ret;
>> +
>> + ret = meson_nfc_nand_chip_cleanup(nfc);
>> + if (ret)
>> + return ret;
>> +
>> + meson_nfc_disable_clk(nfc);
>> +
>> + platform_set_drvdata(pdev, NULL);
>> +
>> + return 0;
>> +}
>> +
>> +static struct platform_driver meson_nfc_driver = {
>> + .probe = meson_nfc_probe,
>> + .remove = meson_nfc_remove,
>> + .driver = {
>> + .name = "meson-nand",
>> + .of_match_table = meson_nfc_id_table,
>> + },
>> +};
>> +module_platform_driver(meson_nfc_driver);
>> +
>> +MODULE_LICENSE("Dual MIT/GPL");
>> +MODULE_AUTHOR("Liang Yang <liang.yang@amlogic.com>");
>> +MODULE_DESCRIPTION("Amlogic's Meson NAND Flash Controller driver");
>
>
>
>
> Thanks,
> Miquèl
>
> .
>
_______________________________________________
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/3] mtd: spi-nor: cadence-quadspi: Add support for Octal SPI controller
From: Vignesh R @ 2018-12-10 11:19 UTC (permalink / raw)
To: Boris Brezillon
Cc: devicetree, Yogesh Gaur, linux-kernel, Marek Vasut, Rob Herring,
linux-mtd, Brian Norris, Linux ARM Mailing List
In-Reply-To: <20181210094513.6282d55e@bbrezillon>
On 10/12/18 2:15 PM, Boris Brezillon wrote:
> On Wed, 3 Oct 2018 22:26:03 +0530
> Vignesh R <vigneshr@ti.com> wrote:
>
>> Cadence OSPI controller IP supports Octal IO (x8 IO lines),
>> It also has an integrated PHY. IP register layout is very
>> similar to existing QSPI IP except for additional bits to support Octal
>> and Octal DDR mode. Therefore, extend current driver to support Octal
>> mode.
>>
>> Signed-off-by: Vignesh R <vigneshr@ti.com>
>> ---
>> drivers/mtd/spi-nor/cadence-quadspi.c | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
>> index e24db817154e..48b00e75a879 100644
>> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
>> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
>> @@ -101,6 +101,7 @@ struct cqspi_st {
>> #define CQSPI_INST_TYPE_SINGLE 0
>> #define CQSPI_INST_TYPE_DUAL 1
>> #define CQSPI_INST_TYPE_QUAD 2
>> +#define CQSPI_INST_TYPE_OCTAL 3
>>
>> #define CQSPI_DUMMY_CLKS_PER_BYTE 8
>> #define CQSPI_DUMMY_BYTES_MAX 4
>> @@ -898,6 +899,9 @@ static int cqspi_set_protocol(struct spi_nor *nor, const int read)
>> case SNOR_PROTO_1_1_4:
>> f_pdata->data_width = CQSPI_INST_TYPE_QUAD;
>> break;
>> + case SNOR_PROTO_1_1_8:
>> + f_pdata->data_width = CQSPI_INST_TYPE_OCTAL;
>> + break;
>> default:
>> return -EINVAL;
>> }
>> @@ -1205,6 +1209,7 @@ static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
>> SNOR_HWCAPS_READ_FAST |
>> SNOR_HWCAPS_READ_1_1_2 |
>> SNOR_HWCAPS_READ_1_1_4 |
>> + SNOR_HWCAPS_READ_1_1_8 |
>
> Is this really supported on qspi versions of this IP? I guess not given
> the description in the commit message and the name of the new
> compatible (ospi instead of qspi).
No, qspi version does not support Octal mode. I guess you are pointing
out its logically wrong for driver with "*-qspi" compatible to declare
SNOR_HWCAPS_READ_1_1_8 capability.
Will update patch to declare SNOR_HWCAPS_READ_1_1_8 based on compatible.
>
>> SNOR_HWCAPS_PP,
>> };
>> struct platform_device *pdev = cqspi->pdev;
>> @@ -1456,6 +1461,10 @@ static const struct of_device_id cqspi_dt_ids[] = {
>> .compatible = "ti,k2g-qspi",
>> .data = (void *)CQSPI_NEEDS_WR_DELAY,
>> },
>> + {
>> + .compatible = "ti,am654-ospi",
>> + .data = (void *)CQSPI_NEEDS_WR_DELAY,
>> + },
>> { /* end of table */ }
>> };
>>
>
--
Regards
Vignesh
_______________________________________________
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 v5 3/7] mtd: spi-nor: add opcodes for octal Read/Write commands
From: Yogesh Narayan Gaur @ 2018-12-10 11:17 UTC (permalink / raw)
To: Boris Brezillon
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, vigneshr@ti.com,
robh@kernel.org, linux-kernel@vger.kernel.org,
linux-spi@vger.kernel.org, marek.vasut@gmail.com,
frieder.schrempf@exceet.de, broonie@kernel.org,
linux-mtd@lists.infradead.org, computersforpeace@gmail.com,
shawnguo@kernel.org, linux-arm-kernel@lists.infradead.org
In-Reply-To: <20181210115717.01ad2e60@bbrezillon>
Hi Boris,
> -----Original Message-----
> From: Boris Brezillon [mailto:boris.brezillon@bootlin.com]
> Sent: Monday, December 10, 2018 4:27 PM
> To: Yogesh Narayan Gaur <yogeshnarayan.gaur@nxp.com>
> Cc: linux-mtd@lists.infradead.org; broonie@kernel.org;
> marek.vasut@gmail.com; vigneshr@ti.com; linux-spi@vger.kernel.org;
> devicetree@vger.kernel.org; robh@kernel.org; mark.rutland@arm.com;
> shawnguo@kernel.org; linux-arm-kernel@lists.infradead.org;
> computersforpeace@gmail.com; frieder.schrempf@exceet.de; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH v5 3/7] mtd: spi-nor: add opcodes for octal Read/Write
> commands
>
> On Mon, 3 Dec 2018 08:39:18 +0000
> Yogesh Narayan Gaur <yogeshnarayan.gaur@nxp.com> wrote:
>
> > - Add opcodes for octal I/O commands
> > * Read : 1-1-8 and 1-8-8 protocol
> > * Write : 1-1-8 and 1-8-8 protocol
> > * opcodes for 4-byte address mode command
> >
> > - Entry of macros in _convert_3to4_xxx function
> >
> > - Add flag specifying flash support octal read commands.
> >
> > Signed-off-by: Vignesh R <vigneshr@ti.com>
> > Signed-off-by: Yogesh Gaur <yogeshnarayan.gaur@nxp.com>
>
> Looks like the SoB and Author lines do not match
>
> Yogesh Narayan Gaur <yogeshnarayan.gaur@nxp.com> vs Yogesh Gaur
> <yogeshnarayan.gaur@nxp.com>
>
> Can you find a way to make them match?
I am sending the patches with my smtp server of OutlookOffice and in that my user name is "Yogesh Narayan Gaur" and in gitconfig of my Linux machine I set user name as "Yogesh Gaur".
Is it mandatory to have same Author name in SoB and Author lines, if yes I would change the settings in gitconfig file.
>
> Thanks,
>
> Boris
_______________________________________________
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 7/8] arm64: KVM: Handle ARM erratum 1165522 in TLB invalidation
From: James Morse @ 2018-12-10 11:15 UTC (permalink / raw)
To: Marc Zyngier, Christoffer Dall
Cc: Mark Rutland, kvm, Suzuki K Poulose, Catalin Marinas, Will Deacon,
kvmarm, linux-arm-kernel
In-Reply-To: <50e6a07d-868b-db43-fd4e-4b6359a5128e@arm.com>
Hi Marc, Christoffer,
On 10/12/2018 10:46, Marc Zyngier wrote:
> On 10/12/2018 10:19, Christoffer Dall wrote:
>> On Thu, Dec 06, 2018 at 05:31:25PM +0000, Marc Zyngier wrote:
>>> In order to avoid TLB corruption whilst invalidating TLBs on CPUs
>>> affected by erratum 1165522, we need to prevent S1 page tables
>>> from being usable.
>>>
>>> For this, we set the EL1 S1 MMU on, and also disable the page table
>>> walker (by setting the TCR_EL1.EPD* bits to 1).
>>>
>>> This ensures that once we switch to the EL1/EL0 translation regime,
>>> speculated AT instructions won't be able to parse the page tables.
>>> @@ -64,11 +93,18 @@ static void __hyp_text __tlb_switch_to_host_vhe(struct kvm *kvm,
>>> write_sysreg(0, vttbr_el2);
>>> write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
>>> isb();
>>> - local_irq_restore(flags);
>>> +
>>> + if (cpus_have_const_cap(ARM64_WORKAROUND_1165522)) {
>>> + /* Restore the guest's registers to what they were */
>>
>> host's ?
>
> Hum... Yes, silly thinko.
I thought these were the guests registers because they are EL1 registers and
this is a VHE-only path.
'interrupted guest' was how I read this. This stuff can get called if memory is
allocated for guest-A while a vcpu is loaded, and reclaims memory from guest-B
causing an mmu-notifier call for stage2. This is why we have to put guest-A's
registers back as we weren't pre-empted, and we expect EL1 to be untouched.
I agree they could belong to no-guest if a vcpu isn't loaded at all... is host
the term used here?
Thanks,
James
_______________________________________________
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 v2 26/34] dt-bindings: arm: Convert Renesas board/soc bindings to json-schema
From: Simon Horman @ 2018-12-10 11:12 UTC (permalink / raw)
To: Rob Herring
Cc: Mark Rutland, devicetree, Kumar Gala, Grant Likely, Sean Hudson,
linuxppc-dev, Magnus Damm, linux-kernel@vger.kernel.org,
open list:MEDIA DRIVERS FOR RENESAS - FCP, ARM-SoC Maintainers,
Geert Uytterhoeven, Frank Rowand,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <CAL_JsqKHwYhHSb7GNFgOtBCqEeMshbRnU+0ONe_yW8OOiRC=SA@mail.gmail.com>
On Thu, Dec 06, 2018 at 01:38:42PM -0600, Rob Herring wrote:
> On Wed, Dec 5, 2018 at 1:44 PM Simon Horman <horms@verge.net.au> wrote:
> >
> > On Tue, Dec 04, 2018 at 09:08:57AM -0600, Rob Herring wrote:
> > > On Tue, Dec 4, 2018 at 8:57 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > On Tue, Dec 4, 2018 at 3:48 PM Simon Horman <horms@verge.net.au> wrote:
> > > > > On Mon, Dec 03, 2018 at 03:32:15PM -0600, Rob Herring wrote:
> > > > > > Convert Renesas SoC bindings to DT schema format using json-schema.
> > > > > >
> > > > > > Cc: Simon Horman <horms@verge.net.au>
> > > > > > Cc: Magnus Damm <magnus.damm@gmail.com>
> > > > > > Cc: Mark Rutland <mark.rutland@arm.com>
> > > > > > Cc: linux-renesas-soc@vger.kernel.org
> > > > > > Cc: devicetree@vger.kernel.org
> > > > > > Signed-off-by: Rob Herring <robh@kernel.org>
> > > > > > ---
> > > > > > .../devicetree/bindings/arm/shmobile.txt | 151 ------------
> > > > > > .../devicetree/bindings/arm/shmobile.yaml | 218 ++++++++++++++++++
> > > > > > 2 files changed, 218 insertions(+), 151 deletions(-)
> > > > > > delete mode 100644 Documentation/devicetree/bindings/arm/shmobile.txt
> > > > > > create mode 100644 Documentation/devicetree/bindings/arm/shmobile.yaml
> > > > >
> > > > > Hi Rob,
> > > > >
> > > > > what is this based on? I get a conflict when applying the .txt change
> > > > > and if I knew the base for this patch it would be rather easy to work
> > > > > out what has changed.
> > >
> > > 4.20-rc2
> > >
> > > > >
> > > > > Also, should we do an s/shmobile.txt/shmobile.yaml/ in MAINTAINERS?
> > >
> > > Yes. Though it was pointed out that get_maintainers.pl can pull emails
> > > out of this file. We'd need to get that to work by default though.
> > >
> > > > Probably even s/shmobile.yaml/renesas.yaml/, while at it?
> > >
> > > Sure, if that's what you all want.
> >
> > How about this?
>
> LGTM
Thanks.
As my tree is already closed for v4.21 I have applied this for v4.22.
_______________________________________________
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 7/7] drm: Split out drm_probe_helper.h
From: Benjamin Gaignard @ 2018-12-10 11:10 UTC (permalink / raw)
To: Thierry Reding
Cc: moderated list:ARM/S5P EXYNOS AR..., linux-tegra, spice-devel,
Daniel Vetter, Intel Graphics Development, etnaviv, amd-gfx,
virtualization, linux-renesas-soc, linux-rockchip, linux-mediatek,
ML dri-devel, linux-arm-msm, nouveau, Daniel Vetter,
linux-amlogic, xen-devel, freedreno, linux-stm32, Linux ARM
In-Reply-To: <20181210102426.GG15154@ulmo>
Le lun. 10 déc. 2018 à 11:24, Thierry Reding
<thierry.reding@gmail.com> a écrit :
>
> On Mon, Dec 10, 2018 at 11:11:33AM +0100, Daniel Vetter wrote:
> > Having the probe helper stuff (which pretty much everyone needs) in
> > the drm_crtc_helper.h file (which atomic drivers should never need) is
> > confusing. Split them out.
> >
> > To make sure I actually achieved the goal here I went through all
> > drivers. And indeed, all atomic drivers are now free of
> > drm_crtc_helper.h includes.
> >
I have difficulties to apply this with git on top of drm-misc-next.
It is because of that I got errors (encoder and connector types not
found) while compiling adv7511_audio.c and exynos_dp.c ?
Benjamin
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: linux-arm-kernel@lists.infradead.org
> > Cc: virtualization@lists.linux-foundation.org
> > Cc: etnaviv@lists.freedesktop.org
> > Cc: linux-samsung-soc@vger.kernel.org
> > Cc: intel-gfx@lists.freedesktop.org
> > Cc: linux-mediatek@lists.infradead.org
> > Cc: linux-amlogic@lists.infradead.org
> > Cc: linux-arm-msm@vger.kernel.org
> > Cc: freedreno@lists.freedesktop.org
> > Cc: nouveau@lists.freedesktop.org
> > Cc: spice-devel@lists.freedesktop.org
> > Cc: amd-gfx@lists.freedesktop.org
> > Cc: linux-renesas-soc@vger.kernel.org
> > Cc: linux-rockchip@lists.infradead.org
> > Cc: linux-stm32@st-md-mailman.stormreply.com
> > Cc: linux-tegra@vger.kernel.org
> > Cc: xen-devel@lists.xen.org
> > ---
> > .../gpu/drm/amd/amdgpu/amdgpu_connectors.c | 2 +-
> > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
> > drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +-
> > drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 1 +
> > .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 2 +-
> > .../amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c | 2 +-
> > .../display/amdgpu_dm/amdgpu_dm_services.c | 2 +-
> > drivers/gpu/drm/arc/arcpgu_crtc.c | 2 +-
> > drivers/gpu/drm/arc/arcpgu_drv.c | 2 +-
> > drivers/gpu/drm/arc/arcpgu_sim.c | 2 +-
> > drivers/gpu/drm/arm/hdlcd_crtc.c | 2 +-
> > drivers/gpu/drm/arm/hdlcd_drv.c | 2 +-
> > drivers/gpu/drm/arm/malidp_crtc.c | 2 +-
> > drivers/gpu/drm/arm/malidp_drv.c | 2 +-
> > drivers/gpu/drm/arm/malidp_mw.c | 2 +-
> > drivers/gpu/drm/armada/armada_510.c | 2 +-
> > drivers/gpu/drm/armada/armada_crtc.c | 2 +-
> > drivers/gpu/drm/armada/armada_drv.c | 2 +-
> > drivers/gpu/drm/armada/armada_fb.c | 2 +-
> > drivers/gpu/drm/ast/ast_drv.c | 1 +
> > drivers/gpu/drm/ast/ast_mode.c | 1 +
> > .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 2 +-
> > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 2 +-
> > drivers/gpu/drm/bochs/bochs_drv.c | 1 +
> > drivers/gpu/drm/bochs/bochs_kms.c | 1 +
> > drivers/gpu/drm/bridge/adv7511/adv7511.h | 2 +-
> > drivers/gpu/drm/bridge/analogix-anx78xx.c | 3 +-
> > .../drm/bridge/analogix/analogix_dp_core.c | 2 +-
> > drivers/gpu/drm/bridge/cdns-dsi.c | 2 +-
> > drivers/gpu/drm/bridge/dumb-vga-dac.c | 2 +-
> > .../bridge/megachips-stdpxxxx-ge-b850v3-fw.c | 2 +-
> > drivers/gpu/drm/bridge/nxp-ptn3460.c | 2 +-
> > drivers/gpu/drm/bridge/panel.c | 2 +-
> > drivers/gpu/drm/bridge/parade-ps8622.c | 2 +-
> > drivers/gpu/drm/bridge/sii902x.c | 2 +-
> > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +-
> > drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 2 +-
> > drivers/gpu/drm/bridge/tc358764.c | 2 +-
> > drivers/gpu/drm/bridge/tc358767.c | 2 +-
> > drivers/gpu/drm/bridge/ti-sn65dsi86.c | 2 +-
> > drivers/gpu/drm/bridge/ti-tfp410.c | 2 +-
> > drivers/gpu/drm/cirrus/cirrus_drv.c | 1 +
> > drivers/gpu/drm/cirrus/cirrus_mode.c | 1 +
> > drivers/gpu/drm/drm_atomic_helper.c | 1 -
> > drivers/gpu/drm/drm_dp_mst_topology.c | 2 +-
> > drivers/gpu/drm/drm_modeset_helper.c | 2 +-
> > drivers/gpu/drm/drm_probe_helper.c | 2 +-
> > drivers/gpu/drm/drm_simple_kms_helper.c | 2 +-
> > drivers/gpu/drm/etnaviv/etnaviv_drv.h | 1 -
> > drivers/gpu/drm/exynos/exynos_dp.c | 2 +-
> > drivers/gpu/drm/exynos/exynos_drm_crtc.c | 2 +-
> > drivers/gpu/drm/exynos/exynos_drm_dpi.c | 2 +-
> > drivers/gpu/drm/exynos/exynos_drm_drv.c | 2 +-
> > drivers/gpu/drm/exynos/exynos_drm_dsi.c | 2 +-
> > drivers/gpu/drm/exynos/exynos_drm_fb.c | 2 +-
> > drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 2 +-
> > drivers/gpu/drm/exynos/exynos_drm_vidi.c | 2 +-
> > drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +-
> > drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 2 +-
> > drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 2 +-
> > drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c | 2 +-
> > drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 2 +-
> > drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c | 2 +-
> > drivers/gpu/drm/gma500/psb_intel_drv.h | 1 +
> > .../gpu/drm/hisilicon/hibmc/hibmc_drm_de.c | 2 +-
> > .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 2 +-
> > .../gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c | 2 +-
> > .../gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 2 +-
> > drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c | 2 +-
> > .../gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 2 +-
> > .../gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 2 +-
> > drivers/gpu/drm/i2c/ch7006_priv.h | 2 +-
> > drivers/gpu/drm/i2c/sil164_drv.c | 2 +-
> > drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
> > drivers/gpu/drm/i915/i915_drv.c | 2 +-
> > drivers/gpu/drm/i915/intel_crt.c | 2 +-
> > drivers/gpu/drm/i915/intel_display.c | 2 +-
> > drivers/gpu/drm/i915/intel_dp.c | 2 +-
> > drivers/gpu/drm/i915/intel_dp_mst.c | 2 +-
> > drivers/gpu/drm/i915/intel_drv.h | 2 +-
> > drivers/gpu/drm/imx/dw_hdmi-imx.c | 2 +-
> > drivers/gpu/drm/imx/imx-drm-core.c | 2 +-
> > drivers/gpu/drm/imx/imx-ldb.c | 2 +-
> > drivers/gpu/drm/imx/imx-tve.c | 2 +-
> > drivers/gpu/drm/imx/ipuv3-crtc.c | 2 +-
> > drivers/gpu/drm/imx/parallel-display.c | 2 +-
> > drivers/gpu/drm/mediatek/mtk_dpi.c | 2 +-
> > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 2 +-
> > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 2 +-
> > drivers/gpu/drm/mediatek/mtk_drm_fb.c | 2 +-
> > drivers/gpu/drm/mediatek/mtk_dsi.c | 2 +-
> > drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +-
> > drivers/gpu/drm/meson/meson_crtc.c | 2 +-
> > drivers/gpu/drm/meson/meson_drv.c | 2 +-
> > drivers/gpu/drm/meson/meson_dw_hdmi.c | 2 +-
> > drivers/gpu/drm/meson/meson_venc_cvbs.c | 2 +-
> > drivers/gpu/drm/mgag200/mgag200_mode.c | 1 +
> > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 2 +-
> > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +-
> > drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 2 +-
> > .../gpu/drm/msm/disp/mdp4/mdp4_dsi_encoder.c | 2 +-
> > .../gpu/drm/msm/disp/mdp4/mdp4_dtv_encoder.c | 2 +-
> > .../gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c | 2 +-
> > .../gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c | 2 +-
> > drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 2 +-
> > drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c | 2 +-
> > drivers/gpu/drm/msm/msm_drv.h | 2 +-
> > drivers/gpu/drm/msm/msm_fb.c | 2 +-
> > drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 2 +-
> > drivers/gpu/drm/mxsfb/mxsfb_drv.c | 2 +-
> > drivers/gpu/drm/mxsfb/mxsfb_out.c | 2 +-
> > drivers/gpu/drm/nouveau/dispnv04/tvnv17.c | 1 +
> > drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +-
> > drivers/gpu/drm/nouveau/nouveau_connector.c | 1 +
> > drivers/gpu/drm/nouveau/nouveau_display.c | 1 +
> > drivers/gpu/drm/omapdrm/omap_connector.c | 2 +-
> > drivers/gpu/drm/omapdrm/omap_crtc.c | 2 +-
> > drivers/gpu/drm/omapdrm/omap_drv.c | 2 +-
> > drivers/gpu/drm/omapdrm/omap_drv.h | 2 +-
> > drivers/gpu/drm/omapdrm/omap_encoder.c | 2 +-
> > drivers/gpu/drm/omapdrm/omap_fb.c | 2 +-
> > drivers/gpu/drm/pl111/pl111_drv.c | 2 +-
> > drivers/gpu/drm/qxl/qxl_display.c | 2 +-
> > drivers/gpu/drm/qxl/qxl_drv.c | 3 +-
> > drivers/gpu/drm/qxl/qxl_fb.c | 2 +-
> > drivers/gpu/drm/qxl/qxl_kms.c | 2 +-
> > drivers/gpu/drm/radeon/radeon_acpi.c | 1 +
> > drivers/gpu/drm/radeon/radeon_connectors.c | 1 +
> > drivers/gpu/drm/radeon/radeon_device.c | 1 +
> > drivers/gpu/drm/radeon/radeon_display.c | 1 +
> > drivers/gpu/drm/radeon/radeon_dp_mst.c | 1 +
> > drivers/gpu/drm/radeon/radeon_drv.c | 1 +
> > drivers/gpu/drm/radeon/radeon_irq_kms.c | 1 +
> > drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 2 +-
> > drivers/gpu/drm/rcar-du/rcar_du_drv.c | 2 +-
> > drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 2 +-
> > drivers/gpu/drm/rcar-du/rcar_du_kms.c | 2 +-
> > drivers/gpu/drm/rcar-du/rcar_du_plane.c | 2 +-
> > drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 2 +-
> > drivers/gpu/drm/rcar-du/rcar_lvds.c | 2 +-
> > .../gpu/drm/rockchip/analogix_dp-rockchip.c | 2 +-
> > drivers/gpu/drm/rockchip/cdn-dp-core.c | 2 +-
> > drivers/gpu/drm/rockchip/cdn-dp-core.h | 2 +-
> > drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 2 +-
> > drivers/gpu/drm/rockchip/inno_hdmi.c | 2 +-
> > drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 2 +-
> > drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 2 +-
> > drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c | 2 +-
> > drivers/gpu/drm/rockchip/rockchip_drm_psr.c | 2 +-
> > drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 2 +-
> > drivers/gpu/drm/rockchip/rockchip_lvds.c | 2 +-
> > drivers/gpu/drm/rockchip/rockchip_rgb.c | 2 +-
> > drivers/gpu/drm/sti/sti_crtc.c | 2 +-
> > drivers/gpu/drm/sti/sti_drv.c | 2 +-
> > drivers/gpu/drm/sti/sti_dvo.c | 2 +-
> > drivers/gpu/drm/sti/sti_hda.c | 2 +-
> > drivers/gpu/drm/sti/sti_hdmi.c | 2 +-
> > drivers/gpu/drm/sti/sti_tvout.c | 2 +-
> > drivers/gpu/drm/stm/drv.c | 2 +-
> > drivers/gpu/drm/stm/ltdc.c | 2 +-
> > drivers/gpu/drm/sun4i/sun4i_backend.c | 2 +-
> > drivers/gpu/drm/sun4i/sun4i_crtc.c | 2 +-
> > drivers/gpu/drm/sun4i/sun4i_drv.c | 2 +-
> > drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 2 +-
> > drivers/gpu/drm/sun4i/sun4i_lvds.c | 2 +-
> > drivers/gpu/drm/sun4i/sun4i_rgb.c | 2 +-
> > drivers/gpu/drm/sun4i/sun4i_tcon.c | 2 +-
> > drivers/gpu/drm/sun4i/sun4i_tv.c | 2 +-
> > drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 2 +-
> > drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 2 +-
> > drivers/gpu/drm/sun4i/sun8i_mixer.c | 2 +-
> > drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 2 +-
> > drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 2 +-
> > drivers/gpu/drm/tegra/drm.h | 2 +-
> > drivers/gpu/drm/tegra/hdmi.c | 2 +-
> > drivers/gpu/drm/tegra/hub.c | 2 +-
> > drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 2 +-
> > drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c | 2 +-
> > drivers/gpu/drm/tve200/tve200_drv.c | 2 +-
> > drivers/gpu/drm/udl/udl_connector.c | 1 +
> > drivers/gpu/drm/udl/udl_drv.c | 1 +
> > drivers/gpu/drm/udl/udl_main.c | 1 +
> > drivers/gpu/drm/vc4/vc4_crtc.c | 2 +-
> > drivers/gpu/drm/vc4/vc4_dpi.c | 2 +-
> > drivers/gpu/drm/vc4/vc4_dsi.c | 2 +-
> > drivers/gpu/drm/vc4/vc4_hdmi.c | 2 +-
> > drivers/gpu/drm/vc4/vc4_kms.c | 2 +-
> > drivers/gpu/drm/vc4/vc4_txp.c | 2 +-
> > drivers/gpu/drm/vc4/vc4_vec.c | 2 +-
> > drivers/gpu/drm/virtio/virtgpu_display.c | 2 +-
> > drivers/gpu/drm/virtio/virtgpu_drv.h | 2 +-
> > drivers/gpu/drm/vkms/vkms_crtc.c | 2 +-
> > drivers/gpu/drm/vkms/vkms_drv.c | 2 +-
> > drivers/gpu/drm/vkms/vkms_output.c | 2 +-
> > drivers/gpu/drm/vmwgfx/vmwgfx_kms.h | 2 +-
> > drivers/gpu/drm/xen/xen_drm_front.c | 2 +-
> > drivers/gpu/drm/xen/xen_drm_front_conn.c | 2 +-
> > drivers/gpu/drm/xen/xen_drm_front_gem.c | 2 +-
> > drivers/gpu/drm/xen/xen_drm_front_kms.c | 2 +-
> > drivers/gpu/drm/zte/zx_drm_drv.c | 2 +-
> > drivers/gpu/drm/zte/zx_hdmi.c | 2 +-
> > drivers/gpu/drm/zte/zx_tvenc.c | 2 +-
> > drivers/gpu/drm/zte/zx_vga.c | 2 +-
> > drivers/gpu/drm/zte/zx_vou.c | 2 +-
> > drivers/staging/vboxvideo/vbox_irq.c | 2 +-
> > drivers/staging/vboxvideo/vbox_mode.c | 2 +-
> > include/drm/drm_crtc_helper.h | 16 ------
> > include/drm/drm_probe_helper.h | 50 +++++++++++++++++++
> > 208 files changed, 256 insertions(+), 200 deletions(-)
> > create mode 100644 include/drm/drm_probe_helper.h
>
> Looks good to me:
>
> Acked-by: Thierry Reding <treding@nvidia.com>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
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 v5 1/5] spi: spi-mem: Add driver for NXP FlexSPI controller
From: Boris Brezillon @ 2018-12-10 11:09 UTC (permalink / raw)
To: Yogesh Narayan Gaur
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, robh@kernel.org,
linux-kernel@vger.kernel.org, Schrempf Frieder,
linux-spi@vger.kernel.org, marek.vasut@gmail.com,
broonie@kernel.org, linux-mtd@lists.infradead.org,
computersforpeace@gmail.com, shawnguo@kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <VI1PR04MB5726F0DDAC84F1AEC20B56DF99A50@VI1PR04MB5726.eurprd04.prod.outlook.com>
On Mon, 10 Dec 2018 10:59:54 +0000
Yogesh Narayan Gaur <yogeshnarayan.gaur@nxp.com> wrote:
> Hi Boris,
>
> > -----Original Message-----
> > From: Boris Brezillon [mailto:boris.brezillon@bootlin.com]
> > Sent: Monday, December 10, 2018 4:20 PM
> > To: Yogesh Narayan Gaur <yogeshnarayan.gaur@nxp.com>
> > Cc: Schrempf Frieder <frieder.schrempf@kontron.de>; linux-
> > mtd@lists.infradead.org; marek.vasut@gmail.com; broonie@kernel.org; linux-
> > spi@vger.kernel.org; devicetree@vger.kernel.org; robh@kernel.org;
> > mark.rutland@arm.com; shawnguo@kernel.org; linux-arm-
> > kernel@lists.infradead.org; computersforpeace@gmail.com; linux-
> > kernel@vger.kernel.org
> > Subject: Re: [PATCH v5 1/5] spi: spi-mem: Add driver for NXP FlexSPI controller
> >
> > On Mon, 10 Dec 2018 10:43:56 +0000
> > Yogesh Narayan Gaur <yogeshnarayan.gaur@nxp.com> wrote:
> >
> > > > > Thus, in LUT preparation we have assigned only the base address.
> > > > > Now if I have assigned ahb_buf_size to FSPI_FLSHXXCR0 register
> > > > > then for
> > > > read/write data beyond limit of ahb_buf_size offset I get data corruption.
> > > >
> > > > Why would you do that? We have the ->adjust_op_size() exactly for
> > > > this reason, so, if someone tries to do a spi_mem_op with
> > > > data.nbytes > ahb_buf_size you should return an error.
> > > >
> > > Let me explain my implementation with example. If I have to write data of size
> > 0x100 bytes at offset 0x1200 for CS1, I would program as below:
> > > In func nxp_fspi_select_mem(), would set value of controller address space
> > size, memmap_phy_size, to FSPI_FLSHA2CR0 and rest all FSPI_FLSHXXCR0 as 0.
> > > Value of memmap_phy_size is 0x10000000 i.e. 256 MB for my LX2160ARDB
> > target.
> > > Then in nxp_fspi_prepare_lut(), I would prepare LUT ADDR with address length
> > requirement 3/4 byte for NOR or 1/2/3/4 bytes for NAND flash.
> > > Also for LUT_NXP_WRITE would program data bytes as 0.
> > >
> > > Then inside func nxp_fspi_do_op(), set register FSPI_IPCR0 as the
> > > address offset i.e. 0x1200 and in register FSPI_IPCR1 program the data
> > > size to write i.e. 0x100
> > >
> > > If, as suggested if I tries to mark value of register FSPI_FLSHA2CR0 equal to
> > ahb_buf_size (0x800), then access for address 0x1200 gives me wrong data. This
> > is because as per the controller specification access to flash connected at CS1
> > can be performed under range of FSPI_ FLSHA1CR0 and FSPI_ FLSHA2CR0.
> >
> > Don't you have a way to set an offset to apply to the address accessed through
> > the AHB? And if you don't, how will it work if your mapping is smaller than the
> > flash size?
>
> Write operations are triggered using IP commands instead of AHB command.
> For Read AHB command is used and in this we are adding the offset when performing memcpy_fromIO operation
> memcpy_fromio(op->data.buf.in, (f->ahb_addr + op->addr.val), len);
>
> AHB/IP operations are independent of the way how CS got selected. CS selection depends, e.g. CS1 on the value of register FSPI_FLSHA1CR0 and FSPI_FLSHA2CR0.
>
> Mapping can never going to be smaller than the connected flash size as per discussion with the Board design team and if it's possible by user manually changes the non-soldered part then flash area beyond complete mapping is not accessible.
> On LX2160ARDB, with mapping of 256MB, for now we are having 4 flash devices connected with size as 64 MB. If user wants he can have only one single flash with flash size of 256MB.
Given that the dirmap interface has now been merged and the MTD side of
things is soon to be merged, I'd recommend you to implement it in your
v6 and only use non-AHB accesses for the ->exec_op() implementation.
_______________________________________________
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 v5 1/5] spi: spi-mem: Add driver for NXP FlexSPI controller
From: Boris Brezillon @ 2018-12-10 11:03 UTC (permalink / raw)
To: Yogesh Narayan Gaur
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, robh@kernel.org,
linux-kernel@vger.kernel.org, Schrempf Frieder,
linux-spi@vger.kernel.org, marek.vasut@gmail.com,
broonie@kernel.org, linux-mtd@lists.infradead.org,
computersforpeace@gmail.com, shawnguo@kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <VI1PR04MB5726F0DDAC84F1AEC20B56DF99A50@VI1PR04MB5726.eurprd04.prod.outlook.com>
On Mon, 10 Dec 2018 10:59:54 +0000
Yogesh Narayan Gaur <yogeshnarayan.gaur@nxp.com> wrote:
> Hi Boris,
>
> > -----Original Message-----
> > From: Boris Brezillon [mailto:boris.brezillon@bootlin.com]
> > Sent: Monday, December 10, 2018 4:20 PM
> > To: Yogesh Narayan Gaur <yogeshnarayan.gaur@nxp.com>
> > Cc: Schrempf Frieder <frieder.schrempf@kontron.de>; linux-
> > mtd@lists.infradead.org; marek.vasut@gmail.com; broonie@kernel.org; linux-
> > spi@vger.kernel.org; devicetree@vger.kernel.org; robh@kernel.org;
> > mark.rutland@arm.com; shawnguo@kernel.org; linux-arm-
> > kernel@lists.infradead.org; computersforpeace@gmail.com; linux-
> > kernel@vger.kernel.org
> > Subject: Re: [PATCH v5 1/5] spi: spi-mem: Add driver for NXP FlexSPI controller
> >
> > On Mon, 10 Dec 2018 10:43:56 +0000
> > Yogesh Narayan Gaur <yogeshnarayan.gaur@nxp.com> wrote:
> >
> > > > > Thus, in LUT preparation we have assigned only the base address.
> > > > > Now if I have assigned ahb_buf_size to FSPI_FLSHXXCR0 register
> > > > > then for
> > > > read/write data beyond limit of ahb_buf_size offset I get data corruption.
> > > >
> > > > Why would you do that? We have the ->adjust_op_size() exactly for
> > > > this reason, so, if someone tries to do a spi_mem_op with
> > > > data.nbytes > ahb_buf_size you should return an error.
> > > >
> > > Let me explain my implementation with example. If I have to write data of size
> > 0x100 bytes at offset 0x1200 for CS1, I would program as below:
> > > In func nxp_fspi_select_mem(), would set value of controller address space
> > size, memmap_phy_size, to FSPI_FLSHA2CR0 and rest all FSPI_FLSHXXCR0 as 0.
> > > Value of memmap_phy_size is 0x10000000 i.e. 256 MB for my LX2160ARDB
> > target.
> > > Then in nxp_fspi_prepare_lut(), I would prepare LUT ADDR with address length
> > requirement 3/4 byte for NOR or 1/2/3/4 bytes for NAND flash.
> > > Also for LUT_NXP_WRITE would program data bytes as 0.
> > >
> > > Then inside func nxp_fspi_do_op(), set register FSPI_IPCR0 as the
> > > address offset i.e. 0x1200 and in register FSPI_IPCR1 program the data
> > > size to write i.e. 0x100
> > >
> > > If, as suggested if I tries to mark value of register FSPI_FLSHA2CR0 equal to
> > ahb_buf_size (0x800), then access for address 0x1200 gives me wrong data. This
> > is because as per the controller specification access to flash connected at CS1
> > can be performed under range of FSPI_ FLSHA1CR0 and FSPI_ FLSHA2CR0.
> >
> > Don't you have a way to set an offset to apply to the address accessed through
> > the AHB? And if you don't, how will it work if your mapping is smaller than the
> > flash size?
>
> Write operations are triggered using IP commands instead of AHB command.
> For Read AHB command is used and in this we are adding the offset when performing memcpy_fromIO operation
> memcpy_fromio(op->data.buf.in, (f->ahb_addr + op->addr.val), len);
>
> AHB/IP operations are independent of the way how CS got selected. CS selection depends, e.g. CS1 on the value of register FSPI_FLSHA1CR0 and FSPI_FLSHA2CR0.
>
> Mapping can never going to be smaller than the connected flash size as per discussion with the Board design team and if it's possible by user manually changes the non-soldered part then flash area beyond complete mapping is not accessible.
How unfortunate is that, especially when all that was required was an
extra reg to specify a "flash_offset" to apply to the address passed by
the AHB logic.
_______________________________________________
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 v9 1/8] KVM: arm/arm64: Share common code in user_mem_abort()
From: Christoffer Dall @ 2018-12-10 11:01 UTC (permalink / raw)
To: Suzuki K Poulose
Cc: Anshuman Khandual, marc.zyngier, will.deacon, linux-kernel,
punitagrawal, kvmarm, linux-arm-kernel
In-Reply-To: <f6410472-9af7-4b41-3809-3cf691e795e6@arm.com>
On Mon, Dec 10, 2018 at 10:47:42AM +0000, Suzuki K Poulose wrote:
>
>
> On 10/12/2018 08:56, Christoffer Dall wrote:
> >On Mon, Dec 03, 2018 at 01:37:37PM +0000, Suzuki K Poulose wrote:
> >>Hi Anshuman,
> >>
> >>On 03/12/2018 12:11, Anshuman Khandual wrote:
> >>>
> >>>
> >>>On 10/31/2018 11:27 PM, Punit Agrawal wrote:
> >>>>The code for operations such as marking the pfn as dirty, and
> >>>>dcache/icache maintenance during stage 2 fault handling is duplicated
> >>>>between normal pages and PMD hugepages.
> >>>>
> >>>>Instead of creating another copy of the operations when we introduce
> >>>>PUD hugepages, let's share them across the different pagesizes.
> >>>>
> >>>>Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
> >>>>Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> >>>>Cc: Christoffer Dall <christoffer.dall@arm.com>
> >>>>Cc: Marc Zyngier <marc.zyngier@arm.com>
> >>>>---
> >>>> virt/kvm/arm/mmu.c | 49 ++++++++++++++++++++++++++++------------------
> >>>> 1 file changed, 30 insertions(+), 19 deletions(-)
> >>>>
> >>>>diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
> >>>>index 5eca48bdb1a6..59595207c5e1 100644
> >>>>--- a/virt/kvm/arm/mmu.c
> >>>>+++ b/virt/kvm/arm/mmu.c
> >>>>@@ -1475,7 +1475,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
> >>>> unsigned long fault_status)
> >>>> {
> >>>> int ret;
> >>>>- bool write_fault, exec_fault, writable, hugetlb = false, force_pte = false;
> >>>>+ bool write_fault, exec_fault, writable, force_pte = false;
> >>>> unsigned long mmu_seq;
> >>>> gfn_t gfn = fault_ipa >> PAGE_SHIFT;
> >>>> struct kvm *kvm = vcpu->kvm;
> >>>>@@ -1484,7 +1484,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
> >>>> kvm_pfn_t pfn;
> >>>> pgprot_t mem_type = PAGE_S2;
> >>>> bool logging_active = memslot_is_logging(memslot);
> >>>>- unsigned long flags = 0;
> >>>>+ unsigned long vma_pagesize, flags = 0;
> >>>
> >>>A small nit s/vma_pagesize/pagesize. Why call it VMA ? Its implicit.
> >>
> >>May be we could call it mapsize. pagesize is confusing.
> >>
> >
> >I'm ok with mapsize. I see the vma_pagesize name coming from the fact
> >that this is initially set to the return value from vma_kernel_pagesize.
> >
> >I have not problems with either vma_pagesize or mapsize.
> >
> >>>
> >>>> write_fault = kvm_is_write_fault(vcpu);
> >>>> exec_fault = kvm_vcpu_trap_is_iabt(vcpu);
> >>>>@@ -1504,10 +1504,16 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
> >>>> return -EFAULT;
> >>>> }
> >>>>- if (vma_kernel_pagesize(vma) == PMD_SIZE && !logging_active) {
> >>>>- hugetlb = true;
> >>>>+ vma_pagesize = vma_kernel_pagesize(vma);
> >>>>+ if (vma_pagesize == PMD_SIZE && !logging_active) {
> >>>> gfn = (fault_ipa & PMD_MASK) >> PAGE_SHIFT;
> >>>> } else {
> >>>>+ /*
> >>>>+ * Fallback to PTE if it's not one of the Stage 2
> >>>>+ * supported hugepage sizes
> >>>>+ */
> >>>>+ vma_pagesize = PAGE_SIZE;
> >>>
> >>>This seems redundant and should be dropped. vma_kernel_pagesize() here either
> >>>calls hugetlb_vm_op_pagesize (via hugetlb_vm_ops->pagesize) or simply returns
> >>>PAGE_SIZE. The vm_ops path is taken if the QEMU VMA covering any given HVA is
> >>>backed either by HugeTLB pages or simply normal pages. vma_pagesize would
> >>>either has a value of PMD_SIZE (HugeTLB hstate based) or PAGE_SIZE. Hence if
> >>>its not PMD_SIZE it must be PAGE_SIZE which should not be assigned again.
> >>
> >>We may want to force using the PTE mappings when logging_active (e.g, migration
> >>?) to prevent keep tracking of huge pages. So the check is still valid.
> >>
> >>
> >
> >Agreed, and let's not try additionally change the logic and flow with
> >this patch.
> >
> >>>
> >>>>+
> >>>> /*
> >>>> * Pages belonging to memslots that don't have the same
> >>>> * alignment for userspace and IPA cannot be mapped using
> >>>>@@ -1573,23 +1579,33 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
> >>>> if (mmu_notifier_retry(kvm, mmu_seq))
> >>>> goto out_unlock;
> >>>>- if (!hugetlb && !force_pte)
> >>>>- hugetlb = transparent_hugepage_adjust(&pfn, &fault_ipa);
> >>>>+ if (vma_pagesize == PAGE_SIZE && !force_pte) {
> >>>>+ /*
> >>>>+ * Only PMD_SIZE transparent hugepages(THP) are
> >>>>+ * currently supported. This code will need to be
> >>>>+ * updated to support other THP sizes.
> >>>>+ */
> >>>
> >>>This comment belongs to transparent_hugepage_adjust() but not here.
> >>
> >>I think this is relevant here than in thp_adjust, unless we rename
> >>the function below to something generic, handle_hugepage_adjust().
> >>
> >
> >Agreed.
> >
> >>>>+ if (transparent_hugepage_adjust(&pfn, &fault_ipa))
> >>>>+ vma_pagesize = PMD_SIZE;
> >>>
> >>>IIUC transparent_hugepage_adjust() is only getting called here. Instead of
> >>>returning 'true' when it is able to detect a huge page backing and doing
> >>>an adjustment there after, it should rather return THP size (PMD_SIZE) to
> >>>accommodate probable multi size THP support in future .
> >>
> >>That makes sense.
> >>
> >
> >That's fine.
> >
>
> Btw, after a further thought, since we don't have any THP support for anything
> other than PMD_SIZE, I am dropping the above suggestion. We need to make changes
> in our stage2 page table manipulation code anyway to support the new sizes. So
> this could be addressed when we get there, to keep the changes minimal and
> specific to the PUD huge page support.
>
>
Sounds good to me.
Thanks,
Christoffer
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: TK1: DRM, Nouveau and VIC
From: Thierry Reding @ 2018-12-10 11:00 UTC (permalink / raw)
To: Marcel Ziswiler, Ben Skeggs
Cc: nouveau@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
mperttunen@nvidia.com, linux-tegra@vger.kernel.org,
jonathanh@nvidia.com, linux-arm-kernel@lists.infradead.org
In-Reply-To: <20181210102147.GF15154@ulmo>
[-- Attachment #1.1: Type: text/plain, Size: 9797 bytes --]
On Mon, Dec 10, 2018 at 11:21:47AM +0100, Thierry Reding wrote:
> On Sat, Dec 08, 2018 at 02:54:45PM +0000, Marcel Ziswiler wrote:
> > Hi Thierry et al.
> >
> > I noticed that since commit 3dde5a2342cd ("ARM: tegra: Add VIC on
> > Tegra124") graphics on Apalis TK1 is broken. During boot it fails
> > loading the vic firmware:
> >
> > [ 1.595824] tegra-vic 54340000.vic: Direct firmware load for
> > nvidia/tegra124/vic03_ucode.bin failed with error -2
> > [ 1.606140] tegra-vic: probe of 54340000.vic failed with error -2
> >
> > Subsequently Tegra HDMI seems to fail completely:
> >
> > [ 2.379860] tegra-hdmi 54280000.hdmi: failed to get PLL regulator
> >
> > And finally, Nouveau even crashes:
> >
> > [ 8.241115] nouveau 57000000.gpu: Linked as a consumer to
> > regulator.31
> > [ 8.247889] nouveau 57000000.gpu: NVIDIA GK20A (0ea000a1)
> > [ 8.253396] nouveau 57000000.gpu: imem: using IOMMU
> > [ 8.270210] Unable to handle kernel NULL pointer dereference at
> > virtual address 0000006c
> > [ 8.278340] pgd = (ptrval)
> > [ 8.281250] [0000006c] *pgd=00000000
> > [ 8.284944] Internal error: Oops: 5 [#1] PREEMPT SMP ARM
> > [ 8.290260] Modules linked in: nouveau(+) ttm
> > [ 8.294625] CPU: 2 PID: 203 Comm: systemd-udevd Not tainted 4.20.0-
> > rc5-next-20181207-00008-g85b0f8e25f86-dirty #110
> > [ 8.305055] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
> > [ 8.311331] PC is at drm_plane_register_all+0x18/0x50
> > [ 8.316373] LR is at drm_modeset_register_all+0xc/0x70
> > [ 8.321513] pc : [<c056200c>] lr : [<c0564cc8>] psr: a0060013
> > [ 8.327768] sp : ed527c70 ip : ecc43ec0 fp : 00000000
> > [ 8.332993] r10: 00000016 r9 : ecc43e80 r8 : 00000000
> > [ 8.338209] r7 : bf182c80 r6 : 00000000 r5 : ed61b24c r4 :
> > fffffffc
> > [ 8.344735] r3 : 0002f000 r2 : ffffffff r1 : 2e124000 r0 :
> > ed61b000
> > [ 8.351260] Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA
> > ARM Segment none
> > [ 8.358383] Control: 10c5387d Table: ad64c06a DAC: 00000051
> > [ 8.364127] Process systemd-udevd (pid: 203, stack limit =
> > 0x(ptrval))
> > [ 8.370654] Stack: (0xed527c70 to 0xed528000)
> > [ 8.375004] 7c60: ed61b000
> > ed61b000 00000000 c0564cc8
> > [ 8.383177] 7c80: ed61b000 00000000 00000000 c054b5b8 00000001
> > 00000001 ffffffff ffffffff
> > [ 8.391355] 7ca0: ed527cc0 c0f08c48 ed61b000 00000000 00000000
> > 00000000 bf180c5c bf0dc900
> > [ 8.399531] 7cc0: eda29208 5dfe844b 00000000 ee9f2a10 00000000
> > bf180c5c 00000000 c05a9328
> > [ 8.407695] 7ce0: c1006828 ee9f2a10 c100682c 00000000 00000000
> > c05a744c ee9f2a10 bf180c5c
> > [ 8.415871] 7d00: ee9f2a44 c05a77a8 00000000 c0f08c48 bf182980
> > c05a769c eefd14d0 c05a77a8
> > [ 8.424048] 7d20: 00000000 ee9f2a10 bf180c5c ee9f2a44 c05a77a8
> > 00000000 c0f08c48 bf182980
> > [ 8.432226] 7d40: 00000000 c05a7884 ee9ebfb4 c0f08c48 bf180c5c
> > c05a5790 00000000 ee88135c
> > [ 8.440405] 7d60: ee9ebfb4 5dfe844b c0f71168 bf180c5c ee379e80
> > c0f71168 00000000 c05a692c
> > [ 8.448570] 7d80: bf15dc00 bf180ac8 ffffe000 bf180c5c bf180ac8
> > ffffe000 bf1aa000 c05a84a0
> > [ 8.456746] 7da0: bf182b80 bf180ac8 ffffe000 bf1aa170 c0fbd220
> > c0f08c48 ffffe000 c0102ed0
> > [ 8.464924] 7dc0: ed53f4c0 006000c0 c01b3d98 0000000c 60000113
> > bf182980 00000040 c02592d0
> > [ 8.473102] 7de0: eda60200 2e124000 ee800000 006000c0 006000c0
> > c01b3d98 0000000c c025a8cc
> > [ 8.481281] 7e00: c024ce54 a0000113 bf182980 5dfe844b bf182980
> > 00000002 ed53f4c0 00000002
> > [ 8.489459] 7e20: eceba000 c01b3dd4 c0f08c48 bf182980 00000000
> > ed527f40 00000002 eceb9fc0
> > [ 8.497625] 7e40: 00000002 c01b61a4 bf18298c 00007fff bf182980
> > c01b2f88 00000000 c01b279c
> > [ 8.505800] 7e60: bf1829c8 bf182a80 bf182b6c bf182ab0 c0b03ab0
> > c0d58964 c0ca726c c0ca7278
> > [ 8.513978] 7e80: c0ca72d0 c0f08c48 00000000 c02654a0 00000000
> > 00000000 ffffe000 bf000000
> > [ 8.522157] 7ea0: 00000000 00000000 00000000 00000000 00000000
> > 00000000 6e72656b 00006c65
> > [ 8.530336] 7ec0: 00000000 00000000 00000000 00000000 00000000
> > 00000000 00000000 00000000
> > [ 8.538502] 7ee0: 00000000 00000000 00000000 00000000 00000000
> > 5dfe844b 7fffffff c0f08c48
> > [ 8.546677] 7f00: 00000000 0000000f b6f761cc c0101204 ed526000
> > 0000017b 004a3270 c01b66a4
> > [ 8.554855] 7f20: 7fffffff 00000000 00000003 00000001 004a3270
> > f0ced000 06e8994c 00000000
> > [ 8.563032] 7f40: f0e37f3a f0e50a40 f0ced000 06e8994c f7b75f9c
> > f7b75d34 f63e62dc 0016b000
> > [ 8.571209] 7f60: 0017f6f0 00000000 00000000 00000000 00050a48
> > 0000003b 0000003c 00000023
> > [ 8.579388] 7f80: 00000000 00000014 00000000 5dfe844b 00000000
> > 004c0ec0 00000000 00000001
> > [ 8.587554] 7fa0: 0000017b c0101000 004c0ec0 00000000 0000000f
> > b6f761cc 00000000 00020000
> > [ 8.595730] 7fc0: 004c0ec0 00000000 00000001 0000017b 0048e114
> > 00000000 00000000 004a3270
> > [ 8.603908] 7fe0: bea8f990 bea8f980 b6f71269 b6e9f6c0 400d0010
> > 0000000f 00000000 00000000
> > [ 8.612096] [<c056200c>] (drm_plane_register_all) from [<c0564cc8>]
> > (drm_modeset_register_all+0xc/0x70)
> > [ 8.621499] [<c0564cc8>] (drm_modeset_register_all) from
> > [<c054b5b8>] (drm_dev_register+0x168/0x1c4)
> > [ 8.630855] [<c054b5b8>] (drm_dev_register) from [<bf0dc900>]
> > (nouveau_platform_probe+0x6c/0x88 [nouveau])
> > [ 8.640739] [<bf0dc900>] (nouveau_platform_probe [nouveau]) from
> > [<c05a9328>] (platform_drv_probe+0x48/0x98)
> > [ 8.650574] [<c05a9328>] (platform_drv_probe) from [<c05a744c>]
> > (really_probe+0x1e0/0x2cc)
> > [ 8.658827] [<c05a744c>] (really_probe) from [<c05a769c>]
> > (driver_probe_device+0x60/0x16c)
> > [ 8.667096] [<c05a769c>] (driver_probe_device) from [<c05a7884>]
> > (__driver_attach+0xdc/0xe0)
> > [ 8.675543] [<c05a7884>] (__driver_attach) from [<c05a5790>]
> > (bus_for_each_dev+0x74/0xb4)
> > [ 8.683729] [<c05a5790>] (bus_for_each_dev) from [<c05a692c>]
> > (bus_add_driver+0x1c0/0x204)
> > [ 8.692004] [<c05a692c>] (bus_add_driver) from [<c05a84a0>]
> > (driver_register+0x74/0x108)
> > [ 8.700324] [<c05a84a0>] (driver_register) from [<bf1aa170>]
> > (nouveau_drm_init+0x170/0x1000 [nouveau])
> > [ 8.709857] [<bf1aa170>] (nouveau_drm_init [nouveau]) from
> > [<c0102ed0>] (do_one_initcall+0x54/0x284)
> > [ 8.718980] [<c0102ed0>] (do_one_initcall) from [<c01b3dd4>]
> > (do_init_module+0x64/0x214)
> > [ 8.727079] [<c01b3dd4>] (do_init_module) from [<c01b61a4>]
> > (load_module+0x21b8/0x246c)
> > [ 8.735094] [<c01b61a4>] (load_module) from [<c01b66a4>]
> > (sys_finit_module+0xc4/0xdc)
> > [ 8.742937] [<c01b66a4>] (sys_finit_module) from [<c0101000>]
> > (ret_fast_syscall+0x0/0x54)
> > [ 8.751114] Exception stack(0xed527fa8 to 0xed527ff0)
> > [ 8.756157] 7fa0: 004c0ec0 00000000 0000000f
> > b6f761cc 00000000 00020000
> > [ 8.764333] 7fc0: 004c0ec0 00000000 00000001 0000017b 0048e114
> > 00000000 00000000 004a3270
> > [ 8.772510] 7fe0: bea8f990 bea8f980 b6f71269 b6e9f6c0
> > [ 8.777556] Code: e5b5424c e1550004 0a00000c e2444004 (e5943070)
> > [ 8.784011] ---[ end trace ad8c21587c118655 ]---
> >
> > Of course my root file system does include resp. vic firmware:
> >
> > 7ef01d2e3f507c91ca79584e89edcc64 /lib/firmware/nvidia/tegra124/vic03_u
> > code.bin
> >
> > If I bake that one into the kernel binary, Nouveau still crashes like
> > above albeit VIC loading and Tegra DRM now at least showing something
> > on HDMI.
>
> Yeah, this is a fairly common pitfall. The general rule of thumb is that
> the firmware has to live on the same medium as the module. So if you've
> built Tegra DRM as a loadable kernel module and installed it in the root
> filesystem, then that's where your firmware file also needs to be. If
> the driver is built-in (or a loadable module installed in the initial
> ramdisk), then the firmware needs to be in the initial ramdisk (or built
> into the kernel image itself). That's somewhat annoying, but it is what
> it is. At least it's logical.
>
> > Just reverting above mentioned commit still leaves Nouveau crashing.
> >
> > This has been observed using latest next-20181207.
> >
> > Does anybody know what exactly is going on and how exactly one may get
> > graphics working again as before?
>
> So this is something that should be fixed by this:
>
> https://patchwork.freedesktop.org/patch/260547/
>
> And there's another patch that fixes a subsequent crash when you
> actually start to use the GPU:
>
> https://patchwork.freedesktop.org/patch/263588/
>
> It'd be great if you could apply both and verify that they fix the crash
> for you. If so, can you provide a Tested-by? Both were Cc'ed to
> linux-tegra, so you should have a copy to reply to. If not, let me know
> and I can bounce it.
>
> Ben, can you pick up the two patches above? They're kind of high-
> priority because they fix issues that crept into v4.20-rc1, so should
> ideally be fixed before v4.20 final.
Actually, it looks as if only the last patch is needed, since it
superseeds the first. The second one calls drm_mode_config_init() via
nouveau_display_create() and nouveau_drm_device_init(), making the
first patch obsolete.
There's more confirmation here:
https://lists.freedesktop.org/archives/nouveau/2018-December/031636.html
So Ben, correction, please only apply:
https://patchwork.freedesktop.org/patch/263587/
Preferably in time for v4.20 final.
Thanks,
Thierry
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 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
* Re: [PATCH v10 0/8] Introduce on-chip interconnect API
From: Rafael J. Wysocki @ 2018-12-10 11:00 UTC (permalink / raw)
To: Georgi Djakov
Cc: Mark Rutland, Doug Anderson, sanjayc, Rafael J. Wysocki,
maxime.ripard, Michael Turquette, daidavid1, bjorn.andersson,
Saravana Kannan, abailon, Lorenzo Pieralisi, Vincent Guittot,
seansw, Kevin Hilman, evgreen, ksitaraman,
devicetree@vger.kernel.org, Arnd Bergmann, Linux PM,
linux-arm-msm, Rob Herring, linux-tegra, Linux ARM,
Greg Kroah-Hartman, Rafael J. Wysocki, Linux Kernel Mailing List,
Amit Kucheria, Thierry Reding
In-Reply-To: <6923d6ed-e357-b083-1830-8396d788efe5@linaro.org>
On Mon, Dec 10, 2018 at 11:18 AM Georgi Djakov <georgi.djakov@linaro.org> wrote:
>
> Hi Rafael,
>
> On 12/10/18 11:04, Rafael J. Wysocki wrote:
> > On Thu, Dec 6, 2018 at 3:55 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >>
> >> On Wed, Dec 05, 2018 at 12:41:35PM -0800, Evan Green wrote:
> >>> On Tue, Nov 27, 2018 at 10:03 AM Georgi Djakov <georgi.djakov@linaro.org> wrote:
> >>>>
> >>>> Modern SoCs have multiple processors and various dedicated cores (video, gpu,
> >>>> graphics, modem). These cores are talking to each other and can generate a
> >>>> lot of data flowing through the on-chip interconnects. These interconnect
> >>>> buses could form different topologies such as crossbar, point to point buses,
> >>>> hierarchical buses or use the network-on-chip concept.
> >>>>
> >>>> These buses have been sized usually to handle use cases with high data
> >>>> throughput but it is not necessary all the time and consume a lot of power.
> >>>> Furthermore, the priority between masters can vary depending on the running
> >>>> use case like video playback or CPU intensive tasks.
> >>>>
> >>>> Having an API to control the requirement of the system in terms of bandwidth
> >>>> and QoS, so we can adapt the interconnect configuration to match those by
> >>>> scaling the frequencies, setting link priority and tuning QoS parameters.
> >>>> This configuration can be a static, one-time operation done at boot for some
> >>>> platforms or a dynamic set of operations that happen at run-time.
> >>>>
> >>>> This patchset introduce a new API to get the requirement and configure the
> >>>> interconnect buses across the entire chipset to fit with the current demand.
> >>>> The API is NOT for changing the performance of the endpoint devices, but only
> >>>> the interconnect path in between them.
> >>>
> >>> For what it's worth, we are ready to land this in Chrome OS. I think
> >>> this series has been very well discussed and reviewed, hasn't changed
> >>> much in the last few spins, and is in good enough shape to use as a
> >>> base for future patches. Georgi's also done a great job reaching out
> >>> to other SoC vendors, and there appears to be enough consensus that
> >>> this framework will be usable by more than just Qualcomm. There are
> >>> also several drivers out on the list trying to add patches to use this
> >>> framework, with more to come, so it made sense (to us) to get this
> >>> base framework nailed down. In my experiments this is an important
> >>> piece of the overall power management story, especially on systems
> >>> that are mostly idle.
> >>>
> >>> I'll continue to track changes to this series and we will ultimately
> >>> reconcile with whatever happens upstream, but I thought it was worth
> >>> sending this note to express our "thumbs up" towards this framework.
> >>
> >> Looks like a v11 will be forthcoming, so I'll wait for that one to apply
> >> it to the tree if all looks good.
> >
> > I'm honestly not sure if it is ready yet.
> >
> > New versions are coming on and on, which may make such an impression,
> > but we had some discussion on it at the LPC and some serious questions
> > were asked during it, for instance regarding the DT binding introduced
> > here. I'm not sure how this particular issue has been addressed here,
> > for example.
>
> There have been no changes in bindings since v4 (other than squashing
> consumer and provider bindings into a single patch and fixing typos).
>
> The last DT comment was on v9 [1] where Rob wanted confirmation from
> other SoC vendors that this works for them too. And now we have that
> confirmation and there are patches posted on the list [2].
OK
> The second thing (also discussed at LPC) was about possible cases where
> some consumer drivers can't calculate how much bandwidth they actually
> need and how to address that. The proposal was to extend the OPP
> bindings with one more property, but this is not part of this patchset.
> It is a future step that needs more discussion on the mailing list. If a
> driver really needs some bandwidth data now, it should be put into the
> driver and not in DT. After we have enough consumers, we can discuss
> again if it makes sense to extract something into DT or not.
That's fine by me.
Admittedly, I have some reservations regarding the extent to which
this approach will turn out to be useful in practice, but I guess as
long as there is enough traction, the best way to find out it to try
and see. :-)
From now on I will assume that this series is going to be applied by Greg.
Thanks,
Rafael
_______________________________________________
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 v5 1/5] spi: spi-mem: Add driver for NXP FlexSPI controller
From: Yogesh Narayan Gaur @ 2018-12-10 10:59 UTC (permalink / raw)
To: Boris Brezillon
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, robh@kernel.org,
linux-kernel@vger.kernel.org, Schrempf Frieder,
linux-spi@vger.kernel.org, marek.vasut@gmail.com,
broonie@kernel.org, linux-mtd@lists.infradead.org,
computersforpeace@gmail.com, shawnguo@kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20181210115001.6c7af1d7@bbrezillon>
Hi Boris,
> -----Original Message-----
> From: Boris Brezillon [mailto:boris.brezillon@bootlin.com]
> Sent: Monday, December 10, 2018 4:20 PM
> To: Yogesh Narayan Gaur <yogeshnarayan.gaur@nxp.com>
> Cc: Schrempf Frieder <frieder.schrempf@kontron.de>; linux-
> mtd@lists.infradead.org; marek.vasut@gmail.com; broonie@kernel.org; linux-
> spi@vger.kernel.org; devicetree@vger.kernel.org; robh@kernel.org;
> mark.rutland@arm.com; shawnguo@kernel.org; linux-arm-
> kernel@lists.infradead.org; computersforpeace@gmail.com; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH v5 1/5] spi: spi-mem: Add driver for NXP FlexSPI controller
>
> On Mon, 10 Dec 2018 10:43:56 +0000
> Yogesh Narayan Gaur <yogeshnarayan.gaur@nxp.com> wrote:
>
> > > > Thus, in LUT preparation we have assigned only the base address.
> > > > Now if I have assigned ahb_buf_size to FSPI_FLSHXXCR0 register
> > > > then for
> > > read/write data beyond limit of ahb_buf_size offset I get data corruption.
> > >
> > > Why would you do that? We have the ->adjust_op_size() exactly for
> > > this reason, so, if someone tries to do a spi_mem_op with
> > > data.nbytes > ahb_buf_size you should return an error.
> > >
> > Let me explain my implementation with example. If I have to write data of size
> 0x100 bytes at offset 0x1200 for CS1, I would program as below:
> > In func nxp_fspi_select_mem(), would set value of controller address space
> size, memmap_phy_size, to FSPI_FLSHA2CR0 and rest all FSPI_FLSHXXCR0 as 0.
> > Value of memmap_phy_size is 0x10000000 i.e. 256 MB for my LX2160ARDB
> target.
> > Then in nxp_fspi_prepare_lut(), I would prepare LUT ADDR with address length
> requirement 3/4 byte for NOR or 1/2/3/4 bytes for NAND flash.
> > Also for LUT_NXP_WRITE would program data bytes as 0.
> >
> > Then inside func nxp_fspi_do_op(), set register FSPI_IPCR0 as the
> > address offset i.e. 0x1200 and in register FSPI_IPCR1 program the data
> > size to write i.e. 0x100
> >
> > If, as suggested if I tries to mark value of register FSPI_FLSHA2CR0 equal to
> ahb_buf_size (0x800), then access for address 0x1200 gives me wrong data. This
> is because as per the controller specification access to flash connected at CS1
> can be performed under range of FSPI_ FLSHA1CR0 and FSPI_ FLSHA2CR0.
>
> Don't you have a way to set an offset to apply to the address accessed through
> the AHB? And if you don't, how will it work if your mapping is smaller than the
> flash size?
Write operations are triggered using IP commands instead of AHB command.
For Read AHB command is used and in this we are adding the offset when performing memcpy_fromIO operation
memcpy_fromio(op->data.buf.in, (f->ahb_addr + op->addr.val), len);
AHB/IP operations are independent of the way how CS got selected. CS selection depends, e.g. CS1 on the value of register FSPI_FLSHA1CR0 and FSPI_FLSHA2CR0.
Mapping can never going to be smaller than the connected flash size as per discussion with the Board design team and if it's possible by user manually changes the non-soldered part then flash area beyond complete mapping is not accessible.
On LX2160ARDB, with mapping of 256MB, for now we are having 4 flash devices connected with size as 64 MB. If user wants he can have only one single flash with flash size of 256MB.
--
Regards
Yogesh Gaur
_______________________________________________
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 v5 3/7] mtd: spi-nor: add opcodes for octal Read/Write commands
From: Boris Brezillon @ 2018-12-10 10:57 UTC (permalink / raw)
To: Yogesh Narayan Gaur
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, vigneshr@ti.com,
robh@kernel.org, linux-kernel@vger.kernel.org,
linux-spi@vger.kernel.org, marek.vasut@gmail.com,
frieder.schrempf@exceet.de, broonie@kernel.org,
linux-mtd@lists.infradead.org, computersforpeace@gmail.com,
shawnguo@kernel.org, linux-arm-kernel@lists.infradead.org
In-Reply-To: <1543826226-30898-4-git-send-email-yogeshnarayan.gaur@nxp.com>
On Mon, 3 Dec 2018 08:39:18 +0000
Yogesh Narayan Gaur <yogeshnarayan.gaur@nxp.com> wrote:
> - Add opcodes for octal I/O commands
> * Read : 1-1-8 and 1-8-8 protocol
> * Write : 1-1-8 and 1-8-8 protocol
> * opcodes for 4-byte address mode command
>
> - Entry of macros in _convert_3to4_xxx function
>
> - Add flag specifying flash support octal read commands.
>
> Signed-off-by: Vignesh R <vigneshr@ti.com>
> Signed-off-by: Yogesh Gaur <yogeshnarayan.gaur@nxp.com>
Looks like the SoB and Author lines do not match
Yogesh Narayan Gaur <yogeshnarayan.gaur@nxp.com>
vs
Yogesh Gaur <yogeshnarayan.gaur@nxp.com>
Can you find a way to make them match?
Thanks,
Boris
_______________________________________________
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 06/12] KVM: arm64: Support Live Physical Time reporting
From: Mark Rutland @ 2018-12-10 10:56 UTC (permalink / raw)
To: Steven Price
Cc: Marc Zyngier, Catalin Marinas, Will Deacon, Christoffer Dall,
kvmarm, linux-arm-kernel
In-Reply-To: <20181128144527.44710-7-steven.price@arm.com>
On Wed, Nov 28, 2018 at 02:45:21PM +0000, Steven Price wrote:
> Provide a method for a guest to derive a paravirtualized counter/timer
> which isn't dependent on the host's counter frequency. This allows a
> guest to be migrated onto a new host which doesn't have the same
> frequency without the virtual counter being disturbed.
I have a number of concerns about paravirtualizing the timer frequency,
but I'll bring that up in reply to the cover letter.
I have some orthogonal comments below.
> The host provides a shared page which contains coefficients that can be
> used to map the real counter from the host (the Arm "virtual counter")
> to a paravirtualized view of time. On migration the new host updates the
> coefficients to ensure that the guests view of time (after using the
> coefficients) doesn't change and that the derived counter progresses at
> the same real frequency.
Can we please avoid using the term 'page' here?
There is a data structure in shared memory, but it is not page-sized,
and referring to it as a page here and elsewhere is confusing. The spec
never uses the term 'page'
Could we please say something like:
The host provides a datastrucutre in shared memory which ...
... to avoid the implication this is page sized/aligned etc.
[...]
> + struct kvm_arch_pvtime {
> + void *pv_page;
> +
> + gpa_t lpt_page;
> + u32 lpt_fpv;
> + } pvtime;
To remove the page terminology, perhaps something like:
struct kvm_arch_pvtime {
struct lpt *lpt;
gpa_t lpt_gpa;
u32 lpt_fpv;
};
[...]
> diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h
> index 8bf259dae9f6..fd3a2caabeb2 100644
> --- a/include/linux/kvm_types.h
> +++ b/include/linux/kvm_types.h
> @@ -49,6 +49,8 @@ typedef unsigned long gva_t;
> typedef u64 gpa_t;
> typedef u64 gfn_t;
>
> +#define GPA_INVALID -1
To avoid any fun with signed/unsigned comparison, can we please make
this:
#define GPA_INVALID ((gpa_t)-1)
... or:
#define GPA_INVALID (~(gpa_t)0)
[...]
> +static void update_vtimer_cval(struct kvm *kvm, u32 previous_rate)
> +{
> + u32 current_rate = arch_timer_get_rate();
> + u64 current_time = kvm_phys_timer_read();
> + int i;
> + struct kvm_vcpu *vcpu;
> + u64 rel_cval;
> +
> + /* Early out if there's nothing to do */
> + if (likely(previous_rate == current_rate))
> + return;
Given this only happens on migration, I don't think we need to care
about likely/unlikely here, and can drop that from the condition.
[...]
> +int kvm_arm_update_lpt_sequence(struct kvm *kvm)
> +{
> + struct pvclock_vm_time_info *pvclock;
> + u64 lpt_ipa = kvm->arch.pvtime.lpt_page;
> + u64 native_freq, pv_freq, scale_mult, div_by_pv_freq_mult;
> + u64 shift = 0;
> + u64 sequence_number = 0;
> +
> + if (lpt_ipa == GPA_INVALID)
> + return -EINVAL;
> +
> + /* Page address must be 64 byte aligned */
> + if (lpt_ipa & 63)
> + return -EINVAL;
Please use IS_ALIGNED(), e.g.
if (!IS_ALIGNED(lpt_ipa, 64))
return -EINVAL;
Thanks,
Mark.
_______________________________________________
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 4/4] ARM: dts: imx7d: sbc imx7: add uart5
From: Hans Ole Hatzel @ 2018-12-10 10:51 UTC (permalink / raw)
To: Shawn Guo
Cc: fabio.estevam, devicetree, Julian Scheel, linux-imx,
linux-arm-kernel
In-Reply-To: <20181206031520.GE3987@dragon>
On 12/6/18 4:15 AM, Shawn Guo wrote:
> On Tue, Dec 04, 2018 at 12:07:41PM +0100, Hans Ole Hatzel wrote:
>> Adds uart5 connections as used on the sbc.
>> In the sbc's reference material this is known as uart1, it does however
>> connect to the som as uart5. This uart is connected via the soc's
>> pins 11 (TX) and 13 (RX). On the sbc it is pinned out on P5
>> using pins 15 (TX) and 17 (RX).
>>
>> Signed-off-by: Hans Ole Hatzel <hohatzel@jusst.de>
>> Signed-off-by: Julian Scheel <jscheel@jusst.de>
>> ---
>> arch/arm/boot/dts/imx7d-cl-som-imx7.dts | 16 ++++++++++++++++
>> arch/arm/boot/dts/imx7d-sbc-imx7.dts | 4 ++++
>> 2 files changed, 20 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/imx7d-cl-som-imx7.dts b/arch/arm/boot/dts/imx7d-cl-som-imx7.dts
>> index 6c2c844dc052..f7c002093c67 100644
>> --- a/arch/arm/boot/dts/imx7d-cl-som-imx7.dts
>> +++ b/arch/arm/boot/dts/imx7d-cl-som-imx7.dts
>> @@ -223,6 +223,15 @@
>> fsl;
>> };
>>
>> +&uart5 {
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_uart5>;
>> + assigned-clocks = <&clks IMX7D_UART5_ROOT_SRC>;
>> + assigned-clock-parents = <&clks IMX7D_PLL_SYS_MAIN_240M_CLK>;
>
> Can we have some comments in the commit log on why we have to use this
> particular clock source?
>
> Shawn
>
imx7d-pico.dts does this the same way. Is that good enough of a reason?
If so, should it be included in the commit message?
Hans
>> + status = "okay";
>> + fsl;
>> +};
>> +
>> &usbotg1 {
>> pinctrl-names = "default";
>> pinctrl-0 = <&pinctrl_usbotg1>;
>> @@ -330,6 +339,13 @@
>> >;
>> };
>>
>> + pinctrl_uart5: uart5grp {
>> + fsl,pins = <
>> + MX7D_PAD_I2C4_SCL__UART5_DCE_RX 0x79
>> + MX7D_PAD_I2C4_SDA__UART5_DCE_TX 0x79
>> + >;
>> + };
>> +
>> pinctrl_usdhc2: usdhc2grp {
>> fsl,pins = <
>> MX7D_PAD_SD2_CMD__SD2_CMD 0x59
>> diff --git a/arch/arm/boot/dts/imx7d-sbc-imx7.dts b/arch/arm/boot/dts/imx7d-sbc-imx7.dts
>> index 74904127fbc6..d23d62aceb82 100644
>> --- a/arch/arm/boot/dts/imx7d-sbc-imx7.dts
>> +++ b/arch/arm/boot/dts/imx7d-sbc-imx7.dts
>> @@ -30,6 +30,10 @@
>> status = "okay";
>> };
>>
>> +&uart5 {
>> + status = "okay";
>> +};
>> +
>> &iomuxc {
>> pinctrl_usdhc1: usdhc1grp {
>> fsl,pins = <
>> --
>> 2.19.2
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
_______________________________________________
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 v5 1/5] spi: spi-mem: Add driver for NXP FlexSPI controller
From: Boris Brezillon @ 2018-12-10 10:50 UTC (permalink / raw)
To: Yogesh Narayan Gaur
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, robh@kernel.org,
linux-kernel@vger.kernel.org, Schrempf Frieder,
linux-spi@vger.kernel.org, marek.vasut@gmail.com,
broonie@kernel.org, linux-mtd@lists.infradead.org,
computersforpeace@gmail.com, shawnguo@kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <VI1PR04MB57268740CA806FFB164780EB99A50@VI1PR04MB5726.eurprd04.prod.outlook.com>
On Mon, 10 Dec 2018 10:43:56 +0000
Yogesh Narayan Gaur <yogeshnarayan.gaur@nxp.com> wrote:
> > > Thus, in LUT preparation we have assigned only the base address.
> > > Now if I have assigned ahb_buf_size to FSPI_FLSHXXCR0 register then for
> > read/write data beyond limit of ahb_buf_size offset I get data corruption.
> >
> > Why would you do that? We have the ->adjust_op_size() exactly for this reason,
> > so, if someone tries to do a spi_mem_op with data.nbytes > ahb_buf_size you
> > should return an error.
> >
> Let me explain my implementation with example. If I have to write data of size 0x100 bytes at offset 0x1200 for CS1, I would program as below:
> In func nxp_fspi_select_mem(), would set value of controller address space size, memmap_phy_size, to FSPI_FLSHA2CR0 and rest all FSPI_FLSHXXCR0 as 0.
> Value of memmap_phy_size is 0x10000000 i.e. 256 MB for my LX2160ARDB target.
> Then in nxp_fspi_prepare_lut(), I would prepare LUT ADDR with address length requirement 3/4 byte for NOR or 1/2/3/4 bytes for NAND flash.
> Also for LUT_NXP_WRITE would program data bytes as 0.
>
> Then inside func nxp_fspi_do_op(), set register FSPI_IPCR0 as the address offset i.e. 0x1200 and in register FSPI_IPCR1 program the data size to write i.e. 0x100
>
> If, as suggested if I tries to mark value of register FSPI_FLSHA2CR0 equal to ahb_buf_size (0x800), then access for address 0x1200 gives me wrong data. This is because as per the controller specification access to flash connected at CS1 can be performed under range of FSPI_ FLSHA1CR0 and FSPI_ FLSHA2CR0.
Don't you have a way to set an offset to apply to the address accessed
through the AHB? And if you don't, how will it work if your mapping
is smaller than the flash size?
_______________________________________________
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/8] arm64: KVM: Make VHE Stage-2 TLB invalidation operations non-interruptible
From: Christoffer Dall @ 2018-12-10 10:49 UTC (permalink / raw)
To: Marc Zyngier
Cc: Mark Rutland, kvm, Suzuki K Poulose, Catalin Marinas, Will Deacon,
James Morse, kvmarm, linux-arm-kernel
In-Reply-To: <d9219768-bbef-5d51-3bb4-84cf35714a4d@arm.com>
On Mon, Dec 10, 2018 at 10:24:31AM +0000, Marc Zyngier wrote:
> Hi Christoffer,
>
> On 10/12/2018 10:03, Christoffer Dall wrote:
> > On Thu, Dec 06, 2018 at 05:31:19PM +0000, Marc Zyngier wrote:
> >> Contrary to the non-VHE version of the TLB invalidation helpers, the VHE
> >> code has interrupts enabled, meaning that we can take an interrupt in
> >> the middle of such a sequence, and start running something else with
> >> HCR_EL2.TGE cleared.
> >
> > Do we have to clear TGE to perform the TLB invalidation, or is that just
> > a side-effect of re-using code?
>
> We really do need to clear TGE. From the description of TLBI VMALLE1IS:
>
> <quote>
> When EL2 is implemented and enabled in the current Security state:
> — If HCR_EL2.{E2H, TGE} is not {1, 1}, the entry would be used with the
> current VMID and would be required to translate the specified VA using
> the EL1&0 translation regime.
> — If HCR_EL2.{E2H, TGE} is {1, 1}, the entry would be required to
> translate the specified VA using the EL2&0 translation regime.
> </quote>
>
> > Also, do we generally perform TLB invalidations in the kernel with
> > interrupts disabled, or is this just a result of clearing TGE?
>
> That's definitely a result of clearing TGE. We could be taking an
> interrupt here, and execute a user access on the back of it (perf will
> happily walk a user-space stack in that context, for example). Having
> TGE clear in that context. An alternative solution would be to
> save/restore TGE on interrupt entry/exit, but that's a bit overkill when
> you consider how rarely we issue such TLB invalidation.
>
> > Somehow I feel like this should look more like just another TLB
> > invalidation in the kernel, but if there's a good reason why it can't
> > then this is fine.
>
> The rest of the TLB invalidation in the kernel doesn't need to
> save/restore any context. They apply to a set of parameters that are
> already loaded on the CPU. What we have here is substantially different.
>
Thanks for the explanation and Arm ARM quote. I failed to find that on
my own this particular Monday morning.
Acked-by: Christoffer Dall <christoffer.dall@arm.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: [PATCH v9 1/8] KVM: arm/arm64: Share common code in user_mem_abort()
From: Suzuki K Poulose @ 2018-12-10 10:47 UTC (permalink / raw)
To: Christoffer Dall
Cc: Anshuman Khandual, marc.zyngier, will.deacon, linux-kernel,
punitagrawal, kvmarm, linux-arm-kernel
In-Reply-To: <20181210085616.GB30263@e113682-lin.lund.arm.com>
On 10/12/2018 08:56, Christoffer Dall wrote:
> On Mon, Dec 03, 2018 at 01:37:37PM +0000, Suzuki K Poulose wrote:
>> Hi Anshuman,
>>
>> On 03/12/2018 12:11, Anshuman Khandual wrote:
>>>
>>>
>>> On 10/31/2018 11:27 PM, Punit Agrawal wrote:
>>>> The code for operations such as marking the pfn as dirty, and
>>>> dcache/icache maintenance during stage 2 fault handling is duplicated
>>>> between normal pages and PMD hugepages.
>>>>
>>>> Instead of creating another copy of the operations when we introduce
>>>> PUD hugepages, let's share them across the different pagesizes.
>>>>
>>>> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
>>>> Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>>>> Cc: Christoffer Dall <christoffer.dall@arm.com>
>>>> Cc: Marc Zyngier <marc.zyngier@arm.com>
>>>> ---
>>>> virt/kvm/arm/mmu.c | 49 ++++++++++++++++++++++++++++------------------
>>>> 1 file changed, 30 insertions(+), 19 deletions(-)
>>>>
>>>> diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
>>>> index 5eca48bdb1a6..59595207c5e1 100644
>>>> --- a/virt/kvm/arm/mmu.c
>>>> +++ b/virt/kvm/arm/mmu.c
>>>> @@ -1475,7 +1475,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>>>> unsigned long fault_status)
>>>> {
>>>> int ret;
>>>> - bool write_fault, exec_fault, writable, hugetlb = false, force_pte = false;
>>>> + bool write_fault, exec_fault, writable, force_pte = false;
>>>> unsigned long mmu_seq;
>>>> gfn_t gfn = fault_ipa >> PAGE_SHIFT;
>>>> struct kvm *kvm = vcpu->kvm;
>>>> @@ -1484,7 +1484,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>>>> kvm_pfn_t pfn;
>>>> pgprot_t mem_type = PAGE_S2;
>>>> bool logging_active = memslot_is_logging(memslot);
>>>> - unsigned long flags = 0;
>>>> + unsigned long vma_pagesize, flags = 0;
>>>
>>> A small nit s/vma_pagesize/pagesize. Why call it VMA ? Its implicit.
>>
>> May be we could call it mapsize. pagesize is confusing.
>>
>
> I'm ok with mapsize. I see the vma_pagesize name coming from the fact
> that this is initially set to the return value from vma_kernel_pagesize.
>
> I have not problems with either vma_pagesize or mapsize.
>
>>>
>>>> write_fault = kvm_is_write_fault(vcpu);
>>>> exec_fault = kvm_vcpu_trap_is_iabt(vcpu);
>>>> @@ -1504,10 +1504,16 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>>>> return -EFAULT;
>>>> }
>>>> - if (vma_kernel_pagesize(vma) == PMD_SIZE && !logging_active) {
>>>> - hugetlb = true;
>>>> + vma_pagesize = vma_kernel_pagesize(vma);
>>>> + if (vma_pagesize == PMD_SIZE && !logging_active) {
>>>> gfn = (fault_ipa & PMD_MASK) >> PAGE_SHIFT;
>>>> } else {
>>>> + /*
>>>> + * Fallback to PTE if it's not one of the Stage 2
>>>> + * supported hugepage sizes
>>>> + */
>>>> + vma_pagesize = PAGE_SIZE;
>>>
>>> This seems redundant and should be dropped. vma_kernel_pagesize() here either
>>> calls hugetlb_vm_op_pagesize (via hugetlb_vm_ops->pagesize) or simply returns
>>> PAGE_SIZE. The vm_ops path is taken if the QEMU VMA covering any given HVA is
>>> backed either by HugeTLB pages or simply normal pages. vma_pagesize would
>>> either has a value of PMD_SIZE (HugeTLB hstate based) or PAGE_SIZE. Hence if
>>> its not PMD_SIZE it must be PAGE_SIZE which should not be assigned again.
>>
>> We may want to force using the PTE mappings when logging_active (e.g, migration
>> ?) to prevent keep tracking of huge pages. So the check is still valid.
>>
>>
>
> Agreed, and let's not try additionally change the logic and flow with
> this patch.
>
>>>
>>>> +
>>>> /*
>>>> * Pages belonging to memslots that don't have the same
>>>> * alignment for userspace and IPA cannot be mapped using
>>>> @@ -1573,23 +1579,33 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>>>> if (mmu_notifier_retry(kvm, mmu_seq))
>>>> goto out_unlock;
>>>> - if (!hugetlb && !force_pte)
>>>> - hugetlb = transparent_hugepage_adjust(&pfn, &fault_ipa);
>>>> + if (vma_pagesize == PAGE_SIZE && !force_pte) {
>>>> + /*
>>>> + * Only PMD_SIZE transparent hugepages(THP) are
>>>> + * currently supported. This code will need to be
>>>> + * updated to support other THP sizes.
>>>> + */
>>>
>>> This comment belongs to transparent_hugepage_adjust() but not here.
>>
>> I think this is relevant here than in thp_adjust, unless we rename
>> the function below to something generic, handle_hugepage_adjust().
>>
>
> Agreed.
>
>>>> + if (transparent_hugepage_adjust(&pfn, &fault_ipa))
>>>> + vma_pagesize = PMD_SIZE;
>>>
>>> IIUC transparent_hugepage_adjust() is only getting called here. Instead of
>>> returning 'true' when it is able to detect a huge page backing and doing
>>> an adjustment there after, it should rather return THP size (PMD_SIZE) to
>>> accommodate probable multi size THP support in future .
>>
>> That makes sense.
>>
>
> That's fine.
>
Btw, after a further thought, since we don't have any THP support for anything
other than PMD_SIZE, I am dropping the above suggestion. We need to make changes
in our stage2 page table manipulation code anyway to support the new sizes. So
this could be addressed when we get there, to keep the changes minimal and
specific to the PUD huge page support.
Cheers
Suzuki
_______________________________________________
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 7/8] arm64: KVM: Handle ARM erratum 1165522 in TLB invalidation
From: Marc Zyngier @ 2018-12-10 10:46 UTC (permalink / raw)
To: Christoffer Dall
Cc: Mark Rutland, kvm, Suzuki K Poulose, Catalin Marinas, Will Deacon,
James Morse, kvmarm, linux-arm-kernel
In-Reply-To: <20181210101912.GK30263@e113682-lin.lund.arm.com>
On 10/12/2018 10:19, Christoffer Dall wrote:
> On Thu, Dec 06, 2018 at 05:31:25PM +0000, Marc Zyngier wrote:
>> In order to avoid TLB corruption whilst invalidating TLBs on CPUs
>> affected by erratum 1165522, we need to prevent S1 page tables
>> from being usable.
>>
>> For this, we set the EL1 S1 MMU on, and also disable the page table
>> walker (by setting the TCR_EL1.EPD* bits to 1).
>>
>> This ensures that once we switch to the EL1/EL0 translation regime,
>> speculated AT instructions won't be able to parse the page tables.
>>
>> Reviewed-by: James Morse <james.morse@arm.com>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>> arch/arm64/kvm/hyp/tlb.c | 66 +++++++++++++++++++++++++++++++---------
>> 1 file changed, 51 insertions(+), 15 deletions(-)
>>
>> diff --git a/arch/arm64/kvm/hyp/tlb.c b/arch/arm64/kvm/hyp/tlb.c
>> index 7fcc9c1a5f45..ec157543d5a9 100644
>> --- a/arch/arm64/kvm/hyp/tlb.c
>> +++ b/arch/arm64/kvm/hyp/tlb.c
>> @@ -21,12 +21,36 @@
>> #include <asm/kvm_mmu.h>
>> #include <asm/tlbflush.h>
>>
>> +struct tlb_inv_context {
>> + unsigned long flags;
>> + u64 tcr;
>> + u64 sctlr;
>> +};
>> +
>> static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm,
>> - unsigned long *flags)
>> + struct tlb_inv_context *cxt)
>> {
>> u64 val;
>>
>> - local_irq_save(*flags);
>> + local_irq_save(cxt->flags);
>> +
>> + if (cpus_have_const_cap(ARM64_WORKAROUND_1165522)) {
>> + /*
>> + * For CPUs that are affected by ARM erratum 1165522, we
>> + * cannot trust stage-1 to be in a correct state at that
>> + * point. Since we do not want to force a full load of the
>> + * vcpu state, we prevent the EL1 page-table walker to
>> + * allocate new TLBs. This is done by setting the EPD bits
>> + * in the TCR_EL1 register. We also need to prevent it to
>> + * allocate IPA->PA walks, so we enable the S1 MMU...
>> + */
>> + val = cxt->tcr = read_sysreg_el1(tcr);
>> + val |= TCR_EPD1_MASK | TCR_EPD0_MASK;
>> + write_sysreg_el1(val, tcr);
>> + val = cxt->sctlr = read_sysreg_el1(sctlr);
>> + val |= SCTLR_ELx_M;
>> + write_sysreg_el1(val, sctlr);
>> + }
>>
>> /*
>> * With VHE enabled, we have HCR_EL2.{E2H,TGE} = {1,1}, and
>> @@ -34,6 +58,11 @@ static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm,
>> * guest TLBs (EL1/EL0), we need to change one of these two
>> * bits. Changing E2H is impossible (goodbye TTBR1_EL2), so
>> * let's flip TGE before executing the TLB operation.
>> + *
>> + * ARM erratum 1165522 requires some special handling (again),
>> + * as we need to make sure both stages of translation are in
>> + * place before clearing TGE. __load_guest_stage2() already
>> + * has an ISB in order to deal with this.
>> */
>> __load_guest_stage2(kvm);
>> val = read_sysreg(hcr_el2);
>> @@ -43,7 +72,7 @@ static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm,
>> }
>>
>> static void __hyp_text __tlb_switch_to_guest_nvhe(struct kvm *kvm,
>> - unsigned long *flags)
>> + struct tlb_inv_context *cxt)
>> {
>> __load_guest_stage2(kvm);
>> isb();
>> @@ -55,7 +84,7 @@ static hyp_alternate_select(__tlb_switch_to_guest,
>> ARM64_HAS_VIRT_HOST_EXTN);
>>
>> static void __hyp_text __tlb_switch_to_host_vhe(struct kvm *kvm,
>> - unsigned long flags)
>> + struct tlb_inv_context *cxt)
>> {
>> /*
>> * We're done with the TLB operation, let's restore the host's
>> @@ -64,11 +93,18 @@ static void __hyp_text __tlb_switch_to_host_vhe(struct kvm *kvm,
>> write_sysreg(0, vttbr_el2);
>> write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
>> isb();
>> - local_irq_restore(flags);
>> +
>> + if (cpus_have_const_cap(ARM64_WORKAROUND_1165522)) {
>> + /* Restore the guest's registers to what they were */
>
> host's ?
Hum... Yes, silly thinko.
[...]
>
> Otherwise:
>
> Acked-by: Christoffer Dall <christoffer.dall@arm.com>
>
Thanks,
M.
--
Jazz is not dead. It just smells funny...
_______________________________________________
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 v5 3/7] mtd: spi-nor: add opcodes for octal Read/Write commands
From: Tudor.Ambarus @ 2018-12-10 10:44 UTC (permalink / raw)
To: yogeshnarayan.gaur, linux-mtd, boris.brezillon, broonie,
marek.vasut, vigneshr, linux-spi, devicetree
Cc: mark.rutland, robh, linux-kernel, frieder.schrempf,
computersforpeace, shawnguo, linux-arm-kernel
In-Reply-To: <1543826226-30898-4-git-send-email-yogeshnarayan.gaur@nxp.com>
Hi, Yogesh,
On 12/03/2018 10:39 AM, Yogesh Narayan Gaur wrote:
> - Add opcodes for octal I/O commands
> * Read : 1-1-8 and 1-8-8 protocol
> * Write : 1-1-8 and 1-8-8 protocol
> * opcodes for 4-byte address mode command
>
> - Entry of macros in _convert_3to4_xxx function
>
> - Add flag specifying flash support octal read commands.
>
> Signed-off-by: Vignesh R <vigneshr@ti.com>
> Signed-off-by: Yogesh Gaur <yogeshnarayan.gaur@nxp.com>
> ---
> Changes for v5:
> - Modified string 'octo' with 'octal'.
> Changes for v4:
> - None
> Changes for v3:
> - Modified string 'octal' with 'octo'.
> Changes for v2:
> - Incorporated review comments of Boris and Vignesh
>
> drivers/mtd/spi-nor/spi-nor.c | 16 ++++++++++++++--
> include/linux/mtd/spi-nor.h | 8 ++++++++
> 2 files changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 398d273..7a2176d 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -90,6 +90,7 @@ struct flash_info {
> #define NO_CHIP_ERASE BIT(12) /* Chip does not support chip erase */
> #define SPI_NOR_SKIP_SFDP BIT(13) /* Skip parsing of SFDP tables */
> #define USE_CLSR BIT(14) /* use CLSR command */
> +#define SPI_NOR_OCTAL_READ BIT(15) /* Flash supports Octal Read */
>
> int (*quad_enable)(struct spi_nor *nor);
> };
> @@ -209,6 +210,8 @@ static inline u8 spi_nor_convert_3to4_read(u8 opcode)
> { SPINOR_OP_READ_1_2_2, SPINOR_OP_READ_1_2_2_4B },
> { SPINOR_OP_READ_1_1_4, SPINOR_OP_READ_1_1_4_4B },
> { SPINOR_OP_READ_1_4_4, SPINOR_OP_READ_1_4_4_4B },
> + { SPINOR_OP_READ_1_1_8, SPINOR_OP_READ_1_1_8_4B },
> + { SPINOR_OP_READ_1_8_8, SPINOR_OP_READ_1_8_8_4B },
>
> { SPINOR_OP_READ_1_1_1_DTR, SPINOR_OP_READ_1_1_1_DTR_4B },
> { SPINOR_OP_READ_1_2_2_DTR, SPINOR_OP_READ_1_2_2_DTR_4B },
> @@ -225,6 +228,8 @@ static inline u8 spi_nor_convert_3to4_program(u8 opcode)
> { SPINOR_OP_PP, SPINOR_OP_PP_4B },
> { SPINOR_OP_PP_1_1_4, SPINOR_OP_PP_1_1_4_4B },
> { SPINOR_OP_PP_1_4_4, SPINOR_OP_PP_1_4_4_4B },
> + { SPINOR_OP_PP_1_1_8, SPINOR_OP_PP_1_1_8_4B },
> + { SPINOR_OP_PP_1_8_8, SPINOR_OP_PP_1_8_8_4B },
> };
>
> return spi_nor_convert_opcode(opcode, spi_nor_3to4_program,
> @@ -2093,7 +2098,7 @@ enum spi_nor_read_command_index {
> SNOR_CMD_READ_4_4_4,
> SNOR_CMD_READ_1_4_4_DTR,
>
> - /* Octo SPI */
> + /* Octal SPI */
> SNOR_CMD_READ_1_1_8,
> SNOR_CMD_READ_1_8_8,
> SNOR_CMD_READ_8_8_8,
> @@ -2110,7 +2115,7 @@ enum spi_nor_pp_command_index {
> SNOR_CMD_PP_1_4_4,
> SNOR_CMD_PP_4_4_4,
>
> - /* Octo SPI */
> + /* Octal SPI */
> SNOR_CMD_PP_1_1_8,
> SNOR_CMD_PP_1_8_8,
> SNOR_CMD_PP_8_8_8,
> @@ -3195,6 +3200,13 @@ static int spi_nor_init_params(struct spi_nor *nor,
> SNOR_PROTO_1_1_4);
> }
>
> + if (info->flags & SPI_NOR_OCTAL_READ) {
> + params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_8;
> + spi_nor_set_read_settings(¶ms->reads[SNOR_CMD_READ_1_1_8],
> + 0, 8, SPINOR_OP_READ_1_1_8,
> + SNOR_PROTO_1_1_8);
> + }
> +> /* Page Program settings. */
> params->hwcaps.mask |= SNOR_HWCAPS_PP;
> spi_nor_set_pp_settings(¶ms->page_programs[SNOR_CMD_PP],
At the end of spi_nor_init_params we check the conditions for parsing the sfdp.
Shouldn't SPI_NOR_OCTAL_READ trigger the sfdp parsing when SPI_NOR_SKIP_SFDP is
not set?
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index 8b1acf6..019f534 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -50,9 +50,13 @@
Can you please s/octo/octal on these as well:
$ grep -ni octo include/linux/mtd/spi-nor.h
471: * As a matter of performances, it is relevant to use Octo SPI protocols first,
492:#define SNOR_HWCPAS_READ_OCTO GENMASK(14, 11)
501: * Like (Fast) Read capabilities, Octo/Quad SPI protocols are preferred to the
515:#define SNOR_HWCAPS_PP_OCTO GENMASK(22, 20)
thanks,
ta
_______________________________________________
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 v5 1/5] spi: spi-mem: Add driver for NXP FlexSPI controller
From: Yogesh Narayan Gaur @ 2018-12-10 10:43 UTC (permalink / raw)
To: Boris Brezillon
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, robh@kernel.org,
linux-kernel@vger.kernel.org, Schrempf Frieder,
linux-spi@vger.kernel.org, marek.vasut@gmail.com,
broonie@kernel.org, linux-mtd@lists.infradead.org,
computersforpeace@gmail.com, shawnguo@kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20181210111909.35384eee@bbrezillon>
Hi Boris, Frieder,
> -----Original Message-----
> From: Boris Brezillon [mailto:boris.brezillon@bootlin.com]
> Sent: Monday, December 10, 2018 3:49 PM
> To: Yogesh Narayan Gaur <yogeshnarayan.gaur@nxp.com>
> Cc: Schrempf Frieder <frieder.schrempf@kontron.de>; linux-
> mtd@lists.infradead.org; marek.vasut@gmail.com; broonie@kernel.org; linux-
> spi@vger.kernel.org; devicetree@vger.kernel.org; robh@kernel.org;
> mark.rutland@arm.com; shawnguo@kernel.org; linux-arm-
> kernel@lists.infradead.org; computersforpeace@gmail.com; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH v5 1/5] spi: spi-mem: Add driver for NXP FlexSPI controller
>
> On Mon, 10 Dec 2018 09:41:51 +0000
> Yogesh Narayan Gaur <yogeshnarayan.gaur@nxp.com> wrote:
>
> > > > +/* Instead of busy looping invoke readl_poll_timeout functionality.
> > > > +*/ static int fspi_readl_poll_tout(struct nxp_fspi *f, void __iomem *base,
> > > > + u32 mask, u32 delay_us,
> > > > + u32 timeout_us, bool condition) {
> > > > + u32 reg;
> > > > +
> > > > + if (!f->devtype_data->little_endian)
> > > > + mask = (u32)cpu_to_be32(mask);
> > > > +
> > > > + if (condition)
> > > > + return readl_poll_timeout(base, reg, (reg & mask),
> > > > + delay_us, timeout_us);
> > > > + else
> > > > + return readl_poll_timeout(base, reg, !(reg & mask),
> > > > + delay_us, timeout_us);
> > >
> > > I would rather use a local variable to store the condition:
> > >
> > > bool c = condition ? (reg & mask):!(reg & mask);
> > >
> > With these type of usage getting below warning messages.
> >
> > drivers/spi/spi-nxp-fspi.c: In function ‘fspi_readl_poll_tout.isra.10.constprop’:
> > drivers/spi/spi-nxp-fspi.c:446:21: warning: ‘reg’ may be used uninitialized in
> this function [-Wmaybe-uninitialized]
> > bool cn = c ? (reg & mask) : !(reg & mask);
> >
> > If assign value to reg = 0xffffffff then timeout is start getting hit for False case
> and if assign value 0 then start getting timeout hit for true case.
> >
> > I would rather not try to modify this function.
>
> I agree. Let's keep this function readable even if this implies duplicating a few
> lines of code.
>
> >
> > > return readl_poll_timeout(base, reg, c, delay_us, timeout_us);
> > >
> > > > +}
> > > > +
> > > > +/*
> > > > + * If the slave device content being changed by Write/Erase, need
> > > > +to
> > > > + * invalidate the AHB buffer. This can be achieved by doing the
> > > > +reset
> > > > + * of controller after setting MCR0[SWRESET] bit.
> > > > + */
> > > > +static inline void nxp_fspi_invalid(struct nxp_fspi *f) {
> > > > + u32 reg;
> > > > + int ret;
> > > > +
> > > > + reg = fspi_readl(f, f->iobase + FSPI_MCR0);
> > > > + fspi_writel(f, reg | FSPI_MCR0_SWRST, f->iobase + FSPI_MCR0);
> > > > +
> > > > + /* w1c register, wait unit clear */
> > > > + ret = fspi_readl_poll_tout(f, f->iobase + FSPI_MCR0,
> > > > + FSPI_MCR0_SWRST, 0, POLL_TOUT, false);
> > > > + WARN_ON(ret);
> > > > +}
> > > > +
> > > > +static void nxp_fspi_prepare_lut(struct nxp_fspi *f,
> > > > + const struct spi_mem_op *op) {
> > > > + void __iomem *base = f->iobase;
> > > > + u32 lutval[4] = {};
> > > > + int lutidx = 1, i;
> > > > +
> > > > + /* cmd */
> > > > + lutval[0] |= LUT_DEF(0, LUT_CMD, LUT_PAD(op->cmd.buswidth),
> > > > + op->cmd.opcode);
> > > > +
> > > > + /* addr bus width */
> > > > + if (op->addr.nbytes) {
> > > > + u32 addrlen = 0;
> > > > +
> > > > + switch (op->addr.nbytes) {
> > > > + case 1:
> > > > + addrlen = ADDR8BIT;
> > > > + break;
> > > > + case 2:
> > > > + addrlen = ADDR16BIT;
> > > > + break;
> > > > + case 3:
> > > > + addrlen = ADDR24BIT;
> > > > + break;
> > > > + case 4:
> > > > + addrlen = ADDR32BIT;
> > > > + break;
> > > > + default:
> > > > + dev_err(f->dev, "In-correct address length\n");
> > > > + return;
> > > > + }
> > >
> > > You don't need to validate op->addr.nbytes here, this is already
> > > done in nxp_fspi_supports_op().
> >
> > Yes, I need to validate op->addr.nbytes else LUT would going to be
> programmed for 0 addrlen.
> > I have checked this on the target.
>
> Also agree there. Some operations have 0 address bytes. We could also test
> addr.buswidth, but I'm fine with the addr.nbytes test too.
>
>
> > > > +static void nxp_fspi_select_mem(struct nxp_fspi *f, struct
> > > > +spi_device
> > > > +*spi) {
> > > > + unsigned long rate = spi->max_speed_hz;
> > > > + int ret;
> > > > + uint64_t size_kb;
> > > > +
> > > > + /*
> > > > + * Return, if previously selected slave device is same as current
> > > > + * requested slave device.
> > > > + */
> > > > + if (f->selected == spi->chip_select)
> > > > + return;
> > > > +
> > > > + /* Reset FLSHxxCR0 registers */
> > > > + fspi_writel(f, 0, f->iobase + FSPI_FLSHA1CR0);
> > > > + fspi_writel(f, 0, f->iobase + FSPI_FLSHA2CR0);
> > > > + fspi_writel(f, 0, f->iobase + FSPI_FLSHB1CR0);
> > > > + fspi_writel(f, 0, f->iobase + FSPI_FLSHB2CR0);
> > > > +
> > > > + /* Assign controller memory mapped space as size, KBytes, of flash. */
> > > > + size_kb = FSPI_FLSHXCR0_SZ(f->memmap_phy_size);
> > >
> > Above description of this function, explains the reason for using
> memmap_phy_size.
> > This is not the arbitrary size, but the memory mapped size being assigned to
> the controller.
> >
> > > You are still using memory of arbitrary size (memmap_phy_size) for
> > > mapping the flash. Why not use the same approach as in the QSPI
> > > driver and just map ahb_buf_size until we implement the dirmap API?
> > The approach which being used in QSPI driver didn't work here, I have tried
> with that.
> > In QSPI driver, while preparing LUT we are assigning read/write address in the
> LUT preparation and have to for some unknown hack have to provide macro for
> LUT_MODE instead of LUT_ADDR.
> > But this thing didn't work for FlexSPI.
> > I discussed with HW IP owner and they suggested only to use LUT_ADDR for
> specifying the address length of the command i.e. 3-byte or 4-byte address
> command (NOR) or 1-2 byte address command for NAND.
>
> Actually, we would have used a LUT_ADDR too if the QSPI IP was support ADDR
> instructions with a number of bytes < 3, but for some unknown reasons it does
> not work.
>
> >
> > Thus, in LUT preparation we have assigned only the base address.
> > Now if I have assigned ahb_buf_size to FSPI_FLSHXXCR0 register then for
> read/write data beyond limit of ahb_buf_size offset I get data corruption.
>
> Why would you do that? We have the ->adjust_op_size() exactly for this reason,
> so, if someone tries to do a spi_mem_op with data.nbytes > ahb_buf_size you
> should return an error.
>
Let me explain my implementation with example. If I have to write data of size 0x100 bytes at offset 0x1200 for CS1, I would program as below:
In func nxp_fspi_select_mem(), would set value of controller address space size, memmap_phy_size, to FSPI_FLSHA2CR0 and rest all FSPI_FLSHXXCR0 as 0.
Value of memmap_phy_size is 0x10000000 i.e. 256 MB for my LX2160ARDB target.
Then in nxp_fspi_prepare_lut(), I would prepare LUT ADDR with address length requirement 3/4 byte for NOR or 1/2/3/4 bytes for NAND flash.
Also for LUT_NXP_WRITE would program data bytes as 0.
Then inside func nxp_fspi_do_op(), set register FSPI_IPCR0 as the address offset i.e. 0x1200 and in register FSPI_IPCR1 program the data size to write i.e. 0x100
If, as suggested if I tries to mark value of register FSPI_FLSHA2CR0 equal to ahb_buf_size (0x800), then access for address 0x1200 gives me wrong data. This is because as per the controller specification access to flash connected at CS1 can be performed under range of FSPI_ FLSHA1CR0 and FSPI_ FLSHA2CR0.
So either FSPI_ FLSHA2CR0 should have the value of the actual connected flash size but we don't have mechanism to know the flash size in current implementation.
Thus instead of using some other arbitrary value, I have used the full size being allocated to the FlexSPI controller.
> >
> > Thus, for generic approach have assigned FSPI_FLSHXXCR0 equal to the
> memory mapped size to the controller. This would also not going to depend on
> the number of CS present on the target.
>
> I kind of agree with Frieder on that one, I think it's preferable to limit the per-
> read-op size to ahb_buf_size and let the upper layer split the request in several
> sub-requests. On the controller side of things, you just have to have a mapping
> of ahb_buf_size per-CS. If you want to further optimize things, implement the
> dirmap hooks.
>
> >
> > > You are already aligning the AHB reads for this in nxp_fspi_adjust_op_size().
> > >
> > Yes, max read data size can be ahb_buf_size. Thus we need to check max read
> size with ahb_buf_size.
>
> Well, it's never a bad thing to check it twice, just in case the spi-mem user is
> misusing the API.
>
> > > > +static void nxp_fspi_fill_txfifo(struct nxp_fspi *f,
> > > > + const struct spi_mem_op *op) {
> > > > + void __iomem *base = f->iobase;
> > > > + int i, j, ret;
> > > > + int size, tmp_size, wm_size;
> > > > + u32 data = 0;
> > > > + u32 *txbuf = (u32 *) op->data.buf.out;
> > > > +
> > > > + /* clear the TX FIFO. */
> > > > + fspi_writel(f, FSPI_IPTXFCR_CLR, base + FSPI_IPTXFCR);
> > > > +
> > > > + /* Default value of water mark level is 8 bytes. */
> > > > + wm_size = 8;
> > > > + size = op->data.nbytes / wm_size;
> > > > + for (i = 0; i < size; i++) {
> > > > + /* Wait for TXFIFO empty */
> > > > + ret = fspi_readl_poll_tout(f, f->iobase + FSPI_INTR,
> > > > + FSPI_INTR_IPTXWE, 0,
> > > > + POLL_TOUT, true);
> > > > + WARN_ON(ret);
> > > > +
> > > > + j = 0;
> > > > + tmp_size = wm_size;
> > > > + while (tmp_size > 0) {
> > > > + data = 0;
> > > > + memcpy(&data, txbuf, 4);
> > > > + fspi_writel(f, data, base + FSPI_TFDR + j * 4);
> > > > + tmp_size -= 4;
> > > > + j++;
> > > > + txbuf += 1;
> > > > + }
> > > > + fspi_writel(f, FSPI_INTR_IPTXWE, base + FSPI_INTR);
> > > > + }
> > > > +
> > > > + size = op->data.nbytes % wm_size;
> > > > + if (size) {
> > > > + /* Wait for TXFIFO empty */
> > > > + ret = fspi_readl_poll_tout(f, f->iobase + FSPI_INTR,
> > > > + FSPI_INTR_IPTXWE, 0,
> > > > + POLL_TOUT, true);
> > > > + WARN_ON(ret);
> > > > +
> > > > + j = 0;
> > > > + tmp_size = 0;
> > > > + while (size > 0) {
> > > > + data = 0;
> > > > + tmp_size = (size < 4) ? size : 4;
> > > > + memcpy(&data, txbuf, tmp_size);
> > > > + fspi_writel(f, data, base + FSPI_TFDR + j * 4);
> > > > + size -= tmp_size;
> > > > + j++;
> > > > + txbuf += 1;
> > > > + }
> > > > + fspi_writel(f, FSPI_INTR_IPTXWE, base + FSPI_INTR);
> > > > + }
> > >
> > > All these nested loops to fill the TX buffer and also the ones below
> > > to read the RX buffer look much more complicated than they should
> > > really be. Can you try to make this more readable?
> > Yes
> > >
> > > Maybe something like this would work:
> > >
> > > for (i = 0; i < ALIGN_DOWN(op->data.nbytes, 8); i += 8) {
> > > /* Wait for TXFIFO empty */
> > > ret = fspi_readl_poll_tout(f, f->iobase + FSPI_INTR,
> > > FSPI_INTR_IPTXWE, 0,
> > > POLL_TOUT, true);
> > >
> > > fspi_writel(f, op->data.buf.out + i, base + FSPI_TFDR);
> > > fspi_writel(f, op->data.buf.out + i + 4, base + FSPI_TFDR + 4);
> > > fspi_writel(f, FSPI_INTR_IPTXWE, base + FSPI_INTR); }
> > With this above 2 lines we are hardcoding it for read/write with watermark
> size as 8 bytes.
> > Watermark size can be variable and depends on the value of
> > IPRXFCR/IPTXFCR register with default value as 8 bytes Thus, I would still
> prefer to use the internal for loop instead of 2 fspi_writel(...) for FSPI_TFDR and
> FSPI_TFDR + 4 register write commands.
>
> Just like you're hardcoding wm_size to 8, so I don't see a difference here. And I
> indeed prefer Frieder's version.
Ok. But, instead of hardcoding and doing fspi_writel() twice, I have modified this as below in my upcoming next version
for (tmp = wm_size, j = 0; tmp > 0; tmp -= 4, j++)
fspi_writel(f, *txbuf++, base + FSPI_TFDR + j * 4);
This would going to give us freedom for future use if watermark size is being used as 16 or 24 (max 64) instead of its current default value of 8.
--
Regards
Yogesh Gaur
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox