* Re: [PATCH v5 03/16] spi: dw: Locally wait for the DMA transactions completion
From: Serge Semin @ 2020-05-29 8:12 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Serge Semin, Mark Brown, Georgy Vlasov, Ramil Zaripov,
Alexey Malahov, Thomas Bogendoerfer, Arnd Bergmann, Feng Tang,
Andy Shevchenko, Rob Herring, linux-mips, devicetree, linux-spi,
Linux Kernel Mailing List
In-Reply-To: <CAHp75VcT2zKnuRW3uxCQtbF0A65cbS20OFpz9sX0hftbjFp1hA@mail.gmail.com>
On Fri, May 29, 2020 at 10:55:32AM +0300, Andy Shevchenko wrote:
> On Fri, May 29, 2020 at 7:02 AM Serge Semin
> <Sergey.Semin@baikalelectronics.ru> wrote:
> >
> > Even if DMA transactions are finished it doesn't mean that the SPI
> > transfers are also completed. It's specifically concerns the Tx-only
> > SPI transfers, since there might be data left in the SPI Tx FIFO after
> > the DMA engine notifies that the Tx DMA procedure is done. In order to
> > completely fix the problem first the driver has to wait for the DMA
> > transaction completion, then for the corresponding SPI operations to be
> > finished. In this commit we implement the former part of the solution.
> >
> > Note we can't just move the SPI operations wait procedure to the DMA
> > completion callbacks, since these callbacks might be executed in the
> > tasklet context (and they will be in case of the DW DMA). In case of
> > slow SPI bus it can cause significant system performance drop.
>
> I read commit message, I read the code. What's going on here since you
> repeated xfer_completion (and its wait routine) from SPI core and I'm
> wondering what happened to it? Why we are not calling
> spi_finalize_current_transfer()?
We discussed that in v4. You complained about using ndelay() for slow SPI bus,
which may cause too long atomic context execution. We agreed. Since we can't wait
in the tasklet context and using a dedicated kernel thread for waiting would be too
much, Me and Mark agreed, that even if it causes us of the local wait-function
re-implementation the best approach would be not to use the generic
spi_transfer_wait() method, but instead wait for the DMA transactions locally
in the DMA driver and just return 0 from the transfer_one callback indicating
that the SPI transfer is finished and there is no need for SPI core to wait. As
a lot of DMA-based SPI drivers do.
If you don't understand what the commit message says, just say so. I'll
reformulate it.
-Sergey
>
> ...
>
> > dws->master->cur_msg->status = -EIO;
> > - spi_finalize_current_transfer(dws->master);
> > + complete(&dws->dma_completion);
> > return IRQ_HANDLED;
>
> ...
>
> > +static int dw_spi_dma_wait(struct dw_spi *dws, struct spi_transfer *xfer)
> > +{
> > + unsigned long long ms;
> > +
> > + ms = xfer->len * MSEC_PER_SEC * BITS_PER_BYTE;
> > + do_div(ms, xfer->effective_speed_hz);
> > + ms += ms + 200;
> > +
> > + if (ms > UINT_MAX)
> > + ms = UINT_MAX;
> > +
> > + ms = wait_for_completion_timeout(&dws->dma_completion,
> > + msecs_to_jiffies(ms));
> > +
> > + if (ms == 0) {
> > + dev_err(&dws->master->cur_msg->spi->dev,
> > + "DMA transaction timed out\n");
> > + return -ETIMEDOUT;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > /*
> > * dws->dma_chan_busy is set before the dma transfer starts, callback for tx
> > * channel will clear a corresponding bit.
> > @@ -155,7 +184,7 @@ static void dw_spi_dma_tx_done(void *arg)
> > return;
> >
> > dw_writel(dws, DW_SPI_DMACR, 0);
> > - spi_finalize_current_transfer(dws->master);
> > + complete(&dws->dma_completion);
> > }
> >
> > static struct dma_async_tx_descriptor *dw_spi_dma_prepare_tx(struct dw_spi *dws,
> > @@ -204,7 +233,7 @@ static void dw_spi_dma_rx_done(void *arg)
> > return;
> >
> > dw_writel(dws, DW_SPI_DMACR, 0);
> > - spi_finalize_current_transfer(dws->master);
> > + complete(&dws->dma_completion);
> > }
>
>
> --
> With Best Regards,
> Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: rtc: add wakeup-source for FlexTimer
From: Alexandre Belloni @ 2020-05-29 8:18 UTC (permalink / raw)
To: Ran Wang
Cc: Alessandro Zummo, Rob Herring, Li Biwen, linux-rtc, devicetree,
linux-kernel
In-Reply-To: <20200529061035.18912-1-ran.wang_1@nxp.com>
On 29/05/2020 14:10:34+0800, Ran Wang wrote:
> Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
> ---
> Documentation/devicetree/bindings/rtc/rtc-fsl-ftm-alarm.txt | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/rtc/rtc-fsl-ftm-alarm.txt b/Documentation/devicetree/bindings/rtc/rtc-fsl-ftm-alarm.txt
> index fffac74..d7c482c 100644
> --- a/Documentation/devicetree/bindings/rtc/rtc-fsl-ftm-alarm.txt
> +++ b/Documentation/devicetree/bindings/rtc/rtc-fsl-ftm-alarm.txt
> @@ -20,6 +20,7 @@ Required properties:
> Optional properties:
> - big-endian: If the host controller is big-endian mode, specify this property.
> The default endian mode is little-endian.
> +- wakeup-source: Enable it as a wakeup source
>
This is already covered by Documentation/devicetree/bindings/rtc/rtc.yaml
> Example:
> rcpm: rcpm@1e34040 {
> @@ -32,5 +33,6 @@ ftm_alarm0: timer@2800000 {
> compatible = "fsl,ls1088a-ftm-alarm";
> reg = <0x0 0x2800000 0x0 0x10000>;
> fsl,rcpm-wakeup = <&rcpm 0x0 0x0 0x0 0x0 0x4000 0x0>;
> + wakeup-source;
> interrupts = <0 44 4>;
> };
> --
> 2.7.4
>
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH 2/2] rtc: fsl-ftm-alarm: fix freeze(s2idle) doesnot wake
From: Alexandre Belloni @ 2020-05-29 8:23 UTC (permalink / raw)
To: Ran Wang
Cc: Alessandro Zummo, Rob Herring, Li Biwen, linux-rtc, devicetree,
linux-kernel
In-Reply-To: <20200529061035.18912-2-ran.wang_1@nxp.com>
On 29/05/2020 14:10:35+0800, Ran Wang wrote:
> Use dev_pm_set_wake_irq() instead of flag IRQF_NO_SUSPEND to enable
> wakeup system feature for both freeze(s2idle) and mem(deep).
>
> Use property 'wakeup-source' to control this feature.
>
> Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
> ---
> drivers/rtc/rtc-fsl-ftm-alarm.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/rtc/rtc-fsl-ftm-alarm.c b/drivers/rtc/rtc-fsl-ftm-alarm.c
> index 756af62..c6945d84 100644
> --- a/drivers/rtc/rtc-fsl-ftm-alarm.c
> +++ b/drivers/rtc/rtc-fsl-ftm-alarm.c
> @@ -21,6 +21,7 @@
> #include <linux/rtc.h>
> #include <linux/time.h>
> #include <linux/acpi.h>
> +#include <linux/pm_wakeirq.h>
>
> #define FTM_SC_CLK(c) ((c) << FTM_SC_CLK_MASK_SHIFT)
>
> @@ -41,6 +42,7 @@ struct ftm_rtc {
> struct rtc_device *rtc_dev;
> void __iomem *base;
> bool big_endian;
> + bool wakeup;
> u32 alarm_freq;
> };
>
> @@ -267,6 +269,9 @@ static int ftm_rtc_probe(struct platform_device *pdev)
> return PTR_ERR(rtc->base);
> }
>
> + rtc->wakeup =
> + device_property_read_bool(&pdev->dev, "wakeup-source");
> +
> irq = platform_get_irq(pdev, 0);
> if (irq < 0) {
> dev_err(&pdev->dev, "can't get irq number\n");
> @@ -274,7 +279,7 @@ static int ftm_rtc_probe(struct platform_device *pdev)
> }
>
> ret = devm_request_irq(&pdev->dev, irq, ftm_rtc_alarm_interrupt,
> - IRQF_NO_SUSPEND, dev_name(&pdev->dev), rtc);
> + 0, dev_name(&pdev->dev), rtc);
> if (ret < 0) {
> dev_err(&pdev->dev, "failed to request irq\n");
> return ret;
> @@ -286,7 +291,10 @@ static int ftm_rtc_probe(struct platform_device *pdev)
> rtc->alarm_freq = (u32)FIXED_FREQ_CLK / (u32)MAX_FREQ_DIV;
> rtc->rtc_dev->ops = &ftm_rtc_ops;
>
> - device_init_wakeup(&pdev->dev, true);
> + device_init_wakeup(&pdev->dev, rtc->wakeup);
As long as you have an irq, you should be able to wakeup, do you really
need the wakeup-source property?
Usually, wakeup-source is used when the RTC interrupt line is not
connected directly to the SoC but is still able to wake it up.
In your case, is it to cover the case of the flex timers that can't
wake the CPU? If so, then please be more explicit in your commit
message.
> + ret = dev_pm_set_wake_irq(&pdev->dev, irq);
> + if (ret)
> + dev_err(&pdev->dev, "irq wake enable failed.\n");
>
> ret = rtc_register_device(rtc->rtc_dev);
> if (ret) {
> --
> 2.7.4
>
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH v7 16/20] mtd: nand: Convert generic NAND bits to use the ECC framework
From: Boris Brezillon @ 2020-05-29 8:32 UTC (permalink / raw)
To: Miquel Raynal
Cc: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd,
Rob Herring, Mark Rutland, devicetree, Thomas Petazzoni,
Paul Cercueil, Chuanhong Guo, Weijie Gao, linux-arm-kernel,
Mason Yang, Julien Su
In-Reply-To: <20200529002517.3546-17-miquel.raynal@bootlin.com>
On Fri, 29 May 2020 02:25:13 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Embed a generic NAND ECC high-level object in the nand_device
> structure to carry all the ECC engine configuration/data. Adapt the
> raw NAND and SPI-NAND cores to fit the change.
In order to split that one, and make future re-organizations less
painful (hope we won't have to do that again, but who knows), I would
recommend doing things in this order:
1/ create a nanddev_get_ecc_requirements() helper that returns a
const struct nand_ecc_props *
2/ patch spinand to use this helper
3/ introduce nand_ecc
4/ patch rawnand to use the new ecc layer
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> drivers/mtd/nand/raw/atmel/nand-controller.c | 9 +++--
> drivers/mtd/nand/raw/brcmnand/brcmnand.c | 7 ++--
> drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c | 12 +++---
> drivers/mtd/nand/raw/marvell_nand.c | 7 ++--
> drivers/mtd/nand/raw/mtk_nand.c | 4 +-
> drivers/mtd/nand/raw/nand_base.c | 25 ++++++------
> drivers/mtd/nand/raw/nand_esmt.c | 11 +++---
> drivers/mtd/nand/raw/nand_hynix.c | 41 ++++++++++----------
> drivers/mtd/nand/raw/nand_jedec.c | 4 +-
> drivers/mtd/nand/raw/nand_micron.c | 14 ++++---
> drivers/mtd/nand/raw/nand_onfi.c | 8 ++--
> drivers/mtd/nand/raw/nand_samsung.c | 19 ++++-----
> drivers/mtd/nand/raw/nand_toshiba.c | 11 +++---
> drivers/mtd/nand/raw/sunxi_nand.c | 5 ++-
> drivers/mtd/nand/raw/tegra_nand.c | 9 +++--
> drivers/mtd/nand/spi/core.c | 8 ++--
> drivers/mtd/nand/spi/macronix.c | 6 +--
> drivers/mtd/nand/spi/toshiba.c | 6 +--
> include/linux/mtd/nand.h | 8 ++--
> 19 files changed, 114 insertions(+), 100 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
> index 85cf396731ce..2ebcf3087d8d 100644
> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> @@ -1043,6 +1043,7 @@ static int atmel_hsmc_nand_pmecc_read_page_raw(struct nand_chip *chip,
>
> static int atmel_nand_pmecc_init(struct nand_chip *chip)
> {
> + struct nand_ecc_props *requirements = &chip->base.ecc.requirements;
> struct mtd_info *mtd = nand_to_mtd(chip);
> struct atmel_nand *nand = to_atmel_nand(chip);
> struct atmel_nand_controller *nc;
> @@ -1072,15 +1073,15 @@ static int atmel_nand_pmecc_init(struct nand_chip *chip)
> req.ecc.strength = ATMEL_PMECC_MAXIMIZE_ECC_STRENGTH;
> else if (chip->ecc.strength)
> req.ecc.strength = chip->ecc.strength;
> - else if (chip->base.eccreq.strength)
> - req.ecc.strength = chip->base.eccreq.strength;
> + else if (requirements->strength)
> + req.ecc.strength = requirements->strength;
> else
> req.ecc.strength = ATMEL_PMECC_MAXIMIZE_ECC_STRENGTH;
>
> if (chip->ecc.size)
> req.ecc.sectorsize = chip->ecc.size;
> - else if (chip->base.eccreq.step_size)
> - req.ecc.sectorsize = chip->base.eccreq.step_size;
> + else if (requirements->step_size)
> + req.ecc.sectorsize = requirements->step_size;
> else
> req.ecc.sectorsize = ATMEL_PMECC_SECTOR_SIZE_AUTO;
>
> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> index 164617b33942..40f6d107ffa2 100644
> --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> @@ -2563,10 +2563,11 @@ static int brcmnand_setup_dev(struct brcmnand_host *host)
>
> if (chip->ecc.engine_type != NAND_ECC_ENGINE_TYPE_NONE &&
> (!chip->ecc.size || !chip->ecc.strength)) {
> - if (chip->base.eccreq.step_size && chip->base.eccreq.strength) {
> + if (chip->base.ecc.requirements.step_size &&
> + chip->base.ecc.requirements.strength) {
> /* use detected ECC parameters */
> - chip->ecc.size = chip->base.eccreq.step_size;
> - chip->ecc.strength = chip->base.eccreq.strength;
> + chip->ecc.size = chip->base.ecc.requirements.step_size;
> + chip->ecc.strength = chip->base.ecc.requirements.strength;
> dev_info(ctrl->dev, "Using ECC step-size %d, strength %d\n",
> chip->ecc.size, chip->ecc.strength);
> }
> diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> index d1ea6df9fd64..5f56570ce04d 100644
> --- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> @@ -272,8 +272,8 @@ static int set_geometry_by_ecc_info(struct gpmi_nand_data *this,
> default:
> dev_err(this->dev,
> "unsupported nand chip. ecc bits : %d, ecc size : %d\n",
> - chip->base.eccreq.strength,
> - chip->base.eccreq.step_size);
> + chip->base.ecc.requirements.strength,
> + chip->base.ecc.requirements.step_size);
> return -EINVAL;
> }
> geo->ecc_chunk_size = ecc_step;
> @@ -517,13 +517,13 @@ static int common_nfc_set_geometry(struct gpmi_nand_data *this)
>
> if ((of_property_read_bool(this->dev->of_node, "fsl,use-minimum-ecc"))
> || legacy_set_geometry(this)) {
> - if (!(chip->base.eccreq.strength > 0 &&
> - chip->base.eccreq.step_size > 0))
> + if (!(chip->base.ecc.requirements.strength > 0 &&
> + chip->base.ecc.requirements.step_size > 0))
> return -EINVAL;
>
> return set_geometry_by_ecc_info(this,
> - chip->base.eccreq.strength,
> - chip->base.eccreq.step_size);
> + chip->base.ecc.requirements.strength,
> + chip->base.ecc.requirements.step_size);
> }
>
> return 0;
> diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c
> index f9cc03c11deb..e7a269461a6d 100644
> --- a/drivers/mtd/nand/raw/marvell_nand.c
> +++ b/drivers/mtd/nand/raw/marvell_nand.c
> @@ -2244,14 +2244,15 @@ static int marvell_nand_ecc_init(struct mtd_info *mtd,
> struct nand_ecc_ctrl *ecc)
> {
> struct nand_chip *chip = mtd_to_nand(mtd);
> + struct nand_ecc_props *requirements = &chip->base.ecc.requirements;
> struct marvell_nfc *nfc = to_marvell_nfc(chip->controller);
> int ret;
>
> if (ecc->engine_type != NAND_ECC_ENGINE_TYPE_NONE &&
> (!ecc->size || !ecc->strength)) {
> - if (chip->base.eccreq.step_size && chip->base.eccreq.strength) {
> - ecc->size = chip->base.eccreq.step_size;
> - ecc->strength = chip->base.eccreq.strength;
> + if (requirements->step_size && requirements->strength) {
> + ecc->size = requirements->step_size;
> + ecc->strength = requirements->strength;
> } else {
> dev_info(nfc->dev,
> "No minimum ECC strength, using 1b/512B\n");
> diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
> index a0294c9161dd..1f8dbae38286 100644
> --- a/drivers/mtd/nand/raw/mtk_nand.c
> +++ b/drivers/mtd/nand/raw/mtk_nand.c
> @@ -1234,8 +1234,8 @@ static int mtk_nfc_ecc_init(struct device *dev, struct mtd_info *mtd)
> /* if optional dt settings not present */
> if (!nand->ecc.size || !nand->ecc.strength) {
> /* use datasheet requirements */
> - nand->ecc.strength = nand->base.eccreq.strength;
> - nand->ecc.size = nand->base.eccreq.step_size;
> + nand->ecc.strength = nand->base.ecc.requirements.strength;
> + nand->ecc.size = nand->base.ecc.requirements.step_size;
>
> /*
> * align eccstrength and eccsize
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 1ce2cbe72e4c..bc2d5d2e8f4c 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -4746,8 +4746,8 @@ static bool find_full_id_nand(struct nand_chip *chip,
> memorg->pagesize *
> memorg->pages_per_eraseblock);
> chip->options |= type->options;
> - chip->base.eccreq.strength = NAND_ECC_STRENGTH(type);
> - chip->base.eccreq.step_size = NAND_ECC_STEP(type);
> + chip->base.ecc.requirements.strength = NAND_ECC_STRENGTH(type);
> + chip->base.ecc.requirements.step_size = NAND_ECC_STEP(type);
> chip->onfi_timing_mode_default =
> type->onfi_timing_mode_default;
>
> @@ -5485,8 +5485,8 @@ nand_match_ecc_req(struct nand_chip *chip,
> {
> struct mtd_info *mtd = nand_to_mtd(chip);
> const struct nand_ecc_step_info *stepinfo;
> - int req_step = chip->base.eccreq.step_size;
> - int req_strength = chip->base.eccreq.strength;
> + int req_step = chip->base.ecc.requirements.step_size;
> + int req_strength = chip->base.ecc.requirements.strength;
> int req_corr, step_size, strength, nsteps, ecc_bytes, ecc_bytes_total;
> int best_step, best_strength, best_ecc_bytes;
> int best_ecc_bytes_total = INT_MAX;
> @@ -5677,9 +5677,10 @@ static bool nand_ecc_strength_good(struct nand_chip *chip)
> {
> struct mtd_info *mtd = nand_to_mtd(chip);
> struct nand_ecc_ctrl *ecc = &chip->ecc;
> + struct nand_ecc_props *requirements = &chip->base.ecc.requirements;
> int corr, ds_corr;
>
> - if (ecc->size == 0 || chip->base.eccreq.step_size == 0)
> + if (ecc->size == 0 || requirements->step_size == 0)
> /* Not enough information */
> return true;
>
> @@ -5688,10 +5689,10 @@ static bool nand_ecc_strength_good(struct nand_chip *chip)
> * the correction density.
> */
> corr = (mtd->writesize * ecc->strength) / ecc->size;
> - ds_corr = (mtd->writesize * chip->base.eccreq.strength) /
> - chip->base.eccreq.step_size;
> + ds_corr = (mtd->writesize * requirements->strength) /
> + requirements->step_size;
>
> - return corr >= ds_corr && ecc->strength >= chip->base.eccreq.strength;
> + return corr >= ds_corr && ecc->strength >= requirements->strength;
> }
>
> static int rawnand_erase(struct nand_device *nand, const struct nand_pos *pos)
> @@ -5977,9 +5978,11 @@ static int nand_scan_tail(struct nand_chip *chip)
> /* ECC sanity check: warn if it's too weak */
> if (!nand_ecc_strength_good(chip))
> pr_warn("WARNING: %s: the ECC used on your system (%db/%dB) is too weak compared to the one required by the NAND chip (%db/%dB)\n",
> - mtd->name, chip->ecc.strength, chip->ecc.size,
> - chip->base.eccreq.strength,
> - chip->base.eccreq.step_size);
> + mtd->name,
> + chip->base.ecc.ctx.conf.strength,
> + chip->base.ecc.ctx.conf.step_size,
> + chip->base.ecc.requirements.strength,
> + chip->base.ecc.requirements.step_size);
>
> /* Allow subpage writes up to ecc.steps. Not possible for MLC flash */
> if (!(chip->options & NAND_NO_SUBPAGE_WRITE) && nand_is_slc(chip)) {
> diff --git a/drivers/mtd/nand/raw/nand_esmt.c b/drivers/mtd/nand/raw/nand_esmt.c
> index 3338c68aaaf1..11f25ec3e4fc 100644
> --- a/drivers/mtd/nand/raw/nand_esmt.c
> +++ b/drivers/mtd/nand/raw/nand_esmt.c
> @@ -10,24 +10,25 @@
>
> static void esmt_nand_decode_id(struct nand_chip *chip)
> {
> + struct nand_ecc_props *requirements = &chip->base.ecc.requirements;
> nand_decode_ext_id(chip);
>
> /* Extract ECC requirements from 5th id byte. */
> if (chip->id.len >= 5 && nand_is_slc(chip)) {
> - chip->base.eccreq.step_size = 512;
> + requirements->step_size = 512;
> switch (chip->id.data[4] & 0x3) {
> case 0x0:
> - chip->base.eccreq.strength = 4;
> + requirements->strength = 4;
> break;
> case 0x1:
> - chip->base.eccreq.strength = 2;
> + requirements->strength = 2;
> break;
> case 0x2:
> - chip->base.eccreq.strength = 1;
> + requirements->strength = 1;
> break;
> default:
> WARN(1, "Could not get ECC info");
> - chip->base.eccreq.step_size = 0;
> + requirements->step_size = 0;
> break;
> }
> }
> diff --git a/drivers/mtd/nand/raw/nand_hynix.c b/drivers/mtd/nand/raw/nand_hynix.c
> index 7caedaa5b9e5..bac7732a86e9 100644
> --- a/drivers/mtd/nand/raw/nand_hynix.c
> +++ b/drivers/mtd/nand/raw/nand_hynix.c
> @@ -495,34 +495,35 @@ static void hynix_nand_extract_oobsize(struct nand_chip *chip,
> static void hynix_nand_extract_ecc_requirements(struct nand_chip *chip,
> bool valid_jedecid)
> {
> + struct nand_ecc_props *requirements = &chip->base.ecc.requirements;
> u8 ecc_level = (chip->id.data[4] >> 4) & 0x7;
>
> if (valid_jedecid) {
> /* Reference: H27UCG8T2E datasheet */
> - chip->base.eccreq.step_size = 1024;
> + requirements->step_size = 1024;
>
> switch (ecc_level) {
> case 0:
> - chip->base.eccreq.step_size = 0;
> - chip->base.eccreq.strength = 0;
> + requirements->step_size = 0;
> + requirements->strength = 0;
> break;
> case 1:
> - chip->base.eccreq.strength = 4;
> + requirements->strength = 4;
> break;
> case 2:
> - chip->base.eccreq.strength = 24;
> + requirements->strength = 24;
> break;
> case 3:
> - chip->base.eccreq.strength = 32;
> + requirements->strength = 32;
> break;
> case 4:
> - chip->base.eccreq.strength = 40;
> + requirements->strength = 40;
> break;
> case 5:
> - chip->base.eccreq.strength = 50;
> + requirements->strength = 50;
> break;
> case 6:
> - chip->base.eccreq.strength = 60;
> + requirements->strength = 60;
> break;
> default:
> /*
> @@ -543,14 +544,14 @@ static void hynix_nand_extract_ecc_requirements(struct nand_chip *chip,
> if (nand_tech < 3) {
> /* > 26nm, reference: H27UBG8T2A datasheet */
> if (ecc_level < 5) {
> - chip->base.eccreq.step_size = 512;
> - chip->base.eccreq.strength = 1 << ecc_level;
> + requirements->step_size = 512;
> + requirements->strength = 1 << ecc_level;
> } else if (ecc_level < 7) {
> if (ecc_level == 5)
> - chip->base.eccreq.step_size = 2048;
> + requirements->step_size = 2048;
> else
> - chip->base.eccreq.step_size = 1024;
> - chip->base.eccreq.strength = 24;
> + requirements->step_size = 1024;
> + requirements->strength = 24;
> } else {
> /*
> * We should never reach this case, but if that
> @@ -563,14 +564,14 @@ static void hynix_nand_extract_ecc_requirements(struct nand_chip *chip,
> } else {
> /* <= 26nm, reference: H27UBG8T2B datasheet */
> if (!ecc_level) {
> - chip->base.eccreq.step_size = 0;
> - chip->base.eccreq.strength = 0;
> + requirements->step_size = 0;
> + requirements->strength = 0;
> } else if (ecc_level < 5) {
> - chip->base.eccreq.step_size = 512;
> - chip->base.eccreq.strength = 1 << (ecc_level - 1);
> + requirements->step_size = 512;
> + requirements->strength = 1 << (ecc_level - 1);
> } else {
> - chip->base.eccreq.step_size = 1024;
> - chip->base.eccreq.strength = 24 +
> + requirements->step_size = 1024;
> + requirements->strength = 24 +
> (8 * (ecc_level - 5));
> }
> }
> diff --git a/drivers/mtd/nand/raw/nand_jedec.c b/drivers/mtd/nand/raw/nand_jedec.c
> index b15c42f48755..5ed3656d88dd 100644
> --- a/drivers/mtd/nand/raw/nand_jedec.c
> +++ b/drivers/mtd/nand/raw/nand_jedec.c
> @@ -120,8 +120,8 @@ int nand_jedec_detect(struct nand_chip *chip)
> ecc = &p->ecc_info[0];
>
> if (ecc->codeword_size >= 9) {
> - chip->base.eccreq.strength = ecc->ecc_bits;
> - chip->base.eccreq.step_size = 1 << ecc->codeword_size;
> + chip->base.ecc.requirements.strength = ecc->ecc_bits;
> + chip->base.ecc.requirements.step_size = 1 << ecc->codeword_size;
> } else {
> pr_warn("Invalid codeword size\n");
> }
> diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c
> index c8ebfd8c77a1..d1dc684ecd6c 100644
> --- a/drivers/mtd/nand/raw/nand_micron.c
> +++ b/drivers/mtd/nand/raw/nand_micron.c
> @@ -413,6 +413,7 @@ enum {
> */
> static int micron_supports_on_die_ecc(struct nand_chip *chip)
> {
> + struct nand_ecc_props *requirements = &chip->base.ecc.requirements;
> u8 id[5];
> int ret;
>
> @@ -425,7 +426,7 @@ static int micron_supports_on_die_ecc(struct nand_chip *chip)
> /*
> * We only support on-die ECC of 4/512 or 8/512
> */
> - if (chip->base.eccreq.strength != 4 && chip->base.eccreq.strength != 8)
> + if (requirements->strength != 4 && requirements->strength != 8)
> return MICRON_ON_DIE_UNSUPPORTED;
>
> /* 0x2 means on-die ECC is available. */
> @@ -466,7 +467,7 @@ static int micron_supports_on_die_ecc(struct nand_chip *chip)
> /*
> * We only support on-die ECC of 4/512 or 8/512
> */
> - if (chip->base.eccreq.strength != 4 && chip->base.eccreq.strength != 8)
> + if (requirements->strength != 4 && requirements->strength != 8)
> return MICRON_ON_DIE_UNSUPPORTED;
>
> return MICRON_ON_DIE_SUPPORTED;
> @@ -474,6 +475,7 @@ static int micron_supports_on_die_ecc(struct nand_chip *chip)
>
> static int micron_nand_init(struct nand_chip *chip)
> {
> + struct nand_ecc_props *requirements = &chip->base.ecc.requirements;
> struct mtd_info *mtd = nand_to_mtd(chip);
> struct micron_nand *micron;
> int ondie;
> @@ -523,7 +525,7 @@ static int micron_nand_init(struct nand_chip *chip)
> * That's not needed for 8-bit ECC, because the status expose
> * a better approximation of the number of bitflips in a page.
> */
> - if (chip->base.eccreq.strength == 4) {
> + if (requirements->strength == 4) {
> micron->ecc.rawbuf = kmalloc(mtd->writesize +
> mtd->oobsize,
> GFP_KERNEL);
> @@ -533,16 +535,16 @@ static int micron_nand_init(struct nand_chip *chip)
> }
> }
>
> - if (chip->base.eccreq.strength == 4)
> + if (requirements->strength == 4)
> mtd_set_ooblayout(mtd,
> µn_nand_on_die_4_ooblayout_ops);
> else
> mtd_set_ooblayout(mtd,
> µn_nand_on_die_8_ooblayout_ops);
>
> - chip->ecc.bytes = chip->base.eccreq.strength * 2;
> + chip->ecc.bytes = requirements->strength * 2;
> chip->ecc.size = 512;
> - chip->ecc.strength = chip->base.eccreq.strength;
> + chip->ecc.strength = requirements->strength;
> chip->ecc.algo = NAND_ECC_ALGO_BCH;
> chip->ecc.read_page = micron_nand_read_page_on_die_ecc;
> chip->ecc.write_page = micron_nand_write_page_on_die_ecc;
> diff --git a/drivers/mtd/nand/raw/nand_onfi.c b/drivers/mtd/nand/raw/nand_onfi.c
> index be3456627288..c9ae29774c1f 100644
> --- a/drivers/mtd/nand/raw/nand_onfi.c
> +++ b/drivers/mtd/nand/raw/nand_onfi.c
> @@ -94,8 +94,8 @@ static int nand_flash_detect_ext_param_page(struct nand_chip *chip,
> goto ext_out;
> }
>
> - chip->base.eccreq.strength = ecc->ecc_bits;
> - chip->base.eccreq.step_size = 1 << ecc->codeword_size;
> + chip->base.ecc.requirements.strength = ecc->ecc_bits;
> + chip->base.ecc.requirements.step_size = 1 << ecc->codeword_size;
> ret = 0;
>
> ext_out:
> @@ -265,8 +265,8 @@ int nand_onfi_detect(struct nand_chip *chip)
> chip->options |= NAND_BUSWIDTH_16;
>
> if (p->ecc_bits != 0xff) {
> - chip->base.eccreq.strength = p->ecc_bits;
> - chip->base.eccreq.step_size = 512;
> + chip->base.ecc.requirements.strength = p->ecc_bits;
> + chip->base.ecc.requirements.step_size = 512;
> } else if (onfi_version >= 21 &&
> (le16_to_cpu(p->features) & ONFI_FEATURE_EXT_PARAM_PAGE)) {
>
> diff --git a/drivers/mtd/nand/raw/nand_samsung.c b/drivers/mtd/nand/raw/nand_samsung.c
> index 3a4a19e808f6..0ee85e88aeb5 100644
> --- a/drivers/mtd/nand/raw/nand_samsung.c
> +++ b/drivers/mtd/nand/raw/nand_samsung.c
> @@ -10,6 +10,7 @@
>
> static void samsung_nand_decode_id(struct nand_chip *chip)
> {
> + struct nand_ecc_props *requirements = &chip->base.ecc.requirements;
> struct mtd_info *mtd = nand_to_mtd(chip);
> struct nand_memory_organization *memorg;
>
> @@ -71,23 +72,23 @@ static void samsung_nand_decode_id(struct nand_chip *chip)
> /* Extract ECC requirements from 5th id byte*/
> extid = (chip->id.data[4] >> 4) & 0x07;
> if (extid < 5) {
> - chip->base.eccreq.step_size = 512;
> - chip->base.eccreq.strength = 1 << extid;
> + requirements->step_size = 512;
> + requirements->strength = 1 << extid;
> } else {
> - chip->base.eccreq.step_size = 1024;
> + requirements->step_size = 1024;
> switch (extid) {
> case 5:
> - chip->base.eccreq.strength = 24;
> + requirements->strength = 24;
> break;
> case 6:
> - chip->base.eccreq.strength = 40;
> + requirements->strength = 40;
> break;
> case 7:
> - chip->base.eccreq.strength = 60;
> + requirements->strength = 60;
> break;
> default:
> WARN(1, "Could not decode ECC info");
> - chip->base.eccreq.step_size = 0;
> + requirements->step_size = 0;
> }
> }
> } else {
> @@ -97,8 +98,8 @@ static void samsung_nand_decode_id(struct nand_chip *chip)
> switch (chip->id.data[1]) {
> /* K9F4G08U0D-S[I|C]B0(T00) */
> case 0xDC:
> - chip->base.eccreq.step_size = 512;
> - chip->base.eccreq.strength = 1;
> + requirements->step_size = 512;
> + requirements->strength = 1;
> break;
>
> /* K9F1G08U0E 21nm chips do not support subpage write */
> diff --git a/drivers/mtd/nand/raw/nand_toshiba.c b/drivers/mtd/nand/raw/nand_toshiba.c
> index 436ed90a90ad..1180068007a9 100644
> --- a/drivers/mtd/nand/raw/nand_toshiba.c
> +++ b/drivers/mtd/nand/raw/nand_toshiba.c
> @@ -145,6 +145,7 @@ static void toshiba_nand_benand_init(struct nand_chip *chip)
>
> static void toshiba_nand_decode_id(struct nand_chip *chip)
> {
> + struct nand_ecc_props *requirements = &chip->base.ecc.requirements;
> struct mtd_info *mtd = nand_to_mtd(chip);
> struct nand_memory_organization *memorg;
>
> @@ -175,20 +176,20 @@ static void toshiba_nand_decode_id(struct nand_chip *chip)
> * - 24nm: 8 bit ECC for each 512Byte is required.
> */
> if (chip->id.len >= 6 && nand_is_slc(chip)) {
> - chip->base.eccreq.step_size = 512;
> + requirements->step_size = 512;
> switch (chip->id.data[5] & 0x7) {
> case 0x4:
> - chip->base.eccreq.strength = 1;
> + requirements->strength = 1;
> break;
> case 0x5:
> - chip->base.eccreq.strength = 4;
> + requirements->strength = 4;
> break;
> case 0x6:
> - chip->base.eccreq.strength = 8;
> + requirements->strength = 8;
> break;
> default:
> WARN(1, "Could not get ECC info");
> - chip->base.eccreq.step_size = 0;
> + requirements->step_size = 0;
> break;
> }
> }
> diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
> index c6dd2e6d9ef8..a5eefdf89660 100644
> --- a/drivers/mtd/nand/raw/sunxi_nand.c
> +++ b/drivers/mtd/nand/raw/sunxi_nand.c
> @@ -1732,6 +1732,7 @@ static void sunxi_nand_ecc_cleanup(struct nand_ecc_ctrl *ecc)
>
> static int sunxi_nand_attach_chip(struct nand_chip *nand)
> {
> + struct nand_ecc_props *requirements = &nand->base.ecc.requirements;
> struct nand_ecc_ctrl *ecc = &nand->ecc;
> struct device_node *np = nand_get_flash_node(nand);
> int ret;
> @@ -1745,8 +1746,8 @@ static int sunxi_nand_attach_chip(struct nand_chip *nand)
> nand->options |= NAND_SUBPAGE_READ;
>
> if (!ecc->size) {
> - ecc->size = nand->base.eccreq.step_size;
> - ecc->strength = nand->base.eccreq.strength;
> + ecc->size = requirements->step_size;
> + ecc->strength = requirements->strength;
> }
>
> if (!ecc->size || !ecc->strength)
> diff --git a/drivers/mtd/nand/raw/tegra_nand.c b/drivers/mtd/nand/raw/tegra_nand.c
> index 2325b06ccc9a..8264bb991d03 100644
> --- a/drivers/mtd/nand/raw/tegra_nand.c
> +++ b/drivers/mtd/nand/raw/tegra_nand.c
> @@ -855,7 +855,7 @@ static int tegra_nand_get_strength(struct nand_chip *chip, const int *strength,
> } else {
> strength_sel = strength[i];
>
> - if (strength_sel < chip->base.eccreq.strength)
> + if (strength_sel < chip->base.ecc.requirements.strength)
> continue;
> }
>
> @@ -908,6 +908,7 @@ static int tegra_nand_select_strength(struct nand_chip *chip, int oobsize)
> static int tegra_nand_attach_chip(struct nand_chip *chip)
> {
> struct tegra_nand_controller *ctrl = to_tegra_ctrl(chip->controller);
> + struct nand_ecc_props *requirements = &chip->base.ecc.requirements;
> struct tegra_nand_chip *nand = to_tegra_chip(chip);
> struct mtd_info *mtd = nand_to_mtd(chip);
> int bits_per_step;
> @@ -919,9 +920,9 @@ static int tegra_nand_attach_chip(struct nand_chip *chip)
> chip->ecc.engine_type = NAND_ECC_ENGINE_TYPE_ON_HOST;
> chip->ecc.size = 512;
> chip->ecc.steps = mtd->writesize / chip->ecc.size;
> - if (chip->base.eccreq.step_size != 512) {
> + if (requirements->step_size != 512) {
> dev_err(ctrl->dev, "Unsupported step size %d\n",
> - chip->base.eccreq.step_size);
> + requirements->step_size);
> return -EINVAL;
> }
>
> @@ -952,7 +953,7 @@ static int tegra_nand_attach_chip(struct nand_chip *chip)
> if (ret < 0) {
> dev_err(ctrl->dev,
> "No valid strength found, minimum %d\n",
> - chip->base.eccreq.strength);
> + requirements->strength);
> return ret;
> }
>
> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
> index 6f6ec8aa143d..edc8ec2923d5 100644
> --- a/drivers/mtd/nand/spi/core.c
> +++ b/drivers/mtd/nand/spi/core.c
> @@ -419,7 +419,7 @@ static int spinand_check_ecc_status(struct spinand_device *spinand, u8 status)
> * fixed, so let's return the maximum possible value so that
> * wear-leveling layers move the data immediately.
> */
> - return nand->eccreq.strength;
> + return nand->ecc.ctx.conf.strength;
>
> case STATUS_ECC_UNCOR_ERROR:
> return -EBADMSG;
> @@ -903,7 +903,7 @@ int spinand_match_and_init(struct spinand_device *spinand,
> continue;
>
> nand->memorg = table[i].memorg;
> - nand->eccreq = table[i].eccreq;
> + nand->ecc.requirements = table[i].eccreq;
> spinand->eccinfo = table[i].eccinfo;
> spinand->flags = table[i].flags;
> spinand->id.len = 1 + table[i].devid.len;
> @@ -1091,8 +1091,8 @@ static int spinand_init(struct spinand_device *spinand)
> mtd->oobavail = ret;
>
> /* Propagate ECC information to mtd_info */
> - mtd->ecc_strength = nand->eccreq.strength;
> - mtd->ecc_step_size = nand->eccreq.step_size;
> + mtd->ecc_strength = nand->ecc.ctx.conf.strength;
> + mtd->ecc_step_size = nand->ecc.ctx.conf.step_size;
>
> return 0;
>
> diff --git a/drivers/mtd/nand/spi/macronix.c b/drivers/mtd/nand/spi/macronix.c
> index 0f900f3aa21a..9db55828a995 100644
> --- a/drivers/mtd/nand/spi/macronix.c
> +++ b/drivers/mtd/nand/spi/macronix.c
> @@ -84,10 +84,10 @@ static int mx35lf1ge4ab_ecc_get_status(struct spinand_device *spinand,
> * data around if it's not necessary.
> */
> if (mx35lf1ge4ab_get_eccsr(spinand, &eccsr))
> - return nand->eccreq.strength;
> + return nand->ecc.ctx.conf.strength;
>
> - if (WARN_ON(eccsr > nand->eccreq.strength || !eccsr))
> - return nand->eccreq.strength;
> + if (WARN_ON(eccsr > nand->ecc.ctx.conf.strength || !eccsr))
> + return nand->ecc.ctx.conf.strength;
>
> return eccsr;
>
> diff --git a/drivers/mtd/nand/spi/toshiba.c b/drivers/mtd/nand/spi/toshiba.c
> index bc801d83343e..c3e5b1a85e3e 100644
> --- a/drivers/mtd/nand/spi/toshiba.c
> +++ b/drivers/mtd/nand/spi/toshiba.c
> @@ -90,12 +90,12 @@ static int tx58cxgxsxraix_ecc_get_status(struct spinand_device *spinand,
> * data around if it's not necessary.
> */
> if (spi_mem_exec_op(spinand->spimem, &op))
> - return nand->eccreq.strength;
> + return nand->ecc.ctx.conf.strength;
>
> mbf >>= 4;
>
> - if (WARN_ON(mbf > nand->eccreq.strength || !mbf))
> - return nand->eccreq.strength;
> + if (WARN_ON(mbf > nand->ecc.ctx.conf.strength || !mbf))
> + return nand->ecc.ctx.conf.strength;
>
> return mbf;
>
> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> index 2f838394b5f7..975ddf26ac8c 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -290,7 +290,7 @@ struct nand_ecc {
> * struct nand_device - NAND device
> * @mtd: MTD instance attached to the NAND device
> * @memorg: memory layout
> - * @eccreq: ECC requirements
> + * @ecc: NAND ECC object attached to the NAND device
> * @rowconv: position to row address converter
> * @bbt: bad block table info
> * @ops: NAND operations attached to the NAND device
> @@ -298,8 +298,8 @@ struct nand_ecc {
> * Generic NAND object. Specialized NAND layers (raw NAND, SPI NAND, OneNAND)
> * should declare their own NAND object embedding a nand_device struct (that's
> * how inheritance is done).
> - * struct_nand_device->memorg and struct_nand_device->eccreq should be filled
> - * at device detection time to reflect the NAND device
> + * struct_nand_device->memorg and struct_nand_device->ecc.ctx.conf should
> + * be filled at device detection time to reflect the NAND device
> * capabilities/requirements. Once this is done nanddev_init() can be called.
> * It will take care of converting NAND information into MTD ones, which means
> * the specialized NAND layers should never manually tweak
> @@ -308,7 +308,7 @@ struct nand_ecc {
> struct nand_device {
> struct mtd_info mtd;
> struct nand_memory_organization memorg;
> - struct nand_ecc_props eccreq;
> + struct nand_ecc ecc;
> struct nand_row_converter rowconv;
> struct nand_bbt bbt;
> const struct nand_ops *ops;
^ permalink raw reply
* Re: [PATCH v7 16/20] mtd: nand: Convert generic NAND bits to use the ECC framework
From: Boris Brezillon @ 2020-05-29 8:35 UTC (permalink / raw)
To: Miquel Raynal
Cc: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd,
Rob Herring, Mark Rutland, devicetree, Thomas Petazzoni,
Paul Cercueil, Chuanhong Guo, Weijie Gao, linux-arm-kernel,
Mason Yang, Julien Su
In-Reply-To: <20200529002517.3546-17-miquel.raynal@bootlin.com>
On Fri, 29 May 2020 02:25:13 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> - * struct_nand_device->memorg and struct_nand_device->eccreq should be filled
> - * at device detection time to reflect the NAND device
> + * struct_nand_device->memorg and struct_nand_device->ecc.ctx.conf should
> + * be filled at device detection time to reflect the NAND device
Is it not struct_nand_device->ecc.requirements?
^ permalink raw reply
* Re: [PATCH 1/3] dt-bindings: mips: Document two Loongson generic boards
From: WANG Xuerui @ 2020-05-29 8:37 UTC (permalink / raw)
To: Jiaxun Yang, maz
Cc: Thomas Bogendoerfer, Rob Herring, Huacai Chen, linux-mips,
devicetree, linux-kernel
In-Reply-To: <20200529034338.1137776-2-jiaxun.yang@flygoat.com>
Hi Jiaxun,
On 2020/5/29 11:43, Jiaxun Yang wrote:
> Document loongson3-8core-ls7a and loongson3-r4-ls7a, with
> two boards LS7A PCH.
"with two boards LS7A PCH" -- maybe you mean "two boards with LS7A PCH"?
>
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
> .../devicetree/bindings/mips/loongson/devices.yaml | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mips/loongson/devices.yaml b/Documentation/devicetree/bindings/mips/loongson/devices.yaml
> index 74ed4e397a78..6164b0fcb493 100644
> --- a/Documentation/devicetree/bindings/mips/loongson/devices.yaml
> +++ b/Documentation/devicetree/bindings/mips/loongson/devices.yaml
> @@ -24,4 +24,12 @@ properties:
> - description: Generic Loongson3 Octa Core + RS780E
> items:
> - const: loongson,loongson3-8core-rs780e
> +
> + - description: Generic Loongson3 Quad Core + LS7A
> + items:
> + - const: loongson,loongson3-8core-ls7a
> +
> + - description: Generic Loongson3 Release 4 + LS7A
"R4" instead of "Release 4", as in /proc/cpuinfo output?
> + items:
> + - const: loongson,loongson3-r4-ls7a
> ...
^ permalink raw reply
* Re: [PATCH v7 17/20] mtd: rawnand: Hide the generic OOB layout objects behind helpers
From: Boris Brezillon @ 2020-05-29 8:45 UTC (permalink / raw)
To: Miquel Raynal
Cc: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd,
Rob Herring, Mark Rutland, devicetree, Thomas Petazzoni,
Paul Cercueil, Chuanhong Guo, Weijie Gao, linux-arm-kernel,
Mason Yang, Julien Su
In-Reply-To: <20200529002517.3546-18-miquel.raynal@bootlin.com>
On Fri, 29 May 2020 02:25:14 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> +const struct mtd_ooblayout_ops *nand_get_large_page_layout(void)
nand_get_large_page_ooblayout() (same for the other helpers).
> +{
> + return &nand_ooblayout_lp_ops;
> +}
> +EXPORT_SYMBOL_GPL(nand_get_large_page_layout);
^ permalink raw reply
* Re: [PATCH v7 17/20] mtd: rawnand: Hide the generic OOB layout objects behind helpers
From: Boris Brezillon @ 2020-05-29 8:46 UTC (permalink / raw)
To: Miquel Raynal
Cc: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd,
Rob Herring, Mark Rutland, devicetree, Thomas Petazzoni,
Paul Cercueil, Chuanhong Guo, Weijie Gao, linux-arm-kernel,
Mason Yang, Julien Su
In-Reply-To: <20200529104548.0038bbe7@collabora.com>
On Fri, 29 May 2020 10:45:48 +0200
Boris Brezillon <boris.brezillon@collabora.com> wrote:
> On Fri, 29 May 2020 02:25:14 +0200
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> > +const struct mtd_ooblayout_ops *nand_get_large_page_layout(void)
>
> nand_get_large_page_ooblayout() (same for the other helpers).
>
And sorry if I got it wrong in my previous review :-/.
> > +{
> > + return &nand_ooblayout_lp_ops;
> > +}
> > +EXPORT_SYMBOL_GPL(nand_get_large_page_layout);
^ permalink raw reply
* Re: [PATCH v5 1/6] irqchip: Add Loongson HyperTransport Vector support
From: Marc Zyngier @ 2020-05-29 8:47 UTC (permalink / raw)
To: Jiaxun Yang
Cc: Thomas Gleixner, Jason Cooper, Rob Herring, Huacai Chen,
linux-kernel, devicetree, linux-mips
In-Reply-To: <20200528152757.1028711-2-jiaxun.yang@flygoat.com>
On 2020-05-28 16:27, Jiaxun Yang wrote:
> This controller appears on Loongson-3 chips for receiving interrupt
> vectors from PCH's PIC and PCH's PCIe MSI interrupts.
>
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
> v2:
> - Style cleanup
> - Set ack callback and set correct edge_irq handler
>
> v3:
> - Correct bitops in ACK callback
> v4:
> - Drop irqsave for spinlocks
> - Fix brace align and ordering issue thanks to tglx
> ---
When adding a changelog to a patch, please add the text after the '---'
delimiter, without adding an extra '---' at the end. This otherwise
confuses tools like mb2q or b4.
Please see Documentation/process/submitting-patches.rst ("14) The
canonical patch format").
I've fixed it up locally, no need to resend.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* Re: [PATCH v5 01/16] spi: dw: Set xfer effective_speed_hz
From: Sergei Shtylyov @ 2020-05-29 8:49 UTC (permalink / raw)
To: Serge Semin, Mark Brown
Cc: Serge Semin, Georgy Vlasov, Ramil Zaripov, Alexey Malahov,
Thomas Bogendoerfer, Arnd Bergmann, Feng Tang, Andy Shevchenko,
Rob Herring, linux-mips, devicetree, linux-spi, linux-kernel
In-Reply-To: <20200529035915.20790-2-Sergey.Semin@baikalelectronics.ru>
Hello!
On 29.05.2020 6:58, Serge Semin wrote:
> Seeing DW APB SSI controller doesn't support setting the exactly
> requested SPI bus frequency, but only a rounded frequency determined
> by means of the odd-numbered half-worded reference clock divider,
> it would be good tune the SPI core up and initialize the current
^ to?
> transfer effective_speed_hz. By doing so the core will be able to
> execute the xfer-related delays with better accuracy.
>
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Georgy Vlasov <Georgy.Vlasov@baikalelectronics.ru>
> Cc: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Feng Tang <feng.tang@intel.com>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: linux-mips@vger.kernel.org
> Cc: devicetree@vger.kernel.org
[...]
MBR, Sergei
^ permalink raw reply
* Re: [PATCH v5 0/6] Three Loongson irqchip support
From: Marc Zyngier @ 2020-05-29 8:52 UTC (permalink / raw)
To: Jiaxun Yang
Cc: linux-kernel, Jason Cooper, Huacai Chen, Rob Herring, devicetree,
linux-mips, Thomas Gleixner
In-Reply-To: <20200528152757.1028711-1-jiaxun.yang@flygoat.com>
On Thu, 28 May 2020 23:27:48 +0800, Jiaxun Yang wrote:
> v5:
> - Add some range checks in dt-schema
>
> Jiaxun Yang (6):
> irqchip: Add Loongson HyperTransport Vector support
> dt-bindings: interrupt-controller: Add Loongson HTVEC
> irqchip: Add Loongson PCH PIC controller
> dt-bindings: interrupt-controller: Add Loongson PCH PIC
> irqchip: Add Loongson PCH MSI controller
> dt-bindings: interrupt-controller: Add Loongson PCH MSI
>
> [...]
Applied to irq/irqchip-next, thanks!
[1/6] irqchip: Add Loongson HyperTransport Vector support
commit: 818e915fbac518e8c78e1877a0048d92d4965e5a
[2/6] dt-bindings: interrupt-controller: Add Loongson HTVEC
commit: 6c2832c3c6edc38ab58bad29731b4951c0a90cf8
[3/6] irqchip: Add Loongson PCH PIC controller
commit: ef8c01eb64ca6719da449dab0aa9424e13c58bd0
[4/6] dt-bindings: interrupt-controller: Add Loongson PCH PIC
commit: b6e4bc125fc517969f97d901b1845ebf47bbea26
[5/6] irqchip: Add Loongson PCH MSI controller
commit: 632dcc2c75ef6de3272aa4ddd8f19da1f1ace323
[6/6] dt-bindings: interrupt-controller: Add Loongson PCH MSI
commit: da10a4b626657387845f32d37141fc7d48ebbdb3
I've cherry-picked Rob's Rbs that were posted on the v4 series.
Cheers,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply
* Re: [V6 PATCH 1/2] dt-bindings: Added device tree binding for max98390
From: Steve Lee @ 2020-05-29 9:14 UTC (permalink / raw)
To: Rob Herring
Cc: Steve Lee, Liam Girdwood, Mark Brown, ALSA development,
devicetree, Linux Kernel Mailing List, ryan.lee.maxim, ryans.lee
In-Reply-To: <20200528141749.GB4186430@bogus>
On Thu, May 28, 2020 at 11:17 PM Rob Herring <robh@kernel.org> wrote:
>
> On Thu, May 28, 2020 at 07:37:55PM +0900, Steve Lee wrote:
> > Add DT binding of max98390 amplifier driver.
> >
> > Signed-off-by: Steve Lee <steves.lee@maximintegrated.com>
> > ---
> > Changed since V5:
> > * Change txt to yaml and fix up the examples.
> > Changed since V4:
> > * No changes.
> > Changed since V3:
> > * No changes.
> > Changed since V2:
> > * No changes.
> > Changed since V1:
> > * Modified sample text in example
> >
> > .../bindings/sound/maxim,max98390.yaml | 39 +++++++++++++++++++
> > 1 file changed, 39 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/sound/maxim,max98390.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/sound/maxim,max98390.yaml b/Documentation/devicetree/bindings/sound/maxim,max98390.yaml
> > new file mode 100644
> > index 000000000000..1ed4ab9e1c37
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/sound/maxim,max98390.yaml
> > @@ -0,0 +1,39 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/sound/maxim,max98390.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Maxim Integrated MAX98390 Speaker Amplifier with Integrated Dynamic Speaker Management
> > +
> > +maintainers:
> > + - Steve Lee <steves.lee@maximintegrated.com>
> > +
> > +properties:
> > + compatible:
> > + const: maxim,max98390
> > +
> > + reg:
> > + maxItems: 1
> > + description: I2C address of the device.
> > +
> > + temperature_calib:
>
> s/_/-/
This is follow as coreboot in Chromium OS case.
I'd follow this name unchanged.
>
> And missing 'maxim' prefix.
Added missed prefix maxim prefix.
>
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description: The calculated temperature data was measured while doing the calibration. Data : Temp / 100 * 2^12
>
> Any constraints? 0-2^32 are valid values?
I added range of the values
>
> > +
> > + r0_calib:
>
> Same here.
I added range of the values.
>
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description: This is r0 calibration data which was measured in factory mode.
> > +
> > +required:
> > + - compatible
> > + - reg
>
> Add:
>
> additionalProperties: false
I have added this.
>
> > +
> > +examples:
> > + - |
> > + max98390: amplifier@38 {
> > + compatible = "maxim,max98390";
> > + reg = <0x38>;
> > + maxim,temperature_calib = <1024>;
> > + maxim,r0_calib = <100232>;
> > + };
> > --
> > 2.17.1
> >
^ permalink raw reply
* Re: [PATCH v7 17/20] mtd: rawnand: Hide the generic OOB layout objects behind helpers
From: Miquel Raynal @ 2020-05-29 9:17 UTC (permalink / raw)
To: Boris Brezillon
Cc: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd,
Rob Herring, Mark Rutland, devicetree, Thomas Petazzoni,
Paul Cercueil, Chuanhong Guo, Weijie Gao, linux-arm-kernel,
Mason Yang, Julien Su
In-Reply-To: <20200529104640.1997434f@collabora.com>
Boris Brezillon <boris.brezillon@collabora.com> wrote on Fri, 29 May
2020 10:46:40 +0200:
> On Fri, 29 May 2020 10:45:48 +0200
> Boris Brezillon <boris.brezillon@collabora.com> wrote:
>
> > On Fri, 29 May 2020 02:25:14 +0200
> > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > > +const struct mtd_ooblayout_ops *nand_get_large_page_layout(void)
> >
> > nand_get_large_page_ooblayout() (same for the other helpers).
> >
>
> And sorry if I got it wrong in my previous review :-/.
Haha, no pb ;)
^ permalink raw reply
* Re: [PATCH v7 04/24] iommu: Add a page fault handler
From: Xiang Zheng @ 2020-05-29 9:18 UTC (permalink / raw)
To: Jean-Philippe Brucker, iommu, devicetree, linux-arm-kernel,
linux-pci, linux-mm
Cc: fenghua.yu, kevin.tian, jacob.jun.pan, jgg, catalin.marinas, joro,
robin.murphy, hch, zhangfei.gao, Jonathan.Cameron, felix.kuehling,
xuzaibo, will, christian.koenig, baolu.lu, Wang Haibin
In-Reply-To: <20200519175502.2504091-5-jean-philippe@linaro.org>
Hi,
On 2020/5/20 1:54, Jean-Philippe Brucker wrote:
> Some systems allow devices to handle I/O Page Faults in the core mm. For
> example systems implementing the PCIe PRI extension or Arm SMMU stall
> model. Infrastructure for reporting these recoverable page faults was
> added to the IOMMU core by commit 0c830e6b3282 ("iommu: Introduce device
> fault report API"). Add a page fault handler for host SVA.
>
> IOMMU driver can now instantiate several fault workqueues and link them
> to IOPF-capable devices. Drivers can choose between a single global
> workqueue, one per IOMMU device, one per low-level fault queue, one per
> domain, etc.
>
> When it receives a fault event, supposedly in an IRQ handler, the IOMMU
> driver reports the fault using iommu_report_device_fault(), which calls
> the registered handler. The page fault handler then calls the mm fault
> handler, and reports either success or failure with iommu_page_response().
> When the handler succeeded, the IOMMU retries the access.
>
> The iopf_param pointer could be embedded into iommu_fault_param. But
> putting iopf_param into the iommu_param structure allows us not to care
> about ordering between calls to iopf_queue_add_device() and
> iommu_register_device_fault_handler().
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
> v6->v7: Fix leak in iopf_queue_discard_partial()
> ---
> drivers/iommu/Kconfig | 4 +
> drivers/iommu/Makefile | 1 +
> include/linux/iommu.h | 51 +++++
> drivers/iommu/io-pgfault.c | 459 +++++++++++++++++++++++++++++++++++++
> 4 files changed, 515 insertions(+)
> create mode 100644 drivers/iommu/io-pgfault.c
>
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index d9fa5b410015..15e9dc4e503c 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -107,6 +107,10 @@ config IOMMU_SVA
> bool
> select IOASID
>
> +config IOMMU_PAGE_FAULT
> + bool
> + select IOMMU_SVA
> +
> config FSL_PAMU
> bool "Freescale IOMMU support"
> depends on PCI
> diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
> index 40c800dd4e3e..bf5cb4ee8409 100644
> --- a/drivers/iommu/Makefile
> +++ b/drivers/iommu/Makefile
> @@ -4,6 +4,7 @@ obj-$(CONFIG_IOMMU_API) += iommu-traces.o
> obj-$(CONFIG_IOMMU_API) += iommu-sysfs.o
> obj-$(CONFIG_IOMMU_DEBUGFS) += iommu-debugfs.o
> obj-$(CONFIG_IOMMU_DMA) += dma-iommu.o
> +obj-$(CONFIG_IOMMU_PAGE_FAULT) += io-pgfault.o
> obj-$(CONFIG_IOMMU_IO_PGTABLE) += io-pgtable.o
> obj-$(CONFIG_IOMMU_IO_PGTABLE_ARMV7S) += io-pgtable-arm-v7s.o
> obj-$(CONFIG_IOMMU_IO_PGTABLE_LPAE) += io-pgtable-arm.o
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index b62525747bd9..a462157c855b 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -46,6 +46,7 @@ struct iommu_domain;
> struct notifier_block;
> struct iommu_sva;
> struct iommu_fault_event;
> +struct iopf_queue;
>
> /* iommu fault flags */
> #define IOMMU_FAULT_READ 0x0
> @@ -347,6 +348,7 @@ struct iommu_fault_param {
> * struct dev_iommu - Collection of per-device IOMMU data
> *
> * @fault_param: IOMMU detected device fault reporting data
> + * @iopf_param: I/O Page Fault queue and data
> * @fwspec: IOMMU fwspec data
> * @priv: IOMMU Driver private data
> *
> @@ -356,6 +358,7 @@ struct iommu_fault_param {
> struct dev_iommu {
> struct mutex lock;
> struct iommu_fault_param *fault_param;
> + struct iopf_device_param *iopf_param;
> struct iommu_fwspec *fwspec;
> void *priv;
> };
[...]
> #endif /* __LINUX_IOMMU_H */
> diff --git a/drivers/iommu/io-pgfault.c b/drivers/iommu/io-pgfault.c
> new file mode 100644
> index 000000000000..1f61c1bc05da
> --- /dev/null
> +++ b/drivers/iommu/io-pgfault.c
> @@ -0,0 +1,459 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Handle device page faults
> + *
> + * Copyright (C) 2020 ARM Ltd.
> + */
> +
> +#include <linux/iommu.h>
> +#include <linux/list.h>
> +#include <linux/sched/mm.h>
> +#include <linux/slab.h>
> +#include <linux/workqueue.h>
> +
> +#include "iommu-sva.h"
> +
> +/**
> + * struct iopf_queue - IO Page Fault queue
> + * @wq: the fault workqueue
> + * @devices: devices attached to this queue
> + * @lock: protects the device list
> + */
> +struct iopf_queue {
> + struct workqueue_struct *wq;
> + struct list_head devices;
> + struct mutex lock;
> +};
> +
> +/**
> + * struct iopf_device_param - IO Page Fault data attached to a device
> + * @dev: the device that owns this param
> + * @queue: IOPF queue
> + * @queue_list: index into queue->devices
> + * @partial: faults that are part of a Page Request Group for which the last
> + * request hasn't been submitted yet.
> + */
> +struct iopf_device_param {
> + struct device *dev;
> + struct iopf_queue *queue;
> + struct list_head queue_list;
> + struct list_head partial;
> +};
> +
> +struct iopf_fault {
> + struct iommu_fault fault;
> + struct list_head list;
> +};
> +
> +struct iopf_group {
> + struct iopf_fault last_fault;
> + struct list_head faults;
> + struct work_struct work;
> + struct device *dev;
> +};
> +
>
[...]
> +
> +static void iopf_handle_group(struct work_struct *work)
> +{
> + struct iopf_group *group;
> + struct iopf_fault *iopf, *next;
> + enum iommu_page_response_code status = IOMMU_PAGE_RESP_SUCCESS;
> +
> + group = container_of(work, struct iopf_group, work);
> +
> + list_for_each_entry_safe(iopf, next, &group->faults, list) {
> + /*
> + * For the moment, errors are sticky: don't handle subsequent
> + * faults in the group if there is an error.
> + */
> + if (status == IOMMU_PAGE_RESP_SUCCESS)
> + status = iopf_handle_single(iopf);
> +
> + if (!(iopf->fault.prm.flags &
> + IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE))
> + kfree(iopf);
> + }
> +
> + iopf_complete_group(group->dev, &group->last_fault, status);
> + kfree(group);
> +}
> +
> +/**
> + * iommu_queue_iopf - IO Page Fault handler
> + * @evt: fault event
> + * @cookie: struct device, passed to iommu_register_device_fault_handler.
> + *
> + * Add a fault to the device workqueue, to be handled by mm.
> + *
> + * This module doesn't handle PCI PASID Stop Marker; IOMMU drivers must discard
> + * them before reporting faults. A PASID Stop Marker (LRW = 0b100) doesn't
> + * expect a response. It may be generated when disabling a PASID (issuing a
> + * PASID stop request) by some PCI devices.
> + *
> + * The PASID stop request is issued by the device driver before unbind(). Once
> + * it completes, no page request is generated for this PASID anymore and
> + * outstanding ones have been pushed to the IOMMU (as per PCIe 4.0r1.0 - 6.20.1
> + * and 10.4.1.2 - Managing PASID TLP Prefix Usage). Some PCI devices will wait
> + * for all outstanding page requests to come back with a response before
> + * completing the PASID stop request. Others do not wait for page responses, and
> + * instead issue this Stop Marker that tells us when the PASID can be
> + * reallocated.
> + *
> + * It is safe to discard the Stop Marker because it is an optimization.
> + * a. Page requests, which are posted requests, have been flushed to the IOMMU
> + * when the stop request completes.
> + * b. We flush all fault queues on unbind() before freeing the PASID.
> + *
> + * So even though the Stop Marker might be issued by the device *after* the stop
> + * request completes, outstanding faults will have been dealt with by the time
> + * we free the PASID.
> + *
> + * Return: 0 on success and <0 on error.
> + */
> +int iommu_queue_iopf(struct iommu_fault *fault, void *cookie)
> +{
> + int ret;
> + struct iopf_group *group;
> + struct iopf_fault *iopf, *next;
> + struct iopf_device_param *iopf_param;
> +
> + struct device *dev = cookie;
> + struct dev_iommu *param = dev->iommu;
> +
> + lockdep_assert_held(¶m->lock);
> +
> + if (fault->type != IOMMU_FAULT_PAGE_REQ)
> + /* Not a recoverable page fault */
> + return -EOPNOTSUPP;
> +
> + /*
> + * As long as we're holding param->lock, the queue can't be unlinked
> + * from the device and therefore cannot disappear.
> + */
> + iopf_param = param->iopf_param;
> + if (!iopf_param)
> + return -ENODEV;
> +
> + if (!(fault->prm.flags & IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE)) {
> + iopf = kzalloc(sizeof(*iopf), GFP_KERNEL);
> + if (!iopf)
> + return -ENOMEM;
> +
> + iopf->fault = *fault;
> +
> + /* Non-last request of a group. Postpone until the last one */
> + list_add(&iopf->list, &iopf_param->partial);
> +
> + return 0;
> + }
> +
> + group = kzalloc(sizeof(*group), GFP_KERNEL);
> + if (!group) {
> + /*
> + * The caller will send a response to the hardware. But we do
> + * need to clean up before leaving, otherwise partial faults
> + * will be stuck.
> + */
> + ret = -ENOMEM;
> + goto cleanup_partial;
> + }
> +
> + group->dev = dev;
> + group->last_fault.fault = *fault;
> + INIT_LIST_HEAD(&group->faults);
> + list_add(&group->last_fault.list, &group->faults);
> + INIT_WORK(&group->work, iopf_handle_group);
> +
> + /* See if we have partial faults for this group */
> + list_for_each_entry_safe(iopf, next, &iopf_param->partial, list) {
> + if (iopf->fault.prm.grpid == fault->prm.grpid)
> + /* Insert *before* the last fault */
> + list_move(&iopf->list, &group->faults);
> + }
> +
> + queue_work(iopf_param->queue->wq, &group->work);
> + return 0;
> +
> +cleanup_partial:
> + list_for_each_entry_safe(iopf, next, &iopf_param->partial, list) {
> + if (iopf->fault.prm.grpid == fault->prm.grpid) {
> + list_del(&iopf->list);
> + kfree(iopf);
> + }
> + }
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(iommu_queue_iopf);
[...]
> +/**
> + * iopf_queue_remove_device - Remove producer from fault queue
> + * @queue: IOPF queue
> + * @dev: device to remove
> + *
> + * Caller makes sure that no more faults are reported for this device.
> + *
> + * Return: 0 on success and <0 on error.
> + */
> +int iopf_queue_remove_device(struct iopf_queue *queue, struct device *dev)
> +{
> + int ret = 0;
> + struct iopf_fault *iopf, *next;
> + struct iopf_device_param *iopf_param;
> + struct dev_iommu *param = dev->iommu;
> +
> + if (!param || !queue)
> + return -EINVAL;
> +
> + mutex_lock(&queue->lock);
> + mutex_lock(¶m->lock);
> + iopf_param = param->iopf_param;
> + if (iopf_param && iopf_param->queue == queue) {
> + list_del(&iopf_param->queue_list);
> + param->iopf_param = NULL;
> + } else {
> + ret = -EINVAL;
> + }
> + mutex_unlock(¶m->lock);
> + mutex_unlock(&queue->lock);
> + if (ret)
> + return ret;
> +
> + /* Just in case some faults are still stuck */
> + list_for_each_entry_safe(iopf, next, &iopf_param->partial, list)
> + kfree(iopf);
> +
> + kfree(iopf_param);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(iopf_queue_remove_device);
> +
> +/**
> + * iopf_queue_alloc - Allocate and initialize a fault queue
> + * @name: a unique string identifying the queue (for workqueue)
> + *
> + * Return: the queue on success and NULL on error.
> + */
> +struct iopf_queue *iopf_queue_alloc(const char *name)
> +{
> + struct iopf_queue *queue;
> +
> + queue = kzalloc(sizeof(*queue), GFP_KERNEL);
> + if (!queue)
> + return NULL;
> +
> + /*
> + * The WQ is unordered because the low-level handler enqueues faults by
> + * group. PRI requests within a group have to be ordered, but once
> + * that's dealt with, the high-level function can handle groups out of
> + * order.
> + */
> + queue->wq = alloc_workqueue("iopf_queue/%s", WQ_UNBOUND, 0, name);
> + if (!queue->wq) {
> + kfree(queue);
> + return NULL;
> + }
> +
> + INIT_LIST_HEAD(&queue->devices);
> + mutex_init(&queue->lock);
> +
> + return queue;
> +}
> +EXPORT_SYMBOL_GPL(iopf_queue_alloc);
> +
> +/**
> + * iopf_queue_free - Free IOPF queue
> + * @queue: queue to free
> + *
> + * Counterpart to iopf_queue_alloc(). The driver must not be queuing faults or
> + * adding/removing devices on this queue anymore.
> + */
> +void iopf_queue_free(struct iopf_queue *queue)
> +{
> + struct iopf_device_param *iopf_param, *next;
> +
> + if (!queue)
> + return;
> +
> + list_for_each_entry_safe(iopf_param, next, &queue->devices, queue_list)
> + iopf_queue_remove_device(queue, iopf_param->dev);
> +
> + destroy_workqueue(queue->wq);
Do we need to free iopf_group(s) here in case the work queue of the group(s) are not
scheduled yet? If that occurs, we might leak memory here.
If the caller can ensure that would never occur or I miss something, please ignore
my noise. :)
> + kfree(queue);
> +}
> +EXPORT_SYMBOL_GPL(iopf_queue_free);
>
--
Thanks,
Xiang
^ permalink raw reply
* Re: [PATCH v7 16/20] mtd: nand: Convert generic NAND bits to use the ECC framework
From: Miquel Raynal @ 2020-05-29 9:25 UTC (permalink / raw)
To: Boris Brezillon
Cc: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd,
Rob Herring, Mark Rutland, devicetree, Thomas Petazzoni,
Paul Cercueil, Chuanhong Guo, Weijie Gao, linux-arm-kernel,
Mason Yang, Julien Su
In-Reply-To: <20200529103222.18c5b89b@collabora.com>
Boris Brezillon <boris.brezillon@collabora.com> wrote on Fri, 29 May
2020 10:32:22 +0200:
> On Fri, 29 May 2020 02:25:13 +0200
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> > Embed a generic NAND ECC high-level object in the nand_device
> > structure to carry all the ECC engine configuration/data. Adapt the
> > raw NAND and SPI-NAND cores to fit the change.
>
> In order to split that one, and make future re-organizations less
> painful (hope we won't have to do that again, but who knows), I would
> recommend doing things in this order:
>
> 1/ create a nanddev_get_ecc_requirements() helper that returns a
> const struct nand_ecc_props *
> 2/ patch spinand to use this helper
> 3/ introduce nand_ecc
> 4/ patch rawnand to use the new ecc layer
Sounds like a lot of efforts for a mechanical change to me. Not
mentioning that the diff is pretty small now. But ok, I'll try.
> >
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> > drivers/mtd/nand/raw/atmel/nand-controller.c | 9 +++--
> > drivers/mtd/nand/raw/brcmnand/brcmnand.c | 7 ++--
> > drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c | 12 +++---
> > drivers/mtd/nand/raw/marvell_nand.c | 7 ++--
> > drivers/mtd/nand/raw/mtk_nand.c | 4 +-
> > drivers/mtd/nand/raw/nand_base.c | 25 ++++++------
> > drivers/mtd/nand/raw/nand_esmt.c | 11 +++---
> > drivers/mtd/nand/raw/nand_hynix.c | 41 ++++++++++----------
> > drivers/mtd/nand/raw/nand_jedec.c | 4 +-
> > drivers/mtd/nand/raw/nand_micron.c | 14 ++++---
> > drivers/mtd/nand/raw/nand_onfi.c | 8 ++--
> > drivers/mtd/nand/raw/nand_samsung.c | 19 ++++-----
> > drivers/mtd/nand/raw/nand_toshiba.c | 11 +++---
> > drivers/mtd/nand/raw/sunxi_nand.c | 5 ++-
> > drivers/mtd/nand/raw/tegra_nand.c | 9 +++--
> > drivers/mtd/nand/spi/core.c | 8 ++--
> > drivers/mtd/nand/spi/macronix.c | 6 +--
> > drivers/mtd/nand/spi/toshiba.c | 6 +--
> > include/linux/mtd/nand.h | 8 ++--
> > 19 files changed, 114 insertions(+), 100 deletions(-)
> >
^ permalink raw reply
* Re: [PATCH v5 03/16] spi: dw: Locally wait for the DMA transactions completion
From: Andy Shevchenko @ 2020-05-29 9:26 UTC (permalink / raw)
To: Serge Semin
Cc: Serge Semin, Mark Brown, Georgy Vlasov, Ramil Zaripov,
Alexey Malahov, Thomas Bogendoerfer, Arnd Bergmann, Feng Tang,
Rob Herring, linux-mips, devicetree, linux-spi,
Linux Kernel Mailing List
In-Reply-To: <20200529081204.e2j5unvvfikr2y7v@mobilestation>
On Fri, May 29, 2020 at 11:12:04AM +0300, Serge Semin wrote:
> On Fri, May 29, 2020 at 10:55:32AM +0300, Andy Shevchenko wrote:
> > On Fri, May 29, 2020 at 7:02 AM Serge Semin
> > <Sergey.Semin@baikalelectronics.ru> wrote:
> > >
> > > Even if DMA transactions are finished it doesn't mean that the SPI
> > > transfers are also completed. It's specifically concerns the Tx-only
> > > SPI transfers, since there might be data left in the SPI Tx FIFO after
> > > the DMA engine notifies that the Tx DMA procedure is done. In order to
> > > completely fix the problem first the driver has to wait for the DMA
> > > transaction completion, then for the corresponding SPI operations to be
> > > finished. In this commit we implement the former part of the solution.
> > >
> > > Note we can't just move the SPI operations wait procedure to the DMA
> > > completion callbacks, since these callbacks might be executed in the
> > > tasklet context (and they will be in case of the DW DMA). In case of
> > > slow SPI bus it can cause significant system performance drop.
> >
>
> > I read commit message, I read the code. What's going on here since you
> > repeated xfer_completion (and its wait routine) from SPI core and I'm
> > wondering what happened to it? Why we are not calling
> > spi_finalize_current_transfer()?
>
> We discussed that in v4. You complained about using ndelay() for slow SPI bus,
> which may cause too long atomic context execution. We agreed. Since we can't wait
> in the tasklet context and using a dedicated kernel thread for waiting would be too
> much, Me and Mark agreed, that
> even if it causes us of the local wait-function
> re-implementation the best approach would be not to use the generic
> spi_transfer_wait() method, but instead wait for the DMA transactions locally
> in the DMA driver and just return 0 from the transfer_one callback indicating
> that the SPI transfer is finished and there is no need for SPI core to wait. As
> a lot of DMA-based SPI drivers do.
The above is missed in the commit message.
> If you don't understand what the commit message says, just say so. I'll
> reformulate it.
See above. A bit of elaboration would be good. Thank you!
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v7 3/3] ARM: dts: qcom: ipq4019: add USB devicetree nodes
From: Robert Marko @ 2020-05-29 9:36 UTC (permalink / raw)
To: Vinod Koul
Cc: Andy Gross, Bjorn Andersson, Kishon Vijay Abraham I, linux-kernel,
linux-arm-msm, robh+dt, Mark Rutland, devicetree, John Crispin,
Luka Perkov
In-Reply-To: <20200504073914.GQ1375924@vkoul-mobl>
On Mon, May 4, 2020 at 9:39 AM Vinod Koul <vkoul@kernel.org> wrote:
>
> On 03-05-20, 22:18, Robert Marko wrote:
> > From: John Crispin <john@phrozen.org>
> >
> > Since we now have driver for the USB PHY, lets add the necessary nodes to DTSI.
>
> Reviewed-by: Vinod Koul <vkoul@kernel.org>
>
> Bjorn, I have picked the phy and dt binding, feel free to apply this one
>
> Thanks
> --
> ~Vinod
Any chance of this landing into 5.7?
Driver and bindings have been merged, but I don't see DT nodes queued.
Regards,
Robert
^ permalink raw reply
* Re: [PATCH v5 05/16] spi: dw: Add SPI Rx-done wait method to DMA-based transfer
From: Andy Shevchenko @ 2020-05-29 9:46 UTC (permalink / raw)
To: Serge Semin
Cc: Mark Brown, Grant Likely, Linus Walleij, Feng Tang, Alan Cox,
Vinod Koul, Serge Semin, Georgy Vlasov, Ramil Zaripov,
Alexey Malahov, Thomas Bogendoerfer, Arnd Bergmann, Rob Herring,
linux-mips, devicetree, linux-spi, linux-kernel
In-Reply-To: <20200529035915.20790-6-Sergey.Semin@baikalelectronics.ru>
On Fri, May 29, 2020 at 06:59:03AM +0300, Serge Semin wrote:
> Having any data left in the Rx FIFO after the DMA engine claimed it has
> finished all DMA transactions is an abnormal situation, since the DW SPI
> controller driver expects to have all the data being fetched and placed
> into the SPI Rx buffer at that moment. In case if this has happened we
> assume that DMA engine still may be doing the data fetching, thus we give
> it sometime to finish. If after a short period of time the data is still
> left in the Rx FIFO, the driver will give up waiting and return an error
> indicating that the SPI controller/DMA engine must have hung up or failed
> at some point of doing their duties.
...
> +static int dw_spi_dma_wait_rx_done(struct dw_spi *dws)
> +{
> + int retry = WAIT_RETRIES;
> + struct spi_delay delay;
> + unsigned long ns, us;
> + u32 nents;
> +
> + /*
> + * It's unlikely that DMA engine is still doing the data fetching, but
> + * if it's let's give it some reasonable time. The timeout calculation
> + * is based on the synchronous APB/SSI reference clock rate, on a
> + * number of data entries left in the Rx FIFO, times a number of clock
> + * periods normally needed for a single APB read/write transaction
> + * without PREADY signal utilized (which is true for the DW APB SSI
> + * controller).
> + */
> + nents = dw_readl(dws, DW_SPI_RXFLR);
> + ns = NSEC_PER_SEC / dws->max_freq * 4 * nents;
I think we may slightly increase precision by writing this like
ns = 4 * NSEC_PER_SEC / dws->max_freq * nents;
> + if (ns <= NSEC_PER_USEC) {
> + delay.unit = SPI_DELAY_UNIT_NSECS;
> + delay.value = ns;
> + } else {
> + us = DIV_ROUND_UP(ns, NSEC_PER_USEC);
> + delay.unit = SPI_DELAY_UNIT_USECS;
> + delay.value = clamp_val(us, 0, USHRT_MAX);
> + }
> +
> + while (dw_spi_dma_rx_busy(dws) && retry--)
> + spi_delay_exec(&delay, NULL);
> +
> + if (retry < 0) {
> + dev_err(&dws->master->dev, "Rx hanged up\n");
> + return -EIO;
> + }
> +
> + return 0;
> +}
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 0/3] MIPS: Loongson64: Initial LS7A PCH support
From: Huacai Chen @ 2020-05-29 9:47 UTC (permalink / raw)
To: Jiaxun Yang
Cc: Marc Zyngier, Thomas Bogendoerfer, Rob Herring, open list:MIPS,
devicetree, LKML
In-Reply-To: <82608FDB-FEF8-45FA-85D7-236B46F906B7@flygoat.com>
Hi, Jiaxun,
On Fri, May 29, 2020 at 12:37 PM Jiaxun Yang <jiaxun.yang@flygoat.com> wrote:
>
>
>
> 于 2020年5月29日 GMT+08:00 下午12:13:36, Huacai Chen <chenhc@lemote.com> 写到:
> >Hi, Jiaxun,
> >
> >On Fri, May 29, 2020 at 11:45 AM Jiaxun Yang <jiaxun.yang@flygoat.com> wrote:
> >>
> >> With this series, LS7A and Loongson-3A4000 is finally supported
> >> note that this series should depend on irqchip support[1], which
> >> is likely to get merged soon.
> >>
> >> Thanks.
> >>
> >> [1]: https://lkml.org/lkml/2020/5/16/72
> >>
> >> Jiaxun Yang (3):
> >> dt-bindings: mips: Document two Loongson generic boards
> >> MIPS: Loongson64: DeviceTree for LS7A PCH
> >> MIPS: Loongson64:Load LS7A dtbs
> >>
> >> .../bindings/mips/loongson/devices.yaml | 8 +
> >> arch/mips/boot/dts/loongson/Makefile | 5 +-
> >> .../dts/loongson/loongson3-r4-package.dtsi | 74 +++++++
> >> .../dts/loongson/loongson3_4core_ls7a.dts | 25 +++
> >> .../boot/dts/loongson/loongson3_r4_ls7a.dts | 10 +
> >> arch/mips/boot/dts/loongson/ls7a-pch.dtsi | 185 ++++++++++++++++++
> >> .../asm/mach-loongson64/builtin_dtbs.h | 2 +
> >> arch/mips/loongson64/env.c | 56 +++---
> >> 8 files changed, 342 insertions(+), 23 deletions(-)
> >> create mode 100644 arch/mips/boot/dts/loongson/loongson3-r4-package.dtsi
> >> create mode 100644 arch/mips/boot/dts/loongson/loongson3_4core_ls7a.dts
> >> create mode 100644 arch/mips/boot/dts/loongson/loongson3_r4_ls7a.dts
> >> create mode 100644 arch/mips/boot/dts/loongson/ls7a-pch.dtsi
> >I think the naming can be like this: Old processor (Loongson 3A R1~R3)
> >use loongson64c_ prefix instead of loongson3, new processor (Loongson
> >3A R4) use loongson64g_ prefix instead of loongson3_r4, and
> >Loongson-2K use loongson64r_ prefix, this makes them consistent with
> >their PRID definitions.
>
>
> DeviceTree bindings have stable ABI. Although they're currently
> only used internally in Kernel. I don't think it's a good idea
> to rename existing bindings.
I think consistency is important, and this renaming doesn't break anything.
>
> Also, Loongson64C/64G/64R never came to a part of Loongson's
> official naming. I doubt if end users will recognize these names.
>
> I'd prefer keep naming as is. It's really not a big deal.
>
> Thanks.
>
>
> >
> >>
> >> --
> >> 2.27.0.rc0
> >>
>
> --
> Jiaxun Yang
^ permalink raw reply
* Re: [PATCH v5 01/16] spi: dw: Set xfer effective_speed_hz
From: Serge Semin @ 2020-05-29 9:49 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Serge Semin, Mark Brown, Georgy Vlasov, Ramil Zaripov,
Alexey Malahov, Thomas Bogendoerfer, Arnd Bergmann, Feng Tang,
Andy Shevchenko, Rob Herring, linux-mips, devicetree, linux-spi,
linux-kernel
In-Reply-To: <afdf414a-df95-b130-85e8-cda5bf4e9405@cogentembedded.com>
On Fri, May 29, 2020 at 11:49:12AM +0300, Sergei Shtylyov wrote:
> Hello!
>
> On 29.05.2020 6:58, Serge Semin wrote:
>
> > Seeing DW APB SSI controller doesn't support setting the exactly
> > requested SPI bus frequency, but only a rounded frequency determined
> > by means of the odd-numbered half-worded reference clock divider,
> > it would be good tune the SPI core up and initialize the current
> ^ to?
Thanks! I'll fix it in the next patchset version.
-Sergey
>
> > transfer effective_speed_hz. By doing so the core will be able to
> > execute the xfer-related delays with better accuracy.
> >
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > Cc: Georgy Vlasov <Georgy.Vlasov@baikalelectronics.ru>
> > Cc: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
> > Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: Feng Tang <feng.tang@intel.com>
> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: linux-mips@vger.kernel.org
> > Cc: devicetree@vger.kernel.org
> [...]
>
> MBR, Sergei
^ permalink raw reply
* Re: [PATCH v5 03/16] spi: dw: Locally wait for the DMA transactions completion
From: Serge Semin @ 2020-05-29 9:56 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Serge Semin, Mark Brown, Georgy Vlasov, Ramil Zaripov,
Alexey Malahov, Thomas Bogendoerfer, Arnd Bergmann, Feng Tang,
Rob Herring, linux-mips, devicetree, linux-spi,
Linux Kernel Mailing List
In-Reply-To: <20200529092610.GX1634618@smile.fi.intel.com>
On Fri, May 29, 2020 at 12:26:10PM +0300, Andy Shevchenko wrote:
> On Fri, May 29, 2020 at 11:12:04AM +0300, Serge Semin wrote:
> > On Fri, May 29, 2020 at 10:55:32AM +0300, Andy Shevchenko wrote:
> > > On Fri, May 29, 2020 at 7:02 AM Serge Semin
> > > <Sergey.Semin@baikalelectronics.ru> wrote:
> > > >
> > > > Even if DMA transactions are finished it doesn't mean that the SPI
> > > > transfers are also completed. It's specifically concerns the Tx-only
> > > > SPI transfers, since there might be data left in the SPI Tx FIFO after
> > > > the DMA engine notifies that the Tx DMA procedure is done. In order to
> > > > completely fix the problem first the driver has to wait for the DMA
> > > > transaction completion, then for the corresponding SPI operations to be
> > > > finished. In this commit we implement the former part of the solution.
> > > >
> > > > Note we can't just move the SPI operations wait procedure to the DMA
> > > > completion callbacks, since these callbacks might be executed in the
> > > > tasklet context (and they will be in case of the DW DMA). In case of
> > > > slow SPI bus it can cause significant system performance drop.
> > >
> >
> > > I read commit message, I read the code. What's going on here since you
> > > repeated xfer_completion (and its wait routine) from SPI core and I'm
> > > wondering what happened to it? Why we are not calling
> > > spi_finalize_current_transfer()?
> >
> > We discussed that in v4. You complained about using ndelay() for slow SPI bus,
> > which may cause too long atomic context execution. We agreed. Since we can't wait
> > in the tasklet context and using a dedicated kernel thread for waiting would be too
> > much, Me and Mark agreed, that
>
> > even if it causes us of the local wait-function
> > re-implementation the best approach would be not to use the generic
> > spi_transfer_wait() method, but instead wait for the DMA transactions locally
> > in the DMA driver and just return 0 from the transfer_one callback indicating
> > that the SPI transfer is finished and there is no need for SPI core to wait. As
> > a lot of DMA-based SPI drivers do.
>
> The above is missed in the commit message.
>
> > If you don't understand what the commit message says, just say so. I'll
> > reformulate it.
>
> See above. A bit of elaboration would be good. Thank you!
Agreed. I'll create a more detailed commit description, which will have the
info you cited.
-Sergey
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply
* Re: [RFC v3 0/3] Re-introduce TX FIFO resize for larger EP bursting
From: Greg KH @ 2020-05-29 10:12 UTC (permalink / raw)
To: Wesley Cheng
Cc: robh+dt, bjorn.andersson, balbi, agross, linux-kernel,
linux-arm-msm, devicetree, linux-usb
In-Reply-To: <1590630363-3934-1-git-send-email-wcheng@codeaurora.org>
On Wed, May 27, 2020 at 06:46:00PM -0700, Wesley Cheng wrote:
> Changes in V3:
> - Removed "Reviewed-by" tags
> - Renamed series back to RFC
> - Modified logic to ensure that fifo_size is reset if we pass the minimum
> threshold. Tested with binding multiple FDs requesting 6 FIFOs.
>
> Changes in V2:
> - Modified TXFIFO resizing logic to ensure that each EP is reserved a
> FIFO.
> - Removed dev_dbg() prints and fixed typos from patches
> - Added some more description on the dt-bindings commit message
>
> Currently, there is no functionality to allow for resizing the TXFIFOs, and
> relying on the HW default setting for the TXFIFO depth. In most cases, the
> HW default is probably sufficient, but for USB compositions that contain
> multiple functions that require EP bursting, the default settings
> might not be enough. Also to note, the current SW will assign an EP to a
> function driver w/o checking to see if the TXFIFO size for that particular
> EP is large enough. (this is a problem if there are multiple HW defined
> values for the TXFIFO size)
>
> It is mentioned in the SNPS databook that a minimum of TX FIFO depth = 3
> is required for an EP that supports bursting. Otherwise, there may be
> frequent occurences of bursts ending. For high bandwidth functions,
> such as data tethering (protocols that support data aggregation), mass
> storage, and media transfer protocol (over FFS), the bMaxBurst value can be
> large, and a bigger TXFIFO depth may prove to be beneficial in terms of USB
> throughput. (which can be associated to system access latency, etc...) It
> allows for a more consistent burst of traffic, w/o any interruptions, as
> data is readily available in the FIFO.
>
> With testing done using the mass storage function driver, the results show
> that with a larger TXFIFO depth, the bandwidth increased significantly.
Why is this still a "RFC" series? That implies you don't want this
applied...
^ permalink raw reply
* Re: [PATCH v3 1/9] dt-bindings: atmel-tcb: convert bindings to json-schema
From: Sebastian Andrzej Siewior @ 2020-05-29 10:13 UTC (permalink / raw)
To: Rob Herring, devicetree
Cc: Daniel Lezcano, Thomas Gleixner, Nicolas Ferre, kamel.bouhara,
linux-arm-kernel, linux-kernel, Alexandre Belloni
In-Reply-To: <20200506080554.283177-2-alexandre.belloni@bootlin.com>
Rob, could you please bless the DT parts of this series? Daniel Lezcano
asked for the blessing in:
https://lkml.kernel.org/r/f0feb409-11fb-08de-cc06-216a16de994a@linaro.org
On 2020-05-06 10:05:46 [+0200], Alexandre Belloni wrote:
> Convert Atmel Timer Counter Blocks bindings to DT schema format using
> json-schema.
>
> Also move it out of mfd as it is not and has never been related to mfd.
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> ---
> Cc: Rob Herring <robh+dt@kernel.org>
>
> Changes in v3:
> - Moved the child node documentation to the parent documentation
>
> Changes in v2:
> - Rebased on v5.7-rc1
> - Moved the binding documentation to its proper place
> - Added back the atmel,tcb-timer child node documentation
>
>
> .../devicetree/bindings/mfd/atmel-tcb.txt | 56 --------
> .../soc/microchip/atmel,at91rm9200-tcb.yaml | 126 ++++++++++++++++++
Sebastian
^ permalink raw reply
* Re: [PATCH v5 05/16] spi: dw: Add SPI Rx-done wait method to DMA-based transfer
From: Serge Semin @ 2020-05-29 10:13 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Serge Semin, Mark Brown, Grant Likely, Linus Walleij, Feng Tang,
Alan Cox, Vinod Koul, Georgy Vlasov, Ramil Zaripov,
Alexey Malahov, Thomas Bogendoerfer, Arnd Bergmann, Rob Herring,
linux-mips, devicetree, linux-spi, linux-kernel
In-Reply-To: <20200529094648.GY1634618@smile.fi.intel.com>
On Fri, May 29, 2020 at 12:46:48PM +0300, Andy Shevchenko wrote:
> On Fri, May 29, 2020 at 06:59:03AM +0300, Serge Semin wrote:
> > Having any data left in the Rx FIFO after the DMA engine claimed it has
> > finished all DMA transactions is an abnormal situation, since the DW SPI
> > controller driver expects to have all the data being fetched and placed
> > into the SPI Rx buffer at that moment. In case if this has happened we
> > assume that DMA engine still may be doing the data fetching, thus we give
> > it sometime to finish. If after a short period of time the data is still
> > left in the Rx FIFO, the driver will give up waiting and return an error
> > indicating that the SPI controller/DMA engine must have hung up or failed
> > at some point of doing their duties.
>
> ...
>
> > +static int dw_spi_dma_wait_rx_done(struct dw_spi *dws)
> > +{
> > + int retry = WAIT_RETRIES;
> > + struct spi_delay delay;
> > + unsigned long ns, us;
> > + u32 nents;
> > +
> > + /*
> > + * It's unlikely that DMA engine is still doing the data fetching, but
> > + * if it's let's give it some reasonable time. The timeout calculation
> > + * is based on the synchronous APB/SSI reference clock rate, on a
> > + * number of data entries left in the Rx FIFO, times a number of clock
> > + * periods normally needed for a single APB read/write transaction
> > + * without PREADY signal utilized (which is true for the DW APB SSI
> > + * controller).
> > + */
> > + nents = dw_readl(dws, DW_SPI_RXFLR);
>
> > + ns = NSEC_PER_SEC / dws->max_freq * 4 * nents;
>
> I think we may slightly increase precision by writing this like
>
> ns = 4 * NSEC_PER_SEC / dws->max_freq * nents;
Good point. Although both 4 and NSEC_PER_SEC are signed. The later is
1000000000L. Formally speaking on x32 systems (4 * 1000 000 000L) equals
to a negative value. Though overflow still won't happen so the result will
be correct. Anyway to be on a safe side it would be better to use an explicit
unsigned literal:
+ ns = 4U * NSEC_PER_SEC / dws->max_freq * nents;
-Sergey
>
>
> > + if (ns <= NSEC_PER_USEC) {
> > + delay.unit = SPI_DELAY_UNIT_NSECS;
> > + delay.value = ns;
> > + } else {
> > + us = DIV_ROUND_UP(ns, NSEC_PER_USEC);
> > + delay.unit = SPI_DELAY_UNIT_USECS;
> > + delay.value = clamp_val(us, 0, USHRT_MAX);
> > + }
> > +
> > + while (dw_spi_dma_rx_busy(dws) && retry--)
> > + spi_delay_exec(&delay, NULL);
> > +
> > + if (retry < 0) {
> > + dev_err(&dws->master->dev, "Rx hanged up\n");
> > + return -EIO;
> > + }
> > +
> > + return 0;
> > +}
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply
* Re: [PATCH 04/12] dt-bindings: irqchip: ti,sci-intr: Update bindings to drop the usage of gic as parent
From: Lokesh Vutla @ 2020-05-29 10:14 UTC (permalink / raw)
To: Rob Herring
Cc: Marc Zyngier, Thomas Gleixner, Nishanth Menon, Tero Kristo,
Santosh Shilimkar, Linux ARM Mailing List, Sekhar Nori,
Grygorii Strashko, Peter Ujfalusi, Device Tree Mailing List
In-Reply-To: <20200528221406.GA769073@bogus>
Hi Rob,
On 29/05/20 3:44 am, Rob Herring wrote:
> On Wed, May 20, 2020 at 06:14:46PM +0530, Lokesh Vutla wrote:
>> Drop the firmware related dt-bindings and use the hardware specified
>> interrupt numbers within Interrupt Router. This ensures interrupt router
>> DT node need not assume any interrupt parent type.
>
> I didn't like this binding to begin with, but now you're breaking
> compatibility.
Yes, I do agree that this change is breaking backward compatibility. But IMHO,
this does cleanup of firmware specific properties from DT. Since this is not
deployed out yet in the wild market, I took the leverage of breaking backward
compatibility. Before accepting these changes from firmware team, I did
discuss[0] with Marc on this topic.
>
>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>> ---
>> .../interrupt-controller/ti,sci-intr.txt | 31 ++++++++++---------
>> 1 file changed, 16 insertions(+), 15 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
>> index 1a8718f8855d..8b56b2de1c73 100644
>> --- a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
>> +++ b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
>> @@ -44,15 +44,17 @@ Required Properties:
>> 4: If intr supports level triggered interrupts.
>> - interrupt-controller: Identifies the node as an interrupt controller
>> - #interrupt-cells: Specifies the number of cells needed to encode an
>> - interrupt source. The value should be 2.
>> - First cell should contain the TISCI device ID of source
>> - Second cell should contain the interrupt source offset
>> - within the device.
>> + interrupt source. The value should be 1.
>> + First cell should contain interrupt router input number
>> + as specified by hardware.
>> - ti,sci: Phandle to TI-SCI compatible System controller node.
>> -- ti,sci-dst-id: TISCI device ID of the destination IRQ controller.
>> -- ti,sci-rm-range-girq: Array of TISCI subtype ids representing the host irqs
>> - assigned to this interrupt router. Each subtype id
>> - corresponds to a range of host irqs.
>> +- ti,sci-dev-id: TISCI device id of interrupt controller.
>> +- ti,interrupt-ranges: Set of triplets containing ranges that convert
>> + the INTR output interrupt numbers to parent's
>> + interrupt number. Each triplet has following entries:
>> + - First entry specifies the base for intr output irq
>> + - Second entry specifies the base for parent irqs
>> + - Third entry specifies the limit
>
> Humm, sounds like what interrupt-map does.
Okay, Ill look at it.
[0]
https://lore.kernel.org/linux-arm-kernel/2331f04eacee3b06cc152fc609225233@www.loen.fr/
Thanks and regards,
Lokesh
^ 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