* [PATCH v2 11/15] net: phy: adin: implement PHY subsystem software reset
From: Alexandru Ardelean @ 2019-08-08 12:30 UTC (permalink / raw)
To: netdev, devicetree, linux-kernel
Cc: davem, robh+dt, mark.rutland, f.fainelli, hkallweit1, andrew,
Alexandru Ardelean
In-Reply-To: <20190808123026.17382-1-alexandru.ardelean@analog.com>
The ADIN PHYs supports 4 types of reset:
1. The standard PHY reset via BMCR_RESET bit in MII_BMCR reg
2. Reset via GPIO
3. Reset via reg GeSftRst (0xff0c) & reload previous pin configs
4. Reset via reg GeSftRst (0xff0c) & request new pin configs
Resets 2 & 4 are almost identical, with the exception that the crystal
oscillator is available during reset for 2.
As it turns out, phylib already supports GPIO reset.
In case this is configured, the PHY driver won't do anything. In case it
isn't specified the subsystem software reset will kick in.
Resetting via GeSftRst or via GPIO is useful when doing a warm reboot,
because this will reset the subsystem registers to default values.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
drivers/net/phy/adin.c | 43 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 43 insertions(+)
diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index c1cea1b6bd75..f276d692bdee 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -6,6 +6,7 @@
*/
#include <linux/kernel.h>
#include <linux/bitfield.h>
+#include <linux/delay.h>
#include <linux/errno.h>
#include <linux/init.h>
#include <linux/module.h>
@@ -53,6 +54,9 @@
#define ADIN1300_CLOCK_STOP_REG 0x9400
#define ADIN1300_LPI_WAKE_ERR_CNT_REG 0xa000
+#define ADIN1300_GE_SOFT_RESET_REG 0xff0c
+#define ADIN1300_GE_SOFT_RESET BIT(0)
+
#define ADIN1300_GE_RGMII_CFG_REG 0xff23
#define ADIN1300_GE_RGMII_RX_MSK GENMASK(8, 6)
#define ADIN1300_GE_RGMII_RX_SEL(x) \
@@ -457,11 +461,49 @@ static int adin_read_status(struct phy_device *phydev)
return genphy_read_status(phydev);
}
+static int adin_subsytem_soft_reset(struct phy_device *phydev)
+{
+ int reg, rc, i;
+
+ rc = phy_set_bits_mmd(phydev, MDIO_MMD_VEND1,
+ ADIN1300_GE_SOFT_RESET_REG,
+ ADIN1300_GE_SOFT_RESET);
+ if (rc < 0)
+ return rc;
+
+ for (i = 0; i < 20; i++) {
+ usleep_range(500, 1000);
+ reg = phy_read_mmd(phydev, MDIO_MMD_VEND1,
+ ADIN1300_GE_SOFT_RESET_REG);
+ if (reg < 0 || (reg & ADIN1300_GE_SOFT_RESET))
+ continue;
+ return 0;
+ }
+
+ return -ETIMEDOUT;
+}
+
+static int adin_reset(struct phy_device *phydev)
+{
+ /* If there is a reset GPIO just exit */
+ if (!IS_ERR_OR_NULL(phydev->mdio.reset_gpio))
+ return 0;
+
+ /* Reset PHY core regs & subsystem regs */
+ return adin_subsytem_soft_reset(phydev);
+}
+
+static int adin_probe(struct phy_device *phydev)
+{
+ return adin_reset(phydev);
+}
+
static struct phy_driver adin_driver[] = {
{
PHY_ID_MATCH_MODEL(PHY_ID_ADIN1200),
.name = "ADIN1200",
.config_init = adin_config_init,
+ .probe = adin_probe,
.config_aneg = adin_config_aneg,
.read_status = adin_read_status,
.get_features = genphy_read_abilities,
@@ -476,6 +518,7 @@ static struct phy_driver adin_driver[] = {
PHY_ID_MATCH_MODEL(PHY_ID_ADIN1300),
.name = "ADIN1300",
.config_init = adin_config_init,
+ .probe = adin_probe,
.config_aneg = adin_config_aneg,
.read_status = adin_read_status,
.get_features = genphy_read_abilities,
--
2.20.1
^ permalink raw reply related
* [PATCH v2 12/15] net: phy: adin: implement Energy Detect Powerdown mode
From: Alexandru Ardelean @ 2019-08-08 12:30 UTC (permalink / raw)
To: netdev, devicetree, linux-kernel
Cc: davem, robh+dt, mark.rutland, f.fainelli, hkallweit1, andrew,
Alexandru Ardelean
In-Reply-To: <20190808123026.17382-1-alexandru.ardelean@analog.com>
The ADIN PHYs support Energy Detect Powerdown mode, which puts the PHY into
a low power mode when there is no signal on the wire (typically cable
unplugged).
This behavior is enabled by default, but can be disabled via device
property.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
drivers/net/phy/adin.c | 40 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index f276d692bdee..bc4393195de7 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -24,6 +24,11 @@
#define ADIN1300_AUTO_MDI_EN BIT(10)
#define ADIN1300_MAN_MDIX_EN BIT(9)
+#define ADIN1300_PHY_CTRL_STATUS2 0x0015
+#define ADIN1300_NRG_PD_EN BIT(3)
+#define ADIN1300_NRG_PD_TX_EN BIT(2)
+#define ADIN1300_NRG_PD_STATUS BIT(1)
+
#define ADIN1300_INT_MASK_REG 0x0018
#define ADIN1300_INT_MDIO_SYNC_EN BIT(9)
#define ADIN1300_INT_ANEG_STAT_CHNG_EN BIT(8)
@@ -138,6 +143,14 @@ static struct adin_clause45_mmd_map adin_clause45_mmd_map[] = {
{ MDIO_MMD_PCS, MDIO_PCS_EEE_WK_ERR, ADIN1300_LPI_WAKE_ERR_CNT_REG },
};
+/**
+ * struct adin_priv - ADIN PHY driver private data
+ * edpd_enabled true if Energy Detect Powerdown mode is enabled
+ */
+struct adin_priv {
+ bool edpd_enabled;
+};
+
static int adin_lookup_reg_value(const struct adin_cfg_reg_map *tbl, int cfg)
{
size_t i;
@@ -246,6 +259,18 @@ static int adin_config_rmii_mode(struct phy_device *phydev)
ADIN1300_GE_RMII_CFG_REG, reg);
}
+static int adin_config_init_edpd(struct phy_device *phydev)
+{
+ struct adin_priv *priv = phydev->priv;
+
+ if (priv->edpd_enabled)
+ return phy_set_bits(phydev, ADIN1300_PHY_CTRL_STATUS2,
+ (ADIN1300_NRG_PD_EN | ADIN1300_NRG_PD_TX_EN));
+
+ return phy_clear_bits(phydev, ADIN1300_PHY_CTRL_STATUS2,
+ (ADIN1300_NRG_PD_EN | ADIN1300_NRG_PD_TX_EN));
+}
+
static int adin_config_init(struct phy_device *phydev)
{
int rc;
@@ -264,6 +289,10 @@ static int adin_config_init(struct phy_device *phydev)
if (rc < 0)
return rc;
+ rc = adin_config_init_edpd(phydev);
+ if (rc < 0)
+ return rc;
+
phydev_dbg(phydev, "PHY is using mode '%s'\n",
phy_modes(phydev->interface));
@@ -495,6 +524,17 @@ static int adin_reset(struct phy_device *phydev)
static int adin_probe(struct phy_device *phydev)
{
+ struct device *dev = &phydev->mdio.dev;
+ struct adin_priv *priv;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->edpd_enabled =
+ device_property_read_bool(dev, "adi,disable-energy-detect");
+ phydev->priv = priv;
+
return adin_reset(phydev);
}
--
2.20.1
^ permalink raw reply related
* [PATCH v2 13/15] net: phy: adin: configure downshift on config_init
From: Alexandru Ardelean @ 2019-08-08 12:30 UTC (permalink / raw)
To: netdev, devicetree, linux-kernel
Cc: davem, robh+dt, mark.rutland, f.fainelli, hkallweit1, andrew,
Alexandru Ardelean
In-Reply-To: <20190808123026.17382-1-alexandru.ardelean@analog.com>
Down-speed auto-negotiation may not always be enabled, in which case the
PHY won't down-shift to 100 or 10 during auto-negotiation.
This change enables downshift and configures the number of retries to
default 8 (maximum supported value).
The change has been adapted from the Marvell PHY driver.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
drivers/net/phy/adin.c | 39 +++++++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index bc4393195de7..d6d1f5037eb7 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -29,6 +29,18 @@
#define ADIN1300_NRG_PD_TX_EN BIT(2)
#define ADIN1300_NRG_PD_STATUS BIT(1)
+#define ADIN1300_PHY_CTRL2 0x0016
+#define ADIN1300_DOWNSPEED_AN_100_EN BIT(11)
+#define ADIN1300_DOWNSPEED_AN_10_EN BIT(10)
+#define ADIN1300_GROUP_MDIO_EN BIT(6)
+#define ADIN1300_DOWNSPEEDS_EN \
+ (ADIN1300_DOWNSPEED_AN_100_EN | ADIN1300_DOWNSPEED_AN_10_EN)
+
+#define ADIN1300_PHY_CTRL3 0x0017
+#define ADIN1300_LINKING_EN BIT(13)
+#define ADIN1300_DOWNSPEED_RETRIES_MSK GENMASK(12, 10)
+#define ADIN1300_DOWNSPEED_RETRIES_OFF 10
+
#define ADIN1300_INT_MASK_REG 0x0018
#define ADIN1300_INT_MDIO_SYNC_EN BIT(9)
#define ADIN1300_INT_ANEG_STAT_CHNG_EN BIT(8)
@@ -259,6 +271,29 @@ static int adin_config_rmii_mode(struct phy_device *phydev)
ADIN1300_GE_RMII_CFG_REG, reg);
}
+static int adin_config_down_shift(struct phy_device *phydev, bool enable,
+ u8 retries)
+{
+ u16 mask, set;
+ int rc;
+
+ if (!enable)
+ return phy_clear_bits(phydev, ADIN1300_PHY_CTRL2,
+ ADIN1300_DOWNSPEEDS_EN);
+
+ mask = ADIN1300_LINKING_EN | ADIN1300_DOWNSPEED_RETRIES_MSK;
+ set = (retries << ADIN1300_DOWNSPEED_RETRIES_OFF);
+ set &= ADIN1300_DOWNSPEED_RETRIES_MSK;
+ set |= ADIN1300_LINKING_EN;
+
+ rc = phy_modify_changed(phydev, ADIN1300_PHY_CTRL3, mask, set);
+ if (rc < 0)
+ return rc;
+
+ return phy_set_bits(phydev, ADIN1300_PHY_CTRL2,
+ ADIN1300_DOWNSPEEDS_EN);
+}
+
static int adin_config_init_edpd(struct phy_device *phydev)
{
struct adin_priv *priv = phydev->priv;
@@ -289,6 +324,10 @@ static int adin_config_init(struct phy_device *phydev)
if (rc < 0)
return rc;
+ rc = adin_config_down_shift(phydev, true, 8);
+ if (rc < 0)
+ return rc;
+
rc = adin_config_init_edpd(phydev);
if (rc < 0)
return rc;
--
2.20.1
^ permalink raw reply related
* [PATCH v2 14/15] net: phy: adin: add ethtool get_stats support
From: Alexandru Ardelean @ 2019-08-08 12:30 UTC (permalink / raw)
To: netdev, devicetree, linux-kernel
Cc: davem, robh+dt, mark.rutland, f.fainelli, hkallweit1, andrew,
Alexandru Ardelean
In-Reply-To: <20190808123026.17382-1-alexandru.ardelean@analog.com>
This change implements retrieving all the error counters from the PHY.
The PHY supports several error counters/stats. The `Mean Square Errors`
status values are only valie when a link is established, and shouldn't be
accumulated. These values characterize the quality of a signal.
The rest of the error counters are self-clearing on read.
Most of them are reports from the Frame Checker engine that the PHY has.
Not retrieving the `LPI Wake Error Count Register` here, since that is used
by the PHY framework to check for any EEE errors. And that register is
self-clearing when read (as per IEEE spec).
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
drivers/net/phy/adin.c | 109 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 109 insertions(+)
diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index d6d1f5037eb7..d170d1e837b5 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -155,12 +155,40 @@ static struct adin_clause45_mmd_map adin_clause45_mmd_map[] = {
{ MDIO_MMD_PCS, MDIO_PCS_EEE_WK_ERR, ADIN1300_LPI_WAKE_ERR_CNT_REG },
};
+struct adin_hw_stat {
+ const char *string;
+ u16 reg1;
+ u16 reg2;
+ bool do_not_accumulate;
+};
+
+/* Named just like in the datasheet */
+static struct adin_hw_stat adin_hw_stats[] = {
+ { "RxErrCnt", 0x0014, },
+ { "MseA", 0x8402, 0, true },
+ { "MseB", 0x8403, 0, true },
+ { "MseC", 0x8404, 0, true },
+ { "MseD", 0x8405, 0, true },
+ { "FcFrmCnt", 0x940A, 0x940B }, /* FcFrmCntH + FcFrmCntL */
+ { "FcLenErrCnt", 0x940C },
+ { "FcAlgnErrCnt", 0x940D },
+ { "FcSymbErrCnt", 0x940E },
+ { "FcOszCnt", 0x940F },
+ { "FcUszCnt", 0x9410 },
+ { "FcOddCnt", 0x9411 },
+ { "FcOddPreCnt", 0x9412 },
+ { "FcDribbleBitsCnt", 0x9413 },
+ { "FcFalseCarrierCnt", 0x9414 },
+};
+
/**
* struct adin_priv - ADIN PHY driver private data
* edpd_enabled true if Energy Detect Powerdown mode is enabled
+ * stats statistic counters for the PHY
*/
struct adin_priv {
bool edpd_enabled;
+ u64 stats[ARRAY_SIZE(adin_hw_stats)];
};
static int adin_lookup_reg_value(const struct adin_cfg_reg_map *tbl, int cfg)
@@ -561,6 +589,81 @@ static int adin_reset(struct phy_device *phydev)
return adin_subsytem_soft_reset(phydev);
}
+static int adin_get_sset_count(struct phy_device *phydev)
+{
+ return ARRAY_SIZE(adin_hw_stats);
+}
+
+static void adin_get_strings(struct phy_device *phydev, u8 *data)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(adin_hw_stats); i++) {
+ strlcpy(&data[i * ETH_GSTRING_LEN],
+ adin_hw_stats[i].string, ETH_GSTRING_LEN);
+ }
+}
+
+static int adin_read_mmd_stat_regs(struct phy_device *phydev,
+ struct adin_hw_stat *stat,
+ u32 *val)
+{
+ int ret;
+
+ ret = phy_read_mmd(phydev, MDIO_MMD_VEND1, stat->reg1);
+ if (ret < 0)
+ return ret;
+
+ *val = (ret & 0xffff);
+
+ if (stat->reg2 == 0)
+ return 0;
+
+ ret = phy_read_mmd(phydev, MDIO_MMD_VEND1, stat->reg2);
+ if (ret < 0)
+ return ret;
+
+ *val <<= 16;
+ *val |= (ret & 0xffff);
+
+ return 0;
+}
+
+static u64 adin_get_stat(struct phy_device *phydev, int i)
+{
+ struct adin_hw_stat *stat = &adin_hw_stats[i];
+ struct adin_priv *priv = phydev->priv;
+ u32 val;
+ int ret;
+
+ if (stat->reg1 > 0x1f) {
+ ret = adin_read_mmd_stat_regs(phydev, stat, &val);
+ if (ret < 0)
+ return (u64)(~0);
+ } else {
+ ret = phy_read(phydev, stat->reg1);
+ if (ret < 0)
+ return (u64)(~0);
+ val = (ret & 0xffff);
+ }
+
+ if (stat->do_not_accumulate)
+ priv->stats[i] = val;
+ else
+ priv->stats[i] += val;
+
+ return priv->stats[i];
+}
+
+static void adin_get_stats(struct phy_device *phydev,
+ struct ethtool_stats *stats, u64 *data)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(adin_hw_stats); i++)
+ data[i] = adin_get_stat(phydev, i);
+}
+
static int adin_probe(struct phy_device *phydev)
{
struct device *dev = &phydev->mdio.dev;
@@ -588,6 +691,9 @@ static struct phy_driver adin_driver[] = {
.get_features = genphy_read_abilities,
.ack_interrupt = adin_phy_ack_intr,
.config_intr = adin_phy_config_intr,
+ .get_sset_count = adin_get_sset_count,
+ .get_strings = adin_get_strings,
+ .get_stats = adin_get_stats,
.resume = genphy_resume,
.suspend = genphy_suspend,
.read_mmd = adin_read_mmd,
@@ -603,6 +709,9 @@ static struct phy_driver adin_driver[] = {
.get_features = genphy_read_abilities,
.ack_interrupt = adin_phy_ack_intr,
.config_intr = adin_phy_config_intr,
+ .get_sset_count = adin_get_sset_count,
+ .get_strings = adin_get_strings,
+ .get_stats = adin_get_stats,
.resume = genphy_resume,
.suspend = genphy_suspend,
.read_mmd = adin_read_mmd,
--
2.20.1
^ permalink raw reply related
* [PATCH v2 15/15] dt-bindings: net: add bindings for ADIN PHY driver
From: Alexandru Ardelean @ 2019-08-08 12:30 UTC (permalink / raw)
To: netdev, devicetree, linux-kernel
Cc: davem, robh+dt, mark.rutland, f.fainelli, hkallweit1, andrew,
Alexandru Ardelean
In-Reply-To: <20190808123026.17382-1-alexandru.ardelean@analog.com>
This change adds bindings for the Analog Devices ADIN PHY driver, detailing
all the properties implemented by the driver.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
.../devicetree/bindings/net/adi,adin.yaml | 76 +++++++++++++++++++
MAINTAINERS | 1 +
2 files changed, 77 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/adi,adin.yaml
diff --git a/Documentation/devicetree/bindings/net/adi,adin.yaml b/Documentation/devicetree/bindings/net/adi,adin.yaml
new file mode 100644
index 000000000000..86177c8fe23a
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/adi,adin.yaml
@@ -0,0 +1,76 @@
+# SPDX-License-Identifier: GPL-2.0+
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/adi,adin.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices ADIN1200/ADIN1300 PHY
+
+maintainers:
+ - Alexandru Ardelean <alexandru.ardelean@analog.com>
+
+description: |
+ Bindings for Analog Devices Industrial Ethernet PHYs
+
+allOf:
+ - $ref: ethernet-phy.yaml#
+
+properties:
+ adi,rx-internal-delay-ps:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: |
+ RGMII RX Clock Delay used only when PHY operates in RGMII mode with
+ internal delay (phy-mode is 'rgmii-id' or 'rgmii-rxid') in pico-seconds.
+ enum: [ 1600, 1800, 2000, 2200, 2400 ]
+ default: 2000
+
+ adi,tx-internal-delay-ps:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: |
+ RGMII TX Clock Delay used only when PHY operates in RGMII mode with
+ internal delay (phy-mode is 'rgmii-id' or 'rgmii-txid') in pico-seconds.
+ enum: [ 1600, 1800, 2000, 2200, 2400 ]
+ default: 2000
+
+ adi,fifo-depth-bits:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: |
+ When operating in RMII mode, this option configures the FIFO depth.
+ enum: [ 4, 8, 12, 16, 20, 24 ]
+ default: 8
+
+ adi,disable-energy-detect:
+ description: |
+ Disables Energy Detect Powerdown Mode (default disabled, i.e energy detect
+ is enabled if this property is unspecified)
+ type: boolean
+
+examples:
+ - |
+ ethernet {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ phy-mode = "rgmii-id";
+
+ ethernet-phy@0 {
+ reg = <0>;
+
+ adi,rx-internal-delay-ps = <1800>;
+ adi,tx-internal-delay-ps = <2200>;
+ };
+ };
+ - |
+ ethernet {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ phy-mode = "rmii";
+
+ ethernet-phy@1 {
+ reg = <1>;
+
+ adi,fifo-depth-bits = <16>;
+ adi,disable-energy-detect;
+ };
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index e8aa8a667864..fd9ab61c2670 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -944,6 +944,7 @@ L: netdev@vger.kernel.org
W: http://ez.analog.com/community/linux-device-drivers
S: Supported
F: drivers/net/phy/adin.c
+F: Documentation/devicetree/bindings/net/adi,adin.yaml
ANALOG DEVICES INC ADIS DRIVER LIBRARY
M: Alexandru Ardelean <alexandru.ardelean@analog.com>
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v2 3/9] soc: samsung: Add Exynos Adaptive Supply Voltage driver
From: Krzysztof Kozlowski @ 2019-08-08 12:31 UTC (permalink / raw)
To: Sylwester Nawrocki
Cc: devicetree, linux-samsung-soc@vger.kernel.org, linux-pm,
pankaj.dubey, Bartłomiej Żołnierkiewicz,
linux-kernel@vger.kernel.org, robh+dt, kgene, vireshk,
linux-arm-kernel, Marek Szyprowski
In-Reply-To: <a56fe2d8-1f26-b462-1564-f23902f7dbb5@samsung.com>
On Thu, 8 Aug 2019 at 14:07, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
> >> +static unsigned int exynos5422_asv_parse_table(struct exynos_asv *asv,
> >> + unsigned int pkg_id)
> >> +{
> >> + return (pkg_id >> EXYNOS5422_TABLE_OFFSET) & EXYNOS5422_TABLE_MASK;
> >> +}
> >> +
> >> +static bool exynos5422_asv_parse_bin2(struct exynos_asv *asv,
> >> + unsigned int pkg_id)
> >> +{
> >> + return (pkg_id >> EXYNOS5422_BIN2_OFFSET) & EXYNOS5422_BIN2_MASK;
> >
> > return !!() for converting to boolean.
>
> I'm not convinced it is needed, the return type of the function is bool
> and value of the expression will be implicitly converted to that type.
> Is there any compiler warning related to that?
Yeah, but bool is int so there will be no implicit conversion... I
guess it is a convention. In theory !! is the proper conversion to
bool but if bool==int then it's essentially conversion to 1. I am not
sure what's the benefit, maybe for some wrong code which would do
comparisons on result like if (exynos5422_asv_parse_bin2() == TRUE)...
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 2/2] ARM: dts: at91: sama5d27_som1_ek: add mmc capabilities for SDMMC0
From: Ludovic Desroches @ 2019-08-08 12:42 UTC (permalink / raw)
To: Eugen Hristev - M18282
Cc: Nicolas Ferre - M43238, alexandre.belloni@bootlin.com,
adrian.hunter@intel.com, ulf.hansson@linaro.org,
linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-mmc@vger.kernel.org
In-Reply-To: <1565252928-28994-2-git-send-email-eugen.hristev@microchip.com>
On Thu, Aug 08, 2019 at 10:35:43AM +0200, Eugen Hristev - M18282 wrote:
> From: Eugen Hristev <eugen.hristev@microchip.com>
>
> Add mmc capabilities for SDMMC0 for this board.
> With this enabled, eMMC connected card is detected as:
>
> mmc0: new DDR MMC card at address 0001
>
> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
Acked-by: Ludovic Desroches <ludovic.desroches@microchip.com>
I am interested to have the some insights about the use of sd-uhs-*
properties.
Our IP can't deal with 1V8 by itself. It has a 1V8SEL signal which can
be used as the logic control input of a mux. So even if the IP claims
to support UHS modes, it depends on the board.
Are the sd-uhs-* properties a way to deal with this? I tend to think no
as sdhci_setup_host() will set the caps depending on the content of the
capabilities register. Do we have to use the SDHCI_QUIRK_MISSING_CAPS
quirk or sdhci-caps/sdhci-caps-mask?
Regards
Ludovic
> ---
> arch/arm/boot/dts/at91-sama5d27_som1_ek.dts | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/boot/dts/at91-sama5d27_som1_ek.dts b/arch/arm/boot/dts/at91-sama5d27_som1_ek.dts
> index 149e539..194b3a3 100644
> --- a/arch/arm/boot/dts/at91-sama5d27_som1_ek.dts
> +++ b/arch/arm/boot/dts/at91-sama5d27_som1_ek.dts
> @@ -54,6 +54,7 @@
>
> sdmmc0: sdio-host@a0000000 {
> bus-width = <8>;
> + mmc-ddr-3_3v;
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_sdmmc0_default>;
> status = "okay";
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH 1/2] mmc: sdhci-of-at91: add quirk for broken HS200
From: Ludovic Desroches @ 2019-08-08 12:42 UTC (permalink / raw)
To: Eugen Hristev - M18282
Cc: Nicolas Ferre - M43238, alexandre.belloni@bootlin.com,
adrian.hunter@intel.com, ulf.hansson@linaro.org,
linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-mmc@vger.kernel.org
In-Reply-To: <1565252928-28994-1-git-send-email-eugen.hristev@microchip.com>
On Thu, Aug 08, 2019 at 10:35:40AM +0200, Eugen Hristev - M18282 wrote:
> From: Eugen Hristev <eugen.hristev@microchip.com>
>
> HS200 is not implemented in the driver, but the controller claims it
> through caps.
> Remove it via quirk.
> Without this quirk, the mmc core will try to enable hs200, which will fail,
> and the eMMC initialization will fail.
>
> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
Acked-by: Ludovic Desroches <ludovic.desroches@microchip.com>
Thanks
Ludovic
> ---
> drivers/mmc/host/sdhci-of-at91.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-of-at91.c b/drivers/mmc/host/sdhci-of-at91.c
> index 57fe3b2..3a8c6d8 100644
> --- a/drivers/mmc/host/sdhci-of-at91.c
> +++ b/drivers/mmc/host/sdhci-of-at91.c
> @@ -370,6 +370,9 @@ static int sdhci_at91_probe(struct platform_device *pdev)
> pm_runtime_set_autosuspend_delay(&pdev->dev, 50);
> pm_runtime_use_autosuspend(&pdev->dev);
>
> + /* HS200 is broken at this moment */
> + host->quirks2 = SDHCI_QUIRK2_BROKEN_HS200;
> +
> ret = sdhci_add_host(host);
> if (ret)
> goto pm_runtime_disable;
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH 5/6] tty: serial: Add linflexuart driver for S32V234
From: Stefan-gabriel Mirea @ 2019-08-08 12:47 UTC (permalink / raw)
To: Will Deacon
Cc: corbet@lwn.net, robh+dt@kernel.org, mark.rutland@arm.com,
gregkh@linuxfoundation.org, catalin.marinas@arm.com,
shawnguo@kernel.org, Leo Li, jslaby@suse.com,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
devicetree@vger.kernel.org, linux-serial@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, Cosmin Stefan Stoica,
Larisa Ileana Grigore
In-Reply-To: <20190808080832.nleult5bknmzr3ze@willie-the-truck>
Hello Will,
On 8/8/2019 11:08 AM, Will Deacon wrote:
> On Fri, Aug 02, 2019 at 07:47:23PM +0000, Stefan-gabriel Mirea wrote:
>> + linflex,<addr>
>> + Use early console provided by Freescale LinFlex UART
>> + serial driver for NXP S32V234 SoCs. A valid base
>> + address must be provided, and the serial port must
>> + already be setup and configured.
>
> Why isn't earlycon= sufficient for this?
"earlycon=" is not actually supported. I will fix this in the next
version by adding a /chosen/stdout-path to the dts. The compatible
string provided to OF_EARLYCON_DECLARE will also be changed from
"fsl,s32v234-linflexuart" to "fsl,s32-linflexuart" to match the one in
the device tree nodes. I missed this after importing a rename from our
codebase.
Should I remove this addition from kernel-parameters.txt after that?
Regards,
Stefan
^ permalink raw reply
* Re: [PATCH v2 3/9] soc: samsung: Add Exynos Adaptive Supply Voltage driver
From: Robin Murphy @ 2019-08-08 12:48 UTC (permalink / raw)
To: Krzysztof Kozlowski, Sylwester Nawrocki
Cc: devicetree, linux-samsung-soc@vger.kernel.org, linux-pm,
pankaj.dubey, Bartłomiej Żołnierkiewicz,
linux-kernel@vger.kernel.org, robh+dt, kgene, vireshk,
linux-arm-kernel, Marek Szyprowski
In-Reply-To: <CAJKOXPc8iFo=2JAGEZSC46N3sZae4+JcZYBCjpKysb6PFPzyaQ@mail.gmail.com>
On 08/08/2019 13:31, Krzysztof Kozlowski wrote:
> On Thu, 8 Aug 2019 at 14:07, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
>>>> +static unsigned int exynos5422_asv_parse_table(struct exynos_asv *asv,
>>>> + unsigned int pkg_id)
>>>> +{
>>>> + return (pkg_id >> EXYNOS5422_TABLE_OFFSET) & EXYNOS5422_TABLE_MASK;
>>>> +}
>>>> +
>>>> +static bool exynos5422_asv_parse_bin2(struct exynos_asv *asv,
>>>> + unsigned int pkg_id)
>>>> +{
>>>> + return (pkg_id >> EXYNOS5422_BIN2_OFFSET) & EXYNOS5422_BIN2_MASK;
>>>
>>> return !!() for converting to boolean.
>>
>> I'm not convinced it is needed, the return type of the function is bool
>> and value of the expression will be implicitly converted to that type.
>> Is there any compiler warning related to that?
>
> Yeah, but bool is int so there will be no implicit conversion... I
> guess it is a convention. In theory !! is the proper conversion to
> bool but if bool==int then it's essentially conversion to 1. I am not
> sure what's the benefit, maybe for some wrong code which would do
> comparisons on result like if (exynos5422_asv_parse_bin2() == TRUE)...
Not so - since we use "-std=gnu89", we have C99-like _Bool, which our
bool is a typedef of. Conversions, either implicit or explicit, are
well-defined:
"6.3.1.2 Boolean type
When any scalar value is converted to _Bool, the result is 0 if the
value compares equal
to 0; otherwise, the result is 1."
This is even called out in Documentation/process/coding-style.rst:
"When using bool types the !! construction is not needed, which
eliminates a class of bugs."
Robin.
^ permalink raw reply
* Re: [PATCH 0/6] hwspinlock: allow sharing of hwspinlocks
From: Fabien DESSENNE @ 2019-08-08 12:52 UTC (permalink / raw)
To: Suman Anna, Bjorn Andersson
Cc: Ohad Ben-Cohen, Mark Rutland, Alexandre TORGUE, Jonathan Corbet,
linux-doc@vger.kernel.org, linux-remoteproc@vger.kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
Rob Herring, Maxime Coquelin,
linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org, Benjamin GAIGNARD
In-Reply-To: <f0893b3f-0124-007a-3ca2-831f60ad9a80@ti.com>
On 07/08/2019 6:19 PM, Suman Anna wrote:
> Hi Fabien,
>
> On 8/7/19 3:39 AM, Fabien DESSENNE wrote:
>> Hi
>>
>> On 06/08/2019 11:30 PM, Suman Anna wrote:
>>> On 8/6/19 1:21 PM, Bjorn Andersson wrote:
>>>> On Tue 06 Aug 10:38 PDT 2019, Suman Anna wrote:
>>>>
>>>>> Hi Fabien,
>>>>>
>>>>> On 8/5/19 12:46 PM, Bjorn Andersson wrote:
>>>>>> On Mon 05 Aug 01:48 PDT 2019, Fabien DESSENNE wrote:
>>>>>>
>>>>>>> On 01/08/2019 9:14 PM, Bjorn Andersson wrote:
>>>>>>>> On Wed 13 Mar 08:50 PDT 2019, Fabien Dessenne wrote:
>>>> [..]
>>>>>>> B/ This would introduce some inconsistency between the two 'request' API
>>>>>>> which are hwspin_lock_request() and hwspin_lock_request_specific().
>>>>>>> hwspin_lock_request() looks for an unused lock, so requests for an exclusive
>>>>>>> usage. On the other side, request_specific() would request shared locks.
>>>>>>> Worst the following sequence can transform an exclusive usage into a shared
>>>>>>>
>>>>>> There is already an inconsistency in between these; as with above any
>>>>>> system that uses both request() and request_specific() will be suffering
>>>>>> from intermittent failures due to probe ordering.
>>>>>>
>>>>>>> one:
>>>>>>> -hwspin_lock_request() -> returns Id#0 (exclusive)
>>>>>>> -hwspin_lock_request() -> returns Id#1 (exclusive)
>>>>>>> -hwspin_lock_request_specific(0) -> returns Id#0 and makes Id#0 shared
>>>>>>> Honestly I am not sure that this is a real issue, but it's better to have it
>>>>>>> in mind before we take ay decision
>>>>> Wouldn't it be actually simpler to just introduce a new specific API
>>>>> variant for this, similar to the reset core for example (it uses a
>>>>> separate exclusive API), without having to modify the bindings at all.
>>>>> It is just a case of your driver using the right API, and the core can
>>>>> be modified to use the additional tag semantics based on the API. It
>>>>> should avoid any confusion with say using a different second cell value
>>>>> for the same lock in two different nodes.
>>>>>
>>>> But this implies that there is an actual need to hold these locks
>>>> exclusively. Given that they are (except for the raw case) all wrapped
>>>> by Linux locking primitives there shouldn't be a problem sharing a lock
>>>> (except possibly for the raw case).
>>> Yes agreed, the HWLOCK_RAW and HWLOCK_IN_ATOMIC cases are unprotected. I
>>> am still trying to understand better the usecase to see if the same lock
>>> is being multiplexed for different protection contexts, or if all of
>>> them are protecting the same context.
>>
>> Here are two different examples that explain the need for changes.
>> In both cases the Linux clients are talking to a single entity on the
>> remote-side.
>>
>> Example 1:
>> exti: interrupt-controller@5000d000 {
>> compatible = "st,stm32mp1-exti", "syscon";
>> interrupt-controller;
>> #interrupt-cells = <2>;
>> reg = <0x5000d000 0x400>;
>> hwlocks = <&hsem 1>;
>> };
>> The two drivers (stm32mp1-exti and syscon) refer to the same hwlock.
>> With the current hwspinlock implementation, only the first driver succeeds
>> in requesting (hwspin_lock_request_specific) the hwlock. The second request
>> fails.
>> Here, we really need to share the hwlock between the two drivers.
>> Note: hardware spinlock support for regmap was 'recently' introduced in 4.15
>> see https://lore.kernel.org/patchwork/patch/845941/
>>
>>
>>
>> Example 2:
>> Here it is more a question of optimization : we want to save the number of
>> hwlocks used to protect resources, using an unique hwlock to protect all
>> pinctrl resources:
>> pinctrl: pin-controller@50002000 {
>> compatible = "st,stm32mp157-pinctrl";
>> ranges = <0 0x50002000 0xa400>;
>> hwlocks = <&hsem 0 1>;
>>
>> pinctrl_z: pin-controller-z@54004000 {
>> compatible = "st,stm32mp157-z-pinctrl";
>> ranges = <0 0x54004000 0x400>;
>> pins-are-numbered;
>> hwlocks = <&hsem 0 1>;
> Thanks for the examples.
>
>>>> I agree that we shouldn't specify this property in DT - if anything it
>>>> should be a variant of the API.
>>
>> If we decide to add a 'shared' API, then, what about the generic regmap
>> driver?
>>
>> In the context of above example1, this would require to update the
>> regmap driver.
>>
>> But would this be acceptable for any driver using syscon/regmap?
>>
>>
>> I think it is better to keep the existing API (modifying it so it always
>> allows
>>
>> hwlocks sharing, so no need for bindings update) than adding another API.
> For your usecases, you would definitely need the syscon/regmap behavior
> to be shared right. Whether we introduce a 'shared' API or an
> 'exclusive' API and change the current API behavior to shared, it is
> definitely a case-by-case usage scenario for the existing drivers and
> usage right. The main contention point is what to do with the
> unprotected usecases like Bjorn originally pointed out.
OK, I see : the hwspinlock framework does not offer any lock protection
with the RAW/IN_ATOMIC modes.
This is an issue if several different 'local' drivers try to get a
shared lock in the same time.
And this is a personal problem since I need to use shared locks in
...atomic mode.
I have tried to see how it is possible to put a constraint on the
callers, just like this is documented for the RAW mode which is:
"Caution: If the mode is HWLOCK_RAW, that means user must protect
the routine
of getting hardware lock with mutex or spinlock.."
I do not think that it is acceptable to ask several drivers to share a
common mutex/spinlock for shared locks.
But I think about another option: the driver implementing the trylock
ops may offer such protection. This is the case if the driver returns
"busy" if the lock is already taken, not only by the remote processor,
but also by the local host.
So what do you think about adding such a documentation note :
"Caution : the HWLOCK_RAW / HWLOCK_IN_ATOMIC modes shall not be used
with shared locks unless the hwspinlock driver supports local lock
protection"
Optionally, we may add a "local_lock_protection" flag in the
hwspinlock_device struct, set by the driver before it calls
hwspin_lock_register().
This flag can then be checked by hwspinlock core to allow/deny use of
shared locks in the raw/atomic modes.
Let me know what you think about it.
BR
Fabien
>
> regards
> Suman
>
>>
>>
>>>>> If you are sharing a hwlock on the Linux side, surely your driver should
>>>>> be aware that it is a shared lock. The tag can be set during the first
>>>>> request API, and you look through both tags when giving out a handle.
>>>>>
>>>> Why would the driver need to know about it?
>>> Just the semantics if we were to support single user vs multiple users
>>> on Linux-side to even get a handle. Your point is that this may be moot
>>> since we have protection anyway other than the raw cases. But we need to
>>> be able to have the same API work across all cases.
>>>
>>> So far, it had mostly been that there would be one user on Linux
>>> competing with other equivalent peer entities on different processors.
>>> It is not common to have multiple users since these protection schemes
>>> are usually needed only at the lowest levels of a stack, so the
>>> exclusive handle stuff had been sufficient.
>>>
>>>>> Obviously, the hwspin_lock_request() API usage semantics always had the
>>>>> implied additional need for communicating the lock id to the other peer
>>>>> entity, so a realistic usage is most always the specific API variant. I
>>>>> doubt this API would be of much use for the shared driver usage. This
>>>>> also implies that the client user does not care about specifying a lock
>>>>> in DT.
>>>>>
>>>> Afaict if the lock are shared then there shouldn't be a problem with
>>>> some clients using the request API and others request_specific(). As any
>>>> collisions would simply mean that there are more contention on the lock.
>>>>
>>>> With the current exclusive model that is not possible and the success of
>>>> the request_specific will depend on probe order.
>>>>
>>>> But perhaps it should be explicitly prohibited to use both APIs on the
>>>> same hwspinlock instance?
>>> Yeah, they are meant to be complimentary usage, though I doubt we will
>>> ever have any realistic users for the generic API if we haven't had a
>>> usage so far. I had posted a concept of reserved locks long back [1] to
>>> keep away certain locks from the generic requestor, but dropped it since
>>> we did not have an actual use-case needing it.
>>>
>>> regards
>>> Suman
>>>
>>> [1] https://lwn.net/Articles/611944/
_______________________________________________
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/2] ARM: dts: at91: sama5d27_som1_ek: add mmc capabilities for SDMMC0
From: Adrian Hunter @ 2019-08-08 12:57 UTC (permalink / raw)
To: Eugen Hristev - M18282, Nicolas Ferre - M43238,
alexandre.belloni@bootlin.com, ulf.hansson@linaro.org,
linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-mmc@vger.kernel.org
In-Reply-To: <20190808124217.wrmcxohw5i6ju2qe@M43218.corp.atmel.com>
On 8/08/19 3:42 PM, Ludovic Desroches wrote:
> On Thu, Aug 08, 2019 at 10:35:43AM +0200, Eugen Hristev - M18282 wrote:
>> From: Eugen Hristev <eugen.hristev@microchip.com>
>>
>> Add mmc capabilities for SDMMC0 for this board.
>> With this enabled, eMMC connected card is detected as:
>>
>> mmc0: new DDR MMC card at address 0001
>>
>> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
> Acked-by: Ludovic Desroches <ludovic.desroches@microchip.com>
>
> I am interested to have the some insights about the use of sd-uhs-*
> properties.
>
> Our IP can't deal with 1V8 by itself. It has a 1V8SEL signal which can
> be used as the logic control input of a mux. So even if the IP claims
> to support UHS modes, it depends on the board.
>
> Are the sd-uhs-* properties a way to deal with this? I tend to think no
> as sdhci_setup_host() will set the caps depending on the content of the
> capabilities register. Do we have to use the SDHCI_QUIRK_MISSING_CAPS
> quirk or sdhci-caps/sdhci-caps-mask?
There is "no-1-8-v" which it looks like sdhci-of-at91.c already supports:
sdhci_at91_probe() -> sdhci_get_of_property() -> sdhci_get_property()
if (device_property_present(dev, "no-1-8-v"))
host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;
>
> Regards
>
> Ludovic
>
>> ---
>> arch/arm/boot/dts/at91-sama5d27_som1_ek.dts | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm/boot/dts/at91-sama5d27_som1_ek.dts b/arch/arm/boot/dts/at91-sama5d27_som1_ek.dts
>> index 149e539..194b3a3 100644
>> --- a/arch/arm/boot/dts/at91-sama5d27_som1_ek.dts
>> +++ b/arch/arm/boot/dts/at91-sama5d27_som1_ek.dts
>> @@ -54,6 +54,7 @@
>>
>> sdmmc0: sdio-host@a0000000 {
>> bus-width = <8>;
>> + mmc-ddr-3_3v;
>> pinctrl-names = "default";
>> pinctrl-0 = <&pinctrl_sdmmc0_default>;
>> status = "okay";
>> --
>> 2.7.4
>>
>
^ permalink raw reply
* Re: [PATCH 1/2] mmc: sdhci-of-at91: add quirk for broken HS200
From: Adrian Hunter @ 2019-08-08 13:00 UTC (permalink / raw)
To: Eugen.Hristev, Nicolas.Ferre, Ludovic.Desroches,
alexandre.belloni, ulf.hansson, linux-arm-kernel, devicetree,
linux-kernel, linux-mmc
In-Reply-To: <1565252928-28994-1-git-send-email-eugen.hristev@microchip.com>
On 8/08/19 11:35 AM, Eugen.Hristev@microchip.com wrote:
> From: Eugen Hristev <eugen.hristev@microchip.com>
>
> HS200 is not implemented in the driver, but the controller claims it
> through caps.
> Remove it via quirk.
> Without this quirk, the mmc core will try to enable hs200, which will fail,
> and the eMMC initialization will fail.
>
> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> drivers/mmc/host/sdhci-of-at91.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-of-at91.c b/drivers/mmc/host/sdhci-of-at91.c
> index 57fe3b2..3a8c6d8 100644
> --- a/drivers/mmc/host/sdhci-of-at91.c
> +++ b/drivers/mmc/host/sdhci-of-at91.c
> @@ -370,6 +370,9 @@ static int sdhci_at91_probe(struct platform_device *pdev)
> pm_runtime_set_autosuspend_delay(&pdev->dev, 50);
> pm_runtime_use_autosuspend(&pdev->dev);
>
> + /* HS200 is broken at this moment */
> + host->quirks2 = SDHCI_QUIRK2_BROKEN_HS200;
> +
> ret = sdhci_add_host(host);
> if (ret)
> goto pm_runtime_disable;
>
^ permalink raw reply
* Re: [PATCH 00/15] thermal: qcom: tsens: Add interrupt support
From: Amit Kucheria @ 2019-08-08 13:04 UTC (permalink / raw)
To: LKML, linux-arm-msm, Bjorn Andersson, Eduardo Valentin,
Andy Gross, Daniel Lezcano, Mark Rutland, Rob Herring, Zhang Rui,
sivaa
Cc: Marc Gonzalez, Brian Masney,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux PM list
In-Reply-To: <cover.1564091601.git.amit.kucheria@linaro.org>
On Fri, Jul 26, 2019 at 3:48 AM Amit Kucheria <amit.kucheria@linaro.org> wrote:
>
> Add interrupt support to TSENS. The first 6 patches are general fixes and
> cleanups to the driver before interrupt support is introduced.
>
> This series has been developed against qcs404 and sdm845 and then tested on
> msm8916. Testing on msm8998 and msm8974 would be appreciated since I don't
> have hardware handy. Further, I plan to test on msm8996 and also submit to
> kernelci.
Gentle nudge for reviews. This has now been successfully tested on
8974 (along with 8916, qcs404, sdm845). Testing on msm8998 would be
much appreciated.
> I'm sending this out for more review to get help with testing.
>
> Amit Kucheria (15):
> drivers: thermal: tsens: Get rid of id field in tsens_sensor
> drivers: thermal: tsens: Simplify code flow in tsens_probe
> drivers: thermal: tsens: Add __func__ identifier to debug statements
> drivers: thermal: tsens: Add debugfs support
> arm: dts: msm8974: thermal: Add thermal zones for each sensor
> arm64: dts: msm8916: thermal: Fixup HW ids for cpu sensors
> dt: thermal: tsens: Document interrupt support in tsens driver
> arm64: dts: sdm845: thermal: Add interrupt support
> arm64: dts: msm8996: thermal: Add interrupt support
> arm64: dts: msm8998: thermal: Add interrupt support
> arm64: dts: qcs404: thermal: Add interrupt support
> arm64: dts: msm8974: thermal: Add interrupt support
> arm64: dts: msm8916: thermal: Add interrupt support
> drivers: thermal: tsens: Create function to return sign-extended
> temperature
> drivers: thermal: tsens: Add interrupt support
>
> .../bindings/thermal/qcom-tsens.txt | 5 +
> arch/arm/boot/dts/qcom-msm8974.dtsi | 108 +++-
> arch/arm64/boot/dts/qcom/msm8916.dtsi | 26 +-
> arch/arm64/boot/dts/qcom/msm8996.dtsi | 60 +-
> arch/arm64/boot/dts/qcom/msm8998.dtsi | 82 +--
> arch/arm64/boot/dts/qcom/qcs404.dtsi | 42 +-
> arch/arm64/boot/dts/qcom/sdm845.dtsi | 88 +--
> drivers/thermal/qcom/tsens-8960.c | 4 +-
> drivers/thermal/qcom/tsens-common.c | 610 +++++++++++++++++-
> drivers/thermal/qcom/tsens-v0_1.c | 11 +
> drivers/thermal/qcom/tsens-v1.c | 29 +
> drivers/thermal/qcom/tsens-v2.c | 18 +
> drivers/thermal/qcom/tsens.c | 52 +-
> drivers/thermal/qcom/tsens.h | 285 +++++++-
> 14 files changed, 1214 insertions(+), 206 deletions(-)
>
> --
> 2.17.1
>
^ permalink raw reply
* [PATCH v2 0/2] spi: add NPCM FIU controller driver
From: Tomer Maimon @ 2019-08-08 13:14 UTC (permalink / raw)
To: broonie, robh+dt, mark.rutland, vigneshr, bbrezillon,
avifishman70, tali.perry1, venture, yuenn, benjaminfair
Cc: linux-spi, devicetree, openbmc, linux-kernel, Tomer Maimon
This patch set adds Flash Interface Unit(FIU) SPI
master support for the Nuvoton NPCM Baseboard
Management Controller (BMC).
The FIU supports single, dual or quad communication interface.
the FIU controller can operate in following modes:
- User Mode Access(UMA): provides flash access by using an
indirect address/data mechanism.
- direct rd/wr mode: maps the flash memory into the core
address space.
- SPI-X mode: used for an expansion bus to an ASIC or CPLD.
The NPCM750/730/715/710 supports up to three FIU devices:
- FIU0 supports two chip select.
- FIU3 supports four chip select.
- FIUX supports two chip select.
The NPCM FIU driver tested on NPCM750 evaluation board.
The FIU controller driver using direct map API SPI-MEM
interface and tested with the latest m25p80 driver patch
https://www.spinics.net/lists/linux-mtd/msg07358.html
According a conversion about direct SPI-MEM API
https://www.spinics.net/lists/linux-mtd/msg08225.html
The m25p80 driver will merge to the spi-nor driver we
need to make sure the m25p80 direct SPI-MEM will merge
as well.
The FIU controller driver tested with the latest spi-nor driver patch
https://www.spinics.net/lists/linux-mtd/msg08472.html
Changes since version 1:
- Support spi-mem no data transferred option (SPI_MEM_NO_DATA)
Tomer Maimon (2):
dt-binding: spi: add NPCM FIU controller
spi: npcm-fiu: add NPCM FIU controller driver
.../bindings/spi/nuvoton,npcm-fiu.txt | 47 ++
drivers/spi/Kconfig | 10 +
drivers/spi/Makefile | 1 +
drivers/spi/spi-npcm-fiu.c | 761 ++++++++++++++++++
4 files changed, 819 insertions(+)
create mode 100644 Documentation/devicetree/bindings/spi/nuvoton,npcm-fiu.txt
create mode 100644 drivers/spi/spi-npcm-fiu.c
--
2.18.0
^ permalink raw reply
* [PATCH v2 1/2] dt-binding: spi: add NPCM FIU controller
From: Tomer Maimon @ 2019-08-08 13:14 UTC (permalink / raw)
To: broonie, robh+dt, mark.rutland, vigneshr, bbrezillon,
avifishman70, tali.perry1, venture, yuenn, benjaminfair
Cc: linux-spi, devicetree, openbmc, linux-kernel, Tomer Maimon
In-Reply-To: <20190808131448.349161-1-tmaimon77@gmail.com>
Added device tree binding documentation for Nuvoton BMC
NPCM Flash Interface Unit(FIU) SPI master controller
using SPI-MEM interface.
Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
---
.../bindings/spi/nuvoton,npcm-fiu.txt | 47 +++++++++++++++++++
1 file changed, 47 insertions(+)
create mode 100644 Documentation/devicetree/bindings/spi/nuvoton,npcm-fiu.txt
diff --git a/Documentation/devicetree/bindings/spi/nuvoton,npcm-fiu.txt b/Documentation/devicetree/bindings/spi/nuvoton,npcm-fiu.txt
new file mode 100644
index 000000000000..ab37aae91d19
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/nuvoton,npcm-fiu.txt
@@ -0,0 +1,47 @@
+* Nuvoton FLASH Interface Unit (FIU) SPI Controller
+
+NPCM FIU supports single, dual and quad communication interface.
+
+The NPCM7XX supports three FIU modules,
+FIU0 and FIUx supports two chip selects,
+FIU3 support four chip select.
+
+Required properties:
+ - compatible : "nuvoton,npcm750-fiu" for the NPCM7XX BMC
+ - #address-cells : should be 1.
+ - #size-cells : should be 0.
+ - reg : the first contains the register location and length,
+ the second contains the memory mapping address and length
+ - reg-names: Should contain the reg names "control" and "memory"
+ - clocks : phandle of FIU reference clock.
+
+Required properties in case the pins can be muxed:
+ - pinctrl-names : a pinctrl state named "default" must be defined.
+ - pinctrl-0 : phandle referencing pin configuration of the device.
+
+Optional property:
+ - spix-mode: enable spix-mode for an expansion bus to an ASIC or CPLD.
+
+Aliases:
+- All the FIU controller nodes should be represented in the aliases node using
+ the following format 'fiu{n}' where n is a unique number for the alias.
+ In the NPCM7XX BMC:
+ fiu0 represent fiu 0 controller
+ fiu1 represent fiu 3 controller
+ fiu2 represent fiu x controller
+
+Example:
+fiu3: fiu@c00000000 {
+ compatible = "nuvoton,npcm750-fiu";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0xfb000000 0x1000>, <0x80000000 0x10000000>;
+ reg-names = "control", "memory";
+ clocks = <&clk NPCM7XX_CLK_AHB>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&spi3_pins>;
+ spi-nor@0 {
+ ...
+ };
+};
+
--
2.18.0
^ permalink raw reply related
* [PATCH v2 2/2] spi: npcm-fiu: add NPCM FIU controller driver
From: Tomer Maimon @ 2019-08-08 13:14 UTC (permalink / raw)
To: broonie, robh+dt, mark.rutland, vigneshr, bbrezillon,
avifishman70, tali.perry1, venture, yuenn, benjaminfair
Cc: linux-spi, devicetree, openbmc, linux-kernel, Tomer Maimon
In-Reply-To: <20190808131448.349161-1-tmaimon77@gmail.com>
Add Nuvoton NPCM BMC Flash Interface Unit(FIU) SPI master
controller driver using SPI-MEM interface.
The FIU supports single, dual or quad communication interface.
the FIU controller can operate in following modes:
- User Mode Access(UMA): provides flash access by using an
indirect address/data mechanism.
- direct rd/wr mode: maps the flash memory into the core
address space.
- SPI-X mode: used for an expansion bus to an ASIC or CPLD.
Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
---
drivers/spi/Kconfig | 10 +
drivers/spi/Makefile | 1 +
drivers/spi/spi-npcm-fiu.c | 761 +++++++++++++++++++++++++++++++++++++
3 files changed, 772 insertions(+)
create mode 100644 drivers/spi/spi-npcm-fiu.c
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 3a1d8f1170de..6ee514fd0920 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -433,6 +433,16 @@ config SPI_MT7621
help
This selects a driver for the MediaTek MT7621 SPI Controller.
+config SPI_NPCM_FIU
+ tristate "Nuvoton NPCM FLASH Interface Unit"
+ depends on ARCH_NPCM || COMPILE_TEST
+ depends on OF && HAS_IOMEM
+ help
+ This enables support for the Flash Interface Unit SPI controller
+ in master mode.
+ This driver does not support generic SPI. The implementation only
+ supports spi-mem interface.
+
config SPI_NPCM_PSPI
tristate "Nuvoton NPCM PSPI Controller"
depends on ARCH_NPCM || COMPILE_TEST
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 63dcab552bcb..adbebee93a75 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -63,6 +63,7 @@ obj-$(CONFIG_SPI_MT65XX) += spi-mt65xx.o
obj-$(CONFIG_SPI_MT7621) += spi-mt7621.o
obj-$(CONFIG_SPI_MXIC) += spi-mxic.o
obj-$(CONFIG_SPI_MXS) += spi-mxs.o
+obj-$(CONFIG_SPI_NPCM_FIU) += spi-npcm-fiu.o
obj-$(CONFIG_SPI_NPCM_PSPI) += spi-npcm-pspi.o
obj-$(CONFIG_SPI_NUC900) += spi-nuc900.o
obj-$(CONFIG_SPI_NXP_FLEXSPI) += spi-nxp-fspi.o
diff --git a/drivers/spi/spi-npcm-fiu.c b/drivers/spi/spi-npcm-fiu.c
new file mode 100644
index 000000000000..2d8c281e8fa9
--- /dev/null
+++ b/drivers/spi/spi-npcm-fiu.c
@@ -0,0 +1,761 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2019 Nuvoton Technology corporation.
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/ioport.h>
+#include <linux/clk.h>
+#include <linux/platform_device.h>
+#include <linux/io.h>
+#include <linux/vmalloc.h>
+#include <linux/regmap.h>
+#include <linux/of_device.h>
+#include <linux/spi/spi-mem.h>
+#include <linux/mfd/syscon.h>
+
+/* NPCM7xx GCR module */
+#define NPCM7XX_INTCR3_OFFSET 0x9C
+#define NPCM7XX_INTCR3_FIU_FIX BIT(6)
+
+/* Flash Interface Unit (FIU) Registers */
+#define NPCM_FIU_DRD_CFG 0x00
+#define NPCM_FIU_DWR_CFG 0x04
+#define NPCM_FIU_UMA_CFG 0x08
+#define NPCM_FIU_UMA_CTS 0x0C
+#define NPCM_FIU_UMA_CMD 0x10
+#define NPCM_FIU_UMA_ADDR 0x14
+#define NPCM_FIU_PRT_CFG 0x18
+#define NPCM_FIU_UMA_DW0 0x20
+#define NPCM_FIU_UMA_DW1 0x24
+#define NPCM_FIU_UMA_DW2 0x28
+#define NPCM_FIU_UMA_DW3 0x2C
+#define NPCM_FIU_UMA_DR0 0x30
+#define NPCM_FIU_UMA_DR1 0x34
+#define NPCM_FIU_UMA_DR2 0x38
+#define NPCM_FIU_UMA_DR3 0x3C
+#define NPCM_FIU_MAX_REG_LIMIT 0x80
+
+/* FIU Direct Read Configuration Register */
+#define NPCM_FIU_DRD_CFG_LCK BIT(31)
+#define NPCM_FIU_DRD_CFG_R_BURST GENMASK(25, 24)
+#define NPCM_FIU_DRD_CFG_ADDSIZ GENMASK(17, 16)
+#define NPCM_FIU_DRD_CFG_DBW GENMASK(13, 12)
+#define NPCM_FIU_DRD_CFG_ACCTYPE GENMASK(9, 8)
+#define NPCM_FIU_DRD_CFG_RDCMD GENMASK(7, 0)
+#define NPCM_FIU_DRD_ADDSIZ_SHIFT 16
+#define NPCM_FIU_DRD_DBW_SHIFT 12
+#define NPCM_FIU_DRD_ACCTYPE_SHIFT 8
+
+/* FIU Direct Write Configuration Register */
+#define NPCM_FIU_DWR_CFG_LCK BIT(31)
+#define NPCM_FIU_DWR_CFG_W_BURST GENMASK(25, 24)
+#define NPCM_FIU_DWR_CFG_ADDSIZ GENMASK(17, 16)
+#define NPCM_FIU_DWR_CFG_ABPCK GENMASK(11, 10)
+#define NPCM_FIU_DWR_CFG_DBPCK GENMASK(9, 8)
+#define NPCM_FIU_DWR_CFG_WRCMD GENMASK(7, 0)
+#define NPCM_FIU_DWR_ADDSIZ_SHIFT 16
+#define NPCM_FIU_DWR_ABPCK_SHIFT 10
+#define NPCM_FIU_DWR_DBPCK_SHIFT 8
+
+/* FIU UMA Configuration Register */
+#define NPCM_FIU_UMA_CFG_LCK BIT(31)
+#define NPCM_FIU_UMA_CFG_CMMLCK BIT(30)
+#define NPCM_FIU_UMA_CFG_RDATSIZ GENMASK(28, 24)
+#define NPCM_FIU_UMA_CFG_DBSIZ GENMASK(23, 21)
+#define NPCM_FIU_UMA_CFG_WDATSIZ GENMASK(20, 16)
+#define NPCM_FIU_UMA_CFG_ADDSIZ GENMASK(13, 11)
+#define NPCM_FIU_UMA_CFG_CMDSIZ BIT(10)
+#define NPCM_FIU_UMA_CFG_RDBPCK GENMASK(9, 8)
+#define NPCM_FIU_UMA_CFG_DBPCK GENMASK(7, 6)
+#define NPCM_FIU_UMA_CFG_WDBPCK GENMASK(5, 4)
+#define NPCM_FIU_UMA_CFG_ADBPCK GENMASK(3, 2)
+#define NPCM_FIU_UMA_CFG_CMBPCK GENMASK(1, 0)
+#define NPCM_FIU_UMA_CFG_ADBPCK_SHIFT 2
+#define NPCM_FIU_UMA_CFG_WDBPCK_SHIFT 4
+#define NPCM_FIU_UMA_CFG_DBPCK_SHIFT 6
+#define NPCM_FIU_UMA_CFG_RDBPCK_SHIFT 8
+#define NPCM_FIU_UMA_CFG_ADDSIZ_SHIFT 11
+#define NPCM_FIU_UMA_CFG_WDATSIZ_SHIFT 16
+#define NPCM_FIU_UMA_CFG_DBSIZ_SHIFT 21
+#define NPCM_FIU_UMA_CFG_RDATSIZ_SHIFT 24
+
+/* FIU UMA Control and Status Register */
+#define NPCM_FIU_UMA_CTS_RDYIE BIT(25)
+#define NPCM_FIU_UMA_CTS_RDYST BIT(24)
+#define NPCM_FIU_UMA_CTS_SW_CS BIT(16)
+#define NPCM_FIU_UMA_CTS_DEV_NUM GENMASK(9, 8)
+#define NPCM_FIU_UMA_CTS_EXEC_DONE BIT(0)
+#define NPCM_FIU_UMA_CTS_DEV_NUM_SHIFT 8
+
+/* FIU UMA Command Register */
+#define NPCM_FIU_UMA_CMD_DUM3 GENMASK(31, 24)
+#define NPCM_FIU_UMA_CMD_DUM2 GENMASK(23, 16)
+#define NPCM_FIU_UMA_CMD_DUM1 GENMASK(15, 8)
+#define NPCM_FIU_UMA_CMD_CMD GENMASK(7, 0)
+
+/* FIU UMA Address Register */
+#define NPCM_FIU_UMA_ADDR_UMA_ADDR GENMASK(31, 0)
+#define NPCM_FIU_UMA_ADDR_AB3 GENMASK(31, 24)
+#define NPCM_FIU_UMA_ADDR_AB2 GENMASK(23, 16)
+#define NPCM_FIU_UMA_ADDR_AB1 GENMASK(15, 8)
+#define NPCM_FIU_UMA_ADDR_AB0 GENMASK(7, 0)
+
+/* FIU UMA Write Data Bytes 0-3 Register */
+#define NPCM_FIU_UMA_DW0_WB3 GENMASK(31, 24)
+#define NPCM_FIU_UMA_DW0_WB2 GENMASK(23, 16)
+#define NPCM_FIU_UMA_DW0_WB1 GENMASK(15, 8)
+#define NPCM_FIU_UMA_DW0_WB0 GENMASK(7, 0)
+
+/* FIU UMA Write Data Bytes 4-7 Register */
+#define NPCM_FIU_UMA_DW1_WB7 GENMASK(31, 24)
+#define NPCM_FIU_UMA_DW1_WB6 GENMASK(23, 16)
+#define NPCM_FIU_UMA_DW1_WB5 GENMASK(15, 8)
+#define NPCM_FIU_UMA_DW1_WB4 GENMASK(7, 0)
+
+/* FIU UMA Write Data Bytes 8-11 Register */
+#define NPCM_FIU_UMA_DW2_WB11 GENMASK(31, 24)
+#define NPCM_FIU_UMA_DW2_WB10 GENMASK(23, 16)
+#define NPCM_FIU_UMA_DW2_WB9 GENMASK(15, 8)
+#define NPCM_FIU_UMA_DW2_WB8 GENMASK(7, 0)
+
+/* FIU UMA Write Data Bytes 12-15 Register */
+#define NPCM_FIU_UMA_DW3_WB15 GENMASK(31, 24)
+#define NPCM_FIU_UMA_DW3_WB14 GENMASK(23, 16)
+#define NPCM_FIU_UMA_DW3_WB13 GENMASK(15, 8)
+#define NPCM_FIU_UMA_DW3_WB12 GENMASK(7, 0)
+
+/* FIU UMA Read Data Bytes 0-3 Register */
+#define NPCM_FIU_UMA_DR0_RB3 GENMASK(31, 24)
+#define NPCM_FIU_UMA_DR0_RB2 GENMASK(23, 16)
+#define NPCM_FIU_UMA_DR0_RB1 GENMASK(15, 8)
+#define NPCM_FIU_UMA_DR0_RB0 GENMASK(7, 0)
+
+/* FIU UMA Read Data Bytes 4-7 Register */
+#define NPCM_FIU_UMA_DR1_RB15 GENMASK(31, 24)
+#define NPCM_FIU_UMA_DR1_RB14 GENMASK(23, 16)
+#define NPCM_FIU_UMA_DR1_RB13 GENMASK(15, 8)
+#define NPCM_FIU_UMA_DR1_RB12 GENMASK(7, 0)
+
+/* FIU UMA Read Data Bytes 8-11 Register */
+#define NPCM_FIU_UMA_DR2_RB15 GENMASK(31, 24)
+#define NPCM_FIU_UMA_DR2_RB14 GENMASK(23, 16)
+#define NPCM_FIU_UMA_DR2_RB13 GENMASK(15, 8)
+#define NPCM_FIU_UMA_DR2_RB12 GENMASK(7, 0)
+
+/* FIU UMA Read Data Bytes 12-15 Register */
+#define NPCM_FIU_UMA_DR3_RB15 GENMASK(31, 24)
+#define NPCM_FIU_UMA_DR3_RB14 GENMASK(23, 16)
+#define NPCM_FIU_UMA_DR3_RB13 GENMASK(15, 8)
+#define NPCM_FIU_UMA_DR3_RB12 GENMASK(7, 0)
+
+/* FIU Read Mode */
+enum {
+ DRD_SINGLE_WIRE_MODE = 0,
+ DRD_DUAL_IO_MODE = 1,
+ DRD_QUAD_IO_MODE = 2,
+ DRD_SPI_X_MODE = 3,
+};
+
+enum {
+ DWR_ABPCK_BIT_PER_CLK = 0,
+ DWR_ABPCK_2_BIT_PER_CLK = 1,
+ DWR_ABPCK_4_BIT_PER_CLK = 2,
+};
+
+enum {
+ DWR_DBPCK_BIT_PER_CLK = 0,
+ DWR_DBPCK_2_BIT_PER_CLK = 1,
+ DWR_DBPCK_4_BIT_PER_CLK = 2,
+};
+
+#define NPCM_FIU_DRD_16_BYTE_BURST 0x3000000
+#define NPCM_FIU_DWR_16_BYTE_BURST 0x3000000
+
+#define MAP_SIZE_128MB 0x8000000
+#define MAP_SIZE_16MB 0x1000000
+#define MAP_SIZE_8MB 0x800000
+
+#define NUM_BITS_IN_BYTE 8
+#define FIU_DRD_MAX_DUMMY_NUMBER 3
+#define NPCM_MAX_CHIP_NUM 4
+#define CHUNK_SIZE 16
+#define UMA_MICRO_SEC_TIMEOUT 150
+
+enum {
+ FIU0 = 0,
+ FIU3,
+ FIUX,
+};
+
+struct npcm_fiu_info {
+ char *name;
+ u32 fiu_id;
+ u32 max_map_size;
+ u32 max_cs;
+};
+
+struct fiu_data {
+ const struct npcm_fiu_info *npcm_fiu_data_info;
+ int fiu_max;
+};
+
+static const struct npcm_fiu_info npxm7xx_fiu_info[] = {
+ {.name = "FIU0", .fiu_id = FIU0,
+ .max_map_size = MAP_SIZE_128MB, .max_cs = 2},
+ {.name = "FIU3", .fiu_id = FIU3,
+ .max_map_size = MAP_SIZE_128MB, .max_cs = 4},
+ {.name = "FIUX", .fiu_id = FIUX,
+ .max_map_size = MAP_SIZE_16MB, .max_cs = 2} };
+
+static const struct fiu_data npxm7xx_fiu_data = {
+ .npcm_fiu_data_info = npxm7xx_fiu_info,
+ .fiu_max = 3,
+};
+
+struct npcm_fiu_spi;
+
+struct npcm_fiu_chip {
+ void __iomem *flash_region_mapped_ptr;
+ struct npcm_fiu_spi *fiu;
+ unsigned long clkrate;
+ u32 chipselect;
+};
+
+struct npcm_fiu_spi {
+ struct npcm_fiu_chip chip[NPCM_MAX_CHIP_NUM];
+ const struct npcm_fiu_info *info;
+ struct spi_mem_op drd_op;
+ struct resource *res_mem;
+ struct regmap *regmap;
+ unsigned long clkrate;
+ struct device *dev;
+ struct clk *clk;
+ bool spix_mode;
+};
+
+static const struct regmap_config npcm_mtd_regmap_config = {
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = 4,
+ .max_register = NPCM_FIU_MAX_REG_LIMIT,
+};
+
+static void npcm_fiu_set_drd(struct npcm_fiu_spi *fiu,
+ const struct spi_mem_op *op)
+{
+ regmap_update_bits(fiu->regmap, NPCM_FIU_DRD_CFG,
+ NPCM_FIU_DRD_CFG_ACCTYPE,
+ ilog2(op->addr.buswidth) <<
+ NPCM_FIU_DRD_ACCTYPE_SHIFT);
+ fiu->drd_op.addr.buswidth = op->addr.buswidth;
+ regmap_update_bits(fiu->regmap, NPCM_FIU_DRD_CFG,
+ NPCM_FIU_DRD_CFG_DBW,
+ ((op->dummy.nbytes * ilog2(op->addr.buswidth))
+ / NUM_BITS_IN_BYTE) << NPCM_FIU_DRD_DBW_SHIFT);
+ fiu->drd_op.dummy.nbytes = op->dummy.nbytes;
+ regmap_update_bits(fiu->regmap, NPCM_FIU_DRD_CFG,
+ NPCM_FIU_DRD_CFG_RDCMD, op->cmd.opcode);
+ fiu->drd_op.cmd.opcode = op->cmd.opcode;
+ regmap_update_bits(fiu->regmap, NPCM_FIU_DRD_CFG,
+ NPCM_FIU_DRD_CFG_ADDSIZ,
+ (op->addr.nbytes - 3) << NPCM_FIU_DRD_ADDSIZ_SHIFT);
+ fiu->drd_op.addr.nbytes = op->addr.nbytes;
+}
+
+static ssize_t npcm_fiu_direct_read(struct spi_mem_dirmap_desc *desc,
+ u64 offs, size_t len, void *buf)
+{
+ struct npcm_fiu_spi *fiu =
+ spi_controller_get_devdata(desc->mem->spi->master);
+ struct npcm_fiu_chip *chip = &fiu->chip[desc->mem->spi->chip_select];
+ void __iomem *src = (void __iomem *)(chip->flash_region_mapped_ptr +
+ offs);
+ u8 *buf_rx = buf;
+ u32 i;
+
+ if (fiu->spix_mode) {
+ for (i = 0 ; i < len ; i++)
+ *(buf_rx + i) = ioread8(src + i);
+ } else {
+ if (desc->info.op_tmpl.addr.buswidth != fiu->drd_op.addr.buswidth ||
+ desc->info.op_tmpl.dummy.nbytes != fiu->drd_op.dummy.nbytes ||
+ desc->info.op_tmpl.cmd.opcode != fiu->drd_op.cmd.opcode ||
+ desc->info.op_tmpl.addr.nbytes != fiu->drd_op.addr.nbytes)
+ npcm_fiu_set_drd(fiu, &desc->info.op_tmpl);
+
+ memcpy_fromio(buf_rx, src, len);
+ }
+
+ return len;
+}
+
+static ssize_t npcm_fiu_direct_write(struct spi_mem_dirmap_desc *desc,
+ u64 offs, size_t len, const void *buf)
+{
+ struct npcm_fiu_spi *fiu =
+ spi_controller_get_devdata(desc->mem->spi->master);
+ struct npcm_fiu_chip *chip = &fiu->chip[desc->mem->spi->chip_select];
+ void __iomem *dst = (void __iomem *)(chip->flash_region_mapped_ptr +
+ offs);
+ const u8 *buf_tx = buf;
+ u32 i;
+
+ if (fiu->spix_mode)
+ for (i = 0 ; i < len ; i++)
+ iowrite8(*(buf_tx + i), dst + i);
+ else
+ memcpy_toio(dst, buf_tx, len);
+
+ return len;
+}
+
+static int npcm_fiu_uma_read(struct spi_mem *mem,
+ const struct spi_mem_op *op, u32 addr,
+ bool is_address_size, u8 *data, u32 data_size)
+{
+ struct npcm_fiu_spi *fiu =
+ spi_controller_get_devdata(mem->spi->master);
+ u32 uma_cfg = BIT(10);
+ u32 data_reg[4];
+ int ret;
+ u32 val;
+ u32 i;
+
+ regmap_update_bits(fiu->regmap, NPCM_FIU_UMA_CTS,
+ NPCM_FIU_UMA_CTS_DEV_NUM,
+ (mem->spi->chip_select <<
+ NPCM_FIU_UMA_CTS_DEV_NUM_SHIFT));
+ regmap_update_bits(fiu->regmap, NPCM_FIU_UMA_CMD,
+ NPCM_FIU_UMA_CMD_CMD, op->cmd.opcode);
+
+ if (is_address_size) {
+ uma_cfg |= ilog2(op->cmd.buswidth);
+ uma_cfg |= ilog2(op->addr.buswidth)
+ << NPCM_FIU_UMA_CFG_ADBPCK_SHIFT;
+ uma_cfg |= ilog2(op->dummy.buswidth)
+ << NPCM_FIU_UMA_CFG_DBPCK_SHIFT;
+ uma_cfg |= ilog2(op->data.buswidth)
+ << NPCM_FIU_UMA_CFG_RDBPCK_SHIFT;
+ uma_cfg |= op->dummy.nbytes << NPCM_FIU_UMA_CFG_DBSIZ_SHIFT;
+ uma_cfg |= op->addr.nbytes << NPCM_FIU_UMA_CFG_ADDSIZ_SHIFT;
+ regmap_write(fiu->regmap, NPCM_FIU_UMA_ADDR, addr);
+ } else {
+ regmap_write(fiu->regmap, NPCM_FIU_UMA_ADDR, 0x0);
+ }
+
+ uma_cfg |= data_size << NPCM_FIU_UMA_CFG_RDATSIZ_SHIFT;
+ regmap_write(fiu->regmap, NPCM_FIU_UMA_CFG, uma_cfg);
+ regmap_write_bits(fiu->regmap, NPCM_FIU_UMA_CTS,
+ NPCM_FIU_UMA_CTS_EXEC_DONE,
+ NPCM_FIU_UMA_CTS_EXEC_DONE);
+ ret = regmap_read_poll_timeout(fiu->regmap, NPCM_FIU_UMA_CTS, val,
+ (!(val & NPCM_FIU_UMA_CTS_EXEC_DONE)), 0,
+ UMA_MICRO_SEC_TIMEOUT);
+ if (ret)
+ return ret;
+
+ if (data_size) {
+ for (i = 0; i < DIV_ROUND_UP(data_size, 4); i++)
+ regmap_read(fiu->regmap, NPCM_FIU_UMA_DR0 + (i * 4),
+ &data_reg[i]);
+ memcpy(data, data_reg, data_size);
+ }
+
+ return 0;
+}
+
+static int npcm_fiu_uma_write(struct spi_mem *mem,
+ const struct spi_mem_op *op, u8 cmd,
+ bool is_address_size, u8 *data, u32 data_size)
+{
+ struct npcm_fiu_spi *fiu =
+ spi_controller_get_devdata(mem->spi->master);
+ u32 uma_cfg = BIT(10);
+ u32 data_reg[4] = {0};
+ u32 val;
+ u32 i;
+
+ regmap_update_bits(fiu->regmap, NPCM_FIU_UMA_CTS,
+ NPCM_FIU_UMA_CTS_DEV_NUM,
+ (mem->spi->chip_select <<
+ NPCM_FIU_UMA_CTS_DEV_NUM_SHIFT));
+
+ regmap_update_bits(fiu->regmap, NPCM_FIU_UMA_CMD,
+ NPCM_FIU_UMA_CMD_CMD, cmd);
+
+ if (data_size) {
+ memcpy(data_reg, data, data_size);
+ for (i = 0; i < DIV_ROUND_UP(data_size, 4); i++)
+ regmap_write(fiu->regmap, NPCM_FIU_UMA_DW0 + (i * 4),
+ data_reg[i]);
+ }
+
+ if (is_address_size) {
+ uma_cfg |= ilog2(op->cmd.buswidth);
+ uma_cfg |= ilog2(op->addr.buswidth) <<
+ NPCM_FIU_UMA_CFG_ADBPCK_SHIFT;
+ uma_cfg |= ilog2(op->data.buswidth) <<
+ NPCM_FIU_UMA_CFG_WDBPCK_SHIFT;
+ uma_cfg |= op->addr.nbytes << NPCM_FIU_UMA_CFG_ADDSIZ_SHIFT;
+ regmap_write(fiu->regmap, NPCM_FIU_UMA_ADDR, op->addr.val);
+ } else {
+ regmap_write(fiu->regmap, NPCM_FIU_UMA_ADDR, 0x0);
+ }
+
+ uma_cfg |= (data_size << NPCM_FIU_UMA_CFG_WDATSIZ_SHIFT);
+ regmap_write(fiu->regmap, NPCM_FIU_UMA_CFG, uma_cfg);
+
+ regmap_write_bits(fiu->regmap, NPCM_FIU_UMA_CTS,
+ NPCM_FIU_UMA_CTS_EXEC_DONE,
+ NPCM_FIU_UMA_CTS_EXEC_DONE);
+
+ return regmap_read_poll_timeout(fiu->regmap, NPCM_FIU_UMA_CTS, val,
+ (!(val & NPCM_FIU_UMA_CTS_EXEC_DONE)), 0,
+ UMA_MICRO_SEC_TIMEOUT);
+}
+
+static int npcm_fiu_manualwrite(struct spi_mem *mem,
+ const struct spi_mem_op *op)
+{
+ struct npcm_fiu_spi *fiu =
+ spi_controller_get_devdata(mem->spi->master);
+ u8 *data = (u8 *)op->data.buf.out;
+ u32 num_data_chunks;
+ u32 remain_data;
+ u32 idx = 0;
+ int ret;
+
+ num_data_chunks = op->data.nbytes / CHUNK_SIZE;
+ remain_data = op->data.nbytes % CHUNK_SIZE;
+
+ regmap_update_bits(fiu->regmap, NPCM_FIU_UMA_CTS,
+ NPCM_FIU_UMA_CTS_DEV_NUM,
+ (mem->spi->chip_select <<
+ NPCM_FIU_UMA_CTS_DEV_NUM_SHIFT));
+ regmap_update_bits(fiu->regmap, NPCM_FIU_UMA_CTS,
+ NPCM_FIU_UMA_CTS_SW_CS, 0);
+
+ ret = npcm_fiu_uma_write(mem, op, op->cmd.opcode, true, NULL, 0);
+ if (ret)
+ return ret;
+
+ /* Starting the data writing loop in multiples of 8 */
+ for (idx = 0; idx < num_data_chunks; ++idx) {
+ ret = npcm_fiu_uma_write(mem, op, data[0], false,
+ &data[1], CHUNK_SIZE - 1);
+ if (ret)
+ return ret;
+
+ data += CHUNK_SIZE;
+ }
+
+ /* Handling chunk remains */
+ if (remain_data > 0) {
+ ret = npcm_fiu_uma_write(mem, op, data[0], false,
+ &data[1], remain_data - 1);
+ if (ret)
+ return ret;
+ }
+
+ regmap_update_bits(fiu->regmap, NPCM_FIU_UMA_CTS,
+ NPCM_FIU_UMA_CTS_SW_CS, NPCM_FIU_UMA_CTS_SW_CS);
+
+ return 0;
+}
+
+static int npcm_fiu_read(struct spi_mem *mem, const struct spi_mem_op *op)
+{
+ u8 *data = op->data.buf.in;
+ int i, readlen, currlen;
+ size_t retlen = 0;
+ u8 *buf_ptr;
+ u32 addr;
+ int ret;
+
+ i = 0;
+ currlen = op->data.nbytes;
+
+ do {
+ addr = ((u32)op->addr.val + i);
+ if (currlen < 16)
+ readlen = currlen;
+ else
+ readlen = 16;
+
+ buf_ptr = data + i;
+ ret = npcm_fiu_uma_read(mem, op, addr, true, buf_ptr,
+ readlen);
+ if (ret)
+ return ret;
+
+ i += readlen;
+ currlen -= 16;
+ } while (currlen > 0);
+
+ retlen = i;
+
+ return 0;
+}
+
+static void npcm_fiux_set_direct_wr(struct npcm_fiu_spi *fiu)
+{
+ regmap_write(fiu->regmap, NPCM_FIU_DWR_CFG,
+ NPCM_FIU_DWR_16_BYTE_BURST);
+ regmap_update_bits(fiu->regmap, NPCM_FIU_DWR_CFG,
+ NPCM_FIU_DWR_CFG_ABPCK,
+ DWR_ABPCK_4_BIT_PER_CLK << NPCM_FIU_DWR_ABPCK_SHIFT);
+ regmap_update_bits(fiu->regmap, NPCM_FIU_DWR_CFG,
+ NPCM_FIU_DWR_CFG_DBPCK,
+ DWR_DBPCK_4_BIT_PER_CLK << NPCM_FIU_DWR_DBPCK_SHIFT);
+}
+
+static void npcm_fiux_set_direct_rd(struct npcm_fiu_spi *fiu)
+{
+ u32 rx_dummy = 0;
+
+ regmap_write(fiu->regmap, NPCM_FIU_DRD_CFG,
+ NPCM_FIU_DRD_16_BYTE_BURST);
+ regmap_update_bits(fiu->regmap, NPCM_FIU_DRD_CFG,
+ NPCM_FIU_DRD_CFG_ACCTYPE,
+ DRD_SPI_X_MODE << NPCM_FIU_DRD_ACCTYPE_SHIFT);
+ regmap_update_bits(fiu->regmap, NPCM_FIU_DRD_CFG,
+ NPCM_FIU_DRD_CFG_DBW,
+ rx_dummy << NPCM_FIU_DRD_DBW_SHIFT);
+}
+
+static int npcm_fiu_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
+{
+ struct npcm_fiu_spi *fiu =
+ spi_controller_get_devdata(mem->spi->master);
+ struct npcm_fiu_chip *chip = &fiu->chip[mem->spi->chip_select];
+ int ret = 0;
+ u8 *buf;
+
+ dev_dbg(fiu->dev, "cmd:%#x mode:%d.%d.%d.%d addr:%#llx len:%#x\n",
+ op->cmd.opcode, op->cmd.buswidth, op->addr.buswidth,
+ op->dummy.buswidth, op->data.buswidth, op->addr.val,
+ op->data.nbytes);
+
+ if (fiu->spix_mode)
+ return -ENOTSUPP;
+
+ if (fiu->clkrate != chip->clkrate) {
+ ret = clk_set_rate(fiu->clk, chip->clkrate);
+ if (ret < 0)
+ dev_warn(fiu->dev, "Failed setting %lu frequancy, stay at %lu frequancy\n", chip->clkrate, fiu->clkrate);
+ else
+ fiu->clkrate = chip->clkrate;
+ }
+
+ if (op->data.dir == SPI_MEM_DATA_IN) {
+ if (!op->addr.nbytes) {
+ buf = op->data.buf.in;
+ ret = npcm_fiu_uma_read(mem, op, op->addr.val, false,
+ buf, op->data.nbytes);
+ } else {
+ ret = npcm_fiu_read(mem, op);
+ }
+ } else {
+ if (!op->addr.nbytes || !op->data.nbytes) {
+ if (op->data.nbytes)
+ buf = (u8 *)op->data.buf.out;
+ else
+ buf = NULL;
+ ret = npcm_fiu_uma_write(mem, op, op->cmd.opcode, false,
+ buf, op->data.nbytes);
+ } else {
+ ret = npcm_fiu_manualwrite(mem, op);
+ }
+ }
+
+ return ret;
+}
+
+static int npcm_fiu_dirmap_create(struct spi_mem_dirmap_desc *desc)
+{
+ struct npcm_fiu_spi *fiu =
+ spi_controller_get_devdata(desc->mem->spi->master);
+ struct npcm_fiu_chip *chip = &fiu->chip[desc->mem->spi->chip_select];
+ struct regmap *gcr_regmap;
+
+ if (!fiu->res_mem) {
+ dev_warn(fiu->dev, "Reserved memory not defined, direct read disabled\n");
+ desc->nodirmap = true;
+ return 0;
+ }
+
+ if (!fiu->spix_mode &&
+ desc->info.op_tmpl.data.dir == SPI_MEM_DATA_OUT) {
+ desc->nodirmap = true;
+ return 0;
+ }
+
+ if (!chip->flash_region_mapped_ptr) {
+ chip->flash_region_mapped_ptr =
+ devm_ioremap_nocache(fiu->dev, (fiu->res_mem->start +
+ (fiu->info->max_map_size *
+ desc->mem->spi->chip_select)),
+ (u32)desc->info.length);
+ if (!chip->flash_region_mapped_ptr) {
+ dev_warn(fiu->dev, "Error mapping memory region, direct read disabled\n");
+ desc->nodirmap = true;
+ return 0;
+ }
+ }
+
+ if (of_device_is_compatible(fiu->dev->of_node, "nuvoton,npcm750-fiu")) {
+ gcr_regmap =
+ syscon_regmap_lookup_by_compatible("nuvoton,npcm750-gcr");
+ if (IS_ERR(gcr_regmap)) {
+ dev_warn(fiu->dev, "Didn't find nuvoton,npcm750-gcr, direct read disabled\n");
+ desc->nodirmap = true;
+ return 0;
+ }
+ regmap_update_bits(gcr_regmap, NPCM7XX_INTCR3_OFFSET,
+ NPCM7XX_INTCR3_FIU_FIX,
+ NPCM7XX_INTCR3_FIU_FIX);
+ }
+
+ if (desc->info.op_tmpl.data.dir == SPI_MEM_DATA_IN) {
+ if (!fiu->spix_mode)
+ npcm_fiu_set_drd(fiu, &desc->info.op_tmpl);
+ else
+ npcm_fiux_set_direct_rd(fiu);
+
+ } else {
+ npcm_fiux_set_direct_wr(fiu);
+ }
+
+ return 0;
+}
+
+static int npcm_fiu_setup(struct spi_device *spi)
+{
+ struct spi_controller *ctrl = spi->master;
+ struct npcm_fiu_spi *fiu = spi_controller_get_devdata(ctrl);
+ struct npcm_fiu_chip *chip;
+
+ chip = &fiu->chip[spi->chip_select];
+ chip->fiu = fiu;
+ chip->chipselect = spi->chip_select;
+ chip->clkrate = spi->max_speed_hz;
+
+ fiu->clkrate = clk_get_rate(fiu->clk);
+
+ return 0;
+}
+
+static const struct spi_controller_mem_ops npcm_fiu_mem_ops = {
+ .exec_op = npcm_fiu_exec_op,
+ .dirmap_create = npcm_fiu_dirmap_create,
+ .dirmap_read = npcm_fiu_direct_read,
+ .dirmap_write = npcm_fiu_direct_write,
+};
+
+static const struct of_device_id npcm_fiu_dt_ids[] = {
+ { .compatible = "nuvoton,npcm750-fiu", .data = &npxm7xx_fiu_data },
+ { /* sentinel */ }
+};
+
+static int npcm_fiu_probe(struct platform_device *pdev)
+{
+ const struct fiu_data *fiu_data_match;
+ const struct of_device_id *match;
+ struct device *dev = &pdev->dev;
+ struct spi_controller *ctrl;
+ struct npcm_fiu_spi *fiu;
+ void __iomem *regbase;
+ struct resource *res;
+ int ret;
+ int id;
+
+ ctrl = spi_alloc_master(dev, sizeof(*fiu));
+ if (!ctrl)
+ return -ENOMEM;
+
+ fiu = spi_controller_get_devdata(ctrl);
+
+ match = of_match_device(npcm_fiu_dt_ids, dev);
+ if (!match || !match->data) {
+ dev_err(dev, "No compatible OF match\n");
+ return -ENODEV;
+ }
+
+ fiu_data_match = match->data;
+ id = of_alias_get_id(dev->of_node, "fiu");
+ if (id < 0 || id >= fiu_data_match->fiu_max) {
+ dev_err(dev, "Invalid platform device id: %d\n", id);
+ return -EINVAL;
+ }
+
+ fiu->info = &fiu_data_match->npcm_fiu_data_info[id];
+
+ platform_set_drvdata(pdev, fiu);
+ fiu->dev = dev;
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "control");
+ regbase = devm_ioremap_resource(dev, res);
+ if (IS_ERR(regbase))
+ return PTR_ERR(regbase);
+
+ fiu->regmap = devm_regmap_init_mmio(dev, regbase,
+ &npcm_mtd_regmap_config);
+ if (IS_ERR(fiu->regmap)) {
+ dev_err(dev, "Failed to create regmap\n");
+ return PTR_ERR(fiu->regmap);
+ }
+
+ fiu->res_mem = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+ "memory");
+ fiu->clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(fiu->clk))
+ return PTR_ERR(fiu->clk);
+
+ fiu->spix_mode = of_property_read_bool(dev->of_node, "spix-mode");
+
+ platform_set_drvdata(pdev, fiu);
+ clk_prepare_enable(fiu->clk);
+
+ ctrl->mode_bits = SPI_RX_DUAL | SPI_RX_QUAD
+ | SPI_TX_DUAL | SPI_TX_QUAD;
+ ctrl->setup = npcm_fiu_setup;
+ ctrl->bus_num = -1;
+ ctrl->mem_ops = &npcm_fiu_mem_ops;
+ ctrl->num_chipselect = fiu->info->max_cs;
+ ctrl->dev.of_node = dev->of_node;
+
+ ret = devm_spi_register_master(dev, ctrl);
+ if (ret)
+ return ret;
+
+ dev_info(dev, "NPCM %s probe succeed\n", fiu->info->name);
+
+ return 0;
+}
+
+static int npcm_fiu_remove(struct platform_device *pdev)
+{
+ struct npcm_fiu_spi *fiu = platform_get_drvdata(pdev);
+
+ clk_disable_unprepare(fiu->clk);
+ return 0;
+}
+
+MODULE_DEVICE_TABLE(of, npcm_fiu_dt_ids);
+
+static struct platform_driver npcm_fiu_driver = {
+ .driver = {
+ .name = "NPCM-FIU",
+ .bus = &platform_bus_type,
+ .of_match_table = npcm_fiu_dt_ids,
+ },
+ .probe = npcm_fiu_probe,
+ .remove = npcm_fiu_remove,
+};
+module_platform_driver(npcm_fiu_driver);
+
+MODULE_DESCRIPTION("Nuvoton FLASH Interface Unit SPI Controller Driver");
+MODULE_AUTHOR("Tomer Maimon <tomer.maimon@nuvoton.com>");
+MODULE_LICENSE("GPL v2");
--
2.18.0
^ permalink raw reply related
* Re: [PATCH v2 3/9] soc: samsung: Add Exynos Adaptive Supply Voltage driver
From: Krzysztof Kozlowski @ 2019-08-08 13:14 UTC (permalink / raw)
To: Robin Murphy
Cc: Sylwester Nawrocki, devicetree, linux-samsung-soc@vger.kernel.org,
linux-pm, pankaj.dubey, Bartłomiej Żołnierkiewicz,
linux-kernel@vger.kernel.org, robh+dt, kgene, vireshk,
linux-arm-kernel, Marek Szyprowski
In-Reply-To: <669c6b25-eb7e-ed3a-72a2-ee155a568363@arm.com>
On Thu, 8 Aug 2019 at 14:48, Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 08/08/2019 13:31, Krzysztof Kozlowski wrote:
> > On Thu, 8 Aug 2019 at 14:07, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
> >>>> +static unsigned int exynos5422_asv_parse_table(struct exynos_asv *asv,
> >>>> + unsigned int pkg_id)
> >>>> +{
> >>>> + return (pkg_id >> EXYNOS5422_TABLE_OFFSET) & EXYNOS5422_TABLE_MASK;
> >>>> +}
> >>>> +
> >>>> +static bool exynos5422_asv_parse_bin2(struct exynos_asv *asv,
> >>>> + unsigned int pkg_id)
> >>>> +{
> >>>> + return (pkg_id >> EXYNOS5422_BIN2_OFFSET) & EXYNOS5422_BIN2_MASK;
> >>>
> >>> return !!() for converting to boolean.
> >>
> >> I'm not convinced it is needed, the return type of the function is bool
> >> and value of the expression will be implicitly converted to that type.
> >> Is there any compiler warning related to that?
> >
> > Yeah, but bool is int so there will be no implicit conversion... I
> > guess it is a convention. In theory !! is the proper conversion to
> > bool but if bool==int then it's essentially conversion to 1. I am not
> > sure what's the benefit, maybe for some wrong code which would do
> > comparisons on result like if (exynos5422_asv_parse_bin2() == TRUE)...
>
> Not so - since we use "-std=gnu89", we have C99-like _Bool, which our
> bool is a typedef of. Conversions, either implicit or explicit, are
> well-defined:
>
> "6.3.1.2 Boolean type
>
> When any scalar value is converted to _Bool, the result is 0 if the
> value compares equal
> to 0; otherwise, the result is 1."
>
> This is even called out in Documentation/process/coding-style.rst:
>
> "When using bool types the !! construction is not needed, which
> eliminates a class of bugs."
Good point, thanks!
Best regards,
Krzysztof
^ permalink raw reply
* Re: [RFC PATCH 09/11] devfreq: exynos-bus: Add interconnect functionality to exynos-bus
From: Artur Świgoń @ 2019-08-08 13:18 UTC (permalink / raw)
To: Chanwoo Choi, devicetree, linux-arm-kernel, linux-samsung-soc,
linux-kernel, linux-pm, dri-devel
Cc: krzk, myungjoo.ham, inki.dae, sw0312.kim, georgi.djakov,
m.szyprowski
In-Reply-To: <5a82bf8a-d925-ba54-a26f-98b64bedc6e1@samsung.com>
Hi,
Thank you for your remarks. I will take them into account while preparing RFCv2.
On Mon, 2019-07-29 at 10:52 +0900, Chanwoo Choi wrote:
> Hi,
>
> On 19. 7. 23. 오후 9:20, Artur Świgoń wrote:
> > This patch adds interconnect functionality to the exynos-bus devfreq
> > driver.
> >
> > The SoC topology is a graph (or, more specifically, a tree) and most of its
> > edges are taken from the devfreq parent-child hierarchy (cf.
> > Documentation/devicetree/bindings/devfreq/exynos-bus.txt). The previous
> > patch adds missing edges to the DT (under the name 'parent'). Due to
> > unspecified relative probing order, -EPROBE_DEFER may be propagated to
> > guarantee that a child is probed before its parent.
> >
> > Each bus is now an interconnect provider and an interconnect node as well
> > (cf. Documentation/interconnect/interconnect.rst), i.e. every bus registers
> > itself as a node. Node IDs are not hardcoded but rather assigned at
> > runtime, in probing order (subject to the above-mentioned exception
> > regarding relative order). This approach allows for using this driver with
> > various Exynos SoCs.
> >
> > The devfreq target() callback provided by exynos-bus now selects either the
> > frequency calculated by the devfreq governor or the frequency requested via
> > the interconnect API for the given node, whichever is higher.
>
> Basically, I agree to support the QoS requirement between devices.
> But, I think that need to consider the multiple cases.
>
>
> 1. When changing the devfreq governor by user,
> For example of the connection between bus_dmc/leftbus/display on patch8,
> there are possible multiple cases with various devfreq governor
> which is changed on the runtime by user through sysfs interface.
>
> If users changes the devfreq governor as following:
> Before,
> - bus_dmc (simple_ondemand, available frequency 100/200/300/400 MHz)
> --> bus_leftbus(simple_ondemand, available frequency 100/200/300/400 MHz)
> ----> bus_display(passive)
>
> After changed governor of bus_dmc,
> if the min_freq by interconnect requirement is 400Mhz,
> - bus_dmc (powersave) : min_freq and max_freq and cur_freq is 100MHz
> --> bus_leftbus(simple_ondemand) : cur_freq is 400Mhz
> ----> bus_display(passive)
>
> The final frequency is 400MHz of bus_dmc
> even if the min_freq/max_freq/cur_freq is 100MHz.
> It cannot show the correct min_freq/max_freq through
> devfreq sysfs interface.
>
>
> 2. When disabling the some frequency by devfreq-thermal throttling,
> This patch checks the min_freq of interconnect requirement
> in the exynos_bus_target() and exynos_bus_passive_target().
> Also, it cannot show the correct min_freq/max_freq through
> devfreq sysfs interface.
>
> For example of bus_dmc bus,
> - The available frequencies are 100MHz, 200MHz, 300MHz, 400MHz
> - Disable 400MHz by devfreq-thermal throttling
> - min_freq is 100MHz
> - max_freq is 300MHz
> - min_freq of interconnect is 400MHz
>
> In result, the final frequency is 400MHz by exynos_bus_target()
> There are no problem for working. But, the user cannot know
> reason why cur_freq is 400MHz even if max_freq is 300MHz.
>
> Basically, update_devfreq() considers the all constraints
> of min_freq/max_freq to decide the proper target frequency.
>
>
> 3.
> I think that the exynos_bus_passive_target() is used for devfreq device
> using 'passive' governor. The frequency already depends on the parent device.
>
> If already the parent devfreq device like bus_leftbus consider
> the minimum frequency of QoS requirement like interconnect,
> it is not necessary. The next frequency of devfreq device
> with 'passive' governor, it will apply the QoS requirement
> without any additional code.
>
> >
> > Please note that it is not an error when CONFIG_INTERCONNECT is 'n', in
> > which case all interconnect API functions are no-op.
> >
> > Signed-off-by: Artur Świgoń <a.swigon@partner.samsung.com>
> > ---
> > drivers/devfreq/exynos-bus.c | 145 +++++++++++++++++++++++++++++++++++
> > 1 file changed, 145 insertions(+)
> >
> > diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
> > index 412511ca7703..12fb7c84ae50 100644
> > --- a/drivers/devfreq/exynos-bus.c
> > +++ b/drivers/devfreq/exynos-bus.c
> > @@ -14,6 +14,7 @@
> > #include <linux/devfreq-event.h>
> > #include <linux/device.h>
> > #include <linux/export.h>
> > +#include <linux/interconnect-provider.h>
> > #include <linux/module.h>
> > #include <linux/of.h>
> > #include <linux/pm_opp.h>
> > @@ -23,6 +24,8 @@
> > #define DEFAULT_SATURATION_RATIO 40
> > #define DEFAULT_VOLTAGE_TOLERANCE 2
> >
> > +#define icc_units_to_hz(x) ((x) * 1000UL / 8)
> > +
> > struct exynos_bus {
> > struct device *dev;
> >
> > @@ -31,12 +34,17 @@ struct exynos_bus {
> > unsigned int edev_count;
> > struct mutex lock;
> >
> > + unsigned long min_freq;
> > unsigned long curr_freq;
> >
> > struct regulator *regulator;
> > struct clk *clk;
> > unsigned int voltage_tolerance;
> > unsigned int ratio;
> > +
> > + /* One provider per bus, one node per provider */
> > + struct icc_provider provider;
> > + struct icc_node *node;
> > };
> >
> > /*
> > @@ -61,6 +69,13 @@ exynos_bus_ops_edev(enable_edev);
> > exynos_bus_ops_edev(disable_edev);
> > exynos_bus_ops_edev(set_event);
> >
> > +static int exynos_bus_next_id(void)
> > +{
> > + static int exynos_bus_node_id;
> > +
> > + return exynos_bus_node_id++;
> > +}
> > +
> > static int exynos_bus_get_event(struct exynos_bus *bus,
> > struct devfreq_event_data *edata)
> > {
> > @@ -98,6 +113,8 @@ static int exynos_bus_target(struct device *dev, unsigned
> > long *freq, u32 flags)
> > unsigned long old_freq, new_freq, new_volt, tol;
> > int ret = 0;
> >
> > + *freq = max(*freq, bus->min_freq);
> > +
> > /* Get new opp-bus instance according to new bus clock */
> > new_opp = devfreq_recommended_opp(dev, freq, flags);
> > if (IS_ERR(new_opp)) {
> > @@ -208,6 +225,8 @@ static int exynos_bus_passive_target(struct device *dev,
> > unsigned long *freq,
> > unsigned long old_freq, new_freq;
> > int ret = 0;
> >
> > + *freq = max(*freq, bus->min_freq);
> > +
> > /* Get new opp-bus instance according to new bus clock */
> > new_opp = devfreq_recommended_opp(dev, freq, flags);
> > if (IS_ERR(new_opp)) {
> > @@ -251,6 +270,35 @@ static void exynos_bus_passive_exit(struct device *dev)
> > clk_disable_unprepare(bus->clk);
> > }
> >
> > +static int exynos_bus_icc_set(struct icc_node *src, struct icc_node *dst)
> > +{
> > + struct exynos_bus *src_bus = src->data, *dst_bus = dst->data;
> > +
> > + src_bus->min_freq = icc_units_to_hz(src->peak_bw);
> > + dst_bus->min_freq = icc_units_to_hz(dst->peak_bw);
> > +
> > + return 0;
> > +}
> > +
> > +static int exynos_bus_icc_aggregate(struct icc_node *node, u32 avg_bw,
> > + u32 peak_bw, u32 *agg_avg, u32 *agg_peak)
> > +{
> > + *agg_peak = *agg_avg = peak_bw;
> > +
> > + return 0;
> > +}
> > +
> > +static struct icc_node *exynos_bus_icc_xlate(struct of_phandle_args *spec,
> > + void *data)
> > +{
> > + struct exynos_bus *bus = data;
> > +
> > + if (spec->np != bus->dev->of_node)
> > + return ERR_PTR(-EINVAL);
> > +
> > + return bus->node;
> > +}
> > +
> > static int exynos_bus_parent_parse_of(struct device_node *np,
> > struct exynos_bus *bus)
> > {
> > @@ -469,6 +517,95 @@ static int exynos_bus_profile_init_passive(struct
> > exynos_bus *bus,
> > return ret;
> > }
> >
> > +static int exynos_bus_icc_connect(struct exynos_bus *bus)
> > +{
> > + struct device_node *np = bus->dev->of_node;
> > + struct devfreq *parent_devfreq;
> > + struct icc_node *parent_node = NULL;
> > + struct of_phandle_args args;
> > + int ret = 0;
> > +
> > + parent_devfreq = devfreq_get_devfreq_by_phandle(bus->dev, 0);
> > + if (!IS_ERR(parent_devfreq)) {
> > + struct exynos_bus *parent_bus;
> > +
> > + parent_bus = dev_get_drvdata(parent_devfreq->dev.parent);
> > + parent_node = parent_bus->node;
> > + } else {
> > + /* Look for parent in DT */
> > + int num = of_count_phandle_with_args(np, "parent",
> > + "#interconnect-cells");
> > + if (num != 1)
> > + goto out;
> > +
> > + ret = of_parse_phandle_with_args(np, "parent",
> > + "#interconnect-cells",
> > + 0, &args);
> > + if (ret < 0)
> > + goto out;
> > +
> > + of_node_put(args.np);
> > +
> > + parent_node = of_icc_get_from_provider(&args);
> > + if (IS_ERR(parent_node)) {
> > + /* May be -EPROBE_DEFER */
> > + ret = PTR_ERR(parent_node);
> > + goto out;
> > + }
> > + }
> > +
> > + ret = icc_link_create(bus->node, parent_node->id);
> > +
> > +out:
> > + return ret;
> > +}
> > +
> > +static int exynos_bus_icc_init(struct exynos_bus *bus)
> > +{
> > + struct device *dev = bus->dev;
> > + struct icc_provider *provider = &bus->provider;
> > + struct icc_node *node;
> > + int id, ret;
> > +
> > + /* Initialize the interconnect provider */
> > + provider->set = exynos_bus_icc_set;
> > + provider->aggregate = exynos_bus_icc_aggregate;
> > + provider->xlate = exynos_bus_icc_xlate;
> > + provider->dev = dev;
> > + provider->data = bus;
> > +
> > + ret = icc_provider_add(provider);
> > + if (ret < 0)
> > + goto out;
> > +
> > + id = exynos_bus_next_id();
> > + node = icc_node_create(id);
> > + if (IS_ERR(node)) {
> > + ret = PTR_ERR(node);
> > + goto err_node;
> > + }
> > +
> > + bus->node = node;
> > + node->name = dev->of_node->name;
> > + node->data = bus;
> > + icc_node_add(node, provider);
> > +
> > + ret = exynos_bus_icc_connect(bus);
> > + if (ret < 0)
> > + goto err_connect;
> > +
> > +out:
> > + return ret;
> > +
> > +err_connect:
> > + icc_node_del(node);
> > + icc_node_destroy(id);
> > +err_node:
> > + icc_provider_del(provider);
> > +
> > + return ret;
> > +}
> > +
> > static int exynos_bus_probe(struct platform_device *pdev)
> > {
> > struct device *dev = &pdev->dev;
> > @@ -517,6 +654,14 @@ static int exynos_bus_probe(struct platform_device
> > *pdev)
> > goto err;
> > }
> >
> > + /*
> > + * Initialize interconnect provider. A return value of -ENOTSUPP means
> > + * that CONFIG_INTERCONNECT is disabled.
> > + */
> > + ret = exynos_bus_icc_init(bus);
> > + if (ret < 0 && ret != -ENOTSUPP)
> > + goto err;
> > +
> > max_state = bus->devfreq->profile->max_state;
> > min_freq = (bus->devfreq->profile->freq_table[0] / 1000);
> > max_freq = (bus->devfreq->profile->freq_table[max_state - 1] / 1000);
> >
>
>
--
Artur Świgoń
Samsung R&D Institute Poland
Samsung Electronics
^ permalink raw reply
* Re: [RFC PATCH 09/11] devfreq: exynos-bus: Add interconnect functionality to exynos-bus
From: Artur Świgoń @ 2019-08-08 13:19 UTC (permalink / raw)
To: Leonard Crestez, Georgi Djakov
Cc: devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-pm@vger.kernel.org, dri-devel@lists.freedesktop.org,
krzk@kernel.org, cw00.choi@samsung.com, myungjoo.ham@samsung.com,
inki.dae@samsung.com, sw0312.kim@samsung.com,
m.szyprowski@samsung.com
In-Reply-To: <VI1PR04MB5055BED59960B4F4147479AEEED50@VI1PR04MB5055.eurprd04.prod.outlook.com>
Hi,
Thank you for your comments.
On Tue, 2019-08-06 at 13:41 +0000, Leonard Crestez wrote:
> On 23.07.2019 15:21, Artur Świgoń wrote:
>
> > +static int exynos_bus_icc_aggregate(struct icc_node *node, u32 avg_bw,
> > + u32 peak_bw, u32 *agg_avg, u32 *agg_peak)
> > +{
> > + *agg_peak = *agg_avg = peak_bw;
> > +
> > + return 0;
> > +}
>
> The only current provider aggregates "avg" with "sum" and "peak" with
> "max", any particular reason to do something different? This function
> doesn't even really do aggregation, if there is a second request for "0"
> then the first request is lost.
Yes, you're right. I adopted an oversimplified solution for the purpose of this
RFC (please bear in mind that currently only one icc_path is used, so there is
no aggregation anyway).
> I didn't find any docs but my interpretation of avg/peak is that "avg"
> is for constant traffic like a display or VPU pushing pixels and "peak"
> is for variable traffic like networking. I assume devices which make
> "peak" requests are aggregated with max because they're not expected to
> all max-out together.
That's correct (according to my understanding).
> In PATCH 11 you're making a bandwidth request based on resolution, that
> traffic is constant and guaranteed to happend while the display is on so
> it would make sense to request it as an "avg" and aggregate it with "sum".
>
> --
> Regards,
> Leonard
>
--
Artur Świgoń
Samsung R&D Institute Poland
Samsung Electronics
^ permalink raw reply
* Re: [PATCH v2 2/2] spi: npcm-fiu: add NPCM FIU controller driver
From: Mark Brown @ 2019-08-08 13:27 UTC (permalink / raw)
To: Tomer Maimon
Cc: robh+dt, mark.rutland, vigneshr, bbrezillon, avifishman70,
tali.perry1, venture, yuenn, benjaminfair, linux-spi, devicetree,
openbmc, linux-kernel
In-Reply-To: <20190808131448.349161-3-tmaimon77@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 428 bytes --]
On Thu, Aug 08, 2019 at 04:14:48PM +0300, Tomer Maimon wrote:
> + ctrl->mode_bits = SPI_RX_DUAL | SPI_RX_QUAD
> + | SPI_TX_DUAL | SPI_TX_QUAD;
> + ctrl->setup = npcm_fiu_setup;
I'm not seeing where we implement dual or quad modes in the driver?
There's some
> + dev_info(dev, "NPCM %s probe succeed\n", fiu->info->name);
Just remove this, it makes the log more verbose but doesn't really add
any information.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [RFC PATCH 09/11] devfreq: exynos-bus: Add interconnect functionality to exynos-bus
From: Artur Świgoń @ 2019-08-08 13:28 UTC (permalink / raw)
To: Georgi Djakov, devicetree, linux-arm-kernel, linux-samsung-soc,
linux-kernel, linux-pm, dri-devel
Cc: krzk, cw00.choi, myungjoo.ham, inki.dae, sw0312.kim, m.szyprowski,
Bartłomiej Żołnierkiewicz
In-Reply-To: <4155482f-8f8f-a659-63ba-25701540b2c5@linaro.org>
Hi,
On Wed, 2019-08-07 at 17:21 +0300, Georgi Djakov wrote:
> Hi Artur,
>
> On 8/1/19 10:59, Artur Świgoń wrote:
> > Hi Georgi,
> >
> > On Fri, 2019-07-26 at 11:05 +0300, Georgi Djakov wrote:
> > > Hi Artur,
> > >
> > > On 7/23/19 15:20, Artur Świgoń wrote:
> > > > This patch adds interconnect functionality to the exynos-bus devfreq
> > > > driver.
> > > >
> > > > The SoC topology is a graph (or, more specifically, a tree) and most of
> > > > its
> > > > edges are taken from the devfreq parent-child hierarchy (cf.
> > > > Documentation/devicetree/bindings/devfreq/exynos-bus.txt). The previous
> > > > patch adds missing edges to the DT (under the name 'parent'). Due to
> > > > unspecified relative probing order, -EPROBE_DEFER may be propagated to
> > > > guarantee that a child is probed before its parent.
> > > >
> > > > Each bus is now an interconnect provider and an interconnect node as
> > > > well
> > > > (cf. Documentation/interconnect/interconnect.rst), i.e. every bus
> > > > registers
> > > > itself as a node. Node IDs are not hardcoded but rather assigned at
> > > > runtime, in probing order (subject to the above-mentioned exception
> > > > regarding relative order). This approach allows for using this driver
> > > > with
> > > > various Exynos SoCs.
> > >
> > > I am not familiar with the Exynos bus topology, but it seems to me that
> > > it's not
> > > represented correctly. An interconnect provider with just a single node
> > > (port)
> > > is odd. I would expect that each provider consists of multiple master and
> > > slave
> > > nodes. This data would be used by a framework to understand what are the
> > > links
> > > and how the traffic flows between the IP blocks and through which buses.
> >
> > To summarize the exynos-bus topology[1] used by the devfreq driver: There
> > are
> > many data buses for data transfer in Samsung Exynos SoC. Every bus has its
> > own
> > clock. Buses often share power lines, in which case one of the buses on the
> > power line is referred to as 'parent' (or as 'devfreq' in the DT). In the
> > particular case of Exynos4412[1][2], the topology can be expressed as
> > follows:
> >
> > bus_dmc
> > -- bus_acp
> > -- bus_c2c
> >
> > bus_leftbus
> > -- bus_rightbus
> > -- bus_display
> > -- bus_fsys
> > -- bus_peri
> > -- bus_mfc
> >
> > Where bus_dmc and bus_leftbus probably could be referred to as masters, and
> > the
> > following indented nodes as slaves. Patch 08/11 of this RFC additionally
> > adds
> > the following to the DT:
> >
> > bus_dmc
> > -- bus_leftbus
> >
> > Which makes the topology a valid tree.
> >
> > The exynos-bus concept in devfreq[3] is designed in such a way that every
> > bus is
> > probed separately as a platform device, and is a largely independent entity.
> >
> > This RFC proposes an extension to the existing devfreq driver that basically
> > provides a simple QoS to ensure minimum clock frequency for selected buses
> > (possibly overriding devfreq governor calculations) using the interconnect
> > framework.
> >
> > The hierarchy is modelled in such a way that every bus is an interconnect
> > node.
> > On the other hand, what is considered an interconnect provider here is quite
> > arbitrary, but for the reasons mentioned in the above paragraph, this RFC
> > assumes that every bus is a provider of itself as a node. Using an
> > alternative
>
> IIUC, in case we want to transfer data between the display and the memory
> controller, the path would look like this:
>
> display --> bus_display --> bus_leftbus --> bus_dmc --> memory
>
> But the bus_display for example would have not one, but two nodes (ports),
> right? One representing the link to the display controller and another one
> representing the link to bus_leftbus? So i think that all the buses should
> have at least two nodes, to represent each end of the wire.
I do not think we really need that for our simple tree hierarchy. Of course, I
can split every tree node into two nodes/ports (e.g., 'in' for children and
'out' for parent), but neither 'display' nor 'memory' from your diagram above
are registered with the interconnect framework (only buses are). The devfreq
devices used in the driver are virtual anyway.
> > singleton provider approach was deemed more complicated since the 'dev'
> > field in
> > 'struct icc_provider' has to be set to something meaningful and we are tied
> > to
> > the 'samsung,exynos-bus' compatible string in the driver (and multiple
> > instances
> > of exynos-bus probed in indeterminate relative order).
> >
>
> Sure, the rest makes sense to me.
>
> Thanks,
> Georgi
Regards,
--
Artur Świgoń
Samsung R&D Institute Poland
Samsung Electronics
^ permalink raw reply
* Re: [V1, 2/2] media: i2c: Add more sensor mode for ov8856 camera sensor
From: Sakari Ailus @ 2019-08-08 13:53 UTC (permalink / raw)
To: dongchun.zhu
Cc: mark.rutland, devicetree, drinkcat, srv_heupstream, shengnan.wang,
tfiga, louis.kuo, sj.huang, robh+dt, linux-mediatek, sakari.ailus,
matthias.bgg, bingbu.cao, mchehab, linux-arm-kernel, linux-media
In-Reply-To: <20190808092215.5608-3-dongchun.zhu@mediatek.com>
Hi Dongchun,
Thanks for the patch.
On Thu, Aug 08, 2019 at 05:22:15PM +0800, dongchun.zhu@mediatek.com wrote:
> From: Dongchun Zhu <dongchun.zhu@mediatek.com>
>
> This patch mainly adds two more sensor modes for OV8856 image sensor.
> The OV8856 driver currently supports output format: 10-bit Raw,
> the resolution of 1632*1224 and 3264*2448, and the bayer order of BGGR.
> The hardware version also differs in some OTP regiser,
> as well as PLL register setting.
>
> Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> ---
> drivers/media/i2c/ov8856.c | 624 ++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 621 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/i2c/ov8856.c b/drivers/media/i2c/ov8856.c
> index cd347d6..e0610b6 100644
> --- a/drivers/media/i2c/ov8856.c
> +++ b/drivers/media/i2c/ov8856.c
> @@ -1,12 +1,15 @@
> // SPDX-License-Identifier: GPL-2.0
> // Copyright (c) 2019 Intel Corporation.
>
> +#include <linux/clk.h>
> #include <asm/unaligned.h>
> #include <linux/acpi.h>
> #include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> #include <linux/i2c.h>
> #include <linux/module.h>
> #include <linux/pm_runtime.h>
> +#include <linux/regulator/consumer.h>
> #include <media/v4l2-ctrls.h>
> #include <media/v4l2-device.h>
> #include <media/v4l2-fwnode.h>
> @@ -19,6 +22,7 @@
> #define OV8856_LINK_FREQ_180MHZ 180000000ULL
> #define OV8856_SCLK 144000000ULL
> #define OV8856_MCLK 19200000
> +#define OV8856_XVCLK_FREQ 24000000
The driver currenctly uses, perhaps misleadingly, OV8856_MCLK for this
purpose. You could rename the existing MCLK as XVCLK.
This also means the driver needs to differentiate configurations for 24 and
19,2 MHz which it currently does not do. I think it may make sense to make
this a separate patch from the rest.
> #define OV8856_DATA_LANES 4
> #define OV8856_RGB_DEPTH 10
>
> @@ -29,6 +33,18 @@
> #define OV8856_MODE_STANDBY 0x00
> #define OV8856_MODE_STREAMING 0x01
>
> +/* define 1B module */
> +#define OV8856_1B_MODULE 0x02
> +
> +/* otp sram register */
> +#define OV8856_OTP_REG 0x700f
> +#define OV8856_OTP_REG_ONE 0x3d84
> +#define OV8856_OTP_REG_TWO 0x3d81
> +
> +/* clock register */
> +#define OV8856_CLK_REG 0x3614
> +#define OV8856_CLK_REG_1B_VAL 0x20
> +
> /* vertical-timings from sensor */
> #define OV8856_REG_VTS 0x380e
> #define OV8856_VTS_MAX 0x7fff
> @@ -64,6 +80,14 @@
>
> #define to_ov8856(_sd) container_of(_sd, struct ov8856, sd)
>
> +static const char * const ov8856_supply_names[] = {
> + "dovdd", /* Digital I/O power */
> + "avdd", /* Analog power */
> + "dvdd", /* Digital core power */
> +};
> +
> +#define OV8856_NUM_SUPPLIES ARRAY_SIZE(ov8856_supply_names)
> +
> enum {
> OV8856_LINK_FREQ_720MBPS,
> OV8856_LINK_FREQ_360MBPS,
> @@ -316,6 +340,208 @@ static const struct ov8856_reg mode_3280x2464_regs[] = {
> {0x5e00, 0x00}
> };
>
> +static const struct ov8856_reg mode_3264x2448_regs[] = {
> + {0x0103, 0x01},
> + {0x0302, 0x3c},
> + {0x0303, 0x01},
> + {0x031e, 0x0c},
> + {0x3000, 0x00},
> + {0x300e, 0x00},
> + {0x3010, 0x00},
> + {0x3015, 0x84},
> + {0x3018, 0x72},
> + {0x3021, 0x23},
> + {0x3033, 0x24},
> + {0x3500, 0x00},
> + {0x3501, 0x9a},
> + {0x3502, 0x20},
> + {0x3503, 0x08},
> + {0x3505, 0x83},
> + {0x3508, 0x01},
> + {0x3509, 0x80},
> + {0x350c, 0x00},
> + {0x350d, 0x80},
> + {0x350e, 0x04},
> + {0x350f, 0x00},
> + {0x3510, 0x00},
> + {0x3511, 0x02},
> + {0x3512, 0x00},
> + {0x3600, 0x72},
> + {0x3601, 0x40},
> + {0x3602, 0x30},
> + {0x3610, 0xc5},
> + {0x3611, 0x58},
> + {0x3612, 0x5c},
> + {0x3613, 0xca},
> + {0x3614, 0x60},
> + {0x3628, 0xff},
> + {0x3629, 0xff},
> + {0x362a, 0xff},
> + {0x3633, 0x10},
> + {0x3634, 0x10},
> + {0x3635, 0x10},
> + {0x3636, 0x10},
> + {0x3663, 0x08},
> + {0x3669, 0x34},
> + {0x366d, 0x00},
> + {0x366e, 0x10},
> + {0x3706, 0x86},
> + {0x370b, 0x7e},
> + {0x3714, 0x23},
> + {0x3730, 0x12},
> + {0x3733, 0x10},
> + {0x3764, 0x00},
> + {0x3765, 0x00},
> + {0x3769, 0x62},
> + {0x376a, 0x2a},
> + {0x376b, 0x30},
> + {0x3780, 0x00},
> + {0x3781, 0x24},
> + {0x3782, 0x00},
> + {0x3783, 0x23},
> + {0x3798, 0x2f},
> + {0x37a1, 0x60},
> + {0x37a8, 0x6a},
> + {0x37ab, 0x3f},
> + {0x37c2, 0x04},
> + {0x37c3, 0xf1},
> + {0x37c9, 0x80},
> + {0x37cb, 0x16},
> + {0x37cc, 0x16},
> + {0x37cd, 0x16},
> + {0x37ce, 0x16},
> + {0x3800, 0x00},
> + {0x3801, 0x00},
> + {0x3802, 0x00},
> + {0x3803, 0x0c},
> + {0x3804, 0x0c},
> + {0x3805, 0xdf},
> + {0x3806, 0x09},
> + {0x3807, 0xa3},
> + {0x3808, 0x0c},
> + {0x3809, 0xc0},
> + {0x380a, 0x09},
> + {0x380b, 0x90},
> + {0x380c, 0x07},
> + {0x380d, 0x8c},
> + {0x380e, 0x09},
> + {0x380f, 0xb2},
> + {0x3810, 0x00},
> + {0x3811, 0x04},
> + {0x3812, 0x00},
> + {0x3813, 0x02},
> + {0x3814, 0x01},
> + {0x3815, 0x01},
> + {0x3816, 0x00},
> + {0x3817, 0x00},
> + {0x3818, 0x00},
> + {0x3819, 0x00},
> + {0x3820, 0x80},
> + {0x3821, 0x46},
> + {0x382a, 0x01},
> + {0x382b, 0x01},
> + {0x3830, 0x06},
> + {0x3836, 0x02},
> + {0x3862, 0x04},
> + {0x3863, 0x08},
> + {0x3cc0, 0x33},
> + {0x3d85, 0x17},
> + {0x3d8c, 0x73},
> + {0x3d8d, 0xde},
> + {0x4001, 0xe0},
> + {0x4003, 0x40},
> + {0x4008, 0x00},
> + {0x4009, 0x0b},
> + {0x400a, 0x00},
> + {0x400b, 0x84},
> + {0x400f, 0x80},
> + {0x4010, 0xf0},
> + {0x4011, 0xff},
> + {0x4012, 0x02},
> + {0x4013, 0x01},
> + {0x4014, 0x01},
> + {0x4015, 0x01},
> + {0x4042, 0x00},
> + {0x4043, 0x80},
> + {0x4044, 0x00},
> + {0x4045, 0x80},
> + {0x4046, 0x00},
> + {0x4047, 0x80},
> + {0x4048, 0x00},
> + {0x4049, 0x80},
> + {0x4041, 0x03},
> + {0x404c, 0x20},
> + {0x404d, 0x00},
> + {0x404e, 0x20},
> + {0x4203, 0x80},
> + {0x4307, 0x30},
> + {0x4317, 0x00},
> + {0x4502, 0x50},
> + {0x4503, 0x08},
> + {0x4601, 0x80},
> + {0x4800, 0x44},
> + {0x4816, 0x53},
> + {0x481b, 0x50},
> + {0x481f, 0x27},
> + {0x4823, 0x3c},
> + {0x482b, 0x00},
> + {0x4831, 0x66},
> + {0x4837, 0x16},
> + {0x483c, 0x0f},
> + {0x484b, 0x05},
> + {0x5000, 0x77},
> + {0x5001, 0x0a},
> + {0x5003, 0xc8},
> + {0x5004, 0x04},
> + {0x5006, 0x00},
> + {0x5007, 0x00},
> + {0x502e, 0x03},
> + {0x5030, 0x41},
> + {0x5780, 0x14},
> + {0x5781, 0x0f},
> + {0x5782, 0x44},
> + {0x5783, 0x02},
> + {0x5784, 0x01},
> + {0x5785, 0x01},
> + {0x5786, 0x00},
> + {0x5787, 0x04},
> + {0x5788, 0x02},
> + {0x5789, 0x0f},
> + {0x578a, 0xfd},
> + {0x578b, 0xf5},
> + {0x578c, 0xf5},
> + {0x578d, 0x03},
> + {0x578e, 0x08},
> + {0x578f, 0x0c},
> + {0x5790, 0x08},
> + {0x5791, 0x04},
> + {0x5792, 0x00},
> + {0x5793, 0x52},
> + {0x5794, 0xa3},
> + {0x5795, 0x02},
> + {0x5796, 0x20},
> + {0x5797, 0x20},
> + {0x5798, 0xd5},
> + {0x5799, 0xd5},
> + {0x579a, 0x00},
> + {0x579b, 0x50},
> + {0x579c, 0x00},
> + {0x579d, 0x2c},
> + {0x579e, 0x0c},
> + {0x579f, 0x40},
> + {0x57a0, 0x09},
> + {0x57a1, 0x40},
> + {0x59f8, 0x3d},
> + {0x5a08, 0x02},
> + {0x5b00, 0x02},
> + {0x5b01, 0x10},
> + {0x5b02, 0x03},
> + {0x5b03, 0xcf},
> + {0x5b05, 0x6c},
> + {0x5e00, 0x00},
> + {0x5e10, 0xfc}
> +};
> +
> static const struct ov8856_reg mode_1640x1232_regs[] = {
> {0x3000, 0x20},
> {0x3003, 0x08},
> @@ -506,6 +732,208 @@ static const struct ov8856_reg mode_1640x1232_regs[] = {
> {0x5e00, 0x00}
> };
>
> +static const struct ov8856_reg mode_1632x1224_regs[] = {
> + {0x0103, 0x01},
> + {0x0302, 0x3c},
> + {0x0303, 0x01},
> + {0x031e, 0x0c},
> + {0x3000, 0x00},
> + {0x300e, 0x00},
> + {0x3010, 0x00},
> + {0x3015, 0x84},
> + {0x3018, 0x72},
> + {0x3021, 0x23},
> + {0x3033, 0x24},
> + {0x3500, 0x00},
> + {0x3501, 0x4c},
> + {0x3502, 0xe0},
> + {0x3503, 0x08},
> + {0x3505, 0x83},
> + {0x3508, 0x01},
> + {0x3509, 0x80},
> + {0x350c, 0x00},
> + {0x350d, 0x80},
> + {0x350e, 0x04},
> + {0x350f, 0x00},
> + {0x3510, 0x00},
> + {0x3511, 0x02},
> + {0x3512, 0x00},
> + {0x3600, 0x72},
> + {0x3601, 0x40},
> + {0x3602, 0x30},
> + {0x3610, 0xc5},
> + {0x3611, 0x58},
> + {0x3612, 0x5c},
> + {0x3613, 0xca},
> + {0x3614, 0x60},
> + {0x3628, 0xff},
> + {0x3629, 0xff},
> + {0x362a, 0xff},
> + {0x3633, 0x10},
> + {0x3634, 0x10},
> + {0x3635, 0x10},
> + {0x3636, 0x10},
> + {0x3663, 0x08},
> + {0x3669, 0x34},
> + {0x366d, 0x00},
> + {0x366e, 0x08},
> + {0x3706, 0x86},
> + {0x370b, 0x7e},
> + {0x3714, 0x27},
> + {0x3730, 0x12},
> + {0x3733, 0x10},
> + {0x3764, 0x00},
> + {0x3765, 0x00},
> + {0x3769, 0x62},
> + {0x376a, 0x2a},
> + {0x376b, 0x30},
> + {0x3780, 0x00},
> + {0x3781, 0x24},
> + {0x3782, 0x00},
> + {0x3783, 0x23},
> + {0x3798, 0x2f},
> + {0x37a1, 0x60},
> + {0x37a8, 0x6a},
> + {0x37ab, 0x3f},
> + {0x37c2, 0x14},
> + {0x37c3, 0xf1},
> + {0x37c9, 0x80},
> + {0x37cb, 0x16},
> + {0x37cc, 0x16},
> + {0x37cd, 0x16},
> + {0x37ce, 0x16},
> + {0x3800, 0x00},
> + {0x3801, 0x00},
> + {0x3802, 0x00},
> + {0x3803, 0x0c},
> + {0x3804, 0x0c},
> + {0x3805, 0xdf},
> + {0x3806, 0x09},
> + {0x3807, 0xa3},
> + {0x3808, 0x06},
> + {0x3809, 0x60},
> + {0x380a, 0x04},
> + {0x380b, 0xc8},
> + {0x380c, 0x07},
> + {0x380d, 0x8c},
> + {0x380e, 0x09},
> + {0x380f, 0xb2},
> + {0x3810, 0x00},
> + {0x3811, 0x02},
> + {0x3812, 0x00},
> + {0x3813, 0x02},
> + {0x3814, 0x03},
> + {0x3815, 0x01},
> + {0x3816, 0x00},
> + {0x3817, 0x00},
> + {0x3818, 0x00},
> + {0x3819, 0x00},
> + {0x3820, 0x80},
> + {0x3821, 0x47},
> + {0x382a, 0x03},
> + {0x382b, 0x01},
> + {0x3830, 0x06},
> + {0x3836, 0x02},
> + {0x3862, 0x04},
> + {0x3863, 0x08},
> + {0x3cc0, 0x33},
> + {0x3d85, 0x17},
> + {0x3d8c, 0x73},
> + {0x3d8d, 0xde},
> + {0x4001, 0xe0},
> + {0x4003, 0x40},
> + {0x4008, 0x00},
> + {0x4009, 0x05},
> + {0x400a, 0x00},
> + {0x400b, 0x84},
> + {0x400f, 0x80},
> + {0x4010, 0xf0},
> + {0x4011, 0xff},
> + {0x4012, 0x02},
> + {0x4013, 0x01},
> + {0x4014, 0x01},
> + {0x4015, 0x01},
> + {0x4042, 0x00},
> + {0x4043, 0x80},
> + {0x4044, 0x00},
> + {0x4045, 0x80},
> + {0x4046, 0x00},
> + {0x4047, 0x80},
> + {0x4048, 0x00},
> + {0x4049, 0x80},
> + {0x4041, 0x03},
> + {0x404c, 0x20},
> + {0x404d, 0x00},
> + {0x404e, 0x20},
> + {0x4203, 0x80},
> + {0x4307, 0x30},
> + {0x4317, 0x00},
> + {0x4502, 0x50},
> + {0x4503, 0x08},
> + {0x4601, 0x80},
> + {0x4800, 0x44},
> + {0x4816, 0x53},
> + {0x481b, 0x50},
> + {0x481f, 0x27},
> + {0x4823, 0x3c},
> + {0x482b, 0x00},
> + {0x4831, 0x66},
> + {0x4837, 0x16},
> + {0x483c, 0x0f},
> + {0x484b, 0x05},
> + {0x5000, 0x77},
> + {0x5001, 0x0a},
> + {0x5003, 0xc8},
> + {0x5004, 0x04},
> + {0x5006, 0x00},
> + {0x5007, 0x00},
> + {0x502e, 0x03},
> + {0x5030, 0x41},
> + {0x5795, 0x00},
> + {0x5796, 0x10},
> + {0x5797, 0x10},
> + {0x5798, 0x73},
> + {0x5799, 0x73},
> + {0x579a, 0x00},
> + {0x579b, 0x28},
> + {0x579c, 0x00},
> + {0x579d, 0x16},
> + {0x579e, 0x06},
> + {0x579f, 0x20},
> + {0x57a0, 0x04},
> + {0x57a1, 0xa0},
> + {0x5780, 0x14},
> + {0x5781, 0x0f},
> + {0x5782, 0x44},
> + {0x5783, 0x02},
> + {0x5784, 0x01},
> + {0x5785, 0x01},
> + {0x5786, 0x00},
> + {0x5787, 0x04},
> + {0x5788, 0x02},
> + {0x5789, 0x0f},
> + {0x578a, 0xfd},
> + {0x578b, 0xf5},
> + {0x578c, 0xf5},
> + {0x578d, 0x03},
> + {0x578e, 0x08},
> + {0x578f, 0x0c},
> + {0x5790, 0x08},
> + {0x5791, 0x04},
> + {0x5792, 0x00},
> + {0x5793, 0x52},
> + {0x5794, 0xa3},
> + {0x59f8, 0x3d},
> + {0x5a08, 0x02},
> + {0x5b00, 0x02},
> + {0x5b01, 0x10},
> + {0x5b02, 0x03},
> + {0x5b03, 0xcf},
> + {0x5b05, 0x6c},
> + {0x5e00, 0x00},
> + {0x5e10, 0xfc}
> +};
> +
> static const char * const ov8856_test_pattern_menu[] = {
> "Disabled",
> "Standard Color Bar",
> @@ -548,6 +976,18 @@ static const struct ov8856_mode supported_modes[] = {
> .link_freq_index = OV8856_LINK_FREQ_720MBPS,
> },
> {
> + .width = 3264,
> + .height = 2448,
> + .hts = 1932,
> + .vts_def = 2482,
> + .vts_min = 2482,
> + .reg_list = {
> + .num_of_regs = ARRAY_SIZE(mode_3264x2448_regs),
> + .regs = mode_3264x2448_regs,
> + },
> + .link_freq_index = OV8856_LINK_FREQ_720MBPS,
> + },
> + {
> .width = 1640,
> .height = 1232,
> .hts = 3820,
> @@ -558,6 +998,18 @@ static const struct ov8856_mode supported_modes[] = {
> .regs = mode_1640x1232_regs,
> },
> .link_freq_index = OV8856_LINK_FREQ_360MBPS,
> + },
> + {
> + .width = 1632,
> + .height = 1224,
> + .hts = 1932,
> + .vts_def = 2482,
> + .vts_min = 2482,
> + .reg_list = {
> + .num_of_regs = ARRAY_SIZE(mode_1632x1224_regs),
> + .regs = mode_1632x1224_regs,
> + },
> + .link_freq_index = OV8856_LINK_FREQ_360MBPS,
> }
> };
>
> @@ -566,6 +1018,10 @@ struct ov8856 {
> struct media_pad pad;
> struct v4l2_ctrl_handler ctrl_handler;
>
> + struct clk *xvclk;
> + struct gpio_desc *reset_gpio;
> + struct regulator_bulk_data supplies[OV8856_NUM_SUPPLIES];
> +
> /* V4L2 Controls */
> struct v4l2_ctrl *link_freq;
> struct v4l2_ctrl *pixel_rate;
> @@ -576,6 +1032,9 @@ struct ov8856 {
> /* Current mode */
> const struct ov8856_mode *cur_mode;
>
> + /* module hardware version */
> + bool is_1B_module;
What other hardware versions are there, and what are the differences?
> +
> /* To serialize asynchronus callbacks */
> struct mutex mutex;
>
> @@ -696,6 +1155,24 @@ static int ov8856_test_pattern(struct ov8856 *ov8856, u32 pattern)
> OV8856_REG_VALUE_08BIT, pattern);
> }
>
> +static int ov8856_update_otp_reg(struct ov8856 *ov8856)
> +{
> + int ret;
> +
> + ret = ov8856_write_reg(ov8856, OV8856_REG_MODE_SELECT,
> + OV8856_REG_VALUE_08BIT, OV8856_MODE_STREAMING);
> + if (ret)
> + return ret;
> +
> + ret = ov8856_write_reg(ov8856, OV8856_OTP_REG_ONE,
> + OV8856_REG_VALUE_08BIT, OV8856_MODE_STANDBY);
> + if (ret)
> + return ret;
> +
> + return ov8856_write_reg(ov8856, OV8856_OTP_REG_TWO,
> + OV8856_REG_VALUE_08BIT, OV8856_MODE_STREAMING);
> +}
What does this do?
> +
> static int ov8856_set_ctrl(struct v4l2_ctrl *ctrl)
> {
> struct ov8856 *ov8856 = container_of(ctrl->handler,
> @@ -825,7 +1302,13 @@ static void ov8856_update_pad_format(const struct ov8856_mode *mode,
> {
> fmt->width = mode->width;
> fmt->height = mode->height;
> - fmt->code = MEDIA_BUS_FMT_SGRBG10_1X10;
> +
> + /* Bayer Order is determined by image resolution */
Ouch.
This rather looks like a side effect of vertical cropping. How about
either cropping one line above or below, to keep the same Bayer order?
The driver is based on register lists that heavily restricts the
possibilities of configuring the sensor. The alternative, should more
free-form configuration be enabled, would be to expose the cropping
capability to the user --- as well as binning.
> + if (fmt->width == 3264 || fmt->width == 1632)
> + fmt->code = MEDIA_BUS_FMT_SBGGR10_1X10;
> + else
> + fmt->code = MEDIA_BUS_FMT_SGRBG10_1X10;
> +
> fmt->field = V4L2_FIELD_NONE;
> }
>
> @@ -850,6 +1333,17 @@ static int ov8856_start_streaming(struct ov8856 *ov8856)
> return ret;
> }
>
> + /* update R3614 for 1B module */
> + if (ov8856->is_1B_module) {
> + ret = ov8856_write_reg(ov8856, OV8856_CLK_REG,
> + OV8856_REG_VALUE_08BIT,
> + OV8856_CLK_REG_1B_VAL);
> + if (ret) {
> + dev_err(&client->dev, "failed to set R3614");
> + return ret;
> + }
> + }
> +
> ret = __v4l2_ctrl_handler_setup(ov8856->sd.ctrl_handler);
> if (ret)
> return ret;
> @@ -882,6 +1376,8 @@ static int ov8856_set_stream(struct v4l2_subdev *sd, int enable)
> if (ov8856->streaming == enable)
> return 0;
>
> + dev_dbg(&client->dev, "hardware version: (%d)\n", ov8856->is_1B_module);
> +
> mutex_lock(&ov8856->mutex);
> if (enable) {
> ret = pm_runtime_get_sync(&client->dev);
> @@ -908,6 +1404,54 @@ static int ov8856_set_stream(struct v4l2_subdev *sd, int enable)
> return ret;
> }
>
> +/* Calculate the delay in us by clock rate and clock cycles */
> +static inline u32 ov8856_cal_delay(u32 cycles)
> +{
> + return DIV_ROUND_UP(cycles, OV8856_XVCLK_FREQ / 1000 / 1000);
The frequency is rounded down. As it is used to calculate a delay needed,
rounding up should be done for the frequency, too.
> +}
> +
> +static int __ov8856_power_on(struct ov8856 *ov8856)
> +{
> + int ret;
> + u32 delay_us;
> + struct i2c_client *client = v4l2_get_subdevdata(&ov8856->sd);
> +
> + ret = clk_prepare_enable(ov8856->xvclk);
> + if (ret < 0) {
> + dev_err(&client->dev, "Failed to enable xvclk\n");
> + return ret;
> + }
> +
> + gpiod_set_value_cansleep(ov8856->reset_gpio, 1);
> +
> + ret = regulator_bulk_enable(OV8856_NUM_SUPPLIES, ov8856->supplies);
> + if (ret < 0) {
> + dev_err(&client->dev, "Failed to enable regulators\n");
> + goto disable_clk;
> + }
> +
> + gpiod_set_value_cansleep(ov8856->reset_gpio, 0);
> +
> + /* 8192 cycles prior to first SCCB transaction */
> + delay_us = ov8856_cal_delay(8192);
> + usleep_range(delay_us * 2, delay_us * 4);
Why multiply by 2?
Note that the driver still needs to work even if the resources aren't
visible to the software. That's the case e.g. on ACPI based systems.
> +
> + return 0;
> +
> +disable_clk:
> + clk_disable_unprepare(ov8856->xvclk);
> +
> + return ret;
> +}
> +
> +static void __ov8856_power_off(struct ov8856 *ov8856)
> +{
> + clk_disable_unprepare(ov8856->xvclk);
> + gpiod_set_value_cansleep(ov8856->reset_gpio, 1);
> +
> + regulator_bulk_disable(OV8856_NUM_SUPPLIES, ov8856->supplies);
> +}
> +
> static int __maybe_unused ov8856_suspend(struct device *dev)
> {
> struct i2c_client *client = to_i2c_client(dev);
> @@ -915,8 +1459,8 @@ static int __maybe_unused ov8856_suspend(struct device *dev)
> struct ov8856 *ov8856 = to_ov8856(sd);
>
> mutex_lock(&ov8856->mutex);
> - if (ov8856->streaming)
> - ov8856_stop_streaming(ov8856);
This seems like an unrelated change.
> +
> + __ov8856_power_off(ov8856);
>
> mutex_unlock(&ov8856->mutex);
>
> @@ -1089,6 +1633,20 @@ static int ov8856_identify_module(struct ov8856 *ov8856)
> return -ENXIO;
> }
>
> + /* set R3614 to distinguish harward versions */
> + ret = ov8856_update_otp_reg(ov8856);
> + if (ret) {
> + dev_err(&client->dev, "failed to set otp register");
> + return ret;
> + }
> +
> + ret = ov8856_read_reg(ov8856, OV8856_OTP_REG,
> + OV8856_REG_VALUE_08BIT, &val);
> + if (ret)
> + return ret;
> +
> + ov8856->is_1B_module = (val == OV8856_1B_MODULE) ? 1 : 0;
> +
> return 0;
> }
>
> @@ -1164,11 +1722,27 @@ static int ov8856_remove(struct i2c_client *client)
> media_entity_cleanup(&sd->entity);
> v4l2_ctrl_handler_free(sd->ctrl_handler);
> pm_runtime_disable(&client->dev);
> + if (!pm_runtime_status_suspended(&client->dev))
> + __ov8856_power_off(ov8856);
> + pm_runtime_set_suspended(&client->dev);
> mutex_destroy(&ov8856->mutex);
>
> return 0;
> }
>
> +static int ov8856_configure_regulators(struct ov8856 *ov8856)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&ov8856->sd);
> + int i;
unsigned int
> +
> + for (i = 0; i < OV8856_NUM_SUPPLIES; i++)
> + ov8856->supplies[i].supply = ov8856_supply_names[i];
> +
> + return devm_regulator_bulk_get(&client->dev,
> + OV8856_NUM_SUPPLIES,
> + ov8856->supplies);
Remember to put the regulators, too.
> +}
> +
> static int ov8856_probe(struct i2c_client *client)
> {
> struct ov8856 *ov8856;
> @@ -1186,6 +1760,40 @@ static int ov8856_probe(struct i2c_client *client)
> return -ENOMEM;
>
> v4l2_i2c_subdev_init(&ov8856->sd, client, &ov8856_subdev_ops);
> +
> + ov8856->xvclk = devm_clk_get(&client->dev, "xvclk");
> + if (IS_ERR(ov8856->xvclk)) {
> + dev_err(&client->dev, "Failed to get xvclk\n");
> + return -EINVAL;
> + }
> +
> + ret = clk_set_rate(ov8856->xvclk, OV8856_XVCLK_FREQ);
> + if (ret < 0) {
> + dev_err(&client->dev, "Failed to set xvclk rate (24MHz)\n");
> + return ret;
> + }
> + if (clk_get_rate(ov8856->xvclk) != OV8856_XVCLK_FREQ)
> + dev_warn(&client->dev,
> + "xvclk mismatched, modes are based on 24MHz\n");
> +
> + ov8856->reset_gpio = devm_gpiod_get(&client->dev,
> + "reset",
Fits on the previous line.
> + GPIOD_OUT_LOW);
> + if (IS_ERR(ov8856->reset_gpio)) {
> + dev_err(&client->dev, "Failed to get reset-gpios\n");
> + return -EINVAL;
> + }
> +
> + ret = ov8856_configure_regulators(ov8856);
> + if (ret) {
> + dev_err(&client->dev, "Failed to get power regulators\n");
> + return ret;
> + }
> +
> + ret = __ov8856_power_on(ov8856);
> + if (ret)
> + goto probe_error_v4l2_ctrl_handler_free;
> +
> ret = ov8856_identify_module(ov8856);
> if (ret) {
> dev_err(&client->dev, "failed to find sensor: %d", ret);
> @@ -1251,11 +1859,21 @@ static const struct acpi_device_id ov8856_acpi_ids[] = {
> MODULE_DEVICE_TABLE(acpi, ov8856_acpi_ids);
> #endif
>
> +#if IS_ENABLED(CONFIG_OF)
> +static const struct of_device_id ov8856_of_match[] = {
> + { .compatible = "ovti,ov8856" },
> + {},
> +};
> +
> +MODULE_DEVICE_TABLE(of, ov8856_of_match);
> +#endif
> +
> static struct i2c_driver ov8856_i2c_driver = {
> .driver = {
> .name = "ov8856",
> .pm = &ov8856_pm_ops,
> .acpi_match_table = ACPI_PTR(ov8856_acpi_ids),
> + .of_match_table = of_match_ptr(ov8856_of_match),
> },
> .probe_new = ov8856_probe,
> .remove = ov8856_remove,
--
Regards,
Sakari Ailus
^ permalink raw reply
* [PATCHv3] drivers/amba: add reset control to amba bus probe
From: Dinh Nguyen @ 2019-08-08 14:01 UTC (permalink / raw)
To: devicetree
Cc: dinguyen, linux-kernel, robh+dt, frowand.list, keescook, anton,
ccross, tony.luck
The primecell controller on some SoCs, i.e. SoCFPGA, is held in reset by
default. Until recently, the DMA controller was brought out of reset by the
bootloader(i.e. U-Boot). But a recent change in U-Boot, the peripherals that
are not used are held in reset and are left to Linux to bring them out of
reset.
Add a mechanism for getting the reset property and de-assert the primecell
module from reset if found. This is a not a hard fail if the reset property
is not present in the device tree node, so the driver will continue to probe.
Because there are different variants of the controller that may have multiple
reset signals, the code will find all reset(s) specified and de-assert them.
Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
---
v3: add a reset_control_put()
add error handling for -EPROBE_DEFER
v2: move reset control to bus code
find all reset properties and de-assert them
---
drivers/amba/bus.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
index 100e798a5c82..00e68ea416ca 100644
--- a/drivers/amba/bus.c
+++ b/drivers/amba/bus.c
@@ -18,6 +18,7 @@
#include <linux/limits.h>
#include <linux/clk/clk-conf.h>
#include <linux/platform_device.h>
+#include <linux/reset.h>
#include <asm/irq.h>
@@ -401,6 +402,28 @@ static int amba_device_try_add(struct amba_device *dev, struct resource *parent)
ret = amba_get_enable_pclk(dev);
if (ret == 0) {
u32 pid, cid;
+ int count;
+ struct reset_control *rstc;
+
+ /*
+ * Find reset control(s) of the amba bus and de-assert them.
+ */
+ count = reset_control_get_count(&dev->dev);
+ while (count > 0) {
+ rstc = of_reset_control_get_shared_by_index(dev->dev.of_node, count - 1);
+ if (IS_ERR(rstc)) {
+ if (PTR_ERR(rstc) == -EPROBE_DEFER) {
+ ret = -EPROBE_DEFER;
+ } else {
+ dev_err(&dev->dev, "Can't get amba reset!\n");
+ }
+ break;
+ } else {
+ reset_control_deassert(rstc);
+ reset_control_put(rstc);
+ count--;
+ }
+ }
/*
* Read pid and cid based on size of resource
--
2.20.0
^ permalink raw reply related
* [PATCH] of/platform: fix compilation warning of of_link_property()
From: Anders Roxell @ 2019-08-08 14:18 UTC (permalink / raw)
To: robh+dt, frowand.list; +Cc: devicetree, linux-kernel, Anders Roxell
GCC warns that a negative integer can be returned but the
of_link_property() function should return a boolean.
../drivers/of/platform.c: In function ‘of_link_property’:
../drivers/of/platform.c:650:18: warning: ?: using integer constants in boolean context [-Wint-in-bool-context]
return done ? 0 : -ENODEV;
Rework so function of_link_property() return an integer instead of a boolean.
Fixes: 690ff7881b26 ("of/platform: Add functional dependency link from DT bindings")
Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
---
drivers/of/platform.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 21838226d68a..86fb8ab8c012 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -625,7 +625,7 @@ static const struct supplier_bindings bindings[] = {
{ },
};
-static bool of_link_property(struct device *dev, struct device_node *con_np,
+static int of_link_property(struct device *dev, struct device_node *con_np,
const char *prop)
{
struct device_node *phandle;
--
2.20.1
^ permalink raw reply related
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