Devicetree
 help / color / mirror / Atom feed
* Re: [EXT] Re: [PATCH v8 3/8] perf: imx_perf: let the driver manage the counter usage rather the user
From: Frank Li @ 2024-04-10 16:35 UTC (permalink / raw)
  To: Will Deacon
  Cc: Xu Yang, mark.rutland@arm.com, robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
	shawnguo@kernel.org, s.hauer@pengutronix.de,
	kernel@pengutronix.de, festevam@gmail.com,
	john.g.garry@oracle.com, jolsa@kernel.org, namhyung@kernel.org,
	irogers@google.com, mike.leach@linaro.org, peterz@infradead.org,
	mingo@redhat.com, acme@kernel.org,
	alexander.shishkin@linux.intel.com, adrian.hunter@intel.com,
	linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
	imx@lists.linux.dev
In-Reply-To: <20240410154546.GA25225@willie-the-truck>

On Wed, Apr 10, 2024 at 04:45:47PM +0100, Will Deacon wrote:
> On Wed, Apr 10, 2024 at 07:39:46AM +0000, Xu Yang wrote:
> > > On Fri, Mar 22, 2024 at 02:39:25PM +0800, Xu Yang wrote:
> > > > In current design, the user of perf app needs to input counter ID to count
> > > > events. However, this is not user-friendly since the user needs to lookup
> > > > the map table to find the counter. Instead of letting the user to input
> > > > the counter, let this driver to manage the counters in this patch.
> > > 
> > > I think we still have to support the old interface so that we don't break
> > > those existing users (even if the driver just ignores whatever counter ID
> > > is provided in a backwards-compatible way).
> > > 
> > > > This will be implemented by:
> > > >  1. allocate counter 0 for cycle event.
> > > >  2. find unused counter from 1-10 for reference events.
> > > >  3. allocate specific counter for counter-specific events.
> > > >
> > > > In this patch, counter attribute is removed too. To mark counter-specific
> > > > events, counter ID will be encoded into perf_pmu_events_attr.id.
> > > >
> > > > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > > > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > > >
> > > > ---
> > > > Changes in v6:
> > > >  - new patch
> > > > Changes in v7:
> > > >  - no changes
> > > > Changes in v8:
> > > >  - add Rb tag
> > > > ---
> > > >  drivers/perf/fsl_imx9_ddr_perf.c | 168 ++++++++++++++++++-------------
> > > >  1 file changed, 99 insertions(+), 69 deletions(-)
> > > >
> > > > diff --git a/drivers/perf/fsl_imx9_ddr_perf.c b/drivers/perf/fsl_imx9_ddr_perf.c
> > > > index 0017f2c9ef91..b728719b494c 100644
> > > > --- a/drivers/perf/fsl_imx9_ddr_perf.c
> > > > +++ b/drivers/perf/fsl_imx9_ddr_perf.c
> > > > @@ -245,14 +249,12 @@ static const struct attribute_group ddr_perf_events_attr_group = {
> > > >       .attrs = ddr_perf_events_attrs,
> > > >  };
> > > >
> > > > -PMU_FORMAT_ATTR(event, "config:0-7");
> > > > -PMU_FORMAT_ATTR(counter, "config:8-15");
> > > > +PMU_FORMAT_ATTR(event, "config:0-15");
> > > 
> > > Sadly, this is a user-visible change so I think it will break old tools,
> > > won't it?
> > 
> > For imx ddr pmu, most of the people will use metrics rather event itself,
> > and we have speficy the format of event parameters in metrics table.
> > The parameters is also updated in this patchset.
> > 
> > And to easy use for user, the counter should be hidden (transparent) to
> > user after I had talk with Frank. Then, the user need't to look up the event
> > table to find which counter to use. 
> > 
> > So this patchset will basically not break the usage of perf tools and will
> > improve practicality.
> 
> Sorry, but I don't agree. The original commit adding this driver
> (55691f99d417) gives the following examples in the commit message:
> 
> For example:
>       perf stat -a -I 1000 -e imx9_ddr0/eddrtq_pm_rd_trans_filt,counter=2,axi_mask=ID_MASK,axi_id=ID/
>       perf stat -a -I 1000 -e imx9_ddr0/eddrtq_pm_wr_trans_filt,counter=3,axi_mask=ID_MASK,axi_id=ID/
>       perf stat -a -I 1000 -e imx9_ddr0/eddrtq_pm_rd_beat_filt,counter=4,axi_mask=ID_MASK,axi_id=ID/
> 
> I don't think these will work any more if we apply this patch.

Yang:

    keep compatible is important. Please try well's suggestion
    "if the driver just ignores whatever counter ID is provided in a 
backwards-compatible way)." 

    If you have further question, you can ping directly. Please avoid
use outlook to reply community email. It always append annoised "EXT".

Frank

> 
> Will

^ permalink raw reply

* Re: [RESEND v7 22/37] dt-bindings: display: smi,sm501: SMI SM501 binding json-schema
From: Rob Herring @ 2024-04-10 16:33 UTC (permalink / raw)
  To: Yoshinori Sato
  Cc: linux-sh, Damien Le Moal, Niklas Cassel, Krzysztof Kozlowski,
	Conor Dooley, Geert Uytterhoeven, Michael Turquette, Stephen Boyd,
	David Airlie, Daniel Vetter, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, Thomas Gleixner, Bjorn Helgaas,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Greg Kroah-Hartman,
	Jiri Slaby, Magnus Damm, Daniel Lezcano, Rich Felker,
	John Paul Adrian Glaubitz, Lee Jones, Helge Deller,
	Heiko Stuebner, Shawn Guo, Sebastian Reichel, Chris Morgan,
	Linus Walleij, Arnd Bergmann, David Rientjes, Hyeonggon Yoo,
	Vlastimil Babka, Baoquan He, Andrew Morton, Guenter Roeck,
	Kefeng Wang, Stephen Rothwell, Javier Martinez Canillas, Guo Ren,
	Azeem Shaikh, Max Filippov, Jonathan Corbet, Jacky Huang,
	Herve Codina, Manikanta Guntupalli, Anup Patel, Biju Das,
	Uwe Kleine-König, Sam Ravnborg, Sergey Shtylyov,
	Laurent Pinchart, linux-ide, devicetree, linux-kernel,
	linux-renesas-soc, linux-clk, dri-devel, linux-pci, linux-serial,
	linux-fbdev
In-Reply-To: <9858ef1c149bd27b27594b3bd388601681d83460.1712207606.git.ysato@users.sourceforge.jp>

On Thu, Apr 04, 2024 at 02:14:33PM +0900, Yoshinori Sato wrote:
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> ---
>  .../bindings/display/smi,sm501.yaml           | 398 ++++++++++++++++++
>  1 file changed, 398 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/smi,sm501.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/smi,sm501.yaml b/Documentation/devicetree/bindings/display/smi,sm501.yaml
> new file mode 100644
> index 000000000000..06c6af4fa4a9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/smi,sm501.yaml
> @@ -0,0 +1,398 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/smi,sm501.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Silicon Motion SM501 Mobile Multimedia Companion Chip
> +
> +maintainers:
> +  - Yoshinori Sato <ysato@user.sourceforge.jp>
> +
> +description: |

Don't need '|'

> +  These DT bindings describe the SM501.

Drop "These DT bindings describe" and just describe what the h/w is.

> +
> +properties:
> +  compatible:
> +    const:
> +      smi,sm501
> +
> +  reg:
> +    maxItems: 2
> +    description: |
> +     First entry: System Configuration register
> +     Second entry: IO space (Display Controller register)

items:
  - description: System Configuration register
  - description: IO space (Display Controller register)

Is it just 1 register in each or should be "registers"?


> +
> +  interrupts:
> +    description: SM501 interrupt to the cpu should be described here.
> +
> +  mode:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    description: select a video mode
> +
> +  edid:
> +    description: |

Don't need '|'.

> +      verbatim EDID data block describing attached display.

s/verbatim/Verbatim/

> +      Data from the detailed timing descriptor will be used to
> +      program the display controller.
> +
> +  little-endian:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description: available on big endian systems, to set different foreign endian.
> +  big-endian:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description: available on little endian systems, to set different foreign endian.
> +
> +  swap-fb-endian:

All these custom properties need vendor prefix.

But really, why are so many custom properties needed? Other display 
controllers don't need so many, why does this one? Do you actually have 
users of all of them.

> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description: swap framebuffer byteorder.
> +
> +  route-crt-panel:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description: Panel output merge to CRT.
> +
> +  crt:
> +    type: object
> +    description: CRT output control
> +    properties:
> +      edid:

Huh? You already defined edid elsewhere.

> +        $ref: /schemas/types.yaml#/definitions/uint8-array
> +        description: |
> +          verbatim EDID data block describing attached display.
> +          Data from the detailed timing descriptor will be used to
> +          program the display controller.
> +
> +      smi,flags:
> +        $ref: /schemas/types.yaml#/definitions/string-array
> +        description: Display control flags.
> +        items:
> +          anyOf:
> +            - const: use-init-done
> +            - const: disable-at-exit
> +            - const: use-hwcursor
> +            - const: use-hwaccel
> +            - const: panel-no-fpen
> +            - const: panel-no-vbiasen
> +            - const: panel-inv-fpen
> +            - const: panel-inv-vbiasen
> +        maxItems: 8
> +
> +      bpp:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: Color depth
> +
> +  panel:

Isn't this just the same as 'crt'?

> +    type: object
> +    description: Panel output control
> +    properties:
> +      edid:
> +        $ref: /schemas/types.yaml#/definitions/uint8-array
> +        description: |
> +          verbatim EDID data block describing attached display.
> +          Data from the detailed timing descriptor will be used to
> +          program the display controller.
> +
> +      smi,flags:
> +        $ref: /schemas/types.yaml#/definitions/string-array
> +        description: Display control flags.
> +        items:
> +          anyOf:
> +            - const: use-init-done
> +            - const: disable-at-exit
> +            - const: use-hwcursor
> +            - const: use-hwaccel
> +            - const: panel-no-fpen
> +            - const: panel-no-vbiasen
> +            - const: panel-inv-fpen
> +            - const: panel-inv-vbiasen
> +        maxItems: 8
> +
> +      bpp:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: Color depth
> +
> +  smi,devices:
> +    $ref: /schemas/types.yaml#/definitions/string-array
> +    description: Select SM501 device functions.
> +    items:
> +      anyOf:
> +        - const: usb-host
> +        - const: usb-slave
> +        - const: ssp0
> +        - const: ssp1
> +        - const: uart0
> +        - const: uart1
> +        - const: fbaccel
> +        - const: ac97
> +        - const: i2s
> +        - const: gpio
> +    minItems: 1
> +    maxItems: 10
> +
> +  smi,mclk:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: mclk frequency.
> +
> +  smi,m1xclk:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: m1xclk frequency.

Clock stuff? Use the clock binding.

> +
> +  misc-timing:
> +    type: object
> +    description: Miscellaneous Timing register values.
> +    properties:
> +      ex:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: Extend bus holding time.
> +
> +      xc:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: Xscale clock input select.
> +
> +      usb-over-current-detect-disable:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: USB host current detection disable (Us=0).
> +
> +      usb-over-current-detect-enable:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: USB host current detection disable (Us=1).
> +
> +      sdram-clock-mode1-288mhz:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: SSM1 bit is clear.
> +
> +      sdram-clock-mode1-div:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: SSM1 bit is set.
> +
> +      sm1:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: SDRAM clock divider for PW mode 1.
> +
> +      sdram-clock-mode0-288mhz:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: SSM0 bit is clear.
> +
> +      sdram-clock-mode0-div:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: SSM0 bit is set.
> +
> +      sm0:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: SDRAM clock divider for PW mode 0.
> +
> +      pll-debug-input:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: 96MHz PLL debug input reference frequency (Deb=0).
> +
> +      pll-debug-output:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: 96MHz PLL debug output frequency (Deb=1).
> +
> +      no-acpi-control:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: No ACPI control (A=0).
> +
> +      acpi-control:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: No ACPI control (A=1).
> +
> +      divider:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: Second PLL output frequency.
> +
> +      usb-host-normal:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: USB Host normal mode.
> +
> +      usb-host-simulation:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: USB Host simulation mode.
> +
> +      delay:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: Delay time to latch read data. Set the value to 10x.
> +
> +  misc-control:
> +    type: object
> +    description: Miscellaneous Control register values.
> +    properties:
> +      pad:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: PCI Pad drive strength.
> +
> +      usbclk:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: USB Clcok Select.
> +
> +      uart1:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: UART1 (SSP=0)
> +
> +      ssp1:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: SSP1 (SSP=1)
> +
> +      latch-address-disable:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: 8051 Latch disable (Lat=0).
> +
> +      latch-address-enable:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: 8051 Latch enable (Lat=1).
> +
> +      panel-data-18bit:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: Flat Panel data 18bit (FP=0).
> +
> +      panel-data-24bit:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: Flat Panel data 24bit (FP=1).
> +

> +      xtal-freq-24mhz:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: Crystal frequency 24MHz (Freq=0).
> +
> +      xtal-freq-12mhz:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: Crystal frequency 12MHz (Freq=1).

What's the relationship between these 2 properties? What if neither is 
present? What if both are? Define properties such that you can't have 
invalid combinations. Yes, we could just handle that with constraints, 
but why start with a bad design. There's other cases of this same 
pattern here.


> +
> +      refresh:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: Internal memory refresh timing.
> +
> +      hold:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: BUS Hold time.
> +
> +      sh-ready-low:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: SuperH ready polarity active low (SH=0).
> +
> +      sh-ready-high:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: SuperH ready polarity active high (SH=1).
> +
> +      interrupt-normal:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: Interrupt normal (II=0).
> +
> +      interrupt-inverted:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: Interrupt Inverting (II=1).
> +
> +      pll-clock-count-disable:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: PLL clock count disable.
> +
> +      pll-clock-count-enaable:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: PLL clock count enable.
> +
> +      dac-power-enable:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: DAC Power enable (DAC=0).
> +
> +      dac-power-disable:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: DAC Power disable (DAC=1).
> +
> +      usb-slave-cpu:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: USB slave controller cpu (MC=0).
> +
> +      usb-slave-8051:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: USB slave controller 8051MCU (MC=1).
> +
> +      burst-length-8:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: CPU Master burst length 8 (BL=0).
> +
> +      burst-length-1:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: CPU Master burst length 1 (BL=1).
> +
> +      usb-port-master:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: USB port master.
> +
> +      usb-port-slave:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: USB port slave.
> +
> +      vr-mmio-30mb:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: NEC VR Memory map MMIO locatedat 30MB (VR=0)
> +
> +      vr-mmio-62mb:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: NEC VR Memory map MMIO locatedat 62MB (VR=1)
> +
> +  gpio-pin-control:
> +    type: object
> +    description: GPIO control configuration.
> +    properties:
> +      pin:
> +        type: object
> +        properties:
> +          gpio:

'gpio' is already in use as a property name.

> +            $ref: /schemas/types.yaml#/definitions/flag
> +            description: pin in/out use GPIO.
> +          function:
> +            $ref: /schemas/types.yaml#/definitions/flag
> +            description: pin in/out use function.

Why do you need 2 nodes and 2 properties to define 3 possible states? 
There is not present, 'gpio', or 'function'. That's a single 
tri-state property. What does not present mean?

> +
> +  gpio-i2c:
> +    type: object
> +    description: GPIO I2C definition.
> +    properties:
> +      i2c:
> +        type: object
> +        properties:
> +          bus:
> +            $ref: /schemas/types.yaml#/definitions/uint32
> +            description: I2C bus number.

How is bus number a property of the h/w?

> +
> +          sda:
> +            $ref: /schemas/types.yaml#/definitions/uint32
> +            description: I2C SDA pin port number.
> +
> +          scl:
> +            $ref: /schemas/types.yaml#/definitions/uint32
> +            description: I2C SCL pin port number.
> +
> +          delay:
> +            $ref: /schemas/types.yaml#/definitions/uint32
> +            description: bit transmission delay.
> +
> +          timeout:
> +            $ref: /schemas/types.yaml#/definitions/uint32
> +            description: transmission timeout.
> +
> +additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +
> +examples:
> +  # MPC5200
> +  - |
> +    display@1,0 {

Not a correct unit address.

> +        compatible = "smi,sm501";
> +        reg = <0x00000000 0x00800000
> +               0x03e00000 0x00200000>;
> +        interrupts = <1 1 3>;
> +        mode = "640x480-32@60";
> +        edid = [00 ff ff ff ff ff ff 00 00 00 00 00 00 00 00 00
> +                00 00 01 04 00 00 00 00 00 00 00 00 00 00 00 00
> +                00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> +                00 00 00 00 00 00 f0 0a 80 fb 20 e0 25 10 32 60
> +                02 00 00 00 00 00 00 06 00 00 00 00 00 00 00 00
> +                00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> +                00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> +                00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 bd];

Kind of a sparse example. Please make examples using optional 
properties.

> +    };
> -- 
> 2.39.2
> 

^ permalink raw reply

* Re: [PATCH] media: dt-bindings: media: add access-controllers to STM32MP25 video codecs
From: Rob Herring @ 2024-04-10 16:31 UTC (permalink / raw)
  To: Hugues Fruchet
  Cc: Rob Herring, linux-media, Mauro Carvalho Chehab, linux-arm-kernel,
	devicetree, linux-stm32, linux-kernel, Maxime Coquelin,
	Krzysztof Kozlowski, Alexandre Torgue
In-Reply-To: <20240410144222.714172-1-hugues.fruchet@foss.st.com>


On Wed, 10 Apr 2024 16:42:22 +0200, Hugues Fruchet wrote:
> access-controllers is an optional property that allows a peripheral to
> refer to one or more domain access controller(s).
> 
> Signed-off-by: Hugues Fruchet <hugues.fruchet@foss.st.com>
> ---
>  .../devicetree/bindings/media/st,stm32mp25-video-codec.yaml   | 4 ++++
>  1 file changed, 4 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/st,stm32mp25-video-codec.yaml: access-controllers: missing type definition

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240410144222.714172-1-hugues.fruchet@foss.st.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.


^ permalink raw reply

* Re: [PATCH v2 5/7] dt-bindings: phy: qcom,ipq8074-qmp-pcie: add ipq9574 gen3x2 PHY
From: mr.nuke.me @ 2024-04-10 16:29 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Bjorn Andersson, Konrad Dybcio, Vinod Koul,
	Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel
In-Reply-To: <a23adb9c-6377-467b-ac3c-0ec51fc97253@linaro.org>



On 4/10/24 02:02, Krzysztof Kozlowski wrote:
> On 10/04/2024 08:59, Krzysztof Kozlowski wrote:
>> On 09/04/2024 22:19, mr.nuke.me@gmail.com wrote:
>>>>
>>>>
>>>>>    
>>>>>      clock-names:
>>>>>        items:
>>>>>          - const: aux
>>>>>          - const: cfg_ahb
>>>>>          - const: pipe
>>>>> +      - const: anoc
>>>>> +      - const: snoc
>>>>
>>>> OK, you did not test it. Neither this, nor DTS. I stop review, please
>>>> test first.
>>>
>>> I ran both `checkpatch.pl` and `make dt_binding_check`. What in this
>>> patch makes you say I "did not test it", and what test or tests did I miss?
>>>
>>
>> ... and no, you did not. If you tested, you would easily see error:
>> 	clock-names: ['aux', 'cfg_ahb', 'pipe'] is too short
>>
>> When you receive comment from reviewer, please investigate thoroughly
>> what could get wrong. Don't answer just to get rid of reviewer. It's
>> fine to make mistakes, but if reviewer points to issue and you
>> immediately respond "no issue", that's waste of my time.
> 
> To clarify: "no issue" response is waste of my time. If you responded
> "oh, I see the error, but I don't know how to fix it", it would be ok, I
> can clarify and help in this.

I apologize if I gave you this impression. I tried to follow the testing 
process, it did not turn out as expected. Obviously, I missed something. 
I tried to ask what I missed, and in order for that question to make 
sense, I need to describe what I tried.

It turns out what I missed was "make check_dtbs". I only found that out 
after an automated email from Rob describing some troubleshooting steps.

If I may have a few sentences to rant, I see the dt-schema as a hurdle 
to making an otherwise useful change. I am told I can ask for help when 
I get stuck, yet I manage to insult the maintainer by aking for help. I 
find this very intimidating.

Alex

^ permalink raw reply

* Re: [PATCH v3 1/2] dt-bindings: dma: fsl-edma: remove 'clocks' from required
From: Frank Li @ 2024-04-10 16:24 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: conor+dt, devicetree, dmaengine, imx, krzysztof.kozlowski+dt,
	linux-kernel, peng.fan, robh, vkoul,
	20240409185416.2224609-1-Frank.Li, Xu Yang, Shengjiu Wang,
	Pankaj Gupta, Shenwei Wang
In-Reply-To: <6db0c7fc-a94f-4f02-840a-1d64d6e2daa0@kernel.org>

On Wed, Apr 10, 2024 at 08:47:00AM +0200, Krzysztof Kozlowski wrote:
> On 10/04/2024 08:32, Krzysztof Kozlowski wrote:
> > On 10/04/2024 08:30, Krzysztof Kozlowski wrote:
> >> On 09/04/2024 23:09, Frank Li wrote:
> >>> On Tue, Apr 09, 2024 at 10:02:32PM +0200, Krzysztof Kozlowski wrote:
> >>>> On 09/04/2024 20:54, Frank Li wrote:
> >>>>> fsl,imx8qm-adma and fsl,imx8qm-edma don't require 'clocks'. Remove it from
> >>>>> required and add 'if' block for other compatible string to keep the same
> >>>>> restrictions.
> >>>>>
> >>>>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >>>>> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> >>>>> ---
> >>>>>
> >>>>> Notes:
> >>>>>     Change from v2 to v3
> >>>>>       - rebase to dmaengine/next
> >>>>
> >>>> This fails...
> >>>
> >>> What's wrong? 
> >>>
> >>> https://git.kernel.org/pub/scm/linux/kernel/git/vkoul/dmaengine.git/log/?h=next
> >>>
> >>>>
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/dma/fsl,edma.yaml b/Documentation/devicetree/bindings/dma/fsl,edma.yaml
> >>>>> index 825f4715499e5..657a7d3ebf857 100644
> >>>>> --- a/Documentation/devicetree/bindings/dma/fsl,edma.yaml
> >>>>> +++ b/Documentation/devicetree/bindings/dma/fsl,edma.yaml
> >>>>> @@ -82,7 +82,6 @@ required:
> >>>>>    - compatible
> >>>>>    - reg
> >>>>>    - interrupts
> >>>>> -  - clocks
> >>>>>    - dma-channels
> >>>>>  
> >>>>>  allOf:
> >>>>> @@ -187,6 +186,22 @@ allOf:
> >>>>>          "#dma-cells":
> >>>>>            const: 3
> >>>>>  
> >>>>> +  - if:
> >>>>> +      properties:
> >>>>> +        compatible:
> >>>>> +	  contains:
> >>>>
> >>>> 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.
> >>>
> >>> Strange, Test passed
> >>>
> >>> make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- -j8  dt_binding_check DT_SCHEMA_FILES=fsl,edma.yaml
> >>>   LINT    Documentation/devicetree/bindings
> >>>   DTEX    Documentation/devicetree/bindings/dma/fsl,edma.example.dts
> >>>   CHKDT   Documentation/devicetree/bindings/processed-schema.json
> >>>   SCHEMA  Documentation/devicetree/bindings/processed-schema.json
> >>>   DTC_CHK Documentation/devicetree/bindings/dma/fsl,edma.example.dtb
> >>
> >> Nope, you tested other patch. Just look at your second patch for this.
> >> When reviewer points to errors to your code, please investigate?
> >>
> >> NAK, fix your patches.
> > 
> > And to prove it, so you will stop wasting my time:
> > ../Documentation/devicetree/bindings/dma/fsl,edma.yaml:192:1: found
> > character that cannot start any token
> > 
> > ../Documentation/devicetree/bindings/dma/fsl,edma.yaml:192:1: [error]
> > syntax error: found character '\t' that cannot start any token (syntax)
> > 
> > ../Documentation/devicetree/bindings/dma/fsl,edma.yaml:192:1: found
> > character that cannot start any token
> > 
> > Documentation/devicetree/bindings/dma/fsl,edma.yaml: ignoring, error
> > parsing file
> 
> Dear NXP,
> 
> Quality of patches from NXP is terrible. Several of them are poorly
> coded, not following coding style, their submission is not following the
> process and requires a lot of effort from reviewers. I was already
> complaining about this on mailing lists months ago.
> 
> Things did not improve much.

I understand what's your concern. I just said I run dt_binding_check before
submit patch. Never said I will not take time to inversitage it. Actually
I found the problem yesterday and will fix next version. I also find what's
wrong why success in my side.

I also read many of your email about other thread in dt-binding. Most is
the similar question.

The words in your all email "It does not look like you tested the bindings"
is most frequency words regardless from nxp.com or NOT.

Yaml file look like as document, but actually NOT. Unlike C code, which
can compile and running.  And more people get well trainning at debug C
driver. Actually it takes more time handle binding yaml doc than debug
driver code. Some item is actually quite difficult to understand without
deep into json scheme. It is not simple as what look like. It may cause
many low level error because it it not simple as what look like and even
hard.

The whole reviewer team take great efforts to make dt-binding better. 

I think it should stop complain and start improve tools. 

For example: 

I try to add --check in b4.
https://lore.kernel.org/tools/20240319045332.2304950-1-Frank.Li@nxp.com/T/#t

Some self-auto-bot help filter low lever problem.

And from another point, it means more and more NXP engineer want to take
more time and efforts to do upstream works.

> 
> However another trouble is the quality of responses during review. In
> many patchsets your responses to reviewers comments were half-baked, not
> on actual topic or just with minimal effort to close the topic from your
> side. That's not how it works.

Some background is difference. There are many argument spaces at some side.
'minimal effort' is not exist. I bet the email reply to community is 10x
carefull more than other internal email.

> 
> If you receive comment, you must investigate. Don't respond immediately
> "no, I don't see error" or "but I want something else", but be sure that
> you fixed the problem.

My intention "strange, test pass" just said my suprised "why it success in
my side" and "will check tools and my environment", Not "skip run
dt-binding" before submit. But what your understand is , "my comment is
wrong, and you stop inverstiage it and close this topic".
  
> 
> Such responses of minimal effort or pushing your own patch is
> significant effort on reviewers side. I was complaining about this as
> well. This patch here, which does not even build/test yet you claim in
> response that you test, is perfect example of it. You got comment from
> reviewer and instead really investigating this, you respond that
> everything is good on your side. Typical response with minimal effort on
> your side, but pushing it to the community.
> 
> That's it, that's too much.
> 
> NXP, your contributions are poor quality and put too much effort on
> community.

Many time, I want to help filter out some low level problem for nxp patche.
A long-term illness makes a good doctor. Your team work are too efficency.
before I can do anything, you already provide professional feedback.

> 
> Please improve your process, e.g. by training people interacting with
> community and using extensive internal review. You can also reach to
> experienced community members for help in training and explaining
> upstream work, like Denx, Pengutronix, Bootlin, Linaro, Baylibre,
> Collabora and others.

If you can help do trainning for us, weclome!!

> 
> Till the situation improves, I will be ignoring all patches from @nxp.com.
> 
> Best regards,
> Krzysztof
> 

^ permalink raw reply

* Re: [RESEND v7 17/37] dt-bindings: interrupt-controller: renesas,sh7751-intc: Add json-schema
From: Rob Herring @ 2024-04-10 16:14 UTC (permalink / raw)
  To: Yoshinori Sato
  Cc: Thomas Gleixner, Krzysztof Wilczyński, Daniel Lezcano,
	Azeem Shaikh, Sebastian Reichel, Laurent Pinchart, linux-ide,
	Hyeonggon Yoo, linux-renesas-soc, Helge Deller, linux-kernel,
	linux-fbdev, Stephen Boyd, Stephen Rothwell, Geert Uytterhoeven,
	David Rientjes, Baoquan He, Andrew Morton, Jiri Slaby,
	Conor Dooley, Vlastimil Babka, Herve Codina, devicetree, linux-sh,
	Lee Jones, Kefeng Wang, Manikanta Guntupalli,
	John Paul Adrian Glaubitz, Max Filippov, Biju Das, Guo Ren,
	Magnus Damm, David Airlie, Greg Kroah-Hartman, Niklas Cassel,
	Javier Martinez Canillas, Linus Walleij, linux-pci,
	Sergey Shtylyov, Heiko Stuebner, Daniel Vetter, dri-devel,
	Thomas Zimmermann, Anup Patel, Damien Le Moal, Jacky Huang,
	linux-serial, Chris Morgan, Guenter Roeck, Lorenzo Pieralisi,
	linux-clk, Sam Ravnborg, Maarten Lankhorst, Michael Turquette,
	Bjorn Helgaas, Rich Felker, Arnd Bergmann, Maxime Ripard,
	Jonathan Corbet, Uwe Kleine-König, Krzysztof Kozlowski,
	Shawn Guo
In-Reply-To: <cd7aae0800d9fc97f4d265c34ad4ac8c19dfd8f3.1712207606.git.ysato@users.sourceforge.jp>


On Thu, 04 Apr 2024 14:14:28 +0900, Yoshinori Sato wrote:
> Renesas SH7751 INTC json-schema.
> 
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> ---
>  .../renesas,sh7751-intc.yaml                  | 53 +++++++++++++++++++
>  1 file changed, 53 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,sh7751-intc.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>


^ permalink raw reply

* Re: [RESEND v7 13/37] dt-bindings: clock: sh7750-cpg: Add renesas,sh7750-cpg header.
From: Rob Herring @ 2024-04-10 16:13 UTC (permalink / raw)
  To: Yoshinori Sato
  Cc: linux-sh, Damien Le Moal, Niklas Cassel, Krzysztof Kozlowski,
	Conor Dooley, Geert Uytterhoeven, Michael Turquette, Stephen Boyd,
	David Airlie, Daniel Vetter, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, Thomas Gleixner, Bjorn Helgaas,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Greg Kroah-Hartman,
	Jiri Slaby, Magnus Damm, Daniel Lezcano, Rich Felker,
	John Paul Adrian Glaubitz, Lee Jones, Helge Deller,
	Heiko Stuebner, Shawn Guo, Sebastian Reichel, Chris Morgan,
	Linus Walleij, Arnd Bergmann, David Rientjes, Hyeonggon Yoo,
	Vlastimil Babka, Baoquan He, Andrew Morton, Guenter Roeck,
	Kefeng Wang, Stephen Rothwell, Javier Martinez Canillas, Guo Ren,
	Azeem Shaikh, Max Filippov, Jonathan Corbet, Jacky Huang,
	Herve Codina, Manikanta Guntupalli, Anup Patel, Biju Das,
	Uwe Kleine-König, Sam Ravnborg, Sergey Shtylyov,
	Laurent Pinchart, linux-ide, devicetree, linux-kernel,
	linux-renesas-soc, linux-clk, dri-devel, linux-pci, linux-serial,
	linux-fbdev
In-Reply-To: <1db8627e4ca50b9d2d27e95d245518cac1cd62dc.1712207606.git.ysato@users.sourceforge.jp>

On Thu, Apr 04, 2024 at 02:14:24PM +0900, Yoshinori Sato wrote:
> SH7750 CPG Clock output define.

This and the subject don't match what the patch does.

> 
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> ---
>  .../bindings/clock/renesas,sh7750-cpg.yaml    | 105 ++++++++++++++++++
>  include/dt-bindings/clock/sh7750-cpg.h        |  26 +++++
>  2 files changed, 131 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/renesas,sh7750-cpg.yaml
>  create mode 100644 include/dt-bindings/clock/sh7750-cpg.h
> 
> diff --git a/Documentation/devicetree/bindings/clock/renesas,sh7750-cpg.yaml b/Documentation/devicetree/bindings/clock/renesas,sh7750-cpg.yaml
> new file mode 100644
> index 000000000000..04c10b0834ee
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/renesas,sh7750-cpg.yaml
> @@ -0,0 +1,105 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/renesas,sh7750-cpg.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas SH7750/7751 Clock Pulse Generator (CPG)
> +
> +maintainers:
> +  - Yoshinori Sato <ysato@users.sourceforge.jp>
> +
> +description:
> +  The Clock Pulse Generator (CPG) generates core clocks for the SoC.  It
> +  includes PLLs, and variable ratio dividers.
> +
> +  The CPG may also provide a Clock Domain for SoC devices, in combination with
> +  the CPG Module Stop (MSTP) Clocks.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - renesas,sh7750-cpg             # SH7750
> +      - renesas,sh7750s-cpg            # SH775S
> +      - renesas,sh7750r-cpg            # SH7750R
> +      - renesas,sh7751-cpg             # SH7751
> +      - renesas,sh7751r-cpg            # SH7751R
> +
> +  reg: true
> +
> +  reg-names: true
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    const: extal
> +
> +  '#clock-cells':
> +    const: 1
> +
> +  renesas,mode:
> +    description: Board-specific settings of the MD[0-2] pins on SoC
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 6
> +
> +  '#power-domain-cells':
> +    const: 0
> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - clocks
> +  - clock-names
> +  - '#clock-cells'
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - renesas,sh7750-cpg
> +              - renesas,sh7750s-cpg
> +    then:
> +      properties:
> +        reg:
> +          maxItems: 1
> +        reg-names:
> +          items:
> +            - const: FRQCR
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - renesas,sh7750r-cpg
> +              - renesas,sh7751-cpg
> +              - renesas,sh7751r-cpg
> +    then:
> +      properties:
> +        reg:
> +          maxItems: 2

minItems: 2 (instead)

> +        reg-names:
> +          items:
> +            - const: FRQCR
> +            - const: CLKSTP00

Move this to the top-level and add 'minItems: 1'. Then above you just 
need 'maxItems: 1' and here 'minItems: 2'.


> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/sh7750-cpg.h>
> +    cpg: clock-controller@ffc00000 {
> +        #clock-cells = <1>;
> +        #power-domain-cells = <0>;
> +        compatible = "renesas,sh7751r-cpg";
> +        clocks = <&extal>;
> +        clock-names = "extal";
> +        reg = <0xffc00000 20>, <0xfe0a0000 16>;
> +        reg-names = "FRQCR", "CLKSTP00";
> +        renesas,mode = <0>;
> +    };
> diff --git a/include/dt-bindings/clock/sh7750-cpg.h b/include/dt-bindings/clock/sh7750-cpg.h
> new file mode 100644
> index 000000000000..ec267be91adf
> --- /dev/null
> +++ b/include/dt-bindings/clock/sh7750-cpg.h
> @@ -0,0 +1,26 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> + *
> + * Copyright 2023 Yoshinori Sato
> + */
> +
> +#ifndef __DT_BINDINGS_CLOCK_SH7750_H__
> +#define __DT_BINDINGS_CLOCK_SH7750_H__
> +
> +#define SH7750_CPG_PLLOUT	0
> +
> +#define SH7750_CPG_PCK		1
> +#define SH7750_CPG_BCK		2
> +#define SH7750_CPG_ICK		3
> +
> +#define SH7750_MSTP_SCI		4
> +#define SH7750_MSTP_RTC		5
> +#define SH7750_MSTP_TMU012	6
> +#define SH7750_MSTP_SCIF	7
> +#define SH7750_MSTP_DMAC	8
> +#define SH7750_MSTP_UBC		9
> +#define SH7750_MSTP_SQ		10
> +#define SH7750_CSTP_INTC	11
> +#define SH7750_CSTP_TMU34	12
> +#define SH7750_CSTP_PCIC	13
> +
> +#endif
> -- 
> 2.39.2
> 

^ permalink raw reply

* Re: [PATCH v7 16/16] PCI/pwrctl: add a PCI power control driver for power sequenced devices
From: Jeff Johnson @ 2024-04-10 16:05 UTC (permalink / raw)
  To: Bartosz Golaszewski, Marcel Holtmann, Luiz Augusto von Dentz,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Kalle Valo,
	Bjorn Andersson, Konrad Dybcio, Liam Girdwood, Mark Brown,
	Catalin Marinas, Will Deacon, Bjorn Helgaas, Saravana Kannan,
	Geert Uytterhoeven, Arnd Bergmann, Neil Armstrong,
	Marek Szyprowski, Alex Elder, Srini Kandagatla,
	Greg Kroah-Hartman, Abel Vesa, Manivannan Sadhasivam,
	Lukas Wunner, Dmitry Baryshkov, Amit Pundir, Xilin Wu
  Cc: linux-bluetooth, netdev, devicetree, linux-kernel, linux-wireless,
	linux-arm-msm, linux-arm-kernel, linux-pci, linux-pm,
	Bartosz Golaszewski
In-Reply-To: <20240410124628.171783-17-brgl@bgdev.pl>

On 4/10/2024 5:46 AM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> Add a PCI power control driver that's capable of correctly powering up
> devices using the power sequencing subsystem. The first user of this
> driver is the ath11k module on QCA6390.
[...]
> +config PCI_PWRCTL_PWRSEQ
> +	tristate "PCI Power Control driver using the Power Sequencing subsystem"
> +	select POWER_SEQUENCING
> +	select PCI_PWRCTL
> +	default m if (ATH11K_PCI && ARCH_QCOM)
[...]
> +static const struct of_device_id pci_pwrctl_pwrseq_of_match[] = {
> +	{
> +		/* ATH11K in QCA6390 package. */
> +		.compatible = "pci17cb,1101",
> +		.data = "wlan",
> +	},
> +	{
> +		/* ATH12K in WCN7850 package. */
> +		.compatible = "pci17cb,1107",
> +		.data = "wlan",

since you are adding both ath11k and ath12k packages, should you update the
commit text and the config "default m if" condition to include ath12k?

/jeff

^ permalink raw reply

* Re: [PATCH 2/2] media: i2c: Add GT97xx VCM driver
From: Andy Shevchenko @ 2024-04-10 16:00 UTC (permalink / raw)
  To: Zhi Mao
  Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Matthias Brugger, AngeloGioacchino Del Regno,
	Philipp Zabel, Laurent Pinchart, Heiko Stuebner, Sakari Ailus,
	Hans Verkuil, Hans de Goede, Tomi Valkeinen, Alain Volmat,
	Paul Elder, Mehdi Djait, Bingbu Cao, linux-media, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek, shengnan.wang,
	yaya.chang, yunkec, 10572168
In-Reply-To: <20240410104002.1197-3-zhi.mao@mediatek.com>

On Wed, Apr 10, 2024 at 1:40 PM Zhi Mao <zhi.mao@mediatek.com> wrote:
>
> Add a V4L2 sub-device driver for Giantec GT97xx VCM.

...

> +/*
> + * Ring control and Power control register
> + * Bit[1] RING_EN
> + * 0: Direct mode
> + * 1: AAC mode (ringing control mode)
> + * Bit[0] PD
> + * 0: Normal operation mode
> + * 1: Power down mode
> + * gt97xx requires waiting time of Topr after PD reset takes place.
> + */
> +#define GT97XX_RING_PD_CONTROL_REG CCI_REG8(0x02)

> +#define GT97XX_PD_MODE_OFF 0x00

Okay, this seems to be bitmapped, but do you really need to have this
separate definition?

> +#define GT97XX_PD_MODE_EN BIT(0)
> +#define GT97XX_AAC_MODE_EN BIT(1)

...

> +#define GT97XX_TVIB_MS_BASE10 (64 - 1)

Should it be (BIT(6) - 1) ?

...

> +#define GT97XX_AAC_MODE_DEFAULT 2
> +#define GT97XX_AAC_TIME_DEFAULT 0x20

Some are decimal, some are hexadecimal, but look to me like bitfields.

...

> +#define GT97XX_MAX_FOCUS_POS (1024 - 1)

(BIT(10) - 1) ?

...

> +enum vcm_giantec_reg_desc {
> +       GT_IC_INFO_REG,
> +       GT_RING_PD_CONTROL_REG,
> +       GT_MSB_ADDR_REG,
> +       GT_AAC_PRESC_REG,
> +       GT_AAC_TIME_REG,

> +       GT_MAX_REG,

No comma for the terminator.

> +};

...

> +static u32 gt97xx_find_ot_multi(u32 aac_mode_param)
> +{
> +       u32 cur_ot_multi_base100 = 70;
> +       unsigned int i;
> +
> +       for (i = 0; i < ARRAY_SIZE(aac_mode_ot_multi); i++) {
> +               if (aac_mode_ot_multi[i].aac_mode_enum == aac_mode_param) {
> +                       cur_ot_multi_base100 =
> +                               aac_mode_ot_multi[i].ot_multi_base100;
> +               }

No break / return here?

> +       }
> +
> +       return cur_ot_multi_base100;
> +}
> +
> +static u32 gt97xx_find_dividing_rate(u32 presc_param)

Same comments as per above function.

...

> +static inline u32 gt97xx_cal_move_delay(u32 aac_mode_param, u32 presc_param,
> +                                       u32 aac_timing_param)

Why inline?

...

> +       return tvib_us * ot_multi_base100 / 100;

HECTO ?

...

> +       val = ((unsigned char)read_val & ~mask) | (val & mask);

Why casting?

...

> +static int gt97xx_power_on(struct device *dev)
> +{
> +       struct v4l2_subdev *sd = dev_get_drvdata(dev);
> +       struct gt97xx *gt97xx = sd_to_gt97xx(sd);
> +       int ret;
> +
> +       ret = regulator_bulk_enable(ARRAY_SIZE(gt97xx_supply_names),
> +                                   gt97xx->supplies);
> +       if (ret < 0) {
> +               dev_err(dev, "failed to enable regulators\n");

> +               return ret;

Dup.

> +       }
> +
> +       return ret;
> +}
> +
> +static int gt97xx_power_off(struct device *dev)
> +{
> +       struct v4l2_subdev *sd = dev_get_drvdata(dev);
> +       struct gt97xx *gt97xx = sd_to_gt97xx(sd);
> +       int ret;
> +
> +       ret = regulator_bulk_disable(ARRAY_SIZE(gt97xx_supply_names),
> +                                    gt97xx->supplies);
> +       if (ret < 0) {
> +               dev_err(dev, "failed to disable regulators\n");

> +               return ret;

Ditto.

> +       }
> +
> +       return ret;
> +}

...

> +static int gt97xx_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> +{
> +       return pm_runtime_resume_and_get(sd->dev);
> +}
> +
> +static int gt97xx_close(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> +{
> +       return pm_runtime_put(sd->dev);
> +}

Hmm... Shouldn't v4l2 take care about these (PM calls)?

...

> +       gt97xx->chip = of_device_get_match_data(dev);

device_get_match_data()

...

> +       fwnode_property_read_u32(dev_fwnode(dev), "giantec,aac-mode",
> +                                &gt97xx->aac_mode);

No, use device_property_read_u32() directly.

...

> +       fwnode_property_read_u32(dev_fwnode(dev), "giantec,clock-presc",
> +                                &gt97xx->clock_presc);

Ditto.

...

> +       fwnode_property_read_u32(dev_fwnode(dev), "giantec,aac-timing",
> +                                &gt97xx->aac_timing);

Ditto.

...

> +       /*power on and Initialize hw*/

Missing spaces (and capitalisation?).

...

> +       ret = gt97xx_runtime_resume(dev);
> +       if (ret < 0) {
> +               dev_err(dev, "failed to power on: %d\n", ret);

Use dev_err_probe() to match the style of the messages.

> +               goto err_clean_entity;
> +       }

...

> +       ret = v4l2_async_register_subdev(&gt97xx->sd);
> +       if (ret < 0) {
> +               dev_err(dev, "failed to register V4L2 subdev: %d", ret);

Ditto.

> +               goto err_power_off;
> +       }

--
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH v2 4/4] dt-bindings: rtc: stmp3xxx-rtc: convert to dtschema
From: Krzysztof Kozlowski @ 2024-04-10 15:58 UTC (permalink / raw)
  To: Javier Carrasco, Alexandre Belloni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Jiaxun Yang,
	Vladimir Zapolskiy, Joel Stanley, Andrew Jeffery, Maxime Coquelin,
	Alexandre Torgue
  Cc: linux-rtc, devicetree, linux-kernel, linux-arm-kernel,
	linux-aspeed, linux-stm32
In-Reply-To: <20240410-rtc_dtschema-v2-4-d32a11ab0745@gmail.com>

On 10/04/2024 17:55, Javier Carrasco wrote:
> Convert existing binding to dtschema to support validation and
> add the undocumented compatible 'fsl,imx23-rtc'.
> 
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> ---
>  .../devicetree/bindings/rtc/fsl,stmp3xxx-rtc.yaml  | 51 ++++++++++++++++++++++
>  .../devicetree/bindings/rtc/stmp3xxx-rtc.txt       | 21 ---------
>  2 files changed, 51 insertions(+), 21 deletions(-)
> 

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH v7 14/16] power: pwrseq: add a driver for the PMU module on the QCom WCN chipsets
From: Jeff Johnson @ 2024-04-10 15:57 UTC (permalink / raw)
  To: Bartosz Golaszewski, Marcel Holtmann, Luiz Augusto von Dentz,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Kalle Valo,
	Bjorn Andersson, Konrad Dybcio, Liam Girdwood, Mark Brown,
	Catalin Marinas, Will Deacon, Bjorn Helgaas, Saravana Kannan,
	Geert Uytterhoeven, Arnd Bergmann, Neil Armstrong,
	Marek Szyprowski, Alex Elder, Srini Kandagatla,
	Greg Kroah-Hartman, Abel Vesa, Manivannan Sadhasivam,
	Lukas Wunner, Dmitry Baryshkov, Amit Pundir, Xilin Wu
  Cc: linux-bluetooth, netdev, devicetree, linux-kernel, linux-wireless,
	linux-arm-msm, linux-arm-kernel, linux-pci, linux-pm,
	Bartosz Golaszewski
In-Reply-To: <20240410124628.171783-15-brgl@bgdev.pl>

On 4/10/2024 5:46 AM, Bartosz Golaszewski wrote:
[...]
> +if POWER_SEQUENCING
> +
> +config POWER_SEQUENCING_QCOM_WCN
> +	tristate "Qualcomm WCN family PMU driver"
> +	default m if ARCH_QCOM
> +	help
> +	  Say U here to enable the power sequencing driver for Qualcomm

did you mean: Say Y here?

> +	  WCN Bluetooth/WLAN chipsets.


^ permalink raw reply

* Re: [PATCH v2 2/4] dt-bindings: rtc: lpc32xx-rtc: convert to dtschema
From: Krzysztof Kozlowski @ 2024-04-10 15:57 UTC (permalink / raw)
  To: Javier Carrasco, Alexandre Belloni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Jiaxun Yang,
	Vladimir Zapolskiy, Joel Stanley, Andrew Jeffery, Maxime Coquelin,
	Alexandre Torgue
  Cc: linux-rtc, devicetree, linux-kernel, linux-arm-kernel,
	linux-aspeed, linux-stm32
In-Reply-To: <20240410-rtc_dtschema-v2-2-d32a11ab0745@gmail.com>

On 10/04/2024 17:55, Javier Carrasco wrote:
> Convert existing binding to dtschema to support validation.
> 
> Add the undocumented 'clocks' property.
> 
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> ---
>  .../devicetree/bindings/rtc/lpc32xx-rtc.txt        | 15 --------
>  .../devicetree/bindings/rtc/nxp,lpc32xx-rtc.yaml   | 41 ++++++++++++++++++++++
>  2 files changed, 41 insertions(+), 15 deletions(-)


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof


^ permalink raw reply

* [PATCH v2 4/4] dt-bindings: rtc: stmp3xxx-rtc: convert to dtschema
From: Javier Carrasco @ 2024-04-10 15:55 UTC (permalink / raw)
  To: Alexandre Belloni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jiaxun Yang, Vladimir Zapolskiy, Joel Stanley, Andrew Jeffery,
	Maxime Coquelin, Alexandre Torgue
  Cc: linux-rtc, devicetree, linux-kernel, linux-arm-kernel,
	linux-aspeed, linux-stm32, Javier Carrasco
In-Reply-To: <20240410-rtc_dtschema-v2-0-d32a11ab0745@gmail.com>

Convert existing binding to dtschema to support validation and
add the undocumented compatible 'fsl,imx23-rtc'.

Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
 .../devicetree/bindings/rtc/fsl,stmp3xxx-rtc.yaml  | 51 ++++++++++++++++++++++
 .../devicetree/bindings/rtc/stmp3xxx-rtc.txt       | 21 ---------
 2 files changed, 51 insertions(+), 21 deletions(-)

diff --git a/Documentation/devicetree/bindings/rtc/fsl,stmp3xxx-rtc.yaml b/Documentation/devicetree/bindings/rtc/fsl,stmp3xxx-rtc.yaml
new file mode 100644
index 000000000000..534de4196a4f
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/fsl,stmp3xxx-rtc.yaml
@@ -0,0 +1,51 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/fsl,stmp3xxx-rtc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: STMP3xxx/i.MX28 Time Clock Controller
+
+maintainers:
+  - Javier Carrasco <javier.carrasco.cruz@gmail.com>
+
+allOf:
+  - $ref: rtc.yaml#
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - enum:
+              - fsl,imx28-rtc
+              - fsl,imx23-rtc
+          - const: fsl,stmp3xxx-rtc
+      - const: fsl,stmp3xxx-rtc
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  stmp,crystal-freq:
+    description:
+      Override crystal frequency as determined from fuse bits.
+      Use <0> for "no crystal".
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [0, 32000, 32768]
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    rtc@80056000 {
+        compatible = "fsl,imx28-rtc", "fsl,stmp3xxx-rtc";
+        reg = <0x80056000 2000>;
+        interrupts = <29>;
+    };
diff --git a/Documentation/devicetree/bindings/rtc/stmp3xxx-rtc.txt b/Documentation/devicetree/bindings/rtc/stmp3xxx-rtc.txt
deleted file mode 100644
index fa6a94226669..000000000000
--- a/Documentation/devicetree/bindings/rtc/stmp3xxx-rtc.txt
+++ /dev/null
@@ -1,21 +0,0 @@
-* STMP3xxx/i.MX28 Time Clock controller
-
-Required properties:
-- compatible: should be one of the following.
-    * "fsl,stmp3xxx-rtc"
-- reg: physical base address of the controller and length of memory mapped
-  region.
-- interrupts: rtc alarm interrupt
-
-Optional properties:
-- stmp,crystal-freq: override crystal frequency as determined from fuse bits.
-  Only <32000> and <32768> are possible for the hardware.  Use <0> for
-  "no crystal".
-
-Example:
-
-rtc@80056000 {
-	compatible = "fsl,imx28-rtc", "fsl,stmp3xxx-rtc";
-	reg = <0x80056000 2000>;
-	interrupts = <29>;
-};

-- 
2.40.1


^ permalink raw reply related

* [PATCH v2 3/4] dt-bindings: rtc: pxa-rtc: convert to dtschema
From: Javier Carrasco @ 2024-04-10 15:55 UTC (permalink / raw)
  To: Alexandre Belloni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jiaxun Yang, Vladimir Zapolskiy, Joel Stanley, Andrew Jeffery,
	Maxime Coquelin, Alexandre Torgue
  Cc: linux-rtc, devicetree, linux-kernel, linux-arm-kernel,
	linux-aspeed, linux-stm32, Javier Carrasco, Krzysztof Kozlowski
In-Reply-To: <20240410-rtc_dtschema-v2-0-d32a11ab0745@gmail.com>

Convert existing binding to dtschema to support validation.

The missing 'reg' and 'interrupts' properties have been added, taking
the 2 supported interrupts into account to fix the example.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
 .../devicetree/bindings/rtc/marvell,pxa-rtc.yaml   | 40 ++++++++++++++++++++++
 Documentation/devicetree/bindings/rtc/pxa-rtc.txt  | 14 --------
 2 files changed, 40 insertions(+), 14 deletions(-)

diff --git a/Documentation/devicetree/bindings/rtc/marvell,pxa-rtc.yaml b/Documentation/devicetree/bindings/rtc/marvell,pxa-rtc.yaml
new file mode 100644
index 000000000000..43d68681a1bf
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/marvell,pxa-rtc.yaml
@@ -0,0 +1,40 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/marvell,pxa-rtc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: PXA Real Time Clock
+
+maintainers:
+  - Javier Carrasco <javier.carrasco.cruz@gmail.com>
+
+allOf:
+  - $ref: rtc.yaml#
+
+properties:
+  compatible:
+    const: marvell,pxa-rtc
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    items:
+      - description: 1 Hz
+      - description: Alarm
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    rtc@40900000 {
+        compatible = "marvell,pxa-rtc";
+        reg = <0x40900000 0x3c>;
+        interrupts = <30>, <31>;
+    };
diff --git a/Documentation/devicetree/bindings/rtc/pxa-rtc.txt b/Documentation/devicetree/bindings/rtc/pxa-rtc.txt
deleted file mode 100644
index 8c6672a1b7d7..000000000000
--- a/Documentation/devicetree/bindings/rtc/pxa-rtc.txt
+++ /dev/null
@@ -1,14 +0,0 @@
-* PXA RTC
-
-PXA specific RTC driver.
-
-Required properties:
-- compatible : Should be "marvell,pxa-rtc"
-
-Examples:
-
-rtc@40900000 {
-	compatible = "marvell,pxa-rtc";
-	reg = <0x40900000 0x3c>;
-	interrupts = <30 31>;
-};

-- 
2.40.1


^ permalink raw reply related

* [PATCH v2 2/4] dt-bindings: rtc: lpc32xx-rtc: convert to dtschema
From: Javier Carrasco @ 2024-04-10 15:55 UTC (permalink / raw)
  To: Alexandre Belloni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jiaxun Yang, Vladimir Zapolskiy, Joel Stanley, Andrew Jeffery,
	Maxime Coquelin, Alexandre Torgue
  Cc: linux-rtc, devicetree, linux-kernel, linux-arm-kernel,
	linux-aspeed, linux-stm32, Javier Carrasco
In-Reply-To: <20240410-rtc_dtschema-v2-0-d32a11ab0745@gmail.com>

Convert existing binding to dtschema to support validation.

Add the undocumented 'clocks' property.

Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
 .../devicetree/bindings/rtc/lpc32xx-rtc.txt        | 15 --------
 .../devicetree/bindings/rtc/nxp,lpc32xx-rtc.yaml   | 41 ++++++++++++++++++++++
 2 files changed, 41 insertions(+), 15 deletions(-)

diff --git a/Documentation/devicetree/bindings/rtc/lpc32xx-rtc.txt b/Documentation/devicetree/bindings/rtc/lpc32xx-rtc.txt
deleted file mode 100644
index a87a1e9bc060..000000000000
--- a/Documentation/devicetree/bindings/rtc/lpc32xx-rtc.txt
+++ /dev/null
@@ -1,15 +0,0 @@
-* NXP LPC32xx SoC Real Time Clock controller
-
-Required properties:
-- compatible: must be "nxp,lpc3220-rtc"
-- reg: physical base address of the controller and length of memory mapped
-  region.
-- interrupts: The RTC interrupt
-
-Example:
-
-	rtc@40024000 {
-		compatible = "nxp,lpc3220-rtc";
-		reg = <0x40024000 0x1000>;
-		interrupts = <52 0>;
-	};
diff --git a/Documentation/devicetree/bindings/rtc/nxp,lpc32xx-rtc.yaml b/Documentation/devicetree/bindings/rtc/nxp,lpc32xx-rtc.yaml
new file mode 100644
index 000000000000..62ddeef961e9
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/nxp,lpc32xx-rtc.yaml
@@ -0,0 +1,41 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/nxp,lpc32xx-rtc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP LPC32xx SoC Real Time Clock
+
+maintainers:
+  - Javier Carrasco <javier.carrasco.cruz@gmail.com>
+
+allOf:
+  - $ref: rtc.yaml#
+
+properties:
+  compatible:
+    const: nxp,lpc3220-rtc
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    rtc@40024000 {
+        compatible = "nxp,lpc3220-rtc";
+        reg = <0x40024000 0x1000>;
+        interrupts = <52 0>;
+    };

-- 
2.40.1


^ permalink raw reply related

* [PATCH v2 1/4] dt-bindings: rtc: convert trivial devices into dtschema
From: Javier Carrasco @ 2024-04-10 15:55 UTC (permalink / raw)
  To: Alexandre Belloni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jiaxun Yang, Vladimir Zapolskiy, Joel Stanley, Andrew Jeffery,
	Maxime Coquelin, Alexandre Torgue
  Cc: linux-rtc, devicetree, linux-kernel, linux-arm-kernel,
	linux-aspeed, linux-stm32, Javier Carrasco, Krzysztof Kozlowski
In-Reply-To: <20240410-rtc_dtschema-v2-0-d32a11ab0745@gmail.com>

These RTCs meet the requirements for a direct conversion into
trivial-rtc:

- orion-rtc
- google,goldfish-rtc
- maxim,ds1742
- rtc-aspped
- spear-rtc
- via,vt8500-rtc

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Andrew Jeffery <andrew@codeconstruct.com.au>
Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
 .../bindings/rtc/google,goldfish-rtc.txt           | 17 -----------------
 .../devicetree/bindings/rtc/maxim,ds1742.txt       | 12 ------------
 .../devicetree/bindings/rtc/orion-rtc.txt          | 18 ------------------
 .../devicetree/bindings/rtc/rtc-aspeed.txt         | 22 ----------------------
 .../devicetree/bindings/rtc/spear-rtc.txt          | 15 ---------------
 .../devicetree/bindings/rtc/trivial-rtc.yaml       | 16 ++++++++++++++++
 .../devicetree/bindings/rtc/via,vt8500-rtc.txt     | 15 ---------------
 MAINTAINERS                                        |  1 -
 8 files changed, 16 insertions(+), 100 deletions(-)

diff --git a/Documentation/devicetree/bindings/rtc/google,goldfish-rtc.txt b/Documentation/devicetree/bindings/rtc/google,goldfish-rtc.txt
deleted file mode 100644
index 634312dd95ca..000000000000
--- a/Documentation/devicetree/bindings/rtc/google,goldfish-rtc.txt
+++ /dev/null
@@ -1,17 +0,0 @@
-Android Goldfish RTC
-
-Android Goldfish RTC device used by Android emulator.
-
-Required properties:
-
-- compatible : should contain "google,goldfish-rtc"
-- reg        : <registers mapping>
-- interrupts : <interrupt mapping>
-
-Example:
-
-	goldfish_timer@9020000 {
-		compatible = "google,goldfish-rtc";
-		reg = <0x9020000 0x1000>;
-		interrupts = <0x3>;
-	};
diff --git a/Documentation/devicetree/bindings/rtc/maxim,ds1742.txt b/Documentation/devicetree/bindings/rtc/maxim,ds1742.txt
deleted file mode 100644
index d0f937c355b5..000000000000
--- a/Documentation/devicetree/bindings/rtc/maxim,ds1742.txt
+++ /dev/null
@@ -1,12 +0,0 @@
-* Maxim (Dallas) DS1742/DS1743 Real Time Clock
-
-Required properties:
-- compatible: Should contain "maxim,ds1742".
-- reg: Physical base address of the RTC and length of memory
-  mapped region.
-
-Example:
-	rtc: rtc@10000000 {
-		compatible = "maxim,ds1742";
-		reg = <0x10000000 0x800>;
-	};
diff --git a/Documentation/devicetree/bindings/rtc/orion-rtc.txt b/Documentation/devicetree/bindings/rtc/orion-rtc.txt
deleted file mode 100644
index 3bf63ffa5160..000000000000
--- a/Documentation/devicetree/bindings/rtc/orion-rtc.txt
+++ /dev/null
@@ -1,18 +0,0 @@
-* Mvebu Real Time Clock
-
-RTC controller for the Kirkwood, the Dove, the Armada 370 and the
-Armada XP SoCs
-
-Required properties:
-- compatible : Should be "marvell,orion-rtc"
-- reg: physical base address of the controller and length of memory mapped
-  region.
-- interrupts: IRQ line for the RTC.
-
-Example:
-
-rtc@10300 {
-        compatible = "marvell,orion-rtc";
-        reg = <0xd0010300 0x20>;
-        interrupts = <50>;
-};
diff --git a/Documentation/devicetree/bindings/rtc/rtc-aspeed.txt b/Documentation/devicetree/bindings/rtc/rtc-aspeed.txt
deleted file mode 100644
index 2e956b3dc276..000000000000
--- a/Documentation/devicetree/bindings/rtc/rtc-aspeed.txt
+++ /dev/null
@@ -1,22 +0,0 @@
-ASPEED BMC RTC
-==============
-
-Required properties:
- - compatible: should be one of the following
-   * aspeed,ast2400-rtc for the ast2400
-   * aspeed,ast2500-rtc for the ast2500
-   * aspeed,ast2600-rtc for the ast2600
-
- - reg: physical base address of the controller and length of memory mapped
-   region
-
- - interrupts: The interrupt number
-
-Example:
-
-   rtc@1e781000 {
-           compatible = "aspeed,ast2400-rtc";
-           reg = <0x1e781000 0x18>;
-           interrupts = <22>;
-           status = "disabled";
-   };
diff --git a/Documentation/devicetree/bindings/rtc/spear-rtc.txt b/Documentation/devicetree/bindings/rtc/spear-rtc.txt
deleted file mode 100644
index fecf8e4ad4b4..000000000000
--- a/Documentation/devicetree/bindings/rtc/spear-rtc.txt
+++ /dev/null
@@ -1,15 +0,0 @@
-* SPEAr RTC
-
-Required properties:
-- compatible : "st,spear600-rtc"
-- reg : Address range of the rtc registers
-- interrupt: Should contain the rtc interrupt number
-
-Example:
-
-	rtc@fc000000 {
-		compatible = "st,spear600-rtc";
-		reg = <0xfc000000 0x1000>;
-		interrupt-parent = <&vic1>;
-		interrupts = <12>;
-	};
diff --git a/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml b/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml
index c9e3c5262c21..b590bf35d440 100644
--- a/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml
+++ b/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml
@@ -24,6 +24,12 @@ properties:
       - abracon,abb5zes3
       # AB-RTCMC-32.768kHz-EOZ9: Real Time Clock/Calendar Module with I2C Interface
       - abracon,abeoz9
+      # ASPEED BMC ast2400 Real-time Clock
+      - aspeed,ast2400-rtc
+      # ASPEED BMC ast2500 Real-time Clock
+      - aspeed,ast2500-rtc
+      # ASPEED BMC ast2600 Real-time Clock
+      - aspeed,ast2600-rtc
       # I2C, 32-Bit Binary Counter Watchdog RTC with Trickle Charger and Reset Input/Output
       - dallas,ds1374
       # Dallas DS1672 Real-time Clock
@@ -41,10 +47,16 @@ properties:
       - epson,rx8571
       # I2C-BUS INTERFACE REAL TIME CLOCK MODULE
       - epson,rx8581
+      # Android Goldfish Real-time Clock
+      - google,goldfish-rtc
       # Intersil ISL1208 Low Power RTC with Battery Backed SRAM
       - isil,isl1208
       # Intersil ISL1218 Low Power RTC with Battery Backed SRAM
       - isil,isl1218
+      # Mvebu Real-time Clock
+      - marvell,orion-rtc
+      # Maxim DS1742/DS1743 Real-time Clock
+      - maxim,ds1742
       # SPI-BUS INTERFACE REAL TIME CLOCK MODULE
       - maxim,mcp795
       # Real Time Clock Module with I2C-Bus
@@ -67,6 +79,10 @@ properties:
       - ricoh,rv5c387a
       # 2-wire CMOS real-time clock
       - sii,s35390a
+      # ST SPEAr Real-time Clock
+      - st,spear600-rtc
+      # VIA/Wondermedia VT8500 Real-time Clock
+      - via,vt8500-rtc
       # I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC
       - whwave,sd3078
       # Xircom X1205 I2C RTC
diff --git a/Documentation/devicetree/bindings/rtc/via,vt8500-rtc.txt b/Documentation/devicetree/bindings/rtc/via,vt8500-rtc.txt
deleted file mode 100644
index 3c0484c49582..000000000000
--- a/Documentation/devicetree/bindings/rtc/via,vt8500-rtc.txt
+++ /dev/null
@@ -1,15 +0,0 @@
-VIA/Wondermedia VT8500 Realtime Clock Controller
------------------------------------------------------
-
-Required properties:
-- compatible : "via,vt8500-rtc"
-- reg : Should contain 1 register ranges(address and length)
-- interrupts : alarm interrupt
-
-Example:
-
-	rtc@d8100000 {
-		compatible = "via,vt8500-rtc";
-		reg = <0xd8100000 0x10000>;
-		interrupts = <48>;
-	};
diff --git a/MAINTAINERS b/MAINTAINERS
index aea47e04c3a5..f24469714f1e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1442,7 +1442,6 @@ F:	drivers/irqchip/irq-goldfish-pic.c
 ANDROID GOLDFISH RTC DRIVER
 M:	Jiaxun Yang <jiaxun.yang@flygoat.com>
 S:	Supported
-F:	Documentation/devicetree/bindings/rtc/google,goldfish-rtc.txt
 F:	drivers/rtc/rtc-goldfish.c
 
 AOA (Apple Onboard Audio) ALSA DRIVER

-- 
2.40.1


^ permalink raw reply related

* [PATCH v2 0/4] rtc: convert multiple bindings into dtschema
From: Javier Carrasco @ 2024-04-10 15:55 UTC (permalink / raw)
  To: Alexandre Belloni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jiaxun Yang, Vladimir Zapolskiy, Joel Stanley, Andrew Jeffery,
	Maxime Coquelin, Alexandre Torgue
  Cc: linux-rtc, devicetree, linux-kernel, linux-arm-kernel,
	linux-aspeed, linux-stm32, Javier Carrasco, Krzysztof Kozlowski

This series converts the following bindings into dtschema, moving them
to trivial-rtc whenever possible:

- orion-rtc: trival-rtc, referenced in arm arch.
- google,goldfish-rtc: trivial-rtc, referenced in mips arch.
- lpc32xx-rtc: add missing property and convert, referenced in arm arch.
- maxim,ds1742: trivial-rtc, not referenced in arch, cheap conversion.
- rtc-aspeed: 3 devices to trivial-rtc, all referenced in arm arch.
- pxa-rtc: add missing properties and convert. Referenced in arm arch.
- st,spear600-rtc: trivial-rtc, referenced in arm arch.
- stmp3xxx-rtc: add compatibles and convert, referenced in arm arch.
- via,vt8500-rtc: trivial-rtc, referenced in arm arch.

Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
Changes in v2:
- General: squash all moves to trivial-rtc into a single patch.
- MAINTAINERS: remove reference to google,goldfish-rtc.txt
- lpc32xx-rtc: create own binding to add the undocumented 'clocks'
  property.
- fsl,stmp3xxx-rtc.yaml: document missing compatibles.
- Link to v1: https://lore.kernel.org/r/20240408-rtc_dtschema-v1-0-c447542fc362@gmail.com

---
Javier Carrasco (4):
      dt-bindings: rtc: convert trivial devices into dtschema
      dt-bindings: rtc: lpc32xx-rtc: convert to dtschema
      dt-bindings: rtc: pxa-rtc: convert to dtschema
      dt-bindings: rtc: stmp3xxx-rtc: convert to dtschema

 .../devicetree/bindings/rtc/fsl,stmp3xxx-rtc.yaml  | 51 ++++++++++++++++++++++
 .../bindings/rtc/google,goldfish-rtc.txt           | 17 --------
 .../devicetree/bindings/rtc/lpc32xx-rtc.txt        | 15 -------
 .../devicetree/bindings/rtc/marvell,pxa-rtc.yaml   | 40 +++++++++++++++++
 .../devicetree/bindings/rtc/maxim,ds1742.txt       | 12 -----
 .../devicetree/bindings/rtc/nxp,lpc32xx-rtc.yaml   | 41 +++++++++++++++++
 .../devicetree/bindings/rtc/orion-rtc.txt          | 18 --------
 Documentation/devicetree/bindings/rtc/pxa-rtc.txt  | 14 ------
 .../devicetree/bindings/rtc/rtc-aspeed.txt         | 22 ----------
 .../devicetree/bindings/rtc/spear-rtc.txt          | 15 -------
 .../devicetree/bindings/rtc/stmp3xxx-rtc.txt       | 21 ---------
 .../devicetree/bindings/rtc/trivial-rtc.yaml       | 16 +++++++
 .../devicetree/bindings/rtc/via,vt8500-rtc.txt     | 15 -------
 MAINTAINERS                                        |  1 -
 14 files changed, 148 insertions(+), 150 deletions(-)
---
base-commit: fec50db7033ea478773b159e0e2efb135270e3b7
change-id: 20240406-rtc_dtschema-302824d1ec20

Best regards,
-- 
Javier Carrasco <javier.carrasco.cruz@gmail.com>


^ permalink raw reply

* Re: [PATCH] arm64: dts: qcom: sm8650: add description of CCI controllers
From: Krzysztof Kozlowski @ 2024-04-10 15:45 UTC (permalink / raw)
  To: Vladimir Zapolskiy, Bjorn Andersson, Konrad Dybcio,
	Jagadeesh Kona
  Cc: Krzysztof Kozlowski, Neil Armstrong, linux-arm-msm, devicetree
In-Reply-To: <d91239fb-d15c-4984-96e4-1c2bd1361ad2@linaro.org>

On 10/04/2024 15:22, Vladimir Zapolskiy wrote:
> On 4/10/24 11:26, Krzysztof Kozlowski wrote:
>> On 10/04/2024 09:49, Vladimir Zapolskiy wrote:
>>> Qualcomm SM8650 SoC has three CCI controllers with two I2C busses
>>> connected to each of them.
>>>
>>> The CCI controllers on SM8650 are compatible with the ones found on
>>> many other older generations of Qualcomm SoCs.
>>>
>>> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>>> ---
>>> The change is based and depends on a patch series from Jagadeesh Kona:
>>>
>>>    https://lore.kernel.org/linux-arm-msm/20240321092529.13362-1-quic_jkona@quicinc.com/
>>>
>>> It might be an option to add this change right to the series,
>>> since it anyway requires a respin.
>>>
>>> A new compatible value "qcom,sm8650-cci" is NOT added to
>>> Documentation/devicetree/bindings/i2c/qcom,i2c-cci.yaml , because
>>> the controller IP description and selection is covered by a generic
>>> compatible value "qcom,msm8996-cci".
>>
>> I do not understand this reasoning. So you introduce known errors
>> because errors are ok?
>>
>> How does it pass dtbs_check validation?
> 
> To continue the technical discussion let me ask you to comment on the
> absolutely identical subject, which has been taken in the past in connection
> to "qcom,sc8280xp-cci" compatible, probably it didn't attact any sufficient
> attention before, so let's continue now.
> 
> https://lore.kernel.org/linux-arm-msm/0a3cd2f3-85e9-4769-9749-62353e842625@linaro.org/

You mix topics. First, you cannot send patch which knowingly introduces
errors, regardless these are build errors or dtbs_check errors.

Plus checkpatch also complains about it.

Second, you linked to a driver discussion, but we talk here about
bindings. Not driver. Each binding must be documented.

Now, about driver, there is no single point nor need to add there
duplicated entry, that's why we don't add.

Best regards,
Krzysztof


^ permalink raw reply

* Re: [EXT] Re: [PATCH v8 3/8] perf: imx_perf: let the driver manage the counter usage rather the user
From: Will Deacon @ 2024-04-10 15:45 UTC (permalink / raw)
  To: Xu Yang
  Cc: Frank Li, mark.rutland@arm.com, robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
	shawnguo@kernel.org, s.hauer@pengutronix.de,
	kernel@pengutronix.de, festevam@gmail.com,
	john.g.garry@oracle.com, jolsa@kernel.org, namhyung@kernel.org,
	irogers@google.com, mike.leach@linaro.org, peterz@infradead.org,
	mingo@redhat.com, acme@kernel.org,
	alexander.shishkin@linux.intel.com, adrian.hunter@intel.com,
	linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
	imx@lists.linux.dev
In-Reply-To: <DU2PR04MB882209413AB62531713166B38C062@DU2PR04MB8822.eurprd04.prod.outlook.com>

On Wed, Apr 10, 2024 at 07:39:46AM +0000, Xu Yang wrote:
> > On Fri, Mar 22, 2024 at 02:39:25PM +0800, Xu Yang wrote:
> > > In current design, the user of perf app needs to input counter ID to count
> > > events. However, this is not user-friendly since the user needs to lookup
> > > the map table to find the counter. Instead of letting the user to input
> > > the counter, let this driver to manage the counters in this patch.
> > 
> > I think we still have to support the old interface so that we don't break
> > those existing users (even if the driver just ignores whatever counter ID
> > is provided in a backwards-compatible way).
> > 
> > > This will be implemented by:
> > >  1. allocate counter 0 for cycle event.
> > >  2. find unused counter from 1-10 for reference events.
> > >  3. allocate specific counter for counter-specific events.
> > >
> > > In this patch, counter attribute is removed too. To mark counter-specific
> > > events, counter ID will be encoded into perf_pmu_events_attr.id.
> > >
> > > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > >
> > > ---
> > > Changes in v6:
> > >  - new patch
> > > Changes in v7:
> > >  - no changes
> > > Changes in v8:
> > >  - add Rb tag
> > > ---
> > >  drivers/perf/fsl_imx9_ddr_perf.c | 168 ++++++++++++++++++-------------
> > >  1 file changed, 99 insertions(+), 69 deletions(-)
> > >
> > > diff --git a/drivers/perf/fsl_imx9_ddr_perf.c b/drivers/perf/fsl_imx9_ddr_perf.c
> > > index 0017f2c9ef91..b728719b494c 100644
> > > --- a/drivers/perf/fsl_imx9_ddr_perf.c
> > > +++ b/drivers/perf/fsl_imx9_ddr_perf.c
> > > @@ -245,14 +249,12 @@ static const struct attribute_group ddr_perf_events_attr_group = {
> > >       .attrs = ddr_perf_events_attrs,
> > >  };
> > >
> > > -PMU_FORMAT_ATTR(event, "config:0-7");
> > > -PMU_FORMAT_ATTR(counter, "config:8-15");
> > > +PMU_FORMAT_ATTR(event, "config:0-15");
> > 
> > Sadly, this is a user-visible change so I think it will break old tools,
> > won't it?
> 
> For imx ddr pmu, most of the people will use metrics rather event itself,
> and we have speficy the format of event parameters in metrics table.
> The parameters is also updated in this patchset.
> 
> And to easy use for user, the counter should be hidden (transparent) to
> user after I had talk with Frank. Then, the user need't to look up the event
> table to find which counter to use. 
> 
> So this patchset will basically not break the usage of perf tools and will
> improve practicality.

Sorry, but I don't agree. The original commit adding this driver
(55691f99d417) gives the following examples in the commit message:

For example:
      perf stat -a -I 1000 -e imx9_ddr0/eddrtq_pm_rd_trans_filt,counter=2,axi_mask=ID_MASK,axi_id=ID/
      perf stat -a -I 1000 -e imx9_ddr0/eddrtq_pm_wr_trans_filt,counter=3,axi_mask=ID_MASK,axi_id=ID/
      perf stat -a -I 1000 -e imx9_ddr0/eddrtq_pm_rd_beat_filt,counter=4,axi_mask=ID_MASK,axi_id=ID/

I don't think these will work any more if we apply this patch.

Will

^ permalink raw reply

* Re: [PATCH v4 13/18] ASoC: mediatek: mt8186: Unify mt8186-mt6366 machine drivers
From: Alexandre Mergnat @ 2024-04-10 15:38 UTC (permalink / raw)
  To: AngeloGioacchino Del Regno, broonie
  Cc: wenst, lgirdwood, robh, krzysztof.kozlowski+dt, conor+dt,
	matthias.bgg, perex, tiwai, trevor.wu, maso.huang, xiazhengqiao,
	arnd, kuninori.morimoto.gx, shraash, nicolas.ferre,
	u.kleine-koenig, dianders, frank.li, allen-kh.cheng,
	eugen.hristev, claudiu.beznea, jarkko.nikula, jiaxin.yu,
	alpernebiyasak, ckeepax, zhourui, nfraprado, alsa-devel,
	shane.chien, linux-sound, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek, kernel
In-Reply-To: <20240409113310.303261-14-angelogioacchino.delregno@collabora.com>



On 09/04/2024 13:33, AngeloGioacchino Del Regno wrote:
> @@ -318,16 +409,24 @@ static int mt8186_it6505_i2s_hw_params_fixup(struct snd_soc_pcm_runtime *rtd,
>   static int mt8186_sof_dai_link_fixup(struct snd_soc_pcm_runtime *rtd,
>   				     struct snd_pcm_hw_params *params)
>   {
> +	struct mtk_soc_card_data *soc_card_data = snd_soc_card_get_drvdata(rtd->card);
>   	int ret;
>   
>   	ret = mtk_sof_dai_link_fixup(rtd, params);
>   
>   	if (!strcmp(rtd->dai_link->name, "I2S0") ||
>   	    !strcmp(rtd->dai_link->name, "I2S1") ||
> -	    !strcmp(rtd->dai_link->name, "I2S2"))
> -		mt8186_i2s_hw_params_fixup(rtd, params);
> -	else if (!strcmp(rtd->dai_link->name, "I2S3"))
> -		mt8186_it6505_i2s_hw_params_fixup(rtd, params);
> +	    !strcmp(rtd->dai_link->name, "I2S2")) {
> +		if (soc_card_data->card_data->flags & DA7219_CODEC_PRESENT)

Is the flag missing like you did for mt8195-mt6359.c ?
			if (!(codec_init & MT6359_CODEC_INIT)) {
				dai_link->init = mt8195_mt6359_init;
				codec_init |= MT6359_CODEC_INIT;


> +			mt8186_i2s_hw_params_32le_fixup(rtd, params);
> +		else
> +			mt8186_i2s_hw_params_24le_fixup(rtd, params);
> +	} else if (!strcmp(rtd->dai_link->name, "I2S3")) {
> +		if (soc_card_data->card_data->flags & DA7219_CODEC_PRESENT)
> +			mt8186_i2s_hw_params_24le_fixup(rtd, params);
> +		else
> +			mt8186_i2s_hw_params_32le_fixup(rtd, params);
> +	}
>   
>   	return ret;
>   }

-- 
Regards,
Alexandre

^ permalink raw reply

* Re: [PATCH v4 05/18] ASoC: mediatek: mt8192: Migrate to mtk_soundcard_common_probe
From: AngeloGioacchino Del Regno @ 2024-04-10 15:28 UTC (permalink / raw)
  To: Alexandre Mergnat, broonie
  Cc: wenst, lgirdwood, robh, krzysztof.kozlowski+dt, conor+dt,
	matthias.bgg, perex, tiwai, trevor.wu, maso.huang, xiazhengqiao,
	arnd, kuninori.morimoto.gx, shraash, nicolas.ferre,
	u.kleine-koenig, dianders, frank.li, allen-kh.cheng,
	eugen.hristev, claudiu.beznea, jarkko.nikula, jiaxin.yu,
	alpernebiyasak, ckeepax, zhourui, nfraprado, alsa-devel,
	shane.chien, linux-sound, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek, kernel
In-Reply-To: <cf0184c4-2b3e-4074-9e30-bf65ec7f0aaa@baylibre.com>

Il 10/04/24 15:59, Alexandre Mergnat ha scritto:
> 
> 
> On 09/04/2024 13:32, AngeloGioacchino Del Regno wrote:
>> @@ -1211,52 +1196,85 @@ static int mt8192_mt6359_dev_probe(struct platform_device 
>> *pdev)
>>           if (dai_link->num_codecs && dai_link->codecs[0].dai_name &&
>>               strcmp(dai_link->codecs[0].dai_name, RT1015_CODEC_DAI) == 0)
>>               dai_link->ops = &mt8192_rt1015_i2s_ops;
>> -
>> -        if (!dai_link->platforms->name)
>> -            dai_link->platforms->of_node = platform_node;
>> -    }
>> -
>> -    priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
>> -    if (!priv) {
>> -        ret = -ENOMEM;
>> -        goto err_probe;
>> -    }
>> -    snd_soc_card_set_drvdata(card, priv);
>> -
>> -    ret = mt8192_afe_gpio_init(&pdev->dev);
>> -    if (ret) {
>> -        dev_err_probe(&pdev->dev, ret, "%s init gpio error\n", __func__);
> 
> I don't think __func__ is necessary.
> 
>> -        goto err_probe;
>>       }
>> -    ret = devm_snd_soc_register_card(&pdev->dev, card);
>> -    if (ret)
>> -        dev_err_probe(&pdev->dev, ret, "%s snd_soc_register_card fail\n", 
>> __func__);
> 
> I don't think __func__ is necessary.

I am removing the line, here :-P

> 
>> -
>> -err_probe:
>>       of_node_put(headset_codec);
>>   err_headset_codec:
>>       of_node_put(speaker_codec);
>>   err_speaker_codec:
>> -    of_node_put(platform_node);
>> -err_platform_node:
>> -    of_node_put(hdmi_codec);
>> +    if (hdmi_codec)
>> +        of_node_put(hdmi_codec);
>> +
>>       return ret;
>>   }
>> +static int mt8192_mt6359_soc_card_probe(struct mtk_soc_card_data *soc_card_data, 
>> bool legacy)
>> +{
>> +    struct mtk_platform_card_data *card_data = soc_card_data->card_data;
>> +    struct snd_soc_card *card = card_data->card;
>> +    int ret;
>> +
>> +    if (legacy) {
>> +        ret = mt8192_mt6359_legacy_probe(soc_card_data);
>> +        if (ret)
>> +            return ret;
>> +    } else {
>> +        struct snd_soc_dai_link *dai_link;
>> +        int i;
>> +
>> +        for_each_card_prelinks(card, i, dai_link)
>> +            if (dai_link->num_codecs && dai_link->codecs[0].dai_name &&
>> +                strcmp(dai_link->codecs[0].dai_name, RT1015_CODEC_DAI) == 0)
>> +                dai_link->ops = &mt8192_rt1015_i2s_ops;
>> +    }
>> +
>> +    ret = mt8192_afe_gpio_init(card->dev);
>> +    if (ret)
>> +        return dev_err_probe(card->dev, ret, "%s init gpio error\n", __func__);
> 
> I don't think __func__ is necessary.
> 

That was on purpose.
I'm migrating things, but I am leaving the prints as they were.

Cheers,
Angelo

>> +
>> +    return 0;
>> +}
> 
> Beside that,
> Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
> 




^ permalink raw reply

* Re: [PATCH v7 05/16] dt-bindings: net: wireless: describe the ath12k PCI module
From: Jeff Johnson @ 2024-04-10 15:27 UTC (permalink / raw)
  To: Bartosz Golaszewski, Marcel Holtmann, Luiz Augusto von Dentz,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Kalle Valo,
	Bjorn Andersson, Konrad Dybcio, Liam Girdwood, Mark Brown,
	Catalin Marinas, Will Deacon, Bjorn Helgaas, Saravana Kannan,
	Geert Uytterhoeven, Arnd Bergmann, Neil Armstrong,
	Marek Szyprowski, Alex Elder, Srini Kandagatla,
	Greg Kroah-Hartman, Abel Vesa, Manivannan Sadhasivam,
	Lukas Wunner, Dmitry Baryshkov, Amit Pundir, Xilin Wu
  Cc: linux-bluetooth, netdev, devicetree, linux-kernel, linux-wireless,
	linux-arm-msm, linux-arm-kernel, linux-pci, linux-pm,
	Bartosz Golaszewski, Krzysztof Kozlowski
In-Reply-To: <20240410124628.171783-6-brgl@bgdev.pl>

On 4/10/2024 5:46 AM, Bartosz Golaszewski wrote:
[...]
> +description:
> +  Qualcomm Technologies IEEE 802.11ax PCIe devices.

if you respin, nit: s/11ax/11be/


^ permalink raw reply

* Re: [PATCH] arm64: dts: qcom: sm8650: add description of CCI controllers
From: neil.armstrong @ 2024-04-10 15:26 UTC (permalink / raw)
  To: Vladimir Zapolskiy, Bjorn Andersson, Konrad Dybcio,
	Jagadeesh Kona
  Cc: Krzysztof Kozlowski, linux-arm-msm, devicetree
In-Reply-To: <f5611116-df8e-4118-8aad-16561f65c79f@linaro.org>

On 10/04/2024 17:19, Vladimir Zapolskiy wrote:
> Hi Neil,
> 
> On 4/10/24 16:50, neil.armstrong@linaro.org wrote:
>> On 10/04/2024 15:11, Vladimir Zapolskiy wrote:
>>> On 4/10/24 10:52, Neil Armstrong wrote:
>>>> Hi,
>>>>
>>>> On 10/04/2024 09:49, Vladimir Zapolskiy wrote:
>>>>> Qualcomm SM8650 SoC has three CCI controllers with two I2C busses
>>>>> connected to each of them.
>>>>>
>>>>> The CCI controllers on SM8650 are compatible with the ones found on
>>>>> many other older generations of Qualcomm SoCs.
>>>>>
>>>>> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>>>>> ---
>>>>> The change is based and depends on a patch series from Jagadeesh Kona:
>>>>>
>>>>>      https://lore.kernel.org/linux-arm-msm/20240321092529.13362-1-quic_jkona@quicinc.com/
>>>>>
>>>>> It might be an option to add this change right to the series,
>>>>> since it anyway requires a respin.
>>>>>
>>>>> A new compatible value "qcom,sm8650-cci" is NOT added to
>>>>> Documentation/devicetree/bindings/i2c/qcom,i2c-cci.yaml , because
>>>>> the controller IP description and selection is covered by a generic
>>>>> compatible value "qcom,msm8996-cci".
>>>>
>>>> You'll still need to add qcom,sm8650-cci to the "CCI v2" list in qcom,i2c-cci.yaml,
>>>> otherwise the DTBS check fail, even if the fallback is already present.
>>>
>>> I do recognize the problem related to a build time warning, my motivation was
>>> to follow the rationale described in commit 3e383dce513f
>>> ("Revert "dt-bindings: i2c: qcom-cci: Document sc8280xp compatible"").
>>>
>>> For a similar sc8280xp-cci case it was asked by Konrad to drop a new
>>> compatible, I kindly ask the reviewers and maintainers to stick to one
>>> of the two contradicting asks.
>>
>> This is totally different, this commit added a new compatible that is used in the driver,
>> while here, you use a per-soc compatible that is (for now), only used in DT and uses
> 
> I'm confused, please elaborate what do you mean above by "this commit" and "here".
> Could you please be more specific to avoid any possible disambiguation?

"this" refer to "dt-bindings: i2c: qcom-cci: Document sc8280xp compatible".

> If you refer to the driver drivers/i2c/busses/i2c-qcom-cci.c, then there is no
> difference between sc8280xp-cci and sm8650-cci. What is the total difference,
> which you found?

If there's no difference between sc8280xp-cci and sm8650-cci, then the policy says
you need to _not_ add a new compatible in the driver, which is what you did here.

> 
>> the generic "qcom,msm8996-cci" as a fallback because it is considered as beeing 99%
>> compatible and no software change is needed.
>>
> 
> I have no objections to revert a "Revert "dt-bindings: i2c: qcom-cci: Document sc8280xp compatible""
> commit and to update the change for sm8650-cci accordingly, but as I've
> already said it would be good to have and follow one common approach for both
> cases, since I based my change on the maintainer's decision from the past.

The "new" policy is to use a fallback of an already defined compatible if no driver change
is needed, this is the case for the last year so far.
And updating the yaml bindings for the new per-soc compatible is also a year-old
policy, upstreaming of SM8550, SM8650 and X1E80100 have been done following this policy
in order to :
1) reduce useless driver changes
2) have a fully verifiable DT against bindings, so we can ensure the DT is 100% valid against the bindings

Neil

> 
> -- 
> Best wishes,
> Vladimir


^ permalink raw reply

* Re: [PATCH] arm64: dts: qcom: sm8650: add description of CCI controllers
From: Vladimir Zapolskiy @ 2024-04-10 15:19 UTC (permalink / raw)
  To: neil.armstrong, Bjorn Andersson, Konrad Dybcio, Jagadeesh Kona
  Cc: Krzysztof Kozlowski, linux-arm-msm, devicetree
In-Reply-To: <4162174b-df35-4282-859e-84b0579ff91b@linaro.org>

Hi Neil,

On 4/10/24 16:50, neil.armstrong@linaro.org wrote:
> On 10/04/2024 15:11, Vladimir Zapolskiy wrote:
>> On 4/10/24 10:52, Neil Armstrong wrote:
>>> Hi,
>>>
>>> On 10/04/2024 09:49, Vladimir Zapolskiy wrote:
>>>> Qualcomm SM8650 SoC has three CCI controllers with two I2C busses
>>>> connected to each of them.
>>>>
>>>> The CCI controllers on SM8650 are compatible with the ones found on
>>>> many other older generations of Qualcomm SoCs.
>>>>
>>>> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>>>> ---
>>>> The change is based and depends on a patch series from Jagadeesh Kona:
>>>>
>>>>      https://lore.kernel.org/linux-arm-msm/20240321092529.13362-1-quic_jkona@quicinc.com/
>>>>
>>>> It might be an option to add this change right to the series,
>>>> since it anyway requires a respin.
>>>>
>>>> A new compatible value "qcom,sm8650-cci" is NOT added to
>>>> Documentation/devicetree/bindings/i2c/qcom,i2c-cci.yaml , because
>>>> the controller IP description and selection is covered by a generic
>>>> compatible value "qcom,msm8996-cci".
>>>
>>> You'll still need to add qcom,sm8650-cci to the "CCI v2" list in qcom,i2c-cci.yaml,
>>> otherwise the DTBS check fail, even if the fallback is already present.
>>
>> I do recognize the problem related to a build time warning, my motivation was
>> to follow the rationale described in commit 3e383dce513f
>> ("Revert "dt-bindings: i2c: qcom-cci: Document sc8280xp compatible"").
>>
>> For a similar sc8280xp-cci case it was asked by Konrad to drop a new
>> compatible, I kindly ask the reviewers and maintainers to stick to one
>> of the two contradicting asks.
> 
> This is totally different, this commit added a new compatible that is used in the driver,
> while here, you use a per-soc compatible that is (for now), only used in DT and uses

I'm confused, please elaborate what do you mean above by "this commit" and "here".
Could you please be more specific to avoid any possible disambiguation?

If you refer to the driver drivers/i2c/busses/i2c-qcom-cci.c, then there is no
difference between sc8280xp-cci and sm8650-cci. What is the total difference,
which you found?

> the generic "qcom,msm8996-cci" as a fallback because it is considered as beeing 99%
> compatible and no software change is needed.
> 

I have no objections to revert a "Revert "dt-bindings: i2c: qcom-cci: Document sc8280xp compatible""
commit and to update the change for sm8650-cci accordingly, but as I've
already said it would be good to have and follow one common approach for both
cases, since I based my change on the maintainer's decision from the past.

--
Best wishes,
Vladimir

^ permalink raw reply

* Re: [PATCH v4 12/18] ASoC: mediatek: Use common mtk_afe_pcm_platform with common probe cb
From: Alexandre Mergnat @ 2024-04-10 15:14 UTC (permalink / raw)
  To: AngeloGioacchino Del Regno, broonie
  Cc: wenst, lgirdwood, robh, krzysztof.kozlowski+dt, conor+dt,
	matthias.bgg, perex, tiwai, trevor.wu, maso.huang, xiazhengqiao,
	arnd, kuninori.morimoto.gx, shraash, nicolas.ferre,
	u.kleine-koenig, dianders, frank.li, allen-kh.cheng,
	eugen.hristev, claudiu.beznea, jarkko.nikula, jiaxin.yu,
	alpernebiyasak, ckeepax, zhourui, nfraprado, alsa-devel,
	shane.chien, linux-sound, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek, kernel
In-Reply-To: <20240409113310.303261-13-angelogioacchino.delregno@collabora.com>

Also done for mt8365-afe-pcm in my serie

Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>

On 09/04/2024 13:33, AngeloGioacchino Del Regno wrote:
> Since the mtk-afe-platform-driver generic mtk_afe_pcm_platform now has
> a common .probe() callback, there is no reason to keep duplicating this
> function over and over in the SoC specific AFE-PCM drivers: switch over
> to register with the common bits instead.
> 
> Note that MT8186 was left out of this because it is registering some
> extra sinegen controls in the AFE-PCM probe callback and needs extra
> cleanups to be able to use the common bits.

-- 
Regards,
Alexandre

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox