* [PATCH 5/10] dt: bindings: Add bindings for Marvell Xenon SD Host Controller
From: Ziji Hu @ 2016-11-24 10:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161124111029.035553ce@free-electrons.com>
Hi all,
On 2016/11/24 18:10, Thomas Petazzoni wrote:
> Hello,
>
> On Thu, 24 Nov 2016 10:49:23 +0100, Marcin Wojtas wrote:
>
>> How about to avoid confusion, by simply renaming this number to
>> port-id/xenon-id or anything else but slot? I guess this may allow to
>> avoid some misunderstandings.
>
We borrow the term "slot" from PCIe interface from SD spec.
According to Appendix C in SD spec 3.0, slot means an independent set of register from the view of SW.
I can avoid using "slot" and replace "slot index" with "sdhc-id".
Thanks for the suggestions.
Thank you.
Best regards,
Hu Ziji
> Agreed.
>
> Thomas
>
^ permalink raw reply
* [PATCH 6/10] mmc: sdhci-xenon: Add Marvell Xenon SDHC core functionality
From: Ulf Hansson @ 2016-11-24 10:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <0390e7a05b6163deabb545f93729ea615eeaaee2.1477911954.git-series.gregory.clement@free-electrons.com>
On 31 October 2016 at 12:09, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:
> From: Ziji Hu <huziji@marvell.com>
>
> Add Xenon eMMC/SD/SDIO host controller core functionality.
> Add Xenon specific intialization process.
> Add Xenon specific mmc_host_ops APIs.
> Add Xenon specific register definitions.
>
> Add CONFIG_MMC_SDHCI_XENON support in drivers/mmc/host/Kconfig.
>
> Marvell Xenon SDHC conforms to SD Physical Layer Specification
> Version 3.01 and is designed according to the guidelines provided
> in the SD Host Controller Standard Specification Version 3.00.
>
> Signed-off-by: Hu Ziji <huziji@marvell.com>
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
> MAINTAINERS | 1 +-
> drivers/mmc/host/Kconfig | 9 +-
> drivers/mmc/host/Makefile | 3 +-
> drivers/mmc/host/sdhci-xenon.c | 594 ++++++++++++++++++++++++++++++++++-
> drivers/mmc/host/sdhci-xenon.h | 142 ++++++++-
> 5 files changed, 749 insertions(+), 0 deletions(-)
> create mode 100644 drivers/mmc/host/sdhci-xenon.c
> create mode 100644 drivers/mmc/host/sdhci-xenon.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 850a0afb0c8d..d92f4175574b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7608,6 +7608,7 @@ MARVELL XENON MMC/SD/SDIO HOST CONTROLLER DRIVER
> M: Ziji Hu <huziji@marvell.com>
> L: linux-mmc at vger.kernel.org
> S: Supported
> +F: drivers/mmc/host/sdhci-xenon.*
> F: Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
>
> MATROX FRAMEBUFFER DRIVER
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 5274f503a39a..85a53623526a 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -798,3 +798,12 @@ config MMC_SDHCI_BRCMSTB
> Broadcom STB SoCs.
>
> If unsure, say Y.
> +
> +config MMC_SDHCI_XENON
> + tristate "Marvell Xenon eMMC/SD/SDIO SDHCI driver"
> + depends on MMC_SDHCI && MMC_SDHCI_PLTFM
> + help
> + This selects Marvell Xenon eMMC/SD/SDIO SDHCI.
> + If you have a machine with integrated Marvell Xenon SDHC IP,
> + say Y or M here.
> + If unsure, say N.
> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> index e2bdaaf43184..75eaf743486c 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -80,3 +80,6 @@ obj-$(CONFIG_MMC_SDHCI_BRCMSTB) += sdhci-brcmstb.o
> ifeq ($(CONFIG_CB710_DEBUG),y)
> CFLAGS-cb710-mmc += -DDEBUG
> endif
> +
> +obj-$(CONFIG_MMC_SDHCI_XENON) += sdhci-xenon-driver.o
> +sdhci-xenon-driver-y += sdhci-xenon.o
> diff --git a/drivers/mmc/host/sdhci-xenon.c b/drivers/mmc/host/sdhci-xenon.c
> new file mode 100644
> index 000000000000..3ea059f2aaab
> --- /dev/null
> +++ b/drivers/mmc/host/sdhci-xenon.c
> @@ -0,0 +1,594 @@
> +/*
> + * Driver for Marvell SOC Platform Group Xenon SDHC as a platform device
> + *
> + * Copyright (C) 2016 Marvell, All Rights Reserved.
> + *
> + * Author: Hu Ziji <huziji@marvell.com>
> + * Date: 2016-8-24
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation version 2.
> + *
> + * Inspired by Jisheng Zhang <jszhang@marvell.com>
> + * Special thanks to Video BG4 project team.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/mmc/host.h>
> +#include <linux/mmc/mmc.h>
> +#include <linux/mmc/sdio.h>
> +#include <linux/mmc/card.h>
> +#include <linux/mmc/host.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +
> +#include "sdhci-pltfm.h"
> +#include "sdhci.h"
> +#include "sdhci-xenon.h"
> +
> +/* Set SDCLK-off-while-idle */
> +static void xenon_set_sdclk_off_idle(struct sdhci_host *host,
> + unsigned char slot_idx, bool enable)
> +{
> + u32 reg;
> + u32 mask;
> +
> + reg = sdhci_readl(host, SDHC_SYS_OP_CTRL);
> + /* Get the bit shift basing on the slot index */
> + mask = (0x1 << (SDCLK_IDLEOFF_ENABLE_SHIFT + slot_idx));
> + if (enable)
> + reg |= mask;
> + else
> + reg &= ~mask;
> +
> + sdhci_writel(host, reg, SDHC_SYS_OP_CTRL);
> +}
> +
> +/* Enable/Disable the Auto Clock Gating function */
> +static void xenon_set_acg(struct sdhci_host *host, bool enable)
> +{
> + u32 reg;
> +
> + reg = sdhci_readl(host, SDHC_SYS_OP_CTRL);
> + if (enable)
> + reg &= ~AUTO_CLKGATE_DISABLE_MASK;
> + else
> + reg |= AUTO_CLKGATE_DISABLE_MASK;
> + sdhci_writel(host, reg, SDHC_SYS_OP_CTRL);
> +}
> +
> +/* Enable this slot */
> +static void xenon_enable_slot(struct sdhci_host *host,
> + unsigned char slot_idx)
> +{
> + u32 reg;
> +
> + reg = sdhci_readl(host, SDHC_SYS_OP_CTRL);
> + reg |= (BIT(slot_idx) << SLOT_ENABLE_SHIFT);
> + sdhci_writel(host, reg, SDHC_SYS_OP_CTRL);
> +
> + /*
> + * Manually set the flag which all the slots require,
> + * including SD, eMMC, SDIO
> + */
> + host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY;
> +}
> +
> +/* Disable this slot */
> +static void xenon_disable_slot(struct sdhci_host *host,
> + unsigned char slot_idx)
> +{
> + u32 reg;
> +
> + reg = sdhci_readl(host, SDHC_SYS_OP_CTRL);
> + reg &= ~(BIT(slot_idx) << SLOT_ENABLE_SHIFT);
> + sdhci_writel(host, reg, SDHC_SYS_OP_CTRL);
> +}
> +
> +/* Enable Parallel Transfer Mode */
> +static void xenon_enable_slot_parallel_tran(struct sdhci_host *host,
> + unsigned char slot_idx)
> +{
> + u32 reg;
> +
> + reg = sdhci_readl(host, SDHC_SYS_EXT_OP_CTRL);
> + reg |= BIT(slot_idx);
> + sdhci_writel(host, reg, SDHC_SYS_EXT_OP_CTRL);
> +}
> +
> +static void xenon_slot_tuning_setup(struct sdhci_host *host)
> +{
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> + u32 reg;
> +
> + /* Disable the Re-Tuning Request functionality */
> + reg = sdhci_readl(host, SDHC_SLOT_RETUNING_REQ_CTRL);
> + reg &= ~RETUNING_COMPATIBLE;
> + sdhci_writel(host, reg, SDHC_SLOT_RETUNING_REQ_CTRL);
> +
> + /* Disable the Re-tuning Event Signal Enable */
> + reg = sdhci_readl(host, SDHCI_SIGNAL_ENABLE);
> + reg &= ~SDHCI_INT_RETUNE;
> + sdhci_writel(host, reg, SDHCI_SIGNAL_ENABLE);
> +
> + /* Force to use Tuning Mode 1 */
> + host->tuning_mode = SDHCI_TUNING_MODE_1;
> + /* Set re-tuning period */
> + host->tuning_count = 1 << (priv->tuning_count - 1);
> +}
> +
> +/*
> + * Operations inside struct sdhci_ops
> + */
> +/* Recover the Register Setting cleared during SOFTWARE_RESET_ALL */
> +static void sdhci_xenon_reset_exit(struct sdhci_host *host,
> + unsigned char slot_idx, u8 mask)
> +{
> + /* Only SOFTWARE RESET ALL will clear the register setting */
> + if (!(mask & SDHCI_RESET_ALL))
> + return;
> +
> + /* Disable tuning request and auto-retuning again */
> + xenon_slot_tuning_setup(host);
> +
> + xenon_set_acg(host, true);
> +
> + xenon_set_sdclk_off_idle(host, slot_idx, false);
> +}
> +
> +static void sdhci_xenon_reset(struct sdhci_host *host, u8 mask)
> +{
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> +
> + sdhci_reset(host, mask);
> + sdhci_xenon_reset_exit(host, priv->slot_idx, mask);
> +}
> +
> +/*
> + * Xenon defines different values for HS200 and SDR104
> + * in Host_Control_2
> + */
> +static void xenon_set_uhs_signaling(struct sdhci_host *host,
> + unsigned int timing)
> +{
> + u16 ctrl_2;
> +
> + ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> + /* Select Bus Speed Mode for host */
> + ctrl_2 &= ~SDHCI_CTRL_UHS_MASK;
> + if (timing == MMC_TIMING_MMC_HS200)
> + ctrl_2 |= XENON_SDHCI_CTRL_HS200;
> + else if (timing == MMC_TIMING_UHS_SDR104)
> + ctrl_2 |= SDHCI_CTRL_UHS_SDR104;
> + else if (timing == MMC_TIMING_UHS_SDR12)
> + ctrl_2 |= SDHCI_CTRL_UHS_SDR12;
> + else if (timing == MMC_TIMING_UHS_SDR25)
> + ctrl_2 |= SDHCI_CTRL_UHS_SDR25;
> + else if (timing == MMC_TIMING_UHS_SDR50)
> + ctrl_2 |= SDHCI_CTRL_UHS_SDR50;
> + else if ((timing == MMC_TIMING_UHS_DDR50) ||
> + (timing == MMC_TIMING_MMC_DDR52))
> + ctrl_2 |= SDHCI_CTRL_UHS_DDR50;
> + else if (timing == MMC_TIMING_MMC_HS400)
> + ctrl_2 |= XENON_SDHCI_CTRL_HS400;
> + sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
> +}
> +
> +static const struct sdhci_ops sdhci_xenon_ops = {
> + .set_clock = sdhci_set_clock,
> + .set_bus_width = sdhci_set_bus_width,
> + .reset = sdhci_xenon_reset,
> + .set_uhs_signaling = xenon_set_uhs_signaling,
> + .get_max_clock = sdhci_pltfm_clk_get_max_clock,
> +};
> +
> +static const struct sdhci_pltfm_data sdhci_xenon_pdata = {
> + .ops = &sdhci_xenon_ops,
> + .quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC |
> + SDHCI_QUIRK_NO_SIMULT_VDD_AND_POWER |
> + SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
> +};
> +
> +/*
> + * Xenon Specific Operations in mmc_host_ops
> + */
> +static void xenon_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> +{
> + struct sdhci_host *host = mmc_priv(mmc);
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> + unsigned long flags;
> + u32 reg;
> +
> + /*
> + * HS400/HS200/eMMC HS doesn't have Preset Value register.
> + * However, sdhci_set_ios will read HS400/HS200 Preset register.
> + * Disable Preset Value register for HS400/HS200.
> + * eMMC HS with preset_enabled set will trigger a bug in
> + * get_preset_value().
> + */
> + spin_lock_irqsave(&host->lock, flags);
> + if ((ios->timing == MMC_TIMING_MMC_HS400) ||
> + (ios->timing == MMC_TIMING_MMC_HS200) ||
> + (ios->timing == MMC_TIMING_MMC_HS)) {
> + host->preset_enabled = false;
> + host->quirks2 |= SDHCI_QUIRK2_PRESET_VALUE_BROKEN;
> +
> + reg = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> + reg &= ~SDHCI_CTRL_PRESET_VAL_ENABLE;
> + sdhci_writew(host, reg, SDHCI_HOST_CONTROL2);
> + } else {
> + host->quirks2 &= ~SDHCI_QUIRK2_PRESET_VALUE_BROKEN;
> + }
> + spin_unlock_irqrestore(&host->lock, flags);
> +
> + sdhci_set_ios(mmc, ios);
> +
> + if (host->clock > DEFAULT_SDCLK_FREQ) {
> + spin_lock_irqsave(&host->lock, flags);
> + xenon_set_sdclk_off_idle(host, priv->slot_idx, true);
> + spin_unlock_irqrestore(&host->lock, flags);
> + }
> +}
> +
> +static int __emmc_signal_voltage_switch(struct mmc_host *mmc,
> + const unsigned char signal_voltage)
> +{
> + u32 ctrl;
> + unsigned char voltage_code;
> + struct sdhci_host *host = mmc_priv(mmc);
> +
> + if (signal_voltage == MMC_SIGNAL_VOLTAGE_330)
> + voltage_code = EMMC_VCCQ_3_3V;
> + else if (signal_voltage == MMC_SIGNAL_VOLTAGE_180)
> + voltage_code = EMMC_VCCQ_1_8V;
> + else
> + return -EINVAL;
> +
> + /*
> + * This host is for eMMC, XENON self-defined
> + * eMMC slot control register should be accessed
> + * instead of Host Control 2
> + */
> + ctrl = sdhci_readl(host, SDHC_SLOT_EMMC_CTRL);
> + ctrl &= ~EMMC_VCCQ_MASK;
> + ctrl |= voltage_code;
> + sdhci_writel(host, ctrl, SDHC_SLOT_EMMC_CTRL);
> +
> + /* There is no standard to determine this waiting period */
> + usleep_range(1000, 2000);
> +
> + /* Check whether io voltage switch is done */
> + ctrl = sdhci_readl(host, SDHC_SLOT_EMMC_CTRL);
> + ctrl &= EMMC_VCCQ_MASK;
> + /*
> + * This bit is set only when regulator feeds back the voltage switch
> + * results to Xenon SDHC.
> + * However, in actaul implementation, regulator might not provide
> + * this feedback.
> + * Thus we shall not rely on this bit to determine if switch failed.
> + * If the bit is not set, just throw a message.
> + * Besides, error code should not be returned.
> + */
> + if (ctrl != voltage_code)
> + dev_info(mmc_dev(mmc), "fail to detect eMMC signal voltage stable\n");
> + return 0;
> +}
> +
> +static int xenon_emmc_signal_voltage_switch(struct mmc_host *mmc,
> + struct mmc_ios *ios)
> +{
> + unsigned char voltage = ios->signal_voltage;
> +
> + if ((voltage == MMC_SIGNAL_VOLTAGE_330) ||
> + (voltage == MMC_SIGNAL_VOLTAGE_180))
> + return __emmc_signal_voltage_switch(mmc, voltage);
> +
> + dev_err(mmc_dev(mmc), "Unsupported signal voltage: %d\n",
> + voltage);
> + return -EINVAL;
This wrapper function seems unnessarry. It only adds a dev_err(), so
then might as well do that in __emmc_signal_voltage_switch().
> +}
> +
> +static int xenon_start_signal_voltage_switch(struct mmc_host *mmc,
> + struct mmc_ios *ios)
> +{
> + struct sdhci_host *host = mmc_priv(mmc);
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> +
> + /*
> + * Before SD/SDIO set signal voltage, SD bus clock should be
> + * disabled. However, sdhci_set_clock will also disable the Internal
> + * clock in mmc_set_signal_voltage().
If that's the case then that is wrong in the generic sdhci code.
What's the reason why it can't be fixed there instead of having this
workaround?
> + * If Internal clock is disabled, the 3.3V/1.8V bit can not be updated.
> + * Thus here manually enable internal clock.
> + *
> + * After switch completes, it is unnecessary to disable internal clock,
> + * since keeping internal clock active obeys SD spec.
> + */
> + enable_xenon_internal_clk(host);
> +
> + if (priv->emmc_slot)
> + return xenon_emmc_signal_voltage_switch(mmc, ios);
> +
> + return sdhci_start_signal_voltage_switch(mmc, ios);
> +}
> +
> +/*
> + * After determining which slot is used for SDIO,
> + * some additional task is required.
> + */
> +static void xenon_init_card(struct mmc_host *mmc, struct mmc_card *card)
> +{
> + struct sdhci_host *host = mmc_priv(mmc);
> + u32 reg;
> + u8 slot_idx;
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> +
> + /* Link the card for delay adjustment */
> + priv->card_candidate = card;
> + /* Set tuning functionality of this slot */
> + xenon_slot_tuning_setup(host);
This looks weird. I assume this can be done as a part of the regular
tuning seqeunce!?
> +
> + slot_idx = priv->slot_idx;
> + if (!mmc_card_sdio(card)) {
> + /* Clear SDIO Card Inserted indication */
Why do you need this?
If you need to reset this, I think it's better to do it from
->set_ios() at MMC_POWER_OFF.
> + reg = sdhci_readl(host, SDHC_SYS_CFG_INFO);
> + reg &= ~(1 << (slot_idx + SLOT_TYPE_SDIO_SHIFT));
> + sdhci_writel(host, reg, SDHC_SYS_CFG_INFO);
> +
> + if (mmc_card_mmc(card)) {
> + mmc->caps |= MMC_CAP_NONREMOVABLE;
> + if (!(host->quirks2 & SDHCI_QUIRK2_NO_1_8_V))
> + mmc->caps |= MMC_CAP_1_8V_DDR;
> + /*
> + * Force to clear BUS_TEST to
> + * skip bus_test_pre and bus_test_post
> + */
> + mmc->caps &= ~MMC_CAP_BUS_WIDTH_TEST;
> + mmc->caps2 |= MMC_CAP2_HC_ERASE_SZ |
> + MMC_CAP2_PACKED_CMD;
> + if (mmc->caps & MMC_CAP_8_BIT_DATA)
> + mmc->caps2 |= MMC_CAP2_HS400_1_8V;
Most of this can be specified as DT configurations. Please use that instead.
More importantly, please don't use the ->init_card() ops to assign
host caps. If not DT, please do it from ->probe().
> + }
> + } else {
> + /*
> + * Set SDIO Card Inserted indication
> + * to inform that the current slot is for SDIO
> + */
> + reg = sdhci_readl(host, SDHC_SYS_CFG_INFO);
> + reg |= (1 << (slot_idx + SLOT_TYPE_SDIO_SHIFT));
> + sdhci_writel(host, reg, SDHC_SYS_CFG_INFO);
So this makes sence to have in the ->init_card() ops. The rest above, not.
> + }
> +}
> +
> +static int xenon_execute_tuning(struct mmc_host *mmc, u32 opcode)
> +{
> + struct sdhci_host *host = mmc_priv(mmc);
> +
> + if (host->timing == MMC_TIMING_UHS_DDR50)
> + return 0;
> +
> + return sdhci_execute_tuning(mmc, opcode);
> +}
> +
> +static void xenon_replace_mmc_host_ops(struct sdhci_host *host)
> +{
> + host->mmc_host_ops.set_ios = xenon_set_ios;
> + host->mmc_host_ops.start_signal_voltage_switch =
> + xenon_start_signal_voltage_switch;
> + host->mmc_host_ops.init_card = xenon_init_card;
> + host->mmc_host_ops.execute_tuning = xenon_execute_tuning;
> +}
> +
> +static int xenon_probe_dt(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct sdhci_host *host = platform_get_drvdata(pdev);
> + struct mmc_host *mmc = host->mmc;
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> + int err;
> + u32 slot_idx, nr_slot;
> + u32 tuning_count;
> + u32 reg;
> +
> + /* Standard MMC property */
> + err = mmc_of_parse(mmc);
> + if (err)
> + return err;
> +
> + /* Standard SDHCI property */
> + sdhci_get_of_property(pdev);
> +
> + /*
> + * Xenon Specific property:
> + * emmc: explicitly indicate whether this slot is for eMMC
> + * slotno: the index of slot. Refer to SDHC_SYS_CFG_INFO register
> + * tun-count: the interval between re-tuning
> + * PHY type: "sdhc phy", "emmc phy 5.0" or "emmc phy 5.1"
> + */
> + if (of_property_read_bool(np, "marvell,xenon-emmc"))
> + priv->emmc_slot = true;
So, you need this because of the eMMC voltage switch behaviour, right?
Then I would rather like to describe this a generic DT bindings for
the eMMC voltage level support. There have acutally been some earlier
discussions for this, but we haven't yet made some changes.
I think what is missing is a mmc-ddr-3_3v DT binding, which when set,
allows the host driver to accept I/O voltage switches to 3.3V. If not
supported the ->start_signal_voltage_switch() ops may return -EINVAL.
This would inform the mmc core to move on to the next supported
voltage level. There might be some minor additional changes to the mmc
card initialization sequence, but those should be simple.
I can help out to look into this, unless you want to do it yourself of course!?
> + else
> + priv->emmc_slot = false;
> +
> + if (!of_property_read_u32(np, "marvell,xenon-slotno", &slot_idx)) {
> + nr_slot = sdhci_readl(host, SDHC_SYS_CFG_INFO);
> + nr_slot &= NR_SUPPORTED_SLOT_MASK;
> + if (unlikely(slot_idx > nr_slot)) {
> + dev_err(mmc_dev(mmc), "Slot Index %d exceeds Number of slots %d\n",
> + slot_idx, nr_slot);
> + return -EINVAL;
> + }
> + } else {
> + priv->slot_idx = 0x0;
> + }
> +
> + if (!of_property_read_u32(np, "marvell,xenon-tun-count",
> + &tuning_count)) {
> + if (unlikely(tuning_count >= TMR_RETUN_NO_PRESENT)) {
> + dev_err(mmc_dev(mmc), "Wrong Re-tuning Count. Set default value %d\n",
> + DEF_TUNING_COUNT);
> + tuning_count = DEF_TUNING_COUNT;
> + }
> + } else {
> + priv->tuning_count = DEF_TUNING_COUNT;
> + }
To make the code a bit easier...
Maybe set "priv->tuning_count = DEF_TUNING_COUNT" before the "if", and
instead have the of_property_read_u32() to update the value when set.
> +
> + if (of_property_read_bool(np, "marvell,xenon-mask-conflict-err")) {
> + reg = sdhci_readl(host, SDHC_SYS_EXT_OP_CTRL);
> + reg |= MASK_CMD_CONFLICT_ERROR;
> + sdhci_writel(host, reg, SDHC_SYS_EXT_OP_CTRL);
> + }
> +
> + return err;
> +}
> +
> +static int xenon_slot_probe(struct sdhci_host *host)
> +{
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> + u8 slot_idx = priv->slot_idx;
> +
> + /* Enable slot */
> + xenon_enable_slot(host, slot_idx);
> +
> + /* Enable ACG */
> + xenon_set_acg(host, true);
> +
> + /* Enable Parallel Transfer Mode */
> + xenon_enable_slot_parallel_tran(host, slot_idx);
> +
> + priv->timing = MMC_TIMING_FAKE;
> + priv->clock = 0;
What are these used for?
> +
> + return 0;
> +}
> +
> +static void xenon_slot_remove(struct sdhci_host *host)
> +{
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> + u8 slot_idx = priv->slot_idx;
> +
> + /* disable slot */
> + xenon_disable_slot(host, slot_idx);
> +}
> +
> +static int sdhci_xenon_probe(struct platform_device *pdev)
> +{
> + struct sdhci_pltfm_host *pltfm_host;
> + struct sdhci_host *host;
> + struct clk *clk, *axi_clk;
> + struct sdhci_xenon_priv *priv;
> + int err;
> +
> + host = sdhci_pltfm_init(pdev, &sdhci_xenon_pdata,
> + sizeof(struct sdhci_xenon_priv));
> + if (IS_ERR(host))
> + return PTR_ERR(host);
> +
> + pltfm_host = sdhci_priv(host);
> + priv = sdhci_pltfm_priv(pltfm_host);
> +
> + xenon_set_acg(host, false);
> +
> + /*
> + * Link Xenon specific mmc_host_ops function,
> + * to replace standard ones in sdhci_ops.
> + */
> + xenon_replace_mmc_host_ops(host);
> +
> + clk = devm_clk_get(&pdev->dev, "core");
> + if (IS_ERR(clk)) {
> + dev_err(&pdev->dev, "Failed to setup input clk.\n");
> + err = PTR_ERR(clk);
> + goto free_pltfm;
> + }
> + clk_prepare_enable(clk);
Check error code.
> + pltfm_host->clk = clk;
Why not assign pltfm_host->clk immedately when doing devm_clk_get(),
that would make this a bit cleaner, right?
> +
> + /*
> + * Some SOCs require additional clock to
> + * manage AXI bus clock.
> + * It is optional.
> + */
> + axi_clk = devm_clk_get(&pdev->dev, "axi");
> + if (!IS_ERR(axi_clk)) {
> + clk_prepare_enable(axi_clk);
> + priv->axi_clk = axi_clk;
> + }
Same comments as for the above core clock.
> +
> + err = xenon_probe_dt(pdev);
> + if (err)
> + goto err_clk;
> +
> + err = xenon_slot_probe(host);
> + if (err)
> + goto err_clk;
> +
> + err = sdhci_add_host(host);
> + if (err)
> + goto remove_slot;
> +
> + return 0;
> +
> +remove_slot:
> + xenon_slot_remove(host);
> +err_clk:
> + clk_disable_unprepare(pltfm_host->clk);
> + if (!IS_ERR(axi_clk))
> + clk_disable_unprepare(axi_clk);
> +free_pltfm:
> + sdhci_pltfm_free(pdev);
> + return err;
> +}
> +
> +static int sdhci_xenon_remove(struct platform_device *pdev)
> +{
> + struct sdhci_host *host = platform_get_drvdata(pdev);
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> + int dead = (readl(host->ioaddr + SDHCI_INT_STATUS) == 0xFFFFFFFF);
> +
> + xenon_slot_remove(host);
> +
> + sdhci_remove_host(host, dead);
> +
> + clk_disable_unprepare(pltfm_host->clk);
> + clk_disable_unprepare(priv->axi_clk);
> +
> + sdhci_pltfm_free(pdev);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id sdhci_xenon_dt_ids[] = {
> + { .compatible = "marvell,xenon-sdhci",},
> + { .compatible = "marvell,armada-3700-sdhci",},
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, sdhci_xenon_dt_ids);
> +
> +static struct platform_driver sdhci_xenon_driver = {
> + .driver = {
> + .name = "xenon-sdhci",
> + .of_match_table = sdhci_xenon_dt_ids,
> + .pm = &sdhci_pltfm_pmops,
> + },
> + .probe = sdhci_xenon_probe,
> + .remove = sdhci_xenon_remove,
> +};
> +
> +module_platform_driver(sdhci_xenon_driver);
> +
> +MODULE_DESCRIPTION("SDHCI platform driver for Marvell Xenon SDHC");
> +MODULE_AUTHOR("Hu Ziji <huziji@marvell.com>");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/mmc/host/sdhci-xenon.h b/drivers/mmc/host/sdhci-xenon.h
> new file mode 100644
> index 000000000000..4601d0a4b22f
> --- /dev/null
> +++ b/drivers/mmc/host/sdhci-xenon.h
I don't think you need a specific header for this, let's instead just
put everthing in the c-file.
> @@ -0,0 +1,142 @@
> +/*
> + * Copyright (C) 2016 Marvell, All Rights Reserved.
> + *
> + * Author: Hu Ziji <huziji@marvell.com>
> + * Date: 2016-8-24
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation version 2.
> + */
> +#ifndef SDHCI_XENON_H_
> +#define SDHCI_XENON_H_
> +
> +#include <linux/clk.h>
> +#include <linux/mmc/card.h>
> +#include <linux/of.h>
> +#include "sdhci.h"
> +
> +/* Register Offset of SD Host Controller SOCP self-defined register */
> +#define SDHC_SYS_CFG_INFO 0x0104
> +#define SLOT_TYPE_SDIO_SHIFT 24
> +#define SLOT_TYPE_EMMC_MASK 0xFF
> +#define SLOT_TYPE_EMMC_SHIFT 16
> +#define SLOT_TYPE_SD_SDIO_MMC_MASK 0xFF
> +#define SLOT_TYPE_SD_SDIO_MMC_SHIFT 8
> +#define NR_SUPPORTED_SLOT_MASK 0x7
> +
> +#define SDHC_SYS_OP_CTRL 0x0108
> +#define AUTO_CLKGATE_DISABLE_MASK BIT(20)
> +#define SDCLK_IDLEOFF_ENABLE_SHIFT 8
> +#define SLOT_ENABLE_SHIFT 0
> +
> +#define SDHC_SYS_EXT_OP_CTRL 0x010C
> +#define MASK_CMD_CONFLICT_ERROR BIT(8)
> +
> +#define SDHC_SLOT_OP_STATUS_CTRL 0x0128
> +#define DELAY_90_DEGREE_MASK_EMMC5 BIT(7)
> +#define DELAY_90_DEGREE_SHIFT_EMMC5 7
> +#define EMMC_5_0_PHY_FIXED_DELAY_MASK 0x7F
> +#define EMMC_PHY_FIXED_DELAY_MASK 0xFF
> +#define EMMC_PHY_FIXED_DELAY_WINDOW_MIN (EMMC_PHY_FIXED_DELAY_MASK >> 3)
> +#define SDH_PHY_FIXED_DELAY_MASK 0x1FF
> +#define SDH_PHY_FIXED_DELAY_WINDOW_MIN (SDH_PHY_FIXED_DELAY_MASK >> 4)
> +
> +#define TUN_CONSECUTIVE_TIMES_SHIFT 16
> +#define TUN_CONSECUTIVE_TIMES_MASK 0x7
> +#define TUN_CONSECUTIVE_TIMES 0x4
> +#define TUNING_STEP_SHIFT 12
> +#define TUNING_STEP_MASK 0xF
> +#define TUNING_STEP_DIVIDER BIT(6)
> +
> +#define FORCE_SEL_INVERSE_CLK_SHIFT 11
> +
> +#define SDHC_SLOT_EMMC_CTRL 0x0130
> +#define ENABLE_DATA_STROBE BIT(24)
> +#define SET_EMMC_RSTN BIT(16)
> +#define DISABLE_RD_DATA_CRC BIT(14)
> +#define DISABLE_CRC_STAT_TOKEN BIT(13)
> +#define EMMC_VCCQ_MASK 0x3
> +#define EMMC_VCCQ_1_8V 0x1
> +#define EMMC_VCCQ_3_3V 0x3
> +
> +#define SDHC_SLOT_RETUNING_REQ_CTRL 0x0144
> +/* retuning compatible */
> +#define RETUNING_COMPATIBLE 0x1
> +
> +#define SDHC_SLOT_EXT_PRESENT_STATE 0x014C
> +#define LOCK_STATE 0x1
> +
> +#define SDHC_SLOT_DLL_CUR_DLY_VAL 0x0150
> +
> +/* Tuning Parameter */
> +#define TMR_RETUN_NO_PRESENT 0xF
> +#define DEF_TUNING_COUNT 0x9
> +
> +#define MMC_TIMING_FAKE 0xFF
> +
> +#define DEFAULT_SDCLK_FREQ (400000)
> +
> +/* Xenon specific Mode Select value */
> +#define XENON_SDHCI_CTRL_HS200 0x5
> +#define XENON_SDHCI_CTRL_HS400 0x6
For all defines above:
All these defines needs some *SDHCI* prefix. Can you please update that.
> +
> +struct sdhci_xenon_priv {
> + /*
> + * The bus_width, timing, and clock fields in below
> + * record the current setting of Xenon SDHC.
> + * Driver will call a Sampling Fixed Delay Adjustment
> + * if any setting is changed.
> + */
> + unsigned char bus_width;
> + unsigned char timing;
These two are not used. Please remove.
> + unsigned char tuning_count;
> + unsigned int clock;
"clock" isn't used, please remove.
> + struct clk *axi_clk;
> +
> + /* Slot idx */
> + u8 slot_idx;
> + /* Whether this slot is for eMMC */
> + bool emmc_slot;
> +
> + /*
> + * When initializing card, Xenon has to determine card type and
> + * adjust Sampling Fixed delay for the speed mode in which
> + * DLL tuning is not support.
> + * However, at that time, card structure is not linked to mmc_host.
> + * Thus a card pointer is added here to provide
> + * the delay adjustment function with the card structure
> + * of the card during initialization.
> + *
> + * It is only valid during initialization after it is updated in
> + * xenon_init_card().
> + * Do not access this variable in normal transfers after
> + * initialization completes.
> + */
> + struct mmc_card *card_candidate;
Not activley used in this change, please remove and let's discuss it
in the next step.
> +};
> +
> +static inline int enable_xenon_internal_clk(struct sdhci_host *host)
> +{
> + u32 reg;
> + u8 timeout;
> +
> + reg = sdhci_readl(host, SDHCI_CLOCK_CONTROL);
> + reg |= SDHCI_CLOCK_INT_EN;
> + sdhci_writel(host, reg, SDHCI_CLOCK_CONTROL);
> + /* Wait max 20 ms */
> + timeout = 20;
> + while (!((reg = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
> + & SDHCI_CLOCK_INT_STABLE)) {
> + if (timeout == 0) {
> + pr_err("%s: Internal clock never stabilised.\n",
> + mmc_hostname(host->mmc));
> + return -ETIMEDOUT;
> + }
> + timeout--;
> + mdelay(1);
> + }
> +
> + return 0;
> +}
> +#endif
> --
> git-series 0.8.10
Kind regards
Uffe
^ permalink raw reply
* [PATCH] SCPI (pre-v1.0): fix reading sensor value
From: Sudeep Holla @ 2016-11-24 10:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161124001845.20830-1-martin.blumenstingl@googlemail.com>
On 24/11/16 00:18, Martin Blumenstingl wrote:
> I observed the following "strange" value when trying to read the SCPI
> temperature sensor on my Amlogic GXM S912 device:
> $ cat /sys/class/hwmon/hwmon0/temp1_input
> 6875990994467160116
>
> The value reported by the original kernel (Amlogic vendor kernel, after
> a reboot obviously) was 53C.
> The Amlogic SCPI driver only uses a single 32bit value to read the
> sensor value, instead of two. After stripping the upper 32bits from
> above value gives "52" as result, which is basically identical to
> what the vendor kernel reports.
Can you check why the upper 32-bit is not set to 0 ?
In scpi_process_cmd, we memset extra rx_buf length by 0 and that should
take care. Neil had mentioned that works but now I doubt if firmware
returns 8 instead of 4 in the size which is wrong as it supports only
32-bit.
--
Regards,
Sudeep
^ permalink raw reply
* Tearing down DMA transfer setup after DMA client has finished
From: Mason @ 2016-11-24 10:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <yw1xh96yaz84.fsf@unicorn.mansr.com>
On 23/11/2016 18:21, M?ns Rullg?rd wrote:
> Mason writes:
>
>> On 23/11/2016 13:13, M?ns Rullg?rd wrote:
>>
>>> Mason wrote:
>>>
>>>> On my platform, setting up a DMA transfer is a two-step process:
>>>>
>>>> 1) configure the "switch box" to connect a device to a memory channel
>>>> 2) configure the transfer details (address, size, command)
>>>>
>>>> When the transfer is done, the sbox setup can be torn down,
>>>> and the DMA driver can start another transfer.
>>>>
>>>> The current software architecture for my NFC (NAND Flash controller)
>>>> driver is as follows (for one DMA transfer).
>>>>
>>>> sg_init_one
>>>> dma_map_sg
>>>> dmaengine_prep_slave_sg
>>>> dmaengine_submit
>>>> dma_async_issue_pending
>>>> configure_NFC_transfer
>>>> wait_for_IRQ_from_DMA_engine // via DMA_PREP_INTERRUPT
>>>> wait_for_NFC_idle
>>>> dma_unmap_sg
>>>>
>>>> The problem is that the DMA driver tears down the sbox setup
>>>> as soon as it receives the IRQ. However, when writing to the
>>>> device, the interrupt only means "I have pushed all data from
>>>> memory to the memory channel". These data have not reached
>>>> the device yet, and may still be "in flight". Thus the sbox
>>>> setup can only be torn down after the NFC is idle.
>>>>
>>>> How do I call back into the DMA driver after wait_for_NFC_idle,
>>>> to request sbox tear down?
>>>>
>>>> The new architecture would become:
>>>>
>>>> sg_init_one
>>>> dma_map_sg
>>>> dmaengine_prep_slave_sg
>>>> dmaengine_submit
>>>> dma_async_issue_pending
>>>> configure_NFC_transfer
>>>> wait_for_IRQ_from_DMA_engine // via DMA_PREP_INTERRUPT
>>>> wait_for_NFC_idle
>>>> request_sbox_tear_down /*** HOW TO DO THAT ***/
>>>> dma_unmap_sg
>>>>
>>>> As far as I can tell, my NFC driver should call dmaengine_synchronize ??
>>>> (In other words request_sbox_tear_down == dmaengine_synchronize)
>>>>
>>>> So the DMA driver should implement the device_synchronize hook,
>>>> and tear the sbox down in that function.
>>>>
>>>> Is that correct? Or am I on the wrong track?
>>>
>>> dmaengine_synchronize() is not meant for this. See the documentation:
>>>
>>> /**
>>> * dmaengine_synchronize() - Synchronize DMA channel termination
>>> * @chan: The channel to synchronize
>>> *
>>> * Synchronizes to the DMA channel termination to the current context. When this
>>> * function returns it is guaranteed that all transfers for previously issued
>>> * descriptors have stopped and and it is safe to free the memory assoicated
>>> * with them. Furthermore it is guaranteed that all complete callback functions
>>> * for a previously submitted descriptor have finished running and it is safe to
>>> * free resources accessed from within the complete callbacks.
>>> *
>>> * The behavior of this function is undefined if dma_async_issue_pending() has
>>> * been called between dmaengine_terminate_async() and this function.
>>> *
>>> * This function must only be called from non-atomic context and must not be
>>> * called from within a complete callback of a descriptor submitted on the same
>>> * channel.
>>> */
>>>
>>> This is for use after a dmaengine_terminate_async() call to wait for the
>>> dma engine to finish whatever it was doing. This is not the problem
>>> here. Your problem is that the dma engine interrupt fires before the
>>> transfer is actually complete. Although you get an indication from the
>>> target device when it has received all the data, there is no way to make
>>> the dma driver wait for this.
>>
>> Hello Mans,
>>
>> I'm confused. Are you saying there is no solution to my problem
>> within the existing DMA framework?
>>
>> In its current form, the tangox-dma.c driver will fail randomly
>> for writes to a device (SATA, NFC).
>>
>> Maybe an extra hook can be added to the DMA framework?
>>
>> I'd like to hear from the framework's maintainers. Perhaps they
>> can provide some guidance.
>
> You could have the dma descriptor callback wait for the receiving device
> to finish. Bear in mind this runs from a tasklet, so it's not allowed
> to sleep.
Thanks for the suggestion, but I don't think it works :-(
This is my DMA desc callback:
static void tango_dma_callback(void *arg)
{
printk("%s from %pf\n", __func__, __builtin_return_address(0));
mdelay(10000);
printk("DONE FAKE SPINNING\n");
complete(arg);
}
I also added
printk("%s from %pf\n", __func__, __builtin_return_address(0));
after tangox_dma_pchan_detach(pchan);
And I get this output:
[ 35.085854] SETUP DMA
[ 35.088272] START NAND TRANSFER
[ 35.091670] tangox_dma_pchan_start from tangox_dma_irq
[ 35.096882] tango_dma_callback from vchan_complete
[ 45.102513] DONE FAKE SPINNING
So the IRQ rolls in, the ISR calls tangox_dma_pchan_start,
which calls tangox_dma_pchan_detach to tear down the sbox
setup; and only sometime later does the DMA framework call
my callback function.
So far, the work-arounds I've tested are:
1) delay sbox tear-down by 10 ?s in tangox_dma_pchan_detach.
2) statically setup sbox in probe, and never touch it henceforth.
WA1 is fragile, it might break for devices other than NFC.
WA2 is what I used when I wrote the NFC driver.
Can tangox_dma_irq() be changed to have the framework call
the client's callback *before* tangox_dma_pchan_start?
(Thinking out loud) The DMA_PREP_INTERRUPT requests that the
DMA framework invoke the callback from tasklet context,
maybe a different flag DMA_PREP_INTERRUPT_EX can request
calling the call-back directly from within the ISR?
(Looking at existing flags) Could I use DMA_CTRL_ACK?
Description sounds like some kind hand-shake between
client and dmaengine.
Grepping for DMA_PREP_INTERRUPT, I don't see where the framework
checks that flag to spawn the tasklet? Or is that up to each
driver individually?
Regards.
^ permalink raw reply
* [RFC PATCH 2/5] dmaengine: allow sun6i-dma for more SoCs
From: Maxime Ripard @ 2016-11-24 10:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAGb2v65G7=9ah+sEet=z5vss60kL5ZLSkNsAcGpwu8V6AWdEGA@mail.gmail.com>
On Thu, Nov 24, 2016 at 05:30:45PM +0800, Chen-Yu Tsai wrote:
> On Thu, Nov 24, 2016 at 5:16 PM, Andre Przywara <andre.przywara@arm.com> wrote:
> > Hi,
> >
> > On 24/11/16 04:16, Chen-Yu Tsai wrote:
> >> Hi,
> >>
> >> On Thu, Nov 24, 2016 at 9:17 AM, Andre Przywara <andre.przywara@arm.com> wrote:
> >>> The sun6i DMA driver is used in the Allwinner A64 and H5 SoC, which
> >>> have arm64 capable cores. Add the generic sunxi config symbol to allow
> >>> the driver to be selected by arm64 Kconfigs, which don't feature
> >>> SoC specific MACH_xxxx configs.
> >>>
> >>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> >>> ---
> >>> drivers/dma/Kconfig | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> >>> index af63a6b..003c284 100644
> >>> --- a/drivers/dma/Kconfig
> >>> +++ b/drivers/dma/Kconfig
> >>> @@ -157,7 +157,7 @@ config DMA_SUN4I
> >>>
> >>> config DMA_SUN6I
> >>> tristate "Allwinner A31 SoCs DMA support"
> >>> - depends on MACH_SUN6I || MACH_SUN8I || COMPILE_TEST
> >>> + depends on MACH_SUN6I || MACH_SUN8I || COMPILE_TEST || ARCH_SUNXI
> >>
> >> AFAIK ARCH_SUNXI encompasses/supersedes MACH_SUN*I.
> >> (And I don't have to add MACH_SUN9I later :) )
> >
> > Sure, admittedly it was just a quick hack to get things going.
> > Actually I don't know why we had a *depend* on those MACH_s before. I
> > think technically it does not depend on a certain SoC (having the
> > COMPILE_TEST in there hints on that). So what about:
>
> It was really because this DMA engine only comes with the later
> SoCs. We have dma-sun4i for the older one.
Indeed.
> But yes, there's no reason why you can't build it for the earlier
> SoC. It just doesn't get used.
I'm still in favor of keeping the depends on. There's no point of
compiling something we know have zero chance of running.
(But that would be (ARCH_SUNXI && ARM64))
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161124/a116f60d/attachment.sig>
^ permalink raw reply
* [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell Xenon SDHC
From: Ziji Hu @ 2016-11-24 10:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2204525.IWIYQVjIXl@wuerfel>
Hi Arnd,
On 2016/11/24 17:56, Arnd Bergmann wrote:
> On Monday, October 31, 2016 12:09:56 PM CET Gregory CLEMENT wrote:
>> From: Ziji Hu <huziji@marvell.com>
>>
>> Marvell Xenon eMMC/SD/SDIO Host Controller contains PHY.
>> Three types of PHYs are supported.
>>
>> Add support to multiple types of PHYs init and configuration.
>> Add register definitions of PHYs.
>>
>> Signed-off-by: Hu Ziji <huziji@marvell.com>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>>
>
> Please explain in the changelog why this is not a generic
> phy driver (or three of them).
>
Actually we tried to put the PHY code into Linux PHY framework.
But it cannot fit in Linux common PHY framework.
Our Xenon SDHC PHY register is a part of Xenon SDHC register set.
Besides, during MMC initialization, MMC sequence has to call several PHY functions to complete timing setting.
In those PHY setting functions, they have to access SDHC register and know current MMC setting, such as bus width, clock frequency and speed mode.
As a result, we have to implement PHY under MMC directory.
Thank you.
Best regards,
Hu Ziji
> Arnd
>
^ permalink raw reply
* [RFC PATCH 3/5] arm64: defconfig: sunxi: include options for Allwinner H5 SoC
From: Maxime Ripard @ 2016-11-24 10:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479950235-26821-4-git-send-email-andre.przywara@arm.com>
On Thu, Nov 24, 2016 at 01:17:13AM +0000, Andre Przywara wrote:
> The Allwinner H5 SoC is closely related to the H3 SoC, so select the
> basic pinctrl driver and the DMA driver to let a defconfig kernel boot
> on those boards.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
> arch/arm64/Kconfig.platforms | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
> index cfbdf02..8300677 100644
> --- a/arch/arm64/Kconfig.platforms
> +++ b/arch/arm64/Kconfig.platforms
> @@ -5,8 +5,12 @@ config ARCH_SUNXI
> select GENERIC_IRQ_CHIP
> select PINCTRL
> select PINCTRL_SUN50I_A64
> + select PINCTRL_SUN8I_H3
> + select PINCTRL_SUN8I_H3_R
> + select DMA_SUN6I
I'm not sure I want to get an ever growing select which will be an
union of all the drivers that all the arm64 Allwinner SoCs will
require.
Select leaves no option to disable that option, and we have defconfig
to deal with that nicely.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161124/00e005e3/attachment.sig>
^ permalink raw reply
* [RFC PATCH 0/5] arm64: Allwinner H5 support
From: Maxime Ripard @ 2016-11-24 10:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479950235-26821-1-git-send-email-andre.przywara@arm.com>
On Thu, Nov 24, 2016 at 01:17:10AM +0000, Andre Przywara wrote:
> This series adds support for the recently released Allwinner H5 SoC [1] and
> the Orange Pi PC 2 board [2].
> This exercise is rather easy this time, since the new SoC is very similar
> to the existing H3 SoC and can thus share a lot of support.
> To express this, the first patch splits the H3 .dtsi to allow reusing
> it later. The last two patches add the H5 .dtsi and the .dts for the
> first available board featuring this chip, based on that shared base DT.
>
> This is some early version, it's based on a merge of various -for-4.10
> branches from Maxime's repository.
> I can boot this on the OPi board and MMC and USB seem to work fine.
> I haven't tested any other peripherals yet.
> Some open issues:
> - The naming: Following the Allwinner scheme this should be "sun50i-h5"
> (which I use in this series), but it shares so much with the H3 that
> "sun8i-h5" wouldn't be wrong either. It gets a bit weird with that shared
> .dtsi, which I call sun8i-h3-h5.dtsi for now.
> - The clocks and pinctrl look _almost_ similar. I may sound like a broken
> record, but our habit of requiring kernel support for those almost identical
> SoCs really bites us now. As the MMC got updated, I fear there is _one_
> additional pin that we need for the HS400 transfer mode. Also I am afraid
> the MMC clock may be slightly different due to the advanced MMC support.
> At the moment this is not an issue, as the driver only support DDR50 at
> most anyway, so we get away with it now.
> I wonder if it's feasible to add those things to the existing H3 clocks
> and pinctrl to avoid another set of drivers.
> - I just see that I missed those patches that add just the names to the
> binding docs. I will send them once we agreed on the naming.
I don't have any major comments but I guess it all depends on the DT
maintainers view on the symbolic link to share the DTSI.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161124/938ef169/attachment-0001.sig>
^ permalink raw reply
* [RFC PATCH 3/5] arm64: defconfig: sunxi: include options for Allwinner H5 SoC
From: Chen-Yu Tsai @ 2016-11-24 11:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161124105725.va6gf6b3min74occ@lukather>
On Thu, Nov 24, 2016 at 6:57 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Thu, Nov 24, 2016 at 01:17:13AM +0000, Andre Przywara wrote:
>> The Allwinner H5 SoC is closely related to the H3 SoC, so select the
>> basic pinctrl driver and the DMA driver to let a defconfig kernel boot
>> on those boards.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>> ---
>> arch/arm64/Kconfig.platforms | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
>> index cfbdf02..8300677 100644
>> --- a/arch/arm64/Kconfig.platforms
>> +++ b/arch/arm64/Kconfig.platforms
>> @@ -5,8 +5,12 @@ config ARCH_SUNXI
>> select GENERIC_IRQ_CHIP
>> select PINCTRL
>> select PINCTRL_SUN50I_A64
>> + select PINCTRL_SUN8I_H3
>> + select PINCTRL_SUN8I_H3_R
>> + select DMA_SUN6I
>
> I'm not sure I want to get an ever growing select which will be an
> union of all the drivers that all the arm64 Allwinner SoCs will
> require.
>
> Select leaves no option to disable that option, and we have defconfig
> to deal with that nicely.
I have to agree. We should only select things that aren't selectable
by the user. In our case, that's only the pinctrl drivers.
We should use default y (ARCH_SUNXI && ARM64) for every driver that
has a prompt entry in Kconfig.
ChenYu
^ permalink raw reply
* [RFC PATCH 0/5] arm64: Allwinner H5 support
From: Andre Przywara @ 2016-11-24 11:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161124105958.xzp4fr2fs4p42upx@lukather>
Hi Maxime,
thanks for looking at this.
On 24/11/16 10:59, Maxime Ripard wrote:
> On Thu, Nov 24, 2016 at 01:17:10AM +0000, Andre Przywara wrote:
>> This series adds support for the recently released Allwinner H5 SoC [1] and
>> the Orange Pi PC 2 board [2].
>> This exercise is rather easy this time, since the new SoC is very similar
>> to the existing H3 SoC and can thus share a lot of support.
>> To express this, the first patch splits the H3 .dtsi to allow reusing
>> it later. The last two patches add the H5 .dtsi and the .dts for the
>> first available board featuring this chip, based on that shared base DT.
>>
>> This is some early version, it's based on a merge of various -for-4.10
>> branches from Maxime's repository.
>> I can boot this on the OPi board and MMC and USB seem to work fine.
>> I haven't tested any other peripherals yet.
>> Some open issues:
>> - The naming: Following the Allwinner scheme this should be "sun50i-h5"
>> (which I use in this series), but it shares so much with the H3 that
>> "sun8i-h5" wouldn't be wrong either. It gets a bit weird with that shared
>> .dtsi, which I call sun8i-h3-h5.dtsi for now.
>> - The clocks and pinctrl look _almost_ similar. I may sound like a broken
>> record, but our habit of requiring kernel support for those almost identical
>> SoCs really bites us now. As the MMC got updated, I fear there is _one_
>> additional pin that we need for the HS400 transfer mode. Also I am afraid
>> the MMC clock may be slightly different due to the advanced MMC support.
>> At the moment this is not an issue, as the driver only support DDR50 at
>> most anyway, so we get away with it now.
>> I wonder if it's feasible to add those things to the existing H3 clocks
>> and pinctrl to avoid another set of drivers.
>> - I just see that I missed those patches that add just the names to the
>> binding docs. I will send them once we agreed on the naming.
>
> I don't have any major comments but I guess it all depends on the DT
> maintainers view on the symbolic link to share the DTSI.
I am curious too ;-)
But I saw symlinks for the RaspberryPi 3 (check
arch/arm64/boot/dts/broadcom) and VExpress, so I picked that low hanging
fruit ;-)
Cheers,
Andre.
^ permalink raw reply
* [PATCH V6 1/1] ARM64/PCI: Manage controller-specific information on the host controller basis
From: Tomasz Nowicki @ 2016-11-24 11:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473449047-10499-1-git-send-email-tn@semihalf.com>
Currently we use one shared global acpi_pci_root_ops structure to keep
controller-specific ops. Then its pointer is passed to acpi_pci_root_create()
and associated with host bridge instance for good. Such design implies
serious drawback. Any potential manipulation on the single system-wide
acpi_pci_root_ops leads to kernel crash. The structure content is not
really changing even across multiple host bridges creation thus it was not
the issue so far.
In preparation for adding ECAM quirks mechanism (where controller-specific
PCI ops may be different for each host bridge) allocate new
acpi_pci_root_ops and fill in with data for each bridge. Now it is safe
to have different controller-specific info. As a consequence free
acpi_pci_root_ops when host bridge is released.
No functional changes in this patch.
Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
---
arch/arm64/kernel/pci.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
index fb439c7..5c08baf 100644
--- a/arch/arm64/kernel/pci.c
+++ b/arch/arm64/kernel/pci.c
@@ -152,33 +152,36 @@ static void pci_acpi_generic_release_info(struct acpi_pci_root_info *ci)
ri = container_of(ci, struct acpi_pci_generic_root_info, common);
pci_ecam_free(ri->cfg);
+ kfree(ci->ops);
kfree(ri);
}
-static struct acpi_pci_root_ops acpi_pci_root_ops = {
- .release_info = pci_acpi_generic_release_info,
-};
-
/* Interface called from ACPI code to setup PCI host controller */
struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
{
int node = acpi_get_node(root->device->handle);
struct acpi_pci_generic_root_info *ri;
struct pci_bus *bus, *child;
+ struct acpi_pci_root_ops *root_ops;
ri = kzalloc_node(sizeof(*ri), GFP_KERNEL, node);
if (!ri)
return NULL;
+ root_ops = kzalloc_node(sizeof(*root_ops), GFP_KERNEL, node);
+ if (!root_ops)
+ return NULL;
+
ri->cfg = pci_acpi_setup_ecam_mapping(root);
if (!ri->cfg) {
kfree(ri);
+ kfree(root_ops);
return NULL;
}
- acpi_pci_root_ops.pci_ops = &ri->cfg->ops->pci_ops;
- bus = acpi_pci_root_create(root, &acpi_pci_root_ops, &ri->common,
- ri->cfg);
+ root_ops->release_info = pci_acpi_generic_release_info;
+ root_ops->pci_ops = &ri->cfg->ops->pci_ops;
+ bus = acpi_pci_root_create(root, root_ops, &ri->common, ri->cfg);
if (!bus)
return NULL;
--
2.7.4
^ permalink raw reply related
* [RFC PATCH 3/5] arm64: defconfig: sunxi: include options for Allwinner H5 SoC
From: Andre Przywara @ 2016-11-24 11:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAGb2v67AntaTBCTzJf_-oUcAqaFsQv3t4pp-E6jzpkXLNctKww@mail.gmail.com>
Hi,
On 24/11/16 11:01, Chen-Yu Tsai wrote:
> On Thu, Nov 24, 2016 at 6:57 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
>> On Thu, Nov 24, 2016 at 01:17:13AM +0000, Andre Przywara wrote:
>>> The Allwinner H5 SoC is closely related to the H3 SoC, so select the
>>> basic pinctrl driver and the DMA driver to let a defconfig kernel boot
>>> on those boards.
>>>
>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>> ---
>>> arch/arm64/Kconfig.platforms | 6 +++++-
>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
>>> index cfbdf02..8300677 100644
>>> --- a/arch/arm64/Kconfig.platforms
>>> +++ b/arch/arm64/Kconfig.platforms
>>> @@ -5,8 +5,12 @@ config ARCH_SUNXI
>>> select GENERIC_IRQ_CHIP
>>> select PINCTRL
>>> select PINCTRL_SUN50I_A64
>>> + select PINCTRL_SUN8I_H3
>>> + select PINCTRL_SUN8I_H3_R
>>> + select DMA_SUN6I
>>
>> I'm not sure I want to get an ever growing select which will be an
>> union of all the drivers that all the arm64 Allwinner SoCs will
>> require.
>>
>> Select leaves no option to disable that option, and we have defconfig
>> to deal with that nicely.
> I have to agree. We should only select things that aren't selectable
> by the user. In our case, that's only the pinctrl drivers.
Right, I missed that.
> We should use default y (ARCH_SUNXI && ARM64) for every driver that
> has a prompt entry in Kconfig.
Sounds like a plan. I will look into this.
Thanks,
Andre.
^ permalink raw reply
* [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell Xenon SDHC
From: Arnd Bergmann @ 2016-11-24 11:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <f7334e41-39dd-f868-ff10-199afaebe926@marvell.com>
On Thursday, November 24, 2016 6:57:18 PM CET Ziji Hu wrote:
> >
> > Please explain in the changelog why this is not a generic
> > phy driver (or three of them).
> >
> Actually we tried to put the PHY code into Linux PHY framework.
> But it cannot fit in Linux common PHY framework.
>
> Our Xenon SDHC PHY register is a part of Xenon SDHC register set.
> Besides, during MMC initialization, MMC sequence has to call several PHY functions to complete timing setting.
> In those PHY setting functions, they have to access SDHC register and know current MMC setting, such as bus width, clock frequency and speed mode.
> As a result, we have to implement PHY under MMC directory.
>
Ok, that makes sense, just put the same text in the changelog comment.
Arnd
^ permalink raw reply
* [PATCH V9 11/11] ARM64/PCI: Support for ACPI based PCI host controller
From: Tomasz Nowicki @ 2016-11-24 11:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161123182243.GF16033@bhelgaas-glaptop.roam.corp.google.com>
On 23.11.2016 19:22, Bjorn Helgaas wrote:
> On Wed, Nov 23, 2016 at 12:21:03PM +0100, Tomasz Nowicki wrote:
>> Hi Bjorn,
>>
>> On 23.11.2016 00:13, Bjorn Helgaas wrote:
>>> Hi Tomasz,
>>>
>>> On Fri, Jun 10, 2016 at 09:55:19PM +0200, Tomasz Nowicki wrote:
>>>> Implement pci_acpi_scan_root and other arch-specific call so that ARM64
>>>> can start using ACPI to setup and enumerate PCI buses.
>>>>
>>>> Prior to buses enumeration the pci_acpi_scan_root() implementation looks
>>>> for configuration space start address (obtained through ACPI _CBA method or
>>>> MCFG interface). If succeed, it uses ECAM library to create new mapping.
>>>> Then it attaches generic ECAM ops (pci_generic_ecam_ops) which are used
>>>> for accessing configuration space later on.
>>>> ...
>>>
>>>> +static struct acpi_pci_root_ops acpi_pci_root_ops = {
>>>> + .release_info = pci_acpi_generic_release_info,
>>>> +};
>>>> +
>>>> +/* Interface called from ACPI code to setup PCI host controller */
>>>> struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
>>>> {
>>>> - /* TODO: Should be revisited when implementing PCI on ACPI */
>>>> - return NULL;
>>>> + int node = acpi_get_node(root->device->handle);
>>>> + struct acpi_pci_generic_root_info *ri;
>>>> + struct pci_bus *bus, *child;
>>>> +
>>>> + ri = kzalloc_node(sizeof(*ri), GFP_KERNEL, node);
>>>> + if (!ri)
>>>> + return NULL;
>>>> +
>>>> + ri->cfg = pci_acpi_setup_ecam_mapping(root);
>>>> + if (!ri->cfg) {
>>>> + kfree(ri);
>>>> + return NULL;
>>>> + }
>>>> +
>>>> + acpi_pci_root_ops.pci_ops = &ri->cfg->ops->pci_ops;
>>>
>>> This has already been merged, but this isn't right, is it? We're
>>> writing a host controller-specific pointer into the single system-wide
>>> acpi_pci_root_ops, then passing it on to acpi_pci_root_create().
>>>
>>> Today, I think ri->cfg->ops->pci_ops is always &pci_generic_ecam_ops,
>> >from this path:
>>>
>>> ri->cfg = pci_acpi_setup_ecam_mapping
>>> cfg = pci_ecam_create(..., &pci_generic_ecam_ops)
>>> cfg = kzalloc(...)
>>> cfg->ops = ops # &pci_generic_ecam_ops
>>>
>>> But we're about to merge the ECAM quirks series, which will mean it
>>> may not be &pci_generic_ecam_ops. Even apart from the ECAM quirks, we
>>> should avoid this pattern of putting device-specific info in a single
>>> shared structure because it's too difficult to verify that it's
>>> correct.
>>>
>>
>> Well spotted. I agree, we need to fix this. How about this:
>> diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
>> index fb439c7..31c0e1c 100644
>> --- a/arch/arm64/kernel/pci.c
>> +++ b/arch/arm64/kernel/pci.c
>> @@ -152,33 +152,35 @@ static void
>> pci_acpi_generic_release_info(struct acpi_pci_root_info *ci)
>>
>> ri = container_of(ci, struct acpi_pci_generic_root_info, common);
>> pci_ecam_free(ri->cfg);
>> + kfree(ci->ops);
>> kfree(ri);
>> }
>>
>> -static struct acpi_pci_root_ops acpi_pci_root_ops = {
>> - .release_info = pci_acpi_generic_release_info,
>> -};
>> -
>> /* Interface called from ACPI code to setup PCI host controller */
>> struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
>> {
>> int node = acpi_get_node(root->device->handle);
>> struct acpi_pci_generic_root_info *ri;
>> struct pci_bus *bus, *child;
>> + struct acpi_pci_root_ops *root_ops;
>>
>> ri = kzalloc_node(sizeof(*ri), GFP_KERNEL, node);
>> if (!ri)
>> return NULL;
>>
>> + root_ops = kzalloc_node(sizeof(*root_ops), GFP_KERNEL, node);
>> + if (!root_ops)
>> + return NULL;
>> +
>> ri->cfg = pci_acpi_setup_ecam_mapping(root);
>> if (!ri->cfg) {
>> kfree(ri);
>> + kfree(root_ops);
>> return NULL;
>> }
>>
>> - acpi_pci_root_ops.pci_ops = &ri->cfg->ops->pci_ops;
>> - bus = acpi_pci_root_create(root, &acpi_pci_root_ops, &ri->common,
>> - ri->cfg);
>> + root_ops->release_info = pci_acpi_generic_release_info;
>> + root_ops->pci_ops = &ri->cfg->ops->pci_ops;
>> + bus = acpi_pci_root_create(root, root_ops, &ri->common, ri->cfg);
>> if (!bus)
>> return NULL;
>>
>> Of course, this should be the part of ECAM quirks core patches.
>>
>> The other option we have is to remove "struct pci_ops *pci_ops;"
>> from acpi_pci_root_ops structure and pass struct pci_ops as an extra
>> argument to acpi_pci_root_create(). What do you think?
>
> I think your patch above is fine and avoids the need to change the x86 and
> ia64 code. Would you mind packaging this up with a changelog and
> signed-off-by? I can take care of putting it in the ECAM series.
>
Sure, I have just sent the patch in replay to ECAM quirks V6 patch set.
Let us know when you update your branch so we base our quirks on it.
Thanks,
Tomasz
^ permalink raw reply
* [PATCH] SCPI (pre-v1.0): fix reading sensor value
From: Martin Blumenstingl @ 2016-11-24 11:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <f3d6f39e-ec45-7c62-22d4-fd7b0952b499@arm.com>
On Thu, Nov 24, 2016 at 11:47 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
>
> On 24/11/16 00:18, Martin Blumenstingl wrote:
>>
>> I observed the following "strange" value when trying to read the SCPI
>> temperature sensor on my Amlogic GXM S912 device:
>> $ cat /sys/class/hwmon/hwmon0/temp1_input
>> 6875990994467160116
>>
>> The value reported by the original kernel (Amlogic vendor kernel, after
>> a reboot obviously) was 53C.
>> The Amlogic SCPI driver only uses a single 32bit value to read the
>> sensor value, instead of two. After stripping the upper 32bits from
>> above value gives "52" as result, which is basically identical to
>> what the vendor kernel reports.
>
>
> Can you check why the upper 32-bit is not set to 0 ?
>
> In scpi_process_cmd, we memset extra rx_buf length by 0 and that should
> take care. Neil had mentioned that works but now I doubt if firmware
> returns 8 instead of 4 in the size which is wrong as it supports only
> 32-bit.
according to the code "RX Length is not replied by the legacy
Firmware", so for legacy firmwares the "if (match->rx_len > len)"
condition will never be true (because both values are always equal).
in the sensor case we then go and copy 8 byte from mem->payload to
match->rx_buf, but SCPI firmware only wrote 4 bytes to mem->payload.
This means we are simply reading 4 byte (hi_val) of uninitialized
memory - which may be all zeroes if we're lucky - but in my case I got
"garbage" (I guess it's the second byte from the *previous* command
which are leaking here).
while writing this I see a second (more generic) approach which might
work as well:
scpi_chan does not hold any information about rx_payload/tx_payload
sizes (these are calculated in scpi_probe but not stored anywhere).
(for now, let's assume we had the rx_payload_size available)
we could then go ahead and memset(rx_payload, 0, rx_payload_size) in
scpi_tx_prepare or scpi_send_message.
However, I am not sure if that would have any side-effects (for
example on newer SCPI implementations).
Regards,
Martin
^ permalink raw reply
* [RFC PATCH 2/5] dmaengine: allow sun6i-dma for more SoCs
From: Andre Przywara @ 2016-11-24 11:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161124105516.tv2ybfgka5teiv2h@lukather>
On 24/11/16 10:55, Maxime Ripard wrote:
> On Thu, Nov 24, 2016 at 05:30:45PM +0800, Chen-Yu Tsai wrote:
>> On Thu, Nov 24, 2016 at 5:16 PM, Andre Przywara <andre.przywara@arm.com> wrote:
>>> Hi,
>>>
>>> On 24/11/16 04:16, Chen-Yu Tsai wrote:
>>>> Hi,
>>>>
>>>> On Thu, Nov 24, 2016 at 9:17 AM, Andre Przywara <andre.przywara@arm.com> wrote:
>>>>> The sun6i DMA driver is used in the Allwinner A64 and H5 SoC, which
>>>>> have arm64 capable cores. Add the generic sunxi config symbol to allow
>>>>> the driver to be selected by arm64 Kconfigs, which don't feature
>>>>> SoC specific MACH_xxxx configs.
>>>>>
>>>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>>>> ---
>>>>> drivers/dma/Kconfig | 2 +-
>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
>>>>> index af63a6b..003c284 100644
>>>>> --- a/drivers/dma/Kconfig
>>>>> +++ b/drivers/dma/Kconfig
>>>>> @@ -157,7 +157,7 @@ config DMA_SUN4I
>>>>>
>>>>> config DMA_SUN6I
>>>>> tristate "Allwinner A31 SoCs DMA support"
>>>>> - depends on MACH_SUN6I || MACH_SUN8I || COMPILE_TEST
>>>>> + depends on MACH_SUN6I || MACH_SUN8I || COMPILE_TEST || ARCH_SUNXI
>>>>
>>>> AFAIK ARCH_SUNXI encompasses/supersedes MACH_SUN*I.
>>>> (And I don't have to add MACH_SUN9I later :) )
>>>
>>> Sure, admittedly it was just a quick hack to get things going.
>>> Actually I don't know why we had a *depend* on those MACH_s before. I
>>> think technically it does not depend on a certain SoC (having the
>>> COMPILE_TEST in there hints on that). So what about:
>>
>> It was really because this DMA engine only comes with the later
>> SoCs. We have dma-sun4i for the older one.
>
> Indeed.
>
>> But yes, there's no reason why you can't build it for the earlier
>> SoC. It just doesn't get used.
>
> I'm still in favor of keeping the depends on. There's no point of
> compiling something we know have zero chance of running.
>
> (But that would be (ARCH_SUNXI && ARM64))
I am OK with that, just wondering if there is a definition of what
"depends" really means. My impression what that it's a about code
dependencies (requires a certain subsystem, for instance), not really if
it's useful in a particular configuration.
Cheers,
Andre.
^ permalink raw reply
* [PATCH RFC] drm/sun4i: rgb: Add 5% tolerance to dot clock frequency check
From: Chen-Yu Tsai @ 2016-11-24 11:22 UTC (permalink / raw)
To: linux-arm-kernel
The panels shipped with Allwinner devices are very "generic", i.e.
they do not have model numbers or reliable sources of information
for the timings (that we know of) other than the fex files shipped
on them. The dot clock frequency provided in the fex files have all
been rounded to the nearest MHz, as that is the unit used in them.
We were using the simple panel "urt,umsh-8596md-t" as a substitute
for the A13 Q8 tablets in the absence of a specific model for what
may be many different but otherwise timing compatible panels. This
was usable without any visual artifacts or side effects, until the
dot clock rate check was added in commit bb43d40d7c83 ("drm/sun4i:
rgb: Validate the clock rate").
The reason this check fails is because the dotclock frequency for
this model is 33.26 MHz, which is not achievable with our dot clock
hardware, and the rate returned by clk_round_rate deviates slightly,
causing the driver to reject the display mode.
The LCD panels have some tolerance on the dot clock frequency, even
if it's not specified in their datasheets.
This patch adds a 5% tolerence to the dot clock check.
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
The few LCD panel datasheets I found did not list minimums or maximums
for the dot clock rate. The 5% tolerance is just something I made up.
The point is to be able to use our dot clock, which doesn't have the
resolution needed to generate the exact clock rate requested. AFAIK
the sun4i driver is one of the strictest ones with regards to the dot
clock frequency. Some drivers don't even check it.
The clock rate given in vendor fex files are already rounded down to
MHz resolution. I doubt not using the exact rate as specified in simple
panels would cause any issues. But my experience is limited here.
Feedback on this is requested.
---
drivers/gpu/drm/sun4i/sun4i_rgb.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_rgb.c b/drivers/gpu/drm/sun4i/sun4i_rgb.c
index d198ad7e5323..66ad86afa561 100644
--- a/drivers/gpu/drm/sun4i/sun4i_rgb.c
+++ b/drivers/gpu/drm/sun4i/sun4i_rgb.c
@@ -93,11 +93,12 @@ static int sun4i_rgb_mode_valid(struct drm_connector *connector,
DRM_DEBUG_DRIVER("Vertical parameters OK\n");
+ /* Check against a 5% tolerance for the dot clock */
rounded_rate = clk_round_rate(tcon->dclk, rate);
- if (rounded_rate < rate)
+ if (rounded_rate < rate * 19 / 20)
return MODE_CLOCK_LOW;
- if (rounded_rate > rate)
+ if (rounded_rate > rate * 21 / 20)
return MODE_CLOCK_HIGH;
DRM_DEBUG_DRIVER("Clock rate OK\n");
--
2.10.2
^ permalink raw reply related
* [PATCH 0/5] spi: atmel: enhance the DMA handling
From: Nicolas Ferre @ 2016-11-24 11:24 UTC (permalink / raw)
To: linux-arm-kernel
Use the dummy buffer handling and DMA mapping core infrastructure to simplify
spi-atmel driver.
By using these frameworks we also fix the use of any kind of memory with
dmaengine. This work was triggered by issues encountered while using UBI/UBIFS
on a serial flash.
Some good ideas of this series have been inspired by spi-rspi.c and commits by
Geert Uytterhoeven.
I have a side question still: is the is_dma_mapped tag still used and considered into
the core spi routines or is it always the driver itself that have to deal with
it (so how to do with the can_dma() routine)?
Cyrille Pitchen (1):
spi: atmel: Use SPI core DMA mapping framework
Nicolas Ferre (4):
spi: atmel: trivial: move info banner to latest probe action
spi: atmel: Use core SPI_MASTER_MUST_[RT]X handling
spi: atmel: trivial: remove unused fields in DMA structure
spi: atmel: remove the use of private channel fields
drivers/spi/spi-atmel.c | 268 ++++++++++++++++++------------------------------
1 file changed, 98 insertions(+), 170 deletions(-)
--
2.9.0
^ permalink raw reply
* [PATCH 1/5] spi: atmel: trivial: move info banner to latest probe action
From: Nicolas Ferre @ 2016-11-24 11:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1479985886.git.nicolas.ferre@atmel.com>
The info banner is here to tell that everything went well, so place
it at the very end of the probe function.
Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
drivers/spi/spi-atmel.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index a038ffe90766..68e22bf94647 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -1658,10 +1658,6 @@ static int atmel_spi_probe(struct platform_device *pdev)
spi_writel(as, CR, SPI_BIT(FIFOEN));
}
- /* go! */
- dev_info(&pdev->dev, "Atmel SPI Controller at 0x%08lx (irq %d)\n",
- (unsigned long)regs->start, irq);
-
pm_runtime_set_autosuspend_delay(&pdev->dev, AUTOSUSPEND_TIMEOUT);
pm_runtime_use_autosuspend(&pdev->dev);
pm_runtime_set_active(&pdev->dev);
@@ -1671,6 +1667,10 @@ static int atmel_spi_probe(struct platform_device *pdev)
if (ret)
goto out_free_dma;
+ /* go! */
+ dev_info(&pdev->dev, "Atmel SPI Controller at 0x%08lx (irq %d)\n",
+ (unsigned long)regs->start, irq);
+
return 0;
out_free_dma:
--
2.9.0
^ permalink raw reply related
* [PATCH 2/5] spi: atmel: Use core SPI_MASTER_MUST_[RT]X handling
From: Nicolas Ferre @ 2016-11-24 11:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1479985886.git.nicolas.ferre@atmel.com>
We need both RX and TX data for each transfer in any case (PIO, PDC, DMA).
So convert the driver to the core dummy buffer handling with the
SPI_MASTER_MUST_RX/SPI_MASTER_MUST_TX infrastructure.
This move changes the maximum PDC/DMA buffer handling to 65535 bytes
instead of a single page and sets master->max_dma_len to this value.
All dummy buffer management is removed from the driver.
Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
drivers/spi/spi-atmel.c | 131 +++++++++++++-----------------------------------
1 file changed, 35 insertions(+), 96 deletions(-)
diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index 68e22bf94647..d0a56e11e148 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -304,10 +304,6 @@ struct atmel_spi {
struct completion xfer_completion;
- /* scratch buffer */
- void *buffer;
- dma_addr_t buffer_dma;
-
struct atmel_spi_caps caps;
bool use_dma;
@@ -328,7 +324,7 @@ struct atmel_spi_device {
u32 csr;
};
-#define BUFFER_SIZE PAGE_SIZE
+#define SPI_MAX_DMA_XFER 65535 /* true for both PDC and DMA */
#define INVALID_DMA_ADDRESS 0xffffffff
/*
@@ -613,14 +609,10 @@ static void atmel_spi_next_xfer_single(struct spi_master *master,
cpu_relax();
}
- if (xfer->tx_buf) {
- if (xfer->bits_per_word > 8)
- spi_writel(as, TDR, *(u16 *)(xfer->tx_buf + xfer_pos));
- else
- spi_writel(as, TDR, *(u8 *)(xfer->tx_buf + xfer_pos));
- } else {
- spi_writel(as, TDR, 0);
- }
+ if (xfer->bits_per_word > 8)
+ spi_writel(as, TDR, *(u16 *)(xfer->tx_buf + xfer_pos));
+ else
+ spi_writel(as, TDR, *(u8 *)(xfer->tx_buf + xfer_pos));
dev_dbg(master->dev.parent,
" start pio xfer %p: len %u tx %p rx %p bitpw %d\n",
@@ -667,17 +659,12 @@ static void atmel_spi_next_xfer_fifo(struct spi_master *master,
/* Fill TX FIFO */
while (num_data >= 2) {
- if (xfer->tx_buf) {
- if (xfer->bits_per_word > 8) {
- td0 = *words++;
- td1 = *words++;
- } else {
- td0 = *bytes++;
- td1 = *bytes++;
- }
+ if (xfer->bits_per_word > 8) {
+ td0 = *words++;
+ td1 = *words++;
} else {
- td0 = 0;
- td1 = 0;
+ td0 = *bytes++;
+ td1 = *bytes++;
}
spi_writel(as, TDR, (td1 << 16) | td0);
@@ -685,14 +672,10 @@ static void atmel_spi_next_xfer_fifo(struct spi_master *master,
}
if (num_data) {
- if (xfer->tx_buf) {
- if (xfer->bits_per_word > 8)
- td0 = *words++;
- else
- td0 = *bytes++;
- } else {
- td0 = 0;
- }
+ if (xfer->bits_per_word > 8)
+ td0 = *words++;
+ else
+ td0 = *bytes++;
spi_writew(as, TDR, td0);
num_data--;
@@ -751,24 +734,14 @@ static int atmel_spi_next_xfer_dma_submit(struct spi_master *master,
/* prepare the RX dma transfer */
sg_init_table(&as->dma.sgrx, 1);
- if (xfer->rx_buf) {
- as->dma.sgrx.dma_address = xfer->rx_dma + xfer->len - *plen;
- } else {
- as->dma.sgrx.dma_address = as->buffer_dma;
- if (len > BUFFER_SIZE)
- len = BUFFER_SIZE;
- }
+ as->dma.sgrx.dma_address = xfer->rx_dma + xfer->len - *plen;
/* prepare the TX dma transfer */
sg_init_table(&as->dma.sgtx, 1);
- if (xfer->tx_buf) {
- as->dma.sgtx.dma_address = xfer->tx_dma + xfer->len - *plen;
- } else {
- as->dma.sgtx.dma_address = as->buffer_dma;
- if (len > BUFFER_SIZE)
- len = BUFFER_SIZE;
- memset(as->buffer, 0, len);
- }
+ as->dma.sgtx.dma_address = xfer->tx_dma + xfer->len - *plen;
+
+ if (len > master->max_dma_len)
+ len = master->max_dma_len;
sg_dma_len(&as->dma.sgtx) = len;
sg_dma_len(&as->dma.sgrx) = len;
@@ -835,25 +808,10 @@ static void atmel_spi_next_xfer_data(struct spi_master *master,
struct atmel_spi *as = spi_master_get_devdata(master);
u32 len = *plen;
- /* use scratch buffer only when rx or tx data is unspecified */
- if (xfer->rx_buf)
- *rx_dma = xfer->rx_dma + xfer->len - *plen;
- else {
- *rx_dma = as->buffer_dma;
- if (len > BUFFER_SIZE)
- len = BUFFER_SIZE;
- }
-
- if (xfer->tx_buf)
- *tx_dma = xfer->tx_dma + xfer->len - *plen;
- else {
- *tx_dma = as->buffer_dma;
- if (len > BUFFER_SIZE)
- len = BUFFER_SIZE;
- memset(as->buffer, 0, len);
- dma_sync_single_for_device(&as->pdev->dev,
- as->buffer_dma, len, DMA_TO_DEVICE);
- }
+ *rx_dma = xfer->rx_dma + xfer->len - *plen;
+ *tx_dma = xfer->tx_dma + xfer->len - *plen;
+ if (len > master->max_dma_len)
+ len = master->max_dma_len;
*plen = len;
}
@@ -1027,16 +985,12 @@ atmel_spi_pump_single_data(struct atmel_spi *as, struct spi_transfer *xfer)
u16 *rxp16;
unsigned long xfer_pos = xfer->len - as->current_remaining_bytes;
- if (xfer->rx_buf) {
- if (xfer->bits_per_word > 8) {
- rxp16 = (u16 *)(((u8 *)xfer->rx_buf) + xfer_pos);
- *rxp16 = spi_readl(as, RDR);
- } else {
- rxp = ((u8 *)xfer->rx_buf) + xfer_pos;
- *rxp = spi_readl(as, RDR);
- }
+ if (xfer->bits_per_word > 8) {
+ rxp16 = (u16 *)(((u8 *)xfer->rx_buf) + xfer_pos);
+ *rxp16 = spi_readl(as, RDR);
} else {
- spi_readl(as, RDR);
+ rxp = ((u8 *)xfer->rx_buf) + xfer_pos;
+ *rxp = spi_readl(as, RDR);
}
if (xfer->bits_per_word > 8) {
if (as->current_remaining_bytes > 2)
@@ -1075,12 +1029,10 @@ atmel_spi_pump_fifo_data(struct atmel_spi *as, struct spi_transfer *xfer)
/* Read data */
while (num_data) {
rd = spi_readl(as, RDR);
- if (xfer->rx_buf) {
- if (xfer->bits_per_word > 8)
- *words++ = rd;
- else
- *bytes++ = rd;
- }
+ if (xfer->bits_per_word > 8)
+ *words++ = rd;
+ else
+ *bytes++ = rd;
num_data--;
}
}
@@ -1562,29 +1514,22 @@ static int atmel_spi_probe(struct platform_device *pdev)
master->bus_num = pdev->id;
master->num_chipselect = master->dev.of_node ? 0 : 4;
master->setup = atmel_spi_setup;
+ master->flags = (SPI_MASTER_MUST_RX | SPI_MASTER_MUST_TX);
master->transfer_one_message = atmel_spi_transfer_one_message;
master->cleanup = atmel_spi_cleanup;
master->auto_runtime_pm = true;
+ master->max_dma_len = SPI_MAX_DMA_XFER;
platform_set_drvdata(pdev, master);
as = spi_master_get_devdata(master);
- /*
- * Scratch buffer is used for throwaway rx and tx data.
- * It's coherent to minimize dcache pollution.
- */
- as->buffer = dma_alloc_coherent(&pdev->dev, BUFFER_SIZE,
- &as->buffer_dma, GFP_KERNEL);
- if (!as->buffer)
- goto out_free;
-
spin_lock_init(&as->lock);
as->pdev = pdev;
as->regs = devm_ioremap_resource(&pdev->dev, regs);
if (IS_ERR(as->regs)) {
ret = PTR_ERR(as->regs);
- goto out_free_buffer;
+ goto out_unmap_regs;
}
as->phybase = regs->start;
as->irq = irq;
@@ -1685,9 +1630,6 @@ static int atmel_spi_probe(struct platform_device *pdev)
clk_disable_unprepare(clk);
out_free_irq:
out_unmap_regs:
-out_free_buffer:
- dma_free_coherent(&pdev->dev, BUFFER_SIZE, as->buffer,
- as->buffer_dma);
out_free:
spi_master_put(master);
return ret;
@@ -1712,9 +1654,6 @@ static int atmel_spi_remove(struct platform_device *pdev)
spi_readl(as, SR);
spin_unlock_irq(&as->lock);
- dma_free_coherent(&pdev->dev, BUFFER_SIZE, as->buffer,
- as->buffer_dma);
-
clk_disable_unprepare(as->clk);
pm_runtime_put_noidle(&pdev->dev);
--
2.9.0
^ permalink raw reply related
* [PATCH 3/5] spi: atmel: Use SPI core DMA mapping framework
From: Nicolas Ferre @ 2016-11-24 11:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1479985886.git.nicolas.ferre@atmel.com>
From: Cyrille Pitchen <cyrille.pitchen@atmel.com>
Use the SPI core DMA mapping framework instead of our own
in case of DMA support. PDC support is not converted to this
framework.
The driver is now able to transfer a complete sg list through DMA.
This eventually fix an issue with vmalloc'ed DMA memory that is
provided for example by UBI/UBIFS layers.
Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
[nicolas.ferre at atmel.com: restrict the use to non-PDC DMA]
Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
drivers/spi/spi-atmel.c | 57 ++++++++++++++++++++++---------------------------
1 file changed, 25 insertions(+), 32 deletions(-)
diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index d0a56e11e148..f62bc2d27c9e 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -268,8 +268,6 @@
struct atmel_spi_dma {
struct dma_chan *chan_rx;
struct dma_chan *chan_tx;
- struct scatterlist sgrx;
- struct scatterlist sgtx;
struct dma_async_tx_descriptor *data_desc_rx;
struct dma_async_tx_descriptor *data_desc_tx;
@@ -454,6 +452,15 @@ static inline bool atmel_spi_use_dma(struct atmel_spi *as,
return as->use_dma && xfer->len >= DMA_MIN_BYTES;
}
+static bool atmel_spi_can_dma(struct spi_master *master,
+ struct spi_device *spi,
+ struct spi_transfer *xfer)
+{
+ struct atmel_spi *as = spi_master_get_devdata(master);
+
+ return atmel_spi_use_dma(as, xfer);
+}
+
static int atmel_spi_dma_slave_config(struct atmel_spi *as,
struct dma_slave_config *slave_config,
u8 bits_per_word)
@@ -721,7 +728,6 @@ static int atmel_spi_next_xfer_dma_submit(struct spi_master *master,
struct dma_async_tx_descriptor *txdesc;
struct dma_slave_config slave_config;
dma_cookie_t cookie;
- u32 len = *plen;
dev_vdbg(master->dev.parent, "atmel_spi_next_xfer_dma_submit\n");
@@ -732,34 +738,22 @@ static int atmel_spi_next_xfer_dma_submit(struct spi_master *master,
/* release lock for DMA operations */
atmel_spi_unlock(as);
- /* prepare the RX dma transfer */
- sg_init_table(&as->dma.sgrx, 1);
- as->dma.sgrx.dma_address = xfer->rx_dma + xfer->len - *plen;
-
- /* prepare the TX dma transfer */
- sg_init_table(&as->dma.sgtx, 1);
- as->dma.sgtx.dma_address = xfer->tx_dma + xfer->len - *plen;
-
- if (len > master->max_dma_len)
- len = master->max_dma_len;
-
- sg_dma_len(&as->dma.sgtx) = len;
- sg_dma_len(&as->dma.sgrx) = len;
-
- *plen = len;
+ *plen = xfer->len;
if (atmel_spi_dma_slave_config(as, &slave_config,
xfer->bits_per_word))
goto err_exit;
/* Send both scatterlists */
- rxdesc = dmaengine_prep_slave_sg(rxchan, &as->dma.sgrx, 1,
+ rxdesc = dmaengine_prep_slave_sg(rxchan,
+ xfer->rx_sg.sgl, xfer->rx_sg.nents,
DMA_FROM_DEVICE,
DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
if (!rxdesc)
goto err_dma;
- txdesc = dmaengine_prep_slave_sg(txchan, &as->dma.sgtx, 1,
+ txdesc = dmaengine_prep_slave_sg(txchan,
+ xfer->tx_sg.sgl, xfer->tx_sg.nents,
DMA_TO_DEVICE,
DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
if (!txdesc)
@@ -805,15 +799,10 @@ static void atmel_spi_next_xfer_data(struct spi_master *master,
dma_addr_t *rx_dma,
u32 *plen)
{
- struct atmel_spi *as = spi_master_get_devdata(master);
- u32 len = *plen;
-
*rx_dma = xfer->rx_dma + xfer->len - *plen;
*tx_dma = xfer->tx_dma + xfer->len - *plen;
- if (len > master->max_dma_len)
- len = master->max_dma_len;
-
- *plen = len;
+ if (*plen > master->max_dma_len)
+ *plen = master->max_dma_len;
}
static int atmel_spi_set_xfer_speed(struct atmel_spi *as,
@@ -1253,7 +1242,7 @@ static int atmel_spi_one_transfer(struct spi_master *master,
* better fault reporting.
*/
if ((!msg->is_dma_mapped)
- && (atmel_spi_use_dma(as, xfer) || as->use_pdc)) {
+ && as->use_pdc) {
if (atmel_spi_dma_map_xfer(as, xfer) < 0)
return -ENOMEM;
}
@@ -1330,7 +1319,7 @@ static int atmel_spi_one_transfer(struct spi_master *master,
}
if (!msg->is_dma_mapped
- && (atmel_spi_use_dma(as, xfer) || as->use_pdc))
+ && as->use_pdc)
atmel_spi_dma_unmap_xfer(master, xfer);
return 0;
@@ -1341,7 +1330,7 @@ static int atmel_spi_one_transfer(struct spi_master *master,
}
if (!msg->is_dma_mapped
- && (atmel_spi_use_dma(as, xfer) || as->use_pdc))
+ && as->use_pdc)
atmel_spi_dma_unmap_xfer(master, xfer);
if (xfer->delay_usecs)
@@ -1519,6 +1508,7 @@ static int atmel_spi_probe(struct platform_device *pdev)
master->cleanup = atmel_spi_cleanup;
master->auto_runtime_pm = true;
master->max_dma_len = SPI_MAX_DMA_XFER;
+ master->can_dma = atmel_spi_can_dma;
platform_set_drvdata(pdev, master);
as = spi_master_get_devdata(master);
@@ -1555,10 +1545,13 @@ static int atmel_spi_probe(struct platform_device *pdev)
as->use_pdc = false;
if (as->caps.has_dma_support) {
ret = atmel_spi_configure_dma(as);
- if (ret == 0)
+ if (ret == 0) {
+ master->dma_tx = as->dma.chan_tx;
+ master->dma_rx = as->dma.chan_rx;
as->use_dma = true;
- else if (ret == -EPROBE_DEFER)
+ } else if (ret == -EPROBE_DEFER) {
return ret;
+ }
} else {
as->use_pdc = true;
}
--
2.9.0
^ permalink raw reply related
* [PATCH 4/5] spi: atmel: trivial: remove unused fields in DMA structure
From: Nicolas Ferre @ 2016-11-24 11:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1479985886.git.nicolas.ferre@atmel.com>
The atmel_spi_dma structure was cluttered with unused fields relative
to older DMA channel selection API. Remove them.
Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
drivers/spi/spi-atmel.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index f62bc2d27c9e..f3a225585575 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -268,10 +268,6 @@
struct atmel_spi_dma {
struct dma_chan *chan_rx;
struct dma_chan *chan_tx;
- struct dma_async_tx_descriptor *data_desc_rx;
- struct dma_async_tx_descriptor *data_desc_tx;
-
- struct at_dma_slave dma_slave;
};
struct atmel_spi_caps {
--
2.9.0
^ permalink raw reply related
* [PATCH 5/5] spi: atmel: remove the use of private channel fields
From: Nicolas Ferre @ 2016-11-24 11:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1479985886.git.nicolas.ferre@atmel.com>
For DMA transfers, we now use the core DMA framework which provides
channel fields in the spi_master structure. Remove the private channels
from atmel_spi stucture which were located in a sub-structure. This
last one (atmel_spi_dma) which is now empty is also removed.
Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
drivers/spi/spi-atmel.c | 86 ++++++++++++++++++++++++-------------------------
1 file changed, 43 insertions(+), 43 deletions(-)
diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index f3a225585575..0e7712bac3b6 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -265,11 +265,6 @@
#define AUTOSUSPEND_TIMEOUT 2000
-struct atmel_spi_dma {
- struct dma_chan *chan_rx;
- struct dma_chan *chan_tx;
-};
-
struct atmel_spi_caps {
bool is_spi2;
bool has_wdrbt;
@@ -303,8 +298,6 @@ struct atmel_spi {
bool use_dma;
bool use_pdc;
bool use_cs_gpios;
- /* dmaengine data */
- struct atmel_spi_dma dma;
bool keep_cs;
bool cs_active;
@@ -461,6 +454,7 @@ static int atmel_spi_dma_slave_config(struct atmel_spi *as,
struct dma_slave_config *slave_config,
u8 bits_per_word)
{
+ struct spi_master *master = platform_get_drvdata(as->pdev);
int err = 0;
if (bits_per_word > 8) {
@@ -492,7 +486,7 @@ static int atmel_spi_dma_slave_config(struct atmel_spi *as,
* path works the same whether FIFOs are available (and enabled) or not.
*/
slave_config->direction = DMA_MEM_TO_DEV;
- if (dmaengine_slave_config(as->dma.chan_tx, slave_config)) {
+ if (dmaengine_slave_config(master->dma_tx, slave_config)) {
dev_err(&as->pdev->dev,
"failed to configure tx dma channel\n");
err = -EINVAL;
@@ -507,7 +501,7 @@ static int atmel_spi_dma_slave_config(struct atmel_spi *as,
* enabled) or not.
*/
slave_config->direction = DMA_DEV_TO_MEM;
- if (dmaengine_slave_config(as->dma.chan_rx, slave_config)) {
+ if (dmaengine_slave_config(master->dma_rx, slave_config)) {
dev_err(&as->pdev->dev,
"failed to configure rx dma channel\n");
err = -EINVAL;
@@ -516,7 +510,8 @@ static int atmel_spi_dma_slave_config(struct atmel_spi *as,
return err;
}
-static int atmel_spi_configure_dma(struct atmel_spi *as)
+static int atmel_spi_configure_dma(struct spi_master *master,
+ struct atmel_spi *as)
{
struct dma_slave_config slave_config;
struct device *dev = &as->pdev->dev;
@@ -526,26 +521,26 @@ static int atmel_spi_configure_dma(struct atmel_spi *as)
dma_cap_zero(mask);
dma_cap_set(DMA_SLAVE, mask);
- as->dma.chan_tx = dma_request_slave_channel_reason(dev, "tx");
- if (IS_ERR(as->dma.chan_tx)) {
- err = PTR_ERR(as->dma.chan_tx);
+ master->dma_tx = dma_request_slave_channel_reason(dev, "tx");
+ if (IS_ERR(master->dma_tx)) {
+ err = PTR_ERR(master->dma_tx);
if (err == -EPROBE_DEFER) {
dev_warn(dev, "no DMA channel available at the moment\n");
- return err;
+ goto error_clear;
}
dev_err(dev,
"DMA TX channel not available, SPI unable to use DMA\n");
err = -EBUSY;
- goto error;
+ goto error_clear;
}
/*
* No reason to check EPROBE_DEFER here since we have already requested
* tx channel. If it fails here, it's for another reason.
*/
- as->dma.chan_rx = dma_request_slave_channel(dev, "rx");
+ master->dma_rx = dma_request_slave_channel(dev, "rx");
- if (!as->dma.chan_rx) {
+ if (!master->dma_rx) {
dev_err(dev,
"DMA RX channel not available, SPI unable to use DMA\n");
err = -EBUSY;
@@ -558,31 +553,38 @@ static int atmel_spi_configure_dma(struct atmel_spi *as)
dev_info(&as->pdev->dev,
"Using %s (tx) and %s (rx) for DMA transfers\n",
- dma_chan_name(as->dma.chan_tx),
- dma_chan_name(as->dma.chan_rx));
+ dma_chan_name(master->dma_tx),
+ dma_chan_name(master->dma_rx));
+
return 0;
error:
- if (as->dma.chan_rx)
- dma_release_channel(as->dma.chan_rx);
- if (!IS_ERR(as->dma.chan_tx))
- dma_release_channel(as->dma.chan_tx);
+ if (master->dma_rx)
+ dma_release_channel(master->dma_rx);
+ if (!IS_ERR(master->dma_tx))
+ dma_release_channel(master->dma_tx);
+error_clear:
+ master->dma_tx = master->dma_rx = NULL;
return err;
}
-static void atmel_spi_stop_dma(struct atmel_spi *as)
+static void atmel_spi_stop_dma(struct spi_master *master)
{
- if (as->dma.chan_rx)
- dmaengine_terminate_all(as->dma.chan_rx);
- if (as->dma.chan_tx)
- dmaengine_terminate_all(as->dma.chan_tx);
+ if (master->dma_rx)
+ dmaengine_terminate_all(master->dma_rx);
+ if (master->dma_tx)
+ dmaengine_terminate_all(master->dma_tx);
}
-static void atmel_spi_release_dma(struct atmel_spi *as)
+static void atmel_spi_release_dma(struct spi_master *master)
{
- if (as->dma.chan_rx)
- dma_release_channel(as->dma.chan_rx);
- if (as->dma.chan_tx)
- dma_release_channel(as->dma.chan_tx);
+ if (master->dma_rx) {
+ dma_release_channel(master->dma_rx);
+ master->dma_rx = NULL;
+ }
+ if (master->dma_tx) {
+ dma_release_channel(master->dma_tx);
+ master->dma_tx = NULL;
+ }
}
/* This function is called by the DMA driver from tasklet context */
@@ -718,8 +720,8 @@ static int atmel_spi_next_xfer_dma_submit(struct spi_master *master,
u32 *plen)
{
struct atmel_spi *as = spi_master_get_devdata(master);
- struct dma_chan *rxchan = as->dma.chan_rx;
- struct dma_chan *txchan = as->dma.chan_tx;
+ struct dma_chan *rxchan = master->dma_rx;
+ struct dma_chan *txchan = master->dma_tx;
struct dma_async_tx_descriptor *rxdesc;
struct dma_async_tx_descriptor *txdesc;
struct dma_slave_config slave_config;
@@ -783,7 +785,7 @@ static int atmel_spi_next_xfer_dma_submit(struct spi_master *master,
err_dma:
spi_writel(as, IDR, SPI_BIT(OVRES));
- atmel_spi_stop_dma(as);
+ atmel_spi_stop_dma(master);
err_exit:
atmel_spi_lock(as);
return -ENOMEM;
@@ -1311,7 +1313,7 @@ static int atmel_spi_one_transfer(struct spi_master *master,
spi_readl(as, SR);
} else if (atmel_spi_use_dma(as, xfer)) {
- atmel_spi_stop_dma(as);
+ atmel_spi_stop_dma(master);
}
if (!msg->is_dma_mapped
@@ -1540,10 +1542,8 @@ static int atmel_spi_probe(struct platform_device *pdev)
as->use_dma = false;
as->use_pdc = false;
if (as->caps.has_dma_support) {
- ret = atmel_spi_configure_dma(as);
+ ret = atmel_spi_configure_dma(master, as);
if (ret == 0) {
- master->dma_tx = as->dma.chan_tx;
- master->dma_rx = as->dma.chan_rx;
as->use_dma = true;
} else if (ret == -EPROBE_DEFER) {
return ret;
@@ -1612,7 +1612,7 @@ static int atmel_spi_probe(struct platform_device *pdev)
pm_runtime_set_suspended(&pdev->dev);
if (as->use_dma)
- atmel_spi_release_dma(as);
+ atmel_spi_release_dma(master);
spi_writel(as, CR, SPI_BIT(SWRST));
spi_writel(as, CR, SPI_BIT(SWRST)); /* AT91SAM9263 Rev B workaround */
@@ -1634,8 +1634,8 @@ static int atmel_spi_remove(struct platform_device *pdev)
/* reset the hardware and block queue progress */
spin_lock_irq(&as->lock);
if (as->use_dma) {
- atmel_spi_stop_dma(as);
- atmel_spi_release_dma(as);
+ atmel_spi_stop_dma(master);
+ atmel_spi_release_dma(master);
}
spi_writel(as, CR, SPI_BIT(SWRST));
--
2.9.0
^ permalink raw reply related
* [PATCH] ARM: dts: sun6i: hummingbird: Enable USB OTG
From: Chen-Yu Tsai @ 2016-11-24 11:29 UTC (permalink / raw)
To: linux-arm-kernel
The A31 Hummingbird has a mini USB OTG port, and uses GPIO pins from the
SoC for ID pin and VBUS detection and VBUS control. The PMIC can also do
VBUS detection and control.
Here we prefer to use the PMIC's DRIVEVBUS function to control VBUS for
USB OTG, as that is the hardware default.
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
arch/arm/boot/dts/sun6i-a31-hummingbird.dts | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/arch/arm/boot/dts/sun6i-a31-hummingbird.dts b/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
index 83643bbd51dc..f094eeb6c499 100644
--- a/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
+++ b/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
@@ -248,6 +248,7 @@
reg = <0x68>;
interrupt-parent = <&nmi_intc>;
interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+ x-powers,drive-vbus-en;
};
};
@@ -306,6 +307,11 @@
regulator-name = "vcc-dram";
};
+®_drivevbus {
+ regulator-name = "usb0-vbus";
+ status = "okay";
+};
+
®_usb1_vbus {
gpio = <&pio 7 24 GPIO_ACTIVE_HIGH>; /* PH24 */
status = "okay";
@@ -330,12 +336,25 @@
status = "okay";
};
+&usb_otg {
+ dr_mode = "otg";
+ status = "okay";
+};
+
+&usb_power_supply {
+ status = "okay";
+};
+
&usb1_vbus_pin_a {
/* different pin from sunxi-common-regulators */
pins = "PH24";
};
&usbphy {
+ usb0_id_det-gpio = <&pio 0 15 GPIO_ACTIVE_HIGH>; /* PA15 */
+ usb0_vbus_det-gpio = <&pio 0 16 GPIO_ACTIVE_HIGH>; /* PA16 */
+ usb0_vbus_power-supply = <&usb_power_supply>;
+ usb0_vbus-supply = <®_drivevbus>;
usb1_vbus-supply = <®_usb1_vbus>;
status = "okay";
};
--
2.10.2
^ permalink raw reply related
* [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell Xenon SDHC
From: Ulf Hansson @ 2016-11-24 11:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <a05ffd140f4edc02fc3128db8445b2264cf38723.1477911954.git-series.gregory.clement@free-electrons.com>
On 31 October 2016 at 12:09, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:
> From: Ziji Hu <huziji@marvell.com>
>
> Marvell Xenon eMMC/SD/SDIO Host Controller contains PHY.
> Three types of PHYs are supported.
>
> Add support to multiple types of PHYs init and configuration.
> Add register definitions of PHYs.
>
> Signed-off-by: Hu Ziji <huziji@marvell.com>
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
> MAINTAINERS | 2 +-
> drivers/mmc/host/Makefile | 2 +-
> drivers/mmc/host/sdhci-xenon-phy.c | 1181 +++++++++++++++++++++++++++++-
> drivers/mmc/host/sdhci-xenon-phy.h | 157 ++++-
> drivers/mmc/host/sdhci-xenon.c | 4 +-
> drivers/mmc/host/sdhci-xenon.h | 17 +-
> 6 files changed, 1361 insertions(+), 2 deletions(-)
> create mode 100644 drivers/mmc/host/sdhci-xenon-phy.c
> create mode 100644 drivers/mmc/host/sdhci-xenon-phy.h
Can you please consider to split this up somehow!? It would make it
easier to review...
Anyway, allow me to provide some initial feedback, particularly around
those things that Adrian and you requested for my input.
[...]
>
> +
> +static int __xenon_emmc_delay_adj_test(struct mmc_card *card)
> +{
> + int err;
> + u8 *ext_csd = NULL;
> +
> + err = mmc_get_ext_csd(card, &ext_csd);
> + kfree(ext_csd);
Why do you read the ext csd here?
> +
> + return err;
> +}
> +
> +static int __xenon_sdio_delay_adj_test(struct mmc_card *card)
> +{
> + struct mmc_command cmd = {0};
> + int err;
> +
> + cmd.opcode = SD_IO_RW_DIRECT;
> + cmd.flags = MMC_RSP_R5 | MMC_CMD_AC;
> +
> + err = mmc_wait_for_cmd(card->host, &cmd, 0);
> + if (err)
> + return err;
> +
> + if (cmd.resp[0] & R5_ERROR)
> + return -EIO;
> + if (cmd.resp[0] & R5_FUNCTION_NUMBER)
> + return -EINVAL;
> + if (cmd.resp[0] & R5_OUT_OF_RANGE)
> + return -ERANGE;
> + return 0;
No thanks! MMC/SD/SDIO protocol code belongs in the core.
> +}
> +
> +static int __xenon_sd_delay_adj_test(struct mmc_card *card)
> +{
> + struct mmc_command cmd = {0};
> + int err;
> +
> + cmd.opcode = MMC_SEND_STATUS;
> + cmd.arg = card->rca << 16;
> + cmd.flags = MMC_RSP_R1 | MMC_CMD_AC;
> +
> + err = mmc_wait_for_cmd(card->host, &cmd, 0);
> + return err;
No thanks! MMC/SD/SDIO protocol code belongs in the core.
> +}
> +
[...]
> +int xenon_phy_adj(struct sdhci_host *host, struct mmc_ios *ios)
> +{
> + struct mmc_host *mmc = host->mmc;
> + struct mmc_card *card;
> + int ret = 0;
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> +
> + if (!host->clock) {
> + priv->clock = 0;
> + return 0;
> + }
> +
> + /*
> + * The timing, frequency or bus width is changed,
> + * better to set eMMC PHY based on current setting
> + * and adjust Xenon SDHC delay.
> + */
> + if ((host->clock == priv->clock) &&
> + (ios->bus_width == priv->bus_width) &&
> + (ios->timing == priv->timing))
> + return 0;
> +
> + xenon_phy_set(host, ios->timing);
> +
> + /* Update the record */
> + priv->bus_width = ios->bus_width;
> + /* Temp stage from HS200 to HS400 */
> + if (((priv->timing == MMC_TIMING_MMC_HS200) &&
> + (ios->timing == MMC_TIMING_MMC_HS)) ||
> + ((ios->timing == MMC_TIMING_MMC_HS) &&
> + (priv->clock > host->clock))) {
> + priv->timing = ios->timing;
> + priv->clock = host->clock;
> + return 0;
> + }
> + /*
> + * Skip temp stages from HS400 t0 HS200:
> + * from 200MHz to 52MHz in HS400
> + * from HS400 to HS DDR in 52MHz
> + * from HS DDR to HS in 52MHz
> + * from HS to HS200 in 52MHz
> + */
> + if (((priv->timing == MMC_TIMING_MMC_HS400) &&
> + ((host->clock == MMC_HIGH_52_MAX_DTR) ||
> + (ios->timing == MMC_TIMING_MMC_DDR52))) ||
> + ((priv->timing == MMC_TIMING_MMC_DDR52) &&
> + (ios->timing == MMC_TIMING_MMC_HS)) ||
> + ((ios->timing == MMC_TIMING_MMC_HS200) &&
> + (ios->clock == MMC_HIGH_52_MAX_DTR))) {
> + priv->timing = ios->timing;
> + priv->clock = host->clock;
> + return 0;
> + }
> + priv->timing = ios->timing;
> + priv->clock = host->clock;
> +
> + /* Legacy mode is a special case */
> + if (ios->timing == MMC_TIMING_LEGACY)
> + return 0;
> +
> + if (mmc->card)
> + card = mmc->card;
> + else
> + /*
> + * Only valid during initialization
> + * before mmc->card is set
> + */
> + card = priv->card_candidate;
> + if (unlikely(!card)) {
> + dev_warn(mmc_dev(mmc), "card is not present\n");
> + return -EINVAL;
> + }
That your host need to hold a copy of the card pointer, tells me that
something is not really correct.
I might be wrong, if this turns out to be a special case, but I doubt
it. Although, if it *is* a special such case, we shall most likely try
to extend the the mmc core layer instead of adding all these hacks in
your host driver.
[...]
Another suggestion of a general improvement; could you perhaps try to
add some brief information about what goes on in function headers.
Perhaps that could help to more easily understand things.
Kind regards
Uffe
^ 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