Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH v5 0/3] arm64: dts: qcom: apq8016: Add Schneider HMIBSC board DTS
From: Rob Herring @ 2024-04-03 14:15 UTC (permalink / raw)
  To: Sumit Garg
  Cc: devicetree, neil.armstrong, caleb.connolly, conor+dt,
	pascal.eberhard, krzysztof.kozlowski+dt, stephan, linux-kernel,
	benjamin.missey, daniel.thompson, linux-arm-msm, andersson,
	dmitry.baryshkov, jimmy.lalande, konrad.dybcio, robh+dt,
	laetitia.mariottini, abdou.saker
In-Reply-To: <20240403043416.3800259-1-sumit.garg@linaro.org>


On Wed, 03 Apr 2024 10:04:13 +0530, Sumit Garg wrote:
> Add Schneider Electric HMIBSC board DTS. The HMIBSC board is an IIoT Edge
> Box Core board based on the Qualcomm APQ8016E SoC. For more information
> refer to the product page [1].
> 
> One of the major difference from db410c is serial port where HMIBSC board
> uses UART1 as the debug console with a default RS232 mode (UART1 mode mux
> configured via gpio99 and gpio100).
> 
> Support for Schneider Electric HMIBSC. Features:
> - Qualcomm Snapdragon 410C SoC - APQ8016 (4xCortex A53, Adreno 306)
> - 1GiB RAM
> - 8GiB eMMC, SD slot
> - WiFi and Bluetooth
> - 2x Host, 1x Device USB port
> - HDMI
> - Discrete TPM2 chip over SPI
> - USB ethernet adaptors (soldered)
> 
> This series is a v2 since v1 of this DTS file has been reviewed on the
> U-Boot mailing list [2].
> 
> Changes in v5:
> - Addressed another nitpick from Stephen.
> - Collected Stephen's review tag.
> - Warnings reported by Rob's DT check bot aren't related to HMIBSC
>   board DTS but rather they are due to msm8916.dtsi or extcon-usb-gpio.txt
>   still not converted to YAML format.
> 
> Changes in v4:
> - Dropped IRQ_TYPE_EDGE_FALLING for pm8916_resin given the expectations
>   of Linux kernel driver. Instead depend on systemd workaround suggested
>   by Caleb to get expected HMIBSC reset behaviour.
> - Incorporated further DT coding style comments from Stephen.
> - Warnings reported by Rob's DT check bot aren't related to HMIBSC
>   board DTS but rather they are due to msm8916.dtsi or extcon-usb-gpio.txt
>   still not converted to YAML format.
> 
> Changes in v3:
> - Picked up tags.
> - Fixed further DT schema warnings.
> - Configure resin/power button interrupt as falling edge.
> - Incorporate DTS coding style comments from Krzysztof and Konrad.
> 
> Changes in v2:
> - Fix DT schema warnings.
> - Incorporate suggestions from Stephan.
> - Document UART1 mode GPIOs based mux.
> 
> [1] https://www.se.com/us/en/product/HMIBSCEA53D1L0T/iiot-edge-box-core-harmony-ipc-emmc-dc-linux-tpm/
> [2] https://patchwork.ozlabs.org/project/uboot/patch/20240311111027.44577-6-sumit.garg@linaro.org/
> 
> Sumit Garg (3):
>   dt-bindings: vendor-prefixes: Add Schneider Electric
>   dt-bindings: arm: qcom: Add Schneider Electric HMIBSC board
>   arm64: dts: qcom: apq8016: Add Schneider HMIBSC board DTS
> 
>  .../devicetree/bindings/arm/qcom.yaml         |   1 +
>  .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
>  arch/arm64/boot/dts/qcom/Makefile             |   1 +
>  .../dts/qcom/apq8016-schneider-hmibsc.dts     | 491 ++++++++++++++++++
>  4 files changed, 495 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/qcom/apq8016-schneider-hmibsc.dts
> 
> --
> 2.34.1
> 
> 
> 


My bot found new DTB warnings on the .dts files added or changed in this
series.

Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
are fixed by another series. Ultimately, it is up to the platform
maintainer whether these warnings are acceptable or not. No need to reply
unless the platform maintainer has comments.

If you already ran DT checks and didn't see these error(s), then
make sure dt-schema is up to date:

  pip3 install dtschema --upgrade


New warnings running 'make CHECK_DTBS=y qcom/apq8016-schneider-hmibsc.dtb' for 20240403043416.3800259-1-sumit.garg@linaro.org:

arch/arm64/boot/dts/qcom/apq8016-schneider-hmibsc.dtb: pmic@0: gpio@c000:gpio-line-names: ['USB_HUB_RESET_N_PM', 'USB_SW_SEL_PM', 'NC', 'NC'] is too short
	from schema $id: http://devicetree.org/schemas/mfd/qcom,spmi-pmic.yaml#
arch/arm64/boot/dts/qcom/apq8016-schneider-hmibsc.dtb: gpio@c000: gpio-line-names: ['USB_HUB_RESET_N_PM', 'USB_SW_SEL_PM', 'NC', 'NC'] is too short
	from schema $id: http://devicetree.org/schemas/pinctrl/qcom,pmic-gpio.yaml#
arch/arm64/boot/dts/qcom/apq8016-schneider-hmibsc.dtb: /soc@0/audio-codec@771c000: failed to match any schema with compatible: ['qcom,msm8916-wcd-digital-codec']
arch/arm64/boot/dts/qcom/apq8016-schneider-hmibsc.dtb: /soc@0/power-manager@b088000: failed to match any schema with compatible: ['qcom,msm8916-acc']
arch/arm64/boot/dts/qcom/apq8016-schneider-hmibsc.dtb: /soc@0/power-manager@b098000: failed to match any schema with compatible: ['qcom,msm8916-acc']
arch/arm64/boot/dts/qcom/apq8016-schneider-hmibsc.dtb: /soc@0/power-manager@b0a8000: failed to match any schema with compatible: ['qcom,msm8916-acc']
arch/arm64/boot/dts/qcom/apq8016-schneider-hmibsc.dtb: /soc@0/power-manager@b0b8000: failed to match any schema with compatible: ['qcom,msm8916-acc']
arch/arm64/boot/dts/qcom/apq8016-schneider-hmibsc.dtb: /usb-id: failed to match any schema with compatible: ['linux,extcon-usb-gpio']






^ permalink raw reply

* Re: [PATCH v9 0/9] Initial Marvell PXA1908 support
From: Rob Herring @ 2024-04-03 14:15 UTC (permalink / raw)
  To: Duje Mihanović
  Cc: phone-devel, linux-gpio, Krzysztof Kozlowski,
	Guilherme G. Piccoli, linux-kernel, Catalin Marinas,
	~postmarketos/upstreaming, Andy Shevchenko, linux-arm-kernel,
	devicetree, Krzysztof Kozlowski, Linus Walleij, Kees Cook,
	Rob Herring, Stephen Boyd, Lubomir Rintel, Will Deacon,
	Karel Balej, linux-clk, Conor Dooley, Tony Lindgren, David Wronek,
	Haojian Zhuang, Tony Luck, Conor Dooley, Michael Turquette
In-Reply-To: <20240402-pxa1908-lkml-v9-0-25a003e83c6f@skole.hr>


On Tue, 02 Apr 2024 22:55:36 +0200, Duje Mihanović wrote:
> Hello,
> 
> This series adds initial support for the Marvell PXA1908 SoC and
> "samsung,coreprimevelte", a smartphone using the SoC.
> 
> USB works and the phone can boot a rootfs from an SD card, but there are
> some warnings in the dmesg:
> 
> During SMP initialization:
> [    0.006519] CPU features: SANITY CHECK: Unexpected variation in SYS_CNTFRQ_EL0. Boot CPU: 0x000000018cba80, CPU1: 0x00000000000000
> [    0.006542] CPU features: Unsupported CPU feature variation detected.
> [    0.006589] CPU1: Booted secondary processor 0x0000000001 [0x410fd032]
> [    0.010710] Detected VIPT I-cache on CPU2
> [    0.010716] CPU features: SANITY CHECK: Unexpected variation in SYS_CNTFRQ_EL0. Boot CPU: 0x000000018cba80, CPU2: 0x00000000000000
> [    0.010758] CPU2: Booted secondary processor 0x0000000002 [0x410fd032]
> [    0.014849] Detected VIPT I-cache on CPU3
> [    0.014855] CPU features: SANITY CHECK: Unexpected variation in SYS_CNTFRQ_EL0. Boot CPU: 0x000000018cba80, CPU3: 0x00000000000000
> [    0.014895] CPU3: Booted secondary processor 0x0000000003 [0x410fd032]
> 
> SMMU probing fails:
> [    0.101798] arm-smmu c0010000.iommu: probing hardware configuration...
> [    0.101809] arm-smmu c0010000.iommu: SMMUv1 with:
> [    0.101816] arm-smmu c0010000.iommu:         no translation support!
> 
> A 3.14 based Marvell tree is available on GitHub
> acorn-marvell/brillo_pxa_kernel, and a Samsung one on GitHub
> CoderCharmander/g361f-kernel.
> 
> Andreas Färber attempted to upstream support for this SoC in 2017:
> https://lore.kernel.org/lkml/20170222022929.10540-1-afaerber@suse.de/
> 
> Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
> 
> Changes in v9:
> - Update trailers and rebase on v6.9-rc2, no changes
> - Link to v8: https://lore.kernel.org/20240110-pxa1908-lkml-v8-0-fea768a59474@skole.hr
> 
> Changes in v8:
> - Drop SSPA patch
> - Drop broken-cd from eMMC node
> - Specify S-Boot hardcoded initramfs location in device tree
> - Add ARM PMU node
> - Correct inverted modem memory base and size
> - Update trailers
> - Rebase on next-20240110
> - Link to v7: https://lore.kernel.org/20231102-pxa1908-lkml-v7-0-cabb1a0cb52b@skole.hr
>   and https://lore.kernel.org/20231102152033.5511-1-duje.mihanovic@skole.hr
> 
> Changes in v7:
> - Suppress SND_MMP_SOC_SSPA on ARM64
> - Update trailers
> - Rebase on v6.6-rc7
> - Link to v6: https://lore.kernel.org/r/20231010-pxa1908-lkml-v6-0-b2fe09240cf8@skole.hr
> 
> Changes in v6:
> - Address maintainer comments:
>   - Add "marvell,pxa1908-padconf" binding to pinctrl-single driver
> - Drop GPIO patch as it's been pulled
> - Update trailers
> - Rebase on v6.6-rc5
> - Link to v5: https://lore.kernel.org/r/20230812-pxa1908-lkml-v5-0-a5d51937ee34@skole.hr
> 
> Changes in v5:
> - Address maintainer comments:
>   - Move *_NR_CLKS to clock driver from dt binding file
> - Allocate correct number of clocks for each block instead of blindly
>   allocating 50 for each
> - Link to v4: https://lore.kernel.org/r/20230807-pxa1908-lkml-v4-0-cb387d73b452@skole.hr
> 
> Changes in v4:
> - Address maintainer comments:
>   - Relicense clock binding file to BSD-2
> - Add pinctrl-names to SD card node
> - Add vgic registers to GIC node
> - Rebase on v6.5-rc5
> - Link to v3: https://lore.kernel.org/r/20230804-pxa1908-lkml-v3-0-8e48fca37099@skole.hr
> 
> Changes in v3:
> - Address maintainer comments:
>   - Drop GPIO dynamic allocation patch
>   - Move clock register offsets into driver (instead of bindings file)
>   - Add missing Tested-by trailer to u32_fract patch
>   - Move SoC binding to arm/mrvl/mrvl.yaml
> - Add serial0 alias and stdout-path to board dts to enable UART
>   debugging
> - Rebase on v6.5-rc4
> - Link to v2: https://lore.kernel.org/r/20230727162909.6031-1-duje.mihanovic@skole.hr
> 
> Changes in v2:
> - Remove earlycon patch as it's been merged into tty-next
> - Address maintainer comments:
>   - Clarify GPIO regressions on older PXA platforms
>   - Add Fixes tag to commit disabling GPIO pinctrl calls for this SoC
>   - Add missing includes to clock driver
>   - Clock driver uses HZ_PER_MHZ, u32_fract and GENMASK
>   - Dual license clock bindings
>   - Change clock IDs to decimal
>   - Fix underscores in dt node names
>   - Move chosen node to top of board dts
>   - Clean up documentation
>   - Reorder commits
>   - Drop pxa,rev-id
> - Rename muic-i2c to i2c-muic
> - Reword some commits
> - Move framebuffer node to chosen
> - Add aliases for mmc nodes
> - Rebase on v6.5-rc3
> - Link to v1: https://lore.kernel.org/r/20230721210042.21535-1-duje.mihanovic@skole.hr
> 
> ---
> Andy Shevchenko (1):
>       clk: mmp: Switch to use struct u32_fract instead of custom one
> 
> Duje Mihanović (8):
>       dt-bindings: pinctrl: pinctrl-single: add marvell,pxa1908-padconf compatible
>       pinctrl: single: add marvell,pxa1908-padconf compatible
>       dt-bindings: clock: Add Marvell PXA1908 clock bindings
>       clk: mmp: Add Marvell PXA1908 clock driver
>       dt-bindings: marvell: Document PXA1908 SoC
>       arm64: Kconfig.platforms: Add config for Marvell PXA1908 platform
>       arm64: dts: Add DTS for Marvell PXA1908 and samsung,coreprimevelte
>       MAINTAINERS: add myself as Marvell PXA1908 maintainer
> 
>  .../devicetree/bindings/arm/mrvl/mrvl.yaml         |   5 +
>  .../devicetree/bindings/clock/marvell,pxa1908.yaml |  48 +++
>  .../bindings/pinctrl/pinctrl-single.yaml           |   4 +
>  MAINTAINERS                                        |   9 +
>  arch/arm64/Kconfig.platforms                       |   8 +
>  arch/arm64/boot/dts/marvell/Makefile               |   3 +
>  .../dts/marvell/pxa1908-samsung-coreprimevelte.dts | 336 +++++++++++++++++++++
>  arch/arm64/boot/dts/marvell/pxa1908.dtsi           | 304 +++++++++++++++++++
>  drivers/clk/mmp/Makefile                           |   2 +-
>  drivers/clk/mmp/clk-frac.c                         |  57 ++--
>  drivers/clk/mmp/clk-of-mmp2.c                      |  26 +-
>  drivers/clk/mmp/clk-of-pxa168.c                    |   4 +-
>  drivers/clk/mmp/clk-of-pxa1908.c                   | 328 ++++++++++++++++++++
>  drivers/clk/mmp/clk-of-pxa1928.c                   |   6 +-
>  drivers/clk/mmp/clk-of-pxa910.c                    |   4 +-
>  drivers/clk/mmp/clk.h                              |  10 +-
>  drivers/pinctrl/pinctrl-single.c                   |   1 +
>  include/dt-bindings/clock/marvell,pxa1908.h        |  88 ++++++
>  18 files changed, 1186 insertions(+), 57 deletions(-)
> ---
> base-commit: 39cd87c4eb2b893354f3b850f916353f2658ae6f
> change-id: 20230803-pxa1908-lkml-6830e8da45c7
> 
> Best regards,
> --
> Duje Mihanović <duje.mihanovic@skole.hr>
> 
> 
> 


My bot found new DTB warnings on the .dts files added or changed in this
series.

Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
are fixed by another series. Ultimately, it is up to the platform
maintainer whether these warnings are acceptable or not. No need to reply
unless the platform maintainer has comments.

If you already ran DT checks and didn't see these error(s), then
make sure dt-schema is up to date:

  pip3 install dtschema --upgrade


New warnings running 'make CHECK_DTBS=y marvell/pxa1908-samsung-coreprimevelte.dtb' for 20240402-pxa1908-lkml-v9-0-25a003e83c6f@skole.hr:

arch/arm64/boot/dts/marvell/pxa1908-samsung-coreprimevelte.dtb: /: memory: False schema does not allow {'device_type': ['memory'], 'reg': [[0, 0, 0, 0]]}
	from schema $id: http://devicetree.org/schemas/root-node.yaml#
arch/arm64/boot/dts/marvell/pxa1908-samsung-coreprimevelte.dtb: pinmux@1e000: #size-cells: 0 was expected
	from schema $id: http://devicetree.org/schemas/pinctrl/pinctrl-single.yaml#
arch/arm64/boot/dts/marvell/pxa1908-samsung-coreprimevelte.dtb: pinmux@1e000: pinctrl-single,gpio-range: [[8, 55, 55, 0], [8, 110, 32, 0], [8, 52, 1, 0]] is too long
	from schema $id: http://devicetree.org/schemas/pinctrl/pinctrl-single.yaml#
arch/arm64/boot/dts/marvell/pxa1908-samsung-coreprimevelte.dtb: pinmux@1e000: 'pinmux-board-1', 'pinmux-board-2', 'pinmux-board-3', 'pinmux-gpio-keys', 'pinmux-i2c-muic', 'pinmux-sdh0-1', 'pinmux-sdh0-2', 'pinmux-sdh0-3', 'pinmux-uart0', 'ranges' do not match any of the regexes: '-pins(-[0-9]+)?$|-pin$', 'pinctrl-[0-9]+'
	from schema $id: http://devicetree.org/schemas/pinctrl/pinctrl-single.yaml#
arch/arm64/boot/dts/marvell/pxa1908-samsung-coreprimevelte.dtb: mmc@80000: pinctrl-names: ['default'] is too short
	from schema $id: http://devicetree.org/schemas/mmc/sdhci-pxa.yaml#
arch/arm64/boot/dts/marvell/pxa1908-samsung-coreprimevelte.dtb: mmc@80000: Unevaluated properties are not allowed ('pinctrl-names' was unexpected)
	from schema $id: http://devicetree.org/schemas/mmc/sdhci-pxa.yaml#






^ permalink raw reply

* Re: [PATCH v1 0/4] arm64: dts: freescale: Add Toradex Colibri iMX8DX
From: Rob Herring @ 2024-04-03 14:15 UTC (permalink / raw)
  To: Hiago De Franco
  Cc: Krzysztof Kozlowski, linux-arm-kernel, Sascha Hauer,
	Pengutronix Kernel Team, Conor Dooley, Hiago De Franco, imx,
	devicetree, Fabio Estevam, Shawn Guo, linux-kernel
In-Reply-To: <20240402193512.240417-1-hiagofranco@gmail.com>


On Tue, 02 Apr 2024 16:35:08 -0300, Hiago De Franco wrote:
> From: Hiago De Franco <hiago.franco@toradex.com>
> 
> This patch series introduces support for Colibri iMX8DX SoM and its
> carrier boards, where the board can be mated with: Aster, Evaluation Board
> v3, Iris v2, and Iris v1. This SoM is a variant of the already supported
> Colibri iMX8QXP, utilizing an NXP i.MX8DX SoC instead of i.MX8QXP.
> Therefore, this patch series also adds support for the i.MX8DX processor.
> 
> Hiago De Franco (4):
>   arm64: dts: freescale: Add i.MX8DX dtsi
>   dt-bindings: arm: fsl: remove reduntant toradex,colibri-imx8x
>   dt-bindings: arm: fsl: Add Colibri iMX8DX
>   arm64: dts: freescale: Add Toradex Colibri iMX8DX
> 
>  Documentation/devicetree/bindings/arm/fsl.yaml   |  7 ++++---
>  arch/arm64/boot/dts/freescale/Makefile           |  4 ++++
>  .../boot/dts/freescale/imx8dx-colibri-aster.dts  | 16 ++++++++++++++++
>  .../dts/freescale/imx8dx-colibri-eval-v3.dts     | 16 ++++++++++++++++
>  .../dts/freescale/imx8dx-colibri-iris-v2.dts     | 16 ++++++++++++++++
>  .../boot/dts/freescale/imx8dx-colibri-iris.dts   | 16 ++++++++++++++++
>  .../arm64/boot/dts/freescale/imx8dx-colibri.dtsi | 11 +++++++++++
>  arch/arm64/boot/dts/freescale/imx8dx.dtsi        | 13 +++++++++++++
>  8 files changed, 96 insertions(+), 3 deletions(-)
>  create mode 100644 arch/arm64/boot/dts/freescale/imx8dx-colibri-aster.dts
>  create mode 100644 arch/arm64/boot/dts/freescale/imx8dx-colibri-eval-v3.dts
>  create mode 100644 arch/arm64/boot/dts/freescale/imx8dx-colibri-iris-v2.dts
>  create mode 100644 arch/arm64/boot/dts/freescale/imx8dx-colibri-iris.dts
>  create mode 100644 arch/arm64/boot/dts/freescale/imx8dx-colibri.dtsi
>  create mode 100644 arch/arm64/boot/dts/freescale/imx8dx.dtsi
> 
> --
> 2.39.2
> 
> 
> 


My bot found new DTB warnings on the .dts files added or changed in this
series.

Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
are fixed by another series. Ultimately, it is up to the platform
maintainer whether these warnings are acceptable or not. No need to reply
unless the platform maintainer has comments.

If you already ran DT checks and didn't see these error(s), then
make sure dt-schema is up to date:

  pip3 install dtschema --upgrade


New warnings running 'make CHECK_DTBS=y freescale/imx8dx-colibri-aster.dtb freescale/imx8dx-colibri-eval-v3.dtb freescale/imx8dx-colibri-iris-v2.dtb freescale/imx8dx-colibri-iris.dtb' for 20240402193512.240417-1-hiagofranco@gmail.com:

arch/arm64/boot/dts/freescale/imx8dx-colibri-aster.dtb: jpegdec@58400000: 'assigned-clock-rates', 'assigned-clocks', 'clock-names', 'clocks', 'slot' do not match any of the regexes: 'pinctrl-[0-9]+'
	from schema $id: http://devicetree.org/schemas/media/nxp,imx8-jpeg.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-aster.dtb: jpegenc@58450000: 'assigned-clock-rates', 'assigned-clocks', 'clock-names', 'clocks', 'slot' do not match any of the regexes: 'pinctrl-[0-9]+'
	from schema $id: http://devicetree.org/schemas/media/nxp,imx8-jpeg.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-eval-v3.dtb: jpegdec@58400000: 'assigned-clock-rates', 'assigned-clocks', 'clock-names', 'clocks', 'slot' do not match any of the regexes: 'pinctrl-[0-9]+'
	from schema $id: http://devicetree.org/schemas/media/nxp,imx8-jpeg.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-eval-v3.dtb: jpegenc@58450000: 'assigned-clock-rates', 'assigned-clocks', 'clock-names', 'clocks', 'slot' do not match any of the regexes: 'pinctrl-[0-9]+'
	from schema $id: http://devicetree.org/schemas/media/nxp,imx8-jpeg.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-iris-v2.dtb: jpegdec@58400000: 'assigned-clock-rates', 'assigned-clocks', 'clock-names', 'clocks', 'slot' do not match any of the regexes: 'pinctrl-[0-9]+'
	from schema $id: http://devicetree.org/schemas/media/nxp,imx8-jpeg.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-iris-v2.dtb: jpegenc@58450000: 'assigned-clock-rates', 'assigned-clocks', 'clock-names', 'clocks', 'slot' do not match any of the regexes: 'pinctrl-[0-9]+'
	from schema $id: http://devicetree.org/schemas/media/nxp,imx8-jpeg.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-iris.dtb: jpegdec@58400000: 'assigned-clock-rates', 'assigned-clocks', 'clock-names', 'clocks', 'slot' do not match any of the regexes: 'pinctrl-[0-9]+'
	from schema $id: http://devicetree.org/schemas/media/nxp,imx8-jpeg.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-iris.dtb: jpegenc@58450000: 'assigned-clock-rates', 'assigned-clocks', 'clock-names', 'clocks', 'slot' do not match any of the regexes: 'pinctrl-[0-9]+'
	from schema $id: http://devicetree.org/schemas/media/nxp,imx8-jpeg.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-aster.dtb: dma-controller@591f0000: 'clocks' is a required property
	from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-aster.dtb: dma-controller@591f0000: Unevaluated properties are not allowed ('power-domains' was unexpected)
	from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-eval-v3.dtb: dma-controller@591f0000: 'clocks' is a required property
	from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-aster.dtb: dma-controller@599f0000: 'clocks' is a required property
	from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-eval-v3.dtb: dma-controller@591f0000: Unevaluated properties are not allowed ('power-domains' was unexpected)
	from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-aster.dtb: dma-controller@599f0000: Unevaluated properties are not allowed ('power-domains' was unexpected)
	from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-iris-v2.dtb: dma-controller@591f0000: 'clocks' is a required property
	from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-iris-v2.dtb: dma-controller@591f0000: Unevaluated properties are not allowed ('power-domains' was unexpected)
	from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-eval-v3.dtb: dma-controller@599f0000: 'clocks' is a required property
	from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-eval-v3.dtb: dma-controller@599f0000: Unevaluated properties are not allowed ('power-domains' was unexpected)
	from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-iris.dtb: dma-controller@591f0000: 'clocks' is a required property
	from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-iris.dtb: dma-controller@591f0000: Unevaluated properties are not allowed ('power-domains' was unexpected)
	from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-iris-v2.dtb: dma-controller@599f0000: 'clocks' is a required property
	from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-iris-v2.dtb: dma-controller@599f0000: Unevaluated properties are not allowed ('power-domains' was unexpected)
	from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-iris.dtb: dma-controller@599f0000: 'clocks' is a required property
	from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-iris.dtb: dma-controller@599f0000: Unevaluated properties are not allowed ('power-domains' was unexpected)
	from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-aster.dtb: dma-controller@5a1f0000: 'clocks' is a required property
	from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-aster.dtb: dma-controller@5a1f0000: Unevaluated properties are not allowed ('power-domains' was unexpected)
	from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-eval-v3.dtb: /bus@5a000000/spi@5a020000/can@0: failed to match any schema with compatible: ['microchip,mcp2515']
arch/arm64/boot/dts/freescale/imx8dx-colibri-eval-v3.dtb: dma-controller@5a1f0000: 'clocks' is a required property
	from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-eval-v3.dtb: dma-controller@5a1f0000: Unevaluated properties are not allowed ('power-domains' was unexpected)
	from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-iris-v2.dtb: dma-controller@5a1f0000: 'clocks' is a required property
	from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-iris-v2.dtb: dma-controller@5a1f0000: Unevaluated properties are not allowed ('power-domains' was unexpected)
	from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-aster.dtb: touchscreen@2c: adi,first-conversion-delay: b'\x03' is not of type 'object', 'integer', 'array', 'boolean', 'null'
	from schema $id: http://devicetree.org/schemas/dt-core.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-aster.dtb: touchscreen@2c: adi,acquisition-time: b'\x01' is not of type 'object', 'integer', 'array', 'boolean', 'null'
	from schema $id: http://devicetree.org/schemas/dt-core.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-aster.dtb: touchscreen@2c: adi,median-filter-size: b'\x02' is not of type 'object', 'integer', 'array', 'boolean', 'null'
	from schema $id: http://devicetree.org/schemas/dt-core.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-aster.dtb: touchscreen@2c: adi,averaging: b'\x01' is not of type 'object', 'integer', 'array', 'boolean', 'null'
	from schema $id: http://devicetree.org/schemas/dt-core.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-aster.dtb: touchscreen@2c: adi,conversion-interval: b'\xff' is not of type 'object', 'integer', 'array', 'boolean', 'null'
	from schema $id: http://devicetree.org/schemas/dt-core.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-iris.dtb: dma-controller@5a1f0000: 'clocks' is a required property
	from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-iris.dtb: dma-controller@5a1f0000: Unevaluated properties are not allowed ('power-domains' was unexpected)
	from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-aster.dtb: /bus@5a000000/i2c@5a800000/touchscreen@2c: failed to match any schema with compatible: ['adi,ad7879-1']
arch/arm64/boot/dts/freescale/imx8dx-colibri-eval-v3.dtb: touchscreen@2c: adi,first-conversion-delay: b'\x03' is not of type 'object', 'integer', 'array', 'boolean', 'null'
	from schema $id: http://devicetree.org/schemas/dt-core.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-eval-v3.dtb: touchscreen@2c: adi,acquisition-time: b'\x01' is not of type 'object', 'integer', 'array', 'boolean', 'null'
	from schema $id: http://devicetree.org/schemas/dt-core.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-eval-v3.dtb: touchscreen@2c: adi,median-filter-size: b'\x02' is not of type 'object', 'integer', 'array', 'boolean', 'null'
	from schema $id: http://devicetree.org/schemas/dt-core.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-eval-v3.dtb: touchscreen@2c: adi,averaging: b'\x01' is not of type 'object', 'integer', 'array', 'boolean', 'null'
	from schema $id: http://devicetree.org/schemas/dt-core.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-eval-v3.dtb: touchscreen@2c: adi,conversion-interval: b'\xff' is not of type 'object', 'integer', 'array', 'boolean', 'null'
	from schema $id: http://devicetree.org/schemas/dt-core.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-aster.dtb: dma-controller@5a9f0000: 'clocks' is a required property
	from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-eval-v3.dtb: /bus@5a000000/i2c@5a800000/touchscreen@2c: failed to match any schema with compatible: ['adi,ad7879-1']
arch/arm64/boot/dts/freescale/imx8dx-colibri-aster.dtb: dma-controller@5a9f0000: Unevaluated properties are not allowed ('power-domains' was unexpected)
	from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-iris-v2.dtb: touchscreen@2c: adi,first-conversion-delay: b'\x03' is not of type 'object', 'integer', 'array', 'boolean', 'null'
	from schema $id: http://devicetree.org/schemas/dt-core.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-iris-v2.dtb: touchscreen@2c: adi,acquisition-time: b'\x01' is not of type 'object', 'integer', 'array', 'boolean', 'null'
	from schema $id: http://devicetree.org/schemas/dt-core.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-iris-v2.dtb: touchscreen@2c: adi,median-filter-size: b'\x02' is not of type 'object', 'integer', 'array', 'boolean', 'null'
	from schema $id: http://devicetree.org/schemas/dt-core.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-iris-v2.dtb: touchscreen@2c: adi,averaging: b'\x01' is not of type 'object', 'integer', 'array', 'boolean', 'null'
	from schema $id: http://devicetree.org/schemas/dt-core.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-iris-v2.dtb: touchscreen@2c: adi,conversion-interval: b'\xff' is not of type 'object', 'integer', 'array', 'boolean', 'null'
	from schema $id: http://devicetree.org/schemas/dt-core.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-iris-v2.dtb: /bus@5a000000/i2c@5a800000/touchscreen@2c: failed to match any schema with compatible: ['adi,ad7879-1']
arch/arm64/boot/dts/freescale/imx8dx-colibri-iris.dtb: touchscreen@2c: adi,first-conversion-delay: b'\x03' is not of type 'object', 'integer', 'array', 'boolean', 'null'
	from schema $id: http://devicetree.org/schemas/dt-core.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-iris.dtb: touchscreen@2c: adi,acquisition-time: b'\x01' is not of type 'object', 'integer', 'array', 'boolean', 'null'
	from schema $id: http://devicetree.org/schemas/dt-core.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-iris.dtb: touchscreen@2c: adi,median-filter-size: b'\x02' is not of type 'object', 'integer', 'array', 'boolean', 'null'
	from schema $id: http://devicetree.org/schemas/dt-core.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-iris.dtb: touchscreen@2c: adi,averaging: b'\x01' is not of type 'object', 'integer', 'array', 'boolean', 'null'
	from schema $id: http://devicetree.org/schemas/dt-core.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-iris.dtb: touchscreen@2c: adi,conversion-interval: b'\xff' is not of type 'object', 'integer', 'array', 'boolean', 'null'
	from schema $id: http://devicetree.org/schemas/dt-core.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-iris.dtb: /bus@5a000000/i2c@5a800000/touchscreen@2c: failed to match any schema with compatible: ['adi,ad7879-1']
arch/arm64/boot/dts/freescale/imx8dx-colibri-eval-v3.dtb: dma-controller@5a9f0000: 'clocks' is a required property
	from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-eval-v3.dtb: dma-controller@5a9f0000: Unevaluated properties are not allowed ('power-domains' was unexpected)
	from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-iris-v2.dtb: dma-controller@5a9f0000: 'clocks' is a required property
	from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-iris-v2.dtb: dma-controller@5a9f0000: Unevaluated properties are not allowed ('power-domains' was unexpected)
	from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-iris.dtb: dma-controller@5a9f0000: 'clocks' is a required property
	from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
arch/arm64/boot/dts/freescale/imx8dx-colibri-iris.dtb: dma-controller@5a9f0000: Unevaluated properties are not allowed ('power-domains' was unexpected)
	from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#






^ permalink raw reply

* Re: [PATCH v2 3/3] arm64: dts: qcom: msm8998: set qcom,no-msa-ready-indicator for wifi
From: Krzysztof Kozlowski @ 2024-04-03 14:14 UTC (permalink / raw)
  To: Marc Gonzalez, Dmitry Baryshkov
  Cc: Konrad Dybcio, Kalle Valo, Jeff Johnson, ath10k, wireless, DT,
	MSM, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Pierre-Hugues Husson, Arnaud Vrac, Bjorn Andersson, Jami Kettunen,
	Jeffrey Hugo
In-Reply-To: <91031ed0-104a-4752-8b1e-0dbe15ebf201@freebox.fr>

On 03/04/2024 15:05, Marc Gonzalez wrote:
> On 02/04/2024 17:55, Dmitry Baryshkov wrote:
> 
>> On Tue, 2 Apr 2024 at 18:31, Marc Gonzalez wrote:
>>
>>> So, if I understand correctly, I take this to mean that I should:
>>>
>>> 1) DELETE the qcom,no-msa-ready-indicator boolean property,
>>> 2) ADD a "qcom,msm8998-wifi" (name OK?) compatible,
>>
>> I'd say, this is not correct. There is no "msm8998-wifi".
> 
> Can you explain what you mean by:
> 'There is no "msm8998-wifi".' ?
> 
> Do you mean that: this compatible string does not exist?
> (I am proposing that it be created.)
> 
> Or do you mean that: "msm8998-wifi" is a bad name?

msm8998 SoC does not have WiFi.


> 
> 
> I meant to mimic these strings for various sub-blocks:
> 
> arch/arm64/boot/dts/qcom/msm8998.dtsi:          compatible = "qcom,msm8998-rpm-proc", "qcom,rpm-proc";
> arch/arm64/boot/dts/qcom/msm8998.dtsi:                                  compatible = "qcom,msm8998-rpmpd";

These are all parts of SoC. What you are adding is not part of original
SoC, I think.


Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH v2 3/3] arm64: dts: qcom: msm8998: set qcom,no-msa-ready-indicator for wifi
From: Dmitry Baryshkov @ 2024-04-03 14:12 UTC (permalink / raw)
  To: Marc Gonzalez
  Cc: Konrad Dybcio, Krzysztof Kozlowski, Kalle Valo, Jeff Johnson,
	ath10k, wireless, DT, MSM, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Pierre-Hugues Husson, Arnaud Vrac, Bjorn Andersson,
	Jami Kettunen, Jeffrey Hugo
In-Reply-To: <91031ed0-104a-4752-8b1e-0dbe15ebf201@freebox.fr>

On Wed, 3 Apr 2024 at 16:05, Marc Gonzalez <mgonzalez@freebox.fr> wrote:
>
> On 02/04/2024 17:55, Dmitry Baryshkov wrote:
>
> > On Tue, 2 Apr 2024 at 18:31, Marc Gonzalez wrote:
> >
> >> So, if I understand correctly, I take this to mean that I should:
> >>
> >> 1) DELETE the qcom,no-msa-ready-indicator boolean property,
> >> 2) ADD a "qcom,msm8998-wifi" (name OK?) compatible,
> >
> > I'd say, this is not correct. There is no "msm8998-wifi".
>
> Can you explain what you mean by:
> 'There is no "msm8998-wifi".' ?
>
> Do you mean that: this compatible string does not exist?
> (I am proposing that it be created.)
>
> Or do you mean that: "msm8998-wifi" is a bad name?

I mean, it is qcom,wcn3990-wifi, because the chip is wcn3990.

> And these strings in ath11k:
>
> Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml:      - qcom,ipq8074-wifi
> Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml:      - qcom,ipq6018-wifi
> Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml:      - qcom,wcn6750-wifi
> Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml:      - qcom,ipq5018-wifi

I must admit, I don't know the IPQ product naming (it well might be
that it is both the name of the SoC and the WiFI SoC).

>
> > I'd say, we should take a step back and actually verify how this was
> > handled in the vendor kernel.
>
> In our commercial product, we use the ath10k driver in the vendor kernel (v4.4 r38-rel).

I see.

> It looks like Jeff has already performed the code analysis
> wrt vendor vs mainline (including user-space tools).

From his message it looks like we are expected to get MSA READY even on msm8998.

-- 
With best wishes
Dmitry

^ permalink raw reply

* Re: Fixing the devicetree of Rock 5 Model B (and possibly others)
From: Sebastian Reichel @ 2024-04-03 13:52 UTC (permalink / raw)
  To: Pratham Patel
  Cc: Saravana Kannan, Dragan Simic, devicetree, linux-arm-kernel,
	linux-rockchip, linux-kernel, regressions, stable
In-Reply-To: <D0A2ZL6S8UG6.2BQKIBQWYB36D@thefossguy.com>

[-- Attachment #1: Type: text/plain, Size: 1529 bytes --]

Hi,

On Wed, Apr 03, 2024 at 01:03:07AM +0000, Pratham Patel wrote:
> > > > Also, can you give the output of <debugfs>/devices_deferred for the
> > > > good vs bad case?
> > >
> > > I can't provide you with requested output from the bad case, since the
> > > kernel never moves past this to an initramfs rescue shell, but following
> > > is the output from v6.8.1 (**with aforementioned patch reverted**).
> > >
> > > # cat /sys/kernel/debug/devices_deferred
> > > fc400000.usb    platform: wait for supplier /phy@fed90000/usb3-port
> > > 1-0022  typec_fusb302: cannot register tcpm port
> > > fc000000.usb    platform: wait for supplier /phy@fed80000/usb3-port
> > >
> > > It seems that v6.8.2 works _without needing to revert the patch_. I will
> > > have to look into this sometime this week but it seems like
> > > a8037ceb8964 (arm64: dts: rockchip: drop rockchip,trcm-sync-tx-only from rk3588 i2s)
> > > seems to be the one that fixed the root issue. I will have to test it
> > > sometime later this week.
> >
> > Ok, once you find the patch that fixes things, let me know too.
> 
> Will do!

FWIW the v6.8.1 kernel referenced above is definitely patched, since
upstream's Rock 5B DT does neither describe fusb302, nor the USB
port it is connected to.

We have a few Rock 5B in Kernel CI and upstream boots perfectly
fine:

https://lava.collabora.dev/scheduler/device_type/rk3588-rock-5b

So it could be one of your downstream patches, which is introducing
this problem.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH 2/2] arm64: dts: qcom: sc7180: Fix UFS PHY clocks
From: Manivannan Sadhasivam @ 2024-04-03 13:54 UTC (permalink / raw)
  To: Danila Tikhonov
  Cc: andersson, konrad.dybcio, vkoul, kishon, robh,
	krzysztof.kozlowski+dt, conor+dt, cros-qcom-dts-watchers,
	davidwronek, linux-arm-msm, linux-phy, devicetree, linux-kernel
In-Reply-To: <20240401182240.55282-3-danila@jiaxyga.com>

On Mon, Apr 01, 2024 at 09:22:40PM +0300, Danila Tikhonov wrote:
> QMP PHY used in SC7180 requires 3 clocks:
> 
> * ref - 19.2MHz reference clock from RPMh
> * ref_aux - Auxiliary reference clock from GCC
> * qref - QREF clock from GCC
> 
> While at it, let's move 'clocks' property before 'clock-names' to match
> the style used commonly.
> 
> Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>

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

- Mani

> ---
>  arch/arm64/boot/dts/qcom/sc7180.dtsi | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> index 2b481e20ae38..5c9ec8047f00 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> @@ -1585,9 +1585,12 @@ ufs_mem_phy: phy@1d87000 {
>  			compatible = "qcom,sc7180-qmp-ufs-phy",
>  				     "qcom,sm7150-qmp-ufs-phy";
>  			reg = <0 0x01d87000 0 0x1000>;
> -			clocks = <&gcc GCC_UFS_MEM_CLKREF_CLK>,
> -				 <&gcc GCC_UFS_PHY_PHY_AUX_CLK>;
> -			clock-names = "ref", "ref_aux";
> +			clocks = <&rpmhcc RPMH_CXO_CLK>,
> +				 <&gcc GCC_UFS_PHY_PHY_AUX_CLK>,
> +				 <&gcc GCC_UFS_MEM_CLKREF_CLK>;
> +			clock-names = "ref",
> +				      "ref_aux",
> +				      "qref";
>  			power-domains = <&gcc UFS_PHY_GDSC>;
>  			resets = <&ufs_mem_hc 0>;
>  			reset-names = "ufsphy";
> -- 
> 2.44.0
> 

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

^ permalink raw reply

* Re: [PATCH 1/2] dt-bindings: phy: qmp-ufs: Fix PHY clocks for SC7180
From: Manivannan Sadhasivam @ 2024-04-03 13:53 UTC (permalink / raw)
  To: Danila Tikhonov
  Cc: andersson, konrad.dybcio, vkoul, kishon, robh,
	krzysztof.kozlowski+dt, conor+dt, cros-qcom-dts-watchers,
	davidwronek, linux-arm-msm, linux-phy, devicetree, linux-kernel
In-Reply-To: <20240401182240.55282-2-danila@jiaxyga.com>

On Mon, Apr 01, 2024 at 09:22:39PM +0300, Danila Tikhonov wrote:
> QMP UFS PHY used in SC7180 requires 3 clocks:
> 
> * ref - 19.2MHz reference clock from RPMh
> * ref_aux - Auxiliary reference clock from GCC
> * qref - QREF clock from GCC
> 
> This change obviously breaks the ABI, but it is inevitable since the
> clock topology needs to be accurately described in the binding.
> 
> Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>

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

- Mani

> ---
>  .../devicetree/bindings/phy/qcom,sc8280xp-qmp-ufs-phy.yaml       | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-ufs-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-ufs-phy.yaml
> index 91a6cc38ff7f..a79fde9a8cdf 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-ufs-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-ufs-phy.yaml
> @@ -86,6 +86,7 @@ allOf:
>              enum:
>                - qcom,msm8998-qmp-ufs-phy
>                - qcom,sa8775p-qmp-ufs-phy
> +              - qcom,sc7180-qmp-ufs-phy
>                - qcom,sc7280-qmp-ufs-phy
>                - qcom,sc8180x-qmp-ufs-phy
>                - qcom,sc8280xp-qmp-ufs-phy
> -- 
> 2.44.0
> 

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

^ permalink raw reply

* Re: [PATCH] dt-bindings: firmware: arm,scmi: Update examples for protocol@13
From: Sudeep Holla @ 2024-04-03 13:53 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Cristian Marussi, Rob Herring, Krzysztof Kozlowski, devicetree,
	linux-arm-kernel
In-Reply-To: <20240403111106.1110940-1-ulf.hansson@linaro.org>

On Wed, Apr 03, 2024 at 01:11:06PM +0200, Ulf Hansson wrote:
> Recently we extended the binding for protocol@13 to allow it to be modelled
> as a generic performance domain. In a way to promote using the new binding,
> let's update the examples.
>

Does it make sense to keep one DVFS example with #clock-cells until we
mark it as deprecated ? Otherwise it may be confusing as the binding still
lists. Or leave some comment in the example or something, I am open for
suggestions.

Other than that,

Acked-by: Sudeep Holla <sudeep.holla@arm.com>

-- 
Regards,
Sudeep

^ permalink raw reply

* Re: Fixing the devicetree of Rock 5 Model B (and possibly others)
From: Dragan Simic @ 2024-04-03 13:51 UTC (permalink / raw)
  To: Pratham Patel
  Cc: Saravana Kannan, sebastian.reichel, devicetree, linux-arm-kernel,
	linux-rockchip, linux-kernel, regressions, stable
In-Reply-To: <D0A122WK7CB9.33B2TP6UCMJBJ@thefossguy.com>

Hello Pratham,

On 2024-04-03 01:32, Pratham Patel wrote:
> On Tue Apr 2, 2024 at 4:54 AM IST, Saravana Kannan wrote:
>> On Sat, Mar 23, 2024 at 10:10 AM Dragan Simic <dsimic@manjaro.org> 
>> wrote:
>> > On 2024-03-23 18:02, Pratham Patel wrote:
>> > > I looked at the patch and tried several things, neither resulted in
>> > > anything that would point me to the core issue. Then I tried this:
>> >
>> > Could you, please, clarify a bit what's the actual issue you're
>> > experiencing on your Rock 5B?
>> 
>> Pratham, can you reply to this please? I don't really understand what
>> your issue is for me to be able to help.
> 
> I apologize for not replying. Somehow, I did not notice the reply from
> Dragan. :(

No worries, I saw the serial console log file in one of your messages,
which actually provided the answer to my question. :)

> Since this patch was applied, an issue in the Rock 5B's DT has been
> unearthed which now results in the kernel being unable to boot 
> properly.
> 
> Following is the relevant call trace from the UART capture:
> 
> [   21.595068] Call trace:
> [   21.595288]  smp_call_function_many_cond+0x174/0x5f8
> [   21.595728]  on_each_cpu_cond_mask+0x2c/0x40
> [   21.596109]  cpuidle_register_driver+0x294/0x318
> [   21.596524]  cpuidle_register+0x24/0x100
> [   21.596875]  psci_cpuidle_probe+0x2e4/0x490
> [   21.597247]  platform_probe+0x70/0xd0
> [   21.597575]  really_probe+0x18c/0x3d8
> [   21.597905]  __driver_probe_device+0x84/0x180
> [   21.598294]  driver_probe_device+0x44/0x120
> [   21.598669]  __device_attach_driver+0xc4/0x168
> [   21.599063]  bus_for_each_drv+0x8c/0xf0
> [   21.599408]  __device_attach+0xa4/0x1c0
> [   21.599748]  device_initial_probe+0x1c/0x30
> [   21.600118]  bus_probe_device+0xb4/0xc0
> [   21.600462]  device_add+0x68c/0x888
> [   21.600775]  platform_device_add+0x19c/0x270
> [   21.601154]  platform_device_register_full+0xdc/0x178
> [   21.601602]  psci_idle_init+0xa0/0xc8
> [   21.601934]  do_one_initcall+0x60/0x290
> [   21.602275]  kernel_init_freeable+0x20c/0x3e0
> [   21.602664]  kernel_init+0x2c/0x1f8
> [   21.602979]  ret_from_fork+0x10/0x20
> 
>> Also, can you give the output of <debugfs>/devices_deferred for the
>> good vs bad case?
> 
> I can't provide you with requested output from the bad case, since the
> kernel never moves past this to an initramfs rescue shell, but 
> following
> is the output from v6.8.1 (**with aforementioned patch reverted**).
> 
> # cat /sys/kernel/debug/devices_deferred
> fc400000.usb    platform: wait for supplier /phy@fed90000/usb3-port
> 1-0022  typec_fusb302: cannot register tcpm port
> fc000000.usb    platform: wait for supplier /phy@fed80000/usb3-port
> 
> It seems that v6.8.2 works _without needing to revert the patch_. I 
> will
> have to look into this sometime this week but it seems like
> a8037ceb8964 (arm64: dts: rockchip: drop rockchip,trcm-sync-tx-only
> from rk3588 i2s)
> seems to be the one that fixed the root issue. I will have to test it
> sometime later this week.

^ permalink raw reply

* Re: [PATCH 3/3] arm64: dts: qcom: msm8996: Add missing UFS host controller reset
From: Manivannan Sadhasivam @ 2024-04-03 13:50 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Bjorn Andersson, Konrad Dybcio, Alim Akhtar, Avri Altman,
	Bart Van Assche, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Andy Gross, Andy Gross, James E.J. Bottomley, Martin K. Petersen,
	linux-arm-msm, linux-scsi, devicetree, linux-kernel
In-Reply-To: <CAA8EJppOPrSfD=VkHm8M0P07=mN_BikT=cGvLe6UFL3OpKVWzA@mail.gmail.com>

On Tue, Apr 02, 2024 at 10:25:32PM +0300, Dmitry Baryshkov wrote:
> On Tue, 2 Apr 2024 at 20:35, Bjorn Andersson <andersson@kernel.org> wrote:
> >
> > On Fri, Feb 09, 2024 at 10:16:25PM +0200, Dmitry Baryshkov wrote:
> > > On Tue, 30 Jan 2024 at 08:55, Manivannan Sadhasivam
> > > <manivannan.sadhasivam@linaro.org> wrote:
> > > >
> > > > On Mon, Jan 29, 2024 at 11:44:15AM +0200, Dmitry Baryshkov wrote:
> > > > > On Mon, 29 Jan 2024 at 09:55, Manivannan Sadhasivam
> > > > > <manivannan.sadhasivam@linaro.org> wrote:
> > > > > >
> > > > > > UFS host controller reset is required for the drivers to properly reset the
> > > > > > controller. Hence, add it.
> > > > > >
> > > > > > Fixes: 57fc67ef0d35 ("arm64: dts: qcom: msm8996: Add ufs related nodes")
> > > > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > > >
> > > > > I think I had issues previously when I attempted to reset the
> > > > > controller, but it might be because of the incomplete clocks
> > > > > programming. Let met check whether it works first.
> > > > >
> > > >
> > > > Sure. Please let me know.
> > >
> > > With the clocking fixes in place (I'll send them in a few minutes) and
> > > with this patch the UFS breaks in the following way:
> > >
> >
> > Was this further reviewed/investigated? What would you like me to do
> > with this patch?
> 
> I'd say, hold until we can understand what is going on.
> 
> Mani, If you have any ideas what can be causing the issue, I'm open to
> testing any ideas or any patches from your side.
> Judging that the UFS breaks after toggling the reset, we might be
> missing some setup steps.
> 

I've bombarded the Qcom UFS team with too many questions, but didn't get answer
for all of them. And this is one of those questions.

Let me try to dig further and come back.

- Mani

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

^ permalink raw reply

* [PATCH v2 2/3] ARM: dts: Disable unused ADC channels for Asrock X570D4U BMC
From: Renze Nicolai @ 2024-04-03 13:28 UTC (permalink / raw)
  To: renze, linux-arm-kernel, devicetree, linux-kernel, linux-aspeed,
	arnd, olof, soc, robh+dt, krzysztof.kozlowski+dt, joel, andrew
In-Reply-To: <20240403133037.37782-1-renze@rnplus.nl>

Additionally adds labels describing the ADC channels.

Signed-off-by: Renze Nicolai <renze@rnplus.nl>
---
 .../dts/aspeed/aspeed-bmc-asrock-x570d4u.dts  | 29 +++++++++----------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts
index dff69d6ff146..66318ef8caae 100644
--- a/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts
+++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts
@@ -337,20 +337,17 @@ fan@5 {
 &adc {
 	status = "okay";
 	pinctrl-names = "default";
-	pinctrl-0 = <&pinctrl_adc0_default
-				&pinctrl_adc1_default
-				&pinctrl_adc2_default
-				&pinctrl_adc3_default
-				&pinctrl_adc4_default
-				&pinctrl_adc5_default
-				&pinctrl_adc6_default
-				&pinctrl_adc7_default
-				&pinctrl_adc8_default
-				&pinctrl_adc9_default
-				&pinctrl_adc10_default
-				&pinctrl_adc11_default
-				&pinctrl_adc12_default
-				&pinctrl_adc13_default
-				&pinctrl_adc14_default
-				&pinctrl_adc15_default>;
+	pinctrl-0 = <&pinctrl_adc0_default       /* 3VSB */
+			&pinctrl_adc1_default    /* 5VSB */
+			&pinctrl_adc2_default    /* VCPU */
+			&pinctrl_adc3_default    /* VSOC */
+			&pinctrl_adc4_default    /* VCCM */
+			&pinctrl_adc5_default    /* APU-VDDP */
+			&pinctrl_adc6_default    /* PM-VDD-CLDO */
+			&pinctrl_adc7_default    /* PM-VDDCR-S5 */
+			&pinctrl_adc8_default    /* PM-VDDCR */
+			&pinctrl_adc9_default    /* VBAT */
+			&pinctrl_adc10_default   /* 3V */
+			&pinctrl_adc11_default   /* 5V */
+			&pinctrl_adc12_default>; /* 12V */
 };
-- 
2.44.0


^ permalink raw reply related

* [PATCH v2 0/3] ARM: dts: Update devicetree of Asrock X570D4U BMC
From: Renze Nicolai @ 2024-04-03 13:28 UTC (permalink / raw)
  To: renze, linux-arm-kernel, devicetree, linux-kernel, linux-aspeed,
	arnd, olof, soc, robh+dt, krzysztof.kozlowski+dt, joel, andrew

These patches change the GPIO table, ADC channel configuration and
I2C bus configuration of the devicetree for the X570D4U BMC as part of
ongoing efforts to support OpenBMC on this platform.

Changes since v1:
 - Fixed warnings indicated by checkpatch.pl
 - Change commit message of ADC channels commit to match imperative mood
 - Restructure GPIO table to better match other ASPEED devices
 - Clarify naming scheme better

Best regards,
Renze Nicolai

Renze Nicolai (3):
  ARM: dts: Modify GPIO table for Asrock X570D4U BMC
  ARM: dts: Disable unused ADC channels for Asrock X570D4U BMC
  ARM: dts: Modify I2C bus configuration

 .../dts/aspeed/aspeed-bmc-asrock-x570d4u.dts  | 131 ++++++++----------
 1 file changed, 57 insertions(+), 74 deletions(-)

-- 
2.44.0


^ permalink raw reply

* [PATCH v2 1/3] ARM: dts: Modify GPIO table for Asrock X570D4U BMC
From: Renze Nicolai @ 2024-04-03 13:28 UTC (permalink / raw)
  To: renze, linux-arm-kernel, devicetree, linux-kernel, linux-aspeed,
	arnd, olof, soc, robh+dt, krzysztof.kozlowski+dt, joel, andrew
In-Reply-To: <20240403133037.37782-1-renze@rnplus.nl>

Restructure GPIO table to fit maximum line length.

Fix mistakes found while working on OpenBMC
userland configuration and based on probing
the board.

Schematic for this board is not available.
Because of this the choice was made to
use a descriptive method for naming the
GPIOs.

 - Push-pull outputs start with output-*
 - Open-drain outputs start with control-*
 - LED outputs start with led-*
 - Inputs start with input-*
 - Button inputs start with button-*
 - Active low signals end with *-n

Signed-off-by: Renze Nicolai <renze@rnplus.nl>
---
 .../dts/aspeed/aspeed-bmc-asrock-x570d4u.dts  | 95 ++++++++-----------
 1 file changed, 37 insertions(+), 58 deletions(-)

diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts
index 3c975bc41ae7..dff69d6ff146 100644
--- a/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts
+++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts
@@ -79,64 +79,43 @@ iio-hwmon {
 &gpio {
 	status = "okay";
 	gpio-line-names =
-	/*A0-A3*/       "status-locatorled-n",                    "",                      "button-nmi-n",          "",
-	/*A4-A7*/       "",                                       "",                      "",                      "",
-	/*B0-B3*/       "input-bios-post-cmplt-n",                "",                      "",                      "",
-	/*B4-B7*/       "",                                       "",                      "",                      "",
-	/*C0-C3*/       "",                                       "",                      "",                      "",
-	/*C4-C7*/       "",                                       "",                      "control-locatorbutton", "",
-	/*D0-D3*/       "button-power",                           "control-power",         "button-reset",          "control-reset",
-	/*D4-D7*/       "",                                       "",                      "",                      "",
-	/*E0-E3*/       "",                                       "",                      "",                      "",
-	/*E4-E7*/       "",                                       "",                      "",                      "",
-	/*F0-F3*/       "",                                       "",                      "",                      "",
-	/*F4-F7*/       "",                                       "",                      "",                      "",
-	/*G0-G3*/       "output-rtc-battery-voltage-read-enable", "input-id0",             "input-id1",             "input-id2",
-	/*G4-G7*/       "input-alert1-n",                         "input-alert2-n",        "input-alert3-n",        "",
-	/*H0-H3*/       "",                                       "",                      "",                      "",
-	/*H4-H7*/       "input-mfg",                              "",                      "led-heartbeat-n",       "input-caseopen",
-	/*I0-I3*/       "",                                       "",                      "",                      "",
-	/*I4-I7*/       "",                                       "",                      "",                      "",
-	/*J0-J3*/       "output-bmc-ready",                       "",                      "",                      "",
-	/*J4-J7*/       "",                                       "",                      "",                      "",
-	/*K0-K3*/       "",                                       "",                      "",                      "",
-	/*K4-K7*/       "",                                       "",                      "",                      "",
-	/*L0-L3*/       "",                                       "",                      "",                      "",
-	/*L4-L7*/       "",                                       "",                      "",                      "",
-	/*M0-M3*/       "",                                       "",                      "",                      "",
-	/*M4-M7*/       "",                                       "",                      "",                      "",
-	/*N0-N3*/       "",                                       "",                      "",                      "",
-	/*N4-N7*/       "",                                       "",                      "",                      "",
-	/*O0-O3*/       "",                                       "",                      "",                      "",
-	/*O4-O7*/       "",                                       "",                      "",                      "",
-	/*P0-P3*/       "",                                       "",                      "",                      "",
-	/*P4-P7*/       "",                                       "",                      "",                      "",
-	/*Q0-Q3*/       "",                                       "",                      "",                      "",
-	/*Q4-Q7*/       "",                                       "",                      "",                      "",
-	/*R0-R3*/       "",                                       "",                      "",                      "",
-	/*R4-R7*/       "",                                       "",                      "",                      "",
-	/*S0-S3*/       "input-bmc-pchhot-n",                     "",                      "",                      "",
-	/*S4-S7*/       "",                                       "",                      "",                      "",
-	/*T0-T3*/       "",                                       "",                      "",                      "",
-	/*T4-T7*/       "",                                       "",                      "",                      "",
-	/*U0-U3*/       "",                                       "",                      "",                      "",
-	/*U4-U7*/       "",                                       "",                      "",                      "",
-	/*V0-V3*/       "",                                       "",                      "",                      "",
-	/*V4-V7*/       "",                                       "",                      "",                      "",
-	/*W0-W3*/       "",                                       "",                      "",                      "",
-	/*W4-W7*/       "",                                       "",                      "",                      "",
-	/*X0-X3*/       "",                                       "",                      "",                      "",
-	/*X4-X7*/       "",                                       "",                      "",                      "",
-	/*Y0-Y3*/       "",                                       "",                      "",                      "",
-	/*Y4-Y7*/       "",                                       "",                      "",                      "",
-	/*Z0-Z3*/       "",                                       "",                      "led-fault-n",           "output-bmc-throttle-n",
-	/*Z4-Z7*/       "",                                       "",                      "",                      "",
-	/*AA0-AA3*/     "input-cpu1-thermtrip-latch-n",           "",                      "input-cpu1-prochot-n",  "",
-	/*AA4-AC7*/     "",                                       "",                      "",                      "",
-	/*AB0-AB3*/     "",                                       "",                      "",                      "",
-	/*AB4-AC7*/     "",                                       "",                      "",                      "",
-	/*AC0-AC3*/     "",                                       "",                      "",                      "",
-	/*AC4-AC7*/     "",                                       "",                      "",                      "";
+	/*  A */ "input-locatorled-n", "", "", "", "", "", "", "",
+	/*  B */ "input-bios-post-cmplt-n", "", "", "", "", "", "", "",
+	/*  C */ "", "", "", "", "", "", "control-locatorbutton-n", "",
+	/*  D */ "button-power-n", "control-power-n", "button-reset-n",
+		 "control-reset-n", "", "", "", "",
+	/*  E */ "", "", "", "", "", "", "", "",
+	/*  F */ "", "", "", "", "", "", "", "",
+	/*  G */ "output-hwm-vbat-enable", "input-id0-n", "input-id1-n",
+		 "input-id2-n", "input-aux-smb-alert-n", "",
+		 "input-psu-smb-alert-n", "",
+	/*  H */ "", "", "", "", "input-mfg-mode-n", "",
+		 "led-heartbeat-n", "input-case-open-n",
+	/*  I */ "", "", "", "", "", "", "", "",
+	/*  J */ "output-bmc-ready-n", "", "", "", "", "", "", "",
+	/*  K */ "", "", "", "", "", "", "", "",
+	/*  L */ "", "", "", "", "", "", "", "",
+	/*  M */ "", "", "", "", "", "", "", "",
+	/*  N */ "", "", "", "", "", "", "", "",
+	/*  O */ "", "", "", "", "", "", "", "",
+	/*  P */ "", "", "", "", "", "", "", "",
+	/*  Q */ "", "", "", "", "input-bmc-smb-present-n", "", "",
+		 "input-pcie-wake-n",
+	/*  R */ "", "", "", "", "", "", "", "",
+	/*  S */ "input-bmc-pchhot-n", "", "", "", "", "", "", "",
+	/*  T */ "", "", "", "", "", "", "", "",
+	/*  U */ "", "", "", "", "", "", "", "",
+	/*  V */ "", "", "", "", "", "", "", "",
+	/*  W */ "", "", "", "", "", "", "", "",
+	/*  X */ "", "", "", "", "", "", "", "",
+	/*  Y */ "input-sleep-s3-n", "input-sleep-s5-n", "", "", "", "",
+		 "", "",
+	/*  Z */ "", "", "led-fault-n", "output-bmc-throttle-n", "", "",
+		 "", "",
+	/* AA */ "input-cpu1-thermtrip-latch-n", "",
+		 "input-cpu1-prochot-n", "", "", "", "", "",
+	/* AB */ "", "input-power-good", "", "", "", "", "", "",
+	/* AC */ "", "", "", "", "", "", "", "";
 };
 
 &fmc {
-- 
2.44.0


^ permalink raw reply related

* [PATCH v2 3/3] ARM: dts: Modify I2C bus configuration
From: Renze Nicolai @ 2024-04-03 13:28 UTC (permalink / raw)
  To: renze, linux-arm-kernel, devicetree, linux-kernel, linux-aspeed,
	arnd, olof, soc, robh+dt, krzysztof.kozlowski+dt, joel, andrew
In-Reply-To: <20240403133037.37782-1-renze@rnplus.nl>

Enable I2C bus 8 which is exposed on the IPMB_1 connector
on the X570D4U mainboard.

Additionally adds a descriptive comment to I2C busses 1 and 5.

Signed-off-by: Renze Nicolai <renze@rnplus.nl>
---
 arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts
index 66318ef8caae..8dee4faa9e07 100644
--- a/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts
+++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts
@@ -162,6 +162,7 @@ &i2c0 {
 };
 
 &i2c1 {
+	/* Hardware monitoring SMBus */
 	status = "okay";
 
 	w83773g@4c {
@@ -219,6 +220,7 @@ i2c4mux0ch3: i2c@3 {
 };
 
 &i2c5 {
+	/* SMBus on BMC connector (BMC_SMB_1) */
 	status = "okay";
 };
 
@@ -243,6 +245,11 @@ eth1_macaddress: macaddress@3f88 {
 	};
 };
 
+&i2c8 {
+	/* SMBus on intelligent platform management bus header (IPMB_1) */
+	status = "okay";
+};
+
 &gfx {
 	status = "okay";
 };
-- 
2.44.0


^ permalink raw reply related

* [PATCH RESEND] arm64: dts: qcom: qcm6490-idp: Name the regulators
From: Umang Chheda @ 2024-04-03 13:29 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-arm-msm, devicetree, linux-kernel, quic_uchheda

Without explicitly specifying names for the regulators they are named
based on the DeviceTree node name. This results in multiple regulators
with the same name, making it impossible to reason debug prints and
regulator_summary.

Signed-off-by: Umang Chheda <quic_uchheda@quicinc.com>
---
 arch/arm64/boot/dts/qcom/qcm6490-idp.dts | 41 ++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
index f8f8a43f638d..ac6d741868ca 100644
--- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
+++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
@@ -195,129 +195,151 @@ regulators-0 {
 		vdd-l14-l16-supply = <&vreg_s8b_1p272>;
 
 		vreg_s1b_1p872: smps1 {
+			regulator-name = "vreg_s1b_1p872";
 			regulator-min-microvolt = <1840000>;
 			regulator-max-microvolt = <2040000>;
 		};
 
 		vreg_s2b_0p876: smps2 {
+			regulator-name = "vreg_s2b_0p876";
 			regulator-min-microvolt = <570070>;
 			regulator-max-microvolt = <1050000>;
 		};
 
 		vreg_s7b_0p972: smps7 {
+			regulator-name = "vreg_s7b_0p972";
 			regulator-min-microvolt = <535000>;
 			regulator-max-microvolt = <1120000>;
 		};
 
 		vreg_s8b_1p272: smps8 {
+			regulator-name = "vreg_s8b_1p272";
 			regulator-min-microvolt = <1200000>;
 			regulator-max-microvolt = <1500000>;
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_RET>;
 		};
 
 		vreg_l1b_0p912: ldo1 {
+			regulator-name = "vreg_l1b_0p912";
 			regulator-min-microvolt = <825000>;
 			regulator-max-microvolt = <925000>;
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
 		};
 
 		vreg_l2b_3p072: ldo2 {
+			regulator-name = "vreg_l2b_3p072";
 			regulator-min-microvolt = <2700000>;
 			regulator-max-microvolt = <3544000>;
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
 		};
 
 		vreg_l3b_0p504: ldo3 {
+			regulator-name = "vreg_l3b_0p504";
 			regulator-min-microvolt = <312000>;
 			regulator-max-microvolt = <910000>;
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
 		};
 
 		vreg_l4b_0p752: ldo4 {
+			regulator-name = "vreg_l4b_0p752";
 			regulator-min-microvolt = <752000>;
 			regulator-max-microvolt = <820000>;
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
 		};
 
 		reg_l5b_0p752: ldo5 {
+			regulator-name = "reg_l5b_0p752";
 			regulator-min-microvolt = <552000>;
 			regulator-max-microvolt = <832000>;
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
 		};
 
 		vreg_l6b_1p2: ldo6 {
+			regulator-name = "vreg_l6b_1p2";
 			regulator-min-microvolt = <1140000>;
 			regulator-max-microvolt = <1260000>;
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
 		};
 
 		vreg_l7b_2p952: ldo7 {
+			regulator-name = "vreg_l7b_2p952";
 			regulator-min-microvolt = <2400000>;
 			regulator-max-microvolt = <3544000>;
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
 		};
 
 		vreg_l8b_0p904: ldo8 {
+			regulator-name = "vreg_l8b_0p904";
 			regulator-min-microvolt = <870000>;
 			regulator-max-microvolt = <970000>;
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
 		};
 
 		vreg_l9b_1p2: ldo9 {
+			regulator-name = "vreg_l9b_1p2";
 			regulator-min-microvolt = <1200000>;
 			regulator-max-microvolt = <1304000>;
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
 		};
 
 		vreg_l11b_1p504: ldo11 {
+			regulator-name = "vreg_l11b_1p504";
 			regulator-min-microvolt = <1504000>;
 			regulator-max-microvolt = <2000000>;
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
 		};
 
 		vreg_l12b_0p751: ldo12 {
+			regulator-name = "vreg_l12b_0p751";
 			regulator-min-microvolt = <751000>;
 			regulator-max-microvolt = <824000>;
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
 		};
 
 		vreg_l13b_0p53: ldo13 {
+			regulator-name = "vreg_l13b_0p53";
 			regulator-min-microvolt = <530000>;
 			regulator-max-microvolt = <824000>;
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
 		};
 
 		vreg_l14b_1p08: ldo14 {
+			regulator-name = "vreg_l14b_1p08";
 			regulator-min-microvolt = <1080000>;
 			regulator-max-microvolt = <1304000>;
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
 		};
 
 		vreg_l15b_0p765: ldo15 {
+			regulator-name = "vreg_l15b_0p765";
 			regulator-min-microvolt = <765000>;
 			regulator-max-microvolt = <1020000>;
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
 		};
 
 		vreg_l16b_1p1: ldo16 {
+			regulator-name = "vreg_l16b_1p1";
 			regulator-min-microvolt = <1100000>;
 			regulator-max-microvolt = <1300000>;
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
 		};
 
 		vreg_l17b_1p7: ldo17 {
+			regulator-name = "vreg_l17b_1p7";
 			regulator-min-microvolt = <1700000>;
 			regulator-max-microvolt = <1900000>;
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
 		};
 
 		vreg_l18b_1p8: ldo18 {
+			regulator-name = "vreg_l18b_1p8";
 			regulator-min-microvolt = <1800000>;
 			regulator-max-microvolt = <2000000>;
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
 		};
 
 		vreg_l19b_1p8: ldo19 {
+			regulator-name = "vreg_l19b_1p8";
 			regulator-min-microvolt = <1800000>;
 			regulator-max-microvolt = <2000000>;
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
@@ -349,109 +371,128 @@ regulators-1 {
 		vdd-bob-supply = <&vph_pwr>;
 
 		vreg_s1c_2p19: smps1 {
+			regulator-name = "vreg_s1c_2p19";
 			regulator-min-microvolt = <2190000>;
 			regulator-max-microvolt = <2210000>;
 		};
 
 		vreg_s2c_0p752: smps2 {
+			regulator-name = "vreg_s2c_0p752";
 			regulator-min-microvolt = <750000>;
 			regulator-max-microvolt = <800000>;
 		};
 
 		vreg_s5c_0p752: smps5 {
+			regulator-name = "vreg_s5c_0p752";
 			regulator-min-microvolt = <465000>;
 			regulator-max-microvolt = <1050000>;
 		};
 
 		vreg_s7c_0p752: smps7 {
+			regulator-name = "vreg_s7c_0p752";
 			regulator-min-microvolt = <465000>;
 			regulator-max-microvolt = <800000>;
 		};
 
 		vreg_s9c_1p084: smps9 {
+			regulator-name = "vreg_s9c_1p084";
 			regulator-min-microvolt = <1010000>;
 			regulator-max-microvolt = <1170000>;
 		};
 
 		vreg_l1c_1p8: ldo1 {
+			regulator-name = "vreg_l1c_1p8";
 			regulator-min-microvolt = <1800000>;
 			regulator-max-microvolt = <1980000>;
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
 		};
 
 		vreg_l2c_1p62: ldo2 {
+			regulator-name = "vreg_l2c_1p62";
 			regulator-min-microvolt = <1620000>;
 			regulator-max-microvolt = <1980000>;
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
 		};
 
 		vreg_l3c_2p8: ldo3 {
+			regulator-name = "vreg_l3c_2p8";
 			regulator-min-microvolt = <2800000>;
 			regulator-max-microvolt = <3540000>;
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
 		};
 
 		vreg_l4c_1p62: ldo4 {
+			regulator-name = "vreg_l4c_1p62";
 			regulator-min-microvolt = <1620000>;
 			regulator-max-microvolt = <3300000>;
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
 		};
 
 		vreg_l5c_1p62: ldo5 {
+			regulator-name = "vreg_l5c_1p62";
 			regulator-min-microvolt = <1620000>;
 			regulator-max-microvolt = <3300000>;
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
 		};
 
 		vreg_l6c_2p96: ldo6 {
+			regulator-name = "vreg_l6c_2p96";
 			regulator-min-microvolt = <1650000>;
 			regulator-max-microvolt = <3544000>;
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
 		};
 
 		vreg_l7c_3p0: ldo7 {
+			regulator-name = "vreg_l7c_3p0";
 			regulator-min-microvolt = <3000000>;
 			regulator-max-microvolt = <3544000>;
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
 		};
 
 		vreg_l8c_1p62: ldo8 {
+			regulator-name = "vreg_l8c_1p62";
 			regulator-min-microvolt = <1620000>;
 			regulator-max-microvolt = <2000000>;
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
 		};
 
 		vreg_l9c_2p96: ldo9 {
+			regulator-name = "vreg_l9c_2p96";
 			regulator-min-microvolt = <2700000>;
 			regulator-max-microvolt = <35440000>;
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
 		};
 
 		vreg_l10c_0p88: ldo10 {
+			regulator-name = "vreg_l10c_0p88";
 			regulator-min-microvolt = <720000>;
 			regulator-max-microvolt = <1050000>;
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
 		};
 
 		vreg_l11c_2p8: ldo11 {
+			regulator-name = "vreg_l11c_2p8";
 			regulator-min-microvolt = <2800000>;
 			regulator-max-microvolt = <3544000>;
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
 		};
 
 		vreg_l12c_1p65: ldo12 {
+			regulator-name = "vreg_l12c_1p65";
 			regulator-min-microvolt = <1650000>;
 			regulator-max-microvolt = <2000000>;
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
 		};
 
 		vreg_l13c_2p7: ldo13 {
+			regulator-name = "vreg_l13c_2p7";
 			regulator-min-microvolt = <2700000>;
 			regulator-max-microvolt = <3544000>;
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
 		};
 
 		vreg_bob_3p296: bob {
+			regulator-name = "vreg_bob_3p296";
 			regulator-min-microvolt = <3008000>;
 			regulator-max-microvolt = <3960000>;
 		};
-- 
2.25.1


^ permalink raw reply related

* [PATCH RESEND] arm64: dts: qcom: qcs6490-rb3gen2: enable PMIC Volume and Power buttons
From: Umang Chheda @ 2024-04-03 13:28 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-arm-msm, devicetree, linux-kernel, quic_uchheda

The Volume Down & Power buttons are controlled by the PMIC via
the PON hardware, and the Volume Up is connected to a PMIC gpio.

Enable the necessary hardware and setup the GPIO state for the
Volume Up gpio key.

Signed-off-by: Umang Chheda <quic_uchheda@quicinc.com>
---
 arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 37 ++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
index 63ebe0774f1d..73f6d18d2331 100644
--- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
+++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
@@ -9,6 +9,8 @@
 #define PM7250B_SID 8
 #define PM7250B_SID1 9
 
+#include <dt-bindings/input/linux-event-codes.h>
+#include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
 #include <dt-bindings/regulator/qcom,rpmh-regulator.h>
 #include "sc7280.dtsi"
 #include "pm7250b.dtsi"
@@ -39,6 +41,22 @@ chosen {
 		stdout-path = "serial0:115200n8";
 	};
 
+	gpio-keys {
+		compatible = "gpio-keys";
+
+		pinctrl-0 = <&key_vol_up_default>;
+		pinctrl-names = "default";
+
+		key-volume-up {
+			label = "Volume_up";
+			gpios = <&pm7325_gpios 6 GPIO_ACTIVE_LOW>;
+			linux,code = <KEY_VOLUMEUP>;
+			wakeup-source;
+			debounce-interval = <15>;
+			linux,can-disable;
+		};
+	};
+
 	reserved-memory {
 		xbl_mem: xbl@80700000 {
 			reg = <0x0 0x80700000 0x0 0x100000>;
@@ -471,6 +489,25 @@ &gcc {
 			   <GCC_WPSS_RSCP_CLK>;
 };
 
+&pm7325_gpios {
+	key_vol_up_default: key-vol-up-state {
+		pins = "gpio6";
+		function = "normal";
+		input-enable;
+		bias-pull-up;
+		qcom,drive-strength = <PMIC_GPIO_STRENGTH_LOW>;
+	};
+};
+
+&pon_pwrkey {
+	status = "okay";
+};
+
+&pon_resin {
+	linux,code = <KEY_VOLUMEDOWN>;
+	status = "okay";
+};
+
 &qupv3_id_0 {
 	status = "okay";
 };
-- 
2.25.1


^ permalink raw reply related

* Re: [RFC PATCH 5/6] watchdog: ROHM BD96801 PMIC WDG driver
From: Guenter Roeck @ 2024-04-03 13:26 UTC (permalink / raw)
  To: Matti Vaittinen
  Cc: Matti Vaittinen, Lee Jones, Wim Van Sebroeck, devicetree,
	linux-kernel, linux-watchdog
In-Reply-To: <1d956aab-2892-4a2b-a4b3-0a93504668eb@gmail.com>

On Wed, Apr 03, 2024 at 03:47:12PM +0300, Matti Vaittinen wrote:
> > 
> > Other watchdog drivers call emergency_restart() if the watchdog times out
> > and triggers an interrupt. Are you saying this won't work for this system ?
> > If so, please explain.
> > 
> 
> Thanks Guenter. If it works with systems using other devices, then it should
> work (to the same extent) with systems using this PMIC. Thanks.
> 

You might also consider to just call panic(). What is what we do if the
pretimeout panic governor is enabled.

That makes me wonder if it would make sense to introduce watchdog timeout
governors, similar to the existing pretimeout governors. Maybe if I ever
find the time to do it ...

Guenter

> I'll add the IRQ handling to next version - but it may take a while as I'm
> currently having some problems with the IRQs in general, and because I'll
> wait for feedback from Mark to the regulator part.
> 
> Yours,
> 	-- Matti
> 
> -- 
> Matti Vaittinen
> Linux kernel developer at ROHM Semiconductors
> Oulu Finland
> 
> ~~ When things go utterly wrong vim users can always type :help! ~~
> 

^ permalink raw reply

* Re: [PATCH v2 3/3] arm64: dts: qcom: msm8998: set qcom,no-msa-ready-indicator for wifi
From: Marc Gonzalez @ 2024-04-03 13:05 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Konrad Dybcio, Krzysztof Kozlowski, Kalle Valo, Jeff Johnson,
	ath10k, wireless, DT, MSM, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Pierre-Hugues Husson, Arnaud Vrac, Bjorn Andersson,
	Jami Kettunen, Jeffrey Hugo
In-Reply-To: <CAA8EJppeREj-0g9oGCzzKx5ywhg1mgmJR1q8yvXKN7N45do1Xg@mail.gmail.com>

On 02/04/2024 17:55, Dmitry Baryshkov wrote:

> On Tue, 2 Apr 2024 at 18:31, Marc Gonzalez wrote:
>
>> So, if I understand correctly, I take this to mean that I should:
>>
>> 1) DELETE the qcom,no-msa-ready-indicator boolean property,
>> 2) ADD a "qcom,msm8998-wifi" (name OK?) compatible,
> 
> I'd say, this is not correct. There is no "msm8998-wifi".

Can you explain what you mean by:
'There is no "msm8998-wifi".' ?

Do you mean that: this compatible string does not exist?
(I am proposing that it be created.)

Or do you mean that: "msm8998-wifi" is a bad name?


I meant to mimic these strings for various sub-blocks:

arch/arm64/boot/dts/qcom/msm8998.dtsi:          compatible = "qcom,msm8998-rpm-proc", "qcom,rpm-proc";
arch/arm64/boot/dts/qcom/msm8998.dtsi:                                  compatible = "qcom,msm8998-rpmpd";
arch/arm64/boot/dts/qcom/msm8998.dtsi:                  compatible = "qcom,msm8998-qfprom", "qcom,qfprom";
arch/arm64/boot/dts/qcom/msm8998.dtsi:                  compatible = "qcom,msm8998-tsens", "qcom,tsens-v2";
arch/arm64/boot/dts/qcom/msm8998.dtsi:                  compatible = "qcom,msm8998-tsens", "qcom,tsens-v2";
arch/arm64/boot/dts/qcom/msm8998.dtsi:                  compatible = "qcom,msm8998-smmu-v2", "qcom,smmu-v2";
arch/arm64/boot/dts/qcom/msm8998.dtsi:                  compatible = "qcom,msm8998-smmu-v2", "qcom,smmu-v2";
arch/arm64/boot/dts/qcom/msm8998.dtsi:                  compatible = "qcom,msm8998-qmp-pcie-phy";
arch/arm64/boot/dts/qcom/msm8998.dtsi:                  compatible = "qcom,msm8998-ufshc", "qcom,ufshc", "jedec,ufs-2.0";
arch/arm64/boot/dts/qcom/msm8998.dtsi:                  compatible = "qcom,msm8998-qmp-ufs-phy";
arch/arm64/boot/dts/qcom/msm8998.dtsi:                  compatible = "qcom,msm8998-tcsr", "syscon";
arch/arm64/boot/dts/qcom/msm8998.dtsi:                  compatible = "qcom,msm8998-tcsr", "syscon";
arch/arm64/boot/dts/qcom/msm8998.dtsi:                  compatible = "qcom,msm8998-pinctrl";
arch/arm64/boot/dts/qcom/msm8998.dtsi:                  compatible = "qcom,msm8998-mss-pil";
arch/arm64/boot/dts/qcom/msm8998.dtsi:                  compatible = "qcom,msm8998-smmu-v2", "qcom,smmu-v2",
arch/arm64/boot/dts/qcom/msm8998.dtsi:                  compatible = "qcom,msm8998-gpucc";
arch/arm64/boot/dts/qcom/msm8998.dtsi:                  compatible = "qcom,msm8998-slpi-pas";
arch/arm64/boot/dts/qcom/msm8998.dtsi:                  compatible = "qcom,msm8998-dwc3", "qcom,dwc3";
arch/arm64/boot/dts/qcom/msm8998.dtsi:                  compatible = "qcom,msm8998-qmp-usb3-phy";
arch/arm64/boot/dts/qcom/msm8998.dtsi:                  compatible = "qcom,msm8998-qusb2-phy";
arch/arm64/boot/dts/qcom/msm8998.dtsi:                  compatible = "qcom,msm8998-sdhci", "qcom,sdhci-msm-v4";
arch/arm64/boot/dts/qcom/msm8998.dtsi:                  compatible = "qcom,msm8998-mdss";
arch/arm64/boot/dts/qcom/msm8998.dtsi:                          compatible = "qcom,msm8998-dpu";
arch/arm64/boot/dts/qcom/msm8998.dtsi:                          compatible = "qcom,msm8998-dsi-ctrl", "qcom,mdss-dsi-ctrl";
arch/arm64/boot/dts/qcom/msm8998.dtsi:                          compatible = "qcom,msm8998-dsi-ctrl", "qcom,mdss-dsi-ctrl";
arch/arm64/boot/dts/qcom/msm8998.dtsi:                  compatible = "qcom,msm8998-venus";
arch/arm64/boot/dts/qcom/msm8998.dtsi:                  compatible = "qcom,msm8998-smmu-v2", "qcom,smmu-v2";
arch/arm64/boot/dts/qcom/msm8998.dtsi:                  compatible = "qcom,msm8998-adsp-pas";
arch/arm64/boot/dts/qcom/msm8998.dtsi:                  compatible = "qcom,msm8998-apcs-hmss-global",


And these strings in ath11k:

Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml:      - qcom,ipq8074-wifi
Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml:      - qcom,ipq6018-wifi
Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml:      - qcom,wcn6750-wifi
Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml:      - qcom,ipq5018-wifi


> I'd say, we should take a step back and actually verify how this was
> handled in the vendor kernel.

In our commercial product, we use the ath10k driver in the vendor kernel (v4.4 r38-rel).

It looks like Jeff has already performed the code analysis
wrt vendor vs mainline (including user-space tools).

Regards


^ permalink raw reply

* Re: [PATCH RFT 01/10] arm64: dts: microchip: sparx5: fix mdio reg
From: Steen Hegelund @ 2024-04-03 13:03 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Krzysztof Kozlowski, Nicolas Ferre, Claudiu Beznea, Rob Herring,
	Krzysztof Kozlowski, Lars Povlsen, Daniel Machon, UNGLinuxDriver,
	David S. Miller, Bjarni Jonasson, linux-arm-kernel, devicetree,
	linux-kernel
In-Reply-To: <20240402-drizzly-risotto-eac556bbe95b@spud>

Hi Connor,

On Tue, 2024-04-02 at 18:46 +0100, Conor Dooley wrote:
> [Some people who received this message don't often get email from
> conor@kernel.org. Learn why this is important at
> https://aka.ms/LearnAboutSenderIdentification ]
> 
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe


I will also be looking at the other RFT marked patches, I just have not
had the time to do it yet...

BR
Steen


^ permalink raw reply

* Re: 回复: [PATCH v2 2/2] ASoC: cdns: Add drivers of Cadence Multi-Channel I2S Controller
From: Pierre-Louis Bossart @ 2024-04-02 13:57 UTC (permalink / raw)
  To: Xingyu Wu, Liam Girdwood, Mark Brown, Claudiu Beznea,
	Jaroslav Kysela, Takashi Iwai, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	alsa-devel@alsa-project.org, linux-sound@vger.kernel.org
In-Reply-To: <NTZPR01MB0956BFADB4B3DA507D938F669F35A@NTZPR01MB0956.CHNPR01.prod.partner.outlook.cn>


>>
>>> +#define PERIODS_MIN		2
>>> +
>>> +static unsigned int cdns_i2s_pcm_tx(struct cdns_i2s_dev *dev,
>>> +				    struct snd_pcm_runtime *runtime,
>>> +				    unsigned int tx_ptr, bool *period_elapsed,
>>> +				    snd_pcm_format_t format)
>>> +{
>>> +	unsigned int period_pos = tx_ptr % runtime->period_size;
>>
>> not following what the modulo is for, usually it's modulo the buffer size?
> 
> This is to see if the new data is divisible by period_size and to determine whether
> it is enough for a period_size in the later loop.

That didn't answer to my question, the position is usually between
0..buffer_size.1.

Doing increments on a modulo value then comparisons as done below seems
rather questionable.
	
>>> +
>>> +		iowrite32(data[0], dev->base + CDNS_FIFO_MEM);
>>> +		iowrite32(data[1], dev->base + CDNS_FIFO_MEM);
>>> +		period_pos++;
>>> +		if (++tx_ptr >= runtime->buffer_size)
>>> +			tx_ptr = 0;
>>> +	}
>>> +
>>> +	*period_elapsed = period_pos >= runtime->period_size;
>>> +	return tx_ptr;
>>> +}

>>> +	pm_runtime_enable(&pdev->dev);
>>> +	if (pm_runtime_enabled(&pdev->dev))
>>> +		cdns_i2s_runtime_suspend(&pdev->dev);
>>
>> that sequence looks suspicious.... Why would you suspend immediately during the
>> probe? You're probably missing all the autosuspend stuff?
> 
> Since I have enabled clocks before, and the device is in the suspend state after
> pm_runtime_enable(), I need to disable clocks in cdns_i2s_runtime_suspend()
> to match the suspend state.

That is very odd on two counts
a) if you haven't enabled the clocks, why do you need to disbale them?
b) if you do a pm_runtime_enable(), then the branch if
(pm_runtime_enabled) is always true.


> 
>>
>>> +
>>> +	dev_dbg(&pdev->dev, "I2S supports %d stereo channels with %s.\n",
>>> +		i2s->max_channels, ((i2s->irq < 0) ? "dma" : "interrupt"));
>>> +
>>> +	return 0;
>>> +
>>> +err:
>>> +	return ret;
>>> +}
>>> +
>>> +static int cdns_i2s_remove(struct platform_device *pdev) {
>>> +	pm_runtime_disable(&pdev->dev);
>>> +	if (!pm_runtime_status_suspended(&pdev->dev))
>>> +		cdns_i2s_runtime_suspend(&pdev->dev);
>>
>> ... and this one too. Once you've disabled pm_runtime, checking the status is
>> irrelevant...
> 
> I think the clocks need to be always enabled after probe if disable pm_runtime,
> and should be disabled when remove. This will do that.

if you are disabling pm_runtime, then the pm_runtime state becames invalid.
When pm_runtime_disable() is added in remove operations, it's mainly to
prevent the device from suspending.


^ permalink raw reply

* Re: [RFC PATCH 5/6] watchdog: ROHM BD96801 PMIC WDG driver
From: Matti Vaittinen @ 2024-04-03 12:47 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Matti Vaittinen, Lee Jones, Wim Van Sebroeck, devicetree,
	linux-kernel, linux-watchdog
In-Reply-To: <d2ab33e6-4d3e-472a-b4d7-b703955989ba@roeck-us.net>

On 4/3/24 15:41, Guenter Roeck wrote:
> On Wed, Apr 03, 2024 at 09:34:35AM +0300, Matti Vaittinen wrote:
>> Hi Guenter,
>>
>> First of all, thanks for the review. It was quick! Especially when we speak
>> of a RFC series. Very much appreciated.
>>
>> On 4/2/24 20:11, Guenter Roeck wrote:
>>> On Tue, Apr 02, 2024 at 04:11:41PM +0300, Matti Vaittinen wrote >> +static int init_wdg_hw(struct wdtbd96801 *w)
>>>> +{
>>>> +	u32 hw_margin[2];
>>>> +	int count, ret;
>>>> +	u32 hw_margin_max = BD96801_WDT_DEFAULT_MARGIN, hw_margin_min = 0;
>>>> +
>>>> +	count = device_property_count_u32(w->dev->parent, "rohm,hw-timeout-ms");
>>>> +	if (count < 0 && count != -EINVAL)
>>>> +		return count;
>>>> +
>>>> +	if (count > 0) {
>>>> +		if (count > ARRAY_SIZE(hw_margin))
>>>> +			return -EINVAL;
>>>> +
>>>> +		ret = device_property_read_u32_array(w->dev->parent,
>>>> +						     "rohm,hw-timeout-ms",
>>>> +						     &hw_margin[0], count);
>>>> +		if (ret < 0)
>>>> +			return ret;
>>>> +
>>>> +		if (count == 1)
>>>> +			hw_margin_max = hw_margin[0];
>>>> +
>>>> +		if (count == 2) {
>>>> +			hw_margin_max = hw_margin[1];
>>>> +			hw_margin_min = hw_margin[0];
>>>> +		}
>>>> +	}
>>>> +
>>>> +	ret = bd96801_set_wdt_mode(w, hw_margin_max, hw_margin_min);
>>>> +	if (ret)
>>>> +		return ret;
>>>> +
>>>> +	ret = device_property_match_string(w->dev->parent, "rohm,wdg-action",
>>>> +					   "prstb");
>>>> +	if (ret >= 0) {
>>>> +		ret = regmap_update_bits(w->regmap, BD96801_REG_WD_CONF,
>>>> +				 BD96801_WD_ASSERT_MASK,
>>>> +				 BD96801_WD_ASSERT_RST);
>>>> +		return ret;
>>>> +	}
>>>> +
>>>> +	ret = device_property_match_string(w->dev->parent, "rohm,wdg-action",
>>>> +					   "intb-only");
>>>> +	if (ret >= 0) {
>>>> +		ret = regmap_update_bits(w->regmap, BD96801_REG_WD_CONF,
>>>> +				 BD96801_WD_ASSERT_MASK,
>>>> +				 BD96801_WD_ASSERT_IRQ);
>>>> +		return ret;
>>>> +	}
>>>
>>> I don't see the devicetree bindings documented in the series.
>>
>> Seems like I have missed this WDG binding. But after reading your comment
>> below, I am wondering if I should just drop the binding and default to
>> "prstb" (shutdown should the feeding be skipped) - and leave the "intb-only"
>> case for one who actually needs such.
>>
>>> I am also a bit surprised that the interrupt isn't handled in the driver.
>>> Please explain.
>>
>> Basically, I just had no idea what the IRQ should do in the generic case. If
>> we get an interrupt, it means the WDG feeding has failed. My thinking is
>> that, what should happen is forced reset. I don't see how that can be done
>> in reliably manner from an IRQ handler.
>>
>> When the "prstb WDG action" is set (please, see the above DT binding
>> handling), the PMIC shall shut down power outputs. This should get the
>> watchdog's job done.
>>
>> With the "intb-only"-option, PMIC will not turn off the power. I'd expect
>> there to be some external HW connection which handles the reset by HW.
>>
>> After all this being said, I wonder if I should just unconditionally
>> configure the PMIC to always turn off the power (prstb option) should the
>> feeding fail? Or do someone have some suggestion what the IRQ handler should
>> do (except maybe print an error msg)?
>>
> 
> Other watchdog drivers call emergency_restart() if the watchdog times out
> and triggers an interrupt. Are you saying this won't work for this system ?
> If so, please explain.
> 

Thanks Guenter. If it works with systems using other devices, then it 
should work (to the same extent) with systems using this PMIC. Thanks.

I'll add the IRQ handling to next version - but it may take a while as 
I'm currently having some problems with the IRQs in general, and because 
I'll wait for feedback from Mark to the regulator part.

Yours,
	-- Matti

-- 
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~


^ permalink raw reply

* Re: [RFC PATCH 5/6] watchdog: ROHM BD96801 PMIC WDG driver
From: Guenter Roeck @ 2024-04-03 12:41 UTC (permalink / raw)
  To: Matti Vaittinen
  Cc: Matti Vaittinen, Lee Jones, Wim Van Sebroeck, devicetree,
	linux-kernel, linux-watchdog
In-Reply-To: <279336b3-f28d-48ee-a10f-47abba7b0b89@gmail.com>

On Wed, Apr 03, 2024 at 09:34:35AM +0300, Matti Vaittinen wrote:
> Hi Guenter,
> 
> First of all, thanks for the review. It was quick! Especially when we speak
> of a RFC series. Very much appreciated.
> 
> On 4/2/24 20:11, Guenter Roeck wrote:
> > On Tue, Apr 02, 2024 at 04:11:41PM +0300, Matti Vaittinen wrote >> +static int init_wdg_hw(struct wdtbd96801 *w)
> > > +{
> > > +	u32 hw_margin[2];
> > > +	int count, ret;
> > > +	u32 hw_margin_max = BD96801_WDT_DEFAULT_MARGIN, hw_margin_min = 0;
> > > +
> > > +	count = device_property_count_u32(w->dev->parent, "rohm,hw-timeout-ms");
> > > +	if (count < 0 && count != -EINVAL)
> > > +		return count;
> > > +
> > > +	if (count > 0) {
> > > +		if (count > ARRAY_SIZE(hw_margin))
> > > +			return -EINVAL;
> > > +
> > > +		ret = device_property_read_u32_array(w->dev->parent,
> > > +						     "rohm,hw-timeout-ms",
> > > +						     &hw_margin[0], count);
> > > +		if (ret < 0)
> > > +			return ret;
> > > +
> > > +		if (count == 1)
> > > +			hw_margin_max = hw_margin[0];
> > > +
> > > +		if (count == 2) {
> > > +			hw_margin_max = hw_margin[1];
> > > +			hw_margin_min = hw_margin[0];
> > > +		}
> > > +	}
> > > +
> > > +	ret = bd96801_set_wdt_mode(w, hw_margin_max, hw_margin_min);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	ret = device_property_match_string(w->dev->parent, "rohm,wdg-action",
> > > +					   "prstb");
> > > +	if (ret >= 0) {
> > > +		ret = regmap_update_bits(w->regmap, BD96801_REG_WD_CONF,
> > > +				 BD96801_WD_ASSERT_MASK,
> > > +				 BD96801_WD_ASSERT_RST);
> > > +		return ret;
> > > +	}
> > > +
> > > +	ret = device_property_match_string(w->dev->parent, "rohm,wdg-action",
> > > +					   "intb-only");
> > > +	if (ret >= 0) {
> > > +		ret = regmap_update_bits(w->regmap, BD96801_REG_WD_CONF,
> > > +				 BD96801_WD_ASSERT_MASK,
> > > +				 BD96801_WD_ASSERT_IRQ);
> > > +		return ret;
> > > +	}
> > 
> > I don't see the devicetree bindings documented in the series.
> 
> Seems like I have missed this WDG binding. But after reading your comment
> below, I am wondering if I should just drop the binding and default to
> "prstb" (shutdown should the feeding be skipped) - and leave the "intb-only"
> case for one who actually needs such.
> 
> > I am also a bit surprised that the interrupt isn't handled in the driver.
> > Please explain.
> 
> Basically, I just had no idea what the IRQ should do in the generic case. If
> we get an interrupt, it means the WDG feeding has failed. My thinking is
> that, what should happen is forced reset. I don't see how that can be done
> in reliably manner from an IRQ handler.
> 
> When the "prstb WDG action" is set (please, see the above DT binding
> handling), the PMIC shall shut down power outputs. This should get the
> watchdog's job done.
> 
> With the "intb-only"-option, PMIC will not turn off the power. I'd expect
> there to be some external HW connection which handles the reset by HW.
> 
> After all this being said, I wonder if I should just unconditionally
> configure the PMIC to always turn off the power (prstb option) should the
> feeding fail? Or do someone have some suggestion what the IRQ handler should
> do (except maybe print an error msg)?
> 

Other watchdog drivers call emergency_restart() if the watchdog times out
and triggers an interrupt. Are you saying this won't work for this system ?
If so, please explain.

Thanks,
Guenter

^ permalink raw reply

* Re: [PATCH] media: mediatek: vcodec: fix the error sizeimage for 10bit bitstream
From: Sebastian Fricke @ 2024-04-03 12:39 UTC (permalink / raw)
  To: Yunfei Dong
  Cc: Nícolas F . R . A . Prado, Nicolas Dufresne, Hans Verkuil,
	AngeloGioacchino Del Regno, Benjamin Gaignard, Nathan Hebert,
	Hsin-Yi Wang, Fritz Koenig, Daniel Vetter, Steve Cho, linux-media,
	devicetree, linux-kernel, linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group
In-Reply-To: <20240403093018.13168-1-yunfei.dong@mediatek.com>

Hey Yunfei,

On 03.04.2024 17:30, Yunfei Dong wrote:
>The sizeimage of each plane are calculated the same way for 8bit and

s/The sizeimage of each plane are/The sizeimage for each plane is/

>10bit bitstream. Need to enlarge the sizeimage with simeimage*5/4 for
>10bit bitstream when try and set fmt.

s/bitstream/bistreams/
s/Need to enlarge the sizeimage with simeimage*5/4 for 10bit bitstream when try and set fmt./
   Scale up the sizeimage by 25% for 10-bit bitstreams in try_fmt./

>
>Fixes: 9d86be9bda6c ("media: mediatek: vcodec: Add driver to support 10bit")
>Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
>---
> .../mediatek/vcodec/decoder/mtk_vcodec_dec.c  | 47 ++++++++++++++-----
> 1 file changed, 34 insertions(+), 13 deletions(-)
>
>diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
>index 9107707de6c4..45209894f1fe 100644
>--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
>+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
>@@ -259,6 +259,7 @@ static int vidioc_try_fmt(struct mtk_vcodec_dec_ctx *ctx, struct v4l2_format *f,
> 		pix_fmt_mp->num_planes = 1;
> 		pix_fmt_mp->plane_fmt[0].bytesperline = 0;
> 	} else if (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
>+		unsigned int dram_y, dram_c, dram_y_10bit, dram_c_10bit;
> 		int tmp_w, tmp_h;
>
> 		/*
>@@ -280,22 +281,42 @@ static int vidioc_try_fmt(struct mtk_vcodec_dec_ctx *ctx, struct v4l2_format *f,
> 		    (pix_fmt_mp->height + 64) <= frmsize->max_height)
> 			pix_fmt_mp->height += 64;
>
>-		mtk_v4l2_vdec_dbg(0, ctx,
>-				  "before resize wxh=%dx%d, after resize wxh=%dx%d, sizeimage=%d",
>-				  tmp_w, tmp_h, pix_fmt_mp->width, pix_fmt_mp->height,
>-				  pix_fmt_mp->width * pix_fmt_mp->height);
>+		dram_y = pix_fmt_mp->width * pix_fmt_mp->height;
>+		dram_c = dram_y / 2;
>+
>+		dram_y_10bit = dram_y * 5 / 4;
>+		dram_c_10bit = dram_y_10bit / 2;

I'd skip the two 10 bit variables (dram_y_10bit & dram_c_10bit) and
instead do it like this:

```
   dram_stride = pix_fmt_mp->width;
   if (ctx->is_10bit_bitstream)
     dram_stride = dram_stride * 5 / 4;

   dram_y = dram_stride * pix_fmt_mp->height;
   dram_c = dram_y / 2;

   if (pix_fmt_mp->num_planes == 1) {
     pix_fmt_mp->plane_fmt[0].bytesperline = dram_stride;
     pix_fmt_mp->plane_fmt[0].sizeimage = dram_y + dram_c;
   } else {
     pix_fmt_mp->plane_fmt[0].bytesperline = dram_stride;
     pix_fmt_mp->plane_fmt[1].bytesperline = dram_stride;
     pix_fmt_mp->plane_fmt[0].sizeimage = dram_y;
     pix_fmt_mp->plane_fmt[1].sizeimage = dram_c;
     ...
   }
```

Also, why do you call all the variables dram?

Please this isn't tested, but shows the general direction to repeat a
lot less code.

Greetings,
Sebastian

>
> 		pix_fmt_mp->num_planes = fmt->num_planes;
>-		pix_fmt_mp->plane_fmt[0].sizeimage =
>-				pix_fmt_mp->width * pix_fmt_mp->height;
>-		pix_fmt_mp->plane_fmt[0].bytesperline = pix_fmt_mp->width;
>-
>-		if (pix_fmt_mp->num_planes == 2) {
>-			pix_fmt_mp->plane_fmt[1].sizeimage =
>-				(pix_fmt_mp->width * pix_fmt_mp->height) / 2;
>-			pix_fmt_mp->plane_fmt[1].bytesperline =
>-				pix_fmt_mp->width;
>+		if (pix_fmt_mp->num_planes == 1) {
>+			if (ctx->is_10bit_bitstream) {
>+				pix_fmt_mp->plane_fmt[0].bytesperline = pix_fmt_mp->width * 5 / 4;
>+				pix_fmt_mp->plane_fmt[0].sizeimage = dram_y_10bit + dram_c_10bit;
>+			} else {
>+				pix_fmt_mp->plane_fmt[0].bytesperline = pix_fmt_mp->width;
>+				pix_fmt_mp->plane_fmt[0].sizeimage = dram_y + dram_c;
>+			}
>+		} else {
>+			if (ctx->is_10bit_bitstream) {
>+				pix_fmt_mp->plane_fmt[0].bytesperline = pix_fmt_mp->width * 5 / 4;
>+				pix_fmt_mp->plane_fmt[1].bytesperline = pix_fmt_mp->width * 5 / 4;
>+
>+				pix_fmt_mp->plane_fmt[0].sizeimage = dram_y_10bit;
>+				pix_fmt_mp->plane_fmt[1].sizeimage = dram_c_10bit;
>+			} else {
>+				pix_fmt_mp->plane_fmt[0].bytesperline = pix_fmt_mp->width;
>+				pix_fmt_mp->plane_fmt[1].bytesperline = pix_fmt_mp->width;
>+
>+				pix_fmt_mp->plane_fmt[0].sizeimage = dram_y;
>+				pix_fmt_mp->plane_fmt[1].sizeimage = dram_c;
>+			}
> 		}
>+
>+		mtk_v4l2_vdec_dbg(0, ctx,
>+				  "before resize:%dx%d, after resize:%dx%d, sizeimage=0x%x_0x%x",
>+				  tmp_w, tmp_h, pix_fmt_mp->width, pix_fmt_mp->height,
>+				  pix_fmt_mp->plane_fmt[0].sizeimage,
>+				  pix_fmt_mp->plane_fmt[1].sizeimage);
> 	}
>
> 	pix_fmt_mp->flags = 0;
>-- 
>2.25.1
>

^ permalink raw reply

* Re: [PATCH v2 02/18] PCI: endpoint: Introduce pci_epc_map_align()
From: Kishon Vijay Abraham I @ 2024-04-03 12:33 UTC (permalink / raw)
  To: Damien Le Moal, Manivannan Sadhasivam, Lorenzo Pieralisi,
	Kishon Vijay Abraham I, Shawn Lin, Krzysztof Wilczyński,
	Bjorn Helgaas, Heiko Stuebner, linux-pci, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, devicetree
  Cc: linux-rockchip, linux-arm-kernel, Rick Wertenbroek,
	Wilfred Mallawa, Niklas Cassel
In-Reply-To: <20240330041928.1555578-3-dlemoal@kernel.org>

Hi Damien,

On 3/30/2024 9:49 AM, Damien Le Moal wrote:
> Some endpoint controllers have requirements on the alignment of the
> controller physical memory address that must be used to map a RC PCI
> address region. For instance, the rockchip endpoint controller uses
> at most the lower 20 bits of a physical memory address region as the
> lower bits of an RC PCI address. For mapping a PCI address region of
> size bytes starting from pci_addr, the exact number of address bits
> used is the number of address bits changing in the address range
> [pci_addr..pci_addr + size - 1].
> 
> For this example, this creates the following constraints:
> 1) The offset into the controller physical memory allocated for a
>     mapping depends on the mapping size *and* the starting PCI address
>     for the mapping.
> 2) A mapping size cannot exceed the controller windows size (1MB) minus
>     the offset needed into the allocated physical memory, which can end
>     up being a smaller size than the desired mapping size.
> 
> Handling these constraints independently of the controller being used in
> a PCI EP function driver is not possible with the current EPC API as
> it only provides the ->align field in struct pci_epc_features.
> Furthermore, this alignment is static and does not depend on a mapping
> pci address and size.
> 
> Solve this by introducing the function pci_epc_map_align() and the
> endpoint controller operation ->map_align to allow endpoint function
> drivers to obtain the size and the offset into a controller address
> region that must be used to map an RC PCI address region. The size
> of the physical address region provided by pci_epc_map_align() can then
> be used as the size argument for the function pci_epc_mem_alloc_addr().
> The offset into the allocated controller memory can be used to
> correctly handle data transfers. Of note is that pci_epc_map_align() may
> indicate upon return a mapping size that is smaller (but not 0) than the
> requested PCI address region size. For such case, an endpoint function
> driver must handle data transfers in fragments.
> 
> The controller operation ->map_align is optional: controllers that do
> not have any address alignment constraints for mapping a RC PCI address
> region do not need to implement this operation. For such controllers,
> pci_epc_map_align() always returns the mapping size as equal
> to the requested size and an offset equal to 0.
> 
> The structure pci_epc_map is introduced to represent a mapping start PCI
> address, size and the size and offset into the controller memory needed
> for mapping the PCI address region.
> 
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> ---
>   drivers/pci/endpoint/pci-epc-core.c | 66 +++++++++++++++++++++++++++++
>   include/linux/pci-epc.h             | 33 +++++++++++++++
>   2 files changed, 99 insertions(+)
> 
> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
> index 754afd115bbd..37758ca91d7f 100644
> --- a/drivers/pci/endpoint/pci-epc-core.c
> +++ b/drivers/pci/endpoint/pci-epc-core.c
> @@ -433,6 +433,72 @@ void pci_epc_unmap_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>   }
>   EXPORT_SYMBOL_GPL(pci_epc_unmap_addr);
>   
> +/**
> + * pci_epc_map_align() - Get the offset into and the size of a controller memory
> + *			 address region needed to map a RC PCI address region
> + * @epc: the EPC device on which address is allocated
> + * @func_no: the physical endpoint function number in the EPC device
> + * @vfunc_no: the virtual endpoint function number in the physical function
> + * @pci_addr: PCI address to which the physical address should be mapped
> + * @size: the size of the mapping starting from @pci_addr
> + * @map: populate here the actual size and offset into the controller memory
> + *       that must be allocated for the mapping
> + *
> + * Invoke the controller map_align operation to obtain the size and the offset
> + * into a controller address region that must be allocated to map @size
> + * bytes of the RC PCI address space starting from @pci_addr.
> + *
> + * The size of the mapping that can be handled by the controller is indicated
> + * using the pci_size field of @map. This size may be smaller than the requested
> + * @size. In such case, the function driver must handle the mapping using
> + * several fragments. The offset into the controller memory for the effective
> + * mapping of the @pci_addr..@pci_addr+@map->pci_size address range is indicated
> + * using the map_ofst field of @map.
> + */
> +int pci_epc_map_align(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> +		      u64 pci_addr, size_t size, struct pci_epc_map *map)
> +{
> +	const struct pci_epc_features *features;
> +	size_t mask;
> +	int ret;
> +
> +	if (!pci_epc_function_is_valid(epc, func_no, vfunc_no))
> +		return -EINVAL;
> +
> +	if (!size || !map)
> +		return -EINVAL;
> +
> +	memset(map, 0, sizeof(*map));
> +	map->pci_addr = pci_addr;
> +	map->pci_size = size;
> +
> +	if (epc->ops->map_align) {
> +		mutex_lock(&epc->lock);
> +		ret = epc->ops->map_align(epc, func_no, vfunc_no, map);
> +		mutex_unlock(&epc->lock);
> +		return ret;
> +	}
> +
> +	/*
> +	 * Assume a fixed alignment constraint as specified by the controller
> +	 * features.
> +	 */
> +	features = pci_epc_get_features(epc, func_no, vfunc_no);
> +	if (!features || !features->align) {
> +		map->map_pci_addr = pci_addr;
> +		map->map_size = size;
> +		map->map_ofst = 0;
> +	}

The 'align' of pci_epc_features was initially added only to address the 
inbound ATU constraints. This is also added as comment in [1]. The PCI 
address restrictions (only fixed alignment constraint) were handled by 
the host side driver and depends on the connected endpoint device 
(atleast it was like that for pci_endpoint_test.c [2]).
So pci-epf-test.c used the 'align' in pci_epc_features only as part of 
pci_epf_alloc_space().

Though I have abused 'align' of pci_epc_features in pci-epf-ntb.c using 
it out of pci_epf_alloc_space(), I think we should keep the 'align' of 
pci_epc_features only within pci_epf_alloc_space() and controllers with 
any PCI address restrictions to implement ->map_align(). This could as 
well be done in a phased manner to let controllers implement 
->map_align() and then remove using  pci_epc_features in 
pci_epc_map_align(). Let me know what you think?

Thanks,
Kishon

[1] -> 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/pci-epc.h?h=v6.9-rc2#n187

[2] -> 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/misc/pci_endpoint_test.c?h=v6.9-rc2#n127
> +
> +	mask = features->align - 1;
> +	map->map_pci_addr = map->pci_addr & ~mask;
> +	map->map_ofst = map->pci_addr & mask;
> +	map->map_size = ALIGN(map->map_ofst + map->pci_size, features->align);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(pci_epc_map_align);
> +
>   /**
>    * pci_epc_map_addr() - map CPU address to PCI address
>    * @epc: the EPC device on which address is allocated
> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
> index cc2f70d061c8..8cfb4aaf2628 100644
> --- a/include/linux/pci-epc.h
> +++ b/include/linux/pci-epc.h
> @@ -32,11 +32,40 @@ pci_epc_interface_string(enum pci_epc_interface_type type)
>   	}
>   }
>   
> +/**
> + * struct pci_epc_map - information about EPC memory for mapping a RC PCI
> + *                      address range
> + * @pci_addr: start address of the RC PCI address range to map
> + * @pci_size: size of the RC PCI address range to map
> + * @map_pci_addr: RC PCI address used as the first address mapped
> + * @map_size: size of the controller memory needed for the mapping
> + * @map_ofst: offset into the controller memory needed for the mapping
> + * @phys_base: base physical address of the allocated EPC memory
> + * @phys_addr: physical address at which @pci_addr is mapped
> + * @virt_base: base virtual address of the allocated EPC memory
> + * @virt_addr: virtual address at which @pci_addr is mapped
> + */
> +struct pci_epc_map {
> +	phys_addr_t	pci_addr;
> +	size_t		pci_size;
> +
> +	phys_addr_t	map_pci_addr;
> +	size_t		map_size;
> +	phys_addr_t	map_ofst;
> +
> +	phys_addr_t	phys_base;
> +	phys_addr_t	phys_addr;
> +	void __iomem	*virt_base;
> +	void __iomem	*virt_addr;
> +};
> +
>   /**
>    * struct pci_epc_ops - set of function pointers for performing EPC operations
>    * @write_header: ops to populate configuration space header
>    * @set_bar: ops to configure the BAR
>    * @clear_bar: ops to reset the BAR
> + * @map_align: operation to get the size and offset into a controller memory
> + *             window needed to map an RC PCI address region
>    * @map_addr: ops to map CPU address to PCI address
>    * @unmap_addr: ops to unmap CPU address and PCI address
>    * @set_msi: ops to set the requested number of MSI interrupts in the MSI
> @@ -61,6 +90,8 @@ struct pci_epc_ops {
>   			   struct pci_epf_bar *epf_bar);
>   	void	(*clear_bar)(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>   			     struct pci_epf_bar *epf_bar);
> +	int	(*map_align)(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> +			    struct pci_epc_map *map);
>   	int	(*map_addr)(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>   			    phys_addr_t addr, u64 pci_addr, size_t size);
>   	void	(*unmap_addr)(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> @@ -234,6 +265,8 @@ int pci_epc_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>   		    struct pci_epf_bar *epf_bar);
>   void pci_epc_clear_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>   		       struct pci_epf_bar *epf_bar);
> +int pci_epc_map_align(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> +		      u64 pci_addr, size_t size, struct pci_epc_map *map);
>   int pci_epc_map_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>   		     phys_addr_t phys_addr,
>   		     u64 pci_addr, size_t size);


^ permalink raw reply


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