* [PATCH v3 09/10] phy: Add Cadence D-PHY support
From: Maxime Ripard @ 2018-12-07 13:55 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Boris Brezillon
Cc: Archit Taneja, Rafal Ciepiela, Krzysztof Witos, Maxime Ripard,
linux-kernel, dri-devel, Andrzej Hajda, Chen-Yu Tsai,
Laurent Pinchart, Thomas Petazzoni, linux-arm-kernel, linux-media
In-Reply-To: <cover.ad7c4feb3905658f10b022df4756a5ade280011f.1544190837.git-series.maxime.ripard@bootlin.com>
Cadence has designed a D-PHY that can be used by the, currently in tree,
DSI bridge (DRM), CSI Transceiver and CSI Receiver (v4l2) drivers.
Only the DSI driver has an ad-hoc driver for that phy at the moment, while
the v4l2 drivers are completely missing any phy support. In order to make
that phy support available to all these drivers, without having to
duplicate that code three times, let's create a generic phy framework
driver.
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
drivers/phy/cadence/Kconfig | 13 +-
drivers/phy/cadence/Makefile | 1 +-
drivers/phy/cadence/cdns-dphy.c | 389 +++++++++++++++++++++++++++++++++-
3 files changed, 402 insertions(+), 1 deletion(-)
create mode 100644 drivers/phy/cadence/cdns-dphy.c
diff --git a/drivers/phy/cadence/Kconfig b/drivers/phy/cadence/Kconfig
index 2b8c0851ff33..31f18b67dd7c 100644
--- a/drivers/phy/cadence/Kconfig
+++ b/drivers/phy/cadence/Kconfig
@@ -1,6 +1,7 @@
#
# Phy drivers for Cadence PHYs
#
+
config PHY_CADENCE_DP
tristate "Cadence MHDP DisplayPort PHY driver"
depends on OF
@@ -9,9 +10,19 @@ config PHY_CADENCE_DP
help
Support for Cadence MHDP DisplayPort PHY.
+config PHY_CADENCE_DPHY
+ tristate "Cadence D-PHY Support"
+ depends on HAS_IOMEM && OF
+ select GENERIC_PHY
+ select GENERIC_PHY_MIPI_DPHY
+ help
+ Choose this option if you have a Cadence D-PHY in your
+ system. If M is selected, the module will be called
+ cdns-dphy.
+
config PHY_CADENCE_SIERRA
tristate "Cadence Sierra PHY Driver"
depends on OF && HAS_IOMEM && RESET_CONTROLLER
select GENERIC_PHY
help
- Enable this to support the Cadence Sierra PHY driver
\ No newline at end of file
+ Enable this to support the Cadence Sierra PHY driver
diff --git a/drivers/phy/cadence/Makefile b/drivers/phy/cadence/Makefile
index 412349af0492..2f9e3457b954 100644
--- a/drivers/phy/cadence/Makefile
+++ b/drivers/phy/cadence/Makefile
@@ -1,2 +1,3 @@
obj-$(CONFIG_PHY_CADENCE_DP) += phy-cadence-dp.o
+obj-$(CONFIG_PHY_CADENCE_DPHY) += cdns-dphy.o
obj-$(CONFIG_PHY_CADENCE_SIERRA) += phy-cadence-sierra.o
diff --git a/drivers/phy/cadence/cdns-dphy.c b/drivers/phy/cadence/cdns-dphy.c
new file mode 100644
index 000000000000..1d0abba03f37
--- /dev/null
+++ b/drivers/phy/cadence/cdns-dphy.c
@@ -0,0 +1,389 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright: 2017-2018 Cadence Design Systems, Inc.
+ */
+
+#include <linux/bitops.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+
+#include <linux/phy/phy.h>
+#include <linux/phy/phy-mipi-dphy.h>
+
+#define REG_WAKEUP_TIME_NS 800
+#define DPHY_PLL_RATE_HZ 108000000
+
+/* DPHY registers */
+#define DPHY_PMA_CMN(reg) (reg)
+#define DPHY_PMA_LCLK(reg) (0x100 + (reg))
+#define DPHY_PMA_LDATA(lane, reg) (0x200 + ((lane) * 0x100) + (reg))
+#define DPHY_PMA_RCLK(reg) (0x600 + (reg))
+#define DPHY_PMA_RDATA(lane, reg) (0x700 + ((lane) * 0x100) + (reg))
+#define DPHY_PCS(reg) (0xb00 + (reg))
+
+#define DPHY_CMN_SSM DPHY_PMA_CMN(0x20)
+#define DPHY_CMN_SSM_EN BIT(0)
+#define DPHY_CMN_TX_MODE_EN BIT(9)
+
+#define DPHY_CMN_PWM DPHY_PMA_CMN(0x40)
+#define DPHY_CMN_PWM_DIV(x) ((x) << 20)
+#define DPHY_CMN_PWM_LOW(x) ((x) << 10)
+#define DPHY_CMN_PWM_HIGH(x) (x)
+
+#define DPHY_CMN_FBDIV DPHY_PMA_CMN(0x4c)
+#define DPHY_CMN_FBDIV_VAL(low, high) (((high) << 11) | ((low) << 22))
+#define DPHY_CMN_FBDIV_FROM_REG (BIT(10) | BIT(21))
+
+#define DPHY_CMN_OPIPDIV DPHY_PMA_CMN(0x50)
+#define DPHY_CMN_IPDIV_FROM_REG BIT(0)
+#define DPHY_CMN_IPDIV(x) ((x) << 1)
+#define DPHY_CMN_OPDIV_FROM_REG BIT(6)
+#define DPHY_CMN_OPDIV(x) ((x) << 7)
+
+#define DPHY_PSM_CFG DPHY_PCS(0x4)
+#define DPHY_PSM_CFG_FROM_REG BIT(0)
+#define DPHY_PSM_CLK_DIV(x) ((x) << 1)
+
+#define DSI_HBP_FRAME_OVERHEAD 12
+#define DSI_HSA_FRAME_OVERHEAD 14
+#define DSI_HFP_FRAME_OVERHEAD 6
+#define DSI_HSS_VSS_VSE_FRAME_OVERHEAD 4
+#define DSI_BLANKING_FRAME_OVERHEAD 6
+#define DSI_NULL_FRAME_OVERHEAD 6
+#define DSI_EOT_PKT_SIZE 4
+
+struct cdns_dphy_cfg {
+ u8 pll_ipdiv;
+ u8 pll_opdiv;
+ u16 pll_fbdiv;
+ unsigned int nlanes;
+};
+
+enum cdns_dphy_clk_lane_cfg {
+ DPHY_CLK_CFG_LEFT_DRIVES_ALL = 0,
+ DPHY_CLK_CFG_LEFT_DRIVES_RIGHT = 1,
+ DPHY_CLK_CFG_LEFT_DRIVES_LEFT = 2,
+ DPHY_CLK_CFG_RIGHT_DRIVES_ALL = 3,
+};
+
+struct cdns_dphy;
+struct cdns_dphy_ops {
+ int (*probe)(struct cdns_dphy *dphy);
+ void (*remove)(struct cdns_dphy *dphy);
+ void (*set_psm_div)(struct cdns_dphy *dphy, u8 div);
+ void (*set_clk_lane_cfg)(struct cdns_dphy *dphy,
+ enum cdns_dphy_clk_lane_cfg cfg);
+ void (*set_pll_cfg)(struct cdns_dphy *dphy,
+ const struct cdns_dphy_cfg *cfg);
+ unsigned long (*get_wakeup_time_ns)(struct cdns_dphy *dphy);
+};
+
+struct cdns_dphy {
+ struct cdns_dphy_cfg cfg;
+ void __iomem *regs;
+ struct clk *psm_clk;
+ struct clk *pll_ref_clk;
+ const struct cdns_dphy_ops *ops;
+ struct phy *phy;
+};
+
+static int cdns_dsi_get_dphy_pll_cfg(struct cdns_dphy *dphy,
+ struct cdns_dphy_cfg *cfg,
+ struct phy_configure_opts_mipi_dphy *opts,
+ unsigned int *dsi_hfp_ext)
+{
+ unsigned long pll_ref_hz = clk_get_rate(dphy->pll_ref_clk);
+ u64 dlane_bps;
+
+ memset(cfg, 0, sizeof(*cfg));
+
+ if (pll_ref_hz < 9600000 || pll_ref_hz >= 150000000)
+ return -EINVAL;
+ else if (pll_ref_hz < 19200000)
+ cfg->pll_ipdiv = 1;
+ else if (pll_ref_hz < 38400000)
+ cfg->pll_ipdiv = 2;
+ else if (pll_ref_hz < 76800000)
+ cfg->pll_ipdiv = 4;
+ else
+ cfg->pll_ipdiv = 8;
+
+ dlane_bps = opts->hs_clk_rate;
+
+ if (dlane_bps > 2500000000UL || dlane_bps < 160000000UL)
+ return -EINVAL;
+ else if (dlane_bps >= 1250000000)
+ cfg->pll_opdiv = 1;
+ else if (dlane_bps >= 630000000)
+ cfg->pll_opdiv = 2;
+ else if (dlane_bps >= 320000000)
+ cfg->pll_opdiv = 4;
+ else if (dlane_bps >= 160000000)
+ cfg->pll_opdiv = 8;
+
+ cfg->pll_fbdiv = DIV_ROUND_UP_ULL(dlane_bps * 2 * cfg->pll_opdiv *
+ cfg->pll_ipdiv,
+ pll_ref_hz);
+
+ return 0;
+}
+
+static int cdns_dphy_setup_psm(struct cdns_dphy *dphy)
+{
+ unsigned long psm_clk_hz = clk_get_rate(dphy->psm_clk);
+ unsigned long psm_div;
+
+ if (!psm_clk_hz || psm_clk_hz > 100000000)
+ return -EINVAL;
+
+ psm_div = DIV_ROUND_CLOSEST(psm_clk_hz, 1000000);
+ if (dphy->ops->set_psm_div)
+ dphy->ops->set_psm_div(dphy, psm_div);
+
+ return 0;
+}
+
+static void cdns_dphy_set_clk_lane_cfg(struct cdns_dphy *dphy,
+ enum cdns_dphy_clk_lane_cfg cfg)
+{
+ if (dphy->ops->set_clk_lane_cfg)
+ dphy->ops->set_clk_lane_cfg(dphy, cfg);
+}
+
+static void cdns_dphy_set_pll_cfg(struct cdns_dphy *dphy,
+ const struct cdns_dphy_cfg *cfg)
+{
+ if (dphy->ops->set_pll_cfg)
+ dphy->ops->set_pll_cfg(dphy, cfg);
+}
+
+static unsigned long cdns_dphy_get_wakeup_time_ns(struct cdns_dphy *dphy)
+{
+ return dphy->ops->get_wakeup_time_ns(dphy);
+}
+
+static unsigned long cdns_dphy_ref_get_wakeup_time_ns(struct cdns_dphy *dphy)
+{
+ /* Default wakeup time is 800 ns (in a simulated environment). */
+ return 800;
+}
+
+static void cdns_dphy_ref_set_pll_cfg(struct cdns_dphy *dphy,
+ const struct cdns_dphy_cfg *cfg)
+{
+ u32 fbdiv_low, fbdiv_high;
+
+ fbdiv_low = (cfg->pll_fbdiv / 4) - 2;
+ fbdiv_high = cfg->pll_fbdiv - fbdiv_low - 2;
+
+ writel(DPHY_CMN_IPDIV_FROM_REG | DPHY_CMN_OPDIV_FROM_REG |
+ DPHY_CMN_IPDIV(cfg->pll_ipdiv) |
+ DPHY_CMN_OPDIV(cfg->pll_opdiv),
+ dphy->regs + DPHY_CMN_OPIPDIV);
+ writel(DPHY_CMN_FBDIV_FROM_REG |
+ DPHY_CMN_FBDIV_VAL(fbdiv_low, fbdiv_high),
+ dphy->regs + DPHY_CMN_FBDIV);
+ writel(DPHY_CMN_PWM_HIGH(6) | DPHY_CMN_PWM_LOW(0x101) |
+ DPHY_CMN_PWM_DIV(0x8),
+ dphy->regs + DPHY_CMN_PWM);
+}
+
+static void cdns_dphy_ref_set_psm_div(struct cdns_dphy *dphy, u8 div)
+{
+ writel(DPHY_PSM_CFG_FROM_REG | DPHY_PSM_CLK_DIV(div),
+ dphy->regs + DPHY_PSM_CFG);
+}
+
+/*
+ * This is the reference implementation of DPHY hooks. Specific integration of
+ * this IP may have to re-implement some of them depending on how they decided
+ * to wire things in the SoC.
+ */
+static const struct cdns_dphy_ops ref_dphy_ops = {
+ .get_wakeup_time_ns = cdns_dphy_ref_get_wakeup_time_ns,
+ .set_pll_cfg = cdns_dphy_ref_set_pll_cfg,
+ .set_psm_div = cdns_dphy_ref_set_psm_div,
+};
+
+static int cdns_dphy_config_from_opts(struct phy *phy,
+ struct phy_configure_opts_mipi_dphy *opts,
+ struct cdns_dphy_cfg *cfg)
+{
+ struct cdns_dphy *dphy = phy_get_drvdata(phy);
+ unsigned int dsi_hfp_ext = 0;
+ int ret;
+
+ ret = phy_mipi_dphy_config_validate(opts);
+ if (ret)
+ return ret;
+
+ ret = cdns_dsi_get_dphy_pll_cfg(dphy, cfg,
+ opts, &dsi_hfp_ext);
+ if (ret)
+ return ret;
+
+ opts->wakeup = cdns_dphy_get_wakeup_time_ns(dphy) * 1000;
+
+ return 0;
+}
+
+static int cdns_dphy_validate(struct phy *phy, enum phy_mode mode, int submode,
+ union phy_configure_opts *opts)
+{
+ struct cdns_dphy_cfg cfg = { 0 };
+
+ if (mode != PHY_MODE_MIPI_DPHY)
+ return -EINVAL;
+
+ return cdns_dphy_config_from_opts(phy, &opts->mipi_dphy, &cfg);
+}
+
+static int cdns_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
+{
+ struct cdns_dphy *dphy = phy_get_drvdata(phy);
+ struct cdns_dphy_cfg cfg = { 0 };
+ int ret;
+
+ ret = cdns_dphy_config_from_opts(phy, &opts->mipi_dphy, &cfg);
+ if (ret)
+ return ret;
+
+ /*
+ * Configure the internal PSM clk divider so that the DPHY has a
+ * 1MHz clk (or something close).
+ */
+ ret = cdns_dphy_setup_psm(dphy);
+ if (ret)
+ return ret;
+
+ /*
+ * Configure attach clk lanes to data lanes: the DPHY has 2 clk lanes
+ * and 8 data lanes, each clk lane can be attache different set of
+ * data lanes. The 2 groups are named 'left' and 'right', so here we
+ * just say that we want the 'left' clk lane to drive the 'left' data
+ * lanes.
+ */
+ cdns_dphy_set_clk_lane_cfg(dphy, DPHY_CLK_CFG_LEFT_DRIVES_LEFT);
+
+ /*
+ * Configure the DPHY PLL that will be used to generate the TX byte
+ * clk.
+ */
+ cdns_dphy_set_pll_cfg(dphy, &cfg);
+
+ return 0;
+}
+
+static int cdns_dphy_power_on(struct phy *phy)
+{
+ struct cdns_dphy *dphy = phy_get_drvdata(phy);
+
+ clk_prepare_enable(dphy->psm_clk);
+ clk_prepare_enable(dphy->pll_ref_clk);
+
+ /* Start TX state machine. */
+ writel(DPHY_CMN_SSM_EN | DPHY_CMN_TX_MODE_EN, dphy->regs + DPHY_CMN_SSM);
+
+ return 0;
+}
+
+static int cdns_dphy_power_off(struct phy *phy)
+{
+ struct cdns_dphy *dphy = phy_get_drvdata(phy);
+
+ clk_disable_unprepare(dphy->pll_ref_clk);
+ clk_disable_unprepare(dphy->psm_clk);
+
+ return 0;
+}
+
+static const struct phy_ops cdns_dphy_ops = {
+ .configure = cdns_dphy_configure,
+ .validate = cdns_dphy_validate,
+ .power_on = cdns_dphy_power_on,
+ .power_off = cdns_dphy_power_off,
+};
+
+static int cdns_dphy_probe(struct platform_device *pdev)
+{
+ struct phy_provider *phy_provider;
+ struct cdns_dphy *dphy;
+ struct resource *res;
+ int ret;
+
+ dphy = devm_kzalloc(&pdev->dev, sizeof(*dphy), GFP_KERNEL);
+ if (!dphy)
+ return -ENOMEM;
+ dev_set_drvdata(&pdev->dev, dphy);
+
+ dphy->ops = of_device_get_match_data(&pdev->dev);
+ if (!dphy->ops)
+ return -EINVAL;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ dphy->regs = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(dphy->regs))
+ return PTR_ERR(dphy->regs);
+
+ dphy->psm_clk = devm_clk_get(&pdev->dev, "psm");
+ if (IS_ERR(dphy->psm_clk))
+ return PTR_ERR(dphy->psm_clk);
+
+ dphy->pll_ref_clk = devm_clk_get(&pdev->dev, "pll_ref");
+ if (IS_ERR(dphy->pll_ref_clk))
+ return PTR_ERR(dphy->pll_ref_clk);
+
+ if (dphy->ops->probe) {
+ ret = dphy->ops->probe(dphy);
+ if (ret)
+ return ret;
+ }
+
+ dphy->phy = devm_phy_create(&pdev->dev, NULL, &cdns_dphy_ops);
+ if (IS_ERR(dphy->phy)) {
+ dev_err(&pdev->dev, "failed to create PHY\n");
+ if (dphy->ops->remove)
+ dphy->ops->remove(dphy);
+ return PTR_ERR(dphy->phy);
+ }
+
+ phy_set_drvdata(dphy->phy, dphy);
+ phy_provider = devm_of_phy_provider_register(&pdev->dev, of_phy_simple_xlate);
+
+ return PTR_ERR_OR_ZERO(phy_provider);
+}
+
+static int cdns_dphy_remove(struct platform_device *pdev)
+{
+ struct cdns_dphy *dphy = dev_get_drvdata(&pdev->dev);
+
+ if (dphy->ops->remove)
+ dphy->ops->remove(dphy);
+
+ return 0;
+}
+
+static const struct of_device_id cdns_dphy_of_match[] = {
+ { .compatible = "cdns,dphy", .data = &ref_dphy_ops },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, cdns_dphy_of_match);
+
+static struct platform_driver cdns_dphy_platform_driver = {
+ .probe = cdns_dphy_probe,
+ .remove = cdns_dphy_remove,
+ .driver = {
+ .name = "cdns-mipi-dphy",
+ .of_match_table = cdns_dphy_of_match,
+ },
+};
+module_platform_driver(cdns_dphy_platform_driver);
+
+MODULE_AUTHOR("Maxime Ripard <maxime.ripard@bootlin.com>");
+MODULE_DESCRIPTION("Cadence MIPI D-PHY Driver");
+MODULE_LICENSE("GPL");
--
git-series 0.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v3 03/10] phy: Add MIPI D-PHY configuration options
From: Maxime Ripard @ 2018-12-07 13:55 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Boris Brezillon
Cc: Archit Taneja, Rafal Ciepiela, Krzysztof Witos, Maxime Ripard,
linux-kernel, dri-devel, Andrzej Hajda, Chen-Yu Tsai,
Laurent Pinchart, Thomas Petazzoni, linux-arm-kernel, linux-media
In-Reply-To: <cover.ad7c4feb3905658f10b022df4756a5ade280011f.1544190837.git-series.maxime.ripard@bootlin.com>
Now that we have some infrastructure for it, allow the MIPI D-PHY phy's to
be configured through the generic functions through a custom structure
added to the generic union.
The parameters added here are the ones defined in the MIPI D-PHY spec, plus
the number of lanes in use. The current set of parameters should cover all
the potential users.
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
include/linux/phy/phy-mipi-dphy.h | 279 +++++++++++++++++++++++++++++++-
include/linux/phy/phy.h | 6 +-
2 files changed, 285 insertions(+)
create mode 100644 include/linux/phy/phy-mipi-dphy.h
diff --git a/include/linux/phy/phy-mipi-dphy.h b/include/linux/phy/phy-mipi-dphy.h
new file mode 100644
index 000000000000..29bf94db88ad
--- /dev/null
+++ b/include/linux/phy/phy-mipi-dphy.h
@@ -0,0 +1,279 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2018 Cadence Design Systems Inc.
+ */
+
+#ifndef __PHY_MIPI_DPHY_H_
+#define __PHY_MIPI_DPHY_H_
+
+#include <video/videomode.h>
+
+/**
+ * struct phy_configure_opts_mipi_dphy - MIPI D-PHY configuration set
+ *
+ * This structure is used to represent the configuration state of a
+ * MIPI D-PHY phy.
+ */
+struct phy_configure_opts_mipi_dphy {
+ /**
+ * @clk_miss:
+ *
+ * Timeout, in picoseconds, for receiver to detect absence of
+ * Clock transitions and disable the Clock Lane HS-RX.
+ *
+ * Maximum value: 60000 ps
+ */
+ unsigned int clk_miss;
+
+ /**
+ * @clk_post:
+ *
+ * Time, in picoseconds, that the transmitter continues to
+ * send HS clock after the last associated Data Lane has
+ * transitioned to LP Mode. Interval is defined as the period
+ * from the end of @hs_trail to the beginning of @clk_trail.
+ *
+ * Minimum value: 60000 ps + 52 * @hs_clk_rate period in ps
+ */
+ unsigned int clk_post;
+
+ /**
+ * @clk_pre:
+ *
+ * Time, in UI, that the HS clock shall be driven by
+ * the transmitter prior to any associated Data Lane beginning
+ * the transition from LP to HS mode.
+ *
+ * Minimum value: 8 UI
+ */
+ unsigned int clk_pre;
+
+ /**
+ * @clk_prepare:
+ *
+ * Time, in picoseconds, that the transmitter drives the Clock
+ * Lane LP-00 Line state immediately before the HS-0 Line
+ * state starting the HS transmission.
+ *
+ * Minimum value: 38000 ps
+ * Maximum value: 95000 ps
+ */
+ unsigned int clk_prepare;
+
+ /**
+ * @clk_settle:
+ *
+ * Time interval, in picoseconds, during which the HS receiver
+ * should ignore any Clock Lane HS transitions, starting from
+ * the beginning of @clk_prepare.
+ *
+ * Minimum value: 95000 ps
+ * Maximum value: 300000 ps
+ */
+ unsigned int clk_settle;
+
+ /**
+ * @clk_term_en:
+ *
+ * Time, in picoseconds, for the Clock Lane receiver to enable
+ * the HS line termination.
+ *
+ * Maximum value: 38000 ps
+ */
+ unsigned int clk_term_en;
+
+ /**
+ * @clk_trail:
+ *
+ * Time, in picoseconds, that the transmitter drives the HS-0
+ * state after the last payload clock bit of a HS transmission
+ * burst.
+ *
+ * Minimum value: 60000 ps
+ */
+ unsigned int clk_trail;
+
+ /**
+ * @clk_zero:
+ *
+ * Time, in picoseconds, that the transmitter drives the HS-0
+ * state prior to starting the Clock.
+ */
+ unsigned int clk_zero;
+
+ /**
+ * @d_term_en:
+ *
+ * Time, in picoseconds, for the Data Lane receiver to enable
+ * the HS line termination.
+ *
+ * Maximum value: 35000 ps + 4 * @hs_clk_rate period in ps
+ */
+ unsigned int d_term_en;
+
+ /**
+ * @eot:
+ *
+ * Transmitted time interval, in picoseconds, from the start
+ * of @hs_trail or @clk_trail, to the start of the LP- 11
+ * state following a HS burst.
+ *
+ * Maximum value: 105000 ps + 12 * @hs_clk_rate period in ps
+ */
+ unsigned int eot;
+
+ /**
+ * @hs_exit:
+ *
+ * Time, in picoseconds, that the transmitter drives LP-11
+ * following a HS burst.
+ *
+ * Minimum value: 100000 ps
+ */
+ unsigned int hs_exit;
+
+ /**
+ * @hs_prepare:
+ *
+ * Time, in picoseconds, that the transmitter drives the Data
+ * Lane LP-00 Line state immediately before the HS-0 Line
+ * state starting the HS transmission.
+ *
+ * Minimum value: 40000 ps + 4 * @hs_clk_rate period in ps
+ * Maximum value: 85000 ps + 6 * @hs_clk_rate period in ps
+ */
+ unsigned int hs_prepare;
+
+ /**
+ * @hs_settle:
+ *
+ * Time interval, in picoseconds, during which the HS receiver
+ * shall ignore any Data Lane HS transitions, starting from
+ * the beginning of @hs_prepare.
+ *
+ * Minimum value: 85000 ps + 6 * @hs_clk_rate period in ps
+ * Maximum value: 145000 ps + 10 * @hs_clk_rate period in ps
+ */
+ unsigned int hs_settle;
+
+ /**
+ * @hs_skip:
+ *
+ * Time interval, in picoseconds, during which the HS-RX
+ * should ignore any transitions on the Data Lane, following a
+ * HS burst. The end point of the interval is defined as the
+ * beginning of the LP-11 state following the HS burst.
+ *
+ * Minimum value: 40000 ps
+ * Maximum value: 55000 ps + 4 * @hs_clk_rate period in ps
+ */
+ unsigned int hs_skip;
+
+ /**
+ * @hs_trail:
+ *
+ * Time, in picoseconds, that the transmitter drives the
+ * flipped differential state after last payload data bit of a
+ * HS transmission burst
+ *
+ * Minimum value: max(8 * @hs_clk_rate period in ps,
+ * 60000 ps + 4 * @hs_clk_rate period in ps)
+ */
+ unsigned int hs_trail;
+
+ /**
+ * @hs_zero:
+ *
+ * Time, in picoseconds, that the transmitter drives the HS-0
+ * state prior to transmitting the Sync sequence.
+ */
+ unsigned int hs_zero;
+
+ /**
+ * @init:
+ *
+ * Time, in picoseconds for the initialization period to
+ * complete.
+ *
+ * Minimum value: 100000000 ps
+ */
+ unsigned int init;
+
+ /**
+ * @lpx:
+ *
+ * Transmitted length, in picoseconds, of any Low-Power state
+ * period.
+ *
+ * Minimum value: 50000 ps
+ */
+ unsigned int lpx;
+
+ /**
+ * @ta_get:
+ *
+ * Time, in picoseconds, that the new transmitter drives the
+ * Bridge state (LP-00) after accepting control during a Link
+ * Turnaround.
+ *
+ * Value: 5 * @lpx
+ */
+ unsigned int ta_get;
+
+ /**
+ * @ta_go:
+ *
+ * Time, in picoseconds, that the transmitter drives the
+ * Bridge state (LP-00) before releasing control during a Link
+ * Turnaround.
+ *
+ * Value: 4 * @lpx
+ */
+ unsigned int ta_go;
+
+ /**
+ * @ta_sure:
+ *
+ * Time, in picoseconds, that the new transmitter waits after
+ * the LP-10 state before transmitting the Bridge state
+ * (LP-00) during a Link Turnaround.
+ *
+ * Minimum value: @lpx
+ * Maximum value: 2 * @lpx
+ */
+ unsigned int ta_sure;
+
+ /**
+ * @wakeup:
+ *
+ * Time, in picoseconds, that a transmitter drives a Mark-1
+ * state prior to a Stop state in order to initiate an exit
+ * from ULPS.
+ *
+ * Minimum value: 1000000000 ps
+ */
+ unsigned int wakeup;
+
+ /**
+ * @hs_clk_rate:
+ *
+ * Clock rate, in Hertz, of the high-speed clock.
+ */
+ unsigned long hs_clk_rate;
+
+ /**
+ * @lp_clk_rate:
+ *
+ * Clock rate, in Hertz, of the low-power clock.
+ */
+ unsigned long lp_clk_rate;
+
+ /**
+ * @lanes:
+ *
+ * Number of active data lanes used for the transmissions.
+ */
+ unsigned char lanes;
+};
+
+#endif /* __PHY_MIPI_DPHY_H_ */
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 04476c026b5a..1fdefadf150a 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -20,6 +20,8 @@
#include <linux/pm_runtime.h>
#include <linux/regulator/consumer.h>
+#include <linux/phy/phy-mipi-dphy.h>
+
struct phy;
enum phy_mode {
@@ -44,8 +46,12 @@ enum phy_mode {
/**
* union phy_configure_opts - Opaque generic phy configuration
+ *
+ * @mipi_dphy: Configuration set applicable for phys supporting
+ * the MIPI_DPHY phy mode.
*/
union phy_configure_opts {
+ struct phy_configure_opts_mipi_dphy mipi_dphy;
};
/**
--
git-series 0.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v3 00/10] phy: Add configuration interface for MIPI D-PHY devices
From: Maxime Ripard @ 2018-12-07 13:55 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Boris Brezillon
Cc: Archit Taneja, Rafal Ciepiela, Krzysztof Witos, Maxime Ripard,
linux-kernel, dri-devel, Andrzej Hajda, Chen-Yu Tsai,
Laurent Pinchart, Thomas Petazzoni, linux-arm-kernel, linux-media
Hi,
Here is a set of patches to allow the phy framework consumers to test and
apply runtime configurations.
This is needed to support more phy classes that require tuning based on
parameters depending on the current use case of the device, in addition to
the power state management already provided by the current functions.
A first test bed for that API are the MIPI D-PHY devices. There's a number
of solutions that have been used so far to support these phy, most of the
time being an ad-hoc driver in the consumer.
That approach has a big shortcoming though, which is that this is quite
difficult to deal with consumers integrated with multiple variants of phy,
of multiple consumers integrated with the same phy.
The latter case can be found in the Cadence DSI bridge, and the CSI
transceiver and receivers. All of them are integrated with the same phy, or
can be integrated with different phy, depending on the implementation.
I've looked at all the MIPI DSI drivers I could find, and gathered all the
parameters I could find. The interface should be complete, and most of the
drivers can be converted in the future. The current set converts two of
them: the above mentionned Cadence DSI driver so that the v4l2 drivers can
use them, and the Allwinner MIPI-DSI driver.
Let me know what you think,
Maxime
Changes from v2:
- Rebased on next
- Changed the interface to accomodate for the new submodes
- Changed the timings units from nanoseconds to picoseconds
- Added minimum and maximum boundaries to the documentation
- Moved the clock enabling to phy_power_on in the Cadence DPHY driver
- Exported the phy_configure and phy_validate symbols
- Rework the phy pll divider computation in the cadence dphy driver
Changes from v1:
- Rebased on top of 4.20-rc1
- Removed the bus mode and timings parameters from the MIPI D-PHY
parameters, since that shouldn't have any impact on the PHY itself.
- Reworked the Cadence DSI and D-PHY drivers to take this into account.
- Remove the mode parameter from phy_configure
- Added phy_configure and phy_validate stubs
- Return -EOPNOTSUPP in phy_configure and phy_validate when the operation
is not implemented
Maxime Ripard (10):
phy: Add MIPI D-PHY mode
phy: Add configuration interface
phy: Add MIPI D-PHY configuration options
phy: dphy: Add configuration helpers
sun6i: dsi: Convert to generic phy handling
phy: Move Allwinner A31 D-PHY driver to drivers/phy/
drm/bridge: cdns: Separate DSI and D-PHY configuration
dt-bindings: phy: Move the Cadence D-PHY bindings
phy: Add Cadence D-PHY support
drm/bridge: cdns: Convert to phy framework
Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt | 21 +-
Documentation/devicetree/bindings/phy/cdns,dphy.txt | 20 +-
drivers/gpu/drm/bridge/cdns-dsi.c | 535 +------
drivers/gpu/drm/sun4i/Kconfig | 3 +-
drivers/gpu/drm/sun4i/Makefile | 5 +-
drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c | 292 +----
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 31 +-
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 17 +-
drivers/phy/Kconfig | 8 +-
drivers/phy/Makefile | 1 +-
drivers/phy/allwinner/Kconfig | 12 +-
drivers/phy/allwinner/Makefile | 1 +-
drivers/phy/allwinner/phy-sun6i-mipi-dphy.c | 318 ++++-
drivers/phy/cadence/Kconfig | 13 +-
drivers/phy/cadence/Makefile | 1 +-
drivers/phy/cadence/cdns-dphy.c | 389 +++++-
drivers/phy/phy-core-mipi-dphy.c | 166 ++-
drivers/phy/phy-core.c | 64 +-
include/linux/phy/phy-mipi-dphy.h | 285 ++++-
include/linux/phy/phy.h | 65 +-
20 files changed, 1468 insertions(+), 779 deletions(-)
create mode 100644 Documentation/devicetree/bindings/phy/cdns,dphy.txt
delete mode 100644 drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c
create mode 100644 drivers/phy/allwinner/phy-sun6i-mipi-dphy.c
create mode 100644 drivers/phy/cadence/cdns-dphy.c
create mode 100644 drivers/phy/phy-core-mipi-dphy.c
create mode 100644 include/linux/phy/phy-mipi-dphy.h
base-commit: 74c4a24df7cac1f9213a811d79558ecde23be9a2
--
git-series 0.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v3 02/10] phy: Add configuration interface
From: Maxime Ripard @ 2018-12-07 13:55 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Boris Brezillon
Cc: Archit Taneja, Rafal Ciepiela, Krzysztof Witos, Maxime Ripard,
linux-kernel, dri-devel, Andrzej Hajda, Chen-Yu Tsai,
Laurent Pinchart, Thomas Petazzoni, linux-arm-kernel, linux-media
In-Reply-To: <cover.ad7c4feb3905658f10b022df4756a5ade280011f.1544190837.git-series.maxime.ripard@bootlin.com>
The phy framework is only allowing to configure the power state of the PHY
using the init and power_on hooks, and their power_off and exit
counterparts.
While it works for most, simple, PHYs supported so far, some more advanced
PHYs need some configuration depending on runtime parameters. These PHYs
have been supported by a number of means already, often by using ad-hoc
drivers in their consumer drivers.
That doesn't work too well however, when a consumer device needs to deal
with multiple PHYs, or when multiple consumers need to deal with the same
PHY (a DSI driver and a CSI driver for example).
So we'll add a new interface, through two funtions, phy_validate and
phy_configure. The first one will allow to check that a current
configuration, for a given mode, is applicable. It will also allow the PHY
driver to tune the settings given as parameters as it sees fit.
phy_configure will actually apply that configuration in the phy itself.
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
drivers/phy/phy-core.c | 64 ++++++++++++++++++++++++++++++++++++++++++-
include/linux/phy/phy.h | 58 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 122 insertions(+)
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index df3d4ba516ab..19b05e824ee4 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -408,6 +408,70 @@ int phy_calibrate(struct phy *phy)
EXPORT_SYMBOL_GPL(phy_calibrate);
/**
+ * phy_configure() - Changes the phy parameters
+ * @phy: the phy returned by phy_get()
+ * @opts: New configuration to apply
+ *
+ * Used to change the PHY parameters. phy_init() must have been called
+ * on the phy. The configuration will be applied on the current phy
+ * mode, that can be changed using phy_set_mode().
+ *
+ * Returns: 0 if successful, an negative error code otherwise
+ */
+int phy_configure(struct phy *phy, union phy_configure_opts *opts)
+{
+ int ret;
+
+ if (!phy)
+ return -EINVAL;
+
+ if (!phy->ops->configure)
+ return -EOPNOTSUPP;
+
+ mutex_lock(&phy->mutex);
+ ret = phy->ops->configure(phy, opts);
+ mutex_unlock(&phy->mutex);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(phy_configure);
+
+/**
+ * phy_validate() - Checks the phy parameters
+ * @phy: the phy returned by phy_get()
+ * @mode: phy_mode the configuration is applicable to.
+ * @submode: PHY submode the configuration is applicable to.
+ * @opts: Configuration to check
+ *
+ * Used to check that the current set of parameters can be handled by
+ * the phy. Implementations are free to tune the parameters passed as
+ * arguments if needed by some implementation detail or
+ * constraints. It will not change any actual configuration of the
+ * PHY, so calling it as many times as deemed fit will have no side
+ * effect.
+ *
+ * Returns: 0 if successful, an negative error code otherwise
+ */
+int phy_validate(struct phy *phy, enum phy_mode mode, int submode,
+ union phy_configure_opts *opts)
+{
+ int ret;
+
+ if (!phy)
+ return -EINVAL;
+
+ if (!phy->ops->validate)
+ return -EOPNOTSUPP;
+
+ mutex_lock(&phy->mutex);
+ ret = phy->ops->validate(phy, mode, submode, opts);
+ mutex_unlock(&phy->mutex);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(phy_validate);
+
+/**
* _of_phy_get() - lookup and obtain a reference to a phy by phandle
* @np: device_node for which to get the phy
* @index: the index of the phy
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 453f21834685..04476c026b5a 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -43,6 +43,12 @@ enum phy_mode {
};
/**
+ * union phy_configure_opts - Opaque generic phy configuration
+ */
+union phy_configure_opts {
+};
+
+/**
* struct phy_ops - set of function pointers for performing phy operations
* @init: operation to be performed for initializing phy
* @exit: operation to be performed while exiting
@@ -59,6 +65,37 @@ struct phy_ops {
int (*power_on)(struct phy *phy);
int (*power_off)(struct phy *phy);
int (*set_mode)(struct phy *phy, enum phy_mode mode, int submode);
+
+ /**
+ * @configure:
+ *
+ * Optional.
+ *
+ * Used to change the PHY parameters. phy_init() must have
+ * been called on the phy.
+ *
+ * Returns: 0 if successful, an negative error code otherwise
+ */
+ int (*configure)(struct phy *phy, union phy_configure_opts *opts);
+
+ /**
+ * @validate:
+ *
+ * Optional.
+ *
+ * Used to check that the current set of parameters can be
+ * handled by the phy. Implementations are free to tune the
+ * parameters passed as arguments if needed by some
+ * implementation detail or constraints. It must not change
+ * any actual configuration of the PHY, so calling it as many
+ * times as deemed fit by the consumer must have no side
+ * effect.
+ *
+ * Returns: 0 if the configuration can be applied, an negative
+ * error code otherwise
+ */
+ int (*validate)(struct phy *phy, enum phy_mode mode, int submode,
+ union phy_configure_opts *opts);
int (*reset)(struct phy *phy);
int (*calibrate)(struct phy *phy);
struct module *owner;
@@ -165,6 +202,9 @@ int phy_power_off(struct phy *phy);
int phy_set_mode_ext(struct phy *phy, enum phy_mode mode, int submode);
#define phy_set_mode(phy, mode) \
phy_set_mode_ext(phy, mode, 0)
+int phy_configure(struct phy *phy, union phy_configure_opts *opts);
+int phy_validate(struct phy *phy, enum phy_mode mode, int submode,
+ union phy_configure_opts *opts);
static inline enum phy_mode phy_get_mode(struct phy *phy)
{
@@ -309,6 +349,24 @@ static inline int phy_calibrate(struct phy *phy)
return -ENOSYS;
}
+static inline int phy_configure(struct phy *phy,
+ union phy_configure_opts *opts)
+{
+ if (!phy)
+ return 0;
+
+ return -ENOSYS;
+}
+
+static inline int phy_validate(struct phy *phy, enum phy_mode mode, int submode,
+ union phy_configure_opts *opts)
+{
+ if (!phy)
+ return 0;
+
+ return -ENOSYS;
+}
+
static inline int phy_get_bus_width(struct phy *phy)
{
return -ENOSYS;
--
git-series 0.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v3 01/10] phy: Add MIPI D-PHY mode
From: Maxime Ripard @ 2018-12-07 13:55 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Boris Brezillon
Cc: Archit Taneja, Rafal Ciepiela, Krzysztof Witos, Maxime Ripard,
linux-kernel, dri-devel, Andrzej Hajda, Chen-Yu Tsai,
Laurent Pinchart, Thomas Petazzoni, linux-arm-kernel, linux-media
In-Reply-To: <cover.ad7c4feb3905658f10b022df4756a5ade280011f.1544190837.git-series.maxime.ripard@bootlin.com>
MIPI D-PHY is a MIPI standard meant mostly for display and cameras in
embedded systems. Add a mode for it.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
include/linux/phy/phy.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 79da05a3e28d..453f21834685 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -39,6 +39,7 @@ enum phy_mode {
PHY_MODE_UFS_HS_B,
PHY_MODE_PCIE,
PHY_MODE_ETHERNET,
+ PHY_MODE_MIPI_DPHY,
};
/**
--
git-series 0.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH 05/19] clk: tegra: dfll: registration for multiple SoCs
From: Jon Hunter @ 2018-12-07 13:55 UTC (permalink / raw)
To: Joseph Lo, Thierry Reding, Peter De Schrijver
Cc: linux-tegra, linux-clk, linux-arm-kernel
In-Reply-To: <20181204092548.3038-6-josephl@nvidia.com>
On 04/12/2018 09:25, Joseph Lo wrote:
> From: Peter De Schrijver <pdeschrijver@nvidia.com>
>
> In a future patch, support for the DFLL in Tegra210 will be introduced.
> This requires support for more than 1 set of CVB and CPU max frequency
> tables.
>
> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> ---
> drivers/clk/tegra/clk-tegra124-dfll-fcpu.c | 45 ++++++++++++++++------
> 1 file changed, 34 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c b/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
> index 269d3595758b..1a2cc113e5c8 100644
> --- a/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
> +++ b/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
> @@ -1,7 +1,7 @@
> /*
> * Tegra124 DFLL FCPU clock source driver
> *
> - * Copyright (C) 2012-2014 NVIDIA Corporation. All rights reserved.
> + * Copyright (C) 2012-2018 NVIDIA Corporation. All rights reserved.
> *
> * Aleksandr Frid <afrid@nvidia.com>
> * Paul Walmsley <pwalmsley@nvidia.com>
> @@ -21,6 +21,7 @@
> #include <linux/err.h>
> #include <linux/kernel.h>
> #include <linux/init.h>
> +#include <linux/of_device.h>
> #include <linux/platform_device.h>
> #include <soc/tegra/fuse.h>
>
> @@ -28,8 +29,15 @@
> #include "clk-dfll.h"
> #include "cvb.h"
>
> +struct dfll_fcpu_data {
> + const unsigned long *cpu_max_freq_table;
> + unsigned int cpu_max_freq_table_size;
> + const struct cvb_table *cpu_cvb_tables;
> + unsigned int cpu_cvb_tables_size;
> +};
> +
> /* Maximum CPU frequency, indexed by CPU speedo id */
> -static const unsigned long cpu_max_freq_table[] = {
> +static const unsigned long tegra124_cpu_max_freq_table[] = {
> [0] = 2014500000UL,
> [1] = 2320500000UL,
> [2] = 2116500000UL,
> @@ -82,16 +90,36 @@ static const struct cvb_table tegra124_cpu_cvb_tables[] = {
> },
> };
>
> +static const struct dfll_fcpu_data tegra124_dfll_fcpu_data = {
> + .cpu_max_freq_table = tegra124_cpu_max_freq_table,
> + .cpu_max_freq_table_size = ARRAY_SIZE(tegra124_cpu_max_freq_table),
> + .cpu_cvb_tables = tegra124_cpu_cvb_tables,
> + .cpu_cvb_tables_size = ARRAY_SIZE(tegra124_cpu_cvb_tables)
> +};
> +
> +static const struct of_device_id tegra124_dfll_fcpu_of_match[] = {
> + {
> + .compatible = "nvidia,tegra124-dfll",
> + .data = &tegra124_dfll_fcpu_data,
> + },
> + { },
> +};
> +
> static int tegra124_dfll_fcpu_probe(struct platform_device *pdev)
> {
> int process_id, speedo_id, speedo_value, err;
> struct tegra_dfll_soc_data *soc;
> + const struct dfll_fcpu_data *fcpu_data;
> +
> + fcpu_data = of_device_get_match_data(&pdev->dev);
> + if (!fcpu_data)
> + return -ENODEV;
>
> process_id = tegra_sku_info.cpu_process_id;
> speedo_id = tegra_sku_info.cpu_speedo_id;
> speedo_value = tegra_sku_info.cpu_speedo_value;
>
> - if (speedo_id >= ARRAY_SIZE(cpu_max_freq_table)) {
> + if (speedo_id >= fcpu_data->cpu_max_freq_table_size) {
> dev_err(&pdev->dev, "unknown max CPU freq for speedo_id=%d\n",
> speedo_id);
> return -ENODEV;
> @@ -107,10 +135,10 @@ static int tegra124_dfll_fcpu_probe(struct platform_device *pdev)
> return -ENODEV;
> }
>
> - soc->max_freq = cpu_max_freq_table[speedo_id];
> + soc->max_freq = fcpu_data->cpu_max_freq_table[speedo_id];
>
> - soc->cvb = tegra_cvb_add_opp_table(soc->dev, tegra124_cpu_cvb_tables,
> - ARRAY_SIZE(tegra124_cpu_cvb_tables),
> + soc->cvb = tegra_cvb_add_opp_table(soc->dev, fcpu_data->cpu_cvb_tables,
> + fcpu_data->cpu_cvb_tables_size,
> process_id, speedo_id, speedo_value,
> soc->max_freq);
> if (IS_ERR(soc->cvb)) {
> @@ -142,11 +170,6 @@ static int tegra124_dfll_fcpu_remove(struct platform_device *pdev)
> return 0;
> }
>
> -static const struct of_device_id tegra124_dfll_fcpu_of_match[] = {
> - { .compatible = "nvidia,tegra124-dfll", },
> - { },
> -};
> -
> static const struct dev_pm_ops tegra124_dfll_pm_ops = {
> SET_RUNTIME_PM_OPS(tegra_dfll_runtime_suspend,
> tegra_dfll_runtime_resume, NULL)
>
Acked-by: Jon Hunter <jonathanh@nvidia.com>
Cheers
Jon
--
nvpublic
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 04/19] dt-bindings: cpufreq: tegra124: remove cpu_lp clock from required properties
From: Jon Hunter @ 2018-12-07 13:53 UTC (permalink / raw)
To: Joseph Lo, Thierry Reding, Peter De Schrijver
Cc: linux-tegra, devicetree, linux-clk, linux-arm-kernel
In-Reply-To: <20181204092548.3038-5-josephl@nvidia.com>
On 04/12/2018 09:25, Joseph Lo wrote:
> The cpu_lp clock property is only needed when the CPUfreq driver
> supports CPU cluster switching. But it was not a design for this driver
> and it didn't handle that as well. So removing this property.
>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> ---
> .../devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt
> index 031545a29caf..03196d5ea515 100644
> --- a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt
> +++ b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt
> @@ -9,7 +9,6 @@ Required properties:
> See ../clocks/clock-bindings.txt for details.
> - clock-names: Must include the following entries:
> - cpu_g: Clock mux for the fast CPU cluster.
> - - cpu_lp: Clock mux for the low-power CPU cluster.
> - pll_x: Fast PLL clocksource.
> - pll_p: Auxiliary PLL used during fast PLL rate changes.
> - dfll: Fast DFLL clocksource that also automatically scales CPU voltage.
> @@ -30,11 +29,10 @@ cpus {
> reg = <0>;
>
> clocks = <&tegra_car TEGRA124_CLK_CCLK_G>,
> - <&tegra_car TEGRA124_CLK_CCLK_LP>,
> <&tegra_car TEGRA124_CLK_PLL_X>,
> <&tegra_car TEGRA124_CLK_PLL_P>,
> <&dfll>;
> - clock-names = "cpu_g", "cpu_lp", "pll_x", "pll_p", "dfll";
> + clock-names = "cpu_g", "pll_x", "pll_p", "dfll";
> clock-latency = <300000>;
> };
>
>
Acked-by: Jon Hunter <jonathanh@nvidia.com>
Cheers
Jon
--
nvpublic
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 03/19] dt-bindings: cpufreq: tegra124: remove vdd-cpu-supply from required properties
From: Jon Hunter @ 2018-12-07 13:52 UTC (permalink / raw)
To: Joseph Lo, Thierry Reding, Peter De Schrijver
Cc: linux-tegra, devicetree, linux-clk, linux-arm-kernel
In-Reply-To: <20181204092548.3038-4-josephl@nvidia.com>
On 04/12/2018 09:25, Joseph Lo wrote:
> The Tegra124 cpufreq driver works only with DFLL clock, which is a
> hardware-based frequency/voltage controller. The driver doesn't need to
> control the regulator itself. Hence remove that.
>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> ---
> .../devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt
> index b1669fbfb740..031545a29caf 100644
> --- a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt
> +++ b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt
> @@ -13,7 +13,6 @@ Required properties:
> - pll_x: Fast PLL clocksource.
> - pll_p: Auxiliary PLL used during fast PLL rate changes.
> - dfll: Fast DFLL clocksource that also automatically scales CPU voltage.
> -- vdd-cpu-supply: Regulator for CPU voltage
>
> Optional properties:
> - clock-latency: Specify the possible maximum transition latency for clock,
> @@ -37,7 +36,6 @@ cpus {
> <&dfll>;
> clock-names = "cpu_g", "cpu_lp", "pll_x", "pll_p", "dfll";
> clock-latency = <300000>;
> - vdd-cpu-supply: <&vdd_cpu>;
> };
>
> <...>
>
Acked-by: Jon Hunter <jonathanh@nvidia.com>
Cheers
Jon
--
nvpublic
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 02/19] dt-bindings: clock: tegra124-dfll: add Tegra210 support
From: Jon Hunter @ 2018-12-07 13:50 UTC (permalink / raw)
To: Joseph Lo, Thierry Reding, Peter De Schrijver
Cc: linux-tegra, devicetree, linux-clk, linux-arm-kernel
In-Reply-To: <20181204092548.3038-3-josephl@nvidia.com>
On 04/12/2018 09:25, Joseph Lo wrote:
> Add Tegra210 support for DFLL clock.
>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> ---
> .../devicetree/bindings/clock/nvidia,tegra124-dfll.txt | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
> index 8c97600d2bad..4bd44dd7ec1e 100644
> --- a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
> +++ b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
> @@ -10,7 +10,9 @@ control module that will automatically adjust the VDD_CPU voltage by
> communicating with an off-chip PMIC either via an I2C bus or via PWM signals.
>
> Required properties:
> -- compatible : should be "nvidia,tegra124-dfll"
> +- compatible : should be one of:
> + - "nvidia,tegra124-dfll": for Tegra124
> + - "nvidia,tegra210-dfll": for Tegra210
> - reg : Defines the following set of registers, in the order listed:
> - registers for the DFLL control logic.
> - registers for the I2C output logic.
Acked-by: Jon Hunter <jonathanh@nvidia.com>
Cheers
Jon
--
nvpublic
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFT PATCH v1 0/4] Unify CPU topology across ARM64 & RISC-V
From: Morten Rasmussen @ 2018-12-07 13:45 UTC (permalink / raw)
To: Atish Patra
Cc: Mark Rutland, devicetree, Albert Ou, Thomas Gleixner, Juri Lelli,
Ard Biesheuvel, Dmitriy Cherkasov, Anup Patel, Palmer Dabbelt,
Will Deacon, linux-kernel, Jeremy Linton, Sudeep Holla,
Peter Zijlstra (Intel), Rob Herring, Greg Kroah-Hartman,
Catalin Marinas, Rafael J. Wysocki, linux-riscv, Ingo Molnar,
moderated list:ARM64 PORT (AARCH64 ARCHITECTURE)
In-Reply-To: <1543534100-3654-1-git-send-email-atish.patra@wdc.com>
Hi,
On Thu, Nov 29, 2018 at 03:28:16PM -0800, Atish Patra wrote:
> The cpu-map DT entry in ARM64 can describe the CPU topology in
> much better way compared to other existing approaches. RISC-V can
> easily adopt this binding to represent it's own CPU topology.
> Thus, both cpu-map DT binding and topology parsing code can be
> moved to a common location so that RISC-V or any other
> architecture can leverage that.
>
> The relevant discussion regarding unifying cpu topology can be
> found in [1].
>
> arch_topology seems to be a perfect place to move the common
> code. I have not introduced any functional changes in the moved
> code. The only downside in this approach is that the capacity
> code will be executed for RISC-V as well. But, it will exit
> immediately after not able to find the appropriate DT node. If
> the overhead is considered too much, we can always compile out
> capacity related functions under a different config for the
> architectures that do not support them.
>
> The patches have been tested for RISC-V and compile tested for
> ARM64 & x86.
The cpu-map bindings are used for arch/arm too, and so is
arch_topology.c. In fact, it was introduced to allow code-sharing
between arm and arm64. Applying patch three breaks arm.
Moving the DT parsing to arch_topology.c we have to unify all three
architectures. Be aware that arm and arm64 have some differences in how
they detect cpu capacities. I think we might have to look at the split
of code between arch/* and arch_topology.c again :-/
Morten
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 1/4] arm64: hyperv: Add core Hyper-V include files
From: Will Deacon @ 2018-12-07 13:42 UTC (permalink / raw)
To: kys
Cc: mark.rutland, olaf, sthemmin, marc.zyngier, catalin.marinas,
jasowang, linux-kernel, Michael Kelley, Michael.H.Kelley, gregkh,
apw, devel, vkuznets, linux-arm-kernel
In-Reply-To: <20181122031059.16338-1-kys@linuxonhyperv.com>
Hi all,
On Thu, Nov 22, 2018 at 03:10:56AM +0000, kys@linuxonhyperv.com wrote:
> From: Michael Kelley <mikelley@microsoft.com>
>
> hyperv-tlfs.h defines Hyper-V interfaces from the Hyper-V Top Level
> Functional Spec (TLFS). The TLFS is distinctly oriented to x86/x64,
> and Hyper-V has not separated out the architecture-dependent parts into
> x86/x64 vs. ARM64. So hyperv-tlfs.h includes information for ARM64
> that is not yet formally published. The TLFS is available here:
When do you plan to publish the spec? It's pretty hard to review this stuff
without knowing what it's supposed to look like.
> docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/reference/tlfs
>
> mshyperv.h defines Linux-specific structures and routines for
> interacting with Hyper-V. It is split into an ARM64 specific file
> and an architecture independent file in include/asm-generic.
>
> Signed-off-by: Michael Kelley <mikelley@microsoft.com>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
> MAINTAINERS | 3 +
> arch/arm64/include/asm/hyperv-tlfs.h | 338 +++++++++++++++++++++++++++
> arch/arm64/include/asm/mshyperv.h | 116 +++++++++
> include/asm-generic/mshyperv.h | 240 +++++++++++++++++++
> 4 files changed, 697 insertions(+)
> create mode 100644 arch/arm64/include/asm/hyperv-tlfs.h
> create mode 100644 arch/arm64/include/asm/mshyperv.h
> create mode 100644 include/asm-generic/mshyperv.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f4855974f325..72f19cef4c48 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6835,6 +6835,8 @@ F: arch/x86/include/asm/trace/hyperv.h
> F: arch/x86/include/asm/hyperv-tlfs.h
> F: arch/x86/kernel/cpu/mshyperv.c
> F: arch/x86/hyperv
> +F: arch/arm64/include/asm/hyperv-tlfs.h
> +F: arch/arm64/include/asm/mshyperv.h
> F: drivers/hid/hid-hyperv.c
> F: drivers/hv/
> F: drivers/input/serio/hyperv-keyboard.c
> @@ -6846,6 +6848,7 @@ F: drivers/video/fbdev/hyperv_fb.c
> F: net/vmw_vsock/hyperv_transport.c
> F: include/linux/hyperv.h
> F: include/uapi/linux/hyperv.h
> +F: include/asm-generic/mshyperv.h
> F: tools/hv/
> F: Documentation/ABI/stable/sysfs-bus-vmbus
>
> diff --git a/arch/arm64/include/asm/hyperv-tlfs.h b/arch/arm64/include/asm/hyperv-tlfs.h
> new file mode 100644
> index 000000000000..924e37600e92
> --- /dev/null
> +++ b/arch/arm64/include/asm/hyperv-tlfs.h
> @@ -0,0 +1,338 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +/*
> + * This file contains definitions from the Hyper-V Hypervisor Top-Level
> + * Functional Specification (TLFS):
> + * https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdocs.microsoft.com%2Fen-us%2Fvirtualization%2Fhyper-v-on-windows%2Freference%2Ftlfs&data=02%7C01%7Ckys%40microsoft.com%7Cc831a45fd63e4a4b083908d641216aa8%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636768009113747528&sdata=jRSrs9ZWXdmeS7LQUEpoSyUfBS7a5KLYy%2FolFdE2tI0%3D&reserved=0
As mentioned elsewhere, please use a better link here and drop the license
boilerplate below.
> + *
> + * Copyright (C) 2018, Microsoft, Inc.
> + *
> + * Author : Michael Kelley <mikelley@microsoft.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published
> + * by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
> + * NON INFRINGEMENT. See the GNU General Public License for more
> + * details.
> + */
> +
> +#ifndef _ASM_ARM64_HYPERV_H
> +#define _ASM_ARM64_HYPERV_H
__ASM_HYPER_V_H please
> +
> +#include <linux/types.h>
> +
> +/*
> + * These Hyper-V registers provide information equivalent to the CPUID
> + * instruction on x86/x64.
> + */
> +#define HV_REGISTER_HYPERVISOR_VERSION 0x00000100 /*CPUID 0x40000002 */
> +#define HV_REGISTER_PRIVILEGES_AND_FEATURES 0x00000200 /*CPUID 0x40000003 */
> +#define HV_REGISTER_FEATURES 0x00000201 /*CPUID 0x40000004 */
> +#define HV_REGISTER_IMPLEMENTATION_LIMITS 0x00000202 /*CPUID 0x40000005 */
> +#define HV_ARM64_REGISTER_INTERFACE_VERSION 0x00090006 /*CPUID 0x40000001 */
> +
> +/*
> + * Feature identification. HvRegisterPrivilegesAndFeaturesInfo returns a
> + * 128-bit value with flags indicating which features are available to the
> + * partition based upon the current partition privileges. The 128-bit
> + * value is broken up with different portions stored in different 32-bit
> + * fields in the ms_hyperv structure.
> + */
> +
> +/* Partition Reference Counter available*/
> +#define HV_MSR_TIME_REF_COUNT_AVAILABLE (1 << 1)
> +
> +/*
> + * Synthetic Timers available
> + */
> +#define HV_MSR_SYNTIMER_AVAILABLE (1 << 3)
> +
> +/* Frequency MSRs available */
> +#define HV_FEATURE_FREQUENCY_MSRS_AVAILABLE (1 << 8)
> +
> +/* Reference TSC available */
> +#define HV_MSR_REFERENCE_TSC_AVAILABLE (1 << 9)
> +
> +/* Crash MSR available */
> +#define HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE (1 << 10)
> +
> +
> +/*
> + * This group of flags is in the high order 64-bits of the returned
> + * 128-bit value.
> + */
> +
> +/* STIMER direct mode is available */
> +#define HV_STIMER_DIRECT_MODE_AVAILABLE (1 << 19)
> +
> +/*
> + * Implementation recommendations in register
> + * HvRegisterFeaturesInfo. Indicates which behaviors the hypervisor
> + * recommends the OS implement for optimal performance.
> + */
> +
> +/*
> + * Recommend not using Auto EOI
> + */
> +#define HV_DEPRECATING_AEOI_RECOMMENDED (1 << 9)
> +
> +/*
> + * Synthetic register definitions equivalent to MSRs on x86/x64
> + */
> +#define HV_REGISTER_CRASH_P0 0x00000210
> +#define HV_REGISTER_CRASH_P1 0x00000211
> +#define HV_REGISTER_CRASH_P2 0x00000212
> +#define HV_REGISTER_CRASH_P3 0x00000213
> +#define HV_REGISTER_CRASH_P4 0x00000214
> +#define HV_REGISTER_CRASH_CTL 0x00000215
> +
> +#define HV_REGISTER_GUEST_OSID 0x00090002
> +#define HV_REGISTER_VPINDEX 0x00090003
> +#define HV_REGISTER_TIME_REFCOUNT 0x00090004
> +#define HV_REGISTER_REFERENCE_TSC 0x00090017
> +
> +#define HV_REGISTER_SINT0 0x000A0000
> +#define HV_REGISTER_SINT1 0x000A0001
> +#define HV_REGISTER_SINT2 0x000A0002
> +#define HV_REGISTER_SINT3 0x000A0003
> +#define HV_REGISTER_SINT4 0x000A0004
> +#define HV_REGISTER_SINT5 0x000A0005
> +#define HV_REGISTER_SINT6 0x000A0006
> +#define HV_REGISTER_SINT7 0x000A0007
> +#define HV_REGISTER_SINT8 0x000A0008
> +#define HV_REGISTER_SINT9 0x000A0009
> +#define HV_REGISTER_SINT10 0x000A000A
> +#define HV_REGISTER_SINT11 0x000A000B
> +#define HV_REGISTER_SINT12 0x000A000C
> +#define HV_REGISTER_SINT13 0x000A000D
> +#define HV_REGISTER_SINT14 0x000A000E
> +#define HV_REGISTER_SINT15 0x000A000F
> +#define HV_REGISTER_SCONTROL 0x000A0010
> +#define HV_REGISTER_SVERSION 0x000A0011
> +#define HV_REGISTER_SIFP 0x000A0012
> +#define HV_REGISTER_SIPP 0x000A0013
> +#define HV_REGISTER_EOM 0x000A0014
> +#define HV_REGISTER_SIRBP 0x000A0015
> +
> +#define HV_REGISTER_STIMER0_CONFIG 0x000B0000
> +#define HV_REGISTER_STIMER0_COUNT 0x000B0001
> +#define HV_REGISTER_STIMER1_CONFIG 0x000B0002
> +#define HV_REGISTER_STIMER1_COUNT 0x000B0003
> +#define HV_REGISTER_STIMER2_CONFIG 0x000B0004
> +#define HV_REGISTER_STIMER2_COUNT 0x000B0005
> +#define HV_REGISTER_STIMER3_CONFIG 0x000B0006
> +#define HV_REGISTER_STIMER3_COUNT 0x000B0007
> +
> +/*
> + * Crash notification flags.
> + */
> +#define HV_CRASH_CTL_CRASH_NOTIFY_MSG BIT_ULL(62)
> +#define HV_CRASH_CTL_CRASH_NOTIFY BIT_ULL(63)
> +
> +/*
> + * The guest OS needs to register the guest ID with the hypervisor.
> + * The guest ID is a 64 bit entity and the structure of this ID is
> + * specified in the Hyper-V specification:
> + *
> + * msdn.microsoft.com/en-us/library/windows/hardware/ff542653%28v=vs.85%29.aspx
Dead link :(
> + *
> + * While the current guideline does not specify how Linux guest ID(s)
> + * need to be generated, our plan is to publish the guidelines for
> + * Linux and other guest operating systems that currently are hosted
> + * on Hyper-V. The implementation here conforms to this yet
> + * unpublished guidelines.
Again, any timeline on publishing this stuff? Right now, this is just a file
full of magic numbers and I'm not sure we're in a good position to maintain
it based on the idea that you have a cunning plan.
> + *
> + *
> + * Bit(s)
> + * 63 - Indicates if the OS is Open Source or not; 1 is Open Source
> + * 62:56 - Os Type; Linux is 0x100
> + * 55:48 - Distro specific identification
> + * 47:16 - Linux kernel version number
> + * 15:0 - Distro specific identification
> + *
> + *
> + */
> +#define HV_LINUX_VENDOR_ID 0x8100
> +
> +/* Declare the various hypercall operations. */
> +#define HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE 0x0002
> +#define HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST 0x0003
> +#define HVCALL_NOTIFY_LONG_SPIN_WAIT 0x0008
> +#define HVCALL_SEND_IPI 0x000b
> +#define HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE_EX 0x0013
> +#define HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST_EX 0x0014
> +#define HVCALL_SEND_IPI_EX 0x0015
> +#define HVCALL_GET_VP_REGISTERS 0x0050
> +#define HVCALL_SET_VP_REGISTERS 0x0051
> +#define HVCALL_POST_MESSAGE 0x005c
> +#define HVCALL_SIGNAL_EVENT 0x005d
> +#define HVCALL_RETARGET_INTERRUPT 0x007e
> +#define HVCALL_START_VIRTUAL_PROCESSOR 0x0099
> +#define HVCALL_GET_VP_INDEX_FROM_APICID 0x009a
This all sounds very x86y...
> +/* Declare standard hypercall field values. */
> +#define HV_PARTITION_ID_SELF ((u64)-1)
> +#define HV_VP_INDEX_SELF ((u32)-2)
> +
> +#define HV_HYPERCALL_FAST_BIT BIT(16)
> +#define HV_HYPERCALL_REP_COUNT_1 BIT_ULL(32)
> +#define HV_HYPERCALL_RESULT_MASK GENMASK_ULL(15, 0)
> +
> +/* Define the hypercall status result */
> +
> +union hv_hypercall_status {
> + u64 as_uint64;
> + struct {
> + u16 status;
> + u16 reserved;
> + u16 reps_completed; /* Low 12 bits */
> + u16 reserved2;
> + };
> +};
> +
> +/* hypercall status code */
> +#define HV_STATUS_SUCCESS 0
> +#define HV_STATUS_INVALID_HYPERCALL_CODE 2
> +#define HV_STATUS_INVALID_HYPERCALL_INPUT 3
> +#define HV_STATUS_INVALID_ALIGNMENT 4
> +#define HV_STATUS_INSUFFICIENT_MEMORY 11
> +#define HV_STATUS_INVALID_CONNECTION_ID 18
> +#define HV_STATUS_INSUFFICIENT_BUFFERS 19
> +
> +/* Define output layout for Get VP Register hypercall */
> +struct hv_get_vp_register_output {
> + u64 registervaluelow;
> + u64 registervaluehigh;
> +};
> +
> +#define HV_FLUSH_ALL_PROCESSORS BIT(0)
> +#define HV_FLUSH_ALL_VIRTUAL_ADDRESS_SPACES BIT(1)
> +#define HV_FLUSH_NON_GLOBAL_MAPPINGS_ONLY BIT(2)
> +#define HV_FLUSH_USE_EXTENDED_RANGE_FORMAT BIT(3)
> +
> +enum HV_GENERIC_SET_FORMAT {
> + HV_GENERIC_SET_SPARSE_4K,
> + HV_GENERIC_SET_ALL,
> +};
> +
> +/*
> + * The Hyper-V TimeRefCount register and the TSC
> + * page provide a guest VM clock with 100ns tick rate
> + */
> +#define HV_CLOCK_HZ (NSEC_PER_SEC/100)
> +
> +/*
> + * The fields in this structure are set by Hyper-V and read
> + * by the Linux guest. They should be accessed with READ_ONCE()
> + * so the compiler doesn't optimize in a way that will cause
> + * problems.
> + */
> +struct ms_hyperv_tsc_page {
> + u32 tsc_sequence;
> + u32 reserved1;
> + u64 tsc_scale;
> + s64 tsc_offset;
> + u64 reserved2[509];
> +};
It might be clearer to express this as a union with the page size:
struct ms_hyperv_tsc_page {
union {
struct {
__u32 tsc_sequence;
__u32 reserved1;
};
__u8 reserved2[HV_HYP_PAGE_SIZE];
};
};
What's the required alignment on this structure?
Also, do you intend for 32-bit guests to use this ABI as well?
> +
> +/* Define the number of synthetic interrupt sources. */
> +#define HV_SYNIC_SINT_COUNT (16)
> +/* Define the expected SynIC version. */
> +#define HV_SYNIC_VERSION_1 (0x1)
> +
> +#define HV_SYNIC_CONTROL_ENABLE (1ULL << 0)
> +#define HV_SYNIC_SIMP_ENABLE (1ULL << 0)
> +#define HV_SYNIC_SIEFP_ENABLE (1ULL << 0)
> +#define HV_SYNIC_SINT_MASKED (1ULL << 16)
> +#define HV_SYNIC_SINT_AUTO_EOI (1ULL << 17)
> +#define HV_SYNIC_SINT_VECTOR_MASK (0xFF)
> +
> +#define HV_SYNIC_STIMER_COUNT (4)
> +
> +/* Define synthetic interrupt controller message constants. */
> +#define HV_MESSAGE_SIZE (256)
> +#define HV_MESSAGE_PAYLOAD_BYTE_COUNT (240)
> +#define HV_MESSAGE_PAYLOAD_QWORD_COUNT (30)
> +
> +/* Define hypervisor message types. */
> +enum hv_message_type {
> + HVMSG_NONE = 0x00000000,
> +
> + /* Memory access messages. */
> + HVMSG_UNMAPPED_GPA = 0x80000000,
> + HVMSG_GPA_INTERCEPT = 0x80000001,
> +
> + /* Timer notification messages. */
> + HVMSG_TIMER_EXPIRED = 0x80000010,
> +
> + /* Error messages. */
> + HVMSG_INVALID_VP_REGISTER_VALUE = 0x80000020,
> + HVMSG_UNRECOVERABLE_EXCEPTION = 0x80000021,
> + HVMSG_UNSUPPORTED_FEATURE = 0x80000022,
> +
> + /* Trace buffer complete messages. */
> + HVMSG_EVENTLOG_BUFFERCOMPLETE = 0x80000040,
> +};
> +
> +/* Define synthetic interrupt controller message flags. */
> +union hv_message_flags {
> + __u8 asu8;
> + struct {
> + __u8 msg_pending:1;
> + __u8 reserved:7;
> + };
> +};
> +
> +/* Define port identifier type. */
> +union hv_port_id {
> + __u32 asu32;
> + struct {
> + __u32 id:24;
> + __u32 reserved:8;
> + } u;
> +};
Hmm, I thought bitfields weren't a good idea for this sort of thing. Isn't
it up to the compiler how they are laid out in memory?
> +
> +/* Define synthetic interrupt controller message header. */
> +struct hv_message_header {
> + __u32 message_type;
> + __u8 payload_size;
> + union hv_message_flags message_flags;
> + __u8 reserved[2];
> + union {
> + __u64 sender;
> + union hv_port_id port;
> + };
> +};
> +
> +/* Define synthetic interrupt controller message format. */
> +struct hv_message {
> + struct hv_message_header header;
> + union {
> + __u64 payload[HV_MESSAGE_PAYLOAD_QWORD_COUNT];
> + } u;
> +};
> +
> +/* Define the synthetic interrupt message page layout. */
> +struct hv_message_page {
> + struct hv_message sint_message[HV_SYNIC_SINT_COUNT];
> +};
> +
> +/* Define timer message payload structure. */
> +struct hv_timer_message_payload {
> + __u32 timer_index;
> + __u32 reserved;
> + __u64 expiration_time; /* When the timer expired */
> + __u64 delivery_time; /* When the message was delivered */
> +};
> +
> +#define HV_STIMER_ENABLE (1ULL << 0)
> +#define HV_STIMER_PERIODIC (1ULL << 1)
> +#define HV_STIMER_LAZY (1ULL << 2)
> +#define HV_STIMER_AUTOENABLE (1ULL << 3)
> +#define HV_STIMER_SINT(config) (__u8)(((config) >> 16) & 0x0F)
> +
> +#endif
> diff --git a/arch/arm64/include/asm/mshyperv.h b/arch/arm64/include/asm/mshyperv.h
> new file mode 100644
> index 000000000000..a87c431d58b3
> --- /dev/null
> +++ b/arch/arm64/include/asm/mshyperv.h
> @@ -0,0 +1,116 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +/*
> + * Linux-specific definitions for managing interactions with Microsoft's
> + * Hyper-V hypervisor. The definitions in this file are specific to
> + * the ARM64 architecture. See include/asm-generic/mshyperv.h for
> + * definitions are that architecture independent.
> + *
> + * Definitions that are specified in the Hyper-V Top Level Functional
> + * Spec (TLFS) should not go in this file, but should instead go in
> + * hyperv-tlfs.h.
> + *
> + * Copyright (C) 2018, Microsoft, Inc.
> + *
> + * Author : Michael Kelley <mikelley@microsoft.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published
> + * by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
> + * NON INFRINGEMENT. See the GNU General Public License for more
> + * details.
> + */
> +
> +#ifndef _ASM_ARM64_MSHYPERV_H
> +#define _ASM_ARM64_MSHYPERV_H
> +
> +#include <linux/types.h>
> +#include <linux/interrupt.h>
> +#include <linux/clocksource.h>
> +#include <linux/irq.h>
> +#include <linux/irqdesc.h>
> +#include <asm/hyperv-tlfs.h>
> +
> +/*
> + * Define the IRQ numbers/vectors used by Hyper-V VMbus interrupts
> + * and by STIMER0 Direct Mode interrupts. Hyper-V should be supplying
> + * these values through ACPI, but there are no other interrupting
> + * devices in a Hyper-V VM on ARM64, so it's OK to hard code for now.
> + * The "CALLBACK_VECTOR" terminology is a left-over from the x86/x64
> + * world that is used in architecture independent Hyper-V code.
> + */
> +#define HYPERVISOR_CALLBACK_VECTOR 16
> +#define HV_STIMER0_IRQNR 17
> +
> +extern u64 hv_do_hvc(u64 control, ...);
> +extern u64 hv_do_hvc_fast_get(u64 control, u64 input1, u64 input2, u64 input3,
> + struct hv_get_vp_register_output *output);
> +
> +/*
> + * Declare calls to get and set Hyper-V VP register values on ARM64, which
> + * requires a hypercall.
> + */
> +extern void hv_set_vpreg(u32 reg, u64 value);
> +extern u64 hv_get_vpreg(u32 reg);
> +extern void hv_get_vpreg_128(u32 reg, struct hv_get_vp_register_output *result);
> +
> +/*
> + * Use the Hyper-V provided stimer0 as the timer that is made
> + * available to the architecture independent Hyper-V drivers.
> + */
> +#define hv_init_timer(timer, tick) \
> + hv_set_vpreg(HV_REGISTER_STIMER0_COUNT + (2*timer), tick)
> +#define hv_init_timer_config(timer, val) \
> + hv_set_vpreg(HV_REGISTER_STIMER0_CONFIG + (2*timer), val)
> +#define hv_get_current_tick(tick) \
> + (tick = hv_get_vpreg(HV_REGISTER_TIME_REFCOUNT))
> +
> +#define hv_get_simp(val) (val = hv_get_vpreg(HV_REGISTER_SIPP))
> +#define hv_set_simp(val) hv_set_vpreg(HV_REGISTER_SIPP, val)
> +
> +#define hv_get_siefp(val) (val = hv_get_vpreg(HV_REGISTER_SIFP))
> +#define hv_set_siefp(val) hv_set_vpreg(HV_REGISTER_SIFP, val)
> +
> +#define hv_get_synic_state(val) (val = hv_get_vpreg(HV_REGISTER_SCONTROL))
> +#define hv_set_synic_state(val) hv_set_vpreg(HV_REGISTER_SCONTROL, val)
> +
> +#define hv_get_vp_index(index) (index = hv_get_vpreg(HV_REGISTER_VPINDEX))
> +
> +#define hv_signal_eom() hv_set_vpreg(HV_REGISTER_EOM, 0)
> +
> +/*
> + * Hyper-V SINT registers are numbered sequentially, so we can just
> + * add the SINT number to the register number of SINT0
> + */
> +#define hv_get_synint_state(sint_num, val) \
> + (val = hv_get_vpreg(HV_REGISTER_SINT0 + sint_num))
> +#define hv_set_synint_state(sint_num, val) \
> + hv_set_vpreg(HV_REGISTER_SINT0 + sint_num, val)
> +
> +#define hv_get_crash_ctl(val) \
> + (val = hv_get_vpreg(HV_REGISTER_CRASH_CTL))
> +
> +#if IS_ENABLED(CONFIG_HYPERV)
> +#define hv_enable_stimer0_percpu_irq(irq) enable_percpu_irq(irq, 0)
> +#define hv_disable_stimer0_percpu_irq(irq) disable_percpu_irq(irq)
> +#endif
> +
> +/* ARM64 specific code to read the hardware clock */
> +static inline u64 hv_read_hwclock(void)
> +{
> + u64 result;
> +
> + isb();
> + result = read_sysreg(cntvct_el0);
> + isb();
> +
> + return result;
> +}
Shouldn't you use arch_timer_read_counter() or arch_counter_get_cntvct()
instead?
> +#include <asm-generic/mshyperv.h>
> +
> +#endif
> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
> new file mode 100644
> index 000000000000..fbe1ec89c85a
> --- /dev/null
> +++ b/include/asm-generic/mshyperv.h
> @@ -0,0 +1,240 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +/*
> + * Linux-specific definitions for managing interactions with Microsoft's
> + * Hyper-V hypervisor. The definitions in this file are architecture
> + * independent. See arch/<arch>/include/asm/mshyperv.h for definitions
> + * that are specific to architecture <arch>.
> + *
> + * Definitions that are specified in the Hyper-V Top Level Functional
> + * Spec (TLFS) should not go in this file, but should instead go in
> + * hyperv-tlfs.h.
> + *
> + * Copyright (C) 2018, Microsoft, Inc.
> + *
> + * Author : Michael Kelley <mikelley@microsoft.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published
> + * by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
> + * NON INFRINGEMENT. See the GNU General Public License for more
> + * details.
> + */
> +
> +#ifndef _ASM_GENERIC_MSHYPERV_H
> +#define _ASM_GENERIC_MSHYPERV_H
> +
> +#include <linux/types.h>
> +#include <linux/interrupt.h>
> +#include <linux/clocksource.h>
> +#include <linux/irq.h>
> +#include <linux/irqdesc.h>
> +#include <asm/hyperv-tlfs.h>
> +
> +/*
> + * Hyper-V always runs with a page size of 4096. These definitions
> + * are used when communicating with Hyper-V using guest physical
> + * pages and guest physical page addresses, since the guest page
> + * size may not be 4096 on ARM64.
> + */
> +#define HV_HYP_PAGE_SIZE 4096
> +#define HV_HYP_PAGE_SHIFT 12
Maybe define the page size in terms of the shift?
> +#define HV_HYP_PAGE_MASK (~(HV_HYP_PAGE_SIZE - 1))
> +
> +
> +struct ms_hyperv_info {
> + u32 features;
> + u32 misc_features;
> + u32 hints;
> + u32 max_vp_index;
> + u32 max_lp_index;
> +};
What is the define endianness for in-memory structures shared with the
hypervisor?
> +extern struct ms_hyperv_info ms_hyperv;
> +
> +extern u64 hv_do_hypercall(u64 control, void *inputaddr, void *outputaddr);
> +extern u64 hv_do_fast_hypercall8(u16 control, u64 input8);
> +
> +/*
> + * The guest OS needs to register the guest ID with the hypervisor.
> + * The guest ID is a 64 bit entity and the structure of this ID is
> + * specified in the Hyper-V specification:
> + *
> + * msdn.microsoft.com/en-us/library/windows/hardware/ff542653%28v=vs.85%29.aspx
> + *
> + * While the current guideline does not specify how Linux guest ID(s)
> + * need to be generated, our plan is to publish the guidelines for
> + * Linux and other guest operating systems that currently are hosted
> + * on Hyper-V. The implementation here conforms to this yet
> + * unpublished guidelines.
> + *
> + *
> + * Bit(s)
> + * 63 - Indicates if the OS is Open Source or not; 1 is Open Source
> + * 62:56 - Os Type; Linux is 0x100
> + * 55:48 - Distro specific identification
> + * 47:16 - Linux kernel version number
> + * 15:0 - Distro specific identification
This looks familiar...
> + * Generate the guest ID based on the guideline described above.
> + */
> +
> +static inline __u64 generate_guest_id(__u64 d_info1, __u64 kernel_version,
> + __u64 d_info2)
> +{
> + __u64 guest_id = 0;
> +
> + guest_id = (((__u64)HV_LINUX_VENDOR_ID) << 48);
> + guest_id |= (d_info1 << 48);
> + guest_id |= (kernel_version << 16);
> + guest_id |= d_info2;
> +
> + return guest_id;
> +}
> +
> +
> +/* Free the message slot and signal end-of-message if required */
> +static inline void vmbus_signal_eom(struct hv_message *msg, u32 old_msg_type)
> +{
> + /*
> + * On crash we're reading some other CPU's message page and we need
> + * to be careful: this other CPU may already had cleared the header
> + * and the host may already had delivered some other message there.
> + * In case we blindly write msg->header.message_type we're going
> + * to lose it. We can still lose a message of the same type but
> + * we count on the fact that there can only be one
> + * CHANNELMSG_UNLOAD_RESPONSE and we don't care about other messages
> + * on crash.
> + */
> + if (cmpxchg(&msg->header.message_type, old_msg_type,
> + HVMSG_NONE) != old_msg_type)
> + return;
> +
> + /*
> + * Make sure the write to MessageType (ie set to
> + * HVMSG_NONE) happens before we read the
> + * MessagePending and EOMing. Otherwise, the EOMing
> + * will not deliver any more messages since there is
> + * no empty slot
> + */
> + mb();
A successful cmpxchg() already implies full barriers, so this isn't needed.
I also wonder whether cmpxchg_acquire() would be sufficient here.
> +
> + if (msg->header.message_flags.msg_pending) {
> + /*
> + * This will cause message queue rescan to
> + * possibly deliver another msg from the
> + * hypervisor
> + */
> + hv_signal_eom();
> + }
> +}
> +
> +void hv_setup_vmbus_irq(void (*handler)(void));
> +void hv_remove_vmbus_irq(void);
> +void hv_enable_vmbus_irq(void);
> +void hv_disable_vmbus_irq(void);
> +
> +void hv_setup_kexec_handler(void (*handler)(void));
> +void hv_remove_kexec_handler(void);
> +void hv_setup_crash_handler(void (*handler)(struct pt_regs *regs));
> +void hv_remove_crash_handler(void);
> +
> +#if IS_ENABLED(CONFIG_HYPERV)
> +extern struct clocksource *hyperv_cs;
> +
> +/*
> + * Hypervisor's notion of virtual processor ID is different from
> + * Linux' notion of CPU ID. This information can only be retrieved
> + * in the context of the calling CPU. Setup a map for easy access
> + * to this information.
> + */
> +extern u32 *hv_vp_index;
> +extern u32 hv_max_vp_index;
> +
> +/* Sentinel value for an uninitialized entry in hv_vp_index array */
> +#define VP_INVAL U32_MAX
> +
> +/**
> + * hv_cpu_number_to_vp_number() - Map CPU to VP.
> + * @cpu_number: CPU number in Linux terms
> + *
> + * This function returns the mapping between the Linux processor
> + * number and the hypervisor's virtual processor number, useful
> + * in making hypercalls and such that talk about specific
> + * processors.
> + *
> + * Return: Virtual processor number in Hyper-V terms
> + */
> +static inline int hv_cpu_number_to_vp_number(int cpu_number)
> +{
> + return hv_vp_index[cpu_number];
> +}
> +
> +void hyperv_report_panic(struct pt_regs *regs, long err);
> +void hyperv_report_panic_msg(phys_addr_t pa, size_t size);
> +bool hv_is_hyperv_initialized(void);
> +void hyperv_cleanup(void);
> +#else /* CONFIG_HYPERV */
> +static inline bool hv_is_hyperv_initialized(void) { return false; }
> +static inline void hyperv_cleanup(void) {}
> +#endif /* CONFIG_HYPERV */
> +
> +#if IS_ENABLED(CONFIG_HYPERV)
> +extern int hv_setup_stimer0_irq(int *irq, int *vector, void (*handler)(void));
> +extern void hv_remove_stimer0_irq(int irq);
> +#endif
> +
> +static inline u64 hv_read_tsc_page_tsc(const struct ms_hyperv_tsc_page *tsc_pg,
> + u64 *cur_tsc)
> +{
> + u64 scale, offset;
> + u32 sequence;
> +
> + /*
> + * The protocol for reading Hyper-V TSC page is specified in Hypervisor
> + * Top-Level Functional Specification. To get the reference time we
> + * must do the following:
> + * - READ ReferenceTscSequence
> + * A special '0' value indicates the time source is unreliable and we
> + * need to use something else.
> + * - ReferenceTime =
> + * ((HWclock val) * ReferenceTscScale) >> 64) + ReferenceTscOffset
> + * - READ ReferenceTscSequence again. In case its value has changed
> + * since our first reading we need to discard ReferenceTime and repeat
> + * the whole sequence as the hypervisor was updating the page in
> + * between.
> + */
> + do {
> + sequence = READ_ONCE(tsc_pg->tsc_sequence);
> + /*
> + * Make sure we read sequence before we read other values from
> + * TSC page.
> + */
> + smp_rmb();
> +
> + scale = READ_ONCE(tsc_pg->tsc_scale);
> + offset = READ_ONCE(tsc_pg->tsc_offset);
> + *cur_tsc = hv_read_hwclock();
> +
> + /*
> + * Make sure we read sequence after we read all other values
> + * from TSC page.
> + */
> + smp_rmb();
> +
> + } while (READ_ONCE(tsc_pg->tsc_sequence) != sequence);
Could you explain what the writer side looks like, please? I'm a bit
confused as to why we're not using the bottom bit of the sequence
number to detect a concurrent update.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 2/4] arm64: hyperv: Add support for Hyper-V as a hypervisor
From: Will Deacon @ 2018-12-07 13:42 UTC (permalink / raw)
To: kys
Cc: mark.rutland, olaf, sthemmin, marc.zyngier, catalin.marinas,
jasowang, linux-kernel, Michael Kelley, Michael.H.Kelley, gregkh,
apw, devel, vkuznets, linux-arm-kernel
In-Reply-To: <20181122031059.16338-2-kys@linuxonhyperv.com>
On Thu, Nov 22, 2018 at 03:10:57AM +0000, kys@linuxonhyperv.com wrote:
> From: Michael Kelley <mikelley@microsoft.com>
>
> Add ARM64-specific code to enable Hyper-V. This code includes:
> * Detecting Hyper-V and initializing the guest/Hyper-V interface
> * Setting up Hyper-V's synthetic clocks
> * Making hypercalls using the HVC instruction
> * Setting up VMbus and stimer0 interrupts
> * Setting up kexec and crash handlers
> This code is architecture dependent code and is mostly driven by
> architecture independent code in the VMbus driver in drivers/hv/hv.c
> and drivers/hv/vmbus_drv.c.
>
> This code is built only when CONFIG_HYPERV is enabled.
>
> Signed-off-by: Michael Kelley <mikelley@microsoft.com>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
> MAINTAINERS | 1 +
> arch/arm64/Makefile | 1 +
> arch/arm64/hyperv/Makefile | 2 +
> arch/arm64/hyperv/hv_hvc.S | 54 +++++
> arch/arm64/hyperv/hv_init.c | 441 +++++++++++++++++++++++++++++++++++
> arch/arm64/hyperv/mshyperv.c | 178 ++++++++++++++
> 6 files changed, 677 insertions(+)
> create mode 100644 arch/arm64/hyperv/Makefile
> create mode 100644 arch/arm64/hyperv/hv_hvc.S
> create mode 100644 arch/arm64/hyperv/hv_init.c
> create mode 100644 arch/arm64/hyperv/mshyperv.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 72f19cef4c48..326eeb32a0cd 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6837,6 +6837,7 @@ F: arch/x86/kernel/cpu/mshyperv.c
> F: arch/x86/hyperv
> F: arch/arm64/include/asm/hyperv-tlfs.h
> F: arch/arm64/include/asm/mshyperv.h
> +F: arch/arm64/hyperv
> F: drivers/hid/hid-hyperv.c
> F: drivers/hv/
> F: drivers/input/serio/hyperv-keyboard.c
> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> index 6cb9fc7e9382..ad9ec0579553 100644
> --- a/arch/arm64/Makefile
> +++ b/arch/arm64/Makefile
> @@ -106,6 +106,7 @@ core-y += arch/arm64/kernel/ arch/arm64/mm/
> core-$(CONFIG_NET) += arch/arm64/net/
> core-$(CONFIG_KVM) += arch/arm64/kvm/
> core-$(CONFIG_XEN) += arch/arm64/xen/
> +core-$(CONFIG_HYPERV) += arch/arm64/hyperv/
> core-$(CONFIG_CRYPTO) += arch/arm64/crypto/
> libs-y := arch/arm64/lib/ $(libs-y)
> core-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a
> diff --git a/arch/arm64/hyperv/Makefile b/arch/arm64/hyperv/Makefile
> new file mode 100644
> index 000000000000..988eda55330c
> --- /dev/null
> +++ b/arch/arm64/hyperv/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0
> +obj-y := hv_init.o hv_hvc.o mshyperv.o
> diff --git a/arch/arm64/hyperv/hv_hvc.S b/arch/arm64/hyperv/hv_hvc.S
> new file mode 100644
> index 000000000000..82636969b4f2
> --- /dev/null
> +++ b/arch/arm64/hyperv/hv_hvc.S
> @@ -0,0 +1,54 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +/*
> + * Microsoft Hyper-V hypervisor invocation routines
> + *
> + * Copyright (C) 2018, Microsoft, Inc.
> + *
> + * Author : Michael Kelley <mikelley@microsoft.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published
> + * by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
> + * NON INFRINGEMENT. See the GNU General Public License for more
> + * details.
> + */
> +
> +#include <linux/linkage.h>
> +
> + .text
> +/*
> + * Do the HVC instruction. For Hyper-V the argument is always 1.
> + * x0 contains the hypercall control value, while additional registers
> + * vary depending on the hypercall, and whether the hypercall arguments
> + * are in memory or in registers (a "fast" hypercall per the Hyper-V
> + * TLFS). When the arguments are in memory x1 is the guest physical
> + * address of the input arguments, and x2 is the guest physical
> + * address of the output arguments. When the arguments are in
> + * registers, the register values depends on the hypercall. Note
> + * that this version cannot return any values in registers.
> + */
> +ENTRY(hv_do_hvc)
> + hvc #1
> + ret
> +ENDPROC(hv_do_hvc)
> +
> +/*
> + * This variant of HVC invocation is for hv_get_vpreg and
> + * hv_get_vpreg_128. The input parameters are passed in registers
> + * along with a pointer in x4 to where the output result should
> + * be stored. The output is returned in x15 and x16. x18 is used as
> + * scratch space to avoid buildng a stack frame, as Hyper-V does
> + * not preserve registers x0-x17.
> + */
> +ENTRY(hv_do_hvc_fast_get)
> + mov x18, x4
> + hvc #1
> + str x15,[x18]
> + str x16,[x18,#8]
> + ret
> +ENDPROC(hv_do_hvc_fast_get)
Why are you not following the ARM SMCCC? It would be /much/ better if you
could just follow the standard, which is already implemented by
include/linux/smccc.h.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 01/19] dt-bindings: clock: tegra124-dfll: Update DFLL binding for PWM regulator
From: Jon Hunter @ 2018-12-07 13:41 UTC (permalink / raw)
To: Joseph Lo, Thierry Reding, Peter De Schrijver
Cc: linux-tegra, devicetree, linux-clk, linux-arm-kernel
In-Reply-To: <20181204092548.3038-2-josephl@nvidia.com>
On 04/12/2018 09:25, Joseph Lo wrote:
> From: Peter De Schrijver <pdeschrijver@nvidia.com>
>
> Add new properties to configure the DFLL PWM regulator support. Also
> add an example and make the I2C clock only required when I2C support is
> used.
>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> ---
> .../bindings/clock/nvidia,tegra124-dfll.txt | 73 ++++++++++++++++++-
> 1 file changed, 71 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
> index dff236f524a7..8c97600d2bad 100644
> --- a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
> +++ b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
> @@ -8,7 +8,6 @@ the fast CPU cluster. It consists of a free-running voltage controlled
> oscillator connected to the CPU voltage rail (VDD_CPU), and a closed loop
> control module that will automatically adjust the VDD_CPU voltage by
> communicating with an off-chip PMIC either via an I2C bus or via PWM signals.
> -Currently only the I2C mode is supported by these bindings.
>
> Required properties:
> - compatible : should be "nvidia,tegra124-dfll"
> @@ -45,10 +44,28 @@ Required properties for the control loop parameters:
> Optional properties for the control loop parameters:
> - nvidia,cg-scale: Boolean value, see the field DFLL_PARAMS_CG_SCALE in the TRM.
>
> +Optional properties for mode selection:
> +- nvidia,pwm-to-pmic: Use PWM to control regulator rather then I2C.
> +
> Required properties for I2C mode:
> - nvidia,i2c-fs-rate: I2C transfer rate, if using full speed mode.
>
> -Example:
> +Required properties for PWM mode:
> +- nvidia,pwm-period: period of PWM square wave in microseconds.
> +- nvidia,init-uv: Regulator voltage in micro volts when PWM control is disabled.
Maybe consider 'pwm-inactive-voltage-microvolt'.
> +- nvidia,align-offset-uv: Regulator voltage in micro volts when PWM control is
> + enabled and PWM output is low.
Would this be considered the minimum pwm active voltage?
> +- nvidia,align-step-uv: Voltage increase in micro volts corresponding to a
> + 1/33th increase in duty cycle. Eg the voltage for 2/33th
> + duty cycle would be:
Maybe consider 'pwm-voltage-step-microvolt'.
> + nvidia,align-offset-uv + nvidia,align-step-uv * 2.
> +- pinctrl-0: I/O pad configuration when PWM control is enabled.
> +- pinctrl-1: I/O pad configuration when PWM control is disabled.
> +- pinctrl-names: must include the following entries:
> + - dvfs_pwm_enable: I/O pad configuration when PWM control is enabled.
> + - dvfs_pwm_disable: I/O pad configuration when PWM control is disabled.
Please see Rob's feedback on the above [0].
Cheers
Jon
[0] https://lore.kernel.org/patchwork/patch/885328/
--
nvpublic
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2] media: cedrus: don't initialize pointers with zero
From: Mauro Carvalho Chehab @ 2018-12-07 13:41 UTC (permalink / raw)
To: Paul Kocialkowski
Cc: devel, Maxime Ripard, Greg Kroah-Hartman, Mauro Carvalho Chehab,
Chen-Yu Tsai, linux-arm-kernel, Linux Media Mailing List
In-Reply-To: <c426c7ea8159349f3cc23bf7d65d2c24f2ade00e.camel@bootlin.com>
Em Fri, 07 Dec 2018 14:21:44 +0100
Paul Kocialkowski <paul.kocialkowski@bootlin.com> escreveu:
> Hi,
>
> On Fri, 2018-12-07 at 08:03 -0500, Mauro Carvalho Chehab wrote:
> > A common mistake is to assume that initializing a var with:
> > struct foo f = { 0 };
> >
> > Would initialize a zeroed struct. Actually, what this does is
> > to initialize the first element of the struct to zero.
> >
> > According to C99 Standard 6.7.8.21:
> >
> > "If there are fewer initializers in a brace-enclosed
> > list than there are elements or members of an aggregate,
> > or fewer characters in a string literal used to initialize
> > an array of known size than there are elements in the array,
> > the remainder of the aggregate shall be initialized implicitly
> > the same as objects that have static storage duration."
> >
> > So, in practice, it could zero the entire struct, but, if the
> > first element is not an integer, it will produce warnings:
> >
> > drivers/staging/media/sunxi/cedrus/cedrus.c:drivers/staging/media/sunxi/cedrus/cedrus.c:78:49: warning: Using plain integer as NULL pointer
> > drivers/staging/media/sunxi/cedrus/cedrus_dec.c:drivers/staging/media/sunxi/cedrus/cedrus_dec.c:29:35: warning: Using plain integer as NULL pointer
> >
> > As the right initialization would be, instead:
> >
> > struct foo f = { NULL };
>
> Thanks for sharing these details, it's definitely interesting and good
> to know :)
Yeah, that's something that was bothering for quite a while, as I've
seen patches using either one of the ways. It took me a while to
do some research, and having it documented at the patch helps, as
we should now handle it the same way for similar stuff :-)
>
> > Another way to initialize it with gcc is to use:
> >
> > struct foo f = {};
> >
> > That seems to be a gcc extension, but clang also does the right thing,
> > and that's a clean way for doing it.
> >
> > Anyway, I decided to check upstream what's the most commonly pattern.
> > The "= {}" pattern has about 2000 entries:
> >
> > $ git grep -E "=\s*\{\s*\}"|wc -l
> > 1951
> >
> > The standard-C compliant pattern has about 2500 entries:
> >
> > $ git grep -E "=\s*\{\s*NULL\s*\}"|wc -l
> > 137
> > $ git grep -E "=\s*\{\s*0\s*\}"|wc -l
> > 2323
> >
> > Meaning that developers have split options on that.
> >
> > So, let's opt to the simpler form.
>
> Acked-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Applied, thanks!
>
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
> > ---
> > drivers/staging/media/sunxi/cedrus/cedrus.c | 2 +-
> > drivers/staging/media/sunxi/cedrus/cedrus_dec.c | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
> > index b538eb0321d8..b7c918fa5fd1 100644
> > --- a/drivers/staging/media/sunxi/cedrus/cedrus.c
> > +++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
> > @@ -75,7 +75,7 @@ static int cedrus_init_ctrls(struct cedrus_dev *dev, struct cedrus_ctx *ctx)
> > memset(ctx->ctrls, 0, ctrl_size);
> >
> > for (i = 0; i < CEDRUS_CONTROLS_COUNT; i++) {
> > - struct v4l2_ctrl_config cfg = { 0 };
> > + struct v4l2_ctrl_config cfg = {};
> >
> > cfg.elem_size = cedrus_controls[i].elem_size;
> > cfg.id = cedrus_controls[i].id;
> > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> > index e40180a33951..f10c25f5460e 100644
> > --- a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> > @@ -26,7 +26,7 @@ void cedrus_device_run(void *priv)
> > {
> > struct cedrus_ctx *ctx = priv;
> > struct cedrus_dev *dev = ctx->dev;
> > - struct cedrus_run run = { 0 };
> > + struct cedrus_run run = {};
> > struct media_request *src_req;
> > unsigned long flags;
> >
Thanks,
Mauro
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] Fix OMAP4430 SDP Ethernet startup
From: Peter Ujfalusi @ 2018-12-07 13:40 UTC (permalink / raw)
To: Russell King - ARM Linux, Tony Lindgren, linux-arm-kernel,
linux-omap, Santosh Shilimkar
In-Reply-To: <20181207125224.GW6920@n2100.armlinux.org.uk>
Russell,
On 07/12/2018 14.52, Russell King - ARM Linux wrote:
> It was noticed that unbinding and rebinding the KSZ8851 ethernet
> resulted in the driver reporting "failed to read device ID" at probe.
> Probing the reset line with a 'scope while repeatedly attempting to
> bind the driver in a shell loop revealed that the KSZ8851 RSTN pin is
> constantly held at zero, meaning the device is held in reset, and
> does not respond on the SPI bus.
>
> Experimentation with the startup delay on the regulator set to 50ms
> shows that the reset is positively released after 20ms.
>
> Schematics for this board are not available, and the traces are buried
> in the inner layers of the board which makes tracing where the RSTN pin
> extremely difficult. We can only guess that the RSTN pin is wired to a
> reset generator chip driven off the ethernet supply, which fits the
> observed behaviour.
Based on the schematics of the Blaze device (which should be very close
to SDP4430):
TPS22902YFPR is used as the regulator switch (gpio48 controlled)
The VOUT is routed to TPS3808G01DBV (SCH Note: Threshold set at 90%.
Vsense: 0.405V).
According to the TPS3808 data sheet the RESET delay time when Ct is open
(this is the case in the schema): MIN/TYP/MAX: 12/20/28 ms.
The 20ms you are seeing confirms this setup.
> Include this delay in the regulator startup delay - effectively
> treating the reset as a "supply stable" indicator.
>
> This can not be modelled as a delay in the KSZ8851 driver since the
> reset generation is board specific - if the RSTN pin had been wired to
> a GPIO, reset could be released earlier via the already provided support
> in the KSZ8851 driver.
Reviewed-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
> arch/arm/boot/dts/omap4-sdp.dts | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/boot/dts/omap4-sdp.dts b/arch/arm/boot/dts/omap4-sdp.dts
> index 0831a2bf6cec..57acf3a8b8a1 100644
> --- a/arch/arm/boot/dts/omap4-sdp.dts
> +++ b/arch/arm/boot/dts/omap4-sdp.dts
> @@ -33,6 +33,7 @@
> gpio = <&gpio2 16 GPIO_ACTIVE_HIGH>; /* gpio line 48 */
> enable-active-high;
> regulator-boot-on;
> + startup-delay-us = <25000>;
> };
>
> vbat: fixedregulator-vbat {
>
- Péter
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [git:media_tree/master] media: Use of_node_name_eq for node name comparisons
From: Mauro Carvalho Chehab @ 2018-12-07 13:28 UTC (permalink / raw)
To: linuxtv-commits
Cc: Rob Herring, linux-samsung-soc, Hyun Kwon, Michal Simek,
Krzysztof Kozlowski, Benoit Parrot, Kyungmin Park, Kukjin Kim,
Laurent Pinchart, Sylwester Nawrocki, Hans Verkuil,
linux-arm-kernel
This is an automatic generated email to let you know that the following patch were queued:
Subject: media: Use of_node_name_eq for node name comparisons
Author: Rob Herring <robh@kernel.org>
Date: Thu Dec 6 14:35:19 2018 -0500
Convert string compares of DT node names to use of_node_name_eq helper
instead. This removes direct access to the node name pointer.
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Kukjin Kim <kgene@kernel.org>
Cc: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Hyun Kwon <hyun.kwon@xilinx.com>
Cc: Michal Simek <michal.simek@xilinx.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-samsung-soc@vger.kernel.org
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Benoit Parrot <bparrot@ti.com>
Signed-off-by: Rob Herring <robh@kernel.org>
Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
drivers/media/platform/exynos4-is/media-dev.c | 12 ++++++------
drivers/media/platform/ti-vpe/cal.c | 4 ++--
drivers/media/platform/xilinx/xilinx-tpg.c | 2 +-
drivers/media/v4l2-core/v4l2-fwnode.c | 6 ++----
4 files changed, 11 insertions(+), 13 deletions(-)
---
diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
index 870501b0f351..463f2d84553e 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -445,7 +445,7 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,
*/
np = of_get_parent(rem);
- if (np && !of_node_cmp(np->name, "i2c-isp"))
+ if (of_node_name_eq(np, "i2c-isp"))
pd->fimc_bus_type = FIMC_BUS_TYPE_ISP_WRITEBACK;
else
pd->fimc_bus_type = pd->sensor_bus_type;
@@ -495,7 +495,7 @@ static int fimc_md_register_sensor_entities(struct fimc_md *fmd)
for_each_available_child_of_node(parent, node) {
struct device_node *port;
- if (of_node_cmp(node->name, "csis"))
+ if (!of_node_name_eq(node, "csis"))
continue;
/* The csis node can have only port subnode. */
port = of_get_next_child(node, NULL);
@@ -720,13 +720,13 @@ static int fimc_md_register_platform_entities(struct fimc_md *fmd,
continue;
/* If driver of any entity isn't ready try all again later. */
- if (!strcmp(node->name, CSIS_OF_NODE_NAME))
+ if (of_node_name_eq(node, CSIS_OF_NODE_NAME))
plat_entity = IDX_CSIS;
- else if (!strcmp(node->name, FIMC_IS_OF_NODE_NAME))
+ else if (of_node_name_eq(node, FIMC_IS_OF_NODE_NAME))
plat_entity = IDX_IS_ISP;
- else if (!strcmp(node->name, FIMC_LITE_OF_NODE_NAME))
+ else if (of_node_name_eq(node, FIMC_LITE_OF_NODE_NAME))
plat_entity = IDX_FLITE;
- else if (!strcmp(node->name, FIMC_OF_NODE_NAME) &&
+ else if (of_node_name_eq(node, FIMC_OF_NODE_NAME) &&
!of_property_read_bool(node, "samsung,lcd-wb"))
plat_entity = IDX_FIMC;
diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
index 95a093f41905..fc3c212b96e1 100644
--- a/drivers/media/platform/ti-vpe/cal.c
+++ b/drivers/media/platform/ti-vpe/cal.c
@@ -1615,7 +1615,7 @@ of_get_next_port(const struct device_node *parent,
return NULL;
}
prev = port;
- } while (of_node_cmp(port->name, "port") != 0);
+ } while (!of_node_name_eq(port, "port"));
}
return port;
@@ -1635,7 +1635,7 @@ of_get_next_endpoint(const struct device_node *parent,
if (!ep)
return NULL;
prev = ep;
- } while (of_node_cmp(ep->name, "endpoint") != 0);
+ } while (!of_node_name_eq(ep, "endpoint"));
return ep;
}
diff --git a/drivers/media/platform/xilinx/xilinx-tpg.c b/drivers/media/platform/xilinx/xilinx-tpg.c
index 1399e71d42c2..ed01bedb5db6 100644
--- a/drivers/media/platform/xilinx/xilinx-tpg.c
+++ b/drivers/media/platform/xilinx/xilinx-tpg.c
@@ -722,7 +722,7 @@ static int xtpg_parse_of(struct xtpg_device *xtpg)
const struct xvip_video_format *format;
struct device_node *endpoint;
- if (!port->name || of_node_cmp(port->name, "port"))
+ if (!of_node_name_eq(port, "port"))
continue;
format = xvip_of_get_format(port);
diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
index 7a3cc10fbe36..f0a6c0c6f162 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -564,8 +564,7 @@ int v4l2_fwnode_parse_link(struct fwnode_handle *__fwnode,
fwnode = fwnode_get_parent(__fwnode);
fwnode_property_read_u32(fwnode, port_prop, &link->local_port);
fwnode = fwnode_get_next_parent(fwnode);
- if (is_of_node(fwnode) &&
- of_node_cmp(to_of_node(fwnode)->name, "ports") == 0)
+ if (is_of_node(fwnode) && of_node_name_eq(to_of_node(fwnode), "ports"))
fwnode = fwnode_get_next_parent(fwnode);
link->local_node = fwnode;
@@ -578,8 +577,7 @@ int v4l2_fwnode_parse_link(struct fwnode_handle *__fwnode,
fwnode = fwnode_get_parent(fwnode);
fwnode_property_read_u32(fwnode, port_prop, &link->remote_port);
fwnode = fwnode_get_next_parent(fwnode);
- if (is_of_node(fwnode) &&
- of_node_cmp(to_of_node(fwnode)->name, "ports") == 0)
+ if (is_of_node(fwnode) && of_node_name_eq(to_of_node(fwnode), "ports"))
fwnode = fwnode_get_next_parent(fwnode);
link->remote_node = fwnode;
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH 3/3] mmc: sdhci_am654: Add Initial Support for AM654 SDHCI driver
From: Adrian Hunter @ 2018-12-07 13:32 UTC (permalink / raw)
To: Faiz Abbas, linux-kernel, linux-arm-kernel, devicetree, linux-mmc
Cc: mark.rutland, ulf.hansson, robh+dt, michal.simek, kishon
In-Reply-To: <dfe6c97a-6383-c697-88e5-279f6ca8136e@ti.com>
On 5/12/18 5:07 PM, Faiz Abbas wrote:
> Hi Adrian,
>
> On 05/12/18 7:12 PM, Adrian Hunter wrote:
>> On 29/11/18 6:15 PM, Faiz Abbas wrote:
>>> The host controllers on TI's AM654 SOCs are not compatible with
>>> the phy and consumer model of the sdhci-of-arasan driver. It turns out
>>> that for optimal operation at higher speeds, a special tuning procedure
>>> needs to be implemented which involves configuration of platform
>>> specific phy registers.
>>>
>>> Therefore, branch out to a new sdhci_am654 driver and add the phy
>>> register space with all phy configurations to it. Populate AM654
>>> specific callbacks to sdhci_ops and add SDHCI_QUIRKS wherever
>>> applicable.
>>>
>>> Only add support for upto High Speed for SD card and upto DDR52 speed
>>> mode for eMMC. Higher speeds will be added in subsequent patches.
>>>
> ...
>>> + sdhci_am654->dll_on = true;
>>> + }
>>> +}
>>> +
>>> +
>>
>> Double blank line
>
> Will fix.
>
>>
>>> +static void sdhci_am654_set_power(struct sdhci_host *host, unsigned char mode,
>>> + unsigned short vdd)
>>> +{
>>> + if (!IS_ERR(host->mmc->supply.vmmc)) {
>>> + struct mmc_host *mmc = host->mmc;
>>> +
>>> + mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
>>> + }
>>> + sdhci_set_power_noreg(host, mode, vdd);
>>> +}
>>> +
>>> +struct sdhci_ops sdhci_am654_ops = {
>>> + .get_max_clock = sdhci_pltfm_clk_get_max_clock,
>>> + .get_timeout_clock = sdhci_pltfm_clk_get_max_clock,
>>> + .set_uhs_signaling = sdhci_set_uhs_signaling,
>>> + .set_bus_width = sdhci_set_bus_width,
>>> + .set_power = sdhci_am654_set_power,
>>> + .set_clock = sdhci_am654_set_clock,
>>> + .reset = sdhci_reset,
>>> +};
>>> +
>>> +static const struct sdhci_pltfm_data sdhci_am654_pdata = {
>>> + .ops = &sdhci_am654_ops,
>>> + .quirks = SDHCI_QUIRK_INVERTED_WRITE_PROTECT |
>>> + SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12,
>>> + .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
>>> +};
>>> +
>>> +static int sdhci_am654_init(struct sdhci_host *host)
>>> +{
>>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>> + struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
>>> + u32 ctl_cfg_2 = 0;
>>> + u32 val;
>>> + int ret;
>>> +
>>> + regmap_read(sdhci_am654->base, PHY_STAT1, &val);
>>> + if (~val & CALDONE_MASK) {
>>> + /* Calibrate IO lines */
>>> + regmap_update_bits(sdhci_am654->base, PHY_CTRL1,
>>> + PDB_MASK, PDB_MASK);
>>> + ret = regmap_read_poll_timeout(sdhci_am654->base, PHY_STAT1,
>>> + val, val & CALDONE_MASK, 1, 20);
>>> + if (ret)
>>> + return ret;
>>> + }
>>> +
>>> + /* Enable pins by setting IO mux to 0 */
>>> + regmap_update_bits(sdhci_am654->base, PHY_CTRL1,
>>> + IOMUX_ENABLE_MASK, 0);
>>> +
>>> + /* Set slot type based on SD or eMMC */
>>> + if (host->mmc->caps & MMC_CAP_NONREMOVABLE)
>>> + ctl_cfg_2 = SLOTTYPE_EMBEDDED;
>>> +
>>> + regmap_update_bits(sdhci_am654->base, CTL_CFG_2,
>>> + ctl_cfg_2, SLOTTYPE_MASK);
>>> +
>>> + return sdhci_add_host(host);
>>> +}
>>> +
>>> +static int sdhci_am654_get_of_property(struct platform_device *pdev,
>>> + struct sdhci_am654_data *sdhci_am654)
>>> +{
>>> + struct device *dev = &pdev->dev;
>>> + int drv_strength;
>>> + int ret;
>>> +
>>> + ret = device_property_read_u32(dev, "ti,trm-icp",
>>> + &sdhci_am654->trm_icp);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + ret = device_property_read_u32(dev, "ti,otap-del-sel",
>>> + &sdhci_am654->otap_del_sel);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + ret = device_property_read_u32(dev, "ti,driver-strength-ohm",
>>> + &drv_strength);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + switch (drv_strength) {
>>> + case 50:
>>> + sdhci_am654->drv_strength = DRIVER_STRENGTH_50_OHM;
>>> + break;
>>> + case 33:
>>> + sdhci_am654->drv_strength = DRIVER_STRENGTH_33_OHM;
>>> + break;
>>> + case 66:
>>> + sdhci_am654->drv_strength = DRIVER_STRENGTH_66_OHM;
>>> + break;
>>> + case 100:
>>> + sdhci_am654->drv_strength = DRIVER_STRENGTH_100_OHM;
>>> + break;
>>> + case 40:
>>> + sdhci_am654->drv_strength = DRIVER_STRENGTH_40_OHM;
>>> + break;
>>> + default:
>>> + dev_err(dev, "Invalid driver strength\n");
>>> + return -EINVAL;
>>> + }
>>> +
>>> + sdhci_get_of_property(pdev);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int sdhci_am654_probe(struct platform_device *pdev)
>>> +{
>>> + struct sdhci_pltfm_host *pltfm_host;
>>> + struct sdhci_am654_data *sdhci_am654;
>>> + struct sdhci_host *host;
>>> + struct resource *res;
>>> + struct clk *clk_xin;
>>> + struct device *dev = &pdev->dev;
>>> + void __iomem *base;
>>> + int ret;
>>> +
>>> + host = sdhci_pltfm_init(pdev, &sdhci_am654_pdata, sizeof(*sdhci_am654));
>>> + if (IS_ERR(host))
>>> + return PTR_ERR(host);
>>> +
>>> + pltfm_host = sdhci_priv(host);
>>> + sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
>>> +
>>> + clk_xin = devm_clk_get(dev, "clk_xin");
>>> + if (IS_ERR(clk_xin)) {
>>> + dev_err(dev, "clk_xin clock not found.\n");
>>> + ret = PTR_ERR(clk_xin);
>>> + goto err_pltfm_free;
>>> + }
>>> +
>>> + sdhci_am654->clk_ahb = devm_clk_get(dev, "clk_ahb");
>>> + if (IS_ERR(sdhci_am654->clk_ahb)) {
>>> + dev_err(dev, "clk_ahb clock not found.\n");
>>> + ret = PTR_ERR(sdhci_am654->clk_ahb);
>>> + goto err_pltfm_free;
>>> + }
>>
>> Did you intend not to enable clks?
>
> Yes. Clocks get enabled as a part of pm_runtime calls.
Ok, but that could use an explanatory comment. Also why get a reference to
clk_ahb if that reference is never used?
>>
>>> +
>>> + pltfm_host->clk = clk_xin;
>>> +
>>> + pm_runtime_enable(dev);
>>> + ret = pm_runtime_get_sync(dev);
>>> + if (ret > 0) {
>>
>> Did you intend 'ret > 0'?
>
> Sorry. That was intended to be < 0.
>
> Thanks,
> Faiz
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] media: cedrus: don't initialize pointers with zero
From: Dan Carpenter @ 2018-12-07 13:22 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: devel, Paul Kocialkowski, Maxime Ripard, Greg Kroah-Hartman,
Mauro Carvalho Chehab, Hans Verkuil, Chen-Yu Tsai,
linux-arm-kernel, Linux Media Mailing List
In-Reply-To: <20181207093106.4f112d0b@coco.lan>
On Fri, Dec 07, 2018 at 09:31:06AM -0200, Mauro Carvalho Chehab wrote:
> Em Fri, 7 Dec 2018 12:14:50 +0100
> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
>
> > On 12/07/2018 11:56 AM, Mauro Carvalho Chehab wrote:
> > > A common mistake is to assume that initializing a var with:
> > > struct foo f = { 0 };
> > >
> > > Would initialize a zeroed struct. Actually, what this does is
> > > to initialize the first element of the struct to zero.
> > >
> > > According to C99 Standard 6.7.8.21:
> > >
> > > "If there are fewer initializers in a brace-enclosed
> > > list than there are elements or members of an aggregate,
> > > or fewer characters in a string literal used to initialize
> > > an array of known size than there are elements in the array,
> > > the remainder of the aggregate shall be initialized implicitly
> > > the same as objects that have static storage duration."
> > >
> > > So, in practice, it could zero the entire struct, but, if the
> > > first element is not an integer, it will produce warnings:
> > >
> > > drivers/staging/media/sunxi/cedrus/cedrus.c:drivers/staging/media/sunxi/cedrus/cedrus.c:78:49: warning: Using plain integer as NULL pointer
> > > drivers/staging/media/sunxi/cedrus/cedrus_dec.c:drivers/staging/media/sunxi/cedrus/cedrus_dec.c:29:35: warning: Using plain integer as NULL pointer
> > >
> > > A proper way to initialize it with gcc is to use:
> > >
> > > struct foo f = { };
> > >
> > > But that seems to be a gcc extension. So, I decided to check upstream
> >
> > No, this is not a gcc extension. It's part of the latest C standard.
>
> Sure? Where the C standard spec states that? I've been seeking for
> such info for a while, as '= {}' is also my personal preference.
>
> I tried to build the Kernel with clang, just to be sure that this
> won't cause issues with the clang support
My test says that clang works with {}.
I support this in Smatch as well.
regards,
dan carpenter
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH v4 08/26] drm/sun4i: sun6i_mipi_dsi: Fix VBP size calculation
From: Maxime Ripard @ 2018-12-07 13:21 UTC (permalink / raw)
To: Jagan Teki
Cc: Mark Rutland, devicetree, Jernej Skrabec, David Airlie,
Catalin Marinas, Michael Turquette, linux-sunxi, Will Deacon,
linux-kernel, dri-devel, Vasily Khoruzhick, Stephen Boyd,
Chen-Yu Tsai, Rob Herring, Michael Trimarchi, linux-amarula,
linux-clk, linux-arm-kernel, Icenowy Zheng
In-Reply-To: <CAMty3ZDTvn4VF_oOvEi2jCw9Rw5WPd1LQT3SZ5X1K0fJAFg2cQ@mail.gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 3874 bytes --]
On Tue, Nov 27, 2018 at 04:34:35PM +0530, Jagan Teki wrote:
> On Tue, Nov 27, 2018 at 3:55 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> >
> > On Tue, Nov 20, 2018 at 09:55:42PM +0530, Jagan Teki wrote:
> > > On Tue, Nov 20, 2018 at 9:27 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > >
> > > > On Thu, Nov 15, 2018 at 11:19:53PM +0530, Jagan Teki wrote:
> > > > > On Thu, Nov 15, 2018 at 3:26 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > On Tue, Nov 13, 2018 at 04:46:15PM +0530, Jagan Teki wrote:
> > > > > > > The horizontal and vertical back porch calculation in BSP
> > > > > > > code is simply following the Linux drm comment diagram, in
> > > > > > > include/drm/drm_modes.h which is
> > > > > > >
> > > > > > > [hv]back porch = [hv]total - [hv]sync_end
> > > > > > >
> > > > > > > BSP code form BPI-M64-bsp is calculating vertical back porch as
> > > > > > > (from linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c)
> > > > > > >
> > > > > > > timmings->ver_sync_time= panel_info->lcd_vspw;
> > > > > > > timmings->ver_back_porch= panel_info->lcd_vbp-panel_info->lcd_vspw;
> > > > > > >
> > > > > > > vbp = panel->lcd_vbp;
> > > > > > > vspw = panel->lcd_vspw;
> > > > > > > dsi_dev[sel]->dsi_basic_size0.bits.vbp = vbp-vspw;
> > > > > > > dsi_dev[sel]->dsi_basic_size0.bits.vbp = panel->lcd_vbp - panel->lcd_vspw;
> > > > > > > => timmings->ver_back_porch + panel_info->lcd_vspw - panel_info->lcd_vspw
> > > > > > > => timmings->ver_back_porch
> > > > > > > => mode->vtotal - mode->end
> > > > > > >
> > > > > > > Which evatually same as mode->vtotal - mode->vsync_end so update the
> > > > > > > same in SUN6I_DSI_BASIC_SIZE0_VBP
> > > > > > >
> > > > > > > On the information note, existing SUN6I_DSI_BASIC_SIZE0_VSA is proper
> > > > > > > value.
> > > > > > >
> > > > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > > >
> > > > > > I've tested your changes on my A33 board, and this commit will break
> > > > > > it.
> > > > > >
> > > > > > It creates vblank timeouts, and visual artifacts at the bottom of the
> > > > > > display.
> > > > >
> > > > > Strange, VBP is earlier gives front porch which is anyway wrong.
> > > > >
> > > > > >
> > > > > > Later commits seem to fix the issue, but will create some blanking on
> > > > > > the upper third of the display.
> > > > > >
> > > > > > Since the documentation is quite sparse, and a MIPI-DSI analyzer is
> > > > > > way too expensive, I'd really like to have at least what each of these
> > > > > > commits are actually fixing, and what symptoms each of these were
> > > > > > causing, and not just "the BSP does it".
> > > > >
> > > > > W/o this 2-lane panel is breaking, same vblank timeout and visual
> > > > > artifacts at the bottom of the panel. though the commits may reference
> > > > > BSP, I have at-least tested on 3 different panels for us to prove its
> > > > > working.
> > > > >
> > > > > > Having some datasheet for the panels you had working would help too.
> > > > >
> > > > > Unfortunately datasheet doesn't have any required information what we
> > > > > actually looking for.
> > > >
> > > > Not even the timings? How did you get that information then?
> > >
> > > datasheet has timing values, but this changes need controller
> > > information about VBP register that I don't have. But again existing
> > > VBP is not back porch for real, it's front porch.
> >
> > Yet, this breaks the existing setup. So again:
>
> Was it with 4-lane or 2-lane panel? can you test it with 2-lane panel?
> I can see the issue on 2-lane but the 4-lane working fine with this
> patch even.
It's a 4 lane display.
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2] media: cedrus: don't initialize pointers with zero
From: Paul Kocialkowski @ 2018-12-07 13:21 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: devel, Maxime Ripard, Greg Kroah-Hartman, Mauro Carvalho Chehab,
Chen-Yu Tsai, linux-arm-kernel, Linux Media Mailing List
In-Reply-To: <9db60f061d1c577f14136f81af641f58bccbead3.1544187795.git.mchehab+samsung@kernel.org>
Hi,
On Fri, 2018-12-07 at 08:03 -0500, Mauro Carvalho Chehab wrote:
> A common mistake is to assume that initializing a var with:
> struct foo f = { 0 };
>
> Would initialize a zeroed struct. Actually, what this does is
> to initialize the first element of the struct to zero.
>
> According to C99 Standard 6.7.8.21:
>
> "If there are fewer initializers in a brace-enclosed
> list than there are elements or members of an aggregate,
> or fewer characters in a string literal used to initialize
> an array of known size than there are elements in the array,
> the remainder of the aggregate shall be initialized implicitly
> the same as objects that have static storage duration."
>
> So, in practice, it could zero the entire struct, but, if the
> first element is not an integer, it will produce warnings:
>
> drivers/staging/media/sunxi/cedrus/cedrus.c:drivers/staging/media/sunxi/cedrus/cedrus.c:78:49: warning: Using plain integer as NULL pointer
> drivers/staging/media/sunxi/cedrus/cedrus_dec.c:drivers/staging/media/sunxi/cedrus/cedrus_dec.c:29:35: warning: Using plain integer as NULL pointer
>
> As the right initialization would be, instead:
>
> struct foo f = { NULL };
Thanks for sharing these details, it's definitely interesting and good
to know :)
> Another way to initialize it with gcc is to use:
>
> struct foo f = {};
>
> That seems to be a gcc extension, but clang also does the right thing,
> and that's a clean way for doing it.
>
> Anyway, I decided to check upstream what's the most commonly pattern.
> The "= {}" pattern has about 2000 entries:
>
> $ git grep -E "=\s*\{\s*\}"|wc -l
> 1951
>
> The standard-C compliant pattern has about 2500 entries:
>
> $ git grep -E "=\s*\{\s*NULL\s*\}"|wc -l
> 137
> $ git grep -E "=\s*\{\s*0\s*\}"|wc -l
> 2323
>
> Meaning that developers have split options on that.
>
> So, let's opt to the simpler form.
Acked-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
> ---
> drivers/staging/media/sunxi/cedrus/cedrus.c | 2 +-
> drivers/staging/media/sunxi/cedrus/cedrus_dec.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
> index b538eb0321d8..b7c918fa5fd1 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
> @@ -75,7 +75,7 @@ static int cedrus_init_ctrls(struct cedrus_dev *dev, struct cedrus_ctx *ctx)
> memset(ctx->ctrls, 0, ctrl_size);
>
> for (i = 0; i < CEDRUS_CONTROLS_COUNT; i++) {
> - struct v4l2_ctrl_config cfg = { 0 };
> + struct v4l2_ctrl_config cfg = {};
>
> cfg.elem_size = cedrus_controls[i].elem_size;
> cfg.id = cedrus_controls[i].id;
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> index e40180a33951..f10c25f5460e 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> @@ -26,7 +26,7 @@ void cedrus_device_run(void *priv)
> {
> struct cedrus_ctx *ctx = priv;
> struct cedrus_dev *dev = ctx->dev;
> - struct cedrus_run run = { 0 };
> + struct cedrus_run run = {};
> struct media_request *src_req;
> unsigned long flags;
>
--
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] PCI: controller: dwc: Make PCI_IMX6 depend on PCIEPORTBUS
From: Niklas Cassel @ 2018-12-07 13:11 UTC (permalink / raw)
To: Andrey Smirnov
Cc: Dong Aisheng, Richard Zhu, linux-arm-kernel, linux-pci,
linux-kernel, linux-imx, Bjorn Helgaas, Leonard Crestez,
Chris Healy, Lucas Stach
In-Reply-To: <CAHQ1cqFeJtxnpfzqWVQ3T9Sv=gfw+cvZZw-uKnZV7pxJg_S+EA@mail.gmail.com>
On Thu, Dec 06, 2018 at 08:55:13PM -0800, Andrey Smirnov wrote:
> On Thu, Dec 6, 2018 at 2:28 AM Lucas Stach <l.stach@pengutronix.de> wrote:
> >
> > Am Mittwoch, den 05.12.2018, 23:45 -0800 schrieb Andrey Smirnov:
> > > Building a kernel with CONFIG_PCI_IMX6=y, but CONFIG_PCIEPORTBUS=n
> > > produces a system where built-in PCIE bridge (16c3:abcd) isn't bound
> > > to pcieport driver. This, in turn, results in a PCIE bus that is
> > > capable of enumerating attached PCIE device, but lacks functional
> > > interrupt support.
> >
> > This is odd. AFAIK PCI port services are a totally optional thing and
> > them being absent should not lead to a non-functional PCI bus. So I
> > would really like to see some deeper analysis what is going on here.
> >
>
> AFAICT, this is due to pcieport driver enabling MSI of the bridge
> device (16c3:abcd) via pcie_port_device_register() ->
> pcie_init_service_irqs() -> pcie_port_enable_irq_vec() -> etc.
>
> I did an experiment on a i.MX8MQ/PCIE -> i210 setup I have: I disabled
> CONFIG_PCIEPORTBUS and hacked igb_main.c enough to make the i210
> driver believe it should fall back onto legacy interrupts. Even
> without pcieport present in the system, i210 worked as expected via
> legacy interrupts, which seems to collaborate my conjecture above.
>
> Thanks,
> Andrey Smirnov
IIUC PCIEPORTBUS should not be needed for MSIs to work,
it is only needed if you want e.g. PME or AER.
The difference is that if PCIEPORTBUS is enabled, a MSI irq vector will be
allocated for the Root Complex itself, so that it can send an irq when
e.g. AER has detected an error.
If we disregard that MSI handling is currently broken on DWC PCIe:
https://marc.info/?l=linux-pci&m=154214986924244&w=2
It is very possible to have MSIs on dragonboard 820c, which also
uses the DWC PCIe controller, without having PCIEPORTBUS selected:
# zcat /proc/config.gz | grep -E "PCIE_QCOM|PCIEPORTBUS"
# CONFIG_PCIEPORTBUS is not set
CONFIG_PCIE_QCOM=y
# lspci -v -s 0000:00:00.0
0000:00:00.0 PCI bridge: Qualcomm Device 0104 (prog-if 00 [Normal decode])
...
Capabilities: [50] MSI: Enable- Count=1/32 Maskable+ 64bit+
# lspci -v -s 0000:01:00.0
0000:01:00.0 Network controller: Qualcomm Atheros QCA6174 802.11ac Wireless Network Adapter (rev 32)
...
Capabilities: [50] MSI: Enable+ Count=1/8 Maskable+ 64bit-
# cat /proc/interrupts | grep MSI
70: 5620 0 0 0 PCI-MSI 524288 Edge ath10k_pci
So perhaps this is a bug specific to imx6?
Kind regards,
Niklas
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v2] media: cedrus: don't initialize pointers with zero
From: Mauro Carvalho Chehab @ 2018-12-07 13:03 UTC (permalink / raw)
Cc: devel, Maxime Ripard, Greg Kroah-Hartman, Mauro Carvalho Chehab,
Paul Kocialkowski, Chen-Yu Tsai, Mauro Carvalho Chehab,
linux-arm-kernel, Linux Media Mailing List
A common mistake is to assume that initializing a var with:
struct foo f = { 0 };
Would initialize a zeroed struct. Actually, what this does is
to initialize the first element of the struct to zero.
According to C99 Standard 6.7.8.21:
"If there are fewer initializers in a brace-enclosed
list than there are elements or members of an aggregate,
or fewer characters in a string literal used to initialize
an array of known size than there are elements in the array,
the remainder of the aggregate shall be initialized implicitly
the same as objects that have static storage duration."
So, in practice, it could zero the entire struct, but, if the
first element is not an integer, it will produce warnings:
drivers/staging/media/sunxi/cedrus/cedrus.c:drivers/staging/media/sunxi/cedrus/cedrus.c:78:49: warning: Using plain integer as NULL pointer
drivers/staging/media/sunxi/cedrus/cedrus_dec.c:drivers/staging/media/sunxi/cedrus/cedrus_dec.c:29:35: warning: Using plain integer as NULL pointer
As the right initialization would be, instead:
struct foo f = { NULL };
Another way to initialize it with gcc is to use:
struct foo f = {};
That seems to be a gcc extension, but clang also does the right thing,
and that's a clean way for doing it.
Anyway, I decided to check upstream what's the most commonly pattern.
The "= {}" pattern has about 2000 entries:
$ git grep -E "=\s*\{\s*\}"|wc -l
1951
The standard-C compliant pattern has about 2500 entries:
$ git grep -E "=\s*\{\s*NULL\s*\}"|wc -l
137
$ git grep -E "=\s*\{\s*0\s*\}"|wc -l
2323
Meaning that developers have split options on that.
So, let's opt to the simpler form.
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
---
drivers/staging/media/sunxi/cedrus/cedrus.c | 2 +-
drivers/staging/media/sunxi/cedrus/cedrus_dec.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
index b538eb0321d8..b7c918fa5fd1 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
@@ -75,7 +75,7 @@ static int cedrus_init_ctrls(struct cedrus_dev *dev, struct cedrus_ctx *ctx)
memset(ctx->ctrls, 0, ctrl_size);
for (i = 0; i < CEDRUS_CONTROLS_COUNT; i++) {
- struct v4l2_ctrl_config cfg = { 0 };
+ struct v4l2_ctrl_config cfg = {};
cfg.elem_size = cedrus_controls[i].elem_size;
cfg.id = cedrus_controls[i].id;
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
index e40180a33951..f10c25f5460e 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
@@ -26,7 +26,7 @@ void cedrus_device_run(void *priv)
{
struct cedrus_ctx *ctx = priv;
struct cedrus_dev *dev = ctx->dev;
- struct cedrus_run run = { 0 };
+ struct cedrus_run run = {};
struct media_request *src_req;
unsigned long flags;
--
2.19.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH] media: cetrus: return an error if alloc fails
From: Paul Kocialkowski @ 2018-12-07 13:02 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: devel, Maxime Ripard, Greg Kroah-Hartman, Mauro Carvalho Chehab,
Chen-Yu Tsai, linux-arm-kernel, Linux Media Mailing List
In-Reply-To: <7d746ddf408ab1bc100d4d676daf3e2400637fbe.1544181226.git.mchehab+samsung@kernel.org>
Hi,
On Fri, 2018-12-07 at 06:13 -0500, Mauro Carvalho Chehab wrote:
> As warned by smatch:
>
> drivers/staging/media/sunxi/cedrus/cedrus.c: drivers/staging/media/sunxi/cedrus/cedrus.c:93 cedrus_init_ctrls() error: potential null dereference 'ctx->ctrls'. (kzalloc returns null)
>
> While here, remove the memset(), as kzalloc() already zeroes the
> struct.
Good catch, thanks for the patch!
> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
Acked-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
> drivers/staging/media/sunxi/cedrus/cedrus.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
> index 44c45c687503..24b89cd2b692 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
> @@ -72,7 +72,8 @@ static int cedrus_init_ctrls(struct cedrus_dev *dev, struct cedrus_ctx *ctx)
> ctrl_size = sizeof(ctrl) * CEDRUS_CONTROLS_COUNT + 1;
>
> ctx->ctrls = kzalloc(ctrl_size, GFP_KERNEL);
> - memset(ctx->ctrls, 0, ctrl_size);
> + if (!ctx->ctrls)
> + return -ENOMEM;
>
> for (i = 0; i < CEDRUS_CONTROLS_COUNT; i++) {
> struct v4l2_ctrl_config cfg = { NULL };
--
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 1/3] arm: Convert arm boot_lock to raw
From: Russell King - ARM Linux @ 2018-12-07 13:00 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: Tony Lindgren, Viresh Kumar, Linus Walleij, David Brown, Wei Xu,
Manivannan Sadhasivam, linux-samsung-soc, Viresh Kumar,
Krzysztof Kozlowski, Maxime Ripard, Chen-Yu Tsai, Kukjin Kim,
Andy Gross, Arnd Bergmann, linux-arm-msm, tglx, linux-omap,
linux-arm-kernel, Barry Song, Frank Rowand, Patrice Chotard,
Shiraz Hashim, Andreas Färber
In-Reply-To: <20181207102749.15205-2-bigeasy@linutronix.de>
On Fri, Dec 07, 2018 at 11:27:47AM +0100, Sebastian Andrzej Siewior wrote:
> From: Frank Rowand <frank.rowand@am.sony.com>
>
> The arm boot_lock is used by the secondary processor startup code. The locking
> task is the idle thread, which has idle->sched_class == &idle_sched_class.
> idle_sched_class->enqueue_task == NULL, so if the idle task blocks on the
> lock, the attempt to wake it when the lock becomes available will fail:
>
> try_to_wake_up()
> ...
> activate_task()
> enqueue_task()
> p->sched_class->enqueue_task(rq, p, flags)
>
> Fix by converting boot_lock to a raw spin lock.
I think the bigger question is why are all these platforms using this.
For the ARM development platforms, it's fair as they have no way to
power down the CPUs or reset the other CPUs, and the "boot lock" was
there originally to prevent the delay calibration going awry. (I've
been saying this for years but obviously people like to stuff their
fingers in their ears.)
It annoys me when people cargo-cult copy code and don't bother trying
to understand it, which is shown clearly in your diffstat.
However, modern platforms do have ways to control the other CPUs, so
the boot lock stuff should not be necessary.
Also note that CPU hotplug and CPU unplug is all synchronised in the
core CPU hotplug code, so spinlocks within the arch specific code to
allegedly serialise what they're doing between CPU death and CPU
bringup are completely redundant.
Rather than trying to fix this up, can we instead try to eliminate as
much of the coping of the Versatile Express etc code as possible?
The only place this should be needed is arch/arm/plat-versatile/platsmp.c
> arch/arm/mach-actions/platsmp.c | 6 +++---
> arch/arm/mach-exynos/platsmp.c | 12 ++++++------
> arch/arm/mach-hisi/platmcpm.c | 22 +++++++++++-----------
> arch/arm/mach-omap2/omap-smp.c | 10 +++++-----
> arch/arm/mach-prima2/platsmp.c | 10 +++++-----
> arch/arm/mach-qcom/platsmp.c | 10 +++++-----
> arch/arm/mach-spear/platsmp.c | 10 +++++-----
> arch/arm/mach-sti/platsmp.c | 10 +++++-----
> arch/arm/mach-sunxi/mc_smp.c | 20 ++++++++++----------
> arch/arm/plat-versatile/platsmp.c | 10 +++++-----
> 10 files changed, 60 insertions(+), 60 deletions(-)
>
> diff --git a/arch/arm/mach-actions/platsmp.c b/arch/arm/mach-actions/platsmp.c
> index 3efaa10efc439..770079245d273 100644
> --- a/arch/arm/mach-actions/platsmp.c
> +++ b/arch/arm/mach-actions/platsmp.c
> @@ -39,7 +39,7 @@ static void __iomem *sps_base_addr;
> static void __iomem *timer_base_addr;
> static int ncores;
>
> -static DEFINE_SPINLOCK(boot_lock);
> +static DEFINE_RAW_SPINLOCK(boot_lock);
>
> void owl_secondary_startup(void);
>
> @@ -93,7 +93,7 @@ static int s500_smp_boot_secondary(unsigned int cpu, struct task_struct *idle)
>
> udelay(10);
>
> - spin_lock(&boot_lock);
> + raw_spin_lock(&boot_lock);
>
> smp_send_reschedule(cpu);
>
> @@ -106,7 +106,7 @@ static int s500_smp_boot_secondary(unsigned int cpu, struct task_struct *idle)
> writel(0, timer_base_addr + OWL_CPU1_ADDR + (cpu - 1) * 4);
> writel(0, timer_base_addr + OWL_CPU1_FLAG + (cpu - 1) * 4);
>
> - spin_unlock(&boot_lock);
> + raw_spin_unlock(&boot_lock);
>
> return 0;
> }
> diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
> index 6a1e682371b32..17dca0ff336e0 100644
> --- a/arch/arm/mach-exynos/platsmp.c
> +++ b/arch/arm/mach-exynos/platsmp.c
> @@ -239,7 +239,7 @@ static void write_pen_release(int val)
> sync_cache_w(&pen_release);
> }
>
> -static DEFINE_SPINLOCK(boot_lock);
> +static DEFINE_RAW_SPINLOCK(boot_lock);
>
> static void exynos_secondary_init(unsigned int cpu)
> {
> @@ -252,8 +252,8 @@ static void exynos_secondary_init(unsigned int cpu)
> /*
> * Synchronise with the boot thread.
> */
> - spin_lock(&boot_lock);
> - spin_unlock(&boot_lock);
> + raw_spin_lock(&boot_lock);
> + raw_spin_unlock(&boot_lock);
> }
>
> int exynos_set_boot_addr(u32 core_id, unsigned long boot_addr)
> @@ -317,7 +317,7 @@ static int exynos_boot_secondary(unsigned int cpu, struct task_struct *idle)
> * Set synchronisation state between this boot processor
> * and the secondary one
> */
> - spin_lock(&boot_lock);
> + raw_spin_lock(&boot_lock);
>
> /*
> * The secondary processor is waiting to be released from
> @@ -344,7 +344,7 @@ static int exynos_boot_secondary(unsigned int cpu, struct task_struct *idle)
>
> if (timeout == 0) {
> printk(KERN_ERR "cpu1 power enable failed");
> - spin_unlock(&boot_lock);
> + raw_spin_unlock(&boot_lock);
> return -ETIMEDOUT;
> }
> }
> @@ -390,7 +390,7 @@ static int exynos_boot_secondary(unsigned int cpu, struct task_struct *idle)
> * calibrations, then wait for it to finish
> */
> fail:
> - spin_unlock(&boot_lock);
> + raw_spin_unlock(&boot_lock);
>
> return pen_release != -1 ? ret : 0;
> }
> diff --git a/arch/arm/mach-hisi/platmcpm.c b/arch/arm/mach-hisi/platmcpm.c
> index f66815c3dd07e..00524abd963f7 100644
> --- a/arch/arm/mach-hisi/platmcpm.c
> +++ b/arch/arm/mach-hisi/platmcpm.c
> @@ -61,7 +61,7 @@
>
> static void __iomem *sysctrl, *fabric;
> static int hip04_cpu_table[HIP04_MAX_CLUSTERS][HIP04_MAX_CPUS_PER_CLUSTER];
> -static DEFINE_SPINLOCK(boot_lock);
> +static DEFINE_RAW_SPINLOCK(boot_lock);
> static u32 fabric_phys_addr;
> /*
> * [0]: bootwrapper physical address
> @@ -113,7 +113,7 @@ static int hip04_boot_secondary(unsigned int l_cpu, struct task_struct *idle)
> if (cluster >= HIP04_MAX_CLUSTERS || cpu >= HIP04_MAX_CPUS_PER_CLUSTER)
> return -EINVAL;
>
> - spin_lock_irq(&boot_lock);
> + raw_spin_lock_irq(&boot_lock);
>
> if (hip04_cpu_table[cluster][cpu])
> goto out;
> @@ -147,7 +147,7 @@ static int hip04_boot_secondary(unsigned int l_cpu, struct task_struct *idle)
>
> out:
> hip04_cpu_table[cluster][cpu]++;
> - spin_unlock_irq(&boot_lock);
> + raw_spin_unlock_irq(&boot_lock);
>
> return 0;
> }
> @@ -162,11 +162,11 @@ static void hip04_cpu_die(unsigned int l_cpu)
> cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
> cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
>
> - spin_lock(&boot_lock);
> + raw_spin_lock(&boot_lock);
> hip04_cpu_table[cluster][cpu]--;
> if (hip04_cpu_table[cluster][cpu] == 1) {
> /* A power_up request went ahead of us. */
> - spin_unlock(&boot_lock);
> + raw_spin_unlock(&boot_lock);
> return;
> } else if (hip04_cpu_table[cluster][cpu] > 1) {
> pr_err("Cluster %d CPU%d boots multiple times\n", cluster, cpu);
> @@ -174,7 +174,7 @@ static void hip04_cpu_die(unsigned int l_cpu)
> }
>
> last_man = hip04_cluster_is_down(cluster);
> - spin_unlock(&boot_lock);
> + raw_spin_unlock(&boot_lock);
> if (last_man) {
> /* Since it's Cortex A15, disable L2 prefetching. */
> asm volatile(
> @@ -203,7 +203,7 @@ static int hip04_cpu_kill(unsigned int l_cpu)
> cpu >= HIP04_MAX_CPUS_PER_CLUSTER);
>
> count = TIMEOUT_MSEC / POLL_MSEC;
> - spin_lock_irq(&boot_lock);
> + raw_spin_lock_irq(&boot_lock);
> for (tries = 0; tries < count; tries++) {
> if (hip04_cpu_table[cluster][cpu])
> goto err;
> @@ -211,10 +211,10 @@ static int hip04_cpu_kill(unsigned int l_cpu)
> data = readl_relaxed(sysctrl + SC_CPU_RESET_STATUS(cluster));
> if (data & CORE_WFI_STATUS(cpu))
> break;
> - spin_unlock_irq(&boot_lock);
> + raw_spin_unlock_irq(&boot_lock);
> /* Wait for clean L2 when the whole cluster is down. */
> msleep(POLL_MSEC);
> - spin_lock_irq(&boot_lock);
> + raw_spin_lock_irq(&boot_lock);
> }
> if (tries >= count)
> goto err;
> @@ -231,10 +231,10 @@ static int hip04_cpu_kill(unsigned int l_cpu)
> goto err;
> if (hip04_cluster_is_down(cluster))
> hip04_set_snoop_filter(cluster, 0);
> - spin_unlock_irq(&boot_lock);
> + raw_spin_unlock_irq(&boot_lock);
> return 1;
> err:
> - spin_unlock_irq(&boot_lock);
> + raw_spin_unlock_irq(&boot_lock);
> return 0;
> }
> #endif
> diff --git a/arch/arm/mach-omap2/omap-smp.c b/arch/arm/mach-omap2/omap-smp.c
> index 1c73694c871ad..ac4d2f030b874 100644
> --- a/arch/arm/mach-omap2/omap-smp.c
> +++ b/arch/arm/mach-omap2/omap-smp.c
> @@ -69,7 +69,7 @@ static const struct omap_smp_config omap5_cfg __initconst = {
> .startup_addr = omap5_secondary_startup,
> };
>
> -static DEFINE_SPINLOCK(boot_lock);
> +static DEFINE_RAW_SPINLOCK(boot_lock);
>
> void __iomem *omap4_get_scu_base(void)
> {
> @@ -177,8 +177,8 @@ static void omap4_secondary_init(unsigned int cpu)
> /*
> * Synchronise with the boot thread.
> */
> - spin_lock(&boot_lock);
> - spin_unlock(&boot_lock);
> + raw_spin_lock(&boot_lock);
> + raw_spin_unlock(&boot_lock);
> }
>
> static int omap4_boot_secondary(unsigned int cpu, struct task_struct *idle)
> @@ -191,7 +191,7 @@ static int omap4_boot_secondary(unsigned int cpu, struct task_struct *idle)
> * Set synchronisation state between this boot processor
> * and the secondary one
> */
> - spin_lock(&boot_lock);
> + raw_spin_lock(&boot_lock);
>
> /*
> * Update the AuxCoreBoot0 with boot state for secondary core.
> @@ -270,7 +270,7 @@ static int omap4_boot_secondary(unsigned int cpu, struct task_struct *idle)
> * Now the secondary core is starting up let it run its
> * calibrations, then wait for it to finish
> */
> - spin_unlock(&boot_lock);
> + raw_spin_unlock(&boot_lock);
>
> return 0;
> }
> diff --git a/arch/arm/mach-prima2/platsmp.c b/arch/arm/mach-prima2/platsmp.c
> index 75ef5d4be554c..c17c86e5d860a 100644
> --- a/arch/arm/mach-prima2/platsmp.c
> +++ b/arch/arm/mach-prima2/platsmp.c
> @@ -22,7 +22,7 @@
>
> static void __iomem *clk_base;
>
> -static DEFINE_SPINLOCK(boot_lock);
> +static DEFINE_RAW_SPINLOCK(boot_lock);
>
> static void sirfsoc_secondary_init(unsigned int cpu)
> {
> @@ -36,8 +36,8 @@ static void sirfsoc_secondary_init(unsigned int cpu)
> /*
> * Synchronise with the boot thread.
> */
> - spin_lock(&boot_lock);
> - spin_unlock(&boot_lock);
> + raw_spin_lock(&boot_lock);
> + raw_spin_unlock(&boot_lock);
> }
>
> static const struct of_device_id clk_ids[] = {
> @@ -75,7 +75,7 @@ static int sirfsoc_boot_secondary(unsigned int cpu, struct task_struct *idle)
> /* make sure write buffer is drained */
> mb();
>
> - spin_lock(&boot_lock);
> + raw_spin_lock(&boot_lock);
>
> /*
> * The secondary processor is waiting to be released from
> @@ -107,7 +107,7 @@ static int sirfsoc_boot_secondary(unsigned int cpu, struct task_struct *idle)
> * now the secondary core is starting up let it run its
> * calibrations, then wait for it to finish
> */
> - spin_unlock(&boot_lock);
> + raw_spin_unlock(&boot_lock);
>
> return pen_release != -1 ? -ENOSYS : 0;
> }
> diff --git a/arch/arm/mach-qcom/platsmp.c b/arch/arm/mach-qcom/platsmp.c
> index 5494c9e0c909b..e8ce157d3548a 100644
> --- a/arch/arm/mach-qcom/platsmp.c
> +++ b/arch/arm/mach-qcom/platsmp.c
> @@ -46,7 +46,7 @@
>
> extern void secondary_startup_arm(void);
>
> -static DEFINE_SPINLOCK(boot_lock);
> +static DEFINE_RAW_SPINLOCK(boot_lock);
>
> #ifdef CONFIG_HOTPLUG_CPU
> static void qcom_cpu_die(unsigned int cpu)
> @@ -60,8 +60,8 @@ static void qcom_secondary_init(unsigned int cpu)
> /*
> * Synchronise with the boot thread.
> */
> - spin_lock(&boot_lock);
> - spin_unlock(&boot_lock);
> + raw_spin_lock(&boot_lock);
> + raw_spin_unlock(&boot_lock);
> }
>
> static int scss_release_secondary(unsigned int cpu)
> @@ -284,7 +284,7 @@ static int qcom_boot_secondary(unsigned int cpu, int (*func)(unsigned int))
> * set synchronisation state between this boot processor
> * and the secondary one
> */
> - spin_lock(&boot_lock);
> + raw_spin_lock(&boot_lock);
>
> /*
> * Send the secondary CPU a soft interrupt, thereby causing
> @@ -297,7 +297,7 @@ static int qcom_boot_secondary(unsigned int cpu, int (*func)(unsigned int))
> * now the secondary core is starting up let it run its
> * calibrations, then wait for it to finish
> */
> - spin_unlock(&boot_lock);
> + raw_spin_unlock(&boot_lock);
>
> return ret;
> }
> diff --git a/arch/arm/mach-spear/platsmp.c b/arch/arm/mach-spear/platsmp.c
> index 39038a03836ac..6da5c93872bf8 100644
> --- a/arch/arm/mach-spear/platsmp.c
> +++ b/arch/arm/mach-spear/platsmp.c
> @@ -32,7 +32,7 @@ static void write_pen_release(int val)
> sync_cache_w(&pen_release);
> }
>
> -static DEFINE_SPINLOCK(boot_lock);
> +static DEFINE_RAW_SPINLOCK(boot_lock);
>
> static void __iomem *scu_base = IOMEM(VA_SCU_BASE);
>
> @@ -47,8 +47,8 @@ static void spear13xx_secondary_init(unsigned int cpu)
> /*
> * Synchronise with the boot thread.
> */
> - spin_lock(&boot_lock);
> - spin_unlock(&boot_lock);
> + raw_spin_lock(&boot_lock);
> + raw_spin_unlock(&boot_lock);
> }
>
> static int spear13xx_boot_secondary(unsigned int cpu, struct task_struct *idle)
> @@ -59,7 +59,7 @@ static int spear13xx_boot_secondary(unsigned int cpu, struct task_struct *idle)
> * set synchronisation state between this boot processor
> * and the secondary one
> */
> - spin_lock(&boot_lock);
> + raw_spin_lock(&boot_lock);
>
> /*
> * The secondary processor is waiting to be released from
> @@ -84,7 +84,7 @@ static int spear13xx_boot_secondary(unsigned int cpu, struct task_struct *idle)
> * now the secondary core is starting up let it run its
> * calibrations, then wait for it to finish
> */
> - spin_unlock(&boot_lock);
> + raw_spin_unlock(&boot_lock);
>
> return pen_release != -1 ? -ENOSYS : 0;
> }
> diff --git a/arch/arm/mach-sti/platsmp.c b/arch/arm/mach-sti/platsmp.c
> index 231f19e174365..a3419b7003e6f 100644
> --- a/arch/arm/mach-sti/platsmp.c
> +++ b/arch/arm/mach-sti/platsmp.c
> @@ -35,7 +35,7 @@ static void write_pen_release(int val)
> sync_cache_w(&pen_release);
> }
>
> -static DEFINE_SPINLOCK(boot_lock);
> +static DEFINE_RAW_SPINLOCK(boot_lock);
>
> static void sti_secondary_init(unsigned int cpu)
> {
> @@ -48,8 +48,8 @@ static void sti_secondary_init(unsigned int cpu)
> /*
> * Synchronise with the boot thread.
> */
> - spin_lock(&boot_lock);
> - spin_unlock(&boot_lock);
> + raw_spin_lock(&boot_lock);
> + raw_spin_unlock(&boot_lock);
> }
>
> static int sti_boot_secondary(unsigned int cpu, struct task_struct *idle)
> @@ -60,7 +60,7 @@ static int sti_boot_secondary(unsigned int cpu, struct task_struct *idle)
> * set synchronisation state between this boot processor
> * and the secondary one
> */
> - spin_lock(&boot_lock);
> + raw_spin_lock(&boot_lock);
>
> /*
> * The secondary processor is waiting to be released from
> @@ -91,7 +91,7 @@ static int sti_boot_secondary(unsigned int cpu, struct task_struct *idle)
> * now the secondary core is starting up let it run its
> * calibrations, then wait for it to finish
> */
> - spin_unlock(&boot_lock);
> + raw_spin_unlock(&boot_lock);
>
> return pen_release != -1 ? -ENOSYS : 0;
> }
> diff --git a/arch/arm/mach-sunxi/mc_smp.c b/arch/arm/mach-sunxi/mc_smp.c
> index b4037b603897d..8a6c872f52b59 100644
> --- a/arch/arm/mach-sunxi/mc_smp.c
> +++ b/arch/arm/mach-sunxi/mc_smp.c
> @@ -367,7 +367,7 @@ static void sunxi_cluster_cache_disable_without_axi(void)
> static int sunxi_mc_smp_cpu_table[SUNXI_NR_CLUSTERS][SUNXI_CPUS_PER_CLUSTER];
> int sunxi_mc_smp_first_comer;
>
> -static DEFINE_SPINLOCK(boot_lock);
> +static DEFINE_RAW_SPINLOCK(boot_lock);
>
> static bool sunxi_mc_smp_cluster_is_down(unsigned int cluster)
> {
> @@ -399,7 +399,7 @@ static int sunxi_mc_smp_boot_secondary(unsigned int l_cpu, struct task_struct *i
> if (cluster >= SUNXI_NR_CLUSTERS || cpu >= SUNXI_CPUS_PER_CLUSTER)
> return -EINVAL;
>
> - spin_lock_irq(&boot_lock);
> + raw_spin_lock_irq(&boot_lock);
>
> if (sunxi_mc_smp_cpu_table[cluster][cpu])
> goto out;
> @@ -417,7 +417,7 @@ static int sunxi_mc_smp_boot_secondary(unsigned int l_cpu, struct task_struct *i
>
> out:
> sunxi_mc_smp_cpu_table[cluster][cpu]++;
> - spin_unlock_irq(&boot_lock);
> + raw_spin_unlock_irq(&boot_lock);
>
> return 0;
> }
> @@ -448,13 +448,13 @@ static void sunxi_mc_smp_cpu_die(unsigned int l_cpu)
> cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
> pr_debug("%s: cluster %u cpu %u\n", __func__, cluster, cpu);
>
> - spin_lock(&boot_lock);
> + raw_spin_lock(&boot_lock);
> sunxi_mc_smp_cpu_table[cluster][cpu]--;
> if (sunxi_mc_smp_cpu_table[cluster][cpu] == 1) {
> /* A power_up request went ahead of us. */
> pr_debug("%s: aborting due to a power up request\n",
> __func__);
> - spin_unlock(&boot_lock);
> + raw_spin_unlock(&boot_lock);
> return;
> } else if (sunxi_mc_smp_cpu_table[cluster][cpu] > 1) {
> pr_err("Cluster %d CPU%d boots multiple times\n",
> @@ -463,7 +463,7 @@ static void sunxi_mc_smp_cpu_die(unsigned int l_cpu)
> }
>
> last_man = sunxi_mc_smp_cluster_is_down(cluster);
> - spin_unlock(&boot_lock);
> + raw_spin_unlock(&boot_lock);
>
> gic_cpu_if_down(0);
> if (last_man)
> @@ -542,11 +542,11 @@ static int sunxi_mc_smp_cpu_kill(unsigned int l_cpu)
>
> /* wait for CPU core to die and enter WFI */
> count = TIMEOUT_USEC / POLL_USEC;
> - spin_lock_irq(&boot_lock);
> + raw_spin_lock_irq(&boot_lock);
> for (tries = 0; tries < count; tries++) {
> - spin_unlock_irq(&boot_lock);
> + raw_spin_unlock_irq(&boot_lock);
> usleep_range(POLL_USEC / 2, POLL_USEC);
> - spin_lock_irq(&boot_lock);
> + raw_spin_lock_irq(&boot_lock);
>
> /*
> * If the user turns off a bunch of cores at the same
> @@ -593,7 +593,7 @@ static int sunxi_mc_smp_cpu_kill(unsigned int l_cpu)
> sunxi_cluster_powerdown(cluster);
>
> out:
> - spin_unlock_irq(&boot_lock);
> + raw_spin_unlock_irq(&boot_lock);
> pr_debug("%s: cluster %u cpu %u powerdown: %d\n",
> __func__, cluster, cpu, ret);
> return !ret;
> diff --git a/arch/arm/plat-versatile/platsmp.c b/arch/arm/plat-versatile/platsmp.c
> index c2366510187a8..6b60f582b738c 100644
> --- a/arch/arm/plat-versatile/platsmp.c
> +++ b/arch/arm/plat-versatile/platsmp.c
> @@ -32,7 +32,7 @@ static void write_pen_release(int val)
> sync_cache_w(&pen_release);
> }
>
> -static DEFINE_SPINLOCK(boot_lock);
> +static DEFINE_RAW_SPINLOCK(boot_lock);
>
> void versatile_secondary_init(unsigned int cpu)
> {
> @@ -45,8 +45,8 @@ void versatile_secondary_init(unsigned int cpu)
> /*
> * Synchronise with the boot thread.
> */
> - spin_lock(&boot_lock);
> - spin_unlock(&boot_lock);
> + raw_spin_lock(&boot_lock);
> + raw_spin_unlock(&boot_lock);
> }
>
> int versatile_boot_secondary(unsigned int cpu, struct task_struct *idle)
> @@ -57,7 +57,7 @@ int versatile_boot_secondary(unsigned int cpu, struct task_struct *idle)
> * Set synchronisation state between this boot processor
> * and the secondary one
> */
> - spin_lock(&boot_lock);
> + raw_spin_lock(&boot_lock);
>
> /*
> * This is really belt and braces; we hold unintended secondary
> @@ -87,7 +87,7 @@ int versatile_boot_secondary(unsigned int cpu, struct task_struct *idle)
> * now the secondary core is starting up let it run its
> * calibrations, then wait for it to finish
> */
> - spin_unlock(&boot_lock);
> + raw_spin_unlock(&boot_lock);
>
> return pen_release != -1 ? -ENOSYS : 0;
> }
> --
> 2.20.0.rc2
>
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] media: cedrus: don't initialize pointers with zero
From: Mauro Carvalho Chehab @ 2018-12-07 12:58 UTC (permalink / raw)
To: Ian Arkver
Cc: devel, Paul Kocialkowski, Maxime Ripard, Greg Kroah-Hartman,
Mauro Carvalho Chehab, Hans Verkuil, Chen-Yu Tsai,
linux-arm-kernel, Linux Media Mailing List
In-Reply-To: <948a841b-efde-b43c-9532-abf72c7a6a97@gmail.com>
Em Fri, 7 Dec 2018 12:27:09 +0000
Ian Arkver <ian.arkver.dev@gmail.com> escreveu:
> On 07/12/2018 11:37, Hans Verkuil wrote:
> > On 12/07/2018 12:31 PM, Mauro Carvalho Chehab wrote:
> >> Em Fri, 7 Dec 2018 12:14:50 +0100
> >> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> >>
> >>> On 12/07/2018 11:56 AM, Mauro Carvalho Chehab wrote:
> >>>> A common mistake is to assume that initializing a var with:
> >>>> struct foo f = { 0 };
> >>>>
> >>>> Would initialize a zeroed struct. Actually, what this does is
> >>>> to initialize the first element of the struct to zero.
> >>>>
> >>>> According to C99 Standard 6.7.8.21:
> >>>>
> >>>> "If there are fewer initializers in a brace-enclosed
> >>>> list than there are elements or members of an aggregate,
> >>>> or fewer characters in a string literal used to initialize
> >>>> an array of known size than there are elements in the array,
> >>>> the remainder of the aggregate shall be initialized implicitly
> >>>> the same as objects that have static storage duration."
> >>>>
> >>>> So, in practice, it could zero the entire struct, but, if the
> >>>> first element is not an integer, it will produce warnings:
> >>>>
> >>>> drivers/staging/media/sunxi/cedrus/cedrus.c:drivers/staging/media/sunxi/cedrus/cedrus.c:78:49: warning: Using plain integer as NULL pointer
> >>>> drivers/staging/media/sunxi/cedrus/cedrus_dec.c:drivers/staging/media/sunxi/cedrus/cedrus_dec.c:29:35: warning: Using plain integer as NULL pointer
> >>>>
> >>>> A proper way to initialize it with gcc is to use:
> >>>>
> >>>> struct foo f = { };
> >>>>
> >>>> But that seems to be a gcc extension. So, I decided to check upstream
> >>>
> >>> No, this is not a gcc extension. It's part of the latest C standard.
> >>
> >> Sure? Where the C standard spec states that? I've been seeking for
> >> such info for a while, as '= {}' is also my personal preference.
> >
> > I believe it was added in C11, but I've loaned the book that stated
> > that to someone else, so I can't check.
>
> Sadly I don't see mention of empty initializer lists in section 6.7.9 of
> ISO/IEC 9899:2011, though I've only got a draft version.
Yeah, as far as I checked, this is not really part of the standard.
Depending on how you read:
"If there are fewer initializers in a brace-enclosed
list than there are elements or members of an aggregate,
or fewer characters in a string literal used to initialize
an array of known size than there are elements in the array,
the remainder of the aggregate shall be initialized implicitly
the same as objects that have static storage duration."
One may infere that a brace-enclosed list with zero elements
would fit, and "the remainder of the aggregate shall be
initialized implicitly".
I suspect that this is how gcc people interpreted it.
> I had a play with Compiler Explorer[1] and it seems like clang is OK
> with the {} form though just out of interest msvc isn't.
Yeah, I'm aware that msvc won't support it. Good to know that clang
does the right thing cleaning the struct.
To be realistic, we only really care with gcc at the Kernel side, as
clang upstream versions still can't build upstream Kernels, and
nobody uses any other compiler for the Kernel anymore. Yet, with
regards to clang, there's a push to let it to build the Kernel.
So, it seems wise to add something that would work for both.
Anyway, I'll post a version 2 of this patch using ={} and placing
some rationale on it.
> I didn't try
> exploring other command line options.
>
> [1] https://gcc.godbolt.org/z/XIDC7W
>
> Regards,
> Ian
> >
> > In any case, it's used everywhere:
> >
> > git grep '= *{ *}' drivers/
> >
> > So it is really pointless to *not* use it.
> >
> > Regards,
> >
> > Hans
> >
> >> I tried to build the Kernel with clang, just to be sure that this
> >> won't cause issues with the clang support, but, unfortunately, the
> >> clang compiler (not even the upstream version) is able to build
> >> the upstream Kernel yet, as it lacks asm-goto support (there is an
> >> OOT patch for llvm solving it - but it sounds too much effort for
> >> my time to have to build llvm from their sources just for a simple
> >> check like this).
> >>
> >>> It's used all over in the kernel, so please use {} instead of { NULL }.
> >>>
> >>> Personally I think {} is a fantastic invention and I wish C++ had it as
> >>> well.
> >>
> >> Fully agreed on that.
> >>
> >>>
> >>> Regards,
> >>>
> >>> Hans
> >>>
> >>>> what's the most commonly pattern. The gcc pattern has about 2000 entries:
> >>>>
> >>>> $ git grep -E "=\s*\{\s*\}"|wc -l
> >>>> 1951
> >>>>
> >>>> The standard-C compliant pattern has about 2500 entries:
> >>>>
> >>>> $ git grep -E "=\s*\{\s*NULL\s*\}"|wc -l
> >>>> 137
> >>>> $ git grep -E "=\s*\{\s*0\s*\}"|wc -l
> >>>> 2323
> >>>>
> >>>> So, let's initialize those structs with:
> >>>> = { NULL }
> >>>>
> >>>> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
> >>>> ---
> >>>> drivers/staging/media/sunxi/cedrus/cedrus.c | 2 +-
> >>>> drivers/staging/media/sunxi/cedrus/cedrus_dec.c | 2 +-
> >>>> 2 files changed, 2 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
> >>>> index b538eb0321d8..44c45c687503 100644
> >>>> --- a/drivers/staging/media/sunxi/cedrus/cedrus.c
> >>>> +++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
> >>>> @@ -75,7 +75,7 @@ static int cedrus_init_ctrls(struct cedrus_dev *dev, struct cedrus_ctx *ctx)
> >>>> memset(ctx->ctrls, 0, ctrl_size);
> >>>>
> >>>> for (i = 0; i < CEDRUS_CONTROLS_COUNT; i++) {
> >>>> - struct v4l2_ctrl_config cfg = { 0 };
> >>>> + struct v4l2_ctrl_config cfg = { NULL };
> >>>>
> >>>> cfg.elem_size = cedrus_controls[i].elem_size;
> >>>> cfg.id = cedrus_controls[i].id;
> >>>> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> >>>> index e40180a33951..4099a42dba2d 100644
> >>>> --- a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> >>>> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> >>>> @@ -26,7 +26,7 @@ void cedrus_device_run(void *priv)
> >>>> {
> >>>> struct cedrus_ctx *ctx = priv;
> >>>> struct cedrus_dev *dev = ctx->dev;
> >>>> - struct cedrus_run run = { 0 };
> >>>> + struct cedrus_run run = { NULL };
> >>>> struct media_request *src_req;
> >>>> unsigned long flags;
> >>>>
> >>>>
> >>>
> >>
> >>
> >>
> >> Thanks,
> >> Mauro
> >>
> >
Thanks,
Mauro
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox