* [PATCH v6 0/2] regulator: add QCOM RPMh regulator driver
From: David Collins @ 2018-06-04 19:15 UTC (permalink / raw)
To: broonie, lgirdwood, robh+dt, mark.rutland
Cc: David Collins, linux-arm-msm, linux-arm-kernel, devicetree,
linux-kernel, rnayak, sboyd, dianders
This patch series adds a driver and device tree binding documentation for
PMIC regulator control via Resource Power Manager-hardened (RPMh) on some
Qualcomm Technologies, Inc. SoCs such as SDM845. RPMh is a hardware block
which contains several accelerators which are used to manage various
hardware resources that are shared between the processors of the SoC. The
final hardware state of a regulator is determined within RPMh by performing
max aggregation of the requests made by all of the processors.
The RPMh regulator driver depends upon the RPMh driver [1] and command DB
driver [2] which are both still undergoing review. It also depends upon
three recent regulator changes: [3], [4], and [5].
Changes since v5 [6]:
- Removed unused constants
- Added Reviewed-by tag for DT patch 1/2
Changes since v4 [7]:
- Removed support for DT properties qcom,regulator-drms-modes and
qcom,drms-mode-max-microamps
- Specified fixed DRMS high power mode minimum limits for LDO type
regulators
- Removed DRMS support for SMPS and BOB type regulators
- Simplified voltage caching logic
Changes since v3 [8]:
- Removed support for DT properties qcom,regulator-initial-microvolt
and qcom,headroom-microvolt
- Renamed DT property qcom,allowed-drms-modes to be
qcom,regulator-drms-modes
- Updated DT binding documentation to mention which common regulator
bindings can be used for qcom-rpmh-regulator devices
- Added voltage caching so that voltage requests are only sent to RPMh
after the regulator has been enabled at least once
- Changed 'voltage_selector' default value to be -ENOTRECOVERABLE to
interact with [5]
- Initialized 'enabled' to -EINVAL so that unused regulators are
disabled by regulator_late_cleanup()
- Removed rpmh_regulator_load_default_parameters() as it is no longer
needed
- Updated the mode usage description in qcom,rpmh-regulator.h
Changes since v2 [9]:
- Replaced '_' with '-' in device tree supply property names
- Renamed qcom_rpmh-regulator.c to be qcom-rpmh-regulator.c
- Updated various DT property names to use "microvolt" and "microamp"
- Moved allowed modes constraint specification out of the driver [4]
- Replaced rpmh_client with device pointer to match new RPMh API [1]
- Corrected drms mode threshold checking
- Initialized voltage_selector to -EINVAL when not specified in DT
- Added constants for PMIC regulator hardware modes
- Corrected type sign of mode mapping tables
- Made variable names for mode arrays plural
- Simplified Kconfig depends on
- Removed unnecessary constants and struct fields
- Added some descriptive comments
Changes since v1 [10]:
- Addressed review feedback from Doug, Mark, and Stephen
- Replaced set_voltage()/get_voltage() callbacks with set_voltage_sel()/
get_voltage_sel()
- Added set_bypass()/get_bypass() callbacks for BOB pass-through mode
control
- Removed top-level PMIC data structures
- Removed initialization variables from structs and passed them as
function parameters
- Removed various comments and error messages
- Simplified mode handling
- Refactored per-PMIC rpmh-regulator data specification
- Simplified probe function
- Moved header into DT patch
- Removed redundant property listings from DT binding documentation
[1]: https://lkml.org/lkml/2018/5/9/729
[2]: https://lkml.org/lkml/2018/4/10/714
[3]: https://lkml.org/lkml/2018/4/18/556
[4]: https://lkml.org/lkml/2018/5/11/696
[5]: https://lkml.org/lkml/2018/5/15/1005
[6]: https://lkml.org/lkml/2018/6/1/895
[7]: https://lkml.org/lkml/2018/5/22/1168
[8]: https://lkml.org/lkml/2018/5/11/701
[9]: https://lkml.org/lkml/2018/4/13/687
[10]: https://lkml.org/lkml/2018/3/16/1431
David Collins (2):
regulator: dt-bindings: add QCOM RPMh regulator bindings
regulator: add QCOM RPMh regulator driver
.../bindings/regulator/qcom,rpmh-regulator.txt | 160 +++++
drivers/regulator/Kconfig | 9 +
drivers/regulator/Makefile | 1 +
drivers/regulator/qcom-rpmh-regulator.c | 767 +++++++++++++++++++++
.../dt-bindings/regulator/qcom,rpmh-regulator.h | 36 +
5 files changed, 973 insertions(+)
create mode 100644 Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt
create mode 100644 drivers/regulator/qcom-rpmh-regulator.c
create mode 100644 include/dt-bindings/regulator/qcom,rpmh-regulator.h
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* [PATCH v6 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
From: David Collins @ 2018-06-04 19:15 UTC (permalink / raw)
To: broonie, lgirdwood, robh+dt, mark.rutland
Cc: David Collins, linux-arm-msm, linux-arm-kernel, devicetree,
linux-kernel, rnayak, sboyd, dianders
In-Reply-To: <cover.1528138319.git.collinsd@codeaurora.org>
Introduce bindings for RPMh regulator devices found on some
Qualcomm Technlogies, Inc. SoCs. These devices allow a given
processor within the SoC to make PMIC regulator requests which
are aggregated within the RPMh hardware block along with requests
from other processors in the SoC to determine the final PMIC
regulator hardware state.
Signed-off-by: David Collins <collinsd@codeaurora.org>
Reviewed-by: Rob Herring <robh@kernel.org>
---
.../bindings/regulator/qcom,rpmh-regulator.txt | 160 +++++++++++++++++++++
.../dt-bindings/regulator/qcom,rpmh-regulator.h | 36 +++++
2 files changed, 196 insertions(+)
create mode 100644 Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt
create mode 100644 include/dt-bindings/regulator/qcom,rpmh-regulator.h
diff --git a/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt b/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt
new file mode 100644
index 0000000..7ef2dbe
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt
@@ -0,0 +1,160 @@
+Qualcomm Technologies, Inc. RPMh Regulators
+
+rpmh-regulator devices support PMIC regulator management via the Voltage
+Regulator Manager (VRM) and Oscillator Buffer (XOB) RPMh accelerators. The APPS
+processor communicates with these hardware blocks via a Resource State
+Coordinator (RSC) using command packets. The VRM allows changing three
+parameters for a given regulator: enable state, output voltage, and operating
+mode. The XOB allows changing only a single parameter for a given regulator:
+its enable state. Despite its name, the XOB is capable of controlling the
+enable state of any PMIC peripheral. It is used for clock buffers, low-voltage
+switches, and LDO/SMPS regulators which have a fixed voltage and mode.
+
+=======================
+Required Node Structure
+=======================
+
+RPMh regulators must be described in two levels of device nodes. The first
+level describes the PMIC containing the regulators and must reside within an
+RPMh device node. The second level describes each regulator within the PMIC
+which is to be used on the board. Each of these regulators maps to a single
+RPMh resource.
+
+The names used for regulator nodes must match those supported by a given PMIC.
+Supported regulator node names:
+ PM8998: smps1 - smps13, ldo1 - ldo28, lvs1 - lvs2
+ PMI8998: bob
+ PM8005: smps1 - smps4
+
+========================
+First Level Nodes - PMIC
+========================
+
+- compatible
+ Usage: required
+ Value type: <string>
+ Definition: Must be one of: "qcom,pm8998-rpmh-regulators",
+ "qcom,pmi8998-rpmh-regulators" or
+ "qcom,pm8005-rpmh-regulators".
+
+- qcom,pmic-id
+ Usage: required
+ Value type: <string>
+ Definition: RPMh resource name suffix used for the regulators found on
+ this PMIC. Typical values: "a", "b", "c", "d", "e", "f".
+
+- vdd-s1-supply
+- vdd-s2-supply
+- vdd-s3-supply
+- vdd-s4-supply
+ Usage: optional (PM8998 and PM8005 only)
+ Value type: <phandle>
+ Definition: phandle of the parent supply regulator of one or more of the
+ regulators for this PMIC.
+
+- vdd-s5-supply
+- vdd-s6-supply
+- vdd-s7-supply
+- vdd-s8-supply
+- vdd-s9-supply
+- vdd-s10-supply
+- vdd-s11-supply
+- vdd-s12-supply
+- vdd-s13-supply
+- vdd-l1-l27-supply
+- vdd-l2-l8-l17-supply
+- vdd-l3-l11-supply
+- vdd-l4-l5-supply
+- vdd-l6-supply
+- vdd-l7-l12-l14-l15-supply
+- vdd-l9-supply
+- vdd-l10-l23-l25-supply
+- vdd-l13-l19-l21-supply
+- vdd-l16-l28-supply
+- vdd-l18-l22-supply
+- vdd-l20-l24-supply
+- vdd-l26-supply
+- vin-lvs-1-2-supply
+ Usage: optional (PM8998 only)
+ Value type: <phandle>
+ Definition: phandle of the parent supply regulator of one or more of the
+ regulators for this PMIC.
+
+- vdd-bob-supply
+ Usage: optional (PMI8998 only)
+ Value type: <phandle>
+ Definition: BOB regulator parent supply phandle
+
+===============================
+Second Level Nodes - Regulators
+===============================
+
+- qcom,always-wait-for-ack
+ Usage: optional
+ Value type: <empty>
+ Definition: Boolean flag which indicates that the application processor
+ must wait for an ACK or a NACK from RPMh for every request
+ sent for this regulator including those which are for a
+ strictly lower power state.
+
+Other properties defined in Documentation/devicetree/bindings/regulator.txt
+may also be used. regulator-initial-mode and regulator-allowed-modes may be
+specified for VRM regulators using mode values from
+include/dt-bindings/regulator/qcom,rpmh-regulator.h. regulator-allow-bypass
+may be specified for BOB type regulators managed via VRM.
+regulator-allow-set-load may be specified for LDO type regulators managed via
+VRM.
+
+========
+Examples
+========
+
+#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
+
+&apps_rsc {
+ pm8998-rpmh-regulators {
+ compatible = "qcom,pm8998-rpmh-regulators";
+ qcom,pmic-id = "a";
+
+ vdd-l7-l12-l14-l15-supply = <&pm8998_s5>;
+
+ smps2 {
+ regulator-min-microvolt = <1100000>;
+ regulator-max-microvolt = <1100000>;
+ };
+
+ pm8998_s5: smps5 {
+ regulator-min-microvolt = <1904000>;
+ regulator-max-microvolt = <2040000>;
+ };
+
+ ldo7 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ regulator-allowed-modes =
+ <RPMH_REGULATOR_MODE_LPM
+ RPMH_REGULATOR_MODE_HPM>;
+ regulator-allow-set-load;
+ };
+
+ lvs1 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ };
+ };
+
+ pmi8998-rpmh-regulators {
+ compatible = "qcom,pmi8998-rpmh-regulators";
+ qcom,pmic-id = "b";
+
+ bob {
+ regulator-min-microvolt = <3312000>;
+ regulator-max-microvolt = <3600000>;
+ regulator-allowed-modes =
+ <RPMH_REGULATOR_MODE_AUTO
+ RPMH_REGULATOR_MODE_HPM>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_AUTO>;
+ };
+ };
+};
diff --git a/include/dt-bindings/regulator/qcom,rpmh-regulator.h b/include/dt-bindings/regulator/qcom,rpmh-regulator.h
new file mode 100644
index 0000000..86713dc
--- /dev/null
+++ b/include/dt-bindings/regulator/qcom,rpmh-regulator.h
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2018, The Linux Foundation. All rights reserved. */
+
+#ifndef __QCOM_RPMH_REGULATOR_H
+#define __QCOM_RPMH_REGULATOR_H
+
+/*
+ * These mode constants may be used to specify modes for various RPMh regulator
+ * device tree properties (e.g. regulator-initial-mode). Each type of regulator
+ * supports a subset of the possible modes.
+ *
+ * %RPMH_REGULATOR_MODE_RET: Retention mode in which only an extremely small
+ * load current is allowed. This mode is supported
+ * by LDO and SMPS type regulators.
+ * %RPMH_REGULATOR_MODE_LPM: Low power mode in which a small load current is
+ * allowed. This mode corresponds to PFM for SMPS
+ * and BOB type regulators. This mode is supported
+ * by LDO, HFSMPS, BOB, and PMIC4 FTSMPS type
+ * regulators.
+ * %RPMH_REGULATOR_MODE_AUTO: Auto mode in which the regulator hardware
+ * automatically switches between LPM and HPM based
+ * upon the real-time load current. This mode is
+ * supported by HFSMPS, BOB, and PMIC4 FTSMPS type
+ * regulators.
+ * %RPMH_REGULATOR_MODE_HPM: High power mode in which the full rated current
+ * of the regulator is allowed. This mode
+ * corresponds to PWM for SMPS and BOB type
+ * regulators. This mode is supported by all types
+ * of regulators.
+ */
+#define RPMH_REGULATOR_MODE_RET 0
+#define RPMH_REGULATOR_MODE_LPM 1
+#define RPMH_REGULATOR_MODE_AUTO 2
+#define RPMH_REGULATOR_MODE_HPM 3
+
+#endif
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH v6 2/2] regulator: add QCOM RPMh regulator driver
From: David Collins @ 2018-06-04 19:15 UTC (permalink / raw)
To: broonie, lgirdwood, robh+dt, mark.rutland
Cc: David Collins, linux-arm-msm, linux-arm-kernel, devicetree,
linux-kernel, rnayak, sboyd, dianders
In-Reply-To: <cover.1528138319.git.collinsd@codeaurora.org>
Add the QCOM RPMh regulator driver to manage PMIC regulators
which are controlled via RPMh on some Qualcomm Technologies, Inc.
SoCs. RPMh is a hardware block which contains several
accelerators which are used to manage various hardware resources
that are shared between the processors of the SoC. The final
hardware state of a regulator is determined within RPMh by
performing max aggregation of the requests made by all of the
processors.
Add support for PMIC regulator control via the voltage regulator
manager (VRM) and oscillator buffer (XOB) RPMh accelerators.
VRM supports manipulation of enable state, voltage, and mode.
XOB supports manipulation of enable state.
Signed-off-by: David Collins <collinsd@codeaurora.org>
---
drivers/regulator/Kconfig | 9 +
drivers/regulator/Makefile | 1 +
drivers/regulator/qcom-rpmh-regulator.c | 767 ++++++++++++++++++++++++++++++++
3 files changed, 777 insertions(+)
create mode 100644 drivers/regulator/qcom-rpmh-regulator.c
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 097f617..68b369c 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -671,6 +671,15 @@ config REGULATOR_QCOM_RPM
Qualcomm RPM as a module. The module will be named
"qcom_rpm-regulator".
+config REGULATOR_QCOM_RPMH
+ tristate "Qualcomm Technologies, Inc. RPMh regulator driver"
+ depends on QCOM_RPMH || COMPILE_TEST
+ help
+ This driver supports control of PMIC regulators via the RPMh hardware
+ block found on Qualcomm Technologies Inc. SoCs. RPMh regulator
+ control allows for voting on regulator state between multiple
+ processors within the SoC.
+
config REGULATOR_QCOM_SMD_RPM
tristate "Qualcomm SMD based RPM regulator driver"
depends on QCOM_SMD_RPM
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 590674f..0c1c2ef 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -77,6 +77,7 @@ obj-$(CONFIG_REGULATOR_MT6323) += mt6323-regulator.o
obj-$(CONFIG_REGULATOR_MT6380) += mt6380-regulator.o
obj-$(CONFIG_REGULATOR_MT6397) += mt6397-regulator.o
obj-$(CONFIG_REGULATOR_QCOM_RPM) += qcom_rpm-regulator.o
+obj-$(CONFIG_REGULATOR_QCOM_RPMH) += qcom-rpmh-regulator.o
obj-$(CONFIG_REGULATOR_QCOM_SMD_RPM) += qcom_smd-regulator.o
obj-$(CONFIG_REGULATOR_QCOM_SPMI) += qcom_spmi-regulator.o
obj-$(CONFIG_REGULATOR_PALMAS) += palmas-regulator.o
diff --git a/drivers/regulator/qcom-rpmh-regulator.c b/drivers/regulator/qcom-rpmh-regulator.c
new file mode 100644
index 0000000..a7fffb6
--- /dev/null
+++ b/drivers/regulator/qcom-rpmh-regulator.c
@@ -0,0 +1,767 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2018, The Linux Foundation. All rights reserved. */
+
+#define pr_fmt(fmt) "%s: " fmt, __func__
+
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/of_regulator.h>
+
+#include <soc/qcom/cmd-db.h>
+#include <soc/qcom/rpmh.h>
+
+#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
+
+/**
+ * enum rpmh_regulator_type - supported RPMh accelerator types
+ * %VRM: RPMh VRM accelerator which supports voting on enable, voltage,
+ * and mode of LDO, SMPS, and BOB type PMIC regulators.
+ * %XOB: RPMh XOB accelerator which supports voting on the enable state
+ * of PMIC regulators.
+ */
+enum rpmh_regulator_type {
+ VRM,
+ XOB,
+};
+
+#define RPMH_REGULATOR_REG_VRM_VOLTAGE 0x0
+#define RPMH_REGULATOR_REG_ENABLE 0x4
+#define RPMH_REGULATOR_REG_VRM_MODE 0x8
+
+#define RPMH_REGULATOR_MODE_COUNT 4
+
+#define PMIC4_LDO_MODE_RETENTION 4
+#define PMIC4_LDO_MODE_LPM 5
+#define PMIC4_LDO_MODE_HPM 7
+
+#define PMIC4_SMPS_MODE_RETENTION 4
+#define PMIC4_SMPS_MODE_PFM 5
+#define PMIC4_SMPS_MODE_AUTO 6
+#define PMIC4_SMPS_MODE_PWM 7
+
+#define PMIC4_BOB_MODE_PASS 0
+#define PMIC4_BOB_MODE_PFM 1
+#define PMIC4_BOB_MODE_AUTO 2
+#define PMIC4_BOB_MODE_PWM 3
+
+/**
+ * struct rpmh_vreg_hw_data - RPMh regulator hardware configurations
+ * @regulator_type: RPMh accelerator type used to manage this
+ * regulator
+ * @ops: Pointer to regulator ops callback structure
+ * @voltage_range: The single range of voltages supported by this
+ * PMIC regulator type
+ * @n_voltages: The number of unique voltage set points defined
+ * by voltage_range
+ * @hpm_min_load_uA: Minimum load current in microamps that requires
+ * high power mode (HPM) operation. This is used
+ * for LDO hardware type regulators only.
+ * @pmic_mode_map: Array indexed by regulator framework mode
+ * containing PMIC hardware modes. Must be large
+ * enough to index all framework modes supported
+ * by this regulator hardware type.
+ * @of_map_mode: Maps an RPMH_REGULATOR_MODE_* mode value defined
+ * in device tree to a regulator framework mode
+ */
+struct rpmh_vreg_hw_data {
+ enum rpmh_regulator_type regulator_type;
+ const struct regulator_ops *ops;
+ const struct regulator_linear_range voltage_range;
+ int n_voltages;
+ int hpm_min_load_uA;
+ const int *pmic_mode_map;
+ unsigned int (*of_map_mode)(unsigned int mode);
+};
+
+/**
+ * struct rpmh_vreg - individual RPMh regulator data structure encapsulating a
+ * single regulator device
+ * @dev: Device pointer for the top-level PMIC RPMh
+ * regulator parent device. This is used as a
+ * handle in RPMh write requests.
+ * @addr: Base address of the regulator resource within
+ * an RPMh accelerator
+ * @rdesc: Regulator descriptor
+ * @hw_data: PMIC regulator configuration data for this RPMh
+ * regulator
+ * @always_wait_for_ack: Boolean flag indicating if a request must always
+ * wait for an ACK from RPMh before continuing even
+ * if it corresponds to a strictly lower power
+ * state (e.g. enabled --> disabled).
+ * @enabled: Flag indicating if the regulator is enabled or
+ * not
+ * @bypassed: Boolean indicating if the regulator is in
+ * bypass (pass-through) mode or not. This is
+ * only used by BOB rpmh-regulator resources.
+ * @voltage_selector: Selector used for get_voltage_sel() and
+ * set_voltage_sel() callbacks
+ * @mode: RPMh VRM regulator current framework mode
+ */
+struct rpmh_vreg {
+ struct device *dev;
+ u32 addr;
+ struct regulator_desc rdesc;
+ const struct rpmh_vreg_hw_data *hw_data;
+ bool always_wait_for_ack;
+
+ int enabled;
+ bool bypassed;
+ int voltage_selector;
+ unsigned int mode;
+};
+
+/**
+ * struct rpmh_vreg_init_data - initialization data for an RPMh regulator
+ * @name: Name for the regulator which also corresponds
+ * to the device tree subnode name of the regulator
+ * @resource_name: RPMh regulator resource name format string.
+ * This must include exactly one field: '%s' which
+ * is filled at run-time with the PMIC ID provided
+ * by device tree property qcom,pmic-id. Example:
+ * "ldo%s1" for RPMh resource "ldoa1".
+ * @supply_name: Parent supply regulator name
+ * @hw_data: Configuration data for this PMIC regulator type
+ */
+struct rpmh_vreg_init_data {
+ const char *name;
+ const char *resource_name;
+ const char *supply_name;
+ const struct rpmh_vreg_hw_data *hw_data;
+};
+
+/**
+ * rpmh_regulator_send_request() - send the request to RPMh
+ * @vreg: Pointer to the RPMh regulator
+ * @cmd: RPMh commands to send
+ * @count: Size of cmd array
+ * @wait_for_ack: Boolean indicating if execution must wait until the
+ * request has been acknowledged as complete
+ *
+ * Return: 0 on success, errno on failure
+ */
+static int rpmh_regulator_send_request(struct rpmh_vreg *vreg,
+ struct tcs_cmd *cmd, int count, bool wait_for_ack)
+{
+ int ret;
+
+ if (wait_for_ack || vreg->always_wait_for_ack)
+ ret = rpmh_write(vreg->dev, RPMH_ACTIVE_ONLY_STATE, cmd, count);
+ else
+ ret = rpmh_write_async(vreg->dev, RPMH_ACTIVE_ONLY_STATE, cmd,
+ count);
+
+ return ret;
+}
+
+static int _rpmh_regulator_vrm_set_voltage_sel(struct regulator_dev *rdev,
+ unsigned int selector, bool wait_for_ack)
+{
+ struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+ struct tcs_cmd cmd = {
+ .addr = vreg->addr + RPMH_REGULATOR_REG_VRM_VOLTAGE,
+ };
+ int ret;
+
+ /* VRM voltage control register is set with voltage in millivolts. */
+ cmd.data = DIV_ROUND_UP(regulator_list_voltage_linear_range(rdev,
+ selector), 1000);
+
+ ret = rpmh_regulator_send_request(vreg, &cmd, 1, wait_for_ack);
+ if (!ret)
+ vreg->voltage_selector = selector;
+
+ return 0;
+}
+
+static int rpmh_regulator_vrm_set_voltage_sel(struct regulator_dev *rdev,
+ unsigned int selector)
+{
+ struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+
+ if (vreg->enabled == -EINVAL) {
+ /*
+ * Cache the voltage and send it later when the regulator is
+ * enabled or disabled.
+ */
+ vreg->voltage_selector = selector;
+ return 0;
+ }
+
+ return _rpmh_regulator_vrm_set_voltage_sel(rdev, selector,
+ selector > vreg->voltage_selector);
+}
+
+static int rpmh_regulator_vrm_get_voltage_sel(struct regulator_dev *rdev)
+{
+ struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+
+ return vreg->voltage_selector;
+}
+
+static int rpmh_regulator_is_enabled(struct regulator_dev *rdev)
+{
+ struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+
+ return vreg->enabled;
+}
+
+static int rpmh_regulator_enable(struct regulator_dev *rdev)
+{
+ struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+ struct tcs_cmd cmd = {
+ .addr = vreg->addr + RPMH_REGULATOR_REG_ENABLE,
+ .data = 1,
+ };
+ int ret;
+
+ if (vreg->enabled == -EINVAL &&
+ vreg->voltage_selector != -ENOTRECOVERABLE) {
+ ret = _rpmh_regulator_vrm_set_voltage_sel(rdev,
+ vreg->voltage_selector, true);
+ if (ret < 0)
+ return ret;
+ }
+
+ ret = rpmh_regulator_send_request(vreg, &cmd, 1, true);
+ if (!ret)
+ vreg->enabled = true;
+
+ return ret;
+}
+
+static int rpmh_regulator_disable(struct regulator_dev *rdev)
+{
+ struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+ struct tcs_cmd cmd = {
+ .addr = vreg->addr + RPMH_REGULATOR_REG_ENABLE,
+ .data = 0,
+ };
+ int ret;
+
+ if (vreg->enabled == -EINVAL &&
+ vreg->voltage_selector != -ENOTRECOVERABLE) {
+ ret = _rpmh_regulator_vrm_set_voltage_sel(rdev,
+ vreg->voltage_selector, true);
+ if (ret < 0)
+ return ret;
+ }
+
+ ret = rpmh_regulator_send_request(vreg, &cmd, 1, false);
+ if (!ret)
+ vreg->enabled = false;
+
+ return ret;
+}
+
+static int rpmh_regulator_vrm_set_mode_bypass(struct rpmh_vreg *vreg,
+ unsigned int mode, bool bypassed)
+{
+ struct tcs_cmd cmd = {
+ .addr = vreg->addr + RPMH_REGULATOR_REG_VRM_MODE,
+ };
+ int pmic_mode;
+
+ if (mode > REGULATOR_MODE_STANDBY)
+ return -EINVAL;
+
+ pmic_mode = vreg->hw_data->pmic_mode_map[mode];
+ if (pmic_mode < 0)
+ return pmic_mode;
+
+ cmd.data = bypassed ? PMIC4_BOB_MODE_PASS : pmic_mode;
+
+ return rpmh_regulator_send_request(vreg, &cmd, 1, true);
+}
+
+static int rpmh_regulator_vrm_set_mode(struct regulator_dev *rdev,
+ unsigned int mode)
+{
+ struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+ int ret;
+
+ if (mode == vreg->mode)
+ return 0;
+
+ ret = rpmh_regulator_vrm_set_mode_bypass(vreg, mode, vreg->bypassed);
+ if (!ret)
+ vreg->mode = mode;
+
+ return ret;
+}
+
+static unsigned int rpmh_regulator_vrm_get_mode(struct regulator_dev *rdev)
+{
+ struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+
+ return vreg->mode;
+}
+
+/**
+ * rpmh_regulator_vrm_set_load() - set the regulator mode based upon the load
+ * current requested
+ * @rdev: Regulator device pointer for the rpmh-regulator
+ * @load_uA: Aggregated load current in microamps
+ *
+ * This function is used in the regulator_ops for VRM type RPMh regulator
+ * devices.
+ *
+ * Return: 0 on success, errno on failure
+ */
+static int rpmh_regulator_vrm_set_load(struct regulator_dev *rdev, int load_uA)
+{
+ struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+ unsigned int mode;
+
+ if (load_uA >= vreg->hw_data->hpm_min_load_uA)
+ mode = REGULATOR_MODE_FAST;
+ else
+ mode = REGULATOR_MODE_IDLE;
+
+ return rpmh_regulator_vrm_set_mode(rdev, mode);
+}
+
+static int rpmh_regulator_vrm_set_bypass(struct regulator_dev *rdev,
+ bool enable)
+{
+ struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+ int ret;
+
+ if (vreg->bypassed == enable)
+ return 0;
+
+ ret = rpmh_regulator_vrm_set_mode_bypass(vreg, vreg->mode, enable);
+ if (!ret)
+ vreg->bypassed = enable;
+
+ return ret;
+}
+
+static int rpmh_regulator_vrm_get_bypass(struct regulator_dev *rdev,
+ bool *enable)
+{
+ struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+
+ *enable = vreg->bypassed;
+
+ return 0;
+}
+
+static const struct regulator_ops rpmh_regulator_vrm_ops = {
+ .enable = rpmh_regulator_enable,
+ .disable = rpmh_regulator_disable,
+ .is_enabled = rpmh_regulator_is_enabled,
+ .set_voltage_sel = rpmh_regulator_vrm_set_voltage_sel,
+ .get_voltage_sel = rpmh_regulator_vrm_get_voltage_sel,
+ .list_voltage = regulator_list_voltage_linear_range,
+ .set_mode = rpmh_regulator_vrm_set_mode,
+ .get_mode = rpmh_regulator_vrm_get_mode,
+};
+
+static const struct regulator_ops rpmh_regulator_vrm_drms_ops = {
+ .enable = rpmh_regulator_enable,
+ .disable = rpmh_regulator_disable,
+ .is_enabled = rpmh_regulator_is_enabled,
+ .set_voltage_sel = rpmh_regulator_vrm_set_voltage_sel,
+ .get_voltage_sel = rpmh_regulator_vrm_get_voltage_sel,
+ .list_voltage = regulator_list_voltage_linear_range,
+ .set_mode = rpmh_regulator_vrm_set_mode,
+ .get_mode = rpmh_regulator_vrm_get_mode,
+ .set_load = rpmh_regulator_vrm_set_load,
+};
+
+static const struct regulator_ops rpmh_regulator_vrm_bypass_ops = {
+ .enable = rpmh_regulator_enable,
+ .disable = rpmh_regulator_disable,
+ .is_enabled = rpmh_regulator_is_enabled,
+ .set_voltage_sel = rpmh_regulator_vrm_set_voltage_sel,
+ .get_voltage_sel = rpmh_regulator_vrm_get_voltage_sel,
+ .list_voltage = regulator_list_voltage_linear_range,
+ .set_mode = rpmh_regulator_vrm_set_mode,
+ .get_mode = rpmh_regulator_vrm_get_mode,
+ .set_bypass = rpmh_regulator_vrm_set_bypass,
+ .get_bypass = rpmh_regulator_vrm_get_bypass,
+};
+
+static const struct regulator_ops rpmh_regulator_xob_ops = {
+ .enable = rpmh_regulator_enable,
+ .disable = rpmh_regulator_disable,
+ .is_enabled = rpmh_regulator_is_enabled,
+};
+
+/**
+ * rpmh_regulator_init_vreg() - initialize all attributes of an rpmh-regulator
+ * vreg: Pointer to the individual rpmh-regulator resource
+ * dev: Pointer to the top level rpmh-regulator PMIC device
+ * node: Pointer to the individual rpmh-regulator resource
+ * device node
+ * pmic_id: String used to identify the top level rpmh-regulator
+ * PMIC device on the board
+ * rpmh_data: Pointer to a null-terminated array of rpmh-regulator
+ * resources defined for the top level PMIC device
+ *
+ * Return: 0 on success, errno on failure
+ */
+static int rpmh_regulator_init_vreg(struct rpmh_vreg *vreg, struct device *dev,
+ struct device_node *node, const char *pmic_id,
+ const struct rpmh_vreg_init_data *rpmh_data)
+{
+ struct regulator_config reg_config = {};
+ char rpmh_resource_name[20] = "";
+ struct regulator_dev *rdev;
+ struct regulator_init_data *init_data;
+ int ret;
+
+ vreg->dev = dev;
+
+ for (; rpmh_data->name; rpmh_data++)
+ if (!strcmp(rpmh_data->name, node->name))
+ break;
+
+ if (!rpmh_data->name) {
+ dev_err(dev, "Unknown regulator %s\n", node->name);
+ return -EINVAL;
+ }
+
+ scnprintf(rpmh_resource_name, sizeof(rpmh_resource_name),
+ rpmh_data->resource_name, pmic_id);
+
+ vreg->addr = cmd_db_read_addr(rpmh_resource_name);
+ if (!vreg->addr) {
+ dev_err(dev, "%s: could not find RPMh address for resource %s\n",
+ node->name, rpmh_resource_name);
+ return -ENODEV;
+ }
+
+ vreg->rdesc.name = rpmh_data->name;
+ vreg->rdesc.supply_name = rpmh_data->supply_name;
+ vreg->hw_data = rpmh_data->hw_data;
+
+ vreg->enabled = -EINVAL;
+ vreg->voltage_selector = -ENOTRECOVERABLE;
+ vreg->mode = REGULATOR_MODE_INVALID;
+
+ if (rpmh_data->hw_data->n_voltages) {
+ vreg->rdesc.linear_ranges = &rpmh_data->hw_data->voltage_range;
+ vreg->rdesc.n_linear_ranges = 1;
+ vreg->rdesc.n_voltages = rpmh_data->hw_data->n_voltages;
+ }
+
+ vreg->always_wait_for_ack = of_property_read_bool(node,
+ "qcom,always-wait-for-ack");
+
+ vreg->rdesc.owner = THIS_MODULE;
+ vreg->rdesc.type = REGULATOR_VOLTAGE;
+ vreg->rdesc.ops = vreg->hw_data->ops;
+ vreg->rdesc.of_map_mode = vreg->hw_data->of_map_mode;
+
+ init_data = of_get_regulator_init_data(dev, node, &vreg->rdesc);
+ if (!init_data)
+ return -ENOMEM;
+
+ if (rpmh_data->hw_data->regulator_type == XOB &&
+ init_data->constraints.min_uV &&
+ init_data->constraints.min_uV == init_data->constraints.max_uV) {
+ vreg->rdesc.fixed_uV = init_data->constraints.min_uV;
+ vreg->rdesc.n_voltages = 1;
+ }
+
+ reg_config.dev = dev;
+ reg_config.init_data = init_data;
+ reg_config.of_node = node;
+ reg_config.driver_data = vreg;
+
+ rdev = devm_regulator_register(dev, &vreg->rdesc, ®_config);
+ if (IS_ERR(rdev)) {
+ ret = PTR_ERR(rdev);
+ rdev = NULL;
+ dev_err(dev, "%s: devm_regulator_register() failed, ret=%d\n",
+ node->name, ret);
+ return ret;
+ }
+
+ dev_dbg(dev, "%s regulator registered for RPMh resource %s @ 0x%05X\n",
+ node->name, rpmh_resource_name, vreg->addr);
+
+ return 0;
+}
+
+static const int pmic_mode_map_pmic4_ldo[REGULATOR_MODE_STANDBY + 1] = {
+ [REGULATOR_MODE_INVALID] = -EINVAL,
+ [REGULATOR_MODE_STANDBY] = PMIC4_LDO_MODE_RETENTION,
+ [REGULATOR_MODE_IDLE] = PMIC4_LDO_MODE_LPM,
+ [REGULATOR_MODE_NORMAL] = -EINVAL,
+ [REGULATOR_MODE_FAST] = PMIC4_LDO_MODE_HPM,
+};
+
+static unsigned int rpmh_regulator_pmic4_ldo_of_map_mode(unsigned int mode)
+{
+ static const unsigned int of_mode_map[RPMH_REGULATOR_MODE_COUNT] = {
+ [RPMH_REGULATOR_MODE_RET] = REGULATOR_MODE_STANDBY,
+ [RPMH_REGULATOR_MODE_LPM] = REGULATOR_MODE_IDLE,
+ [RPMH_REGULATOR_MODE_AUTO] = REGULATOR_MODE_INVALID,
+ [RPMH_REGULATOR_MODE_HPM] = REGULATOR_MODE_FAST,
+ };
+
+ if (mode >= RPMH_REGULATOR_MODE_COUNT)
+ return -EINVAL;
+
+ return of_mode_map[mode];
+}
+
+static const int pmic_mode_map_pmic4_smps[REGULATOR_MODE_STANDBY + 1] = {
+ [REGULATOR_MODE_INVALID] = -EINVAL,
+ [REGULATOR_MODE_STANDBY] = PMIC4_SMPS_MODE_RETENTION,
+ [REGULATOR_MODE_IDLE] = PMIC4_SMPS_MODE_PFM,
+ [REGULATOR_MODE_NORMAL] = PMIC4_SMPS_MODE_AUTO,
+ [REGULATOR_MODE_FAST] = PMIC4_SMPS_MODE_PWM,
+};
+
+static unsigned int rpmh_regulator_pmic4_smps_of_map_mode(unsigned int mode)
+{
+ static const unsigned int of_mode_map[RPMH_REGULATOR_MODE_COUNT] = {
+ [RPMH_REGULATOR_MODE_RET] = REGULATOR_MODE_STANDBY,
+ [RPMH_REGULATOR_MODE_LPM] = REGULATOR_MODE_IDLE,
+ [RPMH_REGULATOR_MODE_AUTO] = REGULATOR_MODE_NORMAL,
+ [RPMH_REGULATOR_MODE_HPM] = REGULATOR_MODE_FAST,
+ };
+
+ if (mode >= RPMH_REGULATOR_MODE_COUNT)
+ return -EINVAL;
+
+ return of_mode_map[mode];
+}
+
+static const int pmic_mode_map_pmic4_bob[REGULATOR_MODE_STANDBY + 1] = {
+ [REGULATOR_MODE_INVALID] = -EINVAL,
+ [REGULATOR_MODE_STANDBY] = -EINVAL,
+ [REGULATOR_MODE_IDLE] = PMIC4_BOB_MODE_PFM,
+ [REGULATOR_MODE_NORMAL] = PMIC4_BOB_MODE_AUTO,
+ [REGULATOR_MODE_FAST] = PMIC4_BOB_MODE_PWM,
+};
+
+static unsigned int rpmh_regulator_pmic4_bob_of_map_mode(unsigned int mode)
+{
+ static const unsigned int of_mode_map[RPMH_REGULATOR_MODE_COUNT] = {
+ [RPMH_REGULATOR_MODE_RET] = REGULATOR_MODE_INVALID,
+ [RPMH_REGULATOR_MODE_LPM] = REGULATOR_MODE_IDLE,
+ [RPMH_REGULATOR_MODE_AUTO] = REGULATOR_MODE_NORMAL,
+ [RPMH_REGULATOR_MODE_HPM] = REGULATOR_MODE_FAST,
+ };
+
+ if (mode >= RPMH_REGULATOR_MODE_COUNT)
+ return -EINVAL;
+
+ return of_mode_map[mode];
+}
+
+static const struct rpmh_vreg_hw_data pmic4_pldo = {
+ .regulator_type = VRM,
+ .ops = &rpmh_regulator_vrm_drms_ops,
+ .voltage_range = REGULATOR_LINEAR_RANGE(1664000, 0, 255, 8000),
+ .n_voltages = 256,
+ .hpm_min_load_uA = 10000,
+ .pmic_mode_map = pmic_mode_map_pmic4_ldo,
+ .of_map_mode = rpmh_regulator_pmic4_ldo_of_map_mode,
+};
+
+static const struct rpmh_vreg_hw_data pmic4_pldo_lv = {
+ .regulator_type = VRM,
+ .ops = &rpmh_regulator_vrm_drms_ops,
+ .voltage_range = REGULATOR_LINEAR_RANGE(1256000, 0, 127, 8000),
+ .n_voltages = 128,
+ .hpm_min_load_uA = 10000,
+ .pmic_mode_map = pmic_mode_map_pmic4_ldo,
+ .of_map_mode = rpmh_regulator_pmic4_ldo_of_map_mode,
+};
+
+static const struct rpmh_vreg_hw_data pmic4_nldo = {
+ .regulator_type = VRM,
+ .ops = &rpmh_regulator_vrm_drms_ops,
+ .voltage_range = REGULATOR_LINEAR_RANGE(312000, 0, 127, 8000),
+ .n_voltages = 128,
+ .hpm_min_load_uA = 30000,
+ .pmic_mode_map = pmic_mode_map_pmic4_ldo,
+ .of_map_mode = rpmh_regulator_pmic4_ldo_of_map_mode,
+};
+
+static const struct rpmh_vreg_hw_data pmic4_hfsmps3 = {
+ .regulator_type = VRM,
+ .ops = &rpmh_regulator_vrm_ops,
+ .voltage_range = REGULATOR_LINEAR_RANGE(320000, 0, 215, 8000),
+ .n_voltages = 216,
+ .pmic_mode_map = pmic_mode_map_pmic4_smps,
+ .of_map_mode = rpmh_regulator_pmic4_smps_of_map_mode,
+};
+
+static const struct rpmh_vreg_hw_data pmic4_ftsmps426 = {
+ .regulator_type = VRM,
+ .ops = &rpmh_regulator_vrm_ops,
+ .voltage_range = REGULATOR_LINEAR_RANGE(320000, 0, 258, 4000),
+ .n_voltages = 259,
+ .pmic_mode_map = pmic_mode_map_pmic4_smps,
+ .of_map_mode = rpmh_regulator_pmic4_smps_of_map_mode,
+};
+
+static const struct rpmh_vreg_hw_data pmic4_bob = {
+ .regulator_type = VRM,
+ .ops = &rpmh_regulator_vrm_bypass_ops,
+ .voltage_range = REGULATOR_LINEAR_RANGE(1824000, 0, 83, 32000),
+ .n_voltages = 84,
+ .pmic_mode_map = pmic_mode_map_pmic4_bob,
+ .of_map_mode = rpmh_regulator_pmic4_bob_of_map_mode,
+};
+
+static const struct rpmh_vreg_hw_data pmic4_lvs = {
+ .regulator_type = XOB,
+ .ops = &rpmh_regulator_xob_ops,
+ /* LVS hardware does not support voltage or mode configuration. */
+};
+
+#define RPMH_VREG(_name, _resource_name, _hw_data, _supply_name) \
+{ \
+ .name = _name, \
+ .resource_name = _resource_name, \
+ .hw_data = _hw_data, \
+ .supply_name = _supply_name, \
+}
+
+static const struct rpmh_vreg_init_data pm8998_vreg_data[] = {
+ RPMH_VREG("smps1", "smp%s1", &pmic4_ftsmps426, "vdd-s1"),
+ RPMH_VREG("smps2", "smp%s2", &pmic4_ftsmps426, "vdd-s2"),
+ RPMH_VREG("smps3", "smp%s3", &pmic4_hfsmps3, "vdd-s3"),
+ RPMH_VREG("smps4", "smp%s4", &pmic4_hfsmps3, "vdd-s4"),
+ RPMH_VREG("smps5", "smp%s5", &pmic4_hfsmps3, "vdd-s5"),
+ RPMH_VREG("smps6", "smp%s6", &pmic4_ftsmps426, "vdd-s6"),
+ RPMH_VREG("smps7", "smp%s7", &pmic4_ftsmps426, "vdd-s7"),
+ RPMH_VREG("smps8", "smp%s8", &pmic4_ftsmps426, "vdd-s8"),
+ RPMH_VREG("smps9", "smp%s9", &pmic4_ftsmps426, "vdd-s9"),
+ RPMH_VREG("smps10", "smp%s10", &pmic4_ftsmps426, "vdd-s10"),
+ RPMH_VREG("smps11", "smp%s11", &pmic4_ftsmps426, "vdd-s11"),
+ RPMH_VREG("smps12", "smp%s12", &pmic4_ftsmps426, "vdd-s12"),
+ RPMH_VREG("smps13", "smp%s13", &pmic4_ftsmps426, "vdd-s13"),
+ RPMH_VREG("ldo1", "ldo%s1", &pmic4_nldo, "vdd-l1-l27"),
+ RPMH_VREG("ldo2", "ldo%s2", &pmic4_nldo, "vdd-l2-l8-l17"),
+ RPMH_VREG("ldo3", "ldo%s3", &pmic4_nldo, "vdd-l3-l11"),
+ RPMH_VREG("ldo4", "ldo%s4", &pmic4_nldo, "vdd-l4-l5"),
+ RPMH_VREG("ldo5", "ldo%s5", &pmic4_nldo, "vdd-l4-l5"),
+ RPMH_VREG("ldo6", "ldo%s6", &pmic4_pldo, "vdd-l6"),
+ RPMH_VREG("ldo7", "ldo%s7", &pmic4_pldo_lv, "vdd-l7-l12-l14-l15"),
+ RPMH_VREG("ldo8", "ldo%s8", &pmic4_nldo, "vdd-l2-l8-l17"),
+ RPMH_VREG("ldo9", "ldo%s9", &pmic4_pldo, "vdd-l9"),
+ RPMH_VREG("ldo10", "ldo%s10", &pmic4_pldo, "vdd-l10-l23-l25"),
+ RPMH_VREG("ldo11", "ldo%s11", &pmic4_nldo, "vdd-l3-l11"),
+ RPMH_VREG("ldo12", "ldo%s12", &pmic4_pldo_lv, "vdd-l7-l12-l14-l15"),
+ RPMH_VREG("ldo13", "ldo%s13", &pmic4_pldo, "vdd-l13-l19-l21"),
+ RPMH_VREG("ldo14", "ldo%s14", &pmic4_pldo_lv, "vdd-l7-l12-l14-l15"),
+ RPMH_VREG("ldo15", "ldo%s15", &pmic4_pldo_lv, "vdd-l7-l12-l14-l15"),
+ RPMH_VREG("ldo16", "ldo%s16", &pmic4_pldo, "vdd-l16-l28"),
+ RPMH_VREG("ldo17", "ldo%s17", &pmic4_nldo, "vdd-l2-l8-l17"),
+ RPMH_VREG("ldo18", "ldo%s18", &pmic4_pldo, "vdd-l18-l22"),
+ RPMH_VREG("ldo19", "ldo%s19", &pmic4_pldo, "vdd-l13-l19-l21"),
+ RPMH_VREG("ldo20", "ldo%s20", &pmic4_pldo, "vdd-l20-l24"),
+ RPMH_VREG("ldo21", "ldo%s21", &pmic4_pldo, "vdd-l13-l19-l21"),
+ RPMH_VREG("ldo22", "ldo%s22", &pmic4_pldo, "vdd-l18-l22"),
+ RPMH_VREG("ldo23", "ldo%s23", &pmic4_pldo, "vdd-l10-l23-l25"),
+ RPMH_VREG("ldo24", "ldo%s24", &pmic4_pldo, "vdd-l20-l24"),
+ RPMH_VREG("ldo25", "ldo%s25", &pmic4_pldo, "vdd-l10-l23-l25"),
+ RPMH_VREG("ldo26", "ldo%s26", &pmic4_nldo, "vdd-l26"),
+ RPMH_VREG("ldo27", "ldo%s27", &pmic4_nldo, "vdd-l1-l27"),
+ RPMH_VREG("ldo28", "ldo%s28", &pmic4_pldo, "vdd-l16-l28"),
+ RPMH_VREG("lvs1", "vs%s1", &pmic4_lvs, "vin-lvs-1-2"),
+ RPMH_VREG("lvs2", "vs%s2", &pmic4_lvs, "vin-lvs-1-2"),
+ {},
+};
+
+static const struct rpmh_vreg_init_data pmi8998_vreg_data[] = {
+ RPMH_VREG("bob", "bob%s1", &pmic4_bob, "vdd-bob"),
+ {},
+};
+
+static const struct rpmh_vreg_init_data pm8005_vreg_data[] = {
+ RPMH_VREG("smps1", "smp%s1", &pmic4_ftsmps426, "vdd-s1"),
+ RPMH_VREG("smps2", "smp%s2", &pmic4_ftsmps426, "vdd-s2"),
+ RPMH_VREG("smps3", "smp%s3", &pmic4_ftsmps426, "vdd-s3"),
+ RPMH_VREG("smps4", "smp%s4", &pmic4_ftsmps426, "vdd-s4"),
+ {},
+};
+
+static int rpmh_regulator_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ const struct rpmh_vreg_init_data *vreg_data;
+ struct device_node *node;
+ struct rpmh_vreg *vreg;
+ const char *pmic_id;
+ int ret;
+
+ ret = cmd_db_ready();
+ if (ret < 0) {
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "Command DB not available, ret=%d\n", ret);
+ return ret;
+ }
+
+ vreg_data = of_device_get_match_data(dev);
+ if (!vreg_data)
+ return -ENODEV;
+
+ ret = of_property_read_string(dev->of_node, "qcom,pmic-id", &pmic_id);
+ if (ret < 0) {
+ dev_err(dev, "qcom,pmic-id missing in DT node\n");
+ return ret;
+ }
+
+ for_each_available_child_of_node(dev->of_node, node) {
+ vreg = devm_kzalloc(dev, sizeof(*vreg), GFP_KERNEL);
+ if (!vreg) {
+ of_node_put(node);
+ return -ENOMEM;
+ }
+
+ ret = rpmh_regulator_init_vreg(vreg, dev, node, pmic_id,
+ vreg_data);
+ if (ret < 0) {
+ of_node_put(node);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+static const struct of_device_id rpmh_regulator_match_table[] = {
+ {
+ .compatible = "qcom,pm8998-rpmh-regulators",
+ .data = pm8998_vreg_data,
+ },
+ {
+ .compatible = "qcom,pmi8998-rpmh-regulators",
+ .data = pmi8998_vreg_data,
+ },
+ {
+ .compatible = "qcom,pm8005-rpmh-regulators",
+ .data = pm8005_vreg_data,
+ },
+ {}
+};
+MODULE_DEVICE_TABLE(of, rpmh_regulator_match_table);
+
+static struct platform_driver rpmh_regulator_driver = {
+ .driver = {
+ .name = "qcom-rpmh-regulator",
+ .of_match_table = of_match_ptr(rpmh_regulator_match_table),
+ },
+ .probe = rpmh_regulator_probe,
+};
+module_platform_driver(rpmh_regulator_driver);
+
+MODULE_DESCRIPTION("Qualcomm RPMh regulator driver");
+MODULE_LICENSE("GPL v2");
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* Re: [PATCH v9 3/7] i2c: fsi: Add port structures
From: Andy Shevchenko @ 2018-06-04 19:17 UTC (permalink / raw)
To: Eddie James
Cc: linux-i2c, Linux Kernel Mailing List, devicetree, Wolfram Sang,
Rob Herring, Benjamin Herrenschmidt, Joel Stanley, Mark Rutland,
Greg Kroah-Hartman, Randy Dunlap
In-Reply-To: <1528138850-18259-4-git-send-email-eajames@linux.vnet.ibm.com>
On Mon, Jun 4, 2018 at 10:00 PM, Eddie James <eajames@linux.vnet.ibm.com> wrote:
> Add and initialize I2C adapters for each port on the FSI-attached I2C
> master. Ports for each master are defined in the devicetree.
>
> Signed-off-by: Eddie James <eajames@linux.vnet.ibm.com>
> ---
> drivers/i2c/busses/i2c-fsi.c | 90 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 90 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-fsi.c b/drivers/i2c/busses/i2c-fsi.c
> index e1b183c..12130c3 100644
> --- a/drivers/i2c/busses/i2c-fsi.c
> +++ b/drivers/i2c/busses/i2c-fsi.c
> @@ -17,7 +17,10 @@
> #include <linux/fsi.h>
> #include <linux/i2c.h>
> #include <linux/kernel.h>
> +#include <linux/list.h>
> #include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/slab.h>
>
> #define FSI_ENGID_I2C 0x7
>
> @@ -123,6 +126,14 @@
> struct fsi_i2c_master {
> struct fsi_device *fsi;
> u8 fifo_size;
> + struct list_head ports;
> +};
> +
> +struct fsi_i2c_port {
> + struct list_head list;
> + struct i2c_adapter adapter;
> + struct fsi_i2c_master *master;
> + u16 port;
> };
>
> static int fsi_i2c_read_reg(struct fsi_device *fsi, unsigned int reg,
> @@ -176,9 +187,38 @@ static int fsi_i2c_dev_init(struct fsi_i2c_master *i2c)
> return fsi_i2c_write_reg(i2c->fsi, I2C_FSI_WATER_MARK, &watermark);
> }
>
> +static int fsi_i2c_set_port(struct fsi_i2c_port *port)
> +{
> + int rc;
> + struct fsi_device *fsi = port->master->fsi;
> + u32 mode, dummy = 0;
> +
> + rc = fsi_i2c_read_reg(fsi, I2C_FSI_MODE, &mode);
> + if (rc)
> + return rc;
> +
> + if (FIELD_GET(I2C_MODE_PORT, mode) == port->port)
> + return 0;
> +
> + mode = (mode & ~I2C_MODE_PORT) | FIELD_PREP(I2C_MODE_PORT, port->port);
Did you consider to split this to two lines / assignments?
> + rc = fsi_i2c_write_reg(fsi, I2C_FSI_MODE, &mode);
> + if (rc)
> + return rc;
> +
> + /* reset engine when port is changed */
> + return fsi_i2c_write_reg(fsi, I2C_FSI_RESET_ERR, &dummy);
> +}
> +
> static int fsi_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
> int num)
> {
> + int rc;
> + struct fsi_i2c_port *port = adap->algo_data;
> +
> + rc = fsi_i2c_set_port(port);
> + if (rc)
> + return rc;
> +
> return -EOPNOTSUPP;
> }
>
> @@ -196,23 +236,72 @@ static u32 fsi_i2c_functionality(struct i2c_adapter *adap)
> static int fsi_i2c_probe(struct device *dev)
> {
> struct fsi_i2c_master *i2c;
> + struct fsi_i2c_port *port;
> + struct device_node *np;
> int rc;
> + u32 port_no;
>
> i2c = devm_kzalloc(dev, sizeof(*i2c), GFP_KERNEL);
> if (!i2c)
> return -ENOMEM;
>
> i2c->fsi = to_fsi_dev(dev);
> + INIT_LIST_HEAD(&i2c->ports);
>
> rc = fsi_i2c_dev_init(i2c);
> if (rc)
> return rc;
>
> + /* Add adapter for each i2c port of the master. */
> + for_each_available_child_of_node(dev->of_node, np) {
> + rc = of_property_read_u32(np, "reg", &port_no);
> + if (rc || port_no > USHRT_MAX)
> + continue;
> +
> + port = kzalloc(sizeof(*port), GFP_KERNEL);
> + if (!port)
> + break;
> +
> + port->master = i2c;
> + port->port = port_no;
> +
> + port->adapter.owner = THIS_MODULE;
> + port->adapter.dev.of_node = np;
> + port->adapter.dev.parent = dev;
> + port->adapter.algo = &fsi_i2c_algorithm;
> + port->adapter.algo_data = port;
> +
> + snprintf(port->adapter.name, sizeof(port->adapter.name),
> + "i2c_bus-%u", port_no);
> +
> + rc = i2c_add_adapter(&port->adapter);
> + if (rc < 0) {
> + dev_err(dev, "Failed to register adapter: %d\n", rc);
> + kfree(port);
> + continue;
> + }
> +
> + list_add(&port->list, &i2c->ports);
> + }
> +
> dev_set_drvdata(dev, i2c);
>
> return 0;
> }
>
> +static int fsi_i2c_remove(struct device *dev)
> +{
> + struct fsi_i2c_master *i2c = dev_get_drvdata(dev);
> + struct fsi_i2c_port *port;
> +
> + list_for_each_entry(port, &i2c->ports, list) {
> + i2c_del_adapter(&port->adapter);
> + kfree(port);
> + }
Just to be sure, it will be called if and only if all adapters are not
busy. Correct?
> +
> + return 0;
> +}
> +
> static const struct fsi_device_id fsi_i2c_ids[] = {
> { FSI_ENGID_I2C, FSI_VERSION_ANY },
> { 0 }
> @@ -224,6 +313,7 @@ static int fsi_i2c_probe(struct device *dev)
> .name = "i2c-fsi",
> .bus = &fsi_bus_type,
> .probe = fsi_i2c_probe,
> + .remove = fsi_i2c_remove,
> },
> };
>
> --
> 1.8.3.1
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v9 2/7] i2c: Add FSI-attached I2C master algorithm
From: Andy Shevchenko @ 2018-06-04 19:21 UTC (permalink / raw)
To: Eddie James
Cc: linux-i2c, Linux Kernel Mailing List, devicetree, Wolfram Sang,
Rob Herring, Benjamin Herrenschmidt, Joel Stanley, Mark Rutland,
Greg Kroah-Hartman, Randy Dunlap
In-Reply-To: <1528138850-18259-3-git-send-email-eajames@linux.vnet.ibm.com>
On Mon, Jun 4, 2018 at 10:00 PM, Eddie James <eajames@linux.vnet.ibm.com> wrote:
> Add register definitions for FSI-attached I2C master and functions to
> access those registers over FSI. Add an FSI driver so that our I2C bus
> is probed up during an FSI scan.
>
> Signed-off-by: Eddie James <eajames@linux.vnet.ibm.com>
> ---
> drivers/i2c/busses/Kconfig | 11 ++
> drivers/i2c/busses/Makefile | 1 +
> drivers/i2c/busses/i2c-fsi.c | 234 +++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 246 insertions(+)
> create mode 100644 drivers/i2c/busses/i2c-fsi.c
>
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 4f8df2e..cddd159 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -1330,4 +1330,15 @@ config I2C_ZX2967
> This driver can also be built as a module. If so, the module will be
> called i2c-zx2967.
>
> +config I2C_FSI
> + tristate "FSI I2C driver"
> + depends on FSI
> + help
> + Driver for FSI bus attached I2C masters. These are I2C masters that
> + are connected to the system over an FSI bus, instead of the more
> + common PCI or MMIO interface.
> +
> + This driver can also be built as a module. If so, the module will be
> + called as i2c-fsi.
> +
> endmenu
> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> index 5a86914..4909fd6 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -137,5 +137,6 @@ obj-$(CONFIG_I2C_PCA_ISA) += i2c-pca-isa.o
> obj-$(CONFIG_I2C_SIBYTE) += i2c-sibyte.o
> obj-$(CONFIG_I2C_XGENE_SLIMPRO) += i2c-xgene-slimpro.o
> obj-$(CONFIG_SCx200_ACB) += scx200_acb.o
> +obj-$(CONFIG_I2C_FSI) += i2c-fsi.o
>
> ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG
> diff --git a/drivers/i2c/busses/i2c-fsi.c b/drivers/i2c/busses/i2c-fsi.c
> new file mode 100644
> index 0000000..e1b183c
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-fsi.c
> @@ -0,0 +1,234 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * FSI-attached I2C master algorithm
> + *
> + * Copyright 2018 IBM Corporation
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/bitops.h>
> +#include <linux/device.h>
> +#include <linux/errno.h>
> +#include <linux/fsi.h>
> +#include <linux/i2c.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +
> +#define FSI_ENGID_I2C 0x7
> +
> +#define I2C_DEFAULT_CLK_DIV 6
> +
> +/* i2c registers */
> +#define I2C_FSI_FIFO 0x00
> +#define I2C_FSI_CMD 0x04
> +#define I2C_FSI_MODE 0x08
> +#define I2C_FSI_WATER_MARK 0x0C
> +#define I2C_FSI_INT_MASK 0x10
> +#define I2C_FSI_INT_COND 0x14
> +#define I2C_FSI_OR_INT_MASK 0x14
> +#define I2C_FSI_INTS 0x18
> +#define I2C_FSI_AND_INT_MASK 0x18
> +#define I2C_FSI_STAT 0x1C
> +#define I2C_FSI_RESET_I2C 0x1C
> +#define I2C_FSI_ESTAT 0x20
> +#define I2C_FSI_RESET_ERR 0x20
> +#define I2C_FSI_RESID_LEN 0x24
> +#define I2C_FSI_SET_SCL 0x24
> +#define I2C_FSI_PORT_BUSY 0x28
> +#define I2C_FSI_RESET_SCL 0x2C
> +#define I2C_FSI_SET_SDA 0x30
> +#define I2C_FSI_RESET_SDA 0x34
> +
> +/* cmd register */
> +#define I2C_CMD_WITH_START BIT(31)
> +#define I2C_CMD_WITH_ADDR BIT(30)
> +#define I2C_CMD_RD_CONT BIT(29)
> +#define I2C_CMD_WITH_STOP BIT(28)
> +#define I2C_CMD_FORCELAUNCH BIT(27)
> +#define I2C_CMD_ADDR GENMASK(23, 17)
> +#define I2C_CMD_READ BIT(16)
> +#define I2C_CMD_LEN GENMASK(15, 0)
> +
> +/* mode register */
> +#define I2C_MODE_CLKDIV GENMASK(31, 16)
> +#define I2C_MODE_PORT GENMASK(15, 10)
> +#define I2C_MODE_ENHANCED BIT(3)
> +#define I2C_MODE_DIAG BIT(2)
> +#define I2C_MODE_PACE_ALLOW BIT(1)
> +#define I2C_MODE_WRAP BIT(0)
> +
> +/* watermark register */
> +#define I2C_WATERMARK_HI GENMASK(15, 12)
> +#define I2C_WATERMARK_LO GENMASK(7, 4)
> +
> +#define I2C_FIFO_HI_LVL 4
> +#define I2C_FIFO_LO_LVL 4
> +
> +/* interrupt register */
> +#define I2C_INT_INV_CMD BIT(15)
> +#define I2C_INT_PARITY BIT(14)
> +#define I2C_INT_BE_OVERRUN BIT(13)
> +#define I2C_INT_BE_ACCESS BIT(12)
> +#define I2C_INT_LOST_ARB BIT(11)
> +#define I2C_INT_NACK BIT(10)
> +#define I2C_INT_DAT_REQ BIT(9)
> +#define I2C_INT_CMD_COMP BIT(8)
> +#define I2C_INT_STOP_ERR BIT(7)
> +#define I2C_INT_BUSY BIT(6)
> +#define I2C_INT_IDLE BIT(5)
> +
> +#define I2C_INT_ENABLE 0x0000ff80
> +#define I2C_INT_ERR 0x0000fcc0
Now it looks like a flags combinations.
For me as for reader would be better to see quickly a decoded line.
My proposal is to introduce something like following
_INT_ALL GENMASK()
_INT_ENABLE (_INT_ALL & ~(_FOO | _BAR))
_INT_ERR ... similar way as above ...
What do you think?
> +
> +/* status register */
> +#define I2C_STAT_INV_CMD BIT(31)
> +#define I2C_STAT_PARITY BIT(30)
> +#define I2C_STAT_BE_OVERRUN BIT(29)
> +#define I2C_STAT_BE_ACCESS BIT(28)
> +#define I2C_STAT_LOST_ARB BIT(27)
> +#define I2C_STAT_NACK BIT(26)
> +#define I2C_STAT_DAT_REQ BIT(25)
> +#define I2C_STAT_CMD_COMP BIT(24)
> +#define I2C_STAT_STOP_ERR BIT(23)
> +#define I2C_STAT_MAX_PORT GENMASK(19, 16)
> +#define I2C_STAT_ANY_INT BIT(15)
> +#define I2C_STAT_SCL_IN BIT(11)
> +#define I2C_STAT_SDA_IN BIT(10)
> +#define I2C_STAT_PORT_BUSY BIT(9)
> +#define I2C_STAT_SELF_BUSY BIT(8)
> +#define I2C_STAT_FIFO_COUNT GENMASK(7, 0)
> +
> +#define I2C_STAT_ERR 0xfc800000
> +#define I2C_STAT_ANY_RESP 0xff800000
Ditto.
> +
> +/* extended status register */
> +#define I2C_ESTAT_FIFO_SZ GENMASK(31, 24)
> +#define I2C_ESTAT_SCL_IN_SY BIT(15)
> +#define I2C_ESTAT_SDA_IN_SY BIT(14)
> +#define I2C_ESTAT_S_SCL BIT(13)
> +#define I2C_ESTAT_S_SDA BIT(12)
> +#define I2C_ESTAT_M_SCL BIT(11)
> +#define I2C_ESTAT_M_SDA BIT(10)
> +#define I2C_ESTAT_HI_WATER BIT(9)
> +#define I2C_ESTAT_LO_WATER BIT(8)
> +#define I2C_ESTAT_PORT_BUSY BIT(7)
> +#define I2C_ESTAT_SELF_BUSY BIT(6)
> +#define I2C_ESTAT_VERSION GENMASK(4, 0)
> +
> +struct fsi_i2c_master {
> + struct fsi_device *fsi;
> + u8 fifo_size;
> +};
> +
> +static int fsi_i2c_read_reg(struct fsi_device *fsi, unsigned int reg,
> + u32 *data)
> +{
> + int rc;
> + __be32 data_be;
> +
> + rc = fsi_device_read(fsi, reg, &data_be, sizeof(data_be));
> + if (rc)
> + return rc;
> +
> + *data = be32_to_cpu(data_be);
> +
> + return 0;
> +}
> +
> +static int fsi_i2c_write_reg(struct fsi_device *fsi, unsigned int reg,
> + u32 *data)
> +{
> + __be32 data_be = cpu_to_be32p(data);
> +
> + return fsi_device_write(fsi, reg, &data_be, sizeof(data_be));
> +}
> +
> +static int fsi_i2c_dev_init(struct fsi_i2c_master *i2c)
> +{
> + int rc;
> + u32 mode = I2C_MODE_ENHANCED, extended_status, watermark;
> + u32 interrupt = 0;
> +
> + /* since we use polling, disable interrupts */
> + rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_INT_MASK, &interrupt);
> + if (rc)
> + return rc;
> +
> + mode |= FIELD_PREP(I2C_MODE_CLKDIV, I2C_DEFAULT_CLK_DIV);
> + rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_MODE, &mode);
> + if (rc)
> + return rc;
> +
> + rc = fsi_i2c_read_reg(i2c->fsi, I2C_FSI_ESTAT, &extended_status);
> + if (rc)
> + return rc;
> +
> + i2c->fifo_size = FIELD_GET(I2C_ESTAT_FIFO_SZ, extended_status);
> + watermark = FIELD_PREP(I2C_WATERMARK_HI,
> + i2c->fifo_size - I2C_FIFO_HI_LVL);
> + watermark |= FIELD_PREP(I2C_WATERMARK_LO, I2C_FIFO_LO_LVL);
> +
> + return fsi_i2c_write_reg(i2c->fsi, I2C_FSI_WATER_MARK, &watermark);
> +}
> +
> +static int fsi_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
> + int num)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> +static u32 fsi_i2c_functionality(struct i2c_adapter *adap)
> +{
> + return I2C_FUNC_I2C | I2C_FUNC_PROTOCOL_MANGLING | I2C_FUNC_10BIT_ADDR
> + | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_SMBUS_BLOCK_DATA;
Perhaps one property per line?
> +}
> +
> +static const struct i2c_algorithm fsi_i2c_algorithm = {
> + .master_xfer = fsi_i2c_xfer,
> + .functionality = fsi_i2c_functionality,
> +};
> +
> +static int fsi_i2c_probe(struct device *dev)
> +{
> + struct fsi_i2c_master *i2c;
> + int rc;
> +
> + i2c = devm_kzalloc(dev, sizeof(*i2c), GFP_KERNEL);
> + if (!i2c)
> + return -ENOMEM;
> +
> + i2c->fsi = to_fsi_dev(dev);
> +
> + rc = fsi_i2c_dev_init(i2c);
> + if (rc)
> + return rc;
> +
> + dev_set_drvdata(dev, i2c);
> +
> + return 0;
> +}
> +
> +static const struct fsi_device_id fsi_i2c_ids[] = {
> + { FSI_ENGID_I2C, FSI_VERSION_ANY },
> + { 0 }
0 is not required
> +};
> +
> +static struct fsi_driver fsi_i2c_driver = {
> + .id_table = fsi_i2c_ids,
> + .drv = {
> + .name = "i2c-fsi",
> + .bus = &fsi_bus_type,
> + .probe = fsi_i2c_probe,
> + },
> +};
> +
> +module_fsi_driver(fsi_i2c_driver);
> +
> +MODULE_AUTHOR("Eddie James <eajames@us.ibm.com>");
> +MODULE_DESCRIPTION("FSI attached I2C master");
> +MODULE_LICENSE("GPL");
> --
> 1.8.3.1
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v9 0/7] i2c: Add FSI-attached I2C master algorithm
From: Andy Shevchenko @ 2018-06-04 19:22 UTC (permalink / raw)
To: Eddie James
Cc: linux-i2c, Linux Kernel Mailing List, devicetree, Wolfram Sang,
Rob Herring, Benjamin Herrenschmidt, Joel Stanley, Mark Rutland,
Greg Kroah-Hartman, Randy Dunlap
In-Reply-To: <1528138850-18259-1-git-send-email-eajames@linux.vnet.ibm.com>
On Mon, Jun 4, 2018 at 10:00 PM, Eddie James <eajames@linux.vnet.ibm.com> wrote:
> This series adds an algorithm for an I2C master physically located on an FSI
> slave device. The I2C master has multiple ports, each of which may be connected
> to an I2C slave. Access to the I2C master registers is achieved over FSI bus.
>
> Due to the multi-port nature of the I2C master, the driver instantiates a new
> I2C adapter for each port connected to a slave. The connected ports should be
> defined in the device tree under the I2C master device.
>
Thanks for an update.
With some minor comments this looks indeed excellent work!
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Changes since v8
> - Drop unecessary else statements
> - Use i++ instead of ++i
> - Use kzalloc/kfree instead of devm_kzalloc/devm_kfree for port structure
> - Drop the list_empty check in remove
>
> Changes since v7
> - Fix grammer in Kconfig (a -> an)
> - Change I2C registers to use BIT and GENMASK
> - Remove custom macros and use FIELD_PREP and FIELD_GET
> - Fix a few unecessary initializations and "return rc" that are always zero
> - Clean up the read/write fifo functions a bit
> - Few other clean-up items
>
> Changes since v6
> - Remove spinlock for reset functionality; it's unecessary and doesn't work
> with the latest FSI core.
> - Use a mutex instead of a semaphore, and don't wait for timeout to get the
> lock.
> - Use usleeps instead of schedule_timeout; it's not worth the overhead when
> the wait should be very short in between sending the command and receiving
> the response.
>
> Changes since v5
> - Fix reset functionality and do a reset after every transfer failure
>
> Eddie James (7):
> dt-bindings: i2c: Add FSI-attached I2C master dt binding documentation
> i2c: Add FSI-attached I2C master algorithm
> i2c: fsi: Add port structures
> i2c: fsi: Add abort and hardware reset procedures
> i2c: fsi: Add transfer implementation
> i2c: fsi: Add I2C master locking
> i2c: fsi: Add bus recovery
>
> Documentation/devicetree/bindings/i2c/i2c-fsi.txt | 40 ++
> drivers/i2c/busses/Kconfig | 11 +
> drivers/i2c/busses/Makefile | 1 +
> drivers/i2c/busses/i2c-fsi.c | 721 ++++++++++++++++++++++
> 4 files changed, 773 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/i2c/i2c-fsi.txt
> create mode 100644 drivers/i2c/busses/i2c-fsi.c
>
> --
> 1.8.3.1
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v9 5/7] i2c: fsi: Add transfer implementation
From: Eddie James @ 2018-06-04 19:37 UTC (permalink / raw)
To: Andy Shevchenko
Cc: linux-i2c, Linux Kernel Mailing List, devicetree, Wolfram Sang,
Rob Herring, Benjamin Herrenschmidt, Joel Stanley, Mark Rutland,
Greg Kroah-Hartman, Randy Dunlap
In-Reply-To: <CAHp75VdcKg7fzPtfgb2mZ+07nGtA4HXMO2T=P1m+wovnph93Pg@mail.gmail.com>
On 06/04/2018 02:14 PM, Andy Shevchenko wrote:
> On Mon, Jun 4, 2018 at 10:00 PM, Eddie James <eajames@linux.vnet.ibm.com> wrote:
>> Execute I2C transfers from the FSI-attached I2C master. Use polling
>> instead of interrupts as we have no hardware IRQ over FSI.
>>
>> Signed-off-by: Eddie James <eajames@linux.vnet.ibm.com>
>> ---
>> drivers/i2c/busses/i2c-fsi.c | 195 ++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 193 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-fsi.c b/drivers/i2c/busses/i2c-fsi.c
>> index 101b9c5..4e5964e 100644
>> --- a/drivers/i2c/busses/i2c-fsi.c
>> +++ b/drivers/i2c/busses/i2c-fsi.c
>> @@ -150,6 +150,7 @@ struct fsi_i2c_port {
>> struct i2c_adapter adapter;
>> struct fsi_i2c_master *master;
>> u16 port;
>> + u16 xfrd;
>> };
>>
>> static int fsi_i2c_read_reg(struct fsi_device *fsi, unsigned int reg,
>> @@ -225,6 +226,99 @@ static int fsi_i2c_set_port(struct fsi_i2c_port *port)
>> return fsi_i2c_write_reg(fsi, I2C_FSI_RESET_ERR, &dummy);
>> }
>>
>> +static int fsi_i2c_start(struct fsi_i2c_port *port, struct i2c_msg *msg,
>> + bool stop)
>> +{
>> + struct fsi_i2c_master *i2c = port->master;
>> + u32 cmd = I2C_CMD_WITH_START | I2C_CMD_WITH_ADDR;
>> +
>> + port->xfrd = 0;
>> +
>> + if (msg->flags & I2C_M_RD)
>> + cmd |= I2C_CMD_READ;
>> +
>> + if (stop || msg->flags & I2C_M_STOP)
>> + cmd |= I2C_CMD_WITH_STOP;
>> +
>> + cmd |= FIELD_PREP(I2C_CMD_ADDR, msg->addr >> 1);
> Did you consider to unify READ + ADDR as ADDR_8BIT?
Yes, however the result from the i2c_8bit_addr_from_msg function is not
equivalent. This hardware expects the 8-bit address to be concatenated
with the read/not write bit. Also the function returns a u8, which won't
work for addresses with bit 7 set, since it left-shifts by one.
>
>> + cmd |= FIELD_PREP(I2C_CMD_LEN, msg->len);
>> +
>> + return fsi_i2c_write_reg(i2c->fsi, I2C_FSI_CMD, &cmd);
>> +}
>> +
>> +static int fsi_i2c_get_op_bytes(int op_bytes)
>> +{
>> + /* fsi is limited to max 4 byte aligned ops */
>> + if (op_bytes > 4)
>> + return 4;
>> + else if (op_bytes == 3)
> Since you dropped one of 'else', another also is not needed.
True.
>
>> + return 2;
>> + return op_bytes;
>> +}
>> +
>> +static int fsi_i2c_write_fifo(struct fsi_i2c_port *port, struct i2c_msg *msg,
>> + u8 fifo_count)
>> +{
>> + int write;
>> + int rc;
>> + struct fsi_i2c_master *i2c = port->master;
>> + int bytes_to_write = i2c->fifo_size - fifo_count;
>> + int bytes_remaining = msg->len - port->xfrd;
>> +
>> + bytes_to_write = min(bytes_to_write, bytes_remaining);
>> +
>> + while (bytes_to_write) {
>> + write = fsi_i2c_get_op_bytes(bytes_to_write);
>> +
>> + rc = fsi_device_write(i2c->fsi, I2C_FSI_FIFO,
>> + &msg->buf[port->xfrd], write);
>> + if (rc)
>> + return rc;
>> +
>> + port->xfrd += write;
>> + bytes_to_write -= write;
>> + }
> Just to be sure, if by some reason write == 0 in the loop for a while,
> would it become infinite loop?
It's not possible that write == 0. Same for read below.
Thanks,
Eddie
>
>> +
>> + return 0;
>> +}
>> +
>> +static int fsi_i2c_read_fifo(struct fsi_i2c_port *port, struct i2c_msg *msg,
>> + u8 fifo_count)
>> +{
>> + int read;
>> + int rc;
>> + struct fsi_i2c_master *i2c = port->master;
>> + int bytes_to_read;
>> + int xfr_remaining = msg->len - port->xfrd;
>> + u32 dummy;
>> +
>> + bytes_to_read = min_t(int, fifo_count, xfr_remaining);
>> +
>> + while (bytes_to_read) {
>> + read = fsi_i2c_get_op_bytes(bytes_to_read);
>> +
>> + if (xfr_remaining) {
>> + rc = fsi_device_read(i2c->fsi, I2C_FSI_FIFO,
>> + &msg->buf[port->xfrd], read);
>> + if (rc)
>> + return rc;
>> +
>> + port->xfrd += read;
>> + xfr_remaining -= read;
>> + } else {
>> + /* no more buffer but data in fifo, need to clear it */
>> + rc = fsi_device_read(i2c->fsi, I2C_FSI_FIFO, &dummy,
>> + read);
>> + if (rc)
>> + return rc;
>> + }
>> +
>> + bytes_to_read -= read;
>> + }
> Ditto for read == 0.
>
>> +
>> + return 0;
>> +}
>> +
>> static int fsi_i2c_reset_bus(struct fsi_i2c_master *i2c)
>> {
>> int i, rc;
>> @@ -388,17 +482,114 @@ static int fsi_i2c_abort(struct fsi_i2c_port *port, u32 status)
>> return -ETIMEDOUT;
>> }
>>
>> +static int fsi_i2c_handle_status(struct fsi_i2c_port *port,
>> + struct i2c_msg *msg, u32 status)
>> +{
>> + int rc;
>> + u8 fifo_count;
>> +
>> + if (status & I2C_STAT_ERR) {
>> + rc = fsi_i2c_abort(port, status);
>> + if (rc)
>> + return rc;
>> +
>> + if (status & I2C_STAT_INV_CMD)
>> + return -EINVAL;
>> +
>> + if (status & (I2C_STAT_PARITY | I2C_STAT_BE_OVERRUN |
>> + I2C_STAT_BE_ACCESS))
>> + return -EPROTO;
>> +
>> + if (status & I2C_STAT_NACK)
>> + return -ENXIO;
>> +
>> + if (status & I2C_STAT_LOST_ARB)
>> + return -EAGAIN;
>> +
>> + if (status & I2C_STAT_STOP_ERR)
>> + return -EBADMSG;
>> +
>> + return -EIO;
>> + }
>> +
>> + if (status & I2C_STAT_DAT_REQ) {
>> + fifo_count = FIELD_GET(I2C_STAT_FIFO_COUNT, status);
>> +
>> + if (msg->flags & I2C_M_RD)
>> + return fsi_i2c_read_fifo(port, msg, fifo_count);
>> +
>> + return fsi_i2c_write_fifo(port, msg, fifo_count);
>> + }
>> +
>> + if (status & I2C_STAT_CMD_COMP) {
>> + if (port->xfrd < msg->len)
>> + return -ENODATA;
>> +
>> + return msg->len;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int fsi_i2c_wait(struct fsi_i2c_port *port, struct i2c_msg *msg,
>> + unsigned long timeout)
>> +{
>> + u32 status = 0;
>> + int rc;
>> + unsigned long start = jiffies;
>> +
>> + do {
>> + rc = fsi_i2c_read_reg(port->master->fsi, I2C_FSI_STAT,
>> + &status);
>> + if (rc)
>> + return rc;
>> +
>> + if (status & I2C_STAT_ANY_RESP) {
>> + rc = fsi_i2c_handle_status(port, msg, status);
>> + if (rc < 0)
>> + return rc;
>> +
>> + /* cmd complete and all data xfrd */
>> + if (rc == msg->len)
>> + return 0;
>> +
>> + /* need to xfr more data, but maybe don't need wait */
>> + continue;
>> + }
>> +
>> + usleep_range(I2C_CMD_SLEEP_MIN_US, I2C_CMD_SLEEP_MAX_US);
>> + } while (time_after(start + timeout, jiffies));
>> +
>> + return -ETIMEDOUT;
>> +}
>> +
>> static int fsi_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
>> int num)
>> {
>> - int rc;
>> + int i, rc;
>> + unsigned long start_time;
>> struct fsi_i2c_port *port = adap->algo_data;
>> + struct i2c_msg *msg;
>>
>> rc = fsi_i2c_set_port(port);
>> if (rc)
>> return rc;
>>
>> - return -EOPNOTSUPP;
>> + for (i = 0; i < num; i++) {
>> + msg = msgs + i;
>> + start_time = jiffies;
>> +
>> + rc = fsi_i2c_start(port, msg, i == num - 1);
>> + if (rc)
>> + return rc;
>> +
>> + rc = fsi_i2c_wait(port, msg,
>> + adap->timeout - (jiffies - start_time));
>> + if (rc)
>> + return rc;
>> + }
>> +
>> + return 0;
>> }
>>
>> static u32 fsi_i2c_functionality(struct i2c_adapter *adap)
>> --
>> 1.8.3.1
>>
>
>
^ permalink raw reply
* Re: [PATCH v9 3/7] i2c: fsi: Add port structures
From: Eddie James @ 2018-06-04 19:40 UTC (permalink / raw)
To: Andy Shevchenko
Cc: linux-i2c, Linux Kernel Mailing List, devicetree, Wolfram Sang,
Rob Herring, Benjamin Herrenschmidt, Joel Stanley, Mark Rutland,
Greg Kroah-Hartman, Randy Dunlap
In-Reply-To: <CAHp75VfMsL3rBMT-SMcSCyuQd2YMexXqkrb=zBYEq0=MO=zHNA@mail.gmail.com>
On 06/04/2018 02:17 PM, Andy Shevchenko wrote:
> On Mon, Jun 4, 2018 at 10:00 PM, Eddie James <eajames@linux.vnet.ibm.com> wrote:
>> Add and initialize I2C adapters for each port on the FSI-attached I2C
>> master. Ports for each master are defined in the devicetree.
>>
>> Signed-off-by: Eddie James <eajames@linux.vnet.ibm.com>
>> ---
>> drivers/i2c/busses/i2c-fsi.c | 90 ++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 90 insertions(+)
>>
>> diff --git a/drivers/i2c/busses/i2c-fsi.c b/drivers/i2c/busses/i2c-fsi.c
>> index e1b183c..12130c3 100644
>> --- a/drivers/i2c/busses/i2c-fsi.c
>> +++ b/drivers/i2c/busses/i2c-fsi.c
>> @@ -17,7 +17,10 @@
>> #include <linux/fsi.h>
>> #include <linux/i2c.h>
>> #include <linux/kernel.h>
>> +#include <linux/list.h>
>> #include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/slab.h>
>>
>> #define FSI_ENGID_I2C 0x7
>>
>> @@ -123,6 +126,14 @@
>> struct fsi_i2c_master {
>> struct fsi_device *fsi;
>> u8 fifo_size;
>> + struct list_head ports;
>> +};
>> +
>> +struct fsi_i2c_port {
>> + struct list_head list;
>> + struct i2c_adapter adapter;
>> + struct fsi_i2c_master *master;
>> + u16 port;
>> };
>>
>> static int fsi_i2c_read_reg(struct fsi_device *fsi, unsigned int reg,
>> @@ -176,9 +187,38 @@ static int fsi_i2c_dev_init(struct fsi_i2c_master *i2c)
>> return fsi_i2c_write_reg(i2c->fsi, I2C_FSI_WATER_MARK, &watermark);
>> }
>>
>> +static int fsi_i2c_set_port(struct fsi_i2c_port *port)
>> +{
>> + int rc;
>> + struct fsi_device *fsi = port->master->fsi;
>> + u32 mode, dummy = 0;
>> +
>> + rc = fsi_i2c_read_reg(fsi, I2C_FSI_MODE, &mode);
>> + if (rc)
>> + return rc;
>> +
>> + if (FIELD_GET(I2C_MODE_PORT, mode) == port->port)
>> + return 0;
>> +
>> + mode = (mode & ~I2C_MODE_PORT) | FIELD_PREP(I2C_MODE_PORT, port->port);
> Did you consider to split this to two lines / assignments?
It fit on one line (< 80 chars) so I left it as-is...
>
>> + rc = fsi_i2c_write_reg(fsi, I2C_FSI_MODE, &mode);
>> + if (rc)
>> + return rc;
>> +
>> + /* reset engine when port is changed */
>> + return fsi_i2c_write_reg(fsi, I2C_FSI_RESET_ERR, &dummy);
>> +}
>> +
>> static int fsi_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
>> int num)
>> {
>> + int rc;
>> + struct fsi_i2c_port *port = adap->algo_data;
>> +
>> + rc = fsi_i2c_set_port(port);
>> + if (rc)
>> + return rc;
>> +
>> return -EOPNOTSUPP;
>> }
>>
>> @@ -196,23 +236,72 @@ static u32 fsi_i2c_functionality(struct i2c_adapter *adap)
>> static int fsi_i2c_probe(struct device *dev)
>> {
>> struct fsi_i2c_master *i2c;
>> + struct fsi_i2c_port *port;
>> + struct device_node *np;
>> int rc;
>> + u32 port_no;
>>
>> i2c = devm_kzalloc(dev, sizeof(*i2c), GFP_KERNEL);
>> if (!i2c)
>> return -ENOMEM;
>>
>> i2c->fsi = to_fsi_dev(dev);
>> + INIT_LIST_HEAD(&i2c->ports);
>>
>> rc = fsi_i2c_dev_init(i2c);
>> if (rc)
>> return rc;
>>
>> + /* Add adapter for each i2c port of the master. */
>> + for_each_available_child_of_node(dev->of_node, np) {
>> + rc = of_property_read_u32(np, "reg", &port_no);
>> + if (rc || port_no > USHRT_MAX)
>> + continue;
>
>
>> +
>> + port = kzalloc(sizeof(*port), GFP_KERNEL);
>> + if (!port)
>> + break;
>> +
>> + port->master = i2c;
>> + port->port = port_no;
>> +
>> + port->adapter.owner = THIS_MODULE;
>> + port->adapter.dev.of_node = np;
>> + port->adapter.dev.parent = dev;
>> + port->adapter.algo = &fsi_i2c_algorithm;
>> + port->adapter.algo_data = port;
>> +
>> + snprintf(port->adapter.name, sizeof(port->adapter.name),
>> + "i2c_bus-%u", port_no);
>> +
>> + rc = i2c_add_adapter(&port->adapter);
>> + if (rc < 0) {
>> + dev_err(dev, "Failed to register adapter: %d\n", rc);
>> + kfree(port);
>> + continue;
>> + }
>> +
>> + list_add(&port->list, &i2c->ports);
>> + }
>> +
>> dev_set_drvdata(dev, i2c);
>>
>> return 0;
>> }
>>
>> +static int fsi_i2c_remove(struct device *dev)
>> +{
>> + struct fsi_i2c_master *i2c = dev_get_drvdata(dev);
>> + struct fsi_i2c_port *port;
>> +
>> + list_for_each_entry(port, &i2c->ports, list) {
>> + i2c_del_adapter(&port->adapter);
>> + kfree(port);
>> + }
> Just to be sure, it will be called if and only if all adapters are not
> busy. Correct?
If I understand the code and comments correctly, i2c_del_adapter will
block until all references to the adapter device are gone. So, should be
safe to free it now.
Thanks,
Eddie
>
>> +
>> + return 0;
>> +}
>> +
>> static const struct fsi_device_id fsi_i2c_ids[] = {
>> { FSI_ENGID_I2C, FSI_VERSION_ANY },
>> { 0 }
>> @@ -224,6 +313,7 @@ static int fsi_i2c_probe(struct device *dev)
>> .name = "i2c-fsi",
>> .bus = &fsi_bus_type,
>> .probe = fsi_i2c_probe,
>> + .remove = fsi_i2c_remove,
>> },
>> };
>>
>> --
>> 1.8.3.1
>>
>
>
^ permalink raw reply
* Re: [PATCH v9 5/7] i2c: fsi: Add transfer implementation
From: Peter Rosin @ 2018-06-04 19:45 UTC (permalink / raw)
To: Eddie James, linux-i2c
Cc: linux-kernel, devicetree, wsa, robh+dt, benh, joel, mark.rutland,
gregkh, rdunlap, andy.shevchenko
In-Reply-To: <1528138850-18259-6-git-send-email-eajames@linux.vnet.ibm.com>
On 2018-06-04 21:00, Eddie James wrote:
> Execute I2C transfers from the FSI-attached I2C master. Use polling
> instead of interrupts as we have no hardware IRQ over FSI.
>
> Signed-off-by: Eddie James <eajames@linux.vnet.ibm.com>
> ---
> drivers/i2c/busses/i2c-fsi.c | 195 ++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 193 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-fsi.c b/drivers/i2c/busses/i2c-fsi.c
> index 101b9c5..4e5964e 100644
> --- a/drivers/i2c/busses/i2c-fsi.c
> +++ b/drivers/i2c/busses/i2c-fsi.c
> @@ -150,6 +150,7 @@ struct fsi_i2c_port {
> struct i2c_adapter adapter;
> struct fsi_i2c_master *master;
> u16 port;
> + u16 xfrd;
> };
>
> static int fsi_i2c_read_reg(struct fsi_device *fsi, unsigned int reg,
> @@ -225,6 +226,99 @@ static int fsi_i2c_set_port(struct fsi_i2c_port *port)
> return fsi_i2c_write_reg(fsi, I2C_FSI_RESET_ERR, &dummy);
> }
>
> +static int fsi_i2c_start(struct fsi_i2c_port *port, struct i2c_msg *msg,
> + bool stop)
> +{
> + struct fsi_i2c_master *i2c = port->master;
> + u32 cmd = I2C_CMD_WITH_START | I2C_CMD_WITH_ADDR;
> +
> + port->xfrd = 0;
> +
> + if (msg->flags & I2C_M_RD)
> + cmd |= I2C_CMD_READ;
> +
> + if (stop || msg->flags & I2C_M_STOP)
> + cmd |= I2C_CMD_WITH_STOP;
> +
> + cmd |= FIELD_PREP(I2C_CMD_ADDR, msg->addr >> 1);
> + cmd |= FIELD_PREP(I2C_CMD_LEN, msg->len);
> +
> + return fsi_i2c_write_reg(i2c->fsi, I2C_FSI_CMD, &cmd);
> +}
> +
> +static int fsi_i2c_get_op_bytes(int op_bytes)
> +{
> + /* fsi is limited to max 4 byte aligned ops */
> + if (op_bytes > 4)
> + return 4;
> + else if (op_bytes == 3)
> + return 2;
> + return op_bytes;
> +}
> +
> +static int fsi_i2c_write_fifo(struct fsi_i2c_port *port, struct i2c_msg *msg,
> + u8 fifo_count)
> +{
> + int write;
> + int rc;
> + struct fsi_i2c_master *i2c = port->master;
> + int bytes_to_write = i2c->fifo_size - fifo_count;
> + int bytes_remaining = msg->len - port->xfrd;
> +
> + bytes_to_write = min(bytes_to_write, bytes_remaining);
> +
> + while (bytes_to_write) {
> + write = fsi_i2c_get_op_bytes(bytes_to_write);
> +
> + rc = fsi_device_write(i2c->fsi, I2C_FSI_FIFO,
> + &msg->buf[port->xfrd], write);
> + if (rc)
> + return rc;
> +
> + port->xfrd += write;
> + bytes_to_write -= write;
> + }
> +
> + return 0;
> +}
> +
> +static int fsi_i2c_read_fifo(struct fsi_i2c_port *port, struct i2c_msg *msg,
> + u8 fifo_count)
> +{
> + int read;
> + int rc;
> + struct fsi_i2c_master *i2c = port->master;
> + int bytes_to_read;
> + int xfr_remaining = msg->len - port->xfrd;
> + u32 dummy;
> +
> + bytes_to_read = min_t(int, fifo_count, xfr_remaining);
> +
> + while (bytes_to_read) {
> + read = fsi_i2c_get_op_bytes(bytes_to_read);
> +
> + if (xfr_remaining) {
> + rc = fsi_device_read(i2c->fsi, I2C_FSI_FIFO,
> + &msg->buf[port->xfrd], read);
> + if (rc)
> + return rc;
> +
> + port->xfrd += read;
> + xfr_remaining -= read;
> + } else {
> + /* no more buffer but data in fifo, need to clear it */
> + rc = fsi_device_read(i2c->fsi, I2C_FSI_FIFO, &dummy,
> + read);
> + if (rc)
> + return rc;
> + }
> +
> + bytes_to_read -= read;
> + }
> +
> + return 0;
> +}
> +
> static int fsi_i2c_reset_bus(struct fsi_i2c_master *i2c)
> {
> int i, rc;
> @@ -388,17 +482,114 @@ static int fsi_i2c_abort(struct fsi_i2c_port *port, u32 status)
> return -ETIMEDOUT;
> }
>
> +static int fsi_i2c_handle_status(struct fsi_i2c_port *port,
> + struct i2c_msg *msg, u32 status)
> +{
> + int rc;
> + u8 fifo_count;
> +
> + if (status & I2C_STAT_ERR) {
> + rc = fsi_i2c_abort(port, status);
> + if (rc)
> + return rc;
> +
> + if (status & I2C_STAT_INV_CMD)
> + return -EINVAL;
> +
> + if (status & (I2C_STAT_PARITY | I2C_STAT_BE_OVERRUN |
> + I2C_STAT_BE_ACCESS))
> + return -EPROTO;
> +
> + if (status & I2C_STAT_NACK)
> + return -ENXIO;
> +
> + if (status & I2C_STAT_LOST_ARB)
> + return -EAGAIN;
> +
> + if (status & I2C_STAT_STOP_ERR)
> + return -EBADMSG;
> +
> + return -EIO;
> + }
> +
> + if (status & I2C_STAT_DAT_REQ) {
> + fifo_count = FIELD_GET(I2C_STAT_FIFO_COUNT, status);
> +
> + if (msg->flags & I2C_M_RD)
> + return fsi_i2c_read_fifo(port, msg, fifo_count);
> +
> + return fsi_i2c_write_fifo(port, msg, fifo_count);
> + }
> +
> + if (status & I2C_STAT_CMD_COMP) {
> + if (port->xfrd < msg->len)
> + return -ENODATA;
> +
> + return msg->len;
> + }
> +
> + return 0;
> +}
> +
> +static int fsi_i2c_wait(struct fsi_i2c_port *port, struct i2c_msg *msg,
> + unsigned long timeout)
> +{
> + u32 status = 0;
> + int rc;
> + unsigned long start = jiffies;
> +
> + do {
> + rc = fsi_i2c_read_reg(port->master->fsi, I2C_FSI_STAT,
> + &status);
> + if (rc)
> + return rc;
> +
> + if (status & I2C_STAT_ANY_RESP) {
> + rc = fsi_i2c_handle_status(port, msg, status);
> + if (rc < 0)
> + return rc;
> +
> + /* cmd complete and all data xfrd */
> + if (rc == msg->len)
> + return 0;
> +
> + /* need to xfr more data, but maybe don't need wait */
> + continue;
> + }
> +
> + usleep_range(I2C_CMD_SLEEP_MIN_US, I2C_CMD_SLEEP_MAX_US);
> + } while (time_after(start + timeout, jiffies));
> +
> + return -ETIMEDOUT;
> +}
> +
> static int fsi_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
> int num)
> {
> - int rc;
> + int i, rc;
> + unsigned long start_time;
> struct fsi_i2c_port *port = adap->algo_data;
> + struct i2c_msg *msg;
>
> rc = fsi_i2c_set_port(port);
> if (rc)
> return rc;
>
> - return -EOPNOTSUPP;
> + for (i = 0; i < num; i++) {
> + msg = msgs + i;
> + start_time = jiffies;
> +
> + rc = fsi_i2c_start(port, msg, i == num - 1);
> + if (rc)
> + return rc;
> +
> + rc = fsi_i2c_wait(port, msg,
> + adap->timeout - (jiffies - start_time));
> + if (rc)
> + return rc;
> + }
> +
> + return 0;
Zero is not the return value of success for .master_xfer
Hint, "num" is your friend...
Cheers,
Peter
> }
>
> static u32 fsi_i2c_functionality(struct i2c_adapter *adap)
>
^ permalink raw reply
* Re: [PATCH v3 4/6] mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver
From: Stefan Agner @ 2018-06-04 20:56 UTC (permalink / raw)
To: Randolph Maaßen
Cc: boris.brezillon, dwmw2, computersforpeace, marek.vasut, robh+dt,
mark.rutland, thierry.reding, dev, miquel.raynal, richard, marcel,
krzk, digetx, benjamin.lindqvist, jonathanh, pdeschrijver,
pgaikwad, mirza.krak, linux-mtd, linux-tegra, devicetree,
linux-kernel
In-Reply-To: <1528132590.3048.4.camel@gaireg.de>
Hi Randolph,
On 04.06.2018 19:16, Randolph Maaßen wrote:
> Am Freitag, den 01.06.2018, 00:16 +0200 schrieb Stefan Agner:
>> Add support for the NAND flash controller found on NVIDIA
>> Tegra 2 SoCs. This implementation does not make use of the
>> command queue feature. Regular operations/data transfers are
>> done in PIO mode. Page read/writes with hardware ECC make
>> use of the DMA for data transfer.
>>
>> Signed-off-by: Lucas Stach <dev@lynxeye.de>
>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> ---
>> MAINTAINERS | 7 +
>> drivers/mtd/nand/raw/Kconfig | 6 +
>> drivers/mtd/nand/raw/Makefile | 1 +
>> drivers/mtd/nand/raw/tegra_nand.c | 1143
>> +++++++++++++++++++++++++++++
>> 4 files changed, 1157 insertions(+)
>> create mode 100644 drivers/mtd/nand/raw/tegra_nand.c
>>
[...]
>> +static int tegra_nand_chips_init(struct device *dev,
>> + struct tegra_nand_controller *ctrl)
>> +{
>> + struct device_node *np = dev->of_node;
>> + struct device_node *np_nand;
>> + int nchips = of_get_child_count(np);
>> + struct tegra_nand_chip *nand;
>> + struct mtd_info *mtd;
>> + struct nand_chip *chip;
>> + unsigned long config, bch_config = 0;
>> + int bits_per_step;
>> + int ret;
>> +
>> + if (nchips != 1) {
>> + dev_err(dev, "Currently only one NAND chip
>> supported\n");
>> + return -EINVAL;
>> + }
>> +
>> + np_nand = of_get_next_child(np, NULL);
>> +
>> + nand = devm_kzalloc(dev, sizeof(*nand), GFP_KERNEL);
>> + if (!nand)
>> + return -ENOMEM;
>> +
>> + nand->wp_gpio = devm_gpiod_get_optional(dev, "wp",
>> GPIOD_OUT_LOW);
>> +
>> + if (IS_ERR(nand->wp_gpio)) {
>> + ret = PTR_ERR(nand->wp_gpio);
>> + dev_err(dev, "Failed to request WP GPIO: %d\n",
>> ret);
>> + return ret;
>> + }
>> +
>> + chip = &nand->chip;
>> + chip->controller = &ctrl->controller;
>> +
>> + mtd = nand_to_mtd(chip);
>> +
>> + mtd->dev.parent = dev;
>> + if (!mtd->name)
>> + mtd->name = "tegra_nand";
>> + mtd->owner = THIS_MODULE;
>> +
>> + nand_set_flash_node(chip, np_nand);
>
> Hi,
> i just tried this driver and it works great so far, thanks.
> I just found, that assigning the of node after setting the mtd->name
> makes it impossible to assign a name via devicetree label. I have read
> the discussion about the label on this list, so I'm curious if this is
> intentional? Setting mtd->name after nand_set_flash_node() enables the
> label parameter.
>
Hm, good catch. No that was not intentional. The name indeed should be
assigned after the call to nand_set_flash_node. Will fix this in the
next revision.
>> +
>> + chip->options = NAND_NO_SUBPAGE_WRITE |
>> NAND_USE_BOUNCE_BUFFER;
>> + chip->exec_op = tegra_nand_exec_op;
>> + chip->select_chip = tegra_nand_select_chip;
>> + chip->setup_data_interface =
>> tegra_nand_setup_data_interface;
>> +
>> + ret = nand_scan_ident(mtd, 1, NULL);
>> + if (ret)
>> + return ret;
>> +
>> + if (chip->bbt_options & NAND_BBT_USE_FLASH)
>> + chip->bbt_options |= NAND_BBT_NO_OOB;
>> +
>> + chip->ecc.mode = NAND_ECC_HW;
>> + chip->ecc.size = 512;
>> + chip->ecc.steps = mtd->writesize / chip->ecc.size;
>> + if (chip->ecc_step_ds != 512) {
>> + dev_err(dev, "Unsupported step size %d\n", chip-
>> >ecc_step_ds);
>> + return -EINVAL;
>> + }
>> +
>> + chip->ecc.read_page = tegra_nand_read_page_hwecc;
>> + chip->ecc.write_page = tegra_nand_write_page_hwecc;
>> +
>> + config = readl_relaxed(ctrl->regs + CFG);
>> + config |= CFG_PIPE_EN | CFG_SKIP_SPARE |
>> CFG_SKIP_SPARE_SIZE_4;
>> +
>> + if (chip->options & NAND_BUSWIDTH_16)
>> + config |= CFG_BUS_WIDTH_16;
>> +
>> + if (chip->ecc.algo == NAND_ECC_UNKNOWN) {
>> + if (mtd->writesize < 2048)
>> + chip->ecc.algo = NAND_ECC_RS;
>> + else
>> + chip->ecc.algo = NAND_ECC_BCH;
>> + }
>> +
>> + if (chip->ecc.algo == NAND_ECC_BCH && mtd->writesize < 2048)
>> {
>> + dev_err(dev, "BCH supportes 2K or 4K page size
>> only\n");
>> + return -EINVAL;
>> + }
>> +
>> + if (!chip->ecc.strength) {
>> + ret = tegra_nand_select_strength(chip, mtd-
>> >oobsize);
>> + if (ret < 0) {
>> + dev_err(dev, "No valid strenght found,
>> minimum %d\n",
>> + chip->ecc_strength_ds);
>> + return ret;
>> + }
>> +
>> + chip->ecc.strength = ret;
>> + }
>> +
>> + switch (chip->ecc.algo) {
>> + case NAND_ECC_RS:
>> + bits_per_step = BITS_PER_STEP_RS * chip-
>> >ecc.strength;
>> + mtd_set_ooblayout(mtd, &tegra_nand_oob_rs_ops);
>> + switch (chip->ecc.strength) {
>> + case 4:
>> + config |= CFG_ECC_SEL | CFG_TVAL_4;
>> + break;
>> + case 6:
>> + config |= CFG_ECC_SEL | CFG_TVAL_6;
>> + break;
>> + case 8:
>> + config |= CFG_ECC_SEL | CFG_TVAL_8;
>> + break;
>> + default:
>> + dev_err(dev, "ECC strength %d not
>> supported\n",
>> + chip->ecc.strength);
>> + return -EINVAL;
>> + }
>> + break;
>> + case NAND_ECC_BCH:
>> + bits_per_step = BITS_PER_STEP_BCH * chip-
>> >ecc.strength;
>> + mtd_set_ooblayout(mtd, &tegra_nand_oob_bch_ops);
>> + switch (chip->ecc.strength) {
>> + case 4:
>> + bch_config = BCH_TVAL_4;
>> + break;
>> + case 8:
>> + bch_config = BCH_TVAL_8;
>> + break;
>> + case 14:
>> + bch_config = BCH_TVAL_14;
>> + break;
>> + case 16:
>> + bch_config = BCH_TVAL_16;
>> + break;
>> + default:
>> + dev_err(dev, "ECC strength %d not
>> supported\n",
>> + chip->ecc.strength);
>> + return -EINVAL;
>> + }
>> + break;
>> + default:
>> + dev_err(dev, "ECC algorithm not supported\n");
>> + return -EINVAL;
>> + }
>> +
>> + dev_info(dev, "Using %s with strength %d per 512 byte
>> step\n",
>> + chip->ecc.algo == NAND_ECC_BCH ? "BCH" :
>> "RS",
>> + chip->ecc.strength);
>> +
>> + chip->ecc.bytes = DIV_ROUND_UP(bits_per_step,
>> BITS_PER_BYTE);
>> +
>> + switch (mtd->writesize) {
>> + case 256:
>> + config |= CFG_PS_256;
>> + break;
>> + case 512:
>> + config |= CFG_PS_512;
>> + break;
>> + case 1024:
>> + config |= CFG_PS_1024;
>> + break;
>> + case 2048:
>> + config |= CFG_PS_2048;
>> + break;
>> + case 4096:
>> + config |= CFG_PS_4096;
>> + break;
>> + default:
>> + dev_err(dev, "Unsupported writesize %d\n", mtd-
>> >writesize);
>> + return -ENODEV;
>> + }
>> +
>> + writel_relaxed(config, ctrl->regs + CFG);
>> + writel_relaxed(bch_config, ctrl->regs + BCH_CONFIG);
>> +
>> + ret = nand_scan_tail(mtd);
>> + if (ret)
>> + return ret;
>> +
>> + mtd_ooblayout_free(mtd, 0, &nand->tag);
>> +
>> + config |= CFG_TAG_BYTE_SIZE(nand->tag.length - 1);
>> + writel_relaxed(config, ctrl->regs + CFG);
>> +
>> + ret = mtd_device_register(mtd, NULL, 0);
>> + if (ret) {
>> + dev_err(dev, "Failed to register mtd device: %d\n",
>> ret);
>> + nand_cleanup(chip);
>> + return ret;
>> + }
>> +
>> + ctrl->chip = chip;
>> +
>> + return 0;
>> +}
>> +
>> +static int tegra_nand_probe(struct platform_device *pdev)
>> +{
>> + struct reset_control *rst;
>> + struct tegra_nand_controller *ctrl;
>> + struct resource *res;
>> + unsigned long reg;
>> + int irq, err = 0;
>> +
>> + ctrl = devm_kzalloc(&pdev->dev, sizeof(*ctrl), GFP_KERNEL);
>> + if (!ctrl)
>> + return -ENOMEM;
>> +
>> + ctrl->dev = &pdev->dev;
>> + nand_hw_control_init(&ctrl->controller);
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + ctrl->regs = devm_ioremap_resource(&pdev->dev, res);
>> + if (IS_ERR(ctrl->regs))
>> + return PTR_ERR(ctrl->regs);
>> +
>> + rst = devm_reset_control_get(&pdev->dev, "nand");
>> + if (IS_ERR(rst))
>> + return PTR_ERR(rst);
>> +
>> + ctrl->clk = devm_clk_get(&pdev->dev, "nand");
>> + if (IS_ERR(ctrl->clk))
>> + return PTR_ERR(ctrl->clk);
>> +
>> + err = clk_prepare_enable(ctrl->clk);
>> + if (err)
>> + return err;
>> +
>> + err = reset_control_reset(rst);
>> + if (err)
>> + goto err_disable_clk;
>> +
>> + reg = HWSTATUS_RDSTATUS_MASK(1) | HWSTATUS_RDSTATUS_VALUE(0)
>> |
>> + HWSTATUS_RBSY_MASK(NAND_STATUS_READY) |
>> + HWSTATUS_RBSY_VALUE(NAND_STATUS_READY);
>> + writel_relaxed(NAND_CMD_STATUS, ctrl->regs + HWSTATUS_CMD);
>> + writel_relaxed(reg, ctrl->regs + HWSTATUS_MASK);
>> +
>> + init_completion(&ctrl->command_complete);
>> + init_completion(&ctrl->dma_complete);
>> +
>> + /* clear interrupts */
>> + reg = readl_relaxed(ctrl->regs + ISR);
>> + writel_relaxed(reg, ctrl->regs + ISR);
>> +
>> + irq = platform_get_irq(pdev, 0);
>> + err = devm_request_irq(&pdev->dev, irq, tegra_nand_irq, 0,
>> + dev_name(&pdev->dev), ctrl);
>> + if (err)
>> + goto err_disable_clk;
>> +
>> + writel_relaxed(DMA_CTRL_IS_DONE, ctrl->regs + DMA_CTRL);
>> +
>> + /* enable interrupts */
>> + reg = IER_UND | IER_OVR | IER_CMD_DONE | IER_GIE;
>> + writel_relaxed(reg, ctrl->regs + IER);
>> +
>> + /* reset config */
>> + writel_relaxed(0, ctrl->regs + CFG);
>> +
>> + err = tegra_nand_chips_init(ctrl->dev, ctrl);
>> + if (err)
>> + goto err_disable_clk;
>> +
>> + platform_set_drvdata(pdev, ctrl);
>> +
>> + return 0;
>> +
>> +err_disable_clk:
>> + clk_disable_unprepare(ctrl->clk);
>> + return err;
>> +}
>> +
>> +static int tegra_nand_remove(struct platform_device *pdev)
>> +{
>> + struct tegra_nand_controller *ctrl =
>> platform_get_drvdata(pdev);
>> +
>> + nand_release(nand_to_mtd(ctrl->chip));
>> +
>> + clk_disable_unprepare(ctrl->clk);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id tegra_nand_of_match[] = {
>> + { .compatible = "nvidia,tegra20-nand" },
>> + { /* sentinel */ }
>> +};
>> +
>> +static struct platform_driver tegra_nand_driver = {
>> + .driver = {
>> + .name = "tegra-nand",
>> + .of_match_table = tegra_nand_of_match,
>> + },
>> + .probe = tegra_nand_probe,
>> + .remove = tegra_nand_remove,
>> +};
>> +module_platform_driver(tegra_nand_driver);
>> +
>> +MODULE_DESCRIPTION("NVIDIA Tegra NAND driver");
>> +MODULE_AUTHOR("Thierry Reding <thierry.reding@nvidia.com>");
>> +MODULE_AUTHOR("Lucas Stach <dev@lynxeye.de>");
>> +MODULE_AUTHOR("Stefan Agner <stefan@agner.ch>");
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_DEVICE_TABLE(of, tegra_nand_of_match);
>
> Sorry for any noise/mistake, I'm new to kernel development
That was constructive feedback! Thanks for pointing out!
--
Stefan
^ permalink raw reply
* Re: [PATCH] arm64: dts: stingray: move common board components to stingray-board-base
From: Florian Fainelli @ 2018-06-04 21:09 UTC (permalink / raw)
To: Scott Branden, Rob Herring, Mark Rutland, Catalin Marinas,
Will Deacon, Florian Fainelli
Cc: BCM Kernel Feedback, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <1527015342-9317-1-git-send-email-scott.branden@broadcom.com>
On 05/22/2018 11:55 AM, Scott Branden wrote:
> Move common board components from base bcm958742 dtsi file to new
> stingray-board-base dtsi file so they can be shared between many stingray
> boards following common design.
>
> Signed-off-by: Scott Branden <scott.branden@broadcom.com>
Applied to devicetree-arm64/next, though this did not apply cleanly,
please check the results at:
https://github.com/Broadcom/stblinux/commit/0b2cf5a855cd235fa95fbdfedfc524a97a71a7fe
> ---
> .../boot/dts/broadcom/stingray/bcm958742-base.dtsi | 35 +--------------
> .../dts/broadcom/stingray/stingray-board-base.dtsi | 51 ++++++++++++++++++++++
> 2 files changed, 52 insertions(+), 34 deletions(-)
> create mode 100644 arch/arm64/boot/dts/broadcom/stingray/stingray-board-base.dtsi
>
> diff --git a/arch/arm64/boot/dts/broadcom/stingray/bcm958742-base.dtsi b/arch/arm64/boot/dts/broadcom/stingray/bcm958742-base.dtsi
> index cacc25e..d74f6df 100644
> --- a/arch/arm64/boot/dts/broadcom/stingray/bcm958742-base.dtsi
> +++ b/arch/arm64/boot/dts/broadcom/stingray/bcm958742-base.dtsi
> @@ -30,20 +30,9 @@
> * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> */
>
> -#include "stingray.dtsi"
> +#include "stingray-board-base.dtsi"
>
> / {
> - chosen {
> - stdout-path = "serial0:115200n8";
> - };
> -
> - aliases {
> - serial0 = &uart1;
> - serial1 = &uart0;
> - serial2 = &uart2;
> - serial3 = &uart3;
> - };
> -
> sdio0_vddo_ctrl_reg: sdio0_vddo_ctrl {
> compatible = "regulator-gpio";
> regulator-name = "sdio0_vddo_ctrl_reg";
> @@ -67,23 +56,6 @@
> };
> };
>
> -&memory { /* Default DRAM banks */
> - reg = <0x00000000 0x80000000 0x0 0x80000000>, /* 2G @ 2G */
> - <0x00000008 0x80000000 0x1 0x80000000>; /* 6G @ 34G */
> -};
> -
> -&mdio_mux_iproc {
> - mdio@10 {
> - gphy0: eth-phy@10 {
> - reg = <0x10>;
> - };
> - };
> -};
> -
> -&uart1 {
> - status = "okay";
> -};
> -
> &pwm {
> status = "okay";
> };
> @@ -111,8 +83,6 @@
> };
>
> &enet {
> - phy-mode = "rgmii-id";
> - phy-handle = <&gphy0>;
> status = "okay";
> };
>
> @@ -133,13 +103,10 @@
>
> &sdio0 {
> vqmmc-supply = <&sdio0_vddo_ctrl_reg>;
> - non-removable;
> - full-pwr-cycle;
> status = "okay";
> };
>
> &sdio1 {
> vqmmc-supply = <&sdio1_vddo_ctrl_reg>;
> - full-pwr-cycle;
> status = "okay";
> };
> diff --git a/arch/arm64/boot/dts/broadcom/stingray/stingray-board-base.dtsi b/arch/arm64/boot/dts/broadcom/stingray/stingray-board-base.dtsi
> new file mode 100644
> index 0000000..82a2471
> --- /dev/null
> +++ b/arch/arm64/boot/dts/broadcom/stingray/stingray-board-base.dtsi
> @@ -0,0 +1,51 @@
> +// SPDX-License-Identifier: (GPL-2.0 or BSD-3-Clause)
> +/*
> + * Copyright(c) 2016-2018 Broadcom
> + */
> +
> +#include "stingray.dtsi"
> +#include <dt-bindings/gpio/gpio.h>
> +
> +/ {
> + aliases {
> + serial0 = &uart1;
> + serial1 = &uart0;
> + serial2 = &uart2;
> + serial3 = &uart3;
> + };
> +
> + chosen {
> + stdout-path = "serial0:115200n8";
> + };
> +};
> +
> +&memory { /* Default DRAM banks */
> + reg = <0x00000000 0x80000000 0x0 0x80000000>, /* 2G @ 2G */
> + <0x00000008 0x80000000 0x1 0x80000000>; /* 6G @ 34G */
> +};
> +
> +&enet {
> + phy-mode = "rgmii-id";
> + phy-handle = <&gphy0>;
> +};
> +
> +&uart1 {
> + status = "okay";
> +};
> +
> +&sdio0 {
> + non-removable;
> + full-pwr-cycle;
> +};
> +
> +&sdio1 {
> + full-pwr-cycle;
> +};
> +
> +&mdio_mux_iproc {
> + mdio@10 {
> + gphy0: eth-phy@10 {
> + reg = <0x10>;
> + };
> + };
> +};
>
--
Florian
^ permalink raw reply
* Re: [PATCH v9 2/7] i2c: Add FSI-attached I2C master algorithm
From: Eddie James @ 2018-06-04 21:11 UTC (permalink / raw)
To: Andy Shevchenko
Cc: linux-i2c, Linux Kernel Mailing List, devicetree, Wolfram Sang,
Rob Herring, Benjamin Herrenschmidt, Joel Stanley, Mark Rutland,
Greg Kroah-Hartman, Randy Dunlap
In-Reply-To: <CAHp75Vd7NvDFmWSzNDz-tgGvMVyXU7Z16LgGCWnHkY-3kE7fwg@mail.gmail.com>
On 06/04/2018 02:21 PM, Andy Shevchenko wrote:
> On Mon, Jun 4, 2018 at 10:00 PM, Eddie James <eajames@linux.vnet.ibm.com> wrote:
>> Add register definitions for FSI-attached I2C master and functions to
>> access those registers over FSI. Add an FSI driver so that our I2C bus
>> is probed up during an FSI scan.
>>
>> Signed-off-by: Eddie James <eajames@linux.vnet.ibm.com>
>> ---
>> drivers/i2c/busses/Kconfig | 11 ++
>> drivers/i2c/busses/Makefile | 1 +
>> drivers/i2c/busses/i2c-fsi.c | 234 +++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 246 insertions(+)
>> create mode 100644 drivers/i2c/busses/i2c-fsi.c
>>
>> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
>> index 4f8df2e..cddd159 100644
>> --- a/drivers/i2c/busses/Kconfig
>> +++ b/drivers/i2c/busses/Kconfig
>> @@ -1330,4 +1330,15 @@ config I2C_ZX2967
>> This driver can also be built as a module. If so, the module will be
>> called i2c-zx2967.
>>
>> +config I2C_FSI
>> + tristate "FSI I2C driver"
>> + depends on FSI
>> + help
>> + Driver for FSI bus attached I2C masters. These are I2C masters that
>> + are connected to the system over an FSI bus, instead of the more
>> + common PCI or MMIO interface.
>> +
>> + This driver can also be built as a module. If so, the module will be
>> + called as i2c-fsi.
>> +
>> endmenu
>> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
>> index 5a86914..4909fd6 100644
>> --- a/drivers/i2c/busses/Makefile
>> +++ b/drivers/i2c/busses/Makefile
>> @@ -137,5 +137,6 @@ obj-$(CONFIG_I2C_PCA_ISA) += i2c-pca-isa.o
>> obj-$(CONFIG_I2C_SIBYTE) += i2c-sibyte.o
>> obj-$(CONFIG_I2C_XGENE_SLIMPRO) += i2c-xgene-slimpro.o
>> obj-$(CONFIG_SCx200_ACB) += scx200_acb.o
>> +obj-$(CONFIG_I2C_FSI) += i2c-fsi.o
>>
>> ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG
>> diff --git a/drivers/i2c/busses/i2c-fsi.c b/drivers/i2c/busses/i2c-fsi.c
>> new file mode 100644
>> index 0000000..e1b183c
>> --- /dev/null
>> +++ b/drivers/i2c/busses/i2c-fsi.c
>> @@ -0,0 +1,234 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * FSI-attached I2C master algorithm
>> + *
>> + * Copyright 2018 IBM Corporation
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * as published by the Free Software Foundation; either version
>> + * 2 of the License, or (at your option) any later version.
>> + */
>> +
>> +#include <linux/bitfield.h>
>> +#include <linux/bitops.h>
>> +#include <linux/device.h>
>> +#include <linux/errno.h>
>> +#include <linux/fsi.h>
>> +#include <linux/i2c.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +
>> +#define FSI_ENGID_I2C 0x7
>> +
>> +#define I2C_DEFAULT_CLK_DIV 6
>> +
>> +/* i2c registers */
>> +#define I2C_FSI_FIFO 0x00
>> +#define I2C_FSI_CMD 0x04
>> +#define I2C_FSI_MODE 0x08
>> +#define I2C_FSI_WATER_MARK 0x0C
>> +#define I2C_FSI_INT_MASK 0x10
>> +#define I2C_FSI_INT_COND 0x14
>> +#define I2C_FSI_OR_INT_MASK 0x14
>> +#define I2C_FSI_INTS 0x18
>> +#define I2C_FSI_AND_INT_MASK 0x18
>> +#define I2C_FSI_STAT 0x1C
>> +#define I2C_FSI_RESET_I2C 0x1C
>> +#define I2C_FSI_ESTAT 0x20
>> +#define I2C_FSI_RESET_ERR 0x20
>> +#define I2C_FSI_RESID_LEN 0x24
>> +#define I2C_FSI_SET_SCL 0x24
>> +#define I2C_FSI_PORT_BUSY 0x28
>> +#define I2C_FSI_RESET_SCL 0x2C
>> +#define I2C_FSI_SET_SDA 0x30
>> +#define I2C_FSI_RESET_SDA 0x34
>> +
>> +/* cmd register */
>> +#define I2C_CMD_WITH_START BIT(31)
>> +#define I2C_CMD_WITH_ADDR BIT(30)
>> +#define I2C_CMD_RD_CONT BIT(29)
>> +#define I2C_CMD_WITH_STOP BIT(28)
>> +#define I2C_CMD_FORCELAUNCH BIT(27)
>> +#define I2C_CMD_ADDR GENMASK(23, 17)
>> +#define I2C_CMD_READ BIT(16)
>> +#define I2C_CMD_LEN GENMASK(15, 0)
>> +
>> +/* mode register */
>> +#define I2C_MODE_CLKDIV GENMASK(31, 16)
>> +#define I2C_MODE_PORT GENMASK(15, 10)
>> +#define I2C_MODE_ENHANCED BIT(3)
>> +#define I2C_MODE_DIAG BIT(2)
>> +#define I2C_MODE_PACE_ALLOW BIT(1)
>> +#define I2C_MODE_WRAP BIT(0)
>> +
>> +/* watermark register */
>> +#define I2C_WATERMARK_HI GENMASK(15, 12)
>> +#define I2C_WATERMARK_LO GENMASK(7, 4)
>> +
>> +#define I2C_FIFO_HI_LVL 4
>> +#define I2C_FIFO_LO_LVL 4
>> +
>> +/* interrupt register */
>> +#define I2C_INT_INV_CMD BIT(15)
>> +#define I2C_INT_PARITY BIT(14)
>> +#define I2C_INT_BE_OVERRUN BIT(13)
>> +#define I2C_INT_BE_ACCESS BIT(12)
>> +#define I2C_INT_LOST_ARB BIT(11)
>> +#define I2C_INT_NACK BIT(10)
>> +#define I2C_INT_DAT_REQ BIT(9)
>> +#define I2C_INT_CMD_COMP BIT(8)
>> +#define I2C_INT_STOP_ERR BIT(7)
>> +#define I2C_INT_BUSY BIT(6)
>> +#define I2C_INT_IDLE BIT(5)
>> +
>> +#define I2C_INT_ENABLE 0x0000ff80
>> +#define I2C_INT_ERR 0x0000fcc0
> Now it looks like a flags combinations.
> For me as for reader would be better to see quickly a decoded line.
>
> My proposal is to introduce something like following
>
> _INT_ALL GENMASK()
> _INT_ENABLE (_INT_ALL & ~(_FOO | _BAR))
> _INT_ERR ... similar way as above ...
>
> What do you think?
Yes I considered that. I do prefer the simplicity of just writing the
value directly. Doing GENMASK and then AND-ing out certain bits isn't
that much more understandable, in my opinion. To understand each bit
you'd still have to check the GENMASK against the bit fields defined
above. And just OR-ing together all the bits would be very lengthy for
such an expression. Up to you, I will follow your suggestion if you want.
>
>> +
>> +/* status register */
>> +#define I2C_STAT_INV_CMD BIT(31)
>> +#define I2C_STAT_PARITY BIT(30)
>> +#define I2C_STAT_BE_OVERRUN BIT(29)
>> +#define I2C_STAT_BE_ACCESS BIT(28)
>> +#define I2C_STAT_LOST_ARB BIT(27)
>> +#define I2C_STAT_NACK BIT(26)
>> +#define I2C_STAT_DAT_REQ BIT(25)
>> +#define I2C_STAT_CMD_COMP BIT(24)
>> +#define I2C_STAT_STOP_ERR BIT(23)
>> +#define I2C_STAT_MAX_PORT GENMASK(19, 16)
>> +#define I2C_STAT_ANY_INT BIT(15)
>> +#define I2C_STAT_SCL_IN BIT(11)
>> +#define I2C_STAT_SDA_IN BIT(10)
>> +#define I2C_STAT_PORT_BUSY BIT(9)
>> +#define I2C_STAT_SELF_BUSY BIT(8)
>> +#define I2C_STAT_FIFO_COUNT GENMASK(7, 0)
>> +
>> +#define I2C_STAT_ERR 0xfc800000
>> +#define I2C_STAT_ANY_RESP 0xff800000
> Ditto.
>
>> +
>> +/* extended status register */
>> +#define I2C_ESTAT_FIFO_SZ GENMASK(31, 24)
>> +#define I2C_ESTAT_SCL_IN_SY BIT(15)
>> +#define I2C_ESTAT_SDA_IN_SY BIT(14)
>> +#define I2C_ESTAT_S_SCL BIT(13)
>> +#define I2C_ESTAT_S_SDA BIT(12)
>> +#define I2C_ESTAT_M_SCL BIT(11)
>> +#define I2C_ESTAT_M_SDA BIT(10)
>> +#define I2C_ESTAT_HI_WATER BIT(9)
>> +#define I2C_ESTAT_LO_WATER BIT(8)
>> +#define I2C_ESTAT_PORT_BUSY BIT(7)
>> +#define I2C_ESTAT_SELF_BUSY BIT(6)
>> +#define I2C_ESTAT_VERSION GENMASK(4, 0)
>> +
>> +struct fsi_i2c_master {
>> + struct fsi_device *fsi;
>> + u8 fifo_size;
>> +};
>> +
>> +static int fsi_i2c_read_reg(struct fsi_device *fsi, unsigned int reg,
>> + u32 *data)
>> +{
>> + int rc;
>> + __be32 data_be;
>> +
>> + rc = fsi_device_read(fsi, reg, &data_be, sizeof(data_be));
>> + if (rc)
>> + return rc;
>> +
>> + *data = be32_to_cpu(data_be);
>> +
>> + return 0;
>> +}
>> +
>> +static int fsi_i2c_write_reg(struct fsi_device *fsi, unsigned int reg,
>> + u32 *data)
>> +{
>> + __be32 data_be = cpu_to_be32p(data);
>> +
>> + return fsi_device_write(fsi, reg, &data_be, sizeof(data_be));
>> +}
>> +
>> +static int fsi_i2c_dev_init(struct fsi_i2c_master *i2c)
>> +{
>> + int rc;
>> + u32 mode = I2C_MODE_ENHANCED, extended_status, watermark;
>> + u32 interrupt = 0;
>> +
>> + /* since we use polling, disable interrupts */
>> + rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_INT_MASK, &interrupt);
>> + if (rc)
>> + return rc;
>> +
>> + mode |= FIELD_PREP(I2C_MODE_CLKDIV, I2C_DEFAULT_CLK_DIV);
>> + rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_MODE, &mode);
>> + if (rc)
>> + return rc;
>> +
>> + rc = fsi_i2c_read_reg(i2c->fsi, I2C_FSI_ESTAT, &extended_status);
>> + if (rc)
>> + return rc;
>> +
>> + i2c->fifo_size = FIELD_GET(I2C_ESTAT_FIFO_SZ, extended_status);
>> + watermark = FIELD_PREP(I2C_WATERMARK_HI,
>> + i2c->fifo_size - I2C_FIFO_HI_LVL);
>> + watermark |= FIELD_PREP(I2C_WATERMARK_LO, I2C_FIFO_LO_LVL);
>> +
>> + return fsi_i2c_write_reg(i2c->fsi, I2C_FSI_WATER_MARK, &watermark);
>> +}
>> +
>> +static int fsi_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
>> + int num)
>> +{
>> + return -EOPNOTSUPP;
>> +}
>> +
>> +static u32 fsi_i2c_functionality(struct i2c_adapter *adap)
>> +{
>> + return I2C_FUNC_I2C | I2C_FUNC_PROTOCOL_MANGLING | I2C_FUNC_10BIT_ADDR
>> + | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_SMBUS_BLOCK_DATA;
> Perhaps one property per line?
At a quick glance through some other I2C bus drivers, it appears more
standard to fit as many on a line as possible. I'll leave it as is
unless anyone objects. I didn't get any checkpatch warning.
Thanks,
Eddie
>
>> +}
>> +
>> +static const struct i2c_algorithm fsi_i2c_algorithm = {
>> + .master_xfer = fsi_i2c_xfer,
>> + .functionality = fsi_i2c_functionality,
>> +};
>> +
>> +static int fsi_i2c_probe(struct device *dev)
>> +{
>> + struct fsi_i2c_master *i2c;
>> + int rc;
>> +
>> + i2c = devm_kzalloc(dev, sizeof(*i2c), GFP_KERNEL);
>> + if (!i2c)
>> + return -ENOMEM;
>> +
>> + i2c->fsi = to_fsi_dev(dev);
>> +
>> + rc = fsi_i2c_dev_init(i2c);
>> + if (rc)
>> + return rc;
>> +
>> + dev_set_drvdata(dev, i2c);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct fsi_device_id fsi_i2c_ids[] = {
>> + { FSI_ENGID_I2C, FSI_VERSION_ANY },
>> + { 0 }
> 0 is not required
>
>> +};
>> +
>> +static struct fsi_driver fsi_i2c_driver = {
>> + .id_table = fsi_i2c_ids,
>> + .drv = {
>> + .name = "i2c-fsi",
>> + .bus = &fsi_bus_type,
>> + .probe = fsi_i2c_probe,
>> + },
>> +};
>> +
>> +module_fsi_driver(fsi_i2c_driver);
>> +
>> +MODULE_AUTHOR("Eddie James <eajames@us.ibm.com>");
>> +MODULE_DESCRIPTION("FSI attached I2C master");
>> +MODULE_LICENSE("GPL");
>> --
>> 1.8.3.1
>>
>
>
^ permalink raw reply
* Re: [PATCH v3 3/3] arm64: dts: Update Stingray clock DT nodes
From: Florian Fainelli @ 2018-06-04 21:11 UTC (permalink / raw)
To: Ray Jui, Michael Turquette, Stephen Boyd, Rob Herring,
Mark Rutland
Cc: linux-clk, linux-kernel, bcm-kernel-feedback-list, devicetree,
linux-arm-kernel, Pramod Kumar
In-Reply-To: <1527900968-12017-4-git-send-email-ray.jui@broadcom.com>
On 06/01/2018 05:56 PM, Ray Jui wrote:
> From: Pramod Kumar <pramod.kumar@broadcom.com>
>
> Update clock output names in the Stingray clock DT nodes so they match
> the binding document and the latest ASIC datasheet. Also add entries
> for LCPLL2
>
> Signed-off-by: Pramod Kumar <pramod.kumar@broadcom.com>
> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
Applied to devicetree-arm64/next, thanks!
--
Florian
^ permalink raw reply
* Re: [PATCH v9 5/7] i2c: fsi: Add transfer implementation
From: Eddie James @ 2018-06-04 21:12 UTC (permalink / raw)
To: Peter Rosin, linux-i2c
Cc: linux-kernel, devicetree, wsa, robh+dt, benh, joel, mark.rutland,
gregkh, rdunlap, andy.shevchenko
In-Reply-To: <92b61473-b5bb-ea2e-56ce-5375641c5242@axentia.se>
On 06/04/2018 02:45 PM, Peter Rosin wrote:
> On 2018-06-04 21:00, Eddie James wrote:
>> Execute I2C transfers from the FSI-attached I2C master. Use polling
>> instead of interrupts as we have no hardware IRQ over FSI.
>>
>> Signed-off-by: Eddie James <eajames@linux.vnet.ibm.com>
>> ---
>> drivers/i2c/busses/i2c-fsi.c | 195 ++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 193 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-fsi.c b/drivers/i2c/busses/i2c-fsi.c
>> index 101b9c5..4e5964e 100644
>> --- a/drivers/i2c/busses/i2c-fsi.c
>> +++ b/drivers/i2c/busses/i2c-fsi.c
>> @@ -150,6 +150,7 @@ struct fsi_i2c_port {
>> struct i2c_adapter adapter;
>> struct fsi_i2c_master *master;
>> u16 port;
>> + u16 xfrd;
>> };
>>
>> static int fsi_i2c_read_reg(struct fsi_device *fsi, unsigned int reg,
>> @@ -225,6 +226,99 @@ static int fsi_i2c_set_port(struct fsi_i2c_port *port)
>> return fsi_i2c_write_reg(fsi, I2C_FSI_RESET_ERR, &dummy);
>> }
>>
>> +static int fsi_i2c_start(struct fsi_i2c_port *port, struct i2c_msg *msg,
>> + bool stop)
>> +{
>> + struct fsi_i2c_master *i2c = port->master;
>> + u32 cmd = I2C_CMD_WITH_START | I2C_CMD_WITH_ADDR;
>> +
>> + port->xfrd = 0;
>> +
>> + if (msg->flags & I2C_M_RD)
>> + cmd |= I2C_CMD_READ;
>> +
>> + if (stop || msg->flags & I2C_M_STOP)
>> + cmd |= I2C_CMD_WITH_STOP;
>> +
>> + cmd |= FIELD_PREP(I2C_CMD_ADDR, msg->addr >> 1);
>> + cmd |= FIELD_PREP(I2C_CMD_LEN, msg->len);
>> +
>> + return fsi_i2c_write_reg(i2c->fsi, I2C_FSI_CMD, &cmd);
>> +}
>> +
>> +static int fsi_i2c_get_op_bytes(int op_bytes)
>> +{
>> + /* fsi is limited to max 4 byte aligned ops */
>> + if (op_bytes > 4)
>> + return 4;
>> + else if (op_bytes == 3)
>> + return 2;
>> + return op_bytes;
>> +}
>> +
>> +static int fsi_i2c_write_fifo(struct fsi_i2c_port *port, struct i2c_msg *msg,
>> + u8 fifo_count)
>> +{
>> + int write;
>> + int rc;
>> + struct fsi_i2c_master *i2c = port->master;
>> + int bytes_to_write = i2c->fifo_size - fifo_count;
>> + int bytes_remaining = msg->len - port->xfrd;
>> +
>> + bytes_to_write = min(bytes_to_write, bytes_remaining);
>> +
>> + while (bytes_to_write) {
>> + write = fsi_i2c_get_op_bytes(bytes_to_write);
>> +
>> + rc = fsi_device_write(i2c->fsi, I2C_FSI_FIFO,
>> + &msg->buf[port->xfrd], write);
>> + if (rc)
>> + return rc;
>> +
>> + port->xfrd += write;
>> + bytes_to_write -= write;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int fsi_i2c_read_fifo(struct fsi_i2c_port *port, struct i2c_msg *msg,
>> + u8 fifo_count)
>> +{
>> + int read;
>> + int rc;
>> + struct fsi_i2c_master *i2c = port->master;
>> + int bytes_to_read;
>> + int xfr_remaining = msg->len - port->xfrd;
>> + u32 dummy;
>> +
>> + bytes_to_read = min_t(int, fifo_count, xfr_remaining);
>> +
>> + while (bytes_to_read) {
>> + read = fsi_i2c_get_op_bytes(bytes_to_read);
>> +
>> + if (xfr_remaining) {
>> + rc = fsi_device_read(i2c->fsi, I2C_FSI_FIFO,
>> + &msg->buf[port->xfrd], read);
>> + if (rc)
>> + return rc;
>> +
>> + port->xfrd += read;
>> + xfr_remaining -= read;
>> + } else {
>> + /* no more buffer but data in fifo, need to clear it */
>> + rc = fsi_device_read(i2c->fsi, I2C_FSI_FIFO, &dummy,
>> + read);
>> + if (rc)
>> + return rc;
>> + }
>> +
>> + bytes_to_read -= read;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> static int fsi_i2c_reset_bus(struct fsi_i2c_master *i2c)
>> {
>> int i, rc;
>> @@ -388,17 +482,114 @@ static int fsi_i2c_abort(struct fsi_i2c_port *port, u32 status)
>> return -ETIMEDOUT;
>> }
>>
>> +static int fsi_i2c_handle_status(struct fsi_i2c_port *port,
>> + struct i2c_msg *msg, u32 status)
>> +{
>> + int rc;
>> + u8 fifo_count;
>> +
>> + if (status & I2C_STAT_ERR) {
>> + rc = fsi_i2c_abort(port, status);
>> + if (rc)
>> + return rc;
>> +
>> + if (status & I2C_STAT_INV_CMD)
>> + return -EINVAL;
>> +
>> + if (status & (I2C_STAT_PARITY | I2C_STAT_BE_OVERRUN |
>> + I2C_STAT_BE_ACCESS))
>> + return -EPROTO;
>> +
>> + if (status & I2C_STAT_NACK)
>> + return -ENXIO;
>> +
>> + if (status & I2C_STAT_LOST_ARB)
>> + return -EAGAIN;
>> +
>> + if (status & I2C_STAT_STOP_ERR)
>> + return -EBADMSG;
>> +
>> + return -EIO;
>> + }
>> +
>> + if (status & I2C_STAT_DAT_REQ) {
>> + fifo_count = FIELD_GET(I2C_STAT_FIFO_COUNT, status);
>> +
>> + if (msg->flags & I2C_M_RD)
>> + return fsi_i2c_read_fifo(port, msg, fifo_count);
>> +
>> + return fsi_i2c_write_fifo(port, msg, fifo_count);
>> + }
>> +
>> + if (status & I2C_STAT_CMD_COMP) {
>> + if (port->xfrd < msg->len)
>> + return -ENODATA;
>> +
>> + return msg->len;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int fsi_i2c_wait(struct fsi_i2c_port *port, struct i2c_msg *msg,
>> + unsigned long timeout)
>> +{
>> + u32 status = 0;
>> + int rc;
>> + unsigned long start = jiffies;
>> +
>> + do {
>> + rc = fsi_i2c_read_reg(port->master->fsi, I2C_FSI_STAT,
>> + &status);
>> + if (rc)
>> + return rc;
>> +
>> + if (status & I2C_STAT_ANY_RESP) {
>> + rc = fsi_i2c_handle_status(port, msg, status);
>> + if (rc < 0)
>> + return rc;
>> +
>> + /* cmd complete and all data xfrd */
>> + if (rc == msg->len)
>> + return 0;
>> +
>> + /* need to xfr more data, but maybe don't need wait */
>> + continue;
>> + }
>> +
>> + usleep_range(I2C_CMD_SLEEP_MIN_US, I2C_CMD_SLEEP_MAX_US);
>> + } while (time_after(start + timeout, jiffies));
>> +
>> + return -ETIMEDOUT;
>> +}
>> +
>> static int fsi_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
>> int num)
>> {
>> - int rc;
>> + int i, rc;
>> + unsigned long start_time;
>> struct fsi_i2c_port *port = adap->algo_data;
>> + struct i2c_msg *msg;
>>
>> rc = fsi_i2c_set_port(port);
>> if (rc)
>> return rc;
>>
>> - return -EOPNOTSUPP;
>> + for (i = 0; i < num; i++) {
>> + msg = msgs + i;
>> + start_time = jiffies;
>> +
>> + rc = fsi_i2c_start(port, msg, i == num - 1);
>> + if (rc)
>> + return rc;
>> +
>> + rc = fsi_i2c_wait(port, msg,
>> + adap->timeout - (jiffies - start_time));
>> + if (rc)
>> + return rc;
>> + }
>> +
>> + return 0;
> Zero is not the return value of success for .master_xfer
>
> Hint, "num" is your friend...
>
> Cheers,
> Peter
Good catch, I'll fix that.
Thanks,
Eddie
>
>> }
>>
>> static u32 fsi_i2c_functionality(struct i2c_adapter *adap)
>>
^ permalink raw reply
* [PATCH] regulator: tps65911: fix regulator-compatible documentation
From: Christoph Fritz @ 2018-06-04 21:14 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Tony Lindgren; +Cc: devicetree, linux-omap
Signed-off-by: Christoph Fritz <chf.fritz@googlemail.com>
---
Documentation/devicetree/bindings/mfd/tps65910.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/mfd/tps65910.txt b/Documentation/devicetree/bindings/mfd/tps65910.txt
index 8af1202..4f62143 100644
--- a/Documentation/devicetree/bindings/mfd/tps65910.txt
+++ b/Documentation/devicetree/bindings/mfd/tps65910.txt
@@ -22,7 +22,7 @@ Required properties:
The valid regulator-compatible values are:
tps65910: vrtc, vio, vdd1, vdd2, vdd3, vdig1, vdig2, vpll, vdac, vaux1,
vaux2, vaux33, vmmc, vbb
- tps65911: vrtc, vio, vdd1, vdd3, vddctrl, ldo1, ldo2, ldo3, ldo4, ldo5,
+ tps65911: vrtc, vio, vdd1, vdd2, vddctrl, ldo1, ldo2, ldo3, ldo4, ldo5,
ldo6, ldo7, ldo8
- xxx-supply: Input voltage supply regulator.
--
2.1.4
^ permalink raw reply related
* Re: [PATCH 1/2] soc: bcm: brcmstb: pm: Add support for newer rev B3.0 controllers
From: Florian Fainelli @ 2018-06-04 21:15 UTC (permalink / raw)
To: bcm-kernel-feedback-list, linux-arm-kernel
Cc: Doug Berger, Rob Herring, Mark Rutland, Brian Norris,
Gregory Fong, Justin Chen,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
In-Reply-To: <20180511220242.837-2-f.fainelli@gmail.com>
On Fri, 11 May 2018 15:02:41 -0700, Florian Fainelli <f.fainelli@gmail.com> wrote:
> From: Doug Berger <opendmb@gmail.com>
>
> Update the Device Tree binding document and add a matching entry for the
> MEMC DDR controller revision B3.0 which is found on chips like 7278A0
> and newer.
>
> Signed-off-by: Doug Berger <opendmb@gmail.com>
> [florian: tweak commit message, make it apply to upstream kernel]
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
Applied to drivers/next, thanks!
--
Florian
^ permalink raw reply
* Re: [PATCH 2/2] soc: bcm: brcmstb: Add missing DDR MEMC compatible strings
From: Florian Fainelli @ 2018-06-04 21:15 UTC (permalink / raw)
To: bcm-kernel-feedback-list, linux-arm-kernel
Cc: Rob Herring, Mark Rutland, Brian Norris, Gregory Fong,
Doug Berger, Justin Chen,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
In-Reply-To: <20180511220242.837-3-f.fainelli@gmail.com>
On Fri, 11 May 2018 15:02:42 -0700, Florian Fainelli <f.fainelli@gmail.com> wrote:
> We would not be matching the following chip/compatible strings
> combinations, which would lead to not setting the warm boot flag
> correctly, fix that:
>
> 7260A0/B0: brcm,brcmstb-memc-ddr-rev-b.2.1
> 7255A0: brcm,brcmstb-memc-ddr-rev-b.2.3
> 7278Bx: brcm,brcmstb-memc-ddr-rev-b.3.1
>
> The B2.1 core (which is in 7260 A0 and B0) doesn't have the
> SHIMPHY_ADDR_CNTL_0_DDR_PAD_CNTRL setup in the memsys init code, nor
> does it have the warm boot flag re-definition on entry. Those changes
> were for B2.2 and later MEMSYS cores. Fall back to the previous S2/S3
> entry method for these specific chips.
>
> Fixes: 0b741b8234c8 ("soc: bcm: brcmstb: Add support for S2/S3/S5 suspend states (ARM)")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
Applied to drivers/next, thanks!
--
Florian
^ permalink raw reply
* Re: [PATCH 1/2] soc: bcm: brcmstb: pm: Add support for newer rev B3.0 controllers
From: Florian Fainelli @ 2018-06-04 21:16 UTC (permalink / raw)
To: Rob Herring, Florian Fainelli
Cc: linux-arm-kernel, Doug Berger, Mark Rutland, Brian Norris,
Gregory Fong, maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
Justin Chen,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
In-Reply-To: <20180522223024.GA7817@rob-hp-laptop>
On 05/22/2018 03:30 PM, Rob Herring wrote:
> On Fri, May 11, 2018 at 03:02:41PM -0700, Florian Fainelli wrote:
>> From: Doug Berger <opendmb@gmail.com>
>>
>> Update the Device Tree binding document and add a matching entry for the
>> MEMC DDR controller revision B3.0 which is found on chips like 7278A0
>> and newer.
>>
>> Signed-off-by: Doug Berger <opendmb@gmail.com>
>> [florian: tweak commit message, make it apply to upstream kernel]
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>> Documentation/devicetree/bindings/arm/bcm/brcm,brcmstb.txt | 1 +
>> drivers/soc/bcm/brcmstb/pm/pm-arm.c | 4 ++++
>> 2 files changed, 5 insertions(+)
>
> Reviewed-by: Rob Herring <robh@kernel.org>
>
> Side note: this should really move out of bindings/arm/ to
> bindings/memory-controllers/ or at least to its own file.
Good idea, thanks, will do that.
--
--
Florian
^ permalink raw reply
* Re: [PATCH] arm64: dts: stingray: Add otp device node
From: Florian Fainelli @ 2018-06-04 21:24 UTC (permalink / raw)
To: Scott Branden, Rob Herring, Mark Rutland, Catalin Marinas,
Will Deacon, Florian Fainelli
Cc: BCM Kernel Feedback, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <1527106630-4760-1-git-send-email-scott.branden@broadcom.com>
On 05/23/2018 01:17 PM, Scott Branden wrote:
> Add otp device node for Stingray SOC.
>
> Fixes: 2fa9e9e29ea2 ("arm64: dts: Add GPIO DT nodes for Stingray SOC")
> Signed-off-by: Scott Branden <scott.branden@broadcom.com>
Applied to devicetree-arm64/next with s/otp/OTP/ and removing the Fixes
line since that is not a bug fix AFAICT.
Thank you
--
Florian
^ permalink raw reply
* Re: [PATCH] arm64: dts: stingray: Add otp device node
From: Scott Branden @ 2018-06-04 21:30 UTC (permalink / raw)
To: Florian Fainelli, Rob Herring, Mark Rutland, Catalin Marinas,
Will Deacon
Cc: BCM Kernel Feedback, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <c9219029-6e45-1889-6326-470d2bc9e46c@gmail.com>
Hi Florian,
On 18-06-04 02:24 PM, Florian Fainelli wrote:
> On 05/23/2018 01:17 PM, Scott Branden wrote:
>> Add otp device node for Stingray SOC.
>>
>> Fixes: 2fa9e9e29ea2 ("arm64: dts: Add GPIO DT nodes for Stingray SOC")
>> Signed-off-by: Scott Branden <scott.branden@broadcom.com>
> Applied to devicetree-arm64/next with s/otp/OTP/ and removing the Fixes
> line since that is not a bug fix AFAICT.
It fixes the issue that OTP is not active as it is missing the device node?
>
> Thank you
^ permalink raw reply
* Re: [PATCH] arm64: dts: stingray: Add otp device node
From: Florian Fainelli @ 2018-06-04 21:33 UTC (permalink / raw)
To: Scott Branden, Florian Fainelli, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon
Cc: BCM Kernel Feedback, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <f6f3e76e-eac9-797d-fea5-eac64327ec7f@broadcom.com>
On 06/04/2018 02:30 PM, Scott Branden wrote:
> Hi Florian,
>
>
> On 18-06-04 02:24 PM, Florian Fainelli wrote:
>> On 05/23/2018 01:17 PM, Scott Branden wrote:
>>> Add otp device node for Stingray SOC.
>>>
>>> Fixes: 2fa9e9e29ea2 ("arm64: dts: Add GPIO DT nodes for Stingray SOC")
>>> Signed-off-by: Scott Branden <scott.branden@broadcom.com>
>> Applied to devicetree-arm64/next with s/otp/OTP/ and removing the Fixes
>> line since that is not a bug fix AFAICT.
> It fixes the issue that OTP is not active as it is missing the device node?
By that token, any peripheral that is being added at some point in the
lifetime of this DTS would qualify as a bugfix when it is in fact
feature/peripheral enabling.
I could not see the relationship between the commit being provided in
the "Fixes:" tag and OTP, am I missing something?
--
Florian
^ permalink raw reply
* Re: [PATCH 1/1] arm64: dts: rockchip: correct voltage selector Firefly-RK3399
From: Heinrich Schuchardt @ 2018-06-04 21:45 UTC (permalink / raw)
To: Rob Herring, Mark Rutland
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Heiko Stuebner,
Vagrant Cascadian, Catalin Marinas, Shawn Lin, Will Deacon,
Kever Yang, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Enric Balletbo i Serra, Pierre-Hugues Husson, Jianqun Xu,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20180604171523.28454-1-xypron.glpk-Mmb7MZpHnFY@public.gmane.org>
On 06/04/2018 07:15 PM, Heinrich Schuchardt wrote:
> Without this patch the Firefly-RK3399 board boot process hangs after these
> lines:
>
> fan53555-regulator 0-0040: FAN53555 Option[8] Rev[1] Detected!
> fan53555-reg: supplied by vcc_sys
> vcc1v8_s3: supplied by vcc_1v8
>
> Blacklisting driver fan53555 allows booting.
>
> The device tree uses a value of fcs,suspend-voltage-selector different to
> any other board.
>
> Changing this setting to the usual value is sufficient to enable booting.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk-Mmb7MZpHnFY@public.gmane.org>
> ---
> arch/arm64/boot/dts/rockchip/rk3399-firefly.dts | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-firefly.dts b/arch/arm64/boot/dts/rockchip/rk3399-firefly.dts
> index 4f28628aa091..50940ef844a7 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-firefly.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-firefly.dts
> @@ -458,7 +458,7 @@
> vdd_cpu_b: regulator@40 {
> compatible = "silergy,syr827";
> reg = <0x40>;
> - fcs,suspend-voltage-selector = <0>;
> + fcs,suspend-voltage-selector = <1>;
The same value <1> is used in the legacy kernel:
https://github.com/rockchip-linux/kernel/blob/release-4.4/arch/arm64/boot/dts/rockchip/rk3399-firefly-linux.dts
vdd_cpu_b: syr827@40 {
compatible = "silergy,syr827";
reg = <0x40>;
vin-supply = <&vcc5v0_sys>;
regulator-compatible = "fan53555-reg";
regulator-name = "vdd_cpu_b";
regulator-min-microvolt = <712500>;
regulator-max-microvolt = <1500000>;
regulator-ramp-delay = <1000>;
vsel-gpios = <&gpio1 18 GPIO_ACTIVE_HIGH>;
fcs,suspend-voltage-selector = <1>;
regulator-always-on;
regulator-boot-on;
regulator-initial-state = <3>;
regulator-state-mem {
regulator-off-in-suspend;
};
};
Best regards
Heinrich
> regulator-name = "vdd_cpu_b";
> regulator-min-microvolt = <712500>;
> regulator-max-microvolt = <1500000>;
>
^ permalink raw reply
* Re: [PATCH] arm64: dts: stingray: move common board components to stingray-board-base
From: Scott Branden @ 2018-06-04 22:24 UTC (permalink / raw)
To: Florian Fainelli, Rob Herring, Mark Rutland, Catalin Marinas,
Will Deacon
Cc: devicetree, BCM Kernel Feedback, linux-kernel, linux-arm-kernel
In-Reply-To: <05b33af8-c938-f124-f918-a9d7f8679d90@gmail.com>
Hi Florian,
On 18-06-04 02:09 PM, Florian Fainelli wrote:
> On 05/22/2018 11:55 AM, Scott Branden wrote:
>> Move common board components from base bcm958742 dtsi file to new
>> stingray-board-base dtsi file so they can be shared between many stingray
>> boards following common design.
>>
>> Signed-off-by: Scott Branden <scott.branden@broadcom.com>
>
> Applied to devicetree-arm64/next, though this did not apply cleanly,
> please check the results at:
>
> https://github.com/Broadcom/stblinux/commit/0b2cf5a855cd235fa95fbdfedfc524a97a71a7fe
It's looks fine.
>
>> ---
>> .../boot/dts/broadcom/stingray/bcm958742-base.dtsi | 35 +--------------
>> .../dts/broadcom/stingray/stingray-board-base.dtsi | 51 ++++++++++++++++++++++
>> 2 files changed, 52 insertions(+), 34 deletions(-)
>> create mode 100644 arch/arm64/boot/dts/broadcom/stingray/stingray-board-base.dtsi
>>
>> diff --git a/arch/arm64/boot/dts/broadcom/stingray/bcm958742-base.dtsi b/arch/arm64/boot/dts/broadcom/stingray/bcm958742-base.dtsi
>> index cacc25e..d74f6df 100644
>> --- a/arch/arm64/boot/dts/broadcom/stingray/bcm958742-base.dtsi
>> +++ b/arch/arm64/boot/dts/broadcom/stingray/bcm958742-base.dtsi
>> @@ -30,20 +30,9 @@
>> * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>> */
>>
>> -#include "stingray.dtsi"
>> +#include "stingray-board-base.dtsi"
>>
>> / {
>> - chosen {
>> - stdout-path = "serial0:115200n8";
>> - };
>> -
>> - aliases {
>> - serial0 = &uart1;
>> - serial1 = &uart0;
>> - serial2 = &uart2;
>> - serial3 = &uart3;
>> - };
>> -
>> sdio0_vddo_ctrl_reg: sdio0_vddo_ctrl {
>> compatible = "regulator-gpio";
>> regulator-name = "sdio0_vddo_ctrl_reg";
>> @@ -67,23 +56,6 @@
>> };
>> };
>>
>> -&memory { /* Default DRAM banks */
>> - reg = <0x00000000 0x80000000 0x0 0x80000000>, /* 2G @ 2G */
>> - <0x00000008 0x80000000 0x1 0x80000000>; /* 6G @ 34G */
>> -};
>> -
>> -&mdio_mux_iproc {
>> - mdio@10 {
>> - gphy0: eth-phy@10 {
>> - reg = <0x10>;
>> - };
>> - };
>> -};
>> -
>> -&uart1 {
>> - status = "okay";
>> -};
>> -
>> &pwm {
>> status = "okay";
>> };
>> @@ -111,8 +83,6 @@
>> };
>>
>> &enet {
>> - phy-mode = "rgmii-id";
>> - phy-handle = <&gphy0>;
>> status = "okay";
>> };
>>
>> @@ -133,13 +103,10 @@
>>
>> &sdio0 {
>> vqmmc-supply = <&sdio0_vddo_ctrl_reg>;
>> - non-removable;
>> - full-pwr-cycle;
>> status = "okay";
>> };
>>
>> &sdio1 {
>> vqmmc-supply = <&sdio1_vddo_ctrl_reg>;
>> - full-pwr-cycle;
>> status = "okay";
>> };
>> diff --git a/arch/arm64/boot/dts/broadcom/stingray/stingray-board-base.dtsi b/arch/arm64/boot/dts/broadcom/stingray/stingray-board-base.dtsi
>> new file mode 100644
>> index 0000000..82a2471
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/broadcom/stingray/stingray-board-base.dtsi
>> @@ -0,0 +1,51 @@
>> +// SPDX-License-Identifier: (GPL-2.0 or BSD-3-Clause)
>> +/*
>> + * Copyright(c) 2016-2018 Broadcom
>> + */
>> +
>> +#include "stingray.dtsi"
>> +#include <dt-bindings/gpio/gpio.h>
>> +
>> +/ {
>> + aliases {
>> + serial0 = &uart1;
>> + serial1 = &uart0;
>> + serial2 = &uart2;
>> + serial3 = &uart3;
>> + };
>> +
>> + chosen {
>> + stdout-path = "serial0:115200n8";
>> + };
>> +};
>> +
>> +&memory { /* Default DRAM banks */
>> + reg = <0x00000000 0x80000000 0x0 0x80000000>, /* 2G @ 2G */
>> + <0x00000008 0x80000000 0x1 0x80000000>; /* 6G @ 34G */
>> +};
>> +
>> +&enet {
>> + phy-mode = "rgmii-id";
>> + phy-handle = <&gphy0>;
>> +};
>> +
>> +&uart1 {
>> + status = "okay";
>> +};
>> +
>> +&sdio0 {
>> + non-removable;
>> + full-pwr-cycle;
>> +};
>> +
>> +&sdio1 {
>> + full-pwr-cycle;
>> +};
>> +
>> +&mdio_mux_iproc {
>> + mdio@10 {
>> + gphy0: eth-phy@10 {
>> + reg = <0x10>;
>> + };
>> + };
>> +};
>>
>
^ permalink raw reply
* Re: [PATCH] arm64: dts: stingray: Add otp device node
From: Scott Branden @ 2018-06-04 22:33 UTC (permalink / raw)
To: Florian Fainelli, Rob Herring, Mark Rutland, Catalin Marinas,
Will Deacon
Cc: BCM Kernel Feedback, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <efaf0373-2959-b238-e5f3-ae7e16534384@gmail.com>
On 18-06-04 02:33 PM, Florian Fainelli wrote:
> On 06/04/2018 02:30 PM, Scott Branden wrote:
>> Hi Florian,
>>
>>
>> On 18-06-04 02:24 PM, Florian Fainelli wrote:
>>> On 05/23/2018 01:17 PM, Scott Branden wrote:
>>>> Add otp device node for Stingray SOC.
>>>>
>>>> Fixes: 2fa9e9e29ea2 ("arm64: dts: Add GPIO DT nodes for Stingray SOC")
>>>> Signed-off-by: Scott Branden <scott.branden@broadcom.com>
>>> Applied to devicetree-arm64/next with s/otp/OTP/ and removing the Fixes
>>> line since that is not a bug fix AFAICT.
>> It fixes the issue that OTP is not active as it is missing the device node?
> By that token, any peripheral that is being added at some point in the
> lifetime of this DTS would qualify as a bugfix when it is in fact
> feature/peripheral enabling.
>
> I could not see the relationship between the commit being provided in
> the "Fixes:" tag and OTP, am I missing something?
The relationship is the fixes tag points was selected to the last tag
when the commit applies directly against (and is far enough back that it
covers any possible LTS kernels that would have needed it). In this case
I don't care too much about whether this is fixed in LTS or not. If
needed I'll send a request for the commit be ported to stable.
^ permalink raw reply
* Re: [PATCH] arm64: dts: stingray: Add otp device node
From: Florian Fainelli @ 2018-06-04 22:41 UTC (permalink / raw)
To: Scott Branden, Florian Fainelli, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon
Cc: BCM Kernel Feedback, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <e641f180-0b7e-678c-07c4-834ff0676b34@broadcom.com>
On 06/04/2018 03:33 PM, Scott Branden wrote:
>
>
> On 18-06-04 02:33 PM, Florian Fainelli wrote:
>> On 06/04/2018 02:30 PM, Scott Branden wrote:
>>> Hi Florian,
>>>
>>>
>>> On 18-06-04 02:24 PM, Florian Fainelli wrote:
>>>> On 05/23/2018 01:17 PM, Scott Branden wrote:
>>>>> Add otp device node for Stingray SOC.
>>>>>
>>>>> Fixes: 2fa9e9e29ea2 ("arm64: dts: Add GPIO DT nodes for Stingray SOC")
>>>>> Signed-off-by: Scott Branden <scott.branden@broadcom.com>
>>>> Applied to devicetree-arm64/next with s/otp/OTP/ and removing the Fixes
>>>> line since that is not a bug fix AFAICT.
>>> It fixes the issue that OTP is not active as it is missing the device
>>> node?
>> By that token, any peripheral that is being added at some point in the
>> lifetime of this DTS would qualify as a bugfix when it is in fact
>> feature/peripheral enabling.
>>
>> I could not see the relationship between the commit being provided in
>> the "Fixes:" tag and OTP, am I missing something?
> The relationship is the fixes tag points was selected to the last tag
> when the commit applies directly against (and is far enough back that it
> covers any possible LTS kernels that would have needed it).
I understand how ones gets to select an appropriate Fixes: tag, what I
don't understand is the relationship between the two changes, like why
would a GPIO Device Tree node influence the OTP peripheral here when
there is no pinmux or GPIO phandle of some sort linking the two. Also,
since you guys were very trigger happy with Fixes: tag lately (referring
to the internal review of Srinath's changes) this one looked a lot like
that.
The only thing I am questioning here is treating that particular
changeset as a bugfix proper. Yes it is technically a fix in that,
without this changeset the OTP node is not there and that is
functionality you want, but it is not preventing the platform from not
booting for instance, or having an incorrect behavior for an established
behavior before, right?
> In this case
> I don't care too much about whether this is fixed in LTS or not. If
> needed I'll send a request for the commit be ported to stable.
If this is a true fix, I don't mind taking it as-is and keeping the
Fixes: tag, keep in mind the following:
I always treat bug fixes with a higher priority than features, and I
will do my best to queue up fixes (separate branches, all ending in
/fixes) and submit those at any time in the release cycle so they can
land in Linus' tree and in the -stable trees as fast as possible.
Don't bypass the maintainer if you can convince me this is a proper fix,
then I will just move that patch into the appropriate branch, and you
will get those stable backports automatically.
Thanks for reading me.
--
Florian
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox