* ath9k ARMv7 OOPS in v4.8.6, v4.2.8
From: Jason Cooper @ 2016-11-24 12:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161123214053.GJ2799@io.lakedaemon.net>
All,
On Wed, Nov 23, 2016 at 09:40:53PM +0000, Jason Cooper wrote:
> I'm running with ATH9K_DEBUGFS=y now. If it goes a couple of days
> without crashing, I'll gin up a patch.
Well, it survived overnight, which it's never done before. :-) I'm
testing the relay_open() NULL patch now.
thx,
Jason.
^ permalink raw reply
* [PATCH 6/10] mmc: sdhci-xenon: Add Marvell Xenon SDHC core functionality
From: Ziji Hu @ 2016-11-24 12:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAPDyKFpqPSqiEi=0UW5LoZmy+y-KHm9nbcGKBSy3RzchdLU9cA@mail.gmail.com>
Hi Ulf,
On 2016/11/24 18:43, Ulf Hansson wrote:
> On 31 October 2016 at 12:09, Gregory CLEMENT
> <gregory.clement@free-electrons.com> wrote:
>> From: Ziji Hu <huziji@marvell.com>
>>
<snip>
>> +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().
>
Sure. Will merge it back to __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?
>
In my very own opinion, SD Spec doesn't specify whether SDCLK should be
enabled or not during power setting.
Enabling SDCLK might be a special condition only required by our SDHC.
I try to avoid breaking other vendors' SDHC functionality
if their SDHCs require SDCLK disabled.
Thus I prefer to keep it inside our SDHC driver.
>> + * 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!?
>
It is our SDHC specific preparation prior to tuning, rather than a
standard step in spec.
Thus I leave it inside our driver.
>> +
>> + 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.
>
This field indicates SDIO card and controls async interrupt feature
of SDIO in our SDHC.
This async interrupt feature is enabled when SDIO card is inserted.
It should be disabled if SD card is inserted instead.
>> + 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().
>
Sure. Will try to use DT instead.
>> + }
>> + } 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!?
>
Yes. One of the reasons is to provide eMMC specific voltage setting.
But in my very own opinion, it should be irrelevant to voltage level.
The eMMC voltage setting on our SDHC is different from SD/SDIO voltage switch.
It will become more complex with different SOC implementation details.
Unfortunately, MMC driver cannot determine the card type yet when eMMC voltage
setting should be executed.
Thus an flag is required here to tell driver to execute eMMC voltage setting.
Besides, additional eMMC specific settings might be implemented in future, besides
voltage setting. Most of them should be completed before MMC driver recognizes the
card type. Thus I have to keep this flag to indicate current SDHC is for eMMC.
>> + 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.
>
Yes. You are correct.
>> +
>> + 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?
>
During card initialization, our SDHC PHY setting depends on current
timing and SDCLK frequency.
priv->timing and priv->clock will be used in PHY setting later.
It can be considered as a clean-up.
Anyway, it does look ugly. I will improve them after our PHY setting
passes your review.
>> +
>> + 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?
>
Yes, of course.
>> +
>> + /*
>> + * 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.
>
OK.
>> +
>> + err = xenon_probe_dt(pdev);
>> + if (err)
>> + goto err_clk;
>> +
>> + err = xenon_slot_probe(host);
>> + if (err)
>> + goto err_clk;
>> +
>> + err = sdhci_add_host(host);
>> + if (err)
>> + goto remove_slot;
>> +
>> + return 0;
>> +
>> +remove_slot:
>> + xenon_slot_remove(host);
>> +err_clk:
>> + clk_disable_unprepare(pltfm_host->clk);
>> + if (!IS_ERR(axi_clk))
>> + clk_disable_unprepare(axi_clk);
>> +free_pltfm:
>> + sdhci_pltfm_free(pdev);
>> + return err;
>> +}
>> +
>> +static int sdhci_xenon_remove(struct platform_device *pdev)
>> +{
>> + struct sdhci_host *host = platform_get_drvdata(pdev);
>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>> + int dead = (readl(host->ioaddr + SDHCI_INT_STATUS) == 0xFFFFFFFF);
>> +
>> + xenon_slot_remove(host);
>> +
>> + sdhci_remove_host(host, dead);
>> +
>> + clk_disable_unprepare(pltfm_host->clk);
>> + clk_disable_unprepare(priv->axi_clk);
>> +
>> + sdhci_pltfm_free(pdev);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id sdhci_xenon_dt_ids[] = {
>> + { .compatible = "marvell,xenon-sdhci",},
>> + { .compatible = "marvell,armada-3700-sdhci",},
>> + {}
>> +};
>> +MODULE_DEVICE_TABLE(of, sdhci_xenon_dt_ids);
>> +
>> +static struct platform_driver sdhci_xenon_driver = {
>> + .driver = {
>> + .name = "xenon-sdhci",
>> + .of_match_table = sdhci_xenon_dt_ids,
>> + .pm = &sdhci_pltfm_pmops,
>> + },
>> + .probe = sdhci_xenon_probe,
>> + .remove = sdhci_xenon_remove,
>> +};
>> +
>> +module_platform_driver(sdhci_xenon_driver);
>> +
>> +MODULE_DESCRIPTION("SDHCI platform driver for Marvell Xenon SDHC");
>> +MODULE_AUTHOR("Hu Ziji <huziji@marvell.com>");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/drivers/mmc/host/sdhci-xenon.h b/drivers/mmc/host/sdhci-xenon.h
>> new file mode 100644
>> index 000000000000..4601d0a4b22f
>> --- /dev/null
>> +++ b/drivers/mmc/host/sdhci-xenon.h
>
> I don't think you need a specific header for this, let's instead just
> put everthing in the c-file.
>
Some definitions inside this file will also be referred in PHY setting in
sdhci-xenon-phy.c.
Thus I put all the definitions together into a header file.
>> @@ -0,0 +1,142 @@
>> +/*
>> + * Copyright (C) 2016 Marvell, All Rights Reserved.
>> + *
>> + * Author: Hu Ziji <huziji@marvell.com>
>> + * Date: 2016-8-24
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation version 2.
>> + */
>> +#ifndef SDHCI_XENON_H_
>> +#define SDHCI_XENON_H_
>> +
>> +#include <linux/clk.h>
>> +#include <linux/mmc/card.h>
>> +#include <linux/of.h>
>> +#include "sdhci.h"
>> +
>> +/* Register Offset of SD Host Controller SOCP self-defined register */
>> +#define SDHC_SYS_CFG_INFO 0x0104
>> +#define SLOT_TYPE_SDIO_SHIFT 24
>> +#define SLOT_TYPE_EMMC_MASK 0xFF
>> +#define SLOT_TYPE_EMMC_SHIFT 16
>> +#define SLOT_TYPE_SD_SDIO_MMC_MASK 0xFF
>> +#define SLOT_TYPE_SD_SDIO_MMC_SHIFT 8
>> +#define NR_SUPPORTED_SLOT_MASK 0x7
>> +
>> +#define SDHC_SYS_OP_CTRL 0x0108
>> +#define AUTO_CLKGATE_DISABLE_MASK BIT(20)
>> +#define SDCLK_IDLEOFF_ENABLE_SHIFT 8
>> +#define SLOT_ENABLE_SHIFT 0
>> +
>> +#define SDHC_SYS_EXT_OP_CTRL 0x010C
>> +#define MASK_CMD_CONFLICT_ERROR BIT(8)
>> +
>> +#define SDHC_SLOT_OP_STATUS_CTRL 0x0128
>> +#define DELAY_90_DEGREE_MASK_EMMC5 BIT(7)
>> +#define DELAY_90_DEGREE_SHIFT_EMMC5 7
>> +#define EMMC_5_0_PHY_FIXED_DELAY_MASK 0x7F
>> +#define EMMC_PHY_FIXED_DELAY_MASK 0xFF
>> +#define EMMC_PHY_FIXED_DELAY_WINDOW_MIN (EMMC_PHY_FIXED_DELAY_MASK >> 3)
>> +#define SDH_PHY_FIXED_DELAY_MASK 0x1FF
>> +#define SDH_PHY_FIXED_DELAY_WINDOW_MIN (SDH_PHY_FIXED_DELAY_MASK >> 4)
>> +
>> +#define TUN_CONSECUTIVE_TIMES_SHIFT 16
>> +#define TUN_CONSECUTIVE_TIMES_MASK 0x7
>> +#define TUN_CONSECUTIVE_TIMES 0x4
>> +#define TUNING_STEP_SHIFT 12
>> +#define TUNING_STEP_MASK 0xF
>> +#define TUNING_STEP_DIVIDER BIT(6)
>> +
>> +#define FORCE_SEL_INVERSE_CLK_SHIFT 11
>> +
>> +#define SDHC_SLOT_EMMC_CTRL 0x0130
>> +#define ENABLE_DATA_STROBE BIT(24)
>> +#define SET_EMMC_RSTN BIT(16)
>> +#define DISABLE_RD_DATA_CRC BIT(14)
>> +#define DISABLE_CRC_STAT_TOKEN BIT(13)
>> +#define EMMC_VCCQ_MASK 0x3
>> +#define EMMC_VCCQ_1_8V 0x1
>> +#define EMMC_VCCQ_3_3V 0x3
>> +
>> +#define SDHC_SLOT_RETUNING_REQ_CTRL 0x0144
>> +/* retuning compatible */
>> +#define RETUNING_COMPATIBLE 0x1
>> +
>> +#define SDHC_SLOT_EXT_PRESENT_STATE 0x014C
>> +#define LOCK_STATE 0x1
>> +
>> +#define SDHC_SLOT_DLL_CUR_DLY_VAL 0x0150
>> +
>> +/* Tuning Parameter */
>> +#define TMR_RETUN_NO_PRESENT 0xF
>> +#define DEF_TUNING_COUNT 0x9
>> +
>> +#define MMC_TIMING_FAKE 0xFF
>> +
>> +#define DEFAULT_SDCLK_FREQ (400000)
>> +
>> +/* Xenon specific Mode Select value */
>> +#define XENON_SDHCI_CTRL_HS200 0x5
>> +#define XENON_SDHCI_CTRL_HS400 0x6
>
> For all defines above:
>
> All these defines needs some *SDHCI* prefix. Can you please update that.
Sure. Will add prefix for all of them.
>
>> +
>> +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.
>
The above two variables will be used in PHY setting
in sdhci-xenon-phy.c.
Could you please help review them in next patch?
>> + unsigned char tuning_count;
>> + unsigned int clock;
>
> "clock" isn't used, please remove.
>
It will be accessed in PHY setting in sdhci-xenon-phy.c.
Could you please help review it in next patch?
>> + 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.
>
This varible will be accessed in PHY setting in sdhci-xenon-phy.c.
I would like to discuss about it in PHY file. Could you please help
review it in next patch?
Thank you.
Best regards,
Hu Ziji
>> +};
>> +
>> +static inline int enable_xenon_internal_clk(struct sdhci_host *host)
>> +{
>> + u32 reg;
>> + u8 timeout;
>> +
>> + reg = sdhci_readl(host, SDHCI_CLOCK_CONTROL);
>> + reg |= SDHCI_CLOCK_INT_EN;
>> + sdhci_writel(host, reg, SDHCI_CLOCK_CONTROL);
>> + /* Wait max 20 ms */
>> + timeout = 20;
>> + while (!((reg = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
>> + & SDHCI_CLOCK_INT_STABLE)) {
>> + if (timeout == 0) {
>> + pr_err("%s: Internal clock never stabilised.\n",
>> + mmc_hostname(host->mmc));
>> + return -ETIMEDOUT;
>> + }
>> + timeout--;
>> + mdelay(1);
>> + }
>> +
>> + return 0;
>> +}
>> +#endif
>> --
>> git-series 0.8.10
>
> Kind regards
> Uffe
>
^ permalink raw reply
* [PATCH 0/2] OF phandle nexus support + GPIO nexus
From: Pantelis Antoniou @ 2016-11-24 12:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161124102529.20212-1-stephen.boyd@linaro.org>
Hi Stephen,
> On Nov 24, 2016, at 12:25 , Stephen Boyd <stephen.boyd@linaro.org> wrote:
>
> 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.
>
Excellent. It was about time this happened.
> 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
>
Comments inline?
Regards
? Pantelis
^ permalink raw reply
* [PATCH 7/9] clocksource/drivers/rockchip_timer: implement clocksource timer
From: Alexander Kochetkov @ 2016-11-24 13:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1800621.EhepfMxccR@diego>
Hello Heiko!
> 24 ????. 2016 ?., ? 15:17, Heiko St?bner <heiko@sntech.de> ???????(?):
>
> In your dts-patch you reuse the rk3288-timer compatible value, which is also
> non-ideal.
rockchip,rk-timer.txt states what I should use rockchip,rk3288-timer for rk3188 timers:
Required properties:
- compatible: shall be one of:
"rockchip,rk3288-timer" - for rk3066, rk3036, rk3188, rk322x, rk3288, rk3368
Should I add "rockchip,rk3188-timer? (or better "rockchip,rk3036-timer?) ? Or may be second
approach should be used?
> What you may want to do is introduce a rockchip,rk3188-timer compatible and
> then make the timer-driver act accordingly, as you then know you are on a
> rk3188-board ... see drivers attaching specific structs to the of_device_id
> entries. From the documentation
May be it is better to prepend compatible string with "rockchip,rk3188-timer? in the dts
file only? elinux[1] recommend build compatible string from "exact device? string and
?other devices? that the device is compatible with.
As ?other devices? string defined already, I use it.
[1] http://elinux.org/Device_Tree_Usage#Understanding_the_compatible_Property
timer0: timer at 20038000 {
compatible = "rockchip,rk3188-timer", "rockchip,rk3288-timer?;
?
};
Just found what rk3036 already declared such way:
rk3036.dtsi: timer: timer at 20044000 {
rk3036.dtsi: compatible = "rockchip,rk3036-timer", "rockchip,rk3288-timer?;
>
> it also shouldn't really matter which timer
> you use as clocksource, as on the rk3188 it seems all of them act the same way
> (except timer3 being always on).
You are right.
I sent dts file with general timer settings, without clockevent enabled, so one could pick
up timer and setup it in the board dts (radxarock, for example) like:
&timer0 {
rockchip,clocksource;
status = ?okay?;
};
> When touching devicetree-properties, please also adapt the binding document
> Documentation/devicetree/bindings/timer,rockchip,rk-timer.txt
> in this case and also include the devicetree maintainers.
If the patch[2] ok, I'll resend it and include devicetree maintainers.
[2] http://lists.infradead.org/pipermail/linux-rockchip/2016-November/013148.html
Regards,
Alexander.
^ permalink raw reply
* TDA998x crash on HDLCD probe failure
From: Robin Murphy @ 2016-11-24 13:18 UTC (permalink / raw)
To: linux-arm-kernel
Hi Liviu, Russell,
I'd been meaning to try digging into this if it hadn't gone away since I
first noticed it, but I don't really have the time and it still happens
with 4.9-rc and today's -next. Representative splat below, but in
summary what happens is that if the HDLCD fails to probe, the TDA998x
connector seems to get cleaned up twice, resulting in a NULL dereference
the second time. I got as far as sketching out the following flow from a
debug session (on the same 4.8-rc2 kernel), but I don't know nearly
enough to tell which driver is at fault:
hdlcd_drm_bind
-> drm_fbdev_cma_init (fails)
...
-> drm_mode_config_cleanup
...
-> drm_connector_cleanup
-> component_unbind_all
...
-> tda998x_unbind
-> drm_connector_cleanup (NULL connector)
It's easily reproduced on Juno by booting arm64 defconfig with
CONFIG_CMA_SIZE_MBYTES=1 and a sufficiently large monitor connected to
warrant a >1MB framebuffer.
Robin.
[ 4.107349] hdlcd 7ff60000.hdlcd: failed to allocate buffer with size
7680000
[ 4.114459] hdlcd 7ff60000.hdlcd: Failed to set initial hw configuration.
[ 4.121888] Unable to handle kernel NULL pointer dereference at
virtual address 00000000
[ 4.129951] pgd = ffffff80091e0000
[ 4.133345] [00000000] *pgd=00000009ffffe003, *pud=00000009ffffe003,
*pmd=0000000000000000
[ 4.141613] Internal error: Oops: 96000005 [#1] PREEMPT SMP
[ 4.147144] Modules linked in:
[ 4.150188] CPU: 0 PID: 122 Comm: kworker/u12:2 Not tainted
4.8.0-rc2+ #989
[ 4.157097] Hardware name: ARM Juno development board (r1) (DT)
[ 4.162981] Workqueue: deferwq deferred_probe_work_func
[ 4.168173] task: ffffffc975d93200 task.stack: ffffffc975dac000
[ 4.174055] PC is at drm_connector_cleanup+0x58/0x1c0
[ 4.179074] LR is at tda998x_unbind+0x24/0x40
[ 4.183401] pc : [<ffffff80084c46f0>] lr : [<ffffff800850414c>]
pstate: 00000045
[ 4.190750] sp : ffffffc975dafa10
[ 4.194041] x29: ffffffc975dafa10 x28: ffffffc9768152a8
[ 4.199325] x27: ffffffc97ff46450 x26: ffffff8008d99000
[ 4.204608] x25: dead000000000100 x24: dead000000000200
[ 4.209891] x23: ffffffc976bf91e8 x22: 0000000000000000
[ 4.215172] x21: ffffffc976bf9170 x20: ffffffc976bf9170
[ 4.220454] x19: ffffffc976bf9018 x18: 0000000000000000
[ 4.225737] x17: 0000000074ce71ee x16: 000000008ff5d35f
[ 4.231019] x15: ffffffc97681e91c x14: ffffffffffffffff
[ 4.236301] x13: ffffffc97681e185 x12: 0000000000000038
[ 4.241583] x11: 0101010101010101 x10: 0000000000000000
[ 4.246866] x9 : 0000000040000000 x8 : 0000000000210d00
[ 4.252148] x7 : ffffffc97fea8c00 x6 : 000000000000001b
[ 4.257430] x5 : ffffff80084b7b8c x4 : 0000000000000080
[ 4.262712] x3 : ffffff8008504128 x2 : ffffffc975df3800
[ 4.267993] x1 : 0000000000000000 x0 : 0000000000000000
[ 4.273274]
[ 4.274759] Process kworker/u12:2 (pid: 122, stack limit =
0xffffffc975dac020)
[ 4.281938] Stack: (0xffffffc975dafa10 to 0xffffffc975db0000)
[ 4.285576] ata1: SATA link up 3.0 Gbps (SStatus 123 SControl 0)
[ 4.293602] fa00: ffffffc975dafa60
ffffff800850414c
[ 4.301386] fa20: ffffffc976bf9018 ffffffc976bf9170 0000000000000000
ffffffc975df3800
[ 4.309170] fa40: ffffffc975c1e1a0 ffffffc976b14400 ffffffc976b14410
ffffff8008518784
[ 4.316954] fa60: ffffffc975dafa80 ffffff8008507918 ffffffc976bdf080
ffffffffffffffd8
[ 4.324737] fa80: ffffffc975dafaa0 ffffff8008507a0c ffffffc975c1e180
ffffffc976b14400
[ 4.332521] faa0: ffffffc975dafae0 ffffff80084d5adc ffffffc975df3800
fffffffffffffff4
[ 4.340305] fac0: ffffffc976b14410 ffffffc975df3018 ffffffc975df3018
ffffff80084d5ab4
[ 4.348089] fae0: ffffffc975dafb30 ffffff8008507b58 ffffffc976bdf080
0000000000000028
[ 4.355873] fb00: ffffff8008d3d600 0000000000000001 ffffffc975c1e180
ffffffc976bdf080
[ 4.363657] fb20: ffffffc975c1e098 ffffff80084d5428 ffffffc975dafb90
ffffff8008507c50
[ 4.371441] fb40: ffffffc975c1e180 ffffff8008d3d5f0 ffffffc976bdf080
ffffffc976b57000
[ 4.379225] fb60: ffffff8008d3d000 ffffffc976815000 ffffff8008d926c6
ffffffc976815020
[ 4.387009] fb80: ffffffc976815078 ffffffc9768152a8 ffffffc975dafbd0
ffffff8008504b90
[ 4.394793] fba0: ffffff8008d3d4e8 ffffffc976b57020 ffffffc976b57004
ffffffc976b57000
[ 4.402577] fbc0: ffffff8008504b78 ffffff80086bf8cc ffffffc975dafbe0
ffffff80086bf914
[ 4.410360] fbe0: ffffffc975dafc20 ffffff800850d094 ffffffc976b57020
ffffff8008dc4000
[ 4.418144] fc00: 0000000000000000 ffffff8008d3d448 0000000000000004
ffffff8008dc4000
[ 4.425928] fc20: ffffffc975dafc60 ffffff800850d28c ffffff8008d3d448
ffffffc975dafd00
[ 4.433711] fc40: ffffffc976b57020 0000000000000001 ffffffc975e3b400
ffffff800850b114
[ 4.441495] fc60: ffffffc975dafc90 ffffff800850b108 0000000000000000
ffffffc975dafd00
[ 4.449279] fc80: ffffff800850d1f0 0000000000000001 ffffffc975dafcd0
ffffff800850cd64
[ 4.457062] fca0: ffffffc976b57020 ffffffc976b57080 ffffff8008d584f0
ffffffc976b57080
[ 4.464846] fcc0: ffffffc976af38d0 ffffffc975c20168 ffffffc975dafd10
ffffff800850d338
[ 4.472630] fce0: ffffffc976b57020 ffffffc976b57020 ffffff8008d584f0
ffffff8008d3d000
[ 4.480414] fd00: ffffffc976b57020 0000000000000001 ffffffc975dafd20
ffffff800850c124
[ 4.488198] fd20: ffffffc975dafd50 ffffff800850c5b0 ffffffc976b57020
ffffff8008d3d848
[ 4.495982] fd40: ffffff8008d3d820 ffffff800850c5a8 ffffffc975dafd80
ffffff80080d2998
[ 4.503765] fd60: 0000000000000000 ffffffc976bd4500 ffffff8008d3d880
0000000000000000
[ 4.511549] fd80: ffffffc975dafdc0 ffffff80080d2c40 ffffffc976bd4500
ffffffc976815000
[ 4.519333] fda0: ffffffc976bd4530 ffffffc976815020 ffffff8008ce5000
ffffffc975dac000
[ 4.527117] fdc0: ffffffc975dafe20 ffffff80080d8918 ffffffc976bd3880
ffffff8008d9cb08
[ 4.534900] fde0: ffffff8008ace5f0 ffffffc976bd4500 ffffff80080d2bf8
0000000000000000
[ 4.542683] fe00: 0000000000000000 0000000000000000 0000000000000000
0000000000000000
[ 4.550467] fe20: 0000000000000000 ffffff8008082e90 ffffff80080d8848
ffffffc976bd3880
[ 4.558250] fe40: 0000000000000000 0000000000000000 0000000000000000
0000d00420000400
[ 4.566034] fe60: ffffffc975dafea0 0000000000000000 ffffff80080d8848
ffffffc976bd4500
[ 4.573817] fe80: 0000000000000000 0000000000000000 ffffffc975dafe90
ffffffc975dafe90
[ 4.581602] fea0: 0000000000000000 ffffff8000000000 ffffffc975dafeb0
ffffffc975dafeb0
[ 4.589384] fec0: 0000000000000000 0000000000000000 0000000000000000
0000000000000000
[ 4.597168] fee0: 0000000000000000 0000000000000000 0000000000000000
0000000000000000
[ 4.604951] ff00: 0000000000000000 0000000000000000 0000000000000000
0000000000000000
[ 4.612734] ff20: 0000000000000000 0000000000000000 0000000000000000
0000000000000000
[ 4.620517] ff40: 0000000000000000 0000000000000000 0000000000000000
0000000000000000
[ 4.628299] ff60: 0000000000000000 0000000000000000 0000000000000000
0000000000000000
[ 4.636082] ff80: 0000000000000000 0000000000000000 0000000000000000
0000000000000000
[ 4.643865] ffa0: 0000000000000000 0000000000000000 0000000000000000
0000000000000000
[ 4.651648] ffc0: 0000000000000000 0000000000000005 0000000000000000
0000000000000000
[ 4.659431] ffe0: 0000000000000000 0000000000000000 100a0c0062800800
42200c0000000c00
[ 4.667210] Call trace:
[ 4.669643] Exception stack(0xffffffc975daf840 to 0xffffffc975daf970)
[ 4.676040] f840: ffffffc976bf9018 0000008000000000 ffffffc975dafa10
ffffff80084c46f0
[ 4.683823] f860: 0000000000000000 0000000000000000 00000001800f000e
0000000000000001
[ 4.691607] f880: ffffffc9769ef2a0 00000001000f000f ffffffbf25da7a00
ffffffc976803100
[ 4.699390] f8a0: ffffffc975daf9b0 ffffff80081a899c ffffffc976803100
ffffff80083446b4
[ 4.707174] f8c0: ffffffbf25da7a00 ffffffc975dac000 ffffffc9769ef2a0
000000000003c380
[ 4.714958] f8e0: 0000000000000000 0000000000000000 ffffffc975df3800
ffffff8008504128
[ 4.722741] f900: 0000000000000080 ffffff80084b7b8c 000000000000001b
ffffffc97fea8c00
[ 4.730524] f920: 0000000000210d00 0000000040000000 0000000000000000
0101010101010101
[ 4.738308] f940: 0000000000000038 ffffffc97681e185 ffffffffffffffff
ffffffc97681e91c
[ 4.746090] f960: 000000008ff5d35f 0000000074ce71ee
[ 4.750937] [<ffffff80084c46f0>] drm_connector_cleanup+0x58/0x1c0
[ 4.756990] [<ffffff800850414c>] tda998x_unbind+0x24/0x40
[ 4.762354] [<ffffff8008507918>] component_unbind.isra.4+0x28/0x50
[ 4.768492] [<ffffff8008507a0c>] component_unbind_all+0xcc/0xd8
[ 4.774373] [<ffffff80084d5adc>] hdlcd_drm_bind+0x234/0x418
[ 4.779909] [<ffffff8008507b58>] try_to_bring_up_master+0x140/0x1a0
[ 4.786133] [<ffffff8008507c50>] component_add+0x98/0x170
[ 4.791496] [<ffffff8008504b90>] tda998x_probe+0x18/0x20
[ 4.796774] [<ffffff80086bf914>] i2c_device_probe+0x164/0x258
[ 4.802481] [<ffffff800850d094>] driver_probe_device+0x204/0x2b0
[ 4.808447] [<ffffff800850d28c>] __device_attach_driver+0x9c/0xf8
[ 4.814498] [<ffffff800850b108>] bus_for_each_drv+0x58/0x98
[ 4.820033] [<ffffff800850cd64>] __device_attach+0xc4/0x138
[ 4.825567] [<ffffff800850d338>] device_initial_probe+0x10/0x18
[ 4.831446] [<ffffff800850c124>] bus_probe_device+0x94/0xa0
[ 4.836981] [<ffffff800850c5b0>] deferred_probe_work_func+0x78/0xb0
[ 4.843207] [<ffffff80080d2998>] process_one_work+0x118/0x378
[ 4.848914] [<ffffff80080d2c40>] worker_thread+0x48/0x498
[ 4.854276] [<ffffff80080d8918>] kthread+0xd0/0xe8
[ 4.859036] [<ffffff8008082e90>] ret_from_fork+0x10/0x40
[ 4.864314] Code: f2fbd5b9 f2fbd5b8 f8478ee0 eb17001f (f9400013)
[ 4.870472] ---[ end trace a643cfe4ce1d838b ]---
^ permalink raw reply
* [PATCH 7/9] clocksource/drivers/rockchip_timer: implement clocksource timer
From: Heiko Stübner @ 2016-11-24 13:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <0E972AC9-1132-4D0B-BF25-918BC1AA1A49@gmail.com>
Hi Alexander,
Am Donnerstag, 24. November 2016, 16:05:55 schrieb Alexander Kochetkov:
> Hello Heiko!
>
> > 24 ????. 2016 ?., ? 15:17, Heiko St?bner <heiko@sntech.de> ???????(?):
> >
> > In your dts-patch you reuse the rk3288-timer compatible value, which is
> > also non-ideal.
>
> rockchip,rk-timer.txt states what I should use rockchip,rk3288-timer for
> rk3188 timers:
>
> Required properties:
> - compatible: shall be one of:
> "rockchip,rk3288-timer" - for rk3066, rk3036, rk3188, rk322x, rk3288,
> rk3368
>
> Should I add "rockchip,rk3188-timer? (or better "rockchip,rk3036-timer?) ?
> Or may be second approach should be used?
>
> > What you may want to do is introduce a rockchip,rk3188-timer compatible
> > and
> > then make the timer-driver act accordingly, as you then know you are on a
> > rk3188-board ... see drivers attaching specific structs to the
> > of_device_id
> > entries. From the documentation
>
> May be it is better to prepend compatible string with
> "rockchip,rk3188-timer? in the dts file only? elinux[1] recommend build
> compatible string from "exact device? string and ?other devices? that the
> device is compatible with.
>
> As ?other devices? string defined already, I use it.
>
> [1]
> http://elinux.org/Device_Tree_Usage#Understanding_the_compatible_Property
>
> timer0: timer at 20038000 {
> compatible = "rockchip,rk3188-timer", "rockchip,rk3288-timer?;
> ?
> };
>
> Just found what rk3036 already declared such way:
>
> rk3036.dtsi: timer: timer at 20044000 {
> rk3036.dtsi: compatible = "rockchip,rk3036-timer", "rockchip,rk3288-
timer?;
correct, use both but also update the binding, like
mmc/rockchip-dw-mshc.txt does.
> > it also shouldn't really matter which timer
> > you use as clocksource, as on the rk3188 it seems all of them act the same
> > way (except timer3 being always on).
>
> You are right.
>
> I sent dts file with general timer settings, without clockevent enabled, so
> one could pick up timer and setup it in the board dts (radxarock, for
> example) like:
>
> &timer0 {
> rockchip,clocksource;
> status = ?okay?;
> };
what I actually meant was that the driver could also recognize the rk3188-
timer compatible as "we need a clocksource" and it shouldn't matter which
timer actually gets used for this.
> > When touching devicetree-properties, please also adapt the binding
> > document
> >
> > Documentation/devicetree/bindings/timer,rockchip,rk-timer.txt
> >
> > in this case and also include the devicetree maintainers.
>
> If the patch[2] ok, I'll resend it and include devicetree maintainers.
>
> [2]
> http://lists.infradead.org/pipermail/linux-rockchip/2016-November/013148.ht
> ml
Only devicetree people can really tell you if that is ok :-) .
The devicetree is supposed to be a hardware-description and implementation-
independent, so rockchip,clocksource sounds pretty much like linux-specific
configuration things leaking into the devicetree.
Heiko
^ permalink raw reply
* TDA998x crash on HDLCD probe failure
From: Russell King - ARM Linux @ 2016-11-24 13:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <8281efe7-bac7-8e35-5784-e05cf0dc7eab@arm.com>
On Thu, Nov 24, 2016 at 01:18:39PM +0000, Robin Murphy wrote:
> Hi Liviu, Russell,
>
> I'd been meaning to try digging into this if it hadn't gone away since I
> first noticed it, but I don't really have the time and it still happens
> with 4.9-rc and today's -next. Representative splat below, but in
> summary what happens is that if the HDLCD fails to probe, the TDA998x
> connector seems to get cleaned up twice, resulting in a NULL dereference
> the second time. I got as far as sketching out the following flow from a
> debug session (on the same 4.8-rc2 kernel), but I don't know nearly
> enough to tell which driver is at fault:
>
> hdlcd_drm_bind
> -> drm_fbdev_cma_init (fails)
> ...
> -> drm_mode_config_cleanup
> ...
> -> drm_connector_cleanup
> -> component_unbind_all
> ...
> -> tda998x_unbind
> -> drm_connector_cleanup (NULL connector)
>
> It's easily reproduced on Juno by booting arm64 defconfig with
> CONFIG_CMA_SIZE_MBYTES=1 and a sufficiently large monitor connected to
> warrant a >1MB framebuffer.
It looks to me like a hdlcd bug.
The probe path operates in this order:
- allocates hdlcd - 1
- allocates drm device - 2
- drm_mode_config_init - 3
- hdlcd_load - 4
- binds all components - 5
- enables runtime PM - 6
- drm_vblank_init - 7
- drm_mode_config_reset - 8
- drm_kms_helper_poll_init - 9
- drm_fbdev_cma_init - 10
- drm_dev_register - 11
However, the cleanup operates in this order:
- drm_fbdev_cma_fini - undoes 10
- drm_kms_helper_poll_fini - undoes 9
- drm_mode_config_cleanup - undoes 3
- drm_vblank_cleanup - undoes 7
- pm_runtime_disable - undoes 6
- component_unbind_all - undoes 5
- drm_irq_uninstall - undoes 4
- of_reserved_mem_device_release - undoes other half of 4
- drm_dev_unref - undoes 2
Spot the step which is out of the correct order - drm_mode_config_cleanup()
is misplaced - it's reversing the actions of drm_mode_config_init(), not
drm_mode_config_reset().
So, drm_mode_config_cleanup() should be much later, after step 4 has
been undone, otherwise there are paths that leave various DRM objects
(created by drm_mode_create_standard_properties()) referenced, and
will cause problems exactly like you're seeing here.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* [PATCH 6/10] mmc: sdhci-xenon: Add Marvell Xenon SDHC core functionality
From: Ulf Hansson @ 2016-11-24 13:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <dd230463-04f6-df31-7056-1a185eb6cfc7@marvell.com>
On 24 November 2016 at 13:41, Ziji Hu <huziji@marvell.com> wrote:
> Hi Ulf,
>
> On 2016/11/24 18:43, Ulf Hansson wrote:
>> On 31 October 2016 at 12:09, Gregory CLEMENT
>> <gregory.clement@free-electrons.com> wrote:
>>> From: Ziji Hu <huziji@marvell.com>
>>>
> <snip>
>>> +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().
>>
> Sure. Will merge it back to __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?
>>
> In my very own opinion, SD Spec doesn't specify whether SDCLK should be
> enabled or not during power setting.
> Enabling SDCLK might be a special condition only required by our SDHC.
> I try to avoid breaking other vendors' SDHC functionality
> if their SDHCs require SDCLK disabled.
> Thus I prefer to keep it inside our SDHC driver.
I let Adrian comment on this.
For sure we should avoid breaking other sdhci variant, but on the
other hand *if* the generic code is wrong we should fix it!
>
>>> + * 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!?
>>
> It is our SDHC specific preparation prior to tuning, rather than a
> standard step in spec.
> Thus I leave it inside our driver.
My point is that this isn't the purpose of ->init_card(). thus you are
abusing it.
Try to make it work in another way, please. I think you can.
>
>>> +
>>> + 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.
>>
> This field indicates SDIO card and controls async interrupt feature
> of SDIO in our SDHC.
> This async interrupt feature is enabled when SDIO card is inserted.
> It should be disabled if SD card is inserted instead.
What do you mean by SDIO async interupts? Are you talking about SDIO
irqs on DAT1 line?
Those is supposed to be enabled when someone explicitly requests them,
not when the card is inserted.
In other words when an SDIO func driver have called sdio_claim_irq().
Moreover, we have ->enable_sdio_irq() ops that deals with this.
[...]
>>> +
>>> + /*
>>> + * 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!?
>>
> Yes. One of the reasons is to provide eMMC specific voltage setting.
> But in my very own opinion, it should be irrelevant to voltage level.
> The eMMC voltage setting on our SDHC is different from SD/SDIO voltage switch.
> It will become more complex with different SOC implementation details.
Got it. Although I think we can cope with that fine just by using the
different SD/eMMC speed modes settings defined in DT (or from the
SDHCI caps register)
> Unfortunately, MMC driver cannot determine the card type yet when eMMC voltage
> setting should be executed.
> Thus an flag is required here to tell driver to execute eMMC voltage setting.
>
> Besides, additional eMMC specific settings might be implemented in future, besides
> voltage setting. Most of them should be completed before MMC driver recognizes the
> card type. Thus I have to keep this flag to indicate current SDHC is for eMMC.
I doubt you will need a generic "eMMC" flag, but let's see when we go forward.
Currently it's clear you don't need such a flag, so I will submit a
change adding a DT binding for "mmc-ddr-3_3v" then we can take it from
there, to see if it suits your needs.
[...]
Kind regards
Uffe
^ permalink raw reply
* [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell Xenon SDHC
From: Ziji Hu @ 2016-11-24 13:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAPDyKFpkcoVMKbVOwjX1WDyNgc1vvUX60D6XRX6=YHGvkvHvnA@mail.gmail.com>
Hi Ulf,
Thanks a lot for the review.
On 2016/11/24 19:37, Ulf Hansson wrote:
> On 31 October 2016 at 12:09, Gregory CLEMENT
> <gregory.clement@free-electrons.com> wrote:
>> From: Ziji Hu <huziji@marvell.com>
>>
>> Marvell Xenon eMMC/SD/SDIO Host Controller contains PHY.
>> Three types of PHYs are supported.
>>
>> Add support to multiple types of PHYs init and configuration.
>> Add register definitions of PHYs.
>>
>> Signed-off-by: Hu Ziji <huziji@marvell.com>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>> ---
>> MAINTAINERS | 2 +-
>> drivers/mmc/host/Makefile | 2 +-
>> drivers/mmc/host/sdhci-xenon-phy.c | 1181 +++++++++++++++++++++++++++++-
>> drivers/mmc/host/sdhci-xenon-phy.h | 157 ++++-
>> drivers/mmc/host/sdhci-xenon.c | 4 +-
>> drivers/mmc/host/sdhci-xenon.h | 17 +-
>> 6 files changed, 1361 insertions(+), 2 deletions(-)
>> create mode 100644 drivers/mmc/host/sdhci-xenon-phy.c
>> create mode 100644 drivers/mmc/host/sdhci-xenon-phy.h
>
> Can you please consider to split this up somehow!? It would make it
> easier to review...
>
Sure. I will try to split them into smaller pieces.
> Anyway, allow me to provide some initial feedback, particularly around
> those things that Adrian and you requested for my input.
>
> [...]
>
>>
>> +
>> +static int __xenon_emmc_delay_adj_test(struct mmc_card *card)
>> +{
>> + int err;
>> + u8 *ext_csd = NULL;
>> +
>> + err = mmc_get_ext_csd(card, &ext_csd);
>> + kfree(ext_csd);
>
> Why do you read the ext csd here?
>
I would like to simply introduce the PHY setting of our SDHC.
The target of the PHY setting is to achieve a perfect sampling
point for transfers, during card initialization.
For HS200/HS400/SDR104 whose SDCLK is more than 50MHz, SDHC HW
will search for this sampling point with DLL's help.
For other speed mode whose SDLCK is less than or equals to 50MHz,
SW has to scan the PHY delay line to find out this perfect sampling
point. Our driver sends a command to verify a sampling point
in current environment.
As result, our SDHC driver has to implement the functionality to
send commands and check the results, in host layer.
If directly calling mmc_wait_for_cmd() is improper, could you please
give us some suggestions?
For eMMC, CMD8 is used to test current sampling point set in PHY.
>> +
>> + return err;
>> +}
>> +
>> +static int __xenon_sdio_delay_adj_test(struct mmc_card *card)
>> +{
>> + struct mmc_command cmd = {0};
>> + int err;
>> +
>> + cmd.opcode = SD_IO_RW_DIRECT;
>> + cmd.flags = MMC_RSP_R5 | MMC_CMD_AC;
>> +
>> + err = mmc_wait_for_cmd(card->host, &cmd, 0);
>> + if (err)
>> + return err;
>> +
>> + if (cmd.resp[0] & R5_ERROR)
>> + return -EIO;
>> + if (cmd.resp[0] & R5_FUNCTION_NUMBER)
>> + return -EINVAL;
>> + if (cmd.resp[0] & R5_OUT_OF_RANGE)
>> + return -ERANGE;
>> + return 0;
>
> No thanks! MMC/SD/SDIO protocol code belongs in the core.
>
For SDIO, SD_IO_RW_DIRECT command is sent to test current sampling point
in PHY.
Please help provide some suggestion to implement the command transfer.
>> +}
>> +
>> +static int __xenon_sd_delay_adj_test(struct mmc_card *card)
>> +{
>> + struct mmc_command cmd = {0};
>> + int err;
>> +
>> + cmd.opcode = MMC_SEND_STATUS;
>> + cmd.arg = card->rca << 16;
>> + cmd.flags = MMC_RSP_R1 | MMC_CMD_AC;
>> +
>> + err = mmc_wait_for_cmd(card->host, &cmd, 0);
>> + return err;
>
> No thanks! MMC/SD/SDIO protocol code belongs in the core.
>
>> +}
>> +
>
> [...]
>
>> +int xenon_phy_adj(struct sdhci_host *host, struct mmc_ios *ios)
>> +{
>> + struct mmc_host *mmc = host->mmc;
>> + struct mmc_card *card;
>> + int ret = 0;
>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>> +
>> + if (!host->clock) {
>> + priv->clock = 0;
>> + return 0;
>> + }
>> +
>> + /*
>> + * The timing, frequency or bus width is changed,
>> + * better to set eMMC PHY based on current setting
>> + * and adjust Xenon SDHC delay.
>> + */
>> + if ((host->clock == priv->clock) &&
>> + (ios->bus_width == priv->bus_width) &&
>> + (ios->timing == priv->timing))
>> + return 0;
>> +
>> + xenon_phy_set(host, ios->timing);
>> +
>> + /* Update the record */
>> + priv->bus_width = ios->bus_width;
>> + /* Temp stage from HS200 to HS400 */
>> + if (((priv->timing == MMC_TIMING_MMC_HS200) &&
>> + (ios->timing == MMC_TIMING_MMC_HS)) ||
>> + ((ios->timing == MMC_TIMING_MMC_HS) &&
>> + (priv->clock > host->clock))) {
>> + priv->timing = ios->timing;
>> + priv->clock = host->clock;
>> + return 0;
>> + }
>> + /*
>> + * Skip temp stages from HS400 t0 HS200:
>> + * from 200MHz to 52MHz in HS400
>> + * from HS400 to HS DDR in 52MHz
>> + * from HS DDR to HS in 52MHz
>> + * from HS to HS200 in 52MHz
>> + */
>> + if (((priv->timing == MMC_TIMING_MMC_HS400) &&
>> + ((host->clock == MMC_HIGH_52_MAX_DTR) ||
>> + (ios->timing == MMC_TIMING_MMC_DDR52))) ||
>> + ((priv->timing == MMC_TIMING_MMC_DDR52) &&
>> + (ios->timing == MMC_TIMING_MMC_HS)) ||
>> + ((ios->timing == MMC_TIMING_MMC_HS200) &&
>> + (ios->clock == MMC_HIGH_52_MAX_DTR))) {
>> + priv->timing = ios->timing;
>> + priv->clock = host->clock;
>> + return 0;
>> + }
>> + priv->timing = ios->timing;
>> + priv->clock = host->clock;
>> +
>> + /* Legacy mode is a special case */
>> + if (ios->timing == MMC_TIMING_LEGACY)
>> + return 0;
>> +
>> + if (mmc->card)
>> + card = mmc->card;
>> + else
>> + /*
>> + * Only valid during initialization
>> + * before mmc->card is set
>> + */
>> + card = priv->card_candidate;
>> + if (unlikely(!card)) {
>> + dev_warn(mmc_dev(mmc), "card is not present\n");
>> + return -EINVAL;
>> + }
>
> That your host need to hold a copy of the card pointer, tells me that
> something is not really correct.
>
> I might be wrong, if this turns out to be a special case, but I doubt
> it. Although, if it *is* a special such case, we shall most likely try
> to extend the the mmc core layer instead of adding all these hacks in
> your host driver.
>
This card pointer copies the temporary structure mmc_card
used in mmc_init_card(), mmc_sd_init_card() and mmc_sdio_init_card().
Since we call mmc_wait_for_cmd() to send test commands, we need a copy
of that temporary mmc_card here in our host driver.
During PHY setting in card initialization, mmc_host->card is not updated
yet with that temporary mmc_card. Thus we are not able to directly use
mmc_host->card. Instead, this card pointer is introduced to enable
mmc_wait_for_cmd().
If we can improve our host driver to send test commands without mmc_card,
this card pointer can be removed.
Could you please share your opinion please?
> [...]
>
> Another suggestion of a general improvement; could you perhaps try to
> add some brief information about what goes on in function headers.
> Perhaps that could help to more easily understand things.
>
Sorry about any inconvenience. Most of the functions here are our host specific.
It is really difficult to understand them without proper comment.
I will add more information.
Thank you.
Best regards,
Hu Ziji
> Kind regards
> Uffe
>
^ permalink raw reply
* [PATCH 0/9] arm64: Expose CPUID registers via emulation
From: Suzuki K Poulose @ 2016-11-24 13:40 UTC (permalink / raw)
To: linux-arm-kernel
This series adds a new ABI to expose the CPU feature registers
to the user space via emulation of MRS instruction. The system exposes
only a limited set of feature values (See the documentation patch)
from the cpufeature infrastructure. The feature bits that are not
exposed are set to the 'safe value' which implies 'not supported'.
Apart from the selected feature registers, we expose MIDR_EL1 (Main
ID Register). The user should be aware that, reading MIDR_EL1 can be
tricky on a heterogeneous system (just like getcpu()). We export the
value of the current CPU where 'MRS' is executed.
Mark Rutland (2):
arm64: cpufeature: treat unknown fields as RES0
arm64: cpufeature: remove explicit RAZ fields
Suzuki K Poulose (7):
arm64: cpufeature: Cleanup feature bit tables
arm64: cpufeature: Document the rules of safe value for features
arm64: cpufeature: Define helpers for sys_reg id
arm64: Add helper to decode register from instruction
arm64: cpufeature: Track user visible fields
arm64: cpufeature: Expose CPUID registers by emulation
arm64: Documentation - Expose CPU feature registers
Documentation/arm64/cpu-feature-registers.txt | 198 +++++++++++++++
arch/arm64/include/asm/cpufeature.h | 27 +-
arch/arm64/include/asm/insn.h | 2 +
arch/arm64/include/asm/sysreg.h | 25 +-
arch/arm64/include/uapi/asm/hwcap.h | 1 +
arch/arm64/kernel/cpufeature.c | 339 +++++++++++++++++---------
arch/arm64/kernel/cpuinfo.c | 1 +
arch/arm64/kernel/insn.c | 29 +++
arch/arm64/kernel/traps.c | 2 +-
9 files changed, 503 insertions(+), 121 deletions(-)
create mode 100644 Documentation/arm64/cpu-feature-registers.txt
--
2.7.4
^ permalink raw reply
* [PATCH 1/9] arm64: cpufeature: treat unknown fields as RES0
From: Suzuki K Poulose @ 2016-11-24 13:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479994809-9081-1-git-send-email-suzuki.poulose@arm.com>
From: Mark Rutland <mark.rutland@arm.com>
Any fields not defined in an arm64_ftr_bits entry are propagated to the
system-wide register value in init_cpu_ftr_reg(), and while we require
that these strictly match for the sanity checks, we don't update them in
update_cpu_ftr_reg().
Generally, the lack of an arm64_ftr_bits entry indicates that the bits
are currently RES0 (as is the case for the upper 32 bits of all
supposedly 32-bit registers).
A better default would be to use zero for the system-wide value of
unallocated bits, making all register checking consistent, and allowing
for subsequent simplifications to the arm64_ftr_bits arrays.
This patch updates init_cpu_ftr_reg() to treat unallocated bits as RES0
for the purpose of the system-wide safe value. These bits will still be
sanity checked with strict match requirements, as is currently the case.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
arch/arm64/kernel/cpufeature.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index c02504e..a6ecf51 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -409,23 +409,33 @@ static void __init sort_ftr_regs(void)
/*
* Initialise the CPU feature register from Boot CPU values.
* Also initiliases the strict_mask for the register.
+ * Any bits that are not covered by an arm64_ftr_bits entry are considered
+ * RES0 for the system-wide value, and must strictly match.
*/
static void __init init_cpu_ftr_reg(u32 sys_reg, u64 new)
{
u64 val = 0;
u64 strict_mask = ~0x0ULL;
+ u64 valid_mask = 0;
+
const struct arm64_ftr_bits *ftrp;
struct arm64_ftr_reg *reg = get_arm64_ftr_reg(sys_reg);
BUG_ON(!reg);
for (ftrp = reg->ftr_bits; ftrp->width; ftrp++) {
+ u64 ftr_mask = arm64_ftr_mask(ftrp);
s64 ftr_new = arm64_ftr_value(ftrp, new);
val = arm64_ftr_set_value(ftrp, val, ftr_new);
+
+ valid_mask |= ftr_mask;
if (!ftrp->strict)
- strict_mask &= ~arm64_ftr_mask(ftrp);
+ strict_mask &= ~ftr_mask;
}
+
+ val &= valid_mask;
+
reg->sys_val = val;
reg->strict_mask = strict_mask;
}
--
2.7.4
^ permalink raw reply related
* [PATCH 2/9] arm64: cpufeature: remove explicit RAZ fields
From: Suzuki K Poulose @ 2016-11-24 13:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479994809-9081-1-git-send-email-suzuki.poulose@arm.com>
From: Mark Rutland <mark.rutland@arm.com>
We currently have some RAZ fields described explicitly in our
arm64_ftr_bits arrays. These are inconsistently commented, grouped,
and/or applied, and maintaining these is error-prone.
Luckily, we don't need these at all. We'll never need to inspect RAZ
fields to determine feature support, and init_cpu_ftr_reg() will ensure
that any bits without a corresponding arm64_ftr_bits entry are treated
as RES0 with strict matching requirements. In check_update_ftr_reg()
we'll then compare these bits from the relevant cpuinfo_arm64
structures, and need not store them in a arm64_ftr_reg.
This patch removes the unnecessary arm64_ftr_bits entries for RES0 bits.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
arch/arm64/kernel/cpufeature.c | 15 ---------------
1 file changed, 15 deletions(-)
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index a6ecf51..aaf3cba 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -80,21 +80,16 @@ cpufeature_pan_not_uao(const struct arm64_cpu_capabilities *entry, int __unused)
static const struct arm64_ftr_bits ftr_id_aa64isar0[] = {
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 32, 32, 0),
ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_AA64ISAR0_RDM_SHIFT, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 24, 4, 0),
ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR0_ATOMICS_SHIFT, 4, 0),
ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR0_CRC32_SHIFT, 4, 0),
ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR0_SHA2_SHIFT, 4, 0),
ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR0_SHA1_SHIFT, 4, 0),
ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR0_AES_SHIFT, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 0, 4, 0), /* RAZ */
ARM64_FTR_END,
};
static const struct arm64_ftr_bits ftr_id_aa64pfr0[] = {
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 32, 32, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 28, 4, 0),
ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_AA64PFR0_GIC_SHIFT, 4, 0),
S_ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR0_ASIMD_SHIFT, 4, ID_AA64PFR0_ASIMD_NI),
S_ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR0_FP_SHIFT, 4, ID_AA64PFR0_FP_NI),
@@ -107,7 +102,6 @@ static const struct arm64_ftr_bits ftr_id_aa64pfr0[] = {
};
static const struct arm64_ftr_bits ftr_id_aa64mmfr0[] = {
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 32, 32, 0),
S_ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_AA64MMFR0_TGRAN4_SHIFT, 4, ID_AA64MMFR0_TGRAN4_NI),
S_ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_AA64MMFR0_TGRAN64_SHIFT, 4, ID_AA64MMFR0_TGRAN64_NI),
ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_AA64MMFR0_TGRAN16_SHIFT, 4, ID_AA64MMFR0_TGRAN16_NI),
@@ -125,7 +119,6 @@ static const struct arm64_ftr_bits ftr_id_aa64mmfr0[] = {
};
static const struct arm64_ftr_bits ftr_id_aa64mmfr1[] = {
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 32, 32, 0),
ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR1_PAN_SHIFT, 4, 0),
ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_AA64MMFR1_LOR_SHIFT, 4, 0),
ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_AA64MMFR1_HPD_SHIFT, 4, 0),
@@ -146,7 +139,6 @@ static const struct arm64_ftr_bits ftr_id_aa64mmfr2[] = {
static const struct arm64_ftr_bits ftr_ctr[] = {
ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 31, 1, 1), /* RAO */
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 28, 3, 0),
ARM64_FTR_BITS(FTR_STRICT, FTR_HIGHER_SAFE, 24, 4, 0), /* CWG */
ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, 20, 4, 0), /* ERG */
ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, 16, 4, 1), /* DminLine */
@@ -156,7 +148,6 @@ static const struct arm64_ftr_bits ftr_ctr[] = {
* If we have differing I-cache policies, report it as the weakest - AIVIVT.
*/
ARM64_FTR_BITS(FTR_NONSTRICT, FTR_EXACT, 14, 2, ICACHE_POLICY_AIVIVT), /* L1Ip */
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 4, 10, 0), /* RAZ */
ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, 0, 4, 0), /* IminLine */
ARM64_FTR_END,
};
@@ -190,14 +181,12 @@ static const struct arm64_ftr_bits ftr_id_aa64dfr0[] = {
};
static const struct arm64_ftr_bits ftr_mvfr2[] = {
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 8, 24, 0), /* RAZ */
ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 4, 4, 0), /* FPMisc */
ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 0, 4, 0), /* SIMDMisc */
ARM64_FTR_END,
};
static const struct arm64_ftr_bits ftr_dczid[] = {
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 5, 27, 0), /* RAZ */
ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 4, 1, 1), /* DZP */
ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, 0, 4, 0), /* BS */
ARM64_FTR_END,
@@ -206,7 +195,6 @@ static const struct arm64_ftr_bits ftr_dczid[] = {
static const struct arm64_ftr_bits ftr_id_isar5[] = {
ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_ISAR5_RDM_SHIFT, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 20, 4, 0), /* RAZ */
ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_ISAR5_CRC32_SHIFT, 4, 0),
ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_ISAR5_SHA2_SHIFT, 4, 0),
ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_ISAR5_SHA1_SHIFT, 4, 0),
@@ -216,14 +204,11 @@ static const struct arm64_ftr_bits ftr_id_isar5[] = {
};
static const struct arm64_ftr_bits ftr_id_mmfr4[] = {
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 8, 24, 0), /* RAZ */
ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 4, 4, 0), /* ac2 */
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 0, 4, 0), /* RAZ */
ARM64_FTR_END,
};
static const struct arm64_ftr_bits ftr_id_pfr0[] = {
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 16, 16, 0), /* RAZ */
ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 12, 4, 0), /* State3 */
ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 8, 4, 0), /* State2 */
ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 4, 4, 0), /* State1 */
--
2.7.4
^ permalink raw reply related
* [PATCH 3/9] arm64: cpufeature: Cleanup feature bit tables
From: Suzuki K Poulose @ 2016-11-24 13:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479994809-9081-1-git-send-email-suzuki.poulose@arm.com>
This patch does the following clean ups :
1) All undescribed fields of a register are now treated as "strict"
with a safe value of 0. Hence we could leave an empty table for
describing registers which are RAZ.
2) ID_AA64DFR1_EL1 is RAZ and should use the table for RAZ register.
3) ftr_generic32 is used to represent a register with a 32bit feature
value. Rename this to ftr_singl32 to make it more obvious. Since
we don't have a 64bit singe feature register, kill ftr_generic.
Based on a patch by Mark Rutland.
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
arch/arm64/kernel/cpufeature.c | 19 +++++++------------
1 file changed, 7 insertions(+), 12 deletions(-)
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index aaf3cba..67e6935 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -246,18 +246,13 @@ static const struct arm64_ftr_bits ftr_generic_32bits[] = {
ARM64_FTR_END,
};
-static const struct arm64_ftr_bits ftr_generic[] = {
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 0, 64, 0),
- ARM64_FTR_END,
-};
-
-static const struct arm64_ftr_bits ftr_generic32[] = {
+/* Table for a single 32bit feature value */
+static const struct arm64_ftr_bits ftr_single32[] = {
ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 0, 32, 0),
ARM64_FTR_END,
};
-static const struct arm64_ftr_bits ftr_aa64raz[] = {
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 0, 64, 0),
+static const struct arm64_ftr_bits ftr_raz[] = {
ARM64_FTR_END,
};
@@ -298,15 +293,15 @@ static const struct __ftr_reg_entry {
/* Op1 = 0, CRn = 0, CRm = 4 */
ARM64_FTR_REG(SYS_ID_AA64PFR0_EL1, ftr_id_aa64pfr0),
- ARM64_FTR_REG(SYS_ID_AA64PFR1_EL1, ftr_aa64raz),
+ ARM64_FTR_REG(SYS_ID_AA64PFR1_EL1, ftr_raz),
/* Op1 = 0, CRn = 0, CRm = 5 */
ARM64_FTR_REG(SYS_ID_AA64DFR0_EL1, ftr_id_aa64dfr0),
- ARM64_FTR_REG(SYS_ID_AA64DFR1_EL1, ftr_generic),
+ ARM64_FTR_REG(SYS_ID_AA64DFR1_EL1, ftr_raz),
/* Op1 = 0, CRn = 0, CRm = 6 */
ARM64_FTR_REG(SYS_ID_AA64ISAR0_EL1, ftr_id_aa64isar0),
- ARM64_FTR_REG(SYS_ID_AA64ISAR1_EL1, ftr_aa64raz),
+ ARM64_FTR_REG(SYS_ID_AA64ISAR1_EL1, ftr_raz),
/* Op1 = 0, CRn = 0, CRm = 7 */
ARM64_FTR_REG(SYS_ID_AA64MMFR0_EL1, ftr_id_aa64mmfr0),
@@ -318,7 +313,7 @@ static const struct __ftr_reg_entry {
ARM64_FTR_REG(SYS_DCZID_EL0, ftr_dczid),
/* Op1 = 3, CRn = 14, CRm = 0 */
- ARM64_FTR_REG(SYS_CNTFRQ_EL0, ftr_generic32),
+ ARM64_FTR_REG(SYS_CNTFRQ_EL0, ftr_single32),
};
static int search_cmp_ftr_reg(const void *id, const void *regp)
--
2.7.4
^ permalink raw reply related
* [PATCH 4/9] arm64: cpufeature: Document the rules of safe value for features
From: Suzuki K Poulose @ 2016-11-24 13:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479994809-9081-1-git-send-email-suzuki.poulose@arm.com>
Document the rules for choosing the safe value for different types
of features.
Cc: Dave Martin <dave.martin@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
arch/arm64/include/asm/cpufeature.h | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index 0bc0b1d..59508a9 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -29,7 +29,21 @@
#include <linux/kernel.h>
-/* CPU feature register tracking */
+/*
+ * CPU feature register tracking
+ *
+ * The safe value of a CPUID feature field is dependent on the implications
+ * of the values assigned to it by the architecture. Based on the relationship
+ * between the values, the features are classified into 3 types.
+ *
+ * a) LOWER_SAFE - The value 'n+1' indicates, value 'n' and some
+ * additional features. (where n >= 0). The smaller value (n) is
+ * considered safer in this case.
+ * b) HIGHER_SAFE - The value 'n+1' is safer than 'n' (for n>= 0).
+ * c) EXACT - If the values of the feature don't have any relationship,
+ * a predefined safe value is used.
+ */
+
enum ftr_type {
FTR_EXACT, /* Use a predefined safe value */
FTR_LOWER_SAFE, /* Smaller value is safe */
--
2.7.4
^ permalink raw reply related
* [PATCH 5/9] arm64: cpufeature: Define helpers for sys_reg id
From: Suzuki K Poulose @ 2016-11-24 13:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479994809-9081-1-git-send-email-suzuki.poulose@arm.com>
Define helper macros to extract op0, op1, CRn, CRm & op2
for a given sys_reg id.
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
arch/arm64/include/asm/sysreg.h | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 6c80b36..488b939 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -34,8 +34,27 @@
* [11-8] : CRm
* [7-5] : Op2
*/
+#define Op0_shift 19
+#define Op0_mask 0x3
+#define Op1_shift 16
+#define Op1_mask 0x7
+#define CRn_shift 12
+#define CRn_mask 0xf
+#define CRm_shift 8
+#define CRm_mask 0xf
+#define Op2_shift 5
+#define Op2_mask 0x7
+
#define sys_reg(op0, op1, crn, crm, op2) \
- ((((op0)&3)<<19)|((op1)<<16)|((crn)<<12)|((crm)<<8)|((op2)<<5))
+ ((((op0) & Op0_mask) << Op0_shift) | \
+ ((op1) << Op1_shift) | ((crn) << CRn_shift) | \
+ ((crm) << CRm_shift) | ((op2) << Op2_shift))
+
+#define sys_reg_Op0(id) (((id) >> Op0_shift) & Op0_mask)
+#define sys_reg_Op1(id) (((id) >> Op1_shift) & Op1_mask)
+#define sys_reg_CRn(id) (((id) >> CRn_shift) & CRn_mask)
+#define sys_reg_CRm(id) (((id) >> CRm_shift) & CRm_mask)
+#define sys_reg_Op2(id) (((id) >> Op2_shift) & Op2_mask)
#define SYS_MIDR_EL1 sys_reg(3, 0, 0, 0, 0)
#define SYS_MPIDR_EL1 sys_reg(3, 0, 0, 0, 5)
--
2.7.4
^ permalink raw reply related
* [PATCH 6/9] arm64: Add helper to decode register from instruction
From: Suzuki K Poulose @ 2016-11-24 13:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479994809-9081-1-git-send-email-suzuki.poulose@arm.com>
Add a helper to extract the register field from a given
instruction.
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
arch/arm64/include/asm/insn.h | 2 ++
arch/arm64/kernel/insn.c | 29 +++++++++++++++++++++++++++++
2 files changed, 31 insertions(+)
diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h
index bc85366..aecc07e 100644
--- a/arch/arm64/include/asm/insn.h
+++ b/arch/arm64/include/asm/insn.h
@@ -332,6 +332,8 @@ bool aarch64_insn_is_branch(u32 insn);
u64 aarch64_insn_decode_immediate(enum aarch64_insn_imm_type type, u32 insn);
u32 aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type,
u32 insn, u64 imm);
+u32 aarch64_insn_decode_register(enum aarch64_insn_register_type type,
+ u32 insn);
u32 aarch64_insn_gen_branch_imm(unsigned long pc, unsigned long addr,
enum aarch64_insn_branch_type type);
u32 aarch64_insn_gen_comp_branch_imm(unsigned long pc, unsigned long addr,
diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c
index 6f2ac4f..755b3dd 100644
--- a/arch/arm64/kernel/insn.c
+++ b/arch/arm64/kernel/insn.c
@@ -418,6 +418,35 @@ u32 __kprobes aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type,
return insn;
}
+u32 aarch64_insn_decode_register(enum aarch64_insn_register_type type,
+ u32 insn)
+{
+ int shift;
+
+ switch (type) {
+ case AARCH64_INSN_REGTYPE_RT:
+ case AARCH64_INSN_REGTYPE_RD:
+ shift = 0;
+ break;
+ case AARCH64_INSN_REGTYPE_RN:
+ shift = 5;
+ break;
+ case AARCH64_INSN_REGTYPE_RT2:
+ case AARCH64_INSN_REGTYPE_RA:
+ shift = 10;
+ break;
+ case AARCH64_INSN_REGTYPE_RM:
+ shift = 16;
+ break;
+ default:
+ pr_err("%s: unknown register type encoding %d\n", __func__,
+ type);
+ return 0;
+ }
+
+ return (insn >> shift) & GENMASK(4, 0);
+}
+
static u32 aarch64_insn_encode_register(enum aarch64_insn_register_type type,
u32 insn,
enum aarch64_insn_register reg)
--
2.7.4
^ permalink raw reply related
* [PATCH 7/9] arm64: cpufeature: Track user visible fields
From: Suzuki K Poulose @ 2016-11-24 13:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479994809-9081-1-git-send-email-suzuki.poulose@arm.com>
Track the user visible fields of a CPU feature register. This will be
used for exposing the value to the userspace. All the user visible
fields of a feature register will be passed on as it is, while the
others would be filled with their respective safe value.
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
arch/arm64/include/asm/cpufeature.h | 11 +++
arch/arm64/kernel/cpufeature.c | 189 +++++++++++++++++++-----------------
arch/arm64/kernel/traps.c | 2 +-
3 files changed, 111 insertions(+), 91 deletions(-)
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index 59508a9..10d478e 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -56,8 +56,12 @@ enum ftr_type {
#define FTR_SIGNED true /* Value should be treated as signed */
#define FTR_UNSIGNED false /* Value should be treated as unsigned */
+#define FTR_VISIBLE true /* Feature visible to the user space */
+#define FTR_HIDDEN false /* Featured is hidden from the user */
+
struct arm64_ftr_bits {
bool sign; /* Value is signed ? */
+ bool visible;
bool strict; /* CPU Sanity check: strict matching required ? */
enum ftr_type type;
u8 shift;
@@ -73,7 +77,9 @@ struct arm64_ftr_bits {
struct arm64_ftr_reg {
const char *name;
u64 strict_mask;
+ u64 user_mask;
u64 sys_val;
+ u64 user_val;
const struct arm64_ftr_bits *ftr_bits;
};
@@ -168,6 +174,11 @@ static inline u64 arm64_ftr_mask(const struct arm64_ftr_bits *ftrp)
return (u64)GENMASK(ftrp->shift + ftrp->width - 1, ftrp->shift);
}
+static inline u64 arm64_ftr_reg_user_value(const struct arm64_ftr_reg *reg)
+{
+ return (reg->user_val | (reg->sys_val & reg->user_mask));
+}
+
static inline int __attribute_const__
cpuid_feature_extract_field(u64 features, int field, bool sign)
{
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 67e6935..a7532dd 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -51,9 +51,10 @@ DECLARE_BITMAP(cpu_hwcaps, ARM64_NCAPS);
DEFINE_STATIC_KEY_ARRAY_FALSE(cpu_hwcap_keys, ARM64_NCAPS);
EXPORT_SYMBOL(cpu_hwcap_keys);
-#define __ARM64_FTR_BITS(SIGNED, STRICT, TYPE, SHIFT, WIDTH, SAFE_VAL) \
+#define __ARM64_FTR_BITS(SIGNED, VISIBLE, STRICT, TYPE, SHIFT, WIDTH, SAFE_VAL) \
{ \
.sign = SIGNED, \
+ .visible = VISIBLE, \
.strict = STRICT, \
.type = TYPE, \
.shift = SHIFT, \
@@ -62,12 +63,12 @@ EXPORT_SYMBOL(cpu_hwcap_keys);
}
/* Define a feature with unsigned values */
-#define ARM64_FTR_BITS(STRICT, TYPE, SHIFT, WIDTH, SAFE_VAL) \
- __ARM64_FTR_BITS(FTR_UNSIGNED, STRICT, TYPE, SHIFT, WIDTH, SAFE_VAL)
+#define ARM64_FTR_BITS(VISIBLE, STRICT, TYPE, SHIFT, WIDTH, SAFE_VAL) \
+ __ARM64_FTR_BITS(FTR_UNSIGNED, VISIBLE, STRICT, TYPE, SHIFT, WIDTH, SAFE_VAL)
/* Define a feature with a signed value */
-#define S_ARM64_FTR_BITS(STRICT, TYPE, SHIFT, WIDTH, SAFE_VAL) \
- __ARM64_FTR_BITS(FTR_SIGNED, STRICT, TYPE, SHIFT, WIDTH, SAFE_VAL)
+#define S_ARM64_FTR_BITS(VISIBLE, STRICT, TYPE, SHIFT, WIDTH, SAFE_VAL) \
+ __ARM64_FTR_BITS(FTR_SIGNED, VISIBLE, STRICT, TYPE, SHIFT, WIDTH, SAFE_VAL)
#define ARM64_FTR_END \
{ \
@@ -80,75 +81,75 @@ cpufeature_pan_not_uao(const struct arm64_cpu_capabilities *entry, int __unused)
static const struct arm64_ftr_bits ftr_id_aa64isar0[] = {
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_AA64ISAR0_RDM_SHIFT, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR0_ATOMICS_SHIFT, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR0_CRC32_SHIFT, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR0_SHA2_SHIFT, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR0_SHA1_SHIFT, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR0_AES_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_AA64ISAR0_RDM_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR0_ATOMICS_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR0_CRC32_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR0_SHA2_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR0_SHA1_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR0_AES_SHIFT, 4, 0),
ARM64_FTR_END,
};
static const struct arm64_ftr_bits ftr_id_aa64pfr0[] = {
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_AA64PFR0_GIC_SHIFT, 4, 0),
- S_ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR0_ASIMD_SHIFT, 4, ID_AA64PFR0_ASIMD_NI),
- S_ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR0_FP_SHIFT, 4, ID_AA64PFR0_FP_NI),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_AA64PFR0_GIC_SHIFT, 4, 0),
+ S_ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR0_ASIMD_SHIFT, 4, ID_AA64PFR0_ASIMD_NI),
+ S_ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR0_FP_SHIFT, 4, ID_AA64PFR0_FP_NI),
/* Linux doesn't care about the EL3 */
- ARM64_FTR_BITS(FTR_NONSTRICT, FTR_EXACT, ID_AA64PFR0_EL3_SHIFT, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_AA64PFR0_EL2_SHIFT, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_AA64PFR0_EL1_SHIFT, 4, ID_AA64PFR0_EL1_64BIT_ONLY),
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_AA64PFR0_EL0_SHIFT, 4, ID_AA64PFR0_EL0_64BIT_ONLY),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_EXACT, ID_AA64PFR0_EL3_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_AA64PFR0_EL2_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_AA64PFR0_EL1_SHIFT, 4, ID_AA64PFR0_EL1_64BIT_ONLY),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_AA64PFR0_EL0_SHIFT, 4, ID_AA64PFR0_EL0_64BIT_ONLY),
ARM64_FTR_END,
};
static const struct arm64_ftr_bits ftr_id_aa64mmfr0[] = {
- S_ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_AA64MMFR0_TGRAN4_SHIFT, 4, ID_AA64MMFR0_TGRAN4_NI),
- S_ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_AA64MMFR0_TGRAN64_SHIFT, 4, ID_AA64MMFR0_TGRAN64_NI),
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_AA64MMFR0_TGRAN16_SHIFT, 4, ID_AA64MMFR0_TGRAN16_NI),
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_AA64MMFR0_BIGENDEL0_SHIFT, 4, 0),
+ S_ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_AA64MMFR0_TGRAN4_SHIFT, 4, ID_AA64MMFR0_TGRAN4_NI),
+ S_ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_AA64MMFR0_TGRAN64_SHIFT, 4, ID_AA64MMFR0_TGRAN64_NI),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_AA64MMFR0_TGRAN16_SHIFT, 4, ID_AA64MMFR0_TGRAN16_NI),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_AA64MMFR0_BIGENDEL0_SHIFT, 4, 0),
/* Linux shouldn't care about secure memory */
- ARM64_FTR_BITS(FTR_NONSTRICT, FTR_EXACT, ID_AA64MMFR0_SNSMEM_SHIFT, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_AA64MMFR0_BIGENDEL_SHIFT, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_AA64MMFR0_ASID_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_EXACT, ID_AA64MMFR0_SNSMEM_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_AA64MMFR0_BIGENDEL_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_AA64MMFR0_ASID_SHIFT, 4, 0),
/*
* Differing PARange is fine as long as all peripherals and memory are mapped
* within the minimum PARange of all CPUs
*/
- ARM64_FTR_BITS(FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64MMFR0_PARANGE_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64MMFR0_PARANGE_SHIFT, 4, 0),
ARM64_FTR_END,
};
static const struct arm64_ftr_bits ftr_id_aa64mmfr1[] = {
- ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR1_PAN_SHIFT, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_AA64MMFR1_LOR_SHIFT, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_AA64MMFR1_HPD_SHIFT, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_AA64MMFR1_VHE_SHIFT, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_AA64MMFR1_VMIDBITS_SHIFT, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_AA64MMFR1_HADBS_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR1_PAN_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_AA64MMFR1_LOR_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_AA64MMFR1_HPD_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_AA64MMFR1_VHE_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_AA64MMFR1_VMIDBITS_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_AA64MMFR1_HADBS_SHIFT, 4, 0),
ARM64_FTR_END,
};
static const struct arm64_ftr_bits ftr_id_aa64mmfr2[] = {
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_AA64MMFR2_LVA_SHIFT, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_AA64MMFR2_IESB_SHIFT, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_AA64MMFR2_LSM_SHIFT, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_AA64MMFR2_UAO_SHIFT, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_AA64MMFR2_CNP_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_AA64MMFR2_LVA_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_AA64MMFR2_IESB_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_AA64MMFR2_LSM_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_AA64MMFR2_UAO_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_AA64MMFR2_CNP_SHIFT, 4, 0),
ARM64_FTR_END,
};
static const struct arm64_ftr_bits ftr_ctr[] = {
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 31, 1, 1), /* RAO */
- ARM64_FTR_BITS(FTR_STRICT, FTR_HIGHER_SAFE, 24, 4, 0), /* CWG */
- ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, 20, 4, 0), /* ERG */
- ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, 16, 4, 1), /* DminLine */
+ ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_EXACT, 31, 1, 1), /* RAO */
+ ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_HIGHER_SAFE, 24, 4, 0), /* CWG */
+ ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, 20, 4, 0), /* ERG */
+ ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, 16, 4, 1), /* DminLine */
/*
* Linux can handle differing I-cache policies. Userspace JITs will
* make use of *minLine.
* If we have differing I-cache policies, report it as the weakest - AIVIVT.
*/
- ARM64_FTR_BITS(FTR_NONSTRICT, FTR_EXACT, 14, 2, ICACHE_POLICY_AIVIVT), /* L1Ip */
- ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, 0, 4, 0), /* IminLine */
+ ARM64_FTR_BITS(FTR_VISIBLE, FTR_NONSTRICT, FTR_EXACT, 14, 2, ICACHE_POLICY_AIVIVT), /* L1Ip */
+ ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, 0, 4, 0), /* IminLine */
ARM64_FTR_END,
};
@@ -158,73 +159,73 @@ struct arm64_ftr_reg arm64_ftr_reg_ctrel0 = {
};
static const struct arm64_ftr_bits ftr_id_mmfr0[] = {
- S_ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 28, 4, 0xf), /* InnerShr */
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 24, 4, 0), /* FCSE */
- ARM64_FTR_BITS(FTR_NONSTRICT, FTR_LOWER_SAFE, 20, 4, 0), /* AuxReg */
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 16, 4, 0), /* TCM */
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 12, 4, 0), /* ShareLvl */
- S_ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 8, 4, 0xf), /* OuterShr */
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 4, 4, 0), /* PMSA */
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 0, 4, 0), /* VMSA */
+ S_ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, 28, 4, 0xf), /* InnerShr */
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, 24, 4, 0), /* FCSE */
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, 20, 4, 0), /* AuxReg */
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, 16, 4, 0), /* TCM */
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, 12, 4, 0), /* ShareLvl */
+ S_ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, 8, 4, 0xf), /* OuterShr */
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, 4, 4, 0), /* PMSA */
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, 0, 4, 0), /* VMSA */
ARM64_FTR_END,
};
static const struct arm64_ftr_bits ftr_id_aa64dfr0[] = {
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 32, 32, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, ID_AA64DFR0_CTX_CMPS_SHIFT, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, ID_AA64DFR0_WRPS_SHIFT, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, ID_AA64DFR0_BRPS_SHIFT, 4, 0),
- S_ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_AA64DFR0_PMUVER_SHIFT, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_AA64DFR0_TRACEVER_SHIFT, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_AA64DFR0_DEBUGVER_SHIFT, 4, 0x6),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, 32, 32, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64DFR0_CTX_CMPS_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64DFR0_WRPS_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64DFR0_BRPS_SHIFT, 4, 0),
+ S_ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_AA64DFR0_PMUVER_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_AA64DFR0_TRACEVER_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_AA64DFR0_DEBUGVER_SHIFT, 4, 0x6),
ARM64_FTR_END,
};
static const struct arm64_ftr_bits ftr_mvfr2[] = {
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 4, 4, 0), /* FPMisc */
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 0, 4, 0), /* SIMDMisc */
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, 4, 4, 0), /* FPMisc */
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, 0, 4, 0), /* SIMDMisc */
ARM64_FTR_END,
};
static const struct arm64_ftr_bits ftr_dczid[] = {
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 4, 1, 1), /* DZP */
- ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, 0, 4, 0), /* BS */
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, 4, 1, 1), /* DZP */
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 0, 4, 0), /* BS */
ARM64_FTR_END,
};
static const struct arm64_ftr_bits ftr_id_isar5[] = {
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_ISAR5_RDM_SHIFT, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_ISAR5_CRC32_SHIFT, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_ISAR5_SHA2_SHIFT, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_ISAR5_SHA1_SHIFT, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_ISAR5_AES_SHIFT, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, ID_ISAR5_SEVL_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_ISAR5_RDM_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_ISAR5_CRC32_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_ISAR5_SHA2_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_ISAR5_SHA1_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_ISAR5_AES_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_ISAR5_SEVL_SHIFT, 4, 0),
ARM64_FTR_END,
};
static const struct arm64_ftr_bits ftr_id_mmfr4[] = {
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 4, 4, 0), /* ac2 */
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, 4, 4, 0), /* ac2 */
ARM64_FTR_END,
};
static const struct arm64_ftr_bits ftr_id_pfr0[] = {
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 12, 4, 0), /* State3 */
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 8, 4, 0), /* State2 */
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 4, 4, 0), /* State1 */
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 0, 4, 0), /* State0 */
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, 12, 4, 0), /* State3 */
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, 8, 4, 0), /* State2 */
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, 4, 4, 0), /* State1 */
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, 0, 4, 0), /* State0 */
ARM64_FTR_END,
};
static const struct arm64_ftr_bits ftr_id_dfr0[] = {
- ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, 28, 4, 0),
- S_ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, 24, 4, 0xf), /* PerfMon */
- ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, 20, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, 16, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, 12, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, 8, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, 4, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, 0, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 28, 4, 0),
+ S_ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 24, 4, 0xf), /* PerfMon */
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 20, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 16, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 12, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 8, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 4, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 0, 4, 0),
ARM64_FTR_END,
};
@@ -235,20 +236,20 @@ static const struct arm64_ftr_bits ftr_id_dfr0[] = {
* id_isar[0-4], id_mmfr[1-3], id_pfr1, mvfr[0-1]
*/
static const struct arm64_ftr_bits ftr_generic_32bits[] = {
- ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, 28, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, 24, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, 20, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, 16, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, 12, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, 8, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, 4, 4, 0),
- ARM64_FTR_BITS(FTR_STRICT, FTR_LOWER_SAFE, 0, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 28, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 24, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 20, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 16, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 12, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 8, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 4, 4, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 0, 4, 0),
ARM64_FTR_END,
};
/* Table for a single 32bit feature value */
static const struct arm64_ftr_bits ftr_single32[] = {
- ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 0, 32, 0),
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, 0, 32, 0),
ARM64_FTR_END,
};
@@ -396,6 +397,7 @@ static void __init init_cpu_ftr_reg(u32 sys_reg, u64 new)
{
u64 val = 0;
u64 strict_mask = ~0x0ULL;
+ u64 user_mask = 0;
u64 valid_mask = 0;
const struct arm64_ftr_bits *ftrp;
@@ -412,12 +414,19 @@ static void __init init_cpu_ftr_reg(u32 sys_reg, u64 new)
valid_mask |= ftr_mask;
if (!ftrp->strict)
strict_mask &= ~ftr_mask;
+ if (ftrp->visible)
+ user_mask |= ftr_mask;
+ else
+ reg->user_val = arm64_ftr_set_value(ftrp,
+ reg->user_val,
+ ftrp->safe_val);
}
val &= valid_mask;
reg->sys_val = val;
reg->strict_mask = strict_mask;
+ reg->user_mask = user_mask;
}
void __init init_cpu_features(struct cpuinfo_arm64 *info)
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index c9986b3..1f4b6df 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -488,7 +488,7 @@ static void ctr_read_handler(unsigned int esr, struct pt_regs *regs)
{
int rt = (esr & ESR_ELx_SYS64_ISS_RT_MASK) >> ESR_ELx_SYS64_ISS_RT_SHIFT;
- regs->regs[rt] = arm64_ftr_reg_ctrel0.sys_val;
+ regs->regs[rt] = arm64_ftr_reg_user_value(&arm64_ftr_reg_ctrel0);
regs->pc += 4;
}
--
2.7.4
^ permalink raw reply related
* [PATCH 8/9] arm64: cpufeature: Expose CPUID registers by emulation
From: Suzuki K Poulose @ 2016-11-24 13:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479994809-9081-1-git-send-email-suzuki.poulose@arm.com>
This patch adds the hook for emulating MRS instruction to
export the 'user visible' value of supported system registers.
We emulate only the following id space for system registers:
Op0=3, Op1=0, CRn=0, CRm=[0-7]
The rest will fall back to SIGILL. This capability is also
advertised via a new HWCAP_CPUID.
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
arch/arm64/include/asm/sysreg.h | 4 ++
arch/arm64/include/uapi/asm/hwcap.h | 1 +
arch/arm64/kernel/cpufeature.c | 100 ++++++++++++++++++++++++++++++++++++
arch/arm64/kernel/cpuinfo.c | 1 +
4 files changed, 106 insertions(+)
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 488b939..eb1fc46 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -239,6 +239,10 @@
#define ID_AA64MMFR0_TGRAN_SUPPORTED ID_AA64MMFR0_TGRAN64_SUPPORTED
#endif
+
+/* Safe value for MPIDR_EL1: Bit31:RES1, Bit30:U:0, Bit24:MT:1 */
+#define SYS_MPIDR_SAFE_VAL ((1UL<<31)|(1UL<<24))
+
#ifdef __ASSEMBLY__
.irp num,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30
diff --git a/arch/arm64/include/uapi/asm/hwcap.h b/arch/arm64/include/uapi/asm/hwcap.h
index a739287..773c90b 100644
--- a/arch/arm64/include/uapi/asm/hwcap.h
+++ b/arch/arm64/include/uapi/asm/hwcap.h
@@ -30,5 +30,6 @@
#define HWCAP_ATOMICS (1 << 8)
#define HWCAP_FPHP (1 << 9)
#define HWCAP_ASIMDHP (1 << 10)
+#define HWCAP_CPUID (1 << 11)
#endif /* _UAPI__ASM_HWCAP_H */
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index a7532dd..94c188f 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -29,6 +29,7 @@
#include <asm/mmu_context.h>
#include <asm/processor.h>
#include <asm/sysreg.h>
+#include <asm/traps.h>
#include <asm/virt.h>
unsigned long elf_hwcap __read_mostly;
@@ -916,6 +917,8 @@ static bool cpus_have_elf_hwcap(const struct arm64_cpu_capabilities *cap)
static void __init setup_elf_hwcaps(const struct arm64_cpu_capabilities *hwcaps)
{
+ /* We support emulation of accesses to CPU ID feature registers */
+ elf_hwcap |= HWCAP_CPUID;
for (; hwcaps->matches; hwcaps++)
if (hwcaps->matches(hwcaps, hwcaps->def_scope))
cap_set_elf_hwcap(hwcaps);
@@ -1103,3 +1106,100 @@ cpufeature_pan_not_uao(const struct arm64_cpu_capabilities *entry, int __unused)
{
return (cpus_have_cap(ARM64_HAS_PAN) && !cpus_have_cap(ARM64_HAS_UAO));
}
+
+/*
+ * We emulate only the following system register space.
+ * Op0 = 0x3, CRn = 0x0, Op1 = 0x0, CRm = [0 - 7]
+ * See Table C5-6 System instruction encodings for System register accesses,
+ * ARMv8 ARM(ARM DDI 0487A.f) for more details.
+ */
+static inline bool __attribute_const__ is_emulated(u32 id)
+{
+ return (sys_reg_Op0(id) == 0x3 &&
+ sys_reg_CRn(id) == 0x0 &&
+ sys_reg_Op1(id) == 0x0 &&
+ sys_reg_CRm(id) <= 7);
+}
+
+/*
+ * With CRm == 0, reg should be one of :
+ * MIDR_EL1, MPIDR_EL1 or REVIDR_EL1.
+ */
+static inline int emulate_id_reg(u32 id, u64 *valp)
+{
+ switch(id) {
+ case SYS_MIDR_EL1:
+ *valp = read_cpuid_id();
+ break;
+ case SYS_MPIDR_EL1:
+ *valp = SYS_MPIDR_SAFE_VAL;
+ break;
+ case SYS_REVIDR_EL1:
+ /* IMPLEMENTATION DEFINED values are emulated with 0 */
+ *valp = 0;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int emulate_sys_reg(u32 id, u64 *valp)
+{
+ struct arm64_ftr_reg *regp;
+
+ if (!is_emulated(id))
+ return -EINVAL;
+
+ if (sys_reg_CRm(id) == 0)
+ return emulate_id_reg(id, valp);
+
+ regp = get_arm64_ftr_reg(id);
+ if (regp)
+ *valp = arm64_ftr_reg_user_value(regp);
+ else
+ /*
+ * The untracked registers are either IMPLEMENTATION DEFINED
+ * (e.g, ID_AFR0_EL1) or reserved RAZ.
+ */
+ *valp = 0;
+ return 0;
+}
+
+static int emulate_mrs(struct pt_regs *regs, u32 insn)
+{
+ int rc;
+ u32 sys_reg, dst;
+ u64 val;
+
+ /*
+ * sys_reg values are defined as used in mrs/msr instruction.
+ * shift the imm value to get the encoding.
+ */
+ sys_reg = (u32)aarch64_insn_decode_immediate(AARCH64_INSN_IMM_16, insn) << 5;
+ rc = emulate_sys_reg(sys_reg, &val);
+ if (!rc) {
+ dst = aarch64_insn_decode_register(AARCH64_INSN_REGTYPE_RT, insn);
+ regs->user_regs.regs[dst] = val;
+ regs->pc += 4;
+ }
+
+ return rc;
+}
+
+static struct undef_hook mrs_hook = {
+ .instr_mask = 0xfff00000,
+ .instr_val = 0xd5300000,
+ .pstate_mask = COMPAT_PSR_MODE_MASK,
+ .pstate_val = PSR_MODE_EL0t,
+ .fn = emulate_mrs,
+};
+
+int __init enable_mrs_emulation(void)
+{
+ register_undef_hook(&mrs_hook);
+ return 0;
+}
+
+late_initcall(enable_mrs_emulation);
diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
index b3d5b3e..ef8406f 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -63,6 +63,7 @@ static const char *const hwcap_str[] = {
"atomics",
"fphp",
"asimdhp",
+ "cpuid",
NULL
};
--
2.7.4
^ permalink raw reply related
* [PATCH 9/9] arm64: Documentation - Expose CPU feature registers
From: Suzuki K Poulose @ 2016-11-24 13:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479994809-9081-1-git-send-email-suzuki.poulose@arm.com>
Documentation for the infrastructure to expose CPU feature
register by emulating MRS.
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Dave Martin <dave.martin@arm.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
Documentation/arm64/cpu-feature-registers.txt | 198 ++++++++++++++++++++++++++
arch/arm64/kernel/cpufeature.c | 4 +
2 files changed, 202 insertions(+)
create mode 100644 Documentation/arm64/cpu-feature-registers.txt
diff --git a/Documentation/arm64/cpu-feature-registers.txt b/Documentation/arm64/cpu-feature-registers.txt
new file mode 100644
index 0000000..e10bbc2
--- /dev/null
+++ b/Documentation/arm64/cpu-feature-registers.txt
@@ -0,0 +1,198 @@
+ ARM64 CPU Feature Registers
+ ===========================
+
+Author: Suzuki K Poulose <suzuki.poulose@arm.com>
+
+
+This file describes the API for exporting the AArch64 CPU ID/feature
+registers to userspace. The availability of this API is advertised
+via the HWCAP_CPUID in HWCAPs.
+
+1. Motivation
+---------------
+
+The ARM architecture defines a set of feature registers, which describe
+the capabilities of the CPU/system. Access to these system registers is
+restricted from EL0 and there is no reliable way for an application to
+extract this information to make better decisions at runtime. There is
+limited information available to the application via HWCAPs, however
+there are some issues with their usage.
+
+ a) Any change to the HWCAPs requires an update to userspace (e.g libc)
+ to detect the new changes, which can take a long time to appear in
+ distributions. Exposing the registers allows applications to get the
+ information without requiring updates to the toolchains.
+
+ b) Access to HWCAPs is sometimes limited (e.g prior to libc, or
+ when ld is initialised at startup time).
+
+ c) HWCAPs cannot represent non-boolean information effectively. The
+ architecture defines a canonical format for representing features
+ in the ID registers; this is well defined and is capable of
+ representing all valid architecture variations. Exposing the ID
+ registers avoids having to come up with HWCAP representations and
+ parsing code.
+
+
+2. Requirements
+-----------------
+
+ a) Safety :
+ Applications should be able to use the information provided by the
+ infrastructure to run safely across the system. This has greater
+ implications on a system with heterogeneous CPUs.
+ The infrastructure exports a value that is safe across all the
+ available CPU on the system.
+
+ e.g, If at least one CPU doesn't implement CRC32 instructions, while
+ others do, we should report that the CRC32 is not implemented.
+ Otherwise an application could crash when scheduled on the CPU
+ which doesn't support CRC32.
+
+ b) Security :
+ Applications should only be able to receive information that is
+ relevant to the normal operation in userspace. Hence, some of the
+ fields are masked out and the values of the fields are set to
+ indicate the feature is 'not supported' (See the 'visible' field in
+ the table in Section 4). Also, the kernel may manipulate the fields
+ based on what it supports. e.g, If FP is not supported by the
+ kernel, the values could indicate that the FP is not available
+ (even when the CPU provides it).
+
+ c) Implementation Defined Features
+ The infrastructure doesn't expose any register which is
+ IMPLEMENTATION DEFINED as per ARMv8-A Architecture.
+
+ d) CPU Identification :
+ MIDR_EL1 is exposed to help identify the processor. On a
+ heterogeneous system, this could be racy (just like getcpu()). The
+ process could be migrated to another CPU by the time it uses the
+ register value, unless the CPU affinity is set. Hence, there is no
+ guarantee that the value reflects the processor that it is
+ currently executing on. The REVIDR is not exposed due to this
+ constraint, as REVIDR makes sense only in conjunction with the
+ MIDR. Alternately, MIDR_EL1 and REVIDR_EL1 are exposed via sysfs
+ at:
+
+ /sys/devices/system/cpu/cpu$ID/regs/identification/
+ \- midr
+ \- revidr
+
+The list of supported registers and the attributes of individual
+feature bits are listed in section 4. Unless there is absolute
+necessity, we don't encourage the addition of new feature registers to
+the list. In any case, it should comply to the requirements listed
+above.
+
+3. Implementation
+--------------------
+
+The infrastructure is built on the emulation of the 'MRS' instruction.
+Accessing a restricted system register from an application generates an
+exception and ends up in SIGILL being delivered to the process.
+The infrastructure hooks into the exception handler and emulates the
+operation if the source belongs to the supported system register space.
+
+The infrastructure emulates only the following system register space:
+ Op0=3, Op1=0, CRn=0
+
+(See Table C5-6 'System instruction encodings for non-Debug System
+register accesses' in ARMv8 ARM DDI 0487A.h, for the list of
+registers).
+
+
+The following rules are applied to the value returned by the
+infrastructure:
+
+ a) The value of an 'IMPLEMENTATION DEFINED' field is set to 0.
+ b) The value of a reserved field is populated with the reserved
+ value as defined by the architecture.
+ c) The value of a field marked as not 'visible', is set to indicate
+ the feature is missing (as defined by the architecture).
+ d) The value of a 'visible' field holds the system wide safe value
+ for the particular feature(except for MIDR_EL1, see section 4).
+ See Appendix I for more information on safe value.
+
+There are only a few registers visible to the userspace. See Section 4,
+for the list of 'visible' registers.
+
+All others are emulated as having 'invisible' features.
+
+4. List of exposed registers
+-----------------------------
+
+ 1) ID_AA64ISAR0_EL1 - Instruction Set Attribute Register 0
+ x--------------------------------------------------x
+ | Name | bits | visible |
+ |--------------------------------------------------|
+ | RES0 | [63-24] | y |
+ |--------------------------------------------------|
+ | ATOMICS | [20-23] | y |
+ |--------------------------------------------------|
+ | CRC32 | [19-16] | y |
+ |--------------------------------------------------|
+ | SHA2 | [15-12] | y |
+ |--------------------------------------------------|
+ | SHA1 | [11-8] | y |
+ |--------------------------------------------------|
+ | AES | [7-4] | y |
+ |--------------------------------------------------|
+ | RES0 | [3-0] | y |
+ x--------------------------------------------------x
+
+ 2) ID_AA64ISAR1_EL1 - Instruction Set Attribute Register 1
+ x--------------------------------------------------x
+ | Name | bits | visible |
+ |--------------------------------------------------|
+ | RES0 | [63-0] | y |
+ x--------------------------------------------------x
+
+ 3) ID_AA64PFR0_EL1 - Processor Feature Register 0
+ x--------------------------------------------------x
+ | Name | bits | visible |
+ |--------------------------------------------------|
+ | RES0 | [63-28] | y |
+ |--------------------------------------------------|
+ | GIC | [27-24] | n |
+ |--------------------------------------------------|
+ | AdvSIMD | [23-20] | y |
+ |--------------------------------------------------|
+ | FP | [19-16] | y |
+ |--------------------------------------------------|
+ | EL3 | [15-12] | n |
+ |--------------------------------------------------|
+ | EL2 | [11-8] | n |
+ |--------------------------------------------------|
+ | EL1 | [7-4] | n |
+ |--------------------------------------------------|
+ | EL0 | [3-0] | n |
+ x--------------------------------------------------x
+
+ 4) ID_AA64PFR1_EL1 - Processor Feature Register 1
+ x--------------------------------------------------x
+ | Name | bits | visible |
+ |--------------------------------------------------|
+ | RES0 | [63-0] | y |
+ x--------------------------------------------------x
+
+ 5) MIDR_EL1 - Main ID Register
+ x--------------------------------------------------x
+ | Name | bits | visible |
+ |--------------------------------------------------|
+ | RES0 | [63-32] | y |
+ |--------------------------------------------------|
+ | Implementer | [31-24] | y |
+ |--------------------------------------------------|
+ | Variant | [23-20] | y |
+ |--------------------------------------------------|
+ | Architecture | [19-16] | y |
+ |--------------------------------------------------|
+ | PartNum | [15-4] | y |
+ |--------------------------------------------------|
+ | Revision | [3-0] | y |
+ x--------------------------------------------------x
+
+ NOTE: The 'visible' fields of MIDR_EL1 will contain the value
+ as available on the CPU where it is fetched and is not a system
+ wide safe value.
+
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 94c188f..fb331de 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -81,6 +81,10 @@ static bool __maybe_unused
cpufeature_pan_not_uao(const struct arm64_cpu_capabilities *entry, int __unused);
+/*
+ * NOTE: Any changes to the visibility of features should be kept in
+ * sync with the documentation of the CPU feature register API.
+ */
static const struct arm64_ftr_bits ftr_id_aa64isar0[] = {
ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_AA64ISAR0_RDM_SHIFT, 4, 0),
ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR0_ATOMICS_SHIFT, 4, 0),
--
2.7.4
^ permalink raw reply related
* [PATCH] arm64: mm: Fix memmap to be initialized for the entire section
From: Robert Richter @ 2016-11-24 13:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKv+Gu8aWpz5vXSWdKHwL5Xk2aWyQBOoR1WHOnVPaWZOwqojBQ@mail.gmail.com>
On 23.11.16 21:25:06, Ard Biesheuvel wrote:
> Why? MEMREMAP_WB is used often, among other things for mapping
> firmware tables, which are marked as NOMAP, so in these cases, the
> linear address is not mapped.
If fw tables are mapped wb, that is wrong and needs a separate fix.
> > If you think pfn_valid() is wrong here, I am happy to send a patch
> > that fixes this by using page_is_ram(). In any case, the worst case
> > that may happen is to behave the same as v4.4, we might fix then the
> > wrong use of pfn_valid() where it is not correctly used to check for
> > ram.
> >
>
> page_is_ram() uses string comparisons to look for regions called
> 'System RAM'. Is that something we can tolerate for each pfn_valid()
> calll?
>
> Perhaps the solution is to reimplement page_is_ram() for arm64 using
> memblock_is_memory() instead, But that still means we need to modify
> the generic memremap() code first to switch to it before changing the
> arm64 implementation of pfn_valid
No, that's not the solution. pfn_valid() should just check if there is
a valid struct page, as other archs do. And if there is a mis-use of
pfn_valid() to check for ram, only that calls should be fixed to use
page_is_ram(), however this is implemented, or something appropriate.
But I don't see any problematic code, and if so, I will fix that.
-Robert
^ permalink raw reply
* [PATCH] arm64: mm: Fix memmap to be initialized for the entire section
From: Ard Biesheuvel @ 2016-11-24 13:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161124134238.GI10776@rric.localdomain>
On 24 November 2016 at 13:42, Robert Richter <robert.richter@cavium.com> wrote:
> On 23.11.16 21:25:06, Ard Biesheuvel wrote:
>> Why? MEMREMAP_WB is used often, among other things for mapping
>> firmware tables, which are marked as NOMAP, so in these cases, the
>> linear address is not mapped.
>
> If fw tables are mapped wb, that is wrong and needs a separate fix.
>
Why is that wrong?
>> > If you think pfn_valid() is wrong here, I am happy to send a patch
>> > that fixes this by using page_is_ram(). In any case, the worst case
>> > that may happen is to behave the same as v4.4, we might fix then the
>> > wrong use of pfn_valid() where it is not correctly used to check for
>> > ram.
>> >
>>
>> page_is_ram() uses string comparisons to look for regions called
>> 'System RAM'. Is that something we can tolerate for each pfn_valid()
>> calll?
>>
>> Perhaps the solution is to reimplement page_is_ram() for arm64 using
>> memblock_is_memory() instead, But that still means we need to modify
>> the generic memremap() code first to switch to it before changing the
>> arm64 implementation of pfn_valid
>
> No, that's not the solution. pfn_valid() should just check if there is
> a valid struct page, as other archs do. And if there is a mis-use of
> pfn_valid() to check for ram, only that calls should be fixed to use
> page_is_ram(), however this is implemented, or something appropriate.
> But I don't see any problematic code, and if so, I will fix that.
>
memremap() uses pfn_valid() to decide whether some address is covered
by the linear mapping. If we correct pfn_valid() to adhere to your
definition, we will need to fix memremap() first in any case.
^ permalink raw reply
* TDA998x crash on HDLCD probe failure
From: Robin Murphy @ 2016-11-24 13:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161124132905.GG14217@n2100.armlinux.org.uk>
On 24/11/16 13:29, Russell King - ARM Linux wrote:
> On Thu, Nov 24, 2016 at 01:18:39PM +0000, Robin Murphy wrote:
>> Hi Liviu, Russell,
>>
>> I'd been meaning to try digging into this if it hadn't gone away since I
>> first noticed it, but I don't really have the time and it still happens
>> with 4.9-rc and today's -next. Representative splat below, but in
>> summary what happens is that if the HDLCD fails to probe, the TDA998x
>> connector seems to get cleaned up twice, resulting in a NULL dereference
>> the second time. I got as far as sketching out the following flow from a
>> debug session (on the same 4.8-rc2 kernel), but I don't know nearly
>> enough to tell which driver is at fault:
>>
>> hdlcd_drm_bind
>> -> drm_fbdev_cma_init (fails)
>> ...
>> -> drm_mode_config_cleanup
>> ...
>> -> drm_connector_cleanup
>> -> component_unbind_all
>> ...
>> -> tda998x_unbind
>> -> drm_connector_cleanup (NULL connector)
>>
>> It's easily reproduced on Juno by booting arm64 defconfig with
>> CONFIG_CMA_SIZE_MBYTES=1 and a sufficiently large monitor connected to
>> warrant a >1MB framebuffer.
>
> It looks to me like a hdlcd bug.
>
> The probe path operates in this order:
>
> - allocates hdlcd - 1
> - allocates drm device - 2
> - drm_mode_config_init - 3
> - hdlcd_load - 4
> - binds all components - 5
> - enables runtime PM - 6
> - drm_vblank_init - 7
> - drm_mode_config_reset - 8
> - drm_kms_helper_poll_init - 9
> - drm_fbdev_cma_init - 10
> - drm_dev_register - 11
>
> However, the cleanup operates in this order:
> - drm_fbdev_cma_fini - undoes 10
> - drm_kms_helper_poll_fini - undoes 9
> - drm_mode_config_cleanup - undoes 3
> - drm_vblank_cleanup - undoes 7
> - pm_runtime_disable - undoes 6
> - component_unbind_all - undoes 5
> - drm_irq_uninstall - undoes 4
> - of_reserved_mem_device_release - undoes other half of 4
> - drm_dev_unref - undoes 2
>
> Spot the step which is out of the correct order - drm_mode_config_cleanup()
> is misplaced - it's reversing the actions of drm_mode_config_init(), not
> drm_mode_config_reset().
Thanks for the explanation - that saves at least a day's worth of me
trying to understand DRM code :)
> So, drm_mode_config_cleanup() should be much later, after step 4 has
> been undone, otherwise there are paths that leave various DRM objects
> (created by drm_mode_create_standard_properties()) referenced, and
> will cause problems exactly like you're seeing here.
Liviu, can I leave this with you then?
Robin.
^ permalink raw reply
* [PATCH] arm64: mm: Fix memmap to be initialized for the entire section
From: Robert Richter @ 2016-11-24 13:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKv+Gu_Ug3XOD6jy5xziy1pS7WV=TC0RLD4j3DT6nytujQDzRQ@mail.gmail.com>
On 24.11.16 13:44:31, Ard Biesheuvel wrote:
> On 24 November 2016 at 13:42, Robert Richter <robert.richter@cavium.com> wrote:
> > On 23.11.16 21:25:06, Ard Biesheuvel wrote:
> >> Why? MEMREMAP_WB is used often, among other things for mapping
> >> firmware tables, which are marked as NOMAP, so in these cases, the
> >> linear address is not mapped.
> >
> > If fw tables are mapped wb, that is wrong and needs a separate fix.
> >
>
> Why is that wrong?
The whole issue with mapping acpi tables is not marking them cachable,
what wb does. Otherwise we could just use linear mapping for those mem
ranges.
> >> > If you think pfn_valid() is wrong here, I am happy to send a patch
> >> > that fixes this by using page_is_ram(). In any case, the worst case
> >> > that may happen is to behave the same as v4.4, we might fix then the
> >> > wrong use of pfn_valid() where it is not correctly used to check for
> >> > ram.
> >> >
> >>
> >> page_is_ram() uses string comparisons to look for regions called
> >> 'System RAM'. Is that something we can tolerate for each pfn_valid()
> >> calll?
> >>
> >> Perhaps the solution is to reimplement page_is_ram() for arm64 using
> >> memblock_is_memory() instead, But that still means we need to modify
> >> the generic memremap() code first to switch to it before changing the
> >> arm64 implementation of pfn_valid
> >
> > No, that's not the solution. pfn_valid() should just check if there is
> > a valid struct page, as other archs do. And if there is a mis-use of
> > pfn_valid() to check for ram, only that calls should be fixed to use
> > page_is_ram(), however this is implemented, or something appropriate.
> > But I don't see any problematic code, and if so, I will fix that.
> >
>
> memremap() uses pfn_valid() to decide whether some address is covered
> by the linear mapping. If we correct pfn_valid() to adhere to your
> definition, we will need to fix memremap() first in any case.
As said, will fix that if needed. But I think the caller is wrong
then.
-Robert
^ permalink raw reply
* [PATCH] arm64: mm: Fix memmap to be initialized for the entire section
From: Ard Biesheuvel @ 2016-11-24 13:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161124135151.GJ10776@rric.localdomain>
On 24 November 2016 at 13:51, Robert Richter <robert.richter@cavium.com> wrote:
> On 24.11.16 13:44:31, Ard Biesheuvel wrote:
>> On 24 November 2016 at 13:42, Robert Richter <robert.richter@cavium.com> wrote:
>> > On 23.11.16 21:25:06, Ard Biesheuvel wrote:
>> >> Why? MEMREMAP_WB is used often, among other things for mapping
>> >> firmware tables, which are marked as NOMAP, so in these cases, the
>> >> linear address is not mapped.
>> >
>> > If fw tables are mapped wb, that is wrong and needs a separate fix.
>> >
>>
>> Why is that wrong?
>
> The whole issue with mapping acpi tables is not marking them cachable,
> what wb does.
What 'issue'?
> Otherwise we could just use linear mapping for those mem
> ranges.
>
Regions containing firmware tables are owned by the firmware, and it
is the firmware that tells us which memory attributes we are allowed
to use. If those attributes include WB, it is perfectly legal to use a
cacheable mapping. That does *not* mean they should be covered by the
linear mapping. The linear mapping is read-write-non-exec, for
instance, and we may prefer to use a read-only mapping and/or
executable mapping.
>> >> > If you think pfn_valid() is wrong here, I am happy to send a patch
>> >> > that fixes this by using page_is_ram(). In any case, the worst case
>> >> > that may happen is to behave the same as v4.4, we might fix then the
>> >> > wrong use of pfn_valid() where it is not correctly used to check for
>> >> > ram.
>> >> >
>> >>
>> >> page_is_ram() uses string comparisons to look for regions called
>> >> 'System RAM'. Is that something we can tolerate for each pfn_valid()
>> >> calll?
>> >>
>> >> Perhaps the solution is to reimplement page_is_ram() for arm64 using
>> >> memblock_is_memory() instead, But that still means we need to modify
>> >> the generic memremap() code first to switch to it before changing the
>> >> arm64 implementation of pfn_valid
>> >
>> > No, that's not the solution. pfn_valid() should just check if there is
>> > a valid struct page, as other archs do. And if there is a mis-use of
>> > pfn_valid() to check for ram, only that calls should be fixed to use
>> > page_is_ram(), however this is implemented, or something appropriate.
>> > But I don't see any problematic code, and if so, I will fix that.
>> >
>>
>> memremap() uses pfn_valid() to decide whether some address is covered
>> by the linear mapping. If we correct pfn_valid() to adhere to your
>> definition, we will need to fix memremap() first in any case.
>
> As said, will fix that if needed. But I think the caller is wrong
> then.
>
^ permalink raw reply
* [PATCH 1/3] devicetree: bindings: pinctrl: Add binding for ti, da850-pupd
From: Linus Walleij @ 2016-11-24 14:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479871767-20160-2-git-send-email-david@lechnology.com>
On Wed, Nov 23, 2016 at 4:29 AM, David Lechner <david@lechnology.com> wrote:
> Device-tree bindings for TI DA8XX/OMAP-L138/AM18XX pullup/pulldown
> pinconf controller.
>
> Signed-off-by: David Lechner <david@lechnology.com>
Looks good to me.
Yours,
Linus Walleij
^ 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