* Re: [PATCH 6/10] mmc: sdhci-xenon: Add Marvell Xenon SDHC core functionality
From: Ulf Hansson @ 2016-11-24 10:43 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: Adrian Hunter, linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Rob Herring,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Thomas Petazzoni,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Ziji Hu, Jack(SH) Zhu, Jimmy Xu, Jisheng Zhang, Nadav Haklai,
Ryan Gao, Doug Jones, Shiwu Zhang, Victor Gu, Wei(SOCP)
In-Reply-To: <0390e7a05b6163deabb545f93729ea615eeaaee2.1477911954.git-series.gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
On 31 October 2016 at 12:09, Gregory CLEMENT
<gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> From: Ziji Hu <huziji-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
>
> 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-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
> 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-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
> L: linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.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-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
> + * 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-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
> + * 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-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>");
> +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-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
> + * 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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [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: Thomas Petazzoni, Marcin Wojtas
Cc: Gregory CLEMENT, Arnd Bergmann,
linux-arm-kernel@lists.infradead.org, Jimmy Xu, Andrew Lunn,
Ulf Hansson, Romain Perier, Liuliu Zhao, Peng Zhu,
linux-kernel@vger.kernel.org, Nadav Haklai, Victor Gu, Doug Jones,
Jisheng Zhang, Yehuda Yitschak, Xueping Liu, Hilbert Zhang,
Shiwu Zhang, Yu Cao, Sebastian
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 3/3] gpio: Support gpio nexus dt bindings
From: Stephen Boyd @ 2016-11-24 10:25 UTC (permalink / raw)
To: Rob Herring, Frank Rowand
Cc: linux-arm-kernel, linux-kernel, devicetree, linux-gpio,
Pantelis Antoniou, Linus Walleij, Mark Brown
In-Reply-To: <20161124102529.20212-1-stephen.boyd@linaro.org>
Platforms like 96boards have a standardized connector/expansion
slot that exposes signals like GPIOs to expansion boards in an
SoC agnostic way. We'd like the DT overlays for the expansion
boards to be written once without knowledge of the SoC on the
other side of the connector. This avoids the unscalable
combinatorial explosion of a different DT overlay for each
expansion board and SoC pair.
Now that we have nexus support in the OF core let's change the
function call here that parses the phandle lists of gpios to use
the nexus variant. This allows us to remap phandles and their
arguments through any number of nexus nodes and end up with the
actual gpio provider being used.
Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Mark Brown <broonie@kernel.org>
Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
---
TODO: Document gpio-map and gpio-map-mask in GPIO devicetree binding
drivers/gpio/gpiolib-of.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index ecad3f0e3b77..3117397c4c41 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -71,8 +71,9 @@ struct gpio_desc *of_get_named_gpiod_flags(struct device_node *np,
struct gpio_desc *desc;
int ret;
- ret = of_parse_phandle_with_args(np, propname, "#gpio-cells", index,
- &gpiospec);
+ ret = of_parse_phandle_with_args_map(np, propname, "#gpio-cells",
+ "gpio-map", "gpio-map-mask",
+ index, &gpiospec);
if (ret) {
pr_debug("%s: can't parse '%s' property of node '%s[%d]'\n",
__func__, propname, np->full_name, index);
--
2.10.0.297.gf6727b0
^ permalink raw reply related
* [PATCH 2/3] of: unittest: Add phandle remapping test
From: Stephen Boyd @ 2016-11-24 10:25 UTC (permalink / raw)
To: Rob Herring, Frank Rowand
Cc: linux-arm-kernel, linux-kernel, devicetree, linux-gpio,
Pantelis Antoniou, Linus Walleij, Mark Brown
In-Reply-To: <20161124102529.20212-1-stephen.boyd@linaro.org>
Test the functionality of of_parse_phandle_with_args_map().
Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Mark Brown <broonie@kernel.org>
Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
---
drivers/of/unittest-data/testcases.dts | 11 +++
drivers/of/unittest-data/tests-phandle.dtsi | 24 ++++++
drivers/of/unittest.c | 124 ++++++++++++++++++++++++++++
3 files changed, 159 insertions(+)
diff --git a/drivers/of/unittest-data/testcases.dts b/drivers/of/unittest-data/testcases.dts
index 12f7c3d649c8..f4c653418515 100644
--- a/drivers/of/unittest-data/testcases.dts
+++ b/drivers/of/unittest-data/testcases.dts
@@ -26,12 +26,23 @@
/ { __local_fixups__ {
testcase-data {
phandle-tests {
+ provider4 {
+ phandle-map = <0x00000008 0x00000018
+ 0x00000024 0x0000003c
+ 0x00000050 0x0000005c>;
+ };
consumer-a {
phandle-list = <0x00000000 0x00000008
0x00000018 0x00000028
0x00000034 0x00000038>;
phandle-list-bad-args = <0x00000000 0x0000000c>;
};
+ consumer-b {
+ phandle-list = <0x00000000 0x00000008
+ 0x00000018 0x00000024
+ 0x00000030 0x00000034>;
+ phandle-list-bad-args = <0x00000000 0x0000000c>;
+ };
};
interrupts {
intmap0 {
diff --git a/drivers/of/unittest-data/tests-phandle.dtsi b/drivers/of/unittest-data/tests-phandle.dtsi
index 5b1527e8a7fb..80428bfafa10 100644
--- a/drivers/of/unittest-data/tests-phandle.dtsi
+++ b/drivers/of/unittest-data/tests-phandle.dtsi
@@ -25,6 +25,17 @@
#phandle-cells = <3>;
};
+ provider4: provider4 {
+ #phandle-cells = <2>;
+ phandle-map = <0 1 &provider1 3>,
+ <4 0 &provider0>,
+ <16 5 &provider3 3 5 0>,
+ <200 8 &provider2 23 54>,
+ <19 0 &provider0>,
+ <2 3 &provider3 2 5 3>;
+ phandle-map-mask = <0xff 0xf>;
+ };
+
consumer-a {
phandle-list = <&provider1 1>,
<&provider2 2 0>,
@@ -43,6 +54,19 @@
unterminated-string = [40 41 42 43];
unterminated-string-list = "first", "second", [40 41 42 43];
};
+
+ consumer-b {
+ phandle-list = <&provider1 1>,
+ <&provider4 2 3>,
+ <0>,
+ <&provider4 4 256>,
+ <&provider4 0 97>,
+ <&provider0>,
+ <&provider4 19 32>;
+ phandle-list-bad-phandle = <12345678 0 0>;
+ phandle-list-bad-args = <&provider2 1 0>,
+ <&provider4 0>;
+ };
};
};
};
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 53c83d66eb7e..52a70da32f04 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -386,6 +386,129 @@ static void __init of_unittest_parse_phandle_with_args(void)
unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc);
}
+static void __init of_unittest_parse_phandle_with_args_map(void)
+{
+ struct device_node *np, *p0, *p1, *p2, *p3;
+ struct of_phandle_args args;
+ int i, rc;
+
+ np = of_find_node_by_path("/testcase-data/phandle-tests/consumer-b");
+ if (!np) {
+ pr_err("missing testcase data\n");
+ return;
+ }
+
+ p0 = of_find_node_by_path("/testcase-data/phandle-tests/provider0");
+ if (!p0) {
+ pr_err("missing testcase data\n");
+ return;
+ }
+
+ p1 = of_find_node_by_path("/testcase-data/phandle-tests/provider1");
+ if (!p1) {
+ pr_err("missing testcase data\n");
+ return;
+ }
+
+ p2 = of_find_node_by_path("/testcase-data/phandle-tests/provider2");
+ if (!p2) {
+ pr_err("missing testcase data\n");
+ return;
+ }
+
+ p3 = of_find_node_by_path("/testcase-data/phandle-tests/provider3");
+ if (!p3) {
+ pr_err("missing testcase data\n");
+ return;
+ }
+
+ rc = of_count_phandle_with_args(np, "phandle-list", "#phandle-cells");
+ unittest(rc == 7, "of_count_phandle_with_args() returned %i, expected 7\n", rc);
+
+ for (i = 0; i < 8; i++) {
+ bool passed = true;
+
+ rc = of_parse_phandle_with_args_map(np, "phandle-list",
+ "#phandle-cells", "phandle-map",
+ "phandle-map-mask", i, &args);
+
+ /* Test the values from tests-phandle.dtsi */
+ switch (i) {
+ case 0:
+ passed &= !rc;
+ passed &= (args.np == p1);
+ passed &= (args.args_count == 1);
+ passed &= (args.args[0] == 1);
+ break;
+ case 1:
+ passed &= !rc;
+ passed &= (args.np == p3);
+ passed &= (args.args_count == 3);
+ passed &= (args.args[0] == 2);
+ passed &= (args.args[1] == 5);
+ passed &= (args.args[2] == 3);
+ break;
+ case 2:
+ passed &= (rc == -ENOENT);
+ break;
+ case 3:
+ passed &= !rc;
+ passed &= (args.np == p0);
+ passed &= (args.args_count == 0);
+ break;
+ case 4:
+ passed &= !rc;
+ passed &= (args.np == p1);
+ passed &= (args.args_count == 1);
+ passed &= (args.args[0] == 3);
+ break;
+ case 5:
+ passed &= !rc;
+ passed &= (args.np == p0);
+ passed &= (args.args_count == 0);
+ break;
+ case 6:
+ passed &= !rc;
+ passed &= (args.np == p0);
+ passed &= (args.args_count == 0);
+ break;
+ case 7:
+ passed &= (rc == -ENOENT);
+ break;
+ default:
+ passed = false;
+ }
+
+ unittest(passed, "index %i - data error on node %s rc=%i\n",
+ i, args.np->full_name, rc);
+ }
+
+ /* Check for missing list property */
+ rc = of_parse_phandle_with_args_map(np, "phandle-list-missing",
+ "#phandle-cells", "phandle-map",
+ "phandle-map-mask", 0, &args);
+ unittest(rc == -ENOENT, "expected:%i got:%i\n", -ENOENT, rc);
+
+ /* Check for missing cells property */
+ rc = of_parse_phandle_with_args_map(np, "phandle-list",
+ "#phandle-cells-missing",
+ "phandle-map", "phandle-map-mask",
+ 0, &args);
+ unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc);
+
+ /* Check for bad phandle in list */
+ rc = of_parse_phandle_with_args_map(np, "phandle-list-bad-phandle",
+ "#phandle-cells", "phandle-map",
+ "phandle-map-mask", 0, &args);
+ unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc);
+
+ /* Check for incorrectly formed argument list */
+ rc = of_parse_phandle_with_args_map(np, "phandle-list-bad-args",
+ "#phandle-cells", "phandle-map",
+ "phandle-map-mask", 1, &args);
+ unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc);
+}
+
static void __init of_unittest_property_string(void)
{
const char *strings[4];
@@ -1951,6 +2074,7 @@ static int __init of_unittest(void)
of_unittest_find_node_by_name();
of_unittest_dynamic();
of_unittest_parse_phandle_with_args();
+ of_unittest_parse_phandle_with_args_map();
of_unittest_property_string();
of_unittest_property_copy();
of_unittest_changeset();
--
2.10.0.297.gf6727b0
^ permalink raw reply related
* [PATCH 1/3] of: Support parsing phandle argument lists through a nexus node
From: Stephen Boyd @ 2016-11-24 10:25 UTC (permalink / raw)
To: Rob Herring, Frank Rowand
Cc: devicetree, Linus Walleij, Pantelis Antoniou, linux-kernel,
linux-gpio, Mark Brown, linux-arm-kernel
In-Reply-To: <20161124102529.20212-1-stephen.boyd@linaro.org>
Platforms like 96boards have a standardized connector/expansion
slot that exposes signals like GPIOs to expansion boards in an
SoC agnostic way. We'd like the DT overlays for the expansion
boards to be written once without knowledge of the SoC on the
other side of the connector. This avoids the unscalable
combinatorial explosion of a different DT overlay for each
expansion board and SoC pair.
We need a way to describe the GPIOs routed through the connector
in an SoC agnostic way. Let's introduce nexus property parsing
into the OF core to do this. This is largely based on the
interrupt nexus support we already have. This allows us to remap
a phandle list in a consumer node (e.g. reset-gpios) through a
connector in a generic way (e.g. via gpio-map). Do this in a
generic routine so that we can remap any sort of variable length
phandle list.
Taking GPIOs as an example, the connector would be a GPIO nexus,
supporting the remapping of a GPIO specifier space to multiple
GPIO providers on the SoC. DT would look as shown below, where
'soc_gpio1' and 'soc_gpio2' are inside the SoC, 'connector' is an
expansion port where boards can be plugged in, and
'expansion_device' is a device on the expansion board.
soc {
soc_gpio1: gpio-controller1 {
#gpio-cells = <2>;
};
soc_gpio2: gpio-controller2 {
#gpio-cells = <2>;
};
};
connector: connector {
#gpio-cells = <2>;
gpio-map = <0 GPIO_ACTIVE_LOW &soc_gpio1 1 GPIO_ACTIVE_LOW>,
<1 GPIO_ACTIVE_LOW &soc_gpio2 4 GPIO_ACTIVE_LOW>,
<2 GPIO_ACTIVE_LOW &soc_gpio1 3 GPIO_ACTIVE_LOW>,
<3 GPIO_ACTIVE_LOW &soc_gpio2 2 GPIO_ACTIVE_LOW>;
gpio-map-mask = <0xf 0x1>;
};
expansion_device {
reset-gpios = <&connector 2 GPIO_ACTIVE_LOW>;
};
The GPIO core would use of_parse_phandle_with_args_map() instead
of of_parse_phandle_with_args() and arrive at the same type of
result, a phandle and argument list. The difference is that the
phandle and arguments will be remapped through the nexus node to
the underlying SoC GPIO controller node. In the example above,
we would remap 'reset-gpios' from <&connector 2 GPIO_ACTIVE_LOW>
to <&soc_gpio1 3 GPIO_ACTIVE_LOW>.
Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Mark Brown <broonie@kernel.org>
Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
---
drivers/of/base.c | 146 +++++++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/of.h | 14 +++++
2 files changed, 160 insertions(+)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index d687e6de24a0..693b73f33675 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1772,6 +1772,152 @@ int of_parse_phandle_with_args(const struct device_node *np, const char *list_na
EXPORT_SYMBOL(of_parse_phandle_with_args);
/**
+ * of_parse_phandle_with_args_map() - Find a node pointed by phandle in a list and remap it
+ * @np: pointer to a device tree node containing a list
+ * @list_name: property name that contains a list
+ * @cells_name: property name that specifies phandles' arguments count
+ * @index: index of a phandle to parse out
+ * @out_args: optional pointer to output arguments structure (will be filled)
+ *
+ * This function is useful to parse lists of phandles and their arguments.
+ * Returns 0 on success and fills out_args, on error returns appropriate
+ * errno value.
+ *
+ * Caller is responsible to call of_node_put() on the returned out_args->np
+ * pointer.
+ *
+ * Example:
+ *
+ * phandle1: node1 {
+ * #list-cells = <2>;
+ * }
+ *
+ * phandle2: node2 {
+ * #list-cells = <1>;
+ * }
+ *
+ * phandle3: node3 {
+ * #list-cells = <1>;
+ * list-map = <0 &phandle2 3>,
+ * <1 &phandle2 2>,
+ * <2 &phandle1 5 1>;
+ * list-map-mask = <0x3>;
+ * };
+ *
+ * node4 {
+ * list = <&phandle1 1 2 &phandle3 0>;
+ * }
+ *
+ * To get a device_node of the `node2' node you may call this:
+ * of_parse_phandle_with_args(node4, "list", "#list-cells", "list-map",
+ * "list-map-mask", 1, &args);
+ */
+int of_parse_phandle_with_args_map(const struct device_node *np,
+ const char *list_name,
+ const char *cells_name,
+ const char *map_name,
+ const char *mask_name,
+ int index, struct of_phandle_args *out_args)
+{
+ struct device_node *cur, *new = NULL;
+ const __be32 *map, *mask, *tmp;
+ const __be32 dummy_mask[] = { [0 ... MAX_PHANDLE_ARGS] = ~0 };
+ __be32 initial_match_array[MAX_PHANDLE_ARGS];
+ const __be32 *match_array = initial_match_array;
+ int i, ret, map_len, match;
+ u32 list_size, new_size;
+
+ if (index < 0)
+ return -EINVAL;
+
+ ret = __of_parse_phandle_with_args(np, list_name, cells_name, 0, index,
+ out_args);
+ if (ret)
+ return ret;
+
+ /* Get the #<list>-cells property */
+ cur = out_args->np;
+ ret = of_property_read_u32(cur, cells_name, &list_size);
+ if (ret < 0)
+ goto fail;
+
+ /* Precalculate the match array - this simplifies match loop */
+ for (i = 0; i < list_size; i++)
+ initial_match_array[i] = cpu_to_be32(out_args->args[i]);
+
+ while (cur) {
+ /* Get the <list>-map property */
+ map = of_get_property(cur, map_name, &map_len);
+ if (!map)
+ return 0;
+ map_len /= sizeof(u32);
+
+ /* Get the <list>-map-mask property (optional) */
+ mask = of_get_property(cur, mask_name, NULL);
+ if (!mask)
+ mask = dummy_mask;
+
+ /* Iterate through <list>-map property */
+ match = 0;
+ while (map_len > (list_size + 1) && !match) {
+ /* Compare specifiers */
+ match = 1;
+ for (i = 0; i < list_size; i++, map_len--)
+ match &= !((match_array[i] ^ *map++) & mask[i]);
+
+ of_node_put(new);
+ new = of_find_node_by_phandle(be32_to_cpup(map));
+ map++;
+ map_len--;
+
+ /* Check if not found */
+ if (!new)
+ goto fail;
+
+ if (!of_device_is_available(new))
+ match = 0;
+
+ tmp = of_get_property(new, cells_name, NULL);
+ if (!tmp)
+ goto fail;
+
+ new_size = be32_to_cpu(*tmp);
+
+ /* Check for malformed properties */
+ if (WARN_ON(new_size > MAX_PHANDLE_ARGS))
+ goto fail;
+ if (map_len < new_size)
+ goto fail;
+
+ /* Move forward by new node's #<list>-cells amount */
+ map += new_size;
+ map_len -= new_size;
+ }
+ if (!match)
+ goto fail;
+
+ /*
+ * Successfully parsed a <list>-map translation; copy new
+ * specifier into the out_args structure.
+ */
+ match_array = map - new_size;
+ for (i = 0; i < new_size; i++)
+ out_args->args[i] = be32_to_cpup(map - new_size + i);
+ out_args->args_count = list_size = new_size;
+ /* Iterate again with new provider */
+ out_args->np = new;
+ of_node_put(cur);
+ cur = new;
+ }
+fail:
+ of_node_put(cur);
+ of_node_put(new);
+
+ return -EINVAL;
+}
+EXPORT_SYMBOL(of_parse_phandle_with_args_map);
+
+/**
* of_parse_phandle_with_fixed_args() - Find a node pointed by phandle in a list
* @np: pointer to a device tree node containing a list
* @list_name: property name that contains a list
diff --git a/include/linux/of.h b/include/linux/of.h
index d3a9c2e69001..65ff306403a2 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -344,6 +344,9 @@ extern struct device_node *of_parse_phandle(const struct device_node *np,
extern int of_parse_phandle_with_args(const struct device_node *np,
const char *list_name, const char *cells_name, int index,
struct of_phandle_args *out_args);
+extern int of_parse_phandle_with_args_map(const struct device_node *np,
+ const char *list_name, const char *cells_name, const char *map_name,
+ const char *mask_name, int index, struct of_phandle_args *out_args);
extern int of_parse_phandle_with_fixed_args(const struct device_node *np,
const char *list_name, int cells_count, int index,
struct of_phandle_args *out_args);
@@ -738,6 +741,17 @@ static inline int of_parse_phandle_with_args(const struct device_node *np,
return -ENOSYS;
}
+static inline int of_parse_phandle_with_args_map(const struct device_node *np,
+ const char *list_name,
+ const char *cells_name,
+ const char *map_name,
+ const char *mask_name,
+ int index,
+ struct of_phandle_args *out_args)
+{
+ return -ENOSYS;
+}
+
static inline int of_parse_phandle_with_fixed_args(const struct device_node *np,
const char *list_name, int cells_count, int index,
struct of_phandle_args *out_args)
--
2.10.0.297.gf6727b0
^ permalink raw reply related
* [PATCH 0/2] OF phandle nexus support + GPIO nexus
From: Stephen Boyd @ 2016-11-24 10:25 UTC (permalink / raw)
To: Rob Herring, Frank Rowand
Cc: linux-arm-kernel, linux-kernel, devicetree, linux-gpio,
Pantelis Antoniou, Linus Walleij, Mark Brown
This is one small chunk of work related to DT overlays for expansion
boards. It would be good to have a way to expose #<list>-cells types of
providers through a connector in a standard way. So we introduce a way
to make "nexus" nodes for these types of properties to remap the consumer
number space to the other side of the connector's number space. It's
basically a copy of the interrupt nexus implementation, but without
the address space matching design and interrupt-parent walking.
The first patch implements a generic method to do this, and the second patch
adds a unit test for it. The third patch is more of an example than anything
else. It shows how we would modify frameworks to use the new API.
Stephen Boyd (3):
of: Support parsing phandle argument lists through a nexus node
of: unittest: Add phandle remapping test
gpio: Support gpio nexus dt bindings
drivers/gpio/gpiolib-of.c | 5 +-
drivers/of/base.c | 146 ++++++++++++++++++++++++++++
drivers/of/unittest-data/testcases.dts | 11 +++
drivers/of/unittest-data/tests-phandle.dtsi | 24 +++++
drivers/of/unittest.c | 124 +++++++++++++++++++++++
include/linux/of.h | 14 +++
6 files changed, 322 insertions(+), 2 deletions(-)
--
2.10.0.297.gf6727b0
^ permalink raw reply
* Re: [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on Hip06
From: Arnd Bergmann @ 2016-11-24 10:24 UTC (permalink / raw)
To: zhichang.yuan
Cc: mark.rutland@arm.com, Gabriele Paoloni, benh@kernel.crashing.org,
liviu.dudau@arm.com, Linuxarm, lorenzo.pieralisi@arm.com,
xuwei (O), Jason Gunthorpe, T homas Petazzoni,
linux-serial@vger.kernel.org, catalin.marinas@arm.com,
devicetree@vger.kernel.org, minyard@acm.org, will.deacon@arm.com,
John Garry, zourongrong@gmail.com, robh+dt@kernel.org,
bhelgaas@go og le.com, kantyzc
In-Reply-To: <5836AF11.7060400@hisilicon.com>
On Thursday, November 24, 2016 5:12:49 PM CET zhichang.yuan wrote:
> On 2016/11/24 7:23, Arnd Bergmann wrote:
> > On Wednesday, November 23, 2016 6:07:11 PM CET Arnd Bergmann wrote:
> >> On Wednesday, November 23, 2016 3:22:33 PM CET Gabriele Paoloni wrote:
> >>> From: Arnd Bergmann [mailto:arnd@arndb.de]
> >>>> On Friday, November 18, 2016 5:03:11 PM CET Gabriele Paoloni wrote:
> >>
> > @@ -781,7 +778,8 @@ int acpi_pci_probe_root_resources(struct acpi_pci_root_info *info)
> > else {
> > resource_list_for_each_entry_safe(entry, tmp, list) {
> > if (entry->res->flags & IORESOURCE_IO)
> > - acpi_pci_root_remap_iospace(entry);
> > + acpi_pci_root_remap_iospace(&device->fwnode,
> > + entry);
> >
> > if (entry->res->flags & IORESOURCE_DISABLED)
> > resource_list_destroy_entry(entry);
>
> I think those changes in pci_root.c is only to match the new definition of
> pci_register_io_range() and work for PCI I/O. It doesn't make sense for LPC, is
> it right?
Right, we wouldn't call acpi_pci_probe_root_resources() for LPC,
the change is just that we always pass the fwnode pointer to allow
matching based on that for any I/O space that does not have a
physical memory address associated with it.
I tried to keep this part general, so in theory that allows us to
have more than one I/O space without a CPU mapping, even though
we don't strictly need that for supporting your LPC controller.
> > diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> > index a50025a3777f..df96955a43f8 100644
> > --- a/drivers/block/nbd.c
> > +++ b/drivers/block/nbd.c
> > @@ -760,8 +760,10 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
> > set_bit(NBD_RUNNING, &nbd->runtime_flags);
> > blk_mq_update_nr_hw_queues(&nbd->tag_set, nbd->num_connections);
> > args = kcalloc(num_connections, sizeof(*args), GFP_KERNEL);
> > - if (!args)
> > + if (!args) {
> > + error = -ENOMEM;
> > goto out_err;
> > + }
> > nbd->task_recv = current;
> > mutex_unlock(&nbd->config_lock);
> >
> I think change here is none of the business.:)
Right, sorry about that, I forgot to commit this bugfix before looking
at the I/O space stuff.
> > + *host = NULL;
> > /* Get parent & match bus type */
> > parent = of_get_parent(dev);
> > if (parent == NULL)
> > @@ -600,8 +605,9 @@ static u64 __of_translate_address(struct device_node *dev,
> > pbus = of_match_bus(parent);
> > pbus->count_cells(dev, &pna, &pns);
> > if (!OF_CHECK_COUNTS(pna, pns)) {
> > - pr_err("Bad cell count for %s\n",
> > - of_node_full_name(dev));
> > + pr_debug("Bad cell count for %s\n",
> > + of_node_full_name(dev));
> > + *host = of_node_get(parent);
> > break;
> > }
> I don't think here is the right place to fill *host. I think you want to return
> the parent where the of_translate_one() failed for the 'ranges' property
> missing. So, I think this seems better:
>
> if (of_translate_one(dev, bus, pbus, addr, na, ns, pna, rprop)) {
> *host = of_node_get(dev);
> break;
> }
You are right, I got the wrong place. The parent node will have
a #address-cells but won't have ranges for the I/O space.
> > @@ -3272,7 +3276,12 @@ int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
> > /* check if the range hasn't been previously recorded */
> > spin_lock(&io_range_lock);
> > list_for_each_entry(range, &io_range_list, list) {
> > - if (addr >= range->start && addr + size <= range->start + size) {
> > + if (node == range->node)
> > + goto end_register;
> > +
> I don't think it is safe to only check the node had been registered. For
> PCI/PCIE, there is only one I/O windows in bridge, it seems ok. But for non-pci
> devices, such as ISA/LPC, I wonder it is possible there are several disjoint I/O
> 'ranges' entries...
I think this part is completely safe, I can't imagine why you'd
have more than one range of I/O ports that have valid translations.
Do you have a specific example in mind where that would not be
the case, or are you just worried about the principle in general?
> What parameters are necessary for linux PIO allocation?
> 1) For those bus devices which have no MMIO( that is to say, indirectIO is
> using), I think 'addr' is not needed, but 'size' is mandatory;
Agreed.
> I am thinking for our LPC, as there is no cpu address, we should not input
> 'addr' for the io range register. With 'size' as parameter, we implement a new
> io range register function where can assign an unique linux PIO for this
> register calling. The output linux PIO can allocate from a sub-range of whole
> I/O space of [0, IO_SPACE_LIMIT]. This sub-range is specific for indirectIO, I
> want to define a new macro, such as EXTIO_LIMIT, to represent the upper limit of
> indirect IO space.
>
> #if defined(CONFIG_PCI) && defined(CONFIG_INDIRECT_PIO)
> #define EXTIO_LIMIT PCIBIOS_MIN_IO
> #elif defined(CONFIG_INDIRECT_PIO)
> #define EXTIO_LIMIT 0x1000
> #else
> #define EXTIO_LIMIT 0x00
> #end
>
> We should do some checkings to ensure EXTIO_LIMIT < IO_SPACE_LIMIT.
I think we don't need to limit the EXTIO range at all. For your
specific case of LPC, we know it is limited, but my prototype
patch leaves this part generic enough to also allow using it
for a PCI host with indirect I/O space, and that can have a larger
size.
> Then when someone call pci_register_io_range() or a new function for the linux
> PIO register, we can allocate linux PIO from [0, EXTIO_LIMIT) for indirectIO
> bus, from [EXTIO_LIMIT, IO_SPACE_LIMIT] for MMIO;
>
> But there are issues confused me yet. For example, how to know the IO size for
> the indirectIO bus? You known, there is no 'ranges' property for those buses....
Good point. We normally call pci_register_io_range() from
of_pci_range_to_resource and its ACPI equivalent.
When there is no ranges, we obviously won't call it, but there is
also no size associated with it. I think this is ok because
the host driver would already know the size based on the hardware
register layout, and it can just register that directly.
> 2) For PCI MMIO, I think 'addr' is needed
So far I assumed it was, but actually we can perhaps remove
the address if we manage to kill off pci_address_to_pio()
and pci_pio_to_address.
> As for the current pci_register_io_range()/pci_address_to_pio(), I have two doubts:
>
> 2.1) If there are multiple PCI host bridges which support I/O transaction, I
> wonder whether the first host bridge can access the downstream devices with bus
> I/O address in [0, PCIBIOS_MIN_IO)
>
> for the first host bridge, pci_address_to_pio() will return a linux PIO range
> start from 0.
> But when calling __pci_assign_resource() to allocate the linux PIO for PCI/PCIE
> devices/buses which are just children of first host bus, it can not allocate
> linux PIO less than PCIBIOS_MIN_IO, which means kernel can not called in/out()
> with port less than PCIBIOS_MIN_IO. But we had ioremap [PCI_IOBASE + 0,
> PCI_IOBASE + size) to [pci_ioadd, pci_ioadd + size) before.
>
>
> static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev,
> int resno, resource_size_t size, resource_size_t align)
> {
> struct resource *res = dev->resource + resno;
> resource_size_t min;
> int ret;
>
> min = (res->flags & IORESOURCE_IO) ? PCIBIOS_MIN_IO : PCIBIOS_MIN_MEM;
>
>
> and in the later function:
>
> static int pci_bus_alloc_from_region(struct pci_bus *bus, struct resource *res,
> resource_size_t size, resource_size_t align,
> resource_size_t min, unsigned long type_mask,
> resource_size_t (*alignf)(void *,
>
> ....
> pci_bus_for_each_resource(bus, r, i) {
> resource_size_t min_used = min;
> ....
> if (avail.start)
> min_used = avail.start;
>
> max = avail.end;
>
> /* Ok, try it out.. */
> ret = allocate_resource(r, res, size, min_used, max,
> align, alignf, alignf_data);
>
> After allocate_resource(), a IO resource is allocated, but whose 'start' is not
> less than min_used.( since avail.start is 0, min_used will keep the 'min'
> without change to avail.start; Should be PCIBIOS_MIN_IO).
I'm not completely sure I'm following here. Generally speaking, addresses
below PCIBIOS_MIN_IO are intended for PCI-ISA bridges and PCI devices with
hardcoded port numbers for ISA compatibility, while __pci_assign_resource
is meant to do dynamic assignment of I/O resources above PCIBIOS_MIN_IO
so it does not conflict with the legacy ISA ports.
Does that address your concern?
> 2.2) Is it possible the return linux PIO isn't page-aligned?
>
> When calling pci_remap_iospace(const struct resource *res, phys_addr_t
> phys_addr), if res->start is not page-aligned, it seems that
> ioremap_page_range() will meet some issues for duplication iorempa for same
> virtual page.
>
> of-course, if we always configure the I/O ranges size as page-aligned, it will
> be OK.
>
> I found PowerPC will ensure the 'vaddress' and the 'size' are page-aligned
> before ioremap, do we need to improve the current handling in
> pci_register_io_range/pci_address_to_pio?
I think it would be a good idea to enforce page-alignment here, even
though everything could still work if it's not page-aligned.
The requirement for ioremap_page_range() is that the offset within
a page must be the same for the virtual and physical addresses.
Adding page-alignment to pci_register_io_range() could be an enhancement
that we can do independent of the other patches.
Thanks a lot for your detailed analysis and feedback.
Arnd
^ permalink raw reply
* Re: [RFC v2: PATCH 1/2] dt-bindings: Document the hi3660 reset bindings
From: zhangfei @ 2016-11-24 10:20 UTC (permalink / raw)
To: Philipp Zabel
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1479981017.2472.14.camel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
On 2016年11月24日 17:50, Philipp Zabel wrote:
> Am Donnerstag, den 24.11.2016, 17:40 +0800 schrieb zhangfei:
>> On 2016年11月24日 17:26, Philipp Zabel wrote:
>>> Am Mittwoch, den 23.11.2016, 16:07 +0800 schrieb Zhangfei Gao:
>>>> Add DT bindings documentation for hi3660 SoC reset controller.
>>>>
>>>> Signed-off-by: Zhangfei Gao <zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>>> ---
>>>> .../bindings/reset/hisilicon,hi3660-reset.txt | 51 ++++++++++++++++++++++
>>>> 1 file changed, 51 insertions(+)
>>>> create mode 100644 Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt b/Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt
>>>> new file mode 100644
>>>> index 0000000..250daf2
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt
>>>> @@ -0,0 +1,51 @@
>>>> +Hisilicon System Reset Controller
>>>> +======================================
>>>> +
>>>> +Please also refer to reset.txt in this directory for common reset
>>>> +controller binding usage.
>>>> +
>>>> +The reset controller registers are part of the system-ctl block on
>>>> +hi3660 SoC.
>>>> +
>>>> +Required properties:
>>>> +- compatible: should be
>>>> + "hisilicon,hi3660-reset"
>>>> +- #reset-cells: 1, see below
>>>> +- hisi,rst-syscon: phandle of the reset's syscon.
>>>> +- hisi,reset-bits: Contains the reset control register information
>>>> + Should contain 2 cells for each reset exposed to
>>>> + consumers, defined as:
>>>> + Cell #1 : offset from the syscon register base
>>>> + Cell #2 : bits position of the control register
>>>> +
>>>> +Example:
>>>> + iomcu: iomcu@ffd7e000 {
>>>> + compatible = "hisilicon,hi3660-iomcu", "syscon";
>>>> + reg = <0x0 0xffd7e000 0x0 0x1000>;
>>>> + };
>>>> +
>>>> + iomcu_rst: iomcu_rst_controller {
>>> This should be
>>> iomcu_rst: reset-controller {
>>>
>>>> + compatible = "hisilicon,hi3660-reset";
>>>> + #reset-cells = <1>;
>>>> + hisi,rst-syscon = <&iomcu>;
>>>> + hisi,reset-bits = <0x20 0x8 /* 0: i2c0 */
>>>> + 0x20 0x10 /* 1: i2c1 */
>>>> + 0x20 0x20 /* 2: i2c2 */
>>>> + 0x20 0x8000000>; /* 3: i2c6 */
>>>> + };
>>> The reset lines are controlled through iomcu bits, is there a reason not
>>> to put the iomcu_rst node inside the iomcu node? That way the
>>> hisi,rst-syscon property could be removed and the syscon could be
>>> retrieved via the reset-controller parent node.
>> iomcu is common registers, controls clock and reset, etc.
>> So we use syscon, without mapping the registers everywhere.
>> It is common case in hisilicon, same in hi6220.
>>
>> Also the #clock-cells and #reset-cells can not be put in the same node,
>> if they are both using probe, since reset_probe will not be called.
>>
>> So we use hisi,rst-syscon as a general solution.
> What I meant is this:
>
> iomcu: iomcu@ffd7e000 {
> compatible = "hisilicon,hi3660-iomcu", "syscon", "simple-mfd";
> reg = <0x0 0xffd7e000 0x0 0x1000>;
#clock-cells = <1>;
In my test, if there add #clock-cells = <1>, reset_probe will not be
called any more.
Since clk_probe is called first.
No matter iomcu_rst is child node or not.
Thanks
>
> iomcu_rst: reset-controller {
> compatible = "hisilicon,hi3660-reset";
> #reset-cells = <1>;
> hisi,reset-bits = <0x20 0x8 /* 0: i2c0 */
> 0x20 0x10 /* 1: i2c1 */
> 0x20 0x20 /* 2: i2c2 */
> 0x20 0x8000000>; /* 3: i2c6 */
> };
> };
>
> regards
> Philipp
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 0/3] arm64: dts: r8a7796: Add CAN/CAN FD support
From: Simon Horman @ 2016-11-24 10:17 UTC (permalink / raw)
To: Chris Paterson
Cc: Mark Rutland, devicetree@vger.kernel.org, Magnus Damm,
linux-can@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
Rob Herring, Marc Kleine-Budde,
linux-arm-kernel@lists.infradead.org, Ramesh Shanmugasundaram,
Wolfgang Grandegger
In-Reply-To: <HK2PR0601MB132929329C3574A0B4D9A0EAB7B60@HK2PR0601MB1329.apcprd06.prod.outlook.com>
Hi Chris,
On Thu, Nov 24, 2016 at 10:05:08AM +0000, Chris Paterson wrote:
> Hello Simon,
>
> From: Simon Horman [mailto:horms@verge.net.au]
> Sent: 23 November 2016 14:30
> > On Wed, Nov 23, 2016 at 02:18:13PM +0100, Marc Kleine-Budde wrote:
> > > On 11/23/2016 01:14 PM, Chris Paterson wrote:
> > > > This patch series adds CAN and CAN FD support to the r8a7796.
> > > >
> > > > Based on renesas-devel-20161122-v4.9-rc6.
> > > >
> > > > Chris Paterson (3):
> > > > arm64: dts: r8a7796: Add CAN external clock support
> > > > arm64: dts: r8a7796: Add CAN support
> > > > arm64: dts: r8a7796: Add CAN FD support
> > > >
> > > > .../devicetree/bindings/net/can/rcar_can.txt | 12 +++--
> > > > .../devicetree/bindings/net/can/rcar_canfd.txt | 12 +++--
> > > > arch/arm64/boot/dts/renesas/r8a7796.dtsi | 61
> > ++++++++++++++++++++++
> > > > 3 files changed, 75 insertions(+), 10 deletions(-)
> > >
> > > For all three:
> > >
> > > Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
> > >
> > > Who takes this series?
> >
> > I would like to see these patches split up so that the .../devicetree/bindings/
> > portions can go through you whole the arch/arm64/boot/dts/renesas/
> > portions go thorugh my renesas tree.
>
> Okay, will do.
Thanks.
> > Regarding the arch/arm64/boot/dts/renesas/ portion, I would like some
> > consideration given to what effect enabling memory above 4Gb (64bit
> > addressing) would have.
>
> Can you give me some guidance here? I'm not sure what you're referring
> to. As far as I know the DT reg definition here is 64-bit, or are you
> referring to DMA usage? If the later, neither CAN driver uses DMA.
Sorry for not being clearer.
What I would like to know is if there are any problems in the CAN driver
or hardware that would prevent it from functioning with memory that
requires 64bit addressing present.
If the CAN hardware cannot use DMA then DMA doesn't need to be taken into
account. But if it DMA could be enabled in future for CAN, for example
after some driver enhancements, then it would be good to know if 64bit
memory can be supported - if not it would imply DMA cannot be enabled.
As for non-DMA mode, will this function if memory above 4G is present?
If not then in theory such memory couldn't be enabled if the CAN driver
is enabled. This is my main concern.
Does the above help?
^ permalink raw reply
* Re: [RFC PATCH 2/5] dmaengine: allow sun6i-dma for more SoCs
From: Andre Przywara @ 2016-11-24 10:12 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Maxime Ripard, Icenowy Zheng, linux-sunxi, linux-arm-kernel,
Mark Rutland, Rob Herring, devicetree
In-Reply-To: <CAGb2v65G7=9ah+sEet=z5vss60kL5ZLSkNsAcGpwu8V6AWdEGA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi,
On 24/11/16 09:30, Chen-Yu Tsai wrote:
> On Thu, Nov 24, 2016 at 5:16 PM, Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org> 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-5wv7dgnIgG8@public.gmane.org> 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-5wv7dgnIgG8@public.gmane.org>
>>>> ---
>>>> 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. But yes, there's no
> reason why you can't build it for the earlier SoC. It just doesn't
> get used.
>
>>
>> depends on ARCH_SUNXI || COMPILE_TEST
>>
>> and maybe:
>>
>> default y if MACH_SUN6I || MACH_SUN8I
>>
>> Though I see that both multi_v7_defconfig and sunxi_defconfig explicitly
>> set this, so this wouldn't be needed?
>
> I guess it's just nice to get stuff out of defconfig?
> Why not go all the way and just have
>
> default y if ARCH_SUNXI
Well, I am all for it, but I had the impression that there is a lot of
opposition against this approach. Apparently people still want to save
some bytes by building a kernel tailored to one particular SoC.
So I didn't dare to come up with this one.
But it should work to use "# DMA_SUN6I is not selected" in a particular
.config or defconfig to deselect it, right?
Waiting for Maxime's opinion here.
(And also need to check whether the DMA really works on ARM64.
Surprisingly the code compiled cleanly, but I am wondering whether it
properly deals with 32-bit limitation of this controller. I just needed
it because the H3 DT references DMA for SPI and UART).
Cheers,
Andre.
^ permalink raw reply
* Re: [PATCH 5/10] dt: bindings: Add bindings for Marvell Xenon SD Host Controller
From: Thomas Petazzoni @ 2016-11-24 10:10 UTC (permalink / raw)
To: Marcin Wojtas
Cc: Gregory CLEMENT, Arnd Bergmann,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Jimmy Xu, Andrew Lunn, Ulf Hansson, Romain Perier, Liuliu Zhao,
Peng Zhu, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Nadav Haklai, Ziji Hu, Victor Gu, Doug Jones, Jisheng Zhang,
Yehuda Yitschak, Xueping Liu, Hilbert Zhang, Shiwu Zhang
In-Reply-To: <CAPv3WKddPHgpRU2_tVoDF=5Z-nqfFPxjgJ-+z9o-1tR2=fFvAA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
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.
Agreed.
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* RE: [PATCH 0/3] arm64: dts: r8a7796: Add CAN/CAN FD support
From: Chris Paterson @ 2016-11-24 10:05 UTC (permalink / raw)
To: Simon Horman, Marc Kleine-Budde
Cc: Wolfgang Grandegger, Magnus Damm, Rob Herring, Mark Rutland,
Ramesh Shanmugasundaram, linux-renesas-soc@vger.kernel.org,
devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-can@vger.kernel.org
In-Reply-To: <20161123142938.GF9057@verge.net.au>
Hello Simon,
From: Simon Horman [mailto:horms@verge.net.au]
Sent: 23 November 2016 14:30
> On Wed, Nov 23, 2016 at 02:18:13PM +0100, Marc Kleine-Budde wrote:
> > On 11/23/2016 01:14 PM, Chris Paterson wrote:
> > > This patch series adds CAN and CAN FD support to the r8a7796.
> > >
> > > Based on renesas-devel-20161122-v4.9-rc6.
> > >
> > > Chris Paterson (3):
> > > arm64: dts: r8a7796: Add CAN external clock support
> > > arm64: dts: r8a7796: Add CAN support
> > > arm64: dts: r8a7796: Add CAN FD support
> > >
> > > .../devicetree/bindings/net/can/rcar_can.txt | 12 +++--
> > > .../devicetree/bindings/net/can/rcar_canfd.txt | 12 +++--
> > > arch/arm64/boot/dts/renesas/r8a7796.dtsi | 61
> ++++++++++++++++++++++
> > > 3 files changed, 75 insertions(+), 10 deletions(-)
> >
> > For all three:
> >
> > Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
> >
> > Who takes this series?
>
> I would like to see these patches split up so that the .../devicetree/bindings/
> portions can go through you whole the arch/arm64/boot/dts/renesas/
> portions go thorugh my renesas tree.
Okay, will do.
>
> Regarding the arch/arm64/boot/dts/renesas/ portion, I would like some
> consideration given to what effect enabling memory above 4Gb (64bit
> addressing) would have.
Can you give me some guidance here? I'm not sure what you're referring to. As far as I know the DT reg definition here is 64-bit, or are you referring to DMA usage? If the later, neither CAN driver uses DMA.
Kind regards, Chris
^ permalink raw reply
* Re: [PATCH 5/10] dt: bindings: Add bindings for Marvell Xenon SD Host Controller
From: Arnd Bergmann @ 2016-11-24 10:04 UTC (permalink / raw)
To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Thomas Petazzoni, Gregory CLEMENT, Jimmy Xu, Andrew Lunn,
Ulf Hansson, Romain Perier, Liuliu Zhao, Peng Zhu, Adrian Hunter,
Nadav Haklai, Ziji Hu, Victor Gu, Doug Jones, Jisheng Zhang,
Yehuda Yitschak, Wei(SOCP) Liu, Xueping Liu, Hilbert Zhang,
Shiwu Zhang, Yu Cao, Sebastian
In-Reply-To: <20161124104858.3604c11d-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
On Thursday, November 24, 2016 10:48:58 AM CET Thomas Petazzoni wrote:
> Hello,
>
> On Thu, 24 Nov 2016 10:44:48 +0100, Gregory CLEMENT wrote:
>
> > "A single Xenon IP can support multiple slots.
> > Each slot acts as an independent SDHC. It owns independent resources, such
> > as register sets clock and PHY.
> > Each slot should have an independent device tree node."
>
> I think this wording is still very confusing, and continues to cause
> confusion.
>
> We should just state that each Xenon controller supports a single slot,
> and that's it.
>
> The text still says "a single Xenon IP can support multiple slots",
> which continues to cause confusion.
Agreed. Ideally we'd find out why exactly the slot number must
be used for accessing some of the registers to have a better
explanation to put in there, aside from stating that only one
slot is supported but the number must be set.
Could it be that this is some form of pinmuxing, i.e. that each
controller could in theory be used for any of the slots but you
have to pick one of them?
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v28 9/9] Documentation: dt: chosen properties for arm64 kdump
From: AKASHI Takahiro @ 2016-11-24 9:59 UTC (permalink / raw)
To: catalin.marinas-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8
Cc: james.morse-5wv7dgnIgG8, geoff-wEGCiKHe2LqWVfeAwA7xHQ,
bauerman-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
dyoung-H+wXaHxf7aLQT0dZR+AlfA,
kexec-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, AKASHI Takahiro
In-Reply-To: <20161124095523.6972-1-takahiro.akashi-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
From: James Morse <james.morse-5wv7dgnIgG8@public.gmane.org>
Add documentation for
linux,crashkernel-base and crashkernel-size,
linux,usable-memory-range
linux,elfcorehdr
used by arm64 kdump to decribe the kdump reserved area, and
the elfcorehdr's location within it.
Signed-off-by: James Morse <james.morse-5wv7dgnIgG8@public.gmane.org>
[takahiro.akashi-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org: added "linux,crashkernel-base" and "-size" ]
Signed-off-by: AKASHI Takahiro <takahiro.akashi-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
---
Documentation/devicetree/bindings/chosen.txt | 50 ++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)
diff --git a/Documentation/devicetree/bindings/chosen.txt b/Documentation/devicetree/bindings/chosen.txt
index 6ae9d82..7b11516 100644
--- a/Documentation/devicetree/bindings/chosen.txt
+++ b/Documentation/devicetree/bindings/chosen.txt
@@ -52,3 +52,53 @@ This property is set (currently only on PowerPC, and only needed on
book3e) by some versions of kexec-tools to tell the new kernel that it
is being booted by kexec, as the booting environment may differ (e.g.
a different secondary CPU release mechanism)
+
+linux,crashkernel-base
+linux,crashkernel-size
+----------------------
+
+These properties (currently used on PowerPC and arm64) indicates
+the base address and the size, respectively, of the reserved memory
+range for crash dump kernel.
+e.g.
+
+/ {
+ chosen {
+ linux,crashkernel-base = <0x9 0xf0000000>;
+ linux,crashkernel-size = <0x0 0x10000000>;
+ };
+};
+
+linux,usable-memory-range
+-------------------------
+
+This property (currently used only on arm64) holds the memory range,
+the base address and the size, which can be used as system ram on
+the *current* kernel. Note that, if this property is present, any memory
+regions under "memory" nodes in DT blob or ones marked as "conventional
+memory" in EFI memory map should be ignored.
+e.g.
+
+/ {
+ chosen {
+ linux,usable-memory-range = <0x9 0xf0000000 0x0 0x10000000>;
+ };
+};
+
+The main usage is for crash dump kernel to identify its own usable
+memory and exclude, at its boot time, any other memory areas that are
+part of the panicked kernel's memory.
+
+linux,elfcorehdr
+----------------
+
+This property (currently used only on arm64) holds the memory range,
+the address and the size, of the elf core header which mainly describes
+the panicked kernel's memory layout as PT_LOAD segments of elf format.
+e.g.
+
+/ {
+ chosen {
+ linux,elfcorehdr = <0x9 0xfffff000 0x0 0x800>;
+ };
+};
--
2.10.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell Xenon SDHC
From: Arnd Bergmann @ 2016-11-24 9:56 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: Ulf Hansson, Adrian Hunter, linux-mmc-u79uwXL29TY76Z2rM5mHXA,
Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Rob Herring,
devicetree-u79uwXL29TY76Z2rM5mHXA, Thomas Petazzoni,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Ziji Hu,
Jack(SH) Zhu, Jimmy Xu, Jisheng Zhang, Nadav Haklai, Ryan Gao,
Doug Jones, Shiwu Zhang, Victor Gu, Wei(SOCP) Liu, Wilson Ding,
Xueping Liu <xpli>
In-Reply-To: <a05ffd140f4edc02fc3128db8445b2264cf38723.1477911954.git-series.gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
On Monday, October 31, 2016 12:09:56 PM CET Gregory CLEMENT wrote:
> From: Ziji Hu <huziji-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
>
> 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-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>
Please explain in the changelog why this is not a generic
phy driver (or three of them).
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC v2: PATCH 1/2] dt-bindings: Document the hi3660 reset bindings
From: Philipp Zabel @ 2016-11-24 9:50 UTC (permalink / raw)
To: zhangfei
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <ac3c8e65-e4f2-3a9d-452c-f270d245cf9d-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Am Donnerstag, den 24.11.2016, 17:40 +0800 schrieb zhangfei:
>
> On 2016年11月24日 17:26, Philipp Zabel wrote:
> > Am Mittwoch, den 23.11.2016, 16:07 +0800 schrieb Zhangfei Gao:
> >> Add DT bindings documentation for hi3660 SoC reset controller.
> >>
> >> Signed-off-by: Zhangfei Gao <zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> >> ---
> >> .../bindings/reset/hisilicon,hi3660-reset.txt | 51 ++++++++++++++++++++++
> >> 1 file changed, 51 insertions(+)
> >> create mode 100644 Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt b/Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt
> >> new file mode 100644
> >> index 0000000..250daf2
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt
> >> @@ -0,0 +1,51 @@
> >> +Hisilicon System Reset Controller
> >> +======================================
> >> +
> >> +Please also refer to reset.txt in this directory for common reset
> >> +controller binding usage.
> >> +
> >> +The reset controller registers are part of the system-ctl block on
> >> +hi3660 SoC.
> >> +
> >> +Required properties:
> >> +- compatible: should be
> >> + "hisilicon,hi3660-reset"
> >> +- #reset-cells: 1, see below
> >> +- hisi,rst-syscon: phandle of the reset's syscon.
> >> +- hisi,reset-bits: Contains the reset control register information
> >> + Should contain 2 cells for each reset exposed to
> >> + consumers, defined as:
> >> + Cell #1 : offset from the syscon register base
> >> + Cell #2 : bits position of the control register
> >> +
> >> +Example:
> >> + iomcu: iomcu@ffd7e000 {
> >> + compatible = "hisilicon,hi3660-iomcu", "syscon";
> >> + reg = <0x0 0xffd7e000 0x0 0x1000>;
> >> + };
> >> +
> >> + iomcu_rst: iomcu_rst_controller {
> > This should be
> > iomcu_rst: reset-controller {
> >
> >> + compatible = "hisilicon,hi3660-reset";
> >> + #reset-cells = <1>;
> >> + hisi,rst-syscon = <&iomcu>;
> >> + hisi,reset-bits = <0x20 0x8 /* 0: i2c0 */
> >> + 0x20 0x10 /* 1: i2c1 */
> >> + 0x20 0x20 /* 2: i2c2 */
> >> + 0x20 0x8000000>; /* 3: i2c6 */
> >> + };
> > The reset lines are controlled through iomcu bits, is there a reason not
> > to put the iomcu_rst node inside the iomcu node? That way the
> > hisi,rst-syscon property could be removed and the syscon could be
> > retrieved via the reset-controller parent node.
> iomcu is common registers, controls clock and reset, etc.
> So we use syscon, without mapping the registers everywhere.
> It is common case in hisilicon, same in hi6220.
>
> Also the #clock-cells and #reset-cells can not be put in the same node,
> if they are both using probe, since reset_probe will not be called.
>
> So we use hisi,rst-syscon as a general solution.
What I meant is this:
iomcu: iomcu@ffd7e000 {
compatible = "hisilicon,hi3660-iomcu", "syscon", "simple-mfd";
reg = <0x0 0xffd7e000 0x0 0x1000>;
iomcu_rst: reset-controller {
compatible = "hisilicon,hi3660-reset";
#reset-cells = <1>;
hisi,reset-bits = <0x20 0x8 /* 0: i2c0 */
0x20 0x10 /* 1: i2c1 */
0x20 0x20 /* 2: i2c2 */
0x20 0x8000000>; /* 3: i2c6 */
};
};
regards
Philipp
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 5/10] dt: bindings: Add bindings for Marvell Xenon SD Host Controller
From: Marcin Wojtas @ 2016-11-24 9:49 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: Arnd Bergmann, linux-arm-kernel@lists.infradead.org, Jimmy Xu,
Andrew Lunn, Ulf Hansson, Romain Perier, Liuliu Zhao, Peng Zhu,
linux-kernel@vger.kernel.org, Nadav Haklai, Ziji Hu, Victor Gu,
Doug Jones, Jisheng Zhang, Yehuda Yitschak, Xueping Liu,
Hilbert Zhang, Shiwu Zhang, Yu Cao,
Sebastian Hesselbarth <sebastian.he>
In-Reply-To: <8737ihmctr.fsf@free-electrons.com>
Hi Gregory,
2016-11-24 10:44 GMT+01:00 Gregory CLEMENT <gregory.clement@free-electrons.com>:
> Hi Arnd,
>
> On jeu., nov. 24 2016, Arnd Bergmann <arnd@arndb.de> wrote:
>
>> On Thursday, November 24, 2016 10:22:31 AM CET Gregory CLEMENT wrote:
>>>
>>> I don't have an option for mmc in general, but using child node do not
>>> fit at all the xenon controller.
>>>
>>> For this controller each slot has its own set of register, so there is
>>> no common ressource to share so no advantage to use it. Using child node
>>> in our case will just make the code more complex for no benefit.
>>
>> If every slot has its own registers, what is it that makes up the
>> 'controller'? It sounds to me that you just have to adjust the terminology
>> and talk about multiple controllers then, with one slot per controller.
>>
>
> I agree and actually there were some words about in at the begining of
> the binding:
>
> "A single Xenon IP can support multiple slots.
> Each slot acts as an independent SDHC. It owns independent resources, such
> as register sets clock and PHY.
> Each slot should have an independent device tree node."
>
> All the confusion came from the fact that we still need to identify a
> slot ID. For an obscure reason the hardware can't guess the slot ID from
> the address register."
>
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.
Best regards,
Marcin
^ permalink raw reply
* Re: [PATCH 5/10] dt: bindings: Add bindings for Marvell Xenon SD Host Controller
From: Thomas Petazzoni @ 2016-11-24 9:48 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: Arnd Bergmann, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Jimmy Xu, Andrew Lunn, Ulf Hansson, Romain Perier, Liuliu Zhao,
Peng Zhu, linux-kernel@vger.kernel.org, Nadav Haklai, Ziji Hu,
Victor Gu, Doug Jones, Jisheng Zhang, Yehuda Yitschak,
Marcin Wojtas, Xueping Liu, Hilbert Zhang, Shiwu Zhang, Yu Cao,
Sebastian Hesselbarth <sebastian.hesselbart>
In-Reply-To: <8737ihmctr.fsf-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Hello,
On Thu, 24 Nov 2016 10:44:48 +0100, Gregory CLEMENT wrote:
> "A single Xenon IP can support multiple slots.
> Each slot acts as an independent SDHC. It owns independent resources, such
> as register sets clock and PHY.
> Each slot should have an independent device tree node."
I think this wording is still very confusing, and continues to cause
confusion.
We should just state that each Xenon controller supports a single slot,
and that's it.
The text still says "a single Xenon IP can support multiple slots",
which continues to cause confusion.
Best regards,
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC v2: PATCH 1/2] dt-bindings: Document the hi3660 reset bindings
From: zhangfei @ 2016-11-24 9:40 UTC (permalink / raw)
To: Philipp Zabel; +Cc: devicetree, linux-arm-kernel
In-Reply-To: <1479979605.2472.4.camel@pengutronix.de>
On 2016年11月24日 17:26, Philipp Zabel wrote:
> Am Mittwoch, den 23.11.2016, 16:07 +0800 schrieb Zhangfei Gao:
>> Add DT bindings documentation for hi3660 SoC reset controller.
>>
>> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
>> ---
>> .../bindings/reset/hisilicon,hi3660-reset.txt | 51 ++++++++++++++++++++++
>> 1 file changed, 51 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt
>>
>> diff --git a/Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt b/Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt
>> new file mode 100644
>> index 0000000..250daf2
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt
>> @@ -0,0 +1,51 @@
>> +Hisilicon System Reset Controller
>> +======================================
>> +
>> +Please also refer to reset.txt in this directory for common reset
>> +controller binding usage.
>> +
>> +The reset controller registers are part of the system-ctl block on
>> +hi3660 SoC.
>> +
>> +Required properties:
>> +- compatible: should be
>> + "hisilicon,hi3660-reset"
>> +- #reset-cells: 1, see below
>> +- hisi,rst-syscon: phandle of the reset's syscon.
>> +- hisi,reset-bits: Contains the reset control register information
>> + Should contain 2 cells for each reset exposed to
>> + consumers, defined as:
>> + Cell #1 : offset from the syscon register base
>> + Cell #2 : bits position of the control register
>> +
>> +Example:
>> + iomcu: iomcu@ffd7e000 {
>> + compatible = "hisilicon,hi3660-iomcu", "syscon";
>> + reg = <0x0 0xffd7e000 0x0 0x1000>;
>> + };
>> +
>> + iomcu_rst: iomcu_rst_controller {
> This should be
> iomcu_rst: reset-controller {
>
>> + compatible = "hisilicon,hi3660-reset";
>> + #reset-cells = <1>;
>> + hisi,rst-syscon = <&iomcu>;
>> + hisi,reset-bits = <0x20 0x8 /* 0: i2c0 */
>> + 0x20 0x10 /* 1: i2c1 */
>> + 0x20 0x20 /* 2: i2c2 */
>> + 0x20 0x8000000>; /* 3: i2c6 */
>> + };
> The reset lines are controlled through iomcu bits, is there a reason not
> to put the iomcu_rst node inside the iomcu node? That way the
> hisi,rst-syscon property could be removed and the syscon could be
> retrieved via the reset-controller parent node.
iomcu is common registers, controls clock and reset, etc.
So we use syscon, without mapping the registers everywhere.
It is common case in hisilicon, same in hi6220.
Also the #clock-cells and #reset-cells can not be put in the same node,
if they are both using probe, since reset_probe will not be called.
So we use hisi,rst-syscon as a general solution.
Thanks
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 5/10] dt: bindings: Add bindings for Marvell Xenon SD Host Controller
From: Arnd Bergmann @ 2016-11-24 9:34 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Gregory CLEMENT, Jimmy Xu, Andrew Lunn, Ulf Hansson,
Romain Perier, Liuliu Zhao, Peng Zhu,
linux-kernel@vger.kernel.org, Nadav Haklai, Ziji Hu, Victor Gu,
Doug Jones, Jisheng Zhang, Yehuda Yitschak, Marcin Wojtas,
Xueping Liu, Hilbert Zhang, Shiwu Zhang, Yu Cao,
Sebastian Hesselbarth
In-Reply-To: <877f7tmduw.fsf@free-electrons.com>
On Thursday, November 24, 2016 10:22:31 AM CET Gregory CLEMENT wrote:
>
> I don't have an option for mmc in general, but using child node do not
> fit at all the xenon controller.
>
> For this controller each slot has its own set of register, so there is
> no common ressource to share so no advantage to use it. Using child node
> in our case will just make the code more complex for no benefit.
If every slot has its own registers, what is it that makes up the
'controller'? It sounds to me that you just have to adjust the terminology
and talk about multiple controllers then, with one slot per controller.
Arnd
^ permalink raw reply
* [PATCH] ARM: dts: da850: enable the memctrl and mstpri nodes per board
From: Bartosz Golaszewski @ 2016-11-24 9:31 UTC (permalink / raw)
To: Kevin Hilman, Michael Turquette, Sekhar Nori, Rob Herring,
Frank Rowand, Mark Rutland, Peter Ujfalusi, Russell King
Cc: LKML, arm-soc, linux-drm, linux-devicetree, Jyri Sarha,
Tomi Valkeinen, David Airlie, Laurent Pinchart, Robin Murphy,
Sudeep Holla, Bartosz Golaszewski
Currently the memory controller and master priorities drivers are
enabled in da850.dtsi. For boards for which there are no settings
defined, this makes these drivers emit error messages.
Disable the nodes in da850.dtsi and only enable them for da850-lcdk -
the only board that currently needs them.
Signed-off-by: Bartosz Golaszewski <bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
arch/arm/boot/dts/da850-lcdk.dts | 8 ++++++++
arch/arm/boot/dts/da850.dtsi | 2 ++
2 files changed, 10 insertions(+)
diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
index 3b99a88..94504c8 100644
--- a/arch/arm/boot/dts/da850-lcdk.dts
+++ b/arch/arm/boot/dts/da850-lcdk.dts
@@ -283,3 +283,11 @@
&display {
status = "okay";
};
+
+&prictrl {
+ status = "okay";
+};
+
+&memctrl {
+ status = "okay";
+};
diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index 36066fa..4e40187 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -212,6 +212,7 @@
prictrl: priority-controller@14110 {
compatible = "ti,da850-mstpri";
reg = <0x14110 0x0c>;
+ status = "disabled";
};
cfgchip: chip-controller@1417c {
compatible = "ti,da830-cfgchip", "syscon", "simple-mfd";
@@ -457,5 +458,6 @@
memctrl: memory-controller@b0000000 {
compatible = "ti,da850-ddr-controller";
reg = <0xb0000000 0xe8>;
+ status = "disabled";
};
};
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [RFC PATCH 2/5] dmaengine: allow sun6i-dma for more SoCs
From: Chen-Yu Tsai @ 2016-11-24 9:30 UTC (permalink / raw)
To: Andre Przywara
Cc: Chen-Yu Tsai, Maxime Ripard, Icenowy Zheng, linux-sunxi,
linux-arm-kernel, Mark Rutland, Rob Herring, devicetree
In-Reply-To: <34b5e50f-a091-9bd8-7a74-96e538a7351d-5wv7dgnIgG8@public.gmane.org>
On Thu, Nov 24, 2016 at 5:16 PM, Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org> 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-5wv7dgnIgG8@public.gmane.org> 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-5wv7dgnIgG8@public.gmane.org>
>>> ---
>>> 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. But yes, there's no
reason why you can't build it for the earlier SoC. It just doesn't
get used.
>
> depends on ARCH_SUNXI || COMPILE_TEST
>
> and maybe:
>
> default y if MACH_SUN6I || MACH_SUN8I
>
> Though I see that both multi_v7_defconfig and sunxi_defconfig explicitly
> set this, so this wouldn't be needed?
I guess it's just nice to get stuff out of defconfig?
Why not go all the way and just have
default y if ARCH_SUNXI
ChenYu
>
> Cheers,
> Andre.
^ permalink raw reply
* Re: [RFC v2: PATCH 1/2] dt-bindings: Document the hi3660 reset bindings
From: Philipp Zabel @ 2016-11-24 9:26 UTC (permalink / raw)
To: Zhangfei Gao
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1479888476-13138-2-git-send-email-zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Am Mittwoch, den 23.11.2016, 16:07 +0800 schrieb Zhangfei Gao:
> Add DT bindings documentation for hi3660 SoC reset controller.
>
> Signed-off-by: Zhangfei Gao <zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> .../bindings/reset/hisilicon,hi3660-reset.txt | 51 ++++++++++++++++++++++
> 1 file changed, 51 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt
>
> diff --git a/Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt b/Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt
> new file mode 100644
> index 0000000..250daf2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt
> @@ -0,0 +1,51 @@
> +Hisilicon System Reset Controller
> +======================================
> +
> +Please also refer to reset.txt in this directory for common reset
> +controller binding usage.
> +
> +The reset controller registers are part of the system-ctl block on
> +hi3660 SoC.
> +
> +Required properties:
> +- compatible: should be
> + "hisilicon,hi3660-reset"
> +- #reset-cells: 1, see below
> +- hisi,rst-syscon: phandle of the reset's syscon.
> +- hisi,reset-bits: Contains the reset control register information
> + Should contain 2 cells for each reset exposed to
> + consumers, defined as:
> + Cell #1 : offset from the syscon register base
> + Cell #2 : bits position of the control register
> +
> +Example:
> + iomcu: iomcu@ffd7e000 {
> + compatible = "hisilicon,hi3660-iomcu", "syscon";
> + reg = <0x0 0xffd7e000 0x0 0x1000>;
> + };
> +
> + iomcu_rst: iomcu_rst_controller {
This should be
iomcu_rst: reset-controller {
> + compatible = "hisilicon,hi3660-reset";
> + #reset-cells = <1>;
> + hisi,rst-syscon = <&iomcu>;
> + hisi,reset-bits = <0x20 0x8 /* 0: i2c0 */
> + 0x20 0x10 /* 1: i2c1 */
> + 0x20 0x20 /* 2: i2c2 */
> + 0x20 0x8000000>; /* 3: i2c6 */
> + };
The reset lines are controlled through iomcu bits, is there a reason not
to put the iomcu_rst node inside the iomcu node? That way the
hisi,rst-syscon property could be removed and the syscon could be
retrieved via the reset-controller parent node.
> +Specifying reset lines connected to IP modules
> +==============================================
> +example:
> +
> + i2c0: i2c@..... {
> + ...
> + resets = <&iomcu_rst 0>;
> + ...
> + };
> +
> + i2c1: i2c@..... {
> + ...
> + resets = <&iomcu_rst 1>;
> + ...
> + };
regards
Philipp
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 5/10] dt: bindings: Add bindings for Marvell Xenon SD Host Controller
From: Gregory CLEMENT @ 2016-11-24 9:22 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Ulf Hansson, Rob Herring, Ziji Hu, Adrian Hunter,
linux-mmc@vger.kernel.org, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, devicetree@vger.kernel.org,
Thomas Petazzoni, linux-arm-kernel@lists.infradead.org,
Jack(SH) Zhu, Jimmy Xu, Jisheng Zhang, Nadav Haklai, Ryan Gao,
Doug Jones, Shiwu Zhang, Victor Gu <xi>
In-Reply-To: <4031579.CBE32NHUoW@wuerfel>
Hi Arnd,
On jeu., nov. 24 2016, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> On Thursday, November 24, 2016 10:05:45 AM CET Ulf Hansson wrote:
>> > You also mentioned other bindings using child nodes, but for this one
>> > we have one controller with only one set of register with multiple slots
>> > (Atmel is an example). Here each slot have it own set of register.
>> >
>> > Actually giving the fact that each slot is controlled by a different set
>> > of register I wonder why the hardware can't also deduce the slot number
>> > from the address register. For me it looks like an hardware bug but we
>> > have to deal with it.
>> >
>> > Do you still think we needchild node here?
>>
>> Using child-nodes for slots like what's done in the atmel case, is
>> currently broken. I would recommend to avoid using child-nodes for
>> slots, if possible.
>>
>> To give you some more background, currently the mmc core treats child
>> nodes as embedded non-removable cards or SDIO funcs. However, we can
>> change to make child-nodes also allowed to describe slots, but it
>> requires a specific compatible for "slots" and of course then we also
>> need to update the DT parsing of the child-nodes in the mmc core.
>>
>> Documentation/devicetree/bindings/mmc/mmc.txt
>> Documentation/devicetree/bindings/mmc/mmc-card.txt
>
> I don't see anything wrong with having child nodes for the slots
> even with the current binding, under one condition:
>
> The mmc.txt binding above must refer only to the child node, while
> the parent node conceptually becomes a plain bus or MFD that
> happens to encapsulate multiple MMC host controllers, and possibly
> provides some shared registers to them.
I don't have an option for mmc in general, but using child node do not
fit at all the xenon controller.
For this controller each slot has its own set of register, so there is
no common ressource to share so no advantage to use it. Using child node
in our case will just make the code more complex for no benefit.
Gregory
>
> Arnd
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC PATCH 2/5] dmaengine: allow sun6i-dma for more SoCs
From: Andre Przywara @ 2016-11-24 9:16 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Maxime Ripard, Icenowy Zheng, linux-sunxi, linux-arm-kernel,
Mark Rutland, Rob Herring, devicetree
In-Reply-To: <CAGb2v67M8DrPaf8GzSPEjekgV6cLcXXzO3tVUc9kjUDcM3BE_w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
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-5wv7dgnIgG8@public.gmane.org> 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-5wv7dgnIgG8@public.gmane.org>
>> ---
>> 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:
depends on ARCH_SUNXI || COMPILE_TEST
and maybe:
default y if MACH_SUN6I || MACH_SUN8I
Though I see that both multi_v7_defconfig and sunxi_defconfig explicitly
set this, so this wouldn't be needed?
Cheers,
Andre.
^ 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