* Re: [RFC][PATCH 1/2] Add a layer of abstraction to use the memory reserved by device tree for ima buffer pass.
From: Rob Herring @ 2020-05-12 23:09 UTC (permalink / raw)
To: Prakhar Srivastava
Cc: linux-arm-kernel, linux-kernel, linuxppc-dev, devicetree,
linux-integrity, linux-security-module, catalin.marinas, will,
mpe, benh, paulus, frowand.list, zohar, dmitry.kasatkin, jmorris,
serge, pasha.tatashin, allison, kstewart, takahiro.akashi, tglx,
vincenzo.frascino, mark.rutland, masahiroy, james.morse, bhsharma,
mbrugger, hsinyi, tao.li, christophe.leroy, gregkh, nramas,
tusharsu, balajib
In-Reply-To: <20200504203829.6330-2-prsriva@linux.microsoft.com>
On Mon, May 04, 2020 at 01:38:28PM -0700, Prakhar Srivastava wrote:
> Introduce a device tree layer for to read and store ima buffer
> from the reserved memory section of a device tree.
But why do I need 'a layer of abstraction'? I don't like them.
> Signed-off-by: Prakhar Srivastava <prsriva@linux.microsoft.com>
> ---
> drivers/of/Kconfig | 6 ++
> drivers/of/Makefile | 1 +
> drivers/of/of_ima.c | 165 ++++++++++++++++++++++++++++++++++++++++++++
Who are the users of this code and why does it need to be here? Most
code for specific bindings are not in drivers/of/ but with the user. It
doesn't sound like there's more than 1 user.
> include/linux/of.h | 34 +++++++++
> 4 files changed, 206 insertions(+)
> create mode 100644 drivers/of/of_ima.c
^ permalink raw reply
* Re: [PATCH v2 1/4] dt-bindings: input: gpio-vibrator: Don't require enable-gpios
From: Ondřej Jirman @ 2020-05-12 23:05 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-sunxi, Rob Herring, Mark Rutland, Maxime Ripard,
Chen-Yu Tsai, Luca Weiss, Tomas Novotny, linux-input, devicetree,
linux-kernel, linux-arm-kernel, Rob Herring
In-Reply-To: <20200512225212.GE89269@dtor-ws>
On Tue, May 12, 2020 at 03:52:12PM -0700, Dmitry Torokhov wrote:
> On Wed, May 13, 2020 at 12:22:02AM +0200, Ondrej Jirman wrote:
> > It is possible to turn the motor on/off just by enabling/disabling
> > the vcc-supply.
> >
> > Signed-off-by: Ondrej Jirman <megous@megous.com>
> > Acked-by: Rob Herring <robh@kernel.org>
> > ---
> > Documentation/devicetree/bindings/input/gpio-vibrator.yaml | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/input/gpio-vibrator.yaml b/Documentation/devicetree/bindings/input/gpio-vibrator.yaml
> > index 2384465eaa19..c700b640bd53 100644
> > --- a/Documentation/devicetree/bindings/input/gpio-vibrator.yaml
> > +++ b/Documentation/devicetree/bindings/input/gpio-vibrator.yaml
> > @@ -24,7 +24,6 @@ properties:
> >
> > required:
> > - compatible
> > - - enable-gpios
>
> Hmm we need at least one of the 2 (gpio and supply). Should we encode it
> in the binding?
Not sure how to encode either one or the other property being required, but
not both at once.
Maybe I can add a supply-vibrator compatible to the driver and binding and
make requirements dependent on the compatible?
> Also, in the dirver code, I guess we need to switch to have regulator
> optional (so we are not given the dummy one) and bail if neither
> regulator nor GPIO is found.
Though nothing bad will happen in the driver if binding will lack both
of these. The driver will just not control any HW, so at least it's
failsafe as is.
regards,
o.
> Thanks.
>
> --
> Dmitry
^ permalink raw reply
* Re: [RFC][PATCH 0/2] Add support for using reserved memory for ima buffer pass
From: Rob Herring @ 2020-05-12 23:05 UTC (permalink / raw)
To: Prakhar Srivastava
Cc: Mark Rutland, linux-arm-kernel, linux-kernel, linuxppc-dev,
devicetree, linux-integrity, linux-security-module,
catalin.marinas, will, mpe, benh, paulus, frowand.list, zohar,
dmitry.kasatkin, jmorris, serge, pasha.tatashin, allison,
kstewart, takahiro.akashi, tglx, vincenzo.frascino, masahiroy,
james.morse, bhsharma, mbrugger, hsinyi, tao.li, christophe.leroy,
gregkh, nramas, tusharsu, balajib
In-Reply-To: <e8c7d74e-74bf-caa3-452d-23faa649e825@linux.microsoft.com>
On Wed, May 06, 2020 at 10:50:04PM -0700, Prakhar Srivastava wrote:
> Hi Mark,
Please don't top post.
> This patch set currently only address the Pure DT implementation.
> EFI and ACPI implementations will be posted in subsequent patchsets.
>
> The logs are intended to be carried over the kexec and once read the
> logs are no longer needed and in prior conversation with James(
> https://lore.kernel.org/linux-arm-kernel/0053eb68-0905-4679-c97a-00c5cb6f1abb@arm.com/)
> the apporach of using a chosen node doesn't
> support the case.
>
> The DT entries make the reservation permanent and thus doesnt need kernel
> segments to be used for this, however using a chosen-node with
> reserved memory only changes the node information but memory still is
> reserved via reserved-memory section.
I think Mark's point was whether it needs to be permanent. We don't
hardcode the initrd address for example.
> On 5/5/20 2:59 AM, Mark Rutland wrote:
> > Hi Prakhar,
> >
> > On Mon, May 04, 2020 at 01:38:27PM -0700, Prakhar Srivastava wrote:
> > > IMA during kexec(kexec file load) verifies the kernel signature and measures
What's IMA?
> > > the signature of the kernel. The signature in the logs can be used to verfiy the
> > > authenticity of the kernel. The logs don not get carried over kexec and thus
> > > remote attesation cannot verify the signature of the running kernel.
> > >
> > > Introduce an ABI to carry forward the ima logs over kexec.
> > > Memory reserved via device tree reservation can be used to store and read
> > > via the of_* functions.
> >
> > This flow needs to work for:
> >
> > 1) Pure DT
> > 2) DT + EFI memory map
> > 3) ACPI + EFI memory map
> >
> > ... and if this is just for transiently passing the log, I don't think
> > that a reserved memory region is the right thing to use, since they're
> > supposed to be more permanent.
> >
> > This sounds analogous to passing the initrd, and should probably use
> > properties under the chosen node (which can be used for all three boot
> > flows above).
> >
> > For reference, how big is the IMA log likely to be? Does it need
> > physically contiguous space?
>
> It purely depends on the policy used and the modules/files that are accessed
> for my local testing over a kexec session the log in
> about 30KB.
>
> Current implementation expects enough contiguous memory to allocated to
> carry forward the logs. If the log size exceeds the reserved memory the
> call will fail.
>
> Thanks,
> Prakhar Srivastava
> >
> > Thanks,
> > Mark.
> >
> > >
> > > Reserved memory stores the size(sizeof(size_t)) of the buffer in the starting
> > > address, followed by the IMA log contents.
> > >
> > > Tested on:
> > > arm64 with Uboot
> > >
> > > Prakhar Srivastava (2):
> > > Add a layer of abstraction to use the memory reserved by device tree
> > > for ima buffer pass.
> > > Add support for ima buffer pass using reserved memory for arm64 kexec.
> > > Update the arch sepcific code path in kexec file load to store the
> > > ima buffer in the reserved memory. The same reserved memory is read
> > > on kexec or cold boot.
> > >
> > > arch/arm64/Kconfig | 1 +
> > > arch/arm64/include/asm/ima.h | 22 ++++
> > > arch/arm64/include/asm/kexec.h | 5 +
> > > arch/arm64/kernel/Makefile | 1 +
> > > arch/arm64/kernel/ima_kexec.c | 64 ++++++++++
> > > arch/arm64/kernel/machine_kexec_file.c | 1 +
> > > arch/powerpc/include/asm/ima.h | 3 +-
> > > arch/powerpc/kexec/ima.c | 14 ++-
> > > drivers/of/Kconfig | 6 +
> > > drivers/of/Makefile | 1 +
> > > drivers/of/of_ima.c | 165 +++++++++++++++++++++++++
> > > include/linux/of.h | 34 +++++
> > > security/integrity/ima/ima_kexec.c | 15 ++-
> > > 13 files changed, 325 insertions(+), 7 deletions(-)
> > > create mode 100644 arch/arm64/include/asm/ima.h
> > > create mode 100644 arch/arm64/kernel/ima_kexec.c
> > > create mode 100644 drivers/of/of_ima.c
> > >
> > > --
> > > 2.25.1
> > >
^ permalink raw reply
* Re: [RFC v1 0/3] Add QTI QFPROM-Efuse driver support
From: Doug Anderson @ 2020-05-12 23:03 UTC (permalink / raw)
To: Ravi Kumar Bokka
Cc: Srinivas Kandagatla, Rob Herring, LKML,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Rajendra Nayak, Sai Prakash Ranjan, dhavalp, mturney, sparate,
c_rbokka, mkurumel
In-Reply-To: <1589307480-27508-1-git-send-email-rbokka@codeaurora.org>
Hi,
On Tue, May 12, 2020 at 11:18 AM Ravi Kumar Bokka <rbokka@codeaurora.org> wrote:
>
> This patch series adds qfprom-efuse controller driver support.
>
> This driver can access the raw qfprom regions for fuse blowing.
>
> The current existed qfprom driver is only supports for cpufreq, thermal sensors
> drivers by read out calibration data, speed bins..etc which is stored by
> qfprom efuses.
I don't understand the interaction between this driver and the
existing "qcom,qfprom" driver. Can you please explain? Are they both
acting on the same values and this one has write access? Are there
two instances of the same hardware block and you're managing one of
them with this new driver and one with thue old driver? Something
else?
> Ravi Kumar Bokka (3):
> dt-bindings: nvmem: Add devicetree bindings for qfprom-efuse
> drivers: nvmem: Add driver for QTI qfprom-efuse support
> arm64: dts: qcom: sc7180: Add qfprom-efuse
>
> .../devicetree/bindings/nvmem/qfprom-efuse.yaml | 40 ++
> arch/arm64/boot/dts/qcom/sc7180-idp.dts | 4 +
> arch/arm64/boot/dts/qcom/sc7180.dtsi | 9 +
> drivers/nvmem/Kconfig | 10 +
> drivers/nvmem/Makefile | 2 +
> drivers/nvmem/qfprom-efuse.c | 476 +++++++++++++++++++++
> 6 files changed, 541 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/nvmem/qfprom-efuse.yaml
> create mode 100644 drivers/nvmem/qfprom-efuse.c
>
> --
> Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by the Linux Foundation.
>
^ permalink raw reply
* Re: [RFC v1 1/3] dt-bindings: nvmem: Add devicetree bindings for qfprom-efuse
From: Doug Anderson @ 2020-05-12 23:03 UTC (permalink / raw)
To: Ravi Kumar Bokka
Cc: Srinivas Kandagatla, Rob Herring, LKML,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Rajendra Nayak, Sai Prakash Ranjan, dhavalp, mturney, sparate,
c_rbokka, mkurumel
In-Reply-To: <1589307480-27508-2-git-send-email-rbokka@codeaurora.org>
Hi,
On Tue, May 12, 2020 at 11:18 AM Ravi Kumar Bokka <rbokka@codeaurora.org> wrote:
>
> This patch adds dt-bindings document for qfprom-efuse controller.
>
> Signed-off-by: Ravi Kumar Bokka <rbokka@codeaurora.org>
> ---
> .../devicetree/bindings/nvmem/qfprom-efuse.yaml | 40 ++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/nvmem/qfprom-efuse.yaml
>
> diff --git a/Documentation/devicetree/bindings/nvmem/qfprom-efuse.yaml b/Documentation/devicetree/bindings/nvmem/qfprom-efuse.yaml
> new file mode 100644
> index 0000000..d262c99
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/nvmem/qfprom-efuse.yaml
> @@ -0,0 +1,40 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/nvmem/qfprom-efuse.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Technologies Inc, QFPROM Efuse bindings
> +
> +maintainers:
> + - Ravi Kumar Bokka <rbokka@codeaurora.org>
> +
> +allOf:
> + - $ref: "nvmem.yaml#"
> +
> +properties:
> + compatible:
> + enum:
> + - qcom,sc7180-qfprom-efuse
> +
> + reg:
> + maxItems: 3
Instead of this, add descriptions for the 3 items. AKA:
reg:
items:
- description: The base of the qfprom.
- description: The start of the raw region.
- description: The start of the mem region.
...but do you really need to break this down into 3 ranges? Why can't
you just do:
reg = <0 0x00780000 0 0x2100>;
Then you really don't need any description and you'd just have:
reg:
maxItems: 1
> +
Need something for clocks and clock-names, like:
clocks:
maxItems: 1
clock-names:
items:
- const: sec
> +required:
> + - compatible
> + - reg
> + - clocks
> + - clock-names
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/qcom,gcc-sc7180.h>
> +
> + efuse@780000 {
> + compatible = "qcom,sc7180-qfprom-efuse";
> + reg = <0 0x00780000 0 0x100>,
> + <0 0x00780120 0 0x7a0>,
> + <0 0x00782000 0 0x100>;
> + clocks = <&gcc GCC_SEC_CTRL_CLK_SRC>;
> + clock-names = "secclk";
nit: Folks usually don't like the names of clocks to end in "clk". We
know it's a clock. Just name this "sec" or even a local name like
"core".
^ permalink raw reply
* Re: [RFC v1 2/3] drivers: nvmem: Add driver for QTI qfprom-efuse support
From: Doug Anderson @ 2020-05-12 23:02 UTC (permalink / raw)
To: Ravi Kumar Bokka
Cc: Srinivas Kandagatla, Rob Herring, LKML,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Rajendra Nayak, Sai Prakash Ranjan, dhavalp, mturney, sparate,
c_rbokka, mkurumel
In-Reply-To: <1589307480-27508-3-git-send-email-rbokka@codeaurora.org>
Hi,
On Tue, May 12, 2020 at 11:19 AM Ravi Kumar Bokka <rbokka@codeaurora.org> wrote:
>
> This patch adds new driver for QTI qfprom-efuse controller. This driver can
> access the raw qfprom regions for fuse blowing.
>
> The current existed qfprom driver is only supports for cpufreq, thermal sensors
> drivers by read out calibration data, speed bins..etc which is stored
> by qfprom efuses.
>
> Signed-off-by: Ravi Kumar Bokka <rbokka@codeaurora.org>
> ---
> drivers/nvmem/Kconfig | 10 +
> drivers/nvmem/Makefile | 2 +
> drivers/nvmem/qfprom-efuse.c | 476 +++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 488 insertions(+)
> create mode 100644 drivers/nvmem/qfprom-efuse.c
>
> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
> index d7b7f6d..c9345c5 100644
> --- a/drivers/nvmem/Kconfig
> +++ b/drivers/nvmem/Kconfig
> @@ -121,6 +121,16 @@ config QCOM_QFPROM
> This driver can also be built as a module. If so, the module
> will be called nvmem_qfprom.
>
> +config QTI_QFPROM_EFUSE
nit: it's really weird that the config above has the "QCOM" prefix and
you have "QTI". Can you just use "QCOM"?
> + tristate "QTI_QFPROM_EFUSE Support"
> + depends on HAS_IOMEM
> + help
> + Say y here to enable QFPROM-Efuse support. This driver provides access
> + QTI qfprom efuse via nvmem interface.
> +
> + This driver can also be built as a module. If so, the module
> + will be called nvmem_qfprom-efuse.
> +
> config NVMEM_SPMI_SDAM
> tristate "SPMI SDAM Support"
> depends on SPMI
> diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
> index a7c3772..1d8fe43 100644
> --- a/drivers/nvmem/Makefile
> +++ b/drivers/nvmem/Makefile
> @@ -27,6 +27,8 @@ obj-$(CONFIG_MTK_EFUSE) += nvmem_mtk-efuse.o
> nvmem_mtk-efuse-y := mtk-efuse.o
> obj-$(CONFIG_QCOM_QFPROM) += nvmem_qfprom.o
> nvmem_qfprom-y := qfprom.o
> +obj-$(CONFIG_QTI_QFPROM_EFUSE) += nvmem_qfprom-efuse.o
> +nvmem_qfprom-efuse-y := qfprom-efuse.o
> obj-$(CONFIG_NVMEM_SPMI_SDAM) += nvmem_qcom-spmi-sdam.o
> nvmem_qcom-spmi-sdam-y += qcom-spmi-sdam.o
> obj-$(CONFIG_ROCKCHIP_EFUSE) += nvmem_rockchip_efuse.o
> diff --git a/drivers/nvmem/qfprom-efuse.c b/drivers/nvmem/qfprom-efuse.c
> new file mode 100644
> index 0000000..2e3c275
> --- /dev/null
> +++ b/drivers/nvmem/qfprom-efuse.c
> @@ -0,0 +1,476 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2020, The Linux Foundation. All rights reserved.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/device.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/nvmem-provider.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/regulator/consumer.h>
> +
> +#define QFPROM_BLOW_STATUS_BUSY 0x1
> +#define QFPROM_BLOW_STATUS_READY 0x0
> +
> +/* Blow timer clock frequency in Mhz for 10nm LPe technology */
> +#define QFPROM_BLOW_TIMER_OFFSET 0x03c
> +#define QFPROM_BLOW_TIMER_RESET_VALUE 0x0
> +
> +/* Amount of time required to hold charge to blow fuse in micro-seconds */
> +#define QFPROM_FUSE_BLOW_POLL_PERIOD 100
Am I reading this correctly that your timeout is 10 microseconds and
you're polling every 100 microseconds? That seems weird. Why not
just poll every "fuse_blow_time_in_us / 10" or something?
> +#define QFPROM_BLOW_STATUS_OFFSET 0x048
> +
> +#define QFPROM_ACCEL_OFFSET 0x044
> +
> +/**
> + * struct qfprom_efuse_platform_data - structure holding qfprom-efuse
> + * platform data
> + *
> + * @name: qfprom-efuse compatible name
> + * @fuse_blow_time_in_us: Should contain the wait time when doing the fuse blow
> + * @accel_value: Should contain qfprom accel value
> + * @accel_reset_value: The reset value of qfprom accel value
> + * @qfprom_blow_timer_value: The timer value of qfprom when doing efuse blow
> + * @qfprom_blow_reset_freq: The frequency required to set when fuse blowing
> + * is done
> + * @qfprom_blow_set_freq: The frequency required to set when we start the
> + * fuse blowing
> + * @qfprom_max_vol: max voltage required to set fuse blow
> + * @qfprom_min_vol: min voltage required to set fuse blow
> + */
> +struct qfprom_efuse_platform_data {
> + const char *name;
You don't need the name for anything. Remove.
> + u8 fuse_blow_time_in_us;
> + u32 accel_value;
> + u32 accel_reset_value;
> + u32 qfprom_blow_timer_value;
> + u32 qfprom_blow_reset_freq;
> + u32 qfprom_blow_set_freq;
> + u32 qfprom_max_vol;
> + u32 qfprom_min_vol;
> +};
> +
> +/**
> + * struct qfprom_efuse_priv - structure holding qfprom-efuse attributes
> + *
> + * @qfpbase: iomapped memory space for qfprom base
> + * @qfpraw: iomapped memory space for qfprom raw fuse region
> + * @qfpmap: iomapped memory space for qfprom fuse blow timer
> +
Please error-check your kernel-docs with:
scripts/kernel-doc -rst drivers/nvmem/qfprom-efuse.c > /dev/null
It reports (among other things) that you have a bad line here.
> + * @dev: qfprom device structure
> + * @secclk: clock supply
> + * @vcc: regulator supply
> +
> + * @qfpraw_start: qfprom raw fuse start region
> + * @qfpraw_end: qfprom raw fuse end region
> + * @qfprom_efuse_platform_data: qfprom platform data
> + */
> +struct qfprom_efuse_priv {
> + void __iomem *qfpbase;
> + void __iomem *qfpraw;
> + void __iomem *qfpmap;
> + struct device *dev;
> + struct clk *secclk;
> + struct regulator *vcc;
> + resource_size_t qfpraw_start;
> + resource_size_t qfpraw_end;
> + struct qfprom_efuse_platform_data efuse;
> +};
> +
> +/*
> + * restore the gcc_sec_ctrl_clk frequency to default value(19.2 MHz)
> + */
> +static int qfprom_reset_clock_settings(const struct qfprom_efuse_priv *priv)
> +{
> + int ret;
> +
> + ret = clk_set_rate(priv->secclk, priv->efuse.qfprom_blow_reset_freq);
> + if (ret) {
> + dev_err(priv->dev, "clk_set_rate() failed to enable secclk\n");
> + return ret;
> + }
> +
> + return 0;
Slightly shorter:
ret = clk_set_rate(...);
if (ret)
dev_err(...);
return ret;
> +}
> +
> +/*
> + * set the gcc_sec_ctrl_clk to 4.8 MHz
> + */
> +static int qfprom_set_clock_settings(const struct qfprom_efuse_priv *priv)
> +{
> + int ret;
> +
> + ret = clk_set_rate(priv->secclk, priv->efuse.qfprom_blow_set_freq);
> +
nit: remove extra blank line.
> + if (ret) {
> + dev_err(priv->dev, "clk_set_rate() failed to enable secclk\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * set and reset the voltage for 1.8V and OFF(0V) on VDD_QFPROM (LDO11)
> + */
> +static int qfprom_set_voltage_settings(const struct qfprom_efuse_priv *priv,
> + int min_uV, int max_uV)
> +{
> + int ret;
> +
> + ret = regulator_set_voltage(priv->vcc, min_uV, max_uV);
> +
nit: remove extra blank line.
> + if (ret) {
> + dev_err(priv->dev, "regulator_set_voltage() failed!\n");
> + return ret;
> + }
See above where I suggest that you shouldn't be playing with the
voltage at all. Just enable and disable. Then get rid of this
function and callers should just call regulator_enable() /
regulator_disable().
> +
> + ret = regulator_enable(priv->vcc);
You have regulator_enable() in both the turn on and turn off case.
Presumably that means that every time your code is called there's
another refcount keeping this regulator on. That's definitely a bug.
> + if (ret) {
> + dev_err(priv->dev, "failed to enable regulator\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * resets the value of the blow timer, accel register and the clock
> + * and voltage settings
> + */
> +static int qfprom_disable_fuse_blowing(const struct qfprom_efuse_priv *priv)
> +{
> + int ret;
> +
> + ret = qfprom_set_voltage_settings(priv, 0, priv->efuse.qfprom_max_vol);
> + if (ret) {
> + dev_err(priv->dev, "qfprom_set_voltage_settings failed\n");
qfprom_set_voltage_settings() already printed an error. Don't need to
print all the way down.
> + return ret;
> + }
> +
> + ret = qfprom_reset_clock_settings(priv);
> + if (ret) {
> + dev_err(priv->dev, "qfprom_reset_clock_settings failed\n");
qfprom_reset_clock_settings() already printed an error. Don't need to
print all the way down.
> + return ret;
> + }
> +
> + writel(QFPROM_BLOW_TIMER_RESET_VALUE, priv->qfpmap +
> + QFPROM_BLOW_TIMER_OFFSET);
> + writel(priv->efuse.accel_reset_value,
> + priv->qfpmap + QFPROM_ACCEL_OFFSET);
> +
> + return 0;
> +}
> +
> +/*
> + * sets the value of the blow timer, accel register and the clock
> + * and voltage settings
> + */
> +static int qfprom_enable_fuse_blowing(const struct qfprom_efuse_priv *priv)
> +{
> + int ret;
> +
> + ret = qfprom_disable_fuse_blowing(priv);
> + if (ret) {
> + dev_err(priv->dev, "qfprom_disable_fuse_blowing()\n");
> + return ret;
> + }
Why is the first line of qfprom_enable_fuse_blowing() a call to
qfprom_disable_fuse_blowing()? That doesn't make sense. Please
remove.
> +
> + writel(priv->efuse.qfprom_blow_timer_value, priv->qfpmap +
> + QFPROM_BLOW_TIMER_OFFSET);
> + writel(priv->efuse.accel_value, priv->qfpmap + QFPROM_ACCEL_OFFSET);
> +
> + ret = qfprom_set_clock_settings(priv);
> + if (ret) {
> + dev_err(priv->dev, "qpfrom_set_clock_settings()\n");
Presumably you should undo your writel() in the error case.
qfprom_set_clock_settings() already printed an error. Don't need to
print all the way down.
> + return ret;
> + }
> +
> + ret = qfprom_set_voltage_settings(priv, priv->efuse.qfprom_min_vol,
> + priv->efuse.qfprom_max_vol);
> + if (ret) {
> + dev_err(priv->dev, "qfprom_set_voltage_settings()\n");
Presumably you should undo your writel() and clock settings in the error case.
qfprom_set_voltage_settings() already printed an error. Don't need to
print all the way down.
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * verifying to make sure address being written or read is from qfprom
> + * raw address range
> + */
> +bool addr_in_qfprom_range(const struct qfprom_efuse_priv *priv, u32 reg,
> + size_t bytes)
Function should be static.
> +{
> + if (((reg + bytes) > reg) && (reg >= priv->qfpraw_start) &&
The ((reg + bytes) > reg) is a weird check. I guess you're checking
for overflow? If nothing else this deserves a comment.
> + ((reg + bytes) <= priv->qfpraw_end)) {
> + return 1;
> + }
> +
> + return 0;
No need to convert from bool to bool with an if statement. Your code
reads like:
if (a)
return true;
return false;
When instead you can just say:
return a;
...but also I think this function doesn't actually make sense at all.
You should rely on the nvmem size checking and also have the client's
concept of the register range start at 0. See below.
> +}
> +
> +/*
> + * API for reading from raw qfprom region
> + */
> +static int qfprom_efuse_reg_read(void *context, unsigned int reg, void *_val,
> + size_t bytes)
> +{
> + struct qfprom_efuse_priv *priv = context;
> + u32 *value = _val;
Is it really legal to assume the buffer passed is word-aligned?
Someone with more nvmem subsystem experience might know better than I
do, but I wouldn't assume this.
> + u32 align_check;
> + int i = 0, words = bytes / 4;
> +
> + dev_info(priv->dev,
> + "reading raw qfprom region offset: 0x%08x of size: %zd\n",
> + reg, bytes);
> +
> + if (bytes % 4 != 0x00) {
> + dev_err(priv->dev,
> + "Bytes: %zd to read should be word align\n",
> + bytes);
> + return -EINVAL;
> + }
You should be able to get rid of the above check if you set word_size
to 4, right?
> +
> + if (!addr_in_qfprom_range(priv, reg, bytes)) {
It feels like you should just rely on the "nvmem->size" in the "econfig".
> + dev_err(priv->dev,
> + "Invalid qfprom raw region offset 0x%08x & bytes %zd\n",
> + reg, bytes);
This doesn't feel like the kind of case that should be printing an
error message. It should be up to the client to do that. Just
silently return -EINVAL.
> + return -EINVAL;
> + }
> +
> + align_check = (reg & 0xF);
> +
> + if (((align_check & ~3) == align_check) && value != NULL)
Rather than all this align check, shouldn't you just be able to set
"stride" to 4? As far as I understand it then the core should handle
this guarantee. If the core isn't and it's causing you problems then
you should submit a patch to the core. If (for some reason) you still
need an alignment check, it seems like a much less roundabout way to
do your calculations is "if (reg & 0x3)"
Also: IMO there's no need to check value against NULL. The client is
other Linux code, not userspace. It can be trusted not to give you a
NULL buffer. Worst case if it does is that you get a nice clean
crash.
> + while (words--)
> + *value++ = readl(priv->qfpbase + reg + (i++ * 4));
Adding "reg" to "qfpbase" really seems to violate some abstractions.
Do you have 1 memory range, or do you have 3? Here you're taking a
register that's in the raw range but accessing it as an offset from
the "qfp_sec_base". IMO you should just define a single register
range. If you really need 3 register ranges then you should treat
them separately.
Note also that it feels like you're exposing the wrong number to the
client. If a client wants to access byte 0 of your range they need to
pass 0x120, right? It feels like the client should just be passing 0
and (if you need to) you should worry about offsetting.
Also: I think you can use ioread32_rep() to do your reading rather than
open-coding, right?
> +
> + else
> + dev_err(priv->dev,
> + "Invalid input parameter 0x%08x fuse blow address\n",
> + reg);
> +
> + return 0;
> +}
> +
> +/*
> + * API for writing to raw qfprom region - fuse blowing
> + * returns success or failure code as per the conditions
> + */
> +static int qfprom_efuse_reg_write(void *context, unsigned int reg, void *_val,
> + size_t bytes)
> +{
> + struct qfprom_efuse_priv *priv = context;
> + u32 *value = _val;
Same comments as read wondering whether it's legit to assume that the
buffer passed in is word-aligned.
> + u32 align_check;
> + u32 blow_status = QFPROM_BLOW_STATUS_BUSY;
Don't init values if you don't need to.
> + int ret;
> + int i = 0, words = bytes / 4;
> +
> + dev_info(priv->dev,
> + "writing to raw qfprom region : 0x%08x of size: %zd\n",
> + reg, bytes);
> +
> + if (bytes % 4 != 0x00) {
> + dev_err(priv->dev, "Bytes: %zd should be word align\n", bytes);
> + return -EINVAL;
> + }
Same comments as in read about using word_size.
> +
> + if (!addr_in_qfprom_range(priv, reg, bytes)) {
> + dev_err(priv->dev,
> + "Invalid qfprom raw region offset 0x%08x & bytes %zd\n",
> + reg, bytes);
> + return -EINVAL;
> + }
Same comments as in read.
> +
> + align_check = (reg & 0xF);
> + if (value && ((align_check & ~3) == align_check)) {
> + ret = qfprom_enable_fuse_blowing(priv);
> + if (ret) {
> + dev_err(priv->dev, "qfprom_enable_fuse_blowing");
> + return ret;
> + }
Same comments as in read.
> +
> + ret = readl_relaxed_poll_timeout(priv->qfpmap +
> + QFPROM_BLOW_STATUS_OFFSET, blow_status,
> + (blow_status != QFPROM_BLOW_STATUS_BUSY),
> + QFPROM_FUSE_BLOW_POLL_PERIOD,
> + priv->efuse.fuse_blow_time_in_us);
> +
> + if (ret) {
> + dev_err(priv->dev, "Timeout blow status ready\n");
> + return ret;
> + }
Need to disable fuse blowing in the error case. Right now you leave
the regulator and clock enabled.
> +
> + if (blow_status == QFPROM_BLOW_STATUS_READY) {
...and what happens if blow_status is no longer busy (so you didn't
timeout) but it's not QFPROM_BLOW_STATUS_READY? Is that even
possible? If not then remove this check. If so then (presumably?)
you need to return an error.
> + while (words--)
> + writel(*value++,
> + priv->qfpbase + reg + (i++ * 4));
Similar comments as in read that you should be able to use iowrite32_rep()
> +
> + ret = readl_relaxed_poll_timeout(priv->qfpmap +
> + QFPROM_BLOW_STATUS_OFFSET, blow_status,
> + (blow_status != QFPROM_BLOW_STATUS_BUSY),
> + QFPROM_FUSE_BLOW_POLL_PERIOD,
> + priv->efuse.fuse_blow_time_in_us);
> +
> + if (ret) {
> + dev_err(priv->dev, "Timeout blow-status ready");
Need to disable fuse blowing in the error case. Right now you leave
the regulator and clock enabled.
> + return ret;
> + }
> + }
> +
> + ret = qfprom_disable_fuse_blowing(priv);
> + if (ret)
> + return ret;
> + } else {
> + dev_err(priv->dev, "Invalid input parameter fuse blow address");
> + return -EINVAL;
> + }
> +
> + dev_info(priv->dev, "written successfully raw qfprom region\n");
> +
> + return 0;
> +}
> +
> +static int qfprom_efuse_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct resource *qfpbase, *qfpraw, *qfpmap;
> + struct nvmem_device *nvmem;
> + struct nvmem_config *econfig;
> + struct qfprom_efuse_priv *priv;
> + const struct qfprom_efuse_platform_data *drvdata;
> + int ret;
> +
> + dev_info(&pdev->dev, "[%s]: Invoked\n", __func__);
Please remove. Your driver doesn't need to announce itself.
> +
> + drvdata = of_device_get_match_data(&pdev->dev);
> + if (!drvdata)
> + return -EINVAL;
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + priv->efuse.fuse_blow_time_in_us = drvdata->fuse_blow_time_in_us;
> + priv->efuse.accel_value = drvdata->accel_value;
> + priv->efuse.accel_reset_value = drvdata->accel_reset_value;
> + priv->efuse.qfprom_blow_timer_value = drvdata->qfprom_blow_timer_value;
> + priv->efuse.qfprom_blow_reset_freq = drvdata->qfprom_blow_reset_freq;
> + priv->efuse.qfprom_blow_set_freq = drvdata->qfprom_blow_set_freq;
> + priv->efuse.qfprom_max_vol = drvdata->qfprom_max_vol;
> + priv->efuse.qfprom_min_vol = drvdata->qfprom_min_vol;
> + priv->dev = dev;
> +
> + qfpbase = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +
> + priv->qfpbase = devm_ioremap_resource(dev, qfpbase);
> + if (IS_ERR(priv->qfpbase)) {
> + ret = PTR_ERR(priv->qfpbase);
> + goto err;
> + }
> +
> + qfpraw = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +
> + priv->qfpraw = devm_ioremap_resource(dev, qfpraw);
> + if (IS_ERR(priv->qfpraw)) {
> + ret = PTR_ERR(priv->qfpraw);
> + goto err;
> + }
> +
> + priv->qfpraw_start = qfpraw->start - qfpbase->start;
> + priv->qfpraw_end = qfpraw->end - qfpbase->start;
See above comments, but the math to calculate "qfpraw_start" and
"qfpraw_end" really seems to violate abstractions and implies that you
should just define one memory range, not 3.
> +
> + qfpmap = platform_get_resource(pdev, IORESOURCE_MEM, 2);
> +
> + priv->qfpmap = devm_ioremap_resource(dev, qfpmap);
> + if (IS_ERR(priv->qfpmap)) {
> + ret = PTR_ERR(priv->qfpmap);
> + goto err;
> + }
> +
> + priv->vcc = devm_regulator_get(&pdev->dev, "vcc");
> + if (IS_ERR(priv->vcc)) {
> + ret = PTR_ERR(priv->vcc);
> + if (ret == -ENODEV)
> + ret = -EPROBE_DEFER;
> +
> + goto err;
> + }
> +
> + priv->secclk = devm_clk_get(dev, "secclk");
> + if (IS_ERR(priv->secclk)) {
> + ret = PTR_ERR(priv->secclk);
> + if (ret != -EPROBE_DEFER)
> + dev_err(dev, "secclk error getting : %d\n", ret);
> + goto err;
> + }
> +
> + ret = clk_prepare_enable(priv->secclk);
You really need your clock prepared and enabled _all the time_? Maybe
just prepare it here and then do the enable/disable when you need it?
> + if (ret) {
> + dev_err(dev, "clk_prepare_enable() failed\n");
> + goto err;
> + }
> +
> + econfig = devm_kzalloc(dev, sizeof(*econfig), GFP_KERNEL);
> + if (!econfig)
> + return -ENOMEM;
> +
> + econfig->dev = dev;
> + econfig->name = "qfprom-efuse";
> + econfig->stride = 1;
> + econfig->word_size = 1;
See above about suggestions, specifically about changing ".stride" and
".word_size".
> + econfig->reg_read = qfprom_efuse_reg_read;
> + econfig->reg_write = qfprom_efuse_reg_write;
> + econfig->size = resource_size(qfpraw);
> + econfig->priv = priv;
> +
> + nvmem = devm_nvmem_register(dev, econfig);
> +
> + return PTR_ERR_OR_ZERO(nvmem);
> +
> +err:
> + clk_disable_unprepare(priv->secclk);
> + return ret;
> +}
> +
> +static const struct qfprom_efuse_platform_data sc7180_qfp_efuse_data = {
> + .name = "sc7180-qfprom-efuse",
> + .fuse_blow_time_in_us = 10,
> + .accel_value = 0xD10,
> + .accel_reset_value = 0x800,
> + .qfprom_blow_timer_value = 25,
> + .qfprom_blow_reset_freq = 19200000,
> + .qfprom_blow_set_freq = 4800000,
> + .qfprom_max_vol = 1904000,
> + .qfprom_min_vol = 1800000,
Can you explain why this patch needs to specify voltage at all? I'd
expect that the voltage constraints would be specified in the device
tree and the driver shouldn't worry about it. The only case you
really would need to specify this would be if you're expected to be on
a rail that's shared with other components and they have a different
(but compatible) operating range to yours. Do you actually have this?
> +};
> +
> +static const struct of_device_id qfprom_efuse_of_match[] = {
> + {
> + .compatible = "qcom,sc7180-qfprom-efuse",
> + .data = &sc7180_qfp_efuse_data
> + },
> + {/* sentinel */},
> +};
> +
> +MODULE_DEVICE_TABLE(of, qfprom_efuse_of_match);
> +
> +static struct platform_driver qfprom_efuse_driver = {
> + .probe = qfprom_efuse_probe,
You need a remove to unprepare / disable your clock, right? Unless
you change that to not leave it prepared/enabled all the time.
> + .driver = {
> + .name = "sc7180-qfprom-efuse",
You don't need to put "sc7180" in the driver name. This driver will
be used for other hardware eventually too with just a different
compatible match.
> + .of_match_table = qfprom_efuse_of_match,
> + },
> +};
> +
> +module_platform_driver(qfprom_efuse_driver);
> +MODULE_DESCRIPTION("QTI QFPROM Efuse driver");
> +MODULE_LICENSE("GPL v2");
> --
> Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by the Linux Foundation.
>
^ permalink raw reply
* Re: [PATCH v3 8/8] power: bq25890: document IBAT compensation DT properties
From: Rob Herring @ 2020-05-12 22:59 UTC (permalink / raw)
To: Michał Mirosław
Cc: Sebastian Reichel, Angus Ainslie, Yauhen Kharuzhy, linux-pm,
devicetree, linux-kernel
In-Reply-To: <7bf8bb5654bf67e14b59c5f94d024ac6ef5ff0ce.1588621247.git.mirq-linux@rere.qmqm.pl>
On Mon, May 04, 2020 at 09:47:48PM +0200, Michał Mirosław wrote:
> Document newly introduced IBAT compensation settings.
>
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---
> v2: initial version
> ---
> Documentation/devicetree/bindings/power/supply/bq25890.txt | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/power/supply/bq25890.txt b/Documentation/devicetree/bindings/power/supply/bq25890.txt
> index dc9c8f76e06c..bd214945d9dc 100644
> --- a/Documentation/devicetree/bindings/power/supply/bq25890.txt
> +++ b/Documentation/devicetree/bindings/power/supply/bq25890.txt
> @@ -32,6 +32,10 @@ Optional properties:
> - ti,thermal-regulation-threshold: integer, temperature above which the charge
> current is lowered, to avoid overheating (in degrees Celsius). If omitted,
> the default setting will be used (120 degrees);
> +- ti,ibatcomp-resistance: integer, value of a resistor in series with
> + the battery (in uOhm);
> +- ti,ibatcomp-clamp-voltage: integer, maximum charging voltage adjustment due
> + to expected voltage drop on in-series resistor (in uV);
Need a unit suffix as defined in property-units.txt
Rob
^ permalink raw reply
* Re: [PATCH] docs: dt: fix broken links due to txt->yaml renames
From: Rob Herring @ 2020-05-12 22:58 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Doc Mailing List, linux-kernel, Jonathan Corbet,
David Airlie, Daniel Vetter, Sandy Huang, Heiko Stübner,
Sean Wang, David S. Miller, Liam Girdwood, Mark Brown,
Arnaud Pouliquen, Thomas Bogendoerfer, Matthias Brugger,
Jyri Sarha, Andrzej Hajda, Benjamin Gaignard, Olivier Moysan,
Greg Kroah-Hartman, dri-devel, devicetree, linux-arm-kernel,
linux-rockchip, linux-bluetooth, linux-mediatek, netdev,
alsa-devel, linux-mips
In-Reply-To: <967df5c3303b478b76199d4379fe40f5094f3f9b.1588584538.git.mchehab+huawei@kernel.org>
On Mon, May 04, 2020 at 11:30:20AM +0200, Mauro Carvalho Chehab wrote:
> There are some new broken doc links due to yaml renames
> at DT. Developers should really run:
>
> ./scripts/documentation-file-ref-check
>
> in order to solve those issues while submitting patches.
> This tool can even fix most of the issues with:
>
> ./scripts/documentation-file-ref-check --fix
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
>
> PS.: This patch is against today's linux-next.
That's not a base anyone can apply this patch against.
>
>
> .../devicetree/bindings/display/bridge/sii902x.txt | 2 +-
> .../devicetree/bindings/display/rockchip/rockchip-drm.yaml | 2 +-
> .../devicetree/bindings/net/mediatek-bluetooth.txt | 2 +-
> .../devicetree/bindings/sound/audio-graph-card.txt | 2 +-
> .../devicetree/bindings/sound/st,sti-asoc-card.txt | 2 +-
> Documentation/mips/ingenic-tcu.rst | 2 +-
> MAINTAINERS | 6 +++---
> 7 files changed, 9 insertions(+), 9 deletions(-)
^ permalink raw reply
* Re: [PATCHv2 10/10] net: eth: altera: update devicetree bindings documentation
From: Rob Herring @ 2020-05-12 22:52 UTC (permalink / raw)
To: Joyce Ooi
Cc: Thor Thayer, David S . Miller, netdev, linux-kernel,
Dalon Westergreen, Tan Ley Foon, See Chin Liang, Dinh Nguyen,
devicetree
In-Reply-To: <20200504082558.112627-11-joyce.ooi@intel.com>
On Mon, May 04, 2020 at 04:25:58PM +0800, Joyce Ooi wrote:
> From: Dalon Westergreen <dalon.westergreen@intel.com>
>
> Update devicetree bindings documentation to include msgdma
> prefetcher and ptp bindings.
>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Dalon Westergreen <dalon.westergreen@intel.com>
> Signed-off-by: Joyce Ooi <joyce.ooi@intel.com>
> ---
> v2: no change
> ---
> .../devicetree/bindings/net/altera_tse.txt | 103 +++++++++++++++++----
> 1 file changed, 84 insertions(+), 19 deletions(-)
Reviewed-by: Rob Herring <robh@kernel.org>
One nit below.
>
> diff --git a/Documentation/devicetree/bindings/net/altera_tse.txt b/Documentation/devicetree/bindings/net/altera_tse.txt
> index 0b7d4d3758ea..2f2d12603907 100644
> --- a/Documentation/devicetree/bindings/net/altera_tse.txt
> +++ b/Documentation/devicetree/bindings/net/altera_tse.txt
> @@ -2,53 +2,86 @@
>
> Required properties:
> - compatible: Should be "altr,tse-1.0" for legacy SGDMA based TSE, and should
> - be "altr,tse-msgdma-1.0" for the preferred MSGDMA based TSE.
> + be "altr,tse-msgdma-1.0" for the preferred MSGDMA based TSE,
> + and "altr,tse-msgdma-2.0" for MSGDMA with prefetcher based
> + implementations.
> ALTR is supported for legacy device trees, but is deprecated.
> altr should be used for all new designs.
> - reg: Address and length of the register set for the device. It contains
> the information of registers in the same order as described by reg-names
> - reg-names: Should contain the reg names
> - "control_port": MAC configuration space region
> - "tx_csr": xDMA Tx dispatcher control and status space region
> - "tx_desc": MSGDMA Tx dispatcher descriptor space region
> - "rx_csr" : xDMA Rx dispatcher control and status space region
> - "rx_desc": MSGDMA Rx dispatcher descriptor space region
> - "rx_resp": MSGDMA Rx dispatcher response space region
> - "s1": SGDMA descriptor memory
> - interrupts: Should contain the TSE interrupts and it's mode.
> - interrupt-names: Should contain the interrupt names
> - "rx_irq": xDMA Rx dispatcher interrupt
> - "tx_irq": xDMA Tx dispatcher interrupt
> + "rx_irq": DMA Rx dispatcher interrupt
> + "tx_irq": DMA Tx dispatcher interrupt
> - rx-fifo-depth: MAC receive FIFO buffer depth in bytes
> - tx-fifo-depth: MAC transmit FIFO buffer depth in bytes
> - phy-mode: See ethernet.txt in the same directory.
> - phy-handle: See ethernet.txt in the same directory.
> - phy-addr: See ethernet.txt in the same directory. A configuration should
> include phy-handle or phy-addr.
> -- altr,has-supplementary-unicast:
> - If present, TSE supports additional unicast addresses.
> - Otherwise additional unicast addresses are not supported.
> -- altr,has-hash-multicast-filter:
> - If present, TSE supports a hash based multicast filter.
> - Otherwise, hash-based multicast filtering is not supported.
> -
> - mdio device tree subnode: When the TSE has a phy connected to its local
> mdio, there must be device tree subnode with the following
> required properties:
> -
> - compatible: Must be "altr,tse-mdio".
> - #address-cells: Must be <1>.
> - #size-cells: Must be <0>.
>
> For each phy on the mdio bus, there must be a node with the following
> fields:
> -
> - reg: phy id used to communicate to phy.
> - device_type: Must be "ethernet-phy".
>
> The MAC address will be determined using the optional properties defined in
> ethernet.txt.
>
> +- altr,has-supplementary-unicast:
> + If present, TSE supports additional unicast addresses.
> + Otherwise additional unicast addresses are not supported.
> +- altr,has-hash-multicast-filter:
> + If present, TSE supports a hash based multicast filter.
> + Otherwise, hash-based multicast filtering is not supported.
> +- altr,has-ptp:
> + If present, TSE supports 1588 timestamping. Currently only
> + supported with the msgdma prefetcher.
> +- altr,tx-poll-cnt:
> + Optional cycle count for Tx prefetcher to poll descriptor
> + list. If not present, defaults to 128, which at 125MHz is
> + roughly 1usec. Only for "altr,tse-msgdma-2.0".
> +- altr,rx-poll-cnt:
> + Optional cycle count for Tx prefetcher to poll descriptor
> + list. If not present, defaults to 128, which at 125MHz is
> + roughly 1usec. Only for "altr,tse-msgdma-2.0".
> +
> +Required registers by compatibility string:
> + - "altr,tse-1.0"
> + "control_port": MAC configuration space region
> + "tx_csr": DMA Tx dispatcher control and status space region
> + "rx_csr" : DMA Rx dispatcher control and status space region
> + "s1": DMA descriptor memory
> +
> + - "altr,tse-msgdma-1.0"
> + "control_port": MAC configuration space region
> + "tx_csr": DMA Tx dispatcher control and status space region
> + "tx_desc": DMA Tx dispatcher descriptor space region
> + "rx_csr" : DMA Rx dispatcher control and status space region
> + "rx_desc": DMA Rx dispatcher descriptor space region
> + "rx_resp": DMA Rx dispatcher response space region
> +
> + - "altr,tse-msgdma-2.0"
> + "control_port": MAC configuration space region
> + "tx_csr": DMA Tx dispatcher control and status space region
> + "tx_pref": DMA Tx prefetcher configuration space region
> + "rx_csr" : DMA Rx dispatcher control and status space region
> + "rx_pref": DMA Rx prefetcher configuration space region
> + "tod_ctrl": Time of Day Control register only required when
> + timestamping support is enabled. Timestamping is
> + only supported with the msgdma-2.0 implementation.
> +
> +Optional properties:
> +- local-mac-address: See ethernet.txt in the same directory.
> +- max-frame-size: See ethernet.txt in the same directory.
> +
> Example:
>
> tse_sub_0_eth_tse_0: ethernet@1,00000000 {
> @@ -86,6 +119,11 @@ Example:
> device_type = "ethernet-phy";
> };
>
> + phy2: ethernet-phy@2 {
> + reg = <0x2>;
> + device_type = "ethernet-phy";
> + };
> +
> };
> };
>
> @@ -111,3 +149,30 @@ Example:
> altr,has-hash-multicast-filter;
> phy-handle = <&phy1>;
> };
> +
> +
> + tse_sub_2_eth_tse_0: ethernet@1,00002000 {
What bus is this on? Usually a ',' like this is for a chip select
number. If just a 64-bit address, then no comma.
> + compatible = "altr,tse-msgdma-2.0";
> + reg = <0x00000001 0x00002000 0x00000400>,
> + <0x00000001 0x00002400 0x00000020>,
> + <0x00000001 0x00002420 0x00000020>,
> + <0x00000001 0x00002440 0x00000020>,
> + <0x00000001 0x00002460 0x00000020>,
> + <0x00000001 0x00002480 0x00000040>;
> + reg-names = "control_port", "rx_csr", "rx_pref","tx_csr", "tx_pref", "tod_ctrl";
> + interrupt-parent = <&hps_0_arm_gic_0>;
> + interrupts = <0 45 4>, <0 44 4>;
> + interrupt-names = "rx_irq", "tx_irq";
> + rx-fifo-depth = <2048>;
> + tx-fifo-depth = <2048>;
> + address-bits = <48>;
> + max-frame-size = <1500>;
> + local-mac-address = [ 00 00 00 00 00 00 ];
> + phy-mode = "sgmii";
> + altr,has-supplementary-unicast;
> + altr,has-hash-multicast-filter;
> + altr,has-ptp;
> + altr,tx-poll-cnt = <128>;
> + altr,rx-poll-cnt = <32>;
> + phy-handle = <&phy2>;
> + };
> --
> 2.13.0
>
^ permalink raw reply
* Re: [PATCH v2 1/4] dt-bindings: input: gpio-vibrator: Don't require enable-gpios
From: Dmitry Torokhov @ 2020-05-12 22:52 UTC (permalink / raw)
To: Ondrej Jirman
Cc: linux-sunxi, Rob Herring, Mark Rutland, Maxime Ripard,
Chen-Yu Tsai, Luca Weiss, Tomas Novotny, linux-input, devicetree,
linux-kernel, linux-arm-kernel, Rob Herring
In-Reply-To: <20200512222205.1456300-2-megous@megous.com>
On Wed, May 13, 2020 at 12:22:02AM +0200, Ondrej Jirman wrote:
> It is possible to turn the motor on/off just by enabling/disabling
> the vcc-supply.
>
> Signed-off-by: Ondrej Jirman <megous@megous.com>
> Acked-by: Rob Herring <robh@kernel.org>
> ---
> Documentation/devicetree/bindings/input/gpio-vibrator.yaml | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/input/gpio-vibrator.yaml b/Documentation/devicetree/bindings/input/gpio-vibrator.yaml
> index 2384465eaa19..c700b640bd53 100644
> --- a/Documentation/devicetree/bindings/input/gpio-vibrator.yaml
> +++ b/Documentation/devicetree/bindings/input/gpio-vibrator.yaml
> @@ -24,7 +24,6 @@ properties:
>
> required:
> - compatible
> - - enable-gpios
Hmm we need at least one of the 2 (gpio and supply). Should we encode it
in the binding?
Also, in the dirver code, I guess we need to switch to have regulator
optional (so we are not given the dummy one) and bail if neither
regulator nor GPIO is found.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] dt-bindings: clock: Add YAML schemas for QCOM A53 PLL
From: Rob Herring @ 2020-05-12 22:48 UTC (permalink / raw)
To: Sivaprakash Murugesan
Cc: sboyd, robh+dt, linux-clk, linux-kernel, linux-arm-msm,
mturquette, devicetree, agross, bjorn.andersson
In-Reply-To: <1588573803-3823-1-git-send-email-sivaprak@codeaurora.org>
On Mon, 4 May 2020 12:00:03 +0530, Sivaprakash Murugesan wrote:
> This patch adds schema for primary CPU PLL found on few Qualcomm
> platforms.
>
> Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
> ---
> .../devicetree/bindings/clock/qcom,a53pll.txt | 22 ------------
> .../devicetree/bindings/clock/qcom,a53pll.yaml | 40 ++++++++++++++++++++++
> 2 files changed, 40 insertions(+), 22 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/clock/qcom,a53pll.txt
> create mode 100644 Documentation/devicetree/bindings/clock/qcom,a53pll.yaml
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH V4 1/8] dt-bindings: mailbox: Add YAML schemas for QCOM APCS global block
From: Rob Herring @ 2020-05-12 22:46 UTC (permalink / raw)
To: Sivaprakash Murugesan
Cc: mturquette, agross, sboyd, linux-arm-msm, robh+dt, devicetree,
linux-clk, bjorn.andersson, jassisinghbrar, linux-kernel
In-Reply-To: <1588573224-3038-2-git-send-email-sivaprak@codeaurora.org>
On Mon, 4 May 2020 11:50:17 +0530, Sivaprakash Murugesan wrote:
> Qualcomm APCS global block provides a bunch of generic properties which
> are required in a device tree. Add YAML schema for these properties.
>
> Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
> ---
> [V4]
> * Addressed Rob's review comments
> .../bindings/mailbox/qcom,apcs-kpss-global.txt | 88 ----------------------
> .../bindings/mailbox/qcom,apcs-kpss-global.yaml | 86 +++++++++++++++++++++
> 2 files changed, 86 insertions(+), 88 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt
> create mode 100644 Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH 4/4] dt-bindings: soc: qcom: Document MSM8936 SMD RPM compatible
From: Rob Herring @ 2020-05-12 22:45 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Michael Turquette, linux-kernel, Vincent Knecht, Andy Gross,
linux-clk, linux-arm-msm, Bjorn Andersson, devicetree,
Stephen Boyd, Rob Herring
In-Reply-To: <20200501223232.275800-5-konradybcio@gmail.com>
On Sat, 2 May 2020 00:32:32 +0200, Konrad Dybcio wrote:
> From: Vincent Knecht <vincent.knecht@mailoo.org>
>
> Signed-off-by: Vincent Knecht <vincent.knecht@mailoo.org>
> ---
> Documentation/devicetree/bindings/soc/qcom/qcom,smd-rpm.txt | 1 +
> 1 file changed, 1 insertion(+)
>
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v7 1/2] dt-bindings: mmc: Document the Amlogic Meson SDHC MMC host controller
From: Rob Herring @ 2020-05-12 22:44 UTC (permalink / raw)
To: Martin Blumenstingl
Cc: linux-amlogic, ulf.hansson, linux-mmc, jbrunet, robh+dt,
devicetree, linux-kernel, yinxin_1989, lnykww, jianxin.pan,
linux-arm-kernel, linux.amoon
In-Reply-To: <20200512204147.504087-2-martin.blumenstingl@googlemail.com>
On Tue, 12 May 2020 22:41:46 +0200, Martin Blumenstingl wrote:
> This documents the devicetree bindings for the SDHC MMC host controller
> found in Meson6, Meson8, Meson8b and Meson8m2 SoCs. It can use a
> bus-width of 1/4/8-bit and it supports eMMC spec 4.4x/4.5x including
> HS200 mode (up to 100MHz clock). It embeds an internal clock controller
> which outputs four clocks (mod_clk, sd_clk, tx_clk and rx_clk) and is
> fed by four external input clocks (clkin[0-3]). "pclk" is the module
> register clock, it has to be enabled to access the registers.
>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
> .../bindings/mmc/amlogic,meson-mx-sdhc.yaml | 68 +++++++++++++++++++
> 1 file changed, 68 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mmc/amlogic,meson-mx-sdhc.yaml
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [RESEND PATCH v3] dt-bindings: power: Convert bq27xxx dt to yaml
From: Rob Herring @ 2020-05-12 22:41 UTC (permalink / raw)
To: Dan Murphy
Cc: linux-kernel, Pali Rohár, Andrew F . Davis, linux-pm,
devicetree, sre
In-Reply-To: <20200512191900.966-1-dmurphy@ti.com>
On Tue, 12 May 2020 14:19:00 -0500, Dan Murphy wrote:
> Convert the bq27xxx.txt to yaml format
>
> CC: Pali Rohár <pali@kernel.org>
> CC: Andrew F. Davis <afd@ti.com>
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
>
> v3 - Added power supply reference to yaml, fixed the deprecated compatibles,
> I did not change any compatibles as discussed in the list these should come in
> separate patches - https://lore.kernel.org/patchwork/patch/1237912/
>
> .../bindings/power/supply/bq27xxx.txt | 56 ------------
> .../bindings/power/supply/bq27xxx.yaml | 91 +++++++++++++++++++
> 2 files changed, 91 insertions(+), 56 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/power/supply/bq27xxx.txt
> create mode 100644 Documentation/devicetree/bindings/power/supply/bq27xxx.yaml
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [RFC v1 1/3] dt-bindings: nvmem: Add devicetree bindings for qfprom-efuse
From: Rob Herring @ 2020-05-12 22:36 UTC (permalink / raw)
To: Ravi Kumar Bokka
Cc: rnayak, Rob Herring, saiprakash.ranjan, sparate, mturney,
Srinivas Kandagatla, mkurumel, linux-kernel, dhavalp, c_rbokka,
devicetree
In-Reply-To: <1589307480-27508-2-git-send-email-rbokka@codeaurora.org>
On Tue, 12 May 2020 23:47:58 +0530, Ravi Kumar Bokka wrote:
> This patch adds dt-bindings document for qfprom-efuse controller.
>
> Signed-off-by: Ravi Kumar Bokka <rbokka@codeaurora.org>
> ---
> .../devicetree/bindings/nvmem/qfprom-efuse.yaml | 40 ++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/nvmem/qfprom-efuse.yaml
>
My bot found errors running 'make dt_binding_check' on your patch:
Documentation/devicetree/bindings/nvmem/qfprom-efuse.yaml: while scanning a block scalar
in "<unicode string>", line 30, column 5
found a tab character where an indentation space is expected
in "<unicode string>", line 34, column 1
Documentation/devicetree/bindings/Makefile:12: recipe for target 'Documentation/devicetree/bindings/nvmem/qfprom-efuse.example.dts' failed
make[1]: *** [Documentation/devicetree/bindings/nvmem/qfprom-efuse.example.dts] Error 1
make[1]: *** Waiting for unfinished jobs....
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/nvmem/qfprom-efuse.yaml: ignoring, error parsing file
warning: no schema found in file: ./Documentation/devicetree/bindings/nvmem/qfprom-efuse.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/nvmem/qfprom-efuse.yaml: ignoring, error parsing file
warning: no schema found in file: ./Documentation/devicetree/bindings/nvmem/qfprom-efuse.yaml
Makefile:1300: recipe for target 'dt_binding_check' failed
make: *** [dt_binding_check] Error 2
See https://patchwork.ozlabs.org/patch/1288685
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure dt-schema is up to date:
pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade
Please check and re-submit.
^ permalink raw reply
* Re: [PATCH v9 1/4] dt-bindings: media: i2c: Add bindings for Maxim Integrated MAX9286
From: Rob Herring @ 2020-05-12 22:36 UTC (permalink / raw)
To: Kieran Bingham
Cc: linux-renesas-soc, linux-media, devicetree, linux-kernel,
Mauro Carvalho Chehab, Kieran Bingham, Laurent Pinchart,
Jacopo Mondi, Niklas Söderlund, sakari.ailus, Hans Verkuil,
Hyun Kwon, Manivannan Sadhasivam, Laurent Pinchart, Jacopo Mondi,
Niklas Söderlund
In-Reply-To: <20200512155105.1068064-2-kieran.bingham+renesas@ideasonboard.com>
On Tue, May 12, 2020 at 04:51:02PM +0100, Kieran Bingham wrote:
> From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>
> The MAX9286 deserializes video data received on up to 4 Gigabit
> Multimedia Serial Links (GMSL) and outputs them on a CSI-2 port using up
> to 4 data lanes.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> Reviewed-by: Rob Herring <robh@kernel.org>
>
> ---
>
> v7:
> - Collect Rob's RB tag
> - Remove redundant maxItems from remote-endpoints
> - Fix SPDX licence tag
>
> .../bindings/media/i2c/maxim,max9286.yaml | 287 ++++++++++++++++++
> 1 file changed, 287 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
> new file mode 100644
> index 000000000000..f9d3e5712c59
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
> @@ -0,0 +1,287 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) 2019 Renesas Electronics Corp.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/maxim,max9286.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Maxim Integrated Quad GMSL Deserializer
> +
> +maintainers:
> + - Jacopo Mondi <jacopo+renesas@jmondi.org>
> + - Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> + - Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> + - Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> +
> +description: |
> + The MAX9286 deserializer receives video data on up to 4 Gigabit Multimedia
> + Serial Links (GMSL) and outputs them on a CSI-2 D-PHY port using up to 4 data
> + lanes.
> +
> + In addition to video data, the GMSL links carry a bidirectional control
> + channel that encapsulates I2C messages. The MAX9286 forwards all I2C traffic
> + not addressed to itself to the other side of the links, where a GMSL
> + serializer will output it on a local I2C bus. In the other direction all I2C
> + traffic received over GMSL by the MAX9286 is output on the local I2C bus.
> +
> +properties:
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
> +
> + compatible:
> + const: maxim,max9286
> +
> + reg:
> + description: I2C device address
> + maxItems: 1
> +
> + poc-supply:
> + description: Regulator providing Power over Coax to the cameras
> + maxItems: 1
> +
> + enable-gpios:
> + description: GPIO connected to the \#PWDN pin with inverted polarity
> + maxItems: 1
> +
> + ports:
> + type: object
> + description: |
> + The connections to the MAX9286 GMSL and its endpoint nodes are modelled
> + using the OF graph bindings in accordance with the video interface
> + bindings defined in
> + Documentation/devicetree/bindings/media/video-interfaces.txt.
> +
> + The following table lists the port number corresponding to each device
> + port.
> +
> + Port Description
> + ----------------------------------------
> + Port 0 GMSL Input 0
> + Port 1 GMSL Input 1
> + Port 2 GMSL Input 2
> + Port 3 GMSL Input 3
> + Port 4 CSI-2 Output
> +
> + properties:
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
> +
> + port@[0-3]:
> + type: object
> + properties:
> + reg:
> + enum: [ 0, 1, 2, 3 ]
> +
> + endpoint:
> + type: object
> +
> + properties:
> + remote-endpoint:
> + description: |
> + phandle to the remote GMSL source endpoint subnode in the
> + remote node port.
> +
> + required:
> + - remote-endpoint
> +
> + required:
> + - reg
> + - endpoint
> +
> + additionalProperties: false
> +
> + port@4:
> + type: object
> + properties:
> + reg:
> + const: 4
> +
> + endpoint:
> + type: object
> +
> + properties:
> + remote-endpoint:
> + description: phandle to the remote CSI-2 sink endpoint.
> +
> + data-lanes:
> + description: array of physical CSI-2 data lane indexes.
> +
> + required:
> + - remote-endpoint
> + - data-lanes
> +
> + required:
> + - reg
> + - endpoint
> +
> + additionalProperties: false
> +
> + required:
> + - port@4
> +
> + i2c-mux:
> + type: object
> + description: |
> + Each GMSL link is modelled as a child bus of an i2c bus
> + multiplexer/switch, in accordance with bindings described in
> + Documentation/devicetree/bindings/i2c/i2c-mux.txt. The serializer
> + device on the remote end of the GMSL link shall be modelled as a child
> + node of the corresponding I2C bus.
> +
> + properties:
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
> +
> + additionalProperties: false
Wrong indentation. Should be 2 more or this is a DT property.
> +
> +required:
> + - compatible
> + - reg
> + - ports
> + - i2c-mux
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> +
> + i2c@e66d8000 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + reg = <0 0xe66d8000 0 0x40>;
> +
> + gmsl-deserializer@2c {
> + compatible = "maxim,max9286";
> + reg = <0x2c>;
> + poc-supply = <&camera_poc_12v>;
> + enable-gpios = <&gpio 13 GPIO_ACTIVE_HIGH>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> +
> + max9286_in0: endpoint {
> + remote-endpoint = <&rdacm20_out0>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> +
> + max9286_in1: endpoint {
> + remote-endpoint = <&rdacm20_out1>;
> + };
> + };
> +
> + port@2 {
> + reg = <2>;
> +
> + max9286_in2: endpoint {
> + remote-endpoint = <&rdacm20_out2>;
> + };
> + };
> +
> + port@3 {
> + reg = <3>;
> +
> + max9286_in3: endpoint {
> + remote-endpoint = <&rdacm20_out3>;
> + };
> + };
> +
> + port@4 {
> + reg = <4>;
> +
> + max9286_out: endpoint {
> + data-lanes = <1 2 3 4>;
> + remote-endpoint = <&csi40_in>;
> + };
> + };
> + };
> +
> + i2c-mux {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + i2c@0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + reg = <0>;
> +
> + camera@51 {
> + reg = <0x51>;
> +
> + port {
> + rdacm20_out0: endpoint {
> + remote-endpoint = <&max9286_in0>;
> + };
> + };
> +
> + };
> + };
> +
> + i2c@1 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <1>;
> +
> + camera@52 {
> + reg = <0x52>;
> +
> + port {
> + rdacm20_out1: endpoint {
> + remote-endpoint = <&max9286_in1>;
> + };
> + };
> + };
> + };
> +
> + i2c@2 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <2>;
> +
> + camera@53 {
> + reg = <0x53>;
> +
> + port {
> + rdacm20_out2: endpoint {
> + remote-endpoint = <&max9286_in2>;
> + };
> + };
> + };
> + };
> +
> + i2c@3 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <3>;
> +
> + camera@54 {
> + reg = <0x54>;
> +
> + port {
> + rdacm20_out3: endpoint {
> + remote-endpoint = <&max9286_in3>;
> + };
> + };
> + };
> + };
> + };
> + };
> + };
> --
> 2.25.1
>
^ permalink raw reply
* Re: [PATCH v9 1/4] dt-bindings: media: i2c: Add bindings for Maxim Integrated MAX9286
From: Rob Herring @ 2020-05-12 22:33 UTC (permalink / raw)
To: Kieran Bingham
Cc: Jacopo Mondi, Hans Verkuil, linux-renesas-soc, sakari.ailus,
Laurent Pinchart, Niklas Söderlund, linux-kernel,
Kieran Bingham, Mauro Carvalho Chehab, devicetree, linux-media,
Rob Herring, Laurent Pinchart, Jacopo Mondi,
Manivannan Sadhasivam, Niklas Söderlund, Hyun Kwon
In-Reply-To: <20200512155105.1068064-2-kieran.bingham+renesas@ideasonboard.com>
On Tue, 12 May 2020 16:51:02 +0100, Kieran Bingham wrote:
> From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>
> The MAX9286 deserializes video data received on up to 4 Gigabit
> Multimedia Serial Links (GMSL) and outputs them on a CSI-2 port using up
> to 4 data lanes.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> Reviewed-by: Rob Herring <robh@kernel.org>
>
> ---
>
> v7:
> - Collect Rob's RB tag
> - Remove redundant maxItems from remote-endpoints
> - Fix SPDX licence tag
>
> .../bindings/media/i2c/maxim,max9286.yaml | 287 ++++++++++++++++++
> 1 file changed, 287 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
>
My bot found errors running 'make dt_binding_check' on your patch:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml: properties: {'enum': ['$ref', 'additionalItems', 'additionalProperties', 'allOf', 'anyOf', 'const', 'contains', 'default', 'dependencies', 'deprecated', 'description', 'else', 'enum', 'if', 'items', 'maxItems', 'maximum', 'minItems', 'minimum', 'multipleOf', 'not', 'oneOf', 'pattern', 'patternProperties', 'properties', 'propertyNames', 'required', 'then', 'unevaluatedProperties']} is not allowed for 'additionalProperties'
Documentation/devicetree/bindings/Makefile:12: recipe for target 'Documentation/devicetree/bindings/media/i2c/maxim,max9286.example.dts' failed
make[1]: *** [Documentation/devicetree/bindings/media/i2c/maxim,max9286.example.dts] Error 1
make[1]: *** Waiting for unfinished jobs....
Makefile:1300: recipe for target 'dt_binding_check' failed
make: *** [dt_binding_check] Error 2
See https://patchwork.ozlabs.org/patch/1288573
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure dt-schema is up to date:
pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade
Please check and re-submit.
^ permalink raw reply
* [PATCH v2 1/6] dt-bindings: phy: meson8b-usb2: Convert to json-schema
From: Martin Blumenstingl @ 2020-05-12 22:24 UTC (permalink / raw)
To: kishon, robh+dt, vkoul, devicetree, linux-amlogic
Cc: narmstrong, linux-arm-kernel, linux-kernel, hexdump0815,
Martin Blumenstingl, Thomas Graichen
In-Reply-To: <20200512222424.549351-1-martin.blumenstingl@googlemail.com>
Now that we have the DT validation in place, let's convert the device
tree bindings for the Amlogic Meson8, Meson8b, Meson8m2 and GXBB USB2
PHY over to a YAML schema.
While here, also add the fallback compatible string
"amlogic,meson-gxbb-usb2-phy" which is already used in
arch/arm/boot/dts/meson{,8,8b}.dtsi.
Tested-by: Thomas Graichen <thomas.graichen@gmail.com>
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
.../phy/amlogic,meson8b-usb2-phy.yaml | 63 +++++++++++++++++++
.../bindings/phy/meson8b-usb2-phy.txt | 28 ---------
2 files changed, 63 insertions(+), 28 deletions(-)
create mode 100644 Documentation/devicetree/bindings/phy/amlogic,meson8b-usb2-phy.yaml
delete mode 100644 Documentation/devicetree/bindings/phy/meson8b-usb2-phy.txt
diff --git a/Documentation/devicetree/bindings/phy/amlogic,meson8b-usb2-phy.yaml b/Documentation/devicetree/bindings/phy/amlogic,meson8b-usb2-phy.yaml
new file mode 100644
index 000000000000..0bd4ce39525a
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/amlogic,meson8b-usb2-phy.yaml
@@ -0,0 +1,63 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/phy/amlogic,meson8b-usb2-phy.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Amlogic Meson8, Meson8b, Meson8m2 and GXBB USB2 PHY
+
+maintainers:
+ - Martin Blumenstingl <martin.blumenstingl@googlemail.com>
+
+properties:
+ compatible:
+ oneOf:
+ - items:
+ - enum:
+ - amlogic,meson8-usb2-phy
+ - amlogic,meson8b-usb2-phy
+ - const: amlogic,meson-mx-usb2-phy
+ - const: amlogic,meson-gxbb-usb2-phy
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ minItems: 2
+
+ clock-names:
+ items:
+ - const: usb_general
+ - const: usb
+
+ resets:
+ minItems: 1
+
+ "#phy-cells":
+ const: 0
+
+ phy-supply:
+ description:
+ Phandle to a regulator that provides power to the PHY. This
+ regulator will be managed during the PHY power on/off sequence.
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+ - "#phy-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ usb-phy@c0000000 {
+ compatible = "amlogic,meson-gxbb-usb2-phy";
+ reg = <0xc0000000 0x20>;
+ resets = <&reset_usb_phy>;
+ clocks = <&clk_usb_general>, <&reset_usb>;
+ clock-names = "usb_general", "usb";
+ phy-supply = <&usb_vbus>;
+ #phy-cells = <0>;
+ };
diff --git a/Documentation/devicetree/bindings/phy/meson8b-usb2-phy.txt b/Documentation/devicetree/bindings/phy/meson8b-usb2-phy.txt
deleted file mode 100644
index d81d73aea608..000000000000
--- a/Documentation/devicetree/bindings/phy/meson8b-usb2-phy.txt
+++ /dev/null
@@ -1,28 +0,0 @@
-* Amlogic Meson8, Meson8b and GXBB USB2 PHY
-
-Required properties:
-- compatible: Depending on the platform this should be one of:
- "amlogic,meson8-usb2-phy"
- "amlogic,meson8b-usb2-phy"
- "amlogic,meson-gxbb-usb2-phy"
-- reg: The base address and length of the registers
-- #phys-cells: should be 0 (see phy-bindings.txt in this directory)
-- clocks: phandle and clock identifier for the phy clocks
-- clock-names: "usb_general" and "usb"
-
-Optional properties:
-- resets: reference to the reset controller
-- phy-supply: see phy-bindings.txt in this directory
-
-
-Example:
-
-usb0_phy: usb-phy@c0000000 {
- compatible = "amlogic,meson-gxbb-usb2-phy";
- #phy-cells = <0>;
- reg = <0x0 0xc0000000 0x0 0x20>;
- resets = <&reset RESET_USB_OTG>;
- clocks = <&clkc CLKID_USB>, <&clkc CLKID_USB0>;
- clock-names = "usb_general", "usb";
- phy-supply = <&usb_vbus>;
-};
--
2.26.2
^ permalink raw reply related
* [PATCH v2 4/6] phy: amlogic: meson8b-usb2: Don't set REG_ADP_BC_ACA_ENABLE on Meson8
From: Martin Blumenstingl @ 2020-05-12 22:24 UTC (permalink / raw)
To: kishon, robh+dt, vkoul, devicetree, linux-amlogic
Cc: narmstrong, linux-arm-kernel, linux-kernel, hexdump0815,
Martin Blumenstingl, Thomas Graichen
In-Reply-To: <20200512222424.549351-1-martin.blumenstingl@googlemail.com>
Skip setting REG_ADP_BC_ACA_ENABLE on Meson8 SoCs and polling for the
REG_ADP_BC_ACA_PIN_FLOAT bit. The vendor also skips this part on Meson8
SoCs.
This fixes initialization of the host-only USB PHY on Meson8 which would
otherwise fail with "USB ID detect failed!".
Fixes: 4a3449d1a0a10c ("phy: meson8b-usb2: add support for the USB PHY on Meson8 SoCs")
Reported-by: Thomas Graichen <thomas.graichen@gmail.com>
Tested-by: Thomas Graichen <thomas.graichen@gmail.com>
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
drivers/phy/amlogic/phy-meson8b-usb2.c | 48 ++++++++++++++++++++------
1 file changed, 38 insertions(+), 10 deletions(-)
diff --git a/drivers/phy/amlogic/phy-meson8b-usb2.c b/drivers/phy/amlogic/phy-meson8b-usb2.c
index 86824cc21f11..7236b8885f07 100644
--- a/drivers/phy/amlogic/phy-meson8b-usb2.c
+++ b/drivers/phy/amlogic/phy-meson8b-usb2.c
@@ -10,6 +10,7 @@
#include <linux/io.h>
#include <linux/module.h>
#include <linux/of_device.h>
+#include <linux/property.h>
#include <linux/regmap.h>
#include <linux/reset.h>
#include <linux/phy/phy.h>
@@ -105,12 +106,17 @@
#define RESET_COMPLETE_TIME 500
#define ACA_ENABLE_COMPLETE_TIME 50
+struct phy_meson8b_usb2_match_data {
+ bool host_enable_aca;
+};
+
struct phy_meson8b_usb2_priv {
- struct regmap *regmap;
- enum usb_dr_mode dr_mode;
- struct clk *clk_usb_general;
- struct clk *clk_usb;
- struct reset_control *reset;
+ struct regmap *regmap;
+ enum usb_dr_mode dr_mode;
+ struct clk *clk_usb_general;
+ struct clk *clk_usb;
+ struct reset_control *reset;
+ const struct phy_meson8b_usb2_match_data *match;
};
static const struct regmap_config phy_meson8b_usb2_regmap_conf = {
@@ -166,7 +172,8 @@ static int phy_meson8b_usb2_power_on(struct phy *phy)
regmap_update_bits(priv->regmap, REG_CTRL, REG_CTRL_SOF_TOGGLE_OUT,
REG_CTRL_SOF_TOGGLE_OUT);
- if (priv->dr_mode == USB_DR_MODE_HOST) {
+ if (priv->dr_mode == USB_DR_MODE_HOST &&
+ priv->match->host_enable_aca) {
regmap_update_bits(priv->regmap, REG_ADP_BC,
REG_ADP_BC_ACA_ENABLE,
REG_ADP_BC_ACA_ENABLE);
@@ -216,6 +223,10 @@ static int phy_meson8b_usb2_probe(struct platform_device *pdev)
if (IS_ERR(base))
return PTR_ERR(base);
+ priv->match = device_get_match_data(&pdev->dev);
+ if (!priv->match)
+ return -ENODEV;
+
priv->regmap = devm_regmap_init_mmio(&pdev->dev, base,
&phy_meson8b_usb2_regmap_conf);
if (IS_ERR(priv->regmap))
@@ -254,11 +265,28 @@ static int phy_meson8b_usb2_probe(struct platform_device *pdev)
return PTR_ERR_OR_ZERO(phy_provider);
}
+static const struct phy_meson8b_usb2_match_data phy_meson8_usb2_match_data = {
+ .host_enable_aca = false,
+};
+
+static const struct phy_meson8b_usb2_match_data phy_meson8b_usb2_match_data = {
+ .host_enable_aca = true,
+};
+
static const struct of_device_id phy_meson8b_usb2_of_match[] = {
- { .compatible = "amlogic,meson8-usb2-phy", },
- { .compatible = "amlogic,meson8b-usb2-phy", },
- { .compatible = "amlogic,meson-gxbb-usb2-phy", },
- { },
+ {
+ .compatible = "amlogic,meson8-usb2-phy",
+ .data = &phy_meson8_usb2_match_data
+ },
+ {
+ .compatible = "amlogic,meson8b-usb2-phy",
+ .data = &phy_meson8b_usb2_match_data
+ },
+ {
+ .compatible = "amlogic,meson-gxbb-usb2-phy",
+ .data = &phy_meson8b_usb2_match_data
+ },
+ { /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, phy_meson8b_usb2_of_match);
--
2.26.2
^ permalink raw reply related
* [PATCH v2 5/6] phy: amlogic: meson8b-usb2: unset the IDDQ bit during PHY power-on
From: Martin Blumenstingl @ 2020-05-12 22:24 UTC (permalink / raw)
To: kishon, robh+dt, vkoul, devicetree, linux-amlogic
Cc: narmstrong, linux-arm-kernel, linux-kernel, hexdump0815,
Martin Blumenstingl, Thomas Graichen
In-Reply-To: <20200512222424.549351-1-martin.blumenstingl@googlemail.com>
The vendor driver unsets the set_iddig bit during power-on as well and
sets it when suspending the PHY. I did not notice this in the vendor
driver first, because it's part of the dwc_otg driver there (instead of
their PHY code). While here, also add all other REG_DBG_UART register
bit definitions.
Tested-by: Thomas Graichen <thomas.graichen@gmail.com>
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
drivers/phy/amlogic/phy-meson8b-usb2.c | 44 +++++++++++++++++++-------
1 file changed, 32 insertions(+), 12 deletions(-)
diff --git a/drivers/phy/amlogic/phy-meson8b-usb2.c b/drivers/phy/amlogic/phy-meson8b-usb2.c
index 7236b8885f07..436dfa1a8a04 100644
--- a/drivers/phy/amlogic/phy-meson8b-usb2.c
+++ b/drivers/phy/amlogic/phy-meson8b-usb2.c
@@ -78,6 +78,17 @@
#define REG_ADP_BC_ACA_PIN_FLOAT BIT(26)
#define REG_DBG_UART 0x10
+ #define REG_DBG_UART_BYPASS_SEL BIT(0)
+ #define REG_DBG_UART_BYPASS_DM_EN BIT(1)
+ #define REG_DBG_UART_BYPASS_DP_EN BIT(2)
+ #define REG_DBG_UART_BYPASS_DM_DATA BIT(3)
+ #define REG_DBG_UART_BYPASS_DP_DATA BIT(4)
+ #define REG_DBG_UART_FSV_MINUS BIT(5)
+ #define REG_DBG_UART_FSV_PLUS BIT(6)
+ #define REG_DBG_UART_FSV_BURN_IN_TEST BIT(7)
+ #define REG_DBG_UART_LOOPBACK_EN_B BIT(8)
+ #define REG_DBG_UART_SET_IDDQ BIT(9)
+ #define REG_DBG_UART_ATE_RESET BIT(10)
#define REG_TEST 0x14
#define REG_TEST_DATA_IN_MASK GENMASK(3, 0)
@@ -172,20 +183,24 @@ static int phy_meson8b_usb2_power_on(struct phy *phy)
regmap_update_bits(priv->regmap, REG_CTRL, REG_CTRL_SOF_TOGGLE_OUT,
REG_CTRL_SOF_TOGGLE_OUT);
- if (priv->dr_mode == USB_DR_MODE_HOST &&
- priv->match->host_enable_aca) {
- regmap_update_bits(priv->regmap, REG_ADP_BC,
- REG_ADP_BC_ACA_ENABLE,
- REG_ADP_BC_ACA_ENABLE);
+ if (priv->dr_mode == USB_DR_MODE_HOST) {
+ regmap_update_bits(priv->regmap, REG_DBG_UART,
+ REG_DBG_UART_SET_IDDQ, 0);
- udelay(ACA_ENABLE_COMPLETE_TIME);
+ if (priv->match->host_enable_aca) {
+ regmap_update_bits(priv->regmap, REG_ADP_BC,
+ REG_ADP_BC_ACA_ENABLE,
+ REG_ADP_BC_ACA_ENABLE);
- regmap_read(priv->regmap, REG_ADP_BC, ®);
- if (reg & REG_ADP_BC_ACA_PIN_FLOAT) {
- dev_warn(&phy->dev, "USB ID detect failed!\n");
- clk_disable_unprepare(priv->clk_usb);
- clk_disable_unprepare(priv->clk_usb_general);
- return -EINVAL;
+ udelay(ACA_ENABLE_COMPLETE_TIME);
+
+ regmap_read(priv->regmap, REG_ADP_BC, ®);
+ if (reg & REG_ADP_BC_ACA_PIN_FLOAT) {
+ dev_warn(&phy->dev, "USB ID detect failed!\n");
+ clk_disable_unprepare(priv->clk_usb);
+ clk_disable_unprepare(priv->clk_usb_general);
+ return -EINVAL;
+ }
}
}
@@ -196,6 +211,11 @@ static int phy_meson8b_usb2_power_off(struct phy *phy)
{
struct phy_meson8b_usb2_priv *priv = phy_get_drvdata(phy);
+ if (priv->dr_mode == USB_DR_MODE_HOST)
+ regmap_update_bits(priv->regmap, REG_DBG_UART,
+ REG_DBG_UART_SET_IDDQ,
+ REG_DBG_UART_SET_IDDQ);
+
clk_disable_unprepare(priv->clk_usb);
clk_disable_unprepare(priv->clk_usb_general);
--
2.26.2
^ permalink raw reply related
* [PATCH v2 6/6] phy: amlogic: meson8b-usb2: Add a compatible string for Meson8m2
From: Martin Blumenstingl @ 2020-05-12 22:24 UTC (permalink / raw)
To: kishon, robh+dt, vkoul, devicetree, linux-amlogic
Cc: narmstrong, linux-arm-kernel, linux-kernel, hexdump0815,
Martin Blumenstingl, Thomas Graichen
In-Reply-To: <20200512222424.549351-1-martin.blumenstingl@googlemail.com>
The 3.10 vendor kernel sets the ACA_ENABLE bit on Meson8b, Meson8m2 and
GXBB, but not on Meson8. Add a compatible string for Meson8m2 which also
sets that bit.
While here, also update the Kconfig text and MODULE_DESCRIPTION.
Tested-by: Thomas Graichen <thomas.graichen@gmail.com>
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
drivers/phy/amlogic/Kconfig | 2 +-
drivers/phy/amlogic/phy-meson8b-usb2.c | 6 +++++-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/phy/amlogic/Kconfig b/drivers/phy/amlogic/Kconfig
index 3495b23af797..5ec53874d1ea 100644
--- a/drivers/phy/amlogic/Kconfig
+++ b/drivers/phy/amlogic/Kconfig
@@ -3,7 +3,7 @@
# Phy drivers for Amlogic platforms
#
config PHY_MESON8B_USB2
- tristate "Meson8, Meson8b and GXBB USB2 PHY driver"
+ tristate "Meson8, Meson8b, Meson8m2 and GXBB USB2 PHY driver"
default ARCH_MESON
depends on OF && (ARCH_MESON || COMPILE_TEST)
depends on USB_SUPPORT
diff --git a/drivers/phy/amlogic/phy-meson8b-usb2.c b/drivers/phy/amlogic/phy-meson8b-usb2.c
index 436dfa1a8a04..03c061dd5f0d 100644
--- a/drivers/phy/amlogic/phy-meson8b-usb2.c
+++ b/drivers/phy/amlogic/phy-meson8b-usb2.c
@@ -302,6 +302,10 @@ static const struct of_device_id phy_meson8b_usb2_of_match[] = {
.compatible = "amlogic,meson8b-usb2-phy",
.data = &phy_meson8b_usb2_match_data
},
+ {
+ .compatible = "amlogic,meson8m2-usb2-phy",
+ .data = &phy_meson8b_usb2_match_data
+ },
{
.compatible = "amlogic,meson-gxbb-usb2-phy",
.data = &phy_meson8b_usb2_match_data
@@ -320,5 +324,5 @@ static struct platform_driver phy_meson8b_usb2_driver = {
module_platform_driver(phy_meson8b_usb2_driver);
MODULE_AUTHOR("Martin Blumenstingl <martin.blumenstingl@googlemail.com>");
-MODULE_DESCRIPTION("Meson8, Meson8b and GXBB USB2 PHY driver");
+MODULE_DESCRIPTION("Meson8, Meson8b, Meson8m2 and GXBB USB2 PHY driver");
MODULE_LICENSE("GPL");
--
2.26.2
^ permalink raw reply related
* [PATCH v2 3/6] phy: amlogic: meson8b-usb2: Use a MMIO regmap
From: Martin Blumenstingl @ 2020-05-12 22:24 UTC (permalink / raw)
To: kishon, robh+dt, vkoul, devicetree, linux-amlogic
Cc: narmstrong, linux-arm-kernel, linux-kernel, hexdump0815,
Martin Blumenstingl, Thomas Graichen
In-Reply-To: <20200512222424.549351-1-martin.blumenstingl@googlemail.com>
Using a MMIO regmap and switch to regmap_update_bits() to simplify the
code in the driver. Also switch to devm_platform_ioremap_resource()
instead of open-coding it. No functional changes intended.
Tested-by: Thomas Graichen <thomas.graichen@gmail.com>
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
drivers/phy/amlogic/Kconfig | 1 +
drivers/phy/amlogic/phy-meson8b-usb2.c | 73 ++++++++++++--------------
2 files changed, 35 insertions(+), 39 deletions(-)
diff --git a/drivers/phy/amlogic/Kconfig b/drivers/phy/amlogic/Kconfig
index 71801e30d601..3495b23af797 100644
--- a/drivers/phy/amlogic/Kconfig
+++ b/drivers/phy/amlogic/Kconfig
@@ -9,6 +9,7 @@ config PHY_MESON8B_USB2
depends on USB_SUPPORT
select USB_COMMON
select GENERIC_PHY
+ select REGMAP_MMIO
help
Enable this to support the Meson USB2 PHYs found in Meson8,
Meson8b and GXBB SoCs.
diff --git a/drivers/phy/amlogic/phy-meson8b-usb2.c b/drivers/phy/amlogic/phy-meson8b-usb2.c
index bd66bd723e4a..86824cc21f11 100644
--- a/drivers/phy/amlogic/phy-meson8b-usb2.c
+++ b/drivers/phy/amlogic/phy-meson8b-usb2.c
@@ -10,6 +10,7 @@
#include <linux/io.h>
#include <linux/module.h>
#include <linux/of_device.h>
+#include <linux/regmap.h>
#include <linux/reset.h>
#include <linux/phy/phy.h>
#include <linux/platform_device.h>
@@ -105,34 +106,24 @@
#define ACA_ENABLE_COMPLETE_TIME 50
struct phy_meson8b_usb2_priv {
- void __iomem *regs;
+ struct regmap *regmap;
enum usb_dr_mode dr_mode;
struct clk *clk_usb_general;
struct clk *clk_usb;
struct reset_control *reset;
};
-static u32 phy_meson8b_usb2_read(struct phy_meson8b_usb2_priv *phy_priv,
- u32 reg)
-{
- return readl(phy_priv->regs + reg);
-}
-
-static void phy_meson8b_usb2_mask_bits(struct phy_meson8b_usb2_priv *phy_priv,
- u32 reg, u32 mask, u32 value)
-{
- u32 data;
-
- data = phy_meson8b_usb2_read(phy_priv, reg);
- data &= ~mask;
- data |= (value & mask);
-
- writel(data, phy_priv->regs + reg);
-}
+static const struct regmap_config phy_meson8b_usb2_regmap_conf = {
+ .reg_bits = 8,
+ .val_bits = 32,
+ .reg_stride = 4,
+ .max_register = REG_TUNE,
+};
static int phy_meson8b_usb2_power_on(struct phy *phy)
{
struct phy_meson8b_usb2_priv *priv = phy_get_drvdata(phy);
+ u32 reg;
int ret;
if (!IS_ERR_OR_NULL(priv->reset)) {
@@ -156,34 +147,34 @@ static int phy_meson8b_usb2_power_on(struct phy *phy)
return ret;
}
- phy_meson8b_usb2_mask_bits(priv, REG_CONFIG, REG_CONFIG_CLK_32k_ALTSEL,
- REG_CONFIG_CLK_32k_ALTSEL);
+ regmap_update_bits(priv->regmap, REG_CONFIG, REG_CONFIG_CLK_32k_ALTSEL,
+ REG_CONFIG_CLK_32k_ALTSEL);
- phy_meson8b_usb2_mask_bits(priv, REG_CTRL, REG_CTRL_REF_CLK_SEL_MASK,
- 0x2 << REG_CTRL_REF_CLK_SEL_SHIFT);
+ regmap_update_bits(priv->regmap, REG_CTRL, REG_CTRL_REF_CLK_SEL_MASK,
+ 0x2 << REG_CTRL_REF_CLK_SEL_SHIFT);
- phy_meson8b_usb2_mask_bits(priv, REG_CTRL, REG_CTRL_FSEL_MASK,
- 0x5 << REG_CTRL_FSEL_SHIFT);
+ regmap_update_bits(priv->regmap, REG_CTRL, REG_CTRL_FSEL_MASK,
+ 0x5 << REG_CTRL_FSEL_SHIFT);
/* reset the PHY */
- phy_meson8b_usb2_mask_bits(priv, REG_CTRL, REG_CTRL_POWER_ON_RESET,
- REG_CTRL_POWER_ON_RESET);
+ regmap_update_bits(priv->regmap, REG_CTRL, REG_CTRL_POWER_ON_RESET,
+ REG_CTRL_POWER_ON_RESET);
udelay(RESET_COMPLETE_TIME);
- phy_meson8b_usb2_mask_bits(priv, REG_CTRL, REG_CTRL_POWER_ON_RESET, 0);
+ regmap_update_bits(priv->regmap, REG_CTRL, REG_CTRL_POWER_ON_RESET, 0);
udelay(RESET_COMPLETE_TIME);
- phy_meson8b_usb2_mask_bits(priv, REG_CTRL, REG_CTRL_SOF_TOGGLE_OUT,
- REG_CTRL_SOF_TOGGLE_OUT);
+ regmap_update_bits(priv->regmap, REG_CTRL, REG_CTRL_SOF_TOGGLE_OUT,
+ REG_CTRL_SOF_TOGGLE_OUT);
if (priv->dr_mode == USB_DR_MODE_HOST) {
- phy_meson8b_usb2_mask_bits(priv, REG_ADP_BC,
- REG_ADP_BC_ACA_ENABLE,
- REG_ADP_BC_ACA_ENABLE);
+ regmap_update_bits(priv->regmap, REG_ADP_BC,
+ REG_ADP_BC_ACA_ENABLE,
+ REG_ADP_BC_ACA_ENABLE);
udelay(ACA_ENABLE_COMPLETE_TIME);
- if (phy_meson8b_usb2_read(priv, REG_ADP_BC) &
- REG_ADP_BC_ACA_PIN_FLOAT) {
+ regmap_read(priv->regmap, REG_ADP_BC, ®);
+ if (reg & REG_ADP_BC_ACA_PIN_FLOAT) {
dev_warn(&phy->dev, "USB ID detect failed!\n");
clk_disable_unprepare(priv->clk_usb);
clk_disable_unprepare(priv->clk_usb_general);
@@ -213,18 +204,22 @@ static const struct phy_ops phy_meson8b_usb2_ops = {
static int phy_meson8b_usb2_probe(struct platform_device *pdev)
{
struct phy_meson8b_usb2_priv *priv;
- struct resource *res;
struct phy *phy;
struct phy_provider *phy_provider;
+ void __iomem *base;
priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
return -ENOMEM;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- priv->regs = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(priv->regs))
- return PTR_ERR(priv->regs);
+ base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(base))
+ return PTR_ERR(base);
+
+ priv->regmap = devm_regmap_init_mmio(&pdev->dev, base,
+ &phy_meson8b_usb2_regmap_conf);
+ if (IS_ERR(priv->regmap))
+ return PTR_ERR(priv->regmap);
priv->clk_usb_general = devm_clk_get(&pdev->dev, "usb_general");
if (IS_ERR(priv->clk_usb_general))
--
2.26.2
^ permalink raw reply related
* [PATCH v2 2/6] dt-bindings: phy: meson8b-usb2: Add compatible string for Meson8m2
From: Martin Blumenstingl @ 2020-05-12 22:24 UTC (permalink / raw)
To: kishon, robh+dt, vkoul, devicetree, linux-amlogic
Cc: narmstrong, linux-arm-kernel, linux-kernel, hexdump0815,
Martin Blumenstingl, Thomas Graichen, Rob Herring
In-Reply-To: <20200512222424.549351-1-martin.blumenstingl@googlemail.com>
The USB2 PHY on Meson8m2 is identical to the one on Meson8b but
different to the one on Meson8. The only known difference is that Meson8
does not set the ACA_ENABLE bit while Meson8b and Meson8m2 do.
Add an explicit compatible string for Meson8m2 so those differences can
be taken care of.
Tested-by: Thomas Graichen <thomas.graichen@gmail.com>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
.../devicetree/bindings/phy/amlogic,meson8b-usb2-phy.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/phy/amlogic,meson8b-usb2-phy.yaml b/Documentation/devicetree/bindings/phy/amlogic,meson8b-usb2-phy.yaml
index 0bd4ce39525a..03c4809dbe8d 100644
--- a/Documentation/devicetree/bindings/phy/amlogic,meson8b-usb2-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/amlogic,meson8b-usb2-phy.yaml
@@ -16,6 +16,7 @@ properties:
- enum:
- amlogic,meson8-usb2-phy
- amlogic,meson8b-usb2-phy
+ - amlogic,meson8m2-usb2-phy
- const: amlogic,meson-mx-usb2-phy
- const: amlogic,meson-gxbb-usb2-phy
--
2.26.2
^ permalink raw reply related
* [PATCH v2 0/6] phy: meson8b-usb2: small fixes and improvements
From: Martin Blumenstingl @ 2020-05-12 22:24 UTC (permalink / raw)
To: kishon, robh+dt, vkoul, devicetree, linux-amlogic
Cc: narmstrong, linux-arm-kernel, linux-kernel, hexdump0815,
Martin Blumenstingl
This is a batch of fixes and improvements for the phy-meson8b-usb2
driver:
- convert the existing dt-bindings to json-schema and add a fallback
compatible string which is already in existing .dtsi files
- differentiate between Meson8 and Meson8m2 using a new compatible
string for the latter
- simplify the code by using a MMIO regmap
- set / unset the IDDQ and ACA enable bits depending on the operating
mode (and for the latter also the compatible string)
I suggest that all of these are applied to -next because we will need a
separate .dts patch (which I already have prepared) to use the new
Meson8m2 compatible string in meson8m2.dtsi. Otherwise we will be
changing behavior in patch #4, because meson8m2.dtsi currently inherits
the Meson8 compatible string.
The number of actual Meson8 users is very small (I only know one case),
so keeping Meson8m2 working is more important to me (because I know
several users with boards that have a Meson8m2 SoC).
Changes since v1 at [0]:
- added Thomas' Tested-bys (thank you!)
- also switched to the correct email address of Thomas for Reported-by
in patch #4
- added two more spaces for the indentation of the compatible "enum",
fixed the indentation (was three instead of two spaces) of the
phy-supply property and added additionalProperties: false. All
spotted in patch #1 by Rob - thank you!
- added Rob's Acked-by to patch #2 (thank you!), after fixing up the
indentation to also use four spaces (same as in patch #1)
[0] https://patchwork.kernel.org/cover/11523811/
Martin Blumenstingl (6):
dt-bindings: phy: meson8b-usb2: Convert to json-schema
dt-bindings: phy: meson8b-usb2: Add compatible string for Meson8m2
phy: amlogic: meson8b-usb2: Use a MMIO regmap
phy: amlogic: meson8b-usb2: Don't set REG_ADP_BC_ACA_ENABLE on Meson8
phy: amlogic: meson8b-usb2: unset the IDDQ bit during PHY power-on
phy: amlogic: meson8b-usb2: Add a compatible string for Meson8m2
.../phy/amlogic,meson8b-usb2-phy.yaml | 64 ++++++++
.../bindings/phy/meson8b-usb2-phy.txt | 28 ----
drivers/phy/amlogic/Kconfig | 3 +-
drivers/phy/amlogic/phy-meson8b-usb2.c | 149 ++++++++++++------
4 files changed, 164 insertions(+), 80 deletions(-)
create mode 100644 Documentation/devicetree/bindings/phy/amlogic,meson8b-usb2-phy.yaml
delete mode 100644 Documentation/devicetree/bindings/phy/meson8b-usb2-phy.txt
--
2.26.2
^ 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