* Re: [RFC PATCH v2 3/5] dt-bindings: clock: meson: document A1 SoC audio clock controller driver
From: Krzysztof Kozlowski @ 2024-03-28 9:01 UTC (permalink / raw)
To: Jan Dakinevich, Neil Armstrong, Jerome Brunet, Michael Turquette,
Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Kevin Hilman, Martin Blumenstingl, Philipp Zabel, linux-amlogic,
linux-clk, devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <20240328010831.884487-4-jan.dakinevich@salutedevices.com>
On 28/03/2024 02:08, Jan Dakinevich wrote:
> Add device tree bindings for A1 SoC audio clock and reset controllers.
>
> Signed-off-by: Jan Dakinevich <jan.dakinevich@salutedevices.com>
> ---
> +title: Amlogic A1 Audio Clock Control Unit and Reset Controller
> +
> +maintainers:
> + - Neil Armstrong <neil.armstrong@linaro.org>
> + - Jerome Brunet <jbrunet@baylibre.com>
> + - Jan Dakinevich <jan.dakinevich@salutedevices.com>
> +
> +properties:
> + compatible:
> + enum:
> + - amlogic,a1-audio-clkc
> + - amlogic,a1-audio2-clkc
What is "2"?
> +
> + '#clock-cells':
> + const: 1
> +
> + '#reset-cells':
> + const: 1
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + minItems: 6
> + maxItems: 7
> +
> + clock-names:
> + minItems: 6
> + maxItems: 7
> +
> +required:
> + - compatible
> + - '#clock-cells'
> + - reg
> + - clocks
> + - clock-names
> +
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - amlogic,a1-audio-clkc
> + then:
> + properties:
> + clocks:
> + items:
> + - description: input core clock
> + - description: input main peripheral bus clock
> + - description: input dds_in
> + - description: input fixed pll div2
> + - description: input fixed pll div3
> + - description: input hifi_pll
> + - description: input oscillator (usually at 24MHz)
> + clocks-names:
> + items:
> + - const: core
> + - const: pclk
> + - const: dds_in
> + - const: fclk_div2
> + - const: fclk_div3
> + - const: hifi_pll
> + - const: xtal
> + required:
> + - '#reset-cells'
> + else:
> + properties:
> + clocks:
> + items:
> + - description: input main peripheral bus clock
> + - description: input dds_in
> + - description: input fixed pll div2
> + - description: input fixed pll div3
> + - description: input hifi_pll
> + - description: input oscillator (usually at 24MHz)
> + clock-names:
> + items:
> + - const: pclk
> + - const: dds_in
> + - const: fclk_div2
> + - const: fclk_div3
> + - const: hifi_pll
> + - const: xtal
#reset-cells: false
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/amlogic,a1-pll-clkc.h>
> + #include <dt-bindings/clock/amlogic,a1-peripherals-clkc.h>
> + #include <dt-bindings/clock/amlogic,a1-audio-clkc.h>
> + audio {
If there is going to be any new version/resend:
soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + clkc_audio: audio-clock-controller@fe050000 {
Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
So: clock-controller
> + compatible = "amlogic,a1-audio-clkc";
> + reg = <0x0 0xfe050000 0x0 0xb0>;
> + #clock-cells = <1>;
> + #reset-cells = <1>;
> + clocks = <&clkc_audio2 AUD2_CLKID_AUDIOTOP>,
> + <&clkc_periphs CLKID_AUDIO>,
> + <&clkc_periphs CLKID_DDS_IN>,
> + <&clkc_pll CLKID_FCLK_DIV2>,
> + <&clkc_pll CLKID_FCLK_DIV3>,
> + <&clkc_pll CLKID_HIFI_PLL>,
> + <&xtal>;
> + clock-names = "core",
> + "pclk",
> + "dds_in",
> + "fclk_div2",
> + "fclk_div3",
> + "hifi_pll",
> + "xtal";
> + };
> +
> + clkc_audio2: audio-clock-controller@fe054800 {
clock-controller
(so I expect resend)
> + compatible = "amlogic,a1-audio2-clkc";
> + reg = <0x0 0xfe054800 0x0 0x20>;
> + #clock-cells = <1>;
> + clocks = <&clkc_periphs CLKID_AUDIO>,
> + <&clkc_periphs CLKID_DDS_IN>,
> + <&clkc_pll CLKID_FCLK_DIV2>,
> + <&clkc_pll CLKID_FCLK_DIV3>,
> + <&clkc_pll CLKID_HIFI_PLL>,
> + <&xtal>;
> + clock-names = "pclk",
> + "dds_in",
> + "fclk_div2",
> + "fclk_div3",
> + "hifi_pll",
> + "xtal";
> + };
> + };
> diff --git a/include/dt-bindings/clock/amlogic,a1-audio-clkc.h b/include/dt-bindings/clock/amlogic,a1-audio-clkc.h
> new file mode 100644
> index 000000000000..b30df3b1ae08
> --- /dev/null
> +++ b/include/dt-bindings/clock/amlogic,a1-audio-clkc.h
> @@ -0,0 +1,122 @@
> +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
> +/*
> + * Copyright (c) 2024, SaluteDevices. All Rights Reserved.
> + *
> + * Author: Jan Dakinevich <jan.dakinevich@salutedevices.com>
> + */
> +
> +#ifndef __A1_AUDIO_CLKC_BINDINGS_H
> +#define __A1_AUDIO_CLKC_BINDINGS_H
> +
> +#define AUD_CLKID_DDR_ARB 1
> +#define AUD_CLKID_TDMIN_A 2
> +#define AUD_CLKID_TDMIN_B 3
> +#define AUD_CLKID_TDMIN_LB 4
Why both clock controllers have the same clocks? This is confusing. It
seems you split same block into two!
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH] dt-bindings: ti,pcm1681: Convert to dtschema
From: Krzysztof Kozlowski @ 2024-03-28 8:57 UTC (permalink / raw)
To: Animesh Agarwal
Cc: Shenghao Ding, Kevin Lu, Baojun Xu, Liam Girdwood, Mark Brown,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, alsa-devel,
devicetree, linux-kernel
In-Reply-To: <20240328014029.9710-1-animeshagarwal28@gmail.com>
On 28/03/2024 02:40, Animesh Agarwal wrote:
> Convert the Texas Instruments PCM1681 bindings to DT schema.
>
> Signed-off-by: Animesh Agarwal <animeshagarwal28@gmail.com>
> ---
> .../devicetree/bindings/sound/ti,pcm1681.txt | 15 --------
> .../devicetree/bindings/sound/ti,pcm1681.yaml | 35 +++++++++++++++++++
> 2 files changed, 35 insertions(+), 15 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/sound/ti,pcm1681.txt
> create mode 100644 Documentation/devicetree/bindings/sound/ti,pcm1681.yaml
>
> diff --git a/Documentation/devicetree/bindings/sound/ti,pcm1681.txt b/Documentation/devicetree/bindings/sound/ti,pcm1681.txt
> deleted file mode 100644
> index 4df17185ab80..000000000000
> --- a/Documentation/devicetree/bindings/sound/ti,pcm1681.txt
> +++ /dev/null
> @@ -1,15 +0,0 @@
> -Texas Instruments PCM1681 8-channel PWM Processor
> -
> -Required properties:
> -
> - - compatible: Should contain "ti,pcm1681".
> - - reg: The i2c address. Should contain <0x4c>.
> -
> -Examples:
> -
> - i2c_bus {
> - pcm1681@4c {
> - compatible = "ti,pcm1681";
> - reg = <0x4c>;
> - };
> - };
> diff --git a/Documentation/devicetree/bindings/sound/ti,pcm1681.yaml b/Documentation/devicetree/bindings/sound/ti,pcm1681.yaml
> new file mode 100644
> index 000000000000..4093d0ff654d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/ti,pcm1681.yaml
> @@ -0,0 +1,35 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/ti,pcm1681.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Texas Instruments PCM1681 8-channel PWM Processor
> +
> +maintainers:
> + - Animesh Agarwal <animeshagarwal28@gmail.com>
Why not existing driver maintainers? Do you have this device? Or use it,
or care in terms of your projects?
> +
> +properties:
> + compatible:
> + const: ti,pcm1681
> +
> + reg:
> + maxItems: 1
> +
> +required:
> + - compatible
> + - reg
> +
Missing dai-cells, $ref to dai-common and unevaluatedProperties: false,
just like in other simple DAI devices. Mention briefly in the commit msg
adding these ("Make bindings complete by adding #sound-dai-cells").
> +additionalProperties: false
> +
> +examples:
> + - |
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + pcm1681@4c {
Datasheet says it is dac, but we usually call it "audio-codec".
> + compatible = "ti,pcm1681";
> + reg = <0x4c>;
> + };
> + };
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v3 2/2] dt-bindings: pinctrl: qcom: update functions to match with driver
From: Linus Walleij @ 2024-03-28 8:57 UTC (permalink / raw)
To: Tengfei Fan
Cc: andersson, konrad.dybcio, robh, krzysztof.kozlowski+dt, conor+dt,
dmitry.baryshkov, linux-arm-msm, linux-gpio, devicetree,
linux-kernel, kernel
In-Reply-To: <20240312025807.26075-3-quic_tengfan@quicinc.com>
On Tue, Mar 12, 2024 at 3:59 AM Tengfei Fan <quic_tengfan@quicinc.com> wrote:
> Some functions were consolidated in the SM4450 pinctrl driver, but they
> had not been updated in the binding file before the previous SM4450
> pinctrl patch series was merged.
(...)
> Fixes: 7bf8b78f86db ("dt-bindings: pinctrl: qcom: Add SM4450 pinctrl")
> Signed-off-by: Tengfei Fan <quic_tengfan@quicinc.com>
Patch applied.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v6 1/3] dt-bindings: mtd: Add Loongson-1 NAND Controller
From: Keguang Zhang @ 2024-03-28 8:54 UTC (permalink / raw)
To: Conor Dooley
Cc: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-mtd,
linux-kernel, linux-mips, devicetree
In-Reply-To: <20240327-bonehead-handlebar-1ca8dab95179@spud>
On Thu, Mar 28, 2024 at 12:23 AM Conor Dooley <conor@kernel.org> wrote:
>
> On Wed, Mar 27, 2024 at 06:43:59PM +0800, Keguang Zhang via B4 Relay wrote:
> > From: Keguang Zhang <keguang.zhang@gmail.com>
> >
> > Add devicetree binding document for Loongson-1 NAND Controller.
> >
> > Signed-off-by: Keguang Zhang <keguang.zhang@gmail.com>
> > ---
> > Changes in v6:
> > - A newly added patch
> > ---
> > .../devicetree/bindings/mtd/loongson,ls1x-nfc.yaml | 66 ++++++++++++++++++++++
> > 1 file changed, 66 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/mtd/loongson,ls1x-nfc.yaml b/Documentation/devicetree/bindings/mtd/loongson,ls1x-nfc.yaml
> > new file mode 100644
> > index 000000000000..2494c7b3b506
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mtd/loongson,ls1x-nfc.yaml
> > @@ -0,0 +1,66 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mtd/loongson,ls1x-nfc.yaml#
>
> Please make the filename match the compatible.
>
Got it. I'll rename it to loongson,ls1-nfc.yaml and change the
compatible as follows.
compatible:
items:
- enum:
- loongson,ls1a-nfc
- loongson,ls1b-nfc
- loongson,ls1c-nfc
- const: loongson,ls1-nfc
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Loongson-1 NAND Controller
> > +
> > +maintainers:
> > + - Keguang Zhang <keguang.zhang@gmail.com>
> > +
> > +allOf:
> > + - $ref: nand-controller.yaml
> > +
> > +properties:
> > + compatible:
> > + oneOf:
> > + - const: loongson,ls1b-nfc
> > + - items:
> > + - enum:
> > + - loongson,ls1a-nfc
> > + - loongson,ls1c-nfc
> > + - const: loongson,ls1b-nfc
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + dmas:
> > + maxItems: 1
> > +
> > + dma-names:
> > + const: rxtx
>
> If you only have one dma, why do you need a dma-names entry for it?
>
Without "dma-names", the following error will come out when doing
dt_binding_check.
nand-controller@1fe78000: 'dma-names' is a required property
In addition, loongson1_nand.c calls dma_request_chan(). Then
dma_request_chan() calls of_dma_request_slave_channel(), in which the
'dma-names' is necessary.
struct dma_chan *of_dma_request_slave_channel(struct device_node *np,
const char *name)
{
...
count = of_property_count_strings(np, "dma-names");
if (count < 0) {
pr_err("%s: dma-names property of node '%pOF' missing
or empty\n",
__func__, np);
return ERR_PTR(-ENODEV);
}
...
}
Thanks for your review!
> Looks fine to me otherwise though,
> COnor.
>
> > +
> > +patternProperties:
> > + "^nand@[0-3]$":
> > + type: object
> > + $ref: raw-nand-chip.yaml
> > +
> > + unevaluatedProperties: false
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - dmas
> > + - dma-names
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > + - |
> > + nand-controller@1fe78000 {
> > + compatible = "loongson,ls1b-nfc";
> > + reg = <0x1fe78000 0x40>;
> > +
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + dmas = <&dma 0>;
> > + dma-names = "rxtx";
> > +
> > + nand@0 {
> > + reg = <0>;
> > + nand-use-soft-ecc-engine;
> > + nand-ecc-algo = "hamming";
> > + };
> > + };
> >
> > --
> > 2.40.1
> >
> >
--
Best regards,
Keguang Zhang
^ permalink raw reply
* Re: [PATCH RESEND v6 4/5] spmi: pmic-arb: Make core resources acquiring a version operation
From: Neil Armstrong @ 2024-03-28 8:54 UTC (permalink / raw)
To: Abel Vesa, Stephen Boyd, Matthias Brugger, Bjorn Andersson,
Konrad Dybcio, Dmitry Baryshkov, AngeloGioacchino Del Regno,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Srini Kandagatla, Johan Hovold, linux-kernel, linux-arm-kernel,
linux-arm-msm, linux-mediatek, devicetree
In-Reply-To: <20240326-spmi-multi-master-support-v6-4-1c87d8306c5b@linaro.org>
On 26/03/2024 17:28, Abel Vesa wrote:
> Rather than setting up the core, obsrv and chnls in probe by using
> version specific conditionals, add a dedicated "get_core_resources"
> version specific op and move the acquiring in there.
>
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> ---
> drivers/spmi/spmi-pmic-arb.c | 113 +++++++++++++++++++++++++++----------------
> 1 file changed, 70 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
> index 38fed8a585fe..188252bfb95f 100644
> --- a/drivers/spmi/spmi-pmic-arb.c
> +++ b/drivers/spmi/spmi-pmic-arb.c
> @@ -203,6 +203,7 @@ struct spmi_pmic_arb {
> */
> struct pmic_arb_ver_ops {
> const char *ver_str;
> + int (*get_core_resources)(struct platform_device *pdev, void __iomem *core);
> int (*init_apid)(struct spmi_pmic_arb *pmic_arb, int index);
> int (*ppid_to_apid)(struct spmi_pmic_arb *pmic_arb, u16 ppid);
> /* spmi commands (read_cmd, write_cmd, cmd) functionality */
> @@ -956,6 +957,19 @@ static int pmic_arb_init_apid_min_max(struct spmi_pmic_arb *pmic_arb)
> return 0;
> }
>
> +static int pmic_arb_get_core_resources_v1(struct platform_device *pdev,
> + void __iomem *core)
> +{
> + struct spmi_pmic_arb *pmic_arb = platform_get_drvdata(pdev);
> +
> + pmic_arb->wr_base = core;
> + pmic_arb->rd_base = core;
> +
> + pmic_arb->max_periphs = PMIC_ARB_MAX_PERIPHS;
> +
> + return 0;
> +}
> +
> static int pmic_arb_init_apid_v1(struct spmi_pmic_arb *pmic_arb, int index)
> {
> u32 *mapping_table;
> @@ -1063,6 +1077,33 @@ static u16 pmic_arb_find_apid(struct spmi_pmic_arb *pmic_arb, u16 ppid)
> return apid;
> }
>
> +static int pmic_arb_get_obsrvr_chnls_v2(struct platform_device *pdev)
> +{
> + struct spmi_pmic_arb *pmic_arb = platform_get_drvdata(pdev);
> +
> + pmic_arb->rd_base = devm_platform_ioremap_resource_byname(pdev, "obsrvr");
> + if (IS_ERR(pmic_arb->rd_base))
> + return PTR_ERR(pmic_arb->rd_base);
> +
> + pmic_arb->wr_base = devm_platform_ioremap_resource_byname(pdev, "chnls");
> + if (IS_ERR(pmic_arb->wr_base))
> + return PTR_ERR(pmic_arb->wr_base);
> +
> + return 0;
> +}
> +
> +static int pmic_arb_get_core_resources_v2(struct platform_device *pdev,
> + void __iomem *core)
> +{
> + struct spmi_pmic_arb *pmic_arb = platform_get_drvdata(pdev);
> +
> + pmic_arb->core = core;
> +
> + pmic_arb->max_periphs = PMIC_ARB_MAX_PERIPHS;
> +
> + return pmic_arb_get_obsrvr_chnls_v2(pdev);
> +}
> +
> static int pmic_arb_ppid_to_apid_v2(struct spmi_pmic_arb *pmic_arb, u16 ppid)
> {
> u16 apid_valid;
> @@ -1246,6 +1287,18 @@ static int pmic_arb_offset_v5(struct spmi_pmic_arb *pmic_arb, u8 sid, u16 addr,
> return offset;
> }
>
> +static int pmic_arb_get_core_resources_v7(struct platform_device *pdev,
> + void __iomem *core)
> +{
> + struct spmi_pmic_arb *pmic_arb = platform_get_drvdata(pdev);
> +
> + pmic_arb->core = core;
> +
> + pmic_arb->max_periphs = PMIC_ARB_MAX_PERIPHS_V7;
> +
> + return pmic_arb_get_obsrvr_chnls_v2(pdev);
> +}
> +
> /*
> * Only v7 supports 2 buses. Each bus will get a different apid count, read
> * from different registers.
> @@ -1469,6 +1522,7 @@ pmic_arb_apid_owner_v7(struct spmi_pmic_arb *pmic_arb, u16 n)
>
> static const struct pmic_arb_ver_ops pmic_arb_v1 = {
> .ver_str = "v1",
> + .get_core_resources = pmic_arb_get_core_resources_v1,
> .init_apid = pmic_arb_init_apid_v1,
> .ppid_to_apid = pmic_arb_ppid_to_apid_v1,
> .non_data_cmd = pmic_arb_non_data_cmd_v1,
> @@ -1484,6 +1538,7 @@ static const struct pmic_arb_ver_ops pmic_arb_v1 = {
>
> static const struct pmic_arb_ver_ops pmic_arb_v2 = {
> .ver_str = "v2",
> + .get_core_resources = pmic_arb_get_core_resources_v2,
> .init_apid = pmic_arb_init_apid_v1,
> .ppid_to_apid = pmic_arb_ppid_to_apid_v2,
> .non_data_cmd = pmic_arb_non_data_cmd_v2,
> @@ -1499,6 +1554,7 @@ static const struct pmic_arb_ver_ops pmic_arb_v2 = {
>
> static const struct pmic_arb_ver_ops pmic_arb_v3 = {
> .ver_str = "v3",
> + .get_core_resources = pmic_arb_get_core_resources_v2,
> .init_apid = pmic_arb_init_apid_v1,
> .ppid_to_apid = pmic_arb_ppid_to_apid_v2,
> .non_data_cmd = pmic_arb_non_data_cmd_v2,
> @@ -1514,6 +1570,7 @@ static const struct pmic_arb_ver_ops pmic_arb_v3 = {
>
> static const struct pmic_arb_ver_ops pmic_arb_v5 = {
> .ver_str = "v5",
> + .get_core_resources = pmic_arb_get_core_resources_v2,
> .init_apid = pmic_arb_init_apid_v5,
> .ppid_to_apid = pmic_arb_ppid_to_apid_v5,
> .non_data_cmd = pmic_arb_non_data_cmd_v2,
> @@ -1529,6 +1586,7 @@ static const struct pmic_arb_ver_ops pmic_arb_v5 = {
>
> static const struct pmic_arb_ver_ops pmic_arb_v7 = {
> .ver_str = "v7",
> + .get_core_resources = pmic_arb_get_core_resources_v7,
> .init_apid = pmic_arb_init_apid_v7,
> .ppid_to_apid = pmic_arb_ppid_to_apid_v5,
> .non_data_cmd = pmic_arb_non_data_cmd_v2,
> @@ -1565,16 +1623,6 @@ static int spmi_pmic_arb_probe(struct platform_device *pdev)
> pmic_arb = spmi_controller_get_drvdata(ctrl);
> pmic_arb->spmic = ctrl;
>
> - /*
> - * Please don't replace this with devm_platform_ioremap_resource() or
> - * devm_ioremap_resource(). These both result in a call to
> - * devm_request_mem_region() which prevents multiple mappings of this
> - * register address range. SoCs with PMIC arbiter v7 may define two
> - * arbiter devices, for the two physical SPMI interfaces, which share
> - * some register address ranges (i.e. "core", "obsrvr", and "chnls").
> - * Ensure that both devices probe successfully by calling devm_ioremap()
> - * which does not result in a devm_request_mem_region() call.
> - */
Can you explain in the commit message why you remove this comment ?
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "core");
> core = devm_ioremap(&ctrl->dev, res->start, resource_size(res));
> if (IS_ERR(core))
> @@ -1584,44 +1632,23 @@ static int spmi_pmic_arb_probe(struct platform_device *pdev)
>
> hw_ver = readl_relaxed(core + PMIC_ARB_VERSION);
>
> - if (hw_ver < PMIC_ARB_VERSION_V2_MIN) {
> + if (hw_ver < PMIC_ARB_VERSION_V2_MIN)
> pmic_arb->ver_ops = &pmic_arb_v1;
> - pmic_arb->wr_base = core;
> - pmic_arb->rd_base = core;
> - } else {
> - pmic_arb->core = core;
> -
> - if (hw_ver < PMIC_ARB_VERSION_V3_MIN)
> - pmic_arb->ver_ops = &pmic_arb_v2;
> - else if (hw_ver < PMIC_ARB_VERSION_V5_MIN)
> - pmic_arb->ver_ops = &pmic_arb_v3;
> - else if (hw_ver < PMIC_ARB_VERSION_V7_MIN)
> - pmic_arb->ver_ops = &pmic_arb_v5;
> - else
> - pmic_arb->ver_ops = &pmic_arb_v7;
> -
> - res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> - "obsrvr");
> - pmic_arb->rd_base = devm_ioremap(&ctrl->dev, res->start,
> - resource_size(res));
> - if (IS_ERR(pmic_arb->rd_base))
> - return PTR_ERR(pmic_arb->rd_base);
> -
> - res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> - "chnls");
> - pmic_arb->wr_base = devm_ioremap(&ctrl->dev, res->start,
> - resource_size(res));
> - if (IS_ERR(pmic_arb->wr_base))
> - return PTR_ERR(pmic_arb->wr_base);
> - }
> + else if (hw_ver < PMIC_ARB_VERSION_V3_MIN)
> + pmic_arb->ver_ops = &pmic_arb_v2;
> + else if (hw_ver < PMIC_ARB_VERSION_V5_MIN)
> + pmic_arb->ver_ops = &pmic_arb_v3;
> + else if (hw_ver < PMIC_ARB_VERSION_V7_MIN)
> + pmic_arb->ver_ops = &pmic_arb_v5;
> + else
> + pmic_arb->ver_ops = &pmic_arb_v7;
>
> dev_info(&ctrl->dev, "PMIC arbiter version %s (0x%x)\n",
> pmic_arb->ver_ops->ver_str, hw_ver);
>
> - if (hw_ver < PMIC_ARB_VERSION_V7_MIN)
> - pmic_arb->max_periphs = PMIC_ARB_MAX_PERIPHS;
> - else
> - pmic_arb->max_periphs = PMIC_ARB_MAX_PERIPHS_V7;
> + err = pmic_arb->ver_ops->get_core_resources(pdev, core);
> + if (err)
> + return err;
>
> err = pmic_arb->ver_ops->init_apid(pmic_arb, 0);
> if (err)
>
With that added:
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
^ permalink raw reply
* Re: [PATCH 2/3] dt-bindings: ufs: qcom: document SC7180 UFS
From: Krzysztof Kozlowski @ 2024-03-28 8:52 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Bjorn Andersson, Konrad Dybcio, Alim Akhtar, Avri Altman,
Bart Van Assche, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Andy Gross, linux-arm-msm, linux-scsi, devicetree, linux-kernel
In-Reply-To: <20240328035909.GB3212@thinkpad>
On 28/03/2024 04:59, Manivannan Sadhasivam wrote:
>> - if:
>> properties:
>> compatible:
>> @@ -250,7 +276,7 @@ allOf:
>> reg:
>> maxItems: 1
>> clocks:
>> - minItems: 8
>> + minItems: 7
>> maxItems: 8
>> else:
>> properties:
>> @@ -258,7 +284,7 @@ allOf:
>> minItems: 1
>> maxItems: 2
>> clocks:
>> - minItems: 8
>> + minItems: 7
>
> I'm getting confused by the clock requirements for qcom,ice. Why does specifying
> the qcom,ice phandle require these clocks? These are the UFSHC clocks and
> already defined above.
I am also confused, but I did not change that logic. I don't think that
it is anyway useful, but that separate topic from this patch.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 17/23] dt-bindings: media: imx258: Rename to include vendor prefix
From: Kieran Bingham @ 2024-03-28 8:52 UTC (permalink / raw)
To: Conor Dooley, git
Cc: linux-media, dave.stevenson, jacopo.mondi, mchehab, robh,
krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
festevam, sakari.ailus, devicetree, imx, linux-arm-kernel,
linux-kernel, Conor Dooley
In-Reply-To: <0e658ecc-38d2-4d6f-b0cf-f5f3ec32c1b8@luigi311.com>
Quoting git@luigi311.com (2024-03-28 00:57:34)
> On 3/27/24 17:47, Conor Dooley wrote:
> > On Wed, Mar 27, 2024 at 05:17:03PM -0600, git@luigi311.com wrote:
> >> From: Dave Stevenson <dave.stevenson@raspberrypi.com>
> >>
> >> imx258.yaml doesn't include the vendor prefix of sony, so
> >> rename to add it.
> >> Update the id entry and MAINTAINERS to match.
> >>
> >> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> >> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> >
> > This is a v1 with my ack, something has gone awry here. It's also
> > missing your signoff. Did you pick up someone else's series?
>
> Yes, this is a continuation of Dave's work. I contacted him directly,
> and he mentioned that he is unable to submit a v2 any time soon and
> was open to someone else continuing it in his stead. This is my first
> time submitting a patch via a mailing list, so I'm not sure if I'm
> missing something, but I only added my sign off for anything that
> actually included work from my side and not just bringing his patch
> forward to this patch series.
Your cover letter states v2, but the individual patches do not.
Add the '-v2' (or, rather, next it will be '-v3') to git format-patch
when you save your series and it will add the version to each patch. You
can also add '-s' to that command I believe to add your SoB to each
patch.
--
Kieran
>
> >
> >> ---
> >> .../bindings/media/i2c/{imx258.yaml => sony,imx258.yaml} | 2 +-
> >> MAINTAINERS | 2 +-
> >> 2 files changed, 2 insertions(+), 2 deletions(-)
> >> rename Documentation/devicetree/bindings/media/i2c/{imx258.yaml => sony,imx258.yaml} (97%)
> >>
> >> diff --git a/Documentation/devicetree/bindings/media/i2c/imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> >> similarity index 97%
> >> rename from Documentation/devicetree/bindings/media/i2c/imx258.yaml
> >> rename to Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> >> index 80d24220baa0..bee61a443b23 100644
> >> --- a/Documentation/devicetree/bindings/media/i2c/imx258.yaml
> >> +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> >> @@ -1,7 +1,7 @@
> >> # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >> %YAML 1.2
> >> ---
> >> -$id: http://devicetree.org/schemas/media/i2c/imx258.yaml#
> >> +$id: http://devicetree.org/schemas/media/i2c/sony,imx258.yaml#
> >> $schema: http://devicetree.org/meta-schemas/core.yaml#
> >>
> >> title: Sony IMX258 13 Mpixel CMOS Digital Image Sensor
> >> diff --git a/MAINTAINERS b/MAINTAINERS
> >> index aa3b947fb080..1f17f6734bf5 100644
> >> --- a/MAINTAINERS
> >> +++ b/MAINTAINERS
> >> @@ -20464,7 +20464,7 @@ M: Sakari Ailus <sakari.ailus@linux.intel.com>
> >> L: linux-media@vger.kernel.org
> >> S: Maintained
> >> T: git git://linuxtv.org/media_tree.git
> >> -F: Documentation/devicetree/bindings/media/i2c/imx258.yaml
> >> +F: Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> >> F: drivers/media/i2c/imx258.c
> >>
> >> SONY IMX274 SENSOR DRIVER
> >> --
> >> 2.42.0
> >>
>
^ permalink raw reply
* Re: [PATCH RESEND v6 3/5] spmi: pmic-arb: Make the APID init a version operation
From: Neil Armstrong @ 2024-03-28 8:51 UTC (permalink / raw)
To: Abel Vesa, Stephen Boyd, Matthias Brugger, Bjorn Andersson,
Konrad Dybcio, Dmitry Baryshkov, AngeloGioacchino Del Regno,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Srini Kandagatla, Johan Hovold, linux-kernel, linux-arm-kernel,
linux-arm-msm, linux-mediatek, devicetree
In-Reply-To: <20240326-spmi-multi-master-support-v6-3-1c87d8306c5b@linaro.org>
On 26/03/2024 17:28, Abel Vesa wrote:
> Rather than using conditionals in probe function, add the APID init
> as a version specific operation. Due to v7, which supports multiple
> buses, pass on the bus index to be used for sorting out the apid base
> and count.
>
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> ---
> drivers/spmi/spmi-pmic-arb.c | 199 +++++++++++++++++++++++++++----------------
> 1 file changed, 124 insertions(+), 75 deletions(-)
>
> diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
> index 9ed1180fe31f..38fed8a585fe 100644
> --- a/drivers/spmi/spmi-pmic-arb.c
> +++ b/drivers/spmi/spmi-pmic-arb.c
> @@ -183,6 +183,7 @@ struct spmi_pmic_arb {
> * struct pmic_arb_ver_ops - version dependent functionality.
> *
> * @ver_str: version string.
> + * @init_apid: finds the apid base and count
> * @ppid_to_apid: finds the apid for a given ppid.
> * @non_data_cmd: on v1 issues an spmi non-data command.
> * on v2 no HW support, returns -EOPNOTSUPP.
> @@ -202,6 +203,7 @@ struct spmi_pmic_arb {
> */
> struct pmic_arb_ver_ops {
> const char *ver_str;
> + int (*init_apid)(struct spmi_pmic_arb *pmic_arb, int index);
> int (*ppid_to_apid)(struct spmi_pmic_arb *pmic_arb, u16 ppid);
> /* spmi commands (read_cmd, write_cmd, cmd) functionality */
> int (*offset)(struct spmi_pmic_arb *pmic_arb, u8 sid, u16 addr,
> @@ -942,6 +944,38 @@ static int qpnpint_irq_domain_alloc(struct irq_domain *domain,
> return 0;
> }
>
> +static int pmic_arb_init_apid_min_max(struct spmi_pmic_arb *pmic_arb)
> +{
> + /*
> + * Initialize max_apid/min_apid to the opposite bounds, during
> + * the irq domain translation, we are sure to update these
> + */
> + pmic_arb->max_apid = 0;
> + pmic_arb->min_apid = pmic_arb->max_periphs - 1;
> +
> + return 0;
> +}
> +
> +static int pmic_arb_init_apid_v1(struct spmi_pmic_arb *pmic_arb, int index)
> +{
> + u32 *mapping_table;
> +
> + if (index) {
> + dev_err(&pmic_arb->spmic->dev, "Unsupported buses count %d detected\n",
> + index);
> + return -EINVAL;
> + }
> +
> + mapping_table = devm_kcalloc(&pmic_arb->spmic->dev, pmic_arb->max_periphs,
> + sizeof(*mapping_table), GFP_KERNEL);
> + if (!mapping_table)
> + return -ENOMEM;
> +
> + pmic_arb->mapping_table = mapping_table;
Can you specify in the spmi_pmic_arb->mapping_table struct documentation the mapping_table
is only used in v1 ? or even better rename it to mapping_table_v1
> +
> + return pmic_arb_init_apid_min_max(pmic_arb);
> +}
> +
> static int pmic_arb_ppid_to_apid_v1(struct spmi_pmic_arb *pmic_arb, u16 ppid)
> {
> u32 *mapping_table = pmic_arb->mapping_table;
> @@ -1144,6 +1178,40 @@ static int pmic_arb_offset_v2(struct spmi_pmic_arb *pmic_arb, u8 sid, u16 addr,
> return 0x1000 * pmic_arb->ee + 0x8000 * apid;
> }
>
> +static int pmic_arb_init_apid_v5(struct spmi_pmic_arb *pmic_arb, int index)
> +{
> + int ret;
> +
> + if (index) {
> + dev_err(&pmic_arb->spmic->dev, "Unsupported buses count %d detected\n",
> + index);
> + return -EINVAL;
> + }
> +
> + pmic_arb->base_apid = 0;
> + pmic_arb->apid_count = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES) &
> + PMIC_ARB_FEATURES_PERIPH_MASK;
> +
> + if (pmic_arb->base_apid + pmic_arb->apid_count > pmic_arb->max_periphs) {
> + dev_err(&pmic_arb->spmic->dev, "Unsupported APID count %d detected\n",
> + pmic_arb->base_apid + pmic_arb->apid_count);
> + return -EINVAL;
> + }
> +
> + ret = pmic_arb_init_apid_min_max(pmic_arb);
> + if (ret)
> + return ret;
> +
> + ret = pmic_arb_read_apid_map_v5(pmic_arb);
> + if (ret) {
> + dev_err(&pmic_arb->spmic->dev, "could not read APID->PPID mapping table, rc= %d\n",
> + ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> /*
> * v5 offset per ee and per apid for observer channels and per apid for
> * read/write channels.
> @@ -1178,6 +1246,49 @@ static int pmic_arb_offset_v5(struct spmi_pmic_arb *pmic_arb, u8 sid, u16 addr,
> return offset;
> }
>
> +/*
> + * Only v7 supports 2 buses. Each bus will get a different apid count, read
> + * from different registers.
> + */
> +static int pmic_arb_init_apid_v7(struct spmi_pmic_arb *pmic_arb, int index)
> +{
> + int ret;
> +
> + if (index == 0) {
> + pmic_arb->base_apid = 0;
> + pmic_arb->apid_count = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES) &
> + PMIC_ARB_FEATURES_PERIPH_MASK;
> + } else if (index == 1) {
> + pmic_arb->base_apid = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES) &
> + PMIC_ARB_FEATURES_PERIPH_MASK;
> + pmic_arb->apid_count = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES1) &
> + PMIC_ARB_FEATURES_PERIPH_MASK;
> + } else {
> + dev_err(&pmic_arb->spmic->dev, "Unsupported buses count %d detected\n",
> + index);
> + return -EINVAL;
> + }
> +
> + if (pmic_arb->base_apid + pmic_arb->apid_count > pmic_arb->max_periphs) {
> + dev_err(&pmic_arb->spmic->dev, "Unsupported APID count %d detected\n",
> + pmic_arb->base_apid + pmic_arb->apid_count);
> + return -EINVAL;
> + }
> +
> + ret = pmic_arb_init_apid_min_max(pmic_arb);
> + if (ret)
> + return ret;
> +
> + ret = pmic_arb_read_apid_map_v5(pmic_arb);
> + if (ret) {
> + dev_err(&pmic_arb->spmic->dev, "could not read APID->PPID mapping table, rc= %d\n",
> + ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> /*
> * v7 offset per ee and per apid for observer channels and per apid for
> * read/write channels.
> @@ -1358,6 +1469,7 @@ pmic_arb_apid_owner_v7(struct spmi_pmic_arb *pmic_arb, u16 n)
>
> static const struct pmic_arb_ver_ops pmic_arb_v1 = {
> .ver_str = "v1",
> + .init_apid = pmic_arb_init_apid_v1,
> .ppid_to_apid = pmic_arb_ppid_to_apid_v1,
> .non_data_cmd = pmic_arb_non_data_cmd_v1,
> .offset = pmic_arb_offset_v1,
> @@ -1372,6 +1484,7 @@ static const struct pmic_arb_ver_ops pmic_arb_v1 = {
>
> static const struct pmic_arb_ver_ops pmic_arb_v2 = {
> .ver_str = "v2",
> + .init_apid = pmic_arb_init_apid_v1,
> .ppid_to_apid = pmic_arb_ppid_to_apid_v2,
> .non_data_cmd = pmic_arb_non_data_cmd_v2,
> .offset = pmic_arb_offset_v2,
> @@ -1386,6 +1499,7 @@ static const struct pmic_arb_ver_ops pmic_arb_v2 = {
>
> static const struct pmic_arb_ver_ops pmic_arb_v3 = {
> .ver_str = "v3",
> + .init_apid = pmic_arb_init_apid_v1,
> .ppid_to_apid = pmic_arb_ppid_to_apid_v2,
> .non_data_cmd = pmic_arb_non_data_cmd_v2,
> .offset = pmic_arb_offset_v2,
> @@ -1400,6 +1514,7 @@ static const struct pmic_arb_ver_ops pmic_arb_v3 = {
>
> static const struct pmic_arb_ver_ops pmic_arb_v5 = {
> .ver_str = "v5",
> + .init_apid = pmic_arb_init_apid_v5,
> .ppid_to_apid = pmic_arb_ppid_to_apid_v5,
> .non_data_cmd = pmic_arb_non_data_cmd_v2,
> .offset = pmic_arb_offset_v5,
> @@ -1414,6 +1529,7 @@ static const struct pmic_arb_ver_ops pmic_arb_v5 = {
>
> static const struct pmic_arb_ver_ops pmic_arb_v7 = {
> .ver_str = "v7",
> + .init_apid = pmic_arb_init_apid_v7,
> .ppid_to_apid = pmic_arb_ppid_to_apid_v5,
> .non_data_cmd = pmic_arb_non_data_cmd_v2,
> .offset = pmic_arb_offset_v7,
> @@ -1439,7 +1555,6 @@ static int spmi_pmic_arb_probe(struct platform_device *pdev)
> struct spmi_controller *ctrl;
> struct resource *res;
> void __iomem *core;
> - u32 *mapping_table;
> u32 channel, ee, hw_ver;
> int err;
>
> @@ -1467,12 +1582,6 @@ static int spmi_pmic_arb_probe(struct platform_device *pdev)
>
> pmic_arb->core_size = resource_size(res);
>
> - pmic_arb->ppid_to_apid = devm_kcalloc(&ctrl->dev, PMIC_ARB_MAX_PPID,
> - sizeof(*pmic_arb->ppid_to_apid),
> - GFP_KERNEL);
> - if (!pmic_arb->ppid_to_apid)
> - return -ENOMEM;
> -
> hw_ver = readl_relaxed(core + PMIC_ARB_VERSION);
>
> if (hw_ver < PMIC_ARB_VERSION_V2_MIN) {
> @@ -1506,58 +1615,17 @@ static int spmi_pmic_arb_probe(struct platform_device *pdev)
> return PTR_ERR(pmic_arb->wr_base);
> }
>
> - pmic_arb->max_periphs = PMIC_ARB_MAX_PERIPHS;
> + dev_info(&ctrl->dev, "PMIC arbiter version %s (0x%x)\n",
> + pmic_arb->ver_ops->ver_str, hw_ver);
>
> - if (hw_ver >= PMIC_ARB_VERSION_V7_MIN) {
> + if (hw_ver < PMIC_ARB_VERSION_V7_MIN)
> + pmic_arb->max_periphs = PMIC_ARB_MAX_PERIPHS;
> + else
> pmic_arb->max_periphs = PMIC_ARB_MAX_PERIPHS_V7;
> - /* Optional property for v7: */
> - of_property_read_u32(pdev->dev.of_node, "qcom,bus-id",
> - &pmic_arb->bus_instance);
> - if (pmic_arb->bus_instance > 1) {
> - dev_err(&pdev->dev, "invalid bus instance (%u) specified\n",
> - pmic_arb->bus_instance);
> - return -EINVAL;
> - }
> -
> - if (pmic_arb->bus_instance == 0) {
> - pmic_arb->base_apid = 0;
> - pmic_arb->apid_count =
> - readl_relaxed(core + PMIC_ARB_FEATURES) &
> - PMIC_ARB_FEATURES_PERIPH_MASK;
> - } else {
> - pmic_arb->base_apid =
> - readl_relaxed(core + PMIC_ARB_FEATURES) &
> - PMIC_ARB_FEATURES_PERIPH_MASK;
> - pmic_arb->apid_count =
> - readl_relaxed(core + PMIC_ARB_FEATURES1) &
> - PMIC_ARB_FEATURES_PERIPH_MASK;
> - }
>
> - if (pmic_arb->base_apid + pmic_arb->apid_count > pmic_arb->max_periphs) {
> - dev_err(&pdev->dev, "Unsupported APID count %d detected\n",
> - pmic_arb->base_apid + pmic_arb->apid_count);
> - return -EINVAL;
> - }
> - } else if (hw_ver >= PMIC_ARB_VERSION_V5_MIN) {
> - pmic_arb->base_apid = 0;
> - pmic_arb->apid_count = readl_relaxed(core + PMIC_ARB_FEATURES) &
> - PMIC_ARB_FEATURES_PERIPH_MASK;
> -
> - if (pmic_arb->apid_count > pmic_arb->max_periphs) {
> - dev_err(&pdev->dev, "Unsupported APID count %d detected\n",
> - pmic_arb->apid_count);
> - return -EINVAL;
> - }
> - }
> -
> - pmic_arb->apid_data = devm_kcalloc(&ctrl->dev, pmic_arb->max_periphs,
> - sizeof(*pmic_arb->apid_data),
> - GFP_KERNEL);
> - if (!pmic_arb->apid_data)
> - return -ENOMEM;
> -
> - dev_info(&ctrl->dev, "PMIC arbiter version %s (0x%x)\n",
> - pmic_arb->ver_ops->ver_str, hw_ver);
> + err = pmic_arb->ver_ops->init_apid(pmic_arb, 0);
> + if (err)
> + return err;
>
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "intr");
> pmic_arb->intr = devm_ioremap_resource(&ctrl->dev, res);
> @@ -1599,16 +1667,6 @@ static int spmi_pmic_arb_probe(struct platform_device *pdev)
> }
>
> pmic_arb->ee = ee;
> - mapping_table = devm_kcalloc(&ctrl->dev, pmic_arb->max_periphs,
> - sizeof(*mapping_table), GFP_KERNEL);
> - if (!mapping_table)
> - return -ENOMEM;
> -
> - pmic_arb->mapping_table = mapping_table;
> - /* Initialize max_apid/min_apid to the opposite bounds, during
> - * the irq domain translation, we are sure to update these */
> - pmic_arb->max_apid = 0;
> - pmic_arb->min_apid = pmic_arb->max_periphs - 1;
>
> platform_set_drvdata(pdev, ctrl);
> raw_spin_lock_init(&pmic_arb->lock);
> @@ -1617,15 +1675,6 @@ static int spmi_pmic_arb_probe(struct platform_device *pdev)
> ctrl->read_cmd = pmic_arb_read_cmd;
> ctrl->write_cmd = pmic_arb_write_cmd;
>
> - if (hw_ver >= PMIC_ARB_VERSION_V5_MIN) {
> - err = pmic_arb_read_apid_map_v5(pmic_arb);
> - if (err) {
> - dev_err(&pdev->dev, "could not read APID->PPID mapping table, rc= %d\n",
> - err);
> - return err;
> - }
> - }
> -
> dev_dbg(&pdev->dev, "adding irq domain\n");
> pmic_arb->domain = irq_domain_add_tree(pdev->dev.of_node,
> &pmic_arb_irq_domain_ops, pmic_arb);
>
With that clarified:
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
^ permalink raw reply
* Re: [PATCH v2 0/2] Add board-id support for multiple DT selection
From: Krzysztof Kozlowski @ 2024-03-28 8:50 UTC (permalink / raw)
To: Amrit Anand, robh, krzysztof.kozlowski+dt, conor+dt, agross,
andersson, konrad.dybcio
Cc: devicetree, linux-kernel, linux-arm-msm, kernel, peter.griffin,
caleb.connolly, linux-riscv, chrome-platform, linux-mediatek
In-Reply-To: <1710418312-6559-1-git-send-email-quic_amrianan@quicinc.com>
On 14/03/2024 13:11, Amrit Anand wrote:
> The software packages are shipped with multiple device tree blobs supporting
> multiple boards. For instance, suppose we have 3 SoC, each with 4 boards supported,
> along with 2 PMIC support for each case which would lead to total of 24 DTB files.
> Along with these configurations, OEMs may also add certain additional board variants.
> Hence, a mechanism is required to pick the correct DTB for the board on which the
> software package is deployed. Introduce a unique property for adding board identifiers
> to device trees. Here, board-id property provides a mechanism for Qualcomm based
> bootloaders to select the appropriate DTB.
>
> Isn't that what the compatible property is for?
> -----------------------------------------------
> The compatible property can be used for board matching, but requires
> bootloaders and/or firmware to maintain a database of possible strings
> to match against or have complex compatible string parsing and matching.
> Compatible string matching becomes complicated when there are multiple
> versions of the same board. It becomes difficult for the device tree
> selection mechanism to recognize the right DTB to pick, with minor
> differences in compatible strings.
>
> The solution proposed here is simpler to implement and doesn't require the need
> to update bootloader for every new board.
One of the concerns you got in v1 was: show us second user, so I believe
in your interest is to Cc other platform maintainers which could support
this idea.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2 2/3] dt-bindings: power: Add mediatek larb definition
From: Krzysztof Kozlowski @ 2024-03-28 8:44 UTC (permalink / raw)
To: Yu-chang Lee (李禹璋), amergnat@baylibre.com
Cc: linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org,
devicetree@vger.kernel.org, linux-pm@vger.kernel.org,
MandyJH Liu (劉人僖), conor+dt@kernel.org,
Project_Global_Chrome_Upstream_Group, robh@kernel.org,
Xiufeng Li (李秀峰),
linux-arm-kernel@lists.infradead.org,
krzysztof.kozlowski+dt@linaro.org, matthias.bgg@gmail.com,
ulf.hansson@linaro.org, Fan Chen (陳凡),
angelogioacchino.delregno@collabora.com
In-Reply-To: <3b04c5344435cdb941b5d132e8f5fbfdf9188d67.camel@mediatek.com>
On 28/03/2024 07:06, Yu-chang Lee (李禹璋) wrote:
> On Wed, 2024-03-27 at 12:55 +0100, Alexandre Mergnat wrote:
>>
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>> Hello Yu-chang Lee,
>>
>> SMI LARB must have a power domain, according to "mediatek,smi-
>> larb.yaml"
>> Now you try to create a link from power domain to larb.
>>
>> Here is my understanding: when you enable/disable power domain, the
>> larb linked to this power domain may have an issue. Then you want to
>> retrieve de LARB linked to the power domain though the dts to manage
>> the LARB.
>
> Yes, this is what I am trying to do.
>
>> IMHO, using the dts to have this information into the power
>> driver isn't necessary and may introduce some bugs if the LARB node
>> and power node in the DTS aren't aligned.
>>
>> It seems not implemented today but during the LARB probe, it should
>> "subscribe" to the linked power domain. Then, when the power domain
>> status is changing, it is able to "notify" (callback) the LARB, then
>> implement the good stuff to handle this power domain status change
>> into LARB driver.
>>
>
> The problem with this method and why "smi clamp" is in power domain
> driver is that our HW designer gave us a programming guide strictly
> states the sequence of what we need to do to power on/off power domain.
> Using callback, this sequence is no longer guaranteed and the side
> effect is unknown...
>
> So I would like to stick with adding larbs just like smi into powe
You want your power domain driver to poke, asynchronously, without
locking into registers of another device. And how does this not cause
issues?
No, work with your hardware engineers or Linux engineers on sane behavior.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: phy: Add QMP UFS PHY comptible for SM8475
From: Krzysztof Kozlowski @ 2024-03-28 8:42 UTC (permalink / raw)
To: Danila Tikhonov, andersson, konrad.dybcio, vkoul, kishon, robh,
krzysztof.kozlowski+dt, conor+dt
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel
In-Reply-To: <20240327180642.20146-2-danila@jiaxyga.com>
On 27/03/2024 19:06, Danila Tikhonov wrote:
> Document the QMP UFS PHY compatible for SM8475.
>
> Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>
> ---
> .../devicetree/bindings/phy/qcom,sc8280xp-qmp-ufs-phy.yaml | 2 ++
> 1 file changed, 2 insertions(+)
>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
(not reviewed, please provide link to DTS)
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 09/23] media: i2c: imx258: Add support for running on 2 CSI data lanes
From: Sakari Ailus @ 2024-03-28 8:19 UTC (permalink / raw)
To: git
Cc: linux-media, dave.stevenson, jacopo.mondi, mchehab, robh,
krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
festevam, devicetree, imx, linux-arm-kernel, linux-kernel
In-Reply-To: <20240327231710.53188-10-git@luigi311.com>
Hi Luigi311, Dave,
On Wed, Mar 27, 2024 at 05:16:55PM -0600, git@luigi311.com wrote:
> From: Dave Stevenson <dave.stevenson@raspberrypi.com>
>
> Extends the driver to also support 2 data lanes.
> Frame rates are obviously more restricted on 2 lanes, but some
> hardware simply hasn't wired more up.
>
> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> Signed-off-by: Luigi311 <git@luigi311.com>
> ---
> drivers/media/i2c/imx258.c | 214 ++++++++++++++++++++++++++++++++-----
> 1 file changed, 190 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
> index 6ee7de079454..c65b9aad3b0a 100644
> --- a/drivers/media/i2c/imx258.c
> +++ b/drivers/media/i2c/imx258.c
> @@ -86,12 +86,18 @@ struct imx258_reg_list {
> const struct imx258_reg *regs;
> };
>
> +enum {
> + IMX258_2_LANE_MODE,
> + IMX258_4_LANE_MODE,
> + IMX258_LANE_CONFIGS,
> +};
> +
> /* Link frequency config */
> struct imx258_link_freq_config {
> u32 pixels_per_line;
>
> /* PLL registers for this link frequency */
> - struct imx258_reg_list reg_list;
> + struct imx258_reg_list reg_list[IMX258_LANE_CONFIGS];
> };
>
> /* Mode : resolution and related config&values */
> @@ -111,8 +117,34 @@ struct imx258_mode {
> struct imx258_reg_list reg_list;
> };
>
> -/* 4208x3120 needs 1267Mbps/lane, 4 lanes */
> -static const struct imx258_reg mipi_1267mbps_19_2mhz[] = {
> +/*
> + * 4208x3120 @ 30 fps needs 1267Mbps/lane, 4 lanes.
> + * To avoid further computation of clock settings, adopt the same per
> + * lane data rate when using 2 lanes, thus allowing a maximum of 15fps.
> + */
> +static const struct imx258_reg mipi_1267mbps_19_2mhz_2l[] = {
> + { 0x0136, 0x13 },
> + { 0x0137, 0x33 },
> + { 0x0301, 0x0A },
> + { 0x0303, 0x02 },
> + { 0x0305, 0x03 },
> + { 0x0306, 0x00 },
> + { 0x0307, 0xC6 },
> + { 0x0309, 0x0A },
> + { 0x030B, 0x01 },
> + { 0x030D, 0x02 },
> + { 0x030E, 0x00 },
> + { 0x030F, 0xD8 },
> + { 0x0310, 0x00 },
> +
> + { 0x0114, 0x01 },
> + { 0x0820, 0x09 },
> + { 0x0821, 0xa6 },
> + { 0x0822, 0x66 },
> + { 0x0823, 0x66 },
> +};
> +
> +static const struct imx258_reg mipi_1267mbps_19_2mhz_4l[] = {
> { 0x0136, 0x13 },
> { 0x0137, 0x33 },
> { 0x0301, 0x05 },
> @@ -126,16 +158,18 @@ static const struct imx258_reg mipi_1267mbps_19_2mhz[] = {
> { 0x030E, 0x00 },
> { 0x030F, 0xD8 },
> { 0x0310, 0x00 },
> +
> + { 0x0114, 0x03 },
> { 0x0820, 0x13 },
> { 0x0821, 0x4C },
> { 0x0822, 0xCC },
> { 0x0823, 0xCC },
> };
>
> -static const struct imx258_reg mipi_1272mbps_24mhz[] = {
> +static const struct imx258_reg mipi_1272mbps_24mhz_2l[] = {
> { 0x0136, 0x18 },
> { 0x0137, 0x00 },
> - { 0x0301, 0x05 },
> + { 0x0301, 0x0a },
> { 0x0303, 0x02 },
> { 0x0305, 0x04 },
> { 0x0306, 0x00 },
> @@ -146,13 +180,59 @@ static const struct imx258_reg mipi_1272mbps_24mhz[] = {
> { 0x030E, 0x00 },
> { 0x030F, 0xD8 },
> { 0x0310, 0x00 },
> +
> + { 0x0114, 0x01 },
> { 0x0820, 0x13 },
> { 0x0821, 0x4C },
> { 0x0822, 0xCC },
> { 0x0823, 0xCC },
> };
>
> -static const struct imx258_reg mipi_640mbps_19_2mhz[] = {
> +static const struct imx258_reg mipi_1272mbps_24mhz_4l[] = {
> + { 0x0136, 0x18 },
> + { 0x0137, 0x00 },
> + { 0x0301, 0x05 },
> + { 0x0303, 0x02 },
> + { 0x0305, 0x04 },
> + { 0x0306, 0x00 },
> + { 0x0307, 0xD4 },
> + { 0x0309, 0x0A },
> + { 0x030B, 0x01 },
> + { 0x030D, 0x02 },
> + { 0x030E, 0x00 },
> + { 0x030F, 0xD8 },
> + { 0x0310, 0x00 },
> +
> + { 0x0114, 0x03 },
> + { 0x0820, 0x13 },
> + { 0x0821, 0xE0 },
> + { 0x0822, 0x00 },
> + { 0x0823, 0x00 },
> +};
> +
> +static const struct imx258_reg mipi_640mbps_19_2mhz_2l[] = {
> + { 0x0136, 0x13 },
> + { 0x0137, 0x33 },
> + { 0x0301, 0x05 },
> + { 0x0303, 0x02 },
> + { 0x0305, 0x03 },
> + { 0x0306, 0x00 },
> + { 0x0307, 0x64 },
> + { 0x0309, 0x0A },
> + { 0x030B, 0x01 },
> + { 0x030D, 0x02 },
> + { 0x030E, 0x00 },
> + { 0x030F, 0xD8 },
> + { 0x0310, 0x00 },
> +
> + { 0x0114, 0x01 },
> + { 0x0820, 0x05 },
> + { 0x0821, 0x00 },
> + { 0x0822, 0x00 },
> + { 0x0823, 0x00 },
> +};
> +
> +static const struct imx258_reg mipi_640mbps_19_2mhz_4l[] = {
> { 0x0136, 0x13 },
> { 0x0137, 0x33 },
> { 0x0301, 0x05 },
> @@ -166,13 +246,37 @@ static const struct imx258_reg mipi_640mbps_19_2mhz[] = {
> { 0x030E, 0x00 },
> { 0x030F, 0xD8 },
> { 0x0310, 0x00 },
> +
> + { 0x0114, 0x03 },
> + { 0x0820, 0x0A },
> + { 0x0821, 0x00 },
> + { 0x0822, 0x00 },
> + { 0x0823, 0x00 },
> +};
> +
> +static const struct imx258_reg mipi_642mbps_24mhz_2l[] = {
> + { 0x0136, 0x18 },
> + { 0x0137, 0x00 },
> + { 0x0301, 0x0A },
> + { 0x0303, 0x02 },
> + { 0x0305, 0x04 },
> + { 0x0306, 0x00 },
> + { 0x0307, 0x6B },
> + { 0x0309, 0x0A },
> + { 0x030B, 0x01 },
> + { 0x030D, 0x02 },
> + { 0x030E, 0x00 },
> + { 0x030F, 0xD8 },
> + { 0x0310, 0x00 },
> +
> + { 0x0114, 0x01 },
> { 0x0820, 0x0A },
> { 0x0821, 0x00 },
> { 0x0822, 0x00 },
> { 0x0823, 0x00 },
> };
>
> -static const struct imx258_reg mipi_642mbps_24mhz[] = {
> +static const struct imx258_reg mipi_642mbps_24mhz_4l[] = {
> { 0x0136, 0x18 },
> { 0x0137, 0x00 },
> { 0x0301, 0x05 },
> @@ -186,6 +290,8 @@ static const struct imx258_reg mipi_642mbps_24mhz[] = {
> { 0x030E, 0x00 },
> { 0x030F, 0xD8 },
> { 0x0310, 0x00 },
> +
> + { 0x0114, 0x03 },
> { 0x0820, 0x0A },
> { 0x0821, 0x00 },
> { 0x0822, 0x00 },
> @@ -240,7 +346,6 @@ static const struct imx258_reg mode_common_regs[] = {
> { 0x5F05, 0xED },
> { 0x0112, 0x0A },
> { 0x0113, 0x0A },
> - { 0x0114, 0x03 },
> { 0x0342, 0x14 },
> { 0x0343, 0xE8 },
> { 0x0344, 0x00 },
> @@ -359,11 +464,13 @@ enum {
>
> /*
> * pixel_rate = link_freq * data-rate * nr_of_lanes / bits_per_sample
> - * data rate => double data rate; number of lanes => 4; bits per pixel => 10
> + * data rate => double data rate;
> + * number of lanes => (configurable 2 or 4);
> + * bits per pixel => 10
> */
> -static u64 link_freq_to_pixel_rate(u64 f)
> +static u64 link_freq_to_pixel_rate(u64 f, unsigned int nlanes)
> {
> - f *= 2 * 4;
> + f *= 2 * nlanes;
> do_div(f, 10);
>
> return f;
> @@ -393,15 +500,27 @@ static const struct imx258_link_freq_config link_freq_configs_19_2[] = {
> [IMX258_LINK_FREQ_1267MBPS] = {
> .pixels_per_line = IMX258_PPL_DEFAULT,
> .reg_list = {
> - .num_of_regs = ARRAY_SIZE(mipi_1267mbps_19_2mhz),
> - .regs = mipi_1267mbps_19_2mhz,
> + [IMX258_2_LANE_MODE] = {
> + .num_of_regs = ARRAY_SIZE(mipi_1267mbps_19_2mhz_2l),
> + .regs = mipi_1267mbps_19_2mhz_2l,
> + },
> + [IMX258_4_LANE_MODE] = {
> + .num_of_regs = ARRAY_SIZE(mipi_1267mbps_19_2mhz_4l),
> + .regs = mipi_1267mbps_19_2mhz_4l,
> + },
> }
> },
> [IMX258_LINK_FREQ_640MBPS] = {
> .pixels_per_line = IMX258_PPL_DEFAULT,
> .reg_list = {
> - .num_of_regs = ARRAY_SIZE(mipi_640mbps_19_2mhz),
> - .regs = mipi_640mbps_19_2mhz,
> + [IMX258_2_LANE_MODE] = {
> + .num_of_regs = ARRAY_SIZE(mipi_640mbps_19_2mhz_2l),
> + .regs = mipi_640mbps_19_2mhz_2l,
> + },
> + [IMX258_4_LANE_MODE] = {
> + .num_of_regs = ARRAY_SIZE(mipi_640mbps_19_2mhz_4l),
> + .regs = mipi_640mbps_19_2mhz_4l,
> + },
> }
> },
> };
> @@ -410,15 +529,27 @@ static const struct imx258_link_freq_config link_freq_configs_24[] = {
> [IMX258_LINK_FREQ_1267MBPS] = {
> .pixels_per_line = IMX258_PPL_DEFAULT,
> .reg_list = {
> - .num_of_regs = ARRAY_SIZE(mipi_1272mbps_24mhz),
> - .regs = mipi_1272mbps_24mhz,
> + [IMX258_2_LANE_MODE] = {
> + .num_of_regs = ARRAY_SIZE(mipi_1272mbps_24mhz_2l),
> + .regs = mipi_1272mbps_24mhz_2l,
> + },
> + [IMX258_4_LANE_MODE] = {
> + .num_of_regs = ARRAY_SIZE(mipi_1272mbps_24mhz_4l),
> + .regs = mipi_1272mbps_24mhz_4l,
> + },
> }
> },
> [IMX258_LINK_FREQ_640MBPS] = {
> .pixels_per_line = IMX258_PPL_DEFAULT,
> .reg_list = {
> - .num_of_regs = ARRAY_SIZE(mipi_642mbps_24mhz),
> - .regs = mipi_642mbps_24mhz,
> + [IMX258_2_LANE_MODE] = {
> + .num_of_regs = ARRAY_SIZE(mipi_642mbps_24mhz_2l),
> + .regs = mipi_642mbps_24mhz_2l,
> + },
> + [IMX258_4_LANE_MODE] = {
> + .num_of_regs = ARRAY_SIZE(mipi_642mbps_24mhz_4l),
> + .regs = mipi_642mbps_24mhz_4l,
> + },
> }
> },
> };
> @@ -477,6 +608,7 @@ struct imx258 {
>
> const struct imx258_link_freq_config *link_freq_configs;
> const s64 *link_freq_menu_items;
> + unsigned int nlanes;
>
> /*
> * Mutex for serialized access:
> @@ -782,7 +914,7 @@ static int imx258_set_pad_format(struct v4l2_subdev *sd,
> __v4l2_ctrl_s_ctrl(imx258->link_freq, mode->link_freq_index);
>
> link_freq = imx258->link_freq_menu_items[mode->link_freq_index];
> - pixel_rate = link_freq_to_pixel_rate(link_freq);
> + pixel_rate = link_freq_to_pixel_rate(link_freq, imx258->nlanes);
> __v4l2_ctrl_s_ctrl_int64(imx258->pixel_rate, pixel_rate);
> /* Update limits and set FPS to default */
> vblank_def = imx258->cur_mode->vts_def -
> @@ -811,11 +943,13 @@ static int imx258_start_streaming(struct imx258 *imx258)
> {
> struct i2c_client *client = v4l2_get_subdevdata(&imx258->sd);
> const struct imx258_reg_list *reg_list;
> + const struct imx258_link_freq_config *link_freq_cfg;
> int ret, link_freq_index;
>
> /* Setup PLL */
> link_freq_index = imx258->cur_mode->link_freq_index;
> - reg_list = &imx258->link_freq_configs[link_freq_index].reg_list;
> + link_freq_cfg = &imx258->link_freq_configs[link_freq_index];
> + reg_list = &link_freq_cfg->reg_list[imx258->nlanes == 2 ? 0 : 1];
> ret = imx258_write_regs(imx258, reg_list->regs, reg_list->num_of_regs);
> if (ret) {
> dev_err(&client->dev, "%s failed to set plls\n", __func__);
> @@ -1033,9 +1167,11 @@ static int imx258_init_controls(struct imx258 *imx258)
> vflip->flags |= V4L2_CTRL_FLAG_READ_ONLY;
>
> pixel_rate_max =
> - link_freq_to_pixel_rate(imx258->link_freq_menu_items[0]);
> + link_freq_to_pixel_rate(imx258->link_freq_menu_items[0],
> + imx258->nlanes);
> pixel_rate_min =
> - link_freq_to_pixel_rate(imx258->link_freq_menu_items[1]);
> + link_freq_to_pixel_rate(imx258->link_freq_menu_items[1],
> + imx258->nlanes);
> /* By default, PIXEL_RATE is read only */
> imx258->pixel_rate = v4l2_ctrl_new_std(ctrl_hdlr, &imx258_ctrl_ops,
> V4L2_CID_PIXEL_RATE,
> @@ -1132,6 +1268,10 @@ static int imx258_get_regulators(struct imx258 *imx258,
> static int imx258_probe(struct i2c_client *client)
> {
> struct imx258 *imx258;
> + struct fwnode_handle *endpoint;
> + struct v4l2_fwnode_endpoint ep = {
> + .bus_type = V4L2_MBUS_CSI2_DPHY
> + };
> int ret;
> u32 val = 0;
>
> @@ -1172,13 +1312,35 @@ static int imx258_probe(struct i2c_client *client)
> return -EINVAL;
> }
>
> + endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(&client->dev), NULL);
> + if (!endpoint) {
> + dev_err(&client->dev, "Endpoint node not found\n");
> + return -EINVAL;
> + }
> +
> + ret = v4l2_fwnode_endpoint_alloc_parse(endpoint, &ep);
Here you're obtaining the list of supported link frequencies from the
firmware but it is not validated (nor it was validated by the driver
previously). I'd regard that a driver bug but fixing it at this point could
introduce adverse effects elsewhere.
I think what I'd do here is that I'd ignore the issue if there are no
frequencies defined for the endpoint but if there are, then enable only
those that are listed in the endpoint.
Could you add a patch to do this, please? v4l2_link_freq_to_bitmap() has
been recently added to facilitate this.
> + fwnode_handle_put(endpoint);
> + if (ret) {
> + dev_err(&client->dev, "Parsing endpoint node failed\n");
> + return ret;
> + }
> +
> + /* Get number of data lanes */
> + imx258->nlanes = ep.bus.mipi_csi2.num_data_lanes;
> + if (imx258->nlanes != 2 && imx258->nlanes != 4) {
> + dev_err(&client->dev, "Invalid data lanes: %u\n",
> + imx258->nlanes);
> + ret = -EINVAL;
> + goto error_endpoint_free;
> + }
> +
> /* Initialize subdev */
> v4l2_i2c_subdev_init(&imx258->sd, client, &imx258_subdev_ops);
>
> /* Will be powered off via pm_runtime_idle */
> ret = imx258_power_on(&client->dev);
> if (ret)
> - return ret;
> + goto error_endpoint_free;
>
> /* Check module identity */
> ret = imx258_identify_module(imx258);
> @@ -1211,6 +1373,7 @@ static int imx258_probe(struct i2c_client *client)
> pm_runtime_set_active(&client->dev);
> pm_runtime_enable(&client->dev);
> pm_runtime_idle(&client->dev);
> + v4l2_fwnode_endpoint_free(&ep);
>
> return 0;
>
> @@ -1223,6 +1386,9 @@ static int imx258_probe(struct i2c_client *client)
> error_identify:
> imx258_power_off(&client->dev);
>
> +error_endpoint_free:
> + v4l2_fwnode_endpoint_free(&ep);
> +
> return ret;
> }
>
--
Kind regards,
Sakari Ailus
^ permalink raw reply
* Re: [RFC PATCH 08/13] pinctrl: renesas: pinctrl-rzg2l: Add function pointers for writing to PFC
From: claudiu beznea @ 2024-03-28 8:13 UTC (permalink / raw)
To: Prabhakar, Geert Uytterhoeven, Linus Walleij, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Magnus Damm
Cc: linux-renesas-soc, linux-gpio, devicetree, linux-kernel,
Fabrizio Castro, Lad Prabhakar
In-Reply-To: <4bd3b33d-564a-45e0-905c-d0deb52e6f38@tuxon.dev>
On 28.03.2024 10:02, claudiu beznea wrote:
> Hi, Prabhakar,
>
> On 27.03.2024 00:28, Prabhakar wrote:
>> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>>
>> On the RZ/G2L SoC, the PFCWE bit controls writing to PFC registers.
>> However, on the RZ/V2H(P) SoC, the PFCWE (REGWE_A on RZ/V2H) bit controls
>> writing to both PFC and PMC registers. To accommodate these differences
>> across SoC variants, introduce set_pfc_mode() and pm_set_pfc() function
>> pointers.
>
> I think the overall code can be simplified if you add 1 function that does
> the lock/unlock for PWPR. See patch 13.
I meant to say one function for lock and one for unlock.
>
>>
>> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>> ---
>> drivers/pinctrl/renesas/pinctrl-rzg2l.c | 14 ++++++++++++--
>> 1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
>> index 705372faaeff..4cdebdbd8a04 100644
>> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
>> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
>> @@ -246,6 +246,8 @@ struct rzg2l_variable_pin_cfg {
>> u32 pin:3;
>> };
>>
>> +struct rzg2l_pinctrl;
>> +
>> struct rzg2l_pinctrl_data {
>> const char * const *port_pins;
>> const u64 *port_pin_configs;
>> @@ -256,6 +258,8 @@ struct rzg2l_pinctrl_data {
>> const struct rzg2l_hwcfg *hwcfg;
>> const struct rzg2l_variable_pin_cfg *variable_pin_cfg;
>> unsigned int n_variable_pin_cfg;
>> + void (*set_pfc_mode)(struct rzg2l_pinctrl *pctrl, u8 pin, u8 off, u8 func);
>> + void (*pm_set_pfc)(struct rzg2l_pinctrl *pctrl);
>> };
>>
>> /**
>> @@ -526,7 +530,7 @@ static int rzg2l_pinctrl_set_mux(struct pinctrl_dev *pctldev,
>> dev_dbg(pctrl->dev, "port:%u pin: %u off:%x PSEL:%u\n",
>> RZG2L_PIN_ID_TO_PORT(pins[i]), pin, off, psel_val[i] - hwcfg->func_base);
>>
>> - rzg2l_pinctrl_set_pfc_mode(pctrl, pin, off, psel_val[i] - hwcfg->func_base);
>> + pctrl->data->set_pfc_mode(pctrl, pin, off, psel_val[i] - hwcfg->func_base);
>> }
>>
>> return 0;
>> @@ -2607,7 +2611,7 @@ static int rzg2l_pinctrl_resume_noirq(struct device *dev)
>> writeb(cache->eth_poc[i], pctrl->base + ETH_POC(regs->eth_poc, i));
>> }
>>
>> - rzg2l_pinctrl_pm_setup_pfc(pctrl);
>> + pctrl->data->pm_set_pfc(pctrl);
>> rzg2l_pinctrl_pm_setup_regs(pctrl, false);
>> rzg2l_pinctrl_pm_setup_dedicated_regs(pctrl, false);
>> rzg2l_gpio_irq_restore(pctrl);
>> @@ -2672,6 +2676,8 @@ static struct rzg2l_pinctrl_data r9a07g043_data = {
>> .variable_pin_cfg = r9a07g043f_variable_pin_cfg,
>> .n_variable_pin_cfg = ARRAY_SIZE(r9a07g043f_variable_pin_cfg),
>> #endif
>> + .set_pfc_mode = &rzg2l_pinctrl_set_pfc_mode,
>> + .pm_set_pfc = &rzg2l_pinctrl_pm_setup_pfc,
>> };
>>
>> static struct rzg2l_pinctrl_data r9a07g044_data = {
>> @@ -2683,6 +2689,8 @@ static struct rzg2l_pinctrl_data r9a07g044_data = {
>> .n_dedicated_pins = ARRAY_SIZE(rzg2l_dedicated_pins.common) +
>> ARRAY_SIZE(rzg2l_dedicated_pins.rzg2l_pins),
>> .hwcfg = &rzg2l_hwcfg,
>> + .set_pfc_mode = &rzg2l_pinctrl_set_pfc_mode,
>> + .pm_set_pfc = &rzg2l_pinctrl_pm_setup_pfc,
>> };
>>
>> static struct rzg2l_pinctrl_data r9a08g045_data = {
>> @@ -2693,6 +2701,8 @@ static struct rzg2l_pinctrl_data r9a08g045_data = {
>> .n_port_pins = ARRAY_SIZE(r9a08g045_gpio_configs) * RZG2L_PINS_PER_PORT,
>> .n_dedicated_pins = ARRAY_SIZE(rzg3s_dedicated_pins),
>> .hwcfg = &rzg3s_hwcfg,
>> + .set_pfc_mode = &rzg2l_pinctrl_set_pfc_mode,
>> + .pm_set_pfc = &rzg2l_pinctrl_pm_setup_pfc,
>> };
>>
>> static const struct of_device_id rzg2l_pinctrl_of_table[] = {
^ permalink raw reply
* Re: [PATCH 08/23] media: i2c: imx258: Add support for 24MHz clock
From: Sakari Ailus @ 2024-03-28 8:09 UTC (permalink / raw)
To: git
Cc: linux-media, dave.stevenson, jacopo.mondi, mchehab, robh,
krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
festevam, devicetree, imx, linux-arm-kernel, linux-kernel
In-Reply-To: <20240327231710.53188-9-git@luigi311.com>
Hi Luigi311,
Thank you for the patchset.
On Wed, Mar 27, 2024 at 05:16:54PM -0600, git@luigi311.com wrote:
> From: Dave Stevenson <dave.stevenson@raspberrypi.com>
>
> There's no reason why only a clock of 19.2MHz is supported.
> Indeed this isn't even a frequency listed in the datasheet.
>
> Add support for 24MHz as well.
> The PLL settings result in slightly different link frequencies,
> so parameterise those.
>
> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> Signed-off-by: Luigi311 <git@luigi311.com>
Is Luigi311 your real name? As per
Documentation/process/submitting-patches.rst, anonymous (or pseudonym I'd
say as well) contributions are not an option.
> ---
> drivers/media/i2c/imx258.c | 133 +++++++++++++++++++++++++++++--------
> 1 file changed, 107 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
> index 351add1bc5d5..6ee7de079454 100644
> --- a/drivers/media/i2c/imx258.c
> +++ b/drivers/media/i2c/imx258.c
> @@ -76,9 +76,6 @@
> #define REG_CONFIG_MIRROR_FLIP 0x03
> #define REG_CONFIG_FLIP_TEST_PATTERN 0x02
>
> -/* Input clock frequency in Hz */
> -#define IMX258_INPUT_CLOCK_FREQ 19200000
> -
> struct imx258_reg {
> u16 address;
> u8 val;
> @@ -115,7 +112,9 @@ struct imx258_mode {
> };
>
> /* 4208x3120 needs 1267Mbps/lane, 4 lanes */
> -static const struct imx258_reg mipi_data_rate_1267mbps[] = {
> +static const struct imx258_reg mipi_1267mbps_19_2mhz[] = {
> + { 0x0136, 0x13 },
> + { 0x0137, 0x33 },
> { 0x0301, 0x05 },
> { 0x0303, 0x02 },
> { 0x0305, 0x03 },
> @@ -133,7 +132,29 @@ static const struct imx258_reg mipi_data_rate_1267mbps[] = {
> { 0x0823, 0xCC },
> };
>
> -static const struct imx258_reg mipi_data_rate_640mbps[] = {
> +static const struct imx258_reg mipi_1272mbps_24mhz[] = {
> + { 0x0136, 0x18 },
> + { 0x0137, 0x00 },
> + { 0x0301, 0x05 },
> + { 0x0303, 0x02 },
> + { 0x0305, 0x04 },
> + { 0x0306, 0x00 },
> + { 0x0307, 0xD4 },
> + { 0x0309, 0x0A },
> + { 0x030B, 0x01 },
> + { 0x030D, 0x02 },
> + { 0x030E, 0x00 },
> + { 0x030F, 0xD8 },
> + { 0x0310, 0x00 },
> + { 0x0820, 0x13 },
> + { 0x0821, 0x4C },
> + { 0x0822, 0xCC },
> + { 0x0823, 0xCC },
> +};
> +
> +static const struct imx258_reg mipi_640mbps_19_2mhz[] = {
> + { 0x0136, 0x13 },
> + { 0x0137, 0x33 },
> { 0x0301, 0x05 },
> { 0x0303, 0x02 },
> { 0x0305, 0x03 },
> @@ -151,9 +172,27 @@ static const struct imx258_reg mipi_data_rate_640mbps[] = {
> { 0x0823, 0x00 },
> };
>
> +static const struct imx258_reg mipi_642mbps_24mhz[] = {
> + { 0x0136, 0x18 },
> + { 0x0137, 0x00 },
> + { 0x0301, 0x05 },
> + { 0x0303, 0x02 },
> + { 0x0305, 0x04 },
> + { 0x0306, 0x00 },
> + { 0x0307, 0x6B },
> + { 0x0309, 0x0A },
> + { 0x030B, 0x01 },
> + { 0x030D, 0x02 },
> + { 0x030E, 0x00 },
> + { 0x030F, 0xD8 },
> + { 0x0310, 0x00 },
> + { 0x0820, 0x0A },
> + { 0x0821, 0x00 },
> + { 0x0822, 0x00 },
> + { 0x0823, 0x00 },
> +};
> +
> static const struct imx258_reg mode_common_regs[] = {
> - { 0x0136, 0x13 },
> - { 0x0137, 0x33 },
> { 0x3051, 0x00 },
> { 0x3052, 0x00 },
> { 0x4E21, 0x14 },
> @@ -313,10 +352,6 @@ static const char * const imx258_supply_name[] = {
>
> #define IMX258_NUM_SUPPLIES ARRAY_SIZE(imx258_supply_name)
>
> -/* Configurations for supported link frequencies */
> -#define IMX258_LINK_FREQ_634MHZ 633600000ULL
> -#define IMX258_LINK_FREQ_320MHZ 320000000ULL
> -
> enum {
> IMX258_LINK_FREQ_1267MBPS,
> IMX258_LINK_FREQ_640MBPS,
> @@ -335,25 +370,55 @@ static u64 link_freq_to_pixel_rate(u64 f)
> }
>
> /* Menu items for LINK_FREQ V4L2 control */
> -static const s64 link_freq_menu_items[] = {
> +/* Configurations for supported link frequencies */
> +#define IMX258_LINK_FREQ_634MHZ 633600000ULL
> +#define IMX258_LINK_FREQ_320MHZ 320000000ULL
> +
> +static const s64 link_freq_menu_items_19_2[] = {
> IMX258_LINK_FREQ_634MHZ,
> IMX258_LINK_FREQ_320MHZ,
> };
>
> +/* Configurations for supported link frequencies */
> +#define IMX258_LINK_FREQ_636MHZ 636000000ULL
> +#define IMX258_LINK_FREQ_321MHZ 321000000ULL
These values aren't used outside the array below and the macro names are
imprecise anyway. Could you put the numerical values to the array instead?
> +
> +static const s64 link_freq_menu_items_24[] = {
> + IMX258_LINK_FREQ_636MHZ,
> + IMX258_LINK_FREQ_321MHZ,
> +};
> +
> /* Link frequency configs */
> -static const struct imx258_link_freq_config link_freq_configs[] = {
> +static const struct imx258_link_freq_config link_freq_configs_19_2[] = {
> [IMX258_LINK_FREQ_1267MBPS] = {
> .pixels_per_line = IMX258_PPL_DEFAULT,
> .reg_list = {
> - .num_of_regs = ARRAY_SIZE(mipi_data_rate_1267mbps),
> - .regs = mipi_data_rate_1267mbps,
> + .num_of_regs = ARRAY_SIZE(mipi_1267mbps_19_2mhz),
> + .regs = mipi_1267mbps_19_2mhz,
> }
> },
> [IMX258_LINK_FREQ_640MBPS] = {
> .pixels_per_line = IMX258_PPL_DEFAULT,
> .reg_list = {
> - .num_of_regs = ARRAY_SIZE(mipi_data_rate_640mbps),
> - .regs = mipi_data_rate_640mbps,
> + .num_of_regs = ARRAY_SIZE(mipi_640mbps_19_2mhz),
> + .regs = mipi_640mbps_19_2mhz,
> + }
> + },
> +};
> +
> +static const struct imx258_link_freq_config link_freq_configs_24[] = {
> + [IMX258_LINK_FREQ_1267MBPS] = {
> + .pixels_per_line = IMX258_PPL_DEFAULT,
> + .reg_list = {
> + .num_of_regs = ARRAY_SIZE(mipi_1272mbps_24mhz),
> + .regs = mipi_1272mbps_24mhz,
> + }
> + },
> + [IMX258_LINK_FREQ_640MBPS] = {
> + .pixels_per_line = IMX258_PPL_DEFAULT,
> + .reg_list = {
> + .num_of_regs = ARRAY_SIZE(mipi_642mbps_24mhz),
> + .regs = mipi_642mbps_24mhz,
> }
> },
> };
> @@ -410,6 +475,9 @@ struct imx258 {
> /* Current mode */
> const struct imx258_mode *cur_mode;
>
> + const struct imx258_link_freq_config *link_freq_configs;
> + const s64 *link_freq_menu_items;
> +
> /*
> * Mutex for serialized access:
> * Protect sensor module set pad format and start/stop streaming safely.
> @@ -713,7 +781,7 @@ static int imx258_set_pad_format(struct v4l2_subdev *sd,
> imx258->cur_mode = mode;
> __v4l2_ctrl_s_ctrl(imx258->link_freq, mode->link_freq_index);
>
> - link_freq = link_freq_menu_items[mode->link_freq_index];
> + link_freq = imx258->link_freq_menu_items[mode->link_freq_index];
> pixel_rate = link_freq_to_pixel_rate(link_freq);
> __v4l2_ctrl_s_ctrl_int64(imx258->pixel_rate, pixel_rate);
> /* Update limits and set FPS to default */
> @@ -727,7 +795,7 @@ static int imx258_set_pad_format(struct v4l2_subdev *sd,
> vblank_def);
> __v4l2_ctrl_s_ctrl(imx258->vblank, vblank_def);
> h_blank =
> - link_freq_configs[mode->link_freq_index].pixels_per_line
> + imx258->link_freq_configs[mode->link_freq_index].pixels_per_line
> - imx258->cur_mode->width;
> __v4l2_ctrl_modify_range(imx258->hblank, h_blank,
> h_blank, 1, h_blank);
> @@ -747,7 +815,7 @@ static int imx258_start_streaming(struct imx258 *imx258)
>
> /* Setup PLL */
> link_freq_index = imx258->cur_mode->link_freq_index;
> - reg_list = &link_freq_configs[link_freq_index].reg_list;
> + reg_list = &imx258->link_freq_configs[link_freq_index].reg_list;
> ret = imx258_write_regs(imx258, reg_list->regs, reg_list->num_of_regs);
> if (ret) {
> dev_err(&client->dev, "%s failed to set plls\n", __func__);
> @@ -946,9 +1014,9 @@ static int imx258_init_controls(struct imx258 *imx258)
> imx258->link_freq = v4l2_ctrl_new_int_menu(ctrl_hdlr,
> &imx258_ctrl_ops,
> V4L2_CID_LINK_FREQ,
> - ARRAY_SIZE(link_freq_menu_items) - 1,
> + ARRAY_SIZE(link_freq_menu_items_19_2) - 1,
> 0,
> - link_freq_menu_items);
> + imx258->link_freq_menu_items);
>
> if (imx258->link_freq)
> imx258->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> @@ -964,8 +1032,10 @@ static int imx258_init_controls(struct imx258 *imx258)
> if (vflip)
> vflip->flags |= V4L2_CTRL_FLAG_READ_ONLY;
>
> - pixel_rate_max = link_freq_to_pixel_rate(link_freq_menu_items[0]);
> - pixel_rate_min = link_freq_to_pixel_rate(link_freq_menu_items[1]);
> + pixel_rate_max =
> + link_freq_to_pixel_rate(imx258->link_freq_menu_items[0]);
> + pixel_rate_min =
> + link_freq_to_pixel_rate(imx258->link_freq_menu_items[1]);
The arrays currently have two entries so this works but it'd nice to have a
bit more robust way to handle differences between the two arrays. Could you
maintain e.g. the number of entries in the array in a struct field perhaps?
> /* By default, PIXEL_RATE is read only */
> imx258->pixel_rate = v4l2_ctrl_new_std(ctrl_hdlr, &imx258_ctrl_ops,
> V4L2_CID_PIXEL_RATE,
> @@ -1086,8 +1156,19 @@ static int imx258_probe(struct i2c_client *client)
> } else {
> val = clk_get_rate(imx258->clk);
> }
> - if (val != IMX258_INPUT_CLOCK_FREQ) {
> - dev_err(&client->dev, "input clock frequency not supported\n");
> +
> + switch (val) {
> + case 19200000:
> + imx258->link_freq_configs = link_freq_configs_19_2;
> + imx258->link_freq_menu_items = link_freq_menu_items_19_2;
> + break;
> + case 24000000:
> + imx258->link_freq_configs = link_freq_configs_24;
> + imx258->link_freq_menu_items = link_freq_menu_items_24;
> + break;
> + default:
> + dev_err(&client->dev, "input clock frequency of %u not supported\n",
> + val);
> return -EINVAL;
> }
>
--
Kind regards,
Sakari Ailus
^ permalink raw reply
* Re: [RFC PATCH 13/13] pinctrl: renesas: pinctrl-rzg2l: Add support for RZ/V2H SoC
From: claudiu beznea @ 2024-03-28 8:04 UTC (permalink / raw)
To: Prabhakar, Geert Uytterhoeven, Linus Walleij, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Magnus Damm
Cc: linux-renesas-soc, linux-gpio, devicetree, linux-kernel,
Fabrizio Castro, Lad Prabhakar
In-Reply-To: <20240326222844.1422948-14-prabhakar.mahadev-lad.rj@bp.renesas.com>
Hi, Prabhakar,
On 27.03.2024 00:28, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Add pinctrl driver support for RZ/V2H(P) SoC.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> drivers/pinctrl/renesas/pinctrl-rzg2l.c | 483 +++++++++++++++++++++++-
> 1 file changed, 481 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> index 6f0c85bb97a8..716c11ca5a8f 100644
> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> @@ -59,6 +59,13 @@
> #define PIN_CFG_OEN BIT(15)
> #define PIN_CFG_VARIABLE BIT(16)
> #define PIN_CFG_NOGPIO_INT BIT(17)
> +#define PIN_CFG_OPEN_DRAIN BIT(18)
> +#define PIN_CFG_SCHMIT_CTRL BIT(19)
> +#define PIN_CFG_ELC BIT(20)
> +#define PIN_CFG_IOLH_1 BIT(21)
> +#define PIN_CFG_IOLH_2 BIT(22)
> +#define PIN_CFG_IOLH_3 BIT(23)
> +#define PIN_CFG_IOLH_4 BIT(24)
>
> #define RZG2L_MPXED_COMMON_PIN_FUNCS(group) \
> (PIN_CFG_IOLH_##group | \
> @@ -70,6 +77,10 @@
> #define RZG2L_MPXED_PIN_FUNCS (RZG2L_MPXED_COMMON_PIN_FUNCS(A) | \
> PIN_CFG_SR)
>
> +#define RZV2H_MPXED_PIN_FUNCS(group) (RZG2L_MPXED_COMMON_PIN_FUNCS(group) | \
> + PIN_CFG_OPEN_DRAIN | \
> + PIN_CFG_SR)
> +
> #define RZG3S_MPXED_PIN_FUNCS(group) (RZG2L_MPXED_COMMON_PIN_FUNCS(group) | \
> PIN_CFG_SOFT_PS)
>
> @@ -133,6 +144,8 @@
>
> #define PWPR_B0WI BIT(7) /* Bit Write Disable */
> #define PWPR_PFCWE BIT(6) /* PFC Register Write Enable */
> +#define PWPR_REGWE_A BIT(6) /* PFC and PMC Register Write Enable */
> +#define PWPR_REGWE_B BIT(5) /* OEN Register Write Enable */
>
> #define PM_MASK 0x03
> #define PFC_MASK 0x07
> @@ -149,6 +162,19 @@
> #define RZG2L_TINT_IRQ_START_INDEX 9
> #define RZG2L_PACK_HWIRQ(t, i) (((t) << 16) | (i))
>
> +/* Custom pinconf parameters */
> +#define RENESAS_RZV2H_PIN_CONFIG_OUTPUT_IMPEDANCE (PIN_CONFIG_END + 1)
> +
> +static const struct pinconf_generic_params renesas_rzv2h_custom_bindings[] = {
> + { "renesas-rzv2h,output-impedance", RENESAS_RZV2H_PIN_CONFIG_OUTPUT_IMPEDANCE, 1 },
> +};
> +
> +#ifdef CONFIG_DEBUG_FS
> +static const struct pin_config_item renesas_rzv2h_conf_items[] = {
> + PCONFDUMP(RENESAS_RZV2H_PIN_CONFIG_OUTPUT_IMPEDANCE, "output-impedance", "x", true),
> +};
> +#endif
> +
> /* Read/write 8 bits register */
> #define RZG2L_PCTRL_REG_ACCESS8(_read, _addr, _val) \
> do { \
> @@ -324,6 +350,8 @@ struct rzg2l_pinctrl {
> spinlock_t lock; /* lock read/write registers */
> struct mutex mutex; /* serialize adding groups and functions */
>
> + raw_spinlock_t pwpr_lock; /* serialize PWPR register access */
> +
> struct rzg2l_pinctrl_pin_settings *settings;
> struct rzg2l_pinctrl_reg_cache *cache;
> struct rzg2l_pinctrl_reg_cache *dedicated_cache;
> @@ -348,6 +376,79 @@ static u64 rzg2l_pinctrl_get_variable_pin_cfg(struct rzg2l_pinctrl *pctrl,
> return 0;
> }
>
> +static const struct rzg2l_variable_pin_cfg r9a09g057_variable_pin_cfg[] = {
> + {
> + .port = 9,
> + .pin = 0,
> + .cfg = RZV2H_MPXED_PIN_FUNCS(2) | PIN_CFG_SCHMIT_CTRL,
> + },
> + {
> + .port = 9,
> + .pin = 1,
> + .cfg = RZV2H_MPXED_PIN_FUNCS(2) | PIN_CFG_SCHMIT_CTRL,
> + },
> + {
> + .port = 9,
> + .pin = 2,
> + .cfg = RZV2H_MPXED_PIN_FUNCS(2) | PIN_CFG_SCHMIT_CTRL,
> + },
> + {
> + .port = 9,
> + .pin = 3,
> + .cfg = RZV2H_MPXED_PIN_FUNCS(3) | PIN_CFG_SCHMIT_CTRL,
> + },
> + {
> + .port = 9,
> + .pin = 4,
> + .cfg = RZV2H_MPXED_PIN_FUNCS(3) | PIN_CFG_SCHMIT_CTRL,
> + },
> + {
> + .port = 9,
> + .pin = 5,
> + .cfg = RZV2H_MPXED_PIN_FUNCS(3) | PIN_CFG_SCHMIT_CTRL,
> + },
> + {
> + .port = 9,
> + .pin = 6,
> + .cfg = RZV2H_MPXED_PIN_FUNCS(3) | PIN_CFG_SCHMIT_CTRL,
> + },
> + {
> + .port = 9,
> + .pin = 7,
> + .cfg = RZV2H_MPXED_PIN_FUNCS(3) | PIN_CFG_SCHMIT_CTRL,
> + },
> + {
> + .port = 11,
> + .pin = 0,
> + .cfg = RZV2H_MPXED_PIN_FUNCS(2) | PIN_CFG_SCHMIT_CTRL,
> + },
> + {
> + .port = 11,
> + .pin = 1,
> + .cfg = RZV2H_MPXED_PIN_FUNCS(2) | PIN_CFG_SCHMIT_CTRL | PIN_CFG_IEN,
> + },
> + {
> + .port = 11,
> + .pin = 2,
> + .cfg = RZV2H_MPXED_PIN_FUNCS(2) | PIN_CFG_SCHMIT_CTRL | PIN_CFG_IEN,
> + },
> + {
> + .port = 11,
> + .pin = 3,
> + .cfg = RZV2H_MPXED_PIN_FUNCS(2) | PIN_CFG_SCHMIT_CTRL | PIN_CFG_IEN,
> + },
> + {
> + .port = 11,
> + .pin = 4,
> + .cfg = RZV2H_MPXED_PIN_FUNCS(2) | PIN_CFG_SCHMIT_CTRL | PIN_CFG_IEN,
> + },
> + {
> + .port = 11,
> + .pin = 5,
> + .cfg = RZV2H_MPXED_PIN_FUNCS(2) | PIN_CFG_SCHMIT_CTRL | PIN_CFG_IEN,
> + },
> +};
> +
> #ifdef CONFIG_RISCV
> static const struct rzg2l_variable_pin_cfg r9a07g043f_variable_pin_cfg[] = {
> {
> @@ -474,6 +575,19 @@ static void rzg2l_pmc_writeb(struct rzg2l_pinctrl *pctrl, u8 val, void __iomem *
> writeb(val, addr);
> }
>
> +static void rzv2h_pmc_writeb(struct rzg2l_pinctrl *pctrl, u8 val, void __iomem *addr)
> +{
> + const struct rzg2l_register_offsets *regs = &pctrl->data->hwcfg->regs;
> + u8 pwpr;
> +
> + raw_spin_lock(&pctrl->pwpr_lock);
> + pwpr = readb(pctrl->base + regs->pwpr);
> + writeb(pwpr | PWPR_REGWE_A, pctrl->base + regs->pwpr);
What about having a device specific function that locks/unlocks the PWPR,
this part ^ being the lock.
> + writeb(val, addr);
And this starting here:
> + writeb(pwpr & ~PWPR_REGWE_A, pctrl->base + regs->pwpr);
> + raw_spin_unlock(&pctrl->pwpr_lock);
ending here: the unlock function. It should generate les diffs at least in
this patch.
And you can add, were needed:
if (pctrl->pwpr_lock_function)
pctrl->pwpr_lock_function();
write(val, addr);
if (pctrl->pwpr_unlock_function)
pctrl->pwpr_unlock_function();
With this you can avoid adding rzv2h_pinctrl_set_pfc_mode() which is alomst
identical w/ rzg2l_pinctrl_set_pfc_mode(), or adding
rzv2h_pinctrl_pm_setup_pfc() almost identical with
rzg2l_pinctrl_pm_setup_pfc().
> +}
> +
> static void rzg2l_pinctrl_set_pfc_mode(struct rzg2l_pinctrl *pctrl,
> u8 pin, u8 off, u8 func)
> {
> @@ -512,6 +626,47 @@ static void rzg2l_pinctrl_set_pfc_mode(struct rzg2l_pinctrl *pctrl,
> spin_unlock_irqrestore(&pctrl->lock, flags);
> };
>
> +static void rzv2h_pinctrl_set_pfc_mode(struct rzg2l_pinctrl *pctrl,
> + u8 pin, u8 off, u8 func)
> +{
> + const struct rzg2l_register_offsets *regs = &pctrl->data->hwcfg->regs;
> + unsigned long flags;
> + u32 reg;
> + u8 pwpr;
> +
> + spin_lock_irqsave(&pctrl->lock, flags);
> +
> + /* Set pin to 'Non-use (Hi-Z input protection)' */
> + reg = readw(pctrl->base + PM(off));
> + reg &= ~(PM_MASK << (pin * 2));
> + writew(reg, pctrl->base + PM(off));
> +
> + /* Set the PWPR register to allow PFC and PMC register to write */
> + raw_spin_lock(&pctrl->pwpr_lock);
> + pwpr = readb(pctrl->base + regs->pwpr);
> + writeb(PWPR_PFCWE | pwpr, pctrl->base + regs->pwpr);
> +
> + /* Temporarily switch to GPIO mode with PMC register */
> + reg = readb(pctrl->base + PMC(off));
> + writeb(reg & ~BIT(pin), pctrl->base + PMC(off));
> +
> + /* Select Pin function mode with PFC register */
> + reg = readl(pctrl->base + PFC(off));
> + reg &= ~(PFC_MASK << (pin * 4));
> + writel(reg | (func << (pin * 4)), pctrl->base + PFC(off));
> +
> + /* Switch to Peripheral pin function with PMC register */
> + reg = readb(pctrl->base + PMC(off));
> + writeb(reg | BIT(pin), pctrl->base + PMC(off));
> +
> + /* Set the PWPR register to be write-protected */
> + pwpr = readb(pctrl->base + regs->pwpr);
> + writeb(pwpr & ~PWPR_PFCWE, pctrl->base + regs->pwpr);
> + raw_spin_unlock(&pctrl->pwpr_lock);
> +
> + spin_unlock_irqrestore(&pctrl->lock, flags);
> +};
> +
> static int rzg2l_pinctrl_set_mux(struct pinctrl_dev *pctldev,
> unsigned int func_selector,
> unsigned int group_selector)
> @@ -1087,14 +1242,26 @@ static int rzg2l_write_oen(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8
> return 0;
> }
>
> +static u32 rzv2h_read_oen(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin)
> +{
> + /* stub */
> + return 0;
> +}
> +
> +static int rzv2h_write_oen(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin, u8 oen)
> +{
> + /* stub */
> + return -EINVAL;
> +}
> +
What about chekcing:
if (pctrl->data->read_oen)
ret = pctrl->data->read_oen()
if (pctrl->data->write_oen)
ret = pctrl->data->write_oen()
Accross the driver. This will avoid adding stubs each time suppor for a new
IP is added.
> static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev,
> unsigned int _pin,
> unsigned long *config)
> {
> struct rzg2l_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
> - enum pin_config_param param = pinconf_to_config_param(*config);
> const struct rzg2l_hwcfg *hwcfg = pctrl->data->hwcfg;
> const struct pinctrl_pin_desc *pin = &pctrl->desc.pins[_pin];
> + u32 param = pinconf_to_config_param(*config);
> u64 *pin_data = pin->drv_data;
> unsigned int arg = 0;
> u32 off, cfg;
> @@ -1180,6 +1347,30 @@ static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev,
> break;
> }
>
> + case RENESAS_RZV2H_PIN_CONFIG_OUTPUT_IMPEDANCE: {
> + u8 val;
> +
> + if (!(cfg & (PIN_CFG_IOLH_1 | PIN_CFG_IOLH_2 | PIN_CFG_IOLH_3 | PIN_CFG_IOLH_4)))
> + return -EINVAL;
> +
> + val = rzg2l_read_pin_config(pctrl, IOLH(off), bit, IOLH_MASK);
> + switch (val) {
> + case 0:
> + arg = 1;
> + break;
> + case 1:
> + arg = 2;
> + break;
> + case 2:
> + arg = 4;
> + break;
> + default:
> + arg = 6;
> + break;
> + }
> + break;
> + }
> +
> default:
> return -ENOTSUPP;
> }
> @@ -1199,9 +1390,9 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev,
> const struct rzg2l_hwcfg *hwcfg = pctrl->data->hwcfg;
> struct rzg2l_pinctrl_pin_settings settings = pctrl->settings[_pin];
> u64 *pin_data = pin->drv_data;
> - enum pin_config_param param;
> unsigned int i, arg, index;
> u32 cfg, off;
> + u32 param;
> int ret;
> u8 bit;
>
> @@ -1283,6 +1474,32 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev,
> rzg2l_rmw_pin_config(pctrl, IOLH(off), bit, IOLH_MASK, index);
> break;
>
> + case RENESAS_RZV2H_PIN_CONFIG_OUTPUT_IMPEDANCE:
> + arg = pinconf_to_config_argument(_configs[i]);
> +
> + if (!(cfg & (PIN_CFG_IOLH_1 | PIN_CFG_IOLH_2 |
> + PIN_CFG_IOLH_3 | PIN_CFG_IOLH_4)))
> + return -EINVAL;
> +
> + switch (arg) {
> + case 1:
> + index = 0;
> + break;
> + case 2:
> + index = 1;
> + break;
> + case 4:
> + index = 2;
> + break;
> + case 6:
> + index = 3;
> + break;
> + default:
> + return -EINVAL;
> + }
> + rzg2l_rmw_pin_config(pctrl, IOLH(off), bit, IOLH_MASK, index);
> + break;
> +
> default:
> return -EOPNOTSUPP;
> }
> @@ -1730,6 +1947,38 @@ static const u64 r9a08g045_gpio_configs[] = {
> RZG2L_GPIO_PORT_PACK(6, 0x2a, RZG3S_MPXED_PIN_FUNCS(A)), /* P18 */
> };
>
> +static const char * const rzv2h_gpio_names[] = {
> + "P00", "P01", "P02", "P03", "P04", "P05", "P06", "P07",
> + "P10", "P11", "P12", "P13", "P14", "P15", "P16", "P17",
> + "P20", "P21", "P22", "P23", "P24", "P25", "P26", "P27",
> + "P30", "P31", "P32", "P33", "P34", "P35", "P36", "P37",
> + "P40", "P41", "P42", "P43", "P44", "P45", "P46", "P47",
> + "P50", "P51", "P52", "P53", "P54", "P55", "P56", "P57",
> + "P60", "P61", "P62", "P63", "P64", "P65", "P66", "P67",
> + "P70", "P71", "P72", "P73", "P74", "P75", "P76", "P77",
> + "P80", "P81", "P82", "P83", "P84", "P85", "P86", "P87",
> + "P90", "P91", "P92", "P93", "P94", "P95", "P96", "P97",
> + "PA0", "PA1", "PA2", "PA3", "PA4", "PA5", "PA6", "PA7",
> + "PB0", "PB1", "PB2", "PB3", "PB4", "PB5", "PB6", "PB7",
> +};
> +
> +static const u64 r9a09g057_gpio_configs[] = {
> + RZG2L_GPIO_PORT_PACK(8, 0x20, RZV2H_MPXED_PIN_FUNCS(3) | PIN_CFG_SCHMIT_CTRL), /* P0 */
> + RZG2L_GPIO_PORT_PACK(6, 0x21, RZV2H_MPXED_PIN_FUNCS(3) | PIN_CFG_SCHMIT_CTRL), /* P1 */
> + RZG2L_GPIO_PORT_PACK(2, 0x22, RZV2H_MPXED_PIN_FUNCS(4)), /* P2 */
> + RZG2L_GPIO_PORT_PACK(8, 0x23, RZV2H_MPXED_PIN_FUNCS(3) | PIN_CFG_SCHMIT_CTRL), /* P3 */
> + RZG2L_GPIO_PORT_PACK(8, 0x24, RZV2H_MPXED_PIN_FUNCS(3) | PIN_CFG_SCHMIT_CTRL), /* P4 */
> + RZG2L_GPIO_PORT_PACK(8, 0x25, RZV2H_MPXED_PIN_FUNCS(3) | PIN_CFG_SCHMIT_CTRL), /* P5 */
> + RZG2L_GPIO_PORT_PACK(8, 0x26, RZV2H_MPXED_PIN_FUNCS(3) | PIN_CFG_SCHMIT_CTRL |
> + PIN_CFG_ELC), /* P6 */
> + RZG2L_GPIO_PORT_PACK(8, 0x27, RZV2H_MPXED_PIN_FUNCS(3) | PIN_CFG_SCHMIT_CTRL), /* P7 */
> + RZG2L_GPIO_PORT_PACK(8, 0x28, RZV2H_MPXED_PIN_FUNCS(3) | PIN_CFG_SCHMIT_CTRL |
> + PIN_CFG_ELC), /* P8 */
> + RZG2L_GPIO_PORT_PACK(8, 0x29, PIN_CFG_VARIABLE), /* P9 */
> + RZG2L_GPIO_PORT_PACK(8, 0x2a, RZV2H_MPXED_PIN_FUNCS(3) | PIN_CFG_SCHMIT_CTRL), /* PA */
> + RZG2L_GPIO_PORT_PACK(6, 0x2b, PIN_CFG_VARIABLE), /* PB */
> +};
> +
> static const struct {
> struct rzg2l_dedicated_configs common[35];
> struct rzg2l_dedicated_configs rzg2l_pins[7];
> @@ -1856,6 +2105,139 @@ static const struct rzg2l_dedicated_configs rzg3s_dedicated_pins[] = {
> PIN_CFG_IO_VMC_SD1)) },
> };
>
> +static struct rzg2l_dedicated_configs rzv2h_dedicated_pins[] = {
> + { "NMI", RZG2L_SINGLE_PIN_PACK(0x1, 0, (PIN_CFG_FILONOFF | PIN_CFG_FILNUM |
> + PIN_CFG_FILCLKSEL)) },
> + { "TMS_SWDIO", RZG2L_SINGLE_PIN_PACK(0x3, 0, (PIN_CFG_IOLH_1 | PIN_CFG_SR |
> + PIN_CFG_IEN)) },
> + { "TDO", RZG2L_SINGLE_PIN_PACK(0x3, 2, (PIN_CFG_IOLH_1 | PIN_CFG_SR)) },
> + { "WDTUDFCA", RZG2L_SINGLE_PIN_PACK(0x5, 0, (PIN_CFG_IOLH_1 | PIN_CFG_SR |
> + PIN_CFG_PUPD)) },
> + { "WDTUDFCM", RZG2L_SINGLE_PIN_PACK(0x5, 1, (PIN_CFG_IOLH_1 | PIN_CFG_SR |
> + PIN_CFG_PUPD)) },
> + { "SCIF_RXD", RZG2L_SINGLE_PIN_PACK(0x6, 0, (PIN_CFG_IOLH_1 | PIN_CFG_SR |
> + PIN_CFG_PUPD)) },
> + { "SCIF_TXD", RZG2L_SINGLE_PIN_PACK(0x6, 1, (PIN_CFG_IOLH_1 | PIN_CFG_SR |
> + PIN_CFG_PUPD)) },
> + { "XSPI0_CKP", RZG2L_SINGLE_PIN_PACK(0x7, 0, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_PUPD | PIN_CFG_OEN)) },
> + { "XSPI0_CKN", RZG2L_SINGLE_PIN_PACK(0x7, 1, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_PUPD | PIN_CFG_OEN)) },
> + { "XSPI0_CS0N", RZG2L_SINGLE_PIN_PACK(0x7, 2, (PIN_CFG_IOLH_1 | PIN_CFG_SR |
> + PIN_CFG_PUPD | PIN_CFG_OEN)) },
> + { "XSPI0_DS", RZG2L_SINGLE_PIN_PACK(0x7, 3, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_PUPD)) },
> + { "XSPI0_RESET0N", RZG2L_SINGLE_PIN_PACK(0x7, 4, (PIN_CFG_IOLH_1 | PIN_CFG_SR |
> + PIN_CFG_PUPD | PIN_CFG_OEN)) },
> + { "XSPI0_RSTO0N", RZG2L_SINGLE_PIN_PACK(0x7, 5, (PIN_CFG_PUPD)) },
> + { "XSPI0_INT0N", RZG2L_SINGLE_PIN_PACK(0x7, 6, (PIN_CFG_PUPD)) },
> + { "XSPI0_ECS0N", RZG2L_SINGLE_PIN_PACK(0x7, 7, (PIN_CFG_PUPD)) },
> + { "XSPI0_IO0", RZG2L_SINGLE_PIN_PACK(0x8, 0, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_PUPD)) },
> + { "XSPI0_IO1", RZG2L_SINGLE_PIN_PACK(0x8, 1, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_PUPD)) },
> + { "XSPI0_IO2", RZG2L_SINGLE_PIN_PACK(0x8, 2, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_PUPD)) },
> + { "XSPI0_IO3", RZG2L_SINGLE_PIN_PACK(0x8, 3, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_PUPD)) },
> + { "XSPI0_IO4", RZG2L_SINGLE_PIN_PACK(0x8, 4, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_PUPD)) },
> + { "XSPI0_IO5", RZG2L_SINGLE_PIN_PACK(0x8, 5, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_PUPD)) },
> + { "XSPI0_IO6", RZG2L_SINGLE_PIN_PACK(0x8, 6, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_PUPD)) },
> + { "XSPI0_IO7", RZG2L_SINGLE_PIN_PACK(0x8, 7, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_PUPD)) },
> + { "SD0CLK", RZG2L_SINGLE_PIN_PACK(0x9, 0, (PIN_CFG_IOLH_2 | PIN_CFG_SR)) },
> + { "SD0CMD", RZG2L_SINGLE_PIN_PACK(0x9, 1, (PIN_CFG_IOLH_2 | PIN_CFG_SR)) },
> + { "SD0RSTN", RZG2L_SINGLE_PIN_PACK(0x9, 2, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_IEN | PIN_CFG_PUPD)) },
> + { "SD0DAT0", RZG2L_SINGLE_PIN_PACK(0xa, 0, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_IEN | PIN_CFG_PUPD)) },
> + { "SD0DAT1", RZG2L_SINGLE_PIN_PACK(0xa, 1, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_IEN | PIN_CFG_PUPD)) },
> + { "SD0DAT2", RZG2L_SINGLE_PIN_PACK(0xa, 2, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_IEN | PIN_CFG_PUPD)) },
> + { "SD0DAT3", RZG2L_SINGLE_PIN_PACK(0xa, 3, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_IEN | PIN_CFG_PUPD)) },
> + { "SD0DAT4", RZG2L_SINGLE_PIN_PACK(0xa, 4, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_IEN | PIN_CFG_PUPD)) },
> + { "SD0DAT5", RZG2L_SINGLE_PIN_PACK(0xa, 5, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_IEN | PIN_CFG_PUPD)) },
> + { "SD0DAT6", RZG2L_SINGLE_PIN_PACK(0xa, 6, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_IOLH_2 | PIN_CFG_PUPD)) },
> + { "SD0DAT7", RZG2L_SINGLE_PIN_PACK(0xa, 7, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_IEN | PIN_CFG_PUPD)) },
> + { "SD1_CLK", RZG2L_SINGLE_PIN_PACK(0xb, 0, (PIN_CFG_IOLH_2 | PIN_CFG_SR)) },
> + { "SD1_CMD", RZG2L_SINGLE_PIN_PACK(0xb, 1, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_IEN | PIN_CFG_PUPD)) },
> + { "SD1_DAT0", RZG2L_SINGLE_PIN_PACK(0xc, 0, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_IEN | PIN_CFG_PUPD)) },
> + { "SD1_DAT1", RZG2L_SINGLE_PIN_PACK(0xc, 1, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_IEN | PIN_CFG_PUPD)) },
> + { "SD1_DAT2", RZG2L_SINGLE_PIN_PACK(0xc, 2, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_IEN | PIN_CFG_PUPD)) },
> + { "SD1_DAT3", RZG2L_SINGLE_PIN_PACK(0xc, 3, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_IEN | PIN_CFG_PUPD)) },
> + { "PCIE0_RSTOUTB", RZG2L_SINGLE_PIN_PACK(0xe, 0, (PIN_CFG_IOLH_1 | PIN_CFG_SR)) },
> + { "PCIE1_RSTOUTB", RZG2L_SINGLE_PIN_PACK(0xe, 1, (PIN_CFG_IOLH_1 | PIN_CFG_SR)) },
> + { "ET0_MDIO", RZG2L_SINGLE_PIN_PACK(0xf, 0, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_IEN | PIN_CFG_PUPD)) },
> + { "ET0_MDC", RZG2L_SINGLE_PIN_PACK(0xf, 1, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_PUPD)) },
> + { "ET0_RXCTL_RXDV", RZG2L_SINGLE_PIN_PACK(0x10, 0, (PIN_CFG_PUPD)) },
> + { "ET0_TXCTL_TXEN", RZG2L_SINGLE_PIN_PACK(0x10, 1, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_PUPD)) },
> + { "ET0_TXER", RZG2L_SINGLE_PIN_PACK(0x10, 2, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_PUPD)) },
> + { "ET0_RXER", RZG2L_SINGLE_PIN_PACK(0x10, 3, (PIN_CFG_PUPD)) },
> + { "ET0_RXC_RXCLK", RZG2L_SINGLE_PIN_PACK(0x10, 4, (PIN_CFG_PUPD)) },
> + { "ET0_TXC_TXCLK", RZG2L_SINGLE_PIN_PACK(0x10, 5, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_PUPD | PIN_CFG_OEN)) },
> + { "ET0_CRS", RZG2L_SINGLE_PIN_PACK(0x10, 6, (PIN_CFG_PUPD)) },
> + { "ET0_COL", RZG2L_SINGLE_PIN_PACK(0x10, 7, (PIN_CFG_PUPD)) },
> + { "ET0_TXD0", RZG2L_SINGLE_PIN_PACK(0x11, 0, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_PUPD)) },
> + { "ET0_TXD1", RZG2L_SINGLE_PIN_PACK(0x11, 1, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_PUPD)) },
> + { "ET0_TXD2", RZG2L_SINGLE_PIN_PACK(0x11, 2, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_PUPD)) },
> + { "ET0_TXD3", RZG2L_SINGLE_PIN_PACK(0x11, 3, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_PUPD)) },
> + { "ET0_RXD0", RZG2L_SINGLE_PIN_PACK(0x11, 4, (PIN_CFG_PUPD)) },
> + { "ET0_RXD1", RZG2L_SINGLE_PIN_PACK(0x11, 5, (PIN_CFG_PUPD)) },
> + { "ET0_RXD2", RZG2L_SINGLE_PIN_PACK(0x11, 6, (PIN_CFG_PUPD)) },
> + { "ET0_RXD3", RZG2L_SINGLE_PIN_PACK(0x11, 7, (PIN_CFG_PUPD)) },
> + { "ET1_MDIO", RZG2L_SINGLE_PIN_PACK(0x12, 0, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_SR | PIN_CFG_IEN |
> + PIN_CFG_PUPD)) },
> + { "ET1_MDC", RZG2L_SINGLE_PIN_PACK(0x12, 1, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_PUPD)) },
> + { "ET1_RXCTL_RXDV", RZG2L_SINGLE_PIN_PACK(0x13, 0, (PIN_CFG_PUPD)) },
> + { "ET1_TXCTL_TXEN", RZG2L_SINGLE_PIN_PACK(0x13, 1, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_PUPD)) },
> + { "ET1_TXER", RZG2L_SINGLE_PIN_PACK(0x13, 2, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_PUPD)) },
> + { "ET1_RXER", RZG2L_SINGLE_PIN_PACK(0x13, 3, (PIN_CFG_PUPD)) },
> + { "ET1_RXC_RXCLK", RZG2L_SINGLE_PIN_PACK(0x13, 4, (PIN_CFG_PUPD)) },
> + { "ET1_TXC_TXCLK", RZG2L_SINGLE_PIN_PACK(0x13, 5, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_PUPD | PIN_CFG_OEN)) },
> + { "ET1_CRS", RZG2L_SINGLE_PIN_PACK(0x13, 6, (PIN_CFG_PUPD)) },
> + { "ET1_COL", RZG2L_SINGLE_PIN_PACK(0x13, 7, (PIN_CFG_PUPD)) },
> + { "ET1_TXD0", RZG2L_SINGLE_PIN_PACK(0x14, 0, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_PUPD)) },
> + { "ET1_TXD1", RZG2L_SINGLE_PIN_PACK(0x14, 1, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_PUPD)) },
> + { "ET1_TXD2", RZG2L_SINGLE_PIN_PACK(0x14, 2, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_PUPD)) },
> + { "ET1_TXD3", RZG2L_SINGLE_PIN_PACK(0x14, 3, (PIN_CFG_IOLH_2 | PIN_CFG_SR |
> + PIN_CFG_PUPD)) },
> + { "ET1_RXD0", RZG2L_SINGLE_PIN_PACK(0x14, 4, (PIN_CFG_PUPD)) },
> + { "ET1_RXD1", RZG2L_SINGLE_PIN_PACK(0x14, 5, (PIN_CFG_PUPD)) },
> + { "ET1_RXD2", RZG2L_SINGLE_PIN_PACK(0x14, 6, (PIN_CFG_PUPD)) },
> + { "ET1_RXD3", RZG2L_SINGLE_PIN_PACK(0x14, 7, (PIN_CFG_PUPD)) },
> +};
> +
> static int rzg2l_gpio_get_gpioint(unsigned int virq, struct rzg2l_pinctrl *pctrl)
> {
> const struct pinctrl_pin_desc *pin_desc = &pctrl->desc.pins[virq];
> @@ -2380,6 +2762,9 @@ static int rzg2l_pinctrl_probe(struct platform_device *pdev)
> BUILD_BUG_ON(ARRAY_SIZE(r9a08g045_gpio_configs) * RZG2L_PINS_PER_PORT >
> ARRAY_SIZE(rzg2l_gpio_names));
>
> + BUILD_BUG_ON(ARRAY_SIZE(r9a09g057_gpio_configs) * RZG2L_PINS_PER_PORT >
> + ARRAY_SIZE(rzv2h_gpio_names));
> +
> pctrl = devm_kzalloc(&pdev->dev, sizeof(*pctrl), GFP_KERNEL);
> if (!pctrl)
> return -ENOMEM;
> @@ -2402,6 +2787,7 @@ static int rzg2l_pinctrl_probe(struct platform_device *pdev)
>
> spin_lock_init(&pctrl->lock);
> spin_lock_init(&pctrl->bitmap_lock);
> + raw_spin_lock_init(&pctrl->pwpr_lock);
> mutex_init(&pctrl->mutex);
> atomic_set(&pctrl->wakeup_path, 0);
>
> @@ -2578,6 +2964,65 @@ static void rzg2l_pinctrl_pm_setup_pfc(struct rzg2l_pinctrl *pctrl)
> writel(PWPR_B0WI, pctrl->base + regs->pwpr); /* B0WI=1, PFCWE=0 */
> }
>
> +static void rzv2h_pinctrl_pm_setup_pfc(struct rzg2l_pinctrl *pctrl)
Have you managed to test this?
> +{
> + u32 nports = pctrl->data->n_port_pins / RZG2L_PINS_PER_PORT;
> + const struct rzg2l_hwcfg *hwcfg = pctrl->data->hwcfg;
> + const struct rzg2l_register_offsets *regs = &hwcfg->regs;
> + u8 pwpr;
> +
> + /* Set the PWPR register to allow PFC + PMC register to write. */
> + raw_spin_lock(&pctrl->pwpr_lock);
> + pwpr = readb(pctrl->base + regs->pwpr);
> + writeb(pwpr | PWPR_REGWE_A, pctrl->base + regs->pwpr); /* REGWE_A=1 */
> +
> + /* Restore port registers. */
> + for (u32 port = 0; port < nports; port++) {
> + unsigned long pinmap;
> + u8 pmc = 0, max_pin;
> + u32 off, pfc = 0;
> + u64 cfg;
> + u16 pm;
> + u8 pin;
> +
> + cfg = pctrl->data->port_pin_configs[port];
> + off = RZG2L_PIN_CFG_TO_PORT_OFFSET(cfg);
> + pinmap = FIELD_GET(PIN_CFG_PIN_MAP_MASK, cfg);
> + max_pin = fls(pinmap);
> +
> + pm = readw(pctrl->base + PM(off));
> + for_each_set_bit(pin, &pinmap, max_pin) {
> + struct rzg2l_pinctrl_reg_cache *cache = pctrl->cache;
> +
> + /* Nothing to do if PFC was not configured before. */
> + if (!(cache->pmc[port] & BIT(pin)))
> + continue;
> +
> + /* Set pin to 'Non-use (Hi-Z input protection)' */
> + pm &= ~(PM_MASK << (pin * 2));
> + writew(pm, pctrl->base + PM(off));
> +
> + /* Temporarily switch to GPIO mode with PMC register */
> + pmc &= ~BIT(pin);
> + writeb(pmc, pctrl->base + PMC(off));
> +
> + /* Select Pin function mode. */
> + pfc &= ~(PFC_MASK << (pin * 4));
> + pfc |= (cache->pfc[port] & (PFC_MASK << (pin * 4)));
> + writel(pfc, pctrl->base + PFC(off));
> +
> + /* Switch to Peripheral pin function. */
> + pmc |= BIT(pin);
> + writeb(pmc, pctrl->base + PMC(off));
> + }
> + }
> +
> + /* Set the PWPR register to be write-protected. */
> + pwpr = readb(pctrl->base + regs->pwpr);
> + writeb(pwpr & ~PWPR_REGWE_A, pctrl->base + regs->pwpr); /* REGWE_A=0 */
> + raw_spin_unlock(&pctrl->pwpr_lock);
> +}
> +
> static int rzg2l_pinctrl_suspend_noirq(struct device *dev)
> {
> struct rzg2l_pinctrl *pctrl = dev_get_drvdata(dev);
> @@ -2682,6 +3127,14 @@ static const struct rzg2l_hwcfg rzg3s_hwcfg = {
> .oen_max_port = 7, /* P7_1 is the maximum OEN port. */
> };
>
> +static const struct rzg2l_hwcfg rzv2h_hwcfg = {
> + .regs = {
> + .pwpr = 0x3c04,
> + .sd_ch = 0x0,
> + .eth_poc = 0x0,
> + },
> +};
> +
> static struct rzg2l_pinctrl_data r9a07g043_data = {
> .port_pins = rzg2l_gpio_names,
> .port_pin_configs = r9a07g043_gpio_configs,
> @@ -2732,6 +3185,28 @@ static struct rzg2l_pinctrl_data r9a08g045_data = {
> .write_oen = &rzg2l_write_oen,
> };
>
> +static struct rzg2l_pinctrl_data r9a09g057_data = {
> + .port_pins = rzv2h_gpio_names,
> + .port_pin_configs = r9a09g057_gpio_configs,
> + .n_ports = ARRAY_SIZE(r9a09g057_gpio_configs),
> + .dedicated_pins = rzv2h_dedicated_pins,
> + .n_port_pins = ARRAY_SIZE(r9a09g057_gpio_configs) * RZG2L_PINS_PER_PORT,
> + .n_dedicated_pins = ARRAY_SIZE(rzv2h_dedicated_pins),
> + .hwcfg = &rzv2h_hwcfg,
> + .variable_pin_cfg = r9a09g057_variable_pin_cfg,
> + .n_variable_pin_cfg = ARRAY_SIZE(r9a09g057_variable_pin_cfg),
> + .num_custom_params = ARRAY_SIZE(renesas_rzv2h_custom_bindings),
> + .custom_params = renesas_rzv2h_custom_bindings,
> +#ifdef CONFIG_DEBUG_FS
> + .custom_conf_items = renesas_rzv2h_conf_items,
> +#endif
> + .set_pfc_mode = &rzv2h_pinctrl_set_pfc_mode,
> + .pm_set_pfc = &rzv2h_pinctrl_pm_setup_pfc,
> + .pmc_writeb = &rzv2h_pmc_writeb,
> + .read_oen = &rzv2h_read_oen,
> + .write_oen = &rzv2h_write_oen,
> +};
> +
> static const struct of_device_id rzg2l_pinctrl_of_table[] = {
> {
> .compatible = "renesas,r9a07g043-pinctrl",
> @@ -2745,6 +3220,10 @@ static const struct of_device_id rzg2l_pinctrl_of_table[] = {
> .compatible = "renesas,r9a08g045-pinctrl",
> .data = &r9a08g045_data,
> },
> + {
> + .compatible = "renesas,r9a09g057-pinctrl",
> + .data = &r9a09g057_data,
> + },
> { /* sentinel */ }
> };
>
^ permalink raw reply
* Re: [RFC PATCH 08/13] pinctrl: renesas: pinctrl-rzg2l: Add function pointers for writing to PFC
From: claudiu beznea @ 2024-03-28 8:02 UTC (permalink / raw)
To: Prabhakar, Geert Uytterhoeven, Linus Walleij, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Magnus Damm
Cc: linux-renesas-soc, linux-gpio, devicetree, linux-kernel,
Fabrizio Castro, Lad Prabhakar
In-Reply-To: <20240326222844.1422948-9-prabhakar.mahadev-lad.rj@bp.renesas.com>
Hi, Prabhakar,
On 27.03.2024 00:28, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> On the RZ/G2L SoC, the PFCWE bit controls writing to PFC registers.
> However, on the RZ/V2H(P) SoC, the PFCWE (REGWE_A on RZ/V2H) bit controls
> writing to both PFC and PMC registers. To accommodate these differences
> across SoC variants, introduce set_pfc_mode() and pm_set_pfc() function
> pointers.
I think the overall code can be simplified if you add 1 function that does
the lock/unlock for PWPR. See patch 13.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> drivers/pinctrl/renesas/pinctrl-rzg2l.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> index 705372faaeff..4cdebdbd8a04 100644
> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> @@ -246,6 +246,8 @@ struct rzg2l_variable_pin_cfg {
> u32 pin:3;
> };
>
> +struct rzg2l_pinctrl;
> +
> struct rzg2l_pinctrl_data {
> const char * const *port_pins;
> const u64 *port_pin_configs;
> @@ -256,6 +258,8 @@ struct rzg2l_pinctrl_data {
> const struct rzg2l_hwcfg *hwcfg;
> const struct rzg2l_variable_pin_cfg *variable_pin_cfg;
> unsigned int n_variable_pin_cfg;
> + void (*set_pfc_mode)(struct rzg2l_pinctrl *pctrl, u8 pin, u8 off, u8 func);
> + void (*pm_set_pfc)(struct rzg2l_pinctrl *pctrl);
> };
>
> /**
> @@ -526,7 +530,7 @@ static int rzg2l_pinctrl_set_mux(struct pinctrl_dev *pctldev,
> dev_dbg(pctrl->dev, "port:%u pin: %u off:%x PSEL:%u\n",
> RZG2L_PIN_ID_TO_PORT(pins[i]), pin, off, psel_val[i] - hwcfg->func_base);
>
> - rzg2l_pinctrl_set_pfc_mode(pctrl, pin, off, psel_val[i] - hwcfg->func_base);
> + pctrl->data->set_pfc_mode(pctrl, pin, off, psel_val[i] - hwcfg->func_base);
> }
>
> return 0;
> @@ -2607,7 +2611,7 @@ static int rzg2l_pinctrl_resume_noirq(struct device *dev)
> writeb(cache->eth_poc[i], pctrl->base + ETH_POC(regs->eth_poc, i));
> }
>
> - rzg2l_pinctrl_pm_setup_pfc(pctrl);
> + pctrl->data->pm_set_pfc(pctrl);
> rzg2l_pinctrl_pm_setup_regs(pctrl, false);
> rzg2l_pinctrl_pm_setup_dedicated_regs(pctrl, false);
> rzg2l_gpio_irq_restore(pctrl);
> @@ -2672,6 +2676,8 @@ static struct rzg2l_pinctrl_data r9a07g043_data = {
> .variable_pin_cfg = r9a07g043f_variable_pin_cfg,
> .n_variable_pin_cfg = ARRAY_SIZE(r9a07g043f_variable_pin_cfg),
> #endif
> + .set_pfc_mode = &rzg2l_pinctrl_set_pfc_mode,
> + .pm_set_pfc = &rzg2l_pinctrl_pm_setup_pfc,
> };
>
> static struct rzg2l_pinctrl_data r9a07g044_data = {
> @@ -2683,6 +2689,8 @@ static struct rzg2l_pinctrl_data r9a07g044_data = {
> .n_dedicated_pins = ARRAY_SIZE(rzg2l_dedicated_pins.common) +
> ARRAY_SIZE(rzg2l_dedicated_pins.rzg2l_pins),
> .hwcfg = &rzg2l_hwcfg,
> + .set_pfc_mode = &rzg2l_pinctrl_set_pfc_mode,
> + .pm_set_pfc = &rzg2l_pinctrl_pm_setup_pfc,
> };
>
> static struct rzg2l_pinctrl_data r9a08g045_data = {
> @@ -2693,6 +2701,8 @@ static struct rzg2l_pinctrl_data r9a08g045_data = {
> .n_port_pins = ARRAY_SIZE(r9a08g045_gpio_configs) * RZG2L_PINS_PER_PORT,
> .n_dedicated_pins = ARRAY_SIZE(rzg3s_dedicated_pins),
> .hwcfg = &rzg3s_hwcfg,
> + .set_pfc_mode = &rzg2l_pinctrl_set_pfc_mode,
> + .pm_set_pfc = &rzg2l_pinctrl_pm_setup_pfc,
> };
>
> static const struct of_device_id rzg2l_pinctrl_of_table[] = {
^ permalink raw reply
* [PATCH v1] ARM: dts: stm32: Update button on stm32mp135f-dk
From: patrice.chotard @ 2024-03-28 8:01 UTC (permalink / raw)
To: robh+dt, Krzysztof Kozlowski, alexandre.torgue
Cc: linux-stm32, linux-arm-kernel, linux-kernel, devicetree,
Patrice Chotard
From: Patrice Chotard <patrice.chotard@foss.st.com>
On schematics, 2 buttons are available on stm32mp135-dk board:
_ button "user1" connected to GPIOA14
_ button "user2" connected to GPIOA13
Reflect that on device tree.
Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com>
---
arch/arm/boot/dts/st/stm32mp135f-dk.dts | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/st/stm32mp135f-dk.dts b/arch/arm/boot/dts/st/stm32mp135f-dk.dts
index 52171214a308..f7e03bc7eccb 100644
--- a/arch/arm/boot/dts/st/stm32mp135f-dk.dts
+++ b/arch/arm/boot/dts/st/stm32mp135f-dk.dts
@@ -48,9 +48,15 @@ optee@dd000000 {
gpio-keys {
compatible = "gpio-keys";
- button-user {
- label = "User-PA13";
+ button-user-1 {
+ label = "User-1";
linux,code = <BTN_1>;
+ gpios = <&gpioa 14 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>;
+ };
+
+ button-user-2 {
+ label = "User-2";
+ linux,code = <BTN_2>;
gpios = <&gpioa 13 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>;
};
};
--
2.25.1
^ permalink raw reply related
* Re: [RFC PATCH 07/13] pinctrl: renesas: pinctrl-rzg2l: Validate power registers for SD and ETH
From: claudiu beznea @ 2024-03-28 8:01 UTC (permalink / raw)
To: Prabhakar, Geert Uytterhoeven, Linus Walleij, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Magnus Damm
Cc: linux-renesas-soc, linux-gpio, devicetree, linux-kernel,
Fabrizio Castro, Lad Prabhakar
In-Reply-To: <20240326222844.1422948-8-prabhakar.mahadev-lad.rj@bp.renesas.com>
Hi, Prabhakar,
On 27.03.2024 00:28, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> On RZ/V2H(P) SoC, the power registers for SD and ETH do not exist,
> resulting in invalid register offsets. Ensure that the register offsets
> are valid before any read/write operations are performed. If the power
> registers are not available, both SD and ETH will be set to -EINVAL.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> drivers/pinctrl/renesas/pinctrl-rzg2l.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> index 348fdccaff72..705372faaeff 100644
> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> @@ -184,8 +184,8 @@
> */
> struct rzg2l_register_offsets {
> u16 pwpr;
> - u16 sd_ch;
> - u16 eth_poc;
> + int sd_ch;
> + int eth_poc;
> };
>
> /**
> @@ -2567,8 +2567,10 @@ static int rzg2l_pinctrl_suspend_noirq(struct device *dev)
> rzg2l_pinctrl_pm_setup_dedicated_regs(pctrl, true);
>
> for (u8 i = 0; i < 2; i++) {
> - cache->sd_ch[i] = readb(pctrl->base + SD_CH(regs->sd_ch, i));
> - cache->eth_poc[i] = readb(pctrl->base + ETH_POC(regs->eth_poc, i));
> + if (regs->sd_ch != -EINVAL)
As of my knowledge, the current users of this driver uses SD and ETH
offsets different from zero. To avoid populating these values for all the
SoCs and avoid increasing the size of these fields I think you can add
checks like these:
if (regs->sd_ch)
// set sd_ch
Same for the rest.
> + cache->sd_ch[i] = readb(pctrl->base + SD_CH(regs->sd_ch, i));
> + if (regs->eth_poc != -EINVAL)
> + cache->eth_poc[i] = readb(pctrl->base + ETH_POC(regs->eth_poc, i));
> }
>
> cache->qspi = readb(pctrl->base + QSPI);
> @@ -2599,8 +2601,10 @@ static int rzg2l_pinctrl_resume_noirq(struct device *dev)
> writeb(cache->qspi, pctrl->base + QSPI);
> writeb(cache->eth_mode, pctrl->base + ETH_MODE);
> for (u8 i = 0; i < 2; i++) {
> - writeb(cache->sd_ch[i], pctrl->base + SD_CH(regs->sd_ch, i));
> - writeb(cache->eth_poc[i], pctrl->base + ETH_POC(regs->eth_poc, i));
> + if (regs->sd_ch != -EINVAL)
> + writeb(cache->sd_ch[i], pctrl->base + SD_CH(regs->sd_ch, i));
> + if (regs->eth_poc != -EINVAL)
> + writeb(cache->eth_poc[i], pctrl->base + ETH_POC(regs->eth_poc, i));
> }
>
> rzg2l_pinctrl_pm_setup_pfc(pctrl);
^ permalink raw reply
* [PATCH v5 5/5] arm64: dts: qcom: ipq9574: Add icc provider ability to gcc
From: Varadarajan Narayanan @ 2024-03-28 7:59 UTC (permalink / raw)
To: andersson, konrad.dybcio, mturquette, sboyd, robh,
krzysztof.kozlowski+dt, conor+dt, djakov, dmitry.baryshkov,
quic_varada, quic_anusha, linux-arm-msm, linux-clk, devicetree,
linux-kernel, linux-pm
In-Reply-To: <20240328075936.223461-1-quic_varada@quicinc.com>
IPQ SoCs dont involve RPM in managing NoC related clocks and
there is no NoC scaling. Linux itself handles these clocks.
However, these should not be exposed as just clocks and align
with other Qualcomm SoCs that handle these clocks from a
interconnect provider.
Hence include icc provider capability to the gcc node so that
peripherals can use the interconnect facility to enable these
clocks.
Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v2: Fix include file order
Move to separate patch
---
arch/arm64/boot/dts/qcom/ipq9574.dtsi | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
index 7f2e5cbf3bbb..5b3e69379b1f 100644
--- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
@@ -8,6 +8,7 @@
#include <dt-bindings/clock/qcom,apss-ipq.h>
#include <dt-bindings/clock/qcom,ipq9574-gcc.h>
+#include <dt-bindings/interconnect/qcom,ipq9574.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>
#include <dt-bindings/reset/qcom,ipq9574-gcc.h>
#include <dt-bindings/thermal/thermal.h>
@@ -306,6 +307,7 @@ gcc: clock-controller@1800000 {
#clock-cells = <1>;
#reset-cells = <1>;
#power-domain-cells = <1>;
+ #interconnect-cells = <1>;
};
tcsr_mutex: hwlock@1905000 {
--
2.34.1
^ permalink raw reply related
* [PATCH v5 4/5] clk: qcom: ipq9574: Use icc-clk for enabling NoC related clocks
From: Varadarajan Narayanan @ 2024-03-28 7:59 UTC (permalink / raw)
To: andersson, konrad.dybcio, mturquette, sboyd, robh,
krzysztof.kozlowski+dt, conor+dt, djakov, dmitry.baryshkov,
quic_varada, quic_anusha, linux-arm-msm, linux-clk, devicetree,
linux-kernel, linux-pm
In-Reply-To: <20240328075936.223461-1-quic_varada@quicinc.com>
Use the icc-clk framework to enable few clocks to be able to
create paths and use the peripherals connected on those NoCs.
Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v5: Split from common.c changes into separate patch
No functional changes
---
drivers/clk/qcom/Kconfig | 2 ++
drivers/clk/qcom/gcc-ipq9574.c | 54 ++++++++++++++++++++++++++++++++++
2 files changed, 56 insertions(+)
diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index 8ab08e7b5b6c..af73a0b396eb 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -243,6 +243,8 @@ config IPQ_GCC_8074
config IPQ_GCC_9574
tristate "IPQ9574 Global Clock Controller"
+ select INTERCONNECT
+ select INTERCONNECT_CLK
help
Support for global clock controller on ipq9574 devices.
Say Y if you want to use peripheral devices such as UART, SPI,
diff --git a/drivers/clk/qcom/gcc-ipq9574.c b/drivers/clk/qcom/gcc-ipq9574.c
index 0a3f846695b8..187fd9dcdf49 100644
--- a/drivers/clk/qcom/gcc-ipq9574.c
+++ b/drivers/clk/qcom/gcc-ipq9574.c
@@ -12,6 +12,7 @@
#include <dt-bindings/clock/qcom,ipq9574-gcc.h>
#include <dt-bindings/reset/qcom,ipq9574-gcc.h>
+#include <dt-bindings/interconnect/qcom,ipq9574.h>
#include "clk-alpha-pll.h"
#include "clk-branch.h"
@@ -4301,6 +4302,56 @@ static const struct qcom_reset_map gcc_ipq9574_resets[] = {
[GCC_WCSS_Q6_TBU_BCR] = { 0x12054, 0 },
};
+#define IPQ_APPS_ID 9574 /* some unique value */
+
+enum {
+ ICC_ANOC_PCIE0,
+ ICC_SNOC_PCIE0,
+ ICC_ANOC_PCIE1,
+ ICC_SNOC_PCIE1,
+ ICC_ANOC_PCIE2,
+ ICC_SNOC_PCIE2,
+ ICC_ANOC_PCIE3,
+ ICC_SNOC_PCIE3,
+ ICC_SNOC_USB,
+ ICC_ANOC_USB_AXI,
+ ICC_NSSNOC_NSSCC,
+ ICC_NSSNOC_SNOC_0,
+ ICC_NSSNOC_SNOC_1,
+ ICC_NSSNOC_PCNOC_1,
+ ICC_NSSNOC_QOSGEN_REF,
+ ICC_NSSNOC_TIMEOUT_REF,
+ ICC_NSSNOC_XO_DCD,
+ ICC_NSSNOC_ATB,
+ ICC_MEM_NOC_NSSNOC,
+ ICC_NSSNOC_MEMNOC,
+ ICC_NSSNOC_MEM_NOC_1,
+};
+
+static struct clk_hw *icc_ipq9574_hws[] = {
+ [ICC_ANOC_PCIE0] = &gcc_anoc_pcie0_1lane_m_clk.clkr.hw,
+ [ICC_SNOC_PCIE0] = &gcc_anoc_pcie1_1lane_m_clk.clkr.hw,
+ [ICC_ANOC_PCIE1] = &gcc_anoc_pcie2_2lane_m_clk.clkr.hw,
+ [ICC_SNOC_PCIE1] = &gcc_anoc_pcie3_2lane_m_clk.clkr.hw,
+ [ICC_ANOC_PCIE2] = &gcc_snoc_pcie0_1lane_s_clk.clkr.hw,
+ [ICC_SNOC_PCIE2] = &gcc_snoc_pcie1_1lane_s_clk.clkr.hw,
+ [ICC_ANOC_PCIE3] = &gcc_snoc_pcie2_2lane_s_clk.clkr.hw,
+ [ICC_SNOC_PCIE3] = &gcc_snoc_pcie3_2lane_s_clk.clkr.hw,
+ [ICC_SNOC_USB] = &gcc_snoc_usb_clk.clkr.hw,
+ [ICC_ANOC_USB_AXI] = &gcc_anoc_usb_axi_clk.clkr.hw,
+ [ICC_NSSNOC_NSSCC] = &gcc_nssnoc_nsscc_clk.clkr.hw,
+ [ICC_NSSNOC_SNOC_0] = &gcc_nssnoc_snoc_clk.clkr.hw,
+ [ICC_NSSNOC_SNOC_1] = &gcc_nssnoc_snoc_1_clk.clkr.hw,
+ [ICC_NSSNOC_PCNOC_1] = &gcc_nssnoc_pcnoc_1_clk.clkr.hw,
+ [ICC_NSSNOC_QOSGEN_REF] = &gcc_nssnoc_qosgen_ref_clk.clkr.hw,
+ [ICC_NSSNOC_TIMEOUT_REF] = &gcc_nssnoc_timeout_ref_clk.clkr.hw,
+ [ICC_NSSNOC_XO_DCD] = &gcc_nssnoc_xo_dcd_clk.clkr.hw,
+ [ICC_NSSNOC_ATB] = &gcc_nssnoc_atb_clk.clkr.hw,
+ [ICC_MEM_NOC_NSSNOC] = &gcc_mem_noc_nssnoc_clk.clkr.hw,
+ [ICC_NSSNOC_MEMNOC] = &gcc_nssnoc_memnoc_clk.clkr.hw,
+ [ICC_NSSNOC_MEM_NOC_1] = &gcc_nssnoc_mem_noc_1_clk.clkr.hw,
+};
+
static const struct of_device_id gcc_ipq9574_match_table[] = {
{ .compatible = "qcom,ipq9574-gcc" },
{ }
@@ -4323,6 +4374,9 @@ static const struct qcom_cc_desc gcc_ipq9574_desc = {
.num_resets = ARRAY_SIZE(gcc_ipq9574_resets),
.clk_hws = gcc_ipq9574_hws,
.num_clk_hws = ARRAY_SIZE(gcc_ipq9574_hws),
+ .icc_hws = icc_ipq9574_hws,
+ .num_icc_hws = ARRAY_SIZE(icc_ipq9574_hws),
+ .first_id = IPQ_APPS_ID,
};
static int gcc_ipq9574_probe(struct platform_device *pdev)
--
2.34.1
^ permalink raw reply related
* [PATCH v5 3/5] clk: qcom: common: Add interconnect clocks support
From: Varadarajan Narayanan @ 2024-03-28 7:59 UTC (permalink / raw)
To: andersson, konrad.dybcio, mturquette, sboyd, robh,
krzysztof.kozlowski+dt, conor+dt, djakov, dmitry.baryshkov,
quic_varada, quic_anusha, linux-arm-msm, linux-clk, devicetree,
linux-kernel, linux-pm
In-Reply-To: <20240328075936.223461-1-quic_varada@quicinc.com>
Unlike MSM platforms that manage NoC related clocks and scaling
from RPM, IPQ SoCs dont involve RPM in managing NoC related
clocks and there is no NoC scaling.
However, there is a requirement to enable some NoC interface
clocks for accessing the peripheral controllers present on
these NoCs. Though exposing these as normal clocks would work,
having a minimalistic interconnect driver to handle these clocks
would make it consistent with other Qualcomm platforms resulting
in common code paths. This is similar to msm8996-cbf's usage of
icc-clk framework.
Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v5: Split changes in common.c to separate patch
Fix error handling
Use devm_icc_clk_register instead of icc_clk_register
v4: Use clk_hw instead of indices
Do icc register in qcom_cc_probe() call stream
Add icc clock info to qcom_cc_desc structure
v3: Use indexed identifiers here to avoid confusion
Fix error messages and move to common.c
v2: Move DTS to separate patch
Update commit log
Auto select CONFIG_INTERCONNECT & CONFIG_INTERCONNECT_CLK to fix build error
---
drivers/clk/qcom/common.c | 39 ++++++++++++++++++++++++++++++++++++++-
drivers/clk/qcom/common.h | 3 +++
2 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
index 75f09e6e057e..9fa271812373 100644
--- a/drivers/clk/qcom/common.c
+++ b/drivers/clk/qcom/common.c
@@ -8,6 +8,8 @@
#include <linux/regmap.h>
#include <linux/platform_device.h>
#include <linux/clk-provider.h>
+#include <linux/interconnect-clk.h>
+#include <linux/interconnect-provider.h>
#include <linux/reset-controller.h>
#include <linux/of.h>
@@ -234,6 +236,41 @@ static struct clk_hw *qcom_cc_clk_hw_get(struct of_phandle_args *clkspec,
return cc->rclks[idx] ? &cc->rclks[idx]->hw : NULL;
}
+#if IS_ENABLED(CONFIG_INTERCONNECT_CLK)
+static int qcom_cc_icc_register(struct device *dev,
+ const struct qcom_cc_desc *desc)
+{
+ struct icc_clk_data *icd;
+ int i;
+
+ if (!desc->icc_hws)
+ return 0;
+
+ icd = devm_kcalloc(dev, desc->num_icc_hws, sizeof(*icd), GFP_KERNEL);
+ if (!icd)
+ return -ENOMEM;
+
+ for (i = 0; i < desc->num_icc_hws; i++) {
+ icd[i].clk = devm_clk_hw_get_clk(dev, desc->icc_hws[i], "qcom");
+ if (IS_ERR(icd[i].clk))
+ return dev_err_probe(dev, PTR_ERR(icd[i].clk),
+ "get clock failed (%ld)\n",
+ PTR_ERR(icd[i].clk));
+
+ icd[i].name = clk_hw_get_name(desc->icc_hws[i]);
+ }
+
+ return PTR_ERR_OR_ZERO(devm_icc_clk_register(dev, desc->first_id,
+ desc->num_icc_hws, icd));
+}
+#else
+static int qcom_cc_icc_register(struct device *dev,
+ const struct qcom_cc_desc *desc)
+{
+ return 0;
+}
+#endif
+
int qcom_cc_really_probe(struct platform_device *pdev,
const struct qcom_cc_desc *desc, struct regmap *regmap)
{
@@ -303,7 +340,7 @@ int qcom_cc_really_probe(struct platform_device *pdev,
if (ret)
return ret;
- return 0;
+ return qcom_cc_icc_register(dev, desc);
}
EXPORT_SYMBOL_GPL(qcom_cc_really_probe);
diff --git a/drivers/clk/qcom/common.h b/drivers/clk/qcom/common.h
index 9c8f7b798d9f..d8ac26d83f3c 100644
--- a/drivers/clk/qcom/common.h
+++ b/drivers/clk/qcom/common.h
@@ -29,6 +29,9 @@ struct qcom_cc_desc {
size_t num_gdscs;
struct clk_hw **clk_hws;
size_t num_clk_hws;
+ struct clk_hw **icc_hws;
+ size_t num_icc_hws;
+ unsigned int first_id;
};
/**
--
2.34.1
^ permalink raw reply related
* [PATCH v5 2/5] interconnect: icc-clk: Add devm_icc_clk_register
From: Varadarajan Narayanan @ 2024-03-28 7:59 UTC (permalink / raw)
To: andersson, konrad.dybcio, mturquette, sboyd, robh,
krzysztof.kozlowski+dt, conor+dt, djakov, dmitry.baryshkov,
quic_varada, quic_anusha, linux-arm-msm, linux-clk, devicetree,
linux-kernel, linux-pm
In-Reply-To: <20240328075936.223461-1-quic_varada@quicinc.com>
Wrap icc_clk_register to create devm_icc_clk_register to be
able to release the resources properly.
Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v5: Introduced devm_icc_clk_register
---
drivers/interconnect/icc-clk.c | 29 +++++++++++++++++++++++++++++
include/linux/interconnect-clk.h | 4 ++++
2 files changed, 33 insertions(+)
diff --git a/drivers/interconnect/icc-clk.c b/drivers/interconnect/icc-clk.c
index d787f2ea36d9..89f11fed8820 100644
--- a/drivers/interconnect/icc-clk.c
+++ b/drivers/interconnect/icc-clk.c
@@ -148,6 +148,35 @@ struct icc_provider *icc_clk_register(struct device *dev,
}
EXPORT_SYMBOL_GPL(icc_clk_register);
+static void devm_icc_release(struct device *dev, void *res)
+{
+ icc_clk_unregister(res);
+}
+
+struct icc_provider *devm_icc_clk_register(struct device *dev,
+ unsigned int first_id,
+ unsigned int num_clocks,
+ const struct icc_clk_data *data)
+{
+ struct icc_provider *prov, **provp;
+
+ provp = devres_alloc(devm_icc_release, sizeof(*provp), GFP_KERNEL);
+ if (!provp)
+ return ERR_PTR(-ENOMEM);
+
+ prov = icc_clk_register(dev, first_id, num_clocks, data);
+
+ if (!IS_ERR(prov)) {
+ *provp = prov;
+ devres_add(dev, provp);
+ } else {
+ devres_free(provp);
+ }
+
+ return prov;
+}
+EXPORT_SYMBOL_GPL(devm_icc_clk_register);
+
/**
* icc_clk_unregister() - unregister a previously registered clk interconnect provider
* @provider: provider returned by icc_clk_register()
diff --git a/include/linux/interconnect-clk.h b/include/linux/interconnect-clk.h
index 0cd80112bea5..cb7b648eb1c0 100644
--- a/include/linux/interconnect-clk.h
+++ b/include/linux/interconnect-clk.h
@@ -17,6 +17,10 @@ struct icc_provider *icc_clk_register(struct device *dev,
unsigned int first_id,
unsigned int num_clocks,
const struct icc_clk_data *data);
+struct icc_provider *devm_icc_clk_register(struct device *dev,
+ unsigned int first_id,
+ unsigned int num_clocks,
+ const struct icc_clk_data *data);
void icc_clk_unregister(struct icc_provider *provider);
#endif
--
2.34.1
^ permalink raw reply related
* [PATCH v5 1/5] dt-bindings: interconnect: Add Qualcomm IPQ9574 support
From: Varadarajan Narayanan @ 2024-03-28 7:59 UTC (permalink / raw)
To: andersson, konrad.dybcio, mturquette, sboyd, robh,
krzysztof.kozlowski+dt, conor+dt, djakov, dmitry.baryshkov,
quic_varada, quic_anusha, linux-arm-msm, linux-clk, devicetree,
linux-kernel, linux-pm
Cc: Krzysztof Kozlowski
In-Reply-To: <20240328075936.223461-1-quic_varada@quicinc.com>
Add interconnect-cells to clock provider so that it can be
used as icc provider.
Add master/slave ids for Qualcomm IPQ9574 Network-On-Chip
interfaces. This will be used by the gcc-ipq9574 driver
that will for providing interconnect services using the
icc-clk framework.
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v3:
Squash Documentation/ and include/ changes into same patch
qcom,ipq9574.h
Move 'first id' to clock driver
---
.../bindings/clock/qcom,ipq9574-gcc.yaml | 3 +
.../dt-bindings/interconnect/qcom,ipq9574.h | 59 +++++++++++++++++++
2 files changed, 62 insertions(+)
create mode 100644 include/dt-bindings/interconnect/qcom,ipq9574.h
diff --git a/Documentation/devicetree/bindings/clock/qcom,ipq9574-gcc.yaml b/Documentation/devicetree/bindings/clock/qcom,ipq9574-gcc.yaml
index 944a0ea79cd6..824781cbdf34 100644
--- a/Documentation/devicetree/bindings/clock/qcom,ipq9574-gcc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,ipq9574-gcc.yaml
@@ -33,6 +33,9 @@ properties:
- description: PCIE30 PHY3 pipe clock source
- description: USB3 PHY pipe clock source
+ '#interconnect-cells':
+ const: 1
+
required:
- compatible
- clocks
diff --git a/include/dt-bindings/interconnect/qcom,ipq9574.h b/include/dt-bindings/interconnect/qcom,ipq9574.h
new file mode 100644
index 000000000000..9c95fbd5dc46
--- /dev/null
+++ b/include/dt-bindings/interconnect/qcom,ipq9574.h
@@ -0,0 +1,59 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+#ifndef INTERCONNECT_QCOM_IPQ9574_H
+#define INTERCONNECT_QCOM_IPQ9574_H
+
+#define MASTER_ANOC_PCIE0_1 0
+#define SLAVE_ANOC_PCIE0_1 1
+#define MASTER_SNOC_PCIE0_1 2
+#define SLAVE_SNOC_PCIE0_1 3
+#define MASTER_ANOC_PCIE1_1 4
+#define SLAVE_ANOC_PCIE1_1 5
+#define MASTER_SNOC_PCIE1_1 6
+#define SLAVE_SNOC_PCIE1_1 7
+#define MASTER_ANOC_PCIE2_2 8
+#define SLAVE_ANOC_PCIE2_2 9
+#define MASTER_SNOC_PCIE2_2 10
+#define SLAVE_SNOC_PCIE2_2 11
+#define MASTER_ANOC_PCIE3_2 12
+#define SLAVE_ANOC_PCIE3_2 13
+#define MASTER_SNOC_PCIE3_2 14
+#define SLAVE_SNOC_PCIE3_2 15
+#define MASTER_USB 16
+#define SLAVE_USB 17
+#define MASTER_USB_AXI 18
+#define SLAVE_USB_AXI 19
+#define MASTER_NSSNOC_NSSCC 20
+#define SLAVE_NSSNOC_NSSCC 21
+#define MASTER_NSSNOC_SNOC 22
+#define SLAVE_NSSNOC_SNOC 23
+#define MASTER_NSSNOC_SNOC_1 24
+#define SLAVE_NSSNOC_SNOC_1 25
+#define MASTER_NSSNOC_PCNOC_1 26
+#define SLAVE_NSSNOC_PCNOC_1 27
+#define MASTER_NSSNOC_QOSGEN_REF 28
+#define SLAVE_NSSNOC_QOSGEN_REF 29
+#define MASTER_NSSNOC_TIMEOUT_REF 30
+#define SLAVE_NSSNOC_TIMEOUT_REF 31
+#define MASTER_NSSNOC_XO_DCD 32
+#define SLAVE_NSSNOC_XO_DCD 33
+#define MASTER_NSSNOC_ATB 34
+#define SLAVE_NSSNOC_ATB 35
+#define MASTER_MEM_NOC_NSSNOC 36
+#define SLAVE_MEM_NOC_NSSNOC 37
+#define MASTER_NSSNOC_MEMNOC 38
+#define SLAVE_NSSNOC_MEMNOC 39
+#define MASTER_NSSNOC_MEM_NOC_1 40
+#define SLAVE_NSSNOC_MEM_NOC_1 41
+
+#define MASTER_NSS_CC_NSSNOC_PPE 0
+#define SLAVE_NSS_CC_NSSNOC_PPE 1
+#define MASTER_NSS_CC_NSSNOC_PPE_CFG 2
+#define SLAVE_NSS_CC_NSSNOC_PPE_CFG 3
+#define MASTER_NSS_CC_NSSNOC_NSS_CSR 4
+#define SLAVE_NSS_CC_NSSNOC_NSS_CSR 5
+#define MASTER_NSS_CC_NSSNOC_IMEM_QSB 6
+#define SLAVE_NSS_CC_NSSNOC_IMEM_QSB 7
+#define MASTER_NSS_CC_NSSNOC_IMEM_AHB 8
+#define SLAVE_NSS_CC_NSSNOC_IMEM_AHB 9
+
+#endif /* INTERCONNECT_QCOM_IPQ9574_H */
--
2.34.1
^ permalink raw reply related
* [PATCH v5 0/5] Add interconnect driver for IPQ9574 SoC
From: Varadarajan Narayanan @ 2024-03-28 7:59 UTC (permalink / raw)
To: andersson, konrad.dybcio, mturquette, sboyd, robh,
krzysztof.kozlowski+dt, conor+dt, djakov, dmitry.baryshkov,
quic_varada, quic_anusha, linux-arm-msm, linux-clk, devicetree,
linux-kernel, linux-pm
MSM platforms manage NoC related clocks and scaling from RPM.
However, in IPQ SoCs, RPM is not involved in managing NoC
related clocks and there is no NoC scaling.
However, there is a requirement to enable some NoC interface
clocks for the accessing the peripherals present in the
system. Hence add a minimalistic interconnect driver that
establishes a path from the processor/memory to those peripherals
and vice versa.
---
v5:
Split gcc-ipq9574.c and common.c changes into separate patches
Introduce devm_icc_clk_register
Fix error handling
v4:
gcc-ipq9574.c
Use clk_hw instead of indices
common.c
Do icc register in qcom_cc_probe() call stream
common.h
Add icc clock info to qcom_cc_desc structure
v3:
qcom,ipq9574.h
Move 'first id' define to clock driver
gcc-ipq9574.c:
Use indexed identifiers here to avoid confusion
Fix error messages and move code to common.c as it can be
shared with future SoCs
v2:
qcom,ipq9574.h
Fix license identifier
Rename macros
qcom,ipq9574-gcc.yaml
Include interconnect-cells
gcc-ipq9574.c
Update commit log
Remove IS_ENABLED(CONFIG_INTERCONNECT) and auto select it from Kconfig
ipq9574.dtsi
Moved to separate patch
Include interconnect-cells to clock controller node
drivers/clk/qcom/Kconfig:
Auto select CONFIG_INTERCONNECT & CONFIG_INTERCONNECT_CLK
Varadarajan Narayanan (5):
dt-bindings: interconnect: Add Qualcomm IPQ9574 support
interconnect: icc-clk: Add devm_icc_clk_register
clk: qcom: common: Add interconnect clocks support
clk: qcom: ipq9574: Use icc-clk for enabling NoC related clocks
arm64: dts: qcom: ipq9574: Add icc provider ability to gcc
.../bindings/clock/qcom,ipq9574-gcc.yaml | 3 +
arch/arm64/boot/dts/qcom/ipq9574.dtsi | 2 +
drivers/clk/qcom/Kconfig | 2 +
drivers/clk/qcom/common.c | 39 +++++++++++-
drivers/clk/qcom/common.h | 3 +
drivers/clk/qcom/gcc-ipq9574.c | 54 +++++++++++++++++
drivers/interconnect/icc-clk.c | 29 +++++++++
.../dt-bindings/interconnect/qcom,ipq9574.h | 59 +++++++++++++++++++
include/linux/interconnect-clk.h | 4 ++
9 files changed, 194 insertions(+), 1 deletion(-)
create mode 100644 include/dt-bindings/interconnect/qcom,ipq9574.h
--
2.34.1
^ permalink raw reply
* Re: [PATCH 18/23] dt-bindings: media: imx258: Add alternate compatible strings
From: Krzysztof Kozlowski @ 2024-03-28 7:47 UTC (permalink / raw)
To: git, linux-media
Cc: dave.stevenson, jacopo.mondi, mchehab, robh,
krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
festevam, sakari.ailus, devicetree, imx, linux-arm-kernel,
linux-kernel
In-Reply-To: <20240327231710.53188-19-git@luigi311.com>
On 28/03/2024 00:17, git@luigi311.com wrote:
> From: Dave Stevenson <dave.stevenson@raspberrypi.com>
>
> There are a number of variants of the imx258 modules that can not
> be differentiated at runtime, so add compatible strings for them.
>
> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> Signed-off-by: Luigi311 <git@luigi311.com>
> ---
> .../devicetree/bindings/media/i2c/sony,imx258.yaml | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> index bee61a443b23..c7856de15ba3 100644
> --- a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> @@ -14,10 +14,14 @@ description: |-
> type stacked image sensor with a square pixel array of size 4208 x 3120. It
> is programmable through I2C interface. Image data is sent through MIPI
> CSI-2.
> + There are a number of variants of the sensor which cannot be detected at
> + runtime, so multiple compatible strings are required to differentiate these.
>
> properties:
> compatible:
> - const: sony,imx258
> + - enum:
> + - sony,imx258
Two people working on patch but no one tested it before sending. Do not
send untested code.
It does not look like you tested the bindings, at least after quick
look. Please run `make dt_binding_check` (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).
Maybe you need to update your dtschema and yamllint.
Best regards,
Krzysztof
^ 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