Devicetree
 help / color / mirror / Atom feed
* 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: [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 25/37] dt-binding: sh: cpus: Add SH CPUs json-schema
From: Rob Herring @ 2024-04-10 16:37 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: <7731111c9e383c41173ac08f3a040a18449ac542.1712207606.git.ysato@users.sourceforge.jp>

On Thu, Apr 04, 2024 at 02:14:36PM +0900, Yoshinori Sato wrote:
> Renesas SH series and compatible ISA CPUs.
> 
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> ---
>  .../devicetree/bindings/sh/cpus.yaml          | 63 +++++++++++++++++++
>  1 file changed, 63 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sh/cpus.yaml
> 
> diff --git a/Documentation/devicetree/bindings/sh/cpus.yaml b/Documentation/devicetree/bindings/sh/cpus.yaml
> new file mode 100644
> index 000000000000..9e5640793d76
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sh/cpus.yaml
> @@ -0,0 +1,63 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sh/cpus.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas SuperH CPUs
> +
> +maintainers:
> +  - Yoshinori Sato <ysato@users.sourceforge.jp>
> +
> +description: |+
> +  Definition of CPU core with Renesas SuperH and compatible instruction set.
> +
> +properties:
> +  compatible:
> +    anyOf:

oneOf

> +      - items:
> +          - enum:
> +              - renesas,sh2a
> +              - renesas,sh3
> +              - renesas,sh4
> +              - renesas,sh4a
> +              - jcore,j2
> +          - const: renesas,sh2
> +      - const: renesas,sh2
> +
> +  clocks:
> +    maxItems: 1
> +
> +  reg:
> +    maxItems: 1
> +
> +  device_type:
> +    const: cpu
> +
> +required:
> +  - compatible
> +  - reg
> +  - device_type
> +
> +additionalProperties: true

This is a problem with the other cpu bindings, but should not be copied 
here. Add a $ref to schemas/cpu.yaml and make this 
'unevaluatedProperties: false'.

> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/sh7750-cpg.h>
> +    cpus {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        cpu: cpu@0 {
> +            compatible = "renesas,sh4", "renesas,sh2";
> +            device_type = "cpu";
> +            reg = <0>;
> +            clocks = <&cpg SH7750_CPG_ICK>;
> +            clock-names = "ick";
> +            icache-size = <16384>;
> +            icache-line-size = <32>;
> +            dcache-size = <32768>;
> +            dcache-line-size = <32>;
> +        };
> +    };
> +...
> -- 
> 2.39.2
> 

^ permalink raw reply

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


On Thu, 04 Apr 2024 14:14:39 +0900, Yoshinori Sato wrote:
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> ---
>  .../devicetree/bindings/soc/renesas/sh.yaml   | 27 +++++++++++++++++++
>  1 file changed, 27 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/soc/renesas/sh.yaml
> 

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


^ permalink raw reply

* Re: [PATCH RESEND v8 01/10] watchdog: rzg2l_wdt: Restrict the driver to ARCH_RZG2L and ARCH_R9A09G011
From: Guenter Roeck @ 2024-04-10 16:38 UTC (permalink / raw)
  To: Claudiu
  Cc: wim, robh, krzk+dt, conor+dt, p.zabel, geert+renesas, magnus.damm,
	biju.das.jz, linux-watchdog, devicetree, linux-kernel,
	linux-renesas-soc, Claudiu Beznea
In-Reply-To: <20240410134044.2138310-2-claudiu.beznea.uj@bp.renesas.com>

On Wed, Apr 10, 2024 at 04:40:35PM +0300, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> The rzg2l_wdt driver is used only by ARCH_RZG2L and ARCH_R9A09G011
> micro-architectures of Renesas. Thus, limit it's usage only to these.
> 
> Suggested-by: Biju Das <biju.das.jz@bp.renesas.com>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> 
> Changes in v8:
> - none
> 
> Changes in v7:
> - none
> 
> Changes in v6:
> - none
> 
> Changes in v5:
> - none
> 
> Changes in v4:
> - none; this patch is introduced in v4
> 
>  drivers/watchdog/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 6bee137cfbe0..e2439967417a 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -920,7 +920,7 @@ config RENESAS_RZN1WDT
>  
>  config RENESAS_RZG2LWDT
>  	tristate "Renesas RZ/G2L WDT Watchdog"
> -	depends on ARCH_RENESAS || COMPILE_TEST
> +	depends on ARCH_RZG2L || ARCH_R9A09G011 || COMPILE_TEST
>  	select WATCHDOG_CORE
>  	help
>  	  This driver adds watchdog support for the integrated watchdogs in the
> -- 
> 2.39.2
> 

^ permalink raw reply

* Re: [PATCH RESEND v8 02/10] watchdog: rzg2l_wdt: Make the driver depend on PM
From: Guenter Roeck @ 2024-04-10 16:39 UTC (permalink / raw)
  To: Claudiu
  Cc: wim, robh, krzk+dt, conor+dt, p.zabel, geert+renesas, magnus.damm,
	biju.das.jz, linux-watchdog, devicetree, linux-kernel,
	linux-renesas-soc, Claudiu Beznea, kernel test robot
In-Reply-To: <20240410134044.2138310-3-claudiu.beznea.uj@bp.renesas.com>

On Wed, Apr 10, 2024 at 04:40:36PM +0300, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> The rzg2l_wdt watchdog driver cannot work w/o CONFIG_PM=y (e.g. the
> clocks are enabled though pm_runtime_* specific APIs). To avoid building
> a driver that doesn't work make explicit the dependency on CONFIG_PM.
> 
> Suggested-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> 
> Changes in v8:
> - collected tags
> 
> Changes in v7:
> - updated the dependency to PM || COMPILE_TEST to be able to
>   compile-test the driver when compiling for a
>   !(ARCH_RZG2L || ARCH_R9A09G011) platform and CONFIG_PM is disabled
> 
> Changes in v6:
> - update patch description
> - fixed the dependency on COMPILE_TEST previously introduced
> 
> Changes in v5:
> - updated patch description
> - added on a new line the dependency on PM and COMPILE_TEST
> 
> Changes in v4:
> - s/ARCH_RENESAS/ARCH_RZG2L || ARCH_R9A09G011 due to patch 1/9
> 
> Changes in v3:
> - make driver depend on PM; with that the "unmet direct dependency"
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/linux-devicetree/202402020445.TOBlFPcS-lkp@intel.com
>   was also fixed
> - adapt commit message
> 
> Changes in v2:
> - this patch is new
> 
>  drivers/watchdog/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index e2439967417a..7b66fe06ac85 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -921,6 +921,7 @@ config RENESAS_RZN1WDT
>  config RENESAS_RZG2LWDT
>  	tristate "Renesas RZ/G2L WDT Watchdog"
>  	depends on ARCH_RZG2L || ARCH_R9A09G011 || COMPILE_TEST
> +	depends on PM || COMPILE_TEST
>  	select WATCHDOG_CORE
>  	help
>  	  This driver adds watchdog support for the integrated watchdogs in the
> -- 
> 2.39.2
> 

^ permalink raw reply

* Re: [PATCH RESEND v8 03/10] watchdog: rzg2l_wdt: Use pm_runtime_resume_and_get()
From: Guenter Roeck @ 2024-04-10 16:39 UTC (permalink / raw)
  To: Claudiu
  Cc: wim, robh, krzk+dt, conor+dt, p.zabel, geert+renesas, magnus.damm,
	biju.das.jz, linux-watchdog, devicetree, linux-kernel,
	linux-renesas-soc, Claudiu Beznea
In-Reply-To: <20240410134044.2138310-4-claudiu.beznea.uj@bp.renesas.com>

On Wed, Apr 10, 2024 at 04:40:37PM +0300, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> pm_runtime_get_sync() may return with error. In case it returns with error
> dev->power.usage_count needs to be decremented. pm_runtime_resume_and_get()
> takes care of this. Thus use it.
> 
> Along with it the rzg2l_wdt_set_timeout() function was updated to
> propagate the result of rzg2l_wdt_start() to its caller.
> 
> Fixes: 2cbc5cd0b55f ("watchdog: Add Watchdog Timer driver for RZ/G2L")
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> 
> Changes in v8:
> - none
> 
> Changes in v7:
> - none
> 
> Changes in v6:
> - none
> 
> Changes in v5:
> - none
> 
> Changes in v4:
> - none
> 
> Changes in v3:
> - none
> 
> Changes in v2:
> - propagate the return code of rzg2l_wdt_start() to it's callers
> 
> 
>  drivers/watchdog/rzg2l_wdt.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/watchdog/rzg2l_wdt.c b/drivers/watchdog/rzg2l_wdt.c
> index 1741f98ca67c..d87d4f50180c 100644
> --- a/drivers/watchdog/rzg2l_wdt.c
> +++ b/drivers/watchdog/rzg2l_wdt.c
> @@ -123,8 +123,11 @@ static void rzg2l_wdt_init_timeout(struct watchdog_device *wdev)
>  static int rzg2l_wdt_start(struct watchdog_device *wdev)
>  {
>  	struct rzg2l_wdt_priv *priv = watchdog_get_drvdata(wdev);
> +	int ret;
>  
> -	pm_runtime_get_sync(wdev->parent);
> +	ret = pm_runtime_resume_and_get(wdev->parent);
> +	if (ret)
> +		return ret;
>  
>  	/* Initialize time out */
>  	rzg2l_wdt_init_timeout(wdev);
> @@ -150,6 +153,8 @@ static int rzg2l_wdt_stop(struct watchdog_device *wdev)
>  
>  static int rzg2l_wdt_set_timeout(struct watchdog_device *wdev, unsigned int timeout)
>  {
> +	int ret = 0;
> +
>  	wdev->timeout = timeout;
>  
>  	/*
> @@ -159,10 +164,10 @@ static int rzg2l_wdt_set_timeout(struct watchdog_device *wdev, unsigned int time
>  	 */
>  	if (watchdog_active(wdev)) {
>  		rzg2l_wdt_stop(wdev);
> -		rzg2l_wdt_start(wdev);
> +		ret = rzg2l_wdt_start(wdev);
>  	}
>  
> -	return 0;
> +	return ret;
>  }
>  
>  static int rzg2l_wdt_restart(struct watchdog_device *wdev,
> -- 
> 2.39.2
> 

^ permalink raw reply

* Re: [PATCH 02/17] dt-bindings: soc: google: exynos-sysreg: add dedicated hsi2 sysreg compatible
From: Rob Herring @ 2024-04-10 16:40 UTC (permalink / raw)
  To: Peter Griffin
  Cc: avri.altman, linux-phy, vkoul, martin.petersen, alim.akhtar,
	devicetree, willmcvicker, linux-scsi, mturquette, conor+dt,
	ebiggers, kishon, sboyd, krzk+dt, jejb, linux-arm-kernel,
	saravanak, andre.draszik, linux-clk, bvanassche,
	linux-samsung-soc, chanho61.park, s.nawrocki, linux-kernel,
	cw00.choi, tudor.ambarus
In-Reply-To: <20240404122559.898930-3-peter.griffin@linaro.org>


On Thu, 04 Apr 2024 13:25:44 +0100, Peter Griffin wrote:
> Update dt schema to include the gs101 hsi2 sysreg compatible.
> 
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> ---
>  .../devicetree/bindings/soc/samsung/samsung,exynos-sysreg.yaml  | 2 ++
>  1 file changed, 2 insertions(+)
> 

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


^ permalink raw reply

* Re: [PATCH RESEND v8 04/10] watchdog: rzg2l_wdt: Check return status of pm_runtime_put()
From: Guenter Roeck @ 2024-04-10 16:41 UTC (permalink / raw)
  To: Claudiu
  Cc: wim, robh, krzk+dt, conor+dt, p.zabel, geert+renesas, magnus.damm,
	biju.das.jz, linux-watchdog, devicetree, linux-kernel,
	linux-renesas-soc, Claudiu Beznea
In-Reply-To: <20240410134044.2138310-5-claudiu.beznea.uj@bp.renesas.com>

On Wed, Apr 10, 2024 at 04:40:38PM +0300, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> pm_runtime_put() may return an error code. Check its return status.
> 
> Along with it the rzg2l_wdt_set_timeout() function was updated to
> propagate the result of rzg2l_wdt_stop() to its caller.
> 
> Fixes: 2cbc5cd0b55f ("watchdog: Add Watchdog Timer driver for RZ/G2L")
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
> 
> Changes in v8:
> - none
> 
> Changes in v7:
> - none
> 
> Changes in v6:
> - none
> 
> Changes in v5:
> - none
> 
> Changes in v4:
> - none
> 
> Changes in v3:
> - none
> 
> Changes in v2:
> - propagate the return code of rzg2l_wdt_stop() to it's callers
> 
> 
>  drivers/watchdog/rzg2l_wdt.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/watchdog/rzg2l_wdt.c b/drivers/watchdog/rzg2l_wdt.c
> index d87d4f50180c..7bce093316c4 100644
> --- a/drivers/watchdog/rzg2l_wdt.c
> +++ b/drivers/watchdog/rzg2l_wdt.c
> @@ -144,9 +144,13 @@ static int rzg2l_wdt_start(struct watchdog_device *wdev)
>  static int rzg2l_wdt_stop(struct watchdog_device *wdev)
>  {
>  	struct rzg2l_wdt_priv *priv = watchdog_get_drvdata(wdev);
> +	int ret;
>  
>  	rzg2l_wdt_reset(priv);
> -	pm_runtime_put(wdev->parent);
> +
> +	ret = pm_runtime_put(wdev->parent);
> +	if (ret < 0)
> +		return ret;

Nit:
	return pm_runtime_put(wdev->parent);

would have been sufficient.

Nevertheless,

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

>  
>  	return 0;
>  }
> @@ -163,7 +167,10 @@ static int rzg2l_wdt_set_timeout(struct watchdog_device *wdev, unsigned int time
>  	 * to reset the module) so that it is updated with new timeout values.
>  	 */
>  	if (watchdog_active(wdev)) {
> -		rzg2l_wdt_stop(wdev);
> +		ret = rzg2l_wdt_stop(wdev);
> +		if (ret)
> +			return ret;
> +
>  		ret = rzg2l_wdt_start(wdev);
>  	}
>  
> -- 
> 2.39.2
> 

^ permalink raw reply

* Re: [PATCH RESEND v8 05/10] watchdog: rzg2l_wdt: Remove reset de-assert from probe
From: Guenter Roeck @ 2024-04-10 16:41 UTC (permalink / raw)
  To: Claudiu
  Cc: wim, robh, krzk+dt, conor+dt, p.zabel, geert+renesas, magnus.damm,
	biju.das.jz, linux-watchdog, devicetree, linux-kernel,
	linux-renesas-soc, Claudiu Beznea
In-Reply-To: <20240410134044.2138310-6-claudiu.beznea.uj@bp.renesas.com>

On Wed, Apr 10, 2024 at 04:40:39PM +0300, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> There is no need to de-assert the reset signal on probe as the watchdog
> is not used prior executing start. Also, the clocks are not enabled in
> probe (pm_runtime_enable() doesn't do that), thus this is another indicator
> that the watchdog wasn't used previously like this. Instead, keep the
> watchdog hardware in its previous state at probe (by default it is in
> reset state), enable it when it is started and move it to reset state
> when it is stopped. This saves some extra power when the watchdog is
> unused.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> 
> Changes in v8:
> - none
> 
> Changes in v7:
> - none
> 
> Changes in v6:
> - none
> 
> Changes in v5:
> - update patch title
> 
> Changes in v4:
> - none
> 
> Changes in v3:
> - none
> 
> Changes in v2:
> - none
> 
>  drivers/watchdog/rzg2l_wdt.c | 26 +++++++++++++++-----------
>  1 file changed, 15 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/watchdog/rzg2l_wdt.c b/drivers/watchdog/rzg2l_wdt.c
> index 7bce093316c4..93a49fd0c7aa 100644
> --- a/drivers/watchdog/rzg2l_wdt.c
> +++ b/drivers/watchdog/rzg2l_wdt.c
> @@ -129,6 +129,10 @@ static int rzg2l_wdt_start(struct watchdog_device *wdev)
>  	if (ret)
>  		return ret;
>  
> +	ret = reset_control_deassert(priv->rstc);
> +	if (ret)
> +		return ret;
> +
>  	/* Initialize time out */
>  	rzg2l_wdt_init_timeout(wdev);
>  
> @@ -146,7 +150,9 @@ static int rzg2l_wdt_stop(struct watchdog_device *wdev)
>  	struct rzg2l_wdt_priv *priv = watchdog_get_drvdata(wdev);
>  	int ret;
>  
> -	rzg2l_wdt_reset(priv);
> +	ret = reset_control_assert(priv->rstc);
> +	if (ret)
> +		return ret;
>  
>  	ret = pm_runtime_put(wdev->parent);
>  	if (ret < 0)
> @@ -186,6 +192,12 @@ static int rzg2l_wdt_restart(struct watchdog_device *wdev,
>  	clk_prepare_enable(priv->osc_clk);
>  
>  	if (priv->devtype == WDT_RZG2L) {
> +		int ret;
> +
> +		ret = reset_control_deassert(priv->rstc);
> +		if (ret)
> +			return ret;
> +
>  		/* Generate Reset (WDTRSTB) Signal on parity error */
>  		rzg2l_wdt_write(priv, 0, PECR);
>  
> @@ -236,13 +248,11 @@ static const struct watchdog_ops rzg2l_wdt_ops = {
>  	.restart = rzg2l_wdt_restart,
>  };
>  
> -static void rzg2l_wdt_reset_assert_pm_disable(void *data)
> +static void rzg2l_wdt_pm_disable(void *data)
>  {
>  	struct watchdog_device *wdev = data;
> -	struct rzg2l_wdt_priv *priv = watchdog_get_drvdata(wdev);
>  
>  	pm_runtime_disable(wdev->parent);
> -	reset_control_assert(priv->rstc);
>  }
>  
>  static int rzg2l_wdt_probe(struct platform_device *pdev)
> @@ -285,10 +295,6 @@ static int rzg2l_wdt_probe(struct platform_device *pdev)
>  		return dev_err_probe(&pdev->dev, PTR_ERR(priv->rstc),
>  				     "failed to get cpg reset");
>  
> -	ret = reset_control_deassert(priv->rstc);
> -	if (ret)
> -		return dev_err_probe(dev, ret, "failed to deassert");
> -
>  	priv->devtype = (uintptr_t)of_device_get_match_data(dev);
>  
>  	if (priv->devtype == WDT_RZV2M) {
> @@ -309,9 +315,7 @@ static int rzg2l_wdt_probe(struct platform_device *pdev)
>  	priv->wdev.timeout = WDT_DEFAULT_TIMEOUT;
>  
>  	watchdog_set_drvdata(&priv->wdev, priv);
> -	ret = devm_add_action_or_reset(&pdev->dev,
> -				       rzg2l_wdt_reset_assert_pm_disable,
> -				       &priv->wdev);
> +	ret = devm_add_action_or_reset(&pdev->dev, rzg2l_wdt_pm_disable, &priv->wdev);
>  	if (ret < 0)
>  		return ret;
>  
> -- 
> 2.39.2
> 

^ permalink raw reply

* Re: [PATCH RESEND v8 06/10] watchdog: rzg2l_wdt: Remove comparison with zero
From: Guenter Roeck @ 2024-04-10 16:41 UTC (permalink / raw)
  To: Claudiu
  Cc: wim, robh, krzk+dt, conor+dt, p.zabel, geert+renesas, magnus.damm,
	biju.das.jz, linux-watchdog, devicetree, linux-kernel,
	linux-renesas-soc, Claudiu Beznea
In-Reply-To: <20240410134044.2138310-7-claudiu.beznea.uj@bp.renesas.com>

On Wed, Apr 10, 2024 at 04:40:40PM +0300, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> devm_add_action_or_reset() could return -ENOMEM or zero. Thus, remove
> comparison with zero of the returning value to make code simpler.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> 
> Changes in v8:
> - none
> 
> Changes in v7:
> - none
> 
> Changes in v6:
> - none
> 
> Changes in v5:
> - none
> 
> Changes in v4:
> - none
> 
> Changes in v3:
> - none
> 
> Changes in v2:
> - none
> 
> 
>  drivers/watchdog/rzg2l_wdt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/rzg2l_wdt.c b/drivers/watchdog/rzg2l_wdt.c
> index 93a49fd0c7aa..29eb47bcf984 100644
> --- a/drivers/watchdog/rzg2l_wdt.c
> +++ b/drivers/watchdog/rzg2l_wdt.c
> @@ -316,7 +316,7 @@ static int rzg2l_wdt_probe(struct platform_device *pdev)
>  
>  	watchdog_set_drvdata(&priv->wdev, priv);
>  	ret = devm_add_action_or_reset(&pdev->dev, rzg2l_wdt_pm_disable, &priv->wdev);
> -	if (ret < 0)
> +	if (ret)
>  		return ret;
>  
>  	watchdog_set_nowayout(&priv->wdev, nowayout);
> -- 
> 2.39.2
> 

^ permalink raw reply

* Re: [PATCH RESEND v8 07/10] watchdog: rzg2l_wdt: Rely on the reset driver for doing proper reset
From: Guenter Roeck @ 2024-04-10 16:42 UTC (permalink / raw)
  To: Claudiu
  Cc: wim, robh, krzk+dt, conor+dt, p.zabel, geert+renesas, magnus.damm,
	biju.das.jz, linux-watchdog, devicetree, linux-kernel,
	linux-renesas-soc, Claudiu Beznea
In-Reply-To: <20240410134044.2138310-8-claudiu.beznea.uj@bp.renesas.com>

On Wed, Apr 10, 2024 at 04:40:41PM +0300, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> The reset driver has been adapted in commit da235d2fac21
> ("clk: renesas: rzg2l: Check reset monitor registers") to check the reset
> monitor bits before declaring reset asserts/de-asserts as
> successful/failure operations. With that, there is no need to keep the
> reset workaround for RZ/V2M in place in the watchdog driver.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> 
> Changes in v8:
> - none
> 
> Changes in v7:
> - none
> 
> Changes in v6:
> - none
> 
> Changes in v5:
> - none
> 
> Changes in v4:
> - collected tag
> 
> Changes in v3:
> - none
> 
> Changes in v2:
> - none
> 
>  drivers/watchdog/rzg2l_wdt.c | 39 ++++--------------------------------
>  1 file changed, 4 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/watchdog/rzg2l_wdt.c b/drivers/watchdog/rzg2l_wdt.c
> index 29eb47bcf984..42f1d5d6f07e 100644
> --- a/drivers/watchdog/rzg2l_wdt.c
> +++ b/drivers/watchdog/rzg2l_wdt.c
> @@ -8,7 +8,6 @@
>  #include <linux/clk.h>
>  #include <linux/delay.h>
>  #include <linux/io.h>
> -#include <linux/iopoll.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> @@ -54,35 +53,11 @@ struct rzg2l_wdt_priv {
>  	struct reset_control *rstc;
>  	unsigned long osc_clk_rate;
>  	unsigned long delay;
> -	unsigned long minimum_assertion_period;
>  	struct clk *pclk;
>  	struct clk *osc_clk;
>  	enum rz_wdt_type devtype;
>  };
>  
> -static int rzg2l_wdt_reset(struct rzg2l_wdt_priv *priv)
> -{
> -	int err, status;
> -
> -	if (priv->devtype == WDT_RZV2M) {
> -		/* WDT needs TYPE-B reset control */
> -		err = reset_control_assert(priv->rstc);
> -		if (err)
> -			return err;
> -		ndelay(priv->minimum_assertion_period);
> -		err = reset_control_deassert(priv->rstc);
> -		if (err)
> -			return err;
> -		err = read_poll_timeout(reset_control_status, status,
> -					status != 1, 0, 1000, false,
> -					priv->rstc);
> -	} else {
> -		err = reset_control_reset(priv->rstc);
> -	}
> -
> -	return err;
> -}
> -
>  static void rzg2l_wdt_wait_delay(struct rzg2l_wdt_priv *priv)
>  {
>  	/* delay timer when change the setting register */
> @@ -187,13 +162,12 @@ static int rzg2l_wdt_restart(struct watchdog_device *wdev,
>  			     unsigned long action, void *data)
>  {
>  	struct rzg2l_wdt_priv *priv = watchdog_get_drvdata(wdev);
> +	int ret;
>  
>  	clk_prepare_enable(priv->pclk);
>  	clk_prepare_enable(priv->osc_clk);
>  
>  	if (priv->devtype == WDT_RZG2L) {
> -		int ret;
> -
>  		ret = reset_control_deassert(priv->rstc);
>  		if (ret)
>  			return ret;
> @@ -205,7 +179,9 @@ static int rzg2l_wdt_restart(struct watchdog_device *wdev,
>  		rzg2l_wdt_write(priv, PEEN_FORCE, PEEN);
>  	} else {
>  		/* RZ/V2M doesn't have parity error registers */
> -		rzg2l_wdt_reset(priv);
> +		ret = reset_control_reset(priv->rstc);
> +		if (ret)
> +			return ret;
>  
>  		wdev->timeout = 0;
>  
> @@ -297,13 +273,6 @@ static int rzg2l_wdt_probe(struct platform_device *pdev)
>  
>  	priv->devtype = (uintptr_t)of_device_get_match_data(dev);
>  
> -	if (priv->devtype == WDT_RZV2M) {
> -		priv->minimum_assertion_period = RZV2M_A_NSEC +
> -			3 * F2CYCLE_NSEC(pclk_rate) + 5 *
> -			max(F2CYCLE_NSEC(priv->osc_clk_rate),
> -			    F2CYCLE_NSEC(pclk_rate));
> -	}
> -
>  	pm_runtime_enable(&pdev->dev);
>  
>  	priv->wdev.info = &rzg2l_wdt_ident;
> -- 
> 2.39.2
> 

^ permalink raw reply

* Re: [PATCH RESEND v8 08/10] watchdog: rzg2l_wdt: Add suspend/resume support
From: Guenter Roeck @ 2024-04-10 16:42 UTC (permalink / raw)
  To: Claudiu
  Cc: wim, robh, krzk+dt, conor+dt, p.zabel, geert+renesas, magnus.damm,
	biju.das.jz, linux-watchdog, devicetree, linux-kernel,
	linux-renesas-soc, Claudiu Beznea
In-Reply-To: <20240410134044.2138310-9-claudiu.beznea.uj@bp.renesas.com>

On Wed, Apr 10, 2024 at 04:40:42PM +0300, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> The RZ/G3S supports deep sleep states where power to most of the IP blocks
> is cut off. To ensure proper working of the watchdog when resuming from
> such states, the suspend function is stopping the watchdog and the resume
> function is starting it. There is no need to configure the watchdog
> in case the watchdog was stopped prior to starting suspend.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> 
> Changes in v8:
> - none
> 
> Changes in v7:
> - none
> 
> Changes in v6:
> - none
> 
> Changes in v5:
> - none
> 
> Changes in v4:
> - none
> 
> Changes in v3:
> - none
> 
> Changes in v2:
> - remove the usage of pm_ptr()
> 
> 
>  drivers/watchdog/rzg2l_wdt.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/drivers/watchdog/rzg2l_wdt.c b/drivers/watchdog/rzg2l_wdt.c
> index 42f1d5d6f07e..c8c20cfb97a3 100644
> --- a/drivers/watchdog/rzg2l_wdt.c
> +++ b/drivers/watchdog/rzg2l_wdt.c
> @@ -284,6 +284,7 @@ static int rzg2l_wdt_probe(struct platform_device *pdev)
>  	priv->wdev.timeout = WDT_DEFAULT_TIMEOUT;
>  
>  	watchdog_set_drvdata(&priv->wdev, priv);
> +	dev_set_drvdata(dev, priv);
>  	ret = devm_add_action_or_reset(&pdev->dev, rzg2l_wdt_pm_disable, &priv->wdev);
>  	if (ret)
>  		return ret;
> @@ -305,10 +306,35 @@ static const struct of_device_id rzg2l_wdt_ids[] = {
>  };
>  MODULE_DEVICE_TABLE(of, rzg2l_wdt_ids);
>  
> +static int rzg2l_wdt_suspend_late(struct device *dev)
> +{
> +	struct rzg2l_wdt_priv *priv = dev_get_drvdata(dev);
> +
> +	if (!watchdog_active(&priv->wdev))
> +		return 0;
> +
> +	return rzg2l_wdt_stop(&priv->wdev);
> +}
> +
> +static int rzg2l_wdt_resume_early(struct device *dev)
> +{
> +	struct rzg2l_wdt_priv *priv = dev_get_drvdata(dev);
> +
> +	if (!watchdog_active(&priv->wdev))
> +		return 0;
> +
> +	return rzg2l_wdt_start(&priv->wdev);
> +}
> +
> +static const struct dev_pm_ops rzg2l_wdt_pm_ops = {
> +	LATE_SYSTEM_SLEEP_PM_OPS(rzg2l_wdt_suspend_late, rzg2l_wdt_resume_early)
> +};
> +
>  static struct platform_driver rzg2l_wdt_driver = {
>  	.driver = {
>  		.name = "rzg2l_wdt",
>  		.of_match_table = rzg2l_wdt_ids,
> +		.pm = &rzg2l_wdt_pm_ops,
>  	},
>  	.probe = rzg2l_wdt_probe,
>  };
> -- 
> 2.39.2
> 

^ permalink raw reply

* Re: [PATCH RESEND v8 09/10] watchdog: rzg2l_wdt: Power on the PM domain in rzg2l_wdt_restart()
From: Guenter Roeck @ 2024-04-10 16:43 UTC (permalink / raw)
  To: Claudiu
  Cc: wim, robh, krzk+dt, conor+dt, p.zabel, geert+renesas, magnus.damm,
	biju.das.jz, linux-watchdog, devicetree, linux-kernel,
	linux-renesas-soc, Claudiu Beznea
In-Reply-To: <20240410134044.2138310-10-claudiu.beznea.uj@bp.renesas.com>

On Wed, Apr 10, 2024 at 04:40:43PM +0300, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> The rzg2l_wdt_restart() is called from atomic context. Calling
> pm_runtime_{get_sync, resume_and_get}() or any other runtime PM resume
> APIs is not an option as it may lead to issues as described in commit
> e4cf89596c1f ("watchdog: rzg2l_wdt: Fix 'BUG: Invalid wait context'")
> that removed the pm_runtime_get_sync() and used directly the
> clk_prepare_enable() APIs.
> 
> Starting with RZ/G3S the watchdog could be part of its own software
> controlled power domain (see the initial implementation in Link section).
> In case the watchdog is not used the power domain is off and accessing
> watchdog registers leads to aborts.
> 
> To solve this the patch powers on the power domain using
> dev_pm_genpd_resume() API before enabling its clock. This is not
> sleeping or taking any other locks as the power domain will not be
> registered with GENPD_FLAG_IRQ_SAFE flags.
> 
> Link: https://lore.kernel.org/all/20240208124300.2740313-1-claudiu.beznea.uj@bp.renesas.com
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> 
> Changes in v8:
> - none, this patch is new
> 
>  drivers/watchdog/rzg2l_wdt.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/watchdog/rzg2l_wdt.c b/drivers/watchdog/rzg2l_wdt.c
> index c8c20cfb97a3..98e5e9914a5d 100644
> --- a/drivers/watchdog/rzg2l_wdt.c
> +++ b/drivers/watchdog/rzg2l_wdt.c
> @@ -12,6 +12,7 @@
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/reset.h>
>  #include <linux/units.h>
> @@ -164,6 +165,17 @@ static int rzg2l_wdt_restart(struct watchdog_device *wdev,
>  	struct rzg2l_wdt_priv *priv = watchdog_get_drvdata(wdev);
>  	int ret;
>  
> +	/*
> +	 * The device may be part of a power domain that is currently
> +	 * powered off. We need to power it up before accessing registers.
> +	 * We don't undo the dev_pm_genpd_resume() as the device need to
> +	 * be up for the reboot to happen. Also, as we are in atomic context
> +	 * here there is no need to increment PM runtime usage counter
> +	 * (to make sure pm_runtime_active() doesn't return wrong code).
> +	 */
> +	if (!pm_runtime_active(wdev->parent))
> +		dev_pm_genpd_resume(wdev->parent);
> +
>  	clk_prepare_enable(priv->pclk);
>  	clk_prepare_enable(priv->osc_clk);
>  
> -- 
> 2.39.2
> 

^ permalink raw reply

* Re: [PATCH RESEND v8 10/10] dt-bindings: watchdog: renesas,wdt: Document RZ/G3S support
From: Guenter Roeck @ 2024-04-10 16:43 UTC (permalink / raw)
  To: Claudiu
  Cc: wim, robh, krzk+dt, conor+dt, p.zabel, geert+renesas, magnus.damm,
	biju.das.jz, linux-watchdog, devicetree, linux-kernel,
	linux-renesas-soc, Claudiu Beznea, Conor Dooley
In-Reply-To: <20240410134044.2138310-11-claudiu.beznea.uj@bp.renesas.com>

On Wed, Apr 10, 2024 at 04:40:44PM +0300, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> Document the support for the watchdog IP available on RZ/G3S SoC. The
> watchdog IP available on RZ/G3S SoC is identical to the one found on
> RZ/G2L SoC.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> 
> Changes in v8:
> - none
> 
> Changes in v7:
> - none
> 
> Changes in v6:
> - none
> 
> Changes in v5:
> - none
> 
> Changes in v4:
> - none
> 
> Changes in v3:
> - re-arranged the tags as my b4 am/shazam placed previously the
>   Ab, Rb tags before the author's Sob
> 
> Changes in v2:
> - collected tags
> - s/G2UL/G2L in patch description
> 
>  Documentation/devicetree/bindings/watchdog/renesas,wdt.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/renesas,wdt.yaml b/Documentation/devicetree/bindings/watchdog/renesas,wdt.yaml
> index ffb17add491a..eba454d1680f 100644
> --- a/Documentation/devicetree/bindings/watchdog/renesas,wdt.yaml
> +++ b/Documentation/devicetree/bindings/watchdog/renesas,wdt.yaml
> @@ -29,6 +29,7 @@ properties:
>                - renesas,r9a07g043-wdt    # RZ/G2UL and RZ/Five
>                - renesas,r9a07g044-wdt    # RZ/G2{L,LC}
>                - renesas,r9a07g054-wdt    # RZ/V2L
> +              - renesas,r9a08g045-wdt    # RZ/G3S
>            - const: renesas,rzg2l-wdt
>  
>        - items:
> -- 
> 2.39.2
> 

^ permalink raw reply

* Re: DT Query on "New Compatible vs New Property"
From: Nikunj Kela @ 2024-04-10 16:53 UTC (permalink / raw)
  To: Trilok Soni, Srinivas Kandagatla, Sudeep Holla, Ulf Hansson
  Cc: Krzysztof Kozlowski, Manivannan Sadhasivam,
	krzysztof.kozlowski+dt, Vincent Guittot, robh+dt, conor+dt,
	devicetree, Prasad Sodagudi (QUIC)
In-Reply-To: <c02952a7-20a1-be77-35a5-d6a8b760e2f0@quicinc.com>


On 3/13/2024 3:40 PM, Trilok Soni wrote:
> On 3/13/2024 4:49 AM, Srinivas Kandagatla wrote:
>> On 12/03/2024 17:25, Trilok Soni wrote:
>>> On 3/12/2024 10:21 AM, Srinivas Kandagatla wrote:
>>>>> Basically, I would prefer better than "qcom, fw-managed" since this is not
>>>>> a qcom specific problem.
>>>>
>>>> We already have something like this in mainline where the BAM DMA controller is remotely powered.
>>>>
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml?h=v6.8
>>>>
>>> As you can see it is already fragmented. Why we need to create one more approach
>>> which is not scalable and specific to SOC vendor?
>> The whole issue around this new checks is that the driver/binding is not designed to expect same set of resources from different TYPES of providers.
>> If the driver was designed to support opp's and power domains and make the resources handle in a unified way then some/all of these changes will naturally fit in.
>>
>>
>>> SCMI or RPMI based firmware is not a QC specific. I also have allergic reaction
>> I agree this are not QC specific, am fine with generic dt-binding like firmware-managed-resources or something on those lines if DT-maintainers are happy with.
>>
>> What is your suggestion?
> Yes, DT-spec will be a good start.
>
>>
>>> when I see drivers modified w/ if (fw_managed) {..} but that is a discussion
>> I don't think we have a choice here, either we do this check at compatible level or dt-property level or resource level in every drivers.
> I don't understand yet why we don't have any other choices but do the conditional checks? Maybe explaining
> with the example will help? Start w/ clocks?

We are not using SCMI in traditional way in that, we are not dealing 
with individual resources(clocks, regulators, phy, gpio, interconnect 
etc.). We are defining logical/modelled domains(power and performance). 
We need to think in terms of device states. For example, emac device 
might have two states like this:

Clock C1 running at frequency F1 + Regulators R running at voltage V1 + 
Interconnect I running at B1 bandwidth --> Lets call it a level-1 (for 
the performance domain)

Clock C1 running at frquuency F2 + Regulators R running at voltage V1 + 
Interconnect I running at B1 bandwidth --> Lets call it a level-2

Now if the emac driver needs to set the device performance state to 
Level-1, it will call the OPP APIs to set the device performance state 
to Level-1.

Similarly, emac may also define power domains like this:

Power_on --> enable ‘n’ clocks + enable ‘m’ regulators + set ICB 
bandwidth to default ‘b’ + setting some GPIO pin + Enabling n` phy 
clocks + enabling m` phy regulators + follow phy_init sequence and 
caliberation

Power_off --> does just the opposite.

So now you can see dealing with individual clocks etc. would not be 
always possible in our scenario. Therefore, we need to use ‘qcom, 
firmware-managed-resources’ property to figure out if we are abstracting 
resources or not.
>

^ permalink raw reply

* Re: DT Query on "New Compatible vs New Property"
From: Nikunj Kela @ 2024-04-10 16:55 UTC (permalink / raw)
  To: Sudeep Holla, Srinivas Kandagatla
  Cc: Trilok Soni, Ulf Hansson, Krzysztof Kozlowski,
	Manivannan Sadhasivam, krzysztof.kozlowski+dt, Vincent Guittot,
	robh+dt, conor+dt, devicetree, Prasad Sodagudi (QUIC)
In-Reply-To: <Zfm5m2zp4A06ZunW@bogus>


On 3/19/2024 9:13 AM, Sudeep Holla wrote:
> On Tue, Mar 19, 2024 at 03:41:40PM +0000, Srinivas Kandagatla wrote:
>> On 19/03/2024 15:17, Sudeep Holla wrote:
>>> I am not debating on the implementation just to be clear. I accept changes
>>> might be needed there. The $subject is all about DT bindings and what need
>>> to be changes and for me nothing, just use existing bindings and if there
>>> are issues there, let us discuss it with specifics.
>>>
>> How can changes to dt bindings be nothing? All the resources
>> clk/regulators/resets will become optional and a new power or perf domain
>> will become required for each device with firmwares that support SCMI Perf.
>>
> Correct, sorry to miss the point that few properties are now optional from
> mandatory before. Very good point. I was so caught up with the addition of
> the new "firmware controlled blah blah" property/compatible that I missed
> to observe mandatory->optional as a change. Thanks for correcting me.
>
> --
> Regards,
> Sudeep
If there are no more questions on this and everyone is on the same page, 
I would like to conclude this thread in favor of using a new DT property 
'qcom,firmware-managed-resources'.

^ permalink raw reply

* Re: [RFC PATCH 2/2] PCI: Add Qualcomm PCIe ECAM root complex driver
From: Rob Herring @ 2024-04-10 16:58 UTC (permalink / raw)
  To: Mayank Rana
  Cc: Manivannan Sadhasivam, linux-pci, lpieralisi, kw, bhelgaas,
	andersson, krzysztof.kozlowski+dt, conor+dt, devicetree,
	linux-arm-msm, quic_ramkri, quic_nkela, quic_shazhuss,
	quic_msarkar, quic_nitegupt
In-Reply-To: <0b738556-0042-43ab-80f2-d78ed3b432f7@quicinc.com>

On Mon, Apr 08, 2024 at 11:57:58AM -0700, Mayank Rana wrote:
> Hi Mani
> 
> On 4/5/2024 9:17 PM, Manivannan Sadhasivam wrote:
> > On Fri, Apr 05, 2024 at 10:41:15AM -0700, Mayank Rana wrote:
> > > Hi Mani
> > > 
> > > On 4/4/2024 10:30 PM, Manivannan Sadhasivam wrote:
> > > > On Thu, Apr 04, 2024 at 12:11:24PM -0700, Mayank Rana wrote:
> > > > > On some of Qualcomm platform, firmware configures PCIe controller into
> > > > > ECAM mode allowing static memory allocation for configuration space of
> > > > > supported bus range. Firmware also takes care of bringing up PCIe PHY
> > > > > and performing required operation to bring PCIe link into D0. Firmware
> > > > > also manages system resources (e.g. clocks/regulators/resets/ bus voting).
> > > > > Hence add Qualcomm PCIe ECAM root complex driver which enumerates PCIe
> > > > > root complex and connected PCIe devices. Firmware won't be enumerating
> > > > > or powering up PCIe root complex until this driver invokes power domain
> > > > > based notification to bring PCIe link into D0/D3cold mode.
> > > > > 
> > > > 
> > > > Is this an in-house PCIe IP of Qualcomm or the same DWC IP that is used in other
> > > > SoCs?
> > > > 
> > > > - Mani
> > > Driver is validated on SA8775p-ride platform using PCIe DWC IP for
> > > now.Although this driver doesn't need to know used PCIe controller and PHY
> > > IP as well programming sequence as that would be taken care by firmware.
> > > 
> > 
> > Ok, so it is the same IP but firmware is controlling the resources now. This
> > information should be present in the commit message.
> > 
> > Btw, there is an existing generic ECAM host controller driver:
> > drivers/pci/controller/pci-host-generic.c
> > 
> > This driver is already being used by several vendors as well. So we should try
> > to extend it for Qcom usecase also.

I would take it a bit further and say if you need your own driver, then 
just use the default QCom driver. Perhaps extend it to support ECAM. 
Better yet, copy your firmware setup and always configure the QCom h/w 
to use ECAM.

If you want to extend the generic driver, that's fine, but we don't need 
a 3rd.

> I did review pci-host-generic.c driver for usage. although there are more
> functionalityneeded for use case purpose as below:
> 1. MSI functionality

Pretty sure the generic driver already supports that.

> 2. Suspend/Resume

Others might want that to work as well.

> 3. Wakeup Functionality (not part of current change, but would be added
> later)

Others might want that to work as well.

> 4. Here this driver provides way to virtualized PCIe controller. So VMs only
> talk to a generic ECAM whereas HW is only directed accessed by service VM.

That's the existing driver. If if doesn't work for a VM, fix the VM.

> 5. Adding more Auto based safety use cases related implementation

Now that's just hand waving.

> Hence keeping pci-host-generic.c as generic driver where above functionality
> may not be needed. 

Duplicating things to avoid touching existing drivers is not how kernel 
development works.

Rob

^ permalink raw reply

* Re: [PATCH v14 4/4] remoteproc: zynqmp: parse TCM from device tree
From: Mathieu Poirier @ 2024-04-10 16:59 UTC (permalink / raw)
  To: Tanmay Shah
  Cc: andersson, robh, krzysztof.kozlowski+dt, conor+dt, michal.simek,
	ben.levinsky, linux-remoteproc, devicetree, linux-arm-kernel,
	linux-kernel
In-Reply-To: <20240408205313.3552165-5-tanmay.shah@amd.com>

On Mon, Apr 08, 2024 at 01:53:14PM -0700, Tanmay Shah wrote:
> ZynqMP TCM information was fixed in driver. Now ZynqMP TCM information
> is available in device-tree. Parse TCM information in driver
> as per new bindings.
> 
> Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
> ---
> 
> Changes in v14:
>   - Add Versal platform support
>   - Add Versal-NET platform support
>   - Maintain backward compatibility for ZynqMP platform and use hardcode
>     TCM addresses
>   - Configure TCM based on xlnx,tcm-mode property for Versal
>   - Avoid TCM configuration if that property isn't available in DT 
> 
>  drivers/remoteproc/xlnx_r5_remoteproc.c | 173 ++++++++++++++++++------
>  1 file changed, 132 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/remoteproc/xlnx_r5_remoteproc.c b/drivers/remoteproc/xlnx_r5_remoteproc.c
> index 0f942440b4e2..504492f930ac 100644
> --- a/drivers/remoteproc/xlnx_r5_remoteproc.c
> +++ b/drivers/remoteproc/xlnx_r5_remoteproc.c
> @@ -74,8 +74,8 @@ struct mbox_info {
>  };
>  
>  /*
> - * Hardcoded TCM bank values. This will be removed once TCM bindings are
> - * accepted for system-dt specifications and upstreamed in linux kernel
> + * Hardcoded TCM bank values. This will stay in driver to maintain backward
> + * compatibility with device-tree that does not have TCM information.
>   */
>  static const struct mem_bank_data zynqmp_tcm_banks_split[] = {
>  	{0xffe00000UL, 0x0, 0x10000UL, PD_R5_0_ATCM, "atcm0"}, /* TCM 64KB each */
> @@ -300,36 +300,6 @@ static void zynqmp_r5_rproc_kick(struct rproc *rproc, int vqid)
>  		dev_warn(dev, "failed to send message\n");
>  }
>  
> -/*
> - * zynqmp_r5_set_mode()
> - *
> - * set RPU cluster and TCM operation mode
> - *
> - * @r5_core: pointer to zynqmp_r5_core type object
> - * @fw_reg_val: value expected by firmware to configure RPU cluster mode
> - * @tcm_mode: value expected by fw to configure TCM mode (lockstep or split)
> - *
> - * Return: 0 for success and < 0 for failure
> - */
> -static int zynqmp_r5_set_mode(struct zynqmp_r5_core *r5_core,
> -			      enum rpu_oper_mode fw_reg_val,
> -			      enum rpu_tcm_comb tcm_mode)
> -{
> -	int ret;
> -
> -	ret = zynqmp_pm_set_rpu_mode(r5_core->pm_domain_id, fw_reg_val);
> -	if (ret < 0) {
> -		dev_err(r5_core->dev, "failed to set RPU mode\n");
> -		return ret;
> -	}
> -
> -	ret = zynqmp_pm_set_tcm_config(r5_core->pm_domain_id, tcm_mode);
> -	if (ret < 0)
> -		dev_err(r5_core->dev, "failed to configure TCM\n");
> -
> -	return ret;
> -}
> -
>  /*
>   * zynqmp_r5_rproc_start()
>   * @rproc: single R5 core's corresponding rproc instance
> @@ -761,6 +731,103 @@ static struct zynqmp_r5_core *zynqmp_r5_add_rproc_core(struct device *cdev)
>  	return ERR_PTR(ret);
>  }
>  
> +static int zynqmp_r5_get_tcm_node_from_dt(struct zynqmp_r5_cluster *cluster)
> +{
> +	int i, j, tcm_bank_count, ret, tcm_pd_idx, pd_count;
> +	struct of_phandle_args out_args;
> +	struct zynqmp_r5_core *r5_core;
> +	struct platform_device *cpdev;
> +	struct mem_bank_data *tcm;
> +	struct device_node *np;
> +	struct resource *res;
> +	u64 abs_addr, size;
> +	struct device *dev;
> +
> +	for (i = 0; i < cluster->core_count; i++) {
> +		r5_core = cluster->r5_cores[i];
> +		dev = r5_core->dev;
> +		np = r5_core->np;
> +
> +		pd_count = of_count_phandle_with_args(np, "power-domains",
> +						      "#power-domain-cells");
> +
> +		if (pd_count <= 0) {
> +			dev_err(dev, "invalid power-domains property, %d\n", pd_count);
> +			return -EINVAL;
> +		}
> +
> +		/* First entry in power-domains list is for r5 core, rest for TCM. */
> +		tcm_bank_count = pd_count - 1;
> +
> +		if (tcm_bank_count <= 0) {
> +			dev_err(dev, "invalid TCM count %d\n", tcm_bank_count);
> +			return -EINVAL;
> +		}
> +
> +		r5_core->tcm_banks = devm_kcalloc(dev, tcm_bank_count,
> +						  sizeof(struct mem_bank_data *),
> +						  GFP_KERNEL);
> +		if (!r5_core->tcm_banks)
> +			return -ENOMEM;
> +
> +		r5_core->tcm_bank_count = tcm_bank_count;
> +		for (j = 0, tcm_pd_idx = 1; j < tcm_bank_count; j++, tcm_pd_idx++) {
> +			tcm = devm_kzalloc(dev, sizeof(struct mem_bank_data),
> +					   GFP_KERNEL);
> +			if (!tcm)
> +				return -ENOMEM;
> +
> +			r5_core->tcm_banks[j] = tcm;
> +
> +			/* Get power-domains id of TCM. */
> +			ret = of_parse_phandle_with_args(np, "power-domains",
> +							 "#power-domain-cells",
> +							 tcm_pd_idx, &out_args);
> +			if (ret) {
> +				dev_err(r5_core->dev,
> +					"failed to get tcm %d pm domain, ret %d\n",
> +					tcm_pd_idx, ret);
> +				return ret;
> +			}
> +			tcm->pm_domain_id = out_args.args[0];
> +			of_node_put(out_args.np);
> +
> +			/* Get TCM address without translation. */
> +			ret = of_property_read_reg(np, j, &abs_addr, &size);
> +			if (ret) {
> +				dev_err(dev, "failed to get reg property\n");
> +				return ret;
> +			}
> +
> +			/*
> +			 * Remote processor can address only 32 bits
> +			 * so convert 64-bits into 32-bits. This will discard
> +			 * any unwanted upper 32-bits.
> +			 */
> +			tcm->da = (u32)abs_addr;
> +			tcm->size = (u32)size;
> +
> +			cpdev = to_platform_device(dev);
> +			res = platform_get_resource(cpdev, IORESOURCE_MEM, j);
> +			if (!res) {
> +				dev_err(dev, "failed to get tcm resource\n");
> +				return -EINVAL;
> +			}
> +
> +			tcm->addr = (u32)res->start;
> +			tcm->bank_name = (char *)res->name;
> +			res = devm_request_mem_region(dev, tcm->addr, tcm->size,
> +						      tcm->bank_name);
> +			if (!res) {
> +				dev_err(dev, "failed to request tcm resource\n");
> +				return -EINVAL;
> +			}
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  /**
>   * zynqmp_r5_get_tcm_node()
>   * Ideally this function should parse tcm node and store information
> @@ -839,9 +906,16 @@ static int zynqmp_r5_core_init(struct zynqmp_r5_cluster *cluster,
>  	struct zynqmp_r5_core *r5_core;
>  	int ret, i;
>  
> -	ret = zynqmp_r5_get_tcm_node(cluster);
> -	if (ret < 0) {
> -		dev_err(dev, "can't get tcm node, err %d\n", ret);
> +	r5_core = cluster->r5_cores[0];
> +
> +	/* Maintain backward compatibility for zynqmp by using hardcode TCM address. */
> +	if (device_is_compatible(dev, "xlnx,zynqmp-r5fss"))

The previous patch moved the definition of the R5FSS to the new bindings but
this is forcing to use the old bindings - did I something?

> +		ret = zynqmp_r5_get_tcm_node(cluster);
> +	else
> +		ret = zynqmp_r5_get_tcm_node_from_dt(cluster);
> +
> +	if (ret) {
> +		dev_err(dev, "can't get tcm, err %d\n", ret);
>  		return ret;
>  	}
>  
> @@ -856,12 +930,18 @@ static int zynqmp_r5_core_init(struct zynqmp_r5_cluster *cluster,
>  			return ret;
>  		}
>  
> -		ret = zynqmp_r5_set_mode(r5_core, fw_reg_val, tcm_mode);
> -		if (ret) {
> -			dev_err(dev, "failed to set r5 cluster mode %d, err %d\n",
> -				cluster->mode, ret);
> +		ret = zynqmp_pm_set_rpu_mode(r5_core->pm_domain_id, fw_reg_val);
> +		if (ret < 0) {
> +			dev_err(r5_core->dev, "failed to set RPU mode\n");
>  			return ret;
>  		}
> +
> +		if (device_is_compatible(dev, "xlnx,zynqmp-r5fss") ||
> +		    of_find_property(dev_of_node(dev), "xlnx,tcm-mode", NULL)) {
> +			ret = zynqmp_pm_set_tcm_config(r5_core->pm_domain_id, tcm_mode);
> +			if (ret < 0)
> +				dev_err(r5_core->dev, "failed to configure TCM\n");
> +		}
>  	}
>  
>  	return 0;
> @@ -906,16 +986,25 @@ static int zynqmp_r5_cluster_init(struct zynqmp_r5_cluster *cluster)
>  	 * fail driver probe if either of that is not set in dts.
>  	 */
>  	if (cluster_mode == LOCKSTEP_MODE) {
> -		tcm_mode = PM_RPU_TCM_COMB;
>  		fw_reg_val = PM_RPU_MODE_LOCKSTEP;
>  	} else if (cluster_mode == SPLIT_MODE) {
> -		tcm_mode = PM_RPU_TCM_SPLIT;
>  		fw_reg_val = PM_RPU_MODE_SPLIT;
>  	} else {
>  		dev_err(dev, "driver does not support cluster mode %d\n", cluster_mode);
>  		return -EINVAL;
>  	}
>  
> +	if (device_is_compatible(dev, "xlnx,zynqmp-r5fss")) {
> +		if (cluster_mode == LOCKSTEP_MODE)
> +			tcm_mode = PM_RPU_TCM_COMB;
> +		else
> +			tcm_mode = PM_RPU_TCM_SPLIT;
> +	} else if (of_find_property(dev_node, "xlnx,tcm-mode", NULL)) {
> +		ret = of_property_read_u32(dev_node, "xlnx,tcm-mode", (u32 *)&tcm_mode);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	/*
>  	 * Number of cores is decided by number of child nodes of
>  	 * r5f subsystem node in dts. If Split mode is used in dts
> @@ -1100,6 +1189,8 @@ static int zynqmp_r5_remoteproc_probe(struct platform_device *pdev)
>  /* Match table for OF platform binding */
>  static const struct of_device_id zynqmp_r5_remoteproc_match[] = {
>  	{ .compatible = "xlnx,zynqmp-r5fss", },
> +	{ .compatible = "xlnx,versal-r5fss", },
> +	{ .compatible = "xlnx,versal-net-r52fss", },
>  	{ /* end of list */ },
>  };
>  MODULE_DEVICE_TABLE(of, zynqmp_r5_remoteproc_match);
> -- 
> 2.25.1
> 

^ permalink raw reply

* Re: [PATCH v6 3/7] PCI: dwc: Add PCIE_PORT_{FORCE,LANE_SKEW} macros
From: Manivannan Sadhasivam @ 2024-04-10 17:11 UTC (permalink / raw)
  To: Yoshihiro Shimoda
  Cc: lpieralisi, kw, robh, bhelgaas, krzysztof.kozlowski+dt, conor+dt,
	jingoohan1, mani, marek.vasut+renesas, linux-pci, devicetree,
	linux-renesas-soc
In-Reply-To: <20240410004832.3726922-4-yoshihiro.shimoda.uh@renesas.com>

On Wed, Apr 10, 2024 at 09:48:28AM +0900, Yoshihiro Shimoda wrote:
> R-Car Gen4 PCIe controller needs to use the Synopsys-specific PCIe
> configuration registers. So, add the macros.
> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

- Mani

> ---
>  drivers/pci/controller/dwc/pcie-designware.h | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 26dae4837462..aa4db6eaf02a 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -71,6 +71,9 @@
>  #define LINK_WAIT_IATU			9
>  
>  /* Synopsys-specific PCIe configuration registers */
> +#define PCIE_PORT_FORCE			0x708
> +#define PORT_FORCE_DO_DESKEW_FOR_SRIS	BIT(23)
> +
>  #define PCIE_PORT_AFR			0x70C
>  #define PORT_AFR_N_FTS_MASK		GENMASK(15, 8)
>  #define PORT_AFR_N_FTS(n)		FIELD_PREP(PORT_AFR_N_FTS_MASK, n)
> @@ -92,6 +95,9 @@
>  #define PORT_LINK_MODE_4_LANES		PORT_LINK_MODE(0x7)
>  #define PORT_LINK_MODE_8_LANES		PORT_LINK_MODE(0xf)
>  
> +#define PCIE_PORT_LANE_SKEW		0x714
> +#define PORT_LANE_SKEW_INSERT_MASK	GENMASK(23, 0)
> +
>  #define PCIE_PORT_DEBUG0		0x728
>  #define PORT_LOGIC_LTSSM_STATE_MASK	0x1f
>  #define PORT_LOGIC_LTSSM_STATE_L0	0x11
> -- 
> 2.25.1
> 
> 

-- 
மணிவண்ணன் சதாசிவம்

^ permalink raw reply

* [PATCH 00/11] Add Mobileye EyeQ system controller support (clk, reset, pinctrl)
From: Théo Lebrun @ 2024-04-10 17:12 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michael Turquette,
	Stephen Boyd, Philipp Zabel, Linus Walleij
  Cc: linux-mips, devicetree, linux-kernel, linux-clk, linux-gpio,
	Vladimir Kondratiev, Gregory CLEMENT, Thomas Petazzoni,
	Tawfik Bayouk, Théo Lebrun

Hello,

This builds on previous EyeQ5 system-controller revisions [0] and adds
EyeQ6L + EyeQ6H support. Pinctrl is not affected because it is not
handled in the same manner on those new platforms; it will be dealt
with with pinctrl-single. Only clk and reset expand to support those
new platform.

Changes in drivers are pretty massive, which explains why I started a
new revision count.

EyeQ6H is particular in that it has not one but seven OLB instances.
All seven host a (different) clock controller. Three host a reset
controller.

Patches are targeting MIPS, clk, reset and pinctrl trees independently.

 - dt-bindings: soc: mobileye: add EyeQ5 OLB system controller
 - MIPS: mobileye: eyeq5: add OLB syscon node
 - MIPS: mobileye: eyeq5: use OLB clocks controller node
 - MIPS: mobileye: eyeq5: add OLB reset controller node
 - MIPS: mobileye: eyeq5: add pinctrl node & pinmux function nodes

   Patches have no dependencies on this series. All required devicetrees
   and bindings got in the last release. Bindings changes below are
   only related to EyeQ6L/EyeQ6H.

 - dt-bindings: clock: mobileye,eyeq5-clk: add EyeQ6L and EyeQ6H
 - clk: divider: Introduce CLK_DIVIDER_EVEN_INTEGERS flag
 - clk: eyeq: add driver

   Clocks on all three platforms are rather similar. We have a bunch of
   PLLs children to the main crystal, some being required early
   (at of_clk_init(), before platform bus init).

   We have a few divider clocks in some instances. A custom clk-divider
   flag is introduced for the divisor because one clk would have a 256
   entries table otherwise.

   Compatible match data stores it all, nothing is declared in DT. Match
   data has three arrays for the three types of clocks: early PLLs,
   standard PLLs and divider clks.

 - dt-bindings: reset: mobileye,eyeq5-reset: add EyeQ6L and EyeQ6H
 - reset: eyeq: add platform driver

   Resets on all three platforms are rather similar. We therefore
   declare reset domain types and assign compatibles to an array of
   domains, with types and valid masks associated. The rest is pretty
   similar to previous code.

   EyeQ6H west and east OLB each host an instance of the same compat.

 - pinctrl: eyeq5: add platform driver

   Not affected by EyeQ6L/EyeQ6H additions. It has not changed.

Have a nice day,
Théo

[0]: https://lore.kernel.org/lkml/20240301-mbly-clk-v9-0-cbf06eb88708@bootlin.com/

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
Théo Lebrun (11):
      dt-bindings: soc: mobileye: add EyeQ5 OLB system controller
      dt-bindings: clock: mobileye,eyeq5-clk: add EyeQ6L and EyeQ6H
      dt-bindings: reset: mobileye,eyeq5-reset: add EyeQ6L and EyeQ6H
      clk: divider: Introduce CLK_DIVIDER_EVEN_INTEGERS flag
      clk: eyeq: add driver
      reset: eyeq: add platform driver
      pinctrl: eyeq5: add platform driver
      MIPS: mobileye: eyeq5: add OLB syscon node
      MIPS: mobileye: eyeq5: use OLB clocks controller node
      MIPS: mobileye: eyeq5: add OLB reset controller node
      MIPS: mobileye: eyeq5: add pinctrl node & pinmux function nodes

 .../bindings/clock/mobileye,eyeq5-clk.yaml         | 103 +++-
 .../bindings/reset/mobileye,eyeq5-reset.yaml       |  88 ++-
 .../bindings/soc/mobileye/mobileye,eyeq5-olb.yaml  | 125 ++++
 MAINTAINERS                                        |   8 +
 .../{eyeq5-fixed-clocks.dtsi => eyeq5-clocks.dtsi} |  54 +-
 arch/mips/boot/dts/mobileye/eyeq5-pins.dtsi        | 125 ++++
 arch/mips/boot/dts/mobileye/eyeq5.dtsi             |  42 +-
 drivers/clk/Kconfig                                |  11 +
 drivers/clk/Makefile                               |   1 +
 drivers/clk/clk-divider.c                          |  12 +-
 drivers/clk/clk-eyeq.c                             | 644 +++++++++++++++++++++
 drivers/pinctrl/Kconfig                            |  14 +
 drivers/pinctrl/Makefile                           |   1 +
 drivers/pinctrl/pinctrl-eyeq5.c                    | 579 ++++++++++++++++++
 drivers/reset/Kconfig                              |  13 +
 drivers/reset/Makefile                             |   1 +
 drivers/reset/reset-eyeq.c                         | 543 +++++++++++++++++
 include/dt-bindings/clock/mobileye,eyeq5-clk.h     |  21 +
 include/linux/clk-provider.h                       |  11 +-
 19 files changed, 2322 insertions(+), 74 deletions(-)
---
base-commit: c8e31f416e99bd460f6f8847709bf69c72a3e146
change-id: 20240408-mbly-olb-75a85f5cfde3

Best regards,
-- 
Théo Lebrun <theo.lebrun@bootlin.com>


^ permalink raw reply

* [PATCH 01/11] dt-bindings: soc: mobileye: add EyeQ5 OLB system controller
From: Théo Lebrun @ 2024-04-10 17:12 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michael Turquette,
	Stephen Boyd, Philipp Zabel, Linus Walleij
  Cc: linux-mips, devicetree, linux-kernel, linux-clk, linux-gpio,
	Vladimir Kondratiev, Gregory CLEMENT, Thomas Petazzoni,
	Tawfik Bayouk, Théo Lebrun
In-Reply-To: <20240410-mbly-olb-v1-0-335e496d7be3@bootlin.com>

Add documentation to describe the "Other Logic Block" syscon.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 .../bindings/soc/mobileye/mobileye,eyeq5-olb.yaml  | 125 +++++++++++++++++++++
 MAINTAINERS                                        |   1 +
 2 files changed, 126 insertions(+)

diff --git a/Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.yaml b/Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.yaml
new file mode 100644
index 000000000000..c4e33a167fab
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.yaml
@@ -0,0 +1,125 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/mobileye/mobileye,eyeq5-olb.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mobileye EyeQ SoC system controller
+
+maintainers:
+  - Grégory Clement <gregory.clement@bootlin.com>
+  - Théo Lebrun <theo.lebrun@bootlin.com>
+  - Vladimir Kondratiev <vladimir.kondratiev@mobileye.com>
+
+description:
+  OLB ("Other Logic Block") is a hardware block grouping smaller blocks. Clocks,
+  resets, pinctrl are being handled from here. EyeQ5 and EyeQ6L host a single
+  instance. EyeQ6H hosts seven instances.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - mobileye,eyeq5-olb
+          - mobileye,eyeq6l-olb
+          - mobileye,eyeq6h-olb
+      - const: syscon
+      - const: simple-mfd
+
+  reg:
+    maxItems: 1
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 1
+
+  ranges: true
+
+patternProperties:
+  '^clock-controller@[0-9a-f]+$':
+    $ref: /schemas/clock/mobileye,eyeq5-clk.yaml#
+
+  '^reset-controller@[0-9a-f]+$':
+    $ref: /schemas/reset/mobileye,eyeq5-reset.yaml#
+
+  '^pinctrl@[0-9a-f]+$':
+    $ref: /schemas/pinctrl/mobileye,eyeq5-pinctrl.yaml#
+
+required:
+  - compatible
+  - reg
+  - '#address-cells'
+  - '#size-cells'
+  - ranges
+
+additionalProperties: false
+
+examples:
+  - |
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      system-controller@e00000 {
+        compatible = "mobileye,eyeq5-olb", "syscon", "simple-mfd";
+        reg = <0x0 0xe00000 0x0 0x400>;
+        ranges = <0x0 0x0 0xe00000 0x400>;
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        reset-controller@e00000 {
+          compatible = "mobileye,eyeq5-reset";
+          reg = <0x000 0x0c>, <0x200 0x34>, <0x120 0x04>;
+          reg-names = "d0", "d1", "d2";
+          #reset-cells = <2>;
+        };
+
+        clock-controller@e0002c {
+          compatible = "mobileye,eyeq5-clk";
+          reg = <0x02c 0x50>, <0x11c 0x04>;
+          reg-names = "plls", "ospi";
+          #clock-cells = <1>;
+          clocks = <&xtal>;
+          clock-names = "ref";
+        };
+
+        pinctrl@e000b0 {
+          compatible = "mobileye,eyeq5-pinctrl";
+          reg = <0x0b0 0x30>;
+
+          uart2_pins: uart2-pins {
+            function = "uart2";
+            pins = "PB8", "PB9";
+          };
+        };
+      };
+    };
+  - |
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      system-controller@d2003000 {
+        compatible = "mobileye,eyeq6h-olb", "syscon", "simple-mfd";
+        reg = <0x0 0xd2003000 0x0 0x1000>;
+        ranges = <0x0 0x0 0xd2003000 0x1000>;
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        reset-controller@d2003000 {
+          compatible = "mobileye,eyeq6h-acc-reset";
+          reg = <0x0 0x3c>;
+          #reset-cells = <1>;
+        };
+
+        clock-controller@d2003040 {
+          compatible = "mobileye,eyeq6h-acc-clk";
+          reg = <0x40 0x38>;
+          #clock-cells = <1>;
+          clocks = <&xtal>;
+          clock-names = "ref";
+        };
+      };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index aa3b947fb080..30dfbee84007 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14926,6 +14926,7 @@ M:	Théo Lebrun <theo.lebrun@bootlin.com>
 L:	linux-mips@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/mips/mobileye.yaml
+F:	Documentation/devicetree/bindings/soc/mobileye/
 F:	arch/mips/boot/dts/mobileye/
 F:	arch/mips/configs/eyeq5_defconfig
 F:	arch/mips/mobileye/board-epm5.its.S

-- 
2.44.0


^ permalink raw reply related

* [PATCH 02/11] dt-bindings: clock: mobileye,eyeq5-clk: add EyeQ6L and EyeQ6H
From: Théo Lebrun @ 2024-04-10 17:12 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michael Turquette,
	Stephen Boyd, Philipp Zabel, Linus Walleij
  Cc: linux-mips, devicetree, linux-kernel, linux-clk, linux-gpio,
	Vladimir Kondratiev, Gregory CLEMENT, Thomas Petazzoni,
	Tawfik Bayouk, Théo Lebrun
In-Reply-To: <20240410-mbly-olb-v1-0-335e496d7be3@bootlin.com>

Add bindings describing EyeQ6L and EyeQ6H clock controllers.
Add constants to index clocks.

Bindings are conditional for two reasons:
 - Some compatibles expose a single clock; they do not take clock cells.
 - All compatibles take a PLLs resource, not all take others (aimed at
   divider clocks). Those that only take a resource for PLLs do not
   require named resources.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 .../bindings/clock/mobileye,eyeq5-clk.yaml         | 103 ++++++++++++++++++---
 MAINTAINERS                                        |   2 +
 include/dt-bindings/clock/mobileye,eyeq5-clk.h     |  21 +++++
 3 files changed, 113 insertions(+), 13 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.yaml b/Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.yaml
index 2d4f2cde1e58..a1651fcce258 100644
--- a/Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.yaml
+++ b/Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.yaml
@@ -4,12 +4,13 @@
 $id: http://devicetree.org/schemas/clock/mobileye,eyeq5-clk.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Mobileye EyeQ5 clock controller
+title: Mobileye EyeQ clock controller
 
 description:
-  The EyeQ5 clock controller handles 10 read-only PLLs derived from the main
-  crystal clock. It also exposes one divider clock, a child of one of the PLLs.
-  Its registers live in a shared region called OLB.
+  EyeQ clock controllers expose read-only PLLs derived from main crystal clock.
+  Some also expose divider clocks, children of specific PLLs. Its registers
+  live in a shared region called OLB. EyeQ5 and EyeQ6L have a single OLB
+  instance while EyeQ6H have seven, leading to seven clock controllers.
 
 maintainers:
   - Grégory Clement <gregory.clement@bootlin.com>
@@ -18,18 +19,23 @@ maintainers:
 
 properties:
   compatible:
-    const: mobileye,eyeq5-clk
+    enum:
+      - mobileye,eyeq5-clk
+      - mobileye,eyeq6l-clk
+      - mobileye,eyeq6h-central-clk
+      - mobileye,eyeq6h-west-clk
+      - mobileye,eyeq6h-east-clk
+      - mobileye,eyeq6h-south-clk
+      - mobileye,eyeq6h-ddr0-clk
+      - mobileye,eyeq6h-ddr1-clk
+      - mobileye,eyeq6h-acc-clk
 
-  reg:
-    maxItems: 2
+  reg: true
 
-  reg-names:
-    items:
-      - const: plls
-      - const: ospi
+  reg-names: true
 
   "#clock-cells":
-    const: 1
+    enum: [0, 1]
 
   clocks:
     maxItems: 1
@@ -43,9 +49,80 @@ properties:
 required:
   - compatible
   - reg
-  - reg-names
   - "#clock-cells"
   - clocks
   - clock-names
 
+allOf:
+  # "mobileye,eyeq5-clk" provides:
+  #  - PLLs and,
+  #  - One divider clock related to ospi.
+  - if:
+      properties:
+        compatible:
+          const: mobileye,eyeq5-clk
+    then:
+      properties:
+        reg:
+          minItems: 2
+          maxItems: 2
+        reg-names:
+          minItems: 2
+          maxItems: 2
+          items:
+            enum: [ plls, ospi ]
+      required:
+        - reg-names
+
+  # "mobileye,eyeq6h-south-clk" provides:
+  #  - PLLs and,
+  #  - Four divider clocks related to emmc, ospi and tsu.
+  - if:
+      properties:
+        compatible:
+          const: mobileye,eyeq6h-south-clk
+    then:
+      properties:
+        reg:
+          minItems: 4
+          maxItems: 4
+        reg-names:
+          minItems: 4
+          maxItems: 4
+          items:
+            enum: [ plls, emmc, ospi, tsu ]
+      required:
+        - reg-names
+
+  # Other compatibles only provide PLLs. Do not ask for named resources.
+  - if:
+      not:
+        required:
+          - reg-names
+    then:
+      properties:
+        reg:
+          minItems: 1
+          maxItems: 1
+        reg-names: false
+
+  # Some compatibles provide a single clock; they do not take a clock cell.
+  - if:
+      properties:
+        compatible:
+          enum:
+            - mobileye,eyeq6h-central-clk
+            - mobileye,eyeq6h-west-clk
+            - mobileye,eyeq6h-east-clk
+            - mobileye,eyeq6h-ddr0-clk
+            - mobileye,eyeq6h-ddr1-clk
+    then:
+      properties:
+        "#clock-cells":
+          const: 0
+    else:
+      properties:
+        "#clock-cells":
+          const: 1
+
 additionalProperties: false
diff --git a/MAINTAINERS b/MAINTAINERS
index 30dfbee84007..f5a488331b38 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14925,11 +14925,13 @@ M:	Gregory CLEMENT <gregory.clement@bootlin.com>
 M:	Théo Lebrun <theo.lebrun@bootlin.com>
 L:	linux-mips@vger.kernel.org
 S:	Maintained
+F:	Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.yaml
 F:	Documentation/devicetree/bindings/mips/mobileye.yaml
 F:	Documentation/devicetree/bindings/soc/mobileye/
 F:	arch/mips/boot/dts/mobileye/
 F:	arch/mips/configs/eyeq5_defconfig
 F:	arch/mips/mobileye/board-epm5.its.S
+F:	include/dt-bindings/clock/mobileye,eyeq5-clk.h
 
 MODULE SUPPORT
 M:	Luis Chamberlain <mcgrof@kernel.org>
diff --git a/include/dt-bindings/clock/mobileye,eyeq5-clk.h b/include/dt-bindings/clock/mobileye,eyeq5-clk.h
index 26d8930335e4..b433c1772c28 100644
--- a/include/dt-bindings/clock/mobileye,eyeq5-clk.h
+++ b/include/dt-bindings/clock/mobileye,eyeq5-clk.h
@@ -19,4 +19,25 @@
 
 #define EQ5C_DIV_OSPI	10
 
+#define EQ6LC_PLL_DDR		0
+#define EQ6LC_PLL_CPU		1
+#define EQ6LC_PLL_PER		2
+#define EQ6LC_PLL_VDI		3
+
+#define EQ6HC_SOUTH_PLL_VDI		0
+#define EQ6HC_SOUTH_PLL_PCIE		1
+#define EQ6HC_SOUTH_PLL_PER		2
+#define EQ6HC_SOUTH_PLL_ISP		3
+
+#define EQ6HC_SOUTH_DIV_EMMC		4
+#define EQ6HC_SOUTH_DIV_OSPI_REF	5
+#define EQ6HC_SOUTH_DIV_OSPI_SYS	6
+#define EQ6HC_SOUTH_DIV_TSU		7
+
+#define EQ6HC_ACC_PLL_XNN		0
+#define EQ6HC_ACC_PLL_VMP		1
+#define EQ6HC_ACC_PLL_PMA		2
+#define EQ6HC_ACC_PLL_MPC		3
+#define EQ6HC_ACC_PLL_NOC		4
+
 #endif

-- 
2.44.0


^ permalink raw reply related

* [PATCH 03/11] dt-bindings: reset: mobileye,eyeq5-reset: add EyeQ6L and EyeQ6H
From: Théo Lebrun @ 2024-04-10 17:12 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michael Turquette,
	Stephen Boyd, Philipp Zabel, Linus Walleij
  Cc: linux-mips, devicetree, linux-kernel, linux-clk, linux-gpio,
	Vladimir Kondratiev, Gregory CLEMENT, Thomas Petazzoni,
	Tawfik Bayouk, Théo Lebrun
In-Reply-To: <20240410-mbly-olb-v1-0-335e496d7be3@bootlin.com>

Add bindings for EyeQ6L and EyeQ6H reset controllers.

Some controllers host a single domain, meaning a single cell is enough.
We do not enforce reg-names for such nodes.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 .../bindings/reset/mobileye,eyeq5-reset.yaml       | 88 ++++++++++++++++++----
 MAINTAINERS                                        |  1 +
 2 files changed, 74 insertions(+), 15 deletions(-)

diff --git a/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml b/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml
index 062b4518347b..799bcf15bed9 100644
--- a/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml
+++ b/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml
@@ -4,11 +4,13 @@
 $id: http://devicetree.org/schemas/reset/mobileye,eyeq5-reset.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Mobileye EyeQ5 reset controller
+title: Mobileye EyeQ reset controller
 
 description:
-  The EyeQ5 reset driver handles three reset domains. Its registers live in a
-  shared region called OLB.
+  EyeQ reset controller handles one or more reset domains. They live in shared
+  regions called OLB. EyeQ5 and EyeQ6L host one OLB each, each with one reset
+  instance. EyeQ6H hosts 7 OLB regions; three of those (west, east,
+  accelerator) host reset controllers. West and east are duplicates.
 
 maintainers:
   - Grégory Clement <gregory.clement@bootlin.com>
@@ -17,27 +19,83 @@ maintainers:
 
 properties:
   compatible:
-    const: mobileye,eyeq5-reset
+    enum:
+      - mobileye,eyeq5-reset
+      - mobileye,eyeq6l-reset
+      - mobileye,eyeq6h-we-reset
+      - mobileye,eyeq6h-acc-reset
 
-  reg:
-    maxItems: 3
+  reg: true
 
-  reg-names:
-    items:
-      - const: d0
-      - const: d1
-      - const: d2
+  reg-names: true
 
   "#reset-cells":
-    const: 2
     description:
-      The first cell is the domain (0 to 2 inclusive) and the second one is the
-      reset index inside that domain.
+      First cell is domain, second is reset index inside that domain. If
+      controller has a single domain, first cell is implicitly zero.
+    enum: [ 1, 2 ]
 
 required:
   - compatible
   - reg
-  - reg-names
   - "#reset-cells"
 
+allOf:
+  # EyeQ5 and EyeQ6L have multiple domains, other compatibles have one.
+  # Multiple domains means named resources and two reset cells.
+  # Single domain means a single unnamed resource and one reset cell.
+  - if:
+      properties:
+        compatible:
+          enum:
+            - mobileye,eyeq5-reset
+            - mobileye,eyeq6l-reset
+    then:
+      properties:
+        "#reset-cells":
+          const: 2
+      required:
+        - reg-names
+    else:
+      properties:
+        reg:
+          maxItems: 1
+        reg-names: false
+        "#reset-cells":
+          const: 1
+
+  # EyeQ5 has three domains.
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: mobileye,eyeq5-reset
+    then:
+      properties:
+        reg:
+          minItems: 3
+          maxItems: 3
+        reg-names:
+          minItems: 3
+          maxItems: 3
+          items:
+            enum: [ d0, d1, d2 ]
+
+  # EyeQ6L has two domains.
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: mobileye,eyeq6l-reset
+    then:
+      properties:
+        reg:
+          minItems: 2
+          maxItems: 2
+        reg-names:
+          minItems: 2
+          maxItems: 2
+          items:
+            enum: [ d0, d1 ]
+
 additionalProperties: false
diff --git a/MAINTAINERS b/MAINTAINERS
index f5a488331b38..42553da10be9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14927,6 +14927,7 @@ L:	linux-mips@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.yaml
 F:	Documentation/devicetree/bindings/mips/mobileye.yaml
+F:	Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml
 F:	Documentation/devicetree/bindings/soc/mobileye/
 F:	arch/mips/boot/dts/mobileye/
 F:	arch/mips/configs/eyeq5_defconfig

-- 
2.44.0


^ permalink raw reply related

* [PATCH 04/11] clk: divider: Introduce CLK_DIVIDER_EVEN_INTEGERS flag
From: Théo Lebrun @ 2024-04-10 17:12 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michael Turquette,
	Stephen Boyd, Philipp Zabel, Linus Walleij
  Cc: linux-mips, devicetree, linux-kernel, linux-clk, linux-gpio,
	Vladimir Kondratiev, Gregory CLEMENT, Thomas Petazzoni,
	Tawfik Bayouk, Théo Lebrun
In-Reply-To: <20240410-mbly-olb-v1-0-335e496d7be3@bootlin.com>

Add CLK_DIVIDER_EVEN_INTEGERS flag to support divisor of 2, 4, 6, etc.
The same divisor can be done using a table, which would be big and
wasteful for a clock dividor of width 8 (256 entries).

Require increasing flags size from u8 to u16 because
CLK_DIVIDER_EVEN_INTEGERS is the eighth flag.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/clk/clk-divider.c    | 12 +++++++++---
 include/linux/clk-provider.h | 11 +++++++----
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index a2c2b5203b0a..b6654c5c36d2 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -72,6 +72,8 @@ static unsigned int _get_maxdiv(const struct clk_div_table *table, u8 width,
 		return clk_div_mask(width);
 	if (flags & CLK_DIVIDER_POWER_OF_TWO)
 		return 1 << clk_div_mask(width);
+	if (flags & CLK_DIVIDER_EVEN_INTEGERS)
+		return 2 * (clk_div_mask(width) + 1);
 	if (table)
 		return _get_table_maxdiv(table, width);
 	return clk_div_mask(width) + 1;
@@ -97,6 +99,8 @@ static unsigned int _get_div(const struct clk_div_table *table,
 		return 1 << val;
 	if (flags & CLK_DIVIDER_MAX_AT_ZERO)
 		return val ? val : clk_div_mask(width) + 1;
+	if (flags & CLK_DIVIDER_EVEN_INTEGERS)
+		return 2 * (val + 1);
 	if (table)
 		return _get_table_div(table, val);
 	return val + 1;
@@ -122,6 +126,8 @@ static unsigned int _get_val(const struct clk_div_table *table,
 		return __ffs(div);
 	if (flags & CLK_DIVIDER_MAX_AT_ZERO)
 		return (div == clk_div_mask(width) + 1) ? 0 : div;
+	if (flags & CLK_DIVIDER_EVEN_INTEGERS)
+		return (div >> 1) - 1;
 	if (table)
 		return  _get_table_val(table, div);
 	return div - 1;
@@ -538,7 +544,7 @@ struct clk_hw *__clk_hw_register_divider(struct device *dev,
 		struct device_node *np, const char *name,
 		const char *parent_name, const struct clk_hw *parent_hw,
 		const struct clk_parent_data *parent_data, unsigned long flags,
-		void __iomem *reg, u8 shift, u8 width, u8 clk_divider_flags,
+		void __iomem *reg, u8 shift, u8 width, u16 clk_divider_flags,
 		const struct clk_div_table *table, spinlock_t *lock)
 {
 	struct clk_divider *div;
@@ -610,7 +616,7 @@ EXPORT_SYMBOL_GPL(__clk_hw_register_divider);
 struct clk *clk_register_divider_table(struct device *dev, const char *name,
 		const char *parent_name, unsigned long flags,
 		void __iomem *reg, u8 shift, u8 width,
-		u8 clk_divider_flags, const struct clk_div_table *table,
+		u16 clk_divider_flags, const struct clk_div_table *table,
 		spinlock_t *lock)
 {
 	struct clk_hw *hw;
@@ -664,7 +670,7 @@ struct clk_hw *__devm_clk_hw_register_divider(struct device *dev,
 		struct device_node *np, const char *name,
 		const char *parent_name, const struct clk_hw *parent_hw,
 		const struct clk_parent_data *parent_data, unsigned long flags,
-		void __iomem *reg, u8 shift, u8 width, u8 clk_divider_flags,
+		void __iomem *reg, u8 shift, u8 width, u16 clk_divider_flags,
 		const struct clk_div_table *table, spinlock_t *lock)
 {
 	struct clk_hw **ptr, *hw;
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 4a537260f655..cb348e502e41 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -675,13 +675,15 @@ struct clk_div_table {
  * CLK_DIVIDER_BIG_ENDIAN - By default little endian register accesses are used
  *	for the divider register.  Setting this flag makes the register accesses
  *	big endian.
+ * CLK_DIVIDER_EVEN_INTEGERS - clock divisor is 2, 4, 6, 8, 10, etc.
+ *	Formula is 2 * (value read from hardware + 1).
  */
 struct clk_divider {
 	struct clk_hw	hw;
 	void __iomem	*reg;
 	u8		shift;
 	u8		width;
-	u8		flags;
+	u16		flags;
 	const struct clk_div_table	*table;
 	spinlock_t	*lock;
 };
@@ -697,6 +699,7 @@ struct clk_divider {
 #define CLK_DIVIDER_READ_ONLY		BIT(5)
 #define CLK_DIVIDER_MAX_AT_ZERO		BIT(6)
 #define CLK_DIVIDER_BIG_ENDIAN		BIT(7)
+#define CLK_DIVIDER_EVEN_INTEGERS	BIT(8)
 
 extern const struct clk_ops clk_divider_ops;
 extern const struct clk_ops clk_divider_ro_ops;
@@ -726,18 +729,18 @@ struct clk_hw *__clk_hw_register_divider(struct device *dev,
 		struct device_node *np, const char *name,
 		const char *parent_name, const struct clk_hw *parent_hw,
 		const struct clk_parent_data *parent_data, unsigned long flags,
-		void __iomem *reg, u8 shift, u8 width, u8 clk_divider_flags,
+		void __iomem *reg, u8 shift, u8 width, u16 clk_divider_flags,
 		const struct clk_div_table *table, spinlock_t *lock);
 struct clk_hw *__devm_clk_hw_register_divider(struct device *dev,
 		struct device_node *np, const char *name,
 		const char *parent_name, const struct clk_hw *parent_hw,
 		const struct clk_parent_data *parent_data, unsigned long flags,
-		void __iomem *reg, u8 shift, u8 width, u8 clk_divider_flags,
+		void __iomem *reg, u8 shift, u8 width, u16 clk_divider_flags,
 		const struct clk_div_table *table, spinlock_t *lock);
 struct clk *clk_register_divider_table(struct device *dev, const char *name,
 		const char *parent_name, unsigned long flags,
 		void __iomem *reg, u8 shift, u8 width,
-		u8 clk_divider_flags, const struct clk_div_table *table,
+		u16 clk_divider_flags, const struct clk_div_table *table,
 		spinlock_t *lock);
 /**
  * clk_register_divider - register a divider clock with the clock framework

-- 
2.44.0


^ permalink raw reply related


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