* Re: [PATCH V5 2/8] dt-bindings: clock: add ipq6018 a53 pll compatible
From: Rob Herring @ 2020-05-26 23:08 UTC (permalink / raw)
To: Sivaprakash Murugesan
Cc: agross, bjorn.andersson, mturquette, sboyd, jassisinghbrar,
linux-arm-msm, linux-clk, devicetree, linux-kernel
In-Reply-To: <1590314686-11749-3-git-send-email-sivaprak@codeaurora.org>
On Sun, May 24, 2020 at 03:34:40PM +0530, Sivaprakash Murugesan wrote:
> cpus on ipq6018 are clocked by a53 pll, add device compatible for a53
> pll found on ipq6018 devices.
>
> Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
> ---
> [V5]
> * Addressed Bjorn's review comment.
> now the a53 dt-binding represents ipq a53 pll as well.
> .../devicetree/bindings/clock/qcom,a53pll.yaml | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [RESEND v5 14/21] dt-bindings: mtd: Add the nand-ecc-placement property
From: Boris Brezillon @ 2020-05-26 23:08 UTC (permalink / raw)
To: Miquel Raynal
Cc: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd,
Rob Herring, Mark Rutland, devicetree
In-Reply-To: <20200526195633.11543-15-miquel.raynal@bootlin.com>
On Tue, 26 May 2020 21:56:26 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Placement is where the ECC bytes are expected.
> No value means the controller can do whatever it desires (default is
> to put ECC bytes at the end of the OOB area). Alternative placement is
> "interleaved" (also sometimes referred as "syndrome") where data and
> OOB are mixed.
Hm, unless you can figure out what all controllers use, I'd suggest to
have the default mean "undefined", as in "controller put the ECC bytes
somewhere, but we don't where that is". Then let the explicit values be
"oob" or "interleaved".
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> .../devicetree/bindings/mtd/nand-controller.yaml | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mtd/nand-controller.yaml b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
> index d529f8587ba6..35512f2c66fa 100644
> --- a/Documentation/devicetree/bindings/mtd/nand-controller.yaml
> +++ b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
> @@ -56,6 +56,16 @@ patternProperties:
> (Linux will handle the calculations). soft_bch is deprecated
> and should be replaced by soft and nand-ecc-algo.
>
> + nand-ecc-placement:
> + allOf:
> + - $ref: /schemas/types.yaml#/definitions/string
> + - enum: [ free, interleaved ]
> + description:
> + Location for the ECC bytes. Free is the default and means the
> + controller in charge is free to put them where it wants.
> + Default state is to put ECC bytes at the end of the OOB area.
> + Otherwise, ECC bytes may be interleaved with data.
> +
> nand-ecc-algo:
> allOf:
> - $ref: /schemas/types.yaml#/definitions/string
^ permalink raw reply
* Re: [RESEND v5 15/21] dt-bindings: mtd: Deprecate hw_syndrome from the ECC modes
From: Boris Brezillon @ 2020-05-26 23:09 UTC (permalink / raw)
To: Miquel Raynal
Cc: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd,
Rob Herring, Mark Rutland, devicetree
In-Reply-To: <20200526195633.11543-16-miquel.raynal@bootlin.com>
On Tue, 26 May 2020 21:56:27 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> This mode should not be used anymore, it is actually a mix between a
> mode and a placement.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> Documentation/devicetree/bindings/mtd/nand-controller.yaml | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/mtd/nand-controller.yaml b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
> index 35512f2c66fa..a35ff8227427 100644
> --- a/Documentation/devicetree/bindings/mtd/nand-controller.yaml
> +++ b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
> @@ -49,12 +49,14 @@ patternProperties:
> nand-ecc-mode:
> allOf:
> - $ref: /schemas/types.yaml#/definitions/string
> - - enum: [ none, soft, hw, hw_syndrome, on-die ]
> + - enum: [ none, soft, hw, on-die ]
> description:
> Desired ECC engine, either hardware (most of the time
> embedded in the NAND controller) or software correction
> (Linux will handle the calculations). soft_bch is deprecated
> and should be replaced by soft and nand-ecc-algo.
> + hw_syndrome is deprecated and should be
> + replaced by hw and nand-ecc-placement.
Well, I'd expect the whole property to be deprecated in favor of
nand-ecc-engine-type.
>
> nand-ecc-placement:
> allOf:
^ permalink raw reply
* Re: [RESEND v5 14/21] dt-bindings: mtd: Add the nand-ecc-placement property
From: Rob Herring @ 2020-05-26 23:10 UTC (permalink / raw)
To: Miquel Raynal
Cc: Richard Weinberger, Rob Herring, Mark Rutland, linux-mtd,
Tudor Ambarus, devicetree, Boris Brezillon, Vignesh Raghavendra
In-Reply-To: <20200526195633.11543-15-miquel.raynal@bootlin.com>
On Tue, 26 May 2020 21:56:26 +0200, Miquel Raynal wrote:
> Placement is where the ECC bytes are expected.
> No value means the controller can do whatever it desires (default is
> to put ECC bytes at the end of the OOB area). Alternative placement is
> "interleaved" (also sometimes referred as "syndrome") where data and
> OOB are mixed.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> .../devicetree/bindings/mtd/nand-controller.yaml | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
My bot found errors running 'make dt_binding_check' on your patch:
Documentation/devicetree/bindings/mtd/nand-controller.yaml: while scanning a plain scalar
in "<unicode string>", line 64, column 11
found a tab character that violates indentation
in "<unicode string>", line 65, column 1
Documentation/devicetree/bindings/Makefile:12: recipe for target 'Documentation/devicetree/bindings/mtd/nand-controller.example.dts' failed
make[1]: *** [Documentation/devicetree/bindings/mtd/nand-controller.example.dts] Error 1
make[1]: *** Waiting for unfinished jobs....
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/nand-controller.yaml: ignoring, error parsing file
warning: no schema found in file: ./Documentation/devicetree/bindings/mtd/nand-controller.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/nand-controller.yaml: ignoring, error parsing file
warning: no schema found in file: ./Documentation/devicetree/bindings/mtd/nand-controller.yaml
Makefile:1300: recipe for target 'dt_binding_check' failed
make: *** [dt_binding_check] Error 2
See https://patchwork.ozlabs.org/patch/1298408
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: [RESEND v5 16/21] dt-bindings: mtd: Deprecate the nand-ecc-mode property
From: Boris Brezillon @ 2020-05-26 23:12 UTC (permalink / raw)
To: Miquel Raynal
Cc: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd,
Rob Herring, Mark Rutland, devicetree
In-Reply-To: <20200526195633.11543-17-miquel.raynal@bootlin.com>
On Tue, 26 May 2020 21:56:28 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> This property does not describe very well its purpose: it describes
> the ECC engine type, also called provider. Deprecate it in favor of
> nand-ecc-provider. In the mean time, update a bit the description of
> the property to make it more accurate.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> .../devicetree/bindings/mtd/nand-controller.yaml | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mtd/nand-controller.yaml b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
> index a35ff8227427..256080ba50bd 100644
> --- a/Documentation/devicetree/bindings/mtd/nand-controller.yaml
> +++ b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
> @@ -46,16 +46,17 @@ patternProperties:
> description:
> Contains the native Ready/Busy IDs.
>
> - nand-ecc-mode:
> + nand-ecc-provider:
> allOf:
> - $ref: /schemas/types.yaml#/definitions/string
> - enum: [ none, soft, hw, on-die ]
^"on-controller" ?
> description:
> - Desired ECC engine, either hardware (most of the time
> - embedded in the NAND controller) or software correction
> - (Linux will handle the calculations). soft_bch is deprecated
> - and should be replaced by soft and nand-ecc-algo.
> - hw_syndrome is deprecated and should be
> + Desired ECC engine provider, either hardware (most of the time
^Preferred ECC engine type, ...
> + embedded in the NAND controller, but can also be external) or
> + software correction (the OS will handle the calculations).
> + The nand-ecc-mode property is deprecated in favor of this one.
> + soft_bch is deprecated and should be replaced by soft and
> + nand-ecc-algo. hw_syndrome is deprecated and should be
> replaced by hw and nand-ecc-placement.
Which you do here, so patch 15 is not needed.
>
> nand-ecc-placement:
> @@ -148,7 +149,7 @@ examples:
>
> nand@0 {
> reg = <0>;
> - nand-ecc-mode = "soft";
> + nand-ecc-provider = "soft";
> nand-ecc-algo = "bch";
>
> /* controller specific properties */
^ permalink raw reply
* Re: [RESEND v5 09/21] mtd: rawnand: Create a new enumeration to describe properly ECC types
From: Boris Brezillon @ 2020-05-26 23:21 UTC (permalink / raw)
To: Miquel Raynal
Cc: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd,
Rob Herring, Mark Rutland, devicetree
In-Reply-To: <20200527005518.2d780ecc@collabora.com>
On Wed, 27 May 2020 00:55:18 +0200
Boris Brezillon <boris.brezillon@collabora.com> wrote:
> On Tue, 26 May 2020 21:56:21 +0200
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> > Now that the misleading mix between ECC engine type and OOB placement
> > has been addressed, add a new enumeration to properly define ECC types
> > (also called provider or mode).
>
> Let's pick a name and stick to it. I think "ECC provider type" or
> "ECC engine type" are good names.
Okay, I think I remember now where the 'ECC provider' concept comes
from. IIRC, the property will be used to select one ECC engine among
possibly more than one choice: there are systems with both
on-controller and on-die ECC engines, and you can always decide to use
the SW implementation of course.
>
> >
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> > ---
> > drivers/mtd/nand/raw/nand_base.c | 7 +++++++
> > include/linux/mtd/rawnand.h | 16 ++++++++++++++++
> > 2 files changed, 23 insertions(+)
> >
> > diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> > index 515cd4681660..5c6ab5b93270 100644
> > --- a/drivers/mtd/nand/raw/nand_base.c
> > +++ b/drivers/mtd/nand/raw/nand_base.c
> > @@ -5018,6 +5018,13 @@ static const char * const nand_ecc_modes[] = {
> > [NAND_ECC_ON_DIE] = "on-die",
> > };
> >
> > +static const char * const nand_ecc_engine_providers[] = {
>
> I'd rename that one nand_ecc_engine_types or nand_ecc_provider_types.
>
> > + [NAND_ECC_ENGINE_NONE] = "none",
> > + [NAND_ECC_ENGINE_SOFT] = "soft",
> > + [NAND_ECC_ENGINE_CONTROLLER] = "hw",
> > + [NAND_ECC_ENGINE_ON_DIE] = "on-die",
> > +};
> > +
> > static const char * const nand_ecc_placement[] = {
> > [NAND_ECC_PLACEMENT_INTERLEAVED] = "interleaved",
> > };
> > diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> > index dc909fb977c7..a2078c5f3d21 100644
> > --- a/include/linux/mtd/rawnand.h
> > +++ b/include/linux/mtd/rawnand.h
> > @@ -92,6 +92,22 @@ enum nand_ecc_mode {
> > NAND_ECC_ON_DIE,
> > };
> >
> > +/**
> > + * enum nand_ecc_engine_type - NAND ECC engine type/provider
> > + * @NAND_ECC_ENGINE_INVALID: Invalid value
> > + * @NAND_ECC_ENGINE_NONE: No ECC correction
> > + * @NAND_ECC_ENGINE_SOFT: Software ECC correction
> > + * @NAND_ECC_ENGINE_CONTROLLER: Hardware controller ECC correction
> > + * @NAND_ECC_ENGINE_ON_DIE: On chip hardware ECC correction
> > + */
> > +enum nand_ecc_engine_type {
>
> Looks like you went for ecc_engine_type here, so let's stick to that.
>
> > + NAND_ECC_ENGINE_INVALID,
>
> NAND_ECC_ENGINE_TYPE_xxx
>
> > + NAND_ECC_ENGINE_NONE,
>
> Do we really need a value for NONE? I'd expect the engine type to be
> applicable to NAND that have some sort of ECC engine connected to them.
>
> > + NAND_ECC_ENGINE_SOFT,
> > + NAND_ECC_ENGINE_CONTROLLER,
> > + NAND_ECC_ENGINE_ON_DIE,
> > +};
> > +
> > /**
> > * enum nand_ecc_placement - NAND ECC placement
> > * @NAND_ECC_PLACEMENT_FREE: The driver can decide where to put ECC bytes.
>
^ permalink raw reply
* Re: [PATCH v2] dt-bindings: clock: renesas: cpg: Convert to json-schema
From: Stephen Boyd @ 2020-05-26 23:25 UTC (permalink / raw)
To: Geert Uytterhoeven, Michael Turquette, Rob Herring
Cc: linux-renesas-soc, linux-clk, devicetree, Geert Uytterhoeven
In-Reply-To: <20200518081644.23683-1-geert+renesas@glider.be>
Quoting Geert Uytterhoeven (2020-05-18 01:16:44)
> Convert the Renesas Clock Pulse Generator (CPG) Device Tree
> binding documentation to json-schema, combining support for:
> - R-Mobile APE6 (R8A73A4) and A1 (R8A7740),
> - R-Car M1 (R8A7778) and H1 (R8A7779),
> - RZ/A1 (R7S72100),
> - SH-Mobile AG5 (SH73A0).
>
> Keep the example for R-Mobile A1, which shows most properties.
> Drop the consumer examples, as they do not belong here.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
Acked-by: Stephen Boyd <sboyd@kernel.org>
^ permalink raw reply
* Re: [PATCH 5/5] dt-bindings: timer: Add CLINT bindings
From: Palmer Dabbelt @ 2020-05-27 0:32 UTC (permalink / raw)
To: seanga2
Cc: anup, Anup Patel, Paul Walmsley, aou, robh+dt, daniel.lezcano,
tglx, devicetree, Damien Le Moal, linux-kernel, Atish Patra,
Alistair Francis, linux-riscv
In-Reply-To: <c0e9e625-daf8-b72f-2237-06018ff5d8a0@gmail.com>
On Thu, 21 May 2020 23:29:36 PDT (-0700), seanga2@gmail.com wrote:
> On 5/22/20 1:54 AM, Anup Patel wrote:
>> On Fri, May 22, 2020 at 1:35 AM Sean Anderson <seanga2@gmail.com> wrote:
>>>
>>> On 5/21/20 9:45 AM, Anup Patel wrote:
>>>> +Required properties:
>>>> +- compatible : "sifive,clint-1.0.0" and a string identifying the actual
>>>> + detailed implementation in case that specific bugs need to be worked around.
>>>
>>> Should the "riscv,clint0" compatible string be documented here? This
>>
>> Yes, I forgot to add this compatible string. I will add in v2.
>>
>>> peripheral is not really specific to sifive, as it is present in most
>>> rocket-chip cores.
>>
>> I agree that CLINT is present in a lot of non-SiFive RISC-V SOCs and
>> FPGAs but this IP is only documented as part of SiFive FU540 SOC.
>> (Refer, https://static.dev.sifive.com/FU540-C000-v1.0.pdf)
>>
>> The RISC-V foundation should host the CLINT spec independently
>> under https://github.com/riscv and make CLINT spec totally open.
>>
>> For now, I have documented it just like PLIC DT bindings found at:
>> Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.txt
>
> The PLIC seems to have its own RISC-V-sponsored documentation [1] which
> was split off from the older privileged specs. By your logic above,
> should it be renamed to riscv,plic0.txt (with a corresponding change in
> the documented compatible strings)?
>
> [1] https://github.com/riscv/riscv-plic-spec
Let's propose tagging that PLIC spec as v1.0.0 in the platform spec group, but
I don't see a reason why that wouldn't be viable. Assuming that's all OK, we
can start calling this a RISC-V PLIC (in addition to a SiFive PLIC, as they'll
be compatible).
>>
>> If RISC-V maintainers agree then I will document it as "RISC-V CLINT".
>>
>> @Palmer ?? @Paul ??
The CLINT is a SiFive spec. It has open source RTL so it's been implemented in
other designs, but it's not a RISC-V spec. The CLIC, which is a superset of
the CLINT, is a RISC-V spec. IIRC it's not finished yet (it's the fast
interrupts task group), but presumably we should have a "riscv,clic-2.0.0" (or
whatever it ends up being called) compat string to go along with the
specification.
>> Regards,
>> Anup
>>
>
> --Sean
^ permalink raw reply
* Re: [PATCH net-next v3 2/4] net: phy: Add a helper to return the index for of the internal delay
From: Andrew Lunn @ 2020-05-27 0:42 UTC (permalink / raw)
To: Dan Murphy
Cc: f.fainelli, hkallweit1, davem, robh, netdev, linux-kernel,
devicetree
In-Reply-To: <20200526174716.14116-3-dmurphy@ti.com>
> +/**
> + * phy_get_delay_index - returns the index of the internal delay
> + * @phydev: phy_device struct
> + * @delay_values: array of delays the PHY supports
> + * @size: the size of the delay array
> + * @int_delay: the internal delay to be looked up
> + * @descending: if the delay array is in descending order
> + *
> + * Returns the index within the array of internal delay passed in.
> + * Return errno if the delay is invalid or cannot be found.
> + */
> +s32 phy_get_delay_index(struct phy_device *phydev, int *delay_values, int size,
> + int int_delay, bool descending)
> +{
> + if (int_delay < 0)
> + return -EINVAL;
> +
> + if (size <= 0)
> + return -EINVAL;
> +
> + if (descending)
> + return phy_find_descending_delay(phydev, delay_values, size,
> + int_delay);
> +
> + return phy_find_ascending_delay(phydev, delay_values, size, int_delay);
> +}
> +EXPORT_SYMBOL(phy_get_delay_index);
Do we really need this ascending vs descending? This array is not
coming from device tree of anything, it is a static list in the PHY
driver. I would just define it needs to be ascending and be done.
Andrew
^ permalink raw reply
* Re: [PATCH net-next v3 4/4] net: dp83869: Add RGMII internal delay configuration
From: Andrew Lunn @ 2020-05-27 0:52 UTC (permalink / raw)
To: Dan Murphy
Cc: f.fainelli, hkallweit1, davem, robh, netdev, linux-kernel,
devicetree
In-Reply-To: <20200526174716.14116-5-dmurphy@ti.com>
> @@ -218,6 +224,7 @@ static int dp83869_of_init(struct phy_device *phydev)
> ret = phy_read_mmd(phydev, DP83869_DEVADDR, DP83869_STRAP_STS1);
> if (ret < 0)
> return ret;
> +
> if (ret & DP83869_STRAP_MIRROR_ENABLED)
> dp83869->port_mirroring = DP83869_PORT_MIRRORING_EN;
> else
This random white space change does not belong in this patch.
> @@ -232,6 +239,20 @@ static int dp83869_of_init(struct phy_device *phydev)
> &dp83869->tx_fifo_depth))
> dp83869->tx_fifo_depth = DP83869_PHYCR_FIFO_DEPTH_4_B_NIB;
>
> + ret = of_property_read_u32(of_node, "rx-internal-delay-ps",
> + &dp83869->rx_id_delay);
> + if (ret) {
> + dp83869->rx_id_delay = ret;
> + ret = 0;
> + }
This looks odd.
If this optional property is not found, -EINVAL will be returned. It
could also return -ENODATA. You then assign this error value to
dp83869->rx_id_delay? I would of expected you to assign 2000, the
default value?
> +
> + ret = of_property_read_u32(of_node, "tx-internal-delay-ps",
> + &dp83869->tx_id_delay);
> + if (ret) {
> + dp83869->tx_id_delay = ret;
> + ret = 0;
> + }
> +
> return ret;
> }
> #else
> @@ -367,10 +388,45 @@ static int dp83869_configure_mode(struct phy_device *phydev,
> return ret;
> }
>
> +static int dp83869_get_delay(struct phy_device *phydev)
> +{
> + struct dp83869_private *dp83869 = phydev->priv;
> + int delay_size = ARRAY_SIZE(dp83869_internal_delay);
> + int tx_delay = 0;
> + int rx_delay = 0;
> +
> + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
> + phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
> + tx_delay = phy_get_delay_index(phydev,
> + &dp83869_internal_delay[0],
> + delay_size, dp83869->tx_id_delay,
> + false);
> + if (tx_delay < 0) {
> + phydev_err(phydev, "Tx internal delay is invalid\n");
> + return tx_delay;
> + }
> + }
> +
> + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
> + phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
> + rx_delay = phy_get_delay_index(phydev,
> + &dp83869_internal_delay[0],
> + delay_size, dp83869->rx_id_delay,
> + false);
> + if (rx_delay < 0) {
> + phydev_err(phydev, "Rx internal delay is invalid\n");
> + return rx_delay;
> + }
> + }
So any PHY using these properties is going to pretty much reproduce
this code. Meaning is should all be in a helper.
Andrew
^ permalink raw reply
* Re: [PATCH v10 3/6] dt-bindings: clock: Add X1830 bindings.
From: Rob Herring @ 2020-05-27 0:54 UTC (permalink / raw)
To: 周琰杰 (Zhou Yanjie)
Cc: zhenwenjin, linux-clk, dongsheng.qiu, sboyd, sernia.zhou, robh+dt,
devicetree, mturquette, paul, linux-kernel, aric.pzqi
In-Reply-To: <20200526144044.71413-5-zhouyanjie@wanyeetech.com>
On Tue, 26 May 2020 22:40:41 +0800, 周琰杰 (Zhou Yanjie) wrote:
> Add the clock bindings for the X1830 Soc from Ingenic.
>
> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
>
> Notes:
> v2->v3:
> Adjust order from [3/5] in v2 to [4/5] in v3.
>
> v3->v4:
> Adjust order from [4/5] in v3 to [3/4] in v4.
>
> v4->v5:
> Rebase on top of kernel 5.6-rc1.
>
> v5->v6:
> Add missing part of X1830's CGU.
>
> v6->v7:
> No change.
>
> v7->v8:
> Rebase on top of linux-next.
>
> v8->v9:
> No change.
>
> v9->v10:
> Add missing "X1830_CLK_TCU".
>
> .../devicetree/bindings/clock/ingenic,cgu.yaml | 2 +
> include/dt-bindings/clock/x1830-cgu.h | 55 ++++++++++++++++++++++
> 2 files changed, 57 insertions(+)
> create mode 100644 include/dt-bindings/clock/x1830-cgu.h
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH net-next v3 4/4] net: dp83869: Add RGMII internal delay configuration
From: Andrew Lunn @ 2020-05-27 0:54 UTC (permalink / raw)
To: Dan Murphy
Cc: f.fainelli, hkallweit1, davem, robh, netdev, linux-kernel,
devicetree
In-Reply-To: <20200526174716.14116-5-dmurphy@ti.com>
> +static int dp83869_internal_delay[] = {250, 500, 750, 1000, 1250, 1500, 1750,
> + 2000, 2250, 2500, 2750, 3000, 3250,
> + 3500, 3750, 4000};
> +
You should make this const. Otherwise it takes up twice the space.
Andrew
^ permalink raw reply
* Re: [PATCH v3 01/12] scripts/dtc: check: Add 10bit/slave i2c reg flags support
From: Rob Herring @ 2020-05-27 1:17 UTC (permalink / raw)
To: Serge Semin
Cc: Jarkko Nikula, Wolfram Sang, Frank Rowand, Serge Semin,
Alexey Malahov, Thomas Bogendoerfer, Mika Westerberg, linux-mips,
linux-i2c, devicetree, linux-kernel
In-Reply-To: <20200526215528.16417-2-Sergey.Semin@baikalelectronics.ru>
On Wed, May 27, 2020 at 12:55:17AM +0300, Serge Semin wrote:
> Recently the I2C-controllers slave interface support was added to the
> kernel I2C subsystem. In this case Linux can be used as, for example,
> a I2C EEPROM machine. See [1] for details. Other than instantiating
> the EEPROM-slave device from user-space there is a way to declare the
> device in dts. In this case firstly the I2C bus controller must support
> the slave interface. Secondly I2C-slave sub-node of that controller
> must have "reg"-property with flag I2C_OWN_SLAVE_ADDRESS set (flag is
> declared in [2]). That flag is declared as (1 << 30), which when set
> makes dtc unhappy about too big address set for a I2C-slave:
>
> Warning (i2c_bus_reg): /example-2/i2c@1120000/eeprom@64: I2C bus unit address format error, expected "40000064"
> Warning (i2c_bus_reg): /example-2/i2c@1120000/eeprom@64:reg: I2C address must be less than 10-bits, got "0x40000064"
>
> Similar problem would have happened if we had set the 10-bit address
> flag I2C_TEN_BIT_ADDRESS in the "reg"-property.
>
> In order to fix the problem we suggest to alter the I2C-bus reg-check
> algorithm, so one would be aware of the upper bits set. Normally if no
> flag specified, the 7-bit address is expected in the "reg"-property.
> If I2C_TEN_BIT_ADDRESS is set, then the 10-bit address check will be
> performed. The I2C_OWN_SLAVE_ADDRESS flag will be just ignored.
>
> [1] Documentation/i2c/slave-interface.rst
> [2] include/dt-bindings/i2c/i2c.h
>
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> Cc: linux-mips@vger.kernel.org
> Cc: linux-i2c@vger.kernel.org
> ---
> scripts/dtc/checks.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
I've lost track of who all I've said this to already for this issue, but
patches to dtc should be against upstream and a version of this has been
sent there already. But it seems they've lost interest in addressing the
review comments. So feel free to send another one. The same comment
applies here.
Rob
^ permalink raw reply
* Re: [PATCH v3 02/12] dt-bindings: i2c: Convert DW I2C binding to DT schema
From: Rob Herring @ 2020-05-27 1:20 UTC (permalink / raw)
To: Serge Semin
Cc: linux-i2c, Wolfram Sang, devicetree, Mika Westerberg, Serge Semin,
Rob Herring, Jarkko Nikula, Thomas Bogendoerfer, Andy Shevchenko,
linux-kernel, linux-mips, Alexey Malahov
In-Reply-To: <20200526215528.16417-3-Sergey.Semin@baikalelectronics.ru>
On Wed, 27 May 2020 00:55:18 +0300, Serge Semin wrote:
> Modern device tree bindings are supposed to be created as YAML-files
> in accordance with dt-schema. This commit replaces Synopsys DW I2C
> legacy bare text bindings with YAML file. As before the bindings file
> states that the corresponding dts node is supposed to be compatible
> either with generic DW I2C controller or with Microsemi Ocelot SoC I2C
> one, to have registers, interrupts and clocks properties. In addition
> the node may have clock-frequency, i2c-sda-hold-time-ns,
> i2c-scl-falling-time-ns and i2c-sda-falling-time-ns optional properties.
>
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> Cc: linux-mips@vger.kernel.org
>
> ---
>
> Changelog v2:
> - Make sure that "mscc,ocelot-i2c" compatible node may have up to two
> registers space defined, while normal DW I2C controller will have only
> one registers space.
> - Add "mscc,ocelot-i2c" example to test the previous fix.
> - Declare "unevaluatedProperties" property instead of
> "additionalProperties" one.
> - Due to the previous fix we can now discard the dummy boolean properties
> definitions, since the proper type evaluation will be performed by the
> generic i2c-controller.yaml schema.
>
> Changelog v3:
> - Discard $ref from the "-ns" suffixed properties since they've got the
> uint32-array type by default applied in the common schema. Set "maxItems: 1"
> there instead to make sure the property will have a single value specified.
> ---
> .../bindings/i2c/i2c-designware.txt | 73 ---------
> .../bindings/i2c/snps,designware-i2c.yaml | 154 ++++++++++++++++++
> 2 files changed, 154 insertions(+), 73 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-designware.txt
> create mode 100644 Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH 16/17] dt-bindings: watchdog: renesas,wdt: Document r8a7742 support
From: Rob Herring @ 2020-05-27 1:31 UTC (permalink / raw)
To: Lad Prabhakar
Cc: Geert Uytterhoeven, Jens Axboe, Wolfram Sang, Ulf Hansson,
Sergei Shtylyov, David S. Miller, Wim Van Sebroeck, Guenter Roeck,
linux-ide, devicetree, linux-kernel, linux-i2c, linux-mmc, netdev,
linux-renesas-soc, linux-watchdog, Prabhakar
In-Reply-To: <1589555337-5498-17-git-send-email-prabhakar.mahadev-lad.rj@bp.renesas.com>
On Fri, May 15, 2020 at 04:08:56PM +0100, Lad Prabhakar wrote:
> RZ/G1H (R8A7742) watchdog implementation is compatible with R-Car Gen2,
> therefore add relevant documentation.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
> ---
> Documentation/devicetree/bindings/watchdog/renesas,wdt.txt | 1 +
> 1 file changed, 1 insertion(+)
Meanwhile in the DT tree, converting this schema landed. Can you prepare
a version based on the schema.
Rob
^ permalink raw reply
* Re: [PATCH 1/3] dt-bindings: usb: convert keystone-usb.txt to YAML
From: Rob Herring @ 2020-05-27 1:37 UTC (permalink / raw)
To: Roger Quadros; +Cc: balbi, vigneshr, linux-usb, devicetree, linux-kernel
In-Reply-To: <20200513130709.10239-2-rogerq@ti.com>
On Wed, May 13, 2020 at 04:07:07PM +0300, Roger Quadros wrote:
> Convert keystone-usb documentation to YAML format.
>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
> .../devicetree/bindings/usb/keystone-usb.txt | 56 ----------------
> .../bindings/usb/ti,keystone-dwc3.yaml | 67 +++++++++++++++++++
> 2 files changed, 67 insertions(+), 56 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/usb/keystone-usb.txt
> create mode 100644 Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml
>
> diff --git a/Documentation/devicetree/bindings/usb/keystone-usb.txt b/Documentation/devicetree/bindings/usb/keystone-usb.txt
> deleted file mode 100644
> index 77df82e36138..000000000000
> --- a/Documentation/devicetree/bindings/usb/keystone-usb.txt
> +++ /dev/null
> @@ -1,56 +0,0 @@
> -TI Keystone Soc USB Controller
> -
> -DWC3 GLUE
> -
> -Required properties:
> - - compatible: should be
> - "ti,keystone-dwc3" for Keystone 2 SoCs
> - "ti,am654-dwc3" for AM654 SoC
> - - #address-cells, #size-cells : should be '1' if the device has sub-nodes
> - with 'reg' property.
> - - reg : Address and length of the register set for the USB subsystem on
> - the SOC.
> - - interrupts : The irq number of this device that is used to interrupt the
> - MPU.
> - - ranges: allows valid 1:1 translation between child's address space and
> - parent's address space.
> -
> -SoC-specific Required Properties:
> -The following are mandatory properties for Keystone 2 66AK2HK, 66AK2L and 66AK2E
> -SoCs only:
> -
> -- clocks: Clock ID for USB functional clock.
> -- clock-names: Must be "usb".
> -
> -
> -The following are mandatory properties for 66AK2G and AM654:
> -
> -- power-domains: Should contain a phandle to a PM domain provider node
> - and an args specifier containing the USB device id
> - value. This property is as per the binding,
> - Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
> -
> -Sub-nodes:
> -The dwc3 core should be added as subnode to Keystone DWC3 glue.
> -- dwc3 :
> - The binding details of dwc3 can be found in:
> - Documentation/devicetree/bindings/usb/dwc3.txt
> -
> -Example:
> - usb: usb@2680000 {
> - compatible = "ti,keystone-dwc3";
> - #address-cells = <1>;
> - #size-cells = <1>;
> - reg = <0x2680000 0x10000>;
> - clocks = <&clkusb>;
> - clock-names = "usb";
> - interrupts = <GIC_SPI 393 IRQ_TYPE_EDGE_RISING>;
> - ranges;
> -
> - dwc3@2690000 {
> - compatible = "synopsys,dwc3";
> - reg = <0x2690000 0x70000>;
> - interrupts = <GIC_SPI 393 IRQ_TYPE_EDGE_RISING>;
> - usb-phy = <&usb_phy>, <&usb_phy>;
> - };
> - };
> diff --git a/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml b/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml
> new file mode 100644
> index 000000000000..14d2fe329b93
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml
> @@ -0,0 +1,67 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/usb/ti,keystone-dwc3.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TI Keystone Soc USB Controller
> +
> +maintainers:
> + - Roger Quadros <rogerq@ti.com>
> +
> +properties:
> + compatible:
> + oneOf:
> + - const: "ti,keystone-dwc3"
> + - const: "ti,am654-dwc3"
Use enum rather than oneOf+const.
> +
> + reg:
> + maxItems: 1
> + description: Address and length of the register set for the USB subsystem on
> + the SOC.
> +
> + interrupts:
> + maxItems: 1
> + description: The irq number of this device that is used to interrupt the MPU.
No need for genericish descriptions when a single item.
> +
> +
> + clocks:
> + description: Clock ID for USB functional clock.
How many?
> +
> + power-domains:
> + description: Should contain a phandle to a PM domain provider node
> + and an args specifier containing the USB device id
> + value. This property is as per the binding,
> + Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
How many?
> +
> + dwc3:
This doesn't work because there's a unit address. You need a pattern.
> + description: This is the node representing the DWC3 controller instance
> + Documentation/devicetree/bindings/usb/dwc3.txt
type: object
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clocks
additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> + usb: usb@2680000 {
> + compatible = "ti,keystone-dwc3";
> + #address-cells = <1>;
> + #size-cells = <1>;
These have to be documented.
> + reg = <0x2680000 0x10000>;
> + clocks = <&clkusb>;
> + clock-names = "usb";
> + interrupts = <GIC_SPI 393 IRQ_TYPE_EDGE_RISING>;
> + ranges;
This too.
> +
> + dwc3@2690000 {
> + compatible = "synopsys,dwc3";
> + reg = <0x2690000 0x70000>;
> + interrupts = <GIC_SPI 393 IRQ_TYPE_EDGE_RISING>;
> + usb-phy = <&usb_phy>, <&usb_phy>;
> + };
> + };
> --
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>
^ permalink raw reply
* Re: [PATCH v5 5/8] clk: stm32: Fix stm32f429's ltdc driver hang in set clock rate, fix duplicated ltdc clock register to 'clk_core' case ltdc's clock turn off by clk_disable_unused()
From: Stephen Boyd @ 2020-05-27 1:44 UTC (permalink / raw)
To: broonie, dillon.minfei, linus.walleij
Cc: devicetree, linux-arm-kernel, linux-kernel, linux-spi,
linux-stm32, dri-devel, linux-clk, dillon min
In-Reply-To: <1590378348-8115-6-git-send-email-dillon.minfei@gmail.com>
Quoting dillon.minfei@gmail.com (2020-05-24 20:45:45)
> From: dillon min <dillon.minfei@gmail.com>
>
> ltdc set clock rate crashed
> 'post_div_data[]''s pll_num is PLL_I2S, PLL_SAI (number is 1,2). but,
Please write "post_div_data[]'s" if it is possessive. "But" doesn't
start a sentence. This is one sentence, not two.
> as pll_num is offset of 'clks[]' input to clk_register_pll_div(), which
> is FCLK, CLK_LSI, defined in 'include/dt-bindings/clock/stm32fx-clock.h'
> so, this is a null object at the register time.
> then, in ltdc's clock is_enabled(), enable(), will call to_clk_gate().
> will return a null object, cause kernel crashed.
> need change pll_num to PLL_VCO_I2S, PLL_VCO_SAI for 'post_div_data[]'
>
> duplicated ltdc clock
> 'stm32f429_gates[]' has a member 'ltdc' register to 'clk_core', but no
> upper driver use it, ltdc driver use the lcd-tft defined in
> 'stm32f429_aux_clk[]'. after system startup, as stm32f429_gates[]'s ltdc
> enable_count is zero, so turn off by clk_disable_unused()
I sort of follow this. Is this another patch? Seems like two things are
going on here.
>
> Changes since V3:
> 1 drop last wrong changes about 'CLK_IGNORE_UNUSED' patch
> 2 fix PLL_SAI mismatch with PLL_VCO_SAI
This change log goes under the --- below.
>
> Signed-off-by: dillon min <dillon.minfei@gmail.com>
Any Fixes tag?
^ permalink raw reply
* Re: [PATCH v8 1/2] dt-bindings: clk: intel: Add bindings document & header file for CGU
From: Stephen Boyd @ 2020-05-27 1:48 UTC (permalink / raw)
To: Rahul Tanwar, linux-clk, mturquette
Cc: robh, mark.rutland, linux-kernel, devicetree, andriy.shevchenko,
qi-ming.wu, yixin.zhu, cheol.yong.kim, Rahul Tanwar
In-Reply-To: <8dce2be13195aab20c6b11fca6af0fffe22d5241.1587102634.git.rahul.tanwar@linux.intel.com>
Quoting Rahul Tanwar (2020-04-16 22:54:46)
> Clock generation unit(CGU) is a clock controller IP of Intel's Lightning
> Mountain(LGM) SoC. Add DT bindings include file and document for CGU clock
> controller driver of LGM.
>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Rahul Tanwar <rahul.tanwar@linux.intel.com>
> ---
Applied to clk-next
^ permalink raw reply
* Re: [PATCH v25 01/16] dt: bindings: Add multicolor class dt bindings documention
From: Rob Herring @ 2020-05-27 1:49 UTC (permalink / raw)
To: Dan Murphy; +Cc: jacek.anaszewski, pavel, devicetree, linux-leds, linux-kernel
In-Reply-To: <20200526164652.2331-2-dmurphy@ti.com>
On Tue, May 26, 2020 at 11:46:37AM -0500, Dan Murphy wrote:
> Add DT bindings for the LEDs multicolor class framework.
> Add multicolor ID to the color ID list for device tree bindings.
>
> CC: Rob Herring <robh@kernel.org>
> Acked-by: Pavel Machek <pavel@ucw.cz>
> Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
> .../bindings/leds/leds-class-multicolor.yaml | 71 +++++++++++++++++++
> drivers/leds/led-core.c | 1 +
> include/dt-bindings/leds/common.h | 3 +-
> 3 files changed, 74 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/devicetree/bindings/leds/leds-class-multicolor.yaml
>
> diff --git a/Documentation/devicetree/bindings/leds/leds-class-multicolor.yaml b/Documentation/devicetree/bindings/leds/leds-class-multicolor.yaml
> new file mode 100644
> index 000000000000..fa6ea8e5c46b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-class-multicolor.yaml
> @@ -0,0 +1,71 @@
> +# SPDX-License-Identifier: GPL-2.0
Dual license new bindings please.
GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/leds-class-multicolor.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Common properties for the multicolor LED class.
> +
> +maintainers:
> + - Dan Murphy <dmurphy@ti.com>
> +
> +description: |
> + Bindings for multi color LEDs show how to describe current outputs of
> + either integrated multi-color LED elements (like RGB, RGBW, RGBWA-UV
> + etc.) or standalone LEDs, to achieve logically grouped multi-color LED
> + modules. This is achieved by adding multi-led nodes layer to the
> + monochrome LED bindings.
> + The nodes and properties defined in this document are unique to the multicolor
> + LED class. Common LED nodes and properties are inherited from the common.txt
> + within this documentation directory.
> +
> +properties:
> + color:
> + description: |
> + For multicolor LED support this property should be defined as
> + LED_COLOR_ID_MULTI and further definition can be found in
> + include/linux/leds/common.h.
This should have:
const: X # LED_COLOR_ID_MULTI
It can't use the define, so you have to use the value.
> +
> +required:
> + - color
> +
> +examples:
> + - |
> + #include <dt-bindings/leds/common.h>
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + led-controller@14 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "ti,lp5009";
> + reg = <0x14>;
> +
> + multi-led@1 {
Define $nodename must be 'multi-led(@[0-9a-f]+)?'
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <1>;
> + color = <LED_COLOR_ID_MULTI>;
> + function = LED_FUNCTION_CHARGING;
> +
> + led@0 {
Then the schema should define child nodes named 'led'.
> + reg = <0>;
> + color = <LED_COLOR_ID_RED>;
I assume in this case 'reg' and 'color' need to be defined as required?
> + };
> +
> + led@1 {
> + reg = <1>;
> + color = <LED_COLOR_ID_GREEN>;
> + };
> +
> + led@2 {
> + reg = <2>;
> + color = <LED_COLOR_ID_BLUE>;
> + };
> + };
> + };
> + };
> +
> +additionalProperties: false
> +...
> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
This isn't a binding file. Belongs in another patch.
> index f1f718dbe0f8..846248a0693d 100644
> --- a/drivers/leds/led-core.c
> +++ b/drivers/leds/led-core.c
> @@ -34,6 +34,7 @@ const char * const led_colors[LED_COLOR_ID_MAX] = {
> [LED_COLOR_ID_VIOLET] = "violet",
> [LED_COLOR_ID_YELLOW] = "yellow",
> [LED_COLOR_ID_IR] = "ir",
> + [LED_COLOR_ID_MULTI] = "multicolor",
> };
> EXPORT_SYMBOL_GPL(led_colors);
>
> diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h
> index 0ce7dfc00dcb..a463ce6a8794 100644
> --- a/include/dt-bindings/leds/common.h
> +++ b/include/dt-bindings/leds/common.h
> @@ -30,7 +30,8 @@
> #define LED_COLOR_ID_VIOLET 5
> #define LED_COLOR_ID_YELLOW 6
> #define LED_COLOR_ID_IR 7
> -#define LED_COLOR_ID_MAX 8
> +#define LED_COLOR_ID_MULTI 8
> +#define LED_COLOR_ID_MAX 9
>
> /* Standard LED functions */
> /* Keyboard LEDs, usually it would be input4::capslock etc. */
> --
> 2.25.1
>
^ permalink raw reply
* Re: [PATCH v8 06/14] media: platform: Improve the implementation of the system PM ops
From: Xia Jiang @ 2020-05-27 1:52 UTC (permalink / raw)
To: Tomasz Figa
Cc: Hans Verkuil, Mauro Carvalho Chehab, Rob Herring,
Matthias Brugger, Rick Chang, linux-media, devicetree,
linux-kernel, linux-arm-kernel, linux-mediatek, Marek Szyprowski,
srv_heupstream, senozhatsky, mojahsu, drinkcat, maoguang.meng,
sj.huang
In-Reply-To: <20200521153257.GF209565@chromium.org>
On Thu, 2020-05-21 at 15:32 +0000, Tomasz Figa wrote:
> Hi Xia,
>
> On Fri, Apr 03, 2020 at 05:40:25PM +0800, Xia Jiang wrote:
> > Cancel reset hw operation in suspend and resume function because this
> > will be done in device_run().
>
> This and...
>
> > Add spin_lock and unlock operation in irq and resume function to make
> > sure that the current frame is processed completely before suspend.
>
> ...this are two separate changes. Please split.
>
> >
> > Signed-off-by: Xia Jiang <xia.jiang@mediatek.com>
> > ---
> > drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c | 11 +++++++++--
> > 1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > index dd5cadd101ef..2fa3711fdc9b 100644
> > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > @@ -911,6 +911,8 @@ static irqreturn_t mtk_jpeg_dec_irq(int irq, void *priv)
> > u32 dec_ret;
> > int i;
> >
> > + spin_lock(&jpeg->hw_lock);
> > +
>
> nit: For consistency, it is recommended to always use the same, i.e. the
> strongest, spin_(un)lock_ primitives when operating on the same spinlock.
> In this case it would be the irqsave(restore) variants.
>
> > dec_ret = mtk_jpeg_dec_get_int_status(jpeg->dec_reg_base);
> > dec_irq_ret = mtk_jpeg_dec_enum_result(dec_ret);
> > ctx = v4l2_m2m_get_curr_priv(jpeg->m2m_dev);
> > @@ -941,6 +943,7 @@ static irqreturn_t mtk_jpeg_dec_irq(int irq, void *priv)
> > v4l2_m2m_buf_done(src_buf, buf_state);
> > v4l2_m2m_buf_done(dst_buf, buf_state);
> > v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
> > + spin_unlock(&jpeg->hw_lock);
> > pm_runtime_put_sync(ctx->jpeg->dev);
> > return IRQ_HANDLED;
> > }
> > @@ -1191,7 +1194,6 @@ static __maybe_unused int mtk_jpeg_pm_suspend(struct device *dev)
> > {
> > struct mtk_jpeg_dev *jpeg = dev_get_drvdata(dev);
> >
> > - mtk_jpeg_dec_reset(jpeg->dec_reg_base);
> > mtk_jpeg_clk_off(jpeg);
> >
> > return 0;
> > @@ -1202,19 +1204,24 @@ static __maybe_unused int mtk_jpeg_pm_resume(struct device *dev)
> > struct mtk_jpeg_dev *jpeg = dev_get_drvdata(dev);
> >
> > mtk_jpeg_clk_on(jpeg);
> > - mtk_jpeg_dec_reset(jpeg->dec_reg_base);
> >
> > return 0;
> > }
> >
> > static __maybe_unused int mtk_jpeg_suspend(struct device *dev)
> > {
> > + struct mtk_jpeg_dev *jpeg = dev_get_drvdata(dev);
> > + unsigned long flags;
> > int ret;
> >
> > if (pm_runtime_suspended(dev))
> > return 0;
> >
> > + spin_lock_irqsave(&jpeg->hw_lock, flags);
>
> What does this spinlock protect us from? I can see that it would prevent
> the interrupt handler from being called, but is it okay to suspend the
> system without handling the interrupt?
Dear Tomasz,
I mean that if current image is processed in irq handler,suspend
function can not get the lock(it was locked in irq handler).Should I
move the spin_lock_irqsave(&jpeg->hw_lock, flags) to the start location
of suspend function or use wait_event_timeout() to handle the interrupt
before suspend?
Best Regards,
Xia Jiang
>
> > +
> > ret = mtk_jpeg_pm_suspend(dev);
> > +
>
> Looking at the implementation of mtk_jpeg_pm_suspend(), all it does is
> disabling the clock. How do we make sure that there is no frame currently
> being processed by the hardware?
>
> Best regards,
> Tomasz
^ permalink raw reply
* Re: [PATCH v25 03/16] dt: bindings: lp50xx: Introduce the lp50xx family of RGB drivers
From: Rob Herring @ 2020-05-27 1:59 UTC (permalink / raw)
To: Dan Murphy; +Cc: jacek.anaszewski, pavel, devicetree, linux-leds, linux-kernel
In-Reply-To: <20200526164652.2331-4-dmurphy@ti.com>
On Tue, May 26, 2020 at 11:46:39AM -0500, Dan Murphy wrote:
> Introduce the bindings for the Texas Instruments LP5036, LP5030, LP5024,
> LP5018, LP5012 and LP5009 RGB LED device driver. The LP5036/30/24/18/12/9
> can control RGB LEDs individually or as part of a control bank group.
> These devices have the ability to adjust the mixing control for the RGB
> LEDs to obtain different colors independent of the overall brightness of
> the LED grouping.
>
> Datasheet:
> http://www.ti.com/lit/ds/symlink/lp5012.pdf
> http://www.ti.com/lit/ds/symlink/lp5024.pdf
> http://www.ti.com/lit/ds/symlink/lp5036.pdf
>
> Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
> .../devicetree/bindings/leds/leds-lp50xx.yaml | 180 ++++++++++++++++++
> 1 file changed, 180 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/leds/leds-lp50xx.yaml
>
> diff --git a/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml b/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml
> new file mode 100644
> index 000000000000..a2ea03e07f6d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml
> @@ -0,0 +1,180 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/leds-lp50xx.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: LED driver for LP50XX RGB LED from Texas Instruments.
> +
> +maintainers:
> + - Dan Murphy <dmurphy@ti.com>
> +
> +description: |
> + The LP50XX is multi-channel, I2C RGB LED Drivers that can group RGB LEDs into
> + a LED group or control them individually.
> +
> + The difference in these RGB LED drivers is the number of supported RGB
> + modules.
> +
> + For more product information please see the link below:
> + http://www.ti.com/lit/ds/symlink/lp5012.pdf
> + http://www.ti.com/lit/ds/symlink/lp5024.pdf
> + http://www.ti.com/lit/ds/symlink/lp5036.pdf
> +
> +properties:
> + compatible:
> + oneOf:
> + - const: ti,lp5009
> + - const: ti,lp5012
> + - const: ti,lp5018
> + - const: ti,lp5024
> + - const: ti,lp5030
> + - const: ti,lp5036
Use enum rather than oneOf+const.
> +
> + reg:
> + maxItems: 1
> + description:
> + I2C slave address
> + lp5009/12 - 0x14, 0x15, 0x16, 0x17
> + lp5018/24 - 0x28, 0x29, 0x2a, 0x2b
> + lp5030/36 - 0x30, 0x31, 0x32, 0x33
> +
> + enable-gpios:
> + description: GPIO pin to enable/disable the device.
How many? (maxItems: 1)
> +
> + vled-supply:
> + description: LED supply.
> +
> + child-node:
This literally requires a node called 'child-node'. Not what you want.
You need a $ref to the multi-color schema in here and then only define
what's specific to this chip.
> + type: object
> + properties:
> + reg:
> + description: This is the LED module number.
Constraints?
> +
> + color:
> + description: Must be LED_COLOR_ID_MULTI
> +
> + function:
> + description: see Documentation/devicetree/bindings/leds/common.txt
> +
> + ti,led-bank:
> + description:
> + This property denotes the LED module numbers that will be controlled as
> + a single RGB cluster. Each LED module number will be controlled by a
> + single LED class instance.
> + There can only be one instance of the ti,led-bank
> + property for each device node. This is a required node is the LED
> + modules are to be backed.
> + $ref: /schemas/types.yaml#definitions/uint32-array
What is reg then? Some made up index? Can't you do:
reg = <1 2 3>;
led@1 {};
led@2 {};
led@2 {};
> +
> + required:
> + - reg
> + - color
> + - function
> +
> + grandchild-node:
Again, no.
> + type: object
> + properties:
> + reg:
> + description:
> + A single entry denoting the LED output that controls the monochrome LED.
Constraints?
> +
> + color:
> + description:
> + see Documentation/devicetree/bindings/leds/common.txt
Have you read this file recently? Don't add new references to it. (And
generally freeform references to other files are wrong with schemas).
> +
> + led-sources:
> + description:
> + see Documentation/devicetree/bindings/leds/common.txt
> + The LED outputs associated with the LED modules are defined in Table 1
> + of the corresponding data sheets.
> + LP5009 - 3 Total RGB cluster LED outputs 0-2
> + LP5012 - 4 Total RGB cluster LED outputs 0-3
> + LP5018 - 6 Total RGB cluster LED outputs 0-5
> + LP5024 - 8 Total RGB cluster LED outputs 0-7
> + LP5030 - 10 Total RGB cluster LED outputs 0-9
> + LP5036 - 12 Total RGB cluster LED outputs 0-11
> +
> + label:
> + description: |
> + Optional node - see Documentation/devicetree/bindings/leds/common.txt
> +
> + linux,default-trigger:
> + description: |
> + Optional node - see Documentation/devicetree/bindings/leds/common.txt
> +
> + required:
> + - reg
> + - color
> +
> +required:
> + - compatible
> + - reg
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> + #include <dt-bindings/leds/common.h>
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + led-controller@14 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "ti,lp5009";
> + reg = <0x14>;
> + enable-gpios = <&gpio1 16>;
> + multi-led@1 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <1>;
> + color = <LED_COLOR_ID_MULTI>;
> + function = LED_FUNCTION_CHARGING;
> +
> + led@0 {
> + reg = <0>;
> + color = <LED_COLOR_ID_RED>;
> + };
> +
> + led@1 {
> + reg = <1>;
> + color = <LED_COLOR_ID_GREEN>;
> + };
> +
> + led@2 {
> + reg = <2>;
> + color = <LED_COLOR_ID_BLUE>;
> + };
> + };
> +
> + multi-led@2 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <2>;
> + color = <LED_COLOR_ID_MULTI>;
> + function = LED_FUNCTION_STANDBY;
> + ti,led-bank = <2 3 5>;
> +
> + led@6 {
> + reg = <0x6>;
> + color = <LED_COLOR_ID_RED>;
> + led-sources = <6 9 15>;
> + };
> +
> + led@7 {
> + reg = <0x7>;
> + color = <LED_COLOR_ID_GREEN>;
> + led-sources = <7 10 16>;
> + };
> +
> + led@8 {
> + reg = <0x8>;
> + color = <LED_COLOR_ID_BLUE>;
> + led-sources = <8 11 17>;
> + };
> + };
> + };
> + };
> +
> +...
> --
> 2.25.1
>
^ permalink raw reply
* Re: [PATCH v25 05/16] dt: bindings: lp55xx: Be consistent in the document with LED acronym
From: Rob Herring @ 2020-05-27 1:59 UTC (permalink / raw)
To: Dan Murphy; +Cc: pavel, linux-kernel, jacek.anaszewski, linux-leds, devicetree
In-Reply-To: <20200526164652.2331-6-dmurphy@ti.com>
On Tue, 26 May 2020 11:46:41 -0500, Dan Murphy wrote:
> Update the document to be consistent in case when using "LED".
> This acronym should be capital throughout the document.
>
> Acked-by: Pavel Machek <pavel@ucw.cz>
> Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
> Documentation/devicetree/bindings/leds/leds-lp55xx.txt | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v25 06/16] dt: bindings: lp55xx: Update binding for Multicolor Framework
From: Rob Herring @ 2020-05-27 2:01 UTC (permalink / raw)
To: Dan Murphy
Cc: jacek.anaszewski, pavel, devicetree, linux-leds, linux-kernel,
Linus Walleij, Tony Lindgren, Benoît Cousson, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team
In-Reply-To: <20200526164652.2331-7-dmurphy@ti.com>
On Tue, May 26, 2020 at 11:46:42AM -0500, Dan Murphy wrote:
> Update the DT binding to include the properties to use the
> multicolor framework for the devices that use the LP55xx
> framework.
>
> Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> CC: Tony Lindgren <tony@atomide.com>
> CC: "Benoît Cousson" <bcousson@baylibre.com>
> CC: Linus Walleij <linus.walleij@linaro.org>
> CC: Shawn Guo <shawnguo@kernel.org>
> CC: Sascha Hauer <s.hauer@pengutronix.de>
> CC: Pengutronix Kernel Team <kernel@pengutronix.de>
> CC: Fabio Estevam <festevam@gmail.com>
> CC: NXP Linux Team <linux-imx@nxp.com>
> ---
> .../devicetree/bindings/leds/leds-lp55xx.txt | 149 +++++++++++++++---
> 1 file changed, 124 insertions(+), 25 deletions(-)
Convert this to schema first because it's going to need to reference
the multi-color schema.
Rob
^ permalink raw reply
* Re: [PATCH v8 2/2] clk: intel: Add CGU clock driver for a new SoC
From: Stephen Boyd @ 2020-05-27 2:10 UTC (permalink / raw)
To: Rahul Tanwar, linux-clk, mturquette
Cc: robh, mark.rutland, linux-kernel, devicetree, andriy.shevchenko,
qi-ming.wu, yixin.zhu, cheol.yong.kim, rtanwar, Rahul Tanwar
In-Reply-To: <42a4f71847714df482bacffdcd84341a4052800b.1587102634.git.rahul.tanwar@linux.intel.com>
Quoting Rahul Tanwar (2020-04-16 22:54:47)
> From: rtanwar <rahul.tanwar@intel.com>
>
> Clock Generation Unit(CGU) is a new clock controller IP of a forthcoming
> Intel network processor SoC named Lightning Mountain(LGM). It provides
> programming interfaces to control & configure all CPU & peripheral clocks.
> Add common clock framework based clock controller driver for CGU.
>
> Signed-off-by: Rahul Tanwar <rahul.tanwar@linux.intel.com>
> ---
Applied to clk-next
^ permalink raw reply
* Re: [PATCH v8 2/2] clk: intel: Add CGU clock driver for a new SoC
From: Stephen Boyd @ 2020-05-27 2:10 UTC (permalink / raw)
To: Rahul Tanwar, linux-clk, mturquette
Cc: robh, mark.rutland, linux-kernel, devicetree, andriy.shevchenko,
qi-ming.wu, yixin.zhu, cheol.yong.kim, rtanwar, Rahul Tanwar
In-Reply-To: <42a4f71847714df482bacffdcd84341a4052800b.1587102634.git.rahul.tanwar@linux.intel.com>
Quoting Rahul Tanwar (2020-04-16 22:54:47)
> diff --git a/drivers/clk/x86/clk-cgu.c b/drivers/clk/x86/clk-cgu.c
> new file mode 100644
> index 000000000000..802a7fa88535
> --- /dev/null
> +++ b/drivers/clk/x86/clk-cgu.c
> @@ -0,0 +1,636 @@
[...]
> +
> +static unsigned long
> +lgm_clk_ddiv_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
> +{
> + struct lgm_clk_ddiv *ddiv = to_lgm_clk_ddiv(hw);
> + unsigned int div0, div1, exdiv;
> + unsigned long flags;
> + u64 prate;
> +
> + spin_lock_irqsave(&ddiv->lock, flags);
Is there any reason to take the lock here? We should be able to
calculate the new rate and not care what the values are "right now"
because they can change in the interim. Instead we should recalculate a
rate that is possible regardless of the current state of the clk.
> + div0 = lgm_get_clk_val(ddiv->membase, ddiv->reg,
> + ddiv->shift0, ddiv->width0) + 1;
> + div1 = lgm_get_clk_val(ddiv->membase, ddiv->reg,
> + ddiv->shift1, ddiv->width1) + 1;
> + exdiv = lgm_get_clk_val(ddiv->membase, ddiv->reg,
> + ddiv->shift2, ddiv->width2);
> + spin_unlock_irqrestore(&ddiv->lock, flags);
> +
> + prate = (u64)parent_rate;
> + do_div(prate, div0);
> + do_div(prate, div1);
> +
> + if (exdiv) {
> + do_div(prate, ddiv->div);
> + prate *= ddiv->mult;
> + }
> +
> + return prate;
> +}
[...]
> +
> +static long
> +lgm_clk_ddiv_round_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long *prate)
> +{
> + struct lgm_clk_ddiv *ddiv = to_lgm_clk_ddiv(hw);
> + u32 div, ddiv1, ddiv2;
> + unsigned long flags;
> + u64 rate64 = rate;
> +
> + div = DIV_ROUND_CLOSEST_ULL((u64)*prate, rate);
> +
> + /* if predivide bit is enabled, modify div by factor of 2.5 */
> + spin_lock_irqsave(&ddiv->lock, flags);
> + if (lgm_get_clk_val(ddiv->membase, ddiv->reg, ddiv->shift2, 1)) {
> + div = div * 2;
> + div = DIV_ROUND_CLOSEST_ULL((u64)div, 5);
> + }
> +
> + if (div <= 0) {
> + spin_unlock_irqrestore(&ddiv->lock, flags);
> + return *prate;
> + }
> +
> + if (lgm_clk_get_ddiv_val(div, &ddiv1, &ddiv2) != 0) {
> + if (lgm_clk_get_ddiv_val(div + 1, &ddiv1, &ddiv2) != 0) {
> + spin_unlock_irqrestore(&ddiv->lock, flags);
> + return -EINVAL;
> + }
> + }
> +
> + rate64 = *prate;
> + do_div(rate64, ddiv1);
> + do_div(rate64, ddiv2);
> +
> + /* if predivide bit is enabled, modify rounded rate by factor of 2.5 */
> + if (lgm_get_clk_val(ddiv->membase, ddiv->reg, ddiv->shift2, 1)) {
> + rate64 = rate64 * 2;
> + rate64 = DIV_ROUND_CLOSEST_ULL(rate64, 5);
> + }
> + spin_unlock_irqrestore(&ddiv->lock, flags);
There's a lot of locking in here that can probably be tightened up.
Please look into only holding the spinlock as long as you need to.
> +
> + return rate64;
> +}
> +
> +static const struct clk_ops lgm_clk_ddiv_ops = {
> + .recalc_rate = lgm_clk_ddiv_recalc_rate,
> + .enable = lgm_clk_ddiv_enable,
> + .disable = lgm_clk_ddiv_disable,
> + .set_rate = lgm_clk_ddiv_set_rate,
> + .round_rate = lgm_clk_ddiv_round_rate,
> +};
> +
> +int lgm_clk_register_ddiv(struct lgm_clk_provider *ctx,
> + const struct lgm_clk_ddiv_data *list,
> + unsigned int nr_clk)
> +{
> + struct device *dev = ctx->dev;
> + struct clk_init_data init = {};
> + struct lgm_clk_ddiv *ddiv;
> + struct clk_hw *hw;
> + unsigned int idx;
> + int ret;
> +
> + for (idx = 0; idx < nr_clk; idx++, list++) {
> + ddiv = NULL;
Why assign to NULL?
> + ddiv = devm_kzalloc(dev, sizeof(*ddiv), GFP_KERNEL);
And then assign to it?
> + if (!ddiv)
> + return -ENOMEM;
> +
> + memset(&init, 0, sizeof(init));
Maybe 'init' and 'ddiv' should declared inside the for loop so that
they're automatically allocated again each time through the loop. Then
it would avoid the memset call and we wouldn't worry about 'ddiv'
needing to be NULL?
> + init.name = list->name;
> + init.ops = &lgm_clk_ddiv_ops;
> + init.flags = list->flags;
> + init.parent_data = list->parent_data;
> + init.num_parents = 1;
> +
> + ddiv->membase = ctx->membase;
> + ddiv->lock = ctx->lock;
> + ddiv->reg = list->reg;
> + ddiv->shift0 = list->shift0;
> + ddiv->width0 = list->width0;
> + ddiv->shift1 = list->shift1;
> + ddiv->width1 = list->width1;
> + ddiv->shift_gate = list->shift_gate;
> + ddiv->width_gate = list->width_gate;
> + ddiv->shift2 = list->ex_shift;
> + ddiv->width2 = list->ex_width;
> + ddiv->flags = list->div_flags;
> + ddiv->mult = 2;
> + ddiv->div = 5;
> + ddiv->hw.init = &init;
> +
> + hw = &ddiv->hw;
> + ret = clk_hw_register(dev, hw);
> + if (ret) {
> + dev_err(dev, "register clk: %s failed!\n", list->name);
> + return ret;
> + }
> + ctx->clk_data.hws[list->id] = hw;
> + }
> +
> + return 0;
> +}
> diff --git a/drivers/clk/x86/clk-cgu.h b/drivers/clk/x86/clk-cgu.h
> new file mode 100644
> index 000000000000..4e22bfb22312
> --- /dev/null
> +++ b/drivers/clk/x86/clk-cgu.h
> @@ -0,0 +1,335 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright(c) 2020 Intel Corporation.
> + * Zhu YiXin <yixin.zhu@intel.com>
> + * Rahul Tanwar <rahul.tanwar@intel.com>
> + */
> +
> +#ifndef __CLK_CGU_H
> +#define __CLK_CGU_H
> +
> +#include <linux/io.h>
> +
> +struct lgm_clk_mux {
> + struct clk_hw hw;
> + void __iomem *membase;
> + unsigned int reg;
> + u8 shift;
> + u8 width;
> + unsigned long flags;
> + spinlock_t lock;
> +};
> +
> +struct lgm_clk_divider {
> + struct clk_hw hw;
> + void __iomem *membase;
> + unsigned int reg;
> + u8 shift;
> + u8 width;
> + u8 shift_gate;
> + u8 width_gate;
> + unsigned long flags;
> + const struct clk_div_table *table;
> + spinlock_t lock;
> +};
> +
> +struct lgm_clk_ddiv {
> + struct clk_hw hw;
> + void __iomem *membase;
> + unsigned int reg;
> + u8 shift0;
> + u8 width0;
> + u8 shift1;
> + u8 width1;
> + u8 shift2;
> + u8 width2;
> + u8 shift_gate;
> + u8 width_gate;
> + unsigned int mult;
> + unsigned int div;
> + unsigned long flags;
> + spinlock_t lock;
> +};
> +
> +struct lgm_clk_gate {
> + struct clk_hw hw;
> + void __iomem *membase;
> + unsigned int reg;
> + u8 shift;
> + unsigned long flags;
> + spinlock_t lock;
> +};
> +
> +enum lgm_clk_type {
> + CLK_TYPE_FIXED,
> + CLK_TYPE_MUX,
> + CLK_TYPE_DIVIDER,
> + CLK_TYPE_FIXED_FACTOR,
> + CLK_TYPE_GATE,
> + CLK_TYPE_NONE,
> +};
> +
> +/**
> + * struct lgm_clk_provider
> + * @membase: IO mem base address for CGU.
> + * @np: device node
> + * @dev: device
> + * @clk_data: array of hw clocks and clk number.
> + */
> +struct lgm_clk_provider {
> + void __iomem *membase;
> + struct device_node *np;
> + struct device *dev;
> + struct clk_hw_onecell_data clk_data;
> + spinlock_t lock;
> +};
> +
> +enum pll_type {
> + TYPE_ROPLL,
> + TYPE_LJPLL,
> + TYPE_NONE,
> +};
> +
> +struct lgm_clk_pll {
> + struct clk_hw hw;
> + void __iomem *membase;
> + unsigned int reg;
> + unsigned long flags;
> + enum pll_type type;
> + spinlock_t lock;
> +};
> +
> +/**
> + * struct lgm_pll_clk_data
> + * @id: platform specific id of the clock.
> + * @name: name of this pll clock.
> + * @parent_data: parent clock data.
> + * @num_parents: number of parents.
> + * @flags: optional flags for basic clock.
> + * @type: platform type of pll.
> + * @reg: offset of the register.
> + */
> +struct lgm_pll_clk_data {
> + unsigned int id;
> + const char *name;
> + const struct clk_parent_data *parent_data;
> + u8 num_parents;
> + unsigned long flags;
> + enum pll_type type;
> + int reg;
> +};
> +
> +#define LGM_PLL(_id, _name, _pdata, _flags, \
> + _reg, _type) \
> + { \
> + .id = _id, \
> + .name = _name, \
> + .parent_data = _pdata, \
> + .num_parents = ARRAY_SIZE(_pdata), \
> + .flags = _flags, \
> + .reg = _reg, \
> + .type = _type, \
> + }
> +
> +struct lgm_clk_ddiv_data {
> + unsigned int id;
> + const char *name;
> + const struct clk_parent_data *parent_data;
> + u8 flags;
> + unsigned long div_flags;
> + unsigned int reg;
> + u8 shift0;
> + u8 width0;
> + u8 shift1;
> + u8 width1;
> + u8 shift_gate;
> + u8 width_gate;
> + u8 ex_shift;
> + u8 ex_width;
> +};
> +
> +#define LGM_DDIV(_id, _name, _pname, _flags, _reg, \
> + _shft0, _wdth0, _shft1, _wdth1, \
> + _shft_gate, _wdth_gate, _xshft, _df) \
> + { \
> + .id = _id, \
> + .name = _name, \
> + .parent_data = &(const struct clk_parent_data){ \
> + .fw_name = _pname, \
> + .name = _pname, \
> + }, \
> + .flags = _flags, \
> + .reg = _reg, \
> + .shift0 = _shft0, \
> + .width0 = _wdth0, \
> + .shift1 = _shft1, \
> + .width1 = _wdth1, \
> + .shift_gate = _shft_gate, \
> + .width_gate = _wdth_gate, \
> + .ex_shift = _xshft, \
> + .ex_width = 1, \
> + .div_flags = _df, \
> + }
> +
> +struct lgm_clk_branch {
> + unsigned int id;
> + enum lgm_clk_type type;
> + const char *name;
> + const struct clk_parent_data *parent_data;
> + u8 num_parents;
> + unsigned long flags;
> + unsigned int mux_off;
> + u8 mux_shift;
> + u8 mux_width;
> + unsigned long mux_flags;
> + unsigned int mux_val;
> + unsigned int div_off;
> + u8 div_shift;
> + u8 div_width;
> + u8 div_shift_gate;
> + u8 div_width_gate;
> + unsigned long div_flags;
> + unsigned int div_val;
> + const struct clk_div_table *div_table;
> + unsigned int gate_off;
> + u8 gate_shift;
> + unsigned long gate_flags;
> + unsigned int gate_val;
> + unsigned int mult;
> + unsigned int div;
> +};
> +
> +/* clock flags definition */
> +#define CLOCK_FLAG_VAL_INIT BIT(16)
> +#define MUX_CLK_SW BIT(17)
> +
> +#define LGM_MUX(_id, _name, _pdata, _f, _reg, \
> + _shift, _width, _cf, _v) \
> + { \
> + .id = _id, \
> + .type = CLK_TYPE_MUX, \
> + .name = _name, \
> + .parent_data = _pdata, \
> + .num_parents = ARRAY_SIZE(_pdata), \
> + .flags = _f, \
> + .mux_off = _reg, \
> + .mux_shift = _shift, \
> + .mux_width = _width, \
> + .mux_flags = _cf, \
> + .mux_val = _v, \
> + }
> +
> +#define LGM_DIV(_id, _name, _pname, _f, _reg, _shift, _width, \
> + _shift_gate, _width_gate, _cf, _v, _dtable) \
> + { \
> + .id = _id, \
> + .type = CLK_TYPE_DIVIDER, \
> + .name = _name, \
> + .parent_data = &(const struct clk_parent_data){ \
> + .fw_name = _pname, \
> + .name = _pname, \
> + }, \
> + .num_parents = 1, \
> + .flags = _f, \
> + .div_off = _reg, \
> + .div_shift = _shift, \
> + .div_width = _width, \
> + .div_shift_gate = _shift_gate, \
> + .div_width_gate = _width_gate, \
> + .div_flags = _cf, \
> + .div_val = _v, \
> + .div_table = _dtable, \
> + }
> +
> +#define LGM_GATE(_id, _name, _pname, _f, _reg, \
> + _shift, _cf, _v) \
> + { \
> + .id = _id, \
> + .type = CLK_TYPE_GATE, \
> + .name = _name, \
> + .parent_data = &(const struct clk_parent_data){ \
> + .fw_name = _pname, \
> + .name = _pname, \
> + }, \
> + .num_parents = !_pname ? 0 : 1, \
> + .flags = _f, \
> + .gate_off = _reg, \
> + .gate_shift = _shift, \
> + .gate_flags = _cf, \
> + .gate_val = _v, \
> + }
> +
> +#define LGM_FIXED(_id, _name, _pname, _f, _reg, \
> + _shift, _width, _cf, _freq, _v) \
> + { \
> + .id = _id, \
> + .type = CLK_TYPE_FIXED, \
> + .name = _name, \
> + .parent_data = &(const struct clk_parent_data){ \
> + .fw_name = _pname, \
> + .name = _pname, \
> + }, \
> + .num_parents = !_pname ? 0 : 1, \
> + .flags = _f, \
> + .div_off = _reg, \
> + .div_shift = _shift, \
> + .div_width = _width, \
> + .div_flags = _cf, \
> + .div_val = _v, \
> + .mux_flags = _freq, \
> + }
> +
> +#define LGM_FIXED_FACTOR(_id, _name, _pname, _f, _reg, \
> + _shift, _width, _cf, _v, _m, _d) \
> + { \
> + .id = _id, \
> + .type = CLK_TYPE_FIXED_FACTOR, \
> + .name = _name, \
> + .parent_data = &(const struct clk_parent_data){ \
> + .fw_name = _pname, \
> + .name = _pname, \
> + }, \
> + .num_parents = 1, \
> + .flags = _f, \
> + .div_off = _reg, \
> + .div_shift = _shift, \
> + .div_width = _width, \
> + .div_flags = _cf, \
> + .div_val = _v, \
> + .mult = _m, \
> + .div = _d, \
> + }
> +
> +static inline void lgm_set_clk_val(void __iomem *membase, u32 reg,
> + u8 shift, u8 width, u32 set_val)
> +{
> + u32 mask = (GENMASK(width - 1, 0) << shift);
> + u32 regval;
> +
> + regval = readl(membase + reg);
> + regval = (regval & ~mask) | ((set_val << shift) & mask);
> + writel(regval, membase + reg);
> +}
> +
> +static inline u32 lgm_get_clk_val(void __iomem *membase, u32 reg,
> + u8 shift, u8 width)
> +{
> + u32 mask = (GENMASK(width - 1, 0) << shift);
> + u32 val;
> +
> + val = readl(membase + reg);
> + val = (val & mask) >> shift;
> +
> + return val;
> +}
> +
> +int lgm_clk_register_branches(struct lgm_clk_provider *ctx,
> + const struct lgm_clk_branch *list,
> + unsigned int nr_clk);
> +int lgm_clk_register_plls(struct lgm_clk_provider *ctx,
> + const struct lgm_pll_clk_data *list,
> + unsigned int nr_clk);
> +int lgm_clk_register_ddiv(struct lgm_clk_provider *ctx,
> + const struct lgm_clk_ddiv_data *list,
> + unsigned int nr_clk);
> +#endif /* __CLK_CGU_H */
> diff --git a/drivers/clk/x86/clk-lgm.c b/drivers/clk/x86/clk-lgm.c
> new file mode 100644
> index 000000000000..ffbd2c425dc3
> --- /dev/null
> +++ b/drivers/clk/x86/clk-lgm.c
> @@ -0,0 +1,492 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020 Intel Corporation.
> + * Zhu YiXin <yixin.zhu@intel.com>
> + * Rahul Tanwar <rahul.tanwar@intel.com>
> + */
> +#include <linux/clk-provider.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <dt-bindings/clock/intel,lgm-clk.h>
> +#include "clk-cgu.h"
> +
> +#define PLL_DIV_WIDTH 4
> +#define PLL_DDIV_WIDTH 3
> +
> +/* Gate0 clock shift */
> +#define G_C55_SHIFT 7
> +#define G_QSPI_SHIFT 9
> +#define G_EIP197_SHIFT 11
> +#define G_VAULT130_SHIFT 12
> +#define G_TOE_SHIFT 13
> +#define G_SDXC_SHIFT 14
> +#define G_EMMC_SHIFT 15
> +#define G_SPIDBG_SHIFT 17
> +#define G_DMA3_SHIFT 28
> +
> +/* Gate1 clock shift */
> +#define G_DMA0_SHIFT 0
> +#define G_LEDC0_SHIFT 1
> +#define G_LEDC1_SHIFT 2
> +#define G_I2S0_SHIFT 3
> +#define G_I2S1_SHIFT 4
> +#define G_EBU_SHIFT 5
> +#define G_PWM_SHIFT 6
> +#define G_I2C0_SHIFT 7
> +#define G_I2C1_SHIFT 8
> +#define G_I2C2_SHIFT 9
> +#define G_I2C3_SHIFT 10
> +
> +#define G_SSC0_SHIFT 12
> +#define G_SSC1_SHIFT 13
> +#define G_SSC2_SHIFT 14
> +#define G_SSC3_SHIFT 15
> +
> +#define G_GPTC0_SHIFT 17
> +#define G_GPTC1_SHIFT 18
> +#define G_GPTC2_SHIFT 19
> +#define G_GPTC3_SHIFT 20
> +
> +#define G_ASC0_SHIFT 22
> +#define G_ASC1_SHIFT 23
> +#define G_ASC2_SHIFT 24
> +#define G_ASC3_SHIFT 25
> +
> +#define G_PCM0_SHIFT 27
> +#define G_PCM1_SHIFT 28
> +#define G_PCM2_SHIFT 29
> +
> +/* Gate2 clock shift */
> +#define G_PCIE10_SHIFT 1
> +#define G_PCIE11_SHIFT 2
> +#define G_PCIE30_SHIFT 3
> +#define G_PCIE31_SHIFT 4
> +#define G_PCIE20_SHIFT 5
> +#define G_PCIE21_SHIFT 6
> +#define G_PCIE40_SHIFT 7
> +#define G_PCIE41_SHIFT 8
> +
> +#define G_XPCS0_SHIFT 10
> +#define G_XPCS1_SHIFT 11
> +#define G_XPCS2_SHIFT 12
> +#define G_XPCS3_SHIFT 13
> +#define G_SATA0_SHIFT 14
> +#define G_SATA1_SHIFT 15
> +#define G_SATA2_SHIFT 16
> +#define G_SATA3_SHIFT 17
> +
> +/* Gate3 clock shift */
> +#define G_ARCEM4_SHIFT 0
> +#define G_IDMAR1_SHIFT 2
> +#define G_IDMAT0_SHIFT 3
> +#define G_IDMAT1_SHIFT 4
> +#define G_IDMAT2_SHIFT 5
> +
> +#define G_PPV4_SHIFT 8
> +#define G_GSWIPO_SHIFT 9
> +#define G_CQEM_SHIFT 10
> +#define G_XPCS5_SHIFT 14
> +#define G_USB1_SHIFT 25
> +#define G_USB2_SHIFT 26
> +
> +
> +/* Register definition */
> +#define CGU_PLL0CZ_CFG0 0x000
> +#define CGU_PLL0CM0_CFG0 0x020
> +#define CGU_PLL0CM1_CFG0 0x040
> +#define CGU_PLL0B_CFG0 0x060
> +#define CGU_PLL1_CFG0 0x080
> +#define CGU_PLL2_CFG0 0x0A0
> +#define CGU_PLLPP_CFG0 0x0C0
> +#define CGU_LJPLL3_CFG0 0x0E0
> +#define CGU_LJPLL4_CFG0 0x100
> +#define CGU_C55_PCMCR 0x18C
> +#define CGU_PCMCR 0x190
> +#define CGU_IF_CLK1 0x1A0
> +#define CGU_IF_CLK2 0x1A4
> +#define CGU_GATE0 0x300
> +#define CGU_GATE1 0x310
> +#define CGU_GATE2 0x320
> +#define CGU_GATE3 0x310
> +
> +#define PLL_DIV(x) ((x) + 0x04)
> +#define PLL_SSC(x) ((x) + 0x10)
> +
> +#define CLK_NR_CLKS (LGM_GCLK_USB2 + 1)
> +
> +/*
> + * Below table defines the pair's of regval & effective dividers.
> + * It's more efficient to provide an explicit table due to non-linear
> + * relation between values.
> + */
> +static const struct clk_div_table pll_div[] = {
> + { .val = 0, .div = 1 },
> + { .val = 1, .div = 2 },
> + { .val = 2, .div = 3 },
> + { .val = 3, .div = 4 },
> + { .val = 4, .div = 5 },
> + { .val = 5, .div = 6 },
> + { .val = 6, .div = 8 },
> + { .val = 7, .div = 10 },
> + { .val = 8, .div = 12 },
> + { .val = 9, .div = 16 },
> + { .val = 10, .div = 20 },
> + { .val = 11, .div = 24 },
> + { .val = 12, .div = 32 },
> + { .val = 13, .div = 40 },
> + { .val = 14, .div = 48 },
> + { .val = 15, .div = 64 },
> + {}
> +};
> +
> +static const struct clk_div_table dcl_div[] = {
> + { .val = 0, .div = 6 },
> + { .val = 1, .div = 12 },
> + { .val = 2, .div = 24 },
> + { .val = 3, .div = 32 },
> + { .val = 4, .div = 48 },
> + { .val = 5, .div = 96 },
> + {}
> +};
> +
> +static const struct clk_parent_data pll_p[] = {
> + { .fw_name = "osc", .name = "osc" },
> +};
> +static const struct clk_parent_data pllcm_p[] = {
> + { .fw_name = "cpu_cm", .name = "cpu_cm" },
> +};
> +static const struct clk_parent_data emmc_p[] = {
> + { .fw_name = "emmc4", .name = "emmc4" },
> + { .fw_name = "noc4", .name = "noc4" },
> +};
> +static const struct clk_parent_data sdxc_p[] = {
> + { .fw_name = "sdxc3", .name = "sdxc3" },
> + { .fw_name = "sdxc2", .name = "sdxc2" },
> +};
> +static const struct clk_parent_data pcm_p[] = {
> + { .fw_name = "v_docsis", .name = "v_docsis" },
> + { .fw_name = "dcl", .name = "dcl" },
> +};
> +static const struct clk_parent_data cbphy_p[] = {
> + { .fw_name = "dd_serdes", .name = "dd_serdes" },
> + { .fw_name = "dd_pcie", .name = "dd_pcie" },
> +};
> +
> +static const struct lgm_pll_clk_data lgm_pll_clks[] = {
> + LGM_PLL(LGM_CLK_PLL0CZ, "pll0cz", pll_p, CLK_IGNORE_UNUSED,
> + CGU_PLL0CZ_CFG0, TYPE_ROPLL),
> + LGM_PLL(LGM_CLK_PLL0CM0, "pll0cm0", pllcm_p, CLK_IGNORE_UNUSED,
> + CGU_PLL0CM0_CFG0, TYPE_ROPLL),
> + LGM_PLL(LGM_CLK_PLL0CM1, "pll0cm1", pllcm_p, CLK_IGNORE_UNUSED,
> + CGU_PLL0CM1_CFG0, TYPE_ROPLL),
> + LGM_PLL(LGM_CLK_PLL0B, "pll0b", pll_p, CLK_IGNORE_UNUSED,
> + CGU_PLL0B_CFG0, TYPE_ROPLL),
> + LGM_PLL(LGM_CLK_PLL1, "pll1", pll_p, 0, CGU_PLL1_CFG0, TYPE_ROPLL),
> + LGM_PLL(LGM_CLK_PLL2, "pll2", pll_p, CLK_IGNORE_UNUSED,
> + CGU_PLL2_CFG0, TYPE_ROPLL),
> + LGM_PLL(LGM_CLK_PLLPP, "pllpp", pll_p, 0, CGU_PLLPP_CFG0, TYPE_ROPLL),
> + LGM_PLL(LGM_CLK_LJPLL3, "ljpll3", pll_p, 0, CGU_LJPLL3_CFG0, TYPE_LJPLL),
> + LGM_PLL(LGM_CLK_LJPLL4, "ljpll4", pll_p, 0, CGU_LJPLL4_CFG0, TYPE_LJPLL),
> +};
> +
> +static const struct lgm_clk_branch lgm_branch_clks[] = {
> + LGM_DIV(LGM_CLK_PP_HW, "pp_hw", "pllpp", 0, PLL_DIV(CGU_PLLPP_CFG0),
> + 0, PLL_DIV_WIDTH, 24, 1, 0, 0, pll_div),
> + LGM_DIV(LGM_CLK_PP_UC, "pp_uc", "pllpp", 0, PLL_DIV(CGU_PLLPP_CFG0),
> + 4, PLL_DIV_WIDTH, 25, 1, 0, 0, pll_div),
> + LGM_DIV(LGM_CLK_PP_FXD, "pp_fxd", "pllpp", 0, PLL_DIV(CGU_PLLPP_CFG0),
> + 8, PLL_DIV_WIDTH, 26, 1, 0, 0, pll_div),
> + LGM_DIV(LGM_CLK_PP_TBM, "pp_tbm", "pllpp", 0, PLL_DIV(CGU_PLLPP_CFG0),
> + 12, PLL_DIV_WIDTH, 27, 1, 0, 0, pll_div),
> + LGM_DIV(LGM_CLK_DDR, "ddr", "pll2", CLK_IGNORE_UNUSED,
> + PLL_DIV(CGU_PLL2_CFG0), 0, PLL_DIV_WIDTH, 24, 1, 0, 0,
> + pll_div),
> + LGM_DIV(LGM_CLK_CM, "cpu_cm", "pll0cz", 0, PLL_DIV(CGU_PLL0CZ_CFG0),
> + 0, PLL_DIV_WIDTH, 24, 1, 0, 0, pll_div),
> +
> + LGM_DIV(LGM_CLK_IC, "cpu_ic", "pll0cz", CLK_IGNORE_UNUSED,
> + PLL_DIV(CGU_PLL0CZ_CFG0), 4, PLL_DIV_WIDTH, 25,
> + 1, 0, 0, pll_div),
> +
> + LGM_DIV(LGM_CLK_SDXC3, "sdxc3", "pll0cz", 0, PLL_DIV(CGU_PLL0CZ_CFG0),
> + 8, PLL_DIV_WIDTH, 26, 1, 0, 0, pll_div),
> +
> + LGM_DIV(LGM_CLK_CPU0, "cm0", "pll0cm0",
> + CLK_IGNORE_UNUSED, PLL_DIV(CGU_PLL0CM0_CFG0),
> + 0, PLL_DIV_WIDTH, 24, 1, 0, 0, pll_div),
> + LGM_DIV(LGM_CLK_CPU1, "cm1", "pll0cm1",
> + CLK_IGNORE_UNUSED, PLL_DIV(CGU_PLL0CM1_CFG0),
> + 0, PLL_DIV_WIDTH, 24, 1, 0, 0, pll_div),
> +
> + /*
> + * Marking ngi_clk (next generation interconnect) and noc_clk
> + * (network on chip peripheral clk) as critical clocks because
> + * these are shared parent clock sources for many different
> + * peripherals.
> + */
> + LGM_DIV(LGM_CLK_NGI, "ngi", "pll0b",
> + (CLK_IGNORE_UNUSED|CLK_IS_CRITICAL), PLL_DIV(CGU_PLL0B_CFG0),
> + 0, PLL_DIV_WIDTH, 24, 1, 0, 0, pll_div),
> + LGM_DIV(LGM_CLK_NOC4, "noc4", "pll0b",
> + (CLK_IGNORE_UNUSED|CLK_IS_CRITICAL), PLL_DIV(CGU_PLL0B_CFG0),
> + 4, PLL_DIV_WIDTH, 25, 1, 0, 0, pll_div),
> + LGM_DIV(LGM_CLK_SW, "switch", "pll0b", 0, PLL_DIV(CGU_PLL0B_CFG0),
> + 8, PLL_DIV_WIDTH, 26, 1, 0, 0, pll_div),
> + LGM_DIV(LGM_CLK_QSPI, "qspi", "pll0b", 0, PLL_DIV(CGU_PLL0B_CFG0),
> + 12, PLL_DIV_WIDTH, 27, 1, 0, 0, pll_div),
> + LGM_DIV(LGM_CLK_CT, "v_ct", "pll1", 0, PLL_DIV(CGU_PLL1_CFG0),
> + 0, PLL_DIV_WIDTH, 24, 1, 0, 0, pll_div),
> + LGM_DIV(LGM_CLK_DSP, "v_dsp", "pll1", 0, PLL_DIV(CGU_PLL1_CFG0),
> + 8, PLL_DIV_WIDTH, 26, 1, 0, 0, pll_div),
> + LGM_DIV(LGM_CLK_VIF, "v_ifclk", "pll1", 0, PLL_DIV(CGU_PLL1_CFG0),
> + 12, PLL_DIV_WIDTH, 27, 1, 0, 0, pll_div),
> +
> + LGM_FIXED_FACTOR(LGM_CLK_EMMC4, "emmc4", "sdxc3", 0, 0,
> + 0, 0, 0, 0, 1, 4),
> + LGM_FIXED_FACTOR(LGM_CLK_SDXC2, "sdxc2", "noc4", 0, 0,
> + 0, 0, 0, 0, 1, 4),
> + LGM_MUX(LGM_CLK_EMMC, "emmc", emmc_p, 0, CGU_IF_CLK1,
> + 0, 1, CLK_MUX_ROUND_CLOSEST, 0),
> + LGM_MUX(LGM_CLK_SDXC, "sdxc", sdxc_p, 0, CGU_IF_CLK1,
> + 1, 1, CLK_MUX_ROUND_CLOSEST, 0),
> + LGM_FIXED(LGM_CLK_OSC, "osc", NULL, 0, 0, 0, 0, 0, 40000000, 0),
> + LGM_FIXED(LGM_CLK_SLIC, "slic", NULL, 0, CGU_IF_CLK1,
> + 8, 2, CLOCK_FLAG_VAL_INIT, 8192000, 2),
> + LGM_FIXED(LGM_CLK_DOCSIS, "v_docsis", NULL, 0, 0, 0, 0, 0, 16000000, 0),
> + LGM_DIV(LGM_CLK_DCL, "dcl", "v_ifclk", 0, CGU_PCMCR,
> + 25, 3, 0, 0, 0, 0, dcl_div),
> + LGM_MUX(LGM_CLK_PCM, "pcm", pcm_p, 0, CGU_C55_PCMCR,
> + 0, 1, CLK_MUX_ROUND_CLOSEST, 0),
> + LGM_FIXED_FACTOR(LGM_CLK_DDR_PHY, "ddr_phy", "ddr",
> + CLK_IGNORE_UNUSED, 0,
> + 0, 0, 0, 0, 2, 1),
> + LGM_FIXED_FACTOR(LGM_CLK_PONDEF, "pondef", "dd_pool",
> + CLK_SET_RATE_PARENT, 0, 0, 0, 0, 0, 1, 2),
> + LGM_MUX(LGM_CLK_CBPHY0, "cbphy0", cbphy_p, 0, 0,
> + 0, 0, MUX_CLK_SW | CLK_MUX_ROUND_CLOSEST, 0),
> + LGM_MUX(LGM_CLK_CBPHY1, "cbphy1", cbphy_p, 0, 0,
> + 0, 0, MUX_CLK_SW | CLK_MUX_ROUND_CLOSEST, 0),
> + LGM_MUX(LGM_CLK_CBPHY2, "cbphy2", cbphy_p, 0, 0,
> + 0, 0, MUX_CLK_SW | CLK_MUX_ROUND_CLOSEST, 0),
> + LGM_MUX(LGM_CLK_CBPHY3, "cbphy3", cbphy_p, 0, 0,
> + 0, 0, MUX_CLK_SW | CLK_MUX_ROUND_CLOSEST, 0),
> +
> + LGM_GATE(LGM_GCLK_C55, "g_c55", NULL, 0, CGU_GATE0,
> + G_C55_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_QSPI, "g_qspi", "qspi", 0, CGU_GATE0,
> + G_QSPI_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_EIP197, "g_eip197", NULL, 0, CGU_GATE0,
> + G_EIP197_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_VAULT, "g_vault130", NULL, 0, CGU_GATE0,
> + G_VAULT130_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_TOE, "g_toe", NULL, 0, CGU_GATE0,
> + G_TOE_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_SDXC, "g_sdxc", "sdxc", 0, CGU_GATE0,
> + G_SDXC_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_EMMC, "g_emmc", "emmc", 0, CGU_GATE0,
> + G_EMMC_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_SPI_DBG, "g_spidbg", NULL, 0, CGU_GATE0,
> + G_SPIDBG_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_DMA3, "g_dma3", NULL, 0, CGU_GATE0,
> + G_DMA3_SHIFT, 0, 0),
> +
> + LGM_GATE(LGM_GCLK_DMA0, "g_dma0", NULL, 0, CGU_GATE1,
> + G_DMA0_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_LEDC0, "g_ledc0", NULL, 0, CGU_GATE1,
> + G_LEDC0_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_LEDC1, "g_ledc1", NULL, 0, CGU_GATE1,
> + G_LEDC1_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_I2S0, "g_i2s0", NULL, 0, CGU_GATE1,
> + G_I2S0_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_I2S1, "g_i2s1", NULL, 0, CGU_GATE1,
> + G_I2S1_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_EBU, "g_ebu", NULL, 0, CGU_GATE1,
> + G_EBU_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_PWM, "g_pwm", NULL, 0, CGU_GATE1,
> + G_PWM_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_I2C0, "g_i2c0", NULL, 0, CGU_GATE1,
> + G_I2C0_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_I2C1, "g_i2c1", NULL, 0, CGU_GATE1,
> + G_I2C1_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_I2C2, "g_i2c2", NULL, 0, CGU_GATE1,
> + G_I2C2_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_I2C3, "g_i2c3", NULL, 0, CGU_GATE1,
> + G_I2C3_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_SSC0, "g_ssc0", "noc4", 0, CGU_GATE1,
> + G_SSC0_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_SSC1, "g_ssc1", "noc4", 0, CGU_GATE1,
> + G_SSC1_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_SSC2, "g_ssc2", "noc4", 0, CGU_GATE1,
> + G_SSC2_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_SSC3, "g_ssc3", "noc4", 0, CGU_GATE1,
> + G_SSC3_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_GPTC0, "g_gptc0", "noc4", 0, CGU_GATE1,
> + G_GPTC0_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_GPTC1, "g_gptc1", "noc4", 0, CGU_GATE1,
> + G_GPTC1_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_GPTC2, "g_gptc2", "noc4", 0, CGU_GATE1,
> + G_GPTC2_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_GPTC3, "g_gptc3", "osc", 0, CGU_GATE1,
> + G_GPTC3_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_ASC0, "g_asc0", "noc4", 0, CGU_GATE1,
> + G_ASC0_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_ASC1, "g_asc1", "noc4", 0, CGU_GATE1,
> + G_ASC1_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_ASC2, "g_asc2", "noc4", 0, CGU_GATE1,
> + G_ASC2_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_ASC3, "g_asc3", "osc", 0, CGU_GATE1,
> + G_ASC3_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_PCM0, "g_pcm0", NULL, 0, CGU_GATE1,
> + G_PCM0_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_PCM1, "g_pcm1", NULL, 0, CGU_GATE1,
> + G_PCM1_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_PCM2, "g_pcm2", NULL, 0, CGU_GATE1,
> + G_PCM2_SHIFT, 0, 0),
> +
> + LGM_GATE(LGM_GCLK_PCIE10, "g_pcie10", NULL, 0, CGU_GATE2,
> + G_PCIE10_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_PCIE11, "g_pcie11", NULL, 0, CGU_GATE2,
> + G_PCIE11_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_PCIE30, "g_pcie30", NULL, 0, CGU_GATE2,
> + G_PCIE30_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_PCIE31, "g_pcie31", NULL, 0, CGU_GATE2,
> + G_PCIE31_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_PCIE20, "g_pcie20", NULL, 0, CGU_GATE2,
> + G_PCIE20_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_PCIE21, "g_pcie21", NULL, 0, CGU_GATE2,
> + G_PCIE21_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_PCIE40, "g_pcie40", NULL, 0, CGU_GATE2,
> + G_PCIE40_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_PCIE41, "g_pcie41", NULL, 0, CGU_GATE2,
> + G_PCIE41_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_XPCS0, "g_xpcs0", NULL, 0, CGU_GATE2,
> + G_XPCS0_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_XPCS1, "g_xpcs1", NULL, 0, CGU_GATE2,
> + G_XPCS1_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_XPCS2, "g_xpcs2", NULL, 0, CGU_GATE2,
> + G_XPCS2_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_XPCS3, "g_xpcs3", NULL, 0, CGU_GATE2,
> + G_XPCS3_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_SATA0, "g_sata0", NULL, 0, CGU_GATE2,
> + G_SATA0_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_SATA1, "g_sata1", NULL, 0, CGU_GATE2,
> + G_SATA1_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_SATA2, "g_sata2", NULL, 0, CGU_GATE2,
> + G_SATA2_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_SATA3, "g_sata3", NULL, 0, CGU_GATE2,
> + G_SATA3_SHIFT, 0, 0),
> +
> + LGM_GATE(LGM_GCLK_ARCEM4, "g_arcem4", NULL, 0, CGU_GATE3,
> + G_ARCEM4_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_IDMAR1, "g_idmar1", NULL, 0, CGU_GATE3,
> + G_IDMAR1_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_IDMAT0, "g_idmat0", NULL, 0, CGU_GATE3,
> + G_IDMAT0_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_IDMAT1, "g_idmat1", NULL, 0, CGU_GATE3,
> + G_IDMAT1_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_IDMAT2, "g_idmat2", NULL, 0, CGU_GATE3,
> + G_IDMAT2_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_PPV4, "g_ppv4", NULL, 0, CGU_GATE3,
> + G_PPV4_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_GSWIPO, "g_gswipo", "switch", 0, CGU_GATE3,
> + G_GSWIPO_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_CQEM, "g_cqem", "switch", 0, CGU_GATE3,
> + G_CQEM_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_XPCS5, "g_xpcs5", NULL, 0, CGU_GATE3,
> + G_XPCS5_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_USB1, "g_usb1", NULL, 0, CGU_GATE3,
> + G_USB1_SHIFT, 0, 0),
> + LGM_GATE(LGM_GCLK_USB2, "g_usb2", NULL, 0, CGU_GATE3,
> + G_USB2_SHIFT, 0, 0),
> +};
> +
> +
> +static const struct lgm_clk_ddiv_data lgm_ddiv_clks[] = {
> + LGM_DDIV(LGM_CLK_CML, "dd_cml", "ljpll3", 0,
> + PLL_DIV(CGU_LJPLL3_CFG0), 0, PLL_DDIV_WIDTH,
> + 3, PLL_DDIV_WIDTH, 24, 1, 29, 0),
> + LGM_DDIV(LGM_CLK_SERDES, "dd_serdes", "ljpll3", 0,
> + PLL_DIV(CGU_LJPLL3_CFG0), 6, PLL_DDIV_WIDTH,
> + 9, PLL_DDIV_WIDTH, 25, 1, 28, 0),
> + LGM_DDIV(LGM_CLK_POOL, "dd_pool", "ljpll3", 0,
> + PLL_DIV(CGU_LJPLL3_CFG0), 12, PLL_DDIV_WIDTH,
> + 15, PLL_DDIV_WIDTH, 26, 1, 28, 0),
> + LGM_DDIV(LGM_CLK_PTP, "dd_ptp", "ljpll3", 0,
> + PLL_DIV(CGU_LJPLL3_CFG0), 18, PLL_DDIV_WIDTH,
> + 21, PLL_DDIV_WIDTH, 27, 1, 28, 0),
> + LGM_DDIV(LGM_CLK_PCIE, "dd_pcie", "ljpll4", 0,
> + PLL_DIV(CGU_LJPLL4_CFG0), 0, PLL_DDIV_WIDTH,
> + 3, PLL_DDIV_WIDTH, 24, 1, 29, 0),
> +};
> +
> +static inline struct lgm_clk_provider *
> +lgm_clk_init(struct device *dev, unsigned int nr_clks)
> +{
> + struct lgm_clk_provider *ctx;
> +
> + ctx = devm_kzalloc(dev, struct_size(ctx, clk_data.hws, nr_clks),
> + GFP_KERNEL);
> + if (!ctx)
> + return ERR_PTR(-ENOMEM);
> +
> + ctx->clk_data.num = nr_clks;
> +
> + return ctx;
> +}
> +
> +static int lgm_cgu_probe(struct platform_device *pdev)
> +{
[...]
> +
> + ctx->membase = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(ctx->membase))
> + return PTR_ERR(ctx->membase);
> +
> + ctx->np = np;
> + ctx->dev = dev;
> + spin_lock_init(&ctx->lock);
> +
> + ret = lgm_clk_register_plls(ctx, lgm_pll_clks,
> + ARRAY_SIZE(lgm_pll_clks));
> + if (ret)
> + return ret;
> +
> + ret = lgm_clk_register_branches(ctx, lgm_branch_clks,
> + ARRAY_SIZE(lgm_branch_clks));
> + if (ret)
> + return ret;
> +
> + ret = lgm_clk_register_ddiv(ctx, lgm_ddiv_clks,
> + ARRAY_SIZE(lgm_ddiv_clks));
> + if (ret)
> + return ret;
> +
> + ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
> + &ctx->clk_data);
> + if (ret)
> + return ret;
Are any of the clks unregistered on failure? It looks like devm_ isn't
used for registration so nothing can be undone? Please fix this in a
future patch.
^ 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