* Re: [PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi
From: Mark Brown @ 2018-05-24 18:15 UTC (permalink / raw)
To: Radu Pirea
Cc: Andy Shevchenko, devicetree, open list:SERIAL DRIVERS,
Linux Kernel Mailing List, linux-arm Mailing List, linux-spi,
Mark Rutland, Rob Herring, Lee Jones, Greg Kroah-Hartman,
Jiri Slaby, Richard Genoud, alexandre.belloni, Nicolas Ferre
In-Reply-To: <0e6e71e2-f8ac-7889-0d81-8d8a4c15223d@microchip.com>
[-- Attachment #1: Type: text/plain, Size: 526 bytes --]
On Thu, May 24, 2018 at 07:04:11PM +0300, Radu Pirea wrote:
> if (ctlr->cs_gpios){
> spi->cs_gpio = ctlr->cs_gpios[spi->chip_select];
> if(gpio_is_valid(spi->cs_gpio))
> gpio_direction_output(spi->cs_gpio, !(spi->mode & SPI_CS_HIGH));
>
> }
You're expected to request the GPIOs in your driver using one of the
modern request functions like gpio_request_one() (or ideally the GPIO
descriptor APIs now) which combine the direction setting and request
into a single operation.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi
From: Mark Brown @ 2018-05-24 18:16 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Radu Pirea, Andy Shevchenko, devicetree, open list:SERIAL DRIVERS,
Linux Kernel Mailing List, linux-arm Mailing List, linux-spi,
Mark Rutland, Rob Herring, Lee Jones, Greg Kroah-Hartman,
Jiri Slaby, Richard Genoud, Nicolas Ferre
In-Reply-To: <20180524175620.GB12093@piout.net>
[-- Attachment #1: Type: text/plain, Size: 565 bytes --]
On Thu, May 24, 2018 at 07:56:20PM +0200, Alexandre Belloni wrote:
> Back in 2014, I was suggesting using devm_gpio_request_one() in
> of_spi_register_master(). That would take care of setting the direction
> of the GPIO:
> https://www.spinics.net/lists/arm-kernel/msg351251.html
> I never took the time to create the patch and test though.
Right, this'd be ideal but unfortunately it does mean we'd have to
transition all the existing users that open code their requests which is
a pain. It'd be really nice if someone had the time/enthusiam to do it
though.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v2 1/8] driver core: make deferring probe after init optional
From: Mark Brown @ 2018-05-24 18:18 UTC (permalink / raw)
To: Rob Herring
Cc: Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
Bjorn Andersson, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
Joerg Roedel, Robin Murphy, Frank Rowand, linux-kernel,
devicetree, boot-architecture, linux-arm-kernel
In-Reply-To: <20180524175024.19874-2-robh@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 598 bytes --]
On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:
> Subsystems or drivers may opt-in to this behavior by calling
> driver_deferred_probe_check_init_done() instead of just returning
> -EPROBE_DEFER. They may use additional information from DT or kernel's
> config to decide whether to continue to defer probe or not.
Should userspace have some involvement in this decision? It knows if
it's got any intention of loading modules for example. Kernel config
checks might be good enough, though it's going to be a pain to work out
if the relevant driver is built as a module for example.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v2 1/8] driver core: make deferring probe after init optional
From: Greg Kroah-Hartman @ 2018-05-24 18:56 UTC (permalink / raw)
To: Rob Herring
Cc: Linus Walleij, Alexander Graf, Bjorn Andersson, Rafael J. Wysocki,
Kevin Hilman, Ulf Hansson, Joerg Roedel, Robin Murphy, Mark Brown,
Frank Rowand, linux-kernel, devicetree, boot-architecture,
linux-arm-kernel
In-Reply-To: <20180524175024.19874-2-robh@kernel.org>
On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:
> Deferred probe will currently wait forever on dependent devices to probe,
> but sometimes a driver will never exist. It's also not always critical for
> a driver to exist. Platforms can rely on default configuration from the
> bootloader or reset defaults for things such as pinctrl and power domains.
> This is often the case with initial platform support until various drivers
> get enabled. There's at least 2 scenarios where deferred probe can render
> a platform broken. Both involve using a DT which has more devices and
> dependencies than the kernel supports. The 1st case is a driver may be
> disabled in the kernel config. The 2nd case is the kernel version may
> simply not have the dependent driver. This can happen if using a newer DT
> (provided by firmware perhaps) with a stable kernel version.
>
> Subsystems or drivers may opt-in to this behavior by calling
> driver_deferred_probe_check_init_done() instead of just returning
> -EPROBE_DEFER. They may use additional information from DT or kernel's
> config to decide whether to continue to defer probe or not.
>
> Cc: Alexander Graf <agraf@suse.de>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> drivers/base/dd.c | 17 +++++++++++++++++
> include/linux/device.h | 2 ++
> 2 files changed, 19 insertions(+)
>
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index c9f54089429b..d6034718da6f 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -226,6 +226,16 @@ void device_unblock_probing(void)
> driver_deferred_probe_trigger();
> }
>
> +int driver_deferred_probe_check_init_done(struct device *dev, bool optional)
> +{
> + if (optional && initcalls_done) {
> + dev_WARN(dev, "ignoring dependency for device, assuming no driver");
You really only need dev_warn(), here, right?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v2 1/8] driver core: make deferring probe after init optional
From: Greg Kroah-Hartman @ 2018-05-24 19:00 UTC (permalink / raw)
To: Rob Herring
Cc: Linus Walleij, Alexander Graf, Bjorn Andersson, Rafael J. Wysocki,
Kevin Hilman, Ulf Hansson, Joerg Roedel, Robin Murphy, Mark Brown,
Frank Rowand, linux-kernel, devicetree, boot-architecture,
linux-arm-kernel
In-Reply-To: <20180524175024.19874-2-robh@kernel.org>
On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:
> Deferred probe will currently wait forever on dependent devices to probe,
> but sometimes a driver will never exist. It's also not always critical for
> a driver to exist. Platforms can rely on default configuration from the
> bootloader or reset defaults for things such as pinctrl and power domains.
> This is often the case with initial platform support until various drivers
> get enabled. There's at least 2 scenarios where deferred probe can render
> a platform broken. Both involve using a DT which has more devices and
> dependencies than the kernel supports. The 1st case is a driver may be
> disabled in the kernel config. The 2nd case is the kernel version may
> simply not have the dependent driver. This can happen if using a newer DT
> (provided by firmware perhaps) with a stable kernel version.
>
> Subsystems or drivers may opt-in to this behavior by calling
> driver_deferred_probe_check_init_done() instead of just returning
> -EPROBE_DEFER. They may use additional information from DT or kernel's
> config to decide whether to continue to defer probe or not.
>
> Cc: Alexander Graf <agraf@suse.de>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> drivers/base/dd.c | 17 +++++++++++++++++
> include/linux/device.h | 2 ++
> 2 files changed, 19 insertions(+)
>
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index c9f54089429b..d6034718da6f 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -226,6 +226,16 @@ void device_unblock_probing(void)
> driver_deferred_probe_trigger();
> }
>
> +int driver_deferred_probe_check_init_done(struct device *dev, bool optional)
> +{
> + if (optional && initcalls_done) {
Wait, what's the "optional" mess here?
The caller knows this value, so why do you need to even pass it in here?
And bool values that are not obvious are horrid. I had to go look this
up when reading the later patches that just passed "true" in this
variable as I had no idea what that meant.
So as-is, no, this isn't ok, sorry.
And at the least, this needs some kerneldoc to explain it :)
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v2 2/8] driver core: add a deferred probe timeout
From: Greg Kroah-Hartman @ 2018-05-24 19:01 UTC (permalink / raw)
To: Rob Herring
Cc: Linus Walleij, Alexander Graf, Bjorn Andersson, Rafael J. Wysocki,
Kevin Hilman, Ulf Hansson, Joerg Roedel, Robin Murphy, Mark Brown,
Frank Rowand, linux-kernel, devicetree, boot-architecture,
linux-arm-kernel
In-Reply-To: <20180524175024.19874-3-robh@kernel.org>
On Thu, May 24, 2018 at 12:50:18PM -0500, Rob Herring wrote:
> Deferring probe can wait forever on dependencies that may never appear
> for a variety of reasons. This can be difficult to debug especially if
> the console has dependencies or userspace fails to boot to a shell. Add
> a timeout to retry probing without possibly optional dependencies and to
> dump out the deferred probe pending list after retrying.
>
> This mechanism is intended for debug purposes. It won't work for the
> console which needs to be enabled before userspace starts. However, if
> the console's dependencies are resolved, then the kernel log will be
> printed (as opposed to no output).
>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> .../admin-guide/kernel-parameters.txt | 7 +++++
> drivers/base/dd.c | 28 ++++++++++++++++++-
> 2 files changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 11fc28ecdb6d..dd3f40b34a24 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -809,6 +809,13 @@
> Defaults to the default architecture's huge page size
> if not specified.
>
> + deferred_probe_timeout=
> + [KNL] Set a timeout in seconds for deferred probe to
> + give up waiting on dependencies to probe. Only specific
> + dependencies (subsystems or drivers) that have opted in
> + will be ignored. This option also dumps out devices
> + still on the deferred probe list after retrying.
Doesn't sound like a debugging-only option. I can see devices enabling
this when they figure out that's the only way their platform can boot :)
thanks,
greg k-h
^ permalink raw reply
* Applied "dt-bindings: qcom_spmi: Document SAW support" to the regulator tree
From: Mark Brown @ 2018-05-24 19:25 UTC (permalink / raw)
Cc: Mark Brown, linux-kernel, devicetree, lgirdwood
In-Reply-To: <1520166709-554-3-git-send-email-ilialin@codeaurora.org>
The patch
dt-bindings: qcom_spmi: Document SAW support
has been applied to the regulator tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
>From f50e5ddae899d1599e4d6ccb46eadd78b7ed14bc Mon Sep 17 00:00:00 2001
From: Ilia Lin <ilialin@codeaurora.org>
Date: Mon, 21 May 2018 14:25:31 +0300
Subject: [PATCH] dt-bindings: qcom_spmi: Document SAW support
Document the DT bindings for the SAW regulators.
The saw-leader is the only property that is configurable in DT.
The saw-slave property allows ganging (grouping) of
several regulators so that their outputs can be combined.
Signed-off-by: Ilia Lin <ilialin@codeaurora.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
.../regulator/qcom,spmi-regulator.txt | 45 +++++++++++++++++++
1 file changed, 45 insertions(+)
diff --git a/Documentation/devicetree/bindings/regulator/qcom,spmi-regulator.txt b/Documentation/devicetree/bindings/regulator/qcom,spmi-regulator.txt
index 57d2c65899df..406f2e570c50 100644
--- a/Documentation/devicetree/bindings/regulator/qcom,spmi-regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/qcom,spmi-regulator.txt
@@ -110,6 +110,11 @@ Qualcomm SPMI Regulators
Definition: Reference to regulator supplying the input pin, as
described in the data sheet.
+- qcom,saw-reg:
+ Usage: optional
+ Value type: <phandle>
+ Description: Reference to syscon node defining the SAW registers.
+
The regulator node houses sub-nodes for each regulator within the device. Each
sub-node is identified using the node's name, with valid values listed for each
@@ -201,6 +206,17 @@ see regulator.txt - with additional custom properties described below:
2 = 0.55 uA
3 = 0.75 uA
+- qcom,saw-slave:
+ Usage: optional
+ Value type: <boo>
+ Description: SAW controlled gang slave. Will not be configured.
+
+- qcom,saw-leader:
+ Usage: optional
+ Value type: <boo>
+ Description: SAW controlled gang leader. Will be configured as
+ SAW regulator.
+
Example:
regulators {
@@ -221,3 +237,32 @@ Example:
....
};
+
+Example 2:
+
+ saw3: syscon@9A10000 {
+ compatible = "syscon";
+ reg = <0x9A10000 0x1000>;
+ };
+
+ ...
+
+ spm-regulators {
+ compatible = "qcom,pm8994-regulators";
+ qcom,saw-reg = <&saw3>;
+ s8 {
+ qcom,saw-slave;
+ };
+ s9 {
+ qcom,saw-slave;
+ };
+ s10 {
+ qcom,saw-slave;
+ };
+ pm8994_s11_saw: s11 {
+ qcom,saw-leader;
+ regulator-always-on;
+ regulator-min-microvolt = <900000>;
+ regulator-max-microvolt = <1140000>;
+ };
+ };
--
2.17.0
^ permalink raw reply related
* Applied "regulator: qcom_spmi: Add support for SAW" to the regulator tree
From: Mark Brown @ 2018-05-24 19:25 UTC (permalink / raw)
Cc: Mark Brown, linux-kernel, devicetree, lgirdwood
In-Reply-To: <1520166709-554-2-git-send-email-ilialin@codeaurora.org>
The patch
regulator: qcom_spmi: Add support for SAW
has been applied to the regulator tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
>From 0caecaa87202b667591d57e8ca233ee1b548ba13 Mon Sep 17 00:00:00 2001
From: Ilia Lin <ilialin@codeaurora.org>
Date: Mon, 21 May 2018 14:25:30 +0300
Subject: [PATCH] regulator: qcom_spmi: Add support for SAW
Add support for SAW controlled regulators.
The regulators defined as SAW controlled in the device tree
will be controlled through special CPU registers instead of direct
SPMI accesses.
This is required especially for CPU supply regulators to synchronize
with clock scaling and for Automatic Voltage Switching.
Signed-off-by: Ilia Lin <ilialin@codeaurora.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
drivers/regulator/qcom_spmi-regulator.c | 133 +++++++++++++++++++++++-
1 file changed, 130 insertions(+), 3 deletions(-)
diff --git a/drivers/regulator/qcom_spmi-regulator.c b/drivers/regulator/qcom_spmi-regulator.c
index 63c7a0c17777..9817f1a75342 100644
--- a/drivers/regulator/qcom_spmi-regulator.c
+++ b/drivers/regulator/qcom_spmi-regulator.c
@@ -25,6 +25,8 @@
#include <linux/regulator/driver.h>
#include <linux/regmap.h>
#include <linux/list.h>
+#include <linux/mfd/syscon.h>
+#include <linux/io.h>
/* Pin control enable input pins. */
#define SPMI_REGULATOR_PIN_CTRL_ENABLE_NONE 0x00
@@ -181,6 +183,23 @@ enum spmi_boost_byp_registers {
SPMI_BOOST_BYP_REG_CURRENT_LIMIT = 0x4b,
};
+enum spmi_saw3_registers {
+ SAW3_SECURE = 0x00,
+ SAW3_ID = 0x04,
+ SAW3_SPM_STS = 0x0C,
+ SAW3_AVS_STS = 0x10,
+ SAW3_PMIC_STS = 0x14,
+ SAW3_RST = 0x18,
+ SAW3_VCTL = 0x1C,
+ SAW3_AVS_CTL = 0x20,
+ SAW3_AVS_LIMIT = 0x24,
+ SAW3_AVS_DLY = 0x28,
+ SAW3_AVS_HYSTERESIS = 0x2C,
+ SAW3_SPM_STS2 = 0x38,
+ SAW3_SPM_PMIC_DATA_3 = 0x4C,
+ SAW3_VERSION = 0xFD0,
+};
+
/* Used for indexing into ctrl_reg. These are offets from 0x40 */
enum spmi_common_control_register_index {
SPMI_COMMON_IDX_VOLTAGE_RANGE = 0,
@@ -1035,6 +1054,89 @@ static irqreturn_t spmi_regulator_vs_ocp_isr(int irq, void *data)
return IRQ_HANDLED;
}
+#define SAW3_VCTL_DATA_MASK 0xFF
+#define SAW3_VCTL_CLEAR_MASK 0x700FF
+#define SAW3_AVS_CTL_EN_MASK 0x1
+#define SAW3_AVS_CTL_TGGL_MASK 0x8000000
+#define SAW3_AVS_CTL_CLEAR_MASK 0x7efc00
+
+static struct regmap *saw_regmap = NULL;
+
+static void spmi_saw_set_vdd(void *data)
+{
+ u32 vctl, data3, avs_ctl, pmic_sts;
+ bool avs_enabled = false;
+ unsigned long timeout;
+ u8 voltage_sel = *(u8 *)data;
+
+ regmap_read(saw_regmap, SAW3_AVS_CTL, &avs_ctl);
+ regmap_read(saw_regmap, SAW3_VCTL, &vctl);
+ regmap_read(saw_regmap, SAW3_SPM_PMIC_DATA_3, &data3);
+
+ /* select the band */
+ vctl &= ~SAW3_VCTL_CLEAR_MASK;
+ vctl |= (u32)voltage_sel;
+
+ data3 &= ~SAW3_VCTL_CLEAR_MASK;
+ data3 |= (u32)voltage_sel;
+
+ /* If AVS is enabled, switch it off during the voltage change */
+ avs_enabled = SAW3_AVS_CTL_EN_MASK & avs_ctl;
+ if (avs_enabled) {
+ avs_ctl &= ~SAW3_AVS_CTL_TGGL_MASK;
+ regmap_write(saw_regmap, SAW3_AVS_CTL, avs_ctl);
+ }
+
+ regmap_write(saw_regmap, SAW3_RST, 1);
+ regmap_write(saw_regmap, SAW3_VCTL, vctl);
+ regmap_write(saw_regmap, SAW3_SPM_PMIC_DATA_3, data3);
+
+ timeout = jiffies + usecs_to_jiffies(100);
+ do {
+ regmap_read(saw_regmap, SAW3_PMIC_STS, &pmic_sts);
+ pmic_sts &= SAW3_VCTL_DATA_MASK;
+ if (pmic_sts == (u32)voltage_sel)
+ break;
+
+ cpu_relax();
+
+ } while (time_before(jiffies, timeout));
+
+ /* After successful voltage change, switch the AVS back on */
+ if (avs_enabled) {
+ pmic_sts &= 0x3f;
+ avs_ctl &= ~SAW3_AVS_CTL_CLEAR_MASK;
+ avs_ctl |= ((pmic_sts - 4) << 10);
+ avs_ctl |= (pmic_sts << 17);
+ avs_ctl |= SAW3_AVS_CTL_TGGL_MASK;
+ regmap_write(saw_regmap, SAW3_AVS_CTL, avs_ctl);
+ }
+}
+
+static int
+spmi_regulator_saw_set_voltage(struct regulator_dev *rdev, unsigned selector)
+{
+ struct spmi_regulator *vreg = rdev_get_drvdata(rdev);
+ int ret;
+ u8 range_sel, voltage_sel;
+
+ ret = spmi_sw_selector_to_hw(vreg, selector, &range_sel, &voltage_sel);
+ if (ret)
+ return ret;
+
+ if (0 != range_sel) {
+ dev_dbg(&rdev->dev, "range_sel = %02X voltage_sel = %02X", \
+ range_sel, voltage_sel);
+ return -EINVAL;
+ }
+
+ /* Always do the SAW register writes on the first CPU */
+ return smp_call_function_single(0, spmi_saw_set_vdd, \
+ &voltage_sel, true);
+}
+
+static struct regulator_ops spmi_saw_ops = {};
+
static struct regulator_ops spmi_smps_ops = {
.enable = regulator_enable_regmap,
.disable = regulator_disable_regmap,
@@ -1250,6 +1352,7 @@ static int spmi_regulator_match(struct spmi_regulator *vreg, u16 force_type)
}
dig_major_rev = version[SPMI_COMMON_REG_DIG_MAJOR_REV
- SPMI_COMMON_REG_DIG_MAJOR_REV];
+
if (!force_type) {
type = version[SPMI_COMMON_REG_TYPE -
SPMI_COMMON_REG_DIG_MAJOR_REV];
@@ -1648,7 +1751,9 @@ static int qcom_spmi_regulator_probe(struct platform_device *pdev)
struct regmap *regmap;
const char *name;
struct device *dev = &pdev->dev;
- int ret;
+ struct device_node *node = pdev->dev.of_node;
+ struct device_node *syscon;
+ int ret, lenp;
struct list_head *vreg_list;
vreg_list = devm_kzalloc(dev, sizeof(*vreg_list), GFP_KERNEL);
@@ -1665,7 +1770,22 @@ static int qcom_spmi_regulator_probe(struct platform_device *pdev)
if (!match)
return -ENODEV;
+ if (of_find_property(node, "qcom,saw-reg", &lenp)) {
+ syscon = of_parse_phandle(node, "qcom,saw-reg", 0);
+ saw_regmap = syscon_node_to_regmap(syscon);
+ of_node_put(syscon);
+ if (IS_ERR(regmap))
+ dev_err(dev, "ERROR reading SAW regmap\n");
+ }
+
for (reg = match->data; reg->name; reg++) {
+
+ if (saw_regmap && \
+ of_find_property(of_find_node_by_name(node, reg->name), \
+ "qcom,saw-slave", &lenp)) {
+ continue;
+ }
+
vreg = devm_kzalloc(dev, sizeof(*vreg), GFP_KERNEL);
if (!vreg)
return -ENOMEM;
@@ -1673,7 +1793,6 @@ static int qcom_spmi_regulator_probe(struct platform_device *pdev)
vreg->dev = dev;
vreg->base = reg->base;
vreg->regmap = regmap;
-
if (reg->ocp) {
vreg->ocp_irq = platform_get_irq_byname(pdev, reg->ocp);
if (vreg->ocp_irq < 0) {
@@ -1681,7 +1800,6 @@ static int qcom_spmi_regulator_probe(struct platform_device *pdev)
goto err;
}
}
-
vreg->desc.id = -1;
vreg->desc.owner = THIS_MODULE;
vreg->desc.type = REGULATOR_VOLTAGE;
@@ -1698,6 +1816,15 @@ static int qcom_spmi_regulator_probe(struct platform_device *pdev)
if (ret)
continue;
+ if (saw_regmap && \
+ of_find_property(of_find_node_by_name(node, reg->name), \
+ "qcom,saw-leader", &lenp)) {
+ spmi_saw_ops = *(vreg->desc.ops);
+ spmi_saw_ops.set_voltage_sel = \
+ spmi_regulator_saw_set_voltage;
+ vreg->desc.ops = &spmi_saw_ops;
+ }
+
config.dev = dev;
config.driver_data = vreg;
config.regmap = regmap;
--
2.17.0
^ permalink raw reply related
* Re: [PATCH v2 1/8] driver core: make deferring probe after init optional
From: Rob Herring @ 2018-05-24 19:42 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Linus Walleij, Alexander Graf, Bjorn Andersson, Rafael J. Wysocki,
Kevin Hilman, Ulf Hansson, Joerg Roedel, Robin Murphy, Mark Brown,
Frank Rowand, linux-kernel@vger.kernel.org, devicetree,
Architecture Mailman List,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <20180524185639.GA31019@kroah.com>
On Thu, May 24, 2018 at 1:56 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:
>> Deferred probe will currently wait forever on dependent devices to probe,
>> but sometimes a driver will never exist. It's also not always critical for
>> a driver to exist. Platforms can rely on default configuration from the
>> bootloader or reset defaults for things such as pinctrl and power domains.
>> This is often the case with initial platform support until various drivers
>> get enabled. There's at least 2 scenarios where deferred probe can render
>> a platform broken. Both involve using a DT which has more devices and
>> dependencies than the kernel supports. The 1st case is a driver may be
>> disabled in the kernel config. The 2nd case is the kernel version may
>> simply not have the dependent driver. This can happen if using a newer DT
>> (provided by firmware perhaps) with a stable kernel version.
>>
>> Subsystems or drivers may opt-in to this behavior by calling
>> driver_deferred_probe_check_init_done() instead of just returning
>> -EPROBE_DEFER. They may use additional information from DT or kernel's
>> config to decide whether to continue to defer probe or not.
>>
>> Cc: Alexander Graf <agraf@suse.de>
>> Signed-off-by: Rob Herring <robh@kernel.org>
>> ---
>> drivers/base/dd.c | 17 +++++++++++++++++
>> include/linux/device.h | 2 ++
>> 2 files changed, 19 insertions(+)
>>
>> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
>> index c9f54089429b..d6034718da6f 100644
>> --- a/drivers/base/dd.c
>> +++ b/drivers/base/dd.c
>> @@ -226,6 +226,16 @@ void device_unblock_probing(void)
>> driver_deferred_probe_trigger();
>> }
>>
>> +int driver_deferred_probe_check_init_done(struct device *dev, bool optional)
>> +{
>> + if (optional && initcalls_done) {
>> + dev_WARN(dev, "ignoring dependency for device, assuming no driver");
>
> You really only need dev_warn(), here, right?
No, the screaming is on purpose.
Bjorn had concerns about debugging/supporting subtle problems that
could stem from this. Such as electrical settings not quite right that
cause intermittent errors.
Rob
^ permalink raw reply
* Re: [PATCH v2 2/8] driver core: add a deferred probe timeout
From: Rob Herring @ 2018-05-24 19:45 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Linus Walleij, Alexander Graf, Bjorn Andersson, Rafael J. Wysocki,
Kevin Hilman, Ulf Hansson, Joerg Roedel, Robin Murphy, Mark Brown,
Frank Rowand, linux-kernel@vger.kernel.org, devicetree,
Architecture Mailman List,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <20180524190126.GC31019@kroah.com>
On Thu, May 24, 2018 at 2:01 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Thu, May 24, 2018 at 12:50:18PM -0500, Rob Herring wrote:
>> Deferring probe can wait forever on dependencies that may never appear
>> for a variety of reasons. This can be difficult to debug especially if
>> the console has dependencies or userspace fails to boot to a shell. Add
>> a timeout to retry probing without possibly optional dependencies and to
>> dump out the deferred probe pending list after retrying.
>>
>> This mechanism is intended for debug purposes. It won't work for the
>> console which needs to be enabled before userspace starts. However, if
>> the console's dependencies are resolved, then the kernel log will be
>> printed (as opposed to no output).
>>
>> Signed-off-by: Rob Herring <robh@kernel.org>
>> ---
>> .../admin-guide/kernel-parameters.txt | 7 +++++
>> drivers/base/dd.c | 28 ++++++++++++++++++-
>> 2 files changed, 34 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index 11fc28ecdb6d..dd3f40b34a24 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -809,6 +809,13 @@
>> Defaults to the default architecture's huge page size
>> if not specified.
>>
>> + deferred_probe_timeout=
>> + [KNL] Set a timeout in seconds for deferred probe to
>> + give up waiting on dependencies to probe. Only specific
>> + dependencies (subsystems or drivers) that have opted in
>> + will be ignored. This option also dumps out devices
>> + still on the deferred probe list after retrying.
>
> Doesn't sound like a debugging-only option. I can see devices enabling
> this when they figure out that's the only way their platform can boot :)
Here's some rope...
No doubt it can be abused. So are you saying don't call it a debug
option or hide it behind a config option? And for the latter, what's
one that distros don't just turn on?
Rob
^ permalink raw reply
* Re: [PATCH v2 2/8] driver core: add a deferred probe timeout
From: Greg Kroah-Hartman @ 2018-05-24 19:51 UTC (permalink / raw)
To: Rob Herring
Cc: Linus Walleij, Alexander Graf, Bjorn Andersson, Rafael J. Wysocki,
Kevin Hilman, Ulf Hansson, Joerg Roedel, Robin Murphy, Mark Brown,
Frank Rowand, linux-kernel@vger.kernel.org, devicetree,
Architecture Mailman List,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <CAL_Jsq+wMJ+9JTGFGSsSW2Vww_mvAkzc3ujmftgbpQxDTrLHag@mail.gmail.com>
On Thu, May 24, 2018 at 02:45:48PM -0500, Rob Herring wrote:
> On Thu, May 24, 2018 at 2:01 PM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > On Thu, May 24, 2018 at 12:50:18PM -0500, Rob Herring wrote:
> >> Deferring probe can wait forever on dependencies that may never appear
> >> for a variety of reasons. This can be difficult to debug especially if
> >> the console has dependencies or userspace fails to boot to a shell. Add
> >> a timeout to retry probing without possibly optional dependencies and to
> >> dump out the deferred probe pending list after retrying.
> >>
> >> This mechanism is intended for debug purposes. It won't work for the
> >> console which needs to be enabled before userspace starts. However, if
> >> the console's dependencies are resolved, then the kernel log will be
> >> printed (as opposed to no output).
> >>
> >> Signed-off-by: Rob Herring <robh@kernel.org>
> >> ---
> >> .../admin-guide/kernel-parameters.txt | 7 +++++
> >> drivers/base/dd.c | 28 ++++++++++++++++++-
> >> 2 files changed, 34 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> >> index 11fc28ecdb6d..dd3f40b34a24 100644
> >> --- a/Documentation/admin-guide/kernel-parameters.txt
> >> +++ b/Documentation/admin-guide/kernel-parameters.txt
> >> @@ -809,6 +809,13 @@
> >> Defaults to the default architecture's huge page size
> >> if not specified.
> >>
> >> + deferred_probe_timeout=
> >> + [KNL] Set a timeout in seconds for deferred probe to
> >> + give up waiting on dependencies to probe. Only specific
> >> + dependencies (subsystems or drivers) that have opted in
> >> + will be ignored. This option also dumps out devices
> >> + still on the deferred probe list after retrying.
> >
> > Doesn't sound like a debugging-only option. I can see devices enabling
> > this when they figure out that's the only way their platform can boot :)
>
> Here's some rope...
>
> No doubt it can be abused. So are you saying don't call it a debug
> option or hide it behind a config option? And for the latter, what's
> one that distros don't just turn on?
If it is a debug option, make it obvious it's only for debugging. The
commit log here says it's a debug option, but your documentation does
not say that at all, and that's what people will read.
Well, they might read it, probably not. But at least give us something
to point at when they mess things up :)
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v3 2/3] arm64: dts: renesas: draak: Describe CVBS input
From: jacopo mondi @ 2018-05-24 19:59 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Jacopo Mondi, niklas.soderlund, horms, geert, magnus.damm,
robh+dt, linux-renesas-soc, devicetree, linux-arm-kernel,
linux-kernel
In-Reply-To: <1728709.thk7gXDxlo@avalon>
[-- Attachment #1: Type: text/plain, Size: 5326 bytes --]
HI Laurent,
On Mon, May 21, 2018 at 04:10:34PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Monday, 21 May 2018 15:33:40 EEST jacopo mondi wrote:
> > On Mon, May 21, 2018 at 01:54:55PM +0300, Laurent Pinchart wrote:
> > > On Monday, 21 May 2018 12:57:05 EEST jacopo mondi wrote:
> > >> On Fri, May 18, 2018 at 06:12:15PM +0300, Laurent Pinchart wrote:
> > >>> On Friday, 18 May 2018 17:47:57 EEST Jacopo Mondi wrote:
> > >>>> Describe CVBS video input through analog video decoder ADV7180
> > >>>> connected to video input interface VIN4.
> > >>>>
> > >>>> The video input signal path is shared with HDMI video input, and
> > >>>> selected by on-board switches SW-53 and SW-54 with CVBS input
> > >>>> selected by the default switches configuration.
> > >>>>
> > >>>> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > >>>> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > >>>>
> > >>>> ---
> > >>>> v2 -> v3:
> > >>>> - Add comment to describe the shared input video path
> > >>>> - Add my SoB and Niklas' R-b tags
> > >>>> ---
> > >>>>
> > >>>> arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 42 ++++++++++++++++++
> > >>>> 1 file changed, 42 insertions(+)
> > >>>>
> > >>>> diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > >>>> b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts index
> > >>>> 9d73de8..95745fc
> > >>>> 100644
> > >>>> --- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > >>>> +++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > >>>> @@ -142,6 +142,11 @@
> > >>>> groups = "usb0";
> > >>>> function = "usb0";
> > >>>> };
> > >>>> +
> > >>>> + vin4_pins_cvbs: vin4 {
> > >>>> + groups = "vin4_data8", "vin4_sync", "vin4_clk";
> > >>>> + function = "vin4";
> > >>>> + };
> > >>>> };
> > >>>>
> > >>>> &i2c0 {
> > >>>> @@ -154,6 +159,23 @@
> > >>>> reg = <0x50>;
> > >>>> pagesize = <8>;
> > >>>> };
> > >>>> +
> > >>>> + analog-video@20 {
> > >>>> + compatible = "adi,adv7180";
> > >>>> + reg = <0x20>;
> > >>>> +
> > >>>> + port {
> > >>>
> > >>> The adv7180 DT bindings document the output port as 3 or 6
> > >>> (respectively for the CP and ST versions of the chip). You should thus
> > >>> number the port. Apart from that the patch looks good.
> > >>
> > >> I admit I have barely copied this from Gen-2 boards DTS, but reading
> > >> the driver code and binding description again, I think this is
> > >> correct, as the output port numbering and mandatory input port (which
> > >> is missing here) only apply to adv7180cp/st chip versions.
> > >>
> > >> Here we describe plain adv7180, no need to number output ports afaict.
> > >
> > > Indeed, my bad.
> > >
> > > Shouldn't you however use "adi,adv7180cp" as that's the chip we are using
> > > ?
> > > The "adi,adv7180" has no port documented in its DT bindings, so it
> > > shouldn't have any port node at all.
> >
> > I'm a bit confused here.
> >
> > The only Gen-2 board using the "adi,adv7180cp" compatible string is
> > Gose, which is also the only one I can get schematics for. That board
> > indeed feature an ADV7180WBCP32Z, as the Draak does. I cannot get
> > schematics for any other Gen-2 board, to compare the ADV7180 variant
> > installed there. If anyone can confirm that all other Gen-2 board uses
> > a different version (or that all other Gen-2 board DTS file use a
> > wrong compatible string value), I'll re-send this with a different
> > compatible value and proper port nodes numbering.
>
> Other Gen2 boards use a ADV7180WBCP32Z as well. The issue here isn't that the
> chip you're trying to support is different. The DT bindings that were
> initially written for the adi,adv7180 didn't have port nodes. When it was time
> to add them, we realized that two variants of the chip existed with different
> connectivity. We have thus added two new compatible strings to differentiate
> them, with different port numbers. The old compatible string should be
> deprecated in favour of the new ones.
>
Ack, thanks for explaining.
I don't have any Gen2 board here, but if someone is willing to test, I
can change the Gen2 bindings to use the new version in a follow-up
series.
Thanks
j
> > >>>> + /*
> > >>>> + * The VIN4 video input path is shared between
> > >>>> + * CVBS and HDMI inputs through SW[49-54] switches.
> > >>>> + *
> > >>>> + * CVBS is the default selection, link it to VIN4 here.
> > >>>> + */
> > >>>> + adv7180_out: endpoint {
> > >>>> + remote-endpoint = <&vin4_in>;
> > >>>> + };
> > >>>> + };
> > >>>> + };
> > >>>> };
> > >>>>
> > >>>> &i2c1 {
> > >>>> @@ -246,3 +268,23 @@
> > >>>> timeout-sec = <60>;
> > >>>> status = "okay";
> > >>>> };
> >>>> +
> > >>>> +&vin4 {
> > >>>> + pinctrl-0 = <&vin4_pins_cvbs>;
> > >>>> + pinctrl-names = "default";
> > >>>> +
> > >>>> + status = "okay";
> > >>>> +
> > >>>> + ports {
> > >>>> + #address-cells = <1>;
> > >>>> + #size-cells = <0>;
> > >>>> +
> > >>>> + port@0 {
> > >>>> + reg = <0>;
> > >>>> +
> > >>>> + vin4_in: endpoint {
> > >>>> + remote-endpoint = <&adv7180_out>;
> > >>>> + };
> > >>>> + };
> > >>>> + };
> > >>>> +};
>
> --
> Regards,
>
> Laurent Pinchart
>
>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [reset-control] How to initialize hardware state with the shared reset line?
From: Martin Blumenstingl @ 2018-05-24 20:09 UTC (permalink / raw)
To: Philipp Zabel
Cc: Masahiro Yamada, Rob Herring, Lee Jones, Hans de Goede,
Felipe Balbi, DTML, Arnd Bergmann, Linus Walleij,
Linux Kernel Mailing List, linux-arm-kernel, linux-amlogic
In-Reply-To: <1526997879.3671.27.camel@pengutronix.de>
Hi Philipp,
On Tue, May 22, 2018 at 4:04 PM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> Hi Martin,
>
> On Mon, 2018-05-21 at 12:40 +0200, Martin Blumenstingl wrote:
>> Hello,
>>
>> On Mon, May 21, 2018 at 3:27 AM, Masahiro Yamada
>> <yamada.masahiro@socionext.com> wrote:
>> > Hi.
>> >
>> >
>> > 2018-05-20 19:57 GMT+09:00 Martin Blumenstingl
>> > <martin.blumenstingl@googlemail.com>:
>> > > Hi,
>> > >
>> > > On Thu, May 10, 2018 at 11:16 AM, Masahiro Yamada
>> > > <yamada.masahiro@socionext.com> wrote:
>> > > [snip]
>> > > > I may be missing something, but
>> > > > one solution might be reset hogging on the
>> > > > reset provider side. This allows us to describe
>> > > > the initial state of reset lines in the reset controller.
>> > > >
>> > > > The idea for "reset-hog" is similar to:
>> > > > - "gpio-hog" defined in
>> > > > Documentation/devicetree/bindings/gpio/gpio.txt
>> > > > - "assigned-clocks" defined in
>> > > > Documetation/devicetree/bindings/clock/clock-bindings.txt
>> > > >
>> > > >
>> > > >
>> > > > For example,
>> > > >
>> > > > reset-controller {
>> > > > ....
>> > > >
>> > > > line_a {
>> > > > reset-hog;
>> > > > resets = <1>;
>> > > > reset-assert;
>> > > > };
>> > > > }
>> > > >
>> > > >
>> > > > When the reset controller is registered,
>> > > > the reset ID '1' is asserted.
>> > > >
>> > > >
>> > > > So, all reset consumers that share the reset line '1'
>> > > > will start from the asserted state
>> > > > (i.e. defined state machine state).
>> > >
>> > > I wonder if a "reset hog" can be board specific:
>> > > - GPIO hogs are definitely board specific (meson-gxbb-odroidc2.dts for
>> > > example uses it to take the USB hub out of reset)
>> > > - assigned-clock-parents (and the like) can also be board specific (I
>> > > made up a use-case since I don't know of any actual examples: board A
>> > > uses an external XTAL while board B uses some other internal
>> > > clock-source because it doesn't have an external XTAL)
>> > >
>> > > however, can reset lines be board specific? or in other words: do we
>> > > need to describe them in device-tree?
>> >
>> > Indeed.
>> >
>> > I did not come up with board-specific cases.
>> >
>> > The problem we are discussing is SoC-specific,
>> > and reset-controller drivers are definitely SoC-specific.
>> >
>> > So, I think the initial state can be coded in drivers instead of DT.
>>
>> OK, let's also hear Philipp's (reset framework maintainer) opinion on this
>
> I'd like to know if there are other SoC families besides Amlogic Meson
> that potentially could have this problem and about how many of the
> resets that are documented in include/dt-bindings/reset/amlogic,meson*
> we are actually talking. Are all of those initially deasserted and none
> of the connected peripherals have power-on reset mechanisms?
I cannot speak for other SoC families besides Amlogic
Meson8/Meson8b/Meson8m2 and GX (disclaimer: I am a community
contributor, I don't have access to Amlogic's internal datasheets - my
knowledge is based on their public datasheets, their GPL kernel/u-boot
sources and trial and error)
it seems that at least "some" (but I don't know the exact number)
resets are de-asserted by the bootloader
Amlogic's u-boot for example also enables all gate clocks by default
I CC'ed the Amlogic mailing list because I'm not sure if everyone
working on that SoC family is watching the linux-arm-kernel mailing
list
>> > > we could extend struct reset_controller_dev (= reset controller
>> > > driver) if they are not board specific:
>> > > - either assert all reset lines by default except if they are listed
>> > > in a new field (may break backwards compatibility, requires testing of
>> > > all reset controller drivers)
>> >
>> > This is quite simple, but I am afraid there are some cases where the forcible
>> > reset-assert is not preferred.
>> >
>> > For example, the earlycon. When we use earlycon, we would expect it has been
>> > initialized by a boot-loader, or something.
>> > If it is reset-asserted on the while, the console output
>> > will not be good.
>>
>> indeed, so let's skip this idea
>
> Maybe we should at first add initial reset assertion to the Meson driver
> on a case by case bases?
this seems simple enough to test it - we can still generalize this
later on (either by adding support to the reset framework, DT bindings
or something else)
> We can't add required reset hog DT bindings to the Meson reset
> controller anyway without breaking DT backwards compatibility.
>
>> > > - specify a list of reset lines and their desired state (or to keep it
>> > > easy: specify a list of reset lines that should be asserted)
>> > > (I must admit that this is basically your idea but the definition is
>> > > moved from device-tree to the reset controller driver)
>> >
>> > Yes, I think the list of "reset line ID" and "init state" pairs
>> > would be nicer.
>>
>> $ grep -R "of_reset_n_cells = [^1]" drivers/reset/
>> drivers/reset/reset-berlin.c: priv->rcdev.of_reset_n_cells = 2;
>> drivers/reset/hisilicon/reset-hi3660.c: rc->rst.of_reset_n_cells = 2;
>> drivers/reset/reset-ti-sci.c: data->rcdev.of_reset_n_cells = 2;
>> drivers/reset/reset-lantiq.c: priv->rcdev.of_reset_n_cells = 2;
>>
>> everything else uses only one reset cell
>> from the lantiq reset dt-binding documentation: "The first cell takes
>> the reset set bit and the second cell takes the status bit."
>>
>> I'm not sure what to do with drivers that specify != 1 reset-cell
>> though if we use a simple "init state pair"
>> maybe Philipp can share his opinion on this one as well
>
> See above, so far I am not convinced (either way) whether this should be
> described in the DT at all.
>
>> > > any "chip" specific differences could be expressed by using a
>> > > different of_device_id
>> > >
>> > > one the other hand: your "reset hog" solution looks fine to me if
>> > > reset lines can be board specific
>> > >
>> > > > From the discussion with Martin Blumenstingl
>> > > > (https://lkml.org/lkml/2018/4/28/115),
>> > > > the problem for Amlogic is that
>> > > > the reset line is "de-asserted" by default.
>> > > > If so, the 'reset-hog' would fix the problem,
>> > > > and DWC3 driver would be able to use
>> > > > shared, level reset, I think.
>> > >
>> > > I think you are right: if we could control the initial state then we
>> > > should be able to use level resets
>> >
>> >
>> > Even further, can we drop the shared reset_control_reset() support, maybe?
>> > (in other words, revert commit 7da33a37b48f11)
>>
>> I believe we need to keep this if there's hardware out there:
>> - where the reset controller only supports reset pulses
>> - at least one reset line is shared between multiple devices
>>
>> I didn't have a closer look at the Amlogic Meson6 SoC yet, but I think
>> it matches above criteria. as far as I know:
>> - the USB situation there is similar to Meson8b (USB controllers and
>> PHYs share a reset line)
>> - it uses an older reset controller IP block which does not support
>> level resets (only reset pulses)
>
> See my answer to Masahiro's first mail. I think somebody suggested in
> the past to add a fallback from the deassert to the reset op. I think
> this is something that should work in this case.
this is an interesting idea - it should work for Meson6 (in case
mainline ever gains support for this old SoC)
Regards
Martin
^ permalink raw reply
* Re: [PATCH v2 1/8] driver core: make deferring probe after init optional
From: Rob Herring @ 2018-05-24 20:25 UTC (permalink / raw)
To: Mark Brown
Cc: Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
Bjorn Andersson, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
Joerg Roedel, Robin Murphy, Frank Rowand,
linux-kernel@vger.kernel.org, devicetree,
Architecture Mailman List,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <20180524181834.GF4828@sirena.org.uk>
On Thu, May 24, 2018 at 1:18 PM, Mark Brown <broonie@kernel.org> wrote:
> On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:
>
>> Subsystems or drivers may opt-in to this behavior by calling
>> driver_deferred_probe_check_init_done() instead of just returning
>> -EPROBE_DEFER. They may use additional information from DT or kernel's
>> config to decide whether to continue to defer probe or not.
>
> Should userspace have some involvement in this decision? It knows if
> it's got any intention of loading modules for example. Kernel config
> checks might be good enough, though it's going to be a pain to work out
> if the relevant driver is built as a module for example.
I looked into whether we could hook into uevents in some way. If we
knew when all the coldplug events had been handled, that would be
sufficient. But it doesn't look to me like we can tell when that
happens with the uevent netlink socket. I think about the only thing
we can tell is if userspace has opened a socket. I'm not all that
familiar with how the whole sequence works, so other opinions on this
would be helpful.
Also, for this to work with serial consoles, we have to make the
decision before we get to userspace. I couldn't get systemd to create
serial gettys either if we deferred later. There's some dependence on
/dev/console, but I didn't get to the bottom of it.
Rob
^ permalink raw reply
* Re: Re: [PATCH 05/15] drm/sun4i: Add TCON TOP driver
From: Jernej Škrabec @ 2018-05-24 20:33 UTC (permalink / raw)
To: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ
Cc: wens-jdAy2FN1RRM, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-clk-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20180524084351.x4ugbbsz3mqv6fh7@flea>
Hi,
Dne četrtek, 24. maj 2018 ob 10:43:51 CEST je Maxime Ripard napisal(a):
> Hi,
>
> On Mon, May 21, 2018 at 05:15:15PM +0200, Jernej Škrabec wrote:
> > > > + /*
> > > > + * Default register values might have some reserved bits set,
which
> > > > + * prevents TCON TOP from working properly. Set them to 0 here.
> > > > + */
> > > > + writel(0, tcon_top->regs + TCON_TOP_PORT_SEL_REG);
> > > > + writel(0, tcon_top->regs + TCON_TOP_GATE_SRC_REG);
> > > > +
> > > > + for (i = 0; i < CLK_NUM; i++) {
> > > > + const char *parent_name = "bus-tcon-top";
> > >
> > > I guess retrieving the parent's clock name at runtime would be more
> > > flexible.
> >
> > It is, but will it ever be anything else?
>
> Probably not, but when the complexity is exactly the same (using
> __clk_get_name), we'd better use the more appropriate solution. If we
> ever need to change that clock name, or to use the driver with an SoC
> that wouldn't have the same clock name for whatever reason, it will
> just work.
>
> > > > + struct clk_init_data init;
> > > > + struct clk_gate *gate;
> > > > +
> > > > + gate = devm_kzalloc(dev, sizeof(*gate), GFP_KERNEL);
> > > > + if (!gate) {
> > > > + ret = -ENOMEM;
> > > > + goto err_disable_clock;
> > > > + }
> > > > +
> > > > + init.name = gates[i].name;
> > > > + init.ops = &clk_gate_ops;
> > > > + init.flags = CLK_IS_BASIC;
> > > > + init.parent_names = &parent_name;
> > > > + init.num_parents = 1;
> > > > +
> > > > + gate->reg = tcon_top->regs + TCON_TOP_GATE_SRC_REG;
> > > > + gate->bit_idx = gates[i].bit;
> > > > + gate->lock = &tcon_top->reg_lock;
> > > > + gate->hw.init = &init;
> > > > +
> > > > + ret = devm_clk_hw_register(dev, &gate->hw);
> > > > + if (ret)
> > > > + goto err_disable_clock;
> > >
> > > Isn't it what clk_hw_register_gate is doing?
> >
> > Almost, but not exactly. My goal was to use devm_* functions, so there is
> > no need to do any special cleanup.
>
> Is it the only difference? If so, you can just create a
> devm_clk_hw_register gate.
I checked around and it seems that in clk core there are only non devm_*
helpers like clk_hw_register_gate() for some reason. I guess I'll just use
that and manually unregister all the clocks in cleanup function.
Best regards,
Jernej
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* Re: [PATCH v2 1/8] driver core: make deferring probe after init optional
From: Rob Herring @ 2018-05-24 20:57 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Linus Walleij, Alexander Graf, Bjorn Andersson, Rafael J. Wysocki,
Kevin Hilman, Ulf Hansson, Joerg Roedel, Robin Murphy, Mark Brown,
Frank Rowand, linux-kernel@vger.kernel.org, devicetree,
Architecture Mailman List,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <20180524190031.GB31019@kroah.com>
On Thu, May 24, 2018 at 2:00 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:
>> Deferred probe will currently wait forever on dependent devices to probe,
>> but sometimes a driver will never exist. It's also not always critical for
>> a driver to exist. Platforms can rely on default configuration from the
>> bootloader or reset defaults for things such as pinctrl and power domains.
>> This is often the case with initial platform support until various drivers
>> get enabled. There's at least 2 scenarios where deferred probe can render
>> a platform broken. Both involve using a DT which has more devices and
>> dependencies than the kernel supports. The 1st case is a driver may be
>> disabled in the kernel config. The 2nd case is the kernel version may
>> simply not have the dependent driver. This can happen if using a newer DT
>> (provided by firmware perhaps) with a stable kernel version.
>>
>> Subsystems or drivers may opt-in to this behavior by calling
>> driver_deferred_probe_check_init_done() instead of just returning
>> -EPROBE_DEFER. They may use additional information from DT or kernel's
>> config to decide whether to continue to defer probe or not.
>>
>> Cc: Alexander Graf <agraf@suse.de>
>> Signed-off-by: Rob Herring <robh@kernel.org>
>> ---
>> drivers/base/dd.c | 17 +++++++++++++++++
>> include/linux/device.h | 2 ++
>> 2 files changed, 19 insertions(+)
>>
>> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
>> index c9f54089429b..d6034718da6f 100644
>> --- a/drivers/base/dd.c
>> +++ b/drivers/base/dd.c
>> @@ -226,6 +226,16 @@ void device_unblock_probing(void)
>> driver_deferred_probe_trigger();
>> }
>>
>> +int driver_deferred_probe_check_init_done(struct device *dev, bool optional)
>> +{
>> + if (optional && initcalls_done) {
>
> Wait, what's the "optional" mess here?
My intent was that subsystems just always call this function and never
return EPROBE_DEFER themselves. Then the driver core can make
decisions as to what to do (such as the timeout added in the next
patch). Or it can print common error/debug messages. So optional is a
hint to allow subsystems per device control.
>
> The caller knows this value, so why do you need to even pass it in here?
Because regardless of the value, we always stop deferring when/if we
hit the timeout and the caller doesn't know about the timeout. If we
get rid of it, we'd need functions for both init done and for deferred
timeout.
> And bool values that are not obvious are horrid. I had to go look this
> up when reading the later patches that just passed "true" in this
> variable as I had no idea what that meant.
Perhaps inverting it and calling it "keep_deferring" would be better.
However, the flag is ignored if we have timed out.
>
> So as-is, no, this isn't ok, sorry.
>
> And at the least, this needs some kerneldoc to explain it :)
That part is easy enough to fix.
Rob
^ permalink raw reply
* Re: [PATCH 06/15] drm/sun4i: tcon: Add support for tcon-top
From: Chen-Yu Tsai @ 2018-05-24 22:01 UTC (permalink / raw)
To: Maxime Ripard
Cc: Jernej Škrabec, Rob Herring, Mark Rutland, dri-devel,
devicetree, linux-arm-kernel, linux-kernel, linux-clk,
linux-sunxi
In-Reply-To: <20180524085021.qto6266adjvpj6ai@flea>
On Thu, May 24, 2018 at 1:50 AM, Maxime Ripard
<maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote:
> On Mon, May 21, 2018 at 07:27:46PM +0200, Jernej Škrabec wrote:
>> Hi,
>>
>> Dne ponedeljek, 21. maj 2018 ob 10:07:59 CEST je Maxime Ripard napisal(a):
>> > On Sat, May 19, 2018 at 08:31:18PM +0200, Jernej Skrabec wrote:
>> > > If SoC has TCON TOP unit, it has to be configured from TCON, since it
>> > > has all information needed. Additionally, if it is TCON TV, it must also
>> > > enable bus gate inside TCON TOP unit.
>> >
>> > Why?
>>
>> I'll explain my design decision below.
>>
>> >
>> > > Add support for such TCONs.
>> > >
>> > > Signed-off-by: Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
>> > > ---
>> > >
>> > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 28 ++++++++++++++++++++++++++++
>> > > drivers/gpu/drm/sun4i/sun4i_tcon.h | 8 ++++++++
>> > > 2 files changed, 36 insertions(+)
>> > >
>> > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
>> > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 08747fc3ee71..e0c562ce1c22
>> > > 100644
>> > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
>> > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
>> > > @@ -688,6 +688,16 @@ static int sun4i_tcon_init_clocks(struct device *dev,
>> > >
>> > > dev_err(dev, "Couldn't get the TCON bus clock\n");
>> > > return PTR_ERR(tcon->clk);
>> > >
>> > > }
>> > >
>> > > +
>> > > + if (tcon->quirks->needs_tcon_top && tcon->quirks->has_channel_1) {
>> > > + tcon->top_clk = devm_clk_get(dev, "tcon-top");
>> > > + if (IS_ERR(tcon->top_clk)) {
>> > > + dev_err(dev, "Couldn't get the TCON TOP bus clock\n");
>> > > + return PTR_ERR(tcon->top_clk);
>> > > + }
>> > > + clk_prepare_enable(tcon->top_clk);
>> > > + }
>> > > +
>> > >
>> > > clk_prepare_enable(tcon->clk);
>> > >
>> > > if (tcon->quirks->has_channel_0) {
>> > >
>> > > @@ -712,6 +722,7 @@ static int sun4i_tcon_init_clocks(struct device *dev,
>> > >
>> > > static void sun4i_tcon_free_clocks(struct sun4i_tcon *tcon)
>> > > {
>> > >
>> > > clk_disable_unprepare(tcon->clk);
>> > >
>> > > + clk_disable_unprepare(tcon->top_clk);
>> > >
>> > > }
>> > >
>> > > static int sun4i_tcon_init_irq(struct device *dev,
>> > >
>> > > @@ -980,6 +991,23 @@ static int sun4i_tcon_bind(struct device *dev, struct
>> > > device *master,>
>> > > tcon->id = engine->id;
>> > > tcon->quirks = of_device_get_match_data(dev);
>> > >
>> > > + if (tcon->quirks->needs_tcon_top) {
>> > > + struct device_node *np;
>> > > +
>> > > + np = of_parse_phandle(dev->of_node, "allwinner,tcon-top", 0);
>> > > + if (np) {
>> > > + struct platform_device *pdev;
>> > > +
>> > > + pdev = of_find_device_by_node(np);
>> > > + if (pdev)
>> > > + tcon->tcon_top = platform_get_drvdata(pdev);
>> > > + of_node_put(np);
>> > > +
>> > > + if (!tcon->tcon_top)
>> > > + return -EPROBE_DEFER;
>> > > + }
>> > > + }
>> > > +
>> >
>> > I might have missed it, but I've not seen the bindings additions for
>> > that property. This shouldn't really be done that way anyway, instead
>> > of using a direct phandle, you should be using the of-graph, with the
>> > TCON-top sitting where it belongs in the flow of data.
>>
>> Just to answer to the first question, it did describe it in "[PATCH 07/15] dt-
>> bindings: display: sun4i-drm: Add R40 HDMI pipeline".
>>
>> As why I designed it that way - HW representation could be described that way
>> (ASCII art makes sense when fixed width font is used to view it):
>>
>> / LCD0/LVDS0
>> / TCON-LCD0
>> | \ MIPI DSI
>> mixer0 |
>> \ / TCON-LCD1 - LCD1/LVDS1
>> TCON-TOP
>> / \ TCON-TV0 - TVE0/RGB
>> mixer1 | \
>> | TCON-TOP - HDMI
>> | /
>> \ TCON-TV1 - TVE1/RGB
>>
>> This is a bit simplified, since there is also TVE-TOP, which is responsible
>> for sharing 4 DACs between both TVE encoders. You can have two TV outs (PAL/
>> NTSC) or TVE0 as TV out and TVE1 as RGB or vice versa. It even seems that you
>> can arbitrarly choose which DAC is responsible for which signal, so there is a
>> ton of possible end combinations, but I'm not 100% sure.
>>
>> Even though I wrote TCON-TOP twice, this is same unit in HW. R40 manual
>> suggest more possibilities, although some of them seem wrong, like RGB feeding
>> from LCD TCON. That is confirmed to be wrong when checking BSP code.
>>
>> Additionally, TCON-TOP comes in the middle of TVE0 and LCD0, TVE1 and LCD1 for
>> pin muxing, although I'm not sure why is that needed at all, since according
>> to R40 datasheet, TVE0 and TVE1 pins are dedicated and not on PORT D and PORT
>> H, respectively, as TCON-TOP documentation suggest. However, HSYNC and PSYNC
>> lines might be shared between TVE (when it works in RGB mode) and LCD. But
>> that is just my guess since I'm not really familiar with RGB and LCD
>> interfaces.
>>
>> I'm really not sure what would be the best representation in OF-graph. Can you
>> suggest one?
>
> Rob might disagree on this one, but I don't see anything wrong with
> having loops in the graph. If the TCON-TOP can be both the input and
> output of the TCONs, then so be it, and have it described that way in
> the graph.
>
> The code is already able to filter out nodes that have already been
> added to the list of devices we need to wait for in the component
> framework, so that should work as well.
>
> And we'd need to describe TVE-TOP as well, even though we don't have a
> driver for it yet. That will simplify the backward compatibility later
> on.
I'm getting the feeling that TCON-TOP / TVE-TOP is the glue layer that
binds everything together, and provides signal routing, kind of like
DE-TOP on A64. So the signal mux controls that were originally found
in TCON0 and TVE0 were moved out.
The driver needs to know about that, but the graph about doesn't make
much sense directly. Without looking at the manual, I understand it to
likely be one mux between the mixers and TCONs, and one between the
TCON-TVs and HDMI. Would it make more sense to just have the graph
connections between the muxed components, and remove TCON-TOP from
it, like we had in the past? A phandle could be used to reference
the TCON-TOP for mux controls, in addition to the clocks and resets.
For TVE, we would need something to represent each of the output pins,
so the device tree can actually describe what kind of signal, be it
each component of RGB/YUV or composite video, is wanted on each pin,
if any. This is also needed on the A20 for the Cubietruck, so we can
describe which pins are tied to the VGA connector, and which one does
R, G, or B.
Regards
ChenYu
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* Re: [PATCH v2 1/8] driver core: make deferring probe after init optional
From: Bjorn Andersson @ 2018-05-24 22:28 UTC (permalink / raw)
To: Rob Herring, Arnd Bergmann
Cc: Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
Rafael J. Wysocki, Kevin Hilman, Ulf Hansson, Joerg Roedel,
Robin Murphy, Mark Brown, Frank Rowand, linux-kernel, devicetree,
boot-architecture, linux-arm-kernel
In-Reply-To: <20180524175024.19874-2-robh@kernel.org>
On Thu 24 May 10:50 PDT 2018, Rob Herring wrote:
> Deferred probe will currently wait forever on dependent devices to probe,
> but sometimes a driver will never exist. It's also not always critical for
> a driver to exist. Platforms can rely on default configuration from the
> bootloader or reset defaults for things such as pinctrl and power domains.
> This is often the case with initial platform support until various drivers
> get enabled. There's at least 2 scenarios where deferred probe can render
> a platform broken. Both involve using a DT which has more devices and
> dependencies than the kernel supports. The 1st case is a driver may be
> disabled in the kernel config. The 2nd case is the kernel version may
> simply not have the dependent driver. This can happen if using a newer DT
> (provided by firmware perhaps) with a stable kernel version.
>
> Subsystems or drivers may opt-in to this behavior by calling
> driver_deferred_probe_check_init_done() instead of just returning
> -EPROBE_DEFER. They may use additional information from DT or kernel's
> config to decide whether to continue to defer probe or not.
>
For builtin drivers this still looks reasonable.
But I would like to have an additional clarification here stating that
drivers that might be targeted by this query must not be compiled as
modules.
And I would prefer to see an ack from e.g. Arnd that arm-soc is okay
that we drop "tristate" on drivers affected by this; e.g. if we put this
in the pinctrl core then all pinctrl drivers should be "bool" and so
should any i2c, ssbi and spmi buses and drivers be.
Regards,
Bjorn
> Cc: Alexander Graf <agraf@suse.de>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> drivers/base/dd.c | 17 +++++++++++++++++
> include/linux/device.h | 2 ++
> 2 files changed, 19 insertions(+)
>
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index c9f54089429b..d6034718da6f 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -226,6 +226,16 @@ void device_unblock_probing(void)
> driver_deferred_probe_trigger();
> }
>
> +int driver_deferred_probe_check_init_done(struct device *dev, bool optional)
> +{
> + if (optional && initcalls_done) {
> + dev_WARN(dev, "ignoring dependency for device, assuming no driver");
> + return -ENODEV;
> + }
> +
> + return -EPROBE_DEFER;
> +}
> +
> /**
> * deferred_probe_initcall() - Enable probing of deferred devices
> *
> @@ -240,6 +250,13 @@ static int deferred_probe_initcall(void)
> /* Sort as many dependencies as possible before exiting initcalls */
> flush_work(&deferred_probe_work);
> initcalls_done = true;
> +
> + /*
> + * Trigger deferred probe again, this time we won't defer anything
> + * that is optional
> + */
> + driver_deferred_probe_trigger();
> + flush_work(&deferred_probe_work);
> return 0;
> }
> late_initcall(deferred_probe_initcall);
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 477956990f5e..f3dafd44c285 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -334,6 +334,8 @@ struct device *driver_find_device(struct device_driver *drv,
> struct device *start, void *data,
> int (*match)(struct device *dev, void *data));
>
> +int driver_deferred_probe_check_init_done(struct device *dev, bool optional);
> +
> /**
> * struct subsys_interface - interfaces to device functions
> * @name: name of the device function
> --
> 2.17.0
>
^ permalink raw reply
* [PATCH] arm64: dts: qcom: msm8996: Use UFS_GDSC for UFS
From: Bjorn Andersson @ 2018-05-24 22:31 UTC (permalink / raw)
To: Andy Gross, Manu Gautam, Vivek Gautam
Cc: linux-arm-msm, linux-soc, devicetree, linux-arm-kernel,
linux-kernel
The UFS host controller occationally (20%) fails to enable
gcc_ufs_axi_clk because the UFS GDSC is not enabled. In most cases it's
enabled through the UFS phy driver, but to make sure it's enabled let's
enable it directly from the UFS host controller directly as well.
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
arch/arm64/boot/dts/qcom/msm8996.dtsi | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index 380e14591686..03c7904bda14 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -674,6 +674,8 @@
vccq-max-microamp = <450000>;
vccq2-max-microamp = <450000>;
+ power-domains = <&gcc UFS_GDSC>;
+
clock-names =
"core_clk_src",
"core_clk",
--
2.17.0
^ permalink raw reply related
* Re: [RESEND PATCH 2/5] mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver
From: Stefan Agner @ 2018-05-24 22:56 UTC (permalink / raw)
To: Boris Brezillon
Cc: dwmw2, computersforpeace, marek.vasut, robh+dt, mark.rutland,
thierry.reding, mturquette, sboyd, dev, miquel.raynal, richard,
marcel, krzk, digetx, benjamin.lindqvist, jonathanh, pdeschrijver,
pgaikwad, mirza.krak, linux-mtd, linux-tegra, devicetree,
linux-kernel, linux-clk
In-Reply-To: <20180524144134.41a71063@bbrezillon>
On 24.05.2018 14:41, Boris Brezillon wrote:
> On Thu, 24 May 2018 14:23:56 +0200
> Boris Brezillon <boris.brezillon@bootlin.com> wrote:
>
>> On Thu, 24 May 2018 13:09:53 +0200
>> Stefan Agner <stefan@agner.ch> wrote:
>>
>> > On 24.05.2018 10:56, Boris Brezillon wrote:
>> > > On Thu, 24 May 2018 10:46:27 +0200
>> > > Stefan Agner <stefan@agner.ch> wrote:
>> > >
>> > >> Hi Boris,
>> > >>
>> > >> Thanks for the initial review! One small question below:
>> > >>
>> > >> On 23.05.2018 16:18, Boris Brezillon wrote:
>> > >> > Hi Stefan,
>> > >> >
>> > >> > On Tue, 22 May 2018 14:07:06 +0200
>> > >> > Stefan Agner <stefan@agner.ch> wrote:
>> > >> >> +
>> > >> >> +struct tegra_nand {
>> > >> >> + void __iomem *regs;
>> > >> >> + struct clk *clk;
>> > >> >> + struct gpio_desc *wp_gpio;
>> > >> >> +
>> > >> >> + struct nand_chip chip;
>> > >> >> + struct device *dev;
>> > >> >> +
>> > >> >> + struct completion command_complete;
>> > >> >> + struct completion dma_complete;
>> > >> >> + bool last_read_error;
>> > >> >> +
>> > >> >> + dma_addr_t data_dma;
>> > >> >> + void *data_buf;
>> > >> >> + dma_addr_t oob_dma;
>> > >> >> + void *oob_buf;
>> > >> >> +
>> > >> >> + int cur_chip;
>> > >> >> +};
>> > >> >
>> > >> > This struct should be split in 2 structures: one representing the NAND
>> > >> > controller and one representing the NAND chip:
>> > >> >
>> > >> > struct tegra_nand_controller {
>> > >> > struct nand_hw_control base;
>> > >> > void __iomem *regs;
>> > >> > struct clk *clk;
>> > >> > struct device *dev;
>> > >> > struct completion command_complete;
>> > >> > struct completion dma_complete;
>> > >> > bool last_read_error;
>> > >> > int cur_chip;
>> > >> > };
>> > >> >
>> > >> > struct tegra_nand {
>> > >> > struct nand_chip base;
>> > >> > dma_addr_t data_dma;
>> > >> > void *data_buf;
>> > >> > dma_addr_t oob_dma;
>> > >> > void *oob_buf;
>> > >> > };
>> > >>
>> > >> Is there a particular reason why you would leave DMA buffers in the chip
>> > >> structure? It seems that is more a controller thing...
>> > >
>> > > The size of those buffers is likely to be device dependent, so if you
>> > > have several NANDs connected to the controller, you'll either have to
>> > > have one buffer at the controller level which is max(all-chip-buf-size)
>> > > or a buffer per device.
>> > >
>> > > Also, do you really need these buffers? The core already provide some
>> > > which are suitable for DMA (chip->oob_poi and chip->data_buf).
>> > >
>> >
>> > Good question, I am not sure, that was existing code.
>> >
>> > Are you sure data_buf it is DMA capable?
>> >
>> > nand_scan_tail allocates with kmalloc:
>> >
>> > chip->data_buf = kmalloc(mtd->writesize + mtd->oobsize, GFP_KERNEL);
>>
>> Yes, kmalloc() allocates DMA-able buffers, so those are DMA-safe.
>
> Hm, that's not exactly true. It depends on the dma_mask attached to the
> device.
It seems to work (tm).
I am not sure how to deal with the OOB buffer. I now use the given
pointer also for oob (offset writesize). I think mtk_nand does the same
thing.
dma_len = mtd->writesize + (oob_required ? mtd->oobsize : 0);
dma_addr = dma_map_single(ctrl->dev, buf, dma_len, DMA_FROM_DEVICE);
...
Is there a test which allows to test my (read|write)_page implementation
with oob_required set?
--
Stefan
^ permalink raw reply
* Re: [PATCH v2 1/3] input: touchscreen: edt-ft5x06: don't make device a wakeup source by default
From: Dmitry Torokhov @ 2018-05-24 23:17 UTC (permalink / raw)
To: Rob Herring
Cc: Mark Rutland, devicetree,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Sascha Hauer, linux-input, Fabio Estevam, Shawn Guo, Daniel Mack
In-Reply-To: <CAL_JsqJQt6vFqH-KzNvjW+FZXCgzNAwXj_nSYow2Uz2Vcmi_Cg@mail.gmail.com>
On Wed, May 23, 2018 at 09:45:05AM -0500, Rob Herring wrote:
> On Wed, May 23, 2018 at 3:27 AM, Daniel Mack <daniel@zonque.org> wrote:
> > On Tuesday, May 22, 2018 07:54 PM, Rob Herring wrote:
> >>
> >> On Thu, May 17, 2018 at 11:05:50AM +0200, Daniel Mack wrote:
> >>>
> >>> Allow configuring the device as wakeup source through device properties,
> >>> as
> >>> not all platforms want to wake up on touch screen activity.
> >>>
> >>> The I2C core automatically reads the "wakeup-source" DT property to
> >>> configure a device's wakeup capability, and board supports files can set
> >>> I2C_CLIENT_WAKE in the flags.
> >>
> >>
> >> This will break wake-up on working systems. Looks like mostly i.MX, but
> >> there's one AM437x board. If that board doesn't care, then it is up to
> >> Shawn.
> >
> >
> > I added the property to the dts files, but as Dmitry pointed out, I missed
> > some. Sorry for that.
>
> Just adding the property to dts files doesn't fix the compatibility
> problem. If a user uses an existing dtb (before this change) with a
> new kernel (after this change), then wakeup will stop working.
Is this a practical problem though? Do we know of any products with
this touch panel that use DTS not distributed with the kernel?
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2 1/8] driver core: make deferring probe after init optional
From: Rob Herring @ 2018-05-24 23:47 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Arnd Bergmann, Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
Rafael J. Wysocki, Kevin Hilman, Ulf Hansson, Joerg Roedel,
Robin Murphy, Mark Brown, Frank Rowand,
linux-kernel@vger.kernel.org, devicetree,
Architecture Mailman List,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <20180524222855.GD14924@minitux>
On Thu, May 24, 2018 at 5:28 PM, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
> On Thu 24 May 10:50 PDT 2018, Rob Herring wrote:
>
>> Deferred probe will currently wait forever on dependent devices to probe,
>> but sometimes a driver will never exist. It's also not always critical for
>> a driver to exist. Platforms can rely on default configuration from the
>> bootloader or reset defaults for things such as pinctrl and power domains.
>> This is often the case with initial platform support until various drivers
>> get enabled. There's at least 2 scenarios where deferred probe can render
>> a platform broken. Both involve using a DT which has more devices and
>> dependencies than the kernel supports. The 1st case is a driver may be
>> disabled in the kernel config. The 2nd case is the kernel version may
>> simply not have the dependent driver. This can happen if using a newer DT
>> (provided by firmware perhaps) with a stable kernel version.
>>
>> Subsystems or drivers may opt-in to this behavior by calling
>> driver_deferred_probe_check_init_done() instead of just returning
>> -EPROBE_DEFER. They may use additional information from DT or kernel's
>> config to decide whether to continue to defer probe or not.
>>
>
> For builtin drivers this still looks reasonable.
>
> But I would like to have an additional clarification here stating that
> drivers that might be targeted by this query must not be compiled as
> modules.
>
> And I would prefer to see an ack from e.g. Arnd that arm-soc is okay
> that we drop "tristate" on drivers affected by this; e.g. if we put this
> in the pinctrl core then all pinctrl drivers should be "bool" and so
> should any i2c, ssbi and spmi buses and drivers be.
For pinctrl, you are not affected now unless you change your DT. I
made pinctrl opt-in since as you said it could lead to some
intermittent problems. If iommu or power domains are required, then it
should fail pretty hard.
However, good luck getting your serial console to work if it has a
pinctrl dependency and that's a module. It won't work because the
console has to be up before userspace. Of course, you can just rely on
earlycon, but that just proves that pinctrl is not required (at least
for 1 serial pin). :)
Rob
>
> Regards,
> Bjorn
>
>> Cc: Alexander Graf <agraf@suse.de>
>> Signed-off-by: Rob Herring <robh@kernel.org>
>> ---
>> drivers/base/dd.c | 17 +++++++++++++++++
>> include/linux/device.h | 2 ++
>> 2 files changed, 19 insertions(+)
>>
>> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
>> index c9f54089429b..d6034718da6f 100644
>> --- a/drivers/base/dd.c
>> +++ b/drivers/base/dd.c
>> @@ -226,6 +226,16 @@ void device_unblock_probing(void)
>> driver_deferred_probe_trigger();
>> }
>>
>> +int driver_deferred_probe_check_init_done(struct device *dev, bool optional)
>> +{
>> + if (optional && initcalls_done) {
>> + dev_WARN(dev, "ignoring dependency for device, assuming no driver");
>> + return -ENODEV;
>> + }
>> +
>> + return -EPROBE_DEFER;
>> +}
>> +
>> /**
>> * deferred_probe_initcall() - Enable probing of deferred devices
>> *
>> @@ -240,6 +250,13 @@ static int deferred_probe_initcall(void)
>> /* Sort as many dependencies as possible before exiting initcalls */
>> flush_work(&deferred_probe_work);
>> initcalls_done = true;
>> +
>> + /*
>> + * Trigger deferred probe again, this time we won't defer anything
>> + * that is optional
>> + */
>> + driver_deferred_probe_trigger();
>> + flush_work(&deferred_probe_work);
>> return 0;
>> }
>> late_initcall(deferred_probe_initcall);
>> diff --git a/include/linux/device.h b/include/linux/device.h
>> index 477956990f5e..f3dafd44c285 100644
>> --- a/include/linux/device.h
>> +++ b/include/linux/device.h
>> @@ -334,6 +334,8 @@ struct device *driver_find_device(struct device_driver *drv,
>> struct device *start, void *data,
>> int (*match)(struct device *dev, void *data));
>>
>> +int driver_deferred_probe_check_init_done(struct device *dev, bool optional);
>> +
>> /**
>> * struct subsys_interface - interfaces to device functions
>> * @name: name of the device function
>> --
>> 2.17.0
>>
^ permalink raw reply
* Re: [PATCH v5 1/3] phy: Power on PHY before start Serdes configuration
From: cang @ 2018-05-25 1:32 UTC (permalink / raw)
To: Vivek Gautam
Cc: subhashj, asutoshd, mgautam, kishon, robh+dt, mark.rutland,
linux-kernel, devicetree
In-Reply-To: <dd6b1e3d-e894-e40f-452a-2dec2421b1c7@codeaurora.org>
On 2018-05-24 16:17, Vivek Gautam wrote:
> Hi Can,
>
>
> On 5/23/2018 9:17 AM, Can Guo wrote:
>> PHYs should be powered on before register configuration starts.
>>
>> Signed-off-by: Can Guo <cang@codeaurora.org>
>> ---
>
> Thanks for fixing this.
>
>> drivers/phy/qualcomm/phy-qcom-qmp.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c
>> b/drivers/phy/qualcomm/phy-qcom-qmp.c
>> index 97ef942..9bfdba1 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-qmp.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
>> @@ -1000,6 +1000,12 @@ static int qcom_qmp_phy_com_init(struct
>> qcom_qmp *qmp)
>> SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);
>> }
>> + /*
>> + * Pull out PHY from POWER DOWN state.
>> + * This is active low enable signal to power-down PHY.
>> + */
>> + qphy_setbits(pcs, QPHY_POWER_DOWN_CONTROL, cfg->pwrdn_ctrl);
>> +
>
> Thanks. This is in sync with the requirements of USB and UFS phys
> across platforms
> using this qmp phy driver, viz. 8996 and 845.
> However, as discussed with you offline the PCIe phy has different
> requirement.
> PCIe phy on 8996 doesn't need the QPHY_POWER_DOWN_CONTROL (pcs level
> power down)
> before configuring the phys. Rather COM_POWER_DOWN_CONTROL is enough.
> It needs the QPHY_POWER_DOWN_CONTROL after programming the
> serdes, tx, rx, and pcs blocks, and right before doing SW_RESET, and
> START_CONTROL.
>
> So we should just do the above QPHY_POWER_DOWN_CONTROL in
> qcom_qmp_phy_com_init() only for non-PCIe phys at the moment, and
> skip the QPHY_POWER_DOWN_CONTROL in qcom_qmp_phy_init()
> for these non-PCIe phys.
>
> Thanks
> Vivek
>
Sure Vivek, I shall make the change per we talked.
Thanks
Can
>> /* Serdes configuration */
>> qcom_qmp_phy_configure(serdes, cfg->regs, cfg->serdes_tbl,
>> cfg->serdes_tbl_num);
^ permalink raw reply
* [PATCH v3 0/8] Add support for mediatek SOC MT2712
From: stu.hsieh @ 2018-05-25 2:34 UTC (permalink / raw)
To: CK Hu, Philipp Zabel
Cc: Mark Rutland, devicetree, srv_heupstream, David Airlie,
linux-kernel, dri-devel, Rob Herring, linux-mediatek, Stu Hsieh,
Matthias Brugger, linux-arm-kernel
From: Stu Hsieh <stu.hsieh@mediatek.com>
This patch add support for the Mediatek MT2712 DISP subsystem.
MT2712 is base on MT8173, there are some difference as following:
MT2712 support three disp output(two ovl and one rdma)
Change in v3:
- Added patch for ddp component AAL1
- Added patch for ddp component OD1
- Added patch for ddp component PWM2
- Added patch to create third ddp path
- Rebase patch "support maximum 64 mutex mod" before
"Add support for mediatek SOC MT2712"
- Rebase patch "add connection from OD1 to RDMA1" before
"Add support for mediatek SOC MT2712"
- Remove two definition about RDMA0 and RDMA2
- Change the definition about mutex module from
bitwise to index
Changes in v2:
- update dt-bindings for mt2712
- Added patch to connection from OD1 to RDMA1
- Added patch to support maximum 64 mutex mod
- rewrite mutex mod condition for reducing one byte
- Change the component name AAL/OD to AAL0/OD0 for naming consistency
- Move the compatible infomation about dpi to other patch which modify
the dpi driver for mt2712
Stu Hsieh (8):
drm/mediatek: update dt-bindings for mt2712
drm/mediatek: support maximum 64 mutex mod
drm/mediatek: add connection from OD1 to RDMA1
drm/mediatek: add ddp component AAL1
drm/mediatek: add ddp component OD1
drm/mediatek: add ddp component PWM2
drm/mediatek: Add support for mediatek SOC MT2712
drm/mediatek: add third ddp path
.../bindings/display/mediatek/mediatek,disp.txt | 2 +-
drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 124 +++++++++++++++------
drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 8 +-
drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 7 +-
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 47 +++++++-
drivers/gpu/drm/mediatek/mtk_drm_drv.h | 7 +-
6 files changed, 155 insertions(+), 40 deletions(-)
--
2.12.5
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* [PATCH v3 1/8] drm/mediatek: update dt-bindings for mt2712
From: stu.hsieh @ 2018-05-25 2:34 UTC (permalink / raw)
To: CK Hu, Philipp Zabel
Cc: Mark Rutland, devicetree, srv_heupstream, David Airlie,
linux-kernel, dri-devel, Rob Herring, linux-mediatek, Stu Hsieh,
Matthias Brugger, linux-arm-kernel
In-Reply-To: <1527215665-11937-1-git-send-email-stu.hsieh@mediatek.com>
From: Stu Hsieh <stu.hsieh@mediatek.com>
Update device tree binding documentation for the display subsystem for
Mediatek MT2712 SoCs.
Signed-off-by: Stu Hsieh <stu.hsieh@mediatek.com>
---
Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt b/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt
index 383183a89164..8469de510001 100644
--- a/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt
+++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt
@@ -40,7 +40,7 @@ Required properties (all function blocks):
"mediatek,<chip>-dpi" - DPI controller, see mediatek,dpi.txt
"mediatek,<chip>-disp-mutex" - display mutex
"mediatek,<chip>-disp-od" - overdrive
- the supported chips are mt2701 and mt8173.
+ the supported chips are mt2701, mt2712 and mt8173.
- reg: Physical base address and length of the function block register space
- interrupts: The interrupt signal from the function block (required, except for
merge and split function blocks).
--
2.12.5
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox